Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Sema] Avoid an undesired pack expansion while transforming PackIndexingType #90195

Merged
merged 6 commits into from
May 1, 2024
Merged

[Sema] Avoid an undesired pack expansion while transforming PackIndexingType #90195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f708694
[Sema] Avoid an undesired pack expansion while transforming PackIndex…
zyn0217 Apr 26, 2024
7b0ae16
Clarify comments
zyn0217 Apr 27, 2024
1dbfe52
Make dependent PackIndexingExpr always an LValue
zyn0217 Apr 28, 2024
7a087f0
Apply changes from Corentin
zyn0217 May 1, 2024
5e85c87
Expand -> expands
zyn0217 May 1, 2024
b23d40d
Run clang-format
zyn0217 May 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion clang/lib/AST/ExprClassification.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,13 @@ static Cl::Kinds ClassifyInternal(ASTContext &Ctx, const Expr *E) {
return ClassifyInternal(Ctx,
cast<SubstNonTypeTemplateParmExpr>(E)->getReplacement());

case Expr::PackIndexingExprClass:
case Expr::PackIndexingExprClass: {
// A pack-index-expression always expands to an id-expression.
// Consider it as an LValue expression.
if (cast<PackIndexingExpr>(E)->isInstantiationDependent())
return Cl::CL_LValue;
return ClassifyInternal(Ctx, cast<PackIndexingExpr>(E)->getSelectedExpr());
}

// C, C++98 [expr.sub]p1: The result is an lvalue of type "T".
// C++11 (DR1213): in the case of an array operand, the result is an lvalue
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/Sema/TreeTransform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6649,6 +6649,10 @@ TreeTransform<Derived>::TransformPackIndexingType(TypeLocBuilder &TLB,
}
}

// A pack indexing type can appear in a larger pack expansion,
// e.g. `Pack...[pack_of_indexes]...`
// so we need to temporarily disable substitution of pack elements
Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), -1);
QualType Result = getDerived().TransformType(TLB, TL.getPatternLoc());

QualType Out = getDerived().RebuildPackIndexingType(
Expand Down
34 changes: 34 additions & 0 deletions clang/test/SemaCXX/cxx2c-pack-indexing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,37 @@ namespace GH88929 {
using E = P...[0]; // expected-error {{unknown type name 'P'}} \
// expected-error {{expected ';' after alias declaration}}
}

namespace GH88925 {
template <typename...> struct S {};

template <auto...> struct W {};

template <int...> struct sequence {};

template <typename... args, int... indices> auto f(sequence<indices...>) {
return S<args...[indices]...>(); // #use
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect there is the same problem for expressions

template <auto...> struct S2 {};
template <auto... Args, int... indices> auto f(sequence<indices...>) {
  return S2<Args...[indices]...>(); // #use
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, yeah, good catch! Honestly, I have tried one similar example (with a fold expression) but had no luck:

template <auto... Args, int... indices> auto f(sequence<indices...>) {
  return (Args...[indices] + ...);
}

Let me see what's happening here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

template <auto...> struct S2 {};
template <auto... Args, int... indices> auto f(sequence<indices...>) {
 return S2<Args...[indices]...>(); // #use
}

OK, so the problem here is not the same: we end up classifying the PackIndexingExpr's category while deducing against the NTTP of S2. This happens before the instantiation of f, and therefore Args...[indices]... is still dependent.

I guess we can perceive dependent PackIndexingExprs as LValues in such cases, like what we did for UnresolvedLookupExpr. Additionally, P2662 now limits them to id-expressions only, and hence we can probably always treat them as LValues?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can perceive dependent PackIndexingExprs as LValues in such cases, like what we did for UnresolvedLookupExpr. Additionally, P2662 now limits them to id-expressions only, and hence we can probably always treat them as LValues?

Yes, they are always LValues for now. So we might want to do that indeed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized those expressions could be PRValues after an instantiation because in PackIndexingExpr we have:

if (!isInstantiationDependent())
  setValueKind(getSelectedExpr()->getValueKind());

So, I'm going to make it always an LValue iff it's dependent.

template <auto... args, int... indices> auto g(sequence<indices...>) {
return W<args...[indices]...>(); // #nttp-use
}

void h() {
static_assert(__is_same(decltype(f<int>(sequence<0, 0>())), S<int, int>));
static_assert(__is_same(decltype(f<int, long>(sequence<0, 0>())), S<int, int>));
static_assert(__is_same(decltype(f<int, long>(sequence<0, 1>())), S<int, long>));
f<int, long>(sequence<3>());
// expected-error@#use {{invalid index 3 for pack 'args' of size 2}}}
// expected-note-re@-2 {{function template specialization '{{.*}}' requested here}}

struct foo {};
struct bar {};
struct baz {};

static_assert(__is_same(decltype(g<foo{}, bar{}, baz{}>(sequence<0, 2, 1>())), W<foo{}, baz{}, bar{}>));
g<foo{}>(sequence<4>());
// expected-error@#nttp-use {{invalid index 4 for pack args of size 1}}
// expected-note-re@-2 {{function template specialization '{{.*}}' requested here}}
}
}
Loading