Skip to content

[CIR] Disable gcc partially overloaded virtual warning #130322

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

Merged
merged 1 commit into from
Mar 10, 2025

Conversation

andykaylor
Copy link
Contributor

GCC, unlike clang, issues a warning when one virtual function is overridden in a derived class but one or more other virtual functions with the same name and different signature from a base class are not overridden. This leads to many warnings in the MLIR and ClangIR code when using the OpenConversionPattern<>::matchAndRewrite() function in the ordinary way. The "hiding" behavior is what we want, so we're just disabling the warning here.

GCC, unlike clang, issues a warning when one virtual function is overridden
in a derived class but one or more other virtual functions with the same
name and different signature from a base class are not overridden. This
leads to many warnings in the MLIR and ClangIR code when using the
OpenConversionPattern<>::matchAndRewrite() function in the ordinary way.
The "hiding" behavior is what we want, so we're just disabling the warning
here.
@llvmbot llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels Mar 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 7, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clangir

Author: Andy Kaylor (andykaylor)

Changes

GCC, unlike clang, issues a warning when one virtual function is overridden in a derived class but one or more other virtual functions with the same name and different signature from a base class are not overridden. This leads to many warnings in the MLIR and ClangIR code when using the OpenConversionPattern<>::matchAndRewrite() function in the ordinary way. The "hiding" behavior is what we want, so we're just disabling the warning here.


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

2 Files Affected:

  • (modified) clang/lib/CIR/CMakeLists.txt (+11)
  • (modified) clang/tools/cir-opt/CMakeLists.txt (+11)
diff --git a/clang/lib/CIR/CMakeLists.txt b/clang/lib/CIR/CMakeLists.txt
index 4a99ecb33dfb2..7bdf3fcc59035 100644
--- a/clang/lib/CIR/CMakeLists.txt
+++ b/clang/lib/CIR/CMakeLists.txt
@@ -1,6 +1,17 @@
 include_directories(${LLVM_MAIN_SRC_DIR}/../mlir/include)
 include_directories(${CMAKE_BINARY_DIR}/tools/mlir/include)
 
+# GCC, unlike clang, issues a warning when one virtual function is overridden
+# in a derived class but one or more other virtual functions with the same
+# name and different signature from a base class are not overridden. This
+# leads to many warnings in the MLIR and ClangIR code when using the
+# OpenConversionPattern<>::matchAndRewrite() function in the ordinary way.
+# The "hiding" behavior is what we want, so we're just disabling the warning
+# here.
+if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND (NOT "${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang"))
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-overloaded-virtual")
+endif()
+
 add_subdirectory(Dialect)
 add_subdirectory(CodeGen)
 add_subdirectory(FrontendAction)
diff --git a/clang/tools/cir-opt/CMakeLists.txt b/clang/tools/cir-opt/CMakeLists.txt
index 75bec5f4e1b0b..ca7ee44f6fd75 100644
--- a/clang/tools/cir-opt/CMakeLists.txt
+++ b/clang/tools/cir-opt/CMakeLists.txt
@@ -4,6 +4,17 @@ get_property(conversion_libs GLOBAL PROPERTY MLIR_CONVERSION_LIBS)
 include_directories(${LLVM_MAIN_SRC_DIR}/../mlir/include)
 include_directories(${CMAKE_BINARY_DIR}/tools/mlir/include)
 
+# GCC, unlike clang, issues a warning when one virtual function is overridden
+# in a derived class but one or more other virtual functions with the same
+# name and different signature from a base class are not overridden. This
+# leads to many warnings in the MLIR and ClangIR code when using the
+# OpenConversionPattern<>::matchAndRewrite() function in the ordinary way.
+# The "hiding" behavior is what we want, so we're just disabling the warning
+# here.
+if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND (NOT "${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang"))
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-overloaded-virtual")
+endif()
+
 add_clang_tool(cir-opt
   cir-opt.cpp
 )

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Someone better equipped to review cmake should review this, but looks ok to me.

@andykaylor
Copy link
Contributor Author

Someone better equipped to review cmake should review this, but looks ok to me.

I'm not sure who is well-versed in the dark art of cmake. I made this same change in the incubator, and it's working there, so I'm confident that it works as intended.

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, same observation as Erich regarding CMAKE stuff tho

@andykaylor andykaylor merged commit e1bd39c into llvm:main Mar 10, 2025
14 checks passed
@andykaylor andykaylor deleted the cir-overload-warning branch March 10, 2025 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants