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

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11226
+def err_module_private_use : Error<
+  "%select{declaration|definition|default argument|"
+  "explicit specialization|partial specialization}0 of %1 is private to module "
----------------
I only see `declaration` and `definition` cases covered for this diagnostic in the tests.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11230
+def err_module_internal_use : Error<
+  "%select{declaration|definition|default argument|"
+  "explicit specialization|partial specialization}0 of %1 is internal to "
----------------
I only see the `declaration` case covered for this diagnostic. Please add tests to cover the other cases.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11234
+def note_suggest_export : Note<
+  "export the %select{declaration|definition|default argument|"
+  "explicit specialization|partial specialization}0 to make it available">;
----------------
I only see the `declaration` and `definition` case covered in the tests we should cover all possible diagnostics. Especially in new code, bugs often come about on code we don't cover.


================
Comment at: clang/lib/Sema/SemaLookup.cpp:4520
+  for (/**/; DI != DE; ++DI) {
+    if (!LookupResult::isVisible(SemaRef, *DI, /*ForTypo*/ true))
       break;
----------------
Fix to conform w/ [bugprone-argument-comment](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html)


================
Comment at: clang/lib/Sema/SemaLookup.cpp:4533
   for (/**/; DI != DE; ++DI) {
-    if (LookupResult::isVisible(SemaRef, *DI)) {
+    if (LookupResult::isVisible(SemaRef, *DI, /*ForTypo*/ true)) {
       if (!AnyVisibleDecls) {
----------------



================
Comment at: clang/lib/Sema/SemaLookup.cpp:4619
   // names that exactly match.
-  if (!LookupResult::isVisible(SemaRef, ND) && Name != Typo)
+  if (!LookupResult::isVisible(SemaRef, ND, /*ForTypo*/ true) && Name != Typo)
     return;
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145965