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

Clang Build fails with -Wshorten-64-to-32 or -Wconversion #7813

Closed
paul-elliott-arm opened this issue Jun 21, 2023 · 2 comments · Fixed by #8817
Closed

Clang Build fails with -Wshorten-64-to-32 or -Wconversion #7813

paul-elliott-arm opened this issue Jun 21, 2023 · 2 comments · Fixed by #8817
Labels
bug priority-medium Medium priority - this can be reviewed as time permits size-m Estimated task size: medium (~1w)

Comments

@paul-elliott-arm
Copy link
Member

paul-elliott-arm commented Jun 21, 2023

Summary

As reported in #7787 we are not currently defining -Wshorten-64-to-32 for clang builds. I tried turning this on, and unfortunately got lots of warnings (mostly size_t being forced into unsigned int which is obviously potentially dangerous based on platform). Some of these warnings come from the test framework, so fixing the generation code will clearly fix many of them.

Further investigation also reveals that -Wshorten-64-to-32 is part of a bigger group -Wconversion which when turned on reveals even more issues, including some (at first glance) slightly worrying implicit sign conversions.

My method for silencing these warnings so far has basically been to add the obvious cast, however there are many cases in which I am not entirely sure this is the right thing. This task would involve drilling down on some of the more interesting cases to make sure that simply silencing the warning is the right thing.

My tests involved adding the relevant flags to the base compiler flags inside the if(CMAKE_COMPILER_IS_CLANG) group in
the root CMakeLists.txt. Part of this work would be to check at what point this option was introduced, in case it can only be added for certain versions of clang.

It would be nice to enable this for make, but given we cannot detect compiler or compiler version in make, I doubt this is possible, as there is not currently (as far as I can tell) any equivalent for this (in C anyway) for GCC (-Wnarrowing exists, but apparently only covers C++11 onwards).

@paul-elliott-arm paul-elliott-arm added bug size-m Estimated task size: medium (~1w) priority-medium Medium priority - this can be reviewed as time permits labels Jun 21, 2023
@paul-elliott-arm
Copy link
Member Author

@paulocoutinhox
Copy link

Hi,

When have a fix, im here to test in all platforms. Only add a comment to me here with @.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug priority-medium Medium priority - this can be reviewed as time permits size-m Estimated task size: medium (~1w)
Projects
Development

Successfully merging a pull request may close this issue.

2 participants