[PATCH] PR22931 - When cloning scopes, reset the scope in Sema instead of using the cloned scope as the current scope

[Delesley Hutchins]

Nice catch!  LGTM.   BTW, I'm surprised it has taken so long for this bug
to manifest, since we should have lots of code with multiple
late parsed attributes.  When did it show up?



On Tue, Mar 17, 2015 at 8:12 PM, Richard Trieu <rtrieu at google.com> wrote:

> Hi delesley,
>
> Fix for PR22931, assertion failure with attributes and templates.
>
> When cloning a scope, the constructor of LocalInstantiationScope will set
> the current scope in Sema to be that new scope.  However, cloning scopes
> doesn't change what the current scope should be.  This manifests when
> multiple late parsed attributed are used.  The first attribute has a
> properly cloned scope, but the other attributes get the wrong scope.  The
> assertion in PR22931 is triggered when a Decl is looked up within an
> incorrect scope and is not found.
>
> http://reviews.llvm.org/D8407
>
> Files:
>   include/clang/Sema/Template.h
>   test/SemaCXX/warn-thread-safety-negative.cpp
>
> Index: include/clang/Sema/Template.h
> ===================================================================
> --- include/clang/Sema/Template.h
> +++ include/clang/Sema/Template.h
> @@ -273,6 +273,11 @@
>      /// outermost scope.
>      LocalInstantiationScope *cloneScopes(LocalInstantiationScope
> *Outermost) {
>        if (this == Outermost) return this;
> +
> +      // Save the current scope from SemaRef since the
> LocalInstantiationScope
> +      // will overwrite it on construction
> +      LocalInstantiationScope *oldScope =
> SemaRef.CurrentInstantiationScope;
> +
>        LocalInstantiationScope *newScope =
>          new LocalInstantiationScope(SemaRef, CombineWithOuterScope);
>
> @@ -299,6 +304,8 @@
>            newScope->ArgumentPacks.push_back(NewPack);
>          }
>        }
> +      // Restore the saved scope to SemaRef
> +      SemaRef.CurrentInstantiationScope = oldScope;
>        return newScope;
>      }
>
> Index: test/SemaCXX/warn-thread-safety-negative.cpp
> ===================================================================
> --- test/SemaCXX/warn-thread-safety-negative.cpp
> +++ test/SemaCXX/warn-thread-safety-negative.cpp
> @@ -102,3 +102,20 @@
>  };
>
>  }  // end namespace SimpleTest
> +
> +namespace DoubleAttribute {
> +
> +struct Foo {
> +  Mutex &mutex();
> +};
> +
> +template <typename A>
> +class TemplateClass {
> +  template <typename B>
> +  static void Function(Foo *F)
> +      EXCLUSIVE_LOCKS_REQUIRED(F->mutex()) UNLOCK_FUNCTION(F->mutex()) {}
> +};
> +
> +void test() { TemplateClass<int> TC; }
> +
> +}  // end namespace DoubleAttribute
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>



-- 
DeLesley Hutchins | Software Engineer | delesley at google.com | 505-206-0315
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150318/66c43f1d/attachment.html>