[cfe-commits] r160068 - in /cfe/trunk: include/clang/Frontend/ lib/Frontend/ test/Frontend/ test/Lexer/ test/Modules/ test/Modules/Inputs/Module.framework/Headers/ test/PCH/ test/Preprocessor/ test/SemaCXX/

[Andy Gibbs]

Hi,

Firstly, thank you ever so much for all your work getting all these patches
through.

I had a quick scan through to see what you changed, and it all seemed fine 
to
me, although you could simplify the \r \n algorithm somewhat but this is far
from critical.  (I'm afraid I come from an embedded background and my mind 
is
unfortunately tuned to spotting reducible branch operations.  It is a truly
terrible habit! :o)

The one change I am in two minds about is this:

> +  if (!C2.empty())
> +    if (ParseDirective(C2, ED, SM, CommentBegin, PP.getDiagnostics()))
> +      if (const FileEntry *E = 
> SM.getFileEntryForID(SM.getFileID(CommentBegin)))
> +        FilesWithDirectives.insert(E);

(This appears in a similar way in an earlier location).

Superficially this change makes sense, and in the end once we've eliminated
the need for the FilesWithDiagnostics and FilesWithDirectives lists then it
is irrelevant.  In the meantime, it means that files with directives only
inside #if blocks will now always get reparsed at the end of processing and
all the directives contained will be exposed.

In the case in my patch, if that file at least had a comment outside a 
skipped
#if block, then this file was considered to have been parsed for directives
and was not parsed again.  Its tenuous and I'm wrestling in my head as to
whether the change in behaviour really matters or not...

Obviously the sooner I can pin down all the exceptions that currently make
this post-processing necessary, the better.

Anyway, I am very grateful to you for all your patience while I got the
patches up to scratch.

Cheers
Andy