On 7/2/19 2:59 PM, Jeff Law wrote:
On 6/30/19 3:50 PM, Martin Sebor wrote:
On 6/26/19 6:11 PM, Jeff Law wrote:
[ Another big snip ]
Is there a strong need for an overloaded operator? Our guidelines
generally discourage operator overloads. This one seems on the border
to me. Others may have different ideas where the line is. If we really
don't need the overloaded operator, then we can avoid the controversy
completely.
The copy assignment operator is necessary for the type to be stored
in a hash_map. Otherwise, the implicitly generated assignment will
be used instead which is unsafe in vec and results in memory
corription (I opened bug 90904 for this). After working around this
bug by providing the assignment operator I then ran into the hash_map
bug 90959 that also results in memory corruption. I spent a few fun
hours tracking down the GCC miscompilations that they led to.
OK. THanks for explaining why we need the copy assignment operator.
The golden rule in C++ is that every type should either be safe to
copy and assign with the expected semantics or made not assignable
and not copyable by declaring the copy ctor and copy assignment
operator private (or deleted in more modern C++). It would be very
helpful to detect this kind of problem (classes that aren't safely
assignable and copyable because they acquire/release resources in
ctors/dtors). Not just to us in GCC but I'm sure to others as well.
It's something I've been hoping to implement for a while now.
You've got my blessing to do that :-)
So how do you deal with the case where one operand of a
{MIN,MAX,COND}_EXPR is the address of a local, but the other is not? It
seems like we'll end up returning true from this function in that case
and potentially trigger changing the return value to NULL. Or am I
missing something?
I only briefly considered MIN/MAX but because in C and C++ the less
than and greater than expressions are only defined for pointers into
the same array/object or subobject and I didn't ponder it too hard.
(In C they're undefined otherwise, in C++ the result is unspecified.)
But you bring it up because you think the address should be returned
regardless, even if the expression is invalid (or if it's COND_EXPR
with mixed local/nonlocal addresses) so I've changed it to do that
for all these explicitly handled codes and added tests to verify it.
THanks.
So in the thread for strlen/sprintf the issue around unbounded walks up
through PHI argument's SSA_NAME_DEF_STMT was raised.
I think we've got two options here.
One would be to hold this patch until we sort out what the bounds should
look like, then adjust this patch to do something similar.
Or go forward with this patch, then come back and adjust the walker once
we have settled on solution in the other thread.
I'm OK with either. Your choice.
I committed the patch as is until the question of which algorithms
to put the limit on is answered (as discussed in the thread Re:
sprintf/strlen integration).
Martin
