Skip to content

SSA: Make shared library a parameterized module #10203

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 8 commits into from
Sep 1, 2022

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Aug 29, 2022

This PR changes SsaImplCommon.qll to be a parameterized module. This means that we no longer need 4 copies for C#, but we still need a copy per language (I plan to change that in a follow-up PR).

@github-actions github-actions bot added the C# label Aug 29, 2022
* ```
*/
module Make<SsaInputSig Input> {
private import Input

Check warning

Code scanning / CodeQL

QL-for-QL encountered an internal consistency error

ModConsistency::uniqueResolve
* ```
*/
module Make<InputSig Input> {
private import Input

Check warning

Code scanning / CodeQL

QL-for-QL encountered an internal consistency error

ModConsistency::uniqueResolve
* ```
*/
module Make<InputSig Input> {
private import Input

Check warning

Code scanning / CodeQL

QL-for-QL encountered an internal consistency error

ModConsistency::uniqueResolve
* ```
*/
module Make<InputSig Input> {
private import Input

Check warning

Code scanning / CodeQL

QL-for-QL encountered an internal consistency error

ModConsistency::uniqueResolve
@hvitved hvitved force-pushed the ssa/param-module branch 2 times, most recently from ad1445a to 1710326 Compare August 29, 2022 17:18
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Aug 30, 2022
@hvitved hvitved marked this pull request as ready for review August 30, 2022 07:30
@hvitved hvitved requested review from a team as code owners August 30, 2022 07:30
MathiasVP
MathiasVP previously approved these changes Aug 30, 2022
Copy link
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

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

The changes LGTM! I'll delay the merge until @ginsbach (or someone else from the core team) has looked at the parameterized modules usage.

@alexet
Copy link
Contributor

alexet commented Aug 31, 2022

Are these predicates meant to be publically usable? If they are then they will disappear from the online docs.

@hvitved
Copy link
Contributor Author

hvitved commented Aug 31, 2022

Are these predicates meant to be publically usable? If they are then they will disappear from the online docs.

Nope. Each language should provide its own public layer on top.

@hvitved hvitved merged commit d5200ef into github:main Sep 1, 2022
@hvitved hvitved deleted the ssa/param-module branch September 1, 2022 07:27
geoffw0 added a commit to geoffw0/ql that referenced this pull request Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# C++ no-change-note-required This PR does not need a change note Ruby Swift
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants