[PATCH] D43159: Modernize: Use nullptr more.

ldionne updated this revision to Diff 307398. ldionne added a comment. Herald added a project: libc++. Herald added a reviewer: libc++. Rebase onto master Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43159/new/ https://reviews.llvm.org/D43159

[PATCH] D43159: Modernize: Use nullptr more.

ldionne added a comment. Ping -- is there still interest in moving forward with this? If so, please address the comments and update the patch, otherwise you can abandon the revision to clean up the libc++ review queue. Repository: rCXX libc++ CHANGES SINCE LAST ACTION

[PATCH] D43159: Modernize: Use nullptr more.

ldionne added inline comments. Comment at: include/__threading_support:323 bool __libcpp_thread_isnull(const __libcpp_thread_t *__t) { - return *__t == 0; + return *__t == nullptr; } mclow.lists wrote: > mclow.lists wrote: > > This one is wrong. >

[PATCH] D43159: Modernize: Use nullptr more.

mclow.lists added inline comments. Comment at: include/__threading_support:323 bool __libcpp_thread_isnull(const __libcpp_thread_t *__t) { - return *__t == 0; + return *__t == nullptr; } mclow.lists wrote: > This one is wrong. `__libcpp_thread_t` is an alias

[PATCH] D43159: Modernize: Use nullptr more.

ldionne requested changes to this revision. ldionne added a comment. I agree with Marshall's requests. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43159/new/ https://reviews.llvm.org/D43159 ___ cfe-commits

[PATCH] D43159: Modernize: Use nullptr more.

mclow.lists added inline comments. Comment at: include/algorithm:4431 value_type* __p = __buff; -for (_BidirectionalIterator __i = __first; __i != __middle; __d.__incr((value_type*)0), (void) ++__i, ++__p) +for (_BidirectionalIterator __i = __first; __i

[PATCH] D43159: Modernize: Use nullptr more.

mclow.lists added a comment. In ``, you missed a couple of `(value_type*)0`. Line 3356, 3369, 3486 and 3499. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43159/new/ https://reviews.llvm.org/D43159 ___

[PATCH] D43159: Modernize: Use nullptr more.

mclow.lists requested changes to this revision. mclow.lists added a comment. This revision now requires changes to proceed. Did you try to build libc++ or run the tests before submitting this? Comment at: include/__threading_support:323 bool __libcpp_thread_isnull(const

[PATCH] D43159: Modernize: Use nullptr more.

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. Herald added a subscriber: dexonsmith. I'm fine with this given what the author said about `nullptr` already being used in those headers. Repository: rCXX libc++ CHANGES SINCE LAST

[PATCH] D43159: Modernize: Use nullptr more.

brucem added a comment. In addition to my previous explanation for the concerns about using `nullptr` more ... it should be noted that many of these files already use `nullptr`. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43159/new/

[PATCH] D43159: Modernize: Use nullptr more.

brucem updated this revision to Diff 207356. brucem added a comment. Updating to current master. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43159/new/ https://reviews.llvm.org/D43159 Files: include/__locale include/__sso_allocator include/__string

[PATCH] D43159: Modernize: Use nullptr more.

brucem marked an inline comment as done. brucem added a comment. In D43159#1526215 , @mclow.lists wrote: > What was the result of testing with `-std=c++98` and/or `-std=gnu++98` ? > The code changes look fine; but as @Ericwf said It seems to work

[PATCH] D43159: Modernize: Use nullptr more.

brucem marked 2 inline comments as done. brucem added inline comments. Comment at: include/memory:1259 template static __two __test(...); template static char __test(typename _Xp::template rebind<_Up>::other* = 0); public: zoecarver wrote: > This

[PATCH] D43159: Modernize: Use nullptr more.

zoecarver added inline comments. Comment at: include/memory:1259 template static __two __test(...); template static char __test(typename _Xp::template rebind<_Up>::other* = 0); public: This could be `nullptr` too. Repository: rCXX libc++

[PATCH] D43159: Modernize: Use nullptr more.

brucem updated this revision to Diff 202555. brucem added a comment. Remove CMakeLists.txt change. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43159/new/ https://reviews.llvm.org/D43159 Files: include/__functional_base include/__locale

[PATCH] D43159: Modernize: Use nullptr more.

brucem updated this revision to Diff 202554. brucem added a comment. Herald added a subscriber: mgorny. Updated to current master and added more nullptr usage. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43159/new/ https://reviews.llvm.org/D43159 Files:

[PATCH] D43159: Modernize: Use nullptr more.

mclow.lists added a comment. In D43159#1526170 , @brucem wrote: > Can we revive this review? I'd still like to land this ... What was the result of testing with `-std=c++98` and/or `-std=gnu++98` ? The code changes look fine; but as @Ericwf said

[PATCH] D43159: Modernize: Use nullptr more.

brucem added a comment. Herald added subscribers: libcxx-commits, jfb, ldionne, christof. Can we revive this review? I'd still like to land this ... Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43159/new/ https://reviews.llvm.org/D43159

[PATCH] D43159: Modernize: Use nullptr more.

brucem added a comment. In https://reviews.llvm.org/D43159#1004617, @jroelofs wrote: > Is it worth adding `-Werror=zero-as-null-pointer-constant` to the build? I'll look at this as a follow up. Repository: rCXX libc++ https://reviews.llvm.org/D43159

[PATCH] D43159: Modernize: Use nullptr more.

brucem added a comment. In https://reviews.llvm.org/D43159#1004639, @dim wrote: > In https://reviews.llvm.org/D43159#1004625, @EricWF wrote: > > > So my main concern with this patch is that `nullptr` is actually > > `#defined`'ed in C++03 mode. That definition comes from the `__nullptr` > >

[PATCH] D43159: Modernize: Use nullptr more.

brucem updated this revision to Diff 133996. brucem added a comment. Addressed minor issues. - Addressed missing __end_ initialization from valarray. - Removed cast that was no longer needed. - Added nullptr usage to include/regex. Repository: rCXX libc++ https://reviews.llvm.org/D43159

[PATCH] D43159: Modernize: Use nullptr more.

dim added a comment. In https://reviews.llvm.org/D43159#1004625, @EricWF wrote: > So my main concern with this patch is that `nullptr` is actually > `#defined`'ed in C++03 mode. That definition comes from the `__nullptr` > header, and therefore we would need to add that header to each include

[PATCH] D43159: Modernize: Use nullptr more.

EricWF added a comment. So my main concern with this patch is that `nullptr` is actually `#defined`'ed in C++03 mode. That definition comes from the `__nullptr` header, and therefore we would need to add that header to each include which uses it. Which kind of sucks. Repository: rCXX

[PATCH] D43159: Modernize: Use nullptr more.

jroelofs added a comment. Is it worth adding `-Werror=zero-as-null-pointer-constant` to the build? Repository: rCXX libc++ https://reviews.llvm.org/D43159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43159: Modernize: Use nullptr more.

dim added a comment. LGTM minus a few nits, though it would be nice if you can verify that all the changed headers still compile in `-std=c++98` and/or `-std=gnu++98` mode. Comment at: include/functional:1573 return &__f_.first(); -return (const void*)0; +

[PATCH] D43159: Modernize: Use nullptr more.

brucem updated this revision to Diff 133778. brucem added a comment. More nullptr usage. Repository: rCXX libc++ https://reviews.llvm.org/D43159 Files: include/__locale include/__sso_allocator include/__string include/__threading_support include/algorithm include/bitset

[PATCH] D43159: Modernize: Use nullptr more.

brucem created this revision. brucem added reviewers: mclow.lists, EricWF. Repository: rCXX libc++ https://reviews.llvm.org/D43159 Files: include/__locale include/__string include/__threading_support include/algorithm include/bitset include/chrono include/fstream