* [PATCH] Add gnu::diagnose_as attribute [not found] ` <20210427094648.GL3008@redhat.com> @ 2021-05-04 11:13 ` Matthias Kretz 2021-05-04 13:34 ` David Malcolm ` (2 more replies) 0 siblings, 3 replies; 30+ messages in thread From: Matthias Kretz @ 2021-05-04 11:13 UTC (permalink / raw) To: gcc-patches; +Cc: libstdc++ [-- Attachment #1: Type: text/plain, Size: 3647 bytes --] 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. 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]::. 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. 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 * 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_aggr_type): If the type has a diagnose_as attribute, print the associated string instead of printing the original type name. (dump_simple_decl): Call dump_decl_name_or_diagnose_as instead of dump_decl. (dump_decl): Ditto. (lang_decl_name): Ditto. (dump_function_decl): Ensure complete replacement of the class template diagnostics if a diagnose_as attribute is present. (dump_function_name): Replace the function diagnostic output if the diagnose_as attribute 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/c-family/c.opt | 4 ++ gcc/cp/error.c | 85 ++++++++++++++++++++++++++++--- gcc/cp/name-lookup.c | 27 ++++++++++ gcc/cp/tree.c | 117 +++++++++++++++++++++++++++++++++++++++++++ gcc/doc/extend.texi | 37 ++++++++++++++ gcc/doc/invoke.texi | 9 +++- 6 files changed, 270 insertions(+), 9 deletions(-) -- ────────────────────────────────────────────────────────────────────────── 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 ────────────────────────────────────────────────────────────────────────── [-- Attachment #2: 0001-Add-gnu-diagnose_as-attribute.patch --] [-- Type: text/x-patch, Size: 16239 bytes --] diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 3f8b72cdc00..0cf01c6dba4 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -1582,6 +1582,10 @@ fdiagnostics-show-template-tree C++ ObjC++ Var(flag_diagnostics_show_template_tree) Init(0) Print hierarchical comparisons when template types are mismatched. +fdiagnostics-use-aliases +C++ Var(flag_diagnostics_use_aliases) Init(1) +Replace identifiers or scope names in diagnostics as defined by the diagnose_as attribute. + fdirectives-only C ObjC C++ ObjC++ Preprocess directives only. diff --git a/gcc/cp/error.c b/gcc/cp/error.c index c88d1749a0f..10b547afaa7 100644 --- a/gcc/cp/error.c +++ b/gcc/cp/error.c @@ -35,6 +35,7 @@ along with GCC; see the file COPYING3. If not see #include "internal-fn.h" #include "gcc-rich-location.h" #include "cp-name-hint.h" +#include "attribs.h" #define pp_separate_with_comma(PP) pp_cxx_separate_with (PP, ',') #define pp_separate_with_semicolon(PP) pp_cxx_separate_with (PP, ';') @@ -66,6 +67,7 @@ static void dump_alias_template_specialization (cxx_pretty_printer *, tree, int) static void dump_type (cxx_pretty_printer *, tree, int); static void dump_typename (cxx_pretty_printer *, tree, int); static void dump_simple_decl (cxx_pretty_printer *, tree, tree, int); +static void dump_decl_name_or_diagnose_as (cxx_pretty_printer *, tree, int); static void dump_decl (cxx_pretty_printer *, tree, int); static void dump_template_decl (cxx_pretty_printer *, tree, int); static void dump_function_decl (cxx_pretty_printer *, tree, int); @@ -231,7 +233,15 @@ dump_scope (cxx_pretty_printer *pp, tree scope, int flags) { if (scope != global_namespace) { - dump_decl (pp, scope, f); + tree diagnose_as + = flag_diagnostics_use_aliases + ? lookup_attribute ("diagnose_as", DECL_ATTRIBUTES (scope)) + : NULL_TREE; + if (diagnose_as) + pp_cxx_ws_string ( + pp, TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (diagnose_as)))); + else + dump_decl (pp, scope, f); pp_cxx_colon_colon (pp); } } @@ -743,6 +753,7 @@ dump_aggr_type (cxx_pretty_printer *pp, tree t, int flags) tree decl = TYPE_NAME (t); + tree diagnose_as = NULL_TREE; if (decl) { typdef = (!DECL_ARTIFICIAL (decl) @@ -769,8 +780,15 @@ dump_aggr_type (cxx_pretty_printer *pp, tree t, int flags) if (! (flags & TFF_UNQUALIFIED_NAME)) dump_scope (pp, CP_DECL_CONTEXT (decl), flags | TFF_SCOPE); flags &= ~TFF_UNQUALIFIED_NAME; + + tree diagnose_as_specialized = NULL_TREE; if (tmplate) { + if (flag_diagnostics_use_aliases) + diagnose_as_specialized + = lookup_attribute ("diagnose_as", + TYPE_ATTRIBUTES (TREE_TYPE (decl))); + /* Because the template names are mangled, we have to locate the most general template, and use that name. */ tree tpl = TYPE_TI_TEMPLATE (t); @@ -779,9 +797,25 @@ dump_aggr_type (cxx_pretty_printer *pp, tree t, int flags) tpl = DECL_TI_TEMPLATE (tpl); decl = tpl; } + + if (flag_diagnostics_use_aliases) + diagnose_as = lookup_attribute ("diagnose_as", + TYPE_ATTRIBUTES (TREE_TYPE (decl))); + if (diagnose_as_specialized + && (!diagnose_as || TREE_VALUE(diagnose_as_specialized) + != TREE_VALUE(diagnose_as))) + { + pp_cxx_ws_string ( + pp, TREE_STRING_POINTER ( + TREE_VALUE (TREE_VALUE (diagnose_as_specialized)))); + return; + } } - if (LAMBDA_TYPE_P (t)) + if (diagnose_as) + pp_cxx_ws_string (pp, TREE_STRING_POINTER ( + TREE_VALUE (TREE_VALUE (diagnose_as)))); + else if (LAMBDA_TYPE_P (t)) { /* A lambda's "type" is essentially its signature. */ pp_string (pp, M_("<lambda")); @@ -1103,7 +1137,7 @@ dump_simple_decl (cxx_pretty_printer *pp, tree t, tree type, int flags) pp_string (pp, " capture>"); } else - dump_decl (pp, DECL_NAME (t), flags); + dump_decl_name_or_diagnose_as (pp, t, flags); } else if (DECL_DECOMPOSITION_P (t)) pp_string (pp, M_("<structured bindings>")); @@ -1147,6 +1181,25 @@ dump_decl_name (cxx_pretty_printer *pp, tree t, int flags) pp_cxx_tree_identifier (pp, t); } +/* Print the DECL_NAME of DECL unless a different string was requested by the + diagnose_as attribute. */ + +static void +dump_decl_name_or_diagnose_as (cxx_pretty_printer *pp, tree decl, int flags) +{ + if (flag_diagnostics_use_aliases) + { + tree attr = lookup_attribute ("diagnose_as", DECL_ATTRIBUTES (decl)); + if (attr) + { + pp_cxx_ws_string ( + pp, TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr)))); + return; + } + } + dump_decl_name (pp, DECL_NAME (decl), flags); +} + /* Dump a human readable string for the decl T under control of FLAGS. */ static void @@ -1192,7 +1245,7 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags) || flags & TFF_CLASS_KEY_OR_ENUM)) { pp_cxx_ws_string (pp, "using"); - dump_decl (pp, DECL_NAME (t), flags); + dump_decl_name_or_diagnose_as(pp, t, flags); pp_cxx_whitespace (pp); pp_cxx_ws_string (pp, "="); pp_cxx_whitespace (pp); @@ -1374,7 +1427,7 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags) TREE_CODE (DECL_INITIAL (t)) == TEMPLATE_PARM_INDEX)) dump_simple_decl (pp, t, TREE_TYPE (t), flags); else if (DECL_NAME (t)) - dump_decl (pp, DECL_NAME (t), flags); + dump_decl_name_or_diagnose_as (pp, t, flags); else if (DECL_INITIAL (t)) dump_expr (pp, DECL_INITIAL (t), flags | TFF_EXPR_IN_PARENS); else @@ -1393,7 +1446,7 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags) } dump_type (pp, scope, flags); pp_cxx_colon_colon (pp); - dump_decl (pp, DECL_NAME (t), flags); + dump_decl_name_or_diagnose_as (pp, t, flags); if (variadic) pp_cxx_ws_string (pp, "..."); } @@ -1657,7 +1710,13 @@ dump_function_decl (cxx_pretty_printer *pp, tree t, int flags) /* Pretty print template instantiations only. */ if (DECL_USE_TEMPLATE (t) && DECL_TEMPLATE_INFO (t) && !(flags & TFF_NO_TEMPLATE_BINDINGS) - && flag_pretty_templates) + && flag_pretty_templates + // skip the output of template parameters if the function is a member of a + // class with diagnose_as attribute: + && !(flag_diagnostics_use_aliases + && DECL_CLASS_SCOPE_P (t) + && lookup_attribute ("diagnose_as", + TYPE_ATTRIBUTES (DECL_CONTEXT (t))))) { tree tmpl; @@ -1885,6 +1944,16 @@ dump_exception_spec (cxx_pretty_printer *pp, tree t, int flags) static void dump_function_name (cxx_pretty_printer *pp, tree t, int flags) { + if (flag_diagnostics_use_aliases) + { + tree attr = lookup_attribute ("diagnose_as", DECL_ATTRIBUTES (t)); + if (attr) + { + pp_cxx_ws_string ( + pp, TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr)))); + return; + } + } tree name = DECL_NAME (t); /* We can get here with a decl that was synthesized by language- @@ -3133,7 +3202,7 @@ lang_decl_name (tree decl, int v, bool translate) && TREE_CODE (decl) == NAMESPACE_DECL) dump_decl (cxx_pp, decl, TFF_PLAIN_IDENTIFIER | TFF_UNQUALIFIED_NAME); else - dump_decl (cxx_pp, DECL_NAME (decl), TFF_PLAIN_IDENTIFIER); + dump_decl_name_or_diagnose_as (cxx_pp, decl, TFF_PLAIN_IDENTIFIER); return pp_ggc_formatted_text (cxx_pp); } diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index 4e84e2f9987..80637503310 100644 --- a/gcc/cp/name-lookup.c +++ b/gcc/cp/name-lookup.c @@ -6082,6 +6082,33 @@ handle_namespace_attrs (tree ns, tree attributes) DECL_ATTRIBUTES (ns) = tree_cons (name, args, DECL_ATTRIBUTES (ns)); } + else if (is_attribute_p ("diagnose_as", name)) + { + if (!args || TREE_CHAIN(args)) + { + error ("the %qE attribute requires exactly one argument", name); + continue; + } + if (TREE_CODE (TREE_VALUE (args)) != STRING_CST) + { + error ("the argument to the %qE attribute must be a string " + "literal", name); + continue; + } + tree existing + = lookup_attribute ("diagnose_as", DECL_ATTRIBUTES (ns)); + if (existing + && !cp_tree_equal(TREE_VALUE (args), + TREE_VALUE (TREE_VALUE (existing)))) + { + error ("the namespace %qE already uses a different diagnose_as " + "attribute value", ns); + inform (DECL_SOURCE_LOCATION (ns), "previous declaration here"); + continue; + } + DECL_ATTRIBUTES (ns) = tree_cons (name, args, + DECL_ATTRIBUTES (ns)); + } else { warning (OPT_Wattributes, "%qD attribute directive ignored", diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index a8bfd5fc053..f7b93dc89d7 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -46,6 +46,7 @@ static tree verify_stmt_tree_r (tree *, int *, void *); static tree handle_init_priority_attribute (tree *, tree, tree, int, bool *); static tree handle_abi_tag_attribute (tree *, tree, tree, int, bool *); +static tree handle_diagnose_as_attribute (tree *, tree, tree, int, bool *); /* If REF is an lvalue, returns the kind of lvalue that REF is. Otherwise, returns clk_none. */ @@ -4860,6 +4861,8 @@ const struct attribute_spec cxx_attribute_table[] = handle_init_priority_attribute, NULL }, { "abi_tag", 1, -1, false, false, false, true, handle_abi_tag_attribute, NULL }, + { "diagnose_as", 1, 1, false, false, false, false, + handle_diagnose_as_attribute, NULL }, { NULL, 0, 0, false, false, false, false, NULL, NULL } }; @@ -5128,6 +5131,120 @@ handle_abi_tag_attribute (tree* node, tree name, tree args, return NULL_TREE; } +static bool +check_diagnose_as_redeclaration (const_tree decl, const_tree old, + const_tree new_) +{ + if (!old) + return true; + if (TREE_CODE (TREE_VALUE (old)) == TREE_LIST) + old = TREE_VALUE (old); + if (TREE_CODE (TREE_VALUE (new_)) == TREE_LIST) + new_ = TREE_VALUE (new_); + tree old_value = TREE_VALUE (old); + tree new_value = TREE_VALUE (new_); + if (cp_tree_equal (old_value, new_value)) + return true; + error ("conflicting declaration of %<diagnose_as%> attribute on %qD would " + "overwrite %qE with %qE", decl, old_value, new_value); + return false; +} + +static tree +handle_diagnose_as_attribute (tree* node, tree name, tree args, + int flags, bool* no_add_attrs) +{ + tree decl = NULL_TREE; + if (!args || TREE_CHAIN (args)) + { + error ("the %qE attribute requires exactly one argument", name); + goto fail; + } + if (TREE_CODE (TREE_VALUE (args)) != STRING_CST) + { + error ("the argument to the %qE attribute must be a string literal", + name); + goto fail; + } + 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_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); + 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); + } + } + } + else if (TYPE_P (*node)) + { + if (!OVERLOAD_TYPE_P (*node)) + { + error ("%qE attribute applied to non-class, non-enum type %qT", + name, *node); + goto fail; + } + else if (!(flags & (int)ATTR_FLAG_TYPE_IN_PLACE)) + { + error ("%qE attribute applied to %qT after its definition", + name, *node); + goto fail; + } + decl = TYPE_NAME (*node); + } + else + { + if (!VAR_OR_FUNCTION_DECL_P (*node)) + { + error ("%qE attribute applied to non-function, non-variable %qD", + name, *node); + goto fail; + } + else if (DECL_LANGUAGE (*node) == lang_c) + { + error ("%qE attribute applied to extern \"C\" declaration %qD", + name, *node); + goto fail; + } + decl = *node; + } + + // Make sure all declarations have the same diagnose_as string. + if (DECL_SOURCE_LOCATION (decl) != input_location) + { + tree attributes = TYPE_P (*node) ? TYPE_ATTRIBUTES (*node) + : DECL_ATTRIBUTES (decl); + if (!check_diagnose_as_redeclaration ( + decl, lookup_attribute ("diagnose_as", attributes), args)) + goto fail; + } + else if (CLASS_TYPE_P (*node) && CLASSTYPE_IMPLICIT_INSTANTIATION (*node)) + { + // The above branch (different source location) is taken for declarations + // of type aliases that modify an implicit template specialization on the + // RHS. This branch is taken when the template is instantiated via + // instantiate_class_template_1, in which case the attribute either + // already has the value from the general template or from a type alias. + goto fail; + } + + return NULL_TREE; + + fail: + *no_add_attrs = true; + return NULL_TREE; +} + /* Return a new PTRMEM_CST of the indicated TYPE. The MEMBER is the thing pointed to by the constant. */ diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index c8caf36f293..7976e3a72d4 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -2865,6 +2865,43 @@ types (@pxref{Variable Attributes}, @pxref{Type Attributes}.) The message attached to the attribute is affected by the setting of the @option{-fmessage-length} option. +@item diagnose_as ("@var{string}") +@cindex @code{diagnose_as} function attribute +The @code{diagnose_as} attribute modifies how the entity the attribute +appertains to is diagnosed in compiler messages and __FUNCTION__ / +__PRETTY_FUNCTION__ macros. If the attribute is applied to a @code{namespace}, +the specified string replaces the complete enclosing scope. The effect of the +@code{diagnose_as} attribute can be disabled with the +@option{-fno-diagnostics-use-aliases} command line option. + +@smallexample +namespace Foo @{ + namespace Bar @{ + inline namespace v1 [[gnu::diagnose_as("Foobar")]] @{ + int f() @{ + // __PRETTY_FUNCTION__ == "void Foobar::f()" + @} + @} + @} +@} +// In function 'int Foobar::f()': +// warning: no return statement in function returning non-void [-Wreturn-type] +@end smallexample + +The @code{diagnose_as} attribute can be used with namespaces, functions, +variables, alias declarations (but not alias templates), and user-defined types +(classes, unions, and enums). If the alias declaration aliases an implicit class +template specialization, the attribute is additionally applied to the class +template specialization. + +@smallexample +template <class T> struct basic_string @{@}; +using string [[gnu::diagnose_as("string")]] = basic_string<char>; +int f(basic_string<char>) @{@} +// In function 'int f(string)': +// warning: no return statement in function returning non-void [-Wreturn-type] +@end smallexample + @item error ("@var{message}") @itemx warning ("@var{message}") @cindex @code{error} function attribute diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index f5a9dc33819..84c19344c89 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -311,7 +311,8 @@ Objective-C and Objective-C++ Dialects}. -fdiagnostics-show-path-depths @gol -fno-show-column @gol -fdiagnostics-column-unit=@r{[}display@r{|}byte@r{]} @gol --fdiagnostics-column-origin=@var{origin}} +-fdiagnostics-column-origin=@var{origin} @gol +-fno-diagnostics-aliases} @item Warning Options @xref{Warning Options,,Options to Request or Suppress Warnings}. @@ -5077,6 +5078,12 @@ first column. The default value of 1 corresponds to traditional GCC behavior and to the GNU style guide. Some utilities may perform better with an origin of 0; any non-negative value may be specified. +@item -fno-diagnostics-use-aliases +@opindex fno-diagnostics-use-aliases +@opindex fdiagnostics-use-aliases +Do not replace identifiers or scope names with the aliases defined with the +@code{[[gnu::diagnose_as("alias")]]} attribute. + @item -fdiagnostics-format=@var{FORMAT} @opindex fdiagnostics-format Select a different format for printing diagnostics. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-05-04 11:13 ` [PATCH] Add gnu::diagnose_as attribute Matthias Kretz @ 2021-05-04 13:34 ` David Malcolm 2021-05-04 14:23 ` Matthias Kretz 2021-05-14 16:05 ` Martin Sebor 2021-05-27 17:39 ` Jason Merrill 2 siblings, 1 reply; 30+ messages in thread From: David Malcolm @ 2021-05-04 13:34 UTC (permalink / raw) To: Matthias Kretz, gcc-patches; +Cc: libstdc++ On Tue, 2021-05-04 at 13:13 +0200, Matthias Kretz wrote: > 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. > > 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]::. > > 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. Thanks for the patch, it looks very promising. The C++ frontend maintainers will need to review the C++ frontend parts in detail, so I'll defer to them for the bulk of the review. Various thoughts: The patch has no testcases; it should probably add test coverage for: - the various places and ways in which diagnose_as can affect the output, - disabling it with the option - the various ways in which the user can get diagnose_as wrong - etc Does the patch affect the output of labels when underlining ranges of source code in diagnostics? Does the patch interact correctly with the %H and %I codes that try to show the differences between two template types? I have some minor nits from a diagnostics point of view: [...snip...] > diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c > index 4e84e2f9987..80637503310 100644 > --- a/gcc/cp/name-lookup.c > +++ b/gcc/cp/name-lookup.c [...] > + tree existing > + = lookup_attribute ("diagnose_as", DECL_ATTRIBUTES (ns)); > + if (existing > + && !cp_tree_equal(TREE_VALUE (args), > + TREE_VALUE (TREE_VALUE (existing)))) > + { Please add an auto_diagnostic_group here so that the "inform" is associated with the "error". > + error ("the namespace %qE already uses a different diagnose_as " > + "attribute value", ns); diagnose_as should be in quotes here (%< and %>). > + inform (DECL_SOURCE_LOCATION (ns), "previous declaration here"); > + continue; > + } > + DECL_ATTRIBUTES (ns) = tree_cons (name, args, > + DECL_ATTRIBUTES (ns)); > + } > else > { > warning (OPT_Wattributes, "%qD attribute directive ignored", > diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c > index a8bfd5fc053..f7b93dc89d7 100644 > --- a/gcc/cp/tree.c > +++ b/gcc/cp/tree.c [...snip...] > + else if (DECL_LANGUAGE (*node) == lang_c) > + { > + error ("%qE attribute applied to extern \"C\" declaration %qD", Please quote extern "C": %<extern \"C\"%> > + name, *node); [...snip...] Thanks again for the patch; hope this is constructive Dave ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-05-04 13:34 ` David Malcolm @ 2021-05-04 14:23 ` Matthias Kretz 2021-05-04 14:32 ` Matthias Kretz 2021-05-04 19:00 ` David Malcolm 0 siblings, 2 replies; 30+ messages in thread From: Matthias Kretz @ 2021-05-04 14:23 UTC (permalink / raw) To: gcc-patches, David Malcolm; +Cc: libstdc++ On Tuesday, 4 May 2021 15:34:13 CEST David Malcolm wrote: > On Tue, 2021-05-04 at 13:13 +0200, Matthias Kretz wrote: > > 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. > > [...] > > Thanks for the patch, it looks very promising. Thanks. I'm new to modifying the compiler like this, so please be extra careful with my patch. I believe I understand most of what I did, but I might have misunderstood. :) > The patch has no testcases; it should probably add test coverage for: > - the various places and ways in which diagnose_as can affect the > output, > - disabling it with the option > - the various ways in which the user can get diagnose_as wrong > - etc Right. If you know of an existing similar testcase, that'd help me a lot to get started. > Does the patch affect the output of labels when underlining ranges of > source code in diagnostics? AFAIU (and tested), it doesn't affect source code output. So, no? > Does the patch interact correctly with the %H and %I codes that try to > show the differences between two template types? I don't know. I'll try to find out. If you have a good idea (or pointer) for a testcase, let me know. > I have some minor nits from a diagnostics point of view: > [...] > Please add an auto_diagnostic_group here so that the "inform" is > associated with the "error". > [...] > diagnose_as should be in quotes here (%< and %>). > [...] > Please quote extern "C": Thanks. All done in my tree. I'll work on testcases before sending an updated patch. > Thanks again for the patch; hope this is constructive 👍 -- ────────────────────────────────────────────────────────────────────────── 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 ────────────────────────────────────────────────────────────────────────── ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-05-04 14:23 ` Matthias Kretz @ 2021-05-04 14:32 ` Matthias Kretz 2021-05-04 19:00 ` David Malcolm 1 sibling, 0 replies; 30+ messages in thread From: Matthias Kretz @ 2021-05-04 14:32 UTC (permalink / raw) To: gcc-patches, David Malcolm; +Cc: libstdc++ On Tuesday, 4 May 2021 16:23:23 CEST Matthias Kretz wrote: > On Tuesday, 4 May 2021 15:34:13 CEST David Malcolm wrote: > > Does the patch interact correctly with the %H and %I codes that try to > > show the differences between two template types? > > I don't know. I'll try to find out. If you have a good idea (or pointer) for > a testcase, let me know. I see it now. It currently does not interact with %H and %I (at least in my tests). I'll investigate what it should do. -- ────────────────────────────────────────────────────────────────────────── 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 ────────────────────────────────────────────────────────────────────────── ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-05-04 14:23 ` Matthias Kretz 2021-05-04 14:32 ` Matthias Kretz @ 2021-05-04 19:00 ` David Malcolm 2021-05-04 19:22 ` Matthias Kretz 1 sibling, 1 reply; 30+ messages in thread From: David Malcolm @ 2021-05-04 19:00 UTC (permalink / raw) To: Matthias Kretz, gcc-patches; +Cc: libstdc++ On Tue, 2021-05-04 at 16:23 +0200, Matthias Kretz wrote: > On Tuesday, 4 May 2021 15:34:13 CEST David Malcolm wrote: > > On Tue, 2021-05-04 at 13:13 +0200, Matthias Kretz wrote: > > > 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. > > > [...] > > > > Thanks for the patch, it looks very promising. > > Thanks. I'm new to modifying the compiler like this, so please be > extra > careful with my patch. I believe I understand most of what I did, but > I might > have misunderstood. :) That's tends to be how I feel when working on the C++ FE :) > > > The patch has no testcases; it should probably add test coverage > > for: > > - the various places and ways in which diagnose_as can affect the > > output, > > - disabling it with the option > > - the various ways in which the user can get diagnose_as wrong > > - etc > > Right. If you know of an existing similar testcase, that'd help me a > lot to > get started. FWIW I implemented the %H and %I codes in f012c8ef4b35dcee9b5a3807868d050812d5b3b9 and that commit adds various DejaGnu testcases that exercise C++ diagnostics with and without various options, verifying the precise wording of errors. So that might be a good place to look. > > > Does the patch affect the output of labels when underlining ranges of > > source code in diagnostics? > > AFAIU (and tested), it doesn't affect source code output. So, no? Sorry, I was unclear. I was referring to this kind of thing: $ cat t.C #include <string> std::string test (std::string s) { return &s; } $ g++ t.C t.C: In function ‘std::string test(std::string)’: t.C:5:12: error: could not convert ‘& s’ from ‘std::string*’ {aka ‘std::__cxx11::basic_string<char>*’} to ‘std::string’ {aka ‘std::__cxx11::basic_string<char>’} 5 | return &s; | ^~ | | | std::string* {aka std::__cxx11::basic_string<char>*} i.e. the final line in the output above, where the underlined range of source code in line 5 is labelled, showing the type of the expression. Hopefully it ought to automatically just drop out of the work you've already done. FWIW an example of implementation this can be seen in a14feb3c783fba6af8d66b8138214a3a313be5c5 (which added labels for type errors in C++ binary operators). > Does the patch interact correctly with the %H and %I codes that try to > show the differences between two template types? I don't know. I'll try to find out. If you have a good idea (or pointer) for a testcase, let me know. See above. Dave ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-05-04 19:00 ` David Malcolm @ 2021-05-04 19:22 ` Matthias Kretz 0 siblings, 0 replies; 30+ messages in thread From: Matthias Kretz @ 2021-05-04 19:22 UTC (permalink / raw) To: gcc-patches, David Malcolm; +Cc: libstdc++ [-- Attachment #1: Type: text/plain, Size: 2034 bytes --] > On Tuesday, 4 May 2021 15:34:13 CEST David Malcolm wrote: > > Does the patch interact correctly with the %H and %I codes that try to > > show the differences between two template types? While looking into this, I noticed that given namespace std { struct A {}; typedef A B; } const std::B would print as "'const B' {aka 'const std::A'}", i.e. without printing the scope of the typedef. I traced it to cp/error.c (dump_type). In the `if (TYPE_P (t) && typedef_variant_p (t))` branch, in the final else branch only cv-qualifiers and identifier are printed: pp_cxx_cv_qualifier_seq (pp, t); pp_cxx_tree_identifier (pp, TYPE_IDENTIFIER (t)); I believe the following should go in between, correct? pp_cxx_cv_qualifier_seq (pp, t); if (! (flags & TFF_UNQUALIFIED_NAME)) dump_scope (pp, CP_DECL_CONTEXT (TYPE_NAME (t)), flags); pp_cxx_tree_identifier (pp, TYPE_IDENTIFIER (t)); This is important for my diagnose_as patch because otherwise the output is: 'const string' {aka 'const std::string'} which is confusing and unnecessarily verbose. Patch below. From: Matthias Kretz <kretz@kde.org> dump_type on 'const std::string' should not print 'const string' unless TFF_UNQUALIFIED_NAME is requested. gcc/cp/ChangeLog: * error.c: Call dump_scope when printing a typedef. --- gcc/cp/error.c | 2 ++ 1 file changed, 2 insertions(+) -- ────────────────────────────────────────────────────────────────────────── 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 ────────────────────────────────────────────────────────────────────────── [-- Attachment #2: 0001-Add-missing-scope-in-typedef-diagnostic.patch --] [-- Type: text/x-patch, Size: 427 bytes --] diff --git a/gcc/cp/error.c b/gcc/cp/error.c index 10b547afaa7..edeaad44bcd 100644 --- a/gcc/cp/error.c +++ b/gcc/cp/error.c @@ -511,6 +511,8 @@ dump_type (cxx_pretty_printer *pp, tree t, int flags) else { pp_cxx_cv_qualifier_seq (pp, t); + if (! (flags & TFF_UNQUALIFIED_NAME)) + dump_scope (pp, CP_DECL_CONTEXT (TYPE_NAME (t)), flags); pp_cxx_tree_identifier (pp, TYPE_IDENTIFIER (t)); return; } ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-05-04 11:13 ` [PATCH] Add gnu::diagnose_as attribute Matthias Kretz 2021-05-04 13:34 ` David Malcolm @ 2021-05-14 16:05 ` Martin Sebor 2021-05-26 21:33 ` Matthias Kretz 2021-05-27 17:39 ` Jason Merrill 2 siblings, 1 reply; 30+ messages in thread From: Martin Sebor @ 2021-05-14 16:05 UTC (permalink / raw) To: Matthias Kretz, gcc-patches; +Cc: libstdc++ On 5/4/21 5:13 AM, Matthias Kretz wrote: > 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. There are other mechanisms to change symbol names such as macros, typedefs or alias or using declarations, and at the object level, ELF aliases, and even the C++ ABI with its substitutions for types like std::string and with the abi_tag attribute. They all have a potential to cause some confusion when a symbol with one name is referred to by another. For the syntactic mechanisms compilers usually deal with the problem by mentioning both the macro or alias name and its expansion or target in the diagnostics they issue. The renaming prescribed by the C++ is reflected in the mangled symbols that demanglers know how to decode and restore the their originals. For ELF aliases no such solution exists but they are used only rarely by well-tested system code and so the different names don't typically come up in compiler (or even linker) diagnostics. When they do, the names often share some common prefix or suffix so the confusion is minimal. I like the idea of printing names in diagnostics that are familiar to users while leaving out implementation details that's in most cases irrelevant to them. 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. 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__. 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.) 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? (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.) Below are a few comments on the changes below, mostly focused on the phrasing and style of diagnostic messages. As others already mentioned, the patch should include tests (including them early in the review process helps get an idea of how the whole feature works). > > 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]::. > 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. > > 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 > * 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_aggr_type): If the type has a diagnose_as attribute, print > the associated string instead of printing the original type > name. > (dump_simple_decl): Call dump_decl_name_or_diagnose_as instead > of dump_decl. > (dump_decl): Ditto. > (lang_decl_name): Ditto. > (dump_function_decl): Ensure complete replacement of the class > template diagnostics if a diagnose_as attribute is present. > (dump_function_name): Replace the function diagnostic output if > the diagnose_as attribute 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/c-family/c.opt | 4 ++ > gcc/cp/error.c | 85 ++++++++++++++++++++++++++++--- > gcc/cp/name-lookup.c | 27 ++++++++++ > gcc/cp/tree.c | 117 +++++++++++++++++++++++++++++++++++++++++++ > gcc/doc/extend.texi | 37 ++++++++++++++ > gcc/doc/invoke.texi | 9 +++- > 6 files changed, 270 insertions(+), 9 deletions(-) > > > -- > ────────────────────────────────────────────────────────────────────────── > Dr. Matthias Kretzhttps://mattkretz.github.io > GSI Helmholtz Centre for Heavy Ion Researchhttps://gsi.de > std::experimental::simdhttps://github.com/VcDevel/std-simd > ────────────────────────────────────────────────────────────────────────── > > > 0001-Add-gnu-diagnose_as-attribute.patch > > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt > index 3f8b72cdc00..0cf01c6dba4 100644 > --- a/gcc/c-family/c.opt > +++ b/gcc/c-family/c.opt > @@ -1582,6 +1582,10 @@ fdiagnostics-show-template-tree > C++ ObjC++ Var(flag_diagnostics_show_template_tree) Init(0) > Print hierarchical comparisons when template types are mismatched. > > +fdiagnostics-use-aliases > +C++ Var(flag_diagnostics_use_aliases) Init(1) > +Replace identifiers or scope names in diagnostics as defined by the diagnose_as attribute. > + > fdirectives-only > C ObjC C++ ObjC++ > Preprocess directives only. > diff --git a/gcc/cp/error.c b/gcc/cp/error.c > index c88d1749a0f..10b547afaa7 100644 > --- a/gcc/cp/error.c > +++ b/gcc/cp/error.c > @@ -35,6 +35,7 @@ along with GCC; see the file COPYING3. If not see > #include "internal-fn.h" > #include "gcc-rich-location.h" > #include "cp-name-hint.h" > +#include "attribs.h" > > #define pp_separate_with_comma(PP) pp_cxx_separate_with (PP, ',') > #define pp_separate_with_semicolon(PP) pp_cxx_separate_with (PP, ';') > @@ -66,6 +67,7 @@ static void dump_alias_template_specialization (cxx_pretty_printer *, tree, int) > static void dump_type (cxx_pretty_printer *, tree, int); > static void dump_typename (cxx_pretty_printer *, tree, int); > static void dump_simple_decl (cxx_pretty_printer *, tree, tree, int); > +static void dump_decl_name_or_diagnose_as (cxx_pretty_printer *, tree, int); > static void dump_decl (cxx_pretty_printer *, tree, int); > static void dump_template_decl (cxx_pretty_printer *, tree, int); > static void dump_function_decl (cxx_pretty_printer *, tree, int); > @@ -231,7 +233,15 @@ dump_scope (cxx_pretty_printer *pp, tree scope, int flags) > { > if (scope != global_namespace) > { > - dump_decl (pp, scope, f); > + tree diagnose_as > + = flag_diagnostics_use_aliases > + ? lookup_attribute ("diagnose_as", DECL_ATTRIBUTES (scope)) > + : NULL_TREE; > + if (diagnose_as) > + pp_cxx_ws_string ( > + pp, TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (diagnose_as)))); > + else > + dump_decl (pp, scope, f); > pp_cxx_colon_colon (pp); > } > } > @@ -743,6 +753,7 @@ dump_aggr_type (cxx_pretty_printer *pp, tree t, int flags) > > tree decl = TYPE_NAME (t); > > + tree diagnose_as = NULL_TREE; > if (decl) > { > typdef = (!DECL_ARTIFICIAL (decl) > @@ -769,8 +780,15 @@ dump_aggr_type (cxx_pretty_printer *pp, tree t, int flags) > if (! (flags & TFF_UNQUALIFIED_NAME)) > dump_scope (pp, CP_DECL_CONTEXT (decl), flags | TFF_SCOPE); > flags &= ~TFF_UNQUALIFIED_NAME; > + > + tree diagnose_as_specialized = NULL_TREE; > if (tmplate) > { > + if (flag_diagnostics_use_aliases) > + diagnose_as_specialized > + = lookup_attribute ("diagnose_as", > + TYPE_ATTRIBUTES (TREE_TYPE (decl))); > + > /* Because the template names are mangled, we have to locate > the most general template, and use that name. */ > tree tpl = TYPE_TI_TEMPLATE (t); > @@ -779,9 +797,25 @@ dump_aggr_type (cxx_pretty_printer *pp, tree t, int flags) > tpl = DECL_TI_TEMPLATE (tpl); > decl = tpl; > } > + > + if (flag_diagnostics_use_aliases) > + diagnose_as = lookup_attribute ("diagnose_as", > + TYPE_ATTRIBUTES (TREE_TYPE (decl))); > + if (diagnose_as_specialized > + && (!diagnose_as || TREE_VALUE(diagnose_as_specialized) > + != TREE_VALUE(diagnose_as))) > + { > + pp_cxx_ws_string ( > + pp, TREE_STRING_POINTER ( > + TREE_VALUE (TREE_VALUE (diagnose_as_specialized)))); > + return; > + } > } > > - if (LAMBDA_TYPE_P (t)) > + if (diagnose_as) > + pp_cxx_ws_string (pp, TREE_STRING_POINTER ( > + TREE_VALUE (TREE_VALUE (diagnose_as)))); > + else if (LAMBDA_TYPE_P (t)) > { > /* A lambda's "type" is essentially its signature. */ > pp_string (pp, M_("<lambda")); > @@ -1103,7 +1137,7 @@ dump_simple_decl (cxx_pretty_printer *pp, tree t, tree type, int flags) > pp_string (pp, " capture>"); > } > else > - dump_decl (pp, DECL_NAME (t), flags); > + dump_decl_name_or_diagnose_as (pp, t, flags); > } > else if (DECL_DECOMPOSITION_P (t)) > pp_string (pp, M_("<structured bindings>")); > @@ -1147,6 +1181,25 @@ dump_decl_name (cxx_pretty_printer *pp, tree t, int flags) > pp_cxx_tree_identifier (pp, t); > } > > +/* Print the DECL_NAME of DECL unless a different string was requested by the > + diagnose_as attribute. */ > + > +static void > +dump_decl_name_or_diagnose_as (cxx_pretty_printer *pp, tree decl, int flags) > +{ > + if (flag_diagnostics_use_aliases) > + { > + tree attr = lookup_attribute ("diagnose_as", DECL_ATTRIBUTES (decl)); > + if (attr) > + { > + pp_cxx_ws_string ( > + pp, TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr)))); > + return; > + } > + } > + dump_decl_name (pp, DECL_NAME (decl), flags); > +} > + > /* Dump a human readable string for the decl T under control of FLAGS. */ > > static void > @@ -1192,7 +1245,7 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags) > || flags & TFF_CLASS_KEY_OR_ENUM)) > { > pp_cxx_ws_string (pp, "using"); > - dump_decl (pp, DECL_NAME (t), flags); > + dump_decl_name_or_diagnose_as(pp, t, flags); > pp_cxx_whitespace (pp); > pp_cxx_ws_string (pp, "="); > pp_cxx_whitespace (pp); > @@ -1374,7 +1427,7 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags) > TREE_CODE (DECL_INITIAL (t)) == TEMPLATE_PARM_INDEX)) > dump_simple_decl (pp, t, TREE_TYPE (t), flags); > else if (DECL_NAME (t)) > - dump_decl (pp, DECL_NAME (t), flags); > + dump_decl_name_or_diagnose_as (pp, t, flags); > else if (DECL_INITIAL (t)) > dump_expr (pp, DECL_INITIAL (t), flags | TFF_EXPR_IN_PARENS); > else > @@ -1393,7 +1446,7 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags) > } > dump_type (pp, scope, flags); > pp_cxx_colon_colon (pp); > - dump_decl (pp, DECL_NAME (t), flags); > + dump_decl_name_or_diagnose_as (pp, t, flags); > if (variadic) > pp_cxx_ws_string (pp, "..."); > } > @@ -1657,7 +1710,13 @@ dump_function_decl (cxx_pretty_printer *pp, tree t, int flags) > /* Pretty print template instantiations only. */ > if (DECL_USE_TEMPLATE (t) && DECL_TEMPLATE_INFO (t) > && !(flags & TFF_NO_TEMPLATE_BINDINGS) > - && flag_pretty_templates) > + && flag_pretty_templates > + // skip the output of template parameters if the function is a member of a > + // class with diagnose_as attribute: > + && !(flag_diagnostics_use_aliases > + && DECL_CLASS_SCOPE_P (t) > + && lookup_attribute ("diagnose_as", > + TYPE_ATTRIBUTES (DECL_CONTEXT (t))))) > { > tree tmpl; > > @@ -1885,6 +1944,16 @@ dump_exception_spec (cxx_pretty_printer *pp, tree t, int flags) > static void > dump_function_name (cxx_pretty_printer *pp, tree t, int flags) > { > + if (flag_diagnostics_use_aliases) > + { > + tree attr = lookup_attribute ("diagnose_as", DECL_ATTRIBUTES (t)); > + if (attr) > + { > + pp_cxx_ws_string ( > + pp, TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr)))); > + return; > + } > + } > tree name = DECL_NAME (t); > > /* We can get here with a decl that was synthesized by language- > @@ -3133,7 +3202,7 @@ lang_decl_name (tree decl, int v, bool translate) > && TREE_CODE (decl) == NAMESPACE_DECL) > dump_decl (cxx_pp, decl, TFF_PLAIN_IDENTIFIER | TFF_UNQUALIFIED_NAME); > else > - dump_decl (cxx_pp, DECL_NAME (decl), TFF_PLAIN_IDENTIFIER); > + dump_decl_name_or_diagnose_as (cxx_pp, decl, TFF_PLAIN_IDENTIFIER); > > return pp_ggc_formatted_text (cxx_pp); > } > diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c > index 4e84e2f9987..80637503310 100644 > --- a/gcc/cp/name-lookup.c > +++ b/gcc/cp/name-lookup.c > @@ -6082,6 +6082,33 @@ handle_namespace_attrs (tree ns, tree attributes) > DECL_ATTRIBUTES (ns) = tree_cons (name, args, > DECL_ATTRIBUTES (ns)); > } > + else if (is_attribute_p ("diagnose_as", name)) > + { > + if (!args || TREE_CHAIN(args)) > + { > + error ("the %qE attribute requires exactly one argument", name); 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). > + 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. > + continue; > + } > + tree existing > + = lookup_attribute ("diagnose_as", DECL_ATTRIBUTES (ns)); > + if (existing > + && !cp_tree_equal(TREE_VALUE (args), > + TREE_VALUE (TREE_VALUE (existing)))) > + { > + error ("the namespace %qE already uses a different diagnose_as " > + "attribute value", ns); Here too, please drop the article. Also, please quote the attribute name (using %qs or %<diagnose_as%>). > + inform (DECL_SOURCE_LOCATION (ns), "previous declaration here"); > + continue; > + } > + DECL_ATTRIBUTES (ns) = tree_cons (name, args, > + DECL_ATTRIBUTES (ns)); > + } > else > { > warning (OPT_Wattributes, "%qD attribute directive ignored", > diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c > index a8bfd5fc053..f7b93dc89d7 100644 > --- a/gcc/cp/tree.c > +++ b/gcc/cp/tree.c > @@ -46,6 +46,7 @@ static tree verify_stmt_tree_r (tree *, int *, void *); > > static tree handle_init_priority_attribute (tree *, tree, tree, int, bool *); > static tree handle_abi_tag_attribute (tree *, tree, tree, int, bool *); > +static tree handle_diagnose_as_attribute (tree *, tree, tree, int, bool *); > > /* If REF is an lvalue, returns the kind of lvalue that REF is. > Otherwise, returns clk_none. */ > @@ -4860,6 +4861,8 @@ const struct attribute_spec cxx_attribute_table[] = > handle_init_priority_attribute, NULL }, > { "abi_tag", 1, -1, false, false, false, true, > handle_abi_tag_attribute, NULL }, > + { "diagnose_as", 1, 1, false, false, false, false, > + handle_diagnose_as_attribute, NULL }, > { NULL, 0, 0, false, false, false, false, NULL, NULL } > }; > > @@ -5128,6 +5131,120 @@ handle_abi_tag_attribute (tree* node, tree name, tree args, > return NULL_TREE; > } > > +static bool > +check_diagnose_as_redeclaration (const_tree decl, const_tree old, > + const_tree new_) > +{ > + if (!old) > + return true; > + if (TREE_CODE (TREE_VALUE (old)) == TREE_LIST) > + old = TREE_VALUE (old); > + if (TREE_CODE (TREE_VALUE (new_)) == TREE_LIST) > + new_ = TREE_VALUE (new_); > + tree old_value = TREE_VALUE (old); > + tree new_value = TREE_VALUE (new_); > + if (cp_tree_equal (old_value, new_value)) > + return true; > + error ("conflicting declaration of %<diagnose_as%> attribute on %qD would " > + "overwrite %qE with %qE", decl, old_value, new_value); There are a few messages GCC issues to diagnose attribute conflicts. Conflicts between different attributes are usually diagnosed by warnings: msgid "ignoring attribute %qE because it conflicts with attribute %qs" but some conflicts cause errors: msgid "section of %q+D conflicts with previous declaration" Either way, I would again recommend adopting one of the existing styles for consistency rather than introducing a new one. > + return false; > +} > + > +static tree > +handle_diagnose_as_attribute (tree* node, tree name, tree args, > + int flags, bool* no_add_attrs) > +{ > + tree decl = NULL_TREE; > + if (!args || TREE_CHAIN (args)) > + { > + error ("the %qE attribute requires exactly one argument", name); > + goto fail; > + } > + if (TREE_CODE (TREE_VALUE (args)) != STRING_CST) > + { > + error ("the argument to the %qE attribute must be a string literal", > + name); Same comment here. E.g., msgid "%qE attribute argument must be a string constant" in the same file. > + goto fail; > + } > + 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_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" > + 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); > + } > + } > + } > + else if (TYPE_P (*node)) > + { > + if (!OVERLOAD_TYPE_P (*node)) > + { > + error ("%qE attribute applied to non-class, non-enum type %qT", > + name, *node); > + goto fail; > + } > + else if (!(flags & (int)ATTR_FLAG_TYPE_IN_PLACE)) > + { > + error ("%qE attribute applied to %qT after its definition", > + name, *node); > + goto fail; > + } > + decl = TYPE_NAME (*node); > + } > + else > + { > + if (!VAR_OR_FUNCTION_DECL_P (*node)) > + { > + error ("%qE attribute applied to non-function, non-variable %qD", > + name, *node); > + goto fail; > + } > + else if (DECL_LANGUAGE (*node) == lang_c) > + { > + error ("%qE attribute applied to extern \"C\" declaration %qD", Please quote extern "C" (as "%<extern \"C\"%>). > + name, *node); > + goto fail; > + } > + decl = *node; > + } > + > + // Make sure all declarations have the same diagnose_as string. > + if (DECL_SOURCE_LOCATION (decl) != input_location) > + { > + tree attributes = TYPE_P (*node) ? TYPE_ATTRIBUTES (*node) > + : DECL_ATTRIBUTES (decl); > + if (!check_diagnose_as_redeclaration ( > + decl, lookup_attribute ("diagnose_as", attributes), args)) > + goto fail; > + } > + else if (CLASS_TYPE_P (*node) && CLASSTYPE_IMPLICIT_INSTANTIATION (*node)) > + { > + // The above branch (different source location) is taken for declarations > + // of type aliases that modify an implicit template specialization on the > + // RHS. This branch is taken when the template is instantiated via > + // instantiate_class_template_1, in which case the attribute either > + // already has the value from the general template or from a type alias. > + goto fail; > + } > + > + return NULL_TREE; > + > + fail: > + *no_add_attrs = true; > + return NULL_TREE; > +} > + > /* Return a new PTRMEM_CST of the indicated TYPE. The MEMBER is the > thing pointed to by the constant. */ > > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi > index c8caf36f293..7976e3a72d4 100644 > --- a/gcc/doc/extend.texi > +++ b/gcc/doc/extend.texi > @@ -2865,6 +2865,43 @@ types (@pxref{Variable Attributes}, @pxref{Type Attributes}.) > The message attached to the attribute is affected by the setting of > the @option{-fmessage-length} option. > > +@item diagnose_as ("@var{string}") > +@cindex @code{diagnose_as} function attribute > +The @code{diagnose_as} attribute modifies how the entity the attribute > +appertains to is diagnosed in compiler messages and __FUNCTION__ / > +__PRETTY_FUNCTION__ macros. Presumably also __func__. Symbol names should be quoted in @code{}. If the attribute is applied to a @code{namespace}, > +the specified string replaces the complete enclosing scope. The effect of the > +@code{diagnose_as} attribute can be disabled with the > +@option{-fno-diagnostics-use-aliases} command line option. If the attribute doesn't change typeinfo it might be worth mentioning in the documentation. Martin > + > +@smallexample > +namespace Foo @{ > + namespace Bar @{ > + inline namespace v1 [[gnu::diagnose_as("Foobar")]] @{ > + int f() @{ > + // __PRETTY_FUNCTION__ == "void Foobar::f()" > + @} > + @} > + @} > +@} > +// In function 'int Foobar::f()': > +// warning: no return statement in function returning non-void [-Wreturn-type] > +@end smallexample > + > +The @code{diagnose_as} attribute can be used with namespaces, functions, > +variables, alias declarations (but not alias templates), and user-defined types > +(classes, unions, and enums). If the alias declaration aliases an implicit class > +template specialization, the attribute is additionally applied to the class > +template specialization. > + > +@smallexample > +template <class T> struct basic_string @{@}; > +using string [[gnu::diagnose_as("string")]] = basic_string<char>; > +int f(basic_string<char>) @{@} > +// In function 'int f(string)': > +// warning: no return statement in function returning non-void [-Wreturn-type] > +@end smallexample > + > @item error ("@var{message}") > @itemx warning ("@var{message}") > @cindex @code{error} function attribute > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index f5a9dc33819..84c19344c89 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -311,7 +311,8 @@ Objective-C and Objective-C++ Dialects}. > -fdiagnostics-show-path-depths @gol > -fno-show-column @gol > -fdiagnostics-column-unit=@r{[}display@r{|}byte@r{]} @gol > --fdiagnostics-column-origin=@var{origin}} > +-fdiagnostics-column-origin=@var{origin} @gol > +-fno-diagnostics-aliases} > > @item Warning Options > @xref{Warning Options,,Options to Request or Suppress Warnings}. > @@ -5077,6 +5078,12 @@ first column. The default value of 1 corresponds to traditional GCC > behavior and to the GNU style guide. Some utilities may perform better with an > origin of 0; any non-negative value may be specified. > > +@item -fno-diagnostics-use-aliases > +@opindex fno-diagnostics-use-aliases > +@opindex fdiagnostics-use-aliases > +Do not replace identifiers or scope names with the aliases defined with the > +@code{[[gnu::diagnose_as("alias")]]} attribute. > + > @item -fdiagnostics-format=@var{FORMAT} > @opindex fdiagnostics-format > Select a different format for printing diagnostics. > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-05-14 16:05 ` Martin Sebor @ 2021-05-26 21:33 ` Matthias Kretz 0 siblings, 0 replies; 30+ messages in thread From: Matthias Kretz @ 2021-05-26 21:33 UTC (permalink / raw) To: gcc-patches, Martin Sebor; +Cc: libstdc++, David Malcolm [-- Attachment #1: Type: text/plain, Size: 11592 bytes --] 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 ────────────────────────────────────────────────────────────────────────── [-- Attachment #2: 0004-Add-gnu-diagnose_as-attribute.patch --] [-- Type: text/x-patch, Size: 49794 bytes --] diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 3f8b72cdc00..0cf01c6dba4 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -1582,6 +1582,10 @@ fdiagnostics-show-template-tree C++ ObjC++ Var(flag_diagnostics_show_template_tree) Init(0) Print hierarchical comparisons when template types are mismatched. +fdiagnostics-use-aliases +C++ Var(flag_diagnostics_use_aliases) Init(1) +Replace identifiers or scope names in diagnostics as defined by the diagnose_as attribute. + fdirectives-only C ObjC C++ ObjC++ Preprocess directives only. diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index cb254e0adea..139ef8f1c0d 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -5858,7 +5858,9 @@ enum auto_deduction_context identical to their defaults. TFF_NO_TEMPLATE_BINDINGS: do not print information about the template arguments for a function template specialization. - TFF_POINTER: we are printing a pointer type. */ + TFF_POINTER: we are printing a pointer type. + TFF_AS_PRIMARY: treat as primary template even if printing a specialized + type. */ #define TFF_PLAIN_IDENTIFIER (0) #define TFF_SCOPE (1) @@ -5876,6 +5878,7 @@ enum auto_deduction_context #define TFF_NO_OMIT_DEFAULT_TEMPLATE_ARGUMENTS (1 << 12) #define TFF_NO_TEMPLATE_BINDINGS (1 << 13) #define TFF_POINTER (1 << 14) +#define TFF_AS_PRIMARY (1 << 15) /* These constants can be used as bit flags to control strip_typedefs. diff --git a/gcc/cp/error.c b/gcc/cp/error.c index b0836d83888..1950ffb8ab6 100644 --- a/gcc/cp/error.c +++ b/gcc/cp/error.c @@ -35,6 +35,7 @@ along with GCC; see the file COPYING3. If not see #include "internal-fn.h" #include "gcc-rich-location.h" #include "cp-name-hint.h" +#include "attribs.h" #define pp_separate_with_comma(PP) pp_cxx_separate_with (PP, ',') #define pp_separate_with_semicolon(PP) pp_cxx_separate_with (PP, ';') @@ -66,6 +67,7 @@ static void dump_alias_template_specialization (cxx_pretty_printer *, tree, int) static void dump_type (cxx_pretty_printer *, tree, int); static void dump_typename (cxx_pretty_printer *, tree, int); static void dump_simple_decl (cxx_pretty_printer *, tree, tree, int); +static void dump_decl_name_or_diagnose_as (cxx_pretty_printer *, tree, int); static void dump_decl (cxx_pretty_printer *, tree, int); static void dump_template_decl (cxx_pretty_printer *, tree, int); static void dump_function_decl (cxx_pretty_printer *, tree, int); @@ -231,7 +233,15 @@ dump_scope (cxx_pretty_printer *pp, tree scope, int flags) { if (scope != global_namespace) { - dump_decl (pp, scope, f); + tree diagnose_as + = flag_diagnostics_use_aliases + ? lookup_attribute ("diagnose_as", DECL_ATTRIBUTES (scope)) + : NULL_TREE; + if (diagnose_as) + pp_cxx_ws_string ( + pp, TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (diagnose_as)))); + else + dump_decl (pp, scope, f); pp_cxx_colon_colon (pp); } } @@ -760,6 +770,45 @@ class_key_or_enum_as_string (tree t) return "struct"; } +/* Print out an enclosing scope of a template instance by inspecting the tree + SPECIAL of the template instantiation and the tree GENERAL of the most + general template in parallel under the control of FLAGS while adjusting + PARMS as needed. This allows the diagnose_as attribute to override the name + of full specializations, while hiding its template parameters, and to + override the name of partial specializations without falling back to printing + the template parameters of the general template decl. */ + +static void +dump_template_scope (cxx_pretty_printer *pp, tree special, tree general, + tree *parms, int flags) +{ + gcc_assert (parms); + if (special != general && RECORD_OR_UNION_TYPE_P (special) && *parms) + { + gcc_assert (RECORD_OR_UNION_TYPE_P (general)); + const bool tmplate + = TYPE_LANG_SPECIFIC (special) && CLASSTYPE_TEMPLATE_INFO (special) + && (TREE_CODE (CLASSTYPE_TI_TEMPLATE (special)) != TEMPLATE_DECL + || PRIMARY_TEMPLATE_P (CLASSTYPE_TI_TEMPLATE (special))); + dump_template_scope (pp, CP_TYPE_CONTEXT (special), + CP_TYPE_CONTEXT (general), + tmplate ? &TREE_CHAIN(*parms) : parms, flags); + tree attr = lookup_attribute ("diagnose_as", TYPE_ATTRIBUTES (special)); + if (attr) + { + pp_cxx_ws_string (pp, TREE_STRING_POINTER ( + TREE_VALUE (TREE_VALUE (attr)))); + if (tmplate) + TREE_VALUE (*parms) = make_tree_vec (0); + } + else + dump_aggr_type (pp, general, flags | TFF_UNQUALIFIED_NAME); + pp_cxx_colon_colon (pp); + } + else + dump_scope (pp, general, flags); +} + /* Print out a class declaration T under the control of FLAGS, in the form `class foo'. */ @@ -777,6 +826,7 @@ dump_aggr_type (cxx_pretty_printer *pp, tree t, int flags) tree decl = TYPE_NAME (t); + tree diagnose_as = NULL_TREE; if (decl) { typdef = (!DECL_ARTIFICIAL (decl) @@ -803,8 +853,14 @@ dump_aggr_type (cxx_pretty_printer *pp, tree t, int flags) if (! (flags & TFF_UNQUALIFIED_NAME)) dump_scope (pp, CP_DECL_CONTEXT (decl), flags | TFF_SCOPE); flags &= ~TFF_UNQUALIFIED_NAME; + + tree diagnose_as_specialized = NULL_TREE; if (tmplate) { + if (flag_diagnostics_use_aliases) + diagnose_as_specialized + = lookup_attribute ("diagnose_as", TYPE_ATTRIBUTES (t)); + /* Because the template names are mangled, we have to locate the most general template, and use that name. */ tree tpl = TYPE_TI_TEMPLATE (t); @@ -813,9 +869,54 @@ dump_aggr_type (cxx_pretty_printer *pp, tree t, int flags) tpl = DECL_TI_TEMPLATE (tpl); decl = tpl; } + + if (flag_diagnostics_use_aliases) + { + diagnose_as = lookup_attribute ("diagnose_as", + TYPE_ATTRIBUTES (TREE_TYPE (decl))); + if (diagnose_as_specialized + && (!diagnose_as || TREE_VALUE (diagnose_as_specialized) + != TREE_VALUE (diagnose_as))) + { + diagnose_as = diagnose_as_specialized; + /* Skip dump_template_parms if diagnose_as applies to a full + specialization. */ + tree args = CLASSTYPE_TI_ARGS (t); + args = INNERMOST_TEMPLATE_ARGS (args); + gcc_assert (args); + tmplate = false; + for (int i = 0; i < NUM_TMPL_ARGS (args); ++i) + { + tree arg = TREE_VEC_ELT (args, i); + while (INDIRECT_TYPE_P (arg)) + arg = TREE_TYPE (arg); + if (WILDCARD_TYPE_P (arg)) + { + tmplate = true; + break; + } + } + } + /* Given + + template <class T> struct [[gnu::diagnose_as("AA")]] A { + struct [[gnu::diagnose_as("BB")]] B {}; + }; + + A<int>::B will reach the following branch and find the diagnose_as + attribute for B. */ + else if (!tmplate && !diagnose_as && TYPE_TEMPLATE_INFO (t)) + diagnose_as + = lookup_attribute ("diagnose_as", + TYPE_ATTRIBUTES ( + TREE_TYPE (TYPE_TI_TEMPLATE (t)))); + } } - if (LAMBDA_TYPE_P (t)) + if (diagnose_as) + pp_cxx_ws_string (pp, TREE_STRING_POINTER ( + TREE_VALUE (TREE_VALUE (diagnose_as)))); + else if (LAMBDA_TYPE_P (t)) { /* A lambda's "type" is essentially its signature. */ pp_string (pp, M_("<lambda")); @@ -1137,7 +1238,7 @@ dump_simple_decl (cxx_pretty_printer *pp, tree t, tree type, int flags) pp_string (pp, " capture>"); } else - dump_decl (pp, DECL_NAME (t), flags); + dump_decl_name_or_diagnose_as (pp, t, flags); } else if (DECL_DECOMPOSITION_P (t)) pp_string (pp, M_("<structured bindings>")); @@ -1181,6 +1282,25 @@ dump_decl_name (cxx_pretty_printer *pp, tree t, int flags) pp_cxx_tree_identifier (pp, t); } +/* Print the DECL_NAME of DECL unless a different string was requested by the + diagnose_as attribute. */ + +static void +dump_decl_name_or_diagnose_as (cxx_pretty_printer *pp, tree decl, int flags) +{ + if (flag_diagnostics_use_aliases) + { + tree attr = lookup_attribute ("diagnose_as", DECL_ATTRIBUTES (decl)); + if (attr) + { + pp_cxx_ws_string ( + pp, TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr)))); + return; + } + } + dump_decl_name (pp, DECL_NAME (decl), flags); +} + /* Dump a human readable string for the decl T under control of FLAGS. */ static void @@ -1226,7 +1346,7 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags) || flags & TFF_CLASS_KEY_OR_ENUM)) { pp_cxx_ws_string (pp, "using"); - dump_decl (pp, DECL_NAME (t), flags); + dump_decl_name_or_diagnose_as(pp, t, flags); pp_cxx_whitespace (pp); pp_cxx_ws_string (pp, "="); pp_cxx_whitespace (pp); @@ -1408,7 +1528,7 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags) TREE_CODE (DECL_INITIAL (t)) == TEMPLATE_PARM_INDEX)) dump_simple_decl (pp, t, TREE_TYPE (t), flags); else if (DECL_NAME (t)) - dump_decl (pp, DECL_NAME (t), flags); + dump_decl_name_or_diagnose_as (pp, t, flags); else if (DECL_INITIAL (t)) dump_expr (pp, DECL_INITIAL (t), flags | TFF_EXPR_IN_PARENS); else @@ -1427,7 +1547,7 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags) } dump_type (pp, scope, flags); pp_cxx_colon_colon (pp); - dump_decl (pp, DECL_NAME (t), flags); + dump_decl_name_or_diagnose_as (pp, t, flags); if (variadic) pp_cxx_ws_string (pp, "..."); } @@ -1667,7 +1787,8 @@ dump_function_decl (cxx_pretty_printer *pp, tree t, int flags) tree template_args = NULL_TREE; tree template_parms = NULL_TREE; int show_return = flags & TFF_RETURN_TYPE || flags & TFF_DECL_SPECIFIERS; - int do_outer_scope = ! (flags & TFF_UNQUALIFIED_NAME); + bool do_outer_scope = ! (flags & TFF_UNQUALIFIED_NAME); + bool do_template_scope = false; tree exceptions; bool constexpr_p; tree ret = NULL_TREE; @@ -1684,6 +1805,11 @@ dump_function_decl (cxx_pretty_printer *pp, tree t, int flags) /* Likewise for the constexpr specifier, in case t is a specialization. */ constexpr_p = DECL_DECLARED_CONSTEXPR_P (t); + /* Keep t before the following branch makes t point to a more general + template. Without the specialized template, the diagnose_as attribute on + the function is lost. */ + tree specialized_t = t; + /* Pretty print template instantiations only. */ if (DECL_USE_TEMPLATE (t) && DECL_TEMPLATE_INFO (t) && !(flags & TFF_NO_TEMPLATE_BINDINGS) @@ -1695,8 +1821,46 @@ dump_function_decl (cxx_pretty_printer *pp, tree t, int flags) tmpl = most_general_template (t); if (tmpl && TREE_CODE (tmpl) == TEMPLATE_DECL) { + /* Inspect whether any record/union type in the context of the + instantiated function template carries a diagnose_as attribute. If + one does, we need to print the scope and template argument list + differently. Consider: + + template <class T> struct A { + template <class U0, class U1> struct B; + template <class U> struct B<U, int> { + void f(); + }; + }; + using Achar [[gnu::diagnose_as("Achar")]] = A<char>; + + Now Achar::B<int, int>::f() needs to print as "Achar::B<U, int>::f() + [with U = int]". However, DECL_CONTEXT (t) would print as + "Achar::B<int, int>" and dump_aggr_type has no sensible way to + retrieve A<T>::B<U, int>. tmpl was generalized to A<T>::B<U, int>::f + and thus dump_aggr_type (DECL_CONTEXT (tmpl)) would print + "A<T>::B<U, int> [with T = char, U = int]". I.e. the diagnose_as + attribute on the A<char> instantiation is lost. */ + if (tmpl != t && do_outer_scope && flag_diagnostics_use_aliases) + { + for (tree context = DECL_CONTEXT (t); + context && RECORD_OR_UNION_TYPE_P (context); + context = TYPE_CONTEXT (context)) + { + if (lookup_attribute ("diagnose_as", + TYPE_ATTRIBUTES (context))) + { + do_template_scope = true; + break; + } + } + } + template_parms = DECL_TEMPLATE_PARMS (tmpl); t = tmpl; + /* The "[with ...]" clause is printed, thus dump_template_params must + unconditionally present functions as primary templates. */ + dump_function_name_flags |= TFF_AS_PRIMARY; } } @@ -1743,6 +1907,25 @@ dump_function_decl (cxx_pretty_printer *pp, tree t, int flags) /* Print the function name. */ if (!do_outer_scope) /* Nothing. */; + else if (do_template_scope) + { + template_parms = copy_list (template_parms); + bool func_template = TREE_TYPE (TREE_VALUE (template_parms)) == t; + dump_template_scope(pp, DECL_CONTEXT (specialized_t), DECL_CONTEXT (t), + func_template ? &TREE_CHAIN (template_parms) + : &template_parms, + flags); + if (TREE_VEC_LENGTH (TREE_VALUE (template_parms)) == 0) + { + /* If no template parms are left, make it a NULL_TREE so that no + "[with ]" is printed. */ + tree p = TREE_CHAIN (template_parms); + for (; p && TREE_VEC_LENGTH (TREE_VALUE (p)) == 0; p = TREE_CHAIN (p)) + ; + if (!p) + template_parms = template_args = NULL_TREE; + } + } else if (cname) { dump_type (pp, cname, flags); @@ -1751,7 +1934,7 @@ dump_function_decl (cxx_pretty_printer *pp, tree t, int flags) else dump_scope (pp, CP_DECL_CONTEXT (t), flags); - dump_function_name (pp, t, dump_function_name_flags); + dump_function_name (pp, specialized_t, dump_function_name_flags); if (!(flags & TFF_NO_FUNCTION_ARGUMENTS)) { @@ -1931,6 +2114,14 @@ dump_function_name (cxx_pretty_printer *pp, tree t, int flags) if (TREE_CODE (t) == TEMPLATE_DECL) t = DECL_TEMPLATE_RESULT (t); + if (flag_diagnostics_use_aliases) + { + tree attr = lookup_attribute ("diagnose_as", DECL_ATTRIBUTES (t)); + if (attr) + name = get_identifier ( + TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr)))); + } + /* Don't let the user see __comp_ctor et al. */ if (DECL_CONSTRUCTOR_P (t) || DECL_DESTRUCTOR_P (t)) @@ -1985,6 +2176,8 @@ dump_template_parms (cxx_pretty_printer *pp, tree info, { tree args = info ? TI_ARGS (info) : NULL_TREE; + if (flags & TFF_AS_PRIMARY) + primary = true; if (primary && flags & TFF_TEMPLATE_NAME) return; flags &= ~(TFF_CLASS_KEY_OR_ENUM | TFF_TEMPLATE_NAME); @@ -3164,7 +3357,7 @@ lang_decl_name (tree decl, int v, bool translate) && TREE_CODE (decl) == NAMESPACE_DECL) dump_decl (cxx_pp, decl, TFF_PLAIN_IDENTIFIER | TFF_UNQUALIFIED_NAME); else - dump_decl (cxx_pp, DECL_NAME (decl), TFF_PLAIN_IDENTIFIER); + dump_decl_name_or_diagnose_as (cxx_pp, decl, TFF_PLAIN_IDENTIFIER); return pp_ggc_formatted_text (cxx_pp); } @@ -3949,6 +4142,13 @@ comparable_template_types_p (tree type_a, tree type_b) if (!CLASS_TYPE_P (type_b)) return false; + /* If one of the types has a diagnose_as attribute set it is presented as a + non-template (even if it's a template specialization). */ + if (flag_diagnostics_use_aliases + && (lookup_attribute ("diagnose_as", TYPE_ATTRIBUTES (type_a)) + || lookup_attribute ("diagnose_as", TYPE_ATTRIBUTES (type_b)))) + return false; + tree tinfo_a = TYPE_TEMPLATE_INFO (type_a); tree tinfo_b = TYPE_TEMPLATE_INFO (type_b); if (!tinfo_a || !tinfo_b) @@ -4048,8 +4248,21 @@ print_template_differences (pretty_printer *pp, tree type_a, tree type_b, tree tinfo_a = TYPE_TEMPLATE_INFO (type_a); tree tinfo_b = TYPE_TEMPLATE_INFO (type_b); - pp_printf (pp, "%s<", - IDENTIFIER_POINTER (DECL_NAME (TI_TEMPLATE (tinfo_a)))); + const char* identifier_a + = IDENTIFIER_POINTER (DECL_NAME (TI_TEMPLATE (tinfo_a))); + if (flag_diagnostics_use_aliases) + { + /* Locate the most general template, and see whether it carries the + diagnose_as attribute. If it does, use it as identifier for type_a. */ + tree tpl = TI_TEMPLATE (tinfo_a); + while (DECL_TEMPLATE_INFO (tpl)) + tpl = DECL_TI_TEMPLATE (tpl); + tree attr = lookup_attribute ("diagnose_as", + TYPE_ATTRIBUTES (TREE_TYPE (tpl))); + if (attr) + identifier_a = TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))); + } + pp_printf (pp, "%s<", identifier_a); tree args_a = TI_ARGS (tinfo_a); tree args_b = TI_ARGS (tinfo_b); diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index 4e84e2f9987..7782fd09ead 100644 --- a/gcc/cp/name-lookup.c +++ b/gcc/cp/name-lookup.c @@ -6082,6 +6082,31 @@ handle_namespace_attrs (tree ns, tree attributes) DECL_ATTRIBUTES (ns) = tree_cons (name, args, DECL_ATTRIBUTES (ns)); } + else if (is_attribute_p ("diagnose_as", name)) + { + if (!args || TREE_CHAIN (args) + || TREE_CODE (TREE_VALUE (args)) != STRING_CST) + { + warning (OPT_Wattributes, + "%qD attribute requires a single NTBS argument", + name); + continue; + } + tree existing + = lookup_attribute ("diagnose_as", DECL_ATTRIBUTES (ns)); + if (existing + && !cp_tree_equal (TREE_VALUE (args), + TREE_VALUE (TREE_VALUE (existing)))) + { + auto_diagnostic_group d; + warning (OPT_Wattributes, "%qD redeclared with different %qD " + "attribute value", ns, name); + inform (DECL_SOURCE_LOCATION (ns), "previous declaration here"); + continue; + } + DECL_ATTRIBUTES (ns) = tree_cons (name, args, + DECL_ATTRIBUTES (ns)); + } else { warning (OPT_Wattributes, "%qD attribute directive ignored", diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index a8bfd5fc053..251f6db4055 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -46,6 +46,7 @@ static tree verify_stmt_tree_r (tree *, int *, void *); static tree handle_init_priority_attribute (tree *, tree, tree, int, bool *); static tree handle_abi_tag_attribute (tree *, tree, tree, int, bool *); +static tree handle_diagnose_as_attribute (tree *, tree, tree, int, bool *); /* If REF is an lvalue, returns the kind of lvalue that REF is. Otherwise, returns clk_none. */ @@ -4860,6 +4861,8 @@ const struct attribute_spec cxx_attribute_table[] = handle_init_priority_attribute, NULL }, { "abi_tag", 1, -1, false, false, false, true, handle_abi_tag_attribute, NULL }, + { "diagnose_as", 1, 1, false, false, false, false, + handle_diagnose_as_attribute, NULL }, { NULL, 0, 0, false, false, false, false, NULL, NULL } }; @@ -5128,6 +5131,119 @@ handle_abi_tag_attribute (tree* node, tree name, tree args, return NULL_TREE; } +static bool +check_diagnose_as_redeclaration (const_tree decl, const_tree name, + const_tree old, const_tree new_) +{ + if (!old) + return true; + if (TREE_CODE (TREE_VALUE (old)) == TREE_LIST) + old = TREE_VALUE (old); + if (TREE_CODE (TREE_VALUE (new_)) == TREE_LIST) + new_ = TREE_VALUE (new_); + tree old_value = TREE_VALUE (old); + tree new_value = TREE_VALUE (new_); + if (cp_tree_equal (old_value, new_value)) + return true; + warning (OPT_Wattributes, "%qD redeclared with %<%D(%E)%> " + "attribute", decl, name, new_value); + inform (DECL_SOURCE_LOCATION (decl), "previous declaration here"); + return false; +} + +static tree +handle_diagnose_as_attribute (tree* node, tree name, tree args, + int flags, bool* no_add_attrs) +{ + tree decl = NULL_TREE; + if (!args || TREE_CHAIN (args) + || TREE_CODE (TREE_VALUE (args)) != STRING_CST) + { + warning (OPT_Wattributes, + "%qD attribute requires a single NTBS argument", + name); + goto fail; + } + 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); + } + } + } + else if (TYPE_P (*node)) + { + if (!OVERLOAD_TYPE_P (*node)) + { + error ("%qE attribute applied to non-class, non-enum type %qT", + name, *node); + goto fail; + } + else if (!(flags & (int)ATTR_FLAG_TYPE_IN_PLACE)) + { + error ("%qE attribute applied to %qT after its definition", + name, *node); + goto fail; + } + decl = TYPE_NAME (*node); + } + else + { + if (!VAR_OR_FUNCTION_DECL_P (*node)) + { + error ("%qE attribute applied to non-function, non-variable %qD", + name, *node); + goto fail; + } + else if (DECL_LANGUAGE (*node) == lang_c) + { + error ("%qE attribute applied to %<extern \"C\"%> declaration %qD", + name, *node); + goto fail; + } + decl = *node; + } + + // Make sure all declarations have the same diagnose_as string. + if (DECL_SOURCE_LOCATION (decl) != input_location) + { + tree attributes = TYPE_P (*node) ? TYPE_ATTRIBUTES (*node) + : DECL_ATTRIBUTES (decl); + if (!check_diagnose_as_redeclaration ( + decl, name, lookup_attribute ("diagnose_as", attributes), args)) + goto fail; + } + else if (CLASS_TYPE_P (*node) && CLASSTYPE_IMPLICIT_INSTANTIATION (*node)) + { + // The above branch (different source location) is taken for declarations + // of type aliases that modify an implicit template specialization on the + // RHS. This branch is taken when the template is instantiated via + // instantiate_class_template_1, in which case the attribute either + // already has the value from the general template or from a type alias. + goto fail; + } + + return NULL_TREE; + + fail: + *no_add_attrs = true; + return NULL_TREE; +} + /* Return a new PTRMEM_CST of the indicated TYPE. The MEMBER is the thing pointed to by the constant. */ diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index c8caf36f293..da8928fc667 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -2865,6 +2865,46 @@ types (@pxref{Variable Attributes}, @pxref{Type Attributes}.) The message attached to the attribute is affected by the setting of the @option{-fmessage-length} option. +@item diagnose_as ("@var{string}") +@cindex @code{diagnose_as} function attribute +The @code{diagnose_as} attribute modifies how the entity the attribute +appertains to is diagnosed in compiler messages and @code{__func__}, +@code{__FUNCTION__}, and @code{__PRETTY_FUNCTION__} macros. (The result of +@code{typeid} is not affected.) If the attribute is applied to a +@code{namespace}, the specified string replaces the complete enclosing scope. +The effect of the @code{diagnose_as} attribute can be disabled with the +@option{-fno-diagnostics-use-aliases} command line option. + +@smallexample +namespace Foo @{ + namespace Bar @{ + inline namespace v1 [[gnu::diagnose_as("Foobar")]] @{ + int f() @{ + // __PRETTY_FUNCTION__ == "void Foobar::f()" + @} + @} + @} +@} +// In function 'int Foobar::f()': +// warning: no return statement in function returning non-void [-Wreturn-type] +@end smallexample + +The @code{diagnose_as} attribute can be used with namespaces, functions, +variables, alias declarations (but not alias templates), and user-defined types +(classes, unions, and enums). If the alias declaration aliases an implicit class +template specialization (before instantiation), the attribute is additionally +applied to the class template specialization. Whether the attribute has an +effect on partial template specializations is unspecified and may depend on +several different factors. + +@smallexample +template <class T> struct basic_string @{@}; +using string [[gnu::diagnose_as("string")]] = basic_string<char>; +int f(basic_string<char>) @{@} +// In function 'int f(string)': +// warning: no return statement in function returning non-void [-Wreturn-type] +@end smallexample + @item error ("@var{message}") @itemx warning ("@var{message}") @cindex @code{error} function attribute diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index f5a9dc33819..84c19344c89 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -311,7 +311,8 @@ Objective-C and Objective-C++ Dialects}. -fdiagnostics-show-path-depths @gol -fno-show-column @gol -fdiagnostics-column-unit=@r{[}display@r{|}byte@r{]} @gol --fdiagnostics-column-origin=@var{origin}} +-fdiagnostics-column-origin=@var{origin} @gol +-fno-diagnostics-aliases} @item Warning Options @xref{Warning Options,,Options to Request or Suppress Warnings}. @@ -5077,6 +5078,12 @@ first column. The default value of 1 corresponds to traditional GCC behavior and to the GNU style guide. Some utilities may perform better with an origin of 0; any non-negative value may be specified. +@item -fno-diagnostics-use-aliases +@opindex fno-diagnostics-use-aliases +@opindex fdiagnostics-use-aliases +Do not replace identifiers or scope names with the aliases defined with the +@code{[[gnu::diagnose_as("alias")]]} attribute. + @item -fdiagnostics-format=@var{FORMAT} @opindex fdiagnostics-format Select a different format for printing diagnostics. diff --git a/gcc/testsuite/g++.dg/diagnostic/diagnose-as1.C b/gcc/testsuite/g++.dg/diagnostic/diagnose-as1.C new file mode 100644 index 00000000000..ca9db2b3a5c --- /dev/null +++ b/gcc/testsuite/g++.dg/diagnostic/diagnose-as1.C @@ -0,0 +1,177 @@ +// { dg-options "-fdiagnostics-use-aliases -fpretty-templates" } + +#ifdef __cpp_constexpr +template <typename T> + constexpr bool is_char() { return false; } + +template <> + constexpr bool is_char<char>() { return true; } + +template <typename T> + constexpr bool is_int() { return false; } + +template <> + constexpr bool is_int<int>() { return true; } +#endif + +namespace foo0 __attribute__((diagnose_as())) {} // { dg-warning "'diagnose_as' attribute requires a single NTBS argument" } + +namespace foo1 __attribute__((diagnose_as("foo2"))) {} // { dg-line foo1 } +namespace foo1 __attribute__((diagnose_as("XXXX"))) {} // { dg-warning "'foo1' redeclared with different 'diagnose_as' attribute value" } +// { dg-message "previous declaration here" "" { target *-*-* } foo1 } +namespace foo1 +{ + void f() { + f(1); // { dg-error "too many arguments to function 'void foo2::f\\(\\)'" } + } + + class __attribute__((diagnose_as("XX"))) X; // { dg-line XX } + class __attribute__((diagnose_as("--"))) X { // { dg-warning "'class foo2::XX' redeclared with 'diagnose_as\\(\"--\"\\)' attribute" } + // { dg-message "previous declaration here" "" { target *-*-* } XX } + __attribute__((diagnose_as("ff"))) void f(); + }; + class Y : X + { + void g() { + f(); // { dg-error "'void foo2::XX::ff\\(\\)' is private within this context" } + } + }; + + class __attribute__((diagnose_as())) Z0; // { dg-error "wrong number of arguments specified for 'diagnose_as' attribute" } + class __attribute__((diagnose_as(1))) Z1; // { dg-warning "'diagnose_as' attribute requires a single NTBS argument" } + class __attribute__((diagnose_as("a", "b"))) Z2; // { dg-error "wrong number of arguments specified for 'diagnose_as' attribute" } + + template <typename T> class A0 {}; + typedef A0<char> Achar __attribute__((diagnose_as("Achar"))); + template class A0<int>; + typedef A0<int> Aint __attribute__((diagnose_as("Aint"))); // { dg-warning "ignoring 'diagnose_as' attribute applied to template 'foo2::A0<int>' after instantiation" } +} + +namespace A +{ + inline namespace B __attribute__((diagnose_as("@1"))) + { + template <typename U> + struct __attribute__((diagnose_as("@2"))) C + { + __attribute__((diagnose_as("fun:1"))) + void f() {} // { dg-line ABC_f } + + template <typename T> + __attribute__((diagnose_as("fun:2"))) + void g(T, U) {} // { dg-line ABC_gT } + + template <typename T> + __attribute__((diagnose_as("fun:2.2"))) + void g() {} // { dg-line ABC_g } + + __attribute__((diagnose_as("fun:3"))) + void h() + { +#ifdef __cpp_constexpr + static_assert(__builtin_strcmp(__FUNCTION__, "fun:3") == 0, ""); + static_assert(__builtin_strcmp(__func__, "fun:3") == 0, ""); + constexpr const char* ref + = is_int<U>() ? "void @1::@3::fun:3()" + : "void @1::@2<U>::fun:3() [with U = char]"; + static_assert(__builtin_strcmp(__PRETTY_FUNCTION__, ref) == 0, ""); +#endif + } + + template <typename T> + __attribute__((diagnose_as("fun:4"))) + void ht() + { +#ifdef __cpp_constexpr + static_assert(__builtin_strcmp(__FUNCTION__, "fun:4") == 0, ""); + constexpr const char* ref + = is_int<U>() + ? "void @1::@3::fun:4<T>() [with T = float]" + : "void @1::@2<U>::fun:4<T>() [with T = float; U = char]"; + static_assert(__builtin_strcmp(__PRETTY_FUNCTION__, ref) == 0, ""); +#endif + } + + template <typename T0, typename T1> + struct __attribute__((diagnose_as("@5"))) E + { + __attribute__((diagnose_as("fun:5"))) static void f() { +#ifdef __cpp_constexpr + static_assert(__builtin_strcmp(__FUNCTION__, "fun:5") == 0, ""); + constexpr const char* ref = is_int<U>() + ? is_char<T0>() + ? "static void @1::@3::@6::fun:5()" + : "static void @1::@3::@5<T0, T1>::fun:5() [with T0 = float; T1 = short int]" + : is_char<T0>() + ? "static void @1::@2<U>::@6::fun:5() [with U = char]" + : "static void @1::@2<U>::@5<T0, T1>::fun:5() [with T0 = float; T1 = short int; U = char]"; + static_assert(__builtin_strcmp(__PRETTY_FUNCTION__, ref) == 0, + ""); +#endif + } + }; + typedef E<char, char> F __attribute__((diagnose_as("@6"))); + + template <typename T> + struct __attribute__((diagnose_as("@7"))) E<T, long> + { + __attribute__((diagnose_as("fun:6"))) static void f() { +#ifdef __cpp_constexpr + static_assert(__builtin_strcmp(__FUNCTION__, "fun:6") == 0, ""); + constexpr const char* ref = is_int<U>() + ? "static void @1::@3::@7<T, long int>::fun:6() [with T = float]" + : "static void @1::@2<U>::@7<T, long int>::fun:6() [with T = float; U = char]"; + static_assert(__builtin_strcmp(__PRETTY_FUNCTION__, ref) == 0, + ""); +#endif + } + }; + }; + + typedef C<int> D __attribute__((diagnose_as("@3"))); + + template <> + struct __attribute__((diagnose_as("@4"))) C<float> + { + }; + } +} + +void fn_1(int); +void fn_2(A::D); + +int main () +{ + fn_2 (A::D ()); + fn_1 (A::D ()); // { dg-error "cannot convert '@1::@3' to 'int'" } + fn_1 (A::C<int> ()); // { dg-error "cannot convert '@1::@3' to 'int'" } + fn_1 (A::C<char> ()); // { dg-error "cannot convert '@1::@2<char>' to 'int'" } + fn_1 (A::C<float> ()); // { dg-error "cannot convert '@1::@4' to 'int'" } + + A::C<int>().f(0); // { dg-error "no matching function for call to '@1::@3::f\\(int\\)'" } + // { dg-message "candidate: 'void @1::@3::fun:1\\(\\)'" "" { target *-*-* } ABC_f } + + A::C<char>().f(0); // { dg-error "no matching function for call to '@1::@2<char>::f\\(int\\)'" } + // { dg-message "candidate: 'void @1::@2<U>::fun:1\\(\\) \\\[with U = char\\\]'" "" { target *-*-* } ABC_f } + + A::C<float>().f(0); // { dg-error "'struct @1::@4' has no member named 'f'" } + + A::C<int>().g(); // { dg-error "no matching function for call to '@1::@3::g\\(\\)'" } + // { dg-message "candidate: 'template<class T> void @1::@3::fun:2\\(T, U\\)'" "" { target *-*-* } ABC_gT } + // { dg-message "candidate: 'template<class T> void @1::@3::fun:2.2\\(\\)'" "" { target *-*-* } ABC_g } + + A::C<char>().g(); // { dg-error "no matching function for call to '@1::@2<char>::g\\(\\)'" } + // { dg-message "candidate: 'template<class T> void @1::@2<U>::fun:2\\(T, U\\) \\\[with U = char\\\]'" "" { target *-*-* } ABC_gT } + // { dg-message "candidate: 'template<class T> void @1::@2<U>::fun:2.2\\(\\) \\\[with U = char\\\]'" "" { target *-*-* } ABC_g } + + A::C<int>().h(); + A::C<char>().h(); + A::C<int>().ht<float>(); + A::C<char>().ht<float>(); + A::C<int>::E<float, short>::f(); + A::C<char>::E<float, short>::f(); + A::C<int>::E<float, long>::f(); + A::C<char>::E<float, long>::f(); + A::C<int>::F::f(); + A::C<char>::F::f(); +} diff --git a/gcc/testsuite/g++.dg/diagnostic/diagnose-as2.C b/gcc/testsuite/g++.dg/diagnostic/diagnose-as2.C new file mode 100644 index 00000000000..b1d46d12024 --- /dev/null +++ b/gcc/testsuite/g++.dg/diagnostic/diagnose-as2.C @@ -0,0 +1,144 @@ +// { dg-options "-fdiagnostics-use-aliases -fno-pretty-templates" } + +#ifdef __cpp_constexpr +template <typename T> + constexpr bool is_char() { return false; } + +template <> + constexpr bool is_char<char>() { return true; } + +template <typename T> + constexpr bool is_int() { return false; } + +template <> + constexpr bool is_int<int>() { return true; } +#endif + +namespace A +{ + inline namespace B __attribute__((diagnose_as("@1"))) + { + template <typename U> + struct __attribute__((diagnose_as("@2"))) C + { + __attribute__((diagnose_as("fun:1"))) + void f() {} // { dg-line ABC_f } + + template <typename T> + __attribute__((diagnose_as("fun:2"))) + void g(T, U) {} // { dg-line ABC_gT } + + template <typename T> + __attribute__((diagnose_as("fun:2.2"))) + void g() {} // { dg-line ABC_g } + + __attribute__((diagnose_as("fun:3"))) + void h() + { +#ifdef __cpp_constexpr + static_assert(__builtin_strcmp(__FUNCTION__, "fun:3") == 0, ""); + constexpr const char* ref + = is_int<U>() ? "void @1::@3::fun:3()" + : "void @1::@2<char>::fun:3()"; + static_assert(__builtin_strcmp(__PRETTY_FUNCTION__, ref) == 0, ""); +#endif + } + + template <typename T> + __attribute__((diagnose_as("fun:4"))) + void ht() + { +#ifdef __cpp_constexpr + static_assert(__builtin_strcmp(__FUNCTION__, "fun:4") == 0, ""); + constexpr const char* ref + = is_int<U>() ? "void @1::@3::fun:4<float>()" + : "void @1::@2<char>::fun:4<float>()"; + static_assert(__builtin_strcmp(__PRETTY_FUNCTION__, ref) == 0, ""); +#endif + } + + template <typename T0, typename T1> + struct __attribute__((diagnose_as("@5"))) E + { + __attribute__((diagnose_as("fun:5"))) static void f() { +#ifdef __cpp_constexpr + static_assert(__builtin_strcmp(__FUNCTION__, "fun:5") == 0, ""); + constexpr const char* ref = is_int<U>() + ? is_char<T0>() + ? "static void @1::@3::@6::fun:5()" + : "static void @1::@3::@5<float, short int>::fun:5()" + : is_char<T0>() + ? "static void @1::@2<char>::@6::fun:5()" + : "static void @1::@2<char>::@5<float, short int>::fun:5()"; + static_assert(__builtin_strcmp(__PRETTY_FUNCTION__, ref) == 0, + ""); +#endif + } + }; + typedef E<char, char> F __attribute__((diagnose_as("@6"))); + + template <typename T> + struct __attribute__((diagnose_as("@7"))) E<T, long> + { + __attribute__((diagnose_as("fun:6"))) static void f() { +#ifdef __cpp_constexpr + static_assert(__builtin_strcmp(__FUNCTION__, "fun:6") == 0, ""); + // Note that @7 is ignored with -fno-pretty-templates. After all + // diagnose_as on partial specializations is not supported. + constexpr const char* ref = is_int<U>() + ? "static void @1::@3::@5<float, long int>::fun:6()" + : "static void @1::@2<char>::@5<float, long int>::fun:6()"; + static_assert(__builtin_strcmp(__PRETTY_FUNCTION__, ref) == 0, + ""); +#endif + } + }; + }; + + typedef C<int> D __attribute__((diagnose_as("@3"))); + + template <> + struct __attribute__((diagnose_as("@4"))) C<float> + { + }; + } +} + +void fn_1(int); +void fn_2(A::D); + +int main () +{ + fn_2 (A::D ()); + fn_1 (A::D ()); // { dg-error "cannot convert '@1::@3' to 'int'" } + fn_1 (A::C<int> ()); // { dg-error "cannot convert '@1::@3' to 'int'" } + fn_1 (A::C<char> ()); // { dg-error "cannot convert '@1::@2<char>' to 'int'" } + fn_1 (A::C<float> ()); // { dg-error "cannot convert '@1::@4' to 'int'" } + + A::C<int>().f(0); // { dg-error "no matching function for call to '@1::@3::f\\(int\\)'" } + // { dg-message "candidate: 'void @1::@3::fun:1\\(\\)'" "" { target *-*-* } ABC_f } + + A::C<char>().f(0); // { dg-error "no matching function for call to '@1::@2<char>::f\\(int\\)'" } + // { dg-message "candidate: 'void @1::@2<char>::fun:1\\(\\)'" "" { target *-*-* } ABC_f } + + A::C<float>().f(0); // { dg-error "'struct @1::@4' has no member named 'f'" } + + A::C<int>().g(); // { dg-error "no matching function for call to '@1::@3::g\\(\\)'" } + // { dg-message "candidate: 'template<class T> void @1::@3::fun:2\\(T, int\\)'" "" { target *-*-* } ABC_gT } + // { dg-message "candidate: 'template<class T> void @1::@3::fun:2.2\\(\\)'" "" { target *-*-* } ABC_g } + + A::C<char>().g(); // { dg-error "no matching function for call to '@1::@2<char>::g\\(\\)'" } + // { dg-message "candidate: 'template<class T> void @1::@2<char>::fun:2\\(T, char\\)'" "" { target *-*-* } ABC_gT } + // { dg-message "candidate: 'template<class T> void @1::@2<char>::fun:2.2\\(\\)'" "" { target *-*-* } ABC_g } + + A::C<int>().h(); + A::C<char>().h(); + A::C<int>().ht<float>(); + A::C<char>().ht<float>(); + A::C<int>::E<float, short>::f(); + A::C<char>::E<float, short>::f(); + A::C<int>::E<float, long>::f(); + A::C<char>::E<float, long>::f(); + A::C<int>::F::f(); + A::C<char>::F::f(); +} diff --git a/gcc/testsuite/g++.dg/diagnostic/diagnose-as3.C b/gcc/testsuite/g++.dg/diagnostic/diagnose-as3.C new file mode 100644 index 00000000000..beedb9624a8 --- /dev/null +++ b/gcc/testsuite/g++.dg/diagnostic/diagnose-as3.C @@ -0,0 +1,152 @@ +// { dg-options "-fdiagnostics-use-aliases -fpretty-templates" } +// { dg-do compile { target c++11 } } + +const char* volatile s; + +namespace foo +{ + template <class U> + struct [[gnu::diagnose_as("Bar'")]] Bar + { + template <class T0, class T1> + struct [[gnu::diagnose_as("A'")]] A + { constexpr const char* f() { return __PRETTY_FUNCTION__; } }; + + template <class T> + struct A<T, float> + { constexpr const char* f() { return __PRETTY_FUNCTION__; } }; + + template <class T> + struct [[gnu::diagnose_as("Adouble")]] A<T, double> + { constexpr const char* f() { return __PRETTY_FUNCTION__; } }; + + using Special [[gnu::diagnose_as("SpecialA")]] = A<int, int>; + + static constexpr const char* f() { return __PRETTY_FUNCTION__; } + }; + + template <class P> + struct + [[gnu::diagnose_as("BarPtr")]] + Bar<P*> + { + template <class U = int> + static constexpr const char* f() { return __PRETTY_FUNCTION__; } + }; + + template <> + struct [[gnu::diagnose_as("SpecialBar")]] Bar<void> + { + static constexpr const char* f() { return __PRETTY_FUNCTION__; } + }; + + using barchar [[gnu::diagnose_as("barchar")]] = Bar<char>; +} + +namespace A +{ + inline namespace B __attribute__((diagnose_as("@1"))) + { + template <typename U> + struct __attribute__((diagnose_as("@2"))) C + { + template <class T> + __attribute__((diagnose_as("fun:2"))) + constexpr const char* g() { return __PRETTY_FUNCTION__; } + + __attribute__((diagnose_as("fun:3"))) + constexpr const char* h() + { return __PRETTY_FUNCTION__; } + }; + + typedef C<int> D __attribute__((diagnose_as("@3"))); + } +} + +template <class T0, class U0> +struct [[gnu::diagnose_as("X.0")]] X0 +{ + template <class T1, class U1> + struct X1 + { + template <class T2, class U2> + struct X2 + { + template <class T3, class U3> + [[gnu::diagnose_as("f-1")]] + static constexpr const char* f() { + return __PRETTY_FUNCTION__; + } + }; + + using XX [[gnu::diagnose_as("X2'")]] = X2<int, long>; + }; + + template <class T3, class U3> + [[gnu::diagnose_as("f-1")]] + static constexpr const char* f() { + return __PRETTY_FUNCTION__; + } + + struct [[gnu::diagnose_as("X.3")]] X3 + { + template <class T3, class U3> + [[gnu::diagnose_as("f-1")]] + static constexpr const char* f() { + return __PRETTY_FUNCTION__; + } + }; +}; + +using X0int [[gnu::diagnose_as("[int|int]")]] = X0<int, int>; + +#define TEST(expected, fun) \ + static_assert(__builtin_strcmp(expected, fun) == 0, "") + + +void h() { + TEST("constexpr const char* @1::@3::fun:3()", + A::C<int>().h()); + TEST("constexpr const char* @1::@3::fun:2<T>() [with T = float]", + A::C<int>().g<float>()); + TEST("constexpr const char* @1::@2<U>::fun:3() [with U = char]", + A::C<char>().h()); + TEST("constexpr const char* @1::@2<U>::fun:2<T>() [with T = float; U = char]", + A::C<char>().g<float>()); + TEST("constexpr const char* foo::barchar::A'<T0, T1>::f() [with T0 = char; T1 = char]", + (foo::Bar<char>::A<char, char>().f())); + TEST("constexpr const char* foo::barchar::A'<T, float>::f() [with T = char]", + (foo::Bar<char>::A<char, float>().f())); + TEST("constexpr const char* foo::barchar::Adouble<T, double>::f() [with T = int]", + (foo::Bar<char>::A<int, double>().f())); + TEST("constexpr const char* foo::barchar::SpecialA::f()", + (foo::Bar<char>::A<int, int>().f())); + TEST("constexpr const char* foo::Bar'<U>::A'<T0, T1>::f() [with T0 = char; T1 = char; U = float]", + (foo::Bar<float>::A<char, char>().f())); + TEST("constexpr const char* foo::Bar'<U>::A'<T, float>::f() [with T = char; U = float]", + (foo::Bar<float>::A<char, float>().f())); + TEST("constexpr const char* foo::Bar'<U>::Adouble<T, double>::f() [with T = char; U = float]", + (foo::Bar<float>::A<char, double>().f())); + TEST("constexpr const char* foo::Bar'<U>::SpecialA::f() [with U = float]", + (foo::Bar<float>::A<int, int>().f())); + TEST("static constexpr const char* foo::barchar::f()", + foo::Bar<char>::f()); + TEST("static constexpr const char* foo::BarPtr<P*>::f<U>() [with U = int; P = char]", + foo::Bar<char*>::f()); + TEST("static constexpr const char* foo::BarPtr<P*>::f<U>() [with U = int; P = float]", + foo::Bar<float*>::f()); + TEST("static constexpr const char* foo::Bar'<U>::f() [with U = float]", + foo::Bar<float>::f()); + TEST("static constexpr const char* foo::SpecialBar::f()", + foo::Bar<void>::f()); + TEST("static constexpr const char* X.0<T0, U0>::X1<T1, U1>::X2'::f-1<T3, U3>() [with T3 = long int; U3 = long long int; T1 = short int; U1 = int; T0 = char; U0 = short int]", + (X0<char, short>::X1<short, int>::X2<int, long>::f<long, long long>())); + TEST("static constexpr const char* X.0<T0, U0>::X1<T1, U1>::X2<T2, U2>::f-1<T3, U3>() [with T3 = long int; U3 = long long int; T2 = long int; U2 = int; T1 = short int; U1 = int; T0 = char; U0 = short int]", + (X0<char, short>::X1<short, int>::X2<long, int>::f<long, long long>())); + TEST("static constexpr const char* X.0<T0, U0>::X.3::f-1<T3, U3>() [with T3 = long int; U3 = long long int; T0 = char; U0 = short int]", + (X0<char, short>::X3::f<long, long long>())); + TEST("static constexpr const char* [int|int]::f-1<T3, U3>() [with T3 = long int; U3 = long long int]", + (X0<int, int>::f<long, long long>())); + TEST("static constexpr const char* [int|int]::X.3::f-1<T3, U3>() [with T3 = long int; U3 = long long int]", + (X0<int, int>::X3::f<long, long long>())); +} diff --git a/gcc/testsuite/g++.dg/diagnostic/diagnose-as4.C b/gcc/testsuite/g++.dg/diagnostic/diagnose-as4.C new file mode 100644 index 00000000000..89b800c7b9d --- /dev/null +++ b/gcc/testsuite/g++.dg/diagnostic/diagnose-as4.C @@ -0,0 +1,158 @@ +// { dg-options "-fdiagnostics-use-aliases -fno-pretty-templates" } +// { dg-do compile { target c++11 } } + +const char* volatile s; + +namespace foo +{ + template <class U> + struct [[gnu::diagnose_as("Bar'")]] Bar + { + template <class T0, class T1> + struct [[gnu::diagnose_as("A'")]] A + { constexpr const char* f() { return __PRETTY_FUNCTION__; } }; + + template <class T> + struct A<T, float> + { constexpr const char* f() { return __PRETTY_FUNCTION__; } }; + + template <class T> + struct [[gnu::diagnose_as("Adouble")]] A<T, double> + { constexpr const char* f() { return __PRETTY_FUNCTION__; } }; + + using Special [[gnu::diagnose_as("SpecialA")]] = A<int, int>; + + static constexpr const char* f() { return __PRETTY_FUNCTION__; } + }; + + template <class P> + struct + [[gnu::diagnose_as("BarPtr")]] + Bar<P*> + { + template <class U = int> + static constexpr const char* f() { return __PRETTY_FUNCTION__; } + }; + + template <> + struct [[gnu::diagnose_as("SpecialBar")]] Bar<void> + { + static constexpr const char* f() { return __PRETTY_FUNCTION__; } + }; + + using barchar [[gnu::diagnose_as("barchar")]] = Bar<char>; +} + +namespace A +{ + inline namespace B __attribute__((diagnose_as("@1"))) + { + template <typename U> + struct __attribute__((diagnose_as("@2"))) C + { + template <class T> + __attribute__((diagnose_as("fun:2"))) + constexpr const char* g() { return __PRETTY_FUNCTION__; } + + __attribute__((diagnose_as("fun:3"))) + constexpr const char* h() + { return __PRETTY_FUNCTION__; } + }; + + typedef C<int> D __attribute__((diagnose_as("@3"))); + } +} + +template <class T0, class U0> +struct [[gnu::diagnose_as("XX0")]] X0 +{ + template <class T1, class U1> + struct X1 + { + template <class T2, class U2> + struct X2 + { + template <class T3, class U3> + [[gnu::diagnose_as("f-1")]] + static constexpr const char* f() { + return __PRETTY_FUNCTION__; + } + }; + + using XX [[gnu::diagnose_as("X2'")]] = X2<int, long>; + }; + + template <class T3, class U3> + [[gnu::diagnose_as("f-2")]] + static constexpr const char* f() { + return __PRETTY_FUNCTION__; + } + + struct [[gnu::diagnose_as("XX3")]] X3 + { + template <class T3, class U3> + [[gnu::diagnose_as("f-3")]] + static constexpr const char* f() { // { dg-line X0_X3_f } + return __PRETTY_FUNCTION__; + } + }; +}; + +using X0int [[gnu::diagnose_as("X0intint")]] = X0<int, int>; + +#define TEST(expected, fun) \ + static_assert(__builtin_strcmp(expected, fun) == 0, "") + + +void h() { + TEST("constexpr const char* @1::@3::fun:3()", + A::C<int>().h()); + TEST("constexpr const char* @1::@3::fun:2<float>()", + A::C<int>().g<float>()); + TEST("constexpr const char* @1::@2<char>::fun:3()", + A::C<char>().h()); + TEST("constexpr const char* @1::@2<char>::fun:2<float>()", + A::C<char>().g<float>()); + TEST("constexpr const char* foo::barchar::A'<char, char>::f()", + (foo::Bar<char>::A<char, char>().f())); + TEST("constexpr const char* foo::barchar::A'<char, float>::f()", + (foo::Bar<char>::A<char, float>().f())); + TEST("constexpr const char* foo::barchar::A'<int, double>::f()", + (foo::Bar<char>::A<int, double>().f())); + TEST("constexpr const char* foo::barchar::SpecialA::f()", + (foo::Bar<char>::A<int, int>().f())); + TEST("constexpr const char* foo::Bar'<float>::A'<char, char>::f()", + (foo::Bar<float>::A<char, char>().f())); + TEST("constexpr const char* foo::Bar'<float>::A'<char, float>::f()", + (foo::Bar<float>::A<char, float>().f())); + TEST("constexpr const char* foo::Bar'<float>::A'<char, double>::f()", + (foo::Bar<float>::A<char, double>().f())); + TEST("constexpr const char* foo::Bar'<float>::SpecialA::f()", + (foo::Bar<float>::A<int, int>().f())); + TEST("static constexpr const char* foo::barchar::f()", + foo::Bar<char>::f()); + TEST("static constexpr const char* foo::Bar'<char*>::f<int>()", + foo::Bar<char*>::f()); + TEST("static constexpr const char* foo::Bar'<float*>::f<int>()", + foo::Bar<float*>::f()); + TEST("static constexpr const char* foo::Bar'<float>::f()", + foo::Bar<float>::f()); + TEST("static constexpr const char* foo::SpecialBar::f()", + foo::Bar<void>::f()); + TEST("static constexpr const char* XX0<char, short int>::X1<short int, int>::X2'::f-1<long int, long long int>()", + (X0<char, short>::X1<short, int>::X2<int, long>::f<long, long long>())); + TEST("static constexpr const char* XX0<char, short int>::X1<short int, int>::X2<long int, int>::f-1<long int, long long int>()", + (X0<char, short>::X1<short, int>::X2<long, int>::f<long, long long>())); + TEST("static constexpr const char* XX0<char, short int>::XX3::f-3<long int, long long int>()", + (X0<char, short>::X3::f<long, long long>())); + TEST("static constexpr const char* X0intint::f-2<long int, long long int>()", + (X0<int, int>::f<long, long long>())); + TEST("static constexpr const char* X0intint::XX3::f-3<long int, long long int>()", + (X0<int, int>::X3::f<long, long long>())); + + X0<char, short>::X3::f<long, long long>(1); // { dg-error "no matching function for call to 'XX0<char, short int>::XX3::f<long int, long long int>\\(int\\)" } + // { dg-message "candidate: 'static constexpr const char\\* XX0<char, short int>::XX3::f-3<long int, long long int>\\(\\)'" "" { target *-*-* } X0_X3_f } + + X0<int, int>::X3::f<long, long long>(1); // { dg-error "no matching function for call to 'X0intint::XX3::f<long int, long long int>\\(int\\)" } + // { dg-message "candidate: 'static constexpr const char\\* X0intint::XX3::f-3<long int, long long int>\\(\\)'" "" { target *-*-* } X0_X3_f } +} ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-05-04 11:13 ` [PATCH] Add gnu::diagnose_as attribute Matthias Kretz 2021-05-04 13:34 ` David Malcolm 2021-05-14 16:05 ` Martin Sebor @ 2021-05-27 17:39 ` Jason Merrill 2021-05-27 18:54 ` Matthias Kretz 2 siblings, 1 reply; 30+ messages in thread From: Jason Merrill @ 2021-05-27 17:39 UTC (permalink / raw) To: Matthias Kretz, gcc-patches; +Cc: libstdc++ On 5/4/21 7:13 AM, Matthias Kretz wrote: > 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. > 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? > 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? Jason ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-05-27 17:39 ` Jason Merrill @ 2021-05-27 18:54 ` Matthias Kretz 2021-05-27 21:15 ` Jason Merrill 0 siblings, 1 reply; 30+ messages in thread From: Matthias Kretz @ 2021-05-27 18:54 UTC (permalink / raw) To: gcc-patches, Jason Merrill; +Cc: libstdc++ 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 <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. > > > > 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' > > 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"))) { } } 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")]]; #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); + } + } + } -- ────────────────────────────────────────────────────────────────────────── 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 ────────────────────────────────────────────────────────────────────────── ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-05-27 18:54 ` Matthias Kretz @ 2021-05-27 21:15 ` Jason Merrill 2021-05-27 22:07 ` Matthias Kretz 0 siblings, 1 reply; 30+ messages in thread From: Jason Merrill @ 2021-05-27 21:15 UTC (permalink / raw) To: Matthias Kretz, gcc-patches; +Cc: libstdc++ 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 <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. >>> >>> 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-05-27 21:15 ` Jason Merrill @ 2021-05-27 22:07 ` Matthias Kretz 2021-05-28 3:05 ` Jason Merrill 0 siblings, 1 reply; 30+ messages in thread From: Matthias Kretz @ 2021-05-27 22:07 UTC (permalink / raw) To: gcc-patches, Jason Merrill; +Cc: libstdc++ 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 ────────────────────────────────────────────────────────────────────────── ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-05-27 22:07 ` Matthias Kretz @ 2021-05-28 3:05 ` Jason Merrill 2021-05-28 7:42 ` Matthias Kretz 0 siblings, 1 reply; 30+ messages in thread From: Jason Merrill @ 2021-05-28 3:05 UTC (permalink / raw) To: Matthias Kretz, gcc-patches; +Cc: libstdc++ 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: >>> 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. Only briefly puzzling, I think; Vir::A is a valid way of referring to both types. >>> 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. 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. >>> - 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. 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. > 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. > 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. Well, in your sample code, _Scalar isn't complete at the point of the alias-declaration. But even if it were, since the attribute doesn't affect code generation, it doesn't strike me as a big problem. Still, perhaps it would be better to store these aliases in a separate hash table instead of *_ATTRIBUTES. Jason ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-05-28 3:05 ` Jason Merrill @ 2021-05-28 7:42 ` Matthias Kretz 2021-06-01 19:12 ` Jason Merrill 0 siblings, 1 reply; 30+ messages in thread From: Matthias Kretz @ 2021-05-28 7:42 UTC (permalink / raw) To: gcc-patches, Jason Merrill; +Cc: libstdc++ 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 ────────────────────────────────────────────────────────────────────────── ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-05-28 7:42 ` Matthias Kretz @ 2021-06-01 19:12 ` Jason Merrill 2021-06-01 21:01 ` Matthias Kretz 2021-06-11 10:01 ` Matthias Kretz 0 siblings, 2 replies; 30+ messages in thread From: Jason Merrill @ 2021-06-01 19:12 UTC (permalink / raw) To: Matthias Kretz, gcc-patches; +Cc: libstdc++, David Malcolm, Jonathan Wakely On 5/28/21 3:42 AM, Matthias Kretz wrote: > 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. This is the basic disconnect: I think that these goals are contradictory, and that replacement names that are not valid identifiers will just confuse users that don't know about them. If a user sees stdx::foo in a diagnostic and then tries to refer to stdx::foo and gets an error, the diagnostic is not more helpful than one that uses the fully qualified name. Jonathan, David, any thoughts on this issue? > 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.) Wouldn't it be better to print stdx::native_simd<float> if that's how the users write the type? >>> 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? I was thinking for not messing with the type after it's defined, but there are other things that do that (such as lazy declaration of implicit constructors) so I wouldn't worry about it. Jason ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-06-01 19:12 ` Jason Merrill @ 2021-06-01 21:01 ` Matthias Kretz 2021-06-11 10:01 ` Matthias Kretz 1 sibling, 0 replies; 30+ messages in thread From: Matthias Kretz @ 2021-06-01 21:01 UTC (permalink / raw) To: gcc-patches, Jason Merrill; +Cc: libstdc++, David Malcolm, Jonathan Wakely On Tuesday, 1 June 2021 21:12:18 CEST Jason Merrill wrote: > On 5/28/21 3:42 AM, Matthias Kretz wrote: > > On Friday, 28 May 2021 05:05:52 CEST Jason Merrill wrote: > >> 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. > > This is the basic disconnect: I think that these goals are > contradictory, and that replacement names that are not valid identifiers > will just confuse users that don't know about them. With signal-to-noise ratio I meant the ratio (averaged over all GCC users - so yes, we can't give actual numbers for these): #characters one needs to read to understand / #total diagnostic characters. Or more specifically 1 - #characters that are distracting from understanding the issue / #total diagnostic characters. Consider that for the stdx::simd case I regularly hit the problem that vim's QuickFix truncates at 4095 characters and the message basically just got started (i.e. it's sometimes impossible to use vim's QuickFix to understand errors involving stdx::simd). There's *a lot* of noise that must be removed *per default*. WRT "invalid identifiers", there are two types: (1) string of characters that is not a valid C++ identifier (2) valid C++ identifier, but not defined for the given TU (2) can be confusing, I agree, but doesn't have to be. (1) provides a stronger hint that something is either abbreviated or intentionally hidden from the user. If I write `std::experimental::simd<float>` in my code and get a diagnostic that says 'stdₓ::simd<float, simd_abi::[SSE]>' then it's relatively easy to make the connection what happened here: 'stdₓ' clearly must mean something else than a literal 'stdₓ' in my code. The user knows there's no `std::simd' so it must be `std::experimental::simd`. (Note that once std::experimental::simd goes into the IS, I would be the first to propose a change for 'stdₓ::simd' back to 'std::experimental::simd'.) > If a user sees stdx::foo in a diagnostic and then tries to refer to > stdx::foo and gets an error, the diagnostic is not more helpful than one > that uses the fully qualified name. Hmm, if GCC prints an actual suggestion like "write 'stdₓ::foo' here" then yes, I agree. That should not make use of diagnose_as. > Jonathan, David, any thoughts on this issue? > > > 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.) > > Wouldn't it be better to print stdx::native_simd<float> if that's how > the users write the type? No. For example, I might expect that native_simd maps to AVX-512 vectors but forgot the relevant -m flag(s). If the diagnostics show 'simd<float, simd_abi::[SSE]>' I have a good chance of catching that issue. And the other way around: If I wrote `stdx::simd<float>` and it happens to be the same type as the native_simd typedef, it would show the latter in diagnostics. Similar issue with asking for a simd ABI with `simd_abi::deduce_t<float, 16>`: I typically don't want to know whether that's also native_simd<float> but rather what exact simd_abi I got. And no, as a user I don't typically care about the libstdc++ implementation details but what those details mean. -- ────────────────────────────────────────────────────────────────────────── 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 ────────────────────────────────────────────────────────────────────────── ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-06-01 19:12 ` Jason Merrill 2021-06-01 21:01 ` Matthias Kretz @ 2021-06-11 10:01 ` Matthias Kretz 2021-06-15 15:51 ` Jason Merrill 1 sibling, 1 reply; 30+ messages in thread From: Matthias Kretz @ 2021-06-11 10:01 UTC (permalink / raw) To: gcc-patches, Jason Merrill; +Cc: libstdc++, David Malcolm, Jonathan Wakely [-- Attachment #1: Type: text/plain, Size: 2374 bytes --] How can we make progress here? I could try to produce some "Tony Tables" of diagnostic output of my modified stdx::simd. I believe it's a major productivity boost to see abbreviated / "obfuscated" diagnostics *out-of-the box* (with the possibility to opt-out). Actually, it already *is* a productivity boost to me. Understanding diagnostics has improved from "1. ooof, I'm not going to read this, let me rather guess what the issue was 2. sh** I have to read it 3. several minutes later: I finally found the five words to understand the problem; I could use a break" to "1. right, let me check that" For reference I'll attach my stdx::simd diagnose_as patch. We could also talk about extending the feature to provide more information about the diagnose_as substition. E.g. print a list of all diagnose_as substitutions, which were used, at the end of the output stream. Or simpler, print "note: some identifiers were simplified, use -fno-diagnostics-use- aliases to see their real names". On Tuesday, 1 June 2021 21:12:18 CEST Jason Merrill wrote: > > 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. > > This is the basic disconnect: I think that these goals are > contradictory, and that replacement names that are not valid identifiers > will just confuse users that don't know about them. > > If a user sees stdx::foo in a diagnostic and then tries to refer to > stdx::foo and gets an error, the diagnostic is not more helpful than one > that uses the fully qualified name. > > Jonathan, David, any thoughts on this issue? -- ────────────────────────────────────────────────────────────────────────── 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 ────────────────────────────────────────────────────────────────────────── [-- Attachment #2: 0001-libstdc-Decrease-noise-in-diagnostics-via-diagnose_a.patch --] [-- Type: text/x-patch, Size: 3454 bytes --] diff --git a/libstdc++-v3/include/experimental/bits/simd.h b/libstdc++-v3/include/experimental/bits/simd.h index 43331134301..8e0cceff860 100644 --- a/libstdc++-v3/include/experimental/bits/simd.h +++ b/libstdc++-v3/include/experimental/bits/simd.h @@ -80,13 +80,13 @@ using __m512d [[__gnu__::__vector_size__(64)]] = double; using __m512i [[__gnu__::__vector_size__(64)]] = long long; #endif -namespace simd_abi { +namespace simd_abi [[__gnu__::__diagnose_as__("simd_abi")]] { // simd_abi forward declarations {{{ // implementation details: -struct _Scalar; + struct [[__gnu__::__diagnose_as__("scalar")]] _Scalar; template <int _Np> - struct _Fixed; + struct [[__gnu__::__diagnose_as__("fixed_size")]] _Fixed; // There are two major ABIs that appear on different architectures. // Both have non-boolean values packed into an N Byte register @@ -105,28 +105,11 @@ template <int _UsedBytes> template <int _UsedBytes> struct _VecBltnBtmsk; -template <typename _Tp, int _Np> - using _VecN = _VecBuiltin<sizeof(_Tp) * _Np>; - -template <int _UsedBytes = 16> - using _Sse = _VecBuiltin<_UsedBytes>; - -template <int _UsedBytes = 32> - using _Avx = _VecBuiltin<_UsedBytes>; - -template <int _UsedBytes = 64> - using _Avx512 = _VecBltnBtmsk<_UsedBytes>; - -template <int _UsedBytes = 16> - using _Neon = _VecBuiltin<_UsedBytes>; - -// implementation-defined: -using __sse = _Sse<>; -using __avx = _Avx<>; -using __avx512 = _Avx512<>; -using __neon = _Neon<>; -using __neon128 = _Neon<16>; -using __neon64 = _Neon<8>; +#if defined __i386__ || defined __x86_64__ +using __sse [[__gnu__::__diagnose_as__("[SSE]")]] = _VecBuiltin<16>; +using __avx [[__gnu__::__diagnose_as__("[AVX]")]] = _VecBuiltin<32>; +using __avx512 [[__gnu__::__diagnose_as__("[AVX512]")]] = _VecBltnBtmsk<64>; +#endif // standard: template <typename _Tp, size_t _Np, typename...> @@ -364,7 +347,7 @@ namespace __detail * users link TUs compiled with different flags. This is especially important * for using simd in libraries. */ - using __odr_helper + using __odr_helper [[__gnu__::__diagnose_as__("[ODR helper]")]] = conditional_t<__machine_flags() == 0, _OdrEnforcer, _MachineFlagsTemplate<__machine_flags(), __floating_point_flags()>>; @@ -689,7 +672,7 @@ template <typename _Abi> __is_avx512_abi() { constexpr auto _Bytes = __abi_bytes_v<_Abi>; - return _Bytes <= 64 && is_same_v<simd_abi::_Avx512<_Bytes>, _Abi>; + return _Bytes <= 64 && is_same_v<simd_abi::_VecBltnBtmsk<_Bytes>, _Abi>; } // }}} diff --git a/libstdc++-v3/include/experimental/bits/simd_detail.h b/libstdc++-v3/include/experimental/bits/simd_detail.h index 78ad33f74e4..1f127cd0d52 100644 --- a/libstdc++-v3/include/experimental/bits/simd_detail.h +++ b/libstdc++-v3/include/experimental/bits/simd_detail.h @@ -36,7 +36,7 @@ { \ _GLIBCXX_BEGIN_NAMESPACE_VERSION \ namespace experimental { \ - inline namespace parallelism_v2 { + inline namespace parallelism_v2 [[__gnu__::__diagnose_as__("std\u2093")]] { #define _GLIBCXX_SIMD_END_NAMESPACE \ } \ } \ ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-06-11 10:01 ` Matthias Kretz @ 2021-06-15 15:51 ` Jason Merrill 2021-06-15 20:56 ` Matthias Kretz 0 siblings, 1 reply; 30+ messages in thread From: Jason Merrill @ 2021-06-15 15:51 UTC (permalink / raw) To: Matthias Kretz, gcc-patches; +Cc: libstdc++, David Malcolm, Jonathan Wakely On 6/11/21 6:01 AM, Matthias Kretz wrote: > How can we make progress here? I could try to produce some "Tony Tables" of > diagnostic output of my modified stdx::simd. I believe it's a major > productivity boost to see abbreviated / "obfuscated" diagnostics *out-of-the > box* (with the possibility to opt-out). Actually, it already *is* a > productivity boost to me. Understanding diagnostics has improved from > > "1. ooof, I'm not going to read this, let me rather guess what the issue was > 2. sh** I have to read it > 3. several minutes later: I finally found the five words to understand the > problem; I could use a break" > > to > > "1. right, let me check that" That's great. > For reference I'll attach my stdx::simd diagnose_as patch. > > We could also talk about extending the feature to provide more information > about the diagnose_as substition. E.g. print a list of all diagnose_as > substitutions, which were used, at the end of the output stream. Or simpler, > print "note: some identifiers were simplified, use -fno-diagnostics-use- > aliases to see their real names". Or perhaps before the first use of a name that doesn't correspond to a source-level name. > On Tuesday, 1 June 2021 21:12:18 CEST Jason Merrill wrote: >>> 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. >> >> This is the basic disconnect: I think that these goals are >> contradictory, and that replacement names that are not valid identifiers >> will just confuse users that don't know about them. >> >> If a user sees stdx::foo in a diagnostic and then tries to refer to >> stdx::foo and gets an error, the diagnostic is not more helpful than one >> that uses the fully qualified name. >> >> Jonathan, David, any thoughts on this issue? > -struct _Scalar; > + struct [[__gnu__::__diagnose_as__("scalar")]] _Scalar; > template <int _Np> > - struct _Fixed; > + struct [[__gnu__::__diagnose_as__("fixed_size")]] _Fixed; Thes two could be the variant of the attribute without an explicit string, attached to the alias-declaration. > +using __sse [[__gnu__::__diagnose_as__("[SSE]")]] = _VecBuiltin<16>; > +using __avx [[__gnu__::__diagnose_as__("[AVX]")]] = _VecBuiltin<32>; > +using __avx512 [[__gnu__::__diagnose_as__("[AVX512]")]] = _VecBltnBtmsk<64>; > + using __odr_helper [[__gnu__::__diagnose_as__("[ODR helper]")]] These [] names seem like minimal improvements over the __ names that you would get from the attribute without an explicit string. > + inline namespace parallelism_v2 [[__gnu__::__diagnose_as__("std\u2093")]] { This could go on std::experimental itself, along with my proposed change to hide inline namespaces by default (with a note similar to the one above). Jason ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-06-15 15:51 ` Jason Merrill @ 2021-06-15 20:56 ` Matthias Kretz 2021-06-16 0:48 ` Jason Merrill 0 siblings, 1 reply; 30+ messages in thread From: Matthias Kretz @ 2021-06-15 20:56 UTC (permalink / raw) To: gcc-patches, Jason Merrill; +Cc: libstdc++, David Malcolm, Jonathan Wakely On Tuesday, 15 June 2021 17:51:20 CEST Jason Merrill wrote: > On 6/11/21 6:01 AM, Matthias Kretz wrote: > > For reference I'll attach my stdx::simd diagnose_as patch. > > > > We could also talk about extending the feature to provide more information > > about the diagnose_as substition. E.g. print a list of all diagnose_as > > substitutions, which were used, at the end of the output stream. Or > > simpler, print "note: some identifiers were simplified, use > > -fno-diagnostics-use- aliases to see their real names". > > Or perhaps before the first use of a name that doesn't correspond to a > source-level name. Right. I guess that would be even easier to implement than printing it at the end. > > -struct _Scalar; > > + struct [[__gnu__::__diagnose_as__("scalar")]] _Scalar; > > > > template <int _Np> > > > > - struct _Fixed; > > + struct [[__gnu__::__diagnose_as__("fixed_size")]] _Fixed; > > Thes two could be the variant of the attribute without an explicit > string, attached to the alias-declaration. Agreed. (since you don't have implementation concerns...) > > +using __sse [[__gnu__::__diagnose_as__("[SSE]")]] = _VecBuiltin<16>; > > +using __avx [[__gnu__::__diagnose_as__("[AVX]")]] = _VecBuiltin<32>; > > +using __avx512 [[__gnu__::__diagnose_as__("[AVX512]")]] = > > _VecBltnBtmsk<64>; + using __odr_helper [[__gnu__::__diagnose_as__("[ODR > > helper]")]] > These [] names seem like minimal improvements over the __ names that you > would get from the attribute without an explicit string. Right. It would, however, give the user an identifier that I don't want them to use in their code. We could argue "it has a double-underscore and it's not a documented implementation-defined type, so you're shooting yourself in the foot". Or we could just avoid the issue altogether. I agree this is not a huge issue. > > + inline namespace parallelism_v2 > > [[__gnu__::__diagnose_as__("std\u2093")]] { > This could go on std::experimental itself, along with my proposed change > to hide inline namespaces by default (with a note similar to the one above). Yes, with the following consequences: * If only the std::experimental::parallelism_v2::simd headers set the diagnose_as attribute on std::experimental, the #inclusion of <experimental/ simd> changes the diagnostics of all other TS implementations. * If all TS implementations set the diagnose_as attribute, then it's basically impossible to go back to the long and scary name. Which is what we really should do as soon as there's both a std::simd and a stdₓ::simd. Attaching the diagnose_as attribute to the inline namespace allows for better granularity, even if it's maybe not good enough for some TSs. * If `namespace std { namespace experimental [[gnu::diagnose_as("foo")]] {` turns the scope into 'foo::' and not 'std::foo::' (not sure what you intended) then I could still attach the attribute to the inline namespace. So, yes, I could improve stdx::simd with what you propose. IMHO it wouldn't be as good as what I can do with the patch at hand, though. IIUC, your main concern is that my proposed diagnose_as *can* be used to make diagnostics worse, by replacing names with strings that are not valid identifiers. Of course, whoever uses the attribute to that effect should have a good reason to do so. Is your other concern that using the attribute in a "good" way is repetitive? Would you be happier if I make the string argument to the attribute optional for type aliases? -- ────────────────────────────────────────────────────────────────────────── 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 ────────────────────────────────────────────────────────────────────────── ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-06-15 20:56 ` Matthias Kretz @ 2021-06-16 0:48 ` Jason Merrill 2021-06-22 7:30 ` Matthias Kretz 0 siblings, 1 reply; 30+ messages in thread From: Jason Merrill @ 2021-06-16 0:48 UTC (permalink / raw) To: Matthias Kretz, gcc-patches; +Cc: libstdc++, David Malcolm, Jonathan Wakely On 6/15/21 4:56 PM, Matthias Kretz wrote: > On Tuesday, 15 June 2021 17:51:20 CEST Jason Merrill wrote: >> On 6/11/21 6:01 AM, Matthias Kretz wrote: >>> For reference I'll attach my stdx::simd diagnose_as patch. >>> >>> We could also talk about extending the feature to provide more information >>> about the diagnose_as substition. E.g. print a list of all diagnose_as >>> substitutions, which were used, at the end of the output stream. Or >>> simpler, print "note: some identifiers were simplified, use >>> -fno-diagnostics-use- aliases to see their real names". >> >> Or perhaps before the first use of a name that doesn't correspond to a >> source-level name. > > Right. I guess that would be even easier to implement than printing it at the > end. > >>> -struct _Scalar; >>> + struct [[__gnu__::__diagnose_as__("scalar")]] _Scalar; >>> >>> template <int _Np> >>> >>> - struct _Fixed; >>> + struct [[__gnu__::__diagnose_as__("fixed_size")]] _Fixed; >> >> Thes two could be the variant of the attribute without an explicit >> string, attached to the alias-declaration. > > Agreed. (since you don't have implementation concerns...) > >>> +using __sse [[__gnu__::__diagnose_as__("[SSE]")]] = _VecBuiltin<16>; >>> +using __avx [[__gnu__::__diagnose_as__("[AVX]")]] = _VecBuiltin<32>; >>> +using __avx512 [[__gnu__::__diagnose_as__("[AVX512]")]] = >>> _VecBltnBtmsk<64>; + using __odr_helper [[__gnu__::__diagnose_as__("[ODR >>> helper]")]] >> These [] names seem like minimal improvements over the __ names that you >> would get from the attribute without an explicit string. > > Right. It would, however, give the user an identifier that I don't want them > to use in their code. We could argue "it has a double-underscore and it's not > a documented implementation-defined type, so you're shooting yourself in the > foot". Or we could just avoid the issue altogether. I agree this is not a huge > issue. > >>> + inline namespace parallelism_v2 >>> [[__gnu__::__diagnose_as__("std\u2093")]] { >> This could go on std::experimental itself, along with my proposed change >> to hide inline namespaces by default (with a note similar to the one above). > > Yes, with the following consequences: > > * If only the std::experimental::parallelism_v2::simd headers set the > diagnose_as attribute on std::experimental, the #inclusion of <experimental/ > simd> changes the diagnostics of all other TS implementations. > > * If all TS implementations set the diagnose_as attribute, then it's basically > impossible to go back to the long and scary name. Which is what we really > should do as soon as there's both a std::simd and a stdₓ::simd. Attaching the > diagnose_as attribute to the inline namespace allows for better granularity, > even if it's maybe not good enough for some TSs. > > * If `namespace std { namespace experimental [[gnu::diagnose_as("foo")]] {` > turns the scope into 'foo::' and not 'std::foo::' (not sure what you intended) > then I could still attach the attribute to the inline namespace. > > > So, yes, I could improve stdx::simd with what you propose. IMHO it wouldn't be > as good as what I can do with the patch at hand, though. > > IIUC, your main concern is that my proposed diagnose_as *can* be used to make > diagnostics worse, by replacing names with strings that are not valid > identifiers. Of course, whoever uses the attribute to that effect should have > a good reason to do so. Is your other concern that using the attribute in a > "good" way is repetitive? Would you be happier if I make the string argument > to the attribute optional for type aliases? Yes, and namespace aliases. Jason ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-06-16 0:48 ` Jason Merrill @ 2021-06-22 7:30 ` Matthias Kretz 2021-06-22 19:52 ` Jason Merrill 0 siblings, 1 reply; 30+ messages in thread From: Matthias Kretz @ 2021-06-22 7:30 UTC (permalink / raw) To: gcc-patches, Jason Merrill; +Cc: libstdc++ On Wednesday, 16 June 2021 02:48:09 CEST Jason Merrill wrote: > > IIUC, your main concern is that my proposed diagnose_as *can* be used to > > make diagnostics worse, by replacing names with strings that are not > > valid identifiers. Of course, whoever uses the attribute to that effect > > should have a good reason to do so. Is your other concern that using the > > attribute in a "good" way is repetitive? Would you be happier if I make > > the string argument to the attribute optional for type aliases? > > Yes, and namespace aliases. I'll look into making the attribute argument optional for aliases. Would you accept the patch with this change? Questions: 1. If a type alias applies the attribute after a type was completed / implicitly instantiated (and possibly already used in diagnostics) should / can I still modify the type and add the attribute? 2. About the namespace aliases: IIUC an attribute would currently be rejected because of the C++ grammar. Do you want to make it valid before WG21 officially decides how to proceed? And if you have a pointer for me where I'd have to adjust the grammar rules, that'd help. :) Best, Matthias -- ────────────────────────────────────────────────────────────────────────── 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 ────────────────────────────────────────────────────────────────────────── ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-06-22 7:30 ` Matthias Kretz @ 2021-06-22 19:52 ` Jason Merrill 2021-06-22 20:01 ` Matthias Kretz 2021-07-07 8:23 ` Matthias Kretz 0 siblings, 2 replies; 30+ messages in thread From: Jason Merrill @ 2021-06-22 19:52 UTC (permalink / raw) To: Matthias Kretz, gcc-patches; +Cc: libstdc++ On 6/22/21 3:30 AM, Matthias Kretz wrote: > On Wednesday, 16 June 2021 02:48:09 CEST Jason Merrill wrote: >>> IIUC, your main concern is that my proposed diagnose_as *can* be used to >>> make diagnostics worse, by replacing names with strings that are not >>> valid identifiers. Of course, whoever uses the attribute to that effect >>> should have a good reason to do so. Is your other concern that using the >>> attribute in a "good" way is repetitive? Would you be happier if I make >>> the string argument to the attribute optional for type aliases? >> >> Yes, and namespace aliases. > > I'll look into making the attribute argument optional for aliases. Would you > accept the patch with this change? Yes (after resolving any technical details, of course). > Questions: > > 1. If a type alias applies the attribute after a type was completed / > implicitly instantiated (and possibly already used in diagnostics) should / > can I still modify the type and add the attribute? Yes, because it has no semantic effect. For alias templates, you probably want the attribute only on the templated class, not on the instantiations. > 2. About the namespace aliases: IIUC an attribute would currently be rejected > because of the C++ grammar. Do you want to make it valid before WG21 > officially decides how to proceed? And if you have a pointer for me where I'd > have to adjust the grammar rules, that'd help. :) You will want to adjust cp_parser_namespace_alias_definition to handle attributes like cp_parser_namespace_definition. The latter currently accepts attributes both before and after the name, which seems like a good pattern to follow so it doesn't matter which WG21 chooses. Probably best to pedwarn about C++11 attributes in both locations for now, not just after. Jason ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-06-22 19:52 ` Jason Merrill @ 2021-06-22 20:01 ` Matthias Kretz 2021-06-22 20:12 ` Jason Merrill 2021-07-07 8:23 ` Matthias Kretz 1 sibling, 1 reply; 30+ messages in thread From: Matthias Kretz @ 2021-06-22 20:01 UTC (permalink / raw) To: gcc-patches, Jason Merrill; +Cc: libstdc++ On Tuesday, 22 June 2021 21:52:16 CEST Jason Merrill wrote: > For alias templates, you probably want the attribute only on the > templated class, not on the instantiations. Oh good point. My current patch does not allow the attribute on alias templates. Consider: template <class T, class U> struct X {}; template <class T> using foo [[gnu::diagnose_as]] = X<T, int>; I have no idea how this could work. I would have to set the attribute for an implicit partial specialization (not that I know of the existence of such a thing)? I.e. X<int, int> would have to be diagnosed as foo<int>, but X<int, float> would have to be diagnosed as X<int, float>, not foo. So if anything it should only support alias templates if they are strictly "renaming" the type. I.e. their template parameters must match up exactly. Can I constrain the attribute like this? Or should we rely on developers to be reasonable and only use it for template aliases with matching template params? -Matthias -- ────────────────────────────────────────────────────────────────────────── 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 ────────────────────────────────────────────────────────────────────────── ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-06-22 20:01 ` Matthias Kretz @ 2021-06-22 20:12 ` Jason Merrill 2021-07-01 9:28 ` Matthias Kretz 0 siblings, 1 reply; 30+ messages in thread From: Jason Merrill @ 2021-06-22 20:12 UTC (permalink / raw) To: Matthias Kretz, gcc-patches; +Cc: libstdc++ On 6/22/21 4:01 PM, Matthias Kretz wrote: > On Tuesday, 22 June 2021 21:52:16 CEST Jason Merrill wrote: >> For alias templates, you probably want the attribute only on the >> templated class, not on the instantiations. > > Oh good point. My current patch does not allow the attribute on alias > templates. Consider: > > template <class T, class U> > struct X {}; > > template <class T> > using foo [[gnu::diagnose_as]] = X<T, int>; > > I have no idea how this could work. I would have to set the attribute for an > implicit partial specialization (not that I know of the existence of such a > thing)? I.e. X<int, int> would have to be diagnosed as foo<int>, but X<int, > float> would have to be diagnosed as X<int, float>, not foo. > > So if anything it should only support alias templates if they are strictly > "renaming" the type. I.e. their template parameters must match up exactly. Can > I constrain the attribute like this? Yes. You can check that with get_underlying_template. Or you could support the above by putting the attribute on the instantiation with the TEMPLATE_INFO for foo<A> rather than a simple name. Jason ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-06-22 20:12 ` Jason Merrill @ 2021-07-01 9:28 ` Matthias Kretz 2021-07-01 15:18 ` Jason Merrill 0 siblings, 1 reply; 30+ messages in thread From: Matthias Kretz @ 2021-07-01 9:28 UTC (permalink / raw) To: gcc-patches, Jason Merrill On Tuesday, 22 June 2021 22:12:42 CEST Jason Merrill wrote: > On 6/22/21 4:01 PM, Matthias Kretz wrote: > > On Tuesday, 22 June 2021 21:52:16 CEST Jason Merrill wrote: > >> For alias templates, you probably want the attribute only on the > >> templated class, not on the instantiations. > > > > Oh good point. My current patch does not allow the attribute on alias > > templates. Consider: > > > > template <class T, class U> > > > > struct X {}; > > > > template <class T> > > > > using foo [[gnu::diagnose_as]] = X<T, int>; > > > > I have no idea how this could work. I would have to set the attribute for > > an implicit partial specialization (not that I know of the existence of > > such a thing)? I.e. X<int, int> would have to be diagnosed as foo<int>, > > but X<int, float> would have to be diagnosed as X<int, float>, not foo. > > > > So if anything it should only support alias templates if they are strictly > > "renaming" the type. I.e. their template parameters must match up exactly. > > Can I constrain the attribute like this? > > Yes. You can check that with get_underlying_template. > > Or you could support the above by putting the attribute on the > instantiation with the TEMPLATE_INFO for foo<A> rather than a simple name. Question, given: template <class T> using foo = bar<T>; The diagnose_as attribute handler isn't called until e.g. `foo<int>` is instantiated. Meaning that even after the declaration of the alias template `bar<int>` will not be diagnosed as `foo<int>`, which happens only after the first use of `foo<int>`. I find that more confusing than helpful, even if the expectation would be that users only use the alias template. So do you still expect alias templates to support diagnose_as? And if yes, how could I handle the attribute so that the diagnose_as attribute is applied to `bar` on declaration of `foo`? -- ────────────────────────────────────────────────────────────────────────── 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 ────────────────────────────────────────────────────────────────────────── ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-07-01 9:28 ` Matthias Kretz @ 2021-07-01 15:18 ` Jason Merrill 2021-07-05 14:18 ` Matthias Kretz 0 siblings, 1 reply; 30+ messages in thread From: Jason Merrill @ 2021-07-01 15:18 UTC (permalink / raw) To: Matthias Kretz, gcc-patches On 7/1/21 5:28 AM, Matthias Kretz wrote: > On Tuesday, 22 June 2021 22:12:42 CEST Jason Merrill wrote: >> On 6/22/21 4:01 PM, Matthias Kretz wrote: >>> On Tuesday, 22 June 2021 21:52:16 CEST Jason Merrill wrote: >>>> For alias templates, you probably want the attribute only on the >>>> templated class, not on the instantiations. >>> >>> Oh good point. My current patch does not allow the attribute on alias >>> templates. Consider: >>> >>> template <class T, class U> >>> >>> struct X {}; >>> >>> template <class T> >>> >>> using foo [[gnu::diagnose_as]] = X<T, int>; >>> >>> I have no idea how this could work. I would have to set the attribute for >>> an implicit partial specialization (not that I know of the existence of >>> such a thing)? I.e. X<int, int> would have to be diagnosed as foo<int>, >>> but X<int, float> would have to be diagnosed as X<int, float>, not foo. >>> >>> So if anything it should only support alias templates if they are strictly >>> "renaming" the type. I.e. their template parameters must match up exactly. >>> Can I constrain the attribute like this? >> >> Yes. You can check that with get_underlying_template. >> >> Or you could support the above by putting the attribute on the >> instantiation with the TEMPLATE_INFO for foo<A> rather than a simple name. > > Question, given: > > template <class T> using foo = bar<T>; > > The diagnose_as attribute handler isn't called until e.g. `foo<int>` is > instantiated. You probably want to adjust is_late_template_attribute to change that. > Meaning that even after the declaration of the alias template > `bar<int>` will not be diagnosed as `foo<int>`, which happens only after the > first use of `foo<int>`. I find that more confusing than helpful, even if the > expectation would be that users only use the alias template. > > So do you still expect alias templates to support diagnose_as? And if yes, how > could I handle the attribute so that the diagnose_as attribute is applied to > `bar` on declaration of `foo`? > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-07-01 15:18 ` Jason Merrill @ 2021-07-05 14:18 ` Matthias Kretz 2021-07-07 22:34 ` Jason Merrill 0 siblings, 1 reply; 30+ messages in thread From: Matthias Kretz @ 2021-07-05 14:18 UTC (permalink / raw) To: gcc-patches, Jason Merrill On Thursday, 1 July 2021 17:18:26 CEST Jason Merrill wrote: > You probably want to adjust is_late_template_attribute to change that. Right, I hacked is_late_template_attribute but now I only see a TYPE_DECL passed to my attribute handler (!DECL_ALIAS_TEMPLATE_P). I.e. I don't know how your previous comment is supposed to help me: On Tuesday, 22 June 2021 22:12:42 CEST Jason Merrill wrote: > Yes. You can check that with get_underlying_template. FWIW, I don't feel qualified to implement the diagnose_as attribute on alias templates. The trees I've seen while testing the following test case don't make sense to me. :( // { dg-do compile { target c++11 } } // { dg-options "-fdiagnostics-use-aliases -fpretty-templates" } template <typename T> class A0 {}; template <typename T> using B0 [[gnu::diagnose_as]] = A0<T>; // #1 template <typename T> using C0 [[gnu::diagnose_as]] = A0<T*>; // #2 template <typename T> class A1 {}; template <typename T> class A1<T*> {}; template <typename T> using B1 [[gnu::diagnose_as]] = A1<T*>; // #3 void fn_1(int); int main () { fn_1 (A0<float> ()); // { dg-error "cannot convert 'B0<float>' to 'int'" } fn_1 (A1<float> ()); // { dg-error "cannot convert 'A1<float>' to 'int'" } fn_1 (A1<float*> ()); // { dg-error "cannot convert 'B1<float>' to 'int'" } } On #1 I see !COMPLETE_TYPE_P (TREE_TYPE (*node)) while on #3 TREE_TYPE (*node) is a complete type. Like I said, I don't get to see the TEMPLATE_DECL of either #1, #2, or #3, only a TYPE_DECL whose TREE_TYPE is A0. I thus have no idea how to reject #2. -- ────────────────────────────────────────────────────────────────────────── 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 ────────────────────────────────────────────────────────────────────────── ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-07-05 14:18 ` Matthias Kretz @ 2021-07-07 22:34 ` Jason Merrill 0 siblings, 0 replies; 30+ messages in thread From: Jason Merrill @ 2021-07-07 22:34 UTC (permalink / raw) To: Matthias Kretz, gcc-patches On 7/5/21 10:18 AM, Matthias Kretz wrote: > On Thursday, 1 July 2021 17:18:26 CEST Jason Merrill wrote: >> You probably want to adjust is_late_template_attribute to change that. > > Right, I hacked is_late_template_attribute but now I only see a TYPE_DECL > passed to my attribute handler (!DECL_ALIAS_TEMPLATE_P). > I.e. I don't know how > your previous comment is supposed to help me: > > On Tuesday, 22 June 2021 22:12:42 CEST Jason Merrill wrote: >> Yes. You can check that with get_underlying_template. > > FWIW, I don't feel qualified to implement the diagnose_as attribute on alias > templates. The trees I've seen while testing the following test case don't > make sense to me. :( > > > // { dg-do compile { target c++11 } } > // { dg-options "-fdiagnostics-use-aliases -fpretty-templates" } > > template <typename T> class A0 {}; > template <typename T> using B0 [[gnu::diagnose_as]] = A0<T>; // #1 > template <typename T> using C0 [[gnu::diagnose_as]] = A0<T*>; // #2 > > template <typename T> class A1 {}; > template <typename T> class A1<T*> {}; > template <typename T> using B1 [[gnu::diagnose_as]] = A1<T*>; // #3 > > void fn_1(int); > > int main () > { > fn_1 (A0<float> ()); // { dg-error "cannot convert 'B0<float>' to 'int'" } > fn_1 (A1<float> ()); // { dg-error "cannot convert 'A1<float>' to 'int'" } > fn_1 (A1<float*> ()); // { dg-error "cannot convert 'B1<float>' to 'int'" } > } > > On #1 I see !COMPLETE_TYPE_P (TREE_TYPE (*node)) Yes; a use that matches the primary template but is not the primary template gets a separate dependent type. > while on #3 TREE_TYPE (*node) is a complete type. Indeed, we don't do the same thing for partial specializations. > Like I said, I don't get to see the TEMPLATE_DECL of > either #1, #2, or #3, only a TYPE_DECL whose TREE_TYPE is A0. I thus have no > idea how to reject #2. The difference between #1 and #2 is that same_type_p (#1, CLASSTYPE_PRIMARY_TEMPLATE_TYPE (TREE_TYPE (#1)) is true; see the use of that in resolve_typename_type. Jason ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-06-22 19:52 ` Jason Merrill 2021-06-22 20:01 ` Matthias Kretz @ 2021-07-07 8:23 ` Matthias Kretz 2021-07-08 1:19 ` Jason Merrill 1 sibling, 1 reply; 30+ messages in thread From: Matthias Kretz @ 2021-07-07 8:23 UTC (permalink / raw) To: gcc-patches, Jason Merrill On Tuesday, 22 June 2021 21:52:16 CEST Jason Merrill wrote: > > 2. About the namespace aliases: IIUC an attribute would currently be > > rejected because of the C++ grammar. Do you want to make it valid before > > WG21 officially decides how to proceed? And if you have a pointer for me > > where I'd have to adjust the grammar rules, that'd help. > > You will want to adjust cp_parser_namespace_alias_definition to handle > attributes like cp_parser_namespace_definition. The latter currently > accepts attributes both before and after the name, which seems like a > good pattern to follow so it doesn't matter which WG21 chooses. > Probably best to pedwarn about C++11 attributes in both locations for > now, not just after. This introduces an ambiguity in cp_parser_declaration. The function has to decide whether to call cp_parser_namespace_definition or fall back to cp_parser_block_declaration (which calls cp_parser_namespace_alias_definition). But now the parser has to look ahead a lot farther: namespace foo [[whatever]] {} namespace bar [[whatever]] = foo; I.e. only at '{' vs. '=' can cp_parser_declaration decide to call cp_parser_namespace_definition. Consequently, should I really modify cp_parser_namespace_definition to handle namespace aliases? Or can/should cp_parser_declaration look ahead behind the attribute(s)? How? With pedantic standard C++ it would be easy, since only these attribute placements are allowed: namespace [[whatever] foo {} namespace bar [[whatever]] = foo; -- ────────────────────────────────────────────────────────────────────────── 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 ────────────────────────────────────────────────────────────────────────── ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] Add gnu::diagnose_as attribute 2021-07-07 8:23 ` Matthias Kretz @ 2021-07-08 1:19 ` Jason Merrill 0 siblings, 0 replies; 30+ messages in thread From: Jason Merrill @ 2021-07-08 1:19 UTC (permalink / raw) To: Matthias Kretz, gcc-patches On 7/7/21 4:23 AM, Matthias Kretz wrote: > On Tuesday, 22 June 2021 21:52:16 CEST Jason Merrill wrote: >>> 2. About the namespace aliases: IIUC an attribute would currently be >>> rejected because of the C++ grammar. Do you want to make it valid before >>> WG21 officially decides how to proceed? And if you have a pointer for me >>> where I'd have to adjust the grammar rules, that'd help. >> >> You will want to adjust cp_parser_namespace_alias_definition to handle >> attributes like cp_parser_namespace_definition. The latter currently >> accepts attributes both before and after the name, which seems like a >> good pattern to follow so it doesn't matter which WG21 chooses. >> Probably best to pedwarn about C++11 attributes in both locations for >> now, not just after. > > This introduces an ambiguity in cp_parser_declaration. The function has to > decide whether to call cp_parser_namespace_definition or fall back to > cp_parser_block_declaration (which calls > cp_parser_namespace_alias_definition). But now the parser has to look ahead a > lot farther: > > namespace foo [[whatever]] {} > namespace bar [[whatever]] = foo; > > I.e. only at '{' vs. '=' can cp_parser_declaration decide to call > cp_parser_namespace_definition. > > Consequently, should I really modify cp_parser_namespace_definition to handle > namespace aliases? aliases can also appear at block scope, unlike namespace definitions, but you could factor out some of the alias handling to call from both places. > Or can/should cp_parser_declaration look ahead behind the > attribute(s)? How? cp_parser_skip_attributes_opt Jason ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2021-07-08 1:19 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <14205410.xuKvIAzr1H@excalibur>
[not found] ` <20210427094648.GL3008@redhat.com>
2021-05-04 11:13 ` [PATCH] Add gnu::diagnose_as attribute Matthias Kretz
2021-05-04 13:34 ` David Malcolm
2021-05-04 14:23 ` Matthias Kretz
2021-05-04 14:32 ` Matthias Kretz
2021-05-04 19:00 ` David Malcolm
2021-05-04 19:22 ` Matthias Kretz
2021-05-14 16:05 ` Martin Sebor
2021-05-26 21:33 ` Matthias Kretz
2021-05-27 17:39 ` Jason Merrill
2021-05-27 18:54 ` Matthias Kretz
2021-05-27 21:15 ` Jason Merrill
2021-05-27 22:07 ` Matthias Kretz
2021-05-28 3:05 ` Jason Merrill
2021-05-28 7:42 ` Matthias Kretz
2021-06-01 19:12 ` Jason Merrill
2021-06-01 21:01 ` Matthias Kretz
2021-06-11 10:01 ` Matthias Kretz
2021-06-15 15:51 ` Jason Merrill
2021-06-15 20:56 ` Matthias Kretz
2021-06-16 0:48 ` Jason Merrill
2021-06-22 7:30 ` Matthias Kretz
2021-06-22 19:52 ` Jason Merrill
2021-06-22 20:01 ` Matthias Kretz
2021-06-22 20:12 ` Jason Merrill
2021-07-01 9:28 ` Matthias Kretz
2021-07-01 15:18 ` Jason Merrill
2021-07-05 14:18 ` Matthias Kretz
2021-07-07 22:34 ` Jason Merrill
2021-07-07 8:23 ` Matthias Kretz
2021-07-08 1:19 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).