From d068607c136298bec0891d750389a55bac9f5c98 Mon Sep 17 00:00:00 2001 From: Aaron Ballman Date: Wed, 11 Sep 2013 19:47:58 +0000 Subject: [PATCH] Tablegen now generates a StringSwitch for attributes containing enumeration arguments to map strings to the proper enumeration value. This makes error checking more consistent and reduces the amount of hand-written code required. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@190545 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 7 +- lib/AST/AttrImpl.cpp | 1 + lib/Sema/SemaAttr.cpp | 17 +--- lib/Sema/SemaDeclAttr.cpp | 96 +++++++--------------- lib/Sema/SemaType.cpp | 4 +- test/Misc/warning-flags.c | 3 +- test/SemaCXX/warn-consumed-parsing.cpp | 2 +- utils/TableGen/ClangAttrEmitter.cpp | 25 ++++++ 8 files changed, 66 insertions(+), 89 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 7f8184cdb1..ee82137ae7 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1836,7 +1836,6 @@ def err_format_attribute_implicit_this_format_string : Error< "string">; def err_common_not_supported_cplusplus : Error< "common attribute is not supported in C++">; -def warn_unknown_method_family : Warning<"unrecognized method family">; def err_init_method_bad_return_type : Error< "init methods must return an object pointer type, not %0">; def err_attribute_invalid_size : Error< @@ -2202,8 +2201,6 @@ def warn_attr_on_unconsumable_class : Warning< def warn_return_typestate_for_unconsumable_type : Warning< "return state set for an unconsumable type '%0'">, InGroup, DefaultIgnore; -def warn_unknown_consumed_state : Warning< - "unknown consumed analysis state '%0'">, InGroup, DefaultIgnore; def warn_return_typestate_mismatch : Warning< "return value not in expected state; expected '%0', observed '%1'">, InGroup, DefaultIgnore; @@ -2317,9 +2314,9 @@ def warn_transparent_union_attribute_zero_fields : Warning< "transparent_union attribute ignored">, InGroup; def warn_attribute_type_not_supported : Warning< - "'%0' attribute argument not supported: %1">, + "%0 attribute argument not supported: %1">, InGroup; -def warn_attribute_unknown_visibility : Warning<"unknown visibility '%0'">, +def warn_attribute_unknown_visibility : Warning<"unknown visibility %0">, InGroup; def warn_attribute_protected_visibility : Warning<"target does not support 'protected' visibility; using 'default'">, diff --git a/lib/AST/AttrImpl.cpp b/lib/AST/AttrImpl.cpp index daf65e56bd..7af3c8b162 100644 --- a/lib/AST/AttrImpl.cpp +++ b/lib/AST/AttrImpl.cpp @@ -15,6 +15,7 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/Expr.h" #include "clang/AST/Type.h" +#include "llvm/ADT/StringSwitch.h" using namespace clang; Attr::~Attr() { } diff --git a/lib/Sema/SemaAttr.cpp b/lib/Sema/SemaAttr.cpp index ebe294a70c..8f9ab32517 100644 --- a/lib/Sema/SemaAttr.cpp +++ b/lib/Sema/SemaAttr.cpp @@ -368,21 +368,12 @@ void Sema::ActOnPragmaVisibility(const IdentifierInfo* VisType, SourceLocation PragmaLoc) { if (VisType) { // Compute visibility to use. - VisibilityAttr::VisibilityType type; - if (VisType->isStr("default")) - type = VisibilityAttr::Default; - else if (VisType->isStr("hidden")) - type = VisibilityAttr::Hidden; - else if (VisType->isStr("internal")) - type = VisibilityAttr::Hidden; // FIXME - else if (VisType->isStr("protected")) - type = VisibilityAttr::Protected; - else { - Diag(PragmaLoc, diag::warn_attribute_unknown_visibility) << - VisType->getName(); + VisibilityAttr::VisibilityType T; + if (!VisibilityAttr::ConvertStrToVisibilityType(VisType->getName(), T)) { + Diag(PragmaLoc, diag::warn_attribute_unknown_visibility) << VisType; return; } - PushPragmaVisibility(*this, type, PragmaLoc); + PushPragmaVisibility(*this, T, PragmaLoc); } else { PopPragmaVisibility(false, PragmaLoc); } diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index 47b130888f..c226215c5d 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -947,19 +947,13 @@ static void handleConsumableAttr(Sema &S, Decl *D, const AttributeList &Attr) { ConsumableAttr::ConsumedState DefaultState; if (Attr.isArgIdent(0)) { - StringRef Param = Attr.getArgAsIdent(0)->Ident->getName(); - - if (Param == "unknown") - DefaultState = ConsumableAttr::Unknown; - else if (Param == "consumed") - DefaultState = ConsumableAttr::Consumed; - else if (Param == "unconsumed") - DefaultState = ConsumableAttr::Unconsumed; - else { - S.Diag(Attr.getLoc(), diag::warn_unknown_consumed_state) << Param; + IdentifierLoc *IL = Attr.getArgAsIdent(0); + if (!ConsumableAttr::ConvertStrToConsumedState(IL->Ident->getName(), + DefaultState)) { + S.Diag(IL->Loc, diag::warn_attribute_type_not_supported) + << Attr.getName() << IL->Ident; return; } - } else { S.Diag(Attr.getLoc(), diag::err_attribute_argument_type) << Attr.getName() << AANT_ArgumentIdentifier; @@ -1062,19 +1056,13 @@ static void handleReturnTypestateAttr(Sema &S, Decl *D, ReturnTypestateAttr::ConsumedState ReturnState; if (Attr.isArgIdent(0)) { - StringRef Param = Attr.getArgAsIdent(0)->Ident->getName(); - - if (Param == "unknown") { - ReturnState = ReturnTypestateAttr::Unknown; - } else if (Param == "consumed") { - ReturnState = ReturnTypestateAttr::Consumed; - } else if (Param == "unconsumed") { - ReturnState = ReturnTypestateAttr::Unconsumed; - } else { - S.Diag(Attr.getLoc(), diag::warn_unknown_consumed_state) << Param; + IdentifierLoc *IL = Attr.getArgAsIdent(0); + if (!ReturnTypestateAttr::ConvertStrToConsumedState(IL->Ident->getName(), + ReturnState)) { + S.Diag(IL->Loc, diag::warn_attribute_type_not_supported) + << Attr.getName() << IL->Ident; return; } - } else { S.Diag(Attr.getLoc(), diag::err_attribute_argument_type) << Attr.getName() << AANT_ArgumentIdentifier; @@ -2340,25 +2328,18 @@ static void handleVisibilityAttr(Sema &S, Decl *D, const AttributeList &Attr, StringRef TypeStr = Str->getString(); VisibilityAttr::VisibilityType type; + if (!VisibilityAttr::ConvertStrToVisibilityType(TypeStr, type)) { + S.Diag(Attr.getLoc(), diag::warn_attribute_type_not_supported) + << Attr.getName() << TypeStr; + return; + } - if (TypeStr == "default") + // Complain about attempts to use protected visibility on targets + // (like Darwin) that don't support it. + if (type == VisibilityAttr::Protected && + !S.Context.getTargetInfo().hasProtectedVisibility()) { + S.Diag(Attr.getLoc(), diag::warn_attribute_protected_visibility); type = VisibilityAttr::Default; - else if (TypeStr == "hidden") - type = VisibilityAttr::Hidden; - else if (TypeStr == "internal") - type = VisibilityAttr::Hidden; // FIXME - else if (TypeStr == "protected") { - // Complain about attempts to use protected visibility on targets - // (like Darwin) that don't support it. - if (!S.Context.getTargetInfo().hasProtectedVisibility()) { - S.Diag(Attr.getLoc(), diag::warn_attribute_protected_visibility); - type = VisibilityAttr::Default; - } else { - type = VisibilityAttr::Protected; - } - } else { - S.Diag(Attr.getLoc(), diag::warn_attribute_unknown_visibility) << TypeStr; - return; } unsigned Index = Attr.getAttributeSpellingListIndex(); @@ -2388,31 +2369,16 @@ static void handleObjCMethodFamilyAttr(Sema &S, Decl *decl, << Attr.getName() << 1 << AANT_ArgumentIdentifier; return; } - - IdentifierLoc *IL = Attr.getArgAsIdent(0); - StringRef param = IL->Ident->getName(); - ObjCMethodFamilyAttr::FamilyKind family; - if (param == "none") - family = ObjCMethodFamilyAttr::OMF_None; - else if (param == "alloc") - family = ObjCMethodFamilyAttr::OMF_alloc; - else if (param == "copy") - family = ObjCMethodFamilyAttr::OMF_copy; - else if (param == "init") - family = ObjCMethodFamilyAttr::OMF_init; - else if (param == "mutableCopy") - family = ObjCMethodFamilyAttr::OMF_mutableCopy; - else if (param == "new") - family = ObjCMethodFamilyAttr::OMF_new; - else { - // Just warn and ignore it. This is future-proof against new - // families being used in system headers. - S.Diag(IL->Loc, diag::warn_unknown_method_family); + IdentifierLoc *IL = Attr.getArgAsIdent(0); + ObjCMethodFamilyAttr::FamilyKind F; + if (!ObjCMethodFamilyAttr::ConvertStrToFamilyKind(IL->Ident->getName(), F)) { + S.Diag(IL->Loc, diag::warn_attribute_type_not_supported) << Attr.getName() + << IL->Ident; return; } - if (family == ObjCMethodFamilyAttr::OMF_init && + if (F == ObjCMethodFamilyAttr::OMF_init && !method->getResultType()->isObjCObjectPointerType()) { S.Diag(method->getLocation(), diag::err_init_method_bad_return_type) << method->getResultType(); @@ -2421,7 +2387,7 @@ static void handleObjCMethodFamilyAttr(Sema &S, Decl *decl, } method->addAttr(new (S.Context) ObjCMethodFamilyAttr(Attr.getRange(), - S.Context, family)); + S.Context, F)); } static void handleObjCExceptionAttr(Sema &S, Decl *D, @@ -2488,11 +2454,9 @@ static void handleBlocksAttr(Sema &S, Decl *D, const AttributeList &Attr) { IdentifierInfo *II = Attr.getArgAsIdent(0)->Ident; BlocksAttr::BlockType type; - if (II->isStr("byref")) - type = BlocksAttr::ByRef; - else { - S.Diag(Attr.getLoc(), diag::warn_attribute_type_not_supported) << "blocks" - << II; + if (!BlocksAttr::ConvertStrToBlockType(II->getName(), type)) { + S.Diag(Attr.getLoc(), diag::warn_attribute_type_not_supported) + << Attr.getName() << II; return; } diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index 2acfb26700..7abc36afb4 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -4012,7 +4012,7 @@ static bool handleObjCOwnershipTypeAttr(TypeProcessingState &state, lifetime = Qualifiers::OCL_Autoreleasing; else { S.Diag(AttrLoc, diag::warn_attribute_type_not_supported) - << "objc_ownership" << II; + << attr.getName() << II; attr.setInvalid(); return true; } @@ -4146,7 +4146,7 @@ static bool handleObjCGCTypeAttr(TypeProcessingState &state, GCAttr = Qualifiers::Strong; else { S.Diag(attr.getLoc(), diag::warn_attribute_type_not_supported) - << "objc_gc" << II; + << attr.getName() << II; attr.setInvalid(); return true; } diff --git a/test/Misc/warning-flags.c b/test/Misc/warning-flags.c index 0062573ac1..3ba445042e 100644 --- a/test/Misc/warning-flags.c +++ b/test/Misc/warning-flags.c @@ -18,7 +18,7 @@ This test serves two purposes: The list of warnings below should NEVER grow. It should gradually shrink to 0. -CHECK: Warnings without flags (135): +CHECK: Warnings without flags (134): CHECK-NEXT: ext_delete_void_ptr_operand CHECK-NEXT: ext_expected_semi_decl_list CHECK-NEXT: ext_explicit_specialization_storage_class @@ -150,7 +150,6 @@ CHECK-NEXT: warn_unavailable_fwdclass_message CHECK-NEXT: warn_undef_interface CHECK-NEXT: warn_undef_interface_suggest CHECK-NEXT: warn_undef_protocolref -CHECK-NEXT: warn_unknown_method_family CHECK-NEXT: warn_use_out_of_scope_declaration CHECK-NEXT: warn_weak_identifier_undeclared CHECK-NEXT: warn_weak_import diff --git a/test/SemaCXX/warn-consumed-parsing.cpp b/test/SemaCXX/warn-consumed-parsing.cpp index 001fb866b8..9275861985 100644 --- a/test/SemaCXX/warn-consumed-parsing.cpp +++ b/test/SemaCXX/warn-consumed-parsing.cpp @@ -39,7 +39,7 @@ class CONSUMABLE(unknown) AttrTester1 { bool testsUnconsumed() TESTS_UNCONSUMED; }; -AttrTester1 returnTypestateTester0() RETURN_TYPESTATE(not_a_state); // expected-warning {{unknown consumed analysis state 'not_a_state'}} +AttrTester1 returnTypestateTester0() RETURN_TYPESTATE(not_a_state); // expected-warning {{'return_typestate' attribute argument not supported: 'not_a_state'}} AttrTester1 returnTypestateTester1() RETURN_TYPESTATE(42); // expected-error {{'return_typestate' attribute requires an identifier}} class AttrTester2 { diff --git a/utils/TableGen/ClangAttrEmitter.cpp b/utils/TableGen/ClangAttrEmitter.cpp index 8922c5c023..7f42e455bd 100644 --- a/utils/TableGen/ClangAttrEmitter.cpp +++ b/utils/TableGen/ClangAttrEmitter.cpp @@ -134,6 +134,8 @@ namespace { virtual void writeDump(raw_ostream &OS) const = 0; virtual void writeDumpChildren(raw_ostream &OS) const {} virtual void writeHasChildren(raw_ostream &OS) const { OS << "false"; } + + virtual bool isEnumArg() const { return false; } }; class SimpleArgument : public Argument { @@ -524,6 +526,8 @@ namespace { assert(!uniques.empty()); } + bool isEnumArg() const { return true; } + void writeAccessors(raw_ostream &OS) const { OS << " " << type << " get" << getUpperName() << "() const {\n"; OS << " return " << getLowerName() << ";\n"; @@ -583,6 +587,22 @@ namespace { } OS << " }\n"; } + + void writeConversion(raw_ostream &OS) const { + OS << " static bool ConvertStrTo" << type << "(StringRef Val, "; + OS << type << " &Out) {\n"; + OS << " Optional<" << type << "> R = llvm::StringSwitch >(Val)\n"; + for (size_t I = 0; I < enums.size(); ++I) { + OS << " .Case(\"" << values[I] << "\", "; + OS << getAttrName() << "Attr::" << enums[I] << ")\n"; + } + OS << " .Default(Optional<" << type << ">());\n"; + OS << " if (R) {\n"; + OS << " Out = *R;\n return true;\n }\n"; + OS << " return false;\n"; + OS << " }\n"; + } }; class VersionArgument : public Argument { @@ -1027,6 +1047,11 @@ void EmitClangAttrClass(RecordKeeper &Records, raw_ostream &OS) { for (ai = Args.begin(); ai != ae; ++ai) { (*ai)->writeAccessors(OS); OS << "\n\n"; + + if ((*ai)->isEnumArg()) { + EnumArgument *EA = (EnumArgument *)*ai; + EA->writeConversion(OS); + } } OS << R.getValueAsString("AdditionalMembers"); -- 2.40.0