[all-commits] [llvm/llvm-project] 5cca62: clang/Modules: Refactor CompilerInstance::loadModu...

[Duncan P. N. Exon Smith via All-commits]

  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 5cca622310c10fdf6f921b6cce26f91d9f14c762
      https://github.com/llvm/llvm-project/commit/5cca622310c10fdf6f921b6cce26f91d9f14c762
  Author: Duncan P. N. Exon Smith <dexonsmith at apple.com>
  Date:   2019-11-22 (Fri, 22 Nov 2019)

  Changed paths:
    M clang/include/clang/Frontend/CompilerInstance.h
    M clang/include/clang/Lex/ModuleLoader.h
    M clang/lib/Frontend/CompilerInstance.cpp
    M clang/lib/Lex/Pragma.cpp

  Log Message:
  -----------
  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