Page MenuHomePhabricator

mbenfield (Michael Benfield)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 13 2021, 1:34 PM (57 w, 6 d)

Recent Activity

Feb 23 2022

mbenfield accepted D120372: [clang] 'unused-but-set-variable' warning should not apply to __attribute__((objc_precise_lifetime) Objective-C pointers.
Feb 23 2022, 11:34 AM · Restricted Project

Dec 28 2021

mbenfield committed rG89aa87c4e601: [clang] Fix AttrDocs.td formatting. (authored by mbenfield).
[clang] Fix AttrDocs.td formatting.
Dec 28 2021, 11:13 AM

Dec 14 2021

mbenfield committed rGbc5f2d12cadc: [clang] diagnose_as_builtin attribute for Fortify diagnosing like builtins. (authored by mbenfield).
[clang] diagnose_as_builtin attribute for Fortify diagnosing like builtins.
Dec 14 2021, 11:43 AM
mbenfield closed D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..
Dec 14 2021, 11:42 AM · Restricted Project

Dec 13 2021

mbenfield updated the diff for D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..

Fix test clang/test/Misc/pragma-attribute-supported-attributes-list.test

Dec 13 2021, 9:43 AM · Restricted Project

Dec 9 2021

mbenfield updated the diff for D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..

Remove late parsing.

Dec 9 2021, 11:06 AM · Restricted Project

Dec 8 2021

mbenfield added a comment to D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..

@aaron.ballman Look to you like this is ready to land?

Dec 8 2021, 11:59 AM · Restricted Project
mbenfield updated the diff for D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..

Rebase.

Dec 8 2021, 10:35 AM · Restricted Project

Dec 1 2021

mbenfield updated the diff for D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..

Replace DeclFD->getName() with just DeclFD.

Dec 1 2021, 10:40 AM · Restricted Project
mbenfield updated the diff for D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..

Revert spurious whitespace change.

Dec 1 2021, 9:54 AM · Restricted Project

Nov 23 2021

mbenfield updated the diff for D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..

Error on member function. Test case for this.

Nov 23 2021, 4:45 PM · Restricted Project

Nov 15 2021

mbenfield updated the diff for D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..

s/requires/takes in diagnostic message.

Nov 15 2021, 1:41 PM · Restricted Project

Nov 12 2021

mbenfield added inline comments to D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..
Nov 12 2021, 10:26 AM · Restricted Project

Nov 11 2021

mbenfield updated the diff for D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..

Rebase.

Nov 11 2021, 12:29 PM · Restricted Project
mbenfield added a comment to D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..

Would @george.burgess.iv and/or @aaron.ballman mind taking another look at the latest revision?

Nov 11 2021, 12:23 PM · Restricted Project

Nov 8 2021

mbenfield updated the diff for D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..

Rebase.

Nov 8 2021, 10:12 AM · Restricted Project
mbenfield committed rG2db66f8d48be: [clang] Fortify warning for scanf calls with field width too big. (authored by mbenfield).
[clang] Fortify warning for scanf calls with field width too big.
Nov 8 2021, 9:44 AM
mbenfield closed D111833: [clang] Fortify warning for scanf calls with field width too big..
Nov 8 2021, 9:44 AM · Restricted Project

Nov 4 2021

mbenfield added inline comments to D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..
Nov 4 2021, 10:06 AM · Restricted Project

Nov 3 2021

mbenfield updated the diff for D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..

Renamed to diagnose_as_builtin (since two people suggested that name). Let me
know if a name mentioning fortify is preferred.

Nov 3 2021, 8:17 AM · Restricted Project
mbenfield updated the diff for D111833: [clang] Fortify warning for scanf calls with field width too big..

Ignore specifiers with *.

Nov 3 2021, 7:37 AM · Restricted Project
mbenfield reopened D111833: [clang] Fortify warning for scanf calls with field width too big..
Nov 3 2021, 7:35 AM · Restricted Project

Nov 1 2021

mbenfield added a reverting change for rG5a8c1736289f: [clang] Fortify warning for scanf calls with field width too big.: rGd51a8296d374: Revert "[clang] Fortify warning for scanf calls with field width too big.".
Nov 1 2021, 12:38 PM
mbenfield committed rGd51a8296d374: Revert "[clang] Fortify warning for scanf calls with field width too big." (authored by mbenfield).
Revert "[clang] Fortify warning for scanf calls with field width too big."
Nov 1 2021, 12:38 PM
mbenfield added a reverting change for D111833: [clang] Fortify warning for scanf calls with field width too big.: rGd51a8296d374: Revert "[clang] Fortify warning for scanf calls with field width too big.".
Nov 1 2021, 12:37 PM · Restricted Project
mbenfield committed rG5a8c1736289f: [clang] Fortify warning for scanf calls with field width too big. (authored by mbenfield).
[clang] Fortify warning for scanf calls with field width too big.
Nov 1 2021, 10:18 AM
mbenfield closed D111833: [clang] Fortify warning for scanf calls with field width too big..
Nov 1 2021, 10:18 AM · Restricted Project
mbenfield added a comment to D112850: [clang] 'unused-but-set-variable' warning should not apply to __block objective-c pointers.

This seems reasonable to me, but I don't know Objective C. Presumably someone who does should accept.

Nov 1 2021, 8:54 AM · Restricted Project

Oct 28 2021

mbenfield added a comment to D111833: [clang] Fortify warning for scanf calls with field width too big..

Previously this patch did not cover %c and %[, but now it does.

Oct 28 2021, 2:10 PM · Restricted Project
mbenfield updated the diff for D111833: [clang] Fortify warning for scanf calls with field width too big..

Support %c and %[ specifiers.

Oct 28 2021, 2:10 PM · Restricted Project
mbenfield added a comment to D111833: [clang] Fortify warning for scanf calls with field width too big..

Thanks for reverting. Here's another try.

Oct 28 2021, 9:34 AM · Restricted Project
mbenfield updated the diff for D111833: [clang] Fortify warning for scanf calls with field width too big..

Only diagnose if conversion specifier is %s.

Oct 28 2021, 9:31 AM · Restricted Project
mbenfield reopened D111833: [clang] Fortify warning for scanf calls with field width too big..
Oct 28 2021, 9:31 AM · Restricted Project

Oct 27 2021

mbenfield committed rG15e3d39110fa: [clang] Fortify warning for scanf calls with field width too big. (authored by mbenfield).
[clang] Fortify warning for scanf calls with field width too big.
Oct 27 2021, 8:00 PM
mbenfield closed D111833: [clang] Fortify warning for scanf calls with field width too big..
Oct 27 2021, 8:00 PM · Restricted Project
mbenfield updated the diff for D111833: [clang] Fortify warning for scanf calls with field width too big..

const

Oct 27 2021, 4:20 PM · Restricted Project
mbenfield updated the diff for D111833: [clang] Fortify warning for scanf calls with field width too big..

fix function_ref use-after-free

Oct 27 2021, 4:16 PM · Restricted Project
mbenfield updated the diff for D111833: [clang] Fortify warning for scanf calls with field width too big..

rebase and rerun tests

Oct 27 2021, 2:34 PM · Restricted Project

Oct 25 2021

mbenfield updated the diff for D111833: [clang] Fortify warning for scanf calls with field width too big..

rebase and rerun tests

Oct 25 2021, 11:48 AM · Restricted Project

Oct 20 2021

mbenfield updated the diff for D111833: [clang] Fortify warning for scanf calls with field width too big..

respond to comments: null to NUL, remove stray space, use function_ref

Oct 20 2021, 7:59 AM · Restricted Project

Oct 18 2021

mbenfield requested review of D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins..
Oct 18 2021, 11:07 AM · Restricted Project
mbenfield removed a reviewer for D111833: [clang] Fortify warning for scanf calls with field width too big.: gbiv.
Oct 18 2021, 11:06 AM · Restricted Project

Oct 15 2021

mbenfield added inline comments to D111833: [clang] Fortify warning for scanf calls with field width too big..
Oct 15 2021, 2:30 PM · Restricted Project
mbenfield added inline comments to D111833: [clang] Fortify warning for scanf calls with field width too big..
Oct 15 2021, 9:41 AM · Restricted Project

Oct 14 2021

mbenfield requested review of D111833: [clang] Fortify warning for scanf calls with field width too big..
Oct 14 2021, 1:25 PM · Restricted Project

Sep 22 2021

mbenfield committed rGaf9923674787: Don't diagnose unused but set when the Cleanup attribute is used. (authored by mbenfield).
Don't diagnose unused but set when the Cleanup attribute is used.
Sep 22 2021, 10:48 AM
mbenfield closed D109862: Don't diagnose unused but set when the Cleanup attribute is used..
Sep 22 2021, 10:48 AM · Restricted Project

Sep 16 2021

mbenfield updated the diff for D109862: Don't diagnose unused but set when the Cleanup attribute is used..

Added a test case.

Sep 16 2021, 2:18 PM · Restricted Project

Sep 15 2021

mbenfield requested review of D109862: Don't diagnose unused but set when the Cleanup attribute is used..
Sep 15 2021, 4:47 PM · Restricted Project

Sep 13 2021

mbenfield added a comment to D109642: -Wunused-but-set-parameter/-Wunused-but-set-variable Add to the release notes.

I'd request small changes for grammar:

Sep 13 2021, 9:01 AM · Restricted Project

Jul 19 2021

mbenfield updated the diff for D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source..

Rebase.

Jul 19 2021, 4:37 PM · Restricted Project

Jul 18 2021

mbenfield updated the diff for D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source..

Rebased and addressed comments.

Jul 18 2021, 4:05 PM · Restricted Project

Jul 16 2021

mbenfield added inline comments to D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source..
Jul 16 2021, 3:15 PM · Restricted Project
mbenfield added inline comments to D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source..
Jul 16 2021, 2:10 PM · Restricted Project

Jul 13 2021

mbenfield updated the diff for D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source..

undo commenting out in source_location.cpp

Jul 13 2021, 9:32 AM · Restricted Project

Jul 7 2021

mbenfield updated the diff for D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source..

fix failing tests

Jul 7 2021, 2:25 PM · Restricted Project

Jun 25 2021

mbenfield updated the diff for D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source..

Accommodate the fact that APSInt::toString(unsigned) was removed.

Jun 25 2021, 11:32 AM · Restricted Project

Jun 24 2021

mbenfield requested review of D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source..
Jun 24 2021, 5:10 PM · Restricted Project

Jun 11 2021

mbenfield updated the diff for D103623: [Clang] Test case for -Wunused-but-set-variable, warn for volatile..

rebase and rerun builds

Jun 11 2021, 2:16 PM · Restricted Project

Jun 10 2021

mbenfield accepted D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*.

LGTM

Jun 10 2021, 9:31 AM · Restricted Project

Jun 9 2021

mbenfield added inline comments to D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*.
Jun 9 2021, 9:08 AM · Restricted Project

Jun 3 2021

mbenfield requested review of D103623: [Clang] Test case for -Wunused-but-set-variable, warn for volatile..
Jun 3 2021, 8:54 AM · Restricted Project
mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

Is it intentional that this warns about volatile variables as in

Jun 3 2021, 8:42 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Jun 2 2021

mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

I think there is a false positive with this @george.burgess.iv:
In this we have the warning triggered:
mozglue/baseprofiler/core/platform-linux-android.cpp:216:9: error: variable 'r' set but not used [-Werror,-Wunused-but-set-variable]

SigHandlerCoordinator() {
  PodZero(&mUContext);
  int r = sem_init(&mMessage2, /* pshared */ 0, 0);
  r |= sem_init(&mMessage3, /* pshared */ 0, 0);
  r |= sem_init(&mMessage4, /* pshared */ 0, 0);
  MOZ_ASSERT(r == 0);
}
Jun 2 2021, 3:31 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Jun 1 2021

mbenfield added a comment to D102942: Remove or use variables which are unused but set..

I also heard via email from Amara Emerson that the change is fine for similar reasons.

Jun 1 2021, 9:05 AM · Restricted Project, Restricted Project

May 24 2021

mbenfield added a comment to D102942: Remove or use variables which are unused but set..

Sure, all sounds good - if you can, please reach out to the authors of any of the semantics changing changes (the ones related to the Changed values in transformations) to see if they could add missing test coverage.

May 24 2021, 10:23 AM · Restricted Project, Restricted Project

May 21 2021

mbenfield added a comment to D102942: Remove or use variables which are unused but set..

Several variables I have just removed outright, including

May 21 2021, 1:25 PM · Restricted Project, Restricted Project
mbenfield updated the diff for D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

Move fixes into a separate change https://reviews.llvm.org/D102942.

May 21 2021, 1:23 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield requested review of D102942: Remove or use variables which are unused but set..
May 21 2021, 1:20 PM · Restricted Project, Restricted Project

May 20 2021

mbenfield updated the diff for D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

Don't warn for reference or dependent types (fixing false positives).

May 20 2021, 6:54 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield reopened D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.
May 20 2021, 6:49 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

May 5 2021

mbenfield requested review of D101937: [Clang][Manual] Mention -fdebug-info-for-profiling..
May 5 2021, 12:53 PM · Restricted Project

Apr 30 2021

mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

FWIW gcc does not warn in this case regardless of any cast:

Apr 30 2021, 7:06 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

I see lots of instances from the kernel that look like this when reduced:

Apr 30 2021, 4:37 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield updated the diff for D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

Count a non-ODR use as a reference, so that examples like that of nickdesaulniers don't warn.

Apr 30 2021, 3:42 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield updated the diff for D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

Correct struct and vector behavior.

Apr 30 2021, 2:16 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

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.

Apr 30 2021, 1:35 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield updated the diff for D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

A second commit with UnusedButSetParameter and UnusedButSetVariable into groups.

Apr 30 2021, 10:25 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Apr 29 2021

mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

Will also make a new revision tomorrow with these warnings added to the appropriate groups.

Apr 29 2021, 4:50 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.
Apr 29 2021, 4:49 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield updated the diff for D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

Another try at these warnings, using the implementation strategy outlined by rsmith.

Apr 29 2021, 4:09 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Apr 28 2021

mbenfield abandoned D101480: Revert "[Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable".
Apr 28 2021, 3:06 PM · Restricted Project
mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.
Apr 28 2021, 1:57 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

So it seems there are 3 issues here:

Apr 28 2021, 1:24 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.
Apr 28 2021, 12:12 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield added a reverting change for rG9b0501abc7b5: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable: D101480: Revert "[Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable".
Apr 28 2021, 12:12 PM
mbenfield requested review of D101480: Revert "[Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable".
Apr 28 2021, 12:12 PM · Restricted Project
mbenfield added a reverting change for D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable: D101480: Revert "[Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable".
Apr 28 2021, 12:12 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

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 revert and re-review.

Apr 28 2021, 11:16 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

One more false-positive:

void foo() {
  extern int yydebug;
  yydebug = 1;
}

It was triggered in ISPC build.

Apr 28 2021, 10:18 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Apr 26 2021

mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

I can land this for you, but could you update the description to be a bit more descriptive? Explaining what the warnings do.

Apr 26 2021, 1:51 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Apr 23 2021

mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

Great. If it's been approved, I am not able to commit, so I think someone else will have to do that.

Apr 23 2021, 10:50 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Apr 22 2021

mbenfield updated the diff for D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

Fix test warning-wall.c.

Apr 22 2021, 2:08 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

Meanwhile I collected some interesting testcases from gcc bugzilla (reported as false positives), please try:
https://godbolt.org/z/747ndKEvd - No "unused-but-set" warnings expected.

Apr 22 2021, 2:06 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Apr 21 2021

mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

So I think we should put UnusedButSetParameter under -Wextra

So I would just put UnusedButSetVariable under -Wunused (and -Wunused is part of -Wall):

WDYT?

Apr 21 2021, 9:39 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield updated the diff for D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

Response to review comments.

Apr 21 2021, 9:38 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Apr 19 2021

mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

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 be surprised that gcc fires and clang does not even when we claim we implemented this “gcc’s” warning.

Apr 19 2021, 5:02 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Apr 18 2021

mbenfield updated the diff for D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

Fixed misfirings reported by aeubanks and added tests for those cases.

Apr 18 2021, 8:49 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
mbenfield added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

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 -Wall.

IMHO we should follow gcc here.

Apr 18 2021, 8:44 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Apr 16 2021

mbenfield added inline comments to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.
Apr 16 2021, 3:18 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project