[PATCH] D141959: [clang-format] Fix inconsistent identification of operator&

David K Turner via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 20 08:03:08 PST 2023


dkt01 marked an inline comment as done.
dkt01 added inline comments.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2544
   SmallVector<Context, 8> Contexts;
+  static SmallVector<ScopeType, 8> Scopes;
 
----------------
HazardyKnusperkeks wrote:
> dkt01 wrote:
> > HazardyKnusperkeks wrote:
> > > Why static?
> > The AnnotatingParser object lifetime is too short to track the scope information I wanted.  From my testing, it seems like this object is created once per line.  This is why I haven't added the scope information as part of the contexts member.
> > 
> > I would like to make this a non-static member, but I'm not sure which object it should be a member of.  Do you have a recommendation?
> The `TokenAnnotator`? And then by reference to the parser.
I was over-thinking it...  Adding to `TokenAnnotator` works great!
I did have to re-introduce the size checks before popping scopes because otherwise the UnbalancedStructuralElements test fails.


================
Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:181
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
----------------
HazardyKnusperkeks wrote:
> dkt01 wrote:
> > MyDeveloperDay wrote:
> > > how does this differentiate from 
> > > 
> > > `MyType & val2;`
> > > 
> > > is that a binary operator? I don't think so?
> > The heuristic I'm using is that [symbol] & [symbol]; outside a class/struct declaration is more likely an instance of operator& than an uninitialized reference.  Tests on line 206 and 215 demonstrate how the two cases differ.
> > The heuristic I'm using is that [symbol] & [symbol]; outside a class/struct declaration is more likely an instance of operator& than an uninitialized reference.  Tests on line 206 and 215 demonstrate how the two cases differ.
> 
> I'm with this one. A reference without initialization is an error.
> 
> Can you add a test with an assignment, I didn't see one right now.
Added two new tests for assignment to pointer using unary operator & and initializing assignment to reference because neither of these assignments was in existing format tests I could find.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141959/new/

https://reviews.llvm.org/D141959



More information about the cfe-commits mailing list