Skip to content

C++ interop: Add support for C++ Primitive Types #5263

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.

Sign up for GitHub

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

Jump to bottom
Open
ivanaivanovska opened this issue Apr 8, 2025 · 7 comments
Open

C++ interop: Add support for C++ Primitive Types #5263

ivanaivanovska opened this issue Apr 8, 2025 · 7 comments
Assignees
@ivanaivanovska

Comments

@ivanaivanovska
Copy link
Contributor

ivanaivanovska commented Apr 8, 2025 •
edited
Loading

Use Carbon: C++ Interop - Primitive Types doc as a reference for the type mappings.

  • Signed integer types
Carbon type C++ type Status PRs
i8 signed char / int8_t TODO
i16 short / int16_t DONE #5393
i32 int / int32_t DONE #5197; #5392
Core.Cpp.long long TODO
Core.Cpp.long_long long long TODO
i64 int64_t TODO
  • Unsigned integer types
Carbon type C++ type Status
u8 unsigned char / uint8_t TODO
u16 unsigned short / uint16_t TODO
u32 unsigned int / uint32_t TODO
Core.Cpp.unsigned_long unsigned long TODO
Core.Cpp.unsigned_long_long unsigned long long TODO
u64 uint64_t TODO
  • Floating-point types
Carbon type C++ type Status
f32 float TODO
f64 double TODO
TBD  long double TODO


@jonmeow
Copy link
Contributor

jonmeow commented Apr 8, 2025

Note, for now maybe just i8/u8 for char types. We don't have a byte type yet, and we may want a char type.

For long, I'll also suggest forcing i64/u64 for now. We don't have Windows testing yet, but we need a clearer decision about how to handle cross-platform. It's possible we'll actually want a platform-dependent type for long ("Alternative: Provide variable size types" in the doc), and instead encourage int64_t for platform-independent use.

@jonmeow
Copy link
Contributor

jonmeow commented Apr 8, 2025

For integer types, discussion with @chandlerc and @zygoloid seems to have leaned (for now, at least):

  • i8 == int8_t, which should be the same as signed char
    • And so on for i16, int16_t/short, and i32/int32_t/int
  • i64 == int64_t, which should be either long or long long, platform-dependent
  • There will be some Carbon name for both long and long long, although that name isn't clear (e.g., Core.Cpp.long and Core.Cpp.long_long, Core.Cpp.unsigned_long_long as a possible choice).
    • One of these will be a platform-dependent alias to i64. The other will be either 32-bit or 64-bit, and use an appropriate int type that is equivalent (but different) from i32/i64.
  • Corresponding for unsigned types

When transforming Carbon types to C++, always use this mapping. When transforming C++ types to Carbon, fit into these buckets as much as possible.

@zygoloid
Copy link
Contributor

zygoloid commented Apr 9, 2025

Had some further discussion with @chandlerc about this, largely reaffirming the approach that @jonmeow described previously. Some new observations:

  • We should make sure that the platform-dependent type that is not an alias for iN is nonetheless compatible with the corresponding iN type, for example by defining the non-alias type as an extending adapter of the alias type. In particular, this would allow us to type-pun between them and for example cast from Core.Cpp.long_long* to i64* even when long_long is not an alias for i64.
  • Once we reach the point of using C++ overload resolution for function calls in interop, we can consider allowing forming an implicit conversion sequence from (eg) one of the two 64-bit integer types to the other one.
  • We will probably want to add a new type to Carbon at some point that is a machine-word-sized integer type (Core.Int(N) where N is pointer-width). We should check whether that type will always be the same type as ptrdiff_t / ssize_t under this model. We suspect it will, but there might be surprises lurking here.

@ivanaivanovska
Copy link
Contributor Author

Thanks a lot for your input! @jonmeow @zygoloid
I started drafting a new design doc reflecting on these discussions in: Carbon: C++ Interop - Primitive Types. It’s still a WIP, but any early feedback is welcome. Thanks.

@rahiladmin
Copy link

Hey, I would like to work on this issue.

@ivanaivanovska ivanaivanovska mentioned this issue Apr 30, 2025
Merged
@ivanaivanovska ivanaivanovska mentioned this issue May 8, 2025
Open
github-merge-queue bot pushed a commit that referenced this issue May 9, 2025
@ivanaivanovska
[Carbon/C++ interop] Add support for C++ type short (#5393)
f8443ae 
Added support for `short`/`int16_t`. Both function parameters and return
values this type will be supported.

Demo:

```c++
// hello_short.h

#include <cstdint>
auto foo_short(int16_t a) -> int16_t;
```

```c++
// hello_short.cpp

#include "hello_short.h"
#include <cstdio>

auto foo_short(int16_t a) -> int16_t {
    printf("a = %i \n", a);
    return a;
}
```

```c++
// main.carbon

library "Main";

import Cpp library "hello_short.h";
import Core library "io";

fn Run() -> i32 {
  var a: i16 = 3;
  Cpp.foo_short(a);
  return 0;
}
```

```
$ clang -c hello_short.cpp
$ bazel-bin/toolchain/carbon compile main.carbon
$ bazel-bin/toolchain/carbon link hello_short.o main.o --output=demo
$ ./demo
a = 3 
```

Part of #5263
@bricknerb bricknerb mentioned this issue May 12, 2025
Merged
bricknerb added a commit to bricknerb/carbon-lang that referenced this issue May 14, 2025
@bricknerb
Refactor toolchain/check/testdata/interop/cpp/function_decl.carbon
eb0e1e8 
* Split to 4 files: `function_param_int16`, `function_param_int32`, `function_param_unsupported.carbon`, `function_return`.
* Add // === headlines on group of file shards.
* Rename shard files to shorten them and make them more consistent given the file context.
* Deduplicate identical .h files and group their tests.

Potential future improvements:
* Split further. For example, pointers and references might be somewhat separate from int primitives.
* Remove `import_` shard file prefix, as it repeats itself, but leaving for now as it makes it more explicit.

Part of carbon-language#5263
bricknerb added a commit to bricknerb/carbon-lang that referenced this issue May 14, 2025
@bricknerb
Refactor toolchain/check/testdata/interop/cpp/function_decl.carbon
5f81f0d 
* Split to 4 files: `function_param_int16`, `function_param_int32`, `function_param_unsupported.carbon`, `function_return`.
* Add // === headlines on group of file shards.
* Rename shard files to shorten them and make them more consistent given the file context.
* Deduplicate identical .h files and group their tests.

Potential future improvements:
* Split further. For example, pointers and references might be somewhat separate from int primitives.
* Remove `import_` shard file prefix, as it repeats itself, but leaving for now as it makes it more explicit.

Part of carbon-language#5263
github-merge-queue bot pushed a commit that referenced this issue May 14, 2025
@bricknerb
Refactor toolchain/check/testdata/interop/cpp/function_decl.carbon (#…
2b48033 
…5457)

* Split to 4 files: `function_param_int16`, `function_param_int32`,
`function_param_unsupported.carbon`, `function_return`.
* Add // === headlines on group of file shards.
* Rename shard files to shorten them and make them more consistent given
the file context.
* Deduplicate identical .h files and group their tests.

Potential future improvements:
* Split further. For example, pointers and references might be somewhat
separate from int primitives.
* Remove `import_` shard file prefix, as it repeats itself, but leaving
for now as it makes it more explicit.

Part of #5263
@ivanaivanovska
Copy link
Contributor Author

The primitive types mapping proposal (#5448) is now open for review.

@ivanaivanovska
Copy link
Contributor Author

Hey, I would like to work on this issue.

Thanks @rahiladmin for your interest in participating to this issue.

While the proposal (#5448) is still in review, the mapping of signed char/int8_t -> i8 can be implemented next and you could take that if you’re still interested. You could use as an example the implementation for short / int16_t -> i16 (PR #5393). If you work on this, please add myself and @bricknerb as reviewers and notify us of your work. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ivanaivanovska ivanaivanovska

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@zygoloid @ivanaivanovska @jonmeow @rahiladmin