-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Rust: Refactor MaD provenance-based filtering #21072
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -111,27 +111,61 @@ predicate interpretModelForTest(QlBuiltins::ExtensionId madId, string model) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private class SummarizedCallableFromModel extends SummarizedCallable::Range { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private string path; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private predicate summaryModel( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Function f, string input, string output, string kind, Provenance provenance, boolean isExact, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| QlBuiltins::ExtensionId madId | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exists(string path, Function f0 | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| summaryModel(path, input, output, kind, provenance, madId) and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f0.getCanonicalPath() = path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f = f0 and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isExact = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| or | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f.implements(f0) and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isExact = false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SummarizedCallableFromModel() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| summaryModel(path, _, _, _, _, _) and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.getCanonicalPath() = path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private predicate summaryModelRelevant( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Function f, string input, string output, string kind, Provenance provenance, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| QlBuiltins::ExtensionId madId | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exists(boolean isExact | summaryModel(f, input, output, kind, provenance, isExact, madId) | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| provenance.isManual() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| or | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // only apply generated models to functions not defined in source code, and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // when there are no exact manual models for the functions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| provenance.isGenerated() and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| not any(Provenance manual | summaryModel(f, _, _, _, manual, true, _)).isManual() and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| not f.fromSource() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isExact = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| or | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // only apply trait models to concrete implementations when they are not | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // defined in source code, and when there are no exact model for the functions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // defined in source code, and when there are no exact model for the functions | |
| // defined in source code, and when there is no exact model with the same provenance for the function |
Outdated
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment uses plural "functions" but should use singular "function" since it refers to a specific function f.
| // defined in source code, and when there are no exact model for the functions | |
| // defined in source code, and when there is no exact model for the function |
Outdated
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition checks for the absence of an exact model with the same provenance. However, this means a generated trait model could still be applied even when a manual specific implementation model exists (since they have different provenance values). Based on the comment stating "when there are no exact model for the functions" (line 148), this should likely check for any exact model regardless of provenance: not summaryModel(f, _, _, _, _, true, _)
| not summaryModel(f, _, _, _, provenance, true, _) and | |
| not summaryModel(f, _, _, _, _, true, _) and |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like this to avoid a bit of repetition?
| exists(boolean isExact | summaryModel(f, input, output, kind, provenance, isExact, madId) | | |
| ( | |
| provenance.isManual() | |
| or | |
| // only apply generated models to functions not defined in source code, and | |
| // when there are no exact manual models for the functions | |
| provenance.isGenerated() and | |
| not any(Provenance manual | summaryModel(f, _, _, _, manual, true, _)).isManual() and | |
| not f.fromSource() | |
| ) and | |
| ( | |
| isExact = true | |
| or | |
| // only apply trait models to concrete implementations when they are not | |
| // defined in source code, and when there are no exact model for the functions | |
| isExact = false and | |
| not summaryModel(f, _, _, _, provenance, true, _) and | |
| not f.fromSource() | |
| ) | |
| exists(boolean isExact | summaryModel(f, input, output, kind, provenance, isExact, madId) | | |
| // Only apply generated or inherited models to functions in library code and | |
| // when no strictly better model exist | |
| if provenance.isGenerated() or isExact = false | |
| then | |
| not f.fromSource() and | |
| not exists(Provenance provenance2 | summaryModel(f, _, _, _, provenance2, true, _) | | |
| provenance.isGenerated() and provenance2.isManual() | |
| or | |
| provenance = provenance2 and isExact = false | |
| ) | |
| else any() | |
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about
isDirectorisInherited(with opposite value) instead of "exact"?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went for
isExact, since that is what other languages use, and will be shared once #21051 is ready.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, but maybe that's an additional reason to get the naming right? :)
To me
isExactsounds like something about the accuracy of the model itself and where the opposite would be an "approximate" model. The suggested names (andisInheritedin particular) sounds more like what this is actually about.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair, I have changed it to
isInherited.