[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

Yeah, it is indeed a problem that `Sema::isModuleUnitOfCurrentTU` doesn't work for implementation units. And I have been thinking about this for a while. My thought is that the root cause may be that we shouldn't push the module interface to `Sema::ModuleScopes()` for the implementation unit. (We need some other small refactoring). I feel this is more natural and consistent. I thought to take this one but we didn't use implementation units in the downstream and there is no related issue reports. So I didn't start to work on it... If you are not hurry, I'd like to take this. Otherwise I'd suggest you to try the above method I mentioned.

BTW, I suggest we file an issue first if we want to do something. So that we can avoid solving the same problem.



================
Comment at: clang/lib/Sema/SemaModule.cpp:304-305
   switch (MDK) {
+  case ModuleDeclKind::NoModule:
+    llvm_unreachable("we should be building some kind of module");
   case ModuleDeclKind::Interface:
----------------
This can be a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145965