ClassReader and BufWriter

[Brian Goetz]

On 2/9/2023 9:28 AM, Maurizio Cimadamore wrote:
>
> On 09/02/2023 14:15, Brian Goetz wrote:
>>
>>
>> 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".
> Sure, but DirectClassBuilder is an implementation class. I'm talking 
> about API usages here. It seems to me that BufWriter is an 
> implementation abstraction that "happens" to implement the interface 
> for attribute writing (which is the only place where the user needs to 
> write things by hand).

It may be the case at this point that the only place that BufWriter 
pokes its head out in the APIs is in the attributes (that wasn't always 
true), but it would definitely be wrong to put BufWriter in the 
attribute package -- it has nothing to do with attributes, and it is 
entirely imaginable that it could show up elsewhere again.

You could argue it is a "utility" abstraction and therefore goes in a 
'util' package, and that would be a fair argument.  I'm not sure whether 
it buys us much in terms of API approachability, but I wouldn't object.  
(Though the current `util` package is more user-facing utilities, like 
ClassPrinter, and has only one thing in it.)

Other things that could be moved elsewhere:

  - The ClassFile-specific elements -- Superclass, Interfaces, 
ClassfileVersion, maybe AccessFlags (which is shared with method and 
field models).  But, where?  And there's just not many of them.
  - Instruction-related classes -- Instruction, PseudoInstruction, 
Opcode could go into `instruction` (though its a little weird for the 
Element types (Instruction) to be elsewhere, which leaves just Opcode as 
a candidate)
  - AttributeMapper and Attributes could go into `attribute`
  - BootstrapMethodEntry could go into `constantpool`
  - *Transform could go into a `transform` package, but that severs the 
Xxx{Model,Element,Builder,Transform} symmetry, and there's just a 
handful of them

A bigger reorg would put all the Model and Element types into a `model` 
package.  This might achieve the outcome I think you are looking for -- 
that when someone looks at the top level javadoc, they see the key entry 
points.