hubert.reinterpretcast added inline comments.
================
Comment at: include/clang/Sema/Sema.h:6194
+ SourceLocation TemplateLoc,
+ const TemplateArgumentListInfo *TemplateArgs);
+
----------------
changyu wrote:
> hubert.reinterpretcast wrote:
> > Indentation issue here too.
> That last line is 79 characters long.
clang-format is happy to give:
```
ExprResult
CheckConceptTemplateId(const CXXScopeSpec &SS,
const DeclarationNameInfo &NameInfo,
ConceptDecl *Template, SourceLocation TemplateLoc,
const TemplateArgumentListInfo *TemplateArgs);
```
I'm no fan of blindly using clang-format, but its output is sometimes useful.
================
Comment at: lib/Parse/ParseTemplate.cpp:374
+
+ ExprResult ConstraintExpr = ParseConstraintExpression();
+
----------------
changyu wrote:
> hubert.reinterpretcast wrote:
> > changyu wrote:
> > > saar.raz wrote:
> > > > Add a check to ParseConstraintExpression that the type is either
> > > > dependent or bool, and add an apropriate diagnostic.
> > > >
> > > > ```
> > > > if (!ConstraintExpr->isTypeDependent() && ConstraintExpr->getType()
> > > > != Context.BoolTy) {
> > > > Diag(Init->getSourceRange().getBegin(),
> > > > diag::err_concept_initialized_with_non_bool_type) <<
> > > > Init->getType();
> > > > Concept->setInvalidDecl();
> > > > return;
> > > > }
> > > > ```
> > > I'm guessing you meant for this to be in `class Sema` so I added this to
> > > `Sema::ActOnConceptDefinition`. Also what is `Init` here?
> > I think that would still need a TODO to instead walk the constraint
> > expression for atomic constraints and diagnose those.
> > ```
> > template <typename T>
> > concept C = 1 || T::value; // error
> > ```
> Why is that an error? [temp.constr.normal] in p0734r0 seems to say it's valid?
From N4700 subclause 17.4.1.2 [temp.constr.atomic] paragraph 3:
[ ... ], and E shall be a constant expression of type bool
A search of "bool" in P0734R0 seems to indicate that is also the basis for the
diagnostic Saar is requesting.
Although that wording only applies clearly when determining the satisfaction of
C<T> for some T, it would be good to catch it early. I believe that the
particular case I presented falls under the "no valid specialization" wording
in [temp.res].
I think there is a gap between the wording and the intent if overloaded binary
logical operators, detectable without substitution, are not sufficiently wrong
on the part of the user that a compiler may refuse to translate the program.
================
Comment at: lib/Sema/SemaTemplate.cpp:7693
+Decl *Sema::ActOnConceptDefinition(Scope *S,
+ MultiTemplateParamsArg TemplateParameterLists,
+ IdentifierInfo *Name, SourceLocation L,
----------------
changyu wrote:
> Rakete1111 wrote:
> > Did you run this through clang-format?
> No, when I run the file through clang-format (with no arguments except the
> file), it reformats the whole file. How should I be running clang-format?
One workflow that works is to clang-format the base file, clang-format with
your changes, grab a patch and then apply it to the original base file
(probably needs some manual work).
https://reviews.llvm.org/D40381
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
