rjmccall added inline comments.
================ Comment at: clang/lib/Sema/TreeTransform.h:10515-10525 + // 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; + ---------------- aaron.ballman wrote: > These changes look correct to me, but I am not certain why they've been > unnecessary for so long. This code is 11 years old and has never transformed > the `OpaqueValueExpr` before (see > https://github.com/llvm/llvm-project/commit/8d69a2160e48b73a4515c9401e67971fb8c14e07). > @rjmccall, was this an oversight and we just got lucky for a long time, or > is there a reason we didn't transform these expressions? OVE always has to appear within a containing construct that has to handle it specially to preserve OVE equality. It is likely that the structured bindings transform doesn't do that properly. That should probably be fixed rather than trying to handle it here; creating a new OVE every time we encounter one is very problematic. Perhaps this case should assert that it's unreachable. For example, TreeTransform on PseudoObjectExpr recreates the syntactic form and then transforms that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108482/new/ https://reviews.llvm.org/D108482 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
