[clang] adds warning to alert user when they use alternative tokens in declarations

[cjdb]

Summary

Although the alternative tokens can be used in place of symbols outside
of bitwise/logical expressions, this is arguably feature misuse in
contemporary programming (e.g. int bitand does not communicate that
the type is a reference-to-int). As of this commit, Clang will warn—by
default—when a programmer attempts to declare something using
alternative tokens in place of symbols.

[cjdb]

clang/test/Parser/warn-declare-references-with-symbols.cpp

3

I'm pretty confident there's a lit way for me to test specific lines with specific C++ modes, but I can't find it :(

[aaron.ballman]

clang/include/clang/Basic/DiagnosticParseKinds.td

1552

Pretty sure you don't need DefaultWarn here (I think you'd use that for warnings that are off by default, like ones spelled with Extension instead of Warning or ExtWarn).

clang/lib/Parse/ParseDecl.cpp

5862–5864

For consistency with the Lvalue case above.

clang/test/Parser/warn-declare-references-with-symbols.cpp

3

Are you thinking of -verify=prefix and // prefix-warning {{whatever}} combinations?

[cjdb]

clang/lib/Parse/ParseDecl.cpp

5862–5864

I originally had Rvalue, but this is technically diagnosing both rvalue refs and forwarding refs (hence the awkward name). I guess it doesn't really matter at the parsing stage though, so this is just commentary.

clang/test/Parser/warn-declare-references-with-symbols.cpp

3

I thought it was possible to do have a RUN line where C++98 mode would only check lines 4-17, for example.

[aaron.ballman]

clang/lib/Parse/ParseDecl.cpp

5862–5864

Oh, good point on forwarding refs. Maybe change the previous one to Amp in that case?

clang/test/Parser/warn-declare-references-with-symbols.cpp

3

Ah, I usually use #if __cplusplus < NNNNNN in the code for that sort of thing.

[cjdb]

applies feedback

clang/test/Parser/warn-declare-references-with-symbols.cpp

3

Thanks!

[dblaikie]

Not a huge objection - but minor quandry: What's the motivation for this patch? I don't know of any codebase that encourages/uses the alternative tokens and I wonder if adding more usability to them is a worthwhile investment in clang's codebase complexity, etc.

[cjdb]

In D107292#2920637, @dblaikie wrote:

Not a huge objection - but minor quandry: What's the motivation for this patch? I don't know of any codebase that encourages/uses the alternative tokens and I wonder if adding more usability to them is a worthwhile investment in clang's codebase complexity, etc.

There are codebases that use them (all of my non-Google, non-LLVM code does, for example, and I'm not the sole user: just a loud one who's also in a position to patch tooling). The motivation for this warning is essentially to catch someone who misunderstands/misreads the diagnostic of what's currently in D107294.

[dblaikie]

In D107292#2920746, @cjdb wrote:

In D107292#2920637, @dblaikie wrote:

Not a huge objection - but minor quandry: What's the motivation for this patch? I don't know of any codebase that encourages/uses the alternative tokens and I wonder if adding more usability to them is a worthwhile investment in clang's codebase complexity, etc.

There are codebases that use them (all of my non-Google, non-LLVM code does, for example, and I'm not the sole user: just a loud one who's also in a position to patch tooling).

Ah, any pointers to large open source projects that use this?

The motivation for this warning is essentially to catch someone who misunderstands/misreads the diagnostic of what's currently in D107294.

Ah, OK - I see some of the usual objections are being voiced over there so I'll leave it over there/subscribe there to follow that discussion. Thanks!

[cjdb]

In D107292#2920774, @dblaikie wrote:

In D107292#2920746, @cjdb wrote:

In D107292#2920637, @dblaikie wrote:

Not a huge objection - but minor quandry: What's the motivation for this patch? I don't know of any codebase that encourages/uses the alternative tokens and I wonder if adding more usability to them is a worthwhile investment in clang's codebase complexity, etc.

There are codebases that use them (all of my non-Google, non-LLVM code does, for example, and I'm not the sole user: just a loud one who's also in a position to patch tooling).

Ah, any pointers to large open source projects that use this?

Large OSS? No, sorry. range-v3 used to use them, but it was having issues integrating with Folly (I think) and @eric_niebler had to abandon using them.

[• Quuxplusone]

@dblaikie wrote:

Not a huge objection - but minor quandry: What's the motivation for this patch?

AIUI, this diagnostic is a PR stunt, similar to Clang's existing -Wunicode-homoglyph:

test.cpp:3:13: warning: treating Unicode character <U+037E> as identifier character rather than as ';' symbol [-Wunicode-homoglyph]
    return 0;
            ^

It's not that anyone would ever intentionally write code like void print(const std::string bitand s); it's that if someone copy-pastes such code from Reddit into Clang, Clang will helpfully explain the trick to them. I'm ambivalent as to whether this is worth it; but the patch is certainly tiny enough that it's not obviously not worth it...

clang/include/clang/Basic/DiagnosticParseKinds.td

1552

I suggest:
"use '%select{&|&&}0' when declaring an %select{lvalue|rvalue or forwarding}0 reference"
Besides the minor grammar tweak, this should avoid the need to pass the same argument twice (as both argument 0 and argument 1). It's possible that this ends up not working for some technical reason, but I'd be surprised if it didn't Just Work.

clang/lib/Parse/ParseDecl.cpp

5862–5864

In both cases, I'd write

Diag(Loc, diag::warn_declare_references_with_symbols)
            << (Kind == tok::ampamp);

or

<< static_cast<int>(Kind == tok::ampamp);

only if absolutely necessary (e.g. because the former doesn't build, or if it runtime-errors).

[aaron.ballman]

In D107292#2920637, @dblaikie wrote:

Not a huge objection - but minor quandry: What's the motivation for this patch? I don't know of any codebase that encourages/uses the alternative tokens and I wonder if adding more usability to them is a worthwhile investment in clang's codebase complexity, etc.

To me, the motivation comes from a signature like: void foo(int and); because it looks reasonable until you realize in C++ it means void foo(int &&); rather than declaring a parameter name of type int. What's worse, it can come up naturally in header files because it's a valid declaration in C if iso646.h is not included. So it's plausible to write the signature in a library exposing a C interface that changes the function signature when compiled in C++ -- this warning helps alert the programmer to such a situation.

I'm not aware of any popular coding guidelines that suggest using alternative tokens, but their use does show up in the wild.

[aaron.ballman]

In D107292#2920774, @dblaikie wrote:

In D107292#2920746, @cjdb wrote:

In D107292#2920637, @dblaikie wrote:

Not a huge objection - but minor quandry: What's the motivation for this patch? I don't know of any codebase that encourages/uses the alternative tokens and I wonder if adding more usability to them is a worthwhile investment in clang's codebase complexity, etc.

There are codebases that use them (all of my non-Google, non-LLVM code does, for example, and I'm not the sole user: just a loud one who's also in a position to patch tooling).

Ah, any pointers to large open source projects that use this?

https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=bitand&search=Search

(Searching for 'and' is a bit less useful because of how much it shows up in assembly, comments, etc.)

[dblaikie]

In D107292#2921901, @aaron.ballman wrote:

In D107292#2920774, @dblaikie wrote:

In D107292#2920746, @cjdb wrote:

In D107292#2920637, @dblaikie wrote:

Not a huge objection - but minor quandry: What's the motivation for this patch? I don't know of any codebase that encourages/uses the alternative tokens and I wonder if adding more usability to them is a worthwhile investment in clang's codebase complexity, etc.

There are codebases that use them (all of my non-Google, non-LLVM code does, for example, and I'm not the sole user: just a loud one who's also in a position to patch tooling).

Ah, any pointers to large open source projects that use this?

https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=bitand&search=Search

(Searching for 'and' is a bit less useful because of how much it shows up in assembly, comments, etc.)

Ah, cool. Only case I could find there (that wasn't C code or compiler test cases) was something called FuzzyLite (which looks like it hasn't been touched in 4 years or so).

I don't fundamentally object to this now it's being proposed as a clang-tidy thing - bar should be low/easy for experiments, getting user experience, adoption, etc.

[aaron.ballman]

In D107292#2922575, @dblaikie wrote:

In D107292#2921901, @aaron.ballman wrote:

In D107292#2920774, @dblaikie wrote:

In D107292#2920746, @cjdb wrote:

In D107292#2920637, @dblaikie wrote:

Not a huge objection - but minor quandry: What's the motivation for this patch? I don't know of any codebase that encourages/uses the alternative tokens and I wonder if adding more usability to them is a worthwhile investment in clang's codebase complexity, etc.

There are codebases that use them (all of my non-Google, non-LLVM code does, for example, and I'm not the sole user: just a loud one who's also in a position to patch tooling).

Ah, any pointers to large open source projects that use this?

https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=bitand&search=Search

(Searching for 'and' is a bit less useful because of how much it shows up in assembly, comments, etc.)

Ah, cool. Only case I could find there (that wasn't C code or compiler test cases) was something called FuzzyLite (which looks like it hasn't been touched in 4 years or so).

I suspect that bitand is used less than and because I believe bitwise operations are less common than logical ones.

I don't fundamentally object to this now it's being proposed as a clang-tidy thing - bar should be low/easy for experiments, getting user experience, adoption, etc.

FWIW, my feeling is that *this* proposed patch is reasonable for Clang but the proposed patch for suggesting use of alternative tokens (or not) belongs in clang-tidy. This patch proposes a diagnostic that catches bugs and can be on by default without false positives, the other patch proposed diagnostics that were more about coding style.

[dblaikie]

In D107292#2922599, @aaron.ballman wrote:

In D107292#2922575, @dblaikie wrote:

In D107292#2921901, @aaron.ballman wrote:

In D107292#2920774, @dblaikie wrote:

In D107292#2920746, @cjdb wrote:

In D107292#2920637, @dblaikie wrote:

Not a huge objection - but minor quandry: What's the motivation for this patch? I don't know of any codebase that encourages/uses the alternative tokens and I wonder if adding more usability to them is a worthwhile investment in clang's codebase complexity, etc.

There are codebases that use them (all of my non-Google, non-LLVM code does, for example, and I'm not the sole user: just a loud one who's also in a position to patch tooling).

Ah, any pointers to large open source projects that use this?

https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=bitand&search=Search

(Searching for 'and' is a bit less useful because of how much it shows up in assembly, comments, etc.)

Ah, cool. Only case I could find there (that wasn't C code or compiler test cases) was something called FuzzyLite (which looks like it hasn't been touched in 4 years or so).

I suspect that bitand is used less than and because I believe bitwise operations are less common than logical ones.

I don't fundamentally object to this now it's being proposed as a clang-tidy thing - bar should be low/easy for experiments, getting user experience, adoption, etc.

FWIW, my feeling is that *this* proposed patch is reasonable for Clang but the proposed patch for suggesting use of alternative tokens (or not) belongs in clang-tidy. This patch proposes a diagnostic that catches bugs and can be on by default without false positives, the other patch proposed diagnostics that were more about coding style.

Oh, sorry, got the two patches jumbled up. Right right.

Any data on a large codebase about whether this finds enough bugs* to be interesting? But yeah, I don't mind it terribly - seems low cost enough that I won't stand in the way of it if you're in favor.

• for some value of bug - that's been a point of debate for a while, turning on warnings like parentheses by default is an example. If we have this on by default, for codebases where it fires at all, are we creating busywork for people to go and cleanup code that's weird, but wasn't actively harmful/likely to be causing real problems for them? If we are, then that feels like it's more a stylistic thing than a bug-finding warning.

[cjdb]

<truncating previous conversation>

Yes, the intention is for this patch to remain a Clang warning since it's cheap and can be on by default.

[rsmith]

We should also warn on:

• and or bitand used as a ref-qualifier in a member function declaration
• xor used to declare a block pointer type or block pointer literal
• bitand used as address-of and and used to form a GNU address-of-label
• compl used to name a destructor

Of these, I think it's somewhat important to handle ref-qualifiers and probably block pointer types at around the same time that this patch lands (though it doesn't need to be part of this patch) for consistency among "type operators".

clang/include/clang/Basic/DiagnosticParseKinds.td

1551

Drop the "and forwarding" part; a forwarding reference is a kind of rvalue reference, and distinguishing them here will lead to confusion.

[cjdb]

In D107292#2923003, @rsmith wrote:

We should also warn on:

• and or bitand used as a ref-qualifier in a member function declaration
• xor used to declare a block pointer type or block pointer literal
• bitand used as address-of and and used to form a GNU address-of-label
• compl used to name a destructor

Of these, I think it's somewhat important to handle ref-qualifiers and probably block pointer types at around the same time that this patch lands (though it doesn't need to be part of this patch) for consistency among "type operators".

I'm happy to work on these patches too. I think this patch should concern itself with declarators, and a second patch should be for expressions. This does drastically expand the scope of the warning, so we should probably come up with a new name for it.

Edit:

To make it clearer, I think the breakdown of the two patches I have in mind reads as

# Patch 1: declarators

T bitand
T and
T xor
void C::f() bitand
void C::f() and
C::compl C

# Patch 2: expressions

xor {}
bitand x // warning will suggest std::addressof in C++ land
and label

[cjdb]

extends functionality to account for destructors, member functions, and block pointers

[cjdb]

Ping @aaron.ballman

[aaron.ballman]

In D107292#2923261, @cjdb wrote:

Patch 2: expressions

xor {}
bitand x // warning will suggest std::addressof in C++ land
and label

An additional expression to cover, not that I think anyone would be this awful by accident, is: foo->compl Foo(); // Pseudo-destructor call

One question I have about both declarations and expressions are whether we have an appetite to diagnose overloaded operators or not. Personally, I think it'd be reasonable to diagnose something like foo->operator bitand(); or operator not_eq(A, B); as expressions, but not reasonable to diagnose the declaration of the overloaded operators using alternative tokens.

clang/lib/Parse/ParseDecl.cpp

5830–5832

We typically either use comments for one-off uses, or we hoist this sort of thing to an enumeration associated with the diagnostic if we think the diagnostic will be used in enough places to warrant it. I think we should probably stick to comments for this one (e.g., Diag(Loc, diag::whatever) << /*frobble*/1;, but if you think a hoisted enum is a better approach I won't argue it.

(Same comment applies elsewhere in the patch.)

[cjdb]

In D107292#2939521, @aaron.ballman wrote:

In D107292#2923261, @cjdb wrote:

Patch 2: expressions

xor {}
bitand x // warning will suggest std::addressof in C++ land
and label

An additional expression to cover, not that I think anyone would be this awful by accident, is: foo->compl Foo(); // Pseudo-destructor call

Nice catch!

One question I have about both declarations and expressions are whether we have an appetite to diagnose overloaded operators or not. Personally, I think it'd be reasonable to diagnose something like foo->operator bitand(); or operator not_eq(A, B); as expressions, but not reasonable to diagnose the declaration of the overloaded operators using alternative tokens.

I agree that bool operator and(T, T); shouldn't be diagnosed on (and this patch's clang-tidy sibling will one day also diagnose that, but it's way off).

I think that foo->operator bitand() and operator not_eq(expr1, expr2) should only diagnose if foo->operator&() and operator!=(expr1, expr2) are diagnosed, and I think that should be a separate warning (I'm not saying that's a good or bad thing to do yet: let me sleep on that). I might be misunderstanding your intention though.

[aaron.ballman]

In D107292#2943101, @cjdb wrote:

In D107292#2939521, @aaron.ballman wrote:

One question I have about both declarations and expressions are whether we have an appetite to diagnose overloaded operators or not. Personally, I think it'd be reasonable to diagnose something like foo->operator bitand(); or operator not_eq(A, B); as expressions, but not reasonable to diagnose the declaration of the overloaded operators using alternative tokens.

I agree that bool operator and(T, T); shouldn't be diagnosed on (and this patch's clang-tidy sibling will one day also diagnose that, but it's way off).

I think that foo->operator bitand() and operator not_eq(expr1, expr2) should only diagnose if foo->operator&() and operator!=(expr1, expr2) are diagnosed, and I think that should be a separate warning (I'm not saying that's a good or bad thing to do yet: let me sleep on that). I might be misunderstanding your intention though.

The situation I was thinking of was where the the declaration is for an operator&() function but the expression is calling operator bitand() (or vice versa), but 1) I don't feel strongly it needs to be diagnosed, mostly just exploring the shape of the diagnostic, and 2) I'd be fine if it was handled under a separate flag at some later date.

[• Quuxplusone]

clang/include/clang/Basic/DiagnosticParseKinds.td

1553

@cjdb: I suggest splitting this diagnostic up, mainly for sanity's sake (it currently has 4x2x4 = 32 possible outputs, which is Too Much) but also to improve greppability (it took me five tries to find the source of the message "when declaring destructors" in this PR). I would do one diagnostic "use '%{&|&&}0' when declaring %{lvalue|rvalue}1 %{references|ref-qualified member functions}"; another diagnostic "use '~' when declaring destructors"; and a third "use '^' when declaring block pointers".

@aaron.ballman wrote:

One question I have about both declarations and expressions are whether we have an appetite to diagnose overloaded operators or not. Personally, I think it'd be reasonable to diagnose something like foo->operator bitand(); or operator not_eq(A, B); as expressions, but not reasonable to diagnose the declaration of the overloaded operators using alternative tokens.

AIUI, @cjdb is deferring all diagnostics related to expressions into a later PR, and I think that's reasonable. There's a subtlety to your example I can't tell if you intended: foo->operator bitand() is clearly wrong because the unary operator & being invoked there is not bitand; it's address-of! foo->operator bitand() will presumably fall into the same category as foo->compl Foo(). Whereas there's nothing so wrong with foo->operator not_eq(bar) or operator not_eq(foo, bar).

However, all that has at least two adjacent issues that I know are on @cjdb's radar: (1) auto transformed = foo bitor std::views::transform(bar) is "wrong", and in fact (2) any use of alternative tokens other than and, or, not is unstylish. If you diagnose all usage of bitand, bitor, compl, etc., then you don't get any of these subtle problems, because &&/||/! only have one meaning each (except for rvalue references, which is handled in this PR; and GCC-extension &&label, which should be easy to catch).

[cjdb]

I think there was a clang-tidy patch that handled this.

clang/include/clang/Basic/DiagnosticParseKinds.td

def warn_max_tokens : Warning<
   InGroup<MaxTokens>, DefaultIgnore;
 
 def warn_max_tokens_total : Warning<
   "the total number of preprocessor source tokens (%0) exceeds the token limit (%1)">,
   InGroup<MaxTokens>, DefaultIgnore;
 
 def note_max_tokens_total_override : Note<"total token limit set here">;
 
+def warn_declare_with_symbols : Warning<
+  "use '%select{&|&&|~|^}0' when declaring%select{ lvalue| rvalue|}1 "
+  "%select{references|destructors|block pointers|ref-qualified member functions}2">,
+  InGroup<DiagGroup<"declare-with-symbols">>;
+
 } // end of Parser diagnostics

clang/lib/Parse/ParseDecl.cpp

 //===--- ParseDecl.cpp - Declaration Parsing --------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file implements the Declaration portions of the Parser interfaces.
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/Parse/Parser.h"
-#include "clang/Parse/RAIIObjectsForParser.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/PrettyDeclStackTrace.h"
 #include "clang/Basic/AddressSpaces.h"
 #include "clang/Basic/Attributes.h"
 #include "clang/Basic/CharInfo.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Parse/ParseDiagnostic.h"
+#include "clang/Parse/Parser.h"
+#include "clang/Parse/RAIIObjectsForParser.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
if (!isPtrOperatorToken(Kind, getLangOpts(), D.getContext())) {
     return;
   }
 
   // Otherwise, '*' -> pointer, '^' -> block, '&' -> lvalue reference,
   // '&&' -> rvalue reference
   SourceLocation Loc = ConsumeToken();  // Eat the *, ^, & or &&.
   D.SetRangeEnd(Loc);
 
+  const SourceManager &SM = PP.getSourceManager();
+
   if (Kind == tok::star || Kind == tok::caret) {
     // Is a pointer.
     DeclSpec DS(AttrFactory);
 
     // GNU attributes are not allowed here in a new-type-id, but Declspec and
     // C++11 attributes are allowed.
     unsigned Reqs = AR_CXX11AttributesParsed | AR_DeclspecAttributesParsed |
                     ((D.getContext() != DeclaratorContext::CXXNew)
                          ? AR_GNUAttributesParsed
                          : AR_GNUAttributesParsedAndRejected);
     ParseTypeQualifierListOpt(DS, Reqs, true, !D.mayOmitIdentifier());
     D.ExtendWithDeclSpec(DS);
 
     // Recursively parse the declarator.
     ParseDeclaratorInternal(D, DirectDeclParser);
     if (Kind == tok::star)
       // Remember that we parsed a pointer type, and remember the type-quals.
       D.AddTypeInfo(DeclaratorChunk::getPointer(
                         DS.getTypeQualifiers(), Loc, DS.getConstSpecLoc(),
                         DS.getVolatileSpecLoc(), DS.getRestrictSpecLoc(),
                         DS.getAtomicSpecLoc(), DS.getUnalignedSpecLoc()),
                     std::move(DS.getAttributes()), SourceLocation());
-    else
+    else {
+      if (*SM.getCharacterData(Loc) != '^') {
+        constexpr int Caret = 3;
+        constexpr int Empty = 2;
+        constexpr int BlockPointers = 2;
+        Diag(Loc, diag::warn_declare_with_symbols)
+            << Caret << Empty << BlockPointers
+            << FixItHint::CreateReplacement(Loc, "^");
+      }
+
       // Remember that we parsed a Block type, and remember the type-quals.
       D.AddTypeInfo(
           DeclaratorChunk::getBlockPointer(DS.getTypeQualifiers(), Loc),
           std::move(DS.getAttributes()), SourceLocation());
+    }
   } else {
     // Is a reference
     DeclSpec DS(AttrFactory);
 
+    constexpr int References = 0;
+    if (Kind == tok::amp && *SM.getCharacterData(Loc) != '&') {
+      constexpr int Lvalue = 0;
+      Diag(Loc, diag::warn_declare_with_symbols)
+          << Lvalue << Lvalue << References
+          << FixItHint::CreateReplacement(Loc, "&");
+    }
+    if (Kind == tok::ampamp) {
       // Complain about rvalue references in C++03, but then go on and build
       // the declarator.
-    if (Kind == tok::ampamp)
       Diag(Loc, getLangOpts().CPlusPlus11 ?
            diag::warn_cxx98_compat_rvalue_reference :
            diag::ext_rvalue_reference);
 
+      if (*SM.getCharacterData(Loc) != '&') {
+        constexpr int Rvalue = 1;
+        Diag(Loc, diag::warn_declare_with_symbols)
+            << Rvalue << Rvalue << References
+            << FixItHint::CreateReplacement(Loc, "&&");
+      }
+    }
+
     // GNU-style and C++11 attributes are allowed here, as is restrict.
     ParseTypeQualifierListOpt(DS);
     D.ExtendWithDeclSpec(DS);
 
     // C++ 8.3.2p1: cv-qualified references are ill-formed except when the
     // cv-qualifiers are introduced through the use of a typedef or of a
     // template type argument, in which case the cv-qualifiers are ignored.
     if (DS.getTypeQualifiers() != DeclSpec::TQ_unspecified) {
bool Parser::ParseRefQualifier(bool &RefQualifierIsLValueRef,
                                SourceLocation &RefQualifierLoc) {
   if (Tok.isOneOf(tok::amp, tok::ampamp)) {
     Diag(Tok, getLangOpts().CPlusPlus11 ?
          diag::warn_cxx98_compat_ref_qualifier :
          diag::ext_ref_qualifier);
 
     RefQualifierIsLValueRef = Tok.is(tok::amp);
     RefQualifierLoc = ConsumeToken();
+
+    if (*PP.getSourceManager().getCharacterData(RefQualifierLoc) != '&') {
+      int Qualifier = RefQualifierIsLValueRef ? 0 : 1;
+      constexpr int RefQualifiedMemberFunc = 3;
+      Diag(Tok.getLocation(), diag::warn_declare_with_symbols)
+          << Qualifier << Qualifier << RefQualifiedMemberFunc
+          << FixItHint::CreateReplacement(Tok.getLocation(),
+                                          RefQualifierIsLValueRef ? "&" : "&&");
+    }
+
     return true;
   }
   return false;
 }
 
 /// isFunctionDeclaratorIdentifierList - This parameter list may have an
 /// identifier list form for a K&R-style function:  void foo(a,b,c)
 ///

clang/lib/Parse/ParseExprCXX.cpp

if (Tok.is(tok::identifier)) {
     CCLoc = ConsumeToken();
   } else {
     assert(SS.isEmpty() && "missing last component of nested name specifier");
     FirstTypeName.setIdentifier(nullptr, SourceLocation());
   }
 
   // Parse the tilde.
   assert(Tok.is(tok::tilde) && "ParseOptionalCXXScopeSpecifier fail");
+
+  if (*PP.getSourceManager().getCharacterData(Tok.getLocation()) != '~') {
+    constexpr int Tilde = 2;
+    constexpr int Empty = 2;
+    constexpr int Destructors = 1;
+    Diag(Tok.getLocation(), diag::warn_declare_with_symbols)
+        << Tilde << Empty << Destructors
+        << FixItHint::CreateReplacement(Tok.getLocation(), "~");
+  }
+
   SourceLocation TildeLoc = ConsumeToken();
 
   if (Tok.is(tok::kw_decltype) && !FirstTypeName.isValid()) {
     DeclSpec DS(AttrFactory);
     ParseDecltypeSpecifier(DS);
     if (DS.getTypeSpecType() == TST_error)
       return ExprError();
     return Actions.ActOnPseudoDestructorExpr(getCurScope(), Base, OpLoc, OpKind,
bool Parser::ParseUnqualifiedId(CXXScopeSpec &SS, ParsedType ObjectType,
   if (getLangOpts().CPlusPlus &&
       (AllowDestructorName || SS.isSet()) && Tok.is(tok::tilde)) {
     // C++ [expr.unary.op]p10:
     //   There is an ambiguity in the unary-expression ~X(), where X is a
     //   class-name. The ambiguity is resolved in favor of treating ~ as a
     //    unary complement rather than treating ~X as referring to a destructor.
 
     // Parse the '~'.
+
+    if (*PP.getSourceManager().getCharacterData(Tok.getLocation()) != '~') {
+      constexpr int Tilde = 2;
+      constexpr int Empty = 2;
+      constexpr int Destructors = 1;
+      Diag(Tok.getLocation(), diag::warn_declare_with_symbols)
+          << Tilde << Empty << Destructors
+          << FixItHint::CreateReplacement(Tok.getLocation(), "~");
+    }
+
     SourceLocation TildeLoc = ConsumeToken();
 
     if (TemplateSpecified) {
       // C++ [temp.names]p3:
       //   A name prefixed by the keyword template shall be a template-id [...]
       //
       // A template-id cannot begin with a '~' token. This would never work
       // anyway: x.~A<int>() would specify that the destructor is a template,

clang/test/Parser/cxx-decl.cpp

-// RUN: %clang_cc1 -verify -fsyntax-only -triple i386-linux -Wredundant-parens -pedantic-errors -fcxx-exceptions -fexceptions %s
-// RUN: %clang_cc1 -verify -fsyntax-only -triple i386-linux -Wredundant-parens -pedantic-errors -fcxx-exceptions -fexceptions -std=c++98 %s
-// RUN: %clang_cc1 -verify -fsyntax-only -triple i386-linux -Wredundant-parens -pedantic-errors -fcxx-exceptions -fexceptions -std=c++11 %s
+// RUN: %clang_cc1 -verify -fsyntax-only -triple i386-linux -Wredundant-parens -Wno-declare-with-symbols -pedantic-errors -fcxx-exceptions -fexceptions %s
+// RUN: %clang_cc1 -verify -fsyntax-only -triple i386-linux -Wredundant-parens -Wno-declare-with-symbols -pedantic-errors -fcxx-exceptions -fexceptions -std=c++98 %s
+// RUN: %clang_cc1 -verify -fsyntax-only -triple i386-linux -Wredundant-parens -Wno-declare-with-symbols -pedantic-errors -fcxx-exceptions -fexceptions -std=c++11 %s
 
 const char const *x10; // expected-error {{duplicate 'const' declaration specifier}}
 
 int x(*g); // expected-error {{use of undeclared identifier 'g'}}
 
 private int cplusplus_is_not_opencl; // expected-error {{expected unqualified-id}}
 
 struct Type {

clang/test/Parser/cxx0x-decl.cpp

-// RUN: %clang_cc1 -verify -fsyntax-only -std=c++2a -pedantic-errors -triple x86_64-linux-gnu %s
+// RUN: %clang_cc1 -verify -fsyntax-only -std=c++2a -Wno-declare-with-symbols -pedantic-errors -triple x86_64-linux-gnu %s
 
 // Make sure we know these are legitimate commas and not typos for ';'.
 namespace Commas {
   int a,
   b [[ ]],
   c alignas(double);
 }
 

clang/test/Parser/warn-declare-references-with-symbols.cpp

+// RUN: %clang_cc1 -std=c++98 -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++98 -fblocks -fsyntax-only -Wdeclare-with-symbols -verify %s
+// RUN: %clang_cc1 -std=c++11 -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fblocks -fsyntax-only -Wdeclare-with-symbols -verify %s
+// RUN: %clang_cc1 -std=c++14 -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++14 -fblocks -fsyntax-only -Wdeclare-with-symbols -verify %s
+// RUN: %clang_cc1 -std=c++20 -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++20 -fblocks -fsyntax-only -Wdeclare-with-symbols -verify %s
+
+int i = 0;
+
+int bitand lr1 = i; // expected-warning{{use '&' when declaring lvalue references}}
+int &lr2 = i; // no warning
+
+template<class T>
+void f(T bitand); // expected-warning{{use '&' when declaring lvalue references}}
+
+template<class T>
+void f(T &); // no warning
+
+int(xor bp1)(); // expected-warning{{use '^' when declaring block pointers}}
+int(^bp2)();
+
+struct S1 {
+  compl S1(); // expected-warning{{use '~' when declaring destructors}}
+};
+
+struct S2 {
+  ~S2();
+};
+
+S2::compl S2() // expected-warning{{use '~' when declaring destructors}}
+{}
+
+struct S3 {
+  ~S3(); // no warning
+};
+
+struct S4 {
+  ~S4();
+};
+
+S4::~S4() // no warning
+{}
+
+#if __cplusplus >= 201103L
+int and rr1 = 0; // expected-warning{{use '&&' when declaring rvalue references}}
+int &&rr2 = 0; // no warning
+
+using bad_block_ptr = int(xor)(); // expected-warning{{use '^' when declaring block pointers}}
+using good_block_ptr = int(^)();
+
+using bad_lr = int bitand; // expected-warning{{use '&' when declaring lvalue references}}
+using good_lr = int &; // no warning
+
+using bad_rr = int and; // expected-warning{{use '&&' when declaring rvalue references}}
+using good_rr = int &&; // no warning
+
+auto and fr1 = i; // expected-warning{{use '&&' when declaring rvalue references}}
+auto &&fr2 = i; // no warning
+
+auto and fr3 = 0; // expected-warning{{use '&&' when declaring rvalue references}}
+auto &&fr4 = 0; // no warning
+
+template<class T>
+void f(T and); // expected-warning{{use '&&' when declaring rvalue references}}
+
+template<class T>
+void f(T &&); // no warning
+
+struct S5 {
+  void f() bitand;
+  // expected-warning@-1{{use '&' when declaring lvalue ref-qualified member functions}}
+  void f() const bitand;
+  // expected-warning@-1{{use '&' when declaring lvalue ref-qualified member functions}}
+  void f() volatile bitand;
+  // expected-warning@-1{{use '&' when declaring lvalue ref-qualified member functions}}
+  void f() const volatile bitand;
+  // expected-warning@-1{{use '&' when declaring lvalue ref-qualified member functions}}
+  void f() and;
+  // expected-warning@-1{{use '&&' when declaring rvalue ref-qualified member functions}}
+  void f() const and;
+  // expected-warning@-1{{use '&&' when declaring rvalue ref-qualified member functions}}
+  void f() volatile and;
+  // expected-warning@-1{{use '&&' when declaring rvalue ref-qualified member functions}}
+  void f() const volatile and;
+  // expected-warning@-1{{use '&&' when declaring rvalue ref-qualified member functions}}
+};
+
+struct S6 {
+  void f() &; // no warning
+  void f() const &; // no warning
+  void f() volatile &; // no warning
+  void f() const volatile &; // no warning
+  void f() &&; // no warning
+  void f() const &&; // no warning
+  void f() volatile &&; // no warning
+  void f() const volatile &&; // no warning
+};
+#endif // __cplusplus > 201103L
+
+#if __cplusplus >= 201402L
+template<class T>
+T bitand lr3 = i; // expected-warning{{use '&' when declaring lvalue references}}
+
+template<class T>
+T &lr4 = i; // no warning
+
+template<class T>
+T and rr3 = i; // expected-warning{{use '&&' when declaring rvalue references}}
+
+template<class T>
+T &&rr4 = i; // no warning
+#endif // __cplusplus >= 201402L
+
+#if __cplusplus >= 202002L
+template<class T>
+concept C1 = requires(T bitand x) { x; };
+// expected-warning@-1{{use '&' when declaring lvalue references}}
+
+template<class T>
+concept C2 = requires(T &x) { x; }; // no warning
+
+template<class T>
+concept C3 = requires(T and x) { x; };
+// expected-warning@-1{{use '&&' when declaring rvalue references}}
+
+template<class T>
+concept C4 = requires(T &&x) { x; }; // no warning
+#endif // __cplusplus >= 202002L