Skip to content

Conversation

@vedranmiletic
Copy link
Contributor

@vedranmiletic vedranmiletic commented Nov 27, 2025

Building LLVM with CMAKE_CXX_STANDARD set to 20 fails since the iterator facade is not fully compatible with C++20. To make it compatible, specific operator overloads have to be constrained.

Overload for operator- in ADT iterator is now constrained with concept BaseT::IsRandomAccess.

Patch by Jonathan Wakely.

Fixes #139072.

Overload for operator- in ADT iterator is now constrained with concept
BaseT::IsRandomAccess.

Patch by Jonathan Wakely.

Fixes llvm#139072.
@llvmbot
Copy link
Member

llvmbot commented Nov 27, 2025

@llvm/pr-subscribers-llvm-adt

Author: Vedran Miletić (vedranmiletic)

Changes

Overload for operator- in ADT iterator is now constrained with concept BaseT::IsRandomAccess.

Patch by Jonathan Wakely.

Fixes #139072.


Full diff: https://github.com/llvm/llvm-project/pull/169772.diff

1 Files Affected:

  • (modified) llvm/include/llvm/ADT/iterator.h (+5-1)
diff --git a/llvm/include/llvm/ADT/iterator.h b/llvm/include/llvm/ADT/iterator.h
index 6f0c42fe08bec..f55f9d4a3e7f2 100644
--- a/llvm/include/llvm/ADT/iterator.h
+++ b/llvm/include/llvm/ADT/iterator.h
@@ -267,7 +267,11 @@ class iterator_adaptor_base
     return *static_cast<DerivedT *>(this);
   }
   using BaseT::operator-;
-  difference_type operator-(const DerivedT &RHS) const {
+  difference_type operator-(const DerivedT &RHS) const
+#ifdef __cpp_concepts
+    requires(bool(BaseT::IsRandomAccess))
+#endif
+  {
     static_assert(
         BaseT::IsRandomAccess,
         "The '-' operator is only defined for random access iterators.");

Copy link
Contributor

@jwakely jwakely left a comment

Choose a reason for hiding this comment

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

As noted at #139072 (comment) there are other operators which should really be constrained similarly. So this fixes the immediate issue, but there are still latent bugs that might cause similar errors later.

@jwakely
Copy link
Contributor

jwakely commented Nov 27, 2025

but there are still latent bugs that might cause similar errors later.

But they could be fixed later, if needed.

@cor3ntin
Copy link
Contributor

Please update the description of the PR with a description of the problem you are trying to fix, I had to find this comment to understand the context. Thanks
#139072 (comment)

@github-actions
Copy link

github-actions bot commented Nov 27, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@vedranmiletic
Copy link
Contributor Author

Please update the description of the PR with a description of the problem you are trying to fix, I had to find this comment to understand the context. Thanks #139072 (comment)

Hope it is better now.

@vedranmiletic
Copy link
Contributor Author

Can someone merge this for me? I don't have commit access.

@kuhar kuhar merged commit a666d1f into llvm:main Dec 23, 2025
10 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Dec 23, 2025

LLVM Buildbot has detected a new failure on builder ppc64le-mlir-rhel-clang running on ppc64le-mlir-rhel-test while building llvm at step 6 "test-build-check-mlir-build-only-check-mlir".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/129/builds/35481

Here is the relevant piece of the build log for the reference
Step 6 (test-build-check-mlir-build-only-check-mlir) failure: 1200 seconds without output running [b'ninja', b'check-mlir'], attempting to kill
...
PASS: MLIR-Unit :: Dialect/SparseTensor/./MLIRSparseTensorTests/95/210 (3773 of 3784)
PASS: MLIR-Unit :: Dialect/SparseTensor/./MLIRSparseTensorTests/97/210 (3774 of 3784)
PASS: MLIR-Unit :: IR/./MLIRIRTests/0/131 (3775 of 3784)
PASS: MLIR-Unit :: IR/./MLIRIRTests/39/131 (3776 of 3784)
PASS: MLIR-Unit :: IR/./MLIRIRTests/38/131 (3777 of 3784)
PASS: MLIR-Unit :: Interfaces/./MLIRInterfacesTests/11/22 (3778 of 3784)
PASS: MLIR-Unit :: Interfaces/./MLIRInterfacesTests/12/22 (3779 of 3784)
PASS: MLIR-Unit :: Interfaces/./MLIRInterfacesTests/13/22 (3780 of 3784)
PASS: MLIR-Unit :: Pass/./MLIRPassTests/11/14 (3781 of 3784)
PASS: MLIR :: mlir-reduce/dce-test.mlir (3782 of 3784)
command timed out: 1200 seconds without output running [b'ninja', b'check-mlir'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=4205.691615

@vedranmiletic
Copy link
Contributor Author

Thanks!

valadaptive pushed a commit to valadaptive/llvm-project that referenced this pull request Dec 24, 2025
Building LLVM with CMAKE_CXX_STANDARD set to 20 fails since the iterator
facade is not fully compatible with C++20. To make it compatible,
specific operator overloads have to be constrained.

Overload for operator- in ADT iterator is now constrained with concept
BaseT::IsRandomAccess.

Patch by Jonathan Wakely.

Fixes llvm#139072.

---------

Co-authored-by: A. Jiang <[email protected]>
Co-authored-by: Jakub Kuderski <[email protected]>
jtuyls added a commit to iree-org/llvm-project that referenced this pull request Dec 26, 2025
jtuyls added a commit to iree-org/iree that referenced this pull request Dec 26, 2025
Integrate
llvm/llvm-project@c0f4a8a

Existing local reverts carried forward:
* Local revert of llvm/llvm-project#169614 due
to llvm/llvm-project#172932.

Also adds a revert for a commit that breaks the MSVC build:
* Local revert of llvm/llvm-project#169772. This
can be dropped after llvm/llvm-project#173495.

Signed-off-by: Jorn Tuyls <[email protected]>
jtuyls added a commit to iree-org/iree that referenced this pull request Dec 29, 2025
Integrate
llvm/llvm-project@75a0347

Removes a revert:
* Local revert of llvm/llvm-project#169772 as
the fix in llvm/llvm-project#173495 got merged.

Existing local reverts carried forward:
* Local revert of llvm/llvm-project#169614 due
to llvm/llvm-project#172932.

Signed-off-by: Jorn Tuyls <[email protected]>
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jan 6, 2026
Building LLVM with CMAKE_CXX_STANDARD set to 20 fails since the iterator
facade is not fully compatible with C++20. To make it compatible,
specific operator overloads have to be constrained.

Overload for operator- in ADT iterator is now constrained with concept
BaseT::IsRandomAccess.

Patch by Jonathan Wakely.

Fixes llvm#139072.

---------

Co-authored-by: A. Jiang <[email protected]>
Co-authored-by: Jakub Kuderski <[email protected]>
keshavvinayak01 pushed a commit to iree-org/iree that referenced this pull request Jan 27, 2026
Integrate
llvm/llvm-project@c0f4a8a

Existing local reverts carried forward:
* Local revert of llvm/llvm-project#169614 due
to llvm/llvm-project#172932.

Also adds a revert for a commit that breaks the MSVC build:
* Local revert of llvm/llvm-project#169772. This
can be dropped after llvm/llvm-project#173495.

Signed-off-by: Jorn Tuyls <[email protected]>
Signed-off-by: Keshav Vinayak Jha <[email protected]>
keshavvinayak01 pushed a commit to iree-org/iree that referenced this pull request Jan 27, 2026
Integrate
llvm/llvm-project@75a0347

Removes a revert:
* Local revert of llvm/llvm-project#169772 as
the fix in llvm/llvm-project#173495 got merged.

Existing local reverts carried forward:
* Local revert of llvm/llvm-project#169614 due
to llvm/llvm-project#172932.

Signed-off-by: Jorn Tuyls <[email protected]>
Signed-off-by: Keshav Vinayak Jha <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Building LLVM 20.1.4 fails with C++20

7 participants