Skip to content
This repository has been archived by the owner on Dec 1, 2022. It is now read-only.

Replace unique_ptr with raw pointer in expression implementation #546

Merged
merged 20 commits into from
Jun 29, 2021

Conversation

Aiee
Copy link
Contributor

@Aiee Aiee commented May 27, 2021

  1. Replace all unique_ptr(s) with raw pointers in expression implementation and manage object lifetime using ObjectPool.
  2. Disable default expression constructors to preventing mixing raw pointer and unique_ptr.

TODOs:

  • Replace expression constructor with static functions, specifying expression type, and add expression into ObjectPool during the construction.
  • Split expression UTs to separate files
  • Modify other related common UTs
  • Encode/Decode test

@Aiee Aiee added enhancement Type: make the code neat or more efficient wip Solution: work in progress do not review PR: not ready for the code review yet labels May 27, 2021
@Shylock-Hg
Copy link
Contributor

The raw pointer is too implicit for the lifetime management. You could force everyone remember all object in expression is managed by objpool, but when these object referenced by other or passed from others it's more confuse.

@Aiee Aiee force-pushed the refactor-expression branch 3 times, most recently from ac74ebd to 5e3d68b Compare June 2, 2021 07:13
@Aiee Aiee added the ready-for-testing PR: ready for the CI test label Jun 2, 2021
@Aiee Aiee force-pushed the refactor-expression branch 3 times, most recently from 10a3920 to 349c938 Compare June 2, 2021 17:00
@Aiee Aiee removed do not review PR: not ready for the code review yet wip Solution: work in progress labels Jun 2, 2021
@Aiee Aiee requested a review from a team June 3, 2021 02:05
@Aiee Aiee changed the title [WIP] Replace unique_ptr with raw pointer in expression implementation Replace unique_ptr with raw pointer in expression implementation Jun 4, 2021
@Aiee Aiee force-pushed the refactor-expression branch 3 times, most recently from c42e383 to 6c4516b Compare June 10, 2021 02:13
@Aiee
Copy link
Contributor Author

Aiee commented Jun 11, 2021

The raw pointer is too implicit for the lifetime management. You could force everyone remember all object in expression is managed by objpool, but when these object referenced by other or passed from others it's more confuse.

After off-line discussion, this scenario could be solved by changing all expression-related members from unique_ptrs to raw pointers.

@CLAassistant
Copy link

CLAassistant commented Jun 17, 2021

CLA assistant check
All committers have signed the CLA.

@Aiee Aiee force-pushed the refactor-expression branch 3 times, most recently from 20c279b to 0b15f5d Compare June 23, 2021 13:04
@Aiee Aiee added the incompatible PR: incompatible with the master branches in the storage or common repos label Jun 24, 2021
CPWstatic
CPWstatic previously approved these changes Jun 28, 2021
Clean up lambda capture
Copy link
Contributor

@yixinglu yixinglu left a comment

Choose a reason for hiding this comment

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

ltgm

@yixinglu yixinglu merged commit c815e9e into vesoft-inc:master Jun 29, 2021
@Aiee Aiee deleted the refactor-expression branch June 29, 2021 07:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Type: make the code neat or more efficient ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants