[PATCH] D27836: Store the "current position" index within the ASTRecordReader.

2016-12-20 Thread David L. Jones via Phabricator via cfe-commits
dlj added a comment.

In https://reviews.llvm.org/D27836#628029, @rsmith wrote:

> LGTM, any chance I can tempt you to lowerCamelCase all the other 
> ASTRecordReader members to match the new ones as a follow-up change?


Yup, will do.


https://reviews.llvm.org/D27836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D27836: Store the "current position" index within the ASTRecordReader.

2016-12-20 Thread Richard Smith via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

LGTM, any chance I can tempt you to lowerCamelCase all the other 
ASTRecordReader members to match the new ones as a follow-up change?


https://reviews.llvm.org/D27836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D27836: Store the "current position" index within the ASTRecordReader.

2016-12-17 Thread David L. Jones via Phabricator via cfe-commits
dlj added a comment.

Yeah, that makes more sense. Switched to readInt/peekInt/skipInts, let me know 
if you have a better idea for the names.


https://reviews.llvm.org/D27836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D27836: Store the "current position" index within the ASTRecordReader.

2016-12-16 Thread Richard Smith via Phabricator via cfe-commits
rsmith added a comment.

I like moving `Idx` into the reader, but I have mixed feelings about the 
iterator-like interface. For consistency with the rest of the `ASTRecordReader` 
interface, and with how we model the writer side, I think perhaps 
`Record.ReadInt()` would fit better than using `*Record++`.


https://reviews.llvm.org/D27836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits