[PATCH] D55076: [analyzer] RetainCountChecker: recognize that OSObject can be created directly using an operator "new"

[Aaron Ballman via cfe-commits]

On Fri, Nov 30, 2018 at 2:26 PM George Karpenkov <ekarpenkov at 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 <aaron at aaronballman.com> wrote:
> >
> > On Fri, Nov 30, 2018 at 2:19 PM George Karpenkov <ekarpenkov at 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 <aaron at aaronballman.com> wrote:
> >>
> >> On Fri, Nov 30, 2018 at 9:37 AM Artem Dergachev via Phabricator via
> >> cfe-commits <cfe-commits at 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 at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >>
> >>
>