aaron.ballman added a comment.

In D157572#4604595 <https://reviews.llvm.org/D157572#4604595>, @philnik wrote:

> In D157572#4604482 <https://reviews.llvm.org/D157572#4604482>, @aaron.ballman 
> wrote:
>
>>> This allows standard libraries to mark symbols as extensions, so the 
>>> compiler can generate extension warnings when they are used.
>>
>> Huh, so this is basically the opposite of the `__extension__` macro (which 
>> is used to silence extension warnings)?
>
> I guess, kind-of. I never really understood the semantics of `__extension__` 
> though, so I'm not 100% certain.

It's used in system headers to say "I'm using an extension here, don't warn 
about it in -pedantic mode".

>> I don't think we need to introduce a new attribute to do this, we already 
>> have `diagnose_if`. e.g., https://godbolt.org/z/a5ae4T56o would that suffice?
>
> Part of the idea here is that the diagnostics should be part of 
> `-Wc++ab-extension`.

Hmmm, okay. And I'm assuming `-Wsystem-headers -pedantic` is too chatty because 
it's telling the user about all use of extensions, not extensions being 
introduced by the library itself? (e.g., https://godbolt.org/z/Gs3YGheMM is 
also not what you're after)

> I guess we could allow warning flags instead of just `"warning"` and 
> `"error"` in `diagnose_if` that specifies which warning group the diagnostic 
> should be part of. Something like `__attribute__((__diagnose_if__(__cplusplus 
> >= 201703L, "basic_string_view is a C++17 extension", 
> "-Wc++17-extensions")))`. I'm not sure how one could implement that, but I 
> guess there is some mechanism to translate "-Wwhatever" to a warning group, 
> since you can push and pop warnings.  That would open people up to add a 
> diagnostic to pretty much any warning group. I don't know if that's a good 
> idea. I don't really see a problem with that other than people writing weird 
> code, but people do that all the time anyways. Maybe I'm missing something 
> really problematic though.

That's actually a pretty interesting idea; `diagnose_if` could be given another 
parameter to specify a diagnostic group to associate the diagnostic with. This 
would let you do some really handy things like:

  void func(int i) __attribute__((diagnose_if(i < 0, "passing a negative value 
to 'func' is deprecated", "warning", "-Wdeprecated")));

But if we went this route, would we want to expose other diagnostic-related 
knobs like "show in system header" and "default to an error"? Also, the 
attribute currently can only be associated with a function; we can use this for 
classes by sticking it on a constructor but there's not much help for putting 
it on say a namespace or an enumeration. So we may need to extend the attribute 
in other ways. CC @cjdb as this seems of interest to you as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157572/new/

https://reviews.llvm.org/D157572

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to