Closed
Bug 1424548
Opened 7 years ago
Closed 7 years ago
Regenerate HTML parser code to redo formatting and add MOZ_FALLTHROUGH and override annotations
Categories
(Core :: DOM: HTML Parser, enhancement, P3)
Core
DOM: HTML Parser
Tracking
()
People
(Reporter: cpeterson, Assigned: cpeterson)
References
Details
Attachments
(14 files)
(deleted),
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
Bug 1400460 renamed nsIAtom to nsAtom in the generated parser code (and reformatted it) without updating the Java translator. Bug 500617 removed always-zero offsets in the generated parser code, but missed one in CPPONLY code in a parser Java file. Bug 1236322 suppressed clang -Wimplicit-fallthrough warnings in the generated parser code because it did not include MOZ_FALLTHROUGH annotations.
Assignee | ||
Comment 1•7 years ago
|
||
Assignee | ||
Comment 2•7 years ago
|
||
The NS_ABORT_IF_FALSE macro no longer exists.
Attachment #8936092 -
Flags: review?(hsivonen)
Assignee | ||
Comment 3•7 years ago
|
||
Comment on attachment 8936091 [details] [diff] [review] Part 1: Fix non-unified build of HTML parser code Part 1: Fix non-unified build of HTML parser code.
Attachment #8936091 -
Flags: review?(hsivonen)
Assignee | ||
Comment 5•7 years ago
|
||
#include "jArray.h" #include "nscore.h" #include "nsDebug.h" #include "mozilla/Logging.h" #include "nsMemory.h"
Attachment #8936094 -
Flags: review?(hsivonen)
Assignee | ||
Comment 9•7 years ago
|
||
This format matches the modified generated HTML parser code and Mozilla's current C++ style guidelines. https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style
Attachment #8936098 -
Flags: review?(hsivonen)
Assignee | ||
Comment 10•7 years ago
|
||
And remove always-zero offset from CPPONLY code block bug 500617
Attachment #8936099 -
Flags: review?(hsivonen)
Assignee | ||
Comment 12•7 years ago
|
||
And stop suppressing -Wimplicit-fallthrough warnings. We no longer need to suppress these clang warnings because the generated parser code now includes MOZ_FALLTHROUGH annotations.
Attachment #8936101 -
Flags: review?(hsivonen)
Assignee | ||
Comment 14•7 years ago
|
||
We convert Java @Override annotations to C++ override specifiers in generated HTML parser code. Overridden virtual functions without override specifiers will become errors if/when I enable gcc -Wsuggest-override warnings.
Attachment #8936103 -
Flags: review?(hsivonen)
Assignee | ||
Comment 15•7 years ago
|
||
Overridden virtual functions without override specifiers will become errors if/when I enable gcc -Wsuggest-override warnings.
Attachment #8936104 -
Flags: review?(hsivonen)
Attachment #8936092 -
Flags: review?(hsivonen) → review+
Attachment #8936093 -
Flags: review?(hsivonen) → review+
Attachment #8936094 -
Flags: review?(hsivonen) → review+
Attachment #8936095 -
Flags: review?(hsivonen) → review+
Attachment #8936096 -
Flags: review?(hsivonen) → review+
Attachment #8936097 -
Flags: review?(hsivonen) → review+
Comment on attachment 8936098 [details] [diff] [review] Part 8: Reformat C++ constructor initializer lists Review of attachment 8936098 [details] [diff] [review]: ----------------------------------------------------------------- Running clang-format afterwards overwrites anyway.
Attachment #8936098 -
Flags: review?(hsivonen) → review+
Comment on attachment 8936099 [details] [diff] [review] Part 9: Regenerate HTML parser code to undo formatting changes to made the generated code directly Review of attachment 8936099 [details] [diff] [review]: ----------------------------------------------------------------- r+ provided that you run ./mach clang-format afterwards.
Attachment #8936099 -
Flags: review?(hsivonen) → review+
Attachment #8936100 -
Flags: review?(hsivonen) → review+
Attachment #8936101 -
Flags: review?(hsivonen) → review+
Attachment #8936102 -
Flags: review?(hsivonen) → review+
Attachment #8936103 -
Flags: review?(hsivonen) → review+
Attachment #8936104 -
Flags: review?(hsivonen) → review+
Comment on attachment 8936091 [details] [diff] [review] Part 1: Fix non-unified build of HTML parser code Review of attachment 8936091 [details] [diff] [review]: ----------------------------------------------------------------- ::: parser/html/moz.build @@ +53,5 @@ > 'nsIContentHandle.h', > 'nsParserUtils.h', > ] > > +SOURCES += [ What's the reason not to keep these unified?
Assignee | ||
Comment 19•7 years ago
|
||
(In reply to Henri Sivonen (:hsivonen) from comment #17) > Part 9: Regenerate HTML parser code to undo formatting changes to made the > generated code directly ... > r+ provided that you run ./mach clang-format afterwards. OK. I'll run ./mach clang-format before checking in. > > +SOURCES += [ > > What's the reason not to keep these unified? Oops! That's a bug. I meant to change UNIFIED_SOURCES to SOURCES only temporarily to find the non-unified build problems.
Comment on attachment 8936091 [details] [diff] [review] Part 1: Fix non-unified build of HTML parser code Review of attachment 8936091 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Chris Peterson [:cpeterson] from comment #19) > Oops! That's a bug. I meant to change UNIFIED_SOURCES to SOURCES only > temporarily to find the non-unified build problems. I see. r+ provided that you change it back to UNIFIED_SOURCES.
Attachment #8936091 -
Flags: review?(hsivonen) → review+
Comment 21•7 years ago
|
||
Pushed by cpeterson@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7516d49fc731 Part 1: Fix non-unified build of HTML parser code. r=hsivonen https://hg.mozilla.org/integration/mozilla-inbound/rev/9874df8448d8 Part 9a: clang-format generated HTML parser code. r=hsivonen https://hg.mozilla.org/integration/mozilla-inbound/rev/091d01097937 Part 9b: Regenerate HTML parser code to remove always-zero offset from CPPONLY code. r=hsivonen https://hg.mozilla.org/integration/mozilla-inbound/rev/7ea2b1f67e08 Part 11: Regenerate HTML parser code with MOZ_FALLTHROUGH annotations. r=hsivonen https://hg.mozilla.org/integration/mozilla-inbound/rev/1fcbcdfba8e5 Part 14: Regenerate HTML parser code with override specifiers. r=hsivonen
Assignee | ||
Comment 22•7 years ago
|
||
Henri, does pushing to the https://hg.mozilla.org/projects/htmlparser repo require different permissions than pushing to inbound? I have Commit Access Level 3 (Core Product Access). I can successfully `hg outgoing` with the htmlparser repo, but `hg push` fails with "abort: authorization failed": $ hg push --verbose pushing to https://hg.mozilla.org/projects/htmlparser searching for changes 10 changesets found uncompressed size of bundle content: 5433 (changelog) 2868 (manifests) 1026 src/nu/validator/htmlparser/impl/MetaScanner.java 527 src/nu/validator/htmlparser/impl/StateSnapshot.java 8655 src/nu/validator/htmlparser/impl/Tokenizer.java 2291 src/nu/validator/htmlparser/impl/TreeBuilder.java 288 translator-src/nu/validator/htmlparser/cpptranslate/AnnotationHelperVisitor.java 1126 translator-src/nu/validator/htmlparser/cpptranslate/CppTypes.java 827 translator-src/nu/validator/htmlparser/cpptranslate/CppVisitor.java 326 translator-src/nu/validator/htmlparser/generator/GenerateNamedCharactersCpp.java abort: authorization failed
Flags: needinfo?(hsivonen)
Keywords: leave-open
Comment 23•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7516d49fc731 https://hg.mozilla.org/mozilla-central/rev/9874df8448d8 https://hg.mozilla.org/mozilla-central/rev/091d01097937 https://hg.mozilla.org/mozilla-central/rev/7ea2b1f67e08 https://hg.mozilla.org/mozilla-central/rev/1fcbcdfba8e5
(In reply to Chris Peterson [:cpeterson] from comment #22) > Henri, does pushing to the https://hg.mozilla.org/projects/htmlparser repo > require different permissions than pushing to inbound? I thought it didn't, but since the server doesn't allow you or Jessica to push, apparently there's something I don't know. Filed bug 1427696.
Flags: needinfo?(hsivonen)
Assignee | ||
Comment 25•7 years ago
|
||
Resolving fixed because I finally landed the Java htmlparser translator changes: https://hg.mozilla.org/projects/htmlparser/rev/95d2e02251df https://hg.mozilla.org/projects/htmlparser/rev/afa68d0380ed https://hg.mozilla.org/projects/htmlparser/rev/79a997f5156f https://hg.mozilla.org/projects/htmlparser/rev/93bff4c75f39 https://hg.mozilla.org/projects/htmlparser/rev/83ed0725ed47 https://hg.mozilla.org/projects/htmlparser/rev/63de2bd615b5 https://hg.mozilla.org/projects/htmlparser/rev/219397dfab48 https://hg.mozilla.org/projects/htmlparser/rev/62abdd11d044 https://hg.mozilla.org/projects/htmlparser/rev/cfd1e575d398 https://hg.mozilla.org/projects/htmlparser/rev/f61ddeeb235d https://hg.mozilla.org/projects/htmlparser/rev/1e8c5d1b6096
Keywords: leave-open
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•5 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•