I've been saying for a while[1][2][3] that adding extra abstractions makes
things harder for newcomers, and adding new notations doubly so. And then
I notice this DSL in my own backyard. Makes me feel like a hypocrite.
[1] https://news.ycombinator.com/item?id=13565743#13570092
[2] https://lobste.rs/s/to8wpr/configuration_files_are_canary_warning
[3] https://lobste.rs/s/mdmcdi/little_languages_by_jon_bentley_1986#c_3miuf2
The implementation of the DSL was also highly hacky:
a) It was happening in the tangle/ tool, but was utterly unrelated to tangling
layers.
b) There were several persnickety constraints on the different kinds of
lines and the specific order they were expected in. I kept finding bugs
where the translator would silently do the wrong thing. Or the error messages
sucked, and readers may be stuck looking at the generated code to figure
out what happened. Fixing error messages would require a lot more code,
which is one of my arguments against DSLs in the first place: they may
be easy to implement, but they're hard to design to go with the grain of
the underlying platform. They require lots of iteration. Is that effort
worth prioritizing in this project?
On the other hand, the DSL did make at least some readers' life easier,
the ones who weren't immediately put off by having to learn a strange syntax.
There were fewer quotes to parse, fewer backslash escapes.
Anyway, since there are also people who dislike having to put up with strange
syntaxes, we'll call that consideration a wash and tear this DSL out.
---
This commit was sheer drudgery. Hopefully it won't need to be redone with
a new DSL because I grow sick of backslashes.
Bring back support for incrementally printing the trace to the screen (stderr).
I previously thought I didn't need this as long as I'm always incrementally
saving to the 'last_run' trace file. But I quickly ran into a use for it:
when I want to see a complete trace including switching into the sandbox's
trace and back out again.
So there are now two separate commandline flags:
--trace to save the trace to file
--dump to print the trace to screen
The former won't handle sandbox traces. But the latter will.
I'm deemphasizing --dump in the help message since it should be rarely
used.
One other situation where I've used stderr in the past is for just raw
convenience. But trying to always use the trace was a foolish consistency.
Conclusion:
a) For simple debugging, feel free to just use cout/cerr. Delete them
before committing.
b) If the prints get too complex, switch to the trace and browse_trace
tool.
c) If using nested sandboxes, emit to stderr, redirect to file, and browse_trace.
I've gone back and forth on these questions in the past; now I'm trying
to be a little more rigorous about capturing reasoning.
I've extracted it into a separate binary, independent of my Mu prototype.
I also cleaned up my tracing layer to be a little nicer. Major improvements:
- Realized that incremental tracing really ought to be the default.
And to minimize printing traces to screen.
- Finally figured out how to combine layers and call stack frames in a
single dimension of depth. The answer: optimize for the experience of
`browse_trace`. Instructions occupy a range of depths based on their call
stack frame, and minor details of an instruction lie one level deeper
in each case.
Other than that, I spent some time adjusting levels everywhere to make
`browse_trace` useful.
Clean up the rat's nest that all my trace management globals had
gradually turned into.
a) Get rid of 'Start_tracing'. Horryibly named, I don't know how I
missed that until now.
b) Never use START_TRACING_UNTIL_END_OF_SCOPE in main(). It's
confusing to combine it with atexit(delete Trace_stream), because the
atexit() never has to run. Instead we'll just manually initialize
Trace_stream and let atexit() clean up.
c) If we run tests we only want a trace for the test run itself. So
delete the Trace_stream that was initialized at the top of main --
once it's clear we had no load-time errors.
d) Clean up horribly "Load Recipes" waypoints, combine them with the better
name, "Mu Prelude".
Putting these together, we have the following manual tests:
- CFLAGS=-g mu x.mu
Should not create last_run.
- CFLAGS=-g mu --trace x.mu
Should create last_run.
Should write it out exactly once.
- CFLAGS=-g mu --trace x.mu # when x.mu has an error
Should create last_run.
Should write it out exactly once.
- CFLAGS=-g mu --trace test copy_literal # C test
Should create last_run.
Should write it out exactly once.
- CFLAGS=-g mu --trace test recipe_with_header # Mu test
Should create last_run.
Should write it out exactly once.
I don't know how to automate these scenarios yet. We need a way to run
our build toolchain atop our stack.
The trouble with rewriting 'unused' to '__attribute__(unused)' is that
if we happen to deliberately introduce '__attribute__(unused)' somehow,
say in the standard headers, then it gets expanded twice to '__attribute__(__attribute__(unused))'.
So we switch to a synonym.
Clean up how we reclaim local scopes.
It used to work like this (commit 3216):
1. Update refcounts of products after every instruction, EXCEPT:
a) when instruction is a non-primitive and the callee starts with
'local-scope' (because it's already not decremented in 'return')
OR:
b) when instruction is primitive 'next-ingredient' or
'next-ingredient-without-typechecking', and its result is saved to a
variable in the default space (because it's already incremented at
the time of the call)
2. If a function starts with 'local-scope', force it to be reclaimed
before each return. However, since locals may be returned, *very
carefully* don't reclaim those. (See the logic in the old `escaping`
and `should_update_refcount` functions.)
However, this approach had issues. We needed two separate commands for
'local-scope' (reclaim locals on exit) and 'new-default-space'
(programmer takes charge of reclaiming locals). The hard-coded
reclamation duplicated refcounting logic. In addition to adding
complexity, this implementation failed to work if a function overwrites
default-space after setting up a local-scope (the old default-space is
leaked). It also fails in the presence of continuations. Calling a
continuation more than once was guaranteed to corrupt memory (commit
3986).
After this commit, reclaiming local scopes now works like this:
Update refcounts of products for every PRIMITIVE instruction.
For non-primitive instructions, all the work happens in the `return`
instruction:
increment refcount of ingredients to `return`
(unless -- one last bit of ugliness -- they aren't saved in the
caller)
decrement the refcount of the default-space
use existing infrastructure for reclaiming as necessary
if reclaiming default-space, first decrement refcount of each
local
again, use existing infrastructure for reclaiming as necessary
This commit (finally!) completes the bulk[1] of step 2 of the plan in
commit 3991. It was very hard until I gave up trying to tweak the
existing implementation and just test-drove layer 43 from scratch.
[1] There's still potential for memory corruption if we abuse
`default-space`. I should probably try to add warnings about that at
some point (todo in layer 45).
Standardize exit paths. Most layers now don't need to know about termbox.
We can't really use `assert` in console-mode apps; it can't just exit because
we want to be able to check assertion failures in tests.
This was a large commit, and most of it is a follow-up to commit 3309,
undoing what is probably the final ill-considered optimization I added
to s-expressions in Mu: I was always representing (a b c) as (a b . c),
etc. That is now gone.
Why did I need to take it out? The key problem was the error silently
ignored in layer 30. That was causing size_of("(type)") to silently
return garbage rather than loudly complain (assuming 'type' was a simple
type).
But to take it out I had to modify types_strictly_match (layer 21) to
actually strictly match and not just do a prefix match.
In the process of removing the prefix match, I had to make extracting
recipe types from recipe headers more robust. So far it only matched the
first element of each ingredient's type; these matched:
(recipe address:number -> address:number)
(recipe address -> address)
I didn't notice because the dotted notation optimization was actually
representing this as:
(recipe address:number -> address number)
---
One final little thing in this commit: I added an alias for 'assert'
called 'assert_for_now', to indicate that I'm not sure something's
really an invariant, that it might be triggered by (invalid) user
programs, and so require more thought on error handling down the road.
But this may well be an ill-posed distinction. It may be overwhelmingly
uneconomic to continually distinguish between model invariants and error
states for input. I'm starting to grow sympathetic to Google Analytics's
recent approach of just banning assertions altogether. We'll see..
I had this early on. Why did I delete it? Was it because I was only
flushing the current trace when I started on the next one? We don't have
that problem anymore..
Fix CI.
Figuring out this memory leak was an epic story. I was able to quickly
track down that it was caused by commit 3365, in particular the
overloading of to-text to support characters. But beyond that I was
stumped. Why were layer 3's trace_stream::curr_stream objects being
leaked in layer 81 by a change in layer 59?!
Triaging through the layers, I found the following:
layer 81 - leaks 2 blocks (in clear-line-erases-printed-characters)
layer 83 - leaks 4 additional blocks (in clear-line-erases-printed-characters-2)
I also figured out that the leaks were happening because of the call to
'trace' on a character inside print:character (that's the 'print'
function called with a character argument)
trace 90, [print-character], c
So I tried to create a simple scenario:
scenario trace-on-character-leaks [
1:character <- copy 111/o
trace 90, [print-character], 1:character
]
But that triggered no leaks. Which made sense because there were plenty
of calls to that 'trace' instruction in print:character. The leak only
happened when print:character was called from clear-line. Oh, it happens
only when tracing 0/nul characters. Tracing a Mu string with a nul
character creates an empty C++ string, which is weird. But why should it
leak memory?!
Anyway, I tried a new scenario at layer 62 (when 'trace' starts
auto-converting characters to text)
scenario stashing-nul-character-leaks [
1:character <- copy 0/nul
trace 90, [dbg], 1:character
]
But still, no leak! I played around with running layers until 70, 80.
But then it didn't leak even at layer 82 where I'd seen it leak before.
What had I done?
Turns out it was only leaking if I used names for variables and not
numeric addresses. Eventually I was able to get layer 59 to leak:
scenario stashing-nul-character-leaks [
c:character <- copy 0/nul
x:text <- to-text c
trace 90, [dbg], x
]
At that point I finally went to look at layer 3 (I'd been thinking about
it before, but hadn't bothered to *actually go look*!) And the leak was
obvious.
In the end, all the information I needed was right there in the leak
report. The reason this was hard to find was that I wasn't ready to
believe there could be a bug in layer 3 after all these months. I had to
go through the five stages of grief before I was ready for that
realization.
Final mystery: why was the memory leak not triggered by numeric
variables? Because the transform to auto-convert ingredients to text
only operated on named variables. Manually performing the transform did
leak:
scenario stashing-text-containing-nul-character-leaks [
1:text <- new character:type, 1/capacity
put-index *1:text, 0, 0/nul
trace 90, [dbg], 1:text
]
Undo 3272. The trouble with creating a new section for constants is that
there's no good place to order it since constants can be initialized
using globals as well as vice versa. And I don't want to add constraints
disallowing either side.
Instead, a new plan: always declare constants in the Globals section
using 'extern const' rather than just 'const', since otherwise constants
implicitly have internal linkage (http://stackoverflow.com/questions/14894698/why-does-extern-const-int-n-not-work-as-expected)
Move global constants into their own section since we seem to be having
trouble linking in 'extern const' variables when manually cleaving mu.cc
into separate compilation units.
Clean up the Globals section so that we can generate extern declarations
for all globals out using this command after we carve it out into
globals.cc:
grep ';' globals.cc |perl -pwe 's/[=(].*/;/' |perl -pwe 's/^[^\/# ]/extern $&/' > globals.h
The first perl command strips out initializers. The second prepends
'extern'. This simplistic approach requires each global definition to
lie all on one line.
Deconstruct the tracing layer which had been an exception to our
includes-types-prototypes-globals-functions organization thus far.
To do this we predefine a few primitive globals before the types that
use them, and we pull some method definitions out of struct definitions
at the cost of having to manually write a couple of prototypes.
I started out incredibly lax about running past errors (I even used to
call them 'warnings' when I started Mu), but I've been gradually seeing
the wisdom of Go and Racket in refusing to run code if it doesn't pass
basic integrity checks (such as using a literal as an address).
Go is right to have no warnings, only errors. But where Go goes wrong is
in even caring about unused variables.
Racket and other languages perform more aggressive integrity checks so
that the can optimize more aggressively, and I'm starting to realize I
don't know enough to disagree with them.
When updating refcounts for a typed segment of memory being copied over
with another, we were only ever using the new copy's data to determine
any tags for exclusive containers. Looks like the right way to do
refcounts is to increment and decrement separately.
Hopefully this is a complete fix for the intermittent but
non-deterministic errors we've been encountering while running the edit/
app.
I'd been toying with this idea for some time now given how large the
repo had been growing. The final straw was noticing that people cloning
the repo were having to wait *5 minutes*! That's not good, particularly
for a project with 'tiny' in its description. After purging .traces/
clone time drops to 7 seconds in my tests.
Major issue: some commits refer to .traces/ but don't really change
anything there. That could get confusing :/
Minor issues:
a) I've linked inside commits on GitHub like a half-dozen times online
or over email. Those links are now liable to eventually break. (I seem
to recall GitHub keeps them around as long as they get used at least
once every 60 days, or something like that.)
b) Numbering of commits is messed up because some commits only had
changes to the .traces/ sub-directory.
This is hopefully quite thorough. I handle nested containers, as well as
exclusive containers that might contain addresses only when the tag has
a specific value.
Kartik Agaram