[PATCH] D70556: clang/Modules: Refactor CompilerInstance::loadModule, NFC

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith created this revision.
dexonsmith added reviewers: bruno, Bigcheese, jkorous, rsmith.
Herald added a subscriber: ributzka.

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

Files:
  clang/include/clang/Frontend/CompilerInstance.h
  clang/include/clang/Lex/ModuleLoader.h
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Lex/Pragma.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70556.230505.patch
Type: text/x-patch
Size: 26022 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191121/c1b4c41b/attachment-0001.bin>