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

prefer x == nil to x.isNil #16191

Closed
wants to merge 3 commits into from
Closed

prefer x == nil to x.isNil #16191

wants to merge 3 commits into from

Conversation

ringabout
Copy link
Member

@ringabout ringabout commented Nov 30, 2020

Let me assume that x == nil or x != nil is preferred.

Araq: use != nil please
Araq: isNil could be deprecated, it's pointless

https://discord.com/channels/371759389889003530/371759389889003532/696718991019999252

EDITED:
So which one is preferred? We need some docs for either of situations I think.

lib/system.nim Outdated Show resolved Hide resolved
@ringabout ringabout marked this pull request as draft November 30, 2020 08:07
@ringabout ringabout marked this pull request as ready for review November 30, 2020 08:31
@timotheecour
Copy link
Member

How about deprecate instead?

@Araq
Copy link
Member

Araq commented Nov 30, 2020

Sorry, even though I did say that, it's not covered by an RFC and I dislike these disruptions more and more. I don't want to see code updates from x.isNil to x != nil, it's a waste of everybody's time. So yeah, there are two ways to write the same, we'll survive.

@Araq Araq closed this Nov 30, 2020
@timotheecour
Copy link
Member

people copy what's in stdlib though; at least a (single) PR that would replace all usages of isNil in stdlib would help

@disruptek
Copy link
Contributor

It's not pointless. It sidesteps the == problem (rarely). I had to fix a bug with isNil not long ago. It may also be easier to adapt to @alehander92's "not nillable" work. I don't use it in the compiler (unless I must, to fix a bug) because it runs contrary to existing style, but IMO style is the only reason not to use it.

@timotheecour
Copy link
Member

It sidesteps the == problem (rarely)

which problem? Maybe you're referring to #15595 (isNil worked for procvar but not ==) but I've fixed this recently; bugs should be fixed, not worked around and there's no reason to have more than 1 way to do the same thing, all else being equal (notably performance)

@metagn
Copy link
Collaborator

metagn commented Dec 2, 2020

which problem?

User definitions of ref T == ref T will also have to account for nil every time, same with distinct ref T or distinct proc or distinct ptr, they could even intentionally but wrongly give a false result for == nil. isNil is more reliable in this regard.

@Araq
Copy link
Member

Araq commented Dec 2, 2020

If isNil is better than x == nil or vice versa we need an RFC to sort it out. I would heavily vote in favor of isNil as comparing to nil is simpler, it might not be clear if == is defined all that well, but comparisons against nil are special. For example once proc deduplication lands in Nim, comparing procs is quirky, but comparing a proc to nil will remain to be fine.

@timotheecour
Copy link
Member

timotheecour commented Dec 2, 2020

good points, but then I'd prefer isDefault, which would work with any type, not just ptr/cstringr/ref/proc.

(somewhat related: nim-lang/RFCs#224, bitEquals and nim-lang/RFCs#229)

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

6 participants