File common/include/byteswap.h clashes with built-in toolchain definition files

[JohnnyonFlame]

Proper solution would be renaming the aforementioned file.
Here's a partial failed scons log for reference.

http://pastebin.com/ZWKabef4

[Matt1360]

What is your build environment?

[JohnnyonFlame]

It's an uClibc based toolchain for mips32r2 built using buildroot, used for building binaries for the GCW-Zero console. You can find the toolchain and more info here:

http://www.gcw-zero.com/develop

[Matt1360]

Thanks for the info, I'll check into it quick. The compiler and paths threw me off, I haven't seen this before. Can you try to compile the following and paste me any output (if failure)?

#include <stdint.h>

static uint32_t some_func( const uint32_t& ) { /* No-op */ }
static int32_t some_func( const int32_t& ) { /* No-op */ }

int main()
{
  return 0;
}

You should be able to compile it with mipsel-linux-g++ test.cpp or whatever you save it as. This seems to be an issue with the toolchain, as the code within is valid, although an implicit cast to uint32_t and uint16_t is probably sufficient, not requiring the need for the signed variants - but I'm not sure where, nor how extensively used this code is.

[JohnnyonFlame]

Build works as intended, it's definately not a problem in the toolchain. Compiling the header as a separate thing works too.

[Matt1360]

This is valid by the C++ spec. I'll let @vLKp weigh in on this, though.

[mthuurne]

There is a system header named "/usr/include/byteswap.h" in both glibc and uClibc, which "/usr/include/endian.h" tries to include, but it gets "common/include/byteswap.h" instead because it comes first in the include search path. I don't know if this causes the problem Johnny reported, but it would be a good precaution in any case to rename "common/include/byteswap.h" to avoid this name clash. Intercepting system header includes can only lead to trouble.

I agree the code is valid. It also builds fine with the GCW Zero toolchain if built in isolation, so there seems to be a strange interaction between the toolchain and the DXX build that breaks things. I'm one of the maintainers of the GCW Zero toolchain, by the way.

[Matt1360]

This is strange - as it builds on my systems fine. Perhaps there is some inclusion ordering issue. I will review it shortly, and possibly rename byteswap.h to byteutil.h or similar.

[Matt1360]

@JohnnyonFlame Try it now, let me know if it works.

Also, forgive that few minutes where I broke everything because I forgot what branch I was on and merged. Whoops. 👍

[JohnnyonFlame]

No biggie. The commit fixes the problem, thanks for your time.

[vLKp]

This was never intended to intercept system headers, and doing so is work fixing. However, I think the originally reported failure shows a problem in the endian header: the Rebirth header works fine when compiled as C++, but not when compiled as C. Though the compiler was invoked in C++ mode, it appears that endian.h included byteswap.h while inside an extern "C" block, thereby switching to C-style parsing. Generally, you should not have a language mode override open when you include other headers. The included headers may only work with the default parse language.

I can reproduce the reported error if I wrap byteswap.h inside an extern "C" block. Due to limited interaction with the preprocessor, wrapping the header in _#ifdef _cplusplus does not help, since the preprocessor definitions remain set to C++. The only ways to get the right results are either to wrap every C++ header in an explicit extern "C++" to reset the language mode or to assume the caller left the language mode on a sane default.

[mthuurne]

Ah, the error message indeed says "declaration of C function". That explains it: in C there is no name mangling of symbols to support overloading. Thanks for solving the mystery :)

The extern "C" is inserted by the __BEGIN_DECLS macro, defined in /usr/include/sys/cdefs.h and used in /usr/include/sys/types.h, before it includes <bits/types.h>. So it seems that glibc does not follow the convention of wrapping each individual header in extern "C". I don't know if that's a bug or whether they have a different convention to ensure that functions use the right linking conventions. In normal circumstances a system header would never include user code and by the time the preprocessor is done with the system header include the extern "C" block has been properly closed.