[lworld] RFR: 8247357: Flattenable field concept needs some cleanup
john.r.rose at oracle.com
Wed Jun 10 19:37:56 UTC 2020
[consolidate and resend after name fix]
On Jun 10, 2020, at 11:55 AM, Frederic Parain <fparain at openjdk.java.net> wrote:
> Please review these changes cleaning up the flattenable field concept.
> The concept has evolved with time and now all fields with an inline type are by definition flattenable, so the need to
> have a "flattenable bit" on the side is gone. The changeset contains a mix of renaming and code cleaning. The changes
> don't include JIT code, which would be fix in a follow-up patch.
I’m glad to see occurrences of “value” go away in favor of “inline”.
(The language gurus won’t absolutely promise that “inline” is the
final word on terminology, but I think at least for the JVM code,
“inline” is a far more descriptive term than “value”.)
The “is_flattenable” bit was often accompanied by an “is_flattened”
bit which reported whether the intended flattening actually took place.
Having their names be similar helped make the code understandable.
So I suggest renaming those guys also, since you are touching all
that code now.
The term “is_inline” is ambiguous when reading the code. Where there
is any doubt about whether it means “is intended to be inlined” vs.
“is actually inlined in the layout”, the term should be made more explicit.
So I suggest:
Maybe that’s overkill? But I think just “is_inline” is not clear enough.
P.S. To be clear: I’m not suggesting that systematically, just where
the distinction exists between “could be” and “actually is” flattened.
Names like STATIC_INLINE in the CFP are completely unambiguous;
they shouldn’t be STATIC_ALLOCATED_INLINE or the like.
In the CFP, “has_flattenable_fields” could go either way, but I think
“has_declared_inline_fields” would be safer. It’s not clear whether it
means “my definer has declared inline fields in me”, or “I actually
flattened one or more of my fields”.
More information about the valhalla-dev