Skip to content

src: general C++ cleanup in node_url.cc#19598

Closed
addaleax wants to merge 2 commits intonodejs:masterfrom
addaleax:url-refactor
Closed

src: general C++ cleanup in node_url.cc#19598
addaleax wants to merge 2 commits intonodejs:masterfrom
addaleax:url-refactor

Conversation

@addaleax
Copy link
Member

  • Merge domain and opaque storage in URL parser:

    This just simplifies the code a bit, having multiple fields
    in an union with the same type is usually just overhead.

  • Add move variant of URLHost::ToString():

    This helps avoid unnecessary string copy operations, especially
    since we control the lifetime of URLHost objects pretty well.

  • Use const refs in node_url.cc where appropriate

  • Remove or reduce overly generous .reserve() calls:

    These would otherwise keep a lot of unused memory lying around.

  • Return return values instead of unnecessary pointer arguments

  • Use more common/expressive variable names

  • Avoid macro use, reduce number of unnecessary JS strings:

    There’s no reason for GET, GET_AND_SET and UTF8STRING to be
    macros. Also, GET would previously create a JS string instance
    for each single call, even though the strings it was called
    with were compile-time constants.

  • Avoid unnecessary JS casts when the type of a value is known

  • Avoid (commonly unnecessary) copy for whitespace stripping

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. whatwg-url Issues and PRs related to the WHATWG URL implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants