Page MenuHomePhabricator

pfultz2 (Paul Fultz II)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 4 2015, 11:26 AM (223 w, 5 d)

Recent Activity

Sep 20 2018

pfultz2 added a comment to D52281: [clang-tidy] Add modernize check to use std::invoke in generic code.

This needs a test when calling in a constexpr function. I believe std::invoke is not constepxr, so a function object call in a constexpr function should not suggest std::invoke.

Sep 20 2018, 5:52 AM · Restricted Project

May 14 2018

pfultz2 updated the diff for D46159: [clang-tidy] Add a flag to enable alpha checkers.

Rebased

May 14 2018, 4:14 PM
pfultz2 added a comment to D46159: [clang-tidy] Add a flag to enable alpha checkers.

Is someone able to merge in my changes?

May 14 2018, 8:23 AM

May 9 2018

pfultz2 updated the diff for D46159: [clang-tidy] Add a flag to enable alpha checkers.

Some changes based on feedback.

May 9 2018, 8:16 AM

May 4 2018

pfultz2 added a comment to D46159: [clang-tidy] Add a flag to enable alpha checkers.

In this sense bug reports against abandoned alpha checkers (which are, unfortunately, the majority) aren't very useful. In most cases it's just too easy to see how broken they are.

May 4 2018, 4:51 PM
pfultz2 updated the diff for D46159: [clang-tidy] Add a flag to enable alpha checkers.

Moved flag for alpha checks to the ClangTidyContext

May 4 2018, 11:20 AM
pfultz2 added a comment to D46159: [clang-tidy] Add a flag to enable alpha checkers.

There doesn't seem to be a simple way to remove it from the ClangTidyOptions class, as a lot more functions need to be changed to support that. I would prefer to leave it there for now, and we can refactor it later. Either way, the check can't be set from a config file.

May 4 2018, 10:50 AM

May 3 2018

pfultz2 updated the diff for D46159: [clang-tidy] Add a flag to enable alpha checkers.

Rename flag to AllowEnablingAnalyzerAlphaCheckers.

May 3 2018, 12:36 PM
pfultz2 added inline comments to D46159: [clang-tidy] Add a flag to enable alpha checkers.
May 3 2018, 12:35 PM
pfultz2 added a comment to D46159: [clang-tidy] Add a flag to enable alpha checkers.

When something is merged into Clang trunk, the expectation is that it will be production quality or will be worked on rapidly to get it to production quality, which is somewhat orthogonal to getting user feedback. I don't know that I have the full history of all of the alpha checks so my generalization may be inaccurate, but it seems like some of these checks were accepted as a WIP and the "progress" stopped for a long time with no one volunteering to remove the features from the analyzer.

May 3 2018, 9:32 AM
pfultz2 updated the diff for D46159: [clang-tidy] Add a flag to enable alpha checkers.

Rename flag to allow-enabling-alpha-checks and removed the option from the clang-tidy file.

May 3 2018, 9:14 AM
pfultz2 added a comment to D46159: [clang-tidy] Add a flag to enable alpha checkers.

I think the premise is a bit off the mark. It's not that these are not for the common user -- it's that they're simply not ready for users at all.

May 3 2018, 8:58 AM
pfultz2 added a comment to D46159: [clang-tidy] Add a flag to enable alpha checkers.

But still, could you explain the use case and why a local modification of clang-tidy is not an option?

May 3 2018, 8:07 AM
pfultz2 added inline comments to D46159: [clang-tidy] Add a flag to enable alpha checkers.
May 3 2018, 7:59 AM
pfultz2 added a comment to D46159: [clang-tidy] Add a flag to enable alpha checkers.

As Devin says (and as we discussed this with Anna Zaks) alpha checkers are still in development, so we don't want to expose them to the users, even very curious ones.

May 3 2018, 7:54 AM

May 2 2018

pfultz2 added a comment to D46159: [clang-tidy] Add a flag to enable alpha checkers.

Do you have some better choices?

May 2 2018, 11:23 AM
pfultz2 added a comment to D46159: [clang-tidy] Add a flag to enable alpha checkers.

Do you want the flag to be renamed?

May 2 2018, 9:13 AM

Apr 27 2018

pfultz2 added a comment to D46159: [clang-tidy] Add a flag to enable alpha checkers.

Am i understanding correctly that the new -enable-alpha-checks flag doesn't enable alpha checks, but it only allows enabling them with a separate command?

Apr 27 2018, 2:17 PM
pfultz2 added a comment to D46159: [clang-tidy] Add a flag to enable alpha checkers.

I'm curious about the use case for this, since I don't think it ever makes sense to run all the alpha checks at once. These checks are typically a work in progress (they're not finished yet) and often have false positives that haven't been fixed yet.

Apr 27 2018, 2:03 PM

Apr 26 2018

pfultz2 created D46159: [clang-tidy] Add a flag to enable alpha checkers.
Apr 26 2018, 5:15 PM
pfultz2 updated the diff for D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow.

So I ran this on clang/llvm code base and fixed some false positives.

Apr 26 2018, 2:41 PM · Restricted Project
pfultz2 added a comment to D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow.

So moving to core will require explicit casts in some of the tests, especially for things like: memcpy(a262.s1, input, -1). Or this could be moved to another section instead of core. In D45532 there is talk of adding a bugprone section. I think this would be good there.

Apr 26 2018, 8:29 AM · Restricted Project
pfultz2 added a comment to D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow.

Have you tried on any large codebases?

Apr 26 2018, 7:42 AM · Restricted Project
pfultz2 added inline comments to D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow.
Apr 26 2018, 7:01 AM · Restricted Project
pfultz2 added a comment to D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow.

While I understand extending the analyzer to cover more is a good approach, there is -Wconversion which seemingly covers this -- or at least the trivial case(?):

Apr 26 2018, 6:59 AM · Restricted Project

Apr 25 2018

pfultz2 abandoned D46066: [analyzer] Add checker for underflowing unsigned integers.

So I submitted my change to the ConversionChecker, instead.

Apr 25 2018, 2:05 PM · Restricted Project
pfultz2 created D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow.
Apr 25 2018, 2:01 PM · Restricted Project
pfultz2 added a comment to D46066: [analyzer] Add checker for underflowing unsigned integers.

Isn't this case already covered by conversion checker?

Apr 25 2018, 10:20 AM · Restricted Project
pfultz2 created D46066: [analyzer] Add checker for underflowing unsigned integers.
Apr 25 2018, 8:21 AM · Restricted Project

Apr 9 2018

pfultz2 added a comment to D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check..

This seems like it would be nice to have in bugprone-* as well.

Apr 9 2018, 10:50 AM · Restricted Project

Apr 3 2018

pfultz2 updated the diff for D44231: [clang-tidy] Check for sizeof that call functions.

I have rebased.

Apr 3 2018, 7:54 AM · Restricted Project

Apr 2 2018

pfultz2 added a comment to D44231: [clang-tidy] Check for sizeof that call functions.

Is someone able to merge in my changes?

Apr 2 2018, 8:20 AM · Restricted Project

Mar 26 2018

pfultz2 added a comment to D44231: [clang-tidy] Check for sizeof that call functions.

Do you need someone to submit this for you?

Mar 26 2018, 10:40 AM · Restricted Project
pfultz2 added a comment to D44231: [clang-tidy] Check for sizeof that call functions.

So, I've added tests for class enums and bols.

Mar 26 2018, 7:47 AM · Restricted Project

Mar 23 2018

pfultz2 updated the diff for D44231: [clang-tidy] Check for sizeof that call functions.
Mar 23 2018, 2:01 PM · Restricted Project

Mar 16 2018

pfultz2 added a comment to D44231: [clang-tidy] Check for sizeof that call functions.

As this patch can catch some mistakes, I'm fine with checking it in. I agree that it is reasonable to write normal code like sizeof(func_call()) (not false positive), maybe set the option to false by default?

Mar 16 2018, 3:47 PM · Restricted Project
pfultz2 added a comment to D44231: [clang-tidy] Check for sizeof that call functions.

I have reworded the documentation. Hopefully it is clearer.

Mar 16 2018, 3:47 PM · Restricted Project
pfultz2 updated the diff for D44231: [clang-tidy] Check for sizeof that call functions.
Mar 16 2018, 3:44 PM · Restricted Project

Mar 15 2018

pfultz2 added a comment to D44231: [clang-tidy] Check for sizeof that call functions.

So I've updated the documentation on this. Are there any other updates needed for this to get it merged in?

Mar 15 2018, 8:20 AM · Restricted Project

Mar 9 2018

pfultz2 updated the diff for D44231: [clang-tidy] Check for sizeof that call functions.
Mar 9 2018, 1:22 PM · Restricted Project
pfultz2 added a comment to D44231: [clang-tidy] Check for sizeof that call functions.

I don't have a script for it. I've used "bear" with at least some of those projects because they use makefiles rather than cmake (https://github.com/rizsotto/Bear). I'm not tied to those projects specifically, either, so if you have a different corpus you'd prefer to use, that's fine. The important bit is testing it across a considerable amount of C code and C++ code to see whether this diagnostic is too chatty or not.

Mar 9 2018, 12:30 PM · Restricted Project
pfultz2 added a comment to D44231: [clang-tidy] Check for sizeof that call functions.

Again, that only works for C++ and not C.

Mar 9 2018, 8:15 AM · Restricted Project

Mar 8 2018

pfultz2 added a comment to D44231: [clang-tidy] Check for sizeof that call functions.

If the return type of foo() is changed, then the use for s1 will be automatically updated

Mar 8 2018, 2:38 PM · Restricted Project
pfultz2 added a comment to D44231: [clang-tidy] Check for sizeof that call functions.

Can you point to some real world code that would benefit from this check?

Mar 8 2018, 8:59 AM · Restricted Project
pfultz2 added a comment to D44241: [clang-tidy] Add a check for constructing non-trivial types without assigning to a variable.

How does this differ from misc-unused-raii?

Mar 8 2018, 7:48 AM · Restricted Project
pfultz2 added a comment to D44231: [clang-tidy] Check for sizeof that call functions.

Can you elaborate a bit more about this?

Mar 8 2018, 7:43 AM · Restricted Project

Mar 7 2018

pfultz2 created D44241: [clang-tidy] Add a check for constructing non-trivial types without assigning to a variable.
Mar 7 2018, 6:04 PM · Restricted Project
pfultz2 updated the diff for D44231: [clang-tidy] Check for sizeof that call functions.
Mar 7 2018, 3:17 PM · Restricted Project
pfultz2 updated the diff for D44231: [clang-tidy] Check for sizeof that call functions.
Mar 7 2018, 3:13 PM · Restricted Project
pfultz2 updated the diff for D44231: [clang-tidy] Check for sizeof that call functions.
Mar 7 2018, 3:12 PM · Restricted Project
pfultz2 created D44231: [clang-tidy] Check for sizeof that call functions.
Mar 7 2018, 3:10 PM · Restricted Project

Apr 6 2015

pfultz2 added a comment to D8309: Improve error reporting for SFINAE.

The difference is, the current diagnostics allow the user to figure out what
happened. This change does not -- there's no way to find out which 'f' caused
the problem, and how we got from 'f' to 'g'.

Apr 6 2015, 3:06 PM
pfultz2 added a comment to D8309: Improve error reporting for SFINAE.

gives the user no idea of how we got from a call to f into a call to g.

Apr 6 2015, 2:07 PM
pfultz2 updated the diff for D8309: Improve error reporting for SFINAE.

Added tests

Apr 6 2015, 1:37 PM

Mar 14 2015

pfultz2 added a reviewer for D8309: Improve error reporting for SFINAE: doug.gregor.
Mar 14 2015, 12:18 PM

Mar 12 2015

pfultz2 updated D8309: Improve error reporting for SFINAE.
Mar 12 2015, 4:23 PM
pfultz2 updated subscribers of D8309: Improve error reporting for SFINAE.
Mar 12 2015, 4:22 PM
pfultz2 retitled D8309: Improve error reporting for SFINAE from to Improve error reporting for SFINAE.
Mar 12 2015, 4:21 PM