fix(core): skip rewriting CoreCLR netstandard facades - #102
Conversation
…bject. Co-authored-by: Cursor <cursoragent@cursor.com>
mcpolo99
left a comment
There was a problem hiding this comment.
Reviewed the change and ran the full local CI with it rebased onto current develop (which now includes the #101 merge).
Local CI: all phases passed — lint, build (incl. C++/CLI), 189 tests passed / 0 failed / 1 skipped (the skip is the UI-interactive Confuser.GUI.Test), and packaging.
Regression paths for this change verified green:
CrossFramework.Library.NetStd20(netstandard2.0 library) builds + covered — the case the second checklist item was aboutCompressorWithResx.Test(12),VisualBasicRenamingResx.Test(1),389_MixedCultureCasing.Test(1)
The fix is correct and it also removes a latent NullReferenceException (the old subAss?.Modules foreach would throw when a ref failed to resolve). One nit only: the added comments are heavier than needed — the DefinesSystemObject helper already documents intent. Suggested collapse to a single PR reference below.
| // Classic netstandard facades forward to mscorlib, which actually defines the types. | ||
| // Moving those types in keeps the confused module referencing netstandard only. | ||
| // CoreCLR publish output is different: netstandard.dll only forwards to System.Runtime, | ||
| // and that assembly forwards again to System.Private.CoreLib. Clearing the forwards | ||
| // there hides System.Object and breaks later analysis. |
There was a problem hiding this comment.
Collapse this block to a one-line reference; the code below is self-describing:
| // Classic netstandard facades forward to mscorlib, which actually defines the types. | |
| // Moving those types in keeps the confused module referencing netstandard only. | |
| // CoreCLR publish output is different: netstandard.dll only forwards to System.Runtime, | |
| // and that assembly forwards again to System.Private.CoreLib. Clearing the forwards | |
| // there hides System.Object and breaks later analysis. | |
| // https://github.com/mcpolo99/ConfuserExx/pull/102 |
| // Only rewrite when a referenced assembly actually defines System.Object | ||
| // (the mscorlib case). CoreCLR facades only forward it onward. |
There was a problem hiding this comment.
DefinesSystemObject + the early return already say this; drop the comment:
| // Only rewrite when a referenced assembly actually defines System.Object | |
| // (the mscorlib case). CoreCLR facades only forward it onward. |
|
|
||
| // Remove them because their types has been removed. | ||
| foreach (var subAss in allAssemblyRefs) { | ||
| // Their types now live on netstandard, so they must not stay cached. |
There was a problem hiding this comment.
Drop; Remove(...) is clear enough:
| // Their types now live on netstandard, so they must not stay cached. |
Summary
etstandard.dll that only type-forwards to System.Runtime / System.Private.CoreLib.
Test plan
Made with Cursor