proposal: go/format: simplify old directive comments

[jimmyfrasche]

Proposal Details

51082 standardized Go directive comments. #37974 extended the syntax to allow for third party directives.

This left three old directives with custom formats: line, export, and extern.

38785 agreed to treat //export and //go:export as synonyms, though this did not go through the proposal process, was never implemented, and did not include line and extern.

We should revive and expand this to all three:

For line (and similarly for export and extern)

1. Accept both //line and //go:line in go1.N
2. Have gofmt rewrite //line to //go:line when the module language version >= go1.N

[jimmyfrasche]

cc @griesemer @adonovan @findleyr @lfolger from the related #68021

[adonovan]

Though I would like to live in the world where the proposed conversion process has completed, it does seem like quite a lot of effort for a marginal improvement, versus living with a few legacy Namespace="" directives.

[jimmyfrasche]

While that's the motivation that led me here, the simplification goes further into having a single simple rule without any special cases to remember. That makes things easier to learn and easier to use. Use covers a lot: writing a directive, using the iterator in the related issue, or adding the pattern to a syntax highlighter. It is minor and perhaps it is more trouble than its worth but may as well weigh that before locking it in further.

[lfolger]

Maybe I'm missing something but unless you fully deprecate the old forms (which seems very costly), you still have to special case and remember them, don't you?

[jimmyfrasche]

(I'm going to stick with line for discussion but everything applies to the other two as well.)

Making line and go:line synonyms allows two simplifications in tooling

• a parsed line can be reported as being in the go namespace since it's the same as go:line
• rendering a parsed line directive back to string doesn't need to handle the special case and can output go:line regardless of how it was originally written

Though minor those simplifications mean no one using go/ast to examine directives needs to worry about the special case. It's handled transparently.

Having gofmt rewrite line to go:line means that instances in the wild will die out so after some years effectively only go:line would exist and only the general directive format would need to be learned.

The code for handling line would still be there but it would be an implementation detail that essentially no one would need to worry about in practice. Even if the old form was entirely removed in newer versions of Go the code would still need to be there to handle compiling old codebases with a -lang from before the change, so it wouldn't really be possible to 100% remove it anyway.

[lfolger]

This makes sense but only if https://github.com/golang/go/issues/68021 is accepted because otherwise the only way to retrieve line directives is to actually parse the raw comments. Or are you suggesting to also change the values of the raw comments?

[jimmyfrasche]

Part 2 of the proposal is to have gofmt rewrite X → go:X (for modules with a language version after they become synonymous). It already moves comment directives so it's not entirely unprecedented and these directives are defined by Go so it is safe to rewrite them.

I think it's important to settle this matter first as #68021 locks the format into the compatibility agreement more tightly than it is now. If the iterator goes in and then later it's decided to do this it would be unfortunate if for compatibility reasons //go:line had to be reported as the empty namespace or, worse, the namespace depended on how it was authored even though they mean the same thing.

[apparentlymart]

I agree that it would be nice to live in a world where this were consistent, and share the concern that it seems somewhat disruptive to get there.

I find myself considering a variation of this proposal that might be less disruptive:

1. Support the go:-prefixed and unprefixed forms as synonyms as currently proposed, but...

2. ...make the tools treat the unprefixed forms as the normalized form, to avoid churning existing codebases that already use the old form.

   (In particular, gofmt would rewrite //go:line to just //line, rather than the other way around.)

This variant means that anyone hand-writing code still doesn't need to remember the rule -- they can just always write a go: prefix and let the formatter "correct" it if it's one of the legacy cases -- but the formatter would not unilaterally rewrite all of the existing code in older codebases.

I think this would also help avoid mistakes where someone uses a new toolchain to work on a codebase that's intended to still work on older Go versions and ends up with the comments rewritten into a shape that would not be accepted (possibly silently ignored?) by other toolchain versions. (Though I understand that the rule about using the version in go.mod was intended to mitigate that, and would probably do so well enough if this were the only drawback.)

Of course, it does mean that the exceptions must remain exceptional forever. This alternative does not lead to a future where the unprefixed forms might become invalid altogether.

[rsc]

For //export and //go:export, that was a detail that only affected cgo really. Changing everyone from //line to //go:line (and also /line .../ to /go:line .../) would be a much bigger change. It's also always going to be an exception because you can't use / / with any other go: comments but it's necessary for position info. In general the change doesn't seem worthwhile.

The same is true, to a lesser extent, for //extern.

[rsc]

This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group

[rsc]

It sounds like this is not worth doing. It has very little benefit and will inevitably cause churn and perhaps even introduce bugs.

[rsc]

Based on the discussion above, this proposal seems like a likely decline. — rsc for the proposal review group

[gopherbot]

No change in consensus, so declined. — aclements for the proposal review group