[PATCH] D145892: [SemaCXX]use CorrectDelayedTyposInExpr in ActOnCXXFoldExpr only when Diag

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D145892#4189603 <https://reviews.llvm.org/D145892#4189603>, @HerrCai0907 wrote:

> The idea is that:
>
> 1. If function return an invalid ExprResult, this error should be handled in the callee
> 2. If function return valid Expr, the potential error should be handled in the caller, in this case, is ActOnExprStmt. If callee handles error, when caller find some error and want to output error message, the error in this expression has been processed and cause crash.
>
>
>
>> It seems this patch would make us miss other issues in the Foo<T>(Unknown) if they were to exist,
>
> I try this code and it will not miss other issues
>
>   c++
>   template <typename... T>
>   void foo(T &&...Params) {
>     Unknown;
>     foo<T>(Unknown);
>     ((foo<T>(Unknown + (10 / 0) + 10++)), ...);
>   }
>
>
>
>   build-demo/a.cpp:3:3: error: use of undeclared identifier 'Unknown'
>     Unknown;
>     ^
>   build-demo/a.cpp:4:3: error: expression contains unexpanded parameter pack 'T'
>     foo<T>(Unknown);
>     ^   ~
>   build-demo/a.cpp:5:35: error: expression is not assignable
>     ((foo<T>(Unknown + (10 / 0) + 10++)), ...);
>                                   ~~^
>   build-demo/a.cpp:5:12: error: use of undeclared identifier 'Unknown'
>     ((foo<T>(Unknown + (10 / 0) + 10++)), ...);
>              ^
>   build-demo/a.cpp:5:26: warning: division by zero is undefined [-Wdivision-by-zero]
>     ((foo<T>(Unknown + (10 / 0) + 10++)), ...);
>                            ^ ~
>   build-demo/a.cpp:4:10: error: use of undeclared identifier 'Unknown'
>     foo<T>(Unknown);
>            ^
>   1 warning and 5 errors generated.

Hmm... What about multiple packs, where the inner pack expansion would consume the ..., but not this one?  Add a few more tests for the above and ones like this as well to confirm.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145892/new/

https://reviews.llvm.org/D145892