Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

C++: Fix typeid IR translation#20060

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
jketema merged 4 commits intogithub:mainfromjketema:typeid-fix
Jul 16, 2025
Merged

Conversation

jketema
Copy link
Contributor

No description provided.

@CopilotCopilotAI review requested due to automatic review settingsJuly 15, 2025 18:20
@jketemajketema requested a review froma team as acode ownerJuly 15, 2025 18:20
Copy link
Contributor

@CopilotCopilotAI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the IR translation for C++typeid expressions by implementing proper IR instructions for bothtypeid with expressions andtypeid with types. The changes address consistency issues in the IR generation wheretypeid operations were previously not properly handled.

Key changes:

  • Adds new IR instruction typesTypeidExpr andTypeidType with corresponding opcodes
  • ImplementsTranslatedTypeidExpr class to handle the translation oftypeid operations
  • Updates test expectations to reflect the corrected IR generation

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

FileDescription
cpp/ql/lib/semmle/code/cpp/ir/implementation/Opcode.qllAddsTTypeidExpr andTTypeidType opcodes with their corresponding classes
cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/Instruction.qllDefinesTypeidExprInstruction andTypeidTypeInstruction classes
cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qllImplementsTranslatedTypeidExpr class for handling typeid expression translation
Multiple .expected filesUpdates test expectations to reflect the fixed IR translation

@jketemajketema added the no-change-note-requiredThis PR does not need a change note labelJul 15, 2025
Copy link
Contributor

@MathiasVPMathiasVP left a comment

Choose a reason for hiding this comment

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

One comment, but otherwise this LGTM if DCA comes back happy!

Copy link
Contributor

@MathiasVPMathiasVP left a comment

Choose a reason for hiding this comment

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

Final rounds of comments and then I think we're there 🥳

Copy link
Contributor

@MathiasVPMathiasVP left a comment

Choose a reason for hiding this comment

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

LGTM (once DCA comes back happy)!

@jketema
Copy link
ContributorAuthor

It looks likes this gives a slowdown on zeek/spicy. It looks like that project usestypeid in some of it's general error handling routines, so I suspect we get a lot more flow through those now. It's difficult to exactly tell from the DCA data though.

The other projects in DCA look fine.

@MathiasVP
Copy link
Contributor

MathiasVP commentedJul 16, 2025•
edited
Loading

Makes sense. I think that's fine. I'm quite happy with the large reduction ininstructionWithoutSuccessor 🥳

Did you see that there's a large increase inmissingOperandType consistency errors fordrogon?

@jketema
Copy link
ContributorAuthor

jketema commentedJul 16, 2025•
edited
Loading

Did you see that there's a large increase inmissingOperandType consistency errors fordrogon?

Yes, drogon also has some uses oftypeid (including some in templates), so I assume it has to do with that. Not sure if I should investigate here.

@MathiasVP
Copy link
Contributor

Yeah, I don't think this needs to be looked at in this PR since I highly doubt that this PR does anything incorrectly.

@jketema
Copy link
ContributorAuthor

Ok, merging.

MathiasVP reacted with hooray emoji

@jketemajketema merged commit200d46f intogithub:mainJul 16, 2025
15 checks passed
@jketemajketema deleted the typeid-fix branchJuly 16, 2025 10:40
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

Copilot code reviewCopilotCopilot left review comments

@MathiasVPMathiasVPMathiasVP approved these changes

Assignees
No one assigned
Labels
C++no-change-note-requiredThis PR does not need a change note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@jketema@MathiasVP
[8]ページ先頭
©2009-2025 Movatter.jp