[PATCH] D129940: [clang-format] Fix misannotation of colon in presence of requires clause
Owen Pan via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Jul 17 21:52:03 PDT 2022
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
More information about the cfe-commits
mailing list