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

[Valentin Clement via Phabricator via llvm-commits]

clementval 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};
----------------
DavidTruby wrote:
> 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.
You probably read between the line because the section you linked does not says avoid hash map like container anywhere. It just mentions that the STL provides such containers.

> The STL provides several other options, such as std::multimap and the various “hash_map” like containers (whether from C++ TR1 or from the SGI library). We never use hash_set and unordered_set because they are generally very expensive (each insertion requires a malloc) and very non-portable.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111709