sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:263
+ DeclContextLookupResult LookupResult;
+ switch (DC->getDeclKind()) {
+ case Decl::TranslationUnit:
----------------
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
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:263
+ DeclContextLookupResult LookupResult;
+ switch (DC->getDeclKind()) {
+ case Decl::TranslationUnit:
----------------
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()
================
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.
----------------
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89790/new/
https://reviews.llvm.org/D89790
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
