joemalle / limn

A tiny parser designed to compile quickly
Boost Software License 1.0
2 stars 1 forks source link

match_ instance is not created, which causes the crash #3

Closed asmwarrior closed 1 year ago

asmwarrior commented 1 year ago

In the recent commit: 0a3865f in 2023-01-13. When testing, I noticed that the code here:

    /// @brief The match operator
    /// @details This side-effect-only function copies the matched part of
    ///     input to its argument output.  If there is no match, then output
    ///     is unchanged.  For example, `(* lm::char_('a'))[output]` will match
    ///     any number of `a` characters in sequence, and it will copy that
    ///     sequence to the `std::string_view` `output`.
    ///
    ///     This is useful for getting the matched string out of the parser.
    ///     It's also sometimes useful for printf-debugging.
    ///
    ///     See tests.cpp or README.md for an example.
    ///
    /// @param[out] output The argument to receive the match portion.
    template <typename Base>
    constexpr inline auto impl::parser_base<Base>::operator[](std::string_view& ouput) const noexcept {
        return impl::match_<Base>(*static_cast<Base const*>(this), ouput);
    }

    /// @brief The match callback operator
    /// @details The std::function will be called when an item get matched.
    ///     The std::function should have the protytype
    ///     `std::function<void(const std::string_view&)>` where the argument
    ///     is the matched string_view.
    ///     If there is no match, then this function will not be called.
    ///     For example, `(* lm::char_('a'))[functor]` will match
    ///     any number of `a` characters in sequence, and it will call the functor.
    ///
    ///     This is useful for add the user defined action in this std::function
    ///     For example, adding the AST of the matched item to the parsing tree
    ///
    ///     See tests.cpp or README.md for an example.
    ///
    /// @param[in] callback The std::function callback function which will be called when
    ///     matched portion, the functor's parameter is the matched string_view.
    template <typename Base>
    constexpr inline auto impl::parser_base<Base>::operator[](std::function<void(const std::string_view&)> callback) const noexcept {
        return impl:: match_call_<Base>(*static_cast<Base const*>(this), std::move(callback));
    }

I see a lot of the code using the member function of impl::match_, if we only initialized the impl:: match_call_ instance, you will get crash, for example, one crash here:

        template <typename Base>
        struct match_ final : public impl::parser_base<match_<Base>> {
            constexpr explicit match_(Base base, std::string_view& sv) noexcept
                : base(std::move(base))
                , out(sv)
            {
                std::cout << "constructor of match_" << std::endl;
            }

            constexpr inline bool visit(std::string_view& sv) const& noexcept {
                std::string_view save = sv;
                if (base.visit(sv)) {
                    out = save.substr(0, save.size() - sv.size());
                    return true;
                }
                return false;
            }

        private:
            Base base;
            std::string_view& out;
        };

In the line

out = save.substr(0, save.size() - sv.size());

Because the out is not initialized.

asmwarrior commented 1 year ago

I think I have found this crash issue, it is here:

diff --git a/limn.h b/limn.h
index 5722ef5..9fa896e 100644
--- a/limn.h
+++ b/limn.h
@@ -369,7 +369,7 @@ namespace lm {
         };

         template <typename Base>
-        struct match_call_ final : public impl::parser_base<match_<Base>> {
+        struct match_call_ final : public impl::parser_base<match_call_<Base>> {
             constexpr explicit match_call_(Base base, std::function<void(const std::string_view&)> callback) noexcept
                 : base(std::move(base))
                 , callback(std::move(callback))

I'm not sure why it used such recursive class inheritance here, since for match_call_, we need:

struct match_call_ final : public impl::parser_base<match_call_<Base>> {

Not the old match_.

asmwarrior commented 1 year ago

I have added a PR to fix this crash issue.

I think it is a copy/paste error originally from your comment in issue #1 , never mind, we have fixed it.

Thanks.

joemalle commented 1 year ago

merged + fixed - thank you!