Closed Bug 1398785 Opened 3 years ago Closed 3 years ago

[Static Analysis][Coverity] Kill structurally dead code in NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_THIS_BEGIN

Categories

(Firefox Build System :: Source Code Analysis, defect)

defect
Not set

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: andi, Assigned: andi)

References

(Blocks 1 open bug)

Details

(Keywords: coverity)

Attachments

(1 file)

Suppress structurally dead code generated by Coverity in NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_THIS_BEGIN, by modifying the modelling file.
did you try if it is working?
(In reply to Sylvestre Ledru [:sylvestre] from comment #2)
> did you try if it is working?

It eliminated one issue 2 month ago. But i haven't updated the version from m-c.
Comment on attachment 8906929 [details]
Bug 1398785 - Kill structurally dead code in NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_THIS_BEGIN generated by Coverity.

https://reviewboard.mozilla.org/r/178668/#review183666

::: tools/coverity/model.cpp:63
(Diff revision 1)
> +#define NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_THIS_BEGIN(_class)                   \
> +  NS_IMETHODIMP_(bool)                                                         \
> +  NS_CYCLE_COLLECTION_CLASSNAME(_class)::CanSkipThisReal(void* p)              \
> +  {                                                                            \
> +    __coverity_panic__();                                                      \
> +    _class* tmp = DowncastCCParticipant<_class>(p);

What will happen if this changes in the base code?
I am concerned that we might miss changes here, don't you think?
(In reply to Sylvestre Ledru [:sylvestre] from comment #4)
> Comment on attachment 8906929 [details]
> Bug 1398785 - Kill structurally dead code in
> NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_THIS_BEGIN generated by Coverity.
> 
> https://reviewboard.mozilla.org/r/178668/#review183666
> 
> ::: tools/coverity/model.cpp:63
> (Diff revision 1)
> > +#define NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_THIS_BEGIN(_class)                   \
> > +  NS_IMETHODIMP_(bool)                                                         \
> > +  NS_CYCLE_COLLECTION_CLASSNAME(_class)::CanSkipThisReal(void* p)              \
> > +  {                                                                            \
> > +    __coverity_panic__();                                                      \
> > +    _class* tmp = DowncastCCParticipant<_class>(p);
> 
> What will happen if this changes in the base code?
> I am concerned that we might miss changes here, don't you think?
I don't think that the current state of the code will change in the near to medium feature since the definition above is part of a meta-data that defines a member function header, and if it changes it would mean that a major refactor will be done to the code.
Let's say that this will change, in this case the only thing that Coverity will ignore is the branch of the definition skipping thus the analysis of the rest of the body.
Comment on attachment 8906929 [details]
Bug 1398785 - Kill structurally dead code in NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_THIS_BEGIN generated by Coverity.

https://reviewboard.mozilla.org/r/178668/#review183674

OK, let's try then!
Attachment #8906929 - Flags: review?(sledru) → review+
Pushed by bpostelnicu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c4cedbf13eb8
Kill structurally dead code in NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_THIS_BEGIN generated by Coverity. r=sylvestre
https://hg.mozilla.org/mozilla-central/rev/c4cedbf13eb8
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.