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 (when your source says ``` namespace stdx = std::experimental; stdx::simd ``` 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::X3` is diagnosed as "[int|int]::X.3", and `typeid(X0::X3).name()` is "N2X0IiiE2X3E" which is "X0::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 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; _Alloc = std::allocator]' into 'template 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 struct X {}; using [[gnu::diagnose_as("XX")]] XX = X; // OK template struct X; using [[gnu::diagnose_as("XY")]] XY = X; // 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 "%). OK. However the msgid was copied from handle_abi_tag_attribute above. New patch (and ChangeLog) below: From: Matthias Kretz 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 ──────────────────────────────────────────────────────────────────────────