[clang-repl] Create virtual files for input_line_N buffers

[devajithvs]

Instead of using memory buffers without file backing, this patch input_line_N buffers as virtual files.

This patch enables us to use input line numbers when verifying tests clang-repl.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Devajith (devajithvs)

Changes

In clang-repl, input buffers (e.g., input_line_1) are virtual and not tracked by the FileManager. This patch enables us to use input line numbers when verifying tests in clang-repl.

---

Full diff: https://github.com/llvm/llvm-project/pull/182044.diff

2 Files Affected:

• (modified) clang/lib/Frontend/VerifyDiagnosticConsumer.cpp (+39-7)
• (added) clang/test/Interpreter/verify-diagnostics.cpp (+13)

diff --git a/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp b/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
index e263e10bb065a..73abb538f35e0 100644
--- a/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
+++ b/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
@@ -556,17 +556,41 @@ static bool ParseDirective(StringRef S, ExpectedData *ED, SourceManager &SM,
           OptionalFileEntryRef File =
               PP->LookupFile(Pos, Filename, false, nullptr, nullptr, nullptr,
                              nullptr, nullptr, nullptr, nullptr, nullptr);
-          if (!File) {
+
+          FileID FID;
+          if (File) {
+            FID = SM.translateFile(*File);
+            if (FID.isInvalid()) {
+              FID = SM.createFileID(*File, Pos, SrcMgr::C_User);
+            }
+          } else if (Filename.starts_with("input_line_")) {
+            // clang-repl buffers (e.g., input_line_1) are virtual and not
+            // tracked by the FileManager. We need to manually scan the
+            // SourceManager's local SLocEntry table to find the buffer matching
+            // this name.
+            for (unsigned i = 0; i < SM.local_sloc_entry_size(); ++i) {
+              const SrcMgr::SLocEntry &Entry = SM.getLocalSLocEntry(i);
+              if (!Entry.isFile())
+                continue;
+              if (auto Buffer =
+                      Entry.getFile().getContentCache().getBufferIfLoaded()) {
+                if (Buffer->getBufferIdentifier() == Filename) {
+                  SourceLocation EntryLoc =
+                      SourceLocation::getFromRawEncoding(Entry.getOffset());
+                  FID = SM.getFileID(EntryLoc);
+                  break;
+                }
+              }
+            }
+          }
+
+          if (FID.isInvalid()) {
             Diags.Report(Pos.getLocWithOffset(PH.C - PH.Begin),
                          diag::err_verify_missing_file)
                 << Filename << KindStr;
             continue;
           }
 
-          FileID FID = SM.translateFile(*File);
-          if (FID.isInvalid())
-            FID = SM.createFileID(*File, Pos, SrcMgr::C_User);
-
           if (PH.Next(Line) && Line > 0)
             ExpectedLoc = SM.translateLineCol(FID, Line, 1);
           else if (PH.Next("*")) {
@@ -949,11 +973,19 @@ static bool IsFromSameFile(SourceManager &SM, SourceLocation DirectiveLoc,
   if (SM.isWrittenInSameFile(DirectiveLoc, DiagnosticLoc))
     return true;
 
-  const FileEntry *DiagFile = SM.getFileEntryForID(SM.getFileID(DiagnosticLoc));
+  FileID DiagFID = SM.getFileID(DiagnosticLoc);
+  FileID DirFID = SM.getFileID(DirectiveLoc);
+
+  // If they are the exact same buffer, we're done.
+  if (DiagFID == DirFID)
+    return true;
+
+  const FileEntry *DiagFile = SM.getFileEntryForID(DiagFID);
+
   if (!DiagFile && SM.isWrittenInMainFile(DirectiveLoc))
     return true;
 
-  return (DiagFile == SM.getFileEntryForID(SM.getFileID(DirectiveLoc)));
+  return DiagFile == SM.getFileEntryForID(DirFID);
 }
 
 /// CheckLists - Compare expected to seen diagnostic lists and return the
diff --git a/clang/test/Interpreter/verify-diagnostics.cpp b/clang/test/Interpreter/verify-diagnostics.cpp
new file mode 100644
index 0000000000000..f64655575314c
--- /dev/null
+++ b/clang/test/Interpreter/verify-diagnostics.cpp
@@ -0,0 +1,13 @@
+// REQUIRES: host-supports-jit
+// RUN: cat %s | clang-repl -Xcc -Xclang -Xcc -verify
+
+auto x = unknown_val;
+// expected-error@input_line_3:1 {{use of undeclared identifier 'unknown_val'}}
+
+int get_int() { return 42; }
+char* ptr = get_int();
+// expected-error@input_line_7:1 {{cannot initialize a variable of type 'char *' with an rvalue of type 'int'}}
+
+// Verify without input_line_*
+int y = ;
+// expected-error {{expected expression}}

[devajithvs]

CC: @vgvassilev
This will enable us use -verify with {input_line*} like in cling.

[vgvassilev]

Looks reasonable to me but I'd like some more input from Ian and Aaron.

[jansvoboda11]

Instead of increasing complexity in Clang, why doesn't clang-repl register these as virtual files in FileManager so that they don't require special-casing here?

[devajithvs]

why doesn't clang-repl register these as virtual files in FileManager so that they don't require special-casing here?

Thanks @jansvoboda11. This is exactly what we do in cling.

But after 84df7a0, we need to ensure HeaderFileInfo exists before lookup to prevent assertion.

As a part of reducing our internal clang patches in cling, I thought I will adapt to what clang-repl does (without registering these as virtual files)

I have updated the PR and the description (we still need minor changes in VerifyDiagnosticConsumer.cpp ).

[jansvoboda11]

How come PP->LookupFile() can't find virtual files? I think it should just work, but maybe we need to provide the Includers argument? Or could we make the virtual file paths absolute by prefixing /?

[devajithvs]

Yes, that's the problem. Easiest way to fix this would be to prepend incremental parsing source files with ./ or /.

But that would mean all the warnings/errors will have that prefix.

In file included from <<< inputs >>>:1:                                                                                                                                                                                                                       
./input_line_4:1:10: error: use of undeclared identifier 'unknown_val'                                                                                                                                                                                          
    1 | auto x = unknown_val;                                                                                                                                                                                                                                 
      |          ^~~~~~~~~~~  

and we do:

// expected-error@./input_line_4:1 {{use of undeclared identifier 'unknown_val'}}

Which looks a bit weird (and will need to update the test-cases in clang-repl with this change)
CC: @vgvassilev

[vgvassilev]

Yeah, it'd be great if we can avoid that prefix..

[devajithvs]

We could also have something like this in HeaderSearch, but I don't know if that is ideal:

diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 5f52d62bd36e..795e91673de3 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1021,9 +1021,20 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
         assert(HFI && "includer without file info");
         return HFI->DirInfo != SrcMgr::C_User;
       }();
-      if (OptionalFileEntryRef FE = getFileAndSuggestModule(
-              TmpDir, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader,
-              RequestingModule, SuggestedModule)) {
+
+      OptionalFileEntryRef FE = getFileAndSuggestModule(
+          TmpDir, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader,
+          RequestingModule, SuggestedModule);
+
+      // Check if it is a virtual file from incremental buffer
+      if (!FE && IncluderAndDir.second.getName() == "." &&
+          Filename.starts_with("input_line_")) {
+        FE = getFileAndSuggestModule(
+            Filename, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader,
+            RequestingModule, SuggestedModule);
+      }
+
+      if (FE) {
         diagnoseHeaderShadowing(Filename, FE, DiagnosedShadowing, IncludeLoc,
                                 FromDir, Includers, isAngled,
                                 &IncluderAndDir - Includers.begin(), nullptr);

[devajithvs]

I've added the check isIncrementalProcessingEnabled() to make this specific to incremental inputs. Closing: #182070 as these changes will fix the issue there.

[jansvoboda11]

LGTM

[vgvassilev]

/cherry-pick 9cc0df9

[devajithvs]

Thanks @vgvassilev

[llvmbot]

/pull-request #184600