alejandro-alvarez-sonarsource added inline comments.
================ Comment at: clang/lib/Parse/ParseDecl.cpp:2306-2310 // Otherwise things are very confused and we skip to recover. if (!isDeclarationSpecifier(ImplicitTypenameContext::No)) { - SkipUntil(tok::r_brace, StopAtSemi | StopBeforeMatch); - TryConsumeToken(tok::semi); + SkipMalformedDecl(); } } ---------------- aaron.ballman wrote: > Changes for our coding style. > > There seems to be some unfortunate interplay here though. Consider: > ``` > void bar() namespace foo { int i; } > > int main() { > foo::i = 12; > } > ``` > Calling `SkipMalformedDecl()` changes the behavior for this test case because > we don't recover as well. With your patch applied, this gives us two > diagnostics: > ``` > C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:1:11: error: > expected ';' after top level declarator > void bar() namespace foo { int i; } > ^ > ; > C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:4:3: error: > use of undeclared identifier 'foo' > foo::i = 12; > ^ > 2 errors generated. > ``` > If the `namespace` keyword is on its own line, then we recover gracefully and > don't emit the "use of undeclared identifier" warning. > > While this is technically a regression in behavior for this test case, I > think the overall changes are still an improvement. I suspect not a whole lot > of code puts `namespace` somewhere other than the start of a line (same for > `inline namespace` which has the same behavior with your patch). > Calling SkipMalformedDecl() changes the behavior for this test case because > we don't recover as well. With your patch applied, this gives us two > diagnostics: True. This can also be recovered by removing `Tok.isAtStartOfLine()` in line 2050. However, this has been around for a long time and would change the behavior of two tests inside `test/Parser/recovery.cpp`, although only because the broken comments contain the namespace keyword. Either case seems unlikely to me, so I think I'd lean toward not modifying `SkipMalformedDecl`. What do you think? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150258/new/ https://reviews.llvm.org/D150258 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits