[PATCH] D126651: [clang-diff] Fix getStmtValue when dealing with wide, UTF16 UTF32 chars

2022-06-10 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. No idea why the conversion fails for the wide string. First step is to reproduce the problem. I guess we should try in a Ubuntu VM. Is there an easy way to see if other builders succeeded? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D126651: [clang-diff] Fix getStmtValue when dealing with wide, UTF16 UTF32 chars

2022-06-07 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. Ok thanks, I will take a look later. Might be a locale issue Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126651/new/ https://reviews.llvm.org/D126651 ___ cfe-commits

[PATCH] D126651: [clang-diff] Fix getStmtValue when dealing with wide, UTF16 UTF32 chars

2022-06-07 Thread Johannes Altmanninger via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe80748ff8840: [clang-diff] Fix assertion error when dealing with wide strings (authored by PRESIDENT810, committed by johannes). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D126651: [clang-diff] Fix getStmtValue when dealing with wide, UTF16 UTF32 chars

2022-06-07 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes accepted this revision. johannes added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/ASTDiff/ASTDiff.cpp:477 + } else if (String->isUTF16()) { +const auto *Chars = reinterpret_cast(Bytes); +if

[PATCH] D126651: [clang-diff] Fix getStmtValue when dealing with wide chars

2022-06-06 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes requested changes to this revision. johannes added a comment. This revision now requires changes to proceed. I've suggested some more refactorings but otherwise this should be good to go Comment at: clang/lib/Tooling/ASTDiff/ASTDiff.cpp:497 + return str; +}

[PATCH] D126651: [clang-diff] Fix getStmtValue when dealing with wide chars

2022-05-31 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes requested changes to this revision. johannes added a comment. This revision now requires changes to proceed. I wonder if clang-diff is useful in its current state, I remember there are many edge cases left over. Anyway, I've left some minor comments. I'm not sure how to download a

[PATCH] D70302: [CodeGen] Fix clang crash on aggregate initialization of array of labels

2019-11-30 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. In D70302#1763025 , @rnk wrote: > Fix looks good post commit, but we should enhance the test. I see, apologies for the premature commit. Comment at: clang/test/CodeGen/label-array-aggregate-init.c:1 +// RUN:

[PATCH] D70302: [CodeGen] Fix clang crash on aggregate initialization of array of labels

2019-11-27 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. Committed as 1ac700cdef787383ad49a0e37d9894491ef19480 - this should be a safe fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70302/new/

[PATCH] D70302: [CodeGen] Fix clang crash on aggregate initialization of array of labels

2019-11-27 Thread Johannes Altmanninger via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG1ac700cdef78: [CodeGen] Fix clang crash on aggregate initialization of array of labels (authored by johannes).

[PATCH] D70302: [CodeGen] Fix clang crash on aggregate initialization of array of labels

2019-11-27 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 231330. johannes edited the summary of this revision. johannes added a comment. update commit message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70302/new/ https://reviews.llvm.org/D70302 Files:

[PATCH] D70302: [CodeGen] Fix clang crash on aggregate initialization of array of labels

2019-11-16 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes marked an inline comment as done. johannes added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:1021 void CodeGenModule::EmitExplicitCastExprType(const ExplicitCastExpr *E, - CodeGenFunction *CGF) { +

[PATCH] D70302: [CodeGen] Fix clang crash on aggregate initialization of array of labels

2019-11-16 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 229556. johannes added a comment. Remove unneccesary const on parameter Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70302/new/ https://reviews.llvm.org/D70302 Files: clang/lib/CodeGen/CGExprAgg.cpp

[PATCH] D70302: [CodeGen] Fix clang crash on aggregate initialization of array of labels

2019-11-15 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a project: clang. Fixes pr/43700. Also simplify another call, and make the CGF member a const pointer since it is public but only assigned in the constructor. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D70302 Files:

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

2018-08-27 Thread 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

[PATCH] D40731: Integrate CHash into CLang

2017-12-04 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. In https://reviews.llvm.org/D40731#943564, @stettberger wrote: > For my changes to StmtDataCollectors: I tried to leave the already existing > entries in StmtDataCollector.td untouched and the test-cases still work OK. > If there is something somebody could break by

[PATCH] D40731: Integrate CHash into CLang

2017-12-04 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. So you can define a category for cHash and add `let cHash = [{ .. }]` or `let cHash = TypeIIClone;` if you just want to reuse it. With this patch you can also use !codeconcat to append some code: https://reviews.llvm.org/D40782 Repository: rC Clang

[PATCH] D40731: Integrate CHash into CLang

2017-12-04 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. In https://reviews.llvm.org/D40731#943469, @stettberger wrote: > @Eugene.Zelenko Thank you for pointing me out on these issues. I ran > clang-tidy and clang-format on CHashVisitor.h > > @rsmith You're right, there is already more than one implemenation of >

[PATCH] D40781: [DataCollection] Allow choosing between collection categories

2017-12-04 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added 1 blocking reviewer(s): teemperor. Herald added a subscriber: ilya-biryukov. This adds a list of collection categories to ClangDataCollectorsEmitter.cpp, in an aim to eventually unify the various AST hashing mechanisms (each one will get its own

[PATCH] D39664: [clang-diff] patching: implement updates

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. This allows to patch nodes that have been updated. We use a LCS algorithm to determine which tokens changed, and update accordingly. https://reviews.llvm.org/D39664 Files: lib/Tooling/ASTDiff/ASTPatch.cpp

[PATCH] D39663: [clang-diff] Split source ranges at points where nodes are inserted.

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. This enables inserting into empty CompoundStmt nodes, for example. Previously the insertion would occur only after the closing parenthesis. https://reviews.llvm.org/D39663 Files: lib/Tooling/ASTDiff/ASTPatch.cpp

[PATCH] D37005: [clang-diff] Initial implementation of patching

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 121656. johannes added a comment. update https://reviews.llvm.org/D37005 Files: include/clang/Tooling/ASTDiff/ASTDiff.h include/clang/Tooling/ASTDiff/ASTPatch.h lib/Tooling/ASTDiff/ASTDiff.cpp lib/Tooling/ASTDiff/ASTPatch.cpp

[PATCH] D39662: [clang-diff] Enable postorder traversal

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. https://reviews.llvm.org/D39662 Files: include/clang/Tooling/ASTDiff/ASTDiff.h lib/Tooling/ASTDiff/ASTDiff.cpp Index: lib/Tooling/ASTDiff/ASTDiff.cpp === ---

[PATCH] D39661: [clang-diff] Move printing of changes to the ASTDiff library

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. https://reviews.llvm.org/D39661 Files: include/clang/Tooling/ASTDiff/ASTDiff.h lib/Tooling/ASTDiff/ASTDiff.cpp tools/clang-diff/ClangDiff.cpp Index: tools/clang-diff/ClangDiff.cpp

[PATCH] D39660: [clang-diff] Don't ignore nodes from other files

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. https://reviews.llvm.org/D39660 Files: lib/Tooling/ASTDiff/ASTDiff.cpp test/Tooling/clang-diff-ast.cpp tools/clang-diff/ClangDiff.cpp Index: tools/clang-diff/ClangDiff.cpp

[PATCH] D39659: [clang-diff] Store changes within ASTDiff::Impl

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. This way we avoid modifying SyntaxTree::Impl except during construction. https://reviews.llvm.org/D39659 Files: include/clang/Tooling/ASTDiff/ASTDiff.h lib/Tooling/ASTDiff/ASTDiff.cpp tools/clang-diff/ClangDiff.cpp

[PATCH] D39658: [clang-diff] Treat QualType / TypeLoc as a node

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. https://reviews.llvm.org/D39658 Files: lib/Tooling/ASTDiff/ASTDiff.cpp test/Tooling/clang-diff-ast.cpp test/Tooling/clang-diff-basic.cpp test/Tooling/clang-diff-bottomup.cpp test/Tooling/clang-diff-heuristics.cpp

[PATCH] D37003: [clang-diff] Support templates

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 121650. johannes added a comment. update https://reviews.llvm.org/D37003 Files: lib/Tooling/ASTDiff/ASTDiff.cpp test/Tooling/Inputs/clang-diff-basic-src.cpp test/Tooling/clang-diff-ast.cpp test/Tooling/clang-diff-basic.cpp

[PATCH] D36688: [clang-diff] Fix matching for unnamed NamedDecs

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 121649. johannes added a comment. update https://reviews.llvm.org/D36688 Files: lib/Tooling/ASTDiff/ASTDiff.cpp test/Tooling/clang-diff-heuristics.cpp Index: test/Tooling/clang-diff-heuristics.cpp

[PATCH] D39656: [clang-diff] Remove getNodeValue

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. https://reviews.llvm.org/D39656 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-bottomup.cpp

[PATCH] D37001: [clang-diff] Use data collectors for node comparison

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 121647. johannes added a comment. use raw source code of owned tokens instead https://reviews.llvm.org/D37001 Files: include/clang/Tooling/ASTDiff/ASTDiff.h lib/Tooling/ASTDiff/ASTDiff.cpp test/Tooling/clang-diff-basic.cpp

[PATCH] D39655: [clang-diff] Expose Node::getSourceRange()

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. https://reviews.llvm.org/D39655 Files: include/clang/Tooling/ASTDiff/ASTDiff.h lib/Tooling/ASTDiff/ASTDiff.cpp test/Tooling/clang-diff-json.cpp Index: test/Tooling/clang-diff-json.cpp

[PATCH] D39653: [clang-diff] Add utility functions, forbid copying Node

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. https://reviews.llvm.org/D39653 Files: include/clang/Tooling/ASTDiff/ASTDiff.h lib/Tooling/ASTDiff/ASTDiff.cpp Index: lib/Tooling/ASTDiff/ASTDiff.cpp === ---

[PATCH] D36997: [clang-diff] Honor macros

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 121642. johannes added a comment. update https://reviews.llvm.org/D36997 Files: include/clang/Tooling/ASTDiff/ASTDiff.h lib/Tooling/ASTDiff/ASTDiff.cpp test/Tooling/Inputs/clang-diff-basic-src.cpp test/Tooling/clang-diff-ast.cpp

[PATCH] D36687: [clang-diff] Match nodes with the same parent and same value

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 121641. johannes added a comment. update https://reviews.llvm.org/D36687 Files: include/clang/Tooling/ASTDiff/ASTDiff.h lib/Tooling/ASTDiff/ASTDiff.cpp test/Tooling/clang-diff-bottomup.cpp test/Tooling/clang-diff-heuristics.cpp

[PATCH] D39652: [clang-diff] NFC: factor out getCompilationDatabase()

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. https://reviews.llvm.org/D39652 Files: tools/clang-diff/ClangDiff.cpp Index: tools/clang-diff/ClangDiff.cpp === --- tools/clang-diff/ClangDiff.cpp +++ tools/clang-diff/ClangDiff.cpp @@ -84,27

[PATCH] D39651: [clang-diff] NFC: renames, moves

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. https://reviews.llvm.org/D39651 Files: lib/Tooling/ASTDiff/ASTDiff.cpp Index: lib/Tooling/ASTDiff/ASTDiff.cpp === --- lib/Tooling/ASTDiff/ASTDiff.cpp +++

[PATCH] D39650: [clang-diff] Make getSourceRangeOffsets a member of Node

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. https://reviews.llvm.org/D39650 Files: include/clang/Tooling/ASTDiff/ASTDiff.h lib/Tooling/ASTDiff/ASTDiff.cpp tools/clang-diff/ClangDiff.cpp Index: tools/clang-diff/ClangDiff.cpp

[PATCH] D39649: [clang-diff] Split findPositionInParent

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. This makes findPositionInParent() a member of Node. Additionally, the function that computes the new position that is dependent on insertions and deletions of siblings, is now a member of ASTDiff::Impl.

[PATCH] D39648: [clang-diff] NFC: organise header

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. https://reviews.llvm.org/D39648 Files: include/clang/Tooling/ASTDiff/ASTDiff.h lib/Tooling/ASTDiff/ASTDiff.cpp Index: lib/Tooling/ASTDiff/ASTDiff.cpp === ---

[PATCH] D39647: [clang-diff] NFC: remove Mapping

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. Store it within ASTDiff::Impl instead. https://reviews.llvm.org/D39647 Files: include/clang/Tooling/ASTDiff/ASTDiff.h lib/Tooling/ASTDiff/ASTDiff.cpp tools/clang-diff/ClangDiff.cpp Index: tools/clang-diff/ClangDiff.cpp

[PATCH] D39646: [clang-diff] Rename ChangeKind::None to NoChange

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. https://reviews.llvm.org/D39646 Files: include/clang/Tooling/ASTDiff/ASTDiff.h tools/clang-diff/ClangDiff.cpp Index: tools/clang-diff/ClangDiff.cpp === ---

[PATCH] D39645: [clang-diff] NFC: replace const Node & with a typedef'd NodeRef

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. https://reviews.llvm.org/D39645 Files: include/clang/Tooling/ASTDiff/ASTDiff.h lib/Tooling/ASTDiff/ASTDiff.cpp tools/clang-diff/ClangDiff.cpp Index: tools/clang-diff/ClangDiff.cpp

[PATCH] D39644: [clang-diff] Use references to Node instead of NodeId

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. This adds to each node a reference to its syntax tree. As a result, instead of passing around the tree plus the node ID, we just use a reference to the node. This removes some potential for errors. Users will almost always use

[PATCH] D37663: [AST] Make RecursiveASTVisitor visit CXXOperatorCallExpr in source order

2017-09-13 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added inline comments. Comment at: include/clang/AST/RecursiveASTVisitor.h:327 + + SmallVector getStmtChildren(CXXOperatorCallExpr *CE) { +SmallVector Children(CE->children()); rsmith wrote: > The copy here is more expensive than I'd like. Instead

[PATCH] D37663: [AST] Make RecursiveASTVisitor visit CXXOperatorCallExpr in source order

2017-09-13 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 115018. johannes added a comment. use CXXOperatorCallExpr::isPrefixOp() to determine whether it's infix or postfix directly traverse statement children instead of copying https://reviews.llvm.org/D37663 Files: include/clang/AST/ExprCXX.h

[PATCH] D37663: [AST] Make RecursiveASTVisitor visit CXXOperatorCallExpr in source order

2017-09-11 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added inline comments. Comment at: include/clang/AST/RecursiveASTVisitor.h:334 +case OO_Arrow: +case OO_Call: +case OO_Subscript: klimek wrote: > Why do we need to swap for calls? The idea is that the opening parenthesis/bracket comes after

[PATCH] D37662: [AST] Make RecursiveASTVisitor visit TemplateDecls in source order

2017-09-09 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. This causes template arguments to be traversed before the templated declaration, which is useful for clients that expect the nodes in the same order as they are in the source code. Additionally, there seems to be no good reason not to do so. This was moved here

[PATCH] D37663: [AST] Make RecursiveASTVisitor visit CXXOperatorCallExpr in source order

2017-09-09 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. This adds a special case for traversing CXXOperatorCallExpr. Its children are traversed in the order in which they appear in the source code, so infix and postfix operators are visited after their first argument. This behavior was previously only in

[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors

2017-09-06 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. @teemperor ok for you? did phabricator make you a blocking reviewer because of the affected code, or did I do that somehow? https://reviews.llvm.org/D37383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D36998: [AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor

2017-09-01 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 113581. johannes added a comment. undo visibility change https://reviews.llvm.org/D36998 Files: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h include/clang/AST/RecursiveASTVisitor.h

[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors

2017-09-01 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. In https://reviews.llvm.org/D37383#858905, @teemperor wrote: > @arphaman I suggested tablegen in https://reviews.llvm.org/D36664 because I > remembered we had some CMake sanity check about not having an *.inc in our > include dir: >

[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors

2017-09-01 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: mgorny. Herald added 1 blocking reviewer(s): teemperor. This adds an option "-gen-clang-data-collectors" to the Clang TableGen that is used to generate StmtDataCollectors.inc. https://reviews.llvm.org/D37383 Files:

[PATCH] D36998: [AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor

2017-09-01 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 113546. johannes added a comment. fix by adding an option in RecursiveASTVisitor https://reviews.llvm.org/D36998 Files: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h include/clang/AST/RecursiveASTVisitor.h

[PATCH] D37001: [clang-diff] Use data collectors for node comparison

2017-09-01 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added inline comments. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:537 + +#include "../../AST/DeclDataCollectors.inc" + arphaman wrote: > I didn't realize that you're including files from within `lib`. That's not > ideal. You should add a pre-commit

[PATCH] D37005: [clang-diff] Initial implementation of patching

2017-08-29 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added inline comments. Comment at: include/clang/Tooling/ASTDiff/ASTDiff.h:73 public: + /// Empty (invalid) SyntaxTree. + SyntaxTree(); arphaman wrote: > Why do you need to create an empty tree? What about using llvm::Optional > instead? ok, i use

[PATCH] D37005: [clang-diff] Initial implementation of patching

2017-08-29 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 113078. johannes added a comment. fixes https://reviews.llvm.org/D37005 Files: include/clang/Tooling/ASTDiff/ASTDiff.h include/clang/Tooling/ASTDiff/ASTPatch.h lib/Tooling/ASTDiff/ASTDiff.cpp lib/Tooling/ASTDiff/ASTPatch.cpp

[PATCH] D36998: [AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor

2017-08-29 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. This is currently broken, if a user provides a TraverseClassTemplateDecl, then the same method in this class will not be called, I think I will add a flag (probably not user visible) in RecursiveASTVisitor.h to switch the order for templates

[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor

2017-08-28 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. Yeah, this seems to be the best solution for this. I think I ran into the same issue when working on the StmtDataCollector - basically there can only be two Traverse*, one in the Derived class and the other one needs to do all the magic with walking the specialisation

[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor

2017-08-28 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 112951. johannes added a comment. use a specialized getStmtChildren to fix the order for CXXOperatorCallExpr https://reviews.llvm.org/D37200 Files: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h include/clang/AST/RecursiveASTVisitor.h

[PATCH] D37005: [clang-diff] Initial implementation of patching

2017-08-28 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 112926. johannes edited the summary of this revision. johannes added a comment. split to ASTDiff/ASTPatch https://reviews.llvm.org/D37005 Files: include/clang/Tooling/ASTDiff/ASTDiff.h include/clang/Tooling/ASTDiff/ASTPatch.h

[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor

2017-08-28 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 112894. johannes added a comment. detect prefix/postfix from number of arguments https://reviews.llvm.org/D37200 Files: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h include/clang/AST/RecursiveASTVisitor.h

[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor

2017-08-28 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. The previous version didn't call TraverseDecl of the derived class, this is fixed now. The public getDerived.TraverseStmt() does not accept a DataRecursionQueue, so this also could not be used (I think) I used the wrapper TraverseStmtBase, which should behave exactly

[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor

2017-08-28 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 112889. johannes added a comment. do call derived TraverseStmt for children of CXXOperatorCallExpr https://reviews.llvm.org/D37200 Files: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h include/clang/AST/RecursiveASTVisitor.h

[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor

2017-08-28 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 112885. johannes added a comment. use RecursiveASTVisitor::TraverseStmt https://reviews.llvm.org/D37200 Files: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp Index:

[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor

2017-08-28 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added inline comments. Comment at: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h:75 + Derived () { return *static_cast(this); } + arphaman wrote: > I don't think you need this since `getDerived` in RecursiveASTVisitor is > already public.

[PATCH] D37005: [clang-diff] Initial implementation of patching

2017-08-27 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 112845. johannes retitled this revision from "Add include/clang/Tooling/ASTDiff/ASTPatch.h" to "[clang-diff] Initial implementation of patching". johannes edited the summary of this revision. johannes added a comment. use rewriter to patch a third AST

[PATCH] D37201: [clang-diff] Use correct SourceRange for CXXConstructExpr

2017-08-27 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. This way the variable name of a declaration is not included https://reviews.llvm.org/D37201 Files: test/Tooling/Inputs/clang-diff-basic-src.cpp test/Tooling/clang-diff-basic.cpp test/Tooling/clang-diff-html.test Index:

[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor

2017-08-27 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. This affects overloaded operators, which are represented by a CXXOperatorCallExpr whose first child is always a DeclRefExpr referring to the operator. For infix, postfix and call operators we want the first argument to be

[PATCH] D36998: [AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor

2017-08-25 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 112707. johannes added a comment. test ordering, class template https://reviews.llvm.org/D36998 Files: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h include/clang/AST/RecursiveASTVisitor.h

[PATCH] D37004: [clang-diff] Fix the html output for CXXOperatorCallExpr

2017-08-25 Thread Johannes Altmanninger via Phabricator via cfe-commits
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);

[PATCH] D37001: [clang-diff] Use data collectors for node comparison

2017-08-25 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. In https://reviews.llvm.org/D37001#852442, @arphaman wrote: > Can you remove `getNodeValue` now or do you still need it? It is only used in the client, I think it makes sense to move it there, or to remove it altogether. I think I'd keep it in the client for now, to

[PATCH] D37003: [clang-diff] Support templates

2017-08-25 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 112673. johannes edited the summary of this revision. johannes added a comment. use LexicallyOrderedRecursiveASTVisitor https://reviews.llvm.org/D37003 Files: lib/Tooling/ASTDiff/ASTDiff.cpp test/Tooling/Inputs/clang-diff-basic-src.cpp

[PATCH] D36998: [AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor

2017-08-25 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 112671. johannes retitled this revision from "[AST] Make TraverseTemplateParameterListHelper public" to "[AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor". johannes edited the summary of this revision. johannes added a reviewer: arphaman.

[PATCH] D36998: [AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor

2017-08-25 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added inline comments. Comment at: unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp:146 + DummyMatchVisitor Visitor; + Visitor.ExpectMatch("/f/T", 2, 11); + Visitor.ExpectMatch("/f/f/", 2, 20); This test also works before the change;

[PATCH] D37004: [clang-diff] Fix the html output for CXXOperatorCallExpr

2017-08-23 Thread Johannes Altmanninger via Phabricator via cfe-commits
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

[PATCH] D37072: [clang-diff] Properly clear the selection in HTML diff

2017-08-23 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. https://reviews.llvm.org/D37072 Files: tools/clang-diff/ClangDiff.cpp Index: tools/clang-diff/ClangDiff.cpp === --- tools/clang-diff/ClangDiff.cpp +++ tools/clang-diff/ClangDiff.cpp @@ -140,9

[PATCH] D36989: [clang-diff] Refactor stop-after command-line flag

2017-08-22 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. In https://reviews.llvm.org/D36989#848835, @jgravelle-google wrote: > If you have more stop-after options it'd probably be simpler to leave it as > one field. I'll just rename it to "stop-diff-after" to avoid the name > collision. Ok, you can go ahead. Or, if I

[PATCH] D37005: Add include/clang/Tooling/ASTDiff/ASTPatch.h

2017-08-22 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added subscribers: mgorny, klimek. This adds a functions to remove AST nodes. It works for Decl, and for Stmt nodes that are children of a CompoundStmt. Sometimes it is not possible to remove a Decl from its context despite DeclContext.containsDecl()

[PATCH] D37004: [clang-diff] Fix the html output for CXXOperatorCallExpr

2017-08-22 Thread Johannes Altmanninger via Phabricator via cfe-commits
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

[PATCH] D37003: [clang-diff] Support templates

2017-08-22 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. TemplateName and TemplateArgument are recognised. https://reviews.llvm.org/D37003 Files: lib/Tooling/ASTDiff/ASTDiff.cpp test/Tooling/Inputs/clang-diff-basic-src.cpp test/Tooling/clang-diff-ast.cpp

[PATCH] D37002: [clang-diff] Treat CXXCtorInitializer as a node

2017-08-22 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. https://reviews.llvm.org/D37002 Files: lib/Tooling/ASTDiff/ASTDiff.cpp test/Tooling/clang-diff-ast.cpp Index: test/Tooling/clang-diff-ast.cpp === ---

[PATCH] D37001: [clang-diff] Use data collectors for node comparison

2017-08-22 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. https://reviews.llvm.org/D37001 Files: lib/Tooling/ASTDiff/ASTDiff.cpp Index: lib/Tooling/ASTDiff/ASTDiff.cpp === --- lib/Tooling/ASTDiff/ASTDiff.cpp +++

[PATCH] D37000: [clang-diff] Remove NodeCountVisitor, NFC

2017-08-22 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. https://reviews.llvm.org/D37000 Files: lib/Tooling/ASTDiff/ASTDiff.cpp Index: lib/Tooling/ASTDiff/ASTDiff.cpp === --- lib/Tooling/ASTDiff/ASTDiff.cpp +++

[PATCH] D36999: [AST] Add DeclDataCollectors.inc

2017-08-22 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. This adds some macros for collecting data from Decl nodes. However, it is incomplete as its not always clear which data should be collected, plus there are missing nodes. So this probably needs some tuning if it is useful enough to get merged.

[PATCH] D36998: [AST] Make TraverseTemplateParameterListHelper public

2017-08-22 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. This is useful for clients that want to create a visitor that visits template parameters before visiting the declaration that uses them. https://reviews.llvm.org/D36998 Files: include/clang/AST/RecursiveASTVisitor.h Index:

[PATCH] D36997: [clang-diff] Honor macros

2017-08-22 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. Nodes that are expanded from some macro are given a special "Macro" kind instead of an ASTNodeKind. They are compared by their textual value including arguments, before expansion. https://reviews.llvm.org/D36997 Files:

[PATCH] D36686: [clang-diff] Add option to compare files across git revisions

2017-08-22 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 112127. johannes added a comment. casing https://reviews.llvm.org/D36686 Files: tools/clang-diff/ClangDiff.cpp Index: tools/clang-diff/ClangDiff.cpp === --- tools/clang-diff/ClangDiff.cpp

[PATCH] D36685: [clang-diff] HTML diff navigation

2017-08-22 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 112126. johannes added a comment. some refactoring https://reviews.llvm.org/D36685 Files: tools/clang-diff/ClangDiff.cpp Index: tools/clang-diff/ClangDiff.cpp === ---

[PATCH] D36989: [clang-diff] Refactor stop-after command-line flag

2017-08-22 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes requested changes to this revision. johannes added a comment. This revision now requires changes to proceed. Oh wait I get linker errors with BUILD_SHARED_LIBS=1 `undefined reference to ClangDiffCategory` in ASTDiff https://reviews.llvm.org/D36989

[PATCH] D36989: [clang-diff] Refactor stop-after command-line flag

2017-08-22 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes accepted this revision. johannes added a comment. This revision is now accepted and ready to land. Looks good, thanks! You can remove StopAfterTopDown from the ComparisonOptions struct definition I have a second option stop-after option coming up but I will use another boolean for it,

[PATCH] D35948: [CommonOptionsParser] Expose ArgumentsAdjustingCompilationDatabase

2017-08-20 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes accepted this revision. johannes added a comment. This revision is now accepted and ready to land. This has already landed. https://reviews.llvm.org/D35948 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D36187: [clang-diff] Use the relative name for NamedDecls

2017-08-18 Thread Johannes Altmanninger via Phabricator via cfe-commits
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

[PATCH] D36187: [clang-diff] Use the relative name for NamedDecls

2017-08-18 Thread Johannes Altmanninger via Phabricator via cfe-commits
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

[PATCH] D36686: [clang-diff] Add option to compare files across git revisions

2017-08-16 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 111328. johannes edited the summary of this revision. johannes added a comment. accept a commit range https://reviews.llvm.org/D36686 Files: tools/clang-diff/ClangDiff.cpp Index: tools/clang-diff/ClangDiff.cpp

[PATCH] D36664: [analyzer] Make StmtDataCollector customizable

2017-08-16 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. In https://reviews.llvm.org/D36664#841758, @teemperor wrote: > Very well done, I really like this patch! I added a few remarks mostly about > the comments that need some small adjusting. > > I'm wondering what would be a nice way of creating a StmtDataCollector that >

[PATCH] D36664: [analyzer] Make StmtDataCollector customizable

2017-08-16 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 111317. johannes added a subscriber: rsmith. johannes added a comment. add https://reviews.llvm.org/D36664 Files: include/clang/AST/DataCollection.h include/clang/Analysis/CloneDetection.h lib/AST/CMakeLists.txt lib/AST/DataCollection.cpp

[PATCH] D36664: [analyzer] Make StmtDataCollector customizable

2017-08-16 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 111316. johannes edited the summary of this revision. johannes removed a subscriber: rsmith. johannes added a comment. - fix comments - add unittest for the new node kinds - make `addData()` take a const reference https://reviews.llvm.org/D36664 Files:

[PATCH] D36686: [clang-diff] Add option to compare files across git revisions

2017-08-14 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. In https://reviews.llvm.org/D36686#840827, @arphaman wrote: > It might be useful to have both source and destination in a different > revision. Maybe something like `-src-git-rev` and `-dst-git-rev`? Then I'd rather have a revision range such as `-git-revs ..` and if

[PATCH] D36688: [clang-diff] Fix matching for unnamed NamedDecs

2017-08-14 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: klimek. This makes Node::getIdentifier return a valid value for special NamedDecl nodes that do not have a name, such as ctors, dtors and unnamed classes / namespaces. https://reviews.llvm.org/D36688 Files:

[PATCH] D36687: [clang-diff] Match nodes with the same parent and same value

2017-08-14 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. This adds another pass to the matching algorithm that tries to match nodes with common parents, if they have similar values. https://reviews.llvm.org/D36687 Files: include/clang/Tooling/ASTDiff/ASTDiff.h lib/Tooling/ASTDiff/ASTDiff.cpp

  1   2   >