bernhardmgruber added a comment. Great, I seems I forgot to submit the inline comments. Sorry about that!
================ 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(), ---------------- aaron.ballman wrote: > 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? That sounded like a scary scenario! Fortunately, `expandIfMacroId` even expands source locations inside the scratch buffer correctly into a location in a real file. I added two tests for it and they seem to pass. Thank you for the hint! ================ Comment at: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:430 + AT->getKeyword() == AutoTypeKeyword::Auto && + !hasAnyNestedLocalQualifiers(F->getDeclaredReturnType())) + return; ---------------- aaron.ballman wrote: > Why do we need to check that there aren't any nested local qualifiers? Because I would like the check to rewrite e.g. `const auto f();` into `auto f() -> const auto;`. If I omit the check for nested local qualifiers, then those kind of declarations would be skipped. ================ 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); ---------------- aaron.ballman wrote: > 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. Thank you for the hint! I considered this for the concept location in an `AutoReturnType` and the expression inside a `decltype` `AutoReturnType`. Unfortunately I could not wrap my head around what is going on in SemaType.cpp :/ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80514/new/ https://reviews.llvm.org/D80514 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
