aaron.ballman added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7784-7785 "because namespace %1 does not enclose namespace %2">; +def err_invalid_declarator_in_export : Error<"cannot export %0 here " + "because it had be declared in %1.">; def err_invalid_declarator_global_scope : Error< ---------------- aaron.ballman wrote: > I think this diagnostic text is more clear based on the standards text you > cited. This would also come with a note diagnostic to point to the previous > declaration. Given that this is not intended for p6 specifically, I think my suggestion is incorrect. But I am also not certain your diagnostic is either, but it's really hard to tell given where our current support is for modules. All of the other compilers suggest that an unqualified id is expected to be found, and I tend to agree -- there's no declaration there *to* export, just the type specifier for a declaration. This makes me think the issue is elsewhere and perhaps we shouldn't even be getting into `diagnoseQualifiedDeclaration()`. ================ Comment at: clang/lib/Sema/SemaDecl.cpp:5748-5750 + else if (isa<ExportDecl>(Cur)) + Diag(Loc, diag::err_invalid_declarator_in_export) + << Name << cast<NamedDecl>(DC) << SS.getRange(); ---------------- ChuanqiXu wrote: > aaron.ballman wrote: > > I don't believe this is sufficient to cover [module.interface]p6. I tried > > out the example from the paragraph in the standard and we still silently > > accept it. > Yes, this patch is not intended to cover [module.interface]p6. The intention > is to fix the crash since I feel it would take a longer time to support > [module.interface]p6. And crash is not good any way. I plan to support > [module.interface]p6 in successive patches. Do you happy with this? > BTW, I have a plan to support clang's c++20 module to a workable state. And I > am working on the reachable definition in > https://eel.is/c++draft/module.reach#3 and > https://eel.is/c++draft/module.global.frag#3. Thank you for the clarification! Fixing the crash is definitely a step in the right direction. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112903/new/ https://reviews.llvm.org/D112903 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
