Detect macros with duplicate metavariable bindings

This is imperfect, as invalid macro definitions should be detected
regardless of whether a macro is used or not.

gcc/rust/ChangeLog:

	* expand/rust-macro-expand.cc
	(MacroExpander::expand_decl_macro): Prevent excess errors.
	(MacroExpander::match_matcher): Detect duplicate metavariable
	names.
	* expand/rust-macro-expand.h (MacroExpander::MacroExpander):
	Initialize field had_duplicate_error.
	(MacroExpander::had_duplicate_error): New field.

gcc/testsuite/ChangeLog:

	* rust/compile/macros/mbe/macro-duplicate-binding.rs: New test.

Signed-off-by: Owen Avery <[email protected]>

Author: powerboat9

gcc/rust/expand/rust-macro-expand.cc

@@ -112,9 +112,13 @@ MacroExpander::expand_decl_macro (location_t invoc_locus,
 
   if (matched_rule == nullptr)
     {
-      rich_location r (line_table, invoc_locus);
-      r.add_range (rules_def.get_locus ());
-      rust_error_at (r, "Failed to match any rule within macro");
+      if (!had_duplicate_error)
+	{
+	  rich_location r (line_table, invoc_locus);
+	  r.add_range (rules_def.get_locus ());
+	  rust_error_at (r, "Failed to match any rule within macro");
+	}
+      had_duplicate_error = false;
       return AST::Fragment::create_error ();
     }
 
@@ -535,6 +539,8 @@ MacroExpander::match_matcher (Parser<MacroInvocLexer> &parser,
 
   const MacroInvocLexer &source = parser.get_token_source ();
 
+  std::unordered_map<std::string, location_t> duplicate_check;
+
   for (auto &match : matcher.get_matches ())
     {
       size_t offs_begin = source.get_offs ();
@@ -548,6 +554,21 @@ MacroExpander::match_matcher (Parser<MacroInvocLexer> &parser,
 	    if (!match_fragment (parser, *fragment))
 	      return false;
 
+	    auto duplicate_result = duplicate_check.insert (
+	      std::make_pair (fragment->get_ident ().as_string (),
+			      fragment->get_ident ().get_locus ()));
+
+	    if (!duplicate_result.second)
+	      {
+		// TODO: add range labels?
+		rich_location r (line_table,
+				 fragment->get_ident ().get_locus ());
+		r.add_range (duplicate_result.first->second);
+		rust_error_at (r, "duplicate matcher binding");
+		had_duplicate_error = true;
+		return false;
+	      }
+
 	    // matched fragment get the offset in the token stream
 	    size_t offs_end = source.get_offs ();
 	    sub_stack.insert_metavar (

gcc/rust/expand/rust-macro-expand.h

@@ -300,7 +300,8 @@ struct MacroExpander
     : cfg (cfg), crate (crate), session (session),
       sub_stack (SubstitutionScope ()),
       expanded_fragment (AST::Fragment::create_error ()),
-      has_changed_flag (false), resolver (Resolver::Resolver::get ()),
+      has_changed_flag (false), had_duplicate_error (false),
+      resolver (Resolver::Resolver::get ()),
       mappings (Analysis::Mappings::get ())
   {}
 
@@ -512,6 +513,9 @@ struct MacroExpander
   tl::optional<AST::MacroRulesDefinition &> last_def;
   tl::optional<AST::MacroInvocation &> last_invoc;
 
+  // used to avoid emitting excess errors
+  bool had_duplicate_error;
+
 public:
   Resolver::Resolver *resolver;
   Analysis::Mappings &mappings;

gcc/testsuite/rust/compile/macros/mbe/macro-duplicate-binding.rs

@@ -0,0 +1,7 @@
+macro_rules! foo {
+    ($a:ident, $a:ident) => {0} // { dg-error "duplicate matcher binding" }
+}
+
+fn main() {
+    foo!(a, b);
+}