Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify err:XS0091 #826

Open
ndw opened this issue Jun 30, 2019 · 21 comments

Comments

Projects
None yet
3 participants
@ndw
Copy link
Contributor

commented Jun 30, 2019

I don't really understand this paragraph:

Neither options nor variables in the prologue may shadow each other. It is a static error (err:XS0091) if a p:option or p:variable declared before the subpipeline begins shadows another option or variable declared within the same p:declare-step. (Within the subpipeline, variables may shadow options and lexically preceding (non-static) variables.)

You can't have two options with the same name, so that case isn't relevant.

The first part of that paragraph talks about "within the prolog" but then we say variables and options within the subpipeline can shadow each othere, except for the casual remark about not shadowing preceding static variables.

  1. Why are we trying to prevent static variables from shadowing each other? What aspect of lexical scoping are we worried about?

  2. Why are we trying to prevent non-static variables from shadowing preceding static variables or options?

@eriksiegel

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

I don't have an answer to this question but I remember a lively discussion about this. Something to do with nasty edge cases...
Wasn't it you, @xml-project, who brought this up?

@xml-project

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

@eriksiegel It does not ring a bell. I will look at my note this afternoon. May be I will find something.

@xml-project

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

See #550 und (for the do not shadow rule) #506
Also see the minutes from our first day at Leipzig last year.

@xml-project

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

Looking at my notes from Leipzig last year, the answer to (1) is pure pragmatic. If they can't be shadowed you do not have to build a scope to find out the last shadowing, which is the real value.

I do not think it is worth it to allow scoping: Static variables are constants and a goody for pipeline authors because they do not have to write / change the same value over and over again. All this can be done without scoping -> choose another name for the new variable.

Concerning (2) I have to think about it.

@xml-project

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

@ndw said:

You can't have two options with the same name, so that case isn't relevant.

I think what we were talking about is something like this:

<p:declare-step xmlns:p="http://www.w3.org/ns/xproc" 
	xmlns:xs="http://www.w3.org/2001/XMLSchema" 
	xmlns:test="http://dummy" version="3.0">
	<p:output port="result" sequence="true"/>
(1)	<p:option name="option" static="true"  />
	<p:declare-step type="test:test">
		<p:output port="result" />
(2)		<p:option name="option" static="true" />
		<p:identity>
			<p:with-input><doc>{$option}</doc></p:with-input>
		</p:identity>
	</p:declare-step>
	<test:test />
</p:declare-step>

I do not like our solution either. If static options are basically static variables coming from the outside (the processor), I would argue they can only be declared in the outer most p:declare-step, but that was not our consensus. We agreed to make no distinction between the outer and any inner pipeline. This was why we said, they could not shadow.

@ndw

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2019

No, I was talking about two options in the same declare-step not being allowed.

@xml-project

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

OK, like:

<p:declare-step>
  <p:option static="true" name="option" select="1" />
  <p:option static="true" name="option" select="2  />
</p:declare-step>

If the option is set from the outside, it is pointless, because @select is never used. If it is not set from the outside it also pointless, because (allowing shadowing) it always "2".
What did I miss?

Btw:
If in my previous example the two "option" are different, how is a processor supposed to know, which is which and to set the two supplied values to the right option?

@ndw

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2019

The example above is what I meant, but note that this is flatly invalid: you can't have two options on the same step with the same name.

In the earlier example, where a static option is declared in a nested step, the same rules apply. The expression that initializes that option must be computed statically and can refer to statics that are already in-scope. (Or that's how I understood it, anyway.)

@ndw

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2019

By "the same rules" I meant, the rule that it has to be resolved statically.

@xml-project

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

Sorry, I lost track. I was trying to explain why

You can't have two options with the same name, so that case isn't relevant.

I try to give an example what this means (because static option to my reading establish only one scope since there is no way to set them).

Do you actually propose to make the minor goody of static options/variables such a big thing. As I said, to me they make sense in the outermost pipeline and shouldn't be declarable in nested steps.

@ndw

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2019

I don't think we should make nested pipelines different from imported pipelines and surely statics have to be initialized in imported pipelines, no?

@xml-project

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

I don't think we should make nested pipelines different from imported pipelines

I know that we disagree in this point and we decided to follow your opinion. But imho that can't be the reason to make static options/variables a monster.

surely statics have to be initialized in imported pipelines, no?

Yes, but that does not mean inner and outer pipelines have to be the same. Remember my argument was to allow static option/variable declaration in a p:library. So if a step to be imported relies on a static option/variable, they have to be put in a p:library.

But I think we should not open up this discussion again. If I remember right, the rule "do not shadow static options/variables" was a consequence of allowing their declaration "everywhere" and the need to find a simple solution. You might say you do not want a simple solution anymore. But then we should concentrate the discussion on this point and its consequences.

I am not sure I even understand the semantics of shadowing a static option. But may be this is just my problem.

@xml-project

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

I think I found the reason in my notes, why static variables do not shadow: Suppose

<p:library>
  <p:variable static="true" name="var" select="1" />
  <p:variable static="true" name="var" select="$var +1" />

What is the value of the imported variable? We need to make a rule for this.

@eriksiegel

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

Gentleman. I find it worrying that this discussion pops up again. I thought we made a decision on this?

@eriksiegel eriksiegel closed this Jul 2, 2019

@eriksiegel eriksiegel reopened this Jul 2, 2019

@xml-project

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

@eriksiegel So what is your proposal?

@eriksiegel

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

Stick to what we discussed. No shadowing of static variables and options.

@xml-project

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

Fine with me.

@ndw

This comment has been minimized.

Copy link
Contributor Author

commented Jul 2, 2019

The spec is unclear and self-contradictory so I don't believe that the decision was adequately recorded.

Observation: we went to a lot of trouble to make sure that variables and options (henceforth "variables", but I mean both) behave in the way users expect. We allow them anywhere and we allow them to shadow.

Opinions follow.

Assertion: the principle of least surprise should apply. A user who understands how variables work shouldn't be surprised by the behavior of a variable just because it's static.

  1. Dynamic variables must be allowed to shadow static ones. Consider a pipeline with a bunch of nested, shadowed variables. Deciding to make the top one static shouldn't break the entire pipeline.
  2. In separate pipelines, it must be possible to shadow statics. Pipelines are self-contained. If I pick one up and drop it into a larger pipeline, it shouldn't suddenly break because of the static variables in the surrounding pipeline.
  3. Given 1 and 2, there's no compelling reason (that I can think of) not to allow statics to shadow each other. And it satisfies the principle of least surprise.

At any given point where an expression occurs, there is an unambiguous set of in-scope variables. Those are the ones that are used to evaluate the expression. If the variable is static, then any reference to non-static variables (or the context node) is an error.

static $foo = 5
static $foo = $foo + 1
$bar = $foo + 2  // $bar = 8
$foo = $foo + 9  // $foo = 15
static $baz = $foo + 1 // ERROR: $foo is not static

Additional constraints:

  1. It is an error to have two p:options with the same name in the same prologue.
  2. It is an error to have two different p:variables with the same name at the top level in a p:library

I'm a little unclear on how we intended a p:option in a p:library to ever get a different value at runtime, I suppose we could imagine a global context in which initial static option values are known. We probably need to be explicit about what happens in this case:

Given lib.xpl:

<p:library>
  <p:option name="foo" static="true" value="'libdefault'"/>
</p:library>

And a pipeline:

<p:declare-step>
  <p:option name="foo" static="true" value="'pipelinedefault'"/>

  <p:declare-step>
    <p:import href="lib.xpl"/>
    // What is the value of $foo here?
    // Does it depend on whether or not I provided a 'runtimevalue' value at runtime?
   ...
...

I think a case could be made for any of the three possible values. If the 'runtimevalue' is provided, I expect the only useful answer is 'runtimevalue. If no runtime value is provided, I'm inclined to say that the value should be libdefault. But I'm also inclined to remove p:option from p:library so we don't have to answer this question.

Consider this library:

<p:library>
  <p:option name="foo" static="true" value="10"/>

  <p:declare-step>
    $foo
  </p:declare-step>

  <p:declare-step>
    $foo
  </p:declare-step>
</p:library>

I assume the idea here is that $foo can be set once and used in multiple pipelines, which is handy.
How would that differ in practice from:

<p:library>
  <p:declare-step>
    <p:option name="foo" static="true" value="10"/>
  $foo
  </p:declare-step>

  <p:declare-step>
    <p:option name="foo" static="true" value="10"/>
    $foo
  </p:declare-step>
</p:library>

Note that the pipelines could not have any other inner declaration of $foo because the outer value would have been shadowed in that case.

And in any event, are we really sure that static values from the "outer context" at runtime flow through into pipelines nested in libraries nested in pipelines nested in libraries?

@xml-project

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

Just a suggestion without expressing any opinion (really, believe me).
Aligning with XSLT 3.0 may be an option because our audience know the rules.

@ndw

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2019

11 July editorial call: propose replace statics entirely with p:param(qname,defaultvalue,[type]) that returns a runtime value or the default. Norm to draft a PR that implements this for review.

@ndw

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2019

This removes static options entirely; static variables still exist but cannot be shadowed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.