Skip to content

Fix old-style function definitions. #1651

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

Merged

Conversation

tilmannOSG
Copy link

Function definitions with no parameters should always use the void keyword to allow the compiler to catch invalid calls of those functions at compile time.

Fixes #1649.

JerryScript-DCO-1.0-Signed-off-by: Tilmann Scheller [email protected]

Copy link
Member

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

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

LGTM

@robertsipka
Copy link
Contributor

robertsipka commented Mar 9, 2017

Could you add the old-style-definition to the jerry_add_compile_warnings macro as a parameter in the CMakeLists.txt? So these warnings will be treated as an error with gcc compiler. What's your opinion?

@LaszloLango LaszloLango added the bug Undesired behaviour label Mar 9, 2017
@tilmannOSG tilmannOSG force-pushed the fix-old-style-definitions branch from 19f0789 to 3d9d51d Compare March 9, 2017 10:57
@tilmannOSG
Copy link
Author

@robertsipka Yeah that's a good idea, I've updated the patch accordingly.

Function definitions with no parameters should always use the void keyword to allow the compiler to catch invalid calls of those functions at compile time.

Enable -Wold-style-definition to catch this early in the future.

Fixes jerryscript-project#1649.

JerryScript-DCO-1.0-Signed-off-by: Tilmann Scheller [email protected]
@tilmannOSG tilmannOSG force-pushed the fix-old-style-definitions branch from 3d9d51d to e99e41c Compare March 9, 2017 11:53
@robertsipka
Copy link
Contributor

LGTM (informally)

Copy link
Contributor

@LaszloLango LaszloLango left a comment

Choose a reason for hiding this comment

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

LGTM

@tilmannOSG tilmannOSG merged commit f8dd54a into jerryscript-project:master Mar 9, 2017
@tilmannOSG tilmannOSG deleted the fix-old-style-definitions branch March 9, 2017 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Undesired behaviour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants