r354035 - [Sema] Fix a regression introduced in "[AST][Sema] Remove CallExpr::setNumArgs"

[Hans Wennborg via cfe-commits]

Merged to release_80 in r354247.

On Thu, Feb 14, 2019 at 4:42 PM Bruno Ricci via cfe-commits
<cfe-commits at lists.llvm.org> 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_iterator> 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<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);
> +    }
> +    // We can now handle the nulled arguments for the default arguments.
> +    TheCall->setNumArgsUnsafe(std::max<unsigned>(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 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'?}}
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits