=?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