diff --git a/clang/include/clang/AST/ASTConcept.h b/clang/include/clang/AST/ASTConcept.h --- a/clang/include/clang/AST/ASTConcept.h +++ b/clang/include/clang/AST/ASTConcept.h @@ -25,6 +25,14 @@ class ConceptDecl; class ConceptSpecializationExpr; +struct ConceptSubstitutionDiagnostic { + SourceLocation Loc; + // FIXME: Store diagnostics semantically and not as prerendered strings. + // Fixing this probably requires serialization of PartialDiagnostic + // objects. + StringRef Msg; +}; + /// The result of a constraint satisfaction check, containing the necessary /// information to diagnose an unsatisfied constraint. class ConstraintSatisfaction : public llvm::FoldingSetNode { @@ -42,7 +50,7 @@ ConstraintOwner(ConstraintOwner), TemplateArgs(TemplateArgs.begin(), TemplateArgs.end()) { } - using SubstitutionDiagnostic = std::pair; + using SubstitutionDiagnostic = ConceptSubstitutionDiagnostic; using Detail = llvm::PointerUnion; bool IsSatisfied = false; @@ -68,8 +76,7 @@ /// an invalid expression. using UnsatisfiedConstraintRecord = std::pair *>>; + llvm::PointerUnion>; /// \brief The result of a constraint satisfaction check, containing the /// necessary information to diagnose an unsatisfied constraint. diff --git a/clang/include/clang/AST/ExprConcepts.h b/clang/include/clang/AST/ExprConcepts.h --- a/clang/include/clang/AST/ExprConcepts.h +++ b/clang/include/clang/AST/ExprConcepts.h @@ -43,7 +43,7 @@ friend class ASTStmtReader; friend TrailingObjects; public: - using SubstitutionDiagnostic = std::pair; + using SubstitutionDiagnostic = ConceptSubstitutionDiagnostic; protected: /// \brief The number of template arguments in the tail-allocated list of @@ -160,11 +160,7 @@ public: struct SubstitutionDiagnostic { StringRef SubstitutedEntity; - // FIXME: Store diagnostics semantically and not as prerendered strings. - // Fixing this probably requires serialization of PartialDiagnostic - // objects. - SourceLocation DiagLoc; - StringRef DiagMessage; + ConceptSubstitutionDiagnostic Diag; }; Requirement(RequirementKind Kind, bool IsDependent, diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2785,7 +2785,7 @@ "constraints not satisfied for %select{class template|function template|variable template|alias template|" "template template parameter|template}0 %1%2">; def note_substituted_constraint_expr_is_ill_formed : Note< - "because substituted constraint expression is ill-formed%0">; + "because substituted constraint expression is ill-formed%select{: %1|}0">; def note_atomic_constraint_evaluated_to_false : Note< "%select{and|because}0 '%1' evaluated to false">; def note_concept_specialization_constraint_evaluated_to_false : Note< diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -6841,6 +6841,9 @@ bool MaybeEmitAmbiguousAtomicConstraintsDiagnostic(NamedDecl *D1, ArrayRef AC1, NamedDecl *D2, ArrayRef AC2); + ConceptSubstitutionDiagnostic + getConceptSubstitutionDiagnostic(sema::TemplateDeductionInfo &TDI); + /// \brief Check whether the given list of constraint expressions are /// satisfied (as if in a 'conjunction') given template arguments. /// \param Template the template-like entity that triggered the constraints diff --git a/clang/lib/AST/ASTConcept.cpp b/clang/lib/AST/ASTConcept.cpp --- a/clang/lib/AST/ASTConcept.cpp +++ b/clang/lib/AST/ASTConcept.cpp @@ -33,12 +33,10 @@ Detail.second.get())}; else { auto &SubstitutionDiagnostic = - *Detail.second.get *>(); - unsigned MessageSize = SubstitutionDiagnostic.second.size(); - char *Mem = new (C) char[MessageSize]; - memcpy(Mem, SubstitutionDiagnostic.second.data(), MessageSize); - auto *NewSubstDiag = new (C) std::pair( - SubstitutionDiagnostic.first, StringRef(Mem, MessageSize)); + *Detail.second.get(); + auto *NewSubstDiag = new (C) ConceptSubstitutionDiagnostic{ + SubstitutionDiagnostic.Loc, + SubstitutionDiagnostic.Msg.copy(C.getAllocator())}; new (getTrailingObjects() + I) UnsatisfiedConstraintRecord{Detail.first, UnsatisfiedConstraintRecord::second_type( diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -123,6 +123,25 @@ return true; } +// FIXME: Concepts: This is an unfortunate consequence of there +// being no serialization code for PartialDiagnostics and the fact +// that serializing them would likely take a lot more storage than +// just storing them as strings. We would still like, in the +// future, to serialize the proper PartialDiagnostic as serializing +// it as a string defeats the purpose of the diagnostic mechanism. +ConceptSubstitutionDiagnostic +Sema::getConceptSubstitutionDiagnostic(TemplateDeductionInfo &TDI) { + if (!TDI.hasSFINAEDiagnostic()) + return {TDI.getLocation(), StringRef()}; + + PartialDiagnosticAt SubstDiag{SourceLocation(), + PartialDiagnostic::NullDiagnostic()}; + TDI.takeSFINAEDiagnostic(SubstDiag); + SmallString<128> DiagString; + SubstDiag.second.EmitToString(getDiagnostics(), DiagString); + return {SubstDiag.first, DiagString.str().copy(Context.getAllocator())}; +} + template static bool calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr, @@ -230,37 +249,21 @@ if (!SubstitutedExpression.isInvalid()) SubstitutedExpression = S.PerformContextuallyConvertToBool(SubstitutedExpression.get()); - if (SubstitutedExpression.isInvalid() || Trap.hasErrorOccurred()) { + if (Trap.hasErrorOccurred()) { // C++2a [temp.constr.atomic]p1 // ...If substitution results in an invalid type or expression, the // constraint is not satisfied. - if (!Trap.hasErrorOccurred()) - // A non-SFINAE error has occured as a result of this - // substitution. - return ExprError(); - - PartialDiagnosticAt SubstDiag{SourceLocation(), - PartialDiagnostic::NullDiagnostic()}; - Info.takeSFINAEDiagnostic(SubstDiag); - // FIXME: Concepts: This is an unfortunate consequence of there - // being no serialization code for PartialDiagnostics and the fact - // that serializing them would likely take a lot more storage than - // just storing them as strings. We would still like, in the - // future, to serialize the proper PartialDiagnostic as serializing - // it as a string defeats the purpose of the diagnostic mechanism. - SmallString<128> DiagString; - DiagString = ": "; - SubstDiag.second.EmitToString(S.getDiagnostics(), DiagString); - unsigned MessageSize = DiagString.size(); - char *Mem = new (S.Context) char[MessageSize]; - memcpy(Mem, DiagString.c_str(), MessageSize); Satisfaction.Details.emplace_back( AtomicExpr, - new (S.Context) ConstraintSatisfaction::SubstitutionDiagnostic{ - SubstDiag.first, StringRef(Mem, MessageSize)}); + new (S.Context) ConstraintSatisfaction::SubstitutionDiagnostic( + S.getConceptSubstitutionDiagnostic(Info))); Satisfaction.IsSatisfied = false; return ExprEmpty(); } + if (SubstitutedExpression.isInvalid()) + // A non-SFINAE error has occured as a result of this + // substitution. + return ExprError(); } if (!S.CheckConstraintExpression(SubstitutedExpression.get())) @@ -422,13 +425,13 @@ break; case concepts::ExprRequirement::SS_ExprSubstitutionFailure: { auto *SubstDiag = Req->getExprSubstitutionDiagnostic(); - if (!SubstDiag->DiagMessage.empty()) - S.Diag(SubstDiag->DiagLoc, + if (!SubstDiag->Diag.Msg.empty()) + S.Diag(SubstDiag->Diag.Loc, diag::note_expr_requirement_expr_substitution_error) - << (int)First << SubstDiag->SubstitutedEntity - << SubstDiag->DiagMessage; + << (int)First << SubstDiag->SubstitutedEntity + << SubstDiag->Diag.Msg; else - S.Diag(SubstDiag->DiagLoc, + S.Diag(SubstDiag->Diag.Loc, diag::note_expr_requirement_expr_unknown_substitution_error) << (int)First << SubstDiag->SubstitutedEntity; break; @@ -441,14 +444,16 @@ case concepts::ExprRequirement::SS_TypeRequirementSubstitutionFailure: { auto *SubstDiag = Req->getReturnTypeRequirement().getSubstitutionDiagnostic(); - if (!SubstDiag->DiagMessage.empty()) - S.Diag(SubstDiag->DiagLoc, + if (!SubstDiag->Diag.Msg.empty()) + S.Diag(SubstDiag->Diag.Loc, diag::note_expr_requirement_type_requirement_substitution_error) << (int)First << SubstDiag->SubstitutedEntity - << SubstDiag->DiagMessage; + << SubstDiag->Diag.Msg; else - S.Diag(SubstDiag->DiagLoc, - diag::note_expr_requirement_type_requirement_unknown_substitution_error) + S.Diag( + SubstDiag->Diag.Loc, + diag:: + note_expr_requirement_type_requirement_unknown_substitution_error) << (int)First << SubstDiag->SubstitutedEntity; break; } @@ -487,12 +492,12 @@ return; case concepts::TypeRequirement::SS_SubstitutionFailure: { auto *SubstDiag = Req->getSubstitutionDiagnostic(); - if (!SubstDiag->DiagMessage.empty()) - S.Diag(SubstDiag->DiagLoc, - diag::note_type_requirement_substitution_error) << (int)First - << SubstDiag->SubstitutedEntity << SubstDiag->DiagMessage; + if (!SubstDiag->Diag.Msg.empty()) + S.Diag(SubstDiag->Diag.Loc, + diag::note_type_requirement_substitution_error) + << (int)First << SubstDiag->SubstitutedEntity << SubstDiag->Diag.Msg; else - S.Diag(SubstDiag->DiagLoc, + S.Diag(SubstDiag->Diag.Loc, diag::note_type_requirement_unknown_substitution_error) << (int)First << SubstDiag->SubstitutedEntity; return; @@ -509,13 +514,12 @@ if (Req->isSubstitutionFailure()) { concepts::Requirement::SubstitutionDiagnostic *SubstDiag = Req->getSubstitutionDiagnostic(); - if (!SubstDiag->DiagMessage.empty()) - S.Diag(SubstDiag->DiagLoc, + if (!SubstDiag->Diag.Msg.empty()) + S.Diag(SubstDiag->Diag.Loc, diag::note_nested_requirement_substitution_error) - << (int)First << SubstDiag->SubstitutedEntity - << SubstDiag->DiagMessage; + << (int)First << SubstDiag->SubstitutedEntity << SubstDiag->Diag.Msg; else - S.Diag(SubstDiag->DiagLoc, + S.Diag(SubstDiag->Diag.Loc, diag::note_nested_requirement_unknown_substitution_error) << (int)First << SubstDiag->SubstitutedEntity; return; @@ -631,8 +635,8 @@ const llvm::PointerUnion &Record, bool First = true) { if (auto *Diag = Record.template dyn_cast()){ - S.Diag(Diag->first, diag::note_substituted_constraint_expr_is_ill_formed) - << Diag->second; + S.Diag(Diag->Loc, diag::note_substituted_constraint_expr_is_ill_formed) + << Diag->Msg.empty() << Diag->Msg; return; } diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -1185,6 +1185,8 @@ concepts::NestedRequirement * TransformNestedRequirement(concepts::NestedRequirement *Req); + ExprResult TransformConceptSpecializationExpr(ConceptSpecializationExpr *E); + private: ExprResult transformNonTypeTemplateParmRef(NonTypeTemplateParmDecl *parm, SourceLocation loc, @@ -1878,27 +1880,12 @@ template static concepts::Requirement::SubstitutionDiagnostic * createSubstDiag(Sema &S, TemplateDeductionInfo &Info, EntityPrinter Printer) { - SmallString<128> Message; - SourceLocation ErrorLoc; - if (Info.hasSFINAEDiagnostic()) { - PartialDiagnosticAt PDA(SourceLocation(), - PartialDiagnostic::NullDiagnostic{}); - Info.takeSFINAEDiagnostic(PDA); - PDA.second.EmitToString(S.getDiagnostics(), Message); - ErrorLoc = PDA.first; - } else { - ErrorLoc = Info.getLocation(); - } - char *MessageBuf = new (S.Context) char[Message.size()]; - std::copy(Message.begin(), Message.end(), MessageBuf); SmallString<128> Entity; llvm::raw_svector_ostream OS(Entity); Printer(OS); - char *EntityBuf = new (S.Context) char[Entity.size()]; - std::copy(Entity.begin(), Entity.end(), EntityBuf); return new (S.Context) concepts::Requirement::SubstitutionDiagnostic{ - StringRef(EntityBuf, Entity.size()), ErrorLoc, - StringRef(MessageBuf, Message.size())}; + Entity.str().copy(S.Context.getAllocator()), + S.getConceptSubstitutionDiagnostic(Info)}; } concepts::TypeRequirement * @@ -2030,6 +2017,46 @@ return RebuildNestedRequirement(TransConstraint.get()); } +ExprResult TemplateInstantiator::TransformConceptSpecializationExpr( + ConceptSpecializationExpr *E) { + + Sema::SFINAETrap Trap(SemaRef); + TemplateDeductionInfo Info(E->getTemplateKWLoc()); + Sema::InstantiatingTemplate Inst( + SemaRef, E->getBeginLoc(), + Sema::InstantiatingTemplate::ConstraintSubstitution{}, + E->getNamedConcept(), Info, E->getSourceRange()); + if (Inst.isInvalid()) + return ExprError(); + + CXXScopeSpec SS; + SS.Adopt(E->getNestedNameSpecifierLoc()); + + const ASTTemplateArgumentListInfo *Old = E->getTemplateArgsAsWritten(); + TemplateArgumentListInfo TransArgs(Old->LAngleLoc, Old->RAngleLoc); + if (TransformTemplateArguments(Old->getTemplateArgs(), Old->NumTemplateArgs, + TransArgs) || + Trap.hasErrorOccurred()) { + ConstraintSatisfaction Satisfaction; + Satisfaction.Details.emplace_back( + nullptr, + new (SemaRef.Context) ConstraintSatisfaction::SubstitutionDiagnostic( + SemaRef.getConceptSubstitutionDiagnostic(Info))); + return ConceptSpecializationExpr::Create( + SemaRef.Context, + SS.isSet() ? SS.getWithLocInContext(SemaRef.Context) + : NestedNameSpecifierLoc{}, + E->getTemplateKWLoc(), E->getConceptNameInfo(), E->getFoundDecl(), + E->getNamedConcept(), Old, ArrayRef{}, &Satisfaction); + } + + ExprResult Result = SemaRef.CheckConceptTemplateId( + SS, E->getTemplateKWLoc(), E->getConceptNameInfo(), E->getFoundDecl(), + E->getNamedConcept(), &TransArgs); + if (Result.isInvalid()) + return ExprError(); + return Result; +} /// Perform substitution on the type T with a given set of template /// arguments. diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -3357,25 +3357,6 @@ return getSema().BuildSourceLocExpr(Kind, BuiltinLoc, RPLoc, ParentContext); } - /// Build a new Objective-C boxed expression. - /// - /// By default, performs semantic analysis to build the new expression. - /// Subclasses may override this routine to provide different behavior. - ExprResult RebuildConceptSpecializationExpr(NestedNameSpecifierLoc NNS, - SourceLocation TemplateKWLoc, DeclarationNameInfo ConceptNameInfo, - NamedDecl *FoundDecl, ConceptDecl *NamedConcept, - TemplateArgumentListInfo *TALI) { - CXXScopeSpec SS; - SS.Adopt(NNS); - ExprResult Result = getSema().CheckConceptTemplateId(SS, TemplateKWLoc, - ConceptNameInfo, - FoundDecl, - NamedConcept, TALI); - if (Result.isInvalid()) - return ExprError(); - return Result; - } - /// \brief Build a new requires expression. /// /// By default, performs semantic analysis to build the new expression. @@ -12271,20 +12252,10 @@ E->getEndLoc()); } -template -ExprResult -TreeTransform::TransformConceptSpecializationExpr( - ConceptSpecializationExpr *E) { - const ASTTemplateArgumentListInfo *Old = E->getTemplateArgsAsWritten(); - TemplateArgumentListInfo TransArgs(Old->LAngleLoc, Old->RAngleLoc); - if (getDerived().TransformTemplateArguments(Old->getTemplateArgs(), - Old->NumTemplateArgs, TransArgs)) - return ExprError(); - - return getDerived().RebuildConceptSpecializationExpr( - E->getNestedNameSpecifierLoc(), E->getTemplateKWLoc(), - E->getConceptNameInfo(), E->getFoundDecl(), E->getNamedConcept(), - &TransArgs); +template +ExprResult TreeTransform::TransformConceptSpecializationExpr( + ConceptSpecializationExpr *E) { + return E; } template diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp --- a/clang/lib/Serialization/ASTWriterStmt.cpp +++ b/clang/lib/Serialization/ASTWriterStmt.cpp @@ -409,10 +409,9 @@ if (E) Record.AddStmt(E); else { - auto *Diag = DetailRecord.second.get *>(); - Record.AddSourceLocation(Diag->first); - Record.AddString(Diag->second); + auto *Diag = DetailRecord.second.get(); + Record.AddSourceLocation(Diag->Loc); + Record.AddString(Diag->Msg); } } } @@ -423,8 +422,8 @@ ASTRecordWriter &Record, const concepts::Requirement::SubstitutionDiagnostic *D) { Record.AddString(D->SubstitutedEntity); - Record.AddSourceLocation(D->DiagLoc); - Record.AddString(D->DiagMessage); + Record.AddSourceLocation(D->Diag.Loc); + Record.AddString(D->Diag.Msg); } void ASTStmtWriter::VisitConceptSpecializationExpr( diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp --- a/clang/test/SemaTemplate/concepts.cpp +++ b/clang/test/SemaTemplate/concepts.cpp @@ -169,3 +169,24 @@ template void f(T, U) = delete; void g() { f(0, 0); } } + +namespace PR50864 { + template concept Valid = T::valid; // expected-note {{evaluated to false}} + template struct A { + template void f(U) requires Valid; + // expected-note@-1 {{candidate template ignored: constraints not satisfied [with U = int]}} + // expected-note@-2 {{because 'typename T::type' does not satisfy 'Valid'}} + // expected-note@-3 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}} + template void f(U) requires Valid; + // expected-note@-1 {{candidate template ignored: constraints not satisfied [with U = int]}} + // expected-note@-2 {{does not satisfy 'Valid'}} + }; + + template struct X { static constexpr bool valid = V; }; + + struct T1 : X {}; + void t1() { A().f(1); } // expected-error {{no matching member function for call to 'f'}} + + struct T2 : X {}; + void t2() { A().f(1); } +}