From 2daaced3152ef67746d3648f794c6dfddc3a1f63 Mon Sep 17 00:00:00 2001 From: Hamdan Al-Musaibeli <80288863+robotbitdev@users.noreply.github.com> Date: Mon, 17 Aug 2026 13:44:04 -0600 Subject: [PATCH] Validate labels in generated LS output Add label validation for generated LS to catch undefined or duplicate LBL[n] usages. Introduce TPPlus::LabelValidator and TPPlus::LabelValidationError (lib/tp_plus/label_validator.rb) that scan motion sections for direct label references and definitions. Track and preserve label names from inlined scopes in Interpreter (reset/merge/label_names_by_number) and call validation from bin/tpp and Function#to_ls so programs and inline outputs are checked before writing. Add tests covering validator behavior and inline-name preservation. --- bin/tpp | 15 ++- lib/tp_plus.rb | 1 + lib/tp_plus/function.rb | 15 ++- lib/tp_plus/interpreter.rb | 29 ++++++ lib/tp_plus/label_validator.rb | 142 +++++++++++++++++++++++++++ test/tp_plus/test_interpreter.rb | 33 +++++++ test/tp_plus/test_label_validator.rb | 87 ++++++++++++++++ 7 files changed, 319 insertions(+), 3 deletions(-) create mode 100644 lib/tp_plus/label_validator.rb create mode 100644 test/tp_plus/test_label_validator.rb diff --git a/bin/tpp b/bin/tpp index f8eabb9..c11a75a 100755 --- a/bin/tpp +++ b/bin/tpp @@ -242,8 +242,19 @@ end output += %(/END\n) -#write functions to disk -output += interpreter.output_functions(options) +begin + TPPlus::LabelValidator.validate!( + output, + program_name: tpp_filename.upcase, + label_names: interpreter.label_names_by_number + ) + + #write functions to disk + output += interpreter.output_functions(options) +rescue TPPlus::LabelValidationError => error + warn error.message + exit 1 +end if line_count > 0 if options[:output] diff --git a/lib/tp_plus.rb b/lib/tp_plus.rb index d7b3ea5..ad63f48 100644 --- a/lib/tp_plus.rb +++ b/lib/tp_plus.rb @@ -11,6 +11,7 @@ module TPPlus #utility functions require_relative 'tp_plus/utility_functions' +require_relative 'tp_plus/label_validator' #karel evnironment require_relative 'tp_plus/karel/karel' diff --git a/lib/tp_plus/function.rb b/lib/tp_plus/function.rb index da70d20..d8e8a54 100644 --- a/lib/tp_plus/function.rb +++ b/lib/tp_plus/function.rb @@ -192,6 +192,9 @@ def preinline(callnode, parent) def inline(parent) #local variable interpreter = @parser.interpreter.clone + # Label metadata is collected independently for every inline expansion. + # This prevents names from an earlier call from leaking into a later one. + interpreter.reset_inlined_label_names # ..IMPORTANT:: needed as interpreter.nodes may be different # from function member @nodes, at this point. Reason # unknown, although def interpret does copy the interpretter @@ -222,6 +225,10 @@ def inline(parent) #list warning messages lines += interpreter.list_warnings + # Preserve source label names after the inline interpreter is discarded. + # The final LS validator uses this to report @name alongside LBL[number]. + parent.merge_label_names(interpreter) + #pass back to parent interpreter what label number we left off on parent.current_label = interpreter.current_label @@ -285,6 +292,12 @@ def output_program(prog_options) output += ": ! ------- ;\n" end + LabelValidator.validate!( + output, + program_name: @name.upcase, + label_names: interpreter.label_names_by_number + ) + if prog_options[:output] filname = "#{prog_options[:output_folder]}/#{@name}.ls" File.write(filname, output) @@ -305,4 +318,4 @@ def contents(filename) return src end end -end \ No newline at end of file +end diff --git a/lib/tp_plus/interpreter.rb b/lib/tp_plus/interpreter.rb index 4f53de0..6495f4a 100644 --- a/lib/tp_plus/interpreter.rb +++ b/lib/tp_plus/interpreter.rb @@ -13,6 +13,7 @@ def initialize @header_data = {} @header_appl_data = [] @labels = {} + @inlined_label_names = {} @current_label = 99 @previous_set_label = [@current_label] @previous_set_label_index = 0 @@ -55,6 +56,34 @@ def renumber_labels end end + def reset_inlined_label_names + @inlined_label_names = {} + end + + def merge_label_names(context) + context.label_names_by_number.each do |number, names| + @inlined_label_names[number] ||= [] + @inlined_label_names[number].concat(names) + @inlined_label_names[number].uniq! + end + end + + def label_names_by_number + names_by_number = {} + + @inlined_label_names.each do |number, names| + names_by_number[number] = names.dup + end + + @labels.each do |name, number| + names_by_number[number] ||= [] + names_by_number[number] << name.to_s + names_by_number[number].uniq! + end + + names_by_number + end + def label_recur(nodes, labels) if nodes.is_a?(Array) nodes.each do |node| diff --git a/lib/tp_plus/label_validator.rb b/lib/tp_plus/label_validator.rb new file mode 100644 index 0000000..632ad87 --- /dev/null +++ b/lib/tp_plus/label_validator.rb @@ -0,0 +1,142 @@ +module TPPlus + class LabelValidationError < StandardError + attr_reader :program_name, :undefined_labels, :duplicate_labels, :label_names + + def initialize(program_name:, undefined_labels:, duplicate_labels:, label_names: {}) + @program_name = program_name + @undefined_labels = undefined_labels + @duplicate_labels = duplicate_labels + @label_names = label_names + + super(build_message) + end + + private + + def build_message + lines = ["TP+ label validation failed for program #{@program_name}."] + + unless @undefined_labels.empty? + lines << "Undefined direct label targets:" + @undefined_labels.each do |label, reference_lines| + lines << " #{label_display(label)} referenced at generated LS #{line_list(reference_lines)}; no definition was emitted." + end + end + + unless @duplicate_labels.empty? + lines << "Duplicate label definitions:" + @duplicate_labels.each do |label, definition_lines| + lines << " #{label_display(label)} defined #{definition_lines.length} times at generated LS #{line_list(definition_lines)}." + end + end + + lines << "Every directly referenced LBL[n] must be defined exactly once, and a label number may not be defined more than once in the same program." + unless @undefined_labels.empty? + lines << "Hint: keep jump_to @name and @name in the same TP+ scope; an inline function cannot target a caller-owned label." + end + lines.join("\n") + end + + def line_list(line_numbers) + noun = line_numbers.length == 1 ? "line" : "lines" + "#{noun} #{line_numbers.join(', ')}" + end + + def label_display(label) + names = Array(@label_names[label]) + return "LBL[#{label}]" if names.empty? + + source_names = names.map { |name| "@#{name.to_s.sub(/\A@/, '')}" } + "LBL[#{label}] (#{source_names.join(', ')})" + end + end + + class LabelValidator + MOTION_SECTION = "/MN".freeze + SECTION_END_PATTERN = %r{\A/(?:POS|END)\z}.freeze + LABEL_DEFINITION_PATTERN = /\ALBL\[(\d+)(?::[^\]]*)?\]\s*;?/i.freeze + DIRECT_LABEL_REFERENCE_PATTERN = /\b(?:JMP\s+|SKIP\s*,\s*|TIMEOUT\s*,\s*)LBL\[(\d+)\]/i.freeze + TP_LINE_PREFIX_PATTERN = /\A\s*:\s*/.freeze + + def self.validate!(output, program_name: "", label_names: {}) + new(output, program_name, label_names).validate! + end + + def initialize(output, program_name, label_names) + @output = output.to_s + @program_name = program_name.to_s + @label_names = normalize_label_names(label_names) + end + + def validate! + definitions, references = collect_labels + + undefined_labels = references.each_with_object({}) do |(label, lines), missing| + missing[label] = lines unless definitions.key?(label) + end + duplicate_labels = definitions.each_with_object({}) do |(label, lines), duplicates| + duplicates[label] = lines if lines.length > 1 + end + + return true if undefined_labels.empty? && duplicate_labels.empty? + + raise LabelValidationError.new( + program_name: @program_name, + undefined_labels: sorted_hash(undefined_labels), + duplicate_labels: sorted_hash(duplicate_labels), + label_names: @label_names + ) + end + + private + + def collect_labels + definitions = Hash.new { |hash, label| hash[label] = [] } + references = Hash.new { |hash, label| hash[label] = [] } + has_motion_section = @output.each_line.any? { |line| line.strip == MOTION_SECTION } + in_motion_section = !has_motion_section + + @output.each_line.with_index(1) do |line, line_number| + stripped_line = line.strip + + if stripped_line == MOTION_SECTION + in_motion_section = true + next + elsif has_motion_section && SECTION_END_PATTERN.match?(stripped_line) + in_motion_section = false + next + end + + next unless in_motion_section + + instruction = line.sub(TP_LINE_PREFIX_PATTERN, "").strip + next if instruction.empty? || instruction.start_with?("!") + + definition = LABEL_DEFINITION_PATTERN.match(instruction) + if definition + definitions[definition[1].to_i] << line_number + next + end + + instruction.scan(DIRECT_LABEL_REFERENCE_PATTERN) do |match| + label = match[0].to_i + references[label] << line_number unless references[label].last == line_number + end + end + + [definitions, references] + end + + def sorted_hash(hash) + hash.keys.sort.each_with_object({}) do |label, sorted| + sorted[label] = hash[label] + end + end + + def normalize_label_names(label_names) + (label_names || {}).each_with_object({}) do |(number, names), normalized| + normalized[number.to_i] = Array(names).compact.map(&:to_s).uniq.sort + end + end + end +end diff --git a/test/tp_plus/test_interpreter.rb b/test/tp_plus/test_interpreter.rb index 161ac41..08fe3d5 100644 --- a/test/tp_plus/test_interpreter.rb +++ b/test/tp_plus/test_interpreter.rb @@ -1172,6 +1172,39 @@ def test_labels_can_be_defined_after_jumps_to_them assert_prog "JMP LBL[100] ;\nLBL[100:foo] ;\n" end + def test_preserves_label_names_from_inline_scopes_for_validation + $stacks = TPPlus::Stacks.new + + parse("namespace Helpers + inline def rerun() + jump_to @non_coord_motion + end + end + + @non_coord_motion + Helpers::rerun() + Helpers::rerun()") + + output = @interpreter.eval + + assert_equal({ + 100 => ["non_coord_motion"], + 101 => ["non_coord_motion"], + 102 => ["non_coord_motion"] + }, @interpreter.label_names_by_number) + + error = assert_raise(TPPlus::LabelValidationError) do + TPPlus::LabelValidator.validate!( + output, + program_name: "INLINE_LABEL", + label_names: @interpreter.label_names_by_number + ) + end + + assert_include error.message, "LBL[101] (@non_coord_motion)" + assert_include error.message, "LBL[102] (@non_coord_motion)" + end + def test_multiple_motion_modifiers parse("p := P[1]\no := PR[1]\nlinear_move.to(p).at('max_speed').term(0).offset(o).time_before(0.5,foo())") assert_prog "L P[1:p] max_speed CNT0 Offset,PR[1:o] TB .50sec,CALL FOO ;\n" diff --git a/test/tp_plus/test_label_validator.rb b/test/tp_plus/test_label_validator.rb new file mode 100644 index 0000000..db715cd --- /dev/null +++ b/test/tp_plus/test_label_validator.rb @@ -0,0 +1,87 @@ +require_relative '../test_helper' + +class TestLabelValidator < Test::Unit::TestCase + def test_accepts_forward_label_references + output = program_with(<<~LS) + : JMP LBL[128] ; + : LBL[128:forward_target] ; + LS + + assert_true TPPlus::LabelValidator.validate!(output, program_name: "FORWARD") + end + + def test_reports_undefined_labels_and_reference_lines + output = program_with(<<~LS) + : JMP LBL[141] ; + : IF R[1]=1,JMP LBL[141] ; + LS + + error = assert_raise(TPPlus::LabelValidationError) do + TPPlus::LabelValidator.validate!( + output, + program_name: "TMP_CYL_PAD", + label_names: { 141 => :non_coord_motion } + ) + end + + assert_equal "TMP_CYL_PAD", error.program_name + assert_equal({ 141 => [3, 4] }, error.undefined_labels) + assert_equal({}, error.duplicate_labels) + assert_include error.message, "TP+ label validation failed for program TMP_CYL_PAD." + assert_equal({ 141 => ["non_coord_motion"] }, error.label_names) + assert_include error.message, "LBL[141] (@non_coord_motion) referenced at generated LS lines 3, 4; no definition was emitted." + assert_include error.message, "Hint: keep jump_to @name and @name in the same TP+ scope; an inline function cannot target a caller-owned label." + end + + def test_reports_duplicate_definitions + output = program_with(<<~LS) + : LBL[120:first] ; + : LBL[120] ; + : JMP LBL[120] ; + LS + + error = assert_raise(TPPlus::LabelValidationError) do + TPPlus::LabelValidator.validate!(output, program_name: "DUPLICATE") + end + + assert_equal({}, error.undefined_labels) + assert_equal({ 120 => [3, 4] }, error.duplicate_labels) + assert_include error.message, "LBL[120] defined 2 times at generated LS lines 3, 4." + end + + def test_ignores_indirect_targets_comments_and_position_data + output = <<~LS + /PROG INDIRECT + /MN + : JMP LBL[R[1]] ; + : ! JMP LBL[998] is example text ; + : MESSAGE[Check LBL[997]] ; + /POS + LBL[999] + /END + LS + + assert_true TPPlus::LabelValidator.validate!(output, program_name: "INDIRECT") + end + + def test_validates_output_fragments_without_an_mn_header + output = <<~LS + : Skip,LBL[150] ; + : TIMEOUT,LBL[151] ; + : LBL[150:skip_target] ; + : LBL[151] ; + LS + + assert_true TPPlus::LabelValidator.validate!(output, program_name: "FRAGMENT") + end + + private + + def program_with(motion_lines) + <<~LS + /PROG TEST + /MN + #{motion_lines}/END + LS + end +end