Fix Message update when broken symbolic ref is fixed

Renaming a user label doesn't cause a re-analysis, just a display
update, because nothing structural is changing.  However, that's not
quite true when you have a reference to a non-existent label (e.g.
"LDA hoser"), and you rename a label to match (e.g. change "blah"
to "hoser").  The most obvious consequence was that the Message list,
which enumerates the broken symbolic references, was not being
updated.

We now identify broken references during the refactoring rename, and
change the reanalysis mode accordingly.

There is a deeper problem, where undoing the label rename does the
wrong thing with the previously-broken symbolic references (in the
earlier example, it "undoes" them to "blah" rather than back to
"hoser").  I added some notes about that, but it's harder to fix.

Also, clean up some code that was still treating ReanalysisScope as
if it were bit flags.

SourceGen/DisasmProject.cs

@@ -1755,7 +1755,7 @@ namespace SourceGen {
                         mAnattribs[offset].Symbol = newSym;
                         SymbolTable.Remove(oldSym);
                         SymbolTable.Add(newSym);
-                        RefactorLabel(offset, oldSym.Label);
+                        RefactorLabel(offset, oldSym.Label, out bool unused);
                     }
                 }
             }
@@ -2244,7 +2244,16 @@ namespace SourceGen {
                                 if (oldValue != null) {
                                     // Update everything in Anattribs and OperandFormats that
                                     // referenced the old symbol.
-                                    RefactorLabel(offset, ((Symbol)oldValue).Label);
+                                    RefactorLabel(offset, ((Symbol)oldValue).Label,
+                                        out bool foundExisting);
+
+                                    // If we fixed one or more broken references, we need to do
+                                    // a deeper re-analysis.
+                                    if (foundExisting) {
+                                        Debug.WriteLine("Found existing broken ref to '" +
+                                            ((Symbol)newValue).Label + "'");
+                                        needReanalysis = UndoableChange.ReanalysisScope.DataOnly;
+                                    }
                                 }
 
                                 // Compute the affected offsets.  We have one special case to
@@ -2259,7 +2268,7 @@ namespace SourceGen {
                                         IsInProjectOrPlatformList((Symbol)newValue)) {
                                     Debug.WriteLine("Label change masked/unmasked " +
                                         "project/platform symbol");
-                                    needReanalysis |= UndoableChange.ReanalysisScope.DataOnly;
+                                    needReanalysis = UndoableChange.ReanalysisScope.DataOnly;
                                 } else {
                                     affectedOffsets.Add(offset);
 
@@ -2291,7 +2300,7 @@ namespace SourceGen {
                             if (mScriptManager.IsLabelSignificant((Symbol)oldValue,
                                     (Symbol)newValue)) {
                                 Debug.WriteLine("Plugin claims symbol is significant");
-                                needReanalysis |= UndoableChange.ReanalysisScope.CodeAndData;
+                                needReanalysis = UndoableChange.ReanalysisScope.CodeAndData;
                             }
                         }
                         break;
@@ -2456,7 +2465,9 @@ namespace SourceGen {
                     default:
                         break;
                 }
-                needReanalysis |= uc.ReanalysisRequired;
+                if (needReanalysis < uc.ReanalysisRequired) {
+                    needReanalysis = uc.ReanalysisRequired;
+                }
             }
 
             return needReanalysis;
@@ -2477,7 +2488,10 @@ namespace SourceGen {
         /// </summary>
         /// <param name="labelOffset">Offset with the just-renamed label.</param>
         /// <param name="oldLabel">Previous value.</param>
-        private void RefactorLabel(int labelOffset, string oldLabel) {
+        /// <param name="foundExisting">Set to true if we found an existing (broken) reference
+        ///   to the new label.</param>
+        private void RefactorLabel(int labelOffset, string oldLabel, out bool foundExisting) {
+            foundExisting = false;
             if (!mXrefs.TryGetValue(labelOffset, out XrefSet xrefs)) {
                 // This can happen if you add a label in a file section that nothing references,
                 // and then rename it.
@@ -2502,6 +2516,11 @@ namespace SourceGen {
                     // override that and display as e.g. hex.
                     continue;
                 }
+                if (Label.LABEL_COMPARER.Equals(newLabel, dfd.SymbolRef.Label)) {
+                    // We found an existing, previously-broken reference to the new label.
+                    // Let the caller know.
+                    foundExisting = true;
+                }
                 if (!Label.LABEL_COMPARER.Equals(oldLabel, dfd.SymbolRef.Label)) {
                     // This can happen if the xref is based on the operand offset,
                     // but the user picked a different symbol.  The xref generator

SourceGen/UndoableChange.cs

@@ -58,7 +58,8 @@ anything in the DisasmProject.
 *** When can we get away with only updating part of the display list (re-analysis=none)?
 - Changing a user label.  All lines that reference the label need to be updated in the
 display, but nothing in the analysis changes.  (This assumes we prevent you from renaming
-a label to be the same as an existing label, e.g. auto-generated labels.)
+a label to be the same as an existing label, e.g. auto-generated labels.)  This doesn't
+work quite right when broken symbolic references suddenly become un-broken, however.
 - Adding/removing/changing cosmetic items, like comments and notes.
 
 NOTE: all re-analysis requirements are symmetric for undo/redo.  Undoing a change requires
@@ -116,6 +117,9 @@ namespace SourceGen {
         /// <summary>
         /// Enum indicating what needs to be reanalyzed after a change.
         /// </summary>
+        /// <remarks>
+        /// These must be in ascending order of thoroughness.
+        /// </remarks>
         public enum ReanalysisScope {
             None = 0,
             DisplayOnly,
@@ -280,6 +284,24 @@ namespace SourceGen {
         /// <summary>
         /// Creates an UndoableChange for a label update.
         /// </summary>
+        /// <remarks>
+        /// TODO(maybe): this is a little broken when it comes to bad symbolic references.
+        ///
+        /// If you set an operand's symbol to a non-existent label "foo1", and then rename an
+        /// existing user from "bar1" to "foo1", the bad references become valid.  However, we
+        /// set the reanalysis mode to "None" here, which means the Message list and
+        /// cross-references aren't updated.  We can catch this easily during the update, in
+        /// the refactoring rename, and just change the reanalysis mode.
+        ///
+        /// However... in the above scenario, if you undo the change, we perform a refactoring
+        /// undo.  The reference to "foo1" becomes a reference to "bar1" instead of a broken
+        /// reference to "foo1".  This happens because we don't keep track of which references
+        /// were valid and which were previously broken.
+        ///
+        /// The fundamental problem is the refactoring rename done way deep down.  The proper
+        /// fix is probably to perform the refactoring at a higher level, and generate an
+        /// explicit change object for every symbol that is updated.
+        /// </remarks>
         /// <param name="offset">Affected offset.</param>
         /// <param name="oldSymbol">Current label.  May be null.</param>
         /// <param name="newSymbol">New label.  May be null.</param>