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

untyped prepass, use for generic procs/template in compat mode #577

Merged
merged 11 commits into from
Feb 21, 2025

Conversation

metagn
Copy link
Collaborator

@metagn metagn commented Feb 19, 2025

Not fully implemented, missing types, type decls and routines, but these can hopefully be done later.

The generic and template prepasses are not too far apart and the attempt here is to implement them in 1 file, mostly based on the template prepass. I don't know every precise difference between the two and there might be mistakes but I'm still guessing this is easier to maintain than 2 files. Fixing old compiler bugs is a non-goal regardless.

The biggest difference is symbol definitions. Both the template and generics prepasses add symbols to the scope. Generics adds every single declared symbol as an skUnknown symbol, every single use of an skUnknown symbol becomes an identifier (i.e. is injected). Templates do not add injected symbols to scope, instead storing a set of injected identifier names for them; but every gensym symbol is added to scope, having the corresponding symbol kind.

The way they are combined here is generics now use injections instead of skUnknown, and injects are now somewhat scoped: an injected symbol defined in a scope will not be injected outside of that scope. For templates this would previously present as this bug:

template foo =
  if false:
    let x {.inject.} = 123
  let x = 456
  echo x # undeclared identifier
foo()

This implies a behavior change to templates, there is an off chance that code might have depended on this but it arguably should have already been the case. Worst case we can disable this scoping behavior in template mode. It should be 1 to 1 with generics though.

Injects are still tracked by a set of identifiers, each scope now tracks which identifiers it newly injected compared to the scopes above it, then the newly injected identifiers are removed from the inject set when exiting the scope. This means injected symbols are still checked before looking for symbols in scope, so stuff like this behaves the same as before:

template foo() =
  let x {.inject.} = 123
  block:
    let x = 456
    echo x # 123
foo()

I am not sure if we should handle this differently, it would most likely mean the inject behavior is moved to the scope object which has overlap with the hypothetical untyped identifier rules in #254. But this won't matter for a while.

Other differences are:

  • Special casing of calls in generics (currently not done):
    • No logic for untyped macro expansion or nullary templates/macros
    • No withinMixin (would prevent "undeclared identifier" errors which are no longer there)
  • Symbol lookup rules are based on templates.
    • Generics turns symbols like params back into identifiers, might not need to be done here.
    • Not sure of other differences
  • The dot expr symchoice stuff isn't implemented yet because the compiler doesn't know how to walk it back, same for [] etc, not sure if they have any differences between generics and templates
  • Qualified dot exprs are not specialized yet, not sure if they need to be
  • No openSym as this is supposed to be for compat

Final note: Compat mode is currently global, meaning it is also active for templates/generic procs in the system module. Don't know if this will become a problem.

Edit: Windows CI implies compat mode is a problem with nifcache (thankfully only changes line infos and still works). Maybe just update hastur to wipe nifcache after the compat category for now. - Now done

@metagn metagn marked this pull request as ready for review February 21, 2025 17:11
@Araq Araq merged commit ced508a into nim-lang:master Feb 21, 2025
3 checks passed
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

Successfully merging this pull request may close these issues.

2 participants