> On Jan. 31, 2016, 5:04 a.m., Andreas Sandberg wrote: > > I just noticed that we have the occasional foo == NULL / foo != NULL check > > in the code base. I usually consider this to be on par with foo == False, > > should we check for that as well?
This doesn't bother me nearly as much. It's unnecessary, but it makes an implicit pointer-to-bool conversion explicit, which some might argue is good for readability. In contrast, the boolean comparisons eliminated in this patch are truly redundant in every sense of the word. I could go either way though; I wouldn't miss the comparisons with NULL if we did eliminate them. It would be nice to hear from others on this. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3308/#review7974 ----------------------------------------------------------- On Jan. 29, 2016, 5:30 p.m., Steve Reinhardt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3308/ > ----------------------------------------------------------- > > (Updated Jan. 29, 2016, 5:30 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11321:42f3fc1ea44c > --------------------------- > style: eliminate explicit boolean comparisons > > Result of running 'hg m5style --skip-all --fix-control -a' to get > rid of '== true' comparisons, plus trivial manual edits to get > rid of '== false'/'== False' comparisons. > > Left a couple of explicit comparisons in where they didn't seem > unreasonable: > invalid boolean comparison in src/arch/mips/interrupts.cc:155 > >> DPRINTF(Interrupt, "Interrupts OnCpuTimerINterrupt(tc) == > >> true\n");<< > invalid boolean comparison in src/unittest/unittest.hh:110 > >> "EXPECT_FALSE(" #expr ")", (expr) == false)<< > > > Diffs > ----- > > src/arch/hsail/gen.py 89fd4a775287e4fcb1313f12ef16d426971f615a > src/arch/hsail/insts/decl.hh 89fd4a775287e4fcb1313f12ef16d426971f615a > src/arch/hsail/insts/mem.hh 89fd4a775287e4fcb1313f12ef16d426971f615a > src/cpu/base.cc 89fd4a775287e4fcb1313f12ef16d426971f615a > src/dev/net/dist_iface.cc 89fd4a775287e4fcb1313f12ef16d426971f615a > src/mem/ruby/common/WriteMask.hh 89fd4a775287e4fcb1313f12ef16d426971f615a > src/mem/ruby/network/garnet/fixed-pipeline/SWallocator_d.cc > 89fd4a775287e4fcb1313f12ef16d426971f615a > src/mem/ruby/system/GPUCoalescer.cc > 89fd4a775287e4fcb1313f12ef16d426971f615a > src/mem/slicc/symbols/Transition.py > 89fd4a775287e4fcb1313f12ef16d426971f615a > > Diff: http://reviews.gem5.org/r/3308/diff/ > > > Testing > ------- > > > Thanks, > > Steve Reinhardt > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
