[PATCH] D67607: [clangd] Fix a crash when renaming operator.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 16 02:59:44 PDT 2019


hokein marked 2 inline comments as done.
hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:77
     return ReasonToReject::UnsupportedSymbol;
+  if (const auto *FD = llvm::dyn_cast<FunctionDecl>(&RenameDecl)) {
+    if (FD->isOverloadedOperator())
----------------
ilya-biryukov wrote:
> Should we more generally disable renaming all non-identifier names? Each of them seems to require special treatment.
> 
> A few examples that come to mind:
> - conversion operators, e.g. `operator int()`;
> - user-defined literals, e.g. `std::chrono::seconds operator ""s(int seconds)`;
> - destructors;
I think so, that would be safer (this should be done in a separate patch), it seems that some cases are being handled by the rename library already.

- for constructor and destructor, the renamelib handle them well;
- for `operator int()`/user-defined literals, the renamelib doesn't find a corresponding decl, means that we will return "no symbol found error", though the message is a bit confusing;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67607





More information about the cfe-commits mailing list