[mlir][cmake] Enable -Wundef.

[ingomueller-net]

This is another follow-up of #83004, which fixed a bug due to some macros not being defined in some situations. By raising warnings on undefined macros, this kind of bug is less likely to be introduced. Similar to #83004, the fix is probably adding an include to mlir-config.h (and potentially defining the macro there).

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Ingo Müller (ingomueller-net)

Changes

This is another follow-up of #83004, which fixed a bug due to some macros not being defined in some situations. By raising warnings on undefined macros, this kind of bug is less likely to be introduced. Similar to #83004, the fix is probably adding an include to mlir-config.h (and potentially defining the macro there).

---

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

1 Files Affected:

• (modified) mlir/CMakeLists.txt (+6)

diff --git a/mlir/CMakeLists.txt b/mlir/CMakeLists.txt
index 070609c94a3b38..9637e9cbb0ef35 100644
--- a/mlir/CMakeLists.txt
+++ b/mlir/CMakeLists.txt
@@ -70,6 +70,12 @@ endif()
 check_c_compiler_flag("-Werror=implicit-function-declaration" C_SUPPORTS_WERROR_IMPLICIT_FUNCTION_DECLARATION)
 append_if(C_SUPPORTS_WERROR_IMPLICIT_FUNCTION_DECLARATION "-Werror=implicit-function-declaration" CMAKE_C_FLAGS)
 
+# Warn on undefined macros. This is often an indication that an include to
+# `mlir-config.h` or similar is missing.
+check_c_compiler_flag("-Wundef" C_SUPPORTS_WUNDEF)
+append_if(C_SUPPORTS_WUNDEF "-Wundef" CMAKE_C_FLAGS)
+append_if(C_SUPPORTS_WUNDEF "-Wundef" CMAKE_CXX_FLAGS)
+
 # Forbid mismatch between declaration and definition for class vs struct. This is
 # harmless on Unix systems, but it'll be a ticking bomb for MSVC/Windows systems
 # where it creeps into the ABI.

[ingomueller-net]

After this and all related PRs have landed and the dust has settled, we may consider turning this into -Werror=undef.

Also note that this seems more challenging to do for the Bazel builds because some of the macros are not defined in its version of mlir-config.h and quite some machinery seems to be missing to change that.

[ingomueller-net]

The only new undefined macro coming up in CI is GTEST_NO_LLVM_SUPPORT. I think it is unset for all LLVM except compiler-rt, so maybe we can add a compile definition for either the mlir or the mlir/unittest folder (or even all of LLVM?)

[joker-eph]

Awesome :)

I had a work-in-progress to do this in LLVM itself, but let's start with MLIR indeed! :)

Can you add some documentation in mlir-config.h that describe that we want cmakedefine01 because we're relying on Wundef for finding missing includes?

[ingomueller-net]

Yeah, good point! Fixed with latest commit.

I'll merge when CI has passed but do let me know in case you aren't happy with the text I added; I can still change it.

[joker-eph]

Looks great, thanks!

[PeimingLiu]

Seems that the change exposes 'GTEST_NO_LLVM_SUPPORT' is not defined, evaluates to 0, any idea how to fix that?

[PeimingLiu]

I have a tentative fix in #84539, but I am not sure whether I am doing the right thing or not

[joker-eph]

I'm puzzled why this does not fail consistently though?

[PeimingLiu]

I'm puzzled why this does not fail consistently though?

Well, the warning is being consistently reported in https://lab.llvm.org/buildbot/#/builders/61/builds/55414