cpp:S1242: 'override' methods wrongly reported as hidding their super-implementation

Hello,

we have 26 hits for the rule cpp:S1242 in SimGrid, but all hits I’ve checked seem to be false positives to me. For example in https://sonarcloud.io/project/issues?id=simgrid_simgrid&issues=AWsDvTpPezZPqKif7EEp&open=AWsDvTpPezZPqKif7EEp the flagged method is actually marked with “override”.

I am not 100% sure, but I think that we are doing exactly what the rule is advising, aren’t we?

Thanks for the fish,
Mt

Hello,

You are right that this code is correct and should not trigger this rule. The reason of this false positive is that the code uses covariant return types for the virtual function, which is a case we forgot to take into account.

I created the ticket https://jira.sonarsource.com/browse/CPP-2227 to track this issue.

It seems that your detection also fails when the methods have a different attribute. For example in https://sonarcloud.io/project/issues?id=simgrid_simgrid&issues=AWsDvTo6ezZPqKif7EEL&open=AWsDvTo6ezZPqKif7EEL we override the symbol of the upper class (notice that we do give the ‘override’ keyword), with __attribute__((deprecated(mesg))). And it raises your detection.

The funny thing is that the overriden symbol is also marked as deprecated, but with a differing message. Maybe that’s where your bug lies: it could be interesting if sonar detects when the overring methods change the attributes (not sure), but it should not complain when the deprecation messge changes. That’s too picky.

I have the feeling that the very same rule fails in a third way on our code. https://sonarcloud.io/project/issues?id=simgrid_simgrid&issues=AWsDvTd7ezZPqKif7D75&open=AWsDvTd7ezZPqKif7D75

Here, the methods don’t have the same amount of parameters, so I’m a bit puzzled.

Hello @mquinson,

I think your second report is probably linked to the first one: There is in Io a wait function that is an override of a function in the base class, but with covariant return type.

For your third case, it looks strange to me. That we report a function with a different number of parameters is normal: Even with a different number of parameters, the function in a derived class hides the function in the base class, which makes for some weird code. But in the case you highlight, there is a using declaration (using AddressSpace::read_string) that imports in the derived class all the names of the base class, so no hiding is supposed to happen. We’ll have a look at it too.