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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1553
+  "%select{references|destructors|block pointers|ref-qualified member functions}2">,
+  InGroup<DiagGroup<"declare-with-symbols">>;
+
----------------
@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).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107292