PCH without original files

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.

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 (2.07 KB)

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

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.

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.

Comments or should I commit?

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

Cheers, Axel.

TextDiagnosticPrinter_invalid_PLoc.diff (7.18 KB)

ASTReader_FileNotFound_v2.diff (2.07 KB)

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

Should it be possible to generate equivalent source code from the AST?

  Tom

Hi,