[clang-tools-extra] 524ed4b - [Serialization] Place command line defines in the correct file

[John Brawn via cfe-commits]

Author: John Brawn
Date: 2023-04-24T14:07:41+01:00
New Revision: 524ed4b1ba518d5dd5d67fb9593e0864b0e664a4

URL: https://github.com/llvm/llvm-project/commit/524ed4b1ba518d5dd5d67fb9593e0864b0e664a4
DIFF: https://github.com/llvm/llvm-project/commit/524ed4b1ba518d5dd5d67fb9593e0864b0e664a4.diff

LOG: [Serialization] Place command line defines in the correct file

Fix several problems related to serialization causing command line
defines to be reported as being built-in defines:
 * When serializing the <built-in> and <command line> files don't
   convert them into absolute paths.
 * When deserializing SM_SLOC_BUFFER_ENTRY we need to call
   setHasLineDirectives in the same way as we do for
   SM_SLOC_FILE_ENTRY.
 * When created suggested predefines based on the current command line
   options we need to add line markers in the same way that
   InitializePreprocessor does.
 * Adjust a place in clangd where it was implicitly relying on command
   line defines being treated as builtin.

Differential Revision: https://reviews.llvm.org/D144651

Added: 
    clang/test/PCH/macro-cmdline.c

Modified: 
    clang-tools-extra/clangd/index/SymbolCollector.cpp
    clang/docs/ReleaseNotes.rst
    clang/lib/Serialization/ASTReader.cpp
    clang/lib/Serialization/ASTWriter.cpp
    clang/test/PCH/ms-pch-macro.c

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 5c8112e60b224..a2f8cd2a0cdaf 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -687,8 +687,10 @@ bool SymbolCollector::handleMacroOccurrence(const IdentifierInfo *Name,
 
   const auto &SM = PP->getSourceManager();
   auto DefLoc = MI->getDefinitionLoc();
-  // Also avoid storing predefined macros like __DBL_MIN__.
+  // Also avoid storing macros that aren't defined in any file, i.e. predefined
+  // macros like __DBL_MIN__ and those defined on the command line.
   if (SM.isWrittenInBuiltinFile(DefLoc) ||
+      SM.isWrittenInCommandLineFile(DefLoc) ||
       Name->getName() == "__GCC_HAVE_DWARF2_CFI_ASM")
     return true;
 

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7beae03247796..a1bb925e8ae24 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -207,8 +207,8 @@ Improvements to Clang's diagnostics
 - Diagnostic notes and fix-its are now generated for ``ifunc``/``alias`` attributes
   which point to functions whose names are mangled.
 - Diagnostics relating to macros on the command line of a preprocessed assembly
-  file are now reported as coming from the file ``<command line>`` instead of
-  ``<built-in>``.
+  file or precompiled header are now reported as coming from the file
+  ``<command line>`` instead of ``<built-in>``.
 - Clang constexpr evaluator now provides a more concise diagnostic when calling
   function pointer that is known to be null.
 - Clang now avoids duplicate warnings on unreachable ``[[fallthrough]];`` statements

diff  --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index b27304deb33c9..098ce5314ab17 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -654,6 +654,10 @@ static bool checkPreprocessorOptions(
   SmallVector<StringRef, 4> ExistingMacroNames;
   collectMacroDefinitions(ExistingPPOpts, ExistingMacros, &ExistingMacroNames);
 
+  // Use a line marker to enter the <command line> file, as the defines and
+  // undefines here will have come from the command line.
+  SuggestedPredefines += "# 1 \"<command line>\" 1\n";
+
   for (unsigned I = 0, N = ExistingMacroNames.size(); I != N; ++I) {
     // Dig out the macro definition in the existing preprocessor options.
     StringRef MacroName = ExistingMacroNames[I];
@@ -713,6 +717,10 @@ static bool checkPreprocessorOptions(
     }
     return true;
   }
+
+  // Leave the <command line> file and return to <built-in>.
+  SuggestedPredefines += "# 1 \"<built-in>\" 2\n";
+
   if (Validation == OptionValidateStrictMatches) {
     // If strict matches are requested, don't tolerate any extra defines in
     // the AST file that are missing on the command line.
@@ -1579,8 +1587,13 @@ bool ASTReader::ReadSLocEntry(int ID) {
     auto Buffer = ReadBuffer(SLocEntryCursor, Name);
     if (!Buffer)
       return true;
-    SourceMgr.createFileID(std::move(Buffer), FileCharacter, ID,
-                           BaseOffset + Offset, IncludeLoc);
+    FileID FID = SourceMgr.createFileID(std::move(Buffer), FileCharacter, ID,
+                                        BaseOffset + Offset, IncludeLoc);
+    if (Record[3]) {
+      auto &FileInfo =
+          const_cast<SrcMgr::FileInfo &>(SourceMgr.getSLocEntry(FID).getFile());
+      FileInfo.setHasLineDirectives();
+    }
     break;
   }
 

diff  --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 7c62aac9901f1..3d738149febcb 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4441,6 +4441,11 @@ void ASTWriter::AddString(StringRef Str, RecordDataImpl &Record) {
 bool ASTWriter::PreparePathForOutput(SmallVectorImpl<char> &Path) {
   assert(Context && "should have context when outputting path");
 
+  // Leave special file names as they are.
+  StringRef PathStr(Path.data(), Path.size());
+  if (PathStr == "<built-in>" || PathStr == "<command line>")
+    return false;
+
   bool Changed =
       cleanPathForOutput(Context->getSourceManager().getFileManager(), Path);
 

diff  --git a/clang/test/PCH/macro-cmdline.c b/clang/test/PCH/macro-cmdline.c
new file mode 100644
index 0000000000000..c4647f5d4c92e
--- /dev/null
+++ b/clang/test/PCH/macro-cmdline.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -emit-pch -o %t1.pch -DMACRO1=1
+// RUN: %clang_cc1 -fsyntax-only %s -include-pch %t1.pch -DMACRO2=1 2>&1 | FileCheck %s
+
+#ifndef HEADER
+#define HEADER
+#else
+#define MACRO1 2
+// CHECK: macro-cmdline.c{{.*}}'MACRO1' macro redefined
+// CHECK: <command line>{{.*}}previous definition is here
+#define MACRO2 2
+// CHECK: macro-cmdline.c{{.*}}'MACRO2' macro redefined
+// CHECK: <command line>{{.*}}previous definition is here
+#endif

diff  --git a/clang/test/PCH/ms-pch-macro.c b/clang/test/PCH/ms-pch-macro.c
index 3a8052eb322e9..a512e66e24866 100644
--- a/clang/test/PCH/ms-pch-macro.c
+++ b/clang/test/PCH/ms-pch-macro.c
@@ -36,4 +36,4 @@ BAR bar = 17;
 // CHECK-FOO: definition of macro 'FOO' 
diff ers between the precompiled header ('1') and the command line ('blah')
 // CHECK-NOFOO: macro 'FOO' was defined in the precompiled header but undef'd on the command line
 
-// expected-warning at 1 {{definition of macro 'BAR' does not match definition in precompiled header}}
+// expected-warning at 2 {{definition of macro 'BAR' does not match definition in precompiled header}}