[PATCH] Implement MSVC header search algorithm in MicrosoftMode

[Reid Kleckner]

Looks good, please commit!



On Fri, Dec 27, 2013 at 10:39 AM, Will Wilson <will at indefiant.com> wrote:

> Hi All,
>
> Thanks for the great feedback. I've fixed the comment formatting issue,
> the spurious typedef for the iterator and integrated Reid's extended test
> proposal.
>
> The other points:
>
> +    if (!FileEnt)
>> +      FileEnt = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
>> +
>> +    if (FileEnt)
>> +      Includers.push_back(FileEnt);
>> Is the second if statement needed?  Could we assert(FileEnt); here?
>
>
> FileEnt is NULL in a few test cases so I've kept the conditional for now.
> I didn't get chance to investigate further but the behavior should be
> unchanged from the previous revision.
>
> Will, do you think it's worth adding a diagnostic (warning) for this
>> new behavior, in the case where the new/MSVC behavior differs from the
>> old/GCC behavior? Something like
>>     warning: in Microsoft mode this #include directive refers to
>> 'a/b/foo.h', where
>>     normal header lookup would have found 'c/foo.h' instead
>> [-Wmsvc-include]
>> Silently accepting the construct just encourages people to rely on it.
>> Giving a diagnostic would at least let the relevant developers know
>> that something bogus is going on and that maybe they should fix their
>> directory structure. (And if they really don't care, they can silence
>> the warning.)
>
>
> An excellent idea. I've implemented what I hope is a reasonable middle
> ground between performance and diagnostics. Running the normal lookup rules
> just for a (probably ignored) warning seemed a little heavy handed. So in
> this case I warn with:
>
> warning: #include resolved using non-portable MSVC search rules as:
> path/to/foo.h [-Wmsvc-include]
>
>
> Let me know what you think and I'll get it in if everyone's happy with it.
>
> Cheers,
> Will.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131227/9bc71fe2/attachment.html>