From fb4347e44d3e1bf9970f361cf8b25c0b7f5d1d9f Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 25 Mar 2011 17:34:44 +0000 Subject: [PATCH] Give an error if a label is defined but not used. From-SVN: r171518 --- gcc/go/gofrontend/gogo.cc | 37 +++++++++++++++- gcc/go/gofrontend/gogo.h | 18 +++++++- gcc/go/gofrontend/parse.cc | 44 ++++++++++++------- gcc/go/gofrontend/parse.h | 19 ++++---- .../go.test/test/fixedbugs/bug055.go | 29 +++++++----- .../go.test/test/fixedbugs/bug076.go | 10 +++-- .../go.test/test/fixedbugs/bug077.go | 5 ++- .../go.test/test/fixedbugs/bug091.go | 11 ++--- .../go.test/test/fixedbugs/bug137.go | 19 +++++--- .../go.test/test/fixedbugs/bug140.go | 13 +++++- .../go.test/test/fixedbugs/bug178.go | 18 +++++--- .../go.test/test/fixedbugs/bug179.go | 14 +++--- .../go.test/test/fixedbugs/bug274.go | 3 +- 13 files changed, 168 insertions(+), 72 deletions(-) diff --git a/gcc/go/gofrontend/gogo.cc b/gcc/go/gofrontend/gogo.cc index a6411d362c4..be308035b58 100644 --- a/gcc/go/gofrontend/gogo.cc +++ b/gcc/go/gofrontend/gogo.cc @@ -1463,6 +1463,7 @@ class Check_types_traverse : public Traverse Check_types_traverse(Gogo* gogo) : Traverse(traverse_variables | traverse_constants + | traverse_functions | traverse_statements | traverse_expressions), gogo_(gogo) @@ -1474,6 +1475,9 @@ class Check_types_traverse : public Traverse int constant(Named_object*, bool); + int + function(Named_object*); + int statement(Block*, size_t* pindex, Statement*); @@ -1542,6 +1546,16 @@ Check_types_traverse::constant(Named_object* named_object, bool) return TRAVERSE_CONTINUE; } +// There are no types to check in a function, but this is where we +// issue warnings about labels which are defined but not referenced. + +int +Check_types_traverse::function(Named_object* no) +{ + no->func_value()->check_labels(); + return TRAVERSE_CONTINUE; +} + // Check that types are valid in a statement. int @@ -2744,7 +2758,7 @@ Function::add_label_definition(const std::string& label_name, } else { - error_at(location, "redefinition of label %qs", + error_at(location, "label %qs already defined", Gogo::message_name(label_name).c_str()); inform(label->location(), "previous definition of %qs was here", Gogo::message_name(label_name).c_str()); @@ -2764,17 +2778,36 @@ Function::add_label_reference(const std::string& label_name) if (!ins.second) { // The label was already in the hash table. - return ins.first->second; + Label* label = ins.first->second; + label->set_is_used(); + return label; } else { gcc_assert(ins.first->second == NULL); Label* label = new Label(label_name); ins.first->second = label; + label->set_is_used(); return label; } } +// Warn about labels that are defined but not used. + +void +Function::check_labels() const +{ + for (Labels::const_iterator p = this->labels_.begin(); + p != this->labels_.end(); + p++) + { + Label* label = p->second; + if (!label->is_used()) + error_at(label->location(), "label %qs defined and not used", + Gogo::message_name(label->name()).c_str()); + } +} + // Swap one function with another. This is used when building the // thunk we use to call a function which calls recover. It may not // work for any other case. diff --git a/gcc/go/gofrontend/gogo.h b/gcc/go/gofrontend/gogo.h index 7a887a54296..365860d36ce 100644 --- a/gcc/go/gofrontend/gogo.h +++ b/gcc/go/gofrontend/gogo.h @@ -882,6 +882,10 @@ class Function Label* add_label_reference(const std::string& label_name); + // Warn about labels that are defined but not used. + void + check_labels() const; + // Whether this function calls the predeclared recover function. bool calls_recover() const @@ -2090,7 +2094,7 @@ class Label { public: Label(const std::string& name) - : name_(name), location_(0), decl_(NULL) + : name_(name), location_(0), is_used_(false), decl_(NULL) { } // Return the label's name. @@ -2103,6 +2107,16 @@ class Label is_defined() const { return this->location_ != 0; } + // Return whether the label has been used. + bool + is_used() const + { return this->is_used_; } + + // Record that the label is used. + void + set_is_used() + { this->is_used_ = true; } + // Return the location of the definition. source_location location() const @@ -2130,6 +2144,8 @@ class Label // The location of the definition. This is 0 if the label has not // yet been defined. source_location location_; + // Whether the label has been used. + bool is_used_; // The LABEL_DECL. tree decl_; }; diff --git a/gcc/go/gofrontend/parse.cc b/gcc/go/gofrontend/parse.cc index f1b93429ff2..cdee68ad6c2 100644 --- a/gcc/go/gofrontend/parse.cc +++ b/gcc/go/gofrontend/parse.cc @@ -3112,7 +3112,7 @@ Parse::unary_expr(bool may_be_sink, bool may_be_composite_lit, // LABEL is the label of this statement if it has one. void -Parse::statement(const Label* label) +Parse::statement(Label* label) { const Token* token = this->peek_token(); switch (token->classification()) @@ -3288,6 +3288,10 @@ Parse::labeled_stmt(const std::string& label_name, source_location location) if (!this->statement_may_start_here()) { + // Mark the label as used to avoid a useless error about an + // unused label. + label->set_is_used(); + error_at(location, "missing statement after label"); this->unget_token(Token::make_operator_token(OPERATOR_SEMICOLON, location)); @@ -3774,7 +3778,7 @@ Parse::if_stat() // TypeSwitchGuard = [ identifier ":=" ] Expression "." "(" "type" ")" . void -Parse::switch_stat(const Label* label) +Parse::switch_stat(Label* label) { gcc_assert(this->peek_token()->is_keyword(KEYWORD_SWITCH)); source_location location = this->location(); @@ -3873,7 +3877,7 @@ Parse::switch_stat(const Label* label) // "{" { ExprCaseClause } "}" Statement* -Parse::expr_switch_body(const Label* label, Expression* switch_val, +Parse::expr_switch_body(Label* label, Expression* switch_val, source_location location) { Switch_statement* statement = Statement::make_switch_statement(switch_val, @@ -3983,7 +3987,7 @@ Parse::expr_switch_case(bool* is_default) // "{" { TypeCaseClause } "}" . Statement* -Parse::type_switch_body(const Label* label, const Type_switch& type_switch, +Parse::type_switch_body(Label* label, const Type_switch& type_switch, source_location location) { Named_object* switch_no = NULL; @@ -4127,7 +4131,7 @@ Parse::type_switch_case(std::vector* types, bool* is_default) // SelectStat = "select" "{" { CommClause } "}" . void -Parse::select_stat(const Label* label) +Parse::select_stat(Label* label) { gcc_assert(this->peek_token()->is_keyword(KEYWORD_SELECT)); source_location location = this->location(); @@ -4435,7 +4439,7 @@ Parse::send_or_recv_stmt(bool* is_send, Expression** channel, Expression** val, // Condition = Expression . void -Parse::for_stat(const Label* label) +Parse::for_stat(Label* label) { gcc_assert(this->peek_token()->is_keyword(KEYWORD_FOR)); source_location location = this->location(); @@ -4654,7 +4658,7 @@ Parse::range_clause_expr(const Expression_list* vals, // Push a statement on the break stack. void -Parse::push_break_statement(Statement* enclosing, const Label* label) +Parse::push_break_statement(Statement* enclosing, Label* label) { if (this->break_stack_ == NULL) this->break_stack_ = new Bc_stack(); @@ -4664,7 +4668,7 @@ Parse::push_break_statement(Statement* enclosing, const Label* label) // Push a statement on the continue stack. void -Parse::push_continue_statement(Statement* enclosing, const Label* label) +Parse::push_continue_statement(Statement* enclosing, Label* label) { if (this->continue_stack_ == NULL) this->continue_stack_ = new Bc_stack(); @@ -4697,8 +4701,13 @@ Parse::find_bc_statement(const Bc_stack* bc_stack, const std::string& label) for (Bc_stack::const_reverse_iterator p = bc_stack->rbegin(); p != bc_stack->rend(); ++p) - if (p->second != NULL && p->second->name() == label) - return p->first; + { + if (p->second != NULL && p->second->name() == label) + { + p->second->set_is_used(); + return p->first; + } + } return NULL; } @@ -4728,9 +4737,11 @@ Parse::break_stat() token->identifier()); if (enclosing == NULL) { - error_at(token->location(), - ("break label %qs not associated with " - "for or switch or select"), + // If there is a label with this name, mark it as used to + // avoid a useless error about an unused label. + this->gogo_->add_label_reference(token->identifier()); + + error_at(token->location(), "invalid break label %qs", Gogo::message_name(token->identifier()).c_str()); this->advance_token(); return; @@ -4781,8 +4792,11 @@ Parse::continue_stat() token->identifier()); if (enclosing == NULL) { - error_at(token->location(), - "continue label %qs not associated with for", + // If there is a label with this name, mark it as used to + // avoid a useless error about an unused label. + this->gogo_->add_label_reference(token->identifier()); + + error_at(token->location(), "invalid continue label %qs", Gogo::message_name(token->identifier()).c_str()); this->advance_token(); return; diff --git a/gcc/go/gofrontend/parse.h b/gcc/go/gofrontend/parse.h index d164414df7b..a9f6455a6f1 100644 --- a/gcc/go/gofrontend/parse.h +++ b/gcc/go/gofrontend/parse.h @@ -150,7 +150,7 @@ class Parse // For break and continue we keep a stack of statements with // associated labels (if any). The top of the stack is used for a // break or continue statement with no label. - typedef std::vector > Bc_stack; + typedef std::vector > Bc_stack; // Parser nonterminals. void identifier_list(Typed_identifier_list*); @@ -220,7 +220,7 @@ class Parse bool* is_type_switch); Expression* qualified_expr(Expression*, source_location); Expression* id_to_expression(const std::string&, source_location); - void statement(const Label*); + void statement(Label*); bool statement_may_start_here(); void labeled_stmt(const std::string&, source_location); Expression* simple_stat(bool, bool, Range_clause*, Type_switch*); @@ -236,26 +236,25 @@ class Parse void go_or_defer_stat(); void return_stat(); void if_stat(); - void switch_stat(const Label*); - Statement* expr_switch_body(const Label*, Expression*, source_location); + void switch_stat(Label*); + Statement* expr_switch_body(Label*, Expression*, source_location); void expr_case_clause(Case_clauses*, bool* saw_default); Expression_list* expr_switch_case(bool*); - Statement* type_switch_body(const Label*, const Type_switch&, - source_location); + Statement* type_switch_body(Label*, const Type_switch&, source_location); void type_case_clause(Named_object*, Type_case_clauses*, bool* saw_default); void type_switch_case(std::vector*, bool*); - void select_stat(const Label*); + void select_stat(Label*); void comm_clause(Select_clauses*, bool* saw_default); bool comm_case(bool*, Expression**, Expression**, Expression**, std::string*, std::string*, bool*); bool send_or_recv_stmt(bool*, Expression**, Expression**, Expression**, std::string*, std::string*); - void for_stat(const Label*); + void for_stat(Label*); void for_clause(Expression**, Block**); void range_clause_decl(const Typed_identifier_list*, Range_clause*); void range_clause_expr(const Expression_list*, Range_clause*); - void push_break_statement(Statement*, const Label*); - void push_continue_statement(Statement*, const Label*); + void push_break_statement(Statement*, Label*); + void push_continue_statement(Statement*, Label*); void pop_break_statement(); void pop_continue_statement(); Statement* find_bc_statement(const Bc_stack*, const std::string&); diff --git a/gcc/testsuite/go.test/test/fixedbugs/bug055.go b/gcc/testsuite/go.test/test/fixedbugs/bug055.go index 0326d828f2c..8617396109d 100644 --- a/gcc/testsuite/go.test/test/fixedbugs/bug055.go +++ b/gcc/testsuite/go.test/test/fixedbugs/bug055.go @@ -7,16 +7,21 @@ package main func main() { - var i int; - var j int; - if true {} - { return } - i = 0; - if true {} else i++; - type s struct {}; - i = 0; - type s2 int; - var k = func (a int) int { return a+1 }(3); - _, _ = j, k; -ro: ; + var i int + var j int + if true { + } + { + return + } + i = 0 + if true { + } else { + i++ + } + type s struct{} + i = 0 + type s2 int + var k = func(a int) int { return a + 1 }(3) + _, _ = j, k } diff --git a/gcc/testsuite/go.test/test/fixedbugs/bug076.go b/gcc/testsuite/go.test/test/fixedbugs/bug076.go index 065cecc015b..2ca518d76d8 100644 --- a/gcc/testsuite/go.test/test/fixedbugs/bug076.go +++ b/gcc/testsuite/go.test/test/fixedbugs/bug076.go @@ -1,4 +1,4 @@ -// $G $D/$F.go && $L $F.$A && ./$A.out +// $G $D/$F.go && $L $F.$A // Copyright 2009 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style @@ -7,12 +7,16 @@ package main func f() { -exit: ; +exit: + ; + goto exit } func main() { -exit: ; // this should be legal (labels not properly scoped?) +exit: + ; // this should be legal (labels not properly scoped?) + goto exit } /* diff --git a/gcc/testsuite/go.test/test/fixedbugs/bug077.go b/gcc/testsuite/go.test/test/fixedbugs/bug077.go index 08028ab10fa..2cbf96d98fe 100644 --- a/gcc/testsuite/go.test/test/fixedbugs/bug077.go +++ b/gcc/testsuite/go.test/test/fixedbugs/bug077.go @@ -7,7 +7,8 @@ package main func main() { - var exit int; + var exit int exit: - _ = exit; + _ = exit + goto exit } diff --git a/gcc/testsuite/go.test/test/fixedbugs/bug091.go b/gcc/testsuite/go.test/test/fixedbugs/bug091.go index cfbb09cd820..c2ede7153ce 100644 --- a/gcc/testsuite/go.test/test/fixedbugs/bug091.go +++ b/gcc/testsuite/go.test/test/fixedbugs/bug091.go @@ -7,18 +7,19 @@ package main func f1() { - exit: - print("hi\n"); +exit: + print("hi\n") + goto exit } func f2() { - const c = 1234; + const c = 1234 } func f3() { - i := c; // ERROR "undef" + i := c // ERROR "undef" } func main() { - f3(); + f3() } diff --git a/gcc/testsuite/go.test/test/fixedbugs/bug137.go b/gcc/testsuite/go.test/test/fixedbugs/bug137.go index 1527924116f..9d43f431be8 100644 --- a/gcc/testsuite/go.test/test/fixedbugs/bug137.go +++ b/gcc/testsuite/go.test/test/fixedbugs/bug137.go @@ -8,16 +8,21 @@ package main func main() { L1: -L2: for i := 0; i < 10; i++ { - print(i); - break L2; +L2: + for i := 0; i < 10; i++ { + print(i) + break L2 } -L3: ; -L4: for i := 0; i < 10; i++ { - print(i); - break L4; +L3: + ; +L4: + for i := 0; i < 10; i++ { + print(i) + break L4 } + goto L1 + goto L3 } /* diff --git a/gcc/testsuite/go.test/test/fixedbugs/bug140.go b/gcc/testsuite/go.test/test/fixedbugs/bug140.go index 298081663b4..e27b370e760 100644 --- a/gcc/testsuite/go.test/test/fixedbugs/bug140.go +++ b/gcc/testsuite/go.test/test/fixedbugs/bug140.go @@ -7,8 +7,17 @@ package main func main() { - if true {} else L1: ; - if true {} else L2: main() ; + if true { + } else { + L1: + } + if true { + } else { + L2: + main() + } + goto L1 + goto L2 } /* diff --git a/gcc/testsuite/go.test/test/fixedbugs/bug178.go b/gcc/testsuite/go.test/test/fixedbugs/bug178.go index 4f586342b46..20596102441 100644 --- a/gcc/testsuite/go.test/test/fixedbugs/bug178.go +++ b/gcc/testsuite/go.test/test/fixedbugs/bug178.go @@ -9,19 +9,25 @@ package main func main() { L: for i := 0; i < 1; i++ { -L1: + L1: for { - break L; + break L } - panic("BUG: not reached - break"); + panic("BUG: not reached - break") } L2: for i := 0; i < 1; i++ { -L3: + L3: for { - continue L2; + continue L2 } - panic("BUG: not reached - continue"); + panic("BUG: not reached - continue") + } + if false { + goto L1 + } + if false { + goto L3 } } diff --git a/gcc/testsuite/go.test/test/fixedbugs/bug179.go b/gcc/testsuite/go.test/test/fixedbugs/bug179.go index 67548733ce6..3347613d8db 100644 --- a/gcc/testsuite/go.test/test/fixedbugs/bug179.go +++ b/gcc/testsuite/go.test/test/fixedbugs/bug179.go @@ -10,16 +10,18 @@ func main() { L: for { for { - break L2; // ERROR "L2" - continue L2; // ERROR "L2" + break L2 // ERROR "L2" + continue L2 // ERROR "L2" } } L1: - x := 1; - _ = x; + x := 1 + _ = x for { - break L1; // ERROR "L1" - continue L1; // ERROR "L1" + break L1 // ERROR "L1" + continue L1 // ERROR "L1" } + + goto L } diff --git a/gcc/testsuite/go.test/test/fixedbugs/bug274.go b/gcc/testsuite/go.test/test/fixedbugs/bug274.go index 621f31eed37..348aed429e9 100644 --- a/gcc/testsuite/go.test/test/fixedbugs/bug274.go +++ b/gcc/testsuite/go.test/test/fixedbugs/bug274.go @@ -24,6 +24,7 @@ func main() { case 1: L1: // ERROR "statement" default: - L2: // correct since no semicolon is required before a '}' + // correct since no semicolon is required before a '}' + L2: // GCCGO_ERROR "not used" } } -- 2.30.2