From 14cc59eef8374ac8013a05d5d14c4cd4af9d0979 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Thu, 6 Jan 2022 18:04:58 +0100 Subject: pkg/compiler: prohibit use of direction attribute on union fields Direction attributes on unions work in a confusing way and don't do what users may think they do. Now we have out_overlay attribute for structs that allows to have overlapping input and output fields. --- pkg/compiler/attrs.go | 8 ++++---- pkg/compiler/check.go | 12 ++++++------ pkg/compiler/gen.go | 4 ++-- pkg/compiler/testdata/errors.txt | 5 +++++ pkg/compiler/testdata/errors2.txt | 10 ++-------- 5 files changed, 19 insertions(+), 20 deletions(-) (limited to 'pkg/compiler') diff --git a/pkg/compiler/attrs.go b/pkg/compiler/attrs.go index 63ba613d9..46ba89b23 100644 --- a/pkg/compiler/attrs.go +++ b/pkg/compiler/attrs.go @@ -28,10 +28,10 @@ var ( attrInOut = &attrDesc{Name: "inout"} attrOutOverlay = &attrDesc{Name: "out_overlay"} - structAttrs = makeAttrs(attrPacked, attrSize, attrAlign) - unionAttrs = makeAttrs(attrVarlen, attrSize) - fieldAttrs = makeAttrs(attrIn, attrOut, attrInOut, attrOutOverlay) - callAttrs = make(map[string]*attrDesc) + structAttrs = makeAttrs(attrPacked, attrSize, attrAlign) + unionAttrs = makeAttrs(attrVarlen, attrSize) + structFieldAttrs = makeAttrs(attrIn, attrOut, attrInOut, attrOutOverlay) + callAttrs = make(map[string]*attrDesc) ) func init() { diff --git a/pkg/compiler/check.go b/pkg/compiler/check.go index 676b96c5c..9f74f8a4b 100644 --- a/pkg/compiler/check.go +++ b/pkg/compiler/check.go @@ -184,7 +184,11 @@ func (comp *compiler) checkStructFields(n *ast.Struct, typ, name string) { } hasDirections, hasOutOverlay := false, false for fieldIdx, f := range n.Fields { - attrs := comp.parseAttrs(fieldAttrs, f, f.Attrs) + if n.IsUnion { + comp.parseAttrs(nil, f, f.Attrs) + continue + } + attrs := comp.parseAttrs(structFieldAttrs, f, f.Attrs) dirCount := attrs[attrIn] + attrs[attrOut] + attrs[attrInOut] if dirCount != 0 { hasDirections = true @@ -194,10 +198,6 @@ func (comp *compiler) checkStructFields(n *ast.Struct, typ, name string) { comp.error(f.Pos, "%v has multiple direction attributes", typ) } if attrs[attrOutOverlay] > 0 { - if n.IsUnion { - _, typ, name := f.Info() - comp.error(f.Pos, "unknown %v %v attribute %v", typ, name, attrOutOverlay.Name) - } if fieldIdx == 0 { comp.error(f.Pos, "%v attribute must not be specified on the first field", attrOutOverlay.Name) } @@ -325,7 +325,7 @@ func (comp *compiler) checkAttributeValues() { for _, f := range st.Fields { isOut := hasOutOverlay for _, attr := range f.Attrs { - desc := fieldAttrs[attr.Ident] + desc := structFieldAttrs[attr.Ident] if desc.CheckConsts != nil { desc.CheckConsts(comp, f, attr) } diff --git a/pkg/compiler/gen.go b/pkg/compiler/gen.go index 78cbc54fe..89571c966 100644 --- a/pkg/compiler/gen.go +++ b/pkg/compiler/gen.go @@ -456,7 +456,7 @@ func genPad(size uint64) prog.Field { func (comp *compiler) genFieldArray(fields []*ast.Field, argSizes []uint64) ([]prog.Field, int) { outOverlay := -1 for i, f := range fields { - attrs := comp.parseAttrs(fieldAttrs, f, f.Attrs) + attrs := comp.parseAttrs(structFieldAttrs, f, f.Attrs) if attrs[attrOutOverlay] > 0 { outOverlay = i } @@ -476,7 +476,7 @@ func (comp *compiler) genFieldArray(fields []*ast.Field, argSizes []uint64) ([]p } func (comp *compiler) genFieldDir(f *ast.Field) (prog.Dir, bool) { - attrs := comp.parseAttrs(fieldAttrs, f, f.Attrs) + attrs := comp.parseAttrs(structFieldAttrs, f, f.Attrs) switch { case attrs[attrIn] != 0: return prog.DirIn, true diff --git a/pkg/compiler/testdata/errors.txt b/pkg/compiler/testdata/errors.txt index 6cc2e2b0f..6d258cf50 100644 --- a/pkg/compiler/testdata/errors.txt +++ b/pkg/compiler/testdata/errors.txt @@ -412,3 +412,8 @@ union$overlay0 [ f0 int32 f1 int32 (out_overlay) ### unknown arg/field f1 attribute out_overlay ] + +union$directions [ + f1 int32 (in) ### unknown arg/field f1 attribute in + f2 int32 (out) ### unknown arg/field f2 attribute out +] diff --git a/pkg/compiler/testdata/errors2.txt b/pkg/compiler/testdata/errors2.txt index e5a51b4a9..c07e6a8a1 100644 --- a/pkg/compiler/testdata/errors2.txt +++ b/pkg/compiler/testdata/errors2.txt @@ -189,13 +189,12 @@ resource r117[int8] ### resource r117 is never used as an input(such resources resource r118[int8] ### resource r118 is never used as an input(such resources are not useful) resource r119[int8] ### resource r119 is never used as an input(such resources are not useful) resource r120[int8] ### resource r120 can't be created (never mentioned as a syscall return value or output argument/field) -resource r121[int8] foo$300(a0 r100, a1 r101, a2 r102, a3 r103, a4 r104, a5 r105, a6 r106, a7 r107, a8 r108) -foo$301(a0 r109, a1 r110, a2 r111, a3 r112, a4 r113, a5 r114, a6 r115, a7 r120, a8 r121) +foo$301(a0 r109, a1 r110, a2 r111, a3 r112, a4 r113, a5 r114, a6 r115, a7 r120) foo$302(a ptr[out, array[r103]], b ptr[in, s300], c r107) r104 foo$303(a ptr[in, s302], b ptr[in, s303], c ptr[in, s304], d ptr[out, s305], e ptr[inout, s306], f ptr[inout, s307], g ptr[in, s308], h ptr[out, s310]) -foo$304(a ptr[out, r117], b ptr[in, s312], c ptr[in, s313], d ptr[inout, u100]) r116 +foo$304(a ptr[out, r117], b ptr[in, s312], c ptr[in, s313]) r116 s300 { f1 ptr[inout, s301] @@ -267,11 +266,6 @@ s314 { f1 r119 (out) } -u100 [ - f1 r120 (in) - f2 r121 (out) -] - # TODO: Two instances of the same resource might exist in the same structure as # both in and out. How common is this and how to handle this? -- cgit mrf-deployment