Skip to content

Conversation

@codebytere
Copy link
Member

@codebytere codebytere commented Jun 13, 2019

Description of Change

Previously, the new warning string in ProcessEmitWarningGeneric was allocated using String::NewFromUtf8, but type and warning, despite also being const char*, were instead allocated using String::NewFromOneByte. I couldn't find a semantic reason for using differing string allocation functions, so this PR improves consistency by using String::NewFromUtf8 for all three.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 13, 2019

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jun 13, 2019
@codebytere codebytere force-pushed the clean-processemitgeneric branch from 38fef02 to 29355d6 Compare June 13, 2019 05:43
@bnoordhuis
Copy link
Member

It's because while the warning is an arbitrary string (in theory - in practice there are just a handful), type and code are fixed ASCII strings.

String::NewFromOneByte() is a little faster than String::NewFromUtf8() because it doesn't attempt to decode UTF-8.

@codebytere codebytere closed this Jun 14, 2019
@codebytere codebytere deleted the clean-processemitgeneric branch June 14, 2019 15:21
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++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants