Re: [PATCH] Add gnu::diagnose_as attribute

[Jason Merrill <jason@redhat.com>]

On 5/28/21 3:42 AM, Matthias Kretz wrote:
> On Friday, 28 May 2021 05:05:52 CEST Jason Merrill wrote:
>> 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:
>>>>> 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.
> 
> True. But that's also what lead to the error. GCC easily clears it up
> nowadays, but wouldn't anymore if inline namespaces were hidden by default.
> 
>> 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.
> 
> Right, but then two of my design goals can't be met:
> 
> 1. Diagnostics have an improved signal-to-noise ratio out of the box.
> 
> 2. We can use replacement names that are not valid identifiers.

This is the basic disconnect: I think that these goals are 
contradictory, and that replacement names that are not valid identifiers 
will just confuse users that don't know about them.

If a user sees stdx::foo in a diagnostic and then tries to refer to 
stdx::foo and gets an error, the diagnostic is not more helpful than one 
that uses the fully qualified name.

Jonathan, David, any thoughts on this issue?

> I don't think libstdc++ would ship with a namespace alias outside of the std
> namespace. So we'd place the "burden" of using diagnose_as correctly on our
> users. Also as a user you'd possibly have to repeat the namespace alias in
> every source file and/or place it in your applications/libraries namespace.
> 
>>>> 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.
> 
> I can imagine using it to make _Internal __names more readable while at the
> same time discouraging users to utter them in their own code. Sorry for the
> bad code obfuscation example above.
> 
> An example for consideration from stdx::simd:
> 
>    namespace std {
>    namespace experimental {
>    namespace parallelism_v2 [[gnu::diagnose_as("stdx")]] {
>    namespace simd_abi [[gnu::diagnose_as("simd_abi")]] {
>      template <int _Bytes>
>        struct _VecBuiltin;
> 
>      template <int _Bytes>
>        struct _VecBltnBtmsk;
> 
>    #if x86
>      using __ignore_me_0 [[gnu::diagnose_as("[SSE]")]] = _VecBuiltin<16>;
>      using __ignore_me_1 [[gnu::diagnose_as("[AVX]")]] = _VecBuiltin<32>;
>      using __ignore_me_2 [[gnu::diagnose_as("[AVX512]")]] = _VecBltnBtmsk<64>;
>    #endif
>    }}}}
> 
> Then diagnostics would print 'stdx::simd<float, simd_abi::[AVX512]>' instead
> of 'stdx::simd<float, simd_abi::_VecBltnBtmsk<64>>'. (Users utter the type by
> saying e.g. 'stdx::native_simd<float>', while compiling with AVX512 flags.)

Wouldn't it be better to print stdx::native_simd<float> if that's how 
the users write the type?

>>> 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.
> 
> Sure. In general, I think
> 
>    namespace foo [[gnu::this_is_the_name_I_want]] = bar;
>    using foo [[gnu::this_is_the_name_I_want]] = bar;
> 
> is not a terribly bad idea on its own. But it's not the solution for the
> problems I set out to solve.
> 
>> Still, perhaps it would be better to store these aliases in a separate hash
>> table instead of *_ATTRIBUTES.
> 
> Maybe. For performance reasons or for simplification of the implementation?

I was thinking for not messing with the type after it's defined, but 
there are other things that do that (such as lazy declaration of 
implicit constructors) so I wouldn't worry about it.

Jason