Deletes DMASTSimplifier and replaces it with TryAsConstant() calls #885
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
redmoogle
reviewed
Oct 30, 2022
wixoaGit
requested changes
Oct 31, 2022
Altoids1
force-pushed
the
simplifier-slaughter
branch
from
226b2f0 to
2f3ce52
Compare
Altoids1
force-pushed
the
simplifier-slaughter
branch
2 times, most recently
from
ff7dda0 to
4c7867c
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Contributor
|
The constant folding is incorrect: /obj/var/derp = "/obj"
/datum/foldme/var/derp = "/datum/foldme"
/proc/Test()
var/datum/foldme/D = new /obj
src << "initial = [initial(D.derp)]"Correct output is "/obj", this outputs "/datum/foldme". |
Altoids1
force-pushed
the
simplifier-slaughter
branch
from
47c021b to
7be88df
Compare
Contributor
Author
|
I pulled back a bit and reserved the initial() folding for static and const variable access, as those do rely on the type of the variable and not the value it necessarily contains. |
Contributor
Author
|
Marking this as having #896 as a dependency, since some things I want to do with |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
ZeWaka
added a commit
to ZeWaka/OpenDream
that referenced
this pull request
Dec 31, 2022
ZeWaka
added a commit
to ZeWaka/OpenDream
that referenced
this pull request
Dec 31, 2022
ZeWaka
added a commit
to ZeWaka/OpenDream
that referenced
this pull request
Dec 31, 2022
wixoaGit
pushed a commit
that referenced
this pull request
Jan 1, 2023
* Const evaluation for comparison exprs * Adds in a comparison unit test * Equality fixes for nulls, for parity * vestigial changes * also leftshiftassign * Remove duplicate behaviors * review changes and better testing * remove bit of code which will be obselete with #885 * this too
Doing this, with other changes, hopefully will significantly improve our ability to const up code. Just doing this change by itself resulted in a 0.1% smaller JSON output.
Now all const-folding of lvalues and rvalues is orchestrated through TryAsConstant. We don't yet have simplification for larger structures like conditional blocks and so on.
This is us acting slightly beyond what BYOND can do, but I expect this to be a VERY useful addition to our const analysis, especially for 514 builds.
This could've been a lot of repetitive writing, but luckily I have a galaxy brain. 😀 EDIT: Also includes null + String and String + null.
EDIT: Also includes fixes for member access and global access.
Vars that are typed can actually store ANYTHING. The type is a weak promise that BYOND uses as a guarantee in some contexts (as in statics and constants) but not in others (as in normal variable accesses). It's still pretty sour to me that we're supporting this, but that is indeed the BYOND behaviour.
Missed a few things during rebase :^)
I *will* be coming back to this, in due time, but for now, this is parity-breaking and can't just be unmodifiable behaviour within OD.
null < "" is FALSE, and null == "" is FALSE, but null <= "" is TRUE. Every weird dynamic language needs an example where it violates the holy trinity of equality, and apparently, this is DM's!
Altoids1
force-pushed
the
simplifier-slaughter
branch
from
7be88df to
af53ccc
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
7 tasks
Member
|
This PR is fairly out of date, and I don't see it being picked back up anytime soon. While the removal of |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Right now, we aren't really consistent on how const-folding is handled. Things are only consistently folded whenever totally necessary, as in constant contexts like switch cases or whatever.
This fixes that by making ALL expressions go through a TryAsConstant pass during creation. Accordingly, this means that we don't really need DMASTSimplifier anymore, so I've removed that whole thing.
Doing this seems to shave off a kilobyte for the stripped TGstation copy I happen to have.
Works on #833 by adding == and != as const-foldable.
Changelog
initial()and the == and != operators are now also const-folded.