[PATCH] D56160: [clang-tidy] modernize-use-trailing-return-type check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:34
+  bool VisitUnqualName(StringRef UnqualName) {
+    // Check for collisions with function arguments
+    for (ParmVarDecl *Param : F.parameters())
----------------
Missing full stop at the end of the sentence.


================
Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:150
+
+    if (T.isOneOf(tok::amp, tok::ampamp, tok::kw_const, tok::kw_volatile)) {
+      Result = T.getEndLoc();
----------------
This list misses an edge case for `__restrict`. e.g., https://godbolt.org/z/d4WDcA


================
Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:180
+      if (Info.hasMacroDefinition()) {
+        // The CV qualifiers of the return type are inside macros
+        diag(F.getLocation(), Message);
----------------
Missing full stop.


================
Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:229
+  auto IsCV = [](Token T) {
+    return T.isOneOf(tok::kw_const, tok::kw_volatile);
+  };
----------------
This also misses the `__restrict` case.


================
Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:274
+
+  if (F->getLocation().isInvalid())
+    return;
----------------
Should this also bail out if the function is `main()`?


================
Comment at: test/clang-tidy/modernize-use-trailing-return-type.cpp:95-97
+extern "C" int d2(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}extern "C" auto d2(int arg) -> int;{{$}}
----------------
This is a rather unexpected transformation, to me.


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

https://reviews.llvm.org/D56160