[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM aside from some minor nits.



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2789
   InGroup<IgnoredAttributes>;
+def warn_nothrow_attribute_ignored : Warning<"nothrow attribute conflicts with"
+  " exception specification; attribute ignored">,
----------------
Add single quotes around the attribute name: `'nothrow' attribute conflicts...`


================
Comment at: clang/include/clang/Sema/DeclSpec.h:1547
 
+  bool hasAttr(ParsedAttr::Kind Kind) const {
+    return llvm::find_if(getAttrs(), [Kind](const ParsedAttr &P) {
----------------
Not that I dislike this, but is this function being used? It seems to be the only `hasAttr` in the review.


================
Comment at: clang/lib/Sema/SemaType.cpp:6968
+
+    // MSVC ignores nothrow for exception specification if it is in conflict.
+    if (Proto->hasExceptionSpec()) {
----------------
How about: `MSVC ignores nothrow if it is in conflict with an explicit exception specification.`


================
Comment at: clang/lib/Sema/SemaType.cpp:6971
+      switch (Proto->getExceptionSpecType()) {
+      case EST_None: llvm_unreachable("This doesn't have an exception spec!");
+      case EST_DynamicNone:
----------------
Will this need a fallthrough attribute because of the statement between the labels?


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

https://reviews.llvm.org/D62435