-
Notifications
You must be signed in to change notification settings - Fork 437
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
Header lean.h not conforming C++ #5570
Comments
Thanks for the report!
Is there a concrete porting effort, or other project, that is impeded by this? Or is this mostly a cosmetic and hypothetical issue? |
I guess it's a matter of pride and standards :-) I.e. is there a goal to write portable code, etc. If you don't care, then no, it doesn't matter, but at the same time, it might be a nice thing to do as a matter of sustainability? For practical matters, people might enjoy being able to compile their code with stricter conformance flags, as that allows their compilers to give them more information about their bugs. By requiring a looser conformance mode just to process your header, you're basically denying users the use of those tools. I think that's not a terrible argument; compiler warnings are a pretty useful tool. |
Not just of pride and standards, but also resources and priorities. Is it worth more or less than, say, a bug fix or a better error message? No doubt if we had unlimited resources, we'd happily make the code amazingly elegant. Until then I fear we have to focus on what actually improves our users' experience. |
Yeah... I don't know how to quantify the trade-offs. E.g. I wouldn't know how to measure how interested users were in having stricter compiler warnings; I expect there's some silent acceptence of the status quo. On the other hand, it's also not a terribly large change, and you could maybe invite a PR from the community? |
@tkoeppe Thanks for the detailed report. We are open to considering issues of portability when they impede such efforts but not before that. |
Description
The public import header
lean.h
appears to be intended as a C and C++ interop header (as evidenced by#ifdef __cplusplus
directives). However, it is does not consist of conforming C++ code, since it uses C language features (namely flexible array members (formerly zero-sized array members)) that do not exist in C++. (Common toolchains support these constructions in both languages as vendor extensions.)This might just be the result of some shortcuts. The definition of the types in question may well not be necessary for consuming C++ code. All the helper functions that access them are defined
inline
in the header and only take pointer parameters. This code could conceivably be refactored so that the header merely declares, but does not define the types and functions, and the type and function definitions would be isolated in a single C translation unit. This would make the header contain conforming C++. C code that wishes to take advantage of the definitions for inlining could perhaps include another, intemediate, C-only header.Impact
This is a mild conformance and portability issue. Code that is strictly standard conforming is possibly easier to maintain, evolve, and port to new platforms than code that relies on vendor extensions.
The text was updated successfully, but these errors were encountered: