On Wed, Nov 22, 2023 at 11:29:56 +0800, Chuanqi Xu wrote:
> Hi Ben,
> > would suffice for not rebuilding *successful* consumers while also
> > ensuring that consumers that *have not yet succeeded* get updated
> > diagnostics. This does mean that any diagnostic-affecting changes would
> > need to be included in this declhash (e.g., `[[deprecated]]`
> > attributes) as adding a deprecation should cause consumers to re-check
> > their usage of any such declarations. I'm not sure what to do with
> > consumers that have extant diagnostics…this would make it tough to
> > iteratively fix warnings as any implementation edit will leave the
> > declhash unchanged and the consumer won't give a fresh set of diagnostic
> > output with the changes included. However, is it possible to craft
> > something where:
> >
> > - a diagnostic comes "from" a module that is triggered by some usage
> > pattern in the consumer
> > - the fix doesn't change the declhash
> > - the importer no longer has the diagnostic (or has more diagnostics)
> >
> > It's late and I'm not well-versed in module details beyond how to build
> > them enough to come up with something right now, but if we can come up
> > with such a scenario, something even more sophisticated will be
> > necessary than the above function.
> IIUC, this paragraph is mainly about avoiding re-run of **failed**
> consumers. Do I understand correctly?

That is covered by the `preserve_mtime` behavior; this is about
successfully compiled bits that would have different diagnostics with
the same declhashes across its imported modules.

> For the fix the warnings case, if we fixed the warnings in the
> consumers, we need to rebuild the consumers for sure. And if we tried
> to fix or modify the warnings-related things in the modules, it should
> be the job of the tool to change.

But if we're only recompiling based on declhashes, then any
diagnostic-affecting change needs to be considered in the declhash.

> > The "Hash of declarations" strategy is workable with a `restat = 1`
> > feature though. Something like this:
> >
> > c++ -o pmi.cppm -fmodule-output=pmi.pch.tmp &&
> > copy_if_needed pmi.pch.tmp pmi.pch
> Oh, I felt you misunderstand my point. The method post here is
> different from the post in
> https://discourse.llvm.org/t/rfc-c-20-modules-introduce-thin-bmi-and-decls-hash/74755
> . It is not about the decls hash of **all the declarations in the
> module**. But only the decls hash used by certain consumer(s). So the
> working example in my mind is 'we'll always change the BMI; but it is
> up to the result of the query to change the consumers or not'.

Oh, it seems I did. The build strategy is something like:

    check_import_decl_hashes … out.o -- c++ …

where `check_import_decl_hashes` checks the declarations from importers
and sees if `out.o` needs its mtime updated (or ignored with `restat =
1`)? This `check_import_decl_hashes` function is considerably more
complex.

However, with ADL and overloading, if a new decl shows up in an imported
module, how do you know it doesn't change the consumer's behavior? Maybe
a new decl it makes a type satisfy a new constraint that choose a
different codepath? I'm not sure how one can say that any decl not
"used" for an arbitrary consumer doesn't matter. Would any
SFINAE-touched decl be added to the consumer list? All concept-mentioned
APIs? That still doesn't seem to consider things like "new
specialization of an operator was added and would be used if
recompiled". We'd need to consider any failed decl lookup as "relevant"
as well (something we just blithely ignore for shadowing headers that my
"appear" in an earlier `-I` path than it was found case eco-system wide
today). This feels far too subtle to leave alone though.

Basically, I think the "push declhash" mechanism sounds way more robust
than any "pull declhash" mechanism (to me).

--Ben