New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Spelling #9005
base: master
Are you sure you want to change the base?
Spelling #9005
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checkpointing notes.
The initial set of suggestions were made by Google Sheets, but all fault is mine. Some of the corrections are wrong. I'm happy to drop things/split things/...
hphp/runtime/ext/CMakeLists.txt
Outdated
set(HRE_LIBARIES) | ||
set(HRE_LIBRARIES) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Afaict this never worked.
I believe it was trying to talk to:
https://github.com/facebook/hhvm/blob/b9eefdbe5550c8f50e280dce9186b8745125fef6/CMake/EXTFunctions.cmake?q=owner%3Acheck-spelling+HRE_LIBRARIES#L9
CMake/FindlibWatchmanClient.cmake
Outdated
@@ -1,4 +1,4 @@ | |||
# Finds Watchamn C++ client library | |||
# Finds Watchman C++ client library |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a brand (I probably won't flag many of these specifically)
* Therefore, a package is really toppest entry point for parsing. | ||
* Therefore, a package is really topmost entry point for parsing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm hoping this is ok, the replacement is a word, and usually that should be the right thing...
* SP relative offset of the firstin the inlined call. | ||
* SP relative offset of the first instruction in the inlined call. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very manual intervention
* backtracing. They are currently separated out to avoid pessimizing the loads | ||
* backtracking. They are currently separated out to avoid pessimizing the loads |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quite possibly wrong.
void ProfData::addTransProfPrologue(TransID transID, SrcKey sk, int nArgs, | ||
uint32_t asmSize) { | ||
m_proflogueDB.emplace(PrologueID{sk.funcID(), nArgs}, transID); | ||
m_prologueDB.emplace(PrologueID{sk.funcID(), nArgs}, transID); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This argues I'm at least partially wrong.
I'm not sure what a proflogue
could be. If there were Prologue
and ProfLog
, it'd be a bit easier to understand they are intentionally unrelated items.
* arguments. (kInvalidTransID|nullptr) is returned if the prologue is not | ||
* associated with a TransID. | ||
*/ | ||
TransID proflogueTransId(const Func* func, int nArgs) const; | ||
TransID prologueTransId(const Func* func, int nArgs) const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like the best argument against proflogue
. It's really rare in English to randomly insert an f
into the middle of a word....
* After running this pass, the caller must run a manditoryDCE to restore IR | ||
* After running this pass, the caller must run a mandatoryDCE to restore IR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This supports my DCE
case change earlier.
// the JIT running in jumpstart seeer mode, don't consider retranslateAll to | ||
// the JIT running in jumpstart serdes mode, don't consider retranslateAll to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was strange. (It definitely wasn't automatic.)
hphp/runtime/vm/jit/vasm-util.cpp
Outdated
* This SSA restoration algorithm is based on "Simple and Efficent Construction | ||
* This SSA restoration algorithm is based on "Simple and Efficient Construction | ||
* of Static Single Assignment Form" by Matthias Braun, Sebastian Buchwald, | ||
* Sebastian Hack, et all (with a few tweaks). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the proper title of the book...
// ExecutionContext and the TC may retain references to Classes, so | ||
// it is possible for Classes to outlive their Unit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Class'es
would never be right.
"bytecode translationc event type. Pass '%s' to get a " | ||
"bytecode translation event type. Pass '%s' to get a " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
things like this tend to worry me
// Enable backtracing through PHP frames (t9814472). | ||
// Enable backtracking through PHP frames (t9814472). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably wrong
nullptr, nullptr, nullptr, nullptr, "there4", "becaus", nullptr, nullptr, | ||
nullptr, nullptr, nullptr, nullptr, "there4", "because", nullptr, nullptr, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is becaus
a thing?
@Atry has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
We might not want to make API changes even if it's a typo. |
Because there are many risky changes, like change of API and constant values, this PR can hardly pass CI unless excluding all the risky changes or splitting into small PRs. |
Happy to split, just provide a recommendation of how. I'm used to doing such changes over long periods of time. I could for instance try to only change Docs in one PR, or comments in one PR, or locals in one PR. Alternatively, I could try to change things on a per directory basis. |
@jsoref has updated the pull request. You must reimport the pull request before landing. |
@jsoref has updated the pull request. You must reimport the pull request before landing. |
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
@jsoref has updated the pull request. You must reimport the pull request before landing. |
Summary: This is a subset of #9005, it should be easier to review/test/clear Note: while the top level item is `hphp/util`, if a commit overlaps other directories, I'm dropping the commit. By excluding spanning commits, I reduce the risk of missing a related change (these tend to result in build/test failures which are painful) at the slight increase in additional commits for my general series. Pull Request resolved: #9070 Reviewed By: alexeyt Differential Revision: D35826622 Pulled By: Wilfred fbshipit-source-id: 847ab9dd919a85d2f0c545f3361791bce7f78ca9
Summary: This is a subset of #9005, it should be easier to review/test/clear Note: while the top level item is `hphp/runtime/base`, if a commit overlaps other directories, I'm dropping the commit. By excluding spanning commits, I reduce the risk of missing a related change (these tend to result in build/test failures which are painful) at the slight increase in additional commits for my general series. Pull Request resolved: #9071 Reviewed By: vsiles Differential Revision: D35826676 Pulled By: Wilfred fbshipit-source-id: d87d39ad34d93431f10ea8e7928d0604e66e0713
This PR corrects misspellings identified by the check-spelling action.
The misspellings have been reported at jsoref@e308e3f#commitcomment-66884198
The action reports that the changes in this PR would make it happy: jsoref@dc6a358
Note: this PR does not include the action. If you're interested in running a spell check on every PR and push, that can be offered separately.
I've annotated this PR. I'm happy to drop changes to files, drop specific terms, or split things as requested, although this may take some time.
I normally take efforts to exclude third party directories, but for various reasons I didn't here. I'm quite happy to drop any particular files / directories, just let me know where they live. Also, if there's an upstream for them, I'm generally happy to queue fixes for them.