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>: Remove EDG workaround for DevCom-10199227 #4939

Merged
merged 7 commits into from
Sep 9, 2024

Conversation

fsb4000
Copy link
Contributor

@fsb4000 fsb4000 commented Sep 5, 2024

Fixed In: Visual Studio 2022 version 17.11.2 DevCom-10199227

@fsb4000 fsb4000 requested a review from a team as a code owner September 5, 2024 16:17
@CaseyCarter CaseyCarter changed the title Remove workaround for DevCom-10199227. Remove workaround for DevCom-10199227 Sep 5, 2024
@CaseyCarter CaseyCarter added the test Related to test code label Sep 5, 2024
@CaseyCarter

This comment was marked as outdated.

@fsb4000
Copy link
Contributor Author

fsb4000 commented Sep 6, 2024

Should we add /D_USE_DETAILED_FUNCTION_NAME_IN_SOURCE_LOCATION=0 to one of PM_CL in usual_20_matrix.lst to test basic function names? is any PM_CL ok for that? After my current changes we test only detailed names.

@fsb4000 fsb4000 requested a review from CaseyCarter September 6, 2024 02:23
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Sep 6, 2024

We have so little test coverage of <source_location>, I think we should not modify the centralized matrices.

We should alter the specific test to provide coverage. (But I don't think we need to alter the modules and header units tests.)

@fsb4000
Copy link
Contributor Author

fsb4000 commented Sep 6, 2024

By the way, how do you find out C1XX values in compile time?
I mean this:

#if !defined(__clang__) && !defined(__EDG__) // TRANSITION, VSO-1285783
    if (is_constant_evaluated()) {
        assert(s.x.loc.function_name() == "main"sv);
    } else
#endif // ^^^ workaround ^^^

How can we find "main"?
It's different from _EDG_ and C1XX in runtime, and you can't print it in compile time...
Error messages are not helpful either:

C:\Dev\STL\tests\std\tests\P1208R6_source_location\test.cpp(320): error C2131: expression did not evaluate to a constant
C:\Dev\STL\tests\std\tests\P1208R6_source_location\test.cpp(174): note: failure was caused by call of undefined function or one not declared 'constexpr'
C:\Dev\STL\tests\std\tests\P1208R6_source_location\test.cpp(174): note: see usage of '_wassert'
C:\Dev\STL\tests\std\tests\P1208R6_source_location\test.cpp(320): note: the call stack of the evaluation (the oldest call first) is
C:\Dev\STL\tests\std\tests\P1208R6_source_location\test.cpp(320): note: while evaluating function 'bool test(void)'
C:\Dev\STL\tests\std\tests\P1208R6_source_location\test.cpp(305): note: while evaluating function 'void sub_member_test(void)'
C:\Dev\STL\tests\std\tests\P1208R6_source_location\test.cpp(174): note: while evaluating function 'void _wassert(const wchar_t *,const wchar_t *,unsigned int)'

For _EDG_ I created a mini app and hover my mouse to variables to find out:
изображение
изображение

tests/std/tests/P1208R6_source_location/env.lst 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
@CaseyCarter
Copy link
Member

CaseyCarter commented Sep 6, 2024

By the way, how do you find out C1XX values in compile time?

You preserve the compile time value until runtime where you can examine it. In this case, the function names all have static storage duration like string literals. You can simply return the value from the function to initialize a constinit const char* that you print in main: https://www.godbolt.org/z/jxj7PPofh.

@StephanTLavavej StephanTLavavej added enhancement Something can be improved and removed test Related to test code labels Sep 7, 2024
@StephanTLavavej
Copy link
Member

Relabeling as this is a workaround removal in product code, purr! 😻

@StephanTLavavej StephanTLavavej changed the title Remove workaround for DevCom-10199227 <source_location>: Remove EDG workaround for DevCom-10199227 Sep 7, 2024
@StephanTLavavej StephanTLavavej self-assigned this Sep 9, 2024
@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 0008e2f into microsoft:main Sep 9, 2024
39 checks passed
@StephanTLavavej
Copy link
Member

Thanks for the extensive workaround removal! 🧹 🪄 😺

@fsb4000 fsb4000 deleted the fix_devcom_10199227 branch September 10, 2024 01:03
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
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants