[PATCH] D69959: [C-index] Fix test when using Debug target & MSVC STL

Alexandre Ganea via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 13 10:51:03 PST 2019


aganea marked 3 inline comments as done.
aganea added inline comments.


================
Comment at: clang-tools-extra/clang-move/tool/ClangMove.cpp:113
   move::MoveDefinitionSpec Spec;
-  Spec.Names = {Names.begin(), Names.end()};
+  Spec.Names = (std::vector<std::string> &)Names;
   Spec.OldHeader = OldHeader;
----------------
dblaikie wrote:
> rnk wrote:
> > Converting from std::list to SmallVector by way of std::vector with a C-style cast is a bit surprising. Is there a simpler way to write this, like `Spec.Names.assign(Names.begin(), Names.end());`?
> Yep, +1 to assign(begin, end) & leaving SmallVector's API alone/not adding a converting assignment operator.
The RHS `Names` is a `cl::list` which defines a cast operator for `std::vector<DataType>`, see [[ https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/Support/CommandLine.h#L1516 | here ]].
However I agree it is a bit surprising, it'd maybe better if that was a real function, so we could write `Names.getVec()` instead.


================
Comment at: llvm/include/llvm/ADT/SmallVector.h:905
+
+  const SmallVector &operator=(const std::vector<T> &Vec) {
+    this->assign(Vec.begin(), Vec.end());
----------------
rnk wrote:
> + at dblaikie, who knows more about STL container details than I do.
This assignment was added to support converting `FrontendOptions`'s members from `std::vector` -> `SmallVector` as suggested by @dexonsmith. As stated above, this was to support code such as:
```
Opts.ASTMergeFiles = Args.getAllArgValues(OPT_ast_merge);
```
Which otherwise wouldn't work, as `getAllArgValues()` returns a `std::vector`.


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

https://reviews.llvm.org/D69959





More information about the llvm-commits mailing list