From: Al Viro <viro@ftp.linux.org.uk>
To: Josh Triplett <josh@freedesktop.org>
Cc: linux-sparse@vger.kernel.org,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: fun with ?:
Date: Sun, 3 Jun 2007 02:05:34 +0100 [thread overview]
Message-ID: <20070603010534.GV4095@ftp.linux.org.uk> (raw)
In-Reply-To: <465392E3.3000300@freedesktop.org>
On Tue, May 22, 2007 at 06:03:31PM -0700, Josh Triplett wrote:
> > Usual sparse constant folding is _almost_ OK, provided that we play a bit
> > with evaluate_expression() and let it decide if subexpression is an integer
> > constant expression. No prototype changes, we just get a global flag
> > and save/restore it around sizeof argument handling and several other
> > places. It's actually pretty easy. And if we get "it is an integer
> > constant expression", well, then caller can call expand stuff.
>
> Sounds reasonable to me. Possibly better written as some kind of generic
> parse-tree-walking operation, but this approach should work fine.
Hrm... Actually, "global flag" approach doesn't work at all, since we
mishandle already-evaluated subtrees (happens with inlined functions).
AFAICS, here's what we can do with relatively little PITA:
Split some of the EXPR_... cases in two; additional variant
would differ by | EXPR_INT_CONST, which would be 1U<<31 (let's call it
i-bit). EXPR_CAST gets split in three - normal, EXPR_CAST with i-bit
and EXPR_CAST with f-bit (1U<<30).
At parse time, do the following:
* EXPR_VALUE, EXPR_TYPE, EXPR_SIZEOF, EXPR_PTRSIZEOF and EXPR_ALIGNOF
get i-bit.
* EXPR_COMPARE, EXPR_BINOP, EXPR_LOGICAL, EXPR_CONDITIONAL,
EXPR_PREOP with !, + , -, ~ or ( and EXPR_CAST get i-bit if all their
arguments have it.
* EXPR_CAST also gets both i- and f-bit if its argument is EXPR_FVALUE,
possibly wrapped into some EXPR_PREOP[(].
That won't really complicate the parser. Now, at that point i-bit
is weaker than "subexpression is an integer constant expression", but not
by much - the only things we are missing are "it typechecks OK" and "all
casts are to integer types" (note that sizeof(VLA) would have to be caught
when we start handling those; as it is, it fails typechecking, plain and
simple).
So what we do at evaluate_expression() is simple - we remove some i-bits.
Rules:
* EXPR_COMPARE, EXPR_BINOP, EXPR_LOGICAL, EXPR_CONDITIONAL, EXPR_PREOP:
lose i-bit if some argument doesn't have it after evaluation.
* EXPR_IMPLIED_CAST to an integer type inherits i-bit from argument.
* EXPR_CAST loses i-bit if the type we are casting to is not an
integer one; it also loses i-bit if evaluated argument doesn't have i-bit
*and* EXPR_CAST itself doesn't have f-bit.
* in cannibalizing EXPR_PREOP in &*p and *&p simplifications,
keep the (lack of) i-bit and f-bit on the overwritten node.
In expand_expression() we keep i-bit through the replacement.
That's it. Now, "has i-bit after evaluate_expression()" == "expression
is an integer constant one".
Now, we do the following:
* static struct symbol null_ctype, initialized to SYM_PTR over
void.
* evaluation of EXPR_CAST with target type being a pointer to void
and argument bearing an i-bit should check if argument is in fact 0; if
it is, replace the node with EXPR_VALUE[0] and &null_ctype as type.
* int is_null_pointer_constant(expr) - return 1 if type is &null_ctype,
2 if argument bears i-bit and is in fact 0, return 0 otherwise.
* have degenerate() turn &null_ctype into a normal pointer to void
* callers of degenerate() in contexts where we care about null
pointer constants (?:, assignment, argument of function call, pointer
comparison, cast) should do is_null_pointer_constant() first. Act accordingly,
warn if it had returned 2. Note that we can't blindly generate a warning
in is_null_pointer_constant() - sometimes 0 is simply 0 and we don't want it
to become a pointer at all, let alone generate any warnings.
AFAICS, that will do the right thing with little pain. I'm not all that
happy about splitting EXPR_... (in effect, stealing bit from expr->type);
perhaps reducing the size of expr->type and adding expr->flags would be
better. Hell knows...
Comments?
next prev parent reply other threads:[~2007-06-03 1:05 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-05-19 2:52 fun with ?: Al Viro
2007-05-22 21:40 ` Josh Triplett
2007-05-22 22:46 ` Al Viro
2007-05-22 23:24 ` Josh Triplett
2007-05-23 0:02 ` Al Viro
2007-05-23 0:25 ` Al Viro
2007-05-23 1:05 ` Josh Triplett
2007-05-23 4:53 ` Al Viro
2007-05-23 12:26 ` Morten Welinder
2007-05-23 1:03 ` Josh Triplett
2007-06-03 1:05 ` Al Viro [this message]
2007-05-23 14:25 ` Neil Booth
2007-05-23 14:32 ` Al Viro
2007-05-23 14:47 ` Neil Booth
2007-05-23 15:32 ` Al Viro
2007-05-23 23:01 ` Neil Booth
2007-05-24 0:10 ` Derek M Jones
2007-05-24 0:14 ` Al Viro
2007-05-23 21:16 ` Derek M Jones
2007-05-23 21:59 ` Linus Torvalds
2007-05-23 23:29 ` Derek M Jones
2007-05-24 0:02 ` Al Viro
2007-05-24 0:29 ` Linus Torvalds
2007-05-24 1:36 ` Brett Nash
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20070603010534.GV4095@ftp.linux.org.uk \
--to=viro@ftp.linux.org.uk \
--cc=josh@freedesktop.org \
--cc=linux-sparse@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).