Date: Tue, 16 Oct 2018 09:06:12 -0700
From: Andres Freund <andres@anarazel.de>
To: Robert Haas <robertmhaas@gmail.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
	Thomas Munro <thomas.munro@enterprisedb.com>
Subject: Re: Large writable variables
Message-ID: <20181016160612.eand666kosvv32cc@alap3.anarazel.de>
References: <20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de>
 <20181016045051.wr35ubb6h7h3ee2b@alap3.anarazel.de>
 <20181016050200.c65yjz2qgxulgouv@alap3.anarazel.de>
 <20181016063013.kigj6e5pqx6n75y3@alap3.anarazel.de>
 <CA+TgmoYbL42rJbn3-fakviMurnnwS+p=rt5iXH8ccP+gQsZdMQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CA+TgmoYbL42rJbn3-fakviMurnnwS+p=rt5iXH8ccP+gQsZdMQ@mail.gmail.com>
Precedence: bulk

On 2018-10-16 11:22:31 -0400, Robert Haas wrote:
> On Tue, Oct 16, 2018 at 2:30 AM Andres Freund <andres@anarazel.de> wrote:
> > This just reminded me that a couple times I wanted a cast that casts
> > away const, but otherwise makes sure the type stays the same. I don't
> > think there's a way to do that in C, but we can write one that verifies
> > the cast doesn't do something bad if gcc is used:
> >
> > #if defined(HAVE__BUILTIN_TYPES_COMPATIBLE_P)
> > #define unconstify(cst, var) StaticAssertExpr(__builtin_types_compatible_p(__typeof(var), const cst), "wrong cast"), (cst) (var)
> > #else
> > #define unconstify(cst, var) ((cst) (var))
> > #endif
> >
> > Does anybody besides me see value in adding a cleaned up version of
> > that?
> 
> Under what circumstances would we consider this to be a legitimate thing to use?

When the variable actually *will not* be modified, but language or API
design reasons makes it unfeasiable to express that. Look e.g.
DestReceiver * CreateDestReceiver(CommandDest dest);
some of the returned receivers (e.g. donothingDR, printsimpleDR) are
statically allocated and *any* modification would be a bug. But other
return values will be modified, e.g. CreateIntoRelDestReceiver().

It's safe to cast constness away if the variable will not actually be
modified after. Which is e.g. the case above. But making the static
allocations const will a) save memory b) trigger sigbuses if you modify
them. So the casting constness away here *increases* robustness.

The problem is that just adding a cast like
		case DestNone:
			return (DestReceiver *) &donothingDR;
also hides errors. If you e.g. changed the type of donothingDR you'd
still not get an error.

So I was wishing for a form of a cast that only casts the const away,
but errors out if there's any other type difference. That's the above, I
think.


> I think if we add something this, we'd better accompany it with some
> detailed and very clearly-written statements about when you're allowed
> to use it.  Otherwise, I predict that people will use it in cases
> where it's not actually safe, and we'll end up with low-grade bugs.

Well, right now people will (and have) just cast the const away like
above. So I don't really see it being more likely to cause problems than
we're doing now.  But yea, it definitely should have a big red warning
label.

Greetings,

Andres Freund