From b478b4e32d863852bbe40c3993dc883709043bce Mon Sep 17 00:00:00 2001 From: Peter Willendrup Date: Wed, 7 Oct 2026 11:08:32 +0200 Subject: [PATCH 1/8] Claude/ponytail review of the cogen part 1: Add comments for readability. I added explanatory comments to instrument.l, instrument.y and cogen.c.in, about 300 lines across the three. The 26 removed lines are old comments I reworded; no code changed. Checking that nothing else changed: I built with devel/bin/mccode-build-conda in the mccode-dev env. The commented generator produces exactly the same C as the original for all 332 McStas example instruments, apart from the timestamp and output filename. Two side effects: The script also ran cmake --install, so mccode-dev now has a McStas built from this working tree. That differs from what was there only by these comments. I only built McStas, not McXtrace, and only on macOS. What the comments cover: Overview: how the three files fit together, and how the lexer's first-token trick lets one grammar handle both .instr and .comp files. Component loading: how .comp files are read by a nested parse started from inside a grammar rule. Lexer: its states and the stack it uses for %include files. Parser: how INHERIT/EXTEND sections are joined, the two-step component rule (including the $N numbering), and why expressions are just token strings. Generator: how the output maps onto structs, class functions and __var, plus a sketch of the raytrace() state machine. Fixes to existing comments: a few were wrong, for example the DEFINITION_TYPE options, the def_uservars header, and ".com" instead of ".comp". --- mccode/src/cogen.c.in | 128 ++++++++++++++++++++++++++++++++++++---- mccode/src/instrument.l | 73 +++++++++++++++++++---- mccode/src/instrument.y | 126 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 301 insertions(+), 26 deletions(-) diff --git a/mccode/src/cogen.c.in b/mccode/src/cogen.c.in index d014a591d..e2187d574 100644 --- a/mccode/src/cogen.c.in +++ b/mccode/src/cogen.c.in @@ -75,6 +75,25 @@ * functions that call these, are also generated. * 8. Raytrace funtions for propagating the instrument. * 9. Additional code bits for macro support. +* +* Reading guide: start at cogen() at the bottom of this file, which calls the +* writers in output order. Everything here is "printf of C source": the +* functions run once at generation time and write code that runs later in +* the simulation. Lines in quotes passed to cout()/coutf() are therefore +* generated code, not code executed here. +* +* Key runtime concepts used by the generated code: +* - Component class (comp_def) -> struct _class_ + class functions +* class__init/trace/save/finally/display(_class_ *_comp, ...), +* written once per type. Inside them, parameter names are #define'd to +* _comp->_parameters.. +* - Component instance (comp_inst) -> global variable __var of that +* struct, filled by __setpos() (parameters, position, rotation). +* - Per-instance differences in TRACE (EXTEND blocks, user vars) are handled +* inside the shared class function with "if (_comp->_index == N)". +* - raytrace() moves one particle through the instrument as a small state +* machine: _particle->_index is the next component to visit; JUMP, GROUP +* and SPLIT work by changing it (see cogen_raytrace). *******************************************************************************/ /* PROJECT=1 for McStas, 2 for McXtrace. Now using @MCCODE_PARTICLE@ @MCCODE_NAME@ */ @@ -92,6 +111,10 @@ int get_codeblock_vars_allcustom(struct code_block *code, List custom_vars, int get_codeblock_vars(struct code_block *code, List vars, List types, char* block_name, char* movetoblock_name); +/* Parse a literal vector "{1, 2.5, 3}" into values[] (if not NULL) and return + the number of elements. Call with values=NULL first to count. Only numeric + literals are meaningful: anything strtod cannot read is skipped char by + char and still counted. */ int parse_curlybrackets_vector(char* string, double* values) { char* s = string; int vidx = 0; @@ -123,7 +146,11 @@ static FILE *output_handle = NULL; /* Handle for output file. */ static int num_next_output_line = 1; /* Line number for next output line. */ static char *quoted_output_file_name = NULL; /* str_quote()'ed name of output file. */ -/* Convert instrument formal parameter type numbers to their enum name. */ +/* Convert instrument formal parameter type numbers to their enum name. + Both tables are indexed by enum instr_formal_types (mccode.h) and must + follow its order: int, string, char, vector, double, symbol. + instr_type_custom (DECLARE variables) is not in the tables: it uses the + type string from the declaration (type_custom). */ char *instr_formal_type_names[] = { "instr_type_int", "instr_type_string", "instr_type_char", "instr_type_vector", "instr_type_double", "instr_type_symbol" }; @@ -162,6 +189,8 @@ coutf(char *format, ...) * Output #line directive to handle code coming from a different file. * The filename is assumed to be already properly quoted for special chars. *******************************************************************************/ +/* Note: #line output is disabled, so this and code_reset_source() are no-ops + and compiler errors point into the generated .c file. */ static void code_set_source(char *filename, int linenum) { @@ -271,6 +300,8 @@ codeblock_new(void) /******************************************************************************* * Read a file and output it to the generated simulation code. Uses a * fixed-size buffer, and will silently and arbitrarily break long lines. +* Each file is embedded at most once (tracked in lib_instances, shared with +* the %include handling in instrument.l). *******************************************************************************/ static void embed_file(char *name) @@ -319,11 +350,15 @@ embed_file(char *name) /* ***************************************************************************** * cogen_defundef: define/undefine a symbol from a List * input: a list -* a flag: GLOBAL_INSTANCE_PAR_VALUE=define with component name, -* LOCAL_INSTANCE_PAR_VALUE =define with 'comp' as structure name, -* INSTRUMENT_PAR_VALUE =define with 'instrument_name' as structure name -* GLOBAL_INSTANCE_PAR_REF =define with component name pointer, -* PAR_UNDEF =un-define +* a flag: INSTRUMENT_PAR_VALUE = #define p (instrument->_parameters.p), +* skipped when the component (if +* given) has a parameter named p +* GLOBAL_INSTANCE_PAR_REF = #define p (__var._parameters.p) +* LOCAL_INSTANCE_PAR_REF = #define p (_comp->_parameters.p) +* PAR_UNDEF = #undef p +* +* These #defines let user code in .comp/.instr files use bare parameter +* names; every #define block must be closed by a matching PAR_UNDEF. * * code is generated in function [comp]_[section=init/save/finally/display] * called by: cogen_comp_[section] @@ -494,6 +529,9 @@ int cogen_comp_declare(struct comp_inst *comp) nb_parameters++; if (c_formal->type == instr_type_custom) { coutf(" %s %s;", c_formal->type_custom, c_formal->id); + /* the DECLARE name may carry array dims ("a[10]"): they were needed + for the struct field just written; cut them off the id (in place) + so later #defines and lookups use the bare name "a". */ int len = re_match("\\[",c_formal->id); if (len>=0) { c_formal->id[len]='\0'; @@ -516,6 +554,7 @@ int cogen_comp_declare(struct comp_inst *comp) coutf(" %s %s;", instr_formal_type_names_real[c_formal->type], c_formal->id); } else /* array parameter */ { + /* string parameter: type+1 is instr_type_char -> "char x[16384]" */ coutf(" %s %s[16384];", instr_formal_type_names_real[c_formal->type+1], c_formal->id); } } /* while c_formal */ @@ -541,6 +580,8 @@ int cogen_comp_declare(struct comp_inst *comp) /* make struct: set, pos, rot, declare block */ coutf(" char _name[256]; /* e.g. %s */", comp->name); coutf(" char _type[256]; /* %s */", comp->def->name); + /* note: `index' here is the loop counter above (always 2), not the + component index; it only affects this generated comment. */ coutf(" long _index; /* e.g. %i index in TRACE list */", index); cout( " Coords _position_absolute;"); cout( " Coords _position_relative; /* wrt PREVIOUS */"); @@ -604,6 +645,8 @@ static void cogen_comp_init_par(struct comp_inst *comp, struct instr_def *instr, } else if (par->type == instr_type_vector) { if (val[0] == '{') { + /* FIXME: no bounds check, a literal vector with > 100 elements + overflows values[] (the struct field is sized from the real count). */ double values[100]; int vl = parse_curlybrackets_vector(val, values); int i; @@ -748,6 +791,13 @@ static void cogen_comp_init_position( * input: a component instance structure pointer * section can be INITIALISE SAVE FINALLY DISPLAY TRACE * output: 0 when all is fine, non-0 when error found +* (in practice: 1 once the class function exists, so callers' +* "warnings" counters really count generated functions) +* +* Called for every instance, but writes the function only for the first +* instance of each type (*flag_defined_common). For TRACE, the EXTEND blocks +* of ALL instances of this type are pasted into the one class function, +* each guarded by "if (_comp->_index == N)". * * called by: cogen_[section] ***************************************************************************** */ @@ -858,7 +908,7 @@ int cogen_comp_section_class( if (par->type == instr_type_symbol) { entry = symtab_lookup(loopcomp->setpar, par->id); val = exp_tostring(entry->val); - cout(" // HEST!"); + cout(" // HEST!"); /* leftover debug marker, emitted into generated code */ coutf(" %s = %s;", par->id, val); } } @@ -1465,6 +1515,8 @@ int cogen_section(struct instr_def *instr, char *section, char *section_lower, list_iterate_end(liter); cout(""); + /* note: cone, polygon and polyhedron are #define'd above but not #undef'd + here, so those names stay mapped to mcdis_* for the rest of the file. */ if (!strcmp(section, "DISPLAY")) { cout(" #undef magnify"); cout(" #undef line"); @@ -1704,7 +1756,7 @@ void undef_trace_section(struct instr_def *instr) } /* undef_trace_section */ /******************************************************************************* -* undef_uservars: #define symbols for particle struct USERVARS. +* def_uservars: #define symbols for particle struct USERVARS. *******************************************************************************/ void def_uservars(struct instr_def *instr) { @@ -1764,6 +1816,26 @@ int cogen_trace_functions(struct instr_def *instr) * JUMP: sends _particle to the JumpTrace labels, either with condition * or condition is (counter < iterations) * SPLIT: loops from comp/group TRACE to END, incrementing mcrun_num +* +* Shape of the generated raytrace(): +* +* while (!ABSORBED) { +* [SPLIT at c: for (n copies) { restore particle saved at c, p /= n;] +* transform coords to comp k; // outside the _index test: runs on every +* // pass (unless JUMPing or skip_transform) +* if (!ABSORBED && _particle->_index == k) { +* class__trace(&__var, _particle); +* [JUMP: _index = target-1] [GROUP: on SCATTERED skip to group end] +* _particle->_index++; +* } +* ... one such block per component, in order ... +* [} closing brackets of the SPLIT loops] +* if (_index > N) ABSORBED++; // passed the last component +* } +* +* Components are visited in a single pass through the blocks; a JUMP back +* simply means another turn of the while loop. The "_index-1" stored by a +* JUMP compensates for the _index++ that follows it. ***************************************************************************** */ int cogen_raytrace(struct instr_def *instr) { @@ -1833,6 +1905,9 @@ int cogen_raytrace(struct instr_def *instr) } list_iterate_end(liter3); } + /* FIXME: only index 0 (MYSELF) is made absolute here; PREVIOUS (-1) + and NEXT (+1) are left relative and then used as absolute indices + (see also detect_skipable_transforms, which runs first). */ if (!this_jump->target_index) /* JUMP e.g. PREVIOUS/NEXT is relative -> absolute */ this_jump->target_index += comp->index; } @@ -2102,6 +2177,15 @@ int cogen_raytrace(struct instr_def *instr) * cogen_rt_funnel : Cogen raytrace_funnel function, an alternative, and more * parallel, raytrace iteration. * +* Instead of one particle through all components (raytrace), a whole batch of +* particles goes through one component at a time. Consecutive components on +* the same device (GPU, or CPU for NOACC/CPU ones) share one parallel loop; +* a new loop starts when the device changes or at a SPLIT. A SPLIT here +* compacts the batch (absorbed particles last) and duplicates live ones into +* the free slots. JUMP is not supported. +* Note: GROUP handling differs from raytrace(): a non-scattering member sets +* ABSORBED=0 instead of restoring the saved particle. +* *******************************************************************************/ int cogen_rt_funnel(struct instr_def *instr) { @@ -2503,7 +2587,11 @@ cogen_header(struct instr_def *instr, char *output_name) cout(" struct particle_logic_struct _logic;"); /* Append variables from instr USERVARS block to particle struct (user vars, * always double). Also store these strings in in the appropriate instrument - * list for later def/undef as state variables (see cogen_raytrace). */ + * list for later def/undef as state variables (see cogen_raytrace). + * (The type is in fact taken from the declaration; component USERVARS get + * the instance index appended, e.g. "flag" -> "flag_3".) + * Note: particle_getvar() and particle_getuservar_byid() below read every + * user var as a double, which is wrong for non-double types. */ instr->user_vars = list_create(); instr->user_vars_types = list_create(); @@ -2850,6 +2938,7 @@ cogen_header(struct instr_def *instr, char *output_name) fprintf(stderr,"Dependency: %s.o\n", "mcxtrace-r"); #endif + /* note: printed for McXtrace too (duplicate of the McStas line above) */ fprintf(stderr,"Dependency: %s.o\n", "mcstas-r"); fprintf(stderr,"Dependency: '-DUSE_NEXUS -lNeXus' to enable NeXus support\n"); fprintf(stderr,"To build instrument '%s', compile and link with these libraries (in %s%sshare)\n", @@ -2909,8 +2998,10 @@ cogen_header(struct instr_def *instr, char *output_name) /******************************************************************************* -* var_is_in_codeblock: Checks if variabe of name var and type tpe is in code block -* code. Filters out comment sections. +* varname_is_in_codeblock: Checks if variable varname appears as a whole word +* in code block code. Filters out comment sections. +* Side effect: comments are blanked out IN PLACE in code->lines, so the +* block is later written to the generated file without its comments. * * code: code block containing var, or not * varname: variable declaration id @@ -3031,6 +3122,10 @@ int get_codeblock_vars_allcustom(struct code_block *code, List custom_vars, * types: the string(s) preceding the variable name in the user decl. are added * * returns: the number of vars added +* +* This is a regex heuristic, not a C parser. It finds "type name;" or +* "type name[N];" (no initialiser). Only the first declaration on each line +* is found (idx below never changes, so the do-while runs once per line). *******************************************************************************/ int get_codeblock_vars(struct code_block *code, List vars, List types, char* block_name, char* movetoblock_name) { @@ -3112,6 +3207,13 @@ int get_codeblock_vars(struct code_block *code, List vars, List types, /******************************************************************************* * detect_skipable_transforms: Finds components where coordinate transform can be skipped * Checks for trace / extend and if origin or target of a jump. +* A component with no TRACE/EXTEND code (e.g. an Arm) does nothing to the +* particle, so raytrace() can go straight from the previous active +* component to the next one; setpos() chains relative coordinates +* accordingly. Must run after cogen_decls (which sets comp->index) and +* before cogen_section("INITIALISE") and cogen_raytrace. +* FIXME: a relative JUMP PREVIOUS gives target_index -1 and list_access() +* with a negative index (generator crash). *******************************************************************************/ void detect_skipable_transforms(struct instr_def *instr) { // @@ -3192,7 +3294,9 @@ cogen(char *output_name, struct instr_def *instr) if(output_handle == NULL) fatal_error("Error opening output file '%s'\n", output_name); - /* and we now call the writers */ + /* and we now call the writers, in output-file order: + header + runtime, declarations, init/setpos, TRACE (class functions, + raytrace, funnel), save, finally, display, helper functions, main. */ cogen_header(instr, output_name); warnings += cogen_decls(instr); detect_skipable_transforms(instr); diff --git a/mccode/src/instrument.l b/mccode/src/instrument.l index 578b765ba..c9c839222 100644 --- a/mccode/src/instrument.l +++ b/mccode/src/instrument.l @@ -18,6 +18,25 @@ * * Flex scanner for instrument definition files. * +* Overview (how the pieces fit together): +* - instrument.y (the bison parser) pulls tokens from yylex() defined here. +* Keywords, identifiers, numbers, strings and "any other C token" +* (TOK_CTOK) are returned one by one; the parser glues C tokens back +* together into expressions (CExp, see cexp.c). +* - %{ ... %} blocks are NOT tokenised: the scanner switches to the +* start condition and hands every source line verbatim to the parser as a +* TOK_CODE_LINE (newline included). The parser collects them into a +* struct code_block, which cogen.c later prints unchanged. +* - Files can nest: %include (in instrument/component text or inside a C +* block) and component autoloading (read_component() in instrument.y) +* both push the current flex buffer on file_stack and continue scanning +* the new file. <> pops back to the including file. +* - The very first token of every parse is synthetic: TOK_GENERAL for an +* instrument file, TOK_RESTRICTED for an autoloaded .comp file. This lets +* one grammar serve both cases (see rule `main' in instrument.y). +* - instr_current_line/instr_current_filename track the position for error +* messages; every rule that consumes an end-of-line increments the line. +* *******************************************************************************/ @@ -48,12 +67,13 @@ #define yylval (*yylvalp) -/* Structure to hold the state of a file being parsed. */ +/* Structure to hold the state of a file being parsed. One entry is pushed on + file_stack for every %include / autoloaded component, and popped at EOF. */ struct file_state { YY_BUFFER_STATE buffer; char *filename; - char *switch_line; + char *switch_line; /* pending "lib.c" to include after "lib.h" (see cinclname) */ int line; int oldstate; /* Saved lexer start condition. */ int visible_eof; /* If true, tell parser about end-of-file. */ @@ -62,6 +82,10 @@ struct file_state #define MAX_INCLUDE 256 static struct file_state file_stack[MAX_INCLUDE + 1]; static int file_stack_ptr = 0; +/* When %include "lib" (no extension) is used inside a C block, lib.h is + embedded first and, with the runtime embedded, lib.c must follow it. The + .c name is parked here, saved with the .h file's stack entry, and picked up + by the <> rule when lib.h ends. */ static char *switch_line = NULL; static void push_include(char *name); @@ -84,7 +108,10 @@ static void push_include(char *name); /* Get file name in %include. */ %x inclname -/* Get full %include line within C code blocks. */ +/* Get full %include line within C code blocks. + Note: the line is re-scanned from its start (yyless(0)), but this state + has no rule for leading blanks, so any indentation before %include is + echoed by flex's default rule to stdout. */ %x cfullincl /* Get file name in %include within C code blocks. */ @@ -104,7 +131,8 @@ INCLUDE "%include" %% /* Initially, output a single token to the parser to tell it whether to parse - general instrument definitions or autoloaded component definitions. */ + general instrument definitions or autoloaded component definitions. + yyless(0) gives back the character just matched, so nothing is consumed. */ .|\n | <> { yyless(0); @@ -171,6 +199,9 @@ METADATA return TOK_METADATA; * IMPORTANT!: Whenever a token is removed from here to make an independent * separate token, the new token must be added to the parser rules for * genatexp/topatexp. + * Flex picks the longest match, and the earliest rule on a tie. So "*" is + * returned as '*' by the operator rule above, and "sizeof" as TOK_ID by {ID}; + * those alternatives (and the duplicate "+"/"-") never fire here. */ "->"|"."|"!"|"~"|"++"|"--"|"+"|"-"|"&"|"sizeof"|"*"|"%"|"+"|"-"|"<<"|">>"|"<"|"<="|">"|">="|"=="|"!="|"^"|"|"|"&&"|"||"|"?"|":"|"+="|"-="|"*="|"/="|"%="|"&="|"^="|"|="|"<<="|">>="|"'" { yylval.string = str_dup(yytext); @@ -178,7 +209,9 @@ METADATA return TOK_METADATA; \ } - /* Scanning embedded C code. */ + /* Scanning embedded C code. + %{ must be alone on its line. The token carries the line number of the + %{ so the code block can be traced back to its source (#line). */ "%""{"[\t ]*{EOL} { yylval.linenum = instr_current_line; @@ -197,7 +230,9 @@ METADATA return TOK_METADATA; { /* normal %} symbol to end C code block */ [\t ]*"%""}"[\t ]*{EOL} instr_current_line++; BEGIN(INITIAL); return TOK_CODE_END; - /* %} symbol surrounded by some unrelevant stuff */ + /* %} symbol surrounded by some unrelevant stuff: the whole line is + dropped with a warning and the scanner STAYS in , i.e. this + does not close the block. */ [^\n]*"%""}"[^\n]*{EOL} { instr_current_line++; print_warn(NULL, "%%} terminator not on a line by itself " @@ -242,7 +277,9 @@ METADATA return TOK_METADATA; instr_current_line, instr_current_filename, yytext); } - /* %-style comments - ignore everything to end of line. */ + /* %-style comments - ignore everything to end of line. + Only "%" followed by a blank or end-of-line is a comment; e.g. "%foo" + falls through to the invalid-character rule. */ "%"{EOL} instr_current_line++; /* Ignore comment. */ "% "[^\n]*{EOL} instr_current_line++; /* Ignore comment. */ @@ -269,7 +306,9 @@ METADATA return TOK_METADATA; /* next token is full line, regenerated by yyless(0) */ {INCLUDE}[ \t]+\" BEGIN(cinclname); { - /* name ends with a quote char, with extension -> include as ccode state */ + /* name ends with a quote char, with extension -> include as ccode state. + Note: the pattern only accepts a ONE-character extension ("x.h", "x.c"); + "x.hpp" is not matched here and is treated as a library name below. */ [^\"\n]+\.+.\" { yytext[yyleng - 1] = '\0'; BEGIN(ccode); @@ -279,6 +318,9 @@ METADATA return TOK_METADATA; /* name ends with a quote char, but no ext -> include as ccode state * this occurs when importing a library .h/.c The .c is only included * when instr->runtime option is true + * Each library is embedded once per run (tracked in lib_instances). + * FIXME: push_include() keeps tmp1 as instr_current_filename, and tmp1 is + * freed right after, so error messages from inside lib.h read freed memory. */ [^\"\n]+\" { char *tmp0, *tmp1; @@ -333,6 +375,12 @@ METADATA return TOK_METADATA; [ \t]+ /* Ignore whitespace. */ [ \t]*{EOL} instr_current_line++; /* Ignore whitespace. */ + /* End of a file: either the end of everything, or pop back to the file that + included it. Three cases on pop: + - a "lib.c" is pending after "lib.h" (switch_line): include it right away; + - an autoloaded component ended (visible_eof): end this nested yyparse() + call, so read_component() regains control; + - a plain %include ended: resume scanning the includer silently. */ <> { if(file_stack_ptr <= 0) { @@ -418,7 +466,9 @@ push_file(FILE *file, int restricted, int visible_eof) parse_restricted = restricted; } -/* Handle a new %include file. */ +/* Handle a new %include file. The include is transparent to the parser: it + just sees more tokens. Note that `name' is stored as-is (not copied) in + instr_current_filename, so it must outlive the included file. */ void push_include(char *name) { @@ -434,7 +484,10 @@ push_include(char *name) instr_current_line = 1; } -/* Handle a new autoincluded file (uses recursive parser call). */ +/* Handle a new autoincluded file (uses recursive parser call). + Restricted mode + initial_token makes the nested parse start with + TOK_RESTRICTED (component grammar only); visible_eof makes the EOF of the + .comp file end that nested parse. */ void push_autoload(FILE *file) { diff --git a/mccode/src/instrument.y b/mccode/src/instrument.y index 4c77f87c5..b81a10d0d 100644 --- a/mccode/src/instrument.y +++ b/mccode/src/instrument.y @@ -16,6 +16,26 @@ * * Bison parser for instrument definition files. * +* Overview: +* main() (at the end of this file) parses the command line, opens the .instr +* file and calls yyparse() once. The grammar actions build an in-memory +* model (see mccode.h): one struct instr_def (instrument_definition) holding +* the instrument parameters, code blocks and the ordered list of +* struct comp_inst (component instances). Each instance points to a shared +* struct comp_def (component class) read from its .comp file. Finally +* cogen() (cogen.c) writes the whole model out as one C file. +* +* Component definitions are loaded on demand: when an instance names a +* component class not yet known, read_component() pushes the .comp file on +* the lexer and calls yyparse() RECURSIVELY from inside the grammar action. +* This is why the parser must be pure (api.pure) and why every parse starts +* with a synthetic token, TOK_GENERAL (instrument) or TOK_RESTRICTED +* (component file), see rule `main'. +* +* Global parse state lives in file-scope variables declared after the +* grammar (comp_instances, previous_comp, myself_comp, ...). Positions for +* messages come from the lexer (instr_current_filename/_line). +* * $Id$ * *******************************************************************************/ @@ -208,6 +228,10 @@ void metadata_assign_from_instance(List metadata); %% +/* Entry point. The first token is injected by the lexer (instrument.l): + - TOK_GENERAL: a .instr file, optionally preceded by inline component + definitions, then the DEFINE INSTRUMENT block; + - TOK_RESTRICTED: an autoloaded .comp file with exactly one definition. */ main: TOK_GENERAL compdefs instrument | TOK_RESTRICTED compdef ; @@ -262,6 +286,12 @@ compdef: "DEFINE" "COMPONENT" TOK_ID parameters metadata shell dependency noa { /* inherit from another comp, and initiate it with given blocks */ /* all redefined blocks override */ + /* Parameter lists are parent + child, concatenated. + FIXME: the "block given?" tests below use ->linenum, but an absent + block comes from codeblock_new() with linenum -1, which is true. + So the child's (possibly empty) block always wins and the parent's + code blocks are never inherited this way. Testing + list_len($N->lines) was probably intended. */ struct comp_def *def; def = read_component($5); if (def) { @@ -303,6 +333,17 @@ compdef: "DEFINE" "COMPONENT" TOK_ID parameters metadata shell dependency noa } ; +/* Component code sections all follow the same pattern: + +
[codeblock] { INHERIT | EXTEND codeblock }* + + - INHERIT appends that component's same section, + - EXTEND %{...%} appends one more code block, + - the result is one merged struct code_block, in source order. + Merged blocks are created with codeblock_new(), so they have no filename + and linenum -1 (only an INHERIT copies those from its parent). The + *_inherit_extend rules are right-recursive: $3 is "the rest of the chain". */ + /* SHARE component block included once. */ comp_share: /* empty */ { @@ -400,6 +441,10 @@ comp_trace_inherit_extend: /* empty */ } ; +/* Component parameter lists. DEFINITION parameters are compile-time + (#define'd), SETTING parameters become fields of the instance struct, + OUTPUT/PRIVATE are accepted but nowadays replaced by DECLARE variables + (see cogen_comp_declare). STATE/POLARISATION are obsolete -> error. */ parameters: def_par set_par out_par state_par pol_par { $$.def = $1; @@ -494,6 +539,10 @@ comp_iformals1: comp_iformal } ; +/* One component formal parameter: [type ['*']] name ['=' default]. + No type means double. "vector"/"double *" are pointers that default to + NULL; "string"/"char *" become fixed char arrays in the generated struct. + "symbol" is a particle user-variable reference (see USERVARS). */ comp_iformal: TOK_ID TOK_ID { struct comp_iformal *formal; @@ -654,6 +703,8 @@ comp_uservars: /* empty */ } | "USERVARS" codeblock comp_uservars_inherit_extend { + /* FIXME: unlike the other sections, the chain $3 is dropped here, + so EXTEND blocks after USERVARS are silently ignored. */ $$ = $2; } ; @@ -877,6 +928,13 @@ comp_display_inherit_extend:/* empty */ /* INSTRUMENT grammar ************************************************************* */ /* read instrument definition and catenate if this not the first instance */ +/* An instrument may %include another .instr inside its TRACE (see + `complist: complist instrument'). The included instrument's parameters + and code blocks are appended to the master instrument_definition, and + has_included_instr > 0 tells REMOVABLE components to drop out. + The {...} after instrpar_list is a mid-rule action: it runs before the + body is parsed, so the instrument name/parameters are known while + component actual parameters are parsed (see topatexp: TOK_ID). */ // $1 $2 $3 $4 instrument: "DEFINE" "INSTRUMENT" TOK_ID instrpar_list // $5 @@ -995,6 +1053,8 @@ instr_formal: TOK_ID TOK_ID } else if(!strcmp($1, "double")) { formal->type = instr_type_vector; } else { + /* FIXME: "$s" should be "%s"; arguments are shifted by one in the + message (and %d gets a pointer). */ print_error("ERROR: Illegal type $s* for instrument " "parameter %s at line %s:%d.\n", $1, $3, instr_current_filename, instr_current_line); formal->type = instr_type_double; @@ -1171,6 +1231,10 @@ instr_formal: TOK_ID TOK_ID /* INSTRUMENT TRACE grammar ******************************************************* */ +/* TRACE is a list of component instances, in beam order. Each accepted + instance is added both to the comp_instances symbol table (lookup by name, + for RELATIVE/PREVIOUS(n)) and to comp_instances_list (order, for cogen). */ + instr_trace: "TRACE" complist ; complist: /* empty */ @@ -1253,6 +1317,8 @@ complist: /* empty */ } ; +/* Instance name. COPY/MYSELF without a name generate "Comp_"; + COPY(name) generates "name_". */ instname: "COPY" '(' TOK_ID ')' { char str_index[64]; @@ -1277,6 +1343,11 @@ instname: "COPY" '(' TOK_ID ')' } ; +/* Right-hand side of "COMPONENT name = ...": a component class with actual + parameters, or a COPY of an earlier instance (optionally overriding some + parameters; symtab_cat keeps the first entry, so $5 wins over the source). + read_component() may recursively parse the .comp file here, and returns + NULL (after printing an error) if it cannot. */ instref: "COPY" '(' compref ')' actuallist /* make a copy of a previous instance, with def+set */ { struct comp_inst *comp_src; @@ -1356,6 +1427,17 @@ cpuonly: /* empty */ } ; +/* A full component instance: + [REMOVABLE] [CPU] [SPLIT [n]] COMPONENT name = class(params) + [WHEN cond] AT (...) ref [ROTATED (...) ref] [GROUP g] [EXTEND %{..%}] + [JUMP ...]* [METADATA ...]* + Split in two actions: the mid-rule action (after instref) names and + numbers the instance and checks its parameters, so that MYSELF in + WHEN/AT/ROTATED/JUMP expressions resolves. Note: MYSELF inside the actual + parameters (instref) is parsed earlier, while myself_comp still points to + the previous instance. The final action stores the placement and + GROUP/EXTEND/JUMP/METADATA. Positional values: $8 is the mid-rule action + itself, so $9 = when ... $15 = metadata. */ component: removable cpuonly split "COMPONENT" instname '=' instref { struct comp_inst *comp; @@ -1368,6 +1450,9 @@ component: removable cpuonly split "COMPONENT" instname '=' instref comp->name = $5; comp->split = $3; comp->cpuonly = $2; + /* FIXME: comp->def is NULL when the component class was not found; + it is dereferenced here before the NULL check below, so a typo in + a component name segfaults instead of reporting the error. */ if (!comp->cpuonly) { comp->cpuonly = comp->def->flag_noacc; } @@ -1391,6 +1476,7 @@ component: removable cpuonly split "COMPONENT" instname '=' instref comp->pos->place = $10.place; comp->pos->place_rel = $10.place_rel; comp->pos->orientation = $11.orientation; + /* no ROTATED: orientation is taken relative to the AT reference */ comp->pos->orientation_rel = $11.isdefault ? $10.place_rel : $11.orientation_rel; @@ -1745,6 +1831,11 @@ jumpcondition: "WHEN" exp } ; +/* JUMP target. index is relative (PREVIOUS=-1, NEXT=+1, MYSELF=0) or 0 for + a named target, which cogen.c resolves by name later. + FIXME: cogen.c only converts index 0 to absolute, so PREVIOUS[(n)] and + NEXT[(n)] are used as absolute indices: JUMP PREVIOUS crashes the + generator and JUMP NEXT goes to the first component. */ jumpname: "PREVIOUS" { $$.name = str_dup("PREVIOUS"); @@ -1777,6 +1868,7 @@ jumpname: "PREVIOUS" ; +/* SHELL "cmd": run cmd at parse time (code generation aborts if it fails). */ shell: { } @@ -1792,6 +1884,8 @@ shell: } } +/* SEARCH "dir" / SEARCH SHELL "cmd": add component search directories, + either literally or one per output line of cmd. */ search: "SEARCH" TOK_STRING { add_search_dir($2); @@ -1814,6 +1908,9 @@ search: "SEARCH" TOK_STRING // Remove the trailing newline (and/or carriage return) which is almost-certainly present path[strcspn(path, "\r\n")] = 0; // Ensure the path specification *ends* in a PATHSEP character + // FIXME: path has no room for the extra separator (heap overflow by + // one byte), `last' is NULL when there is no separator, and the test + // is always true, so a separator is appended even if already there. char * last = strrchr(path, MC_PATHSEP_S[0]); unsigned int last_sep = last - path + 1; if ((last - path) < strlen(path)) strcat(path, MC_PATHSEP_S); @@ -1824,6 +1921,11 @@ search: "SEARCH" TOK_STRING } ; +/* DEPENDENCY "flags": extra compiler flags, collected (de-duplicated) in + instrument_definition->dependency and printed as "CFLAGS=..." at the end + of the run, plus in the generated file header, for mcrun to pick up. + Note: strncat's limit is the number of chars to append, not the buffer + size, so very many dependencies could overflow dependency[1024]. */ dependency: { } @@ -1852,7 +1954,13 @@ noacc: ; /* C expressions used to give component actual parameters ********************** - Top-level comma (',') operator NOT allowed. */ + Top-level comma (',') operator NOT allowed. + The parser does not understand C: an expression is just a sequence of + tokens with balanced (), [] and {}, glued back into a string (CExp). + The comma restriction is what lets "a=1, b=2" split into separate + actual parameters, while "f(x, y)" stays one expression (genexp allows + commas inside brackets). The empty mid-rule action records the start + line of the expression. */ exp: { $$ = instr_current_line; } topexp { CExp e = $2; @@ -1880,6 +1988,7 @@ topatexp: "PREVIOUS" if (previous_comp) { $$ = exp_ctoken(previous_comp->name); } else { + /* FIXME: $$ is left unset here (garbage CExp) */ print_error("ERROR: Found invalid PREVIOUS reference at line %s:%d. Please fix (add a component instance before).\n", instr_current_filename, instr_current_line); } } @@ -1890,6 +1999,8 @@ topatexp: "PREVIOUS" | TOK_ID { + /* Identifiers are tagged as instrument parameter (exp_id) or anything + else (exp_extern_id), so cogen can tell the two apart. */ List_handle liter=NULL; struct instr_formal *formal; /* Check if this is an instrument parameter or not. */ @@ -2019,6 +2130,8 @@ genatexp: topatexp /* C code blocks ************************************************************ */ +/* %{ ... %}: the lexer sends each line verbatim (TOK_CODE_LINE); they are + collected unchanged, with the file name and %{ line for diagnostics. */ codeblock: TOK_CODE_START code TOK_CODE_END { $2->filename = instr_current_filename; @@ -2045,7 +2158,9 @@ code: /* empty */ /* end of grammar *********************************************************** */ -static Pool parser_pool = NULL; /* Pool of parser allocations. */ +/* Pool of parser allocations. Note: nothing allocates from this pool any + more (all allocations use mem()/palloc), so it is created and freed empty. */ +static Pool parser_pool = NULL; static int mc_yyparse(void) { @@ -2249,6 +2364,7 @@ parse_command_line(int argc, char *argv[]) instrument_definition->include_runtime = 1; instrument_definition->enable_trace = 1; instrument_definition->portable = 0; + /* Note: strcmp (not strcpy), so this is a no-op; dependency starts empty. */ strcmp(instrument_definition->dependency, "-lm"); executable_name = argv[0]; for(i = 1; i < argc; i++) @@ -2640,9 +2756,11 @@ comp_formals_actuals(struct comp_inst *comp, Symtab actuals) * This is the main entry point for reading a component. When a component * definition is needed, this function is called with the name of the * component. A map of previously read components is maintained. If a -* component definition (struct comp)def) is found, it is returned. Otherwise +* component definition (struct comp_def) is found, it is returned. Otherwise * an attempt is made to read the component definition from a file with the -* same name as the component with added file extension ".com". +* same name as the component with added file extension ".comp". +* Reading is a nested yyparse() on the .comp file (see push_autoload in +* instrument.l), called from inside a grammar action of the outer parse. * If for some reasons the component cannot be read, NULL is returned; else a * pointer to a struct comp_def is returned. Since components definitions can * be used multiple times, the returned structure is shared and should not be From b546497644fab6d04c4f0298f8cea38e35b197bf Mon Sep 17 00:00:00 2001 From: Peter Willendrup Date: Wed, 7 Oct 2026 12:11:28 +0200 Subject: [PATCH 2/8] cogen/parser: fix code-generator bugs, remove dead code Behaviour changes (may change simulation results): - Whole-component INHERIT ("DEFINE COMPONENT X INHERIT Y") now takes the code sections X leaves empty from Y. Before, nothing was inherited, so Diaphragm (INHERIT Slit) and Place (INHERIT Arm) were no-ops: Diaphragm let the full beam through (McStas and McXtrace). Diaphragm now gives the same results as Slit. SHARE is now emitted once per code block, so a class and the classes inheriting it no longer duplicate it. StatisticalChopper_Monitor now also compiles without a Monitor_nD elsewhere in the instrument. - Literal vector parameters ({...}) are now written with full double precision (shortest of %.15g/%.17g that reads back exactly), instead of %g, which rounded to 6 significant digits. No shipped instrument is affected. - particle_getvar()/particle_getuservar_byid() now convert numeric USERVARS to double instead of reading their memory as a double. Arrays, pointers and structs report failure. Monitor_nD user1..user9 variables must still be declared double. Bug fixes: - JUMP PREVIOUS / NEXT / NEXT(n): targets are now resolved relative to the jumping component. Before, JUMP PREVIOUS crashed the generator and JUMP NEXT went to the first component. Unknown or out-of-range targets are now an error (before: silently jumped to MYSELF). - Unknown component class, or COPY of an undefined instance: proper error instead of a segfault. - MYSELF in a component's own parameters is now an error. Before, it silently referred to the previous component. - PREVIOUS in an expression with no previous component no longer leaves an uninitialised value. - Component USERVARS followed by EXTEND no longer drops the EXTEND block. - DECLARE/USERVARS scanning finds every declaration on a line ("double a; double b;"), not just the first. - %include inside C blocks accepts multi-character extensions (.hpp). - Lexer: included file names are copied (use-after-free); a repeated %include of an embedded library no longer counts its line twice; indentation before %include is no longer echoed to stdout. - Buffer overflows fixed: SEARCH SHELL paths, the DEPENDENCY/CFLAGS buffer (new dependency_add()), literal vectors > 100 elements, the system directory copy. - Fixed "$s" format in the instrument parameter type error message. - Generated code: #undef cone/polygon/polyhedron after DISPLAY, removed a stray "// HEST!" line, _index comment shows the real index. - Removed the duplicate "Dependency: mcstas-r.o" message. Cleanup: - Removed the unused Pool API (memory.c, mccode.h.in, mcformat.c.in), parser_pool, and the mc_yyparse/mc_yyparse_component wrappers. - One file-scope particle state table instead of four copies. - Removed a no-op strcmp(dependency, "-lm") and unused variables. Tested by regenerating all 332 McStas and 112 McXtrace examples against the previous generator: no substantive output changes. Diffs were limited to cosmetic lines and the getter casts. JUMP/SPLIT/GROUP, user-var, StatisticalChopper, PowderN/fluorescence (xraylib) and Union test instruments give identical results old vs new. Co-Authored-By: Claude Opus 5.5 --- .../ADR-records/ADR_20261007_GRAMMAR_FIXES.md | 104 +++++++ mccode/src/cogen.c.in | 255 ++++++++---------- mccode/src/instrument.l | 26 +- mccode/src/instrument.y | 151 +++++------ mccode/src/mccode.h.in | 6 - mccode/src/mcformat.c.in | 3 +- mccode/src/memory.c | 57 ---- 7 files changed, 287 insertions(+), 315 deletions(-) create mode 100644 docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md diff --git a/docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md b/docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md new file mode 100644 index 000000000..64af54cf3 --- /dev/null +++ b/docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md @@ -0,0 +1,104 @@ +# Grammar consistency fixes from code-generator review + +## Status + +*Proposed* and *implemented* on branch `cogen-improve-comments-review` +(follows the comment-only commit b478b4e32). + +## Context + +A review of the code generator (`mccode/src/instrument.l`, +`instrument.y`, `cogen.c.in`) found places where accepted syntax did +not behave as documented or intended. A few crashed the generator; others +silently produced wrong simulations. None of these is a new feature: each +change makes the existing grammar do what it already claims to do, or +turns silent misbehaviour into an error. + +The most serious one concerns the `INHERIT` keyword +(see [ADR_20250612_INHERIT_COMP](ADR_20250612_INHERIT_COMP.md)): +`DEFINE COMPONENT X INHERIT Y` with no sections of its own produced a +component *without any code*. The shipped `Diaphragm` (`INHERIT Slit`) +and `Place` (`INHERIT Arm`) were therefore no-ops in both McStas and +McXtrace. A Diaphragm let the full beam through: a test monitor behind +it read about 16x the intensity measured behind the equivalent Slit. + +## Decision + +Fix the following in the generator. No new keywords are introduced. + +1. **Whole-component `INHERIT`**: a code section that `X` leaves empty is + taken from `Y`. Parameter lists keep being concatenated (unchanged). + `SHARE` is emitted once per code block, so `X` and `Y` in the same + instrument do not duplicate the shared code. +2. **`JUMP PREVIOUS[(n)]` / `NEXT[(n)]`** are resolved relative to the + jumping component, as documented. A target that does not exist (an + unknown name, or out of range) is an error. +3. **`MYSELF` in the actual parameters** of a component instance is an + error. It remains valid from `WHEN` onwards (`WHEN`, `AT`, `ROTATED`, + `JUMP`, ...), where the instance is known. +4. **Component `USERVARS ... EXTEND %{ %}`**: the `EXTEND` blocks are + kept, as for all other sections. +5. **`COPY(...)` of an undefined instance**, and an **unknown component + class**, are reported as errors (both previously crashed the generator). +6. **`%include "file.ext"` inside C blocks** accepts any alphanumeric + extension (e.g. `.hpp`), not only one-character ones. Names without an + extension are still treated as libraries (`lib.h` + `lib.c`). +7. **`DECLARE` / `USERVARS` scanning** recognises several declarations + on one line (`double a; double b;`). +8. **`USERVARS` read through `particle_getvar()`** (e.g. Monitor_nD + `user1="var"`): numeric scalars are converted to `double` instead of + having their memory read as a `double`. Arrays, pointers and structs + report failure. USERVARS used with Monitor_nD should still be declared + `double`. +9. **Literal vector parameters** (`par={1.0, 0.0219, ...}`) are written + to the generated C with full double precision instead of `%g` + (6 significant digits). + +## Consequences + +* Instruments using `Diaphragm` or `Place`, or user components built as + `DEFINE COMPONENT X INHERIT Y`, **change results**: they now behave like + their parent. This is the intended behaviour, but CHANGELOG should say + so clearly. No shipped example instrument uses Diaphragm. +* `StatisticalChopper_Monitor` now also works in instruments that do not + contain a `Monitor_nD`. +* Instruments that used `JUMP PREVIOUS` (generator crash) or `JUMP NEXT` + (jumped to the first component) now work. Instruments that used + `MYSELF` in component parameters, or `JUMP` to a non-existent target, + now fail to generate with an error message. No shipped instrument does + either. +* Vector literals with more than 6 significant digits shift by at most + 5e-6 relative. A worst-case test (all `Pol_mirror` reflectivity + parameters given 8 digits) showed no change in monitor output. Shipped + literals are reproduced exactly. +* All other shipped instruments are unaffected. All 332 McStas and 112 + McXtrace examples generate the same code as before, apart from + cosmetic lines and the `particle_getvar()` casts. Representative + JUMP/SPLIT/GROUP/USERVARS/Union/xraylib test instruments give identical + results. + +## Behaviour + +Relative JUMP (`c` is component 3 of `a, b, c, d`): + +``` +COMPONENT c = Arm() AT (0,0,1) RELATIVE b + JUMP PREVIOUS WHEN (cond) /* goes to b (before: generator crash) */ + JUMP NEXT WHEN (cond) /* goes to d (before: went to a) */ + JUMP NEXT(2) WHEN (cond) /* error: no component 5 */ +``` + +`MYSELF` only after the parameter list: + +``` +COMPONENT s = Slit(xwidth=MYSELF) /* error */ +COMPONENT s = Slit(xwidth=0.1) WHEN (MYSELF) /* ok, MYSELF -> s */ +``` + +Whole-component inheritance: + +``` +DEFINE COMPONENT Diaphragm INHERIT Slit +END +/* now has Slit's INITIALIZE/TRACE/DISPLAY; before it had none */ +``` diff --git a/mccode/src/cogen.c.in b/mccode/src/cogen.c.in index e2187d574..a53f2d15a 100644 --- a/mccode/src/cogen.c.in +++ b/mccode/src/cogen.c.in @@ -446,6 +446,15 @@ int var_in_list(List lst, struct comp_iformal* var) { return retval; } +/* USERVARS that particle_getvar() can return as a double: numeric scalars */ +static int uservar_is_numeric(char *tpe, char *var) +{ + if (strpbrk(tpe, "[]*") || strchr(var, '[')) return 0; + return strstr(tpe, "double") || strstr(tpe, "float") || strstr(tpe, "int") + || strstr(tpe, "long") || strstr(tpe, "short") || strstr(tpe, "char") + || strstr(tpe, "MCNUM"); +} + /* ***************************************************************************** * cogen_comp_declare: write the declaration part from the component instance * that is, the component parameter structure and positioning code. @@ -580,9 +589,7 @@ int cogen_comp_declare(struct comp_inst *comp) /* make struct: set, pos, rot, declare block */ coutf(" char _name[256]; /* e.g. %s */", comp->name); coutf(" char _type[256]; /* %s */", comp->def->name); - /* note: `index' here is the loop counter above (always 2), not the - component index; it only affects this generated comment. */ - coutf(" long _index; /* e.g. %i index in TRACE list */", index); + coutf(" long _index; /* e.g. %i index in TRACE list */", comp->index); cout( " Coords _position_absolute;"); cout( " Coords _position_relative; /* wrt PREVIOUS */"); cout( " Rotation _rotation_absolute;"); @@ -645,14 +652,17 @@ static void cogen_comp_init_par(struct comp_inst *comp, struct instr_def *instr, } else if (par->type == instr_type_vector) { if (val[0] == '{') { - /* FIXME: no bounds check, a literal vector with > 100 elements - overflows values[] (the struct field is sized from the real count). */ - double values[100]; - int vl = parse_curlybrackets_vector(val, values); + int vl = parse_curlybrackets_vector(val, NULL); + double *values = mem((vl + 1) * sizeof(double)); int i; + parse_curlybrackets_vector(val, values); for (i=0; iname, par->id, i, values[i]); + char num[32]; /* shortest of %.15g/%.17g that reads back exactly */ + snprintf(num, sizeof(num), "%.15g", values[i]); + if (strtod(num, NULL) != values[i]) snprintf(num, sizeof(num), "%.17g", values[i]); + coutf(" _%s_var._parameters.%s[%i] = %s;", comp->name, par->id, i, num); } + memfree(values); } else coutf(" _%s_var._parameters.%s = %s; // default pointer allocation", comp->name, par->id, val); } @@ -908,7 +918,6 @@ int cogen_comp_section_class( if (par->type == instr_type_symbol) { entry = symtab_lookup(loopcomp->setpar, par->id); val = exp_tostring(entry->val); - cout(" // HEST!"); /* leftover debug marker, emitted into generated code */ coutf(" %s = %s;", par->id, val); } } @@ -1391,7 +1400,14 @@ int cogen_decls(struct instr_def *instr) } coutf("/* Shared user declarations for all components types '%s'. */", comp->def->name); codeblock_out(comp->def->share_code); - comp->def->flag_defined_share = 1; /* flag the component so that SHARE outputs only once */ + /* flag so that SHARE outputs only once, also for classes that INHERIT + this very block (same pointer, e.g. "DEFINE COMPONENT X INHERIT Y") */ + List_handle liter2 = list_iterate(instr->complist); + struct comp_inst *other; + while((other = list_next(liter2))) + if (other->def->share_code == comp->def->share_code) + other->def->flag_defined_share = 1; + list_iterate_end(liter2); cout(""); index++; } @@ -1515,8 +1531,6 @@ int cogen_section(struct instr_def *instr, char *section, char *section_lower, list_iterate_end(liter); cout(""); - /* note: cone, polygon and polyhedron are #define'd above but not #undef'd - here, so those names stay mapped to mcdis_* for the rest of the file. */ if (!strcmp(section, "DISPLAY")) { cout(" #undef magnify"); cout(" #undef line"); @@ -1527,6 +1541,9 @@ int cogen_section(struct instr_def *instr, char *section, char *section_lower, cout(" #undef circle"); cout(" #undef cylinder"); cout(" #undef sphere"); + cout(" #undef cone"); + cout(" #undef polygon"); + cout(" #undef polyhedron"); } cout(""); @@ -1651,6 +1668,18 @@ int cogen_section(struct instr_def *instr, char *section, char *section_lower, } /* cogen_section */ +/* particle state parameter names, #define'd as _particle->name in TRACE */ +static char *statepars_all[] = { +#if MCCODE_PROJECT == 1 /* neutron */ + "x", "y", "z", "vx", "vy", "vz", + "t", "sx", "sy", "sz", "p", "mcgravitation", "mcMagnet", "allow_backprop", "_mctmp_a", "_mctmp_b", "_mctmp_c" +#elif MCCODE_PROJECT == 2 /* xray */ + "x", "y", "z", "kx", "ky", "kz", + "phi", "t", "Ex", "Ey","Ez", "p", "allow_backprop", "_mctmp_a", "_mctmp_b", "_mctmp_c" +#endif +}; +static const int num_state_pars = sizeof(statepars_all)/sizeof(statepars_all[0]); + /******************************************************************************* * def_trace_section: #define symbols for entire TRACE section *******************************************************************************/ @@ -1658,18 +1687,6 @@ void def_trace_section(struct instr_def *instr) { List_handle liter; int i = 0; - static char *statepars_all[] = - { /* particle state parameter names are used for defines */ - #if MCCODE_PROJECT == 1 /* neutron */ - #define NUM_STATE_PARS 17 - "x", "y", "z", "vx", "vy", "vz", - "t", "sx", "sy", "sz", "p", "mcgravitation", "mcMagnet", "allow_backprop", "_mctmp_a", "_mctmp_b", "_mctmp_c" - #elif MCCODE_PROJECT == 2 /* xray */ - #define NUM_STATE_PARS 16 - "x", "y", "z", "kx", "ky", "kz", - "phi", "t", "Ex", "Ey","Ez", "p", "allow_backprop", "_mctmp_a", "_mctmp_b", "_mctmp_c" - #endif - }; cout("/*******************************************************************************"); coutf("* components %s", "TRACE"); @@ -1677,7 +1694,7 @@ void def_trace_section(struct instr_def *instr) cout(""); /* define state parameters for all TRACE*/ - for(i=0; i%s)", statepars_all[i], statepars_all[i]); cout("/* if on GPU, globally nullify sprintf,fprintf,printfs */"); @@ -1715,22 +1732,9 @@ void undef_trace_section(struct instr_def *instr) { List_handle liter; int i = 0; - static char *statepars_all[] = - { /* particle state parameter names are used for defines */ - #if MCCODE_PROJECT == 1 /* neutron */ - #define NUM_STATE_PARS 17 - "x", "y", "z", "vx", "vy", "vz", - "t", "sx", "sy", "sz", "p", "mcgravitation", "mcMagnet", "allow_backprop", "_mctmp_a", "_mctmp_b", "_mctmp_c" - #elif MCCODE_PROJECT == 2 /* xray */ - #define NUM_STATE_PARS 16 - "x", "y", "z", "kx", "ky", "kz", - "phi", "t", "Ex", "Ey","Ez", "p", "allow_backprop", "_mctmp_a", "_mctmp_b", "_mctmp_c" - #endif - }; - List l; // undef everything after the "raytrace" function - for(i = 0; i < NUM_STATE_PARS; i++) + for(i = 0; i < num_state_pars; i++) coutf("#undef %s", statepars_all[i]); cout("#ifdef OPENACC"); @@ -1843,20 +1847,6 @@ int cogen_raytrace(struct instr_def *instr) struct comp_inst *comp = NULL; int warnings = 0; int i = 0; - int split_counts = 0; - static char *statepars_all[] = - { /* particle state parameter names are used for defines */ - #if MCCODE_PROJECT == 1 /* neutron */ - #define NUM_STATE_PARS 17 - "x", "y", "z", "vx", "vy", "vz", - "t", "sx", "sy", "sz", "p", "mcgravitation", "mcMagnet", "allow_backprop", "_mctmp_a", "_mctmp_b", "_mctmp_c" - #elif MCCODE_PROJECT == 2 /* xray */ - #define NUM_STATE_PARS 16 - "x", "y", "z", "kx", "ky", "kz", - "phi", "t", "Ex", "Ey","Ez", "p", "allow_backprop", "_mctmp_a", "_mctmp_b", "_mctmp_c" - #endif - }; - List l; // // write the raytrace function @@ -1895,21 +1885,7 @@ int cogen_raytrace(struct instr_def *instr) /* create counter for JUMP iteration */ if (this_jump->iterate) coutf(" _particle->_logic.Jump_%s_%s=0;", comp->name, this_jump->target); - /* check that the target_index is defined, else search for the target comp index */ - if (!this_jump->target_index) { - List_handle liter3 = list_iterate(instr->complist); - struct comp_inst *target=NULL; - while((target = list_next(liter3)) != NULL) { - if (!strcmp(target->name, this_jump->target)) - this_jump->target_index = target->index; - } - list_iterate_end(liter3); - } - /* FIXME: only index 0 (MYSELF) is made absolute here; PREVIOUS (-1) - and NEXT (+1) are left relative and then used as absolute indices - (see also detect_skipable_transforms, which runs first). */ - if (!this_jump->target_index) /* JUMP e.g. PREVIOUS/NEXT is relative -> absolute */ - this_jump->target_index += comp->index; + /* target_index is already absolute, see detect_skipable_transforms */ } list_iterate_end(liter2); } @@ -2183,8 +2159,8 @@ int cogen_raytrace(struct instr_def *instr) * a new loop starts when the device changes or at a SPLIT. A SPLIT here * compacts the batch (absorbed particles last) and duplicates live ones into * the free slots. JUMP is not supported. -* Note: GROUP handling differs from raytrace(): a non-scattering member sets -* ABSORBED=0 instead of restoring the saved particle. +* GROUP handling deliberately differs from raytrace(): a non-scattering +* member sets ABSORBED=0 instead of restoring the saved particle. * *******************************************************************************/ int cogen_rt_funnel(struct instr_def *instr) @@ -2193,20 +2169,6 @@ int cogen_rt_funnel(struct instr_def *instr) struct comp_inst *comp = NULL; int warnings = 0; int i = 0; - int split_counts = 0; - static char *statepars_all[] = - { /* particle state parameter names are used for defines */ - #if MCCODE_PROJECT == 1 /* neutron */ - #define NUM_STATE_PARS 17 - "x", "y", "z", "vx", "vy", "vz", - "t", "sx", "sy", "sz", "p", "mcgravitation", "mcMagnet", "allow_backprop", "_mctmp_a", "_mctmp_b", "_mctmp_c" - #elif MCCODE_PROJECT == 2 /* xray */ - #define NUM_STATE_PARS 16 - "x", "y", "z", "kx", "ky", "kz", - "phi", "t", "Ex", "Ey","Ez", "p", "allow_backprop", "_mctmp_a", "_mctmp_b", "_mctmp_c" - #endif - }; - List l; printf("\n-----------------------------------------------------------\n"); printf("\nGenerating GPU/CPU -DFUNNEL layout:\n"); @@ -2479,7 +2441,7 @@ cogen_header(struct instr_def *instr, char *output_name) else strcpy(pathsep, "\\\\"); sysdir_orig = get_sys_dir(); - sysdir_new = (char *)mem(2*strlen(sysdir_orig)); + sysdir_new = (char *)mem(2*strlen(sysdir_orig)+1); for (i=0; i < strlen(sysdir_orig); i++) { if (sysdir_orig[i] == '\\') @@ -2590,8 +2552,10 @@ cogen_header(struct instr_def *instr, char *output_name) * list for later def/undef as state variables (see cogen_raytrace). * (The type is in fact taken from the declaration; component USERVARS get * the instance index appended, e.g. "flag" -> "flag_3".) - * Note: particle_getvar() and particle_getuservar_byid() below read every - * user var as a double, which is wrong for non-double types. */ + * particle_getvar() and particle_getuservar_byid() below return user vars + * as double: numeric scalars are converted, others report failure (*suc). + * To use a USERVAR with Monitor_nD (user1="var" ...), it must be declared + * double. */ instr->user_vars = list_create(); instr->user_vars_types = list_create(); @@ -2724,10 +2688,14 @@ cogen_header(struct instr_def *instr, char *output_name) cout(" if(!str_comp(\"_mctmp_a\",name)){rval=p->_mctmp_a;s=0;}"); cout(" if(!str_comp(\"_mctmp_b\",name)){rval=p->_mctmp_b;s=0;}"); cout(" if(!str_comp(\"_mctmp_c\",name)){rval=p->_mctmp_c;s=0;}"); + liter2 = list_iterate(instr->user_vars_types); while((var = list_next(liter))) { - coutf(" if(!str_comp(\"%s\",name)){rval=*( (double *)(&(p->%s)) );s=0;}",var,var); + tpe = list_next(liter2); + if (uservar_is_numeric(tpe, var)) + coutf(" if(!str_comp(\"%s\",name)){rval=(double)(p->%s);s=0;}",var,var); } list_iterate_end(liter); + list_iterate_end(liter2); cout(" if (suc!=0x0) {*suc=s;}"); cout(" return rval;"); cout("}"); @@ -2875,9 +2843,14 @@ cogen_header(struct instr_def *instr, char *output_name) cout(" double rval=0;"); cout(" switch(id){"); int jvar=0; + liter2 = list_iterate(instr->user_vars_types); while((var = list_next(liter))) { - coutf(" case %d: { rval=*( (double *)(&(p->%s)) );s=0;break;}",jvar++,var); + tpe = list_next(liter2); + if (uservar_is_numeric(tpe, var)) // ids count all user vars, readable or not + coutf(" case %d: { rval=(double)(p->%s);s=0;break;}",jvar,var); + jvar++; } + list_iterate_end(liter2); cout(" }"); cout(" if (suc!=0x0) {*suc=s;}"); cout(" return rval;"); @@ -2938,8 +2911,6 @@ cogen_header(struct instr_def *instr, char *output_name) fprintf(stderr,"Dependency: %s.o\n", "mcxtrace-r"); #endif - /* note: printed for McXtrace too (duplicate of the McStas line above) */ - fprintf(stderr,"Dependency: %s.o\n", "mcstas-r"); fprintf(stderr,"Dependency: '-DUSE_NEXUS -lNeXus' to enable NeXus support\n"); fprintf(stderr,"To build instrument '%s', compile and link with these libraries (in %s%sshare)\n", instrument_definition->quoted_source, sysdir_new, pathsep); @@ -3000,8 +2971,8 @@ cogen_header(struct instr_def *instr, char *output_name) /******************************************************************************* * varname_is_in_codeblock: Checks if variable varname appears as a whole word * in code block code. Filters out comment sections. -* Side effect: comments are blanked out IN PLACE in code->lines, so the -* block is later written to the generated file without its comments. +* Side effect: comments are blanked out IN PLACE in code->lines. Harmless +* today, as the DECLARE/USERVARS blocks scanned here are not written out. * * code: code block containing var, or not * varname: variable declaration id @@ -3124,8 +3095,7 @@ int get_codeblock_vars_allcustom(struct code_block *code, List custom_vars, * returns: the number of vars added * * This is a regex heuristic, not a C parser. It finds "type name;" or -* "type name[N];" (no initialiser). Only the first declaration on each line -* is found (idx below never changes, so the do-while runs once per line). +* "type name[N];" (no initialiser), also several per line. *******************************************************************************/ int get_codeblock_vars(struct code_block *code, List vars, List types, char* block_name, char* movetoblock_name) { @@ -3135,56 +3105,35 @@ int get_codeblock_vars(struct code_block *code, List vars, List types, List_handle myiter; myiter = list_iterate(code->lines); - char *l; - int idx = -1; - int pos = -1; - int len = -1; int ans = 0; while((l = list_next(myiter))) { - do { - // line must match this format - pos = re_match("\\w[\\s\\w\\*]+\\s+\\**\\w+\\[?\\w*\\]?\\[?\\w*\\]?;", l); // match general var decl pattern - - if (pos <= -1) { - // skip - } - else { - char* tpe; - char* vn; - - // extract the type - len = re_match("\\w+\\[?\\w*\\]?\\[?\\w*\\]?;", l+pos); // end of type string - tpe = (char*) malloc((len + 1) * sizeof(char)); // new location - *(tpe+len) = '\0'; // null terminate - if (len > -1) { - strncpy(tpe, l+pos, len); - } - - // extract the varname - pos = re_match("\\w+\\[?\\w*\\]?\\[?\\w*\\]?;", l); // match start of varname - if (pos > -1) { - len = re_match(";", l+pos); // match end of varname - - if (len > -1) { - vn = (char*) malloc((len + 1) * sizeof(char)); // new location - *(vn+len) = '\0'; // null terminate - - strncpy(vn, l+pos, len); - l = l + pos + len + 1; // inc line to pos after scolon - ans++; - } - } - - // check that symbol was not found inside a comment block - if (varname_is_in_codeblock(code, vn)) { - list_add(types, tpe); - list_add(vars, vn); - } + int pos; + // one or more "type name;" declarations on this line + while ((pos = re_match("\\w[\\s\\w\\*]+\\s+\\**\\w+\\[?\\w*\\]?\\[?\\w*\\]?;", l)) > -1) { + // the type is everything up to the varname, the varname runs to ';' + int tlen = re_match("\\w+\\[?\\w*\\]?\\[?\\w*\\]?;", l+pos); + if (tlen < 0) break; + char *v = l + pos + tlen; + int vlen = re_match(";", v); + if (vlen < 0) break; + char *tpe = str_dup_n(l+pos, tlen); + char *vn = str_dup_n(v, vlen); + l = v + vlen + 1; // continue after the ';' + + // check that symbol was not found inside a comment block + if (varname_is_in_codeblock(code, vn)) { + list_add(types, tpe); + list_add(vars, vn); + ans++; + } else { + str_free(tpe); + str_free(vn); } - } while(idx > -1); + } } + list_iterate_end(myiter); // print a warning if code block contains '=' char int warnings = 0; @@ -3212,8 +3161,7 @@ int get_codeblock_vars(struct code_block *code, List vars, List types, * component to the next one; setpos() chains relative coordinates * accordingly. Must run after cogen_decls (which sets comp->index) and * before cogen_section("INITIALISE") and cogen_raytrace. -* FIXME: a relative JUMP PREVIOUS gives target_index -1 and list_access() -* with a negative index (generator crash). +* Also turns every JUMP target into an absolute component index. *******************************************************************************/ void detect_skipable_transforms(struct instr_def *instr) { // @@ -3248,16 +3196,23 @@ void detect_skipable_transforms(struct instr_def *instr) { List_handle literJ = list_iterate(comp->jump); while ((this_jump = list_next(literJ))) { - // Find target (not initialized yet) - List_handle liter3 = list_iterate(instr->complist); - struct comp_inst *target=NULL; - while((target = list_next(liter3)) != NULL) { - if (!strcmp(target->name, this_jump->target)) - this_jump->target_index = target->index; - } - list_iterate_end(liter3); - if (!this_jump->target_index) /* JUMP e.g. PREVIOUS/NEXT is relative -> absolute */ + // Resolve the target to an absolute component index (only + // here, cogen_raytrace relies on it). PREVIOUS/NEXT/MYSELF + // are offsets from this component, others are names. + if (this_jump->target_index || !strcmp(this_jump->target, "MYSELF")) this_jump->target_index += comp->index; + else { + List_handle liter3 = list_iterate(instr->complist); + struct comp_inst *target=NULL; + while((target = list_next(liter3)) != NULL) { + if (!strcmp(target->name, this_jump->target)) + this_jump->target_index = target->index; + } + list_iterate_end(liter3); + } + if (this_jump->target_index < 1 || this_jump->target_index > list_len(instr->complist)) + fatal_error("JUMP at component %s: target %s is not a component of this instrument.\n", + comp->name, this_jump->target); // component list is 1 indexed so subtract one from target_index target_comp = list_access(instr->complist, this_jump->target_index - 1); diff --git a/mccode/src/instrument.l b/mccode/src/instrument.l index c9c839222..87784a3bc 100644 --- a/mccode/src/instrument.l +++ b/mccode/src/instrument.l @@ -109,9 +109,8 @@ static void push_include(char *name); %x inclname /* Get full %include line within C code blocks. - Note: the line is re-scanned from its start (yyless(0)), but this state - has no rule for leading blanks, so any indentation before %include is - echoed by flex's default rule to stdout. */ + The line is re-scanned from its start (yyless(0)), so leading blanks are + skipped there first. */ %x cfullincl /* Get file name in %include within C code blocks. */ @@ -304,12 +303,13 @@ METADATA return TOK_METADATA; } /* end inclname */ /* Include files within C code blocks (ccode state)*/ /* next token is full line, regenerated by yyless(0) */ +[ \t]+ /* skip indentation (else flex would echo it) */ {INCLUDE}[ \t]+\" BEGIN(cinclname); { - /* name ends with a quote char, with extension -> include as ccode state. - Note: the pattern only accepts a ONE-character extension ("x.h", "x.c"); - "x.hpp" is not matched here and is treated as a library name below. */ - [^\"\n]+\.+.\" { + /* name ends with a quote char, with extension ("x.h", "x.hpp", "dir/x.c") + -> include as ccode state. Both rules match the same text, so this one + wins by being first. */ + [^\"\n]+\.[A-Za-z0-9]+\" { yytext[yyleng - 1] = '\0'; BEGIN(ccode); if (verbose) fprintf(stderr, "Embedding file %s\n", yytext); @@ -319,8 +319,6 @@ METADATA return TOK_METADATA; * this occurs when importing a library .h/.c The .c is only included * when instr->runtime option is true * Each library is embedded once per run (tracked in lib_instances). - * FIXME: push_include() keeps tmp1 as instr_current_filename, and tmp1 is - * freed right after, so error messages from inside lib.h read freed memory. */ [^\"\n]+\" { char *tmp0, *tmp1; @@ -345,8 +343,7 @@ METADATA return TOK_METADATA; } else { - BEGIN(ccode); - instr_current_line++; /* library was previously embedded */ + BEGIN(ccode); /* library was previously embedded */ } str_free(tmp0); } @@ -467,8 +464,7 @@ push_file(FILE *file, int restricted, int visible_eof) } /* Handle a new %include file. The include is transparent to the parser: it - just sees more tokens. Note that `name' is stored as-is (not copied) in - instr_current_filename, so it must outlive the included file. */ + just sees more tokens. */ void push_include(char *name) { @@ -480,7 +476,9 @@ push_include(char *name) "on line %d of file '%s'.\n", name, instr_current_line, instr_current_filename); push_file(file, FALSE, FALSE); - instr_current_filename = name; + /* copy: callers pass yytext or temporaries, but the name is kept in + code blocks and error messages long after */ + instr_current_filename = str_dup(name); instr_current_line = 1; } diff --git a/mccode/src/instrument.y b/mccode/src/instrument.y index b81a10d0d..b05dcd2d2 100644 --- a/mccode/src/instrument.y +++ b/mccode/src/instrument.y @@ -96,6 +96,7 @@ void run_command_to_add_search_dir(char * input); int metadata_construct_table(instr_ptr_t); void metadata_assign_from_definition(List metadata); void metadata_assign_from_instance(List metadata); +static void dependency_add(char *s); %} @@ -286,12 +287,8 @@ compdef: "DEFINE" "COMPONENT" TOK_ID parameters metadata shell dependency noa { /* inherit from another comp, and initiate it with given blocks */ /* all redefined blocks override */ - /* Parameter lists are parent + child, concatenated. - FIXME: the "block given?" tests below use ->linenum, but an absent - block comes from codeblock_new() with linenum -1, which is true. - So the child's (possibly empty) block always wins and the parent's - code blocks are never inherited this way. Testing - list_len($N->lines) was probably intended. */ + /* Parameter lists are parent + child, concatenated. A code section + the child leaves empty is taken from the parent. */ struct comp_def *def; def = read_component($5); if (def) { @@ -314,14 +311,14 @@ compdef: "DEFINE" "COMPONENT" TOK_ID parameters metadata shell dependency noa c->flag_noacc = $10; - c->share_code = ($11->linenum ? $11 : def->share_code); - c->uservar_code = ($12->linenum ? $12 : def->uservar_code); - c->decl_code = ($13->linenum ? $13 : def->decl_code); - c->init_code = ($14->linenum ? $14 : def->init_code); - c->trace_code = ($15->linenum ? $15 : def->trace_code); - c->save_code = ($16->linenum ? $16 : def->save_code); - c->finally_code = ($17->linenum ? $17 : def->finally_code); - c->display_code = ($18->linenum ? $18 : def->display_code); + c->share_code = (list_len($11->lines) ? $11 : def->share_code); + c->uservar_code = (list_len($12->lines) ? $12 : def->uservar_code); + c->decl_code = (list_len($13->lines) ? $13 : def->decl_code); + c->init_code = (list_len($14->lines) ? $14 : def->init_code); + c->trace_code = (list_len($15->lines) ? $15 : def->trace_code); + c->save_code = (list_len($16->lines) ? $16 : def->save_code); + c->finally_code = (list_len($17->lines) ? $17 : def->finally_code); + c->display_code = (list_len($18->lines) ? $18 : def->display_code); /* Check definition and setting params for uniqueness */ check_comp_formals(c->def_par, c->set_par, c->name); @@ -703,9 +700,11 @@ comp_uservars: /* empty */ } | "USERVARS" codeblock comp_uservars_inherit_extend { - /* FIXME: unlike the other sections, the chain $3 is dropped here, - so EXTEND blocks after USERVARS are silently ignored. */ - $$ = $2; + struct code_block *cb; + cb = codeblock_new(); + list_cat(cb->lines, $2->lines); + list_cat(cb->lines, $3->lines); + $$ = cb; } ; @@ -1053,9 +1052,7 @@ instr_formal: TOK_ID TOK_ID } else if(!strcmp($1, "double")) { formal->type = instr_type_vector; } else { - /* FIXME: "$s" should be "%s"; arguments are shifted by one in the - message (and %d gets a pointer). */ - print_error("ERROR: Illegal type $s* for instrument " + print_error("ERROR: Illegal type %s* for instrument " "parameter %s at line %s:%d.\n", $1, $3, instr_current_filename, instr_current_line); formal->type = instr_type_double; } @@ -1153,7 +1150,7 @@ instr_formal: TOK_ID TOK_ID } else if(!strcmp($1, "double")) { formal->type = instr_type_vector; } else { - print_error("ERROR: Illegal type $s* for instrument " + print_error("ERROR: Illegal type %s* for instrument " "parameter %s at line %s:%d.\n", $1, $3, instr_current_filename, instr_current_line); formal->type = instr_type_double; } @@ -1353,6 +1350,11 @@ instref: "COPY" '(' compref ')' actuallist /* make a copy of a previous instance struct comp_inst *comp_src; struct comp_inst *comp; comp_src = $3; + if (!comp_src) { + print_error("ERROR: COPY of an undefined component instance at line %s:%d.\n", + instr_current_filename, instr_current_line); + YYABORT; + } palloc(comp); comp->def = comp_src->def; /* now catenate src and actual parameters */ @@ -1372,6 +1374,11 @@ instref: "COPY" '(' compref ')' actuallist /* make a copy of a previous instance struct comp_inst *comp_src; struct comp_inst *comp; comp_src = $3; + if (!comp_src) { + print_error("ERROR: COPY of an undefined component instance at line %s:%d.\n", + instr_current_filename, instr_current_line); + YYABORT; + } palloc(comp); comp->defpar = comp_src->defpar; comp->setpar = comp_src->setpar; @@ -1421,9 +1428,8 @@ cpuonly: /* empty */ | "CPU" { $$ = 1; - if (strstr(instrument_definition->dependency," -DFUNNEL ") == NULL) { - strncat(instrument_definition->dependency, " -DFUNNEL ", 1024); - } + if (strstr(instrument_definition->dependency," -DFUNNEL ") == NULL) + dependency_add(" -DFUNNEL "); } ; @@ -1433,9 +1439,9 @@ cpuonly: /* empty */ [JUMP ...]* [METADATA ...]* Split in two actions: the mid-rule action (after instref) names and numbers the instance and checks its parameters, so that MYSELF in - WHEN/AT/ROTATED/JUMP expressions resolves. Note: MYSELF inside the actual - parameters (instref) is parsed earlier, while myself_comp still points to - the previous instance. The final action stores the placement and + WHEN/AT/ROTATED/JUMP expressions resolves. MYSELF inside the actual + parameters (instref) is parsed before that and is an error (myself_comp is + reset to NULL after each instance). The final action stores the placement and GROUP/EXTEND/JUMP/METADATA. Positional values: $8 is the mid-rule action itself, so $9 = when ... $15 = metadata. */ component: removable cpuonly split "COMPONENT" instname '=' instref @@ -1443,17 +1449,12 @@ component: removable cpuonly split "COMPONENT" instname '=' instref struct comp_inst *comp; myself_comp = comp = $7; - // Trying to check or assign metadata before the previous line is accessing a null pointer! if (comp->metadata == NULL || list_undef(comp->metadata)) comp->metadata = list_create(); - if (myself_comp->metadata == NULL || list_undef(myself_comp->metadata)) myself_comp->metadata = list_create(); comp->name = $5; comp->split = $3; comp->cpuonly = $2; - /* FIXME: comp->def is NULL when the component class was not found; - it is dereferenced here before the NULL check below, so a typo in - a component name segfaults instead of reporting the error. */ - if (!comp->cpuonly) { + if (!comp->cpuonly && comp->def) { /* def is NULL if class not found */ comp->cpuonly = comp->def->flag_noacc; } comp->removable = $1; @@ -1519,6 +1520,7 @@ component: removable cpuonly split "COMPONENT" instname '=' instref debugn((DEBUG_HIGH, "Component[%i]: %s = %s().\n", comp_current_index, $5, $7->def->name)); /* this comp will be 'previous' for the next, except if removed at include */ if (!comp->removable) previous_comp = comp; + myself_comp = NULL; /* MYSELF is only valid inside an instance */ $$ = comp; } @@ -1832,10 +1834,7 @@ jumpcondition: "WHEN" exp ; /* JUMP target. index is relative (PREVIOUS=-1, NEXT=+1, MYSELF=0) or 0 for - a named target, which cogen.c resolves by name later. - FIXME: cogen.c only converts index 0 to absolute, so PREVIOUS[(n)] and - NEXT[(n)] are used as absolute indices: JUMP PREVIOUS crashes the - generator and JUMP NEXT goes to the first component. */ + a named target. cogen.c (detect_skipable_transforms) makes it absolute. */ jumpname: "PREVIOUS" { $$.name = str_dup("PREVIOUS"); @@ -1903,17 +1902,14 @@ search: "SEARCH" TOK_STRING } while (fgets(svalue, sizeof(svalue), sfp) != NULL){ // Make a copy of the char array -- We can't free this memory until the program is done, so we're going to leak it :/ - char * path = calloc(strlen(svalue)+1, sizeof(char)); + char * path = calloc(strlen(svalue)+2, sizeof(char)); // +1 for a PATHSEP strcpy(path, svalue); // Remove the trailing newline (and/or carriage return) which is almost-certainly present path[strcspn(path, "\r\n")] = 0; + size_t len = strlen(path); + if (!len) { free(path); continue; } // skip empty lines // Ensure the path specification *ends* in a PATHSEP character - // FIXME: path has no room for the extra separator (heap overflow by - // one byte), `last' is NULL when there is no separator, and the test - // is always true, so a separator is appended even if already there. - char * last = strrchr(path, MC_PATHSEP_S[0]); - unsigned int last_sep = last - path + 1; - if ((last - path) < strlen(path)) strcat(path, MC_PATHSEP_S); + if (path[len-1] != MC_PATHSEP_C) strcat(path, MC_PATHSEP_S); // Add the specified path to the search list add_search_dir(path); } @@ -1923,17 +1919,15 @@ search: "SEARCH" TOK_STRING /* DEPENDENCY "flags": extra compiler flags, collected (de-duplicated) in instrument_definition->dependency and printed as "CFLAGS=..." at the end - of the run, plus in the generated file header, for mcrun to pick up. - Note: strncat's limit is the number of chars to append, not the buffer - size, so very many dependencies could overflow dependency[1024]. */ + of the run, plus in the generated file header, for mcrun to pick up. */ dependency: { } | "DEPENDENCY" TOK_STRING { if (strstr(instrument_definition->dependency,$2) == NULL) { - strncat(instrument_definition->dependency, " ", 1024); - strncat(instrument_definition->dependency, $2, 1023); // 1023 because we already appended a space + dependency_add(" "); + dependency_add($2); } } ; @@ -1947,9 +1941,8 @@ noacc: { /* Comp class is CPU only */ $$ = 1; - if (strstr(instrument_definition->dependency," -DFUNNEL ") == NULL) { - strncat(instrument_definition->dependency, " -DFUNNEL ", 1024); - } + if (strstr(instrument_definition->dependency," -DFUNNEL ") == NULL) + dependency_add(" -DFUNNEL "); } ; @@ -1988,13 +1981,20 @@ topatexp: "PREVIOUS" if (previous_comp) { $$ = exp_ctoken(previous_comp->name); } else { - /* FIXME: $$ is left unset here (garbage CExp) */ + $$ = exp_number("0"); print_error("ERROR: Found invalid PREVIOUS reference at line %s:%d. Please fix (add a component instance before).\n", instr_current_filename, instr_current_line); } } | "MYSELF" { - $$ = exp_ctoken(myself_comp->name); + if (myself_comp) { + $$ = exp_ctoken(myself_comp->name); + } else { + $$ = exp_number("0"); + print_error("ERROR: MYSELF can not be used here at line %s:%d. It is only available " + "after the component parameters (WHEN, AT, ROTATED, JUMP, ...).\n", + instr_current_filename, instr_current_line); + } } | TOK_ID @@ -2158,34 +2158,6 @@ code: /* empty */ /* end of grammar *********************************************************** */ -/* Pool of parser allocations. Note: nothing allocates from this pool any - more (all allocations use mem()/palloc), so it is created and freed empty. */ -static Pool parser_pool = NULL; - -static int mc_yyparse(void) -{ - int ret; - Pool oldpool; - oldpool = parser_pool; - parser_pool = pool_create(); - ret = yyparse(); - pool_free(parser_pool); - parser_pool = oldpool; - return ret; -} - -// Separate identical parser to make debugging a bit easier -static int mc_yyparse_component(void){ - int ret; - Pool old; - old = parser_pool; - parser_pool = pool_create(); - ret = yyparse(); - pool_free(parser_pool); - parser_pool = old; - return ret; -} - /* Name of the file currently being parsed. */ char *instr_current_filename = NULL; /* Number of the line currently being parsed. */ @@ -2244,6 +2216,15 @@ Symtab read_components = NULL; /* name of executable, e.g. mcstas or mcxtrace */ char *executable_name=NULL; +/* Append s to the instrument CFLAGS without overflowing the fixed buffer. */ +static void +dependency_add(char *s) +{ + char *d = instrument_definition->dependency; + size_t n = strlen(d); + snprintf(d + n, sizeof(instrument_definition->dependency) - n, "%s", s); +} + /* Print a summary of the command usage. */ static void print_usage(void) @@ -2364,8 +2345,6 @@ parse_command_line(int argc, char *argv[]) instrument_definition->include_runtime = 1; instrument_definition->enable_trace = 1; instrument_definition->portable = 0; - /* Note: strcmp (not strcpy), so this is a no-op; dependency starts empty. */ - strcmp(instrument_definition->dependency, "-lm"); executable_name = argv[0]; for(i = 1; i < argc; i++) { @@ -2497,7 +2476,7 @@ main(int argc, char *argv[]) lex_new_file(file); read_components = symtab_create(); /* Create table of components. */ lib_instances = symtab_create(); /* Create table of libraries. */ - err = mc_yyparse(); + err = yyparse(); fclose(file); if (err != 0 && !error_encountered) error_encountered++; if(error_encountered != 0) @@ -2797,7 +2776,7 @@ read_component(char *name) must not be freed. */ instr_current_filename = component_pathname; instr_current_line = 1; - err = mc_yyparse_component(); /* Read definition from file. */ + err = yyparse(); /* Read definition from file. */ if(err != 0) fatal_error("Errors encountered during autoload of component %s. The component definition has syntax errors.\n", name); diff --git a/mccode/src/mccode.h.in b/mccode/src/mccode.h.in index 32f217b21..ab316c94b 100644 --- a/mccode/src/mccode.h.in +++ b/mccode/src/mccode.h.in @@ -75,8 +75,6 @@ * Functions defined in memory.c *******************************************************************************/ -typedef struct Pool_header *Pool; - void *mem(size_t); /* Allocate memory. */ void memfree(void *); /* Free memory. */ char *str_dup(char *); /* Allocate new copy of string. */ @@ -85,10 +83,6 @@ char *str_cat(char *first, ...);/* Concatenate strings to allocated string. */ char *str_quote(char *string); /* Quote string for inclusion in C code */ void str_free(char *); /* Free memory for string. */ -Pool pool_create(void); /* Create pool. */ -void pool_free(Pool p); /* Free pool and associated memory. */ -void *pool_mem(Pool p, size_t size); /* Allocate memory in pool. */ - /* Allocate memory to a pointer. If p is a pointer to type t, palloc(p) will make p point to dynamically allocated memory for one element of type t. Used to dynamicaaly allocate structures, eg. diff --git a/mccode/src/mcformat.c.in b/mccode/src/mcformat.c.in index 356af56c8..d06267e76 100644 --- a/mccode/src/mcformat.c.in +++ b/mccode/src/mcformat.c.in @@ -37,7 +37,7 @@ *******************************************************************************/ #ifndef MCFORMAT -#define MCFORMAT "$Revision$" /* avoid memory.c to define Pool functions */ +#define MCFORMAT "$Revision$" #endif #ifdef USE_MPI @@ -54,7 +54,6 @@ #define debug(msg) #define MCCODE_H /* avoids memory.c to import mccode.h */ -typedef struct Pool_header *Pool; /* allows memory to be included */ #include "memory.c" #include "../lib/share/mccode-r.h" /* with decl of MC_PATHSEP */ diff --git a/mccode/src/memory.c b/mccode/src/memory.c index 471b11a92..68044da47 100644 --- a/mccode/src/memory.c +++ b/mccode/src/memory.c @@ -165,60 +165,3 @@ str_free(char *string) { memfree(string); } - -#ifndef MCFORMAT - -struct Pool_header - { - List list; - }; - -/******************************************************************************* -* Create a pool in which to allocate memory that may be easily freed all at a -* time by freeing the pool. -*******************************************************************************/ -Pool -pool_create(void) -{ - Pool p; - - palloc(p); - p->list = list_create(); - return p; -} - -/******************************************************************************* -* Deallocate a pool as well as all memory allocated within it. -*******************************************************************************/ -void -pool_free(Pool p) -{ -// List_handle liter; -// void *mem; -// -// liter = list_iterate(p->list); -// while((mem = list_next(liter))) -// { -// memfree(mem); -// } -// list_iterate_end(liter); -// -// memfree(p); - // pools *are* lists, which have automatically-allocated memory (which we should free) - // the method above only frees part of the memory - list_free(p->list, memfree); -} - - -/******************************************************************************* -* Allocate memory in a pool. -*******************************************************************************/ -void * -pool_mem(Pool p, size_t size) -{ - void *m = mem(size); - list_add(p->list, m); - return m; -} - -#endif /* MCSTAS_H */ From eb522551106985417627022ca39e23f2dd03973f Mon Sep 17 00:00:00 2001 From: Peter Willendrup Date: Wed, 7 Oct 2026 12:42:26 +0200 Subject: [PATCH 3/8] Tests_grammar: unit tests for the code-generator grammar fixes McStas and McXtrace versions of tests for ADR_20261007_GRAMMAR_FIXES: - Unittest_JUMP_RELATIVE: JUMP PREVIOUS(2) ITERATE and JUMP NEXT(2) - Unittest_INHERIT_DIAPHRAGM: whole-component INHERIT (Diaphragm vs Slit) - Unittest_USERVARS_TYPES: int USERVAR read by Monitor_nD, several declarations per line, component USERVARS+EXTEND, %include "x.hpp" - Unittest_GRAMMAR_ERRORS: *.instr.fail cases that must fail code generation (MYSELF in parameters, bad JUMP targets, COPY of undefined instance, unknown component, illegal pointer type), with a README. Expected values are in %Example lines. The old generator segfaults on or mis-handles each of them; see the README for the observed old/new behaviour. Co-Authored-By: Claude Opus 5.5 --- .../Err_COPY_undefined.instr.fail | 7 ++ .../Err_JUMP_out_of_range.instr.fail | 10 +++ .../Err_JUMP_unknown_target.instr.fail | 9 +++ .../Err_MYSELF_in_parameters.instr.fail | 8 ++ .../Err_illegal_pointer_type.instr.fail | 7 ++ .../Err_unknown_component.instr.fail | 7 ++ .../Unittest_GRAMMAR_ERRORS/README.md | 29 +++++++ .../Unittest_INHERIT_DIAPHRAGM.instr | 67 +++++++++++++++ .../Unittest_JUMP_RELATIVE.instr | 81 +++++++++++++++++++ .../Unittest_USERVARS_TYPES.instr | 77 ++++++++++++++++++ .../Unittest_UV_check.comp | 73 +++++++++++++++++ .../Unittest_USERVARS_TYPES/unittest_uv.hpp | 4 + .../Err_COPY_undefined.instr.fail | 7 ++ .../Err_JUMP_out_of_range.instr.fail | 10 +++ .../Err_JUMP_unknown_target.instr.fail | 9 +++ .../Err_MYSELF_in_parameters.instr.fail | 8 ++ .../Err_illegal_pointer_type.instr.fail | 7 ++ .../Err_unknown_component.instr.fail | 7 ++ .../Unittest_GRAMMAR_ERRORS/README.md | 29 +++++++ .../Unittest_INHERIT_DIAPHRAGM.instr | 67 +++++++++++++++ .../Unittest_JUMP_RELATIVE.instr | 81 +++++++++++++++++++ .../Unittest_USERVARS_TYPES.instr | 77 ++++++++++++++++++ .../Unittest_UV_check.comp | 73 +++++++++++++++++ .../Unittest_USERVARS_TYPES/unittest_uv.hpp | 4 + 24 files changed, 758 insertions(+) create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_COPY_undefined.instr.fail create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_out_of_range.instr.fail create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_unknown_target.instr.fail create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_MYSELF_in_parameters.instr.fail create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_illegal_pointer_type.instr.fail create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_unknown_component.instr.fail create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/README.md create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_DIAPHRAGM/Unittest_INHERIT_DIAPHRAGM.instr create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_JUMP_RELATIVE/Unittest_JUMP_RELATIVE.instr create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_USERVARS_TYPES.instr create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_UV_check.comp create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/unittest_uv.hpp create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_COPY_undefined.instr.fail create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_out_of_range.instr.fail create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_unknown_target.instr.fail create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_MYSELF_in_parameters.instr.fail create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_illegal_pointer_type.instr.fail create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_unknown_component.instr.fail create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/README.md create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_DIAPHRAGM/Unittest_INHERIT_DIAPHRAGM.instr create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_JUMP_RELATIVE/Unittest_JUMP_RELATIVE.instr create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_USERVARS_TYPES.instr create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_UV_check.comp create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/unittest_uv.hpp diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_COPY_undefined.instr.fail b/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_COPY_undefined.instr.fail new file mode 100644 index 000000000..4b971c1bb --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_COPY_undefined.instr.fail @@ -0,0 +1,7 @@ +/* Must FAIL code generation, see README.md and + docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md + COPY of an undefined instance (before: code generator segfault) */ +DEFINE INSTRUMENT Err_COPY_undefined() +TRACE +COMPONENT a = COPY(PREVIOUS) AT (0,0,0) ABSOLUTE +END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_out_of_range.instr.fail b/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_out_of_range.instr.fail new file mode 100644 index 000000000..428f273f4 --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_out_of_range.instr.fail @@ -0,0 +1,10 @@ +/* Must FAIL code generation, see README.md and + docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md + JUMP NEXT(2) from the 2nd of 3 components (before: jumped to component 2) */ +DEFINE INSTRUMENT Err_JUMP_out_of_range() +TRACE +COMPONENT a = Arm() AT (0,0,0) ABSOLUTE +COMPONENT b = Arm() AT (0,0,1) RELATIVE a + JUMP NEXT(2) WHEN (0) +COMPONENT c = Arm() AT (0,0,1) RELATIVE b +END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_unknown_target.instr.fail b/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_unknown_target.instr.fail new file mode 100644 index 000000000..64c2aae32 --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_unknown_target.instr.fail @@ -0,0 +1,9 @@ +/* Must FAIL code generation, see README.md and + docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md + JUMP to a name that is not a component (before: silently JUMP MYSELF) */ +DEFINE INSTRUMENT Err_JUMP_unknown_target() +TRACE +COMPONENT a = Arm() AT (0,0,0) ABSOLUTE +COMPONENT b = Arm() AT (0,0,1) RELATIVE a + JUMP nosuch WHEN (0) +END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_MYSELF_in_parameters.instr.fail b/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_MYSELF_in_parameters.instr.fail new file mode 100644 index 000000000..4aba7ce67 --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_MYSELF_in_parameters.instr.fail @@ -0,0 +1,8 @@ +/* Must FAIL code generation, see README.md and + docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md + MYSELF in the component's own parameters (before: silently the PREVIOUS instance) */ +DEFINE INSTRUMENT Err_MYSELF_in_parameters() +TRACE +COMPONENT a = Arm() AT (0,0,0) ABSOLUTE +COMPONENT b = Slit(xwidth=MYSELF, yheight=0.1) AT (0,0,1) RELATIVE a +END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_illegal_pointer_type.instr.fail b/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_illegal_pointer_type.instr.fail new file mode 100644 index 000000000..330f73420 --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_illegal_pointer_type.instr.fail @@ -0,0 +1,7 @@ +/* Must FAIL code generation, see README.md and + docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md + Illegal pointer type for an instrument parameter (before: garbled message) */ +DEFINE INSTRUMENT Err_illegal_pointer_type(int *x) +TRACE +COMPONENT a = Arm() AT (0,0,0) ABSOLUTE +END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_unknown_component.instr.fail b/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_unknown_component.instr.fail new file mode 100644 index 000000000..065e7d628 --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_unknown_component.instr.fail @@ -0,0 +1,7 @@ +/* Must FAIL code generation, see README.md and + docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md + Unknown component class (before: error message, then segfault) */ +DEFINE INSTRUMENT Err_unknown_component() +TRACE +COMPONENT a = NoSuchComponent() AT (0,0,0) ABSOLUTE +END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/README.md b/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/README.md new file mode 100644 index 000000000..58737885d --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/README.md @@ -0,0 +1,29 @@ +# Grammar error cases (must FAIL code generation) + +Each `*.instr.fail` file here is an instrument that the code generator must +**reject with an error**. They belong with the positive tests +`Unittest_JUMP_RELATIVE`, `Unittest_INHERIT_DIAPHRAGM` and +`Unittest_USERVARS_TYPES`. All of them document the decisions in +[ADR_20261007_GRAMMAR_FIXES](../../../../docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md). +They are mainly meant as reference cases for other implementations of the +grammar, such as mccode-antlr. + +The files use the `.instr.fail` extension so that test and documentation +tools, which look for `*.instr`, do not try to build them. To check one, +copy it to `*.instr` and run the code generator: + +```bash +cp Err_MYSELF_in_parameters.instr.fail /tmp/Err_MYSELF_in_parameters.instr +mcstas /tmp/Err_MYSELF_in_parameters.instr # must exit non-zero +``` + +| File | Case | Before the fix | Now | +|---|---|---|---| +| `Err_MYSELF_in_parameters` | `MYSELF` in the instance's own parameters | accepted, meant the previous instance | `ERROR: MYSELF can not be used here ...` | +| `Err_JUMP_unknown_target` | `JUMP` to a name that is no component | accepted, silently `JUMP MYSELF` | `JUMP at component b: target nosuch is not a component ...` | +| `Err_JUMP_out_of_range` | `JUMP NEXT(2)` past the last component | accepted, jumped to component 2 | `JUMP at component b: target NEXT_2 is not a component ...` | +| `Err_COPY_undefined` | `COPY(PREVIOUS)` on the first instance | code generator segfault | `ERROR: COPY of an undefined component instance ...` | +| `Err_unknown_component` | unknown component class | error message, then segfault | error message, exit code 1 | +| `Err_illegal_pointer_type` | `int *x` instrument parameter | garbled message (`$s*`, wrong line) | `ERROR: Illegal type int* for instrument parameter x ...` | + +`MYSELF` remains valid from `WHEN` onwards (`WHEN`, `AT`, `ROTATED`, `JUMP`, ...). diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_DIAPHRAGM/Unittest_INHERIT_DIAPHRAGM.instr b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_DIAPHRAGM/Unittest_INHERIT_DIAPHRAGM.instr new file mode 100644 index 000000000..498030097 --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_DIAPHRAGM/Unittest_INHERIT_DIAPHRAGM.instr @@ -0,0 +1,67 @@ +/******************************************************************************* +* Instrument: Unittest_INHERIT_DIAPHRAGM +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* %INSTRUMENT_SITE: Tests_grammar +* +* Whole-component INHERIT: Diaphragm (DEFINE COMPONENT Diaphragm INHERIT Slit) +* +* %D +* Unit test for whole-component inheritance, see +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* Diaphragm.comp is just "DEFINE COMPONENT Diaphragm INHERIT Slit" + END, +* so all of its code sections must come from Slit. +* +* One unit of intensity is emitted uniformly from a 1x1 m square: +* - a Slit of 0.5x0.5 m passes 1/4: AfterSlit_I = 0.25 +* - a Diaphragm of 0.25x0.25 m passes 1/16 of the source: AfterDiaphragm_I = 0.0625 +* +* Before the fix, a component inheriting all of its code got none of it, so +* the Diaphragm let everything through (AfterDiaphragm_I = 0.25). +* +* %Example: Detector: AfterSlit_I=0.25 +* %Example: Detector: AfterDiaphragm_I=0.0625 +* +* %P +* Pp0: [1] Dummy input parameter used internally +* +* %L +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %E +*******************************************************************************/ +DEFINE INSTRUMENT Unittest_INHERIT_DIAPHRAGM(Pp0=0) + +INITIALIZE +%{ + Pp0=1.0/mcget_ncount(); +%} + +TRACE + +COMPONENT Src = Arm() + AT (0,0,0) ABSOLUTE +EXTEND %{ + x = rand01()-0.5; + y = rand01()-0.5; + p = INSTRUMENT_GETPAR(Pp0); + vz=1000; +%} + +COMPONENT Slit = Slit(xwidth=0.5, yheight=0.5) + AT (0,0,0.1) ABSOLUTE + +COMPONENT AfterSlit = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterSlit", nx=10, ny=10) + AT (0,0,0.2) ABSOLUTE + +COMPONENT Diaphragm = Diaphragm(xwidth=0.25, yheight=0.25) + AT (0,0,0.3) ABSOLUTE + +COMPONENT AfterDiaphragm = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterDiaphragm", nx=10, ny=10) + AT (0,0,0.4) ABSOLUTE + +END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_JUMP_RELATIVE/Unittest_JUMP_RELATIVE.instr b/mcstas-comps/examples/Tests_grammar/Unittest_JUMP_RELATIVE/Unittest_JUMP_RELATIVE.instr new file mode 100644 index 000000000..499b4c556 --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_JUMP_RELATIVE/Unittest_JUMP_RELATIVE.instr @@ -0,0 +1,81 @@ +/******************************************************************************* +* Instrument: Unittest_JUMP_RELATIVE +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* %INSTRUMENT_SITE: Tests_grammar +* +* Relative JUMP targets: JUMP PREVIOUS(n) and JUMP NEXT(n) +* +* %D +* Unit test for relative JUMP targets, see +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* One unit of intensity is emitted from a 1x1 m square. +* - Back jumps to PREVIOUS(2), i.e. to Target, 'iter' times in total +* (JUMP ... ITERATE), so the Mon monitor in between is passed 'iter' times +* and reads Mon_I = iter. +* - Skip jumps to NEXT(2), i.e. over Skipped and onto Landed, so +* Skipped_I = 0 and Landed_I = 1. +* +* Before the fix, relative targets were used as absolute component indices: +* JUMP PREVIOUS(n) crashed the code generator and JUMP NEXT(n) went to +* component n. JUMP PREVIOUS is PREVIOUS(1) and JUMP NEXT is NEXT(1). +* +* %Example: iter=3 Detector: Mon_I=3 +* %Example: iter=3 Detector: Landed_I=1 +* %Example: iter=3 Detector: Skipped_I=0 +* +* %P +* iter: [1] Number of passes through Mon (JUMP ITERATE count) +* Pp0: [1] Dummy input parameter used internally +* +* %L +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %E +*******************************************************************************/ +DEFINE INSTRUMENT Unittest_JUMP_RELATIVE(int iter=3, Pp0=0) + +INITIALIZE +%{ + Pp0=1.0/mcget_ncount(); +%} + +TRACE + +COMPONENT Src = Arm() + AT (0,0,0) ABSOLUTE +EXTEND %{ + x = rand01()-0.5; + y = rand01()-0.5; + p = INSTRUMENT_GETPAR(Pp0); + vz=1000; +%} + +/* JUMP PREVIOUS(2) from Back lands here */ +COMPONENT Target = Arm() + AT (0,0,0) ABSOLUTE + +COMPONENT Mon = PSD_monitor(xwidth=2.0, yheight=2.0, filename="Mon", nx=10, ny=10) + AT (0,0,0.1) ABSOLUTE + +COMPONENT Back = Arm() + AT (0,0,0.1) ABSOLUTE +JUMP PREVIOUS(2) ITERATE iter + +COMPONENT Skip = Arm() + AT (0,0,0.2) ABSOLUTE +JUMP NEXT(2) WHEN (1) + +/* jumped over */ +COMPONENT Skipped = PSD_monitor(xwidth=2.0, yheight=2.0, filename="Skipped", nx=10, ny=10) + AT (0,0,0.3) ABSOLUTE + +/* JUMP NEXT(2) from Skip lands here */ +COMPONENT Landed = PSD_monitor(xwidth=2.0, yheight=2.0, filename="Landed", nx=10, ny=10) + AT (0,0,0.4) ABSOLUTE + +END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_USERVARS_TYPES.instr b/mcstas-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_USERVARS_TYPES.instr new file mode 100644 index 000000000..f9b401d3b --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_USERVARS_TYPES.instr @@ -0,0 +1,77 @@ +/******************************************************************************* +* Instrument: Unittest_USERVARS_TYPES +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* %INSTRUMENT_SITE: Tests_grammar +* +* USERVARS of non-double type, several declarations per line, USERVARS+EXTEND +* +* %D +* Unit test for USERVARS/DECLARE handling in the code generator, see +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* One unit of intensity is emitted from a 1x1 m square. +* - USERVARS declares "int flag; double wsum;" on ONE line. Before the fix +* only the first declaration on a line was found, so wsum was missing. +* - flag (an int) is set to 1 for x>0 and read by Monitor_nD (user1="flag") +* through particle_getvar(). Before the fix its memory was read as a +* double (garbage); now it is converted, and the [0.5 1.5] range holds +* half of the beam: Flag_I = 0.5. (2 bins: a 1-bin Monitor_nD is 0-D and +* ignores the limits.) +* Note: USERVARS used with Monitor_nD should still be declared double; +* int is used here only to test the conversion. +* - Unittest_UV_check (local component) tests %include "x.hpp", component +* USERVARS followed by EXTEND, and two DECLARE variables on one line. It +* absorbs the particle if its EXTEND-declared USERVAR is missing, so +* AfterCheck_I = 1. +* +* %Example: Detector: AfterCheck_I=1 +* %Example: Detector: Flag_I=0.5 +* +* %P +* Pp0: [1] Dummy input parameter used internally +* +* %L +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %E +*******************************************************************************/ +DEFINE INSTRUMENT Unittest_USERVARS_TYPES(Pp0=0) + +USERVARS +%{ + int flag; double wsum; +%} + +INITIALIZE +%{ + Pp0=1.0/mcget_ncount(); +%} + +TRACE + +COMPONENT Src = Arm() + AT (0,0,0) ABSOLUTE +EXTEND %{ + x = rand01()-0.5; + y = rand01()-0.5; + p = INSTRUMENT_GETPAR(Pp0); + vz=1000; + flag = (x > 0); + wsum = p; +%} + +COMPONENT Check = Unittest_UV_check() + AT (0,0,0.1) ABSOLUTE + +COMPONENT AfterCheck = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterCheck", nx=10, ny=10) + AT (0,0,0.2) ABSOLUTE + +COMPONENT Flag = Monitor_nD(xwidth=2.0, yheight=2.0, user1="flag", username1="flag", + options="user1 limits=[0.5 1.5] bins=2", filename="Flag") + AT (0,0,0.3) ABSOLUTE + +END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_UV_check.comp b/mcstas-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_UV_check.comp new file mode 100644 index 000000000..9ef4847a8 --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_UV_check.comp @@ -0,0 +1,73 @@ +/******************************************************************************* +* +* McStas, neutron ray-tracing package +* Copyright (C) 1997-2026, All rights reserved +* DTU Physics, Kgs. Lyngby, Denmark +* +* Component: Unittest_UV_check +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* +* Grammar unit test helper for Unittest_USERVARS_TYPES +* +* %D +* Exercises code-generator fixes from +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md: +* - %include "unittest_uv.hpp" (multi-character extension) in SHARE, +* - USERVARS followed by EXTEND: uv_b is declared in the EXTEND block and +* was silently dropped before the fix, +* - two DECLARE variables on one line: only the first was found before. +* +* In TRACE the component writes this instance's uv_b USERVAR (by name, +* "uv_b_") and reads it back. If that fails, the particle is +* absorbed, so a monitor after it reads 0 instead of the full intensity. +* +* %P +* (none) +* +* %E +*******************************************************************************/ +DEFINE COMPONENT Unittest_UV_check +SETTING PARAMETERS () + +SHARE +%{ +%include "unittest_uv.hpp" +%} + +USERVARS +%{ + double uv_a; +%} +EXTEND +%{ + double uv_b; +%} + +DECLARE +%{ + double uvc_value; int uvc_count; +%} + +INITIALIZE +%{ + uvc_value = UNITTEST_UV_VALUE; + uvc_count = 0; +%} + +TRACE +%{ + char uvc_name[64]; + double uvc_back; + int uvc_fail; + sprintf(uvc_name, "uv_b_%li", _comp->_index); + if (particle_setvar_void(_particle, uvc_name, &uvc_value)) ABSORB; + uvc_back = particle_getvar(_particle, uvc_name, &uvc_fail); + if (uvc_fail || uvc_back != uvc_value) ABSORB; + uvc_count++; +%} + +END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/unittest_uv.hpp b/mcstas-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/unittest_uv.hpp new file mode 100644 index 000000000..04de7a437 --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/unittest_uv.hpp @@ -0,0 +1,4 @@ +/* Header with a multi-character extension, %include'd from a C block in + Unittest_UV_check.comp (before the fix only one-character extensions + were accepted there, "x.hpp" was taken as a library "x.hpp.h"). */ +#define UNITTEST_UV_VALUE 1.0 diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_COPY_undefined.instr.fail b/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_COPY_undefined.instr.fail new file mode 100644 index 000000000..4b971c1bb --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_COPY_undefined.instr.fail @@ -0,0 +1,7 @@ +/* Must FAIL code generation, see README.md and + docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md + COPY of an undefined instance (before: code generator segfault) */ +DEFINE INSTRUMENT Err_COPY_undefined() +TRACE +COMPONENT a = COPY(PREVIOUS) AT (0,0,0) ABSOLUTE +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_out_of_range.instr.fail b/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_out_of_range.instr.fail new file mode 100644 index 000000000..428f273f4 --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_out_of_range.instr.fail @@ -0,0 +1,10 @@ +/* Must FAIL code generation, see README.md and + docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md + JUMP NEXT(2) from the 2nd of 3 components (before: jumped to component 2) */ +DEFINE INSTRUMENT Err_JUMP_out_of_range() +TRACE +COMPONENT a = Arm() AT (0,0,0) ABSOLUTE +COMPONENT b = Arm() AT (0,0,1) RELATIVE a + JUMP NEXT(2) WHEN (0) +COMPONENT c = Arm() AT (0,0,1) RELATIVE b +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_unknown_target.instr.fail b/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_unknown_target.instr.fail new file mode 100644 index 000000000..64c2aae32 --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_JUMP_unknown_target.instr.fail @@ -0,0 +1,9 @@ +/* Must FAIL code generation, see README.md and + docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md + JUMP to a name that is not a component (before: silently JUMP MYSELF) */ +DEFINE INSTRUMENT Err_JUMP_unknown_target() +TRACE +COMPONENT a = Arm() AT (0,0,0) ABSOLUTE +COMPONENT b = Arm() AT (0,0,1) RELATIVE a + JUMP nosuch WHEN (0) +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_MYSELF_in_parameters.instr.fail b/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_MYSELF_in_parameters.instr.fail new file mode 100644 index 000000000..4aba7ce67 --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_MYSELF_in_parameters.instr.fail @@ -0,0 +1,8 @@ +/* Must FAIL code generation, see README.md and + docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md + MYSELF in the component's own parameters (before: silently the PREVIOUS instance) */ +DEFINE INSTRUMENT Err_MYSELF_in_parameters() +TRACE +COMPONENT a = Arm() AT (0,0,0) ABSOLUTE +COMPONENT b = Slit(xwidth=MYSELF, yheight=0.1) AT (0,0,1) RELATIVE a +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_illegal_pointer_type.instr.fail b/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_illegal_pointer_type.instr.fail new file mode 100644 index 000000000..330f73420 --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_illegal_pointer_type.instr.fail @@ -0,0 +1,7 @@ +/* Must FAIL code generation, see README.md and + docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md + Illegal pointer type for an instrument parameter (before: garbled message) */ +DEFINE INSTRUMENT Err_illegal_pointer_type(int *x) +TRACE +COMPONENT a = Arm() AT (0,0,0) ABSOLUTE +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_unknown_component.instr.fail b/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_unknown_component.instr.fail new file mode 100644 index 000000000..065e7d628 --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/Err_unknown_component.instr.fail @@ -0,0 +1,7 @@ +/* Must FAIL code generation, see README.md and + docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md + Unknown component class (before: error message, then segfault) */ +DEFINE INSTRUMENT Err_unknown_component() +TRACE +COMPONENT a = NoSuchComponent() AT (0,0,0) ABSOLUTE +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/README.md b/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/README.md new file mode 100644 index 000000000..59dbda756 --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_GRAMMAR_ERRORS/README.md @@ -0,0 +1,29 @@ +# Grammar error cases (must FAIL code generation) + +Each `*.instr.fail` file here is an instrument that the code generator must +**reject with an error**. They belong with the positive tests +`Unittest_JUMP_RELATIVE`, `Unittest_INHERIT_DIAPHRAGM` and +`Unittest_USERVARS_TYPES`. All of them document the decisions in +[ADR_20261007_GRAMMAR_FIXES](../../../../docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md). +They are mainly meant as reference cases for other implementations of the +grammar, such as mccode-antlr. + +The files use the `.instr.fail` extension so that test and documentation +tools, which look for `*.instr`, do not try to build them. To check one, +copy it to `*.instr` and run the code generator: + +```bash +cp Err_MYSELF_in_parameters.instr.fail /tmp/Err_MYSELF_in_parameters.instr +mcxtrace /tmp/Err_MYSELF_in_parameters.instr # must exit non-zero +``` + +| File | Case | Before the fix | Now | +|---|---|---|---| +| `Err_MYSELF_in_parameters` | `MYSELF` in the instance's own parameters | accepted, meant the previous instance | `ERROR: MYSELF can not be used here ...` | +| `Err_JUMP_unknown_target` | `JUMP` to a name that is no component | accepted, silently `JUMP MYSELF` | `JUMP at component b: target nosuch is not a component ...` | +| `Err_JUMP_out_of_range` | `JUMP NEXT(2)` past the last component | accepted, jumped to component 2 | `JUMP at component b: target NEXT_2 is not a component ...` | +| `Err_COPY_undefined` | `COPY(PREVIOUS)` on the first instance | code generator segfault | `ERROR: COPY of an undefined component instance ...` | +| `Err_unknown_component` | unknown component class | error message, then segfault | error message, exit code 1 | +| `Err_illegal_pointer_type` | `int *x` instrument parameter | garbled message (`$s*`, wrong line) | `ERROR: Illegal type int* for instrument parameter x ...` | + +`MYSELF` remains valid from `WHEN` onwards (`WHEN`, `AT`, `ROTATED`, `JUMP`, ...). diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_DIAPHRAGM/Unittest_INHERIT_DIAPHRAGM.instr b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_DIAPHRAGM/Unittest_INHERIT_DIAPHRAGM.instr new file mode 100644 index 000000000..b432ec2b9 --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_DIAPHRAGM/Unittest_INHERIT_DIAPHRAGM.instr @@ -0,0 +1,67 @@ +/******************************************************************************* +* Instrument: Unittest_INHERIT_DIAPHRAGM +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* %INSTRUMENT_SITE: Tests_grammar +* +* Whole-component INHERIT: Diaphragm (DEFINE COMPONENT Diaphragm INHERIT Slit) +* +* %D +* Unit test for whole-component inheritance, see +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* Diaphragm.comp is just "DEFINE COMPONENT Diaphragm INHERIT Slit" + END, +* so all of its code sections must come from Slit. +* +* One unit of intensity is emitted uniformly from a 1x1 m square: +* - a Slit of 0.5x0.5 m passes 1/4: AfterSlit_I = 0.25 +* - a Diaphragm of 0.25x0.25 m passes 1/16 of the source: AfterDiaphragm_I = 0.0625 +* +* Before the fix, a component inheriting all of its code got none of it, so +* the Diaphragm let everything through (AfterDiaphragm_I = 0.25). +* +* %Example: Detector: AfterSlit_I=0.25 +* %Example: Detector: AfterDiaphragm_I=0.0625 +* +* %P +* Pp0: [1] Dummy input parameter used internally +* +* %L +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %E +*******************************************************************************/ +DEFINE INSTRUMENT Unittest_INHERIT_DIAPHRAGM(Pp0=0) + +INITIALIZE +%{ + Pp0=1.0/mcget_ncount(); +%} + +TRACE + +COMPONENT Src = Arm() + AT (0,0,0) ABSOLUTE +EXTEND %{ + x = rand01()-0.5; + y = rand01()-0.5; + p = INSTRUMENT_GETPAR(Pp0); + kz=1; +%} + +COMPONENT Slit = Slit(xwidth=0.5, yheight=0.5) + AT (0,0,0.1) ABSOLUTE + +COMPONENT AfterSlit = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterSlit", nx=10, ny=10) + AT (0,0,0.2) ABSOLUTE + +COMPONENT Diaphragm = Diaphragm(xwidth=0.25, yheight=0.25) + AT (0,0,0.3) ABSOLUTE + +COMPONENT AfterDiaphragm = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterDiaphragm", nx=10, ny=10) + AT (0,0,0.4) ABSOLUTE + +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_JUMP_RELATIVE/Unittest_JUMP_RELATIVE.instr b/mcxtrace-comps/examples/Tests_grammar/Unittest_JUMP_RELATIVE/Unittest_JUMP_RELATIVE.instr new file mode 100644 index 000000000..9bf543426 --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_JUMP_RELATIVE/Unittest_JUMP_RELATIVE.instr @@ -0,0 +1,81 @@ +/******************************************************************************* +* Instrument: Unittest_JUMP_RELATIVE +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* %INSTRUMENT_SITE: Tests_grammar +* +* Relative JUMP targets: JUMP PREVIOUS(n) and JUMP NEXT(n) +* +* %D +* Unit test for relative JUMP targets, see +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* One unit of intensity is emitted from a 1x1 m square. +* - Back jumps to PREVIOUS(2), i.e. to Target, 'iter' times in total +* (JUMP ... ITERATE), so the Mon monitor in between is passed 'iter' times +* and reads Mon_I = iter. +* - Skip jumps to NEXT(2), i.e. over Skipped and onto Landed, so +* Skipped_I = 0 and Landed_I = 1. +* +* Before the fix, relative targets were used as absolute component indices: +* JUMP PREVIOUS(n) crashed the code generator and JUMP NEXT(n) went to +* component n. JUMP PREVIOUS is PREVIOUS(1) and JUMP NEXT is NEXT(1). +* +* %Example: iter=3 Detector: Mon_I=3 +* %Example: iter=3 Detector: Landed_I=1 +* %Example: iter=3 Detector: Skipped_I=0 +* +* %P +* iter: [1] Number of passes through Mon (JUMP ITERATE count) +* Pp0: [1] Dummy input parameter used internally +* +* %L +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %E +*******************************************************************************/ +DEFINE INSTRUMENT Unittest_JUMP_RELATIVE(int iter=3, Pp0=0) + +INITIALIZE +%{ + Pp0=1.0/mcget_ncount(); +%} + +TRACE + +COMPONENT Src = Arm() + AT (0,0,0) ABSOLUTE +EXTEND %{ + x = rand01()-0.5; + y = rand01()-0.5; + p = INSTRUMENT_GETPAR(Pp0); + kz=1; +%} + +/* JUMP PREVIOUS(2) from Back lands here */ +COMPONENT Target = Arm() + AT (0,0,0) ABSOLUTE + +COMPONENT Mon = PSD_monitor(xwidth=2.0, yheight=2.0, filename="Mon", nx=10, ny=10) + AT (0,0,0.1) ABSOLUTE + +COMPONENT Back = Arm() + AT (0,0,0.1) ABSOLUTE +JUMP PREVIOUS(2) ITERATE iter + +COMPONENT Skip = Arm() + AT (0,0,0.2) ABSOLUTE +JUMP NEXT(2) WHEN (1) + +/* jumped over */ +COMPONENT Skipped = PSD_monitor(xwidth=2.0, yheight=2.0, filename="Skipped", nx=10, ny=10) + AT (0,0,0.3) ABSOLUTE + +/* JUMP NEXT(2) from Skip lands here */ +COMPONENT Landed = PSD_monitor(xwidth=2.0, yheight=2.0, filename="Landed", nx=10, ny=10) + AT (0,0,0.4) ABSOLUTE + +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_USERVARS_TYPES.instr b/mcxtrace-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_USERVARS_TYPES.instr new file mode 100644 index 000000000..15a8bfb61 --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_USERVARS_TYPES.instr @@ -0,0 +1,77 @@ +/******************************************************************************* +* Instrument: Unittest_USERVARS_TYPES +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* %INSTRUMENT_SITE: Tests_grammar +* +* USERVARS of non-double type, several declarations per line, USERVARS+EXTEND +* +* %D +* Unit test for USERVARS/DECLARE handling in the code generator, see +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* One unit of intensity is emitted from a 1x1 m square. +* - USERVARS declares "int flag; double wsum;" on ONE line. Before the fix +* only the first declaration on a line was found, so wsum was missing. +* - flag (an int) is set to 1 for x>0 and read by Monitor_nD (user1="flag") +* through particle_getvar(). Before the fix its memory was read as a +* double (garbage); now it is converted, and the [0.5 1.5] range holds +* half of the beam: Flag_I = 0.5. (2 bins: a 1-bin Monitor_nD is 0-D and +* ignores the limits.) +* Note: USERVARS used with Monitor_nD should still be declared double; +* int is used here only to test the conversion. +* - Unittest_UV_check (local component) tests %include "x.hpp", component +* USERVARS followed by EXTEND, and two DECLARE variables on one line. It +* absorbs the particle if its EXTEND-declared USERVAR is missing, so +* AfterCheck_I = 1. +* +* %Example: Detector: AfterCheck_I=1 +* %Example: Detector: Flag_I=0.5 +* +* %P +* Pp0: [1] Dummy input parameter used internally +* +* %L +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %E +*******************************************************************************/ +DEFINE INSTRUMENT Unittest_USERVARS_TYPES(Pp0=0) + +USERVARS +%{ + int flag; double wsum; +%} + +INITIALIZE +%{ + Pp0=1.0/mcget_ncount(); +%} + +TRACE + +COMPONENT Src = Arm() + AT (0,0,0) ABSOLUTE +EXTEND %{ + x = rand01()-0.5; + y = rand01()-0.5; + p = INSTRUMENT_GETPAR(Pp0); + kz=1; + flag = (x > 0); + wsum = p; +%} + +COMPONENT Check = Unittest_UV_check() + AT (0,0,0.1) ABSOLUTE + +COMPONENT AfterCheck = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterCheck", nx=10, ny=10) + AT (0,0,0.2) ABSOLUTE + +COMPONENT Flag = Monitor_nD(xwidth=2.0, yheight=2.0, user1="flag", username1="flag", + options="user1 limits=[0.5 1.5] bins=2", filename="Flag") + AT (0,0,0.3) ABSOLUTE + +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_UV_check.comp b/mcxtrace-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_UV_check.comp new file mode 100644 index 000000000..e319419fc --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/Unittest_UV_check.comp @@ -0,0 +1,73 @@ +/******************************************************************************* +* +* McXtrace, X-ray tracing package +* Copyright (C) 1997-2026, All rights reserved +* DTU Physics, Kgs. Lyngby, Denmark +* +* Component: Unittest_UV_check +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* +* Grammar unit test helper for Unittest_USERVARS_TYPES +* +* %D +* Exercises code-generator fixes from +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md: +* - %include "unittest_uv.hpp" (multi-character extension) in SHARE, +* - USERVARS followed by EXTEND: uv_b is declared in the EXTEND block and +* was silently dropped before the fix, +* - two DECLARE variables on one line: only the first was found before. +* +* In TRACE the component writes this instance's uv_b USERVAR (by name, +* "uv_b_") and reads it back. If that fails, the particle is +* absorbed, so a monitor after it reads 0 instead of the full intensity. +* +* %P +* (none) +* +* %E +*******************************************************************************/ +DEFINE COMPONENT Unittest_UV_check +SETTING PARAMETERS () + +SHARE +%{ +%include "unittest_uv.hpp" +%} + +USERVARS +%{ + double uv_a; +%} +EXTEND +%{ + double uv_b; +%} + +DECLARE +%{ + double uvc_value; int uvc_count; +%} + +INITIALIZE +%{ + uvc_value = UNITTEST_UV_VALUE; + uvc_count = 0; +%} + +TRACE +%{ + char uvc_name[64]; + double uvc_back; + int uvc_fail; + sprintf(uvc_name, "uv_b_%li", _comp->_index); + if (particle_setvar_void(_particle, uvc_name, &uvc_value)) ABSORB; + uvc_back = particle_getvar(_particle, uvc_name, &uvc_fail); + if (uvc_fail || uvc_back != uvc_value) ABSORB; + uvc_count++; +%} + +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/unittest_uv.hpp b/mcxtrace-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/unittest_uv.hpp new file mode 100644 index 000000000..04de7a437 --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_USERVARS_TYPES/unittest_uv.hpp @@ -0,0 +1,4 @@ +/* Header with a multi-character extension, %include'd from a C block in + Unittest_UV_check.comp (before the fix only one-character extensions + were accepted there, "x.hpp" was taken as a library "x.hpp.h"). */ +#define UNITTEST_UV_VALUE 1.0 From aa7196036d3cea33574ada2b07bad138f16742e9 Mon Sep 17 00:00:00 2001 From: Peter Willendrup Date: Wed, 7 Oct 2026 14:28:34 +0200 Subject: [PATCH 4/8] cogen: keep expressions in {...} vector parameters; FUNNEL guards - Vector parameters given as {...} are now split at top-level commas and each element is written into the generated code as the C expression it is. Before, the elements were parsed with strtod, so an expression such as {1, 3.2*0.0219, ...} became {1, 3.2, 0, 0.0219, ...}: too many, shifted values. That affected IN15_Vpolariser/WASP_Vpolariser (Pol_guide_vmirror) in ILL_H5 and ILL_H5_new, which now get their intended reflectivity parameters: intensity downstream of these polarisers changes by factors of ~3-12 (results change, intended). Instrument parameters can now also be used in such vectors. All other shipped instruments generate the same values (literals are now copied as written, e.g. "1.0" instead of "1"). - Removed the "static vectors support literal numbers ONLY" warning, which no longer applies, and the unused copy of the old parser in pygen.c.in. - FUNNEL: check the particle buffer allocations, and reset livebatchsize at the start of each batch (a SPLIT in the previous batch changes it). Co-Authored-By: Claude Opus 5.5 --- mccode/src/cogen.c.in | 73 ++++++++++++++++++++++--------------------- mccode/src/pygen.c.in | 24 -------------- 2 files changed, 37 insertions(+), 60 deletions(-) diff --git a/mccode/src/cogen.c.in b/mccode/src/cogen.c.in index a53f2d15a..0581fdb51 100644 --- a/mccode/src/cogen.c.in +++ b/mccode/src/cogen.c.in @@ -111,32 +111,34 @@ int get_codeblock_vars_allcustom(struct code_block *code, List custom_vars, int get_codeblock_vars(struct code_block *code, List vars, List types, char* block_name, char* movetoblock_name); -/* Parse a literal vector "{1, 2.5, 3}" into values[] (if not NULL) and return - the number of elements. Call with values=NULL first to count. Only numeric - literals are meaningful: anything strtod cannot read is skipped char by - char and still counted. */ -int parse_curlybrackets_vector(char* string, double* values) { - char* s = string; - int vidx = 0; - char* r; - while(*s != '\0') { - // jump to a non-trivial position - if (*s == ' ' || *s == ',' || *s == '{') { - ++s; +/* Split a vector initialiser "{1, 3.2*0.0219, f(a,b)}" into its elements at + top-level commas (commas inside (), [] or {} do not split). Returns the + number of elements; when elems is not NULL, elems[i] receives a str_dup()'ed, + trimmed copy of element i. The elements are C expressions, written as-is + into the generated code. */ +int split_curlybrackets_vector(char* string, char** elems) { + int depth = 0, n = 0; + char *start = NULL, *s; + for (s = string; *s; s++) { + if (depth == 0) { + if (*s == '{') { depth = 1; start = s + 1; } continue; } - if (*s == '}') return vidx; - - // extract a value - double val = strtod(s, &r); - if (values!=NULL) values[vidx] = val; - ++vidx; - - // iterate - if (*r == '\0') return vidx; - s = r + 1; + if (*s == '(' || *s == '[' || *s == '{') depth++; + else if (*s == ')' || *s == ']' || *s == '}') depth--; + if (depth == 0 || (depth == 1 && *s == ',')) { /* element is [start, s) */ + char *b = start, *e = s; + while (b < e && (*b == ' ' || *b == '\t' || *b == '\n')) b++; + while (e > b && (e[-1] == ' ' || e[-1] == '\t' || e[-1] == '\n')) e--; + if (e > b) { + if (elems) elems[n] = str_dup_n(b, e - b); + n++; + } + start = s + 1; + if (depth == 0) break; + } } - return vidx; + return n; } /* Functions for outputting code. */ @@ -550,11 +552,7 @@ int cogen_comp_declare(struct comp_inst *comp) entry = symtab_lookup(comp->setpar, c_formal->id); char *val = exp_tostring(entry->val); /* replaced by instrument parameter when exists */ if (val[0] == '{') { - int vl = parse_curlybrackets_vector(val, NULL); - printf("\nWARNING:\n The parameter %s of %s is initialized \n using a static {,,,} vector.\n",c_formal->id,comp->name); - printf(" -> Such static vectors support literal numbers ONLY.\n"); - printf(" -> Any vector use of variables or defines must happen via a \n"); - printf(" DECLARE/INITIALIZE pointer.\n\n"); + int vl = split_curlybrackets_vector(val, NULL); coutf(" %s %s[%i];", instr_formal_type_names_real[c_formal->type], c_formal->id, vl); } else { coutf(" %s* %s;", instr_formal_type_names_real[c_formal->type], c_formal->id); @@ -652,17 +650,15 @@ static void cogen_comp_init_par(struct comp_inst *comp, struct instr_def *instr, } else if (par->type == instr_type_vector) { if (val[0] == '{') { - int vl = parse_curlybrackets_vector(val, NULL); - double *values = mem((vl + 1) * sizeof(double)); + int vl = split_curlybrackets_vector(val, NULL); + char **elems = mem((vl + 1) * sizeof(char *)); int i; - parse_curlybrackets_vector(val, values); + split_curlybrackets_vector(val, elems); for (i=0; iname, par->id, i, num); + coutf(" _%s_var._parameters.%s[%i] = %s;", comp->name, par->id, i, elems[i]); + str_free(elems[i]); } - memfree(values); + memfree(elems); } else coutf(" _%s_var._parameters.%s = %s; // default pointer allocation", comp->name, par->id, val); } @@ -2225,6 +2221,10 @@ int cogen_rt_funnel(struct instr_def *instr) coutf(" // create particles struct and pointer arrays (same memory used by all batches)"); coutf(" _class_particle* particles = malloc(gpu_innerloop*sizeof(_class_particle));"); coutf(" _class_particle* pbuffer = malloc(gpu_innerloop*sizeof(_class_particle));"); + coutf(" if (!particles || !pbuffer) {"); + coutf(" fprintf(stderr, \"Error: cannot allocate %%ld particles (raytrace_all_funnel). Use a smaller --gpu_innerloop.\\n\", gpu_innerloop);"); + coutf(" exit(1);"); + coutf(" }"); coutf(" long livebatchsize = gpu_innerloop;"); coutf(""); @@ -2241,6 +2241,7 @@ int cogen_rt_funnel(struct instr_def *instr) coutf(""); // init batch + coutf(" livebatchsize = gpu_innerloop; // a SPLIT in the previous batch may have changed it"); coutf(" // init particles"); coutf(" #pragma acc parallel loop present(particles[0:livebatchsize])"); coutf(" for (unsigned long pidx=0 ; pidx < livebatchsize ; pidx++) {"); diff --git a/mccode/src/pygen.c.in b/mccode/src/pygen.c.in index c28562308..987672b03 100644 --- a/mccode/src/pygen.c.in +++ b/mccode/src/pygen.c.in @@ -101,30 +101,6 @@ int get_codeblock_vars_allcustom(struct code_block *code, List custom_vars, int get_codeblock_vars(struct code_block *code, List vars, List types, char* block_name, char* movetoblock_name); -int parse_curlybrackets_vector(char* string, double* values) { - char* s = string; - int vidx = 0; - char* r; - while(*s != '\0') { - // jump to a non-trivial position - if (*s == ' ' || *s == ',' || *s == '{') { - ++s; - continue; - } - if (*s == '}') return vidx; - - // extract a value - double val = strtod(s, &r); - if (values!=NULL) values[vidx] = val; - ++vidx; - - // iterate - if (*r == '\0') return vidx; - s = r + 1; - } - return vidx; -} - /* Functions for outputting code. */ /* Handle for output file. */ From 4eab34a80b6f026252c6ad79b718882ef0882eb1 Mon Sep 17 00:00:00 2001 From: Peter Willendrup Date: Wed, 7 Oct 2026 14:56:09 +0200 Subject: [PATCH 5/8] ADR_20261007_GRAMMAR_FIXES: add {...} vector expressions Document that vector parameters keep their expressions, and the resulting change in ILL_H5/ILL_H5_new (IN15/WASP V-cavity polarisers). Co-Authored-By: Claude Opus 5.5 --- .../ADR-records/ADR_20261007_GRAMMAR_FIXES.md | 37 +++++++++++++++---- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md b/docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md index 64af54cf3..ab4cb6a82 100644 --- a/docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +++ b/docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md @@ -50,9 +50,15 @@ Fix the following in the generator. No new keywords are introduced. having their memory read as a `double`. Arrays, pointers and structs report failure. USERVARS used with Monitor_nD should still be declared `double`. -9. **Literal vector parameters** (`par={1.0, 0.0219, ...}`) are written - to the generated C with full double precision instead of `%g` - (6 significant digits). +9. **Vector parameters given as `{...}`** are split at top-level commas, + and each element is written into the generated C as the expression it + is. Before, the elements were parsed as plain numbers (`strtod`), so + an expression became several wrong elements: `{1, 3.2*0.0219, ...}` + gave `{1, 3.2, 0, 0.0219, ...}`, one element too many and shifted. + Literals were also rounded to 6 significant digits (`%g`); they are now + copied exactly as written. Instrument parameters and function calls can + be used in such vectors. The vector length is still fixed at code + generation time. ## Consequences @@ -67,10 +73,17 @@ Fix the following in the generator. No new keywords are introduced. `MYSELF` in component parameters, or `JUMP` to a non-existent target, now fail to generate with an error message. No shipped instrument does either. -* Vector literals with more than 6 significant digits shift by at most - 5e-6 relative. A worst-case test (all `Pol_mirror` reflectivity - parameters given 8 digits) showed no change in monitor output. Shipped - literals are reproduced exactly. +* `ILL_H5` and `ILL_H5_new` **change results**: their `IN15_Vpolariser` / + `WASP_Vpolariser` (`Pol_guide_vmirror`) use expressions such as + `3.2*0.0219` in `rPar`/`rUpPar`/`rDownPar` and now get the intended + reflectivity parameters. Intensity downstream of these polarisers rises + by factors of about 3 to 12 (e.g. `H511_IN15_Detector` from 0 to + 1.6e4); `H5_I` and the other detectors are unchanged. Their `%Example` + values may need updating. +* Vector literals with more than 6 significant digits are now exact; the + old rounding was at most 5e-6 relative. A worst-case test (all + `Pol_mirror` reflectivity parameters given 8 digits) showed no change in + monitor output. * All other shipped instruments are unaffected. All 332 McStas and 112 McXtrace examples generate the same code as before, apart from cosmetic lines and the `particle_getvar()` casts. Representative @@ -95,6 +108,16 @@ COMPONENT s = Slit(xwidth=MYSELF) /* error */ COMPONENT s = Slit(xwidth=0.1) WHEN (MYSELF) /* ok, MYSELF -> s */ ``` +Vector parameters with expressions (as in ILL_H5): + +``` +rUpPar={1, 3.2*0.0219, 4.07, 1, 0.003} +/* now: 5 elements, rUpPar[1] = 3.2 * 0.0219 */ +/* before: {1, 3.2, 0, 0.0219, 4.07, 1, 0.003}: 7 elements, wrong values */ +``` + +Instrument parameters can now be used too, e.g. `rUpPar={1, 0.0219, 4.07, 2*m, 0.003}`. + Whole-component inheritance: ``` From e9c2c8bffe72333be09e7116e9dd56df6f334bbc Mon Sep 17 00:00:00 2001 From: Peter Willendrup Date: Wed, 7 Oct 2026 16:10:36 +0200 Subject: [PATCH 6/8] INHERIT: a written section, even empty, replaces the parent's Review of PR #2767 (g5t): for DEFINE COMPONENT X INHERIT Y the generator decided whether X provides a section by its number of lines, so an explicitly written empty block (%{ %}) counted as "not written" and Y's section was used, while the same block with a blank line inside overrode it. - Every way of writing a section (own %{..%}, INHERIT Z, EXTEND %{..%}) now marks it present (codeblock_present: linenum > 0), and the INHERIT merge tests that. Same rule as mccode-antlr (mccode-antlr#321, #325). Within a section, parts are still concatenated in order; EXTEND never pulls in the parent implicitly. - USERVARS now has the same form as the other sections and mccode-antlr: INHERIT allowed, repeated USERVARS keyword removed (unused). - Merged blocks keep their %{ line/file, so SIG_MESSAGE debug strings now show the real location; otherwise all 450 examples generate identical code. - New grammar test Unittest_INHERIT_OVERRIDE (McStas and McXtrace) and ADR update. Co-Authored-By: Claude Opus 5.5 --- .../ADR-records/ADR_20261007_GRAMMAR_FIXES.md | 35 ++++++- mccode/src/instrument.y | 92 +++++++++++++------ .../Unittest_INHERIT_OVERRIDE.instr | 78 ++++++++++++++++ .../Unittest_IO_cancel.comp | 32 +++++++ .../Unittest_IO_extend.comp | 35 +++++++ .../Unittest_IO_keep.comp | 26 ++++++ .../Unittest_IO_parent.comp | 38 ++++++++ .../Unittest_INHERIT_OVERRIDE.instr | 78 ++++++++++++++++ .../Unittest_IO_cancel.comp | 32 +++++++ .../Unittest_IO_extend.comp | 35 +++++++ .../Unittest_IO_keep.comp | 26 ++++++ .../Unittest_IO_parent.comp | 38 ++++++++ 12 files changed, 512 insertions(+), 33 deletions(-) create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_cancel.comp create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_extend.comp create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_keep.comp create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_cancel.comp create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_extend.comp create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_keep.comp create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp diff --git a/docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md b/docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md index ab4cb6a82..3f6cc2053 100644 --- a/docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +++ b/docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md @@ -26,10 +26,20 @@ it read about 16x the intensity measured behind the equivalent Slit. Fix the following in the generator. No new keywords are introduced. -1. **Whole-component `INHERIT`**: a code section that `X` leaves empty is - taken from `Y`. Parameter lists keep being concatenated (unchanged). - `SHARE` is emitted once per code block, so `X` and `Y` in the same - instrument do not duplicate the shared code. +1. **Whole-component `INHERIT`** (`DEFINE COMPONENT X INHERIT Y`): a code + section that `X` does not write is taken from `Y`. A section that `X` + writes, even an empty `%{ %}` block, replaces `Y`'s. This is the rule + mccode-antlr implements (mccode-dev/mccode-antlr#321, #325). + Parameter lists keep being concatenated (unchanged). `SHARE` is emitted + once per code block, so `X` and `Y` in the same instrument do not + duplicate the shared code. + Within one section, the parts are concatenated in the order given: + `SECTION [%{..%}] (INHERIT Z | EXTEND %{..%})*`. `EXTEND` never pulls in + `Y`'s code implicitly; write `INHERIT Y` inside the section for that. + `USERVARS` now has the same form as the other sections (`INHERIT` allowed, + the repeated `USERVARS` keyword is gone; no shipped file used it). + Unchanged: `COPY(instance)` never copies `EXTEND` + (mccode-dev/mccode-antlr#330). 2. **`JUMP PREVIOUS[(n)]` / `NEXT[(n)]`** are resolved relative to the jumping component, as documented. A target that does not exist (an unknown name, or out of range) is an error. @@ -124,4 +134,21 @@ Whole-component inheritance: DEFINE COMPONENT Diaphragm INHERIT Slit END /* now has Slit's INITIALIZE/TRACE/DISPLAY; before it had none */ + +DEFINE COMPONENT X INHERIT Y +TRACE +%{ +%} +END +/* X has all of Y's sections except TRACE, which is empty (before: an + empty block counted as not written, so Y's TRACE was used) */ + +DEFINE COMPONENT X INHERIT Y +INITIALIZE INHERIT Y EXTEND +%{ + more(); +%} +END +/* Y's INITIALIZE followed by more(); all other sections from Y. + INITIALIZE EXTEND %{ more(); %} alone would give just more(). */ ``` diff --git a/mccode/src/instrument.y b/mccode/src/instrument.y index b05dcd2d2..8d5614452 100644 --- a/mccode/src/instrument.y +++ b/mccode/src/instrument.y @@ -97,6 +97,7 @@ int metadata_construct_table(instr_ptr_t); void metadata_assign_from_definition(List metadata); void metadata_assign_from_instance(List metadata); static void dependency_add(char *s); +static struct code_block *codeblock_present(struct code_block *cb, struct code_block *own); %} @@ -288,7 +289,9 @@ compdef: "DEFINE" "COMPONENT" TOK_ID parameters metadata shell dependency noa /* inherit from another comp, and initiate it with given blocks */ /* all redefined blocks override */ /* Parameter lists are parent + child, concatenated. A code section - the child leaves empty is taken from the parent. */ + the child does not write is taken from the parent; a section the + child writes, even an empty one, replaces the parent's (as in + mccode-antlr). codeblock_present() marks written sections. */ struct comp_def *def; def = read_component($5); if (def) { @@ -311,14 +314,14 @@ compdef: "DEFINE" "COMPONENT" TOK_ID parameters metadata shell dependency noa c->flag_noacc = $10; - c->share_code = (list_len($11->lines) ? $11 : def->share_code); - c->uservar_code = (list_len($12->lines) ? $12 : def->uservar_code); - c->decl_code = (list_len($13->lines) ? $13 : def->decl_code); - c->init_code = (list_len($14->lines) ? $14 : def->init_code); - c->trace_code = (list_len($15->lines) ? $15 : def->trace_code); - c->save_code = (list_len($16->lines) ? $16 : def->save_code); - c->finally_code = (list_len($17->lines) ? $17 : def->finally_code); - c->display_code = (list_len($18->lines) ? $18 : def->display_code); + c->share_code = ($11->linenum > 0 ? $11 : def->share_code); + c->uservar_code = ($12->linenum > 0 ? $12 : def->uservar_code); + c->decl_code = ($13->linenum > 0 ? $13 : def->decl_code); + c->init_code = ($14->linenum > 0 ? $14 : def->init_code); + c->trace_code = ($15->linenum > 0 ? $15 : def->trace_code); + c->save_code = ($16->linenum > 0 ? $16 : def->save_code); + c->finally_code = ($17->linenum > 0 ? $17 : def->finally_code); + c->display_code = ($18->linenum > 0 ? $18 : def->display_code); /* Check definition and setting params for uniqueness */ check_comp_formals(c->def_par, c->set_par, c->name); @@ -337,9 +340,12 @@ compdef: "DEFINE" "COMPONENT" TOK_ID parameters metadata shell dependency noa - INHERIT appends that component's same section, - EXTEND %{...%} appends one more code block, - the result is one merged struct code_block, in source order. - Merged blocks are created with codeblock_new(), so they have no filename - and linenum -1 (only an INHERIT copies those from its parent). The - *_inherit_extend rules are right-recursive: $3 is "the rest of the chain". */ + - there is no implicit parent: EXTEND only appends to what the section + lists itself; use INHERIT to include another component's code. + A section that is written at all, even as an empty %{ %} block, is marked + present (linenum > 0, see codeblock_present); an absent section keeps + linenum -1. DEFINE COMPONENT X INHERIT Y relies on that distinction. + The *_inherit_extend rules are right-recursive: $3 is "the rest of the chain". */ /* SHARE component block included once. */ comp_share: /* empty */ @@ -352,11 +358,11 @@ comp_share: /* empty */ cb = codeblock_new(); list_cat(cb->lines, $2->lines); list_cat(cb->lines, $3->lines); - $$ = cb; + $$ = codeblock_present(cb, $2); } | "SHARE" comp_share_inherit_extend { - $$ = $2; + $$ = codeblock_present($2, NULL); } ; @@ -400,11 +406,11 @@ comp_trace: /* empty */ cb = codeblock_new(); list_cat(cb->lines, $2->lines); list_cat(cb->lines, $3->lines); - $$ = cb; + $$ = codeblock_present(cb, $2); } | "TRACE" comp_trace_inherit_extend { - $$ = $2; + $$ = codeblock_present($2, NULL); } ; @@ -656,11 +662,11 @@ comp_declare: /* empty */ cb = codeblock_new(); list_cat(cb->lines, $2->lines); list_cat(cb->lines, $3->lines); - $$ = cb; + $$ = codeblock_present(cb, $2); } | "DECLARE" comp_decl_inherit_extend { - $$ = $2; + $$ = codeblock_present($2, NULL); } ; @@ -704,7 +710,11 @@ comp_uservars: /* empty */ cb = codeblock_new(); list_cat(cb->lines, $2->lines); list_cat(cb->lines, $3->lines); - $$ = cb; + $$ = codeblock_present(cb, $2); + } + | "USERVARS" comp_uservars_inherit_extend + { + $$ = codeblock_present($2, NULL); } ; @@ -712,11 +722,19 @@ comp_uservars_inherit_extend: /* empty */ { $$ = codeblock_new(); } - | "USERVARS" codeblock comp_uservars_inherit_extend + | "INHERIT" TOK_ID comp_uservars_inherit_extend { struct code_block *cb; + struct comp_def *def; cb = codeblock_new(); - list_cat(cb->lines, $2->lines); + def = read_component($2); + if (def) { + struct code_block *cb1 = def->uservar_code; + cb->filename = cb1->filename; + cb->quoted_filename = cb1->quoted_filename; + cb->linenum = cb1->linenum; + list_cat(cb->lines, cb1->lines); + } list_cat(cb->lines, $3->lines); $$ = cb; } @@ -740,11 +758,11 @@ comp_initialize: /* empty */ cb = codeblock_new(); list_cat(cb->lines, $2->lines); list_cat(cb->lines, $3->lines); - $$ = cb; + $$ = codeblock_present(cb, $2); } | "INITIALISE" comp_init_inherit_extend { - $$ = $2; + $$ = codeblock_present($2, NULL); } ; @@ -784,7 +802,7 @@ comp_save: /* empty */ } | "SAVE" comp_save_inherit_extend { - $$ = $2; + $$ = codeblock_present($2, NULL); } | "SAVE" codeblock comp_save_inherit_extend { @@ -792,7 +810,7 @@ comp_save: /* empty */ cb = codeblock_new(); list_cat(cb->lines, $2->lines); list_cat(cb->lines, $3->lines); - $$ = cb; + $$ = codeblock_present(cb, $2); } ; @@ -832,7 +850,7 @@ comp_finally: /* empty */ } | "FINALLY" comp_finally_inherit_extend { - $$ = $2; + $$ = codeblock_present($2, NULL); } | "FINALLY" codeblock comp_finally_inherit_extend { @@ -840,7 +858,7 @@ comp_finally: /* empty */ cb = codeblock_new(); list_cat(cb->lines, $2->lines); list_cat(cb->lines, $3->lines); - $$ = cb; + $$ = codeblock_present(cb, $2); } ; @@ -880,7 +898,7 @@ comp_display: /* empty */ } | "DISPLAY" comp_display_inherit_extend { - $$ = $2; + $$ = codeblock_present($2, NULL); } | "DISPLAY" codeblock comp_display_inherit_extend { @@ -888,7 +906,7 @@ comp_display: /* empty */ cb = codeblock_new(); list_cat(cb->lines, $2->lines); list_cat(cb->lines, $3->lines); - $$ = cb; + $$ = codeblock_present(cb, $2); } ; @@ -2216,6 +2234,22 @@ Symtab read_components = NULL; /* name of executable, e.g. mcstas or mcxtrace */ char *executable_name=NULL; +/* Mark a component code section as written (present), even when empty. + Its line number comes from the section's own %{ block when there is one, + else from the current line. Absent sections keep linenum -1. */ +static struct code_block * +codeblock_present(struct code_block *cb, struct code_block *own) +{ + if (own) { + cb->linenum = own->linenum; + cb->filename = own->filename; + cb->quoted_filename = own->quoted_filename; + } + if (cb->linenum <= 0) + cb->linenum = instr_current_line > 0 ? instr_current_line : 1; + return cb; +} + /* Append s to the instrument CFLAGS without overflowing the fixed buffer. */ static void dependency_add(char *s) diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr new file mode 100644 index 000000000..aeb1bf4ec --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr @@ -0,0 +1,78 @@ +/******************************************************************************* +* Instrument: Unittest_INHERIT_OVERRIDE +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* %INSTRUMENT_SITE: Tests_grammar +* +* DEFINE COMPONENT X INHERIT Y: written sections (even empty) replace Y's +* +* %D +* Unit test for whole-component inheritance, see +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* (same rule as mccode-antlr, mccode-dev/mccode-antlr#321). +* +* Local components: Unittest_IO_parent absorbs particles with x>0. Its +* children, DEFINE COMPONENT ... INHERIT Unittest_IO_parent: +* - Unittest_IO_cancel writes an empty TRACE %{ %}: replaces the parent's +* TRACE, absorbs nothing. Before, an empty block counted as "not written" +* and the parent's TRACE was used. +* - Unittest_IO_keep writes no sections: inherits the parent's TRACE. +* - Unittest_IO_extend writes TRACE INHERIT Unittest_IO_parent EXTEND %{..%}: +* the parent's TRACE followed by more code (absorbs x>0 and y>0). It also +* uses INHERIT within USERVARS. +* +* One unit of intensity is emitted uniformly from a 1x1 m square: +* AfterCancel_I = 1, AfterKeep_I = 0.5, AfterExtend_I = 0.25. +* +* %Example: Detector: AfterCancel_I=1 +* %Example: Detector: AfterKeep_I=0.5 +* %Example: Detector: AfterExtend_I=0.25 +* +* %P +* Pp0: [1] Dummy input parameter used internally +* +* %L +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %E +*******************************************************************************/ +DEFINE INSTRUMENT Unittest_INHERIT_OVERRIDE(Pp0=0) + +INITIALIZE +%{ + Pp0=1.0/mcget_ncount(); +%} + +TRACE + +COMPONENT Src = Arm() + AT (0,0,0) ABSOLUTE +EXTEND %{ + x = rand01()-0.5; + y = rand01()-0.5; + p = INSTRUMENT_GETPAR(Pp0); + vz=1000; +%} + +COMPONENT Canceller = Unittest_IO_cancel() + AT (0,0,0.1) ABSOLUTE + +COMPONENT AfterCancel = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterCancel", nx=10, ny=10) + AT (0,0,0.2) ABSOLUTE + +COMPONENT Keeper = Unittest_IO_keep() + AT (0,0,0.3) ABSOLUTE + +COMPONENT AfterKeep = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterKeep", nx=10, ny=10) + AT (0,0,0.4) ABSOLUTE + +COMPONENT Extender = Unittest_IO_extend() + AT (0,0,0.5) ABSOLUTE + +COMPONENT AfterExtend = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterExtend", nx=10, ny=10) + AT (0,0,0.6) ABSOLUTE + +END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_cancel.comp b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_cancel.comp new file mode 100644 index 000000000..2f7b89389 --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_cancel.comp @@ -0,0 +1,32 @@ +/******************************************************************************* +* +* McStas, neutron ray-tracing package +* Copyright (C) 1997-2026, All rights reserved +* DTU Physics, Kgs. Lyngby, Denmark +* +* Component: Unittest_IO_cancel +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* +* Grammar unit test helper for Unittest_INHERIT_OVERRIDE +* +* %D +* Inherits everything from Unittest_IO_parent but cancels its TRACE with an +* explicitly written, empty TRACE section: absorbs nothing. +* See docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %P +* (none) +* +* %E +*******************************************************************************/ +DEFINE COMPONENT Unittest_IO_cancel INHERIT Unittest_IO_parent + +TRACE +%{ +%} + +END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_extend.comp b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_extend.comp new file mode 100644 index 000000000..6ef950deb --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_extend.comp @@ -0,0 +1,35 @@ +/******************************************************************************* +* +* McStas, neutron ray-tracing package +* Copyright (C) 1997-2026, All rights reserved +* DTU Physics, Kgs. Lyngby, Denmark +* +* Component: Unittest_IO_extend +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* +* Grammar unit test helper for Unittest_INHERIT_OVERRIDE +* +* %D +* Writes TRACE as the parent's TRACE plus more code: absorbs x>0 and y>0. +* Also uses INHERIT within USERVARS. +* See docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %P +* (none) +* +* %E +*******************************************************************************/ +DEFINE COMPONENT Unittest_IO_extend INHERIT Unittest_IO_parent + +USERVARS INHERIT Unittest_IO_parent + +TRACE INHERIT Unittest_IO_parent EXTEND +%{ + if (y > 0) ABSORB; +%} + +END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_keep.comp b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_keep.comp new file mode 100644 index 000000000..77029f100 --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_keep.comp @@ -0,0 +1,26 @@ +/******************************************************************************* +* +* McStas, neutron ray-tracing package +* Copyright (C) 1997-2026, All rights reserved +* DTU Physics, Kgs. Lyngby, Denmark +* +* Component: Unittest_IO_keep +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* +* Grammar unit test helper for Unittest_INHERIT_OVERRIDE +* +* %D +* Inherits everything from Unittest_IO_parent (writes no sections): absorbs x>0. +* See docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %P +* (none) +* +* %E +*******************************************************************************/ +DEFINE COMPONENT Unittest_IO_keep INHERIT Unittest_IO_parent +END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp new file mode 100644 index 000000000..943a184e9 --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp @@ -0,0 +1,38 @@ +/******************************************************************************* +* +* McStas, neutron ray-tracing package +* Copyright (C) 1997-2026, All rights reserved +* DTU Physics, Kgs. Lyngby, Denmark +* +* Component: Unittest_IO_parent +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* +* Grammar unit test helper for Unittest_INHERIT_OVERRIDE +* +* %D +* Parent: absorbs particles with x>0, and declares a USERVAR. +* See docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %P +* (none) +* +* %E +*******************************************************************************/ +DEFINE COMPONENT Unittest_IO_parent +SETTING PARAMETERS () + +USERVARS +%{ + double io_tag; +%} + +TRACE +%{ + if (x > 0) ABSORB; +%} + +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr new file mode 100644 index 000000000..67148b492 --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr @@ -0,0 +1,78 @@ +/******************************************************************************* +* Instrument: Unittest_INHERIT_OVERRIDE +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* %INSTRUMENT_SITE: Tests_grammar +* +* DEFINE COMPONENT X INHERIT Y: written sections (even empty) replace Y's +* +* %D +* Unit test for whole-component inheritance, see +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* (same rule as mccode-antlr, mccode-dev/mccode-antlr#321). +* +* Local components: Unittest_IO_parent absorbs particles with x>0. Its +* children, DEFINE COMPONENT ... INHERIT Unittest_IO_parent: +* - Unittest_IO_cancel writes an empty TRACE %{ %}: replaces the parent's +* TRACE, absorbs nothing. Before, an empty block counted as "not written" +* and the parent's TRACE was used. +* - Unittest_IO_keep writes no sections: inherits the parent's TRACE. +* - Unittest_IO_extend writes TRACE INHERIT Unittest_IO_parent EXTEND %{..%}: +* the parent's TRACE followed by more code (absorbs x>0 and y>0). It also +* uses INHERIT within USERVARS. +* +* One unit of intensity is emitted uniformly from a 1x1 m square: +* AfterCancel_I = 1, AfterKeep_I = 0.5, AfterExtend_I = 0.25. +* +* %Example: Detector: AfterCancel_I=1 +* %Example: Detector: AfterKeep_I=0.5 +* %Example: Detector: AfterExtend_I=0.25 +* +* %P +* Pp0: [1] Dummy input parameter used internally +* +* %L +* docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %E +*******************************************************************************/ +DEFINE INSTRUMENT Unittest_INHERIT_OVERRIDE(Pp0=0) + +INITIALIZE +%{ + Pp0=1.0/mcget_ncount(); +%} + +TRACE + +COMPONENT Src = Arm() + AT (0,0,0) ABSOLUTE +EXTEND %{ + x = rand01()-0.5; + y = rand01()-0.5; + p = INSTRUMENT_GETPAR(Pp0); + kz=1; +%} + +COMPONENT Canceller = Unittest_IO_cancel() + AT (0,0,0.1) ABSOLUTE + +COMPONENT AfterCancel = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterCancel", nx=10, ny=10) + AT (0,0,0.2) ABSOLUTE + +COMPONENT Keeper = Unittest_IO_keep() + AT (0,0,0.3) ABSOLUTE + +COMPONENT AfterKeep = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterKeep", nx=10, ny=10) + AT (0,0,0.4) ABSOLUTE + +COMPONENT Extender = Unittest_IO_extend() + AT (0,0,0.5) ABSOLUTE + +COMPONENT AfterExtend = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterExtend", nx=10, ny=10) + AT (0,0,0.6) ABSOLUTE + +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_cancel.comp b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_cancel.comp new file mode 100644 index 000000000..3932c90a3 --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_cancel.comp @@ -0,0 +1,32 @@ +/******************************************************************************* +* +* McXtrace, X-ray tracing package +* Copyright (C) 1997-2026, All rights reserved +* DTU Physics, Kgs. Lyngby, Denmark +* +* Component: Unittest_IO_cancel +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* +* Grammar unit test helper for Unittest_INHERIT_OVERRIDE +* +* %D +* Inherits everything from Unittest_IO_parent but cancels its TRACE with an +* explicitly written, empty TRACE section: absorbs nothing. +* See docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %P +* (none) +* +* %E +*******************************************************************************/ +DEFINE COMPONENT Unittest_IO_cancel INHERIT Unittest_IO_parent + +TRACE +%{ +%} + +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_extend.comp b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_extend.comp new file mode 100644 index 000000000..ddfeb8a7c --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_extend.comp @@ -0,0 +1,35 @@ +/******************************************************************************* +* +* McXtrace, X-ray tracing package +* Copyright (C) 1997-2026, All rights reserved +* DTU Physics, Kgs. Lyngby, Denmark +* +* Component: Unittest_IO_extend +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* +* Grammar unit test helper for Unittest_INHERIT_OVERRIDE +* +* %D +* Writes TRACE as the parent's TRACE plus more code: absorbs x>0 and y>0. +* Also uses INHERIT within USERVARS. +* See docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %P +* (none) +* +* %E +*******************************************************************************/ +DEFINE COMPONENT Unittest_IO_extend INHERIT Unittest_IO_parent + +USERVARS INHERIT Unittest_IO_parent + +TRACE INHERIT Unittest_IO_parent EXTEND +%{ + if (y > 0) ABSORB; +%} + +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_keep.comp b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_keep.comp new file mode 100644 index 000000000..fbac01edf --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_keep.comp @@ -0,0 +1,26 @@ +/******************************************************************************* +* +* McXtrace, X-ray tracing package +* Copyright (C) 1997-2026, All rights reserved +* DTU Physics, Kgs. Lyngby, Denmark +* +* Component: Unittest_IO_keep +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* +* Grammar unit test helper for Unittest_INHERIT_OVERRIDE +* +* %D +* Inherits everything from Unittest_IO_parent (writes no sections): absorbs x>0. +* See docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %P +* (none) +* +* %E +*******************************************************************************/ +DEFINE COMPONENT Unittest_IO_keep INHERIT Unittest_IO_parent +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp new file mode 100644 index 000000000..58ab40b40 --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp @@ -0,0 +1,38 @@ +/******************************************************************************* +* +* McXtrace, X-ray tracing package +* Copyright (C) 1997-2026, All rights reserved +* DTU Physics, Kgs. Lyngby, Denmark +* +* Component: Unittest_IO_parent +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* +* Grammar unit test helper for Unittest_INHERIT_OVERRIDE +* +* %D +* Parent: absorbs particles with x>0, and declares a USERVAR. +* See docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %P +* (none) +* +* %E +*******************************************************************************/ +DEFINE COMPONENT Unittest_IO_parent +SETTING PARAMETERS () + +USERVARS +%{ + double io_tag; +%} + +TRACE +%{ + if (x > 0) ABSORB; +%} + +END From 0eb71166cd6c8e7cc93f53ca08453b632886fde5 Mon Sep 17 00:00:00 2001 From: Peter Willendrup Date: Wed, 7 Oct 2026 16:22:22 +0200 Subject: [PATCH 7/8] Unittest_INHERIT_OVERRIDE: inherit everything but INITIALIZE New child Unittest_IO_noinit writes only an empty INITIALIZE EXTEND %{ %}: it keeps the parent's DECLARE and TRACE but not its INITIALIZE (review of #2767). AfterNoInit_I = 0.125 (before the fix: 0.25, the parent's INITIALIZE was used). McStas and McXtrace. Co-Authored-By: Claude Opus 5.5 --- .../Unittest_INHERIT_OVERRIDE.instr | 13 +++++++- .../Unittest_IO_noinit.comp | 32 +++++++++++++++++++ .../Unittest_IO_parent.comp | 14 +++++++- .../Unittest_INHERIT_OVERRIDE.instr | 13 +++++++- .../Unittest_IO_noinit.comp | 32 +++++++++++++++++++ .../Unittest_IO_parent.comp | 14 +++++++- 6 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_noinit.comp create mode 100644 mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_noinit.comp diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr index aeb1bf4ec..2a94b7b83 100644 --- a/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr +++ b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr @@ -23,13 +23,18 @@ * - Unittest_IO_extend writes TRACE INHERIT Unittest_IO_parent EXTEND %{..%}: * the parent's TRACE followed by more code (absorbs x>0 and y>0). It also * uses INHERIT within USERVARS. +* - Unittest_IO_noinit writes an empty INITIALIZE EXTEND %{ %}: keeps the +* parent's DECLARE and TRACE but not its INITIALIZE (io_val stays 0, so +* its TRACE halves the weight). "Inherit everything but one section". * * One unit of intensity is emitted uniformly from a 1x1 m square: -* AfterCancel_I = 1, AfterKeep_I = 0.5, AfterExtend_I = 0.25. +* AfterCancel_I = 1, AfterKeep_I = 0.5, AfterExtend_I = 0.25, +* AfterNoInit_I = 0.125. * * %Example: Detector: AfterCancel_I=1 * %Example: Detector: AfterKeep_I=0.5 * %Example: Detector: AfterExtend_I=0.25 +* %Example: Detector: AfterNoInit_I=0.125 * * %P * Pp0: [1] Dummy input parameter used internally @@ -75,4 +80,10 @@ COMPONENT Extender = Unittest_IO_extend() COMPONENT AfterExtend = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterExtend", nx=10, ny=10) AT (0,0,0.6) ABSOLUTE +COMPONENT NoIniter = Unittest_IO_noinit() + AT (0,0,0.7) ABSOLUTE + +COMPONENT AfterNoInit = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterNoInit", nx=10, ny=10) + AT (0,0,0.8) ABSOLUTE + END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_noinit.comp b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_noinit.comp new file mode 100644 index 000000000..15bcc1f0e --- /dev/null +++ b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_noinit.comp @@ -0,0 +1,32 @@ +/******************************************************************************* +* +* McStas, neutron ray-tracing package +* Copyright (C) 1997-2026, All rights reserved +* DTU Physics, Kgs. Lyngby, Denmark +* +* Component: Unittest_IO_noinit +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* +* Grammar unit test helper for Unittest_INHERIT_OVERRIDE +* +* %D +* Inherits everything from Unittest_IO_parent but cancels its INITIALIZE with +* an empty INITIALIZE EXTEND section: io_val stays 0, so TRACE halves the weight. +* See docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %P +* (none) +* +* %E +*******************************************************************************/ +DEFINE COMPONENT Unittest_IO_noinit INHERIT Unittest_IO_parent + +INITIALIZE EXTEND +%{ +%} + +END diff --git a/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp index 943a184e9..45cd164bb 100644 --- a/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp +++ b/mcstas-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp @@ -14,7 +14,8 @@ * Grammar unit test helper for Unittest_INHERIT_OVERRIDE * * %D -* Parent: absorbs particles with x>0, and declares a USERVAR. +* Parent: absorbs particles with x>0, and declares a USERVAR. Its DECLARE/ +* INITIALIZE set io_val=1; TRACE halves the weight if io_val is not set. * See docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md * * %P @@ -30,9 +31,20 @@ USERVARS double io_tag; %} +DECLARE +%{ + double io_val; +%} + +INITIALIZE +%{ + io_val = 1; +%} + TRACE %{ if (x > 0) ABSORB; + if (!io_val) p *= 0.5; %} END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr index 67148b492..13db3e5af 100644 --- a/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_INHERIT_OVERRIDE.instr @@ -23,13 +23,18 @@ * - Unittest_IO_extend writes TRACE INHERIT Unittest_IO_parent EXTEND %{..%}: * the parent's TRACE followed by more code (absorbs x>0 and y>0). It also * uses INHERIT within USERVARS. +* - Unittest_IO_noinit writes an empty INITIALIZE EXTEND %{ %}: keeps the +* parent's DECLARE and TRACE but not its INITIALIZE (io_val stays 0, so +* its TRACE halves the weight). "Inherit everything but one section". * * One unit of intensity is emitted uniformly from a 1x1 m square: -* AfterCancel_I = 1, AfterKeep_I = 0.5, AfterExtend_I = 0.25. +* AfterCancel_I = 1, AfterKeep_I = 0.5, AfterExtend_I = 0.25, +* AfterNoInit_I = 0.125. * * %Example: Detector: AfterCancel_I=1 * %Example: Detector: AfterKeep_I=0.5 * %Example: Detector: AfterExtend_I=0.25 +* %Example: Detector: AfterNoInit_I=0.125 * * %P * Pp0: [1] Dummy input parameter used internally @@ -75,4 +80,10 @@ COMPONENT Extender = Unittest_IO_extend() COMPONENT AfterExtend = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterExtend", nx=10, ny=10) AT (0,0,0.6) ABSOLUTE +COMPONENT NoIniter = Unittest_IO_noinit() + AT (0,0,0.7) ABSOLUTE + +COMPONENT AfterNoInit = PSD_monitor(xwidth=2.0, yheight=2.0, filename="AfterNoInit", nx=10, ny=10) + AT (0,0,0.8) ABSOLUTE + END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_noinit.comp b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_noinit.comp new file mode 100644 index 000000000..caa0b8881 --- /dev/null +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_noinit.comp @@ -0,0 +1,32 @@ +/******************************************************************************* +* +* McXtrace, X-ray tracing package +* Copyright (C) 1997-2026, All rights reserved +* DTU Physics, Kgs. Lyngby, Denmark +* +* Component: Unittest_IO_noinit +* +* %I +* Written by: Peter Willendrup +* Date: Oct 7th, 2026 +* Origin: DTU +* +* Grammar unit test helper for Unittest_INHERIT_OVERRIDE +* +* %D +* Inherits everything from Unittest_IO_parent but cancels its INITIALIZE with +* an empty INITIALIZE EXTEND section: io_val stays 0, so TRACE halves the weight. +* See docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +* +* %P +* (none) +* +* %E +*******************************************************************************/ +DEFINE COMPONENT Unittest_IO_noinit INHERIT Unittest_IO_parent + +INITIALIZE EXTEND +%{ +%} + +END diff --git a/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp index 58ab40b40..86b05ba30 100644 --- a/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp +++ b/mcxtrace-comps/examples/Tests_grammar/Unittest_INHERIT_OVERRIDE/Unittest_IO_parent.comp @@ -14,7 +14,8 @@ * Grammar unit test helper for Unittest_INHERIT_OVERRIDE * * %D -* Parent: absorbs particles with x>0, and declares a USERVAR. +* Parent: absorbs particles with x>0, and declares a USERVAR. Its DECLARE/ +* INITIALIZE set io_val=1; TRACE halves the weight if io_val is not set. * See docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md * * %P @@ -30,9 +31,20 @@ USERVARS double io_tag; %} +DECLARE +%{ + double io_val; +%} + +INITIALIZE +%{ + io_val = 1; +%} + TRACE %{ if (x > 0) ABSORB; + if (!io_val) p *= 0.5; %} END From bb70a9b40f4458542a2eceded0cfa2ad9a1f96a5 Mon Sep 17 00:00:00 2001 From: Peter Willendrup Date: Thu, 8 Oct 2026 13:49:31 +0200 Subject: [PATCH 8/8] cogen: numeric USERVARS by whole words, as mccode-antlr Review of #2767 (g5t): uservar_is_numeric() used strstr, so type names containing "int", "char", ... (Point, struct interaction, charge_t) counted as numeric and the generated (double) cast did not compile, while unsigned/signed alone, size_t, bool/_Bool and types were reported as not readable. - A USERVAR is numeric if it is not an array or pointer and every word of its type is one of char short int long float double signed unsigned _Bool bool const volatile MCNUM size_t, or matches u?int(_least|_fast)?N_t (same rule as is_numeric_scalar in mccode-antlr). Other typedefs are not recognised (ADR updated). - particle_uservar_init() uses the same rule. It had the same substring test, and wrote p->arr[N]=0 (one past the end) for array USERVARS. char and long long USERVARS are now zeroed at start; they were uninitialised before. All 452 examples generate identical code except 10 that now also zero char/long long USERVARS; those 10 give identical detector results. Co-Authored-By: Claude Opus 5.5 --- .../ADR-records/ADR_20261007_GRAMMAR_FIXES.md | 12 +++-- mccode/src/cogen.c.in | 53 ++++++++++++++----- 2 files changed, 50 insertions(+), 15 deletions(-) diff --git a/docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md b/docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md index 3f6cc2053..9276cb5f7 100644 --- a/docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md +++ b/docs/GRAMMAR/ADR-records/ADR_20261007_GRAMMAR_FIXES.md @@ -57,9 +57,15 @@ Fix the following in the generator. No new keywords are introduced. on one line (`double a; double b;`). 8. **`USERVARS` read through `particle_getvar()`** (e.g. Monitor_nD `user1="var"`): numeric scalars are converted to `double` instead of - having their memory read as a `double`. Arrays, pointers and structs - report failure. USERVARS used with Monitor_nD should still be declared - `double`. + having their memory read as a `double`. A USERVAR counts as numeric when + it is not an array or pointer and every word of its type is one of + `char short int long float double signed unsigned _Bool bool const + volatile MCNUM size_t` or a `` integer name + (`u?int(_least|_fast)?N_t`), the same rule as mccode-antlr's + `is_numeric_scalar`. Others (arrays, pointers, structs, and typedefs + such as `typedef double real;`) are not readable this way and report + failure. The same rule decides which USERVARS `particle_uservar_init()` + zeroes. USERVARS used with Monitor_nD should still be declared `double`. 9. **Vector parameters given as `{...}`** are split at top-level commas, and each element is written into the generated C as the expression it is. Before, the elements were parsed as plain numbers (`strtod`), so diff --git a/mccode/src/cogen.c.in b/mccode/src/cogen.c.in index 0581fdb51..a322c3105 100644 --- a/mccode/src/cogen.c.in +++ b/mccode/src/cogen.c.in @@ -448,13 +448,44 @@ int var_in_list(List lst, struct comp_iformal* var) { return retval; } -/* USERVARS that particle_getvar() can return as a double: numeric scalars */ +/* One word of a USERVAR type: a C arithmetic keyword, MCNUM, size_t or a + integer name u?int(_least|_fast)?N_t. */ +static int uservar_numeric_word(const char *w, size_t n) +{ + static const char *words[] = { "char", "short", "int", "long", "float", "double", + "signed", "unsigned", "_Bool", "bool", "const", "volatile", "MCNUM", "size_t", NULL }; + int i; + size_t d = 0; + for (i = 0; words[i]; i++) + if (strlen(words[i]) == n && !strncmp(w, words[i], n)) return 1; + if (n && *w == 'u') { w++; n--; } + if (n < 3 || strncmp(w, "int", 3)) return 0; + w += 3; n -= 3; + if (n >= 6 && !strncmp(w, "_least", 6)) { w += 6; n -= 6; } + else if (n >= 5 && !strncmp(w, "_fast", 5)) { w += 5; n -= 5; } + while (d < n && w[d] >= '0' && w[d] <= '9') d++; + return d > 0 && n == d + 2 && w[d] == '_' && w[d+1] == 't'; +} + +/* USERVARS that particle_getvar() can return as a double: numeric scalars, + i.e. not an array or pointer, and every word of the type is numeric (as + is_numeric_scalar in mccode-antlr). Other typedefs, e.g. "typedef double + real;", are not recognised. */ static int uservar_is_numeric(char *tpe, char *var) { + int words = 0; + char *s = tpe; if (strpbrk(tpe, "[]*") || strchr(var, '[')) return 0; - return strstr(tpe, "double") || strstr(tpe, "float") || strstr(tpe, "int") - || strstr(tpe, "long") || strstr(tpe, "short") || strstr(tpe, "char") - || strstr(tpe, "MCNUM"); + while (*s) { + size_t n = strcspn(s, " \t\n"); + if (n) { + if (!uservar_numeric_word(s, n)) return 0; + words++; + } + s += n; + s += strspn(s, " \t\n"); + } + return words > 0; } /* ***************************************************************************** @@ -2864,14 +2895,12 @@ cogen_header(struct instr_def *instr, char *output_name) cout("void particle_uservar_init(_class_particle *p){"); while((var = list_next(liter))) { tpe = list_next(liter2); - if (strstr(tpe, "double") || strstr(tpe, "MCNUM") || strstr(tpe, "int")){ - if (!strstr(tpe, "[") && !strstr(tpe, "]") && !strstr(tpe, "*")) { - coutf(" p->%s=0;",var); - } else { - printf("\nWARNING:\n --> USERVAR %s is of type %s (array/pointer?)\n", var,tpe); - printf(" --> and may need specific per-particle\n"); - printf(" --> initialisation through an EXTEND block!\n"); - } + if (uservar_is_numeric(tpe, var)) { + coutf(" p->%s=0;",var); + } else if (strpbrk(tpe, "[]*") || strchr(var, '[')) { + printf("\nWARNING:\n --> USERVAR %s is of type %s (array/pointer?)\n", var,tpe); + printf(" --> and may need specific per-particle\n"); + printf(" --> initialisation through an EXTEND block!\n"); } else { printf("\nWARNING:\n --> USERVAR %s is of type %s and may need specific\n", var,tpe); printf(" --> per-particle initialisation through an\n");