erichkeane marked 6 inline comments as done.
erichkeane added inline comments.


================
Comment at: clang/include/clang/Sema/DeclSpec.h:1547
 
+  bool hasAttr(ParsedAttr::Kind Kind) const {
+    return llvm::find_if(getAttrs(), [Kind](const ParsedAttr &P) {
----------------
aaron.ballman wrote:
> Not that I dislike this, but is this function being used? It seems to be the 
> only `hasAttr` in the review.
Woops, likely a leftover from a previous iteration, removing it!


================
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:
----------------
aaron.ballman wrote:
> Will this need a fallthrough attribute because of the statement between the 
> labels?
Ah, I think it depends on what llvm_unreachable ends up expanding to (actually, 
LLVM_ATTRIBUTE_NORETURN). 

Basically, here: https://llvm.org/doxygen/Support_2ErrorHandling_8h_source.html

I'm not sure of what compilers has that expand to nothing don't support it 
(would need !def __GNUC__ and !def _MSC_VER), but an LLVM_FALLTHROUGH is easy 
enough to add, thanks!


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

https://reviews.llvm.org/D62435



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to