[PATCH] Add gnu::diagnose_as attribute
Matthias Kretz
m.kretz@gsi.de
Wed May 26 21:33:15 GMT 2021
On Friday, 14 May 2021 18:05:03 CEST Martin Sebor wrote:
> [...]
>
> At the same time, my concern with adding another syntactic renaming
> mechanism that's specifically intended to change symbol names in
> diagnostics (and the two macros) but nowhere else is that it would
> considerably raise the risk of confusion between the name in
> the source and the name in the diagnostic. (E.g., one source
> file renames symbol Foo to Bar but another one doesn't; messages
> from one file will refer to Foo and other to Bar with no way of
> knowing they're the same. This could be solved by printing both
> the renamed symbol and what it stands for (like for typedefs or
> template instantiations) but that would then increase the S/R
> ratio this solution is aimed at improving. Providing an option
> to disable the renaming is great but suffers from the same problem:
> to be sure what symbol is being referred to, users would have to
> disable the renaming.
I agree with your concern. This attribute is easy to use for obfuscation and
slightly harder to use in a significantly helpful way. If you've ever had to
parse diagnostic output involving
std::experimental::parallelism_v2::simd<float,
std::experimental::parallelism_v2::simd_abi::_Scalar> (when your source says
```
namespace stdx = std::experimental;
stdx::simd<float, stdx::simd_abi::scalar>
```
you know the potential of this attribute.
> It doesn't seem like we can have it both ways. But maybe indicating
> in some way when a symbol mentioned in a diagnostic is the result of
> renaming by the attribute, without spelling out the original name,
> would be a good enough compromise. Though that won't help with
> the same problem in the expansion of macros like __FUNCTION__.
I've been thinking about it. It would not be helpful to always display the
original names when diagnose_as overrides something. But maybe there could be
like a glossary at the bottom of the diagnostic output? Or simply a "note:
Some names above do not reflect their real names in the source. Use -fno-
diagnostics-use-aliases to disable the replacement."
> Other than that, I have a couple of questions:
>
> Are there any implementations that provide this attribute? (If
> so does the syntax and effects match? It would be nice to be
> compatible if possible.)
There exists nothing like it AFAIK.
> Does the attribute change the typeinfo strings? If not, did you
> consider the interplay between compile-time and runtime diagnostics
> involving names decorated with the attribute?
Good question. From all my tests (and my understanding how typeinfo strings
are construted) the attribute has no effect on it. From one of my tests:
`X0<int, int>::X3` is diagnosed as "[int|int]::X.3", and `typeid(X0<int,
int>::X3).name()` is "N2X0IiiE2X3E" which is "X0<int, int>::X3" again. I
experienced the difference between compile-time and runtime diagnostics myself
(i.e. objdump and gdb disagree with diagnostic output) when working on
stdx::simd (I've been using the attribute since March with stdx::simd).
> (A related concern
> is the runtime output of messages involving macros like
> __FUNCTION__ issued from object files compiled by different
> compilers or versions of the same compiler.)
Right, if you make __FUNCTION__ part of your ABI or communication protocol
then the attribute can create incompatibilities. I'm not sure we should care
about this part too much. Note that -fpretty-templates also changes the
__PRETTY_FUNCTION__ string.
While I plan to submit a patch for std::__cxx11::basic_string to e.g. turn
'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>]'
into
'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)'
, i.e. affecting close to all C++ users, I don't believe the accumulated
headache will increase. ;-)
> > [...]
>
> Other diagnostics involving attributes do not start with an article.
> Can you please drop the "the"? (In general, I would suggest to either
> reuse or follow the style of existing diagnostics issued from the same
> file, and/or look at those in gcc/po/gcc.pot. Not nearly all of then
> are consistent with one another but it's best to avoid introducing
> yet another style; it will make converging on a single style easier,
> and reduces the effort involved in translating messages).
> [...]
Will do. Thanks for your detailed comments on this topic. Very helpful 👍.
> > + continue;
> > + }
> > + if (TREE_CODE (TREE_VALUE (args)) != STRING_CST)
> > + {
> > + error ("the argument to the %qE attribute must be a string "
> > + "literal", name);
>
> Similarly here, recommend to follow one of the existing styles (see
> c-family/c-attribs.c) rather than adding another variation to the mix.
The visibility attribute on namespaces says: "%qD attribute requires a single
NTBS argument". So I copied that (and its logic) for now. However, I believe
the use of "NTBS" is not very user friendly.
> > + if (CLASS_TYPE_P (type) && CLASSTYPE_IMPLICIT_INSTANTIATION (type))
> > + {
> > + if (COMPLETE_OR_OPEN_TYPE_P (type))
> > + warning (OPT_Wattributes, "%qE attribute cannot be applied to %qT
"
> > + "after its instantiation", name, type);
>
> Ditto here:
> msgid "ignoring %qE attribute applied to template instantiation %qT"
Ah, here I want to be more precise. Because the attribute can be applied to a
template instantiation. But only before its instantiation. Example:
template<class T> struct X {};
using [[gnu::diagnose_as("XX")]] XX = X<int>; // OK
template struct X<char>;
using [[gnu::diagnose_as("XY")]] XY = X<char>; // not OK
msgid "ignoring %qE attribute applied to template %qT after instantiation"
OK?
> > + error ("%qE attribute applied to extern \"C\" declaration %qD",
>
> Please quote extern "C" (as "%<extern \"C\"%>).
OK. However the msgid was copied from handle_abi_tag_attribute above.
New patch (and ChangeLog) below:
From: Matthias Kretz <kretz@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.
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 in the C++ frontend.
gcc/ChangeLog:
PR c++/89370
* doc/extend.texi: Document the diagnose_as attribute.
* doc/invoke.texi: Document -fno-diagnostics-use-aliases.
gcc/c-family/ChangeLog:
PR c++/89370
* c.opt (fdiagnostics-use-aliases): New diagnostics flag.
gcc/cp/ChangeLog:
PR c++/89370
* cp-tree.h: Add TFF_AS_PRIMARY.
* error.c (dump_scope): When printing the name of a namespace,
look for the diagnose_as attribute. If found, print the
associated string instead of calling dump_decl.
(dump_decl_name_or_diagnose_as): New function to replace
dump_decl (pp, DECL_NAME(t), flags) and inspect the tree for the
diagnose_as attribute before printing the DECL_NAME.
(dump_template_scope): New function. Prints the scope of a
template instance correctly applying diagnose_as attributes and
adjusting the list of template parms accordingly.
(dump_aggr_type): If the type has a diagnose_as attribute, print
the associated string instead of printing the original type
name. Print template parms only if the attribute was not applied
to the instantiation / full specialization.
(dump_simple_decl): Call dump_decl_name_or_diagnose_as instead
of dump_decl.
(dump_decl): Ditto.
(lang_decl_name): Ditto.
(dump_function_decl): Walk the functions context list to
determine whether a call to dump_template_scope is required.
Ensure function templates are presented as primary templates.
(dump_function_name): Replace the function's identifier with the
diagnose_as attribute value, if set.
(dump_template_parms): Treat as primary template if flags
contains TFF_AS_PRIMARY.
(comparable_template_types_p): Consider the types not a template
if one carries a diagnose_as attribute.
(print_template_differences): Replace the identifier with the
diagnose_as attribute value on the most general template, if it
is set.
* name-lookup.c (handle_namespace_attrs): Handle the diagnose_as
attribute. Ensure exactly one string argument. Ensure previous
diagnose_as attributes used the same name.
* tree.c (cxx_attribute_table): Add diagnose_as attribute to the
table.
(check_diagnose_as_redeclaration): New function; copied and
adjusted from check_abi_tag_redeclaration.
(handle_diagnose_as_attribute): New function; copied and
adjusted from handle_abi_tag_attribute. If the given *node is a
TYPE_DECL and the TREE_TYPE is an implicit class template
instantiation, call decl_attributes to add the diagnose_as
attribute to the TREE_TYPE.
gcc/testsuite/ChangeLog:
PR c++/89370
* g++.dg/diagnostic/diagnose-as1.C: New test.
* g++.dg/diagnostic/diagnose-as2.C: New test.
* g++.dg/diagnostic/diagnose-as3.C: New test.
* g++.dg/diagnostic/diagnose-as4.C: New test.
---
gcc/c-family/c.opt | 4 +
gcc/cp/cp-tree.h | 5 +-
gcc/cp/error.c | 235 +++++++++++++++++-
gcc/cp/name-lookup.c | 25 ++
gcc/cp/tree.c | 116 +++++++++
gcc/doc/extend.texi | 40 +++
gcc/doc/invoke.texi | 9 +-
.../g++.dg/diagnostic/diagnose-as1.C | 177 +++++++++++++
.../g++.dg/diagnostic/diagnose-as2.C | 144 +++++++++++
.../g++.dg/diagnostic/diagnose-as3.C | 152 +++++++++++
.../g++.dg/diagnostic/diagnose-as4.C | 158 ++++++++++++
11 files changed, 1052 insertions(+), 13 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/diagnostic/diagnose-as1.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/diagnose-as2.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/diagnose-as3.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/diagnose-as4.C
--
──────────────────────────────────────────────────────────────────────────
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
──────────────────────────────────────────────────────────────────────────
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Add-gnu-diagnose_as-attribute.patch
Type: text/x-patch
Size: 49794 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20210526/6dd41f38/attachment-0001.bin>
More information about the Libstdc++
mailing list