This is an archive of the discontinued LLVM Phabricator instance.

Fix dupe word typos
ClosedPublic

Authored by Rageking8 on Nov 3 2022, 7:50 AM.

Details

Summary

This revision fixes typos where there are 2 consecutive words which are duplicated.
There should be no code changes in this revision (only changes to comments and docs).
Do let me know if there are any undesirable changes in this revision. Thanks.

P.S. sorry for the large amt of reviewers and subscribers auto assigned cause I did the regex search for the aforementioned type of typos throughout the whole codebase (typo fixes present in this revision are hand picked although there might be judgement errors since I am new to LLVM).

Diff Detail

Event Timeline

Rageking8 created this revision.Nov 3 2022, 7:50 AM
Herald added a reviewer: rafauler. · View Herald Transcript
Herald added a reviewer: Amir. · View Herald Transcript
Herald added a reviewer: maksfb. · View Herald Transcript
Herald added a reviewer: NoQ. · View Herald Transcript
Herald added a reviewer: ributzka. · View Herald Transcript
Herald added projects: Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
Rageking8 requested review of this revision.Nov 3 2022, 7:50 AM
Herald added a reviewer: dang. · View Herald Transcript
Herald added a reviewer: dcaballe. · View Herald Transcript
Herald added projects: Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript

This is going to be impossible to cleanly review as-is. Could it be broken into lots of smaller chunks?

Rageking8 edited the summary of this revision. (Show Details)Nov 3 2022, 7:58 AM
arsenm accepted this revision.Nov 3 2022, 8:00 AM
glider added subscribers: pcc, glider.Nov 3 2022, 8:08 AM
glider added inline comments.
clang/docs/DataFlowSanitizerDesign.rst
54

I believe the word duplication is on purpose here. I'd rewrite the comment as:

Returns whether the given label @label contains the label @elem.

@pcc, what do you think?

compiler-rt/include/sanitizer/dfsan_interface.h
65

Same as clang/docs/DataFlowSanitizerDesign.rst above.

compiler-rt/lib/asan/asan_allocator.cpp
1161

Ack.

compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_test.cpp
211

Ack.

compiler-rt/lib/tsan/rtl-old/tsan_interceptors_posix.cpp
2510

Ack.

compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
2529

Ack.

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
540

Ack.

foad added a comment.Nov 3 2022, 8:35 AM

I committed the lib/Target/AMDGPU parts as 5073ae2a883f.

Changes in /polly/ look good to me.

LGTM for sparse changes

(note that you could have made your life a bit easier if you had broken this revision up at least over different projects, getting a global "LGTM" from somebody may be a bit hard now ;-)

dang accepted this revision.Nov 3 2022, 9:24 AM
ldionne accepted this revision.Nov 3 2022, 9:39 AM
ldionne added a subscriber: ldionne.

Thanks for the fixes. LGTM for libcxx/, libcxxabi/ and libunwind/.

Looked at the lldb changes, some comments for you. If you want to get a "looks good" for those please submit a separate review with only the lldb parts and I'll review that instead.

As others said, appreciate the effort but the review process doesn't scale well to so many changes in one patch.

lldb/include/lldb/Target/Process.h
1204

This should be "which OS it is emulating".

lldb/source/Symbol/LineTable.cpp
92

This should be "Instead it is issuing".

lldb/source/Target/RegisterContextUnwind.cpp
701

This one is intended. It's not very scientific but hey, it's getting the point across.

lldb/tools/lldb-vscode/JSONUtils.cpp
1045

This one is intended. Given the context of batching work, it's making the point for very large amounts of children.

Changes to openmp look good to me.

sivachandra accepted this revision.Nov 3 2022, 9:45 AM
sivachandra added a subscriber: sivachandra.

Changes in the libc directory LGTM.

Amir accepted this revision.Nov 3 2022, 12:34 PM

BOLT changes LGTM

int3 added a subscriber: int3.Nov 3 2022, 12:38 PM

lld/MachO changes lgtm

lld/MachO/Driver.cpp
1386

"assumes that the page size" sounds more natural and is probably what was intended

lld/MachO/UnwindInfoSection.cpp
576–577

I think we can reflow this paragraph to max the line length

The clang parts generally LG, but I spotted a few things. Thank you for this cleanup effort!

clang/include/clang/ASTMatchers/ASTMatchersInternal.h
467

This is a good change, but you also need to run clang/docs/tools/dump_ast_matchers.py to regenerate the public-facing documentation that is generated from this file.

clang/lib/CodeGen/CodeGenTBAA.cpp
341

I think this should read:

// Handle C++ base classes. Non-virtual bases can treated as a kind of
clang/lib/Driver/ToolChains/Darwin.cpp
47

This is duplicated just often enough and in just enough contexts where I think "driver driver" means "the driver that drives another driver", but it'd be nice if someone else could confirm or deny that.

clang/lib/Driver/ToolChains/Gnu.cpp
87

Same for this one as above.

curdeius added inline comments.
clang/lib/CodeGen/CodeGenTBAA.cpp
341

A mistake: can *be* treated.

aaron.ballman added inline comments.Nov 3 2022, 1:50 PM
clang/lib/CodeGen/CodeGenTBAA.cpp
341

Oh gosh, good catch! 😳

I am ok with you guys taking the parts of this revision that you reviewed and directly commiting them to the repo, just like how the person above did it. This is to break the revision up to smaller parts.

I am ok with you guys taking the parts of this revision that you reviewed and directly commiting them to the repo, just like how the person above did it. This is to break the revision up to smaller parts.

What name and email address would you like us to use for patch attribution when landing these bits?

I am ok with you guys taking the parts of this revision that you reviewed and directly commiting them to the repo, just like how the person above did it. This is to break the revision up to smaller parts.

What name and email address would you like us to use for patch attribution when landing these bits?

Rageking8
tomleetyt@gmail.com

LGTM for changes under clang-tools-extra/clangd

I landed the Clang bits in 94738a5ac34283bb034b022602b9f9e93d67081f, thank you for the cleanup!

thakis added a subscriber: thakis.Nov 8 2022, 5:46 AM
thakis added inline comments.
clang/lib/Driver/ToolChains/Darwin.cpp
47

I think that's correct. IIRC, the "driver driver" was a feature in apple gcc long ago, and it was a driver that invoked the gcc driver once per -arch or some such. One of the early goals of clang was to just have a driver, but no driver driver.

This revision is now accepted and ready to land.Dec 9 2022, 8:58 PM

The libcxx, libcxxabi and libunwind parts were committed in b397921fc7ef4e2882b3ae9c5f12fb40daca4078. Is it possible to close this or rebase it so we can remove the libc++ subscription to clear the review queue? We're trying to clean things up as part of the Github PR transition.

owenpan added inline comments.Aug 31 2023, 1:14 PM
clang/lib/Format/TokenAnnotator.cpp
4899–4902

We should reflow the comment:

// Note that this allows the "{" to go over the column limit when the
// column limit is just between ":" and "{", but that does not happen too
// often and alternative formattings in this case are not much better.
clang/lib/Format/WhitespaceManager.h
202–203

We should merge the comment into a single line.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please try to ensure all code changes have a unit test (unless this is an NFC or refactoring, adding documentation etc..)

Add your unit tests in clang/unittests/Format and you can build with ninja FormatTests. We recommend using the verifyFormat(xxx) format of unit tests rather than EXPECT_EQ as this will ensure you change is tolerant to random whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can happen if you are trying to improve the annotation phase to ensure we are correctly identifying the type of a token, please add a token annotator test in TokenAnnotatorTest.cpp

MaskRay closed this revision.Sep 1 2023, 9:36 PM