Skip to content
This repository has been archived by the owner on Nov 18, 2021. It is now read-only.

Erroneous closedness check for interpolated field+disjunction+Fill" #342

Closed
aluzzardi opened this issue Apr 10, 2020 · 5 comments
Closed
Labels
roadmap/evaluator Specific tag for roadmap issue #338

Comments

@aluzzardi
Copy link

aluzzardi commented Apr 10, 2020

What version of CUE are you using (cue version)?

master@d6f1eec7d713a25ef09af6ebaa34295c89c4a677

Does this issue reproduce with the latest release?

Yes

What did you do?

instance.Fill() followed by instance.Validate(cue.Concrete(true)) on a "dynamic" field with a disjunction.

Given this configuration:

Simple :: {
    ref: string
}

Complex :: {
	{
		ref: string
	} | {
		local: string
	}
}

var: "XXX"

// Validate(concrete) after Fill() on Complex.ref from a "static" key will succeed
"test_1": Complex

// Validate(concrete) after Fill() on "Simple.ref" from a "dynamic" key will succeed
"test_2_\(var)": Simple

// Validate(concrete) after Fill() on Complex.ref from a "dynamic" key will fail
"test_3_\(var)": Complex

It will yield:

instance.Fill("hello world", "test_1", "ref")
instance.Lookup("test_1").Validate(cue.Concrete(true)) == nil

instance.Fill("hello world", "test_2_XXX", "ref")
instance.Lookup("test_2_XXX").Validate(cue.Concrete(true)) == nil

instance.Fill("hello world", "test_3_XXX", "ref")
instance.Lookup("test_3_XXX").Validate(cue.Concrete(true)) == error
test_3_XXX: incomplete value ((C{ref: string, "hello world"} | C{ref: "hello world", local: string})):
    /Users/al/work/blocklayer/bl/runtime/tmp/test.cue:7:12
    /Users/al/work/blocklayer/bl/runtime/tmp/test.cue:8:2
    /Users/al/work/blocklayer/bl/runtime/tmp/test.cue:24:18

The full code and cue files to reproduce this issue can be found here:
https://gist.github.com/aluzzardi/5ee3a316593600c3eced3a3d37a66a94#file-main-go

@aluzzardi
Copy link
Author

aluzzardi commented Apr 10, 2020

I have another repro case ... it's not just about interpolated field, it's about "dynamic" (?) fields

var: "XXX"

// Validate(concrete) after Fill() on Complex.ref from a "static" key will succeed
"test_1": Complex

// Validate(concrete) after Fill() on "Simple.ref" from a "dynamic" key will succeed
if var == "XXX" {
	"test_2": Simple
}

// Validate(concrete) after Fill() on Complex.ref from a "dynamic" key will fail
if var == "XXX" {
	"test_3": Complex
}
instance.Fill("hello world", "test_1", "ref")
instance.Lookup("test_1").Validate(cue.Concrete(true)) = nil

instance.Fill("hello world", "test_2", "ref")
instance.Lookup("test_2").Validate(cue.Concrete(true)) = nil

instance.Fill("hello world", "test_3", "ref")
instance.Lookup("test_3").Validate(cue.Concrete(true)) = test_3: incomplete value ((C{ref: string, "hello world"} | C{ref: "hello world", local: string})):
    /Users/al/work/blocklayer/bl/runtime/tmp/test.cue:7:12
    /Users/al/work/blocklayer/bl/runtime/tmp/test.cue:8:2
    /Users/al/work/blocklayer/bl/runtime/tmp/test.cue:27:12

exit status 1

@mpvl
Copy link
Contributor

mpvl commented Apr 14, 2020

I'm able to reproduce it simulating Fill and Lookup, so not using the API:

X: {
	Simple :: {
		ref: string
	}
	
	Complex :: {
		{
			ref: string
		} | {
			local: string
		}
	}

	var: "XXX"
	
	// Validate(concrete) after Fill() on Complex.ref from a "static" key will succeed
	"test_1": Complex
	
	// Validate(concrete) after Fill() on "Simple.ref" from a "dynamic" key will succeed
	"test_2_\(var)": Simple
	
	// Validate(concrete) after Fill() on Complex.ref from a "dynamic" key will fail
	"test_3_\(var)": Complex

	test_3_XXX: ref: "FOO"
}

Z: X.test_3_XXX.ref

It probably is much easier to fix this with the new evaluator.

@aluzzardi
Copy link
Author

aluzzardi commented May 8, 2020

Hit this problem again, with a slight variation:

Complex :: {
	{
		ref: string
	} | {
		local: string
	}
}

v: Complex

if true {
	vv: Complex
}

Fill("test", "vv") followed by Vaidate(cue.Concrete(true)) fails (whereas the same with v works)

However, by making the disjunction fills optional, the problem seems to go away:

Complex :: {
	{
		ref?: string
	} | {
		local?: string
	}
}

@mpvl mpvl added roadmap/evaluator Specific tag for roadmap issue #338 and removed NeedsInvestigation labels May 11, 2020
@mpvl
Copy link
Contributor

mpvl commented May 11, 2020

From @aluzzardi

if true {
	v1 : {
		{
			x: string
		} | {
			y: string
		}
	}
	D :: {
		{
			x: string
		} | {
			y: string
		}
	}
	v2: D
}

Fill of v1.x + Validate(cue.Concrete) of v1 works
Fill of v2.x + Validate(cue.Concrete) of v2 fails (complains about y not being concrete
Removing the outer if true fixes the issue
As seen with v1 disjunctions work fine if they are not in definitions
In summary: It appears disjunctions in definitions get "flattened" under if/for (or interpolated struct keys -- basically any non "standard" tree path)

@cueckoo
Copy link

cueckoo commented Jul 3, 2021

This issue has been migrated to cue-lang/cue#342.

For more details about CUE's migration to a new home, please see cue-lang/cue#1078.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
roadmap/evaluator Specific tag for roadmap issue #338
Projects
None yet
Development

No branches or pull requests

3 participants