From f2631ad031d936fb2a2e2ceeec39ff870bd9f6c8 Mon Sep 17 00:00:00 2001 From: Erik Verbruggen Date: Thu, 30 May 2013 15:03:54 +0200 Subject: [PATCH] C++: do not strip trailing newlines in the preprocessor output. Doing so resulted in an incorrect position for the EOF token when the preprocessed output would be parsed. That in turn leads to incorrect insertion positions for refactoring actions. This is especially true when a file contains only preprocessor directives: the EOF token would point to line 1 column 1, which is usually not the place where code should be inserted. Change-Id: I7d359aa7a6c04bc52c8b873fd49ad6afc3a77319 Reviewed-by: hjk --- src/libs/cplusplus/pp-engine.cpp | 15 ---- src/libs/cplusplus/pp-engine.h | 1 - src/plugins/cppeditor/cppquickfix_test.cpp | 20 ++--- .../preprocessor/data/empty-macro.2.cpp | 1 - .../data/identifier-expansion.5.out.cpp | 8 ++ .../preprocessor/data/macro-test.out.cpp | 5 ++ .../preprocessor/tst_preprocessor.cpp | 80 ++++++++++++++++++- 7 files changed, 101 insertions(+), 29 deletions(-) diff --git a/src/libs/cplusplus/pp-engine.cpp b/src/libs/cplusplus/pp-engine.cpp index 91aa73efb9b..77ff055b1bc 100644 --- a/src/libs/cplusplus/pp-engine.cpp +++ b/src/libs/cplusplus/pp-engine.cpp @@ -1332,19 +1332,6 @@ void Preprocessor::synchronizeOutputLines(const PPToken &tk, bool forceLine) adjustForCommentOrStringNewlines(&m_env->currentLine, tk); } -void Preprocessor::removeTrailingOutputLines() -{ - QByteArray &buffer = currentOutputBuffer(); - int i = buffer.size() - 1; - while (i >= 0 && buffer.at(i) == '\n') - --i; - const int mightChop = buffer.size() - i - 1; - if (mightChop > 1) { - // Keep one new line at end. - buffer.chop(mightChop - 1); - } -} - std::size_t Preprocessor::computeDistance(const Preprocessor::PPToken &tk, bool forceTillLine) { // Find previous non-space character or line begin. @@ -1450,8 +1437,6 @@ void Preprocessor::preprocess(const QString &fileName, const QByteArray &source, } while (tk.isNot(T_EOF_SYMBOL)); - removeTrailingOutputLines(); - if (includeGuardMacroName) { if (m_state.m_includeGuardState == State::IncludeGuardState_AfterDefine || m_state.m_includeGuardState == State::IncludeGuardState_AfterEndif) diff --git a/src/libs/cplusplus/pp-engine.h b/src/libs/cplusplus/pp-engine.h index f118a00da40..2614c107b15 100644 --- a/src/libs/cplusplus/pp-engine.h +++ b/src/libs/cplusplus/pp-engine.h @@ -237,7 +237,6 @@ private: void maybeStartOutputLine(); void generateOutputLineMarker(unsigned lineno); void synchronizeOutputLines(const PPToken &tk, bool forceLine = false); - void removeTrailingOutputLines(); void enforceSpacing(const PPToken &tk, bool forceSpacing = false); static std::size_t computeDistance(const PPToken &tk, bool forceTillLine = false); diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp index 17823ecadef..74e64c8766e 100644 --- a/src/plugins/cppeditor/cppquickfix_test.cpp +++ b/src/plugins/cppeditor/cppquickfix_test.cpp @@ -842,17 +842,17 @@ void CppEditorPlugin::test_quickfix_InsertDefFromDecl_headerSource_namespace2() // Source File original = - "#include \"file.h\"\n" - "using namespace N;\n" - "\n" - ; + "#include \"file.h\"\n" + "using namespace N;\n" + ; expected = original + - "\n" - "Foo::Foo()\n" - "{\n\n" - "}\n" - "\n" - ; + "\n" + "\n" + "Foo::Foo()\n" + "{\n\n" + "}\n" + "\n" + ; testFiles << TestDocument::create(original, expected, QLatin1String("file.cpp")); InsertDefFromDecl factory; diff --git a/tests/auto/cplusplus/preprocessor/data/empty-macro.2.cpp b/tests/auto/cplusplus/preprocessor/data/empty-macro.2.cpp index 389eef523ca..8afb10a6cb3 100644 --- a/tests/auto/cplusplus/preprocessor/data/empty-macro.2.cpp +++ b/tests/auto/cplusplus/preprocessor/data/empty-macro.2.cpp @@ -10,4 +10,3 @@ private: public: Test(); }; - diff --git a/tests/auto/cplusplus/preprocessor/data/identifier-expansion.5.out.cpp b/tests/auto/cplusplus/preprocessor/data/identifier-expansion.5.out.cpp index 3d328fe2513..bdae3153b59 100644 --- a/tests/auto/cplusplus/preprocessor/data/identifier-expansion.5.out.cpp +++ b/tests/auto/cplusplus/preprocessor/data/identifier-expansion.5.out.cpp @@ -1 +1,9 @@ # 1 "data/identifier-expansion.5.cpp" + + + + + + + + diff --git a/tests/auto/cplusplus/preprocessor/data/macro-test.out.cpp b/tests/auto/cplusplus/preprocessor/data/macro-test.out.cpp index d13e2c5b0b3..fd938150162 100644 --- a/tests/auto/cplusplus/preprocessor/data/macro-test.out.cpp +++ b/tests/auto/cplusplus/preprocessor/data/macro-test.out.cpp @@ -10,3 +10,8 @@ void thisFunctionIsEnabled(); void thisFunctionIsEnabled2(); # 31 "data/macro-test.cpp" void thisFunctionIsEnabled3(); + + + + + diff --git a/tests/auto/cplusplus/preprocessor/tst_preprocessor.cpp b/tests/auto/cplusplus/preprocessor/tst_preprocessor.cpp index 7a4db38b60a..bb08b578aea 100644 --- a/tests/auto/cplusplus/preprocessor/tst_preprocessor.cpp +++ b/tests/auto/cplusplus/preprocessor/tst_preprocessor.cpp @@ -353,6 +353,8 @@ private slots: void skip_unknown_directives_data(); void include_guard(); void include_guard_data(); + void empty_trailing_lines(); + void empty_trailing_lines_data(); }; // Remove all #... lines, and 'simplify' string, to allow easily comparing the result @@ -1507,7 +1509,13 @@ void tst_Preprocessor::skip_unknown_directives_data() "# 10 \"file.cpp\"\n" "# ()\n" "#\n"; - expected = "# 1 \"\"\n"; + expected = + "# 1 \"\"\n" + "\n" + "\n" + "\n" + "\n" + ; QTest::newRow("case 1") << original << expected; } @@ -1593,6 +1601,74 @@ void tst_Preprocessor::include_guard_data() ; } +void tst_Preprocessor::empty_trailing_lines() +{ + compare_input_output(); +} + +void tst_Preprocessor::empty_trailing_lines_data() +{ + // Test if the number of lines at the end of a file is correct. This is important to make the + // EOF token for the end up at the correct line. + + QTest::addColumn("input"); + QTest::addColumn("output"); + + QByteArray original; + QByteArray expected; + + original = + "\n" + "\n" + "\n" + "\n" + "\n" + "\n" + "\n" + "\n" + ; + expected = "# 1 \"\"\n" + original; + QTest::newRow("9 empty lines") << original << expected; + + original = + "\n" + "\n" + "\n" + "\n" + "\n" + "\n" + "\n" + "\n" + "\n" + "\n" + ; + expected = + "# 1 \"\"\n" + "# 11 \"\"\n" + ; + QTest::newRow("11 empty lines") << original << expected; + + original = + "#include \n" + ; + expected = + "# 1 \"\"\n" + "\n" + ; + QTest::newRow("1 include") << original << expected; + + original = + "#include \n" + "\n" + ; + expected = + "# 1 \"\"\n" + "\n" + "\n" + ; + QTest::newRow("1 empty line with 1 include") << original << expected; +} + void tst_Preprocessor::compare_input_output(bool keepComments) { QFETCH(QByteArray, input); @@ -1602,7 +1678,7 @@ void tst_Preprocessor::compare_input_output(bool keepComments) Preprocessor preprocess(0, &env); preprocess.setKeepComments(keepComments); QByteArray prep = preprocess.run(QLatin1String(""), input); - QCOMPARE(output, prep); + QCOMPARE(prep.constData(), output.constData()); } QTEST_APPLESS_MAIN(tst_Preprocessor)