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

Compiler thinks an unused proc is used when an auto parameter is used #21724

Closed
diegomrsantos opened this issue Apr 24, 2023 · 4 comments · Fixed by #21738
Closed

Compiler thinks an unused proc is used when an auto parameter is used #21724

diegomrsantos opened this issue Apr 24, 2023 · 4 comments · Fixed by #21738

Comments

@diegomrsantos
Copy link

diegomrsantos commented Apr 24, 2023

Description

When executing

type
  SomeObj = object
    hey: bool

proc hey() {.deprecated: "Shouldn't use this".} = echo "hey"

proc gen(o: auto) =
  if o.hey:
    echo "bool"

gen(SomeObj(hey: true))

The compiler thinks the hey proc is being called and outputs the warning, but it is not actually being called.

Nim Version

Nim Compiler Version 1.6.12 [MacOSX: amd64]
Compiled at 2023-03-16
Copyright (c) 2006-2023 by Andreas Rumpf

active boot switches: -d:release

Current Output

.................................................................
t.nim(8, 8) Warning: Shouldn't use this; hey is deprecated [Deprecated]
CC: t.nim
Hint:  [Link]
Hint: gc: refc; threads: on; opt: none (DEBUG BUILD, `-d:release` generates faster code)
64653 lines; 0.353s; 75.875MiB peakmem; proj: t; out: t [SuccessX]
Hint: t  [Exec]
bool

Expected Output

.................................................................
t.nim(5, 6) Hint: 'hey' is declared but not used [XDeclaredButNotUsed]
CC: t.nim
Hint:  [Link]
Hint: gc: refc; threads: on; opt: none (DEBUG BUILD, `-d:release` generates faster code)
64653 lines; 0.353s; 75.875MiB peakmem; proj: t; out: t [SuccessX]
Hint: t  [Exec]
bool

Possible Solution

No response

Additional Information

No response

@Menduist
Copy link
Contributor

When using error instead of deprecated, the code doesn't even compile, making this issue more problematic

@beef331
Copy link
Collaborator

beef331 commented Apr 25, 2023

Could not help but minimize it further, the issue is that closed symbol causes semantic analysis to be done, which means it compiles the deprecated. Swapping bind for mixin causes this deprecation to disappear.

proc hey() {.deprecated: "Shouldn't use this".} = echo "hey"

proc gen(o: auto) =
  bind hey

@metagn

This comment was marked as off-topic.

@beef331
Copy link
Collaborator

beef331 commented Apr 25, 2023

I assume it's just to do exactly as this issue does but in a sensible way

proc doThing(x: auto) = discard
proc myGen(x: auto) = doThing(x) # We use `doThing` so it's used

Arguably the issue here is really that o.hey does field access instead of invoking the closed symbol and erroring that it expects 0 parameters.

metagn added a commit to metagn/Nim that referenced this issue Apr 27, 2023
metagn added a commit to metagn/Nim that referenced this issue May 4, 2023
Araq pushed a commit that referenced this issue May 5, 2023
* always force open generic dot field symbols?

fixes #21724 but might break code

* alternative, should fix CI

* other alternative, add test for previous CI failure

* not needed

* make sure call doesn't compile too

* ok actual second test

* ok final actual correct test

* apply performance idea

* don't make fromDotExpr static
capocasa pushed a commit to capocasa/Nim that referenced this issue May 15, 2023
…#21738)

* always force open generic dot field symbols?

fixes nim-lang#21724 but might break code

* alternative, should fix CI

* other alternative, add test for previous CI failure

* not needed

* make sure call doesn't compile too

* ok actual second test

* ok final actual correct test

* apply performance idea

* don't make fromDotExpr static
capocasa pushed a commit to capocasa/Nim that referenced this issue May 16, 2023
…#21738)

* always force open generic dot field symbols?

fixes nim-lang#21724 but might break code

* alternative, should fix CI

* other alternative, add test for previous CI failure

* not needed

* make sure call doesn't compile too

* ok actual second test

* ok final actual correct test

* apply performance idea

* don't make fromDotExpr static
narimiran pushed a commit that referenced this issue Jun 15, 2023
* always force open generic dot field symbols?

fixes #21724 but might break code

* alternative, should fix CI

* other alternative, add test for previous CI failure

* not needed

* make sure call doesn't compile too

* ok actual second test

* ok final actual correct test

* apply performance idea

* don't make fromDotExpr static

(cherry picked from commit e92d768)
narimiran pushed a commit that referenced this issue Jun 19, 2023
* always force open generic dot field symbols?

fixes #21724 but might break code

* alternative, should fix CI

* other alternative, add test for previous CI failure

* not needed

* make sure call doesn't compile too

* ok actual second test

* ok final actual correct test

* apply performance idea

* don't make fromDotExpr static

(cherry picked from commit e92d768)
bung87 pushed a commit to bung87/Nim that referenced this issue Jul 29, 2023
…#21738)

* always force open generic dot field symbols?

fixes nim-lang#21724 but might break code

* alternative, should fix CI

* other alternative, add test for previous CI failure

* not needed

* make sure call doesn't compile too

* ok actual second test

* ok final actual correct test

* apply performance idea

* don't make fromDotExpr static
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.

5 participants