hfinkel added a comment. In D59712#1472544 <https://reviews.llvm.org/D59712#1472544>, @jdenny wrote:
> 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. No problem. > 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? That's what I do. > > >> 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. Indeed. I forgot that you were changing overrides. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59712/new/ https://reviews.llvm.org/D59712 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
