erichkeane 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)) ---------------- 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. 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
