[PATCH] D41217: [Concepts] Concept Specialization Expressions

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Only minor comments, let me know if you'd like me to take another look after addressing these, or just go ahead and commit. Thanks! Comment at:

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz marked 14 inline comments as done and 2 inline comments as done. saar.raz added inline comments. Comment at: lib/Sema/SemaConcept.cpp:51-68 + if (auto *BO = dyn_cast(ConstraintExpr)) { +if (BO->getOpcode() == BO_LAnd) { + if

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 224789. saar.raz marked 2 inline comments as done. saar.raz added a comment. Address CR comments by rsmith Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D41217/new/ https://reviews.llvm.org/D41217 Files:

[PATCH] D41217: [Concepts] Concept Specialization Expressions

rsmith marked an inline comment as done. rsmith added inline comments. Comment at: include/clang/Sema/Sema.h:5904 + Expr *ConstraintExpr, + bool ); + (Nit: please align the second

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 209686. saar.raz added a comment. Herald added a subscriber: erik.pilkington. Rebase onto trunk. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D41217/new/ https://reviews.llvm.org/D41217 Files: include/clang/AST/ExprCXX.h

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 204784. saar.raz added a comment. Delay support for mangling to later patch (where CSEs are formed instead of UnresolvedLookupExprs with dependent args) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D41217/new/

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 204764. saar.raz added a comment. Add support for CSE mangling Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D41217/new/ https://reviews.llvm.org/D41217 Files: include/clang/AST/ExprCXX.h

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 194979. saar.raz added a comment. Fix wrong patch uploaded Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D41217/new/ https://reviews.llvm.org/D41217 Files: include/clang/AST/DeclTemplate.h include/clang/AST/ExprCXX.h

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 194978. saar.raz added a comment. Add new CodeSynthesisContexts to switch where they were missing Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D41217/new/ https://reviews.llvm.org/D41217 Files:

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 194811. saar.raz marked 3 inline comments as done. saar.raz added a comment. Moved from getLocStart, getLocEnd to getBeginLoc, getEndLoc. Rebase onto master/trunk. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D41217/new/

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 161372. saar.raz added a comment. - Address CR comments - add FoundDecl tracking, instantiationdependent calculation, display non-constant expression diagnostic notes Repository: rC Clang https://reviews.llvm.org/D41217 Files:

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz added inline comments. Comment at: lib/Sema/SemaTemplateInstantiate.cpp:679-681 + Diags.Report(Active->PointOfInstantiation, + diag::note_constraint_substitution_here) + << Active->InstantiationRange; saar.raz wrote: >

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz added inline comments. Comment at: lib/Sema/SemaTemplateInstantiate.cpp:679-681 + Diags.Report(Active->PointOfInstantiation, + diag::note_constraint_substitution_here) + << Active->InstantiationRange; rsmith wrote: > Is

[PATCH] D41217: [Concepts] Concept Specialization Expressions

rsmith added inline comments. Comment at: include/clang/AST/ExprCXX.h:4420 + /// \brief The concept named. + ConceptDecl *NamedConcept; + saar.raz wrote: > rsmith wrote: > > You should also track the `FoundDecl` and the optional > > `NestedNameSpecifierLoc`

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 161343. saar.raz added a comment. - Fix bad ArgsAsWritten assertion, add missing null initializer in ConceptSpecializationExpr. Repository: rC Clang https://reviews.llvm.org/D41217 Files: include/clang/AST/DeclTemplate.h

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz added inline comments. Comment at: include/clang/AST/DeclTemplate.h:3063 SourceRange getSourceRange() const override LLVM_READONLY { -return SourceRange(getLocation(), getLocation()); +return SourceRange(getLocation(), getConstraintExpr()->getLocEnd()); }

[PATCH] D41217: [Concepts] Concept Specialization Expressions

steveire added inline comments. Comment at: include/clang/AST/DeclTemplate.h:3063 SourceRange getSourceRange() const override LLVM_READONLY { -return SourceRange(getLocation(), getLocation()); +return SourceRange(getLocation(), getConstraintExpr()->getLocEnd()); }

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 160437. saar.raz added a comment. Address Arthur's comments, add missing CorrectDelayedTyposInExpr Repository: rC Clang https://reviews.llvm.org/D41217 Files: include/clang/AST/DeclTemplate.h include/clang/AST/ExprCXX.h

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz added inline comments. Comment at: test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp:121 +template +concept C7 = sizeof(T) == 1 || sizeof(typename T3::type) == 1; // expected-note{{while substituting template arguments into constraint expression here}}

[PATCH] D41217: [Concepts] Concept Specialization Expressions

Quuxplusone added inline comments. Comment at: include/clang/Sema/Sema.h:7134 + // We are substituting template arguments into a constraint expression. + ConstraintSubstitution } Kind; Missing a trailing comma here. Comment at:

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 160237. saar.raz added a comment. Removed unused "note_in_concept_specialization" diagnostic ID. Repository: rC Clang https://reviews.llvm.org/D41217 Files: include/clang/AST/DeclTemplate.h include/clang/AST/ExprCXX.h

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 160216. saar.raz added a comment. Address Richard's CR comments. Among other things: - CSEs overhauled and now store both source and converted template arguments (latter are tail-allocated), template KW location and NNS. - CSEs no longer violate layering -

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz added inline comments. Comment at: lib/Sema/SemaConcept.cpp:34 + Diag(ConstraintExpression->getExprLoc(), + diag::err_non_bool_atomic_constraint) + << ConstraintExpression << ConstraintExpression->getType(); saar.raz wrote: >

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz added a comment. @rsmith - thanks for the responses! Will also address the comments I haven't responded to soon. Comment at: include/clang/AST/ExprCXX.h:4420 + /// \brief The concept named. + ConceptDecl *NamedConcept; + rsmith wrote: > You should

[PATCH] D41217: [Concepts] Concept Specialization Expressions

rsmith added inline comments. Comment at: include/clang/AST/ExprCXX.h:4420 + /// \brief The concept named. + ConceptDecl *NamedConcept; + You should also track the `FoundDecl` and the optional `NestedNameSpecifierLoc` (just like a `DeclRefExpr` would).

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 159234. saar.raz added a comment. Herald added a subscriber: jfb. - Switch to ASTTemplateArgumentListInfo, add ConstantEvaluated context when parsing constraints Repository: rC Clang https://reviews.llvm.org/D41217 Files:

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 140948. saar.raz added a comment. - Switched to piecewise substitution of atomic constraints when evaluating constraint expressions - Added tests for piecewise substitution behavior - Removed some redundant parentheses Repository: rC Clang

[PATCH] D41217: [Concepts] Concept Specialization Expressions

hubert.reinterpretcast added inline comments. Comment at: lib/AST/ExprCXX.cpp:1478 + { +// We do not want error diagnostics escaping here. +Sema::SFINAETrap Trap(S); saar.raz wrote: > hubert.reinterpretcast wrote: > > saar.raz wrote: > > > faisalv

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz added inline comments. Comment at: lib/AST/ExprCXX.cpp:1478 + { +// We do not want error diagnostics escaping here. +Sema::SFINAETrap Trap(S); hubert.reinterpretcast wrote: > saar.raz wrote: > > faisalv wrote: > > > Hubert: This needs a TODO:

[PATCH] D41217: [Concepts] Concept Specialization Expressions

hubert.reinterpretcast added inline comments. Comment at: lib/AST/ExprCXX.cpp:1478 + { +// We do not want error diagnostics escaping here. +Sema::SFINAETrap Trap(S); saar.raz wrote: > faisalv wrote: > > Hubert: This needs a TODO: the idea is not to drop

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 138499. saar.raz added a comment. Applied missing clang-format. Repository: rC Clang https://reviews.llvm.org/D41217 Files: include/clang/AST/DeclTemplate.h include/clang/AST/ExprCXX.h include/clang/AST/RecursiveASTVisitor.h

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 138497. saar.raz added a comment. Addressed most comments. Repository: rC Clang https://reviews.llvm.org/D41217 Files: include/clang/AST/DeclTemplate.h include/clang/AST/ExprCXX.h include/clang/AST/RecursiveASTVisitor.h

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz added inline comments. Comment at: lib/AST/ExprCXX.cpp:1478 + { +// We do not want error diagnostics escaping here. +Sema::SFINAETrap Trap(S); faisalv wrote: > Hubert: This needs a TODO: the idea is not to drop SFINAE errors, but to > avoid

[PATCH] D41217: [Concepts] Concept Specialization Expressions

faisalv added inline comments. Comment at: include/clang/AST/ExprCXX.h:4416 +/// According to C++2a [expr.prim.id]p3 an id-expression that denotes the +/// specialization of a concepts results in a prvalue of type bool. +class ConceptSpecializationExpr final : public Expr {

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 137906. saar.raz added a comment. - Fixed incorrect checking of atomic constraint types. Repository: rC Clang https://reviews.llvm.org/D41217 Files: include/clang/AST/DeclTemplate.h include/clang/AST/ExprCXX.h

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 133799. saar.raz added a comment. - Addressed comments: Added semantic tests for value and template concepts, removed extra period and modified CSE ctor to fit in nicer. Repository: rC Clang https://reviews.llvm.org/D41217 Files:

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz added inline comments. Comment at: test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp:12 + +template concept C3 = sizeof(*T{}) == 4; +static_assert(C3); Quuxplusone wrote: > "test/Parser/cxx-concept-declaration.cpp" has some syntax tests for

[PATCH] D41217: [Concepts] Concept Specialization Expressions

Quuxplusone added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2404 +def err_concept_non_constant_constraint_expression : Error< + "concept specialization '%0' resulted in a non-constant expression '%1'.">; +def err_non_bool_atomic_constraint : Error<

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 132743. saar.raz added a comment. Herald added subscribers: hintonda, mgorny. - Moved general concepts-related function into SemaConcept.cpp Repository: rC Clang https://reviews.llvm.org/D41217 Files: include/clang/AST/DeclTemplate.h

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 128121. saar.raz added a comment. Reverted to original constraint parsing code. https://reviews.llvm.org/D41217 Files: include/clang/AST/DeclTemplate.h include/clang/AST/ExprCXX.h include/clang/AST/RecursiveASTVisitor.h

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz added inline comments. Comment at: lib/Parse/ParseExpr.cpp:223 ExprResult Parser::ParseConstraintExpression() { - // FIXME: this may erroneously consume a function-body as the braced - // initializer list of a compound literal - // - // FIXME: this may erroneously

[PATCH] D41217: [Concepts] Concept Specialization Expressions

faisalv added inline comments. Comment at: lib/Parse/ParseExpr.cpp:223 ExprResult Parser::ParseConstraintExpression() { - // FIXME: this may erroneously consume a function-body as the braced - // initializer list of a compound literal - // - // FIXME: this may erroneously

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz marked 2 inline comments as done. saar.raz added a comment. Fixed indentations. Repository: rC Clang https://reviews.llvm.org/D41217 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 126953. saar.raz added a comment. Previous updated diff mistakenly included https://reviews.llvm.org/D40381, fixed that. Repository: rC Clang https://reviews.llvm.org/D41217 Files: include/clang/AST/DeclTemplate.h include/clang/AST/ExprCXX.h

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz updated this revision to Diff 126950. saar.raz added a comment. - Fixed indentation problems Repository: rC Clang https://reviews.llvm.org/D41217 Files: include/clang/AST/DeclTemplate.h include/clang/AST/ExprCXX.h include/clang/AST/RecursiveASTVisitor.h

[PATCH] D41217: [Concepts] Concept Specialization Expressions

changyu added inline comments. Comment at: lib/AST/ExprCXX.cpp:1481 +if (E.isInvalid() || Trap.hasErrorOccurred()) { +// C++2a [temp.constr.atomic]p1 +// ...If substitution results in an invalid type or expression, the You have four spaces

[PATCH] D41217: [Concepts] Concept Specialization Expressions

saar.raz created this revision. saar.raz added reviewers: changyu, rsmith, hubert.reinterpretcast, nwilson. Herald added a subscriber: cfe-commits. Part of the P0734r0 Concepts implementation effort. Added Concept Specialization Expressions and tests thereof. Also added constraint expression