aaron.ballman updated this revision to Diff 82816. aaron.ballman added a comment.
I've updated the patch to not hard code the type adjustments, but instead added `Type::getAsAdjusted()` and `TypeLoc::getAsAdjusted()`. This patch has one broken test case, but I do not think the test case was being triggered as its author expected. The failing test is clang\test\CodeGen\microsoft-call-conv-x64.c, and the failure is that the call to `f7()` does not match the CHECK line. Instead of `call void @f7(i32 0)`, you now get `call void bitcast (void (i32)* @f7 to void (i64)*)(i64 0)`. Sema::BuildDeclarationNameExpr() has some code to check for K&R-style functions and ensuring that we lose prototype information in accordance with C DR 316. However, under the code prior to this patch, the predicate was false for this test case because `FD->hasPrototype()` would return true despite the function being a K&R function with no prototype. Since we corrected that behavior `FD->hasPrototype()` now returns false, prompting the adjustment, which alters the IR we generate. However, I'm not certain whether the altered IR is desired or not, so advice on this is welcome. https://reviews.llvm.org/D28166 Files: include/clang/AST/Type.h include/clang/AST/TypeLoc.h lib/Sema/SemaDecl.cpp test/Sema/knr-def-call.c test/Sema/warn-strict-prototypes.c
Index: test/Sema/warn-strict-prototypes.c =================================================================== --- test/Sema/warn-strict-prototypes.c +++ test/Sema/warn-strict-prototypes.c @@ -60,3 +60,8 @@ // K&R function definition with previous prototype declared is not diagnosed. void foo11(int p, int p2); void foo11(p, p2) int p; int p2; {} + +// PR31020 +void __attribute__((cdecl)) foo12(d) // expected-warning {{this old-style function definition is not preceded by a prototype}} + short d; +{} Index: test/Sema/knr-def-call.c =================================================================== --- test/Sema/knr-def-call.c +++ test/Sema/knr-def-call.c @@ -39,3 +39,9 @@ proto(42.1); // expected-warning{{implicit conversion from 'double' to 'int' changes value from 42.1 to 42}} (&proto)(42.1); // expected-warning{{implicit conversion from 'double' to 'int' changes value from 42.1 to 42}} } + +// PR31020 +void func(short d) __attribute__((cdecl)); // expected-note{{previous declaration is here}} +void __attribute__((cdecl)) func(d) + short d; // expected-warning{{promoted type 'int' of K&R function parameter is not compatible with the parameter type 'short' declared in a previous prototype}} +{} Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -7458,11 +7458,12 @@ // Determine whether the function was written with a // prototype. This true when: // - there is a prototype in the declarator, or - // - the type R of the function is some kind of typedef or other reference - // to a type name (which eventually refers to a function type). + // - the type R of the function is some kind of typedef or other non- + // attributed reference to a type name (which eventually refers to a + // function type). bool HasPrototype = (D.isFunctionDeclarator() && D.getFunctionTypeInfo().hasPrototype) || - (!isa<FunctionType>(R.getTypePtr()) && R->isFunctionProtoType()); + (!R->getAsAdjusted<FunctionType>() && R->isFunctionProtoType()); NewFD = FunctionDecl::Create(SemaRef.Context, DC, D.getLocStart(), NameInfo, R, @@ -11950,7 +11951,7 @@ !LangOpts.CPlusPlus) { TypeSourceInfo *TI = FD->getTypeSourceInfo(); TypeLoc TL = TI->getTypeLoc(); - FunctionTypeLoc FTL = TL.castAs<FunctionTypeLoc>(); + FunctionTypeLoc FTL = TL.getAsAdjusted<FunctionTypeLoc>(); Diag(FTL.getLParenLoc(), diag::warn_strict_prototypes) << 1; } } Index: include/clang/AST/TypeLoc.h =================================================================== --- include/clang/AST/TypeLoc.h +++ include/clang/AST/TypeLoc.h @@ -70,6 +70,26 @@ return t; } + /// \brief Convert to the specified TypeLoc type, returning a null TypeLoc if + /// this TypeLock is not of the desired type. It will consider type + /// adjustments from a type that wad written as a T to another type that is + /// still canonically a T (ignores parens, attributes, elaborated types, etc). + template <typename T> + T getAsAdjusted() const { + TypeLoc Cur = *this; + while (!T::isKind(Cur)) { + if (auto PTL = Cur.getAs<ParenTypeLoc>()) + Cur = PTL.getInnerLoc(); + else if (auto ATL = Cur.getAs<AttributedTypeLoc>()) + Cur = ATL.getModifiedLoc(); + else if (auto ETL = Cur.getAs<ElaboratedTypeLoc>()) + Cur = ETL.getNamedTypeLoc(); + else + break; + } + return Cur.getAs<T>(); + } + /// The kinds of TypeLocs. Equivalent to the Type::TypeClass enum, /// except it also defines a Qualified enum that corresponds to the /// QualifiedLoc class. Index: include/clang/AST/Type.h =================================================================== --- include/clang/AST/Type.h +++ include/clang/AST/Type.h @@ -1875,6 +1875,13 @@ /// immediately following this class. template <typename T> const T *getAs() const; + /// Member-template getAsAdjusted<specific type>. Look through specific kinds + /// of sugar (parens, attributes, etc) for an instance of \<specific type>. + /// This is used when you need to walk over sugar nodes that represent some + /// kind of type adjustment from a type that was written as a \<specific type> + /// to another type that is still canonically a \<specific type>. + template <typename T> const T *getAsAdjusted() const; + /// A variant of getAs<> for array types which silently discards /// qualifiers from the outermost type. const ArrayType *getAsArrayTypeUnsafe() const; @@ -5932,6 +5939,36 @@ return cast<T>(getUnqualifiedDesugaredType()); } +template <typename T> const T *Type::getAsAdjusted() const { + static_assert(!TypeIsArrayType<T>::value, "ArrayType cannot be used with getAsAdjusted!"); + + // If this is directly a T type, return it. + if (const T *Ty = dyn_cast<T>(this)) + return Ty; + + // If the canonical form of this type isn't the right kind, reject it. + if (!isa<T>(CanonicalType)) + return nullptr; + + // Strip off type adjustments that do not modify the underlying nature of the + // type. + const Type *Ty = this; + while (Ty) { + if (const auto *A = dyn_cast<AttributedType>(Ty)) + Ty = A->getModifiedType().getTypePtr(); + else if (const auto *E = dyn_cast<ElaboratedType>(Ty)) + Ty = E->desugar().getTypePtr(); + else if (const auto *P = dyn_cast<ParenType>(Ty)) + Ty = P->desugar().getTypePtr(); + else + break; + } + + // Just because the canonical type is correct does not mean we can use cast<>, + // since we may not have stripped off all the sugar down to the base type. + return dyn_cast<T>(Ty); +} + inline const ArrayType *Type::getAsArrayTypeUnsafe() const { // If this is directly an array type, return it. if (const ArrayType *arr = dyn_cast<ArrayType>(this))
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits