Re: [PATCH] Add gnu::diagnose_as attribute

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

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.

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

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

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.      

-- 
──────────────────────────────────────────────────────────────────────────
 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
──────────────────────────────────────────────────────────────────────────