On Thu, 16 Feb 2023 10:25:04 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org> 
wrote:

>> I'm not quite sure what exactly do you propose.
>> `ConstantPool` should not accept anything as it is read-only, so "accept" 
>> would be confusing.
>> `ConstantPoolBuilder::maybeClone` is rather a `Function`, where the name 
>> might be changed to `ConstantPoolBuilder::apply`.
>> However there are so many "accepts" and "applies" across the API, that 
>> reducing API verbosity to just these functional terms might significantly 
>> decrease readability.
>
> As I read the javadoc (I have not looked at impl yet), this method can 
> effectively be used to add an entry to the constant pool (if the entry does 
> not yet exist - in which case existing entry is returned).
> 
> After having looked at the implementation a bit, I think my assumption is 
> correct - e.g. when calling `getfield` on a code builder, the FieldRef passed 
> to the code builder is added to the constant pool using `maybeClone`.
> 
> So, in my mind at least, this `maybeClone` is the main avenue by which new 
> constant pool entries are added. In builders we have a `with` method - I 
> think a `with` method would also be ok here, given what the method does (ok, 
> there is the wrinkle that, if the entry already exists it is not added, but 
> that seems more of an optimization to avoid duplicates).

The main difference is that if the given entry is not directly applicable (when 
it comes from 3rd constant pool or when the pool is not shared during 
transformation) - a new or matching entry to the target pool is returned.
This is not a builder pattern, this is projection of an entry to the target CP 
and it also prevents duplicates.

-------------

PR: https://git.openjdk.org/jdk/pull/10982

Reply via email to