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

Fixes #9, Internal error in FSI: FS0192: binding null type in envBindTyp... #250

Closed
wants to merge 4 commits into from
Closed

Conversation

KevinRansom
Copy link
Member

Fixes #9, internal error: binding null type in envBindTypeRef
Fixes #10, internal error: binding null type in envBindTypeRef

Both issues were caused by the same underlying issue. RefEmit automagically applies escaping to names that
contain a ',' So , becomes \,.

When we tried to create a ref to a nested type we didn't add the ',' escaping so we couldnot bind to the created type.

The fix is very straightforward, change BasicQualifiedName to correctly escape names with comma's.

…TypeRef #9

Fixes #10, internal error: binding null type in envBindTypeRef

Both issues were caused by the same underlying issue.  RefEmit automagically applies escaping to names that
contain a ','  So ``,`` becomes ``\,``.

When we tried to create a ref to a nested type we didn't add the ',' escaping so we couldnot bind to the created type.

The fix is very straightforward, change BasicQualifiedName to correctly escape names with comma's.
@latkin
Copy link
Contributor

latkin commented Feb 19, 2015

1-line fix FTW!

Banging on this, I see some related issues with pickling/de-pickling quotations containing types with commas. They don't seem to be caused by this change, though, so I'll open as separate bugs.

Have you checked how this affects type providers? They rely on comma-encoding in the type name to handle static parameters, so might be worth a careful look.

For the tests - the bug was with FSI only, but current tests won't be run through FSI (at least not usually). Rather than add a new core area, it might be a better fit to expand existing backtick stuff at fsharpqa\source\Conformance\LexicalAnalysis\IdentifiersAndKeywords. You can add 2 lines to the env.lst file, one that runs via FSC, one via FSI (add FSIMODE=EXEC).

@KevinRansom
Copy link
Member Author

I'm looking at some type provider stuff right now, so I can move right on to the new bugs.
I was in two minds about re-using an existing test area. Although I concluded that item names have a bunch of test holes, they may not be terribly important, for example there are a number of banned characters that are not yet validated. I was thinking of adding a bug to get those test holes fixed.
Good call on the fsimode-exec.

@forki forki mentioned this pull request Feb 19, 2015
@dsyme
Copy link
Contributor

dsyme commented Feb 20, 2015

Nice work!

@KevinRansom
Copy link
Member Author

Closed Fixes #9 and Fixes #10

@ovatsus
Copy link

ovatsus commented Mar 22, 2015

Hi, I started hitting this FS0192 problem recently, even when testing on latest release of 4.0:

fsprojects/FSharp.Formatting#272

@latkin
Copy link
Contributor

latkin commented Mar 23, 2015

@ovatsus, looks like everything is cleared up in that issue, can you confirm? Turns out to be unrelated?

@ovatsus
Copy link

ovatsus commented Mar 23, 2015

It's still a very bad error message for missing a reference, but yes, it is fixed now. Thanks

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

Successfully merging this pull request may close these issues.

None yet

4 participants