[clang-tools-extra] r342227 - [clangd] NFC: Fix IndexBenchmark CLI arguments handling

[Kirill Bobyrev via cfe-commits]

Thanks for the explanation! I didn’t know that benchmark’s Initialize does that, that was probably the source of my confusion. The suggestion looks reasonable, I should try this approach, it looks to be cleaner.

-Kirill

> On 18 Sep 2018, at 21:16, Roman Lebedev <lebedev.ri at gmail.com> wrote:
> 
> On Tue, Sep 18, 2018 at 10:09 PM, Kirill Bobyrev
> <kbobyrev.opensource at gmail.com> wrote:
>> Hi Roman,
>> 
>> Is there any benefit of doing so? Also, I’m not sure whether I understood you correctly. Consuming benchmark options *before* trimming would probably not be the desired behaviour since the first two arguments arguments are passed directly to the tool driver.
> Currently, you have to call it like
> $ IndexBenchmark IndexFilename RequestsFilename --benchmark_something
> 
> If you do
> $ IndexBenchmark --benchmark_something IndexFilename RequestsFilename
> your code will still consume argv[1] (i.e. "--benchmark_something") as
> IndexFilename
> 
> But if you do call  ::benchmark::Initialize(&argc, argv);  first it
> will work as you'd expect,
> i.e. you would still consume argv[1], but it then would have been
> adjusted to IndexFilename
> 
>> I might have misunderstood you, could you please elaborate on the proposed idea?
>> 
>> Kind regards,
>> Kirill
> Roman.
> 
>>> On 14 Sep 2018, at 14:46, Roman Lebedev <lebedev.ri at gmail.com> wrote:
>>> 
>>> On Fri, Sep 14, 2018 at 3:21 PM, Kirill Bobyrev via cfe-commits
>>> <cfe-commits at lists.llvm.org> wrote:
>>>> Author: omtcyfz
>>>> Date: Fri Sep 14 05:21:09 2018
>>>> New Revision: 342227
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=342227&view=rev
>>>> Log:
>>>> [clangd] NFC: Fix IndexBenchmark CLI arguments handling
>>>> 
>>>> Modified:
>>>>   clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp
>>>> 
>>>> Modified: clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp?rev=342227&r1=342226&r2=342227&view=diff
>>>> ==============================================================================
>>>> --- clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp (original)
>>>> +++ clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp Fri Sep 14 05:21:09 2018
>>>> @@ -101,9 +101,11 @@ int main(int argc, char *argv[]) {
>>>>  }
>>> 
>>>>  IndexFilename = argv[1];
>>>>  RequestsFilename = argv[2];
>>>> -  // Trim first two arguments of the benchmark invocation.
>>>> -  argv += 3;
>>>> -  argc -= 3;
>>>> +  // Trim first two arguments of the benchmark invocation and pretend no
>>>> +  // arguments were passed in the first place.
>>>> +  argv[2] = argv[0];
>>>> +  argv += 2;
>>>> +  argc -= 2;
>>>>  ::benchmark::Initialize(&argc, argv);
>>> Passing-by thought: why is this being done in *this* order?
>>> Why not first let the ::benchmark::Initialize() consume it's flags first?
>>> 
>>>>  ::benchmark::RunSpecifiedBenchmarks();
>>>> }
>>>> 
>>> Roman.
>>> 
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>