[BUG] Poor debugging experience of generated code

[JohelEGP]

Title: Poor debugging experience of generated code.

Description:

A lot of Cpp2 can be generated, not visible in the Cpp2 source code itself.
For example, generated constructors and assignments,
as well as member functions generated by metafunctions.

When debugging the reproducer,
until you step into a standard function,
attempting to step into generated code

• keeps the cursor at the EOF, and
• you can see some movement in various panes.

This results in a poor user experience.
I think we can and should do better.

Minimal reproducer (https://cpp2.godbolt.org/z/P7ssbdoYe):

number: @union @print type = {
  i: int;
  d: double;
}

main: () = {
  n: number = ();
  n.set_i(42);
  if n.is_i() { n.set_d(17.29); }
  _ = n;
}

Commands:

cppfront main.cpp2
clang++18 -std=c++23 -stdlib=libc++ -lc++abi -pedantic-errors -Wall -Wextra -Wconversion -Werror=unused-result -I . main.cpp

Expected result:

• Stepping into generated code jumps to the generated code.
  • cppfront to output a Cpp2 source file with the generated code.
  • The lowered Cpp1 #line directives of generated code to point to the new file.

Actual result and error: Stepping into generated code jumps to the EOF.

Cpp2 lowered to Cpp1:

//=== Cpp2 type declarations ====================================================


#include "cpp2util.h"

class number;
  

//=== Cpp2 type definitions and function declarations ===========================

class number {
private: std::aligned_storage_t<cpp2::max(sizeof(int), sizeof(double))> _storage {}; private: cpp2::i8 _discriminator {-1}; public: [[nodiscard]] auto is_i() const& -> bool;
public: [[nodiscard]] auto i() const& -> int const&;
public: [[nodiscard]] auto i() & -> int&;
public: auto set_i(cpp2::in<int> _value) & -> void;
public: auto set_i(auto&& ..._args) & -> void;
public: [[nodiscard]] auto is_d() const& -> bool;
public: [[nodiscard]] auto d() const& -> double const&;
public: [[nodiscard]] auto d() & -> double&;
public: auto set_d(cpp2::in<double> _value) & -> void;
public: auto set_d(auto&& ..._args) & -> void;
private: auto _destroy() & -> void;
public: ~number() noexcept;
public: explicit number();
public: number(number const& that);

public: number(number&& that) noexcept;
public: auto operator=(number const& that) -> number& ;
public: auto operator=(number&& that) noexcept -> number& ;

};

auto main() -> int;

//=== Cpp2 function definitions =================================================



[[nodiscard]] auto number::is_i() const& -> bool { return _discriminator == 0; }
[[nodiscard]] auto number::i() const& -> int const& { 
                                           cpp2::Default.expects(is_i(), "");return *cpp2::assert_not_null(reinterpret_cast<int const*>(&_storage)); }
[[nodiscard]] auto number::i() & -> int& { 
                                                 cpp2::Default.expects(is_i(), "");return *cpp2::assert_not_null(reinterpret_cast<int*>(&_storage)); }
auto number::set_i(cpp2::in<int> _value) & -> void{if (!(is_i())) {_destroy();std::construct_at(reinterpret_cast<int*>(&_storage), _value);}else {*cpp2::assert_not_null(reinterpret_cast<int*>(&_storage)) = _value;}_discriminator = 0;}
auto number::set_i(auto&& ..._args) & -> void{if (!(is_i())) {_destroy();std::construct_at(reinterpret_cast<int*>(&_storage), _args...);}else {*cpp2::assert_not_null(reinterpret_cast<int*>(&_storage)) = int{_args...};}_discriminator = 0;}
[[nodiscard]] auto number::is_d() const& -> bool { return _discriminator == 1; }
[[nodiscard]] auto number::d() const& -> double const& { 
                                              cpp2::Default.expects(is_d(), "");return *cpp2::assert_not_null(reinterpret_cast<double const*>(&_storage)); }
[[nodiscard]] auto number::d() & -> double& { 
                                                    cpp2::Default.expects(is_d(), "");return *cpp2::assert_not_null(reinterpret_cast<double*>(&_storage)); }
auto number::set_d(cpp2::in<double> _value) & -> void{if (!(is_d())) {_destroy();std::construct_at(reinterpret_cast<double*>(&_storage), _value);}else {*cpp2::assert_not_null(reinterpret_cast<double*>(&_storage)) = _value;}_discriminator = 1;}
auto number::set_d(auto&& ..._args) & -> void{if (!(is_d())) {_destroy();std::construct_at(reinterpret_cast<double*>(&_storage), _args...);}else {*cpp2::assert_not_null(reinterpret_cast<double*>(&_storage)) = double{_args...};}_discriminator = 1;}
auto number::_destroy() & -> void{
if (_discriminator == 0) {std::destroy_at(reinterpret_cast<int*>(&_storage));}
if (_discriminator == 1) {std::destroy_at(reinterpret_cast<double*>(&_storage));}
_discriminator = -1;
}

number::~number() noexcept{_destroy();}
number::number(){}
number::number(number const& that)
        : _storage{  }
        , _discriminator{ -1 }{
if (CPP2_UFCS_0(is_i, that)) {set_i(CPP2_UFCS_0(i, that));}
if (CPP2_UFCS_0(is_d, that)) {set_d(CPP2_UFCS_0(d, that));}
}

number::number(number&& that) noexcept
        : _storage{  }
        , _discriminator{ -1 }{
if (CPP2_UFCS_0(is_i, std::move(that))) {set_i(CPP2_UFCS_0(i, std::move(that)));}
if (CPP2_UFCS_0(is_d, std::move(that))) {set_d(CPP2_UFCS_0(d, std::move(that)));}
}

auto number::operator=(number const& that) -> number& {
if (CPP2_UFCS_0(is_i, that)) {set_i(CPP2_UFCS_0(i, that));}
if (CPP2_UFCS_0(is_d, that)) {set_d(CPP2_UFCS_0(d, that));}
        return *this;
}

auto number::operator=(number&& that) noexcept -> number& {
if (CPP2_UFCS_0(is_i, std::move(that))) {set_i(CPP2_UFCS_0(i, std::move(that)));}
if (CPP2_UFCS_0(is_d, std::move(that))) {set_d(CPP2_UFCS_0(d, std::move(that)));}
        return *this;
}
auto main() -> int{
  number n {}; 
  CPP2_UFCS(set_i, n, 42);
  if (CPP2_UFCS_0(is_i, n)) {CPP2_UFCS(set_d, n, 17.29); }
  static_cast<void>(std::move(n));
}

Output:

Step build returned: 0
[1/5] Generating main.cpp
main.cpp2...

number: type = 
{
    _storage: std::aligned_storage_t<cpp2::max(sizeof(int), sizeof(double))> = ();

    _discriminator: i8 = -1;

    is_i:(in this) -> move bool = _discriminator == 0;

    i:(in this) -> forward int
        pre( is_i() ) = reinterpret_cast<* const int>(_storage&)*;

    i:(inout this) -> forward int
        pre( is_i() ) = reinterpret_cast<* int>(_storage&)*;

    set_i:(
        inout this, 
        in _value: int
    ) = 
    {
        if !is_i()
        {
            _destroy();
            std::construct_at(reinterpret_cast<* int>(_storage&), _value);
        }
        else 
        {
            reinterpret_cast<* int>(_storage&)* = _value;
        }
        _discriminator = 0;
    }

    set_i:(
        inout this, 
        forward _args...:
    ) = 
    {
        if !is_i()
        {
            _destroy();
            std::construct_at(reinterpret_cast<* int>(_storage&), _args...);
        }
        else 
        {
            reinterpret_cast<* int>(_storage&)* = : int = (_args...);
        }
        _discriminator = 0;
    }

    is_d:(in this) -> move bool = _discriminator == 1;

    d:(in this) -> forward double
        pre( is_d() ) = reinterpret_cast<* const double>(_storage&)*;

    d:(inout this) -> forward double
        pre( is_d() ) = reinterpret_cast<* double>(_storage&)*;

    set_d:(
        inout this, 
        in _value: double
    ) = 
    {
        if !is_d()
        {
            _destroy();
            std::construct_at(reinterpret_cast<* double>(_storage&), _value);
        }
        else 
        {
            reinterpret_cast<* double>(_storage&)* = _value;
        }
        _discriminator = 1;
    }

    set_d:(
        inout this, 
        forward _args...:
    ) = 
    {
        if !is_d()
        {
            _destroy();
            std::construct_at(reinterpret_cast<* double>(_storage&), _args...);
        }
        else 
        {
            reinterpret_cast<* double>(_storage&)* = : double = (_args...);
        }
        _discriminator = 1;
    }

    private _destroy:(inout this) = 
    {
        if _discriminator == 0
        {
            std::destroy_at(reinterpret_cast<* int>(_storage&));
        }
        if _discriminator == 1
        {
            std::destroy_at(reinterpret_cast<* double>(_storage&));
        }
        _discriminator = -1;
    }

    operator=:(move this) = 
    {
        _destroy();
    }

    operator=:(out this) = 
    {
    }

    operator=:(
        out this, 
        in that
    ) = 
    {
        _storage = ();
        _discriminator = -1;
        if that.is_i()
        {
            set_i(that.i());
        }
        if that.is_d()
        {
            set_d(that.d());
        }
    }

    operator=:(
        inout this, 
        in that
    ) = 
    {
        _storage = _;
        _discriminator = _;
        if that.is_i()
        {
            set_i(that.i());
        }
        if that.is_d()
        {
            set_d(that.d());
        }
    }
}
 ok (all Cpp2, passes safety checks)

[2/5] Scanning /app/build/main.cpp for CXX dependencies
[3/5] Generating CXX dyndep file CMakeFiles/main.dir/CXX.dd
[4/5] Building CXX object CMakeFiles/main.dir/main.cpp.o
main.cpp2:2:83: warning: 'using std::aligned_storage_t = union std::aligned_storage<8, 16>::type' is deprecated [-Wdeprecated-declarations]
In file included from /opt/compiler-explorer/gcc-trunk-20231120/include/c++/14.0.0/bits/stl_pair.h:60,
                 from /opt/compiler-explorer/gcc-trunk-20231120/include/c++/14.0.0/bits/stl_algobase.h:64,
                 from /opt/compiler-explorer/gcc-trunk-20231120/include/c++/14.0.0/algorithm:60,
                 from /app/raw.githubusercontent.com/hsutter/cppfront/main/include/cpp2util.h:232,
                 from /app/cpp2util.h:1,
                 from /app/build/main.cpp:6:
/opt/compiler-explorer/gcc-trunk-20231120/include/c++/14.0.0/type_traits:2611:11: note: declared here
 2611 |     using aligned_storage_t _GLIBCXX23_DEPRECATED = typename aligned_storage<_Len, _Align>::type;
      |           ^~~~~~~~~~~~~~~~~
[5/5] Linking CXX executable main
Program returned: 0

[gregmarr]

I've noticed that the Compiler Explorer output from cppfront doesn't contain the #line directives that cppfront puts out. I think it must be hardcoding the clean-cpp1 flag somewhere.

[JohelEGP]

Clean Cpp1 files don't have the phase comments, like //=== Cpp2 type declarations ==.
It does feel weird when there are no #line directives, though.

[gregmarr]

Then my other assumption is that it is stripping them out from the output itself. Is the experience any better with the #line directives there?

[JohelEGP]

I don't think there's #line directives for generated code.

[gregmarr]

There definitely is for generated constructors and assignment operators.

[JohelEGP]

Probably because that's emitted during lowering.

[gregmarr]

Yes, Compiler Explorer is explicitly stripping them.. https://github.com/compiler-explorer/compiler-explorer/blob/main/lib/parsers/asm-parser-cpp.ts#L48 But it's an option you can turn off from the Filters menu.

There are line numbers added for constructors and assignment operators, but only if they're written in the original source, not if they're added by a metafunction.

[JohelEGP]

I wasn't aware of that, thank you.
Since that only affects the pane output, and not the CMake project, I never noticed.

[gregmarr]

So I would say that in general your report is accurate for metafunction generated code. The "keeps the cursor at the EOF" depends on the length of your file. It would be more accurate to say that it's on an unrelated line, which is often the last line of the file when the file contains a single small class that has a lot of added code.

I wonder if this is something that metafunctions will be able to do once they have the ability to write to a file for generating other languages. Probably the hardest part would be tracking which line in the file backs each bit of AST. I guess that if mycode.cpp2 generates mycode.cpp it could also generate mycode-generated.cpp2 in the same directory.

[JohelEGP]

#478 fixes #422 by generating a default constructor.
to_cpp1.h takes care of lowering a default constructor
that both uses the NSDMIs and #lines to their .cpp2 source.

operator=s could also be generated.
to_cpp1.h, on top of lowering member initializer lists, simulates the generation during lowering.
That has resulted in bugs because some logic isn't repeated for the member-wise assignments.
For example, #487 didn't work in the member-wise assignments, so #680 had to add a special case for it.
Another example might be #822.
If the operator=s were generated:

• Those bugs would be a thing of the past.
• Debugging a .cpp1 function generated from a Cpp2 operator= would #line to the .cpp2 source.
  Or if not, there would be a bug in the generation of #lines.
  But it's definitely not some logic that would have to also be
  necessarily repeated or kept up to date in the lowering of operator=s.

[JohelEGP]

An alternative expected result is for generated code to disable the effect of #line.
That means that source locations in generated code (e.g., in error messages),
as well as stepping through generated code while debugging,
would go through the lowered Cpp1 instead since there's no equivalent Cpp2 source code.

[JohelEGP]

IIRC, -clean-cpp1 does that, but for all code, as the Cpp1 source isn't mapped back to Cpp2.

[DyXel]

IIRC, -clean-cpp1 does that, but for all code, as the Cpp1 source isn't mapped back to Cpp2.

Yes. I have also tinkered by passing it through clang-format as well, since code comes all packed or scrambled (specially confusing with metafunctions), I also do that mostly because tabs are not printed back as tabs, but as a single space, which misaligns everything.