[PATCH] D80514: [clang-tidy] modernize-use-trailing-return-type support for C++20 concepts and decltype

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:293
+    StringRef File = SM.getBufferData(Loc.first);
+    const char *TokenBegin = File.data() + Loc.second;
+    Lexer Lexer(SM.getLocForStartOfFile(Loc.first), LangOpts, File.begin(),
----------------
Should we verify that `End` is valid before doing this pointer arithmetic with a value derived from it? For instance, what if `End` points into the scratch buffer because it relies on a macro defined on the command line -- will the logic still work?


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:430
+      AT->getKeyword() == AutoTypeKeyword::Auto &&
+      !hasAnyNestedLocalQualifiers(F->getDeclaredReturnType()))
+    return;
----------------
Why do we need to check that there aren't any nested local qualifiers?


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:285-302
+    SourceLocation End =
+        expandIfMacroId(ReturnLoc.getSourceRange().getEnd(), SM);
+    SourceLocation BeginNameF = expandIfMacroId(F.getLocation(), SM);
+
+    // Extend the ReturnTypeRange until the last token before the function
+    // name.
+    std::pair<FileID, unsigned> Loc = SM.getDecomposedLoc(End);
----------------
bernhardmgruber wrote:
> Is there an easier way to get the token previous to the function name?
There isn't, but if you find you're missing source location information for something, you can also consider modifying Clang to add that source location information and mark this as a dependent patch. It's not clear to me whether that would be worth the effort for this patch or not, but my preference is to avoid these little hacks whenever possible.


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

https://reviews.llvm.org/D80514