Merge #1088

1088: rust-session-manager: better crate name handling logic r=philberty a=liushuyu

- rust-session-manager: set and validate crate name properly
- testsuite/rust: fix the testcases and add more testcases for testing crate name handling

Co-authored-by: liushuyu <[email protected]>

Author: bors[bot]

gcc/rust/rust-lang.cc

@@ -458,6 +458,7 @@ run_rust_tests ()
   simple_assert ();
   rust_cfg_parser_test ();
   rust_privacy_ctx_test ();
+  rust_crate_name_validation_test ();
 }
 } // namespace selftest
 

gcc/rust/rust-session-manager.cc

@@ -35,6 +35,7 @@
 #include "diagnostic.h"
 #include "input.h"
 #include "rust-target.h"
+#include "selftest.h"
 
 extern bool
 saw_errors (void);
@@ -54,7 +55,65 @@ const char *kHIRDumpFile = "gccrs.hir.dump";
 const char *kHIRTypeResolutionDumpFile = "gccrs.type-resolution.dump";
 const char *kTargetOptionsDumpFile = "gccrs.target-options.dump";
 
-const std::string kDefaultCrateName = "example";
+const std::string kDefaultCrateName = "rust_out";
+const size_t kMaxNameLength = 64;
+
+static std::string
+infer_crate_name (const std::string &filename)
+{
+  if (filename == "-")
+    return kDefaultCrateName;
+
+  std::string crate = std::string (filename);
+  size_t path_sep = crate.find_last_of (file_separator);
+
+  // find the base filename
+  if (path_sep != std::string::npos)
+    crate.erase (0, path_sep + 1);
+
+  // find the file stem name (remove file extension)
+  size_t ext_position = crate.find_last_of ('.');
+  if (ext_position != std::string::npos)
+    crate.erase (ext_position);
+
+  // Replace all the '-' symbols with '_' per Rust rules
+  for (auto &c : crate)
+    {
+      if (c == '-')
+	c = '_';
+    }
+  return crate;
+}
+
+/* Validate the crate name using the ASCII rules
+   TODO: Support Unicode version of the rules */
+
+static bool
+validate_crate_name (const std::string &crate_name, Error &error)
+{
+  if (crate_name.empty ())
+    {
+      error = Error (Location (), "crate name cannot be empty");
+      return false;
+    }
+  if (crate_name.length () > kMaxNameLength)
+    {
+      error = Error (Location (), "crate name cannot exceed %ld characters",
+		     kMaxNameLength);
+      return false;
+    }
+  for (auto &c : crate_name)
+    {
+      if (!(ISALNUM (c) || c == '_'))
+	{
+	  error = Error (Location (),
+			 "invalid character %<%c%> in crate name: %<%s%>", c,
+			 crate_name.c_str ());
+	  return false;
+	}
+    }
+  return true;
+}
 
 // Implicitly enable a target_feature (and recursively enable dependencies).
 void
@@ -311,10 +370,6 @@ Session::init ()
 
   // setup backend to GCC GIMPLE
   backend = rust_get_backend ();
-
-  // set the default crate name if crate name was unset
-  if (options.crate_name.empty ())
-    options.set_crate_name (kDefaultCrateName);
 }
 
 /* Initialise default options. Actually called before handle_option, unlike init
@@ -347,7 +402,16 @@ Session::handle_option (
     case OPT_frust_crate_:
       // set the crate name
       if (arg != nullptr)
-	ret = options.set_crate_name (arg);
+	{
+	  auto error = Error (Location (), std::string ());
+	  if ((ret = validate_crate_name (arg, error)))
+	    options.set_crate_name (arg);
+	  else
+	    {
+	      rust_assert (!error.message.empty ());
+	      error.emit_error ();
+	    }
+	}
       else
 	ret = false;
       break;
@@ -479,6 +543,32 @@ Session::enable_dump (std::string arg)
 void
 Session::parse_files (int num_files, const char **files)
 {
+  if (options.crate_name.empty ())
+    {
+      /* HACK: We use the first file to infer the crate name, which might be
+       * incorrect: since rustc only allows one file to be supplied in the
+       * command-line */
+      auto filename = "-";
+      if (num_files > 0)
+	filename = files[0];
+
+      auto crate_name = infer_crate_name (filename);
+      Error error ((Location ()), std::string ());
+      rust_debug ("inferred crate name: %s", crate_name.c_str ());
+      if (!validate_crate_name (crate_name, error))
+	{
+	  // fake a linemapping so that we can show the filename
+	  linemap->start_file (filename, 0);
+	  linemap->start_line (0, 1);
+	  error.emit_error ();
+	  rust_inform (linemap->get_location (0),
+		       "crate name inferred from this file");
+	  linemap->stop ();
+	  return;
+	}
+      options.set_crate_name (crate_name);
+    }
+
   auto mappings = Analysis::Mappings::get ();
   CrateNum crate_num = mappings->setup_crate_mappings (options.crate_name);
   mappings->set_current_crate (crate_num);
@@ -1121,3 +1211,34 @@ TargetOptions::enable_implicit_feature_reqs (std::string feature)
  *  - code generation
  *  - link */
 } // namespace Rust
+
+#if CHECKING_P
+namespace selftest {
+void
+rust_crate_name_validation_test (void)
+{
+  auto error = Rust::Error (Location (), std::string ());
+  ASSERT_TRUE (Rust::validate_crate_name ("example", error));
+  ASSERT_TRUE (Rust::validate_crate_name ("abcdefg_1234", error));
+  ASSERT_TRUE (Rust::validate_crate_name ("1", error));
+  // FIXME: The next test does not pass as of current implementation
+  // ASSERT_TRUE (Rust::CompileOptions::validate_crate_name ("惊吓"));
+  // NOTE: - is not allowed in the crate name ...
+
+  ASSERT_FALSE (Rust::validate_crate_name ("abcdefg-1234", error));
+  ASSERT_FALSE (Rust::validate_crate_name ("a+b", error));
+  ASSERT_FALSE (Rust::validate_crate_name ("/a+b/", error));
+
+  /* Tests for crate name inference */
+  ASSERT_EQ (Rust::infer_crate_name ("c.rs"), "c");
+  // NOTE: ... but - is allowed when in the filename
+  ASSERT_EQ (Rust::infer_crate_name ("a-b.rs"), "a_b");
+  ASSERT_EQ (Rust::infer_crate_name ("book.rs.txt"), "book.rs");
+#if defined(HAVE_DOS_BASED_FILE_SYSTEM)
+  ASSERT_EQ (Rust::infer_crate_name ("a\\c\\a-b.rs"), "a_b");
+#else
+  ASSERT_EQ (Rust::infer_crate_name ("a/c/a-b.rs"), "a_b");
+#endif
+}
+} // namespace selftest
+#endif // CHECKING_P

gcc/rust/rust-session-manager.h

@@ -22,6 +22,7 @@
 
 #include "rust-linemap.h"
 #include "rust-backend.h"
+#include "safe-ctype.h"
 
 #include "config.h"
 #include "rust-system.h"
@@ -212,11 +213,11 @@ struct CompileOptions
     enable_dump_option (DumpOption::TYPE_RESOLUTION_DUMP);
   }
 
-  bool set_crate_name (std::string name)
+  void set_crate_name (std::string name)
   {
-    // TODO: validate the crate name?
+    rust_assert (!name.empty ());
+
     crate_name = std::move (name);
-    return true;
   }
 
   void set_edition (int raw_edition)
@@ -304,4 +305,11 @@ struct Session
 };
 } // namespace Rust
 
+#if CHECKING_P
+namespace selftest {
+extern void
+rust_crate_name_validation_test (void);
+}
+#endif // CHECKING_P
+
 #endif

gcc/testsuite/rust/compile/bad-crate-name.rs

@@ -0,0 +1,4 @@
+// { dg-additional-options "-frust-crate=bad+name" }
+// { dg-excess-errors "invalid crate name: ...." }
+// { dg-excess-errors "unrecognized command-line option ...." }
+fn main() {}

gcc/testsuite/rust/compile/bad=file-name.rs

@@ -0,0 +1,7 @@
+// { dg-additional-options "-fdump-tree-gimple -frust-crate=good_name" }
+pub fn does_nothing() {}
+fn main() {
+    does_nothing()
+}
+// { dg-final { scan-tree-dump-times {good_name::does_nothing} 2 gimple } }
+// { dg-final { scan-tree-dump-times {good_name::main} 1 gimple } }

gcc/testsuite/rust/compile/bad_file_name.txt.rs

@@ -0,0 +1,3 @@
+// { dg-excess-errors "invalid crate name: ...." }
+// { dg-bogus "unrecognized command-line option ...." }
+fn main() {}

gcc/testsuite/rust/compile/canonical_paths1.rs

@@ -1,4 +1,4 @@
-// { dg-additional-options "-w -fdump-tree-gimple" }
+// { dg-additional-options "-w -fdump-tree-gimple -frust-crate=example" }
 struct Foo(i32);
 
 trait TR {

gcc/testsuite/rust/compile/infer-crate-name.rs

@@ -0,0 +1,7 @@
+// { dg-additional-options "-fdump-tree-gimple" }
+pub fn does_nothing() {}
+fn main() {
+    does_nothing()
+}
+// { dg-final { scan-tree-dump-times {infer_crate_name::does_nothing} 2 gimple } }
+// { dg-final { scan-tree-dump-times {infer_crate_name::main} 1 gimple } }

gcc/testsuite/rust/compile/specify-crate-name.rs

@@ -0,0 +1,7 @@
+// { dg-additional-options "-frust-crate=fancy_crate_name -fdump-tree-gimple" }
+pub fn does_nothing() {}
+fn main() {
+    does_nothing()
+}
+// { dg-final { scan-tree-dump-times {fancy_crate_name::does_nothing} 2 gimple } }
+// { dg-final { scan-tree-dump-times {fancy_crate_name::main} 1 gimple } }

gcc/testsuite/rust/compile/torture/struct_decl.rs

@@ -1,4 +1,4 @@
-// { dg-additional-options -fdump-tree-gimple }
+// { dg-additional-options "-fdump-tree-gimple -frust-crate=example" }
 
 struct Foo {
     a: u16,

gcc/testsuite/rust/compile/traits9.rs

@@ -1,3 +1,4 @@
+// { dg-additional-options -frust-crate=example }
 struct Foo(i32);
 trait Bar {
     fn baz(&self);