Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

documentation: exact syntax of line directives #24183

Closed
griesemer opened this issue Feb 28, 2018 · 8 comments
Closed

documentation: exact syntax of line directives #24183

griesemer opened this issue Feb 28, 2018 · 8 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@griesemer
Copy link
Contributor

griesemer commented Feb 28, 2018

This is a follow-up on #22662 and #24143 which implement line directives that permit column specification. While the overall mechanism is now defined and implemented, details of what makes a valid line directive were defined "ad-hoc". We should pin those details down.

The current implementation work as follows:

  1. Line directives have one of the following forms:
//line :<line>
//line :<line>:<col>
//line <filename>:<line>
//line <filename>:<line>:<col>
/*line :<line>*/
/*line :<line>:<col>*/
/*line <filename>:<line>*/
/*line <filename>:<line>:<col>*/
  1. Line directives of the //line form (using a line comment) must start at the beginning of the line or they are ignored (they are just ordinary line comments).
  2. A line directive specifies the position for the character immediately following the directive: For a //line comment, this is the first character of the next line, for a /*line comment this is the character immediately following the closing */.
  3. If no filename is given, the filename is empty ("") for line directives that don't specify a column number (for backward compatibility); and it is the currently used filename (actual filename, or filename specified by previous line directive) if a column number is present.
  4. When reporting (errors), a position p is printed with (a relative) column number if the most recent line directive before p (in the source) specified a column value, or if there is no line directive before p. Otherwise no column value is printed. If a column value is printed, it is relative to the line directive's column if on the same line to which the directive applies (which is the next line for //line directives), otherwise it is the absolute column (i.e., relative to the beginning of the line).

The following details are somewhat arbitrary and these are the ones we should decide upon:

a) A line directive that doesn't contain a : is ignored (considered a regular comment)

b) Column and line numbers are "scanned from the back": first the last :xxx is peeled off from the directive text. If xxx is a valid number > 0, it is accepted. Then the 2nd (now last) :xxx is peeled off and is accepted if xxx is a valid number > 0. The reason for this is to be able to deal with Windows filenames that may contain :. Finally, everything before that : and immediately after the initial line (incl. blank) is considered the filename.

There are several questions here:

  • What should happen if the xxx are not valid numbers? Should it be an error (to highlight a well-meant but wrong line directive)? What about an xxx of the form 123 (extra blank)? Currently, the compiler reports an error. Alternatively, the line directive could be silently ignored.

  • What should happen with filenames that contain leading and trailing blanks? Currently, the compiler accepts them. Alternatively, the line directive could trim whitespace. (As an aside, gccgo trims whitespace in the front but not in the back for filenames).

@griesemer griesemer added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Feb 28, 2018
@griesemer griesemer added this to the Go1.11 milestone Feb 28, 2018
@griesemer griesemer self-assigned this Feb 28, 2018
@griesemer
Copy link
Contributor Author

cc: @ianlancetaylor
cc: @rsc

@gopherbot
Copy link

Change https://golang.org/cl/97795 mentions this issue: go/scanner: recognize //line and /*line directives incl. columns

gopherbot pushed a commit that referenced this issue Mar 9, 2018
This change updates go/scanner to recognize the extended line
directives that are now also handled by cmd/compile:

//line filename:line
//line filename:line:column
/*line filename:line*/
/*line filename:line:column*/

As before, //-style line directives must start in column 1.
/*-style line directives may be placed anywhere in the code.
In both cases, the specified position applies to the character
immediately following the comment; for line comments that is
the first character on the next line (after the newline of the
comment).

The go/token API is extended by a new method

File.AddLineColumnInfo(offset int, filename string, line, column int)

which extends the existing

File.AddLineInfo(offset int, filename string, line int)

by adding a column parameter.

Adjusted token.Position computation is changed to take into account
column information if provided via a line directive: A (line-directive)
relative position will have a non-zero column iff the line directive
specified a column; if the position is on the same line as the line
directive, the column is relative to the specified column (otherwise
it is relative to the line beginning). See also #24183.

Finally, Position.String() has been adjusted to not print a column
value if the column is unknown (== 0).

Fixes #24143.

Change-Id: I5518c825ad94443365c049a95677407b46ba55a1
Reviewed-on: https://go-review.googlesource.com/97795
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@ianlancetaylor
Copy link
Contributor

Decision: an invalid line directive should cause a compiler error.

Decision: file names can contain any character--everything after the first space is part of the file name.

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 12, 2018
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 12, 2018
@griesemer
Copy link
Contributor Author

@ianlancetaylor Do you have a better suggestion for where to document this besides https://golang.org/cmd/compile/ ? (As an aside, that section also seems awfully incomplete).

@ianlancetaylor
Copy link
Contributor

The cmd/compile docs intentionally do not document magic comments that are only intended for use by the runtime.

Regardless of where else we document them, we should document them in cmd/compile.

I think the other obvious place to mention them would be in the docs for go generate (cmdGenerate in cmd/go/internal/generate/generate.go).

@gopherbot
Copy link

Change https://golang.org/cl/100235 mentions this issue: go/scanner: report errors for incorrect line directives

@griesemer
Copy link
Contributor Author

@ianlancetaylor The go generate documentation doesn't really talk about the generators; so I'm inclined to not mention line directives there.

@gopherbot
Copy link

Change https://golang.org/cl/100462 mentions this issue: cmd/compile: document new line directives

gopherbot pushed a commit that referenced this issue Mar 15, 2018
Based on decision for #24183. This makes the go/scanner behavior
match cmd/compile behavior. Adjusted a go/printer test that assumed
silent behavior for invalid line directive, and added more scanner
tests verifying the correct error position and message for invalid
line directives.

The filenames in line directives now remain untouched by the scanner;
there is no cleanup or conversion of relative into absolute paths
anymore, in sync with what the compiler's scanner/parser are doing.
Any kind of filename transformation has to be done by a client. This
makes the scanner code simpler and also more predictable.

For #24183.

Change-Id: Ia091548e1d3d89dfdf6e7d82dab50bea05742ce3
Reviewed-on: https://go-review.googlesource.com/100235
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
gopherbot pushed a commit that referenced this issue Jul 31, 2018
Due to a new specification of //line: directives, missing
column info is now treated as column 0, aka "unknown column"
(see #24183 for details).

As cgo does not add column number to generated //line: directive,
resulting files parsed do not have column info.

Fix by adding column of 1 to generated line directives.

Fixes #26692

Change-Id: Ie9263c0cf666b92d19c34240e745e8f32ffe7174
Reviewed-on: https://go-review.googlesource.com/126675
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Mar 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants