Re: Re: patch: don't issue -Wreorder warnings when order doesn't matter

[Daniel Marjamäki <daniel dot marjamaki at gmail dot com>]

2011/7/29 Richard Guenther <richard.guenther@gmail.com>:
> 2011/7/29 Daniel Marjamäki <daniel.marjamaki@gmail.com>:
>> Hello!
>>
>>> Why doesn't it matter in this case but it matters when the initializer
>>> are non-constant?
>>
>> It doesn't matter because the program will behave the same no matter
>> if the initializations are reordered or not. Logically it will behave
>> just as the user expects no matter if he expects reordering or not.
>>
>> When one initializer is non-constant there might be a dependency
>> between the two initializations and the wrong order might be a bug.
>> I would like to silence such warnings also, but I don't want to try to
>> determine if there is dependencies or not.
>>
>>
>>> If you don't want to fix up your code, just compile it with -Wno-reorder.
>>
>> I don't want to use -Wno-reorder , because then I won't see the real
>> problems. Don't get me wrong, I like this check.
>>
>>
>> When gcc generates noise I think it is better to fix gcc than to fix my code.
>>
>> The only reason I can think of to keep this noise is for purely
>> stylistic reasons. Somebody might think it is a stylistic problem to
>> initialize members backwards. But then -Wreorder should also warn
>> about common assignments and I doubt anybody wants that.
>
> Ok, I think the idea of the patch sounds reasonable then (but maybe
> we can do even better for more cases by adjusting the
>
> + ? ? ? ? ?if (TREE_CODE(tree_value) != INTEGER_CST)
>
> check to, say, if (! TREE_CONSTANT (tree_value)) which at least
> would also cover floating-point constants and strings. ?Eventually
> C++0x offers some more useful predicate give its constexpr feature.
>
> I suppose C++ maintainers will have to have a look here anyway.
>
> And I guess the example in the manual should be adjusted as Jakub
> says.
>
> Thanks,
> Richard.
>
>> Best regards,
>> Daniel
>>
>

Hello!

Thanks for your tip. I adjusted the patch, using TREE_CONSTANT. It works.

I also agree about updating the example in the manual.

Best regards,
Daniel




Index: gcc/cp/init.c
===================================================================
--- gcc/cp/init.c	(revision 176925)
+++ gcc/cp/init.c	(working copy)
@@ -711,6 +711,7 @@
   VEC(tree,gc) *vbases;
   int i;
   int uses_unions_p;
+  int all_inits_are_const;   /* all members are initialized with a
constant value */

   /* Build up a list of initializations.  The TREE_PURPOSE of entry
      will be the subobject (a FIELD_DECL or BINFO) to initialize.  The
@@ -741,6 +742,26 @@
      without issuing a warning.  */
   next_subobject = sorted_inits;

+  /* check if all explicit initializations use constant values */
+  all_inits_are_const = 1;
+  if (warn_reorder)
+    {
+      for (init = mem_inits; init; init = TREE_CHAIN (init))
+        {
+          tree tree_value;
+
+          tree_value = TREE_VALUE(init);
+          if (TREE_CODE(tree_value) == TREE_LIST)
+            tree_value = TREE_VALUE(tree_value);
+
+          if (! TREE_CONSTANT (tree_value))
+            {
+              all_inits_are_const = 0;
+              break;
+            }
+        }
+    }
+
   /* Go through the explicit initializers, filling in TREE_PURPOSE in
      the SORTED_INITS.  */
   for (init = mem_inits; init; init = TREE_CHAIN (init))
@@ -762,7 +783,7 @@
       /* Issue a warning if the explicit initializer order does not
 	 match that which will actually occur.
 	 ??? Are all these on the correct lines?  */
-      if (warn_reorder && !subobject_init)
+      if (warn_reorder && !all_inits_are_const && !subobject_init)
 	{
 	  if (TREE_CODE (TREE_PURPOSE (next_subobject)) == FIELD_DECL)
 	    warning (OPT_Wreorder, "%q+D will be initialized after",