Implement <source_location>

[SuperWig]

Description

This PR is blocked until issues with the compiler builtins are solved (as well as consteval?)

So after now being able to run the tests there's some more potential oddities I found. Thought may as well open a draft PR as it might be helpful for compiler devs.

So following what I found in #54.

• Tests 17 and 18 fail the column number asserts except when current() is used as a default argument (i.e. 1st argument test, sloc, and lambda pass)

•  source_location::current();
   ^
   source_location::current();
                    ^
  

  When used as a lambda capture the column number is the first otherwise the second.

Issues to resolve

• consteval source_location::current()
• constexpr __builtin_FILE() and __builtin_FUNCTION()
• Consistent column number location (lambda_test())
• Column number when not a default argument under tests 17 and 18
• Column number with tabs
• sub_member_test()
• NTTP test

---

I'm unsure how to test the tab column issue as that would mean this file fails validation, right? Thought maybe put it in a different file with a file extension that could be skipped e.g. test.tab?

Is using ends_with sufficient for the file name tests?

Checklist

Be sure you've read README.md and understand the scope of this repo.

If you're unsure about a box, leave it unchecked. A maintainer will help you.

• Identifiers in product code changes are properly _Ugly as per
  https://eel.is/c++draft/lex.name#3.1 or there are no product code changes.
• The STL builds successfully and all tests have passed (must be manually
  verified by an STL maintainer before automated testing is enabled on GitHub,
  leave this unchecked for initial submission).
• These changes introduce no known ABI breaks (adding members, renaming
  members, adding virtual functions, changing whether a type is an aggregate
  or trivially copyable, etc.).
• These changes were written from scratch using only this repository,
  the C++ Working Draft (including any cited standards), other WG21 papers
  (excluding reference implementations outside of proposed standard wording),
  and LWG issues as reference material. If they were derived from a project
  that's already listed in NOTICE.txt, that's fine, but please mention it.
  If they were derived from any other project (including Boost and libc++,
  which are not yet listed in NOTICE.txt), you must mention it here,
  so we can determine whether the license is compatible and what else needs
  to be done.

[StephanTLavavej]

Microsoft-internal VSO-778944 tracks consteval support in the compiler, currently being implemented by Jennifer Yao.

[xiangfan-ms]

Hi,
The compiler intrinsics expose the internal source location in the compiler. The implementation doesn't try to change the existing source location. If you find some source locations confusing, we can take a closer look and see whether we can adjust them (though it often has implications on other things like diagnosis, debugging behavior, static analysis, etc.).

For some of the issues you raised:

1. The source code is stored as UTF-8 internally. The column is the number of bytes between the current character and the first character at the beginning of the line. So tab is counted as 1 and a Japanese character may be counted as 3.
   You can validate the column using /diagnostics:column
2. The source location of a qualified-id points to the start of the last component.
3. The ''s2 x;" example involves compiler generated function (ctor of s2). Its body reuses whatever the source location the last parsing left, so in general it isn't something reliable. You can validate using the debugger :-).

[SuperWig]

1. Thanks for clarifying :)
2. I figured this would work differently it would be impossible to get a column number less than 18 with how it is now just realised you could put current() on a new line, looks weird though. Clang has it working the way I was expecting
   https://godbolt.org/z/NAhDEW
3. Ah, so this should work how I was expecting? (it does with Clang)

   struct s2 {
       s x = source_location::current();
   };

   Clang gives the location of s2 for the declaration of s2. MSVC gives the location of } for s2 both of which make sense, I guess.
   EDIT: Derp, I'm missing the constructor that takes source_location aren't I. Now I'm getting the same thing as s. So that code above is testing something else (which doesn't seem like a practical use of the feature, worth testing that?).
   Interestingly Clang rejects that code due to "cannot take address of consteval function 'current' outside of an immediate invocation" for having a = source_location::current() constructor. Is this just not supposed to work or is Clang incorrect to reject that?

Note: I'm not arguing one way or the other for what the column numbers are supposed to be, just explaining my reasons for my expectations.

[StephanTLavavej]

Removing the blocked tag - while MSVC's frontend C1XX is still working on consteval, Clang supports it, and we can ship features active for Clang only at first (as we have done so in the past with CTAD).

[SuperWig]

Should I remove the test from the test.lst? I suppose the tab test isn't needed after that clarification, so that's not needed either, right?

[StephanTLavavej]

For a test that works for some compilers but not others, I think the current best way is to #ifdef within the test itself.

For tab characters, we have the ability to exempt files from the "no tabs anywhere" validation (see tabby_filenames in validate.cpp 🐈 ), but I think that the STL's tests should validate basic functionality of source_location - the behavior in corner cases like tab characters should be left up to the compiler. Other maintainers may feel differently about "end-to-end" testing here.

[SuperWig]

Had to change the header file name test as clang expects

tests\std\tests\P1208R6_source_location/header.h

while MSVC would expect

tests\std\tests\P1208R6_source_location\header.h

if I'm not mistaken. Or would it be prefered to do an #ifdef for that?

[StephanTLavavej]

I think what you did is fine for now, but I'd recommend filing a bug against Clang, reporting their behavior divergence from MSVC (I think they're the "strange" ones since they're not consistently using Windows' preferred separators in the path).

[SuperWig]

Not being all \ or / was definitely strange when trying to figure out why that test was failing...

[SuperWig]

Unless clang plans on changing their column number, shouldn't there be #ifdefs for clang?

[StephanTLavavej]

We'll need to add ifdefs for Clang and EDG when we can activate them.

[SuperWig]

Actually, since Clang is going to be self activating would it not make sense to pre-emptively have a function like this to compare against instead?

constexpr int get_c(int n)
{
#ifdef __clang__
    return n - 17;
#elif defined(__EDG__)
    return n + 8;
#else
    return n;
#endif
}

That way its less work for the PR that updates Clang to a version that defines __cpp_consteval

[StephanTLavavej]

I would prefer to defer any work for Clang until the toolset update that activates Clang - because I want to land this very soon, to meet the 16.10 Preview 2 deadline, and I just finished validating this against the internal and public builds, so resetting testing would add additional headache and risk. Your suggested function sounds like a great idea and I'll try to remember it (since I expect to perform that toolset update). Note that consteval still isn't implemented in Clang 12 so it'll be a while.

Of the changes that I recently pushed, the notable ones are:

• When testing file_name ends_with, use only test.cpp or header.h, not the relative path with the test directory. This is because (AFAIK) the internal "Contest" test harness copies files to temporary directories before compiling them. As a result, LLVM-45731 is no longer marked as TRANSITION (they can continue to investigate it, of course).
• /analyze expects slightly different column numbers. This might be a bug - similarly for Standard Library Header Units returning column() == 1. For all column number behavior, bugs can be reported to the various compilers, but I don't think they should block this checkin.
• include_each_header_alone_matrix.lst is gaining an MSVC configuration. We initially tested only Clang because it provided a highly conformant mode with two-phase name lookup. Now that MSVC has /permissive- strict mode, and we are increasingly implementing features like <source_location> that are active for one compiler earlier than another, having coverage for both codegen compilers is increasingly important. Fortunately, this revealed no issues.
• We should use _NODISCARD_CTOR for constructors to be consistent, although I believe it makes no difference for C++20-only code.

I've mirrored this to internal MSVC-PR-307468 along with the necessary "hack these files" changes to add <source_location> to the installer and so forth.

Please note: Further changes are still possible (e.g. if you notice something, or if final review leads to more feedback that should be addressed before merging), but they must be replicated to the mirror PR in order to avoid codebase divergence.

[SuperWig]

Thanks for the super detailed answer as always, Stephan.

All changes requested in Apr 2020 have been made or resolved

[StephanTLavavej]

Thanks for implementing this feature far in advance of the compilers being ready! 😹 This will be available in VS 2019 16.10 Preview 2 along with MSVC's support for consteval. 🚀 😺 🗺️ 🎯

[robert-andrzejuk]

@StephanTLavavej [INDENTATION STYLE RANT] Theres some ugly indentation here. I will suggest a lecture by Kevlin Henney: ITT 2016 - Kevlin Henney - Seven Ineffective Coding Habits of Many Programmers https://www.youtube.com/watch?t=1263&v=ZsHMHukIlJY [/INDENTATION STYLE RANT]

The below formatting, I believe I get by using clang-format parameters (https://clang.llvm.org/docs/ClangFormatStyleOptions.html):
AllowAllConstructorInitializersOnNextLine: false
AllowAllParametersOfDeclarationOnNextLine: false
BinPackArguments: false
BinPackParameters: false
AlignConsecutiveAssignments: AcrossComments
AlignConsecutiveDeclarations: AcrossComments
ConstructorInitializerIndentWidth: 6
ContinuationIndentWidth: 6

static constexpr source_location current(uint_least32_t _InLine     = __builtin_LINE(),
                                         uint_least32_t _InColumn   = __builtin_COLUMN(),
                                         const char*    _InFile     = __builtin_FILE(),
                                         const char*    _InFunction = __builtin_FUNCTION()) noexcept
{
    source_location _Result;
    _Result._Line     = _InLine;
    _Result._Column   = _InColumn;
    _Result._File     = _InFile;
    _Result._Function = _InFunction;
    return _Result;
}

As style is a contentious issue, please feel free to ignore this rant.