arphaman added inline comments.
================ Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:474 + for (SNodeId D1 = LMD1 + 1; D1 <= Id1; ++D1) { + double DeletionCost = 1.0; + ForestDist[D1][LMD2] = ForestDist[D1 - 1][LMD2] + DeletionCost; ---------------- johannes wrote: > arphaman wrote: > > Are the `DeletionCost` and `InsertionCost` constants or are you planning to > > modify them in the future? > I think they could be modified for minor improvements of the matching, but I > am probably not going to do so anytime soon. Maybe it is better to store them > at class scope as static const / constexpr? Yeah, I suppose for now they should be declared as constants (the static const / constexpr doesn't matter). They should probably be documented as well. ================ Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:693 + H2 = L2.pop(); + for (NodeId Id1 : H1) + for (NodeId Id2 : H2) ---------------- johannes wrote: > arphaman wrote: > > Please wrap these loops in braces. > Not sure if I got this right. It looks better, thanks. Generally we prefer to use braces for `if`/`for`/etc. if they have more than one statement. https://reviews.llvm.org/D34329 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits