Skip to content

C++ overlays: Tweak the dbscheme tables; adds source_file_name#21320

Open
igfoo wants to merge 3 commits intogithub:mainfrom
igfoo:igfoo/star_ids
Open

C++ overlays: Tweak the dbscheme tables; adds source_file_name#21320
igfoo wants to merge 3 commits intogithub:mainfrom
igfoo:igfoo/star_ids

Conversation

@igfoo
Copy link
Member

@igfoo igfoo commented Feb 12, 2026

No description provided.

@github-actions github-actions bot added the C++ label Feb 12, 2026
@igfoo igfoo force-pushed the igfoo/star_ids branch 4 times, most recently from fb72e76 to a5b814f Compare February 13, 2026 11:08
@igfoo igfoo marked this pull request as ready for review February 13, 2026 16:06
@igfoo igfoo requested a review from a team as a code owner February 13, 2026 16:06
Copilot AI review requested due to automatic review settings February 13, 2026 16:06
Copy link
Contributor

Copilot AI 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 pull request modifies the C++ dbscheme to support overlay mode improvements by introducing a new @source_file type and associated tables. The key change is refactoring how source files are represented - moving from string-based identification to entity-based identification with a separate name mapping table.

Changes:

  • Adds a new source_file_name table that maps @source_file entities to their string names
  • Modifies source_file_uses_trap table to use @source_file references instead of raw strings
  • Includes upgrade and downgrade scripts to migrate existing databases

Reviewed changes

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

Show a summary per file
File Description
cpp/ql/lib/semmlecode.cpp.dbscheme Main dbscheme file: adds source_file_name table and changes source_file_uses_trap to use @source_file refs
cpp/ql/lib/upgrades/.../upgrade.properties Upgrade configuration marking the change as backwards compatible
cpp/ql/lib/upgrades/.../source_file_uses_trap.ql Upgrade query that generates fresh @source_file entities (empty result set for migration)
cpp/ql/lib/upgrades/.../semmlecode.cpp.dbscheme Target dbscheme schema after upgrade
cpp/ql/lib/upgrades/.../old.dbscheme Source dbscheme schema before upgrade
cpp/downgrades/.../upgrade.properties Downgrade configuration to reverse the change
cpp/downgrades/.../source_file_uses_trap.ql Downgrade query that converts back to string-based representation
cpp/downgrades/.../semmlecode.dbscheme Target dbscheme schema after downgrade (original format)

Copy link
Contributor

@IdrissRio IdrissRio left a comment

Choose a reason for hiding this comment

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

LGTM

@igfoo igfoo added the no-change-note-required This PR does not need a change note label Feb 18, 2026
Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

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

This looks unfinished.

}

from string source_file, Trap trap
where none()
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this query actually do anything? none() looks odd here, and source_file is never bound.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it's worth trying to make the upgrade/downgrade scripts do anything useful for overlays across this change. Overlays aren't supported on one side of the boundary anyway.

I think we need to explicitly make empty tables with the right types in order to achieve that, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why would it be non-functional? This just seems a re-arrangement of tables for efficiency reasons.

Copy link
Member Author

Choose a reason for hiding this comment

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

We haven't finished implementing overlay support yet. I don't think it's worth thinking about upgrading the contents of the overlay tables to or from a time when overlay isn't complete. I can make the scripts populate the tables if you disagree, though.

}

from SourceFile source_file, Trap trap
where none()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the none() here, and shouldn't we be filling source_file_name based on the old source_file_uses_trap?

Copy link
Member Author

Choose a reason for hiding this comment

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

See above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C++ no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments