search

ericniebler / range-v3

Range library for C++14/17/20, basis for C++20's std::ranges
Other
4.06k stars 437 forks source link

The `Nil` struct conflicts with Objective-C #define abominations #1738

Open rnikander opened 1 year ago

rnikander commented 1 year ago

This isn't really a bug in this library, but maybe it's worth doing something about it. I'm trying to use Apple's "Metal Cpp" headers, which they provide for using C++ and Metal (their low level GPU API). Those headers transitively include objc.h, which uses the preprocessor to do this...

#define Nil nullptr

Note, it wasn't enough to define nil, the had to do the capitalized symbol too, even though I've never seen it in Objective-C code.

Anyway, it leads to this:

In file included from /..../include/range/v3/view/any_view.hpp:22:
In file included from /..../include/range/v3/range_fwd.hpp:22:
/..../include/concepts/concepts.hpp:780:9: error: declaration of anonymous struct must be a definition
        struct Nil
        ^
/..../include/concepts/concepts.hpp:805:22: error: expected a type
        using xNil = Nil;
                     ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk/usr/include/objc/objc.h:98:16: note: expanded from macro 'Nil'

I'm not sure what the right way to handle stuff like this is. I worked around it for now my adjusting the order of my #include lines so that range-v3 gets included before Apple's Metal stuff.

I'm kind of kind of new to C++, and without that last error message line - pointing to the macro buried 5 levels deep in #includes, I might have stared at this for days and simply gone insane. I wonder: is this the kind of thing that C++20 modules would solve?

brevzin commented 1 year ago

Gross.

is this the kind of thing that C++20 modules would solve?

Yep, since if you import range.v3; (no hyphens unfortunately?), that wouldn't consume any externally defined macros, so there wouldn't be any conflict here.

But until then, Nil is purely an implementation detail, that is only used in that one header file, and it's actual name isn't important at all. So if you want to open a PR to just rename it to something that doesn't conflict with the macro, we can pretty easily (if annoyingly) just do that.

Obviously the right name here is KnightsWhoSayNil (or TypeFormerlyKnownAsNil?), or perhaps @ericniebler can provide a better one. Since it's only used in this one file, we at least don't have to worry about having a nice, user-facing name. Just something good enough for us to understand.

rnikander commented 1 year ago

Funny, I was also thinking about "Ni" when I wrote the initial post.

I'm trying to update my compiler today to the latest clang. I don't know how much, if any, of modules are working yet. I'll tinker with that first, and maybe submit the PR if it looks like module use is still too far off.

rnikander commented 1 year ago

Here's a pull request to fix it. I tried to rename the Nils. It works for me but I haven't tested a lot.

https://github.com/ericniebler/range-v3/pull/1775