[PATCH] D126676: [clang] Disallow differences in defines used for creating and using PCH

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D126676#3587243 <https://reviews.llvm.org/D126676#3587243>, @rnk wrote:

> I don't have a great answer here, but yes, dllexport macro norms sort of run directly counter to the normal ways that people use PCH. It seems like projects will need to have a library module / PCH file for building a DLL and then a PCH / module for consuming the DLL.

This isn’t about sharing a PCH between building and consuming a DLL - that’d be highly unusual use of PCHs indeed.

It’s about building a library as both DLL and static library at the same time. For that, you’d have a PCH directory containing two versions of the same PCH, one built with `-DDLLEXPORT` and one without. When building the object files for the two copies of the library (for the static or DLL version), the compiler gets called with or without `-DDLLEXPORT`, and is pointed at the directory containing the two copies of the PCH.

GCC successfully picks the right one out of the directory, while Clang just picks the first one it tries. Clang also does try to pick the right one out of a directory with multiple copies (picking the right one, if you have one built with `-DFOO=1` and one with `-DFOO=2`), but Clang considers that if you have a define entirely missing (PCH built with `-DDLLEXPORT` but the same define absent when including the header, or vice versa) that they’re probably compatible, and pick the first one out of the directory.

I can somewhat understand the rationale - if not using a PCH directory, but only one single file, Clang does allow you to include that while defining `-DUNRELATED`. (Clang’s implementation does have a 10 year old todo about checking that the differing define indeed was unrelated and not used by the PCH in any way.) GCC doesn’t allow that, but requires there to be an exact match between all `-D` arguments between when the PCH was built and consumed.

So with GCC’s stricter logic here, I think it would make sense to make Clang equally strict, but I wonder if that’d break Clang-only usecases that have ended up relying on Clang’s more tolerant behaviour here. (There even are tests testing specifically the fuzzy allowing of differences.) Maybe the safest would be to only make it strict in the case when picking the right one out of a directory with alternatives?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126676