[cfe-dev] [PATCH] Fixit for incorrect includes

[David Blaikie]

On Tue, Jul 17, 2012 at 10:48 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Tue, Jul 17, 2012 at 12:10 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> On Tue, Jul 17, 2012 at 9:14 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>> This patch creates a fixit for include directives where the file could
>>> not be found when using angle brackets, but can be found when using
>>> quotes.  The converse is not needed since quoted includes will search
>>> angle bracket locations by default.
>>>
>>> Eg)
>>> #include <header.h>  // can be found via #include "header.h" instead
>>>
>>> Thoughts?
>>
>> Seems like a neat idea to me - but I'm not an authoritative sign-off.
>>
>> You used NULL as null constants for 2 of the conditional operators,
>> then 0 for the third - that seems inconsistent. You might want to
>> check what the prevailing style is in this file & stick to that
>> (generally in LLVM, '0' seems to be winning as the authoritative null
>> pointer constant, I believe).
>
> Easy enough to rectify (I'll standardize on 0) -- this was a copy from
> above, so I'll fix up there as well.
>
>> Did you consider adding this case to the existing fixit testing files?
>> They're already a grab-bag of things that can be fixed (this helps
>> keep the test suite fast by not adding more separate test file
>> executions) & this seems like it'd be at home there.
>
> I thought about it, but it strikes me as different enough to warrant
> its own test case.  Specifically, the fixit behavior is kind of
> tricky.  Because the fixit writes out a separate file, but in a
> different directory, I have to be a bit sneaky otherwise the header
> file isn't available.  This is because the temp file goes to the
> Output directory, but the header file remains in the FixIt directory.
> To work around this, I copy the testcase to a temp file, modify the
> testcase file directly with the fixit,

Why can't you modify the temp file directly instead? (& you'd have to
copy the header over too, I suppose - you can identify the temp
directory with %T which means you can copy to fixed name files if
that's important (as it would be for the header name): %T/foo.cpp or
whatever)

& yeah, arguable whether the existing fixit cases should be burdened
with this mucking about - I don't mind too strongly either way.

> then copy the temp back
> (restoring the original contents).
>
>> [& this makes me wonder: how scary (would it even be
>> possible/practical?) would it be to do typo correction on #includes?
>> Though we should check if it's worthwhile first - I wonder how often
>> people make typos there]
>
> I was wondering the same thing.  For instance, one (perhaps) simple
> extension would be to try a case insensitive search for the file and
> suggest the proper casing if the file cannot be found.  This is
> especially interesting to x-platform developers where OS X and *nix
> are generally case sensitive files, and Windows is generally case
> insensitive.
>
> ~Aaron