organicmaps/organicmaps-tmp

Archived

removing strange case size_t -> int to fix compiler warning #356

Merged

Dushistov merged 1 commit from drape-ptr-refs into master

2021-04-30 10:24:10 +00:00

Dushistov commented

2021-04-29 23:29:49 +00:00

(Migrated from github.com)

segments.size() has size_t type, i also has size_t type.
We inside cycle via segments, so segments.size() > 0.
I can not see any reason to cast size_t to int before comparing.

`segments.size()` has `size_t` type, `i` also has `size_t` type. We inside cycle via `segments`, so `segments.size() > 0`. I can not see any reason to cast size_t to int before comparing.

vng commented

2021-04-30 06:38:59 +00:00

(Migrated from github.com)

I'd also prefer to take out

size_t const count = segments.size();

to prettify this code block.

I'd also prefer to take out ``` size_t const count = segments.size(); ``` to prettify this code block.

vng (Migrated from github.com) approved these changes 2021-04-30 08:48:56 +00:00

Dushistov commented

2021-04-30 08:51:39 +00:00

(Migrated from github.com)

to prettify this code block.

Done.
In fact, it helps not only for beauty. Even with modern gcc sometimes it can not understand
that result of size() is not changed and recalc it, see https://codingnest.com/the-little-things-everyday-efficiencies/
on every interation it does:

        mov     rdx, QWORD PTR [r12]
        mov     rax, QWORD PTR [r12+8]
        sub     rax, rdx
        sar     rax, 3
        imul    rax, rbp

for size() recalculation.

> to prettify this code block. Done. In fact, it helps not only for beauty. Even with modern `gcc` sometimes it can not understand that result of `size()` is not changed and recalc it, see https://codingnest.com/the-little-things-everyday-efficiencies/ on every interation it does: ``` mov rdx, QWORD PTR [r12] mov rax, QWORD PTR [r12+8] sub rax, rdx sar rax, 3 imul rax, rbp ``` for `size()` recalculation.

vng (Migrated from github.com) requested changes 2021-04-30 09:00:56 +00:00

drape_frontend/selection_shape_generator.cpp

vng (Migrated from github.com) commented

2021-04-30 09:00:51 +00:00

Forgot ; at the end.

Forgot ; at the end.

vng (Migrated from github.com) approved these changes 2021-04-30 09:56:24 +00:00

biodranik commented

2021-04-30 12:20:48 +00:00

(Migrated from github.com)

Is clang smarter?

Is clang smarter?

Dushistov commented

2021-04-30 12:42:04 +00:00

(Migrated from github.com)

@biodranik

As I see clang generates the same code, I suppose compilers think that there is some kind of
aliasing between vector element and vector by itself.
You can try it by your self with any compiler: https://godbolt.org/z/3ddeea

@biodranik As I see clang generates the same code, I suppose compilers think that there is some kind of aliasing between vector element and vector by itself. You can try it by your self with any compiler: https://godbolt.org/z/3ddeea

This repo is archived. You cannot comment on pull requests.

No reviewers

No labels

Android Automotive (AAOS)

Android Automotive (AAOS)

Battery and Performance

Battery and Performance

Bookmarks and Tracks

Bookmarks and Tracks

External Map Datasets

External Map Datasets

Frequently User Reported

Frequently User Reported

Good first issue

Good first issue

Linux packaging

Linux packaging

NLnet 2024-06-281

NLnet 2024-06-281

No Feature Parity

No Feature Parity

Public Transport

Public Transport

Track Recording

Track Recording

Walk Navigation

Walk Navigation

No milestone

No project

No assignees

1 participant

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: organicmaps/organicmaps-tmp#356

No description provided.