Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash for comma in array subscript #43

Closed
spencersalazar opened this issue Jan 7, 2016 · 4 comments
Closed

Crash for comma in array subscript #43

spencersalazar opened this issue Jan 7, 2016 · 4 comments
Labels

Comments

@spencersalazar
Copy link
Member

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.

@tim-torres
Copy link
Contributor

tim-torres commented Feb 24, 2017

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?

@spencersalazar
Copy link
Member Author

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

tim-torres added a commit to tim-torres/chuck that referenced this issue Feb 25, 2017
As requested in issue ccrma#43. This addresses but one issue of potentially many brought upon by not disallowing commas in array subscripts.
tim-torres added a commit to tim-torres/chuck that referenced this issue Feb 25, 2017
As requested in issue ccrma#43. This addresses but one issue of potentially many brought upon by not disallowing commas in array subscripts.
tim-torres added a commit to tim-torres/chuck that referenced this issue Feb 25, 2017
As requested in issue ccrma#43. This addresses but one issue of potentially many brought upon by not disallowing commas in array subscripts.
tim-torres added a commit to tim-torres/chuck that referenced this issue Feb 25, 2017
As requested in [issue ccrma#43](ccrma#43 (comment)). This addresses but one issue of potentially many brought upon by not disallowing commas in array subscripts.
@tim-torres
Copy link
Contributor

tim-torres commented Feb 25, 2017

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

spencersalazar pushed a commit that referenced this issue Jun 16, 2023
As requested in [issue #43](#43 (comment)). This addresses but one issue of potentially many brought upon by not disallowing commas in array subscripts.
@gewang
Copy link
Member

gewang commented Aug 30, 2023

thanks to @tim-torres and @spencersalazar, this issue is marked as resolved!

test.ck:2:14: error: [..., ...] is invalid subscript syntax.
[2] 54 => chordz[0,0];
                 ^

@gewang gewang closed this as completed Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants