[cfe-commits] [PATCH] -rewrite-includes

[Lubos Lunak]

On Tuesday 27 of March 2012, Matt Beaumont-Gay wrote:
> On Fri, Mar 23, 2012 at 02:08, Lubos Lunak <l.lunak at suse.cz> wrote:
> > On Friday 23 of March 2012, Matt Beaumont-Gay wrote:
> >> Prefer std::vector -- no reason to use std::list here.
> >
> >  I've meanwhile confirmed that it should be actually std::map for
> > noticeably better performance.
>
> Does a DenseMap (#include "llvm/ADT/DenseMap.h") of SourceLocation to
> pair<FileID, CharacteristicKind> improve performance any further?

 No, because it doesn't compile. Even if I use it properly, the difference is 
neglibible.

> >  Clang does not even bother to perform repeated inclusion of the same
> > file if it somehow detects it's unnecessary, but the avoided
> > FileChanged() callback would mean -rewrite-includes would otherwise leave
> > some #include directives in the result that would be performed the next
> > time.
>
> Can you use the FileSkipped() callback to make this any more efficient?

 No. If you are asking about the quoted part above, then it's not about 
efficiency, it's about correct/incorrect. If it was a question in general, 
then it's more efficient (and probably also correct, given the action is 
supposed to rewrite includes) to just comment out everything instead of 
figuring out exactly what needs it or not.

> A few new comments, in addition to the general style issues which are
> still unaddressed (doxygen, whitespace, etc):

 I have already converted function/type comments into doxygen comments, and 
there's no point in converting code comments. If you meant to say something 
else with "Doxygen comments throughout", you didn't actually say it.

 Updated patch attached.

-- 
 Lubos Lunak
 l.lunak at suse.cz
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rewrite-includes.patch
Type: text/x-diff
Size: 29094 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120331/dbbc9d7b/attachment.patch>