RFR: 8255968: Confusing error message for inaccessible constructor

[Guoxiong Li]

On Mon, 23 Nov 2020 15:42:50 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Hi all,
>> 
>> When using inaccessible constructor, compiler would output some confusing error message occasionally. These confusing error message is related to the constructor order. See [JDK-8255968](https://bugs.openjdk.java.net/browse/JDK-8255968) for more information.
>> This patch solves this bug, regardless of the construction order, compiler can always output expected message.
>> Thank you for taking the time to review.
>> 
>> Best Regards.
>
> While I somewhat sympathize with the problem you are trying to solve, note that, by doing this, the output of the compiler will regress in case where not _all_ the overload candidates are not accessible. E.g.
> 
> m(1): //client
> 
> m();
> private m(int);
> 
> In this case I guess it's less clear as what "should" happen: does the user want to call the first, or second version of `m` ? Both cases are possible - e.g. the user might accidentally call the private version of some method (in which the compiler is right in pointing out that the callsite is wrong), or the user indeed might want to access the private method (in which case your patch to shift the focus onto the access error might seem more appropriate).
> 
> I'm a bit skeptical that there exist a silver bullet for these kind of mixed cases. Technically speaking, from a JLS perspective, non-accessible methods should not even take place in overload resolution - so the current compiler behavior is cases where both accessible and inaccessible methods are present seems more appropriate.
> 
> In other words, if we really want to improve the status quo, we should detect cases where _all_ candidates are not accessible. Or, we should question whether javac's behavior of tagging inaccessible methods with the kind `WRONG_MTH`/`WRONG_MTHS` is correct in the first place.
> 
> It would be nice, I guess, if we could make the inaccessible error more powerful, so that we could keep adding inaccessble symbols as we go along, so that javac could display something like "cannot resolve symbol `m` - but some inaccessible symbols have been found <list>".

Currently, the compiler output the error message depending on the order of the methods. Please see the below test cases.

- the `private Test(int x) {}` is at the end of other(one or more) method(s).
class T8255968 {
    Test c = new Test(0);
}
class Test {
    private Test(String[] x) {}
    Test(String x) {}
    private Test(int x) {}  // at the end
}

current output:
error: Test(int) has private access in Test
    Test c = new Test(0);
             ^
1 error
- the `private Test(int x) {}` is not at the end of other(only one) method.
class T8255968 {
    Test c = new Test(0);
}
class Test {
    private Test(int x) {}  // not at the end
    Test(String x) {} // can be private, output same error message
}

current output:
error: incompatible types: int cannot be converted to String
    Test c = new Test(0);
                      ^
Note: Some messages have been simplified; recompile with -Xdiags:verbose to get full output
1 error

- the `private Test(int x) {}` is not at the end of other(more than one) methods.
class T8255968 {
    Test c = new Test(0);
}
class Test {
    private Test(String x) {}
    private Test(int x) {}  // not at the end
    Test(String[] x) {}
}

current output:
error: no suitable constructor found for Test(int)
    Test c = new Test(0);
             ^
    constructor Test.Test(String) is not applicable
      (argument mismatch; int cannot be converted to String)
    constructor Test.Test(String[]) is not applicable
      (argument mismatch; int cannot be converted to String[])
1 error
These output messages which depending on the order of the methods would confuse users. Because the order of the methods  should not have any influence.

@mcimadamore According to your comment, I want to unify the output messages as below.

- the `private Test(int x) {}` is at the end of other(one or more) method(s).
class T8255968 {
    Test c = new Test(0);
}
class Test {
    private Test(String[] x) {}
    Test(String x) {}
    private Test(int x) {}  // at the end
}

expected output:
error: no suitable constructor found for Test(int)
    Test c = new Test(0);
             ^
    constructor Test.Test(String[]) is not applicable
      (argument mismatch; int cannot be converted to String[])
    constructor Test.Test(int) is not applicable
      (Test(int) has private access in Test)
    constructor Test.Test(String) is not applicable
      (argument mismatch; int cannot be converted to String)
1 error
- the `private Test(int x) {}` is not at the end of other(only one) method.
class T8255968 {
    Test c = new Test(0);
}
class Test {
    private Test(int x) {}  // not at the end
    Test(String x) {} // can be private, output same error message
}

expected output:
error: no suitable constructor found for Test(int)
    Test c = new Test(0);
             ^
    constructor Test.Test(int) is not applicable
      (Test(int) has private access in Test)
    constructor Test.Test(String) is not applicable
      (argument mismatch; int cannot be converted to String)
1 error

- the `private Test(int x) {}` is not at the end of other(more than one) methods.
class T8255968 {
    Test c = new Test(0);
}
class Test {
    private Test(String x) {}
    private Test(int x) {}  // not at the end
    Test(String[] x) {}
}

expected output:
error: no suitable constructor found for Test(int)
    Test c = new Test(0);
             ^
    constructor Test.Test(String) is not applicable
      (argument mismatch; int cannot be converted to String)
    constructor Test.Test(int) is not applicable
      (Test(int) has private access in Test)
    constructor Test.Test(String[]) is not applicable
      (argument mismatch; int cannot be converted to String[])
1 error

And the following two situations are not modified.
class T8255968 {
    Test c = new Test(0);
}
class Test {
    private Test(int x) {}
}

current and expected output:
error: Test(int) has private access in Test
    Test c = new Test(0);
             ^
1 error
class T8255968 {
    Test c = new Test(0);
}
class Test {
    Test(String x) {}
}

current and expected output:
error: incompatible types: int cannot be converted to String
    Test c = new Test(0);
                      ^
Note: Some messages have been simplified; recompile with -Xdiags:verbose to get full output
1 error

I implement this unified format and test the feature locally. @mcimadamore If you agree with these unified output messages, I would update my code for reviewing.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1389