diff --git a/docs/src/hal/comp.adoc b/docs/src/hal/comp.adoc index 0e66c1a16b3..f1b086a2dac 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 147517d59b9..1654712b01e 100644 --- a/src/hal/components/Submakefile +++ b/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)) diff --git a/src/hal/utils/halcompile.g b/src/hal/utils/halcompile.g index e4bae9c1f57..0d6da5c0318 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) @@ -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"): 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'