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.

Author: philberty

gcc/rust/ast/rust-item.h

@@ -484,25 +484,20 @@ struct SelfParam
 // 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 @@ struct FunctionQualifiers
   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 @@ class Method : public InherentImplItem, public TraitImplItem
   // 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,

gcc/rust/hir/rust-ast-lower-item.h

@@ -800,15 +800,23 @@ class ASTLoweringItem : public ASTLoweringBase
 	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 ());
 

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

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;

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 @@ struct FunctionQualifiers
   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 @@ class ExternalFunctionItem : public ExternalItem
 // 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 @@ class ExternBlock : public VisItem
   // 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)
   {}
 

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));
 }
 

gcc/rust/typecheck/rust-hir-type-check-implitem.h

@@ -127,17 +127,11 @@ class TypeCheckTopLevelExternItem : public TypeCheckBase
 			      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);
   }

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 }