philnik accepted this revision as: philnik. philnik added a comment. Mostly LGTM. Just a few nits.
================ Comment at: clang/www/hacking.html:276 + <!--=====================================================================--> + <h3 id="testingLibcxx">Testing changes affecting libcxx</h3> + <!--=====================================================================--> ---------------- To make it consistent throughout ================ Comment at: clang/www/hacking.html:284 + <li>Changing compiler builtins, especially the builtins used for type traits + or replacements of library functions like <tt>std::move</tt>, + <tt>std::forward</tt>.</li> ---------------- ================ Comment at: clang/www/hacking.html:296 + file will also add the libc++ group to the list of reviewers. The status of + the build will be available in Phabricator.</p> + ---------------- ================ Comment at: clang/www/hacking.html:300 + CI lacks the resources to build every Clang diff. A single run takes about + one hour. Secondly most changes of Clang won't affect libc++.</a> + ---------------- Having a secondly without a firstly seems weird. ================ Comment at: clang/www/hacking.html:310 + + <p>Unlike Clang, libc++ supports multiple versions of Clang. Therefore when a + patch changes the diagnostics it might be required to use a regex in the ---------------- This reads like you want to say that clang doesn't support multiple versions of clang, which seems self-evident. Maybe just drop the `Unlike Clang, `? ================ Comment at: libcxx/docs/Contributing.rst:87 -* C++20 for the Linux platform. -* MacOS C++20 for the Apple platform. +* C++XX for the Linux platform (where XX is the latest C++ version). +* MacOS X86_64 and MacOS arm64 for the Apple platform. ---------------- Complete nit, but `XX` reads to me like the digits have to be the same. I'd suggest `XY` to make it obvious that they can be different. ================ Comment at: libcxx/docs/Contributing.rst:88 +* C++XX for the Linux platform (where XX is the latest C++ version). +* MacOS X86_64 and MacOS arm64 for the Apple platform. + ---------------- ================ Comment at: libcxx/docs/Contributing.rst:107 + +The CI tests libc++ for all :ref:`supported platforms <SupportedPlatforms>`. +The build is started for every diff uploaded. A complete CI run takes ---------------- Not really currently. We still claim FreeBSD support without a CI runner. ================ Comment at: libcxx/docs/Contributing.rst:231-233 + export CC=clang-14 + export CXX=clang++-14 + run-buildbot generic-cxx17 ---------------- Complete nit, but I think `CC=clang-14 CXX=clang++-14 run-builtbot generic-cxx17` as an example would be better to avoid polluting people's environment if they're unfamiliar with a terminal. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133249/new/ https://reviews.llvm.org/D133249 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
