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

FSharpEntity.IsByRef does not recognize byref`2 #876

Closed
Jand42 opened this issue Sep 12, 2018 · 3 comments
Closed

FSharpEntity.IsByRef does not recognize byref`2 #876

Jand42 opened this issue Sep 12, 2018 · 3 comments

Comments

@Jand42
Copy link
Contributor

Jand42 commented Sep 12, 2018

With FSharp.Core 4.5+, the byref type has 2 generic arguments internally. This is breaking the behavior of .IsByRef property.

Repro steps

Use ParseAndCheckProject on a project containing byref type usage, and use .IsByRef on an FSharpEntity for it.

Expected behavior

Returns true.

Actual behavior

Returns false.

Known workarounds

Use x.IsByRef || x.CompiledName = "byref`2".

Related information

Using FSharp.Compiler.Service nuget version 25.0.1.

dsyme added a commit to dsyme/fsharp that referenced this issue Sep 13, 2018
@dsyme
Copy link
Contributor

dsyme commented Sep 13, 2018

@Jand42 Proposed fix is here, please take a look dotnet/fsharp#5635

@Jand42
Copy link
Contributor Author

Jand42 commented Sep 13, 2018

@dsyme Sorry, I haven't tested right now, but my guess would be that
https://github.com/fsharp/FSharp.Compiler.Service/blob/e082581b1a4a6b71d4f55c3741679320bba3cfaf/src/fsharp/symbols/Symbols.fs#L476-L478
also needs to be changed to have

    (tyconRefEq cenv.g cenv.g.byref_tcr entity || tyconRefEq cenv.g cenv.g.v_byref2_tcr entity)

because byref`2 is the underlying type for 4.5. Thanks.

@dsyme
Copy link
Contributor

dsyme commented Sep 13, 2018

isByrefTyconRef covers that, though admittedly it also covers other cases: https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/TastOps.fs#L2938

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants