Repository navigation
fix(semantics): conform to the type a filter names by element, not by name - #1365
Merged
Merged
Conversation
… name A filter condition, import filter, filtered expose and classification compiled the type they name to its qualified name and judged conformance by that name, so a Type declared twice under one qualified name (the same package opened in two documents) admitted elements the other annotated. The compiled predicate now carries the element the name resolved to and conformance is decided through its specialization closure; the name alone decides only when no element resolved, or across copies of a library. Co-Authored-By: jason.han <hanhuijun@gmail.com>
Contributor
Author
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
…red root does not collide Co-Authored-By: jason.han <hanhuijun@gmail.com>
… in the baseline Co-Authored-By: jason.han <hanhuijun@gmail.com>
…ready resolved to Co-Authored-By: jason.han <hanhuijun@gmail.com>
…res in the baseline Co-Authored-By: jason.han <hanhuijun@gmail.com>
…name A workspace namesake of a library type no longer conforms to it through the library-copy name fallback, and Base::Anything is the top type only as the library declares it, not as a workspace type of that name. Co-Authored-By: jason.han <hanhuijun@gmail.com>
…ment Co-Authored-By: jason.han <hanhuijun@gmail.com> # Conflicts: # docs/project/api-json-differential-baseline.json # docs/project/api-json-differential.md
Open
6 tasks done
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What and why
A filter condition (
filter @T;), an import filter (import P::*[@T]), a filtered expose (expose X::**[@T]) and a classification (@T,@@T) compiled the type they name to its qualified name and judged conformance by that name (conformsByName, comparing the owner chain of each supertype). Two consequences, both present ondevelopbefore and after #1349:metadata def Safety— were one type to a filter: a filter in document B naming itsMeta::Safetyadmitted an element document A annotated with A'sMeta::Safety. Shown wrong on develop bytests/testdata/filters/namesake_{a,b}.sysml:sysml -validateresolvedpart a :> belt;inClientBcleanly; it is now an unresolved reference, whileClientAbeside the annotating declaration still resolves it.aliasmember or a re-exporting import was conformed to by name only becauseresolveFQNhappened to canonicalize the alias before the name was taken. That case was not wrong on develop (tests/testdata/filters/alias_{member,import}.sysmlgive the same verdicts before and after), but it was correct by accident; it is now correct by construction and pinned.The compiled
FilterPredicatenow carries the element the name resolved to (FilterPredicate.Type), andannotationConformsdecides throughModel.Conformsover the specialization closure (element identity viasymbols.SameElement). The name alone decides only when the name resolved to no element, or when the named type is declared by a library document (copies of the standard library are the one normative element — this keepsTestExposedElementsFilterByAMetaclassDeclaredTwiceand the metaclass path, which is identified by its normative name, as they were).#1349 merged into
developwhile this was in flight; the branch was cut from its head and has mergedorigin/developsince, so it applies after #1349 and targetsdevelopdirectly. #1349's property — no name is built on a repeated conformance check — is kept:conformsByNameis unchanged andTestConformsByNameDoesNotBuildNamesstill passes;TestFilterNamingANamesakeTypeSelectsByElementadds the same zero-allocation assertion for the element path, in both the conforming and the rejecting direction.Specification basis
@Tclassifies by the TypeTnames and its specialization closure (§7.3.3.2 Specialization), not by its spelling.docs/project/spec-compliance.md(still ✅, now stating element identity and the library-copy rule).Behaviour changes
An element annotated by a Type no longer conforms to a filter naming a different Type that shares its qualified name (two documents each opening
Meta { metadata def Safety; }): the filter now admits only what the Type it resolves to annotates. Reference: KerML 1.0 §7.4.9, §7.3.3.2; SysML v2 §7.4.4, §8.3.26.No change for a type named through an alias or a re-exporting import (verified same verdicts on develop), for an unresolved name (still a
filter-not-evaluable/ unresolved diagnostic from the compile step), or for a metaclass classification (@SysML::PartUsage), which is still judged against the normative library element by name across copies of the library.The name fallback for library types is now
conformsByLibraryName: it matches only a library element, so two bundled copies of one declaration are still one type while a workspace namesake of a library type (package Meta { metadata def Safety; }in a workspace document beside the library's) no longer conforms to it; the metaclass path (@SysML::PartUsage) applies the same rule.Base::Anythingis the top type every type conforms to (Model.isTopType, KerML 1.0 §9.2.2.2) only as the library declares it; a workspace type of that name is an ordinary type and no longer admits every candidate. Pinned byTestFilterNamingALibraryTypeTellsAWorkspaceNamesakeApartandTestFilterNamingAWorkspaceAnythingIsNotTheTopType;TestConformsToAnythingnow marks its stand-inBasedocument as library content, which is the declaration the spec's rule is about.How it was verified
Before/after with
sysml -validateontests/testdata/filters/(develop build vs this branch): namesake fixtureClientB— develop: no errors; this branch:unresolved reference: belt;ClientA: resolves on both. Alias fixtures: identical verdicts on both (belt admitted, radio rejected through import filter,filtermember and filteredexpose). Each fixture uses its own package names so the four load together under one root without colliding.New tests:
internal/semantic/semantics/filter_identity_test.go(alias compiles to the aliased element; namesake rejects by element; zero allocations on the repeated check in both directions),internal/workspace/model/filter_conformance_test.goover the fixtures. The workspace namesake test fails on develop (got []) and passes here.go build ./...,go vet ./...,gofmt -l .(empty) andgo test -count=1 ./...pass locally with the training, pilot, spec-example and pilot-library-XMI corpora present (TestTrainingExamples,TestPilotCorpora,TestSpecExamples,TestPilotLibraryXMIall pass; no corpus expectation moves — the change only affects filters naming a type whose qualified name is declared twice, which no corpus file does).Pilot differential (
go run -C tools ./cmd/pilot-diff -update): the only movement is the four new fixtures undertests/testdata(32 → 36 files, 912 → 916 overall). Every added row agrees with the pilot:alias_member.sysmllines 17/23/29 andalias_import.sysmllines 17/23 (radiorejected by the alias-named filter on both sides),namesake_b.sysmlline 6 (beltrejected: the pilot too refuses to admit it through a filter naming the other document'sMeta::Safety),namesake_a.sysmlclean on both sides. Totals move 510 → 514 fully agreeing, 1085 → 1091 agreed, only-ours 366 and only-pilot 2445 unchanged; no pre-existing row moved. The document table, README headline and skill headlines are updated andmake docs-counts -check,tools/referee/diffguards pass.API-JSON differential baseline re-recorded on the merged tree (
pilot-apijson-diff -update, exporter rebuilt for the--extension-librarybridge; provenance guardTestCommittedBaselineStatesThisRepositorysProvenance). Movement against develop's recording, adjudicated per root:tests/testdatais this PR's — the four fixtures (32 → 36 files, 1201 → 1357 elements matched, 7837 → 8800 present-equal, 366 → 485 present-different, 13187 → 14752 ours-missing, 877 → 1015 pilot-missing); the two namesake fixtures are the two newoursCollidingdocuments (11 → 13 in that root, 24 → 26 overall): they declare the sameMeta::Safetyby design, so each converts alone, and the pilot reports the same ambiguous key.sysml-v2-training(typeMismatch 12 → 10),pilot-corpora/sysml-examples(25778 → 25864 matched, 182464 → 182942 present-equal, 9934 → 10084 present-different),pilot-corpora/sysml-validation(typeMismatch 17 → 15) andexamples(typeMismatch 200 → 186) are develop's own drift, not this change: the referee run from a bareorigin/develop(3e04a29) worktree over those four roots reproduces this PR's figures for each of them to the last field and differs from develop's committed recording in exactly those fields, so develop's recording was made before a later-merged change moved them.kerml-examples,spec-examples, the probes anddocs/manual/examplesdo not move. Headline 920 → 924 files, 90023 → 90265 elements matched. The record's headline, totals prose, per-root table and launch-ready gap table are refreshed from the baseline, and thetools/referee/apijsonguards pass.origin/developwas merged in again as a merge commit after fix(runtime): hold a calculation's result in its effective multiplicity on every surface #1308 and fix(libs): record a document whose metadata names undeclared types or elements #1314 landed, and once more after feat(export): own the referent Membership of a FeatureReferenceExpression where the notation places it #1331/fix(solve): read a |quoted| symbol back whole from the solver's model #1360 (3e04a29; the conflict was the API-JSON recording above, resolved by re-recording on the merged tree); the referee guards,make docs-counts -check,pilot-diff -check(916 files, 514 fully agreeing, reproduced),pilot-reject -check(334 cases: 323 both reject, 11 only ours, 0 only the pilot's, 0 both accept — the committed baseline, reproduced) and the full local gate were re-run on the merged tree.Checklist
make testandmake lintpass locallychanges/unreleased/<slug>.<section>.md, not as an edit toCHANGELOG.mdmake docs-countsrun if a gate count moved (compliance rows need nothing: the census is counted at docs build)F4,K5) in the body, docs, or changelog