-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Introduce a module alias option to the frontend #39376
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -244,6 +244,11 @@ bool ArgsToFrontendOptionsConverter::convert( | |
if (const Arg *A = Args.getLastArg(OPT_module_link_name)) | ||
Opts.ModuleLinkName = A->getValue(); | ||
|
||
// This must be called after computing module name, module abi name, | ||
// and module link name. | ||
if (computeModuleAliases()) | ||
return true; | ||
|
||
if (const Arg *A = Args.getLastArg(OPT_access_notes_path)) | ||
Opts.AccessNotesPath = A->getValue(); | ||
|
||
|
@@ -498,7 +503,65 @@ bool ArgsToFrontendOptionsConverter::setUpImmediateArgs() { | |
return false; | ||
} | ||
|
||
bool ArgsToFrontendOptionsConverter::computeModuleAliases() { | ||
auto list = Args.getAllArgValues(options::OPT_module_alias); | ||
if (!list.empty()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: If the list is empty, the loop will run zero times. Will that have the same effect as skipping the loop? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leaving this check as is since I'm allocating a lambda. |
||
auto validate = [this](StringRef value, bool allowModuleName) -> bool | ||
{ | ||
if (!allowModuleName) { | ||
if (value == Opts.ModuleName || | ||
value == Opts.ModuleABIName || | ||
value == Opts.ModuleLinkName) { | ||
Diags.diagnose(SourceLoc(), diag::error_module_alias_forbidden_name, value); | ||
return false; | ||
} | ||
} | ||
if (value == STDLIB_NAME) { | ||
Diags.diagnose(SourceLoc(), diag::error_module_alias_forbidden_name, value); | ||
return false; | ||
} | ||
if (!Lexer::isIdentifier(value)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does that mean that emojis are accepted? 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point :) It's the same checked used by computing module name. Most likely a c99 identifier will be used in practice, but we can add a check to enforce that later if needed. |
||
Diags.diagnose(SourceLoc(), diag::error_bad_module_name, value, false); | ||
return false; | ||
} | ||
return true; | ||
}; | ||
|
||
for (auto item: list) { | ||
auto str = StringRef(item); | ||
// splits to an alias and the underlying name | ||
auto pair = str.split('='); | ||
auto lhs = pair.first; | ||
auto rhs = pair.second; | ||
|
||
if (rhs.empty()) { // '=' is missing | ||
Diags.diagnose(SourceLoc(), diag::error_module_alias_invalid_format, str); | ||
return true; | ||
} | ||
if (!validate(lhs, false) || !validate(rhs, true)) { | ||
return true; | ||
} | ||
|
||
// First, add the underlying name as a key to prevent it from being | ||
// used as an alias | ||
if (!Opts.ModuleAliasMap.insert({rhs, StringRef()}).second) { | ||
Diags.diagnose(SourceLoc(), diag::error_module_alias_duplicate, rhs); | ||
return true; | ||
} | ||
// Next, add the alias as a key and the underlying name as a value to the map | ||
auto underlyingName = Opts.ModuleAliasMap.find(rhs)->first(); | ||
if (!Opts.ModuleAliasMap.insert({lhs, underlyingName}).second) { | ||
Diags.diagnose(SourceLoc(), diag::error_module_alias_duplicate, lhs); | ||
return true; | ||
} | ||
} | ||
} | ||
return false; | ||
} | ||
|
||
bool ArgsToFrontendOptionsConverter::computeModuleName() { | ||
assert(Opts.ModuleAliasMap.empty() && "Module name must be computed before computing module aliases"); | ||
|
||
const Arg *A = Args.getLastArg(options::OPT_module_name); | ||
if (A) { | ||
Opts.ModuleName = A->getValue(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
func quip() { | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
// Tests for invalid module alias format and values. | ||
|
||
// RUN: not %target-swift-frontend -emit-silgen -parse-as-library %S/Inputs/invalid-module-alias.swift -module-name foo -module-alias foo=bar 2>&1 | %FileCheck -check-prefix=INVALID_MODULE_ALIAS %s | ||
// INVALID_MODULE_ALIAS: error: invalid module alias "foo"; make sure the alias differs from the module name, module ABI name, module link name, and a standard library name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I meant to make it sound more general, so names like lib- are not passed in either. |
||
|
||
// RUN: not %target-swift-frontend -emit-silgen -parse-as-library %S/Inputs/invalid-module-alias.swift -module-name foo -module-alias Swift=Bar 2>&1 | %FileCheck -check-prefix=INVALID_MODULE_ALIAS1 %s | ||
// INVALID_MODULE_ALIAS1: error: invalid module alias "Swift"; make sure the alias differs from the module name, module ABI name, module link name, and a standard library name | ||
|
||
// RUN: not %target-swift-frontend -emit-silgen -parse-as-library %S/Inputs/invalid-module-alias.swift -module-name foo -module-alias bar=bar 2>&1 | %FileCheck -check-prefix=INVALID_MODULE_ALIAS2 %s | ||
// INVALID_MODULE_ALIAS2: error: duplicate module alias; the name "bar" is already used for a module alias or an underlying name | ||
|
||
// RUN: not %target-swift-frontend -emit-silgen -parse-as-library %S/Inputs/invalid-module-alias.swift -module-name foo -module-alias bar=baz -module-alias baz=cat 2>&1 | %FileCheck -check-prefix=INVALID_MODULE_ALIAS3 %s | ||
// INVALID_MODULE_ALIAS3: error: duplicate module alias; the name "baz" is already used for a module alias or an underlying name | ||
|
||
// RUN: not %target-swift-frontend -emit-silgen -parse-as-library %S/Inputs/invalid-module-alias.swift -module-name foo -module-alias bar 2>&1 | %FileCheck -check-prefix=INVALID_MODULE_ALIAS4 %s | ||
// INVALID_MODULE_ALIAS4: error: invalid module alias format "bar"; make sure to use the format '-module-alias alias_name=underlying_name' | ||
|
||
// RUN: not %target-swift-frontend -emit-silgen -parse-as-library %S/Inputs/invalid-module-alias.swift -module-name foo -module-alias bar=c-a.t 2>&1 | %FileCheck -check-prefix=INVALID_MODULE_NAME %s | ||
// INVALID_MODULE_NAME: error: module name "c-a.t" is not a valid identifier | ||
|
||
// These should succeed. | ||
// RUN: %target-swift-frontend -emit-silgen %S/Inputs/invalid-module-alias.swift > /dev/null | ||
// RUN: %target-swift-frontend -emit-silgen -parse-as-library -module-name foo %S/Inputs/invalid-module-alias.swift -module-alias bar=cat > /dev/null | ||
// RUN: %target-swift-frontend -typecheck -parse-as-library -module-name foo %S/Inputs/invalid-module-alias.swift -module-alias bar=cat | ||
|
Uh oh!
There was an error while loading. Please reload this page.