ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land.
LGTM generally. ================ Comment at: clang/lib/Sema/SemaDecl.cpp:12957-12958 + // units. + if (getLangOpts().CPlusPlus20 && getLangOpts().CPlusPlusModules && + !ModuleScopes.empty() && ModuleScopes.back().Module->isHeaderUnit()) { + if (VDecl->getFormalLinkage() == Linkage::ExternalLinkage && ---------------- `getLangOpts().CPlusPlus20` is redundant. It is also good to define a helper interface `withinHeaderUnit` in Sema (not required). ================ Comment at: clang/lib/Sema/SemaDecl.cpp:15116-15117 + // units. + if (getLangOpts().CPlusPlus20 && getLangOpts().CPlusPlusModules && + !ModuleScopes.empty() && ModuleScopes.back().Module->isHeaderUnit()) { + if (FD->getFormalLinkage() == Linkage::ExternalLinkage && ---------------- ditto Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140261/new/ https://reviews.llvm.org/D140261 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits