Roc standard library in C++ #6230

brian-carroll · 2023-12-09T11:49:59Z

Copying over from https://github.com/brian-carroll/roc-std-cpp/releases/tag/copy-to-main-repo
This will not be useful until we also have a CppGlue.roc, which will probably import the .h files as strings.

I'm not sure what we want to do from a CI point of view.
The tests depend on GNU Make and clang or gcc.
You can run the tests with make check
If you want to use gcc instead of clang, it's make check CC=gcc

Oh and I also vendored a test framework! Hopefully the license is OK?

…tag/copy-to-main-repo

bhansconnect

Just a few high level comments. Didn't really dig into the impl much. At some point I'll probably want to push a PR with some suggested changes, but that is for another time.

bhansconnect · 2023-12-09T20:18:42Z

crates/glue/cpp/src/internal.h

+    /**
+     * Base class for all Roc value types
+     * Implements common logic like reference counting.
+     * We don't use virtual functions because we want to avoid the vtable.
+     */


Unless I am mixing something up, this will still use a vtable, the base class just has an implementation. To avoid a vtable, you would have to use some sort of template.

Anyway, given there will be little shared logic, I would just drop this and not have a generic Value type.

OK I misunderstood vtables then. I deleted the comment.

I'm not going to get rid of Value as part of this PR because the idea is to take what I have and bring it into the repo. The understanding is that we can make improvements later and this can be one of them.

bhansconnect · 2023-12-09T20:19:03Z

crates/glue/cpp/src/dict.h

Probably should remove this file? Our dictionaries haven't been an associative list in quite a long time. On top of that, the impl may change more. I don't think it would be better to force the roc platform to expose dictionary functions to the platform than for the platform to attempt to implement this.

Our dictionaries haven't been an associative list in quite a long time.

OK then we also need to update or delete crates/roc_std/src/roc_dict.rs which is what I based this on, and what our RustGlue.roc uses.

Anton-4 · 2023-12-10T14:38:52Z

I'll set up some CI tests

ayazhafiz · 2023-12-10T16:16:08Z

crates/glue/cpp/src/internal.h

+void *roc_alloc(size_t size, uint32_t alignment);
+void *roc_realloc(void *ptr, size_t new_size, size_t old_size, size_t alignment);
+void roc_dealloc(void *ptr, uint32_t alignment);
+void roc_panic(const char *message, uint32_t _tag_id);


Just FYI this should take a Str in the first parameter

Oh yes of course. We must have this bug in so many places. I copied this code from somewhere else in the repo.

Unfortunately, fixing this creates a cyclic dependency. I need to make internal.h depend on str.h but str.h already depends on internal.h for the allocation functions.

I might be able to break it by reorganising the files or pre-declaring something.

Anton-4 · 2023-12-10T16:56:08Z

make check fails on apple silicon:

$ make check
clang++ -g -Wall -std=c++14 -Isrc -Itest/lib -o run-tests test/main.cpp
./run-tests
Running all tests.
run-tests(53973,0x1e58d5ec0) malloc: Heap corruption detected, free list is damaged at 0x600002e301c0
*** Incorrect guard value: 63974227902911
run-tests(53973,0x1e58d5ec0) malloc: *** set a breakpoint in malloc_error_break to debug
make: *** [Makefile:14: check] Abort trap: 6

brian-carroll · 2023-12-11T07:28:43Z

That's weird, I developed this on an M1 Mac!
I ran make clean; make check just now on this branch and it succeeds for me.
😕

brian-carroll · 2023-12-11T07:34:49Z

Managed to reproduce using the exact same commands as CI

brian-carroll · 2023-12-11T08:40:16Z

My local installation of Clang

❯ clang++ --version 
Apple clang version 14.0.0 (clang-1400.0.29.202)
Target: arm64-apple-darwin22.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

Roc nix version of Clang

❯ clang++ --version
clang version 11.1.0
Target: aarch64-apple-darwin
Thread model: posix
InstalledDir: /nix/store/mk342jj556ad5x464srgj7qi4gczf3d9-clang-11.1.0/bin

Clang 11 is throwing in the Result tests somewhere.
There are 4 tests. If I run any of them individually, they're fine, but if I run more than one, I get this heap corruption issue.

./run-tests --group Result # fails
./run-tests --group Result --name Ok # succeeds
./run-tests --group Result --name Err # succeeds
./run-tests --group Result --name OkSameType # succeeds
./run-tests --group Result --name ErrSameType # succeeds

brian-carroll · 2023-12-17T10:50:11Z

@Anton-4 do you know if there's a good reason we're on Clang 11.1? From the LLVM releases page it seems to date back to Feb 2021.
It looks like LLVM versions correspond to Clang versions, and I believe the compiler is on Clang 16 now, right?
Perhaps a way forward here is to upgrade Clang to match our LLVM version. Then this will pass CI.
If for some reason we really need Clang 11 backward compatibility then we can file an issue for that.

Anton-4 · 2023-12-19T15:35:56Z

The Undefined symbols for architecture arm64 is the same one as in #6146, I'm going to bisect the nix revision to find the root cause.

Anton-4 · 2023-12-23T10:28:37Z

After a long and difficult bisect journey I was able to find the nix commit that caused the issue. Follow up discussion will happen in #6303.

Signed-off-by: Anton-4 <17049058+Anton-4@users.noreply.github.com>

Needed for RUSTFLAGS Signed-off-by: Anton-4 <17049058+Anton-4@users.noreply.github.com>

Anton-4 · 2023-12-26T17:52:23Z

Hmm, I'm seeing issues with clang++ 16 and 15 as well @brian-carroll
clang++ 16:

$ make clean; make check
rm -f run-tests
rm -rf run-tests.dSYM
clang++ -g -Wall -std=c++14 -Isrc -Itest/lib -o run-tests test/main.cpp
./run-tests
Running all tests.
Running [Str/EmptyBytes]: Passed 1 out of 1 tests in 4us
Running [Str/SingleCharBytes]: Passed 1 out of 1 tests in 1us
Running [Str/MaxSmallStrBytes]: Passed 1 out of 1 tests in 2us
Running [Str/ShortLength]: Passed 1 out of 1 tests in 1us
Running [Str/LongLength]: Passed 1 out of 1 tests in 0us
Running [Str/EmptyLength]: Passed 1 out of 1 tests in 1us
Running [Str/EmptyCapacity]: Passed 1 out of 1 tests in 1us
Running [List/FromArrayI64]: Passed 2 out of 2 tests in 2us
Running [List/FromArrayOfStruct]: Passed 2 out of 2 tests in 0us
Running [List/ReserveSmall]: Passed 2 out of 2 tests in 2us
Running [List/Void]: Passed 3 out of 3 tests in 1us
Running [Box/Deref]: Passed 1 out of 1 tests in 1us
Running [Box/Arrow]: Passed 1 out of 1 tests in 1us
Running [Result/Ok]: Passed 1 out of 1 tests in 2us
run-tests(11629,0x1e19c5ec0) malloc: Heap corruption detected, free list is damaged at 0x600001a381c0
*** Incorrect guard value: 153512106820031
run-tests(11629,0x1e19c5ec0) malloc: *** set a breakpoint in malloc_error_break to debug
Running [Result/Err]make: *** [Makefile:14: check] Abort trap: 6
admins-Mac-Studio:cpp admin$ clang++ --version
clang version 16.0.6
Target: arm64-apple-darwin
Thread model: posix
InstalledDir: /nix/store/3vzbr8yb4c233lhi1qz78b8kaaxrjqcj-clang-16.0.6/bin

clang++ 15 (outside nix):

❯ make clean; make check
rm -f run-tests
rm -rf run-tests.dSYM
clang++ -g -Wall -std=c++14 -Isrc -Itest/lib -o run-tests test/main.cpp
./run-tests
Running all tests.
Running [Str/EmptyBytes]: Passed 1 out of 1 tests in 10us
Running [Str/SingleCharBytes]: Passed 1 out of 1 tests in 1us
Running [Str/MaxSmallStrBytes]: Passed 1 out of 1 tests in 2us
Running [Str/ShortLength]: Passed 1 out of 1 tests in 0us
Running [Str/LongLength]: Passed 1 out of 1 tests in 1us
Running [Str/EmptyLength]: Passed 1 out of 1 tests in 2us
Running [Str/EmptyCapacity]: Passed 1 out of 1 tests in 1us
Running [List/FromArrayI64]: Passed 2 out of 2 tests in 2us
Running [List/FromArrayOfStruct]: Passed 2 out of 2 tests in 2us
Running [List/ReserveSmall]: Passed 2 out of 2 tests in 2us
Running [List/Void]: Passed 3 out of 3 tests in 2us
Running [Box/Deref]: Passed 1 out of 1 tests in 2us
Running [Box/Arrow]: Passed 1 out of 1 tests in 2us
Running [Result/Ok]: Passed 1 out of 1 tests in 2us
run-tests(11708,0x1e19c5ec0) malloc: Heap corruption detected, free list is damaged at 0x60000075c200
*** Incorrect guard value: 237818967605759
run-tests(11708,0x1e19c5ec0) malloc: *** set a breakpoint in malloc_error_break to debug
Running [Result/Err]make: *** [check] Abort trap: 6

crates/glue/cpp on  roc-std-cpp 
❯ clang++ --version
Apple clang version 15.0.0 (clang-1500.0.40.1)
Target: arm64-apple-darwin23.1.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

github-actions · 2024-01-26T01:48:45Z

Thank you for your contribution! Sometimes PRs end up staying open for a long time without activity, which can make the list of open PRs get long and time-consuming to review. To keep things manageable for reviewers, this bot automatically closes PRs that haven’t had activity in 60 days. This PR hasn’t had activity in 30 days, so it will be automatically closed if there is no more activity in the next 30 days. Keep in mind that PRs marked Closed are not deleted, so no matter what, the PR will still be right here in the repo. You can always access it and reopen it anytime you like!

brian-carroll added 2 commits December 9, 2023 11:22

Copy code from https://github.com/brian-carroll/roc-std-cpp/releases/…

d7019bb

…tag/copy-to-main-repo

README changes

b4dae34

brian-carroll requested review from rtfeldman, Anton-4 and bhansconnect December 9, 2023 11:50

Fix typo

4a74af3

bhansconnect previously approved these changes Dec 9, 2023

View reviewed changes

add c++ tests

7e87d76

Anton-4 dismissed bhansconnect’s stale review via 7e87d76 December 10, 2023 14:46

brian-carroll added 2 commits December 10, 2023 15:35

Remove outdated Dict implementation

4631c86

Remove inaccurate comment

d6e8319

Anton-4 previously approved these changes Dec 10, 2023

View reviewed changes

ayazhafiz reviewed Dec 10, 2023

View reviewed changes

brian-carroll added 4 commits December 11, 2023 07:44

Merge branch 'main' of github.com:roc-lang/roc into roc-std-cpp

eb39cc4

roc_panic takes a Roc::Str*, not a char*

aedd14c

Make C++ tests verbose by default

954a86d

Usage feedback for C++ test program

ed87a97

brian-carroll dismissed Anton-4’s stale review via ed87a97 December 11, 2023 08:40

brian-carroll mentioned this pull request Dec 17, 2023

Rust roc_std::Dict does not match Roc Dict #6292

Open

update nixpkgs rev

7d64732

Anton-4 added 2 commits December 26, 2023 16:28

fix #6303

8a66759

Signed-off-by: Anton-4 <17049058+Anton-4@users.noreply.github.com>

fix lib not found

0dfee06

Signed-off-by: Anton-4 <17049058+Anton-4@users.noreply.github.com>

Anton-4 added 2 commits December 26, 2023 17:46

remove -lc++abi for wasm repl

50f7957

Signed-off-by: Anton-4 <17049058+Anton-4@users.noreply.github.com>

allow unbound variables

c7105e8

Needed for RUSTFLAGS Signed-off-by: Anton-4 <17049058+Anton-4@users.noreply.github.com>

github-actions bot added the inactive for 30 days label Jan 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Roc standard library in C++ #6230

Roc standard library in C++ #6230

brian-carroll commented Dec 9, 2023 •

edited

bhansconnect left a comment

bhansconnect Dec 9, 2023

bhansconnect Dec 9, 2023

brian-carroll Dec 10, 2023

brian-carroll Dec 10, 2023 •

edited

bhansconnect Dec 9, 2023

brian-carroll Dec 10, 2023 •

edited

Anton-4 commented Dec 10, 2023

ayazhafiz Dec 10, 2023

brian-carroll Dec 11, 2023

Anton-4 commented Dec 10, 2023

brian-carroll commented Dec 11, 2023 •

edited

brian-carroll commented Dec 11, 2023

brian-carroll commented Dec 11, 2023

brian-carroll commented Dec 17, 2023 •

edited

Anton-4 commented Dec 19, 2023

Anton-4 commented Dec 23, 2023

Anton-4 commented Dec 26, 2023 •

edited

github-actions bot commented Jan 26, 2024

Roc standard library in C++ #6230

Are you sure you want to change the base?

Roc standard library in C++ #6230

Conversation

brian-carroll commented Dec 9, 2023 • edited

bhansconnect left a comment

Choose a reason for hiding this comment

bhansconnect Dec 9, 2023

Choose a reason for hiding this comment

bhansconnect Dec 9, 2023

Choose a reason for hiding this comment

brian-carroll Dec 10, 2023

Choose a reason for hiding this comment

brian-carroll Dec 10, 2023 • edited

Choose a reason for hiding this comment

bhansconnect Dec 9, 2023

Choose a reason for hiding this comment

brian-carroll Dec 10, 2023 • edited

Choose a reason for hiding this comment

Anton-4 commented Dec 10, 2023

ayazhafiz Dec 10, 2023

Choose a reason for hiding this comment

brian-carroll Dec 11, 2023

Choose a reason for hiding this comment

Anton-4 commented Dec 10, 2023

brian-carroll commented Dec 11, 2023 • edited

brian-carroll commented Dec 11, 2023

brian-carroll commented Dec 11, 2023

brian-carroll commented Dec 17, 2023 • edited

Anton-4 commented Dec 19, 2023

Anton-4 commented Dec 23, 2023

Anton-4 commented Dec 26, 2023 • edited

github-actions bot commented Jan 26, 2024

brian-carroll commented Dec 9, 2023 •

edited

brian-carroll Dec 10, 2023 •

edited

brian-carroll Dec 10, 2023 •

edited

brian-carroll commented Dec 11, 2023 •

edited

brian-carroll commented Dec 17, 2023 •

edited

Anton-4 commented Dec 26, 2023 •

edited