[PATCH] D50360: [Concepts] Requires Expressions

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.


================
Comment at: include/clang/AST/ExprCXX.h:4671
+  bool IsSatisfied;
+  SourceLocation RequiresKWLoc;
+  RequiresExprBodyDecl *Body;
----------------
You can stash these in the bit-field classes of `Stmt` to save some space.


================
Comment at: include/clang/AST/ExprCXX.h:4674
+  llvm::SmallVector<ParmVarDecl *, 2> LocalParameters;
+  llvm::SmallVector<Requirement *, 3> Requirements;
+  SourceLocation RBraceLoc;
----------------
Can you tail-allocate them ?


================
Comment at: include/clang/AST/ExprCXX.h:4703
+  }
+
+  SourceLocation getRequiresKWLoc() const { return RequiresKWLoc; }
----------------
If in general you don't strictly need to modify some internal state of an AST node, I think it would be better to not provide the corresponding method.


================
Comment at: include/clang/AST/ExprCXX.h:4715
+  SourceLocation getBeginLoc() const LLVM_READONLY { return RequiresKWLoc; }
+  SourceLocation getEndLoc() const LLVM_READONLY {
+    return RBraceLoc;
----------------
`LLVM_READONLY` is pointless here.


================
Comment at: include/clang/AST/ExprCXX.h:4722
+    return child_range(child_iterator(), child_iterator());
+  }
+};
----------------
You should also provide the const-qualified version.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D50360/new/

https://reviews.llvm.org/D50360