From ea6413a9ced368f594634d7ad3e9829c8924b3a8 Mon Sep 17 00:00:00 2001 From: tzuohann <2057796+tzuohann@users.noreply.github.com> Date: Thu, 30 Jul 2026 14:33:39 -0400 Subject: [PATCH] halcompile: warn about, and reject colliding, mangled HAL names A name declared in a .comp file is a C identifier, but it is exported under a mangled HAL identifier: underscores become dashes and a trailing dash or period is removed (comp.adoc, HALNAME). Nothing said so at compile time, so "pin in float my_input" silently became component.N.my-input. Worse, check_name_ok() compares only declared names. Two declarations that mangle to the same HAL name -- x_y and x_y_, the two rows of the HALNAME table that share a HAL identifier -- therefore compiled cleanly and failed much later, at load time: HAL: ERROR: duplicate variable 'collide.0.x-y' collide: rtapi_app_main: Invalid argument (-22) Add check_hal_name(), which rejects that collision at the offending line, and a once-per-file warning listing the names whose HAL identifier differs from the declaration. Both messages point at the HALNAME documentation. The warning is suppressed by -N (--no-name-warnings), which the in-tree component rules pass, since those names are deliberate. All 133 in-tree .comp files preprocess with no new error, and silently under -N. tests/halcompile/halname covers the warning, -N, and the rejected collision. Co-Authored-By: Claude Opus 5 (1M context) --- docs/src/hal/comp.adoc | 12 +++++ docs/src/man/man1/halcompile.1.adoc | 19 +++++++ src/hal/components/Submakefile | 8 +-- src/hal/utils/halcompile.g | 53 +++++++++++++++++-- tests/halcompile/halname/.gitignore | 1 + tests/halcompile/halname/expected | 4 ++ .../halcompile/halname/halname_collision.comp | 7 +++ tests/halcompile/halname/halname_mangled.comp | 6 +++ tests/halcompile/halname/test.sh | 20 +++++++ 9 files changed, 124 insertions(+), 6 deletions(-) create mode 100644 tests/halcompile/halname/.gitignore create mode 100644 tests/halcompile/halname/expected create mode 100644 tests/halcompile/halname/halname_collision.comp create mode 100644 tests/halcompile/halname/halname_mangled.comp create mode 100755 tests/halcompile/halname/test.sh diff --git a/docs/src/hal/comp.adoc b/docs/src/hal/comp.adoc index d382a70f475..444c8e1b935 100644 --- a/docs/src/hal/comp.adoc +++ b/docs/src/hal/comp.adoc @@ -227,6 +227,18 @@ A trailing "_" is retained, so that HAL identifiers which would otherwise collid |x.## | x(MM) | x.MM |=== +[NOTE] +==== +The HAL identifier, not the declared HALNAME, is what HAL files, `halcmd` and +`halshow` see. `halcompile` prints one warning per file listing the names that +differ, suppressed by the *-N* (*--no-name-warnings*) option. + +Two declarations that produce the same HAL identifier -- 'x_y_z' and 'x_y_z_' +in the table above -- are rejected by `halcompile`, because they would +otherwise compile and then fail at `loadrt` with "HAL: ERROR: duplicate +variable". +==== + * 'if CONDITION' - An expression involving the variable 'personality' which is nonzero when the pin or parameter should be created. * 'SIZE' - A number that gives the size of an array. The array items are numbered from 0 to 'SIZE'-1. diff --git a/docs/src/man/man1/halcompile.1.adoc b/docs/src/man/man1/halcompile.1.adoc index c06cd1b8688..a48759dba27 100644 --- a/docs/src/man/man1/halcompile.1.adoc +++ b/docs/src/man/man1/halcompile.1.adoc @@ -57,6 +57,9 @@ this option and it has no effect when only asciidoc formatted documentation is requested using the *-a* or *--adoc* option. *-l*, *--require-license*:: Obsolete. The component is always required to have a *licence* tag. +*-N*, *--no-name-warnings*:: +Do not warn about declared names that are exported under a different HAL name. +See *NAMES* below. *-o* _file_, *--outfile*=_file_:: Write output to _file_. Can _only_ be used with *--preprocess*, *--adoc* and *--document* processing. @@ -97,6 +100,22 @@ Extra arguments passed to the linker. require _sudo_ to write to system directories. * Preprocess *.comp* files into *.c* files (the *--preprocess* flag) +== NAMES + +A name declared in a *.comp* file is a C identifier, but it is exported under a +mangled HAL identifier: underscores become dashes, and a trailing dash or +period is removed. A pin declared *pin in float my_input* is therefore reached +from HAL as *component.N.my-input*, not *component.N.my_input*, and a component +loaded with *loadrt my_comp* exports its pins under *my-comp.N.*. + +*halcompile* prints one warning per file listing the names this applies to; +pass *-N* to suppress it. Two declarations that mangle to the same HAL name +(for example *x_y* and *x_y_*) are rejected, since they would otherwise be +accepted here and fail later at *loadrt* with "HAL: ERROR: duplicate variable". + +See HALNAME under _Syntax_ in the _Halcompile HAL Component Generator_ +documentation for the full mangling rules. + == SEE ALSO * _Halcompile_ / _HAL Component Generator_ in the LinuxCNC documentation for a diff --git a/src/hal/components/Submakefile b/src/hal/components/Submakefile index 718ed39d1a6..e14343aec88 100644 --- a/src/hal/components/Submakefile +++ b/src/hal/components/Submakefile @@ -65,12 +65,12 @@ COMP_DRIVER_MANPAGE_ADOCS := $(patsubst hal/drivers/%.comp, objects/man/man9/%.9 $(COMP_MANPAGE_ADOCS): objects/man/man9/%.9.adoc: hal/components/%.comp ../bin/halcompile $(ECHO) Extracting adoc manpage $(notdir $@) @mkdir -p $(dir $@) - $(Q)../bin/halcompile -U --adoc -o $@ $< + $(Q)../bin/halcompile -N -U --adoc -o $@ $< $(COMP_DRIVER_MANPAGE_ADOCS): objects/man/man9/%.9.adoc: hal/drivers/%.comp ../bin/halcompile $(ECHO) Extracting adoc manpage $(notdir $@) @mkdir -p $(dir $@) - $(Q)../bin/halcompile -U --adoc -o $@ $< + $(Q)../bin/halcompile -N -U --adoc -o $@ $< # Build troff from the adoc via asciidoctor. Used to be halcompile # emitting troff directly with sed post-processing to escape .als / .URL @@ -94,10 +94,12 @@ objects/%.mak: %.comp hal/components/Submakefile $(Q)echo ../rtlib/$(notdir $*)$(MODULE_EXT): objects/rtobjects/$*.o >> $@.tmp $(Q)mv -f $@.tmp $@ +# -N: the HAL names of in-tree components are deliberate, so skip the +# reminder that declared names are mangled when they are exported. objects/%.c: %.comp ../bin/halcompile $(ECHO) "Preprocessing $(notdir $<)" @mkdir -p $(dir $@) - $(Q)../bin/halcompile -U -o $@ $< + $(Q)../bin/halcompile -N -U -o $@ $< modules: $(patsubst %.comp, objects/%.c, $(COMPS) $(COMP_DRIVERS)) diff --git a/src/hal/utils/halcompile.g b/src/hal/utils/halcompile.g index ab5a38310ed..f281256507e 100644 --- a/src/hal/utils/halcompile.g +++ b/src/hal/utils/halcompile.g @@ -140,6 +140,7 @@ def parse(filename): a, b = f.split("\n;;\n", 1) p = _parse('File', a + "\n\n", filename) if not p: raise SystemExit(1) + warn_mangled_names(filename) if require_license: if not finddoc('license'): raise SystemExit("%s:0: License not specified" % filename) @@ -156,13 +157,19 @@ deprecated = ['s32', 'u32'] def initialize(): global functions, params, pins, comp_name, names, docs, variables - global modparams, includes + global modparams, includes, hal_names, mangled_names functions = []; params = []; pins = []; options = {}; variables = [] modparams = []; docs = []; includes = []; comp_name = None names = {} + hal_names = {} + mangled_names = [] + +# Cleared by -N (--no-name-warnings); the in-tree build sets it for components +# whose HAL names are already known to be correct. +warn_hal_names = True def Warn(msg, *args): if args: @@ -224,10 +231,40 @@ def check_name_ok(name): if name in names: Error("Duplicate item name %s" % name) +HALNAME_DOC = ("see HALNAME under 'Syntax' in the Halcompile HAL Component " + "Generator documentation, " + "https://linuxcnc.org/docs/html/hal/comp.html") + +def check_hal_name(kind, name): + """A declaration is a C identifier, but it is exported under a mangled HAL + identifier. check_name_ok() only compares declared names, so two + declarations that mangle to one HAL name compile cleanly and fail later, at + loadrt, with "HAL: ERROR: duplicate variable".""" + if name == "_": return # the unnamed singleton function + hal_name = to_hal(name) + if (kind, hal_name) in hal_names: + Error("'%s' and '%s' both export the HAL name '%s'; %s" + % (hal_names[(kind, hal_name)], name, hal_name, HALNAME_DOC)) + hal_names[(kind, hal_name)] = name + if hal_name != name: + mangled_names.append((name, hal_name)) + +def warn_mangled_names(filename): + if not mangled_names or not warn_hal_names: return + # undo the printf conversion to_hal() applies to array names + unarray = lambda s: re.sub(r"%0(\d+)d", lambda m: "#" * int(m.group(1)), s) + shown = ", ".join("%s -> %s" % (n, unarray(h)) for n, h in mangled_names[:3]) + if len(mangled_names) > 3: + shown += ", ... (%d more)" % (len(mangled_names) - 3) + print("%s:0: Warning: %d declared name(s) are exported under a different " + "HAL name: %s. Use the HAL name in HAL files, halcmd and halshow; %s" + % (filename, len(mangled_names), shown, HALNAME_DOC), file=sys.stderr) + def pin(name, type_, array, dir_, doc, value, personality): checkarray(name, array) type_ = type2type(type_) check_name_ok(name) + check_hal_name('pin', name) docs.append(('pin', name, type_, array, dir_, doc, value, personality)) names[name] = None pins.append((name, type_, array, dir_, value, personality)) @@ -236,12 +273,14 @@ def param(name, type_, array, dir_, doc, value, personality): checkarray(name, array) type_ = type2type(type_) check_name_ok(name) + check_hal_name('pin', name) # pins and params share a namespace docs.append(('param', name, type_, array, dir_, doc, value, personality)) names[name] = None params.append((name, type_, array, dir_, value, personality)) def function(name, fp, doc): check_name_ok(name) + check_hal_name('function', name) docs.append(('funct', name, fp, doc)) names[name] = None functions.append((name, fp)) @@ -1183,6 +1222,10 @@ Usage: Option to set maximum 'personalities' items: --personalities=integer_value (default is %(dflt)d) +Option to suppress the warning about declared names that are exported under a +different HAL name: + -N, --no-name-warnings + Options to add compile and link flags (only for userspace, only for .c files) --extra-compile-args="-I/usr/include/..." --extra-link-args="-l..." @@ -1199,6 +1242,7 @@ def main(): require_license = True global require_unix_line_endings require_unix_line_endings = False + global warn_hal_names mode = PREPROCESS adoc = False keepadoc = None @@ -1207,8 +1251,9 @@ def main(): global options options = {} try: - opts, args = getopt.getopt(sys.argv[1:], "UluijJcpdak:o:h?P:", - ['unix', 'install', 'compile', 'preprocess', 'outfile=', + opts, args = getopt.getopt(sys.argv[1:], "NUluijJcpdak:o:h?P:", + ['unix', 'no-name-warnings', 'install', 'compile', + 'preprocess', 'outfile=', 'document', 'adoc', 'keep-adoc=', 'help', 'userspace', 'install-doc', 'view-doc', 'require-license', 'print-modinc', 'personalities=', "extra-compile-args=", @@ -1218,6 +1263,8 @@ def main(): for k, v in opts: if k in ("-U", "--unix"): require_unix_line_endings = True + if k in ("-N", "--no-name-warnings"): + warn_hal_names = False if k in ("-u", "--userspace"): userspace = True if k in ("-i", "--install"): diff --git a/tests/halcompile/halname/.gitignore b/tests/halcompile/halname/.gitignore new file mode 100644 index 00000000000..85d6eda5fe4 --- /dev/null +++ b/tests/halcompile/halname/.gitignore @@ -0,0 +1 @@ +halname_mangled.c diff --git a/tests/halcompile/halname/expected b/tests/halcompile/halname/expected new file mode 100644 index 00000000000..c6c993f4c65 --- /dev/null +++ b/tests/halcompile/halname/expected @@ -0,0 +1,4 @@ +halname_mangled.comp:0: Warning: 1 declared name(s) are exported under a different HAL name: my_pin -> my-pin. Use the HAL name in HAL files, halcmd and halshow; see HALNAME under 'Syntax' in the Halcompile HAL Component Generator documentation, https://linuxcnc.org/docs/html/hal/comp.html +halname_collision.comp:4:18: 'x_y' and 'x_y_' both export the HAL name 'x-y'; see HALNAME under 'Syntax' in the Halcompile HAL Component Generator documentation, https://linuxcnc.org/docs/html/hal/comp.html +> pin out bit x_y_; +> ^ diff --git a/tests/halcompile/halname/halname_collision.comp b/tests/halcompile/halname/halname_collision.comp new file mode 100644 index 00000000000..3528b33107b --- /dev/null +++ b/tests/halcompile/halname/halname_collision.comp @@ -0,0 +1,7 @@ +component halname_collision; +license "GPL"; +pin in bit x_y; +pin out bit x_y_; +function _; +;; +FUNCTION(_) {} diff --git a/tests/halcompile/halname/halname_mangled.comp b/tests/halcompile/halname/halname_mangled.comp new file mode 100644 index 00000000000..82422eb87c4 --- /dev/null +++ b/tests/halcompile/halname/halname_mangled.comp @@ -0,0 +1,6 @@ +component halname_mangled; +license "GPL"; +pin in bit my_pin; +function _; +;; +FUNCTION(_) {} diff --git a/tests/halcompile/halname/test.sh b/tests/halcompile/halname/test.sh new file mode 100755 index 00000000000..6e47e9346f1 --- /dev/null +++ b/tests/halcompile/halname/test.sh @@ -0,0 +1,20 @@ +#!/bin/bash +set -e + +# A declared name that is exported under a different HAL name must warn, +# but must still compile. +rm -f halname_mangled.c +halcompile --preprocess halname_mangled.comp 2>&1 +test -f halname_mangled.c || echo 'halcompile failed to produce halname_mangled.c' + +# -N silences that warning. +rm -f halname_mangled.c +halcompile -N --preprocess halname_mangled.comp 2>&1 + +# Two declarations that mangle to one HAL name must be rejected, not left +# to fail at loadrt as "HAL: ERROR: duplicate variable". +rm -f halname_collision.c +if halcompile --preprocess halname_collision.comp 2>&1; then + echo 'halcompile erroneously accepted halname_collision.comp' +fi +test ! -f halname_collision.c || echo 'halcompile erroneously produced halname_collision.c'