diff --git a/SourceGen/DataAnalysis.cs b/SourceGen/DataAnalysis.cs index eff399a..5b2991b 100644 --- a/SourceGen/DataAnalysis.cs +++ b/SourceGen/DataAnalysis.cs @@ -1141,7 +1141,7 @@ namespace SourceGen { /// Offset of first byte in range. /// Offset of last byte in range. /// Character test delegate. - /// If set, the high bit in all character must be the + /// If set, the high bit in all characters must be the /// same. Used to enforce a single encoding when "low or high ASCII" is used. /// Number of strings found, or -1 if bad data identified. public static int RecognizeLen16Strings(byte[] fileData, int start, int end, diff --git a/SourceGen/MainController.cs b/SourceGen/MainController.cs index 67879a9..1b4b9ff 100644 --- a/SourceGen/MainController.cs +++ b/SourceGen/MainController.cs @@ -734,7 +734,7 @@ namespace SourceGen { DisasmProject proj = new DisasmProject(); mDataPathName = dataPathName; mProjectPathName = string.Empty; - byte[] fileData = null; + byte[] fileData; try { fileData = LoadDataFile(dataPathName); } catch (Exception ex) { @@ -1070,7 +1070,7 @@ namespace SourceGen { return; } - SystemDefSet sds = null; + SystemDefSet sds; try { sds = SystemDefSet.ReadFile(sysDefsPath); } catch (Exception ex) { @@ -1231,7 +1231,7 @@ namespace SourceGen { cancel = (dataPathName == null); return null; } - byte[] fileData = null; + byte[] fileData; try { fileData = LoadDataFile(dataPathName); } catch (Exception ex) { @@ -1877,7 +1877,6 @@ namespace SourceGen { int firstOffset = CodeLineList[selIndex].FileOffset; int lastOffset = CodeLineList[lastIndex].FileOffset; int nextOffset = lastOffset + CodeLineList[lastIndex].OffsetSpan; - AddressMap addrMap = mProject.AddrMap; // The offset of an arend directive is the last byte in the address region. It // has a span length of zero because it's a directive, so if it's selected as @@ -2246,7 +2245,7 @@ namespace SourceGen { } else { // We allow the selection to include meta-data like .org and Notes. //Debug.Assert(CodeLineList[selIndex].LineType == LineListGen.Line.Type.Data); - EditDataOperand(selOffset); + EditDataOperand(); } } @@ -2291,22 +2290,27 @@ namespace SourceGen { Debug.WriteLine("No change to label"); } - // Check for changes to a project property. The dialog can create a new entry or - // modify an existing entry. - if (dlg.ProjectPropertyResult != null) { - DefSymbol oldSym = dlg.PrevProjectPropertyResult; - DefSymbol newSym = dlg.ProjectPropertyResult; - ProjectProperties newProps = new ProjectProperties(mProject.ProjectProps); - // Add new entry, or replace existing entry. - if (oldSym != null) { - newProps.ProjectSyms.Remove(oldSym.Label); + // Check for changes to a project symbol. The dialog can create a new entry or + // modify an existing entry, but can't delete an entry. + if (dlg.ProjectSymbolResult != null) { + DefSymbol oldSym = dlg.OrigProjectSymbolResult; + DefSymbol newSym = dlg.ProjectSymbolResult; + if (oldSym == newSym) { + Debug.WriteLine("No actual change to project symbol"); + } else { + // Generate a completely new set of project properties. + ProjectProperties newProps = new ProjectProperties(mProject.ProjectProps); + // Add new symbol entry, or replace existing entry. + if (oldSym != null) { + newProps.ProjectSyms.Remove(oldSym.Label); + } + newProps.ProjectSyms.Add(newSym.Label, newSym); + UndoableChange uc = UndoableChange.CreateProjectPropertiesChange( + mProject.ProjectProps, newProps); + cs.Add(uc); } - newProps.ProjectSyms.Add(newSym.Label, newSym); - UndoableChange uc = UndoableChange.CreateProjectPropertiesChange( - mProject.ProjectProps, newProps); - cs.Add(uc); } else { - Debug.WriteLine("No change to project property"); + Debug.WriteLine("No change to project symbol"); } Debug.WriteLine("EditInstructionOperand: " + cs.Count + " changes"); @@ -2315,7 +2319,7 @@ namespace SourceGen { } } - private void EditDataOperand(int offset) { + private void EditDataOperand() { Debug.Assert(mMainWin.CodeListView_GetSelectionCount() > 0); TypedRangeSet trs = GroupedOffsetSetFromSelected(); @@ -2396,7 +2400,7 @@ namespace SourceGen { Debug.Assert(origDefSym.SymbolSource == Symbol.Source.Project); EditDefSymbol dlg = new EditDefSymbol(mMainWin, mFormatter, - mProject.ProjectProps.ProjectSyms, origDefSym, null); + mProject.ProjectProps.ProjectSyms, origDefSym, origDefSym, null); switch (col) { case CodeListColumn.Operand: @@ -2427,7 +2431,6 @@ namespace SourceGen { if (SelectionAnalysis.mNumItemsSelected != 1) { return false; } - EntityCounts counts = SelectionAnalysis.mEntityCounts; // Single line, must be code or a RegWidth directive. return (SelectionAnalysis.mLineType == LineListGen.Line.Type.Code || SelectionAnalysis.mLineType == LineListGen.Line.Type.RegWidthDirective); @@ -2455,7 +2458,6 @@ namespace SourceGen { if (SelectionAnalysis.mNumItemsSelected != 1) { return false; } - EntityCounts counts = SelectionAnalysis.mEntityCounts; // Single line, must be a visualization set or a place where one can be created. LineListGen.Line.Type lineType = SelectionAnalysis.mLineType; return (lineType == LineListGen.Line.Type.VisualizationSet || @@ -2680,7 +2682,6 @@ namespace SourceGen { Debug.Assert(thisAttr.DataDescriptor.Length == 1); int nextOffset = rng.Low + 1; - Anattrib nextAttr = mProject.GetAnattrib(nextOffset); // This must match what GroupedOffsetSetFromSelected() does. if (!mProject.UserLabels.ContainsKey(nextOffset) && !mProject.HasCommentNoteOrVis(nextOffset) && @@ -2963,7 +2964,6 @@ namespace SourceGen { matchType = LineListGen.Line.Type.ArEndDirective; } // Find first instance of specified type. - LineListGen.Line tmpLine = CodeLineList[topLineIndex]; while (CodeLineList[topLineIndex].LineType != matchType) { if (CodeLineList[topLineIndex].FileOffset > newLoc.Offset) { // This can happen if the region got deleted. @@ -3666,7 +3666,6 @@ namespace SourceGen { // header area len = 1; } - Anattrib attr = mProject.GetAnattrib(offset); Debug.Assert(len > 0); for (int i = offset; i < offset + len; i++) { rs.Add(i); diff --git a/SourceGen/UndoableChange.cs b/SourceGen/UndoableChange.cs index f3a055b..0f9836a 100644 --- a/SourceGen/UndoableChange.cs +++ b/SourceGen/UndoableChange.cs @@ -501,8 +501,8 @@ namespace SourceGen { /// /// Creates an UndoableChange for a change to the project properties. /// - /// Current note. - /// New note. + /// Old property set. + /// New property set. /// Change record. public static UndoableChange CreateProjectPropertiesChange(ProjectProperties oldProps, ProjectProperties newProps) { diff --git a/SourceGen/WpfGui/EditDefSymbol.xaml.cs b/SourceGen/WpfGui/EditDefSymbol.xaml.cs index 633b723..12a5beb 100644 --- a/SourceGen/WpfGui/EditDefSymbol.xaml.cs +++ b/SourceGen/WpfGui/EditDefSymbol.xaml.cs @@ -141,9 +141,14 @@ namespace SourceGen.WpfGui { private Formatter mNumFormatter; /// - /// Old symbol value. May be null. + /// Initial values for fields. May be null. /// - private DefSymbol mOldSym; + private DefSymbol mInitialSym; + + /// + /// Original symbol. May be null if this is a new symbol. + /// + private DefSymbol mOrigSym; /// /// List of existing symbols, for uniqueness check. The list will not be modified. @@ -173,19 +178,30 @@ namespace SourceGen.WpfGui { /// Constructor, for editing a project or platform symbol. /// public EditDefSymbol(Window owner, Formatter formatter, - SortedList defList, DefSymbol defSym, + SortedList defList, DefSymbol origSym, DefSymbol initVals, SymbolTable symbolTable) - : this(owner, formatter, defList, defSym, symbolTable, false, false) { } + : this(owner, formatter, defList, origSym, initVals, symbolTable, false, false) { } /// /// Constructor, for editing a local variable, or editing a project symbol with /// the value field locked. /// + /// Parent window. + /// Formatter object. + /// List of DefSymbols against which we test label uniqueness. + /// Original symbol definition. The new label is allowed to + /// match this. + /// Initial values for fields. This might be null, might be + /// the same as origSym, or might be the result of a previous unsaved edit. + /// Full symbol table, for an extended uniqueness check + /// used for local variables (which must not clash with user labels). + /// Set true when editing a local variable table entry. + /// Set true to prevents edits to the value and type. /// /// TODO(someday): disable the "constant" radio button unless CPU=65816. /// public EditDefSymbol(Window owner, Formatter formatter, - SortedList defList, DefSymbol defSym, + SortedList defList, DefSymbol origSym, DefSymbol initVals, SymbolTable symbolTable, bool isVariable, bool lockValueAndType) { InitializeComponent(); Owner = owner; @@ -193,7 +209,8 @@ namespace SourceGen.WpfGui { mNumFormatter = formatter; mDefSymbolList = defList; - mOldSym = defSym; + mOrigSym = origSym; + mInitialSym = initVals; mSymbolTable = symbolTable; IsVariable = isVariable; mReadOnlyValueAndType = lockValueAndType; @@ -215,24 +232,24 @@ namespace SourceGen.WpfGui { } private void Window_Loaded(object sender, RoutedEventArgs e) { - if (mOldSym != null) { - Label = mOldSym.GenerateDisplayLabel(mNumFormatter); - Value = mNumFormatter.FormatValueInBase(mOldSym.Value, - mOldSym.DataDescriptor.NumBase); - if (mOldSym.HasWidth) { - VarWidth = mOldSym.DataDescriptor.Length.ToString(); + if (mInitialSym != null) { + Label = mInitialSym.GenerateDisplayLabel(mNumFormatter); + Value = mNumFormatter.FormatValueInBase(mInitialSym.Value, + mInitialSym.DataDescriptor.NumBase); + if (mInitialSym.HasWidth) { + VarWidth = mInitialSym.DataDescriptor.Length.ToString(); } - Comment = mOldSym.Comment; + Comment = mInitialSym.Comment; - if (mOldSym.IsConstant) { + if (mInitialSym.IsConstant) { IsConstant = true; } else { IsAddress = true; } - if (mOldSym.Direction == DefSymbol.DirectionFlags.Read) { + if (mInitialSym.Direction == DefSymbol.DirectionFlags.Read) { IsReadChecked = true; - } else if (mOldSym.Direction == DefSymbol.DirectionFlags.Write) { + } else if (mInitialSym.Direction == DefSymbol.DirectionFlags.Write) { IsWriteChecked = true; } else { IsReadChecked = IsWriteChecked = true; @@ -279,37 +296,17 @@ namespace SourceGen.WpfGui { if (mDefSymbolList.TryGetValue(trimLabel, out DefSymbol existing)) { // We found a match. See if we're just seeing the symbol we're editing. + // If there's no "original" symbol, then the fact that we matched anything + // means the label is not unique. Otherwise, we consider it unique if the + // label matches the original symbol. // - // We mostly want to check the label, not the entire symbol, because otherwise - // things can go funny when multiple edits are done without flushing the data - // back to the symbol table. For example, when this is invoked from the - // Edit Project Symbol button in the instruction operand editor, the user might - // edit the comment field of an existing project symbol, hit OK here, then try - // to edit it again before closing the operand editor. In that case we get - // passed the edited DefSymbol, which no longer fully matches what's in the - // symbol table. - // - // We want to check the value as well, because of a weird case when the user - // edits an instruction operand with a user-modified symbol. For example, if - // the operand needs to be "FOO-1", so the user hand-edited the label to FOO. - // This allows the user to "create project symbol" with the symbol as the initial - // value, but the symbol would be for address FOO not FOO-1. (It would be best - // to disable "create project symbol" in this case.) - // - // TODO: we still don't handle the case where the user changes the label from - // FOO to FOO1 and then back to FOO without closing the instruction edit dialog. - // The problem is that we find a match for FOO in the symbol table without - // realizing that it was the original name before the edits began. To fix this - // we need to pass in the original label as well as the recently-edited symbol, - // and allow the new name to match either. - - // If there's no "previous" symbol, then any match means the label is not unique. - // Otherwise, we consider it unique if the label and value match what they were - // when this edit dialog was opened. - //labelUnique = (existing == mOldSym); - labelUnique = mOldSym != null && - (Asm65.Label.LABEL_COMPARER.Equals(existing.Label, mOldSym.Label) && - existing.Value == mOldSym.Value); + // We only need to check the label. Since we're comparing the original + // symbol to the value from the symbol table, it should be a total match, + // but the other fields don't actually matter. It's safer to let the Symbol + // class comparison operators do the work though. + labelUnique = (existing == mOrigSym); + //labelUnique = mOrigSym != null && + // Asm65.Label.LABEL_COMPARER.Equals(existing.Label, mOrigSym.Label); } else { labelUnique = true; } @@ -376,7 +373,7 @@ namespace SourceGen.WpfGui { bool valueUniqueValid = true; if (IsVariable && valueValid && widthValid) { foreach (KeyValuePair kvp in mDefSymbolList) { - if (kvp.Value != mOldSym && + if (kvp.Value != mOrigSym && DefSymbol.CheckOverlap(kvp.Value, thisValue, thisWidth, symbolType)) { valueUniqueValid = false; break; diff --git a/SourceGen/WpfGui/EditInstructionOperand.xaml.cs b/SourceGen/WpfGui/EditInstructionOperand.xaml.cs index 83adc58..328830f 100644 --- a/SourceGen/WpfGui/EditInstructionOperand.xaml.cs +++ b/SourceGen/WpfGui/EditInstructionOperand.xaml.cs @@ -49,14 +49,19 @@ namespace SourceGen.WpfGui { public int SymbolEditOffsetResult { get; private set; } /// - /// Edited project property, or null if no changes were made. + /// Project symbol for this operand, when this dialog was opened. Will be null if + /// there was no matching project symbol. This is used as the "before" value for + /// updates to the project symbol set. /// - public DefSymbol ProjectPropertyResult { get; private set; } + public DefSymbol OrigProjectSymbolResult { get; private set; } /// - /// The project property that was modified, or null if none. + /// Edited project symbol, or null if no changes were made. This is used as the "after" + /// value for updates to the project symbol. This dialog is not allowed to delete + /// project symbols, so if this is null there's nothing to do. /// - public DefSymbol PrevProjectPropertyResult { get; private set; } + public DefSymbol ProjectSymbolResult { get; private set; } + /// /// Updated label. @@ -216,7 +221,7 @@ namespace SourceGen.WpfGui { SymbolEditResult = mEditedLabel; } - ProjectPropertyResult = mEditedProjectSymbol; + ProjectSymbolResult = mEditedProjectSymbol; DialogResult = true; } @@ -970,7 +975,7 @@ namespace SourceGen.WpfGui { CreateEditProjectSymbolText = EDIT_PROJECT_SYMBOL; mEditedProjectSymbol = (DefSymbol)firstProject; - PrevProjectPropertyResult = mEditedProjectSymbol; + OrigProjectSymbolResult = mEditedProjectSymbol; } else { ShowNarNoProjectMatch = true; CreateEditProjectSymbolText = CREATE_PROJECT_SYMBOL; @@ -1019,20 +1024,25 @@ namespace SourceGen.WpfGui { } private void EditProjectSymbol_Click(object sender, RoutedEventArgs e) { - DefSymbol origSym = mEditedProjectSymbol; - if (origSym == null) { + DefSymbol initVals = mEditedProjectSymbol; + if (initVals == null) { // Need to start with a symbol so we can set the value field. string symName = "SYM"; if (!string.IsNullOrEmpty(SymbolLabel) && Asm65.Label.ValidateLabel(SymbolLabel)) { symName = SymbolLabel; } - origSym = new DefSymbol(symName, mOperandValue, Symbol.Source.Project, + initVals = new DefSymbol(symName, mOperandValue, Symbol.Source.Project, Symbol.Type.ExternalAddr, FormatDescriptor.SubType.None); } + // Edit the symbol, locking the value so it can only apply to this operand. + // We want to pass in the symbol as it was before we made any edits, so that + // the user can always change the label back to what it was when this dialog + // was first opened. EditDefSymbol dlg = new EditDefSymbol(this, mFormatter, - mProject.ProjectProps.ProjectSyms, origSym, null, false, true); + mProject.ProjectProps.ProjectSyms, OrigProjectSymbolResult, initVals, null, + false, true); if (dlg.ShowDialog() != true) { return; } @@ -1218,9 +1228,12 @@ namespace SourceGen.WpfGui { DefSymbol.DirectionFlags.ReadWrite, null, string.Empty); } + // Unlike project symbols, we don't pass the original symbol value in. This is + // because we update a local copy of the LV table immediately, and pass that in + // for the uniqueness checks. EditDefSymbol dlg = new EditDefSymbol(this, mFormatter, - mEditedLvTable.GetSortedByLabel(), initialVar, mProject.SymbolTable, - true, true); + mEditedLvTable.GetSortedByLabel(), mEditedLocalVar, initialVar, + mProject.SymbolTable, true, true); if (dlg.ShowDialog() == true) { if (mEditedLocalVar != dlg.NewSym) { // Integrate result. Future edits will start with this. diff --git a/SourceGen/WpfGui/EditLocalVariableTable.xaml.cs b/SourceGen/WpfGui/EditLocalVariableTable.xaml.cs index e7f66f4..71041cc 100644 --- a/SourceGen/WpfGui/EditLocalVariableTable.xaml.cs +++ b/SourceGen/WpfGui/EditLocalVariableTable.xaml.cs @@ -313,7 +313,7 @@ namespace SourceGen.WpfGui { private void NewSymbolButton_Click(object sender, RoutedEventArgs e) { EditDefSymbol dlg = new EditDefSymbol(this, mFormatter, mWorkTable.GetSortedByLabel(), - null, mSymbolTable, true, false); + null, null, mSymbolTable, true, false); dlg.ShowDialog(); if (dlg.DialogResult == true) { Debug.WriteLine("ADD: " + dlg.NewSym); @@ -338,7 +338,7 @@ namespace SourceGen.WpfGui { private void DoEditSymbol(DefSymbol defSym) { EditDefSymbol dlg = new EditDefSymbol(this, mFormatter, mWorkTable.GetSortedByLabel(), - defSym, mSymbolTable, true, false); + defSym, defSym, mSymbolTable, true, false); dlg.ShowDialog(); if (dlg.DialogResult == true) { // Label might have changed, so remove old before adding new. diff --git a/SourceGen/WpfGui/EditProjectProperties.xaml.cs b/SourceGen/WpfGui/EditProjectProperties.xaml.cs index 8e13066..52fa33b 100644 --- a/SourceGen/WpfGui/EditProjectProperties.xaml.cs +++ b/SourceGen/WpfGui/EditProjectProperties.xaml.cs @@ -656,8 +656,8 @@ namespace SourceGen.WpfGui { } private void NewSymbolButton_Click(object sender, RoutedEventArgs e) { - EditDefSymbol dlg = new EditDefSymbol(this, mFormatter, mWorkProps.ProjectSyms, null, - null); + EditDefSymbol dlg = new EditDefSymbol(this, mFormatter, mWorkProps.ProjectSyms, + null, null, null); dlg.ShowDialog(); if (dlg.DialogResult == true) { Debug.WriteLine("ADD: " + dlg.NewSym); @@ -693,7 +693,7 @@ namespace SourceGen.WpfGui { private void DoEditSymbol(DefSymbol defSym) { EditDefSymbol dlg = new EditDefSymbol(this, mFormatter, mWorkProps.ProjectSyms, - defSym, null); + defSym, defSym, null); dlg.ShowDialog(); if (dlg.DialogResult == true) { // Label might have changed, so remove old before adding new. @@ -886,7 +886,7 @@ namespace SourceGen.WpfGui { } private void MoveSingleItem(int selIndex, object selectedItem, int adj) { - string selected = (string)symbolFilesListBox.SelectedItem; + string selected = (string)selectedItem; PlatformSymbolIdentifiers.Remove(selected); PlatformSymbolIdentifiers.Insert(selIndex + adj, selected); symbolFilesListBox.SelectedIndex = selIndex + adj;