Fixes: #4957 --- Intellisense for Indexed properties were broken by a recent #4958
Conversation
src/fsharp/infos.fs
Outdated
| match x with | ||
| | ILMeth(_,ilminfo,_) -> ilminfo.ApparentEnclosingAppType | ||
| | _ -> x.ApparentEnclosingType | ||
| helpEnsureTypeHasMetadata x.TcGlobals x.ApparentEnclosingType |
There was a problem hiding this comment.
Not directly related to this change, but was exactly does "help ensure" mean? Can you please rename it to just "ensure"?
There was a problem hiding this comment.
@forki It doesn't guarantee to convert to a type with metadata, but for function and tuple types it does. So it's something like convertToTypeWithMetadataIfPossible.
| | ILMeth(_,ilmeth,_) -> ilmeth.FormalMethodTypars | ||
| | FSMeth(g,typ,vref,_) -> | ||
| | FSMeth(g,_,vref,_) -> | ||
| let typ = x.ApparentEnclosingAppType |
There was a problem hiding this comment.
Ugh, yes. However there are a few other paths to AnalyzeTypeOfMemberVal, which is assuming that typ is an AppTy, e.g. via GetCompiledReturnTy and GetParamTypes which I think will also fail. I'm surprised we're not hitting those
There was a problem hiding this comment.
@dsyme, yes I was going to look through those today. The trick is figuring out the repro, as you say, with all this code base none hit it.
|
Then I vote to use that name
Don Syme <[email protected]> schrieb am Mo., 21. Mai 2018, 13:15:
… ***@***.**** commented on this pull request.
------------------------------
In src/fsharp/infos.fs
<#4958 (comment)>
:
> @@ -897,9 +897,7 @@ type MethInfo =
/// Get the enclosing type of the method info, using a nominal type for tuple types
member x.ApparentEnclosingAppType =
- match x with
- | ILMeth(_,ilminfo,_) -> ilminfo.ApparentEnclosingAppType
- | _ -> x.ApparentEnclosingType
+ helpEnsureTypeHasMetadata x.TcGlobals x.ApparentEnclosingType
@forki <https://github.com/forki> It doesn't guarantee to convert to a
type with metadata, but for function and tuple types it does. So it's
something like convertToTypeWithMetadataIfPossible.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4958 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADgNOiOZtU507FXIwWn3493qYlmFN6Mks5t0qHtgaJpZM4UGb2P>
.
|
|
@dsyme, can you take a look at the update changes, I think I have the right paths covered now. Although, I couldn't write any failing repros. |
|
That looks good. |
3 participants
Uh oh!
There was an error while loading. Please reload this page.