[PATCH] c-family: Fix up -Wduplicated-branches for union members [PR99565]

[Marek Polacek]

On Wed, Mar 17, 2021 at 08:28:20AM -0600, Martin Sebor wrote:
> On 3/17/21 2:36 AM, Jakub Jelinek wrote:
> > On Tue, Mar 16, 2021 at 06:28:46PM -0600, Martin Sebor via Gcc-patches wrote:
> > > It seems sort of "inverted:" I'd expect OEP_LEXICOGRAPHIC on its
> > > own to do a lexicographical comparison, without having to set
> > > an additional bit to ask for it.  If a more refined form of
> > 
> > The new flag is really orthogonal to what OEP_LEXICOGRAPHIC does and
> > for the VLA checks you are using OEP_LEXICOGRAPHIC for I think you exactly
> > don't want that behavior, both a and b when they are a union members
> > with the same type will actually have the same value at least in the way
> > GCC implements unions, so you want to treat them the same.
> 
> The same thing can be said about the case you're changing:
> the ternary expression does evaluate to a reference to the same
> subobject.  Whether or not it's worth considering as a potential
> mistake is a judgment call that may or may not be different for
> different warnings.

I think my intention when I added -Wduplicated-branches was to warn for
"obviously" identical expressions in the branches of a conditional -- to
catch, for instance, copy paste errors.  That in this case the offsets for
the FIELD_DECLs match and so the expressions were considered identical by
the compiler is IMHO not a reason to warn.  (If the warning here wasn't
confusing, we wouldn't have gotten the report in the first place).

It also doesn't warn for macros even when they expand to the same thing,
because it's just too verbose.
 
> > The flag can also be called OEP_COMPONENT_REF_SAME_FIELD or whatever,
> > the reason for using the name I've used is just in case -Wduplicate-branches
> > will need another tweak in the future somewhere else, we don't have enough
> > bits to allocate for each detail separately.
> 
> I do think a different name than OEP_DUPLICATE_BRANCHES would be
> preferable, one that describes what the flag does and not where
> it's used.  (Doing otherwise would suggest that every client of
> the function have its own bit, even if has the same value as
> an existing bit.)

Sure, the name is debatable.  I wish we could just use OEP_LEXICOGRAPHIC
but sadly that's not really sufficient.
 
> As an aside, would a COMPONENT_REF and MEM_REF with the same base
> object, offset, and type be considered the same?

Probably depends on whether we're comparing the addresses.

Marek