1

As I've learned that the conditional operator (ternary operator "?") guarantees the order of the evaluation of its operands I want to know whether assigning to a variable the return of ? where this variable is used in one of its two exprs. be a UB or not. Here is what I have:

I've written this program that tries to convert a string into an integer depending on whether the string str contains a valid numeric character e.g: +-.0123456789, So I've used std::stoi combined with std::string::find_first_of and conditional operator in one expression:

std::string str = "a^%^&fggh67%$hf#$";
std::size_t pos = 0;
auto val = (((pos = str.find_first_of("0123456789.+-")) != std::string::npos) ?
    std::stoi(str.substr(pos)) : -1);

std::cout << val << std::endl; // 67

str = "a^%^&fggh__#$!%$hf#$";
pos = 0;
val = (((pos = str.find_first_of("0123456789.+-")) != std::string::npos) ?
    std::stoi(str.substr(pos)) : -1);

std::cout << val << std::endl; // -1

As you can see the piece of code looks to work fine, I know that if value -1 was accidentally found in str, we don't know whether val holds -1 of successful operation or failure (? operator). But I want to know only whether this code which I've written has UB or not?

  • Thanks to members who guided me in the previous topic which was UB and I've corrected it know.
NathanOliver
  • 171,901
  • 28
  • 288
  • 402
Itachi Uchiwa
  • 3,044
  • 12
  • 26
  • 1
    It's not UB, but ugly in my opinion, I wouldn't do this. – geza Sep 03 '19 at 20:26
  • @geza: I am learning to write more-complex and valid and safe-expressions from what I've learned. But in a real world program it'll be so easy for me to make those expressions simpler. – Itachi Uchiwa Sep 03 '19 at 20:30
  • 1
    @ItachiUchiwa "I am learning to write more-complex and valid and safe-expressions " - Writing "more complex" expressions should *not* be a goal. Write *simple* expressions. They will be easier for human readers to understand and, in most cases, also easier for the compiler to optimize. – Jesper Juhl Sep 03 '19 at 20:34
  • Okay. Use this information to understand badly written code, but not for using it. I consider this a bad practice. Code is much clearer, if you set `pos` before the conditional expression. – geza Sep 03 '19 at 20:36
  • You should first wrap it into a function. Later instead of `std::size_t pos = 0` and then `pos = str.find_first_of...` on the comparison, do `auto pos= str.find_first_of...` and then a much cleaner `pos != std::string::npos` or `pos != str.npos` (i think the latter works). Instead of -1 you should use a variable like `int defaultValue= -1` in such a function. – Mirko Sep 03 '19 at 20:54
  • 1
    Using an object twice does not cause undefined behaviour. Modifying an object twice does (except in some cases in recent standards). In your case, evaluation of the condition (which modifies `pos`) is sequenced before either of the possible expressions - including the possible usage of `pos` - so the code does not have undefined behaviour. It is pretty hard to read though, and code which is hard to understand is also unnecessarily hard to get working correctly. – Peter Sep 04 '19 at 01:43

1 Answers1

6

The code does not have undefined behavior. The condition operator is defined by [expr.cond]/1 as

Conditional expressions group right-to-left. The first expression is contextually converted to bool. It is evaluated and if it is true, the result of the conditional expression is the value of the second expression, otherwise that of the third expression. Only one of the second and third expressions is evaluated. Every value computation and side effect associated with the first expression is sequenced before every value computation and side effect associated with the second or third expression.

emphasis mine

So, ((pos = str.find_first_of("0123456789.+-")) != std::string::npos) is evaluated, a sequence point is "reached", and then depending on the result std::stoi(str.substr(pos)) or -1 is evaluated in its own sequenced expression.

NathanOliver
  • 171,901
  • 28
  • 288
  • 402