[llvm] [mlir] [NFC] Add explicit #include abi-breaking.h where its macros are used. (PR #109453)

[David Blaikie via llvm-commits]

dwblaikie wrote:

> > 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)). 



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