[PATCH] D34329: [clang-diff] Initial implementation.

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

@johannes Someone asked for that in Debian: 
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=907269


Repository:
  rL LLVM

https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes added a comment.

In https://reviews.llvm.org/D34329#1213667, @sylvestre.ledru wrote:

> @arphaman @johannes Is that normal that clang-diff isn't installed by cmake? 
> (like clang-format?)


Yes, we did not add that. I don't know if anyone would use it.


Repository:
  rL LLVM

https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.
Herald added subscribers: llvm-commits, kadircet, mgrang.

@arphaman @johannes Is that normal that clang-diff isn't installed by cmake? 
(like clang-format?)


Repository:
  rL LLVM

https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL308731: [clang-diff] Add initial implementation (authored by 
arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D34329?vs=105161=107661#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34329

Files:
  cfe/trunk/include/clang/Tooling/ASTDiff/ASTDiff.h
  cfe/trunk/include/clang/Tooling/ASTDiff/ASTDiffInternal.h
  cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp
  cfe/trunk/lib/Tooling/ASTDiff/CMakeLists.txt
  cfe/trunk/lib/Tooling/CMakeLists.txt
  cfe/trunk/test/Tooling/clang-diff-basic.cpp
  cfe/trunk/tools/CMakeLists.txt
  cfe/trunk/tools/clang-diff/CMakeLists.txt
  cfe/trunk/tools/clang-diff/ClangDiff.cpp

Index: cfe/trunk/include/clang/Tooling/ASTDiff/ASTDiff.h
===
--- cfe/trunk/include/clang/Tooling/ASTDiff/ASTDiff.h
+++ cfe/trunk/include/clang/Tooling/ASTDiff/ASTDiff.h
@@ -0,0 +1,108 @@
+//===- ASTDiff.h - AST differencing API ---*- C++ -*- -===//
+//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file specifies an interface that can be used to compare C++ syntax
+// trees.
+//
+// We use the gumtree algorithm which combines a heuristic top-down search that
+// is able to match large subtrees that are equivalent, with an optimal
+// algorithm to match small subtrees.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_ASTDIFF_ASTDIFF_H
+#define LLVM_CLANG_TOOLING_ASTDIFF_ASTDIFF_H
+
+#include "clang/Tooling/ASTDiff/ASTDiffInternal.h"
+
+namespace clang {
+namespace diff {
+
+class SyntaxTree;
+
+class ASTDiff {
+public:
+  ASTDiff(SyntaxTree , SyntaxTree , const ComparisonOptions );
+  ~ASTDiff();
+
+  // Returns a list of matches.
+  std::vector getMatches();
+  /// Returns an edit script.
+  std::vector getChanges();
+
+  // Prints an edit action.
+  void printChange(raw_ostream , const Change ) const;
+  // Prints a match between two nodes.
+  void printMatch(raw_ostream , const Match ) const;
+
+  class Impl;
+
+private:
+  std::unique_ptr DiffImpl;
+};
+
+/// SyntaxTree objects represent subtrees of the AST.
+/// They can be constructed from any Decl or Stmt.
+class SyntaxTree {
+public:
+  /// Constructs a tree from a translation unit.
+  SyntaxTree(const ASTContext );
+  /// Constructs a tree from any AST node.
+  template 
+  SyntaxTree(T *Node, const ASTContext )
+  : TreeImpl(llvm::make_unique(this, Node, AST)) {}
+
+  /// Serialize the node attributes to a string representation. This should
+  /// uniquely distinguish nodes of the same kind. Note that this function just
+  /// returns a representation of the node value, not considering descendants.
+  std::string getNodeValue(const DynTypedNode ) const;
+
+  void printAsJson(raw_ostream );
+
+  std::unique_ptr TreeImpl;
+};
+
+struct ComparisonOptions {
+  /// During top-down matching, only consider nodes of at least this height.
+  int MinHeight = 2;
+
+  /// During bottom-up matching, match only nodes with at least this value as
+  /// the ratio of their common descendants.
+  double MinSimilarity = 0.2;
+
+  /// Whenever two subtrees are matched in the bottom-up phase, the optimal
+  /// mapping is computed, unless the size of either subtrees exceeds this.
+  int MaxSize = 100;
+
+  /// If this is set to true, nodes that have parents that must not be matched
+  /// (see NodeComparison) will be allowed to be matched.
+  bool EnableMatchingWithUnmatchableParents = false;
+
+  /// Returns false if the nodes should never be matched.
+  bool isMatchingAllowed(const DynTypedNode , const DynTypedNode ) const {
+return N1.getNodeKind().isSame(N2.getNodeKind());
+  }
+
+  /// Returns zero if the nodes are considered to be equal.  Returns a value
+  /// indicating the editing distance between the nodes otherwise.
+  /// There is no need to consider nodes that cannot be matched as input for
+  /// this function (see isMatchingAllowed).
+  double getNodeDistance(const SyntaxTree , const DynTypedNode ,
+ const SyntaxTree , const DynTypedNode ) const {
+if (T1.getNodeValue(N1) == T2.getNodeValue(N2))
+  return 0;
+return 1;
+  }
+};
+
+} // end namespace diff
+} // end namespace clang
+
+#endif
Index: cfe/trunk/include/clang/Tooling/ASTDiff/ASTDiffInternal.h
===
--- cfe/trunk/include/clang/Tooling/ASTDiff/ASTDiffInternal.h
+++ cfe/trunk/include/clang/Tooling/ASTDiff/ASTDiffInternal.h
@@ -0,0 +1,192 @@
+//===- ASTDiffInternal.h --*- C++ -*- -===//
+//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is 

[PATCH] D34329: [clang-diff] Initial implementation.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

I'll commit this on behalf of Johannes today as he didn't get his access yet


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

@johannes https://reviews.llvm.org/D34880 has landed, so feel free to propose 
patches to the StmtDataCollector API that would help you (e.g. to support 
identifiers). You can see examples how to use it in the CloneDetection.cpp 
(once for storing data in a FoldingetSetNodeID and once for directly hashing 
the data with MD5).


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.

LGTM. You can request commit access at 
http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access.


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes updated this revision to Diff 105161.
johannes added a comment.

- style fixes
- correct getSimilarity()


https://reviews.llvm.org/D34329

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  include/clang/Tooling/ASTDiff/ASTDiffInternal.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  lib/Tooling/ASTDiff/CMakeLists.txt
  lib/Tooling/CMakeLists.txt
  test/Tooling/clang-diff-basic.cpp
  tools/CMakeLists.txt
  tools/clang-diff/CMakeLists.txt
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- /dev/null
+++ tools/clang-diff/ClangDiff.cpp
@@ -0,0 +1,110 @@
+//===- ClangDiff.cpp - compare source files by AST nodes --*- C++ -*- -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file implements a tool for syntax tree based comparison using
+// Tooling/ASTDiff.
+//
+//===--===//
+
+#include "clang/Tooling/ASTDiff/ASTDiff.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::tooling;
+
+static cl::OptionCategory ClangDiffCategory("clang-diff options");
+
+static cl::opt
+DumpAST("ast-dump",
+cl::desc("Print the internal representation of the AST as JSON."),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt NoCompilationDatabase(
+"no-compilation-database",
+cl::desc(
+"Do not attempt to load build settings from a compilation database"),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt SourcePath(cl::Positional, cl::desc(""),
+   cl::Required,
+   cl::cat(ClangDiffCategory));
+
+static cl::opt DestinationPath(cl::Positional,
+cl::desc(""),
+cl::Optional,
+cl::cat(ClangDiffCategory));
+
+static std::unique_ptr getAST(const StringRef Filename) {
+  std::string ErrorMessage;
+  std::unique_ptr Compilations;
+  if (!NoCompilationDatabase)
+Compilations =
+CompilationDatabase::autoDetectFromSource(Filename, ErrorMessage);
+  if (!Compilations) {
+if (!NoCompilationDatabase)
+  llvm::errs()
+  << "Error while trying to load a compilation database, running "
+ "without flags.\n"
+  << ErrorMessage;
+Compilations = llvm::make_unique(
+".", std::vector());
+  }
+  std::array Files = {{Filename}};
+  ClangTool Tool(*Compilations, Files);
+  std::vector ASTs;
+  Tool.buildASTs(ASTs);
+  if (ASTs.size() != Files.size())
+return nullptr;
+  return std::move(ASTs[0]);
+}
+
+int main(int argc, const char **argv) {
+  cl::HideUnrelatedOptions(ClangDiffCategory);
+  if (!cl::ParseCommandLineOptions(argc, argv)) {
+cl::PrintOptionValues();
+return 1;
+  }
+
+  if (DumpAST) {
+if (!DestinationPath.empty()) {
+  llvm::errs() << "Error: Please specify exactly one filename.\n";
+  return 1;
+}
+std::unique_ptr AST = getAST(SourcePath);
+if (!AST)
+  return 1;
+diff::SyntaxTree Tree(AST->getASTContext());
+Tree.printAsJson(llvm::outs());
+return 0;
+  }
+
+  if (DestinationPath.empty()) {
+llvm::errs() << "Error: Exactly two paths are required.\n";
+return 1;
+  }
+
+  std::unique_ptr Src = getAST(SourcePath);
+  std::unique_ptr Dst = getAST(DestinationPath);
+  if (!Src || !Dst)
+return 1;
+
+  diff::ComparisonOptions Options;
+  diff::SyntaxTree SrcTree(Src->getASTContext());
+  diff::SyntaxTree DstTree(Dst->getASTContext());
+  diff::ASTDiff DiffTool(SrcTree, DstTree, Options);
+  for (const auto  : DiffTool.getMatches())
+DiffTool.printMatch(llvm::outs(), Match);
+  for (const auto  : DiffTool.getChanges())
+DiffTool.printChange(llvm::outs(), Change);
+
+  return 0;
+}
Index: tools/clang-diff/CMakeLists.txt
===
--- /dev/null
+++ tools/clang-diff/CMakeLists.txt
@@ -0,0 +1,13 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_executable(clang-diff
+  ClangDiff.cpp
+  )
+
+target_link_libraries(clang-diff
+  clangFrontend
+  clangTooling
+  clangToolingASTDiff
+  )
Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -2,6 +2,7 @@
 
 add_clang_subdirectory(diagtool)
 add_clang_subdirectory(driver)
+add_clang_subdirectory(clang-diff)
 add_clang_subdirectory(clang-format)
 

[PATCH] D34329: [clang-diff] Initial implementation.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: include/clang/Tooling/ASTDiff/ASTDiffInternal.h:38
+  operator int() const { return Id; }
+  NodeId ++() { return ++this->Id, *this; }
+  NodeId () { return --this->Id, *this; }

NIT: You don't need `this` here or in `--` as well.


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:359
+
+  explicit SNodeId(int Id) : Id(Id){};
+  explicit SNodeId() = default;

NIT: This ';' is redundant.



Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:363
+  operator int() const { return Id; }
+  SNodeId ++() { return ++this->Id, *this; }
+  SNodeId () { return --this->Id, *this; }

NIT: You don't need 'this' here or in the 2 methods below.



Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:809
+NodeId P1 = M.getSrc(P2);
+assert(P1.isValid());
+Node  = T1.getMutableNode(P1);

Missing assertion message.


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:385
+  }
+  int getSizeS() const { return RootIds.size(); }
+  NodeId getIdInRoot(SNodeId Id) const {

What's the purpose of the `S` prefix in the name of this method and a couple of 
other methods below? Can you replace it with something more descriptive or 
remove it altogether?


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes added a comment.

In https://reviews.llvm.org/D34329#798377, @arphaman wrote:

> @johannes 
>  Are you planning to work on integration with the `StmtDataCollector` in this 
> patch or would you prefer to follow-up with additional patches?


Later would be better


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

@johannes 
Are you planning to work on integration with the `StmtDataCollector` in this 
patch or would you prefer to follow-up with additional patches?


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes updated this revision to Diff 105059.
johannes edited the summary of this revision.

https://reviews.llvm.org/D34329

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  include/clang/Tooling/ASTDiff/ASTDiffInternal.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  lib/Tooling/ASTDiff/CMakeLists.txt
  lib/Tooling/CMakeLists.txt
  test/Tooling/clang-diff-basic.cpp
  tools/CMakeLists.txt
  tools/clang-diff/CMakeLists.txt
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- /dev/null
+++ tools/clang-diff/ClangDiff.cpp
@@ -0,0 +1,110 @@
+//===- ClangDiff.cpp - compare source files by AST nodes --*- C++ -*- -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file implements a tool for syntax tree based comparison using
+// Tooling/ASTDiff.
+//
+//===--===//
+
+#include "clang/Tooling/ASTDiff/ASTDiff.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::tooling;
+
+static cl::OptionCategory ClangDiffCategory("clang-diff options");
+
+static cl::opt
+DumpAST("ast-dump",
+cl::desc("Print the internal representation of the AST as JSON."),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt NoCompilationDatabase(
+"no-compilation-database",
+cl::desc(
+"Do not attempt to load build settings from a compilation database"),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt SourcePath(cl::Positional, cl::desc(""),
+   cl::Required,
+   cl::cat(ClangDiffCategory));
+
+static cl::opt DestinationPath(cl::Positional,
+cl::desc(""),
+cl::Optional,
+cl::cat(ClangDiffCategory));
+
+static std::unique_ptr getAST(const StringRef Filename) {
+  std::string ErrorMessage;
+  std::unique_ptr Compilations;
+  if (!NoCompilationDatabase)
+Compilations =
+CompilationDatabase::autoDetectFromSource(Filename, ErrorMessage);
+  if (!Compilations) {
+if (!NoCompilationDatabase)
+  llvm::errs()
+  << "Error while trying to load a compilation database, running "
+ "without flags.\n"
+  << ErrorMessage;
+Compilations = llvm::make_unique(
+".", std::vector());
+  }
+  std::array Files = {{Filename}};
+  ClangTool Tool(*Compilations, Files);
+  std::vector ASTs;
+  Tool.buildASTs(ASTs);
+  if (ASTs.size() != Files.size())
+return nullptr;
+  return std::move(ASTs[0]);
+}
+
+int main(int argc, const char **argv) {
+  cl::HideUnrelatedOptions(ClangDiffCategory);
+  if (!cl::ParseCommandLineOptions(argc, argv)) {
+cl::PrintOptionValues();
+return 1;
+  }
+
+  if (DumpAST) {
+if (!DestinationPath.empty()) {
+  llvm::errs() << "Error: Please specify exactly one filename.\n";
+  return 1;
+}
+std::unique_ptr AST = getAST(SourcePath);
+if (!AST)
+  return 1;
+diff::SyntaxTree Tree(AST->getASTContext());
+Tree.printAsJson(llvm::outs());
+return 0;
+  }
+
+  if (DestinationPath.empty()) {
+llvm::errs() << "Error: Exactly two paths are required.\n";
+return 1;
+  }
+
+  std::unique_ptr Src = getAST(SourcePath);
+  std::unique_ptr Dst = getAST(DestinationPath);
+  if (!Src || !Dst)
+return 1;
+
+  diff::ComparisonOptions Options;
+  diff::SyntaxTree SrcTree(Src->getASTContext());
+  diff::SyntaxTree DstTree(Dst->getASTContext());
+  diff::ASTDiff DiffTool(SrcTree, DstTree, Options);
+  for (const auto  : DiffTool.getMatches())
+DiffTool.printMatch(llvm::outs(), Match);
+  for (const auto  : DiffTool.getChanges())
+DiffTool.printChange(llvm::outs(), Change);
+
+  return 0;
+}
Index: tools/clang-diff/CMakeLists.txt
===
--- /dev/null
+++ tools/clang-diff/CMakeLists.txt
@@ -0,0 +1,13 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_executable(clang-diff
+  ClangDiff.cpp
+  )
+
+target_link_libraries(clang-diff
+  clangFrontend
+  clangTooling
+  clangToolingASTDiff
+  )
Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -2,6 +2,7 @@
 
 add_clang_subdirectory(diagtool)
 add_clang_subdirectory(driver)
+add_clang_subdirectory(clang-diff)
 add_clang_subdirectory(clang-format)
 add_clang_subdirectory(clang-format-vs)
 

[PATCH] D34329: [clang-diff] Initial implementation.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

Yes, it does indeed skip identifiers and literals for this reason :). It was 
planned to make this template more configurable for use cases like yours, so 
I'm totally fine with adding configuration parameters. I just opened 
https://reviews.llvm.org/D34880 where I make this template public as a first 
step.


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes marked an inline comment as done.
johannes added a comment.

In https://reviews.llvm.org/D34329#796574, @teemperor wrote:

> I didn't have time to have a close look at this patch, but it seems you're 
> interested in the specific TU-independent data of a Stmt to compare them. So 
> if you are interested in such data and don't want to write your own function 
> to collect it for each Stmt subclass, there is the `StmtDataCollector` in the 
> CloneDetection.cpp here 
>  and a 
> example how to use it is here 
> .
>
> I'm fine with moving this into a header and make it more usable for this use 
> case if you think it makes sense to use it here. The main advantage would be 
> that we don't get yet another of these classes in clang (we already have 3 of 
> them: One in the Stmt profiler code, another one in the ODRHash code for 
> modules and another one in the CloneDetection code).


Looking at this again now, it seems to make sense to consolidate this. It 
collects a lot of data I currently don't.
Does it include the values of literals / identifiers? It seems not (since it is 
geared towards detecting structural clones only).  So we could make that 
optional, or I could do that myself.


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

I didn't have time to have a close look at this patch, but it seems you're 
interested in the specific TU-independent data of a Stmt to compare them. So if 
you are interested in the such data and don't want to write your own function 
to collect data it for each Stmt subclass, there is the `StmtDataCollector` in 
the CloneDetection.cpp here 
 and a 
example how to use it is here 
.

I'm fine with moving this into a header and make it more usable for this use 
case if you think it makes sense to use it here. The main advantage would be 
that we don't get yet another of these classes in clang (we already have 3 of 
them: One in the Stmt profiler code, another one in the ODRHash code for 
modules and another one in the CloneDetection code).


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

I think that it's pretty much ready. I think that the test should be expanded 
though. At the very least it should check that all of the node types that are 
supported by `SyntaxTreeImpl::getNodeValueImpl` get matched.


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes marked 2 inline comments as done.
johannes added inline comments.



Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:439
+// Computes an optimal mapping between two trees.
+class ZsMatcher {
+  const ASTDiff::Impl 

arphaman wrote:
> Do you know the reason for the `ZsMatcher` name? I noticed that they used it 
> in the GumTree java implementation as well. If it's based on some paper you 
> should add a comment that mentions it.
Ok, i forgot about that, it's Zhang and Shasha's algorithm for the tree edit 
distance, basically computing the Levenshtein distance for ordered trees.


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes updated this revision to Diff 104664.

https://reviews.llvm.org/D34329

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  include/clang/Tooling/ASTDiff/ASTDiffInternal.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  lib/Tooling/ASTDiff/CMakeLists.txt
  lib/Tooling/CMakeLists.txt
  test/Tooling/clang-diff-basic.cpp
  tools/CMakeLists.txt
  tools/clang-diff/CMakeLists.txt
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- /dev/null
+++ tools/clang-diff/ClangDiff.cpp
@@ -0,0 +1,110 @@
+//===- ClangDiff.cpp - compare source files by AST nodes --*- C++ -*- -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file implements a tool for syntax tree based comparison using
+// Tooling/ASTDiff.
+//
+//===--===//
+
+#include "clang/Tooling/ASTDiff/ASTDiff.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::tooling;
+
+static cl::OptionCategory ClangDiffCategory("clang-diff options");
+
+static cl::opt
+DumpAST("ast-dump",
+cl::desc("Print the internal representation of the AST as JSON."),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt NoCompilationDatabase(
+"no-compilation-database",
+cl::desc(
+"Do not attempt to load build settings from a compilation database"),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt SourcePath(cl::Positional, cl::desc(""),
+   cl::Required,
+   cl::cat(ClangDiffCategory));
+
+static cl::opt DestinationPath(cl::Positional,
+cl::desc(""),
+cl::Optional,
+cl::cat(ClangDiffCategory));
+
+static std::unique_ptr getAST(const StringRef Filename) {
+  std::string ErrorMessage;
+  std::unique_ptr Compilations;
+  if (!NoCompilationDatabase)
+Compilations =
+CompilationDatabase::autoDetectFromSource(Filename, ErrorMessage);
+  if (!Compilations) {
+if (!NoCompilationDatabase)
+  llvm::errs()
+  << "Error while trying to load a compilation database, running "
+ "without flags.\n"
+  << ErrorMessage;
+Compilations = llvm::make_unique(
+".", std::vector());
+  }
+  std::array Files = {{Filename}};
+  ClangTool Tool(*Compilations, Files);
+  std::vector ASTs;
+  Tool.buildASTs(ASTs);
+  if (ASTs.size() != Files.size())
+return nullptr;
+  return std::move(ASTs[0]);
+}
+
+int main(int argc, const char **argv) {
+  cl::HideUnrelatedOptions(ClangDiffCategory);
+  if (!cl::ParseCommandLineOptions(argc, argv)) {
+cl::PrintOptionValues();
+return 1;
+  }
+
+  if (DumpAST) {
+if (!DestinationPath.empty()) {
+  llvm::errs() << "Error: Please specify exactly one filename.\n";
+  return 1;
+}
+std::unique_ptr AST = getAST(SourcePath);
+if (!AST)
+  return 1;
+diff::SyntaxTree Tree(AST->getASTContext());
+Tree.printAsJson(llvm::outs());
+return 0;
+  }
+
+  if (DestinationPath.empty()) {
+llvm::errs() << "Error: Exactly two paths are required.\n";
+return 1;
+  }
+
+  std::unique_ptr Src = getAST(SourcePath);
+  std::unique_ptr Dst = getAST(DestinationPath);
+  if (!Src || !Dst)
+return 1;
+
+  diff::ComparisonOptions Options;
+  diff::SyntaxTree SrcTree(Src->getASTContext());
+  diff::SyntaxTree DstTree(Dst->getASTContext());
+  diff::ASTDiff DiffTool(SrcTree, DstTree, Options);
+  for (const auto  : DiffTool.getMatches())
+DiffTool.printMatch(llvm::outs(), Match);
+  for (const auto  : DiffTool.getChanges())
+DiffTool.printChange(llvm::outs(), Change);
+
+  return 0;
+}
Index: tools/clang-diff/CMakeLists.txt
===
--- /dev/null
+++ tools/clang-diff/CMakeLists.txt
@@ -0,0 +1,13 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_executable(clang-diff
+  ClangDiff.cpp
+  )
+
+target_link_libraries(clang-diff
+  clangFrontend
+  clangTooling
+  clangToolingASTDiff
+  )
Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -2,6 +2,7 @@
 
 add_clang_subdirectory(diagtool)
 add_clang_subdirectory(driver)
+add_clang_subdirectory(clang-diff)
 add_clang_subdirectory(clang-format)
 add_clang_subdirectory(clang-format-vs)
 add_clang_subdirectory(clang-fuzzer)
Index: 

[PATCH] D34329: [clang-diff] Initial implementation.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: include/clang/Tooling/ASTDiff/ASTDiffInternal.h:184
+
+  std::string getNodeValueI(NodeId Id) const;
+  std::string getNodeValueI(const DynTypedNode ) const;

`getNodeValueImpl`?



Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:439
+// Computes an optimal mapping between two trees.
+class ZsMatcher {
+  const ASTDiff::Impl 

Do you know the reason for the `ZsMatcher` name? I noticed that they used it in 
the GumTree java implementation as well. If it's based on some paper you should 
add a comment that mentions it.


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes updated this revision to Diff 104401.

https://reviews.llvm.org/D34329

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  include/clang/Tooling/ASTDiff/ASTDiffInternal.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  lib/Tooling/ASTDiff/CMakeLists.txt
  lib/Tooling/CMakeLists.txt
  test/Tooling/clang-diff-basic.cpp
  tools/CMakeLists.txt
  tools/clang-diff/CMakeLists.txt
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- /dev/null
+++ tools/clang-diff/ClangDiff.cpp
@@ -0,0 +1,110 @@
+//===- ClangDiff.cpp - compare source files by AST nodes --*- C++ -*- -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file implements a tool for syntax tree based comparison using
+// Tooling/ASTDiff.
+//
+//===--===//
+
+#include "clang/Tooling/ASTDiff/ASTDiff.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::tooling;
+
+static cl::OptionCategory ClangDiffCategory("clang-diff options");
+
+static cl::opt
+DumpAST("ast-dump",
+cl::desc("Print the internal representation of the AST as JSON."),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt NoCompilationDatabase(
+"no-compilation-database",
+cl::desc(
+"Do not attempt to load build settings from a compilation database"),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt SourcePath(cl::Positional, cl::desc(""),
+   cl::Required,
+   cl::cat(ClangDiffCategory));
+
+static cl::opt DestinationPath(cl::Positional,
+cl::desc(""),
+cl::Optional,
+cl::cat(ClangDiffCategory));
+
+static std::unique_ptr getAST(const StringRef Filename) {
+  std::string ErrorMessage;
+  std::unique_ptr Compilations;
+  if (!NoCompilationDatabase)
+Compilations =
+CompilationDatabase::autoDetectFromSource(Filename, ErrorMessage);
+  if (!Compilations) {
+if (!NoCompilationDatabase)
+  llvm::errs()
+  << "Error while trying to load a compilation database, running "
+ "without flags.\n"
+  << ErrorMessage;
+Compilations = llvm::make_unique(
+".", std::vector());
+  }
+  std::array Files = {{Filename}};
+  ClangTool Tool(*Compilations, Files);
+  std::vector ASTs;
+  Tool.buildASTs(ASTs);
+  if (ASTs.size() != Files.size())
+return nullptr;
+  return std::move(ASTs[0]);
+}
+
+int main(int argc, const char **argv) {
+  cl::HideUnrelatedOptions(ClangDiffCategory);
+  if (!cl::ParseCommandLineOptions(argc, argv)) {
+cl::PrintOptionValues();
+return 1;
+  }
+
+  if (DumpAST) {
+if (!DestinationPath.empty()) {
+  llvm::errs() << "Error: Please specify exactly one filename.\n";
+  return 1;
+}
+std::unique_ptr AST = getAST(SourcePath);
+if (!AST)
+  return 1;
+diff::SyntaxTree Tree(AST->getASTContext());
+Tree.printAsJson(llvm::outs());
+return 0;
+  }
+
+  if (DestinationPath.empty()) {
+llvm::errs() << "Error: Exactly two paths are required.\n";
+return 1;
+  }
+
+  std::unique_ptr Src = getAST(SourcePath);
+  std::unique_ptr Dst = getAST(DestinationPath);
+  if (!Src || !Dst)
+return 1;
+
+  diff::ComparisonOptions Options;
+  diff::SyntaxTree SrcTree(Src->getASTContext());
+  diff::SyntaxTree DstTree(Dst->getASTContext());
+  diff::ASTDiff DiffTool(SrcTree, DstTree, Options);
+  for (const auto  : DiffTool.getMatches())
+DiffTool.printMatch(llvm::outs(), Match);
+  for (const auto  : DiffTool.getChanges())
+DiffTool.printChange(llvm::outs(), Change);
+
+  return 0;
+}
Index: tools/clang-diff/CMakeLists.txt
===
--- /dev/null
+++ tools/clang-diff/CMakeLists.txt
@@ -0,0 +1,13 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_executable(clang-diff
+  ClangDiff.cpp
+  )
+
+target_link_libraries(clang-diff
+  clangFrontend
+  clangTooling
+  clangToolingASTDiff
+  )
Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -2,6 +2,7 @@
 
 add_clang_subdirectory(diagtool)
 add_clang_subdirectory(driver)
+add_clang_subdirectory(clang-diff)
 add_clang_subdirectory(clang-format)
 add_clang_subdirectory(clang-format-vs)
 add_clang_subdirectory(clang-fuzzer)
Index: 

[PATCH] D34329: [clang-diff] Initial implementation.

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes updated this revision to Diff 104395.

https://reviews.llvm.org/D34329

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  include/clang/Tooling/ASTDiff/ASTDiffInternal.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  lib/Tooling/ASTDiff/CMakeLists.txt
  lib/Tooling/CMakeLists.txt
  test/Tooling/clang-diff-basic.cpp
  tools/CMakeLists.txt
  tools/clang-diff/CMakeLists.txt
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- /dev/null
+++ tools/clang-diff/ClangDiff.cpp
@@ -0,0 +1,110 @@
+//===- ClangDiff.cpp - compare source files by AST nodes --*- C++ -*- -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file implements a tool for syntax tree based comparison using
+// Tooling/ASTDiff.
+//
+//===--===//
+
+#include "clang/Tooling/ASTDiff/ASTDiff.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::tooling;
+
+static cl::OptionCategory ClangDiffCategory("clang-diff options");
+
+static cl::opt
+DumpAST("ast-dump",
+cl::desc("Print the internal representation of the AST as JSON."),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt NoCompilationDatabase(
+"no-compilation-database",
+cl::desc(
+"Do not attempt to load build settings from a compilation database"),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt SourcePath(cl::Positional, cl::desc(""),
+   cl::Required,
+   cl::cat(ClangDiffCategory));
+
+static cl::opt DestinationPath(cl::Positional,
+cl::desc(""),
+cl::Optional,
+cl::cat(ClangDiffCategory));
+
+static std::unique_ptr getAST(const StringRef Filename) {
+  std::string ErrorMessage;
+  std::unique_ptr Compilations;
+  if (!NoCompilationDatabase)
+Compilations =
+CompilationDatabase::autoDetectFromSource(Filename, ErrorMessage);
+  if (!Compilations) {
+if (!NoCompilationDatabase)
+  llvm::errs()
+  << "Error while trying to load a compilation database, running "
+ "without flags.\n"
+  << ErrorMessage;
+Compilations = llvm::make_unique(
+".", std::vector());
+  }
+  std::array Files = {{Filename}};
+  ClangTool Tool(*Compilations, Files);
+  std::vector ASTs;
+  Tool.buildASTs(ASTs);
+  if (ASTs.size() != Files.size())
+return nullptr;
+  return std::move(ASTs[0]);
+}
+
+int main(int argc, const char **argv) {
+  cl::HideUnrelatedOptions(ClangDiffCategory);
+  if (!cl::ParseCommandLineOptions(argc, argv)) {
+cl::PrintOptionValues();
+return 1;
+  }
+
+  if (DumpAST) {
+if (!DestinationPath.empty()) {
+  llvm::errs() << "Error: Please specify exactly one filename.\n";
+  return 1;
+}
+std::unique_ptr AST = getAST(SourcePath);
+if (!AST)
+  return 1;
+diff::SyntaxTree Tree(AST->getASTContext());
+Tree.printAsJson(llvm::outs());
+return 0;
+  }
+
+  if (DestinationPath.empty()) {
+llvm::errs() << "Error: Exactly two paths are required.\n";
+return 1;
+  }
+
+  std::unique_ptr Src = getAST(SourcePath);
+  std::unique_ptr Dst = getAST(DestinationPath);
+  if (!Src || !Dst)
+return 1;
+
+  diff::ComparisonOptions Options;
+  diff::SyntaxTree SrcTree(Src->getASTContext());
+  diff::SyntaxTree DstTree(Dst->getASTContext());
+  diff::ASTDiff DiffTool(SrcTree, DstTree, Options);
+  for (const auto  : DiffTool.getMatches())
+DiffTool.printMatch(llvm::outs(), Match);
+  for (const auto  : DiffTool.getChanges())
+DiffTool.printChange(llvm::outs(), Change);
+
+  return 0;
+}
Index: tools/clang-diff/CMakeLists.txt
===
--- /dev/null
+++ tools/clang-diff/CMakeLists.txt
@@ -0,0 +1,13 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_executable(clang-diff
+  ClangDiff.cpp
+  )
+
+target_link_libraries(clang-diff
+  clangFrontend
+  clangTooling
+  clangToolingASTDiff
+  )
Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -2,6 +2,7 @@
 
 add_clang_subdirectory(diagtool)
 add_clang_subdirectory(driver)
+add_clang_subdirectory(clang-diff)
 add_clang_subdirectory(clang-format)
 add_clang_subdirectory(clang-format-vs)
 add_clang_subdirectory(clang-fuzzer)
Index: 

[PATCH] D34329: [clang-diff] Initial implementation.

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes updated this revision to Diff 104394.
johannes added a comment.

- remove unused struct
- rename getNodeValueI -> getNodeValueImpl


https://reviews.llvm.org/D34329

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  include/clang/Tooling/ASTDiff/ASTDiffInternal.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  lib/Tooling/ASTDiff/CMakeLists.txt
  lib/Tooling/CMakeLists.txt
  test/Tooling/clang-diff-basic.cpp
  tools/CMakeLists.txt
  tools/clang-diff/CMakeLists.txt
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- /dev/null
+++ tools/clang-diff/ClangDiff.cpp
@@ -0,0 +1,110 @@
+//===- ClangDiff.cpp - compare source files by AST nodes --*- C++ -*- -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file implements a tool for syntax tree based comparison using
+// Tooling/ASTDiff.
+//
+//===--===//
+
+#include "clang/Tooling/ASTDiff/ASTDiff.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::tooling;
+
+static cl::OptionCategory ClangDiffCategory("clang-diff options");
+
+static cl::opt
+DumpAST("ast-dump",
+cl::desc("Print the internal representation of the AST as JSON."),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt NoCompilationDatabase(
+"no-compilation-database",
+cl::desc(
+"Do not attempt to load build settings from a compilation database"),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt SourcePath(cl::Positional, cl::desc(""),
+   cl::Required,
+   cl::cat(ClangDiffCategory));
+
+static cl::opt DestinationPath(cl::Positional,
+cl::desc(""),
+cl::Optional,
+cl::cat(ClangDiffCategory));
+
+static std::unique_ptr getAST(const StringRef Filename) {
+  std::string ErrorMessage;
+  std::unique_ptr Compilations;
+  if (!NoCompilationDatabase)
+Compilations =
+CompilationDatabase::autoDetectFromSource(Filename, ErrorMessage);
+  if (!Compilations) {
+if (!NoCompilationDatabase)
+  llvm::errs()
+  << "Error while trying to load a compilation database, running "
+ "without flags.\n"
+  << ErrorMessage;
+Compilations = llvm::make_unique(
+".", std::vector());
+  }
+  std::array Files = {{Filename}};
+  ClangTool Tool(*Compilations, Files);
+  std::vector ASTs;
+  Tool.buildASTs(ASTs);
+  if (ASTs.size() != Files.size())
+return nullptr;
+  return std::move(ASTs[0]);
+}
+
+int main(int argc, const char **argv) {
+  cl::HideUnrelatedOptions(ClangDiffCategory);
+  if (!cl::ParseCommandLineOptions(argc, argv)) {
+cl::PrintOptionValues();
+return 1;
+  }
+
+  if (DumpAST) {
+if (!DestinationPath.empty()) {
+  llvm::errs() << "Error: Please specify exactly one filename.\n";
+  return 1;
+}
+std::unique_ptr AST = getAST(SourcePath);
+if (!AST)
+  return 1;
+diff::SyntaxTree Tree(AST->getASTContext());
+Tree.printAsJson(llvm::outs());
+return 0;
+  }
+
+  if (DestinationPath.empty()) {
+llvm::errs() << "Error: Exactly two paths are required.\n";
+return 1;
+  }
+
+  std::unique_ptr Src = getAST(SourcePath);
+  std::unique_ptr Dst = getAST(DestinationPath);
+  if (!Src || !Dst)
+return 1;
+
+  diff::ComparisonOptions Options;
+  diff::SyntaxTree SrcTree(Src->getASTContext());
+  diff::SyntaxTree DstTree(Dst->getASTContext());
+  diff::ASTDiff DiffTool(SrcTree, DstTree, Options);
+  for (const auto  : DiffTool.getMatches())
+DiffTool.printMatch(llvm::outs(), Match);
+  for (const auto  : DiffTool.getChanges())
+DiffTool.printChange(llvm::outs(), Change);
+
+  return 0;
+}
Index: tools/clang-diff/CMakeLists.txt
===
--- /dev/null
+++ tools/clang-diff/CMakeLists.txt
@@ -0,0 +1,13 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_executable(clang-diff
+  ClangDiff.cpp
+  )
+
+target_link_libraries(clang-diff
+  clangFrontend
+  clangTooling
+  clangToolingASTDiff
+  )
Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -2,6 +2,7 @@
 
 add_clang_subdirectory(diagtool)
 add_clang_subdirectory(driver)
+add_clang_subdirectory(clang-diff)
 add_clang_subdirectory(clang-format)
 

[PATCH] D34329: [clang-diff] Initial implementation.

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes updated this revision to Diff 104249.
johannes added a comment.

- pass Options as a const reference instead of a pointer
- rename TreeComparator -> ASTDiff::Impl, rename Comparator -> DiffImpl
- move declaration of ASTDiff::Impl from the header to the source file so that 
Options does not need to be an opaque pointer
- rename SyntaxTreeI -> SyntaxTreeImpl


https://reviews.llvm.org/D34329

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  include/clang/Tooling/ASTDiff/ASTDiffInternal.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  lib/Tooling/ASTDiff/CMakeLists.txt
  lib/Tooling/CMakeLists.txt
  test/Tooling/clang-diff-basic.cpp
  tools/CMakeLists.txt
  tools/clang-diff/CMakeLists.txt
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- /dev/null
+++ tools/clang-diff/ClangDiff.cpp
@@ -0,0 +1,110 @@
+//===- ClangDiff.cpp - compare source files by AST nodes --*- C++ -*- -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file implements a tool for syntax tree based comparison using
+// Tooling/ASTDiff.
+//
+//===--===//
+
+#include "clang/Tooling/ASTDiff/ASTDiff.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::tooling;
+
+static cl::OptionCategory ClangDiffCategory("clang-diff options");
+
+static cl::opt
+DumpAST("ast-dump",
+cl::desc("Print the internal representation of the AST as JSON."),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt NoCompilationDatabase(
+"no-compilation-database",
+cl::desc(
+"Do not attempt to load build settings from a compilation database"),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt SourcePath(cl::Positional, cl::desc(""),
+   cl::Required,
+   cl::cat(ClangDiffCategory));
+
+static cl::opt DestinationPath(cl::Positional,
+cl::desc(""),
+cl::Optional,
+cl::cat(ClangDiffCategory));
+
+static std::unique_ptr getAST(const StringRef Filename) {
+  std::string ErrorMessage;
+  std::unique_ptr Compilations;
+  if (!NoCompilationDatabase)
+Compilations =
+CompilationDatabase::autoDetectFromSource(Filename, ErrorMessage);
+  if (!Compilations) {
+if (!NoCompilationDatabase)
+  llvm::errs()
+  << "Error while trying to load a compilation database, running "
+ "without flags.\n"
+  << ErrorMessage;
+Compilations = llvm::make_unique(
+".", std::vector());
+  }
+  std::array Files = {{Filename}};
+  ClangTool Tool(*Compilations, Files);
+  std::vector ASTs;
+  Tool.buildASTs(ASTs);
+  if (ASTs.size() != Files.size())
+return nullptr;
+  return std::move(ASTs[0]);
+}
+
+int main(int argc, const char **argv) {
+  cl::HideUnrelatedOptions(ClangDiffCategory);
+  if (!cl::ParseCommandLineOptions(argc, argv)) {
+cl::PrintOptionValues();
+return 1;
+  }
+
+  if (DumpAST) {
+if (!DestinationPath.empty()) {
+  llvm::errs() << "Error: Please specify exactly one filename.\n";
+  return 1;
+}
+std::unique_ptr AST = getAST(SourcePath);
+if (!AST)
+  return 1;
+diff::SyntaxTree Tree(AST->getASTContext());
+Tree.printAsJson(llvm::outs());
+return 0;
+  }
+
+  if (DestinationPath.empty()) {
+llvm::errs() << "Error: Exactly two paths are required.\n";
+return 1;
+  }
+
+  std::unique_ptr Src = getAST(SourcePath);
+  std::unique_ptr Dst = getAST(DestinationPath);
+  if (!Src || !Dst)
+return 1;
+
+  diff::ComparisonOptions Options;
+  diff::SyntaxTree SrcTree(Src->getASTContext());
+  diff::SyntaxTree DstTree(Dst->getASTContext());
+  diff::ASTDiff DiffTool(SrcTree, DstTree, Options);
+  for (const auto  : DiffTool.getMatches())
+DiffTool.printMatch(llvm::outs(), Match);
+  for (const auto  : DiffTool.getChanges())
+DiffTool.printChange(llvm::outs(), Change);
+
+  return 0;
+}
Index: tools/clang-diff/CMakeLists.txt
===
--- /dev/null
+++ tools/clang-diff/CMakeLists.txt
@@ -0,0 +1,13 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_executable(clang-diff
+  ClangDiff.cpp
+  )
+
+target_link_libraries(clang-diff
+  clangFrontend
+  clangTooling
+  clangToolingASTDiff
+  )
Index: tools/CMakeLists.txt

[PATCH] D34329: [clang-diff] Initial implementation.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: include/clang/Tooling/ASTDiff/ASTDiff.h:57
+  SyntaxTree(T *Node, const ASTContext )
+  : TreeImpl(llvm::make_unique(this, Node, AST)) {}
+

If you want to use two different names then something like `SyntaxTreeImpl` is 
better imho


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Ignore the "Just to clarify: I'm fine with adding the" comment, it was from 
last week that was saved in my session and that I didn't delete.


https://reviews.llvm.org/D34329



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Just to clarify: I'm fine with adding the




Comment at: include/clang/Tooling/ASTDiff/ASTDiff.h:32
+public:
+  ASTDiff(SyntaxTree , SyntaxTree , ComparisonOptions *Options);
+

Can you pass-in the options by value instead of a pointer?


https://reviews.llvm.org/D34329



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