Community
Participate
Working Groups
Code that #defines macros with trailing single-line comments will be pasted, with comments, into expansion contexts. D'oh! #define LIT 1 // my value int func(int x) { } int main() { return func(LIT); // fails to parse }
Created attachment 60671 [details] patch and test case
Took a look... looks good to me... Any objections from anyone to committing this?
I have a look.
I don't get an error with the example in comment 0. The reason is probably that the scanner does not pop contexts when skipping a line comment.
(In reply to comment #4) > I don't get an error with the example in comment 0. The reason is probably that > the scanner does not pop contexts when skipping a line comment. Ok, while technically the macro expansion should neither contain single-line nor multi-line comments it has no effect on parsing the code. --> changing severity. A fix should remove both kinds of comments.
I disagree that the severity is minor. At least in 4.0M5, this bug caused the attached unit tests to fail, meaning the parser was affected. The end result of this is that an editor containing uses of such macros shows an Outline that does not include the given function or any others past it. The scanner is storing the definition of the macro "LIT" as "1 // my value". Then substituting this text causes the func(LIT) call to be parsed as: func(1 // my value); which looks like: func(1 Meaning the trailing ");" tokens are ignored, and the function call looks like a syntax error. (Or maybe I misunderstand and the literal '/' and '/' and 'my' and 'value' tokens are being scanned -- in either case, it's a problem, and does affect parsing the code.) My patch removes the comments from the macro definition beforehand. Multi-line trailing (and embedded) comments were already being removed, but not single-line ones.
(In reply to comment #6) > I disagree that the severity is minor. At least in 4.0M5, this bug caused the > attached unit tests to fail, meaning the parser was affected. The end result > of this is that an editor containing uses of such macros shows an Outline that > does not include the given function or any others past it. The unit test does not test whether the parser reports a syntax error in this case. The line comment in the macro expansion is not right, but it does not create a syntax error when expanded, therefore it is not a big issue. Did you actually try your example? There may be another problem why the outline is missing something.
Created attachment 62226 [details] screenshot
I have tested with M5, works fine.
Ok, sorry for the confusion. (1) We are using a patched 4.0M5. Maybe the underlying issue has been fixed elsewhere since then. I did see the scanner from the HEAD neglected to remove single-line comments from macros as well, so I thought the same problem would arise there. But it appears from Anton's comment that comments may be expected to be handled inside macro expansions. (2) The vastly simplified case shown in the summary indeed does NOT show a problem when the trailing comment is left in place. Sorry. My reasoning was incorrect in presuming what a simpler example would look like. The second case in the attached unit test, however, which is based on the actual code our customers reported the bug against, does show the problem for me. Can anyone verify this? Taking out my patch, the failure occurs when parsing the "actual reduced test case" in this test: org.eclipse.cdt.internal.core.parser.ParserException: FAILURE at org.eclipse.cdt.core.parser.tests.ast2.AST2BaseTest.parse(AST2BaseTest.java:140) at org.eclipse.cdt.core.parser.tests.ast2.AST2Tests.testMacroCommentsBug_177154(AST2Tests.java:3558) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:585) at junit.framework.TestCase.runTest(TestCase.java:154) at junit.framework.TestCase.runBare(TestCase.java:127) at junit.framework.TestResult$1.protect(TestResult.java:106) at junit.framework.TestResult.runProtected(TestResult.java:124) at junit.framework.TestResult.run(TestResult.java:109) at junit.framework.TestCase.run(TestCase.java:118) at org.eclipse.cdt.core.testplugin.util.BaseTestCase.run(BaseTestCase.java:85) at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:128) at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:460) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:673) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:386) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:196)
Now, I see the problem. If a macro with a line comment is itself used in an expansion, then the problem arises. Thanks for the clarification!
Patch applied.
Created attachment 64178 [details] another unit test
Our customers found another instance of this bug. If the single line comment is placed on the macro definition, the parser enters an infinite loop. If the comment is absent, the parser succeeds. The attached test demonstrates this against today's HEAD.
Interestingly the endless loop is somehow caused by the trailing '\t' (which is not removed from the macro definition). If the tab is changed to spaces, there is no problem. I still don't know why the tab causes an endless loop in the parser, but I fixed the comment removal such that tabs are replaced by spaces (outside string and character literals) anyway.
(In reply to comment #15) > Interestingly the endless loop is somehow caused by the trailing '\t' (which is > not removed from the macro definition). If the tab is changed to spaces, there > is no problem. During an investigation into bug 179383 I discovered that any runtime exception thrown by doFetchToken() in BaseScanner has the potential to cause an infinite loop in the parser. Maybe you are seeing a symptom of that.
I also want to mention that the LALR based C99 parser that I'm working on (bug 173110) has a lexer that removes all comments before passing the tokens to the preprocessor. This means that things like comments and spaces in macros are ignored by the preprocessor, so the kinds of problems described in this bug will not occur.
