Nothing Special   »   [go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

Improved C backend and added line number option #238

Merged
merged 14 commits into from
Aug 27, 2019
Merged

Improved C backend and added line number option #238

merged 14 commits into from
Aug 27, 2019

Conversation

justinmeiners
Copy link
Contributor
@justinmeiners justinmeiners commented Oct 9, 2018 •
edited
Loading

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
started on C line numbers
e66773f
C line and char numbers and bug fixes
4537989 
- 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
added C line number options to help listing
1ff4fdd
use preincrement in location tracker
053675e
added bison references in a few comments
3ed2cfd
adjust C doctests
b52c8cb
added C++ specific version of comments lex
b8cf658 
- C++ backends no longer uses the C one
modified doctests to fit changes
735aa59
added string parsing to C backend
73b10de
improved verbose error display for C
1cf335e
restart lloc when parse starts
bf6a13c
removed build warning for C
2197aa7
fixed a bug in skeleton generation
64e7790 
- token pragmas were not following camel case correctly
while other code calling them was
- removed standards violating _p_ convention
simplify name formatting
28580a9
@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 •
edited
Loading

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.

This was referenced Aug 27, 2019
Closed
Closed
andreasabel added a commit that referenced this pull request Aug 27, 2019
@andreasabel
Merge branch 'c-line-numbers' of https://github.com/justinmeiners/bnfc …
b818068 
…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
[ changelog ] Justin Meiners closed #237, #238, #239
2f93d35
@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
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Flex regex patterns are incorrect.
2 participants
@justinmeiners @andreasabel