[cfe-dev] case-insensitive #include warning

[Bruno Cardoso Lopes via cfe-dev]

Hi Eric,

I measured it locally and it doesn't seem to introduce any significant
compile time slowdown. Ideally this should be guarded by a flag so
that we can avoid any potential compile time issue; although there's a
flag declaration in the patch, I don't see it getting used to guard
the warning.

The patch is definitely interesting. I would like to measure it once
more after it gets properly reviewed though.
Can you upload your patches to http://reviews.llvm.org? Please add me
as a reviewer.

Please leave the trailing space / cosmetic fixes out of the patch, it
becomes much easier to review.

Thanks,

On Tue, Apr 19, 2016 at 1:27 PM, Eric Niebler via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
> On 4/14/16, 1:14 PM, "Eric Niebler" <eniebler at fb.com> wrote:
>
>>On 4/7/16, 4:44 PM, "Eric Niebler" <eniebler at fb.com> wrote:
>>
>>>On 4/7/16, 11:37 AM, "clattner at apple.com on behalf of Chris Lattner" <clattner at apple.com> wrote:
>>>
>>>>On Apr 5, 2016, at 4:03 PM, Eric Niebler via cfe-dev <cfe-dev at lists.llvm.org> wrote:
>>>>
>>>>> Hi all,
>>>>
>>>>> I have an initial cut at patch that issues a warning when a file is #included
>>>>> on a case-insensitive file system that would not be found on a case-sensitive
>>>>> system. Is there interest?
>>>>
>>>>> Since this is a hard problem to solve perfectly, I have opted for a strategy
>>>>> that is efficient and conservative about issuing diagnostics. That is, it
>>>>> shouldn't give false positives, but it will fail to diagnose some non-portable
>>>>> paths. On *nix systems, the low-level APIs that stat and open files get an
>>>>> extra call to ::realpath, and the result is cached along with the rest of the
>>>>> file metadata. On Windows, I use a combination of GetFullPathName and
>>>>> GetLongPathName to get the same effect. (I don't believe that's guaranteed to
>>>>> get the physical name including case, but it seems to mostly work in my
>>>>> testing.)
>>>>>
>>>>> Due to how I compare path components, a relative path like
>>>>> "NoTtHeRiGhTcAsE/../correctly-cased.h" will not be diagnosed, but
>>>>> "../NoTtHeRiGhTcAsE/correctly-cased.h" will be. Catching more cases requires
>>>>> many more round trips to the disk, which I wanted to avoid.
>>>>
>>>>Hi Eric,
>>>>
>>>>This would be a hugely welcomed feature, but have you done any performance analysis of this?  The preprocessor and the data structures you are touching are very sensitive.
>>>>
>>>>You can stress test the preprocessor by using the "clang -cc1 -Eonly” mode.  If you’re on a mac, I’d recommend timing <Cocoa/Cocoa.h>
>>>
>>>Thanks for the suggestion, Chris. Looks like I've regressed the speed of the preprocessor by 25%. Yikes. I'll see what I can do.
>>>
>>>Eric
>>
>>
>>I have reimplemented this feature with an eye to performance. I now have it down to 2-3% perf regression in the preprocessor, which I hope is acceptable. I have benchmarked perf on MacOS (with Cocoa.h), Linux (with all of Boost.Spirit), and Windows (with Windows.h). For the record, Windows.h is terrible about properly casing its #includes. :-)
>>
>>Incorrectly cased #includes are diagnosed on MacOS, Windows, and on Linux (for case-insensitive file systems). They are even diagnosed in Cygwin. They will not be diagnosed on *nix-like systems that don't have either F_GETPATH (like MacOS) or a mounted /proc filesystem.
>>
>>Comments welcome.
>>
>>Eric
>>
>
>
> Any thoughts about this patch? Are folks here still interested? (Sending updated patch with --ignore-space-change.)
>
> \e
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>



-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc