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

References to embedded fields through an alias do not resolve to concrete values #380

Closed
seh opened this issue May 5, 2020 · 13 comments
Closed
Labels
Documentation FeatureRequest New feature or request FeedbackWanted Further information is requested
Milestone

Comments

@seh
Copy link

seh commented May 5, 2020

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

$ cue version
cue version devel darwin/amd64

(Built via go get on 27 April 2020 at 8:02 a.m. EDT using commit 12927e8)

Does this issue reproduce with the latest release?

Yes.

What did you do?

Using an embedding, I'd like to refer to field in the embedding from the enclosing scope. Since an embedding is not lexically available within its enclosing scope, I bound an alias to that enclosing scope, and attempted to reference the embedding's field through this alias.

The motivating example is available in the CUE Playground. @mpvl and I discussed this problem in the "language" channel of the "CUE" Slack team as well.

In short, though, given an embedded definition #Def with field "a", I'd like to refer to field "a" within an embedding definition #Outer, like this:

#Def: {
  a: string
}

O=#Outer: {
  #Def
  reference: O.a
}

x: #Outer & {
  a: "foo"
}

What did you expect to see?

The example above should have yielded a field "x" with its value as a struct with two fields: "a" with value "foo" and "reference" with value "foo".

What did you see instead?

Field "x" remains incomplete. It has a "reference" field, but its value is string rather than the concrete string "foo". It seems that it doesn't pick up the value from the embedded "a" field.

If instead we repeat the "a" field in the #Outer definition, and refer to it as a sibling to "reference" rather than through the "O" alias, it works, but we're then restating the declaration of "a" that we hoped to avoid by embedding #Def here. (Assume there are many more such fields that would be onerous to restate like this.)

#Def: {
  a: string
}

#Outer: {
  #Def
  // NB: Restate the "a" field here.
  a: string
  // NB: Refer to "a" as a sibling rather than through an alias.
  reference: a
}

x: #Outer & {
  a: "foo"
}
@kumorid
Copy link

kumorid commented May 10, 2020

@seh

Hi, I am going to chime in on this as I believe what you observe is expected (the last word is for mpvl).

What happens in your original cue example is that O.a is a reference to field a of the "instance" defining #Outer, which, of course, is string.

The definition of x does not affect the definition of #Outer, so, x.reference keeps its old constraint, #Outer.a which is string.

In the version you offer at the end of your post, the constraint you pose for field reference is that it should match field a of the struct in which it is.

The definition for x defines x.a to be "foo", thus, x.reference must satisfy the constraint that it must equal x.a, thus it must be equal to "foo" too.

I hope my explanation makes sense and helps...

@seh
Copy link
Author

seh commented May 10, 2020

Thank you for your analysis, @kumorid. Note that Marcel wrote the following when I pointed out my attempted use of the alias "O":

Ah. Yes, that should work.

I asked for clarification as to whether this is a defect in CUE. He then said:

I meant that I expected it to work now and if not than that is a bug.

@kumorid
Copy link

kumorid commented May 10, 2020

Well, some things in cue are trickier than expected.

We decided to use cue as the language to specify the service model for our platform, as I believe its principles are sound, make tons of sense, and will be extremely helpful for our purpose.

Having said that, it is VERY easy to make mistakes of logic derived from stuff like the one I pointed out.
This would be an almost-non-issue if the evaluator was bug-free, and the error reporting was more useful, as error reports would be helpful in learning cue itself.

But of course, the project is not mature enough: error reporting is not great, and there are bugs in the evaluator. Both combined make it challenging to interpret failures when they happen.

As the project matures, the evaluator bugs should be mostly gone, and the messages should be more helpful. Both combined can go a long way towards helping to onboard advanced users.

@mpvl
Copy link
Contributor

mpvl commented May 13, 2020

Actually, @kumorid is right, that this is expected behavior. This used to be different in CUE, but the current behavior is correct, and I made an error looking at the Slack example I see. My apologies. I also agree with @kumorid that things get a lot easier if the evaluator weren't buggy and if we had better error messages. Hence these are the two next top focus points.

A concrete example of why the current example needs to be what it is:

#List: {
  value: _
  next: #List | null
}

This is a common pattern for schema definitions, and one would not expect the next to point to the instantiated copy of #List. In general, references pointing outside a referenced value (like #List here) bind to the original location.

Solutions for a better shorthand:
It may be reasonable to allow referring to the embedding itself:

#Outer: {
  #Def
  reference: #Def.a
}

After old-style aliases have been deprecated for a while (now we have let declarations) we could also consider allowing aliases to embeddings:

#Outer: {
  O=#Def
  reference: O.a
}
// or
#Outer: #Def & {
  O=_
  reference: O.a
}

or even allow aliases for any expression:

#Outer: O={
  reference: O.a
}

This would need some more thinking though.

@mpvl mpvl added Documentation FeatureRequest New feature or request FeedbackWanted Further information is requested and removed NeedsInvestigation labels May 13, 2020
@seh
Copy link
Author

seh commented May 14, 2020

Oh, if you could see all the things I've typed, trying to see what works. I've tried aliasing my embeddings, as you showed, and I've tried referencing them directly like your "#Def.a" example above. That I've tried these things—expecting that they might work—is a good sign that allowing them to work would be helpful. It's kind of like putting a lost then found object back in the first place you thought to look for it.

Referring to the embedding directly ("#Def.a") above mimics Go, in part. Here's an example that shows that embedded fields can have the same name as a field in the embedding struct, but that you can refer to the embedded fields by their type name, which becomes the field name in the embedding struct for lack of an explicit alternative.

Given that the ideas above are not yet implemented, is restating the "a" field in the original example the only way to make this work?

@kumorid
Copy link

kumorid commented May 15, 2020

@mpvl

Well, I would find useful to have some way to reliably refer to the embedded structure. I need it in some of my definitions when embedding a def/struct within another one: I often need to add a field name from within the embedded struct to be able to refer to it in the rest of the definition, similar to the example provided by @seh

#Outer: {
#Def
// NB: Restate the "a" field here.
a: _
// NB: Refer to "a" as a sibling rather than through an alias.
reference: a
}

To avoid those redundant, not very informative, declarations, one immediately jumps to think of something similar to a self reference, which is something very similar to your proposal of O=_ where _ plays the role of self

Using

#Outer: {
  O=_
 #Def
  reference: O.a
}

would allow referring to any field within #Def without explicitly mentioning it with arbitrary types within #Outer, in exactly the way shown above. In addition, it would be clear that a should come from the embedded definition...

I like the notation O=_ better that O=#Def as it is terser in case more than one embedding is involved, and, in addition, avoids the confusion of thinking that O refers only to the struct being embedded, when in reality, it should refer to all the fields from #Def merged, possibly, with additional restrictions imposed in #Outer itself, like in a hypothetical

#Outer: {
  O= #Def
  a:  "foo" | "bar"
  reference: O.a
}

In the above example, notation may lead to think that O does not integrate the extra restricion, when in reality it should.

On the contrary,

#Outer: {
  O=_
 #Def1
 #Def2
  a: "foo" | "bar"
  reference: O.a   // Coming from #Def1
  re2: O.b              // Coming from #Def2
}

Makes it clear that O.a includes the additional restriction, and does not give the false sensation that using O1=#Def1 and O2=#Def2 would give of thinking that O1 and O2 are isolated structs.

To me, the notation O=_ seems to mean exactly what it would mean: an alias for the struct instance itself, not its definition.

The question arises, though, if we could use _ itself as the reference to the struct where it is used, or if that would be too confusing when we have nested structs. The advantage of being able to use _ directly would be consistency of the notation O=_, which would not be special any more.

@seh
Copy link
Author

seh commented May 15, 2020

To support @kumorid's suggestion, coming from Jsonnet, I too find myself reaching for "self" often, then backing up and trying to figure out how to introduce an alias at some scope to make it work. It is hard to guess which fields from which scopes are visible for references, with all the automatic hoisting of fields upward from embedded structs.

@mpvl mpvl added this to the v0.3.x milestone Jan 19, 2021
@mpvl
Copy link
Contributor

mpvl commented Apr 29, 2021

Note that https://cue-review.googlesource.com/c/cue/+/9543/1 implements "field value" aliases, allowing:

foo: X=bar

So instead of writing

foo: {
    X=_
    a: int
}

one could write the following to achieve exactly the same:

foo: X={
    a: int
}

To the user this may be the same thing, and it may be weird we support one form, but not the other. The main reason why only this is supported is because the X=_ used to be what is now let X = _ and we are still phasing this out. The ultimate goal is to allow both to be equivalent.

@seh
Copy link
Author

seh commented Apr 29, 2021

Does that mean that rewriting my original example as

#Outer: O={
  #Def
  reference: O.a
}

would work as I had expected, where the "reference" field would read the "a" field embedded with the #Def definition?

@mpvl
Copy link
Contributor

mpvl commented Apr 29, 2021

@seh: yes.

@mpvl
Copy link
Contributor

mpvl commented Apr 29, 2021

Running in my client:

// in.cue
#Def: {
	a: string
}

#Outer: O={
	#Def
	reference: O.a
}

x: #Outer & {
	a: "foo"
}

Now cue eval in.cue gives:

#Def: {
    a: string
}
#Outer: {
    reference: string
    a:         string
}
x: {
    reference: "foo"
    a:         "foo"
}

cueckoo pushed a commit that referenced this issue Apr 29, 2021
Support aliases of the form:
   a: X=b

This implementation also allows for general alias values,
but these have not yet been implemented as alias
declarations are still being deprecated.

Issue #620
Issue #380

Change-Id: Ide4c888bea187042898e8123480a1cfeb909914c
cueckoo pushed a commit that referenced this issue Apr 30, 2021
Support aliases of the form:
   a: X=b

This implementation also allows for general alias values,
but these have not yet been implemented as alias
declarations are still being deprecated.

Issue #620
Issue #380

Change-Id: Ide4c888bea187042898e8123480a1cfeb909914c
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/9543
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
@myitcv
Copy link
Contributor

myitcv commented May 27, 2021

@seh - I think this is now resolved with the release of value aliases, so I will close this issue. But please shout if that's not the case and we can re-open.

@cueckoo
Copy link

cueckoo commented Jul 3, 2021

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

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
Documentation FeatureRequest New feature or request FeedbackWanted Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants