On 2/5/19 12:14 PM, Jason Merrill wrote:
On 2/5/19 1:46 PM, Martin Sebor wrote:
On 2/1/19 7:41 AM, Jason Merrill wrote:
On 1/31/19 5:49 PM, Martin Sebor wrote:
On 1/30/19 3:15 PM, Jason Merrill wrote:
On 1/29/19 7:15 PM, Martin Sebor wrote:
+      /* Try to convert the original SIZE to a ssizetype.  */
+      if (orig_size != error_mark_node
+          && !TYPE_UNSIGNED (TREE_TYPE (orig_size)))
+        {
+          if (TREE_CODE (size) == INTEGER_CST
+          && tree_int_cst_sign_bit (size))
+        diagsize = build_converted_constant_expr (ssizetype, size,
+                              tsubst_flags_t ());
+          else if (size == error_mark_node
+               && TREE_CODE (orig_size) == INTEGER_CST
+               && tree_int_cst_sign_bit (orig_size))
+        diagsize = build_converted_constant_expr (ssizetype, orig_size,
+                              tsubst_flags_t ());
+        }

Using build_converted_constant_expr here looks odd; that's a language-level notion, and we're dealing with compiler internals. fold_convert seems more appropriate.

Done.


+      if (TREE_CONSTANT (size))
+        {
+          if (!diagsize && TREE_CODE (size) == INTEGER_CST)
+        diagsize = size;
+        }
+      else
         size = osize;
     }

@@ -9732,15 +9758,12 @@ compute_array_index_type_loc (location_t name_loc,
   if (TREE_CODE (size) == INTEGER_CST)
     {
       /* An array must have a positive number of elements.  */
-      if (!valid_constant_size_p (size))
+      if (!diagsize)
+    diagsize = size;

It seems like the earlier hunk here is unnecessary; if size is an INTEGER_CST, it will be unchanged, and so be used for diagsize in the latter hunk without any changes to the earlier location. Actually, why not do all of the diagsize logic down here?  There doesn't seem to be anything above that relies on information we will have lost at this point.

Sure.  Done in the attached revision.

-  tree osize = size;
+  /* The original numeric size as seen in the source code after
+     any substitution and before conversion to size_t.  */
+  tree origsize = NULL_TREE;

Can't you use osize?  instantiate_non_dependent_expr doesn't do any actual substitution, it shouldn't change the type of the expression or affect whether it's an INTEGER_CST.

I went ahead and reused osize but kept the new name origsize (I assume
avoiding introducing a new variable is what you meant).  The longer
name is more descriptive and also has a comment explaining what it's
for (which is why I didn't touch osize initially -- I didn't know
enough about what it was for or how it might change).

Yep, it was saving the original "size" before conversions.  I guess the name change is OK, but please restore the initialization from "size", and drop the added comment on the call to mark_rvalue_use (which is to possibly replace a lambda capture with its constant value).

How about passing origsize into valid_array_size_p rather than than always computing diagsize in the caller?

I'm not sure I understand what you're asking for here.  diagsize
is computed from either size or origsize but valid_array_size_p
takes just one size (or type) argument.  What part do you want
to move to valid_array_size_p (and why)?  Or are you asking to
call fold_convert() in valid_array_size_p instead here?

Martin

Reply via email to