Refactor ABI options as part of HIR function qualifiers

The AST has an ABI string as part of the function qualifiers, this was the
same in the HIR as it was initially a copy-paste. This patch changes the
HIR function qualifiers to have an enum of ABI options, during HIR lowering
the enum is setup and if an unknown ABI option is specified an error is
emitted.

8 files changed, 54 insertions, 61 deletions

diff --git a/gcc/rust/ast/rust-item.h b/gcc/rust/ast/rust-item.h
index 567da0c..a279088 100644
--- a/gcc/rust/ast/rust-item.h
+++ b/gcc/rust/ast/rust-item.h
@@ -484,25 +484,20 @@ public:
 // Qualifiers for function, i.e. const, unsafe, extern etc.
 struct FunctionQualifiers
 {
-public:
-  /* Whether the function is neither const nor async, const only, or async
-   * only. */
-
 private:
   AsyncConstStatus const_status;
   bool has_unsafe;
   bool has_extern;
-  std::string extern_abi; // e.g. extern "C" fn() -> i32 {}
-  // TODO: maybe ensure that extern_abi only exists if extern exists?
-
-  // should this store location info?
+  std::string extern_abi;
+  Location locus;
 
 public:
-  FunctionQualifiers (AsyncConstStatus const_status, bool has_unsafe,
-		      bool has_extern = false,
+  FunctionQualifiers (Location locus, AsyncConstStatus const_status,
+		      bool has_unsafe, bool has_extern = false,
 		      std::string extern_abi = std::string ())
     : const_status (const_status), has_unsafe (has_unsafe),
-      has_extern (has_extern), extern_abi (std::move (extern_abi))
+      has_extern (has_extern), extern_abi (std::move (extern_abi)),
+      locus (locus)
   {
     if (!this->extern_abi.empty ())
       {
@@ -517,6 +512,9 @@ public:
   bool is_unsafe () const { return has_unsafe; }
   bool is_extern () const { return has_extern; }
   std::string get_extern_abi () const { return extern_abi; }
+  bool has_abi () const { return !extern_abi.empty (); }
+
+  Location get_locus () const { return locus; }
 };
 
 // A function parameter
@@ -723,7 +721,7 @@ public:
   // Creates an error state method.
   static Method create_error ()
   {
-    return Method ("", FunctionQualifiers (NONE, true),
+    return Method ("", FunctionQualifiers (Location (), NONE, true),
 		   std::vector<std::unique_ptr<GenericParam>> (),
 		   SelfParam::create_error (), std::vector<FunctionParam> (),
 		   nullptr, WhereClause::create_empty (), nullptr,
diff --git a/gcc/rust/hir/rust-ast-lower-item.h b/gcc/rust/hir/rust-ast-lower-item.h
index 660a30c..2f007ed 100644
--- a/gcc/rust/hir/rust-ast-lower-item.h
+++ b/gcc/rust/hir/rust-ast-lower-item.h
@@ -800,15 +800,23 @@ public:
 	extern_items.push_back (std::unique_ptr<HIR::ExternalItem> (lowered));
       }
 
+    ABI abi = ABI::RUST;
+    if (extern_block.has_abi ())
+      {
+	const std::string &extern_abi = extern_block.get_abi ();
+	abi = get_abi_from_string (extern_abi);
+	if (abi == ABI::UNKNOWN)
+	  rust_error_at (extern_block.get_locus (), "unknown ABI option");
+      }
+
     auto crate_num = mappings->get_current_crate ();
     Analysis::NodeMapping mapping (crate_num, extern_block.get_node_id (),
 				   mappings->get_next_hir_id (crate_num),
 				   mappings->get_next_localdef_id (crate_num));
 
     HIR::ExternBlock *hir_extern_block
-      = new HIR::ExternBlock (mapping, extern_block.get_abi (),
-			      std::move (extern_items), std::move (vis),
-			      extern_block.get_inner_attrs (),
+      = new HIR::ExternBlock (mapping, abi, std::move (extern_items),
+			      std::move (vis), extern_block.get_inner_attrs (),
 			      extern_block.get_outer_attrs (),
 			      extern_block.get_locus ());
 
diff --git a/gcc/rust/hir/rust-ast-lower.cc b/gcc/rust/hir/rust-ast-lower.cc
index 8062f2f..d9ce9aa 100644
--- a/gcc/rust/hir/rust-ast-lower.cc
+++ b/gcc/rust/hir/rust-ast-lower.cc
@@ -602,11 +602,17 @@ ASTLoweringBase::lower_qualifiers (const AST::FunctionQualifiers &qualifiers)
     = qualifiers.is_unsafe () ? Unsafety::Unsafe : Unsafety::Normal;
   bool has_extern = qualifiers.is_extern ();
 
-  // FIXME turn this into the Rust::ABI enum
-  std::string extern_abi = qualifiers.get_extern_abi ();
+  ABI abi = ABI::RUST;
+  if (qualifiers.has_abi ())
+    {
+      const std::string &extern_abi = qualifiers.get_extern_abi ();
+      abi = get_abi_from_string (extern_abi);
+      if (has_extern && abi == ABI::UNKNOWN)
+	rust_error_at (qualifiers.get_locus (), "unknown ABI option");
+    }
 
   return HIR::FunctionQualifiers (qualifiers.get_const_status (), unsafety,
-				  has_extern, extern_abi);
+				  has_extern, abi);
 }
 
 void
diff --git a/gcc/rust/hir/tree/rust-hir-full-test.cc b/gcc/rust/hir/tree/rust-hir-full-test.cc
index 3f3f484..051e503 100644
--- a/gcc/rust/hir/tree/rust-hir-full-test.cc
+++ b/gcc/rust/hir/tree/rust-hir-full-test.cc
@@ -1030,10 +1030,7 @@ ExternBlock::as_string () const
   std::string str = VisItem::as_string ();
 
   str += "extern ";
-  if (has_abi ())
-    {
-      str += "\"" + abi + "\" ";
-    }
+  str += "\"" + get_string_from_abi (abi) + "\" ";
 
   // inner attributes
   str += "\n inner attributes: ";
@@ -2058,10 +2055,7 @@ FunctionQualifiers::as_string () const
   if (has_extern)
     {
       str += "extern";
-      if (extern_abi != "")
-	{
-	  str += " \"" + extern_abi + "\"";
-	}
+      str += " \"" + get_string_from_abi (abi) + "\"";
     }
 
   return str;
diff --git a/gcc/rust/hir/tree/rust-hir-item.h b/gcc/rust/hir/tree/rust-hir-item.h
index 25e5681..6d65837 100644
--- a/gcc/rust/hir/tree/rust-hir-item.h
+++ b/gcc/rust/hir/tree/rust-hir-item.h
@@ -19,6 +19,7 @@
 #ifndef RUST_HIR_ITEM_H
 #define RUST_HIR_ITEM_H
 
+#include "rust-abi.h"
 #include "rust-ast-full-decls.h"
 #include "rust-common.h"
 #include "rust-hir.h"
@@ -481,28 +482,22 @@ private:
   AsyncConstStatus const_status;
   Unsafety unsafety;
   bool has_extern;
-  std::string extern_abi; // e.g. extern "C" fn() -> i32 {}
-  // TODO: maybe ensure that extern_abi only exists if extern exists?
+  ABI abi;
 
 public:
   FunctionQualifiers (AsyncConstStatus const_status, Unsafety unsafety,
-		      bool has_extern = false,
-		      std::string extern_abi = std::string ())
+		      bool has_extern, ABI abi)
     : const_status (const_status), unsafety (unsafety), has_extern (has_extern),
-      extern_abi (std::move (extern_abi))
-  {
-    if (!this->extern_abi.empty ())
-      {
-	// having extern is required; not having it is an implementation error
-	gcc_assert (has_extern);
-      }
-  }
+      abi (abi)
+  {}
 
   std::string as_string () const;
 
   AsyncConstStatus get_status () const { return const_status; }
 
   bool is_const () const { return const_status == AsyncConstStatus::CONST_FN; }
+
+  ABI get_abi () const { return abi; }
 };
 
 // A function parameter
@@ -3082,15 +3077,9 @@ protected:
 // An extern block HIR node
 class ExternBlock : public VisItem
 {
-  // bool has_abi;
-  std::string abi;
-
-  // bool has_inner_attrs;
+  ABI abi;
   AST::AttrVec inner_attrs;
-
-  // bool has_extern_items;
   std::vector<std::unique_ptr<ExternalItem>> extern_items;
-
   Location locus;
 
 public:
@@ -3102,17 +3091,14 @@ public:
   // Returns whether extern block has extern items.
   bool has_extern_items () const { return !extern_items.empty (); }
 
-  // Returns whether extern block has ABI name.
-  bool has_abi () const { return !abi.empty (); }
-
-  std::string get_abi () const { return abi; }
+  ABI get_abi () const { return abi; }
 
-  ExternBlock (Analysis::NodeMapping mappings, std::string abi,
+  ExternBlock (Analysis::NodeMapping mappings, ABI abi,
 	       std::vector<std::unique_ptr<ExternalItem>> extern_items,
 	       Visibility vis, AST::AttrVec inner_attrs,
 	       AST::AttrVec outer_attrs, Location locus)
     : VisItem (std::move (mappings), std::move (vis), std::move (outer_attrs)),
-      abi (std::move (abi)), inner_attrs (std::move (inner_attrs)),
+      abi (abi), inner_attrs (std::move (inner_attrs)),
       extern_items (std::move (extern_items)), locus (locus)
   {}
 
diff --git a/gcc/rust/parse/rust-parse-impl.h b/gcc/rust/parse/rust-parse-impl.h
index 8559c3b..db73828 100644
--- a/gcc/rust/parse/rust-parse-impl.h
+++ b/gcc/rust/parse/rust-parse-impl.h
@@ -2620,13 +2620,13 @@ AST::FunctionQualifiers
 Parser<ManagedTokenSource>::parse_function_qualifiers ()
 {
   AsyncConstStatus const_status = NONE;
-  // bool has_const = false;
   bool has_unsafe = false;
   bool has_extern = false;
   std::string abi;
 
   // Check in order of const, unsafe, then extern
   const_TokenPtr t = lexer.peek_token ();
+  Location locus = t->get_locus ();
   switch (t->get_id ())
     {
     case CONST:
@@ -2662,7 +2662,7 @@ Parser<ManagedTokenSource>::parse_function_qualifiers ()
 	}
     }
 
-  return AST::FunctionQualifiers (const_status, has_unsafe, has_extern,
+  return AST::FunctionQualifiers (locus, const_status, has_unsafe, has_extern,
 				  std::move (abi));
 }
 
diff --git a/gcc/rust/typecheck/rust-hir-type-check-implitem.h b/gcc/rust/typecheck/rust-hir-type-check-implitem.h
index 80fd0ec..1722d16 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-implitem.h
+++ b/gcc/rust/typecheck/rust-hir-type-check-implitem.h
@@ -127,17 +127,11 @@ public:
 			      function.get_item_name ()),
       function.get_locus ()};
 
-    auto abi = get_abi_from_string (parent.get_abi ());
-    if (abi == ABI::UNKNOWN)
-      {
-	rust_error_at (parent.get_locus (), "unknown abi");
-      }
-
     auto fnType = new TyTy::FnType (function.get_mappings ().get_hirid (),
 				    function.get_mappings ().get_defid (),
 				    function.get_item_name (), ident, flags,
-				    abi, std::move (params), ret_type,
-				    std::move (substitutions));
+				    parent.get_abi (), std::move (params),
+				    ret_type, std::move (substitutions));
 
     context->insert_type (function.get_mappings (), fnType);
   }
diff --git a/gcc/testsuite/rust/compile/abi-options1.rs b/gcc/testsuite/rust/compile/abi-options1.rs
new file mode 100644
index 0000000..a4b6241
--- /dev/null
+++ b/gcc/testsuite/rust/compile/abi-options1.rs
@@ -0,0 +1,7 @@
+extern "foobar" {
+    // { dg-error "unknown ABI option" "" { target *-*-* } .-1 }
+    fn printf(s: *const i8, ...);
+}
+
+pub extern "baz" fn test() {}
+// { dg-error "unknown ABI option" "" { target *-*-* } .-1 }