================
@@ -153,23 +153,27 @@ class SetVector {
}
/// Insert a new element into the SetVector.
- /// \returns true if the element was inserted into the SetVector.
- bool insert(const value_type &X) {
+ /// \returns a pair, whose first member is the iterator pointed to the
+ /// element, and the second member indicates if the element was inserted.
+ std::pair<iterator, bool> insert(const value_type &X) {
----------------
eugeneepshteyn wrote:
(This comment was generated with the help of AI)
This return type change silently changes three callers that still compile.
`mlir/lib/IR/AsmPrinter.cpp:1077`, `:2068` and
`mlir/lib/AsmParser/AsmParserImpl.h:608` do `return
success(Stack.insert(Ptr));`. With a `std::pair` argument `success(bool)` is no
longer viable, so the generic `template <typename T> success(T &&)` in
`llvm/Support/LogicalResult.h` binds instead. It returns a `FailureOr`, which
is always a success.
So `pushCyclicPrinting`/`pushCyclicParsing` never report re-entry, which breaks
printing and parsing of recursive LLVM-dialect and SPIR-V struct types. These
are the 8 MLIR failures in this PR's Linux CI
(`Dialect/LLVMIR/types-invalid.mlir`, `Dialect/SPIRV/IR/types.mlir`, ...): a
no-op push still pops, and the outer pop asserts in `SetVector::pop_back` on an
empty vector.
```cpp
#include "llvm/ADT/SetVector.h"
#include "llvm/Support/LogicalResult.h"
#include <cstdio>
int main() {
llvm::SetVector<int> S;
llvm::LogicalResult First = llvm::success(S.insert(1));
llvm::LogicalResult Dup = llvm::success(S.insert(1));
std::printf("first=%d dup=%d\n", llvm::succeeded(First),
llvm::succeeded(Dup));
}
```
```console
$ clang++ -std=c++17 -I llvm/include -I build/include success-rebind.cpp
build/lib/libLLVMSupport.a
main: first=1 dup=0
this PR: first=1 dup=1
```
The three sites need `.second`. Callers that pass the result through a generic
wrapper like this are the ones the compiler will not flag.
https://github.com/llvm/llvm-project/pull/232372
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
