[clang-format] Fix inconsistent identification of operator&

[dkt01]

Summary

Token annotator for clang-format incorrectly identifies operator& as a reference type in situations like Boost serialization archives.

Add annotation rules for standalone and chained operator& instances while preserving behavior for reference declarations at class scope. Add tests to validate annotation and formatting behavior.

[HazardyKnusperkeks]

Could this be added to Context instead of a new type?

clang/lib/Format/TokenAnnotator.cpp

1200

How does this happen? The only push I can see is before parseBrace, where a pop is following directly after.

1670

1671

Full stop at the end of your comments.

1922

You don't need to pass that, you can access it directly from determineStarAmpUsage, right?

2370

That const doesn't seem fit with the LLVM style.

2370

I'd prefer you write it out.

2474–2475

What about

struct S {
  auto Mem = C & D;
};

That would be annotated, or is there another rule above with would kick in? If there isn't a test with such a construct already please add one.

2480–2486

That seems better.

2525

Why static?

[dkt01]

clang/lib/Format/TokenAnnotator.cpp

2525

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?

[dkt01]

clang/lib/Format/TokenAnnotator.cpp

2474–2475

This case was already handled properly by the IsExpression && !Contexts.back().CaretFound condition, but I'll add this test case as there isn't an equivalent test yet.

[dkt01]

Changes based on HazardyKnusperkeks' review

[dkt01]

Further changes based on HazardyKnusperkeks' review

[dkt01]

clang/lib/Format/TokenAnnotator.cpp

1200

I previously had a double pop for some braces, but now the checks are no longer necessary as pushes and pops match.

[MyDeveloperDay]

clang/unittests/Format/TokenAnnotatorTest.cpp

181

how does this differentiate from

MyType & val2;

is that a binary operator? I don't think so?

[dkt01]

clang/unittests/Format/TokenAnnotatorTest.cpp

181

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.

[HazardyKnusperkeks]

clang/lib/Format/TokenAnnotator.cpp

2525

The TokenAnnotator? And then by reference to the parser.

2926

Unrelated.

clang/unittests/Format/TokenAnnotatorTest.cpp

181

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.

[dkt01]

clang/lib/Format/TokenAnnotator.cpp

2525

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.

clang/unittests/Format/TokenAnnotatorTest.cpp

181

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.

[dkt01]

Changes as suggested by HazardyKnusperkeks in review.

[HazardyKnusperkeks]

clang/lib/Format/TokenAnnotator.cpp

867

See below

868

Is this check also necessary? Here we should only be when we have pushed before.
I'd change the if for an assert.

1200

Move the comment in the if? In general it hasn't to be a unbalanced brace, right?

[dkt01]

Changes recommended by HazardyKnusperkeks in review.

[HazardyKnusperkeks]

Looks good to me, but please wait until another one has given their blessing, or at least no veto for a few days.

[MyDeveloperDay]

There is part of me that feels we should not rush this in to get it into 16 (which was due to branch today), I feel it might be a good idea to commit this after branching, and let it live in 'main' so we can take for a real test drive

[dkt01]

Sounds good. I don't believe I have permissions to add commits to three repo anyway, so someone can add on my behalf after the 16 branch.

[MyDeveloperDay]

I think they have branched (https://clang.llvm.org/docs/ClangFormatStyleOptions.html) the documentation has changed to 17.0

We would need your name and email in order to commit on your behalf.

I checked this patch out and tested it on my large project and I didn't see anything immediately jump out.

@owenpan, @rymiel any concerns?

[dkt01]

David Turner <[e-mail]>

Good to know it worked as expected on your project as well.

[MyDeveloperDay]

Good to know it worked as expected on your project as well.

I have some concerns (mainly because it feels like it could be quite invasive), hence I'm delaying on the LGTM, I really want to run this on a large code base, which I think if we commit post the branch (which is now branched) we would probably see more easily. Historically in the past I think the firefox team have kindly picked up recent builds (no obligations) and catch any regressions.

clang/lib/Format/TokenAnnotator.cpp

2768

was there a reason it could no longer be const?

[owenpan]

clang/lib/Format/TokenAnnotator.cpp

119–120

Please keep them in the same order as that of the parameters above.

868

Should we assert that the token type of OpeningBrace matches the ScopeType before popping?

1179–1186

Do we really need these? If we add other special l_brace token types, we will have to update this list.

1203

Can you leave the comment at the top of the case? If we get rid of None for ScopeType, we don't need to pop here. (See below.)

2481–2483

Can you add a lambda for this so that it can be used below?

2484–2486

NextToken is guarded against null on line 2488 but not on lines 2489-2490.

2487–2488

Here we can use the lambda suggested above.

2778

Instead of pushing None, would it better to leave Scopes empty? Then we would not need None for the ScopeType, and instead of checking for Scopes.size() > 1, we could check if it's empty().

clang/lib/Format/TokenAnnotator.h

193

Drop std::.

clang/unittests/Format/TokenAnnotatorTest.cpp

192–194

Can you add a case for periodstar?

207–213

Can you add a case for arrowstar?

[dkt01]

clang/lib/Format/TokenAnnotator.cpp

1179–1186

Agreed. This was left over from a prior implementation that didn't have a default case.

2484–2486

If NextToken is null on 2488, 2489-2490 will be short circuited.

2768

Member Scopes can be modified. Alternate would be to make Scopes mutable, but I thought removing const from this function is cleaner

[dkt01]

Changes as suggested in review from owenpan

[owenpan]

clang/lib/Format/TokenAnnotator.cpp

115–117

Something like the above. (I prefer Scopes to TrackedScopes to be consistent with the naming of the other parameters.)

1200–1203

I don't think it's about unbalanced braces here.

2479–2480

To help simplify the if statement below.

2484–2486

You are right. The redundant parens threw me off.

2485

Redundant parens.

2486–2487

The lambda would check that PrevToken is nonnull.

2488–2492

Something like that.

clang/lib/Format/TokenAnnotator.h

36–45

Consider moving ScopeType out of TokenAnnotator and keeping it consistent with the style of LineType above (and replacing TokenAnnotator::ScopeType with ScopeType everywhere).

193–200

See above.

[dkt01]

clang/lib/Format/TokenAnnotator.cpp

1200–1203

if (!Scopes.empty()) handles unbalanced braces. if(Tok->Previous) handles the case where a line starts with an rbrace.

2486–2487

!PrevToken->getPreviousNonComment() || is a different check than the null check in the lambda. It's acceptable to have no token two tokens prior because that indicates the ampersand is the second token of the line.

[dkt01]

Updates suggested in owenpan's review.

[owenpan]

clang/lib/Format/TokenAnnotator.cpp

123–125

129

868

1169–1181

1200–1203

I can't think of an example. Do you have one?

2479

Sorry about that!

2486–2487

I actually wanted fold this into the lambda. (See above.)

2486–2487

Now we only need the lambda.

[dkt01]

clang/lib/Format/TokenAnnotator.cpp

1200–1203

The format unit test FormatUnbalancedStructuralElements feeds in strings that have only right braces, so without this check the pop fails.

[dkt01]

Updates recommended by owenpan in review.

[owenpan]

clang/lib/Format/TokenAnnotator.cpp

123

As suggested before.

1200–1203

Thanks!

[dkt01]

clang/lib/Format/TokenAnnotator.cpp

123

Oops... missed the consts last time...

[dkt01]

Changes requested in owenpan's review.
Rebased changes onto latest main branch.

clang/lib/Format/TokenAnnotator.h

enum LineType {
   LT_ObjCMethodDecl,
   LT_ObjCProperty, // An @property line.
   LT_Other,
   LT_PreprocessorDirective,
   LT_VirtualFunctionDecl,
   LT_ArrayOfStructInitializer,
   LT_CommentAbovePPDirective,
 };
 
+enum ScopeType {
+  // Contained in class declaration/definition.
+  ST_Class,
+  // Contained within function definition.
+  ST_Function,
+  // Contained within other scope block (loop, if/else, etc).
+  ST_Other,
+};
+
 class AnnotatedLine {
 public:
   AnnotatedLine(const UnwrappedLine &Line)
       : First(Line.Tokens.front().Tok), Level(Line.Level),
         PPLevel(Line.PPLevel),
         MatchingOpeningBlockLineIndex(Line.MatchingOpeningBlockLineIndex),
         MatchingClosingBlockLineIndex(Line.MatchingClosingBlockLineIndex),
         InPPDirective(Line.InPPDirective),
public:
   TokenAnnotator(const FormatStyle &Style, const AdditionalKeywords &Keywords)
       : Style(Style), Keywords(Keywords) {}
 
   /// Adapts the indent levels of comment lines to the indent of the
   /// subsequent line.
   // FIXME: Can/should this be done in the UnwrappedLineParser?
   void setCommentLineLevels(SmallVectorImpl<AnnotatedLine *> &Lines) const;
 
-  void annotate(AnnotatedLine &Line) const;
+  void annotate(AnnotatedLine &Line);
   void calculateFormattingInformation(AnnotatedLine &Line) const;
 
 private:
   /// Calculate the penalty for splitting before \c Tok.
   unsigned splitPenalty(const AnnotatedLine &Line, const FormatToken &Tok,
                         bool InFunctionDecl) const;
 
   bool spaceRequiredBeforeParens(const FormatToken &Right) const;
 
   bool spaceRequiredBetween(const AnnotatedLine &Line, const FormatToken &Left,
                             const FormatToken &Right) const;
 
   bool spaceRequiredBefore(const AnnotatedLine &Line,
                            const FormatToken &Right) const;
 
   bool mustBreakBefore(const AnnotatedLine &Line,
                        const FormatToken &Right) const;
 
private:
   getTokenReferenceAlignment(const FormatToken &PointerOrReference) const;
 
   FormatStyle::PointerAlignmentStyle getTokenPointerOrReferenceAlignment(
       const FormatToken &PointerOrReference) const;
 
   const FormatStyle &Style;
 
   const AdditionalKeywords &Keywords;
+
+  SmallVector<ScopeType> Scopes;
 };
 
 } // end namespace format
 } // end namespace clang
 
 #endif

clang/lib/Format/TokenAnnotator.cpp

 /// A parser that gathers additional information about tokens.
 ///
 /// The \c TokenAnnotator tries to match parenthesis and square brakets and
 /// store a parenthesis levels. It also tries to resolve matching "<" and ">"
 /// into template parameter lists.
 class AnnotatingParser {
 public:
   AnnotatingParser(const FormatStyle &Style, AnnotatedLine &Line,
-                   const AdditionalKeywords &Keywords)
+                   const AdditionalKeywords &Keywords,
+                   SmallVector<ScopeType> &Scopes)
       : Style(Style), Line(Line), CurrentToken(Line.First), AutoFound(false),
-        Keywords(Keywords) {
+        Keywords(Keywords), Scopes(Scopes) {
     Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false));
     resetTokenMetadata();
   }
 
 private:
+  ScopeType getScopeType(const FormatToken &Token) const {
+    switch (Token.getType()) {
+    case TT_FunctionLBrace:
+    case TT_LambdaLBrace:
+      return ST_Function;
+    case TT_ClassLBrace:
+    case TT_StructLBrace:
+    case TT_UnionLBrace:
+      return ST_Class;
+    default:
+      return ST_Other;
+    }
+  }
+
   bool parseAngle() {
     if (!CurrentToken || !CurrentToken->Previous)
       return false;
     if (NonTemplateLess.count(CurrentToken->Previous))
       return false;
 
     const FormatToken &Previous = *CurrentToken->Previous; // The '<'.
     if (Previous.Previous) {
bool parseBrace() {
     if (Style.isJavaScript() && OpeningBrace.Previous &&
         OpeningBrace.Previous->is(TT_JsTypeColon)) {
       Contexts.back().IsExpression = false;
     }
 
     unsigned CommaCount = 0;
     while (CurrentToken) {
       if (CurrentToken->is(tok::r_brace)) {
+        assert(!Scopes.empty());
+        assert(Scopes.back() == getScopeType(OpeningBrace));
+        Scopes.pop_back();
         assert(OpeningBrace.Optional == CurrentToken->Optional);
         OpeningBrace.MatchingParen = CurrentToken;
         CurrentToken->MatchingParen = &OpeningBrace;
         if (Style.AlignArrayOfStructures != FormatStyle::AIAS_None) {
           if (OpeningBrace.ParentBracket == tok::l_brace &&
               couldBeInStructArrayInitializer() && CommaCount > 0) {
             Contexts.back().ContextType = Context::StructArrayInitializer;
           }
case tok::l_square:
         return false;
       break;
     case tok::l_brace:
       if (Style.Language == FormatStyle::LK_TextProto) {
         FormatToken *Previous = Tok->getPreviousNonComment();
         if (Previous && Previous->getType() != TT_DictLiteral)
           Previous->setType(TT_SelectorName);
       }
+      Scopes.push_back(getScopeType(*Tok));
       if (!parseBrace())
         return false;
       break;
     case tok::less:
       if (parseAngle()) {
         Tok->setType(TT_TemplateOpener);
         // In TT_Proto, we must distignuish between:
         //   map<key, value>
         //   msg < item: data >
         //   msg: < item: data >
         // In TT_TextProto, map<key, value> does not occur.
         if (Style.Language == FormatStyle::LK_TextProto ||
             (Style.Language == FormatStyle::LK_Proto && Tok->Previous &&
              Tok->Previous->isOneOf(TT_SelectorName, TT_DictLiteral))) {
           Tok->setType(TT_DictLiteral);
           FormatToken *Previous = Tok->getPreviousNonComment();
           if (Previous && Previous->getType() != TT_DictLiteral)
             Previous->setType(TT_SelectorName);
         }
       } else {
         Tok->setType(TT_BinaryOperator);
         NonTemplateLess.insert(Tok);
         CurrentToken = Tok;
         next();
       }
       break;
     case tok::r_paren:
     case tok::r_square:
       return false;
     case tok::r_brace:
+      // Don't pop scope when encountering unbalanced r_brace.
+      if (!Scopes.empty())
+        Scopes.pop_back();
       // Lines can start with '}'.
       if (Tok->Previous)
         return false;
       break;
     case tok::greater:
       if (Style.Language != FormatStyle::LK_TextProto)
         Tok->setType(TT_BinaryOperator);
       if (Tok->Previous && Tok->Previous->is(TT_TemplateCloser))
         Tok->SpacesRequiredBefore = 1;
enum {
       StructArrayInitializer,
       // Like in `static_cast<int>`.
       TemplateArgument,
       // C11 _Generic selection.
       C11GenericSelection,
     } ContextType = Unknown;
   };
 
   /// Puts a new \c Context onto the stack \c Contexts for the lifetime
   /// of each instance.
   struct ScopedContextCreator {
     AnnotatingParser &P;
 
     ScopedContextCreator(AnnotatingParser &P, tok::TokenKind ContextKind,
                          unsigned Increase)
         : P(P) {
       P.Contexts.push_back(Context(ContextKind,
                                    P.Contexts.back().BindingStrength + Increase,
if (Current.is(Keywords.kw_instanceof)) {
       // Concept implicit conversion constraint needs to be treated like
       // a trailing return type  ... } -> <type>.
       Current.setType(TT_TrailingReturnArrow);
     } else if (isDeductionGuide(Current)) {
       // Deduction guides trailing arrow " A(...) -> A<T>;".
       Current.setType(TT_TrailingReturnArrow);
     } else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) {
       Current.setType(determineStarAmpUsage(
           Current,
           Contexts.back().CanBeExpression && Contexts.back().IsExpression,
           Contexts.back().ContextType == Context::TemplateArgument));
     } else if (Current.isOneOf(tok::minus, tok::plus, tok::caret) ||
                (Style.isVerilog() && Current.is(tok::pipe))) {
       Current.setType(determinePlusMinusCaretUsage(Current));
       if (Current.is(TT_UnaryOperator) && Current.is(tok::caret))
         Contexts.back().CaretFound = true;
     } else if (Current.isOneOf(tok::minusminus, tok::plusplus)) {
bool determineUnaryOperatorByUsage(const FormatToken &Tok) {
     if (PrevToken->is(TT_BinaryOperator))
       return true;
 
     return false;
   }
 
   /// Return the type of the given token assuming it is * or &.
   TokenType determineStarAmpUsage(const FormatToken &Tok, bool IsExpression,
                                   bool InTemplateArgument) {
     if (Style.isJavaScript())
       return TT_BinaryOperator;
 
     // && in C# must be a binary operator.
     if (Style.isCSharp() && Tok.is(tok::ampamp))
       return TT_BinaryOperator;
 
     const FormatToken *PrevToken = Tok.getPreviousNonComment();
if (NextToken->Tok.isAnyIdentifier()) {
         return TT_BinaryOperator;
     }
 
     // It is very unlikely that we are going to find a pointer or reference type
     // definition on the RHS of an assignment.
     if (IsExpression && !Contexts.back().CaretFound)
       return TT_BinaryOperator;
 
+    // Opeartors at class scope are likely pointer or reference members.
+    if (!Scopes.empty() && Scopes.back() == ST_Class)
+      return TT_PointerOrReference;
+
+    // Tokens that indicate member access or chained operator& use.
+    auto IsChainedOperatorAmpOrMember = [](const FormatToken *token) {
+      return !token || token->isOneOf(tok::amp, tok::period, tok::arrow,
+                                      tok::arrowstar, tok::periodstar);
+    };
+
+    // It's more likely that & represents operator& than an uninitialized
+    // reference.
+    if (Tok.is(tok::amp) && PrevToken && PrevToken->Tok.isAnyIdentifier() &&
+        IsChainedOperatorAmpOrMember(PrevToken->getPreviousNonComment()) &&
+        NextToken && NextToken->Tok.isAnyIdentifier()) {
+      if (auto NextNext = NextToken->getNextNonComment();
+          NextNext &&
+          (IsChainedOperatorAmpOrMember(NextNext) || NextNext->is(tok::semi))) {
+        return TT_BinaryOperator;
+      }
+    }
+
     return TT_PointerOrReference;
   }
 
   TokenType determinePlusMinusCaretUsage(const FormatToken &Tok) {
     if (determineUnaryOperatorByUsage(Tok))
       return TT_UnaryOperator;
 
     const FormatToken *PrevToken = Tok.getPreviousNonComment();
if (!PrevToken || PrevToken->is(TT_CastRParen))
       return TT_UnaryOperator;
     if (PrevToken->isOneOf(tok::r_paren, tok::r_square, tok::identifier))
       return TT_TrailingUnaryOperator;
 
     return TT_UnaryOperator;
   }
 
   SmallVector<Context, 8> Contexts;
 
   const FormatStyle &Style;
   AnnotatedLine &Line;
   FormatToken *CurrentToken;
   bool AutoFound;
   const AdditionalKeywords &Keywords;
 
+  SmallVector<ScopeType> &Scopes;
+
   // Set of "<" tokens that do not open a template parameter list. If parseAngle
   // determines that a specific token can't be a template opener, it will make
   // same decision irrespective of the decisions for tokens leading up to it.
   // Store this information to prevent this from causing exponential runtime.
   llvm::SmallPtrSet<FormatToken *, 16> NonTemplateLess;
 };
 
 static const int PrecedenceUnaryOperator = prec::PointerToMember + 1;
private:
   FormatToken *Current;
 };
 
 } // end anonymous namespace
 
 void TokenAnnotator::setCommentLineLevels(
     SmallVectorImpl<AnnotatedLine *> &Lines) const {
   const AnnotatedLine *NextNonCommentLine = nullptr;
   for (AnnotatedLine *Line : llvm::reverse(Lines)) {
     assert(Line->First);
 
     // If the comment is currently aligned with the line immediately following
     // it, that's probably intentional and we should keep it.
     if (NextNonCommentLine && Line->isComment() &&
         NextNonCommentLine->First->NewlinesBefore <= 1 &&
         NextNonCommentLine->First->OriginalColumn ==
             Line->First->OriginalColumn) {

 
 static unsigned maxNestingDepth(const AnnotatedLine &Line) {
   unsigned Result = 0;
   for (const auto *Tok = Line.First; Tok != nullptr; Tok = Tok->Next)
     Result = std::max(Result, Tok->NestingLevel);
   return Result;
 }
 
-void TokenAnnotator::annotate(AnnotatedLine &Line) const {
+void TokenAnnotator::annotate(AnnotatedLine &Line) {
   for (auto &Child : Line.Children)
     annotate(*Child);
 
-  AnnotatingParser Parser(Style, Line, Keywords);
+  AnnotatingParser Parser(Style, Line, Keywords, Scopes);
   Line.Type = Parser.parseLine();
 
   // With very deep nesting, ExpressionParser uses lots of stack and the
   // formatting algorithm is very slow. We're not going to do a good job here
   // anyway - it's probably generated code being formatted by mistake.
   // Just skip the whole line.
   if (maxNestingDepth(Line) > 50)
     Line.Type = LT_Invalid;
static bool isFunctionDeclarationName(bool IsCpp, const FormatToken &Current,
   // Check whether parameter list can belong to a function declaration.
   if (!Next || !Next->is(tok::l_paren) || !Next->MatchingParen)
     return false;
   // If the lines ends with "{", this is likely a function definition.
   if (Line.Last->is(tok::l_brace))
     return true;
   if (Next->Next == Next->MatchingParen)
     return true; // Empty parentheses.
   // If there is an &/&& after the r_paren, this is likely a function.
   if (Next->MatchingParen->Next &&
       Next->MatchingParen->Next->is(TT_PointerOrReference)) {
     return true;
   }
 
   // Check for K&R C function definitions (and C++ function definitions with
   // unnamed parameters), e.g.:
   //   int f(i)

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
   // operator invoked as a member function).
   verifyFormat("void f() { a.operator()(a * a); }");
   verifyFormat("void f() { a->operator()(a & a); }");
   verifyFormat("void f() { a.operator()(*a & *a); }");
   verifyFormat("void f() { a->operator()(*a * *a); }");
 
   verifyFormat("int operator()(T (&&)[N]) { return 1; }");
   verifyFormat("int operator()(T (&)[N]) { return 0; }");
+
+  verifyFormat("val1 & val2;");
+  verifyFormat("val1 & val2 & val3;");
+  verifyFormat("class c {\n"
+               "  void func(type &a) { a & member; }\n"
+               "  anotherType &member;\n"
+               "}");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
   verifyFormat("SomeType s __attribute__((unused)) (InitValue);");
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa __attribute__((unused))\n"
                "aaaaaaaaaaaaaaaaaaaaaaa(int i);");
   verifyFormat("__attribute__((nodebug)) ::qualified_type f();");
   FormatStyle AfterType = getLLVMStyle();

clang/unittests/Format/TokenAnnotatorTest.cpp

TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmp) {
   ASSERT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::amp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[12], tok::amp, TT_PointerOrReference);
 
   Tokens = annotate("void f7() { int f8(Foo&&, Bar&&); }");
   ASSERT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1 &val1 = val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1 *val1 = &val2;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::star, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_UnaryOperator);
+
+  Tokens = annotate("val1 & val2;");
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.*member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1.*member & val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2->*member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1->member & val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 & val3;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 // comment\n"
+                    "     & val3;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_BinaryOperator);
+
+  Tokens =
+      annotate("val1 & val2.member & val3.member() & val4 & val5->member;");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[5], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[13], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("class c {\n"
+                    "  void func(type &a) { a & member; }\n"
+                    "  anotherType &member;\n"
+                    "}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[12], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[17], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("struct S {\n"
+                    "  auto Mem = C & D;\n"
+                    "}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
   auto Tokens = annotate("x - 0");
   ASSERT_EQ(Tokens.size(), 4u) << Tokens;
   EXPECT_TOKEN(Tokens[1], tok::minus, TT_BinaryOperator);
   Tokens = annotate("0 + 0");
   ASSERT_EQ(Tokens.size(), 4u) << Tokens;