This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] PR18785: Support non-native execution charsets


On Wed, 22 Dec 2004, Roger Sayle wrote:

> It does however prevent issuing incorrect diagnostics when we're unsure
> of the execution character set, which is with this patch, is *only*
> when the user explicitly specifies -fexec-charset (on any platform).

But I think the usual case of -fexec-charset would still be where both 
character sets are supersets of ASCII.

> which when compiled on Linux with mainline CVS, using the command line
> gcc -c -Wall -fexec-charset=IBM1047 warn.c
> 
> generates the inappropriate/incorrect diagnostics:
> warn.c: In function `foo':
> warn.c:5: warning: spurious trailing `%' in format
> warn.c:5: warning: too many arguments for format

There would seem to be something wrong with the testing if you get "`" 
from mainline as a left quote.

I don't actually see why this patch would affect format diagnostics since 
they don't use c_getstr.  But if it does, I think that would be losing 
diagnostics for the common case of -fexec-charset to avoid wrong ones for 
the uncommon case.

> I'm very aware that you don't have the authority to approve much of
> this patch as much of it is outside the C front-end, but for the
> benefit of other reviewers, could you just confirm that you consider
> my patch a considerable improvement on the current situation, and
> that you don't dispute that this resolves PR18785?  At the very least
> it corrects the inappropriate -Wformat message above!

If it does, there should be a testcase - but I think correcting it at the 
expense of more valid messages would be wrong.  In any case, the patch 
needs testcases for all affected built-in functions that they are affected 
in the intended way (for now, being disabled, but maybe in future being 
optimized as appropriate for the execution character set).

What I do object to is the c_getstr change, which is far more wide-ranging 
than necessary in its disabling of optimizations.  The disabling should be 
done locally in expand_builtin_printf, expand_builtin_fprintf, 
expand_builtin_sprintf, fold_builtin_sprintf which need it.  In addition I 
see that __builtin_nan could be a problem, but is harder to deal with.

-- 
Joseph S. Myers               http://www.srcf.ucam.org/~jsm28/gcc/
    jsm@polyomino.org.uk (personal mail)
    joseph@codesourcery.com (CodeSourcery mail)
    jsm28@gcc.gnu.org (Bugzilla assignments and CCs)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]