solettaproject / soletta

Soletta Project is a framework for making IoT devices. With Soletta Project's libraries developers can easily write software for devices that control actuators/sensors and communicate using standard technologies. It enables adding smartness even on the smallest edge devices.
http://solettaproject.org
Apache License 2.0
226 stars 109 forks source link

sol-pin-mux-impl.h is not C++-friendly #147

Closed thiagomacieira closed 9 years ago

thiagomacieira commented 9 years ago

The definition:

#define SOL_PIN_MUX_DECLARE(_NAME, decl ...) \
    SOL_API const struct sol_pin_mux SOL_PIN_MUX = { .api_version = SOL_PIN_MUX_API_VERSION, decl }

Will never work in C++ since designated initialisers are not supported.

Please clarify the situation by either:

barbieri commented 9 years ago

Do you know if it is an option to use this macro within extern "C" {} (not declaration, rather when expanding the macro)?

thiagomacieira commented 9 years ago

It isn't. The preprocessor is a search-and-replace, so the extern "C" does not apply to macros. In any case, extern "C" does not change the language, it only declares that a given function is to be mangled as a C function.

You have to drop the ".api_version =" part or you need to ban C++ from using the macro.

barbieri commented 9 years ago

Hum, for pinmux this is okay as it's basically a table declared in binary form. But for things like node types, it's an issue.

OTOH if we don't use named initializers we're fragile on structure changes... (that would break the ABI anyway) @lucasdemarchi is the one that hates relying on the order and is strict about using named initializers.

thiagomacieira commented 9 years ago

Like you said, if you break ABI by changing the order in the struct, you'll need to recompile anyway. I don't see why the macro couldn't be updated too.

mbelluzzo commented 9 years ago

"Who likes C++ anyway" :-)

lucasdemarchi commented 9 years ago

Not really, designated initializers work on both g++ and clang++. On GCC AFAIR it's since 4.8.3, but I'd need to check.

If you want compatibility we could even play silly and use the older extension:

struct bla bla = { api_version : 1, ... }

For headers we may want to be as compatible as possible so if it's not compatible with _sane_ compilers we could open exceptions for things that are on exported headers.

For every other initialization use designated initializers because it's terrible to maintain things like struct bla bla = { 1, 2, 3, true, NULL, 5 }; (and that's not only when you are reordering the members).

thiagomacieira commented 9 years ago

I understand the benefit of designated initialisers, you don't have to tell me. The point is that they're an extension to the language in C++. I've asked and the committee has no intention of standardising it, ever.

Therefore, even sane compilers do not need to support the feature. GCC and Clang support it as an extension. Both print a warning with -Wpedantic / -Wc99-extensions.

The question is: do you want to rely on on a non-standard extension? You can make that choice. But please document it as a conscious choice.

Note: there are other C99 features that C++ has not and will not standardise.

lucasdemarchi commented 9 years ago

As I said, for internal code it is -std=gnu11 that we support that compiles fine with both clang and gcc. For exported headers we may open exceptions... Either using an older extension or not using at all. @barbieri opinions? Having macros like this in exported headers is always troublesome :(

lucasdemarchi commented 9 years ago

I 've asked and the committee has no intention of standardising it, ever.

Did they give a reason other than let the language suck?

ulissesf commented 9 years ago

Let's make them positional for external headers? And again we come to the need of separating the internal and exported headers. Now it's a good time with all the kbuild-related work.

lucasdemarchi commented 9 years ago

Agreed... Making it clear what are the exported headers would be good, too

thiagomacieira commented 9 years ago

I've asked and the committee has no intention of standardising it, ever.

Did they give a reason other than let the language suck?

I don't remember the details, but it was about how designated initialisers conflict with constructors, base classes and with non-POD members. That is, it would be a feature with limited use in C++ for anything but POD structs (POD = whatever can exist in C). Instead, the committee seems to be reserving the syntax for future expansion, such as named arguments in function calls.

As for the other C99 features that C++ has rejected, they're usually because those are misfeatures and have superior solutions in C++. For example, the "x[static 8]" argument in the function parameter and use of sizeof in VLAs can be replaced by any number of array classes. The "atomic" and "complex" specifiers are replaced by template classes.

lucasdemarchi commented 9 years ago

Instead, the committee seems to be reserving the syntax for future expansion, such as named arguments in function calls.

then they go and create the horrible A { 3 }; to initialize classes/PODs.

As for the other C99 features that C++ has rejected, they're usually because those are misfeatures and have superior solutions in C++

Superior here is in the eyes of the beholder. Then they (it's really they, not you) go and complain on C projects that we can't use feature x, y or z because it's not compatible with C++. I'd very much strive for C-compatibility when it's easy to add like designated initializers and others.

thiagomacieira commented 9 years ago

Then they (it's really they, not you) go and complain on C projects that we can't use feature x, y or z because it's not compatible with C++.

You're making the choice of supporting C++ in your header (as evidenced by the #ifdef __cplusplus \n extern "C" {). You could just as well say "no, C++ is not allowed here, please use a C99 compiler".

Unfortunately, the subset of C and C++ is not C. Even as early as C89 and C++98, there were certain features that differed. See http://www.stroustrup.com/bs_faq.html#C-is-subset

barbieri commented 9 years ago

so let's have positional arguments for installed headers (now easier with bdilly's patch to move them all to src/lib/include)