[cfe-dev] PCH without original files

[Argyrios Kyrtzidis]

On Jan 26, 2011, at 7:19 AM, Axel Naumann wrote:

> Hi,
> 
> line and column numbers are calculated from the MemoryBuffer - which we
> don't have if the file doesn't exist. Attached patch
> TextDiagnosticPrinter_invalid_PLoc.diff improves the situation a bit; in
> your example I now get:
> 
> /build/llvm/tmp/relopch/email/test.h (in PCH): warning: padding struct
> 'S' with 3 bytes to align 'y'
> 1 warning generated.
> 
> With this patch, a valid SourceLocation but missing PresumedLoc is not a
> reason anymore for swallowing the diagnostic.

Looks good.

> 
> The alternative would be to store the line+column number in the PCH and
> that's overkill - esp given that in a few days I plan to find the source
> files even if they are at a different location than the absolute path
> stored in the PCH.

Right.

> 
> Comments or should I commit?

Please commit but watch out for the 80 cols violations.
Also could you add a test case to make sure we don't squash errors/warnings ? Here's another test case:

$ cat test.h
template <typename T>
void tf() { T::foo(); }
$ cat test.cxx
void f() {
	tf<int>();
}
$ clang -cc1 -x c++ -emit-pch test.h -o test.h.pch
$ rm test.h
$ clang -cc1 -include-pch test.h.pch -emit-obj test.cxx
/Users/argiris/proj/llvm/src/tools/clang/test.h (in PCH): error: type 'int' cannot be used prior to '::' because it has no members
test.cxx:2:2: note: in instantiation of function template specialization 'tf<int>' requested here
        tf<int>();
        ^
1 error generated.


> 
> And then okay to commit the original patch? (Attached
> ASTReader_FileNotFound_v2.diff is updated to trunk.)

Looks good to me, thanks!

-Argiris

> 
> Cheers, Axel.
> 
> Argyrios Kyrtzidis wrote on 01/25/2011 07:59 PM:
>> On Jan 25, 2011, at 5:07 AM, Axel Naumann wrote:
>> 
>>> Hi,
>>> 
>>> Attached patch allows the ASTReader to use a PCH even if the original
>>> files (which are store as absolute file names or relative to sysroot)
>>> have been removed. Drawback: without these original files, diagnostics
>>> referring to source locations in the original files cannot show the
>>> source code:
>>> 
>>> $ cat test.h
>>> void f(){}
>>> $ cat test.cxx
>>> int f(){}
>>> $ clang -cc1 -x c++ -emit-pch test.h -o test.h.pch
>>> $ clang -cc1 -include-pch test.h.pch -emit-obj test.cxx
>>> test.cxx:1:5: error: functions that differ only in their return type
>>> cannot be overloaded
>>> int f(){}
>>>   ^
>>> /build/llvm/tmp/relopch/email/test.h:1:6: note: previous definition is here
>>> void f(){}
>>>    ^
>>> 1 error generated.
>>> $ rm test.h
>>> $ clang -cc1 -include-pch test.h.pch -emit-obj test.cxx
>>> test.cxx:1:5: error: functions that differ only in their return type
>>> cannot be overloaded
>>> int f(){}
>>>   ^
>>> 1 error generated.
>> 
>> Shouldn't we show the messages that occured, just not the source, e.g:
>> 
>> test.cxx:1:5: error: functions that differ only in their return type
>> cannot be overloaded
>> int f(){}
>>   ^
>> /build/llvm/tmp/relopch/email/test.h:1:6: note: previous definition is here
>> 1 error generated.
>> 
>> This what gcc does, and I think notes are generally informative enough to show.
>> You also squash some warnings, e.g:
>> 
>> $ cat test.h
>> struct S {
>> 	char x;
>> 	int y;
>> };
>> $ cat test.cxx
>> void qq(S*) {}
>> $ clang -cc1 -x c++ -emit-pch test.h -o test.h.pch
>> $ clang -cc1 -include-pch test.h.pch -emit-obj test.cxx -Wpadded
>> /Users/argiris/proj/llvm/src/tools/clang/test.h:3:6: warning: padding struct 'S' with 3 bytes to align 'y'
>>        int y;
>>            ^
>> 1 warning generated.
>> $ rm test.h
>> $ clang -cc1 -include-pch test.h.pch -emit-obj test.cxx -Wpadded
>> 1 warning generated.
>> 
>> 
>> -Argiris
>> 
>>> 
>>> make test is happy. Please let me know whether this is okay to commit.
>>> 
>>> As a next step, I want to implement header search in the ASTReader if
>>> the relocatable flag is set, and if the original file cannot be found.
>>> 
>>> Cheers, Axel.
>>> <ASTReader_FileNotFound.diff>_______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>> 
>> 
> <TextDiagnosticPrinter_invalid_PLoc.diff><ASTReader_FileNotFound_v2.diff>_______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev