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

No XDeclaredButNotUsed in overloaded function with generics #23431

Open
tersec opened this issue Mar 20, 2024 · 6 comments
Open

No XDeclaredButNotUsed in overloaded function with generics #23431

tersec opened this issue Mar 20, 2024 · 6 comments

Comments

@tersec
Copy link
Contributor

tersec commented Mar 20, 2024

Description

proc n() = discard
proc n(g: int|int) = n()

Nim Version

Nim Compiler Version 1.6.20 [Linux: amd64]
Compiled at 2024-03-20
Copyright (c) 2006-2023 by Andreas Rumpf

git hash: 8f9fde0615c686357774736c0bce6f3c2075a94d
active boot switches: -d:release
Nim Compiler Version 2.0.4 [Linux: amd64]
Compiled at 2024-03-20
Copyright (c) 2006-2023 by Andreas Rumpf

git hash: d4b58b0b068475e937e9e25af4f11f649285274c
active boot switches: -d:release
Nim Compiler Version 2.1.1 [Linux: amd64]
Compiled at 2024-03-20
Copyright (c) 2006-2024 by Andreas Rumpf

git hash: 6c4c60eade66e45448a220f4c8a91f866b76baf7
active boot switches: -d:release

Current Output

No XDeclaredButNotUsed hints

Expected Output

r.nim(2, 6) Hint: 'n' is declared but not used [XDeclaredButNotUsed]

Possible Solution

No response

Additional Information

No response

@Araq
Copy link
Member

Araq commented Mar 21, 2024

But it really is not used as you don't instantiate the generic.

@tersec
Copy link
Contributor Author

tersec commented Mar 21, 2024

But it really is not used as you don't instantiate the generic.

Exactly. And yet there's no XDeclaredButNotUsed hint.

@Araq
Copy link
Member

Araq commented Mar 21, 2024

Ah, but it is used in the generic lookup pass. :P In fact, the overload is what causes it to be an "open symchoice". Screwed if you emit the warning, screwed if you don't.

@metagn
Copy link
Collaborator

metagn commented Mar 21, 2024

This mixes in both n symbols and soft marks them as used, meaning only the warning is omitted but things like {.error.} don't trigger.

Otherwise this would warn that every n is unused if no code uses foo:

proc n(x: int) = discard
proc n(x: float) = discard
proc foo*(x: int | float) =
  n(x)

The issue is marking the symbol of the proc itself used in general, this also doesn't give an unused warning:

proc foo() =
  foo()

@tersec
Copy link
Contributor Author

tersec commented Mar 21, 2024

The issue is marking the symbol of the proc itself used in general, this also doesn't give an unused warning:

proc foo() =
  foo()

Sure, #20264 #22274 also an issue, but there's the higher-level semantic argument there (which I disagree with, but it's at least therefore a distinct case) that in some sense recursive functions are always used, by themselves.

This is different because the n(int | int) is unambugously not used, that Nim does this analysis by soft-marking symbols of the procs notwithstanding, so it's a more unambiguous case.

@Araq
Copy link
Member

Araq commented Mar 25, 2024

The real solution here is to type-check generics so that the "pre pass" is not required anymore and it's clear what is going on within a generic (nothing special at all). But lacking this, the overload is not unused since you cannot just remove it and get equivalent behavior in all the edge cases. So a warning would be more harmful then helpful.

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

No branches or pull requests

3 participants