r326926 - Push a function scope when parsing function bodies without a declaration

[Nico Weber via cfe-commits]

On Wed, Mar 7, 2018 at 1:55 PM, Reid Kleckner via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: rnk
> Date: Wed Mar  7 10:55:10 2018
> New Revision: 326926
>
> URL: http://llvm.org/viewvc/llvm-project?rev=326926&view=rev
> Log:
> Push a function scope when parsing function bodies without a declaration
>
> Summary:
> 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 FunctionScopeInfo 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, and callers of getCurFunction() will need to check for
> null.
>
> Reviewers: thakis
>
> Subscribers: cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D43980
>
> Added:
>     cfe/trunk/test/SemaCXX/pr36536.cpp
> Modified:
>     cfe/trunk/lib/Sema/SemaDecl.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaDecl.cpp?rev=326926&r1=326925&r2=326926&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Mar  7 10:55:10 2018
> @@ -12406,8 +12406,13 @@ static void RebuildLambdaScopeInfo(CXXMe
>
>  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 @@ Decl *Sema::ActOnFinishFunctionBody(Decl
>        getCurFunction()->ObjCWarnForNoInitDelegation = false;
>      }
>    } else {
> +    // Parsing the function declaration failed in some way. Pop the fake
> scope
> +    // we pushed on.
> +    PopFunctionScopeInfo(ActivePolicy, dcl);
>      return nullptr;
>    }
>
>
> Added: cfe/trunk/test/SemaCXX/pr36536.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> SemaCXX/pr36536.cpp?rev=326926&view=auto
> ============================================================
> ==================
> --- cfe/trunk/test/SemaCXX/pr36536.cpp (added)
> +++ cfe/trunk/test/SemaCXX/pr36536.cpp Wed Mar  7 10:55:10 2018
> @@ -0,0 +1,44 @@
> +// 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 {
> +// FIXME: Improved our recovery to make this a redeclaration of Foo::f,
>

(nit: s/Improved/Improve/)


> +// even though this is in the wrong namespace. That will allow name
> lookup to
> +// find NameInClass below. Users are likely to hit this when they forget
> to
> +// close namespaces.
> +// expected-error at +1 {{cannot define or redeclare 'f' here}}
> +void Foo::f() {
> +  switch (0) { case 0: ImplicitDefaultCtor1 o; }
> +  // 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; }
> +}
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180307/5e2fb826/attachment.html>