Skip to content

C#: Introduce Expr.getIntValue.#21701

Merged
aschackmull merged 1 commit intogithub:mainfrom
aschackmull:csharp/intvalue
Apr 14, 2026
Merged

C#: Introduce Expr.getIntValue.#21701
aschackmull merged 1 commit intogithub:mainfrom
aschackmull:csharp/intvalue

Conversation

@aschackmull
Copy link
Copy Markdown
Contributor

I noticed that the Expr.getValue().toInt() pattern was generating some poor performance due to lots of repeated string operations, so at first I just wanted to cache this to improve performance, but then I noticed that the pattern actually also yields results for string and char literals (e.g. "4" and '4' both yield 4), which is very unlikely to be intended in the various uses of this pattern. So besides improving some performance, this may also fix some bugs.

@github-actions github-actions bot added the C# label Apr 13, 2026
@aschackmull aschackmull added the no-change-note-required This PR does not need a change note label Apr 14, 2026
@aschackmull aschackmull marked this pull request as ready for review April 14, 2026 08:52
@aschackmull aschackmull requested a review from a team as a code owner April 14, 2026 08:52
Copilot AI review requested due to automatic review settings April 14, 2026 08:52
Copy link
Copy Markdown
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 PR introduces a new Expr.getIntValue() helper in the C# QL libraries to replace the common getValue().toInt() pattern, improving performance (via caching) and tightening semantics to only integral/enum-typed expressions.

Changes:

  • Added cached Expr.getIntValue() in the core expression library.
  • Updated a set of queries/libraries to use getIntValue() instead of getValue().toInt() (and in one place replaced a string comparison "0" with an integer comparison 0).
Show a summary per file
File Description
csharp/ql/lib/semmle/code/csharp/exprs/Expr.qll Adds the new cached getIntValue() API on Expr.
csharp/ql/src/Security Features/InsufficientKeySize.ql Uses getIntValue() for key-size threshold checks.
csharp/ql/src/Likely Bugs/PossibleLossOfPrecision.ql Uses getIntValue() for exact integer-division detection.
csharp/ql/src/Likely Bugs/MishandlingJapaneseEra.ql Uses getIntValue() for date/era argument checks (including era == 0).
csharp/ql/src/Likely Bugs/BadCheckOdd.ql Uses getIntValue() for parity-check constants and positivity checks.
csharp/ql/lib/semmle/code/csharp/frameworks/system/runtime/CompilerServices.qll Uses getIntValue() for attribute constructor argument extraction.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/ConstantUtils.qll Uses getIntValue() for recognizing constant integer expressions.
csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll Replaces local integer-constant parsing with getIntValue().
csharp/ql/lib/semmle/code/csharp/commons/Constants.qll Uses getIntValue() when computing integral min/max for constant comparisons.
csharp/ql/lib/semmle/code/csharp/commons/ComparisonTest.qll Uses getIntValue() for constant extraction in comparison-test logic.
csharp/ql/lib/semmle/code/csharp/Conversion.qll Uses getIntValue() in constant conversion predicates.
csharp/ql/examples/snippets/integer_literal.ql Updates snippet to demonstrate getIntValue().

Copilot's findings

  • Files reviewed: 12/12 changed files
  • Comments generated: 0

Copy link
Copy Markdown
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

LGTM!

@aschackmull aschackmull merged commit e095294 into github:main Apr 14, 2026
30 checks passed
@aschackmull aschackmull deleted the csharp/intvalue branch April 14, 2026 09:23
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