Remove this-> from decltype

[PeterJohnson]

The latest version of MSVC doesn't like the use of this-> in decltype, and removing it from the decltype doesn't seem to harm anything. It's unclear whether this is a bug in MSVC, or a standards-compliant change, given that other compilers don't reject either variant. It feels like this might be a bug in the compiler, as the error messages make it look like MSVC is trying to look up the function in the class where it's being used, rather than the class where it's being defined (e.g. the this pointer maybe isn't properly tracked/associated)?

An identical patch (wpilibsuite/allwpilib#5948) fixes the build failures we are seeing with current HEAD.

Rough code (full code is https://github.com/calcmogul/allwpilib/blob/6e23e70a8c48266ca25fbb818ce8357dce80ac32/wpilibc/src/main/native/cpp/Preferences.cpp):

std::string_view GetPath();
namespace { struct Instance {
std::string path = fmt::format("{}/", GetPath());
}; }

On MSVC 19.38.33129 or similar (e.g. using the latest GitHub Windows-2022 image), this errors with the following (full build log at https://github.com/wpilibsuite/allwpilib/actions/runs/6937548373/job/18871818708?pr=5945):

C:\work\wpiutil\src\main\native\thirdparty\fmtlib\include\fmt\core.h(1438): error C2039: 'map': is not a member of '`anonymous-namespace'::Instance'
C:\work\wpilibc\src\main\native\cpp\Preferences.cpp(23): note: see declaration of '`anonymous-namespace'::Instance'
C:\work\wpiutil\src\main\native\thirdparty\fmtlib\include\fmt\core.h(1438): note: the template instantiation context (the oldest one first) is
C:\work\wpilibc\src\main\native\cpp\Preferences.cpp(33): note: see reference to function template instantiation 'fmt::v10::basic_format_string<char,std::basic_string_view<char,std::char_traits<char>>>::basic_format_string<char[4],0>(const S (&))' being compiled
        with
        [
            S=char [4]
        ]
C:\work\wpiutil\src\main\native\thirdparty\fmtlib\include\fmt\core.h(2740): note: see reference to class template instantiation 'fmt::v10::detail::format_string_checker<Char,std::basic_string_view<char,std::char_traits<char>>>' being compiled
        with
        [
            Char=char
        ]
C:\work\wpiutil\src\main\native\thirdparty\fmtlib\include\fmt\core.h(2608): note: while compiling class template member function 'fmt::v10::detail::format_string_checker<Char,std::basic_string_view<char,std::char_traits<char>>>::format_string_checker(fmt::v10::basic_string_view<Char>)'
        with
        [
            Char=char
        ]
C:\work\wpiutil\src\main\native\thirdparty\fmtlib\include\fmt\core.h(2609): note: see reference to alias template instantiation 'fmt::v10::detail::mapped_type_constant<std::basic_string_view<char,std::char_traits<char>>,fmt::v10::basic_format_context<fmt::v10::appender,char>>' being compiled
C:\work\wpiutil\src\main\native\thirdparty\fmtlib\include\fmt\core.h(1478): note: see reference to class template instantiation 'fmt::v10::detail::arg_mapper<fmt::v10::basic_format_context<fmt::v10::appender,char>>' being compiled
C:\work\wpiutil\src\main\native\thirdparty\fmtlib\include\fmt\core.h(1462): error C2039: 'do_map': is not a member of '`anonymous-namespace'::Instance'
C:\work\wpilibc\src\main\native\cpp\Preferences.cpp(23): note: see declaration of '`anonymous-namespace'::Instance'
C:\work\wpiutil\src\main\native\thirdparty\fmtlib\include\fmt\core.h(1468): error C2039: 'map': is not a member of '`anonymous-namespace'::Instance'
C:\work\wpilibc\src\main\native\cpp\Preferences.cpp(23): note: see declaration of '`anonymous-namespace'::Instance'
C:\work\wpiutil\src\main\native\thirdparty\fmtlib\include\fmt\core.h(2609): error C2062: type 'unknown-type' unexpected

[PeterJohnson]

Interesting... it fails with g++ 4.8, but appears to work with everything newer (although the builds were canceled so it's hard to tell for sure).

[vitaut]

Looks like a compiler bug, please report to Microsoft.

Closing as earlier version of gcc require this->.

[PeterJohnson]

I’ll report the bug, but since it’s in a released version of the compiler that’s active in the Windows-2022 GitHub image, I wouldn’t be surprised if other people started running into this with vcpkg in the near term, so it might be worth an ifdef’ed version of this as a workaround if other people start reporting issues.

[vitaut]

It's unfortunate but we cannot break gcc and this particular case seems to only affect default member initializers in MSVC which can be worked around. Conditionally disabling this will likely cause more problems.

[PeterJohnson]

Given it works without this-> in all more recent compilers (MSVC, newer GCC, and clang), and the body of the function doesn't use this->, isn't the current use of this-> in the decltype actually a workaround for a bug in GCC 4.8?

[vitaut]

Possibly but more likely it's a newer feature (either in the standard or in the implementation). The core of {fmt} only requires a subset of C++11 supported by older compilers including gcc 4.x.

[phprus]

@vitaut

GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57543
Fixed in GCC 5.

I think we can add macro:

if GCC < 5
#define FMT_DETAIL_DECLTYPE_THIS  this->
else
#define FMT_DETAIL_DECLTYPE_THIS
endif

and use it in decltype.

[vitaut]

That'll work (although I'd drop "DETAIL_" from the macro name). Not sure why I thought ifdef would be a problem =).

[phprus]

@vitaut
Godbolt: https://godbolt.org/z/M1P8Yessq

[PeterJohnson]

I pushed the ifdef version to my branch for this PR, https://github.com/PeterJohnson/fmt/tree/remove-decltype-this (I guess this PR won't actually pick it up unless it's reopened?)

[vitaut]

Reopened

[vitaut]

Merged, thanks.

[PeterJohnson]

Thanks! For completeness, I've reported this bug to Microsoft here: https://developercommunity.visualstudio.com/t/decltypethis-member-fails-with-C203/10523643