C++: Add ECMAScript `std::regex` parser by jketema · Pull Request #22300 · github/codeql

jketema · 2026-08-07T16:25:17Z

Commit-by-commit review recommended. This is not hooked into anything yet, as this is getting quite big already, so I didn't want to add the flow config here, or other regex alternatives that can be used with std::regex.

Approach:

Import the Ruby parser, make QL and tests compile, fix position issue with raw strings
Remove unsupported features
Add features that are not supported by the Ruby parser
Add missing test cases
Some after-the-fact fixes.

Note that we are currently still implementing what Ruby thinks a regex is. This is not correct as C++ by default uses a variant of ECMAScript regexes. We will address this in the follow-up commits.

Strings can have a prefix in C++, which affects the location.

Clean up test source while here

Copilot

Pull request overview

Adds an ECMAScript-compatible std::regex parser and parse-tree model for future C++ data-flow integration.

Changes:

Adds parser and regex tree-view libraries.
Supports raw-string source locations and shared regex utilities.
Adds comprehensive parsing, value, and location tests.

Show a summary per file

File	Description
`cpp/ql/lib/semmle/code/cpp/regex/internal/ParseRegExp.qll`	Implements regex parsing.
`cpp/ql/lib/semmle/code/cpp/regex/RegexTreeView.qll`	Exposes parsed regex trees.
`cpp/ql/lib/semmle/code/cpp/exprs/Literal.qll`	Models raw string literals.
`cpp/ql/lib/qlpack.yml`	Adds the regex dependency.
`cpp/ql/lib/change-notes/2026-07-23-std-regex-ecmascript-parser.md`	Documents the feature.
`cpp/ql/test/library-tests/regex/regexp.cpp`	Provides regex test inputs.
`cpp/ql/test/library-tests/regex/regexp.ql`	Tests parsed terms and values.
`cpp/ql/test/library-tests/regex/regexp.expected`	Records expected query results.
`cpp/ql/test/library-tests/regex/parse.ql`	Produces parse-tree graphs.
`cpp/ql/test/library-tests/regex/parse.expected`	Records expected parse trees.
`cpp/ql/test/library-tests/regex/locations.ql`	Tests source locations.
`cpp/ql/test/library-tests/regex/locations.expected`	Records expected locations.

Review details

Tip

Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (3)

cpp/ql/lib/semmle/code/cpp/regex/RegexTreeView.qll:834

The single-character claim contradicts this implementation and the new abc test result, where one RegExpConstant represents the complete three-character constant. Remove that claim to keep the public class documentation accurate.

   * A constant regular expression term, that is, a regular expression
   * term matching a single string. Currently, this will always be a single character.

cpp/ql/lib/semmle/code/cpp/regex/internal/ParseRegExp.qll:65

Remove the accidentally duplicated explanation.

            // check if the character that comes before the previous closing bracket
            // is an opening bracket (taking `^` into account)
            // check if the character that comes before the previous closing bracket
            // is an opening bracket (taking `^` into account)

cpp/ql/lib/semmle/code/cpp/regex/RegexTreeView.qll:604

Add the missing possessive apostrophe.

    /** Holds if this terms name is given by the part following the escape character. */

Files reviewed: 12/12 changed files
Comments generated: 5
Review effort level: Balanced

+      result.getEnd() = part_end
+    }
+
+    /** Hodls if this term may match an unlimited number of times. */


geoffw0 · 2026-08-27T15:29:27Z

+   * A normal character in a regular expression, that is, a character
+   * without special meaning. This includes escaped characters.


Rename to RegExpNormalChars???

geoffw0 · 2026-08-27T15:31:05Z

+  }
+
+  /**
+   * Holds if `root` has the `s` flag for multi-line matching.


Yep, this appears to be a stale comment.

I'm not actually sure about that. The predicate name seems very specific to one specific regex version parsed in one of the languages where we have a similar regex library. I'll double check though.

+ */
+abstract class RegExp extends StringLiteral {
+  /**
+   * Holds if this `RegExp` has the `s` flag for multi-line matching.


Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

geoffw0

Review part 1. Thank you for breaking this PR into logical commits, with commit comments explaining the changes - this was extremely helpful for making sense of the changes you've made from the Ruby implementation I'm already somewhat familiar with.

I've also created a small PR jketema#33 onto this PR, addressing an edge case with string locations that I investigated, and figured I might as well trivially fix and share my code changes for.

geoffw0 · 2026-08-27T11:00:58Z

+std::regex r_cc4("\\[\\][123]");
+std::regex r_cc5("[^A-Z]");
+std::regex r_cc6("[]]");   // MRI gives a warning, but accepts this as matching ']'
+std::regex r_cc7("[^]]"); // MRI gives a warning, but accepts this as matching anything except ']'


What is MRI?

https://en.wikipedia.org/wiki/Ruby_MRI

This is a left-over from Ruby tests that I copied. We should probably just remove the comments.

geoffw0 · 2026-08-27T11:07:26Z

+std::basic_regex<char16_t> r_loc_uR(uR"(a\nc)");
+std::basic_regex<char32_t> r_loc_UR(UR"(a\nc)");
+std::regex r_loc_Rx(R"x(a\nc)x");
+std::regex r_loc_Rfoo(R"foo(a\nc)foo");


Should we have test cases for invalid regexs to ensure nothing too strange happens there - and for that matter string literals that are not remotely regexs, to ensure they are not misidentified as such.

Is there anything specific you're thinking about?

Nothing in particular, just (1) sometimes people write invalid regexs, and sometimes libraries even accept them so they don't get spotted; (2) real world databases will contain a lot of string literals that aren't regexs, so it's important we don't do anything silly (or computationally expensive for that matter) with them. I can't remember how this worked in Ruby / Python / Swift.

I can't remember how this worked in Ruby / Python / Swift.

I think the idea is that we only look at strings that actually flow into the correct std::regex function arguments. That is clearly missing at this point.

geoffw0 · 2026-08-27T11:21:45Z

+ * const wchar_t *s2 = LR"x(123456)x";
+ * ```
+ */
+class RawStringLiteral extends StringLiteral {


It might be nice to have a test for RawStringLiteral itself, even if it's just a very basic test as part of the regex tests e.g.

from RawStringLiteral rsl select rsl

Makes sense. Will add.

jketema · 2026-08-27T12:32:05Z

I've also created a small PR jketema#33 onto this PR, addressing an edge case with string locations that I investigated, and figured I might as well trivially fix and share my code changes for.

Commented on that here: https://github.com/jketema/codeql/pull/33/files#r3871704321

geoffw0

Review part 2. I've now reviewed all of the code and tests.

I'm very happy with what I see, you've clearly put considerably effort into getting this right ... though experience suggests we should be prepared to find bugs and missing features when we start to implement queries on top of the library, as doing so will test it more deeply.

I'm going to do a few quick tests locally, but I'm leaning towards getting this merged quickly so we can build on it.

👍

geoffw0 · 2026-08-27T13:30:36Z

+    basic_regex(const CharT *s, int flags) {}
+    basic_regex &assign(const CharT *s) { return *this; }
+  };
+  typedef basic_regex<char> regex;


Do we want to test std::wregex as well?

And initialization constants such as std::regex_constants::icase? (which I think we can support fairly easily, when we're ready to add that)

And non-ECMA regexs? (which I guess, realistically, we plan to either ignore for the time being???)

icase requires dataflow.

non-ECMA regexes are on my todo list but only after I've rigged up whatever else I need to get to a sensible Redos query. These additional regex formats seem relatively easy to add, but again some dataflow will be required to determine which regex format is being used.

Do we want to test std::wregex as well?

I'm not sure that's adding much at this stage, but let me add the needed prototypes and at least one test.

geoffw0 · 2026-08-27T14:44:40Z

-
-    /**
-     * Holds if the property is inverted. For example, it holds for `[[:^digit:]]`,
-     * which matches non-digits.


Again, I'm a bit surprised this isn't supported.

I think this is pretty Ruby specific?

On https://en.cppreference.com/cpp/regex/ecmascript it has an example [^[:digit:]], but that's slightly different, and looking at it now I think it's just a regular use of [^...].

Indeed. [^[:digit:]] should be covered by the regular [^...] case. I'll add a test case of that if there isn't one already.

geoffw0 · 2026-08-27T15:01:39Z

+std::regex r_posix3("[A-F[:digit:]a-f]");
+
+// POSIX collating symbols
+std::regex r_posix_coll1("[[.a.]]");


If I'm understanding correctly (I'm not confident I am) this is a somewhat strange example - since the collating symbol is intended to join several characters as though they are one?

That might be. I'll have a look and update the test if appropriate.

geoffw0 · 2026-08-27T15:17:37Z

+      not this.isUnicode() and
+      not this.isHex() and
+      not this.isControl() and


I believe these are redundant, as they're checked inside isIdentityEscape.

Suggested change

not this.isUnicode() and

not this.isHex() and

not this.isControl() and

You might be right. I'll check.

geoffw0 · 2026-08-27T15:29:27Z

+   * A normal character in a regular expression, that is, a character
+   * without special meaning. This includes escaped characters.


Rename to RegExpNormalChars???

geoffw0 · 2026-08-27T15:31:05Z

+  }
+
+  /**
+   * Holds if `root` has the `s` flag for multi-line matching.


Yep, this appears to be a stale comment.

geoffw0 · 2026-08-27T17:47:51Z

I'm going to do a few quick tests locally

I've now done my local testing. I noticed locations of regex terms where the regex includes one or more escape sequences (\\) are a little bit off. It doesn't need fixing now, but it's worth noting I think. Other than that I'm happy.

Nothing else to review, I'm happy to approve this when everything above has been answered. I don't think anything I've asked for is critical at this stage (as no queries use the library yet), so feel free to write up a TODO list issue instead of fixing every detail now. It will be good to have some code in main. 🚀

jketema · 2026-08-27T17:50:56Z

I noticed locations of regex terms where the regex includes one or more escape sequences (\\) are a little bit off.

I saw that. I tried fixing that, but that turned out to be non-trivial. I'll add a note in the qll file on this.

Copilot AI and others added 30 commits August 7, 2026 14:26

C++: Copy Ruby regex library and tests verbatim

7f959bf

C++: Make regex library compile

4096725

C++: Make regex test work

cc4f5bd

Note that we are currently still implementing what Ruby thinks a regex is. This is not correct as C++ by default uses a variant of ECMAScript regexes. We will address this in the follow-up commits.

C++: Add location tests for all literal prefixes

d5d73f6

C++: Add RawStringLiteral class

322c1c0

C++: Fix regex starting location

fd929e2

Strings can have a prefix in C++, which affects the location.

C++: Remove Ruby-only \A/\Z/\z/\G anchors from regex grammar

eb53e56

C++: Remove Ruby-only \h and \H character class escapes

6e71f00

C++: Remove Ruby-style \p and \P named character properties

a471af9

C++: Remove Ruby-style (?#...) comment groups

c3fd966

C++: Remove single-quote named capture groups

b2d0766

C++: Remove named capture groups

985a2ba

C++: Remove named backreferences

bb96e90

C++: Remove lookbehind assertions

da931e4

C++: Remove {,m} quantifier form

0a3887d

C++: Remove inverted named character properties

64a9a75

C++: Add control-escape tests (pre-fix)

24251de

Clean up test source while here

C++: Implement \cX control escapes

8d45199

C++: Add NUL-escape tests (pre-fix)

87cdedd

C++: Implement \0 NUL escape

81783e6

C++: Add POSIX collating-symbol tests (pre-fix)

03625be

C++: Fix typo

32e3a11

C++: Implement POSIX collating symbols

57f5d03

C++: Add POSIX equivalence-class tests (pre-fix)

96c9214

C++: Implement POSIX equivalence classes

5cce52e

C++: Replace non-standard \u{...} test with \uhhhh

b1e7fb7

C++: Add coverage for lazy quantifiers and \f/\v escapes

f7797ed

C++: Verify [\b] is backspace inside character classes

009ae43

C++: Add change note for std::regex ECMAScript parser alignment

66c191a

C++: Drop unneeded \U from isUnicode

b2fc6a1

jketema added 6 commits August 7, 2026 17:45

C++: add hex escape test

aa14162

C++: Handle \0 in RegExpEscape

ff038a2

C++: Handle \cX in RegExpEscape and slightly tweak test

40feb22

C++: Handle \xhh in RegExpEscape

e6cfe17

C++: Fix change note category

b30d9d6

C++: Fix off-by-one in \cX handling

aca8aa9

github-actions Bot added C++ documentation labels Aug 7, 2026

github-advanced-security Bot found potential problems Aug 7, 2026

View reviewed changes

Comment thread cpp/ql/lib/semmle/code/cpp/regex/RegexTreeView.qll Fixed

Comment thread cpp/ql/lib/semmle/code/cpp/regex/RegexTreeView.qll Fixed

Comment thread cpp/ql/lib/semmle/code/cpp/regex/RegexTreeView.qll Fixed

jketema added 2 commits August 7, 2026 18:32

C++: Fix QL-for-QL errors

e95ae40

C++: Add RegExpNamedCharacterProperty subclasses

8a41ec0

jketema marked this pull request as ready for review August 10, 2026 09:25

jketema requested a review from a team as a code owner August 10, 2026 09:25

Copilot AI balanced review requested due to automatic review settings August 10, 2026 09:25

Copilot started reviewing on behalf of jketema August 10, 2026 09:25 View session

Copilot AI reviewed Aug 10, 2026

View reviewed changes

jketema and others added 2 commits August 10, 2026 11:30

Fix typo

fbd8737

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Fix typo

b15082f

andersfugmann requested a review from geoffw0 August 19, 2026 11:22

andersfugmann assigned geoffw0 Aug 19, 2026

geoffw0 mentioned this pull request Aug 27, 2026

C++: Small locations fix for regex PR jketema/codeql#33

Open

geoffw0 reviewed Aug 27, 2026

View reviewed changes

		* A normal character in a regular expression, that is, a character
		* without special meaning. This includes escaped characters.

	not this.isUnicode() and
	not this.isHex() and
	not this.isControl() and

Uh oh!

Conversation

jketema commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

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!

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!

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 Aug 27, 2026

Uh oh!

geoffw0 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jketema Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geoffw0 Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jketema Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geoffw0 commented Aug 27, 2026

Uh oh!

jketema commented Aug 27, 2026

jketema commented Aug 7, 2026 •

edited

Loading

jketema Aug 27, 2026 •

edited

Loading

geoffw0 Aug 27, 2026 •

edited

Loading

jketema Aug 27, 2026 •

edited

Loading