Add a direct sub classes data structure to the Painless lookup (#76955)#9
Add a direct sub classes data structure to the Painless lookup (#76955)#9MitchLewis930 wants to merge 1 commit intopr_019_beforefrom
Conversation
…ic#76955) This change has two main components. The first is to have method/field resolution for compile-time and run-time use the same code path for now. This removes copying of member methods between super and sub classes and instead does a resolution through the class hierarchy. This allows us to correctly implement the next change. The second is a data structure that allows for the lookup of direct sub classes for all allow listed classes/interfaces within Painless.
There was a problem hiding this comment.
Pull request overview
This PR adds a direct subclass data structure to the Painless lookup system to track class hierarchy relationships for whitelisted classes.
Changes:
- Added
classesToDirectSubClassesmap to store direct subclass relationships - Replaced
copyPainlessClassMemberswithbuildPainlessClassHierarchyto construct the hierarchy during lookup build - Enhanced
lookupPainlessObjectmethod to handle interface lookups and prevent infinite loops when traversing interfaces
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| org.elasticsearch.painless.lookup | Test resource file defining whitelisted classes for lookup tests |
| LookupTests.java | New test class verifying direct subclass tracking with complex inheritance hierarchies |
| PainlessLookupBuilder.java | Implements hierarchy building logic and integrates direct subclass tracking |
| PainlessLookup.java | Adds direct subclass getter and refactors lookup methods to use unified traversal |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // we check for Object.class as part of the allow listed classes because | ||
| // it is possible for the compiler to work without Object |
There was a problem hiding this comment.
Corrected spelling of 'allow listed' to 'allowlisted' (or 'whitelisted' to match the existing terminology in the codebase).
| Class<?> superClass = subClass.getSuperclass(); | ||
|
|
||
| // this finds the nearest super class for a given sub class | ||
| // because the allow list may have gaps between classes |
There was a problem hiding this comment.
Corrected spelling of 'allow list' to 'allowlist' (or 'whitelist' to match the existing terminology in the codebase).
| break; | ||
| } else { | ||
| // this ensures all interfaces from a sub class that | ||
| // is not allow listed are checked if they are |
There was a problem hiding this comment.
Corrected spelling of 'allow listed' to 'allowlisted' (or 'whitelisted' to match the existing terminology in the codebase).
2 participants
PR_019