[cfe-commits] r72601 - in /cfe/trunk: lib/Sema/SemaDecl.cpp lib/Sema/SemaInherit.cpp lib/Sema/SemaInherit.h test/SemaCXX/abstract.cpp

[Douglas Gregor]

On May 29, 2009, at 5:52 PM, Anders Carlsson wrote:

> Author: andersca
> Date: Fri May 29 19:52:53 2009
> New Revision: 72601
>
> URL: http://llvm.org/viewvc/llvm-project?rev=72601&view=rev
> Log:
> Add a member lookup criteria constructor for searching for  
> overridden virtual member functions. Use this instead of regular  
> name lookup when checking for overriding functions so we will see  
> declarations that would otherwise be hidden. Fixes 6902298.
>
> Modified:
>    cfe/trunk/lib/Sema/SemaDecl.cpp
>    cfe/trunk/lib/Sema/SemaInherit.cpp
>    cfe/trunk/lib/Sema/SemaInherit.h
>    cfe/trunk/test/SemaCXX/abstract.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=72601&r1=72600&r2=72601&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri May 29 19:52:53 2009
> @@ -2145,23 +2145,11 @@
>     // Look for virtual methods in base classes that this method  
> might override.
>
>     BasePaths Paths;
> -    // FIXME: This will not include hidden member functions.
>     if (LookupInBases(cast<CXXRecordDecl>(DC),
> -                      MemberLookupCriteria(Name, LookupMemberName,
> -                                           // FIXME: Shouldn't  
> IDNS_Member be
> -                                           // enough here?
> -                                           Decl::IDNS_Member |
> -                                           Decl::IDNS_Ordinary),  
> Paths)) {
> +                      MemberLookupCriteria(NewMD), Paths)) {
>       for (BasePaths::decl_iterator I = Paths.found_decls_begin(),
>            E = Paths.found_decls_end(); I != E; ++I) {
>         if (CXXMethodDecl *OldMD = dyn_cast<CXXMethodDecl>(*I)) {
> -          OverloadedFunctionDecl::function_iterator MatchedDecl;
> -          // FIXME: Is this OK? Should it be done by LookupInBases?
> -          if (IsOverload(NewMD, OldMD, MatchedDecl))
> -            continue;
> -          if (!OldMD->isVirtual())
> -            continue;
> -
>           if (!CheckOverridingFunctionReturnType(NewMD, OldMD))
>             NewMD->addOverriddenMethod(OldMD);
>         }
>
> Modified: cfe/trunk/lib/Sema/SemaInherit.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInherit.cpp?rev=72601&r1=72600&r2=72601&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Sema/SemaInherit.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaInherit.cpp Fri May 29 19:52:53 2009
> @@ -191,6 +191,22 @@
>         ++Paths.ScratchPath.Decls.first;
>       }
>       break;
> +    case MemberLookupCriteria::LK_OverriddenMember:
> +      Paths.ScratchPath.Decls =
> +        BaseRecord->lookup(Context, Criteria.Method->getDeclName());
> +      while (Paths.ScratchPath.Decls.first !=  
> Paths.ScratchPath.Decls.second) {
> +        CXXMethodDecl *MD =
> +          cast<CXXMethodDecl>(*Paths.ScratchPath.Decls.first);

I believe that you want to dyn_cast here, and then continue if what  
you found isn't a CXXMethodDecl. Here's a test case that current  
asserts:

struct A {
   int f;
   virtual ~A();
};

struct B : public A {
   void f();
};


> +        OverloadedFunctionDecl::function_iterator MatchedDecl;
> +        if (MD->isVirtual() && !IsOverload(Criteria.Method, MD,  
> MatchedDecl)) {
> +          FoundPathToThisBase = true;
> +          break;
> +        }
> +
> +        ++Paths.ScratchPath.Decls.first;
> +      }
> +      break;
>     }

This is very clean. Nice!

	- Doug