Merge #983

983: Parse proper cfg values r=CohenArthur a=CohenArthur

Closes #936 

Co-authored-by: Arthur Cohen <arthur.cohen@embecosm.com>

7 files changed, 154 insertions, 86 deletions

diff --git a/gcc/rust/lex/rust-lex.cc b/gcc/rust/lex/rust-lex.cc
index c23088f..f620e15 100644
--- a/gcc/rust/lex/rust-lex.cc
+++ b/gcc/rust/lex/rust-lex.cc
@@ -128,7 +128,8 @@ Lexer::Lexer (const char *filename, RAIIFile file_input, Linemap *linemap)
     token_queue (TokenSource (this))
 {
   // inform line_table that file is being entered and is in line 1
-  line_map->start_file (filename, current_line);
+  if (linemap)
+    line_map->start_file (filename, current_line);
 }
 
 Lexer::~Lexer ()
@@ -152,7 +153,11 @@ Lexer::~Lexer ()
 Location
 Lexer::get_current_location ()
 {
-  return line_map->get_location (current_column);
+  if (line_map)
+    return line_map->get_location (current_column);
+  else
+    // If we have no linemap, we're lexing something without proper locations
+    return Location ();
 }
 
 int
diff --git a/gcc/rust/lex/rust-lex.h b/gcc/rust/lex/rust-lex.h
index 0ae07fe..c50f632 100644
--- a/gcc/rust/lex/rust-lex.h
+++ b/gcc/rust/lex/rust-lex.h
@@ -23,6 +23,7 @@
 #include "rust-buffered-queue.h"
 #include "rust-token.h"
 
+#include <cstdio>
 #include <utility>
 #include <tuple>
 
@@ -49,6 +50,13 @@ public:
       file = fopen (filename, "r");
   }
 
+  /**
+   * Create a RAIIFile from an existing instance of FILE*
+   */
+  RAIIFile (FILE *raw, const char *filename = nullptr)
+    : file (raw), filename (filename)
+  {}
+
   RAIIFile (const RAIIFile &other) = delete;
   RAIIFile &operator= (const RAIIFile &other) = delete;
 
@@ -57,6 +65,7 @@ public:
   {
     other.file = nullptr;
   }
+
   RAIIFile &operator= (RAIIFile &&other)
   {
     close ();
@@ -132,6 +141,19 @@ public:
   Lexer (const char *filename, RAIIFile input, Linemap *linemap);
   ~Lexer ();
 
+  /**
+   * Lex the contents of a string instead of a file
+   */
+  static Lexer lex_string (std::string input)
+  {
+    // We can perform this ugly cast to a non-const char* since we're only
+    // *reading* the string. This would not be valid if we were doing any
+    // modification to it.
+    auto string_file = fmemopen (&input[0], input.length (), "r");
+
+    return Lexer (nullptr, RAIIFile (string_file), nullptr);
+  }
+
   // don't allow copy semantics (for now, at least)
   Lexer (const Lexer &other) = delete;
   Lexer &operator= (const Lexer &other) = delete;
diff --git a/gcc/rust/parse/rust-cfg-parser.cc b/gcc/rust/parse/rust-cfg-parser.cc
index a6f34b6..f98419b 100644
--- a/gcc/rust/parse/rust-cfg-parser.cc
+++ b/gcc/rust/parse/rust-cfg-parser.cc
@@ -1,45 +1,55 @@
 #include "rust-cfg-parser.h"
+#include "rust-lex.h"
+#include "rust-parse.h"
+#include "rust-session-manager.h"
 #include "selftest.h"
 
 namespace Rust {
 bool
-parse_cfg_option (const std::string &input, std::string &key,
-		  std::string &value)
+parse_cfg_option (std::string &input, std::string &key, std::string &value)
 {
   key.clear ();
   value.clear ();
 
-  auto equal = input.find ('=');
+  auto lexer = Lexer::lex_string (input);
+  auto parser = Parser<Lexer> (std::move (lexer));
 
-  // If there is no equal sign, it means there is no value. Clean up the key
-  // and return
-  if (equal == std::string::npos)
+  auto token = parser.peek_current_token ();
+  if (token->get_id () != IDENTIFIER)
     {
-      key = input;
-
-      // FIXME: Make sure key is a proper identifier
-
-      return true;
+      return false;
     }
 
-  key = input.substr (0, equal);
-
-  auto remaining_input = input.substr (equal + 1);
-  if (remaining_input[0] != '"' || remaining_input.back () != '"')
-    return false;
-
-  // Remove the quotes around the value, by advancing one character
-  value = remaining_input.substr (1);
-  // And trimming the rightmost character. This is fine since we've already
-  // checked that both the first and last characters were quotes.
-  value.resize (value.size () - 1);
+  key = token->get_str ();
 
-  // FIXME: We need to sanitize here and make sure that both key and value
-  // are proper identifiers
+  rust_assert (parser.skip_token (IDENTIFIER));
+  token = parser.peek_current_token ();
 
-  return true;
+  switch (token->get_id ())
+    {
+    case END_OF_FILE:
+      // we're done parsing, we had a valid key, return happily
+      return true;
+    case EQUAL:
+      // We have an equal sign: Skip the token and parse an identifier
+      {
+	rust_assert (parser.skip_token (EQUAL));
+
+	auto value_expr = parser.parse_literal_expr ();
+	// We had an equal sign but no value, error out
+	if (!value_expr)
+	  return false;
+
+	if (value_expr->get_lit_type () != AST::Literal::LitType::STRING)
+	  return false;
+
+	value = value_expr->get_literal ().as_string ();
+	return true;
+      }
+    default:
+      return false;
+    }
 }
-
 } // namespace Rust
 
 #if CHECKING_P
@@ -52,23 +62,49 @@ rust_cfg_parser_test (void)
   std::string key;
   std::string value;
 
-  ASSERT_TRUE (Rust::parse_cfg_option ("key-no-value", key, value));
-  ASSERT_EQ (key, "key-no-value");
+  auto input = std::string ("key_no_value");
+
+  ASSERT_TRUE (Rust::parse_cfg_option (input, key, value));
+  ASSERT_EQ (key, "key_no_value");
   ASSERT_TRUE (value.empty ());
 
-  ASSERT_TRUE (Rust::parse_cfg_option ("k=\"v\"", key, value));
+  input = std::string ("k=\"v\"");
+
+  ASSERT_TRUE (Rust::parse_cfg_option (input, key, value));
   ASSERT_EQ (key, "k");
   ASSERT_EQ (value, "v");
 
   // values should be between double quotes
-  ASSERT_FALSE (Rust::parse_cfg_option ("k=v", key, value));
+  input = std::string ("k=v");
+  ASSERT_FALSE (Rust::parse_cfg_option (input, key, value));
 
   // No value is an error if there is an equal sign
-  ASSERT_FALSE (Rust::parse_cfg_option ("k=", key, value));
+  input = std::string ("k=");
+  ASSERT_FALSE (Rust::parse_cfg_option (input, key, value));
 
   // No key is an error
-  ASSERT_FALSE (Rust::parse_cfg_option ("=", key, value));
-  ASSERT_FALSE (Rust::parse_cfg_option ("=value", key, value));
+  input = std::string ("=");
+  ASSERT_FALSE (Rust::parse_cfg_option (input, key, value));
+
+  input = std::string ("=value");
+  ASSERT_FALSE (Rust::parse_cfg_option (input, key, value));
+
+  // values that are not string literals are an error
+  input = std::string ("key=b\"a\"");
+  ASSERT_FALSE (Rust::parse_cfg_option (input, key, value));
+
+  input = std::string ("key='v'");
+  ASSERT_FALSE (Rust::parse_cfg_option (input, key, value));
+
+  input = std::string ("key=155");
+  ASSERT_FALSE (Rust::parse_cfg_option (input, key, value));
+
+  input = std::string ("key=3.14");
+  ASSERT_FALSE (Rust::parse_cfg_option (input, key, value));
+
+  // kebab case is not valid for an identifier
+  input = std::string ("key-no-value");
+  ASSERT_FALSE (Rust::parse_cfg_option (input, key, value));
 }
 } // namespace selftest
 
diff --git a/gcc/rust/parse/rust-cfg-parser.h b/gcc/rust/parse/rust-cfg-parser.h
index a4b860f..bf4ed4d 100644
--- a/gcc/rust/parse/rust-cfg-parser.h
+++ b/gcc/rust/parse/rust-cfg-parser.h
@@ -41,8 +41,7 @@ namespace Rust {
  * @return false if the given input was invalid, true otherwise
  */
 bool
-parse_cfg_option (const std::string &input, std::string &key,
-		  std::string &value);
+parse_cfg_option (std::string &input, std::string &key, std::string &value);
 } // namespace Rust
 
 #if CHECKING_P
diff --git a/gcc/rust/rust-session-manager.cc b/gcc/rust/rust-session-manager.cc
index cd2c590..54a6443 100644
--- a/gcc/rust/rust-session-manager.cc
+++ b/gcc/rust/rust-session-manager.cc
@@ -366,9 +366,11 @@ Session::handle_option (
       Compile::Mangler::set_mangling (flag_rust_mangling);
       break;
 
-    case OPT_frust_cfg_:
-      ret = handle_cfg_option (std::string (arg));
-      break;
+      case OPT_frust_cfg_: {
+	auto string_arg = std::string (arg);
+	ret = handle_cfg_option (string_arg);
+	break;
+      }
 
     default:
       break;
@@ -378,7 +380,7 @@ Session::handle_option (
 }
 
 bool
-Session::handle_cfg_option (const std::string &input)
+Session::handle_cfg_option (std::string &input)
 {
   std::string key;
   std::string value;
@@ -402,8 +404,8 @@ Session::handle_cfg_option (const std::string &input)
   return true;
 }
 
-/* Enables a certain dump depending on the name passed in. Returns true if name
- * is valid, false otherwise. */
+/* Enables a certain dump depending on the name passed in. Returns true if
+ * name is valid, false otherwise. */
 bool
 Session::enable_dump (std::string arg)
 {
@@ -532,8 +534,8 @@ Session::parse_file (const char *filename)
    * line into crate root)
    *  - injection (some lint checks or dummy, register builtin macros, crate
    * injection)
-   *  - expansion (expands all macros, maybe build test harness, AST validation,
-   * maybe macro crate)
+   *  - expansion (expands all macros, maybe build test harness, AST
+   * validation, maybe macro crate)
    *  - resolution (name resolution, type resolution, maybe feature checking,
    * maybe buffered lints)
    *  TODO not done */
@@ -603,8 +605,8 @@ Session::parse_file (const char *filename)
   if (saw_errors ())
     return;
 
-  // scan unused has to be done after type resolution since methods are resolved
-  // at that point
+  // scan unused has to be done after type resolution since methods are
+  // resolved at that point
   Resolver::ScanUnused::Scan ();
 
   if (saw_errors ())
@@ -638,11 +640,11 @@ Session::debug_dump_load_crates (Parser<Lexer> &parser)
 
   /* TODO: search through inner attrs and see whether any of those attr paths
    * contain "no_core", "no_std", "compiler_builtins". If so/not, save certain
-   * crate names. In these names, insert items at beginning of crate items. This
-   * is crate injection. Also, inject prelude use decl at beginning (first name
-   * is assumed to be prelude - prelude is a use decl automatically generated to
-   * enable using Option and Copy without qualifying it or importing it via
-   * 'use' manually) */
+   * crate names. In these names, insert items at beginning of crate items.
+   * This is crate injection. Also, inject prelude use decl at beginning
+   * (first name is assumed to be prelude - prelude is a use decl
+   * automatically generated to enable using Option and Copy without
+   * qualifying it or importing it via 'use' manually) */
 
   std::vector<std::string> crate_names;
   for (const auto &item : crate.items)
@@ -695,8 +697,8 @@ Session::injection (AST::Crate &crate)
 
   // register builtin macros
   /* In rustc, builtin macros are divided into 3 categories depending on use -
-   * "bang" macros, "attr" macros, and "derive" macros. I think the meanings of
-   * these categories should be fairly obvious to anyone who has used rust.
+   * "bang" macros, "attr" macros, and "derive" macros. I think the meanings
+   * of these categories should be fairly obvious to anyone who has used rust.
    * Builtin macro list by category: Bang
    *      - asm
    *      - assert
@@ -739,8 +741,8 @@ Session::injection (AST::Crate &crate)
    * rustc also has a "quote" macro that is defined differently and is
    * supposedly not stable so eh. */
   /* TODO: actually implement injection of these macros. In particular, derive
-   * macros, cfg, and test should be prioritised since they seem to be used the
-   * most. */
+   * macros, cfg, and test should be prioritised since they seem to be used
+   * the most. */
 
   // crate injection
   std::vector<std::string> names;
@@ -804,11 +806,11 @@ Session::injection (AST::Crate &crate)
   crate.items.insert (crate.items.begin (), std::move (use_decl));
 
   /* TODO: potentially add checking attribute crate type? I can't figure out
-   * what this does currently comment says "Unconditionally collect crate types
-   * from attributes to make them used", which presumably refers to checking the
-   * linkage info by "crate_type". It also seems to ensure that an invalid crate
-   * type is not specified, so maybe just do that. Valid crate types: bin lib
-   * dylib staticlib cdylib rlib proc-macro */
+   * what this does currently comment says "Unconditionally collect crate
+   * types from attributes to make them used", which presumably refers to
+   * checking the linkage info by "crate_type". It also seems to ensure that
+   * an invalid crate type is not specified, so maybe just do that. Valid
+   * crate types: bin lib dylib staticlib cdylib rlib proc-macro */
 
   rust_debug ("finished injection");
 }
@@ -818,8 +820,8 @@ Session::expansion (AST::Crate &crate)
 {
   rust_debug ("started expansion");
 
-  /* rustc has a modification to windows PATH temporarily here, which may end up
-   * being required */
+  /* rustc has a modification to windows PATH temporarily here, which may end
+   * up being required */
 
   // create macro expansion config?
   // if not, would at least have to configure recursion_limit
@@ -1036,10 +1038,10 @@ TargetOptions::enable_implicit_feature_reqs (std::string feature)
  * [types/values] or absolute paths)
  *  - HIR lower (convert modified AST to simpler HIR [both expressions and
  * module tree])
- *  - resolve type aliases (replace any usages of type aliases with actual type
- * [except associated types])
- *  - resolve bind (iterate HIR tree and set binding annotations on all concrete
- * types [avoids path lookups later])
+ *  - resolve type aliases (replace any usages of type aliases with actual
+ * type [except associated types])
+ *  - resolve bind (iterate HIR tree and set binding annotations on all
+ * concrete types [avoids path lookups later])
  *  - resolve HIR markings (generate "markings" [e.g. for Copy/Send/Sync/...]
  * for all types
  *  - sort impls (small pass - sort impls into groups)
@@ -1059,8 +1061,8 @@ TargetOptions::enable_implicit_feature_reqs (std::string feature)
  * function calls)
  *  - expand HIR reborrows (apply reborrow rules [taking '&mut *v' instead of
  * 'v'])
- *  - expand HIR erasedtype (replace all erased types 'impl Trait' with the true
- * type)
+ *  - expand HIR erasedtype (replace all erased types 'impl Trait' with the
+ * true type)
  *  - typecheck expressions (validate - double check that previous passes
  * haven't broke type system rules)
  *  - lower MIR (convert HIR exprs into a control-flow graph [MIR])
@@ -1071,15 +1073,16 @@ TargetOptions::enable_implicit_feature_reqs (std::string feature)
  *  - MIR optimise (perform various simple optimisations on the MIR - constant
  * propagation, dead code elimination, borrow elimination, some inlining)
  *  - MIR validate PO (re-validate the MIR)
- *  - MIR validate full (optionally: perform expensive state-tracking validation
- * on MIR)
- *  - trans enumerate (enumerate all items needed for code generation, primarily
- * types used for generics)
- *  - trans auto impls (create magic trait impls as enumerated in previous pass)
+ *  - MIR validate full (optionally: perform expensive state-tracking
+ * validation on MIR)
+ *  - trans enumerate (enumerate all items needed for code generation,
+ * primarily types used for generics)
+ *  - trans auto impls (create magic trait impls as enumerated in previous
+ * pass)
  *  - trans monomorph (generate monomorphised copies of all functions [with
  * generics replaced with real types])
- *  - MIR optimise inline (run optimisation again, this time with full type info
- * [primarily for inlining])
+ *  - MIR optimise inline (run optimisation again, this time with full type
+ * info [primarily for inlining])
  *  - HIR serialise (write out HIR dump [module tree and generic/inline MIR])
  *  - trans codegen (generate final output file: emit C source file and call C
  * compiler) */
@@ -1087,8 +1090,8 @@ TargetOptions::enable_implicit_feature_reqs (std::string feature)
 /* rustc compile pipeline (basic, in way less detail):
  *  - parse input (parse .rs to AST)
  *  - name resolution, macro expansion, and configuration (process AST
- * recursively, resolving paths, expanding macros, processing #[cfg] nodes [i.e.
- * maybe stripping stuff from AST])
+ * recursively, resolving paths, expanding macros, processing #[cfg] nodes
+ * [i.e. maybe stripping stuff from AST])
  *  - lower to HIR
  *  - type check and other analyses (e.g. privacy checking)
  *  - lower to MIR and post-processing (and do stuff like borrow checking)
@@ -1100,14 +1103,14 @@ TargetOptions::enable_implicit_feature_reqs (std::string feature)
  *  - register plugins (attributes injection, set various options, register
  * lints, load plugins)
  *  - expansion/configure and expand (initial 'cfg' processing, 'loading
- * compiler plugins', syntax expansion, secondary 'cfg' expansion, synthesis of
- * a test harness if required, injection of any std lib dependency and prelude,
- * and name resolution) - actually documented inline
+ * compiler plugins', syntax expansion, secondary 'cfg' expansion, synthesis
+ * of a test harness if required, injection of any std lib dependency and
+ * prelude, and name resolution) - actually documented inline
  *      - seeming pierced-together order: pre-AST expansion lint checks,
  * registering builtin macros, crate injection, then expand all macros, then
- * maybe build test harness, AST validation, maybe create a macro crate (if not
- * rustdoc), name resolution, complete gated feature checking, add all buffered
- * lints
+ * maybe build test harness, AST validation, maybe create a macro crate (if
+ * not rustdoc), name resolution, complete gated feature checking, add all
+ * buffered lints
  *  - create global context (lower to HIR)
  *  - analysis on global context (HIR optimisations? create MIR?)
  *  - code generation
diff --git a/gcc/rust/rust-session-manager.h b/gcc/rust/rust-session-manager.h
index ea0523b..99d1628 100644
--- a/gcc/rust/rust-session-manager.h
+++ b/gcc/rust/rust-session-manager.h
@@ -288,7 +288,7 @@ private:
   void expansion (AST::Crate &crate);
 
   // handle cfg_option
-  bool handle_cfg_option (const std::string &data);
+  bool handle_cfg_option (std::string &data);
 };
 } // namespace Rust
 
diff --git a/gcc/rust/util/rust-hir-map.cc b/gcc/rust/util/rust-hir-map.cc
index d154a8a..9190bd9 100644
--- a/gcc/rust/util/rust-hir-map.cc
+++ b/gcc/rust/util/rust-hir-map.cc
@@ -139,7 +139,10 @@ NodeId
 Mappings::get_next_node_id (CrateNum crateNum)
 {
   auto it = nodeIdIter.find (crateNum);
-  rust_assert (it != nodeIdIter.end ());
+  // We're probably *not* parsing actual rust code... but mostly reusing
+  // the parser in another way. Return 0
+  if (it == nodeIdIter.end ())
+    return 0;
 
   auto id = it->second + 1;
   nodeIdIter[crateNum] = id;