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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits