compiler: require nested flags to be at the end of the list

This commit adds the requirement that nested flags must be at the end of
the list of values. For example,

    flags1 = 1, 2, 3, 4, flags2

flags2 cannot be moved to another position in the list. The goal is to
simplify parsing of the list by humans.

Enforcing that the nested flags be at the end (vs. the beginning) makes
things a bit easier for the parser. If we enforced that they should be
at the beginning, then the parser would need to look further forward to
determine if a flags definition is an integer flags or a string flags.

    flags1 = flags2, flags3, flags4, 5, 6

In this example, the parser would need to look to the 4th value in the
list to tell that it's an integer flags.

Suggested-by: Aleksandr Nogikh <nogikh@google.com>
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>

2 files changed, 25 insertions, 0 deletions

diff --git a/pkg/compiler/check.go b/pkg/compiler/check.go
index b5ced7a13..00695b6b1 100644
--- a/pkg/compiler/check.go
+++ b/pkg/compiler/check.go
@@ -20,6 +20,7 @@ func (comp *compiler) typecheck() {
 	comp.checkComments()
 	comp.checkDirectives()
 	comp.checkNames()
+	comp.checkFlags()
 	comp.checkFields()
 	comp.checkTypedefs()
 	comp.checkTypes()
@@ -156,6 +157,27 @@ func (comp *compiler) checkNames() {
 	}
 }
 
+func (comp *compiler) checkFlags() {
+	checkFlagsGeneric[*ast.IntFlags, *ast.Int](comp, comp.intFlags)
+	checkFlagsGeneric[*ast.StrFlags, *ast.String](comp, comp.strFlags)
+}
+
+func checkFlagsGeneric[F ast.Flags[V], V ast.FlagValue](comp *compiler, allFlags map[string]F) {
+	for name, flags := range allFlags {
+		inConstIdent := true
+		for _, val := range flags.GetValues() {
+			if _, ok := allFlags[val.GetName()]; ok {
+				inConstIdent = false
+			} else {
+				if !inConstIdent {
+					comp.error(flags.GetPos(), "flags identifier not at the end in %v definition", name)
+					break
+				}
+			}
+		}
+	}
+}
+
 func (comp *compiler) checkFields() {
 	for _, decl := range comp.desc.Nodes {
 		switch n := decl.(type) {
diff --git a/pkg/compiler/testdata/errors.txt b/pkg/compiler/testdata/errors.txt
index 32c673f21..1be1bd070 100644
--- a/pkg/compiler/testdata/errors.txt
+++ b/pkg/compiler/testdata/errors.txt
@@ -103,6 +103,9 @@ f130 = 100, 110, f110		### flags f130 used twice or circular dependency on f130
 
 loop_flags(a flags[f110])
 
+f200 = f300, 200, 201		### flags identifier not at the end in f200 definition
+f300 = 300, 301
+
 f210 = "1", "2", "3", f220	### flags f210 used twice or circular dependency on f210
 f220 = "10", "11", f230		### flags f220 used twice or circular dependency on f220
 f230 = "100", "110", f210	### flags f230 used twice or circular dependency on f230