nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.55k stars 1.47k forks source link

No `XDeclaredButNotUsed` in overloaded function with generics #23431

Open tersec opened 7 months ago

tersec commented 7 months ago

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 commented 7 months ago

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

tersec commented 7 months ago

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

Exactly. And yet there's no XDeclaredButNotUsed hint.

Araq commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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, https://github.com/nim-lang/Nim/issues/20264 https://github.com/nim-lang/Nim/issues/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 commented 7 months ago

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.