carlos4242 wrote:
I think unit testing the bug fix to this function (the copy that is in Sema) is
going to be nearly impossible.
To prove the point, I tried hard coding the wrong return value for many types
just now...
```
switch (Kind) {
case tok::kw_bool:
case tok::kw_short:
case tok::kw_long:
case tok::kw___int64:
case tok::kw___int128:
case tok::kw_signed:
case tok::kw_unsigned:
case tok::kw_char:
case tok::kw_int:
case tok::kw_half:
case tok::kw_float:
case tok::kw_double:
case tok::kw___bf16:
case tok::kw__Float16:
case tok::kw___float128:
case tok::kw___ibm128:
case tok::kw_wchar_t:
return false;
```
...Then re-ran the full clang/tests suite (18,000+ tests) and all passed as
normal. So there's basically very little unit test coverage on this function as
it stands. It's not just a question of improving or adding to existing tests.
Whoever first wrote this function seems to have not added really enough unit
test coverage at the time, and it's only called in two obscure places in Sema.
The duplicate copy function in Format is more widely used and would probably be
easier to test. But for this PR I think I'll make the changes you suggest, then
leave it as is... either it gets approved without new tests or it'll get culled
one day as an abandoned PR. I'll leave that up to the maintainers.
https://github.com/llvm/llvm-project/pull/79037
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
