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

2019-08-01 Thread 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">, GCC<"designated_init">]; + let Subjects = SubjectList<[Record]>; emmettneyman

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

2019-07-26 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman updated this revision to Diff 211988. emmettneyman marked 6 inline comments as done. emmettneyman added a comment. Addressed comments and feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64380/new/

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

2019-07-26 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman marked 16 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">, GCC<"designated_init">]; + let Subjects =

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

2019-07-26 Thread 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">, GCC<"designated_init">]; + let Subjects = SubjectList<[Record]>; Why does this

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

2019-07-25 Thread Aditya Kumar via Phabricator via cfe-commits
hiraditya added a comment. Thanks for addressing the feedback @emmettneyman , LGTM, deferring to other reviewers for final call. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64380/new/ https://reviews.llvm.org/D64380

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

2019-07-18 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman updated this revision to Diff 210649. emmettneyman added a comment. Created diagnostic group, added behavior and documentation for private fields, added documentation regarding GCC attribute Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2019-07-16 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman planned changes to this revision. emmettneyman added a comment. Changes planned: - Move diagnostics into a diagnostic group. - Add behavior and test cases for private members of structs/classes. - Investigate behavior of GCC `designated_init` attribute Repository: rG LLVM Github

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

2019-07-11 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:1448 +def RequireDesignatedInitDocs : Documentation { + let Category = DocCatType; aaron.ballman wrote: > The behavior should be documented as to what happens when you apply

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

2019-07-11 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman updated this revision to Diff 209362. emmettneyman marked 2 inline comments as done. emmettneyman added a comment. Added documentation about using the attributes with unions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64380/new/

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

2019-07-11 Thread 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]>; emmettneyman wrote: > compnerd wrote:

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

2019-07-11 Thread Emmett Neyman via Phabricator via cfe-commits
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]>;

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

2019-07-11 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman updated this revision to Diff 209256. emmettneyman added a comment. Small changes: adding `auto` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64380/new/ https://reviews.llvm.org/D64380 Files: clang/include/clang/Basic/Attr.td

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

2019-07-11 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1948 +def RequiresDesignator : InheritableAttr { + let Spellings = [Clang<"requires_designator">]; + let Subjects = SubjectList<[Record]>; aaron.ballman wrote: > Hmm, after making

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

2019-07-11 Thread 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

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

2019-07-10 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman planned changes to this revision. emmettneyman added a comment. Working on adding more information to the documentation and adding more in-depth unit tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64380/new/

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

2019-07-10 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3533-3541 +def err_require_designated_init_failed : Error< + "variable declaration does not use designated initializer syntax">; +def note_declared_required_designated_init_here :

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

2019-07-10 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman updated this revision to Diff 209097. emmettneyman marked 32 inline comments as done. emmettneyman added a comment. Addressed several of the comments regarding naming and style. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64380/new/

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

2019-07-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1947 +def RequireDesignatedInit : InheritableAttr { + let Spellings = [CXX11<"clang", "require_designated_init">]; This should be `RequiresDesignator`.

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

2019-07-09 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman marked 2 inline comments as done. emmettneyman added inline comments. Comment at: clang/test/SemaCXX/attr-designated-init-required.cpp:3 + +#define ATTR [[clang::designated_init_required]] + compnerd wrote: > Why the macro? I modeled this file after

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

2019-07-09 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman added a comment. In D64380#1577350 , @compnerd wrote: > I don't see any cases where `[[clang::required]]` is tested, am I missing > something? I renamed the attribute at your suggestion. It's now called

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

2019-07-09 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added a comment. I don't see any cases where `[[clang::required]]` is tested, am I missing something? Comment at: clang/test/SemaCXX/attr-designated-init-required.cpp:3 + +#define ATTR [[clang::designated_init_required]] + Why the macro?

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

2019-07-09 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman updated this revision to Diff 208842. emmettneyman added a comment. slight change to specification reference format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64380/new/ https://reviews.llvm.org/D64380 Files:

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

2019-07-09 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. C++20 designated initializers don't permit mixing designated fields with non-designated ones, so some of the examples here are invalid. However, I think we should be looking for an attribute design that works well in both C and C++, and with the various Clang extensions

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

2019-07-09 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman updated this revision to Diff 208840. emmettneyman marked an inline comment as done. emmettneyman added a comment. Remove cppreference link, add reference to C++ spec. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64380/new/

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

2019-07-09 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman updated this revision to Diff 208829. emmettneyman added a comment. Updated tests and diagnostic messages with new attribute spelling Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64380/new/ https://reviews.llvm.org/D64380 Files:

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

2019-07-09 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman updated this revision to Diff 208814. emmettneyman marked 3 inline comments as done. emmettneyman added a comment. Changed attribute spelling from GNU to CXX11 and renamed the 'required' attribute to 'designated_init_required' Repository: rG LLVM Github Monorepo CHANGES SINCE

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

2019-07-09 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman updated this revision to Diff 208741. emmettneyman marked 6 inline comments as done. emmettneyman added a comment. Small changes in response to feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64380/new/

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

2019-07-08 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1948 +def RequireDesignatedInit : InheritableAttr { + let Spellings = [GNU<"require_designated_init">]; + let Subjects = SubjectList<[Type]>; This seems like an extension? I think

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

2019-07-08 Thread Emmett Neyman via Phabricator via cfe-commits
emmettneyman updated this revision to Diff 208544. emmettneyman added a comment. Undo clang-format changes to SemaDecl.cpp and SemaDeclAttr.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64380/new/ https://reviews.llvm.org/D64380 Files: