[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

Thu Apr 18 17:13:43 PDT 2019

jdenny added a comment.

In D59712#1472358 <https://reviews.llvm.org/D59712#1472358>, @hfinkel wrote:

> > I've never tried running the other tests you mention, for any patch.  I thought people normally left those to the bots.  Should this patch be handled differently?
>
> We have a lot of people actively working off of trunk, and we try very hard to keep trunk clean. The bots are a second line of defense, not the primary checkers. In any case, this comes down to professional judgement. It is not uncommon to ask for a patch author to check self hosting and a test suite run before committing - specifically, those patches that might affect correctness, or introduce other subtle problems, and for which running the compiler over a bunch of C/C++ code might uncover a problem.

Thanks for explaining.  It's my first time receiving these particular requests (probably because of what parts of LLVM I normally edit), so I wasn't sure I understood.

For self-hosting, is it best to build again with CMAKE_C_COMPILER and CMAKE_CXX_COMPILE pointing into the previous build, or is there a better approach?

> Also, is this review now missing some files? I see here only updates to  APSInt.h (only adding functions), APSIntTest.cpp, and a bunch of tests. Nothing that would cause changes to the tests, however (maybe I'm just missing something).

All looks fine to me.  The APSInt.h changes are the reason for all the test changes.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59712