* Re: The defs.h / common-defs.h / server.h rule [not found] <2f2f552a-0028-49cc-adbe-ca038c1e9acd@polymtl.ca> @ 2024-02-20 12:20 ` Andrew Burgess 2024-02-20 14:52 ` Tom Tromey 1 sibling, 0 replies; 7+ messages in thread From: Andrew Burgess @ 2024-02-20 12:20 UTC (permalink / raw) To: Simon Marchi, Simon Marchi via Gdb-patches Simon Marchi <simon.marchi@polymtl.ca> writes: > Hi all, > > We currently have a rule [1] saying that all .c/.cc file must include > one of defs.h, common-defs.h or server.h, depending on where they are, > and that no header should include them. > > I was wondering about why that rule exists, and if it is still relevant > today. > > The reason I am asking is that I'm usually working on GDB in an editor > that uses clangd behind the scenes, which is really good at showing > accurate errors and warnings in real time. Unfortunately, the > experience is bad for headers when headers don't include what they use. > Since headers in GDB don't include defs.h, but use a lot of things > defined there (or in files included transitively), editing headers in > GDB is really bad. > > Could we switch to having headers include what they use? +1 from me. > There might be > some edge cases where we can't do that with header files used both in > GDB and GDBserver context, We might be able to solve these problems by moving the "whatever" feature into a 'whatever.h' header and then having gdbserver/whatever.h and gdb/whatever.h, then depending on how the file is being compiled we'd pick up the correct version of the 'whatever.h' header. Thought this might not help with your editor as I guess it's searching the whole tree for header files, and would likely go with the first one it found... but ignoring you're editor for a moment, having this split might avoid the need for the .c/.cc file to pull the header for us. Thanks, Andrew > but it would still be nice to make the > experience better for all header files used in only one (which is the > majority). > > As an example, I'm attaching some before / after screenshots of what it > looks like when editing objfiles.h before including defs.h at the top > and after. > > Before someome says "it's your tools' fault, your tools are bad": I > know. There has been some requests [2] to improve clangd for > non-self-contained headers. Having a feature like that would help here, > but I don't think I can expect it any time soon, and I don't have the > skills required to do it. OTOH, I think that GDB's style here is a bit > odd and goes against the typical Good Practices (tm). > > Simon > > [1] https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Include_Files > [2] https://github.com/clangd/clangd/issues/1534 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: The defs.h / common-defs.h / server.h rule [not found] <2f2f552a-0028-49cc-adbe-ca038c1e9acd@polymtl.ca> 2024-02-20 12:20 ` The defs.h / common-defs.h / server.h rule Andrew Burgess @ 2024-02-20 14:52 ` Tom Tromey 2024-02-20 15:14 ` Tom Tromey 2024-02-23 20:35 ` Simon Marchi 1 sibling, 2 replies; 7+ messages in thread From: Tom Tromey @ 2024-02-20 14:52 UTC (permalink / raw) To: Simon Marchi; +Cc: Simon Marchi via Gdb-patches >>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes: Simon> We currently have a rule [1] saying that all .c/.cc file must include Simon> one of defs.h, common-defs.h or server.h, depending on where they are, Simon> and that no header should include them. Simon> I was wondering about why that rule exists, and if it is still relevant Simon> today. Every source file has to include config.h first. In other GNU-ish projects this is done in a straightforward way, but gdb (like gcc) used a kind of prologue header file instead. Simon> Could we switch to having headers include what they use? Do you mean just adding defs.h to every header, or something else? Simon> Before someome says "it's your tools' fault, your tools are bad": I Simon> know. In addition to that, I wonder how to enforce this rule. For example I'm not likely to start using clangd. Or if clangd is changed, will the rule change? Tom ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: The defs.h / common-defs.h / server.h rule 2024-02-20 14:52 ` Tom Tromey @ 2024-02-20 15:14 ` Tom Tromey 2024-02-23 20:35 ` Simon Marchi 1 sibling, 0 replies; 7+ messages in thread From: Tom Tromey @ 2024-02-20 15:14 UTC (permalink / raw) To: Tom Tromey; +Cc: Simon Marchi, Simon Marchi via Gdb-patches Simon> Could we switch to having headers include what they use? Tom> Do you mean just adding defs.h to every header, or something else? To be clear here, I think adding defs.h (or whatever) to every header seems relatively harmless. So if that would help you, it seems ok. Simon> Before someome says "it's your tools' fault, your tools are bad": I Simon> know. Tom> In addition to that, I wonder how to enforce this rule. Tom> For example I'm not likely to start using clangd. Tom> Or if clangd is changed, will the rule change? ... just that I suspect you'll end up having to remind me of the rule from time to time. Tom ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: The defs.h / common-defs.h / server.h rule 2024-02-20 14:52 ` Tom Tromey 2024-02-20 15:14 ` Tom Tromey @ 2024-02-23 20:35 ` Simon Marchi 2024-02-23 21:06 ` Tom Tromey 1 sibling, 1 reply; 7+ messages in thread From: Simon Marchi @ 2024-02-23 20:35 UTC (permalink / raw) To: Tom Tromey; +Cc: Simon Marchi via Gdb-patches On 2024-02-20 09:52, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes: > > Simon> We currently have a rule [1] saying that all .c/.cc file must include > Simon> one of defs.h, common-defs.h or server.h, depending on where they are, > Simon> and that no header should include them. > > Simon> I was wondering about why that rule exists, and if it is still relevant > Simon> today. > > Every source file has to include config.h first. In other GNU-ish > projects this is done in a straightforward way, but gdb (like gcc) used > a kind of prologue header file instead. Ok, so through defs.h, every source file in gdb/ for instance ends up including: - gdbsupport/config.h - gnulib/config.h - gdb/config.h Plus, in gdbsupport/common-defs.h (or transitively included) are some definitions that we want all source files to see all the time. For instance, the stuff in poison.h, or our override of ATTRIBUTE_PRINTF. I suppose the other classic way to do this is with an -include flag, passed to the compiler? Actually that might be a better way to solve my problem. In order to correctly analyze some headers in a standalone way, it is indeed necessary for clangd to include the various config.h files first (and perhaps other stuff from defs.h & co). If we used -include to include whatever we want all code to see all the time, then clangd would see the -include flag in the compilation flags and include those "prologue" headers when analyzing any other header. I gave it a try just for fun, it seems to work nicely: - include config.h files with -include https://review.lttng.org/c/binutils-gdb/+/11857/1 - move more things to gdbsupport.inc.h https://review.lttng.org/c/binutils-gdb/+/11858/2 I like this solution because (a) it helps my problem and (b) it makes defs.h / common-defs.h / server.h no longer special. All things that we want all source files to see all the time are included by that -include flag, so enforced by the build system, and for the rest, source files / header files include what they need (e.g. if they don't need defs.h, they don't have to include it). defs.h & co pull in a ton of stuff that is not needed everywhere, so I think it would be nice to gradually remove some includes from them, and include things where they are really needed. This could help build times and avoid recompiling files unnecessarily. I'm not sure my argument for such a change is very strong, but I would be happy to see a change like that. > Simon> Could we switch to having headers include what they use? > > Do you mean just adding defs.h to every header, or something else? That's what I thought at first (either defs.h, or whatever include files they use). But it's not possible to include defs.h / common-defs.h / server.h in the header files that defs.h / common-defs.h / server.h themselves include. So for those include files, I would be stuck. > Simon> Before someome says "it's your tools' fault, your tools are bad": I > Simon> know. > > In addition to that, I wonder how to enforce this rule. > For example I'm not likely to start using clangd. > Or if clangd is changed, will the rule change? I don't think what I propose is really clangd specific. I think it's just to design headers that can be analyzed in a self-contained way, to the extent possible, which I think is just good software design. Do people use other editors / IDEs that give accurate compilation errors in real time like clangd does? How do they do for header files that assume they've been included in some context? Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: The defs.h / common-defs.h / server.h rule 2024-02-23 20:35 ` Simon Marchi @ 2024-02-23 21:06 ` Tom Tromey 2024-02-23 21:55 ` Simon Marchi 0 siblings, 1 reply; 7+ messages in thread From: Tom Tromey @ 2024-02-23 21:06 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, Simon Marchi via Gdb-patches Simon> I suppose the other classic way to do this is with an -include flag, Simon> passed to the compiler? I don't know if any projects do this, but traditionally it's not done by GNU-ish programs, I guess because -include was GCC-specific, i.e., not portable. Simon> defs.h & co pull in a ton of stuff that is not needed everywhere, so I Simon> think it would be nice to gradually remove some includes from them, and Simon> include things where they are really needed. This could help build Simon> times and avoid recompiling files unnecessarily. Yeah, that'd be great. gdb's header files are a crazy mess. Tom ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: The defs.h / common-defs.h / server.h rule 2024-02-23 21:06 ` Tom Tromey @ 2024-02-23 21:55 ` Simon Marchi 2024-02-27 14:54 ` Tom Tromey 0 siblings, 1 reply; 7+ messages in thread From: Simon Marchi @ 2024-02-23 21:55 UTC (permalink / raw) To: Tom Tromey; +Cc: Simon Marchi via Gdb-patches On 2024-02-23 16:06, Tom Tromey wrote: > Simon> I suppose the other classic way to do this is with an -include flag, > Simon> passed to the compiler? > > I don't know if any projects do this, but traditionally it's not done by > GNU-ish programs, I guess because -include was GCC-specific, i.e., not > portable. Do you think it would be acceptable today? As far as I know, people only build gdb with gcc and clang. But I checked other compilers I know: - icc has -include (it uses the clang frontend now, I think) - xlc has -qinclude (although it seems like there's a version with a clang frontend as well which supports -include too) So, in this day and age, I'd be surprised if anyone compiled gdb with a compiler that doesn't have some equivalent of -include. Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: The defs.h / common-defs.h / server.h rule 2024-02-23 21:55 ` Simon Marchi @ 2024-02-27 14:54 ` Tom Tromey 0 siblings, 0 replies; 7+ messages in thread From: Tom Tromey @ 2024-02-27 14:54 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, Simon Marchi via Gdb-patches >>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes: Simon> On 2024-02-23 16:06, Tom Tromey wrote: Simon> I suppose the other classic way to do this is with an -include flag, Simon> passed to the compiler? >> I don't know if any projects do this, but traditionally it's not done by >> GNU-ish programs, I guess because -include was GCC-specific, i.e., not >> portable. Simon> Do you think it would be acceptable today? As far as I know, people Simon> only build gdb with gcc and clang. I've tried to think of a reason we shouldn't do it, but I haven't really come up with anything compelling. It's hard to really know if it would cause problems for someone. However it seems to me that you could do the experiment. Worst case, we'd have to back it out any maybe add some #includes here and there. Tom ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-02-27 14:54 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <2f2f552a-0028-49cc-adbe-ca038c1e9acd@polymtl.ca> 2024-02-20 12:20 ` The defs.h / common-defs.h / server.h rule Andrew Burgess 2024-02-20 14:52 ` Tom Tromey 2024-02-20 15:14 ` Tom Tromey 2024-02-23 20:35 ` Simon Marchi 2024-02-23 21:06 ` Tom Tromey 2024-02-23 21:55 ` Simon Marchi 2024-02-27 14:54 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).