Author: Erich Keane Date: 2022-10-04T10:32:48-07:00 New Revision: 3d7946c580055dade1dcf4b632738cc54e1a25a9
URL: https://github.com/llvm/llvm-project/commit/3d7946c580055dade1dcf4b632738cc54e1a25a9 DIFF: https://github.com/llvm/llvm-project/commit/3d7946c580055dade1dcf4b632738cc54e1a25a9.diff LOG: Implement DR2565: Invalid types in the parameter-declaration-clause of a requires-expression As reported: https://github.com/llvm/llvm-project/issues/57487 We properly treated a failed instantiation of a concept as a unsatisified constraint, however, we need to do this at the 'requires clause' level as well. This ensures that the parameters on a requires clause that fail instantiation will cause a satisfaction failure. This patch implements this by running requires parameter clause instantiation under a SFINAE trap, then stores any such failure as a requirement failure, so it can be diagnosed later. Added: clang/test/CXX/drs/dr25xx.cpp Modified: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/Sema.h clang/lib/Frontend/FrontendActions.cpp clang/lib/Sema/SemaTemplateInstantiate.cpp clang/lib/Sema/TreeTransform.h clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp clang/www/cxx_dr_status.html Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d113d9b450b74..9ece988929f4f 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -172,7 +172,10 @@ Bug Fixes - The template arguments of a variable template being accessed as a member will now be represented in the AST. - Fix incorrect handling of inline builtins with asm labels. - +- Finished implementing C++ DR2565, which results in a requirement becoming + not satisfied in the event of an instantiation failures in a requires expression's + parameter list. We previously handled this correctly in a constraint evaluation + context, but not in a requires clause evaluated as a boolean. Improvements to Clang's diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index f5a30b507b107..f53b67dd5df13 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -5235,6 +5235,8 @@ def note_template_exception_spec_instantiation_here : Note< "in instantiation of exception specification for %0 requested here">; def note_template_requirement_instantiation_here : Note< "in instantiation of requirement here">; +def note_template_requirement_params_instantiation_here : Note< + "in instantiation of requirement parameters here">; def warn_var_template_missing : Warning<"instantiation of variable %q0 " "required here, but no definition is available">, InGroup<UndefinedVarTemplate>; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index ce6f67ad19af6..9883176cd764d 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -9135,6 +9135,9 @@ class Sema final { // We are normalizing a constraint expression. ConstraintNormalization, + // Instantiating a Requires Expression parameter clause. + RequirementParameterInstantiation, + // We are substituting into the parameter mapping of an atomic constraint // during normalization. ParameterMappingSubstitution, @@ -9464,6 +9467,11 @@ class Sema final { concepts::NestedRequirement *Req, ConstraintsCheck, SourceRange InstantiationRange = SourceRange()); + /// \brief Note that we are checking a requires clause. + InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation, + const RequiresExpr *E, + sema::TemplateDeductionInfo &DeductionInfo, + SourceRange InstantiationRange); /// Note that we have finished instantiating this template. void Clear(); diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index cff61d7929a1b..be3c42e79802f 100644 --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -470,6 +470,8 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback { return "ConstraintSubstitution"; case CodeSynthesisContext::ConstraintNormalization: return "ConstraintNormalization"; + case CodeSynthesisContext::RequirementParameterInstantiation: + return "RequirementParameterInstantiation"; case CodeSynthesisContext::ParameterMappingSubstitution: return "ParameterMappingSubstitution"; case CodeSynthesisContext::RequirementInstantiation: diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index 21b141f96a695..4a9787d7b0043 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -320,6 +320,7 @@ bool Sema::CodeSynthesisContext::isInstantiationRecord() const { return true; case RequirementInstantiation: + case RequirementParameterInstantiation: case DefaultTemplateArgumentChecking: case DeclaringSpecialMember: case DeclaringImplicitEqualityComparison: @@ -505,6 +506,13 @@ Sema::InstantiatingTemplate::InstantiatingTemplate( PointOfInstantiation, InstantiationRange, /*Entity=*/nullptr, /*Template=*/nullptr, /*TemplateArgs=*/None) {} +Sema::InstantiatingTemplate::InstantiatingTemplate( + Sema &SemaRef, SourceLocation PointOfInstantiation, const RequiresExpr *RE, + sema::TemplateDeductionInfo &DeductionInfo, SourceRange InstantiationRange) + : InstantiatingTemplate( + SemaRef, CodeSynthesisContext::RequirementParameterInstantiation, + PointOfInstantiation, InstantiationRange, /*Entity=*/nullptr, + /*Template=*/nullptr, /*TemplateArgs=*/None, &DeductionInfo) {} Sema::InstantiatingTemplate::InstantiatingTemplate( Sema &SemaRef, SourceLocation PointOfInstantiation, @@ -540,6 +548,7 @@ Sema::InstantiatingTemplate::InstantiatingTemplate( SemaRef, CodeSynthesisContext::ParameterMappingSubstitution, PointOfInstantiation, InstantiationRange, Template) {} + void Sema::pushCodeSynthesisContext(CodeSynthesisContext Ctx) { Ctx.SavedInNonInstantiationSFINAEContext = InNonInstantiationSFINAEContext; InNonInstantiationSFINAEContext = false; @@ -845,6 +854,12 @@ void Sema::PrintInstantiationStack() { diag::note_template_requirement_instantiation_here) << Active->InstantiationRange; break; + case CodeSynthesisContext::RequirementParameterInstantiation: + assert("how do we get here?!"); + Diags.Report(Active->PointOfInstantiation, + diag::note_template_requirement_params_instantiation_here) + << Active->InstantiationRange; + break; case CodeSynthesisContext::NestedRequirementConstraintsCheck: Diags.Report(Active->PointOfInstantiation, @@ -1003,6 +1018,7 @@ Optional<TemplateDeductionInfo *> Sema::isSFINAEContext() const { case CodeSynthesisContext::DeducedTemplateArgumentSubstitution: case CodeSynthesisContext::ConstraintSubstitution: case CodeSynthesisContext::RequirementInstantiation: + case CodeSynthesisContext::RequirementParameterInstantiation: // We're either substituting explicitly-specified template arguments, // deduced template arguments, a constraint expression or a requirement // in a requires expression, so SFINAE applies. @@ -1348,6 +1364,12 @@ namespace { TransformExprRequirement(concepts::ExprRequirement *Req); concepts::NestedRequirement * TransformNestedRequirement(concepts::NestedRequirement *Req); + ExprResult TransformRequiresTypeParams( + SourceLocation KWLoc, SourceLocation RBraceLoc, const RequiresExpr *RE, + RequiresExprBodyDecl *Body, ArrayRef<ParmVarDecl *> Params, + SmallVectorImpl<QualType> &PTypes, + SmallVectorImpl<ParmVarDecl *> &TransParams, + Sema::ExtParameterInfoBuilder &PInfos); private: ExprResult transformNonTypeTemplateParmRef(NonTypeTemplateParmDecl *parm, @@ -2065,6 +2087,37 @@ createSubstDiag(Sema &S, TemplateDeductionInfo &Info, EntityPrinter Printer) { StringRef(MessageBuf, Message.size())}; } +ExprResult TemplateInstantiator::TransformRequiresTypeParams( + SourceLocation KWLoc, SourceLocation RBraceLoc, const RequiresExpr *RE, + RequiresExprBodyDecl *Body, ArrayRef<ParmVarDecl *> Params, + SmallVectorImpl<QualType> &PTypes, + SmallVectorImpl<ParmVarDecl *> &TransParams, + Sema::ExtParameterInfoBuilder &PInfos) { + + TemplateDeductionInfo Info(KWLoc); + Sema::InstantiatingTemplate TypeInst(SemaRef, KWLoc, + RE, Info, + SourceRange{KWLoc, RBraceLoc}); + Sema::SFINAETrap Trap(SemaRef); + + unsigned ErrorIdx; + if (getDerived().TransformFunctionTypeParams( + KWLoc, Params, /*ParamTypes=*/nullptr, /*ParamInfos=*/nullptr, PTypes, + &TransParams, PInfos, &ErrorIdx) || + Trap.hasErrorOccurred()) { + SmallVector<concepts::Requirement *, 4> TransReqs; + ParmVarDecl *FailedDecl = Params[ErrorIdx]; + // Add a 'failed' Requirement to contain the error that caused the failure + // here. + TransReqs.push_back(RebuildTypeRequirement(createSubstDiag( + SemaRef, Info, [&](llvm::raw_ostream &OS) { OS << *FailedDecl; }))); + return getDerived().RebuildRequiresExpr(KWLoc, Body, TransParams, TransReqs, + RBraceLoc); + } + + return ExprResult{}; +} + concepts::TypeRequirement * TemplateInstantiator::TransformTypeRequirement(concepts::TypeRequirement *Req) { if (!Req->isDependent() && !AlwaysRebuild()) diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index eb7be2c6118fa..82b6cfacf46b6 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -671,13 +671,49 @@ class TreeTransform { /// The result vectors should be kept in sync; null entries in the /// variables vector are acceptable. /// + /// LastParamTransformed, if non-null, will be set to the index of the last + /// parameter on which transfromation was started. In the event of an error, + /// this will contain the parameter which failed to instantiate. + /// /// Return true on error. bool TransformFunctionTypeParams( SourceLocation Loc, ArrayRef<ParmVarDecl *> Params, const QualType *ParamTypes, const FunctionProtoType::ExtParameterInfo *ParamInfos, SmallVectorImpl<QualType> &PTypes, SmallVectorImpl<ParmVarDecl *> *PVars, - Sema::ExtParameterInfoBuilder &PInfos); + Sema::ExtParameterInfoBuilder &PInfos, unsigned *LastParamTransformed); + + bool TransformFunctionTypeParams( + SourceLocation Loc, ArrayRef<ParmVarDecl *> Params, + const QualType *ParamTypes, + const FunctionProtoType::ExtParameterInfo *ParamInfos, + SmallVectorImpl<QualType> &PTypes, SmallVectorImpl<ParmVarDecl *> *PVars, + Sema::ExtParameterInfoBuilder &PInfos) { + return getDerived().TransformFunctionTypeParams( + Loc, Params, ParamTypes, ParamInfos, PTypes, PVars, PInfos, nullptr); + } + + /// Transforms the parameters of a requires expresison into the given vectors. + /// + /// The result vectors should be kept in sync; null entries in the + /// variables vector are acceptable. + /// + /// Returns an unset ExprResult on success. Returns an ExprResult the 'not + /// satisfied' RequiresExpr if subsitution failed, OR an ExprError, both of + /// which are cases where transformation shouldn't continue. + ExprResult TransformRequiresTypeParams( + SourceLocation KWLoc, SourceLocation RBraceLoc, const RequiresExpr *RE, + RequiresExprBodyDecl *Body, ArrayRef<ParmVarDecl *> Params, + SmallVectorImpl<QualType> &PTypes, + SmallVectorImpl<ParmVarDecl *> &TransParams, + Sema::ExtParameterInfoBuilder &PInfos) { + if (getDerived().TransformFunctionTypeParams( + KWLoc, Params, /*ParamTypes=*/nullptr, + /*ParamInfos=*/nullptr, PTypes, &TransParams, PInfos)) + return ExprError(); + + return ExprResult{}; + } /// Transforms a single function-type parameter. Return null /// on error. @@ -5661,11 +5697,14 @@ bool TreeTransform<Derived>::TransformFunctionTypeParams( const FunctionProtoType::ExtParameterInfo *ParamInfos, SmallVectorImpl<QualType> &OutParamTypes, SmallVectorImpl<ParmVarDecl *> *PVars, - Sema::ExtParameterInfoBuilder &PInfos) { + Sema::ExtParameterInfoBuilder &PInfos, + unsigned *LastParamTransformed) { int indexAdjustment = 0; unsigned NumParams = Params.size(); for (unsigned i = 0; i != NumParams; ++i) { + if (LastParamTransformed) + *LastParamTransformed = i; if (ParmVarDecl *OldParm = Params[i]) { assert(OldParm->getFunctionScopeIndex() == i); @@ -5783,6 +5822,7 @@ bool TreeTransform<Derived>::TransformFunctionTypeParams( // Deal with the possibility that we don't have a parameter // declaration for this parameter. + assert(ParamTypes); QualType OldType = ParamTypes[i]; bool IsPackExpansion = false; Optional<unsigned> NumExpansions; @@ -12581,16 +12621,20 @@ TreeTransform<Derived>::TransformRequiresExpr(RequiresExpr *E) { Sema::ContextRAII SavedContext(getSema(), Body, /*NewThisContext*/false); - if (getDerived().TransformFunctionTypeParams(E->getRequiresKWLoc(), - E->getLocalParameters(), - /*ParamTypes=*/nullptr, - /*ParamInfos=*/nullptr, - TransParamTypes, &TransParams, - ExtParamInfos)) - return ExprError(); + ExprResult TypeParamResult = getDerived().TransformRequiresTypeParams( + E->getRequiresKWLoc(), E->getRBraceLoc(), E, Body, + E->getLocalParameters(), TransParamTypes, TransParams, ExtParamInfos); for (ParmVarDecl *Param : TransParams) - Param->setDeclContext(Body); + if (Param) + Param->setDeclContext(Body); + + // On failure to transform, TransformRequiresTypeParams returns an expression + // in the event that the transformation of the type params failed in some way. + // It is expected that this will result in a 'not satisfied' Requires clause + // when instantiating. + if (!TypeParamResult.isUnset()) + return TypeParamResult; SmallVector<concepts::Requirement *, 4> TransReqs; if (getDerived().TransformRequiresExprRequirements(E->getRequirements(), diff --git a/clang/test/CXX/drs/dr25xx.cpp b/clang/test/CXX/drs/dr25xx.cpp new file mode 100644 index 0000000000000..37b8dea188b60 --- /dev/null +++ b/clang/test/CXX/drs/dr25xx.cpp @@ -0,0 +1,44 @@ +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown %s -verify + +namespace dr2565 { // dr252: 16 + template<typename T> + concept C = requires (typename T::type x) { + x + 1; + }; + static_assert(!C<int>); + + // Variant of this as reported in GH57487. + template<bool B> struct bool_constant + { static constexpr bool value = B; }; + + template<typename T> + using is_referenceable + = bool_constant<requires (T&) { true; }>; + + static_assert(!is_referenceable<void>::value); + static_assert(is_referenceable<int>::value); + + template<typename T, typename U> + concept TwoParams = requires (T *a, U b){ true;}; // #TPC + + template<typename T, typename U> + requires TwoParams<T, U> // #TPSREQ + struct TwoParamsStruct{}; + + using TPSU = TwoParamsStruct<void, void>; + // expected-error@-1{{constraints not satisfied for class template 'TwoParamsStruct'}} + // expected-note@#TPSREQ{{because 'TwoParams<void, void>' evaluated to false}} + // expected-note@#TPC{{because 'b' would be invalid: argument may not have 'void' type}} + + template<typename T, typename ...U> + concept Variadic = requires (U* ... a, T b){ true;}; // #VC + + template<typename T, typename ...U> + requires Variadic<T, U...> // #VSREQ + struct VariadicStruct{}; + + using VSU = VariadicStruct<void, int, char, double>; + // expected-error@-1{{constraints not satisfied for class template 'VariadicStruct'}} + // expected-note@#VSREQ{{because 'Variadic<void, int, char, double>' evaluated to false}} + // expected-note@#VC{{because 'b' would be invalid: argument may not have 'void' type}} +} diff --git a/clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp b/clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp index 4e37a195398e8..624c905ce8204 100644 --- a/clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp +++ b/clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp @@ -88,7 +88,7 @@ using r7i1 = r7<int>; // C++ [expr.prim.req.simple] Example namespace std_example { template<typename T> concept C = - requires (T a, T b) { // expected-note{{because substituted constraint expression is ill-formed: argument may not have 'void' type}} + requires (T a, T b) { // expected-note{{because 'a' would be invalid: argument may not have 'void' type}} a + b; // expected-note{{because 'a + b' would be invalid: invalid operands to binary expression ('int *' and 'int *')}} }; diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html index afaa852e92cac..0289372644056 100755 --- a/clang/www/cxx_dr_status.html +++ b/clang/www/cxx_dr_status.html @@ -15198,7 +15198,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2> <td><a href="https://wg21.link/cwg2565";>2565</a></td> <td>open</td> <td>Invalid types in the <I>parameter-declaration-clause</I> of a <I>requires-expression</I></td> - <td align="center">Not resolved</td> + <td class="full" align="center">Clang 16</td> </tr> <tr class="open" id="2566"> <td><a href="https://wg21.link/cwg2566";>2566</a></td> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
