diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -82,35 +82,35 @@ // // Here, both partial (2) and full (3) specializations are canonicalized to (1) // which ensures all three of them are renamed. -const NamedDecl *canonicalRenameDecl(const NamedDecl *D) { +llvm::DenseSet canonicalRenameDecls(const NamedDecl *D) { if (const auto *VarTemplate = dyn_cast(D)) - return canonicalRenameDecl( + return canonicalRenameDecls( VarTemplate->getSpecializedTemplate()->getTemplatedDecl()); if (const auto *Template = dyn_cast(D)) if (const NamedDecl *TemplatedDecl = Template->getTemplatedDecl()) - return canonicalRenameDecl(TemplatedDecl); + return canonicalRenameDecls(TemplatedDecl); if (const auto *ClassTemplateSpecialization = dyn_cast(D)) - return canonicalRenameDecl( + return canonicalRenameDecls( ClassTemplateSpecialization->getSpecializedTemplate() ->getTemplatedDecl()); if (const auto *Method = dyn_cast(D)) { if (Method->getDeclKind() == Decl::Kind::CXXConstructor || Method->getDeclKind() == Decl::Kind::CXXDestructor) - return canonicalRenameDecl(Method->getParent()); + return canonicalRenameDecls(Method->getParent()); if (const FunctionDecl *InstantiatedMethod = Method->getInstantiatedFromMemberFunction()) - return canonicalRenameDecl(InstantiatedMethod); + return canonicalRenameDecls(InstantiatedMethod); // FIXME(kirillbobyrev): For virtual methods with // size_overridden_methods() > 1, this will not rename all functions it // overrides, because this code assumes there is a single canonical // declaration. if (Method->isVirtual() && Method->size_overridden_methods()) - return canonicalRenameDecl(*Method->overridden_methods().begin()); + return {canonicalRenameDecls(*Method->overridden_methods().begin())}; } if (const auto *Function = dyn_cast(D)) if (const FunctionTemplateDecl *Template = Function->getPrimaryTemplate()) - return canonicalRenameDecl(Template); + return canonicalRenameDecls(Template); if (const auto *Field = dyn_cast(D)) { // This is a hacky way to do something like // CXXMethodDecl::getInstantiatedFromMemberFunction for the field because @@ -119,25 +119,25 @@ const auto *FieldParent = dyn_cast_or_null(Field->getParent()); if (!FieldParent) - return Field->getCanonicalDecl(); + return {Field->getCanonicalDecl()}; FieldParent = FieldParent->getTemplateInstantiationPattern(); // Field is not instantiation. if (!FieldParent || Field->getParent() == FieldParent) - return Field->getCanonicalDecl(); + return {Field->getCanonicalDecl()}; for (const FieldDecl *Candidate : FieldParent->fields()) if (Field->getDeclName() == Candidate->getDeclName()) - return Candidate->getCanonicalDecl(); + return {Candidate->getCanonicalDecl()}; elog("FieldParent should have field with the same name as Field."); } if (const auto *VD = dyn_cast(D)) { if (const VarDecl *OriginalVD = VD->getInstantiatedFromStaticDataMember()) - return canonicalRenameDecl(OriginalVD); + return canonicalRenameDecls(OriginalVD); } if (const auto *UD = dyn_cast(D)) { if (const auto *TargetDecl = UD->getTargetDecl()) - return canonicalRenameDecl(TargetDecl); + return canonicalRenameDecls(TargetDecl); } - return dyn_cast(D->getCanonicalDecl()); + return {dyn_cast(D->getCanonicalDecl())}; } llvm::DenseSet locateDeclAt(ParsedAST &AST, @@ -156,7 +156,7 @@ targetDecl(SelectedNode->ASTNode, DeclRelation::Alias | DeclRelation::TemplatePattern, AST.getHeuristicResolver())) { - Result.insert(canonicalRenameDecl(D)); + Result.insert(D); } return Result; } @@ -267,7 +267,7 @@ std::vector findOccurrencesWithinFile(ParsedAST &AST, const NamedDecl &ND) { trace::Span Tracer("FindOccurrencesWithinFile"); - assert(canonicalRenameDecl(&ND) == &ND && + assert(canonicalRenameDecls(&ND).contains(&ND) && "ND should be already canonicalized."); std::vector Results; @@ -278,7 +278,7 @@ if (Ref.Targets.empty()) return; for (const auto *Target : Ref.Targets) { - if (canonicalRenameDecl(Target) == &ND) { + if (canonicalRenameDecls(Target).contains(&ND)) { Results.push_back(Ref.NameLoc); return; } @@ -529,34 +529,36 @@ // AST-based rename, it renames all occurrences in the main file. llvm::Expected -renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl, +renameWithinFile(ParsedAST &AST, + const llvm::DenseSet &RenameDecls, llvm::StringRef NewName) { trace::Span Tracer("RenameWithinFile"); const SourceManager &SM = AST.getSourceManager(); tooling::Replacements FilteredChanges; - for (SourceLocation Loc : findOccurrencesWithinFile(AST, RenameDecl)) { - SourceLocation RenameLoc = Loc; - // We don't rename in any macro bodies, but we allow rename the symbol - // spelled in a top-level macro argument in the main file. - if (RenameLoc.isMacroID()) { - if (isInMacroBody(SM, RenameLoc)) + for (const auto *RenameDecl : RenameDecls) + for (SourceLocation Loc : findOccurrencesWithinFile(AST, *RenameDecl)) { + SourceLocation RenameLoc = Loc; + // We don't rename in any macro bodies, but we allow rename the symbol + // spelled in a top-level macro argument in the main file. + if (RenameLoc.isMacroID()) { + if (isInMacroBody(SM, RenameLoc)) + continue; + RenameLoc = SM.getSpellingLoc(Loc); + } + // Filter out locations not from main file. + // We traverse only main file decls, but locations could come from an + // non-preamble #include file e.g. + // void test() { + // int f^oo; + // #include "use_foo.inc" + // } + if (!isInsideMainFile(RenameLoc, SM)) continue; - RenameLoc = SM.getSpellingLoc(Loc); + if (auto Err = FilteredChanges.add(tooling::Replacement( + SM, CharSourceRange::getTokenRange(RenameLoc), NewName))) + return std::move(Err); } - // Filter out locations not from main file. - // We traverse only main file decls, but locations could come from an - // non-preamble #include file e.g. - // void test() { - // int f^oo; - // #include "use_foo.inc" - // } - if (!isInsideMainFile(RenameLoc, SM)) - continue; - if (auto Err = FilteredChanges.add(tooling::Replacement( - SM, CharSourceRange::getTokenRange(RenameLoc), NewName))) - return std::move(Err); - } return FilteredChanges; } @@ -592,16 +594,20 @@ // Return all rename occurrences (using the index) outside of the main file, // grouped by the absolute file path. llvm::Expected>> -findOccurrencesOutsideFile(const NamedDecl &RenameDecl, +findOccurrencesOutsideFile(const llvm::DenseSet &RenameDecls, llvm::StringRef MainFile, const SymbolIndex &Index, size_t MaxLimitFiles) { + assert(!RenameDecls.empty() && + "Should have at least one decl to rename here"); trace::Span Tracer("FindOccurrencesOutsideFile"); RefsRequest RQuest; - RQuest.IDs.insert(getSymbolID(&RenameDecl)); - if (const auto *MethodDecl = llvm::dyn_cast(&RenameDecl)) - if (MethodDecl->isVirtual()) - insertTransitiveOverrides(*RQuest.IDs.begin(), RQuest.IDs, Index); + for (const NamedDecl *D : RenameDecls) { + RQuest.IDs.insert(getSymbolID(D)); + if (const auto *MethodDecl = llvm::dyn_cast(D)) + if (MethodDecl->isVirtual()) + insertTransitiveOverrides(*RQuest.IDs.begin(), RQuest.IDs, Index); + } // Absolute file path => rename occurrences in that file. llvm::StringMap> AffectedFiles; @@ -621,7 +627,7 @@ MaxLimitFiles); if (HasMore) return error("The symbol {0} has too many occurrences", - RenameDecl.getQualifiedNameAsString()); + (*RenameDecls.begin())->getQualifiedNameAsString()); // Sort and deduplicate the results, in case that index returns duplications. for (auto &FileAndOccurrences : AffectedFiles) { auto &Ranges = FileAndOccurrences.getValue(); @@ -647,46 +653,47 @@ // as the file content we rename on, and fallback to file content on disk if // there is no dirty buffer. llvm::Expected -renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath, - llvm::StringRef NewName, const SymbolIndex &Index, - size_t MaxLimitFiles, llvm::vfs::FileSystem &FS) { +renameOutsideFile(const llvm::DenseSet &RenameDecls, + llvm::StringRef MainFilePath, llvm::StringRef NewName, + const SymbolIndex &Index, size_t MaxLimitFiles, + llvm::vfs::FileSystem &FS) { trace::Span Tracer("RenameOutsideFile"); - auto AffectedFiles = findOccurrencesOutsideFile(RenameDecl, MainFilePath, + auto AffectedFiles = findOccurrencesOutsideFile(RenameDecls, MainFilePath, Index, MaxLimitFiles); if (!AffectedFiles) return AffectedFiles.takeError(); FileEdits Results; - for (auto &FileAndOccurrences : *AffectedFiles) { - llvm::StringRef FilePath = FileAndOccurrences.first(); - - auto ExpBuffer = FS.getBufferForFile(FilePath); - if (!ExpBuffer) { - elog("Fail to read file content: Fail to open file {0}: {1}", FilePath, - ExpBuffer.getError().message()); - continue; - } - - auto AffectedFileCode = (*ExpBuffer)->getBuffer(); - auto RenameRanges = - adjustRenameRanges(AffectedFileCode, RenameDecl.getNameAsString(), - std::move(FileAndOccurrences.second), - RenameDecl.getASTContext().getLangOpts()); - if (!RenameRanges) { - // Our heuristics fails to adjust rename ranges to the current state of - // the file, it is most likely the index is stale, so we give up the - // entire rename. - return error("Index results don't match the content of file {0} " - "(the index may be stale)", - FilePath); + for (const auto *RenameDecl : RenameDecls) + for (auto &FileAndOccurrences : *AffectedFiles) { + llvm::StringRef FilePath = FileAndOccurrences.first(); + + auto ExpBuffer = FS.getBufferForFile(FilePath); + if (!ExpBuffer) { + elog("Fail to read file content: Fail to open file {0}: {1}", FilePath, + ExpBuffer.getError().message()); + continue; + } + + auto AffectedFileCode = (*ExpBuffer)->getBuffer(); + auto RenameRanges = adjustRenameRanges( + AffectedFileCode, RenameDecl->getNameAsString(), + FileAndOccurrences.second, RenameDecl->getASTContext().getLangOpts()); + if (!RenameRanges) { + // Our heuristics fails to adjust rename ranges to the current state of + // the file, it is most likely the index is stale, so we give up the + // entire rename. + return error("Index results don't match the content of file {0} " + "(the index may be stale)", + FilePath); + } + auto RenameEdit = + buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, NewName); + if (!RenameEdit) + return error("failed to rename in file {0}: {1}", FilePath, + RenameEdit.takeError()); + if (!RenameEdit->Replacements.empty()) + Results.insert({FilePath, std::move(*RenameEdit)}); } - auto RenameEdit = - buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, NewName); - if (!RenameEdit) - return error("failed to rename in file {0}: {1}", FilePath, - RenameEdit.takeError()); - if (!RenameEdit->Replacements.empty()) - Results.insert({FilePath, std::move(*RenameEdit)}); - } return Results; } @@ -762,20 +769,28 @@ return makeError(ReasonToReject::NoSymbolFound); if (DeclsUnderCursor.size() > 1) return makeError(ReasonToReject::AmbiguousSymbol); - const auto &RenameDecl = **DeclsUnderCursor.begin(); - const auto *ID = RenameDecl.getIdentifier(); - if (!ID) - return makeError(ReasonToReject::UnsupportedSymbol); - if (ID->getName() == RInputs.NewName) - return makeError(ReasonToReject::SameName); - auto Invalid = checkName(RenameDecl, RInputs.NewName); - if (Invalid) - return makeError(std::move(*Invalid)); - auto Reject = - renameable(RenameDecl, RInputs.MainFilePath, RInputs.Index, Opts); - if (Reject) - return makeError(*Reject); + llvm::DenseSet RenameDecls; + for (const auto *D : DeclsUnderCursor) { + auto CD = canonicalRenameDecls(D); + RenameDecls.insert(CD.begin(), CD.end()); + } + + for (const NamedDecl *D : RenameDecls) { + auto *ID = D->getIdentifier(); + if (!ID) + return makeError(ReasonToReject::UnsupportedSymbol); + if (ID->getName() == RInputs.NewName) + return makeError(ReasonToReject::SameName); + + auto Invalid = checkName(*D, RInputs.NewName); + if (Invalid) + return makeError(std::move(*Invalid)); + + auto Reject = renameable(*D, RInputs.MainFilePath, RInputs.Index, Opts); + if (Reject) + return makeError(*Reject); + } // We have two implementations of the rename: // - AST-based rename: used for renaming local symbols, e.g. variables @@ -786,7 +801,7 @@ // To make cross-file rename work for local symbol, we use a hybrid solution: // - run AST-based rename on the main file; // - run index-based rename on other affected files; - auto MainFileRenameEdit = renameWithinFile(AST, RenameDecl, RInputs.NewName); + auto MainFileRenameEdit = renameWithinFile(AST, RenameDecls, RInputs.NewName); if (!MainFileRenameEdit) return MainFileRenameEdit.takeError(); @@ -815,9 +830,12 @@ for (const TextEdit &TE : MainFileEdits.asTextEdits()) Result.LocalChanges.push_back(TE.range); - // return the main file edit if this is a within-file rename or the symbol - // being renamed is function local. - if (RenameDecl.getParentFunctionOrMethod()) { + // return the main file edit if this is a within-file rename or the symbols + // being renamed are all function local. + auto IsWithinFileRename = [](const NamedDecl *D) { + return D->getParentFunctionOrMethod() != nullptr; + }; + if (llvm::all_of(RenameDecls, IsWithinFileRename)) { Result.GlobalChanges = FileEdits( {std::make_pair(RInputs.MainFilePath, std::move(MainFileEdits))}); return Result; @@ -831,7 +849,7 @@ } auto OtherFilesEdits = renameOutsideFile( - RenameDecl, RInputs.MainFilePath, RInputs.NewName, *RInputs.Index, + RenameDecls, RInputs.MainFilePath, RInputs.NewName, *RInputs.Index, Opts.LimitFiles == 0 ? std::numeric_limits::max() : Opts.LimitFiles, *RInputs.FS);