This patch have been applied to Debian packages for a few months without
Patch by Rebecca N. Palmer
Needs a test.
Would it be better to just change the actual type of SemaRef.OpenCLTypeExtMap?
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.
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.
After looking at the bug report (https://bugs.debian.org/877359), I would like to point out that there is currently *absolutely* no stability guarantee for the serialization format (ie, you need the exact same revision). In regard of this I am wondering how sane it is to ship pch files.
I agree we shouldn't make guarantees about pch file format stability and that distributing pch files isn't a good idea. Having said that, making clang write the same output when given the same input is still a Good Thing as it enables caching these outputs e.g. with tools like distcc.
I went through each of the structures serialized in ASTWriter.cpp, and unless I missed one, OpenCLDeclExtMap and OpenCLTypeExtMap are the only one which are serialized in a non-deterministic way (so my earlier inline comment was mistaken).
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.
FYI, I switched to std::map in this review:
@sylvestre.ledru do you think it's possible to apply this patch and see if it work for your bug?
Btw I am still confused about the reproducibility - does clang produce the same PCH at least for the runs on the same revision?