[Swan-dev] type_t, type() and TYPE() in initializers

[D. Hugh Redelmeier]

| From: Andrew Cagney <andrew.cagney at gmail.com>

| I'm sure this has come up before -  the discussion concluded that a
| cast within a static initializer probably isn't technically static.

A compound literal starts with what looks like a cast but it isn't a
cast.

On the other hand, a compound literal actually creates an object, not
a value.  I hadn't realized this.  Not to my taste but potentially
useful.

It doesn't make sense to initialize a static with an assignment from
an object.  Amusingly, it does make sense to initialize a static with
the address of a compound literal!

I think GCC makes sense of initializing a static with a compound literal 
unless it has told to be strict in some way.  We always want GCC 
strictness so more of our errors will be caught.

| Through trial and error (read, being repeatedly pinged for the same
| mistake on IRC cf eb3ba97984910933b3ce9ff3155c2932b48597a0), I've
| found the following works best:
| 
|  - lower case function or variable - here chunk(), empty_chunk, ... -
| use this in all dynamic code
| -  upper case macro - here CHUNK(), EMPTY_CHUNK, ... - use this in all
| static code

I take it by "use this in all static code" you really mean "use this
as an initializer".  In other words, it isn't an expression.

That's not our past convention and I don't immediately see why to use
it.

We've used upper-case for constants and for macros that don't quite
work like functions.

- TRUE / FALSE [change to lower case still under consideration]

- LEMPTY

- MAX / MIN

REALTIME_EPOCH is not a constant: it cannot be used in a normal
expression.  It is an initializer.  And it is used only once: hardly
worth defining.  But writing it in upper case does conform: it doesn't
work like a function.

REALTIME_EPOCH is used as the initializer for the definition of the
constant object "realtime_epoch".  realtime_epoch is used once.

It would be quite reasonable to define REALTIME_EPOCH as a compound
literal.  And then use it instead of realtime_epoch.  It would still
only be used once.

| When I tried to cast something like EMPTY_CHUNK so that it, instead of
| empty_chunk, could be used (a foolish premature optimisation on my
| part), I was invariably forced to either:
| 
| - define a further macro with no cast - above avoids needing to do this
| - or hard-wire those cases - as has happened here - defeating the
| point of the abstraction

I don't understand this point.

EMPTY_CHUNK is a compound literal.  It works everywhere empty_chunk
would work.  Neither would work as a static initializer.

There is one exception.  The address of empty_chunk would always be
the same whereas the address of EMPTY_CHUNK would always be different.
But I don't think any of our code would care.

| I don't think performance and/or static analysers are an issue:
| 
|  - given their price, I'd be really surprised if a static analyser,
| which looks at the entire code base, couldn't figure out that:
|   return empty_chunk;
| and
|   return (chunk_t) { NULL,0, }
| are the same

As far as I know we don't run the compiler in "whole program" mode.

The static analyzer that we do use can do a better job with the
EMPTY_CHUNK version than the empty_chunk version.

Better means: potentially better code and potentially better
diagnostics.

I would expect slightly better object code if every rvalue-only use of
empty_chunk were replaced by EMPTY_CHUNK.

| - pluto spends huge amounts of time executing assembler computing DH
| and RSA,, the above is in the wash

For time, yes.  But given the size of the NSS library, it is probably
true for space too (NSS might be shared with other processes).

| OTOH,  I can print empty_chunk (but not EMPTY_CHUNK) and can reliably
| breakpoint and step over a chunk() function (but not the alternatives,
| at least not reliably)

Why would you want to?  These are tiny little transparent operators.
You know from inspection just what they do.

| Andrew
| 
| PS: CHUNK() wasn't a bug, it just wasn't used

Nor documented.  So I assumed that it was to be an expression.  Not an
initializer.

If we're to define initializer macros, I suggest we need a naming
convention to distinguish them from expression macros.  CHUNK_INIT is
a long name but we don't do this often enough to justify inventing and
memorizing something shorter.

How many initializer macros do we have at this point?  REALTIME_EPOCH
is all I know about right now.