[cfe-dev] New warning for mismatched include case

[Jason Haslam]

This updated patch adds a fixit and a test. The test fails on case-sensitive systems. Is it possible to test for an error on some platforms and a warning on others?

Jason

-------------- next part --------------
A non-text attachment was scrubbed...
Name: include_case.diff
Type: application/octet-stream
Size: 5409 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120723/63c6542b/attachment.obj>
-------------- next part --------------



On Jul 19, 2012, at 11:58 PM, Jason Haslam wrote:

> Okay, I added caching which improved performance some. I also changed it to only check in user code, which made the Cocoa.h test case moot. I've run the attached patch over real code from LLVM and I don't see any measurable performance difference. It still may be worth disabling the check on case-sensitive file systems, although I'm not entirely sure how to do that beyond "#if defined(__APPLE__) || defined(_WIN32)".
> 
> If this looks like the right direction then I'll try to address some of the other concerns.
> 
> Jason
> 
> <include_case.diff>
> 
> 
> On Jul 18, 2012, at 11:26 PM, Jason Haslam wrote:
> 
>> Yes, performance is definitely an issue. I see about 30% degradation on Cocoa.h with this naive implementation. That's probably not going to fly under any circumstance. I'll take a closer look...
>> 
>> Jason
>> 
>> 
>> On Jul 18, 2012, at 5:12 PM, Ted Kremenek wrote:
>> 
>>> Hi Jason,
>>> 
>>> Any idea what is the performance impact of this change?  My concern that this is potentially expensive, and we shouldn't be performing the check on a case-sensitve file system (if we can help it).  That said, many people have requested this kind of checking, so I'm happy to see you taking a stab at it.
>>> 
>>> Ted	
>>> 
>>> On Jul 18, 2012, at 3:09 PM, Jason Haslam <jason.haslam at gmail.com> wrote:
>>> 
>>>> The attached (incomplete proof-of-concept) patch provides a new warning that fires on case-insensitive file systems when the case of the file name in the #include directive doesn't match the case of the file name on disk. The idea is to avoid broken commits that fail to build on case-sensitive file systems. For example:
>>>> 
>>>> jason$ ls
>>>> test.cpp test.h
>>>> jason$ cat test.cpp
>>>> #include "Test.h"
>>>> jason$ clang++ -fsyntax-only test.cpp 
>>>> test.cpp:1:10: warning: include case does not match the case of the file on disk
>>>> #include "Test.h"
>>>>      ^
>>>> 1 warning generated.
>>>> 
>>>> What do you think? Is this desirable? Does the direction look right?
>>>> 
>>>> Jason
>>>> 
>>>> <include_case.diff>_______________________________________________
>>>> cfe-dev mailing list
>>>> cfe-dev at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>> 
>> 
>