Skip to content

TestPlugin: Avoid generating warnings in client codes from old style …#1131

Open
bukulin wants to merge 2 commits into
cpputest:masterfrom
bukulin:master
Open

TestPlugin: Avoid generating warnings in client codes from old style …#1131
bukulin wants to merge 2 commits into
cpputest:masterfrom
bukulin:master

Conversation

@bukulin

@bukulin bukulin commented Nov 16, 2017

Copy link
Copy Markdown
Contributor

…casts

When -Wold-style-cast is enabled

@basvodde

Copy link
Copy Markdown
Member

We avoid new casts due to old compilers. However, the failure from the old Watcom compiler is interesting. Did you look at that?

@bukulin

bukulin commented Nov 17, 2017

Copy link
Copy Markdown
Contributor Author

Shame on me! I appologize about the early pull request. I will look into it during the day.

However, I think I was not clear enough in the short description. Sorry about that. So my problem is, if I turn on the -Wold-style-cast warning flag in my project, then I get warnings about old style casts generated by the UT_PTR_SET macro.

If I understand you correctly, the acceptable solution for C++ is a function template instead of a new reinterpret_cast. Is that the correct way?

@basvodde

Copy link
Copy Markdown
Member

Uhm, no worry about early pull requests. It is good as it allows for discussion.

Templates are not good either. Because CppUTest is used in some embedded products, we try to keep the level of C++ used to be 'old' so that we support these compilers. So, in CppUTest itself, I don't think we use the 'new' C++ style casts. I would be tempted to merge it (I understand your problem) but it seems the travis build failed because of it. I don't understand very well why, but you might wanna check that first.

Some compilers might think that everything is (or could be) a
function, so they tent to interpret function calls as function
prototypes or forward declarations.

For the details see Item 6 (Be alert for C++'s most vexing parse) from
the book Effective STL by Scott Meyers
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 99.876% when pulling b34b923 on bukulin:master into 7562465 on cpputest:master.

@arstrube

Copy link
Copy Markdown
Contributor

It failed linking. There doesn't seem to be any problem with any code.

I suspect one binary perhaps grew too large. The tests are distributed over several binaries, and it is possible for one of them to break the +/- 640k limit.

@arstrube

Copy link
Copy Markdown
Contributor

Sorry. Looking at it a second time, I see a problem with the code :-(

@arstrube

Copy link
Copy Markdown
Contributor
.../CheatSheetTest.cpp(15): Error! E784: col(34) reinterpret_cast expression must be pointer or integral object
.../CheatSheetTest.cpp(15): Note! N630: col(34) source conversion type is 'void (* * )( void )'
.../CheatSheetTest.cpp(15): Note! N631: col(34) target conversion type is 'void * *'
Error: Compiler returned a bad status compiling '/home/travis/build/cpputest/cpputest/tests/CppUTest/CheatSheetTest.cpp'

@bukulin

bukulin commented Nov 17, 2017

Copy link
Copy Markdown
Contributor Author

I had an assumption about a vexing parse, but not that is the situation. I think, maybe the Watcom compiler handles pointers to functions and pointers to variables slightly differently.
Unfortunately I do not have any experience with the Watcom compiler, but I try to figure it out.

@arstrube

Copy link
Copy Markdown
Contributor

I wish I could help you there. But even though it was me who added the Watcom stuff (to be able to test some form of 16 bit code), much of my work with Watcom was by trial and error.

@basvodde

Copy link
Copy Markdown
Member

I'm surprised the compiler has support for reinterpret_cast :)

@arstrube

Copy link
Copy Markdown
Contributor

An old architecture, but the compiler has been kept up-to-date ;-)

@basvodde

Copy link
Copy Markdown
Member

Oh, has it. We'll then need to add a really old compiler to the build too.

@arstrube

Copy link
Copy Markdown
Contributor

Perhaps we need to redefine "old" ?

@arstrube

Copy link
Copy Markdown
Contributor

Sometimes companies are required to be able to build firmware from, say, 20 years ago. They have kept the old code. They even have a copy of the old build tools archived somewhere. But sadly, they have no more machine to run it on... And this is really happening! :-)

@Andne

Andne commented Nov 20, 2017

Copy link
Copy Markdown
Contributor

I know what you mean, have a VM at work to run one of our old compilers on since the installer can't even start on most of our computers anymore. We're still using that compiler on active projects too unfortunately, don't know when it will truly become retired.

Even if someone resurrects an old project with an older compiler, wouldn't they be trying to use the version of CppUTest that was around back then? In my opinion, the unit test framework is effectively one of the build tools. I understand maintaining compatibility, but seems like that eventually becomes too much of a burden and the supported environment should be updated to be one that can support newer functionality.

Specific to this issue, I do like the new-style casts myself, the different ones makes it a bit easier to avoid accidental effects by ensuring that you can only perform certain types of casting within each one.

@bukulin

bukulin commented Nov 21, 2017

Copy link
Copy Markdown
Contributor Author

Actually, I don't see any other solution for the problem then a big, fat chunk of #ifdef-s, but I do not like that at all. Do you have any other idea?

@basvodde

basvodde commented Dec 6, 2017

Copy link
Copy Markdown
Member

@bukulin An #if might work, especially for public interfaces.

However, it would be good to first figure out the Watcom thing. If it is due to no support for reinterpretted_cast then I guess it would be a good way to check. But I'm not quite sure why the Watcom compiler is failing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants