Skip to content
This repository was archived by the owner on Mar 24, 2022. It is now read-only.

Make smart pointers great again [#118594399] [#118323445]#22

Open
d wants to merge 2 commits intomasterfrom
autop_autoref
Open

Make smart pointers great again [#118594399] [#118323445]#22
d wants to merge 2 commits intomasterfrom
autop_autoref

Conversation

@d
Copy link
Contributor

@d d commented Apr 29, 2016

This patch makes gpos::CAutoP and gpos::CAutoRef a bit friendlier to developers: when used with wrong template parameter, they blow up at compile time.

Correct usage should always use CAutoP with Non CRefCount object, and CAutoRef with CRefCount object (which has Release() method).

We also removed the VERY DANGEROUS reinterpret_cast in CAutoRef::~CAutoRef(), which would corrupt the object.

@d and @xinzweb

@gpdbdreddbot
Copy link

Hello d!

Thanks for submitting this pull request!

All pull request authors must have a Contributor License Agreement (CLA) on-file with us. Please sign the appropriate CLA (individual or corporate).

When sending signed CLA please provide your github username in case of individual CLA or the list of github usernames that can make pull requests on behalf of your organization.

If you are confident that you're covered under a Corporate CLA, please make sure you've publicized your membership in the appropriate Github Org, per these instructions.

@d
Copy link
Contributor Author

d commented Apr 29, 2016

duh i'm gonna fix this by being openly pivotal i guess

#ifndef GPOS_CAutoP_H
#define GPOS_CAutoP_H

#include <type_traits>
Copy link
Contributor

Choose a reason for hiding this comment

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

why?

@d d closed this May 10, 2016
@gpdbdreddbot
Copy link

Hello d!

Thanks for submitting this pull request!

All pull request authors must have a Contributor License Agreement (CLA) on-file with us. Please sign the appropriate CLA (individual or corporate).

When sending signed CLA please provide your github username in case of individual CLA or the list of github usernames that can make pull requests on behalf of your organization.

If you are confident that you're covered under a Corporate CLA, please make sure you've publicized your membership in the appropriate Github Org, per these instructions.

@d d reopened this May 10, 2016
@d d closed this May 10, 2016
@d d reopened this May 10, 2016
@gpdbdreddbot
Copy link

Hello d!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you've already signed the CLA.

Jesse Zhang and Xin Zhang and others added 2 commits May 19, 2016 18:46
Both snippets should fail at compile-time:
```
GPOS_RESULT AutoPShouldBlowUpGivenRefCount()
{
	// create memory pool
	CAutoMemoryPool amp;
	IMemoryPool *pmp = amp.Pmp();

	CAutoP<CRefCount> aprf(GPOS_NEW(pmp) CRefCount());
	return GPOS_OK;
}
```

```
static GPOS_RESULT AutoRefShouldBlowUpGivenNonRefCount()
{
	// create memory pool
	CAutoMemoryPool amp;
	IMemoryPool *pmp = amp.Pmp();

	CAutoRef<int> aprf(GPOS_NEW(pmp) int());
	return GPOS_OK;
}

```
@d d force-pushed the autop_autoref branch from 2175ff2 to 024fe3c Compare May 20, 2016 20:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants