sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Thank you!
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:418
+ if (const auto *ME = dyn_cast<MemberExpr>(E))
+ if (const auto *TE = dyn_cast<CXXThisExpr>(ME->getBase()))
+ if (TE->isImplicit())
----------------
oops, I forgot one detail: we want ME->getBase()->IgnoreImpCasts()
(in `void nonConstMethod() { constMethod(); }` there's an ImplicitCastExpr in
there...
================
Comment at:
clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp:326
+ // Function Pointers
+ {R"cpp(struct Handlers {
+ void (*handlerFunc)(int);
----------------
can you move the function pointer + NSInteger tests into a separate block with
`ExtraArgs = {"-xc"}`?
================
Comment at:
clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp:338
+ })cpp"},
+ // We don't preserve types through typedefs.
+ {R"cpp(typedef long NSInteger;
----------------
dgoldman wrote:
> sammccall wrote:
> > This comment is inaccurate: if the expression has type `NSInteger`, that
> > code will be written.
> >
> > The issue is that the type of the expression **isn't** `NSInteger`, it's
> > just `int`. **arithmetic** doesn't produce typedef types in the way I think
> > you're expecting.
> >
> > If the expression was something that **was** an `NSInteger` (like a call to
> > a function whose declared return type is NSInteger) then that type would be
> > preserved.
> Gotcha, thanks. Deleted the comment, should I delete this test case as well?
Up to you, I think the test is fine.
I'd add a comment "arithmetic on typedef types yields plain integer types" or
so to explain what it's about.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124486/new/
https://reviews.llvm.org/D124486
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
