Replace cleavir-environment with Trucler#24
Replace cleavir-environment with Trucler#24paulapatience wants to merge 3 commits intos-expressionists:mainfrom
cleavir-environment with Trucler#24Conversation
paulapatience
force-pushed
the
introduce-trucler
branch
from
4fe8188 to
c5cfee7
Compare
paulapatience
force-pushed
the
introduce-trucler
branch
from
c5cfee7 to
fac9a63
Compare
CST-to-AST/convert-special.lisp
Outdated
| ;; FIXME: We probably want to create and use | ||
| ;; env:constantp in case the environments don't match | ||
| ;; up. | ||
| ;; FIXME(paul) What is env:constantp? |
There was a problem hiding this comment.
env:constantp would be a cleavir-env function that's like cl:constantp, only a customizable generic. This is out of scope for Trucler since it could be quite sophisticated in general, e.g. detecting that (let () 4) is a constant. But a minimal implementation would do cl:constantp's minimum, i.e. it can detect self-evaluating objects, quoted forms, and constant variables (which would be found via Trucler). Given that you've deleted cleavir-env, this function would be in cst-to-ast instead.
|
|
||
| ;; FIXME(paul) Does cst-eval here need two environments or only one? | ||
| ;; In the trucler branch of cleavir, it had one only (but two in the | ||
| ;; cleavir-environment package). |
There was a problem hiding this comment.
I believe that eval and cst-eval took one environment as the actual environment argument, and one that was just for specialization. This second predates the use of clients, but now that we are using clients we should do that here too. So, probably cst-eval should take a form, an environment, and a client as arguments (in some order, i guess the convention is client first?)
| (type-expand (next env) type-specifier)) | ||
| ;; FIXME(paul) I don't think this expands repeatedly. | ||
| ;; Then again, what the other one used to do is call TYPE-EXPAND until | ||
| ;; the environment is NIL. |
There was a problem hiding this comment.
Having a default method call the generic function again is a bit dangerous if the client forgets to specialize it. It might be better to have an ordinary type-expand function that calls the generic with the trucler:global-environment to avoid this.
| ;;; | ||
| ;;; Generic function POLICY. | ||
| ;;; | ||
| ;;; FIXME(paul) Add doc? |
There was a problem hiding this comment.
The documentation should include pretty extensive information here, including probably an example of including policy information in an environment so that it does not have to be repeatedly computed. I suspect that the default recomputation will be pretty bad for performance.
| compute policies from optimize declarations, but you should | ||
| avoid doing this on every `OPTIMIZE-INFO` call. | ||
| <!-- FIXME(paul): Does it really have to respect the protocol? | ||
| In what way is it used here? --> |
There was a problem hiding this comment.
this comment makes less sense in a trucler context, i think. it might be enough to say that we use trucler to get optimize info, so if the client wants policies to make sense, they need to implement trucler properly. describe-optimize no longer has to return policy info (though it probably should to avoid recomputation)
Maybe something like...
Make sure your client respects the
OPTIMIZE-DESCRIPTIONprotocol in Trucler. Cleavir expects additional policy information. By default it will compute a policy anew each time it is needed, but it is recommended that clients cache policy information alongside optimize information to avoid this.CLEAVIR-CST-TO-AST:POLICYshould be specialized to retrieve from this cache rather than do computation.
paulapatience
force-pushed
the
introduce-trucler
branch
5 times, most recently
from
7cfef44 to
2812070
Compare
TODO: Review once for possible errors
Removed the dependency on cleavir-environment (in preparation for migrating to Trucler), which involved replacing uses of environments with clients, and moving cleavir-environment:optimize-qualities to here. While here, expanded some tabs to spaces.
paulapatience
force-pushed
the
introduce-trucler
branch
from
2812070 to
f6128f4
Compare
Migrating the features of cleavir-environment missing from Trucler to CST-to-AST.
paulapatience
force-pushed
the
introduce-trucler
branch
from
f6128f4 to
846d371
Compare
2 participants
The first commit (renaming
systemtoclient, etc.) should be done. I would like to check through it once more to make sure I didn't miss anything or introduce bugs. I renamed even instances incleavir-environmentto simplify things, i.e., allsystems were renamed.The second commit (replacing environments with clients in
cleavir-compilation-policy) should also be done.The third commit is the crux of the PR. Everything compiles, but running the BIR visualizer results in some issues I still need to fix. Also, there are several
FIXME(paul)notes that I have to address.