chuck icon indicating copy to clipboard operation
chuck copied to clipboard

Crash for comma in array subscript

Open spencersalazar opened this issue 8 years ago • 3 comments

The following code causes ChucK to crash during compilation:

[[53,57,60,63],[57,60,63,69]] @=> int chordz[][];
54 => chordz[0,0];

The issue seems to be the comma in the array subscript, which is not valid syntax, but should generate a compile error rather than crashing.

spencersalazar avatar Jan 07 '16 22:01 spencersalazar

The crash stems from a failure of the following assertion in the type_engine_check_exp_array function of chuck_type.cpp.

   assert( array->indices->depth == depth );

Generally,

   //ChucK code
   int x[];
   x[v1, v2, ...vn];

array->indices->depth is accurately 1, but depth is n.

As you requested, I rephrased the assertion as an error.

if( array->indices->depth != depth )
{
  EM_error2( array->linepos,
      "[..., ...] is invalid subscript syntax." );
  return NULL;
}

I'll be the first to admit it's an ugly patch. I tried following the erroneous depth value upstream--It's dictated by the number of elements in array's exp_list, which itself is set exclusively by generated code, so I don't think much else can be done.

   //ChucK code
   int z[v1, v2, ...vn];

I also discovered that the above code successfully compiles yet produces a length vn int array of zeroes...How tedious would it be to disallow commas inside hard brackets everywhere except for the left side of the @=> operator?

tim-torres avatar Feb 24 '17 14:02 tim-torres

Hey Tim,

Cool, thats a good analysis. I think the most general solution would be to disallow comma expressions in array subscripts (e.g., right after a variable name) while continuing to allow them in array literals, though Im not sure off the top of my head where that distinction is made in the parser/compiler system. Ideally it could be caught in the .y file which defines ChucK's grammar. Your proposed patch seems like a good solution to start with; if you want to make a pull request with that change, that'd be great!

spencer

spencersalazar avatar Feb 25 '17 04:02 spencersalazar

Christ, this looks like a warzone... Was just trying to change my branch's name..

Okay finally got it probably. I need to sleep now

tim-torres avatar Feb 25 '17 12:02 tim-torres