Delete default constructors, copy constructors, move constructors, copy assignment, move assignment operators on Expr, Stmt and Decl

[ilya-biryukov]

LGTM

clang/include/clang/AST/DeclBase.h

374 ↗

(On Diff #200458)

NIT: move constructors and assignments won't be generated if copy is deleted, so this is redundant.
However, feel free to keep it if you want to be more explicit.

clang/include/clang/AST/Stmt.h

1057 ↗

(On Diff #200458)

NIT: Move the deleted declarations to the start of the class?
Or move the deleted declarations in other classes to the first public section?

(For consistency)

[gribozavr]

clang/include/clang/AST/DeclBase.h

374 ↗

(On Diff #200458)

I feel like it is better to have the API explicitly spelled out.

clang/include/clang/AST/Stmt.h

1057 ↗

(On Diff #200458)

I put these new declarations close to the "primary" constructor.

[ilya-biryukov]

LG either way, the comments were NITs.

clang/include/clang/AST/Stmt.h

1057 ↗

(On Diff #200458)

Why not move declarations inside Stmt to the primary constructor too?

[gribozavr]

Addressed review comments

[gribozavr]

clang/include/clang/AST/Stmt.h

1057 ↗

(On Diff #200458)

Done.

include/clang/AST/DeclBase.h

if (DC) {
           (!D->isFromASTFile() || D->hasLocalOwningModuleStorage()))
         return MOK;
       // If D is not local and we have no local module storage, then we don't
       // need to track module ownership at all.
     }
     return ModuleOwnershipKind::Unowned;
   }
 
+public:
+  Decl() = delete;
+  Decl(const Decl&) = delete;
+  Decl(Decl &&) = delete;
+  Decl &operator=(const Decl&) = delete;
+  Decl &operator=(Decl&&) = delete;
+
 protected:
   Decl(Kind DK, DeclContext *DC, SourceLocation L)
       : NextInContextAndBits(nullptr, getModuleOwnershipKindForChildOf(DC)),
         DeclCtx(DC), Loc(L), DeclKind(DK), InvalidDecl(false), HasAttrs(false),
         Implicit(false), Used(false), Referenced(false),
         TopLevelDeclInObjCContainer(false), Access(AS_none), FromASTFile(0),
         IdentifierNamespace(getIdentifierNamespaceForKind(DK)),
         CacheValidAndLinkage(0) {

include/clang/AST/Expr.h

 };
 
 /// This represents one expression.  Note that Expr's are subclasses of Stmt.
 /// This allows an expression to be transparently used any place a Stmt is
 /// required.
 class Expr : public ValueStmt {
   QualType TR;
 
+public:
+  Expr() = delete;
+  Expr(const Expr&) = delete;
+  Expr(Expr &&) = delete;
+  Expr &operator=(const Expr&) = delete;
+  Expr &operator=(Expr&&) = delete;
+
 protected:
   Expr(StmtClass SC, QualType T, ExprValueKind VK, ExprObjectKind OK,
        bool TD, bool VD, bool ID, bool ContainsUnexpandedParameterPack)
     : ValueStmt(SC)
   {
     ExprBits.TypeDependent = TD;
     ExprBits.ValueDependent = VD;
     ExprBits.InstantiationDependent = ID;

include/clang/AST/Stmt.h

private:
   /// Whether statistic collection is enabled.
   static bool StatisticsEnabled;
 
 protected:
   /// Construct an empty statement.
   explicit Stmt(StmtClass SC, EmptyShell) : Stmt(SC) {}
 
 public:
+  Stmt() = delete;
+  Stmt(const Stmt &) = delete;
+  Stmt(Stmt &&) = delete;
+  Stmt &operator=(const Stmt &) = delete;
+  Stmt &operator=(Stmt &&) = delete;
+
   Stmt(StmtClass SC) {
     static_assert(sizeof(*this) <= 8,
                   "changing bitfields changed sizeof(Stmt)");
     static_assert(sizeof(*this) % alignof(void *) == 0,
                   "Insufficient alignment!");
     StmtBits.sClass = SC;
     StmtBits.IsOMPStructuredBlock = false;
     if (StatisticsEnabled) Stmt::addStmtClass(SC);
   }
 
   StmtClass getStmtClass() const {
     return static_cast<StmtClass>(StmtBits.sClass);
   }
 
-  Stmt(const Stmt &) = delete;
-  Stmt(Stmt &&) = delete;
-  Stmt &operator=(const Stmt &) = delete;
-  Stmt &operator=(Stmt &&) = delete;
-
   const char *getStmtClassName() const;
 
   bool isOMPStructuredBlock() const { return StmtBits.IsOMPStructuredBlock; }
   void setIsOMPStructuredBlock(bool IsOMPStructuredBlock) {
     StmtBits.IsOMPStructuredBlock = IsOMPStructuredBlock;
   }
 
   /// SourceLocation tokens are not useful in isolation - they are low level

lib/CodeGen/CGBuiltin.cpp

for (const auto &Item : Layout.Items)
       OS << "_" << int(Item.getSizeByte()) << "_"
          << int(Item.getDescriptorByte());
   }
 
   if (llvm::Function *F = CGM.getModule().getFunction(Name))
     return F;
 
   llvm::SmallVector<QualType, 4> ArgTys;
-  llvm::SmallVector<ImplicitParamDecl, 4> Params;
-  Params.emplace_back(Ctx, nullptr, SourceLocation(), &Ctx.Idents.get("buffer"),
-                      Ctx.VoidPtrTy, ImplicitParamDecl::Other);
+  FunctionArgList Args;
+  Args.push_back(ImplicitParamDecl::Create(
+      Ctx, nullptr, SourceLocation(), &Ctx.Idents.get("buffer"), Ctx.VoidPtrTy,
+      ImplicitParamDecl::Other));
   ArgTys.emplace_back(Ctx.VoidPtrTy);
 
   for (unsigned int I = 0, E = Layout.Items.size(); I < E; ++I) {
     char Size = Layout.Items[I].getSizeByte();
     if (!Size)
       continue;
 
     QualType ArgTy = getOSLogArgType(Ctx, Size);
-    Params.emplace_back(
+    Args.push_back(ImplicitParamDecl::Create(
         Ctx, nullptr, SourceLocation(),
         &Ctx.Idents.get(std::string("arg") + llvm::to_string(I)), ArgTy,
-        ImplicitParamDecl::Other);
+        ImplicitParamDecl::Other));
     ArgTys.emplace_back(ArgTy);
   }
 
-  FunctionArgList Args;
-  for (auto &P : Params)
-    Args.push_back(&P);
-
   QualType ReturnTy = Ctx.VoidTy;
   QualType FuncionTy = Ctx.getFunctionType(ReturnTy, ArgTys, {});
 
   // The helper function has linkonce_odr linkage to enable the linker to merge
   // identical functions. To ensure the merging always happens, 'noinline' is
   // attached to the function when compiling with -Oz.
   const CGFunctionInfo &FI =
       CGM.getTypes().arrangeBuiltinFunctionDeclaration(ReturnTy, Args);
FunctionDecl *FD = FunctionDecl::Create(
       FuncionTy, nullptr, SC_PrivateExtern, false, false);
 
   StartFunction(FD, ReturnTy, Fn, FI, Args);
 
   // Create a scope with an artificial location for the body of this function.
   auto AL = ApplyDebugLocation::CreateArtificial(*this);
 
   CharUnits Offset;
-  Address BufAddr(Builder.CreateLoad(GetAddrOfLocalVar(&Params[0]), "buf"),
+  Address BufAddr(Builder.CreateLoad(GetAddrOfLocalVar(Args[0]), "buf"),
                   BufferAlignment);
   Builder.CreateStore(Builder.getInt8(Layout.getSummaryByte()),
                       Builder.CreateConstByteGEP(BufAddr, Offset++, "summary"));
   Builder.CreateStore(Builder.getInt8(Layout.getNumArgsByte()),
                       Builder.CreateConstByteGEP(BufAddr, Offset++, "numArgs"));
 
   unsigned I = 1;
   for (const auto &Item : Layout.Items) {
     Builder.CreateStore(
         Builder.getInt8(Item.getDescriptorByte()),
         Builder.CreateConstByteGEP(BufAddr, Offset++, "argDescriptor"));
     Builder.CreateStore(
         Builder.getInt8(Item.getSizeByte()),
         Builder.CreateConstByteGEP(BufAddr, Offset++, "argSize"));
 
     CharUnits Size = Item.size();
     if (!Size.getQuantity())
       continue;
 
-    Address Arg = GetAddrOfLocalVar(&Params[I]);
+    Address Arg = GetAddrOfLocalVar(Args[I]);
     Address Addr = Builder.CreateConstByteGEP(BufAddr, Offset, "argData");
     Addr = Builder.CreateBitCast(Addr, Arg.getPointer()->getType(),
                                  "argDataCast");
     Builder.CreateStore(Builder.CreateLoad(Arg), Addr);
     Offset += Size;
     ++I;
   }