[libcxx-commits] [PATCH] D156052: [libc++][doc] Improves contribution page.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In D156052#4525815 <https://reviews.llvm.org/D156052#4525815>, @jloser wrote:

> So happy to see this finally get written up. I appreciate you doing this! This will be very helpful for new contributors.

You're welcome. I hope it indeed helps new contributors.



================
Comment at: libcxx/docs/Contributing.rst:44
+
+In libc++, like all standard libraries, we use ``__ugly_names``. These
+names are reserved for implementations, so users may not use them in
----------------
philnik wrote:
> Maybe we should mention that we use clang-tidy for some of these things? I think that is good motivation to enable it in your IDE.
Does clang-tidy work out of the box in IDEs. I've heard some people mention there are issues due to copying our files to the build directory. (I tend to use clang-tidy from the command line.)


================
Comment at: libcxx/docs/Contributing.rst:53-54
+
+Function calls in the standard library that use types provided by the
+user are susceptible to ADL. This means calling ``move(UserType)`` may
+not call ``std::move``. Therefore function calls are using qualified
----------------
philnik wrote:
> This applies to all types, not just user-provided types. Maybe something like "Unqualified function calls result in ADL"? Also, linking to https://en.cppreference.com/w/cpp/language/adl would be nice, since most people aren't super familiar with ADL.
True, but I think the biggest issue is with types not in the `std` namespace.


================
Comment at: libcxx/docs/Contributing.rst:59-62
+Function overloading also applies to operators. Using ``&user_object``
+may call a user defined ``operator&``. Use ``std::addressof``  instead.
+Similar for ``operator,``. When using the ``,`` make sure to cast the
+result to ``void``. For example:
----------------
philnik wrote:
> I'd just avoid `operator,` altogether if possible. It makes the code almost always easier to read.
We use `operator,` in for loops. So I think it's good to have information why that is done. I've noticed you remove them sometimes. Still this is used in the wording of the Standard so it's good to know why this is done.


================
Comment at: libcxx/docs/Contributing.rst:150
+  properly implement library features.
+
+
----------------
philnik wrote:
> https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations would also be quite useful I think.
Good one, I actually have it in the bookmarks of my browser.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156052