ClassReader and BufWriter

[Brian Goetz]

On 2/9/2023 7:49 AM, Maurizio Cimadamore wrote:
> Hi,
> as I'm looking at the classfile PR (great work!), I have few questions 
> on ClassReader and BufWriter:
>
> * I believe they are only used for attribute reading/writing - so 
> perhaps they should belong to the `attribute` package?

They are used for more than attribute reading and writing. BufWriter is 
used, for example, by DirectClassBuilder::build to build the entire 
classfile -- the ClassFile, constant pool, method_info structures, 
field_info structures, etc.  (Because we don't always know the sizes, to 
minimize buffer copies, we write pieces of the classfile in chunks and 
join them.  DBC::build has two BWs for this, one for the "head" (magic 
number, fixed ClassFile fields, constant pool) and one for the tail 
(fields, methods, attributes), because we don't know how big the CP will 
be until we finish rendering the entire "tail".

A BufWriter would be a pure buffer abstraction -- it is 99% about just 
writing various kinds of data to a growable buffer -- but it does carry 
with it a reference to a constant pool builder (which may be shared 
across multiple BufWriters if they are writing different chunks of the 
same classfile), since it needs to be able to write not only ints, but 
constant pool indexes, and this might require growing the constant pool 
to add a new entry.

> * BufWriter should be renamed ClassWriter (as it's the dual?)

Yes, but no :)  This code got refactored a number of times (including 
one like what you suggest), but the naming asymmetry derives from a 
functional asymmetry.  We write classfiles in lots of small chunks -- we 
accumulate the bytes of constant pools, methods, attributes, etc in 
their own little buffers -- but we read the classfile monolithically.  
(We have to, because you can't even find the methods until you've parsed 
the constant pool.)  So ClassReader is tied to a single byte[] which 
represents the whole classfile, and manages the constant pool, 
navigation, byte access, everything -- whereas BufWriter only knows 
about writing to a buffer, plus uses the service of a 
ConstantPoolBuilder to render constant pool offsets.

So the two do exist as different levels of abstraction.

> While I understand this is in bikeshed territory, I think it would be 
> nice if the number of classes in the main package was kept as low as 
> possible, to let the true nature of the API (models, elements, 
> builders) stand out more.

I did consider this organizational question.  The `attributes` package 
is easy, because it contains almost exclusively model classes for the 
various attributes, which are derived almost mechanically from JVMS.  
You could arguably move a few things like BufWriter (which mostly serve 
the implementation) into a `util` package, but this wouldn't really do 
very much to declutter the main package; there are just a lot of 
different kinds of entities (methods, fields, classes, attributes, 
constant pool entries, instructions, plus elements of the classfile 
header like Superclass) in a classfile, and each gets models, builders, 
elements, transforms, etc.  These feel like they go together.  So it 
felt that this process reached diminishing returns the last time I tried 
this.