Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(361)

Issue 5712043: code review 5712043: gc: disallow absolute import paths (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 1 month ago by rsc
Modified:
13 years, 1 month ago
Reviewers:
ken3
CC:
ken2, golang-dev
Visibility:
Public.

Description

gc: disallow absolute import paths They are broken and hard to make work. They have never worked: if you import "/tmp/x" from "/home/rsc/p.c" then the compiler rewrites this into import "/home/rsc/tmp/x", which is clearly wrong. Also we just disallowed the : character in import paths, so import "c:/foo" is already not allowed. Finally, in order to support absolute paths well in a build tool we'd have to provide a mechanism to instruct the compiler to resolve absolute imports by looking in some other tree (where the binaries live) and provide a mapping from absolute path to location in that tree. This CL avoids adding that complexity. This is not part of the language spec (and should not be), so no spec change is needed. If we need to make them work later, we can.

Patch Set 1 #

Patch Set 2 : diff -r 9a4d3bff8bb0 https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r 9a4d3bff8bb0 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -0 lines) Patch
M src/cmd/gc/lex.c View 1 1 chunk +5 lines, -0 lines 0 comments Download
M test/import5.go View 1 2 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 3
rsc
Hello ken2 (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
13 years, 1 month ago (2012-02-29 20:28:35 UTC) #1
rsc
*** Submitted as http://code.google.com/p/go/source/detail?r=a6e732d1523e *** gc: disallow absolute import paths They are broken and hard ...
13 years, 1 month ago (2012-02-29 20:28:38 UTC) #2
ken3
13 years, 1 month ago (2012-02-29 23:59:43 UTC) #3
lgtm
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b