[cfe-commits] r159326 - in /cfe/trunk: include/clang/AST/Expr.h lib/AST/Expr.cpp lib/CodeGen/CGExprCXX.cpp test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp

[John McCall]

On Jun 27, 2012, at 6:56 PM, Rafael Espindola wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=159326&view=rev
> Log:
> Fix another issue with devirtualizing calls to final methods by passing them
> the correct this pointer. There is some potential for sharing a bit more
> code with canDevirtualizeMemberFunctionCalls, but that can be done in an
> independent patch.

> 
> Modified:
>    cfe/trunk/include/clang/AST/Expr.h
>    cfe/trunk/lib/AST/Expr.cpp
>    cfe/trunk/lib/CodeGen/CGExprCXX.cpp
>    cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
> 
> Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=159326&r1=159325&r2=159326&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Wed Jun 27 20:56:38 2012
> @@ -172,11 +172,21 @@
>   }
> 
>   // Compute the object pointer.
> +  const Expr *Base = ME->getBase();
> +  bool CanUseVirtualCall = MD->isVirtual() && !ME->hasQualifier();
> +  bool Devirtualize = CanUseVirtualCall &&
> +    canDevirtualizeMemberFunctionCalls(getContext(), Base, MD);
> +
> +  const Expr *Inner = Base;
> +  if (Devirtualize)
> +    Inner = Base->ignoreParenBaseCasts();
> +
>   llvm::Value *This;
>   if (ME->isArrow())
> -    This = EmitScalarExpr(ME->getBase());
> +    This = EmitScalarExpr(Inner);
>   else
> -    This = EmitLValue(ME->getBase()).getAddress();
> +    This = EmitLValue(Inner).getAddress();
> +
> 
>   if (MD->isTrivial()) {
>     if (isa<CXXDestructorDecl>(MD)) return RValue::get(0);
> @@ -223,11 +233,8 @@
>   //
>   // We also don't emit a virtual call if the base expression has a record type
>   // because then we know what the type is.
> -  const Expr *Base = ME->getBase();
> -  bool UseVirtualCall = MD->isVirtual() && !ME->hasQualifier()
> -                        && !canDevirtualizeMemberFunctionCalls(getContext(),
> -                                                               Base, MD);
> -  const CXXRecordDecl *MostDerivedClassDecl = Base->getBestDynamicClassType();
> +  bool UseVirtualCall = CanUseVirtualCall && !Devirtualize;
> +  const CXXRecordDecl *MostDerivedClassDecl = Inner->getBestDynamicClassType();

Inner (and therefore "this") will indeed have type MostDerivedClassDecl
here, but that's not necessarily the type that declares the final override for
the method being called:

  struct Primary { virtual ~Primary(); void *member; };

  struct A { virtual void foo(); }; // its own final override
  struct final B : Primary, A {}; // most derived
  void test(B *b) {
    b->foo();
  }

My example happens to have us devirtualize to the original method,
but that's not necessarily the case;  you can have arbitrary inheritance
and overriding between A and the most-derived class, like so:

  struct A { virtual void foo(); };
  struct B : A { virtual void foo(); }; // final override
  struct final C : Primary, B {}; // most derived
  void test(C *c) {
    static_cast<A*>(c)->foo();
  }

And of course the offset from A to B doesn't have to be zero, either:

  struct A { virtual void foo(); };
  struct B : Primary, A { virtual void foo(); }; // final override
  struct final C : Primary, B {}; // most derived
  void test(C *c) {
    static_cast<A*>(d)->foo();
  }

Really, you have to do a derived-to-base conversion from the most-derived
class to the base actually declaring the method.  Fortunately, that can't
actually fail (they can in general, due to ambiguity) because of the
well-formedness constraints on final overriders.

John.