[Clang][NFC] Remove CallExpr::CreateTemporary #130919
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: cor3ntin (cor3ntin) Changes
We only need a type/value category for that, This simplify the code and avoid partially-formed Fixes #130824 Full diff: https://github.com/llvm/llvm-project/pull/130919.diff 3 Files Affected:
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cfe49acf20b77..b81f3a403baf6 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3005,18 +3005,6 @@ class CallExpr : public Expr {
FPOptionsOverride FPFeatures, unsigned MinNumArgs = 0,
ADLCallKind UsesADL = NotADL);
- /// Create a temporary call expression with no arguments in the memory
- /// pointed to by Mem. Mem must points to at least sizeof(CallExpr)
- /// + sizeof(Stmt *) bytes of storage, aligned to alignof(CallExpr):
- ///
- /// \code{.cpp}
- /// alignas(CallExpr) char Buffer[sizeof(CallExpr) + sizeof(Stmt *)];
- /// CallExpr *TheCall = CallExpr::CreateTemporary(Buffer, etc);
- /// \endcode
- static CallExpr *CreateTemporary(void *Mem, Expr *Fn, QualType Ty,
- ExprValueKind VK, SourceLocation RParenLoc,
- ADLCallKind UsesADL = NotADL);
-
/// Create an empty call expression, for deserialization.
static CallExpr *CreateEmpty(const ASTContext &Ctx, unsigned NumArgs,
bool HasFPFeatures, EmptyShell Empty);
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 1dde64f193dbd..901ebf9592680 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1509,16 +1509,6 @@ CallExpr *CallExpr::Create(const ASTContext &Ctx, Expr *Fn,
RParenLoc, FPFeatures, MinNumArgs, UsesADL);
}
-CallExpr *CallExpr::CreateTemporary(void *Mem, Expr *Fn, QualType Ty,
- ExprValueKind VK, SourceLocation RParenLoc,
- ADLCallKind UsesADL) {
- assert(!(reinterpret_cast<uintptr_t>(Mem) % alignof(CallExpr)) &&
- "Misaligned memory in CallExpr::CreateTemporary!");
- return new (Mem) CallExpr(CallExprClass, Fn, /*PreArgs=*/{}, /*Args=*/{}, Ty,
- VK, RParenLoc, FPOptionsOverride(),
- /*MinNumArgs=*/0, UsesADL);
-}
-
CallExpr *CallExpr::CreateEmpty(const ASTContext &Ctx, unsigned NumArgs,
bool HasFPFeatures, EmptyShell Empty) {
unsigned SizeOfTrailingObjects =
@@ -1655,14 +1645,8 @@ SourceLocation CallExpr::getBeginLoc() const {
if (!isTypeDependent()) {
if (const auto *Method =
dyn_cast_if_present<const CXXMethodDecl>(getCalleeDecl());
- Method && Method->isExplicitObjectMemberFunction()) {
- // Note: while we typically expect the call to have a first argument
- // here, we can't assert it because in some cases it does not, e.g.
- // calls created with CallExpr::CreateTemporary() during overload
- // resolution.
- if (getNumArgs() > 0 && getArg(0))
- return getArg(0)->getBeginLoc();
- }
+ Method && Method->isExplicitObjectMemberFunction())
+ return getArg(0)->getBeginLoc();
}
SourceLocation begin = getCallee()->getBeginLoc();
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index d3c0534b4dd0b..b7f26ec1e86bf 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -8147,17 +8147,15 @@ void Sema::AddConversionCandidate(
ExprValueKind VK = Expr::getValueKindForType(ConversionType);
- // Note that it is safe to allocate CallExpr on the stack here because
- // there are 0 arguments (i.e., nothing is allocated using ASTContext's
- // allocator).
QualType CallResultType = ConversionType.getNonLValueExprType(Context);
- alignas(CallExpr) char Buffer[sizeof(CallExpr) + sizeof(Stmt *)];
- CallExpr *TheTemporaryCall = CallExpr::CreateTemporary(
- Buffer, &ConversionFn, CallResultType, VK, From->getBeginLoc());
+ // Introduce a temporary expression with the right type and value category that
+ // we can use for deduction purposes.
+ OpaqueValueExpr FakeCall(From->getBeginLoc(),
+ CallResultType, VK);
ImplicitConversionSequence ICS =
- TryCopyInitialization(*this, TheTemporaryCall, ToType,
+ TryCopyInitialization(*this, &FakeCall, ToType,
/*SuppressUserConversions=*/true,
/*InOverloadResolution=*/false,
/*AllowObjCWritebackConversion=*/false);
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
cor3ntin
force-pushed
the
corentin/remove_create_temporary
branch
from
0219129 to
b74b407
Compare
|
Oops, I just filed #130921 (Code changes are almost the same, so I'll close my PR) |
|
@zyn0217 yeah, we basically did the same thing at the same time. Sorry! I don't care which gets merged |
zyn0217
approved these changes
Mar 12, 2025
Comment on lines
+1648
to
+1649
| Method && Method->isExplicitObjectMemberFunction()) | ||
| return getArg(0)->getBeginLoc(); |
There was a problem hiding this comment.
I think we can revert that to
bool HasFirstArg = getNumArgs() > 0 && getArg(0);
assert(HasFirstArg);
if (HasFirstArg)
return getArg(0)->getBeginLoc();so at least we have an assertion here. WDYT?
There was a problem hiding this comment.
getArg(0) already has an assertion (and I don;t like having both an assertion and an if)
`CallExpr::CreateTemporary` was only used to deduce a conversion sequence from a conversion operator. We only need a type/value category for that, so we can use a dummy Expression such as a `OpaqueValueExpr`. This simplify the code and avoid partially-formed `CallExpr` with incorrect invariants (see llvm#130725) Fixes llvm#130824
cor3ntin
force-pushed
the
corentin/remove_create_temporary
branch
from
b74b407 to
0e055b6
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/14437 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CallExpr::CreateTemporarywas only used to deduce a conversion sequence from a conversion operator.We only need a type/value category for that,
so we can use a dummy Expression such as a
OpaqueValueExpr.This simplify the code and avoid partially-formed
CallExprwith incorrect invariants (see #130725)Fixes #130824