[PATCH] D34671: This is to address more command from Richard Smith for my change of https://reviews.llvm.org/D33333

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6356-6357
+    : Warning<
+          "%0 has a non-throwing exception specification but can " "still "
+                                                                   "throw">,
+          InGroup<Exceptions>;
----------------
The formatting here is quite strange, especially the string concat.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6359
+          InGroup<Exceptions>;
+def note_throw_in_dtor : Note<"%select{destructor|deallocator}0 has a "
+                              "%select{non-throwing|implicit "
----------------
Formatting here is also a bit off (see other examples in the file for how we usually format diagnostics).


================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:403
+         FD->getDeclName().getCXXOverloadedOperator() == OO_Array_Delete)) {
+      // No point to emit diagnoistic for no-user declared function.
+      if (FD->getTypeSourceInfo()) {
----------------
Typo "diagnoistic", but the comment doesn't appear to match the code.


================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:404-406
+      if (FD->getTypeSourceInfo()) {
+        const auto *Ty =
+            FD->getTypeSourceInfo()->getType()->getAs<FunctionProtoType>();
----------------
I think a more clear way to write this might be:
```
if (const auto *Ty = FD->getType()->getAs<FunctionProtoType>()) {
  // ...
}
```
There's no need to go through the type source infor to get the function's prototype.


================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:414
+      S.Diag(FD->getLocation(), diag::note_throw_in_function)
+          << FD->getExceptionSpecSourceRange();
   }
----------------
In the event `FD->getTypeSourceInfo()` returns null, `getExceptionSpecSourceRange()` will return an empty source range.


Repository:
  rL LLVM

https://reviews.llvm.org/D34671