-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
Define "undefined" and "implementation-defined" (3.0.4) #3779
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is confusing to me, both my general understanding of how the term 'undefined behavior' is used and as defined here. In particular the description "likely to contradict the specification" - my understanding has been that undefined behavior, either explicitly stated as undefined or by a lack of explicit specification, has no expectation for the outcome of that behavior. That would mean there's nothing specified for behavior to contradict.
And the statement "Implementations MAY support undefined scenarios" seems at odds with that behavior being likely to contradict specification. Combining the two, to my interpretation, results in "Implementations MAY have behavior that probably contradicts the specification" or so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having since been reading related issues that led to this (on structural interoperability), this is making more sense - it seems not the undefined behavior itself (treating references' resolved objects as one type when their structural placement indicates another type) that will contradict the spec, but the natural result of doing that in some circumstances (not detecting schema
$id
s, not having the correct base URI).Wording that distinguishes the undefined-but-allowed behavior from the possible specified-disallowed result might be clearer, though I'm not sure quite how that would be worded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @notEthan , I'll try to think about better wording.
That's specifically what I'm trying to avoid doing. Or, rather, I have laid out the specifics before and had people refuse to read it for being too long or complicated. One person complained that the topics are so obscure that "only four or five people" could probably even understand it.
It's much easier to say "just don't do anything like this" than try to explain all of the different things that have to happen for it to not work.
Allowing for implementations that do it anyway is just a concession to reality: Many tools do not implement 3.1 Schema Object reference parsing correctly, and are likely to ignore a directive to rip out their incorrect support. And given that it was not obvious to most how the specifications require different behavior in 3.1, I hesitate to penalize them.
If they have something that works for them and their customers, the most likely outcome is that they keep doing it. But changing the wording in the spec makes it clear that if there is a problem, it's not a spec problem (aside from past lack of clarity). It's outside the spec, and we know it won't work sometimes.
It's a difficult situation to articulate, really.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just "Behavior described as undefined is likely, at least in some circumstances, to result in outcomes that contradict the specification." or so. Undefined behavior itself isn't contradicting the spec but its result likely will - if that isn't too much indirection for one sentence.
I don't know, the wording that goes into spec authorship is hard (much respect for all you have put into various specs my work relies on).