ericniebler / range-v3

Range library for C++14/17/20, basis for C++20's std::ranges
Other
4.14k stars 439 forks source link

Faster root cause identification in concepts violation #94

Open anders-sjogren opened 9 years ago

anders-sjogren commented 9 years ago

When a class violates a concept, it should be as quick and clear as possible to pin down the root cause of the violation. Is there currently such a way to do it?

Example scenario: I have a custom class DataFrame with a custom iterator DataFrameIterator. I had forgotten to mark operator* of DataFrameIterator as const.

I called ranges::for_each and got the error no matching function for call to object of type ‘const for_each_fn’.

My lengthy way to find out the root cause (the non-const operator*) is described below. I describe it just to show that it was quite lengthy for a simple problem. Is there some better current way of going about finding the root cause of such issues? If not, that would be high on the list of making ranges more user friendly for anyone using custom datatypes.

For example, a models_assert<InputIterable>(df) or even better models_assert<for_each_fn::requirements>(df) which would give a compile time error with relevant information (as close as possible to the root cause) would be excellent. In concepts.hpp:116 models_ uses some SFINAE and metaprogramming to give truetype if `requirescompiles and it compiles for all recursively refined types. Using similar metaprogramming, but not SFINAE, it should be straight forward (?) to make such amodels_assert`. Understanding enough of the metaprogramming framework to do that is a bit outside my current time schedule though :-(.

Is there any ongoing work in this direction?

The root cause identification

I started by manually extracting the concept checking pieces of for_each_fn::operator():

typename P = ident,
typename I = range_iterator_t<Rng>,
typename V = iterator_common_reference_t<I>,
typename X = concepts::Invokable::result_t<P, V>,
CONCEPT_REQUIRES_(InputIterable<Rng &>() && Invokable<P, V>() && Invokable<F, X>())

Into equivalent usings and asserts at the call sites. It became something like:

using P = ident;
using I = range_iterator_t<Rng>;
using V = iterator_common_reference_t<I>;
using X = concepts::Invokable::result_t<P, V>;
static_assert(InputIterable<Rng &>(),””);
static_assert(Invokable<P, V>(),””);
static_assert(Invokable<F, X>(),””);

Here static_assert(InputIterable<Rng &>(),””); failed.

I checked the definition of InputIterable and saw that it refined Iterable and had a requires_ method which added that concepts::model_of<InputIterator>(begin(t)). A static_assert(Iterable <Rng &>(),””); succeeded and thus the problem narrowed down to that DataFrameIteratordid not satisfy InputIterator.

I continued in the same fashion.

static_assert(InputIterator<ty>(),””); //fails

//InputIterator refines WeakInputIterator, Iterator and EqualityComparable
static_assert(WeakInputIterator<ty>(),””); //fails
static_assert(Iterator<ty>(),””); //ok
static_assert(EqualityComparable<ty>(),””); //ok

//WeakInputIterator refines WeakIterator and Readable
static_assert(WeakIterator<ty>(),””); //ok
static_assert(Readable<ty>(),””); //fails

//Readable refines SemiRegular
static_assert(SemiRegular<ty>(),””); //ok

Thus one root cause was narrows down to the specifics of Readable (as coded in its requires_ method). Others might possibly exist in the specifics of InputIteratorand WeakInputIterator.

Readable::requires_ look like the following (with comments removed):

auto requires_(I i) -> decltype(
  concepts::valid_expr(
    concepts::model_of<Convertible, reference_t<I>, common_reference_t<I>>(),
    concepts::model_of<Convertible, value_t<I> &, common_reference_t<I>>(),
    concepts::same_type(indirect_move(i), indirect_move(i, *i))
));

By some additional code navigation I narrowed down a problem to that

DataFrameIterator i; adl_move_detail::indirect_move(i);

wouldn’t compile, again with a “No matching function” error, but that indirect_move(i, *i)did compile. indirect_move(i) however only calls indirect_move(i, *i). However, it takes i as const ref argument, and I thus tried with

const DataFrameIterator i; indirect_move(i, *i);

I now got the informative answer: Indirection requires pointer operand (‘const I’ (aka ‘const RowIterator<self_t>’) invalid), and I could fix the problem to add const to operator*. However, this had taken embarrassingly long (something like two hours) to find a very simple problem.

ericniebler commented 9 years ago

Yes, it's a problem. I don't know of a way to fix this without proper support for concepts in the language. I'll leave this bug open for now in the hopes that something comes to me.

pfultz2 commented 9 years ago

For example, a models_assert(df) or even better models_assert(df) which would give a compile time error with relevant information (as close as possible to the root cause) would be excellent.

I believe for_each_fn already provides the nested Concept alias for the type requirements. This could be expanded to provide a better error message by default. For example,

struct for_each_fn
{
    template<typename Rng, typename F>
    using Concept = decltype(capture() ->* 
        Iterable<Rng> && 
        Invokable<F, range_common_reference_t<Rng>> &&
        Iterable<concepts::Invokable::result_t<F, range_common_reference_t<Rng>>>
    );

    template<typename Rng, typename F>
    for_each_view<Rng, F> operator()(Rng && rng, F f) const
    {
        return {std::forward<Rng>(rng), std::move(f)};
    }
};

with_concepts<for_each_fn> for_each = {};

Now the capture() ->* would be used to capture the predicate expression(similar to the catch test framework). However, since each concept is just constructed, the library can use that to figure out which concept failed and provide more detail message about that. All this checking can be done in a with_concepts function adaptor, something like this:

template<class F>
struct with_concepts
{
    // Forward to F when it meets its type requirements
    template<class... Ts, class=typename std::enable_if<F::Concept<Ts&&...>::value>::type>
    auto operator()(Ts&&... xs) const -> decltype(F()(std::forward<Ts>(xs)))
    { return F()(std::forward<Ts>(xs)); }

    // Produce a special assert
    template<class... Ts, class=typename std::enable_if<!F::Concept<Ts&&...>::value>::type>
    void operator()(Ts&&... xs) const
    { return F::Concept<Ts&&...>::assert_check(); }
};

This is just a rough sketch. I don't know how well it would work.

In concepts.hpp:116 models_ uses some SFINAE and metaprogramming to give truetype if requires compiles and it compiles for all recursively refined types.

In the Tick library, I have the TICK_TRAIT_CHECK that will show which concept traits failed including the refinements, so it can help narrow down which concept failed, but it still won't show you exactly why.

I now got the informative answer: Indirection requires pointer operand (‘const I’ (aka ‘const RowIterator’) invalid), and I could fix the problem to add const to operator*

Well, if the expressions were placed in the context where they don't constrain the template, like this:

 struct Readable
: refines<SemiRegular>
{
    template<typename I>
    using value_t = meta::eval<value_type<I>>;
    template<typename I>
    using reference_t = decltype(*std::declval<I>());
    template<typename I>
    using rvalue_reference_t = decltype(indirect_move(std::declval<I>()));
    template<typename I>
    using common_reference_t = meta::eval<iter_common_reference<I>>;
    template<typename I>
    using pointer_t = meta::eval<pointer_type<I>>;

    template<typename I>
    auto requires_(I i) -> decltype(
        concepts::valid_expr(
            concepts::model_of<Convertible, reference_t<I>, common_reference_t<I>>(),
            concepts::model_of<Convertible, value_t<I> &, common_reference_t<I>>(),
            concepts::same_type(indirect_move(i), indirect_move(i, *i))
    ));

    template<typename I>
    void requires_check(I i) 
    {
        using check = decltype(
            concepts::valid_expr(
                concepts::model_of<Convertible, reference_t<I>, common_reference_t<I>>(),
                concepts::model_of<Convertible, value_t<I> &, common_reference_t<I>>(),
                concepts::same_type(indirect_move(i), indirect_move(i, *i))
        ));
    }
};

Then the requires_check can be called to produce the error(I am not sure how clean the error will be). However the requirements have to be written twice, I am not quite sure how the duplication can be reduced. Just some thoughts.

anders-sjogren commented 9 years ago

Thanks for the thoughts and input!

A quick comment ( @pfultz2 ):

I believe for_each_fn already provides the nested Concept alias for the type requirements.

That is true for ranges::view::for_each_fn (thanks for the pointer!) but not so for ranges::for_each_fn (in algorithm/for_each.hpp) which was the function class I had problems using.

I'll see if I can come up with some further feedback after the weekend.

anders-sjogren commented 9 years ago

After ponding about this a bit more: In the current design, the requirements are encoded by SFINAE. That is, a lack of conformance to a concept is shown by requires_ not being found for the argument(s). The info available to us is thus the specific requirements for each concept (in requires_) and refined concepts (and their requires_). This means that the best we can ever do automatically is to point at which concepts have their specific requirements violated, but we cannot show why the specific requirements are violated.

In the DataFrameIterator example above, we could make a metaprogram which shows that a type DataFrameIterator violates InputIterator because it fails in the requires_ of Readable, but we will not be able to automatically get an error message which gives the info that it does so because it has no const operator*().

Now, what would happen if we instead had requires_ fail to compile with a reasonable error message, i.e. we change

template<typename I>
auto requires_(I i) -> decltype(
    concepts::valid_expr(
        concepts::model_of<Convertible, reference_t<I>, common_reference_t<I>>(),
        concepts::model_of<Convertible, value_t<I> &, common_reference_t<I>>(),
        concepts::same_type(indirect_move(i), indirect_move(i, *i))
));

to something like

template<typename I>
void requires_check(I i) {
    concepts::assert_model_of<Convertible, reference_t<I>, common_reference_t<I>>();
    concepts::assert_model_of<Convertible, value_t<I> &, common_reference_t<I>>();
    concepts::assert_same_type(indirect_move(i), indirect_move(i, *i));
)};

Then, when the error message is wanted, one could have a method assert_model_of which calls requires_check of InputIterator and all of its refined concepts. This would mean that Readable::requires_check< DataFrameIterator> would try to be instantiated and we would get the relevant error message. This design is closely related to the one of @pfultz2 above.

If assert_model_of is implemented in a recursive manner (calling requires_checkand then assert_model_of for all of the refined classes), then the “call stack” would also show why DataFrameIterator needs to be readable (i.e. because InputIterator refines WeakInputIterator, which in turn refines Readable).

It seems one could easily get the SFINAE version by

template< typename I>
auto requires_(I i) -> decltype(requires_check(i))

or more conveniently have some free function

template< typename Concept, typename I>
auto requires_(Concept* c, I i) -> decltype(c->requires_check(i))

In my development use case above the steps taken would basically be something along the following:

I have a custom class DataFrame with a custom iterator DataFrameIterator. I have forgotten to mark operator* of DataFrameIterator as const. I call ranges::for_each(df,f) and get the error no matching function for call to object of type ‘const for_each_fn’. I then add a call for_each.assert_concept(df,f) and get the informative error message

 `Indirection requires pointer operand (‘const I’ (aka ‘const DataFrameIterator’) invalid`

and a calls stack which shows that it needs to be so because

The error would thus be found in less than 5 minutes, instead of something like 2 hours. It would require less reading and understanding of the concepts framework, and thus lower the entry threshold for new users of ranges.

Does there exist some strong core reason for not taking this design path (which seems similar to the path of The Boost Concept Check Library)? Is there some drawback of having the functions defined and not just declared? Since the functions can be inlined and are no-operations when they compile, it shouldn’t increase code size? Am I missing some other obvious aspect of concept checking?

pfultz2 commented 9 years ago

It seems one could easily get the SFINAE version by

No you can't turn a compile error into substitution failure. You would need to have two overloads.

Does there exist some strong core reason for not taking this design path (which seems similar to the path of The Boost Concept Check Library)?

Boost.ConceptCheck is fairly limited. It also causes a hard error instead of constraining the template, so overloading of functions is not possible.

Is there some drawback of having the functions defined and not just declared? Since the functions can be inlined and are no-operations when they compile, it shouldn’t increase code size?

The expression has to be in the trailing decltype in order to constrain the template. Putting it in the definition does not constrain the template. Put another way, the trailing decltype will transport the substitution failure, but inside the definition the substitution failures will always be errors. For example, say we had a function f and that tried to call the member function .foo:

template<class T>
auto f(T x) -> decltype(x.foo())
{ return x.foo(); }

If foo is not a member of T it will be a substitution failure. The compiler will say something like 'no matching function call' and point to the call of the function. However, if it is put in the definition only, like this:

template<class T>
decltype(auto) f(T x)
{ return x.foo(); }

Then if foo is not a member of T, it will be a compile error. The compiler will say something like 'foo is not a member of T', and the error will point inside of f. Does that make sense?

Thats why there would need to be two overloads. One for substitution failure and the other one for errors(perhaps a macro could combine the two, but I can't think of a clean way to do it). Ultimately, clang needs to improve the way it reports back substitution failures(see the bug report here) which could help alleviate the need for two overloads.

Additionally, the compiler could report back the diagnostics all together without additional library support. So clang already knows where an enable_if occurs, so it could parse the boolean expression and report back substitution failures that occurred when trying to instantiate the traits that were false.

anders-sjogren commented 9 years ago

It seems one could easily get the SFINAE version by

No you can't turn a compile error into substitution failure.

Yes, you seem to be right! My mistake in how SFINAE work. Thanks for pointing it out.

Thats why there would need to be two overloads. One for substitution failure and the other one for errors(perhaps a macro could combine the two, but I can't think of a clean way to do it)

Yes, some macro should be possible to use. It probably wouldn't be beautiful, but it should do the job and thus be highly helpful.

Alternatively, the metaprogramming approach of giving better information on the requirements of which refined class (e.g. Readable) remains. It should be possible to get that information more easily, by some variant of models_ in concepts.hpp:116.

anders-sjogren commented 9 years ago

A prio bump request. This feature is really highly needed!

After updating to the most recent revision my code broke and I'm in the situation of both finding due to what concept it broke (according to the lengthy steps above) and then understand what that actually means.

To add to the frustration, this is using a simple ranges::for_each on some custom iterators (according to the above) where a simple non-concept-guarded for_each implementation work very fine.

It would be a significantly smoother ride to follow the ranges-implementation, with it's changing concepts, if I was at least told due to what concept my code is not working.

Ok, time for me to stop whining and be constructive... ;-)

The core problem is to provide both soft-failing require_ (which is simply masked by SFINAE and which is typically used when a overload should be masked) and a hard-failing require_ (which is typically used to actually get a meaningful message of why an overload is masked).

As previously stated in this thread, one and the same function can't do both. A solution is thus to use macros to get the two, i.e. one

template<typename T, typename U> auto requires_(soft_fail_mode_t fail_mode, T, U) -> 
    decltype(constraints(fail_mode));

and one

template<typename T, typename U> void requires_(hard_fail_mode_t fail_mode, T, U) {
    using test=decltype(constraints(fail_mode));
};

Inside constraints the mode fail_mode can be further used to pass the fail mode further.

To take a concrete example of the most complicated type of requirements that I could find (for Common):

template<typename T, typename U,
    enable_if_t<!std::is_same<uncvref_t<T>, uncvref_t<U>>::value> = 0,
    typename C = value_t<T, U>,
    typename R = common_reference_t<T const &, U const &>>
auto requires_(T t, U u) -> decltype(
    concepts::valid_expr(
        concepts::model_of<CommonReference, T const &, U const &>(),
        concepts::model_of<CommonReference, C &, R>()
    ));

Here we have three parts (as I see it).

  1. The core template parameters (i.e. typename T, typename U) and a core enable_if where there is otherwise another function which is enabled. This part can remain unchanged in both versions of requires_.
  2. Convenience type aliases (e.g. for C and R). These could potentially also form SFINAE constraints if for example common_reference_t<T const &, U const &> does not compile for T and U.
  3. The parameters (T t, U u)
  4. The actual valid expressions, inside valid_expr.

Parts 1, 3 and 4 seems to be relatively straight forward to get macro-paste into place correctly, while part 2 seems harder:

//Soft fail version (current)
template<
    //Part 1 begin
    typename T, typename U, enable_if_t<!std::is_same<uncvref_t<T>, uncvref_t<U>>::value> = 0
    //Part 1 end

    //Part 2 begin
    ,
    typename C = value_t<T, U>,
    typename R = common_reference_t<T const &, U const &>>
    //Part 2 end
>
auto requires_(soft_fail_mode_t m, T t, U u) -> decltype(
    concepts::valid_expr(
    //Part4 begin
        concepts::model_of<CommonReference, T const &, U const &>(m),
        concepts::model_of<CommonReference, C &, R>(m)
    //Part4 end
    )
);

//Hard fail version
template<
    //Part 1 begin
    typename T, typename U, enable_if_t<!std::is_same<uncvref_t<T>, uncvref_t<U>>::value> = 0
    //Part 1 end
>
void requires_(hard_fail_mode_t m, T t, U u){
    //Part 2 begin
    using C = value_t<T, U>;
    using R = common_reference_t<T const &, U const &>;
    //Part 2 end

    using test = decltype(concepts::valid_expr(
    //Part4 begin
        concepts::model_of<CommonReference, T const &, U const &>(m),
        concepts::model_of<CommonReference, C &, R>(m)
    //Part4 end
    ));
};

Here, one solution could be to skip step 2 by introducing a type alias outside the require function declaration/definition, but inside the concept class.

template<class T, class U> using C = value_t<T, U>;
template<class T, class U> using R = common_reference_t<T const &, U const &>;

We would then get:

//Soft fail version (current)
template<
    //Part 1 begin
    typename T, typename U, enable_if_t<!std::is_same<uncvref_t<T>, uncvref_t<U>>::value> = 0
    //Part 1 end
>
auto requires_(soft_fail_mode_t fail_mode, T t, U u) -> decltype(
    concepts::valid_expr(
    //Part4 begin
        concepts::model_of<CommonReference, T const &, U const &>(m),
        concepts::model_of<CommonReference, C<T,U> &, R<T,U>>(m)
    //Part4 end
));

//Hard fail version
template<
    //Part 1 begin
    typename T, typename U, enable_if_t<!std::is_same<uncvref_t<T>, uncvref_t<U>>::value> = 0
    //Part 1 end
>
void requires_(hard_fail_mode m, T t, U u){ using test = decltype(
    concepts::valid_expr(
    //Part4 begin
        concepts::model_of<CommonReference, T const &, U const &>(m),
        concepts::model_of<CommonReference, C<T,U> &, R<T,U>>(m)
    //Part4 end
    ));
};

And the relevant parts of Common could change from:

struct Common
{
    template<typename T, typename U,
        enable_if_t<!std::is_same<uncvref_t<T>, uncvref_t<U>>::value> = 0,
        typename C = value_t<T, U>,
        typename R = common_reference_t<T const &, U const &>>
    auto requires_(T t, U u) -> decltype(
        concepts::valid_expr(
            concepts::model_of<CommonReference, T const &, U const &>(),
            concepts::model_of<CommonReference, C &, R>()
        ));
};

to:

struct Common
{
    template<class T, class U> using C = value_t<T, U>;
    template<class T, class U> using R = common_reference_t<T const &, U const &>;

    CONCEPT_REQUIRES_IMPL(
        CONCEPT_TEMPLATE_PARAMETERS(
            typename T, typename U,
            enable_if_t<!std::is_same<uncvref_t<T>, uncvref_t<U>>::value> = 0),
        CONCEPT_PARAMETERS(T t, U u),
        CONCEPT_VALID_EXPRESSIONS(
            concepts::model_of<CommonReference, T const &, U const &>(fail_mode),
            concepts::model_of<CommonReference, C<T,U> &, R<T,U>>(fail_mode))
    )    
};

where we have the predefined tags and macros in a header:

struct soft_fail_mode_t{};
struct hard_fail_mode_t{};

//The first macros basically just quote the arguments, to avoid comma problems.
//http://stackoverflow.com/questions/13842468/comma-in-c-c-macro
#define CONCEPT_TEMPLATE_PARAMETERS(...) __VA_ARGS__
#define CONCEPT_PARAMETERS(...) __VA_ARGS__
#define CONCEPT_VALID_EXPRESSIONS(...) __VA_ARGS__
#define CONCEPT_REQUIRES_IMPL(tpar,par,valexpr)\
template< tpar >\
auto requires_(soft_fail_mode_t fail_mode, par) -> decltype(concepts::valid_expr(valexpr));\
template< tpar >\
void requires_(hard_fail_mode_t fail_mode, par) { using test = decltype(concepts::valid_expr(valexpr)); };

We could then call requires_, models, etc, with either soft_fail_mode_t to get the current behaviour or hard_fail_mode_t to get an error in the correct location, which then gives a much better hint on what is actually wrong.

What's everyone's take on this?

ericniebler commented 9 years ago

I'm curious what commit broke your code and how. Unfortunately, I'm unlikely to have time to work on this issue, even though I think it's important. My priority is to work on the proposal and any parts of the code that effect the proposal.

anders-sjogren commented 9 years ago

Embarrassingly, it turns out it was an error of my own that broke the code.

I had merged in the latest changes to a fork of my own (where I allow customization of for_each), and concluded that the cause of the error was in those changes, but instead it was in the merging process. Since the merging error was in a SFINAE situation, I just got the same "no matching function for call to object of type ‘const for_each_fn’" as I get when the concepts are violated, and I erroneously concluded that it was the changed concepts (IndirectInvokable) that broke the code.

Sorry for the false alert on recent changes being of breaking nature.

The topic of this post is still valid though. In a way, this erroneous root cause deduction of mine even provides another example of how better root cause identification of violated concepts is important. Had I been used to getting good error messages for broken concepts, I'd never had thought a violated concept to be the problem.

pfultz2 commented 9 years ago

Ok well using these macros:

#define CAT(x, y) PRIMITIVE_CAT(x, y)
#define PRIMITIVE_CAT(x, y) x ## y

#define IS_PAREN(x) IS_PAREN_CHECK(IS_PAREN_PROBE x)
#define IS_PAREN_CHECK(...) IS_PAREN_CHECK_N(__VA_ARGS__,0)
#define IS_PAREN_PROBE(...) ~, 1,
#define IS_PAREN_CHECK_N(x, n, ...) n

#define EMPTY(...)
#define DEFER(...) __VA_ARGS__ EMPTY()
#define OBSTRUCT(...) __VA_ARGS__ DEFER(EMPTY)()
#define EXPAND(...) __VA_ARGS__

#define IIF(c) PRIMITIVE_CAT(IIF_, c)
#define IIF_0(t, ...) __VA_ARGS__
#define IIF_1(t, ...) t

#define WALL(...) __VA_ARGS__
#define RAIL_IIF(c) PRIMITIVE_CAT(RAIL_IIF_, c)
#define RAIL_IIF_0(t, ...) __VA_ARGS__
#define RAIL_IIF_1(t, ...) t
#define RAIL(macro) \
    RAIL_IIF(IS_PAREN(WALL(())))( \
        RAIL_ID OBSTRUCT()()(macro), \
        macro OBSTRUCT() \
    )
#define RAIL_ID() RAIL

#define VALID_SFINAE(...) decltype(concepts::valid_expr(__VA_ARGS__));
#define VALID_ERROR(...) void() { using check = VALID_SFINAE(__VA_ARGS__) }

#define REQUIRES_IMPL_ERROR_CONTEXT(...) REQUIRES_IMPL_ERROR_CONTEXT_X(WALL(__VA_ARGS__))
#define REQUIRES_IMPL_ERROR_CONTEXT_X(...) __VA_ARGS__

#define REQUIRES_IMPL_SFINAE_CONTEXT(...) REQUIRES_IMPL_SFINAE_CONTEXT_X(WALL(__VA_ARGS__))
#define REQUIRES_IMPL_SFINAE_CONTEXT_X(...) __VA_ARGS__

#define VALID_IMPL() IIF(IS_PAREN(REQUIRES_IMPL_ERROR_CONTEXT(()))) \
    (VALID_SFINAE, VALID_ERROR)

#define VALID RAIL(VALID_IMPL)()

#define REQUIRES_IMPL_SFINAE(...) REQUIRES_IMPL_SFINAE_CONTEXT(__VA_ARGS__)
#define REQUIRES_IMPL_ERROR(...) struct error { REQUIRES_IMPL_ERROR_CONTEXT(__VA_ARGS__) };

#define REQUIRES_IMPL(...) REQUIRES_IMPL_SFINAE(__VA_ARGS__) REQUIRES_IMPL_ERROR(__VA_ARGS__)

You could write the concept like this:

struct Common
{
    template<class T, class U> using C = value_t<T, U>;
    template<class T, class U> using R = common_reference_t<T const &, U const &>;

    REQUIRES_IMPL(
        template<typename T, typename U,
            enable_if_t<!std::is_same<uncvref_t<T>, uncvref_t<U>>::value> = 0>
            auto requires_(T t, U u) -> VALID(
                concepts::model_of<CommonReference, T const &, U const &>(),
                concepts::model_of<CommonReference, C<T,U> &, R<T,U>>()
            )
        ) 
};

And then it would expand to this:

struct Common 
{
    template <class T, class U> using C = value_t<T, U>;
    template <class T, class U>
    using R = common_reference_t<T const &, U const &>;

    template <typename T, typename U,
            enable_if_t<!std::is_same<uncvref_t<T>, uncvref_t<U>>::value> = 0>
    auto requires_(T t, U u) -> decltype(concepts::valid_expr(
        concepts::model_of<CommonReference, T const &, U const &>(),
        concepts::model_of<CommonReference, C<T, U> &, R<T, U>>()
    ));

    struct error 
    {
        template <typename T, typename U,
                  enable_if_t<!std::is_same<uncvref_t<T>, uncvref_t<U>>::value> = 0>
        auto requires_(T t, U u) -> void() {
          using check = decltype(concepts::valid_expr(
                concepts::model_of<CommonReference, T const &, U const &>(),
                concepts::model_of<CommonReference, C<T, U> &, R<T, U>>()
            ));
        }
    };
};

Of course, I don't think Eric wants all this macro vodoo in his range library, but its about the cleanest way I have found so far to write it. The biggest problem with this approach is that the valid expressions are in a macro, so the compiler can't point to the exact expression that caused the error.

Manu343726 commented 9 years ago

Hi. After three days my "stack trace" experiment is starting to look fine: captura de pantalla_2015-06-20_21-39-51

That's the output of an Integral-like concept applied to float. As you can see, it reports exactly what requirements (form both refinements or the concept itself) failed or succeed. The message is a constexpr string generated completely at compile-time. There's still the problem of passing that message as a diagnostic...

I'm currently busy with the university, but I want to clean-up and open the project by July. I hope you like the idea.

ericniebler commented 9 years ago

This looks pretty good.

On Sat, Jun 20, 2015, 12:50 PM Manu343726 notifications@github.com wrote:

Hi. After three days my "stack trace" experiment is starting to look fine: [image: captura de pantalla_2015-06-20_21-39-51] https://cloud.githubusercontent.com/assets/1561197/8268968/93cb54fe-1795-11e5-86b4-1aee36523667.png

That's the output of an Integral-like concept applied to float. As you can see, it reports exactly what requirements (form both refinements or the concept itself) failed or succeed. The message is a constexpr string generated completely at compile-time.

I'm currently busy with the university, but I want to clean-up and open the project by July. I hope you like the idea.

— Reply to this email directly or view it on GitHub https://github.com/ericniebler/range-v3/issues/94#issuecomment-113813848 .

Manu343726 commented 9 years ago

Here are some advances: Trait checking, expected type from expressions, etc. Also I have been working on the format of the error message, now it reflects the refinement hierarchy in a bottom up way: captura de pantalla_2015-07-02_14-00-58

I hope to have this ready to publish next week or so, then you could judge if this is useful or not. In its current state, it's more an experiment than a serious alternative since lack of compiler support. The implementation is based on stateful typelists and only Clang does evaluation in the order I expect to fill those as required.