Canonicalize a type (pointer type) first before checking for constness #1311
Conversation
| @@ -1194,7 +1194,23 @@ impl Type { | |||
| }; | |||
|
|
|||
| let name = if name.is_empty() { None } else { Some(name) }; | |||
| let is_const = ty.is_const(); | |||
There was a problem hiding this comment.
I think this works, but instead the better fix is (I think), changing the following line:
To not check self.is_const(), pointing out that pointer-constness itself can't be represented in rust.
Or in any case that line could be cleaned up.
I think handling it in codegen is better though, since there are a couple other places where we use libclang's is_const in context.rs.
Also, could you test this with constants? This would solve #511. The test-case in that issue's description is pretty good.
There was a problem hiding this comment.
Mhhh. It's somewhat strange, I tried to apply your suggestion but I am getting test failures. It seems that ctx.resolve_type(inner) resolves to an opaque type? I am not sure though, I didn't have much time to debug this (I guess, I'll give it another shot later today).
But this PR should also fix #511 (Well, it seems like it).
There was a problem hiding this comment.
If you get stuck let me know, we can merge this for now and I can take a look at that more carefully. Please do file an issue for that if we leave it as a followup and add the tests for #511 if you can though.
Thank you!
There was a problem hiding this comment.
Yeah, I think that's best for now. I'll create a new issue after this PR is merged.
I've also pushed the test from issue #511.
|
@bors-servo r+ |
|
📌 Commit 1fc8172 has been approved by |
Canonicalize a type (pointer type) first before checking for constness
This fixes cases like:
```cpp
int sodium_munlock(int* const addr); // addr itself is const but the pointer points to mutable data
```
Before fix:
```rust
extern "C" {
pub fn sodium_munlock(
addr: *const ::std::os::raw::c_int,
) -> ::std::os::raw::c_int;
}
```
After this patch:
```rust
extern "C" {
pub fn sodium_munlock(
addr: *mut ::std::os::raw::c_int,
) -> ::std::os::raw::c_int;
}
```
|
☀️ Test successful - status-travis |
|
I filed #1312 with the followup. It required a few fixes indeed. Thanks for working on this! |
ir: Handle *const T at the codegen level. Followup to #1311.
This fixes cases like:
Before fix:
After this patch: