ricejasonf updated this revision to Diff 368485.
ricejasonf added a comment.
I added a simple lit test. The case is the reduced case mentioned in the bug. I
spent some time trying to reproduce this outside decomposition. The only other
way (I know of) to produce an ArrayInitLoopExpr is an implicit copy
constructor, but the AST for that is generated lazily so it never gets
transformed. Other entities containing OpaqueValueExpr do not appear to have
this problem either. Let me know if there are any more deficiencies or
suggested changes. Thank you.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108482/new/
https://reviews.llvm.org/D108482
Files:
clang/lib/Sema/SemaInit.cpp
clang/lib/Sema/TreeTransform.h
clang/test/SemaCXX/pr45964-nested-ove.cpp
Index: clang/test/SemaCXX/pr45964-nested-ove.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/pr45964-nested-ove.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++17 -emit-codegen-only -verify %s
+// Don't crash (Bugzilla - Bug 45964).
+
+// non-dependent ArrayInitLoop should not, upon instantiation,
+// contain an OpaqueValueExpr with a nested OpaqueValueExpr or an
+// uninstantiated source expr. (triggers asserts in CodeGen)
+
+// expected-no-diagnostics
+int a[1];
+template <int> void b() {
+ auto [c] = a;
+}
+void (*d)(){b<0>};
Index: clang/lib/Sema/TreeTransform.h
===================================================================
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -10511,7 +10511,19 @@
TreeTransform<Derived>::TransformOpaqueValueExpr(OpaqueValueExpr *E) {
assert((!E->getSourceExpr() ||
getDerived().AlreadyTransformed(E->getType())) &&
"opaque value expression requires transformation");
- return E;
+
+ // Note that SourceExpr can be nullptr
+ ExprResult SourceExpr = TransformExpr(E->getSourceExpr());
+ if (SourceExpr.isInvalid())
+ return ExprError();
+ if (SourceExpr.get() == E->getSourceExpr() && !getDerived().AlwaysRebuild())
{
+ return E;
+ }
+
+ OpaqueValueExpr *New = new (SemaRef.Context)
+ OpaqueValueExpr(E->getExprLoc(), E->getType(), E->getValueKind(),
+ E->getObjectKind(), SourceExpr.get());
+ return New;
}
template<typename Derived>
Index: clang/lib/Sema/SemaInit.cpp
===================================================================
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -8643,9 +8643,13 @@
case SK_ArrayLoopIndex: {
Expr *Cur = CurInit.get();
- Expr *BaseExpr = new (S.Context)
- OpaqueValueExpr(Cur->getExprLoc(), Cur->getType(),
- Cur->getValueKind(), Cur->getObjectKind(), Cur);
+ // prevent nested OpaqueValueExprs
+ Expr *BaseExpr = dyn_cast<OpaqueValueExpr>(Cur);
+ if (!BaseExpr) {
+ BaseExpr = new (S.Context)
+ OpaqueValueExpr(Cur->getExprLoc(), Cur->getType(),
+ Cur->getValueKind(), Cur->getObjectKind(), Cur);
+ }
Expr *IndexExpr =
new (S.Context) ArrayInitIndexExpr(S.Context.getSizeType());
CurInit = S.CreateBuiltinArraySubscriptExpr(
Index: clang/test/SemaCXX/pr45964-nested-ove.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/pr45964-nested-ove.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++17 -emit-codegen-only -verify %s
+// Don't crash (Bugzilla - Bug 45964).
+
+// non-dependent ArrayInitLoop should not, upon instantiation,
+// contain an OpaqueValueExpr with a nested OpaqueValueExpr or an
+// uninstantiated source expr. (triggers asserts in CodeGen)
+
+// expected-no-diagnostics
+int a[1];
+template <int> void b() {
+ auto [c] = a;
+}
+void (*d)(){b<0>};
Index: clang/lib/Sema/TreeTransform.h
===================================================================
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -10511,7 +10511,19 @@
TreeTransform<Derived>::TransformOpaqueValueExpr(OpaqueValueExpr *E) {
assert((!E->getSourceExpr() || getDerived().AlreadyTransformed(E->getType())) &&
"opaque value expression requires transformation");
- return E;
+
+ // Note that SourceExpr can be nullptr
+ ExprResult SourceExpr = TransformExpr(E->getSourceExpr());
+ if (SourceExpr.isInvalid())
+ return ExprError();
+ if (SourceExpr.get() == E->getSourceExpr() && !getDerived().AlwaysRebuild()) {
+ return E;
+ }
+
+ OpaqueValueExpr *New = new (SemaRef.Context)
+ OpaqueValueExpr(E->getExprLoc(), E->getType(), E->getValueKind(),
+ E->getObjectKind(), SourceExpr.get());
+ return New;
}
template<typename Derived>
Index: clang/lib/Sema/SemaInit.cpp
===================================================================
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -8643,9 +8643,13 @@
case SK_ArrayLoopIndex: {
Expr *Cur = CurInit.get();
- Expr *BaseExpr = new (S.Context)
- OpaqueValueExpr(Cur->getExprLoc(), Cur->getType(),
- Cur->getValueKind(), Cur->getObjectKind(), Cur);
+ // prevent nested OpaqueValueExprs
+ Expr *BaseExpr = dyn_cast<OpaqueValueExpr>(Cur);
+ if (!BaseExpr) {
+ BaseExpr = new (S.Context)
+ OpaqueValueExpr(Cur->getExprLoc(), Cur->getType(),
+ Cur->getValueKind(), Cur->getObjectKind(), Cur);
+ }
Expr *IndexExpr =
new (S.Context) ArrayInitIndexExpr(S.Context.getSizeType());
CurInit = S.CreateBuiltinArraySubscriptExpr(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
