Rust: Use PathResolution module in data flow
#18609
Conversation
hvitved
force-pushed
the
rust/dataflow-path-resolution
branch
from
88186da to
515bc76
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
hvitved
force-pushed
the
rust/dataflow-path-resolution
branch
2 times, most recently
from
225e0e2 to
014f9e5
Compare
hvitved
force-pushed
the
rust/dataflow-path-resolution
branch
from
014f9e5 to
d56bf65
Compare
| ( | ||
| crate.asSome() = r.getResolvedCrateOrigin() | ||
| /** A canonical path pointing to an enum variant. */ | ||
| class VariantLib extends MkVariantLib { |
There was a problem hiding this comment.
What about using VariantInLib or VariantFromLib or something of that sort? My first reading of this was "a QL library for variants" 😅
There was a problem hiding this comment.
Could also be VariantInStdLib assuming we don't extend to take this special casing further than that.
| path = r.getResolvedPath() and | ||
| ( | ||
| crate.asSome() = r.getResolvedCrateOrigin() | ||
| /** A canonical path pointing to an enum variant. */ |
There was a problem hiding this comment.
| /** A canonical path pointing to an enum variant. */ | |
| /** An enum variant and its canonical path in the standard library. */ |
| string toString() { result = name } | ||
| } | ||
|
|
||
| predicate variantLibPos(VariantLib::VariantLib v, int pos) { |
There was a problem hiding this comment.
Could this be a member predicate on VariantLib?
| resolveExtendedCanonicalPath(p, crate, path) and | ||
| s = MkStructCanonicalPath(crate, path) | ||
| ) | ||
| private predicate pathResolveToStruct(PathAstNode p, Struct s) { |
There was a problem hiding this comment.
With this change pathResolveToStruct and pathResolveToVariant are both very simple and identical in implementation. Would it make sense to drop them and inline them or to combine them into one generalized predicate?
| or | ||
| // Pattern destructs a struct. | ||
| structDestruction(pat.getPat(), c.(StructFieldContent).getStructCanonicalPath(field)) | ||
| structDestruction(pat.getPat(), c.(StructFieldContent).getStruct(field)) |
There was a problem hiding this comment.
I find this getStruct (and getVariant) a bit hard to read. It looks like it gets a struct for field. The name hasFieldAndGetStruct would have been a more clear. But it's the same on main, so this is more an idea for a future refactor (that I could do). I think separating this into a getField\0 and getSruct\0 would be easier to read (but more verbose).
|
Would we have to revert this in case we decide to keep path resolution in the extractor (e.g., if the performance turns out ok)? |
It would have to be partly reverted, yes, though I think once we have the extractor properly extracting library code, we would want an interface similar to |
Co-authored-by: Simon Friis Vindum <paldepind@github.com>
This PR updates our data flow library to use the newly introduced
PathResolutionlibrary for identifying record and variant constructions. We still need a workaround for built-in types likeOption, until we extract the relevant source code.