[PATCH] D107690: [Modules] Do not remove failed modules after the control block phase

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Made the first review pass and `return Failure` makes sense to me as recovery isn't the best idea at this point. Still want to check more thoroughly if the removed code for `SUBMODULE_UMBRELLA_HEADER` and `SUBMODULE_UMBRELLA_DIR` has any load-bearing side-effects.

Have no opinion on updating post-control-block functions to llvm::Error.



================
Comment at: clang/test/VFS/module-header-mismatches.m:2
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s;TEST_DIR;%/t;g" %t/sed-overlay.yaml > %t/overlay.yaml
----------------
Didn't know about `split-file`, that's nice.


================
Comment at: clang/test/VFS/umbrella-mismatch.m:4
-
-// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -F %S/Inputs -fsyntax-only %s -Wno-atimport-in-framework-header -verify
-// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs -fsyntax-only %s -Wno-atimport-in-framework-header -verify
----------------
Are you deleting this test case because it is subsumed by module-header-mismatches.m?

Also if it makes sense to delete this test, need to do more cleanup because UsesFoo.framework and Foo.framework seem to be used only from this test. Though I'm not 100% sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107690