riccibruno created this revision.
riccibruno added a reviewer: aaron.ballman.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.
D54902 <https://reviews.llvm.org/D54902> removed `CallExpr::setNumArgs` in
preparation of tail-allocating the arguments of `CallExpr`. It did this by
allocating storage for `max(number of arguments, number of parameters in the
prototype)`. The temporarily nulled arguments however causes issues in
`BuildResolvedCallExpr` when typo correction is done just after the creation of
the call expression.
This was unfortunately missed by the tests /:
To fix this, delay setting the number of arguments to `max(number of arguments,
number of parameters in the prototype)` until we are ready for it. It would be
nice to have this encapsulated in `CallExpr` but this is the best I can come up
with under the constraint that we cannot add anything the `CallExpr`.
Fixes PR40286. This should probably be cherry-picked into the release branch.
Repository:
rC Clang
https://reviews.llvm.org/D57948
Files:
include/clang/AST/Expr.h
lib/AST/Expr.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
test/Sema/typo-correction.c
Index: test/Sema/typo-correction.c
===================================================================
--- test/Sema/typo-correction.c
+++ test/Sema/typo-correction.c
@@ -100,3 +100,19 @@
structVar1.fieldName1.member1, //expected-error{{use of undeclared identifier 'structVar1'}}
structVar2.fieldName2.member2); //expected-error{{use of undeclared identifier 'structVar2'}}
}
+
+void PR40286_g(int x, int y);
+void PR40286_h(int x, int y, int z);
+
+void PR40286_1(int the_value) {
+ PR40286_g(the_walue); // expected-error {{use of undeclared identifier 'the_walue'}}
+}
+void PR40286_2(int the_value) {
+ PR40286_h(the_value, the_walue); // expected-error {{use of undeclared identifier 'the_walue'}}
+}
+void PR40286_3(int the_value) {
+ PR40286_h(the_walue); // expected-error {{use of undeclared identifier 'the_walue'}}
+}
+void PR40286_4(int the_value) { // expected-note {{'the_value' declared here}}
+ PR40286_h(the_value, the_value, the_walue); // expected-error {{use of undeclared identifier 'the_walue'; did you mean 'the_value'?}}
+}
Index: lib/Sema/SemaOverload.cpp
===================================================================
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -12906,6 +12906,8 @@
call, nullptr))
return ExprError();
+ call->setNumArgsUnsafe(
+ std::max<unsigned>(Args.size(), proto->getNumParams()));
if (ConvertArgumentsForCall(call, op, nullptr, proto, Args, RParenLoc))
return ExprError();
@@ -13073,6 +13075,8 @@
}
// Convert the rest of the arguments
+ TheCall->setNumArgsUnsafe(
+ std::max<unsigned>(Args.size(), Proto->getNumParams()));
if (ConvertArgumentsForCall(TheCall, MemExpr, Method, Proto, Args,
RParenLoc))
return ExprError();
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5799,12 +5799,25 @@
// dealt with.
ExprResult Result = CorrectDelayedTyposInExpr(TheCall);
if (!Result.isUsable()) return ExprError();
+ CallExpr *TheOldCall = TheCall;
TheCall = dyn_cast<CallExpr>(Result.get());
+ bool CorrectedTypos = TheCall != TheOldCall;
if (!TheCall) return Result;
- // TheCall at this point has max(Args.size(), NumParams) arguments,
- // with extra arguments nulled. We don't want to introduce nulled
- // arguments in Args and so we only take the first Args.size() arguments.
- Args = llvm::makeArrayRef(TheCall->getArgs(), Args.size());
+ Args = llvm::makeArrayRef(TheCall->getArgs(), TheCall->getNumArgs());
+
+ // A new call expression node was created if some typos were corrected.
+ // However it may not have been constructed with enough storage. In this
+ // case rebuild the node with enough storage. The waste of space is
+ // immaterial since this only happens when some typos were corrected.
+ if (CorrectedTypos && Args.size() < NumParams) {
+ if (Config)
+ TheCall = CUDAKernelCallExpr::Create(
+ Context, Fn, cast<CallExpr>(Config), Args, ResultTy, VK_RValue,
+ RParenLoc, NumParams);
+ else
+ TheCall = CallExpr::Create(Context, Fn, Args, ResultTy, VK_RValue,
+ RParenLoc, NumParams, UsesADL);
+ }
}
// Bail out early if calling a builtin with custom type checking.
@@ -5840,6 +5853,10 @@
TheCall->setValueKind(Expr::getValueKindForType(FuncT->getReturnType()));
if (Proto) {
+ // We allocated space for max(Args.size(), NumParams) arguments in TheCall.
+ // However to avoid exposing null arguments during construction the number
+ // of arguments in TheCall was set to Args.size().
+ TheCall->setNumArgsUnsafe(std::max<unsigned>(Args.size(), NumParams));
if (ConvertArgumentsForCall(TheCall, Fn, FDecl, Proto, Args, RParenLoc,
IsExecConfig))
return ExprError();
Index: lib/AST/Expr.cpp
===================================================================
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -1241,11 +1241,16 @@
Fn->isValueDependent(), Fn->isInstantiationDependent(),
Fn->containsUnexpandedParameterPack()),
RParenLoc(RParenLoc) {
- NumArgs = std::max<unsigned>(Args.size(), MinNumArgs);
+ NumArgs = Args.size();
unsigned NumPreArgs = PreArgs.size();
CallExprBits.NumPreArgs = NumPreArgs;
assert((NumPreArgs == getNumPreArgs()) && "NumPreArgs overflow!");
+ // We allocated max(Args.size(), MinNumArgs) slots for the arguments
+ // but we set NumArgs to Args.size() to avoid exposing null arguments
+ // during construction.
+ unsigned NumArgsAllocated = std::max<unsigned>(Args.size(), MinNumArgs);
+
unsigned OffsetToTrailingObjects = offsetToTrailingObjects(SC);
CallExprBits.OffsetToTrailingObjects = OffsetToTrailingObjects;
assert((CallExprBits.OffsetToTrailingObjects == OffsetToTrailingObjects) &&
@@ -1258,12 +1263,14 @@
updateDependenciesFromArg(PreArgs[I]);
setPreArg(I, PreArgs[I]);
}
- for (unsigned I = 0; I != Args.size(); ++I) {
+
+ Expr **TrailingArgs = getArgs();
+ for (unsigned I = 0; I != NumArgs; ++I) {
updateDependenciesFromArg(Args[I]);
- setArg(I, Args[I]);
+ TrailingArgs[I] = Args[I];
}
- for (unsigned I = Args.size(); I != NumArgs; ++I) {
- setArg(I, nullptr);
+ for (unsigned I = NumArgs; I != NumArgsAllocated; ++I) {
+ TrailingArgs[I] = nullptr;
}
}
Index: include/clang/AST/Expr.h
===================================================================
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -2515,10 +2515,11 @@
/// the return type in general), VK is the value kind of the call expression
/// (lvalue, rvalue, ...), and RParenLoc is the location of the right
/// parenthese in the call expression. MinNumArgs specifies the minimum
- /// number of arguments. The actual number of arguments will be the greater
- /// of Args.size() and MinNumArgs. This is used in a few places to allocate
- /// enough storage for the default arguments. UsesADL specifies whether the
- /// callee was found through argument-dependent lookup.
+ /// number of arguments slots to allocate. The number of arguments will be
+ /// Args.size(), but with storage for max(Args.size(), MinNumArgs) arguments.
+ /// This is used in a few places to allocate enough storage for the default
+ /// arguments. UsesADL specifies whether the callee was found through
+ /// argument-dependent lookup.
///
/// Note that you can use CreateTemporary if you need a temporary call
/// expression on the stack.
@@ -2599,8 +2600,8 @@
}
/// Reduce the number of arguments in this call expression. This is used for
- /// example during error recovery to drop extra arguments. There is no way
- /// to perform the opposite because: 1.) We don't track how much storage
+ /// example during error recovery to drop extra arguments. There is no way to
+ /// perform the opposite safely because: 1.) We don't track how much storage
/// we have for the argument array 2.) This would potentially require growing
/// the argument array, something we cannot support since the arguments are
/// stored in a trailing array.
@@ -2610,6 +2611,11 @@
NumArgs = NewNumArgs;
}
+ /// Bluntly set a new number of arguments without doing any checks whatsoever.
+ /// Only used during construction of a CallExpr in a few places in Sema.
+ /// FIXME: Find a way to remove it.
+ void setNumArgsUnsafe(unsigned NewNumArgs) { NumArgs = NewNumArgs; }
+
typedef ExprIterator arg_iterator;
typedef ConstExprIterator const_arg_iterator;
typedef llvm::iterator_range<arg_iterator> arg_range;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
