[cfe-dev] Feature request: Don't warn for specified "unknown" attribute

[Aaron Ballman via cfe-dev]

On Tue, Apr 16, 2019 at 2:45 PM Justin Bassett via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
>>
>> That's correct, but it's useful beyond just detecting typos. Different
>> vendors support different sets of attributes and this diagnostic is
>> how a developer can tell whether Clang is ignoring a given attribute
>> or not. It can be very difficult to tell the difference between
>> "parsed and silently ignoring this attribute" and "parsed attribute
>> and it functions as designed" depending on the attribute without this
>> diagnostic. For example, in order to tell whether
>> __attribute__((deprecated)) is supported, you would have to alter code
>> to see that it's behaving by design; otherwise, the declarations are
>> just silently marked as deprecated.
>
>
> Indeed, which is why I want the warning turned on by default (not that it's a default compiler warning, but that it's on for all attributes "by default" when the warning is turned on).
>
>> > To solve this, I propose that we add the ability to specify attributes to ignore for -Wunknown-attributes. In other words, something like this: '-Wignore-unknown-attribute=some_ns::some_attribute'.
>>
>> What issue is being solved, though? If the user wishes to ignore a
>> specific attribute on Clang because it's not known to the
>> implementation, the better solution is to use the preprocessor to
>> provide decent fallback behavior for the attribute. This is why
>> __has_attribute and friends exist. Is there a different problem that
>> this new warning flag behavior solves?
>>
>> ~Aaron
>
>
> No, this problem is not solved by the preprocessor. Yes, most C++ code bases today use the preprocessor to hack around this, but IMO, this is a broken solution. C++17 states that unknown attributes should be ignored, not warned on (although I believe the compiler is within its rights to warn). I strongly feel that users should be able to use attributes without macros, otherwise C++11 attributes lose a lot of their appeal, almost to the point where vendor-specific attribute syntaxes should have continued as the way forward.

I disagree that users should avoid the preprocessor here. If their
code is only compiled by Clang, there is not a lot of compelling
reason for them to pass -Wunknown-attributes=whatever in the first
place. If the code is compiled by more than just Clang, then the only
nonfragile cross-compiler solutions are to use the preprocessor or the
build system. In my experience, developers prefer writing code to
writing build scripts.

> In short, I would argue that the preprocessor is a worse solution, as requiring it blocks the perfectly valid syntax of using a C++11 attribute, and using it requires a lot more work to set up the macro.

I'm not certain why you believe using the preprocessor blocks using a
C++11 attribute -- it doesn't. There's no difference between using
[[clang::foobar]] and a macro that expands to [[clang::foobar]] if the
attribute is supported and [[]] if it's not, except for the lack of a
warning about ignored attributes.

I am not convinced that hacking build scripts is less work than using
macros. It's arguable that the first time you need the macro is an
outsized amount of work (because you have to start adding boilerplate
at that point), but I believe the same is true the first time you need
to hack a build script to add the new warning flag + attribute list.
Given that (AFAIK) no other compiler gives you granularity to disable
unknown attribute warnings on a per-attribute basis, I don't think
forcing users to try to deal with this in build scripts does users a
service. This is especially true given that each compiler will require
a different build script change even if the flags are identical
between compilers, because different compilers support different lists
of attributes.

~Aaron

>
> Regards,
> Justin
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev