public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFC] Add gnu::diagnose_as attribute
@ 2021-04-23 15:16 Matthias Kretz
  2021-04-27  9:46 ` Jonathan Wakely
  0 siblings, 1 reply; 26+ messages in thread
From: Matthias Kretz @ 2021-04-23 15:16 UTC (permalink / raw)
  To: libstdc++

[-- 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.

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2021-06-22 20:12 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-23 15:16 [RFC] Add gnu::diagnose_as attribute Matthias Kretz
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

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).