dylan-lang / dylan-emacs-support

Emacs mode for indenting and highlighting Dylan code
GNU General Public License v2.0
27 stars 8 forks source link

Turn optimization-coloring into a minor mode #46

Closed lassik closed 3 years ago

lassik commented 3 years ago

Quoting @housel from https://github.com/dylan-lang/dylan-mode/issues/41#issuecomment-787128909

The dylan-optimization-coloring.el code isn't directly integrated; it can be used on its own to read files generated using the compiler's -dispatch-coloring elisp option.

and https://github.com/dylan-lang/dylan-mode/issues/41#issuecomment-787508821

With optimization-coloring-mode, two files are involved. The main file is the Dylan source file, which starts out using dylan-mode. If you compile using the -dispatch-coloring elisp option, for each source file you get an output file (using elisp syntax) that contains annotations for dispatch coloring (described, in the IDE case, in https://opendylan.org/documentation/getting-started-ide/coloring.html#about-dispatch-optimizations). The dylan-optimization-coloring.el code can be run on an existing dylan-mode buffer to replace the major mode's coloring with coloring based on the output from the compiler.

So yes, there should be a dylan-optimization-coloring-mode, it's just that nobody has ever spent the time to restructure the code as a minor mode.

lassik commented 3 years ago

dylan-optimization-coloring.el is a very long name. How about shortening it to dylan-optimization.el? Here's a sketch of what the definitions' names could be:

face  dylan-optimization-face-not-all-methods-known
face  dylan-optimization-face-failed-to-select-where-all-known
face  dylan-optimization-face-lambda-call
face  dylan-optimization-face-inlining
face  dylan-optimization-face-slot-accessor-fixed-offset
face  dylan-optimization-face-eliminated
face  dylan-optimization-face-dynamic-extent
face  dylan-optimization-face-program-notes
face  dylan-optimization-face-bogus-upgrade

fun   dylan-optimization-file-name
var   dylan-optimization-overlays
fun   dylan-optimization-add-overlays
fun   dylan-optimization-remove-overlays

mode  dylan-optimization-mode
lassik commented 3 years ago

@cgay @housel Any advice on this renaming?

lassik commented 3 years ago

Instead of dylan-optimization we could also simply call it dylan-opt. It would be a good complement to dylan-lid. Emacs has a built-in function regexp-opt for regexp optimization.

The symbols would then be:

face  dylan-opt-face-not-all-methods-known
face  dylan-opt-face-failed-to-select-where-all-known
face  dylan-opt-face-lambda-call
face  dylan-opt-face-inlining
face  dylan-opt-face-slot-accessor-fixed-offset
face  dylan-opt-face-eliminated
face  dylan-opt-face-dynamic-extent
face  dylan-opt-face-program-notes
face  dylan-opt-face-bogus-upgrade

fun   dylan-opt-file-name
var   dylan-opt-overlays
fun   dylan-opt-add-overlays
fun   dylan-opt-remove-overlays

mode  dylan-opt-mode
cgay commented 3 years ago

I'm fine with dylan-opt-*.

lassik commented 3 years ago

It seems the compiler currently emits a .el file which is evaluated as Lisp code by Emacs. Use of eval is often a dodgy coding practice where general-purpose evaluation of arbitrary code is not needed. Could the compiler be modified to simply emit Lisp forms that are data instead of code? dylan-opt.el could then read the compiler's output as data and parse known optimization specs from it.

cgay commented 3 years ago

SGTM. The compiler code lives here: https://github.com/dylan-lang/opendylan/blob/master/sources/project-manager/projects/implementation.dylan#L876 If you're not up for making that change let me know and I can do it.

(The IDE gets this data directly from the compiler database so it won't be affected.)

cgay commented 3 years ago

In fact, we could use the current .el files without calling eval since their format is so simple.

lassik commented 3 years ago

Great. Perusing dump-source-record-emacs-dispatch-colors in that code, it writes out (start-line start-column end-line end-column) tuples to indicate all the regions that should receive each kind of coloring.

There's a file format version comment ; HQNDYLANCOLRINFO 1 0 at the top. If we change the file format, we should take this opportunity to use a real Lisp form instead of a comment to state the format version. If we make the file format indicator a string, it can be a URL that leads to a web page explaining the format.

How about a format like this:

(file-format "opendylan.org/whatever#2021-03-07")
(region <start-line> <start-column> <end-line> <end-column> <type>)
(region <start-line> <start-column> <end-line> <end-column> <type>)
(region <start-line> <start-column> <end-line> <end-column> <type>)
...

Currently the compiler seems to output the regions in the order they appear in the file, whereas the old ELisp optimization coloring thing expects regions to be sorted by type, and then each type sorted by position in the file. As a result, it opens and closes region types several times as it goes through the file.

It would be easier to forgo the sorting by region type, and just write out a tuple for each region as above as the compiler comes across them.

The code distinguishes foreground and background colors, but the ELisp side doesn't do anything with backgrounds currently; the function dealing with background colors was empty. We don't have to be concerned about colors, foregrounds, and backrounds in the emitted; we can simply state the abstract type of each region instead of a concrete color.

lassik commented 3 years ago

Emacs can also read binary files with not much difficulty, if you prefer those.

lassik commented 3 years ago

In fact, we could use the current .el files without calling eval since their format is so simple.

True. Can you give me a sample Dylan file and corresponding optimization file to test with?

housel commented 3 years ago

system-el.zip These are the .el files generated by compiling the system library (https://github.com/dylan-lang/opendylan/tree/master/sources/system) on macOS.

lassik commented 3 years ago

Thank you! I just pushed a PR to make the parser stricter and turn this into a real minor mode.

Now one can do M-x dylan-opt to load the optimization info from a file and show it. Toggling M-x dylan-opt-mode shows and hides the info.

lassik commented 3 years ago

Now that PR #54 is merged, this issue has been solved as far as I can tell.