Skip to content
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

Separate interface method trimming logic and update for static interface methods #2868

Merged
merged 53 commits into from
Aug 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
da56f3d
Mark static virtual interface methods
jtschuster Jun 27, 2022
869748e
Undo IgnoreScope changes
jtschuster Jun 28, 2022
f4af27f
Add tests for virtual statics in preserve scope
jtschuster Jun 29, 2022
80ddcd2
add references folder to test output for skip assemblies
jtschuster Jun 29, 2022
deba9dd
Remove implicit implementations
jtschuster Jun 29, 2022
26fdc82
formatting
jtschuster Jun 29, 2022
d66f76c
Undo cecil and sdk changes
jtschuster Jun 29, 2022
9b9cfae
Update StaticVirtualInterfaceMethodsLibrary.cs
jtschuster Jun 30, 2022
c9c7e88
clean up
jtschuster Jun 30, 2022
8dfbdc9
formatting
jtschuster Jun 30, 2022
9076e0c
formatting
jtschuster Jul 1, 2022
32978f3
formatting
jtschuster Jul 1, 2022
fb5254a
New PreserveScope logic
jtschuster Jul 1, 2022
f522641
Better wording
jtschuster Jul 1, 2022
6a24f50
Move matrix below preserveScope check
jtschuster Jul 1, 2022
9a5fd73
Run dotetn-format latest version
jtschuster Jul 2, 2022
471af7b
whip, switching machines
jtschuster Jul 6, 2022
57e7ea9
Merge remote-tracking branch 'upstream/main' into wip
jtschuster Jul 7, 2022
e6e69a7
Wip
jtschuster Jul 7, 2022
a446e6f
Track interface overrides differently than virtual methods
jtschuster Jul 12, 2022
50f2664
Merge remote-tracking branch 'upstream/main' into markStaticVirtuals
jtschuster Jul 12, 2022
f5502c9
formatting
jtschuster Jul 12, 2022
182b293
clean up after merge
jtschuster Jul 12, 2022
882a3ea
Remove generated tests
jtschuster Jul 12, 2022
602f015
Undo test change from another issue
jtschuster Jul 12, 2022
ff46aed
Add comments to IsInterfaceMethodNeededByTypeDueToInterface
jtschuster Jul 12, 2022
0a40b0b
formatting
jtschuster Jul 12, 2022
dab83a9
remove using
jtschuster Jul 13, 2022
267b080
Use HashSet and add comment
jtschuster Jul 13, 2022
0ce04c7
Remove unnecessary check
jtschuster Jul 13, 2022
fb31da8
Style changes
jtschuster Jul 14, 2022
f2e53ae
Remove RootAssemblyMode check
jtschuster Jul 15, 2022
e554231
Add docs
jtschuster Jul 15, 2022
840b2a4
Update docs/optimizations.md
jtschuster Jul 25, 2022
59a2cc3
PR Feedback and test for virtual base type methods with default imple…
jtschuster Jul 26, 2022
2c6a512
Remove check for unusedinterfaces optimization
jtschuster Jul 26, 2022
e4ece71
Move interface PreservedScope logic to single function
jtschuster Jul 27, 2022
12486ee
Move interface PreservedScope logic to single function
jtschuster Jul 27, 2022
fe409cb
Merge branch 'markStaticVirtuals' of https://github.com/jtschuster/li…
jtschuster Jul 27, 2022
9fa904f
Update MarkStep.cs
jtschuster Jul 28, 2022
0960f91
wip
jtschuster Jul 29, 2022
513ec3a
Update src/linker/Linker.Steps/MarkStep.cs
jtschuster Jul 29, 2022
5ad5c34
Update docs/methods-kept-by-interface.md
jtschuster Jul 29, 2022
a20ef79
Update src/linker/Linker.Steps/MarkStep.cs
jtschuster Jul 29, 2022
4a2a4dc
Merge remote-tracking branch 'origin/markStaticVirtuals' into markSta…
jtschuster Jul 29, 2022
0d6cef7
PR Feedback
jtschuster Jul 29, 2022
8bd3d7c
Formatting and link PR to commented ExpectedWarning
jtschuster Jul 29, 2022
b9ac22c
Add test for base provides static interface method
jtschuster Jul 31, 2022
f9f4c38
Remove additions from another branch
jtschuster Aug 1, 2022
1d0bddc
Remove commented code
jtschuster Aug 1, 2022
4f1252d
formatting
jtschuster Aug 1, 2022
47cb5da
Merge branch 'main' into markStaticVirtuals
jtschuster Aug 1, 2022
cc870cb
Update docs
jtschuster Aug 3, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 106 additions & 0 deletions docs/methods-kept-by-interface.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
# Interface Implementation Methods Marking
#### (Does this method need to be kept due to the interface method it overrides)

The following behavior is expected for interface methods. This logic could be used to begin marking and sweeping the `.Override` of a method since if the method isn't a dependency due to the interface/base type, we should be able to remove the methodImpl. Right now, the methodImpl is always kept if both the interface method and overriding method is kept, but that isn't always necessary.

Whether or not a method implementing an interface method is required due to the _interface_ is affected by the following cases / possibilities (the method could still be kept for other reasons):
- Base method is abstract or has a default implementation (`abstract` vs `virtual` in C#)
- Method is Instance or Static
- Implementing type is relevant to variant casting or not
- Relevant to variant casting means the type token appears, the type is passed as a type argument or array type, or is reflected over.
- Base method is marked as used or not
- Base method is from preserved scope or not
- Implementing type is marked as instantiated or not
- Interface Implementation is marked or not

Note that in library mode, interface methods that can be accessed by COM or native code are marked by the linker.

### If the interface implementation is not marked, do not mark the implementation method
A type that doesn't implement the interface isn't required to have methods that implement the interface. However, a base type may have a public method that implements an interface on a derived type. If the interface implementation on the derived type is marked, then the method may be needed and we should go onto the next step.

Cases left (bold means we know it is only one of the possible options now):
- Base method is abstract or has a default implementation
- Method is Instance or Static
- Implementing type is relevant to variant casting or not
- Base method is marked as used or not
- Base method from preserved scope or not
- Implementing type is marked as instantiated or not
- __Interface Implementation is marked__

### If the interface method is not marked and the interface doesn't come from a preserved scope, do not mark the implementation method
Unmarked interface methods from `link` assemblies will be removed so the implementing method does not need to be kept.

Cases left:
- Base method is abstract or has a default implementation
- Method is Instance or Static
- Implementing type is relevant to variant casting or not
- ~~Base method is marked as used or not~~
- ~~Base method from preserved scope or not~~
- _Base method is either marked as used or from preserved scope (combine above)_
- Implementing type is marked as instantiated or not
- __Interface Implementation is marked__

### If the interface method is abstract, mark the implementation method
The method is needed for valid IL.

Cases left:
- __Base method has a default implementation__
- Method is Instance or Static
- Implementing type is relevant to variant casting or not
- Base method is marked as used or from preserved scope
- Implementing type is marked as instantiated or not
- __Interface Implementation is marked__

### If the method is an instance method then mark the implementation method if the type is instantiated (or instantiable in library mode) and do not mark the implementation otherwise.
An application can call the instance interface method if and only if the type is instantiated.

Cases left:
- __Base method has a default implementation__
- __Method is Static__
- Implementing type is relevant to variant casting or not
- Base method is marked as used or from preserved scope
- Implementing type is marked as instantiated or not
- __Interface Implementation is marked__

The use of static methods is not related to whether or not a type is instantiated or not.

Cases left:
- __Base method has a default implementation__
- __Method is Static__
- Implementing type is relevant to variant casting or not
- Base method is marked as used or from preserved scope
- __Interface Implementation is marked__

### If the implementing type is relevant to variant casting, mark the implementation method.
A static method may only be called through a constrained call if the type is relevant to variant casting.

Cases left:
- __Base method has a default implementation__
- __Method is Static__
- __Implementing type is not relevant to variant casting__
- Base method is marked as used or from preserved scope
- __Interface Implementation is marked__

### If the interface method is in a preserved scope, mark the implementation method.
We assume the implementing type could be relevant to variant casting in the preserved scope assembly and could be called, so we will keep the method.

### Otherwise, do not mark the implementing method


Summary:

if __Interface Implementation is not marked__ then do not mark the implementation method.

else if __Base method is marked as not used__ AND __Interface is not from preserved scope__ do not mark the implementation method

else if __Base method does not have a default implementation__ then mark the implementation method

else if __Implementation method is an instance method__ AND __Implementing type is instantiated__ then mark the implementation method

else if __Implementation method is an instance method__ AND __Implementing type is not instantiated__ then do not mark the implementation method

else if __Method is Static__ AND __Implementing type is relevant to variant casting__ then mark the implementation method

else if __Method is Static__ AND __Interface method is from a preserved scope__ then mark the implementation method

else do not mark the implementation method
5 changes: 5 additions & 0 deletions docs/optimizations.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Optimizations definitions

## `unusedinterfaces` optimization

The `unusedinterfaces` optimization controls whether or not trimming may remove the `interfaceimpl` annotation which denotes whether a class implements an interface. When the optimization is off, the linker will not remove the annotations regardless of the visibility of the interface (even private interface implementations will be kept). However, unused methods from interfaces may still be removed, as well as `.override` directives from methods that implement an interface method. When the optimization is on and the linker can provably determine that an interface will not be used on a type, the annotation will be removed. In order to know whether it is safe to remove an interface implementation, it is necessary to have a "global" view of the code. In other words, if an assembly is passed without selecting `link` for the `action` option for the linker, all classes that implement interfaces from that assembly will keep those interface implementation annotations. For example, if you implement `System.IFormattable` from the `System.Runtime` assembly, but pass the assembly with `--action copy System.Runtime`, the interface implementation will be kept even if your code doesn't use it.
71 changes: 0 additions & 71 deletions docs/removal-behavior.md

This file was deleted.

2 changes: 1 addition & 1 deletion eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
<MicrosoftDotNetApiCompatVersion>7.0.0-beta.22372.1</MicrosoftDotNetApiCompatVersion>
<MicrosoftDotNetCodeAnalysisVersion>6.0.0-beta.21271.1</MicrosoftDotNetCodeAnalysisVersion>
<MicrosoftCodeAnalysisCSharpCodeStyleVersion>3.10.0-2.final</MicrosoftCodeAnalysisCSharpCodeStyleVersion>
<MicrosoftCodeAnalysisVersion>4.3.0-1.22206.2</MicrosoftCodeAnalysisVersion>
<MicrosoftCodeAnalysisVersion>4.3.0-2.final</MicrosoftCodeAnalysisVersion>
<MicrosoftCodeAnalysisCSharpAnalyzerTestingXunitVersion>1.0.1-beta1.*</MicrosoftCodeAnalysisCSharpAnalyzerTestingXunitVersion>
<MicrosoftCodeAnalysisBannedApiAnalyzersVersion>3.3.2</MicrosoftCodeAnalysisBannedApiAnalyzersVersion>
<!-- This controls the version of the cecil package, or the version of cecil in the project graph
Expand Down
Loading