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

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Thanks!

John touched this code last in https://reviews.llvm.org/rL112038 in 2010, so maybe he has some thoughts on how to clean this and the follow-up. I think I'll land this as is since it fixes the crash and we can discuss more improvements in https://reviews.llvm.org/D44039.



================
Comment at: clang/lib/Sema/SemaDecl.cpp:12412
+    // anyway so we can try to parse the function body.
+    PushFunctionScope();
     return D;
----------------
thakis wrote:
> 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?
Well, it's more like these early returns break the invariant that `ActOnStartOfFunctionDef` pushes a function scope. We could try to clean it up with an RAII helper or an layer of function call that ensures that we always push and pop on start and finish, but I'll leave that for the follow-up.


================
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'}}
----------------
thakis wrote:
> 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.
I rewrote this to clarify things.


Repository:
  rC Clang

https://reviews.llvm.org/D43980