On 7/23/21 4:58 AM, Matthias Kretz wrote:
Hi Jason,
I found a few regressions from the last patch in the meantime. Version 4 of
the patch is attached.
Questions:
1. I simplified the condition for calling dump_template_parms in
dump_function_name. !DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION (t) is
equivalent to DECL_USE_TEMPLATE (t) in this context; implying that
dump_template_parms is unconditionally called with `primary = false`. Or am I
missing something?
2. Given a DECL_TI_ARGS tree, can I query whether an argument was deduced or
explicitly specified? I'm asking because I still consider diagnostics of
function templates unfortunate. `template <class T> void f()` is fine, as is
`void f(T) [with T = float]`, but `void f() [with T = float]` could be better.
I.e. if the template parameter appears somewhere in the function parameter
list, dump_template_parms would only produce noise. If, however, the template
parameter was given explicitly, it would be nice if it could show up
accordingly in diagnostics.
3. When parsing tentatively and the parse is rejected, input_location is not
reset, correct? In the attached patch I therefore made
cp_parser_namespace_alias_definition reset input_location on a failed
tentative parse. But it feels wrong. Shouldn't input_location be restored on
cp_parser_parse_definitely?
--
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.kr...@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_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.
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.
(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?
* 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?
(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.
+ 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 */;
?
(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.
+ 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)));
CLASSTYPE_SPECIALIZATION_OF_PRIMARY_TEMPLATE_P?
+ tmplate ? &TREE_CHAIN(*parms) : parms, flags);
Missing space before (
+ if (tmplate)
+ TREE_VALUE (*parms) = make_tree_vec (0);
This could use a comment.
(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)
Checking WILDCARD_TYPE_P is generally not what you want; plenty of
dependent types don't show up specifically as wildcards. T*, for instance.
+ 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.
(dump_simple_decl): Call dump_decl_name_or_diagnose_as instead
of dump_decl.
(dump_decl): Ditto.
+ dump_decl_name_or_diagnose_as(pp, t, flags);
Missing space before (
(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.
(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.
+ 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. Or use cp_parser_skip_attributes_opt
to avoid tentative parsing in the first place.
+ if (nullptr == cp_parser_require (parser, CPP_EQ, RT_EQ))
The usual pattern is
if (!cp_parser_require
* 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 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.
+ /* 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.
+ // 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.
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.
* g++.dg/diagnostic/diagnose-as5.C: New test.
* g++.dg/diagnostic/diagnose-as6.C: New test.
---
gcc/c-family/c.opt | 4 +
gcc/cp/cp-tree.h | 1 +
gcc/cp/decl2.c | 45 ++++
gcc/cp/error.c | 247 ++++++++++++++++--
gcc/cp/name-lookup.c | 52 +++-
gcc/cp/name-lookup.h | 2 +-
gcc/cp/parser.c | 41 +--
gcc/cp/tree.c | 148 +++++++++++
gcc/doc/extend.texi | 45 ++++
gcc/doc/invoke.texi | 9 +-
.../g++.dg/diagnostic/diagnose-as1.C | 213 +++++++++++++++
.../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 ++++
16 files changed, 1291 insertions(+), 36 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
