Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initialize implicit this. #6908

Merged
merged 4 commits into from
Dec 6, 2024
Merged

Initialize implicit this. #6908

merged 4 commits into from
Dec 6, 2024

Conversation

smillst
Copy link
Member

@smillst smillst commented Dec 3, 2024

No description provided.

mernst
mernst previously approved these changes Dec 3, 2024
Copy link
Member

@mernst mernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mernst mernst enabled auto-merge (squash) December 3, 2024 19:05
@mernst
Copy link
Member

mernst commented Dec 3, 2024

@smillst Tests are failing.

@mernst mernst assigned smillst and unassigned mernst Dec 3, 2024
@jyoo980
Copy link
Contributor

jyoo980 commented Dec 4, 2024

The following test case is failing:

@EnsuresNonEmptyIf(result = false, expression = "this")
public boolean isEmpty2() {
  return size() == 0 ? true : false;
}

With the error:

error: [contracts.conditional.postcondition] conditional postcondition is not satisfied when isEmpty2 returns false.
    return size() == 0 ? true : false;
    ^
  found   : this is @UnknownNonEmpty
  required: this is @NonEmpty

And I'm wondering now if it's still a valid test case. Is there enough information to determine whether the container is indeed non-empty in the case that this method returns false?

For context, the implementation of size() used in this method is:

@Pure
@Override
public int size() {
  return -1;
}

Which overrides the size() method provided by the JDK.

@smillst
Copy link
Member Author

smillst commented Dec 4, 2024

And I'm wondering now if it's still a valid test case. Is there enough information to determine whether the container is indeed non-empty in the case that this method returns false?

I think it is. The non-empty checker is assuming that size is implemented correctly. Also the other two similar test cases in that file still pass.

  @EnsuresNonEmptyIf(result = false, expression = "this")
  public boolean isEmpty3() {
    return size() == 0;
  }
  @Pure
  @Override
  @EnsuresNonEmptyIf(result = false, expression = "this")
  public boolean isEmpty() {
    if (size() == 0) {
      return true;
    } else {
      return false;
    }
  }

So I think this failure indicates a bug somewhere in dataflow. Conditional expressions have a different implementation than if statements in dataflow. I'm working on this now.

@jyoo980
Copy link
Contributor

jyoo980 commented Dec 4, 2024

@smillst I see, thanks!

@smillst smillst assigned mernst and unassigned smillst Dec 6, 2024
@mernst mernst merged commit 5071e45 into typetools:master Dec 6, 2024
36 checks passed
@mernst mernst deleted the implicitThis branch December 6, 2024 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants