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

Björn Schäpers via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 20 02:40:20 PST 2023


HazardyKnusperkeks added inline comments.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2544
   SmallVector<Context, 8> Contexts;
+  static SmallVector<ScopeType, 8> Scopes;
 
----------------
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.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2935
     return true; // Empty parentheses.
+
   // If there is an &/&& after the r_paren, this is likely a function.
----------------
Unrelated.


================
Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:181
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
----------------
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.


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

https://reviews.llvm.org/D141959



More information about the cfe-commits mailing list