search

metaborg / nabl

Spoofax' Name Binding Language
Apache License 2.0
7 stars 12 forks source link

Add origin field to constraints #113

Closed Virtlink closed 1 year ago

Virtlink commented 1 year ago

This PR adds an origin field to constraints, tracking the syntactic constraint to which substitutions were applied that resulted in the current constraint.

Virtlink commented 1 year ago

About the withArguments(): in many places in Statix, a new copy is created of a constraint but with slightly different arguments. For example, instead of many instances of:

new CEqual(newTerm1, newTerm2, c.cause().orElse(null), c.message().orElse(null));

now you use:

c.withArguments(newTerm1, newTerm2);

and the remaining fields (metadata such as cause, message, now origin) are preserved. This way I don't also have to preserve origin everywhere only the main arguments of the constraint are updated.

I have found instances where sometimes a completely new constraint instance was constructed while all the fields were equal, which is why I added the if-checks. (I found this only with withArguments(), but the check is cheap so I added it to withCause() and withMessage() too. And I estimate that this will have a cascading effect: since some instances are no longer re-created, other instances detect that the arguments are unchanged and also not re-created.

AZWN commented 1 year ago

Also, we should check how often the exact referential equality actually occurs. E.g., on a substitution that does not change a term, I think a new instance is given.