C++ overlays: Tweak the dbscheme tables; adds source_file_name#21320
C++ overlays: Tweak the dbscheme tables; adds source_file_name#21320igfoo wants to merge 3 commits intogithub:mainfrom
Conversation
fb72e76 to
a5b814f
Compare
There was a problem hiding this comment.
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_nametable that maps@source_fileentities to their string names - Modifies
source_file_uses_traptable to use@source_filereferences 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) |
a5b814f to
8fe2081
Compare
8fe2081 to
f3d5b4b
Compare
| } | ||
|
|
||
| from string source_file, Trap trap | ||
| where none() |
There was a problem hiding this comment.
Does this query actually do anything? none() looks odd here, and source_file is never bound.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Why would it be non-functional? This just seems a re-arrangement of tables for efficiency reasons.
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
Why is the none() here, and shouldn't we be filling source_file_name based on the old source_file_uses_trap?
No description provided.