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

pragma for sfCallsite instead of name check + better semantics, test #20464

Merged
merged 3 commits into from
Oct 3, 2022

Conversation

metagn
Copy link
Collaborator

@metagn metagn commented Sep 29, 2022

Previously sfCallsite is only inferred based on template name & module at template definitions. Now there is a pragma for it named callsite, although another pragma name can be reused (unsure if the shim works for this though). sfCallsite also now acts recursively, like {.line.}.

Can document the pragma, but it seems to be for internal use. The difference with {.line.} is that since sfCallsite acts before the template is evaluated, the substituted parameters keep their original information. The test shows this.

This should also make it possible to make comparisons and setops imports, but this doesn't have to be done.

if sfSystemModule in s.owner.flags and s.name.s in names or
s.owner.name.s == "vm" and s.name.s == "stackTrace":
incl(s.flags, sfCallsite)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of sfCallsite anyway?

Copy link
Collaborator Author

@metagn metagn Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like {.line: instantiationInfo().}, except applies to the top node of the template body, instead of recursively on nodes below it, meaning .line will wipe the info of template parameters and callsite doesn't really do anything to an indented template body. It's implemented for these procs because instantiationInfo is lower in system than where they're implemented (it has to be below arithmetics because it uses -1, but it has to be above comparisons which is the next line after arithmetics).

I can sandwich instantiationInfo between arithmetics and comparisons, or I can try to make sfCallsite recursively copy the line info while not modifying parameter info (which it should be able to do, since it's accounted for before the template is evaluated). I'm partial towards improving callsite because it's more terse than .line, maybe we can reuse ccInline with .inline to save a symbol flag spot.

Edit: Apparently .line assumes instantiationInfo() without a parameter, and applies to expressions. So the only difference is that .line overrides template parameter infos.

Not documented because it seems to be for internal use?

Should also make it possible to make comparisons and setops imports, but this doesn't have to be done.

I can reuse a name like `cursor` for the pragma as well, added a new name just to be safe.
@metagn metagn marked this pull request as draft September 30, 2022 11:09
@metagn metagn force-pushed the remove-callsite-hack branch from 6c30fde to 3ce58eb Compare September 30, 2022 12:31
@metagn metagn marked this pull request as ready for review September 30, 2022 12:31
@metagn metagn changed the title pragma for sfCallsite instead of name check at every template definition pragma for sfCallsite instead of name check + better semantics, test Oct 2, 2022
@Araq Araq merged commit 2cca38d into nim-lang:devel Oct 3, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from 2cca38d

Hint: mm: orc; opt: speed; options: -d:release
162629 lines; 8.395s; 664.504MiB peakmem

capocasa pushed a commit to capocasa/Nim that referenced this pull request Mar 31, 2023
…im-lang#20464)

* pragma for sfCallsite instead of name check at every template definition

Not documented because it seems to be for internal use?

Should also make it possible to make comparisons and setops imports, but this doesn't have to be done.

I can reuse a name like `cursor` for the pragma as well, added a new name just to be safe.

* make sfCallsite recursive, add tests
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