[PATCH] D36187: [clang-diff] Use the relative name for NamedDecls
arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LG Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:371 + std::string ContextPrefix; + if (auto *Namespace = dyn_cast(Context)) +ContextPrefix = Namespace->getQualifiedNameAsString(); johannes wrote: > arphaman wrote: > > You don't need to check both `NamespaceDecl` and `TagDecl`, since you can > > just do one if with a `NamedDecl`. > FunctionDecl should not be used for example Oh yeah, makes sense. https://reviews.llvm.org/D36187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36187: [clang-diff] Use the relative name for NamedDecls
johannes added inline comments. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:371 + std::string ContextPrefix; + if (auto *Namespace = dyn_cast(Context)) +ContextPrefix = Namespace->getQualifiedNameAsString(); arphaman wrote: > You don't need to check both `NamespaceDecl` and `TagDecl`, since you can > just do one if with a `NamedDecl`. FunctionDecl should not be used for example https://reviews.llvm.org/D36187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36187: [clang-diff] Use the relative name for NamedDecls
johannes updated this revision to Diff 111695.
johannes added a comment.
only use Enums as namespace prefix in C++11
https://reviews.llvm.org/D36187
Files:
include/clang/Tooling/ASTDiff/ASTDiff.h
lib/Tooling/ASTDiff/ASTDiff.cpp
test/Tooling/clang-diff-ast.cpp
test/Tooling/clang-diff-basic.cpp
test/Tooling/clang-diff-html.test
test/Tooling/clang-diff-topdown.cpp
Index: test/Tooling/clang-diff-topdown.cpp
===
--- test/Tooling/clang-diff-topdown.cpp
+++ test/Tooling/clang-diff-topdown.cpp
@@ -27,8 +27,19 @@
{{;;}}
}
+int x;
+
+namespace src {
+ int x;
+ int x1 = x + 1;
+ int x2 = ::x + 1;
+}
+
+class A { int x = 1 + 1; void f() { int x1 = x; } };
+
#else
+
void f1() {
{{;}}
@@ -45,4 +56,28 @@
;
}
+int x;
+
+namespace dst {
+ int x;
+ // CHECK: Match DeclRefExpr: :x(17) to DeclRefExpr: :x(22)
+ int x1 = x + 1;
+ // CHECK: Match DeclRefExpr: x(21) to DeclRefExpr: x(26)
+ int x2 = ::x + 1;
+}
+
+class B {
+ // Only the class name changed; it is not included in the field value,
+ // therefore there is no update.
+ // CHECK: Match FieldDecl: :x(int)(24) to FieldDecl: :x(int)(29)
+ // CHECK-NOT: Update FieldDecl: :x(int)(24)
+ int x = 1+1;
+ void f() {
+// CHECK: Match MemberExpr: :x(32) to MemberExpr: :x(37)
+// CHECK-NOT: Update MemberExpr: :x(32)
+int x1 = B::x;
+ }
+
+};
+
#endif
Index: test/Tooling/clang-diff-html.test
===
--- test/Tooling/clang-diff-html.test
+++ test/Tooling/clang-diff-html.test
@@ -11,12 +11,12 @@
// match, move
// CHECK: void foo()
+// CHECK-NEXT: :foo(void ())' class='m'>void foo()
// match
// CHECK: void main()
+// CHECK-NEXT: :main(void ())'>void main()
// deletion
// CHECK: (Context))
+ContextPrefix = Namespace->getQualifiedNameAsString();
+ else if (auto *Record = dyn_cast(Context))
+ContextPrefix = Record->getQualifiedNameAsString();
+ else if (AST.getLangOpts().CPlusPlus11)
+if (auto *Tag = dyn_cast(Context))
+ ContextPrefix = Tag->getQualifiedNameAsString();
+ std::string Val = ND->getQualifiedNameAsString();
+ // Strip the qualifier, if Val refers to somthing in the current scope.
+ // But leave one leading ':' in place, so that we know that this is a
+ // relative path.
+ if (!ContextPrefix.empty() && StringRef(Val).startswith(ContextPrefix))
+Val = Val.substr(ContextPrefix.size() + 1);
+ return Val;
+}
+
+std::string SyntaxTree::Impl::getRelativeName(const NamedDecl *ND) const {
+ return getRelativeName(ND, ND->getDeclContext());
+}
+
+static const DeclContext *getEnclosingDeclContext(ASTContext ,
+ const Stmt *S) {
+ while (S) {
+const auto = AST.getParents(*S);
+if (Parents.empty())
+ return nullptr;
+const auto = Parents[0];
+if (const auto *D = P.get())
+ return D->getDeclContext();
+S = P.get();
+ }
+ llvm_unreachable("Could not find Decl ancestor.");
+}
+
std::string SyntaxTree::Impl::getNodeValue(NodeId Id) const {
return getNodeValue(getNode(Id));
}
@@ -384,8 +426,7 @@
TypePP.AnonymousTagLocations = false;
if (auto *V = dyn_cast(D)) {
-Value += V->getQualifiedNameAsString() + "(" +
- V->getType().getAsString(TypePP) + ")";
+Value += getRelativeName(V) + "(" + V->getType().getAsString(TypePP) + ")";
if (auto *C = dyn_cast(D)) {
for (auto *Init : C->inits()) {
if (!Init->isWritten())
@@ -397,15 +438,15 @@
Value += C->getNameAsString();
} else {
assert(Init->isAnyMemberInitializer());
- Value += Init->getMember()->getQualifiedNameAsString();
+ Value += getRelativeName(Init->getMember());
}
Value += ",";
}
}
return Value;
}
if (auto *N = dyn_cast(D))
-Value += N->getQualifiedNameAsString() + ";";
+Value += getRelativeName(N) + ";";
if (auto *T = dyn_cast(D))
return Value + T->getUnderlyingType().getAsString(TypePP) + ";";
if (auto *T = dyn_cast(D))
@@ -429,7 +470,7 @@
if (auto *B = dyn_cast(S))
return B->getOpcodeStr();
if (auto *M = dyn_cast(S))
-return M->getMemberDecl()->getQualifiedNameAsString();
+return getRelativeName(M->getMemberDecl());
if (auto *I = dyn_cast(S)) {
SmallString<256> Str;
I->getValue().toString(Str, /*Radix=*/10, /*Signed=*/false);
@@ -441,7 +482,7 @@
return Str.str();
}
if (auto *D = dyn_cast(S))
-return D->getDecl()->getQualifiedNameAsString();
+return getRelativeName(D->getDecl(), getEnclosingDeclContext(AST, S));
if (auto *String = dyn_cast(S))
return String->getString();
if (auto *B = dyn_cast(S))
@@ -950,7 +991,7 @@
return DiffImpl->getMapped(SourceTree.TreeImpl, Id);
}
-SyntaxTree::SyntaxTree(const ASTContext )
+SyntaxTree::SyntaxTree(ASTContext )
: TreeImpl(llvm::make_unique(
this,
[PATCH] D36187: [clang-diff] Use the relative name for NamedDecls
arphaman added inline comments. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:371 + std::string ContextPrefix; + if (auto *Namespace = dyn_cast(Context)) +ContextPrefix = Namespace->getQualifiedNameAsString(); You don't need to check both `NamespaceDecl` and `TagDecl`, since you can just do one if with a `NamedDecl`. https://reviews.llvm.org/D36187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36187: [clang-diff] Use the relative name for NamedDecls
johannes updated this revision to Diff 110962.
johannes added a comment.
rebase
https://reviews.llvm.org/D36187
Files:
include/clang/Tooling/ASTDiff/ASTDiff.h
lib/Tooling/ASTDiff/ASTDiff.cpp
test/Tooling/clang-diff-ast.cpp
test/Tooling/clang-diff-basic.cpp
test/Tooling/clang-diff-html.test
test/Tooling/clang-diff-topdown.cpp
Index: test/Tooling/clang-diff-topdown.cpp
===
--- test/Tooling/clang-diff-topdown.cpp
+++ test/Tooling/clang-diff-topdown.cpp
@@ -27,8 +27,19 @@
{{;;}}
}
+int x;
+
+namespace src {
+ int x;
+ int x1 = x + 1;
+ int x2 = ::x + 1;
+}
+
+class A { int x = 1 + 1; void f() { int x1 = x; } };
+
#else
+
void f1() {
{{;}}
@@ -45,4 +56,28 @@
;
}
+int x;
+
+namespace dst {
+ int x;
+ // CHECK: Match DeclRefExpr: :x(17) to DeclRefExpr: :x(22)
+ int x1 = x + 1;
+ // CHECK: Match DeclRefExpr: x(21) to DeclRefExpr: x(26)
+ int x2 = ::x + 1;
+}
+
+class B {
+ // Only the class name changed; it is not included in the field value,
+ // therefore there is no update.
+ // CHECK: Match FieldDecl: :x(int)(24) to FieldDecl: :x(int)(29)
+ // CHECK-NOT: Update FieldDecl: :x(int)(24)
+ int x = 1+1;
+ void f() {
+// CHECK: Match MemberExpr: :x(32) to MemberExpr: :x(37)
+// CHECK-NOT: Update MemberExpr: :x(32)
+int x1 = B::x;
+ }
+
+};
+
#endif
Index: test/Tooling/clang-diff-html.test
===
--- test/Tooling/clang-diff-html.test
+++ test/Tooling/clang-diff-html.test
@@ -11,12 +11,12 @@
// match, move
// CHECK: void foo()
+// CHECK-NEXT: :foo(void ())' class='m'>void foo()
// match
// CHECK: void main()
+// CHECK-NEXT: :main(void ())'>void main()
// deletion
// CHECK: (Context))
+ContextPrefix = Namespace->getQualifiedNameAsString();
+ else if (auto *Tag = dyn_cast(Context))
+ContextPrefix = Tag->getQualifiedNameAsString();
+ std::string Val = ND->getQualifiedNameAsString();
+ // Strip the qualifier, if Val refers to somthing in the current scope.
+ // But leave one leading ':' in place, so that we know that this is a
+ // relative path.
+ if (!ContextPrefix.empty() &&
+ StringRef(Val).startswith(ContextPrefix))
+Val = Val.substr(ContextPrefix.size() + 1);
+ return Val;
+}
+
+static std::string getRelativeName(const NamedDecl *ND) {
+ return getRelativeName(ND, ND->getDeclContext());
+}
+
+static const DeclContext *getEnclosingDeclContext(ASTContext ,
+ const Stmt *S) {
+ while (S) {
+const auto = AST.getParents(*S);
+if (Parents.empty())
+ return nullptr;
+const auto = Parents[0];
+if (const auto *D = P.get())
+ return D->getDeclContext();
+S = P.get();
+ }
+ llvm_unreachable("Could not find Decl ancestor.");
+}
+
std::string SyntaxTree::Impl::getNodeValue(NodeId Id) const {
return getNodeValue(getNode(Id));
}
@@ -384,8 +419,7 @@
TypePP.AnonymousTagLocations = false;
if (auto *V = dyn_cast(D)) {
-Value += V->getQualifiedNameAsString() + "(" +
- V->getType().getAsString(TypePP) + ")";
+Value += getRelativeName(V) + "(" + V->getType().getAsString(TypePP) + ")";
if (auto *C = dyn_cast(D)) {
for (auto *Init : C->inits()) {
if (!Init->isWritten())
@@ -397,15 +431,15 @@
Value += C->getNameAsString();
} else {
assert(Init->isAnyMemberInitializer());
- Value += Init->getMember()->getQualifiedNameAsString();
+ Value += getRelativeName(Init->getMember());
}
Value += ",";
}
}
return Value;
}
if (auto *N = dyn_cast(D))
-Value += N->getQualifiedNameAsString() + ";";
+Value += getRelativeName(N) + ";";
if (auto *T = dyn_cast(D))
return Value + T->getUnderlyingType().getAsString(TypePP) + ";";
if (auto *T = dyn_cast(D))
@@ -429,7 +463,7 @@
if (auto *B = dyn_cast(S))
return B->getOpcodeStr();
if (auto *M = dyn_cast(S))
-return M->getMemberDecl()->getQualifiedNameAsString();
+return getRelativeName(M->getMemberDecl());
if (auto *I = dyn_cast(S)) {
SmallString<256> Str;
I->getValue().toString(Str, /*Radix=*/10, /*Signed=*/false);
@@ -441,7 +475,7 @@
return Str.str();
}
if (auto *D = dyn_cast(S))
-return D->getDecl()->getQualifiedNameAsString();
+return getRelativeName(D->getDecl(), getEnclosingDeclContext(AST, S));
if (auto *String = dyn_cast(S))
return String->getString();
if (auto *B = dyn_cast(S))
@@ -950,7 +984,7 @@
return DiffImpl->getMapped(SourceTree.TreeImpl, Id);
}
-SyntaxTree::SyntaxTree(const ASTContext )
+SyntaxTree::SyntaxTree(ASTContext )
: TreeImpl(llvm::make_unique(
this, AST.getTranslationUnitDecl(), AST)) {}
Index: include/clang/Tooling/ASTDiff/ASTDiff.h
===
---
[PATCH] D36187: [clang-diff] Use the relative name for NamedDecls
arphaman added a comment. Please rebase, it doesn't apply cleanly anymore. https://reviews.llvm.org/D36187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36187: [clang-diff] Use the relative name for NamedDecls
johannes updated this revision to Diff 110557.
johannes added a comment.
substr
https://reviews.llvm.org/D36187
Files:
include/clang/Tooling/ASTDiff/ASTDiff.h
lib/Tooling/ASTDiff/ASTDiff.cpp
test/Tooling/clang-diff-ast.cpp
test/Tooling/clang-diff-basic.cpp
test/Tooling/clang-diff-html.test
test/Tooling/clang-diff-topdown.cpp
Index: test/Tooling/clang-diff-topdown.cpp
===
--- test/Tooling/clang-diff-topdown.cpp
+++ test/Tooling/clang-diff-topdown.cpp
@@ -27,8 +27,19 @@
{{;;}}
}
+int x;
+
+namespace src {
+ int x;
+ int x1 = x + 1;
+ int x2 = ::x + 1;
+}
+
+class A { int x = 1 + 1; void f() { int x1 = x; } };
+
#else
+
void f1() {
{{;}}
@@ -45,4 +56,28 @@
;
}
+int x;
+
+namespace dst {
+ int x;
+ // CHECK: Match DeclRefExpr: :x(17) to DeclRefExpr: :x(22)
+ int x1 = x + 1;
+ // CHECK: Match DeclRefExpr: x(21) to DeclRefExpr: x(26)
+ int x2 = ::x + 1;
+}
+
+class B {
+ // Only the class name changed; it is not included in the field value,
+ // therefore there is no update.
+ // CHECK: Match FieldDecl: :x(int)(24) to FieldDecl: :x(int)(29)
+ // CHECK-NOT: Update FieldDecl: :x(int)(24)
+ int x = 1+1;
+ void f() {
+// CHECK: Match MemberExpr: :x(32) to MemberExpr: :x(37)
+// CHECK-NOT: Update MemberExpr: :x(32)
+int x1 = B::x;
+ }
+
+};
+
#endif
Index: test/Tooling/clang-diff-html.test
===
--- test/Tooling/clang-diff-html.test
+++ test/Tooling/clang-diff-html.test
@@ -11,12 +11,12 @@
// match, move
// CHECK: void foo()
+// CHECK-NEXT: :foo(void ())' class='m'>void foo()
// match
// CHECK: void main()
+// CHECK-NEXT: :main(void ())'>void main()
// deletion
// CHECK: (Context))
+ContextPrefix = Namespace->getQualifiedNameAsString();
+ else if (auto *Tag = dyn_cast(Context))
+ContextPrefix = Tag->getQualifiedNameAsString();
+ std::string Val = ND->getQualifiedNameAsString();
+ // Strip the qualifier, if Val refers to somthing in the current scope.
+ // But leave one leading ':' in place, so that we know that this is a
+ // relative path.
+ if (!ContextPrefix.empty() &&
+ StringRef(Val).startswith(ContextPrefix))
+Val = Val.substr(ContextPrefix.size() + 1);
+ return Val;
+}
+
+static std::string getRelativeName(const NamedDecl *ND) {
+ return getRelativeName(ND, ND->getDeclContext());
+}
+
+static const DeclContext *getEnclosingDeclContext(ASTContext ,
+ const Stmt *S) {
+ while (S) {
+const auto = AST.getParents(*S);
+if (Parents.empty())
+ return nullptr;
+const auto = Parents[0];
+if (const auto *D = P.get())
+ return D->getDeclContext();
+S = P.get();
+ }
+ llvm_unreachable("Could not find Decl ancestor.");
+}
+
std::string SyntaxTree::Impl::getNodeValue(NodeId Id) const {
return getNodeValue(getNode(Id));
}
@@ -384,8 +419,7 @@
TypePP.AnonymousTagLocations = false;
if (auto *V = dyn_cast(D)) {
-Value += V->getQualifiedNameAsString() + "(" +
- V->getType().getAsString(TypePP) + ")";
+Value += getRelativeName(V) + "(" + V->getType().getAsString(TypePP) + ")";
if (auto *C = dyn_cast(D)) {
for (auto *Init : C->inits()) {
if (!Init->isWritten())
@@ -398,14 +432,14 @@
Value += C->getNameAsString() + ",";
} else {
assert(Init->isAnyMemberInitializer());
- Value += Init->getMember()->getQualifiedNameAsString() + ",";
+ Value += getRelativeName(Init->getMember()) + ",";
}
}
}
return Value;
}
if (auto *N = dyn_cast(D))
-Value += N->getQualifiedNameAsString() + ";";
+Value += getRelativeName(N) + ";";
if (auto *T = dyn_cast(D))
return Value + T->getUnderlyingType().getAsString(TypePP) + ";";
if (auto *T = dyn_cast(D))
@@ -429,7 +463,7 @@
if (auto *B = dyn_cast(S))
return B->getOpcodeStr();
if (auto *M = dyn_cast(S))
-return M->getMemberDecl()->getQualifiedNameAsString();
+return getRelativeName(M->getMemberDecl());
if (auto *I = dyn_cast(S)) {
SmallString<256> Str;
I->getValue().toString(Str, /*Radix=*/10, /*Signed=*/false);
@@ -441,7 +475,7 @@
return Str.str();
}
if (auto *D = dyn_cast(S))
-return D->getDecl()->getQualifiedNameAsString();
+return getRelativeName(D->getDecl(), getEnclosingDeclContext(AST, S));
if (auto *String = dyn_cast(S))
return String->getString();
if (auto *B = dyn_cast(S))
@@ -946,7 +980,7 @@
return DiffImpl->getMapped(SourceTree.TreeImpl, Id);
}
-SyntaxTree::SyntaxTree(const ASTContext )
+SyntaxTree::SyntaxTree(ASTContext )
: TreeImpl(llvm::make_unique(
this, AST.getTranslationUnitDecl(), AST)) {}
Index: include/clang/Tooling/ASTDiff/ASTDiff.h
===
--- include/clang/Tooling/ASTDiff/ASTDiff.h
+++
[PATCH] D36187: [clang-diff] Use the relative name for NamedDecls
bkramer added a comment. There's some similar code in tools/clang/lib/Tooling/Core/Lookup.cpp, it might make sense to share it. Otherwise this looks good. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:391 + if (!ContextPrefix.empty() && + Val.substr(0, ContextPrefix.size()) == ContextPrefix) +Val = Val.substr(ContextPrefix.size() + 1); Val.startswith(ContextPrefix) https://reviews.llvm.org/D36187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
