From 9da428dad8eefa8a821214bc0fe0d4159ba4efed Mon Sep 17 00:00:00 2001
From: Austin Bonander <austin.bonander@gmail.com>
Date: Wed, 3 Oct 2018 14:24:31 -0700
Subject: [PATCH] make `Parser::parse_foreign_item()` return a foreign item or
 error

closes #54441
---
 src/libsyntax/ext/expand.rs                 |  4 +---
 src/libsyntax/parse/parser.rs               | 23 ++++++++++-----------
 src/test/parse-fail/duplicate-visibility.rs |  2 +-
 src/test/ui/macros/issue-54441.rs           | 13 ++++++++++++
 src/test/ui/macros/issue-54441.stderr       | 14 +++++++++++++
 5 files changed, 40 insertions(+), 16 deletions(-)
 create mode 100644 src/test/ui/macros/issue-54441.rs
 create mode 100644 src/test/ui/macros/issue-54441.stderr

diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
index 52322e98d46f9..ba70108ea471d 100644
--- a/src/libsyntax/ext/expand.rs
+++ b/src/libsyntax/ext/expand.rs
@@ -1008,9 +1008,7 @@ impl<'a> Parser<'a> {
             AstFragmentKind::ForeignItems => {
                 let mut items = SmallVec::new();
                 while self.token != token::Eof {
-                    if let Some(item) = self.parse_foreign_item()? {
-                        items.push(item);
-                    }
+                    items.push(self.parse_foreign_item()?);
                 }
                 AstFragment::ForeignItems(items)
             }
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 5571a18b59625..d75a3f3fe9bdc 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -6718,10 +6718,9 @@ impl<'a> Parser<'a> {
         attrs.extend(self.parse_inner_attributes()?);
 
         let mut foreign_items = vec![];
-        while let Some(item) = self.parse_foreign_item()? {
-            foreign_items.push(item);
+        while !self.eat(&token::CloseDelim(token::Brace)) {
+            foreign_items.push(self.parse_foreign_item()?);
         }
-        self.expect(&token::CloseDelim(token::Brace))?;
 
         let prev_span = self.prev_span;
         let m = ast::ForeignMod {
@@ -7305,8 +7304,8 @@ impl<'a> Parser<'a> {
     }
 
     /// Parse a foreign item.
-    crate fn parse_foreign_item(&mut self) -> PResult<'a, Option<ForeignItem>> {
-        maybe_whole!(self, NtForeignItem, |ni| Some(ni));
+    crate fn parse_foreign_item(&mut self) -> PResult<'a, ForeignItem> {
+        maybe_whole!(self, NtForeignItem, |ni| ni);
 
         let attrs = self.parse_outer_attributes()?;
         let lo = self.span;
@@ -7326,20 +7325,20 @@ impl<'a> Parser<'a> {
                     ).emit();
             }
             self.bump(); // `static` or `const`
-            return Ok(Some(self.parse_item_foreign_static(visibility, lo, attrs)?));
+            return Ok(self.parse_item_foreign_static(visibility, lo, attrs)?);
         }
         // FOREIGN FUNCTION ITEM
         if self.check_keyword(keywords::Fn) {
-            return Ok(Some(self.parse_item_foreign_fn(visibility, lo, attrs)?));
+            return Ok(self.parse_item_foreign_fn(visibility, lo, attrs)?);
         }
         // FOREIGN TYPE ITEM
         if self.check_keyword(keywords::Type) {
-            return Ok(Some(self.parse_item_foreign_type(visibility, lo, attrs)?));
+            return Ok(self.parse_item_foreign_type(visibility, lo, attrs)?);
         }
 
         match self.parse_assoc_macro_invoc("extern", Some(&visibility), &mut false)? {
             Some(mac) => {
-                Ok(Some(
+                Ok(
                     ForeignItem {
                         ident: keywords::Invalid.ident(),
                         span: lo.to(self.prev_span),
@@ -7348,14 +7347,14 @@ impl<'a> Parser<'a> {
                         vis: visibility,
                         node: ForeignItemKind::Macro(mac),
                     }
-                ))
+                )
             }
             None => {
-                if !attrs.is_empty() {
+                if !attrs.is_empty()  {
                     self.expected_item_err(&attrs);
                 }
 
-                Ok(None)
+                self.unexpected()
             }
         }
     }
diff --git a/src/test/parse-fail/duplicate-visibility.rs b/src/test/parse-fail/duplicate-visibility.rs
index 6899caa7153bb..cc3286fe70512 100644
--- a/src/test/parse-fail/duplicate-visibility.rs
+++ b/src/test/parse-fail/duplicate-visibility.rs
@@ -10,7 +10,7 @@
 
 // compile-flags: -Z parse-only
 
-// error-pattern:expected one of `(`, `fn`, `static`, `type`, or `}` here
+// error-pattern:expected one of `(`, `fn`, `static`, or `type`
 extern {
     pub pub fn foo();
 }
diff --git a/src/test/ui/macros/issue-54441.rs b/src/test/ui/macros/issue-54441.rs
new file mode 100644
index 0000000000000..b45aedb549ecd
--- /dev/null
+++ b/src/test/ui/macros/issue-54441.rs
@@ -0,0 +1,13 @@
+#![feature(macros_in_extern)]
+
+macro_rules! m {
+    () => {
+        let //~ ERROR expected
+    };
+}
+
+extern "C" {
+    m!();
+}
+
+fn main() {}
diff --git a/src/test/ui/macros/issue-54441.stderr b/src/test/ui/macros/issue-54441.stderr
new file mode 100644
index 0000000000000..aa1edb2cf893f
--- /dev/null
+++ b/src/test/ui/macros/issue-54441.stderr
@@ -0,0 +1,14 @@
+error: expected one of `crate`, `fn`, `pub`, `static`, or `type`, found `let`
+  --> $DIR/issue-54441.rs:5:9
+   |
+LL | #![feature(macros_in_extern)]
+   | - expected one of `crate`, `fn`, `pub`, `static`, or `type` here
+...
+LL |         let //~ ERROR expected
+   |         ^^^ unexpected token
+...
+LL |     m!();
+   |     ----- in this macro invocation
+
+error: aborting due to previous error
+