Repository navigation
Adr 20261007 grammar fixes - #370
Merged
Merged
Conversation
The classic code generator fixed relative JUMP targets, MYSELF in instance parameters, COPY of an undefined instance, SHARE of inherited classes and non-double USERVARS read through particle_getvar. These tests check that mccode-antlr behaves the same way. Cases where it does not yet are strict xfails, with the cause as the reason.
visitJumpPrevious and visitJumpNext called int() on the parse-tree node rather than its text, so any JUMP PREVIOUS(n) or NEXT(n) raised a TypeError while parsing. set_jump_absolute_targets now checks that every resolved target is a component of the instrument. A JUMP to an unknown name raised a bare IndexError, and a relative target past either end was used as is: past the start it would index the component list from the end. Both now raise an error naming the jumping component and its target, as the classic code generator does since McCode ADR_20261007_GRAMMAR_FIXES.
Both visitExpressionMyself methods read self.current_instance, which never exists, so MYSELF anywhere in an instrument raised an AttributeError. InstrVisitor now returns the name of the instance being built. Following McCode ADR_20261007_GRAMMAR_FIXES, MYSELF is an error in the instance's own parameters, which are visited before the instance exists, and valid from WHEN onwards. In instrument parameter defaults (InstrParametersVisitor) there is no instance, so it is always an error.
A class defined as DEFINE COMPONENT X INHERIT Y holds the same SHARE blocks as Y, and both were written out. An instrument using Slit and Diaphragm (INHERIT Slit) therefore defined slit_print_if and friends twice and did not compile. component_share_declarations now skips blocks it has already written, as the classic code generator does since McCode ADR_20261007_GRAMMAR_FIXES. Blocks compare by file, line and text, so only an inherited copy is skipped.
particle_getvar and particle_getuservar_byid read every user variable's memory as a double, so an int USERVAR read by, e.g., Monitor_nD user1="flag" gave garbage (3 became 1.48e-323) and reported success. Numeric scalar USERVARS are now converted with a cast. Arrays, pointers, structs and unrecognised typedefs are left out, so the getters report failure for them, as the classic code generator does since McCode ADR_20261007_GRAMMAR_FIXES. A type counts as numeric when every word of it is a C arithmetic type word, MCNUM, size_t or a <stdint.h> integer type, so a struct type whose name contains "int" is not mistaken for one. Ids from particle_getuservar_byid still count every user variable.
COPY(PREVIOUS) on the first instance, or COPY(name) of a name that is no instance, logged an error and then failed with "Undefined component type <class 'NoneType'>". It now raises "COPY of an undefined component instance PREVIOUS" (or the name) with the file and line, as the classic code generator does since McCode ADR_20261007_GRAMMAR_FIXES. An unresolvable PREVIOUS reference no longer goes on to look itself up by name, which logged a second, misleading error about an instance named None.
A RELATIVE reference to a name that is no component instance logged an error and then placed the instance as if it were ABSOLUTE. The classic code generator stops with "Reference to undefined component instance"; this now raises the same, with the file and line. As there, a PREVIOUS reference with no previous instance still falls back to ABSOLUTE.
Instr.finalize_flow_edges and _build_flow_edge_records each resolved a JUMP target by name when its absolute_target was not yet set, which it is not until translation. A JUMP PREVIOUS[(n)], NEXT[(n)] or MYSELF looked up a component named "PREVIOUS", "NEXT_2" or "MYSELF", found none, and gave no edge. Both now use one function that resolves these relative to the jumping instance, as set_jump_absolute_targets does, and named targets by name.
Elements of a literal vector parameter were written from Expr.value,
which turns anything that is not a number into its bare C text. An
instrument parameter in v={1, par} therefore became "v[1] = par;" and
the instrument did not compile. Each element is now printed as an
expression with the parameter prefix, as scalar parameters are, giving
_instrument_var._parameters.par. The classic code generator accepts
such vectors since McCode aa7196036. Constant elements are unchanged.
Before McCode v3.9 the runtime's -y re-applied every instrument parameter default after parsing the command line, overwriting values given there; from v3.9 it only fills in parameters that were not given. CI pins v3.8.7, so the vector test saw par=0.5 instead of par=0.25, and the two ITERATE tests only passed because iter=3 is the default. Those tests now give every parameter explicitly without -y, and the ITERATE tests use iter=4, so they check the parameter with either runtime.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implement fixes for grammar bugs identified as part of testing mccode-dev/McCode#2767 changes