[cfe-commits] r169705 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/TargetInfo.h include/clang/Sema/Sema.h lib/Basic/Targets.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclAttr.cpp lib/Sema/SemaDeclCXX.cpp test/SemaCXX/virtual-override-x64.cpp test/SemaCXX/virtual-override-x86.cpp

David Blaikie dblaikie at gmail.com
Fri Jan 18 15:06:32 PST 2013


On Sun, Dec 9, 2012 at 9:45 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> Author: aaronballman
> Date: Sun Dec  9 11:45:41 2012
> New Revision: 169705
>
> URL: http://llvm.org/viewvc/llvm-project?rev=169705&view=rev
> Log:
> Virtual method overrides can no longer have mismatched calling conventions.  This fixes PR14339.
>
> Added:
>     cfe/trunk/test/SemaCXX/virtual-override-x64.cpp
>     cfe/trunk/test/SemaCXX/virtual-override-x86.cpp
> Modified:
>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>     cfe/trunk/include/clang/Basic/TargetInfo.h
>     cfe/trunk/include/clang/Sema/Sema.h
>     cfe/trunk/lib/Basic/Targets.cpp
>     cfe/trunk/lib/Sema/SemaDecl.cpp
>     cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>     cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=169705&r1=169704&r2=169705&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Dec  9 11:45:41 2012
> @@ -1135,6 +1135,10 @@
>    "than the function it overrides}1,2">;
>  def note_overridden_virtual_function : Note<
>    "overridden virtual function is here">;
> +def err_conflicting_overriding_cc_attributes : Error<
> +  "virtual function %0 has different calling convention attributes "
> +  "%diff{($) than the function it overrides (which has calling convention $)|"
> +  "than the function it overrides}1,2">;
>
>  def err_covariant_return_inaccessible_base : Error<
>    "invalid covariant return for virtual function: %1 is a "
>
> Modified: cfe/trunk/include/clang/Basic/TargetInfo.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=169705&r1=169704&r2=169705&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/TargetInfo.h (original)
> +++ cfe/trunk/include/clang/Basic/TargetInfo.h Sun Dec  9 11:45:41 2012
> @@ -742,13 +742,19 @@
>
>    bool isBigEndian() const { return BigEndian; }
>
> +  enum CallingConvMethodType {
> +    CCMT_Unknown,
> +    CCMT_Member,
> +    CCMT_NonMember
> +  };
> +
>    /// \brief Gets the default calling convention for the given target and
>    /// declaration context.
> -  virtual CallingConv getDefaultCallingConv() const {
> +  virtual CallingConv getDefaultCallingConv(CallingConvMethodType MT) const {
>      // Not all targets will specify an explicit calling convention that we can
>      // express.  This will always do the right thing, even though it's not
>      // an explicit calling convention.
> -    return CC_Default;
> +    return CC_C;
>    }
>
>    enum CallingConvCheckResult {
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=169705&r1=169704&r2=169705&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Sun Dec  9 11:45:41 2012
> @@ -2291,7 +2291,8 @@
>    void checkUnusedDeclAttributes(Declarator &D);
>
>    bool CheckRegparmAttr(const AttributeList &attr, unsigned &value);
> -  bool CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC);
> +  bool CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC,
> +                            const FunctionDecl *FD = 0);
>    bool CheckNoReturnAttr(const AttributeList &attr);
>
>    /// \brief Stmt attributes - this routine is the top level dispatcher.
> @@ -4487,6 +4488,9 @@
>
>    std::string getAmbiguousPathsDisplayString(CXXBasePaths &Paths);
>
> +  bool CheckOverridingFunctionAttributes(const CXXMethodDecl *New,
> +                                         const CXXMethodDecl *Old);
> +
>    /// CheckOverridingFunctionReturnType - Checks whether the return types are
>    /// covariant, according to C++ [class.virtual]p5.
>    bool CheckOverridingFunctionReturnType(const CXXMethodDecl *New,
>
> Modified: cfe/trunk/lib/Basic/Targets.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=169705&r1=169704&r2=169705&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Basic/Targets.cpp (original)
> +++ cfe/trunk/lib/Basic/Targets.cpp Sun Dec  9 11:45:41 2012
> @@ -1806,8 +1806,8 @@
>              CC == CC_X86Pascal) ? CCCR_OK : CCCR_Warning;
>    }
>
> -  virtual CallingConv getDefaultCallingConv() const {
> -    return CC_C;
> +  virtual CallingConv getDefaultCallingConv(CallingConvMethodType MT) const {
> +    return MT == CCMT_Member ? CC_X86ThisCall : CC_C;
>    }
>  };
>
> @@ -2896,8 +2896,8 @@
>      return TargetInfo::checkCallingConvention(CC);
>    }
>
> -  virtual CallingConv getDefaultCallingConv() const {
> -    return CC_Default;
> +  virtual CallingConv getDefaultCallingConv(CallingConvMethodType MT) const {
> +    return CC_C;
>    }
>
>  };
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=169705&r1=169704&r2=169705&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Sun Dec  9 11:45:41 2012
> @@ -4846,6 +4846,7 @@
>        if (CXXMethodDecl *OldMD = dyn_cast<CXXMethodDecl>(*I)) {
>          MD->addOverriddenMethod(OldMD->getCanonicalDecl());
>          if (!CheckOverridingFunctionReturnType(MD, OldMD) &&
> +            !CheckOverridingFunctionAttributes(MD, OldMD) &&
>              !CheckOverridingFunctionExceptionSpec(MD, OldMD) &&
>              !CheckIfOverriddenFunctionIsMarkedFinal(MD, OldMD)) {
>            hasDeletedOverridenMethods |= OldMD->isDeleted();
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=169705&r1=169704&r2=169705&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Sun Dec  9 11:45:41 2012
> @@ -3557,10 +3557,11 @@
>  static void handleCallConvAttr(Sema &S, Decl *D, const AttributeList &Attr) {
>    if (hasDeclarator(D)) return;
>
> +  const FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
>    // Diagnostic is emitted elsewhere: here we store the (valid) Attr
>    // in the Decl node for syntactic reasoning, e.g., pretty-printing.
>    CallingConv CC;
> -  if (S.CheckCallingConvAttr(Attr, CC))
> +  if (S.CheckCallingConvAttr(Attr, CC, FD))
>      return;
>
>    if (!isa<ObjCMethodDecl>(D)) {
> @@ -3615,7 +3616,8 @@
>    D->addAttr(::new (S.Context) OpenCLKernelAttr(Attr.getRange(), S.Context));
>  }
>
> -bool Sema::CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC) {
> +bool Sema::CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC,
> +                                const FunctionDecl *FD) {
>    if (attr.isInvalid())
>      return true;
>
> @@ -3665,7 +3667,12 @@
>    TargetInfo::CallingConvCheckResult A = TI.checkCallingConvention(CC);
>    if (A == TargetInfo::CCCR_Warning) {
>      Diag(attr.getLoc(), diag::warn_cconv_ignored) << attr.getName();
> -    CC = TI.getDefaultCallingConv();
> +
> +    TargetInfo::CallingConvMethodType MT = TargetInfo::CCMT_Unknown;
> +    if (FD)
> +      MT = FD->isCXXInstanceMember() ? TargetInfo::CCMT_Member :
> +                                    TargetInfo::CCMT_NonMember;
> +    CC = TI.getDefaultCallingConv(MT);
>    }
>
>    return false;
>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=169705&r1=169704&r2=169705&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Sun Dec  9 11:45:41 2012
> @@ -26,6 +26,7 @@
>  #include "clang/AST/TypeLoc.h"
>  #include "clang/AST/TypeOrdering.h"
>  #include "clang/Basic/PartialDiagnostic.h"
> +#include "clang/Basic/TargetInfo.h"
>  #include "clang/Lex/Preprocessor.h"
>  #include "clang/Sema/CXXFieldCollector.h"
>  #include "clang/Sema/DeclSpec.h"
> @@ -10950,6 +10951,40 @@
>    }
>  }
>
> +bool Sema::CheckOverridingFunctionAttributes(const CXXMethodDecl *New,
> +                                             const CXXMethodDecl *Old) {
> +  const FunctionType *NewFT = New->getType()->getAs<FunctionType>();
> +  const FunctionType *OldFT = Old->getType()->getAs<FunctionType>();
> +
> +  CallingConv NewCC = NewFT->getCallConv(), OldCC = OldFT->getCallConv();
> +
> +  // If the calling conventions match, everything is fine
> +  if (NewCC == OldCC)
> +    return false;
> +
> +  // If either of the calling conventions are set to "default", we need to pick
> +  // something more sensible based on the target. This supports code where the
> +  // one method explicitly sets thiscall, and another has no explicit calling
> +  // convention.
> +  CallingConv Default =
> +    Context.getTargetInfo().getDefaultCallingConv(TargetInfo::CCMT_Member);
> +  if (NewCC == CC_Default)
> +    NewCC = Default;
> +  if (OldCC == CC_Default)
> +    OldCC = Default;
> +
> +  // If the calling conventions still don't match, then report the error
> +  if (NewCC != OldCC) {
> +    Diag(New->getLocation(),
> +         diag::err_conflicting_overriding_cc_attributes)
> +      << New->getDeclName() << New->getType() << Old->getType();
> +    Diag(Old->getLocation(), diag::note_overridden_virtual_function);
> +    return true;
> +  }

Some windows line endings here (& in the parameter list). Removed in r172865.

- David

> +
> +  return false;
> +}
> +
>  bool Sema::CheckOverridingFunctionReturnType(const CXXMethodDecl *New,
>                                               const CXXMethodDecl *Old) {
>    QualType NewTy = New->getType()->getAs<FunctionType>()->getResultType();
>
> Added: cfe/trunk/test/SemaCXX/virtual-override-x64.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/virtual-override-x64.cpp?rev=169705&view=auto
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/virtual-override-x64.cpp (added)
> +++ cfe/trunk/test/SemaCXX/virtual-override-x64.cpp Sun Dec  9 11:45:41 2012
> @@ -0,0 +1,36 @@
> +// RUN: %clang_cc1 -triple=x86_64-pc-unknown -fsyntax-only -verify %s
> +
> +// Non-x86 targets ignore the calling conventions by default (but will warn
> +// when one is encountered), so we want to make sure the virtual overrides
> +// continue to work.
> +namespace PR14339 {
> +  class A {
> +  public:
> +    virtual void __attribute__((thiscall)) f();        // expected-warning {{calling convention 'thiscall' ignored for this target}}
> +  };
> +
> +  class B : public A {
> +  public:
> +    void __attribute__((cdecl)) f();
> +  };
> +
> +  class C : public A {
> +  public:
> +    void __attribute__((thiscall)) f();  // expected-warning {{calling convention 'thiscall' ignored for this target}}
> +  };
> +
> +  class D : public A {
> +  public:
> +    void f();
> +  };
> +
> +  class E {
> +  public:
> +    virtual void __attribute__((stdcall)) g();  // expected-warning {{calling convention 'stdcall' ignored for this target}}
> +  };
> +
> +  class F : public E {
> +  public:
> +    void g();
> +  };
> +}
>
> Added: cfe/trunk/test/SemaCXX/virtual-override-x86.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/virtual-override-x86.cpp?rev=169705&view=auto
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/virtual-override-x86.cpp (added)
> +++ cfe/trunk/test/SemaCXX/virtual-override-x86.cpp Sun Dec  9 11:45:41 2012
> @@ -0,0 +1,33 @@
> +// RUN: %clang_cc1 -triple=i686-pc-unknown -fsyntax-only -verify %s -std=c++11
> +
> +namespace PR14339 {
> +  class A {
> +  public:
> +    virtual void __attribute__((thiscall)) f();        // expected-note{{overridden virtual function is here}}
> +  };
> +
> +  class B : public A {
> +  public:
> +    void __attribute__((cdecl)) f();  // expected-error{{virtual function 'f' has different calling convention attributes ('void () __attribute__((cdecl))') than the function it overrides (which has calling convention 'void () __attribute__((thiscall))'}}
> +  };
> +
> +  class C : public A {
> +  public:
> +    void __attribute__((thiscall)) f();  // This override is correct
> +  };
> +
> +  class D : public A {
> +  public:
> +    void f();  // This override is correct because thiscall is the default calling convention for class members
> +  };
> +
> +  class E {
> +  public:
> +    virtual void __attribute__((stdcall)) g();  // expected-note{{overridden virtual function is here}}
> +  };
> +
> +  class F : public E {
> +  public:
> +    void g();  // expected-error{{virtual function 'g' has different calling convention attributes ('void ()') than the function it overrides (which has calling convention 'void () __attribute__((stdcall))'}}
> +  };
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list