On 5/27/21 2:54 PM, Matthias Kretz wrote:
On Thursday, 27 May 2021 19:39:48 CEST Jason Merrill wrote:
On 5/4/21 7:13 AM, Matthias Kretz wrote:
From: Matthias Kretz <kr...@kde.org>
This attribute overrides the diagnostics output string for the entity it
appertains to. The motivation is to improve QoI for library TS
implementations, where diagnostics have a very bad signal-to-noise ratio
due to the long namespaces involved.
On Tuesday, 27 April 2021 11:46:48 CEST Jonathan Wakely wrote:
I think it's a great idea and would like to use it for all the TS
implementations where there is some inline namespace that the user
doesn't care about. std::experimental::fundamentals_v1:: would be much
better as just std::experimental::, or something like std::[LFTS]::.
Hmm, how much of the benefit could we get from a flag (probably on by
default) to skip inline namespaces in diagnostics?
I'd say about 20% for the TS's. Even std::experimental::simd (i.e. without the
'::parallelism_v2' part) is still rather noisy. I want stdₓ::simd, std-x::simd
or std::[PTS2]::simd or whatever shortest shorthand Jonathan will allow. ;)
For PR89370, the benefit would be ~2%:
'template<class _Tp> std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::_If_sv<_Tp, std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&>
std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::insert(std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::size_type, const _Tp&, std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::size_type, std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::size_type) [with _Tp = _Tp; _CharT = char; _Traits =
std::char_traits<char>; _Alloc = std::allocator<char>]'
would then only turn into:
'template<class _Tp> std::basic_string<_CharT, _Traits,
_Alloc>::_If_sv<_Tp, std::basic_string<_CharT, _Traits, _Alloc>&>
std::basic_string<_CharT, _Traits,
_Alloc>::insert(std::basic_string<_CharT, _Traits,
_Alloc>::size_type, const _Tp&, std::basic_string<_CharT, _Traits,
_Alloc>::size_type, std::basic_string<_CharT, _Traits,
_Alloc>::size_type) [with _Tp = _Tp; _CharT = char; _Traits =
std::char_traits<char>; _Alloc = std::allocator<char>]'
instead of:
'template<class _Tp> std::string::_If_sv<_Tp, std::string&>
std::string::insert<_Tp>(std::string::size_type, const _Tp&,
std::string::size_type, std::string::size_type)'
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.
With the attribute, it is possible to solve PR89370 and make
std::__cxx11::basic_string<_CharT, _Traits, _Alloc> appear as
std::string in diagnostic output without extra hacks to recognize the
type.
That sounds wrong to me; std::string is the <char> instantiation, not
the template. Your patch doesn't make it possible to apply this
attribute to class template instantiations, does it?
Yes, it does.
Initially, when I tried to improve the TS experience, it didn't. When Jonathan
showed PR89370 to me I tried to make [[gnu::diagnose_as]] more generic &
useful. Since there's no obvious syntax to apply an attribute to a template
instantiation, I had to be creative. 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. And it seems like applying the attribute to a
namespace means that enclosing namespaces are not printed, unlike the
handling for types.
}
namespace __gnu_cxx
{
--- a/libstdc++-v3/include/bits/stringfwd.h
+++ b/libstdc++-v3/include/bits/stringfwd.h
@@ -76,24 +76,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
_GLIBCXX_END_NAMESPACE_CXX11
/// A string of @c char
- 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.
#ifdef _GLIBCXX_USE_WCHAR_T
/// A string of @c wchar_t
- typedef basic_string<wchar_t> wstring;
+ typedef basic_string<wchar_t> wstring
[[__gnu__::__diagnose_as__("wstring")]];
#endif
[...]
The part of my frontend patch that makes this work is in
handle_diagnose_as_attribute:
+ if (TREE_CODE (*node) == TYPE_DECL)
+ {
+ // Apply the attribute to the type alias itself.
+ decl = *node;
+ tree type = TREE_TYPE (*node);
+ if (CLASS_TYPE_P (type) && CLASSTYPE_TEMPLATE_INSTANTIATION (type))
+ {
+ if (COMPLETE_OR_OPEN_TYPE_P (type))
+ warning (OPT_Wattributes,
+ "ignoring %qE attribute applied to template %qT after "
+ "instantiation", name, type);
+ else
+ {
+ type = strip_typedefs(type, nullptr, 0);
+ // And apply the attribute to the specialization on the RHS.
+ tree attributes = tree_cons (name, args, NULL_TREE);
+ decl_attributes (&type, attributes,
+ ATTR_FLAG_TYPE_IN_PLACE, NULL_TREE);
+ }
+ }
+ }
Ah, clever.
Jason
