[PATCH] D33841: [clang-tidy] redundant 'extern' keyword check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.


================
Comment at: clang-tidy/readability/RedundantExternCheck.cpp:30
+
+  if (FD->getStorageClass() != SC_Extern)
+    return;
----------------
Can you do that in `registerMatchers()`?


================
Comment at: clang-tidy/readability/RedundantExternCheck.cpp:43-44
+
+  if (FD->getBeginLoc().isMacroID())
+    return;
+
----------------
Similarly, do this in `registerMatchers()`


================
Comment at: clang-tidy/readability/RedundantExternCheck.cpp:49
+                           *Result.SourceManager, getLangOpts());
+
+  int Offset = Text.find("extern");
----------------
`StringLiteral Extern = StringLiteral("extern");`


================
Comment at: clang-tidy/readability/RedundantExternCheck.cpp:50
+
+  int Offset = Text.find("extern");
+
----------------
Extern


================
Comment at: clang-tidy/readability/RedundantExternCheck.cpp:53
+  SourceRange ExternRange(BeginLoc.getLocWithOffset(Offset),
+                          BeginLoc.getLocWithOffset(Offset + strlen("extern")));
+
----------------
Extern.size()


================
Comment at: clang-tidy/readability/RedundantExternCheck.cpp:59
+
+  if (!ExternText.equals("extern")) // Final check: typedefs, etc.
+    return;
----------------
Extern


================
Comment at: docs/clang-tidy/checks/readability-redundant-extern.rst:10
+
+The default language linkage is C++, so without any additional parameters it is redundant (extern "C++" can also be redundant, but it depends on the context). In C context (extern "C") the situation is the same, extern keyword is redundant for function declarations
+
----------------
Please 80-char wrap


================
Comment at: docs/clang-tidy/checks/readability-redundant-extern.rst:14
+
+  extern void h();
+
----------------
More examples? A namespace one?


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

https://reviews.llvm.org/D33841