From f3f45d9ae3447ccd06924a91f0a4d876fc849539 Mon Sep 17 00:00:00 2001 From: tzuohann <2057796+tzuohann@users.noreply.github.com> Date: Thu, 30 Jul 2026 14:42:10 -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 119 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/man/man1/halcompile.1 | 8 +++ docs/src/hal/comp.adoc | 12 ++++ src/hal/components/Submakefile | 8 ++- src/hal/utils/halcompile.g | 57 ++++++++++++++++++- 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, 117 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/man/man1/halcompile.1 b/docs/man/man1/halcompile.1 index b44aa871192..3ea786740bc 100644 --- a/docs/man/man1/halcompile.1 +++ b/docs/man/man1/halcompile.1 @@ -76,6 +76,14 @@ Install \fB.c\fR and \fB.py\fR files into the proper directory for HAL non-realt Extract documentation from \fB.comp\fR files into \fB.9\fR manpage files in the proper system directory (the \fB\-\-install\fR flag), which may require \fIsudo\fR to write to system directories. .IP \(bu 4 Preprocess \fB.comp\fR files into \fB.c\fR files (the \fB\-\-preprocess\fR flag) +.SH NAMES +A name declared in a \fB.comp\fR 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 \fBpin in float my_input\fR is therefore reached from HAL as \fBcomponent.N.my\-input\fR, not \fBcomponent.N.my_input\fR, and a component loaded with \fBloadrt my_comp\fR exports its pins under \fBmy\-comp.N.\fR +.PP +\fBhalcompile\fR prints one warning per file listing the names this applies to; pass \fB\-N\fR (\fB\-\-no\-name\-warnings\fR) to suppress it. +Two declarations that mangle to the same HAL name (for example \fBx_y\fR and \fBx_y_\fR) are rejected, since they would otherwise be accepted here and fail later at \fBloadrt\fR with "HAL: ERROR: duplicate variable". +.PP +See HALNAME under \fISyntax\fR in the \fIHalcompile HAL Component Generator\fR documentation for the full mangling rules. .SH "SEE ALSO" \fIHalcompile HAL Component Generator\fR in the LinuxCNC documentation for a full description of the \fB.comp\fR syntax, along with examples diff --git a/docs/src/hal/comp.adoc b/docs/src/hal/comp.adoc index 6f9f246b711..607833c2ddc 100644 --- a/docs/src/hal/comp.adoc +++ b/docs/src/hal/comp.adoc @@ -229,6 +229,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/src/hal/components/Submakefile b/src/hal/components/Submakefile index 8d156aa9cc4..d69aea16453 100644 --- a/src/hal/components/Submakefile +++ b/src/hal/components/Submakefile @@ -32,13 +32,13 @@ obj-m += $(patsubst hal/drivers/%.comp, %.o, $(patsubst hal/components/%.comp, % $(COMP_MANPAGES): ../docs/man/man9/%.9: hal/components/%.comp ../bin/halcompile $(ECHO) Making halcompile manpage $(notdir $@) @mkdir -p $(dir $@) - $(Q)../bin/halcompile -U --document -o $@.new $< && preconv -r < $@.new > $@ + $(Q)../bin/halcompile -N -U --document -o $@.new $< && preconv -r < $@.new > $@ $(RM) $@.new $(COMP_DRIVER_MANPAGES): ../docs/man/man9/%.9: hal/drivers/%.comp ../bin/halcompile $(ECHO) Making halcompile manpage $(notdir $@) @mkdir -p $(dir $@) - $(Q)../bin/halcompile -U --document -o $@ $< + $(Q)../bin/halcompile -N -U --document -o $@ $< objects/%.mak: %.comp hal/components/Submakefile $(ECHO) "Creating $(notdir $@)" @@ -47,10 +47,12 @@ objects/%.mak: %.comp hal/components/Submakefile $(Q)echo ../rtlib/$(notdir $*)$(MODULE_EXT): objects/rtobjects/$*.o >> $@.tmp $(Q)mv -f $@.tmp $@ +# -N silences the reminder that declared names are mangled when exported; +# in-tree HAL names are deliberate. It does not affect the collision check. 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 ed605845e74..e61f0f2d906 100644 --- a/src/hal/utils/halcompile.g +++ b/src/hal/utils/halcompile.g @@ -139,6 +139,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) @@ -151,13 +152,20 @@ deprecated = ['s32', 'u32'] def initialize(): global functions, params, pins, comp_name, names, docs, variables - global modparams, includes + global modparams, includes, hal_pin_names, hal_funct_names, mangled_names functions = []; params = []; pins = []; options = {}; variables = [] modparams = []; docs = []; includes = []; comp_name = None names = {} + hal_pin_names = {} + hal_funct_names = {} + mangled_names = [] + +# Cleared by -N (--no-name-warnings). This silences warn_mangled_names() only; +# a HAL name collision is always an error. +warn_hal_names = True def Warn(msg, *args): if args: @@ -215,10 +223,42 @@ 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 to_hal_display(name): + # to_hal() without the array-index expansion, so "x.##" stays readable + return name.replace("_", "-").rstrip("-").rstrip(".") + +def check_hal_name(seen, 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 hal_name in seen: + Error("'%s' and '%s' both export the HAL name '%s'; %s" + % (seen[hal_name], name, hal_name, HALNAME_DOC)) + seen[hal_name] = name + if to_hal_display(name) != name: # ignore the array-index expansion + mangled_names.append(name) + +def warn_mangled_names(filename): + if not mangled_names or not warn_hal_names: return + shown = ", ".join("%s -> %s" % (n, to_hal_display(n)) for n 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(hal_pin_names, name) docs.append(('pin', name, type, array, dir, doc, value, personality)) names[name] = None pins.append((name, type, array, dir, value, personality)) @@ -227,12 +267,15 @@ def param(name, type, array, dir, doc, value, personality): checkarray(name, array) type = type2type(type) check_name_ok(name) + check_hal_name(hal_pin_names, name) # hal_lib.c: setp cannot tell a pin + # and a param of one name apart 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(hal_funct_names, name) docs.append(('funct', name, fp, doc)) names[name] = None functions.append((name, fp)) @@ -1124,6 +1167,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..." @@ -1138,14 +1185,16 @@ def main(): require_license = True global require_unix_line_endings require_unix_line_endings = False + global warn_hal_names mode = PREPROCESS outfile = None userspace = False global options options = {} try: - opts, args = getopt.getopt(sys.argv[1:], "Uluijcpdo:h?P:", - ['unix', 'install', 'compile', 'preprocess', 'outfile=', + opts, args = getopt.getopt(sys.argv[1:], "NUluijcpdo:h?P:", + ['unix', 'no-name-warnings', 'install', 'compile', + 'preprocess', 'outfile=', 'document', 'help', 'userspace', 'install-doc', 'view-doc', 'require-license', 'print-modinc', 'personalities=', "extra-compile-args=", @@ -1155,6 +1204,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'