Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete Ref & VectorRef and add GetDataSafely #22997

Merged
merged 10 commits into from
Apr 4, 2020

Conversation

chenwhql
Copy link
Contributor

@chenwhql chenwhql commented Mar 13, 2020

This PR revome two invalid error check interface, and add a valid error check interface:

not change the code logic, only enhances the error message

RemoveRef & VectorRef check (remove safe_ref.h)

Ref & VectorRef are used to safely obtain the data pointed to by a pointer. Before * ptr, they will check whether the pointer is nullptr. but...

  • The code used these two interface get data from ptr, almost no error messages
  • use Ref & VectorRef cannot tell users the actual error pointer
  • use Ref & VectorRef cannot tell users the actual error file & line number
  • More than 100 places used

example:

int* a = nullptr;
paddle::operators::detail::Ref(a);
----------------------
Error Message Summary:
----------------------
Error:
[Hint: ptr should not be null.] at (/work/paddle/paddle/fluid/operators/detail/safe_ref.h:28)
  • no error message!
  • error ptr value name is a, not ptr!
  • error file & line are enforce_test.cc:369, not safe_ref.h:28!

Add GET_DATA_SAFELY

  • A new error check interface in enforce.h to replace Ref & VectorRef, resolve all above problems!
  • You only need use GET_DATA_SAFELY macro, and pass in 3 key word, you can get a complete and accurate error message
  • The error message using the new interface is like follows:
int* a = nullptr;
GET_DATA_SAFELY(a, "Input", "X", "dummy");
----------------------
Error Message Summary:
----------------------
InvalidArgumentError: Unable to get int data of Input X in operator dummy. Possible reasons are:
  1. The X is not the Input of operator dummy;
  2. The X has no corresponding variable passed in;
  3. The X corresponding variable is not initialized.
  [Hint: pointer a should not be null.] at (/work/paddle/paddle/fluid/platform/enforce_test.cc:383)

@chenwhql chenwhql changed the title Delete invalid check inferface Ref & VectorRef Delete Ref & VectorRef and add PADDLE_GET_DATA_SAFELY Mar 31, 2020
@chenwhql chenwhql changed the title Delete Ref & VectorRef and add PADDLE_GET_DATA_SAFELY Delete Ref & VectorRef and add PADDLE_GET_DATA_SAFELY Mar 31, 2020
@chenwhql chenwhql changed the title Delete Ref & VectorRef and add PADDLE_GET_DATA_SAFELY Delete Ref & VectorRef and add PADDLE GET DATA SAFELY Mar 31, 2020
@chenwhql chenwhql changed the title Delete Ref & VectorRef and add PADDLE GET DATA SAFELY Delete Ref & VectorRef and add GetDataSafely Mar 31, 2020
Copy link
Collaborator

@raindrops2sea raindrops2sea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chenwhql chenwhql merged commit 16315d3 into PaddlePaddle:develop Apr 4, 2020
mapingshuo pushed a commit to mapingshuo/Paddle that referenced this pull request Apr 7, 2020
* delete invalid check inferface Ref & VectorRef, test=develop

* fix vector ref delete error, test=develop

* try the new check inferface, test=develop

* change all related code with new check macro, test=develop

* remove static assert, test=develop

* polish detail, test=develop

* skip coverage problem, test=develop

* add new check macro, test=develop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants