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

Linux fs.h causes compilation problems #376

Open
mark-99 opened this issue Mar 18, 2024 · 4 comments
Open

Linux fs.h causes compilation problems #376

mark-99 opened this issue Mar 18, 2024 · 4 comments

Comments

@mark-99
Copy link

mark-99 commented Mar 18, 2024

/usr/include/linux/fs.h does this:

#define BLOCK_SIZE_BITS 10
#define BLOCK_SIZE (1<<BLOCK_SIZE_BITS)

This definition of BLOCK_SIZE as a preprocessor constant causes subsequent use of concurrentqueue.h or blockingconcurrentqueue.h to not compile correctly, as BLOCK_SIZE is used in moodycamel::ConcurrentQueueDefaultTraits.

This is clearly a Linux / C issue, but it's not something that's likely to be fixable in <linux/fs.h>.

My workaround was #undef BLOCK_SIZE before including concurrentqueue.h. You might consider doing this by default in the header, as clearly a macro of that name is going to cause problems.

Another option would be to #error and at least noisily warn the user of the problem - the compiler errors appear nonsensical and give no real clue what the problem is, so it took me a while to track this down. I wasn't even including <linux/fs.h>, it was being transitively included by liburing.h, which in turn was a dependency for Boost.ASIO's BOOST_ASIO_HAS_IO_URING mode. So while I suspected a #define was to blame, it didn't actually appear in any source code or package dependencies.

vcpkg_installed/x64-linux-cpp20/include/concurrentqueue/concurrentqueue.h:343:29: error: expected unqualified-id before numeric constant
  343 |         static const size_t BLOCK_SIZE = 32;
      |                             ^~~~~~~~~~
vcpkg_installed/x64-linux-cpp20/include/concurrentqueue/concurrentqueue.h:343:29: error: expected ‘)’ before numeric constant
  343 |         static const size_t BLOCK_SIZE = 32;
      |                             ^~~~~~~~~~

I guess the other option would be change the name of the variable you use, but that's a back-compat problem for your own users seeing as it's a customisation point.

@cameron314
Copy link
Owner

IMO silently undefining a system macro would be far more confusing. I could add an #ifdef #error but then the user still wouldn't know where the macro is defined.

@mark-99
Copy link
Author

mark-99 commented Mar 18, 2024

The user's code won't compile either way, but I'd agree probably letting the user decide what to do would be the best bet.

You don't have to be definitive, but pointing towards <linux/fs.h> is certainly the most likely culprit, as any well-behaved 3rd party C headers should be prefixing their macros as has been best practise for decades. I assume it's just basically unfixable in Linux itself.

Indeed any clue that a rogue preprocessor macro is defined would be helpful.

Other option is changing the queue traits var name, which is the best end result, but is a change for existing users who don't currently see any issues.

@cameron314
Copy link
Owner

Looks like I could use push_macro/pop_macro to avoid this for GCC and clang within the header itself. But it's not really improving anything, just hiding it. Also clang's implementation of push_macro appears buggy (warns about the original macro being unused after it's popped back).

@mark-99
Copy link
Author

mark-99 commented Mar 18, 2024

This implies it's present on msvc also: https://gcc.gnu.org/onlinedocs/gcc/Push_002fPop-Macro-Pragmas.html
edit yes here: https://learn.microsoft.com/en-us/cpp/preprocessor/push-macro?view=msvc-170

I guess the warning is because you didn't #define another macro of the same name inside the scope, it doesn't seem to be taking into account there could be other reasons. I guess the warning can also be push/pop disabled.

But yeah... that (if it works) only solves part of the problem, but not if the user has customised ConcurrentQueueDefaultTraits::BLOCK_SIZE.

You could argue an #undef or a similar push_macro/pop_macro in user code is more palatable, and that a #define of a generic symbol can cause all sorts of problem with downstream code in any case. Although it may also make root cause even more obscure, if the identical traits struct in the header compiles just fine.

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