@llvm/issue-subscribers-c-20

I will take look at this

I would love to help out on this task if I can. So let me know if there is anything I could do to help.

Progress update from me.
I have a half-working fix (D129488), but it breaks consteval functions (see comments).
@jacobsa was about to pick it up from there.

@aasimon we'll let you know if we don't make progress here, so you can pick it up.
Unless you feel really strongly about taking this over.

@aasimon we'll let you know if we don't make progress here, so you can pick it up. Unless you feel really strongly about taking this over.

I hardly think I am qualified for taking over the task ;-) I was more referring to helping out with testing and such.

Just for the record, the standard does agree that the current behavior is incorrect. From [support.srcloc.cons]/2 in N4868:

Any call to current that appears as a default argument ([dcl.fct.default]), or as a subexpression thereof, should correspond to the location of the invocation of the function that uses the default argument ([expr.call]).

Just for the record, the standard does agree that the current behavior is incorrect. From [support.srcloc.cons]/2 in N4868:

Any call to current that appears as a default argument ([dcl.fct.default]), or as a subexpression thereof, should correspond to the location of the invocation of the function that uses the default argument ([expr.call]).

Currently the wording is:

Any call to current that appears as a default member initializer ([class.mem]), or as a subexpression thereof, should correspond to the location of the constructor definition or aggregate initialization that uses the default member initializer.

But the example is not default member initializer.

I'm not sure what you mean. The sentence I quoted comes after the one you quoted, right?

I'm not sure what you mean. The sentence I quoted comes after the one you quoted, right?

Yes, my bad for oversee.

I'm currently working on implementing CWG2631 to resolve this issue.

I'm doing multiple things, all of which are probably necessary / useful

• Implement CWG2631 by delaying immediate functions evaluation, and rebuilding expressions when they are used. I avoid rewriting when there is no immediate call. I suspect we will struggle properly rewriting lambdas, blocks...
• Treat __builtin_source_location* as immediate "calls"
• Instead of having overwriting scopes of default arguments , we need to keep track of the outermost default argument/init list
• Then we should be able to remove the codegen code path for source location

@AaronBallman

Thanks for looking. I should have updated above: I kind of gave up on this, because it's a little more involved than expected and I don't have a lot of experience within clang proper.

@cor3ntin hi, although I didn't look the problems in detail, I believe the proper fix will address the issue too: #57459

Hope this helps.

@ChuanqiXu9 There is more work to be done for modules to support that, maybe you could take a look once my PR is merged?
I'm not familiar with how export / reachable works in clang

maybe you could take a look once my PR is merged?

Sure. I looked at your WIP patches and it seems like more complex than I imaged. I imaged we'll transform the calls with default arguments to the general style. For example, https://godbolt.org/z/vx4x5Er8h I imaged with the imaged "proper" fix, the dumped AST would be:

BinaryOperator 0x561a91dfd9b0 <col:12, col:27> 'int' '+'
        |-CallExpr 0x561a91ddd8c0 <col:12, col:16> 'int'
        | |-ImplicitCastExpr 0x561a91ddd8a8 <col:12> 'int (*)(B)' <FunctionToPointerDecay>
        | | `-DeclRefExpr 0x561a91ddd830 <col:12> 'int (B)' lvalue Function 0x561a91ddd5b8 'foo' 'int (B)'
        | `-CXXTemporaryObjectExpr 0x561a91ddd8e8 <<invalid sloc>> 'B':'B'
        `-CallExpr 0x561a91dfd988 <col:20, col:27> 'int'
          |-ImplicitCastExpr 0x561a91dfd970 <col:20> 'int (*)(B)' <FunctionToPointerDecay>
          | `-DeclRefExpr 0x561a91dfd950 <col:20> 'int (B)' lvalue Function 0x561a91ddd5b8 'foo' 'int (B)'
          `-CXXTemporaryObjectExpr 0x561a91dfd920 <col:24, col:26> 'B':'B' 'void () noexcept' zeroing

Then we'll get things addressed correctly. But I may took things too easily. There may be more works on the module's semantics' side.

This will be fixed by https://reviews.llvm.org/D136554

@llvm/issue-subscribers-clang-frontend

This is now fixed on main.

This is awesome news :-D

Any idea which clang version the fix will end up being released in?

Any idea which clang version the fix will end up being released in?

It's expected to be released in Clang 16.

Note that this was reverted temporarily as we found bugs in the fix, but we are still targeting Clang 16

Please backport it to clang 15.0.x

And, if the fix has not landed again, I suggest to re-open the bug.

Alas, it's a pretty involved changed, both in the compiler and in the language, so it's not a great candidate for backporting.
I'm still working through a few issues but I'm hoping to be able to re-reland that shortly.

@cor3ntin re-landed it ca61961 a few days ago, looks like it's stick this time, so:
Fixed for Clang 16. 🎉