iains marked an inline comment as done. iains added inline comments.
================ Comment at: clang/lib/Sema/SemaModule.cpp:814-815 + diagExportedUnnamedDecl(S, UnnamedDeclKind::Namespace, D, BlockStart); + else + ; // We allow an empty named namespace decl. + } else if (DC->getRedeclContext()->isFileContext() && !isa<EnumDecl>(D)) ---------------- erichkeane wrote: > iains wrote: > > ChuanqiXu wrote: > > > I think we should remove it. So that the above `if` could be further > > > merged. > > we can remove the else ; but we cannot merge the !HasName into the if (that > > changes the logic of the expression such that empty & unnamed namespace > > decls get passed to checkExportedDeclContext()) - which emits a diagnostic > > not expected by the standard. > > > > For me, the 'else ;' makes it clear what the alternative is doing (and > > there's a comment as well) - but if the general opinions is to remove it, > > that's also fine. > I'd much prefer we did NOT do the empty else/semicolon here. This ends up > causing a warning with GCC ( warning: suggest braces around empty body in an > ‘else’ statement [-Wempty-body] ) and is jsut strange. I don't mind a > comment, but the else/; is strange. I committed: rG92fed06f800a: [C++20][Modules] Remove an empty statement [NFC]. to resolve this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122119/new/ https://reviews.llvm.org/D122119 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
