From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 755A23858C53 for ; Fri, 23 Feb 2024 20:35:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 755A23858C53 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=polymtl.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 755A23858C53 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=132.207.4.11 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708720538; cv=none; b=loj34CsD1kei3g5shgR/BtIsywTWZ2+X/fa6Afnw2tPsgn6n2XmcWJzDOtDeiRS6stmUenXwNmNy/K01uUxs2vYIKe7x4OCHvR8bdL3yPXm2z0eb5oyq/Xy3Tu6ANgM1xWMjbuCcVlKqCR27HXjQIck5e1y919GP7F5VuIsE9q0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708720538; c=relaxed/simple; bh=BKRvInz2J3Tygcvq7I/EepR0Rl7dOd5NJ8yEy3HtzqY=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=CZwtvZK+Z+FRmFUXe3p5iLqD5RkNf+MGQxhDo8f8hphDnqV/QbHbKtv0diNy8LaOhxfV7raj3+XXWmHBoW1CSR0WwtMV582m+f2CN4ot61tfXpkcifXtYoN4OC4TIKLemZSzVIBpg3kf/rGBgA8onGMjIUq1HKnqgp1wyWPQleA= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 41NKZUvH001150 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 23 Feb 2024 15:35:35 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 41NKZUvH001150 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1708720536; bh=gQ+eEOQ0mlm45ZUf7qMkEoohx3lAMfP/jpgzP9qhMJU=; h=Date:Subject:To:Cc:From:In-Reply-To:From; b=DIoQsWj7TKPAwu76qT0wU1iBhYpKsdQHjw+B1vNJ192dXnBbzMT9mzNrKPgz8ykh3 kRbtFthquNcQc5Drp70O0t8OlghXvO5KZ+evPANeTRjdqVq5m3JKyfe4rxrwke3Dfr rmurnYGBMT/oZvJFYRyJ5AJppktqPG/IZvrSnfUc= Received: from [10.0.0.11] (modemcable238.237-201-24.mc.videotron.ca [24.201.237.238]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 9C4511E030; Fri, 23 Feb 2024 15:35:30 -0500 (EST) Message-ID: <88f15917-7989-4e47-a7cf-2cb62faad7ad@polymtl.ca> Date: Fri, 23 Feb 2024 15:35:30 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: The defs.h / common-defs.h / server.h rule Content-Language: en-US To: Tom Tromey Cc: Simon Marchi via Gdb-patches References: <2f2f552a-0028-49cc-adbe-ca038c1e9acd@polymtl.ca> <87zfvv2non.fsf@tromey.com> From: Simon Marchi In-Reply-To: <87zfvv2non.fsf@tromey.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Fri, 23 Feb 2024 20:35:30 +0000 X-Spam-Status: No, score=-3031.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2024-02-20 09:52, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi 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