C++: More IR QLDoc (including Opcode.qll)
#3819
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
June 26, 2020 09:04
- Remove unused `MemoryAccessOpcode` - Make `OpcodeWithCondition` private - Add QLDoc for `Opcode` module
- Renamed `DynamicCastToVoid` to the more descriptive `CompleteObjectAddress` - Split verbose description from summary in a few Instruction QLDoc comments. - Added `Instruction` classes for the few remaining `Opcode`s that didn't have one. - Removed a use of "e.g."
For every concrete `Opcode`, there is a corresponding `Instruction` class. Rather than duplicate all of the QLDoc by hand, I wrote a quick Python script to copy the QLDoc from `Instruction.qll` to `Opcode.qll`. I don't expect that we will need to do this often, so I'm not hooking it up to a PR check or anything like that, but I did commit the script itself in case we need it again.
added 4 commits
June 26, 2020 13:45
This auto-generates even more QLDoc for `Opcode.qll`
As discussed in today's C++ analysis team meeting. `Opcode` is rarely used directly, so we'll just refer to the documentation for the corresponding `Instruction` class. I've preserved the script in case we want to do a bulk change of all of the `Opcode` comments, but I don't expect it will be needed if we just add a new `Opcode` or two.
|
I've implemented the changes we discussed in today's C++ analysis team meeting, so now the |
added 10 commits
June 29, 2020 12:43
Moved a couple of predicates that were only needed by IR construction into `TranslatedElement.qll`
dbartol
changed the title
Opcode.qllOpcode.qll)
|
Since this PR hadn't been reviewed yet anyway, I've added QLDoc for several more files. |
jbj
approved these changes
Jun 30, 2020
| final Language::Location getLocation() { result = getFirstInstruction().getLocation() } | ||
|
|
||
| /** | ||
| * Gets a string that uniquely identifies this block within its enclosing function. |
There was a problem hiding this comment.
Should this be marked INTERNAL: Do not use.?
| final Language::Location getLocation() { result = getFirstInstruction().getLocation() } | ||
|
|
||
| /** | ||
| * Gets a string that uniquely identifies this block within its enclosing function. |
There was a problem hiding this comment.
Should this be marked INTERNAL: Do not use.? Same for getDisplayIndex.
| final PhiInstruction getAPhiInstruction() { | ||
| Construction::getPhiInstructionBlockStart(result) = getFirstInstruction() | ||
| } | ||
|
|
||
| /** | ||
| * Get the instructions in this block, including `Phi` instructions. |
There was a problem hiding this comment.
Suggested change
| * Get the instructions in this block, including `Phi` instructions. | |
| * Gets an instruction in this block. This includes `Phi` instructions. |
https://github.com/github/codeql/blob/master/docs/qldoc-style-guide.md#predicates-with-result
| @@ -74,28 +110,65 @@ class IRBlockBase extends TIRBlock { | |||
| * instruction of another block. | |||
| */ | |||
| class IRBlock extends IRBlockBase { | |||
| /** | |||
| * Gets the blocks to which control flows directly from this block. | |||
There was a problem hiding this comment.
Suggested change
| * Gets the blocks to which control flows directly from this block. | |
| * Gets a block to which control flows directly from this block. |
| final IRBlock getASuccessor() { blockSuccessor(this, result) } | ||
|
|
||
| /** | ||
| * Gets the blocks from which control flows directly to this block. |
There was a problem hiding this comment.
Suggested change
| * Gets the blocks from which control flows directly to this block. | |
| * Gets a block from which control flows directly to this block. |
| final predicate dominates(IRBlock block) { strictlyDominates(block) or this = block } | ||
|
|
||
| /** | ||
| * Gets the set of blocks on the dominance frontier of this block. |
There was a problem hiding this comment.
Suggested change
| * Gets the set of blocks on the dominance frontier of this block. | |
| * Gets a block on the dominance frontier of this block. |
| @@ -162,20 +163,26 @@ class IRGeneratedVariable extends IRVariable { | |||
|
|
|||
| override string getUniqueId() { none() } | |||
|
|
|||
| /** | |||
| * Gets a string containing the source code location of the AST that generated this variable. | |||
There was a problem hiding this comment.
Should this be marked INTERNAL: Do not use.? Same for getBaseString.
| @@ -1880,6 +1988,10 @@ class BuiltInOperationInstruction extends Instruction { | |||
| operation = Raw::getInstructionBuiltInOperation(this) | |||
| } | |||
|
|
|||
| /** | |||
| * Gets the language-specific `BuildInOperation` object that specifies the operation that is | |||
There was a problem hiding this comment.
s/BuildInOperation/BuiltInOperation/
|
Thanks. I'll address all of the above feedback in a follow-up PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds QLDoc for every opcode in
Opcode.qll. It's is all copied from the correspondingInstruction.qllQLDoc via a Python script, included as part of the PR.There are a few preparatory commits to clean up some existing QLDoc comments, add a few missing
Instructionclasses, etc.