[clang] 3d7946c - Implement DR2565: Invalid types in the parameter-declaration-clause of a
Erich Keane via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 4 10:32:58 PDT 2022
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 at -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 at -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>
More information about the cfe-commits
mailing list