johannes added inline comments.
================
Comment at: include/clang/Tooling/ASTDiff/ASTDiff.h:57
+/// Within a tree, this identifies a node by its preorder offset.
+using NodeId = int;
+
----------------
arphaman wrote:
> I think that it's better to make make `NodeId` a structure as well and make
> `InvalidNodeId` a private member. I suggest the following interface for
> `NodeId`:
>
> ```
> struct NodeId {
> private:
> static const int InvalidNodeId;
> public:
> int Id;
>
> NodeId() : Id(InvalidNodeId) { }
> NodeId(int Id) : Id(Id) { }
>
> bool isValid() const { return Id != InvalidNodeId; }
> bool isInvalid() const { return Id == InvalidNodeId; }
> };
> ```
>
>
> This way you'll get rid of a couple of variable initializations that use
> `InvalidNodeId`. You also won't need to call the `memset` when creating the
> unique pointer array of `NodeId`s.
Ok, I did it like this.
Can I create a header file inside lib/Tooling/ASTDiff and include it from the
public interface? This would help reduce the clutter.
Instead of NodeId we could also just use references / pointer types. I don't
see any particularly good reason for choosing either one above the other.
I guess indices make it more obvious how to compute the number of descendants
and such. On the other hand, when using reference types, there is less
boilerplate to write for loops.
https://reviews.llvm.org/D34329
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
