Skip to content

[rust2cpg] support impl X blocks.#6031

Open
xavierpinho wants to merge 3 commits into
masterfrom
xavierp/basic-impl
Open

[rust2cpg] support impl X blocks.#6031
xavierpinho wants to merge 3 commits into
masterfrom
xavierp/basic-impl

Conversation

@xavierpinho

Copy link
Copy Markdown
Contributor

Only impl X methods are handled here, and they become child methods to the X TYPE_DECL. I intend to remove the ugly storeInDiffGraph inside RustVisitor as I tackle the impl X for Y variant.

// 'impl' GenericParamList? ('const'? '!'? Type 'for')? Type WhereClause?
// AssocItemList
// TODO: support `impl X for Y` and remove the side-effects (storeInDiffGraph).
private def visitImpl(impl: Impl): Seq[Ast] = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does rust allow to specify multiple impl X decls for the same type?
If so, this implementation here would create multiple TypeDecls with the same fullName.

@xavierpinho xavierpinho Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does -- and I don't know why I can no longer paste a proper link, but please see the "multiple impl blocks for the same type" in the current diff.

But that correctly reminds me: the TypeDecl creation actually happens outside (see the last paragraph of #6028). Well, not for structs declared in the source-code, but for external ones. Anyway, for the impl X for Y I was thinking of having an extra pass à la swifsrc2cpg's ExtensionPass so that these kind of changes only happen in one place, rather than potentially scattered inside RustVisitor.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, just one more thing came into my mind: is it possible to specify multiple impl X decls for the same type but spread over different files? Does that lead to multiple TypeDecls with the same fullName?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also fine (no duplicates), but it's good to have a test for it.

code = code(selfParam),
index = 0,
isVariadic = false,
evaluationStrategy = EvaluationStrategies.BY_SHARING,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rust has BY_VALUE parameter passing.

|""".stripMargin)

"have correct fullName" in {
cpg.method.nameExact("bar").fullName.l shouldBe List("rust2cpgtest::Foo::bar")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rust2cpgtest is the module name right? If yes please it something like testModule or a like. The import part that should come across is that it is not the file name.
This this will affect lots of tests you should likely do that in a separate PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test fixture creates at least 2 files:

  • Cargo.toml, which is where rust2cpgtest comes from.
  • src/lib.rs (by default, since code(..) accepts a file name, and there's the usual moreCode to add more files) where the snippet is written into.

So, rust2cpgtest is not coming from the filename. Have I missed your point?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was aware of that. I was just suggesting that if you name the module testModule it is immediately obvious that this is not a file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants