Fix #44 to Warn if Integer Would Lose Precision #695
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Correctly reports E0179 when returning an html tag on a new line. Added unit tests as well.
Co-authored-by: strager <strager.nds@gmail.com>
Fixes the nitpicks strager had and is ready to merge.
A first try at fixing quick-lint#44. Issues with the code include it only working with integers written in decimal and using multiple type conversions.
|
Oops, let me fix this first. |
strager
requested changes
Apr 20, 2022
There was a problem hiding this comment.
Must fix: Fix merge conflicts. You'll need to run ./tools/update-translator-sources, but that script is broken (my fault). I'll fix it ASAP.
Must fix: Your code causes quick-lint-js to crash given the following input:
999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999
I fixed the script in commit aa889de (which is on master now). |
Co-authored-by: strager <strager.nds@gmail.com>
Co-authored-by: strager <strager.nds@gmail.com>
Co-authored-by: strager <strager.nds@gmail.com>
Now using snprintf to write to a stack-allocated buffer. Renamed error to diag in warnings.
strager
requested changes
Apr 26, 2022
Co-authored-by: strager <strager.nds@gmail.com>
Co-authored-by: strager <strager.nds@gmail.com>
Co-authored-by: strager <strager.nds@gmail.com>
Co-authored-by: strager <strager.nds@gmail.com>
Co-authored-by: strager <strager.nds@gmail.com>
You can define them in the function which uses them.
I assume you mean compiler warnings and code formatting. Compiler warnings: You'd need to build with a bunch of different compilers to get full coverage. I don't bother doing this locally; I have CI do it. If CI reports an issue, and it's not obvious how to fix it, I build locally with that compiler. (Sometimes Docker helps.) Code formatting: See https://github.com/quick-lint/quick-lint-js/blob/a4f2dea1827d6a5bac15db62582366d3a2133a7c/docs/CONTRIBUTING.md#clang-format |
|
You can add |
I'm getting some weird errors and the tests I wrote are failing. Also, what's the best way to style the super long strings?
commit a4f2dea Author: Matthew "strager" Glazar <strager.nds@gmail.com> Date: Thu Apr 28 02:33:42 2022 -0700 refactor(chocolatey): remove extra whitespace commit f9e8e87 Author: Matthew "strager" Glazar <strager.nds@gmail.com> Date: Wed Apr 27 17:10:04 2022 -0700 feat(debian): create empty changelog for downstream contribution Create a changelog with only one entry, per feedback from bage@debian.org: > The changelog must have one entry for the initial version that closes > your ITP. > It has to have a "-1" revision. Get rid of all old versions because > they have never been in the Debian archive. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1010040#12 commit ced0288 Author: Matthew "strager" Glazar <strager.nds@gmail.com> Date: Wed Apr 27 11:52:23 2022 -0700 fix(website): fix < in error list The error list (https://quick-lint-js.com/errors/) shows the titles of error files from Markdown. Some titles contain HTML or other Markdown, causing titles such as the following to be shown to the user: * E0187: mismatched JSX tags; expected </foo> * E0195: missing parentheses around operand of `typeof`; `typeof` operator cannot be used before `**` without parentheses Fix the parsing of HTML and Markdown by HTMLifying the titles before storing into titleErrorDescription. Rename titleErrorDescription to titleErrorDescriptionHTML to make it clearer that it now stores HTML. This commit should not affect individual error pages which already HTMLified the title. commit cb67a02 Author: Matthew "strager" Glazar <strager.nds@gmail.com> Date: Wed Apr 27 11:47:11 2022 -0700 refactor(website): stop tracking title for non-title nodes The 'inTitle' variable is never set to false, so we keep accumulating data into 'currentBlock', even though 'currentBlock' won't be referenced. Avoid needless string concatenations by setting 'inTitle' to false when 'currentBlock' won't be used anymore. commit 13e66b0 Author: Matthew "strager" Glazar <strager.nds@gmail.com> Date: Wed Apr 27 01:42:26 2022 -0700 refactor(website): remove unused check for /*TODO*/ This /* TODO */ comment check is dead code. Delete it. commit 8841c72 Author: Matthew "strager" Glazar <strager.nds@gmail.com> Date: Wed Apr 27 01:40:58 2022 -0700 chore(docs): update changelog commit ab055fb Author: Matthew "strager" Glazar <strager.nds@gmail.com> Date: Wed Apr 27 01:36:14 2022 -0700 refactor(website): add 'Async' to name of async functions commit 24fdcd5 Author: Matthew "strager" Glazar <strager.nds@gmail.com> Date: Wed Apr 27 00:52:46 2022 -0700 refactor: fix line endings in char8-debug.cpp DOS (CRLF) -> UNIX (LF) commit 0d2d7bb Author: Matthew "strager" Glazar <strager.nds@gmail.com> Date: Wed Apr 27 00:51:47 2022 -0700 refactor: simplify char8.cpp implementation Remove duplicate implementations by disabling warnings about useless casts. commit 5c19e4e Author: Nico Sonack <nsonack@herrhotzenplotz.de> Date: Mon Apr 25 16:17:55 2022 +0200 tools: Add translation testing tool This still contains a few bugs as it doesn't scrape all the files correctly. This will be fixed in the future. commit dcc5f82 Author: Nico Sonack <nsonack@herrhotzenplotz.de> Date: Mon Apr 25 16:03:46 2022 +0200 po: Update German translations - Update po/de.po - Regenerate translation tables commit 563cee5 Author: Matthew "strager" Glazar <strager.nds@gmail.com> Date: Tue Apr 26 00:55:13 2022 -0700 refactor: inline is_arrow_kind helper function commit 576ade9 Author: Matthew Glazar <strager.nds@gmail.com> Date: Mon Apr 25 16:52:39 2022 -0700 feat(website): document installing with winget commit 4930cfc Author: Matthew Glazar <strager.nds@gmail.com> Date: Mon Apr 25 16:45:31 2022 -0700 fix(website): fix vertical alignment of breadcrumbs commit 00d5f12 Author: Matthew Glazar <strager.nds@gmail.com> Date: Mon Apr 25 16:42:18 2022 -0700 refactor(website): deduplicate breadcrumb HTML commit 9ada865 Author: Matthew Glazar <strager.nds@gmail.com> Date: Mon Apr 25 16:34:42 2022 -0700 fix(website): fix build on Windows 'yarn build' fails on Windows with the following error: Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only file and data URLs are supported by the default ESM loader. On Windows, absolute paths must be valid file:// URLs. Received protocol 'c:' This is because we are passing a file path to 'import()', which expects a URL. Give 'import()' a URL instead of a path, fixing the error and making 'yarn build' succeed. commit 8239f0b Author: Matthew Glazar <strager.nds@gmail.com> Date: Mon Apr 25 16:28:14 2022 -0700 fix(website): fix loading dusty.svg on Windows On Windows, dusty.svg is a symbolic link pointing to another file. In my Git checkout, the core.symlinks Git config is set to false, making Git represent symbolic links as text files containing the target's path, not as a copy or as a Windows link. Work around Git's symlink behavior by following the symlink manually if needed.
strager
reviewed
Apr 28, 2022
|
Here's the test failure I get on my machine: The second (actual) array is identical to the first (expected) array, except the second array has the following extra items at the end: U+0020, U+003E, U+0017, U+0001 Perhaps the problem is that we're reading uninitialized memory? |
|
I ran the test with GCC's ASAN ( |
|
I think I see the bug: char8* rounded_val =
this->allocator_.allocate_uninitialized_array<char8>(
result_string_view.size());
std::copy(result_string_view.begin(), result_string_view.end(),
rounded_val);
this->diag_reporter_->report(diag_number_literal_will_lose_precision{
.characters = source_code_span(number_begin, input),
.rounded_val = rounded_val,
});
I think you should fix this by creating a |
Co-authored-by: strager <strager.nds@gmail.com>
macOS ClangSolution: Remove the parameter names. E.g. check error docsLook at |
strager
approved these changes
Apr 30, 2022
Co-authored-by: strager <strager.nds@gmail.com>
Co-authored-by: strager <strager.nds@gmail.com>
Co-authored-by: strager <strager.nds@gmail.com>
Co-authored-by: strager <strager.nds@gmail.com>
Co-authored-by: strager <strager.nds@gmail.com>
robminer6
changed the title
|
Ready to merge! |
strager
reviewed
May 1, 2022
|
Ready to merge again if the new comments look good. |
strager
reviewed
May 1, 2022
Comment on lines
+1303
to
+1304
| // equal to 2^1023 × (1 + (1 − 2^−52)) ≈ 1.7976931348623157 × 10^308, which is | ||
| // 309 digits long. |
|
Landed as commit eb7418e. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request implements a new warning which warns if an integer literal is used which cannot be accurately represented by JavaScript's "Number" type and would lose precision. This is done by checking the number of digits in the number and using std::stold. Future implementations could expand this to warn about decimal numbers and numbers in binary/octal/hexadecimal.