Reading behind Streams in rpp

[David Nolden]

Am Mittwoch 13 Mai 2009 15:13:35 schrieb Christoph Bartoschek:
> Am Mittwoch, 13. Mai 2009 schrieb David Nolden:
> > Am Dienstag 12 Mai 2009 23:26:42 schrieb Christoph Bartoschek:
> > > Hi,
> > >
> > > the attached patch adds a check for out of bounds accesses in rpp. It
> > > also fixes some usages that violate the assertion. However I am not
> > > 100% sure for the # operator.
> >
> > Why did you do those changes to the # operator? That operator is supposed
> > to stringify the text, thus it has to escape stuff. Not sure about the
> > newline thing, but better leave it as it was.
>
> 1. The old code was wrong by looking at the variable input instead of the
> vriable is.
>
> 2. Most escapes are done in the for-loop above the while-loop.
Ok. I just looked at the patch, not at the whole code. There is some tests for 
the preprocessor in kdevelop/languages/cpp/tests/cppcodecompletiontest and 
also in test_parser or similar, those have to pass. But it's possible that the 
lines you removed are some old relict. I've got no time to look more exactly 
into that now.

> > > One additional question: What is the aim of rpp? Should it implement
> > > the preprocessor correctly? Or should it only show an approximation of
> > > the correct behaviour?
> >
> > It is supposed to correctly model the preprocessor at some point. There
> > is some subtle problems yet though, that are visible in some boost
> > preprocessor libraries. I've already fixed quite a few of those, but
> > there's still some left. I'm not planning to keep working on that part
> > though, so if you want to improve it, feel free to do so. :-)
>
> Then the preprocessor should match the output of cpp. This is currently not
> the case. Especially the operator # is handled wrong in the old code. I
> also see that rpp does not handle the code I normally work with correctly.
Yes. If you fix something in the preprocessor, please always try to also 
create a test(for example in that cppcodecompletiontest, or in test_parser), 
that shows up the problem, and verifies that the change actually fixes it, and 
so it isn't broken again later.

The general text output of rpp should equal that of cpp, except that it also 
contains comments. Also there is some important and complicated source-code 
position mapping going on there, so be cautious. I think though that the 
mapping is well tested.

> I try to find time to fix rpp.
Great.

> > About the basic problem: operator++ and the likes are very performance
> > critical, and we should save every single instruction we can save within
> > that one.
>
> I've removed an unnecessary check in operator++. I assume that the
> assertions are not part of released code.
Yes that is true. Still I like also the debug code to be fast, since I use it 
every day. :-)

> > So maybe it would make sense to just append a zero to each input-vector
> > passed to the preprocessor, then any loop should terminate without adding
> > new code all around rpp, because zero does not equal anything that is
> > expected. What do you think?
>
> An idea worth thinking about. However most checks are necessary anyway to
> get better problem reporting from the preprocessor. For example the two
> checks in pp::handle_include improve the output of the preprocessor in the
> problem section. Additionally the second one prevents a memory leak.
Yes your added error messages make sense anyway. But at least some of the 
atEnd() checks could be removed, and we would be sure that the same problem 
would not appear in other less common places. In this case, those less common 
places would hit the assertion, and I'm currently quite happy that kdevelop is 
_not_ randomly crashing. It would be important that atEnd() would be true 
already when the cursor is on that terminating zero item.

> I look what I can do in my limited time and show you the results for
> discussion.
Cool.

Greetings, David