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

Assignment to default leads to assignment for nodecl #14253

Closed
PMunch opened this issue May 6, 2020 · 5 comments · Fixed by #14258
Closed

Assignment to default leads to assignment for nodecl #14253

PMunch opened this issue May 6, 2020 · 5 comments · Fixed by #14258

Comments

@PMunch
Copy link
Contributor

PMunch commented May 6, 2020

When assigning with let Nim requires a value, even when using nodecl. To make it generic/obvious that it's only the default I wanted to use default in order to just get the default value for the type. However this leads to an assignment being generated in the C code.

Example

{.emit:"const int TEST = 42;".}

let TEST {.importc, nodecl.}: cint = default(cint)
echo TEST

Current Output

error: assignment of read-only variable ‘TEST’
  162 |  TEST2 = 0;
      |        ^

Expected Output

42

Counter example

{.emit:"const int TEST = 42;".}

let TEST {.importc, nodecl.}: cint = 0 #This works fine
echo TEST

This works and outputs 42, so when directly declared to be 0 (or any other value) Nim won't create an assignment. But assignment to default(cint) or anything that comes from a proc creates an assignment. The proc case makes sense, and so does default(cint), but why doesn't literal values create an assignment? It would be nice if the syntax for this was a bit clearer..

Possible Solution

  • I wanted to use this to wrap constants from C without having to know their value (either because it's hard to tell because of pre-processor flags, or because I don't want to wade through documentation/source code to find the actual value of e.g. an error code). It would be nice if importc, nodecl would work in let without an assignment at all.
  • default(cint) seems to output the literal 0 to the C code, so it should just do the same as a regular assignment to 0.
@metagn
Copy link
Collaborator

metagn commented May 7, 2020

Another workaround that has a little bit of readability is

{.emit:"const int TEST = 42;".}

let TEST {.importc, nodecl.}: cint = static(default(cint))
echo TEST

@PMunch
Copy link
Contributor Author

PMunch commented May 7, 2020

Oh, that is definitely a good workaround. I still think that let assignments with importc, nodecl shouldn't require a value though. It would make this a lot cleaner, and a lot more obvious to the reader what is going on.

@timotheecour
Copy link
Member

timotheecour commented May 7, 2020

this is very related to nim-lang/RFCs#205 proposal 5.

I'd prefer this:

# without the preprocessor trick from proposal 5 or in cases where it wouldn't work
let DARWIN_MAXPATHLEN {.importc: "__DARWIN_MAXPATHLEN", header: "<dirent.h>".}: int = discard

# with the preprocessor trick from proposal 5
const DARWIN_MAXPATHLEN {.importc: "__DARWIN_MAXPATHLEN", header: "<dirent.h>".}: int = discard
  • requires no parser change (just a minor compiler change)
  • less misleading than default(cint), 0, or anything similar
  • (for the const version) allows using the symbol at nim CT (I have a branch where this works)

@PMunch
Copy link
Contributor Author

PMunch commented May 7, 2020

Hmm, I agree that discard would be a nice way of solving it as it won't require too many changes. However this would be the only instance of assigning anything to discard which would make it a total special case and an extra thing to remember, in fact I didn't even think of it as a solution until you proposed it (meaning I never would have tried it on my own). So from a usability standpoint it makes more sense to me to have it as simply lacking a value. This also makes it more similar to how this works for procedures. You only declare the signature of the procedure and tell Nim to trust that it exists like that in the C code. Similar scenario here, you only tell Nim the "signature" of the constant, and let Nim trust that it exists in the C code.

@timotheecour
Copy link
Member

timotheecour commented May 7, 2020

I'm fine with other keywords/identifiers than discard, but not default(cint) or 0 or anything that's basically giving a wrong misleading value.
auto would work nicely too (even though it's otherwise only used for return types); but the meaning should be self evident:

const DARWIN_MAXPATHLEN {.importc: "__DARWIN_MAXPATHLEN", header: "<dirent.h>".}: int = auto

note 1:

the alternative that was suggested was to skip =, however this would prevent using const in a backward compatible way since it would fail at lexing time eg:

since (1,3,5):
  let foo {.importc.}: int # ok, that can work
  const foo {.importc.}: int # parser error for nim < 1.3.5

so it would imply having to backport the parser change to all supported releases (1.0.X and 1.2.X) or forgo of the possiblity of using const (which would be the worst option; const should be used when possible)

Hence I stand by my recommendation for auto, which shouldn't confused anyone and avoids all those backward compatiblity parser issues:

since (1,3,5):
  let foo {.importc.}: int = auto
  const foo {.importc.}: int = auto

note 2:

this syntax would work too without parser change nor preventing the use of const:

since (1,3,5):
  let foo {.importc.} = lookup(int)
  const foo {.importc.} = lookup(int)

(or some other identifier instead of lookup)
I still prefer auto but lookup (or similar) works too.

PMunch added a commit to PMunch/Nim that referenced this issue May 7, 2020
This allows a let statement with the `{.importc.}` pragma to not be
initialised with a value. This allows us to declare C constants as Nim
lets without putting the value in the Nim code (which can lead to
errors, and requires us to go looking for the value). Fixes nim-lang#14253
Araq pushed a commit that referenced this issue May 12, 2020
* Allow let to not have value when using importc

This allows a let statement with the `{.importc.}` pragma to not be
initialised with a value. This allows us to declare C constants as Nim
lets without putting the value in the Nim code (which can lead to
errors, and requires us to go looking for the value). Fixes #14253

* Proper fix and documentation + changelog entry

* Improve testcase with one from timotheecour

* Add test to verify it working with macros
EchoPouet pushed a commit to EchoPouet/Nim that referenced this issue Jun 13, 2020
* Allow let to not have value when using importc

This allows a let statement with the `{.importc.}` pragma to not be
initialised with a value. This allows us to declare C constants as Nim
lets without putting the value in the Nim code (which can lead to
errors, and requires us to go looking for the value). Fixes nim-lang#14253

* Proper fix and documentation + changelog entry

* Improve testcase with one from timotheecour

* Add test to verify it working with macros
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants