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

[Regression] converter to string leads fail to compile on 0.19 #9149

Closed
khchen opened this Issue Oct 1, 2018 · 8 comments

Comments

Projects
None yet
6 participants
@khchen
Copy link

khchen commented Oct 1, 2018

type
  Container = ref object
    data: int

converter containerToString*(x: Container): string = $x.data

var c = Container(data: 123)
var str = string c
echo str

if c == nil: # this line can compile on v0.18, but not on 0.19
  echo "c is nil"

if not c.isNil:
  echo "c is not nil"

@khchen khchen changed the title [Regression] converter to string leads fail to compile on 0. [Regression] converter to string leads fail to compile on 0.19 Oct 1, 2018

@Bennyelg

This comment has been minimized.

Copy link

Bennyelg commented Oct 2, 2018

it's not a regression
compile with --nilseqs:on for a migration period

@Araq

This comment has been minimized.

Copy link
Member

Araq commented Oct 2, 2018

This is a regression, nil is available for ref object and so the == should compile.

@krux02

This comment has been minimized.

Copy link
Contributor

krux02 commented Oct 2, 2018

As a workaround if c.isNil: does work.

@Bennyelg

This comment has been minimized.

Copy link

Bennyelg commented Oct 2, 2018

is it really a valid to check if

something == nil

?

@krux02

This comment has been minimized.

Copy link
Contributor

krux02 commented Oct 3, 2018

@Bennyelg yes it is valid to check is something is nil. It is just not valid to test if a string or a seq is nil. In this case it's the converter in combination with the implementation of the error message is causing the problem.

My recommendation is not to use converter. Write the conversion explicity.

@Bennyelg

This comment has been minimized.

Copy link

Bennyelg commented Oct 3, 2018

@krux02 The concept of Converters is new for me, looks like something which magnitude errors and exceptions.

@khchen

This comment has been minimized.

Copy link
Author

khchen commented Oct 3, 2018

My recommendation is not to use converter. Write the conversion explicity.

I agree you. Don't use converter if it is not necessary. However, this code is part of my winim/com , it depends on converter so that people can use COM object like a script langauge.

@narimiran

This comment has been minimized.

Copy link
Member

narimiran commented Jan 24, 2019

Don't use converter if it is not necessary. However, this code is part of my winim/com , it depends on converter so that people can use COM object like a script langauge.

Two lines below you have used if not c.isNil:, why can't you use if c.isNil in the place where if c == nil currently fails?

@Araq Araq closed this in 07a0a61 Jan 29, 2019

narimiran added a commit that referenced this issue Jan 31, 2019

fixes #9149 [backport]
(cherry picked from commit 07a0a61)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment