1298728 - Annotate some non-obvious case fallthroughs in the HTML parser.

Status: RESOLVED WONTFIX

(Core :: DOM: HTML Parser, defect)

Description

[Nicholas Nethercote [inactive]]

nsHtml5TreeBuilder has a couple of case fallthroughs that Coverity finds suspicious. We should make the intent clearer here.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Annotate some non-obvious case fallthroughs in the HTML parser

I have optimistically assumed that the current code is correct, and this patch
is just making it clearer. Please check that assumption! I'd be happy if this
had discovered a genuine bug.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: Annotate some non-obvious case fallthroughs in the HTML parser

I missed one case.

I also now see that this is a generated file. hsivonen can you please check
whether these fallthroughs are ok? If so, then I'll just mark the bug WONTFIX
and mark them as false positives in Coverity. (If not, we can work out how to
fix the Java code.)

Comment 3

[Henri Sivonen (:hsivonen)]

Comment on attachment 8785759 [details] [diff] [review]
Annotate some non-obvious case fallthroughs in the HTML parser

Indeed, this is generated code, so these annotations would be overwritten. Hence, feedback-.

All these cases are documented as comments in the Java source. We should probably change those comments to something like
// CPPONLY: fallThrough();
and then change the translator to special case a method invocation fallThrough() to emit C++ macro invocation that we could then define as expanding to whatever incantations each compiler or static analysis tool wants to see. ("// CPPONLY:" already gets removed by a preprecessor before translation, so that kind of Java comments allow Java code seen by the translator but not seen by the normal Java compiler.)

Comment 4

[Henri Sivonen (:hsivonen)]

Note that there are lots of intentional (commented in the Java source) fall-throughs in the tokenizer. Hence, https://dxr.mozilla.org/mozilla-central/source/parser/html/moz.build#104

Comment 5

[Nicholas Nethercote [inactive]]

> All these cases are documented as comments in the Java source. We should
> probably change those comments to something like
> // CPPONLY: fallThrough();
> and then change the translator to special case a method invocation
> fallThrough() to emit C++ macro invocation that we could then define as
> expanding to whatever incantations each compiler or static analysis tool
> wants to see.

I don't think that's worth the effort. I've marked the reports in Coverity as intentional. Thank you for the feedback.