Hi Paul, Great to see you here!
> Le 15 août 2018 à 18:34, Paul Eggert <[email protected]> a écrit : > > Akim Demaille wrote: >> will C stop requiring >> void some day? > > It’s not likely any time soon, since K&R-style function definitions are still > allowed in C11 and even if they're removed it'll likely require two > iterations of the standard to do it. Bummer… >> + argmatch assert assume > > 'assume'? There is no assume module in gnulib; the 'assume' macro is defined > by verify.h, in the 'verify' module. If reader.c uses ‘assume' it should > include verify.h. Good catch! >> static symbol * >> -find_start_symbol () >> +find_start_symbol (void) >> { >> symbol_list *res = grammar; >> for (; >> - res != NULL && symbol_is_dummy (res->content.sym); >> + res && symbol_is_dummy (res->content.sym); >> res = res->next) >> { >> for (res = res->next; >> - res != NULL && res->content.sym != NULL; >> + res && res->content.sym; >> res = res->next) >> continue; >> - aver (res != NULL); >> + assume (res); >> } >> - aver (res != NULL); >> + assume (res); >> return res->content.sym; >> } > > This doesn't look right, as 'assume (X)' is an instruction from the > programmer to the compiler, saying "I know that X is nonzero even though you > don't, so compile the code assuming that X is nonzero even if the compiled > code would be incorrect if X were zero." And yet here, you have a for loop > that explicitly exits the loop if res is null, and then says 'assume (res)'. > Either the test for res is wrong, or the assume is wrong. > > If you already know that the start symbol is in the grammar, then write the > code that way; there is no need to test whether res is nonzero. That way, you > will probably be able to pacify GCC without using ‘assume' at all. You are 100% right, we should be ‘less cautious’ as it allows the compiler to understand better what we assume. Thanks for the tip! What do you think about this? commit fab0bf6b521bd524751605d263dd6bd7e2426099 Author: Akim Demaille <[email protected]> Date: Wed Aug 15 21:03:47 2018 +0200 reader: simplify the search of the start symbol Suggested by Paul Eggert. * src/reader.c (find_start_symbol): Don't check 'res', we know it is not null. That suffices to avoid the GCC warnings. * bootstrap.conf: We don't need 'assume', which doesn't exist anyway. diff --git a/bootstrap.conf b/bootstrap.conf index b2d0f974..7c436a59 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -17,7 +17,7 @@ # gnulib modules used by this package. gnulib_modules=' - argmatch assert assume + argmatch assert calloc-posix close closeout config-h c-strcase configmake dirname diff --git a/src/reader.c b/src/reader.c index 12faef96..664dea71 100644 --- a/src/reader.c +++ b/src/reader.c @@ -729,17 +729,11 @@ static symbol * find_start_symbol (void) { symbol_list *res = grammar; - for (; - res && symbol_is_dummy (res->content.sym); - res = res->next) - { - for (res = res->next; - res && res->content.sym; - res = res->next) - continue; - assume (res); - } - assume (res); + /* Skip all the possible dummy rules of the first rule. */ + for (; symbol_is_dummy (res->content.sym); res = res->next) + /* Skip the LHS, and then all the RHS of the dummy rule. */ + for (res = res->next; res->content.sym; res = res->next) + continue; return res->content.sym; }
