diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp @@ -48,6 +48,7 @@ #include "llvm/Support/Error.h" #include "llvm/Support/Signals.h" #include "llvm/Support/raw_ostream.h" +#include "FindTarget.h" #include #include #include @@ -93,251 +94,6 @@ return nullptr; } -// There are three types of spellings that needs to be qualified in a function -// body: -// - Types: Foo -> ns::Foo -// - DeclRefExpr: ns2::foo() -> ns1::ns2::foo(); -// - UsingDecls: -// using ns2::foo -> using ns1::ns2::foo -// using namespace ns2 -> using namespace ns1::ns2 -// using ns3 = ns2 -> using ns3 = ns1::ns2 -// -// Goes over all DeclRefExprs, TypeLocs, and Using{,Directive}Decls inside a -// function body to generate replacements that will fully qualify those. So that -// body can be moved into an arbitrary file. -// We perform the qualification by qualyfying the last type/decl in a -// (un)qualified name. e.g: -// namespace a { namespace b { class Bar{}; void foo(); } } -// b::Bar x; -> a::b::Bar x; -// foo(); -> a::b::foo(); -// Currently there is no way to know whether a given TypeLoc is the last one or -// not, therefore we generate replacements for all the TypeLocs we see in a -// given name and pick only the longest one. -// FIXME: Instead of fully qualyfying we should try deducing visible scopes at -// target location and generate minimal edits. -class QualifyingVisitor : public RecursiveASTVisitor { -public: - QualifyingVisitor(const FunctionDecl *FD) - : LangOpts(FD->getASTContext().getLangOpts()), - SM(FD->getASTContext().getSourceManager()), - BodyBegin(SM.getFileOffset(FD->getBody()->getBeginLoc())) { - TraverseStmt(FD->getBody()); - } - - // We override traversals for DeclRefExprs and TypeLocs to generate an edit - // only for the last Type/Decl in a written name. Also it enables us to catch - // current range of the name as written, since the last Type/Decl doesn't - // contain that information. - bool TraverseDeclRefExpr(DeclRefExpr *DRE) { - maybeUpdateCurrentRange(DRE->getSourceRange()); - return RecursiveASTVisitor::TraverseDeclRefExpr(DRE); - } - - bool TraverseTypeLoc(TypeLoc TL) { - maybeUpdateCurrentRange(TL.getSourceRange()); - return RecursiveASTVisitor::TraverseTypeLoc(TL); - } - - // Generates a replacement that will qualify templated name and arguments. - bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TSTL) { - std::string Qualified; - llvm::raw_string_ostream OS(Qualified); - - QualType Ty = TSTL.getType(); - if (Ty->isDependentType()) { - // We don't have a decl if type is dependent, use the TemplateDecl - // instead. - const TemplateDecl *TD = - TSTL.getTypePtr()->getTemplateName().getAsTemplateDecl(); - - TD->printQualifiedName(OS); - // FIXME: For some reason this prints types as written in source code, - // instead of fully qualified version, - // i.e: a::Bar> instead of a::Bar> - printTemplateArgumentList(OS, TSTL.getTypePtr()->template_arguments(), - TD->getASTContext().getPrintingPolicy()); - } else if (const auto *CTSD = - llvm::dyn_cast( - TSTL.getType()->getAsTagDecl())) { - - CTSD->printQualifiedName(OS); - printTemplateArgumentList(OS, CTSD->getTemplateArgs().asArray(), - CTSD->getASTContext().getPrintingPolicy()); - } - OS.flush(); - - maybeUpdateReplacement(Qualified); - return true; - } - - // Qualifies TagTypes, we are not interested in other TypeLocs since they - // can't have nested name specifiers. - bool VisitTagTypeLoc(TagTypeLoc TTL) { - const TagDecl *TD = TTL.getDecl(); - if (index::isFunctionLocalSymbol(TD)) - return true; - - // FIXME: Instead of fully qualifying, try to drop visible scopes. - maybeUpdateReplacement(TD->getQualifiedNameAsString()); - return true; - } - - bool VisitDeclRefExpr(DeclRefExpr *DRE) { - NamedDecl *ND = DRE->getFoundDecl(); - // UsingShadowDecls are considered local, but we might need to qualify them - // if the UsingDecl is not inside function body. - if (auto USD = llvm::dyn_cast(ND)) - ND = USD->getTargetDecl(); - // No need to re-write local symbols. - if (index::isFunctionLocalSymbol(ND)) - return true; - - std::string Qualified; - // For templates, in addition to decl name, also print the argument list. - if (auto *VTSD = llvm::dyn_cast(ND)) { - llvm::raw_string_ostream OS(Qualified); - VTSD->printQualifiedName(OS); - printTemplateArgumentList(OS, VTSD->getTemplateArgs().asArray(), - ND->getASTContext().getPrintingPolicy()); - } else if (auto *FTD = llvm::dyn_cast(ND)) { - llvm::raw_string_ostream OS(Qualified); - FTD->printQualifiedName(OS); - printTemplateArgumentList(OS, - llvm::dyn_cast(DRE->getDecl()) - ->getTemplateSpecializationArgs() - ->asArray(), - ND->getASTContext().getPrintingPolicy()); - } else { - Qualified = ND->getQualifiedNameAsString(); - } - maybeUpdateReplacement(Qualified); - return true; - } - - // For using decls, we chose the first shadow decls. This should not make - // any difference, since all of the shadow decls should have the same - // fully-qualified name. - bool VisitUsingDecl(UsingDecl *UD) { - flushCurrentReplacement(); - maybeUpdateCurrentRange( - SourceRange(UD->getQualifierLoc().getBeginLoc(), UD->getEndLoc())); - maybeUpdateReplacement( - UD->shadow_begin()->getTargetDecl()->getQualifiedNameAsString()); - return true; - } - - // Using-directives are "special" NamedDecls, they span the whole - // declaration and their names are not directly useful, we need to peek - // into underlying NamespaceDecl for qualified name and start location. - bool VisitUsingDirectiveDecl(UsingDirectiveDecl *UDD) { - flushCurrentReplacement(); - - SourceRange RepRange; - if (auto NNSL = UDD->getQualifierLoc()) - RepRange.setBegin(NNSL.getBeginLoc()); - else - RepRange.setBegin(UDD->getIdentLocation()); - RepRange.setEnd(UDD->getEndLoc()); - - maybeUpdateCurrentRange(RepRange); - maybeUpdateReplacement( - UDD->getNominatedNamespaceAsWritten()->getQualifiedNameAsString()); - return true; - } - - // Qualifies namespace alias decls of the form: - // using newns = ns2 -> using newns = ns1::ns2 - bool VisitNamespaceAliasDecl(NamespaceAliasDecl *NAD) { - flushCurrentReplacement(); - - SourceRange RepRange; - if (auto NNSL = NAD->getQualifierLoc()) - RepRange.setBegin(NNSL.getBeginLoc()); - else - RepRange.setBegin(NAD->getTargetNameLoc()); - RepRange.setEnd(NAD->getEndLoc()); - - maybeUpdateCurrentRange(RepRange); - maybeUpdateReplacement( - NAD->getAliasedNamespace()->getQualifiedNameAsString()); - return true; - } - - tooling::Replacements getReplacements() { - // flush the last replacement. - flushCurrentReplacement(); - return Reps; - } - -private: - // Updates the CurrentRange to NewRange if it is not overlapping with - // CurrentRange. Also flushes the replacement for CurrentRange before updating - // it. - void maybeUpdateCurrentRange(SourceRange NewRange) { - if (NewRange.isInvalid()) - return; - - if (CurrentChange && - SM.isBeforeInSLocAddrSpace( - NewRange.getBegin(), - // We use EndLoc+1 since CurrentChange also owns the token - // starting at EndLoc. - CurrentChange->Range.getEnd().getLocWithOffset(1))) { - return; - } - - flushCurrentReplacement(); - CurrentChange.emplace(); - CurrentChange->Range = NewRange; - } - - // Changes CurReplacement to contain RepText if it is longer. - void maybeUpdateReplacement(llvm::StringRef RepText) { - assert(CurrentChange && "Updating non-existing replacement"); - if (CurrentChange->Text.size() > RepText.size()) - return; - CurrentChange->Text = RepText; - } - - // Adds current replacement, the longest one for the current range, to the - // Reps. - void flushCurrentReplacement() { - if (!CurrentChange || CurrentChange->Text.empty()) - return; - SourceLocation Begin = CurrentChange->Range.getBegin(); - SourceLocation End = Lexer::getLocForEndOfToken( - CurrentChange->Range.getEnd(), 0, SM, LangOpts); - - unsigned int BeginOff = SM.getFileOffset(Begin) - BodyBegin; - unsigned int EndOff = SM.getFileOffset(End) - BodyBegin; - const tooling::Replacement Replacement("", BeginOff, EndOff - BeginOff, - CurrentChange->Text); - CurrentChange.reset(); - - if (auto Err = Reps.add(Replacement)) { - handleAllErrors(std::move(Err), [&](const llvm::ErrorInfoBase &EI) { - std::string ErrMsg; - llvm::raw_string_ostream OS(ErrMsg); - EI.log(OS); - elog("Failed to add replacement:{0}", ErrMsg); - }); - } - } - - const LangOptions LangOpts; - const SourceManager &SM; - // Used as reference points for replacements. - const unsigned int BodyBegin; - tooling::Replacements Reps; - - struct Change { - SourceRange Range; // to be replaced in the original source. - std::string Text; // to replace the range. - }; - // Change to apply for ast node currently being visited. - llvm::Optional CurrentChange; -}; - // Runs clang indexing api to get a list of declarations referenced inside // function decl. Skips local symbols. llvm::DenseSet getNonLocalDeclRefs(const FunctionDecl *FD, @@ -414,17 +170,74 @@ } // Rewrites body of FD to fully-qualify all of the decls inside. +// There are three types of spellings that needs to be qualified in a function +// body: +// - Types: Foo -> ns::Foo +// - DeclRefExpr: ns2::foo() -> ns1::ns2::foo(); +// - UsingDecls: +// using ns2::foo -> using ns1::ns2::foo +// using namespace ns2 -> using namespace ns1::ns2 +// using ns3 = ns2 -> using ns3 = ns1::ns2 +// +// Go over all references inside a function body and generate replacements to +// fully qualify those. After that, the body can be moved into an arbitrary +// file. We perform the qualification by qualyfying the last type/decl in a +// (un)qualified name. e.g: +// namespace a { namespace b { class Bar{}; void foo(); } } +// b::Bar x; -> a::b::Bar x; +// foo(); -> a::b::foo(); +// FIXME: Instead of fully qualyfying we should try deducing visible scopes at +// target location and generate minimal edits. llvm::Expected qualifyAllDecls(const FunctionDecl *FD) { + auto &Ctx = FD->getASTContext(); + auto &SM = Ctx.getSourceManager(); + + + // Compute replacements to qualify all reference in the funciton body. + tooling::Replacements Replacements; + findExplicitReferences(FD->getBody(), [&](ReferenceLoc R) { + assert(R.NameLoc.isValid()); + // Only update unqualified references. + if (R.Qualifier || R.Targets.empty()) + return; + // No need to turn `foo.x` into `foo.Class::x`. + if (R.MemberExprBase) + return; + // Make sure we have a location to add the qualifier. + if (!R.NameLoc.isFileID() || SM.getFileID(R.NameLoc) != SM.getMainFileID()) + return; + if (index::isFunctionLocalSymbol(R.Targets.front())) + return; + + std::string Qualifier; + llvm::raw_string_ostream OS(Qualifier); + R.Targets.front()->printQualifier(OS); + OS.flush(); + if (Qualifier.empty()) + return; + + if (auto Err = Replacements.add( + tooling::Replacement(SM, R.NameLoc, /*Length*/ 0, Qualifier))) + elog("Failed to add qualifier: {0}", std::move(Err)); + }); + + auto NewFile = tooling::applyAllReplacements( + SM.getBufferData(SM.getMainFileID()), Replacements); + if (!NewFile) + return NewFile.takeError(); + + // We need to return the new function body, extract it from the whole file. SourceRange OrigFuncRange = FD->getBody()->getSourceRange(); // toSourceCode expects an halfOpenRange, but FuncBody is closed. OrigFuncRange.setEnd(OrigFuncRange.getEnd().getLocWithOffset(1)); - // applyAllReplacements expects a null terminated string, therefore we make a - // copy here. - std::string OrigFuncBody = - toSourceCode(FD->getASTContext().getSourceManager(), OrigFuncRange); - - return tooling::applyAllReplacements(OrigFuncBody, - QualifyingVisitor(FD).getReplacements()); + assert(isValidFileRange(SM, OrigFuncRange)); + unsigned ShiftedBegin = Replacements.getShiftedCodePosition( + SM.getFileOffset(OrigFuncRange.getBegin())); + assert(ShiftedBegin == SM.getFileOffset(OrigFuncRange.getBegin()) && + "expected no changes before start of function body"); + unsigned ShiftedEnd = Replacements.getShiftedCodePosition( + SM.getFileOffset(OrigFuncRange.getEnd())); + return NewFile->substr(ShiftedBegin, ShiftedEnd - ShiftedBegin); } // Returns the canonical declaration for the given FunctionDecl. This will diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp --- a/clang-tools-extra/clangd/unittests/TweakTests.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp @@ -993,8 +993,8 @@ void foo() /*Comment -_-*/ { using namespace a; - a::bar >.bar(); - a::aux >(); + a::bar>.bar(); + a::aux>(); } @@ -1067,7 +1067,6 @@ template void f^oo() { Bar B; - // FIXME: This should be a::Bar > Bar> q; } )cpp", @@ -1080,8 +1079,7 @@ template void foo() /*Comment -_-*/ { a::Bar B; - // FIXME: This should be a::Bar > - a::Bar > q; + a::Bar> q; } @@ -1218,7 +1216,7 @@ void foo() /*Comment -_-*/ { a::Bar B; a::b::Foo foo; - a::Bar >::Baz > q; + a::Bar>::Baz> q; } @@ -1287,7 +1285,7 @@ a::Bar B; B.foo(); a::bar(); - a::Bar >::bar(); + a::Bar>::bar(); a::Bar::bar(); B.x = a::Bar::y; a::Bar::y = 3;