[PATCH] Allow transformation of VariableArray to ConstantArray.

Serge Pavlov sepavloff at gmail.com
Wed Feb 5 08:50:07 PST 2014



================
Comment at: lib/Sema/TreeTransform.h:3932-3936
@@ -3931,3 +3931,7 @@
 
-  VariableArrayTypeLoc NewTL = TLB.push<VariableArrayTypeLoc>(Result);
+  ArrayTypeLoc NewTL;
+  if (Result->getTypeClass() == Type::ConstantArray)
+    NewTL = TLB.push<ConstantArrayTypeLoc>(Result);
+  else
+    NewTL = TLB.push<VariableArrayTypeLoc>(Result);
   NewTL.setLBracketLoc(TL.getLBracketLoc());
----------------
Richard Smith wrote:
> Here's what `TransformDependentSizedArrayType` does:
> 
>   // We might have any sort of array type now, but fortunately they
>   // all have the same location layout.
>   ArrayTypeLoc NewTL = TLB.push<ArrayTypeLoc>(Result);
> 
> Maybe do the same here?
This is more readable, thank you for the hint.

================
Comment at: test/SemaCXX/crashes.cpp:235
@@ +234,3 @@
+
+namespace pr18633 {
+  struct A1 {
----------------
Richard Smith wrote:
> David Blaikie wrote:
> > While the existence of this test file might imply otherwise - in my opinion, we should never simply have a test that "doesn't crash". There must be some ultimate behavior expected of the compiler that was never reached due to the crash - that's missing test coverage that should be added.
> > 
> > (also, presumably there are some related test cases in a more specific test file somewhere that this should be grouped with, perhaps - either the non-templated case, or a template test that tests some similar functionality but missed this case (but I know it's sometimes hard to track down the right test cases - sometimes it can be done by evoking diagnostics for similar features - but since this test has no diagnostics I guess it should be a codegen test))
> Agreed. This test should check that (for instance) we can apply 'sizeof' to the resulting array type and get the right constant size back.
> 
> It'd also be good to run this test with -pedantic and ensure that we do get the 'VLA' extension warning here (it's not quite the right diagnostic, but -- pending feedback from CWG -- we do want to reject this code in strictly-conforming mode).
The test is moved into CXXSema/c99-variable-length-array.cpp, it tests both warning generation and absence of crash.

The fixed behavior does not add any syntactic extensions apart from VLA support itself. Array 'int arr[A1::sz]' is considered as VLA when template definition is built. So 'sizeof(arr)' is not a constant, it cannot be used as a non-type template parameter, for instance. However function template instantiation uses ConstantArrayType for this array. It look like just a detail of implementation.


http://llvm-reviews.chandlerc.com/D2688



More information about the cfe-commits mailing list