clang/Modules: Refactor CompilerInstance::loadModule, NFC
authorDuncan P. N. Exon Smith <dexonsmith@apple.com>
Thu, 21 Nov 2019 18:39:55 +0000 (10:39 -0800)
committerDuncan P. N. Exon Smith <dexonsmith@apple.com>
Sat, 23 Nov 2019 02:23:47 +0000 (18:23 -0800)
commit5cca622310c10fdf6f921b6cce26f91d9f14c762
tree955fc1bebf982c40b454d98f6e8c0e661629bf0b
parent62335188f3ab6d11071802ab3c29963892ced00e
clang/Modules: Refactor CompilerInstance::loadModule, NFC

Refactor the logic on CompilerInstance::loadModule and a couple of
surrounding methods in order to clarify what's going on.

- Rename ModuleLoader::loadModuleFromSource to compileModuleFromSource
  and fix its documentation, since it never loads a module.  It just
  creates/compiles one.
- Rename one of the overloads of compileModuleImpl to compileModule,
  making it more obvious which one calls the other.
- Rename compileAndLoadModule to compileModuleAndReadAST.  This
  clarifies the relationship between this helper and its caller,
  CompilerInstance::loadModule (the old name implied the opposite
  relationship).  It also (correctly) indicates that more needs to be
  done to load the module than this function is responsible for.
- Split findOrCompileModuleAndReadAST out of loadModule.  Besides
  reducing nesting for this code thanks to early returns and the like,
  this refactor clarifies the logic in loadModule, particularly around
  calls to ModuleMap::cacheModuleLoad and
  ModuleMap::getCachedModuleLoad.  findOrCompileModuleAndReadAST also
  breaks early if the initial ReadAST call returns Missing or OutOfDate,
  allowing the last ditch call to compileModuleAndReadAST to come at the
  end of the function body.
    - Additionally split out selectModuleSource, clarifying the logic
      due to early returns.
    - Add ModuleLoadResult::isNormal and OtherUncachedFailure, so that
      loadModule knows whether to cache the result.
      OtherUncachedFailure was added to keep this patch NFC, but there's
      a chance that these cases were uncached by accident, through
      copy/paste/modify failures.  These should be audited as a
      follow-up (maybe we can eliminate this case).
    - Do *not* lift the setting of `ModuleLoadFailed = true` to
      loadModule because there isn't a clear pattern for when it's set.
      This should be reconsidered in a follow-up, in case it would be
      correct to set `ModuleLoadFailed` whenever no module is returned
      and the result is either Normal or OtherUncachedFailure.
- Add some header documentation where it was missing, and fix it where
  it was wrong.

This should have no functionality change.

https://reviews.llvm.org/D70556
clang/include/clang/Frontend/CompilerInstance.h
clang/include/clang/Lex/ModuleLoader.h
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Lex/Pragma.cpp