Sourcemaps WIP - #432
Conversation
|
Most of the way to printing. TODO: VLQs, parsing. Now would be a good time for a round of review. |
|
Sorry, got started looking at this then got distracted. :-) |
binji
left a comment
binji
left a comment
There was a problem hiding this comment.
Looking good, I didn't realize all the neat ways you can use the {} initialization syntax!
| assert(map.segment_groups.empty() || | ||
| mapping.generated.line > last_gen_line); // Not sorted. | ||
| while (++last_gen_line <= mapping.generated.line) { | ||
| map.segment_groups.push_back( |
There was a problem hiding this comment.
Why not emplace_back and drop the {}?
| return true; | ||
| } | ||
|
|
||
| static int32_t cmpLocation(const SourceMapGenerator::SourceLocation& a, |
There was a problem hiding this comment.
We're still pretty inconsistent, but I think this should eventually be CmpLocation
| bool SourceMap::Validate(bool fatal) const { | ||
| for (size_t i = 0; i < segment_groups.size(); ++i) { | ||
| const auto& group = segment_groups[i]; | ||
| if (i > 0 && group.generated_line <= segment_groups[i - 1].generated_line) { |
There was a problem hiding this comment.
I've been writing this sort of thing as CHECK_FOO(<cond>) -- I find it more readable, but curious what you think.
| uint32_t last_source_line = 0; | ||
| uint32_t last_source_col = 0; | ||
| map.segment_groups.clear(); | ||
| const SourceMapGenerator::SourceMapping* last_mapping = nullptr; |
There was a problem hiding this comment.
could drop SourceMapGenerator right?
| private: | ||
| void CompressMappings(); | ||
|
|
||
| bool map_prepared = false; // Is the map compressed and ready for export? |
| } | ||
|
|
||
| void SourceMapGenerator::CompressMappings() { | ||
| std::sort(mappings.begin(), mappings.end()); |
There was a problem hiding this comment.
It's convenient for being self contained, but how likely are we to add mappings out of order? Seems like a lot of this could be simplified if we could assume that AddMapping only can add mappings in order.
| // Representation of mappings | ||
| struct Segment { | ||
| // Field 1 | ||
| uint32_t generated_col = 0; // Start column in generated code. Remove? |
There was a problem hiding this comment.
Yeah, seems like these aren't really necessary since you can always calculate them. Maybe better to just add functions if they're needed?
| } | ||
|
|
||
| void SourceMapGenerator::DumpRawMappings() { | ||
| std::sort(mappings.begin(), mappings.end()); |
There was a problem hiding this comment.
It's a bit weird that a dumping function would modify the mappings.
| TEST(source_maps, serialization_empty) { | ||
| SourceMapGenerator smg("source.out", "source-root"); | ||
| std::string output = smg.SerializeMappings(); | ||
| EXPECT_JSON_CONTAINS_NUM(output, "version", "3"); |
There was a problem hiding this comment.
seems like it would be better to just parse the json here.
2 participants
Not ready to go yet, but just to get this up here.
API is a little rough, and no support for serialization/deserialization yet, but has the basic functions of adding mappings and generating segments. Also tests.