[build, eiffel] Fix for #4694. #4695
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add check to make sure there are no symbolic links in checkin.
Unknown what 2nd grammar was for.
* Make tests simple. Do not include png's of the parse tree--completely untestable, unverifiable. Remove all other files of unknow purpose. Move Eiffel tests to examples/ directory. Update testing script to correct for extra popd.
Added many explicit lexer rules for split grammar string literals.
Parse should not be dependent on the number keyword counts.
Contributor
Author
|
I'm going to add in a new version of trgen so that I can preserve the EiffelGrammar.g4 (no skip tokens) example. |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes #4694. Both the build and the Eiffel grammar are broken.
Background
Windows does not support symbolic links. A comprehensive search was done to find symbolic links in the repo and to remove them (
for f in `find . -type f | fgrep -v '.git'`; do v=`git ls-files --stage $f | awk '{print $1}'`; if [ "$v" == 120000 ]; then echo $f; fi; done, or alternatively on Linuxfind . -type l). Currently, there are two grammars with symbolic links: vhdl2008 and eiffel. The vhdl2008 grammar is being fixed in PR #4693. The other is the Eiffel grammar, which is being corrected with this PR.Changes to the build
The script
_scripts/test-static-checks.shwas modified to check for symbolic linked files in any future PRs. If there are any, the build fails. The check only runs on Ubuntu and checks both what Bashfindandgitreport.The Eiffel grammar did have a symbolic link file. But, the grammar was never integrated into the build correctly, so it didn't matter. This PR removes the file. Conveniently, the "no-symbolic-links" test added in this PR validates that the grammar has no symbolic links.
In order to support the changes to the Eiffel grammar, I needed to update the version of the Trash Toolkit to 0.23.28. This version fixes some problems with the analysis of "top-level" .g4's.
Changes to the Eiffel grammar
I removed the specialized parser driver program from the example/ test files directory. The program does not function as a regression tester. In addition, the app includes another Antlr grammar that overrides a couple of lexer rules: EiffelGrammar.g4 overrides the WhiteSpace and Comment lexer rules with channels. I moved the specialized grammars to the main directory, updated Trash trgen tool to test both grammar pairs.
The examples and the directory structure for each Eiffel application was kept, but I removed the .png file because they are not really that useful for regression testing--best to use Trash to display (
trgen -t CSharp; cd Generated-CSharp; make; trparse ../examples/application.e | trtree) and test the parse trees for invariant properties. I added a few more examples and a readme.md.A pom.xml was added for testing the Eiffel grammar using Maven, but it only tests the EiffelParser.g4/EiffelLexer.g4 pair.
The Eiffel grammar was modified as per comment.
The Eiffel grammar is now target agnostic, and the CSharp and Java ports implemented. The whole point of having a grammar in target-agnostic form is so that the grammar can be ported to other targets. With the CSharp target, it can be tested using the Trash Toolkit. See the next section for such an analysis.
Ambiguity
The Eiffel grammar is ambiguous and has large max-k's. For example, for input containing the substring
"i := 0", here are two parse trees that show ambiguity.In this grammar, the parser cannot distinguish between an assignment and an assigner_call within
instructionfor the input string"i := 0". According to the spec,assigner_callis chosen overassignmentwhen:"[t]he Equivalent Dot Form of target is a qualified Object_call whose feature has an assigner command."In order to make this distinction, a symbol table must be added to the grammar.Ambiguity is a sign of a poorly designed grammar because the parse tree depends on the semantics of the language. Rule
assigner_callsubsumesassignment, so one could just removeassignmentand the input would still parse--and more efficiently. You would need to follow up parsing to distinguish between the two different interpretations. But, the parse tree would be the same regardless.