fix #7997 #8005
fix #7997 #8005
Conversation
compiler/semexprs.nim
Outdated
| if isDefined(c.config, n.sons[1].sym.name.s): | ||
| result.intVal = 1 | ||
| else: | ||
| discard |
There was a problem hiding this comment.
what if someone type something like defined(123), wouldn't it better to tell the user that it is not allowed rather than let it go silently?
There was a problem hiding this comment.
With the change @Araq proposed, that is taken care of, if I'm not mistaken. :)
compiler/semexprs.nim
Outdated
| localError(c.config, n.info, "obsolete usage of 'defined', use 'declared' instead") | ||
| elif isDefined(c.config, n.sons[1].ident.s): | ||
| result.intVal = 1 | ||
| case n.sons[1].kind |
There was a problem hiding this comment.
it's easier than this. Use:
let d = considerQuotedIdent(c.config, n[1], n)
result.intVal = isDefined(c.config, d.s)There was a problem hiding this comment.
Oohhh, that's nice! So many helpful procs in the compiler :)
|
Oh and please add a test case. |
|
I put the test case under templates. Not sure if that's a good place for it? And I hope I covered all aspects of it. |
tests/template/t7997.nim
Outdated
| @@ -0,0 +1,46 @@ | |||
| discard """ | |||
| output: Valid and not defined | |||
There was a problem hiding this comment.
't7997' is a bad test name. Name it 'tdefined' or similiar.
Vindaar
force-pushed
the
fixes-7997
branch
2 times, most recently
from
e60bf5f to
4d38ff9
Compare
compiler/semexprs.nim
Outdated
| elif isDefined(c.config, n.sons[1].ident.s): | ||
| result.intVal = 1 | ||
| let d = considerQuotedIdent(c.config, n[1], n) | ||
| result.intVal = if isDefined(c.config, d.s): 1 else: 0 |
There was a problem hiding this comment.
ord isDefined(c.config, d.s) would be more idiomatic.
There was a problem hiding this comment.
Had no idea that works, hehe. Nice :)
Fixes issue nim-lang#7997, which was caused by an export of a `release` proc in `locks`. Thus the `release` in `defined(release)` of the `ifDebug` template, was of kind `nkSym` instead of `nkIdent`. We fix this by getting the `PIdent` of the argument to `defined` using `considerQuotedIdent`. This has the nice property of also checking for a valid identifier for us. E.g. `defined(123)` would fail with ``` Error: in expression 'defined(123)': identifier expected, but found '123' ``` The `localError` is removed, due to a clear distinction between `declared` and `defined` now.
This is my first PR for the compiler, so I hope I'm not doing anything weird here...
It fixes issue #7997, which was caused by an export of a
releaseprocin
locks. Thus thereleaseindefined(release)of theifDebugtemplate, was of kind
nkSyminstead ofnkIdent.This PR fixes that by casing on the kind of
n.sons[1]insemexpr.semDefinednow. Ifit's of kind
nkSymwe get the correspondingPIdentfrom the symboland check if that exists.
The
localErroris removed, due to a clear distinction betweendeclaredanddefinednow.