Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate Declaration to newer property style, class-ify ClassDefinition #859

Merged
merged 2 commits into from
Sep 29, 2021
Merged

Migrate Declaration to newer property style, class-ify ClassDefinition #859

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
474b255
Migrate Declaration to newer property style, class-ify ClassDefinition
jonmeow Sep 28, 2021
2eaf6e3
tag kind
jonmeow Sep 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions executable_semantics/ast/class_definition.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,22 @@

namespace Carbon {

struct ClassDefinition {
SourceLocation loc;
std::string name;
std::vector<Nonnull<Member*>> members;
class ClassDefinition {
public:
ClassDefinition(SourceLocation source_loc, std::string name,
std::vector<Nonnull<Member*>> members)
: source_loc_(source_loc),
name_(std::move(name)),
members_(std::move(members)) {}

auto source_loc() const -> SourceLocation { return source_loc_; }
auto name() const -> const std::string& { return name_; }
auto members() const -> llvm::ArrayRef<Nonnull<Member*>> { return members_; }

private:
SourceLocation source_loc_;
std::string name_;
std::vector<Nonnull<Member*>> members_;
};

} // namespace Carbon
Expand Down
16 changes: 8 additions & 8 deletions executable_semantics/ast/declaration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@ namespace Carbon {
using llvm::cast;

void Declaration::Print(llvm::raw_ostream& out) const {
switch (Tag()) {
switch (tag()) {
case Kind::FunctionDeclaration:
out << cast<FunctionDeclaration>(*this).Definition();
out << cast<FunctionDeclaration>(*this).definition();
break;

case Kind::ClassDeclaration: {
const ClassDefinition& class_def =
cast<ClassDeclaration>(*this).Definition();
out << "class " << class_def.name << " {\n";
for (Nonnull<Member*> m : class_def.members) {
cast<ClassDeclaration>(*this).definition();
out << "class " << class_def.name() << " {\n";
for (Nonnull<Member*> m : class_def.members()) {
out << *m;
}
out << "}\n";
Expand All @@ -29,8 +29,8 @@ void Declaration::Print(llvm::raw_ostream& out) const {

case Kind::ChoiceDeclaration: {
const auto& choice = cast<ChoiceDeclaration>(*this);
out << "choice " << choice.Name() << " {\n";
for (const auto& alt : choice.Alternatives()) {
out << "choice " << choice.name() << " {\n";
for (const auto& alt : choice.alternatives()) {
out << "alt " << alt.name() << " " << alt.signature() << ";\n";
}
out << "}\n";
Expand All @@ -39,7 +39,7 @@ void Declaration::Print(llvm::raw_ostream& out) const {

case Kind::VariableDeclaration: {
const auto& var = cast<VariableDeclaration>(*this);
out << "var " << *var.Binding() << " = " << *var.Initializer() << "\n";
out << "var " << var.binding() << " = " << var.initializer() << "\n";
break;
}
}
Expand Down
84 changes: 42 additions & 42 deletions executable_semantics/ast/declaration.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,60 +40,59 @@ class Declaration {
Declaration(const Member&) = delete;
Declaration& operator=(const Member&) = delete;

void Print(llvm::raw_ostream& out) const;

// Returns the enumerator corresponding to the most-derived type of this
// object.
auto Tag() const -> Kind { return tag; }
auto tag() const -> Kind { return tag_; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we call this kind instead? That's what I wanted to call these methods originally, if not for the name collision with the Kind type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, #860 covers this but may as well reduce merge conflicts here.


auto SourceLoc() const -> SourceLocation { return loc; }

void Print(llvm::raw_ostream& out) const;
auto source_loc() const -> SourceLocation { return source_loc_; }

protected:
// Constructs a Declaration representing syntax at the given line number.
// `tag` must be the enumerator corresponding to the most-derived type being
// constructed.
Declaration(Kind tag, SourceLocation loc) : tag(tag), loc(loc) {}
Declaration(Kind tag, SourceLocation source_loc)
: tag_(tag), source_loc_(source_loc) {}

private:
const Kind tag;
SourceLocation loc;
const Kind tag_;
SourceLocation source_loc_;
};

class FunctionDeclaration : public Declaration {
public:
FunctionDeclaration(Nonnull<FunctionDefinition*> definition)
: Declaration(Kind::FunctionDeclaration, definition->source_loc()),
definition(definition) {}
definition_(definition) {}

static auto classof(const Declaration* decl) -> bool {
return decl->Tag() == Kind::FunctionDeclaration;
return decl->tag() == Kind::FunctionDeclaration;
}

auto Definition() const -> const FunctionDefinition& { return *definition; }
auto Definition() -> FunctionDefinition& { return *definition; }
auto definition() const -> const FunctionDefinition& { return *definition_; }
auto definition() -> FunctionDefinition& { return *definition_; }

private:
Nonnull<FunctionDefinition*> definition;
Nonnull<FunctionDefinition*> definition_;
};

class ClassDeclaration : public Declaration {
public:
ClassDeclaration(SourceLocation loc, std::string name,
ClassDeclaration(SourceLocation source_loc, std::string name,
std::vector<Nonnull<Member*>> members)
: Declaration(Kind::ClassDeclaration, loc),
definition({.loc = loc,
.name = std::move(name),
.members = std::move(members)}) {}
: Declaration(Kind::ClassDeclaration, source_loc),
definition_(source_loc, std::move(name), std::move(members)) {}

static auto classof(const Declaration* decl) -> bool {
return decl->Tag() == Kind::ClassDeclaration;
return decl->tag() == Kind::ClassDeclaration;
}

auto Definition() const -> const ClassDefinition& { return definition; }
auto Definition() -> ClassDefinition& { return definition; }
auto definition() const -> const ClassDefinition& { return definition_; }
auto definition() -> ClassDefinition& { return definition_; }

private:
ClassDefinition definition;
ClassDefinition definition_;
};

class ChoiceDeclaration : public Declaration {
Expand All @@ -111,50 +110,51 @@ class ChoiceDeclaration : public Declaration {
Nonnull<Expression*> signature_;
};

ChoiceDeclaration(SourceLocation loc, std::string name,
ChoiceDeclaration(SourceLocation source_loc, std::string name,
std::vector<Alternative> alternatives)
: Declaration(Kind::ChoiceDeclaration, loc),
name(std::move(name)),
alternatives(std::move(alternatives)) {}
: Declaration(Kind::ChoiceDeclaration, source_loc),
name_(std::move(name)),
alternatives_(std::move(alternatives)) {}

static auto classof(const Declaration* decl) -> bool {
return decl->Tag() == Kind::ChoiceDeclaration;
return decl->tag() == Kind::ChoiceDeclaration;
}

auto Name() const -> const std::string& { return name; }
auto Alternatives() const -> const std::vector<Alternative>& {
return alternatives;
auto name() const -> const std::string& { return name_; }
auto alternatives() const -> llvm::ArrayRef<Alternative> {
return alternatives_;
Comment on lines +124 to +125
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this have a mutable overload?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference is to do a mutable overload on an as-needed basis. I expect one to be added, it just didn't need one as of yet, I assume because code wasn't implemented to need it.

}

private:
std::string name;
std::vector<Alternative> alternatives;
std::string name_;
std::vector<Alternative> alternatives_;
};

// Global variable definition implements the Declaration concept.
class VariableDeclaration : public Declaration {
public:
VariableDeclaration(SourceLocation loc, Nonnull<BindingPattern*> binding,
VariableDeclaration(SourceLocation source_loc,
Nonnull<BindingPattern*> binding,
Nonnull<Expression*> initializer)
: Declaration(Kind::VariableDeclaration, loc),
binding(binding),
initializer(initializer) {}
: Declaration(Kind::VariableDeclaration, source_loc),
binding_(binding),
initializer_(initializer) {}

static auto classof(const Declaration* decl) -> bool {
return decl->Tag() == Kind::VariableDeclaration;
return decl->tag() == Kind::VariableDeclaration;
}

auto Binding() const -> Nonnull<const BindingPattern*> { return binding; }
auto Binding() -> Nonnull<BindingPattern*> { return binding; }
auto Initializer() const -> Nonnull<const Expression*> { return initializer; }
auto Initializer() -> Nonnull<Expression*> { return initializer; }
auto binding() const -> const BindingPattern& { return *binding_; }
auto binding() -> BindingPattern& { return *binding_; }
auto initializer() const -> const Expression& { return *initializer_; }
auto initializer() -> Expression& { return *initializer_; }

private:
// TODO: split this into a non-optional name and a type, initialized by
// a constructor that takes a BindingPattern and handles errors like a
// missing name.
Nonnull<BindingPattern*> binding;
Nonnull<Expression*> initializer;
Nonnull<BindingPattern*> binding_;
Nonnull<Expression*> initializer_;
};

} // namespace Carbon
Expand Down
22 changes: 11 additions & 11 deletions executable_semantics/interpreter/interpreter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,10 @@ auto Interpreter::EvalPrim(Operator op,
}

void Interpreter::InitEnv(const Declaration& d, Env* env) {
switch (d.Tag()) {
switch (d.tag()) {
case Declaration::Kind::FunctionDeclaration: {
const FunctionDefinition& func_def =
cast<FunctionDeclaration>(d).Definition();
cast<FunctionDeclaration>(d).definition();
Env new_env = *env;
// Bring the deduced parameters into scope.
for (const auto& deduced : func_def.deduced_parameters()) {
Expand All @@ -125,10 +125,10 @@ void Interpreter::InitEnv(const Declaration& d, Env* env) {
}

case Declaration::Kind::ClassDeclaration: {
const ClassDefinition& class_def = cast<ClassDeclaration>(d).Definition();
const ClassDefinition& class_def = cast<ClassDeclaration>(d).definition();
VarValues fields;
VarValues methods;
for (Nonnull<const Member*> m : class_def.members) {
for (Nonnull<const Member*> m : class_def.members()) {
switch (m->Tag()) {
case Member::Kind::FieldMember: {
Nonnull<const BindingPattern*> binding =
Expand All @@ -141,33 +141,33 @@ void Interpreter::InitEnv(const Declaration& d, Env* env) {
}
}
}
auto st = arena->New<ClassType>(class_def.name, std::move(fields),
auto st = arena->New<ClassType>(class_def.name(), std::move(fields),
std::move(methods));
auto a = heap.AllocateValue(st);
env->Set(class_def.name, a);
env->Set(class_def.name(), a);
break;
}

case Declaration::Kind::ChoiceDeclaration: {
const auto& choice = cast<ChoiceDeclaration>(d);
VarValues alts;
for (const auto& alternative : choice.Alternatives()) {
for (const auto& alternative : choice.alternatives()) {
auto t = InterpExp(Env(arena), &alternative.signature());
alts.push_back(make_pair(alternative.name(), t));
}
auto ct = arena->New<ChoiceType>(choice.Name(), std::move(alts));
auto ct = arena->New<ChoiceType>(choice.name(), std::move(alts));
auto a = heap.AllocateValue(ct);
env->Set(choice.Name(), a);
env->Set(choice.name(), a);
break;
}

case Declaration::Kind::VariableDeclaration: {
const auto& var = cast<VariableDeclaration>(d);
// Adds an entry in `globals` mapping the variable's name to the
// result of evaluating the initializer.
auto v = InterpExp(*env, var.Initializer());
auto v = InterpExp(*env, &var.initializer());
Address a = heap.AllocateValue(v);
env->Set(*var.Binding()->Name(), a);
env->Set(*var.binding().Name(), a);
break;
}
}
Expand Down
Loading