New issue
Advanced search Search tips

Issue 746955 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Emit readable diagnostic message on base::Callback compilation error

Project Member Reported by tzik@chromium.org, Jul 20 2017

Issue description

Compilation errors around base::Callback are hard to understand, and hard to find a fix. So, it's nice to have to detect typical error cases and emit readable diagnostic message for them (using a static_assert or a clang plugin).

E.g.:
 - Detect base::OnceCallback on the first parameter of base::Bind.
 - Detect lvalue base::OnceCallback on the first parameter of base::BindOnce.
 - Detect bare move-only types on base::Bind or base::BindOnce.
 

Comment 1 by tzik@chromium.org, Jul 25 2017

Here is an attempt to make the error message less unreadable: https://chromium-review.googlesource.com/c/583976/

For `base::BindOnce([](int){}, nullptr);`, the current code emits an error message below.
------------------------
../../base/bind_internal.h:138:12: error: no matching function for call to object of type 'const (lambda at ../../base/bind_unittest.cc:1414:18)'                                                                                                                                         
    return functor(std::forward<RunArgs>(args)...);                                                                                                                                                                                                                                       
           ^~~~~~~                                                                                                                                                                                                                                                                        
../../base/bind_internal.h:262:20: note: in instantiation of function template specialization 'base::internal::FunctorTraits<(lambda at ../../base/bind_unittest.cc:1414:18), void>::Invoke<nullptr_t>' requested here                                                                    
    return Traits::Invoke(std::forward<Functor>(functor),                                                                                                                                                                                                                                 
                   ^                                                                                                                                                                                                                                                                      
../../base/bind_internal.h:338:43: note: in instantiation of function template specialization 'base::internal::InvokeHelper<false, void>::MakeItSo<(lambda at ../../base/bind_unittest.cc:1414:18), nullptr_t>' requested here                                                            
    return InvokeHelper<is_weak_call, R>::MakeItSo(                                                                                                                                                                                                                                       
                                          ^                                                                                                                                                                                                                                               
../../base/bind_internal.h:303:12: note: in instantiation of function template specialization 'base::internal::Invoker<base::internal::BindState<(lambda at ../../base/bind_unittest.cc:1414:18), nullptr_t>, void ()>::RunImpl<(lambda at ../../base/bind_unittest.cc:1414:18), std::__1\
::tuple<nullptr_t>, 0>' requested here                                                                                                                                                                                                                                                    
    return RunImpl(std::move(storage->functor_),                                                                                                                                                                                                                                          
           ^                                                                                                                                                                                                                                                                              
../../base/bind.h:40:45: note: in instantiation of member function 'base::internal::Invoker<base::internal::BindState<(lambda at ../../base/bind_unittest.cc:1414:18), nullptr_t>, void ()>::RunOnce' requested here                                                                      
  PolymorphicInvoke invoke_func = &Invoker::RunOnce;                                                                                                                                                                                                                                      
                                            ^                                                                                                                                                                                                                                             
../../base/bind_unittest.cc:1414:9: note: in instantiation of function template specialization 'base::BindOnce<(lambda at ../../base/bind_unittest.cc:1414:18), nullptr_t>' requested here                                                                                                
  base::BindOnce([](int){}, nullptr);                                                                                                                                                                                                                                                     
        ^                                                                                                                                                                                                                                                                                 
../../base/bind_unittest.cc:1414:18: note: candidate function not viable: no known conversion from 'nullptr_t' to 'int' for 1st argument                                                                                                                                                  
  base::BindOnce([](int){}, nullptr);                                                                                                                                                                                                                                                     
                 ^                                                                                                                                                                                                                                                                        
../../base/bind_unittest.cc:1414:18: note: conversion candidate of type 'void (*)(int)' 
------------------------

In the new code, it emits another error message below.
------------------------
../../base/bind.h:40:3: error: static_assert failed "|Param| needs to be constructible from |Unwrapped| type. The failing argument is passed as the |i|th parameter, whose type is |Arg|, and delivered as |Unwrapped| into |Param|."                                                     
  static_assert(std::is_constructible<Param, Unwrapped>(),                                                                                                                                                                                                                                
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                                                                                                                                                                                                 
../../base/bind.h:62:7: note: in instantiation of template class 'base::internal::AssertConstructible<0, nullptr_t, nullptr_t &&, int>' requested here                                                                                                                                    
    : AssertConstructible<Ns, Args, Unwrapped, Params>... {                                                                                                                                                                                                                               
      ^                                                                                                                                                                                                                                                                                   
../../base/bind.h:147:7: note: in instantiation of template class 'base::internal::AssertBindArgsValidity<base::IndexSequence<0>, base::internal::TypeList<nullptr_t>, base::internal::TypeList<nullptr_t &&>, base::internal::TypeList<int> >' requested here                            
      check;                                                                                                                                                                                                                                                                              
      ^                                                                                                                                                                                                                                                                                   
../../base/bind_unittest.cc:1414:9: note: in instantiation of function template specialization 'base::BindOnce<(lambda at ../../base/bind_unittest.cc:1414:18), nullptr_t>' requested here                                                                                                
  base::BindOnce([](int){}, nullptr);                                                                                                                                                                                                                                                     
        ^ 

Comment 2 by tzik@chromium.org, Jul 25 2017

Cc: dcheng@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ae4202edba0e7fd9a6a18f096c17a586869446ff

commit ae4202edba0e7fd9a6a18f096c17a586869446ff
Author: tzik <tzik@chromium.org>
Date: Mon Jul 31 10:41:54 2017

Add static_assert on mismatches of base::Bind args and the target params

This CL adds a static_assert to detect the Bind earlier and to emit an
readable error message.

Bug:  746955 
Change-Id: Id8081be63f789c7896a9a4229fe3978a7fdedc41
Reviewed-on: https://chromium-review.googlesource.com/583976
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490741}
[modify] https://crrev.com/ae4202edba0e7fd9a6a18f096c17a586869446ff/base/bind.h
[modify] https://crrev.com/ae4202edba0e7fd9a6a18f096c17a586869446ff/base/bind_internal.h
[modify] https://crrev.com/ae4202edba0e7fd9a6a18f096c17a586869446ff/base/bind_unittest.nc

Comment 4 by tzik@chromium.org, Aug 16 2017

Status: Fixed (was: Assigned)

Sign in to add a comment