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

Chuanqi Xu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 4 01:47:58 PDT 2023


ChuanqiXu added inline comments.


================
Comment at: clang/lib/Sema/SemaLookup.cpp:3912-3936
+          if (Visible) {
+            if (!FM)
+              break;
+            assert (D->hasLinkage() && "an imported func with no linkage?");
+            // Unless the module is a defining one for the
+            bool Ovr = true;
+            for (unsigned I = 0; I < CodeSynthesisContexts.size(); ++I) {
----------------
ChuanqiXu wrote:
> What's the intention for the change? And why is the current behavior bad without this?
> What's the intention for the change? And why is the current behavior bad without this?




================
Comment at: clang/lib/Sema/SemaLookup.cpp:3912-3936
+          if (Visible) {
+            if (!FM)
+              break;
+            assert (D->hasLinkage() && "an imported func with no linkage?");
+            // Unless the module is a defining one for the
+            bool Ovr = true;
+            for (unsigned I = 0; I < CodeSynthesisContexts.size(); ++I) {
----------------
ChuanqiXu wrote:
> ChuanqiXu wrote:
> > What's the intention for the change? And why is the current behavior bad without this?
> > What's the intention for the change? And why is the current behavior bad without this?
> 
> 
Oh, I understand why I feel the code is not good since the decl with internal linkage or module linkage shouldn't be visible. So even if there are problems, we should handle them elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145965



More information about the cfe-commits mailing list