Repository navigation
Unified: Use ParameterEx in type inference - #22790
Conversation
0505c1e to
3af430e
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Swift inferred-property parameters can disappear, and required mutable properties are incorrectly marked as defaulted.
3 open findings
What changed in this PR
Updates Unified type inference to model explicit and synthesized parameters through ParameterEx.
Changes:
- Extends
ParameterExwith type and default-value metadata. - Refactors type inference declarations and callables around Unified AST types.
- Adds callable return-type support to the generated AST API.
| File | Description |
|---|---|
unified/ql/lib/utils/test/TestUtils.qll |
Uses type-inference callable naming. |
unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll |
Refactors declarations and parameters. |
unified/ql/lib/codeql/unified/internal/ParameterEx.qll |
Adds parameter type/default metadata. |
unified/ql/lib/codeql/unified/internal/AstPluginSwift.qll |
Supplies Swift memberwise parameters. |
unified/ql/lib/codeql/unified/internal/AstPlugin.qll |
Expands the plugin contract. |
unified/ql/lib/codeql/unified/internal/Ast.qll |
Exposes callable return types. |
unified/extractor/ast_types.yml |
Adds return types to callable schema. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
3af430e to
2b933c0
Compare
2b933c0 to
26c473b
Compare
26c473b to
92647c7
Compare
asgerf
left a comment
There was a problem hiding this comment.
I'm slightly worried about having an AST plugin that references type inference.
But come of think of it: is there any harm in considering all of these parameters to have a default? The default constructor is only created when there are no other constructors, so it really shouldn't affect overload resolution. Maybe we can simplify it and resolve my above concern at the same time.
It actually only needs static name binding; does that make you less worried? But if we want to support implicitly typed fields at some point, it will need full type inference. |
92647c7 to
57e2a96
Compare
Do you mean fields whose type is inferred from its initializer? In that case, we get a free pass because those fields are recognized as having default value, based on the fact that they have an initializer. |


This PR simplifies the type inference library by pulling in
ParameterEx. The PR also implements typing for parameters of implicitstructparameters via a newdefaultConstructorParameterTypeplugin predicate.DCA looks good; a small improvement in call resolution rate.