don't emit #[repr(align(0))], and reduced test case

[pmarks]

fixes #1565
not sure if this is the right fix, but the test case should be useful.

[emilio]

Oh, so it's a forward declaration, fun... Let me think a bit whether it's the right fix. Thank you so much for the test-case!

[emilio]

Sorry for the lag, got sidetracked with something else :)

Ok, so this is not the right fix. The right fix is returning None in here if we're a forward declaration (you can check self.is_forward_declaration() right there).

Probably with a comment like:

// By definition, we don't have the right layout information here if
// we're a forward declaration.

Or something of that sort. That should make it behave the same way as opaque forward-declared structs in this case.

It'd be worth adding a different test (not to the repr_align one) testing the behavior here.

The header could be as simple as:

// bindgen-flags: --opaque-type "*"

union a;
struct b;

Would you be able to make those changes? Otherwise I'm happy to do that myself and crediting you appropriately for finding the test-case.

Thanks so much again!

[pmarks]

@emilio thanks for your help! The PR now fixes the original problem, but introduces the following test failure, which unfortunately is way over my head. LMK if you have ideas on how to proceed.

---- header_objc_template_h stdout ----



diff expected generated
--- expected: "/Users/patrick/code/rust-bindgen/tests/expectations/tests/libclang-5/objc_template.rs"
+++ generated from: "/Users/patrick/code/rust-bindgen/tests/headers/objc_template.h"
 /* automatically generated by rust-bindgen */

 #![allow(
     dead_code,
     non_snake_case,
     non_camel_case_types,
     non_upper_case_globals
 )]
 #![cfg(target_os = "macos")]

 #[macro_use]
 extern crate objc;
 #[allow(non_camel_case_types)]
 pub type id = *mut objc::runtime::Object;
 pub trait Foo {
-    unsafe fn get(self) -> *mut ObjectType;
+    unsafe fn get(self) -> u64;
 }
 impl Foo for id {
-    unsafe fn get(self) -> *mut ObjectType {
+    unsafe fn get(self) -> u64 {
         msg_send!(self, get)
     }
 }

[emilio]

Yeah, those objective-c tests are known-problematic in newer clang versions.

You can skip those changes with git add -p and CI would be happy, sorry for the hassle.

I need to find a bit of time to get all the relevant clang versions we support on CI to run on my machine and spin off another subdirectory with version-specific tests for this. I had been procrastinating on it for a while now :)

Anyhow, just run cargo test with the relevant environment variable, then git add -p everything but the objective-C tests. You can git commit, then git reset --hard HEAD to undo the objective-c changes. Sorry again for the hassle :(

[emilio]

nit: remove this addition too?

[emilio]

Oh, and fix looks great now, thanks! r=me with the non objective-c expectations updated :)

[emilio]

And to be clear, that failure should happen with on without your fix, I'm pretty sure.

[pmarks]

@emilio apologies: I didn't recheck the original test case. It's still broken. I'm re-reducing w/ the first fix in place.

[pmarks]

Here's the new test case:

template <int> class a {
  union {};
};

[emilio]

Ok, so this looks good to merge as-is. I'll suggest a fix for the second test-case as well. But TL;DR: it should be in the same function. Empty unions don't have zero alignment, but we're coming up with that in that test-case.