[lworld] RFR: 8199429: [lworld] value type (re)constructors expressions value type should not be discardable [v2]

[Jesper Steen Møller]

On Thu, 10 Oct 2024 22:10:38 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Jesper Steen Møller has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8199429: [lworld] Add error example for CheckExamples test
>
> I think the "reconstrcutor" proposal is out-of-date; the latest approach would be "deconstructors" https://github.com/openjdk/amber/pull/94 and "Derived record creation" https://openjdk.org/jeps/468
> 
> For `@MustUse`: in the "CheckReturnValue at Google" presentation in [JVMLS 2024](https://openjdk.org/projects/mlvm/jvmlangsummit/agenda.html), Google finds that the pattern is simple: All builder/fluent methods can have return values ignored by default, and all other methods should have return values "must use" by default. Only a tiny fraction of the methods can have their return value ignored, like `List.add`.

Thanks for the feedback, @liach 

Re. reconstructors: Good point, the case of an JCExpressionStatement "p with { y = 0}" is of course easy to flag as an error, (just from structure alone, like x+1 in the original motivation).

As for the annotation: I could not find the Google presentation on YT, but I guess that what you're saying is that the annotation would make sense for any and all methods which return a computed value without causing any mutations or side-effects. Since values are immutable, only side effects need consideration, so the bar is a lot lower (and enforceable in practice, since we don't want to break people code, even nonsensical parts).

So in the [example](https://github.com/openjdk/valhalla/blob/c2130bd2c1a8e21cb6d547c84d335fdd12325d67/test/langtools/tools/javac/valhalla/must_use/MustUseValueTest.java#L31-L34) given in the PR, the method `next()` could be automatically annotated as `@MustUse` because we know that the entire method call _will be useless_ if the value is not put to good use.
If, however, the method's control flow was ever to leave the method (even by throwing an exception) or a value was assigned to any field (on some referenced object, obviously not on the value), the `@MustUse` annotation could not be set automatically.

As for identity objects, I'm guessing that developers in the future could use `@MustUse` on methods where the only sane usage is to use the returned value, such as in the builder pattern as you mention, to alert their callers of the intent, since we can't apply the same rules as with the value types.

Or am I overthinking this? 😀

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

PR Comment: https://git.openjdk.org/valhalla/pull/1274#issuecomment-2406231106