Skip to content
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

Fix conflict when importing C math library math.h #102

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

germanmarques
Copy link

At the moment the C header math library is imported using quotes. However, if the project already has a math.h file is imported before the math.h C library file, having a conflict. Is it possible to import it using <>?

Thank you!

@XaviTorello
Copy link

Hi @eteq @mhvk any news about this PR?

Many thanks dudes!

@mhvk
Copy link
Contributor

mhvk commented May 8, 2024

Sorry for the delay! This seems uncontroversial except that probably we should try to get it upstream to SOFA, to minimize the difference. Ping @PTW19 to see how we would go about that. In the meantime, I let CI run, which indeed shows that the tests do not care about this.

@germanmarques
Copy link
Author

germanmarques commented May 9, 2024

@mhvk How can we help to try to get it to SOFA? It seems like using <math.h> is the correct way to do it. cc/ @PTW19

@PTW19
Copy link

PTW19 commented May 9, 2024

Can it really be that projects have their own header files with the same names as ones that have been in the C RTL for the past half century?

Having said that, I was startled to find that sofa.h really does include math.h using double quotes just as germanmarques reported. This is surely the wrong thing to do, as math.h is never going to be in the same directory as the program source, so using double quotes adds nothing to what the angle bracket form does. Should I ask the SOFA Board to revisit this issue?

@cxwx
Copy link

cxwx commented May 9, 2024

Indeed, there are cases where libraries define their own custom header files, which can potentially cause conflicts. which is quite an annoying legacy issue. However, the best approach is to avoid using filenames like "math.h" or "time.h" in your own project.
Indeed, some people may attempt to replace the location of "math.h" and libraries to substitute the system libraries. In such cases, using angle brackets < > for include statements might be inconvenient.

@germanmarques
Copy link
Author

Can it really be that projects have their own header files with the same names as ones that have been in the C RTL for the past half century?

Having said that, I was startled to find that sofa.h really does include math.h using double quotes just as germanmarques reported. This is surely the wrong thing to do, as math.h is never going to be in the same directory as the program source, so using double quotes adds nothing to what the angle bracket form does. Should I ask the SOFA Board to revisit this issue?

Yes, it happens :)
https://github.com/CesiumGS/cesium-native/blob/main/CesiumUtility/include/CesiumUtility/Math.h

@mhvk
Copy link
Contributor

mhvk commented May 11, 2024

@PTW19 - it would seem to be that one might as well use the <> to indicate one expects to use the standard library. So, if you can ask the SOFA board, that would be great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants