Good idea about using the mailing list, thanks, I’ll do that! > On Nov 30, 2018, at 11:30 AM, Aaron Ballman <aa...@aaronballman.com> wrote: > > On Fri, Nov 30, 2018 at 2:26 PM George Karpenkov <ekarpen...@apple.com> wrote: >> >> Thanks I’ll take a look. >> >> BTW when reverting could you use “git revert” or mention manually the >> phabricator revision being reverted, >> and apply reverts atomically? >> I (and many others) work exclusively using a git monorepo, so I don’t even >> have a straightforward way to lookup what "r347951” is. > > Given that I work exclusively in svn, I won't be using git revert. :-) > I can add the phab revision to the commit log when possible, but I > expect to continue to use svn revisions until the git transition takes > place. FWIW, you can use the mailing lists to look up what r347951 > (such as > http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20181126/252804.html). > Sorry for the troubles, though! > > ~Aaron > >> >> Thanks! >> >>> On Nov 30, 2018, at 11:20 AM, Aaron Ballman <aa...@aaronballman.com> wrote: >>> >>> On Fri, Nov 30, 2018 at 2:19 PM George Karpenkov <ekarpen...@apple.com> >>> wrote: >>>> >>>> Thanks and sorry about the trouble. I’ll recommit with size_t. >>> >>> No worries, it happens! FYI, I also had to commit r348023 as part of >>> the reverts. >>> >>> ~Aaron >>>> >>>> On Nov 30, 2018, at 10:56 AM, Aaron Ballman <aa...@aaronballman.com> wrote: >>>> >>>> On Fri, Nov 30, 2018 at 9:37 AM Artem Dergachev via Phabricator via >>>> cfe-commits <cfe-commits@lists.llvm.org> wrote: >>>> >>>> >>>> NoQ added inline comments. >>>> >>>> >>>> ================ >>>> Comment at: clang/test/Analysis/osobject-retain-release.cpp:27 >>>> + >>>> + static void * operator new(unsigned long size); >>>> + >>>> ---------------- >>>> NoQ wrote: >>>> >>>> I think we should use `size_t` as much as possible, because this may >>>> otherwise have weird consequences on platforms on which `size_t` is not >>>> defined as `unsigned long`. Not sure if this checker is ran on such >>>> platforms. But the test doesn't have the triple specified, so it runs >>>> under the host triple, which may be arbitrary and cause problems on >>>> buildbots. >>>> >>>> I.e., >>>> >>>> ``` >>>> typedef __typeof(sizeof(int)) size_t; >>>> // use size_t >>>> ``` >>>> >>>> http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/440/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aosobject-retain-release.cpp >>>> >>>> >>>> I reverted r347949 through r347951 in r348020 to get the bots back to >>>> green. >>>> >>>> ~Aaron >>>> >>>> >>>> >>>> Repository: >>>> rC Clang >>>> >>>> CHANGES SINCE LAST ACTION >>>> https://reviews.llvm.org/D55076/new/ >>>> >>>> https://reviews.llvm.org/D55076 >>>> >>>> >>>> >>>> _______________________________________________ >>>> cfe-commits mailing list >>>> cfe-commits@lists.llvm.org >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>> >>>> >>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
