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

Fri Jul 29 06:59:09 PDT 2022

mstorsjo added a comment.

In D126676#3687227 <https://reviews.llvm.org/D126676#3687227>, @dexonsmith wrote:

> I haven’t reviewed the details of the patch and won’t have time to do so (at least for a while), but the description of the intended (more narrow) scope SGTM. With the scope limited to GCC directories this should be fine.
>
> There are cases where it’s safe to have mismatched defines (e.g., if a define can be proven during a cheap dependency scan not to matter for a given PCH, it’s better to canonicalize away the define and share the artifact more broadly) but if I understand correctly this patch won’t preclude compiler smarts like that.

Yup. Any implementation of such logic hasn’t materialized during the 10 years since todos hinting that we should implement that, but it doesn’t seem to be a big practical issue either, outside of false positive matches with gcc PCH directories - so I guess it’s not a high priority in practice.

> If I’m right, then I think this can go in once @rnk is happy. (If I’m wrong, maybe better to wait for one of the people I pinged.)

Yeah, I’d be happy to add a validation level enum to make things a bit clearer - I’ll try to revise the patch soon.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126676