This is an archive of the discontinued LLVM Phabricator instance.

Differential D106320

[ADT] Remove (void) in loop as it is unnecessary and confusing
AbandonedPublic

Authored by gAlfonso-bit on Jul 19 2021, 3:09 PM.

Details

Reviewers
gAlfonso-bit
Group Reviewers
Restricted Project
Summary

Usually (void) is used when a return value is discarded or when it comes before a declaration or definition of a function. It has no place here and is unnecessarily confusing.

Diff Detail

Unit TestsFailed

TimeTest
1,965,710 msx64 debian > LLVM.CodeGen/AArch64/GlobalISel::legalize-constant.mir
2,910 msx64 debian > libarcher.barrier::barrier.c
2,660 msx64 debian > libarcher.critical::critical.c
2,630 msx64 debian > libarcher.critical::lock-nested.c
2,760 msx64 debian > libarcher.races::critical-unrelated.c
View Full Test Results (21 Failed)

Event Timeline

gAlfonso-bit created this revision.Jul 19 2021, 3:09 PM
gAlfonso-bit created this object with edit policy "Administrators".
Herald added a subscriber: dexonsmith. · View Herald TranscriptJul 19 2021, 3:09 PM
gAlfonso-bit requested review of this revision.Jul 19 2021, 3:09 PM
Herald added a subscriber: llvm-commits. · View Herald TranscriptJul 19 2021, 3:09 PM
Harbormaster completed remote builds in B114962: Diff 359934.Jul 19 2021, 5:20 PM
gAlfonso-bit updated this revision to Diff 360256.Jul 20 2021, 1:41 PM

did a decrement after size is used so clang format doesn’t complain

gAlfonso-bit updated this revision to Diff 360257.Jul 20 2021, 1:43 PM

Found a better solution

gAlfonso-bit retitled this revision from Remove (void) in loop as it is unnecessary and confusing to [LLVM] Remove (void) in loop as it is unnecessary and confusing.Jul 20 2021, 1:45 PM
gAlfonso-bit accepted this revision.Jul 20 2021, 1:48 PM
This revision is now accepted and ready to land.Jul 20 2021, 1:48 PM
gAlfonso-bit requested review of this revision.Jul 20 2021, 1:48 PM
gAlfonso-bit resigned from this revision.
dblaikie added a subscriber: dblaikie.Jul 20 2021, 2:30 PM

Why is inverting the order of the inc/dec significant/needed?

Harbormaster completed remote builds in B115178: Diff 360257.Jul 20 2021, 3:46 PM
gAlfonso-bit updated this revision to Diff 360642.Jul 21 2021, 4:43 PM
gAlfonso-bit updated this revision to Diff 360685.Jul 21 2021, 7:10 PM

Really trying to please the linter here

gAlfonso-bit retitled this revision from [LLVM] Remove (void) in loop as it is unnecessary and confusing to [ADT] Remove (void) in loop as it is unnecessary and confusing.Jul 21 2021, 7:10 PM
Harbormaster completed remote builds in B115477: Diff 360685.Jul 21 2021, 8:39 PM
gAlfonso-bit abandoned this revision.Jul 27 2021, 8:58 AM

Revision Contents

PathSize
llvm/
include/
llvm/
ADT/
4 lines

Diff 360685

llvm/include/llvm/ADT/STLExtras.h

Context not available.
// but that would be stdlib dependent. // but that would be stdlib dependent.
typedef typedef
typename std::iterator_traits<Iterator>::difference_type difference_type; typename std::iterator_traits<Iterator>::difference_type difference_type;
for (auto size = last - first; size > 1; ++first, (void)--size) { for (auto Size = last - first; Size > 1; --Size, ++first) {
difference_type offset = g() % size; difference_type offset = g() % Size;
Lint: Pre-merge checks
Inline Actions

clang-tidy: warning: invalid case style for variable 'offset' [readability-identifier-naming]
not useful

Lint: Pre-merge checks: clang-tidy: warning: invalid case style for variable 'offset' [readability-identifier-naming]…
// Avoid self-assignment due to incorrect assertions in libstdc++ // Avoid self-assignment due to incorrect assertions in libstdc++
// containers (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85828). // containers (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85828).
if (offset != difference_type(0)) if (offset != difference_type(0))
Context not available.