From 0cd9066f44caa01a0f9d298491fdd6d4fc850d49 Mon Sep 17 00:00:00 2001
From: joe-conigliaro <joe@vlang.io>
Date: Mon, 15 Jun 2020 22:59:09 +1000
Subject: [PATCH] parser/cgen: multiple attributes & better errors (closes
 #5334)

---
 vlib/v/ast/ast.v                              |   2 +-
 .../checker/tests/fn_attributes_empty_err.out |   5 -
 .../checker/tests/multiple_fn_attributes.out  |   4 -
 .../v/checker/tests/multiple_fn_attributes.vv |   2 -
 vlib/v/gen/cgen.v                             |  10 +-
 vlib/v/gen/fn.v                               | 135 +++++++++---------
 vlib/v/gen/profile.v                          |   2 +-
 vlib/v/parser/fn.v                            |   4 +-
 vlib/v/parser/parser.v                        |  24 ++--
 vlib/v/parser/struct.v                        |   6 +-
 .../fn_attributes_duplicate_multiple.out      |   6 +
 .../tests/fn_attributes_duplicate_multiple.vv |   7 +
 .../tests/fn_attributes_duplicate_single.out  |   5 +
 .../tests/fn_attributes_duplicate_single.vv   |   6 +
 .../parser/tests/fn_attributes_empty_err.out  |   5 +
 .../tests/fn_attributes_empty_err.vv          |   0
 vlib/v/tests/attribute_test.v                 |  19 +++
 17 files changed, 143 insertions(+), 99 deletions(-)
 delete mode 100644 vlib/v/checker/tests/fn_attributes_empty_err.out
 delete mode 100644 vlib/v/checker/tests/multiple_fn_attributes.out
 delete mode 100644 vlib/v/checker/tests/multiple_fn_attributes.vv
 create mode 100644 vlib/v/parser/tests/fn_attributes_duplicate_multiple.out
 create mode 100644 vlib/v/parser/tests/fn_attributes_duplicate_multiple.vv
 create mode 100644 vlib/v/parser/tests/fn_attributes_duplicate_single.out
 create mode 100644 vlib/v/parser/tests/fn_attributes_duplicate_single.vv
 create mode 100644 vlib/v/parser/tests/fn_attributes_empty_err.out
 rename vlib/v/{checker => parser}/tests/fn_attributes_empty_err.vv (100%)

diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v
index abd456400d..3de89ccae8 100644
--- a/vlib/v/ast/ast.v
+++ b/vlib/v/ast/ast.v
@@ -162,7 +162,7 @@ pub:
 	pub_mut_pos int // pub mut:
 	language    table.Language
 	is_union    bool
-	attr        string
+	attrs       []string
 }
 
 pub struct InterfaceDecl {
diff --git a/vlib/v/checker/tests/fn_attributes_empty_err.out b/vlib/v/checker/tests/fn_attributes_empty_err.out
deleted file mode 100644
index 71e37e87d1..0000000000
--- a/vlib/v/checker/tests/fn_attributes_empty_err.out
+++ /dev/null
@@ -1,5 +0,0 @@
-vlib/v/checker/tests/fn_attributes_empty_err.v:1:1: error: attributes cannot be empty
-    1 | [] fn tt() {
-      | ~~
-    2 |     println('text')
-    3 | }
diff --git a/vlib/v/checker/tests/multiple_fn_attributes.out b/vlib/v/checker/tests/multiple_fn_attributes.out
deleted file mode 100644
index b2e2e15528..0000000000
--- a/vlib/v/checker/tests/multiple_fn_attributes.out
+++ /dev/null
@@ -1,4 +0,0 @@
-vlib/v/checker/tests/multiple_fn_attributes.v:1:1: error: multiple attributes detected
-    1 | [inline;deprecated]
-      | ~~~~~~~~~~~~~~~~~~~
-    2 | fn foo(name string) string {}
diff --git a/vlib/v/checker/tests/multiple_fn_attributes.vv b/vlib/v/checker/tests/multiple_fn_attributes.vv
deleted file mode 100644
index 47224e03b9..0000000000
--- a/vlib/v/checker/tests/multiple_fn_attributes.vv
+++ /dev/null
@@ -1,2 +0,0 @@
-[inline;deprecated]
-fn foo(name string) string {}
\ No newline at end of file
diff --git a/vlib/v/gen/cgen.v b/vlib/v/gen/cgen.v
index e4dcaefe54..d12c3b6acf 100644
--- a/vlib/v/gen/cgen.v
+++ b/vlib/v/gen/cgen.v
@@ -98,7 +98,7 @@ mut:
 	is_json_fn           bool // inside json.encode()
 	json_types           []string // to avoid json gen duplicates
 	pcs                  []ProfileCounterMeta // -prof profile counter fn_names => fn counter name
-	attr                 string
+	attrs                []string // attributes before next decl stmt
 	is_builtin_mod       bool
 	hotcode_fn_names     []string
 	fn_main              &ast.FnDecl // the FnDecl of the main function. Needed in order to generate the main function code *last*
@@ -573,6 +573,10 @@ fn (mut g Gen) stmts(stmts []ast.Stmt) {
 		if g.inside_ternary > 0 && i < stmts.len - 1 {
 			g.write(',')
 		}
+		// clear attrs on next non Attr stmt
+		if stmt !is ast.Attr {
+			g.attrs = []
+		}
 	}
 	g.indent--
 	if g.inside_ternary > 0 {
@@ -593,7 +597,7 @@ fn (mut g Gen) stmt(node ast.Stmt) {
 			g.gen_assign_stmt(it)
 		}
 		ast.Attr {
-			g.attr = it.name
+			g.attrs << it.name
 			g.writeln('// Attr: [$it.name]')
 		}
 		ast.Block {
@@ -685,8 +689,6 @@ fn (mut g Gen) stmt(node ast.Stmt) {
 			if it.language != .c {
 				g.writeln('')
 			}
-			// g.attr has to be reset after each function
-			g.attr = ''
 		}
 		ast.ForCStmt {
 			g.write('for (')
diff --git a/vlib/v/gen/fn.v b/vlib/v/gen/fn.v
index 86f382c29a..f89fb8def2 100644
--- a/vlib/v/gen/fn.v
+++ b/vlib/v/gen/fn.v
@@ -38,9 +38,9 @@ fn (mut g Gen) gen_fn_decl(it ast.FnDecl) {
 		return
 	}
 	fn_start_pos := g.out.len
-	msvc_attrs := g.write_fn_attr()
+	msvc_attrs := g.write_fn_attrs()
 	// Live
-	is_livefn := g.attr == 'live'
+	is_livefn := 'live' in g.attrs
 	is_livemain := g.pref.is_livemain && is_livefn
 	is_liveshared := g.pref.is_liveshared && is_livefn
 	is_livemode := g.pref.is_livemain || g.pref.is_liveshared
@@ -703,78 +703,79 @@ fn (g &Gen) fileis(s string) bool {
 	return g.file.path.contains(s)
 }
 
-fn (mut g Gen) write_fn_attr() string{
+fn (mut g Gen) write_fn_attrs() string{
 	mut msvc_attrs := ''
-	match g.attr {
-		'inline' {
-			g.write('inline ')
-		}
-		// since these are supported by GCC, clang and MSVC, we can consider them officially supported.
-		'no_inline' {
-			g.write('__NOINLINE ')
-		}
-		'irq_handler' {
-			g.write('__IRQHANDLER ')
-		}
+	for attr in g.attrs {
+		match attr {
+			'inline' {
+				g.write('inline ')
+			}
+			// since these are supported by GCC, clang and MSVC, we can consider them officially supported.
+			'no_inline' {
+				g.write('__NOINLINE ')
+			}
+			'irq_handler' {
+				g.write('__IRQHANDLER ')
+			}
 
-		// GCC/clang attributes
-		// prefixed by _ to indicate they're for advanced users only and not really supported by V.
-		// source for descriptions: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes
+			// GCC/clang attributes
+			// prefixed by _ to indicate they're for advanced users only and not really supported by V.
+			// source for descriptions: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes
 
-		// The cold attribute on functions is used to inform the compiler that the function is unlikely
-		// to be executed. The function is optimized for size rather than speed and on many targets it
-		// is placed into a special subsection of the text section so all cold functions appear close
-		// together, improving code locality of non-cold parts of program.
-		'_cold' {
-			g.write('__attribute__((cold)) ')
-		}
-		// The constructor attribute causes the function to be called automatically before execution
-		// enters main ().
-		'_constructor' {
-			g.write('__attribute__((constructor)) ')
-		}
-		// The destructor attribute causes the function to be called automatically after main ()
-		// completes or exit () is called.
-		'_destructor' {
-			g.write('__attribute__((destructor)) ')
-		}
-		// Generally, inlining into a function is limited. For a function marked with this attribute,
-		// every call inside this function is inlined, if possible.
-		'_flatten' {
-			g.write('__attribute__((flatten)) ')
-		}
-		// The hot attribute on a function is used to inform the compiler that the function is a hot
-		// spot of the compiled program.
-		'_hot' {
-			g.write('__attribute__((hot)) ')
-		}
-		// This tells the compiler that a function is malloc-like, i.e., that the pointer P returned by
-		// the function cannot alias any other pointer valid when the function returns, and moreover no
-		// pointers to valid objects occur in any storage addressed by P.
-		'_malloc' {
-			g.write('__attribute__((malloc)) ')
-		}
+			// The cold attribute on functions is used to inform the compiler that the function is unlikely
+			// to be executed. The function is optimized for size rather than speed and on many targets it
+			// is placed into a special subsection of the text section so all cold functions appear close
+			// together, improving code locality of non-cold parts of program.
+			'_cold' {
+				g.write('__attribute__((cold)) ')
+			}
+			// The constructor attribute causes the function to be called automatically before execution
+			// enters main ().
+			'_constructor' {
+				g.write('__attribute__((constructor)) ')
+			}
+			// The destructor attribute causes the function to be called automatically after main ()
+			// completes or exit () is called.
+			'_destructor' {
+				g.write('__attribute__((destructor)) ')
+			}
+			// Generally, inlining into a function is limited. For a function marked with this attribute,
+			// every call inside this function is inlined, if possible.
+			'_flatten' {
+				g.write('__attribute__((flatten)) ')
+			}
+			// The hot attribute on a function is used to inform the compiler that the function is a hot
+			// spot of the compiled program.
+			'_hot' {
+				g.write('__attribute__((hot)) ')
+			}
+			// This tells the compiler that a function is malloc-like, i.e., that the pointer P returned by
+			// the function cannot alias any other pointer valid when the function returns, and moreover no
+			// pointers to valid objects occur in any storage addressed by P.
+			'_malloc' {
+				g.write('__attribute__((malloc)) ')
+			}
 
-		// Calls to functions whose return value is not affected by changes to the observable state
-		// of the program and that have no observable effects on such state other than to return a
-		// value may lend themselves to optimizations such as common subexpression elimination.
-		// Declaring such functions with the const attribute allows GCC to avoid emitting some calls in
-		// repeated invocations of the function with the same argument values.
-		'_pure' {
-			g.write('__attribute__((const)) ')
-		}
+			// Calls to functions whose return value is not affected by changes to the observable state
+			// of the program and that have no observable effects on such state other than to return a
+			// value may lend themselves to optimizations such as common subexpression elimination.
+			// Declaring such functions with the const attribute allows GCC to avoid emitting some calls in
+			// repeated invocations of the function with the same argument values.
+			'_pure' {
+				g.write('__attribute__((const)) ')
+			}
 
-		// windows attributes (msvc/mingw)
-		// prefixed by windows to indicate they're for advanced users only and not really supported by V.
+			// windows attributes (msvc/mingw)
+			// prefixed by windows to indicate they're for advanced users only and not really supported by V.
 
-		'windows_stdcall' {
-			msvc_attrs += '__stdcall '
-		}
+			'windows_stdcall' {
+				msvc_attrs += '__stdcall '
+			}
 
-		else {
-			// nothing but keep V happy
+			else {
+				// nothing but keep V happy
+			}
 		}
 	}
 	return msvc_attrs
-
-	}
+}
diff --git a/vlib/v/gen/profile.v b/vlib/v/gen/profile.v
index 945dde9e18..d4a533bf97 100644
--- a/vlib/v/gen/profile.v
+++ b/vlib/v/gen/profile.v
@@ -12,7 +12,7 @@ fn (mut g Gen) profile_fn(fn_name string, is_main bool){
 		g.writeln('\tatexit(vprint_profile_stats);')
 		g.writeln('')
 	}
-	if g.pref.profile_no_inline && g.attr == 'inline' {
+	if g.pref.profile_no_inline && 'inline' in g.attrs {
 		g.defer_profile_code = ''
 		return        
 	}
diff --git a/vlib/v/parser/fn.v b/vlib/v/parser/fn.v
index b71b5324a6..1cb31e9568 100644
--- a/vlib/v/parser/fn.v
+++ b/vlib/v/parser/fn.v
@@ -119,7 +119,7 @@ pub fn (mut p Parser) call_args() []ast.CallArg {
 fn (mut p Parser) fn_decl() ast.FnDecl {
 	p.top_level_statement_start()
 	start_pos := p.tok.position()
-	is_deprecated := p.attr == 'deprecated'
+	is_deprecated := 'deprecated' in p.attrs
 	is_pub := p.tok.kind == .key_pub
 	if is_pub {
 		p.next()
@@ -273,8 +273,6 @@ fn (mut p Parser) fn_decl() ast.FnDecl {
 		stmts = p.parse_block_no_scope(true)
 	}
 	p.close_scope()
-	p.attr = ''
-	p.attr_ctdefine = ''
 	return ast.FnDecl{
 		name: name
 		mod: p.mod
diff --git a/vlib/v/parser/parser.v b/vlib/v/parser/parser.v
index 5245a88702..2fe552755e 100644
--- a/vlib/v/parser/parser.v
+++ b/vlib/v/parser/parser.v
@@ -36,7 +36,7 @@ mut:
 	pref              &pref.Preferences
 	builtin_mod       bool // are we in the `builtin` module?
 	mod               string // current module name
-	attr              string
+	attrs             []string // attributes before next decl stmt
 	attr_ctdefine     string
 	expr_mod          string // for constructing full type names in parse_type()
 	scope             &ast.Scope
@@ -154,7 +154,13 @@ fn (mut p Parser) parse() ast.File {
 			break
 		}
 		// println('stmt at ' + p.tok.str())
-		stmts << p.top_stmt()
+		stmt := p.top_stmt()
+		// clear the attribtes at the end of next non Attr top level stmt
+		if stmt !is ast.Attr {
+			p.attrs = []
+			p.attr_ctdefine = ''
+		}
+		stmts << stmt
 	}
 	// println('nr stmts = $stmts.len')
 	// println(stmts[0])
@@ -409,10 +415,7 @@ pub fn (mut p Parser) top_stmt() ast.Stmt {
 		.lsbr {
 			start_pos := p.tok.position()
 			attrs := p.attributes()
-			if attrs.len > 1 {
-				end_pos := p.tok.position()
-				p.error_with_pos('multiple attributes detected', start_pos.extend(end_pos))
-			} else if attrs.len == 0 {
+			if attrs.len == 0 {
 				end_pos := p.tok.position()
 				p.error_with_pos('attributes cannot be empty', start_pos.extend(end_pos))
 			}
@@ -634,6 +637,7 @@ fn (mut p Parser) attributes() []ast.Attr {
 }
 
 fn (mut p Parser) parse_attr() ast.Attr {
+	start_pos := p.prev_tok.position()
 	mut is_if_attr := false
 	if p.tok.kind == .key_if {
 		p.next()
@@ -650,7 +654,10 @@ fn (mut p Parser) parse_attr() ast.Attr {
 			p.next()
 		}
 	}
-	p.attr = name
+	if name in p.attrs {
+		p.error_with_pos('duplicate attribute `$name`', start_pos.extend(p.tok.position()))
+	}
+	p.attrs << name
 	if is_if_attr {
 		p.attr_ctdefine = name
 	}
@@ -1460,8 +1467,7 @@ fn (mut p Parser) enum_decl() ast.EnumDecl {
 	}
 	p.top_level_statement_end()
 	p.check(.rcbr)
-	attr := p.attr
-	is_flag := attr == 'flag'
+	is_flag := 'flag' in p.attrs
 	if is_flag {
 		if fields.len > 32 {
 			p.error('when an enum is used as bit field, it must have a max of 32 fields')
diff --git a/vlib/v/parser/struct.v b/vlib/v/parser/struct.v
index 9b73099426..8b5a9d5389 100644
--- a/vlib/v/parser/struct.v
+++ b/vlib/v/parser/struct.v
@@ -32,7 +32,7 @@ fn (mut p Parser) struct_decl() ast.StructDecl {
 		p.next() // C || JS
 		p.next() // .
 	}
-	is_typedef := p.attr == 'typedef'
+	is_typedef := 'typedef' in p.attrs
 	no_body := p.peek_tok.kind != .lcbr
 	if language == .v && no_body {
 		p.error('`$p.tok.lit` lacks body')
@@ -180,7 +180,7 @@ fn (mut p Parser) struct_decl() ast.StructDecl {
 			fields: fields
 			is_typedef: is_typedef
 			is_union: is_union
-			is_ref_only: p.attr == 'ref_only'
+			is_ref_only: 'ref_only' in p.attrs
 		}
 		mod: p.mod
 		is_public: is_pub
@@ -208,7 +208,7 @@ fn (mut p Parser) struct_decl() ast.StructDecl {
 		pub_mut_pos: pub_mut_pos
 		language: language
 		is_union: is_union
-		attr: p.attr
+		attrs: p.attrs
 	}
 }
 
diff --git a/vlib/v/parser/tests/fn_attributes_duplicate_multiple.out b/vlib/v/parser/tests/fn_attributes_duplicate_multiple.out
new file mode 100644
index 0000000000..db1d899ca9
--- /dev/null
+++ b/vlib/v/parser/tests/fn_attributes_duplicate_multiple.out
@@ -0,0 +1,6 @@
+vlib/v/parser/tests/fn_attributes_duplicate_multiple.v:2:1: error: duplicate attribute `inline`
+    1 | [inline]
+    2 | [inline]
+      | ~~~~~~~~
+    3 | fn foo() {}
+    4 |
diff --git a/vlib/v/parser/tests/fn_attributes_duplicate_multiple.vv b/vlib/v/parser/tests/fn_attributes_duplicate_multiple.vv
new file mode 100644
index 0000000000..2e1f223779
--- /dev/null
+++ b/vlib/v/parser/tests/fn_attributes_duplicate_multiple.vv
@@ -0,0 +1,7 @@
+[inline]
+[inline]
+fn foo() {}
+
+fn main() {
+    foo()
+}
\ No newline at end of file
diff --git a/vlib/v/parser/tests/fn_attributes_duplicate_single.out b/vlib/v/parser/tests/fn_attributes_duplicate_single.out
new file mode 100644
index 0000000000..69a413edc4
--- /dev/null
+++ b/vlib/v/parser/tests/fn_attributes_duplicate_single.out
@@ -0,0 +1,5 @@
+vlib/v/parser/tests/fn_attributes_duplicate_single.v:1:8: error: duplicate attribute `inline`
+    1 | [inline; inline]
+      |        ~~~~~~~~~
+    2 | fn foo() {}
+    3 |
diff --git a/vlib/v/parser/tests/fn_attributes_duplicate_single.vv b/vlib/v/parser/tests/fn_attributes_duplicate_single.vv
new file mode 100644
index 0000000000..27d6f55288
--- /dev/null
+++ b/vlib/v/parser/tests/fn_attributes_duplicate_single.vv
@@ -0,0 +1,6 @@
+[inline; inline]
+fn foo() {}
+
+fn main() {
+    foo()
+}
\ No newline at end of file
diff --git a/vlib/v/parser/tests/fn_attributes_empty_err.out b/vlib/v/parser/tests/fn_attributes_empty_err.out
new file mode 100644
index 0000000000..c4a1c874d6
--- /dev/null
+++ b/vlib/v/parser/tests/fn_attributes_empty_err.out
@@ -0,0 +1,5 @@
+vlib/v/parser/tests/fn_attributes_empty_err.v:1:1: error: attributes cannot be empty
+    1 | [] fn tt() {
+      | ~~
+    2 |     println('text')
+    3 | }
diff --git a/vlib/v/checker/tests/fn_attributes_empty_err.vv b/vlib/v/parser/tests/fn_attributes_empty_err.vv
similarity index 100%
rename from vlib/v/checker/tests/fn_attributes_empty_err.vv
rename to vlib/v/parser/tests/fn_attributes_empty_err.vv
diff --git a/vlib/v/tests/attribute_test.v b/vlib/v/tests/attribute_test.v
index 9fb28d1adc..382bdda840 100644
--- a/vlib/v/tests/attribute_test.v
+++ b/vlib/v/tests/attribute_test.v
@@ -22,6 +22,14 @@ pub enum PubEnumAttrTest {
 	two
 }
 
+
+[attrone]
+[attrtwo]
+pub enum EnumMultiAttrTest {
+	one
+	two
+}
+
 [testing]
 fn test_fn_attribute() {
 	assert true
@@ -31,3 +39,14 @@ fn test_fn_attribute() {
 pub fn test_pub_fn_attribute() {
 	assert true
 }
+
+[attrone]
+[attrtwo]
+fn test_fn_multi_attribute() {
+	assert true
+}
+
+[attrone; attrtwo]
+fn test_fn_multi_attribute_single() {
+	assert true
+}