[PATCH] D111709: [Flang] flang-omp-report replace std::vector's with llvm::SmallVector

[David Truby via Phabricator via llvm-commits]

DavidTruby added inline comments.


================
Comment at: flang/examples/flang-omp-report-plugin/flang-omp-report-visitor.h:101
+  llvm::SmallVector<OmpWrapperType *> ompWrapperStack;
+  std::map<OmpWrapperType *, llvm::SmallVector<ClauseInfo>> clauseStrings;
   Parsing *parsing{nullptr};
----------------
clementval wrote:
> DavidTruby wrote:
> > clementval wrote:
> > > DavidTruby wrote:
> > > > just putting this here as a note (this should be done in a separate patch):
> > > > This `std::map` should probably be changed to an `llvm::DenseMap` or similar
> > > You can use `std::unordered_map` here instead. 
> > Use of `std::unordered_map` is highly discouraged in llvm because it has portability issues across implementations. See https://llvm.org/docs/ProgrammersManual.html#other-map-like-container-options for more details
> Unordered_set is discourage but not unordered_map. It is used widely in the code base
I'm not sure why `unordered_set` would be discouraged but not `unordered_map`, they're implemented the same way so would surely exhibit the same issues? The section of the manual I linked to specifically says to avoid that "hash map like" containers and then (I think erroneously) calls out `unordered_set` when I presume it means `unordered_map`. Perhaps that needs a fix in the manual?

Regardless with a quick grep through llvm I don't see many uses of `unordered_map` outside of libcxx (which of course defines it), especially compared to uses of DenseMap.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111709