[libcxx-commits] [PATCH] D141334: [libc++][test] Silence allocator conversion warnings

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D141334#4040909 <https://reviews.llvm.org/D141334#4040909>, @CaseyCarter wrote:

> In D141334#4040566 <https://reviews.llvm.org/D141334#4040566>, @ldionne wrote:
>
>> CI issues seem unrelated. Just curious -- how did you catch that? Is there anything we can/should add to our implementation?
>
> As you may be aware, MSVC doesn't have the "system header" notion that GCC/Clang do. Consequently we run testing with TONS of warnings enabled to ensure that our header code only emits warnings when the user has asked us to do something questionable. This is a pretty good example: `Allocator`'s size type is `size_t`, so using this allocator with Standard Library containers results in lots of narrowing conversions from size_t to int. If the user asks for narrowing conversion warnings, they'll get them. The consequence is we have to police our tests pretty carefully to avoid warnings so we only see warnings from bad product code.

Interesting! And I guess we don't enable this warning in our testing configuration. We have a block like this in `params.py`:

  # These warnings should be enabled in order to support the MSVC
  # team using the test suite; They enable the warnings below and
  # expect the test suite to be clean.
  '-Wsign-compare',
  '-Wunused-variable',
  '-Wunused-parameter',
  '-Wunreachable-code',
  '-Wno-unused-local-typedef',

Would it maybe make sense to add a new warning that would catch this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141334