frust-cfg: Only allow double quoted values

This commit separates the `handle_cfg_option()` function in two,
separating the parsing logic from the session logic. The parsing logic
is able to be unit tested, and now only allows quoted values.

Author: CohenArthur

gcc/rust/Make-lang.in

@@ -66,6 +66,7 @@ GRS_OBJS = \
     rust/rust-gcc.o \
     rust/rust-token.o \
     rust/rust-lex.o \
+    rust/rust-cfg-parser.o \
     rust/rust-parse.o \
     rust/rust-ast-full-test.o \
     rust/rust-session-manager.o \

gcc/rust/parse/rust-cfg-parser.cc

@@ -0,0 +1,75 @@
+#include "rust-cfg-parser.h"
+#include "selftest.h"
+
+namespace Rust {
+bool
+parse_cfg_option (const std::string &input, std::string &key,
+		  std::string &value)
+{
+  key.clear ();
+  value.clear ();
+
+  auto equal = input.find ('=');
+
+  // If there is no equal sign, it means there is no value. Clean up the key
+  // and return
+  if (equal == std::string::npos)
+    {
+      key = input;
+
+      // FIXME: Make sure key is a proper identifier
+
+      return true;
+    }
+
+  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);
+
+  // FIXME: We need to sanitize here and make sure that both key and value
+  // are proper identifiers
+
+  return true;
+}
+
+} // namespace Rust
+
+#if CHECKING_P
+
+namespace selftest {
+
+void
+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");
+  ASSERT_TRUE (value.empty ());
+
+  ASSERT_TRUE (Rust::parse_cfg_option ("k=\"v\"", 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));
+
+  // No value is an error if there is an equal sign
+  ASSERT_FALSE (Rust::parse_cfg_option ("k=", key, value));
+
+  // No key is an error
+  ASSERT_FALSE (Rust::parse_cfg_option ("=", key, value));
+  ASSERT_FALSE (Rust::parse_cfg_option ("=value", key, value));
+}
+} // namespace selftest
+
+#endif // CHECKING_P

gcc/rust/parse/rust-cfg-parser.h

@@ -0,0 +1,57 @@
+/* This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>. */
+
+#ifndef RUST_CFG_PARSER_H
+#define RUST_CFG_PARSER_H
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+
+#include <string>
+
+namespace Rust {
+/**
+ * Parse a `key` or `key="value"` pair given to the `-frust-cfg` compiler
+ * option.
+ *
+ * The format is as follows:
+ *
+ * -frust-cfg=<input>
+ *
+ * cfg_input: identifier | identifier '=' '"' identifier '"'
+ *
+ * @param input User input given to the -frust-cfg option
+ * @param key String in which to store the parsed `key`.
+ * @param value String in which to store the parsed `value` if it exists
+ *
+ * @return false if the given input was invalid, true otherwise
+ */
+bool
+parse_cfg_option (const std::string &input, std::string &key,
+		  std::string &value);
+} // namespace Rust
+
+#if CHECKING_P
+
+namespace selftest {
+extern void
+rust_cfg_parser_test (void);
+} // namespace selftest
+
+#endif // CHECKING_P
+
+#endif // RUST_CFG_PARSER_H

gcc/rust/parse/rust-parse.cc

@@ -118,5 +118,4 @@ extract_module_path (const AST::AttrVec &inner_attrs,
 
   return path;
 }
-
 } // namespace Rust

gcc/rust/rust-lang.cc

@@ -33,6 +33,7 @@
 #include "langhooks.h"
 #include "langhooks-def.h"
 #include "selftest.h"
+#include "rust-cfg-parser.h"
 
 #include <mpfr.h>
 // note: header files must be in this order or else forward declarations don't
@@ -453,6 +454,7 @@ run_rust_tests ()
 {
   // Call tests for the rust frontend here
   simple_assert ();
+  rust_cfg_parser_test ();
 }
 } // namespace selftest
 

gcc/rust/rust-session-manager.cc

@@ -29,6 +29,7 @@
 #include "rust-tycheck-dump.h"
 #include "rust-ast-resolve-unused.h"
 #include "rust-compile.h"
+#include "rust-cfg-parser.h"
 
 #include "diagnostic.h"
 #include "input.h"
@@ -382,87 +383,22 @@ Session::handle_cfg_option (const std::string &input)
   std::string key;
   std::string value;
 
-  enum pstate
-  {
-    KEY,
-    EQ,
-    VAL,
-    DONE,
-    ERROR
-  };
-
-  // FIXME
-  // we need to use the GCC self_test framework to unit-test this its
-  // likely got a bunch of bugs. This simple parser could be extracted to a
-  // helper function to be more easily unit-tested or it could be tested via
-  // checking what the target_options contain
-  bool expect_quote = false;
-  pstate s = KEY;
-  for (const auto &ch : input)
+  // Refactor this if needed
+  if (!parse_cfg_option (input, key, value))
     {
-      if (ch == ' ')
-	{
-	  if (!key.empty ())
-	    s = EQ;
-	  else if (!value.empty ())
-	    s = DONE;
-	  else
-	    {
-	      s = ERROR;
-	      break;
-	    }
-	}
-      else if (ch == '"')
-	{
-	  expect_quote = !expect_quote;
-	}
-      else if (ch == '=')
-	{
-	  if (key.empty ())
-	    {
-	      s = ERROR;
-	      break;
-	    }
-
-	  s = VAL;
-	}
-      else
-	{
-	  if (s == KEY)
-	    key.push_back (ch);
-	  else if (s == VAL)
-	    value.push_back (ch);
-	  else
-	    {
-	      s = ERROR;
-	      break;
-	    }
-	}
-    }
-
-  if (key.empty () && value.empty ())
-    s = ERROR;
-
-  if (expect_quote)
-    s = ERROR;
-
-  if (s == ERROR)
-    {
-      rust_error_at (Location (),
-		     "invalid %<-frust-cfg=option%> expected %<key%> or "
-		     "key=%<value%> got %<%s%>",
-		     input.c_str ());
+      rust_error_at (
+	Location (),
+	"invalid argument to %<-frust-cfg%>: Accepted formats are "
+	"%<-frust-cfg=key%> or %<-frust-cfg=key=\"value\"%> (quoted)");
       return false;
     }
 
   if (value.empty ())
-    {
-      // rustc does not seem to error on dup key
-      options.target_data.insert_key (key);
-      return true;
-    }
+    // rustc does not seem to error on dup key
+    options.target_data.insert_key (key);
+  else
+    options.target_data.insert_key_value_pair (key, value);
 
-  options.target_data.insert_key_value_pair (key, value);
   return true;
 }
 

gcc/testsuite/rust/compile/cfg5.rs

@@ -1,4 +1,4 @@
-// { dg-additional-options "-w -frust-cfg=A=B" }
+// { dg-additional-options "-w -frust-cfg=A=\"B\"" }
 struct Foo;
 impl Foo {
     #[cfg(A = "B")]