Re: [PATCH] Add gnu::diagnose_as attribute

[Matthias Kretz <m.kretz@gsi.de>]

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.

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.)


> > 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? 
What entity could I use for hashing? The identifier alone wouldn't suffice 
since different instantiations of the same class template can have different 
diagnose_as values (e.g. std::string, std::wstring, ...).

-- 
──────────────────────────────────────────────────────────────────────────
 Dr. Matthias Kretz                           https://mattkretz.github.io
 GSI Helmholtz Centre for Heavy Ion Research               https://gsi.de
 std::experimental::simd              https://github.com/VcDevel/std-simd
──────────────────────────────────────────────────────────────────────────