owenpan added inline comments.
================ Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:820-833 + ASSERT_EQ(BaseTokens.size(), NumberOfBaseTokens) << BaseTokens; + ASSERT_EQ(ConstrainedTokens.size(), + NumberOfBaseTokens + NumberOfAdditionalRequiresClauseTokens) + << ConstrainedTokens; + + for (auto I = 0u; I < NumberOfBaseTokens; ++I) { + if (I < NumberOfTokensBeforeRequires) { ---------------- curdeius wrote: > owenpan wrote: > > HazardyKnusperkeks wrote: > > > curdeius wrote: > > > > owenpan wrote: > > > > > Can you make it a function or lambda? > > > > :+1: > > > Often thought about that. But as @MyDeveloperDay mentioned in different > > > other reviews, we would loose the line where the EXPECT failed, since it > > > would always be the same line. > > > > > > One step to mitigate that would be to return a `bool`, then one would > > > loose the "subexpect", only knows which subtest failed. > > > > > > But an idea I have right now would be to add a StringRef parameter which > > > is then fed into the expect/assert to identify the subtest. > > Let's take care of this in a separate patch then. If a function or lambda > > won't do, we can at least use a macro instead? > You can then add a function taking a source location maybe (+ optionally a > macro passing `__FILE__` and `__LINE__`). WDYT? See D129982. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129940/new/ https://reviews.llvm.org/D129940 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits