Skip to content

Add an option to copy positions from old tree when doing substitutions - #107

Open
jad-hamza wants to merge 2 commits into
epfl-lara:scala-2.13from
jad-hamza:replaceFromSymbols-positions
Open

Add an option to copy positions from old tree when doing substitutions#107
jad-hamza wants to merge 2 commits into
epfl-lara:scala-2.13from
jad-hamza:replaceFromSymbols-positions

Conversation

@jad-hamza

Copy link
Copy Markdown
Contributor

No description provided.

@romac

romac commented Sep 17, 2019

Copy link
Copy Markdown
Member

Reminder: Revert epfl-lara/stainless@a2f2600 when this is released.

@samarion

Copy link
Copy Markdown
Member

I'm not sure this really makes sense as you'll only copy the position over for the top-level substituted expression and not its children.

What is your use case? Maybe it would make more sense to use a custom transformer?

@romac

romac commented Sep 20, 2019

Copy link
Copy Markdown
Member

I don’t know the specific use-/testcase Jad had in mind, but he indeed ended up using a custom transformer in epfl-lara/stainless@a2f2600 FYI

@jad-hamza

Copy link
Copy Markdown
Contributor Author

Right, I needed it so that we don't lose the positions when substituting a variable with a new variable

you'll only copy the position over for the top-level substituted expression and not its children

Yes that's true, it's a bit weird having it in the general case. Should we move replaceKeepPositions from epfl-lara/stainless@a2f2600 (bad naming sorry) to Inox? (And change the signature to only replace variables with variables?)

@vkuncak

vkuncak commented Apr 15, 2021

Copy link
Copy Markdown
Contributor

@jad-hamza , is this still relevant? Is it for error reporting of positions?

@jad-hamza

jad-hamza commented Apr 16, 2021

Copy link
Copy Markdown
Contributor Author

Yes it's useful when you're freshening variables to maintain positions. Normal substitution will substitute all variables (which are at different position) by just one instance of a variable (which has a single position).

Now that I look at my code again in Stainless, I wonder how it was supposed to work. The function is still replacing with one single instance, instead of cloning the variable.

Edit: we can close here and address this in Stainless if this function is not needed in Inox

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.

4 participants