-
Notifications
You must be signed in to change notification settings - Fork 154
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
zfp/types.h header is not C++ safe #86
Comments
Let me make sure I understand the issue better. The C and C++ standards guarantee that any undefined macros are replaced with 0:
(See C90/6.8.1, C99/6.10.1, C++98/16.1.) Thus, if You make a good point about C++11. zfp itself is written in C, but it's of course possible that |
First off, I want to apologize if my first post came over somewhat arrogant or if it sounded like I'm questioning your competence, because that was not my intention (and English is not my native language). You are of course correct about that In any case, we'll just put a pragma around the include of the zfp.h header to disable the warning. |
No need to apologize--I just wanted to better understand the issue and the circumstances that triggers it. I also suspect that g++ emits this warning only in very special cases, such as when
There's little harm in adding |
I just had a discussion with my colleague who has pointed me to this warning in the first place and it seems that we had a misunderstanding regarding what his problem actually was. First off, you are right about that one has to explictly add The problem that my colleague originally wanted to point out was not the warning itself, but rather the fact that a C compiler, which is used to compile the zfp library, uses the So again, sorry for any confusion that our internal misunterstanding might have caused. |
I can see how taking different paths between C and C++ can lead to different types and potential issues. This is something that is explicitly tested for in |
@peter-zajac I've pushed a fix on the |
Sorry for the delayed reply, but we have been quite busy the past few days. I have talked to my colleague who has tested your fix and the g++ compiler has successfully stopped issuing warnings now. Thank you very much for your effort. PS: Do you have an anticipated release date for the version 0.5.6 source tarball? |
I'm hoping we'll roll out 0.5.6 in the next couple of months. I unfortunately do not have a more precise date than that. |
When trying to use the zfp library for a C++ project, we have encountered a warning regarding the use of the
__STDC_VERSION__
macro at the very beginning of thezfp/types.h
header file. The problem is that the g++ compiler does not define this macro, because it considers itself to be a C++ and not a C compiler, as pointed out in the documentation in https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html:According to the C++ standards, it is perfectly legit for a C++ compiler to leave
__STDC_VERSION__
undefined.Therefore, you should at least check whether
__STDC_VERSION__
is defined first. Also, you might consider preceding this by an#if
block that checks for__cplusplus >= 201103L
, which would indicate a C++11 conforming implementation that offers thecstdint
header along with the necessary typedefs, which you could use in analogy to the C99-conforming case.And furthermore, in lines 62 and 67 of
zfp/types.h
, you are using#elif ZFP_LP64
instead of#elif defined(ZFP_LP64)
, which also leads to at least one warning regarding undefined macros.The text was updated successfully, but these errors were encountered: