================
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;
}
----------------
Richard Smith wrote:
> Is it worth keeping this around?
No. My plan was to do a follow up to remove it, but there are only 4 callsites
left, so I'll nuke it now.
================
Comment at: lib/Sema/SemaDecl.cpp:2376
@@ +2375,3 @@
+ if (FirstTypeInfo.getCC() != NewTypeInfo.getCC()) {
+ bool FirstCCExplicit = getCCTypeAttr(First);
+ bool NewCCExplicit = getCCTypeAttr(New);
----------------
Richard Smith wrote:
> 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.
That's basically true, but it's probably an error if clang deduces two
different implicit CCs for the first decl and a redeclaration, with the
exception of static method definitions.
I'll add an assert.
================
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;
+ }
}
----------------
Richard Smith wrote:
> Can you collapse this into
>
> if (OldTypeInfo.getCC() != NewTypeInfo.getCC()) {
> if (!NewExplicit) {
> NewTypeInfo = NewTypeInfo.withCallingConv(OldTypeInfo.getCC());
> RequiresAdjustment = true;
> } else {
> issue diagnostic
> }
> }
Done, plus the assert.
================
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);
----------------
Richard Smith wrote:
> This looks redundant.
Gone.
================
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);
----------------
Richard Smith wrote:
> This worries me. We default-construct ExtInfo within ExtProtoInfo instances
> all over the place. Can we make ExtInfo not be default-constructible?
It could be done, but it would require significantly rewriting every
construction of ExtProtoInfo as well. ExtInfo() currently zero initializes,
which happens to be equivalent to CC_C. How about I make ExtInfo default
construct with CC_C?
================
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;
}
----------------
Richard Smith wrote:
> The IsVariadic check appears to be new here, but I believe it's correct. Do
> we have test coverage for this?
It looks like the LLVM x86 backend simply lowers x86_stdcallcc as caller
cleanup, so I don't think this IsVariadic check is necessary. I added a
variadic function to the existing -mrtd test, but should it use x86_stdcallcc
on variadic functions or not?
================
Comment at: lib/Sema/SemaDecl.cpp:6269-6271
@@ +6268,5 @@
+
+ // Ignore dependent types.
+ const FunctionType *FT = R->getAs<FunctionType>();
+ if (!FT) return R;
+
----------------
Richard Smith wrote:
> Can this really happen?
Seems to work with castAs.
================
Comment at: lib/Sema/SemaDecl.cpp:6276
@@ +6275,3 @@
+ while (AT && !AT->isCallingConv())
+ AT = AT->getEquivalentType()->getAsAttributedType();
+ if (AT) return R;
----------------
Richard Smith wrote:
> As before, this should presumably be getModifiedType. Can this share code
> with getCCTypeAttr?
Done.
================
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);
----------------
Richard Smith wrote:
> Copy/pasto?
Woops.
================
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;
----------------
Richard Smith wrote:
> It would seem more appropriate to use ConvTemplate->getTemplatedDecl()'s
> calling convention here.
That should already be there from the copy. John McCall added this CC_Default
assignment in r121763. I'm not sure why.
================
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 {
----------------
Richard Smith wrote:
> 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?
MSVC doesn't model the difference between an implicit and explicit CC. I
modified the example, and we get these demangled symbols from cl.exe:
struct S;
template<typename Fn> struct X {
typedef Fn S::*p;
};
X<void()>::p p1;
X<void __cdecl ()>::p p2;
void (S::*p3)();
void (__cdecl S::*p4)();
(void (__thiscall S::* p1)(void))
(void (__thiscall S::* p2)(void))
(void (__thiscall S::* p3)(void))
(void (__cdecl S::* p4)(void))
I added tests for this, but clang currently gets some of these wrong. As
discussed, I will have to introduce an implicit AttributedType node so that
MPT->getPointeeType() returns a function type which is canonically thiscall,
without erasing the typedef from TSI. That's for a later patch.
================
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;
+}
+
----------------
Richard Smith wrote:
> Please add an explicit specialization for getAs<> here, as we do for
> TypedefType and TemplateSpecializationType.
Done.
================
Comment at: lib/Sema/SemaDecl.cpp:2243
@@ +2242,3 @@
+ while (AT && !AT->isCallingConv())
+ AT = AT->getEquivalentType()->getAsAttributedType();
+ if (AT) return AT;
----------------
Richard Smith wrote:
> 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.)
Done. I can't really test this well because we don't build AttributedTypes for
things like noreturn yet.
http://llvm-reviews.chandlerc.com/D1231
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
