[PATCH] D40381: Parse concept definition

[changyu via Phabricator via cfe-commits]

changyu marked 27 inline comments as done.
changyu added inline comments.


================
Comment at: lib/Sema/SemaTemplate.cpp:7713
+
+  if (!ConstraintExpr->isTypeDependent() &&
+      ConstraintExpr->getType() != Context.BoolTy) {
----------------
faisalv wrote:
> saar.raz wrote:
> > faisalv wrote:
> > > Consider refactoring these checks on constraint expressions into a separate function checkConstraintExpression (that we can also call from other contexts such as requires-clauses and nested requires expressions)?
> > I did that in the upcoming patches, no need to do it here as well.
> Once again - relying on a future patch (especially without a clear FIXME) to tweak the architectural/factorization issues that you have been made aware of (and especially those, such as this, that do not require too much effort), is not practice I would generally encourage.  
> 
> So changyu, if you agree that these suggestions would improve the quality of the patch and leave clang in a more robust state (maintenance-wise or execution-wise), then please make the changes.  If not, please share your thoughts as to why these suggestions would either not be an improvement or be inconsistent with the theme of this patch.
> 
> Thanks!
I removed it here since Saar fixed it in his patch.


https://reviews.llvm.org/D40381