Skip to content

Conversation

pan-
Copy link
Member

@pan- pan- commented Nov 13, 2017

Description

This patch adds the compile time configuration "non-copyable-warning" to mbed platform library. When this configuration flag is set to true, compile time errors resulting of the copy of a NonCopyable object are transformed into compile time and runtime warning.

This may be useful for functional legacy code that erroneously copy non copyable resources. It is highly encouraged to fix non complying code; copy of objects not designed to be copied may lead to resources leak, operation on invalid memory location as well as random malfunctioning.

This patch turns compile time errors resulting of the copy of a NonCopyable object into compile time and runtime warnings when the code is compiled with the develop or release profile. The compile time error is still issued when the code is compiled with the debug profile.

Compile time error can be enforced by setting the compile time configuration "force-non-copyable-error" to true.

This patch also adds the macro MBED_PRETTY_FUNCTION which yield the string literal of the enclosing function signature.

Status

READY

Migrations

NO

@pan-
Copy link
Member Author

pan- commented Nov 13, 2017

@sg- @janjongboom Could you review this PR ?

@sg-
Copy link
Contributor

sg- commented Nov 13, 2017

This should be enabled by default (warning, not error) for develop and release profile but error when in debug profile as other traps are also enabled.

@pan- pan- force-pushed the non-copyable-warning branch from d243481 to fef3e41 Compare November 13, 2017 17:07
@pan- pan- changed the title Platform: Non copyable warning Platform: Allow copy of non copyable objects Nov 13, 2017
@pan-
Copy link
Member Author

pan- commented Nov 13, 2017

@sg- I've updated the PR to implement the expected behavior.

@janjongboom
Copy link
Contributor

\o/

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

Many style changes that distract from actual changes, can you please split the style changes into own separate commit?

Copy link
Contributor

Choose a reason for hiding this comment

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

For future reviews, please style changes in a separate commit

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 14, 2017

@pan- 👍 Like the way it is with this patch

@tommikas jenkins has not yet started (no url here) ?

@tommikas
Copy link
Contributor

@0xc0170 Yep, started a scan so it should soon start running.

pan- added 3 commits November 14, 2017 10:06
This macro yields a string literal of the enclosing function name.
Turn the compile time error issued when a NonCopyable resource is copied
into a compile time and runtime warning.

If the application is compiled with the debug profile the compile time
error remains.

The compile time error can be enforced by setting the library option
force-non-copyable-error to true.
@pan- pan- force-pushed the non-copyable-warning branch from fef3e41 to 80c9f8b Compare November 14, 2017 10:09
@pan-
Copy link
Member Author

pan- commented Nov 14, 2017

@0xc0170 EOL whitespace in its own commit.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 14, 2017

/morph build

@mbed-ci
Copy link

mbed-ci commented Nov 14, 2017

Build : SUCCESS

Build number : 513
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5485/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

@mbed-ci
Copy link

mbed-ci commented Nov 14, 2017

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 15, 2017

/morph test

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 15, 2017

@0xc0170 Yep, started a scan so it should soon start running.

@tommikas After additional commit yesterday, no status here, can you manually trigger it please?

@mbed-ci
Copy link

mbed-ci commented Nov 15, 2017

@pan- pan- deleted the non-copyable-warning branch July 3, 2018 11:04
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.

6 participants