kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:89
+      Result.try_emplace(Token(Token::Kind::Trigram, E.first.str()),
+                         std::move(E.second));
+    TrigramDocs.clear();
----------------
sammccall wrote:
> the move here does nothing, we're passing as ArrayRef
> 
> did you want to clear the map values (vectors) as you went, too?
> 
> (this gets a bit verbose, but you could pull out a function template since 
> all 4 cases are so regular)
> did you want to clear the map values (vectors) as you went, too?

yes that's the idea. would you prefer an explicit destructor call?

> (this gets a bit verbose, but you could pull out a function template since 
> all 4 cases are so regular)

the problem is 1 of these is a densemap, other three is a stringmap, and onther 
is a vector. densemap and the stringmap doesn't really go together because 
there's no common method to get the key (densemap has `first` as a field, or 
`getFirst()` as a method. stringmap has `first()` or `getKey()` as methods :/).
happy to move out just the three of them into a helper like:
```
buildPostingList(StringMap<..>, TokenKind, Out);
```
if you think that's still helpful (that way we can also get rid of the calls to 
clear since we can just move the maps into the function arguments).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124240

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to