From a1e6e87df50aaef1dce280405b59ba2bfa93c0f3 Mon Sep 17 00:00:00 2001
From: Paul Chaignon <paul.chaignon@gmail.com>
Date: Fri, 1 Dec 2023 16:22:30 +0100
Subject: 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>
---
 pkg/ast/ast.go                   |  9 +++++++++
 pkg/compiler/check.go            | 22 ++++++++++++++++++++++
 pkg/compiler/testdata/errors.txt |  3 +++
 3 files changed, 34 insertions(+)

diff --git a/pkg/ast/ast.go b/pkg/ast/ast.go
index 2e8a7a015..2458c1245 100644
--- a/pkg/ast/ast.go
+++ b/pkg/ast/ast.go
@@ -30,6 +30,7 @@ type Node interface {
 type Flags[T FlagValue] interface {
 	SetValues(values []T)
 	GetValues() []T
+	GetPos() Pos
 }
 
 type FlagValue interface {
@@ -152,6 +153,10 @@ func (n *IntFlags) GetValues() []*Int {
 	return n.Values
 }
 
+func (n *IntFlags) GetPos() Pos {
+	return n.Pos
+}
+
 type StrFlags struct {
 	Pos    Pos
 	Name   *Ident
@@ -170,6 +175,10 @@ func (n *StrFlags) GetValues() []*String {
 	return n.Values
 }
 
+func (n *StrFlags) GetPos() Pos {
+	return n.Pos
+}
+
 type TypeDef struct {
 	Pos  Pos
 	Name *Ident
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
-- 
cgit mrf-deployment