Spelling cpp by jsoref · Pull Request #10825 · github/codeql

jsoref · 2022-10-13T23:59:35Z

follow-up to #10743

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

jsoref · 2022-10-14T00:00:10Z

 * - `ConstantSignExpr`, for expressions with a compile-time constant value.
 * - `FlowSignExpr`, for expressions whose sign can be computed from the signs of their operands.
- * - `CustomsignExpr`, for expressions shose sign can be computed by a language-specific
+ * - `CustomsignExpr`, for expressions whose sign can be computed by a language-specific


Google Sheets didn't guess this correction...

jsoref · 2022-10-14T00:00:44Z

    // not inside a template instantiation
    not exists(Element other | this.isFromTemplateInstantiation(other)) or
-    // allow DeclarationEntrys of template specializations
+    // allow DeclarationEntries of template specializations


DeclarationEntry is probably a term. I left this change in to be able to hang this comment. I expect to drop this change.

This probably should be something like

`DeclarationEntry`s

but I see already below that if we go down that route we should fix a bunch more formatting issues in this file, so this is probably better dealt with separately.

I agree that

`DeclarationEntry`s

is preferable (rendered as DeclarationEntrys), the original is also OK.

jsoref · 2022-10-14T00:03:01Z

  switch(c1){
 ...
-  dafault: // BAD: maybe it will be right `default`
+  default: // BAD: maybe it will be right `default`


This correction is probably wrong

Yes, please revert. This shows we detect spelling mistakes in C/C++ code 😄

personally, I encourage using things like default0, default_, or de_fault since they tend to be functionally equivalent in that they show that your tool catches spelling mistakes, while not actually upsetting spell checkers.

That said, I'm going to drop this and if there's interest, a change like that can be its own distinct PR.

It depends on what the query actually checks for. I can have a look at that next week.

I encourage using things like default0, default_, or de_fault since they tend to be functionally equivalent in that they show that your tool catches spelling mistakes

Unfortunately that's not going to work here, as the query looks for specific substitutions. This is the relevant query code:

( lb.getName().charAt(0) = "d" or lb.getName().charAt(0) = "c" ) and ( lb.getName().charAt(1) = "e" or lb.getName().charAt(1) = "a" ) and ( lb.getName().charAt(2) = "f" or lb.getName().charAt(2) = "s" )

Note that the query is marked as experimental, so the bar on query quality is very low here.

jketema

Few small comments but otherwise LGTM.

jketema · 2022-10-14T05:52:17Z

    // not inside a template instantiation
    not exists(Element other | this.isFromTemplateInstantiation(other)) or
-    // allow DeclarationEntrys of template specializations
+    // allow DeclarationEntries of template specializations


This probably should be something like

`DeclarationEntry`s

but I see already below that if we go down that route we should fix a bunch more formatting issues in this file, so this is probably better dealt with separately.

jketema · 2022-10-14T06:00:19Z

  switch(c1){
 ...
-  dafault: // BAD: maybe it will be right `default`
+  default: // BAD: maybe it will be right `default`


Yes, please revert. This shows we detect spelling mistakes in C/C++ code 😄

geoffw0

I'm slightly horrified to see this amount of typos, missing characters and (incorrect) phonetic spellings that have accumulated in our code; and surely instyructions could only have been caused by a cosmic ray?

Thanks for fixing.

geoffw0 · 2022-10-14T07:53:23Z

 * The `security` tag has been added to the `cpp/return-stack-allocated-memory` query. As a result, its results will now appear by default.
 * The "Uncontrolled data in arithmetic expression" (cpp/uncontrolled-arithmetic) query has been enhanced to reduce false positive results and its @precision increased to high.
-* A new `cpp/very-likely-overruning-write` query has been added to the default query suite for C/C++. The query reports some results that were formerly flagged by `cpp/overruning-write`.
+* A new `cpp/very-likely-overrunning-write` query has been added to the default query suite for C/C++. The query reports some results that were formerly flagged by `cpp/overrunning-write`.


I'm not sure editing a past release not will affect anything, though I suppose there's no harm in trying.

This was also done in the case of some of the other spelling PRs. I'm guessing this is fine, these changes just not get included in the previous releases.

geoffw0 · 2022-10-14T07:58:59Z

  not ae instanceof ForStmtSideEffectExpr
 select ae,
-  "AV Rule 160: An assignment expression shall be used only as the exprression in an expression statement."
+  "AV Rule 160: An assignment expression shall be used only as the expression in an expression statement."


I'm a bit disappointed there are apparently no test cases for these two old queries (but I'm happy with the spelling fixes on them).


 /** Holds if there are labels inside the block with names similar to `default` or `case`. */
-predicate isWrongLableName(SwitchStmt swtmp) {
+predicate isWrongLabelName(SwitchStmt swtmp) {


jsoref · 2022-10-14T14:39:51Z

Ok, this PR and #10824 conceptually conflict in that they both will trigger a complaint akin to:
https://github.com/github/codeql/actions/runs/3246489164/jobs/5331736472

Run python config/sync-files.py
Loading file groups from ./config/identical-files.json
ERROR: config/sync-files.py:0 - Files from group 'IR Instruction' not in sync.
ERROR: config/sync-files.py:0 - Run this script with a file-name argument among the following to overwrite the remaining files with the contents of that file, or run with the --latest switch to update each group of files from the most recently modified file in the group.
ERROR: config/sync-files.py:0 -     ./cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/Instruction.qll
ERROR: config/sync-files.py:0 -     ./cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll
ERROR: config/sync-files.py:0 -     ./cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll
ERROR: config/sync-files.py:0 -     ./csharp/ql/src/experimental/ir/implementation/raw/Instruction.qll
ERROR: config/sync-files.py:0 -     ./csharp/ql/src/experimental/ir/implementation/unaliased_ssa/Instruction.qll
ERROR: config/sync-files.py:0 - Files from group 'IR SSA SSAConstruction' not in sync.
ERROR: config/sync-files.py:0 - Run this script with a file-name argument among the following to overwrite the remaining files with the contents of that file, or run with the --latest switch to update each group of files from the most recently modified file in the group.
ERROR: config/sync-files.py:0 -     ./cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll
ERROR: config/sync-files.py:0 -     ./cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll
ERROR: config/sync-files.py:0 -     ./csharp/ql/src/experimental/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll
Error: Process completed with exit code 1.

for a in cpp csharp; do echo $a; git log --oneline --graph main..spelling-$a -- ./cpp/ql/lib/semmle/code/cpp/ir/implementation ./csharp/ql/src/experimental/ir/implementation ; done
cpp
* 991067cb20 spelling: instructions
* ab88b3f56d spelling: hypothetical
* e2f8d7e1ab spelling: exploit
* b244d01a37 spelling: dynamic
* 87d1a8b02a spelling: different
* ad192497ba spelling: circuit
* d3802b378e spelling: aliased
csharp
* 3afbdf7f1b spelling: opposed
* b61e02cc17 spelling: known
* a3f1affff7 spelling: instructions
* 0e09910601 spelling: hypothetical
* 9383e1f549 spelling: dynamic
* bf7011612c spelling: different

I'll need to pull some changes into one of these two and drop them from the other (or kick them into a third). I've done each and have no particular preference.

jsoref

I've synced over the cpp/csharp shared unit bits using something equivalent to this:

while [ -e .git/rebase-merge/git-rebase-todo ] ; do
  python3 config/sync-files.py --latest
  git add -u
  GIT_EDITOR=true git rebase --continue
done

jsoref · 2022-10-14T14:52:12Z

 * containing the `ExitFunction` instruction for that function.
 *
- * There are two differet return instructions: `ReturnValueInstruction`, for returning a value from
+ * There are two different return instructions: `ReturnValueInstruction`, for returning a value from


equivalent to 764a9e5#diff-0bf207ef7b91cba94e929ed8c56fcba31c81ed3c8605898b6150ebdfea9ee641R745

jsoref · 2022-10-14T14:52:59Z

 * If the operand holds a null address, the result is a null address.
 *
- * This instruction is used to represent `dyanmic_cast<void*>` in C++, which returns the pointer to
+ * This instruction is used to represent `dynamic_cast<void*>` in C++, which returns the pointer to


equivalent to 9db0c2c#diff-0bf207ef7b91cba94e929ed8c56fcba31c81ed3c8605898b6150ebdfea9ee641R1334

jsoref · 2022-10-14T14:53:16Z

 * If the operand holds a null address, the result is a null address.
 *
- * This instruction is used to represent `dyanmic_cast<void*>` in C++, which returns the pointer to
+ * This instruction is used to represent `dynamic_cast<void*>` in C++, which returns the pointer to


equivalent to 9db0c2c#diff-546a0e7794007683dcf7eec6abc89108b551fc445c071009b996358069e24ee2R1334

jsoref · 2022-10-14T14:53:57Z

    instr = reusedPhiInstruction(_) and
    // Check that the phi instruction is *not* degenerate, but we can't use
-    // getDegeneratePhiOperand in the first stage with phi instyructions
+    // getDegeneratePhiOperand in the first stage with phi instructions


equivalent to 73993ed#diff-f5cf97073ed3726cc84eb446644c71394729d149e48b27d8a26ea7099a821d3bR67

jsoref · 2022-10-14T14:54:24Z


  /**
-   * Gets the rank index of a hyphothetical use one instruction past the end of
+   * Gets the rank index of a hypothetical use one instruction past the end of


equivalent to 10ec41b#diff-f5cf97073ed3726cc84eb446644c71394729d149e48b27d8a26ea7099a821d3bR721

jketema · 2022-10-14T18:55:52Z

Yeah, so all changes related to "IR" (see directory name), are indeed best dealt with on the C++ side, moving the related C# changes to this PR is the correct thing to do.

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

geoffw0

LGTM. 👍

jketema · 2022-10-17T08:43:07Z

@jsoref Thanks for these fixes

spelling: access

413fa3d

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

jsoref requested a review from a team as a code owner October 13, 2022 23:59

github-actions Bot added C++ documentation labels Oct 13, 2022

jsoref commented Oct 14, 2022

View reviewed changes

jketema reviewed Oct 14, 2022

View reviewed changes

geoffw0 reviewed Oct 14, 2022

View reviewed changes

github-advanced-security AI found potential problems Oct 14, 2022

View reviewed changes

jsoref commented Oct 14, 2022

View reviewed changes

Comment thread cpp/ql/lib/semmle/code/cpp/metrics/MetricFile.qll Outdated

jsoref force-pushed the spelling-cpp branch from 7f26378 to a9965eb Compare October 14, 2022 14:30

jsoref force-pushed the spelling-cpp branch from a9965eb to 0d2dee9 Compare October 14, 2022 14:50

jsoref requested a review from a team as a code owner October 14, 2022 14:50

github-actions Bot added the C# label Oct 14, 2022

jsoref commented Oct 14, 2022

View reviewed changes

jsoref added 14 commits October 14, 2022 15:08

spelling: another class

5bc8004

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: aliased

dc2c4f7

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: alignof

950ce8a

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: approximation

0fe91cb

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: behaviour

46b3f9a

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: certain

2a3e2d3

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: circuit

0536fb7

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: configuration

56684ca

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: corresponds

2027278

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: declared

1c290b9

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: description

ff6ce9c

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: determined

df7bdcd

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: different

68ea820

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: dimensions

488dd3f

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

jsoref added 23 commits October 14, 2022 15:08

spelling: initializers

d122858

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: instructions

1453efe

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: interesting

324aa61

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: justified

5a109c9

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: label

0fc69a4

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: library

0c3eb53

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: obtain

1e34019

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: operand

5dd61e3

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: opposed

aa3d43a

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: optimistically

aa70b97

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: overrunning

f7a1647

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: predicate

004f367

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: primitive

bbc9931

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: proposed

281bb0e

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: provides

ad3f6f4

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: representing

a588bcf

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: sequence

c5acca4

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: specified

b75b0ba

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: supplemental

1f8aaf7

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: themselves

9a32aba

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: variable

dd5c455

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: vulnerabilities

360ce7b

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

spelling: whose

86ad9f5

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

jsoref force-pushed the spelling-cpp branch from 0d2dee9 to 86ad9f5 Compare October 14, 2022 19:09

C++: Fix test after spelling fixes

45a0b66

jketema approved these changes Oct 15, 2022

View reviewed changes

geoffw0 approved these changes Oct 17, 2022

View reviewed changes

jketema merged commit 720efd6 into github:main Oct 17, 2022

jsoref deleted the spelling-cpp branch October 18, 2022 23:39

Conversation

jsoref commented Oct 13, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jketema left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geoffw0 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Check warning

Uh oh!

Uh oh!

jsoref commented Oct 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsoref left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jketema commented Oct 14, 2022

Uh oh!

geoffw0 left a comment

Choose a reason for hiding this comment

Uh oh!

jketema commented Oct 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jsoref commented Oct 14, 2022 •

edited

Loading