[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 0/4] RFC: add stats about parsing
From: |
Adrian Vogelsgesang |
Subject: |
Re: [PATCH 0/4] RFC: add stats about parsing |
Date: |
Thu, 4 Jul 2019 10:05:30 +0000 |
User-agent: |
Microsoft-MacOutlook/10.10.b.190609 |
Hi Akim,
>> For my projects, I don’t care which exact option is pursued since
>> I never use `yydebug` in commited code. I only add debug
>> code temporarily, fix the bug, and then remove the debug code
>> again before commiting anything to master.
> Ok. Which means you commit the generated files?
No, I commit the bison grammar and generate the file as part of the CMake
build steps.
I only never commit a grammar which contains debug code, i.e.m I never commit
a grammar which depends on `yydebug` or any other debugging functionality.
If I use `yydebug` to investigate a defect, I remove that debug code again
before
commiting the fixed grammar.
>> One question regarding variant 3a:
>> What would be the outcome of the combination
>>> %define parse.trace false
>>>> %define parse.stats true
> I don't like 3a, I prefer 3b :) But in the case you quote,
> I would probably make it an error: you cannot set parse.stats
> if parse.trace is not enabled.
>
> But then again, I mentioned 3a only to explain the commit.
> I should have emphasized that it's a bad solution: enhancing
> parse.trace is better imho.
If this combination would be an error, I also prefer 3b over 3a.
Instead of checking for a contradiction, let's build the interface in such a
way that it's syntactically impossible to get into that error state.
My personal ranking would be 2a (best), 3b, 2b (worst),
I like 2b the least since it would break compatibility by outputting different
traces.
I prefer 2a over 3b since 2a provides more control from within the program: I
can
switch between traditional traces and stats.
Cheers,
Adrian
On 03/07/2019, 22:45, "Akim Demaille" <address@hidden> wrote:
Hi Adrian,
Thanks for the feedback.
> Le 3 juil. 2019 à 14:08, Adrian Vogelsgesang <address@hidden> a écrit :
>
> Hi Akim,
>
> collecting statistics about a parse sounds like a great idea to me
> and I am looking forward to it.
> Thanks for the time and effort you spent on this! :)
Well, on this particular feature, I was driven by own needs :)
But thank you.
> For my projects, I don’t care which exact option is pursued since
> I never use `yydebug` in commited code. I only add debug
> code temporarily, fix the bug, and then remove the debug code
> again before commiting anything to master.
Ok. Which means you commit the generated files?
> I don’t think anyone should have a dependency in production
> code on `yydebug` behavior.
Well, Bison, when installed, does keep the parser traces on
(and you can trigger them with --trace=parse).
But you are right: this is debatable. I should probably stop
doing this in the generated tarball. It's ok for the git repo,
but maybe the generate tarballs should not have it enabled by
default.
> Hence, I guess breaking the
> POSIX standard here would be fine.
> Still, I share your concerns about breaking standards.
>
> Hence, I would be in favour of variant 2 or 3.
Thanks for the feedback!
> One question regarding variant 3a:
> What would be the outcome of the combination
>> %define parse.trace false
>> %define parse.stats true
I don't like 3a, I prefer 3b :) But in the case you quote,
I would probably make it an error: you cannot set parse.stats
if parse.trace is not enabled.
But then again, I mentioned 3a only to explain the commit.
I should have emphasized that it's a bad solution: enhancing
parse.trace is better imho.
- [PATCH 0/4] RFC: add stats about parsing, Akim Demaille, 2019/07/03
- [PATCH 2/4] parse.stats: add --trace=parse-stats, Akim Demaille, 2019/07/03
- [PATCH 1/4] parse.stats: new feature of yacc.c, Akim Demaille, 2019/07/03
- [PATCH 4/4] parse.stats: documentation, Akim Demaille, 2019/07/03
- [PATCH 3/4] regen, Akim Demaille, 2019/07/03
- Re: [PATCH 0/4] RFC: add stats about parsing, Adrian Vogelsgesang, 2019/07/03
- Re: [PATCH 0/4] RFC: add stats about parsing, Akim Demaille, 2019/07/03
- Re: [PATCH 0/4] RFC: add stats about parsing,
Adrian Vogelsgesang <=
- [RFC PATCH 0/4] RFC: add stats about parsing, Akim Demaille, 2019/07/07
- [RFC PATCH 1/4] parse stats: new feature of yacc.c, Akim Demaille, 2019/07/07
- [RFC PATCH 2/4] parse stats: add --trace=parse-stats, Akim Demaille, 2019/07/07
- [RFC PATCH 3/4] regen, Akim Demaille, 2019/07/07
- [RFC PATCH 4/4] parse stats: documentation, Akim Demaille, 2019/07/07