[clang-tools-extra] r364735 - [clangd] Show better message when we rename macros.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 1 02:26:48 PDT 2019
Author: hokein
Date: Mon Jul 1 02:26:48 2019
New Revision: 364735
URL: http://llvm.org/viewvc/llvm-project?rev=364735&view=rev
Log:
[clangd] Show better message when we rename macros.
Summary:
Previously, when we rename a macro, we get an error message of "there is
no symbol found".
This patch improves the message of this case (as we don't support macros).
Reviewers: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, mgrang, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D63922
Modified:
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/refactor/Rename.cpp
clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp
clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp
Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=364735&r1=364734&r2=364735&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp Mon Jul 1 02:26:48 2019
@@ -16,6 +16,7 @@
#include "clang/Basic/TokenKinds.h"
#include "clang/Format/Format.h"
#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
#include "llvm/ADT/None.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
@@ -653,5 +654,31 @@ llvm::StringSet<> collectWords(llvm::Str
return Result;
}
+llvm::Optional<DefinedMacro> locateMacroAt(SourceLocation Loc,
+ Preprocessor &PP) {
+ const auto &SM = PP.getSourceManager();
+ const auto &LangOpts = PP.getLangOpts();
+ Token Result;
+ if (Lexer::getRawToken(SM.getSpellingLoc(Loc), Result, SM, LangOpts, false))
+ return None;
+ if (Result.is(tok::raw_identifier))
+ PP.LookUpIdentifierInfo(Result);
+ IdentifierInfo *IdentifierInfo = Result.getIdentifierInfo();
+ if (!IdentifierInfo || !IdentifierInfo->hadMacroDefinition())
+ return None;
+
+ std::pair<FileID, unsigned int> DecLoc = SM.getDecomposedExpansionLoc(Loc);
+ // Get the definition just before the searched location so that a macro
+ // referenced in a '#undef MACRO' can still be found.
+ SourceLocation BeforeSearchedLocation =
+ SM.getMacroArgExpandedLocation(SM.getLocForStartOfFile(DecLoc.first)
+ .getLocWithOffset(DecLoc.second - 1));
+ MacroDefinition MacroDef =
+ PP.getMacroDefinitionAtLoc(IdentifierInfo, BeforeSearchedLocation);
+ if (auto *MI = MacroDef.getMacroInfo())
+ return DefinedMacro{IdentifierInfo->getName(), MI};
+ return None;
+}
+
} // namespace clangd
} // namespace clang
Modified: clang-tools-extra/trunk/clangd/SourceCode.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.h?rev=364735&r1=364734&r2=364735&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/SourceCode.h (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.h Mon Jul 1 02:26:48 2019
@@ -201,6 +201,14 @@ llvm::StringSet<> collectWords(llvm::Str
std::vector<std::string> visibleNamespaces(llvm::StringRef Code,
const format::FormatStyle &Style);
+struct DefinedMacro {
+ llvm::StringRef Name;
+ const MacroInfo *Info;
+};
+// Gets the macro at a specified \p Loc.
+llvm::Optional<DefinedMacro> locateMacroAt(SourceLocation Loc,
+ Preprocessor &PP);
+
} // namespace clangd
} // namespace clang
#endif
Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=364735&r1=364734&r2=364735&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Mon Jul 1 02:26:48 2019
@@ -128,14 +128,9 @@ SymbolLocation getPreferredLocation(cons
return Merged.CanonicalDeclaration;
}
-struct MacroDecl {
- llvm::StringRef Name;
- const MacroInfo *Info;
-};
-
/// Finds declarations locations that a given source location refers to.
class DeclarationAndMacrosFinder : public index::IndexDataConsumer {
- std::vector<MacroDecl> MacroInfos;
+ std::vector<DefinedMacro> MacroInfos;
llvm::DenseSet<const Decl *> Decls;
const SourceLocation &SearchedLocation;
const ASTContext &AST;
@@ -158,16 +153,18 @@ public:
return Result;
}
- std::vector<MacroDecl> takeMacroInfos() {
+ std::vector<DefinedMacro> takeMacroInfos() {
// Don't keep the same Macro info multiple times.
- llvm::sort(MacroInfos, [](const MacroDecl &Left, const MacroDecl &Right) {
- return Left.Info < Right.Info;
- });
-
- auto Last = std::unique(MacroInfos.begin(), MacroInfos.end(),
- [](const MacroDecl &Left, const MacroDecl &Right) {
- return Left.Info == Right.Info;
- });
+ llvm::sort(MacroInfos,
+ [](const DefinedMacro &Left, const DefinedMacro &Right) {
+ return Left.Info < Right.Info;
+ });
+
+ auto Last =
+ std::unique(MacroInfos.begin(), MacroInfos.end(),
+ [](const DefinedMacro &Left, const DefinedMacro &Right) {
+ return Left.Info == Right.Info;
+ });
MacroInfos.erase(Last, MacroInfos.end());
return std::move(MacroInfos);
}
@@ -211,38 +208,16 @@ public:
private:
void finish() override {
- // Also handle possible macro at the searched location.
- Token Result;
- auto &Mgr = AST.getSourceManager();
- if (!Lexer::getRawToken(Mgr.getSpellingLoc(SearchedLocation), Result, Mgr,
- AST.getLangOpts(), false)) {
- if (Result.is(tok::raw_identifier)) {
- PP.LookUpIdentifierInfo(Result);
- }
- IdentifierInfo *IdentifierInfo = Result.getIdentifierInfo();
- if (IdentifierInfo && IdentifierInfo->hadMacroDefinition()) {
- std::pair<FileID, unsigned int> DecLoc =
- Mgr.getDecomposedExpansionLoc(SearchedLocation);
- // Get the definition just before the searched location so that a macro
- // referenced in a '#undef MACRO' can still be found.
- SourceLocation BeforeSearchedLocation = Mgr.getMacroArgExpandedLocation(
- Mgr.getLocForStartOfFile(DecLoc.first)
- .getLocWithOffset(DecLoc.second - 1));
- MacroDefinition MacroDef =
- PP.getMacroDefinitionAtLoc(IdentifierInfo, BeforeSearchedLocation);
- MacroInfo *MacroInf = MacroDef.getMacroInfo();
- if (MacroInf) {
- MacroInfos.push_back(MacroDecl{IdentifierInfo->getName(), MacroInf});
- assert(Decls.empty());
- }
- }
+ if (auto DefinedMacro = locateMacroAt(SearchedLocation, PP)) {
+ MacroInfos.push_back(*DefinedMacro);
+ assert(Decls.empty());
}
}
};
struct IdentifiedSymbol {
std::vector<const Decl *> Decls;
- std::vector<MacroDecl> Macros;
+ std::vector<DefinedMacro> Macros;
};
IdentifiedSymbol getSymbolAtPosition(ParsedAST &AST, SourceLocation Pos) {
@@ -740,18 +715,18 @@ static HoverInfo getHoverContents(QualTy
}
/// Generate a \p Hover object given the macro \p MacroDecl.
-static HoverInfo getHoverContents(MacroDecl Decl, ParsedAST &AST) {
+static HoverInfo getHoverContents(const DefinedMacro &Macro, ParsedAST &AST) {
HoverInfo HI;
SourceManager &SM = AST.getSourceManager();
- HI.Name = Decl.Name;
+ HI.Name = Macro.Name;
HI.Kind = indexSymbolKindToSymbolKind(
- index::getSymbolInfoForMacro(*Decl.Info).Kind);
+ index::getSymbolInfoForMacro(*Macro.Info).Kind);
// FIXME: Populate documentation
// FIXME: Pupulate parameters
// Try to get the full definition, not just the name
- SourceLocation StartLoc = Decl.Info->getDefinitionLoc();
- SourceLocation EndLoc = Decl.Info->getDefinitionEndLoc();
+ SourceLocation StartLoc = Macro.Info->getDefinitionLoc();
+ SourceLocation EndLoc = Macro.Info->getDefinitionEndLoc();
if (EndLoc.isValid()) {
EndLoc = Lexer::getLocForEndOfToken(EndLoc, 0, SM,
AST.getASTContext().getLangOpts());
Modified: clang-tools-extra/trunk/clangd/refactor/Rename.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/Rename.cpp?rev=364735&r1=364734&r2=364735&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/refactor/Rename.cpp (original)
+++ clang-tools-extra/trunk/clangd/refactor/Rename.cpp Mon Jul 1 02:26:48 2019
@@ -136,6 +136,25 @@ llvm::Optional<ReasonToReject> renamable
return ReasonToReject::UsedOutsideFile;
}
+llvm::Error makeError(ReasonToReject Reason) {
+ auto Message = [](ReasonToReject Reason) {
+ switch (Reason) {
+ case NoIndexProvided:
+ return "symbol may be used in other files (no index available)";
+ case UsedOutsideFile:
+ return "the symbol is used outside main file";
+ case NonIndexable:
+ return "symbol may be used in other files (not eligible for indexing)";
+ case UnsupportedSymbol:
+ return "symbol is not a supported kind (e.g. namespace, macro)";
+ }
+ llvm_unreachable("unhandled reason kind");
+ };
+ return llvm::make_error<llvm::StringError>(
+ llvm::formatv("Cannot rename symbol: {0}", Message(Reason)),
+ llvm::inconvertibleErrorCode());
+}
+
} // namespace
llvm::Expected<tooling::Replacements>
@@ -145,6 +164,10 @@ renameWithinFile(ParsedAST &AST, llvm::S
ASTContext &ASTCtx = AST.getASTContext();
SourceLocation SourceLocationBeg = clangd::getBeginningOfIdentifier(
AST, Pos, AST.getSourceManager().getMainFileID());
+ // FIXME: renaming macros is not supported yet, the macro-handling code should
+ // be moved to rename tooling library.
+ if (locateMacroAt(SourceLocationBeg, AST.getPreprocessor()))
+ return makeError(UnsupportedSymbol);
tooling::RefactoringRuleContext Context(AST.getSourceManager());
Context.setASTContext(ASTCtx);
auto Rename = clang::tooling::RenameOccurrences::initiate(
@@ -155,24 +178,8 @@ renameWithinFile(ParsedAST &AST, llvm::S
const auto *RenameDecl = Rename->getRenameDecl();
assert(RenameDecl && "symbol must be found at this point");
if (auto Reject =
- renamableWithinFile(*RenameDecl->getCanonicalDecl(), File, Index)) {
- auto Message = [](ReasonToReject Reason) {
- switch (Reason) {
- case NoIndexProvided:
- return "symbol may be used in other files (no index available)";
- case UsedOutsideFile:
- return "the symbol is used outside main file";
- case NonIndexable:
- return "symbol may be used in other files (not eligible for indexing)";
- case UnsupportedSymbol:
- return "symbol is not a supported kind (e.g. namespace)";
- }
- llvm_unreachable("unhandled reason kind");
- };
- return llvm::make_error<llvm::StringError>(
- llvm::formatv("Cannot rename symbol: {0}", Message(*Reject)),
- llvm::inconvertibleErrorCode());
- }
+ renamableWithinFile(*RenameDecl->getCanonicalDecl(), File, Index))
+ return makeError(*Reject);
Rename->invoke(ResultCollector, Context);
Modified: clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp?rev=364735&r1=364734&r2=364735&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp Mon Jul 1 02:26:48 2019
@@ -129,6 +129,13 @@ TEST(RenameTest, Renameable) {
)cpp",
"not a supported kind", HeaderFile},
+ {
+ R"cpp(
+ #define MACRO 1
+ int s = MAC^RO;
+ )cpp",
+ "not a supported kind", HeaderFile},
+
{R"cpp(// foo is declared outside the file.
void fo^o() {}
)cpp", "used outside main file", !HeaderFile/*cc file*/},
Modified: clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp?rev=364735&r1=364734&r2=364735&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp Mon Jul 1 02:26:48 2019
@@ -9,6 +9,7 @@
#include "Context.h"
#include "Protocol.h"
#include "SourceCode.h"
+#include "TestTU.h"
#include "clang/Format/Format.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/raw_os_ostream.h"
@@ -28,6 +29,8 @@ MATCHER_P2(Pos, Line, Col, "") {
return arg.line == int(Line) && arg.character == int(Col);
}
+MATCHER_P(MacroName, Name, "") { return arg.Name == Name; }
+
/// A helper to make tests easier to read.
Position position(int line, int character) {
Position Pos;
@@ -404,6 +407,20 @@ TEST(SourceCodeTests, VisibleNamespaces)
}
}
+TEST(SourceCodeTests, GetMacros) {
+ Annotations Code(R"cpp(
+ #define MACRO 123
+ int abc = MA^CRO;
+ )cpp");
+ TestTU TU = TestTU::withCode(Code.code());
+ auto AST = TU.build();
+ auto Loc = getBeginningOfIdentifier(AST, Code.point(),
+ AST.getSourceManager().getMainFileID());
+ auto Result = locateMacroAt(Loc, AST.getPreprocessor());
+ ASSERT_TRUE(Result);
+ EXPECT_THAT(*Result, MacroName("MACRO"));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list