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) <noreply@anthropic.com>

Author: tzuohann

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
 

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.

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))
 

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"):

tests/halcompile/halname/.gitignore

@@ -0,0 +1 @@
+halname_mangled.c

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_;
+>                   ^

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(_) {}

tests/halcompile/halname/halname_mangled.comp

@@ -0,0 +1,6 @@
+component halname_mangled;
+license "GPL";
+pin in bit my_pin;
+function _;
+;;
+FUNCTION(_) {}

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'