[NFC] Add explicit #include abi-breaking.h where its macros are used. #109453

dfukalov · 2024-09-20T18:06:27Z

Without these explicit includes, removing other headers, who implicitly include abi-breaking.h, may have non-trivial side effects. For example, clangd may report even abi-breaking.h as "no used" in case it defines a macro, that is explicitly used with #ifdef. It is actually amplified with different build configs which use different set of macros.

Example of the code
foo.c:

#include "bar.h"

void main(void) {
#ifdef CONFIG_MACRO
  kill_all_the_people();
#else
  do_nothing();
#endif
}

bar.h:

#include "generated_by_cmake_config.h" // where CONFIG_MACRO is defined or not

// the function used in foo.c a long time ago, but now it is not used
int bar(){
#ifdef CONFIG_MACRO
  return 42;
#else
  return -42;
#endif
}

So in the example clangd can report that first include line #include "bar.h" in foo.c module is not needed since none of its content is used. A human also can easily miss such indirect include. Let's imagine that contents of generated_by_cmake_config.h depends on a lot of parameters, including target, host, config options of a build, etc.
After removing #include "bar.h" line there is no way to appropriate test all of them without unpredictable call of kill_all_people() in one "lucky" environment.

I believe the simplest way to try to avoid it - to explicitly include all of config headers with notes in comments that they are used for the particular macro(s).

Without these explicit includes, removing other headers, who implicitly include `abi-breaking.h`, may have non-trivial side effects. For example, `clangd` may report even `abi-breaking.h` as "no used" in case it defines a macro, that is explicitly used with `#ifdef`. It is actually amplified with different build configs which use different set of macros.

llvmbot · 2024-09-25T11:59:06Z

@llvm/pr-subscribers-mlir-presburger
@llvm/pr-subscribers-llvm-adt
@llvm/pr-subscribers-llvm-selectiondag
@llvm/pr-subscribers-llvm-ir
@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-mlir

Author: Daniil Fukalov (dfukalov)

Changes

Without these explicit includes, removing other headers, who implicitly include abi-breaking.h, may have non-trivial side effects. For example, clangd may report even abi-breaking.h as "no used" in case it defines a macro, that is explicitly used with #ifdef. It is actually amplified with different build configs which use different set of macros.

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

34 Files Affected:

(modified) llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h (+1)
(modified) llvm/include/llvm/CodeGen/MachineScheduler.h (+1)
(modified) llvm/include/llvm/CodeGen/PBQP/ReductionRules.h (+1)
(modified) llvm/include/llvm/CodeGen/RegAllocPBQP.h (+1)
(modified) llvm/include/llvm/CodeGen/SelectionDAGISel.h (+1)
(modified) llvm/include/llvm/IR/ValueHandle.h (+1)
(modified) llvm/include/llvm/Passes/StandardInstrumentations.h (+1)
(modified) llvm/include/llvm/Support/GenericDomTreeConstruction.h (+1)
(modified) llvm/include/llvm/Support/GenericLoopInfo.h (+1)
(modified) llvm/include/llvm/Transforms/Scalar/JumpThreading.h (+1)
(modified) llvm/include/llvm/Transforms/Scalar/LoopPassManager.h (+2-1)
(modified) llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h (+1)
(modified) llvm/lib/CodeGen/MachineScheduler.cpp (+1)
(modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp (+1)
(modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp (+1)
(modified) llvm/lib/Passes/StandardInstrumentations.cpp (+1)
(modified) llvm/lib/Support/Error.cpp (+1)
(modified) llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp (+1)
(modified) llvm/unittests/ADT/BitVectorTest.cpp (+1)
(modified) llvm/unittests/ADT/FallibleIteratorTest.cpp (+1)
(modified) llvm/unittests/ADT/IListSentinelTest.cpp (+1)
(modified) llvm/unittests/ADT/STLExtrasTest.cpp (+1)
(modified) llvm/unittests/IR/ValueHandleTest.cpp (+1)
(modified) llvm/unittests/Support/DataExtractorTest.cpp (+1)
(modified) llvm/unittests/Support/ErrorTest.cpp (+1)
(modified) mlir/include/mlir/Analysis/DataFlowFramework.h (+1)
(modified) mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h (+1)
(modified) mlir/include/mlir/Dialect/Transform/Interfaces/TransformInterfaces.h (+1)
(modified) mlir/include/mlir/Dialect/Transform/Utils/DiagnosedSilenceableFailure.h (+1)
(modified) mlir/include/mlir/IR/PDLPatternMatch.h.inc (+1)
(modified) mlir/include/mlir/Interfaces/DataLayoutInterfaces.h (+1)
(modified) mlir/lib/Dialect/Transform/Interfaces/TransformInterfaces.cpp (+1)
(modified) mlir/lib/Dialect/Transform/Utils/DiagnosedSilenceableFailure.cpp (+1)
(modified) mlir/lib/Interfaces/DataLayoutInterfaces.cpp (+1)

diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h b/llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
index dba3a8a14480c1..a5bed444b0ec77 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
@@ -11,6 +11,7 @@
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 
 namespace llvm {
 
diff --git a/llvm/include/llvm/CodeGen/MachineScheduler.h b/llvm/include/llvm/CodeGen/MachineScheduler.h
index b15abf040058e8..fee2ef0e8d70f1 100644
--- a/llvm/include/llvm/CodeGen/MachineScheduler.h
+++ b/llvm/include/llvm/CodeGen/MachineScheduler.h
@@ -88,6 +88,7 @@
 #include "llvm/CodeGen/ScheduleDAGInstrs.h"
 #include "llvm/CodeGen/ScheduleDAGMutation.h"
 #include "llvm/CodeGen/TargetSchedule.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <algorithm>
diff --git a/llvm/include/llvm/CodeGen/PBQP/ReductionRules.h b/llvm/include/llvm/CodeGen/PBQP/ReductionRules.h
index 043b6b12063235..e20d58e5a6ed17 100644
--- a/llvm/include/llvm/CodeGen/PBQP/ReductionRules.h
+++ b/llvm/include/llvm/CodeGen/PBQP/ReductionRules.h
@@ -16,6 +16,7 @@
 #include "Graph.h"
 #include "Math.h"
 #include "Solution.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include <cassert>
 #include <limits>
 
diff --git a/llvm/include/llvm/CodeGen/RegAllocPBQP.h b/llvm/include/llvm/CodeGen/RegAllocPBQP.h
index 234f1c6ff115ac..94a1904e611926 100644
--- a/llvm/include/llvm/CodeGen/RegAllocPBQP.h
+++ b/llvm/include/llvm/CodeGen/RegAllocPBQP.h
@@ -23,6 +23,7 @@
 #include "llvm/CodeGen/PBQP/ReductionRules.h"
 #include "llvm/CodeGen/PBQP/Solution.h"
 #include "llvm/CodeGen/Register.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/MC/MCRegister.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <algorithm>
diff --git a/llvm/include/llvm/CodeGen/SelectionDAGISel.h b/llvm/include/llvm/CodeGen/SelectionDAGISel.h
index f6191c6fdb7fe6..9e76e892c77ee5 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAGISel.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAGISel.h
@@ -17,6 +17,7 @@
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachinePassManager.h"
 #include "llvm/CodeGen/SelectionDAG.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/IR/BasicBlock.h"
 #include <memory>
 
diff --git a/llvm/include/llvm/IR/ValueHandle.h b/llvm/include/llvm/IR/ValueHandle.h
index 29560815ea559e..444d9f71d6e704 100644
--- a/llvm/include/llvm/IR/ValueHandle.h
+++ b/llvm/include/llvm/IR/ValueHandle.h
@@ -15,6 +15,7 @@
 
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/PointerIntPair.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/IR/Value.h"
 #include "llvm/Support/Casting.h"
 #include <cassert>
diff --git a/llvm/include/llvm/Passes/StandardInstrumentations.h b/llvm/include/llvm/Passes/StandardInstrumentations.h
index fa9c744294a666..cd52116c7f36ef 100644
--- a/llvm/include/llvm/Passes/StandardInstrumentations.h
+++ b/llvm/include/llvm/Passes/StandardInstrumentations.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/OptBisect.h"
 #include "llvm/IR/PassTimingInfo.h"
diff --git a/llvm/include/llvm/Support/GenericDomTreeConstruction.h b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
index 9aab5ec60f4a25..612d897df7d9e3 100644
--- a/llvm/include/llvm/Support/GenericDomTreeConstruction.h
+++ b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
@@ -41,6 +41,7 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/DepthFirstIterator.h"
 #include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/GenericDomTree.h"
 #include <optional>
diff --git a/llvm/include/llvm/Support/GenericLoopInfo.h b/llvm/include/llvm/Support/GenericLoopInfo.h
index d560ca648132c9..c590d89fac74bd 100644
--- a/llvm/include/llvm/Support/GenericLoopInfo.h
+++ b/llvm/include/llvm/Support/GenericLoopInfo.h
@@ -44,6 +44,7 @@
 #include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetOperations.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/GenericDomTree.h"
 
diff --git a/llvm/include/llvm/Transforms/Scalar/JumpThreading.h b/llvm/include/llvm/Transforms/Scalar/JumpThreading.h
index a3f2ce23f7d9aa..65c4d76b97a9dc 100644
--- a/llvm/include/llvm/Transforms/Scalar/JumpThreading.h
+++ b/llvm/include/llvm/Transforms/Scalar/JumpThreading.h
@@ -21,6 +21,7 @@
 #include "llvm/Analysis/BlockFrequencyInfo.h"
 #include "llvm/Analysis/BranchProbabilityInfo.h"
 #include "llvm/Analysis/DomTreeUpdater.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/IR/ValueHandle.h"
 #include "llvm/Transforms/Utils/ValueMapper.h"
 #include <optional>
diff --git a/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h b/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
index 3858be05c61fa9..7093d6bd66dac9 100644
--- a/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
+++ b/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
@@ -40,8 +40,9 @@
 #include "llvm/Analysis/LoopAnalysisManager.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/LoopNestAnalysis.h"
-#include "llvm/IR/PassManager.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/IR/PassInstrumentation.h"
+#include "llvm/IR/PassManager.h"
 #include "llvm/Transforms/Utils/LCSSA.h"
 #include "llvm/Transforms/Utils/LoopSimplify.h"
 #include "llvm/Transforms/Utils/LoopUtils.h"
diff --git a/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h b/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
index 62c1e15a9a60e1..d2461c8e71c566 100644
--- a/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
+++ b/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
@@ -20,6 +20,7 @@
 #include "llvm/Analysis/ScalarEvolutionExpressions.h"
 #include "llvm/Analysis/ScalarEvolutionNormalization.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/ValueHandle.h"
 #include "llvm/Support/CommandLine.h"
diff --git a/llvm/lib/CodeGen/MachineScheduler.cpp b/llvm/lib/CodeGen/MachineScheduler.cpp
index 4e6d34346b1d80..6e99d49edfc894 100644
--- a/llvm/lib/CodeGen/MachineScheduler.cpp
+++ b/llvm/lib/CodeGen/MachineScheduler.cpp
@@ -48,6 +48,7 @@
 #include "llvm/CodeGen/TargetSchedule.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
 #include "llvm/CodeGenTypes/MachineValueType.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/Config/llvm-config.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/MC/LaneBitmask.h"
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 2a97580942df36..d028cc7b7c9234 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -61,6 +61,7 @@
 #include "llvm/CodeGen/ValueTypes.h"
 #include "llvm/CodeGen/WinEHFuncInfo.h"
 #include "llvm/CodeGenTypes/MachineValueType.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
index ac28f62894788d..bd2aeaf5985598 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
@@ -15,6 +15,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/SelectionDAG.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/GraphWriter.h"
 #include "llvm/Support/raw_ostream.h"
diff --git a/llvm/lib/Passes/StandardInstrumentations.cpp b/llvm/lib/Passes/StandardInstrumentations.cpp
index 036484c9c1c0c4..981a394aa820cb 100644
--- a/llvm/lib/Passes/StandardInstrumentations.cpp
+++ b/llvm/lib/Passes/StandardInstrumentations.cpp
@@ -22,6 +22,7 @@
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/MachineVerifier.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Module.h"
diff --git a/llvm/lib/Support/Error.cpp b/llvm/lib/Support/Error.cpp
index baa3c322e9dae1..7ad5fa2c20d7bd 100644
--- a/llvm/lib/Support/Error.cpp
+++ b/llvm/lib/Support/Error.cpp
@@ -10,6 +10,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/Support/ErrorHandling.h"
 #include <system_error>
 
diff --git a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
index 0927a3015818fd..3edcec36f8d572 100644
--- a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
+++ b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
@@ -20,6 +20,7 @@
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/IntrinsicInst.h"
diff --git a/llvm/unittests/ADT/BitVectorTest.cpp b/llvm/unittests/ADT/BitVectorTest.cpp
index 6a4780c143e548..2f2dfab5503745 100644
--- a/llvm/unittests/ADT/BitVectorTest.cpp
+++ b/llvm/unittests/ADT/BitVectorTest.cpp
@@ -9,6 +9,7 @@
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/SmallBitVector.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "gtest/gtest.h"
 
 using namespace llvm;
diff --git a/llvm/unittests/ADT/FallibleIteratorTest.cpp b/llvm/unittests/ADT/FallibleIteratorTest.cpp
index d3389744ffbfe8..b13cebbf9db734 100644
--- a/llvm/unittests/ADT/FallibleIteratorTest.cpp
+++ b/llvm/unittests/ADT/FallibleIteratorTest.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/fallible_iterator.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/Testing/Support/Error.h"
 
 #include "gtest/gtest-spi.h"
diff --git a/llvm/unittests/ADT/IListSentinelTest.cpp b/llvm/unittests/ADT/IListSentinelTest.cpp
index 1f4a8311370a6d..f271eb82728926 100644
--- a/llvm/unittests/ADT/IListSentinelTest.cpp
+++ b/llvm/unittests/ADT/IListSentinelTest.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/ilist_node.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "gtest/gtest.h"
 
 using namespace llvm;
diff --git a/llvm/unittests/ADT/STLExtrasTest.cpp b/llvm/unittests/ADT/STLExtrasTest.cpp
index ee8299c9b48612..458f38fb48b9af 100644
--- a/llvm/unittests/ADT/STLExtrasTest.cpp
+++ b/llvm/unittests/ADT/STLExtrasTest.cpp
@@ -8,6 +8,7 @@
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
diff --git a/llvm/unittests/IR/ValueHandleTest.cpp b/llvm/unittests/IR/ValueHandleTest.cpp
index e11c545520d5a0..cdef5f47db21ac 100644
--- a/llvm/unittests/IR/ValueHandleTest.cpp
+++ b/llvm/unittests/IR/ValueHandleTest.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/ValueHandle.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/LLVMContext.h"
diff --git a/llvm/unittests/Support/DataExtractorTest.cpp b/llvm/unittests/Support/DataExtractorTest.cpp
index e019cf6fc92569..88d1aea0d8d69d 100644
--- a/llvm/unittests/Support/DataExtractorTest.cpp
+++ b/llvm/unittests/Support/DataExtractorTest.cpp
@@ -9,6 +9,7 @@
 #include "llvm/Support/DataExtractor.h"
 
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 using namespace llvm;
diff --git a/llvm/unittests/Support/ErrorTest.cpp b/llvm/unittests/Support/ErrorTest.cpp
index 98d19e8d2a15a3..e24ee4d6e09060 100644
--- a/llvm/unittests/Support/ErrorTest.cpp
+++ b/llvm/unittests/Support/ErrorTest.cpp
@@ -10,6 +10,7 @@
 #include "llvm-c/Error.h"
 
 #include "llvm/ADT/Twine.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Testing/Support/Error.h"
diff --git a/mlir/include/mlir/Analysis/DataFlowFramework.h b/mlir/include/mlir/Analysis/DataFlowFramework.h
index b0450ecdbd99b8..e0bccf3d1bc3fa 100644
--- a/mlir/include/mlir/Analysis/DataFlowFramework.h
+++ b/mlir/include/mlir/Analysis/DataFlowFramework.h
@@ -19,6 +19,7 @@
 #include "mlir/IR/Operation.h"
 #include "mlir/Support/StorageUniquer.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/TypeName.h"
 #include <queue>
diff --git a/mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h b/mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h
index cff79579898710..ee18476cb9903f 100644
--- a/mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h
+++ b/mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h
@@ -16,6 +16,7 @@
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/Support/PointerLikeTypeTraits.h"
 #include "llvm/Support/TypeName.h"
 #include "llvm/Support/raw_ostream.h"
diff --git a/mlir/include/mlir/Dialect/Transform/Interfaces/TransformInterfaces.h b/mlir/include/mlir/Dialect/Transform/Interfaces/TransformInterfaces.h
index 43193e4cd4cf63..a12f6e2a842c72 100644
--- a/mlir/include/mlir/Dialect/Transform/Interfaces/TransformInterfaces.h
+++ b/mlir/include/mlir/Dialect/Transform/Interfaces/TransformInterfaces.h
@@ -15,6 +15,7 @@
 #include "mlir/IR/PatternMatch.h"
 #include "mlir/Interfaces/SideEffectInterfaces.h"
 #include "mlir/Transforms/DialectConversion.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 
 #include "mlir/Dialect/Transform/Interfaces/TransformTypeInterfaces.h.inc"
 
diff --git a/mlir/include/mlir/Dialect/Transform/Utils/DiagnosedSilenceableFailure.h b/mlir/include/mlir/Dialect/Transform/Utils/DiagnosedSilenceableFailure.h
index fcf422a0b6aa34..ef39f57fde7654 100644
--- a/mlir/include/mlir/Dialect/Transform/Utils/DiagnosedSilenceableFailure.h
+++ b/mlir/include/mlir/Dialect/Transform/Utils/DiagnosedSilenceableFailure.h
@@ -14,6 +14,7 @@
 
 #include "mlir/IR/Diagnostics.h"
 #include "mlir/IR/Operation.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include <optional>
 
 #ifndef MLIR_DIALECT_TRANSFORM_UTILS_DIAGNOSEDSILENCEABLEFAILURE_H
diff --git a/mlir/include/mlir/IR/PDLPatternMatch.h.inc b/mlir/include/mlir/IR/PDLPatternMatch.h.inc
index 66286ed7a4c898..573d684981e96a 100644
--- a/mlir/include/mlir/IR/PDLPatternMatch.h.inc
+++ b/mlir/include/mlir/IR/PDLPatternMatch.h.inc
@@ -14,6 +14,7 @@
 #if MLIR_ENABLE_PDL_IN_PATTERNMATCH
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/BuiltinOps.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 
 namespace mlir {
 //===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.h b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.h
index 848d2dee4a6309..021903ac844590 100644
--- a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.h
+++ b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.h
@@ -18,6 +18,7 @@
 #include "mlir/IR/DialectInterface.h"
 #include "mlir/IR/OpDefinition.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/Support/TypeSize.h"
 
 namespace mlir {
diff --git a/mlir/lib/Dialect/Transform/Interfaces/TransformInterfaces.cpp b/mlir/lib/Dialect/Transform/Interfaces/TransformInterfaces.cpp
index 91702ce7cc42b9..ce699065fae529 100644
--- a/mlir/lib/Dialect/Transform/Interfaces/TransformInterfaces.cpp
+++ b/mlir/lib/Dialect/Transform/Interfaces/TransformInterfaces.cpp
@@ -15,6 +15,7 @@
 #include "mlir/Transforms/GreedyPatternRewriteDriver.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 
diff --git a/mlir/lib/Dialect/Transform/Utils/DiagnosedSilenceableFailure.cpp b/mlir/lib/Dialect/Transform/Utils/DiagnosedSilenceableFailure.cpp
index 44cc561485799e..1536f0d2a82952 100644
--- a/mlir/lib/Dialect/Transform/Utils/DiagnosedSilenceableFailure.cpp
+++ b/mlir/lib/Dialect/Transform/Utils/DiagnosedSilenceableFailure.cpp
@@ -13,6 +13,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "mlir/Dialect/Transform/Utils/DiagnosedSilenceableFailure.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 
 using namespace mlir;
 
diff --git a/mlir/lib/Interfaces/DataLayoutInterfaces.cpp b/mlir/lib/Interfaces/DataLayoutInterfaces.cpp
index 2158953c07110a..f6bc598f5e21f5 100644
--- a/mlir/lib/Interfaces/DataLayoutInterfaces.cpp
+++ b/mlir/lib/Interfaces/DataLayoutInterfaces.cpp
@@ -13,6 +13,7 @@
 #include "mlir/IR/Operation.h"
 
 #include "llvm/ADT/TypeSwitch.h"
+#include "llvm/Config/abi-breaking.h" // for LLVM_ENABLE_ABI_BREAKING_CHECKS
 #include "llvm/Support/MathExtras.h"
 
 using namespace mlir;

joker-eph

None of the MLIR changes are needed, we are covered by -Wundef I believe.

Also I feel we debated this before, but the PR description is still written in a way that is just completely confusing to me. Can you please rework all this?

dwblaikie · 2024-09-25T17:39:55Z

None of the MLIR changes are needed, we are covered by -Wundef I believe.

This patch seems to be an "include what you use" fix, which generally fix fragility (as the description says - cases where if an unrelated header was removed, the build would fail because the file that included the unrelated header was depending on that header providing some other header that's used). Looks like -Wundef would only protect from using a macro that's truly undefined, yeah?

Also I feel we debated this before, but the PR description is still written in a way that is just completely confusing to me. Can you please rework all this?

I think I'm mostly following it - could you clarify what's confusing to you?

I will say I don't understand the second sentence ("For example, clangd may report even abi-breaking.h as "no used" in case it defines a macro, that is explicitly used with #ifdef."). ooh, actually, I think I follow - clangd could identify abi-breaking.h as unused in some other header x.h (because clangd would do a local analysis, and find abi-breaking.h unused in x.h), even though the macros are used from from some other file that /includes/ x.h.

joker-eph · 2024-09-25T20:46:25Z

This patch seems to be an "include what you use" fix, which generally fix fragility (as the description says - cases where if an unrelated header was removed, the build would fail because the file that included the unrelated header was depending on that header providing some other header that's used). Looks like -Wundef would only protect from using a macro that's truly undefined, yeah?

Do you believe that any change in MLIR is protecting us from anything not covered by Wunder? If so can you describe the possible failure here?

I think I'm mostly following it - could you clarify what's confusing to you?

Everythig about clangd: the sentence starts with "for example" as if it is an important illustration from "removing other headers, who implicitly include abi-breaking.h, may have non-trivial side effects".
I don't quite see how "removing abi-breaking.h" can have as "side-effect" the fact that "clangd may report even abi-breaking.h as "no used" "

(You're touching on this: this is most of the description though!)

In general a description that is simply saying "IWYU / Without these includes, removing other headers, who implicitly include xxxx.h, may have non-trivial side effects" looks to me like a negation of LLVM policy about "including the minimum needed".

dwblaikie · 2024-09-25T21:05:55Z

This patch seems to be an "include what you use" fix, which generally fix fragility (as the description says - cases where if an unrelated header was removed, the build would fail because the file that included the unrelated header was depending on that header providing some other header that's used). Looks like -Wundef would only protect from using a macro that's truly undefined, yeah?

Do you believe that any change in MLIR is protecting us from anything not covered by Wunder? If so can you describe the possible failure here?

My expectation is that these changes would avoid a correct header removal from causing a build break under -Wundef. Means folks can remove unused headers without the spooky-action-at-a-distance cleanup required.

I think I'm mostly following it - could you clarify what's confusing to you?

Everythig about clangd: the sentence starts with "for example" as if it is an important illustration from "removing other headers, who implicitly include abi-breaking.h, may have non-trivial side effects". I don't quite see how "removing abi-breaking.h" can have as "side-effect" the fact that "clangd may report even abi-breaking.h as "no used" "

(You're touching on this: this is most of the description though!)

Looks like we agree on this, more or less.

In general a description that is simply saying "IWYU / Without these includes, removing other headers, who implicitly include xxxx.h, may have non-trivial side effects" looks to me like a negation of LLVM policy about "including the minimum needed".

Not relying on implicit/indirect dependencies seems to me consistent with LLVM's policy. The https://llvm.org/docs/CodingStandards.html#include-as-little-as-possible is about avoiding the inclusion entirely (to reduce total preprocessed bytes), when an include can be replaced by a forward declaration, etc. That's not the case we're talking about here - where the inclusion is needed, but is relied upon indirectly.

Admittedly the policy here does allow indirect dependencies, but doesn't encourage or discourage it: "You must include all of the header files that you are using — you can include them either directly or indirectly through another header file." - though C++ and C style in general would be that explicit inclusion is preferred for maintainability (so there's no spooky-action-at-a-distance cleanup when someone removes a header that isn't used directly (from the user or the usee, as it were)).

joker-eph · 2024-09-25T21:36:01Z

My expectation is that these changes would avoid a correct header removal from causing a build break under -Wundef. Means folks can remove unused headers without the spooky-action-at-a-distance cleanup required.

If this is really the problem to be solved, then this is what the description should explain clearly, and we can argue about it :)

Right now it seems that you're arguing against the policy of " You must include all of the header files that you are using — you can include them either directly or indirectly through another header file" ; so I'd rather first see a PR that rewrite this to endorse the policy you want to see in place (probably with an RFC on Discourse).

dfukalov · 2024-09-25T22:03:08Z

None of the MLIR changes are needed, we are covered by -Wundef I believe.

There are some places in MLIR where the option will not work, e.g.

llvm-project/mlir/include/mlir/Dialect/Transform/Interfaces/TransformInterfaces.h

Line 199 in fe06a6d

#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS

, since -Wundef works for #if only, not for #ifdef.

We have no coding style requirement of the topic and pre-commit tests/checks. While we have no such requirements, the macros which configure compilation still be fragile.

dwblaikie · 2024-09-25T22:08:02Z

None of the MLIR changes are needed, we are covered by -Wundef I believe.

There are some places in MLIR where the option will not work, e.g.

llvm-project/mlir/include/mlir/Dialect/Transform/Interfaces/TransformInterfaces.h

Line 199 in fe06a6d

#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS

, since -Wundef works for #if only, not for #ifdef.
We have no coding style requirement of the topic and pre-commit tests/checks. While we have no such requirements, the macros which configure compilation still be fragile.

I guess that usage is just buggy/incorrect, unfortunately (unfortunately in terms of this not providing any extra motivation for this proposed patch, I think) - ifdef abi_breaking_checks will be true even when abi_breaking_checks is disabled (because it'll be disabled by being defined to 0, so ifdef will still be true because it is defined (albeit to 0))

joker-eph · 2024-09-26T23:09:43Z

Thanks, fixing in #110185 FYI

dfukalov force-pushed the add-abi-breaking-include branch from f84d593 to f2f453c Compare September 24, 2024 13:56

dfukalov marked this pull request as ready for review September 25, 2024 11:58

dfukalov requested review from Groverkss and Superty as code owners September 25, 2024 11:58

llvmbot added mlir:core MLIR Core Infrastructure llvm:globalisel mlir mlir:presburger llvm:support llvm:SelectionDAG SelectionDAGISel as well llvm:ir llvm:transforms llvm:adt labels Sep 25, 2024

dfukalov requested review from nikic, pogo59 and kuhar and removed request for Superty, Groverkss, nikic and pogo59 September 25, 2024 11:58

joker-eph requested changes Sep 25, 2024

View reviewed changes

dfukalov closed this Mar 25, 2025

dfukalov deleted the add-abi-breaking-include branch March 25, 2025 12:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[NFC] Add explicit #include abi-breaking.h where its macros are used. #109453

[NFC] Add explicit #include abi-breaking.h where its macros are used. #109453

Uh oh!

dfukalov commented Sep 20, 2024 •

edited

Loading

Uh oh!

llvmbot commented Sep 25, 2024 •

edited

Loading

Uh oh!

joker-eph left a comment

Uh oh!

dwblaikie commented Sep 25, 2024

Uh oh!

joker-eph commented Sep 25, 2024 •

edited

Loading

Uh oh!

dwblaikie commented Sep 25, 2024

Uh oh!

joker-eph commented Sep 25, 2024

Uh oh!

dfukalov commented Sep 25, 2024

Uh oh!

dwblaikie commented Sep 25, 2024

Uh oh!

joker-eph commented Sep 26, 2024

Uh oh!

Uh oh!

[NFC] Add explicit #include abi-breaking.h where its macros are used. #109453

[NFC] Add explicit #include abi-breaking.h where its macros are used. #109453

Uh oh!

Conversation

dfukalov commented Sep 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joker-eph left a comment

Choose a reason for hiding this comment

Uh oh!

dwblaikie commented Sep 25, 2024

Uh oh!

joker-eph commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dwblaikie commented Sep 25, 2024

Uh oh!

joker-eph commented Sep 25, 2024

Uh oh!

dfukalov commented Sep 25, 2024

Uh oh!

dwblaikie commented Sep 25, 2024

Uh oh!

joker-eph commented Sep 26, 2024

Uh oh!

Uh oh!

dfukalov commented Sep 20, 2024 •

edited

Loading

llvmbot commented Sep 25, 2024 •

edited

Loading

joker-eph commented Sep 25, 2024 •

edited

Loading