dgoldman added inline comments.
================ Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:170 + if (E->hasPlaceholderType(BuiltinType::PseudoObject)) { + if (const auto *PR = dyn_cast<ObjCPropertyRefExpr>(E)) { + if (PR->isMessagingSetter()) { ---------------- dgoldman wrote: > sammccall wrote: > > dgoldman wrote: > > > sammccall wrote: > > > > E->getObjCProperty()? > > > Hmm I'm not sure when that would be useful looking at the logic there, > > > can you give an example just to make sure I understand what it would > > > handle? > > it's similar to the cast you have, but in addition to handling `foo.bar` it > > handles parens like `(foo.bar)`, commas like `(0, foo.bar)` and casts. All > > together I think this covers more cases where a pseudo-object can be used > > as an lvalue and potentially modified. > Gotcha, thanks. That makes sense, but it doesn't look like that has any > usages and I'm not sure if it's safe to call from this context (e.g. if we > can have a non ObjC property PseudoObject expr). I guess I could modify > getObjCProperty to return nullptr in those invalid cases, WDYT? @sammccall WDYT about this? Leave as is? 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