[PATCH] D119063: [SemaCXX] Properly scope ArgumentPackSubstitutionIndex when expanding base types
colavitam added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119063/new/ https://reviews.llvm.org/D119063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D119063: [SemaCXX] Properly scope ArgumentPackSubstitutionIndex when expanding base types
colavitam created this revision.
colavitam added reviewers: mizvekov, rsmith.
Herald added a subscriber: arphaman.
colavitam requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Resolve the crash in issue #53609. The ArgumentPackSubstitutionIndex
from expanding base types containing parameter packs should not persist
when we check the resulting types. Checking the types may lead to
template substitution in the base type, where the index will no longer
be valid.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D119063
Files:
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/SemaCXX/template-base-class-pack-expansion.cpp
Index: clang/test/SemaCXX/template-base-class-pack-expansion.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/template-base-class-pack-expansion.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+// Don't crash (#53609).
+
+template struct a;
+
+template struct b;
+template
+struct b...> {};
+
+template struct c: b... {};
+
+c d;
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2530,12 +2530,13 @@
// If we should expand this pack expansion now, do so.
if (ShouldExpand) {
for (unsigned I = 0; I != *NumExpansions; ++I) {
+ {
Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(*this, I);
- TypeSourceInfo *BaseTypeLoc = SubstType(Base.getTypeSourceInfo(),
- TemplateArgs,
- Base.getSourceRange().getBegin(),
- DeclarationName());
+BaseTypeLoc =
+SubstType(Base.getTypeSourceInfo(), TemplateArgs,
+ Base.getSourceRange().getBegin(), DeclarationName());
+ }
if (!BaseTypeLoc) {
Invalid = true;
continue;
Index: clang/test/SemaCXX/template-base-class-pack-expansion.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/template-base-class-pack-expansion.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+// Don't crash (#53609).
+
+template struct a;
+
+template struct b;
+template
+struct b...> {};
+
+template struct c: b... {};
+
+c d;
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2530,12 +2530,13 @@
// If we should expand this pack expansion now, do so.
if (ShouldExpand) {
for (unsigned I = 0; I != *NumExpansions; ++I) {
+ {
Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(*this, I);
- TypeSourceInfo *BaseTypeLoc = SubstType(Base.getTypeSourceInfo(),
- TemplateArgs,
- Base.getSourceRange().getBegin(),
- DeclarationName());
+BaseTypeLoc =
+SubstType(Base.getTypeSourceInfo(), TemplateArgs,
+ Base.getSourceRange().getBegin(), DeclarationName());
+ }
if (!BaseTypeLoc) {
Invalid = true;
continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D119063: [SemaCXX] Properly scope ArgumentPackSubstitutionIndex when expanding base types
colavitam updated this revision to Diff 406213.
colavitam added a comment.
Fix comments for new test case.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119063/new/
https://reviews.llvm.org/D119063
Files:
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/SemaCXX/template-base-class-pack-expansion.cpp
Index: clang/test/SemaCXX/template-base-class-pack-expansion.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/template-base-class-pack-expansion.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+// Don't crash (#53609).
+
+// expected-no-diagnostics
+
+template struct a;
+
+template struct b;
+template
+struct b...> {};
+
+template struct c: b... {};
+
+c d;
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2530,12 +2530,13 @@
// If we should expand this pack expansion now, do so.
if (ShouldExpand) {
for (unsigned I = 0; I != *NumExpansions; ++I) {
+ {
Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(*this, I);
- TypeSourceInfo *BaseTypeLoc = SubstType(Base.getTypeSourceInfo(),
- TemplateArgs,
- Base.getSourceRange().getBegin(),
- DeclarationName());
+BaseTypeLoc =
+SubstType(Base.getTypeSourceInfo(), TemplateArgs,
+ Base.getSourceRange().getBegin(), DeclarationName());
+ }
if (!BaseTypeLoc) {
Invalid = true;
continue;
Index: clang/test/SemaCXX/template-base-class-pack-expansion.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/template-base-class-pack-expansion.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+// Don't crash (#53609).
+
+// expected-no-diagnostics
+
+template struct a;
+
+template struct b;
+template
+struct b...> {};
+
+template struct c: b... {};
+
+c d;
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2530,12 +2530,13 @@
// If we should expand this pack expansion now, do so.
if (ShouldExpand) {
for (unsigned I = 0; I != *NumExpansions; ++I) {
+ {
Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(*this, I);
- TypeSourceInfo *BaseTypeLoc = SubstType(Base.getTypeSourceInfo(),
- TemplateArgs,
- Base.getSourceRange().getBegin(),
- DeclarationName());
+BaseTypeLoc =
+SubstType(Base.getTypeSourceInfo(), TemplateArgs,
+ Base.getSourceRange().getBegin(), DeclarationName());
+ }
if (!BaseTypeLoc) {
Invalid = true;
continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D119063: [SemaCXX] Properly scope ArgumentPackSubstitutionIndex when expanding base types
colavitam added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119063/new/ https://reviews.llvm.org/D119063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D119063: [SemaCXX] Properly scope ArgumentPackSubstitutionIndex when expanding base types
colavitam added a comment. Ping on this Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119063/new/ https://reviews.llvm.org/D119063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D119063: [SemaCXX] Properly scope ArgumentPackSubstitutionIndex when expanding base types
colavitam added a comment. Yes, in my opinion the original shadow was accidental—you can see that every usage of BaseTypeLoc is immediately preceded by an initialization, though sometimes in a more nested scope. When initialized in the parameter pack loop, we skip to the next iteration of the outer loop instead of accessing it in the non-parameter pack path. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119063/new/ https://reviews.llvm.org/D119063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D119063: [SemaCXX] Properly scope ArgumentPackSubstitutionIndex when expanding base types
colavitam added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119063/new/ https://reviews.llvm.org/D119063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D119063: [SemaCXX] Properly scope ArgumentPackSubstitutionIndex when expanding base types
colavitam added a comment. Herald added a project: All. Ping. Is there anyone familiar with the template instantiation code I should add as a reviewer? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119063/new/ https://reviews.llvm.org/D119063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
