From: Perry E. M. <pe...@pi...> - 2004-03-30 04:16:05
|
Here are some more warnings fixes. Most are fairly trivial, though I've found at least one dead function and at least one place where a function returning a pointer was mistakenly implicitly declared as integer. If we can get it to the point where everything here is gcc clean, perhaps I can lint the whole thing, but that might be some ways off. By the way, I note that the C style in the runtime is extremely inconsistent -- several very different styles are used. Would it be a problem for this to be made consistent? (I'd even volunteer, but...) Perry Index: src/runtime/gc-common.c =================================================================== RCS file: /cvsroot/sbcl/sbcl/src/runtime/gc-common.c,v retrieving revision 1.12 diff -u -r1.12 gc-common.c --- src/runtime/gc-common.c 9 Jan 2004 10:38:48 -0000 1.12 +++ src/runtime/gc-common.c 30 Mar 2004 03:24:55 -0000 @@ -395,6 +395,7 @@ return nwords; } +#ifndef LISP_FEATURE_GENCGC static int scav_return_pc_header(lispobj *where, lispobj object) { @@ -403,6 +404,7 @@ (unsigned long) object); return 0; /* bogus return value to satisfy static type checking */ } +#endif static lispobj trans_return_pc_header(lispobj object) @@ -447,6 +449,7 @@ } #endif +#ifndef LISP_FEATURE_GENCGC static int scav_fun_header(lispobj *where, lispobj object) { @@ -455,6 +458,7 @@ (unsigned long) object); return 0; /* bogus return value to satisfy static type checking */ } +#endif static lispobj trans_fun_header(lispobj object) Index: src/runtime/gencgc.c =================================================================== RCS file: /cvsroot/sbcl/sbcl/src/runtime/gencgc.c,v retrieving revision 1.50 diff -u -r1.50 gencgc.c --- src/runtime/gencgc.c 30 Jan 2004 20:56:06 -0000 1.50 +++ src/runtime/gencgc.c 30 Mar 2004 03:24:59 -0000 @@ -295,6 +296,7 @@ return count; } +#if QSHOW /* Count the number of dont_move pages. */ static int count_dont_move_pages(void) @@ -308,6 +310,7 @@ } return count; } +#endif /* Work through the pages and add up the number of bytes used for the * given generation. */ @@ -1680,7 +1683,8 @@ return copy_large_object(object, length); } - +/* FIXME: Not obviously used anywhere... */ +#ifdef REMOVEME static lispobj trans_unboxed_large(lispobj object) { @@ -1696,6 +1700,7 @@ return copy_large_unboxed_object(object, length); } +#endif /* @@ -3518,8 +3523,8 @@ for_each_thread(th) { void **ptr; void **esp=(void **)-1; - int i,free; #ifdef LISP_FEATURE_SB_THREAD + int i,free; if(th==arch_os_get_current_thread()) { esp = (void **) &raise; } else { Index: src/runtime/interrupt.c =================================================================== RCS file: /cvsroot/sbcl/sbcl/src/runtime/interrupt.c,v retrieving revision 1.56 diff -u -r1.56 interrupt.c --- src/runtime/interrupt.c 20 Feb 2004 18:15:20 -0000 1.56 +++ src/runtime/interrupt.c 30 Mar 2004 03:25:00 -0000 @@ -612,8 +612,10 @@ extern void post_signal_tramp(void); void arrange_return_to_lisp_function(os_context_t *context, lispobj function) { +#ifndef LISP_FEATURE_X86 void * fun=native_pointer(function); - char *code = &(((struct simple_fun *) fun)->code); + void *code = &(((struct simple_fun *) fun)->code); +#endif /* Build a stack frame showing `interrupted' so that the * user's backtrace makes (as much) sense (as usual) */ Index: src/runtime/purify.c =================================================================== RCS file: /cvsroot/sbcl/sbcl/src/runtime/purify.c,v retrieving revision 1.37 diff -u -r1.37 purify.c --- src/runtime/purify.c 20 Feb 2004 18:15:20 -0000 1.37 +++ src/runtime/purify.c 30 Mar 2004 03:25:01 -0000 @@ -113,7 +113,8 @@ #endif } -static inline newspace_alloc(int nwords, int constantp) +static inline lispobj * +newspace_alloc(int nwords, int constantp) { lispobj *ret; nwords=CEILING(nwords,2); @@ -602,7 +603,7 @@ vector = (struct vector *)native_pointer(thing); nwords = 2 + (CEILING((fixnum_value(vector->length)+extra)*bits,32)>>5); - new=newspace_alloc(nwords, (constant || !boxed)); + new = newspace_alloc(nwords, (constant || !boxed)); bcopy(vector, new, nwords * sizeof(lispobj)); result = make_lispobj(new, lowtag_of(thing)); @@ -856,7 +857,7 @@ struct cons *old, *new, *orig; int length; - orig = newspace_alloc(0,constant); + orig = (lispobj *)newspace_alloc(0,constant); length = 0; do { Index: src/runtime/thread.c =================================================================== RCS file: /cvsroot/sbcl/sbcl/src/runtime/thread.c,v retrieving revision 1.23 diff -u -r1.23 thread.c --- src/runtime/thread.c 29 Jan 2004 04:09:53 -0000 1.23 +++ src/runtime/thread.c 30 Mar 2004 03:25:02 -0000 @@ -203,7 +204,8 @@ th->pid=kid_pid; /* child will not start until this is set */ } -pid_t create_initial_thread(lispobj initial_function) { +void +create_initial_thread(lispobj initial_function) { struct thread *th=create_thread_struct(initial_function); pid_t kid_pid=getpid(); if(th && kid_pid>0) { |
From: Christophe R. <cs...@ca...> - 2004-03-30 12:30:38
|
"Perry E. Metzger" <pe...@pi...> writes: > Here are some more warnings fixes. Most are fairly trivial, though > I've found at least one dead function and at least one place where a > function returning a pointer was mistakenly implicitly declared as > integer. Thank you. Most of this will shortly be merged as sbcl-0.8.9.10 (though some of the LISP_FEATURE_GENCGC conditionals have been changed to LISP_FEATURE_X86). The ones I'm commenting on below probably won't be, though they're for further consideration. > +/* FIXME: Not obviously used anywhere... */ > +#ifdef REMOVEME > static lispobj > trans_unboxed_large(lispobj object) > { Although it appears we've never used trans_unboxed_large, it is perhaps worth pointing out before zapping it that CMUCL uses it in gencgc (but not the Cheney GC) to transport bignums. Can anyone think of an advantage to this? I suppose that for really large bignums (such as those involved in computing (fact 50000) it's a win not to copy them all over the place. Paging GC experts... > +static inline lispobj * > +newspace_alloc(int nwords, int constantp) > { > [...] > - orig = newspace_alloc(0,constant); > + orig = (lispobj *)newspace_alloc(0,constant); > length = 0; Given the first portion of this patch, the second is redundant, is it not? Cheers, Christophe -- http://www-jcsu.jesus.cam.ac.uk/~csr21/ +44 1223 510 299/+44 7729 383 757 (set-pprint-dispatch 'number (lambda (s o) (declare (special b)) (format s b))) (defvar b "~&Just another Lisp hacker~%") (pprint #36rJesusCollegeCambridge) |
From: Perry E. M. <pe...@pi...> - 2004-03-30 12:41:23
|
Christophe Rhodes <cs...@ca...> writes: > Although it appears we've never used trans_unboxed_large, it is > perhaps worth pointing out before zapping it that CMUCL uses it in > gencgc (but not the Cheney GC) to transport bignums. Maybe it should just be #ifdef'ed out then, with a comment to that effect, in case anyone wants it back in the future. It should not be compiled by default, though. Perry |
From: Perry E. M. <pe...@pi...> - 2004-03-30 12:45:03
|
Christophe Rhodes <cs...@ca...> writes: > Thank you. Most of this will shortly be merged as sbcl-0.8.9.10 > (though some of the LISP_FEATURE_GENCGC conditionals have been changed > to LISP_FEATURE_X86). FYI, I used that conditional because all the calls that called the code I was #ifdefing were conditionalized on LISP_FEATURE_GENCGC, so I wanted it to be symmetric. If this should really be X86 conditionalized, perhaps the callers should also be conditionalized on that instead of LISP_FEATURE_GENCGC. Perry |
From: Perry E. M. <pe...@pi...> - 2004-03-30 12:40:02
|
Christophe Rhodes <cs...@ca...> writes: >> +static inline lispobj * >> +newspace_alloc(int nwords, int constantp) >> { >> [...] >> - orig = newspace_alloc(0,constant); >> + orig = (lispobj *)newspace_alloc(0,constant); >> length = 0; > > Given the first portion of this patch, the second is redundant, is it > not? orig is not a lispobj * -- it is another kind of pointer -- so the second part is not redundant. I didn't even notice the need for the second until I did the first... -- Perry E. Metzger pe...@pi... |
From: Christophe R. <cs...@ca...> - 2004-03-30 13:44:50
|
"Perry E. Metzger" <pe...@pi...> writes: > Christophe Rhodes <cs...@ca...> writes: >>> +static inline lispobj * >>> +newspace_alloc(int nwords, int constantp) >>> { >>> [...] >>> - orig = newspace_alloc(0,constant); >>> + orig = (lispobj *)newspace_alloc(0,constant); >>> length = 0; >> >> Given the first portion of this patch, the second is redundant, is it >> not? > > orig is not a lispobj * -- it is another kind of pointer -- so the > second part is not redundant. I didn't even notice the need for the > second until I did the first... OK, so now I don't understand; if orig is not a lispobj *, how does casting the return value of newspace_alloc() from a lispobj * to a lispobj * remove warnings. My intuition would have been that an explicit cast to the type of orig would be required, not the null cast. In other words, I can believe that a cast to (struct cons *) would make sense; I can't see how a cast to (lispobj *) helps in any way (apart from maybe quieting gcc a little -- but the code itself seems just as broken, with just the same implicit conversion occurring afterwards as before). Cheers, Christophe -- http://www-jcsu.jesus.cam.ac.uk/~csr21/ +44 1223 510 299/+44 7729 383 757 (set-pprint-dispatch 'number (lambda (s o) (declare (special b)) (format s b))) (defvar b "~&Just another Lisp hacker~%") (pprint #36rJesusCollegeCambridge) |
From: Perry E. M. <pe...@pi...> - 2004-03-30 14:49:19
|
Christophe Rhodes <cs...@ca...> writes: > "Perry E. Metzger" <pe...@pi...> writes: > >> Christophe Rhodes <cs...@ca...> writes: >>>> +static inline lispobj * >>>> +newspace_alloc(int nwords, int constantp) >>>> { >>>> [...] >>>> - orig = newspace_alloc(0,constant); >>>> + orig = (lispobj *)newspace_alloc(0,constant); >>>> length = 0; >>> >>> Given the first portion of this patch, the second is redundant, is it >>> not? >> >> orig is not a lispobj * -- it is another kind of pointer -- so the >> second part is not redundant. I didn't even notice the need for the >> second until I did the first... > > OK, so now I don't understand; if orig is not a lispobj *, how does > casting the return value of newspace_alloc() from a lispobj * to a > lispobj * remove warnings. It doesn't. I'd meant the second to be a (struct cons *). You caught my mistake. :) Perry |