codeql icon indicating copy to clipboard operation
codeql copied to clipboard

Overlay: Add script to help maintain overlay annotations

Open kaspersv opened this issue 7 months ago • 3 comments

This PR adds a script to automatically add sensible default annotations to files without existing overlay annotations. The script uses naming heuristics to determine which files to annotate. The script adds a module-level overlay[local?] annotation and annotates all non-private inline predicates overlay[caller] for selected files. Maintenance of overlay annotations in already annotated files is not the responsibility of this script and will be handled using QL-for-QL queries. This is intended to allow QL authors to gradually take ownership of overlay annotations and manually add & remove automatically added overlay annotations.

A CI check will be added in a subsequent PR to enforce that the script is run for select languages.

The script is based on @ginsbach's annotation script. However, instead of removing existing overlay annotations, it only annotates files without existing annotations and offloads maintenance of annotated files to QL-for-QL queries.

For https://github.com/github/codeql-core/issues/4951.

kaspersv avatar Jun 16 '25 11:06 kaspersv

@ginsbach I've updated the script to add overlay annotations with a --check mode (to be used by the CI check) and expanded the script such that it can handle all our current QL code in the codeql repo. For the latter I've broken the logic for determining where to insert overlay[local?] into a couple of distinct phases (find file-level module declaration if one exists, find file-module qldoc if one exists, etc.). The code is a bit longer, but hopefully simpler to understand and maintain. The changes have no effect for Java (i.e., we insert annotations exactly as before for Java), but allows us to handle all languages.

I've closed the previous PR as it was based on a kaspersv/codeql branch rather than a github/codeql branch, which makes PR stacking more cumbersome.

Would you mind reviewing this PR before I send this PR and the PR that adds overlay annotations for Java to review with the Java team?

kaspersv avatar Jun 17 '25 06:06 kaspersv

What about inline_late?

aschackmull avatar Jun 19 '25 12:06 aschackmull

What about inline_late?

The reason predicates annotated with pragma[inline] need overlay[caller] annotations is that pragma annotations are not supposed to affect semantics and under overlay evaluation inlining across the frontier can change the semantics. However, bindingset annotations are not pragmas and so we have allowed them to affect overlay semantics and we do allow inlining without overlay[caller] annotations across the overlay frontier for predicates annotated with bindingset annotations, which includes all inline_late predicates.

kaspersv avatar Jun 19 '25 12:06 kaspersv

@ginsbach I've rebased the PR and added the third commit which renames overlay[caller] to overlay[caller?]. Commits 1 and 2 haven't been modified since the last review.

kaspersv avatar Jun 24 '25 06:06 kaspersv