Thread
-
s/pg_attribute_always_inline/pg_always_inline/?
Andres Freund <andres@anarazel.de> — 2026-04-08T21:09:12Z
Hi, When reviewing the index prefetching patch I got a bit dismayed at how long pg_attribute_always_inline is due to the way it triggers pgindent to format stuff. I propose that we remove the _attribute_ part of the name. Given that it is implemented for compilers that don't use the __attribute__((always_inline)) spelling, so the shorter name seems better anyway. Thoughts? Greetings, Andres Freund
-
Re: s/pg_attribute_always_inline/pg_always_inline/?
Peter Geoghegan <pg@bowt.ie> — 2026-04-08T21:30:21Z
On Wed, Apr 8, 2026 at 5:09 PM Andres Freund <andres@anarazel.de> wrote: > When reviewing the index prefetching patch I got a bit dismayed at how long > pg_attribute_always_inline is due to the way it triggers pgindent to format > stuff. I had to significantly reorder function prototypes to avoid overly long prototypes that had pg_attribute_always_inline. While perhaps not strictly necessary, we shouldn't have to work around the fact that pg_attribute_always_inline is an absurdly verbose symbol name. > I propose that we remove the _attribute_ part of the name. +1 -- Peter Geoghegan
-
Re: s/pg_attribute_always_inline/pg_always_inline/?
Peter Eisentraut <peter@eisentraut.org> — 2026-04-09T00:00:37Z
On 08.04.26 23:09, Andres Freund wrote: > Hi, > > When reviewing the index prefetching patch I got a bit dismayed at how long > pg_attribute_always_inline is due to the way it triggers pgindent to format > stuff. > > I propose that we remove the _attribute_ part of the name. > > Given that it is implemented for compilers that don't use the > __attribute__((always_inline)) spelling, so the shorter name seems better > anyway. Yes, and the existing name is also kind of wrong even on GCC because the macro does not expand to only an attribute but also includes the "inline".
-
Re: s/pg_attribute_always_inline/pg_always_inline/?
Andres Freund <andres@anarazel.de> — 2026-04-09T14:40:30Z
Hi, On 2026-04-09 02:00:37 +0200, Peter Eisentraut wrote: > On 08.04.26 23:09, Andres Freund wrote: > > Hi, > > > > When reviewing the index prefetching patch I got a bit dismayed at how long > > pg_attribute_always_inline is due to the way it triggers pgindent to format > > stuff. > > > > I propose that we remove the _attribute_ part of the name. > > > > Given that it is implemented for compilers that don't use the > > __attribute__((always_inline)) spelling, so the shorter name seems better > > anyway. > > Yes, and the existing name is also kind of wrong even on GCC because the > macro does not expand to only an attribute but also includes the "inline". It didn't start out that way, that only came in a bit later, in 434e6e148441... Created a CF entry, to reduce the chances of me forgetting about committing this early in the 20 cycle. Greetings, Andres Freund
-
Re: s/pg_attribute_always_inline/pg_always_inline/?
Peter Geoghegan <pg@bowt.ie> — 2026-05-27T22:17:20Z
On Thu, Apr 9, 2026 at 10:40 AM Andres Freund <andres@anarazel.de> wrote: > Created a CF entry, to reduce the chances of me forgetting about committing > this early in the 20 cycle. We already have a pg_noinline. How about renaming pg_attribute_always_inline to pg_mustinline? That is an alternative that is both consistent with pg_noinline, and even terser than your proposal. I have no intention of holding this patch up with bikeshedding. But I noticed that even your proposed pg_always_inline rename still leaves function prototypes over the column limit with moderately verbose function names. It seems better to avoid that outcome. -- Peter Geoghegan
-
Re: s/pg_attribute_always_inline/pg_always_inline/?
Tomas Vondra <tomas@vondra.me> — 2026-06-12T15:22:58Z
On 5/28/26 00:17, Peter Geoghegan wrote: > On Thu, Apr 9, 2026 at 10:40 AM Andres Freund <andres@anarazel.de> wrote: >> Created a CF entry, to reduce the chances of me forgetting about committing >> this early in the 20 cycle. > > We already have a pg_noinline. How about renaming > pg_attribute_always_inline to pg_mustinline? That is an alternative > that is both consistent with pg_noinline, and even terser than your > proposal. > I agree we should shorten pg_attribute_always_inline, it's way too verbose. And other attributes don't include the _attribute_ either (like the pg_noinline mentioned here). I'm not sure about pg_mustinline. It seems weird to me, and I'm not sure saving the 3 characters is worth it, pg_always_inline seems better. > I have no intention of holding this patch up with bikeshedding. But I > noticed that even your proposed pg_always_inline rename still leaves > function prototypes over the column limit with moderately verbose > function names. It seems better to avoid that outcome. > Yeah, we should not do such weird stuff just because of unnecessarily long attribute names. Question - do we plan to do this in master only, or was the plan to backpatch the change? I'm not sure if these labels are used outside the core code, that might be an issue for backpatching. regards -- Tomas Vondra
-
Re: s/pg_attribute_always_inline/pg_always_inline/?
Peter Geoghegan <pg@bowt.ie> — 2026-06-12T16:10:39Z
On Fri, Jun 12, 2026 at 11:23 AM Tomas Vondra <tomas@vondra.me> wrote: > I'm not sure about pg_mustinline. It seems weird to me, and I'm not sure > saving the 3 characters is worth it, pg_always_inline seems better. I'm not going to make a fuss about it. > Yeah, we should not do such weird stuff just because of unnecessarily > long attribute names. Right. Basically, I don't want to be told that I must completely change the order of function definitions because I used pg_[attribute]_always_inline. It's just not reasonable to impose that requirement on patch authors. > Question - do we plan to do this in master only, or was the plan to > backpatch the change? I'm not sure if these labels are used outside the > core code, that might be an issue for backpatching. I think that we should bite the bullet and backpatch. I count only 17 instances of pg_attribute_always_inline on the master branch. Some extensions will no longer build against the backbranches if we go this way. However, extension authors should find it easy to work around this on an ad-hoc basis. They're going to have to work around it sooner or later, so we might as well favor the new spelling. -- Peter Geoghegan