[PATCH] Add gnu::diagnose_as attribute

[Jason Merrill]

On 5/27/21 6:07 PM, Matthias Kretz wrote:
> On Thursday, 27 May 2021 23:15:46 CEST Jason Merrill wrote:
>> On 5/27/21 2:54 PM, Matthias Kretz wrote:
>>> Also hiding all inline namespace by default might make some error messages
>>> harder to understand:
>>>
>>> namespace Vir {
>>>     inline namespace foo {
>>>       struct A {};
>>>     }
>>>     struct A {};
>>> }
>>> using Vir::A;
>>>
>>> <source>:7:12: error: reference to 'A' is ambiguous
>>> <source>:3:12: note: candidates are: 'struct Vir::A'
>>> <source>:5:10: note:                 'struct Vir::A'
>>
>> That doesn't seem so bad.
> 
> As long as you ignore the line numbers, it's a puzzling diagnostic.

Only briefly puzzling, I think; Vir::A is a valid way of referring to 
both types.

>>> This is from my pending std::string patch:
>>>
>>> --- a/libstdc++-v3/include/bits/c++config
>>> +++ b/libstdc++-v3/include/bits/c++config
>>> @@ -299,7 +299,8 @@ namespace std
>>>
>>>    #if _GLIBCXX_USE_CXX11_ABI
>>>    namespace std
>>>    {
>>>
>>> -  inline namespace __cxx11 __attribute__((__abi_tag__ ("cxx11"))) { }
>>> +  inline namespace __cxx11
>>> +    __attribute__((__abi_tag__ ("cxx11"), __diagnose_as__("std"))) { }
>>
>> This seems to have the same benefits and drawbacks of my inline
>> namespace suggestion.
> 
> True for std::string, not true for TS's where the extra '::experimental' still
> makes finding the relevant information in diagnostics harder than necessary.
> 
>> And it seems like applying the attribute to a
>> namespace means that enclosing namespaces are not printed, unlike the
>> handling for types.
> 
> Yes, that's also how I documented it. For nested namespaces I wanted to enable
> the removal of nesting (e.g. from std::experimental::parallelism_v2::simd to
> stdx::simd). However, for types and functions it would be a problem to drop
> the enclosing scope, because the scope can be class templates and thus the
> diagnose_as attribute would remove all template parms & args.

I'd think you could get the same effect from a hypothetical

namespace [[gnu::diagnose_as]] stdx = std::experimental;

though we'll need to add support for attributes on namespace aliases to 
the grammar.

>>> -  typedef basic_string<char>    string;
>>> +  typedef basic_string<char> string
>>> [[__gnu__::__diagnose_as__("string")]];
>>
>> Here it seems like you want to say "use this typedef as the true name of
>> the type".  Is it useful to have to repeat the name?  Allowing people to
>> use names that don't correspond to actual declarations seems unnecessary.
> 
> Yes, but you could also use it to apply diagnose_as to a template
> instantiation without introducing a name for users. E.g.
> 
>    using __only_to_apply_the_attribute [[gnu::diagnose_as("intvector")]]
>      = std::vector<int>;
> 
> Now all diagnostics of 'std::vector<int>' would print 'intvector' instead.

Yes, but why would you want to?  Making diagnostics print names that the 
user can't use in their own code seems obfuscatory, and requiring users 
to write the same names in two places seems like extra work.

> But in general, I tend to agree, for type aliases there's rarely a case where the
> names wouldn't match.
> 
> However, I didn't want to special-case the attribute parameters for type
> aliases (or introduce another attribute just for this case). The attribute
> works consistently and with the same interface independent of where it's used.
> I tried to build a generic, broad feature instead of a narrow one-problem
> solution.

"Treat this declaration as the name of the type/namespace it refers to 
in diagnostics" also seems consistent to me.

> FWIW, before you suggest to have one attribute for namespaces and one for type
> aliases (to cover the std::string case), I have another use case in stdx::simd
> (the spec requires simd_abi::scalar to be an alias):
> 
>    namespace std::experimental::parallelism_v2::simd_abi {
>      struct [[gnu::diagnose_as("scalar")]] _Scalar;
>      using scalar = _Scalar;
>    }
> 
> If the attribute were on the type alias (using scalar [[gnu::diagnose_as]] =
> _Scalar;), then we'd have to apply the attribute to _Scalar after it was
> completed. That seemed like a bad idea to me.

Well, in your sample code, _Scalar isn't complete at the point of the 
alias-declaration.  But even if it were, since the attribute doesn't 
affect code generation, it doesn't strike me as a big problem.  Still, 
perhaps it would be better to store these aliases in a separate hash 
table instead of *_ATTRIBUTES.

Jason