27

Вот простой C++ фрагмент кода:получил неожиданный ответ от х у: выражение г

int x1 = 10, x2=20, y1=132, y2=12, minx, miny, maxx, maxy; 
x1<=x2 ? minx=x1,maxx=x2 : minx=x2,maxx=x1; 
y1<=y2 ? miny=y1,maxy=y2 : miny=y2,maxy=y1; 
cout<<"minx="<<minx<<"\n"; 
cout<<"maxx="<<maxx<<"\n"; 
cout<<"miny="<<miny<<"\n"; 
cout<<"maxy="<<maxy<<"\n"; 

Я думал, что результат должен быть:

minx=10 
maxx=20 
miny=12 
maxy=132 

Но на самом деле результат:

minx=10 
maxx=10 
miny=12 
maxy=132 

Может ли кто-нибудь дать объяснение, почему maxx не 20? Благодарю.

+7

Parenthesizing решает проблему ... –

+24

Еще одна причина «не пытаться быть умной» с условными выражениями и использовать «if» вместо этого. Компилятор будет делать то же самое в любом случае [при условии, что вы добавите соответствующие скобки, чтобы он делал то, что вы действительно хотели). Иногда термины могут быть полезны, но это хороший пример того, что НЕ делать с ними. –

+0

(И вообще, почему бы и нет: 'maxx = x1> x2? X1: x2'? –

ответ

40

Благодаря оператору старшинства, выражение обрабатывается следующим образом:

(x1<=x2 ? minx=x1,maxx=x2 : minx=x2), maxx=x1; 

вы можете решить эту проблему с:

(x1<=x2) ? (minx=x1,maxx=x2) : (minx=x2, maxx=x1); 

И на самом деле вам не нужны первые две пары скобок. Also check this question.

25

Приоритет условного оператора выше, чем у оператора запятой, так что

x1<=x2 ? minx=x1,maxx=x2 : minx=x2,maxx=x1; 

находится в

в скобки
(x1<=x2 ? minx=x1,maxx=x2 : minx=x2),maxx=x1; 

Таким образом, последнее назначение делается независимо от состояния.

Чтобы исправить это, вы можете

  • использовать круглые скобки:

    x1 <= x2 ? (minx = x1, maxx = x2) : (minx = x2, maxx = x1); 
    

    (вы не необходимости круглые скобки в true отрасли, но ИМО лучше иметь их слишком).

  • использует два условные:

    minx = x1 <= x2 ? x1 : x2; 
    maxx = x1 <= x2 ? x2 : x1; 
    
  • использовать if:

    if (x1 <= x2) { 
        minx = x1; 
        maxx = x2; 
    } else { 
        minx = x2; 
        maxx = x1; 
    } 
    

Составитель с или без оптимизаций, версия if и скобки, одного условно с запятой произвести то же самое сборка как под gcc (4.7.2), так и clang (3.2), разумно ожидать от других компиляторов тоже. Версия с двумя условными выражениями создает разную сборку, но с оптимизацией оба этих компилятора испускают только одну команду cmp.

На мой взгляд, версия if является самой простой для проверки правильности, поэтому предпочтительнее.

+1

+1 для проверки разницы в сгенерированном коде и предоставления аргумента читаемости – fluffy

1

Из-за оператора старшинства:

(x1<=x2 ? minx=x1,maxx=x2 : minx=x2),maxx=x1

Вы можете это исправить с:

int x1=10, x2=20, y1=132, y2=12, minx, miny, maxx, maxy; 
x1<=x2 ? (minx=x1,maxx=x2) : (minx=x2,maxx=x1); 
y1<=y2 ? (miny=y1,maxy=y2) : (miny=y2,maxy=y1); 
cout<<"minx="<<minx<<"\n"; 
cout<<"maxx="<<maxx<<"\n"; 
cout<<"miny="<<miny<<"\n"; 
cout<<"maxy="<<maxy<<"\n"; 
4

Интересный вопрос и об операциях старшинства и генерации кода.

OK, , работа имеет ОЧЕНЬ низкий приоритет (самый низкий, см. reference table). В связи с этим ваш код такой же, как и следующие строки:

((x1<=x2) ? minx=x1,maxx=x2 : minx=x2),maxx=x1; 
((y1<=y2) ? miny=y1,maxy=y2 : miny=y2),maxy=y1; 

На самом деле только C/C++ грамматика предотвращает первый , из того же поведения.

Еще одно действительно опасное место в приоритете операций C/C++ - побитовые операции и сравнение. Рассмотрите о следующем фрагменте:

int a = 2; 
int b = (a == 2|1); // Looks like check for expression result? Nope, results 1! 

Глядя вперед, я бы рекомендовал переписывать фрагмент таким образом поддержанию баланса между эффективностью и читаемостью:

minx = ((x1 <= x2) ? x1 : x2); 
maxx = ((x1 <= x2) ? x2 : x1); 
miny = ((y1 <= y2) ? y1 : y2); 
maxy = ((y1 <= y2) ? y2 : y1); 

Наиболее интересным фактом об этом фрагменте коды, например стиль может привести к наиболее эффективному коду для некоторых архитектур, таких как ARM, из-за флагов состояния бит в наборе инструкций процессора (условие дублирования ничего не стоит и больше, указывает на компилятор в нужные фрагменты кода).

5

Хотя другие объяснили, что причиной этой проблемы, я считаю, что «лучше» решение должно быть, чтобы написать условное, если:

int x1 = 10, x2=20, y1=132, y2=12, minx, miny, maxx, maxy; 
if (x1<=x2) 
{ 
    minx=x1; 
    maxx=x2; 
} 
else 
{ 
    minx=x2; 
    maxx=x1; 
} 
if (y1<=y2) 
{ 
    miny=y1; 
    maxy=y2; 
} 
else 
{ 
    miny=y2; 
    maxy=y1; 
} 

Да, это несколько строк больше, но это также легче читать и четко понимать, что происходит (и если вам нужно пройти через него в отладчике, вы можете легко увидеть, в каком направлении он идет).

Любой современный компилятор должен иметь возможность конвертировать любой из них в довольно эффективные условные задания, которые делают хорошую работу по предотвращению ветвей (и, следовательно, «плохое предсказание ветвей»).

я подготовил небольшой тест, который я скомпилирован с использованием

g++ -O2 -fno-inline -S -Wall ifs.cpp 

Вот источник (я должен был сделать его параметры, чтобы обеспечить компилятор не просто вычислить правильное значение непосредственно и просто сделать mov $12,%rdx, но на самом деле сделал сравнить и решить, с больше):

void mine(int x1, int x2, int y1, int y2) 
{ 
    int minx, miny, maxx, maxy; 
    if (x1<=x2) 
    { 
    minx=x1; 
    maxx=x2; 
    } 
    else 
    { 
    minx=x2; 
    maxx=x1; 
    } 
    if (y1<=y2) 
    { 
    miny=y1; 
    maxy=y2; 
    } 
    else 
    { 
    miny=y2; 
    maxy=y1; 
    } 

    cout<<"minx="<<minx<<"\n"; 
    cout<<"maxx="<<maxx<<"\n"; 
    cout<<"miny="<<miny<<"\n"; 
    cout<<"maxy="<<maxy<<"\n"; 
} 

void original(int x1, int x2, int y1, int y2) 
{ 
    int minx, miny, maxx, maxy; 
    x1<=x2 ? (minx=x1,maxx=x2) : (minx=x2,maxx=x1); 
    y1<=y2 ? (miny=y1,maxy=y2) : (miny=y2,maxy=y1); 
    cout<<"minx="<<minx<<"\n"; 
    cout<<"maxx="<<maxx<<"\n"; 
    cout<<"miny="<<miny<<"\n"; 
    cout<<"maxy="<<maxy<<"\n"; 
} 

void romano(int x1, int x2, int y1, int y2) 
{ 
    int minx, miny, maxx, maxy; 

    minx = ((x1 <= x2) ? x1 : x2); 
    maxx = ((x1 <= x2) ? x2 : x1); 
    miny = ((y1 <= y2) ? y1 : y2); 
    maxy = ((y1 <= y2) ? y2 : y1); 
    cout<<"minx="<<minx<<"\n"; 
    cout<<"maxx="<<maxx<<"\n"; 
    cout<<"miny="<<miny<<"\n"; 
    cout<<"maxy="<<maxy<<"\n"; 
} 

int main() 
{ 
    int x1=10, x2=20, y1=132, y2=12; 
    mine(x1, x2, y1, y2); 
    original(x1, x2, y1, y2); 
    romano(x1, x2, y1, y2); 
    return 0; 
} 

Сгенерированный код выглядит следующим образом:

_Z4mineiiii: 
.LFB966: 
    .cfi_startproc 
    movq %rbx, -32(%rsp) 
    movq %rbp, -24(%rsp) 
    movl %ecx, %ebx 
    movq %r12, -16(%rsp) 
    movq %r13, -8(%rsp) 
    movl %esi, %r12d 
    subq $40, %rsp 
    movl %edi, %r13d 
    cmpl %esi, %edi 
    movl %edx, %ebp 
    cmovg %edi, %r12d 
    cmovg %esi, %r13d 
    movl $_ZSt4cout, %edi 
    cmpl %ecx, %edx 
    movl $.LC0, %esi 
    cmovg %edx, %ebx 
    cmovg %ecx, %ebp 
     .... removed actual printout code that is quite long and unwieldy... 
_Z8originaliiii: 
    movq %rbx, -32(%rsp) 
    movq %rbp, -24(%rsp) 
    movl %ecx, %ebx 
    movq %r12, -16(%rsp) 
    movq %r13, -8(%rsp) 
    movl %esi, %r12d 
    subq $40, %rsp 
    movl %edi, %r13d 
    cmpl %esi, %edi 
    movl %edx, %ebp 
    cmovg %edi, %r12d 
    cmovg %esi, %r13d 
movl $_ZSt4cout, %edi 
cmpl %ecx, %edx 
movl $.LC0, %esi 
cmovg %edx, %ebx 
cmovg %ecx, %ebp 
     ... print code goes here ... 
_Z6romanoiiii: 
    movq %rbx, -32(%rsp) 
    movq %rbp, -24(%rsp) 
    movl %edx, %ebx 
    movq %r12, -16(%rsp) 
    movq %r13, -8(%rsp) 
    movl %edi, %r12d 
    subq $40, %rsp 
    movl %esi, %r13d 
    cmpl %esi, %edi 
    movl %ecx, %ebp 
    cmovle %edi, %r13d 
    cmovle %esi, %r12d 
movl $_ZSt4cout, %edi 
cmpl %ecx, %edx 
movl $.LC0, %esi 
cmovle %edx, %ebp 
cmovle %ecx, %ebx 
     ... printout code here.... 

Как вы можете видеть, mine и original идентичны, и romano использует несколько разные регистры и другую форму cmov, но в остальном они делают то же самое с одинаковым количеством инструкций.

+1

+1 Код, размещенный в вопросе, - это именно тот код, который заставляет многие организации просто запрещать тройной оператор прямо. Тернарный оператор имеет свое место, но опубликованное использование не в этом месте. –

+1

Re * Да, Это Несколько строк длиннее * - Было бы не так уж плохо, если бы вы использовали 1TBS. :) –

+0

@DavidHammen Да, вы можете его по-другому стилизовать [это как раз то, как я обычно пишу свой собственный код], но, вероятно, нет (не делая его довольно запутанным) довести его до двух строк ... Даже четыре разумно читаемых линии подталкивают его. Таким образом, утверждение все еще «несколько строк длиннее». И я хотел бы сделать его более читаемым, а не подходящим для входа в IOCCC. –

1

В C++ 11 вы можете использовать std::tie и std::make_pair, чтобы сделать это, очевидно, правильно, на наглядную (TM)

#include <tuple> 
#include <utility> 
#include <iostream> 

using namespace std; 

int main() 
{ 
    int x1 = 10, x2=20, y1=132, y2=12, minx, miny, maxx, maxy; 

    tie(minx, maxx) = (x1 <= x2)? make_pair(x1, x2) : make_pair(x2, x1); 
    tie(miny, maxy) = (y1 <= y2)? make_pair(y1, y2) : make_pair(y2, y1); 

    cout<<"minx="<<minx<<"\n"; 
    cout<<"maxx="<<maxx<<"\n"; 
    cout<<"miny="<<miny<<"\n"; 
    cout<<"maxy="<<maxy<<"\n"; 
} 

output Online.

Это семантически эквивалентно всем остальным опубликованным решениям и с любым достойным оптимизирующим компилятором не имеет никаких накладных расходов. Это синтаксически гораздо лучше, потому что он имеет

  • минимум кода повторения,
  • 4 назначены к-переменных все на левой стороне задания, и
  • 4 назначены: от переменные все справа.

В качестве небольшого изменения, обобщающего к нахождению указателей к минимальному и максимальному элементу последовательности, можно использовать std::minmax_element и тот факт, что исходные массивы имеют функции, не являющиеся члены begin() и end() (обе функции C++ 11)

#include <algorithm> 
#include <tuple> 
#include <iostream> 

using namespace std; 

int main() 
{ 
    int x[] = { 10, 20 }, y[] = { 132, 12 }, *minx, *miny, *maxx, *maxy; 

    tie(minx, maxx) = minmax_element(begin(x), end(x)); 
    tie(miny, maxy) = minmax_element(begin(y), end(y)); 

    cout<<"minx="<<*minx<<"\n"; 
    cout<<"maxx="<<*maxx<<"\n"; 
    cout<<"miny="<<*miny<<"\n"; 
    cout<<"maxy="<<*maxy<<"\n"; 
} 

output.

Смежные вопросы