SureYeaah created this revision.
SureYeaah added reviewers: sammccall, kadircet.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
Fixed SelectionTree bug for macros
- Fixed SelectionTree claimRange for macros and template instantiations
- Fixed SelectionTree unit tests
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D64329
Files:
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/unittests/SelectionTests.cpp
Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -37,15 +37,13 @@
Range nodeRange(const SelectionTree::Node *N, ParsedAST &AST) {
if (!N)
return Range{};
- SourceManager &SM = AST.getSourceManager();
+ const SourceManager &SM = AST.getSourceManager();
+ const LangOptions &LangOpts = AST.getASTContext().getLangOpts();
StringRef Buffer = SM.getBufferData(SM.getMainFileID());
SourceRange SR = N->ASTNode.getSourceRange();
- SR.setBegin(SM.getFileLoc(SR.getBegin()));
- SR.setEnd(SM.getFileLoc(SR.getEnd()));
- CharSourceRange R =
- Lexer::getAsCharRange(SR, SM, AST.getASTContext().getLangOpts());
- return Range{offsetToPosition(Buffer, SM.getFileOffset(R.getBegin())),
- offsetToPosition(Buffer, SM.getFileOffset(R.getEnd()))};
+ SR = getFileRange(SR, SM, LangOpts);
+ return Range{offsetToPosition(Buffer, SM.getFileOffset(SR.getBegin())),
+ offsetToPosition(Buffer, SM.getFileOffset(SR.getEnd()))};
}
std::string nodeKind(const SelectionTree::Node *N) {
@@ -144,17 +142,17 @@
R"cpp(
void foo();
#define CALL_FUNCTION(X) X()
- void bar() [[{ CALL_FUNC^TION(fo^o); }]]
+ void bar() { [[CALL_FUNC^TION(fo^o)]]; }
)cpp",
- "CompoundStmt",
+ "CallExpr",
},
{
R"cpp(
void foo();
#define CALL_FUNCTION(X) X()
- void bar() [[{ C^ALL_FUNC^TION(foo); }]]
+ void bar() { [[C^ALL_FUNC^TION(foo)]]; }
)cpp",
- "CompoundStmt",
+ "CallExpr",
},
{
R"cpp(
@@ -308,7 +306,7 @@
R"cpp(
template <class T>
struct unique_ptr {};
- void foo(^$C[[unique_ptr<unique_ptr<$C[[int]]>>]]^ a) {}
+ void foo(^$C[[unique_ptr<$C[[unique_ptr<$C[[int]]>]]>]]^ a) {}
)cpp",
};
for (const char *C : Cases) {
Index: clang-tools-extra/clangd/SourceCode.h
===================================================================
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -20,8 +20,8 @@
#include "clang/Basic/SourceManager.h"
#include "clang/Format/Format.h"
#include "clang/Tooling/Core/Replacement.h"
-#include "llvm/ADT/StringSet.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
#include "llvm/Support/SHA1.h"
namespace clang {
@@ -201,6 +201,9 @@
std::vector<std::string> visibleNamespaces(llvm::StringRef Code,
const format::FormatStyle &Style);
+// Return the FileRange for a given range where the ends can be in different
+// files. Note that the end of the FileRange is the end of the last token.
+SourceRange getFileRange(SourceRange Range, const SourceManager &SM, const LangOptions &LangOpts);
} // namespace clangd
} // namespace clang
#endif
Index: clang-tools-extra/clangd/SourceCode.cpp
===================================================================
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -12,6 +12,7 @@
#include "Logger.h"
#include "Protocol.h"
#include "clang/AST/ASTContext.h"
+#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TokenKinds.h"
#include "clang/Format/Format.h"
@@ -24,6 +25,7 @@
#include "llvm/Support/Error.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
#include <algorithm>
namespace clang {
@@ -444,8 +446,8 @@
namespace {
enum NamespaceEvent {
BeginNamespace, // namespace <ns> {. Payload is resolved <ns>.
- EndNamespace, // } // namespace <ns>. Payload is resolved *outer* namespace.
- UsingDirective // using namespace <ns>. Payload is unresolved <ns>.
+ EndNamespace, // } // namespace <ns>. Payload is resolved *outer* namespace.
+ UsingDirective // using namespace <ns>. Payload is unresolved <ns>.
};
// Scans C++ source code for constructs that change the visible namespaces.
void parseNamespaceEvents(
@@ -468,7 +470,7 @@
std::string NSName;
lex(Code, Style, [&](const clang::Token &Tok) {
- switch(Tok.getKind()) {
+ switch (Tok.getKind()) {
case tok::raw_identifier:
// In raw mode, this could be a keyword or a name.
switch (State) {
@@ -570,40 +572,37 @@
// Map from namespace to (resolved) namespaces introduced via using directive.
llvm::StringMap<llvm::StringSet<>> UsingDirectives;
- parseNamespaceEvents(Code, Style,
- [&](NamespaceEvent Event, llvm::StringRef NS) {
- switch (Event) {
- case BeginNamespace:
- case EndNamespace:
- Current = NS;
- break;
- case UsingDirective:
- if (NS.consume_front("::"))
- UsingDirectives[Current].insert(NS);
- else {
- for (llvm::StringRef Enclosing :
- ancestorNamespaces(Current)) {
- if (Enclosing.empty())
- UsingDirectives[Current].insert(NS);
- else
- UsingDirectives[Current].insert(
- (Enclosing + "::" + NS).str());
- }
- }
- break;
- }
- });
+ parseNamespaceEvents(
+ Code, Style, [&](NamespaceEvent Event, llvm::StringRef NS) {
+ switch (Event) {
+ case BeginNamespace:
+ case EndNamespace:
+ Current = NS;
+ break;
+ case UsingDirective:
+ if (NS.consume_front("::"))
+ UsingDirectives[Current].insert(NS);
+ else {
+ for (llvm::StringRef Enclosing : ancestorNamespaces(Current)) {
+ if (Enclosing.empty())
+ UsingDirectives[Current].insert(NS);
+ else
+ UsingDirectives[Current].insert((Enclosing + "::" + NS).str());
+ }
+ }
+ break;
+ }
+ });
std::vector<std::string> Found;
for (llvm::StringRef Enclosing : ancestorNamespaces(Current)) {
Found.push_back(Enclosing);
auto It = UsingDirectives.find(Enclosing);
if (It != UsingDirectives.end())
- for (const auto& Used : It->second)
+ for (const auto &Used : It->second)
Found.push_back(Used.getKey());
}
-
llvm::sort(Found, [&](const std::string &LHS, const std::string &RHS) {
if (Current == RHS)
return false;
@@ -653,5 +652,59 @@
return Result;
}
+// Returns the location of the end of the token present at a given location
+static SourceLocation getLocationAtTokenEnd(SourceLocation Loc,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ unsigned TokLength = Lexer::MeasureTokenLength(Loc, SM, LangOpts);
+ return Loc.getLocWithOffset(TokLength);
+}
+
+// Finds the union of two ranges. If any of the ranges is a Token Range, it uses
+// the token ending.
+static CharSourceRange unionCharSourceRange(CharSourceRange R1,
+ CharSourceRange R2,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ if (R1.isTokenRange())
+ R1.setEnd(getLocationAtTokenEnd(R1.getEnd(), SM, LangOpts));
+ if (R2.isTokenRange())
+ R2.setEnd(getLocationAtTokenEnd(R2.getEnd(), SM, LangOpts));
+ return CharSourceRange({std::min(R1.getBegin(), R2.getBegin()),
+ std::max(R1.getEnd(), R2.getEnd())},
+ false);
+}
+
+// returns the FileRange for a given Location as CharSourceRange. Note that the
+// end of the FileRange is the end of the last token.
+static CharSourceRange getFileRange(SourceLocation Loc, const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ if (Loc.isFileID())
+ return {{Loc, getLocationAtTokenEnd(Loc, SM, LangOpts)}, false};
+ CharSourceRange FileRange({Loc}, false);
+ do {
+ if (SM.isMacroArgExpansion(FileRange.getBegin())) {
+ FileRange.setBegin(SM.getImmediateSpellingLoc(FileRange.getBegin()));
+ FileRange.setEnd(getLocationAtTokenEnd(
+ SM.getImmediateSpellingLoc(FileRange.getEnd()), SM, LangOpts));
+ } else {
+ CharSourceRange ExpansionRangeForBegin =
+ SM.getImmediateExpansionRange(FileRange.getBegin());
+ CharSourceRange ExpansionRangeForEnd =
+ SM.getImmediateExpansionRange(FileRange.getEnd());
+ FileRange = unionCharSourceRange(ExpansionRangeForBegin,
+ ExpansionRangeForEnd, SM, LangOpts);
+ }
+ } while (!FileRange.getBegin().isFileID());
+ return FileRange;
+}
+
+SourceRange getFileRange(SourceRange Range, const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ CharSourceRange R1 = getFileRange(Range.getBegin(), SM, LangOpts);
+ CharSourceRange R2 = getFileRange(Range.getEnd(), SM, LangOpts);
+ return unionCharSourceRange(R1, R2, SM, LangOpts).getAsRange();
+}
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/Selection.cpp
===================================================================
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -8,10 +8,13 @@
#include "Selection.h"
#include "ClangdUnit.h"
+#include "SourceCode.h"
#include "clang/AST/ASTTypeTraits.h"
#include "clang/AST/PrettyPrinter.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
#include "llvm/ADT/STLExtras.h"
#include <algorithm>
@@ -68,8 +71,8 @@
public:
// Runs the visitor to gather selected nodes and their ancestors.
// If there is any selection, the root (TUDecl) is the first node.
- static std::deque<Node> collect(ASTContext &AST, unsigned Begin,
- unsigned End, FileID File) {
+ static std::deque<Node> collect(ASTContext &AST, unsigned Begin, unsigned End,
+ FileID File) {
SelectionVisitor V(AST, Begin, End, File);
V.TraverseAST(AST);
assert(V.Stack.size() == 1 && "Unpaired push/pop?");
@@ -205,6 +208,7 @@
// Performs primary hit detection.
void pop() {
Node &N = *Stack.top();
+ N.ASTNode.dump(llvm::errs(), SM);
if (auto Sel = claimRange(N.ASTNode.getSourceRange()))
N.Selected = Sel;
if (N.Selected || !N.Children.empty()) {
@@ -236,39 +240,42 @@
// Perform hit-testing of a complete Node against the selection.
// This runs for every node in the AST, and must be fast in common cases.
// This is usually called from pop(), so we can take children into account.
+ // FIXME: Doesn't select the binary operator node in
+ // #define FOO(X) X + 1
+ // int a, b = [[FOO(a)]];
SelectionTree::Selection claimRange(SourceRange S) {
+ llvm::errs() << "S: "; S.dump(SM);
if (!S.isValid())
return SelectionTree::Unselected;
- // getTopMacroCallerLoc() allows selection of constructs in macro args. e.g:
+ // getFileRange() allows selecting macro arg expansions
// #define LOOP_FOREVER(Body) for(;;) { Body }
// void IncrementLots(int &x) {
// LOOP_FOREVER( ++x; )
// }
// Selecting "++x" or "x" will do the right thing.
- auto B = SM.getDecomposedLoc(SM.getTopMacroCallerLoc(S.getBegin()));
- auto E = SM.getDecomposedLoc(SM.getTopMacroCallerLoc(S.getEnd()));
+ // The end of the range returned by getFileRange is the ending of last token
+ SourceRange Range = getFileRange(S, SM, LangOpts);
+ auto B = SM.getDecomposedLoc(Range.getBegin());
+ auto E = SM.getDecomposedLoc(Range.getEnd());
// Otherwise, nodes in macro expansions can't be selected.
if (B.first != SelFile || E.first != SelFile)
return SelectionTree::Unselected;
// Cheap test: is there any overlap at all between the selection and range?
- // Note that E.second is the *start* of the last token, which is why we
- // compare against the "rounded-down" SelBegin.
- if (B.second >= SelEnd || E.second < SelBeginTokenStart)
+ if (B.second >= SelEnd || E.second < SelBegin)
return SelectionTree::Unselected;
-
// We may have hit something, need some more precise checks.
- // Adjust [B, E) to be a half-open character range.
- E.second += Lexer::MeasureTokenLength(S.getEnd(), SM, LangOpts);
auto PreciseBounds = std::make_pair(B.second, E.second);
// Trim range using the selection, drop it if empty.
B.second = std::max(B.second, SelBegin);
E.second = std::min(E.second, SelEnd);
if (B.second >= E.second)
return SelectionTree::Unselected;
+ llvm::errs() << "HERE1\n";
// Attempt to claim the remaining range. If there's nothing to claim, only
// children were selected.
if (!Claimed.add(B.second, E.second))
return SelectionTree::Unselected;
+ llvm::errs() << "HERE2\n";
// Some of our own characters are covered, this is a true hit.
// Determine whether the node was completely covered.
return (PreciseBounds.first >= SelBegin && PreciseBounds.second <= SelEnd)
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
