ChuanqiXu added a comment.
> if we do not adjust the typo fixes, we will regress diagnostics.
What the kind of diagnostics will be regressed? I mean, it looks weird to me
that we suggest typo fixes from hidden names.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11181
+ "%select{declaration|definition|default argument|"
+ "explicit specialization|partial specialization}0 of %1 is private to module
"
+ "'%2'">;
----------------
ChuanqiXu wrote:
> The 'private' here makes me to think about module private fragment while it
> is not true. I prefer to refactor it to something like "it is not exported".
let's try rewording `private` to `invisible`?
================
Comment at: clang/include/clang/Sema/Sema.h:2373
+ // Determine whether the module M belongs to the current TU.
+ bool isModuleUnitOfCurrentTU(const Module *M) const;
+
----------------
Let's use `!DeclBase::isInAnotherModuleUnit()` instead now.
================
Comment at: clang/include/clang/Sema/Sema.h:2375-2383
+ /// Determine whether the module MA is part of the same named module as MB.
+ bool arePartOfSameNamedModule(const Module *MA, const Module *MB) const {
+ if (!MA || MA->isGlobalModule())
+ return false;
+ if (!MB || MB->isGlobalModule())
+ return false;
+ return MA->getPrimaryModuleInterfaceName() ==
----------------
nit: I prefer this to be a freestanding function in Module.h. This looks
slightly not good within Sema.
================
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:
> > 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.
Could we tried to address this? The change smells not so good.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145965/new/
https://reviews.llvm.org/D145965
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
