ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments.
================ Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:278 + const auto *Target = + llvm::dyn_cast<NamedDecl>(Ref.Targets.front()->getCanonicalDecl()); + auto It = ParamToNewName.find(Target); ---------------- why not `llvm::cast`? We'd rather fail early (during the cast) than postpone it until we dereference `Target` a few lines further ================ Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:285 + return; + const size_t OldNameLen = Target->getName().size(); + // If decl is unnamed in destination we pad the new name to avoid gluing ---------------- Could we measure the token length instead? There are various bizzare cases when the source text is different and we definitely don't want to accidentally crash on those, e.g. ``` int a = N\ A\ ME; ``` ================ Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:295 + RenamingErrors = + llvm::joinErrors(std::move(RenamingErrors), std::move(Err)); + } ---------------- Wow, TIL I learned something new. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68937/new/ https://reviews.llvm.org/D68937 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
