llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.15k stars 12.03k forks source link

[libc++] `ranges::copy(join_view, out)` isn't constexpr #69083

Closed philnik777 closed 10 months ago

philnik777 commented 1 year ago
#include <algorithm>
#include <ranges>
#include <vector>

constexpr auto test(std::vector<std::vector<int>> v, int* out) {
  std::ranges::copy(v | std::views::join, out);
}

static_assert([] {
  int a[4] = {};
  test({{1, 2}, {3, 4}}, a);
  return true;
}());

Is rejected by libc++ because the _CopySegment helper isn't constexpr.

llvmbot commented 1 year ago

@llvm/issue-subscribers-good-first-issue

Author: None (philnik777)

```c++ #include <algorithm> #include <ranges> #include <vector> constexpr auto test(std::vector<std::vector<int>> v, int* out) { std::ranges::copy(v | std::views::join, out); } static_assert([] { int a[4] = {}; test({{1, 2}, {3, 4}}, a); return true; }()); ``` Is rejected by libc++ because the `_CopySegment` helper isn't `constexpr`.
Rajveer100 commented 1 year ago

@philnik777 Hey, I am actually new to contributing to LLVM, can you give a head start on this to get me going! For more info about me, please checkout my GitHub profile and other projects!

I have gone through the initial guidelines and setup and have done the basic clone and build steps, though, I am not exactly sure to how we can debug and reproduce issues?

Rajveer100 commented 1 year ago

Command:

clang++ -std=c++20 debug.cpp -o debug
debug.cpp:7:37: error: no member named 'join' in namespace 'std::ranges::views'
  std::ranges::copy(v | std::views::join, out);
                        ~~~~~~~~~~~~^
debug.cpp:10:15: error: static assertion expression is not an integral constant expression
static_assert([] {
              ^~~~
2 errors generated.

Is this the intended way to reproduce issues?

philnik777 commented 1 year ago

You have to add -fexperimental-library to get views::join. Then you should get an error about a function not being constexpr.

Rajveer100 commented 1 year ago

Makes sense, also I presume LLVM has now potentially migrated to GitHub?

philnik777 commented 1 year ago

Yes, you can open a PR against this repo now.

Rajveer100 commented 1 year ago

Linker Command Failed:

clang++ -std=c++20 -fexperimental-library debug.cpp -o debug
ld: warning: ignoring file '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/lib/libc++experimental.a[arm64e][3](format.cpp.o)': found architecture 'arm64e', required architecture 'arm64'
ld: warning: ignoring file '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/lib/libc++experimental.a[arm64e][2](memory_resource.cpp.o)': found architecture 'arm64e', required architecture 'arm64'
ld: Undefined symbols:
  _main, referenced from:
      <initial-undefines>
clang: error: linker command failed with exit code 1 (use -v to see invocation)

PS: Adding main function fixed it:

ld: warning: ignoring file '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/lib/libc++experimental.a[arm64e][3](format.cpp.o)': found architecture 'arm64e', required architecture 'arm64'
ld: warning: ignoring file '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/lib/libc++experimental.a[arm64e][2](memory_resource.cpp.o)': found architecture 'arm64e', required architecture 'arm64'
Rajveer100 commented 1 year ago

PS: Adding main function fixed it.

@philnik777 In my case the code snippet is successfully compiling, am I missing anything here?

philnik777 commented 1 year ago

Are you compiling against your system library or against trunk libc++? And what code did you use exactly?

Rajveer100 commented 1 year ago

This is the command I used:

clang++ -std=c++20 -fexperimental-library debug.cpp -o debug

Snippet:

#include <algorithm>
#include <ranges>
#include <vector>

constexpr auto test(std::vector<std::vector<int>> v, int* out) {
  std::ranges::copy(v | std::views::join, out);
}

static_assert([] {
  int a[4] = {};
  test({{1, 2}, {3, 4}}, a);
  return true;
}());

int main() { }
philnik777 commented 1 year ago

Did you build the clang version yourself? Also, you might have to add -stdlib=libc++ depending on which system you're on.

Rajveer100 commented 1 year ago

Well, I am on macOS and libc++ is the default (I had still added the flag to cross check).

Also clang is available by default again.

philnik777 commented 1 year ago

Then you're testing the system libc++ and not the trunk one. This is probably a regression.

Rajveer100 commented 1 year ago

How do I test the regression?

philnik777 commented 1 year ago

You'll have to use a trunk libc++. You can see the regression here. See https://libcxx.llvm.org/BuildingLibcxx.html#id3 for how to build libc++.

Rajveer100 commented 1 year ago

Warning If your operating system already provides libc++, it is important to be careful not to replace it. Replacing your system’s libc++ installation could render it non-functional. Use the CMake option CMAKE_INSTALL_PREFIX to select a safe place to install libc++.

What's the right way to avoid any such issues? Should I just follow the steps for default build, or are there some additions that are needed with this?

Also, if I understand correctly, we cannot do an in-tree directory build and the instructions does a mkdir build which already exists in the llvm repo. What do you suggest for this?

Rajveer100 commented 1 year ago

For now I have created a directory trunk-build in llvm-project/ and have done the installation successfully, although few tests had failed for #Test command in the instructions, hope that isn't an issue.

Rajveer100 commented 1 year ago

Command:

clang++ -std=c++20 -fexperimental-library -DCMAKE_CXX_COMPILER=$HOME/Desktop/llvm/llvm-project/trunk-build/include/c++ debug.cpp -o debug

Still compiles successfully, no errors.

ldionne commented 1 year ago

The easiest way to get started with a build + test of the library would be to do something like:

./libcxx/utils/ci/run-buildbot generic-cxx23

This will build the library and run the tests in C++23 mode. Then you can add some tests for copy / join_view, run ninja -C build/generic-cxx23 check-cxx and see those tests failing. Then you can fix the issue and run it again, it should pass.

Rajveer100 commented 1 year ago

Once I make the changes, can I just target a particular test file associated with my change rather than running all tests?

ldionne commented 1 year ago

Once I make the changes, can I just target a particular test file associated with my change rather than running all tests?

Yes! See https://libcxx.llvm.org/TestingLibcxx.html#usage.

mordante commented 10 months ago

@philnik777 I see some commit for this issue. Has it been fixed?

philnik777 commented 10 months ago

Yes.