Skip to content

Commit 326c94d

Browse files
jviottijviotti
authored
Turn SimpleOutput annotations into a vector (#916)
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
1 parent 4abcf44 commit 326c94d

6 files changed

Lines changed: 192 additions & 194 deletions

File tree

‎src/output/include/sourcemeta/blaze/output_simple.h‎

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
#include <map> // std::map
1717
#include <ostream> // std::ostream
1818
#include <string> // std::string
19-
#include <tuple> // std::tie
2019
#include <utility> // std::pair
2120
#include <vector> // std::vector
2221

@@ -83,6 +82,14 @@ class SOURCEMETA_BLAZE_OUTPUT_EXPORT SimpleOutput {
8382
std::reference_wrapper<const std::string> schema_location;
8483
};
8584

85+
/// A single collected annotation, in evaluation order
86+
struct AnnotationEntry {
87+
sourcemeta::core::WeakPointer instance_location;
88+
sourcemeta::core::WeakPointer evaluate_path;
89+
std::reference_wrapper<const std::string> schema_location;
90+
sourcemeta::core::JSON value;
91+
};
92+
8693
auto operator()(const EvaluationType type, const bool result,
8794
const Instruction &step,
8895
const InstructionExtra &step_metadata,
@@ -97,12 +104,6 @@ class SOURCEMETA_BLAZE_OUTPUT_EXPORT SimpleOutput {
97104
[[nodiscard]] auto cbegin() const -> const_iterator;
98105
[[nodiscard]] auto cend() const -> const_iterator;
99106

100-
/// Access annotations that were collected during evaluation, indexed by
101-
/// instance location and evaluation path
102-
[[nodiscard]] auto annotations() const -> const auto & {
103-
return this->annotations_;
104-
}
105-
106107
/// Move out the collected error entries, leaving this output empty. Useful to
107108
/// take ownership of the trace without copying when the output is no longer
108109
/// needed
@@ -114,22 +115,14 @@ class SOURCEMETA_BLAZE_OUTPUT_EXPORT SimpleOutput {
114115
return result;
115116
}
116117

117-
// NOLINTNEXTLINE(bugprone-exception-escape)
118-
struct Location {
119-
auto operator<(const Location &other) const noexcept -> bool {
120-
// Perform a lexicographical comparison
121-
return std::tie(this->instance_location, this->evaluate_path,
122-
this->schema_location.get()) <
123-
std::tie(other.instance_location, other.evaluate_path,
124-
other.schema_location.get());
125-
}
126-
127-
// NOLINTBEGIN(cppcoreguidelines-avoid-const-or-ref-data-members)
128-
const sourcemeta::core::WeakPointer instance_location;
129-
const sourcemeta::core::WeakPointer evaluate_path;
130-
const std::reference_wrapper<const std::string> schema_location;
131-
// NOLINTEND(cppcoreguidelines-avoid-const-or-ref-data-members)
132-
};
118+
/// Access the annotations collected during evaluation, as a flat log in
119+
/// evaluation order. The log records every emission as-is, so a location may
120+
/// repeat and hold the same value more than once. Consumers that need them
121+
/// grouped by location, or collapsed to distinct values, index this log
122+
/// themselves
123+
[[nodiscard]] auto annotations() const -> const auto & {
124+
return this->annotations_;
125+
}
133126

134127
private:
135128
// Exporting symbols that depends on the standard C++ library is considered
@@ -148,7 +141,7 @@ class SOURCEMETA_BLAZE_OUTPUT_EXPORT SimpleOutput {
148141
std::pair<sourcemeta::core::WeakPointer, sourcemeta::core::WeakPointer>,
149142
std::vector<Entry>>
150143
masked_traces;
151-
std::map<Location, std::vector<sourcemeta::core::JSON>> annotations_;
144+
std::vector<AnnotationEntry> annotations_;
152145
#if defined(_MSC_VER)
153146
#pragma warning(default : 4251)
154147
#endif

‎src/output/output_jsonld.cc‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99

1010
#include <algorithm> // std::ranges::sort, std::ranges::any_of
1111
#include <deque> // std::deque
12-
#include <functional> // std::ref
12+
#include <functional> // std::ref, std::reference_wrapper
13+
#include <map> // std::map
1314
#include <optional> // std::optional
1415
#include <sstream> // std::ostringstream
1516
#include <string> // std::string
@@ -400,6 +401,35 @@ auto array_of_nodes(const Accumulator &accumulator,
400401
return true;
401402
}
402403

404+
// NOLINTNEXTLINE(bugprone-exception-escape)
405+
struct AnnotationLocation {
406+
auto operator<(const AnnotationLocation &other) const noexcept -> bool {
407+
return std::tie(this->instance_location, this->evaluate_path,
408+
this->schema_location.get()) <
409+
std::tie(other.instance_location, other.evaluate_path,
410+
other.schema_location.get());
411+
}
412+
413+
sourcemeta::core::WeakPointer instance_location;
414+
sourcemeta::core::WeakPointer evaluate_path;
415+
std::reference_wrapper<const std::string> schema_location;
416+
};
417+
418+
auto group_annotations(const sourcemeta::blaze::SimpleOutput &output)
419+
-> std::map<AnnotationLocation, std::vector<sourcemeta::core::JSON>> {
420+
std::map<AnnotationLocation, std::vector<sourcemeta::core::JSON>> result;
421+
for (const auto &entry : output.annotations()) {
422+
auto &values{result[{.instance_location = entry.instance_location,
423+
.evaluate_path = entry.evaluate_path,
424+
.schema_location = entry.schema_location}]};
425+
if (values.empty() || values.back() != entry.value) {
426+
values.push_back(entry.value);
427+
}
428+
}
429+
430+
return result;
431+
}
432+
403433
// Turn the JSON-LD annotations into a resolved map, or the first error found.
404434
// The first pass groups the facts by location, the second derives each kind
405435
// from the value shape and validates the facts against it
@@ -409,7 +439,7 @@ auto resolve(const sourcemeta::core::JSON &instance,
409439
sourcemeta::blaze::JSONLDResolutionError> {
410440
Accumulator accumulator;
411441

412-
for (const auto &[location, values] : output.annotations()) {
442+
for (const auto &[location, values] : group_annotations(output)) {
413443
if (location.evaluate_path.empty()) {
414444
continue;
415445
}

‎src/output/output_simple.cc‎

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -66,18 +66,11 @@ auto SimpleOutput::operator()(
6666

6767
if (is_annotation(step.type)) {
6868
if (type == EvaluationType::Post) {
69-
Location location{.instance_location = instance_location,
70-
.evaluate_path = std::move(effective_evaluate_path),
71-
.schema_location = step_metadata.keyword_location};
72-
const auto match{this->annotations_.find(location)};
73-
if (match == this->annotations_.cend()) {
74-
this->annotations_[std::move(location)].push_back(annotation);
75-
76-
// To avoid emitting the exact same annotation more than once
77-
// This is right now mostly because of `unevaluatedItems`
78-
} else if (match->second.back() != annotation) {
79-
match->second.push_back(annotation);
80-
}
69+
this->annotations_.push_back(
70+
{.instance_location = instance_location,
71+
.evaluate_path = std::move(effective_evaluate_path),
72+
.schema_location = step_metadata.keyword_location,
73+
.value = annotation});
8174
}
8275

8376
return;
@@ -122,15 +115,11 @@ auto SimpleOutput::operator()(
122115
}
123116

124117
if (type == EvaluationType::Post && !this->annotations_.empty()) {
125-
for (auto iterator = this->annotations_.begin();
126-
iterator != this->annotations_.end();) {
127-
if (iterator->first.evaluate_path.starts_with_initial(evaluate_path) &&
128-
iterator->first.instance_location == instance_location) {
129-
iterator = this->annotations_.erase(iterator);
130-
} else {
131-
++iterator;
132-
}
133-
}
118+
std::erase_if(
119+
this->annotations_, [&](const AnnotationEntry &entry) -> bool {
120+
return entry.evaluate_path.starts_with_initial(evaluate_path) &&
121+
entry.instance_location == instance_location;
122+
});
134123
}
135124

136125
if (keyword == "if") {

‎src/output/output_standard.cc‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,49 @@
11
#include <sourcemeta/blaze/output_simple.h>
22
#include <sourcemeta/blaze/output_standard.h>
33

4+
#include <sourcemeta/core/json.h>
45
#include <sourcemeta/core/jsonpointer.h>
56

67
#include <cassert> // assert
7-
#include <functional> // std::ref
8+
#include <functional> // std::ref, std::reference_wrapper
9+
#include <map> // std::map
10+
#include <string> // std::string
11+
#include <tuple> // std::tie
12+
#include <vector> // std::vector
813

914
namespace sourcemeta::blaze {
1015

1116
namespace {
1217

18+
// NOLINTNEXTLINE(bugprone-exception-escape)
19+
struct AnnotationLocation {
20+
auto operator<(const AnnotationLocation &other) const noexcept -> bool {
21+
return std::tie(this->instance_location, this->evaluate_path,
22+
this->schema_location.get()) <
23+
std::tie(other.instance_location, other.evaluate_path,
24+
other.schema_location.get());
25+
}
26+
27+
sourcemeta::core::WeakPointer instance_location;
28+
sourcemeta::core::WeakPointer evaluate_path;
29+
std::reference_wrapper<const std::string> schema_location;
30+
};
31+
32+
auto group_annotations(const SimpleOutput &output)
33+
-> std::map<AnnotationLocation, std::vector<sourcemeta::core::JSON>> {
34+
std::map<AnnotationLocation, std::vector<sourcemeta::core::JSON>> result;
35+
for (const auto &entry : output.annotations()) {
36+
auto &values{result[{.instance_location = entry.instance_location,
37+
.evaluate_path = entry.evaluate_path,
38+
.schema_location = entry.schema_location}]};
39+
if (values.empty() || values.back() != entry.value) {
40+
values.push_back(entry.value);
41+
}
42+
}
43+
44+
return result;
45+
}
46+
1347
auto handle_standard(Evaluator &evaluator, const Template &schema,
1448
const sourcemeta::core::JSON &instance,
1549
const StandardOutput format,
@@ -30,7 +64,7 @@ auto handle_standard(Evaluator &evaluator, const Template &schema,
3064
auto result{sourcemeta::core::JSON::make_object()};
3165
result.assign_assume_new("valid", sourcemeta::core::JSON{valid});
3266
auto annotations{sourcemeta::core::JSON::make_array()};
33-
for (const auto &annotation : output.annotations()) {
67+
for (const auto &annotation : group_annotations(output)) {
3468
auto unit{sourcemeta::core::JSON::make_object()};
3569
unit.assign_assume_new(
3670
"keywordLocation",

‎test/evaluator/annotationsuite.cc‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
#include <sourcemeta/core/json.h>
1010
#include <sourcemeta/core/jsonpointer.h>
1111

12-
#include <algorithm>
1312
#include <cassert>
1413
#include <filesystem>
1514
#include <functional>
@@ -61,16 +60,15 @@ has_annotation(const sourcemeta::blaze::SimpleOutput &output,
6160

6261
for (const auto &annotation : output.annotations()) {
6362
std::cerr << "Checking against annotations for instance location \""
64-
<< sourcemeta::core::to_string(annotation.first.instance_location)
65-
<< "\" and schema location \""
66-
<< annotation.first.schema_location.get() << "\"" << "\n";
63+
<< sourcemeta::core::to_string(annotation.instance_location)
64+
<< "\" and schema location \"" << annotation.schema_location.get()
65+
<< "\"" << "\n";
6766

68-
if (annotation.first.instance_location != instance_location ||
69-
!schema_location_matches(annotation.first.schema_location.get(),
67+
if (annotation.instance_location != instance_location ||
68+
!schema_location_matches(annotation.schema_location.get(),
7069
schema_location, frame)) {
7170
continue;
72-
} else if (std::find(annotation.second.cbegin(), annotation.second.cend(),
73-
value) != annotation.second.cend()) {
71+
} else if (annotation.value == value) {
7472
return true;
7573
}
7674
}
@@ -88,15 +86,15 @@ has_matching_annotations(const sourcemeta::blaze::SimpleOutput &output,
8886

8987
for (const auto &annotation : output.annotations()) {
9088
std::cerr << "Checking against annotations for instance location \""
91-
<< sourcemeta::core::to_string(annotation.first.instance_location)
89+
<< sourcemeta::core::to_string(annotation.instance_location)
9290
<< "\" and evaluate path \""
93-
<< sourcemeta::core::to_string(annotation.first.evaluate_path)
94-
<< "\"" << "\n";
91+
<< sourcemeta::core::to_string(annotation.evaluate_path) << "\""
92+
<< "\n";
9593

96-
if (annotation.first.instance_location == instance_location &&
97-
!annotation.first.evaluate_path.empty() &&
98-
annotation.first.evaluate_path.back().is_property() &&
99-
annotation.first.evaluate_path.back().to_property() == keyword) {
94+
if (annotation.instance_location == instance_location &&
95+
!annotation.evaluate_path.empty() &&
96+
annotation.evaluate_path.back().is_property() &&
97+
annotation.evaluate_path.back().to_property() == keyword) {
10098
return true;
10199
}
102100
}

0 commit comments

Comments
 (0)