diff options
| author | ArielG-NV <159081215+ArielG-NV@users.noreply.github.com> | 2024-07-23 09:36:38 -0400 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-07-23 09:36:38 -0400 |
| commit | 509bfd8bbaaf021507c4045b5fd9eaf43276dc0a (patch) | |
| tree | 7adab692d0ec55935d8372d9422b7a653068251a /source/slang/slang-capability.cpp | |
| parent | 15f091aabf5b6f8d37d292dab150395d8a37d644 (diff) | |
Simplify `CapabilitySet` Diagnostic Printing (#4678)
Fixes: #4675
Fixes: #4683
Fixes: #4443
Fixes: #4585
Fixes: #4172
Made the following changes:
1. All capability diagnostic printing logic tries to simplify before printing. This means that we do not print atoms which imply another atom.
2. Do not print the `_` prefix part of atom names since it is misleading users on what they should use to solve a capability issue encountered. (`_Internal` `External` atom changes are not in this PR)
3. Bundle together printing of all sets which contain exactly the same atoms (excluding abstract atoms). This allows printing the following `vertex/fragment/hull/domain/... + glsl` instead of `vertex + glsl | fragment + glsl | hull + glsl | domain + glsl | ....`
4. Rework how entry-point errors are reported to users (example at bottom of PR comment)
5. Rework how atom-provenance data is collected to be leaner and more useful so we can rework the errors. There are 2 notable changes here:
* We no longer store a list which describes where the first of an `CapabilityAtom` comes from. This heavily simplifies AST logic for the capability system. AST parsing of capabilities is much faster. The trade-off is faster AST parsing and correct AST node data for slower diagnostics if an error is found
* atom-provenance data now stores a reference to an atom's use-site to provide information on **where** and **what** is wrong with user code versus only sharing **what** and not where.
Diffstat (limited to 'source/slang/slang-capability.cpp')
| -rw-r--r-- | source/slang/slang-capability.cpp | 183 |
1 files changed, 158 insertions, 25 deletions
diff --git a/source/slang/slang-capability.cpp b/source/slang/slang-capability.cpp index d9201dad6..f06ccb663 100644 --- a/source/slang/slang-capability.cpp +++ b/source/slang/slang-capability.cpp @@ -165,6 +165,57 @@ bool isCapabilityDerivedFrom(CapabilityAtom atom, CapabilityAtom base) return false; } +//CapabilityAtomSet + +CapabilityAtomSet CapabilityAtomSet::newSetWithoutImpliedAtoms() const +{ + // plan is to add all atoms which is impled (=>) another atom. + // Implying an atom appears in the form of atom1=>atom2 or atom2=>atom1. + Dictionary<CapabilityAtom, bool> candidateForSimplifiedList; + CapabilityAtomSet simplifiedSet; + for (auto atom1UInt : *this) + { + CapabilityAtom atom1 = (CapabilityAtom)atom1UInt; + if (!candidateForSimplifiedList.addIfNotExists(atom1, true) + && candidateForSimplifiedList[atom1] == false) + continue; + + for (auto atom2UInt : *this) + { + if (atom1UInt == atom2UInt) + continue; + + CapabilityAtom atom2 = (CapabilityAtom)atom2UInt; + if (!candidateForSimplifiedList.addIfNotExists(atom2, true) + && candidateForSimplifiedList[atom2] == false) + continue; + + auto atomInfo1 = _getInfo(atom1).canonicalRepresentation; + auto atomInfo2 = _getInfo(atom2).canonicalRepresentation; + for (auto atomSet1 : atomInfo1) + { + for (auto atomSet2 : atomInfo2) + { + if (atomSet1->contains(*atomSet2)) + { + candidateForSimplifiedList[atom2] = false; + continue; + } + else if (atomSet2->contains(*atomSet1)) + { + candidateForSimplifiedList[atom1] = false; + continue; + } + } + } + } + } + for (auto i : candidateForSimplifiedList) + if(i.second) + simplifiedSet.add((UInt)i.first); + return simplifiedSet; +} + //// CapabiltySet CapabilityAtom getTargetAtomInSet(const CapabilityAtomSet& atomSet) @@ -897,31 +948,118 @@ void CapabilitySet::addSpirvVersionFromOtherAsGlslSpirvVersion(CapabilitySet& ot } } -void printDiagnosticArg(StringBuilder& sb, const CapabilitySet& capSet) +UnownedStringSlice capabilityNameToStringWithoutPrefix(CapabilityName capabilityName) +{ + auto name = capabilityNameToString(capabilityName); + if (name.startsWith("_")) + return name.tail(1); + return name; +} + +void printDiagnosticArg(StringBuilder& sb, const CapabilityAtomSet atomSet) +{ + bool isFirst = true; + for (auto atom : atomSet.newSetWithoutImpliedAtoms()) + { + CapabilityName formattedAtom = (CapabilityName)atom; + if (!isFirst) + sb << " + "; + sb << capabilityNameToStringWithoutPrefix(formattedAtom); + isFirst = false; + } +} + +// Collection of stages which have same atom sets to compress reprisentation of atom and stage per target +struct CompressedCapabilitySet { - bool isFirstSet = true; - for (auto& set : capSet.getAtomSets()) + /// Collection of stages which have same atom sets to compress reprisentation of atom and stage: {vertex/fragment, ... } + struct StageAndAtomSet + { + CapabilityAtomSet stages; + CapabilityAtomSet atomsWithoutStage; + }; + + auto begin() { - if (!isFirstSet) + return atomSetsOfTargets.begin(); + } + + /// Compress 1 capabilitySet into a reprisentation which merges stages that share all of their atoms for printing. + Dictionary<CapabilityAtom, List<StageAndAtomSet>> atomSetsOfTargets; + CompressedCapabilitySet(const CapabilitySet& capabilitySet) + { + for (auto& atomSet : capabilitySet.getAtomSets()) { - sb<< " | "; + auto target = getTargetAtomInSet(atomSet); + + auto stageInSetAtom = getStageAtomInSet(atomSet); + CapabilityAtomSet stageInSet; + stageInSet.add((UInt)stageInSetAtom); + + CapabilityAtomSet atomsWithoutStage; + CapabilityAtomSet::calcSubtract(atomsWithoutStage, atomSet, stageInSet); + if (!atomSetsOfTargets.containsKey(target)) + { + atomSetsOfTargets[target].add({ stageInSet, atomsWithoutStage }); + continue; + } + + // try to find an equivlent atom set by iterating all of the same `atomSetsOfTarget[target]` and merge these 2 together. + auto& atomSetsOfTarget = atomSetsOfTargets[target]; + for (auto& i : atomSetsOfTarget) + { + if (i.atomsWithoutStage.contains(atomsWithoutStage) && atomsWithoutStage.contains(i.atomsWithoutStage)) + { + i.stages.add((UInt)stageInSetAtom); + } + } } - bool isFirst = true; - for (auto atom : set) + for (auto& targetSets : atomSetsOfTargets) + for (auto& targetSet : targetSets.second) + targetSet.atomsWithoutStage = targetSet.atomsWithoutStage.newSetWithoutImpliedAtoms(); + } +}; + +void printDiagnosticArg(StringBuilder& sb, const CompressedCapabilitySet& capabilitySet) +{ + ////Secondly we will print our new list of atomSet's. + sb << "{"; + bool firstSet = true; + for (auto targetSets : capabilitySet.atomSetsOfTargets) + { + if(!firstSet) + sb << " || "; + for (auto targetSet : targetSets.second) { - CapabilityName formattedAtom = (CapabilityName)atom; - if (!isFirst) + bool firstStage = true; + for (auto stageAtom : targetSet.stages) + { + if (!firstStage) + sb << "/"; + printDiagnosticArg(sb, (CapabilityName)stageAtom); + firstStage = false; + } + for (auto atom : targetSet.atomsWithoutStage) { sb << " + "; + printDiagnosticArg(sb, (CapabilityName)atom); } - auto name = capabilityNameToString((CapabilityName)formattedAtom); - if (name.startsWith("_")) - name = name.tail(1); - sb << name; - isFirst = false; } - isFirstSet = false; + firstSet = false; } + sb << "}"; +} + +void printDiagnosticArg(StringBuilder& sb, const CapabilitySet& capabilitySet) +{ + // Firstly we will compress the printing of capabilities such that any atomSet + // with different abstract atoms but equal non-abstract atoms will be bundled together. + if (capabilitySet.isInvalid() || capabilitySet.isEmpty()) + { + sb << "{}"; + return; + } + printDiagnosticArg(sb, CompressedCapabilitySet(capabilitySet)); } void printDiagnosticArg(StringBuilder& sb, CapabilityAtom atom) @@ -931,20 +1069,15 @@ void printDiagnosticArg(StringBuilder& sb, CapabilityAtom atom) void printDiagnosticArg(StringBuilder& sb, CapabilityName name) { - sb << _getInfo(name).name; + sb << capabilityNameToStringWithoutPrefix(name); } void printDiagnosticArg(StringBuilder& sb, List<CapabilityAtom>& list) { - sb << "{"; - auto count = list.getCount(); - for(Index i = 0; i < count; i++) - { - printDiagnosticArg(sb, list[i]); - if (i + 1 != count) - sb << ", "; - } - sb << "}"; + CapabilityAtomSet set; + for (auto i : list) + set.add((UInt)i); + printDiagnosticArg(sb, set.newSetWithoutImpliedAtoms()); } |
