[clang] clang/limits.h: Avoid including limits.h twice (PR #120526)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 10 06:25:09 PST 2025


AaronBallman wrote:

Maybe my brain still isn't engaging after the holidays, but I'm still not seeing why this change is correct. From the point when Clang gets involved:

* The absolute path includes glibc's `limits.h`, which includes the next header in the chain, which is Clang's: https://sourceware.org/git/?p=glibc.git;a=blob;f=include/limits.h#l122
* If `_GCC_LIMITS_H` is not defined (but it is in this case), we define it: https://github.com/llvm/llvm-project/blob/6b12272353b45def33bf5814cdf9e8587f32d40e/clang/lib/Headers/limits.h#L18
* Then, if we're hosted and there's another `limits.h`, we include it again: https://github.com/llvm/llvm-project/blob/6b12272353b45def33bf5814cdf9e8587f32d40e/clang/lib/Headers/limits.h#L24
* This goes on to find musl's header which doesn't protect against being included this way. But your changes don't impact that situation because it is avoiding the include when `_LIBC_LIMITS_H` is defined, but musl uses `_LIMITS_H`: https://elixir.bootlin.com/musl/v1.2.5/source/include/limits.h#L1
* What's more, glibc's use of `_LIBC_LIMITS_H` ends before doing the `include_next`: https://sourceware.org/git/?p=glibc.git;a=blob;f=include/limits.h#l115

So as best I can tell, this is basically another variant of the same thing we were already doing to avoid re-including (part of) glibc, which is why I don't see how that helps. I would have expected the changes to be something more along the lines of (before the block where we define `_GCC_LIMITS_H`):
```
/* When building glibc on a musl libc system, glibc will include
   Clang's limits.h, which will include musl's limits.h, which
   cannot handle being included that way. Avert this #include_next
   madness by skipping musl's header. */
#if defined __GNUC__ && defined _GCC_LIMITS_H_ && !defined _LIMITS_H
#define _LIMITS_H
#endif
```
but that leads to the question of: why is including musl's `limits.h` a problem? It redefines macros, but that is fine on its own: https://godbolt.org/z/4nGf8zzxE If glibc is expecting different values, that seems like glibc could run into the issue if musl's header was discoverable on the include paths without involving Clang.

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


More information about the cfe-commits mailing list