hbatagelo wrote:
> This is incorrect, I think this is another issue where RecoveryExpr sneaks
> into Sema deeply where it shouldn't. collectUnexpandedPacks shouldn't handle
> errors, and we're supposed to bail out early.
Bailing out earlier (in `ActOnFinishFullExpr`) was my first approach, but it
drops the diagnostic for the unexpanded pack. It diagnoses only the error in
the subexpression. That makes the behavior inconsistent across the sibling
forms (`Int<Ts>()` and `Int<Ts>{}`) and diverges from GCC, which diagnoses the
pack in all cases. GCC happens not to emit the initialization error for
`Int<Ts>({})`, so the patch does not match GCC exactly, but it emits both
diagnostics and keeps the pack diagnostic consistent with the siblings. It is
also consistent with existing cases that expect both diagnostics, e.g., from
`fold_expr_typo.cpp`:
```cpp
template <typename... T>
void foo(T &&...Params) {
foo<T>(Unknown); // expected-error {{expression contains unexpanded parameter
pack 'T'}}\
expected-error {{use of undeclared identifier 'Unknown'}}
}
```
An early bail-out would introduce a regression here (only the "undeclared
identifier" error would be diagnosed).
FWIW, I also tried to bail out after the collection in
`DiagnoseUnexpandedParameterPacks` when `Unexpanded` is empty. It works for the
previous example (the pack is reachable, so `Unexpanded` is non-empty), but
still drops the pack diagnostic for `Int<Ts>({})` because the pack isn't
reachable without traversing the `RecoveryExpr`'s type.
I agree the collector handling a `RecoveryExpr` is awkward, but the diagnostic
cost of bailing out seems worse here. Open to other ideas if you have one in
mind.
https://github.com/llvm/llvm-project/pull/236838
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
