[PATCH] D142905: Change multilib selection algorithm

[Michael Platings via Phabricator via cfe-commits]

michaelplatings added inline comments.


================
Comment at: clang/include/clang/Driver/Multilib.h:31
 public:
-  using flags_list = std::vector<std::string>;
+  using flags_list = std::set<std::string>;
+  using arg_list = std::vector<std::string>;
----------------
peter.smith wrote:
> would flags_set be a better name. I'm guessing we're using set as we want uniqueness and ordering? 
> 
> If we don't need ordering then we may be able to use https://llvm.org/docs/ProgrammersManual.html#llvm-adt-stringset-h
Renamed to flag_set.

Yes, we want uniqueness and ordering. I don't think ordering matters in this change, but later changes rely on it.


================
Comment at: clang/include/clang/Driver/Multilib.h:106
+  /// Select compatible variants
+  std::vector<Multilib> select(const Multilib::flags_list &Flags) const;
+
----------------
peter.smith wrote:
> Worth using multilib_list for consistency with the rest of the file?
> 
> Is this meant to be an overload for bool select below? I mention it as it has a different return type. Perhaps use selectCompatible? 
Changed to use multilib_list  now, thanks.

The overloading goes away in D143059.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142905