Skip to content

Commit

Permalink
Encompass child node's location in parent.
Browse files Browse the repository at this point in the history
Closes #1893

When a child is added with `addChild`, the parent's location should
(generally) span over that child as well. This primarily helps for cases
where a node doesn't have much of a location until it gets children
added - like `AttributeSet`. The locations for those should encompass
all of the attributes within the set. That logic applies for any node
with a child: if it's the child, then its location should reflect that.
  • Loading branch information
evantypanski committed Oct 18, 2024
1 parent 5dd8fb3 commit adc5e8d
Show file tree
Hide file tree
Showing 11 changed files with 328 additions and 298 deletions.
5 changes: 4 additions & 1 deletion hilti/toolchain/include/ast/attribute.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@ class AttributeSet : public Node {
bool has(std::string_view tag) const { return find(tag) != nullptr; }

/** Adds an attribute to the set. */
void add(ASTContext* ctx, Attribute* a) { addChild(ctx, a); }
void add(ASTContext* ctx, Attribute* a) {
addChild(ctx, a);
combineLocations(a);
}

/** Removes all attributes of the given tag. */
void remove(std::string_view tag);
Expand Down
6 changes: 6 additions & 0 deletions hilti/toolchain/include/ast/location.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ class Location {
auto from() const { return _from_line; }
auto to() const { return _to_line; }

/**
* Merges this Location with the provided location. Returns new a location
* which encompasses the entire span.
*/
Location mergedRange(const Location& loc) const;

/**
* Returns a string representation of the location.
*
Expand Down
11 changes: 10 additions & 1 deletion hilti/toolchain/include/ast/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <memory>
#include <optional>
#include <ostream>
#include <set>
#include <string>
#include <unordered_set>
#include <utility>
Expand Down Expand Up @@ -502,6 +501,16 @@ class Node {
n->retain();
}

/**
* Updates this Node's location to encompass the other Node's location.
*
* @param other the node whose location will get merged
*/
void combineLocations(const Node* other) {
auto meta = Meta(_meta->location().mergedRange(other->location()), _meta->comments());
setMeta(std::move(meta));
}

/**
* Adds a series of child nodes. This operates like calling `addChild()` on
* each of them individually.
Expand Down
13 changes: 13 additions & 0 deletions hilti/toolchain/src/ast/location.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,19 @@ const Location location::None;

Location::operator bool() const { return _file != location::None._file; }

Location Location::mergedRange(const Location& loc) const {
if ( _file != loc._file )
return *this;

auto [from_line, from_character] =
std::min(std::tie(_from_line, _from_character), std::tie(loc._from_line, loc._from_character));

auto [to_line, to_character] =
std::max(std::tie(_to_line, _to_character), std::tie(loc._to_line, loc._to_character));

return Location(_file, from_line, to_line, from_character, to_character);
}

std::string Location::dump(bool no_path) const {
if ( ! *this )
return "<no location>";
Expand Down
3 changes: 1 addition & 2 deletions spicy/toolchain/src/compiler/validator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,7 @@ struct VisitorPost : visitor::PreOrder, hilti::validator::VisitorMixIn {
const bool isAlias = n->type()->print() != n->typeID().str();
if ( isAlias ) {
if ( ! n->attributes()->attributes().empty() )
// TODO(#1893): This should diagnose on the attribute set
error("attributes are not allow on type aliases", n);
error("attributes are not allowed on type aliases", n->attributes());
}

if ( n->linkage() == hilti::declaration::Linkage::Public && n->type()->alias() ) {
Expand Down
116 changes: 58 additions & 58 deletions tests/Baseline/hilti.ast.basic-module/debug.log

Large diffs are not rendered by default.

116 changes: 58 additions & 58 deletions tests/Baseline/hilti.ast.coercion/output

Large diffs are not rendered by default.

116 changes: 58 additions & 58 deletions tests/Baseline/hilti.ast.imported-id/output

Large diffs are not rendered by default.

116 changes: 58 additions & 58 deletions tests/Baseline/hilti.ast.types/output

Large diffs are not rendered by default.

116 changes: 58 additions & 58 deletions tests/Baseline/hilti.expressions.ctor-replacement/output

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
[error] <...>/invalid-type-attributes.spicy:11:1-11:23: attributes are not allow on type aliases
[error] <...>/invalid-type-attributes.spicy:12:1-12:23: attributes are not allow on type aliases
[error] <...>/invalid-type-attributes.spicy:15:1-15:46: attributes are not allow on type aliases
[error] <...>/invalid-type-attributes.spicy:16:1-16:53: attributes are not allow on type aliases
[error] <...>/invalid-type-attributes.spicy:11:17-11:22: attributes are not allowed on type aliases
[error] <...>/invalid-type-attributes.spicy:12:15-12:22: attributes are not allowed on type aliases
[error] <...>/invalid-type-attributes.spicy:15:28-15:45: attributes are not allowed on type aliases
[error] <...>/invalid-type-attributes.spicy:16:20-16:52: attributes are not allowed on type aliases
[error] spicyc: aborting after errors

0 comments on commit adc5e8d

Please sign in to comment.