From 78382e72d0254aae07017cc7441dd02440a40d8b Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Mon, 3 Oct 2022 19:05:55 +0200 Subject: [PATCH 1/2] Add support for package export By default an identifier that has been imported into a package is not available for imports by other packages. Only imports that have been exported can be imported again. E.g. ``` package P1; int x; endpackage package P2; import P1::x; export P1::x; endpackage module test; import P2::x; // This will only work if x has been exported. endmodule ``` Exports follow the same syntax as imports and allow both export of specific identifiers or wildcard export. Export supports the special `*::*` target, which will export all imported items. Add support for handling package exports. There is one special cases that needs to be considered. Usually when using wildcard imports from multiple packages it is an error if there multiple import candidates for an identifier. With exports it is possible that there are multiple candidates through different packets, but they all refer to the same identifier. In this case it does not create a conflict. E.g. ``` package P1; int x; endpackage package P2; import P1::x; export P1::x; endpackage package P3; import P1::*; import P2::*; int y = x; // No import conflict endpackage ``` Signed-off-by: Lars-Peter Clausen --- PPackage.h | 8 ++++ PScope.h | 10 ++++- parse.y | 25 +++++++++++ pform.cc | 39 +---------------- pform.h | 6 +++ pform_package.cc | 108 ++++++++++++++++++++++++++++++++++++++++++++--- 6 files changed, 150 insertions(+), 46 deletions(-) diff --git a/PPackage.h b/PPackage.h index c352296d6..075e214b4 100644 --- a/PPackage.h +++ b/PPackage.h @@ -24,6 +24,7 @@ # include "LineInfo.h" # include "StringHeap.h" # include +# include /* * SystemVerilog supports class declarations with their own lexical @@ -42,6 +43,13 @@ class PPackage : public PScopeExtra, public LineInfo { bool elaborate(Design*des, NetScope*scope) const; void pform_dump(std::ostream&out) const; + + struct export_t { + PPackage *pkg; + perm_string name; + }; + + std::vector exports; }; #endif /* IVL_PPackage_H */ diff --git a/PScope.h b/PScope.h index faee69fac..9fd41a461 100644 --- a/PScope.h +++ b/PScope.h @@ -25,6 +25,7 @@ # include "ivl_target.h" # include # include +# include # include class PEvent; @@ -67,9 +68,14 @@ class LexicalScope { // Symbols that are defined or declared in this scope. std::maplocal_symbols; - // Symbols that are explicitly imported. Bind the imported name - // to the package from which the name is imported. + // Symbols that are explicitly imported. This contains the package where + // the symbol has been decelared. When using exports, this might not be + // the same as the package where it has been imported from. std::mapexplicit_imports; + // Symbols that are explicitly imported. This contains the set of + // packages from which the symbol has been imported. When using exports + // the same identifier can be imported via multiple packages. + std::map> explicit_imports_from; // Packages that are wildcard imported. When identifiers from // these packages are referenced, they will be added to the diff --git a/parse.y b/parse.y index 694042529..5af988b4d 100644 --- a/parse.y +++ b/parse.y @@ -2053,6 +2053,30 @@ package_import_item_list | package_import_item ; +package_export_declaration /* IEEE1800-2017 A.2.1.3 */ + : K_export package_export_item_list ';' + | K_export '*' K_SCOPE_RES '*' ';' { pform_package_export(@$, nullptr, nullptr); } + ; + +package_export_item + : PACKAGE_IDENTIFIER K_SCOPE_RES IDENTIFIER + { pform_package_export(@2, $1, $3); + delete[] $3; + } + | PACKAGE_IDENTIFIER K_SCOPE_RES TYPE_IDENTIFIER + { pform_package_export(@2, $1, $3.text); + delete[] $3.text; + } + | PACKAGE_IDENTIFIER K_SCOPE_RES '*' + { pform_package_export(@2, $1, nullptr); + } + ; + +package_export_item_list + : package_export_item_list ',' package_export_item + | package_export_item + ; + package_item /* IEEE1800-2005 A.1.10 */ : timeunits_declaration | parameter_declaration @@ -2061,6 +2085,7 @@ package_item /* IEEE1800-2005 A.1.10 */ | task_declaration | data_declaration | class_declaration + | package_export_declaration ; package_item_list diff --git a/pform.cc b/pform.cc index a03b1209d..e8aac1089 100644 --- a/pform.cc +++ b/pform.cc @@ -472,41 +472,6 @@ static void add_local_symbol(LexicalScope*scope, perm_string name, PNamedItem*it scope->local_symbols[name] = item; } -static PPackage*find_potential_import(const struct vlltype&loc, LexicalScope*scope, - perm_string name, bool tf_call, bool make_explicit) -{ - assert(scope); - - PPackage*found_pkg = 0; - for (list::const_iterator cur_pkg = scope->potential_imports.begin(); - cur_pkg != scope->potential_imports.end(); ++cur_pkg) { - PPackage*search_pkg = *cur_pkg; - map::const_iterator cur_sym - = search_pkg->local_symbols.find(name); - if (cur_sym != search_pkg->local_symbols.end()) { - if (found_pkg && make_explicit) { - cerr << loc.get_fileline() << ": error: " - "Ambiguous use of '" << name << "'. " - "It is exported by both '" - << found_pkg->pscope_name() - << "' and by '" - << search_pkg->pscope_name() - << "'." << endl; - error_count += 1; - } else { - found_pkg = search_pkg; - if (make_explicit) { - if (tf_call) - scope->possible_imports[name] = found_pkg; - else - scope->explicit_imports[name] = found_pkg; - } - } - } - } - return found_pkg; -} - static void check_potential_imports(const struct vlltype&loc, perm_string name, bool tf_call) { LexicalScope*scope = lexical_scope; @@ -515,7 +480,7 @@ static void check_potential_imports(const struct vlltype&loc, perm_string name, return; if (scope->explicit_imports.find(name) != scope->explicit_imports.end()) return; - if (find_potential_import(loc, scope, name, tf_call, true)) + if (pform_find_potential_import(loc, scope, name, tf_call, true)) return; scope = scope->parent_scope(); @@ -938,7 +903,7 @@ typedef_t* pform_test_type_identifier(const struct vlltype&loc, const char*txt) if (cur != cur_scope->typedefs.end()) return cur->second; - PPackage*pkg = find_potential_import(loc, cur_scope, name, false, false); + PPackage*pkg = pform_find_potential_import(loc, cur_scope, name, false, false); if (pkg) { cur = pkg->typedefs.find(name); if (cur != pkg->typedefs.end()) diff --git a/pform.h b/pform.h index ff4027bb7..f88513f03 100644 --- a/pform.h +++ b/pform.h @@ -203,6 +203,12 @@ extern void pform_start_package_declaration(const struct vlltype&loc, extern void pform_end_package_declaration(const struct vlltype&loc); extern void pform_package_import(const struct vlltype&loc, PPackage*pkg, const char*ident); +extern void pform_package_export(const struct vlltype &loc, PPackage *pkg, + const char *ident); +PPackage *pform_package_importable(PPackage *pkg, perm_string name); +PPackage *pform_find_potential_import(const struct vlltype&loc, LexicalScope*scope, + perm_string name, bool tf_call, bool make_explicit); + extern PExpr* pform_package_ident(const struct vlltype&loc, PPackage*pkg, pform_name_t*ident); diff --git a/pform_package.cc b/pform_package.cc index 3006ab58e..63935aecb 100644 --- a/pform_package.cc +++ b/pform_package.cc @@ -72,6 +72,64 @@ void pform_end_package_declaration(const struct vlltype&loc) pform_pop_scope(); } +PPackage *pform_find_potential_import(const struct vlltype&loc, LexicalScope*scope, + perm_string name, bool tf_call, bool make_explicit) +{ + assert(scope); + + PPackage *found_pkg = nullptr; + for (auto search_pkg : scope->potential_imports) { + PPackage *decl_pkg = pform_package_importable(search_pkg, name); + if (!decl_pkg) + continue; + + if (found_pkg && found_pkg != decl_pkg && make_explicit) { + cerr << loc.get_fileline() << ": error: " + "Ambiguous use of '" << name << "'. " + "It is exported by both '" + << found_pkg->pscope_name() + << "' and by '" + << search_pkg->pscope_name() + << "'." << endl; + error_count++; + continue; + } + + found_pkg = decl_pkg; + if (make_explicit) { + if (tf_call) + scope->possible_imports[name] = found_pkg; + else { + scope->explicit_imports[name] = found_pkg; + scope->explicit_imports_from[name].insert(search_pkg); + } + } + } + + return found_pkg; +} + +PPackage *pform_package_importable(PPackage *pkg, perm_string name) +{ + if (pkg->local_symbols.find(name) != pkg->local_symbols.end()) + return pkg; + + auto import_pkg = pkg->explicit_imports.find(name); + if (import_pkg == pkg->explicit_imports.end()) + return nullptr; + + for (auto &exp : pkg->exports) { + // *::* will match all imports, P::* will match all imports + // from a package and P::ID will match a specific identifier + // from a package. + if ((!exp.pkg || exp.pkg == import_pkg->second) && + (exp.name.nil() || exp.name == name)) + return import_pkg->second; + } + + return nullptr; +} + /* * Do the import early, during processing. This requires that the * package is declared in pform ahead of time (it is) and that we can @@ -85,9 +143,8 @@ void pform_package_import(const struct vlltype&loc, PPackage*pkg, const char*ide perm_string use_ident = lex_strings.make(ident); // Check that the requested symbol is available. - map::const_iterator cur_sym - = pkg->local_symbols.find(use_ident); - if (cur_sym == pkg->local_symbols.end()) { + PPackage *pkg_decl = pform_package_importable(pkg, use_ident); + if (!pkg_decl) { cerr << loc.get_fileline() << ": error: " "'" << use_ident << "' is not exported by '" << pkg->pscope_name() << "'." << endl; @@ -96,7 +153,7 @@ void pform_package_import(const struct vlltype&loc, PPackage*pkg, const char*ide } // Check for conflict with local symbol. - cur_sym = scope->local_symbols.find(use_ident); + auto cur_sym = scope->local_symbols.find(use_ident); if (cur_sym != scope->local_symbols.end()) { cerr << loc.get_fileline() << ": error: " "'" << use_ident << "' has already been declared " @@ -112,17 +169,17 @@ void pform_package_import(const struct vlltype&loc, PPackage*pkg, const char*ide map::const_iterator cur_pkg = scope->explicit_imports.find(use_ident); if (cur_pkg != scope->explicit_imports.end()) { - if (cur_pkg->second != pkg) { + if (cur_pkg->second != pkg_decl) { cerr << loc.get_fileline() << ": error: " "'" << use_ident << "' has already been " "imported into this scope from package '" << cur_pkg->second->pscope_name() << "'." << endl; error_count += 1; } - return; } - scope->explicit_imports[use_ident] = pkg; + scope->explicit_imports[use_ident] = pkg_decl; + scope->explicit_imports_from[use_ident].insert(pkg); } else { list::const_iterator cur_pkg @@ -134,6 +191,43 @@ void pform_package_import(const struct vlltype&loc, PPackage*pkg, const char*ide } } +static bool pform_package_exportable(const struct vlltype &loc, PPackage *pkg, + const perm_string &ident) +{ + auto import_pkg = pform_cur_package->explicit_imports_from.find(ident); + if (import_pkg != pform_cur_package->explicit_imports_from.end()) { + auto &pkg_list = import_pkg->second; + if (pkg_list.find(pkg) != pkg_list.end()) + return true; + } + + if (pform_cur_package->local_symbols.find(ident) == pform_cur_package->local_symbols.end()) { + if (pform_find_potential_import(loc, pform_cur_package, + ident, false, true)) + return true; + } + + cerr << loc.get_fileline() << ": error: " + "`" << ident << "` has not been imported from " + << pkg->pscope_name() << "." << endl; + error_count++; + + return false; +} + +void pform_package_export(const struct vlltype &loc, PPackage *pkg, const char *ident) +{ + assert(pform_cur_package); + + perm_string use_ident; + if (ident) { + use_ident = lex_strings.make(ident); + if (!pform_package_exportable(loc, pkg, use_ident)) + return; + } + pform_cur_package->exports.push_back(PPackage::export_t{pkg, use_ident}); +} + PExpr* pform_package_ident(const struct vlltype&loc, PPackage*pkg, pform_name_t*ident_name) { From d868983b9c6e0ffb1ac7ea8cf737fc3e5ed0a528 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Mon, 3 Oct 2022 23:27:55 +0200 Subject: [PATCH 2/2] Add regression tests for package export Check that package exports are supported. Also check for various scenarios where package exports should fail. Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/sv_export1.v | 25 +++++++++++++++++++++++++ ivtest/ivltests/sv_export2.v | 25 +++++++++++++++++++++++++ ivtest/ivltests/sv_export3.v | 25 +++++++++++++++++++++++++ ivtest/ivltests/sv_export4.v | 25 +++++++++++++++++++++++++ ivtest/ivltests/sv_export5.v | 26 ++++++++++++++++++++++++++ ivtest/ivltests/sv_export6.v | 28 ++++++++++++++++++++++++++++ ivtest/ivltests/sv_export_fail1.v | 22 ++++++++++++++++++++++ ivtest/ivltests/sv_export_fail2.v | 17 +++++++++++++++++ ivtest/ivltests/sv_export_fail3.v | 23 +++++++++++++++++++++++ ivtest/ivltests/sv_export_fail4.v | 23 +++++++++++++++++++++++ ivtest/ivltests/sv_export_fail5.v | 25 +++++++++++++++++++++++++ ivtest/ivltests/sv_export_fail6.v | 22 ++++++++++++++++++++++ ivtest/regress-sv.list | 12 ++++++++++++ 13 files changed, 298 insertions(+) create mode 100644 ivtest/ivltests/sv_export1.v create mode 100644 ivtest/ivltests/sv_export2.v create mode 100644 ivtest/ivltests/sv_export3.v create mode 100644 ivtest/ivltests/sv_export4.v create mode 100644 ivtest/ivltests/sv_export5.v create mode 100644 ivtest/ivltests/sv_export6.v create mode 100644 ivtest/ivltests/sv_export_fail1.v create mode 100644 ivtest/ivltests/sv_export_fail2.v create mode 100644 ivtest/ivltests/sv_export_fail3.v create mode 100644 ivtest/ivltests/sv_export_fail4.v create mode 100644 ivtest/ivltests/sv_export_fail5.v create mode 100644 ivtest/ivltests/sv_export_fail6.v diff --git a/ivtest/ivltests/sv_export1.v b/ivtest/ivltests/sv_export1.v new file mode 100644 index 000000000..5413c355e --- /dev/null +++ b/ivtest/ivltests/sv_export1.v @@ -0,0 +1,25 @@ +// Check that it is possible to explicitly export an identifier and import it +// from another scope. + +package P1; + integer x = 123; +endpackage + +package P2; + import P1::x; + export P1::x; +endpackage + +module test; + + import P2::x; + + initial begin + if (x == 123) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end + +endmodule diff --git a/ivtest/ivltests/sv_export2.v b/ivtest/ivltests/sv_export2.v new file mode 100644 index 000000000..3d1bebe68 --- /dev/null +++ b/ivtest/ivltests/sv_export2.v @@ -0,0 +1,25 @@ +// Check that it is possible use package wildcard export an identifier and +// import it from another scope. + +package P1; + integer x = 123; +endpackage + +package P2; + import P1::x; + export P1::*; +endpackage + +module test; + + import P2::x; + + initial begin + if (x == 123) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end + +endmodule diff --git a/ivtest/ivltests/sv_export3.v b/ivtest/ivltests/sv_export3.v new file mode 100644 index 000000000..acfb1e294 --- /dev/null +++ b/ivtest/ivltests/sv_export3.v @@ -0,0 +1,25 @@ +// Check that it is possible use full wildcard export an identifier and import +// it from another scope. + +package P1; + integer x = 123; +endpackage + +package P2; + import P1::x; + export *::*; +endpackage + +module test; + + import P2::x; + + initial begin + if (x == 123) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end + +endmodule diff --git a/ivtest/ivltests/sv_export4.v b/ivtest/ivltests/sv_export4.v new file mode 100644 index 000000000..af51f1fda --- /dev/null +++ b/ivtest/ivltests/sv_export4.v @@ -0,0 +1,25 @@ +// Check that it is possible to export an implicitly imported identifier and +// import it again from another scope. + +package P1; + integer x = 123; +endpackage + +package P2; + import P1::*; + export P1::x; +endpackage + +module test; + + import P2::x; + + initial begin + if (x == 123) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end + +endmodule diff --git a/ivtest/ivltests/sv_export5.v b/ivtest/ivltests/sv_export5.v new file mode 100644 index 000000000..6591c88db --- /dev/null +++ b/ivtest/ivltests/sv_export5.v @@ -0,0 +1,26 @@ +// Check that implicitly imported identifiers can be exported through a package +// wildcard export. + +package P1; + integer x = 123; +endpackage + +package P2; + import P1::*; + export P1::*; + integer y = x; // Creates an import for P1::x +endpackage + +module test; + + import P2::x; + + initial begin + if (x == 123) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end + +endmodule diff --git a/ivtest/ivltests/sv_export6.v b/ivtest/ivltests/sv_export6.v new file mode 100644 index 000000000..e5a676f17 --- /dev/null +++ b/ivtest/ivltests/sv_export6.v @@ -0,0 +1,28 @@ +// Check that it is possible to implicitly import the same identifier through +// multiple paths without causing a conflict. + +package P1; + integer x = 123; +endpackage + +package P2; + import P1::x; + export P1::x; +endpackage + +module test; + + // P1::x is visible through either of the imports below. This should not + // create a conflict since it is the same identifier. + import P1::*; + import P2::*; + + initial begin + if (x == 123) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end + +endmodule diff --git a/ivtest/ivltests/sv_export_fail1.v b/ivtest/ivltests/sv_export_fail1.v new file mode 100644 index 000000000..0ccc9b891 --- /dev/null +++ b/ivtest/ivltests/sv_export_fail1.v @@ -0,0 +1,22 @@ +// Check that it is an error to export an identifier from a package from which +// it has not been imported from. + +package P1; + integer x; +endpackage + +package P2; + import P1::x; + export P1::x; +endpackage + +package P3; + import P1::x; + export P2::x; // This should fail, even though P2::x is the same as P1::x +endpackage + +module test; + initial begin + $display("FAILED"); + end +endmodule diff --git a/ivtest/ivltests/sv_export_fail2.v b/ivtest/ivltests/sv_export_fail2.v new file mode 100644 index 000000000..653ea70c9 --- /dev/null +++ b/ivtest/ivltests/sv_export_fail2.v @@ -0,0 +1,17 @@ +// Check that it is an error to export an identifier that has not been imported. + +package P1; + integer x; + integer y; +endpackage + +package P2; + import P1::x; + export P1::y; // Should fail, P1::y has not been imported. +endpackage + +module test; + initial begin + $display("FAILED"); + end +endmodule diff --git a/ivtest/ivltests/sv_export_fail3.v b/ivtest/ivltests/sv_export_fail3.v new file mode 100644 index 000000000..ae10455e6 --- /dev/null +++ b/ivtest/ivltests/sv_export_fail3.v @@ -0,0 +1,23 @@ +// Check that an identifier importable through a wildcard import is not exported +// through a wildcard export if the identifier has not been referenced in +// package. + +package P1; + integer x = 123; +endpackage + +package P2; + import P1::*; + export *::*; +endpackage + +module test; + + import P2::x; // This should fail, P1::x is not referenced in P2 and hence not + // exportable through P2 + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_export_fail4.v b/ivtest/ivltests/sv_export_fail4.v new file mode 100644 index 000000000..2b2996778 --- /dev/null +++ b/ivtest/ivltests/sv_export_fail4.v @@ -0,0 +1,23 @@ +// Check that it is an error to export an identifier that is importable through +// a wildcard import if it creates a conflict with a local identifier. + +package P1; + integer x = 123; +endpackage + +package P2; + import P1::*; + integer x = 456; + export P1::x; // This should fail, P1::x can not be imported into this scope + // since the name already exists. +endpackage + +module test; + + import P2::x; + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_export_fail5.v b/ivtest/ivltests/sv_export_fail5.v new file mode 100644 index 000000000..d1398b81b --- /dev/null +++ b/ivtest/ivltests/sv_export_fail5.v @@ -0,0 +1,25 @@ +// Check that it is an error to export an identifier that is importable through +// a wildcard import if it creates a conflict with a local identifier, even if +// the local identifier is declared after the export. + +package P1; + integer x = 123; +endpackage + +package P2; + import P1::*; + export P1::x; // This should fail, P1::x can not be imported into this scope + // since the there is a local symbol with the same name. Even if + // it is declared after the export. + integer x = 456; +endpackage + +module test; + + import P2::x; + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_export_fail6.v b/ivtest/ivltests/sv_export_fail6.v new file mode 100644 index 000000000..c403f51d4 --- /dev/null +++ b/ivtest/ivltests/sv_export_fail6.v @@ -0,0 +1,22 @@ +// Check that an error is reported if trying to export an identifier that is +// declared outside of a package + +integer x = 123; + +package P1; +endpackage + +package P2; + import P1::*; + export P1::x; // This should fail. x is visible in P1, but not declared in P1 +endpackage + +module test; + + import P2::x; + + initial begin + $display("FAILED"); + end + +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index c59386de7..a888672ba 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -668,6 +668,18 @@ sv_end_labels normal,-g2009 ivltests sv_end_labels_bad CE,-g2009 ivltests gold=sv_end_labels_bad.gold sv_end_labels_unnamed CE,-g2009 ivltests gold=sv_end_labels_unnamed.gold sv_enum1 normal,-g2009 ivltests +sv_export1 normal,-g2009 ivltests +sv_export2 normal,-g2009 ivltests +sv_export3 normal,-g2009 ivltests +sv_export4 normal,-g2009 ivltests +sv_export5 normal,-g2009 ivltests +sv_export6 normal,-g2009 ivltests +sv_export_fail1 CE,-g2009 ivltests +sv_export_fail2 CE,-g2009 ivltests +sv_export_fail3 CE,-g2009 ivltests +sv_export_fail4 CE,-g2009 ivltests +sv_export_fail5 CE,-g2009 ivltests +sv_export_fail6 CE,-g2009 ivltests sv_for_variable normal,-g2009 ivltests sv_foreach1 normal,-g2009 ivltests sv_foreach2 normal,-g2009 ivltests