On 03/30/2016 06:50 PM, Martin Sebor wrote:
On 03/30/2016 01:25 PM, Jason Merrill wrote:
On 03/30/2016 12:32 PM, Martin Sebor wrote:
On 03/30/2016 09:30 AM, Jason Merrill wrote:
On 03/29/2016 11:57 PM, Martin Sebor wrote:
Are we confident that arr[0] won't make it here as
POINTER_PLUS_EXPR or
some such?
I'm as confident as I can be given that this is my first time
working in this area. Which piece of code or what assumption
in particular are you concerned about?
I want to be sure that we don't fold these conditions to false.
constexpr int *ip = 0;
constexpr struct A { int ar[3]; } *ap = 0;
static_assert(&ip[0] == 0);
static_assert(&(ap->ar[0]) == 0);
I see. Thanks for clarifying. The asserts pass. The expressions
are folded earlier on (in fact, as we discussed, the second one
too early and is accepted even though it's undefined and should be
rejected in a constexpr context) and never reach fold_comparison.
Good, then let's add at least the first to one of the tests.
I've enhanced the new constexpr-nullptr-1.C test to verify this.
I added assertions exercising the relational expressions as well
and for sanity compiled the test with CLang. It turns out that
it rejects the relational expressions with null pointers like
the one below complaining they aren't constant.
constexpr int i = 0;
constexpr const int *p = &i;
constexpr int *q = 0;
static_assert (q < p, "q < p");
I ended up not using a static_assert for the unspecified subset
even though GCC accepts it. It seems that they really aren't
valid constant expressions (their results are unspecified for
null pointers) and should be rejected. Do you agree? (If you
do, I'll add these cases to c++/70248 that's already tracking
another unspecified case that GCC incorrectly accepts).
I agree.
+ /* Avoid folding references to struct members at offset 0 to
+ prevent tests like '&ptr->firstmember == 0' from getting
+ eliminated. When ptr is null, although the -> expression
+ is strictly speaking invalid, GCC retains it as a matter
+ of QoI. See PR c/44555. */
+ && (TREE_CODE (op0) != ADDR_EXPR
+ || TREE_CODE (TREE_OPERAND (op0, 0)) != COMPONENT_REF
+ || compare_tree_int (DECL_FIELD_OFFSET ((TREE_OPERAND
+ (TREE_OPERAND (op0, 0), 1))), 0))
Can we look at offset/bitpos here rather than examine the tree structure
of op0? get_inner_reference already examined it for us.
Good suggestion, thanks!
But you're still examining the tree structure:
+ && (TREE_CODE (op0) != ADDR_EXPR
+ || TREE_CODE (TREE_OPERAND (op0, 0)) != COMPONENT_REF
+ || bitpos0 != 0)
Here instead of looking at op0 we can check (offset0 == NULL_TREE &&
bitpos0 != 0), which indicates a constant non-zero offset.
+ && TREE_CODE (arg1) == INTEGER_CST
+ && integer_zerop (arg1))
And here you don't need the check for INTEGER_CST.
Also, it looks like you aren't handling the case with the operands
switched, i.e. 0 == p and such.
Based on my testing and reading the code I believe the caller
(fold_binary_loc) arranges for the constant argument to always
come second in comparisons. I've added a comment to the code
to make it clear.
Great.
Jason
