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

[ConstraintSystem] Improve score label in the debug output so that we can easily recognize which score is more/less impactful. #72842

Open
kntkymt opened this issue Apr 4, 2024 · 0 comments
Labels
feature A feature request or implementation triage needed This issue needs more specific labels

Comments

@kntkymt
Copy link

kntkymt commented Apr 4, 2024

Motivation

Environment

swift-driver version: 1.90.11.1 Apple Swift version 5.10 (swiftlang-5.10.0.13 clang-1500.3.9.4)
Target: arm64-apple-macosx14.0

In current score label in the debug output,
We cannot easily recognize which score is more impactful.

class Animal {}
class Cat: Animal {}
func f(_ a: [Cat?]) { }
func f(_ a: [Any]) { }
func f(_ a: [Animal]) {}
let a: [Cat] = [Cat()]

f(a)
// swiftc -Xfrontend -debug-constraints Source.swift

---Solver statistics---
--- Solution #0 ---
Fixed score: [component: collection upcast conversion(s), value: 1] [component: value to optional promotion(s), value: 1]
...

--- Solution #1 ---
Fixed score: [component: collection upcast conversion(s), value: 1] [component: empty-existential conversion(s), value: 1]
...

--- Solution #2 ---
Fixed score: [component: value-to-pointer conversion(s), value: 1]
...

---Solution---
Fixed score: [component: value-to-pointer conversion(s), value: 1]
...

Full Output

We cannot recognize which is more/less impactful between value to optional promotion of Solution#0 and empty-existential conversion of Solution#1.

In Addition, Since Solution is value-to-pointer conversion(s), We can guess value-to-pointer conversion is least impactful, but it is also ambiguous.

Proposed solution

Add the number indicates impactful in the debug output. We can recognize which score is more/less impactful comparing the number.

---Solver statistics---
--- Solution #0 ---
Fixed score: [component: #13 collection upcast conversion(s), value: 1] [component: #14 value to optional promotion(s), value: 1]
...

--- Solution #1 ---
Fixed score: [component: #13 collection upcast conversion(s), value: 1] [component: #15 empty-existential conversion(s), value: 1]
...

--- Solution #2 ---
Fixed score: [component: #17 value-to-pointer conversion(s), value: 1]
...

---Solution---
Fixed score: [component: #17 value-to-pointer conversion(s), value: 1]
...

I think these format is also good.

// as suffix?
[component: value-to-pointer conversion(s) #17, value: 1]

// print reversed index value instead of enum value?
[component: #3 value-to-pointer conversion(s), value: 1]

I think It will be easy to implement this feature.

https://github.com/hborla/swift/blob/6676fbfd19781dd6ed1d59bb84146aff336c24d4/include/swift/Sema/ConstraintSystem.h#L1152

  void print(llvm::raw_ostream &out) const {
    bool hasNonDefault = false;
    for (unsigned int i = 0; i < NumScoreKinds; ++i) {
      if (Data[i] != 0) {
        out << " [component: ";
        out << "#";
        out << std::to_string(i);
        out << " ";
        out << getNameFor(ScoreKind(i));
        out << "(s), value: ";
        out << std::to_string(Data[i]);
        out << "]";
        hasNonDefault = true;
      }
    }
    if (!hasNonDefault) {
      out << " <default ";
      out << *this;
      out << ">";
    }
  }

Alternatives considered

Restore the score format used before these PRs were implemented.

#60387
#60927

---Solver statistics---
--- Solution #0 ---
Fixed score: 0 0 0 0 0 0 0 0 0 0 0 0 0 1 1 0 0 0 0 0 0
...

--- Solution #1 ---
Fixed score: 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 1 0 0 0 0 0
...

--- Solution #2 ---
Fixed score: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0
...

---Solution---
Fixed score: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0
...

However, I think it is better to show name of component. So I propose mixed format showing name and number.

Additional information

If it is acceptable, I'd like to challenge creating PR by myself 😎 (It's my first time to contribute).

@kntkymt kntkymt added feature A feature request or implementation triage needed This issue needs more specific labels labels Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A feature request or implementation triage needed This issue needs more specific labels
Projects
None yet
Development

No branches or pull requests

1 participant