iains marked 2 inline comments as done. iains added inline comments.
================ Comment at: clang/lib/Sema/SemaLookup.cpp:2082 + Module *DeclModule = SemaRef.getOwningModule(D); + if (DeclModule && !DeclModule->isModuleMapModule() && + !SemaRef.isModuleUnitOfCurrentTU(DeclModule) && ---------------- ChuanqiXu wrote: > iains wrote: > > ChuanqiXu wrote: > > > We should check header units here. > > The point of checking module map modules was to avoid affecting clang > > modules with the change in semantics. > > > > At the moment, I am not sure why we could exclude lookup from finding decls > > in an imported header unit? > > > > At the moment, I am not sure why we could exclude lookup from finding decls > > in an imported header unit? > > We're not **excluding** decls from an imported header units. We're trying to > include the decls from the headers units. Since we've allowed decls with > internal linkage to appear to header units. We must need to allow decls with > internal linkage in header units to be found here. Otherwise, the following > example may fail: > > ``` > // foo.h > static int a = 43; > > // m.cppm > export module m; > import "foo.h"; > use of a... > ``` > > BTW, given the semantics of header units and clang modules are pretty > similar, we should use `isHeaderModule ` in most cases. ah yes, that special case, I should have remembered. OK I will adjust this at the next iteration (I think you mean `isHeaderLikeModule()`) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145965/new/ https://reviews.llvm.org/D145965 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
