IDE0044 (Add readonly modifier) false positive

[Youssef1313]

UPDATED:

roslyn/src/Compilers/VisualBasic/Portable/Symbols/Source/OverrideHidingHelper.vb

Lines 584 to 586 in eb26ee3

	#Disable Warning IDE0044 ' Add readonly modifier - Adding readonly generates compile error in the constructor. - see https://github.com/dotnet/roslyn/issues/47197
	Private Shared s_runtimeSignatureComparer As IEqualityComparer(Of TSymbol)
	#Enable Warning IDE0044 ' Add readonly modifier

IDE0044 shouldn't have been reported in this case. (See #47197 (comment))

[Youssef1313]

@AlekseyTs Would you be able to give a look here?

[AlekseyTs]

Would you be able to give a look here?

OverrideHidingHelper(Of MethodSymbol) is not OverrideHidingHelper(Of TSymbol), therefore constructor of OverrideHidingHelper(Of TSymbol) shouldn't be able to assign to readonly fields of OverrideHidingHelper(Of MethodSymbol).

[Youssef1313]

@AlekseyTs Thanks. Can you switch to Area-IDE to fix the false positive of IDE0044?

[CyrusNajmabadi]

@Youssef1313 I'm not clear what the issue is that you are reporting. Can you clarify?

[Youssef1313]

@CyrusNajmabadi Simply, see the suppression in:

roslyn/src/Compilers/VisualBasic/Portable/Symbols/Source/OverrideHidingHelper.vb

Lines 584 to 586 in eb26ee3

	#Disable Warning IDE0044 ' Add readonly modifier - Adding readonly generates compile error in the constructor. - see https://github.com/dotnet/roslyn/issues/47197
	Private Shared s_runtimeSignatureComparer As IEqualityComparer(Of TSymbol)
	#Enable Warning IDE0044 ' Add readonly modifier

IDE0044 shouldn't have been reported in this case based on @AlekseyTs comment.

[CyrusNajmabadi]

Could you flesh out the OP more. It wasn't clear waht this was talkign about, and it needed many jumps to otehr bugs and PRs to even get a sense of what might be going on.

[Youssef1313]

@CyrusNajmabadi Sure. Done.

[CyrusNajmabadi]

I can't seem to repro this issue in a small example: Sharplab

Compiler seems fine with:

OverrideHidingHelper(Of MethodSymbol) is not OverrideHidingHelper(Of TSymbol), therefore constructor of OverrideHidingHelper(Of TSymbol) shouldn't be able to assign to readonly fields of OverrideHidingHelper(Of MethodSymbol).

I do get the error if i make the constructor non-shared. But in the original example, the constructor is shared, so i'm not sure why there's a problem inside roslyn, but not in the repro case.

[CyrusNajmabadi]

@AlekseyTs do you know why teh above sharplab link shows no issues? i can't figure out the importance difference between it, and the code actually in OverrideHidingHelper. Thanks!

[AlekseyTs]

do you know why teh above sharplab link shows no issues?

Is SharpLab supposed to run analyzers?

[CyrusNajmabadi]

Is SharpLab supposed to run analyzers?

No. :)

What i was trying to show in my sharplab example was that i get no error for assigning to this readonly field in the trimmed down code example. But if you make the field readonly in roslyn itself, you get:

src\Compilers\VisualBasic\Portable\Symbols\Source\OverrideHidingHelper.vb(578,13): error BC30064: 'ReadOnly' variable cannot be the target of an assignment.

What i cannot figure out is how the code in roslyn is meaningfully different from teh sample code. Clearly in roslyn itself something about the code is triggering the error, i just cannot tell what it is. I thought it was the assignment to the field through a different instantiation. But clearly that's ok in sharplab. So it's currently unclear to me what's up :)

[AlekseyTs]

By default, we duplicate a bug in native compiler. There is a switch to overcome that, vbc /t:library /features:strict :

Friend Class OverrideHidingHelper(Of TSymbol)

    Private Shared ReadOnly s_runtimeSignatureComparer As Integer

    Shared Sub New()
        OverrideHidingHelper(Of Integer).s_runtimeSignatureComparer = 1
    End Sub
    
End Class

Observed:

d:\delete\debug.vb(6) : error BC30064: 'ReadOnly' variable cannot be the target of an assignment.

        OverrideHidingHelper(Of Integer).s_runtimeSignatureComparer = 1
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[AlekseyTs]

Obviously we compile compiler with the "strict" flag

[CyrusNajmabadi]

Basically, why is this legal in sharplab:

But this is not in roslyn:

I'm def missing something and could use some vb compiler/lang experience here :)

[CyrusNajmabadi]

oh.... it's a strict "feature flag" thing. Interesting.

Thanks very much for the clarification. I def didn't realize that.

[CyrusNajmabadi]

Ignore my above post. You answered it :)