[PATCH] D133563: [Clang] Improve diagnostics about the invalid target feature.

2022-09-24 Thread liushuai wang via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG910ad36e1a25: [Clang] Improve diagnostics about the invalid target feature. (authored by MTC). Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D128207: [clang-doc][NFC] Fix reference invalidation assertion failure.

2022-06-22 Thread liushuai wang via Phabricator via cfe-commits
MTC added a comment. Sorry for the delayed response. Adding the regression test is not as simple as I thought, **first of all clang-tools-extra does not enable the //assert//**, see https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/test/lit.site.cfg.py.in . And add the clang-doc

[PATCH] D128207: [clang-doc][NFC] Fix reference invalidation assertion failure.

2022-06-20 Thread liushuai wang via Phabricator via cfe-commits
MTC created this revision. MTC added reviewers: dexonsmith, mehdi_amini. MTC added a project: clang-tools-extra. Herald added subscribers: jsji, usaxena95, pengfei, kadircet, arphaman. Herald added a project: All. MTC requested review of this revision. Herald added subscribers: cfe-commits, ilya-bi

[PATCH] D123924: [clang-apply-replacements] Added an option to ignore insert conflict.

2022-05-04 Thread liushuai wang via Phabricator via cfe-commits
MTC added a comment. In D123924#3482342 , @aaron.ballman wrote: > I don't know enough about this code base to feel comfortable signing off on > it, but the changes look reasonable to me FWIW. Thanks, Aaron! I will find another guy to review the change.

[PATCH] D123924: [clang-apply-replacements] Added an option to ignore insert conflict.

2022-04-25 Thread liushuai wang via Phabricator via cfe-commits
MTC added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/AtomicChange.h:119 + Replacements &getRefReplacements() { return Replaces; } + IMHO, there is no need to rename the method to get the non-const version. The following code is ok e

[PATCH] D117090: [clang-tidy] Fix `readability-non-const-parameter` for parameter referenced by an lvalue

2022-02-23 Thread liushuai wang via Phabricator via cfe-commits
MTC added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-non-const-parameter.cpp:293 + // CHECK-MESSAGES-NOT: warning: pointer parameter 'p' can be + int &x = *p; +} Sockke wrote: > MTC wrote: > > @Sockke Could you please

[PATCH] D117090: [clang-tidy] Fix `readability-non-const-parameter` for parameter referenced by an lvalue

2022-02-23 Thread liushuai wang via Phabricator via cfe-commits
MTC added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-non-const-parameter.cpp:293 + // CHECK-MESSAGES-NOT: warning: pointer parameter 'p' can be + int &x = *p; +} @Sockke Could you please add the following tests? ``` i

[PATCH] D117090: Fix `readability-non-const-parameter` for parameter referenced by an lvalue

2022-01-12 Thread liushuai wang via Phabricator via cfe-commits
MTC added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp:86-99 + // Data passed by nonconst reference should not be made const. + unsigned ArgNr = 0U; + if (const auto *CD = CE->getConstructor()) { +for (co

[PATCH] D116439: [clang-tidy] Fix `readability-const-return-type` for pure virtual function.

2022-01-07 Thread liushuai wang via Phabricator via cfe-commits
MTC added subscribers: flx, MTC. MTC added a comment. FYI: I'm from Bytedance Inc , @Sockke, and I are fixing the AutoFix bugs recently, most of them will lead to the compilation error. For this bug, fixing the override virtual methods but missing the pure virtua

[PATCH] D107450: [clang-tidy] Fix wrong FixIt in performance-move-const-arg

2022-01-07 Thread liushuai wang via Phabricator via cfe-commits
MTC added a comment. This patch has become very complicated now. I summarized this patch and give a figure to illustrate what we have reached. And @Sockke please add some comments to explain the complex part or other means to make this patch more readable. F21496057: D107450.svg

[PATCH] D108370: [clang-tidy] Fix wrong FixIt about union in cppcoreguidelines-pro-type-member-init

2021-08-19 Thread liushuai wang via Phabricator via cfe-commits
MTC added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:49 +void forEachFieldWithFilter(const RecordDecl &Record, const T &Fields, +bool &AnyMemberHasInitPerUnion, Func &&Fn) { + for (const Fie

[PATCH] D67149: Fix argument order for BugType instation for

2021-08-16 Thread liushuai wang via Phabricator via cfe-commits
MTC added a comment. Herald added a subscriber: martong. Hi @NoQ, sorry to bother you. This patch has been on hold for a very long time. It should not have been fixed yet. What's next? Abandon the patch, or improve it yourself? Repository: rC Clang CHANGES SINCE LAST ACTION https://review

[PATCH] D107641: [clang-tidy] fix duplicate '{}' in cppcoreguidelines-pro-type-member-init

2021-08-13 Thread liushuai wang via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1f2d40c47f5f: [clang-tidy] fix duplicate '{}' in cppcoreguidelines-pro-type-member-init (authored by Sockke, committed by MTC). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D107450: [clang-tidy] Fix wrong and missing warnings in performance-move-const-arg

2021-08-11 Thread liushuai wang via Phabricator via cfe-commits
MTC added a comment. I think we're a bit off track, and @Sockke wants to accomplish more than one goal in the same patch. I have summarized what we are currently discussing as follow shows: 1. Fix the wrong AutoFix which blocks the compilation. 2. Find more 'unrecommended' std::move usage and

[PATCH] D107450: [clang-tidy] Fix wrong and missing warnings in performance-move-const-arg

2021-08-10 Thread liushuai wang via Phabricator via cfe-commits
MTC added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:261 + showInt(std::move(a)); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect; consi

[PATCH] D107641: [clang-tidy] fix duplicate '{}' in cppcoreguidelines-pro-type-member-init

2021-08-08 Thread liushuai wang via Phabricator via cfe-commits
MTC added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:454 + + if (AllFieldsToInit.empty()) +return; MTC wrote: > `FieldsToInit` being empty implies `AllFieldsToInit` is empty. We have > checked the

[PATCH] D107641: [clang-tidy] fix duplicate '{}' in cppcoreguidelines-pro-type-member-init

2021-08-07 Thread liushuai wang via Phabricator via cfe-commits
MTC added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:36 template void forEachField(const RecordDecl &Record, const T &Fields, Func &&Fn) { for (const FieldDecl *F : Fields) { Btw, the parameter `R

[PATCH] D107450: [clang-tidy] Fix wrong and missing warnings in performance-move-const-arg

2021-08-05 Thread liushuai wang via Phabricator via cfe-commits
MTC added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:75 + return std::move(x4); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: passing result of std::move() as a const reference argument; no move will actually happe

[PATCH] D106431: [clang-tidy] Fix cppcoreguidelines-init-variables with enum judgement

2021-07-21 Thread liushuai wang via Phabricator via cfe-commits
MTC added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp:103 + // Expect no warning given here. + Color color; + // Expect no warning given here. steven.zhang wrote: > Technical speaking, we should

[PATCH] D106431: [clang-tidy] Fix cppcoreguidelines-init-variables with enum judgement

2021-07-21 Thread liushuai wang via Phabricator via cfe-commits
MTC added a comment. In D106431#2892859 , @whisperity wrote: > Is this the right decision to make, conceptually? It will leave the variable > uninitialised still, and reading such an uninit variable is still an issue, > even if it is an enum. Yeah, th

[PATCH] D105973: [ASTMatchers] NFC: Fix the annotation.

2021-07-14 Thread liushuai wang via Phabricator via cfe-commits
MTC accepted this revision. MTC added a comment. This revision is now accepted and ready to land. LGTM, thanks for your cleanup. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105973/new/ https://reviews.llvm.org/D105973 ___ cfe-commits mailin