When only generating functions, type parameter is lost

[fitzgen]

We still have more issues like #835...

Input C/C++ Header

• TODO

Bindgen Invocation

• TODO

Actual Results

https://treeherder.mozilla.org/logviewer.html#?job_id=117196247&repo=try&lineNumber=6725

error[E0243]: wrong number of type arguments: expected 1, found 0
   --> /home/worker/workspace/build/src/obj-firefox/toolkit/library/x86_64-unknown-linux-gnu/release/build/style-077f5b469d68e8b7/out/gecko/bindings.rs:526:14
    |
526 |      -> *mut nsTArray;
    |              ^^^^^^^^ expected 1 type argument
error[E0243]: wrong number of type arguments: expected 1, found 0
   --> /home/worker/workspace/build/src/obj-firefox/toolkit/library/x86_64-unknown-linux-gnu/release/build/style-077f5b469d68e8b7/out/gecko/bindings.rs:529:65
    |
529 |     pub fn Gecko_DestroyAnonymousContentList(anon_content: *mut nsTArray);
    |                                                                 ^^^^^^^^ expected 1 type argument
error[E0243]: wrong number of type arguments: expected 1, found 0
    --> /home/worker/workspace/build/src/obj-firefox/toolkit/library/x86_64-unknown-linux-gnu/release/build/style-077f5b469d68e8b7/out/gecko/bindings.rs:1073:53
     |
1073 |     pub fn Gecko_ResizeTArrayForStrings(array: *mut nsTArray, length: u32);
     |                                                     ^^^^^^^^ expected 1 type argument
error: aborting due to 3 previous errors
error: Could not compile `style`.

Expected Results

nsTArray should have a type argument.

[fitzgen]

cc @emilio @Manishearth

[fitzgen]

This is really hard to reproduce. I've preprocessed all of nsTArray.h and added nsTArray_Simple and Gecko_ResizeTArrayForStrings to the bottom and it doesn't repro.

Trying reproducing with the full servo bindings next.

[fitzgen]

Blech I can't even reproduce with the full preprocessed servo bindings.

[upsuper]

OK, I eventually figured out a real way to reproduce it...

[upsuper]

And this is reproducible on Windows as well. I have no idea why it didn't cause any issue on other platforms when with Gecko...

[upsuper]

You need two files, in different directory. Let's say input.hpp and subdir/another.h, they have code:

// input.hpp
#include "another.h"

// subdir/another.h
template<typename T>
class nsTArray;

/**
 * <div rustbindgen replaces="nsTArray"></div>
 */
template<typename T>
class nsTArray_Simple {
  T* m;
};

extern "C" {
nsTArray<int>* func();
}

Then invoke bindgen with:

bindgen input.hpp -- -I subdir

[upsuper]

If you move all the code from subdir/another.h into input.hpp directly, then this bug wouldn't appear, that is why a fully preprocessed file doesn't trigger this bug.

[upsuper]

What seem to be necessary for triggering this bug are:

• The other header file need to be searched from a directory specified by -I. If you use "subdir/another.h" directly in input.hpp, it doesn't happen.
• The replacing type (nsTArray_Simple here) must be in the header file mentioned above.

What doesn't seem to be related:

• Where are other things, including the replaced type (nsTArray) and the function. They can be in either file and the result wouldn't change.
• Whether the replaced type is a definition or a declaration.
• The order of things, as far as nsTArray is declared before the function.

[emilio]

Thanks a lot for the investigation an the test-case Xidorn, trying to repro now.

[emilio]

So, I can repro, but the generated code is:

/* automatically generated by rust-bindgen */

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct nsTArray {
    pub mBuffer: *mut ::std::os::raw::c_void,
}
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct nsTArray_Simple<T> {
    pub m: *mut T,
    pub _phantom_0: ::std::marker::PhantomData<::std::cell::UnsafeCell<T>>,
}
extern "C" {
    pub fn func() -> *mut nsTArray;
}

Which means that we're not managing to see the replacement. Does that match what you see @upsuper?

[emilio]

I think I know why this is...

[emilio]

So, we don't even find the replacement annotation, which is surprising, at a glance. I'm trying to bisect when this broke.

[emilio]

I can confirm that v0.26.1 works, so far.

[emilio]

Well, this is fun:

$ g bisect bad                                                                                                                                                                             
ee478a123eec1141e7555ec84eff8a10723d0e00 is the first bad commit
commit ee478a123eec1141e7555ec84eff8a10723d0e00
Author: Shing Lyu <[email protected]>
Date:   Sat Jul 1 01:38:01 2017 +0800

    Passing additional clang arguments for Linux 32 cross compiling

[emilio]

I can confirm that reverting that patch fixes the issue for me... Now, I'm going to see if this is a regression in clang-sys itself (that patch pulled a clang-sys upgrade), or on the options we pass.

[emilio]

So, I can confirm that passing empty flags to the new clang_sys also fixes this, so I bet what is happening is that clang-sys is returning all the paths, not only the system ones, and we're adding them as -isystem when there's no --target argument (which explains why this works on OSX and windows, cc @Manishearth).

I bet clang does different stuff when parsing system headers which makes us not recognise annotations.

[emilio]

So clang-4.0 gives me, as expected:

$ clang -E -x c++ - -v -Iinclude                                                                                                                                                           
clang version 4.0.0 (tags/RELEASE_400/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-redhat-linux/7
Found candidate GCC installation: /usr/lib/gcc/x86_64-redhat-linux/7
Selected GCC installation: /usr/bin/../lib/gcc/x86_64-redhat-linux/7
Candidate multilib: .;@m64
Candidate multilib: 32;@m32
Selected multilib: .;@m64
 "/usr/bin/clang-4.0" -cc1 -triple x86_64-unknown-linux-gnu -E -disable-free -disable-llvm-verifier -discard-value-names -main-file-name - -mrelocation-model static -mthread-model posix -mdisable-fp-elim -fmath-errno -masm-verbose -mconstructor-aliases -munwind-tables -fuse-init-array -target-cpu x86-64 -v -dwarf-column-info -debugger-tuning=gdb -resource-dir /usr/bin/../lib64/clang/4.0.0 -I include -internal-isystem /usr/bin/../lib/gcc/x86_64-redhat-linux/7/../../../../include/c++/7 -internal-isystem /usr/bin/../lib/gcc/x86_64-redhat-linux/7/../../../../include/c++/7/x86_64-redhat-linux -internal-isystem /usr/bin/../lib/gcc/x86_64-redhat-linux/7/../../../../include/c++/7/backward -internal-isystem /usr/local/include -internal-isystem /usr/bin/../lib64/clang/4.0.0/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -fdeprecated-macro -fdebug-compilation-dir /home/emilio/projects/moz/rust-bindgen -ferror-limit 19 -fmessage-length 190 -fobjc-runtime=gcc -fcxx-exceptions -fexceptions -fdiagnostics-show-option -fcolor-diagnostics -o - -x c++ -
clang -cc1 version 4.0.0 based upon LLVM 4.0.0 default target x86_64-unknown-linux-gnu
ignoring nonexistent directory "/include"
#include "..." search starts here:
#include <...> search starts here:
 include
 /usr/bin/../lib/gcc/x86_64-redhat-linux/7/../../../../include/c++/7
 /usr/bin/../lib/gcc/x86_64-redhat-linux/7/../../../../include/c++/7/x86_64-redhat-linux
 /usr/bin/../lib/gcc/x86_64-redhat-linux/7/../../../../include/c++/7/backward
 /usr/local/include
 /usr/bin/../lib64/clang/4.0.0/include
 /usr/include
End of search list.

Which means that we're incorrectly promoting -Iinclude to -isystem include.

[emilio]

I've investigated more, and the key is that clang doesn't try to parse documentation comments in system includes.

[emilio]

Got a fix for this... Maybe we want to rethink all the path fixup, but it is needed so far to cross-compile so...