search

legatoproject / legato-af

Legato Application Framework
Mozilla Public License 2.0
153 stars 118 forks source link

remove c++ "namespace" keyword out from API #31

Closed tatraian closed 6 years ago

tatraian commented 6 years ago

I tested. And my C++ application could be compiled. Since I didn't wrote my C++ app, I just tested with assetData example. And that worked with this modification. As a solution for issue: #30

CoRfr commented 6 years ago

Could you fill the https://github.com/legatoproject/legato-af/blob/master/CONTRIB_INDIVIDUAL.md or https://github.com/legatoproject/legato-af/blob/master/CONTRIB_COMPANY.md before we can accept your contribution (if that's not already done)?

More info at https://legato.io/legato-docs/latest/aboutLegatoContributing.html

Thanks!

tatraian commented 6 years ago

I filled and sent.

tatraian commented 6 years ago

Hello,

Ok! In this case the original solution should be dropped (parameter renaming) due to parameter selection. I'm not expert of this code, but I think the other solution can be implemented in C's api generation lib (codeGenHelpers.py). FormatParameterName function should return a modified name (parameter.name + "_kw" if parameter.name in keywords. Somewhere here: https://github.com/legatoproject/legato-af/blob/79ffa4bda300aeabf9280d5113168c9f418ade6d/framework/tools/ifgen/langC/codeGenHelpers.py#L86

Or if you have better idea please, share.

CoRfr commented 6 years ago

@kdunwoody has a provision that relies on mangling the parameter name (prepending with a _ to be precise) in case it matches a list of keywords. That would address the issue without updating the .api, and that should make it for 18.05.

tatraian commented 6 years ago

Ok! Than I wait for the next release. I close this PR and you can close the bug if the release is out.

CoRfr commented 6 years ago

This change should address the issue: https://github.com/legatoproject/legato-af/commit/d3803ba46e57ccfe22a3b47d4e5750ea9c280aae

I also have an associated test that is in review internally.