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

[Alexandre Ganea via Phabricator via cfe-commits]

aganea marked an inline comment as done.
aganea added inline comments.


================
Comment at: llvm/include/llvm/ADT/SmallVector.h:905
+
+  const SmallVector &operator=(const std::vector<T> &Vec) {
+    this->assign(Vec.begin(), Vec.end());
----------------
dblaikie wrote:
> aganea wrote:
> > 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`.
> I agree with @dexonsmith's suggestion/agreement with @rnk's suggestion to use SmallVector - but to implement the suggestion I'd rather change this:
> 
>   Opts.ASTMergeFiles = Args.getAllArgValues(OPT_ast_merge);
> 
> to this:
> 
>   const cl::list &ASTMergeFiles = Args.getAllArgValues(OPT_ast_merge);
>   Opts.ASTMergeFiles.assign(ASTMergeFiles.begin(), ASTMergeFiles.end());
> 
> As @rnk was mentioning. (alternatively, I suppose, since cl::list already has implicit conversion to std::vector, perhaps adding an implicit conversion to SmallVector /there/ would be consistent/not too bad/better than adding std::vector conversion to SmallVector to SmallVector itself)
Can we do that in a subsequent patch? This current patch as it stand fixes the issue mentioned in the summary.


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

https://reviews.llvm.org/D69959