[PATCH] D69033: [clangd] Improve symbol qualification in DefineInline code action

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 5 04:36:13 PST 2019


ilya-biryukov added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:196
     //                ~~~~~ -> we need to qualify Bar not x.
-    if (!ND->getDeclContext()->isNamespace())
+    if (!ND->getLexicalDeclContext()->isNamespace())
       return;
----------------
Why do we need to change this?
My understanding is that we want semantic decl context, not the lexical one here.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:200
+    std::string Qualifier;
+    // FIXME: Also take using directives and namespace aliases inside function
+    // body into account.
----------------
NIT: it looks like this FIXME belongs to the `getQualification` function itself.


================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1721
+  namespace ns1 {
+  void foo();
+  namespace qq { void test(); }
----------------
NIT: could you indent functions inside the namespaces?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69033/new/

https://reviews.llvm.org/D69033





More information about the cfe-commits mailing list