From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Spencer Baugh Newsgroups: gmane.emacs.bugs Subject: bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros Date: Sat, 08 May 2021 09:35:31 -0400 Message-ID: <87czu1pcv0.fsf@catern.com> References: <877dkbsj9d.fsf@catern.com> <20210506213346.9730-16-sbaugh@catern.com> <835yzudcvz.fsf@gnu.org> <87o8dmr96v.fsf@catern.com> <83tunebsiu.fsf@gnu.org> <87mtt6p6co.fsf@catern.com> <83sg2xaf4l.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="10020"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 48264@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat May 08 15:36:22 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lfN82-0002Ua-DU for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 08 May 2021 15:36:22 +0200 Original-Received: from localhost ([::1]:34292 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lfN81-0005Mo-GZ for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 08 May 2021 09:36:21 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:34074) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lfN7j-0005Lt-Pr for bug-gnu-emacs@gnu.org; Sat, 08 May 2021 09:36:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:38816) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lfN7i-0001sj-Cl for bug-gnu-emacs@gnu.org; Sat, 08 May 2021 09:36:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1lfN7h-0001NY-OL for bug-gnu-emacs@gnu.org; Sat, 08 May 2021 09:36:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 08 May 2021 13:36:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 48264 X-GNU-PR-Package: emacs Original-Received: via spool by 48264-submit@debbugs.gnu.org id=B48264.16204809385292 (code B ref 48264); Sat, 08 May 2021 13:36:01 +0000 Original-Received: (at 48264) by debbugs.gnu.org; 8 May 2021 13:35:38 +0000 Original-Received: from localhost ([127.0.0.1]:50360 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lfN7J-0001NI-Pm for submit@debbugs.gnu.org; Sat, 08 May 2021 09:35:38 -0400 Original-Received: from venus.catern.com ([68.183.49.163]:33124) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lfN7H-0001NC-PX for 48264@debbugs.gnu.org; Sat, 08 May 2021 09:35:36 -0400 Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=98.7.229.235; helo=localhost; envelope-from=sbaugh@catern.com; receiver= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=catern.com; s=mail; t=1620480932; bh=bCLCuHnweiJw2p3H7bn/lQsciHEWM7EWqBVJyrdORgs=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=SFmXNBeQCD/gWegTrOdNblzuySz0NMFOQNaWTexck7/M0oUCOV1aSaK+j7S3NU437 PhLS/5VrGQCp34/JblBqLTvn06PiraoZEBvOlabX23/Gb/vY5N+2eLmoRk0he/miiZ nr7Zhdc0TY5wFM7+Qrg7vEsF1bDRg+KP5olW41Vw= Original-Received: from localhost (cpe-98-7-229-235.nyc.res.rr.com [98.7.229.235]) by venus.catern.com (Postfix) with ESMTPSA id 48CB42E95D8; Sat, 8 May 2021 13:35:32 +0000 (UTC) In-Reply-To: <83sg2xaf4l.fsf@gnu.org> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:206016 Archived-At: Eli Zaretskii writes: >> From: Spencer Baugh >> Cc: 48264@debbugs.gnu.org >> Date: Fri, 07 May 2021 17:43:51 -0400 >> >> > If the sole purpose is to be able to detect coding mistakes, then >> > there are other possibilities to do that, if the compiler cannot help >> > in a way that leaves the sources readable. >> >> Hopefully. Although, I'm not sure this approach is fundamentally >> unreadable? The field names are already mangled with the trailing "_" >> to stop direct access; this is just further mangling them. > > Yes, but it's much more than just the appended _. > > Consider also the case of some developer instructing a user to provide > values of these fields in a GDB session: currently we need to tell the > user to use just "p foo->bar_". With this change, we'd need to make > the user type much more, and possibly also make sure Emacs is compiled > with -g3 to have the macros available to the debugger. Yes, that's fair. >> >> No, this is purely just changing the name of the fields - it has no >> >> impact on functionality, C code can still set the buffer-local >> >> variables. >> > >> > Then I guess the _defaulted_ part is a misnomer? >> >> Possibly; by "defaulted" I intended to mean that the field is one which >> has a default. But I freely acknowledge it's not a great name. > > So how about using _d_ of _def_instead? It's much shorter and > expresses the purpose no worse than _defaulted_. Sure, that would work. >> Keep in mind though, this name isn't exposed to the programmer >> anywhere - it might as well be _ABCDEFGHI_, nothing will change >> outside the definition of the BVAR_DEFAULTED_FIELD macro. > > See above: I'd prefer to get rid of the macro for this purpose. Sure, we could mostly get rid of it, although it's important that the argument to BVAR_OR_DEFAULT be "case_fold_search" rather than, say, "case_fold_search_def", even if the field is named the latter. Otherwise one might accidentally call BVAR with "case_fold_search_def", which would compile but behave wrong at runtime - and preventing that is the whole point of the different names. >> > Failing that, maybe we should simply have a test to detect the >> > mistakes? That wouldn't prevent bad code from being compiled, but it >> > should reveal it soon enough, since tests are regularly run on hydra. >> >> A conditionally-compiled runtime check would be very easy to add - I'd >> just change BVAR to something like: >> >> (eassert (EQ (buffer_defaults->field ## _)); (buf)->field ## _) >> >> Which would make sure that it's not used on anything with a default. >> But of course that's substantially more annoying than a compile time >> check... > > I'm not sure I understand why this is much more annoying, can you > elaborate? We have similar assertions, conditioned on > ENABLE_CHECKING, elsewhere in our macros, like XWINDOW etc, so why not > here? I mean that it's annoying that merely compiling doesn't detect the usage error, one has to actually run tests. Since it seems like an easy mistake to make, that seems like it would be frustrating. But of course it's better than nothing and we can make it compile time later on. If you think such a conditionally-compiled runtime check would be acceptable for applying these changes, I can go ahead and write that. I've actually just convinced myself that going with a runtime check at first and figuring out a compile time check later is a good way to go.