[CFG] [analyzer] NFC: Move construction context allocation into a helper method.

[NoQ]

Summary

It is tedious to allocate and then placement-new the construction context, so i added a tiny variadic template to improve readability.

Also fix more else-after-return thingies.

[NoQ]

I'd rather keep the helper method within the class. This allows us to keep constructors private, which is something i forgot to do originally.

[george.karpenkov]

LGTM, but would actually make even more sense as a static function for the allocator.
E.g. we have llvm::make_shared<>(), so we could also have Allocator::make_obj<...>(...)

[NoQ]

E.g. we have llvm::make_shared<>(), so we could also have Allocator::make_obj<...>(...)

I like your thinking, but that'd require me to make the allocator my friend class (:

lib/Analysis/ConstructionContext.cpp

while (true) {
     if (!Self || !Self->isSameLayer(Other))
       return false;
     Self = Self->getParent();
     Other = Other->getParent();
   }
   llvm_unreachable("The above loop can only be terminated via return!");
 }
 
+// A helper - create a construction context in the given bump pointer allocator
+// with given arguments.
+template<typename T, typename... ArgTypes>
+static T *make(BumpVectorContext &C, ArgTypes... Args) {
+  auto *CC = C.getAllocator().Allocate<T>();
+  return new (CC) T(Args...);
+}
+
 const ConstructionContext *ConstructionContext::createFromLayers(
     BumpVectorContext &C, const ConstructionContextLayer *TopLayer) {
   // Before this point all we've had was a stockpile of arbitrary layers.
   // Now validate that it is shaped as one of the finite amount of expected
   // patterns.
   if (const Stmt *S = TopLayer->getTriggerStmt()) {
     if (const auto *DS = dyn_cast<DeclStmt>(S)) {
       assert(TopLayer->isLast());
-      auto *CC =
-          C.getAllocator().Allocate<SimpleVariableConstructionContext>();
-      return new (CC) SimpleVariableConstructionContext(DS);
-    } else if (const auto *NE = dyn_cast<CXXNewExpr>(S)) {
+      return make<SimpleVariableConstructionContext>(C, DS);
+    }
+    if (const auto *NE = dyn_cast<CXXNewExpr>(S)) {
       assert(TopLayer->isLast());
-      auto *CC =
-          C.getAllocator().Allocate<NewAllocatedObjectConstructionContext>();
-      return new (CC) NewAllocatedObjectConstructionContext(NE);
-    } else if (const auto *BTE = dyn_cast<CXXBindTemporaryExpr>(S)) {
+      return make<NewAllocatedObjectConstructionContext>(C, NE);
+    }
+    if (const auto *BTE = dyn_cast<CXXBindTemporaryExpr>(S)) {
       const MaterializeTemporaryExpr *MTE = nullptr;
       assert(BTE->getType().getCanonicalType()
                 ->getAsCXXRecordDecl()->hasNonTrivialDestructor());
       // For temporaries with destructors, there may or may not be
       // lifetime extension on the parent layer.
       if (const ConstructionContextLayer *ParentLayer = TopLayer->getParent()) {
         assert(ParentLayer->isLast());
         if ((MTE = dyn_cast<MaterializeTemporaryExpr>(
                  ParentLayer->getTriggerStmt()))) {
           // A temporary object which has both destruction and
           // materialization info.
-          auto *CC =
-              C.getAllocator().Allocate<TemporaryObjectConstructionContext>();
-          return new (CC) TemporaryObjectConstructionContext(BTE, MTE);
+          return make<TemporaryObjectConstructionContext>(C, BTE, MTE);
         }
         // C++17 *requires* elision of the constructor at the return site
         // and at variable initialization site, while previous standards
         // were allowing an optional elidable constructor.
         if (auto *RS = dyn_cast<ReturnStmt>(ParentLayer->getTriggerStmt())) {
           assert(!RS->getRetValue()->getType().getCanonicalType()
                     ->getAsCXXRecordDecl()->hasTrivialDestructor());
-          auto *CC =
-              C.getAllocator()
-                  .Allocate<
-                      CXX17ElidedCopyReturnedValueConstructionContext>();
-          return new (CC)
-              CXX17ElidedCopyReturnedValueConstructionContext(RS, BTE);
+          return make<CXX17ElidedCopyReturnedValueConstructionContext>(C,
+                                                                       RS, BTE);
         }
         if (auto *DS = dyn_cast<DeclStmt>(ParentLayer->getTriggerStmt())) {
           assert(!cast<VarDecl>(DS->getSingleDecl())->getType()
                       .getCanonicalType()->getAsCXXRecordDecl()
                       ->hasTrivialDestructor());
-          auto *CC =
-              C.getAllocator()
-                  .Allocate<CXX17ElidedCopyVariableConstructionContext>();
-          return new (CC) CXX17ElidedCopyVariableConstructionContext(DS, BTE);
+          return make<CXX17ElidedCopyVariableConstructionContext>(C, DS, BTE);
         }
         llvm_unreachable("Unexpected construction context with destructor!");
       }
       // A temporary object that doesn't require materialization.
-      auto *CC =
-          C.getAllocator().Allocate<TemporaryObjectConstructionContext>();
-      return new (CC)
-          TemporaryObjectConstructionContext(BTE, /*MTE=*/nullptr);
-    } else if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(S)) {
+      return make<TemporaryObjectConstructionContext>(C, BTE, /*MTE=*/nullptr);
+    }
+    if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(S)) {
       // If the object requires destruction and is not lifetime-extended,
       // then it must have a BTE within its MTE.
       assert(MTE->getType().getCanonicalType()
                 ->getAsCXXRecordDecl()->hasTrivialDestructor() ||
              MTE->getStorageDuration() != SD_FullExpression);
       assert(TopLayer->isLast());
-      auto *CC =
-          C.getAllocator().Allocate<TemporaryObjectConstructionContext>();
-      return new (CC) TemporaryObjectConstructionContext(nullptr, MTE);
-    } else if (const auto *RS = dyn_cast<ReturnStmt>(S)) {
+      return make<TemporaryObjectConstructionContext>(C, nullptr, MTE);
+    }
+    if (const auto *RS = dyn_cast<ReturnStmt>(S)) {
       assert(TopLayer->isLast());
-      auto *CC =
-          C.getAllocator().Allocate<SimpleReturnedValueConstructionContext>();
-      return new (CC) SimpleReturnedValueConstructionContext(RS);
+      return make<SimpleReturnedValueConstructionContext>(C, RS);
     }
+    llvm_unreachable("Unexpected construction context with statement!");
   } else if (const CXXCtorInitializer *I = TopLayer->getTriggerInit()) {
     assert(TopLayer->isLast());
-    auto *CC =
-        C.getAllocator().Allocate<ConstructorInitializerConstructionContext>();
-    return new (CC) ConstructorInitializerConstructionContext(I);
+    return make<ConstructorInitializerConstructionContext>(C, I);
   }
   llvm_unreachable("Unexpected construction context!");
 }