[roslyn] CustomModifiersTests.ConcatModifiers_03 does not work on Mono #10564
Assignees
Comments
|
Standalone repro gist https://gist.github.com/lambdageek/982e57262a8dffe3e19fda6141a67e11 This appears to be passing with Output is: (and monodis of the repro .exe shows the expected modopts on the property - but that part isn't really mono-related - just means Roslyn is working correctly) |
|
Hmm. Actually now I'm not so sure.
I think we need to actually understand what the roslyn test is doing - I'm not sure where serialization comes into it at all. |
2 tasks
|
This is passing with #13320. |
marek-safar
pushed a commit
that referenced
this issue
Mar 14, 2019
…3320) This all stems from ECMA 335, 7.1.1: > The CLI itself shall treat required and optional modifiers in the same > manner. Two signatures that differ only by the addition of a custom > modifier (required or optional) shall not be considered to match. Custom > modifiers have no other effect on the operation of the VES. This rule means that when comparing signatures, we have to treat two types that differ only in their custom modifiers as distinct. In particular, we have to do this when comparing signatures in order to fill in the vtable for a class. And moreover, we have to concatenate custom modifiers when doing generic instantiation. If a method in a generic type has signature `T modopt(IsConst) Method()` and we instantiate T with `int32 modopt(IsLong)`, we are expected to produce the signature `int32 modopt(IsConst) modopt(IsLong) Method()` (with the custom modifiers on `int32` in that order). So this PR implements the following operations: 1. custom modifier appending during `mono_metadata_type_dup_with_cmods` 2. custom modifier-respecting comparison in `do_mono_metadata_type_equal` To do this, we actually have to introduce a second representation of custom modifiers: in the normal representation, all the cmod type specs come from a single image. Whereas now, due to inflation of signatures, we can have a mix of modifiers from different images. So in the MonoAggregateModContainer representation, we pair each typespec token with the image in which it is to be resolved. We keep the old representation (a single image to resolve the entire set of typespec tokens) because it's natural when you're just reading from a single image. Also this representation is part of the embedding API. --- ### But that's not all ### It is possible for generic parameters of a generic class to appear in the _custom modifiers_ of a type in a method signature. For example: ``` T modopt(Nullable`1<T>) Method () ``` In this case, when we inflate this signature, we need to inflate not only the return type, but also the modifiers on a return type. That means our old aggregate representation (where each modifier is a pair of a MonoImage and a typespec token) is inadequate: we can inflate a signature and get a custom modifier that doesn't appear as a typespec in any table of any existing image. This is the same situation as generic instances: as a program runs, it creates generic instances that are not part of the source code. So we solve it the same way as with generic instances. Our new `MonoAggregateModContainer` representation is a pointer into memory owned by a MonoImageSet that contains an entire MonoType for each custom modifier. The invariant is that the aggregate modifier container is allocated from a mempool of a `MonoImageSet` that includes the images of all the modifiers. In general, when inflating, we first create a provisional candidate MonoAggregateModContainer on the stack and then look it up the image set that should own it and then look for a "canonical" mod container in the `MonoImageSet:aggergate_modifiers_cache` --- Also had to bump the AOT image format, which now always reads back in using the aggregate representation. --- This should address #10564, #10565, #12422, #10945 (again), #6936, and the parts of #11350 that **don't** deal with arrays. --- TODO: - [x] Add some representative test cases to this PR. - [ ] Modify our acceptance tests to run the various disabled Roslyn tests.
picenka21
pushed a commit
to picenka21/runtime
that referenced
this issue
Feb 18, 2022
…no/mono#13320) This all stems from ECMA 335, 7.1.1: > The CLI itself shall treat required and optional modifiers in the same > manner. Two signatures that differ only by the addition of a custom > modifier (required or optional) shall not be considered to match. Custom > modifiers have no other effect on the operation of the VES. This rule means that when comparing signatures, we have to treat two types that differ only in their custom modifiers as distinct. In particular, we have to do this when comparing signatures in order to fill in the vtable for a class. And moreover, we have to concatenate custom modifiers when doing generic instantiation. If a method in a generic type has signature `T modopt(IsConst) Method()` and we instantiate T with `int32 modopt(IsLong)`, we are expected to produce the signature `int32 modopt(IsConst) modopt(IsLong) Method()` (with the custom modifiers on `int32` in that order). So this PR implements the following operations: 1. custom modifier appending during `mono_metadata_type_dup_with_cmods` 2. custom modifier-respecting comparison in `do_mono_metadata_type_equal` To do this, we actually have to introduce a second representation of custom modifiers: in the normal representation, all the cmod type specs come from a single image. Whereas now, due to inflation of signatures, we can have a mix of modifiers from different images. So in the MonoAggregateModContainer representation, we pair each typespec token with the image in which it is to be resolved. We keep the old representation (a single image to resolve the entire set of typespec tokens) because it's natural when you're just reading from a single image. Also this representation is part of the embedding API. --- ### But that's not all ### It is possible for generic parameters of a generic class to appear in the _custom modifiers_ of a type in a method signature. For example: ``` T modopt(Nullable`1<T>) Method () ``` In this case, when we inflate this signature, we need to inflate not only the return type, but also the modifiers on a return type. That means our old aggregate representation (where each modifier is a pair of a MonoImage and a typespec token) is inadequate: we can inflate a signature and get a custom modifier that doesn't appear as a typespec in any table of any existing image. This is the same situation as generic instances: as a program runs, it creates generic instances that are not part of the source code. So we solve it the same way as with generic instances. Our new `MonoAggregateModContainer` representation is a pointer into memory owned by a MonoImageSet that contains an entire MonoType for each custom modifier. The invariant is that the aggregate modifier container is allocated from a mempool of a `MonoImageSet` that includes the images of all the modifiers. In general, when inflating, we first create a provisional candidate MonoAggregateModContainer on the stack and then look it up the image set that should own it and then look for a "canonical" mod container in the `MonoImageSet:aggergate_modifiers_cache` --- Also had to bump the AOT image format, which now always reads back in using the aggregate representation. --- This should address mono/mono#10564, mono/mono#10565, mono/mono#12422, mono/mono#10945 (again), mono/mono#6936, and the parts of mono/mono#11350 that **don't** deal with arrays. --- TODO: - [x] Add some representative test cases to this PR. - [ ] Modify our acceptance tests to run the various disabled Roslyn tests. Commit migrated from mono/mono@c0b4879
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The text was updated successfully, but these errors were encountered: