On Thu, 14 Mar 2024 00:12:43 GMT, Alex Menkov <amen...@openjdk.org> wrote:
>> The `attributute_count ` is a property of the `RecordComponent` even though >> it is not stored but calculated. >> The `JvmtiClassFileReconstituter` should have a minimal knowledge about this >> property and how it is calculated, the same as any other consumer of the >> `RecordComponent`. Thre `RedefineClasses` in the future may need this number >> as well. Would it be also its responsibility to calculate it? > > `attributute_count` is a property of `component_info` Record's attribute in > the class file (it contains length of the following > `attribute_info_attributes` array). > `RecordComponent` is VM representation of Record class attribute and contains > data required by VM in a form that is useful for the VM. > `RecordComponent` knows nothing `attribute_info_attributes` array, so it does > not need to know its length. > > ClassFileParser parses `attribute_info_attributes` array and convert it to > corresponding fields of `RecordComponent`. > `JvmtiClassFileReconstituter` performs the reverse operation. > They may have different `attributute_count` (that's what this bug about). > If `RedefineClasses` needs the value, the code may be updated (it depends on > what value it needs - from original class file or from reconstituted class > file) 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. >> Could you add a short comment, please? > > Added. Thanks. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18161#discussion_r1524116453 PR Review Comment: https://git.openjdk.org/jdk/pull/18161#discussion_r1524117292
