Re: r354035 - [Sema] Fix a regression introduced in "[AST][Sema] Remove CallExpr::setNumArgs"
Merged to release_80 in r354247. On Thu, Feb 14, 2019 at 4:42 PM Bruno Ricci via cfe-commits wrote: > > Author: brunoricci > Date: Thu Feb 14 07:43:17 2019 > New Revision: 354035 > > URL: http://llvm.org/viewvc/llvm-project?rev=354035&view=rev > Log: > [Sema] Fix a regression introduced in "[AST][Sema] Remove > CallExpr::setNumArgs" > > 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. > > Differential Revision: https://reviews.llvm.org/D57948 > > Reviewed By: aaron.ballman > > > Modified: > cfe/trunk/include/clang/AST/Expr.h > cfe/trunk/lib/Sema/SemaExpr.cpp > cfe/trunk/test/Sema/typo-correction.c > > Modified: cfe/trunk/include/clang/AST/Expr.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=354035&r1=354034&r2=354035&view=diff > == > --- cfe/trunk/include/clang/AST/Expr.h (original) > +++ cfe/trunk/include/clang/AST/Expr.h Thu Feb 14 07:43:17 2019 > @@ -2610,6 +2610,11 @@ public: > 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_range; > > Modified: cfe/trunk/lib/Sema/SemaExpr.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=354035&r1=354034&r2=354035&view=diff > == > --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) > +++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Feb 14 07:43:17 2019 > @@ -5793,18 +5793,36 @@ ExprResult Sema::BuildResolvedCallExpr(E >} > >if (!getLangOpts().CPlusPlus) { > +// Forget about the nulled arguments since typo correction > +// do not handle them well. > +TheCall->shrinkNumArgs(Args.size()); > // C cannot always handle TypoExpr nodes in builtin calls and direct > // function calls as their argument checking don't necessarily handle > // dependent types properly, so make sure any TypoExprs have been > // dealt with. > ExprResult Result = CorrectDelayedTyposInExpr(TheCall); > if (!Result.isUsable()) return ExprError(); > +CallExpr *TheOldCall = TheCall; > TheCall = dyn_cast(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(Config), Args, ResultTy, VK_RValue, > +RParenLoc, NumParams); > + else > +TheCall = CallExpr::Create(Context, Fn, Args, ResultTy, VK_RValue, > + RParenLoc, NumParams, UsesADL); > +} > +// We can now handle the nulled arguments for the default arguments. > +TheCall->setNumArgsUnsafe(std::max(Args.size(), NumParams)); >} > >// Bail out early if calling a builtin with custom type checking. > > Modified: cfe/trunk/test/Sema/typo-correction.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/typo-correction.c?rev=354035&r1=354034&r2=354035&view=diff > == > --- cfe/trunk/test/Sema/typo-correction.c (original) > +++ cfe/trunk/test/Sema/typo-correction.
r354035 - [Sema] Fix a regression introduced in "[AST][Sema] Remove CallExpr::setNumArgs"
Author: brunoricci Date: Thu Feb 14 07:43:17 2019 New Revision: 354035 URL: http://llvm.org/viewvc/llvm-project?rev=354035&view=rev Log: [Sema] Fix a regression introduced in "[AST][Sema] Remove CallExpr::setNumArgs" 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. Differential Revision: https://reviews.llvm.org/D57948 Reviewed By: aaron.ballman Modified: cfe/trunk/include/clang/AST/Expr.h cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/test/Sema/typo-correction.c Modified: cfe/trunk/include/clang/AST/Expr.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=354035&r1=354034&r2=354035&view=diff == --- cfe/trunk/include/clang/AST/Expr.h (original) +++ cfe/trunk/include/clang/AST/Expr.h Thu Feb 14 07:43:17 2019 @@ -2610,6 +2610,11 @@ public: 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_range; Modified: cfe/trunk/lib/Sema/SemaExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=354035&r1=354034&r2=354035&view=diff == --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) +++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Feb 14 07:43:17 2019 @@ -5793,18 +5793,36 @@ ExprResult Sema::BuildResolvedCallExpr(E } if (!getLangOpts().CPlusPlus) { +// Forget about the nulled arguments since typo correction +// do not handle them well. +TheCall->shrinkNumArgs(Args.size()); // C cannot always handle TypoExpr nodes in builtin calls and direct // function calls as their argument checking don't necessarily handle // dependent types properly, so make sure any TypoExprs have been // dealt with. ExprResult Result = CorrectDelayedTyposInExpr(TheCall); if (!Result.isUsable()) return ExprError(); +CallExpr *TheOldCall = TheCall; TheCall = dyn_cast(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(Config), Args, ResultTy, VK_RValue, +RParenLoc, NumParams); + else +TheCall = CallExpr::Create(Context, Fn, Args, ResultTy, VK_RValue, + RParenLoc, NumParams, UsesADL); +} +// We can now handle the nulled arguments for the default arguments. +TheCall->setNumArgsUnsafe(std::max(Args.size(), NumParams)); } // Bail out early if calling a builtin with custom type checking. Modified: cfe/trunk/test/Sema/typo-correction.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/typo-correction.c?rev=354035&r1=354034&r2=354035&view=diff == --- cfe/trunk/test/Sema/typo-correction.c (original) +++ cfe/trunk/test/Sema/typo-correction.c Thu Feb 14 07:43:17 2019 @@ -100,3 +100,18 @@ void rdar38642201_caller() { structVar1.fieldName1.member1, //expected-error{{use of undeclared identifier 'structVar1'}} structVar2.fieldName2.member2); //expected-error{{use of undeclared identifier 'structVar2'}} } + +void PR40286_g(int