From: Leonard Chan Date: Thu, 24 Jan 2019 00:11:35 +0000 (+0000) Subject: [Sema] Fix Modified Type in address_space AttributedType X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=be1bd1088603320326ae31e046e02185d2d0d19c;p=clang [Sema] Fix Modified Type in address_space AttributedType This is a fix for https://reviews.llvm.org/D51229 where we pass the address_space qualified type as the modified type of an AttributedType. This change now instead wraps the AttributedType with either the address_space qualifier or a DependentAddressSpaceType. Differential Revision: https://reviews.llvm.org/D55447 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@351997 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/docs/ReleaseNotes.rst b/docs/ReleaseNotes.rst index a95a4b9057..67472f2a50 100644 --- a/docs/ReleaseNotes.rst +++ b/docs/ReleaseNotes.rst @@ -162,7 +162,11 @@ clang-format libclang -------- -... +- When `CINDEXTEST_INCLUDE_ATTRIBUTED_TYPES` is not provided when making a + CXType, the equivalent type of the AttributedType is returned instead of the + modified type if the user does not want attribute sugar. The equivalent type + represents the minimally-desugared type which the AttributedType is + canonically equivalent to. Static Analyzer diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 2787495270..5c66a01ed3 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -1411,6 +1411,10 @@ public: QualType BuildVectorType(QualType T, Expr *VecSize, SourceLocation AttrLoc); QualType BuildExtVectorType(QualType T, Expr *ArraySize, SourceLocation AttrLoc); + QualType BuildAddressSpaceAttr(QualType &T, LangAS ASIdx, Expr *AddrSpace, + SourceLocation AttrLoc); + + /// Same as above, but constructs the AddressSpace index if not provided. QualType BuildAddressSpaceAttr(QualType &T, Expr *AddrSpace, SourceLocation AttrLoc); diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index ebbf105d98..452ff6201a 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -3205,6 +3205,7 @@ bool AttributedType::isQualifier() const { case attr::TypeNullable: case attr::TypeNullUnspecified: case attr::LifetimeBound: + case attr::AddressSpace: return true; // All other type attributes aren't qualifiers; they rewrite the modified diff --git a/lib/AST/TypePrinter.cpp b/lib/AST/TypePrinter.cpp index 11f9ec33e9..a98b45bebd 100644 --- a/lib/AST/TypePrinter.cpp +++ b/lib/AST/TypePrinter.cpp @@ -257,11 +257,17 @@ bool TypePrinter::canPrefixQualifiers(const Type *T, case Type::FunctionProto: case Type::FunctionNoProto: case Type::Paren: - case Type::Attributed: case Type::PackExpansion: case Type::SubstTemplateTypeParm: CanPrefixQualifiers = false; break; + + case Type::Attributed: { + // We still want to print the address_space before the type if it is an + // address_space attribute. + const auto *AttrTy = cast(T); + CanPrefixQualifiers = AttrTy->getAttrKind() == attr::AddressSpace; + } } return CanPrefixQualifiers; @@ -1377,7 +1383,10 @@ void TypePrinter::printAttributedBefore(const AttributedType *T, if (T->getAttrKind() == attr::ObjCKindOf) OS << "__kindof "; - printBefore(T->getModifiedType(), OS); + if (T->getAttrKind() == attr::AddressSpace) + printBefore(T->getEquivalentType(), OS); + else + printBefore(T->getModifiedType(), OS); if (T->isMSTypeSpec()) { switch (T->getAttrKind()) { diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index 93b42829bb..b1aac72ea5 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -5787,28 +5787,27 @@ ParsedType Sema::ActOnObjCInstanceType(SourceLocation Loc) { // Type Attribute Processing //===----------------------------------------------------------------------===// -/// BuildAddressSpaceAttr - Builds a DependentAddressSpaceType if an expression -/// is uninstantiated. If instantiated it will apply the appropriate address space -/// to the type. This function allows dependent template variables to be used in -/// conjunction with the address_space attribute -QualType Sema::BuildAddressSpaceAttr(QualType &T, Expr *AddrSpace, - SourceLocation AttrLoc) { +/// Build an AddressSpace index from a constant expression and diagnose any +/// errors related to invalid address_spaces. Returns true on successfully +/// building an AddressSpace index. +static bool BuildAddressSpaceIndex(Sema &S, LangAS &ASIdx, + const Expr *AddrSpace, + SourceLocation AttrLoc) { if (!AddrSpace->isValueDependent()) { - llvm::APSInt addrSpace(32); - if (!AddrSpace->isIntegerConstantExpr(addrSpace, Context)) { - Diag(AttrLoc, diag::err_attribute_argument_type) + if (!AddrSpace->isIntegerConstantExpr(addrSpace, S.Context)) { + S.Diag(AttrLoc, diag::err_attribute_argument_type) << "'address_space'" << AANT_ArgumentIntegerConstant << AddrSpace->getSourceRange(); - return QualType(); + return false; } // Bounds checking. if (addrSpace.isSigned()) { if (addrSpace.isNegative()) { - Diag(AttrLoc, diag::err_attribute_address_space_negative) + S.Diag(AttrLoc, diag::err_attribute_address_space_negative) << AddrSpace->getSourceRange(); - return QualType(); + return false; } addrSpace.setIsSigned(false); } @@ -5817,14 +5816,28 @@ QualType Sema::BuildAddressSpaceAttr(QualType &T, Expr *AddrSpace, max = Qualifiers::MaxAddressSpace - (unsigned)LangAS::FirstTargetAddressSpace; if (addrSpace > max) { - Diag(AttrLoc, diag::err_attribute_address_space_too_high) + S.Diag(AttrLoc, diag::err_attribute_address_space_too_high) << (unsigned)max.getZExtValue() << AddrSpace->getSourceRange(); - return QualType(); + return false; } - LangAS ASIdx = + ASIdx = getLangASFromTargetAS(static_cast(addrSpace.getZExtValue())); + return true; + } + // Default value for DependentAddressSpaceTypes + ASIdx = LangAS::Default; + return true; +} + +/// BuildAddressSpaceAttr - Builds a DependentAddressSpaceType if an expression +/// is uninstantiated. If instantiated it will apply the appropriate address +/// space to the type. This function allows dependent template variables to be +/// used in conjunction with the address_space attribute +QualType Sema::BuildAddressSpaceAttr(QualType &T, LangAS ASIdx, Expr *AddrSpace, + SourceLocation AttrLoc) { + if (!AddrSpace->isValueDependent()) { if (DiagnoseMultipleAddrSpaceAttributes(*this, T.getAddressSpace(), ASIdx, AttrLoc)) return QualType(); @@ -5845,6 +5858,14 @@ QualType Sema::BuildAddressSpaceAttr(QualType &T, Expr *AddrSpace, return Context.getDependentAddressSpaceType(T, AddrSpace, AttrLoc); } +QualType Sema::BuildAddressSpaceAttr(QualType &T, Expr *AddrSpace, + SourceLocation AttrLoc) { + LangAS ASIdx; + if (!BuildAddressSpaceIndex(*this, ASIdx, AddrSpace, AttrLoc)) + return QualType(); + return BuildAddressSpaceAttr(T, ASIdx, AddrSpace, AttrLoc); +} + /// HandleAddressSpaceTypeAttribute - Process an address_space attribute on the /// specified type. The attribute contains 1 argument, the id of the address /// space for the type. @@ -5890,19 +5911,41 @@ static void HandleAddressSpaceTypeAttribute(QualType &Type, ASArgExpr = static_cast(Attr.getArgAsExpr(0)); } - // Create the DependentAddressSpaceType or append an address space onto - // the type. - QualType T = S.BuildAddressSpaceAttr(Type, ASArgExpr, Attr.getLoc()); + LangAS ASIdx; + if (!BuildAddressSpaceIndex(S, ASIdx, ASArgExpr, Attr.getLoc())) { + Attr.setInvalid(); + return; + } - if (!T.isNull()) { - ASTContext &Ctx = S.Context; - auto *ASAttr = ::new (Ctx) AddressSpaceAttr( - Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(), - static_cast(T.getQualifiers().getAddressSpace())); - Type = State.getAttributedType(ASAttr, T, T); + ASTContext &Ctx = S.Context; + auto *ASAttr = ::new (Ctx) AddressSpaceAttr( + Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(), + static_cast(ASIdx)); + + // If the expression is not value dependent (not templated), then we can + // apply the address space qualifiers just to the equivalent type. + // Otherwise, we make an AttributedType with the modified and equivalent + // type the same, and wrap it in a DependentAddressSpaceType. When this + // dependent type is resolved, the qualifier is added to the equivalent type + // later. + QualType T; + if (!ASArgExpr->isValueDependent()) { + QualType EquivType = + S.BuildAddressSpaceAttr(Type, ASIdx, ASArgExpr, Attr.getLoc()); + if (EquivType.isNull()) { + Attr.setInvalid(); + return; + } + T = State.getAttributedType(ASAttr, Type, EquivType); } else { - Attr.setInvalid(); + T = State.getAttributedType(ASAttr, Type, Type); + T = S.BuildAddressSpaceAttr(T, ASIdx, ASArgExpr, Attr.getLoc()); } + + if (!T.isNull()) + Type = T; + else + Attr.setInvalid(); } else { // The keyword-based type attributes imply which address space to use. ASIdx = Attr.asOpenCLLangAS(); @@ -7346,9 +7389,11 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type, if (!IsTypeAttr) continue; } - } else if (TAL != TAL_DeclChunk) { + } else if (TAL != TAL_DeclChunk && + attr.getKind() != ParsedAttr::AT_AddressSpace) { // Otherwise, only consider type processing for a C++11 attribute if // it's actually been applied to a type. + // We also allow C++11 address_space attributes to pass through. continue; } } diff --git a/test/AST/address_space_attribute.cpp b/test/AST/address_space_attribute.cpp new file mode 100644 index 0000000000..554c9ba0a1 --- /dev/null +++ b/test/AST/address_space_attribute.cpp @@ -0,0 +1,23 @@ +// RUN: %clang_cc1 %s -ast-dump | FileCheck %s + +// Veryify the ordering of the address_space attribute still comes before the +// type whereas other attributes are still printed after. + +template +void func() { + // CHECK: VarDecl {{.*}} x '__attribute__((address_space(1))) int *' + __attribute__((address_space(1))) int *x; + + // CHECK: VarDecl {{.*}} a 'int * __attribute__((noderef))' + int __attribute__((noderef)) * a; + + // CHECK: VarDecl {{.*}} y '__attribute__((address_space(2))) int *' + __attribute__((address_space(I))) int *y; + + // CHECK: VarDecl {{.*}} z '__attribute__((address_space(3))) int *' + [[clang::address_space(3)]] int *z; +} + +void func2() { + func<2>(); +} diff --git a/test/Index/print-type.m b/test/Index/print-type.m index 2afdfc0ff8..26389b2527 100644 --- a/test/Index/print-type.m +++ b/test/Index/print-type.m @@ -19,6 +19,6 @@ // CHECK: ObjCInstanceMethodDecl=methodIn:andOut::5:10 (variadic) [Bycopy,] [type=] [typekind=Invalid] [resulttype=id] [resulttypekind=ObjCId] [args= [int] [Int] [short *] [Pointer]] [isPOD=0] // CHECK: ParmDecl=i:5:27 (Definition) [In,] [type=int] [typekind=Int] [isPOD=1] // CHECK: ParmDecl=j:5:49 (Definition) [Out,] [type=short *] [typekind=Pointer] [isPOD=1] [pointeetype=short] [pointeekind=Short] -// CHECK: ParmDecl=p:6:36 (Definition) [type=__kindof Foo *] [typekind=ObjCObjectPointer] [canonicaltype=__kindof Foo *] [canonicaltypekind=ObjCObjectPointer] [basetype=Foo] [basekind=ObjCInterface] [isPOD=1] [pointeetype=Foo] [pointeekind=ObjCInterface] +// CHECK: ParmDecl=p:6:36 (Definition) [type=__kindof Foo *] [typekind=ObjCObjectPointer] [canonicaltype=__kindof Foo *] [canonicaltypekind=ObjCObjectPointer] [basetype=Foo] [basekind=ObjCInterface] [isPOD=1] [pointeetype=__kindof Foo] [pointeekind=ObjCObject] // CHECK: ObjCPropertyDecl=classProp:7:23 [class,] [type=int] [typekind=Int] [isPOD=1] // CHECK: ObjCInstanceMethodDecl=generic:11:12 [type=] [typekind=Invalid] [resulttype=SomeType] [resulttypekind=ObjCTypeParam] [isPOD=0] diff --git a/tools/libclang/CXType.cpp b/tools/libclang/CXType.cpp index f381a45469..55397d6fa7 100644 --- a/tools/libclang/CXType.cpp +++ b/tools/libclang/CXType.cpp @@ -128,7 +128,9 @@ CXType cxtype::MakeCXType(QualType T, CXTranslationUnit TU) { // Handle attributed types as the original type if (auto *ATT = T->getAs()) { if (!(TU->ParsingOptions & CXTranslationUnit_IncludeAttributedTypes)) { - return MakeCXType(ATT->getModifiedType(), TU); + // Return the equivalent type which represents the canonically + // equivalent type. + return MakeCXType(ATT->getEquivalentType(), TU); } } // Handle paren types as the original type