probinson added a comment. Style comments. The two new Sema methods (for namespaces and using references) are C++ specific, so SemaDeclCXX.cpp would seem like a more appropriate home for them.
================ Comment at: lib/Sema/Sema.cpp:1879 +void Sema::MarkNamespaceAliasReferenced(NamedDecl *ND) { + if (ND && !ND->isReferenced()) { + NamespaceAliasDecl *NA = nullptr; ---------------- You could do this as an early return and reduce indentation in the rest of the method. ``` if (!ND || ND->isReferenced()) return; ``` ================ Comment at: lib/Sema/Sema.cpp:1880 + if (ND && !ND->isReferenced()) { + NamespaceAliasDecl *NA = nullptr; + while ((NA = dyn_cast<NamespaceAliasDecl>(ND)) && !NA->isReferenced()) { ---------------- Initializing this to nullptr is redundant, as you set NA in the while-loop expression. ================ Comment at: lib/Sema/Sema.cpp:1891 +/// \brief Mark as referenced any 'using declaration' that have introduced +/// the given declaration in the current context. +void Sema::MarkUsingReferenced(Decl *D, CXXScopeSpec *SS, Expr *E) { ---------------- `\brief` is unnecessary, as we have auto-brief turned on. ================ Comment at: lib/Sema/Sema.cpp:1903 + if (auto *NNS = SS ? SS->getScopeRep() + : E ? dyn_cast<DeclRefExpr>(E)->getQualifier() + : nullptr) { ---------------- This dyn_cast<> can be simply a cast<>. ================ Comment at: lib/Sema/Sema.cpp:1916 + if ((USD = dyn_cast<UsingShadowDecl>(DR)) && !USD->isReferenced()) { + if (USD->getTargetDecl() == D) { + USD->setReferenced(); ---------------- You could sink the declaration of USD like so: ``` for (auto *DR : S->decls()) if (auto *USD = dyn_cast<UsingShadowDecl>(DR)) if (!USD->isReferenced() && USD->getTargetDecl() == D) { ``` Also I would put braces around the 'for' loop body, even though it is technically one statement. ================ Comment at: lib/Sema/Sema.cpp:1927 + // Check if the declaration was introduced by a 'using-directive'. + auto *Target = dyn_cast<NamespaceDecl>(DC); + for (auto *UD : S->using_directives()) ---------------- You verified that his is a namespace earlier, so use cast<> not dyn_cast<>. https://reviews.llvm.org/D46190 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits