From: Matthias Kretz <m.kretz@gsi.de>
To: <libstdc++@gcc.gnu.org>
Subject: [RFC] Add gnu::diagnose_as attribute
Date: Fri, 23 Apr 2021 17:16:57 +0200 [thread overview]
Message-ID: <14205410.xuKvIAzr1H@excalibur> (raw)
[-- Attachment #1: Type: text/plain, Size: 5120 bytes --]
Hi,
before requesting comments on the implementation of this patch, I'd like to
know if there's interest in making use of it at all.
In a nutshell, this solves the problem of bad signal-to-noise ratio with TS
implementations and potentially implementation-internal types.
I've been using the diagnose_as attribute on my parallelism_v2 simd
implementation for two months now, and it significantly improved my
productivity. Like this:
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
namespace experimental
{
inline namespace parallelism_v2 [[__gnu__::__diagnose_as__("stdₓ")]]
{
namespace simd_abi [[__gnu__::__diagnose_as__("simd_abi")]]
{
struct [[__gnu__::__diagnose_as__("scalar")]] _Scalar;
using scalar = _Scalar;
template <int _Np>
struct [[__gnu__::__diagnose_as__("fixed_size")]] _Fixed;
template <int _Np>
using fixed_size = _Fixed<_Np>;
[...]
Effects:
1. __PRETTY_FUNCTION__ in my test output:
PASS: void invoke_test(int) [with V = stdₓ::simd<double,
simd_abi::_VecBuiltin<16> >; <template-parameter-1-2> = stdₓ::simd<double,
simd_abi::_VecBuiltin<16> >]
instead of
PASS: void invoke_test(int) [with V =
std::experimental::parallelism_v2::simd<double,
std::experimental::parallelism_v2::simd_abi::_VecBuiltin<16> >; <template-
parameter-1-2> = std::experimental::parallelism_v2::simd<double,
std::experimental::parallelism_v2::simd_abi::_VecBuiltin<16> >]
2. Error messages:
required from 'static constexpr stdₓ::_SimdWrapper<_Tp, _Np>
stdₓ::_SimdImplBuiltin<_Abi, <template-parameter-1-2>
>::_S_fma(stdₓ::_SimdWrapper<_Tp, _Np>, stdₓ::_SimdWrapper<_Tp, _Np>,
stdₓ::_SimdWrapper<_Tp, _Np>) [with _Tp = double; long unsigned int _Np = 2;
_Abi = simd_abi::_VecBuiltin<16>; <template-parameter-1-2> =
stdₓ::__detail::_MachineFlagsTemplate<31, 9>]'
instead of
required from 'static constexpr
std::experimental::parallelism_v2::_SimdWrapper<_Tp, _Np>
std::experimental::parallelism_v2::_SimdImplBuiltin<_Abi, <template-
parameter-1-2> >::_S_fma(std::experimental::parallelism_v2::_SimdWrapper<_Tp,
_Np>, std::experimental::parallelism_v2::_SimdWrapper<_Tp, _Np>,
std::experimental::parallelism_v2::_SimdWrapper<_Tp, _Np>) [with _Tp = double;
long unsigned int _Np = 2; _Abi =
std::experimental::parallelism_v2::simd_abi::_VecBuiltin<16>; <template-
parameter-1-2> =
std::experimental::parallelism_v2::__detail::_MachineFlagsTemplate<31, 9>]'
Since the main user of the attribute would be the standard library (I
believe), this patch only makes sense to pursue if there's interest in using
it in libstdc++. I'd like to use it for stdₓ::simd. Would you accept such a
patch? I guess using unicode in the alias name would not be acceptable,
though?
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.
gcc/ChangeLog:
* doc/invoke.texi: Document -fno-diagnostics-use-aliases.
gcc/c-family/ChangeLog:
* c.opt (fdiagnostics-use-aliases): New diagnostics flag.
gcc/cp/ChangeLog:
* 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_aggr_type): If the type has a diagnose_as attribute, print
the associated string instead of printing the original type
name.
* 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.
---
gcc/c-family/c.opt | 4 ++
gcc/cp/error.c | 19 +++++++-
gcc/cp/name-lookup.c | 27 ++++++++++
gcc/cp/tree.c | 114 +++++++++++++++++++++++++++++++++++++++++++
gcc/doc/invoke.texi | 9 +++-
5 files changed, 170 insertions(+), 3 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: 8444 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..8526bb9cb35 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, ';')
@@ -231,7 +232,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);
}
}
@@ -781,7 +790,13 @@ dump_aggr_type (cxx_pretty_printer *pp, tree t, int flags)
}
}
- if (LAMBDA_TYPE_P (t))
+ tree attr = flag_diagnostics_use_aliases
+ ? lookup_attribute ("diagnose_as",
+ TYPE_ATTRIBUTES (TREE_TYPE (decl)))
+ : NULL_TREE;
+ if (attr)
+ pp_cxx_ws_string (pp, TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
+ else if (LAMBDA_TYPE_P (t))
{
/* A lambda's "type" is essentially its signature. */
pp_string (pp, M_("<lambda"));
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..00e7bdf5b32 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,117 @@ handle_abi_tag_attribute (tree* node, tree name, tree args,
return NULL_TREE;
}
+bool
+check_diagnose_as_redeclaration (const_tree decl, const_tree old,
+ const_tree new_)
+{
+ if (old && TREE_CODE (TREE_VALUE (old)) == TREE_LIST)
+ old = TREE_VALUE (old);
+ if (new_ && TREE_CODE (TREE_VALUE (new_)) == TREE_LIST)
+ new_ = TREE_VALUE (new_);
+ bool err = false;
+ for (const_tree t = new_; t; t = TREE_CHAIN (t))
+ {
+ tree str = TREE_VALUE (t);
+ for (const_tree in = old; in; in = TREE_CHAIN (in))
+ {
+ tree ostr = TREE_VALUE (in);
+ if (cp_tree_equal (str, ostr))
+ goto found;
+ }
+ error ("redeclaration of %qD overwrites diagnose_as attribute %qE",
+ decl, str);
+ err = true;
+ found:;
+ }
+ if (err)
+ {
+ inform (DECL_SOURCE_LOCATION (decl), "previous declaration here");
+ return false;
+ }
+ return true;
+}
+
+static tree
+handle_diagnose_as_attribute (tree* node, tree name, tree args,
+ int flags, bool* no_add_attrs)
+{
+ 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 (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;
+ }
+ else if (CLASS_TYPE_P (*node)
+ && CLASSTYPE_TEMPLATE_SPECIALIZATION (*node))
+ {
+ warning (OPT_Wattributes, "ignoring %qE attribute applied to "
+ "template specialization %qT", name, *node);
+ goto fail;
+ }
+
+ tree attributes = TYPE_ATTRIBUTES (*node);
+ tree decl = TYPE_NAME (*node);
+
+ /* Make sure all declarations have the same diagnose_as string. */
+ if (DECL_SOURCE_LOCATION (decl) != input_location)
+ {
+ if (!check_diagnose_as_redeclaration (
+ decl, lookup_attribute ("diagnose_as", attributes), args))
+ goto fail;
+ }
+ }
+ 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;
+ }
+ /*else if (DECL_P(*node) && DECL_SOURCE_LOCATION (*node) != input_location)
+ {
+ // Make sure all declarations have the same diagnose_as string.
+ tree attributes = DECL_ATTRIBUTES (*node);
+ if (!check_diagnose_as_redeclaration (
+ *node, lookup_attribute ("diagnose_as", attributes), args))
+ 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/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.
next reply other threads:[~2021-04-23 15:17 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-23 15:16 Matthias Kretz [this message]
2021-04-27 9:46 ` Jonathan Wakely
2021-05-04 11:13 ` [PATCH] " 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=14205410.xuKvIAzr1H@excalibur \
--to=m.kretz@gsi.de \
--cc=libstdc++@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).