|
 |
On 3/25/19 7:41 PM, clipka wrote:
> Am 25.03.2019 um 23:55 schrieb clipka:
>
>> If `Cond_Stack.back().PMac->endPosition == CurrentFilePosition()`
>> throws an exception, it means either of two things:
>>
>> (A) The positions compared are from two different files, which means
>> the calling code has failed to check whether it is even in the right
>> file.
>>
>> (B) The line/column tracking is buggy, and yields different positions
>> in different situations.
>
>
> Well, looking at the code of `IsEndOfInvokedMacro()` again, it
> _strongly_ reeks of scenario (A):
>
> return (Cond_Stack.back().PMac->endPosition ==
> CurrentFilePosition()) &&
> (Cond_Stack.back().PMac->source.fileName ==
> mTokenizer.GetInputStreamName());
>
> should probably be swapped around, like so:
>
> return (Cond_Stack.back().PMac->source.fileName ==
> mTokenizer.GetInputStreamName()) &&
> (Cond_Stack.back().PMac->endPosition ==
> CurrentFilePosition());
>
> so that the `LexemePosition` comparison operator is only invoked if the
> file names match.
>
>
> I guess you got really lucky in your test (and the OP unlucky in their
> use case), in that you happen to have created the following scenario:
>
> - A macro is invoked.
> - That macro includes a different file.
> - That included file happens to have a `#` (e.g. `#declare`) at the same
> file offset as the `#end` of the macro.
> - However, the `#` happens to be in a different line and/or column.
>
>
> In such a scenario, encountering the `#`, the parser would first test
> whether this is the `#end` of the macro it has been waiting for, calling
> `IsEndOfInvokedMacro()`.
>
> `IsEndOfInvokedMacro()` in turn would checks whether the current file
> position matches that of the macro's `#end` by calling
> `LexemePostition::operator==`.
>
> `LexemePostition::operator==` in turn would find that the file position
> does match. It would then verify that the match also reflects in line
> and column, and find that they differ, and report this inconsistency via
> an Exception.
>
> `IsEndOfInvokedMacro()` wouldn't even get a chance to test whether it is
> looking at the right file, because its author (duh, I wonder who that
> stupid git might have been...) chose to do the two tests in the wrong
> order.
>
>
> Now aside from the stupid git who authored `IsEndOfInvokedMacro()`
> there's someone else to blame for the situation, namely whoever thought
> it was a good idea to keep the stack of include files separate from the
> stack of pending "conditions". If they were using one and the same
> structure, having `INVOKING_MACRO_COND` on top of the stack would imply
> that we're in the file with the `#end`, and the file name test would be
> redundant.
>
> I think it should be possible to merge the include stack into the
> "condition" stack without adverse effects, but I haven't finished work
> on that yet.
>
>
> I should note that I haven't tested this hypothesis and fix for the
> error yet; I intend to do that as soon as I find the time, but of course
> you're welcome to beat me to it.
Thanks for the detailed explanations and, though I'm busy with RL
starting in a little while today, I think you've figured it out.
I know from looking at the values the position was matching and the line
and column were not. Further, when I look at my test cases (all some
pretty close variation of Warren's original) the failing ones all have
the # of the declare in just the right spot as you suggested. I'd not
recognized this was so.
My patch, then, just avoided the assertion in the
LexemePosition::operator== I suppose. I also didn't know the parser
assertions were today on always and I wasn't thinking an assert could
cause the exception.
Quick additional question. Is it the --enable-debug configure flag that
- normally - turns on the pov asserts? I've never verified this
assumption of mine by creating a pov assert in the code I know should
fail.
Bill P.
Post a reply to this message
|
|
