klimek 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;
+
----------------
johannes wrote:
> 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.
No, but you can create a header ASTDiffInternal or somesuch next to the header
in the public dir.
https://reviews.llvm.org/D34329
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
