[clang] 3b1d455 - [clang] Correct -frewrite-includes generation of line control directives with mixed EOL forms.

Tom Honermann via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 5 10:24:12 PST 2023


Author: Tom Honermann
Date: 2023-01-05T13:24:01-05:00
New Revision: 3b1d45518910b529104a269d5f6712a720cc3ff7

URL: https://github.com/llvm/llvm-project/commit/3b1d45518910b529104a269d5f6712a720cc3ff7
DIFF: https://github.com/llvm/llvm-project/commit/3b1d45518910b529104a269d5f6712a720cc3ff7.diff

LOG: [clang] Correct -frewrite-includes generation of line control directives with mixed EOL forms.

Previously, if a header file and a source file used different end of line
(EOL) forms, preprocessed output generated with the -frewrite-includes option
would, in some cases, generate line control directives with the wrong line
number due to an error in how source file lines were counted.

Fixes https://github.com/llvm/llvm-project/issues/59736

Reviewed By: cor3ntin

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

Added: 
    clang/test/Frontend/rewrite-includes-mixed-eol-crlf.c
    clang/test/Frontend/rewrite-includes-mixed-eol-crlf.h
    clang/test/Frontend/rewrite-includes-mixed-eol-lf.c
    clang/test/Frontend/rewrite-includes-mixed-eol-lf.h

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Frontend/Rewrite/InclusionRewriter.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b308dd6c87860..ba464df6c0d3b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -331,6 +331,10 @@ Bug Fixes
   `Issue 58800 <https://github.com/llvm/llvm-project/issues/58800>`_
 - Fix an issue that triggers a crash if we instantiate a hidden friend functions.
   This fixes `Issue 54457 <https://github.com/llvm/llvm-project/issues/54457>`_
+- Fix an issue where -frewrite-includes generated line control directives with
+  incorrect line numbers in some cases when a header file used an end of line
+  character sequence that 
diff ered from the primary source file.
+  `Issue 59736 <https://github.com/llvm/llvm-project/issues/59736>`_
 
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

diff  --git a/clang/lib/Frontend/Rewrite/InclusionRewriter.cpp b/clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
index 810ec680448c5..d3a3db0139c6d 100644
--- a/clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
+++ b/clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
@@ -281,27 +281,33 @@ void InclusionRewriter::OutputContentUpTo(const MemoryBufferRef &FromFile,
 
   StringRef TextToWrite(FromFile.getBufferStart() + WriteFrom,
                         WriteTo - WriteFrom);
+  // count lines manually, it's faster than getPresumedLoc()
+  Line += TextToWrite.count(LocalEOL);
 
   if (MainEOL == LocalEOL) {
     OS << TextToWrite;
-    // count lines manually, it's faster than getPresumedLoc()
-    Line += TextToWrite.count(LocalEOL);
-    if (EnsureNewline && !TextToWrite.endswith(LocalEOL))
-      OS << MainEOL;
   } else {
     // Output the file one line at a time, rewriting the line endings as we go.
     StringRef Rest = TextToWrite;
     while (!Rest.empty()) {
-      StringRef LineText;
-      std::tie(LineText, Rest) = Rest.split(LocalEOL);
+      // Identify and output the next line excluding an EOL sequence if present.
+      size_t Idx = Rest.find(LocalEOL);
+      StringRef LineText = Rest.substr(0, Idx);
       OS << LineText;
-      Line++;
-      if (!Rest.empty())
+      if (Idx != StringRef::npos) {
+        // An EOL sequence was present, output the EOL sequence for the
+        // main source file and skip past the local EOL sequence.
         OS << MainEOL;
+        Idx += LocalEOL.size();
+      }
+      // Strip the line just handled. If Idx is npos or matches the end of the
+      // text, Rest will be set to an empty string and the loop will terminate.
+      Rest = Rest.substr(Idx);
     }
-    if (TextToWrite.endswith(LocalEOL) || EnsureNewline)
-      OS << MainEOL;
   }
+  if (EnsureNewline && !TextToWrite.endswith(LocalEOL))
+    OS << MainEOL;
+
   WriteFrom = WriteTo;
 }
 

diff  --git a/clang/test/Frontend/rewrite-includes-mixed-eol-crlf.c b/clang/test/Frontend/rewrite-includes-mixed-eol-crlf.c
new file mode 100644
index 0000000000000..d6724444c0667
--- /dev/null
+++ b/clang/test/Frontend/rewrite-includes-mixed-eol-crlf.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -E -frewrite-includes %s | %clang_cc1 -
+// expected-no-diagnostics
+// Note: This source file has CRLF line endings.
+// This test validates that -frewrite-includes translates the end of line (EOL)
+// form used in header files to the EOL form used in the the primary source
+// file when the files use 
diff erent EOL forms.
+#include "rewrite-includes-mixed-eol-crlf.h"
+#include "rewrite-includes-mixed-eol-lf.h"

diff  --git a/clang/test/Frontend/rewrite-includes-mixed-eol-crlf.h b/clang/test/Frontend/rewrite-includes-mixed-eol-crlf.h
new file mode 100644
index 0000000000000..0439b88b75e2c
--- /dev/null
+++ b/clang/test/Frontend/rewrite-includes-mixed-eol-crlf.h
@@ -0,0 +1,11 @@
+// Note: This header file has CRLF line endings.
+// The indentation in some of the conditional inclusion directives below is
+// intentional and is required for this test to function as a regression test
+// for GH59736.
+_Static_assert(__LINE__ == 5, "");
+#if 1
+_Static_assert(__LINE__ == 7, "");
+  #if 1
+  _Static_assert(__LINE__ == 9, "");
+  #endif
+#endif

diff  --git a/clang/test/Frontend/rewrite-includes-mixed-eol-lf.c b/clang/test/Frontend/rewrite-includes-mixed-eol-lf.c
new file mode 100644
index 0000000000000..24d61fea33188
--- /dev/null
+++ b/clang/test/Frontend/rewrite-includes-mixed-eol-lf.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -E -frewrite-includes %s | %clang_cc1 -
+// expected-no-diagnostics
+// Note: This source file has LF line endings.
+// This test validates that -frewrite-includes translates the end of line (EOL)
+// form used in header files to the EOL form used in the the primary source
+// file when the files use 
diff erent EOL forms.
+#include "rewrite-includes-mixed-eol-crlf.h"
+#include "rewrite-includes-mixed-eol-lf.h"

diff  --git a/clang/test/Frontend/rewrite-includes-mixed-eol-lf.h b/clang/test/Frontend/rewrite-includes-mixed-eol-lf.h
new file mode 100644
index 0000000000000..e103c959676d6
--- /dev/null
+++ b/clang/test/Frontend/rewrite-includes-mixed-eol-lf.h
@@ -0,0 +1,11 @@
+// Note: This header file has LF line endings.
+// The indentation in some of the conditional inclusion directives below is
+// intentional and is required for this test to function as a regression test
+// for GH59736.
+_Static_assert(__LINE__ == 5, "");
+#if 1
+_Static_assert(__LINE__ == 7, "");
+  #if 1
+  _Static_assert(__LINE__ == 9, "");
+  #endif
+#endif


        


More information about the cfe-commits mailing list