Sync htmlparser changes from g3 by amaltas · Pull Request #40261 · ampproject/amphtml

amaltas · 2025-03-28T13:12:51Z

This is an automatically generated pull request.

PiperOrigin-RevId: 540325424

PiperOrigin-RevId: 543822678

PiperOrigin-RevId: 555248479

PiperOrigin-RevId: 555259013

PiperOrigin-RevId: 557610815

PiperOrigin-RevId: 558192832

PiperOrigin-RevId: 582334672

PiperOrigin-RevId: 598876608

PiperOrigin-RevId: 604327651

PiperOrigin-RevId: 604352258

PiperOrigin-RevId: 604359636

PiperOrigin-RevId: 610804847

…e names starting with an equals sign ("="). PiperOrigin-RevId: 610820873

…ad of ABSL's macro. PiperOrigin-RevId: 610875973

…es with duplicate attribute names. PiperOrigin-RevId: 614818595

PiperOrigin-RevId: 617936431

PiperOrigin-RevId: 639907691

PiperOrigin-RevId: 644159337

JsonDict and JsonArray form members to a vector of an incomplete type, which is not allowed. libc++ has historically turned a blind eye here, but an incoming patch will cause these types to break. This commit intends to get in front of that libc++ change by moving all function definitions that reference `items_` to after `JsonObject` is defined. There were also a few other opportunities for reducing complexity that I snuck into this CL, such as correcting use of forward references and ensuring `types.h` and `types.cc` include what they use. PiperOrigin-RevId: 653431283

…iteration. PiperOrigin-RevId: 657262180

PiperOrigin-RevId: 691568917

PiperOrigin-RevId: 704346605

PiperOrigin-RevId: 704347005

PiperOrigin-RevId: 738015588

…re is a large amount to unescape at once, this can slow down parsing significantly. Before: Benchmark Time(ns) CPU(ns) Iterations -------------------------------------------------------------------- BM_UnescapedEntities 27208779620 27204638467 10 After: Benchmark Time(ns) CPU(ns) Iterations -------------------------------------------------------------------- BM_UnescapedEntities 57966588 57949693 10 BM_UnescapedEntities 58013403 58018622 10 BM_UnescapedEntities 57770261 57745786 10 BM_UnescapedEntities 58072494 58062626 10 BM_UnescapedEntities 57877059 57865834 10 BM_UnescapedEntities 57689735 57705073 10 BM_UnescapedEntities 57678852 57673476 10 BM_UnescapedEntities 57786888 57773828 10 BM_UnescapedEntities 58067170 58058291 10 BM_UnescapedEntities 57742296 57723050 10 BM_UnescapedEntities 57841470 57833843 10 BM_UnescapedEntities 58208952 58196325 10 BM_UnescapedEntities_mean 57892931 57883870 120 BM_UnescapedEntities_stddev 163659 162941 120 PiperOrigin-RevId: 738018062

PiperOrigin-RevId: 739988036

…vectors. Before: Benchmark Time(ns) CPU(ns) Allocs Iterations ---------------------------------------------------------------------- BM_Parse 158029720 158862020 888548 1 55.001MB peak-mem BM_Parse 149980188 151992674 888548 1 55.001MB peak-mem BM_Parse 146430455 147999431 888548 1 55.001MB peak-mem BM_Parse 146249045 147933061 888548 1 55.001MB peak-mem BM_Parse 149886603 148022291 888548 1 55.001MB peak-mem BM_Parse 145681736 143954218 888551 1 55.001MB peak-mem BM_Parse_mean 149376291 149793949 888548 6 55.001MB peak-mem BM_Parse_stddev 4235058 4672414 1.12 6 0.000B peak-mem After: Benchmark Time(ns) CPU(ns) Allocs Iterations ---------------------------------------------------------------------- BM_Parse 149049168 147654141 521765 1 51.583MB peak-mem BM_Parse 141454742 139966494 521765 1 51.583MB peak-mem BM_Parse 141263070 139947895 521765 1 51.583MB peak-mem BM_Parse 141052560 144135480 521765 1 51.583MB peak-mem BM_Parse 139456331 139946514 521765 1 51.583MB peak-mem BM_Parse 139357311 135976601 521765 1 51.583MB peak-mem BM_Parse_mean 141938864 141271188 521765 6 51.583MB peak-mem BM_Parse_stddev 3287867 3701058 0.000 6 0.000B peak-mem PiperOrigin-RevId: 740157446

…e. A recent Clang regression (caused by go/llvm-pr/114684) incorrectly allows this invalid syntax. PiperOrigin-RevId: 740231216

Before Benchmark Time(ns) CPU(ns) Allocs Iterations ---------------------------------------------------------------------- BM_Parse 151069991 151982943 521765 1 51.583MB peak-mem BM_Parse 144951470 144099227 521765 1 51.583MB peak-mem BM_Parse 156225543 155978946 521765 1 51.583MB peak-mem BM_Parse 184920674 184011830 521765 1 51.583MB peak-mem BM_Parse 144872684 144154218 521765 1 51.583MB peak-mem BM_Parse 140119083 140093764 521770 1 51.583MB peak-mem BM_Parse_mean 153693241 153386821 521766 6 51.583MB peak-mem BM_Parse_stddev 14871254 14690058 1.86 6 0.000B peak-mem After Benchmark Time(ns) CPU(ns) Allocs Iterations ---------------------------------------------------------------------- BM_Parse 153492099 152312987 440241 1 51.583MB peak-mem BM_Parse 140035079 140298303 440241 1 51.583MB peak-mem BM_Parse 138184381 136143580 440241 1 51.583MB peak-mem BM_Parse 139805091 140279666 440241 1 51.583MB peak-mem BM_Parse 136864430 136069852 440241 1 51.583MB peak-mem BM_Parse 139082381 140114802 440241 1 51.583MB peak-mem BM_Parse_mean 141243910 140869865 440241 6 51.583MB peak-mem BM_Parse_stddev 5579163 5440099 0.000 6 0.000B peak-mem PiperOrigin-RevId: 740376026

These are failing since long time and there is no active development in validator codebase.

erwinmombay · 2025-03-28T19:47:34Z

          name: '⭐⭐⭐ Bundle Size ⭐⭐⭐'
          command: node build-system/pr-check/bundle-size.js
      - teardown_vm
-  validator_tests:


@banaag can you give an LGTM on this part, or do we need it back?

@amaltas why are we turning off the validator tests? is this temporary?

Amaltas says it's just temporary, we can reactivate after submission of the PR

@amaltas instead of disabling all validator tests, can you try changing this line here

amphtml/build-system/pr-check/build-targets.js

Line 297 in 6a610ba

if (isOwnersFile(file) || file.startsWith('validator/js/webui/')) {

you can try adding file.startsWith('validator/cpp/htmlparser/'). This will at least only exclude the htmlparser changes from triggering the validator.

Unfortunately this didn't work.

erwinmombay

approving just to test the CI

erwinmombay · 2025-03-31T18:11:22Z

          name: '⭐⭐⭐ Bundle Size ⭐⭐⭐'
          command: node build-system/pr-check/bundle-size.js
      - teardown_vm
-  validator_tests:


@amaltas instead of disabling all validator tests, can you try changing this line here

amphtml/build-system/pr-check/build-targets.js

Line 297 in 6a610ba

if (isOwnersFile(file) || file.startsWith('validator/js/webui/')) {

you can try adding file.startsWith('validator/cpp/htmlparser/'). This will at least only exclude the htmlparser changes from triggering the validator.

Internal Code Change

c4d78c1

PiperOrigin-RevId: 540325424

amaltas force-pushed the validator_engine_sync branch from a403830 to c4d78c1 Compare March 28, 2025 13:18

banaag and others added 28 commits March 28, 2025 13:18

Fixed lint errors for license.

f4fd254

PiperOrigin-RevId: 543822678

Various lint fixes.

dbd5f13

PiperOrigin-RevId: 555248479

Few more lint errors/warnings fixes.

b06f8f3

PiperOrigin-RevId: 555259013

Create announce mailing list to notify important updates to clients.

4fd05a9

PiperOrigin-RevId: 557610815

Mark htmlparser Node.DebugString as const.

131a63b

PiperOrigin-RevId: 558192832

No description.

c9c47c4

PiperOrigin-RevId: 582334672

Automated Code Change

a17c5b1

PiperOrigin-RevId: 598876608

Add a missing include for std::alignment_of_v

fee2792

PiperOrigin-RevId: 604327651

Internal cleanup.

3696813

PiperOrigin-RevId: 604352258

Split htmlparser README in internal and externals.

b384f42

PiperOrigin-RevId: 604359636

Automated Code Change

60663e0

PiperOrigin-RevId: 610804847

Fix tokenization of HTML tag attributes for the edge case of attribut…

1e1f518

…e names starting with an equals sign ("="). PiperOrigin-RevId: 610820873

Remove unused dependency by using c++ attribute [[fallthrough]] inste…

dfe4bff

…ad of ABSL's macro. PiperOrigin-RevId: 610875973

Fix tokenization of HTML tag attributes for the edge case of attribut…

b1cb77b

…es with duplicate attribute names. PiperOrigin-RevId: 614818595

Fix rendering of RAWTEXT nodes.

5391e42

PiperOrigin-RevId: 617936431

Automated Code Change

0eecb00

PiperOrigin-RevId: 639907691

Internal change.

2131a4c

PiperOrigin-RevId: 644159337

Add const iterators to htmlparser::Document to allow range-based …

fb008e4

…iteration. PiperOrigin-RevId: 657262180

No description.

a6feb62

PiperOrigin-RevId: 691568917

Fix parsing of RawOrRCDATA

3303a69

PiperOrigin-RevId: 704346605

Fix end tag parsing

7a31297

PiperOrigin-RevId: 704347005

Support unescaping decimal single digit html codes.

694cafc

PiperOrigin-RevId: 738015588

Initialize Token struct token_type and atom explicitly.

fede34a

PiperOrigin-RevId: 739988036

Having a trailing comma in function call arguments is invalid C++ cod…

3b7f656

…e. A recent Clang regression (caused by go/llvm-pr/114684) incorrectly allows this invalid syntax. PiperOrigin-RevId: 740231216

amaltas added 3 commits March 28, 2025 06:34

Restore previous build from this pull request

2973255

Restoring validator-internal.cc

b697944

Update validator_test.cc

d5b4eb5

amaltas changed the title ~~Sync for validator cpp engine and cpp htmlparser~~ Sync htmlparser changes from g3 Mar 28, 2025

amaltas requested a review from banaag March 28, 2025 13:39

amaltas self-assigned this Mar 28, 2025

amaltas added 2 commits March 28, 2025 10:17

Update WORKSPACE

62a0b8b

Disable validator tests in circleci flow

7173515

These are failing since long time and there is no active development in validator codebase.

amaltas requested review from banaag and erwinmombay and removed request for banaag March 28, 2025 17:27

erwinmombay reviewed Mar 28, 2025

View reviewed changes

erwinmombay approved these changes Mar 31, 2025

View reviewed changes

erwinmombay requested changes Mar 31, 2025

View reviewed changes

amaltas added 2 commits March 31, 2025 19:15

Update config.yml

743cecb

Skip validator tests if htmlparser only changes

ea5e935

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sync htmlparser changes from g3#40261

Sync htmlparser changes from g3#40261
amaltas wants to merge 36 commits intomainfrom
validator_engine_sync

amaltas commented Mar 28, 2025

Uh oh!

erwinmombay Mar 28, 2025

Uh oh!

banaag Mar 29, 2025

Uh oh!

banaag Mar 31, 2025

Uh oh!

erwinmombay Mar 31, 2025

Uh oh!

amaltas Apr 1, 2025

Uh oh!

amaltas Apr 1, 2025

Uh oh!

erwinmombay left a comment

Uh oh!

erwinmombay Mar 31, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

amaltas commented Mar 28, 2025

Uh oh!

erwinmombay Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

banaag Mar 29, 2025

Choose a reason for hiding this comment

Uh oh!

banaag Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

erwinmombay Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

amaltas Apr 1, 2025

Choose a reason for hiding this comment

Uh oh!

amaltas Apr 1, 2025

Choose a reason for hiding this comment

Uh oh!

erwinmombay left a comment

Choose a reason for hiding this comment

Uh oh!

erwinmombay Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants