Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix endianness traits #25

Closed
rupertnash opened this issue Feb 11, 2019 · 3 comments
Closed

Fix endianness traits #25

rupertnash opened this issue Feb 11, 2019 · 3 comments

Comments

@rupertnash
Copy link
Contributor

Hi @SLongshaw

I saw that your reverted the endian traits patch in #14 . I'd like to get this accepted because without it I can't compile MUI on my laptop.

Can you please give me a bug report? Then I have a chance to fix it!

I tested initially with clang (Apple 10.0 / upstream 6) and gcc/6.3.0, but have just tried on Cirrus (http://www.cirrus.ac.uk) with all the GCC versions available and not had problems. (gcc/6.2.0 gcc/6.3.0(default) gcc/7.2.0 gcc/8.2.0)

Cheers

@SLongshaw
Copy link
Member

Apologies, a bug report should have been included before closing.

Including this patch broke an existing implementation of MUI + OpenFOAM, compiled with GCC 7.3.0 on a Debian based system. The compilation error is due to there being multiple definitions of specialisations found in endian_traits.h.

e.g.:

"In function mui::detail::endian_converter<2ul>::htobe()': (.text+0x0): multiple definition of mui::detail::endian_converter<2ul>::htobe()'
(.text+0x0): first defined here
In function mui::detail::endian_converter<2ul>::betoh()': (.text+0x10): multiple definition of mui::detail::endian_converter<2ul>::betoh()'
(.text+0x10): first defined here
In function mui::detail::endian_converter<4ul>::htobe()': (.text+0x20): multiple definition of mui::detail::endian_converter<4ul>::htobe()'
(.text+0x20): first defined here
In function mui::detail::endian_converter<4ul>::betoh()': (.text+0x30): multiple definition of mui::detail::endian_converter<4ul>::betoh()'
(.text+0x30): first defined here
In function mui::detail::endian_converter<8ul>::htobe()': (.text+0x40): multiple definition of mui::detail::endian_converter<8ul>::htobe()'
(.text+0x40): first defined here
In function mui::detail::endian_converter<8ul>::betoh()': (.text+0x50): multiple definition of mui::detail::endian_converter<8ul>::betoh()'
(.text+0x50): first defined here....."

I suspect this is down to the fact that MUI is embedded as a class that is then utilised by a good majority of other OpenFOAM classes, rather than the more traditional use-case of simply including mui.h at the top of a not heavily-OO design. However, as the original code worked without error on the same system, I reverted the changes to avoid it being broken for any other user of the master branch.

More details will be provided in any future bug reports.

@rupertnash
Copy link
Contributor Author

Thanks Stephen! With that clue I reproduced the issue in LAMMPS (using MUI in main() and in a custom fix for the communication ). I'd forgotten that explicit specialisations are not implicitly inline. I've got a fix on my traits branch - I'll make a PR

@SLongshaw
Copy link
Member

I have integrated the fix and all seems well this time, so I'll leave the patch applied unless somebody else comes across a problem. I'm going to close this bug for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants