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

[openmp] Remove -Wno-enum-constexpr-conversion #81318

Merged
merged 1 commit into from
Feb 11, 2024

Conversation

carlosgalvezp
Copy link
Contributor

This effectively reverts commit 9ff0cc7. For some reason "git revert" lead to "no changes" after fixing conflicts, so a clean revert was not possible.

The original issue (#57022) is no longer reproducible even with this patch, so we can remove the suppression.

This is in line with our goal to make -Wenum-constexpr-conversion a non-downgradeable error, see #59036.

This effectively reverts commit 9ff0cc7.
For some reason "git revert" lead to "no changes" after fixing
conflicts, so a clean revert was not possible.

The original issue (llvm#57022) is no longer reproducible even with this
patch, so we can remove the suppression.

This is in line with our goal to make -Wenum-constexpr-conversion
a non-downgradeable error, see llvm#59036.
@llvmbot llvmbot added openmp:libomp OpenMP host runtime openmp:libomptarget OpenMP offload runtime labels Feb 9, 2024
@carlosgalvezp carlosgalvezp added openmp and removed openmp:libomp OpenMP host runtime openmp:libomptarget OpenMP offload runtime labels Feb 9, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 9, 2024

@llvm/pr-subscribers-openmp

Author: Carlos Galvez (carlosgalvezp)

Changes

This effectively reverts commit 9ff0cc7. For some reason "git revert" lead to "no changes" after fixing conflicts, so a clean revert was not possible.

The original issue (#57022) is no longer reproducible even with this patch, so we can remove the suppression.

This is in line with our goal to make -Wenum-constexpr-conversion a non-downgradeable error, see #59036.


Full diff: https://github.com/llvm/llvm-project/pull/81318.diff

2 Files Affected:

  • (modified) openmp/cmake/HandleOpenMPOptions.cmake (-1)
  • (modified) openmp/cmake/config-ix.cmake (-1)
diff --git a/openmp/cmake/HandleOpenMPOptions.cmake b/openmp/cmake/HandleOpenMPOptions.cmake
index 9387d9b3b0ff75..4809520e1babbf 100644
--- a/openmp/cmake/HandleOpenMPOptions.cmake
+++ b/openmp/cmake/HandleOpenMPOptions.cmake
@@ -41,7 +41,6 @@ append_if(OPENMP_HAVE_WSIGN_COMPARE_FLAG "-Wsign-compare" CMAKE_C_FLAGS CMAKE_CX
 # printed. Therefore, check for whether the compiler supports options in the
 # form -W<foo>, and if supported, add the corresponding -Wno-<foo> option.
 
-append_if(OPENMP_HAVE_WENUM_CONSTEXPR_CONVERSION_FLAG "-Wno-enum-constexpr-conversion" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 append_if(OPENMP_HAVE_WEXTRA_FLAG "-Wno-extra" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 append_if(OPENMP_HAVE_WPEDANTIC_FLAG "-Wno-pedantic" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 append_if(OPENMP_HAVE_WMAYBE_UNINITIALIZED_FLAG "-Wno-maybe-uninitialized" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
diff --git a/openmp/cmake/config-ix.cmake b/openmp/cmake/config-ix.cmake
index a1e1b61d374b81..cfc683385d56e5 100644
--- a/openmp/cmake/config-ix.cmake
+++ b/openmp/cmake/config-ix.cmake
@@ -33,7 +33,6 @@ check_cxx_compiler_flag(-Wsign-compare OPENMP_HAVE_WSIGN_COMPARE_FLAG)
 # printed. Therefore, check for whether the compiler supports options in the
 # form -W<foo>, and if supported, add the corresponding -Wno-<foo> option.
 
-check_cxx_compiler_flag(-Wenum-constexpr-conversion OPENMP_HAVE_WENUM_CONSTEXPR_CONVERSION_FLAG)
 check_cxx_compiler_flag(-Wextra OPENMP_HAVE_WEXTRA_FLAG)
 check_cxx_compiler_flag(-Wpedantic OPENMP_HAVE_WPEDANTIC_FLAG)
 check_cxx_compiler_flag(-Wmaybe-uninitialized OPENMP_HAVE_WMAYBE_UNINITIALIZED_FLAG)

@carlosgalvezp carlosgalvezp added openmp:libomp OpenMP host runtime openmp:libomptarget OpenMP offload runtime labels Feb 9, 2024
@carlosgalvezp
Copy link
Contributor Author

FYI @shafik @AaronBallman @dwblaikie

@jhuber6
Copy link
Contributor

jhuber6 commented Feb 11, 2024

@carlosgalvezp carlosgalvezp merged commit 15279e7 into llvm:main Feb 11, 2024
10 checks passed
@carlosgalvezp carlosgalvezp deleted the openmp_enum branch February 11, 2024 14:04
@shafik
Copy link
Collaborator

shafik commented Feb 21, 2024

Thank you for taking care of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openmp:libomp OpenMP host runtime openmp:libomptarget OpenMP offload runtime openmp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants