Skip to content

Rust: rename TypeRef and *Type to *TypeRepr, ty to type_repr, and expand some abbreviations in generated docs#18174

Merged
paldepind merged 12 commits into
mainfrom
redsun82/rust-tweaks
Dec 4, 2024
Merged

Rust: rename TypeRef and *Type to *TypeRepr, ty to type_repr, and expand some abbreviations in generated docs#18174
paldepind merged 12 commits into
mainfrom
redsun82/rust-tweaks

Conversation

@redsun82
Copy link
Copy Markdown
Contributor

@redsun82 redsun82 commented Dec 2, 2024

We were already renaming the tip of the type reference hierarchy from Type to TypeRef but weren't doing so with all its descendants.

Additionally, the pat and ty abbreviations are now expanded in generated documentation.

@redsun82 redsun82 requested a review from a team as a code owner December 2, 2024 09:16
@github-actions github-actions Bot added the Rust Pull requests that update Rust code label Dec 2, 2024
github-advanced-security[bot]
github-advanced-security AI found potential problems Dec 2, 2024
View reviewed changes
Comment thread rust/ql/lib/codeql/rust/elements.qll Fixed
Comment thread rust/ql/lib/codeql/rust/elements.qll Fixed
Comment thread rust/ql/lib/codeql/rust/elements/FnPtrTypeRef.qll Fixed
Comment thread rust/ql/lib/codeql/rust/elements/RetTypeRef.qll Fixed
@paldepind
Copy link
Copy Markdown
Contributor

paldepind commented Dec 2, 2024

Why do we want to rename *Type to *TypeRef? Why are they type references and not just types?

@redsun82
Copy link
Copy Markdown
Contributor Author

redsun82 commented Dec 2, 2024

Why do we want to rename *Type to *TypeRef? Why are they type references and not just types?

These are all instances of types written in the code (they are AstNodes, they have a location), and especially they can be optional. "Real" types are semantic, unique despite all the different references to them, and also present for terms that don't have a type spelled out in the code, so they really are a different thing from what I changed here.

@redsun82
Copy link
Copy Markdown
Contributor Author

redsun82 commented Dec 2, 2024

so they really are a different thing from what I changed here

(a thing we don't have, yet)

@paldepind
Copy link
Copy Markdown
Contributor

paldepind commented Dec 2, 2024 •
edited
Loading

Thanks for the explanation @redsun82 👍 🙏 . I guess the name "Ref" wasn't immediately intuitive to me, but aside from that it makes sense. It's the distinction between syntax or notation representing a type and the actual type itself.

paldepind
paldepind previously approved these changes Dec 2, 2024
View reviewed changes
@redsun82
Copy link
Copy Markdown
Contributor Author

redsun82 commented Dec 2, 2024

Thanks for the explanation @redsun82 👍 🙏 . I guess the name "Ref" wasn't immediately intuitive to me, but aside from that it makes sense. It's the distinction between syntax or notation representing a type and the actual type itself.

we could also opt for using Repr instead of Ref (that's what we do in Swift for example). Do you think that would be clearer?

@redsun82 redsun82 requested a review from paldepind December 2, 2024 14:16
@paldepind
Copy link
Copy Markdown
Contributor

paldepind commented Dec 2, 2024

we could also opt for using Repr instead of Ref (that's what we do in Swift for example). Do you think that would be clearer?

Yes, subjectively I think that would be better! That also avoids clashes/confusion with the Rust concept of a reference. For instance the current class name RefTypeRef is a bit awkward.

@redsun82 redsun82 changed the title Rust: rename *Type to *TypeRef and expand some abbreviations in generated docs Rust: rename TypeRef and *Type to *TypeRepr and expand some abbreviations in generated docs Dec 3, 2024
github-advanced-security[bot]
github-advanced-security AI found potential problems Dec 3, 2024
View reviewed changes
Comment thread rust/ql/lib/codeql/rust/elements.qll
import codeql.rust.elements.TuplePat
import codeql.rust.elements.TupleStructPat
import codeql.rust.elements.TupleType
import codeql.rust.elements.TupleTypeRepr

Check warning

Code scanning / CodeQL

Redundant import

Redundant import, the module is already imported inside [codeql.rust.elements.PathSegment](1).
Comment thread rust/ql/lib/codeql/rust/elements.qll Outdated

Check warning

Code scanning / CodeQL

Redundant import

Redundant import, the module is already imported inside [codeql.rust.elements.AssocTypeArg](1). Redundant import, the module is already imported inside [codeql.rust.elements.ClosureExpr](2). Redundant import, the module is already imported inside [codeql.rust.elements.FnPtrTypeRepr](3). Redundant import, the module is already imported inside [codeql.rust.elements.Function](4). Redundant import, the module is already imported inside [codeql.rust.elements.PathSegment](5).
Comment thread rust/ql/lib/codeql/rust/elements.qll Outdated

Check warning

Code scanning / CodeQL

Redundant import

Redundant import, the module is already imported inside [codeql.rust.elements.ArrayTypeRepr](1). Redundant import, the module is already imported inside [codeql.rust.elements.AssocTypeArg](2). Redundant import, the module is already imported inside [codeql.rust.elements.CastExpr](3). Redundant import, the module is already imported inside [codeql.rust.elements.Const](4). Redundant import, the module is already imported inside [codeql.rust.elements.ConstParam](5). Redundant import, the module is already imported inside [codeql.rust.elements.DynTraitTypeRepr](6). Redundant import, the module is already imported inside [codeql.rust.elements.FnPtrTypeRepr](7). Redundant import, the module is already imported inside [codeql.rust.elements.ForTypeRepr](8). Redundant import, the module is already imported inside [codeql.rust.elements.Impl](9). Redundant import, the module is already imported inside [codeql.rust.elements.ImplTraitTypeRepr](10). Redundant import, the module is already imported inside [codeql.rust.elements.InferTypeRepr](11). Redundant import, the module is already imported inside [codeql.rust.elements.LetStmt](12). Redundant import, the module is already imported inside [codeql.rust.elements.MacroTypeRepr](13). Redundant import, the module is already imported inside [codeql.rust.elements.NeverTypeRepr](14). Redundant import, the module is already imported inside [codeql.rust.elements.OffsetOfExpr](15). Redundant import, the module is already imported inside [codeql.rust.elements.ParamBase](16). Redundant import, the module is already imported inside [codeql.rust.elements.ParenTypeRepr](17). Redundant import, the module is already imported inside [codeql.rust.elements.PathSegment](18). Redundant import, the module is already imported inside [codeql.rust.elements.PathTypeRepr](19). Redundant import, the module is already imported inside [codeql.rust.elements.PtrTypeRepr](20). Redundant import, the module is already imported inside [codeql.rust.elements.RecordField](21). Redundant import, the module is already imported inside [codeql.rust.elements.RefTypeRepr](22). Redundant import, the module is already imported inside [codeql.rust.elements.RetTypeRepr](23). Redundant import, the module is already imported inside [codeql.rust.elements.SliceTypeRepr](24). Redundant import, the module is already imported inside [codeql.rust.elements.Static](25). Redundant import, the module is already imported inside [codeql.rust.elements.TupleField](26). Redundant import, the module is already imported inside [codeql.rust.elements.TupleTypeRepr](27). Redundant import, the module is already imported inside [codeql.rust.elements.TypeAlias](28). Redundant import, the module is already imported inside [codeql.rust.elements.TypeArg](29). Redundant import, the module is already imported inside [codeql.rust.elements.TypeBound](30). Redundant import, the module is already imported inside [codeql.rust.elements.TypeParam](31). Redundant import, the module is already imported inside [codeql.rust.elements.WherePred](32).
Comment thread rust/ql/lib/codeql/rust/elements/AssocTypeArg.qll
import codeql.rust.elements.ReturnTypeSyntax
import codeql.rust.elements.TypeBoundList
import codeql.rust.elements.TypeRef
import codeql.rust.elements.TypeRepr

Check warning

Code scanning / CodeQL

Redundant import

Redundant import, the module is already imported inside [codeql.rust.elements.RetTypeRepr](1).
Comment thread rust/ql/lib/codeql/rust/elements/FnPtrTypeRepr.qll
import codeql.rust.elements.Abi
import codeql.rust.elements.ParamList
import codeql.rust.elements.RetTypeRepr
import codeql.rust.elements.TypeRepr

Check warning

Code scanning / CodeQL

Redundant import

Redundant import, the module is already imported inside [codeql.rust.elements.RetTypeRepr](1).
Comment thread rust/ql/lib/codeql/rust/elements/PathSegment.qll Outdated

Check warning

Code scanning / CodeQL

Redundant import

Redundant import, the module is already imported inside [codeql.rust.elements.PathTypeRepr](1). Redundant import, the module is already imported inside [codeql.rust.elements.RetTypeRepr](2).
Comment thread rust/ql/lib/codeql/rust/elements/RetTypeRepr.qll
*/

private import internal.RetTypeReprImpl
import codeql.rust.elements.AstNode

Check warning

Code scanning / CodeQL

Redundant import

Redundant import, the module is already imported inside [codeql.rust.elements.TypeRepr](1).
Comment thread rust/ql/lib/codeql/rust/elements/internal/generated/AssocTypeArg.qll
import codeql.rust.elements.ReturnTypeSyntax
import codeql.rust.elements.TypeBoundList
import codeql.rust.elements.TypeRef
import codeql.rust.elements.TypeRepr

Check warning

Code scanning / CodeQL

Redundant import

Redundant import, the module is already imported inside [codeql.rust.elements.RetTypeRepr](1).
Comment thread rust/ql/lib/codeql/rust/elements/internal/generated/PathSegment.qll
import codeql.rust.elements.RetTypeRepr
import codeql.rust.elements.ReturnTypeSyntax
import codeql.rust.elements.TypeRef
import codeql.rust.elements.TypeRepr

Check warning

Code scanning / CodeQL

Redundant import

Redundant import, the module is already imported inside [codeql.rust.elements.PathTypeRepr](1). Redundant import, the module is already imported inside [codeql.rust.elements.RetTypeRepr](2).
@redsun82 redsun82 changed the title Rust: rename TypeRef and *Type to *TypeRepr and expand some abbreviations in generated docs Rust: rename TypeRef and *Type to *TypeRepr, ty to type_repr, and expand some abbreviations in generated docs Dec 3, 2024
@redsun82
Copy link
Copy Markdown
Contributor Author

redsun82 commented Dec 3, 2024

@paldepind I went with the rename to TypeRepr, I also find it more intuitive. I've also changed all the getTy predicates to more explicit getTypeRepr (which will also cause less confusion when we start to have getType for actual semantic types), and went for a more specific getElementTypeRepr for ArrayTypeRepr.

paldepind
paldepind previously approved these changes Dec 3, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@paldepind paldepind left a comment

Choose a reason for hiding this comment

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

Really great changes 🤩

@redsun82 redsun82 requested a review from paldepind December 4, 2024 09:27
@redsun82
Copy link
Copy Markdown
Contributor Author

redsun82 commented Dec 4, 2024

@paldepind I had to merge in main, can you reapprove please?

@paldepind paldepind merged commit 547e232 into main Dec 4, 2024
@paldepind paldepind deleted the redsun82/rust-tweaks branch December 4, 2024 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@paldepind paldepind paldepind approved these changes

Assignees

No one assigned

Labels

Rust Pull requests that update Rust code

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@redsun82 @paldepind @github-advanced-security