RFR: 8353840: JNativeScan should not throw error for missing system classes

[Danish Nawab]

On Tue, 8 Apr 2025 11:55:58 GMT, Danish Nawab <duke at openjdk.org> wrote:

>> Also, instead of passing `err` down to this code, I think we should define an interface for `NativeMethodFinder` to log diagnostics instead. e.g. something like:
>> 
>> 
>> interface Diagnostics {
>>     void error(MethodRef context, JNativeScanFatalError error);
>> } 
>> 
>> 
>> `JNativeScanTask` can then define the implementation how it wants. The benefit of that is that we have more freedom to print the errors how we want. For instance, I think we should first collect all the errors, and de-duplicate error messages before printing them, so that if there are 10 references to the same system class, we only print the error message once, and we can print a header like: `Errors while processing classes: ...`
>
>> This catch block is per-method. It means that if the rest of the method contained references to restricted methods, we would not see them. The catch block should be moved to be just around the call to isRestricted, which can fail. Then we can record the error and keep iterating over the other instructions.
> 
> That makes a lot of sense. I will update it.

> Also, instead of passing err down to this code, I think we should define an interface for NativeMethodFinder to log diagnostics instead. e.g. something like:

This makes sense. I can update it, accordingly.
Regarding deduplication: this means we would need to buffer the errors and print them all at the end, which would require coordination between different layers, and we might need to mark `Diagnostics` as `Closable/AutoClosable`. 
Would it be ok for you, if the buffering/deduplication can be done a follow-up and for now we just change the termination behavior? Or do you see that as an essential component that should be included with this change?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/24499#discussion_r2033037953