[clang] [clang] Store the Input PCH Path Using Absolute Paths in a PCH (PR #178781)

[Qiongsi Wu via cfe-commits]

================
@@ -1752,7 +1752,10 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) {
   Record.push_back(PPOpts.UsePredefines);
   // Detailed record is important since it is used for the module cache hash.
   Record.push_back(PPOpts.DetailedRecord);
-  AddString(PPOpts.ImplicitPCHInclude, Record);
+  if (PPOpts.ImplicitPCHInclude.empty())
+    AddString(PPOpts.ImplicitPCHInclude, Record);
----------------
qiongsiwu wrote:

Adding an empty string to the record is not an no-op. We need to serialize this into a record and the AST reader can read it back (https://github.com/llvm/llvm-project/blob/e438a903dbf2dd256ba2b4097a15ed8e9e0ede57/clang/lib/Serialization/ASTReader.cpp#L6707) and set `PPOpts.ImplicitPCHInclude` correctly, even if the string is empty. I did a sanity check to not add the string when the option is empty, and that led to 1786 failed clang tests. We basically broke PCH.

I made a deliberate choice to not change `AddPath` for two reasons: 
1. It is not quite clear how empty strings should be handled by default. It is not true that we can simply add an empty string record when the input is empty. See https://github.com/llvm/llvm-project/blob/3d4f6b329cccc5abb06c1bd228baae9dc1eb7227/clang/lib/Serialization/ASTWriter.cpp#L1540 for an example. 
2. Other existing call sites do not need to worry about the empty input case, if I modify `AddPath`, we are checking for something that is not likely to happen for the more common cases. 

Let me know if you disagree with my reasoning and I will add the logic to handle an empty path to `AddPath`. A reasonable behavior is to add an empty string to the record and early exit. 

https://github.com/llvm/llvm-project/pull/178781