================ Comment at: lib/Tooling/CommonOptionsParser.cpp:141-142 @@ -93,2 +140,4 @@ } + Compilations = llvm::make_unique<ArgumentsAdjustingCompilations>( + std::move(Compilations), ArgsBefore, ArgsAfter); } ---------------- alexfh wrote: > klimek wrote: > > alexfh wrote: > > > klimek wrote: > > > > It seems to me like we'd want either this to take an ArgumentAdjuster > > > > instead of hard-coding to the args-before / args-after case? > > > This would make sense, if we make this compilation database wrapper > > > public. But for this specific case the total amount of code is not larger > > > than what would be needed for the ArgumentsAdjuster-based variant. > > Well, if the code is not much larger, I'd go with the variant that fits the > > names better... Or do you have a specific reason for not handing in an > > argument adjuster? To me it was just unexpected when reading the code... > I'd like to see ArgumentsAdjuster replaced with an appropriate std::function > eventually. As for the name of the compilation database wrapper, I'd better > change it to something closer to the implementation, than vice versa. How > about ExtraArgumentsInserterCompilations? Or CompilationsWithExtraArguments? I'm not convinced that renaming is better - can you provide an argument on why you prefer the current implementation (you said the more composable implementation would be ~ the same length).
http://reviews.llvm.org/D6073 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
