hokein added inline comments.
================
Comment at: include/clang/Sema/CodeCompleteConsumer.h:271
+ using VisitedContextSet = llvm::SmallPtrSet<DeclContext*, 8>;
+
----------------
ilya-biryukov wrote:
> Given that lookup does not visit the same `DeclContext` twice, we're probably
> fine with vector here, rather than the set.
> On the other hand, using set gives a better understanding that the order of
> items is not specified.
> I don't think we should necessarily changes the code here, just something to
> think about.
>
Good point. Yeah, technically, there should be no duplicated DeclContext.
================
Comment at: include/clang/Sema/Lookup.h:791
+ /// \param Ctx the context which Sema begins to visit.
+ virtual void BeginVisitContext(DeclContext *Ctx) {};
};
----------------
bkramer wrote:
> ilya-biryukov wrote:
> > Maybe rename it to `VisitedContext` ? Seems more in-line with `FoundDecl`.
> > `BeginVisitContext` also suggest there should be `EndVisitContext`
> Semicolon is not needed here.
Renamed it to `EnteredContext` based on offline discussion.
================
Comment at: unittests/Sema/CodeCompleteTest.cpp:53
+ for (const auto *Context : VisitedContexts)
+ if (const auto *NS = llvm::dyn_cast<NamespaceDecl>(Context))
+ NSNames.push_back(NS->getQualifiedNameAsString());
----------------
ilya-biryukov wrote:
> Why onyl namespaces and not all `DeclContext`s?
Yeah, this is intended (the method name has indicated it), we only verify the
results for namespaces in the test (for simplicity).
Repository:
rC Clang
https://reviews.llvm.org/D42071
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
