Page MenuHomePhabricator

Implement a feature to show line numbers in diagnostics
Needs ReviewPublic

Authored by ken-matsui on May 6 2022, 12:28 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

I implemented a feature to show line numbers with left margins in diagnostics like GCC-9 does. Ref: https://developers.redhat.com/blog/2019/03/08/usability-improvements-in-gcc-9

Diff Detail

Event Timeline

ken-matsui created this revision.May 6 2022, 12:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 6 2022, 12:28 AM
ken-matsui requested review of this revision.May 6 2022, 12:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 6 2022, 12:28 AM

Remove unnecessary includes

cjdb added subscribers: rsmith, cjdb.May 9 2022, 9:16 AM

I'm in favour of this, but @rsmith warned me a little while ago that we mightn't be able to have stuff like this on by default because scripts probably depend on specific compiler output. Hyrum's Law strikes again :(

(I personally think we should have it on by default and give a way to opt out, rather than opt in.)

xbolva00 added a comment.EditedMay 9 2022, 10:04 AM

I'm in favour of this, but @rsmith warned me a little while ago that we mightn't be able to have stuff like this on by default because scripts probably depend on specific compiler output. Hyrum's Law strikes again :(

(I personally think we should have it on by default and give a way to opt out, rather than opt in.)

I believe even more tools depend on gcc and gcc devs were able to turn it on by default, so kinda bad excuse, especially with opt out flag. Can you check how they did it? Or they just did it with no useless worries?

This feature is already implemented in GCC, so in my opinion, it would be great if we could turn this on by default.

I also added a feature to separate location info and a message in https://reviews.llvm.org/D125175.
Should its feature be avoided from turning on by default as well?

cjdb added a comment.May 10 2022, 9:33 AM

I support turning it on by default. I'm simply passing on the message.

I support turning it on by default. I'm simply passing on the message.

+1, I support on by default as well.

Thank you all so much :)))