emmettneyman marked 3 inline comments as done.
emmettneyman added inline comments.


================
Comment at: clang/include/clang/Basic/Attr.td:1948
+def RequiresDesignator : InheritableAttr {
+  let Spellings = [Clang<"requires_designator">];
+  let Subjects = SubjectList<[Record]>;
----------------
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."


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

Reply via email to