Skip to content

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