[PATCH] D134457: Document WarnOnSizeOfPointerToAggregate.

This revision was automatically updated to reflect the committed changes. Closed by commit rG33bc9c3bf2e0: Document WarnOnSizeOfPointerToAggregate. (authored by mbenfield). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134457/new/ https://reviews.ll

[PATCH] D134457: Document WarnOnSizeOfPointerToAggregate.

mbenfield created this revision. mbenfield added a reviewer: mizvekov. Herald added a project: All. mbenfield requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. This option was just landed in D134381

[PATCH] D134381: [clang-tidy] Add option WarnOnSizeOfPointerToAggregate.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0ca599374187: [clang-tidy] Add option WarnOnSizeOfPointerToAggregate. (authored by mbenfield). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D134381: [clang-tidy] Add option WarnOnSizeOfPointerToAggregate.

mbenfield updated this revision to Diff 462007. mbenfield added a comment. Add a test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134381/new/ https://reviews.llvm.org/D134381 Files: clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.

[PATCH] D134381: [clang-tidy] Add option WarnOnSizeOfPointerToAggregate.

mbenfield added a comment. I'll add a couple simple tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134381/new/ https://reviews.llvm.org/D134381 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D134381: [clang-tidy] Add option WarnOnSizeOfPointerToAggregate.

mbenfield created this revision. mbenfield added a reviewer: mizvekov. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. mbenfield requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. This is now an opt

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbc5f2d12cadc: [clang] diagnose_as_builtin attribute for Fortify diagnosing like builtins. (authored by mbenfield). Repository: rG LLVM Github Mono

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 393942. mbenfield added a comment. Herald added a subscriber: jdoerfert. Fix test clang/test/Misc/pragma-attribute-supported-attributes-list.test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/ http

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 393226. mbenfield added a comment. Remove late parsing. Switch dyn_cast to cast. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/ https://reviews.llvm.org/D112024 Files: clang/include/clang/Basic

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield added a comment. @aaron.ballman Look to you like this is ready to land? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/ https://reviews.llvm.org/D112024 ___ cfe-commits mailing list c

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 392830. mbenfield added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/ https://reviews.llvm.org/D112024 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/At

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 391076. mbenfield marked 8 inline comments as done. mbenfield added a comment. Replace DeclFD->getName() with just DeclFD. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/ https://reviews.llvm.org/D1

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 391067. mbenfield added a comment. Revert spurious whitespace change. assert(D) No else after return. Allow attribute to be applied to static member functions and test this. Document that it can't be applied to a non-static member function. Repository:

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 389355. mbenfield added a comment. Error on member function. Test case for this. Test case for attribute applied to a non-function declaration. Handle variadic functions. Document this. Test case for variadic functions. Remove const from some local values

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 387389. mbenfield added a comment. s/requires/takes in diagnostic message. Test case with a non-template overloaded function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/ https://reviews.llvm.or

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield added inline comments. Comment at: clang/test/Sema/attr-diagnose-as-builtin.c:62 + +#ifdef __cplusplus +template george.burgess.iv wrote: > nit: can we also add a non-templated overload check in here? > > if the diag isn't beautiful, that's fine IMO.

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 386623. mbenfield added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/ https://reviews.llvm.org/D112024 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/At

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield added a comment. Would @george.burgess.iv and/or @aaron.ballman mind taking another look at the latest revision? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/ https://reviews.llvm.org/D112024

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 385551. mbenfield added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/ https://reviews.llvm.org/D112024 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/At

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

This revision was automatically updated to reflect the committed changes. Closed by commit rG2db66f8d48be: [clang] Fortify warning for scanf calls with field width too big. (authored by mbenfield). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111833

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield marked 8 inline comments as done. mbenfield added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3822 +def DiagnoseAs : InheritableAttr { + let Spellings = [Clang<"diagnose_as">]; + let Args = [ExprArgument<"Function">, serge-sans-pail

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 384451. mbenfield added a comment. Renamed to diagnose_as_builtin (since two people suggested that name). Let me know if a name mentioning fortify is preferred. Validation in handleDiagnoseAsBuiltinAttr: - first argument is a builtin function - number of a

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

mbenfield updated this revision to Diff 384431. mbenfield added a comment. Ignore specifiers with *. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111833/new/ https://reviews.llvm.org/D111833 Files: clang/include/clang/Basic/DiagnosticSemaKinds.

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

This revision was automatically updated to reflect the committed changes. Closed by commit rG5a8c1736289f: [clang] Fortify warning for scanf calls with field width too big. (authored by mbenfield). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111833

[PATCH] D112850: [clang] 'unused-but-set-variable' warning should not apply to __block objective-c pointers

mbenfield added a comment. This seems reasonable to me, but I don't know Objective C. Presumably someone who does should accept. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112850/new/ https://reviews.llvm.org/D112850 __

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

mbenfield added a comment. Previously his patch did not cover %c and %[, but now it does. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111833/new/ https://reviews.llvm.org/D111833 ___ cfe-commits mailin

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

mbenfield updated this revision to Diff 383161. mbenfield added a comment. Support %c and %[ specifiers. Changed the diagnostic message to accommodate. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111833/new/ https://reviews.llvm.org/D111833 Fil

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

mbenfield added a comment. Thanks for reverting. Here's another try. This just shouldn't be warning on any specifier other than %s, so I fixed that. As far as not pointing at code for the second warning, I verified manually that it does now point at code even when warning twice, but it doesn't

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

mbenfield updated this revision to Diff 383064. mbenfield added a comment. Only diagnose if conversion specifier is %s. Give the location of the particular argument rather than the location of the function call. Test to make sure we don't warn on %d. Repository: rG LLVM Github Monorepo CHA

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

This revision was automatically updated to reflect the committed changes. Closed by commit rG15e3d39110fa: [clang] Fortify warning for scanf calls with field width too big. (authored by mbenfield). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111833

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

mbenfield updated this revision to Diff 382845. mbenfield added a comment. const Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111833/new/ https://reviews.llvm.org/D111833 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

mbenfield updated this revision to Diff 382840. mbenfield added a comment. fix function_ref use-after-free Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111833/new/ https://reviews.llvm.org/D111833 Files: clang/include/clang/Basic/DiagnosticSema

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

mbenfield updated this revision to Diff 382802. mbenfield added a comment. rebase and rerun tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111833/new/ https://reviews.llvm.org/D111833 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

mbenfield updated this revision to Diff 382075. mbenfield added a comment. rebase and rerun tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111833/new/ https://reviews.llvm.org/D111833 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

mbenfield updated this revision to Diff 380956. mbenfield added a comment. respond to comments: null to NUL, remove stray space, use function_ref Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111833/new/ https://reviews.llvm.org/D111833 Files: c

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield created this revision. mbenfield added a reviewer: george.burgess.iv. Herald added a reviewer: aaron.ballman. mbenfield requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.or

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

mbenfield added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:662 +if (Index < FD->getNumParams()) { + if (const auto *POS = + FD->getParamDecl(Index)->getAttr()) enh wrote: > mbenfield wrote: > > enh wrote: > > > (stray tabs?

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

mbenfield added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:662 +if (Index < FD->getNumParams()) { + if (const auto *POS = + FD->getParamDecl(Index)->getAttr()) enh wrote: > (stray tabs?) Not sure what you're referring to. A

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

mbenfield created this revision. mbenfield added reviewers: enh, gbiv. Herald added a reviewer: george.burgess.iv. mbenfield requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D11

[PATCH] D109862: Don't diagnose unused but set when the Cleanup attribute is used.

This revision was automatically updated to reflect the committed changes. Closed by commit rGaf9923674787: Don't diagnose unused but set when the Cleanup attribute is used. (authored by mbenfield). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109862

[PATCH] D109862: Don't diagnose unused but set when the Cleanup attribute is used.

mbenfield updated this revision to Diff 373069. mbenfield added a comment. Added a test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109862/new/ https://reviews.llvm.org/D109862 Files: clang/lib/Sema/SemaDecl.cpp clang/test/Sema/warn-un

[PATCH] D109862: Don't diagnose unused but set when the Cleanup attribute is used.

mbenfield created this revision. mbenfield added a reviewer: dblaikie. mbenfield requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This applies to -Wunused-but-set-variable and -Wunused-but-set-parameter. This addresses bug 51865. Reposito

[PATCH] D109642: -Wunused-but-set-parameter/-Wunused-but-set-variable Add to the release notes

mbenfield added a comment. I'd request small changes for grammar: `Wunused-but-set-parameter` and `-Wunused-but-set-variable` emit warnings when a parameter or a variable is set but not used. Otherwise LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source.

mbenfield updated this revision to Diff 359955. mbenfield added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104887/new/ https://reviews.llvm.org/D104887 Files: clang/include/clang/AST/Expr.h clang/include/clang/Basic/Diagn

[PATCH] D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source.

mbenfield updated this revision to Diff 359655. mbenfield added a comment. Rebased and addressed comments. - Renamed ComputeCheckVariantSize. - const Expr *. - Changed existing test case to write only one excess byte. - Added new, non-diagnosing test case. Repository: rG LLVM Github Monore

[PATCH] D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source.

mbenfield added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:739 DiagID = diag::warn_fortify_source_size_mismatch; -SizeIndex = TheCall->getNumArgs() - 1; -ObjectIndex = 0; +SourceSize = ComputeCheckVariantSize(TheCall->getNumArgs() - 1); +Des

[PATCH] D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source.

mbenfield added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:15737 + +// Fall through to slow path. + } cjdb wrote: > This comment is slightly different to the RHS: does that mean that a > diagnostic might not be issued? I just didn't find th

[PATCH] D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source.

mbenfield updated this revision to Diff 358309. mbenfield added a comment. undo commenting out in source_location.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104887/new/ https://reviews.llvm.org/D104887 Files: clang/include/clang/AST/Expr.

[PATCH] D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source.

mbenfield updated this revision to Diff 357077. mbenfield added a comment. fix failing tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104887/new/ https://reviews.llvm.org/D104887 Files: clang/include/clang/AST/Expr.h clang/include/clang/B

[PATCH] D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source.

mbenfield updated this revision to Diff 354563. mbenfield added a comment. Accommodate the fact that APSInt::toString(unsigned) was removed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104887/new/ https://reviews.llvm.org/D104887 Files: clang/

[PATCH] D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source.

mbenfield created this revision. mbenfield added reviewers: george.burgess.iv, cjdb, rsmith. mbenfield requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Also introduce Expr::tryEvaluateStrLen. Repository: rG LLVM Github Monorepo https://

[PATCH] D103623: [Clang] Test case for -Wunused-but-set-variable, warn for volatile.

mbenfield updated this revision to Diff 351563. mbenfield added a comment. rebase and rerun builds Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103623/new/ https://reviews.llvm.org/D103623 Files: clang/test/Sema/warn-unused-but-set-variables.c

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

mbenfield accepted this revision. mbenfield added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103949/new/ https://reviews.llvm.org/D103949 ___ cfe-commits mailing list cfe-com

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

mbenfield added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:7794-7798 +if (BO->getLHS()->getType()->isDependentType() || +BO->getRHS()->getType()->isDependentType()) +{ + if (BO->getOpcode() != BO_Assign) +return; Woul

[PATCH] D103623: [Clang] Test case for -Wunused-but-set-variable, warn for volatile.

mbenfield created this revision. mbenfield added a reviewer: sberg. mbenfield requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This was requested by sberg. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D103623 Files: c

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. In D100581#2795966 , @sberg wrote: > Is it intentional that this warns about volatile variables as in Yes, volatile was intentional. Alright, I will add a test for this case. Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. In D100581#2792854 , @Abpostelnicu wrote: > I think there is a false positive with this @george.burgess.iv: > In this >

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield updated this revision to Diff 347111. mbenfield added a comment. Move fixes into a separate change https://reviews.llvm.org/D102942. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm.org/D100581 Files: clan

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield updated this revision to Diff 346904. mbenfield added a comment. Herald added subscribers: llvm-commits, lldb-commits, kbarton, hiraditya, nemanjai. Herald added projects: LLDB, LLVM. Don't warn for reference or dependent types (fixing false positives). Also fix a few places where this

[PATCH] D101937: [Clang][Manual] Mention -fdebug-info-for-profiling.

mbenfield created this revision. mbenfield added reviewers: rsmith, wmi. Herald added a subscriber: wenlei. mbenfield requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This option is necessary for effective AutoFDO, and it also seems that the

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. FWIW `gcc` does not warn in this case regardless of any cast: void f() { int x; x = 0; sizeof(x); } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm.org/D100581

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. In D100581#2730557 , @nickdesaulniers wrote: > I see lots of instances from the kernel that look like this when reduced: > But adding a new warning flags to a handful of existing tests' RUN lines is > unusual. These tests

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield updated this revision to Diff 342071. mbenfield added a comment. Count a non-ODR use as a reference, so that examples like that of nickdesaulniers don't warn. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield updated this revision to Diff 342037. mbenfield added a comment. Correct struct and vector behavior. gcc doesn't warn for any structs in C++. However, it _does_ warn for vector types. Those warnings are sometimes masked by other errors, leading to my earlier belief that it is inconsis

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. Apparently arc and/or Phabricator doesn't like my attempt to split this review into two separate commits. Next revision will have only one commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield updated this revision to Diff 341965. mbenfield added a comment. A second commit with UnusedButSetParameter and UnusedButSetVariable into groups. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm.org/D100581

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. Will also make a new revision tomorrow with these warnings added to the appropriate groups. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm.org/D100581

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. In D100581#2727357 , @dblaikie wrote: > > Got a link/examples of cases GCC does and doesn't warn about? I'd assume it'd > have something to do with the triviality or non-triviality of certain > operations of the nonscalar

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield updated this revision to Diff 341695. mbenfield added a comment. Another try at these warnings, using the implementation strategy outlined by rsmith. A couple other notes: - At the moment I've removed these warnings from the diagnostic groups -Wextra and -Wunused. It was suggested by

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. In D100581#2723801 , @xbolva00 wrote: > > Yes, the best solution. Also consider to check gcc’s test-suite - if they > have this type of testcase, then this is wanted behaviour. gcc has `Wunused-var-5.c`, where the warning

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. So it seems there are 3 issues here: 1. local variables declared extern should not trigger this warning. I'll fix this. 2. Should `x` in this code trigger the warning: int x; int y = (x = 0); These are essentially the "false positives" reported in the Linux kern

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. Link for the revert: https://reviews.llvm.org/D101480 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm.org/D100581 ___ cfe-commits mailing list cf

[PATCH] D101480: Revert "[Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable"

mbenfield created this revision. mbenfield added reviewers: aeubanks, george.burgess.iv, lebedev.ri, Abpostelnicu. Herald added subscribers: frasercrmck, luismarques, apazos, sameer.abuasal, pengfei, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng,

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. In D100581#2723348 , @lebedev.ri wrote: > Also, from the reviewer list, it doesn't look like anyone with much > familiarity with clang/clang diags actually participated in the review? > I think this might be the point to rever

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. In D100581#2721705 , @dbabokin wrote: > One more false-positive: > > void foo() { > extern int yydebug; > yydebug = 1; > } > > It was triggered in ISPC build. Thanks for the report. It seems the thing to do is to mo

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. In D100581#2717182 , @aeubanks wrote: > I can land this for you, but could you update the description to be a bit > more descriptive? Explaining what the warnings do. Are you looking at the updated message in the commit, or th

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. Great. If it's been approved, I am not able to commit, so I think someone else will have to do that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm.org/D100581 ___

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield updated this revision to Diff 339778. mbenfield added a comment. Fix test warning-wall.c. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm.org/D100581 Files: clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield marked 4 inline comments as done. mbenfield added a comment. In D100581#2705780 , @xbolva00 wrote: > Meanwhile I collected some interesting testcases from gcc bugzilla (reported > as false positives), please try: > https://godbolt.org/z/747ndKE

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. In D100581#2700083 , @xbolva00 wrote: > So I think we should put UnusedButSetParameter under -Wextra > > So I would just put UnusedButSetVariable under -Wunused (and -Wunused is part > of -Wall): > > WDYT? Sounds good; I've do

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield updated this revision to Diff 339275. mbenfield added a comment. Herald added subscribers: frasercrmck, luismarques, apazos, sameer.abuasal, pengfei, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, simoncook, joh

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. In D100581#2699412 , @xbolva00 wrote: > I am a little bit worried that another off by default warning is not ideal > from user point of view. Either the user simply would fail to find out that > there is a new option or will b

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield updated this revision to Diff 338423. mbenfield added a comment. Fixed misfirings reported by aeubanks and added tests for those cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm.org/D100581 Files: c

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. In D100581#2693131 , @xbolva00 wrote: >>> These warnings are not enabled by any other flags. This is different from >>> gcc, where -Wunused-but-set-variable is enabled by -Wextra in combination >>> with either -Wunused or -Wal

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:437 + CompoundStmt *CS = CompoundStmt::Create(Context, Elts, L, R); + DiagnoseUnusedButSetVariables(CS); + return CS; mbenfield wrote: > mbenfield wrote: > > mbenfield wrote: > > > george

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:437 + CompoundStmt *CS = CompoundStmt::Create(Context, Elts, L, R); + DiagnoseUnusedButSetVariables(CS); + return CS; mbenfield wrote: > mbenfield wrote: > > george.burgess.iv wrote: > >

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield marked 4 inline comments as done. mbenfield added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:13751 +bool TraverseBinaryOperator(BinaryOperator *BO) { + auto *LHS = BO->getLHS(); + auto *DRE = dyn_cast(LHS); george.burgess.iv

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield updated this revision to Diff 337999. mbenfield marked an inline comment as done. mbenfield added a comment. Capitalization and periods for comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm.org/D1005

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield updated this revision to Diff 337998. mbenfield added a comment. Respond to comments. - Use /// for function comment - Improve the text of function comment - Use SmallPtrSet instead of SmallDenseMap - Use reference instead of pointer in AllUsesAreSetsVisitor - Capture with [&] - co

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:13740 +// other than assigning to it, sets the corresponding value to false. +static void AreAllUsesSets(Stmt *Body, + llvm::SmallDenseMap *Map) { george.burgess.iv

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield updated this revision to Diff 337877. mbenfield added a comment. Updates in response to comments. - Parameters.empty() and early exit. - comments in VisitDeclRefExpr. - clearer description of the warnings. Also, changed the name of DiagnoseUnusedDecls to DiagnoseUnusedButSetDecls for

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. In D100581#2692425 , @aeubanks wrote: > running this over an existing codebase to see what fires is probably a good > idea (if you haven't already done that) I ran it on LLVM itself (with clang and lldb) and got no firings at

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added inline comments. Comment at: clang/test/Sema/vector-gcc-compat.c:38 v2i64 v2i64_a = (v2i64){0, 1}; - v2i64 v2i64_r; + v2i64 v2i64_r; // expected-warning{{variable 'v2i64_r' set but not used}} aeubanks wrote: > this file isn't really testing

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield updated this revision to Diff 337842. mbenfield added a comment. bugfix: don't increment FalseCount when the value was already false Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm.org/D100581 Files: clan

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. This is my first code submitted to LLVM; please tell me anything that should be done differently. A couple decisions that should be considered: - These warnings are not enabled by any other flags. This is different from gcc, where `-Wunused-but-set-variable` is enabl

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield created this revision. mbenfield added reviewers: george.burgess.iv, cjdb, aeubanks. mbenfield requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. These are intended to mimic warnings available in gcc. Repository: rG LLVM Github M