v1nh1shungry added a comment.
Oops! Thank you for your thoughtful consideration.
You're right, but I currently get stuck there and need more time. And I prefer
to clear the existing patches simultaneously if you don't mind.
Or do you think I should merge the modification of this patch into there and
give up this?
================
Comment at:
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:212
// Produce replacements to add the qualifiers.
std::string Qualifier = printUsingNamespaceName(Ctx, *TargetDirective) +
"::";
for (auto Loc : IdentsToQualify) {
----------------
v1nh1shungry wrote:
> tom-anders wrote:
> > v1nh1shungry wrote:
> > > We can replace `printUsingNamespaceName` with `printNamespaceScope` here
> > > so that we can get `a::foobar()` in the test.
> > >
> > > However, it can sometimes cause redundancy such as in the 10th test.
> > >
> > > And I don't know whether it is worth it. WDYT?
> > Just making sure I understood this correctly:
> >
> > If you replace `printUsingNamespaceName` with `printNamespaceScope`, then...
> >
> > - ...in the test you added it would result in `a::foobar()` instead of
> > `a::b::foobar()` (which is better)
> > - ... but in this test (which is the 10th test if I counted correctly):
> >
> > ```
> > namespace a::b { struct Foo {}; }
> > using namespace a;
> > using namespace a::[[b]];
> > using namespace b;
> > int main() { Foo F;}
> > ```
> > what would be the result..? would you get `a::Foo` instead of `a::b::Foo`?
> >
> Sorry, I mean the next test. I read `10` from the inlay hint but I forgot the
> index starts from `0` :(
>
> The test I want to mention:
> ```
> namespace a::b { struct Foo {}; }
> using namespace a;
> using namespace a::b;
> using namespace [[b]];
> int main() { Foo F;}
> ```
>
> We will get `a::b::Foo` in both the 10th and 11th tests. So in the 10th test,
> we don't get any benefits and don't sacrifice anything. In the 11th test, we
> get more redundancy than the existing version.
>
> Apologize again for my mistake.
FYI, we have a discussion left here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138028/new/
https://reviews.llvm.org/D138028
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
