Support importing C++ _Nonnull pointers as function parameters or return values
#5773
Conversation
…eturn values
We avoid using canonical type before knowing it's not a pointer because we need the nullability attribute.
No support for pointers to pointers, yet.
C++ Interop Demo:
```c++
// hello_world.h
auto hello_world_param(int* _Nonnull i) -> void;
auto hello_world_return() -> int* _Nonnull;
```
```c++
// hello_world.cpp
#include "hello_world.h"
#include <cstdio>
auto hello_world_param(int* _Nonnull i) -> void {
printf("hello_world: %d\n", *i);
}
static int x = 5;
auto hello_world_return() -> int* _Nonnull { return &x; }
```
```carbon
// main.carbon
library "Main";
import Core library "io";
import Cpp library "hello_world.h";
fn Run() -> i32 {
var i: i32 = 10;
Cpp.hello_world_param(&i);
let p: i32* = Cpp.hello_world_return();
Core.Print(*p);
return 0;
}
```
```shell
$ clang -c hello_world.cpp
$ ./bazel-bin/toolchain/install/prefix_root/bin/carbon compile main.carbon
$ ./bazel-bin/toolchain/install/prefix_root/bin/carbon link hello_world.o main.o --output=demo
$ ./demo
hello_world: 10
5
```
Part of carbon-language#5772.
toolchain/check/import_cpp.cpp
Outdated
| .type_id = pointer_type_id}; | ||
| } | ||
|
|
||
| // Maps a C++ type to a Carbon type. |
There was a problem hiding this comment.
Add a comment about this type not being canonicalized before calling MapType and why?
| // CHECK:STDOUT: constants { | ||
| // CHECK:STDOUT: %int_32: Core.IntLiteral = int_value 32 [concrete] | ||
| // CHECK:STDOUT: %i32: type = class_type @Int, @Int(%int_32) [concrete] | ||
| // CHECK:STDOUT: %ptr.235: type = ptr_type %i32 [concrete] | ||
| // CHECK:STDOUT: %foo.type: type = fn_type @foo [concrete] | ||
| // CHECK:STDOUT: %foo: %foo.type = struct_value () [concrete] | ||
| // CHECK:STDOUT: } |
There was a problem hiding this comment.
Yes, I believe so.
Very nice catch!
I think we never create const types regardless of pointers, so I'm not sure whether we need to fix this issue first.
I've added a test for that, so it's explicit and we can follow up on that.
WDYT?
| fn F() { | ||
| //@dump-sem-ir-begin | ||
| var s: const Cpp.S = G(); | ||
| // CHECK:STDERR: fail_todo_import_const_pointer_param.carbon:[[@LINE+5]]:11: error: name `Core.ImplicitAs` implicitly referenced here, but not found [CoreNameNotFound] |
There was a problem hiding this comment.
This error means we should switch the min prelude from destroy.carbon to convert.carbon so we can get the actual conversion error instead of a prelude error
There was a problem hiding this comment.
Done.
FWIW, I did this while debugging this but I assumed that since we should eventually not have an error we shouldn't change includes.
However, I guess we would like to test more use cases here anyways, which might require conversions.
|
|
||
| struct S {}; | ||
|
|
||
| auto foo(const S* _Nonnull) -> void; |
There was a problem hiding this comment.
we could have this return const S* as well, and then try to store it in a const S* in Carbon, to test more things.
There was a problem hiding this comment.
If we want to test multiple use cases I prefer to have separate tests if possible.
Added a test for returning a const pointer.
|
Created #5788 to add |
We avoid using canonical type before knowing it's not a pointer because we need the nullability attribute.
No support for pointers to pointers, yet.
C++ Interop Demo:
Part of #5772.