[cfe-commits] [Patch 1 of 4] -verify fixes (stage 2)

[Andy Gibbs]

On Saturday, July 28, 2012 3:18 AM, Jordan Rose wrote:
> On Jul 26, 2012, at 2:16 AM, Andy Gibbs <andyg1001 at hotmail.co.uk> wrote:
>> I have implemented all of your comments, except the one relating to the
>> tracking on Begin/EndSourceFile calls, for three reasons: firstly, I need
>> to track in order to correctly attach and detach the preprocessor hooks;
>> secondly, doing so highlighted the mismatch bug and (who knows) may do
>> again in future; thirdly, when it comes the ARCMT tests fix in patch 2,
>> I rely on knowing when the last EndSourceFile has been called.
>
> Looks pretty good, but there's one last thing I'm not sure about. It kind
> of scares me that BeginSourceFile/EndSourceFile calls can be nested; I
> always thought that they were more BeginTranslationUnit /
> EndTranslationUnit, since they are most definitely NOT called for
> #includes. But they do seem to be called for searching module map files
> and for "chained includes" (which, by the way, seems like it could also
> have an unbalanced begin/end pair on error).
>
> If they really are supposed to be nested, then your code is correct. If
> not, though, we should probably just be using a boolean for this.

I can't speak for the original implementors and what their intention was
for BeginSourceFile/EndSourceFile but from my observations, it seems they
are called for each source file which is "outside" the main source file.
This means, as you say, it is not called for #includes since these are
"within" the main source file (i.e. incorporated by the preprocessor),
but is called for module map files and also the -ast-merge function.  I
hadn't observed it with chained includes and I will go and check whether
there is a mismatched begin/end pair there.

I will say that while I was not a progenitor, I am now a propagator of
nested use, since fixing CaptureDiagnosticConsumer now relies on nesting
to avoid premature diagnostic checking.

> Sorry for the delay; plenty of other stuff going on...
> Jordan

No worries  :o)

Andy