[cfe-commits] r133671 - in /cfe/trunk: lib/Sema/SemaLookup.cpp test/SemaCXX/using-decl-assignment-cache.cpp

[Douglas Gregor]

On Jun 22, 2011, at 5:26 PM, Sean Hunt wrote:

> Author: coppro
> Date: Wed Jun 22 19:26:20 2011
> New Revision: 133671
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=133671&view=rev
> Log:
> Clean up the heart of the caching code and miss fewer edge cases.
> 
> Added:
>    cfe/trunk/test/SemaCXX/using-decl-assignment-cache.cpp
> Modified:
>    cfe/trunk/lib/Sema/SemaLookup.cpp
> 
> Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=133671&r1=133670&r2=133671&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaLookup.cpp Wed Jun 22 19:26:20 2011
> @@ -2240,7 +2240,7 @@
>     ThisTy.addConst();
>   if (VolatileThis)
>     ThisTy.addVolatile();
> -  Expr::Classification ObjectClassification =
> +  Expr::Classification Classification =
>     (new (Context) OpaqueValueExpr(SourceLocation(), ThisTy,
>                                    RValueThis ? VK_RValue : VK_LValue))->
>         Classify(Context);
> @@ -2256,12 +2256,21 @@
>   assert((I != E) &&
>          "lookup for a constructor or assignment operator was empty");
>   for ( ; I != E; ++I) {
> -    if ((*I)->isInvalidDecl())
> +    Decl *DD = *I;
> +    
> +    if (UsingShadowDecl *U = dyn_cast<UsingShadowDecl>(D))
> +      DD = U->getTargetDecl();

You want to use U->getDeclContext() as the ActingContext here.

> +    if (DD->isInvalidDecl())
>       continue;
> 
> -    if (CXXMethodDecl *M = dyn_cast<CXXMethodDecl>(*I)) {
> -      AddOverloadCandidate(M, DeclAccessPair::make(M, AS_public), &Arg, NumArgs,
> -                           OCS, true);
> +    if (CXXMethodDecl *M = dyn_cast<CXXMethodDecl>(DD)) {
> +      if (SM == CXXCopyAssignment || SM == CXXMoveAssignment)
> +        AddMethodCandidate(M, DeclAccessPair::make(M, AS_public), D, ThisTy,
> +                           Classification, &Arg, NumArgs, OCS, true);

This DeclAccessPair looks wrong... it seems like it should contain the original D (not M) and the access specifier it would have when found in the context we're walking.

> +      else
> +        AddOverloadCandidate(M, DeclAccessPair::make(M, AS_public), &Arg,
> +                             NumArgs, OCS, true);

Same with this access specifier.

>       // Here we're looking for a const parameter to speed up creation of
>       // implicit copy methods.
> @@ -2274,9 +2283,14 @@
>           Result->setConstParamMatch(true);
>       }
>     } else if (FunctionTemplateDecl *Tmpl =
> -                 dyn_cast<FunctionTemplateDecl>(*I)) {
> -      AddTemplateOverloadCandidate(Tmpl, DeclAccessPair::make(Tmpl, AS_public),
> -                                   0, &Arg, NumArgs, OCS, true);
> +                 dyn_cast<FunctionTemplateDecl>(DD)) {
> +      if (SM == CXXCopyAssignment || SM == CXXMoveAssignment)
> +        AddMethodTemplateCandidate(Tmpl, DeclAccessPair::make(Tmpl, AS_public),
> +                                   D, 0, ThisTy, Classification, &Arg, NumArgs,
> +                                   OCS, true);
> +      else
> +        AddTemplateOverloadCandidate(Tmpl, DeclAccessPair::make(Tmpl, AS_public),
> +                                     0, &Arg, NumArgs, OCS, true);
>     }
>   }

Same ActingContext/DeclAccessPair comments here.

	- Doug