Preserve integer literal formatting in type hint

[curiousleo]

Closes #10766, implementing "Proposal 1".

This is a more localised implementation than the implementation proposed in #10767, which changes Asttype.constant. It is based on this comment.

User-visible changes

The goal of this PR is to preserve the original formatting of integer literals (int32, int64 and nativeint) in the "Did you mean ...?" hint.

Before

    1 | let _ = Int64.add 0xAA_BBl 2L;;
                          ^^^^^^^^
    Error: This expression has type int32 but an expression was expected of type
             int64
      Hint: Did you mean `43707L'?

After

    1 | let _ = Int64.add 0xAA_BBl 2L;;
                          ^^^^^^^^
    Error: This expression has type int32 but an expression was expected of type
             int64
      Hint: Did you mean `0xAA_BBL'?

API changes

In Typecore.error:

• Pattern_type_clash contains an optional Parsetree.pattern_desc instead of Typetree.pattern_desc
• Expr_type_clash contains an optional Parsetree.expression_desc instead of Typetree.expression_desc

In both cases, the description was and is only used to output the type hint. The difference is that the Parsetree descriptions contain the original formatting of the integer literal, but the Typetree descriptions do not. Using the Parsetree descriptions instead of the Typetree descriptions allows us to output the integer literals with their original formatting in the type hint.

[curiousleo]

Ready for review. @gasche and @Octachron, you have the context from #10767, perhaps you would be willing to take a look?

[Octachron]

Unsurprisingly, I prefer this implementation to #10767 .

[curiousleo]

Thanks for the suggestions for improvement! All comments addressed, ready for another look.

[Octachron]

The current state looks good to me, I am adding this PR to the post-freeze milestone.

[curiousleo]

The current state looks good to me, I am adding this PR to the post-freeze milestone.

Awesome! I'll rebase on top of trunk and resolve the conflicts.

[Octachron]

Note that you may want to wait before resolving conflicts, there is still many weeks left before the end of the freeze and Changes is quite conflict-prone.

[curiousleo]

Note that you may want to wait before resolving conflicts, there is still many weeks left before the end of the freeze and Changes is quite conflict-prone.

Cool, good to know. I only saw your message after rebasing, but I'll keep in mind that conflicts in Changes are to be expected.

Will I be notified when I should rebase to get the PR ready for merging? (This is my first contribution to the OCaml compiler.)

[Octachron]

Yes, I will notify you when the merge windows reopen.

[Octachron]

@curiousleo : the merge windows has reopened if you wish to rebase your PR it can be merged.

[curiousleo]

Rebase is done!

[Octachron]

The PR looks good to me: it improves error messages by doing less work overall.

[curiousleo]

All review comments addressed.

[curiousleo]

@Octachron thanks for taking a look! What's the next step?

[gasche]

Octachron is on holidays today and tomorrow, I'll go ahead and merge if the CI is happy.

[curiousleo]

Octachron is on holidays today and tomorrow, I'll go ahead and merge if the CI is happy.

Thanks! I fixed the line length issue that was flagged by CI:

./typing/typecore.ml:386.81: [long-line] line is over 80 columns
./typing/typecore.ml:2785.81: [long-line] line is over 80 columns

You may need to re-trigger CI so it runs on the new commit, sorry.

[Octachron]

Merged, thanks for your contribution and patience!

[curiousleo]

Merged, thanks for your contribution and patience!

My pleasure, thanks for the thoughtful reviews!