Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-4191

Java code generator equals() has sub-optimal generated paths (Coverity)



    • Type: Bug
    • Status: Open
    • Priority: Trivial
    • Resolution: Unresolved
    • Affects Version/s: 0.10.0
    • Fix Version/s: None
    • Component/s: Java - Compiler
    • Labels:


      If you make a thrift file with a struct containing a single i32 and then generate java code for it, the equals() generated method will contain the following - this is annotated by Coverity to show the issue:

          	assignment: Assigning: this_present_interRequestPriority = true.
       813    boolean this_present_interRequestPriority = true;
          	assignment: Assigning: that_present_interRequestPriority = true.
       814    boolean that_present_interRequestPriority = true;
          	CID 26017 (#1 of 2): 'Constant' variable guards dead code (DEADCODE) [select issue]
          	cond_const: Condition this_present_interRequestPriority, taking true branch. Now the value of this_present_interRequestPriority is equal to 1.
       815    if (this_present_interRequestPriority || that_present_interRequestPriority) {
          	const: At condition this_present_interRequestPriority, the value of this_present_interRequestPriority must be equal to 1.
          	dead_error_condition: The condition this_present_interRequestPriority must be true.
          	const: At condition that_present_interRequestPriority, the value of that_present_interRequestPriority must be equal to 1.
          	dead_error_condition: The condition that_present_interRequestPriority must be true.
       816      if (!(this_present_interRequestPriority && that_present_interRequestPriority))
      CID 26017 (#2 of 2): 'Constant' variable guards dead code (DEADCODE)
      dead_error_line: Execution cannot reach this statement: return false;.
          	Local variable that_present_interRequestPriority is assigned only once, to a constant value, making it effectively constant throughout its scope. If this is not the intent, examine the logic to see if there is a missing assignment that would make that_present_interRequestPriority not remain constant. Otherwise, declaring that_present_interRequestPriority as final will suppress this defect.
       817        return false;
       818      if (this.interRequestPriority != that.interRequestPriority)
       819        return false;
       820    }

      Since "interRequestPriority" is not an optional i32, I assume we're skipping the call to that.isSetInterRequestPriority(). If both are always true, we can remove the unnecessary branch or make it final as Coverity suggested.




            • Assignee:
              jking3 James E. King III
            • Votes:
              0 Vote for this issue
              1 Start watching this issue


              • Created: