* Go patch committed: Fix -fgo-prefix handling
@ 2015-01-30 3:16 Ian Lance Taylor
2015-01-30 17:48 ` Ian Lance Taylor
0 siblings, 1 reply; 2+ messages in thread
From: Ian Lance Taylor @ 2015-01-30 3:16 UTC (permalink / raw)
To: gcc-patches, gofrontend-dev
[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]
There was bug in the fix for PR 61880: it only worked fully correctly
for code compiled with -fgo-pkgpath. For code that used -fgo-prefix,
or that used neither option, the '.' separating the prefix and the
package name was converted to an underscore, which did not happen
before. This broke SWIG and any other code that expected specific
symbol names. Fortunately all code compiled in libgo and all code
compiled by the go tool uses -fgo-pkgpath, so this probably did not
affect very many people.
A complete fix for this requires modifying the package file format to
record the right symbol to use for a package that is mentioned but not
imported. Fortunately that too is a rare case. This patch fixes the
problem for all other cases while keeping the package file format the
same. This patch will go on the 4.9 branch as well as mainline. I
will follow up with another patch for mainline only that changes the
package file format and fully fixes the problem.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline and 4.9 branch.
Ian
[-- Attachment #2: foo.txt --]
[-- Type: text/plain, Size: 10911 bytes --]
diff -r 65be171781bf go/export.cc
--- a/go/export.cc Fri Jan 23 15:43:20 2015 -0800
+++ b/go/export.cc Thu Jan 29 15:29:32 2015 -0800
@@ -91,6 +91,7 @@
void
Export::export_globals(const std::string& package_name,
+ const std::string& prefix,
const std::string& pkgpath,
int package_priority,
const std::map<std::string, Package*>& imports,
@@ -140,9 +141,18 @@
this->write_string(package_name);
this->write_c_string(";\n");
- // The package path, used for all global symbols.
- this->write_c_string("pkgpath ");
- this->write_string(pkgpath);
+ // The prefix or package path, used for all global symbols.
+ if (prefix.empty())
+ {
+ go_assert(!pkgpath.empty());
+ this->write_c_string("pkgpath ");
+ this->write_string(pkgpath);
+ }
+ else
+ {
+ this->write_c_string("prefix ");
+ this->write_string(prefix);
+ }
this->write_c_string(";\n");
// The package priority.
diff -r 65be171781bf go/export.h
--- a/go/export.h Fri Jan 23 15:43:20 2015 -0800
+++ b/go/export.h Thu Jan 29 15:29:32 2015 -0800
@@ -117,14 +117,17 @@
// Export the identifiers in BINDINGS which are marked for export.
// The exporting is done via a series of calls to THIS->STREAM_. If
// is nothing to export, this->stream_->write will not be called.
- // PKGPATH is the package path.
+ // PREFIX is the package prefix. PKGPATH is the package path.
+ // Only one of PREFIX and PKGPATH will be non-empty.
// PACKAGE_PRIORITY is the priority to use for this package.
+ // IMPORTS is the explicitly imported packages.
// IMPORT_INIT_FN is the name of the import initialization function
// for this package; it will be empty if none is needed.
// IMPORTED_INIT_FNS is the list of initialization functions for
// imported packages.
void
export_globals(const std::string& package_name,
+ const std::string& prefix,
const std::string& pkgpath,
int package_priority,
const std::map<std::string, Package*>& imports,
diff -r 65be171781bf go/gogo.cc
--- a/go/gogo.cc Fri Jan 23 15:43:20 2015 -0800
+++ b/go/gogo.cc Thu Jan 29 15:29:32 2015 -0800
@@ -341,22 +341,28 @@
// Now that we know the name of the package we are compiling, set
// the package path to use for reflect.Type.PkgPath and global
// symbol names.
- if (!this->pkgpath_set_)
+ if (this->pkgpath_set_)
+ this->pkgpath_symbol_ = Gogo::pkgpath_for_symbol(this->pkgpath_);
+ else
{
if (!this->prefix_from_option_ && package_name == "main")
- this->pkgpath_ = package_name;
+ {
+ this->pkgpath_ = package_name;
+ this->pkgpath_symbol_ = Gogo::pkgpath_for_symbol(package_name);
+ }
else
{
if (!this->prefix_from_option_)
this->prefix_ = "go";
this->pkgpath_ = this->prefix_ + '.' + package_name;
+ this->pkgpath_symbol_ = (Gogo::pkgpath_for_symbol(this->prefix_) + '.'
+ + Gogo::pkgpath_for_symbol(package_name));
}
this->pkgpath_set_ = true;
}
- this->pkgpath_symbol_ = Gogo::pkgpath_for_symbol(this->pkgpath_);
-
- this->package_ = this->register_package(this->pkgpath_, location);
+ this->package_ = this->register_package(this->pkgpath_,
+ this->pkgpath_symbol_, location);
this->package_->set_package_name(package_name, location);
if (this->is_main_package())
@@ -1524,10 +1530,11 @@
const std::string& alias_arg,
bool is_alias_exported,
const std::string& pkgpath,
+ const std::string& pkgpath_symbol,
Location location,
bool* padd_to_globals)
{
- Package* ret = this->register_package(pkgpath, location);
+ Package* ret = this->register_package(pkgpath, pkgpath_symbol, location);
ret->set_package_name(real_name, location);
*padd_to_globals = false;
@@ -1556,10 +1563,13 @@
// Register a package. This package may or may not be imported. This
// returns the Package structure for the package, creating if it
// necessary. LOCATION is the location of the import statement that
-// led us to see this package.
+// led us to see this package. PKGPATH_SYMBOL is the symbol to use
+// for names in the package; it may be the empty string, in which case
+// we either get it later or make a guess when we need it.
Package*
-Gogo::register_package(const std::string& pkgpath, Location location)
+Gogo::register_package(const std::string& pkgpath,
+ const std::string& pkgpath_symbol, Location location)
{
Package* package = NULL;
std::pair<Packages::iterator, bool> ins =
@@ -1569,13 +1579,15 @@
// We have seen this package name before.
package = ins.first->second;
go_assert(package != NULL && package->pkgpath() == pkgpath);
+ if (!pkgpath_symbol.empty())
+ package->set_pkgpath_symbol(pkgpath_symbol);
if (Linemap::is_unknown_location(package->location()))
package->set_location(location);
}
else
{
// First time we have seen this package name.
- package = new Package(pkgpath, location);
+ package = new Package(pkgpath, pkgpath_symbol, location);
go_assert(ins.first->second == NULL);
ins.first->second = package;
}
@@ -4333,10 +4345,24 @@
// support streaming to a separate file.
Stream_to_section stream;
+ // Write out either the prefix or pkgpath depending on how we were
+ // invoked.
+ std::string prefix;
+ std::string pkgpath;
+ if (this->pkgpath_from_option_)
+ pkgpath = this->pkgpath_;
+ else if (this->prefix_from_option_)
+ prefix = this->prefix_;
+ else if (this->is_main_package())
+ pkgpath = "main";
+ else
+ prefix = "go";
+
Export exp(&stream);
exp.register_builtin_types(this);
exp.export_globals(this->package_name(),
- this->pkgpath(),
+ prefix,
+ pkgpath,
this->package_priority(),
this->imports_,
(this->need_init_fn_ && !this->is_main_package()
@@ -7478,8 +7504,9 @@
// Class Package.
-Package::Package(const std::string& pkgpath, Location location)
- : pkgpath_(pkgpath), pkgpath_symbol_(Gogo::pkgpath_for_symbol(pkgpath)),
+Package::Package(const std::string& pkgpath,
+ const std::string& pkgpath_symbol, Location location)
+ : pkgpath_(pkgpath), pkgpath_symbol_(pkgpath_symbol),
package_name_(), bindings_(new Bindings(NULL)), priority_(0),
location_(location), used_(false), is_imported_(false),
uses_sink_alias_(false)
@@ -7503,6 +7530,34 @@
package_name.c_str());
}
+// Return the pkgpath symbol, which is a prefix for symbols defined in
+// this package.
+
+std::string
+Package::pkgpath_symbol() const
+{
+ if (this->pkgpath_symbol_.empty())
+ {
+ // In the general case, this is wrong, because the package might
+ // have been compiled with -fprefix. However, it is what we
+ // used to do, so it is no more wrong than we were before.
+ return Gogo::pkgpath_for_symbol(this->pkgpath_);
+ }
+ return this->pkgpath_symbol_;
+}
+
+// Set the package path symbol.
+
+void
+Package::set_pkgpath_symbol(const std::string& pkgpath_symbol)
+{
+ go_assert(!pkgpath_symbol.empty());
+ if (this->pkgpath_symbol_.empty())
+ this->pkgpath_symbol_ = pkgpath_symbol;
+ else
+ go_assert(this->pkgpath_symbol_ == pkgpath_symbol);
+}
+
// Set the priority. We may see multiple priorities for an imported
// package; we want to use the largest one.
diff -r 65be171781bf go/gogo.h
--- a/go/gogo.h Fri Jan 23 15:43:20 2015 -0800
+++ b/go/gogo.h Thu Jan 29 15:29:32 2015 -0800
@@ -276,6 +276,7 @@
add_imported_package(const std::string& real_name, const std::string& alias,
bool is_alias_exported,
const std::string& pkgpath,
+ const std::string& pkgpath_symbol,
Location location,
bool* padd_to_globals);
@@ -283,7 +284,8 @@
// This returns the Package structure for the package, creating if
// it necessary.
Package*
- register_package(const std::string& pkgpath, Location);
+ register_package(const std::string& pkgpath,
+ const std::string& pkgpath_symbol, Location);
// Start compiling a function. ADD_METHOD_TO_TYPE is true if a
// method function should be added to the type of its receiver.
@@ -2622,7 +2624,8 @@
class Package
{
public:
- Package(const std::string& pkgpath, Location location);
+ Package(const std::string& pkgpath, const std::string& pkgpath_symbol,
+ Location location);
// Get the package path used for all symbols exported from this
// package.
@@ -2631,9 +2634,12 @@
{ return this->pkgpath_; }
// Return the package path to use for a symbol name.
- const std::string&
- pkgpath_symbol() const
- { return this->pkgpath_symbol_; }
+ std::string
+ pkgpath_symbol() const;
+
+ // Set the package path symbol.
+ void
+ set_pkgpath_symbol(const std::string&);
// Return the location of the import statement.
Location
diff -r 65be171781bf go/import.cc
--- a/go/import.cc Fri Jan 23 15:43:20 2015 -0800
+++ b/go/import.cc Thu Jan 29 15:29:32 2015 -0800
@@ -301,23 +301,27 @@
this->require_c_string(";\n");
std::string pkgpath;
+ std::string pkgpath_symbol;
if (this->match_c_string("prefix "))
{
this->advance(7);
std::string unique_prefix = this->read_identifier();
this->require_c_string(";\n");
pkgpath = unique_prefix + '.' + package_name;
+ pkgpath_symbol = (Gogo::pkgpath_for_symbol(unique_prefix) + '.'
+ + Gogo::pkgpath_for_symbol(package_name));
}
else
{
this->require_c_string("pkgpath ");
pkgpath = this->read_identifier();
this->require_c_string(";\n");
+ pkgpath_symbol = Gogo::pkgpath_for_symbol(pkgpath);
}
this->package_ = gogo->add_imported_package(package_name, local_name,
is_local_name_exported,
- pkgpath,
+ pkgpath, pkgpath_symbol,
this->location_,
&this->add_to_globals_);
if (this->package_ == NULL)
@@ -392,7 +396,7 @@
stream->advance(1);
this->require_c_string("\";\n");
- Package* p = this->gogo_->register_package(pkgpath,
+ Package* p = this->gogo_->register_package(pkgpath, "",
Linemap::unknown_location());
p->set_package_name(package_name, this->location());
}
@@ -649,7 +653,7 @@
package = this->package_;
else
{
- package = this->gogo_->register_package(pkgpath,
+ package = this->gogo_->register_package(pkgpath, "",
Linemap::unknown_location());
if (!package_name.empty())
package->set_package_name(package_name, this->location());
diff -r 65be171781bf go/unsafe.cc
--- a/go/unsafe.cc Fri Jan 23 15:43:20 2015 -0800
+++ b/go/unsafe.cc Thu Jan 29 15:29:32 2015 -0800
@@ -22,7 +22,7 @@
bool add_to_globals;
Package* package = this->add_imported_package("unsafe", local_name,
is_local_name_exported,
- "unsafe", location,
+ "unsafe", "unsafe", location,
&add_to_globals);
if (package == NULL)
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Go patch committed: Fix -fgo-prefix handling
2015-01-30 3:16 Go patch committed: Fix -fgo-prefix handling Ian Lance Taylor
@ 2015-01-30 17:48 ` Ian Lance Taylor
0 siblings, 0 replies; 2+ messages in thread
From: Ian Lance Taylor @ 2015-01-30 17:48 UTC (permalink / raw)
To: gcc-patches, gofrontend-dev
[-- Attachment #1: Type: text/plain, Size: 340 bytes --]
This is the followup patch. This adds more information to the package
when any imported packages have a pkgpath symbol that is not the
obvious transformation of the pkgpath. This is enough to determine
the right symbol name to use in all cases. Bootstrapped and ran Go
testsuite on x86_64-unknown-linux-gnu. Committed to mainline.
Ian
[-- Attachment #2: foo.txt --]
[-- Type: text/plain, Size: 5568 bytes --]
diff -r fda36669bf1d go/export.cc
--- a/go/export.cc Thu Jan 29 16:35:18 2015 -0800
+++ b/go/export.cc Fri Jan 30 07:53:25 2015 -0800
@@ -94,6 +94,7 @@
const std::string& prefix,
const std::string& pkgpath,
int package_priority,
+ const std::map<std::string, Package*>& packages,
const std::map<std::string, Package*>& imports,
const std::string& import_init_fn,
const std::set<Import_init>& imported_init_fns,
@@ -160,6 +161,8 @@
snprintf(buf, sizeof buf, "priority %d;\n", package_priority);
this->write_c_string(buf);
+ this->write_packages(packages);
+
this->write_imports(imports);
this->write_imported_init_fns(package_name, package_priority, import_init_fn,
@@ -190,6 +193,48 @@
this->stream_->write_checksum(s);
}
+// Sort packages.
+
+static bool
+packages_compare(const Package* a, const Package* b)
+{
+ return a->package_name() < b->package_name();
+}
+
+// Write out all the known packages whose pkgpath symbol is not a
+// simple transformation of the pkgpath, so that the importing code
+// can reliably know it.
+
+void
+Export::write_packages(const std::map<std::string, Package*>& packages)
+{
+ // Sort for consistent output.
+ std::vector<Package*> out;
+ for (std::map<std::string, Package*>::const_iterator p = packages.begin();
+ p != packages.end();
+ ++p)
+ {
+ if (p->second->pkgpath_symbol()
+ != Gogo::pkgpath_for_symbol(p->second->pkgpath()))
+ out.push_back(p->second);
+ }
+
+ std::sort(out.begin(), out.end(), packages_compare);
+
+ for (std::vector<Package*>::const_iterator p = out.begin();
+ p != out.end();
+ ++p)
+ {
+ this->write_c_string("package ");
+ this->write_string((*p)->package_name());
+ this->write_c_string(" ");
+ this->write_string((*p)->pkgpath());
+ this->write_c_string(" ");
+ this->write_string((*p)->pkgpath_symbol());
+ this->write_c_string(";\n");
+ }
+}
+
// Sort imported packages.
static bool
diff -r fda36669bf1d go/export.h
--- a/go/export.h Thu Jan 29 16:35:18 2015 -0800
+++ b/go/export.h Fri Jan 30 07:53:25 2015 -0800
@@ -120,6 +120,7 @@
// PREFIX is the package prefix. PKGPATH is the package path.
// Only one of PREFIX and PKGPATH will be non-empty.
// PACKAGE_PRIORITY is the priority to use for this package.
+ // PACKAGES is all the packages we have seen.
// IMPORTS is the explicitly imported packages.
// IMPORT_INIT_FN is the name of the import initialization function
// for this package; it will be empty if none is needed.
@@ -130,6 +131,7 @@
const std::string& prefix,
const std::string& pkgpath,
int package_priority,
+ const std::map<std::string, Package*>& packages,
const std::map<std::string, Package*>& imports,
const std::string& import_init_fn,
const std::set<Import_init>& imported_init_fns,
@@ -163,6 +165,10 @@
Export(const Export&);
Export& operator=(const Export&);
+ // Write out all known packages.
+ void
+ write_packages(const std::map<std::string, Package*>& packages);
+
// Write out the imported packages.
void
write_imports(const std::map<std::string, Package*>& imports);
diff -r fda36669bf1d go/gogo.cc
--- a/go/gogo.cc Thu Jan 29 16:35:18 2015 -0800
+++ b/go/gogo.cc Fri Jan 30 07:53:25 2015 -0800
@@ -4364,6 +4364,7 @@
prefix,
pkgpath,
this->package_priority(),
+ this->packages_,
this->imports_,
(this->need_init_fn_ && !this->is_main_package()
? this->get_init_fn_name()
@@ -7537,12 +7538,7 @@
Package::pkgpath_symbol() const
{
if (this->pkgpath_symbol_.empty())
- {
- // In the general case, this is wrong, because the package might
- // have been compiled with -fprefix. However, it is what we
- // used to do, so it is no more wrong than we were before.
- return Gogo::pkgpath_for_symbol(this->pkgpath_);
- }
+ return Gogo::pkgpath_for_symbol(this->pkgpath_);
return this->pkgpath_symbol_;
}
diff -r fda36669bf1d go/import.cc
--- a/go/import.cc Thu Jan 29 16:35:18 2015 -0800
+++ b/go/import.cc Fri Jan 30 07:53:25 2015 -0800
@@ -338,6 +338,9 @@
this->package_->set_priority(prio);
this->require_c_string(";\n");
+ while (stream->match_c_string("package"))
+ this->read_one_package();
+
while (stream->match_c_string("import"))
this->read_one_import();
@@ -381,6 +384,25 @@
return this->package_;
}
+// Read a package line. This let us reliably determine the pkgpath
+// symbol, even if the package was compiled with a -fgo-prefix option.
+
+void
+Import::read_one_package()
+{
+ this->require_c_string("package ");
+ std::string package_name = this->read_identifier();
+ this->require_c_string(" ");
+ std::string pkgpath = this->read_identifier();
+ this->require_c_string(" ");
+ std::string pkgpath_symbol = this->read_identifier();
+ this->require_c_string(";\n");
+
+ Package* p = this->gogo_->register_package(pkgpath, pkgpath_symbol,
+ Linemap::unknown_location());
+ p->set_package_name(package_name, this->location());
+}
+
// Read an import line. We don't actually care about these.
void
diff -r fda36669bf1d go/import.h
--- a/go/import.h Thu Jan 29 16:35:18 2015 -0800
+++ b/go/import.h Fri Jan 30 07:53:25 2015 -0800
@@ -220,6 +220,10 @@
find_archive_export_data(const std::string& filename, int fd,
Location);
+ // Read a package line.
+ void
+ read_one_package();
+
// Read an import line.
void
read_one_import();
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-01-30 15:58 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-30 3:16 Go patch committed: Fix -fgo-prefix handling Ian Lance Taylor
2015-01-30 17:48 ` Ian Lance Taylor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).