ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

NIT: typo in the change description

> Randam-access

Rand**o**m

LGTM overall, the improvements in PCH and PCM sizes seem worthwhile. Please see 
the NIT about the naming before landing, maybe there are better opions.



================
Comment at: clang/include/clang/Serialization/SourceLocationEncoding.h:144
+  // enclosing sequence instead of establishing a new one.
+  Root(SourceLocationSequence *Parent = nullptr)
+      : Seq(Parent ? Parent->State : State) {}
----------------
NIT: Naming feels a bit confusing here.
`Root` that may have `Parent` seems a bit weird to me conceptually.

One idea that I have in mind is `Sequence::State` that allows to continue an 
existing sequence or establish a new one. Note that we will need to do 
something about the clashing private member.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125403/new/

https://reviews.llvm.org/D125403

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

Reply via email to