On Thu, 9 Mar 2023 20:49:04 GMT, Frederic Parain <fpar...@openjdk.org> wrote:
>> src/hotspot/share/classfile/classFileParser.cpp line 1491: >> >>> 1489: _temp_field_info = new GrowableArray<FieldInfo>(total_fields); >>> 1490: >>> 1491: ResourceMark rm(THREAD); >> >> Is the ResourceMark ok here or should it go before allocating >> _temp_field_info ? > > _temp_field_info must survive after ClassFileParser::parse_fields() has > returned, so definitively after the allocation of _temp_field_info. That > being said, I don't see any reason to have a ResourceMark here, probably a > remain of some debugging/tracing code. I'll remove it. ok, good. The ResourceMark might be a problem with the GrowableArray if it grows. >> src/hotspot/share/classfile/classFileParser.cpp line 1608: >> >>> 1606: fflags.update_injected(true); >>> 1607: AccessFlags aflags; >>> 1608: FieldInfo fi(aflags, (u2)(injected[n].name_index), >>> (u2)(injected[n].signature_index), 0, fflags); >> >> I don't know why there's a cast here until I read more. If the FieldInfo >> name_index and signature_index fields are only u2 sized, could you pass this >> as an int and then in the constructor assert that the value doesn't overflow >> u2 instead? > > The type of name_index and signature_index is const vmSymbolID, because they > names and signatures of injected fields do not come from a constant pool, but > from the vmSymbol array. ok the cast is fine here. >> src/hotspot/share/oops/fieldStreams.hpp line 104: >> >>> 102: AccessFlags flags; >>> 103: flags.set_flags(field()->access_flags()); >>> 104: return flags; >> >> Did this used to do this for a reason? > > Using the setter rather than the constructor filters out the VM defined flags > and keeps only the flags from the class file. I see, thanks. ------------- PR: https://git.openjdk.org/jdk/pull/12855
