Skip to content

Commit 92d8313

Browse files
authored
[Clang] Fix concept paramater mapping and caching (#161994)
This expression is not handled by default in RAV, so our parameter mapping and cache mechanism don't work when it appears in a template argument list. There are a few other expressions, such as PackIndexingExpr and FunctionParmPackExpr, which are also no-ops by default. I don't have a test case for them now, so let's leave those until users complain :/ There was also a bug in updating the parameter mapping, where the AssociatedDecl was not updated accordingly. Also also, this fixes another regression reported in #161671 (comment), where we failed to account for the variable initializer in cache profiling. Relies on #161671 Fixes #161983 Fixes #161987
1 parent 3e78c31 commit 92d8313

File tree

3 files changed

+91
-13
lines changed

3 files changed

+91
-13
lines changed

clang/lib/Sema/SemaConcept.cpp

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -264,14 +264,6 @@ class HashParameterMapping : public RecursiveASTVisitor<HashParameterMapping> {
264264

265265
UnsignedOrNone OuterPackSubstIndex;
266266

267-
TemplateArgument getPackSubstitutedTemplateArgument(TemplateArgument Arg) {
268-
assert(*SemaRef.ArgPackSubstIndex < Arg.pack_size());
269-
Arg = Arg.pack_begin()[*SemaRef.ArgPackSubstIndex];
270-
if (Arg.isPackExpansion())
271-
Arg = Arg.getPackExpansionPattern();
272-
return Arg;
273-
}
274-
275267
bool shouldVisitTemplateInstantiations() const { return true; }
276268

277269
public:
@@ -294,7 +286,7 @@ class HashParameterMapping : public RecursiveASTVisitor<HashParameterMapping> {
294286
assert(Arg.getKind() == TemplateArgument::Pack &&
295287
"Missing argument pack");
296288

297-
Arg = getPackSubstitutedTemplateArgument(Arg);
289+
Arg = SemaRef.getPackSubstitutedTemplateArgument(Arg);
298290
}
299291

300292
UsedTemplateArgs.push_back(
@@ -312,7 +304,7 @@ class HashParameterMapping : public RecursiveASTVisitor<HashParameterMapping> {
312304
if (NTTP->isParameterPack() && SemaRef.ArgPackSubstIndex) {
313305
assert(Arg.getKind() == TemplateArgument::Pack &&
314306
"Missing argument pack");
315-
Arg = getPackSubstitutedTemplateArgument(Arg);
307+
Arg = SemaRef.getPackSubstitutedTemplateArgument(Arg);
316308
}
317309

318310
UsedTemplateArgs.push_back(
@@ -325,8 +317,11 @@ class HashParameterMapping : public RecursiveASTVisitor<HashParameterMapping> {
325317
}
326318

327319
bool TraverseDecl(Decl *D) {
328-
if (auto *VD = dyn_cast<ValueDecl>(D))
320+
if (auto *VD = dyn_cast<ValueDecl>(D)) {
321+
if (auto *Var = dyn_cast<VarDecl>(VD))
322+
TraverseStmt(Var->getInit());
329323
return TraverseType(VD->getType());
324+
}
330325

331326
return inherited::TraverseDecl(D);
332327
}
@@ -363,6 +358,14 @@ class HashParameterMapping : public RecursiveASTVisitor<HashParameterMapping> {
363358
return inherited::TraverseTemplateArgument(Arg);
364359
}
365360

361+
bool TraverseSizeOfPackExpr(SizeOfPackExpr *SOPE) {
362+
return TraverseDecl(SOPE->getPack());
363+
}
364+
365+
bool VisitSubstNonTypeTemplateParmExpr(SubstNonTypeTemplateParmExpr *E) {
366+
return inherited::TraverseStmt(E->getReplacement());
367+
}
368+
366369
void VisitConstraint(const NormalizedConstraintWithParamMapping &Constraint) {
367370
if (!Constraint.hasParameterMapping()) {
368371
for (const auto &List : TemplateArgs)
@@ -2083,8 +2086,8 @@ bool SubstituteParameterMappings::substitute(ConceptIdConstraint &CC) {
20832086
/*UpdateArgsWithConversions=*/false))
20842087
return true;
20852088
auto TemplateArgs = *MLTAL;
2086-
TemplateArgs.replaceOutermostTemplateArguments(
2087-
TemplateArgs.getAssociatedDecl(0).first, CTAI.SugaredConverted);
2089+
TemplateArgs.replaceOutermostTemplateArguments(CSE->getNamedConcept(),
2090+
CTAI.SugaredConverted);
20882091
return SubstituteParameterMappings(SemaRef, &TemplateArgs, ArgsAsWritten,
20892092
InFoldExpr)
20902093
.substitute(CC.getNormalizedConstraint());

clang/lib/Sema/SemaTemplateDeduction.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6718,6 +6718,10 @@ struct MarkUsedTemplateParameterVisitor : DynamicRecursiveASTVisitor {
67186718
}
67196719
return true;
67206720
}
6721+
6722+
bool TraverseSizeOfPackExpr(SizeOfPackExpr *SOPE) override {
6723+
return TraverseDecl(SOPE->getPack());
6724+
}
67216725
};
67226726
}
67236727

clang/test/SemaTemplate/concepts.cpp

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,4 +1333,75 @@ static_assert(__cpp17_iterator<not_move_constructible>); \
13331333
// expected-note@#is_move_constructible_v {{because 'is_move_constructible_v<parameter_mapping_regressions::case3::not_move_constructible>' evaluated to false}}
13341334
}
13351335

1336+
namespace case4 {
1337+
1338+
template<bool b>
1339+
concept bool_ = b;
1340+
1341+
template<typename... Ts>
1342+
concept unary = bool_<sizeof...(Ts) == 1>;
1343+
1344+
static_assert(!unary<>);
1345+
static_assert(unary<void>);
1346+
1347+
}
1348+
1349+
namespace case5 {
1350+
1351+
template<int size>
1352+
concept true1 = size == size;
1353+
1354+
template<typename... Ts>
1355+
concept true2 = true1<sizeof...(Ts)>;
1356+
1357+
template<typename... Ts>
1358+
concept true3 = true2<Ts...>;
1359+
1360+
static_assert(true3<void>);
1361+
1362+
}
1363+
1364+
namespace case6 {
1365+
1366+
namespace std {
1367+
template <int __v>
1368+
struct integral_constant {
1369+
static const int value = __v;
1370+
};
1371+
1372+
template <class _Tp, class... _Args>
1373+
constexpr bool is_constructible_v = __is_constructible(_Tp, _Args...);
1374+
1375+
template <class _From, class _To>
1376+
constexpr bool is_convertible_v = __is_convertible(_From, _To);
1377+
1378+
template <class>
1379+
struct tuple_size;
1380+
1381+
template <class _Tp>
1382+
constexpr decltype(sizeof(int)) tuple_size_v = tuple_size<_Tp>::value;
1383+
} // namespace std
1384+
1385+
template <int N, int X>
1386+
concept FixedExtentConstructibleFromExtent = X == N;
1387+
1388+
template <int Extent>
1389+
struct span {
1390+
int static constexpr extent = Extent;
1391+
template <typename R, int N = std::tuple_size_v<R>>
1392+
requires(FixedExtentConstructibleFromExtent<extent, N>)
1393+
span(R);
1394+
};
1395+
1396+
template <class, int>
1397+
struct array {};
1398+
1399+
template <class _Tp, decltype(sizeof(int)) _Size>
1400+
struct std::tuple_size<array<_Tp, _Size>> : integral_constant<_Size> {};
1401+
1402+
static_assert(std::is_convertible_v<array<int, 3>, span<3>>);
1403+
static_assert(!std::is_constructible_v<span<4>, array<int, 3>>);
1404+
1405+
}
1406+
13361407
}

0 commit comments

Comments
 (0)