Add support for single file analysis #1691
Conversation
-Add test for single file dangerous patterns
| // CodeBase () | ||
| // | ||
| case IntrinsicId.Assembly_CodeBase when _context.SingleFileAnalysis == true: | ||
| _context.LogWarning ($"'{calledMethodDefinition.GetDisplayName ()}' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'.", 3000, callingMethodDefinition); |
There was a problem hiding this comment.
This actually throws, not return an empty string. I think we should add new warnings for this and other APIs (other than Assembly.Location and Assembly.GetFiles).
There was a problem hiding this comment.
And now that we have these warnings being produced by the linker, we should probably add them to the docs and close #1498.
| @@ -106,6 +107,11 @@ public class LinkContext : IDisposable | |||
| set { _keepTypeForwarderOnlyAssemblies = value; } | |||
| } | |||
|
|
|||
| public bool SingleFileAnalysis { | |||
There was a problem hiding this comment.
nit: similar methods in LinkContext tend to be named IsSomething, maybe name this IsSingleFile?
There was a problem hiding this comment.
Also, why expand the getter/settter.
public bool SingleFileAnalysis { get; set; }
| namespace Mono.Linker.Tests.Cases.SingleFileAnalysis | ||
| { | ||
| [SetupLinkerArgument ("--single-file-analysis", "true")] | ||
| public class WarnOnSingleFileDangerousPatterns |
There was a problem hiding this comment.
If we move this tests here, can we remove them from ILLink.RoslynAnalyzer.Tests and call them in the analyzer with RunTest? Similar to what we already have for RequiresCapability tests.
| @@ -246,6 +246,11 @@ enum IntrinsicId | |||
| AppDomain_CreateInstanceFrom, | |||
| AppDomain_CreateInstanceFromAndUnwrap, | |||
| Assembly_CreateInstance, | |||
| Assembly_Location, | |||
There was a problem hiding this comment.
Should we also add AssemblyName_CodeBase, AssemblyName_EscapedCodeBase, Module_Name, and Module_FullyQualifiedName?
|
What's the value prop for introducing intrinsics for this vs just marking these as SingleFileUnsupported in CoreLib (dotnet/runtime#45705) and have linker understand that attribute? I think the latter will leave us with less code to maintain and I tend to prefer that. |
There was a problem hiding this comment.
I think this is a wrong approach for several reasons
- it forces the linker to be always used for single-file analysis
- it hardcodes implementation which could change or evolve
- it ignores runtime pack variations
- it sets the wrong precedent for all other framework APIs that have a similar problem
|
Closing in favor of #1722 |
This change allows the linker to produce warnings using the ReflectionMethodBodyScanner when the flag --single-file-analysis is used. On this first PR is only allowed to print single file warnings only if trimming is also enabled. Future PR should pursue the end goal of enabling linker analysis without trimming, so we could print single file warnings even when the app is not going to be trimmed
-Add flag for single file analysis in the linker
-Add single file dangerous patterns to the ReflectionMethodBodyScanner list
-Add test for single file dangerous patterns