When a potential immediate invocation is met, it is immediately wrapped by a
ConstantExpr. There is also a TreeTransform that removes this ConstantExpr
wrapper when corresponding expression evaluation context is popped.
So, in case initializer was an immediate invocation, CXXTemporaryObjectExpr
was wrapped by a ConstantExpr, and that caused additional unnecessary
CXXFunctionalCastExpr to be added, which later confused the TreeTransform
that rebuilds immediate invocations, so it was adding unnecessary
constructor call.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
The changes LGTM (though I had a small nit), but you should add a release note when landing.
clang/lib/Sema/SemaExprCXX.cpp | ||
---|---|---|
1593–1595 | This will need reformatting, but because we can use C++17 features, we can be slightly fancier here. :-) NB: I think we don't need to use dyn_cast_or_null here or on the predicate above. We're calling isa<Whatever>(Inner) immediately below and that will assert if Inner was null, so it seems like we're already relying on Inner being nonnull. |
clang/lib/Sema/SemaExprCXX.cpp | ||
---|---|---|
1593–1595 | That makes sense, thank you. |
clang/lib/Sema/SemaExprCXX.cpp | ||
---|---|---|
1593–1595 | Ah, that's fine, no worries about the const then. That suggestion is mostly a reflex for me at this point. :-D |
LGTM! Precommit CI is still falling over because of the libcxx bot. That issue is being actively investigated, but we don't have an ETA on it being resolved. I think these changes are sufficiently safe to land and watch for post-commit failures.
This will need reformatting, but because we can use C++17 features, we can be slightly fancier here. :-)
NB: I think we don't need to use dyn_cast_or_null here or on the predicate above. We're calling isa<Whatever>(Inner) immediately below and that will assert if Inner was null, so it seems like we're already relying on Inner being nonnull.