[cfe-dev] assert when vector variable is used as the size of array

[Douglas Gregor]

On Jul 20, 2010, at 10:39 PM, Anton Yartsev wrote:

> Hi all,
> 
> I accidentally stumbled upon the fact that situation when a vector variable is used as the size of array is not handled. Compiling the code
> 
> void f()
> {
>  int __attribute__ ((vector_size (16))) a;
>  int b[a];
> }
> 
> ends with
> 
> Assertion failed: castIsValid(getOpcode(), S, Ty) && "Illegal BitCast", file Instructions.cpp, line 2654
> 
> While investigating the problem I found out that methods Type::isIntegerType(), Type::isSignedIntegerType() and Type::isUnsignedIntegerType() (.\lib\AST\Type.cpp ) return 'true' for vector types with corresponding element types. It looks strange as vector types are not treated as integer types.
> I made two patches: the first one - vector_size_of_array.patch - removes code for vectors from the above methods and adds additional methods instead - Type::hasIntegerRepresentation(), Type::hasSignedIntRepresentation() and Type::hasUnsignedIntRepresentation() - by analogy with Type::isFloatingType() and Type::hasFloatingRepresentation(). Calls to old methods were replaced with calls to new ones where it was needed (at least no regression bugs). The patch solves the subject. It also make Clang sliiiightly faster..

Your patch looks great. I had meant to introduce hasIntegerRepresentation et al when I added hasFloatingRepresentation, but ran out of time. Only a few comments on the patch:

  1) I'd rather spell out "Integer" in the names of these functions, e.g., Type::hasSignedIntegerRepresentation(). Sure, it's long, but it's also more consistent.
  2) I was surprised at how few places in Sema and CodeGen had to change from is(Signed|Unsigned|)IntegerType to has(Signed|Unsigned|)Representation; did you audit all of the callers of the "is(Signed|Unsigned|)IntegerType" functions?
  3) Could you a test case that verifies that we reject the code above?

- Doug