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

<source_location>: Use __builtin_FUNCSIG when Clang 17 is available #3729

Closed
JMazurkiewicz opened this issue May 23, 2023 · 6 comments · Fixed by #4055
Closed

<source_location>: Use __builtin_FUNCSIG when Clang 17 is available #3729

JMazurkiewicz opened this issue May 23, 2023 · 6 comments · Fixed by #4055
Labels
enhancement Something can be improved fixed Something works now, yay!

Comments

@JMazurkiewicz
Copy link
Contributor

I've implemented __builtin_FUNCSIG in Clang recently (llvm/llvm-project@78d8312):

const char* __builtin_FUNCSIG();

We should use this builtin in source_location implementation after VS gets Clang 17:

#if defined(__clang__) || defined(__EDG__) // TRANSITION, DevCom-10199227 and LLVM-58951
const char* const _Function_ = __builtin_FUNCTION()
#else // ^^^ workaround / no workaround vvv
const char* const _Function_ = __builtin_FUNCSIG()
#endif // TRANSITION, DevCom-10199227 and LLVM-58951

Comparison:

// https://godbolt.org/z/zq6v9v87q
#include <iostream>

void f(const char* old_ = __builtin_FUNCTION(), const char* new_ = __builtin_FUNCSIG()) {
    std::cout << "old: " << old_ << "\nnew: " << new_ << '\n';
}

int main() {
    f();
}
old: main
new: int __cdecl main(void)
@CaseyCarter CaseyCarter added enhancement Something can be improved blocked Something is preventing work on this labels May 24, 2023
@nuuSolutions
Copy link

I'm wondering why we can't have both the simple function name as well as the full signature.

Alternatively: How would I best go about extending/modifying this to my needs? (not only on clang)

@sylveon
Copy link
Contributor

sylveon commented Aug 13, 2023

You can always just copy the definition of source_location and customize it to your needs.

@StephanTLavavej StephanTLavavej added blocked Something is preventing work on this and removed blocked Something is preventing work on this labels Sep 27, 2023
@CaseyCarter
Copy link
Member

CaseyCarter commented Sep 27, 2023

Interestingly, __has_builtin(__builtin_FUNCSIG) doesn't appear to work: https://godbolt.org/z/6Tsa55j8T

Maybe it's only available in MSVC mode? Yeah (IIRC this is rc4):

C:\Users\cacarter\OneDrive - Microsoft\Desktop>type repro.cpp
#include <cstdio>

int main() {
#if __has_builtin(__builtin_FUNCSIG)
    std::puts("has it");
#else
    std::puts("does NOT have it");
#endif
}

C:\Users\cacarter\OneDrive - Microsoft\Desktop>"c:\Program Files\LLVM\bin\clang-cl.exe" --version
clang version 17.0.0
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: c:\Program Files\LLVM\bin

C:\Users\cacarter\OneDrive - Microsoft\Desktop>"c:\Program Files\LLVM\bin\clang-cl.exe" "c:\Users\cacarter\OneDrive - Microsoft\Desktop\repro.cpp" && repro
has it

@StephanTLavavej
Copy link
Member

Oh thanks - I deleted my comments because I had gotten very confused about what I was seeing on Compiler Explorer.

@nuuSolutions
Copy link

I'm more confused that Intellisense shows __builtin_FUNCSIG as an error while does compile & work fine (with msvc 1937)
I'm even more confused that such trivial things are not implemented/implementable in a consistent way across platforms

I did indeed have to adapt std::source_location to include what I need (i.e. the simple function name)

@StephanTLavavej
Copy link
Member

IntelliSense is powered by a different compiler front-end (EDG). MSVC's "C1XX" front-end is what's used by cl.exe to actually compile code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved fixed Something works now, yay!
Projects
None yet
5 participants