Virtual method overrides can no longer have mismatched calling conventions. This fixes PR14339.

llvm-svn: 169705

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -1135,6 +1135,10 @@ def err_different_return_type_for_overriding_virtual_function : Error<
   "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 "

clang/include/clang/Basic/TargetInfo.h

@@ -742,13 +742,19 @@ public:
 
   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 {

clang/include/clang/Sema/Sema.h

@@ -2291,7 +2291,8 @@ public:
   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 @@ public:
 
   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,

clang/lib/Basic/Targets.cpp

@@ -1806,8 +1806,8 @@ public:
             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 @@ public:
     return TargetInfo::checkCallingConvention(CC);
   }
 
-  virtual CallingConv getDefaultCallingConv() const {
-    return CC_Default;
+  virtual CallingConv getDefaultCallingConv(CallingConvMethodType MT) const {
+    return CC_C;
   }
 
 };

clang/lib/Sema/SemaDecl.cpp

@@ -4846,6 +4846,7 @@ bool Sema::AddOverriddenMethods(CXXRecordDecl *DC, CXXMethodDecl *MD) {
       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();

clang/lib/Sema/SemaDeclAttr.cpp

@@ -3557,10 +3557,11 @@ static void handleGNUInlineAttr(Sema &S, Decl *D, const AttributeList &Attr) {
 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 @@ static void handleOpenCLKernelAttr(Sema &S, Decl *D, const AttributeList &Attr){
   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 @@ bool Sema::CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC) {
   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;

clang/lib/Sema/SemaDeclCXX.cpp

@@ -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 @@ void Sema::DiagnoseReturnInConstructorExceptionHandler(CXXTryStmt *TryBlock) {
   }
 }
 
+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;
+  }
+
+  return false;
+}
+
 bool Sema::CheckOverridingFunctionReturnType(const CXXMethodDecl *New,
                                              const CXXMethodDecl *Old) {
   QualType NewTy = New->getType()->getAs<FunctionType>()->getResultType();

clang/test/SemaCXX/virtual-override-x64.cpp

@@ -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();
+  };
+}

clang/test/SemaCXX/virtual-override-x86.cpp

@@ -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))'}}
+  };
+}