Skip to content
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

Update source_location to use __builtin_FUNCSIG #3206

Merged
merged 12 commits into from
Dec 15, 2022

Conversation

SuperWig
Copy link
Contributor

@SuperWig SuperWig commented Nov 12, 2022

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:

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

@SuperWig
Copy link
Contributor Author

SuperWig commented Nov 12, 2022

Oh good that's where I was already looking at

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 {
                                         ^

stl/inc/source_location Outdated Show resolved Hide resolved
tests/std/include/test_header_units_and_modules.hpp Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/header.h Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/test.cpp Outdated Show resolved Hide resolved
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

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

stl/inc/source_location Outdated Show resolved Hide resolved
tests/std/include/test_header_units_and_modules.hpp Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/header.h Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1208R6_source_location/test.cpp Outdated Show resolved Hide resolved
SuperWig and others added 3 commits November 12, 2022 12:22
Co-Authored-By: Igor Zhukov <fsb4000@yandex.ru>
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.

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 😄

stl/inc/source_location Outdated Show resolved Hide resolved
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Nov 15, 2022
Co-Authored-By: nicole mazzuca <83086508+strega-nil-ms@users.noreply.github.com>
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
Copy link
Member

++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

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

@StephanTLavavej
Copy link
Member

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

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

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve std::source_location::function_name() informativeness
5 participants