=?utf-8?q?Félix?= Cloutier <fclout...@apple.com>,
=?utf-8?q?Félix?= Cloutier <fclout...@apple.com>,
=?utf-8?q?Félix?= Cloutier <fclout...@apple.com>,
=?utf-8?q?Félix?= Cloutier <fclout...@apple.com>,
=?utf-8?q?Félix?= Cloutier <fclout...@apple.com>,
=?utf-8?q?Félix?= Cloutier <fclout...@apple.com>,
=?utf-8?q?Félix?= Cloutier <fclout...@apple.com>,
=?utf-8?q?Félix?= Cloutier <fclout...@apple.com>,
=?utf-8?q?Félix?= Cloutier <fclout...@apple.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/116...@github.com>


================
@@ -7400,7 +7407,25 @@ bool DecomposePrintfHandler::GetSpecifiers(
       [](const EquatableFormatArgument &A, const EquatableFormatArgument &B) {
         return A.getPosition() < B.getPosition();
       });
-  return true;
+
+  // If there are duplicate positional arguments, ensure that they are all
+  // mutually compatible.
+  bool HadError = false;
+  auto ArgIter = Args.begin();
+  auto ArgEnd = Args.end();
+  while (ArgIter != ArgEnd) {
----------------
apple-fcloutier wrote:

At a high level, this groups `Args` by value of `getPosition()`, and for each 
group, we check that all arguments starting from index 1 are compatible with 
the argument at index 0. `Args` is already sorted by `getPosition()`, so the 
act of grouping is just to find the end iterator of each range. In `std::`, we 
have `lower_bound`/`upper_bound` that can be used to find the first and the 
one-past-the-end iterators for any given `getPosition()` value, but since we're 
going through the whole array anyways, I started at `begin()` and used 
`find_if` instead of `upper_bound`.

Now that you challenged me to find another way to write the same thing, I came 
up with this:

```
  bool HadError = false;
  auto Iter = Args.begin();
  auto End = Args.end();
  // Group arguments by getPosition() value, and check that each member of the 
group is
  // compatible with the first member. As an optimization, don't test the first 
member
  // against itself.
  while (Iter != End) {
    const auto &First = *Iter;
    for (++Iter; Iter != End && Iter->getPosition() == First.getPosition(); 
++Iter) {
      HadError |= !Iter->VerifyCompatible(*this, First, Str, true);
    }
  }
```

which I think I like better, and if you like it better too, I will go with that.

https://github.com/llvm/llvm-project/pull/116708
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to