-
Notifications
You must be signed in to change notification settings - Fork 236
Fix edit in one project not being reflected in consuming project #12439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -173,16 +173,37 @@ public void Initialize(IncrementalGeneratorInitializationContext context) | |
| return true; | ||
| } | ||
|
|
||
| if (oldSymbol is IAssemblySymbol oldAssembly && newSymbol is IAssemblySymbol newAssembly) | ||
| if (oldSymbol is not IAssemblySymbol oldAssembly || newSymbol is not IAssemblySymbol newAssembly) | ||
| { | ||
| var oldModuleMVIDs = oldAssembly.Modules.Select(GetMVID); | ||
| var newModuleMVIDs = newAssembly.Modules.Select(GetMVID); | ||
| return oldModuleMVIDs.SequenceEqual(newModuleMVIDs); | ||
| return false; | ||
| } | ||
|
|
||
| // Compare the MVIDs of the modules in each assembly. If they aren't present or don't match we don't consider them equal | ||
| var oldModules = oldAssembly.Modules.ToArray(); | ||
| var newModules = newAssembly.Modules.ToArray(); | ||
| if (oldModules.Length != newModules.Length) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| static Guid GetMVID(IModuleSymbol m) => m.GetMetadata()?.GetModuleVersionId() ?? Guid.Empty; | ||
| for (int i = 0; i < oldModules.Length; i++) | ||
| { | ||
| var oldMetadata = oldModules[i].GetMetadata(); | ||
| var newMetadata = newModules[i].GetMetadata(); | ||
|
|
||
| if (oldMetadata is null || newMetadata is null) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| if (oldMetadata.GetModuleVersionId() != newMetadata.GetModuleVersionId()) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
|
Comment on lines
+189
to
203
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the order of modules in an assembly stable? Or, should two assemblies be considered identical if their modules are in a different order? Or, should I be quiet and remember that multi-module assemblies are an uncommon scenario? I'm guessing your answer will be an affirmative to the third question.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Heh, AFAIK the modules should be stable, and I would assume if they had moved around, we would need to re-run discovery anyway as there are odd initialization things that can change, I think. If we were re-running when we shouldn't, that's obviously a potential perf hit, but not a correctness one at least. Generally though it's super rare to actually have multiple modules in an assembly (and I believe it's not even possible in modern .NET)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I gave you an out with my third question BTW. You could have typed way fewer words. 😁 |
||
|
|
||
| return false; | ||
| // All module MVIDs matched. | ||
| return true; | ||
| }))) | ||
| { | ||
| return false; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
grumble grumble Why does
IAssemblySymbol.Modulesreturn anIEnumerable<IModuleSymbol>when it's always backed by anImmutableArray<IModuleSymbol>? grumble grumble(no change requested)