[analysis][NFC] Rename makeLeastUpperBound to join and move it to lattice
#6035
Conversation
|
Current dependencies on/for this PR:
This stack of pull requests is managed by Graphite. |
src/analysis/lattices/powerset.h
Outdated
|
|
||
| // Calculates the LUB of this element with some other element and sets | ||
| // this element to the LUB in place. Returns true if this element before | ||
| // this method call was different than the LUB. |
There was a problem hiding this comment.
Maybe comment on how the name is a shorthand, as explained in the PR description?
| // fits with the conception of the stack starting at the top and having | ||
| // an infinite bottom, which allows stacks of different height and scope | ||
| // to be easily joined. | ||
| bool join(Element& self, const Element& other) const noexcept { |
There was a problem hiding this comment.
Nit: Seems a bit weird to call the first element "self" as it implies the instance of the StackLattice class. Wdyt?
There was a problem hiding this comment.
Fair enough. Not sure what better to call them than a and b or something similarly unimaginative, though. Do you have ideas?
There was a problem hiding this comment.
The Obj-C dev in me quite liked the previous naming of makeLeastUpperBound because it encoded mutability at the function's call site.
Since the first argument always has to match the LUB, maybe we can call it LUB?
There was a problem hiding this comment.
Hmm, the first argument is the LUB at the end of the method, but not necessarily at the beginning. It would be nice to have names that make sense both before and after.
Maybe joinee and joiner?
There was a problem hiding this comment.
Will do this globally as a follow-up.
… lattice In general, the operation of taking the least upper bound of two lattice elements may depend on some state stored in the lattice object rather than in the elements themselves. To avoid forcing the elements to be larger and more complicated in that case (by storing a parent pointer back to the lattice), move the least upper bound operation to make it a method of the lattice rather than the lattice element. This is also more consistent with where we put e.g. the `compare` method. While we are at it, rename `makeLeastUpperBound` to `join`, which is much shorter and nicer. Usually we avoid using esoteric mathematical jargon like this, but "join" as a normal verb actually describes the operation nicely, so I think it is ok in this case.
… lattice (WebAssembly#6035) In general, the operation of taking the least upper bound of two lattice elements may depend on some state stored in the lattice object rather than in the elements themselves. To avoid forcing the elements to be larger and more complicated in that case (by storing a parent pointer back to the lattice), move the least upper bound operation to make it a method of the lattice rather than the lattice element. This is also more consistent with where we put e.g. the `compare` method. While we are at it, rename `makeLeastUpperBound` to `join`, which is much shorter and nicer. Usually we avoid using esoteric mathematical jargon like this, but "join" as a normal verb actually describes the operation nicely, so I think it is ok in this case.
In general, the operation of taking the least upper bound of two lattice
elements may depend on some state stored in the lattice object rather than in
the elements themselves. To avoid forcing the elements to be larger and more
complicated in that case (by storing a parent pointer back to the lattice), move
the least upper bound operation to make it a method of the lattice rather than
the lattice element. This is also more consistent with where we put e.g. the
comparemethod.While we are at it, rename
makeLeastUpperBoundtojoin, which is muchshorter and nicer. Usually we avoid using esoteric mathematical jargon like
this, but "join" as a normal verb actually describes the operation nicely, so I
think it is ok in this case.