shafik added a comment.
Herald added a project: All.
Is this relevant anymore or should we close it?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D37004/new/
https://reviews.llvm.org/D37004
___
cfe-commits mailing list
arphaman added inline comments.
Comment at: tools/clang-diff/ClangDiff.cpp:319
+ "A Binary operator is supposed to have two arguments.");
+for (int I : {1, 0, 2})
+ Offset = printHtmlForNode(OS, Diff, Tree, IsLeft, Children[I], Offset);
johannes added inline comments.
Comment at: tools/clang-diff/ClangDiff.cpp:319
+ "A Binary operator is supposed to have two arguments.");
+for (int I : {1, 0, 2})
+ Offset = printHtmlForNode(OS, Diff, Tree, IsLeft, Children[I], Offset);
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.
LGTM with one request below:
Comment at: tools/clang-diff/ClangDiff.cpp:319
+ "A Binary operator is supposed to have two arguments.");
+for (int I : {1, 0,
johannes updated this revision to Diff 112416.
johannes added a comment.
provide a test
need to update the maxsize parameter to -s=200 here because the test file is
expected to fit within this size
https://reviews.llvm.org/D37004
Files:
test/Tooling/Inputs/clang-diff-basic-src.cpp
arphaman added inline comments.
Comment at: test/Tooling/clang-diff-ast.cpp:42
// CHECK: CXXRecordDecl: X;X;(
-class X : Base {
+struct X : Base {
int m;
Looks like an unrelated change.
https://reviews.llvm.org/D37004
arphaman added inline comments.
Comment at: test/Tooling/clang-diff-ast.cpp:113
+
+// CHECK: CXXOperatorCallExpr
+// CHECK-NEXT: DeclRefExpr: str::operator+
This test doesn't work because it passes prior to this patch. You should test
the HTML output instead.
johannes created this revision.
Herald added subscribers: mgorny, klimek.
The operator is always the first child of such an expression.
If it is an infix operator, we want to print the LHS first.
https://reviews.llvm.org/D37004
Files:
test/Tooling/clang-diff-ast.cpp
