-
Notifications
You must be signed in to change notification settings - Fork 11.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
std::source_location line number wrong when used as default argument value #56379
Comments
@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. |
@aasimon we'll let you know if we don't make progress here, so you can pick it up. |
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:
|
Currently the wording is:
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? |
Yes, my bad for oversee. |
- Removed non-standard extensions - Workaround for llvm/llvm-project#56379 - Formatting
I'm currently working on implementing CWG2631 to resolve this issue. |
I'm doing multiple things, all of which are probably necessary / useful
|
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. |
@ChuanqiXu9 There is more work to be done for modules to support that, 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:
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? |
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. |
Looks like a regression: llvm/llvm-project#56379
Consider the following code snipet (CompilerExplorer link):
The output results for Clang, GCC and MSVC (left-to-right) are:
As you can see - both GCC and MSVC reports function INVOCATION line number, while Clang report function DECLARATION-one
The text was updated successfully, but these errors were encountered: