nim-lang / RFCs

A repository for your Nim proposals.
135 stars 26 forks source link

generic sigmatch should trigger openSymChoice even without overloads #417

Open timotheecour opened 2 years ago

timotheecour commented 2 years ago

Innocuous looking refactorings such as case1=>case2 below are actually currently causing breaking changes due to the fact that a single generic sigmatch currently implies closedSymChoice instead of openSymchoice.

example

# lib.nim
when defined case1:
  proc fn(a: float32): string = $a
  proc fn(a: float): string = $a
when defined case2:
  proc fn(a: float|float32): string = $a
when defined case3:
  proc fn(a: SomeFloat): string = $a
when defined case4:
  proc fn[T](a: T): string = $a

proc bar*[T](a: T): string = fn(a)

# main.nim
import t12746b
proc fn(a: float): string = "override"
echo bar(1.0)

current behavior

nim r -d:case1 main override # open symchoice

nim r -d:case2 main 1.0 # closed symchoice

proposed behavior

nim r -d:case1 main override # open symchoice

nim r -d:case2 main override # open symchoice

ditto for case3, caes4 since in each case the sigmatch is either generic or involves > 1 overloads

links

notes

this is a breaking change, but one that will actually allow code refactorings without breaking changes, so less breaking changes down the line.

implementation

logic to change should be inside: see if i <= 1 and r != scForceOpen: in semtempl.symChoice

konsumlamm commented 2 years ago

In case1, wouldn't you have two functions with the same name and signature? I don't see why that should compile at all.

timotheecour commented 2 years ago

In case1, wouldn't you have two functions with the same name and signature? I don't see why that should compile at all.

procs declared in current module take precedence; conflicts only occur if you'd have ambiguities via imports:

import a # contains proc fn(a: int)=discard
import b # contains proc fn(a: int)=discard
fn(1)

it's a reasonable rule and it's in the manual

barcharcraz commented 2 years ago

The thing is it helps maintainability if the default is closed identifiers, and the current rules are very simple, much, much simpler than trying to nail down exactly which dependent expressions should be open and which should be closed.

We want closed by default (imo) because open identifiers hurt the compilers ability to emit diagnostics while parsing the generic definition, and also make the generic more brittle as it could be broken by subsequent overloads that it has no control over