I believe the problem was in using != intead of >, and at some point we get -1 there which passed through the test, isnt it?

yes, it is.
But, i've decided that empty_gmat_descr comparison is more clearest here, isnt it ?

I don't know, does it actually works for the case?

Empty cv::Mat (as the cv::Mat in general) is not a compile argument, so this message may be misleading

Also - does this check handle the case when an empty cv::Mat is passed to .apply() ?

I've checked the code and found:

1. this parts is called from compile()
2. compile() can be called as standalone method
   or
3. be part of apply(), when arguments set was changed (recompile)

So, message looks like really misleading in direct apply() case.
And empty cv::Mat cannot be passed to .apply() because recompile would be triggered in first and invoke this compile()

@dmatveev What do you thinks about the message?
empty cv::Mat is not allowed as graph input argument

It is more correct but again, we don't deal with cv::Mat at all when compiling a graph, so referring to a structure which may not exist at the time can be misleading.

Why do we need this here? It is a public header - it is exposed in the installed library package. Is it supposed to be used somehow by end users? I believe it is not

it's actually the common part between gproto.cpp & gcompiled.cpp -which uses the same validation fucntionality

I've added it similarly in the same way with validate_input_args because i got the impression after file inspection that entire file exposes a set of auxiliary functions - additionally, i didn't face with usage other function in this file as public interface. This overall assumption score told me that file is utility file
But maybe I wrong and this utility-like function should be implemented in other place

moved to src/ai/gmeta_chec.hpp/cpp

gproto.cpp and gproto_priv.hpp are the right ones.

I believe the check itself and the error message here mix levels of abstractions.

Looking at this code, you don't know what is actually checked by this function, but the assert text complains about some specific fields.

I am not sure if it needs to be an assert at all. If you want provide details, provide it in the exception text you throw from the validate_input_meta() function (so it becomes void not bool).

It make sense, and "incorrect description of cv::*Mat" is enough

about exception i think it is a bad idea:

1. void function with the only reason in throwing exception looks ugly and unsafe and inconvenient in usage and considered as bad practice
2. GAPI_Assert here means that this place is performance critical in Release build, so exception is not good here

It is not performance critical, in the performance-critical code we use DbgAssert.

Why bool here again? I believe the code simplifies a lot if we drop this tracer thing at all and simply throw.

yes, it simplifed here, but after it become more complicated in caller context. It is a bad idea, as for me, to wrap function around try-catch every time

if you dislike optional tracer, then it is possible in the following approach

std::tuple<bool, std::string> validate_();
...
auto res = validate_(...)
if(!res.first)
{
    printf(res.second)
}

what do you think?

Mainly we don't need try/catch in/with our own code.

a separate file may be too much for this simple function, consider gproto.cpp & its _priv.hpp.

yeah, i thought about _priv.hpp, but I was confused with comment // FIXME: why it is here?

https://github.com/opencv/opencv/pull/20184/files#diff-dfc809caba75402a8e78080f173d484de3b67c66af4b57c81c680acf8275b6e4L22

And i realized that this include should be removed is in future
@dmatveev What do you think about that - do you still suggest to move it in _priv.hpp ?

I believe in 2021 nobody remembers the answer to

// FIXME: why it is here?

:)

This line is 139 characters long; I believe the OpenCV's coding style allows something up to 110 but we're trying to follow ~80. I believe the std::string() thing is not needed here as operator+(const char*, std::string) should work things out. throw_error itself is terrible (was done wrong initially and nobody has fixed it then), so we usually write this stuff like

    cv::util::throw_error
        (std::logic_error
         (...);

Also, I believe a simple assert could work fine here (except the failed value won't be printed).

done

I believe this try/catch is redundant - the code throws anyway, the only thing it adds is the error message - you can use the LOG() trick instead to provide the context.

done