Skip to content

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

Merged
merged 5 commits into from
Sep 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion include/swift/AST/DiagnosticsFrontend.def
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,14 @@ ERROR(error_stdlib_module_name,none,
"module name \"%0\" is reserved for the standard library"
"%select{|; use -module-name flag to specify an alternate name}1",
(StringRef, bool))

ERROR(error_stdlib_not_found,Fatal,
"unable to load standard library for target '%0'", (StringRef))
ERROR(error_module_alias_invalid_format,none,
"invalid module alias format \"%0\"; make sure to use the format '-module-alias alias_name=underlying_name'", (StringRef))
ERROR(error_module_alias_forbidden_name,none,
"invalid module alias \"%0\"; make sure the alias differs from the module name, module ABI name, module link name, and a standard library name", (StringRef))
ERROR(error_module_alias_duplicate,none,
"duplicate module alias; the name \"%0\" is already used for a module alias or an underlying name", (StringRef))

ERROR(error_unable_to_load_supplementary_output_file_map, none,
"unable to load supplementary output file map '%0': %1",
Expand Down
4 changes: 4 additions & 0 deletions include/swift/Frontend/FrontendOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "swift/Frontend/FrontendInputsAndOutputs.h"
#include "swift/Frontend/InputFile.h"
#include "llvm/ADT/Hashing.h"
#include "llvm/ADT/StringMap.h"

#include <string>
#include <vector>
Expand Down Expand Up @@ -48,6 +49,9 @@ class FrontendOptions {
/// An Objective-C header to import and make implicitly visible.
std::string ImplicitObjCHeaderPath;

/// The map of aliases and underlying names of imported or referenced modules.
llvm::StringMap<StringRef> ModuleAliasMap;

/// The name of the module that the frontend is building.
std::string ModuleName;

Expand Down
5 changes: 5 additions & 0 deletions include/swift/Option/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,11 @@ def module_name : Separate<["-"], "module-name">,
def module_name_EQ : Joined<["-"], "module-name=">, Flags<[FrontendOption]>,
Alias<module_name>;

def module_alias : Separate<["-"], "module-alias">,
Flags<[FrontendOption]>,
MetaVarName<"<alias_name=underlying_name>">,
HelpText<"If a source file imports or references module <alias_name>, the underlying name is used for the contents of the file">;

def module_link_name : Separate<["-"], "module-link-name">,
Flags<[FrontendOption, ModuleInterfaceOption]>,
HelpText<"Library to link against when using this module">;
Expand Down
63 changes: 63 additions & 0 deletions lib/Frontend/ArgsToFrontendOptionsConverter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -498,7 +503,65 @@ bool ArgsToFrontendOptionsConverter::setUpImmediateArgs() {
return false;
}

bool ArgsToFrontendOptionsConverter::computeModuleAliases() {
auto list = Args.getAllArgValues(options::OPT_module_alias);
if (!list.empty()) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that mean that emojis are accepted? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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();
Expand Down
1 change: 1 addition & 0 deletions lib/Frontend/ArgsToFrontendOptionsConverter.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class ArgsToFrontendOptionsConverter {
void computeDebugTimeOptions();
bool computeFallbackModuleName();
bool computeModuleName();
bool computeModuleAliases();
bool computeMainAndSupplementaryOutputFilenames();
void computeDumpScopeMapLocations();
void computeHelpOptions();
Expand Down
3 changes: 3 additions & 0 deletions test/Frontend/Inputs/invalid-module-alias.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
func quip() {
}

25 changes: 25 additions & 0 deletions test/Frontend/invalid-module-alias.swift
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a standard library name possible value only Swift in practice or does it concern other core libraries? If it has just one possible value we could pass down the string value of STDLIB_NAME in the diagnostic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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