[PATCH] D46190: For a referenced declaration, mark any associated usings as referenced.

[Paul Robinson via Phabricator via cfe-commits]

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