jyknight wrote:
Just to say again, I think the discussion may be crisper if we consider this as
two entirely-distinct features:
1. An expansion of "compatible type" for purposes of pointer casts/non-UB/etc.
2. The ability to provide two definitions of the same struct type in the same
scope, to effectively behave like you can already do for a typedef.
For the former, it can be incorrect (not conservative) if we treat two types as
incompatible when they should be compatible.
But, to continue discussing the latter,
> And with redefinitions, you run into problems you wouldn't be able to run
> into with redeclarations. e.g.,
>
> [snip example adding packed to a field]
>
> You couldn't run into this situation with redeclarations because you
> previously could not redeclare the structure fields.
You do already run into nearly that precise situation with redeclarations
today, because you can (attempt to) add the packed attribute to the struct
_itself_ in a redeclaration!
```
struct S {
char c;
int x;
};
struct [[gnu::packed]] S;
```
We handle that in `checkNewAttributesAfterDef` called from
`mergeDeclAttributes`, usually emitting a warning if you try add an additional
attribute on a declaration that occurs after the definition, `warning:
attribute declaration must precede definition [-Wignored-attributes]`. That
exact same diagnostic behavior seems like it should apply for a definition
after a definition, with a little tweak of the diagnostic's wording.
Of course, that existing handling is only about top-level attributes. For
fields, I think the sensible model would be to handle it as a redeclaration of
the field. Then we treat the field redeclarations in the same way we handle
other redeclaration situations, using the mostly-already-existing code. (We may
need to add a redecl chain to FieldDecls?)
The behavior resulting from using this existing model would be: if a field
declaration in a subsequent struct definition has additional attributes which
the first definition did not, then we'd emit a warning (usually). If a field in
a subsequent definition is _missing_ attributes a prior one had, then we'd
inherit them (unless they're not inheritable).
> Yeah, there's a fair amount of moving parts.
I think it might help to start by implementing only the expansion of
"compatible type". The issues there are much simpler -- no attribute
inheritance to worry about! But I don't think the PR correctly implements it
yet.
Once that is settled, then go back to work on redeclaration/definition support.
Possibly it'd be good to pause and ask the standards body to clarify how it's
supposed to work first. While I think my proposed semantics above make sense,
the standard doesn't actually _specify_ that, one way or the other. And it
should.
https://github.com/llvm/llvm-project/pull/132939
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
