5

I use the clang analyzer to check my C++ code for bugs and errors. I have the following construct:

#include <cstdlib>
#include <iostream>

double
somethingThatMayThrow() throw (std::exception)
{
   if( rand() % 2 ) {
      throw std::exception();
   }
   return 5.0;
}

int
main()
{
   double value = 2.0;
   try {
      value = somethingThatMayThrow();
   } catch( const std::exception& ) {
      std::cout << "oops" << std::endl;
   }
   double someOtherValue = value + 1.0;

   std::cout << someOtherValue << std::endl;

   return 0;
}

The analyzer now complains that the initial value of variable value is never read. However, it is clear that the value is used in the last line if and only if there is an exception in the try block. Is this understanding correct, and am I looking at a bug in the analyzer? Or am I missing something here?

How does the standard define this behavior? What happens to the left hand side of the assignment if the right hand side throws?

The screenshot below shows the actual code that the analyzer complained about, which has the same structure as my example above:

clang analyzer detects a dead store in a try catch block

Arne
  • 2,624
  • 3
  • 24
  • 45
  • 2
    I think the message means that in `double value = someValue;` you never read the value of `value` before reassigning it to `somethingThatMayThrow()`. – Borgleader Aug 21 '15 at 12:50
  • 1
    "the value is used in the last line", no, not in the code and pictures you present. give a **minimal but complete** example. as text. – Cheers and hth. - Alf Aug 21 '15 at 12:50
  • 2
    @Cheersandhth.-Alf He did that. – Shoe Aug 21 '15 at 13:02
  • @Cheersandhth.-Alf My argument is that iff somethingMayThrow throws, the variable `value` will NOT be overwritten and hence the initial value will indeed be used. – Arne Aug 21 '15 at 13:04
  • 2
    @Cheersandhth.-Alf: It's just a typo. Use your brain! – Lightness Races in Orbit Aug 21 '15 at 13:04
  • @Borgleader He knows what the message means. But the value _may_ be read. – Lightness Races in Orbit Aug 21 '15 at 13:07
  • @LightnessRacesinOrbit: We were both wrong. Including your edit, he he. :) – Cheers and hth. - Alf Aug 21 '15 at 13:10
  • @Cheersandhth.-Alf: What's wrong with my edit? – Lightness Races in Orbit Aug 21 '15 at 13:10
  • Let's say the value in the initializer is 43. If there is no exception then the value 43 is used in the statement after the catch block. If there is an exception, however, then that value is overwritten and never used. I.e., it is used if and only if there is no exception. – Cheers and hth. - Alf Aug 21 '15 at 13:12
  • @Cheersandhth.-Alf: How are you getting something so simple so wrong? – Lightness Races in Orbit Aug 21 '15 at 13:13
  • @LightnessRacesinOrbit: You really need to try to explain your position down in DETAILS. I've used my brain, as you suggested. Now you do likewise, thanks. ;-) – Cheers and hth. - Alf Aug 21 '15 at 13:14
  • @Cheersandhth.-Alf: Um. Variable is initialised. Try block is entered. The RHS is evaluated. If there is no exception, the variable is given the evaluated value. If there is an exception, no assignment takes place. – Lightness Races in Orbit Aug 21 '15 at 13:16
  • Yes, okay so far. Now for each case check what values is used in the last statement. – Cheers and hth. - Alf Aug 21 '15 at 13:16
  • @Cheersandhth.-Alf: This is basic and fundamental. I'm not going to handhold you through day 3 introductory C++. Here are some resources for you to perform further research: http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list Good luck! – Lightness Races in Orbit Aug 21 '15 at 13:17
  • @LightnessRacesinOrbit: Not a good strategy to **look away**. I guarantee that you will benefit from following through on this. – Cheers and hth. - Alf Aug 21 '15 at 13:18
  • @LightnessRacesinOrbit: Kindly stop sabotaging the question. You are changing the meaning of the OP's text, to meaningless. Reversing, and please don't sabotage it for 3rd time. Thanks. – Cheers and hth. - Alf Aug 21 '15 at 13:22
  • @Cheersandhth.-Alf: **STOP TROLLING**. You are massively getting on my nerves. It's a clear typo, and all you're doing is making the text nonsense. Please grow up, seriously. Thank you. – Lightness Races in Orbit Aug 21 '15 at 13:24
  • @LightnessRacesinOrbit: Maybe you're reading "value" as "assignment", IDK. But stop sabotaging the question. – Cheers and hth. - Alf Aug 21 '15 at 13:25
  • Ugh I'm done.​​ I honestly cannot comprehend how you can seriously sit there claiming that the initial value of `value` "is used in the last line if there is no exception in the try block". The try block replaces that value. It contains one line, and that line is an assignment. An assignment replaces a value. That's what assignments do. You're denying this basic fact and using it accuse me of "sabotaging" the question and also to rollback a good, proper, anti-typo edit. Ludicrous. Go find someone else to troll. Except... I dunno. I guess I hoped we'd moved past this behaviour, Alf. – Lightness Races in Orbit Aug 21 '15 at 13:26
  • @LightnessRacesinOrbit: This is weird. You're right. I'm sorry, and I can't explain this. Just to be clear, I was not trolling. There was a serious disconnect in my brain just now. :( – Cheers and hth. - Alf Aug 21 '15 at 13:47
  • @Arne: You'd better change the "no exception" to "an exception", as Lightness fixed earlier. I'm sorry, I don't understand how I could sort of invert the logic in my brain, totally. I've even had a cup of coffee recently! – Cheers and hth. - Alf Aug 21 '15 at 13:50
  • @Cheersandhth.-Alf: lol... – Lightness Races in Orbit Aug 21 '15 at 14:06
  • @Cheersandhth.-Alf Done. – Arne Aug 21 '15 at 14:28
  • Since we can't see the implementation of `Base::asValue`, we cannot know, whether the analyzer is right or wrong. The analyzer is right, if `Base::asValue` never throws an exception, or throws an exception that is not of type `std::exception` (or derived from `std::exception`). The analyzer is wrong, if `Base::asValue` can throw exceptions of type `std::exception` (or derived from `std::exception`). – IInspectable Aug 21 '15 at 15:14

2 Answers2

5

The analyser is wrong. You are right.

The analyser could be right if the code inside the try block can never throw std::exceptions or objects of a type derived from it (e.g. with noexcept, or with only objects of other types being thrown).

Either way, your interpretation is correct: the assignment will never occur if evaluation of the value-to-be throws. As such, the original value will remain intact.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
  • " if the code inside the try block can never throw" I think function name says in plain english: "somethingThatMayThrow()" – Slava Aug 21 '15 at 13:08
  • @Slava: `Base::asValue` doesn't say that in plain english, and that's the code actually reproducing the problem. – Lightness Races in Orbit Aug 21 '15 at 13:08
  • I can assure that `Base::asValue` will throw, if it cannot parse the string that is passed to it. So no worries, it is not a `noexcept` function. – Arne Aug 21 '15 at 13:10
  • OP has access to source so he tells you that it can indeed. You want him to publish all sources? huh – Slava Aug 21 '15 at 13:10
  • @Slava: OP is not infallible and my answers go _above and beyond_ to cover that case, since I see it so often. Can I help you with something? – Lightness Races in Orbit Aug 21 '15 at 13:11
  • Yes you can, You did not cover case when LOG_WARN could throw or not, `it` is not valid iterator so it is UB. You should not stop on one moment, suspect OP lies everywhere. – Slava Aug 21 '15 at 13:14
  • 1
    @Arne: The fact, that `Base::asValue` can throw, does not necessarily mean, the analyzer is wrong. If `Base::asValue` can only throw exceptions that are not of type `std::exception` (or a type derived from `std::exception`), then the analyzer is right in its assessment. – IInspectable Aug 21 '15 at 15:19
  • @Arne: What will it throw? You should add this detail to your question. Really, you should be posting [testcases](http://stackoverflow.com/help/mcve). – Lightness Races in Orbit Aug 21 '15 at 15:35
0

The compiler sees that you are assigning someValue in the initialization of value but then, inside the try block, you are reassigning it.

And the analyzer is right in the case in which no exception is thrown, but not in the opposite case, where value will still be the same as original someValue.

Shoe
  • 74,840
  • 36
  • 166
  • 272