[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D153114#4604438 <https://reviews.llvm.org/D153114#4604438>, @nridge wrote:

> In D153114#4603579 <https://reviews.llvm.org/D153114#4603579>, @sammccall wrote:
>
>> - to identify what module names we must have PCMs for in order to build a given TU (either an open file, or a module we're building as PCM)
>> - to build a database mapping module name => filename, which we compose with the CDB to know how to build a PCM for a given module name
>>
>> I think it would be good to clearly separate these. The latter is simpler, more performance-critical, async, and is probably not used at all if the build system can tell us this mapping.
>> The latter is more complex, and will always be needed synchronously for the mainfile regardless of the build system.
>
> I think the second instance of "the latter" was meant to be "the former" :)

Oops, yes!

>> At a high level, `import` decls should be processed with the preamble: [...]
>> However they don't appear in a prefix of the file
>
> This point is not obvious to me: do you mean that there are coding styles that place `import` statements further down in the file, after non-trivial declarations?

Yes:

- inside a module unit, imports must appear directly underneath the module declaration, above non-trivial declarations
- but non-module TUs can place imports anywhere in the file (at TU scope)

> If not, what stops us from altering the definition of "preamble" to something like "everything before the first declaration which is not an import/module declaration"?

We *have* to find and build *all* the PCMs that will be imported - unlike with the current preamble PCH, we can't give up on some of them and fall back to textual inclusion.

We could also have the preamble region cover the imports that happen to be at the top - i.e. the PCH would contain these ImportDecls. But I don't know that actually achieves anything meaningful over stopping the preamble before the imports. Either way, each time we use the preamble we'll have load the imported PCMs.

---

I just realized my ideas for simplifying the dep scanning may not work: as well as named modules we could also have imports of header units, which presumably means we need to at least build an include path and stat files on it to resolve a header unit name :-(


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

https://reviews.llvm.org/D153114