Index: lib/Tooling/Refactoring/Rename/USRFinder.cpp =================================================================== --- lib/Tooling/Refactoring/Rename/USRFinder.cpp +++ lib/Tooling/Refactoring/Rename/USRFinder.cpp @@ -48,7 +48,8 @@ SourceLocation Start = Range.getBegin(); SourceLocation End = Range.getEnd(); if (!Start.isValid() || !Start.isFileID() || !End.isValid() || - !End.isFileID() || !isPointWithin(Start, End)) + !End.isFileID() || + !Context.getSourceManager().isPointWithin(Point, Start, End)) return true; } Result = ND; @@ -58,14 +59,6 @@ const NamedDecl *getNamedDecl() const { return Result; } private: - // Determines if the Point is within Start and End. - bool isPointWithin(const SourceLocation Start, const SourceLocation End) { - // FIXME: Add tests for Point == End. - return Point == Start || Point == End || - (Context.getSourceManager().isBeforeInTranslationUnit(Start, - Point) && - Context.getSourceManager().isBeforeInTranslationUnit(Point, End)); - } const NamedDecl *Result = nullptr; const SourceLocation Point; // The location to find the NamedDecl. @@ -80,14 +73,15 @@ NamedDeclOccurrenceFindingVisitor Visitor(Point, Context); // Try to be clever about pruning down the number of top-level declarations we - // see. If both start and end is either before or after the point we're - // looking for the point cannot be inside of this decl. Don't even look at it. + // see. If the range of the declaration doesn't contain the source location, + // then don't even look at it. for (auto *CurrDecl : Context.getTranslationUnitDecl()->decls()) { - SourceLocation StartLoc = CurrDecl->getBeginLoc(); - SourceLocation EndLoc = CurrDecl->getEndLoc(); - if (StartLoc.isValid() && EndLoc.isValid() && - SM.isBeforeInTranslationUnit(StartLoc, Point) != - SM.isBeforeInTranslationUnit(EndLoc, Point)) + CharSourceRange DeclRange = Lexer::makeFileCharRange( + CharSourceRange::getTokenRange(CurrDecl->getSourceRange()), + Context.getSourceManager(), Context.getLangOpts()); + // FIXME: Use early break if the decl is found. + if (DeclRange.isValid() && + SM.isPointWithin(Point, DeclRange.getBegin(), DeclRange.getEnd())) Visitor.TraverseDecl(CurrDecl); } Index: test/Refactor/LocalRename/Inputs/MultiHeader.h =================================================================== --- /dev/null +++ test/Refactor/LocalRename/Inputs/MultiHeader.h @@ -0,0 +1,9 @@ +#ifdef HAVE_DECLS + +struct MyStruct { + struct Foo { + int field; + } bar; +}; + +#endif Index: test/Refactor/LocalRename/IsPointWithinDifferentInclude.c =================================================================== --- /dev/null +++ test/Refactor/LocalRename/IsPointWithinDifferentInclude.c @@ -0,0 +1,16 @@ +// RUN: clang-refactor local-rename -selection=%S/Inputs/MultiHeader.h:5:9 -new-name=renamedField %s -- -I %S/Inputs 2>&1 | FileCheck %s + +// The MultiHeader file included twice. +// The first inclusion contains no declarations. However, the selection +// "MultiHeader:5:9" is mapped to a location in this inclusion of the file. +#include "MultiHeader.h" + +// The second inclusion contains the declarations. The range of the +// declaration we would like is located in this inclusion. +#define HAVE_DECLS +#include "MultiHeader.h" + +// Ensure that we still find the declaration even though the location and the +// declaration's range are located in two different inclusions. +// CHECK: struct Foo { +// CHECK-NEXT: int renamedField;