[PATCH] D65928: [clangd] Added an early return from VisitMemberExpr in SemanticHighlighting if the MemberLoc is invalid.

Johan Vikström via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 8 01:32:03 PDT 2019


jvikstrom created this revision.
jvikstrom added reviewers: hokein, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Conversion operators contain invalid MemberLocs which causes SemanticHighlighting to emit a lot of error logs in large files as they can occur fairly often (for example converting StringRef to std string).
As the only thing happening was a lot of error logs being emited there doesn't really seem to be any way to test this (no erroneous tokens are added). But emiting as many logs as were being emited is not wanted.

Can't really be patched elsewhere. RecursiveASTVisitor should still traverse the expr as it can contain other "non-implicit" decls/exprs that must be visited (for example DeclRefs). A potential fix could be to special case the TraverseMemberExpr function to not Walk* the Expr if it has an invalid MemberLoc. But that would change the behaviour of RecursiveASTVisitor to be unexpected. (and to change the behaviour to be this for all exprs would change the contract of RecursiveASTVisitor to much)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65928

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp


Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===================================================================
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -52,6 +52,11 @@
       // When calling the destructor manually like: AAA::~A(); The ~ is a
       // MemberExpr. Other methods should still be highlighted though.
       return true;
+    if (ME->getMemberLoc().isInvalid())
+      // If the member is a conversion operator the loc will be invalid. This
+      // causes the addToken function to emit a lot of error logs about trying
+      // to add a token with an invalid range.
+      return true;
     addToken(ME->getMemberLoc(), MD);
     return true;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65928.214083.patch
Type: text/x-patch
Size: 749 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190808/62ff4f8f/attachment.bin>


More information about the cfe-commits mailing list