[PATCH] D64380: Add 'require_designated_init' and 'required' attribute to clang

[Aaron Ballman via Phabricator via cfe-commits]

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]>;
----------------
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?


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3534
+def warn_requires_designator_failed : Warning<
+  "variable declaration does not use designated initializer syntax">;
+def note_declared_requires_designator_here : Note<
----------------
These new warnings should be controlled under a diagnostic group. I would recommend `designated-init` because that's what's used by GCC for their attribute, but that's only because I am assuming you want your attribute to match their behavior.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:11220
+      if (const auto *ILE = dyn_cast<InitListExpr>(Init)) {
+        for (auto *I : ILE->inits()) {
+          if (auto *DIE = dyn_cast<DesignatedInitExpr>(I))
----------------
`const auto *` here as well?


================
Comment at: clang/lib/Sema/SemaDecl.cpp:11221
+        for (auto *I : ILE->inits()) {
+          if (auto *DIE = dyn_cast<DesignatedInitExpr>(I))
+            continue;
----------------
You don't use `DIE` -- this should switch to an `isa<>` instead.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:11254
+        if (const auto *ILE = dyn_cast<InitListExpr>(Init)) {
+          for (auto *I : ILE->inits()) {
+            if (auto *DIE = dyn_cast<DesignatedInitExpr>(I)) {
----------------
`const auto *` here and below?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64380/new/

https://reviews.llvm.org/D64380