Skip to content

<source_location>: Light up detailed function_name() for Clang 17, add escape hatch#4055

Merged
StephanTLavavej merged 4 commits intomicrosoft:mainfrom
StephanTLavavej:one-door-leads-to-the-source
Oct 4, 2023
Merged

<source_location>: Light up detailed function_name() for Clang 17, add escape hatch#4055
StephanTLavavej merged 4 commits intomicrosoft:mainfrom
StephanTLavavej:one-door-leads-to-the-source

Conversation

@StephanTLavavej
Copy link
Member

@StephanTLavavej StephanTLavavej commented Sep 27, 2023

Fixes #3729.

After #3206 in VS 2022 17.6 made source_location::function_name() more detailed, some users experienced significant increases in binary size (see microsoft/cppwinrt#1337), because heavy usage generated lots of long strings which couldn't be folded away. microsoft/cppwinrt#1260 added an escape hatch for that project, but we should add one too, which fixes VSO-1893424 / AB#1893424 .

This uses Clang's __has_builtin to detect the upcoming support. When supported, enabling the detailed function name by default is still the right design - it's much more useful.

C:\Temp>type meow.cpp

#include <print>
#include <source_location>
#include <vector>
using namespace std;

void purr(source_location loc = source_location::current()) {
    println("{}", loc.function_name());
}

template <typename T>
void cats() {
    purr();
}

int main() {
#ifdef __clang__
    println("Clang {}", __clang_major__);
#else
    println("MSVC");
#endif
    cats<vector<int>>();
}
C:\Temp>cl /EHsc /nologo /W4 /std:c++latest meow.cpp && meow
meow.cpp
MSVC
void __cdecl cats<class std::vector<int,class std::allocator<int> >>(void)

C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /D_USE_DETAILED_FUNCTION_NAME_IN_SOURCE_LOCATION=0 meow.cpp && meow
meow.cpp
MSVC
cats

C:\Temp>clang-cl /EHsc /nologo /W4 /std:c++latest meow.cpp && meow
Clang 16
cats

C:\Temp>set PATH=C:\Users\stl\Downloads\LLVM-17.0.1-win64\bin;%PATH%

C:\Temp>clang-cl /EHsc /nologo /W4 /std:c++latest meow.cpp && meow
Clang 17
void __cdecl cats(void) [T = std::vector<int>]

C:\Temp>clang-cl /EHsc /nologo /W4 /std:c++latest /D_USE_DETAILED_FUNCTION_NAME_IN_SOURCE_LOCATION=0 meow.cpp && meow
Clang 17
cats

@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Sep 27, 2023
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner September 27, 2023 23:07
CaseyCarter
CaseyCarter reviewed Sep 28, 2023
View reviewed changes
@CaseyCarter

This comment was marked as resolved.

Sign in to view
@StephanTLavavej

This comment was marked as resolved.

Sign in to view
@StephanTLavavej

This comment was marked as resolved.

Sign in to view
@StephanTLavavej
Copy link
Member Author

StephanTLavavej commented Sep 28, 2023 •
edited
Loading

Ok, I've validated and pushed changes. The test now passes with MSVC, Clang 16, and Clang 17, for both x64 and x86. I also extracted the calling convention variation (using constexpr basic_string!) because that logic was getting unwieldy.

The changes to test_header_units_and_modules.hpp are "speculative" because neither Clang nor EDG are running that yet, but I verified (with classic includes) that Clang will produce that output. Because the named modules don't emit macros, I determine "by hand" whether we're using detailed info.

CaseyCarter
CaseyCarter approved these changes Sep 29, 2023
View reviewed changes
@CaseyCarter

This comment was marked as resolved.

Sign in to view
@azure-pipelines

This comment was marked as resolved.

Sign in to view
@StephanTLavavej StephanTLavavej self-assigned this Oct 3, 2023
@StephanTLavavej
Copy link
Member Author

StephanTLavavej commented Oct 3, 2023

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

@StephanTLavavej StephanTLavavej merged commit 94d9fdd into microsoft:main Oct 4, 2023
@StephanTLavavej StephanTLavavej deleted the one-door-leads-to-the-source branch October 4, 2023 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@CaseyCarter CaseyCarter CaseyCarter 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.

<source_location>: Use __builtin_FUNCSIG when Clang 17 is available

2 participants

@StephanTLavavej @CaseyCarter