[PATCH] D50764: [AST] Make NullStmt final and give it factory functions

[Hamza Sood via Phabricator via cfe-commits]

hamzasood added a comment.

Thanks. I'm not planning to move forward with this until the contracts patch is 
ready (it might change enough to not even require this); that should leave 
plenty of time for others to take a look.


Repository:
  rC Clang

https://reviews.llvm.org/D50764



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

[PATCH] D50764: [AST] Make NullStmt final and give it factory functions

[Stephen Kelly via Phabricator via cfe-commits]

steveire accepted this revision.
steveire added a comment.
This revision is now accepted and ready to land.

This looks like a NFC change.

Given that the next patch moves these methods out of line, you might consider 
introducing them out of line here (and moving the constructors out of line). 
That would make the next patch easier to review because the addition of 
assertion handling would be move visible.

LGTM, but I'm new here, so you may want to wait for another reviewer if you 
wish.


Repository:
  rC Clang

https://reviews.llvm.org/D50764



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

[PATCH] D50764: [AST] Make NullStmt final and give it factory functions

[Hamza Sood via Phabricator via cfe-commits]

hamzasood created this revision.
hamzasood added reviewers: klimek, steveire, bkramer.
Herald added a subscriber: cfe-commits.

I've submitted a patch for contracts (review linked to this) that adds trailing 
objects to NullStmt. This patch contains the changes to make that possible.


Repository:
  rC Clang

https://reviews.llvm.org/D50764

Files:
  include/clang/AST/Stmt.h
  lib/AST/ASTImporter.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp

Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -3141,7 +3141,7 @@
   break;
 
 case STMT_NULL:
-  S = new (Context) NullStmt(Empty);
+  S = NullStmt::CreateEmpty(Context);
   break;
 
 case STMT_COMPOUND:
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -6650,7 +6650,7 @@
 if (Then.isInvalid())
   return StmtError();
   } else {
-Then = new (getSema().Context) NullStmt(S->getThen()->getBeginLoc());
+Then = NullStmt::Create(getSema().Context, S->getThen()->getBeginLoc());
   }
 
   // Transform the "else" branch.
@@ -7511,13 +7511,13 @@
 if (S->isIfExists())
   break;
 
-return new (getSema().Context) NullStmt(S->getKeywordLoc());
+return NullStmt::Create(getSema().Context, S->getKeywordLoc());
 
   case Sema::IER_DoesNotExist:
 if (S->isIfNotExists())
   break;
 
-return new (getSema().Context) NullStmt(S->getKeywordLoc());
+return NullStmt::Create(getSema().Context, S->getKeywordLoc());
 
   case Sema::IER_Dependent:
 Dependent = true;
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -67,7 +67,7 @@
 
 StmtResult Sema::ActOnNullStmt(SourceLocation SemiLoc,
bool HasLeadingEmptyMacro) {
-  return new (Context) NullStmt(SemiLoc, HasLeadingEmptyMacro);
+  return NullStmt::Create(Context, SemiLoc, HasLeadingEmptyMacro);
 }
 
 StmtResult Sema::ActOnDeclStmt(DeclGroupPtrTy dg, SourceLocation StartLoc,
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -5044,8 +5044,8 @@
 
 Stmt *ASTNodeImporter::VisitNullStmt(NullStmt *S) {
   SourceLocation ToSemiLoc = Importer.Import(S->getSemiLoc());
-  return new (Importer.getToContext()) NullStmt(ToSemiLoc,
-S->hasLeadingEmptyMacro());
+  return NullStmt::Create(Importer.getToContext(), ToSemiLoc,
+  S->hasLeadingEmptyMacro());
 }
 
 Stmt *ASTNodeImporter::VisitCompoundStmt(CompoundStmt *S) {
Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -587,26 +587,36 @@
 
 /// NullStmt - This is the null statement ";": C99 6.8.3p3.
 ///
-class NullStmt : public Stmt {
+class NullStmt final : public Stmt {
   SourceLocation SemiLoc;
 
   /// True if the null statement was preceded by an empty macro, e.g:
   /// @code
   ///   #define CALL(x)
   ///   CALL(0);
   /// @endcode
-  bool HasLeadingEmptyMacro = false;
+  bool HasLeadingEmptyMacro;
+
+  NullStmt(SourceLocation L, bool hasLeadingEmptyMacro)
+  : Stmt(NullStmtClass), SemiLoc(L),
+HasLeadingEmptyMacro(hasLeadingEmptyMacro) {}
+
+  explicit NullStmt(EmptyShell Empty)
+  : Stmt(NullStmtClass, Empty), HasLeadingEmptyMacro(false) {}
 
 public:
   friend class ASTStmtReader;
   friend class ASTStmtWriter;
 
-  NullStmt(SourceLocation L, bool hasLeadingEmptyMacro = false)
-  : Stmt(NullStmtClass), SemiLoc(L),
-HasLeadingEmptyMacro(hasLeadingEmptyMacro) {}
+  static NullStmt *Create(const ASTContext , SourceLocation L,
+  bool hasLeadingEmptyMacro = false) {
+return new (C) NullStmt(L, hasLeadingEmptyMacro);
+  }
 
   /// Build an empty null statement.
-  explicit NullStmt(EmptyShell Empty) : Stmt(NullStmtClass, Empty) {}
+  static NullStmt *CreateEmpty(const ASTContext ) {
+return new (C) NullStmt(EmptyShell{});
+  }
 
   SourceLocation getSemiLoc() const { return SemiLoc; }
   void setSemiLoc(SourceLocation L) { SemiLoc = L; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits