aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Basic/Attr.td:1948
+def RequiresDesignator : InheritableAttr {
+ let Spellings = [Clang<"requires_designator">];
+ let Subjects = SubjectList<[Record]>;
----------------
emmettneyman wrote:
> compnerd wrote:
> > aaron.ballman wrote:
> > > Hmm, after making this suggestion, I noticed that GCC seems to support a
> > > similar attribute named `designated_init`
> > > (https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#Common-Type-Attributes).
> > > Was your goal to support the same thing GCC supported?
> > `designated_init` is a suggestion, the original patch seemed to be a
> > stronger version. I think that we should be supporting the GNU spelling
> > for `designated_init` and can support a Clang spelling of
> > `requires_designated_init` if the goal is to have the stronger guarantee
> > that this *must* happen.
> I hadn't known about the GCC attribute until now. Yes, the
> `requires_designator` (originally `require_designated_init`) attribute wants
> to enforce the same thing I believe. I couldn't find more documentation for
> the GCC `designated_init` attribute so it's a little tough to tell whether
> the behavior is the exact same. The attribute in this patch allows a field to
> be default constructed (unless the other attribute is applied to that
> specific field) but enforces that a brace initializer must be used. So `Foo
> foo {};` would be valid (every field is default constructed) but `Foo foo;`
> would not be valid. I'm not sure if that's the same behavior the GCC
> attribute is trying to enforce. But on a high level, both are trying to
> prohibit using positional args when declaring a struct.
>
> @compnerd I don't mind this attribute generating warnings rather than errors.
> It's ok for this attribute to be a "suggestion" as well. Like @aaron.ballman
> mentioned, "users can always use -Werror to strengthen their own
> requirements."
I think the correct approach then is: add this attribute under the name
`designated_init` with the `GCC` spelling kind and match GCC's behavior for it
(or, if we don't want to match GCC's behavior, note where we differ and why).
We should put its diagnostics under a `-Wdesignated-init` warning group.
The next question is: are you interested in finding out whether GCC would be
willing to implement the `requires_init` attribute? If GCC is interested in
picking it up, then we can name it with the `GCC` spelling as well. If the GCC
folks are not interested in picking up the attribute, then we should probably
leave it with the `Clang` spelling. I'd recommend we put the diagnostics into
the `-Wdesignated-init` warning group as well.
I think both of these attributes should also be available in C2x mode as well,
but that can probably be done in a follow-up patch as it will involve tablegen
changes and opens up other questions (i.e., does GCC handle C2x attributes yet,
and if they do, are they making `designated_init` available in that mode?).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64380/new/
https://reviews.llvm.org/D64380
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
