From aeb41dc5b633a87d0ebe69ed1ce0b7f0af24fa3e Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 6 Mar 2015 00:27:32 +0000 Subject: [PATCH] compiler: Do not declare type switch variable outside case statements. For expressions containing a TypeSwitchGuard with a short variable declaration e.g. var := x.(type), the spec says that var is declared at the beginning of the implicit block for each in each clause. Previously, var was declared in the block for the switch statement and each implicit block, which led to errors if the type case clause referenced a type with a similar name as the declared variable. Fixes golang/go#10047. From-SVN: r221230 --- gcc/go/gofrontend/gogo.cc | 5 +- gcc/go/gofrontend/parse.cc | 104 +++++++++++++++++--------------- gcc/go/gofrontend/parse.h | 10 +-- gcc/go/gofrontend/statements.cc | 27 +++------ gcc/go/gofrontend/statements.h | 13 ++-- 5 files changed, 79 insertions(+), 80 deletions(-) diff --git a/gcc/go/gofrontend/gogo.cc b/gcc/go/gofrontend/gogo.cc index ab305cf2741..a42803dd830 100644 --- a/gcc/go/gofrontend/gogo.cc +++ b/gcc/go/gofrontend/gogo.cc @@ -6030,6 +6030,7 @@ Variable::type() Type* type = this->type_; Expression* init = this->init_; if (this->is_type_switch_var_ + && type != NULL && this->type_->is_nil_constant_as_type()) { Type_guard_expression* tge = this->init_->type_guard_expression(); @@ -6103,7 +6104,9 @@ Variable::determine_type() // type here. It will have an initializer which is a type guard. // We want to initialize it to the value without the type guard, and // use the type of that value as well. - if (this->is_type_switch_var_ && this->type_->is_nil_constant_as_type()) + if (this->is_type_switch_var_ + && this->type_ != NULL + && this->type_->is_nil_constant_as_type()) { Type_guard_expression* tge = this->init_->type_guard_expression(); go_assert(tge != NULL); diff --git a/gcc/go/gofrontend/parse.cc b/gcc/go/gofrontend/parse.cc index 29edba7650a..496ab41b6e3 100644 --- a/gcc/go/gofrontend/parse.cc +++ b/gcc/go/gofrontend/parse.cc @@ -50,8 +50,7 @@ Parse::Parse(Lex* lex, Gogo* gogo) break_stack_(NULL), continue_stack_(NULL), iota_(0), - enclosing_vars_(), - type_switch_vars_() + enclosing_vars_() { } @@ -4596,32 +4595,33 @@ Statement* Parse::type_switch_body(Label* label, const Type_switch& type_switch, Location location) { - Named_object* switch_no = NULL; - if (!type_switch.name.empty()) - { - if (Gogo::is_sink_name(type_switch.name)) - error_at(type_switch.location, - "no new variables on left side of %<:=%>"); + Expression* init = type_switch.expr; + std::string var_name = type_switch.name; + if (!var_name.empty()) + { + if (Gogo::is_sink_name(var_name)) + { + error_at(type_switch.location, + "no new variables on left side of %<:=%>"); + var_name.clear(); + } else { - Variable* switch_var = new Variable(NULL, type_switch.expr, false, - false, false, - type_switch.location); - switch_no = this->gogo_->add_variable(type_switch.name, switch_var); + Location loc = type_switch.location; + Temporary_statement* switch_temp = + Statement::make_temporary(NULL, init, loc); + this->gogo_->add_statement(switch_temp); + init = Expression::make_temporary_reference(switch_temp, loc); } } Type_switch_statement* statement = - Statement::make_type_switch_statement(switch_no, - (switch_no == NULL - ? type_switch.expr - : NULL), - location); - + Statement::make_type_switch_statement(var_name, init, location); this->push_break_statement(statement, label); Type_case_clauses* case_clauses = new Type_case_clauses(); bool saw_default = false; + std::vector implicit_vars; while (!this->peek_token()->is_op(OPERATOR_RCURLY)) { if (this->peek_token()->is_eof()) @@ -4629,7 +4629,8 @@ Parse::type_switch_body(Label* label, const Type_switch& type_switch, error_at(this->location(), "missing %<}%>"); return NULL; } - this->type_case_clause(switch_no, case_clauses, &saw_default); + this->type_case_clause(var_name, init, case_clauses, &saw_default, + &implicit_vars); } this->advance_token(); @@ -4637,14 +4638,36 @@ Parse::type_switch_body(Label* label, const Type_switch& type_switch, this->pop_break_statement(); + // If there is a type switch variable implicitly declared in each case clause, + // check that it is used in at least one of the cases. + if (!var_name.empty()) + { + bool used = false; + for (std::vector::iterator p = implicit_vars.begin(); + p != implicit_vars.end(); + ++p) + { + if ((*p)->var_value()->is_used()) + { + used = true; + break; + } + } + if (!used) + error_at(type_switch.location, "%qs declared and not used", + Gogo::message_name(var_name).c_str()); + } return statement; } // TypeCaseClause = TypeSwitchCase ":" [ StatementList ] . +// IMPLICIT_VARS is the list of variables implicitly declared for each type +// case if there is a type switch variable declared. void -Parse::type_case_clause(Named_object* switch_no, Type_case_clauses* clauses, - bool* saw_default) +Parse::type_case_clause(const std::string& var_name, Expression* init, + Type_case_clauses* clauses, bool* saw_default, + std::vector* implicit_vars) { Location location = this->location(); @@ -4661,24 +4684,21 @@ Parse::type_case_clause(Named_object* switch_no, Type_case_clauses* clauses, if (this->statement_list_may_start_here()) { this->gogo_->start_block(this->location()); - if (switch_no != NULL && types.size() == 1) + if (!var_name.empty()) { - Type* type = types.front(); - Expression* init = Expression::make_var_reference(switch_no, - location); - init = Expression::make_type_guard(init, type, location); - Variable* v = new Variable(type, init, false, false, false, - location); - v->set_is_type_switch_var(); - Named_object* no = this->gogo_->add_variable(switch_no->name(), v); + Type* type = NULL; + Location var_loc = init->location(); + if (types.size() == 1) + { + type = types.front(); + init = Expression::make_type_guard(init, type, location); + } - // We don't want to issue an error if the compiler - // introduced special variable is not used. Instead we want - // to issue an error if the variable defined by the switch - // is not used. That is handled via type_switch_vars_ and - // Parse::mark_var_used. + Variable* v = new Variable(type, init, false, false, false, + var_loc); v->set_is_used(); - this->type_switch_vars_[no] = switch_no; + v->set_is_type_switch_var(); + implicit_vars->push_back(this->gogo_->add_variable(var_name, v)); } this->statement_list(); statements = this->gogo_->finish_block(this->location()); @@ -5752,15 +5772,5 @@ void Parse::mark_var_used(Named_object* no) { if (no->is_variable()) - { - no->var_value()->set_is_used(); - - // When a type switch uses := to define a variable, then for - // each case with a single type we introduce a new variable with - // the appropriate type. When we do, if the newly introduced - // variable is used, then the type switch variable is used. - Type_switch_vars::iterator p = this->type_switch_vars_.find(no); - if (p != this->type_switch_vars_.end()) - p->second->var_value()->set_is_used(); - } + no->var_value()->set_is_used(); } diff --git a/gcc/go/gofrontend/parse.h b/gcc/go/gofrontend/parse.h index 9dd3b167b84..734071a7377 100644 --- a/gcc/go/gofrontend/parse.h +++ b/gcc/go/gofrontend/parse.h @@ -156,11 +156,6 @@ class Parse // break or continue statement with no label. typedef std::vector > Bc_stack; - // Map from type switch variables to the variables they mask, so - // that a use of the type switch variable can become a use of the - // real variable. - typedef Unordered_map(Named_object*, Named_object*) Type_switch_vars; - // Parser nonterminals. void identifier_list(Typed_identifier_list*); Expression_list* expression_list(Expression*, bool may_be_sink, @@ -259,7 +254,8 @@ class Parse void expr_case_clause(Case_clauses*, bool* saw_default); Expression_list* expr_switch_case(bool*); Statement* type_switch_body(Label*, const Type_switch&, Location); - void type_case_clause(Named_object*, Type_case_clauses*, bool* saw_default); + void type_case_clause(const std::string&, Expression*, Type_case_clauses*, + bool* saw_default, std::vector*); void type_switch_case(std::vector*, bool*); void select_stat(Label*); void comm_clause(Select_clauses*, bool* saw_default); @@ -327,8 +323,6 @@ class Parse // References from the local function to variables defined in // enclosing functions. Enclosing_vars enclosing_vars_; - // Map from type switch variables to real variables. - Type_switch_vars type_switch_vars_; }; diff --git a/gcc/go/gofrontend/statements.cc b/gcc/go/gofrontend/statements.cc index 7aa397f487d..e1cc75e7e04 100644 --- a/gcc/go/gofrontend/statements.cc +++ b/gcc/go/gofrontend/statements.cc @@ -4279,11 +4279,8 @@ Type_case_clauses::dump_clauses(Ast_dump_context* ast_dump_context) const int Type_switch_statement::do_traverse(Traverse* traverse) { - if (this->var_ == NULL) - { - if (this->traverse_expression(traverse, &this->expr_) == TRAVERSE_EXIT) - return TRAVERSE_EXIT; - } + if (this->traverse_expression(traverse, &this->expr_) == TRAVERSE_EXIT) + return TRAVERSE_EXIT; if (this->clauses_ != NULL) return this->clauses_->traverse(traverse); return TRAVERSE_CONTINUE; @@ -4306,10 +4303,7 @@ Type_switch_statement::do_lower(Gogo*, Named_object*, Block* enclosing, Block* b = new Block(enclosing, loc); - Type* val_type = (this->var_ != NULL - ? this->var_->var_value()->type() - : this->expr_->type()); - + Type* val_type = this->expr_->type(); if (val_type->interface_type() == NULL) { if (!val_type->is_error()) @@ -4326,15 +4320,10 @@ Type_switch_statement::do_lower(Gogo*, Named_object*, Block* enclosing, // descriptor_temp = ifacetype(val_temp) FIXME: This should be // inlined. bool is_empty = val_type->interface_type()->is_empty(); - Expression* ref; - if (this->var_ == NULL) - ref = this->expr_; - else - ref = Expression::make_var_reference(this->var_, loc); Expression* call = Runtime::make_call((is_empty ? Runtime::EFACETYPE : Runtime::IFACETYPE), - loc, 1, ref); + loc, 1, this->expr_); Temporary_reference_expression* lhs = Expression::make_temporary_reference(descriptor_temp, loc); lhs->set_is_lvalue(); @@ -4384,7 +4373,9 @@ Type_switch_statement::do_dump_statement(Ast_dump_context* ast_dump_context) const { ast_dump_context->print_indent(); - ast_dump_context->ostream() << "switch " << this->var_->name() << " = "; + ast_dump_context->ostream() << "switch "; + if (!this->name_.empty()) + ast_dump_context->ostream() << this->name_ << " = "; ast_dump_context->dump_expression(this->expr_); ast_dump_context->ostream() << " .(type)"; if (ast_dump_context->dump_subblocks()) @@ -4399,10 +4390,10 @@ Type_switch_statement::do_dump_statement(Ast_dump_context* ast_dump_context) // Make a type switch statement. Type_switch_statement* -Statement::make_type_switch_statement(Named_object* var, Expression* expr, +Statement::make_type_switch_statement(const std::string& name, Expression* expr, Location location) { - return new Type_switch_statement(var, expr, location); + return new Type_switch_statement(name, expr, location); } // Class Send_statement. diff --git a/gcc/go/gofrontend/statements.h b/gcc/go/gofrontend/statements.h index 5634b61f339..8b8b99b4cfd 100644 --- a/gcc/go/gofrontend/statements.h +++ b/gcc/go/gofrontend/statements.h @@ -250,7 +250,7 @@ class Statement // Make a type switch statement. static Type_switch_statement* - make_type_switch_statement(Named_object* var, Expression*, Location); + make_type_switch_statement(const std::string&, Expression*, Location); // Make a send statement. static Send_statement* @@ -1607,11 +1607,11 @@ class Type_case_clauses class Type_switch_statement : public Statement { public: - Type_switch_statement(Named_object* var, Expression* expr, + Type_switch_statement(const std::string& name, Expression* expr, Location location) : Statement(STATEMENT_TYPE_SWITCH, location), - var_(var), expr_(expr), clauses_(NULL), break_label_(NULL) - { go_assert(var == NULL || expr == NULL); } + name_(name), expr_(expr), clauses_(NULL), break_label_(NULL) + { } // Add the clauses. void @@ -1643,8 +1643,9 @@ class Type_switch_statement : public Statement do_may_fall_through() const; private: - // The variable holding the value we are switching on. - Named_object* var_; + // The name of the variable declared in the type switch guard. Empty if there + // is no variable declared. + std::string name_; // The expression we are switching on if there is no variable. Expression* expr_; // The type case clauses. -- 2.30.2