From: Aaron Ballman Date: Sun, 15 Dec 2013 13:05:48 +0000 (+0000) Subject: Allow target-specific attributes to share a spelling between different attributes... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b9b18ea1d77d4c39acd2a17a7b65ebf84313561b;p=clang Allow target-specific attributes to share a spelling between different attributes via the ParseKind field. Attributes will be given a common parsed attribute identifier (the AttributeList::AT_* enum), but retain distinct Attr subclasses. This new functionality is used to implement the ARM and MSP430 interrupt attribute. Patch reviewed by Richard Smith over IRC. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@197343 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td index f9f4ce1339..b268c80c4c 100644 --- a/include/clang/Basic/Attr.td +++ b/include/clang/Basic/Attr.td @@ -187,7 +187,22 @@ class InheritableAttr : Attr; /// A target-specific attribute that is meant to be processed via /// TargetAttributesSema::ProcessDeclAttribute. This class is meant to be used /// as a mixin with InheritableAttr or Attr depending on the attribute's needs. -class TargetSpecificAttr; +class TargetSpecificAttr { + // Attributes are generally required to have unique spellings for their names + // so that the parser can determine what kind of attribute it has parsed. + // However, target-specific attributes are special in that the attribute only + // "exists" for a given target. So two target-specific attributes can share + // the same name when they exist in different targets. To support this, a + // Kind can be explicitly specified for a target-specific attribute. This + // corresponds to the AttributeList::AT_* enum that is generated and it + // should contain a shared value between the attributes. + // + // Target-specific attributes which use this feature should ensure that the + // spellings match exactly betweeen the attributes, and if the arguments or + // subjects differ, should specify HasCustomParsing = 1 and implement their + // own parsing and semantic handling requirements as-needed. + string ParseKind; +} /// An inheritable parameter attribute is inherited by later /// redeclarations, even when it's written on a parameter. @@ -257,11 +272,15 @@ def Annotate : InheritableParamAttr { } def ARMInterrupt : InheritableAttr, TargetSpecificAttr { + // NOTE: If you add any additional spellings, MSP430Interrupt's spellings + // must match. let Spellings = [GNU<"interrupt">]; let Args = [EnumArgument<"Interrupt", "InterruptType", ["IRQ", "FIQ", "SWI", "ABORT", "UNDEF", ""], ["IRQ", "FIQ", "SWI", "ABORT", "UNDEF", "Generic"], 1>]; + let ParseKind = "Interrupt"; + let HasCustomParsing = 1; } def AsmLabel : InheritableAttr { @@ -522,13 +541,12 @@ def MSABI : InheritableAttr { } def MSP430Interrupt : InheritableAttr, TargetSpecificAttr { - // FIXME: this attribute is spelled the same as the ARMInterrupt attribute, - // but two attributes cannot currently share the same name because of the - // getAttrKind function. However, in this case, the attributes are for - // different targets, so sharing the same name but different arguments is a - // reasonable design. For now, this attribute will remain having no spelling. - let Spellings = []; + // NOTE: If you add any additional spellings, ARMInterrupt's spellings must + // match. + let Spellings = [GNU<"interrupt">]; let Args = [UnsignedArgument<"Number">]; + let ParseKind = "Interrupt"; + let HasCustomParsing = 1; } def Mips16 : InheritableAttr, TargetSpecificAttr { diff --git a/lib/Sema/TargetAttributesSema.cpp b/lib/Sema/TargetAttributesSema.cpp index e5eb591683..70cbbb547d 100644 --- a/lib/Sema/TargetAttributesSema.cpp +++ b/lib/Sema/TargetAttributesSema.cpp @@ -61,7 +61,7 @@ namespace { ARMAttributesSema() { } bool ProcessDeclAttribute(Scope *scope, Decl *D, const AttributeList &Attr, Sema &S) const { - if (Attr.getKind() == AttributeList::AT_ARMInterrupt) { + if (Attr.getKind() == AttributeList::AT_Interrupt) { HandleARMInterruptAttr(D, Attr, S); return true; } @@ -72,38 +72,47 @@ namespace { static void HandleMSP430InterruptAttr(Decl *d, const AttributeList &Attr, Sema &S) { - // FIXME: Check for decl - it should be void ()(void). - - Expr *NumParamsExpr = static_cast(Attr.getArgAsExpr(0)); - llvm::APSInt NumParams(32); - if (!NumParamsExpr->isIntegerConstantExpr(NumParams, S.Context)) { - S.Diag(Attr.getLoc(), diag::err_attribute_argument_type) - << Attr.getName() << AANT_ArgumentIntegerConstant - << NumParamsExpr->getSourceRange(); - return; - } + if (Attr.getNumArgs() != 1) { + S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) + << Attr.getName() << 1; + return; + } - unsigned Num = NumParams.getLimitedValue(255); - if ((Num & 1) || Num > 30) { - S.Diag(Attr.getLoc(), diag::err_attribute_argument_out_of_bounds) - << "interrupt" << (int)NumParams.getSExtValue() - << NumParamsExpr->getSourceRange(); - return; - } + if (!Attr.isArgExpr(0)) { + S.Diag(Attr.getLoc(), diag::err_attribute_argument_type) << Attr.getName() + << AANT_ArgumentIntegerConstant; + return; + } - d->addAttr(::new (S.Context) MSP430InterruptAttr(Attr.getLoc(), S.Context, Num)); - d->addAttr(::new (S.Context) UsedAttr(Attr.getLoc(), S.Context)); + // FIXME: Check for decl - it should be void ()(void). + Expr *NumParamsExpr = Attr.getArgAsExpr(0); + llvm::APSInt NumParams(32); + if (!NumParamsExpr->isIntegerConstantExpr(NumParams, S.Context)) { + S.Diag(Attr.getLoc(), diag::err_attribute_argument_type) + << Attr.getName() << AANT_ArgumentIntegerConstant + << NumParamsExpr->getSourceRange(); + return; + } + + unsigned Num = NumParams.getLimitedValue(255); + if ((Num & 1) || Num > 30) { + S.Diag(Attr.getLoc(), diag::err_attribute_argument_out_of_bounds) + << "interrupt" << (int)NumParams.getSExtValue() + << NumParamsExpr->getSourceRange(); + return; } + d->addAttr(::new (S.Context) MSP430InterruptAttr(Attr.getLoc(), S.Context, Num)); + d->addAttr(::new (S.Context) UsedAttr(Attr.getLoc(), S.Context)); +} + namespace { class MSP430AttributesSema : public TargetAttributesSema { public: MSP430AttributesSema() { } bool ProcessDeclAttribute(Scope *scope, Decl *D, const AttributeList &Attr, Sema &S) const { - // Because this attribute has no spelling (see the FIXME in Attr.td as to - // why), we must check for the name instead of the attribute kind. - if (Attr.getName()->getName() == "interrupt") { + if (Attr.getKind() == AttributeList::AT_Interrupt) { HandleMSP430InterruptAttr(D, Attr, S); return true; } diff --git a/test/Misc/ast-dump-arm-attr.c b/test/Misc/ast-dump-arm-attr.c new file mode 100644 index 0000000000..bec3531828 --- /dev/null +++ b/test/Misc/ast-dump-arm-attr.c @@ -0,0 +1,5 @@ +// RUN: %clang_cc1 -triple arm-apple-darwin -ast-dump -ast-dump-filter Test %s | FileCheck --strict-whitespace %s + +__attribute__((interrupt)) void Test(void); +// CHECK: FunctionDecl{{.*}}Test +// CHECK-NEXT: ARMInterruptAttr diff --git a/test/Misc/ast-dump-msp430-attr.c b/test/Misc/ast-dump-msp430-attr.c new file mode 100644 index 0000000000..170e0bef9d --- /dev/null +++ b/test/Misc/ast-dump-msp430-attr.c @@ -0,0 +1,5 @@ +// RUN: %clang_cc1 -triple msp430-unknown-unknown -ast-dump -ast-dump-filter Test %s | FileCheck --strict-whitespace %s + +__attribute__((interrupt(12))) void Test(void); +// CHECK: FunctionDecl{{.*}}Test +// CHECK-NEXT: MSP430InterruptAttr diff --git a/test/Sema/attr-msp430.c b/test/Sema/attr-msp430.c new file mode 100644 index 0000000000..d08cd8ecc9 --- /dev/null +++ b/test/Sema/attr-msp430.c @@ -0,0 +1,6 @@ +// RUN: %clang_cc1 -triple msp430-unknown-unknown -fsyntax-only -verify %s + +int i; +void f(void) __attribute__((interrupt(i))); /* expected-error {{'interrupt' attribute requires an integer constant}} */ + +void f2(void) __attribute__((interrupt(12))); diff --git a/utils/TableGen/ClangAttrEmitter.cpp b/utils/TableGen/ClangAttrEmitter.cpp index 21920644f3..3b2b9cef76 100644 --- a/utils/TableGen/ClangAttrEmitter.cpp +++ b/utils/TableGen/ClangAttrEmitter.cpp @@ -96,6 +96,35 @@ static StringRef NormalizeAttrSpelling(StringRef AttrSpelling) { return AttrSpelling; } +typedef std::vector > ParsedAttrMap; + +static ParsedAttrMap getParsedAttrList(const RecordKeeper &Records) { + std::vector Attrs = Records.getAllDerivedDefinitions("Attr"); + std::set Seen; + ParsedAttrMap R; + for (std::vector::iterator I = Attrs.begin(), E = Attrs.end(); + I != E; ++I) { + Record &Attr = **I; + if (Attr.getValueAsBit("SemaHandler")) { + std::string AN; + if (Attr.isSubClassOf("TargetSpecificAttr") && + !Attr.isValueUnset("ParseKind")) { + AN = Attr.getValueAsString("ParseKind"); + + // If this attribute has already been handled, it does not need to be + // handled again. + if (Seen.find(AN) != Seen.end()) + continue; + Seen.insert(AN); + } else + AN = NormalizeAttrName(Attr.getName()).str(); + + R.push_back(std::make_pair(AN, *I)); + } + } + return R; +} + namespace { class Argument { std::string lowerName, upperName; @@ -1500,23 +1529,17 @@ void EmitClangAttrSpellingListIndex(RecordKeeper &Records, raw_ostream &OS) { "into internal identifiers", OS); OS << - " unsigned Index = 0;\n" " switch (AttrKind) {\n" " default:\n" " llvm_unreachable(\"Unknown attribute kind!\");\n" " break;\n"; - std::vector Attrs = Records.getAllDerivedDefinitions("Attr"); - for (std::vector::const_iterator I = Attrs.begin(), E = Attrs.end(); + ParsedAttrMap Attrs = getParsedAttrList(Records); + for (ParsedAttrMap::const_iterator I = Attrs.begin(), E = Attrs.end(); I != E; ++I) { - Record &R = **I; - // We only care about attributes that participate in Sema checking, so - // skip those attributes that are not able to make their way to Sema. - if (!R.getValueAsBit("SemaHandler")) - continue; - + Record &R = *I->second; std::vector Spellings = R.getValueAsListOfDefs("Spellings"); - OS << " case AT_" << R.getName() << " : {\n"; + OS << " case AT_" << I->first << ": {\n"; for (unsigned I = 0; I < Spellings.size(); ++ I) { SmallString<16> Namespace; if (Spellings[I]->getValueAsString("Variety") == "CXX11") @@ -1542,7 +1565,7 @@ void EmitClangAttrSpellingListIndex(RecordKeeper &Records, raw_ostream &OS) { } OS << " }\n"; - OS << " return Index;\n"; + OS << " return 0;\n"; } // Emits the LateParsed property for attributes. @@ -1647,23 +1670,6 @@ void EmitClangAttrTemplateInstantiate(RecordKeeper &Records, raw_ostream &OS) { << "} // end namespace clang\n"; } -typedef std::vector > ParsedAttrMap; - -static ParsedAttrMap getParsedAttrList(const RecordKeeper &Records) { - std::vector Attrs = Records.getAllDerivedDefinitions("Attr"); - ParsedAttrMap R; - for (std::vector::iterator I = Attrs.begin(), E = Attrs.end(); - I != E; ++I) { - Record &Attr = **I; - if (Attr.getValueAsBit("SemaHandler")) { - StringRef AttrName = Attr.getName(); - AttrName = NormalizeAttrName(AttrName); - R.push_back(std::make_pair(AttrName.str(), *I)); - } - } - return R; -} - // Emits the list of parsed attributes. void EmitClangAttrParsedAttrList(RecordKeeper &Records, raw_ostream &OS) { emitSourceFileHeader("List of all attributes that Clang recognizes", OS); @@ -1996,6 +2002,7 @@ void EmitClangAttrParsedAttrKinds(RecordKeeper &Records, raw_ostream &OS) { std::vector Attrs = Records.getAllDerivedDefinitions("Attr"); std::vector Matches; + std::set Seen; for (std::vector::iterator I = Attrs.begin(), E = Attrs.end(); I != E; ++I) { Record &Attr = **I; @@ -2005,7 +2012,16 @@ void EmitClangAttrParsedAttrKinds(RecordKeeper &Records, raw_ostream &OS) { if (SemaHandler || Ignored) { std::vector Spellings = Attr.getValueAsListOfDefs("Spellings"); - StringRef AttrName = NormalizeAttrName(StringRef(Attr.getName())); + std::string AttrName; + if (Attr.isSubClassOf("TargetSpecificAttr") && + !Attr.isValueUnset("ParseKind")) { + AttrName = Attr.getValueAsString("ParseKind"); + if (Seen.find(AttrName) != Seen.end()) + continue; + Seen.insert(AttrName); + } else + AttrName = NormalizeAttrName(StringRef(Attr.getName())).str(); + for (std::vector::const_iterator I = Spellings.begin(), E = Spellings.end(); I != E; ++I) { std::string RawSpelling = (*I)->getValueAsString("Name"); @@ -2021,7 +2037,7 @@ void EmitClangAttrParsedAttrKinds(RecordKeeper &Records, raw_ostream &OS) { Matches.push_back( StringMatcher::StringPair( StringRef(Spelling), - "return AttributeList::AT_" + AttrName.str() + ";")); + "return AttributeList::AT_" + AttrName + ";")); else Matches.push_back( StringMatcher::StringPair(