This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/3] improve detection of attribute conflicts (PR 81544)


On 08/09/2017 05:53 AM, Marek Polacek wrote:
(Not a proper review really.)

On Tue, Aug 08, 2017 at 10:13:07AM -0600, Martin Sebor wrote:
@@ -490,7 +583,9 @@ decl_attributes (tree *node, tree attributes, int flags)
 		       | (int) ATTR_FLAG_ARRAY_NEXT))
 	    {
 	      /* Pass on this attribute to be tried again.  */
-	      returned_attrs = tree_cons (name, args, returned_attrs);
+	      tree attr = tree_cons (name, args, NULL_TREE);
+	      returned_attrs = chainon (returned_attrs, attr);
+	      // returned_attrs = tree_cons (name, args, returned_attrs);
 	      continue;
 	    }
 	  else
@@ -535,7 +630,9 @@ decl_attributes (tree *node, tree attributes, int flags)
 	  else if (flags & (int) ATTR_FLAG_FUNCTION_NEXT)
 	    {
 	      /* Pass on this attribute to be tried again.  */
-	      returned_attrs = tree_cons (name, args, returned_attrs);
+	      tree attr = tree_cons (name, args, NULL_TREE);
+	      returned_attrs = chainon (returned_attrs, attr);
+	      // returned_attrs = tree_cons (name, args, returned_attrs);

What's with these // lines?  I suppose they weren't supposed to be here.

Yes, the commented lines should be removed.  Thanks for pointing
them out.  I'll do that in the next revision...


+      /* If the attribute was sussceefully handled on its own and is

"successfully"

...along with fixing typos like this one...


diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index e970ab2..8bbfcb5 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -2143,36 +2143,19 @@ diagnose_mismatched_attributes (tree olddecl, tree newdecl)
 		       newdecl);

   /* Diagnose inline __attribute__ ((noinline)) which is silly.  */
+  const char* noinline = "noinline";

"const char *noinline"

...and adjusting these kinds of things.

...
--- a/libstdc++-v3/include/ext/mt_allocator.h
+++ b/libstdc++-v3/include/ext/mt_allocator.h
@@ -355,7 +355,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }

       // XXX GLIBCXX_ABI Deprecated
-      _GLIBCXX_CONST void
+      void
       _M_destroy_thread_key(void*) throw ();

       size_t

Why this change?  Seems unrelated.

This appears to be a pointless use of attribute const:

  https://gcc.gnu.org/ml/gcc/2017-08/msg00027.html

It was highlighted by tightening up the attribute const handler
also included in the patch.  I forgot to call it out separately
or CC Jonathan/libstdc++ on it (perhaps it should have been
split out into a small patch of its own).


diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
index 146b76c..58a4742 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
@@ -113,7 +113,7 @@ int test9 (int *intarr)

 int test99 (int *intarr)
 {
-  extern int foo9 (int) __attribute__ ((pure));
+  extern int foo9 (int) __attribute__ ((const));
   int h, v;
   g9 = 9;
   h = foo9 (g9);


And why this?  I'd avoid modifying existing tests like that unless
it's directly related to the new diagnostic.

The same function is declared earlier on in the file with attribute
const and the redeclaration with attribute pure triggers a warning
due to the two being considered mutually exclusive.

Thanks for the review!

Martin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]