r186940 - Consolidate several attribute argument diagnostics into a single, selectable diagnostic. This makes the diagnostic more consistent.

Aaron Ballman aaron at aaronballman.com
Tue Jul 23 10:37:23 PDT 2013


I've gone back to getName in r186966

~Aaron

On Tue, Jul 23, 2013 at 12:38 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> I agree -- I will go back to using Attr.getName().  The way we report
> attribute names already needs some work (for instance, the scope names
> are left off entirely), so this will require a bigger change to
> rectify.
>
> ~Aaron
>
> On Tue, Jul 23, 2013 at 12:06 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>> On Tue, Jul 23, 2013 at 7:03 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>> Author: aaronballman
>>> Date: Tue Jul 23 09:03:57 2013
>>> New Revision: 186940
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=186940&view=rev
>>> Log:
>>> Consolidate several attribute argument diagnostics into a single, selectable diagnostic.  This makes the diagnostic more consistent.
>>>
>>> Modified:
>>>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>>     cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>>>     cfe/trunk/lib/Sema/SemaType.cpp
>>>     cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp
>>>
>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=186940&r1=186939&r2=186940&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Jul 23 09:03:57 2013
>>> @@ -1787,8 +1787,9 @@ def err_alignas_mismatch : Error<
>>>    "redeclaration has different alignment requirement (%1 vs %0)">;
>>>  def err_alignas_underaligned : Error<
>>>    "requested alignment is less than minimum alignment of %1 for type %0">;
>>> -def err_attribute_first_argument_not_int_or_bool : Error<
>>> -  "%0 attribute first argument must be of int or bool type">;
>>> +def err_attribute_argument_n_type : Error<
>>> +  "%0 attribute requires parameter %1 to be %select{int or bool|an integer "
>>> +  "constant|a string|an identifier}2">;
>>>  def err_attribute_argument_outof_range : Error<
>>>    "init_priority attribute requires integer constant between "
>>>    "101 and 65535 inclusive">;
>>> @@ -1797,12 +1798,6 @@ def err_init_priority_object_attr : Erro
>>>    "of objects of class type">;
>>>  def err_attribute_argument_vec_type_hint : Error<
>>>    "invalid attribute argument %0 - expecting a vector or vectorizable scalar type">;
>>> -def err_attribute_argument_n_not_int : Error<
>>> -  "'%0' attribute requires parameter %1 to be an integer constant">;
>>> -def err_attribute_argument_n_not_string : Error<
>>> -  "'%0' attribute requires parameter %1 to be a string">;
>>> -def err_attribute_argument_n_not_identifier : Error<
>>> -  "'%0' attribute requires parameter %1 to be an identifier">;
>>>  def err_attribute_argument_out_of_bounds : Error<
>>>    "'%0' attribute parameter %1 is out of bounds">;
>>>  def err_attribute_uuid_malformed_guid : Error<
>>> @@ -2015,10 +2010,10 @@ def err_attribute_wrong_decl_type : Erro
>>>    "variables, functions and tag types|thread-local variables|"
>>>    "variables and fields|variables, data members and tag types|"
>>>    "types and namespaces|Objective-C interfaces}1">;
>>> -def warn_type_attribute_wrong_type : Warning<
>>> -  "'%0' only applies to %select{function|pointer|"
>>> -  "Objective-C object or block pointer}1 types; type here is %2">,
>>> -  InGroup<IgnoredAttributes>;
>>> +def warn_type_attribute_wrong_type : Warning<
>>> +  "'%0' only applies to %select{function|pointer|"
>>> +  "Objective-C object or block pointer}1 types; type here is %2">,
>>> +  InGroup<IgnoredAttributes>;
>>>  def warn_attribute_requires_functions_or_static_globals : Warning<
>>>    "%0 only applies to variables with static storage duration and functions">,
>>>    InGroup<IgnoredAttributes>;
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=186940&r1=186939&r2=186940&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Jul 23 09:03:57 2013
>>> @@ -56,6 +56,15 @@ enum AttributeDeclKind {
>>>    ExpectedObjectiveCInterface
>>>  };
>>>
>>> +/// These constants match the enumerated choices of
>>> +/// err_attribute_argument_n_type.
>>> +enum AttributeArgumentNType {
>>> +  ArgumentIntOrBool,
>>> +  ArgumentIntegerConstant,
>>> +  ArgumentString,
>>> +  ArgumentIdentifier
>>> +};
>>> +
>>>  //===----------------------------------------------------------------------===//
>>>  //  Helper functions
>>>  //===----------------------------------------------------------------------===//
>>> @@ -252,8 +261,9 @@ static bool checkFunctionOrMethodArgumen
>>>    llvm::APSInt IdxInt;
>>>    if (IdxExpr->isTypeDependent() || IdxExpr->isValueDependent() ||
>>>        !IdxExpr->isIntegerConstantExpr(IdxInt, S.Context)) {
>>> -    S.Diag(AttrLoc, diag::err_attribute_argument_n_not_int)
>>> -      << AttrName << AttrArgNum << IdxExpr->getSourceRange();
>>> +    std::string Name = std::string("'") + AttrName.str() + std::string("'");
>>> +    S.Diag(AttrLoc, diag::err_attribute_argument_n_type) << Name.c_str()
>>> +      << AttrArgNum << ArgumentIntegerConstant << IdxExpr->getSourceRange();
>>>      return false;
>>>    }
>>>
>>> @@ -832,8 +842,8 @@ static bool checkTryLockFunAttrCommon(Se
>>>    }
>>>
>>>    if (!isIntOrBool(Attr.getArg(0))) {
>>> -    S.Diag(Attr.getLoc(), diag::err_attribute_first_argument_not_int_or_bool)
>>> -      << Attr.getName();
>>> +    S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
>>> +      << Attr.getName() << 1 << ArgumentIntOrBool;
>>>      return false;
>>>    }
>>>
>>> @@ -1324,8 +1334,8 @@ static void handleOwnershipAttr(Sema &S,
>>>    // a list append function may well be __attribute((ownership_holds)).
>>>
>>>    if (!AL.getParameterName()) {
>>> -    S.Diag(AL.getLoc(), diag::err_attribute_argument_n_not_string)
>>> -        << AL.getName()->getName() << 1;
>>> +    S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
>>> +      << AL.getName()->getName() << 1 << ArgumentString;
>>>      return;
>>>    }
>>>    // Figure out our Kind, and check arguments while we're at it.
>>> @@ -1530,8 +1540,8 @@ static void handleWeakRefAttr(Sema &S, D
>>>      StringLiteral *Str = dyn_cast<StringLiteral>(Arg);
>>>
>>>      if (!Str || !Str->isAscii()) {
>>> -      S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_not_string)
>>> -          << "weakref" << 1;
>>> +      S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
>>> +        << Attr.getName() << 1 << ArgumentString;
>>>        return;
>>>      }
>>>      // GCC will accept anything as the argument of weakref. Should we
>>> @@ -1555,8 +1565,8 @@ static void handleAliasAttr(Sema &S, Dec
>>>    StringLiteral *Str = dyn_cast<StringLiteral>(Arg);
>>>
>>>    if (!Str || !Str->isAscii()) {
>>> -    S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_not_string)
>>> -      << "alias" << 1;
>>> +    S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
>>> +      << Attr.getName() << 1 << ArgumentString;
>>>      return;
>>>    }
>>>
>>> @@ -1985,8 +1995,9 @@ static void handleConstructorAttr(Sema &
>>>      llvm::APSInt Idx(32);
>>>      if (E->isTypeDependent() || E->isValueDependent() ||
>>>          !E->isIntegerConstantExpr(Idx, S.Context)) {
>>> -      S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_not_int)
>>> -        << "constructor" << 1 << E->getSourceRange();
>>> +      S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
>>> +        << Attr.getName() << 1 << ArgumentIntegerConstant
>>> +        << E->getSourceRange();
>>>        return;
>>>      }
>>>      priority = Idx.getZExtValue();
>>> @@ -2016,8 +2027,9 @@ static void handleDestructorAttr(Sema &S
>>>      llvm::APSInt Idx(32);
>>>      if (E->isTypeDependent() || E->isValueDependent() ||
>>>          !E->isIntegerConstantExpr(Idx, S.Context)) {
>>> -      S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_not_int)
>>> -        << "destructor" << 1 << E->getSourceRange();
>>> +      S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
>>> +        << Attr.getName() << 1 << ArgumentIntegerConstant
>>> +        << E->getSourceRange();
>>>        return;
>>>      }
>>>      priority = Idx.getZExtValue();
>>> @@ -2387,8 +2399,8 @@ static void handleVisibilityAttr(Sema &S
>>>    StringLiteral *Str = dyn_cast<StringLiteral>(Arg);
>>>
>>>    if (!Str || !Str->isAscii()) {
>>> -    S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_not_string)
>>> -      << (isTypeVisibility ? "type_visibility" : "visibility") << 1;
>>> +    S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
>>> +      << Attr.getName() << 1 << ArgumentString;
>>>      return;
>>>    }
>>>
>>> @@ -2439,8 +2451,8 @@ static void handleObjCMethodFamilyAttr(S
>>>
>>>    if (Attr.getNumArgs() != 0 || !Attr.getParameterName()) {
>>>      if (!Attr.getParameterName() && Attr.getNumArgs() == 1) {
>>> -      S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_not_string)
>>> -        << "objc_method_family" << 1;
>>> +      S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
>>> +        << Attr.getName() << 1 << ArgumentString;
>>>      } else {
>>>        S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 0;
>>>      }
>>> @@ -2546,8 +2558,8 @@ handleOverloadableAttr(Sema &S, Decl *D,
>>>
>>>  static void handleBlocksAttr(Sema &S, Decl *D, const AttributeList &Attr) {
>>>    if (!Attr.getParameterName()) {
>>> -    S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_not_string)
>>> -      << "blocks" << 1;
>>> +    S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
>>> +      << Attr.getName() << 1 << ArgumentString;
>>>      return;
>>>    }
>>>
>>> @@ -2577,14 +2589,20 @@ static void handleSentinelAttr(Sema &S,
>>>      return;
>>>    }
>>>
>>> +  // Normalize the attribute name, __foo__ becomes foo.
>>> +  StringRef AttrName = Attr.getName()->getName();
>>> +  if (AttrName.startswith("__") && AttrName.endswith("__"))
>>> +    AttrName = AttrName.substr(2, AttrName.size() - 4);
>>
>> Per gcc attribute syntax rules, any attribute can have the extra
>> underscores.  Special-casing the sentinel attribute doesn't seem like
>> the right approach.
>>
>> -Eli



More information about the cfe-commits mailing list