Skip to content

Commit 3b1d455

Browse files
committed
[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 llvm#59736 Reviewed By: cor3ntin Differential Revision: https://reviews.llvm.org/D140984
1 parent 8b5d036 commit 3b1d455

File tree

6 files changed

+58
-10
lines changed

6 files changed

+58
-10
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,10 @@ Bug Fixes
331331
`Issue 58800 <https://github.com/llvm/llvm-project/issues/58800>`_
332332
- Fix an issue that triggers a crash if we instantiate a hidden friend functions.
333333
This fixes `Issue 54457 <https://github.com/llvm/llvm-project/issues/54457>`_
334+
- Fix an issue where -frewrite-includes generated line control directives with
335+
incorrect line numbers in some cases when a header file used an end of line
336+
character sequence that differed from the primary source file.
337+
`Issue 59736 <https://github.com/llvm/llvm-project/issues/59736>`_
334338

335339
Improvements to Clang's diagnostics
336340
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

clang/lib/Frontend/Rewrite/InclusionRewriter.cpp

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -281,27 +281,33 @@ void InclusionRewriter::OutputContentUpTo(const MemoryBufferRef &FromFile,
281281

282282
StringRef TextToWrite(FromFile.getBufferStart() + WriteFrom,
283283
WriteTo - WriteFrom);
284+
// count lines manually, it's faster than getPresumedLoc()
285+
Line += TextToWrite.count(LocalEOL);
284286

285287
if (MainEOL == LocalEOL) {
286288
OS << TextToWrite;
287-
// count lines manually, it's faster than getPresumedLoc()
288-
Line += TextToWrite.count(LocalEOL);
289-
if (EnsureNewline && !TextToWrite.endswith(LocalEOL))
290-
OS << MainEOL;
291289
} else {
292290
// Output the file one line at a time, rewriting the line endings as we go.
293291
StringRef Rest = TextToWrite;
294292
while (!Rest.empty()) {
295-
StringRef LineText;
296-
std::tie(LineText, Rest) = Rest.split(LocalEOL);
293+
// Identify and output the next line excluding an EOL sequence if present.
294+
size_t Idx = Rest.find(LocalEOL);
295+
StringRef LineText = Rest.substr(0, Idx);
297296
OS << LineText;
298-
Line++;
299-
if (!Rest.empty())
297+
if (Idx != StringRef::npos) {
298+
// An EOL sequence was present, output the EOL sequence for the
299+
// main source file and skip past the local EOL sequence.
300300
OS << MainEOL;
301+
Idx += LocalEOL.size();
302+
}
303+
// Strip the line just handled. If Idx is npos or matches the end of the
304+
// text, Rest will be set to an empty string and the loop will terminate.
305+
Rest = Rest.substr(Idx);
301306
}
302-
if (TextToWrite.endswith(LocalEOL) || EnsureNewline)
303-
OS << MainEOL;
304307
}
308+
if (EnsureNewline && !TextToWrite.endswith(LocalEOL))
309+
OS << MainEOL;
310+
305311
WriteFrom = WriteTo;
306312
}
307313

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// RUN: %clang_cc1 -E -frewrite-includes %s | %clang_cc1 -
2+
// expected-no-diagnostics
3+
// Note: This source file has CRLF line endings.
4+
// This test validates that -frewrite-includes translates the end of line (EOL)
5+
// form used in header files to the EOL form used in the the primary source
6+
// file when the files use different EOL forms.
7+
#include "rewrite-includes-mixed-eol-crlf.h"
8+
#include "rewrite-includes-mixed-eol-lf.h"
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Note: This header file has CRLF line endings.
2+
// The indentation in some of the conditional inclusion directives below is
3+
// intentional and is required for this test to function as a regression test
4+
// for GH59736.
5+
_Static_assert(__LINE__ == 5, "");
6+
#if 1
7+
_Static_assert(__LINE__ == 7, "");
8+
#if 1
9+
_Static_assert(__LINE__ == 9, "");
10+
#endif
11+
#endif
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// RUN: %clang_cc1 -E -frewrite-includes %s | %clang_cc1 -
2+
// expected-no-diagnostics
3+
// Note: This source file has LF line endings.
4+
// This test validates that -frewrite-includes translates the end of line (EOL)
5+
// form used in header files to the EOL form used in the the primary source
6+
// file when the files use different EOL forms.
7+
#include "rewrite-includes-mixed-eol-crlf.h"
8+
#include "rewrite-includes-mixed-eol-lf.h"
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Note: This header file has LF line endings.
2+
// The indentation in some of the conditional inclusion directives below is
3+
// intentional and is required for this test to function as a regression test
4+
// for GH59736.
5+
_Static_assert(__LINE__ == 5, "");
6+
#if 1
7+
_Static_assert(__LINE__ == 7, "");
8+
#if 1
9+
_Static_assert(__LINE__ == 9, "");
10+
#endif
11+
#endif

0 commit comments

Comments
 (0)