search

mystor / rust-cpp

Embed C++ directly inside your rust code!
Apache License 2.0
795 stars 44 forks source link

Add support for parsing simple #[cfg(feature = "feature")] to allow conditional module inclusion #63

Closed twistedfall closed 5 years ago

twistedfall commented 5 years ago

Only simple straight-forward #[cfg]'s with only "feature" are supported, no complex conditions are allowed

ogoffart commented 5 years ago

This is quite specific as it only works for modules and for the feature expression.

Are you having a specific problem that cpp_build fails because of an unexisting module?

Maybe we could have some attribute so that cpp_build ignore some part of the code?

Supporting #[cfg ...] everywhere could also be an option, but one probably should develop some advanced config parsing. I believe some other crates might be doing that like cbindgen and the like.

twistedfall commented 5 years ago

True. I've thought about implementing it for any #[cfg], but I don't have that much free time atm. In my case I have 3 similar module trees aimed at 3 different major version of the ffi'ed library (opencv). And only one of those trees is actually imported based on #[cfg(feature)]. When all of them are parsed by rust-cpp I'm getting function redefinition errors because a lot of function are the same across those module trees.

Maybe we could have some attribute so that cpp_build ignore some part of the code?

Well, that's basically what #[cfg]'s are :)

ogoffart commented 5 years ago

The function redefinition is actually another problem caused by the hashes being equal, this also happens when they are in the same file. I guess this could be solved by generating only one function per hash.

Regarding your change: I guess it make sens to integrate it if it solve your needs. Have you thought about documenting this feature? That only simple variant of cfg are understood.

twistedfall commented 5 years ago

The function redefinition is actually another problem caused by the hashes being equal

Yeah, I've run into that too, solved by just renaming the captured variables.

Have you thought about documenting this feature?

Let me do that, I'm going to put it in the cpp/src/lib.rs module level doc comment, that what you expect, right?

ogoffart commented 5 years ago

right, the module level doc seems like the best place

ogoffart commented 5 years ago

Or maybe it should be in the cpp_build documentation.

twistedfall commented 5 years ago

Well, I've checked that first, but cpp_build module documentation refers a reader to cpp doc

twistedfall commented 5 years ago

I've update the module docs for cpp module

ogoffart commented 5 years ago

Thanks for the patch. The documentation looked good.