[libcxx-commits] [PATCH] D113868: [libcxx] Cast to the right `difference_type` in various algorithms

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__algorithm/make_heap.h:34
+        for (difference_type __start = (__n - difference_type(2)) / 2; __start >= 0; --__start) {
+          _VSTD::__sift_down<_Compare>(__first, __last, __comp, __n, __first + __start);
         }
----------------
Mordante wrote:
> ldionne wrote:
> > Quuxplusone wrote:
> > > Mordante wrote:
> > > > fwolff wrote:
> > > > > Quuxplusone wrote:
> > > > > > fwolff wrote:
> > > > > > > Quuxplusone wrote:
> > > > > > > > You've broken the indentation here (and various places, especially in `sort.h`). Please make sure to preserve the existing indentation style wherever you change code. (If it's 2-space, leave it 2-space; if it's 4-space, leave it 4-space; don't mix styles within the same function.)
> > > > > > > I've tried to, but Arcanist wouldn't let me update this revision without the clang-format changes. Is there a special option I can use to suppress this?
> > > > > > I do think there's some `arc` option for that, but I don't use `arc` myself. I just use `git show -U999 > foo.diff` and then "Update Diff" in the right sidebar at the top of the page.
> > > > > OK. I was unsure because the [[ https://libcxx.llvm.org/Contributing.html | "Contributing to libc++" guide ]] says that you have to use `arc` for submitting patches, but then I'll just do it via "Update Diff".
> > > > For libc++ we really prefer `arc` else the CI isn't triggered. For arc you can use `--nolint` or press `n` at all suggestions `arc` makes.
> > > > or else the CI isn't triggered
> > > 
> > > I use plain old `git diff -U999` + "Update Diff", and I'm not aware that my PRs ever fail to trigger CI. (The only thing to watch out for is sometimes if you update twice in very quick succession, the builder may get confused when it goes to pull a revision and the revision is already gone. But I assume `arc` would have the same problem.)
> > > 
> > > That said, yeah, `arc --nolint` was exactly the thing I was trying to think of.
> > I know at some point you at least had to make sure the right tags were added to the review, otherwise CI wouldn't get triggered. And `arc diff` made sure this happened, but I've seen cases where manually uploaded diffs didn't have the right tags.
> > 
> > It's been a while since I've seen this issue, to be fair.
> That issue was why I switched from direct uploads to `arc`. I wasn't aware the issue has since been resolved.
Aha. IIUC, I do this part manually too: when I upload a diff, Phab asks me questions like "what repository" ("llvm"-tab-complete: the GitHub monorepo) and "what project" ("libc++"-tab-complete) and "what reviewers" (typically "ldionne, mordante, libc++"). It would not surprise me if, when I //didn't// put in "libc++" for the project, it failed to trigger buildkite. But for me that's all just part of the process of uploading a diff.


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

https://reviews.llvm.org/D113868