[PATCH] D51812: Simplify CheckFallThroughForBody

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 7 14:53:01 PDT 2018


rsmith added a comment.

I'm not a fan of the duplication introduced here, but the new code is definitely more obvious. On balance, this seems like a small improvement, so let's go for it.



================
Comment at: b/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp:542-544
+  // cpu_dispatch functions permit empty function bodies for ICC compatibility.
+  if (FD->isCPUDispatchMultiVersion())
+    return;
----------------
Hmm. It really doesn't make any sense to apply `cpu_dispatch` to a lambda, because the call operator can't be overloaded. I don't think this special case warning suppression is worthwhile. (Rather, we should probably disallow the attribute on lambdas; I've asked Erich Keane about that.)


================
Comment at: b/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp:595
+                       D->getLocation())) &&
+      (!HasNoReturnAttr))
+      return;
----------------
I know this is just rearranged from where it was before, but... can you remove the redundant parens here, and run the patch through clang-format?


================
Comment at: b/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp:601
+    case MaybeFallThrough:
+      if (ReturnsValue)
+        S.Diag(RBrace, diag::warn_maybe_falloff_nonvoid_coroutine)
----------------
This `if` and the one below are redundant now.


Repository:
  rC Clang

https://reviews.llvm.org/D51812





More information about the cfe-commits mailing list