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

Reply via email to