linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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?

  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).