Skip to content

Update source_location to use __builtin_FUNCSIG#3206

Merged
StephanTLavavej merged 12 commits intomicrosoft:mainfrom
SuperWig:soloc
Dec 15, 2022
Merged

Update source_location to use __builtin_FUNCSIG#3206
StephanTLavavej merged 12 commits intomicrosoft:mainfrom
SuperWig:soloc

Conversation

@SuperWig
Copy link
Contributor

@SuperWig SuperWig commented Nov 12, 2022 •
edited
Loading

Fixes #3063

  • Should there be an escape hatch?
  • The column number is strangely different in the lambda test. x1 is different to all others 
    x1 = source_location::current()
                             ^
x2 = source_location::current()
                      ^
    Edit: scratch that, it already has a transition comment I didn't merge in.

@SuperWig SuperWig requested a review from a team as a code owner November 12, 2022 09:14
@fsb4000
Copy link
Contributor

fsb4000 commented Nov 12, 2022

one more place:

STL/tests/std/include/test_header_units_and_modules.hpp

Line 655 in 0098818

assert(sl.function_name() == "impl_test_source_location"sv);

@SuperWig
Copy link
Contributor Author

SuperWig commented Nov 12, 2022 •
edited
Loading

Oh good that's where I was already looking at

fsb4000
fsb4000 reviewed Nov 12, 2022
View reviewed changes
Copy link
Contributor

@fsb4000 fsb4000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

FAIL: std :: tests/GH_001411_core_headers:23 (23 of 29)
******************** TEST 'std :: tests/GH_001411_core_headers:23' FAILED ********************
Build setup steps:
Build steps:
Build step failed unexpectedly.
Command: "C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.35.32019\bin\HostX64\x64\cl.EXE" "-c" "C:\Dev\STL\tests\std\tests\GH_001411_core_headers\test.cpp" "-IC:\Dev\STL\out\build\x64\out\inc" "-IC:\Dev\STL\llvm-project\libcxx\test\support" "-IC:\Dev\STL\tests\std\include" "/nologo" "/Od" "/W4" "/w14061" "/w14242" "/w14265" "/w14582" "/w14583" "/w14587" "/w14588" "/w14749" "/w14841" "/w14842" "/w15038" "/w15214" "/w15215" "/w15216" "/w15217" "/w15262" "/sdl" "/WX" "/Zc:strictStrings" "/D_ENABLE_STL_INTERNAL_CHECK" "/bigobj" "/FIforce_include.hpp" "/w14365" "/D_ENFORCE_FACET_SPECIALIZATIONS=1" "/D_STL_CALL_ABORT_INSTEAD_OF_INVALID_PARAMETER" "/BE" "/c" "/EHsc" "/MT" "/std:c++20" "/permissive-" "/w14640" "/Zc:threadSafeInit-"
Exit Code: 2
Standard Output:
--
test.cpp
"C:\Dev\STL\out\build\x64\out\inc\source_location", line 31: error: identifier
          "__builtin_FUNCSIG" is undefined
          const char* const _Function_ = __builtin_FUNCSIG()) noexcept {
                                         ^

fsb4000
fsb4000 reviewed Nov 12, 2022
View reviewed changes
Copy link
Contributor

@fsb4000 fsb4000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to

STL/stl/inc/new

Line 51 in 2f8342a

#else // ^^^ workaround / no workaround vvv

SuperWig and others added 3 commits November 12, 2022 12:22
@SuperWig
Fix test for x86
e19c343
@SuperWig
formatting
2d4cf66
@SuperWig @fsb4000
[skip ci] fixup comments
d705ae8 
Co-Authored-By: Igor Zhukov <fsb4000@yandex.ru>
@fsb4000 fsb4000 mentioned this pull request Nov 13, 2022
Merged
strega-nil-ms
strega-nil-ms suggested changes Nov 14, 2022
View reviewed changes
Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the only thing I see rn; should be easily fixable and then I'm 👍

Thanks @SuperWig, and @fsb4000 for reviewing ahead of me 😄

@CaseyCarter CaseyCarter added the enhancement Something can be improved label Nov 15, 2022
@SuperWig @strega-nil-ms
Move paren
ba9f3eb 
Co-Authored-By: nicole mazzuca <83086508+strega-nil-ms@users.noreply.github.com>
SuperWig
SuperWig commented Nov 29, 2022
View reviewed changes
strega-nil-ms
strega-nil-ms reviewed Nov 29, 2022
View reviewed changes
strega-nil-ms
strega-nil-ms approved these changes Nov 29, 2022
View reviewed changes
Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much!

StephanTLavavej
StephanTLavavej reviewed Nov 30, 2022
View reviewed changes
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Nov 30, 2022

++thanks; // 😻

I pushed tiny changes to fix a comment missing an underscore, and to restore a tiny bit of test coverage (exercising what happens when a lambda's definition occurs on a different line from its call). FYI @CaseyCarter, @strega-nil-ms.

@StephanTLavavej StephanTLavavej self-assigned this Dec 5, 2022
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Dec 5, 2022

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej
Copy link
Member

StephanTLavavej commented Dec 5, 2022

Have to drop this from the current merge batch because the internal build is starting with a compiler that's too old:

D:\msvc\src\tools\vctools\Dev14\bin\x64\amd64>.\cl.exe
Microsoft (R) C/C++ Optimizing Compiler Version 19.34.31935 for x64

D:\msvc\binaries\x86chk\inc\source_location(31,38): error C3861: '__builtin_FUNCSIG': identifier not found [D:\msvc\src\vctools\Compiler\Utc\build\arm64\utc-arm64.nativeproj]

@StephanTLavavej StephanTLavavej removed their assignment Dec 5, 2022
@StephanTLavavej StephanTLavavej added the blocked Something is preventing work on this label Dec 5, 2022
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Dec 8, 2022

Casey thinks that MSVC-PR-440319 has the toolset update we need, which should be merged soon.

@StephanTLavavej StephanTLavavej removed the blocked Something is preventing work on this label Dec 8, 2022
@StephanTLavavej StephanTLavavej self-assigned this Dec 15, 2022
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Dec 15, 2022

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 8f5de32 into microsoft:main Dec 15, 2022
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Dec 15, 2022

Thanks for significantly improving the displayed information here! 📈 🚀 😻

@SuperWig SuperWig deleted the soloc branch December 19, 2022 11:51
@StephanTLavavej StephanTLavavej mentioned this pull request Sep 27, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

+3 more reviewers

@CaseyCarter CaseyCarter CaseyCarter approved these changes

@fsb4000 fsb4000 fsb4000 approved these changes

@strega-nil-ms strega-nil-ms strega-nil-ms approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@StephanTLavavej StephanTLavavej

Labels

enhancement Something can be improved

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve std::source_location::function_name() informativeness

5 participants

@SuperWig @fsb4000 @StephanTLavavej @CaseyCarter @strega-nil-ms