[PATCH] Improve -Winvalid-noreturn

Richard Trieu rtrieu at google.com
Wed May 6 17:01:46 PDT 2015


================
Comment at: lib/AST/DeclCXX.cpp:1916-1922
@@ +1915,9 @@
+
+  // Check virtual base classes destructor for noreturn
+  for (const auto &VBase : Record->vbases())
+    if (VBase.getType()
+            ->getAsCXXRecordDecl()
+            ->getDestructor()
+            ->isAnyDestructorNoReturn())
+      return true;
+
----------------
rsmith wrote:
> This seems redundant -- the previous loop already checked all direct bases -- and will be expensive because you'll check every virtual base once for each direct base that inherits from it directly or indirectly.
> 
> Maybe delete this loop and only check direct bases (recursively)?
Removed loop.

================
Comment at: lib/AST/DeclCXX.cpp:1926
@@ +1925,3 @@
+  for (const auto *Field : Record->fields())
+    if (const CXXRecordDecl* RD = Field->getType()->getAsCXXRecordDecl())
+      if (RD->getDestructor()->isAnyDestructorNoReturn())
----------------
rsmith wrote:
> You should insert a `getBaseElementTypeUnsafe()` call here before grabbing the `CXXRecordDecl`, in case it's an array of class type.
Added call to get base element.  Added array test case.

http://reviews.llvm.org/D9454

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list