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

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added a reviewer: rsmith.

This is PR36536.

There are a few ways to reach Sema::ActOnStartOfFunctionDef with a null
Decl. Currently, the parser continues on to attempt to parse the
statements in the function body without pushing a function scope or
declaration context. However, lots of statement parsing logic relies on
getCurFunction() returning something reasonable. It turns out that
getCurFunction() will never return null today because of an optimization
where Sema pre-allocates one FunctionScopeIfno and reuses it when
possible. This goes wrong when something inside the function body causes
us to push another function scope, such as requiring an implicit
definition of a special member function. Reusing the state clears it
out, which will lead to bugs. In PR36536, we found that the SwitchStack
gets unbalanced, because we push a switch, clear out the stack, and then
try to pop a switch that isn't there.

As a follow-up, I plan to move the pre-allocated FunctionScopeInfo out
of the FunctionScopes stack. This means the FunctionScopes stack will
often be empty, which will make getCurFunction() assert. That's a
high-risk change, so I want to separate it out.


https://reviews.llvm.org/D43980

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/pr36536.cpp


Index: clang/test/SemaCXX/pr36536.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/pr36536.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -std=c++11 %s -verify -fno-spell-checking
+
+// These test cases are constructed to make clang call ActOnStartOfFunctionDef
+// with nullptr.
+
+struct ImplicitDefaultCtor1 {};
+struct Foo {
+  typedef int NameInClass;
+  void f();
+};
+namespace bar {
+// expected-error at +1 {{cannot define or redeclare 'f' here}}
+void Foo::f() {
+  switch (0) { case 0: ImplicitDefaultCtor1 o; }
+
+  // FIXME: If we improved our recovery to redeclare Foo::f in the wrong
+  // namespace, we could find the right type name here. Users probably run into
+  // 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'}}
+  NameInClass var;
+}
+} // namespace bar
+
+struct ImplicitDefaultCtor2 {};
+template <typename T> class TFoo { void f(); };
+// expected-error at +1 {{nested name specifier 'decltype(TFoo<T>())::'}}
+template <typename T> void decltype(TFoo<T>())::f() {
+  switch (0) { case 0: ImplicitDefaultCtor1 o; }
+}
+
+namespace tpl2 {
+struct ImplicitDefaultCtor3 {};
+template <class T1> class A {
+  template <class T2> class B {
+    void mf2();
+  };
+};
+template <class Y>
+template <>
+// expected-error at +1 {{nested name specifier 'A<Y>::B<double>::'}}
+void A<Y>::B<double>::mf2() {
+  switch (0) { case 0: ImplicitDefaultCtor3 o; }
+}
+}
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12406,8 +12406,13 @@
 
 Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
                                     SkipBodyInfo *SkipBody) {
-  if (!D)
+  if (!D) {
+    // Parsing the function declaration failed in some way. Push on a fake scope
+    // anyway so we can try to parse the function body.
+    PushFunctionScope();
     return D;
+  }
+
   FunctionDecl *FD = nullptr;
 
   if (FunctionTemplateDecl *FunTmpl = dyn_cast<FunctionTemplateDecl>(D))
@@ -12816,6 +12821,9 @@
       getCurFunction()->ObjCWarnForNoInitDelegation = false;
     }
   } else {
+    // Parsing the function declaration failed in some way. Pop the fake scope
+    // we pushed on.
+    PopFunctionScopeInfo(ActivePolicy, dcl);
     return nullptr;
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43980.136635.patch
Type: text/x-patch
Size: 2474 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180302/539e5a6e/attachment.bin>