[PATCH] Delete CC_Default and use the target default CC everywhere

Richard Smith richard at metafoo.co.uk
Wed Aug 7 16:36:53 PDT 2013



================
Comment at: lib/Sema/SemaDecl.cpp:2363
@@ -2382,11 +2362,3 @@
 
-  } else if (!Context.isSameCallConv(OldTypeInfo.getCC(),
-                                     NewTypeInfo.getCC())) {
-    // Calling conventions really aren't compatible, so complain.
-    Diag(New->getLocation(), diag::err_cconv_change)
-      << FunctionType::getNameForCallConv(NewTypeInfo.getCC())
-      << (OldTypeInfo.getCC() == CC_Default)
-      << (OldTypeInfo.getCC() == CC_Default ? "" :
-          FunctionType::getNameForCallConv(OldTypeInfo.getCC()));
-    Diag(Old->getLocation(), diag::note_previous_declaration);
-    return true;
+  if (FirstTypeInfo.getCC() != NewTypeInfo.getCC()) {
+    bool NewCCExplicit = getCCTypeAttr(New->getType());
----------------
Please compare against `OldTypeInfo` for consistency with the rest of this function, and sink `First` into the `else` below. This makes no difference, because if we don't succeed in unifying the calling conventions, we return `false`, which marks the declaration invalid.

================
Comment at: lib/Sema/SemaDecl.cpp:2373-2376
@@ +2372,6 @@
+      bool FirstCCExplicit = getCCTypeAttr(First->getType());
+      assert((FirstCCExplicit || NewCCExplicit ||
+              !(isa<CXXMethodDecl>(Old) &&
+                cast<CXXMethodDecl>(Old)->isStatic())) &&
+             "clang inferred mismatched implicit CCs");
+      Diag(New->getLocation(), diag::err_cconv_change)
----------------
This `assert` trivially holds: `NewCCExplicit` is `true` here. Also, drop the "clang" in the assert text.

================
Comment at: lib/Sema/SemaType.cpp:4474-4475
@@ +4473,4 @@
+    const AttributedType *AT = type->getAs<AttributedType>();
+    while (AT && (!AT->isCallingConv() || AT->getAttrKind() == CCAttrKind))
+      AT = AT->getModifiedType()->getAs<AttributedType>();
+
----------------
Can you stop if you hit an attribute where `AT->getAttrKind() == CCAttrKind`? If there's a problem within that, we should have diagnosed it when building that type.

================
Comment at: lib/Sema/SemaType.cpp:4475
@@ +4474,3 @@
+    while (AT && (!AT->isCallingConv() || AT->getAttrKind() == CCAttrKind))
+      AT = AT->getModifiedType()->getAs<AttributedType>();
+
----------------
Should we walk through `typedef`s here? I seem to recall we determined that we're only supposed to check this against other attributes on the current declarator.

================
Comment at: lib/Sema/SemaType.cpp:4548
@@ +4547,3 @@
+  FunctionTypeUnwrapper Unwrapped(*this, T);
+  T = Unwrapped.wrap(*this, FT);
+}
----------------
I think we lose source information here if a member function is declared through a `typedef`, and the member and non-member calling conventions differ. Please add a FIXME for this (we should probably produce a type sugar node to track the pre-adjustment type).


http://llvm-reviews.chandlerc.com/D1231



More information about the cfe-commits mailing list