CarlosAlbertoEnciso added a comment. In https://reviews.llvm.org/D46190#1135688, @rsmith wrote:
> The right way to handle this is to pass both the ultimately-selected > declaration and the declaration found by name lookup into the calls to > `MarkAnyDeclReferenced` and friends. We should mark the selected declaration > as `Used` (when appropriate), and mark the found declaration as `Referenced`. > > We should not be trying to reconstruct which using declarations are in scope > after the fact. Only declarations actually found by name lookup and then > selected by the context performing the lookup should be marked referenced. > (There may be cases where name lookup discards equivalent lookup results that > are not redeclarations of one another, though, and to handle such cases > properly, you may need to teach name lookup to track a list of found > declarations rather than only one.) Following your comments, I have replaced the scope traversal with `LookupName` in order to find the declarations. But there are 2 specific cases: - using-directives The `LookupName` function does not record the using-directives. With this patch, there is an extra option to store those directives in the lookup results, to be processed during the setting of the 'Referenced' bit. - namespace-alias I am aware of your comment on the function that recursively traverse the namespace alias, but I can't see another way to do it. If you have a more specific idea, I am happy to explore it. For both cases, may be `LookupName` function can have that functionality and store in the results any namespace-directives and namespace-alias associated with the given declaration. ================ Comment at: lib/Sema/Sema.cpp:1879 +void Sema::MarkNamespaceAliasReferenced(NamedDecl *ND) { + if (ND && !ND->isReferenced()) { + NamespaceAliasDecl *NA = nullptr; ---------------- probinson wrote: > You could do this as an early return and reduce indentation in the rest of > the method. > ``` > if (!ND || ND->isReferenced()) > return; > ``` > Corrected to ``` 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()) { ---------------- probinson wrote: > Initializing this to nullptr is redundant, as you set NA in the while-loop > expression. Removed the `nullptr` initialization. ================ 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) { ---------------- probinson wrote: > `\brief` is unnecessary, as we have auto-brief turned on. Removed the `\brief` format. ================ Comment at: lib/Sema/Sema.cpp:1903 + if (auto *NNS = SS ? SS->getScopeRep() + : E ? dyn_cast<DeclRefExpr>(E)->getQualifier() + : nullptr) { ---------------- probinson wrote: > This dyn_cast<> can be simply a cast<>. Changed the dyn_cast<> to cast<> ================ Comment at: lib/Sema/Sema.cpp:1916 + if ((USD = dyn_cast<UsingShadowDecl>(DR)) && !USD->isReferenced()) { + if (USD->getTargetDecl() == D) { + USD->setReferenced(); ---------------- probinson wrote: > 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. Not available in the new patch. ================ 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()) ---------------- probinson wrote: > You verified that his is a namespace earlier, so use cast<> not dyn_cast<>. Not available in the new patch. https://reviews.llvm.org/D46190 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits