taocpp / PEGTL

Parsing Expression Grammar Template Library
Boost Software License 1.0
1.95k stars 228 forks source link

line_at not finding line ending when inside of trace.hpp #285

Closed mgood7123 closed 2 years ago

mgood7123 commented 2 years ago

im using the following

pegtl::memory_input in(spec, "Specification test");

but in.line_at( p ) does not seem to correctly acquire the line

#5        tao::pegtl::ascii::any]
          success
          position Specification test:2:4
          source VERTEX_CHAIN {
    vec4 position;
    vec4 textureCoordinates;
};

FRAGMENT_CHAIN {
    vec4 color;
}

even if i change the Eol from eol::cr_crlf to eol::lf for all input classes it does not make a difference

mgood7123 commented 2 years ago

using this approach seems to work (can be adapted to replace in with this)

https://github.com/taocpp/PEGTL/issues/286 wont compile so i had to do manually

template <typename ParseInput>
[[nodiscard]] std::string line_at( const ParseInput& in, const pegtl::position& p ) const noexcept
{
    // we need to allocate temporary storage to hold the line
    // this also serves to automatically null-terminate the line
    // we do not want to null-terminate our input
    std::string storage = "";
    // get beginning of line, ignore column
    const char * str = in.begin_of_line( p );
    const char * end = in.end();
    char s[2];
    s[1] = '\0';
    while (str != end) {
        s[0] = *str;
        // lf and crlf both end in '\n' so there is no need to look for '\r'
        // do not add \n to line
        if (s[0] == '\n') break;
        storage += s;
        str++;
    }
    return storage;
}
template <typename ParseInput>
void print_position(const ParseInput& in) const
{
    std::cerr << std::setw( indent() ) << ' ' 
        << TracerTraits::ansi_position << "position" 
        << TracerTraits::ansi_reset << ' ' 
        << m_position << '\n';

    std::cerr << std::setw( indent() ) << ' ' 
        << TracerTraits::ansi_position << "source" 
        << TracerTraits::ansi_reset << ' ' 
        << PSL_line_at(in, m_position) << '\n';

    std::cerr << std::setw( indent() + 6 + m_position.column ) 
        << ' ' << '^' << '\n';
}
#11       tao::pegtl::ascii::any]
          success
          position Specification test:2:10
          source VERTEX_CHAIN {
                          ^
#12       tao::pegtl::ascii::any]
          success
          position Specification test:2:11
          source VERTEX_CHAIN {
                           ^
#13       tao::pegtl::ascii::any]
          success
          position Specification test:2:12
          source VERTEX_CHAIN {
                            ^
#14       tao::pegtl::ascii::any]
          success
          position Specification test:2:13
          source VERTEX_CHAIN {
                             ^
#15       tao::pegtl::ascii::any]
          success
          position Specification test:2:14
          source VERTEX_CHAIN {
                              ^
#16       tao::pegtl::ascii::any]
          success
          position Specification test:2:15
          source VERTEX_CHAIN {
                               ^
#17       tao::pegtl::ascii::any]
          success
          position Specification test:3:1
          source     vec4 position;
                 ^
#18       tao::pegtl::ascii::any]
          success
          position Specification test:3:2
          source     vec4 position;
                  ^
d-frey commented 2 years ago

Can you post the (stripped down but) complete example that didn't work? (The one from your first message)

line_at should work, if it doesn't and there is an actual bug, I'd like to find and fix it. Note that it returns a std::string_view, which is non-owning.

mgood7123 commented 2 years ago

Can you post the (stripped down but) complete example that didn't work? (The one from your first message)

line_at should work, if it doesn't and there is an actual bug, I'd like to find and fix it. Note that it returns a std::string_view, which is non-owning.

it may be related to the tracking mode, as it works as expected in raised exceptions

tho the default is Eager

and adding the following in print_position in trace.hpp is sufficent to reproduce (in.line_at(p) works as expected when outside of *_trace in the try catch)

std::cerr << std::setw( indent() ) << ' ' << TracerTraits::ansi_position << "source" << TracerTraits::ansi_reset << ' ' << in.line_at(m_position) << '\n';

mgood7123 commented 2 years ago

i guess this is just an edge case of sorts

d-frey commented 2 years ago

I think I now understand the problem. It's begin_of_line's fault, as it uses p.column, which is not properly set in lazy mode. I'll investigate further...

d-frey commented 2 years ago

Wait a second... you used:

std::cerr << std::setw( indent() ) << ' ' << TracerTraits::ansi_position << "source" << TracerTraits::ansi_reset << ' ' << in.line_at(m_position) << '\n';

Does that mean you used m_position before update_position() was called? (BTW: Using lazy tracking also means in.position() becomes an expensive operation, hence using lazy mode combined with the tracer make almost no sense as the combination is slower than using eager tracking mode)

mgood7123 commented 2 years ago

Wait a second... you used:

std::cerr << std::setw( indent() ) << ' ' << TracerTraits::ansi_position << "source" << TracerTraits::ansi_reset << ' ' << in.line_at(m_position) << '\n';

Does that mean you used m_position before update_position() was called? (BTW: Using lazy tracking also means in.position() becomes an expensive operation, hence using lazy mode combined with the tracer make almost no sense as the combination is slower than using eager tracking mode)

i dont think so, im using that inside of print_position() which i believe is called from update_position()

d-frey commented 2 years ago

OK. Now, I would really need you to create a stripped down but complete example that shows your problem. I can't really guess what is going on. Something I can copy on my system, compile and see the problem myself.

mgood7123 commented 2 years ago

Alright, I will do that later, its 4 am for me

On Sun., 5 Dec. 2021, 3:41 am Daniel Frey, @.***> wrote:

OK. Now, I would really need you to create a stripped down but complete example that shows your problem. I can't really guess what is going on. Something I can copy on my system, compile and see the problem myself.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/taocpp/PEGTL/issues/285#issuecomment-986064798, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGLITH6WFUCUCQMTTSWXUF3UPJHEZANCNFSM5JJ5C6LA .

mgood7123 commented 2 years ago

see if this will work?

// Copyright (c) 2020-2021 Dr. Colin Hirsch and Daniel Frey
// Distributed under the Boost Software License, Version 1.0.
// (See accompanying file LICENSE_1_0.txt or copy at https://www.boost.org/LICENSE_1_0.txt)

#ifndef ASTRID_PSL_TRACE_H
#define ASTRID_PSL_TRACE_H

#include <cstddef>
#include <iomanip>
#include <iostream>
#include <string_view>
#include <tuple>

#include <tao/pegtl/position.hpp>

#include <tao/pegtl/contrib/state_control.hpp>

#include <tao/pegtl/apply_mode.hpp>
#include <tao/pegtl/config.hpp>
#include <tao/pegtl/demangle.hpp>
#include <tao/pegtl/normal.hpp>
#include <tao/pegtl/nothing.hpp>
#include <tao/pegtl/parse.hpp>
#include <tao/pegtl/rewind_mode.hpp>
#include <tao/pegtl/eol.hpp>

using namespace tao;

namespace PSL_TRACER
{
    template< bool HideInternal = false, bool UseColor = true, std::size_t IndentIncrement = 2, std::size_t InitialIndent = 8 >
    struct tracer_traits
    {
        template< typename Rule >
        static constexpr bool enable = ( HideInternal ? pegtl::normal< Rule >::enable : true );

        static constexpr std::size_t initial_indent = InitialIndent;
        static constexpr std::size_t indent_increment = IndentIncrement;

        static constexpr std::string_view ansi_reset = UseColor ? "\033[m" : "";
        static constexpr std::string_view ansi_rule = UseColor ? "\033[36m" : "";
        static constexpr std::string_view ansi_hide = UseColor ? "\033[37m" : "";

        static constexpr std::string_view ansi_position = UseColor ? "\033[1;34m" : "";
        static constexpr std::string_view ansi_success = UseColor ? "\033[32m" : "";
        static constexpr std::string_view ansi_failure = UseColor ? "\033[31m" : "";
        static constexpr std::string_view ansi_raise = UseColor ? "\033[1;31m" : "";
        static constexpr std::string_view ansi_unwind = UseColor ? "\033[31m" : "";
        static constexpr std::string_view ansi_apply = UseColor ? "\033[1;36m" : "";
    };

    using standard_tracer_traits = tracer_traits< true >;
    using complete_tracer_traits = tracer_traits< false >;

    template< typename TracerTraits >
    struct tracer
    {
        const std::ios_base::fmtflags m_flags;
        std::size_t m_count = 0;
        std::vector< std::size_t > m_stack;
        pegtl::position m_position;

        template< typename Rule >
        static constexpr bool enable = TracerTraits::template enable< Rule >;

        template< typename ParseInput >
        explicit tracer( const ParseInput& in )
                : m_flags( std::cerr.flags() ),
                  m_position( in.position() )
        {
            std::cerr << std::left;
            print_position(in);
        }

        tracer( const tracer& ) = delete;
        tracer( tracer&& ) = delete;

        ~tracer()
        {
            std::cerr.flags( m_flags );
        }

        tracer& operator=( const tracer& ) = delete;
        tracer& operator=( tracer&& ) = delete;

        [[nodiscard]] std::size_t indent() const noexcept
        {
            return TracerTraits::initial_indent + TracerTraits::indent_increment * m_stack.size();
        }

        template <typename ParseInput>
        void print_position(const ParseInput& in) const
        {
            std::cerr << std::setw( indent() ) << ' ' << TracerTraits::ansi_position << "position" << TracerTraits::ansi_reset << ' ' << m_position << '\n';
            std::cerr << std::setw( indent() ) << ' ' << TracerTraits::ansi_position << "source" << TracerTraits::ansi_reset << ' ' << in.line_at(m_position) << '\n';
            std::cerr << std::setw( indent() + 6 + m_position.column ) << ' ' << '^' << '\n';
        }

        template <typename ParseInput>
        void update_position( const ParseInput& in )
        {
            const pegtl::position& p = in.position();
            if( m_position != p ) {
                m_position = p;
                print_position(in);
            }
        }

        template< typename Rule, typename ParseInput, typename... States >
        void start( const ParseInput& /*unused*/, States&&... /*unused*/ )
        {
            std::cerr << '#' << std::setw( indent() - 1 ) << ++m_count << TracerTraits::ansi_rule << pegtl::demangle< Rule >() << TracerTraits::ansi_reset << '\n';
            m_stack.push_back( m_count );
        }

        template< typename Rule, typename ParseInput, typename... States >
        void success( const ParseInput& in, States&&... /*unused*/ )
        {
            const auto prev = m_stack.back();
            m_stack.pop_back();
            std::cerr << std::setw( indent() ) << ' ' << TracerTraits::ansi_success << "success" << TracerTraits::ansi_reset;
            if( m_count != prev ) {
                std::cerr << " #" << prev << ' ' << TracerTraits::ansi_hide << pegtl::demangle< Rule >() << TracerTraits::ansi_reset;
            }
            std::cerr << '\n';
            update_position( in );
        }

        template< typename Rule, typename ParseInput, typename... States >
        void failure( const ParseInput& in, States&&... /*unused*/ )
        {
            const auto prev = m_stack.back();
            m_stack.pop_back();
            std::cerr << std::setw( indent() ) << ' ' << TracerTraits::ansi_failure << "failure" << TracerTraits::ansi_reset;
            if( m_count != prev ) {
                std::cerr << " #" << prev << ' ' << TracerTraits::ansi_hide << pegtl::demangle< Rule >() << TracerTraits::ansi_reset;
            }
            std::cerr << '\n';
            update_position( in );
        }

        template< typename Rule, typename ParseInput, typename... States >
        void raise( const ParseInput& /*unused*/, States&&... /*unused*/ )
        {
            std::cerr << std::setw( indent() ) << ' ' << TracerTraits::ansi_raise << "raise" << TracerTraits::ansi_reset << ' ' << TracerTraits::ansi_rule << pegtl::demangle< Rule >() << TracerTraits::ansi_reset << '\n';
        }

        template< typename Rule, typename ParseInput, typename... States >
        void unwind( const ParseInput& in, States&&... /*unused*/ )
        {
            const auto prev = m_stack.back();
            m_stack.pop_back();
            std::cerr << std::setw( indent() ) << ' ' << TracerTraits::ansi_unwind << "unwind" << TracerTraits::ansi_reset;
            if( m_count != prev ) {
                std::cerr << " #" << prev << ' ' << TracerTraits::ansi_hide << pegtl::demangle< Rule >() << TracerTraits::ansi_reset;
            }
            std::cerr << '\n';
            update_position( in );
        }

        template< typename Rule, typename ParseInput, typename... States >
        void apply( const ParseInput& /*unused*/, States&&... /*unused*/ )
        {
            std::cerr << std::setw( static_cast< int >( indent() - TracerTraits::indent_increment ) ) << ' ' << TracerTraits::ansi_apply << "apply" << TracerTraits::ansi_reset << '\n';
        }

        template< typename Rule, typename ParseInput, typename... States >
        void apply0( const ParseInput& /*unused*/, States&&... /*unused*/ )
        {
            std::cerr << std::setw( static_cast< int >( indent() - TracerTraits::indent_increment ) ) << ' ' << TracerTraits::ansi_apply << "apply0" << TracerTraits::ansi_reset << '\n';
        }

        template< typename Rule,
                template< typename... > class Action = pegtl::nothing,
                template< typename... > class Control = pegtl::normal,
                typename ParseInput,
                typename... States >
        bool parse( ParseInput&& in, States&&... st )
        {
            return TAO_PEGTL_NAMESPACE::parse< Rule, Action, pegtl::state_control< Control >::template type >( in, st..., *this );
        }
    };

    template< typename Rule,
            template< typename... > class Action = pegtl::nothing,
            template< typename... > class Control = pegtl::normal,
            typename ParseInput,
            typename... States >
    bool standard_trace( ParseInput&& in, States&&... st )
    {
        tracer< standard_tracer_traits > tr( in );
        return tr.parse< Rule, Action, Control >( in, st... );
    }

    template< typename Rule,
            template< typename... > class Action = pegtl::nothing,
            template< typename... > class Control = pegtl::normal,
            typename ParseInput,
            typename... States >
    bool complete_trace( ParseInput&& in, States&&... st )
    {
        tracer< complete_tracer_traits > tr( in );
        return tr.parse< Rule, Action, Control >( in, st... );
    }

    template< typename Tracer >
    struct trace
            : pegtl::maybe_nothing
    {
        template< typename Rule,
                pegtl::apply_mode A,
                pegtl::rewind_mode M,
                template< typename... >
                class Action,
                template< typename... >
                class Control,
                typename ParseInput,
                typename... States >
        [[nodiscard]] static bool match( ParseInput& in, States&&... st )
        {
            if constexpr( sizeof...( st ) == 0 ) {
                return TAO_PEGTL_NAMESPACE::match< Rule, A, M, Action, pegtl::state_control< Control >::template type >( in, st..., Tracer( in ) );
            }
            else if constexpr( !std::is_same_v< std::tuple_element_t< sizeof...( st ) - 1, std::tuple< States... > >, Tracer& > ) {
                return TAO_PEGTL_NAMESPACE::match< Rule, A, M, Action, pegtl::state_control< Control >::template type >( in, st..., Tracer( in ) );
            }
            else {
                return TAO_PEGTL_NAMESPACE::match< Rule, A, M, Action, Control >( in, st... );
            }
        }
    };

    using trace_standard = trace< tracer< standard_tracer_traits > >;
    using trace_complete = trace< tracer< complete_tracer_traits > >;

}  // namespace TAO_PEGTL_NAMESPACE

#endif //ASTRID_PSL_TRACE_H
const char * spec = R"(
VERTEX_CHAIN {
    vec4 position;
    vec4 textureCoordinates;
};

FRAGMENT_CHAIN {
    vec4 color;
}
)";

#include "PSL_TRACE.h"

using namespace tao;

#define PSL_STRING(string) TAO_PEGTL_STRING(string)

class SpecParser {
public:

    struct skip_whitespace : pegtl::plus<pegtl::sor<pegtl::plus<pegtl::space>, pegtl::plus<pegtl::eol>>> {};
    struct skip_to_end_of_file : pegtl::seq<pegtl::plus<pegtl::any>, pegtl::eof> {};

    struct vc : PSL_STRING("VERTEX_CHAIN") {};

    struct grammar : pegtl::seq<skip_whitespace, vc, skip_to_end_of_file> {};

    template< typename Rule >
    struct action
    {};

    template<>
    struct action< vc >
    {
        template< typename ActionInput >
        static void apply( const ActionInput& in, std::string& v )
        {
            puts("called apply");
        }
    };

    void test() {
        pegtl::memory_input in(spec, "Specification test");
        bool s = false;

        try {
            s = PSL_TRACER::complete_trace<grammar>(in);
//                s = pegtl::parse<grammar>(in);
        } catch( const pegtl::parse_error& e ) {
            try {
                s = PSL_TRACER::complete_trace<grammar>(in);
            } catch( const pegtl::parse_error& e ) {
            }
        }
        std::cerr << "spec passed (complete trace): " << s << std::endl;
    }
};

void test() {
    SpecParser specParser;
    specParser.test();
}
mgood7123 commented 2 years ago

this should reproduce the problem in the current main/master branch

#60           tao::pegtl::ascii::any]
              success
              position Specification test:4:25
              source     vec4 textureCoordinates;
};

FRAGMENT_CHAIN {
    vec4 color;
}

                                             ^
d-frey commented 2 years ago

You example doesn't even compile, it is also incomplete as there is no main(). After trying to fix the obvious problems and removing unused stuff, I can't reproduce the problem, everything works fine for me. I put it into compiler explorer, so you can see for yourself: https://godbolt.org/z/TsedKM3r6

Can you change the compiler explorer example to show your problem and share the link for it, please?

mgood7123 commented 2 years ago

You example doesn't even compile, it is also incomplete as there is no main(). After trying to fix the obvious problems and removing unused stuff, I can't reproduce the problem, everything works fine for me. I put it into compiler explorer, so you can see for yourself: https://godbolt.org/z/TsedKM3r6

Can you change the compiler explorer example to show your problem and share the link for it, please?

hmm this appears to be an issue with my logging library, as both std::out and std::cerr work as expected, where as my logging library appears to... do something that makes it print the entire contents which is weird as std::out and std::cerr does not do that

so sorry :(

d-frey commented 2 years ago

No worries, it happens. Glad you are now one step further to finding the actual problem :)

mgood7123 commented 2 years ago

yea, it was cus i was not correctly printing string_view

https://en.cppreference.com/w/cpp/string/basic_string_view/data

"Unlike std::basic_string::data() and string literals, data() may return a pointer to a buffer that is not null-terminated. Therefore it is typically a mistake to pass data() to a routine that takes just a const CharT* and expects a null-terminated string. "

XLog &XLog::operator<<(const std::pair<const char*, size_t> &value) {
    for (size_t i = 0; i < value.second; i++) {
        *this << value.first[i]; // we flush on '\n' so we need to output every character anyway
    }
    return *this;
}

XLog& XLog::operator<<(const std::string &value) {
    return *this << std::make_pair(value.data(), value.size());
}

XLog &XLog::operator<<(const std::string_view &value) {
    return *this << std::make_pair(value.data(), value.size());
}
d-frey commented 2 years ago

Yeah, typical trap. Just remember to never use data(), always use c_str() when you need a const char*. When there is no c_str(), you will now know that something is off. And when using data(), it should always be used together with size().