Skip to content

Tcl implementation #106

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

Merged
merged 1 commit into from
Nov 10, 2015
Merged

Tcl implementation #106

merged 1 commit into from
Nov 10, 2015

Conversation

dubek
Copy link
Collaborator

@dubek dubek commented Nov 10, 2015

Full Tcl implementation of mal - passes all tests and self-hosting tests. It requires Tcl 8.6 because I use oo::class. It includes readline support via tclreadline (if available) and very basic Tcl "integration" with the tcl* form (see tcl/tests/stepA_mal.mal for examples).

Memory leak

One issue: this implementation leaks memory - environments are not really deleted. Tcl has no garbage collection, and new objects (in our case, new Env objects) are never deleted unless explicitly destroyed. The following mal program causes the implementation to be killed by the Linux OOM killer:

(def! rec (fn* () (rec)))
(rec)

I noticed this is also a proglem of the C implementation, but does not occur in the Ruby implementation (the Ruby GC kicks into work correctly).

I think this can be solved for Tcl by implementing reference counting over the Env objects, and destroying when the count goes down to zero. Sounds like a headache...

Regression testing

During the development I found it important in some steps to verify that I haven't broken something that was working OK earlier. In this case while implementing TCO in step 5 I broke let* by mistake, but no tests covered it. I wrote a small regression tester (for Tcl - see code below) that runs all the test cases from previous steps (except step0 and step1) against the given step implementation. A better implementation would be in the Makefile with all the info available there.

#!/bin/bash

step=$1

if [ -z $step ] ; then
  echo "Usage: $0 step"
  echo "Example: $0 step9"
  exit 1
fi

step0_regs="step0"
step1_regs="step1"
step2_regs="step2"
step3_regs="$step2_regs step3"
step4_regs="$step3_regs step4"
step5_regs="$step4_regs step5"
step6_regs="$step5_regs step6"
step7_regs="$step6_regs step7"
step8_regs="$step7_regs step8"
step9_regs="$step8_regs step9"
stepA_regs="$step9_regs stepA"

projectdir=$(dirname $0)
cd $projectdir/tcl

testfiles=""
eval "regsteps=\$${step}_regs"
for s in $regsteps ; do
  testfile=$(echo ../tests/$s*.mal)
  testfiles="$testfiles $testfile"
done

../runtest.py <(cat $testfiles) -- tclsh ../tcl/${step}_*

kanaka added a commit that referenced this pull request Nov 10, 2015
@kanaka kanaka merged commit 9bc9601 into kanaka:master Nov 10, 2015
@kanaka
Copy link
Owner

kanaka commented Nov 10, 2015

Yeah, that's a good point about the possibility of regressions and I have thought of that before. Can you actually file a separate bug with that information in it so that I don't lose track of it?

@kanaka
Copy link
Owner

kanaka commented Nov 10, 2015

Oh, and don't forget to tweet your implementation so I can retweet to announce it.

@kanaka
Copy link
Owner

kanaka commented Nov 10, 2015

I was testing the Tcl implementation just now and everything passed except the "^" reader macro in step1:

TEST: ^{"a" 1} [1 2 3] -> ['',(with-meta [1 2 3] {"a" 1})] -> FAIL (line 131):
    Expected : '^{"a" 1} [1 2 3]\r\n(with-meta [1 2 3] {"a" 1})'
    Got      : '^{"a" 1} [1 2 3]\r\nerror'

@kanaka
Copy link
Owner

kanaka commented Nov 10, 2015

Oh, I forgot to answer your point about GC. Obviously that's a really nice to have if you can make it work. There are a few existing implementation that don't have GC: C, bash, GNU make. At some point I plan on adding GC to C using the Boehm GC library which is basically a drop in replacement for malloc. I might even attempt to add some GC to the GNU make implementation once the "undefine" directive is more widespread (GNU Make 3.82 and up). I just haven't gotten around to either of those yet. All that to say, if you're able to get it to work (even if it's only a cleaning up some percentage of memory usage, that's still a win), that's great, but it's not mandatory.

@dubek
Copy link
Collaborator Author

dubek commented Nov 10, 2015

Works for me (:tm:) :

...
Testing read of ^/metadata
TEST: ^{"a" 1} [1 2 3] -> ['',(with-meta [1 2 3] {"a" 1})] -> SUCCESS
...

with:

$ echo 'puts $tcl_version' | tclsh
8.6

@dubek
Copy link
Collaborator Author

dubek commented Nov 10, 2015

Should I add tcl and vimscript to the big tests/docker/Dockerfile ?

@kanaka
Copy link
Owner

kanaka commented Nov 10, 2015

The same with-meta failure happens in Travis: https://travis-ci.org/kanaka/mal/jobs/90243798 That's not really surprising given that travis is using the same docker image that I used to reproduce it (kanaka/mal-test-tcl). Can you see if you can reproduce it using that docker image? Here are the tcl package versions in the image:

libtcl8.6:amd64 8.6.1-6ubuntu1
tcl 8.6.0+6ubuntu3
tcl-tclreadline:amd64 2.1.0-14
tcl8.6 8.6.1-6ubuntu1

Regarding the big dockerfile, I'm actually planning on just dropping that at some point. It results in a multi-gigabyte image that takes forever to build, so I've switched to a per implementation docker file/image model and just try and have them share as many early layers as possible.

@dubek
Copy link
Collaborator Author

dubek commented Nov 10, 2015

Yes I see the issue (previously I was testing on a machine without tclreadline). It seems that the readline interprets ^ at the beginning of the line as a substitution operator on the previous (history) line. Adding a space before that ^ solves it, but I want to disable this history expansion thingy if possible.

@kanaka
Copy link
Owner

kanaka commented Nov 10, 2015

Yeah, usually, some of those more interesting behaviors are opt-in, but I'm not surprised that tcl has some of these on by default. Sometimes there are environment variables that disable this (for example, runtest sets PERL_RL to false in order to disable readline while running tests). Alternately, you could add a --raw option to your implementation that disables use of readline (the mono based implementations end up doing this).

@dubek
Copy link
Collaborator Author

dubek commented Nov 10, 2015

I added --raw switch; see #108 .

micfan pushed a commit to micfan/make-a-lisp that referenced this pull request Nov 8, 2018
luelista pushed a commit to luelista/mal that referenced this pull request Mar 10, 2024
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.

2 participants