Re: [Chicken-hackers] [PATCH] Removed few usages of gcc extensions from runtime
On Wed, Oct 22, 2014 at 12:36:35AM +0400, Oleg Kolosov wrote:
> Hello.
>
> Here is another patch which is useful for MSVC support but not harmful anyway.
Hi there,
This patch looks like a sensible thing to do. It's pretty
weird/unexpected to me that all of GCC, clang and SunW (and I think
pcc, too) even accept this!
Anyway, two notes:
- You used four spaces to indent the second level of indentation,
but 2 spaces for the first level (which is standard in our code).
I've fixed that to use 2 spaces everywhere.
- for (x; y; ) just "looks" wrong to me, like someone forgot to put
an increment operation there. It's more of a style thing, and
it doesn't matter, and you can easily see that the increment happens
on the next line, but it does require a little bit of extra attention.
So I changed it to move the increment back into the for loop. It is
semantically exactly the same, and looks more balanced. I used the
pre-increment operator purely for consistency with the surrounding
code.
Attached is a signed-off copy with the mentioned changes.
Thank you for the patch!
Cheers,
Peter
--
http://www.more-magic.net
>From bf197bbc54dec69cef22aefdfb4f7d5844aba564 Mon Sep 17 00:00:00 2001
From: Oleg Kolosov
Date: Tue, 21 Oct 2014 22:56:28 +0400
Subject: [PATCH] Removed few usages of gcc extensions from runtime
Moved calls to 'mark' and 'remark' macros into the corresponding 'for'
loop bodies. These macros are expanded into a do-while statement which is
not allowed in the loop expression position inside a 'for' loop in
standard C. In particular, MSVC compiler considers this a syntax error.
Signed-off-by: Peter Bex
---
runtime.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/runtime.c b/runtime.c
index fd3a3be..fb5d56c 100644
--- a/runtime.c
+++ b/runtime.c
@@ -2797,11 +2797,13 @@ C_regparm void C_fcall C_reclaim(void *trampoline, void
*proc)
/* Mark literal frames: */
for(lfn = lf_list; lfn != NULL; lfn = lfn->next)
- for(i = 0; i < lfn->count; mark(&lfn->lf[ i++ ]));
+ for(i = 0; i < lfn->count; ++i)
+mark(&lfn->lf[i]);
/* Mark symbol tables: */
for(stp = symbol_table_list; stp != NULL; stp = stp->next)
- for(i = 0; i < stp->size; mark(&stp->table[ i++ ]));
+ for(i = 0; i < stp->size; ++i)
+mark(&stp->table[i]);
/* Mark collectibles: */
for(msp = collectibles; msp < collectibles_top; ++msp)
@@ -2816,14 +2818,16 @@ C_regparm void C_fcall C_reclaim(void *trampoline, void
*proc)
}
else {
/* Mark mutated slots: */
-for(msp = mutation_stack_bottom; msp < mutation_stack_top; mark(*(msp++)));
+for(msp = mutation_stack_bottom; msp < mutation_stack_top; ++msp)
+ mark(*msp);
}
/* Clear the mutated slot stack: */
mutation_stack_top = mutation_stack_bottom;
/* Mark live values: */
- for(p = C_temporary_stack; p < C_temporary_stack_bottom; mark(p++));
+ for(p = C_temporary_stack; p < C_temporary_stack_bottom; ++p)
+mark(p);
/* Mark trace-buffer: */
for(tinfo = trace_buffer; tinfo < trace_buffer_limit; ++tinfo) {
@@ -3299,11 +3303,13 @@ C_regparm void C_fcall C_rereclaim2(C_uword size, int
double_plus)
/* Mark literal frames: */
for(lfn = lf_list; lfn != NULL; lfn = lfn->next)
-for(i = 0; i < lfn->count; remark(&lfn->lf[ i++ ]));
+for(i = 0; i < lfn->count; ++i)
+ remark(&lfn->lf[i]);
/* Mark symbol table: */
for(stp = symbol_table_list; stp != NULL; stp = stp->next)
-for(i = 0; i < stp->size; remark(&stp->table[ i++ ]));
+for(i = 0; i < stp->size; ++i)
+ remark(&stp->table[i]);
/* Mark collectibles: */
for(msp = collectibles; msp < collectibles_top; ++msp)
@@ -3318,7 +3324,8 @@ C_regparm void C_fcall C_rereclaim2(C_uword size, int
double_plus)
mutation_stack_top = mutation_stack_bottom;
/* Mark live values: */
- for(p = C_temporary_stack; p < C_temporary_stack_bottom; remark(p++));
+ for(p = C_temporary_stack; p < C_temporary_stack_bottom; ++p)
+remark(p);
/* Mark locative table: */
for(i = 0; i < locative_table_count; ++i)
--
1.7.10.4
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers
Re: [Chicken-hackers] [PATCH][5] Move foldable binding annotations into types.db
On Tue, Sep 30, 2014 at 12:33:21AM -0700, Evan Hanson wrote: > Hi all, > > The attached patch moves foldable binding annotations out of > c-platform.scm and into types.db. > > While looking into #986[1], I realized there are currently several > library procedures that are marked foldable when they really shouldn't > be (such as the example in the ticket itself!). Hi Evan, I finally made some time to look into this (sorry for the delay!), and I've pushed the changes. Thanks a lot for this improvement! However, I wondered about one thing: why did you decide to make string and list procedures non-foldable? I understand that the spec says literals shouldn't be mutated because they may share storage, but in CHICKEN this never happens, AFAIK (Felix, anyone, please correct me if I'm wrong!). Since it never happens, it may be a good idea to allow string and list constants to be folded anyway. In any case, right now there's an inconsistency: number->string, ##sys#fixnum->string and symbol->string are marked foldable, whereas (for example), list->string and blob->string aren't. Whichever way we decide to approach this, that inconsistency should be resolve, I think. Cheers, Peter -- http://www.more-magic.net ___ Chicken-hackers mailing list Chicken-hackers@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-hackers
[Chicken-hackers] [PATCH][4][5] Fix "missing distribution-files" warning and fix manifest
Hi all, After applying the last of Evan's patches for CHICKEN 5, I thought it would be a good idea to make a snapshot tarball because it requires going through chicken-boot in order to build. I tagged and built the distribution file and as a final sanity check I extracted and built from the tarball. To my surprise, I got this error message: make: *** No rule to make target `c-platform.scm', needed by `c-platform.c'. Stop. Of course, c-platform.scm exists, so that made no sense to me. Looking through the distribution manifest, it became clear that the patch to rename core CHICKEN modules accidentally renamed a few too many files, causing this file to be omitted from the distribution tarball. The makedist.scm script is supposed to warn when it fails to find files in the manifest, but it didn't do that! The reason why is pretty simple: it set! the filename to the list of the filename consed onto the missing files. Instead, it should have set! missing. The first of the attached patches fixes this, using foldl instead (the ones for CHICKEN 4 and 5 are identical). The second patch fixes the broken file listing. It appears there have been a few more undetected missing files that snuck in over the years! (there's a difference between the patches for CHICKEN 4 and 5 here) I'm not sure whether the warning should be an error... Perhaps when making a dev snapshot it's acceptable to have missing files (built HTML manual, maybe?). In any case, the warning is the very last thing that "make dist" shows, so I guess at least for now it's clear enough when this happens. Making a release is still something we do manually, so the developer who makes the release will hopefully spot it and fix it. Cheers, Peter -- http://www.more-magic.net >From 7ce33ef0d76bb23fcd598af965362b12aff124c7 Mon Sep 17 00:00:00 2001 From: Peter Bex Date: Sun, 16 Nov 2014 13:32:05 +0100 Subject: [PATCH 1/2] Fix broken missing file detection in distribution tarball generator --- scripts/makedist.scm | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/scripts/makedist.scm b/scripts/makedist.scm index 6108a8e..9db3cb7 100644 --- a/scripts/makedist.scm +++ b/scripts/makedist.scm @@ -55,13 +55,14 @@ (print "creating " d) (create-directory d 'with-parents (delete-duplicates (filter-map prefix files) string=?)) -(let ((missing '())) - (for-each - (lambda (f) -(if (file-exists? f) -(run (cp -p ,(qs f) ,(qs (make-pathname distname f -(set! f (cons f missing - files) +(let ((missing + (foldl (lambda (missing f) + (cond +((file-exists? f) + (run (cp -p ,(qs f) ,(qs (make-pathname distname f + missing) +(else (cons f missing + '() files))) (unless (null? missing) (warning "files missing" missing) ) ) (run (tar cfz ,(conc distname ".tar.gz") ,distname)) -- 1.7.10.4 >From 4111b5f45c25d1221fd06ec88e72beff4d36e31a Mon Sep 17 00:00:00 2001 From: Peter Bex Date: Sun, 16 Nov 2014 14:00:11 +0100 Subject: [PATCH 2/2] Fix missing/wrong filenames in distribution manifest, as detected by the modified "dist" script --- distribution/manifest |5 - 1 file changed, 5 deletions(-) diff --git a/distribution/manifest b/distribution/manifest index 5a36068..1285858 100644 --- a/distribution/manifest +++ b/distribution/manifest @@ -1,4 +1,3 @@ -INSTALL LICENSE NEWS README @@ -39,7 +38,6 @@ stub.c support.c tcp.c utils.c -build.scm buildversion buildbranch c-backend.scm @@ -138,7 +136,6 @@ tests/port-tests.scm tests/test-gc-hooks.scm tests/test-glob.scm tests/matchable.scm -tests/match-tests.scm tests/module-tests.scm tests/module-tests-2.scm tests/test-finalizers.scm @@ -252,8 +249,6 @@ chicken.import.scm chicken.import.c foreign.import.scm foreign.import.c -compiler.import.scm -compiler.import.c lolevel.import.scm srfi-1.import.scm srfi-4.import.scm -- 1.7.10.4 >From 53d880a924b983eee8f63f72f9821a2050e72d59 Mon Sep 17 00:00:00 2001 From: Peter Bex Date: Sun, 16 Nov 2014 13:32:05 +0100 Subject: [PATCH 1/2] Fix broken missing file detection in distribution tarball generator --- scripts/makedist.scm | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/scripts/makedist.scm b/scripts/makedist.scm index d3891a1..63dfcdf 100644 --- a/scripts/makedist.scm +++ b/scripts/makedist.scm @@ -55,13 +55,14 @@ (print "creating " d) (create-directory d 'with-parents (delete-duplicates (filter-map prefix files) string=?)) -(let ((missing '())) - (for-each - (lambda (f) -(if (file-exists? f) -(run (cp -p ,(qs f) ,(qs (make-pathname distname f -(set! f (cons f missing - files) +(let ((missing + (foldl (lambda (missing f) +
