[llvm] r294555 - [GlobalISel] Simplify StringRef parameters. NFC.

[Daniel Sanders via llvm-commits]

> 'const' on StringRef parameters adds no guarantees. Remove it.

This is incorrect, this const guarantees that the function does not allow assignment to that variable. The string itself is constant in both cases but 'StringRef' can be locally re-assigned to another string whereas 'const StringRef' cannot.

It was my intention to ensure that the InsnVarName always referred to the string provided by the caller so I'd like you to revert this unless there's a good reason to make InsnVarName assignable.

> On 9 Feb 2017, at 02:50, Ahmed Bougacha via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: ab
> Date: Wed Feb  8 20:50:01 2017
> New Revision: 294555
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=294555&view=rev
> Log:
> [GlobalISel] Simplify StringRef parameters. NFC.
> 
> 'const' on StringRef parameters adds no guarantees. Remove it.
> 
> Modified:
>    llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
> 
> Modified: llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp?rev=294555&r1=294554&r2=294555&view=diff
> ==============================================================================
> --- llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp (original)
> +++ llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp Wed Feb  8 20:50:01 2017
> @@ -160,8 +160,7 @@ public:
> 
>   /// Emit a C++ expression that checks the predicate for the OpIdx operand of
>   /// the instruction given in InsnVarName.
> -  virtual void emitCxxPredicateExpr(raw_ostream &OS,
> -                                    const StringRef InsnVarName,
> +  virtual void emitCxxPredicateExpr(raw_ostream &OS, StringRef InsnVarName,
>                                     unsigned OpIdx) const = 0;
> };
> 
> @@ -173,7 +172,7 @@ protected:
> public:
>   LLTOperandMatcher(std::string Ty) : Ty(Ty) {}
> 
> -  void emitCxxPredicateExpr(raw_ostream &OS, const StringRef InsnVarName,
> +  void emitCxxPredicateExpr(raw_ostream &OS, StringRef InsnVarName,
>                             unsigned OpIdx) const override {
>     OS << "MRI.getType(" << InsnVarName << ".getOperand(" << OpIdx
>        << ").getReg()) == (" << Ty << ")";
> @@ -188,7 +187,7 @@ protected:
> public:
>   RegisterBankOperandMatcher(const CodeGenRegisterClass &RC) : RC(RC) {}
> 
> -  void emitCxxPredicateExpr(raw_ostream &OS, const StringRef InsnVarName,
> +  void emitCxxPredicateExpr(raw_ostream &OS, StringRef InsnVarName,
>                             unsigned OpIdx) const override {
>     OS << "(&RBI.getRegBankFromRegClass(" << RC.getQualifiedName()
>        << "RegClass) == RBI.getRegBank(" << InsnVarName << ".getOperand("
> @@ -199,7 +198,7 @@ public:
> /// Generates code to check that an operand is a basic block.
> class MBBOperandMatcher : public OperandPredicateMatcher {
> public:
> -  void emitCxxPredicateExpr(raw_ostream &OS, const StringRef InsnVarName,
> +  void emitCxxPredicateExpr(raw_ostream &OS, StringRef InsnVarName,
>                             unsigned OpIdx) const override {
>     OS << InsnVarName << ".getOperand(" << OpIdx << ").isMBB()";
>   }
> @@ -216,7 +215,7 @@ public:
> 
>   /// Emit a C++ expression that tests whether the instruction named in
>   /// InsnVarName matches all the predicate and all the operands.
> -  void emitCxxPredicateExpr(raw_ostream &OS, const StringRef InsnVarName) const {
> +  void emitCxxPredicateExpr(raw_ostream &OS, StringRef InsnVarName) const {
>     OS << "(";
>     emitCxxPredicateListExpr(OS, InsnVarName, OpIdx);
>     OS << ")";
> @@ -235,7 +234,7 @@ public:
>   /// Emit a C++ expression that tests whether the instruction named in
>   /// InsnVarName matches the predicate.
>   virtual void emitCxxPredicateExpr(raw_ostream &OS,
> -                                    const StringRef InsnVarName) const = 0;
> +                                    StringRef InsnVarName) const = 0;
> };
> 
> /// Generates code to check the opcode of an instruction.
> @@ -247,7 +246,7 @@ public:
>   InstructionOpcodeMatcher(const CodeGenInstruction *I) : I(I) {}
> 
>   void emitCxxPredicateExpr(raw_ostream &OS,
> -                            const StringRef InsnVarName) const override {
> +                            StringRef InsnVarName) const override {
>     OS << InsnVarName << ".getOpcode() == " << I->Namespace
>        << "::" << I->TheDef->getName();
>   }
> @@ -273,7 +272,7 @@ public:
> 
>   /// Emit a C++ expression that tests whether the instruction named in
>   /// InsnVarName matches all the predicates and all the operands.
> -  void emitCxxPredicateExpr(raw_ostream &OS, const StringRef InsnVarName) const {
> +  void emitCxxPredicateExpr(raw_ostream &OS, StringRef InsnVarName) const {
>     emitCxxPredicateListExpr(OS, InsnVarName);
>     for (const auto &Operand : Operands) {
>       OS << " &&\n(";
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits