Label rework, part 4

Update the symbol lookup in EditInstructionOperand, EditDataOperand,
and GotoBox to correctly deal with non-unique labels.

This is a little awkward because we're doing lookups by name on
a non-unique symbol, and must resolve the ambiguity.  In the case of
an instruction operand that refers to an address this is pretty
straightforward.  For partial bytes (LDA #>:foo) or data directives
(.DD1 :foo) we have to take a guess.  We can probably make a more
informed guess than we currently are, e.g. the LDA case could find
the label that minimizes the adjustment, but I don't want to sink a
lot of time into this until I'm sure it'll be useful.

Data operands with multiple regions are something of a challenge,
but I'm not sure specifying a single symbol for multiple locations
is important.

The "goto" box just finds the match that's closest to the selection.
Unlike "find", it always grabs the closest, not the next one forward.
(Not sure if this is useful or confusing.)

SourceGen/DisasmProject.cs

@@ -2353,6 +2353,43 @@ namespace SourceGen {
             return -1;
         }
 
+        /// <summary>
+        /// Finds a user label by name, searching only non-unique local address labels.
+        /// </summary>
+        /// <param name="label">Label to search for.  Must not have the uniquifier tag (if
+        ///   it had one, you wouldn't be here).</param>
+        /// <param name="targetOffset">If multiple labels are found, we want the one that is
+        ///   closest to this offset.</param>
+        /// <returns>The symbol found, or null if no match.</returns>
+        public Symbol FindBestNonUniqueLabel(string label, int targetOffset) {
+            Symbol bestSym = null;
+            int bestDelta = int.MaxValue;
+
+            // Simple linear search.  Right now we're only doing this in a few specific
+            // UI-driven situations (edit operand, goto label), so performance isn't crucial.
+            foreach (KeyValuePair<int, Symbol> kvp in UserLabels) {
+                Symbol sym = kvp.Value;
+                if (sym.SymbolType != Symbol.Type.NonUniqueLocalAddr) {
+                    continue;
+                }
+                if (sym.LabelWithoutTag == label) {
+                    // found a match; is it the best one?
+                    int delta = Math.Abs(kvp.Key - targetOffset);
+                    if (delta < bestDelta) {
+                        //Debug.WriteLine("FindBest: " + sym.Label + "/" + delta + " better than " +
+                        //    (bestSym != null ? bestSym.Label : "-") + "/" + bestDelta);
+                        bestSym = sym;
+                        bestDelta = delta;
+                    } else {
+                        //Debug.WriteLine("FindBest: " + sym.Label + "/" + delta + " not better than " +
+                        //    (bestSym != null ? bestSym.Label : "-") + "/" + bestDelta);
+                    }
+                }
+            }
+
+            return bestSym;
+        }
+
         /// <summary>
         /// For debugging purposes, get some information about the currently loaded
         /// extension scripts.

SourceGen/MainController.cs

@@ -2402,7 +2402,13 @@ namespace SourceGen {
         }
 
         public void Goto() {
-            GotoBox dlg = new GotoBox(mMainWin, mProject, mOutputFormatter);
+            int index = mMainWin.CodeListView_GetFirstSelectedIndex();
+            if (index < 0) {
+                index = mMainWin.CodeListView_GetTopIndex();    // nothing selected
+            }
+            int offset = CodeLineList[index].FileOffset;
+
+            GotoBox dlg = new GotoBox(mMainWin, mProject, offset, mOutputFormatter);
             if (dlg.ShowDialog() == true) {
                 GoToLocation(new NavStack.Location(dlg.TargetOffset, 0, false),
                     GoToMode.JumpToCodeData, true);

SourceGen/ProjectFile.cs

@@ -279,7 +279,7 @@ namespace SourceGen {
 
             public SerSymbol() { }
             public SerSymbol(Symbol sym) {
-                Label = sym.LabelForSerialization;      // use bare label here
+                Label = sym.LabelWithoutTag;      // use bare label here
                 Value = sym.Value;
                 Source = sym.SymbolSource.ToString();
                 Type = sym.SymbolType.ToString();

SourceGen/PseudoOp.cs

@@ -720,7 +720,7 @@ namespace SourceGen {
             // if it's from the assembler.
             if ((flags & FormatNumericOpFlags.OmitLabelPrefixSuffix) == 0) {
                 symLabel = Symbol.ConvertLabelForDisplay(symLabel, sym.LabelAnno,
-                    sym.IsNonUnique, formatter);
+                    true, formatter);
             } else {
                 // TODO(xyzzy): remapper will handle this
                 symLabel = Symbol.ConvertLabelForDisplay(symLabel, Symbol.LabelAnnotation.None,
@@ -863,7 +863,7 @@ namespace SourceGen {
             }
             if ((flags & FormatNumericOpFlags.OmitLabelPrefixSuffix) == 0) {
                 symLabel = Symbol.ConvertLabelForDisplay(symLabel, sym.LabelAnno,
-                    sym.IsNonUnique, formatter);
+                    true, formatter);
             } else {
                 // TODO(xyzzy): remapper will handle this
                 symLabel = Symbol.ConvertLabelForDisplay(symLabel, Symbol.LabelAnnotation.None,
@@ -964,7 +964,7 @@ namespace SourceGen {
             }
             if ((flags & FormatNumericOpFlags.OmitLabelPrefixSuffix) == 0) {
                 symLabel = Symbol.ConvertLabelForDisplay(symLabel, sym.LabelAnno,
-                    sym.IsNonUnique, formatter);
+                    true, formatter);
             } else {
                 // TODO(xyzzy): remapper will handle this
                 symLabel = Symbol.ConvertLabelForDisplay(symLabel, Symbol.LabelAnnotation.None,

SourceGen/SGTestData/Source/2023-non-unique-labels.S

@@ -30,7 +30,7 @@ global1 nop                 ;EDIT
         bne     :loop2
         dex
         bne     :loop1
-        jmp     btarg   
+        jmp     btarg
 
 global2 nop                 ;EDIT
 
@@ -53,16 +53,34 @@ global4 nop                 ;EDIT
 gloop   dex                 ;EDIT: local, name "loop"
 global5 nop
         bne     gloop
-        
+
         nop
 
 ; Test symbolic references.
-
 global6 nop
 :spin1  jsr     :spin2      ;EDIT: local, name "spin1", operand ref to ":spin2"
 :spin2  jsr     :spin1      ;EDIT: local, name "spin2", operand ref to ":spin1"
         nop
 :spin3  lda     :spin3      ;EDIT: local, name "spin1", operand ref to ":spin1"
+        beq     :spin3      ;EDIT: operand ref to ":spin1"
+
+        lda     #<:spin1
+        ldx     #<:spin2
+        lda     #>:spin1
+        ldx     #>:spin2
+        bne     :skip
+
+        dw      :spin1      ;EDIT: local, name "spin1"
+        dw      :spin2      ;EDIT: local, name "spin1" (will be offset)
+        dw      :spin3      ;EDIT: local, name "spin1"
+
+        dfb     <:spin1     ;EDIT: local, name "spin1" (may need to do as
+        dfb     <:spin2     ;EDIT: local, name "spin1"  unique names and then
+        dfb     >:spin1     ;EDIT: local, name "spin1"  rename afterward)
+        dfb     >:spin2     ;EDIT: local, name "spin1"
+
+:skip   nop                 ;EDIT: local
+
 
 ; Semi-related: test labels that are nothing but underscores.
 global_ nop

SourceGen/Symbol.cs

@@ -119,9 +119,9 @@ namespace SourceGen {
 
 
         /// <summary>
-        /// Label without the non-unique tag.
+        /// Label without the non-unique tag.  Used for serialization.
         /// </summary>
-        public string LabelForSerialization {
+        public string LabelWithoutTag {
             get {
                 if (SymbolType != Type.NonUniqueLocalAddr) {
                     return Label;
@@ -217,7 +217,7 @@ namespace SourceGen {
             Debug.Assert(uniqueTag >= 0 && uniqueTag < 0x01000000); // fit in 6 hex digits
             Debug.Assert(label.IndexOf(UNIQUE_TAG_CHAR) < 0);       // already extended?
 
-            Value = value;      // passed a bogus value earlier for assert
+            Value = value;      // passed a bogus value to base ctor for assert
 
             // Add tag to label to make it unique.
             Label = label + UNIQUE_TAG_CHAR + uniqueTag.ToString("x6");
@@ -230,7 +230,7 @@ namespace SourceGen {
         /// <param name="formatter">Formatter object.</param>
         /// <returns>Label suitable for display.</returns>
         public string GenerateDisplayLabel(Asm65.Formatter formatter) {
-            return ConvertLabelForDisplay(Label, LabelAnno, IsNonUnique, formatter);
+            return ConvertLabelForDisplay(Label, LabelAnno, true, formatter);
         }
 
         /// <summary>
@@ -273,14 +273,13 @@ namespace SourceGen {
                 bool showNonUnique, Asm65.Formatter formatter) {
             StringBuilder sb = new StringBuilder(label.Length + 2);
 
-            if (showNonUnique) {
-                sb.Append(formatter.NonUniqueLabelPrefix);
-            }
-
             if (label.Length > NON_UNIQUE_LEN &&
                     label[label.Length - NON_UNIQUE_LEN] == UNIQUE_TAG_CHAR) {
                 // showNonUnique may be false if generating assembly code (but by this
                 // point the unique tag should be remapped away)
+                if (showNonUnique) {
+                    sb.Append(formatter.NonUniqueLabelPrefix);
+                }
                 sb.Append(label.Substring(0, label.Length - NON_UNIQUE_LEN));
             } else {
                 sb.Append(label);

SourceGen/WpfGui/EditDataOperand.xaml.cs

@@ -64,6 +64,11 @@ namespace SourceGen.WpfGui {
         /// </summary>
         public FormatDescriptor mFirstFormatDescriptor;
 
+        /// <summary>
+        /// Project reference.
+        /// </summary>
+        private DisasmProject mProject;
+
         /// <summary>
         /// Raw file data.
         /// </summary>
@@ -129,6 +134,7 @@ namespace SourceGen.WpfGui {
             Owner = owner;
             DataContext = this;
 
+            mProject = project;
             mFileData = project.FileData;
             mSymbolTable = project.SymbolTable;
             mAddrMap = project.AddrMap;
@@ -345,11 +351,14 @@ namespace SourceGen.WpfGui {
             bool isOk = true;
             if (radioSimpleDataSymbolic.IsChecked == true) {
                 // Just check for correct format.  References to non-existent labels are allowed.
-                isOk = Asm65.Label.ValidateLabel(symbolEntryTextBox.Text);
+                Symbol.TrimAndValidateLabel(symbolEntryTextBox.Text,
+                    mFormatter.NonUniqueLabelPrefix, out isOk, out bool unused1,
+                    out bool unused2, out bool unused3, out Symbol.LabelAnnotation unused4);
 
-                // Actually, let's discourage references to auto-labels.
+                // Actually, let's discourage references to auto-labels and variables.
                 if (isOk && mSymbolTable.TryGetValue(symbolEntryTextBox.Text, out Symbol sym)) {
-                    isOk = sym.SymbolSource != Symbol.Source.Auto;
+                    isOk = sym.SymbolSource != Symbol.Source.Auto &&
+                           sym.SymbolSource != Symbol.Source.Variable;
                 }
             }
             IsValid = isOk;
@@ -819,7 +828,9 @@ namespace SourceGen.WpfGui {
                                         break;
                                 }
                                 Debug.Assert(dfd.HasSymbol);
-                                symbolEntryTextBox.Text = dfd.SymbolRef.Label;
+                                symbolEntryTextBox.Text = Symbol.ConvertLabelForDisplay(
+                                    dfd.SymbolRef.Label, Symbol.LabelAnnotation.None,
+                                    true, mFormatter);
                                 break;
                             default:
                                 Debug.Assert(false);
@@ -965,7 +976,35 @@ namespace SourceGen.WpfGui {
                         part = WeakSymbolRef.Part.Low;
                     }
                     subType = FormatDescriptor.SubType.Symbol;
-                    symbolRef = new WeakSymbolRef(symbolEntryTextBox.Text, part);
+
+                    string weakLabel = symbolEntryTextBox.Text;
+                    // Deal with non-unique labels.  If the label refers to an existing
+                    // symbol, use its label, which will have the tag.  If the label doesn't
+                    // have a match, discard it -- we don't support weak refs to ambiguous
+                    // non-unique symbols.
+                    string trimLabel = Symbol.TrimAndValidateLabel(weakLabel,
+                        mFormatter.NonUniqueLabelPrefix, out bool isValid, out bool unused1,
+                        out bool unused2, out bool hasNonUniquePrefix,
+                        out Symbol.LabelAnnotation unused3);
+                    if (isValid && hasNonUniquePrefix) {
+                        // We want to find the match that's closest to the thing we're
+                        // referencing, but that's awkward when there's multiple ranges and
+                        // multiple ways of interpreting the data.  So as a simple measure
+                        // we just grab the lowest offset in the first range.
+                        IEnumerator<TypedRangeSet.TypedRange> oiter = mSelection.RangeListIterator;
+                        oiter.MoveNext();
+                        TypedRangeSet.TypedRange rng = oiter.Current;
+                        int matchOffset = rng.Low;
+
+                        Symbol osym = mProject.FindBestNonUniqueLabel(trimLabel, matchOffset);
+                        if (osym != null) {
+                            weakLabel = osym.Label;
+                        } else {
+                            Debug.WriteLine("Attempt to create ref to nonexistant non-unique sym");
+                            subType = FormatDescriptor.SubType.Hex;
+                        }
+                    }
+                    symbolRef = new WeakSymbolRef(weakLabel, part);
                 } else {
                     Debug.Assert(false);
                 }

SourceGen/WpfGui/EditInstructionOperand.xaml.cs

@@ -225,13 +225,30 @@ namespace SourceGen.WpfGui {
         /// Looks up the symbol in the symbol table.  If not found there, it checks for a
         /// match against the existing or edited project symbol.
         /// </summary>
-        private bool LookupSymbol(string label, out Symbol sym) {
+        private bool LookupSymbol(string label, bool isNonUnique, out Symbol sym) {
-            if (mProject.SymbolTable.TryGetValue(label, out sym)) {
+            if (isNonUnique) {
-                return true;
+                // Only applies to labels, so no need to check mEditedProjectSymbol.  We
-            }
+                // could check mEditedLabel, but there's no reason to add a symbolic reference
-            if (mEditedProjectSymbol != null && label.Equals(mEditedProjectSymbol.Label)) {
+                // on top of the numeric reference.
-                sym = mEditedProjectSymbol;
+                int targetOffset;
-                return true;
+                Anattrib attr = mProject.GetAnattrib(mOffset);
+                if (attr.OperandOffset >= 0) {
+                    targetOffset = attr.OperandOffset;
+                } else {
+                    targetOffset = mOffset;
+                }
+                sym = mProject.FindBestNonUniqueLabel(label, targetOffset);
+                if (sym != null) {
+                    return true;
+                }
+            } else {
+                if (mProject.SymbolTable.TryGetValue(label, out sym)) {
+                    return true;
+                }
+                if (mEditedProjectSymbol != null && label.Equals(mEditedProjectSymbol.Label)) {
+                    sym = mEditedProjectSymbol;
+                    return true;
+                }
             }
             return false;
         }
@@ -253,10 +270,15 @@ namespace SourceGen.WpfGui {
             if (FormatSymbol) {
                 IsPartPanelEnabled = mOpDef.IsExtendedImmediate;
 
-                if (!Asm65.Label.ValidateLabel(SymbolLabel)) {
+                string trimLabel = Symbol.TrimAndValidateLabel(SymbolLabel,
+                    mFormatter.NonUniqueLabelPrefix, out bool isValid, out bool unused1,
+                    out bool unused2, out bool hasNonUniquePrefix,
+                    out Symbol.LabelAnnotation unused3);
+
+                if (!isValid) {
                     SymbolValueHex = SYMBOL_INVALID;
                     IsValid = false;
-                } else if (LookupSymbol(SymbolLabel, out Symbol sym)) {
+                } else if (LookupSymbol(trimLabel, hasNonUniquePrefix, out Symbol sym)) {
                     if (sym.SymbolSource == Symbol.Source.Auto) {
                         // We try to block references to auto labels, but it's possible to get
                         // around it because FormatDescriptors are weak references (replace auto
@@ -278,7 +300,8 @@ namespace SourceGen.WpfGui {
                     SymbolValueHex = mFormatter.FormatHexValue(sym.Value, 4);
                     SymbolValueDecimal = mFormatter.FormatDecimalValue(sym.Value);
                 } else {
-                    // Valid but unknown symbol.  This is fine -- symbols don't have to exist.
+                    // Valid but unknown symbol.  This is fine -- symbols don't have to exist --
+                    // but it's a little weird for non-unique symbols.
                     SymbolValueHex = SYMBOL_UNKNOWN;
                 }
             } else {
@@ -365,7 +388,11 @@ namespace SourceGen.WpfGui {
                     sb.Append(mFormatter.FormatCharacterValue(operandValue, enc));
                     break;
                 case FormatDescriptor.SubType.Symbol:
-                    if (LookupSymbol(dfd.SymbolRef.Label, out Symbol sym)) {
+                    string trimLabel = Symbol.TrimAndValidateLabel(SymbolLabel,
+                        mFormatter.NonUniqueLabelPrefix, out bool isValid, out bool unused1,
+                        out bool unused2, out bool hasNonUniquePrefix,
+                        out Symbol.LabelAnnotation unused3);
+                    if (LookupSymbol(trimLabel, hasNonUniquePrefix, out Symbol sym)) {
                         // Block move is a little weird.  "MVN label1,label2" is supposed to use
                         // the bank byte, while "MVN #const1,#const2" uses the entire symbol.
                         // The easiest thing to do is require the user to specify the "bank"
@@ -500,8 +527,10 @@ namespace SourceGen.WpfGui {
                 // Set the radio button when the user starts typing.
                 if (mLoadDone) {
                     FormatSymbol = true;
+                    // this calls UpdateControls; don't do it twice
+                } else {
+                    UpdateControls();
                 }
-                UpdateControls();
             }
         }
         private string mSymbolLabel;
@@ -634,7 +663,8 @@ namespace SourceGen.WpfGui {
                                     Debug.Assert(false);
                                     break;
                             }
-                            SymbolLabel = dfd.SymbolRef.Label;
+                            SymbolLabel = Symbol.ConvertLabelForDisplay(dfd.SymbolRef.Label,
+                                Symbol.LabelAnnotation.None, true, mFormatter);
                             break;
                         case FormatDescriptor.SubType.None:
                         default:
@@ -685,8 +715,28 @@ namespace SourceGen.WpfGui {
                     Debug.Assert(false);
                     part = WeakSymbolRef.Part.Low;
                 }
+
+                string weakLabel = SymbolLabel;
+
+                // Deal with non-unique labels.  If the label refers to an existing
+                // symbol, use its label, which will have the tag.  If the label doesn't
+                // have a match, discard it -- we don't support weak refs to ambiguous
+                // non-unique symbols.
+                string trimLabel = Symbol.TrimAndValidateLabel(SymbolLabel,
+                    mFormatter.NonUniqueLabelPrefix, out bool isValid, out bool unused1,
+                    out bool unused2, out bool hasNonUniquePrefix,
+                    out Symbol.LabelAnnotation unused3);
+                if (isValid && hasNonUniquePrefix) {
+                    if (LookupSymbol(trimLabel, hasNonUniquePrefix, out Symbol sym)) {
+                        weakLabel = sym.Label;
+                    } else {
+                        Debug.WriteLine("Attempt to create ref to non-existant non-unique sym");
+                        return null;
+                    }
+                }
+
                 return FormatDescriptor.Create(instructionLength,
-                    new WeakSymbolRef(SymbolLabel, part), false);
+                    new WeakSymbolRef(weakLabel, part), false);
             }
 
             FormatDescriptor.SubType subType;
@@ -863,7 +913,7 @@ namespace SourceGen.WpfGui {
                     } else {
                         NarLabelOffsetText = CURRENT_LABEL;
                     }
-                    NarTargetLabel = sym.Label;
+                    NarTargetLabel = sym.GenerateDisplayLabel(mFormatter);
                     mEditedLabel = sym;
                     CreateEditLabelText = EDIT_LABEL;
                 } else {
@@ -943,7 +993,7 @@ namespace SourceGen.WpfGui {
             } else {
                 ShowNarCurrentLabel = true;
                 CreateEditLabelText = EDIT_LABEL;
-                NarTargetLabel = mEditedLabel.Label;
+                NarTargetLabel = mEditedLabel.GenerateDisplayLabel(mFormatter);
             }
 
             // Sort of nice to just hit return twice after entering a label, so move the focus

SourceGen/WpfGui/GotoBox.xaml.cs

@@ -37,6 +37,11 @@ namespace SourceGen.WpfGui {
         /// </summary>
         private DisasmProject mProject;
 
+        /// <summary>
+        /// Initial offset, used for finding the best match among non-unique labels.
+        /// </summary>
+        private int mInitialOffset;
+
         /// <summary>
         /// Reference to formatter.  This determines how values are displayed.
         /// </summary>
@@ -88,12 +93,13 @@ namespace SourceGen.WpfGui {
         }
 
 
-        public GotoBox(Window owner, DisasmProject proj, Formatter formatter) {
+        public GotoBox(Window owner, DisasmProject proj, int initialOffset, Formatter formatter) {
             InitializeComponent();
             Owner = owner;
             DataContext = this;
 
             mProject = proj;
+            mInitialOffset = initialOffset;
             mFormatter = formatter;
             TargetOffset = -1;
         }
@@ -138,7 +144,23 @@ namespace SourceGen.WpfGui {
 
             // Try it as a label.  If they give the label a hex name (e.g. "A001") they
             // can prefix it with '$' to disambiguate the address.
-            int labelOffset = mProject.FindLabelOffsetByName(input);
+            int labelOffset = -1;
+            string trimLabel = Symbol.TrimAndValidateLabel(input,
+                mFormatter.NonUniqueLabelPrefix, out bool isValid, out bool unused1,
+                out bool unused2, out bool hasNonUniquePrefix,
+                out Symbol.LabelAnnotation unused3);
+            if (isValid) {
+                // Could be a label.  See if there's a match.
+                if (hasNonUniquePrefix) {
+                    Symbol sym = mProject.FindBestNonUniqueLabel(trimLabel, mInitialOffset);
+                    if (sym != null) {
+                        labelOffset = mProject.FindLabelOffsetByName(sym.Label);
+                    }
+                } else {
+                    labelOffset = mProject.FindLabelOffsetByName(trimLabel);
+                }
+            }
+
             if (labelOffset >= 0) {
                 TargetOffset = labelOffset;
             } else if (Address.ParseAddress(input, 1 << 24, out int addr)) {
@@ -158,10 +180,10 @@ namespace SourceGen.WpfGui {
             if (TargetOffset >= 0) {
                 offsetStr = mFormatter.FormatOffset24(TargetOffset);
                 int addr = mProject.GetAnattrib(TargetOffset).Address;
-                addressStr = mFormatter.FormatAddress(addr, addr > 0xffff);
+                addressStr = "$" + mFormatter.FormatAddress(addr, addr > 0xffff);
                 Symbol sym = mProject.GetAnattrib(TargetOffset).Symbol;
                 if (sym != null) {
-                    labelStr = sym.Label;
+                    labelStr = sym.GenerateDisplayLabel(mFormatter);
                 }
             }