On Thu, 14 Mar 2024 22:09:55 GMT, Alex Menkov <amen...@openjdk.org> wrote:
>> It does not matter what the `ClassFileParser` does. >> >>> ` JvmtiClassFileReconstituter` performs the reverse operation. >> >> True. It should know how to put the `attribute_count` value into the class >> file but it does not need to know how to calculate its value. What I do not >> like in your model is that there is no one single place which knows how to >> calculate this value and the existing and potential consumers should have >> this knowledge. >> >>> They may have different `attributute_count `(that's what this bug about). >> >> The bug is that the `attributute_count` field value from the class file >> stored in the `RecordComponent` does not match the calculated value because >> the invisible attribute was not saved (was ignored). My suggestion is not to >> store the `attributute_count` field value from the class file as it was >> before. The suggestion is to place the calculating function >> `attributute_count()` into the `RecordComponent` class. > >> True. It should know how to put the `attribute_count` value into the class >> file but it does not need to know how to calculate its value. What I do not >> like in your model is that there is no one single place which knows how to >> calculate this value and the existing and potential consumers should have >> this knowledge. > > It **must** know how to calculate the value. Because this is number of > `attribute_info_attributes` attributes that follow. And only > `JvmtiClassFileReconstituter` knows how many attributes it's going to write. > >> The bug is that the `attributute_count` field value from the class file >> stored in the `RecordComponent` does not match the calculated value because >> the invisible attribute was not saved (was ignored). > > I'd consider this as design issue with the current implementation - > `JvmtiClassFileReconstituter` gets the value from external source > (`RecordComponent`) and uses it without validation. > This approach is inconsistent with other `JvmtiClassFileReconstituter` code. > For other similar cases it calculates record counts in place (from size of > arrays, string length, from list enumerator, etc.). So this makes sense. You only want the number of attributes that are applicable to the RecordComponent, and that is the three attributes that you then write out in the class file reconstituter: generic_signature_index, annotations and type_annotations. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18161#discussion_r1539950465
