[clang-tidy] Update fuchsia-overloaded-operator to check for valid loc

[juliehockett]

Summary

Updating fuchsia-overloaded-operator check to not issue warnings for invalid locations.

Fixes PR35803.

[aaron.ballman]

LGTM with a minor suggestion to not call getLocStart() twice and a formatting fix. I think you should also ping @hans to get this pulled into the 6.0 branch once it's been commit.

clang-tidy/fuchsia/OverloadedOperatorCheck.cpp

33–39

I think this code can be simplified to:

const auto *D = Result.Nodes...
assert(D && "No FunctionDecl captured!");

SourceLocation Loc = D->getLocStart();
if (Loc.isValid())
  diag(Loc, ...);

test/clang-tidy/fuchsia-overloaded-operator.cpp

20

Formatting is off -- this should be operator delete(... (no space after "delete").

[alexfh]

clang-tidy/fuchsia/OverloadedOperatorCheck.cpp

38

This is not related to the fix, but the warning message is unclear, incorrect and confusing: one most certainly _can_ overload these functions, but shouldn't do this in fuchsia code due to certain reasons. Ideally, warning messages should make it clear to the reader what's wrong with the code, why, and how to fix it.

[hans]

Alex, do you think the fix is still OK for the 6.0 branch? Let me know, and I'll merge it.

[juliehockett]

The update to the message is also up for review here: D42120

[alexfh]

In D41708#977000, @hans wrote:

Alex, do you think the fix is still OK for the 6.0 branch? Let me know, and I'll merge it.

Yes, please. Thank you!

[hans]

In D41708#978186, @alexfh wrote:

In D41708#977000, @hans wrote:

Alex, do you think the fix is still OK for the 6.0 branch? Let me know, and I'll merge it.

Yes, please. Thank you!

Oh, it was already merged in r321800. Sorry for the noise.

clang-tidy/fuchsia/OverloadedOperatorCheck.cpp

 }
 
 void OverloadedOperatorCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(functionDecl(isFuchsiaOverloadedOperator()).bind("decl"),
                      this);
 }
 
 void OverloadedOperatorCheck::check(const MatchFinder::MatchResult &Result) {
-  if (const auto *D = Result.Nodes.getNodeAs<FunctionDecl>("decl"))
-    diag(D->getLocStart(), "cannot overload %0") << D;
+  const auto *D = Result.Nodes.getNodeAs<FunctionDecl>("decl");
+  assert(D && "No FunctionDecl captured!");
+  
+  SourceLocation Loc = D->getLocStart();
+  if (Loc.isValid())
+    diag(Loc, "cannot overload %0") << D;
 }
 
 } // namespace fuchsia
 } // namespace tidy
 } // namespace clang

test/clang-tidy/fuchsia-overloaded-operator.cpp

public:
   B &operator=(const B &Other);
   // CHECK-MESSAGES-NOT: [[@LINE-1]]:3: warning: cannot overload 'operator=' [fuchsia-overloaded-operator]
   B &operator=(B &&Other);
   // CHECK-MESSAGES-NOT: [[@LINE-1]]:3: warning: cannot overload 'operator=' [fuchsia-overloaded-operator]
 };
 
 A operator-(const A &A1, const A &A2);
 // CHECK-MESSAGES: [[@LINE-1]]:1: warning: cannot overload 'operator-' [fuchsia-overloaded-operator]
+
+void operator delete(void*, void*) throw();
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: cannot overload 'operator delete' [fuchsia-overloaded-operator]