Skip to content

t/compilers/imcc/syn/clash.t: Is test complete? #335

Closed
jkeenan opened this Issue Nov 21, 2009 · 4 comments

3 participants

@jkeenan
jkeenan commented Nov 21, 2009

In the file in question we see this:

173 pir_error_output_like( <<'CODE', <<'OUTPUT', 'new with a native type, no str    ing constant', todo => 'RT #51662 not done yet' );
174 .sub test :main
175         $P1 = new INTVAL
176     print "never\n"
177     end
178 .end
179 CODE
180 /error:imcc:syntax error, unexpected IDENTIFIER \('INTVAL'\)/
181 OUTPUT

[http://rt.perl.org/rt3/Ticket/Display.html?id=51662 RT #51662] was resolved, but the 'todo' reference was left in the test file.

This must be tracked in Trac.

Originally http://trac.parrot.org/parrot/ticket/1323

@plobsing

The error messages is currently:

error:imcc:The opcode 'new_p_ic' (new<2>) was not found. Check the type and number of the arguments


Here's why:


  • Barewords that aren't predefined variables are assumed to be labels.
  • The 'ic' op fullname arg code is used both for integer constants and labels.
  • Instruction selection occurs right as we parse the op, meaning we cannot know that there isn't a label 'INTVAL' somewhere.


Here's how we might improve the situation:


  • Make label op arguments take a prefix (eg: '&')
  • Make label op arguments represented by a different code in fullnames (eg: 'lbl')
  • Require predeclaration of labels
  • Parse all the way through a sub before selecting instructions


I'm open to suggestions.

@jkeenan
jkeenan commented Oct 17, 2010

Can anyone comment on plobsing's suggestions?

Thank you very much.

kid51

@rurban
Parrot Virtual Machine member
rurban commented Dec 21, 2012

From the 4 options:
1. Make label op arguments take a prefix (eg: '&')
2. Make label op arguments represented by a different code in fullnames (eg: 'lbl')
3. Require predeclaration of labels
4. Parse all the way through a sub before selecting instructions

only the forth seems to make sense.

But I am baffled that new can accept a label at all. labels should be restricted in the specs,
since the make only sense for a limited number of ops.

@rurban
Parrot Virtual Machine member
rurban commented Oct 13, 2014

I didn't change the parser to check for labels beforehand, rather fixed the test to check for the current error: error:imcc:wrong .const value and added the name of the unfound .const, here "a".
See rurban/new-INTVAL-gh335

@rurban rurban added a commit that referenced this issue Oct 13, 2014
@rurban rurban [test][imcc] fix GH #335 test todo, parser error on new INTVAL
Since INTVAL is a valid label name, and we do not want to scan down to look for all
existing labels, we just test for the existing error.
1407d0c
@rurban rurban added Patch and removed Todo labels Oct 13, 2014
@rurban rurban added a commit that referenced this issue Oct 13, 2014
@rurban rurban [test] fix GH #335 test todo, parser error on new INTVAL
Since INTVAL is a valid label name, and we do not want to scan down to look for all
existing labels, we just test for the existing error.
7c0f94c
@rurban rurban closed this Oct 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.