From 86aab16ea4a45b1c67637f4504afdcbd7adfd261 Mon Sep 17 00:00:00 2001 From: Hugo Holgersson Date: Sat, 17 Feb 2018 21:12:15 +0100 Subject: [PATCH] TextEditor: Highlight punctuators as Text This change limits the set of tokens that fall under Token::isOperator(). That allows cpphighlighter.cpp to distinguish operator tokens from punctuator tokens (without changing any logic in cpphighlighter.cpp). This change moves punctuators from "Operator" to the "Text" style category where they belong. Punctuators are not operators. Punctuators are dumb text tokens. Why don't we let the clang backend alone separate these tokens for us? 1. Clang is slow on big files. Sometimes the highlighting dictated by clang is painted _seconds_ after cpphighlighter.cpp runs. CppHighlighter is way faster so we use it to "prepaint" code while clang is busy in the background. 2. Secondly, clang cannot yet handle all operator types. In particular, none if its "operator cursors" CXCursor_UnaryOperator: CXCursor_BinaryOperator: CXCursor_CompoundAssignOperator: CXCursor_ConditionalOperator: includes the -> and . operators. We still need CppHighlighter to paint those tokens. However, once clang has finished processing the file some operator tokens will be repainted. We need clang to get all operators' semantics. In particular, we need clang to tell us if < is a "smaller than"-operator or part of a template parameter like set. Task-number: QTCREATORBUG-19659 Change-Id: I952cb58f7c79134b3281e2a8221425cc1d0ad263 Reviewed-by: Ivan Donchevskii --- src/libs/3rdparty/cplusplus/Parser.cpp | 2 +- src/libs/3rdparty/cplusplus/Token.h | 32 ++++++++++------- src/libs/cplusplus/ExpressionUnderCursor.cpp | 2 +- src/libs/cplusplus/MatchingText.cpp | 2 +- .../cpptools/cppfollowsymbolundercursor.cpp | 2 +- src/plugins/texteditor/texteditorsettings.cpp | 5 ++- src/tools/clangbackend/source/tokeninfo.cpp | 6 ++++ tests/unit/unittest/tokenprocessor-test.cpp | 35 +++++++++++++++++++ 8 files changed, 67 insertions(+), 19 deletions(-) diff --git a/src/libs/3rdparty/cplusplus/Parser.cpp b/src/libs/3rdparty/cplusplus/Parser.cpp index 250576edcf2..05ac1102452 100644 --- a/src/libs/3rdparty/cplusplus/Parser.cpp +++ b/src/libs/3rdparty/cplusplus/Parser.cpp @@ -4998,7 +4998,7 @@ bool Parser::parseNameId(NameAST *&name) return parseName(name, false); default: - if (tok().isLiteral() || tok().isOperator()) { + if (tok().isLiteral() || tok().isPunctuationOrOperator()) { rewind(start); return parseName(name, false); } diff --git a/src/libs/3rdparty/cplusplus/Token.h b/src/libs/3rdparty/cplusplus/Token.h index 7cf9fca3f3f..9d30911169b 100644 --- a/src/libs/3rdparty/cplusplus/Token.h +++ b/src/libs/3rdparty/cplusplus/Token.h @@ -60,6 +60,22 @@ enum Kind { T_LAST_STRING_LITERAL = T_ANGLE_STRING_LITERAL, T_LAST_LITERAL = T_ANGLE_STRING_LITERAL, + T_FIRST_PUNCTUATION_OR_OPERATOR, + T_FIRST_PUNCTUATION = T_FIRST_PUNCTUATION_OR_OPERATOR, + T_COLON = T_FIRST_PUNCTUATION_OR_OPERATOR, + T_COLON_COLON, + T_COMMA, + T_GREATER, + T_LESS, + T_LBRACE, + T_LBRACKET, + T_LPAREN, + T_RBRACE, + T_RBRACKET, + T_RPAREN, + T_SEMICOLON, + T_LAST_PUNCTUATION = T_SEMICOLON, + T_FIRST_OPERATOR, T_AMPER = T_FIRST_OPERATOR, T_AMPER_AMPER, @@ -68,9 +84,6 @@ enum Kind { T_ARROW_STAR, T_CARET, T_CARET_EQUAL, - T_COLON, - T_COLON_COLON, - T_COMMA, T_SLASH, T_SLASH_EQUAL, T_DOT, @@ -80,17 +93,12 @@ enum Kind { T_EQUAL_EQUAL, T_EXCLAIM, T_EXCLAIM_EQUAL, - T_GREATER, T_GREATER_EQUAL, T_GREATER_GREATER, T_GREATER_GREATER_EQUAL, - T_LBRACE, - T_LBRACKET, - T_LESS, T_LESS_EQUAL, T_LESS_LESS, T_LESS_LESS_EQUAL, - T_LPAREN, T_MINUS, T_MINUS_EQUAL, T_MINUS_MINUS, @@ -105,15 +113,12 @@ enum Kind { T_POUND, T_POUND_POUND, T_QUESTION, - T_RBRACE, - T_RBRACKET, - T_RPAREN, - T_SEMICOLON, T_STAR, T_STAR_EQUAL, T_TILDE, T_TILDE_EQUAL, T_LAST_OPERATOR = T_TILDE_EQUAL, + T_LAST_PUNCTUATION_OR_OPERATOR = T_LAST_OPERATOR, T_FIRST_KEYWORD, T_ALIGNAS = T_FIRST_KEYWORD, @@ -327,6 +332,9 @@ public: inline bool isOperator() const { return f.kind >= T_FIRST_OPERATOR && f.kind <= T_LAST_OPERATOR; } + inline bool isPunctuationOrOperator() const + { return f.kind >= T_FIRST_PUNCTUATION_OR_OPERATOR && f.kind <= T_LAST_PUNCTUATION_OR_OPERATOR; } + inline bool isKeyword() const { return f.kind >= T_FIRST_KEYWORD && f.kind < T_FIRST_PRIMITIVE; } diff --git a/src/libs/cplusplus/ExpressionUnderCursor.cpp b/src/libs/cplusplus/ExpressionUnderCursor.cpp index 1d768b2a098..0b4405412bc 100644 --- a/src/libs/cplusplus/ExpressionUnderCursor.cpp +++ b/src/libs/cplusplus/ExpressionUnderCursor.cpp @@ -65,7 +65,7 @@ int ExpressionUnderCursor::startOfExpression(BackwardsScanner &tk, int index) break; default: - if (tok.isOperator()) + if (tok.isPunctuationOrOperator()) return startOfExpression(tk, index - 1); break; diff --git a/src/libs/cplusplus/MatchingText.cpp b/src/libs/cplusplus/MatchingText.cpp index ade81c9b9f8..c22d3fe89cf 100644 --- a/src/libs/cplusplus/MatchingText.cpp +++ b/src/libs/cplusplus/MatchingText.cpp @@ -98,7 +98,7 @@ static bool insertQuote(const QChar ch, const BackwardsScanner &tk) return true; // Insert a matching quote after an operator. - if (token.isOperator()) + if (token.isPunctuationOrOperator()) return true; if (token.isKeyword()) diff --git a/src/plugins/cpptools/cppfollowsymbolundercursor.cpp b/src/plugins/cpptools/cppfollowsymbolundercursor.cpp index 304d938cd8a..65bfd310a55 100644 --- a/src/plugins/cpptools/cppfollowsymbolundercursor.cpp +++ b/src/plugins/cpptools/cppfollowsymbolundercursor.cpp @@ -592,7 +592,7 @@ void FollowSymbolUnderCursor::findLink( documentFromSemanticInfo, symbolFinder); if (link.hasValidLinkText()) return processLinkCallback(link); - } else if (tk.isOperator() && i > 0 && tokens.at(i - 1).is(T_OPERATOR)) { + } else if (tk.isPunctuationOrOperator() && i > 0 && tokens.at(i - 1).is(T_OPERATOR)) { QTextCursor c = cursor; c.movePosition(QTextCursor::Left, QTextCursor::MoveAnchor, positionInBlock - tokens.at(i - 1).utf16charsBegin()); diff --git a/src/plugins/texteditor/texteditorsettings.cpp b/src/plugins/texteditor/texteditorsettings.cpp index 64a618f6fb2..ff6cb706685 100644 --- a/src/plugins/texteditor/texteditorsettings.cpp +++ b/src/plugins/texteditor/texteditorsettings.cpp @@ -92,9 +92,8 @@ TextEditorSettings::TextEditorSettings() // Add font preference page FormatDescriptions formatDescr; formatDescr.reserve(C_LAST_STYLE_SENTINEL); - formatDescr.emplace_back(C_TEXT, tr("Text"), tr("Generic text.\nApplied to " - "text, if no other " - "rules matching.")); + formatDescr.emplace_back(C_TEXT, tr("Text"), tr("Generic text and punctuation tokens.\n" + "Applied to text that matched no other rule.")); // Special categories const QPalette p = QApplication::palette(); diff --git a/src/tools/clangbackend/source/tokeninfo.cpp b/src/tools/clangbackend/source/tokeninfo.cpp index c9311d22214..a5a2220adad 100644 --- a/src/tools/clangbackend/source/tokeninfo.cpp +++ b/src/tools/clangbackend/source/tokeninfo.cpp @@ -549,6 +549,12 @@ void TokenInfo::punctuationOrOperatorKind() case CXCursor_Constructor: collectOutputArguments(m_originalCursor); break; + case CXCursor_UnaryOperator: + case CXCursor_BinaryOperator: + case CXCursor_CompoundAssignOperator: + case CXCursor_ConditionalOperator: + m_types.mainHighlightingType = HighlightingType::Operator; + break; default: break; } diff --git a/tests/unit/unittest/tokenprocessor-test.cpp b/tests/unit/unittest/tokenprocessor-test.cpp index c6df2044b57..50ee2cd9d03 100644 --- a/tests/unit/unittest/tokenprocessor-test.cpp +++ b/tests/unit/unittest/tokenprocessor-test.cpp @@ -850,6 +850,41 @@ TEST_F(TokenProcessor, CurlyRightParenthesisIsAPunctuation) ASSERT_THAT(infos[9], HasOnlyType(HighlightingType::Invalid)); } +TEST_F(TokenProcessor, OperatorColon) +{ + const auto infos = translationUnit.tokenInfosInRange(sourceRange(668, 28)); + + ASSERT_THAT(infos[6], HasOnlyType(HighlightingType::Operator)); +} + +TEST_F(TokenProcessor, PunctuationColon) +{ + const auto infos = translationUnit.tokenInfosInRange(sourceRange(133, 10)); + + ASSERT_THAT(infos[2], HasOnlyType(HighlightingType::Invalid)); +} + +TEST_F(TokenProcessor, LessThanOperator) +{ + const auto infos = translationUnit.tokenInfosInRange(sourceRange(668, 28)); + + ASSERT_THAT(infos[2], HasOnlyType(HighlightingType::Operator)); +} + +TEST_F(TokenProcessor, LessThanPunctuation) +{ + const auto infos = translationUnit.tokenInfosInRange(sourceRange(247, 19)); + + ASSERT_THAT(infos[1], HasOnlyType(HighlightingType::Invalid)); +} + +TEST_F(TokenProcessor, GreaterThanPunctuation) +{ + const auto infos = translationUnit.tokenInfosInRange(sourceRange(247, 19)); + + ASSERT_THAT(infos[4], HasOnlyType(HighlightingType::Invalid)); +} + TEST_F(TokenProcessor, Comment) { const auto infos = translationUnit.tokenInfosInRange(sourceRange(229, 14));