From 96ffb8c608b5ff03aed10495b161834f809dcda2 Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Mon, 3 Apr 2023 18:43:08 +0300 Subject: [PATCH 1/4] ast: Make AST::Kind an enum class We're going to introduce AST::Kind::IDENTIFIER next, and with the default C-style enum member scoping, this would cause name clashes. Instead, convert AST::Kind into an enum class, so that its members are properly namespaced. gcc/rust/ChangeLog: * ast/rust-ast.h (Kind): Convert into a C++ enum class * expand/rust-macro-builtins.cc: Adapt to the change Signed-off-by: Sergey Bugaev --- gcc/rust/ast/rust-ast.h | 2 +- gcc/rust/expand/rust-macro-builtins.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/rust/ast/rust-ast.h b/gcc/rust/ast/rust-ast.h index 540a2624b3f7..ee3c89a298de 100644 --- a/gcc/rust/ast/rust-ast.h +++ b/gcc/rust/ast/rust-ast.h @@ -39,7 +39,7 @@ class ASTVisitor; using AttrVec = std::vector; // The available kinds of AST Nodes -enum Kind +enum class Kind { UNKNOWN, MACRO_RULES_DEFINITION, diff --git a/gcc/rust/expand/rust-macro-builtins.cc b/gcc/rust/expand/rust-macro-builtins.cc index fe401aa868e7..ed670e3b26ca 100644 --- a/gcc/rust/expand/rust-macro-builtins.cc +++ b/gcc/rust/expand/rust-macro-builtins.cc @@ -612,7 +612,7 @@ MacroBuiltin::concat_handler (Location invoc_locus, AST::MacroInvocData &invoc) for (auto &expr : expanded_expr) { if (!expr->is_literal () - && expr->get_ast_kind () != AST::MACRO_INVOCATION) + && expr->get_ast_kind () != AST::Kind::MACRO_INVOCATION) { has_error = true; rust_error_at (expr->get_locus (), "expected a literal"); From 78f919df95e98c87e311d17c2dc44940c8b23275 Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Mon, 3 Apr 2023 18:48:45 +0300 Subject: [PATCH 2/4] ast: Add AST::Kind::IDENTIFIER ...and return it from IdentifierExpr::get_ast_kind (). This enables other code to dynamically test whether an expression is in fact an IdentifierExpr. gcc/rust/ChangeLog: * ast/rust-ast.h: Add AST::Kind::IDENTIFIER Signed-off-by: Sergey Bugaev --- gcc/rust/ast/rust-ast.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gcc/rust/ast/rust-ast.h b/gcc/rust/ast/rust-ast.h index ee3c89a298de..a4db7c7d968d 100644 --- a/gcc/rust/ast/rust-ast.h +++ b/gcc/rust/ast/rust-ast.h @@ -44,6 +44,7 @@ enum class Kind UNKNOWN, MACRO_RULES_DEFINITION, MACRO_INVOCATION, + IDENTIFIER, }; class Visitable @@ -1072,6 +1073,8 @@ class IdentifierExpr : public ExprWithoutBlock outer_attrs = std::move (new_attrs); } + Kind get_ast_kind () const override { return Kind::IDENTIFIER; } + protected: // Clone method implementation IdentifierExpr *clone_expr_without_block_impl () const final override From e19179b58e929a211b5a99303969d84348fcbee2 Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Mon, 3 Apr 2023 18:51:58 +0300 Subject: [PATCH 3/4] resolve: Add ResolveExpr::funny_error ...and thread it through the constructors and the ResolveExpr::go () method. This will be used for implementing the "break rust" Easter egg. gcc/rust/ChangeLog: * resolve/rust-ast-resolve-expr.h, resolve/rust-ast-resolve-expr.cc: Add ResolveExpr::funny_error Signed-off-by: Sergey Bugaev --- gcc/rust/resolve/rust-ast-resolve-expr.cc | 10 ++++++---- gcc/rust/resolve/rust-ast-resolve-expr.h | 6 ++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/gcc/rust/resolve/rust-ast-resolve-expr.cc b/gcc/rust/resolve/rust-ast-resolve-expr.cc index ba9620cf0391..5b0846e8741b 100644 --- a/gcc/rust/resolve/rust-ast-resolve-expr.cc +++ b/gcc/rust/resolve/rust-ast-resolve-expr.cc @@ -29,9 +29,9 @@ namespace Resolver { void ResolveExpr::go (AST::Expr *expr, const CanonicalPath &prefix, - const CanonicalPath &canonical_prefix) + const CanonicalPath &canonical_prefix, bool funny_error) { - ResolveExpr resolver (prefix, canonical_prefix); + ResolveExpr resolver (prefix, canonical_prefix, funny_error); expr->accept_vis (resolver); } @@ -643,8 +643,10 @@ ResolveExpr::resolve_closure_param (AST::ClosureParam ¶m, } ResolveExpr::ResolveExpr (const CanonicalPath &prefix, - const CanonicalPath &canonical_prefix) - : ResolverBase (), prefix (prefix), canonical_prefix (canonical_prefix) + const CanonicalPath &canonical_prefix, + bool funny_error) + : ResolverBase (), prefix (prefix), canonical_prefix (canonical_prefix), + funny_error (funny_error) {} } // namespace Resolver diff --git a/gcc/rust/resolve/rust-ast-resolve-expr.h b/gcc/rust/resolve/rust-ast-resolve-expr.h index 5294b98fe40c..756ea98fce2a 100644 --- a/gcc/rust/resolve/rust-ast-resolve-expr.h +++ b/gcc/rust/resolve/rust-ast-resolve-expr.h @@ -31,7 +31,8 @@ class ResolveExpr : public ResolverBase public: static void go (AST::Expr *expr, const CanonicalPath &prefix, - const CanonicalPath &canonical_prefix); + const CanonicalPath &canonical_prefix, + bool funny_error = false); void visit (AST::TupleIndexExpr &expr) override; void visit (AST::TupleExpr &expr) override; @@ -83,10 +84,11 @@ class ResolveExpr : public ResolverBase private: ResolveExpr (const CanonicalPath &prefix, - const CanonicalPath &canonical_prefix); + const CanonicalPath &canonical_prefix, bool funny_error); const CanonicalPath &prefix; const CanonicalPath &canonical_prefix; + bool funny_error; }; } // namespace Resolver From 32c745f275b1213bcbb6360e0a4984cb2b103069 Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Mon, 3 Apr 2023 18:58:43 +0300 Subject: [PATCH 4/4] resolve: Add "break rust" Easter egg When we encounter a "break rust" statement, emit a funny error message and intentionally cause an ICE. This matches the corresponding Easter egg in rustc. As a GNU extension, "break gcc" is also supported. The conditions for this to happen are: * The break expression must be literally "rust" or "gcc". For instance, "break (rust)" will not trigger the Easter egg. * The name ("rust" or "gcc") must not be in scope; if it is, no error is emitted, and the compilation proceeds as usual. In other words, this only affects how GCC diagnoses programs that would fail to compile anyway. Note that this is different from the conditions under which rustc emits its ICE. For rustc, it matters whether or not the "break" is inside a loop, and for us it matters whether or not the name resolves. The end result should be the same anyway: valid programs continue to compile, and typing in fn main() { break rust; } triggers a funny ICE. Closes https://github.com/Rust-GCC/gccrs/issues/1996 gcc/rust/ChangeLog: * resolve/rust-ast-resolve-expr.cc: Add "break rust" Easter egg gcc/testsuite/ChangeLog: * lib/prune.exp (prune_ices): Also prune "You have broken GCC Rust. This is a feature." * rust/compile/break-rust1.rs: New test * rust/compile/break-rust2.rs: New test * rust/compile/break-rust3.rs: New test Signed-off-by: Sergey Bugaev --- gcc/rust/resolve/rust-ast-resolve-expr.cc | 70 ++++++++++++++++++++++- gcc/testsuite/lib/prune.exp | 1 + gcc/testsuite/rust/compile/break-rust1.rs | 7 +++ gcc/testsuite/rust/compile/break-rust2.rs | 4 ++ gcc/testsuite/rust/compile/break-rust3.rs | 4 ++ 5 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/rust/compile/break-rust1.rs create mode 100644 gcc/testsuite/rust/compile/break-rust2.rs create mode 100644 gcc/testsuite/rust/compile/break-rust3.rs diff --git a/gcc/rust/resolve/rust-ast-resolve-expr.cc b/gcc/rust/resolve/rust-ast-resolve-expr.cc index 5b0846e8741b..18ebce1c2111 100644 --- a/gcc/rust/resolve/rust-ast-resolve-expr.cc +++ b/gcc/rust/resolve/rust-ast-resolve-expr.cc @@ -23,6 +23,7 @@ #include "rust-ast-resolve-type.h" #include "rust-ast-resolve-pattern.h" #include "rust-ast-resolve-path.h" +#include "diagnostic.h" namespace Rust { namespace Resolver { @@ -105,6 +106,45 @@ ResolveExpr::visit (AST::AssignmentExpr &expr) VerifyAsignee::go (expr.get_left_expr ().get ()); } +/* The "break rust" Easter egg. + + Backstory: once upon a time, there used to be a bug in rustc: it would ICE + during typechecking on a 'break' with an expression outside of a loop. The + issue has been reported [0] and fixed [1], but in recognition of this, as a + special Easter egg, "break rust" was made to intentionally cause an ICE. + + [0]: https://github.com/rust-lang/rust/issues/43162 + [1]: https://github.com/rust-lang/rust/pull/43745 + + This was made in a way that does not break valid programs: namely, it only + happens when the 'break' is outside of a loop (so invalid anyway). + + GCC Rust supports this essential feature as well, but in a slightly + different way. Instead of delaying the error until type checking, we emit + it here in the resolution phase. We, too, only do this to programs that + are already invalid: we only emit our funny ICE if the name "rust" (which + must be immediately inside a break-with-a-value expression) fails to + resolve. Note that "break (rust)" does not trigger our ICE, only using + "break rust" directly does, and only if there's no "rust" in scope. We do + this in the same way regardless of whether the "break" is outside of a loop + or inside one. + + As a GNU extension, we also support "break gcc", much to the same effect, + subject to the same rules. */ + +/* The finalizer for our funny ICE. This prints a custom message instead of + the default bug reporting instructions, as there is no bug to report. */ + +static void ATTRIBUTE_NORETURN +funny_ice_finalizer (diagnostic_context *context, diagnostic_info *diagnostic, + diagnostic_t diag_kind) +{ + gcc_assert (diag_kind == DK_ICE_NOBT); + default_diagnostic_finalizer (context, diagnostic, diag_kind); + fnotice (stderr, "You have broken GCC Rust. This is a feature.\n"); + exit (ICE_EXIT_CODE); +} + void ResolveExpr::visit (AST::IdentifierExpr &expr) { @@ -120,6 +160,17 @@ ResolveExpr::visit (AST::IdentifierExpr &expr) { resolver->insert_resolved_type (expr.get_node_id (), resolved_node); } + else if (funny_error) + { + /* This was a "break rust" or "break gcc", and the identifier failed to + resolve. Emit a funny ICE. We set the finalizer to our custom one, + and use the lower-level emit_diagnostic () instead of the more common + internal_error_no_backtrace () in order to pass our locus. */ + diagnostic_finalizer (global_dc) = funny_ice_finalizer; + emit_diagnostic (DK_ICE_NOBT, expr.get_locus ().gcc_location (), -1, + "are you trying to break %s? how dare you?", + expr.as_string ().c_str ()); + } else { rust_error_at (expr.get_locus (), "failed to find name: %s", @@ -376,7 +427,24 @@ ResolveExpr::visit (AST::BreakExpr &expr) } if (expr.has_break_expr ()) - ResolveExpr::go (expr.get_break_expr ().get (), prefix, canonical_prefix); + { + bool funny_error = false; + AST::Expr &break_expr = *expr.get_break_expr ().get (); + if (break_expr.get_ast_kind () == AST::Kind::IDENTIFIER) + { + /* This is a break with an expression, and the expression is just a + single identifier. See if the identifier is either "rust" or + "gcc", in which case we have "break rust" or "break gcc", and so + may need to emit our funny error. We cannot yet emit the error + here though, because the identifier may still be in scope, and + ICE'ing on valid programs would not be very funny. */ + std::string ident + = static_cast (break_expr).as_string (); + if (ident == "rust" || ident == "gcc") + funny_error = true; + } + ResolveExpr::go (&break_expr, prefix, canonical_prefix, funny_error); + } } void diff --git a/gcc/testsuite/lib/prune.exp b/gcc/testsuite/lib/prune.exp index cfe427c99ace..8a448bbb63a2 100644 --- a/gcc/testsuite/lib/prune.exp +++ b/gcc/testsuite/lib/prune.exp @@ -158,6 +158,7 @@ proc prune_file_path { text } { proc prune_ices { text } { regsub -all "(^|\n)\[^\n\]*: internal compiler error:.*\nSee \[^\n\]*" $text "" text regsub -all "(^|\n|')*Internal compiler error:.*\nSee \[^\n\]*" $text "" text + regsub -all "(^|\n)\[^\n\]*: internal compiler error:.*\nYou have broken GCC Rust. This is a feature." $text "" text return $text } diff --git a/gcc/testsuite/rust/compile/break-rust1.rs b/gcc/testsuite/rust/compile/break-rust1.rs new file mode 100644 index 000000000000..65d64f9b82b0 --- /dev/null +++ b/gcc/testsuite/rust/compile/break-rust1.rs @@ -0,0 +1,7 @@ +fn main() { + let rust = "crab"; + let res = loop { + // { dg-warning "unused name" "" { target *-*-* } .-1 } + break rust; + }; +} diff --git a/gcc/testsuite/rust/compile/break-rust2.rs b/gcc/testsuite/rust/compile/break-rust2.rs new file mode 100644 index 000000000000..d02589e6bccd --- /dev/null +++ b/gcc/testsuite/rust/compile/break-rust2.rs @@ -0,0 +1,4 @@ +fn main() { + break (rust); + // { dg-error "failed to find name: rust" "" { target *-*-* } .-1 } +} diff --git a/gcc/testsuite/rust/compile/break-rust3.rs b/gcc/testsuite/rust/compile/break-rust3.rs new file mode 100644 index 000000000000..b18666a6b722 --- /dev/null +++ b/gcc/testsuite/rust/compile/break-rust3.rs @@ -0,0 +1,4 @@ +fn main() { + break rust; + // { dg-ice "are you trying to break rust? how dare you?" } +}