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

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 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>;
----------------
aaron.ballman wrote:
> The formatting here is quite strange, especially the string concat.
But that is from clang-format...

For sure I did not invent that format.  I change to this.  Hope that okay.



================
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 "
----------------
aaron.ballman wrote:
> Formatting here is also a bit off (see other examples in the file for how we usually format diagnostics).
Same here, it is also from clang-format.

I change to this.  


================
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()) {
----------------
aaron.ballman wrote:
> Typo "diagnoistic", but the comment doesn't appear to match the code.
:-( remove it.


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


Repository:
  rL LLVM

https://reviews.llvm.org/D34671