Use StringMap

More specific diag msgs
Add tests

Author: elsh

include/swift/AST/DiagnosticsFrontend.def

@@ -150,10 +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_bad_module_alias,none,
-      "invalid module alias \"%0\" for an imported module; make sure the input format is correct (-module-alias alias=underlying_name), where the alias maps to an imported module, and the names do not contain special characters", (StringRef))
 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 \"%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",

include/swift/Basic/LangOptions.h

@@ -31,7 +31,6 @@
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/VersionTuple.h"
 #include "llvm/Support/raw_ostream.h"
-#include <map>
 #include <string>
 #include <vector>
 
@@ -197,9 +196,6 @@ namespace swift {
     /// Enable features useful for running in the debugger.
     bool DebuggerSupport = false;
 
-    /// A map of aliases and underlying names of imported modules.
-    std::map<StringRef, StringRef> ModuleAliasMap;
-
     /// Enable the MemoryBufferSerializedModuleImporter.
     /// Only used by lldb-moduleimport-test.
     bool EnableMemoryBufferImporter = false;

include/swift/Frontend/Frontend.h

@@ -300,14 +300,6 @@ class CompilerInvocation {
     return FrontendOpts.ParseStdlib;
   }
 
-  void setModuleAliasMap(std::map<StringRef, StringRef> ModAliasMap) {
-      FrontendOpts.ModuleAliasMap = ModAliasMap;
-  }
-
-  std::map<StringRef, StringRef> getModuleAliasMap() const {
-      return FrontendOpts.ModuleAliasMap;
-  }
-
   void setModuleName(StringRef Name) {
     FrontendOpts.ModuleName = Name.str();
     IRGenOpts.ModuleName = Name.str();

include/swift/Frontend/FrontendOptions.h

@@ -18,8 +18,8 @@
 #include "swift/Frontend/FrontendInputsAndOutputs.h"
 #include "swift/Frontend/InputFile.h"
 #include "llvm/ADT/Hashing.h"
+#include "llvm/ADT/StringMap.h"
 
-#include <map>
 #include <string>
 #include <vector>
 
@@ -50,7 +50,7 @@ class FrontendOptions {
   std::string ImplicitObjCHeaderPath;
 
   /// The map of aliases and underlying names of imported modules.
-  std::map<StringRef, StringRef> ModuleAliasMap;
+  llvm::StringMap<StringRef> ModuleAliasMap;
 
   /// The name of the module that the frontend is building.
   std::string ModuleName;

include/swift/Option/Options.td

@@ -432,7 +432,8 @@ def module_name_EQ : Joined<["-"], "module-name=">, Flags<[FrontendOption]>,
 
 def module_alias : Separate<["-"], "module-alias">,
   Flags<[FrontendOption]>,
-  HelpText<"Alias and underlying name 'alias=underlying_name' of the module to be imported">;
+  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]>,

lib/Frontend/ArgsToFrontendOptionsConverter.cpp

@@ -506,38 +506,63 @@ bool ArgsToFrontendOptionsConverter::setUpImmediateArgs() {
 bool ArgsToFrontendOptionsConverter::computeModuleAliases() {
   auto list = Args.getAllArgValues(options::OPT_module_alias);
   if (!list.empty()) {
-    for (auto val: list) {
-      auto i = val.find("=");
-      if (i == std::string::npos) {
-          Diags.diagnose(SourceLoc(), diag::error_bad_module_alias, val);
+    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)) {
+        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;
       }
-
-      std::string *lhs = new std::string(val.substr(0, i));
-      std::string *rhs = new std::string(val.substr(i+1, val.length()));
-      auto modAlias = StringRef(*lhs);
-      auto modValue = StringRef(*rhs);
-
-      if (modAlias.empty() ||
-          modValue.empty() ||
-          modAlias == Opts.ModuleName ||
-          modAlias == Opts.ModuleABIName ||
-          modAlias == Opts.ModuleLinkName ||
-          modValue == Opts.ModuleName ||
-          modValue == Opts.ModuleABIName ||
-          modValue == Opts.ModuleLinkName ||
-          !Lexer::isIdentifier(modAlias) ||
-          !Lexer::isIdentifier(modValue)) {
-          Diags.diagnose(SourceLoc(), diag::error_bad_module_alias, val);
+      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;
       }
-      Opts.ModuleAliasMap.insert({ modAlias, modValue });
     }
   }
   return false;
 }
 
 bool ArgsToFrontendOptionsConverter::computeModuleName() {
+  // Module alias (if any) should be computed after module name is computed
+  assert(Opts.ModuleAliasMap.empty());
+
   const Arg *A = Args.getLastArg(options::OPT_module_name);
   if (A) {
     Opts.ModuleName = A->getValue();

lib/Frontend/Frontend.cpp

@@ -211,7 +211,6 @@ bool CompilerInstance::setUpASTContextIfNeeded() {
   // and single file builds.
   Invocation.getLangOptions().RecordRequestReferences
     = !isWholeModuleCompilation();
-  Invocation.getLangOptions().ModuleAliasMap = Invocation.getFrontendOptions().ModuleAliasMap;
 
   Context.reset(ASTContext::get(
       Invocation.getLangOptions(), Invocation.getTypeCheckerOptions(),

test/Frontend/Inputs/invalid-module-alias.swift

@@ -0,0 +1,3 @@
+func quip() {
+}
+

test/Frontend/invalid-module-alias.swift

@@ -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
+
+// 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 "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
+