[all-commits] [llvm/llvm-project] abcf7c: [clang][modules] Serialize `Module::DefinitionLoc`

[Jan Svoboda via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: abcf7ce45794839a473a236d55d163016cde5ba6
      https://github.com/llvm/llvm-project/commit/abcf7ce45794839a473a236d55d163016cde5ba6
  Author: Jan Svoboda <jan_svoboda at apple.com>
  Date:   2023-07-17 (Mon, 17 Jul 2023)

  Changed paths:
    M clang/include/clang/Serialization/ASTBitCodes.h
    M clang/lib/Lex/ModuleMap.cpp
    M clang/lib/Serialization/ASTReader.cpp
    M clang/lib/Serialization/ASTWriter.cpp

  Log Message:
  -----------
  [clang][modules] Serialize `Module::DefinitionLoc`

This is a prep patch for avoiding the quadratic number of calls to `HeaderSearch::lookupModule()` in `ASTReader` for each (transitively) loaded PCM file. (Specifically in the context of `clang-scan-deps`).

This patch explicitly serializes `Module::DefinitionLoc` so that we can stop relying on it being filled by the module map parser. This change also required change to the module map parser, where we used the absence of `DefinitionLoc` to determine whether a file came from a PCM file. We also need to make sure we consider the "containing" module map affecting when writing a PCM, so that it's not stripped during serialization, which ensures `DefinitionLoc` still ends up pointing to the correct offset. This is intended to be a NFC change.

Reviewed By: benlangmuir

Differential Revision: https://reviews.llvm.org/D150292


  Commit: 227f71995804fa5df3f917ae3a7b1499cd24726c
      https://github.com/llvm/llvm-project/commit/227f71995804fa5df3f917ae3a7b1499cd24726c
  Author: Jan Svoboda <jan_svoboda at apple.com>
  Date:   2023-07-17 (Mon, 17 Jul 2023)

  Changed paths:
    M clang/include/clang/Lex/PreprocessorOptions.h
    M clang/lib/Serialization/ASTReader.cpp
    M clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
    M clang/test/ClangScanDeps/header-search-pruning-transitive.c

  Log Message:
  -----------
  [clang][modules][deps] Avoid checks for relocated modules

Currently, `ASTReader` performs some checks to diagnose relocated modules. This can add quite a bit of overhead to the scanner: it requires looking up, parsing and resolving module maps for all transitively loaded module files (and all the module maps encountered in the search paths on the way). Most of those checks are not really useful in the scanner anyway, since it uses strict context hash and immutable filesystem, which prevent those scenarios in the first place.

This can speed up scanning by up to 30%.

Depends on D150292.

Reviewed By: benlangmuir

Differential Revision: https://reviews.llvm.org/D150320


  Commit: be014563f2f492658abcfa68cfaffc58a4ed7d9a
      https://github.com/llvm/llvm-project/commit/be014563f2f492658abcfa68cfaffc58a4ed7d9a
  Author: Jan Svoboda <jan_svoboda at apple.com>
  Date:   2023-07-17 (Mon, 17 Jul 2023)

  Changed paths:
    M clang/include/clang/Driver/Options.td
    M clang/lib/Lex/HeaderSearch.cpp
    A clang/test/Modules/no-check-relocated-fw-private.c

  Log Message:
  -----------
  [clang][modules][deps] Parse "FW_Private" module map even after loading "FW" PCM

When Clang loads a PCM that depends on another PCM describing framework module "FW", `ModuleMap` registers "FW" as known, without seeing the module map that defines it (or the adjacent "FW_Private" module map). Later, when looking at a header from "FW_Private", `ModuleMap` returns early due to having knowledge about "FW" and never associates that header with "FW_Private", leading to it being treated as textual. This behavior is caused by D150292, where the scanner stops calling `HeaderSearch::lookupModule()` eagerly for every loaded PCM.

This patch skips an early check when trying to figure out the framework module for a header, which ensures the "FW" and (most importantly) "FW_Private" module maps can be parsed even after loading "FW" from a PCM. Note that the `HeaderSearch::loadModuleMapFile()` function we not call unconditionally has caching behavior of its own, meaning it will avoid parsing module map file repeatedly.

Depends on D150320.

Reviewed By: benlangmuir

Differential Revision: https://reviews.llvm.org/D150478


  Commit: dba2b5c9314e1d127ee5200e739e6c8ca53a9831
      https://github.com/llvm/llvm-project/commit/dba2b5c9314e1d127ee5200e739e6c8ca53a9831
  Author: Jan Svoboda <jan_svoboda at apple.com>
  Date:   2023-07-17 (Mon, 17 Jul 2023)

  Changed paths:
    M clang/lib/Lex/ModuleMap.cpp
    A clang/test/Modules/no-check-relocated-fw-private-sub.c
    A clang/test/Modules/shadow-framework.m

  Log Message:
  -----------
  [clang][modules] Skip submodule & framework re-definitions in module maps

Before D150478, there were situations when Clang avoided parsing a module map because it was likely to re-define an already defined module (either by a PCM or by previously-found module map). Since Clang no longer performs that check and does parse the extra module map (due to the FW/FW_Private issue described in D150478), this patch re-implements the same semantics by skipping the duplicate definition of the framework module while parsing the module map.

Depends on D150478.

Reviewed By: benlangmuir

Differential Revision: https://reviews.llvm.org/D150479


Compare: https://github.com/llvm/llvm-project/compare/cbc4bbb85c72...dba2b5c9314e