Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved C backend and added line number option #238

Merged
merged 14 commits into from
Aug 27, 2019

Conversation

justinmeiners
Copy link
Contributor

@justinmeiners justinmeiners commented Oct 9, 2018

Summary

  • Use yylloc for robust line and character location tracking. This is used for error handler and to store line and character number on syntax tree, if the option is enabled.
  • Add string parsing functions to C backend.
  • Fix Flex regex patterns are incorrect. #237. Handle set subtraction for common cases such as char - ... using the ideas found in the JFlex backend.
  • Move RegToFlex.h from NoSTL module to C module since flex generation is in the C module, and all CPP backends reference that one.
  • Improve verbose error messages by associating a literal string with each token.
  • Fix skeleton generation bug C skeleton uses incorrect function names #239.

Questions

  • Some users may not care about tracking the character position, since this slows down the lexer. If you are concerned about this I could use yylineno instead when the RecordPosition options are disabled. This would only be referenced in the yyerror handler. What do you think?
  • CFtoCVisitSkel and CFtoCVisitSkelSTL are only for C++, not C. Would it be better to rename them to something like CFtoCPPVisitSkel and CFToSTLVisitSkel?
  • C currently has no destructors for the Absyn. Is this needed? EDIT Bison provides hooks for handling destruction of failed parses, but neither the C or C++ bison output uses them. This means all C or C++ parsers leak memory. This is a bigger issue than I can take care of. See C and C++ parsers leak memory on syntax errors #240
  • I find the convention of typedef Var_* Var to be extremely confusing, and I am not alone. Can I change this, or is compatibility important here. I understand if it is.

Todo

  • Modify the help string to list the new line number option.
  • verify that it passes the unit tests.
  • make sure the changes don't harm the C++ backends.
  • Anything else?

Justin Meiners added 14 commits October 2, 2018 17:20
- moved RegToFlex.hs from NoSTL to C module since flex source is in C.
- removed broken "#" handling of regex set subtraction.
- handles set substraction for special cases (just like Java JFLex)
- use yylloc for tracking line and character number
- C++ backends no longer uses the C one
- token pragmas were not following camel case correctly
while other code calling them was
- removed standards violating _p_ convention
@andreasabel
Copy link
Member

Sorry for missing this PR, and thanks for your work on it.
It is ready to be merged, meaning, are you satisfied with it in this stage?

Some users may not care about tracking the character position, since this slows down the lexer.
Well, usually the lexer is not the bottleneck. What magnitude of slow down are we talking about here? If it is only a fixed number of extra constant-time instructions per character, we need not worry.

(Your other questions could be taken later, as they are more about style.)

@justinmeiners
Copy link
Contributor Author

I got it to do what I wanted it to, but I am unsure if I am going about things the best way.
If it looks good to you, then go ahead.

However, if you want to address the issues in a different way, or only some of them, I understand.

Unfortunately, I have moved on from this project at work so I cannot work on any more changes to this. But, I am happy to discuss the issues.

@justinmeiners
Copy link
Contributor Author

justinmeiners commented May 14, 2019

I think the C backend could use a refactor from someone who understands the code better. If that is unlikely to happen in the near future, my changes may be helpful until then.

andreasabel added a commit that referenced this pull request Aug 27, 2019
…into pr238

See: #238

Conflicts:
  source/BNFC.cabal
  source/src/BNFC/Backend/C.hs
  source/src/BNFC/Backend/C/CFtoBisonC.hs
  source/src/BNFC/Backend/C/CFtoCAbs.hs
  source/src/BNFC/Backend/C/CFtoCSkel.hs
  source/src/BNFC/Backend/C/RegToFlex.hs
  source/src/BNFC/Options.hs

Changes to be committed:
  modified:   source/BNFC.cabal
  modified:   source/src/BNFC/Backend/C.hs
  modified:   source/src/BNFC/Backend/C/CFtoBisonC.hs
  modified:   source/src/BNFC/Backend/C/CFtoCAbs.hs
  modified:   source/src/BNFC/Backend/C/CFtoCPrinter.hs
  modified:   source/src/BNFC/Backend/C/CFtoCSkel.hs
  modified:   source/src/BNFC/Backend/C/CFtoFlexC.hs
  renamed:    source/src/BNFC/Backend/CPP/NoSTL/RegToFlex.hs -> source/src/BNFC/Backend/C/RegToFlex.hs
  modified:   source/src/BNFC/Backend/CPP/NoSTL/CFtoFlex.hs
  modified:   source/src/BNFC/Options.hs
andreasabel added a commit that referenced this pull request Aug 27, 2019
@andreasabel andreasabel merged commit 28580a9 into BNFC:master Aug 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flex regex patterns are incorrect.
2 participants