[PATCH v5] c++: Add gnu::diagnose_as attribute
Matthias Kretz
m.kretz@gsi.de
Mon Nov 15 00:35:34 GMT 2021
Sorry for taking so long. I hope we can still get this done for GCC 12.
One open question: If we change std::__cxx11::basic_string<char> to
std::string with this feature, should DWARF strings change or not? I.e. should
diagnose_as be conditional on (pp->flags & pp_c_flag_gnu_v3)? If these strings
are only for user consumption, I think the DWARF strings should be affected by
the attribute...
Oh, and note that the current patch depends on the "c++: Print function
template parms when relevant" patch I sent on Nov 8th.
On Wednesday, 8 September 2021 04:21:51 CEST Jason Merrill wrote:
> On 7/23/21 4:58 AM, Matthias Kretz wrote:
> > gcc/cp/ChangeLog:
> > PR c++/89370
> > * cp-tree.h: Add is_alias_template_p declaration.
> > * decl2.c (is_alias_template_p): New function. Determines
> > whether a given TYPE_DECL is actually an alias template that is
> > still missing its template_info.
>
> I still think you want to share code with get_underlying_template. For
> the case where the alias doesn't have DECL_TEMPLATE_INFO yet, you can
> compare to current_template_args (). Or you could do some initial
> processing that doesn't care about templates in the handler, and then do
> more in cp_parser_alias_declaration after the call to grokfield/start_decl.
I still don't understand how I could make use of get_underlying_template. I.e.
I don't even understand how get_underlying_template answers any of the
questions I need answered. I used way too much time trying to make this
work...
> If you still think you need this function, let's call it
> is_renaming_alias_template or renaming_alias_template_p; using both is_
> and _p is redundant. I don't have a strong preference which.
OK.
> > (is_late_template_attribute): Decls with diagnose_as attribute
> > are early attributes only if they are alias templates.
>
> Is there a reason not to apply it early to other templates as well?
Unconditionally returning false for diagnose_as in is_late_template_attribute
makes renamed class templates print without template parameter list. E.g.
template <class T> struct [[diagnose_as("foo")]] A;
using bar [[diagnose_as]] = A<int>;
template <class T> struct A {
template <class U> struct B {};
using C [[diagnose_as]] = B<int>;
};
could query for attributes. So IIUC, member types of class templates require
late attributes.
> > * 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.
>
> Did you decide not to handle this in dump_decl, so we use the
> diagnose_as when referring to the namespace in non-scope contexts as well?
Good question. dump_decl is the more general place for handling the attribute
and that's where I moved it to.
> > + if (flag_diagnostics_use_aliases)
> > + {
> > + tree attr = lookup_attribute ("diagnose_as", DECL_ATTRIBUTES
> > (decl)); + if (attr && TREE_VALUE (attr))
> > + {
> > + pp_cxx_ws_string (
> > + pp, TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
>
> This pattern is used several places outside this function; can we factor
> it into something like
>
> if (maybe_print_diagnose_as (special))
> /* OK */;
Yes, I added the functions lookup_diagnose_as_attribute and
dump_diagnose_as_alias to remove code duplication.
> Missing space before (
OK. I think I found and fixed all of them.
> > + if (tmplate)
> > + TREE_VALUE (*parms) = make_tree_vec (0);
>
> This could use a comment.
Added.
> > (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. Delay call to
> > dump_scope until the diagnose_as attribute is found. If the
> > attribute has a second argument, use it to override the context
> > passed to dump_scope.
> >
> > + 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;
> > + }
> > + }
>
> I think you want any_dependent_template_args_p (args)
Yes, except that I need `++processing_template_decl` before calling it (and
decrement after it, of course). Is that acceptable?
> Checking WILDCARD_TYPE_P is generally not what you want; plenty of
> dependent types don't show up specifically as wildcards. T*, for instance.
Right, that's why I had `while (INDIRECT_TYPE_P (arg)) arg = TREE_TYPE
(arg);` before the wildcard test. But I replaced all of those with calls to
any_dependent_template_arguments_p now.
> > + if (diagnose_as)
> > + pp_cxx_ws_string (pp, TREE_STRING_POINTER (
> > + TREE_VALUE (TREE_VALUE (diagnose_as))));
>
> ( needs to go on the next line. I'd format this as
>
> if (diagnose_as)
> pp_cxx_ws_string (pp, (TREE_STRING_POINTER
> (TREE_VALUE (TREE_VALUE (diagnose_as)))));
>
> There's a lot of this formatting pattern in the patch.
All fixed.
> Missing space before (
fixed.
> > (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 diagnosed with pretty templates set
> > TFF_TEMPLATE_NAME to skip dump_template_parms.
> > (dump_function_name): Replace the function's identifier with the
> > diagnose_as attribute value, if set. Expand
> > DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION to DECL_USE_TEMPLATE
> > and consequently call dump_template_parms with primary = false.
> > (comparable_template_types_p): Consider the types not a template
> > if one carries a diagnose_as attribute.
>
> I'd think it would be more helpful to suppress diagnose_as if the types
> are comparable.
So instead of diagnosing e.g. 'std::string' vs. 'std::wstring' it should print
'std::basic_string<char, ...>' vs. 'std::basic_string<wchar_t, ...>' here? I
believe that would be more confusing than helpful. Either switch all aliases
on or all of them off.
> > (cp_parser_namespace_alias_definition): Allow optional
> > attributes before and after the identifier. Fast exit, restoring
> > input_location, if the expected CPP_EQ token is missing. Pass
> > attributes to do_namespace_alias.
> >
> > + if (attributes
> > + && !cp_parser_uncommitted_to_tentative_parse_p (parser))
> > + pedwarn (input_location, OPT_Wpedantic,
> > + "standard attributes on namespaces aliases must follow "
> > + "the namespace alias name");
>
> Maybe remember where we saw attributes and warn later, after we've
> committed to parsing as an alias.
OK.
> Or use cp_parser_skip_attributes_opt
> to avoid tentative parsing in the first place.
I don't see how to do that easily. We need to parse up to the RT_EQ token
before we know whether it's a namespace alias or namespace definition.
> > + if (nullptr == cp_parser_require (parser, CPP_EQ, RT_EQ))
>
> The usual pattern is
>
> if (!cp_parser_require
done
> > (handle_diagnose_as_attribute): New function; copied and
> > adjusted from handle_abi_tag_attribute. If the given *node is a
> > TYPE_DECL: allow no argument to the attribute, using DECL_NAME
> > instead; apply the attribute to the type on the RHS in place,
> > even if the type is complete. Allow 2 arguments when called from
> > handle_diagnose_as_attribute. For type aliases, append
> > CP_DECL_CONTEXT as second attribute argument when the RHS type
> > has a different context. Warn about alias templates without
> > matching template arguments. Apply the attribute to the primary
> > template type for alias templates.
>
> All this description of semantics should be in a comment rather than the
> CHangeLog.
done
> > + /* Reject alias templates without wildcards on the innermost
> > template arg s
> > + of the RHS type. E.g. template <class> using A = B; */
> > + if (DECL_LANG_SPECIFIC (decl)
> > + && DECL_TEMPLATE_INFO (decl)
> > + && DECL_ALIAS_TEMPLATE_P (DECL_TI_TEMPLATE (decl))
> > + && PRIMARY_TEMPLATE_P (DECL_TI_TEMPLATE (decl)))
> > + return error_mark_node;
>
> I don't think this is doing anything useful; if we had already done
> push_template_decl by this point, it would reject all alias templates.
Without this test e.g.
template <class T>
using foo [[gnu::diagnose_as]] = typename bar<T>::inner;
is not rejected. I want to reject cases where the RHS is not a primary
template because I cannot apply the attribute to bar<T>::inner until foo<U>
instantiates bar<U>. Thus, if bar<U>::inner is used before the alias is used
the diagnose_as attribute is not present.
> > + // Add the DECL_CONTEXT of the alias to the attribute if it is
> > different + // to the context of the type.
>
> How about using the alias TYPE_DECL itself as the argument to the
> attribute on the type? Then we wouldn't need to copy its name into a
> string, either.
Done.
New patch:
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.
Signed-off-by: Matthias Kretz <m.kretz@gsi.de>
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 is_renaming_alias_template declaration.
* decl2.c (is_renaming_alias_template): New function. Determines
whether a given TYPE_DECL is actually an alias template that is
still missing its template_info.
(is_late_template_attribute): Decls with diagnose_as attribute
are early attributes only if they are alias templates.
* error.c (dump_scope): Don't remove TFF_AS_PRIMARY flag.
(lookup_diagnose_as_attribute): New function.
(dump_diagnose_as_alias): New function.
(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. Delay call to
dump_scope until the diagnose_as attribute is found. If the
attribute has a second argument, use it to override the context
passed to dump_scope.
(dump_simple_decl): Call dump_decl_name_or_diagnose_as instead
of dump_decl.
(lang_decl_name): Ditto.
(dump_decl): Ditto. 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_function_decl): Walk the functions context list to
determine whether a call to dump_template_scope is required.
(dump_function_name): Replace the function's identifier with the
diagnose_as attribute value, if set.
(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 on namespaces. Ensure exactly one string argument.
Ensure previous diagnose_as attributes used the same name.
'diagnose_as' on namespace aliases are forwarded to the original
namespace. Support no-argument 'diagnose_as' on namespace
aliases.
(do_namespace_alias): Add attributes parameter and call
handle_namespace_attrs.
* name-lookup.h (do_namespace_alias): Add attributes tree
parameter.
* parser.c (cp_parser_declaration): If the next token is
RID_NAMESPACE, tentatively parse a namespace alias definition.
If this fails expect a namespace definition.
(cp_parser_namespace_alias_definition): Allow optional
attributes before and after the identifier. Fast exit, restoring
input_location, if the expected CPP_EQ token is missing. Pass
attributes to do_namespace_alias.
* 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: allow no argument to the attribute, using the
TYPE_DECL as attribute value instead; apply the attribute to the
type on the RHS in place, even if the type is complete. Allow 2
arguments when called from handle_diagnose_as_attribute. For
type aliases, append CP_DECL_CONTEXT as second attribute
argument when the RHS type has a different context. Warn about
alias templates without matching template arguments. Apply the
attribute to the primary template type for alias templates.
gcc/testsuite/ChangeLog:
PR c++/89370
* g++.dg/diagnostic/default-template-args-1.C: Add testcase with
partial specialization.
* 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.
* g++.dg/diagnostic/diagnose-as5.C: New test.
* g++.dg/diagnostic/diagnose-as6.C: New test.
* g++.dg/diagnostic/diagnose-as7.C: New test.
---
gcc/c-family/c.opt | 4 +
gcc/cp/cp-tree.h | 1 +
gcc/cp/decl2.c | 37 +++
gcc/cp/error.c | 289 ++++++++++++++++--
gcc/cp/name-lookup.c | 53 +++-
gcc/cp/name-lookup.h | 2 +-
gcc/cp/parser.c | 40 ++-
gcc/cp/tree.c | 166 ++++++++++
gcc/doc/extend.texi | 45 +++
gcc/doc/invoke.texi | 9 +-
.../diagnostic/default-template-args-1.C | 8 +
.../g++.dg/diagnostic/diagnose-as1.C | 228 ++++++++++++++
.../g++.dg/diagnostic/diagnose-as2.C | 144 +++++++++
.../g++.dg/diagnostic/diagnose-as3.C | 152 +++++++++
.../g++.dg/diagnostic/diagnose-as4.C | 158 ++++++++++
.../g++.dg/diagnostic/diagnose-as5.C | 21 ++
.../g++.dg/diagnostic/diagnose-as6.C | 45 +++
.../g++.dg/diagnostic/diagnose-as7.C | 43 +++
18 files changed, 1400 insertions(+), 45 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
create mode 100644 gcc/testsuite/g++.dg/diagnostic/diagnose-as5.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/diagnose-as6.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/diagnose-as7.C
--
──────────────────────────────────────────────────────────────────────────
Dr. Matthias Kretz https://mattkretz.github.io
GSI Helmholtz Centre for Heavy Ion Research https://gsi.de
stdₓ::simd
──────────────────────────────────────────────────────────────────────────
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-c-Add-gnu-diagnose_as-attribute.patch
Type: text/x-patch
Size: 69964 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20211115/652c1add/attachment-0001.bin>
More information about the Gcc-patches
mailing list