Some of NativeAOT tests are disabled after sharing test cases with ILLink #82447
Comments
ghost
added
the
|
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas Issue DetailsWith #82216 we started to share the test cases between ILLink and NativeAOT this allows us to delete duplicated code and keep tests consistent, given that a change in one of the tests will affect both codebases. Not all the ILLink tests apply to NativeAOT and that's why NativeAOT only enables a subset of them in its TestDatabase file, the test database file enables testing in a folder fashion, a folder is assigned to a feature being tested. But even in these features NativeAOT has not triaged/enabled all of the tests, and some of the tests inside a feature might not apply to NativeAOT.
|
vitek-karas
added a commit
that referenced
this issue
Mar 16, 2023
Implements most of the missing pieces to get Requires on class working correctly in NativeAOT. Major changes: * Detect Requires mismatch between derived and base class * Warn on field access if the owning class has Requires * Changes to reflection marking to warn on more cases (instance methods on Requires classes for example) Supportive changes: * The helpers to detect Requires attributes now return the found attribute view out parameter Fixes #81158 Still two missing pieces - tracked by #82447: * Requires on attributes - NativeAOT doesn't handle this at all yet, part of it is Requires on the attribute class * Avoid warning when DAM marking an override method which has Requires (or its class has) - this avoids lot of noise, NativeAOT currently generates these warnings in full
vitek-karas
added a commit
that referenced
this issue
Apr 4, 2023
This brings the NativeAOT behavior around static cctor analysis on part with illink. Main change is to add checks for RUC/RAF/RDC on static cctor and produces a warning if there is one (as these are not allowed on static cctor). The rest of the changes are just minor fixes and adjusting tests to the new behavior. Contributes to #82447.
vitek-karas
added a commit
that referenced
this issue
Apr 24, 2023
…s warnings between ILLink and NativeAOT (#84770) This enables the last of the disabled `RequiresCapability` tests for NativeAOT. The implementation basically copies the logic from ILLink on how to produce warnings when method or field is marked due to reflection access. We decided that producing warnings due to reflection access to compiler generated methods is more trouble than benefit and thus we won't do that anymore. See reasoning in #85042. Otherwise the behavior in NativeAOT fully copies the one in ILLink (small exceptions due to better method overload resolution in NativeAOT). High-level behavior changes in NativeAOT: * Finally allow the NativeAOT compiler to produce IL2110, IL2111, IL2113 and IL2115 * Avoid generating warnings for override methods if the warning is already generated on the base method (for DAM marking) * Implement correct marking (and warnings) for "DAM on type" when the derived class has broader annotation than the base class/interface (this is mostly to avoid noise warnings) * Rename a set of methods and classes in data flow scanner to "Token", because they're only used when a given entity is accessed through its token (so `ldtoken` and similar). This allows for more precise handling of the cases above. Testing changes: * Allow specifying tool exceptions for `SkipKepItemsValidationAttribute` - this allows to enable one of the reflection tests for its diagnostics coverage, avoiding the necessity to fix everything around Kept behavior yet. * Couple of fixes in the test validator to correctly compare expected/actual diagnostics This fixes final bits of #68786 Fixes #68688 This helps with #82447
|
@vitek-karas is there something we need for 8.0 or can we move this to 9.0? |
|
No - moved it to 9.0. |
42 tasks
MichalStrehovsky
added a commit
to MichalStrehovsky/runtime
that referenced
this issue
Nov 25, 2024
When I was working on the `DAMT.All*`, I noticed we don't run many Reflection tests, this is progress towards that. Also fixes rare `CreateInstance` overloads - these were never worth the effort to implement honestly, but more code sharing made implementing them a breeze. Contributes to dotnet#82447.
MichalStrehovsky
added a commit
that referenced
this issue
Dec 3, 2024
When I was working on the `DAMT.All*`, I noticed we don't run many Reflection tests, this is progress towards that. Also fixes rare `CreateInstance` overloads - these were never worth the effort to implement honestly, but more code sharing made implementing them a breeze. Contributes to #82447.
MichalStrehovsky
added a commit
to MichalStrehovsky/runtime
that referenced
this issue
Dec 3, 2024
Progress towards dotnet#82447.
eduardo-vp
pushed a commit
to eduardo-vp/runtime
that referenced
this issue
Dec 5, 2024
When I was working on the `DAMT.All*`, I noticed we don't run many Reflection tests, this is progress towards that. Also fixes rare `CreateInstance` overloads - these were never worth the effort to implement honestly, but more code sharing made implementing them a breeze. Contributes to dotnet#82447.
mikelle-rogers
pushed a commit
to mikelle-rogers/runtime
that referenced
this issue
Dec 10, 2024
When I was working on the `DAMT.All*`, I noticed we don't run many Reflection tests, this is progress towards that. Also fixes rare `CreateInstance` overloads - these were never worth the effort to implement honestly, but more code sharing made implementing them a breeze. Contributes to dotnet#82447.
hez2010
pushed a commit
to hez2010/runtime
that referenced
this issue
Dec 14, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
With #82216 we started to share the test cases between ILLink and NativeAOT this allows us to delete duplicated code and keep tests consistent, given that a change in one of the tests will affect both codebases. Not all the ILLink tests apply to NativeAOT and that's why NativeAOT only enables a subset of them in its TestDatabase file, the test database file enables testing in a folder fashion, a folder is assigned to a feature being tested. But even in these features NativeAOT has not triaged/enabled all of the tests, and some of the tests inside a feature might not apply to NativeAOT.
For these reasons we are disabling the test that have not been triaged inside a feature until they are triaged for:
Incomplete list of features which are still missing/tests:
Requires*on attributesRequiresOnAttributeCtor.cs,RequiresOnAttribute.csparts ofRequiresOnClass.csRequires*on a virtual method (directly or indirectly) causes warnings in NativeAOT, but in trimmer we rely on hierarchy checks and only warn on the base method (not on the override when marked via DAM).ReflectionAccessFromCompilerGeneratedCode.csis completely disabled for NativeAOTRequiresOnStaticConstructors.csis completely disabled for NativeAOTRequiresOnVirtualsAndInterfaces.csis completely disabled for NativeAOTActivator.CreateInstancedoesn't parse the string argument and instead always producesIL2032UnconditionalSuppressMessagevia XML is not supported in NativeAOTLocalDataFlowtests - these test various aspects of local data flow which are slightly off between the two compilers. Also NativeAOT in debug mode (running just the JIT) will produce very different results because it would be effectively testing JIT's behavior. Probably not a goal to fix this.IReflectDataFlowthis test validates a lot ofKeptstuff, but NativeAOT seems to handle it very differently from ILLink. Probably needs some more investigation.The text was updated successfully, but these errors were encountered: