On 03/17/2016 03:16 PM, Martin Sebor wrote:
static tree cxx_eval_constant_expression (const constexpr_ctx *, tree,
- bool, bool *, bool *, tree * = NULL);
+ bool, bool *, bool *, bool * = NULL,
+ tree * = NULL);
I didn't look deeply, but do you end up fixing all (most) of the callers
of cxx_eval_constant_expression? If so, then you don't need the default
initialization.
Thanks for the comments. The patch only modifies about 10 out
of the 70 or so calls to the function in the file and the default
argument helps avoid making the rest of the changes. (I have some
ideas for improving the APIs of these functions that I'd like to
run by Jason when we're done with the 6.0 work.)
OK. Then let's keep the default initialization.
The difficulty I've run into with detecting these problems in later
phases is that some invalid expressions have already been simplified
by the front end. The example that applies here (even though this
is still the front end) is this:
Yea. I was hoping that the delayed folding work would be helping in
getting a more faithful representation out of the front-ends.
constexpr int* p = 0;
constexpr bool b0 = &p[0] == 0; // accepted
constexpr bool b1 = &p[1] == 0; // rejected
Both b0 and b1 are invalid and should be diagnosed, but only b1
is. b1 isn't because because by the time we see its initializer
in constexpr.c it's been transformed into the equivalent of "b1
= (int*)ps" (though we don't see the cast which would also make
it invalid).
But if we can avoid these early simplifying transformations and
retain a more faithful representation of the original source then
doing the checking later will likely be simpler and result in
detecting more problems with greater consistency and less effort.
Do we know where the folding is happening for this case and is it
something we can reasonably defer? ie, is this just a case we missed
as part of the deferred folding work and hence should have its own
distinct BZ to track?
Hmm, I thought we already had code to do this somewhere. It looks like
it's moved around quite a bit. I think you want to be using
symtab_node::nonzero_address to determine if a given symbol must bind to
a nonzero address.
Thanks for the hint. I had looked for existing functions but
couldn't find one that worked. decl_with_nonnull_addr_p() in
c-common.c looked promising but it's not accessible here and
it doesn't do the right thing when HAS_DECL_ASSEMBLER_NAME_P()
is false (it ICEs).
Yea, I found the same mis-mash of bits that didn't look directly usable
for the problem you're tackling. What's odd is I would have sworn that
we had code to do exactly what you wanted, but I wasn't able to find it,
either as a distinct routine or open-coded.
In the end, I added a new function, maybe_nonzero_address(),
that calls symtab_node::nonzero_address(), and that I factored
out of tree_single_nonzero_warnv_p() that I was then able to
use in fold_comparison().
Sounds good.
I've made a few other small adjustments to the patch to avoid
one false positive, and a few test cases, and tweak the expected
diagnostics now that Marek has fixed 70194.
I've also wrote myself a small sed script to replace blocks of
8 spaces with tabs and ran the patch through it. I'll integrate
it into my workflow so I hopefully don't have to worry about this
ever again.
I'll try to take a look at the updated patch shortly. It may still hit
too much of the C++ front-end for me to be comfortable reviewing --
we'll see.
jeff
