rsmith added a comment.
(Partial comments; I'll try to complete the review today or tomorrow.)
================
Comment at: clang/include/clang/AST/ExprCXX.h:4933
+
+/// \brief A static requirement that can be used in a requires-expression to
----------------
You're adding a total of ~400 lines here; that seems sufficient to factor out
into a separate header (`ExprConcepts.h`, containing this and
`ConceptSpecializationExpr` maybe?)
================
Comment at: clang/include/clang/AST/ExprCXX.h:4936
+/// check properties of types and expression.
+class Requirement {
+public:
----------------
This name (`clang::Requirement`) is a little too general for my tastes;
consider moving these to a sub-namespace.
================
Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2714-2716
+ if (RetReq.isTypeConstraint())
+ TRY_TO(TraverseTemplateParameterListHelper(
+ RetReq.getTypeConstraintTemplateParameterList()));
----------------
We should traverse the entire //type-constraint// here, including the nested
name specifier and concept name.
================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:766
+def err_requires_expr_simple_requirement_noexcept : Error<
+ "'noexcept' can only be used in a compound requirements (with '{' '}' around
"
+ "the expression)">;
----------------
requirements -> requirement
================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:774
+ "you intend to place it in a nested requirement (add another 'requires' "
+ "before the expression)">, InGroup<DiagGroup<"requires-expression">>;
----------------
Missing `?` somewhere in this diagnostic, I think.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2638
+ "variable template|alias template|template template parameter|template}1 "
+ "and not a type template">;
+def err_type_requirement_no_such_type : Error<
----------------
" and not" -> ", not"
would be a bit more consistent with how we phrase similar things elsewhere.
================
Comment at: clang/include/clang/Sema/Sema.h:6285
void
- DiagnoseUnsatisfiedConstraint(const ConstraintSatisfaction& Satisfaction);
+ DiagnoseUnsatisfiedConstraint(const ConstraintSatisfaction& Satisfaction,
+ bool First = true);
----------------
`&` on the right, please (here and below).
================
Comment at: clang/lib/AST/ExprCXX.cpp:1848-1855
+const TypeConstraint *
+ExprRequirement::ReturnTypeRequirement::getTypeConstraint() const {
+ assert(isTypeConstraint());
+ auto TPL =
+ TypeConstraintInfo.getPointer().get<TemplateParameterList *>();
+ return cast<TemplateTypeParmDecl>(TPL->getParam(0))
+ ->getTypeConstraint();
----------------
This representation for a type-constraint seems a bit surprising. Do we really
need the template parameter + template parameter list here? It seems to me like
we should only really need the `TypeConstraint` itself, plus its
immediately-declared constraint.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D50360/new/
https://reviews.llvm.org/D50360
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
