From 0ed1547e79a9e05e9e2b24eaf59204566cd86b6f Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Wed, 28 Aug 2019 17:34:29 -0700 Subject: [PATCH] Set Anattrib DataDescriptor for local variable references We now generate FormatDescriptors with WeakSymbolRefs for direct page references that match variable table entries. LocalVariableTable got a rewrite. We need to be unique in both name and address, but for the address we have to take the width into account as well. We also want to sort the display by address rather than name. (Some people might want it sorted by name, but we can worry about that some other time.) Updated the DefSymbol editor to require value uniqueness. Note addresses and constants exist in separate namespaces. The various symbols are added to the SymbolTable so that uniqueness checks work correctly. This also allows the operand generation to appear to work, but it doesn't yet handle redefinition of symbols. --- Asm65/OpDef.cs | 22 +++ SourceGen/DefSymbol.cs | 48 ++++- SourceGen/DisasmProject.cs | 112 ++++++++++- SourceGen/LineListGen.cs | 8 +- SourceGen/LocalVariableTable.cs | 175 ++++++++++++++++-- SourceGen/MainController.cs | 5 +- SourceGen/ProjectFile.cs | 16 +- SourceGen/WpfGui/EditDefSymbol.xaml | 5 +- SourceGen/WpfGui/EditDefSymbol.xaml.cs | 56 ++++-- .../WpfGui/EditLocalVariableTable.xaml.cs | 25 ++- 10 files changed, 408 insertions(+), 64 deletions(-) diff --git a/Asm65/OpDef.cs b/Asm65/OpDef.cs index fc70400..a795bd5 100644 --- a/Asm65/OpDef.cs +++ b/Asm65/OpDef.cs @@ -195,6 +195,28 @@ namespace Asm65 { public int Cycles { get { return CycDef & 0xff; } } public CycleMod CycleMods { get { return (CycleMod)(CycDef & ~0xff); } } + /// + /// True if the instruction's address mode is a direct page access. + /// + public bool IsDirectPageInstruction { + get { + switch (AddrMode) { + case AddressMode.DP: + case AddressMode.DPInd: + case AddressMode.DPIndexX: + case AddressMode.DPIndexXInd: + case AddressMode.DPIndexY: + case AddressMode.DPIndIndexY: + case AddressMode.DPIndIndexYLong: + case AddressMode.DPIndLong: + case AddressMode.StackDPInd: + return true; + default: + return false; + } + } + } + /// /// True if the operand's width is uniquely determined by the opcode mnemonic, even /// if the operation supports operands with varying widths. diff --git a/SourceGen/DefSymbol.cs b/SourceGen/DefSymbol.cs index b603d52..c07f300 100644 --- a/SourceGen/DefSymbol.cs +++ b/SourceGen/DefSymbol.cs @@ -134,6 +134,29 @@ namespace SourceGen { Tag = string.Empty; } + /// + /// Determines whether a symbol overlaps with a region. Useful for variables. + /// + /// Symbol to check. + /// Address. + /// Symbol width. + /// Symbol type to check against. + /// True if the symbols overlap. + public static bool CheckOverlap(DefSymbol a, int value, int width, Type type) { + if (a.DataDescriptor.Length <= 0 || width <= 0) { + return false; + } + if (a.Value < 0 || value < 0) { + return false; + } + if (a.SymbolType != type) { + return false; + } + int maxStart = Math.Max(a.Value, value); + int minEnd = Math.Min(a.Value + a.DataDescriptor.Length - 1, value + width - 1); + return (maxStart <= minEnd); + } + public static bool operator ==(DefSymbol a, DefSymbol b) { if (ReferenceEquals(a, b)) { @@ -142,19 +165,28 @@ namespace SourceGen { if (ReferenceEquals(a, null) || ReferenceEquals(b, null)) { return false; // one is null } - // All fields must be equal, except Xrefs. - if (a.DataDescriptor != b.DataDescriptor || - a.Comment != b.Comment || - a.Tag != b.Tag) { - return false; - } - return true; + return a.Equals(b); } public static bool operator !=(DefSymbol a, DefSymbol b) { return !(a == b); } public override bool Equals(object obj) { - return obj is DefSymbol && this == (DefSymbol)obj; + if (!(obj is DefSymbol)) { + return false; + } + // Do base-class equality comparison and the ReferenceEquals check. + if (!base.Equals(obj)) { + return false; + } + + // All fields must be equal, except Xrefs. + DefSymbol other = (DefSymbol)obj; + if (DataDescriptor != other.DataDescriptor || + Comment != other.Comment || + Tag != other.Tag) { + return false; + } + return true; } public override int GetHashCode() { return base.GetHashCode() ^ diff --git a/SourceGen/DisasmProject.cs b/SourceGen/DisasmProject.cs index 15d528e..75954ef 100644 --- a/SourceGen/DisasmProject.cs +++ b/SourceGen/DisasmProject.cs @@ -672,6 +672,11 @@ namespace SourceGen { // need to check all existing refs to confirm that the symbol hasn't been removed. // Symbol updates are sufficiently infrequent that this probably isn't worthwhile. + reanalysisTimer.StartTask("GenerateVariableRefs"); + // Generate references to variables. + GenerateVariableRefs(); + reanalysisTimer.EndTask("GenerateVariableRefs"); + // NOTE: we could at this point apply platform address symbols as code labels, so // that locations in the code that correspond to well-known addresses would pick // up the appropriate label instead of getting auto-labeled. It's unclear @@ -893,6 +898,110 @@ namespace SourceGen { } } + /// + /// Generates references to symbols in the local variable tables. + /// + /// These only apply to instructions with a specific set of addressing modes. + /// + /// This must be called after the code and data analysis passes have completed. It + /// should run before project/platform symbol references are generated, since we want + /// variables to take precedence. + /// + /// This also adds all symbols in non-hidden variable tables to the main SymbolTable, + /// for the benefit of uniqueness checks. + /// + private void GenerateVariableRefs() { + LocalVariableTable curTab = new LocalVariableTable(); + + int nextLvtIndex, nextLvtOffset; + if (LvTables.Count > 0) { + nextLvtIndex = 0; + nextLvtOffset = LvTables.Keys[0]; + } else { + nextLvtIndex = -1; + nextLvtOffset = FileData.Length; + } + + for (int offset = 0; offset < FileData.Length; ) { + // Have we reached the start of the next LV table? + while (offset >= nextLvtOffset) { + // We want to skip over any "hidden" tables. It's possible, if a bunch of + // tables got collapsed inside a data area, that we need to skip more than one. + if (offset == nextLvtOffset && mAnattribs[offset].IsStart) { + //Debug.WriteLine("FOUND +" + offset.ToString("x6")); + LocalVariableTable lvt = LvTables.Values[nextLvtIndex]; + if (lvt.ClearPrevious) { + curTab.Clear(); + } + + // Merge the new entries into the work table. This automatically + // discards entries that clash. + for (int i = 0; i < lvt.Count; i++) { + curTab.AddOrReplace(lvt[i]); + } + //curTab.DebugDump(); + + // All entries also get added to the main SymbolTable. This is a little + // wonky because the symbol might already exist with a different value. + // So long as the previous thing was also a variable, it doesn't matter. + AddVariablesToSymbolTable(lvt); + } else { + // Either this wasn't an instruction/data start, or we passed this + // one, which only happens for non-start items. Whatever the case, + // we're going to ignore it. + Debug.WriteLine("Ignoring LvTable +" + offset.ToString("x6")); + } + // Advance to next table. + nextLvtIndex++; + if (nextLvtIndex < LvTables.Keys.Count) { + nextLvtOffset = LvTables.Keys[nextLvtIndex]; + } else { + nextLvtOffset = FileData.Length; + } + } + + Anattrib attr = mAnattribs[offset]; + if (attr.IsInstructionStart && attr.DataDescriptor == null) { + OpDef op = CpuDef.GetOpDef(FileData[offset]); + if (op.IsDirectPageInstruction) { + Debug.Assert(attr.OperandAddress == FileData[offset + 1]); + DefSymbol defSym = curTab.GetByValueRange(attr.OperandAddress, 1, + Symbol.Type.ExternalAddr); + if (defSym != null) { + mAnattribs[offset].DataDescriptor = + FormatDescriptor.Create(attr.Length, + new WeakSymbolRef(defSym.Label, WeakSymbolRef.Part.Low), false); + } + } else if (op.AddrMode == OpDef.AddressMode.StackRel || + op.AddrMode == OpDef.AddressMode.StackRelIndIndexY) { + DefSymbol defSym = curTab.GetByValueRange(FileData[offset + 1], 1, + Symbol.Type.Constant); + if (defSym != null) { + mAnattribs[offset].DataDescriptor = + FormatDescriptor.Create(attr.Length, + new WeakSymbolRef(defSym.Label, WeakSymbolRef.Part.Low), false); + } + } + } + + if (attr.IsDataStart || attr.IsInlineDataStart) { + offset += attr.Length; + } else { + // Advance by one, not attr.Length, so we don't miss embedded instructions. + offset++; + } + } + } + + private void AddVariablesToSymbolTable(LocalVariableTable lvt) { + for (int i = 0; i < lvt.Count; i++) { + DefSymbol defSym = lvt[i]; + if (!SymbolTable.TryGetValue(defSym.Label, out Symbol sym)) { + SymbolTable[defSym.Label] = defSym; + } + } + } + /// /// Generates references to symbols in the project/platform symbol tables. /// @@ -1068,7 +1177,8 @@ namespace SourceGen { } else if (SymbolTable.TryGetValue(dfd.SymbolRef.Label, out Symbol sym)) { // Is this a reference to a project/platform symbol? if (sym.SymbolSource == Symbol.Source.Project || - sym.SymbolSource == Symbol.Source.Platform) { + sym.SymbolSource == Symbol.Source.Platform || + sym.SymbolSource == Symbol.Source.Variable) { DefSymbol defSym = sym as DefSymbol; int adj = 0; if (operandOffset >= 0) { diff --git a/SourceGen/LineListGen.cs b/SourceGen/LineListGen.cs index 6eb4fd8..c0655b3 100644 --- a/SourceGen/LineListGen.cs +++ b/SourceGen/LineListGen.cs @@ -946,7 +946,7 @@ namespace SourceGen { // Local variable tables come next. Defer rendering. if (mProject.LvTables.TryGetValue(offset, out LocalVariableTable lvt)) { - int count = lvt.Variables.Count; + int count = lvt.Count; // If "clear previous" is set, we output an additional line. if (lvt.ClearPrevious) { count++; @@ -1311,7 +1311,7 @@ namespace SourceGen { } } - if (lvt.Variables.Count == 0) { + if (lvt.Count == 0) { // If ClearPrevious is set, we returned the "clear" line for index zero. // So this is an empty table without a clear. We want to show something so // the user knows there's dead weight here. @@ -1320,11 +1320,11 @@ namespace SourceGen { Res.Strings.LOCAL_VARIABLE_TABLE_EMPTY); } - if (subLineIndex >= lvt.Variables.Values.Count) { + if (subLineIndex >= lvt.Count) { return FormattedParts.CreateLongComment("BAD INDEX +" + offset.ToString("x6") + " sub=" + subLineIndex); } else { - DefSymbol defSym = lvt.Variables.Values[subLineIndex]; + DefSymbol defSym = lvt[subLineIndex]; // Use an operand length of 1 so things are shown as concisely as possible. string addrStr = PseudoOp.FormatNumericOperand(mFormatter, mProject.SymbolTable, null, defSym.DataDescriptor, defSym.Value, 1, diff --git a/SourceGen/LocalVariableTable.cs b/SourceGen/LocalVariableTable.cs index a384158..1bef4c6 100644 --- a/SourceGen/LocalVariableTable.cs +++ b/SourceGen/LocalVariableTable.cs @@ -20,23 +20,28 @@ using System.Diagnostics; namespace SourceGen { /// /// Table of redefinable variables. A project may have several of these, at different - /// offsets. The contents of later tables overwrite the contents of earlier tables. + /// offsets. /// /// The class is mutable, but may only be modified by the LvTable editor (which makes /// changes to a work object that moves through the undo/redo buffer) or the /// deserializer. + /// + /// + /// The contents of later tables overwrite the contents of earlier tables. A + /// variable is replaced if the name is re-used (because a symbol can have only one + /// value at a time) or if the value is re-used (because they're applied automatically + /// and we need to know which symbol to use). + /// + /// The DefSymbols should have symbol type Constant or ExternalAddr. These do not clash + /// with each other, e.g. the statements "LDA $10,S" and "STA $10" use two different + /// variables, because one is an 8-bit stack offset while the other is an 8-bit direct page + /// address. /// /// (Referring to these as "local" variables is a bit of a misnomer, since they have /// global scope from the point where they're defined. The name reflects their intended /// usage, rather than how the assembler will treat them.) - /// + /// public class LocalVariableTable { - /// - /// List of variables. The symbol's label must be unique within a table, so we sort - /// on that. - /// - public SortedList Variables; - /// /// If set, all values from previous VariableTables should be discarded when this /// table is encountered. @@ -54,12 +59,25 @@ namespace SourceGen { /// public bool ClearPrevious { get; set; } + /// + /// List of variables, sorted by label. + /// + private SortedList mVarByLabel; + + /// + /// List of variables. This is manually sorted when needed. The key is a combination + /// of the value and the symbol type, so we can't use a simple SortedList. + /// + private List mVarByValue; + private bool mNeedSort = true; + /// /// Constructs an empty table. /// public LocalVariableTable() { - Variables = new SortedList(); + mVarByLabel = new SortedList(); + mVarByValue = new List(); } /// @@ -69,13 +87,132 @@ namespace SourceGen { public LocalVariableTable(LocalVariableTable src) : this() { ClearPrevious = src.ClearPrevious; - foreach (KeyValuePair kvp in src.Variables) { - Variables[kvp.Key] = kvp.Value; + foreach (KeyValuePair kvp in src.mVarByLabel) { + mVarByLabel[kvp.Value.Label] = kvp.Value; + mVarByValue.Add(kvp.Value); } Debug.Assert(this == src); } + /// + /// Number of entries in the variable table. + /// + public int Count { get { return mVarByLabel.Count; } } + + /// + /// Returns the Nth item, sorted by value. This is NOT a lookup by value. + /// + public DefSymbol this[int index] { + get { + SortIfNeeded(); + return mVarByValue[index]; + } + } + + private void SortIfNeeded() { + if (mNeedSort) { + // Currently sorting primarily by value, secondarily by label. This ordering + // determines how it appears in the code list. If we want to make it + // configurable we just need to replace the sort function. + mVarByValue.Sort((a, b) => { + int diff = a.Value - b.Value; + if (diff != 0) { + return diff; + } + return a.Label.CompareTo(b.Label); + }); + mNeedSort = false; + } + } + + public void Clear() { + mVarByLabel.Clear(); + mVarByValue.Clear(); + } + + public DefSymbol GetByLabel(string label) { + return mVarByLabel[label]; + } + + public void RemoveByLabel(string label) { + if (mVarByLabel.TryGetValue(label, out DefSymbol defSym)) { + mVarByLabel.Remove(defSym.Label); + mVarByValue.Remove(defSym); + } + Debug.Assert(mVarByValue.Count == mVarByLabel.Count); + + // Should not be necessary to re-sort the by-value list. + } + + /// + /// Finds symbols that overlap with the specified value and width. If more than one + /// matching symbol is found, an arbitrary match will be returned. Comparisons are + /// only performed between symbols of the same type, so addresses and constants do + /// not clash. + /// + /// Value to compare. + /// Width to check. + /// One matching symbol, or null if none matched. + public DefSymbol GetByValueRange(int value, int width, Symbol.Type type) { + foreach (KeyValuePair kvp in mVarByLabel) { + if (DefSymbol.CheckOverlap(kvp.Value, value, width, type)) { + return kvp.Value; + } + } + return null; + } + + /// + /// Adds a symbol to the variable table. Existing entries with the same name or + /// overlapping values will be removed. + /// + /// Symbol to add. + public void AddOrReplace(DefSymbol newSym) { + if (newSym.SymbolType != Symbol.Type.Constant && + newSym.SymbolType != Symbol.Type.ExternalAddr) { + Debug.Assert(false, "Unexpected symbol type " + newSym.SymbolType); + return; + } + if (newSym.SymbolSource != Symbol.Source.Variable) { + Debug.Assert(false, "Unexpected symbol source " + newSym.SymbolSource); + return; + } + + // Remove existing entries that match on label or value. The value check must + // take the width into account. + if (mVarByLabel.TryGetValue(newSym.Label, out DefSymbol labelSym)) { + mVarByLabel.Remove(labelSym.Label); + mVarByValue.Remove(labelSym); + } + + // Inefficient, but the list should be small. + DefSymbol valSym; + while ((valSym = GetByValueRange(newSym.Value, + newSym.DataDescriptor.Length, newSym.SymbolType)) != null) { + mVarByLabel.Remove(valSym.Label); + mVarByValue.Remove(valSym); + } + + mVarByLabel.Add(newSym.Label, newSym); + mVarByValue.Add(newSym); + Debug.Assert(mVarByValue.Count == mVarByLabel.Count); + + mNeedSort = true; + } + + /// + /// Returns a reference to the sorted-by-label list. The caller must not modify it. + /// + /// + /// This exists primarily for EditDefSymbol, which wants a list of this type to + /// perform uniqueness checks. + /// + public SortedList GetSortedByLabel() { + return mVarByLabel; + } + + public static bool operator ==(LocalVariableTable a, LocalVariableTable b) { if (ReferenceEquals(a, b)) { return true; // same object, or both null @@ -87,12 +224,12 @@ namespace SourceGen { if (a.ClearPrevious != b.ClearPrevious) { return false; } - if (a.Variables.Count != b.Variables.Count) { + if (a.mVarByLabel.Count != b.mVarByLabel.Count) { return false; } // Compare all list entries. - for (int i = 0; i < a.Variables.Count; i++) { - if (a.Variables.Values[i] != b.Variables.Values[i]) { + for (int i = 0; i < a.mVarByLabel.Count; i++) { + if (a.mVarByLabel.Values[i] != b.mVarByLabel.Values[i]) { return false; } } @@ -106,7 +243,7 @@ namespace SourceGen { } public override int GetHashCode() { int hashCode = 0; - foreach (KeyValuePair kvp in Variables) { + foreach (KeyValuePair kvp in mVarByLabel) { hashCode ^= kvp.Value.GetHashCode(); } if (ClearPrevious) { @@ -114,5 +251,13 @@ namespace SourceGen { } return hashCode; } + + public void DebugDump() { + Debug.WriteLine("LocalVariableTable count=" + Count + " clear-previous=" + + ClearPrevious); + for (int i = 0; i < Count; i++) { + Debug.WriteLine(" " + i + ": " + this[i]); + } + } } } diff --git a/SourceGen/MainController.cs b/SourceGen/MainController.cs index 3fe9c70..c737c21 100644 --- a/SourceGen/MainController.cs +++ b/SourceGen/MainController.cs @@ -1818,7 +1818,8 @@ namespace SourceGen { if (CodeLineList[selIndex].LineType == LineListGen.Line.Type.Code) { EditInstructionOperand(selOffset); } else { - Debug.Assert(CodeLineList[selIndex].LineType == LineListGen.Line.Type.Data); + // We allow the selection to include meta-data like .org and Notes. + //Debug.Assert(CodeLineList[selIndex].LineType == LineListGen.Line.Type.Data); EditDataOperand(selOffset); } } @@ -3189,7 +3190,7 @@ namespace SourceGen { if (mProject.LvTables.TryGetValue(line.FileOffset, out LocalVariableTable lvt)) { extraStr = string.Format("{0} entries, clear-previous={1}", - lvt.Variables.Count, lvt.ClearPrevious); + lvt.Count, lvt.ClearPrevious); } break; default: diff --git a/SourceGen/ProjectFile.cs b/SourceGen/ProjectFile.cs index bcb3755..36fb8de 100644 --- a/SourceGen/ProjectFile.cs +++ b/SourceGen/ProjectFile.cs @@ -322,10 +322,10 @@ namespace SourceGen { public SerLocalVariableTable() { } public SerLocalVariableTable(LocalVariableTable varTab) { - Variables = new List(varTab.Variables.Count); - foreach (KeyValuePair kvp in varTab.Variables) { - // Note kvp.Key is redundant -- same as kvp.Value.Label - Variables.Add(new SerDefSymbol(kvp.Value)); + Variables = new List(varTab.Count); + for (int i = 0; i < varTab.Count; i++) { + DefSymbol defSym = varTab[i]; + Variables.Add(new SerDefSymbol(defSym)); } ClearPrevious = varTab.ClearPrevious; @@ -831,7 +831,13 @@ namespace SourceGen { if (!CreateDefSymbol(serDef, contentVersion, report, out DefSymbol defSym)) { return false; } - outLvt.Variables.Add(defSym.Label, defSym); + if (defSym.SymbolSource != Symbol.Source.Variable) { + // not expected to happen + Debug.WriteLine("Found local variable with bad source: " + + defSym.SymbolSource); + continue; + } + outLvt.AddOrReplace(defSym); } return true; } diff --git a/SourceGen/WpfGui/EditDefSymbol.xaml b/SourceGen/WpfGui/EditDefSymbol.xaml index 9aea60d..07a7fab 100644 --- a/SourceGen/WpfGui/EditDefSymbol.xaml +++ b/SourceGen/WpfGui/EditDefSymbol.xaml @@ -53,6 +53,7 @@ limitations under the License. + @@ -72,8 +73,8 @@ limitations under the License. - - + + diff --git a/SourceGen/WpfGui/EditDefSymbol.xaml.cs b/SourceGen/WpfGui/EditDefSymbol.xaml.cs index 767a9b6..490f5a7 100644 --- a/SourceGen/WpfGui/EditDefSymbol.xaml.cs +++ b/SourceGen/WpfGui/EditDefSymbol.xaml.cs @@ -67,6 +67,18 @@ namespace SourceGen.WpfGui { } private string mComment; + public bool IsAddress { + get { return mIsAddress; } + set { mIsAddress = value; OnPropertyChanged(); UpdateControls(); } + } + private bool mIsAddress; + + public bool IsConstant { + get { return mIsConstant; } + set { mIsConstant = value; OnPropertyChanged(); UpdateControls(); } + } + private bool mIsConstant; + /// /// Format object to use when formatting addresses and constants. /// @@ -127,7 +139,7 @@ namespace SourceGen.WpfGui { widthEntry1.Visibility = widthEntry2.Visibility = labelUniqueLabel.Visibility = Visibility.Collapsed; labelUniqueLabel.Visibility = Visibility.Collapsed; - valueRangeLabel.Visibility = Visibility.Collapsed; + valueRangeLabel.Visibility = valueUniqueLabel.Visibility = Visibility.Collapsed; } } @@ -142,12 +154,12 @@ namespace SourceGen.WpfGui { Comment = mOldSym.Comment; if (mOldSym.SymbolType == Symbol.Type.Constant) { - constantRadioButton.IsChecked = true; + IsConstant = true; } else { - addressRadioButton.IsChecked = true; + IsAddress = true; } } else { - addressRadioButton.IsChecked = true; + IsAddress = true; } labelTextBox.Focus(); @@ -164,6 +176,7 @@ namespace SourceGen.WpfGui { bool labelValid = Asm65.Label.ValidateLabel(Label); bool labelUnique; + // NOTE: should be using Asm65.Label.LABEL_COMPARER? if (mDefSymbolList.TryGetValue(Label, out DefSymbol existing)) { // It's okay if it's the same object. labelUnique = (existing == mOldSym); @@ -171,7 +184,7 @@ namespace SourceGen.WpfGui { labelUnique = true; } - // For local variables, do a secondary uniqueness check. + // For local variables, do a secondary uniqueness check across the full symbol table. if (labelUnique && mSymbolTable != null) { labelUnique = !mSymbolTable.TryGetValue(Label, out Symbol sym); } @@ -179,31 +192,48 @@ namespace SourceGen.WpfGui { // Value must be blank, meaning "erase any earlier definition", or valid value. // (Hmm... don't currently have a way to specify "no symbol" in DefSymbol.) //if (!string.IsNullOrEmpty(valueTextBox.Text)) { - bool valueValid = ParseValue(out int value, out int unused2); + bool valueValid = ParseValue(out int thisValue, out int unused2); //} else { // valueValid = true; //} bool valueRangeValid = true; - if (mIsVariable && valueValid && (value < 0 || value > 255)) { + if (mIsVariable && valueValid && (thisValue < 0 || thisValue > 255)) { valueRangeValid = false; } bool widthValid = true; + int thisWidth = 0; if (widthEntry1.Visibility == Visibility.Visible) { - if (!int.TryParse(VarWidth, out int width) || - width < DefSymbol.MIN_WIDTH || width > DefSymbol.MAX_WIDTH) { + if (!int.TryParse(VarWidth, out thisWidth) || + thisWidth < DefSymbol.MIN_WIDTH || thisWidth > DefSymbol.MAX_WIDTH) { widthValid = false; } } + Symbol.Type symbolType = IsConstant ? Symbol.Type.Constant : Symbol.Type.ExternalAddr; + + // For a variable, the value must also be unique within the table. Values have + // width, so we need to check for overlap. + bool valueUniqueValid = true; + if (mIsVariable && valueValid && widthValid) { + foreach (KeyValuePair kvp in mDefSymbolList) { + if (DefSymbol.CheckOverlap(kvp.Value, thisValue, thisWidth, symbolType)) { + valueUniqueValid = false; + break; + } + } + } + labelNotesLabel.Foreground = labelValid ? mDefaultLabelColor : Brushes.Red; labelUniqueLabel.Foreground = projectLabelUniqueLabel.Foreground = labelUnique ? mDefaultLabelColor : Brushes.Red; valueNotesLabel.Foreground = valueValid ? mDefaultLabelColor : Brushes.Red; valueRangeLabel.Foreground = valueRangeValid ? mDefaultLabelColor : Brushes.Red; + valueUniqueLabel.Foreground = valueUniqueValid ? mDefaultLabelColor : Brushes.Red; widthNotesLabel.Foreground = widthValid ? mDefaultLabelColor : Brushes.Red; - IsValid = labelValid && labelUnique && valueValid && valueRangeValid && widthValid; + IsValid = labelValid && labelUnique && valueValid && valueRangeValid && + valueUniqueValid && widthValid; } private bool ParseValue(out int value, out int numBase) { @@ -218,8 +248,6 @@ namespace SourceGen.WpfGui { } private void OkButton_Click(object sender, RoutedEventArgs e) { - bool isConstant = (constantRadioButton.IsChecked == true); - ParseValue(out int value, out int numBase); FormatDescriptor.SubType subType = FormatDescriptor.GetSubTypeForBase(numBase); int width = DefSymbol.NO_WIDTH; @@ -228,8 +256,8 @@ namespace SourceGen.WpfGui { } NewSym = new DefSymbol(Label, value, - mIsVariable ? Symbol.Source.Project : Symbol.Source.Variable, - isConstant ? Symbol.Type.Constant : Symbol.Type.ExternalAddr, + mIsVariable ? Symbol.Source.Variable : Symbol.Source.Project, + IsConstant ? Symbol.Type.Constant : Symbol.Type.ExternalAddr, subType, Comment, string.Empty, width); DialogResult = true; diff --git a/SourceGen/WpfGui/EditLocalVariableTable.xaml.cs b/SourceGen/WpfGui/EditLocalVariableTable.xaml.cs index 53cdb7e..7c5435e 100644 --- a/SourceGen/WpfGui/EditLocalVariableTable.xaml.cs +++ b/SourceGen/WpfGui/EditLocalVariableTable.xaml.cs @@ -130,8 +130,8 @@ namespace SourceGen.WpfGui { private void LoadVariables() { Variables.Clear(); - foreach (KeyValuePair kvp in mWorkTable.Variables) { - DefSymbol defSym = kvp.Value; + for (int i = 0; i < mWorkTable.Count; i++) { + DefSymbol defSym = mWorkTable[i]; string typeStr; if (defSym.SymbolType == Symbol.Type.Constant) { typeStr = Res.Strings.ABBREV_CONSTANT; @@ -183,17 +183,17 @@ namespace SourceGen.WpfGui { return; } FormattedSymbol item = (FormattedSymbol)lvi.Content; - DefSymbol defSym = mWorkTable.Variables[item.Label]; + DefSymbol defSym = mWorkTable.GetByLabel(item.Label); DoEditSymbol(defSym); } private void NewSymbolButton_Click(object sender, RoutedEventArgs e) { - EditDefSymbol dlg = new EditDefSymbol(this, mFormatter, mWorkTable.Variables, null, - mSymbolTable, true); + EditDefSymbol dlg = new EditDefSymbol(this, mFormatter, mWorkTable.GetSortedByLabel(), + null, mSymbolTable, true); dlg.ShowDialog(); if (dlg.DialogResult == true) { Debug.WriteLine("ADD: " + dlg.NewSym); - mWorkTable.Variables[dlg.NewSym.Label] = dlg.NewSym; + mWorkTable.AddOrReplace(dlg.NewSym); // Reload the contents. This loses the selection, but that shouldn't be an // issue when adding new symbols. To do this incrementally we'd need to add @@ -207,18 +207,18 @@ namespace SourceGen.WpfGui { // Single-select list view, button dimmed when no selection. Debug.Assert(symbolsListView.SelectedItems.Count == 1); FormattedSymbol item = (FormattedSymbol)symbolsListView.SelectedItems[0]; - DefSymbol defSym = mWorkTable.Variables[item.Label]; + DefSymbol defSym = mWorkTable.GetByLabel(item.Label); DoEditSymbol(defSym); } private void DoEditSymbol(DefSymbol defSym) { - EditDefSymbol dlg = new EditDefSymbol(this, mFormatter, mWorkTable.Variables, defSym, - mSymbolTable, true); + EditDefSymbol dlg = new EditDefSymbol(this, mFormatter, mWorkTable.GetSortedByLabel(), + defSym, mSymbolTable, true); dlg.ShowDialog(); if (dlg.DialogResult == true) { // Label might have changed, so remove old before adding new. - mWorkTable.Variables.Remove(defSym.Label); - mWorkTable.Variables[dlg.NewSym.Label] = dlg.NewSym; + mWorkTable.RemoveByLabel(defSym.Label); + mWorkTable.AddOrReplace(dlg.NewSym); LoadVariables(); UpdateControls(); } @@ -230,8 +230,7 @@ namespace SourceGen.WpfGui { int selectionIndex = symbolsListView.SelectedIndex; FormattedSymbol item = (FormattedSymbol)symbolsListView.SelectedItems[0]; - DefSymbol defSym = mWorkTable.Variables[item.Label]; - mWorkTable.Variables.Remove(defSym.Label); + mWorkTable.RemoveByLabel(item.Label); LoadVariables(); UpdateControls();