denis-fatkulin marked 3 inline comments as done.
denis-fatkulin added inline comments.
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:395
// Bail out in templated classes, as it is hard to spell the class name,
i.e
// if the template parameter is unnamed.
----------------
kadircet wrote:
> could you move this comment closer to the `isTempated` if statement below?
Fixed
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:401
+
+ // The refactoring is meaningless for unnamed classes.
+ const auto *Parent = MD->getParent();
----------------
kadircet wrote:
> not just classes, but also for cases with unnamed namespaces.
>
> could you change this to look like:
> ```
> for(DeclContext *DC = MD->getParent(); DC; DC = DC->getParent()) {
> if (auto *ND = llvm::dyn_cast<NamedDecl>(DC)) {
> if(ND->getDeclName().isEmpty())
> return false;
> }
> }
> ```
It's a useful remark. Tanks!
================
Comment at: clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp:97
+ // nesting.
+ EXPECT_UNAVAILABLE(R"cpp(
+ struct Foo {
----------------
kadircet wrote:
> can you also have a test inside an unnamed namespace?
Test case added.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143638/new/
https://reviews.llvm.org/D143638
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
