halcompile: warn about, and reject colliding, mangled HAL names

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.

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

src/hal/components/Submakefile

@@ -67,12 +67,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
@@ -99,10 +99,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

@@ -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)
@@ -157,13 +158,20 @@ newtypes = ['bool', 'sint', 'uint', 'si32', 'ui32', 'real']
 
 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:
@@ -225,10 +233,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))
@@ -237,12 +277,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))
@@ -1238,6 +1281,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..."
@@ -1254,6 +1301,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
@@ -1262,8 +1310,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=",
@@ -1273,6 +1322,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'