[clang] [clang] Improved isSimpleTypeSpecifier (PR #79037)

Carl Peto via cfe-commits cfe-commits at lists.llvm.org
Sun Jan 28 08:03:46 PST 2024


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


More information about the cfe-commits mailing list