Referencing 'new' pulls in a bogus include

[GoogleCodeExporter]

When I reference new in my source, IWYU informs me:

blah.h should add these lines:
#include "/usr/include/c++/<version>/new"  // for operator new

I believe on other platforms this is being handled by NormalizeSystemPath. 
However on Win32 (possibly as a result of some of my other changes) this is 
being picked up as a local include (Case 1 in ConvertToQuotedInclude).

I can see where this include is being generated (iwyu.cc:1920). One (gross) fix 
would be to use "C:/usr/include/c++/<version>/new" for this on Win32. I'll hold 
off on that for now and try to find something a little nicer...

Original issue reported on code.google.com by paul.hol...@gmail.com on 9 Mar 2011 at 11:09

[GoogleCodeExporter]

It's not windows-specific at all; I noticed it too just today.  The normalizer 
doesn't try to deal with arbitrary versions past the "c++" anymore; it uses the 
actual, correct list of include directories instead.

Looking through the code, it looks like it should be possible to replace 
"/usr/include/c++/<version>/new", in iwyu.cc, with just "<new>" -- I designed 
the code to allow for an already-quoted include.  It's possible this breaks 
iwyu_output.cc's SetPublicHeaders.  (The right fix there might be to make 
ConvertToQuotedInclude a noop if the input is already a quoted include.  Or it 
might be to just detect we're already a quoted include.  Or it might be to 
check if suggestd_header_ is not empty, and if so set the public header to 
that.)  Do you want to take a shot at that, and fix up anything that might need 
fixing?

Original comment by csilv...@gmail.com on 9 Mar 2011 at 11:56

[GoogleCodeExporter]

Switching to "<new>" works fine in both the code base I'm working with and a 
few hand-rolled test cases. It seems that SetPublicHeaders() is never invoked 
for this symbol following this change. I think the NeedsSuggestedHeader (as 
used by CalculateMinimalIncludes) is already taking care of this, but I might 
have missed something.

So it seems like the attached (trivial :) patch is a good change to me, but I'm 
not able to run the IWYU testsuite.

Original comment by paul.hol...@gmail.com on 10 Mar 2011 at 10:13

Attachments:

• iwyu_new.patch

[GoogleCodeExporter]

Great, glad it works for you.  I added an assert in iwyu_output.cc, just in 
case, and committed as r60

Original comment by csilv...@gmail.com on 10 Mar 2011 at 11:16

• Changed state: Fixed