Skip to content

Commit c8aecb9

Browse files
richmckeevercopybara-github
authored andcommitted
Simplify the creation of type variables in populate_table_visitor.cc
This buries the mechanics of creating type variables behind some helper functions without changing the logic, except that we now have more descriptive variable names generated. This change also exposed a type_info_to_proto test case with an internal variable erroneously in the output, which is fixed here. PiperOrigin-RevId: 809151498
1 parent e20e2ab commit c8aecb9

File tree

6 files changed

+175
-399
lines changed

6 files changed

+175
-399
lines changed

xls/dslx/frontend/ast.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,9 +1069,10 @@ std::string TypeRefTypeAnnotation::ToString() const {
10691069
// -- class TypeVariableTypeAnnotation
10701070

10711071
TypeVariableTypeAnnotation::TypeVariableTypeAnnotation(
1072-
Module* owner, const NameRef* type_variable)
1072+
Module* owner, const NameRef* type_variable, bool internal)
10731073
: TypeAnnotation(owner, type_variable->span(), kAnnotationKind),
1074-
type_variable_(type_variable) {}
1074+
type_variable_(type_variable),
1075+
internal_(internal) {}
10751076

10761077
std::string TypeVariableTypeAnnotation::ToString() const {
10771078
return absl::StrCat("TypeVariableTypeAnnotation: ",

xls/dslx/frontend/ast.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -513,15 +513,17 @@ class TypeVariableTypeAnnotation : public TypeAnnotation {
513513
static constexpr TypeAnnotationKind kAnnotationKind =
514514
TypeAnnotationKind::kTypeVariable;
515515

516-
explicit TypeVariableTypeAnnotation(Module* owner,
517-
const NameRef* type_variable);
516+
TypeVariableTypeAnnotation(Module* owner, const NameRef* type_variable,
517+
bool internal = true);
518518

519519
// Returns a `NameRef` for the type variable indicated by this annotation. The
520520
// variable may be a type parametric or an internally-defined type variable.
521521
const NameRef* type_variable() const { return type_variable_; }
522522

523523
std::string ToString() const override;
524524

525+
bool internal() const { return internal_; }
526+
525527
std::string_view GetNodeTypeName() const override {
526528
return "TypeVariableTypeAnnotation";
527529
}
@@ -538,6 +540,7 @@ class TypeVariableTypeAnnotation : public TypeAnnotation {
538540

539541
private:
540542
const NameRef* const type_variable_;
543+
bool internal_;
541544
};
542545

543546
// Represents the type of a member of a struct, like

xls/dslx/frontend/module.h

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "absl/strings/str_format.h"
3737
#include "absl/strings/str_split.h"
3838
#include "absl/types/span.h"
39+
#include "xls/common/casts.h"
3940
#include "xls/dslx/frontend/ast.h"
4041
#include "xls/dslx/frontend/pos.h"
4142
#include "xls/dslx/frontend/proc.h"
@@ -198,11 +199,22 @@ class Module : public AstNode {
198199
// internal nodes in some contexts, e.g. in language server adapter
199200
// functionality.
200201
bool IsSyntheticNode(const AstNode* node) const {
201-
// Note that ColonRef is a special case here, because when it's the
202-
// definition in a `TypeRef`, the parser actually creates it as a
203-
// disconnected node.
204-
return node->parent() == nullptr &&
205-
node->kind() != AstNodeKind::kColonRef && !top_set_.contains(node);
202+
// It's synthetic if it's a top-level node that is not in the top set (the
203+
// set is populated by the parser). Note that ColonRef is a special case
204+
// here, because when it's the definition in a `TypeRef`, the parser
205+
// actually creates it as a disconnected node.
206+
if (node->parent() == nullptr && node->kind() != AstNodeKind::kColonRef &&
207+
!top_set_.contains(node)) {
208+
return true;
209+
}
210+
211+
// It's synthetic if it's an internally-generated TVTA. Note that there are
212+
// user-written TVTAs, which are entities whose type is a `T: type` kind of
213+
// parametric, and these are not marked internal.
214+
return node->kind() == AstNodeKind::kTypeAnnotation &&
215+
down_cast<const TypeAnnotation*>(node)->annotation_kind() ==
216+
TypeAnnotationKind::kTypeVariable &&
217+
down_cast<const TypeVariableTypeAnnotation*>(node)->internal();
206218
}
207219

208220
// Finds the first top-level member in top() with the given "target" name as

xls/dslx/frontend/parser.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1096,7 +1096,8 @@ absl::StatusOr<TypeRefOrAnnotation> Parser::ParseTypeRef(Bindings& bindings,
10961096
<< name_def->definer()->ToString();
10971097

10981098
return module_->Make<TypeVariableTypeAnnotation>(
1099-
module_->Make<NameRef>(tok.span(), name_def->identifier(), name_def));
1099+
module_->Make<NameRef>(tok.span(), name_def->identifier(), name_def),
1100+
/*internal=*/false);
11001101
}
11011102
}
11021103
if (!IsOneOf<TypeAlias, EnumDef, StructDef, ProcDef, UseTreeEntry>(

xls/dslx/type_system/testdata/type_info_to_proto_test_v2_BitsConstructorTypeProto.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@
2626
5:23-5:28: TYPE_ANNOTATION :: `u2[2]` :: typeof(uN[2][2])
2727
5:26-5:27: NUMBER :: `2` :: uN[32]
2828
5:29-5:35: ARRAY :: `u2[2]:[1, 1]` :: uN[2][2]
29-
5:29-5:35: TYPE_ANNOTATION :: `TypeVariableTypeAnnotation: internal_type_expr_at_fake.x:6:30-6:36_in_fake_actual_arg_0` :: typeof(uN[2])
30-
5:29-5:35: TYPE_ANNOTATION :: `TypeVariableTypeAnnotation: internal_type_expr_at_fake.x:6:30-6:36_in_fake_actual_arg_0` :: typeof(uN[2])
29+
5:29-5:35: TYPE_ANNOTATION :: `u2` :: typeof(uN[2])
30+
5:29-5:35: TYPE_ANNOTATION :: `u2` :: typeof(uN[2])
3131
5:30-5:31: NUMBER :: `1` :: uN[2]
3232
5:33-5:34: NUMBER :: `1` :: uN[2]
3333
5:37-5:41: TYPE_ANNOTATION :: `bool` :: typeof(uN[1])

0 commit comments

Comments
 (0)