[v5] c++: Add gnu::diagnose_as attribute

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
──────────────────────────────────────────────────────────────────────────

Message ID	1845718.Z2AExFMA8N@excalibur
State	New
Headers	show Return-Path: <gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org> X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=<UNKNOWN>) Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4Hsqxs0HDwz9sPf for <incoming@patchwork.ozlabs.org>; Mon, 15 Nov 2021 11:36:31 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C046C3858034 for <incoming@patchwork.ozlabs.org>; Mon, 15 Nov 2021 00:36:28 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from lxmtout2.gsi.de (lxmtout2.gsi.de [140.181.3.112]) by sourceware.org (Postfix) with ESMTPS id 553823858013 for <gcc-patches@gcc.gnu.org>; Mon, 15 Nov 2021 00:35:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 553823858013 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gsi.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gsi.de Received: from localhost (localhost [127.0.0.1]) by lxmtout2.gsi.de (Postfix) with ESMTP id C6E97202AD67; Mon, 15 Nov 2021 01:35:35 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at lxmtout2.gsi.de Received: from lxmtout2.gsi.de ([127.0.0.1]) by localhost (lxmtout2.gsi.de [127.0.0.1]) (amavisd-new, port 10024) with LMTP id BMmyfP4IdTz8; Mon, 15 Nov 2021 01:35:35 +0100 (CET) Received: from srvex3.campus.gsi.de (unknown [10.10.4.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) by lxmtout2.gsi.de (Postfix) with ESMTPS id 5339D202AD65; Mon, 15 Nov 2021 01:35:35 +0100 (CET) Received: from excalibur.localnet (140.181.3.12) by srvex3.campus.gsi.de (10.10.4.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2375.17; Mon, 15 Nov 2021 01:35:34 +0100 From: Matthias Kretz <m.kretz@gsi.de> To: <gcc-patches@gcc.gnu.org>, Jason Merrill <jason@redhat.com> Subject: [PATCH v5] c++: Add gnu::diagnose_as attribute Date: Mon, 15 Nov 2021 01:35:34 +0100 Message-ID: <1845718.Z2AExFMA8N@excalibur> Organization: GSI Helmholtzzentrum =?utf-8?q?f=C3=BCr?= Schwerionenforschung In-Reply-To: <de35034b-61ca-7d24-bcbc-9840af63700c@redhat.com> References: <4361366.VLH7GnMWUR@minbar> <4019800.7NyaFmOyK0@excalibur> <de35034b-61ca-7d24-bcbc-9840af63700c@redhat.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="nextPart2548101.QqfNXX16uU" Content-Transfer-Encoding: 7Bit X-Originating-IP: [140.181.3.12] X-ClientProxiedBy: srvex3.Campus.gsi.de (10.10.4.16) To srvex3.campus.gsi.de (10.10.4.16) X-Spam-Status: No, score=-10.4 required=5.0 tests=BAYES_00, BODY_8BITS, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H2, SPF_PASS, TXREP, T_SPF_HELO_PERMERROR autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org>
Series	[v5] c++: Add gnu::diagnose_as attribute \| expand [v5] c++: Add gnu::diagnose_as attribute

[v5] c++: Add gnu::diagnose_as attribute

Commit Message

Patch