Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BCPL implementation #511

Open
wants to merge 148 commits into
base: master
Choose a base branch
from
Open

BCPL implementation #511

wants to merge 148 commits into from

Conversation

bjh21
Copy link
Contributor

@bjh21 bjh21 commented May 27, 2020

Here is my BCPL implementation of mal. It's almost ready for merging, but there's one problem I'd like some guidance on.

The Cintsys BCPL implementation likes to print a banner at startup, and I've not yet found a way to stop it. This means that starting Mal looks like this:

bjh21@sole:~/mal/impls/bcpl$ ./run

BCPL 32-bit Cintcode System (3 Jan 2019)
0.000> 
Mal [BCPL]
user> 

The main test script, runtest.py is quite happy to allow the implementation to output rubbish before the prompt (at least, once I'd fixed its prompt detection it was), but the ARGV test in step 6 is much stricter. Arguably this discrepancy is reasonable: runtest.py is running the interpreter in non-interactive mode, while run_argv_test.sh is running it on non-interactive mode.

So my question is: should I amend run_argv_test.sh to permit extra output at startup, or should I find a way to stop BCPL producing that output?

@wasamasa
Copy link
Collaborator

You could patch it to not display the message since you're building from source anyway or maybe even suggest the maintainer to add a -q option to supress the banner. Searching for "bit Cintcode System" shows a few code matches, might be sufficient to edit cintcode/sysb/boot.b.

Copy link
Owner

@kanaka kanaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to get BCPL to stop printing this when used for running scripts. I guess nobody else is using BCPL to run scripts then? I think there might be a couple of targets the print some stuff to stderr when used to run scripts. I still think that's bad, but run_argv_test.sh permits it. But always printing version info to stdout with no option to squelch is right out IMO. So I agree with @wasamasa: patch locally and submit ticket upstream.

I've built and pushed a bcpl image to docker hub (with the rm bcpl.tgz addition).

impls/bcpl/Dockerfile Outdated Show resolved Hide resolved
@bjh21
Copy link
Contributor Author

bjh21 commented May 29, 2020

OK. I'm quite happy to use mal as a means of making toy languages (or toy implementations) less so. Also on the agenda: some way for a program to cause GNU APL to exit.

bjh21 added 27 commits June 4, 2020 20:21
Cintsys annoyingly prints its own prompt even when you've given it a
command to execute, and that prompt ends with '>', causing runtest to
think it's the mal prompt.  This gets runtest one prompt out of sync
with reality, with the expected poor consequences.
From experience, I know that 255-byte strings aren't long enough.  Mal
strings therefor use a whole word to represent their lengths, and I'm
using them from the start.
The readline function now returns nil if it hits EOF, though it appears
that Cintsys stops ^D from generating it.
The BCPL compiler tends to generate partial output on error: this makes
sure such partial output is deleted.
Currently, the repl tokenizes the input and then just prints the list of
tokens, which is good enough for testing the tokenizer.
The BCPL book says that SLCT x is equivalent to SLCT x:0:0, but the
current compiler and its documentation make it equivalent to SLCT 0:0:x.
The code assumed that the book was correct, but now makes no such
assumption.
While it passes the tests, this is actually slightly less than minimal
in that it reads numbers as symbols.  Still, it seems to be otherwise
functional.
No printing of integers yet, though.
The variable "tokstart" was being set to the same value in every
branch of the SWITCHON statement (except those where it was never
used), so it may as well be hoisted out of the SWITCHON.  This also
allows the code handling '~@' to be simplified.
The code is now obviously similar to the comment-skipping code just
above it.
Actually, this commit seems to add all support for printing, since I
left out printer.b heretofore.
Vectors are stored as vectors, so they're more space-efficient than
lists, but the 'rest' and 'cons' functions will be expensive.
I think this will be useful for hash-maps, since life is easier if
only scalar types can be hash-map keys.
The error message for not finding something in an environment is wrong,
and we segfault when evaluating a vector.
bjh21 added 27 commits June 4, 2020 20:21
All mandatory step 9 tests now pass, though we seem to have a
regression or two.
Since all three are basically the same function, re-implement them in
terms of a single parameterised function.
It now records values, because I needed that to find a stupid bug.
This is faster and slightly more obvious than checking the type.
This is a little easier to read, supports >256-char input lines, and
doesn't return the trailing newline.  The latter is required by the
tests.
The changes had somehow ended up only in step 4, which meant all the
later steps were broken.
I accidentally passed gc_root when I meant gc_inner_root.
Macro expansion can change the expression currently being evaluated
(that's the point of it, after all), and so it's important that the new
expression be protected against garbage collection.  Forgetting this
leads to subtle problems when the expression gets evaluated.
At the moment it gets exceeded on step 5, because the garbage collector
runs far too often.
A mark-and-sweep garbage-collector has an overhead each run of the
number of live objects, so running too often is expensive.  The
garbage-collector now only runs if the number of objects allocated since
the last run exceeds the number that were live at the end of the last
run.  This is a fairly common heuristic and improves performance
markedly.

Consequently, remove the test timeout adjustment.  It's more than fast
enough now.
Using sys(Sys_quit, 1) is a bad idea because it causes the Cintsys
runtime to drop into a debugger.  Replace it with sys(Sys_quit, 0),
which does at least exit if not with the exit status we'd like.
@kanaka
Copy link
Owner

kanaka commented Apr 18, 2021

Hi @bjh21. Any word on this? I know I've been incommunicado for the past year, but I should be partly back and paying attention now and I would still love to get BCPL into the main tree. I did a rebuild and re-test of the image and it looks like the banner is still a problem.

@kanaka
Copy link
Owner

kanaka commented Aug 7, 2024

@bjh21 It's been a few years. Any interest in pushing this forward or should I close the ticket. It would be great to get a BCPL implementation merged, but I can understand if you are no longer interested/motivated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants