dblaikie 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()); ---------------- aganea wrote: > rnk wrote: > > +@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) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69959/new/ https://reviews.llvm.org/D69959 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits