Re: [PATCH] D22881: Fix NamedDeclFindingASTVisitor

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

In https://reviews.llvm.org/D22881#498919, @alexshap wrote:

> i took a look at handleNestedNameSpecifierLoc:
>
>   const auto *Decl = NameLoc.getNestedNameSpecifier()->getAsNamespace();
>   if (Decl) {
>setResult(Decl, NameLoc.getLocalBeginLoc(), NameLoc.getLocalEndLoc());
>   }
>


Yes, that's why if something happens twice in the codebase, it should be moved 
to the other place :)

> and added this check to VisitTypeLoc.

> 

> the other Visit* methods are:

>  VisitNamedDecl(const NamedDecl *Decl)

>  VisitDeclRefExpr(const DeclRefExpr *Expr)

>  VisitMemberExpr(const MemberExpr *Expr)


Yes, I am aware of that.


https://reviews.llvm.org/D22881



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22881: Fix NamedDeclFindingASTVisitor

[Alexander Shaposhnikov via cfe-commits]

alexshap added a comment.

i took a look at handleNestedNameSpecifierLoc:

const auto *Decl = NameLoc.getNestedNameSpecifier()->getAsNamespace();
if (Decl) {
 setResult(Decl, NameLoc.getLocalBeginLoc(), NameLoc.getLocalEndLoc());
}

and added this check to VisitTypeLoc.

the other Visit* methods are:
VisitNamedDecl(const NamedDecl *Decl)
VisitDeclRefExpr(const DeclRefExpr *Expr)
VisitMemberExpr(const MemberExpr *Expr)


https://reviews.llvm.org/D22881



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22881: Fix NamedDeclFindingASTVisitor

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

In https://reviews.llvm.org/D22881#498911, @alexshap wrote:

> inside setResult  Decl->getQualifiedNameAsString() is used
>  so RD should not be nullptr there.
>  we get there in the newly added test.


It is used, but then we should add this check to setResult, don't we? Otherwise 
it only works for TypeLocs.


https://reviews.llvm.org/D22881



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22881: Fix NamedDeclFindingASTVisitor

[Alexander Shaposhnikov via cfe-commits]

alexshap added a comment.

inside setResult  Decl->getQualifiedNameAsString() is used
so RD should not be nullptr there.
we get there in the newly added test.


https://reviews.llvm.org/D22881



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22881: Fix NamedDeclFindingASTVisitor

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

  if (auto *RD = Loc.getType()->getAsCXXRecordDecl())
return setResult(RD, TypeBeginLoc, TypeEndLoc);

This isn't needed, too...


https://reviews.llvm.org/D22881



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22881: Fix NamedDeclFindingASTVisitor

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

*please don't accept patches so fast


https://reviews.llvm.org/D22881



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22881: Fix NamedDeclFindingASTVisitor

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

Well, in this case `SourceMgr` should be removed...

Please accept patches so fast; at least let the others take a look at it.


https://reviews.llvm.org/D22881



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22881: Fix NamedDeclFindingASTVisitor

[Saleem Abdulrasool via cfe-commits]

compnerd closed this revision.
compnerd added a comment.

SVN r276948


https://reviews.llvm.org/D22881



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D22881: Fix NamedDeclFindingASTVisitor

[Alexander Shaposhnikov via cfe-commits]

alexshap created this revision.
alexshap added reviewers: klimek, omtcyfz.
alexshap added subscribers: compnerd, cfe-commits.
alexshap changed the visibility of this Differential Revision from "Public (No 
Login Required)" to "All Users".

Properly initialize the field Context in NamedDeclFindingASTVisitor 
constructor: 
it is dereferenced in VisitTypeLoc Context->getLangOpts(). 
Fix passing nullptr to setResult in VisitTypeLoc: 
it is dereferenced in setResult Decl->getQualifiedNameAsString().
Add a test case: 
clang-rename crashes on it without this patch, works correctly with this patch. 
All the other tests are green.

https://reviews.llvm.org/D22881

Files:
  clang-rename/USRFinder.cpp
  test/clang-rename/FunctionWithClassFindByName.cpp

Index: test/clang-rename/FunctionWithClassFindByName.cpp
===
--- test/clang-rename/FunctionWithClassFindByName.cpp
+++ test/clang-rename/FunctionWithClassFindByName.cpp
@@ -0,0 +1,12 @@
+// RUN: clang-rename -old-name=Foo -new-name=Bar %s -- | FileCheck %s
+
+void foo() {
+}
+
+class Foo { // CHECK: class Bar
+};
+
+int main() {
+  Foo *Pointer = 0; // CHECK: Bar *Pointer = 0;
+  return 0;
+}
Index: clang-rename/USRFinder.cpp
===
--- clang-rename/USRFinder.cpp
+++ clang-rename/USRFinder.cpp
@@ -42,8 +42,9 @@
   // \brief Finds the NamedDecl for a name in the source.
   // \param Name the fully qualified name.
   explicit NamedDeclFindingASTVisitor(const SourceManager &SourceMgr,
-  const std::string &Name)
-  : Result(nullptr), SourceMgr(SourceMgr), Name(Name) {}
+  const std::string &Name,
+  const ASTContext *Context)
+  : Result(nullptr), SourceMgr(SourceMgr), Name(Name), Context(Context) {}
 
   // Declaration visitors:
 
@@ -76,8 +77,9 @@
 const auto TypeBeginLoc = Loc.getBeginLoc();
 const auto TypeEndLoc = Lexer::getLocForEndOfToken(
TypeBeginLoc, 0, SourceMgr, Context->getLangOpts());
-return setResult(Loc.getType()->getAsCXXRecordDecl(), TypeBeginLoc,
- TypeEndLoc);
+if (auto *RD = Loc.getType()->getAsCXXRecordDecl())
+  return setResult(RD, TypeBeginLoc, TypeEndLoc);
+return true;
   }
 
   // Other:
@@ -170,7 +172,7 @@
 const NamedDecl *getNamedDeclFor(const ASTContext &Context,
  const std::string &Name) {
   const auto &SourceMgr = Context.getSourceManager();
-  NamedDeclFindingASTVisitor Visitor(SourceMgr, Name);
+  NamedDeclFindingASTVisitor Visitor(SourceMgr, Name, &Context);
   Visitor.TraverseDecl(Context.getTranslationUnitDecl());
 
   return Visitor.getNamedDecl();


Index: test/clang-rename/FunctionWithClassFindByName.cpp
===
--- test/clang-rename/FunctionWithClassFindByName.cpp
+++ test/clang-rename/FunctionWithClassFindByName.cpp
@@ -0,0 +1,12 @@
+// RUN: clang-rename -old-name=Foo -new-name=Bar %s -- | FileCheck %s
+
+void foo() {
+}
+
+class Foo { // CHECK: class Bar
+};
+
+int main() {
+  Foo *Pointer = 0; // CHECK: Bar *Pointer = 0;
+  return 0;
+}
Index: clang-rename/USRFinder.cpp
===
--- clang-rename/USRFinder.cpp
+++ clang-rename/USRFinder.cpp
@@ -42,8 +42,9 @@
   // \brief Finds the NamedDecl for a name in the source.
   // \param Name the fully qualified name.
   explicit NamedDeclFindingASTVisitor(const SourceManager &SourceMgr,
-  const std::string &Name)
-  : Result(nullptr), SourceMgr(SourceMgr), Name(Name) {}
+  const std::string &Name,
+  const ASTContext *Context)
+  : Result(nullptr), SourceMgr(SourceMgr), Name(Name), Context(Context) {}
 
   // Declaration visitors:
 
@@ -76,8 +77,9 @@
 const auto TypeBeginLoc = Loc.getBeginLoc();
 const auto TypeEndLoc = Lexer::getLocForEndOfToken(
TypeBeginLoc, 0, SourceMgr, Context->getLangOpts());
-return setResult(Loc.getType()->getAsCXXRecordDecl(), TypeBeginLoc,
- TypeEndLoc);
+if (auto *RD = Loc.getType()->getAsCXXRecordDecl())
+  return setResult(RD, TypeBeginLoc, TypeEndLoc);
+return true;
   }
 
   // Other:
@@ -170,7 +172,7 @@
 const NamedDecl *getNamedDeclFor(const ASTContext &Context,
  const std::string &Name) {
   const auto &SourceMgr = Context.getSourceManager();
-  NamedDeclFindingASTVisitor Visitor(SourceMgr, Name);
+  NamedDeclFindingASTVisitor Visitor(SourceMgr, Name, &Context);
   Visitor.TraverseDecl(Context.getTranslationUnitDecl());
 
   return Visitor.getNamedDecl();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.or