[M] Change in code/re2[main]: CMakeLists: updates for clang-cl, skipping benchmarks, and C++17

0 views
Skip to first unread message

Russ Cox (Gerrit)

unread,
Jul 30, 2025, 11:28:18 AMJul 30
to Alan Donovan, re2...@googlegroups.com
Attention needed from Alan Donovan

Russ Cox has uploaded the change for review

Russ Cox would like Alan Donovan to review this change.

Commit message

CMakeLists: updates for clang-cl, skipping benchmarks, and C++17

1. Test MSVC instead of CMAKE_CXX_COMPILER_ID MATCHES "MSVC"
When using the 'clang-cl' simulation of MSVC's 'cl' compiler,
we want to execute these adjustments, and MSVC will be true,
but CMAKE_CXX_COMPILER_ID MATCHES "MSVC" will not.
Thanks to Alexander Neumann for flagging and for the explanation.
Fixes #532.

2. Separate out building tests from building benchmarks, because
they have different dependencies, and some people might want to
build tests and skip the benchmarks.

3. Update to C++17 and factor that out to make future updates easier.
We require C++17 now per
https://github.com/google/oss-policies-info/blob/main/foundational-cxx-support-matrix.md
Change-Id: I5691847d5ff17636143981086f370e95e34900bd

Change information

Files:
  • M CMakeLists.txt
Change size: M
Delta: 1 file changed, 66 insertions(+), 57 deletions(-)
Open in Gerrit

Related details

Attention is currently required from:
  • Alan Donovan
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newchange
Gerrit-Project: re2
Gerrit-Branch: main
Gerrit-Change-Id: I5691847d5ff17636143981086f370e95e34900bd
Gerrit-Change-Number: 63774
Gerrit-PatchSet: 1
Gerrit-Owner: Russ Cox <r...@swtch.com>
Gerrit-Reviewer: Alan Donovan <adon...@google.com>
Gerrit-Attention: Alan Donovan <adon...@google.com>
unsatisfied_requirement
open
diffy

Alan Donovan (Gerrit)

unread,
Jul 30, 2025, 11:44:48 AMJul 30
to Russ Cox, re2...@googlegroups.com
Attention needed from Russ Cox

Alan Donovan voted and added 1 comment

Votes added by Alan Donovan

Code-Review+2

1 comment

Patchset-level comments
File-level comment, Patchset 1 (Latest):
Alan Donovan . resolved

The change looks plausible enough, but I don't really know anything about this file or CMake, so my approval is largely based on your reputation.

Open in Gerrit

Related details

Attention is currently required from:
  • Russ Cox
Submit Requirements:
  • requirement satisfiedCode-Review
  • requirement satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: re2
Gerrit-Branch: main
Gerrit-Change-Id: I5691847d5ff17636143981086f370e95e34900bd
Gerrit-Change-Number: 63774
Gerrit-PatchSet: 1
Gerrit-Owner: Russ Cox <r...@swtch.com>
Gerrit-Reviewer: Alan Donovan <adon...@google.com>
Gerrit-Attention: Russ Cox <r...@swtch.com>
Gerrit-Comment-Date: Wed, 30 Jul 2025 15:44:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Russ Cox (Gerrit)

unread,
Jul 30, 2025, 12:28:42 PMJul 30
to Alan Donovan, re2...@googlegroups.com

Russ Cox submitted the change

Change information

Commit message:
CMakeLists: updates for clang-cl, skipping benchmarks, and C++17

1. Test MSVC instead of CMAKE_CXX_COMPILER_ID MATCHES "MSVC"
When using the 'clang-cl' simulation of MSVC's 'cl' compiler,
we want to execute these adjustments, and MSVC will be true,
but CMAKE_CXX_COMPILER_ID MATCHES "MSVC" will not.
Thanks to Alexander Neumann for flagging and for the explanation.
Fixes #532.

2. Separate out building tests from building benchmarks, because
they have different dependencies, and some people might want to
build tests and skip the benchmarks.

3. Update to C++17 and factor that out to make future updates easier.
We require C++17 now per
https://github.com/google/oss-policies-info/blob/main/foundational-cxx-support-matrix.md
Change-Id: I5691847d5ff17636143981086f370e95e34900bd
Files:
  • M CMakeLists.txt
Change size: M
Delta: 1 file changed, 66 insertions(+), 57 deletions(-)
Branch: refs/heads/main
Submit Requirements:
  • requirement satisfiedCode-Review: +2 by Alan Donovan
Open in Gerrit
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: merged
Gerrit-Project: re2
Gerrit-Branch: main
Gerrit-Change-Id: I5691847d5ff17636143981086f370e95e34900bd
Gerrit-Change-Number: 63774
Gerrit-PatchSet: 2
Gerrit-Owner: Russ Cox <r...@swtch.com>
Gerrit-Reviewer: Alan Donovan <adon...@google.com>
Gerrit-Reviewer: Russ Cox <r...@swtch.com>
open
diffy
satisfied_requirement

Russ Cox (Gerrit)

unread,
Jul 30, 2025, 12:29:41 PMJul 30
to Alan Donovan, re2...@googlegroups.com

Russ Cox added 1 comment

Patchset-level comments
Alan Donovan . resolved

The change looks plausible enough, but I don't really know anything about this file or CMake, so my approval is largely based on your reputation.

Russ Cox

Plausible and tested is all I'm going for - I know little more than you do.
And I did test it.

Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Review
  • requirement satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: re2
Gerrit-Branch: main
Gerrit-Change-Id: I5691847d5ff17636143981086f370e95e34900bd
Gerrit-Change-Number: 63774
Gerrit-PatchSet: 2
Gerrit-Owner: Russ Cox <r...@swtch.com>
Gerrit-Reviewer: Alan Donovan <adon...@google.com>
Gerrit-Reviewer: Russ Cox <r...@swtch.com>
Gerrit-Comment-Date: Wed, 30 Jul 2025 16:29:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Alan Donovan <adon...@google.com>
satisfied_requirement
open
diffy
Reply all
Reply to author
Forward
0 new messages