hwright added inline comments.
================
Comment at: clang-tidy/abseil/DurationRewriter.cpp:35
+getInverseForScale(DurationScale Scale) {
+ static const std::unordered_map<DurationScale,
+ std::pair<llvm::StringRef, llvm::StringRef>>
----------------
lebedev.ri wrote:
> https://llvm.org/docs/ProgrammersManual.html#other-map-like-container-options
> > We never use hash_set and unordered_set because they are generally very
> > expensive (each insertion requires a malloc) and very non-portable.
>
> Since the key is an enum, [[
> https://llvm.org/docs/ProgrammersManual.html#llvm-adt-indexedmap-h |
> `llvm/ADT/IndexedMap.h` ]] should be a much better fit.
It doesn't look like `IndexedMap` has a constructor which takes an initializer
list. Without it, this code get a bit more difficult to read, and I'd prefer
to optimize for readability here.
================
Comment at: clang-tidy/abseil/DurationRewriter.cpp:68
+ getInverseForScale(Scale);
+ if (const Expr *MaybeCallArg = selectFirst<const Expr>(
+ "e", match(callExpr(callee(functionDecl(
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > `if (const auto *MaybeCallArg`
> Early return?
I'm not quite sure what you mean by `Early return?` Are you suggesting that
the call to `selectFirst` should be pulled out of the `if` conditional, and
then the inverse checked to return `llvm::None` first?
================
Comment at: clang-tidy/abseil/DurationRewriter.cpp:74
+ Node, *Result.Context))) {
+ return tooling::fixit::getText(*MaybeCallArg, *Result.Context).str();
+ }
----------------
lebedev.ri wrote:
> So you generate fix-it, and then immediately degrade it into a string. Weird.
This doesn't generate a fix-it, it just fetches the text of the given node as a
`StringRef` but we're returning a `string`, so we need to convert.
Is there a more canonical method I should use to fetch a node's text?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55245/new/
https://reviews.llvm.org/D55245
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
