Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start implementations of <algorithm>, <array>, <iterator>, <utility> #95

Merged
merged 2 commits into from
Dec 21, 2022

Conversation

pfusik
Copy link
Contributor

@pfusik pfusik commented Dec 18, 2022

This reduces the list of errors in #52 to:

C:\0\a8\llvm>"install\bin\mos-c64-clang++.bat" -Os -Wall pong.cpp
pong.cpp:260:19: error: no member named 'make_tuple' in namespace 'std'
      return std::make_tuple(
             ~~~~~^
pong.cpp:295:31: error: no matching constructor for initialization of 'const std::array<Color, 16>'
  const std::array<Color, 16> colors = {{
                              ^        ~~
C:/0/a8/llvm/install/bin/../mos-platform/common/include/array:9:8: note: candidate constructor
      (the implicit copy constructor) not viable: cannot convert initializer list argument to 'const
      std::array<(anonymous namespace)::Color, 16>'
struct array {
       ^
C:/0/a8/llvm/install/bin/../mos-platform/common/include/array:9:8: note: candidate constructor
      (the implicit move constructor) not viable: cannot convert initializer list argument to 'std::array<(anonymous
      namespace)::Color, 16>'
C:/0/a8/llvm/install/bin/../mos-platform/common/include/array:9:8: note: candidate constructor
      (the implicit default constructor) not viable: requires 0 arguments, but 1 was provided
2 errors generated.

To be continued.

@ooxi
Copy link

ooxi commented Dec 19, 2022

What's the reason for not using libc++? At least the freestanding parts should work without modification, shouldn't they?

@mysterymath
Copy link
Member

Do be aware that I'm planning to port libc++ itself at some point, and I'll end up replacing many of these functions with those

I'm definitely not going to block any work along these lines though; who knows what I'll actually get around to, and a bird in the hand is worth two in the bush. It's also very likely that hand-written 6502-friendly versions of various functions will be superior to those in libc++; in those cases; I'll try to port any of the work over as an #ifdef inside our fork of libc++.

@@ -0,0 +1,57 @@
#pragma once
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Include guards rather than pragma once; here and throughout.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly straight conservativism; the majority of C++ style guides disallow it. In particular, it's not allowed in the LLVM style guide, which we've been taking as a baseline (although I've been nowhere near as picky about naming etc.) It's also generally desirable to use standardized mechanisms over non-standard ones, unless there's a very compelling reason.

@pfusik
Copy link
Contributor Author

pfusik commented Dec 20, 2022

What's the reason for not using libc++? At least the freestanding parts should work without modification, shouldn't they?

I wanted to check what it takes to implement the library looking at the C++ standard. Also, I guess that writing just the parts needed for pong.cpp (#52) is less work than enabling libc++.

@mysterymath mysterymath merged commit beb2b9a into llvm-mos:main Dec 21, 2022
asiekierka pushed a commit to asiekierka/llvm-mos-sdk that referenced this pull request Jun 29, 2023
…lvm-mos#95)

* Add partial implementations of <algorithm>, <array>, <iterator>, <utility>

See llvm-mos#52.

* Avoid #pragma once.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants