On Fri, 17 May 2024 15:00:39 GMT, Jan Lahoda <jlah...@openjdk.org> wrote:
>> Nizar Benalla has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> - Not checking elements enclosed within a record, I doubt a record class
>> will change after being created
>> - Added a null check as `toString` can return an exception
>
> test/jdk/tools/sincechecker/SinceChecker.java line 352:
>
>> 350: }
>> 351: checkElement(te.getEnclosingElement(), te, types,
>> javadocHelper, version, elementUtils);
>> 352: if( te.getKind() == ElementKind.RECORD){
>
> This seems a bit too broad. Ignoring all members of a record type may lead to
> missed members. `Elements.getOrigin` may be unusable here, but I wonder what
> exactly is the problem which this condition is trying to solve?
I changed my approach in
[ac4df85](https://github.com/openjdk/jdk/pull/18934/commits/ac4df85615557306da606a712ee02e7155e5412f).
I beleieve it is a bit clearer now. I am only skipping some very common
methods (`toString`, `equals` and `hashCode`)
Previously, I failed to get the effective `@since` of some common methods
eclosed within a record.
I will use this as an example
https://github.com/openjdk/jdk/blob/b92bd671835c37cff58e2cdcecd0fe4277557d7f/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiDefaultExecutionControl.java#L346
`te.getEnclosedElements()` on this record returns the following (I truncated
`jdk.jshell.execution.JdiDefaultExecutionControl.JdiStarter.TargetDescription`
into `TargetDescription`)
method: void
TargetDescription.<init>(com.sun.jdi.VirtualMachine,java.lang.Process)
method: void TargetDescription.wait(long,int)
method: java.lang.Process TargetDescription.process()
method: com.sun.jdi.VirtualMachine TargetDescription.vm()
method: java.lang.String TargetDescription.toString()
method: int TargetDescription.hashCode()
method: java.lang.Object TargetDescription.clone()
method: java.lang.Class TargetDescription.getClass()
method: void TargetDescription.finalize()
method: void TargetDescription.notify()
method: void TargetDescription.wait(long)
method: boolean TargetDescription.equals(java.lang.Object)
method: void TargetDescription.notifyAll()
method: void TargetDescription.wait()
Getting the effective `@since` for these 5 methods in particular would fail
method: java.lang.String TargetDescription.toString():
method: int TargetDescription.hashCode():
method: boolean TargetDescription.equals(java.lang.Object):
method: com.sun.jdi.VirtualMachine TargetDescription.vm():
method: java.lang.Process TargetDescription.process():
It seemed to me that these methods would be added in the same version as the
record, so I skipped them all.
I didn't think of a user adding a static variable in a later release and that
it was too broad, my bad.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18934#discussion_r1605899531
