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

__msvc_chrono.hpp & type_traits: chrono duration #3992

Closed
larioteo opened this issue Aug 24, 2023 · 7 comments
Closed

__msvc_chrono.hpp & type_traits: chrono duration #3992

larioteo opened this issue Aug 24, 2023 · 7 comments
Labels
chrono C++20 chrono info needed We need more info before working on this modules C++23 modules, C++20 header units

Comments

@larioteo
Copy link

larioteo commented Aug 24, 2023

Describe the bug

It's hard to describe this bug, it seems to depend on a lot of things. I cannot build you guys the perfect test case, sorry, I only try to report something, that had cost me some hours and nerves..., but anyway you can download the whole code and test with it.

I am exclusively using C++20 modules, with /std:c++latest and I tested it on Visual Studio 2022 17.7.2 and 17.8.0 Preview 1.0, but the error occurred since 17.5.x as I remember.

This code is marked as the problem:

Note: Full Source: https://github.com/OmniVortexStudios/ultra/blob/master/Source/Library/Ultra/Utility/DateTime.ixx

    inline string GetTimeStamp() const { return GetTicks(); }

    inline string GetTicks (const string_view &format = "{:%Y-%m-%dT%H:%M:%S}") const {
        auto args = std::make_format_args(SystemClock::now());
        try {
            return std::vformat(format, args);
        } catch (const std::exception &ex) {
            return ex.what();
        }
    }

It is called over:

Note: Full Source: https://github.com/OmniVortexStudios/ultra/blob/master/Source/Library/Ultra/Core/Logger.ixx

struct LogRecord {
    LogRecord(const char *format, const LogLevel &level = LogLevel::Default, const string &timestamp = apptime.GetTimeStamp(), const SourceLocation &location = SourceLocation::Current()):
        Format(format),
        Level(level),
        Location(location),
        Timestamp(timestamp) {
    }

    const char *Format;
    mutable LogLevel Level;
    SourceLocation Location;
    string Timestamp;
};

In some, not all modules, the compile time error was triggered:

Note:

when using my logger...

LogError("Error"); // Dosn't realy matter what of it, as the timestamp is created over the function call

The issue is simple for you guys, I think, I get the following errors:

/// type_traits(1344,53): error C2794: 'type': is not a member of any direct or indirect base class of 'std::common_type<_Rep1,_Rep2>'
///     with
///     [
///         _Rep1 = __int64,
///         _Rep2 = __int64
///     ]
/// __msvc_chrono.hpp(268,35): error C2938: 'std::common_type_t' : Failed to specialize alias template
/// __msvc_chrono.hpp(268,56): error C2752: 'std::common_type<_Rep1,_Rep2>': more than one partial specialization matches the template argument list
///     with
///     [
///         _Rep1 = __int64,
///         _Rep2 = __int64
///     ]
/// __msvc_chrono.hpp(113,54): error C2955: 'std::chrono::duration': use of class template requires template argument list
/// __msvc_chrono.hpp(118,54): error C2955: 'std::chrono::duration': use of class template requires template argument list
///

Workaround

I found out that a simple #include <chrono> in the affected modules fixed the error, so I think it has something to do with Microsoft's STL implementation in combination with C++20 Modules.

I couldn't track the issue further, sorry, it is far beyond my knowledge.

Expected behavior

std::chrono::time_pointstd::chrono::system_clock;
--> Should work across module boundaries

Hopefully that makes sense...

STL version

  • Option 1: Visual Studio version
    Microsoft Visual Studio Community 2022 Version 17.7.2
    Microsoft Visual Studio Community 2022 Version 17.8.0 Preview 1.0
    
  • Option 2: git commit hash
    • run prepare.bat -silent
    • set Test as start project
    • uncomment the #include <chrono> in one of the affected modules (also visible in the commit log)
      https://github.com/OmniVortexStudios/ultra/commit/e3b0358b785efbb46cdd5ab56a42658be2362f3c
      

Additional context

You can find the affected modules with my marked comment:

/// @brief Hack: This is an nasty fix for Microsoft's STL implementation

Sorry, if i couldn't help you further.

@StephanTLavavej StephanTLavavej added info needed We need more info before working on this chrono C++20 chrono modules C++23 modules, C++20 header units labels Sep 6, 2023
@StephanTLavavej
Copy link
Member

This might be caused by internal VSO-1880921.

@larioteo
Copy link
Author

larioteo commented Oct 8, 2023

The issue must be somewhere in the library itself.

Today, I realized that chrono also breaks IntelliSense syntax highlighting in Visual Studio... sometimes it also breaks other things as described above. As I mentioned, my skills are not sufficient to track down the issue.

It seems to only happen if you import chrono (not include) and use something from it, which doesn't make sense at all.

As an example here: (This module can be used standalone) https://github.com/OmniVortexStudios/ultra/blob/28eb085f7c9675b2c86e5577268c1c509e3e90bd/Source/Library/Ultra/Utility/Timer.ixx

If I import chrono, the syntax highlighting is gone; if I include chrono, then everything is fine. As I recalled, also above, the solution was to include explicitly to fix the template-related bugs.

I hope that this can help you guys to track down this annoying issue...

@StephanTLavavej
Copy link
Member

IntelliSense support for modules is a work in progress, and we have not yet enabled STL test coverage. At this time, you should not expect IntelliSense to work at all for modules - then you may be positively surprised if some things happen to work, but you'll never be disappointed. (Not a joke.)

@larioteo
Copy link
Author

larioteo commented Oct 9, 2023

Yeah, I am aware of that and you're completely right.

Anyways this weird issue above (chrono) happens mixing modules with classic headers, and the solution in such case is to sometimes include chrono or to import it as header unit. It depends on something I don't understand. But the error is always the same, the compiler cannot deduce the template parameters or it kills IntelliSense.

The crazy thing about it I get sometimes also compiler errors, the internal ones, which than breaks completely my builds.

After removing chrono nearly everywhere the most issues are gone and also IntelliSense started working. I also removed global module fragments where possible.

Thats why I am suspecting chrono to be the real culprit in both cases. Maybe it can help you guys to track down the issue.

Also I tracked down that usings of namespaces can brake IntelliSense, but thats a known issue which is already mentioned in the vs developer community.

@StephanTLavavej
Copy link
Member

Mixing modules with classic includes is a known bug/limitation that the compiler team is working on. This should also not be expected to work at all at this time. (It's super duper broken, whereas IntelliSense support for modules is only mostly broken.)

If you're seeing ICEs involving chrono when using only the named module std (or only <chrono> as a header unit), without mixing classic includes, and these are ICEs with the cl.exe codegen compiler (not IntelliSense), please report them on VS Developer Community with self-contained test cases and let me know the bug numbers so I can track them in #1694.

ICEs are by definition compiler bugs; chrono cannot be the root cause, although it can be a contributing factor.

@StephanTLavavej
Copy link
Member

Does this still repro with VS 2022 17.10 Preview 7 or later? I improved the STL so that include-before-import (but not the other order) works, with #4154 in VS 2022 17.10 Preview 1.

@larioteo
Copy link
Author

Yeah, it's solved,

Thank You.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chrono C++20 chrono info needed We need more info before working on this modules C++23 modules, C++20 header units
Projects
None yet
Development

No branches or pull requests

2 participants