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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
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
----------------
Mordante wrote:
> 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.)
It works just fine for me. I have some custom stuff for parts of my setup to work, but clang-tidy worked from the start. That was the reason for D113849.


================
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
----------------
Mordante wrote:
> 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.
I just want to avoid confusing people. The current wording sound good to me.


================
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:
----------------
Mordante wrote:
> 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.
I'm not against explaining why it is done. As-is the text sounds to me like we encourage using `operator,` for these kinds of loops, which at least I definitely do not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156052