================ @@ -0,0 +1,210 @@ +//===--- SwapBinaryOperands.cpp ----------------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +#include "ParsedAST.h" +#include "SourceCode.h" +#include "refactor/Tweak.h" +#include "support/Logger.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Expr.h" +#include "clang/AST/OperationKinds.h" +#include "clang/AST/Stmt.h" +#include "clang/Basic/LangOptions.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Tooling/Core/Replacement.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Casting.h" +#include "llvm/Support/Error.h" + +namespace clang { +namespace clangd { +namespace { +/// Check whether it makes logical sense to swap operands to an operator. +/// Assignment or member access operators are rarely swappable +/// while keeping the meaning intact, whereas comparison operators, mathematical +/// operators, etc. are often desired to be swappable for readability, avoiding +/// bugs by assigning to nullptr when comparison was desired, etc. +auto isOpSwappable(const BinaryOperatorKind Opcode) -> bool { + switch (Opcode) { + case BinaryOperatorKind::BO_Mul: + case BinaryOperatorKind::BO_Add: + case BinaryOperatorKind::BO_Cmp: + case BinaryOperatorKind::BO_LT: + case BinaryOperatorKind::BO_GT: + case BinaryOperatorKind::BO_LE: + case BinaryOperatorKind::BO_GE: + case BinaryOperatorKind::BO_EQ: + case BinaryOperatorKind::BO_NE: + case BinaryOperatorKind::BO_And: + case BinaryOperatorKind::BO_Xor: + case BinaryOperatorKind::BO_Or: + case BinaryOperatorKind::BO_LAnd: + case BinaryOperatorKind::BO_LOr: + case BinaryOperatorKind::BO_Comma: + return true; + // Noncommutative operators: + case BinaryOperatorKind::BO_Div: + case BinaryOperatorKind::BO_Sub: + case BinaryOperatorKind::BO_Shl: + case BinaryOperatorKind::BO_Shr: + case BinaryOperatorKind::BO_Rem: + // Member access: + case BinaryOperatorKind::BO_PtrMemD: + case BinaryOperatorKind::BO_PtrMemI: + // Assignment: + case BinaryOperatorKind::BO_Assign: + case BinaryOperatorKind::BO_MulAssign: + case BinaryOperatorKind::BO_DivAssign: + case BinaryOperatorKind::BO_RemAssign: + case BinaryOperatorKind::BO_AddAssign: + case BinaryOperatorKind::BO_SubAssign: + case BinaryOperatorKind::BO_ShlAssign: + case BinaryOperatorKind::BO_ShrAssign: + case BinaryOperatorKind::BO_AndAssign: + case BinaryOperatorKind::BO_XorAssign: + case BinaryOperatorKind::BO_OrAssign: + return false; + } + return false; +} + +/// Some operators are asymmetric and need to be flipped when swapping their +/// operands +/// @param[out] Opcode the opcode to potentially swap +/// If the opcode does not need to be swapped or is not swappable, does nothing +void swapOperator(BinaryOperatorKind &Opcode) { + switch (Opcode) { + case BinaryOperatorKind::BO_LT: + Opcode = BinaryOperatorKind::BO_GT; + return; + case BinaryOperatorKind::BO_GT: + Opcode = BinaryOperatorKind::BO_LT; + return; + case BinaryOperatorKind::BO_LE: + Opcode = BinaryOperatorKind::BO_GE; + return; + case BinaryOperatorKind::BO_GE: + Opcode = BinaryOperatorKind::BO_LE; + return; + case BinaryOperatorKind::BO_Mul: + case BinaryOperatorKind::BO_Add: + case BinaryOperatorKind::BO_Cmp: + case BinaryOperatorKind::BO_EQ: + case BinaryOperatorKind::BO_NE: + case BinaryOperatorKind::BO_And: + case BinaryOperatorKind::BO_Xor: + case BinaryOperatorKind::BO_Or: + case BinaryOperatorKind::BO_LAnd: + case BinaryOperatorKind::BO_LOr: + case BinaryOperatorKind::BO_Comma: + case BinaryOperatorKind::BO_Div: + case BinaryOperatorKind::BO_Sub: + case BinaryOperatorKind::BO_Shl: + case BinaryOperatorKind::BO_Shr: + case BinaryOperatorKind::BO_Rem: + case BinaryOperatorKind::BO_PtrMemD: + case BinaryOperatorKind::BO_PtrMemI: + case BinaryOperatorKind::BO_Assign: + case BinaryOperatorKind::BO_MulAssign: + case BinaryOperatorKind::BO_DivAssign: + case BinaryOperatorKind::BO_RemAssign: + case BinaryOperatorKind::BO_AddAssign: + case BinaryOperatorKind::BO_SubAssign: + case BinaryOperatorKind::BO_ShlAssign: + case BinaryOperatorKind::BO_ShrAssign: + case BinaryOperatorKind::BO_AndAssign: + case BinaryOperatorKind::BO_XorAssign: + case BinaryOperatorKind::BO_OrAssign: + return; + } +} + +/// Swaps the operands to a binary operator +/// Before: +/// x != nullptr +/// ^ ^^^^^^^ +/// After: +/// nullptr != x +class SwapBinaryOperands : public Tweak { +public: + const char *id() const final; + + bool prepare(const Selection &Inputs) override; + Expected<Effect> apply(const Selection &Inputs) override; + std::string title() const override { + return llvm::formatv("Swap operands to {0}", + Op ? Op->getOpcodeStr() : "binary operator"); + } + llvm::StringLiteral kind() const override { + return CodeAction::REFACTOR_KIND; + } + bool hidden() const override { return false; } + +private: + const BinaryOperator *Op; +}; + +REGISTER_TWEAK(SwapBinaryOperands) + +bool SwapBinaryOperands::prepare(const Selection &Inputs) { + for (const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor(); + N && !Op; N = N->Parent) { + // Stop once we hit a block, e.g. a lambda in the if condition. + if (llvm::isa_and_nonnull<CompoundStmt>(N->ASTNode.get<Stmt>())) + return false; ---------------- 5chmidti wrote:
IMO this can stay (with an updated comment), it makes sure that the selection point is in the 'scope' of the binary operator, not from somewhere inside a lambda for example (`5 < [](){ ^return 1; }`) https://github.com/llvm/llvm-project/pull/78999 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits