[PATCH] D43980: Push a function scope when parsing function bodies without a declaration

[Nico Weber via Phabricator via cfe-commits]

thakis accepted this revision.
thakis added a comment.
This revision is now accepted and ready to land.

Fix LGTM, one optional comment below.



================
Comment at: clang/lib/Sema/SemaDecl.cpp:12412
+    // anyway so we can try to parse the function body.
+    PushFunctionScope();
     return D;
----------------
Feels a bit long-term risky since ActOnStartOfFunctionDef() and ActOnFinishFunctionBody() both need to know about this special-case invariant. Maybe it's worth to add a FakeFunctionScopeCount member to sema in +assert builds, and to increment that here, assert it's > 0 in the other place and decrement it there, and then assert it's 0 at end of TU?


================
Comment at: clang/test/SemaCXX/pr36536.cpp:19
+  // this when they forget to close a namespace, and we'd generate far fewer
+  // errors if names in Foo were in scope.
+  // expected-error at +1 {{unknown type name 'NameInClass'}}
----------------
Not 100% clear to me what the FIXME is here. Maybe "FIXME: We should improve our recovery to redeclare...." if that's what's meant.


https://reviews.llvm.org/D43980