[COMMITTED 28/43] gccrs: Clean up internal attribute checking

arthur.cohen@embecosm.com arthur.cohen@embecosm.com
Tue Apr 14 23:18:49 GMT 2026


From: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>

Move all internal attribute structure checking within specific handler
functions called from the attribute visit function instead of multiple
duplicated sources in the various items. Store those handlers in a map
to avoid the expensive string switch comparisons.

gcc/rust/ChangeLog:

	* checks/errors/rust-builtin-attribute-checker.cc (check_doc_attribute):
	Move from here...
	(doc): ...to here.
	(check_deprecated_attribute): Move from here...
	(deprecated): ...to here.
	(check_link_section_attribute): Move from here...
	(link_section): ... to here.
	(check_export_name_attribute): Move from here...
	(export_name): ... to here.
	(check_no_mangle_function): Remove internal structure checking and move
	it to no_mangle handler.
	(check_lint_attribute): Move from here...
	(lint): ... to here.
	(link_name): Likewise with link_name.
	(check_crate_type): Move to anonymous namespace within the handler
	namespace.
	(proc_macro_derive): Add proc macro specific handler.
	(proc_macro): Likewise.
	(target_feature): Likewise.
	(no_mangle): Add specific handler for no_mangler attribute internal
	structure checking.
	(std::function<void): Add map with attribute name to handler matching.
	(tl::optional<std::function<void): Likewise.
	(lookup_handler): Add an helper function to retrieve the handler of any
	builtin attribute.
	(BuiltinAttributeChecker::visit): Change attribute visitor call with
	new handler call.
	* checks/errors/rust-builtin-attribute-checker.h (check_valid_attribute_for_item):
	Add prototype.

Signed-off-by: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
---
 .../errors/rust-builtin-attribute-checker.cc  | 248 ++++++++++--------
 .../errors/rust-builtin-attribute-checker.h   |   3 +
 2 files changed, 136 insertions(+), 115 deletions(-)

diff --git a/gcc/rust/checks/errors/rust-builtin-attribute-checker.cc b/gcc/rust/checks/errors/rust-builtin-attribute-checker.cc
index a2bf83f93cd..f6a2e2c836d 100644
--- a/gcc/rust/checks/errors/rust-builtin-attribute-checker.cc
+++ b/gcc/rust/checks/errors/rust-builtin-attribute-checker.cc
@@ -80,8 +80,15 @@ check_doc_alias (const std::string &alias_input, const location_t locus)
 		   "%<#[doc(alias)]%> input cannot start or end with a space");
 }
 
-static void
-check_doc_attribute (const AST::Attribute &attribute)
+// This namespace contains handlers for the builtin attribute checker,
+// those handlers must verify the attribute internal structure and emit the
+// appropriate error message if the structure is incorrect.
+//
+// They DO NOT check the attribute validity on the parent item.
+namespace handlers {
+
+void
+doc (const AST::Attribute &attribute)
 {
   if (!attribute.has_attr_input ())
     {
@@ -125,8 +132,8 @@ check_doc_attribute (const AST::Attribute &attribute)
     }
 }
 
-static void
-check_deprecated_attribute (const AST::Attribute &attribute)
+void
+deprecated (const AST::Attribute &attribute)
 {
   if (!attribute.has_attr_input ())
     return;
@@ -219,8 +226,8 @@ check_deprecated_attribute (const AST::Attribute &attribute)
     }
 }
 
-static void
-check_link_section_attribute (const AST::Attribute &attribute)
+void
+link_section (const AST::Attribute &attribute)
 {
   if (!attribute.has_attr_input ())
     {
@@ -231,8 +238,8 @@ check_link_section_attribute (const AST::Attribute &attribute)
     }
 }
 
-static void
-check_export_name_attribute (const AST::Attribute &attribute)
+void
+export_name (const AST::Attribute &attribute)
 {
   if (!attribute.has_attr_input ())
     {
@@ -264,34 +271,131 @@ check_export_name_attribute (const AST::Attribute &attribute)
   rust_error_at (attribute.get_locus (), "attribute must be a string literal");
 }
 
-static void
-check_no_mangle_function (const AST::Attribute &attribute,
-			  const AST::Function &fun)
+void
+lint (const AST::Attribute &attribute)
 {
-  if (attribute.has_attr_input ())
+  if (!attribute.has_attr_input ())
     {
-      rust_error_at (attribute.get_locus (), ErrorCode::E0754,
-		     "malformed %<no_mangle%> attribute input");
+      auto name = attribute.get_path ().as_string ();
+      rust_error_at (attribute.get_locus (), "malformed %qs attribute input",
+		     name.c_str ());
       rust_inform (attribute.get_locus (),
-		   "must be of the form: %<#[no_mangle]%>");
+		   "must be of the form: %<#[%s(lint1, lint2, ...)]%>",
+		   name.c_str ());
     }
-  if (!is_ascii_only (fun.get_function_name ().as_string ()))
-    rust_error_at (fun.get_function_name ().get_locus (),
-		   "the %<#[no_mangle]%> attribute requires ASCII identifier");
 }
 
+void
+link_name (const AST::Attribute &attribute)
+{
+  if (!attribute.has_attr_input ())
+    {
+      rust_error_at (attribute.get_locus (),
+		     "malformed %<link_name%> attribute input");
+      rust_inform (attribute.get_locus (),
+		   "must be of the form: %<#[link_name = \"name\"]%>");
+    }
+}
+
+namespace {
+void
+check_crate_type (const AST::Attribute &attribute)
+{
+  if (!Session::get_instance ().options.is_proc_macro ())
+    {
+      auto name = attribute.get_path ().as_string ();
+
+      rust_error_at (attribute.get_locus (),
+		     "the %<#[%s]%> attribute is only usable with crates of "
+		     "the %<proc-macro%> crate type",
+		     name.c_str ());
+    }
+}
+} // namespace
+
 static void
-check_lint_attribute (const AST::Attribute &attribute, const char *name)
+proc_macro_derive (const AST::Attribute &attribute)
 {
   if (!attribute.has_attr_input ())
     {
+      auto name = attribute.get_path ().as_string ();
       rust_error_at (attribute.get_locus (), "malformed %qs attribute input",
-		     name);
+		     name.c_str ());
+      rust_inform (attribute.get_locus (),
+		   "must be of the form: %<#[proc_macro_derive(TraitName, "
+		   "/*opt*/ attributes(name1, name2, ...))]%>");
+    }
+  check_crate_type (attribute);
+}
+
+static void
+proc_macro (const AST::Attribute &attribute)
+{
+  check_crate_type (attribute);
+}
+
+static void
+target_feature (const AST::Attribute &attribute)
+{
+  if (!attribute.has_attr_input ())
+    {
+      rust_error_at (attribute.get_locus (),
+		     "malformed %<target_feature%> attribute input");
       rust_inform (attribute.get_locus (),
-		   "must be of the form: %<#[%s(lint1, lint2, ...)]%>", name);
+		   "must be of the form: %<#[target_feature(enable = "
+		   "\"name\")]%>");
     }
 }
 
+void
+no_mangle (const AST::Attribute &attribute)
+{
+  if (attribute.has_attr_input ())
+    {
+      rust_error_at (attribute.get_locus (), ErrorCode::E0754,
+		     "malformed %<no_mangle%> attribute input");
+      rust_inform (attribute.get_locus (),
+		   "must be of the form: %<#[no_mangle]%>");
+    }
+}
+
+} // namespace handlers
+
+const std::unordered_map<std::string, std::function<void (AST::Attribute &)>>
+  attribute_checking_handlers
+  = {{Attrs::DOC, handlers::doc},
+     {Attrs::DEPRECATED, handlers::deprecated},
+     {Attrs::LINK_SECTION, handlers::link_section},
+     {Attrs::EXPORT_NAME, handlers::export_name},
+     {Attrs::NO_MANGLE, handlers::no_mangle},
+     {Attrs::ALLOW, handlers::lint},
+     {"deny", handlers::lint},
+     {"warn", handlers::lint},
+     {"forbid", handlers::lint},
+     {Attrs::LINK_NAME, handlers::link_name},
+     {Attrs::PROC_MACRO_DERIVE, handlers::proc_macro_derive},
+     {Attrs::PROC_MACRO, handlers::proc_macro},
+     {Attrs::PROC_MACRO_ATTRIBUTE, handlers::proc_macro},
+     {Attrs::TARGET_FEATURE, handlers::target_feature}};
+
+tl::optional<std::function<void (AST::Attribute &)>>
+lookup_handler (std::string attr_name)
+{
+  auto res = attribute_checking_handlers.find (attr_name);
+  if (res != attribute_checking_handlers.cend ())
+    return res->second;
+  return tl::nullopt;
+}
+
+static void
+check_no_mangle_function (const AST::Attribute &attribute,
+			  const AST::Function &fun)
+{
+  if (!is_ascii_only (fun.get_function_name ().as_string ()))
+    rust_error_at (fun.get_function_name ().get_locus (),
+		   "the %<#[no_mangle]%> attribute requires ASCII identifier");
+}
+
 /**
  * Emit an error when an attribute is attached
  * to an incompatable item type. e.g.:
@@ -302,7 +406,7 @@ check_lint_attribute (const AST::Attribute &attribute, const char *name)
  * Note that "#[derive]" is handled
  * explicitly in rust-derive.cc
  */
-static void
+void
 check_valid_attribute_for_item (const AST::Attribute &attr,
 				const AST::Item &item)
 {
@@ -349,18 +453,9 @@ BuiltinAttributeChecker::visit (AST::Crate &crate)
 void
 BuiltinAttributeChecker::visit (AST::Attribute &attribute)
 {
-  if (auto builtin = lookup_builtin (attribute))
-    {
-      auto result = builtin.value ();
-      // TODO: Add checks here for each builtin attribute
-      // TODO: Have an enum of builtins as well, switching on strings is
-      // annoying and costly
-      if (result.name == Attrs::DOC)
-	check_doc_attribute (attribute);
-      else if (result.name == Attrs::DEPRECATED)
-	check_deprecated_attribute (attribute);
-    }
-
+  lookup_handler (attribute.get_path ().as_string ()).map ([&] (auto handler) {
+    handler (attribute);
+  });
   AST::DefaultASTVisitor::visit (attribute);
 }
 
@@ -385,14 +480,6 @@ BuiltinAttributeChecker::visit (AST::UseDeclaration &declaration)
 void
 BuiltinAttributeChecker::visit (AST::Function &function)
 {
-  auto check_crate_type = [] (const char *name, AST::Attribute &attribute) {
-    if (!Session::get_instance ().options.is_proc_macro ())
-      rust_error_at (attribute.get_locus (),
-		     "the %<#[%s]%> attribute is only usable with crates of "
-		     "the %<proc-macro%> crate type",
-		     name);
-  };
-
   BuiltinAttrDefinition result;
   for (auto &attribute : function.get_outer_attrs ())
     {
@@ -402,37 +489,9 @@ BuiltinAttributeChecker::visit (AST::Function &function)
       if (!result)
 	return;
 
-      auto name = result->name.c_str ();
-
-      if (result->name == Attrs::PROC_MACRO_DERIVE)
-	{
-	  if (!attribute.has_attr_input ())
-	    {
-	      rust_error_at (attribute.get_locus (),
-			     "malformed %qs attribute input", name);
-	      rust_inform (
-		attribute.get_locus (),
-		"must be of the form: %<#[proc_macro_derive(TraitName, "
-		"/*opt*/ attributes(name1, name2, ...))]%>");
-	    }
-	  check_crate_type (name, attribute);
-	}
-      else if (result->name == Attrs::PROC_MACRO
-	       || result->name == Attrs::PROC_MACRO_ATTRIBUTE)
+      if (result->name == Attrs::TARGET_FEATURE)
 	{
-	  check_crate_type (name, attribute);
-	}
-      else if (result->name == Attrs::TARGET_FEATURE)
-	{
-	  if (!attribute.has_attr_input ())
-	    {
-	      rust_error_at (attribute.get_locus (),
-			     "malformed %<target_feature%> attribute input");
-	      rust_inform (attribute.get_locus (),
-			   "must be of the form: %<#[target_feature(enable = "
-			   "\"name\")]%>");
-	    }
-	  else if (!function.get_qualifiers ().is_unsafe ())
+	  if (!function.get_qualifiers ().is_unsafe ())
 	    {
 	      rust_error_at (
 		attribute.get_locus (),
@@ -442,38 +501,7 @@ BuiltinAttributeChecker::visit (AST::Function &function)
 	}
       else if (result->name == Attrs::NO_MANGLE)
 	{
-	  if (attribute.has_attr_input ())
-	    {
-	      rust_error_at (attribute.get_locus (),
-			     "malformed %<no_mangle%> attribute input");
-	      rust_inform (attribute.get_locus (),
-			   "must be of the form: %<#[no_mangle]%>");
-	    }
-	  else
-	    check_no_mangle_function (attribute, function);
-	}
-      else if (result->name == Attrs::EXPORT_NAME)
-	{
-	  check_export_name_attribute (attribute);
-	}
-      else if (result->name == Attrs::ALLOW || result->name == "deny"
-	       || result->name == "warn" || result->name == "forbid")
-	{
-	  check_lint_attribute (attribute, name);
-	}
-      else if (result->name == Attrs::LINK_NAME)
-	{
-	  if (!attribute.has_attr_input ())
-	    {
-	      rust_error_at (attribute.get_locus (),
-			     "malformed %<link_name%> attribute input");
-	      rust_inform (attribute.get_locus (),
-			   "must be of the form: %<#[link_name = \"name\"]%>");
-	    }
-	}
-      else if (result->name == Attrs::LINK_SECTION)
-	{
-	  check_link_section_attribute (attribute);
+	  check_no_mangle_function (attribute, function);
 	}
     }
 
@@ -520,17 +548,7 @@ void
 BuiltinAttributeChecker::visit (AST::StaticItem &item)
 {
   for (auto &attr : item.get_outer_attrs ())
-    {
-      check_valid_attribute_for_item (attr, item);
-
-      if (auto result = lookup_builtin (attr))
-	{
-	  if (result->name == Attrs::LINK_SECTION)
-	    check_link_section_attribute (attr);
-	  else if (result->name == Attrs::EXPORT_NAME)
-	    check_export_name_attribute (attr);
-	}
-    }
+    check_valid_attribute_for_item (attr, item);
 
   AST::DefaultASTVisitor::visit (item);
 }
diff --git a/gcc/rust/checks/errors/rust-builtin-attribute-checker.h b/gcc/rust/checks/errors/rust-builtin-attribute-checker.h
index 957243a1a21..4765e630d35 100644
--- a/gcc/rust/checks/errors/rust-builtin-attribute-checker.h
+++ b/gcc/rust/checks/errors/rust-builtin-attribute-checker.h
@@ -24,6 +24,9 @@
 namespace Rust {
 namespace Analysis {
 
+void check_valid_attribute_for_item (const AST::Attribute &attr,
+				     const AST::Item &item);
+
 class BuiltinAttributeChecker : public AST::DefaultASTVisitor
 {
   using AST::DefaultASTVisitor::visit;
-- 
2.50.1



More information about the Gcc-rust mailing list