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

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D126676#3687491 <https://reviews.llvm.org/D126676#3687491>, @mstorsjo wrote:

> In D126676#3687227 <https://reviews.llvm.org/D126676#3687227>, @dexonsmith wrote:
>
>> 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.

Note, FWIW, that clang-scan-deps is being used to canonicalize command-lines as of the last year; e.g., there are patches up (maybe committed?) to remove unused search paths for implicitly-discovered explicit modules. Finding unused defines is a bit harder and less urgent (search path explosion had to be fixed for performance parity with on-demand implicit modules, which have an unsound optimization of ignoring search path differences), but there’s now a framework where it can be reasonably implemented if/when it’s identified as a major bottleneck in builds.

(Fallout from unsoundness in on-demand implicit modules is a big reason progress has been slow in this area.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126676