ApiCompat: errors when using 'in' modifier

[steveharter]

I am modifying System.Text.Json to expose public APIs using the in modifier to support feature https://github.com/dotnet/corefx/issues/34372

APICompat is failing with several errors like this:

error : ParameterModifiersCannotChange : Custom modifiers on parameter 'reader' on method 'System.Text.Json.Serialization.JsonTypeSerializer<TType>.OnDeserialized(TType, System.Text.Json.Serialization.JsonClassInfo<TType>, System.Text.Json.Utf8JsonReader, System.Text.Json.Serialization.JsonSerializerOptions, System.Collections.Generic.IDictionary<System.String, System.Object>)' are 'System.Runtime.InteropServices.InAttribute' in the implementation but 'System.Runtime.InteropServices.InAttribute' in the contract. [D:\git\corefx2\src\System.Text.Json\src\System.Text.Json.csproj]

From looking at the IL, the ref's in modifier attribute looks like this:
[System.Runtime]System.Runtime.InteropServices.InAttribute
while the implementation IL looks likes this:
[System.Private.CoreLib]System.Runtime.InteropServices.InAttribute

Which is causing ApiCompat to fail here

[ViktorHofer]

cc @ericstj @wtgodbe

[ericstj]

Dealing with types living in different assemblies and following typeforwards is basic functionality of APICompat, however it's possible that one rule like this that's rarely excercised has a bug.

Are you sure you're referencing the System.Runtime facade from your implementation assembly so that API compat can see the typeforward for InAttribute?

Moreover, if this is System.Text.Json I wouldn't expect it to be using System.Private.CoreLib at all, since that was removed here: dotnet/corefx@9074d9f

[ericstj]

@steveharter are you still reproducing this? I checked at the implementation of IsIn in CCI and its actually encoded as a specific bit in the Parameter Table. Indeed if you check ECMA 335 section II.21.2.1 it's documented as a psuedo custom attribute InAttribute: sets the Param.Flags.In bit for the attributed parameter. . So if there is an issue here it may actually be a problem in the C# compiler. /cc @jaredpar

[tannergooding]

Is the method you are using in on virtual? In that case, it sets the in bit and also emits a modreq: https://sharplab.io/#v2:EYLgtghgzgLgpgJwDQxASwDYB8ACAGAAhwEYBuAWACgcBmIgJgIGECBvKgzounAFgICyACjQA7AmJgSAlGw5cAvvM7LuBAG5oEMAK4QMRfgDkR4yTLmUuBJZQVA=

[jaredpar]

@ericstj you're right and wrong here 😉

When you use a in parameter the compiler will mark metadata in up to two different ways:

1. The parameter will have the pseudo custom attribute InAttribute set as you indicated.
2. The parameter will receive a modreq for 'System.Runtime.InteropServices.InAttribute when the containing method is any virtual / defined in an interface

[ericstj]

Taking a closer look, I see this is actually failing in a place where it was already supposed to have checked in/out. It's in Custom modifiers checks. I think this is the same bug as #2136 where APICompat was complaining about custom modifier differences on identical contract/implementation (for C++/CLI generated modifiers).

[steveharter]

@ericstj those changes have been put on hold; so I'm not sure if the compat error is still repro'ing or not.

[ericstj]

Got it, I think I understand enough about what's going on to create a repro and I think this is related to another issue we already have a handle on. I'll see if I can fix it.