Skip to content

Commit ca0b06f

Browse files
bors[bot]tschwinge
bors[bot]
and
tschwinge
authored
Merge #780
780: No side effects in 'assert' expressions r=philberty a=tschwinge Usually, if 'assert'ions are disabled, 'assert' expressions are not evaluated, so in that case won't effect any side effects. Via spurious ICEs/test suite FAILs, this may be observed in GCC/Rust, for example, if configuring with '--enable-checking=no' and disabling a "more forgiving" 'gcc/system.h:gcc_assert' definition, so that '0 && (EXPR)' gets used: /* Use gcc_assert(EXPR) to test invariants. */ #if ENABLE_ASSERT_CHECKING #define gcc_assert(EXPR) \ ((void)(!(EXPR) ? fancy_abort (__FILE__, __LINE__, __FUNCTION__), 0 : 0)) -#elif (GCC_VERSION >= 4005) +#elif (0) //GCC_VERSION >= 4005) #define gcc_assert(EXPR) \ ((void)(__builtin_expect (!(EXPR), 0) ? __builtin_unreachable (), 0 : 0)) #else /* Include EXPR, so that unused variable warnings do not occur. */ #define gcc_assert(EXPR) ((void)(0 && (EXPR))) #endif As that one does cause some issues in GCC proper (that I shall fix separately), may use this change to 'gcc/rust/rust-system.h:rust_assert' instead: +#if 0 #define rust_assert(EXPR) gcc_assert (EXPR) +#else +#define rust_assert(EXPR) ((void) (0 && (EXPR))) +#endif To fix these, use the same pattern as is already used in a lot of existing GCC/Rust code: bool ok = [expression with side effects]; rust_assert (ok); I've only done a quick manual review; maybe there is a tool for doing this? Co-authored-by: Thomas Schwinge <[email protected]>
2 parents a9daecd + f569984 commit ca0b06f

File tree

4 files changed

+31
-22
lines changed

4 files changed

+31
-22
lines changed

‎gcc/rust/backend/rust-compile-context.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,12 @@ class Context
5353
for (auto it = builtins.begin (); it != builtins.end (); it++)
5454
{
5555
HirId ref;
56-
rust_assert (
57-
tyctx->lookup_type_by_node_id ((*it)->get_node_id (), &ref));
56+
bool ok = tyctx->lookup_type_by_node_id ((*it)->get_node_id (), &ref);
57+
rust_assert (ok);
5858

5959
TyTy::BaseType *lookup;
60-
rust_assert (tyctx->lookup_type (ref, &lookup));
60+
ok = tyctx->lookup_type (ref, &lookup);
61+
rust_assert (ok);
6162

6263
Btype *compiled = TyTyCompile::compile (backend, lookup);
6364
compiled_type_map.insert (std::pair<HirId, Btype *> (ref, compiled));

‎gcc/rust/backend/rust-compile-implitem.h‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,10 @@ class CompileInherentImplItem : public HIRCompileBase
6767
Bexpression *value = CompileExpr::Compile (constant.get_expr (), ctx);
6868

6969
const Resolver::CanonicalPath *canonical_path = nullptr;
70-
rust_assert (ctx->get_mappings ()->lookup_canonical_path (
70+
ok = ctx->get_mappings ()->lookup_canonical_path (
7171
constant.get_mappings ().get_crate_num (),
72-
constant.get_mappings ().get_nodeid (), &canonical_path));
72+
constant.get_mappings ().get_nodeid (), &canonical_path);
73+
rust_assert (ok);
7374

7475
std::string ident = canonical_path->get ();
7576
Bexpression *const_expr = ctx->get_backend ()->named_constant_expression (
@@ -148,9 +149,10 @@ class CompileInherentImplItem : public HIRCompileBase
148149
flags |= Backend::function_is_visible;
149150

150151
const Resolver::CanonicalPath *canonical_path = nullptr;
151-
rust_assert (ctx->get_mappings ()->lookup_canonical_path (
152+
bool ok = ctx->get_mappings ()->lookup_canonical_path (
152153
function.get_mappings ().get_crate_num (),
153-
function.get_mappings ().get_nodeid (), &canonical_path));
154+
function.get_mappings ().get_nodeid (), &canonical_path);
155+
rust_assert (ok);
154156

155157
std::string ir_symbol_name
156158
= canonical_path->get () + fntype->subst_as_string ();
@@ -260,7 +262,7 @@ class CompileInherentImplItem : public HIRCompileBase
260262
}
261263

262264
std::vector<Bvariable *> locals;
263-
bool ok = compile_locals_for_block (*rib, fndecl, locals);
265+
ok = compile_locals_for_block (*rib, fndecl, locals);
264266
rust_assert (ok);
265267

266268
Bblock *enclosing_scope = NULL;
@@ -358,9 +360,10 @@ class CompileTraitItem : public HIRCompileBase
358360
= CompileExpr::Compile (constant.get_expr ().get (), ctx);
359361

360362
const Resolver::CanonicalPath *canonical_path = nullptr;
361-
rust_assert (ctx->get_mappings ()->lookup_canonical_path (
363+
bool ok = ctx->get_mappings ()->lookup_canonical_path (
362364
constant.get_mappings ().get_crate_num (),
363-
constant.get_mappings ().get_nodeid (), &canonical_path));
365+
constant.get_mappings ().get_nodeid (), &canonical_path);
366+
rust_assert (ok);
364367

365368
std::string ident = canonical_path->get ();
366369
Bexpression *const_expr = ctx->get_backend ()->named_constant_expression (
@@ -413,9 +416,10 @@ class CompileTraitItem : public HIRCompileBase
413416
unsigned int flags = 0;
414417

415418
const Resolver::CanonicalPath *canonical_path = nullptr;
416-
rust_assert (ctx->get_mappings ()->lookup_canonical_path (
419+
bool ok = ctx->get_mappings ()->lookup_canonical_path (
417420
func.get_mappings ().get_crate_num (), func.get_mappings ().get_nodeid (),
418-
&canonical_path));
421+
&canonical_path);
422+
rust_assert (ok);
419423

420424
std::string fn_identifier = canonical_path->get ();
421425
std::string asm_name = ctx->mangle_item (fntype, *canonical_path);
@@ -522,7 +526,7 @@ class CompileTraitItem : public HIRCompileBase
522526
}
523527

524528
std::vector<Bvariable *> locals;
525-
bool ok = compile_locals_for_block (*rib, fndecl, locals);
529+
ok = compile_locals_for_block (*rib, fndecl, locals);
526530
rust_assert (ok);
527531

528532
Bblock *enclosing_scope = NULL;

‎gcc/rust/backend/rust-compile-item.h‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,10 @@ class CompileItem : public HIRCompileBase
6868
Bexpression *value = CompileExpr::Compile (var.get_expr (), ctx);
6969

7070
const Resolver::CanonicalPath *canonical_path = nullptr;
71-
rust_assert (ctx->get_mappings ()->lookup_canonical_path (
71+
ok = ctx->get_mappings ()->lookup_canonical_path (
7272
var.get_mappings ().get_crate_num (), var.get_mappings ().get_nodeid (),
73-
&canonical_path));
73+
&canonical_path);
74+
rust_assert (ok);
7475

7576
std::string name = canonical_path->get ();
7677
std::string asm_name = ctx->mangle_item (resolved_type, *canonical_path);
@@ -103,9 +104,10 @@ class CompileItem : public HIRCompileBase
103104
Bexpression *value = CompileExpr::Compile (constant.get_expr (), ctx);
104105

105106
const Resolver::CanonicalPath *canonical_path = nullptr;
106-
rust_assert (ctx->get_mappings ()->lookup_canonical_path (
107+
ok = ctx->get_mappings ()->lookup_canonical_path (
107108
constant.get_mappings ().get_crate_num (),
108-
constant.get_mappings ().get_nodeid (), &canonical_path));
109+
constant.get_mappings ().get_nodeid (), &canonical_path);
110+
rust_assert (ok);
109111

110112
std::string ident = canonical_path->get ();
111113
Bexpression *const_expr
@@ -186,9 +188,10 @@ class CompileItem : public HIRCompileBase
186188
flags |= Backend::function_is_visible;
187189

188190
const Resolver::CanonicalPath *canonical_path = nullptr;
189-
rust_assert (ctx->get_mappings ()->lookup_canonical_path (
191+
bool ok = ctx->get_mappings ()->lookup_canonical_path (
190192
function.get_mappings ().get_crate_num (),
191-
function.get_mappings ().get_nodeid (), &canonical_path));
193+
function.get_mappings ().get_nodeid (), &canonical_path);
194+
rust_assert (ok);
192195

193196
std::string ir_symbol_name
194197
= canonical_path->get () + fntype->subst_as_string ();
@@ -259,7 +262,7 @@ class CompileItem : public HIRCompileBase
259262
}
260263

261264
std::vector<Bvariable *> locals;
262-
bool ok = compile_locals_for_block (*rib, fndecl, locals);
265+
ok = compile_locals_for_block (*rib, fndecl, locals);
263266
rust_assert (ok);
264267

265268
Bblock *enclosing_scope = NULL;

‎gcc/rust/backend/rust-compile-stmt.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,10 @@ class CompileStmt : public HIRCompileBase
6060
Bexpression *value = CompileExpr::Compile (constant.get_expr (), ctx);
6161

6262
const Resolver::CanonicalPath *canonical_path = nullptr;
63-
rust_assert (ctx->get_mappings ()->lookup_canonical_path (
63+
ok = ctx->get_mappings ()->lookup_canonical_path (
6464
constant.get_mappings ().get_crate_num (),
65-
constant.get_mappings ().get_nodeid (), &canonical_path));
65+
constant.get_mappings ().get_nodeid (), &canonical_path);
66+
rust_assert (ok);
6667

6768
std::string ident = canonical_path->get ();
6869
Bexpression *const_expr

0 commit comments

Comments
 (0)