all: implement hidden identifiers scoped per-package
This has been part of the spec for a long time, but never implemented.
Note that all files with the same package name within a module
belong to the same package. A hidden identifier is thus uniqued by
module+name.
Closes #65
Fixes #503
Change-Id: I6d97ca1dbcf4ccc5730fde2cf8193c8e667787ad
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/7361
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/internal/core/adt/feature.go b/internal/core/adt/feature.go
index 6ee3b6a..0356e02 100644
--- a/internal/core/adt/feature.go
+++ b/internal/core/adt/feature.go
@@ -15,6 +15,7 @@
package adt
import (
+ "fmt"
"strconv"
"strings"
@@ -68,10 +69,36 @@
}
return literal.String.Quote(s)
default:
- return index.IndexToString(int64(x))
+ return f.IdentString(index)
}
}
+// IdentString reports the identifier of f. The result is undefined if f
+// is not an identifier label.
+func (f Feature) IdentString(index StringIndexer) string {
+ s := index.IndexToString(int64(f.Index()))
+ if f.IsHidden() {
+ if p := strings.IndexByte(s, '\x00'); p >= 0 {
+ s = s[:p]
+ }
+ }
+ return s
+}
+
+// PkgID returns the package identifier, composed of the module and package
+// name, associated with this identifier. It will return "" if this is not
+// a hidden label.
+func (f Feature) PkgID(index StringIndexer) string {
+ if !f.IsHidden() {
+ return ""
+ }
+ s := index.IndexToString(int64(f.Index()))
+ if p := strings.IndexByte(s, '\x00'); p >= 0 {
+ return s[p+1:]
+ }
+ return s
+}
+
// StringValue reports the string value of f, which must be a string label.
func (f Feature) StringValue(index StringIndexer) string {
if !f.IsString() {
@@ -113,17 +140,19 @@
}
// MakeIdentLabel creates a label for the given identifier.
-func MakeIdentLabel(r StringIndexer, s string) Feature {
- i := r.StringToIndex(s)
+func MakeIdentLabel(r StringIndexer, s, pkgpath string) Feature {
t := StringLabel
switch {
case strings.HasPrefix(s, "_#"):
t = HiddenDefinitionLabel
+ s = fmt.Sprintf("%s\x00%s", s, pkgpath)
case strings.HasPrefix(s, "#"):
t = DefinitionLabel
case strings.HasPrefix(s, "_"):
+ s = fmt.Sprintf("%s\x00%s", s, pkgpath)
t = HiddenLabel
}
+ i := r.StringToIndex(s)
f, err := MakeLabel(nil, i, t)
if err != nil {
panic("out of free string slots")
diff --git a/internal/core/adt/feature_test.go b/internal/core/adt/feature_test.go
index 833b425..858cc7e 100644
--- a/internal/core/adt/feature_test.go
+++ b/internal/core/adt/feature_test.go
@@ -77,18 +77,18 @@
isRegular: true,
isInt: true,
}, {
- in: adt.MakeIdentLabel(r, "foo"),
+ in: adt.MakeIdentLabel(r, "foo", "main"),
isRegular: true,
isString: true,
}, {
- in: adt.MakeIdentLabel(r, "#foo"),
+ in: adt.MakeIdentLabel(r, "#foo", "main"),
isDefinition: true,
}, {
- in: adt.MakeIdentLabel(r, "_#foo"),
+ in: adt.MakeIdentLabel(r, "_#foo", "main"),
isDefinition: true,
isHidden: true,
}, {
- in: adt.MakeIdentLabel(r, "_foo"),
+ in: adt.MakeIdentLabel(r, "_foo", "main"),
isHidden: true,
}}
for i, tc := range testCases {
diff --git a/internal/core/compile/compile.go b/internal/core/compile/compile.go
index e261193..740e904 100644
--- a/internal/core/compile/compile.go
+++ b/internal/core/compile/compile.go
@@ -39,18 +39,23 @@
// Imports allows unresolved identifiers to resolve to imports.
//
// Under normal circumstances, identifiers bind to import specifications,
- // which get resolved to an ImportReference. Use this option to automaically
- // resolve identifiers to imports.
+ // which get resolved to an ImportReference. Use this option to
+ // automatically resolve identifiers to imports.
Imports func(x *ast.Ident) (pkgPath string)
+
+ // pkgPath is used to qualify the scope of hidden fields. The default
+ // scope is "main".
+ pkgPath string
}
// Files compiles the given files as a single instance. It disregards
// the package names and it is the responsibility of the user to verify that
-// the packages names are consistent.
+// the packages names are consistent. The pkgID must be a unique identifier
+// for a package in a module, for instance as obtained from build.Instance.ID.
//
// Files may return a completed parse even if it has errors.
-func Files(cfg *Config, r adt.Runtime, files ...*ast.File) (*adt.Vertex, errors.Error) {
- c := newCompiler(cfg, r)
+func Files(cfg *Config, r adt.Runtime, pkgID string, files ...*ast.File) (*adt.Vertex, errors.Error) {
+ c := newCompiler(cfg, pkgID, r)
v := c.compileFiles(files)
@@ -60,14 +65,11 @@
return v, nil
}
-func Expr(cfg *Config, r adt.Runtime, x ast.Expr) (adt.Conjunct, errors.Error) {
- if cfg == nil {
- cfg = &Config{}
- }
- c := &compiler{
- Config: *cfg,
- index: r,
- }
+// Expr compiles the given expression into a conjunct. The pkgID must be a
+// unique identifier for a package in a module, for instance as obtained from
+// build.Instance.ID.
+func Expr(cfg *Config, r adt.Runtime, pkgPath string, x ast.Expr) (adt.Conjunct, errors.Error) {
+ c := newCompiler(cfg, pkgPath, r)
v := c.compileExpr(x)
@@ -77,13 +79,17 @@
return v, nil
}
-func newCompiler(cfg *Config, r adt.Runtime) *compiler {
+func newCompiler(cfg *Config, pkgPath string, r adt.Runtime) *compiler {
c := &compiler{
index: r,
}
if cfg != nil {
c.Config = *cfg
}
+ if pkgPath == "" {
+ pkgPath = "main"
+ }
+ c.Config.pkgPath = pkgPath
return c
}
diff --git a/internal/core/compile/compile_test.go b/internal/core/compile/compile_test.go
index f5a73ac..4072ed9 100644
--- a/internal/core/compile/compile_test.go
+++ b/internal/core/compile/compile_test.go
@@ -53,7 +53,7 @@
a := t.ValidInstances()
- v, err := compile.Files(nil, r, a[0].Files...)
+ v, err := compile.Files(nil, r, "main", a[0].Files...)
// Write the results.
t.WriteErrors(err)
@@ -113,7 +113,7 @@
}
r := runtime.New()
- arc, err := compile.Files(nil, r, file)
+ arc, err := compile.Files(nil, r, "main", file)
if err != nil {
t.Error(errors.Details(err, nil))
}
diff --git a/internal/core/compile/label.go b/internal/core/compile/label.go
index 5fddde8..d35b7c0 100644
--- a/internal/core/compile/label.go
+++ b/internal/core/compile/label.go
@@ -29,7 +29,7 @@
index := c.index
switch x := n.(type) {
case *ast.Ident:
- return adt.MakeIdentLabel(c.index, x.Name)
+ return adt.MakeIdentLabel(c.index, x.Name, c.pkgPath)
case *ast.BasicLit:
switch x.Kind {
diff --git a/internal/core/convert/go.go b/internal/core/convert/go.go
index 2a4173b..1997136 100644
--- a/internal/core/convert/go.go
+++ b/internal/core/convert/go.go
@@ -68,7 +68,7 @@
}
func compileExpr(ctx *adt.OpContext, expr ast.Expr) adt.Value {
- c, err := compile.Expr(nil, ctx, expr)
+ c, err := compile.Expr(nil, ctx, pkgID(), expr)
if err != nil {
return &adt.Bottom{Err: errors.Promote(err, "compile")}
}
@@ -241,7 +241,7 @@
return &adt.Null{Src: ctx.Source()}
case *ast.File:
- x, err := compile.Files(nil, ctx, v)
+ x, err := compile.Files(nil, ctx, pkgID(), v)
if err != nil {
return &adt.Bottom{Err: errors.Promote(err, "compile")}
}
@@ -737,7 +737,7 @@
ctx.AddErrf(msg, args...)
})
var x adt.Expr
- c, err := compile.Expr(nil, ctx, e)
+ c, err := compile.Expr(nil, ctx, pkgID(), e)
if err != nil {
b := &adt.Bottom{Err: err}
ctx.AddBottom(b)
@@ -788,3 +788,8 @@
}
return ast.NewBinExpr(token.OR, null, e)
}
+
+// pkgID returns a package path that can never resolve to an existing package.
+func pkgID() string {
+ return "_"
+}
diff --git a/internal/core/debug/debug.go b/internal/core/debug/debug.go
index a5d8e0b..b789a05 100644
--- a/internal/core/debug/debug.go
+++ b/internal/core/debug/debug.go
@@ -87,6 +87,13 @@
// TODO: fold into label once :: is no longer supported.
func (w *printer) labelString(f adt.Feature) string {
+ if f.IsHidden() {
+ ident := f.IdentString(w.index)
+ if pkgName := f.PkgID(w.index); pkgName != "main" {
+ ident = fmt.Sprintf("%s(%s)", ident, pkgName)
+ }
+ return ident
+ }
return f.SelectorString(w.index)
}
diff --git a/internal/core/eval/optionals_test.go b/internal/core/eval/optionals_test.go
index 4d5c97c..1abadc4 100644
--- a/internal/core/eval/optionals_test.go
+++ b/internal/core/eval/optionals_test.go
@@ -60,7 +60,7 @@
t.Fatal(err)
}
- v, errs := compile.Files(nil, ctx, f)
+ v, errs := compile.Files(nil, ctx, "", f)
if errs != nil {
t.Fatal(errs)
}
diff --git a/internal/core/export/adt.go b/internal/core/export/adt.go
index 8013a05..429d8e4 100644
--- a/internal/core/export/adt.go
+++ b/internal/core/export/adt.go
@@ -28,7 +28,7 @@
)
func (e *exporter) ident(x adt.Feature) *ast.Ident {
- s := e.ctx.IndexToString(int64(x.Index()))
+ s := x.IdentString(e.ctx)
if !ast.IsValidIdent(s) {
panic(s + " is not a valid identifier")
}
diff --git a/internal/core/export/export.go b/internal/core/export/export.go
index 733c202..da23162 100644
--- a/internal/core/export/export.go
+++ b/internal/core/export/export.go
@@ -35,9 +35,13 @@
// IncludeDocs
ShowOptional bool
ShowDefinitions bool
- ShowHidden bool
- ShowDocs bool
- ShowAttributes bool
+
+ // ShowHidden forces the inclusion of hidden fields when these would
+ // otherwise be omitted. Only hidden fields from the current package are
+ // included.
+ ShowHidden bool
+ ShowDocs bool
+ ShowAttributes bool
// AllowErrorType
// Use unevaluated conjuncts for these error types
@@ -77,13 +81,13 @@
// Concrete
// Def exports v as a definition.
-func Def(r adt.Runtime, v *adt.Vertex) (*ast.File, errors.Error) {
- return All.Def(r, v)
+func Def(r adt.Runtime, pkgID string, v *adt.Vertex) (*ast.File, errors.Error) {
+ return All.Def(r, pkgID, v)
}
// Def exports v as a definition.
-func (p *Profile) Def(r adt.Runtime, v *adt.Vertex) (*ast.File, errors.Error) {
- e := newExporter(p, r, v)
+func (p *Profile) Def(r adt.Runtime, pkgID string, v *adt.Vertex) (*ast.File, errors.Error) {
+ e := newExporter(p, r, pkgID, v)
if v.Label.IsDef() {
e.inDefinition++
}
@@ -100,12 +104,12 @@
return e.toFile(v, expr)
}
-func Expr(r adt.Runtime, n adt.Expr) (ast.Expr, errors.Error) {
- return Simplified.Expr(r, n)
+func Expr(r adt.Runtime, pkgID string, n adt.Expr) (ast.Expr, errors.Error) {
+ return Simplified.Expr(r, pkgID, n)
}
-func (p *Profile) Expr(r adt.Runtime, n adt.Expr) (ast.Expr, errors.Error) {
- e := newExporter(p, r, nil)
+func (p *Profile) Expr(r adt.Runtime, pkgID string, n adt.Expr) (ast.Expr, errors.Error) {
+ e := newExporter(p, r, pkgID, nil)
return e.expr(n), nil
}
@@ -156,30 +160,32 @@
// File
-func Vertex(r adt.Runtime, n *adt.Vertex) (*ast.File, errors.Error) {
- return Simplified.Vertex(r, n)
+func Vertex(r adt.Runtime, pkgID string, n *adt.Vertex) (*ast.File, errors.Error) {
+ return Simplified.Vertex(r, pkgID, n)
}
-func (p *Profile) Vertex(r adt.Runtime, n *adt.Vertex) (*ast.File, errors.Error) {
+func (p *Profile) Vertex(r adt.Runtime, pkgID string, n *adt.Vertex) (*ast.File, errors.Error) {
e := exporter{
cfg: p,
index: r,
+ pkgID: pkgID,
}
v := e.value(n, n.Conjuncts...)
return e.toFile(n, v)
}
-func Value(r adt.Runtime, n adt.Value) (ast.Expr, errors.Error) {
- return Simplified.Value(r, n)
+func Value(r adt.Runtime, pkgID string, n adt.Value) (ast.Expr, errors.Error) {
+ return Simplified.Value(r, pkgID, n)
}
// Should take context.
-func (p *Profile) Value(r adt.Runtime, n adt.Value) (ast.Expr, errors.Error) {
+func (p *Profile) Value(r adt.Runtime, pkgID string, n adt.Value) (ast.Expr, errors.Error) {
e := exporter{
ctx: eval.NewContext(r, nil),
cfg: p,
index: r,
+ pkgID: pkgID,
}
v := e.value(n)
return v, e.errs
@@ -199,13 +205,20 @@
inDefinition int // for close() wrapping.
unique int
+
+ // hidden label handling
+ pkgID string
+ hidden map[string]adt.Feature // adt.InvalidFeatures means more than one.
+ usedFeature map[adt.Feature]string
+ usedHidden map[string]bool
}
-func newExporter(p *Profile, r adt.Runtime, v *adt.Vertex) *exporter {
+func newExporter(p *Profile, r adt.Runtime, pkgID string, v *adt.Vertex) *exporter {
return &exporter{
cfg: p,
ctx: eval.NewContext(r, v),
index: r,
+ pkgID: pkgID,
}
}
diff --git a/internal/core/export/export_test.go b/internal/core/export/export_test.go
index d384681..7aa4652 100644
--- a/internal/core/export/export_test.go
+++ b/internal/core/export/export_test.go
@@ -49,7 +49,7 @@
test.Run(t, func(t *cuetxtar.Test) {
a := t.ValidInstances()
- v, errs := compile.Files(nil, r, a[0].Files...)
+ v, errs := compile.Files(nil, r, "", a[0].Files...)
if errs != nil {
t.Fatal(errs)
}
@@ -58,7 +58,7 @@
// TODO: do we need to evaluate v? In principle not necessary.
// v.Finalize(eval.NewContext(r, v))
- file, errs := export.Def(r, v)
+ file, errs := export.Def(r, "", v)
errors.Print(t, errs, nil)
_, _ = t.Write(formatNode(t.T, file))
})
@@ -117,7 +117,7 @@
if err != nil {
return nil, err
}
- c, err := compile.Expr(nil, ctx, expr)
+ c, err := compile.Expr(nil, ctx, "_", expr)
if err != nil {
return nil, err
}
@@ -145,7 +145,7 @@
if err != nil {
t.Fatal("failed test case: ", err)
}
- expr, err := export.Expr(ctx, v)
+ expr, err := export.Expr(ctx, "", v)
if err != nil {
t.Fatal("failed export: ", err)
}
@@ -223,13 +223,13 @@
// astutil.Sanitize(x)
r := runtime.New()
- v, errs := compile.Files(nil, r, a[0].Files...)
+ v, errs := compile.Files(nil, r, "", a[0].Files...)
if errs != nil {
t.Fatal(errs)
}
v.Finalize(eval.NewContext(r, v))
- file, errs := export.Def(r, v)
+ file, errs := export.Def(r, "main", v)
if errs != nil {
t.Fatal(errs)
}
diff --git a/internal/core/export/extract_test.go b/internal/core/export/extract_test.go
index 02a098c..a542975 100644
--- a/internal/core/export/extract_test.go
+++ b/internal/core/export/extract_test.go
@@ -38,7 +38,7 @@
test.Run(t, func(t *cuetxtar.Test) {
a := t.ValidInstances()
- v, err := compile.Files(nil, r, a[0].Files...)
+ v, err := compile.Files(nil, r, "", a[0].Files...)
if err != nil {
t.Fatal(err)
}
diff --git a/internal/core/export/label.go b/internal/core/export/label.go
index b04cb93..50fca4d 100644
--- a/internal/core/export/label.go
+++ b/internal/core/export/label.go
@@ -33,7 +33,8 @@
return ast.NewLit(token.INT, strconv.Itoa(int(x)))
case adt.DefinitionLabel, adt.HiddenLabel, adt.HiddenDefinitionLabel:
- return ast.NewIdent(e.ctx.IndexToString(int64(x)))
+ s := f.IdentString(e.ctx)
+ return ast.NewIdent(s)
case adt.StringLabel:
s := e.ctx.IndexToString(int64(x))
diff --git a/internal/core/export/testdata/hidden.txtar b/internal/core/export/testdata/hidden.txtar
new file mode 100644
index 0000000..334b20e
--- /dev/null
+++ b/internal/core/export/testdata/hidden.txtar
@@ -0,0 +1,57 @@
+-- in.cue --
+package foo
+
+import "acme.com/pkg"
+
+a: pkg.#A
+a: _val: 3
+a: _#val: 6
+-- cue.mod/module.cue --
+module: "acme.com"
+
+-- pkg/pkg.cue --
+package pkg
+
+#A: {
+ _val: 4
+ _#val: 5
+}
+
+-- out/definition --
+package foo
+
+import "acme.com/pkg"
+
+a: {
+ pkg.#A
+ _val: 3
+ _#val: 6
+}
+-- out/doc --
+[]
+[a]
+[a _val]
+[a _#val]
+-- out/value --
+== Simplified
+{
+ a: {}
+}
+== Raw
+{
+ a: {
+ _val: 3
+ _#val: 6
+ }
+}
+== Final
+{
+ a: {}
+}
+== All
+{
+ a: {
+ _val: 3
+ _#val: 6
+ }
+}
diff --git a/internal/core/export/value.go b/internal/core/export/value.go
index 4217425..eef9b21 100644
--- a/internal/core/export/value.go
+++ b/internal/core/export/value.go
@@ -310,8 +310,13 @@
if label.IsDef() && !p.ShowDefinitions {
continue
}
- if label.IsHidden() && !p.ShowHidden {
- continue
+ if label.IsHidden() {
+ if !p.ShowHidden {
+ continue
+ }
+ if label.PkgID(e.ctx) != e.pkgID {
+ continue
+ }
}
f := &ast.Field{Label: e.stringLabel(label)}
diff --git a/internal/core/export/value_test.go b/internal/core/export/value_test.go
index a75674d..e22c21f 100644
--- a/internal/core/export/value_test.go
+++ b/internal/core/export/value_test.go
@@ -47,9 +47,11 @@
test.Run(t, func(t *cuetxtar.Test) {
a := t.ValidInstances()
- v, errs := compile.Files(nil, r, a[0].Files...)
- if errs != nil {
- t.Fatal(errs)
+ pkgID := a[0].ID()
+
+ v, err := r.Build(a[0])
+ if err != nil {
+ t.Fatal(err)
}
ctx := eval.NewContext(r, v)
@@ -57,7 +59,7 @@
for _, tc := range []struct {
name string
- fn func(r adt.Runtime, v adt.Value) (ast.Expr, errors.Error)
+ fn func(r adt.Runtime, id string, v adt.Value) (ast.Expr, errors.Error)
}{
{"Simplified", export.Simplified.Value},
{"Raw", export.Raw.Value},
@@ -65,7 +67,7 @@
{"All", export.All.Value},
} {
fmt.Fprintln(t, "==", tc.name)
- x, errs := tc.fn(r, v)
+ x, errs := tc.fn(r, pkgID, v)
errors.Print(t, errs, nil)
_, _ = t.Write(formatNode(t.T, x))
fmt.Fprintln(t)
@@ -89,7 +91,7 @@
a := cuetxtar.Load(archive, "/tmp/test")
r := runtime.New()
- v, errs := compile.Files(nil, r, a[0].Files...)
+ v, errs := compile.Files(nil, r, "", a[0].Files...)
if errs != nil {
t.Fatal(errs)
}
@@ -97,7 +99,7 @@
ctx := eval.NewContext(r, v)
v.Finalize(ctx)
- x, errs := export.Simplified.Value(r, v)
+ x, errs := export.Simplified.Value(r, "main", v)
if errs != nil {
t.Fatal(errs)
}
diff --git a/internal/core/runtime/index.go b/internal/core/runtime/index.go
index 81cb515..a8e381c 100644
--- a/internal/core/runtime/index.go
+++ b/internal/core/runtime/index.go
@@ -147,8 +147,7 @@
}
func (idx *Index) LabelStr(l adt.Feature) string {
- index := int64(l.Index())
- return idx.IndexToString(index)
+ return l.IdentString(idx)
}
func (x *Index) AddInst(path string, key, p interface{}) {
diff --git a/internal/core/runtime/runtime.go b/internal/core/runtime/runtime.go
index fd53f0c..6361e6b 100644
--- a/internal/core/runtime/runtime.go
+++ b/internal/core/runtime/runtime.go
@@ -80,7 +80,7 @@
return nil, errs
}
- return compile.Files(nil, x, b.Files...)
+ return compile.Files(nil, x, b.ID(), b.Files...)
}
func (x *Runtime) buildSpec(b *build.Instance, spec *ast.ImportSpec) (errs errors.Error) {
diff --git a/internal/core/subsume/structural_test.go b/internal/core/subsume/structural_test.go
index b22ded7..2e1635e 100644
--- a/internal/core/subsume/structural_test.go
+++ b/internal/core/subsume/structural_test.go
@@ -465,7 +465,7 @@
t.Fatal(err)
}
- root, errs := compile.Files(nil, r, file)
+ root, errs := compile.Files(nil, r, "", file)
if errs != nil {
t.Fatal(errs)
}
diff --git a/internal/core/subsume/subsume_test.go b/internal/core/subsume/subsume_test.go
index 757c901..7379689 100644
--- a/internal/core/subsume/subsume_test.go
+++ b/internal/core/subsume/subsume_test.go
@@ -50,7 +50,7 @@
t.Fatal(err)
}
- root, errs := compile.Files(nil, ctx, file)
+ root, errs := compile.Files(nil, ctx, "", file)
if errs != nil {
t.Fatal(errs)
}
diff --git a/internal/core/subsume/value_test.go b/internal/core/subsume/value_test.go
index 80869c4..e5ed21d 100644
--- a/internal/core/subsume/value_test.go
+++ b/internal/core/subsume/value_test.go
@@ -387,7 +387,7 @@
t.Fatal(err)
}
- root, errs := compile.Files(nil, r, file)
+ root, errs := compile.Files(nil, r, "", file)
if errs != nil {
t.Fatal(errs)
}
diff --git a/internal/core/validate/validate_test.go b/internal/core/validate/validate_test.go
index 496ad3b..fd88dd6 100644
--- a/internal/core/validate/validate_test.go
+++ b/internal/core/validate/validate_test.go
@@ -174,13 +174,13 @@
if err != nil {
t.Fatal(err)
}
- v, err := compile.Files(nil, r, f)
+ v, err := compile.Files(nil, r, "", f)
if err != nil {
t.Fatal(err)
}
ctx.Unify(ctx, v, adt.Finalized)
if tc.lookup != "" {
- v = v.Lookup(adt.MakeIdentLabel(r, tc.lookup))
+ v = v.Lookup(adt.MakeIdentLabel(r, tc.lookup, "main"))
}
b := Validate(ctx, v, tc.cfg)
