Status Update, Elisp Compiler
Hi Andy, I'm off from tomorrow until Sunday, and as GSoC seems to 'officially' have ended, I wanted to give you a brief update on my status and plans: Well, as to the status of the elisp compiler, you probably know -- it basically works, just missing are a lot of built-ins and some special stuff. I do of course plan to continue working on it, though, no matter if GSoC is officially finished (hey, I did start late, so its just fair :D). My definite plans to when I'm back are 1) Working further on the elisp reader I've just started (but nothing useful so far) to work on, as Ludo suggested I'll do it in Scheme and look into SILex / (text parse/lalr), but am not sure what of these I'll use or just hand-write. 2) Creating some kind of documentation that is more than my current, vague README-file (and the comments in the source code). 3) Implementing some more advanced stuff, advicing functions comes to mind (I've just never cared so far, I think this should be not too hard by creating some lambdas on the fly or the like). 4) I've not done anything yet regarding converting '() -> %nil in lists that are seen from elisp; I think the final conclusion was that we want such a conversion, and so I will do that. But maybe I can try if it is reasonably possible to allow switching it off to regain performance without? Maybe also allow switching off the #f -> %nil conversion for booleans (t = #t). 5) Other stuff that may come up at some time, like fixing bugs (of course), working on performance improvements (I bet a lot are still possible without going far) or helping with conversion to guile-emacs from the guile side. This of course depends on the time I'll have to spend at the respective time and that might be not much during the autumn/winter because of university, but we'll see. And at least a little should always be possible ;) Yours, Daniel PS: I just remembered that the Elisp compiler still uses #f as %nil, although it can be switched easily at a single place. What's the status of the patch fixing %nil false-ness? -- Done: Arc-Bar-Cav-Ran-Rog-Sam-Tou-Val-Wiz To go: Hea-Kni-Mon-Pri
On white-box tests
Hello!
Just a note that I've been meaning to send for some time.
"Michael Gran" writes:
> http://git.savannah.gnu.org/cgit/guile.git/commit/?id=9b0c25f6d18d5be318ea3a47fd87cf7e63b689e1
[...]
> --- a/test-suite/tests/strings.test
> +++ b/test-suite/tests/strings.test
[...]
> +;;
> +;; string internals
> +;;
> +
> +;; Some abbreviations
> +;; BMP - Basic Multilingual Plane (codepoints below U+)
> +;; SMP - Suplementary Multilingual Plane (codebpoints from U+1 to
> U+1)
> +
> +(with-test-prefix "string internals"
> +
> + (pass-if "new string starts at 1st char in stringbuf"
> +(let ((s "abc"))
> + (= 0 (assq-ref (%string-dump s) 'start
> +
> + (pass-if "length of new string same as stringbuf"
> +(let ((s "def"))
> + (= (string-length s) (assq-ref (%string-dump s) 'stringbuf-length
> +
> + (pass-if "contents of new string same as stringbuf"
> +(let ((s "ghi"))
> + (string=? s (assq-ref (%string-dump s) 'stringbuf-chars
> +
> + (pass-if "writable strings are not read-only"
> +(let ((s "zyx"))
> + (not (assq-ref (%string-dump s) 'read-only
> +
> + (pass-if "read-only strings are read-only"
> +(let ((s (substring/read-only "zyx" 0)))
> + (assq-ref (%string-dump s) 'read-only)))
> +
> + (pass-if "null strings are inlined"
> +(let ((s ""))
> + (assq-ref (%string-dump s) 'stringbuf-inline)))
First of all, thanks for taking the time to write unit tests!
I'm not fully convinced by some of these "string internals" tests,
though. These are "white box tests". I believe these tests are mostly
useful when written by someone different than the one who implemented
the code under test, both of whom following a given specification. When
someone writes both the code and the white box tests, I'm afraid they
end up writing the same code twice, just differently. For example:
SCM
scm_i_substring_read_only (SCM str, size_t start, size_t end)
{
[...]
return scm_double_cell (RO_STRING_TAG, /* <--- read-only */
SCM_UNPACK(buf),
(scm_t_bits)str_start + start,
(scm_t_bits) end - start);
}
and:
(pass-if "read-only strings are read-only"
(let ((s (substring/read-only "zyx" 0)))
(assq-ref (%string-dump s) 'read-only)))
Thus, I think such tests don't provide much information.
Conversely, for this example, a black-box test of the public API would
have helped catch regressions and non-conformance issues:
(pass-if-exception exception:read-only-string
(string-set! (substring/read-only "zyx" 0) 1 #\x))
Another issue is that these tests expose a lot of the implementation
details, e.g.:
(pass-if "short Latin-1 encoded strings are inlined"
(let ((s "m"))
(assq-ref (%string-dump s) 'stringbuf-inline)))
The inline/outline distinction is an implementation detail. If we
change it (that's something we could get rid of in the BDW-GC branch,
for instance) then the tests will have to be rewritten or removed.
Conversely, black box tests can give confidence that a change in
implementation details did have any user-visible impact.
There *are* situations where the gap between the public API and the
internals is too important, and white box tests can come in handy here
(that's why `%string-dump' et al. are nice tools). However, often, I
think it's better to have good coverage of the public API than a wealth
of "obvious" white-box tests.
What do you think?
Thanks,
Ludo'.
Re: On white-box tests
On Wed, 2009-08-19 at 10:38 +0200, Ludovic Courtès wrote: > Hello! > > Just a note that I've been meaning to send for some time. > > "Michael Gran" writes: > > > http://git.savannah.gnu.org/cgit/guile.git/commit/?id=9b0c25f6d18d5be318ea3a47fd87cf7e63b689e1 > > [...] > > I'm not fully convinced by some of these "string internals" tests, > though. These are "white box tests". I believe these tests are mostly > useful when written by someone different than the one who implemented > the code under test, both of whom following a given specification. When > someone writes both the code and the white box tests, I'm afraid they > end up writing the same code twice, just differently. For example: I know what your saying. I was just a kid with a new toy. In my line of work, where 100% code coverage of test suites is a common requirement, we end up writing many such overly obvious tests. They are often the only way to hit some internal branches. They are often pointless. > What do you think? Keep it or dump it. It's all good. > > Thanks, > Ludo'. Thanks, Mike
Re: On white-box tests
Mike Gran writes: > On Wed, 2009-08-19 at 10:38 +0200, Ludovic Courtès wrote: [...] >> What do you think? > > Keep it or dump it. It's all good. Just to be clear: I think we can keep them, it doesn't hurt. I just wanted to hear what you and others thought about the issue, because I think unit tests are a crucial part of software development. Thanks, Ludo'.
apparenty SCM_I_CHAR is fixed, but:
From my autobuild on master cc1: warnings being treated as errors In file included from vm-engine.c:133, from vm.c:322: vm-i-scheme.c: In function 'vm_regular_engine': vm-i-scheme.c:437: warning: comparison is always true due to limited range of data type vm-i-scheme.c:437: warning: comparison is always true due to limited range of data type vm-i-scheme.c:439: warning: comparison is always true due to limited range of data type vm-i-scheme.c:439: warning: comparison is always true due to limited range of data type In file included from vm-engine.c:133, from vm.c:330: vm-i-scheme.c: In function 'vm_debug_engine': vm-i-scheme.c:437: warning: comparison is always true due to limited range of data type vm-i-scheme.c:437: warning: comparison is always true due to limited range of data type vm-i-scheme.c:439: warning: comparison is always true due to limited range of data type vm-i-scheme.c:439: warning: comparison is always true due to limited range of data type gmake[3]: *** [libguile_la-vm.lo] Error 1 gmake[3]: Leaving directory `/home/gdt/BUILD-GUILE-master/guile/libguile' gmake[2]: *** [all] Error 2 gmake[2]: Leaving directory `/home/gdt/BUILD-GUILE-master/guile/libguile' gmake[1]: *** [all-recursive] Error 1 gmake[1]: Leaving directory `/home/gdt/BUILD-GUILE-master/guile' gmake: *** [all] Error 2 pgpJyaHjitFPd.pgp Description: PGP signature
Re: On white-box tests
On Wed, 2009-08-19 at 15:53 +0200, Ludovic Courtès wrote: > Mike Gran writes: > > > On Wed, 2009-08-19 at 10:38 +0200, Ludovic Courtès wrote: > I just wanted to hear what you and others thought about the issue, > because I think unit tests are a crucial part of software development. OK. To say something slightly more cogent. I think when a current phase of development centers around modifying low-level code, it is useful to have a set of low-level tests. If those tests fail, it reminds the developer that s/he has modified something upon which other routines rely. I wrote the string-internals tests to indicate to me when I'd done something that had unexpected side-effects. They are intentionally white-box; they are intentionally reflexive. There is a danger that those tests, should they remain, could be seen as indicating software requirements, which they do not. The software requirement specification for Scheme (RnRS) is high level and leaves much of the implementation detail unspecified. I think it is a good idea to leave them in, probably with comments that express that they test the implementation, not the specification. I also think that it is a good idea to segregate them from tests that exercise the actual software requirements. But, I can see an equally valid argument for stripping them out once strings are no longer in flux, for example at release 2.0, assuming it is bug free ;-) or perhaps 2.1. Thanks, Mike > > Thanks, > Ludo'. > > >
Re: On white-box tests
Mike Gran writes: > I wrote the string-internals tests to indicate to me when I'd done > something that had unexpected side-effects. They are intentionally > white-box; they are intentionally reflexive. OK, I understand now. Then if it's useful to you, well, it's indeed useful. :-) > But, I can see an equally valid argument for stripping them out once > strings are no longer in flux, Makes sense. > for example at release 2.0, assuming it is bug free ;-) or perhaps > 2.1. But I'm sure it'll all be ready by 2.0 or even 1.9.3! :-) Thanks, Ludo'.
Re: apparenty SCM_I_CHAR is fixed, but:
On Wed, 2009-08-19 at 09:56 -0400, Greg Troxel wrote: > From my autobuild on master > > vm-i-scheme.c:437: warning: comparison is always true due to limited range of > data type For gcc, -Wtype-limits will catch this at compile time. Maybe we should put that in the GCC_CFLAGS in the configure.in. -Mike
Re: apparenty SCM_I_CHAR is fixed, but:
Mike Gran writes: > On Wed, 2009-08-19 at 09:56 -0400, Greg Troxel wrote: >> From my autobuild on master >> >> vm-i-scheme.c:437: warning: comparison is always true due to limited range >> of data type > > For gcc, -Wtype-limits will catch this at compile time. IIUC this is precisely a compile-time warning. :-) Ludo'.
Re: apparenty SCM_I_CHAR is fixed, but:
l...@gnu.org (Ludovic Courtès) writes: > Mike Gran writes: > >> On Wed, 2009-08-19 at 09:56 -0400, Greg Troxel wrote: >>> From my autobuild on master >>> >>> vm-i-scheme.c:437: warning: comparison is always true due to limited range >>> of data type >> >> For gcc, -Wtype-limits will catch this at compile time. > > IIUC this is precisely a compile-time warning. :-) Yes, but it's apparently only happening to me, and I think Mike is suggesting adding flags to make it happen for everyone so it's easier to fix. pgpZdHWU1t3xk.pgp Description: PGP signature
Re: apparenty SCM_I_CHAR is fixed, but:
On Wed, 2009-08-19 at 17:25 +0200, Ludovic Courtès wrote: > Mike Gran writes: > > For gcc, -Wtype-limits will catch this at compile time. > > IIUC this is precisely a compile-time warning. :-) It is a compile time warning that is default for Greg, but apparently not for Andy or I. -Mike
