[PATCH] Delete CC_Default and use the target default CC everywhere
Richard Smith
richard at metafoo.co.uk
Mon Jul 29 14:17:35 PDT 2013
================
Comment at: include/clang/AST/ASTContext.h:1754-1758
@@ -1758,7 +1753,7 @@
/// \brief Determines whether two calling conventions name the same
/// calling convention.
bool isSameCallConv(CallingConv lcc, CallingConv rcc) {
- return (getCanonicalCallConv(lcc) == getCanonicalCallConv(rcc));
+ return lcc == rcc;
}
----------------
Is it worth keeping this around?
================
Comment at: lib/AST/ASTContext.cpp:2803
@@ -2808,4 +2802,3 @@
CanonicalEPI.NumExceptions = 0;
- CanonicalEPI.ExtInfo
- = CanonicalEPI.ExtInfo.withCallingConv(getCanonicalCallConv(CallConv));
+ CanonicalEPI.ExtInfo = CanonicalEPI.ExtInfo.withCallingConv(CallConv);
----------------
This looks redundant.
================
Comment at: lib/AST/ASTContext.cpp:7765-7766
@@ -7771,3 +7764,4 @@
FunctionType::ExtInfo EI;
+ EI = EI.withCallingConv(CC_C);
if (BuiltinInfo.isNoReturn(Id)) EI = EI.withNoReturn(true);
----------------
This worries me. We default-construct ExtInfo within ExtProtoInfo instances all over the place. Can we make ExtInfo not be default-constructible?
================
Comment at: lib/AST/ASTContext.cpp:7942
@@ -7946,7 +7941,3 @@
-CallingConv ASTContext::getCanonicalCallConv(CallingConv CC) const {
- if (CC == CC_C && !LangOpts.MRTD &&
- getTargetInfo().getCXXABI().isMemberFunctionCCDefault())
- return CC_Default;
- return CC;
+ return (LangOpts.MRTD && !IsVariadic) ? CC_X86StdCall : CC_C;
}
----------------
The IsVariadic check appears to be new here, but I believe it's correct. Do we have test coverage for this?
================
Comment at: lib/AST/Type.cpp:519-532
@@ -518,1 +518,16 @@
+const AttributedType *Type::getAsAttributedType() const {
+ // AttributedTypes are non-canonical, so we can't use getAs<AttributedType>().
+ // Instead we have to unwrap each level of sugar iteratively until we get back
+ // the same type or find an AttributedType.
+ QualType Prev;
+ QualType T = QualType(this, 0);
+ while (T != Prev) {
+ if (const AttributedType *AT = dyn_cast<AttributedType>(T))
+ return AT;
+ Prev = T;
+ T = T->getLocallyUnqualifiedSingleStepDesugaredType();
+ }
+ return 0;
+}
+
----------------
Please add an explicit specialization for getAs<> here, as we do for TypedefType and TemplateSpecializationType.
================
Comment at: lib/AST/TypePrinter.cpp:1158-1159
@@ -1152,2 +1157,4 @@
case AttributedType::attr_regparm: {
+ // FIXME: When Sema learns to form this AttributedType, avoid printing the
+ // attribute again in printFunctionProtoAfter.
OS << "regparm(";
----------------
Hmm, we should probably recurse into the modified type, rather than the equivalent type, when printing an AttributedType. But not in this patch :)
================
Comment at: lib/Sema/SemaDecl.cpp:2243
@@ +2242,3 @@
+ while (AT && !AT->isCallingConv())
+ AT = AT->getEquivalentType()->getAsAttributedType();
+ if (AT) return AT;
----------------
This should presumably be getModifiedType(), in case the equivalent type has lost some of the sugar. (If you apply an attribute which modifies the function type on top of a function type with a calling convention, you're likely to lose the calling convention sugar from the equivalent type.)
================
Comment at: lib/Sema/SemaDecl.cpp:2254
@@ +2253,3 @@
+ while (AT && !AT->isCallingConv())
+ AT = AT->getEquivalentType()->getAsAttributedType();
+ return AT;
----------------
Likewise.
================
Comment at: lib/Sema/SemaDecl.cpp:2376
@@ +2375,3 @@
+ if (FirstTypeInfo.getCC() != NewTypeInfo.getCC()) {
+ bool FirstCCExplicit = getCCTypeAttr(First);
+ bool NewCCExplicit = getCCTypeAttr(New);
----------------
I don't think it should matter whether the first CC was implicit or explicit; the rule seems to be simply that all subsequent decls must match the first decl.
================
Comment at: lib/Sema/SemaDecl.cpp:2375-2416
@@ -2368,27 +2374,44 @@
- // Inherit the CC from the previous declaration if it was specified
- // there but not here.
- } else if (NewTypeInfo.getCC() == CC_Default) {
- NewTypeInfo = NewTypeInfo.withCallingConv(OldTypeInfo.getCC());
- RequiresAdjustment = true;
+ if (FirstTypeInfo.getCC() != NewTypeInfo.getCC()) {
+ bool FirstCCExplicit = getCCTypeAttr(First);
+ bool NewCCExplicit = getCCTypeAttr(New);
+ if (FirstCCExplicit && !NewCCExplicit) {
+ // Inherit the CC from the previous declaration if it was specified
+ // there but not here.
+ NewTypeInfo = NewTypeInfo.withCallingConv(OldTypeInfo.getCC());
+ RequiresAdjustment = true;
+
+ } else if (!FirstCCExplicit && !NewCCExplicit) {
+ if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(First)) {
+ // We can't tell if an out of line method definition is static until
+ // after we resolve overloads and find its canonical declaration.
+ bool IsVariadic = isa<FunctionProtoType>(FirstType) &&
+ cast<FunctionProtoType>(FirstType)->isVariadic();
+ CallingConv CCStatic =
+ Context.getDefaultCallingConvention(IsVariadic, false);
+ CallingConv CCInstance =
+ Context.getDefaultCallingConvention(IsVariadic, true);
+ if (MD->isStatic() && FirstTypeInfo.getCC() == CCStatic &&
+ NewTypeInfo.getCC() == CCInstance) {
+ NewTypeInfo = NewTypeInfo.withCallingConv(OldTypeInfo.getCC());
+ RequiresAdjustment = true;
+ }
+ }
+ }
- // Don't complain about mismatches when the default CC is
- // effectively the same as the explict one. Only Old decl contains correct
- // information about storage class of CXXMethod.
- } else if (OldTypeInfo.getCC() == CC_Default &&
- isABIDefaultCC(*this, NewTypeInfo.getCC(), Old)) {
- NewTypeInfo = NewTypeInfo.withCallingConv(OldTypeInfo.getCC());
- RequiresAdjustment = true;
+ if (FirstTypeInfo.getCC() != NewTypeInfo.getCC()) {
+ // Calling conventions aren't compatible, so complain.
+ assert(!Context.isSameCallConv(OldTypeInfo.getCC(), NewTypeInfo.getCC()));
+ Diag(New->getLocation(), diag::err_cconv_change)
+ << FunctionType::getNameForCallConv(NewTypeInfo.getCC())
+ << !FirstCCExplicit
+ << (!FirstCCExplicit ? "" :
+ FunctionType::getNameForCallConv(OldTypeInfo.getCC()));
- } 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;
+ // Put the note on the first decl, since it is the one that matters.
+ Diag(Old->getFirstDeclaration()->getLocation(),
+ diag::note_previous_declaration);
+ return true;
+ }
}
----------------
Can you collapse this into
if (OldTypeInfo.getCC() != NewTypeInfo.getCC()) {
if (!NewExplicit) {
NewTypeInfo = NewTypeInfo.withCallingConv(OldTypeInfo.getCC());
RequiresAdjustment = true;
} else {
issue diagnostic
}
}
================
Comment at: lib/Sema/SemaDecl.cpp:6269-6271
@@ +6268,5 @@
+
+ // Ignore dependent types.
+ const FunctionType *FT = R->getAs<FunctionType>();
+ if (!FT) return R;
+
----------------
Can this really happen?
================
Comment at: lib/Sema/SemaDecl.cpp:6276
@@ +6275,3 @@
+ while (AT && !AT->isCallingConv())
+ AT = AT->getEquivalentType()->getAsAttributedType();
+ if (AT) return R;
----------------
As before, this should presumably be getModifiedType. Can this share code with getCCTypeAttr?
================
Comment at: lib/Sema/SemaDeclCXX.cpp:7922
@@ -7906,5 +7921,3 @@
- // Build an exception specification pointing back at this constructor.
- FunctionProtoType::ExtProtoInfo EPI;
- EPI.ExceptionSpecType = EST_Unevaluated;
- EPI.ExceptionSpecDecl = DefaultCon;
+ // Build an exception specification pointing back at this destructor.
+ FunctionProtoType::ExtProtoInfo EPI = getImplicitMethodEPI(*this, DefaultCon);
----------------
Copy/pasto?
================
Comment at: lib/Sema/SemaLookup.cpp:731
@@ -730,3 +730,3 @@
FunctionProtoType::ExtProtoInfo EPI = ConvProto->getExtProtoInfo();
- EPI.ExtInfo = EPI.ExtInfo.withCallingConv(CC_Default);
+ EPI.ExtInfo = EPI.ExtInfo.withCallingConv(CC_C);
EPI.ExceptionSpecType = EST_None;
----------------
It would seem more appropriate to use ConvTemplate->getTemplatedDecl()'s calling convention here.
================
Comment at: lib/Sema/SemaType.cpp:2446-2450
@@ +2445,7 @@
+
+ if (FoundNonParen) {
+ // If we're not the declarator, we're a regular function type unless we're
+ // in a member pointer.
+ IsCXXInstanceMethod =
+ D.getTypeObject(I).Kind == DeclaratorChunk::MemberPointer;
+ } else {
----------------
What happens if a function type becomes a pointer-to-member-function through a typedef-name? For instance:
typedef int fn();
typedef fn S::*p;
Presumably this should give a pointer-to-member-function type with a c++ method cc.
Hmm, what happens here:
struct S;
template<typename Fn> struct X {
typedef Fn S::*p;
};
X<void()>::p p1;
X<__cdecl void()>::p p2;
void (S::*p3)();
__cdecl void (S::*p4)();
Does MSVC believe that p1, p2, p3, and p4 have the same type?
================
Comment at: lib/Sema/SemaType.cpp:4473
@@ +4472,3 @@
+ while (AT && (!AT->isCallingConv() || AT->getAttrKind() == CCAttrKind))
+ AT = AT->getEquivalentType()->getAsAttributedType();
+
----------------
getModifiedType, as before.
http://llvm-reviews.chandlerc.com/D1231
More information about the cfe-commits
mailing list