[clang][diagnostics] `-Wunreachable-code` prints incorrect source range if a constructor has a default value set to a builtin

[alanzhao1]

Originally reported in #91879 (comment)

After #91879, unreachable calls to constructors with default parameters set to builtins would underline the builtin as the source range instead of the call to the constructor - e.g.

struct A {
  explicit A(int = __builtin_LINE());
};

int h(int a) {
  return 3;
  A();
}

used to produce the diagnostic

<source>:7:3: error: code will never be executed [-Werror,-Wunreachable-code]
    7 |   A();
      |   ^~~

but now produces the (incorrect) diagnostic

    2 |   explicit A(int = __builtin_LINE());
      |                    ^~~~~~~~~~~~~~~~

godbolt: https://godbolt.org/z/6TEdrx55d

[aeubanks]

@yronglin @steakhal

[steakhal]

Yes, the issue makes sense.

[yronglin]

Thanks for report this, I'll take a look.

[steakhal]

@yronglin I had a look at the CFG differences and here I'll summarize what I've found.

On clang 18.1.0, we have this CFG:

void h()
 [B3 (ENTRY)]
   Succs (1): B2

 [B1]
   1: 
   2: A() (CXXConstructExpr, A)
   Succs (1): B0

 [B2]
   1: return;
   Preds (1): B3
   Succs (1): B0

 [B0 (EXIT)]
   Preds (2): B1 B2

On clang trunk we have:

void h()
 [B3 (ENTRY)]
   Succs (1): B2

 [B1]
   1: __builtin_LINE()
   2: [B1.1] (ImplicitCastExpr, IntegralCast, int)
   3: 
   4: A() (CXXConstructExpr, A)
   Succs (1): B0

 [B2]
   1: return;
   Preds (1): B3
   Succs (1): B0

 [B0 (EXIT)]
   Preds (2): B1 B2

Notice that B1 has extra two CFGElements at the beginning: CFGStmt of SourceLocExpr and an imp cast.
The 3rd CFGElement (which is dumped as an empty line -.- ) is a CFGStmt wrapping a CXXDefaultArgExpr in both clang trunk and on clang 18.1.0.

We can see that B1 is a dead basic block in the CFG. This is diagnosed in both clang versions, but we also try to report a diagnostic to the very first statement that is unreachable. This means that while previously we selected the CXXDefaultArgExpr (thus reported to the source range of that expr: A(). In contrast to this, on trunk we select the SourceLocExpr with source range __builtin_LINE().

If you could summarize to me what does this expr rewriting do or achieve, maybe I could help you fix this case.

[aeubanks]

@yronglin ping on this, wanted to make sure this bug doesn't get lost since the diagnostic message is pretty bad in clang's current state, we're hitting a lot of cases of this where it's hard to figure out the actual code location

[steakhal]

@yronglin ping on this, wanted to make sure this bug doesn't get lost since the diagnostic message is pretty bad in clang's current state, we're hitting a lot of cases of this where it's hard to figure out the actual code location

I think we are good now. It was reverted along with the actual feature PR.

[aeubanks]

I missed that, thanks. If we reland then please make sure to take this case into account, but since this is not an issue on trunk, I'll close this issue.

[yronglin]

@aeubanks @steakhal Sorry for the very late reply! Since CWG1815 was supported by clang, I have proposal a new PR to handle CXXDefaultArgExpr and CXXDefaultInitExpr in StaticAnalyzer with the bugfix of this tickit: #117437.

[steakhal]

Ill have a look tomorrow for the analyzer part.