[PATCH] D36176: [clang-diff] Fix some errors and inconsistencies
Johannes Altmanninger via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 3 06:17:25 PDT 2017
johannes added a comment.
In https://reviews.llvm.org/D36176#830341, @arphaman wrote:
> LGTM. Although I'm not 100% sure it's fully NFC, so if you can't come up with a test please justify why.
There are changes that affect the output.
Firstly the computation of the rightmost descendant; the added assertion fails every time with the old version.
Secondly, the change around line 811 prevents root nodes from being mapped if they are already mapped. When comparing translation units this happens only if the two trees are identical (in this case the output would not change). Otherwise, it would require some change in the client to demonstrate how the behaviour changed. One needs to compare AST Nodes other than entire translation units. At some point there will be something that uses this feature so we cover it too (or remove the feature). So I'd keep it like this
https://reviews.llvm.org/D36176
More information about the cfe-commits
mailing list