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

wzssyqa wrote: > Do you get the same behavior when you use `-isystem` instead? There is no warning then if I use command ``` clang -E -I build-x86/ -I sysdeps/x86 -I. -isystem ./include -I sysdeps/unix/sysv/linux/ -I sysdeps/generic/ -include include/libc-symbols.h -O2 -H xx.c ``` instead of

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

AaronBallman wrote: > > > > Yes and no. It's due to the redefinitions, for sure. But that > > > > diagnostic is suppressed in system headers: > > > > https://godbolt.org/z/Mb7Kh975f, so I think we need to understand why > > > > there's a `limits.h` being included as though it were not a system

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

AaronBallman wrote: > > Yes and no. It's due to the redefinitions, for sure. But that diagnostic is > > suppressed in system headers: https://godbolt.org/z/Mb7Kh975f, so I think > > we need to understand why there's a `limits.h` being included as though it > > were not a system header, because

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

wzssyqa wrote: > > Yes and no. It's due to the redefinitions, for sure. But that diagnostic is > suppressed in system headers: https://godbolt.org/z/Mb7Kh975f, so I think we > need to understand why there's a `limits.h` being included as though it were > not a system header, because that's a

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

AaronBallman wrote: > > but that leads to the question of: why is including musl's limits.h a > > problem? > > If we build `glibc` on `Alpine` with clang, we will meet the problem like > > ``` > ./include/bits/xopen_lim.h:84:10: warning: "NL_NMAX" redefined >84 | # define NL_NMAXIN

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

https://github.com/wzssyqa updated https://github.com/llvm/llvm-project/pull/120526 >From cc30e98287ec0ceca490f1f6a16a0190ff243ed6 Mon Sep 17 00:00:00 2001 From: YunQiang Su Date: Thu, 19 Dec 2024 14:09:04 +0800 Subject: [PATCH] clang/limits.h: Avoid including limits.h twice The limits.h of gl

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

wzssyqa wrote: > but that leads to the question of: why is including musl's limits.h a problem? It cause some warning like ``` ./include/bits/xopen_lim.h:84:10: warning: "NL_NMAX" redefined 84 | # define NL_NMAXINT_MAX ``` https://github.com/llvm/llvm-project/pull/120526

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

wzssyqa wrote: You are right. My patch only fix the first problem: making a unit test of glibc happy. The failure test case of glibc is: * check-local-headers See: https://github.com/llvm/llvm-project/pull/120673 also. https://github.com/llvm/llvm-project/pull/120526 __

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

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://sou

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

wzssyqa wrote: > I'm confused why this is necessary, we already work around glibc's quirk here: > > https://github.com/llvm/llvm-project/blob/a15fedc399d5d1aa07c14531e5cd8d3efc583600/clang/lib/Headers/limits.h#L18 > > > which should then hit glibc's header guard here: > https://sourceware.org/

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

AaronBallman wrote: I'm confused why this is necessary, we already work around glibc's quirk here: https://github.com/llvm/llvm-project/blob/a15fedc399d5d1aa07c14531e5cd8d3efc583600/clang/lib/Headers/limits.h#L18 which should then hit glibc's header guard here: https://sourceware.org/git/?p=glibc

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

pirama-arumuga-nainar wrote: Does this pattern still occur while doing `echo "#include " | clang-19 -E -O2 -xc - -MM -H`? That is idiomatic compared to including `/usr/include/limits.h` directly. It also doesn't seem correct to depend on the `_LIBC_LIMITS_H_` macro which is defined in glibc.

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

wzssyqa wrote: ping https://github.com/llvm/llvm-project/pull/120526 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

https://github.com/wzssyqa edited https://github.com/llvm/llvm-project/pull/120526 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

https://github.com/wzssyqa edited https://github.com/llvm/llvm-project/pull/120526 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

llvmbot wrote: @llvm/pr-subscribers-backend-x86 Author: YunQiang Su (wzssyqa) Changes The limits.h of glibc, aka /usr/include/limits.h file of *-linux-gnu systems, has `#include_next `, so the limits.h from clang is included. And in the limits.h for clang, `#include_next

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

https://github.com/wzssyqa edited https://github.com/llvm/llvm-project/pull/120526 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

https://github.com/wzssyqa created https://github.com/llvm/llvm-project/pull/120526 The limits.h of glibc, aka /usr/include/limits.h file of *-linux-gnu systems, has `#include_next `, so the limits.h from clang is included. And in the limits.h for clang, `#include_next ` is also used. Normall