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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
