On Thu, Sep 6, 2018 at 10:00 AM, Andrew Savonichev
<andrew.savonic...@intel.com> wrote:
> Hi Aaron,
>
>> On Thu, Sep 6, 2018 at 7:54 AM, Andrew Savonichev via cfe-commits
>> <cfe-commits@lists.llvm.org> wrote:
>>> Author: asavonic
>>> Date: Thu Sep 6 04:54:09 2018
>>> New Revision: 341539
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=341539&view=rev
>>> Log:
>>> [OpenCL] Disallow negative attribute arguments
>>>
>>> [...]
>>>
>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=341539&r1=341538&r2=341539&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep 6
>>> 04:54:09 2018
>>> @@ -2529,6 +2529,8 @@ def err_attribute_argument_type : Error<
>>> "constant|a string|an identifier}1">;
>>> def err_attribute_argument_outof_range : Error<
>>> "%0 attribute requires integer constant between %1 and %2 inclusive">;
>>> +def err_attribute_argument_negative : Error<
>>> + "negative argument is not allowed for %0 attribute">;
>>
>> I don't think we need a new diagnostic here as we already have
>> err_attribute_requires_positive_integer.
>
> `err_attribute_requires_positive_integer' implies that a zero argument
> is not allowed, while `err_attribute_argument_negative' should fire only
> on a strictly negative argument.
>
> I can merge these diagnostics into one (using %select), but I'm not sure
> if it is appropriate.
That's a subtle distinction, but a good catch! I think it's fine to
change it to: "%0 attribute requires a %select{positive|non-negative}1
integral compile time constant expression" but if you think of wording
that makes the distinction less subtle, that'd be even better.
~Aaron
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
