[PATCH] D60379: Make precompiled headers reproducible

Anastasia Stulova via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 9 04:58:27 PDT 2019


Anastasia added inline comments.


================
Comment at: lib/Serialization/ASTWriter.cpp:4283
+  // Sort to allow reproducible .pch files - https://bugs.debian.org/877359
+  std::map<TypeID, std::set<std::string>> sortedOpenCLTypeExtMap;
   for (const auto &I : SemaRef.OpenCLTypeExtMap) {
----------------
riccibruno wrote:
> lebedev.ri wrote:
> > Anastasia wrote:
> > > lebedev.ri wrote:
> > > > Would it be better to just change the actual type of `SemaRef.OpenCLTypeExtMap`?
> > > > https://github.com/llvm-mirror/clang/blob/18917301298ad6df9e989983ed1e22cb0f9dff29/include/clang/Sema/Sema.h#L8710-L8712
> > > What data structure would you suggest to use instead? I think sorting during serialization or de-serialization of AST isn't too bad because it's not a common use case.
> > Just `std::map<>`?
> Why is this a problem ? I thought that pch files where not supposed to be distributed. If we come to the conclusion that pch files must be reproducible, then I believe that this is by far not the only example.
Ok, I would normally prefer to use `DenseMap` for the general case because since the number of elements in `OpenCLTypeExtMap` is very small we can take advantage of non-heap allocation. But at the same time probably a couple of elements won't make any difference for `std::map` either and we can keep the code base more maintainable. So I don't mind either way.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60379





More information about the cfe-commits mailing list