Remove allowWrite and allowUnsafeAccess from FieldHints

[jvalkeal]

ReflectionHints with fields on default adds allowWrite while in graal its value is optional and should be set to true if write should be allowed. We should not write this field unless user explicitely defines it so that we can have whatever graal does when field is not present.

{
  "name": "org.example.Clazz",
  "fields": [
    {
      "name": "value",
      "allowWrite": true
    }
  ]
}

Though looking graal sources these fields allowWrite and allowUnsafeAccess has been deprecated since 21.1 meaning we could probably just remove both.

https://github.com/oracle/graal/blob/22a947877c1d1cff07186999d5b9365f09c2466f/sdk/src/org.graalvm.nativeimage/src/org/graalvm/nativeimage/hosted/RuntimeReflection.java#L120-L129

[sreenath-tm]

I'm interested in contributing to this issue, so before I start working it, would you mind sparing your time pointing me to some resources to get started.

[snicoll]

The issue is yours @sreenath-tm but please keep in mind that our next milestone is due in 10 days. If you don't get to it by then, just let us know and we'll take over.

The changes are to be done by removing FieldHint altogether and changing TypeHint.Builder to hold a Set of field names, to be translated to MemberHint in TypeHint's constructor. Further simplifications are due in the predicates but we can polish that as part of merging. Let me know if you have any question.

[sreenath-tm]

The MemberHint does not have a Builder so for replacing the removed FieldHint where can we generate the set of field names that is referenced in the TypeHint class.

[snicoll]

In TypeHint.Builder there is a private final Map<String, FieldHint.Builder> fields = new HashMap<>();. I am suggessting to replace that with a Set<String>. If the field is registered multiple times, the set will make sure to keep only one copy. MemberHint should no longer be abstract or we can maybe keep a FieldHint. Does that help?

[sreenath-tm]

1 ) Is moving the code from our FieldHint to MemberHint recommended.

2 ) I replaced the FieldHint related references and replaced with Set fields but There are 2 usages of the FieldHint that I just require clarification on how we can handle that.
There are public Builder withField(String name, FieldMode mode) { return withField(name, FieldHint.builtWith(mode)); }
and public Builder withField(String name, Consumer<String> fieldHint) { FieldHint.Builder builder = this.fields.computeIfAbsent(name, FieldHint.Builder::new); fieldHint.accept(builder); return this; }

[snicoll]

1. No. FieldHint should only have a name attribute, which is what this issue is all about.
2. They should go away completely and replaced by withField(String name) as the additional attributes should be removed.

[snicoll]

@sreenath-tm how is it going? Sorry to say but we'd like this (and a few related changes) to get in earlier next week so that the team can adapt their code in preparation of the release Thursday. If you have some work in progress you'd like to share, you can submit a PR and I can complete it if necessary. Thanks!

[sreenath-tm]

@snicoll I have changed the corresponding FieldHint and all the instances of FieldHint. I am still figuring out the usages of the FieldHint class and the impact the removal it can cause.Will raise a draft PR so that you can also take a look at it.

[snicoll]

Closing in favor of PR #29130