Skip to content

Commit f569984

Browse files
committed
No side effects in 'assert' expressions
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?
1 parent cba61d8 commit f569984

File tree

4 files changed

+31
-22
lines changed

4 files changed

+31
-22
lines changed

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

+4-3
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

+14-10
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

+10-7
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

+3-2
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)