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

retain postfix node for typed variable/routine AST; save field syms #23120

Open
wants to merge 11 commits into
base: devel
Choose a base branch
from

Conversation

metagn
Copy link
Collaborator

@metagn metagn commented Dec 23, 2023

fixes #13828, refs #23101

Postfix nodes in var/let/const/routine nodes are now retained in typed AST, previously they would just become either an nkSym node or an nkPragmaExpr node containing the nkSym node. This was done for type sections in #23101 which also handled doc generation of postfix nodes. The newly generated postfix nodes get transformed back into just the symbol nodes in transf.

On top of this, object fields now store their symbol node in the AST. The old output of:

import macros

type Foo = object
  a: int

static:
  echo treeRepr getImpl bindSym"Foo"

was:

TypeDef
  Sym "Foo"
  Empty
  ObjectTy
    Empty
    Empty
    RecList
      IdentDefs
        Ident "a"
        Sym "int"
        Empty

Now the Ident "a" becomes Sym "a", storing the symbol of the field.

@metagn
Copy link
Collaborator Author

metagn commented Dec 23, 2023

It seems like no packages in the CI were affected by the AST changes in this PR, but I added it to the changelog anyway, at least for the type case which did require package updates.

@metagn

This comment was marked as off-topic.

@Araq
Copy link
Member

Araq commented Jan 1, 2024

Sorry for the late reply. This is a step in the right direction but I dream of a more radical change: Introduce an nkIdentDecl which is always the tuple (name, export, pragma, type, value).

@metagn
Copy link
Collaborator Author

metagn commented Jan 4, 2024

That would be nice, I don't know how it would interact with types and procs though, they don't use nkIdentDefs. At the very least we can combine nkPragmaExpr and nkPostfix.

We would also have to either gate it into an experimental switch or have it for internal use only and always produce the old AST to pass to macros and then turn it back after receiving it (presumably it's present in both untyped and typed AST).

@Araq
Copy link
Member

Araq commented Jan 4, 2024

... or have it for internal use only and always produce the old AST to pass to macros and then turn it back after receiving it (presumably it's present in both untyped and typed AST).

That's the solution I have in mind. As for "not used in parameters", maybe it should be nkLetDecl(5 fields), nkVarDecl(5 fields), nkParamDecl(5 fields), etc.

@metagn
Copy link
Collaborator Author

metagn commented Jun 23, 2024

Rebased and added test for #13828

@metagn
Copy link
Collaborator Author

metagn commented Aug 19, 2024

Rebased to fix merge conflicts but the diff for ast2nir is gone since it was removed, just a heads up in case it was moved somewhere else. Maybe we can have this before 2.2?

@Araq
Copy link
Member

Araq commented Aug 19, 2024

It was not moved to anywhere and the whole NIR thing got replaced by NIF which lives in its own repository.

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.

template-generating templates are not exported
2 participants