From 9bdf8329a0c0cac21e699ab842e4d71900fff940 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Wed, 29 May 2024 14:33:04 +0200 Subject: [PATCH] CppEditor: Consider unexpanded token ranges when refactoring When moving or deleting pieces of code, we are normally interested in the actual content of the file in the respective locations, not in what any macros present there will expand to. This supersedes commit 76ae5780c4ada9a44e35103fcc1247ff2b7fc53d. Fixes: QTCREATORBUG-10279 Change-Id: I0fb547b23244cd5875e80c019a3595f3f9c33d52 Reviewed-by: Christian Stenger --- .../3rdparty/cplusplus/TranslationUnit.cpp | 28 ++++++--- src/libs/3rdparty/cplusplus/TranslationUnit.h | 6 ++ .../cppeditor/cpprefactoringchanges.cpp | 28 ++++++--- src/plugins/cppeditor/cpprefactoringchanges.h | 5 +- .../quickfixes/logicaloperationquickfixes.cpp | 60 ++++++++++++++++++- 5 files changed, 108 insertions(+), 19 deletions(-) diff --git a/src/libs/3rdparty/cplusplus/TranslationUnit.cpp b/src/libs/3rdparty/cplusplus/TranslationUnit.cpp index e680ee2660e..8505deb0bdc 100644 --- a/src/libs/3rdparty/cplusplus/TranslationUnit.cpp +++ b/src/libs/3rdparty/cplusplus/TranslationUnit.cpp @@ -27,7 +27,7 @@ #include "Literals.h" #include "DiagnosticClient.h" -#include "cppassert.h" +#include #include #include @@ -88,7 +88,7 @@ int TranslationUnit::sourceLength() const void TranslationUnit::setSource(const char *source, int size) { - CPP_CHECK(source); + QTC_ASSERT(source, return); _firstSourceChar = source; _lastSourceChar = source + size; } @@ -191,6 +191,8 @@ void TranslationUnit::tokenize() int lineColumnIdx = 0; Token tk; + int macroOffset = -1; + int macroLength = -1; do { lex(&tk); @@ -209,17 +211,12 @@ recognize: lex(&tk); // Gather where the expansion happens and its length. - //int macroOffset = static_cast(strtoul(tk.spell(), 0, 0)); + macroOffset = static_cast(strtoul(tk.spell(), 0, 0)); lex(&tk); lex(&tk); // Skip the separating comma - //int macroLength = static_cast(strtoul(tk.spell(), 0, 0)); + macroLength = static_cast(strtoul(tk.spell(), 0, 0)); lex(&tk); - // NOTE: We are currently not using the macro offset and length. They - // are kept here for now because of future use. - //Q_UNUSED(macroOffset) - //Q_UNUSED(macroLength) - // Now we need to gather the real line and columns from the upcoming // tokens. But notice this is only relevant for tokens which are expanded // but not generated. @@ -307,6 +304,11 @@ recognize: tk.f.generated = currentGenerated; _tokens->push_back(tk); + + if (currentExpanded) { + QTC_ASSERT(macroOffset != -1 && macroLength != -1, continue); + _expansionPositions[_tokens->size() - 1] = std::make_pair(macroOffset, macroLength); + } } while (tk.kind()); for (; ! braces.empty(); braces.pop()) { @@ -462,6 +464,14 @@ int TranslationUnit::getTokenEndPositionInDocument(const Token &token, return Utils::Text::positionInText(doc, line, column); } +std::pair TranslationUnit::getExpansionPosition(int tokenIndex) const +{ + QTC_ASSERT(tokenIndex < int(_tokens->size()) && tokenAt(tokenIndex).generated(), return {}); + const auto it = _expansionPositions.find(tokenIndex); + QTC_ASSERT(it != _expansionPositions.end(), return {}); + return it->second; +} + void TranslationUnit::getPosition(int utf16charOffset, int *line, int *column, diff --git a/src/libs/3rdparty/cplusplus/TranslationUnit.h b/src/libs/3rdparty/cplusplus/TranslationUnit.h index 40f79d0091d..1332549ffde 100644 --- a/src/libs/3rdparty/cplusplus/TranslationUnit.h +++ b/src/libs/3rdparty/cplusplus/TranslationUnit.h @@ -133,6 +133,7 @@ public: const StringLiteral **fileName = nullptr) const; int getTokenPositionInDocument(const Token token, const QTextDocument *doc) const; int getTokenEndPositionInDocument(const Token &token, const QTextDocument *doc) const; + std::pair getExpansionPosition(int tokenIndex) const; void pushLineOffset(int offset); void pushPreprocessorLine(int utf16charOffset, @@ -183,6 +184,11 @@ private: std::vector *_comments; std::vector _lineOffsets; std::vector _ppLines; + + // Offset and length. Note that in contrast to token offsets, this is a raw file offset + // with no preprocessor prefix. + std::unordered_map> _expansionPositions; + typedef std::unordered_map > TokenLineColumn; TokenLineColumn _expandedLineColumn; MemoryPool *_pool; diff --git a/src/plugins/cppeditor/cpprefactoringchanges.cpp b/src/plugins/cppeditor/cpprefactoringchanges.cpp index 3c979d9cc2b..d9e5a1c8831 100644 --- a/src/plugins/cppeditor/cpprefactoringchanges.cpp +++ b/src/plugins/cppeditor/cpprefactoringchanges.cpp @@ -221,6 +221,9 @@ ChangeSet::Range CppRefactoringFile::range(const AST *ast) const int CppRefactoringFile::startOf(unsigned index) const { + if (const auto loc = expansionLoc(index)) + return loc->first; + int line, column; cppDocument()->translationUnit()->getPosition(tokenAt(index).utf16charsBegin(), &line, &column); return document()->findBlockByNumber(line - 1).position() + column - 1; @@ -229,15 +232,14 @@ int CppRefactoringFile::startOf(unsigned index) const int CppRefactoringFile::startOf(const AST *ast) const { QTC_ASSERT(ast, return 0); - int firstToken = ast->firstToken(); - const int lastToken = ast->lastToken(); - while (tokenAt(firstToken).generated() && firstToken < lastToken) - ++firstToken; - return startOf(firstToken); + return startOf(ast->firstToken()); } int CppRefactoringFile::endOf(unsigned index) const { + if (const auto loc = expansionLoc(index)) + return loc->first + loc->second; + int line, column; cppDocument()->translationUnit()->getPosition(tokenAt(index).utf16charsEnd(), &line, &column); return document()->findBlockByNumber(line - 1).position() + column - 1; @@ -248,14 +250,17 @@ int CppRefactoringFile::endOf(const AST *ast) const QTC_ASSERT(ast, return 0); int lastToken = ast->lastToken() - 1; QTC_ASSERT(lastToken >= 0, return -1); - const int firstToken = ast->firstToken(); - while (tokenAt(lastToken).generated() && lastToken > firstToken) - --lastToken; return endOf(lastToken); } void CppRefactoringFile::startAndEndOf(unsigned index, int *start, int *end) const { + if (const auto loc = expansionLoc(index)) { + *start = loc->first; + *end = loc->first + loc->second; + return; + } + int line, column; Token token(tokenAt(index)); cppDocument()->translationUnit()->getPosition(token.utf16charsBegin(), &line, &column); @@ -263,6 +268,13 @@ void CppRefactoringFile::startAndEndOf(unsigned index, int *start, int *end) con *end = *start + token.utf16chars(); } +std::optional > CppRefactoringFile::expansionLoc(unsigned int index) const +{ + if (!tokenAt(index).expanded()) + return {}; + return cppDocument()->translationUnit()->getExpansionPosition(index); +} + QString CppRefactoringFile::textOf(const AST *ast) const { return textOf(startOf(ast), endOf(ast)); diff --git a/src/plugins/cppeditor/cpprefactoringchanges.h b/src/plugins/cppeditor/cpprefactoringchanges.h index 6350751a341..957d02674c7 100644 --- a/src/plugins/cppeditor/cpprefactoringchanges.h +++ b/src/plugins/cppeditor/cpprefactoringchanges.h @@ -11,6 +11,8 @@ #include +#include + namespace CppEditor { class CppRefactoringChanges; class CppRefactoringFile; @@ -30,7 +32,6 @@ public: bool isCursorOn(unsigned tokenIndex) const; bool isCursorOn(const CPlusPlus::AST *ast) const; - Range range(int start, int end) const; Range range(unsigned tokenIndex) const; Range range(const CPlusPlus::AST *ast) const; @@ -43,6 +44,8 @@ public: void startAndEndOf(unsigned index, int *start, int *end) const; + std::optional> expansionLoc(unsigned index) const; + QList tokensForCursor() const; QList tokensForCursor(const QTextCursor &cursor) const; QList tokensForLine(int line) const; diff --git a/src/plugins/cppeditor/quickfixes/logicaloperationquickfixes.cpp b/src/plugins/cppeditor/quickfixes/logicaloperationquickfixes.cpp index 6faed4b9f03..05ad7540743 100644 --- a/src/plugins/cppeditor/quickfixes/logicaloperationquickfixes.cpp +++ b/src/plugins/cppeditor/quickfixes/logicaloperationquickfixes.cpp @@ -7,6 +7,11 @@ #include "../cpprefactoringchanges.h" #include "cppquickfix.h" +#ifdef WITH_TESTS +#include "cppquickfix_test.h" +#include +#endif + using namespace CPlusPlus; using namespace Utils; @@ -149,7 +154,7 @@ class FlipLogicalOperands : public CppQuickFixFactory { #ifdef WITH_TESTS public: - static QObject *createTest() { return new QObject; } + static QObject *createTest(); #endif private: @@ -311,6 +316,55 @@ private: } }; +#ifdef WITH_TESTS +using namespace Tests; +class FlipLogicalOperandsTest : public QObject +{ + Q_OBJECT + +private slots: + void test_data() + { + QTest::addColumn("original"); + QTest::addColumn("expected"); + + const auto makeDoc = [](const QString &expr) { + const QString pattern = "#define VALUE 7\n" + "int main() {\n" + " if (%1)\n" + " return 1;\n" + "}\n"; + return pattern.arg(expr).toUtf8(); + }; + + QTest::newRow("macro as left expr") + << makeDoc("VALUE @&& true") + << makeDoc("true && VALUE"); + QTest::newRow("macro in left expr") + << makeDoc("(VALUE + 1) @&& true") + << makeDoc("true && (VALUE + 1)"); + QTest::newRow("macro as right expr") + << makeDoc("false @|| VALUE") + << makeDoc("VALUE || false"); + QTest::newRow("macro in right expr") + << makeDoc("false @|| (VALUE + 1)") + << makeDoc("(VALUE + 1) || false"); + } + + void test() + { + QFETCH(QByteArray, original); + QFETCH(QByteArray, expected); + + FlipLogicalOperands factory; + QuickFixOperationTest(singleDocument(original, expected), &factory); + } +}; + +QObject *FlipLogicalOperands::createTest() { return new FlipLogicalOperandsTest; } + +#endif // WITH_TESTS + } // namespace void registerLogicalOperationQuickfixes() @@ -321,3 +375,7 @@ void registerLogicalOperationQuickfixes() } } // namespace CppEditor::Internal + +#ifdef WITH_TESTS +#include +#endif