Skip to content

Rust: Make Element.toString non-recursive #19162

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

Merged
merged 3 commits into from
Apr 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion misc/codegen/templates/ql_class.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ module Generated {
* Gets the string representation of this element.
*/
cached
final string toString() { result = this.toStringImpl() }
final string toString() {
result = this.toStringImpl() and
// recursion guard to prevent `toString` from being defined recursively
(exists(any(Element e).toStringImpl()) implies any())
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have a negated term that uses toStringImpl and not one that uses toString?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because that would be non-monotonic recursion :-) So the above prevents cases where toStringImpl calls toString.

Copy link
Contributor

@paldepind paldepind Apr 1, 2025

Choose a reason for hiding this comment

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

Thanks! Now I get it. We want to prevent toStringImpl from depending on toString and that's why we put toStringImpl there.

}

/**
* INTERNAL: Do not use.
Expand Down
2 changes: 1 addition & 1 deletion rust/ql/.generated.list

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ module Impl {
* Returns a string suitable to be inserted into the name of the parent. Typically `"..."`,
* but may be overridden by subclasses.
*/
pragma[nomagic]
string toAbbreviatedString() { result = "..." }

predicate isUnknown() { none() } // compatibility with test generation, to be fixed
Expand Down
2 changes: 1 addition & 1 deletion rust/ql/lib/codeql/rust/elements/internal/ModuleImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ module Impl {
* ```
*/
class Module extends Generated::Module {
override string toStringImpl() { result = "mod " + this.getName() }
override string toStringImpl() { result = "mod " + this.getName().getText() }
}
}
2 changes: 1 addition & 1 deletion rust/ql/lib/codeql/rust/elements/internal/UseImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ module Impl {
* ```
*/
class Use extends Generated::Use {
override string toStringImpl() { result = "use " + this.getUseTree() }
override string toStringImpl() { result = "use " + this.getUseTree().toAbbreviatedString() }
}
}
20 changes: 17 additions & 3 deletions rust/ql/lib/codeql/rust/elements/internal/UseTreeImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,28 @@ module Impl {
result = strictconcat(int i | | this.toStringPart(i) order by i)
}

private string toStringPart(int index) {
result = this.getPath().toStringImpl() and index = 0
or
private string toStringPartCommon(int index) {
result = "::{...}" and this.hasUseTreeList() and index = 1
or
result = "::*" and this.isGlob() and index = 2
or
result = " as " + this.getRename().getName().getText() and index = 3
}

private string toStringPart(int index) {
result = this.getPath().toStringImpl() and index = 0
or
result = this.toStringPartCommon(index)
}

override string toAbbreviatedString() {
result = strictconcat(int i | | this.toAbbreviatedStringPart(i) order by i)
}

private string toAbbreviatedStringPart(int index) {
result = this.getPath().toAbbreviatedString() and index = 0
or
result = this.toStringPartCommon(index)
}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion swift/ql/.generated.list

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ private import codeql.swift.elements.decl.Initializer
final private class InitializerLookupExprImpl extends Impl::MethodLookupExpr {
InitializerLookupExprImpl() { super.getMethod() instanceof Initializer }

override string toStringImpl() { result = this.getMember().toString() }
override string toStringImpl() { result = this.getMember().toStringImpl() }
}

final class InitializerLookupExpr extends MethodLookupExpr, InitializerLookupExprImpl {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ module Impl {

override Expr getQualifier() { result = expr.getQualifier() }

override string toStringImpl() { result = "call to " + expr }
override string toStringImpl() { result = "call to " + expr.toStringImpl() }
}

private class FullDotSyntaxBaseIgnoredApplyExpr extends ApplyExpr {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ module Impl {
class ExplicitCastExpr extends Generated::ExplicitCastExpr {
override predicate convertsFrom(Expr e) { e = this.getImmediateSubExpr() }

override string toStringImpl() { result = "(" + this.getType() + ") ..." }
override string toStringImpl() { result = "(" + this.getType().toStringImpl() + ") ..." }
}
}
6 changes: 5 additions & 1 deletion swift/ql/lib/codeql/swift/generated/Element.qll

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading