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

Class with [<AllowNullLiteral(false)>] barred from inheriting from normal non-nullable class #383

Closed
latkin opened this issue Apr 23, 2015 · 3 comments

Comments

@latkin
Copy link
Contributor

@latkin latkin commented Apr 23, 2015

Super minor, but might as well log it. Up for grabs.

I would think this code should be legal

type A() = class end

[<AllowNullLiteral(false)>]
type B() = inherit A() // error FS0935: Types with the 'AllowNullLiteral' attribute may only inherit from or implement types which also allow the use of the null literal

The check makes sense, but fails to consider the case when the attribute explicitly disallows nulls. I made the same mistake in #376.

Fix is to change hasAllowNullLiteralAttr in tc.fs to actually check the value of the attribute, something like TryFindTyconRefBoolAttribute cenv.g m cenv.g.attrib_AllowNullLiteralAttribute thisTyconRef = Some(true). [That is around line 13985 of tc.fs, unfortunately GH won't render that many lines so I can't link directly to it 😭]

@mexx

This comment has been minimized.

Copy link
Contributor

@mexx mexx commented Apr 28, 2015

@latkin I want to look into it, can you point me the right place where a test for this case should be placed?

@mexx

This comment has been minimized.

Copy link
Contributor

@mexx mexx commented Apr 28, 2015

I've found some tests covering AllowNullLiteralAttribute:

  • in test-hw.fsx, the positive ones
  • in neg16.fs, the negative ones
  • in E_AllowNullLiteral02.fs, as well positive as negative ones
@latkin

This comment has been minimized.

Copy link
Contributor Author

@latkin latkin commented Apr 28, 2015

Adding some cases to tests\fsharp\typecheck\sigs\neg16.fs seems like the right place, maybe also add to one of the pos*.fs guys for non-error cases.

mexx added a commit to mexx/visualfsharp that referenced this issue Apr 29, 2015
@latkin latkin added the fix ready label Apr 30, 2015
@latkin latkin closed this in 8b249d8 Apr 30, 2015
yatli pushed a commit to yatli/visualfsharp that referenced this issue May 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.