hokein added inline comments.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:263
+ DeclContextLookupResult LookupResult;
+ switch (DC->getDeclKind()) {
+ case Decl::TranslationUnit:
----------------
sammccall wrote:
> sammccall wrote:
> > explain this list somewhat?
> > e.g. these are the declcontexts which form a namespace where conflicts can
> > occur?
> > and why function doesn't belong here
> I think you want to walk up DC while isTransparentContext()
yeah, that's a good point. added.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:315
+ // conflicts if we perform the rename.
+ // (!) DeclContext::lookup doesn't perform lookup local decls in a
+ // function-kind DeclContext.
----------------
sammccall wrote:
> Can you explain this a bit more (e.g. why?)
>
> My guess is given:
>
> ```
> void foo() {
> int bar;
> }
> ```
>
> `foo` is a DC but `bar` is not declared directly within it (rather within the
> CompoundStmt that is its body), therefore will not be looked up.
>
> In which case an explanation like "Note that the enclosing DeclContext may
> not be the enclosing scope, particularly for function-local declarations, so
> this has both false positives and negatives." might help.
Done. This comment is about why we exclude functionDecl, moved it to the lookup
function there.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89790/new/
https://reviews.llvm.org/D89790
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
