Page MenuHomePhabricator

logan-5 (Logan Smith)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 1 2019, 8:51 AM (29 w, 1 d)

Recent Activity

Fri, Jan 17

logan-5 added a comment to rGbcda877b4309: Fix a compile error to get bots back to green..

Correct, it's not supported for MSVC yet. I just tried to enable it to see if MSVC 2019 has support and I get errors:

C:\llvm-project\llvm\include\llvm\ADT\Optional.h(290): error C2560: 'llvm::Optional<unknown-type> llvm::Optional<T>::map(const Function &) &&': cannot overload a member function with ref-qualifier with a member function without ref-qualifier

Perhaps the map overload just above that simply needs LLVM_LVALUE_FUNCTION on it? Like how the other getValueOr() and operator*s do.

Fri, Jan 17, 10:32 AM
logan-5 added a comment to rGbcda877b4309: Fix a compile error to get bots back to green..

That's a bummer. I wrote *std::move(Fixup) intentionally that way, since llvm::Optional has a &&-qualified operator* overload that returns std::move of the contained value. I guess the Windows configuration doesn't have LLVM_HAS_RVALUE_REFERENCE_THIS?

Fri, Jan 17, 10:13 AM

Thu, Jan 16

logan-5 updated the diff for D72378: [clang-tidy] Add `bugprone-reserved-identifier`.

Should be good now.

Thu, Jan 16, 3:13 PM · Restricted Project, Restricted Project
logan-5 updated the diff for D72640: [libcxx] Qualify make_pair in searcher implementations to prevent ADL.

Fixed some wrong author information in the patch.

Thu, Jan 16, 2:15 PM · Restricted Project
logan-5 added a comment to D72378: [clang-tidy] Add `bugprone-reserved-identifier`.

Great! In that case, I'll need help committing this, as well as the thing it depends on, https://reviews.llvm.org/D72284 (which has also been LGTM'd).

Thu, Jan 16, 10:51 AM · Restricted Project, Restricted Project

Wed, Jan 15

logan-5 updated the diff for D72282: [clang-tidy] Add `bugprone-unintended-adl`.

Added TODO comment.

Wed, Jan 15, 11:55 AM · Restricted Project, Restricted Project
logan-5 added inline comments to D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming.
Wed, Jan 15, 11:46 AM · Restricted Project, Restricted Project

Tue, Jan 14

logan-5 added inline comments to D72378: [clang-tidy] Add `bugprone-reserved-identifier`.
Tue, Jan 14, 12:12 PM · Restricted Project, Restricted Project
logan-5 updated the diff for D72378: [clang-tidy] Add `bugprone-reserved-identifier`.

Renamed check option from "Whitelist" to "AllowedIdentifiers". Added note about missing checks in documentation. Changed to use a %select for diagnostic text. Some nits.

Tue, Jan 14, 12:12 PM · Restricted Project, Restricted Project
logan-5 added inline comments to D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming.
Tue, Jan 14, 11:23 AM · Restricted Project, Restricted Project
logan-5 updated the diff for D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming.

Rebased with trunk.

Tue, Jan 14, 9:55 AM · Restricted Project, Restricted Project

Mon, Jan 13

logan-5 created D72640: [libcxx] Qualify make_pair in searcher implementations to prevent ADL.
Mon, Jan 13, 12:37 PM · Restricted Project

Fri, Jan 10

logan-5 updated the diff for D72378: [clang-tidy] Add `bugprone-reserved-identifier`.

Added a TODO comment for catching more reserved names. Added links in documentation to CERT guidelines covered by the check. Pulled strings into named constants and made that logic easier to read.

Fri, Jan 10, 2:25 PM · Restricted Project, Restricted Project

Thu, Jan 9

logan-5 updated the diff for D72378: [clang-tidy] Add `bugprone-reserved-identifier`.

Addressed nits. Added CERT aliases. Adjusted the check to work for both C and C++, including where they differ.

Thu, Jan 9, 10:49 PM · Restricted Project, Restricted Project
logan-5 added inline comments to D72282: [clang-tidy] Add `bugprone-unintended-adl`.
Thu, Jan 9, 10:42 AM · Restricted Project, Restricted Project
logan-5 added a comment to D72378: [clang-tidy] Add `bugprone-reserved-identifier`.

This check is missing a whole lot of reserved identifiers. For instance, in C++ it is missing everything from http://eel.is/c++draft/zombie.names and for C it is missing everything from future library directions. Are you intending to cover those cases as well?

Thu, Jan 9, 10:42 AM · Restricted Project, Restricted Project

Wed, Jan 8

logan-5 updated the diff for D72378: [clang-tidy] Add `bugprone-reserved-identifier`.

Added tests for template parameters.

Wed, Jan 8, 12:51 PM · Restricted Project, Restricted Project
logan-5 added a comment to D72282: [clang-tidy] Add `bugprone-unintended-adl`.

Hi @Quuxplusone, glad you found your way here. I thought of adding you as a reviewer out the gate but then I didn't.

Wed, Jan 8, 12:32 PM · Restricted Project, Restricted Project
logan-5 updated the diff for D72282: [clang-tidy] Add `bugprone-unintended-adl`.

Beefed up tests and split into separate files. Added tests for instantiations of template functions that do definitely result in ADL, and made the warning messages more descriptive in those cases (listing out concrete types of arguments in a note).

Wed, Jan 8, 12:32 PM · Restricted Project, Restricted Project

Tue, Jan 7

logan-5 updated the diff for D72378: [clang-tidy] Add `bugprone-reserved-identifier`.

Addressed some nits.

Tue, Jan 7, 8:26 PM · Restricted Project, Restricted Project
logan-5 created D72378: [clang-tidy] Add `bugprone-reserved-identifier`.
Tue, Jan 7, 5:34 PM · Restricted Project, Restricted Project
logan-5 added a comment to D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming.

(Just in case this got lost in the shuffle)
If this looks good, (I think) I'll need someone with commit access to help wrap it up.

Tue, Jan 7, 5:32 PM · Restricted Project, Restricted Project
logan-5 added a comment to D72282: [clang-tidy] Add `bugprone-unintended-adl`.

@JonasToth Thanks for the feedback. Will be updating the diff in the next day or so.

Tue, Jan 7, 4:55 PM · Restricted Project, Restricted Project
logan-5 updated the diff for D72282: [clang-tidy] Add `bugprone-unintended-adl`.

Clean up some false positives in macro expansions and in expressions with nested potential ADL usages (e.g. in lambdas passed as function parameters).

Tue, Jan 7, 8:02 AM · Restricted Project, Restricted Project

Mon, Jan 6

logan-5 updated the diff for D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming.

Consistent-ified single-statement-body control flow statements to not use braces.

Mon, Jan 6, 5:10 PM · Restricted Project, Restricted Project
logan-5 added inline comments to D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming.
Mon, Jan 6, 4:34 PM · Restricted Project, Restricted Project
logan-5 updated the diff for D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming.

More nits.

Mon, Jan 6, 4:34 PM · Restricted Project, Restricted Project
logan-5 added inline comments to D72282: [clang-tidy] Add `bugprone-unintended-adl`.
Mon, Jan 6, 2:16 PM · Restricted Project, Restricted Project
logan-5 updated the diff for D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming.

Addressed some nits. If the rest looks good, I'll need someone with commit access to help me wrap this up.

Mon, Jan 6, 1:57 PM · Restricted Project, Restricted Project
logan-5 added inline comments to D72282: [clang-tidy] Add `bugprone-unintended-adl`.
Mon, Jan 6, 11:03 AM · Restricted Project, Restricted Project
logan-5 updated the diff for D72282: [clang-tidy] Add `bugprone-unintended-adl`.

Addressed some formatting stuff.

Mon, Jan 6, 11:03 AM · Restricted Project, Restricted Project
logan-5 abandoned D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names.
Mon, Jan 6, 9:37 AM · Restricted Project, Restricted Project
logan-5 created D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming.
Mon, Jan 6, 9:37 AM · Restricted Project, Restricted Project
logan-5 added a comment to D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names.

I'm taking @JonasToth's suggestion and splitting this into two patches, one for the refactor, followed by one for the new check. The refactor patch is here: https://reviews.llvm.org/D72284. Thanks everyone for your feedback so far.

Mon, Jan 6, 9:37 AM · Restricted Project, Restricted Project
logan-5 created D72282: [clang-tidy] Add `bugprone-unintended-adl`.
Mon, Jan 6, 9:07 AM · Restricted Project, Restricted Project

Sat, Jan 4

logan-5 updated the diff for D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names.

Change double-underscore fix-it finding algorithm to correctly collapse any number of >=2 underscores in a row, not just exactly 2 (or multiples of 2).

Sat, Jan 4, 8:45 PM · Restricted Project, Restricted Project
logan-5 added inline comments to D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names.
Sat, Jan 4, 5:52 PM · Restricted Project, Restricted Project
logan-5 updated the diff for D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names.

Addressed formatting issues, and tweaked handling of the names __ and ::_: the check now flags these but doesn't suggest a fix.

Sat, Jan 4, 5:43 PM · Restricted Project, Restricted Project
logan-5 added a comment to D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names.

@Eugene.Zelenko never mind about pushing back on the nits; I had misread some of them. They all sound good, will address.

Sat, Jan 4, 2:50 PM · Restricted Project, Restricted Project
logan-5 added inline comments to D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names.
Sat, Jan 4, 2:41 PM · Restricted Project, Restricted Project
logan-5 added a comment to D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names.

@Eugene.Zelenko +1 to @Mordante 's question. Otherwise, happy to address most of those nits, though I think I will gently push back on a couple of them (in the inline comments).

Sat, Jan 4, 2:24 PM · Restricted Project, Restricted Project
logan-5 added a comment to D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names.

I wonder what happens if the project already contains a suggested fix, for example:

#define __FOO(X) ...
#define _FOO(X) __FOO(X)
#define FOO(X) _FOO(X)

will it suggest:

#define FOO(X) ...
#define FOO(X) FOO(X)
#define FOO(X) FOO(X)

?

Sat, Jan 4, 2:23 PM · Restricted Project, Restricted Project
logan-5 created D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names.
Sat, Jan 4, 1:01 PM · Restricted Project, Restricted Project

Mon, Dec 30

logan-5 added a comment to D69603: [libcxx] Add deduction guides for shared_ptr and weak_ptr.

Thanks Marshall. As for www/cxx1z_status.html, my best guess is that it doesn't need updating, since the row for P0433R2 lists it as In progress, which I believe is still the case after this patch. Please correct me if I'm wrong. Otherwise, I'd be grateful if someone with commit access could help me tie a bow around this.

Mon, Dec 30, 8:22 AM

Sun, Dec 29

logan-5 added a comment to D69603: [libcxx] Add deduction guides for shared_ptr and weak_ptr.

Hello! Pinging this again.

Sun, Dec 29, 2:04 PM

Nov 18 2019

logan-5 added a comment to D69603: [libcxx] Add deduction guides for shared_ptr and weak_ptr.

ping

Nov 18 2019, 11:18 AM

Nov 1 2019

logan-5 updated the diff for D69603: [libcxx] Add deduction guides for shared_ptr and weak_ptr.

Beefed up the tests in response to some feedback -- thanks.

Nov 1 2019, 11:45 AM

Oct 30 2019

logan-5 updated the diff for D69603: [libcxx] Add deduction guides for shared_ptr and weak_ptr.

Add a newline to synopsis

Oct 30 2019, 10:31 AM

Oct 29 2019

logan-5 created D69603: [libcxx] Add deduction guides for shared_ptr and weak_ptr.
Oct 29 2019, 10:03 PM

Aug 29 2019

logan-5 added a comment to D66621: [clang] Devirtualization for classes with destructors marked as 'final'.

ping

Aug 29 2019, 9:21 AM · Restricted Project, Restricted Project

Aug 27 2019

logan-5 added a comment to D66711: [clang] Warning for non-final classes with final destructors.

Thanks. I'll need someone with commit access to help me out with this.

Aug 27 2019, 11:50 AM · Restricted Project, Restricted Project

Aug 25 2019

logan-5 updated the diff for D66711: [clang] Warning for non-final classes with final destructors.

Addressed some feedback.

Aug 25 2019, 9:21 AM · Restricted Project, Restricted Project

Aug 24 2019

logan-5 created D66711: [clang] Warning for non-final classes with final destructors.
Aug 24 2019, 7:56 PM · Restricted Project, Restricted Project

Aug 23 2019

logan-5 updated the diff for D66621: [clang] Devirtualization for classes with destructors marked as 'final'.

Add a missing null check.

Aug 23 2019, 10:17 PM · Restricted Project, Restricted Project

Aug 22 2019

logan-5 added a comment to D66621: [clang] Devirtualization for classes with destructors marked as 'final'.

@rsmith I agree having a final destructor in a non-final class smells weird. I'd be interested in working on adding a warning like that, if it sounds like a useful thing.

Aug 22 2019, 6:58 PM · Restricted Project, Restricted Project
logan-5 updated the diff for D66621: [clang] Devirtualization for classes with destructors marked as 'final'.

Tweaked comment wording for accuracy.

Aug 22 2019, 6:57 PM · Restricted Project, Restricted Project
logan-5 added a comment to D66621: [clang] Devirtualization for classes with destructors marked as 'final'.

That appears sort of tangential to me. To clarify, this PR is not (necessarily) about devirtualizing destructors themselves, but rather devirtualizing OTHER member function calls for classes whose destructor is marked final, since that is sort of morally equivalent to marking the entire class final.

Aug 22 2019, 3:12 PM · Restricted Project, Restricted Project
logan-5 created D66621: [clang] Devirtualization for classes with destructors marked as 'final'.
Aug 22 2019, 2:21 PM · Restricted Project, Restricted Project

Jul 8 2019

logan-5 added a comment to D64058: [cxx2a] P0624R2 fix: only lambdas with no lambda-capture are default-constructible and assignable..

I believe I need someone to commit it for me... brand new to all this and unsure, frankly.

Jul 8 2019, 12:48 PM · Restricted Project, Restricted Project
logan-5 added a comment to D64058: [cxx2a] P0624R2 fix: only lambdas with no lambda-capture are default-constructible and assignable..

I had the exact same thought about factoring out this and the function pointer conversion condition. I didn't bother, but I agree it could be a useful function to have.

Jul 8 2019, 12:48 PM · Restricted Project, Restricted Project
logan-5 added a comment to D64058: [cxx2a] P0624R2 fix: only lambdas with no lambda-capture are default-constructible and assignable..

*ping*

Jul 8 2019, 10:28 AM · Restricted Project, Restricted Project

Jul 1 2019

logan-5 created D64058: [cxx2a] P0624R2 fix: only lambdas with no lambda-capture are default-constructible and assignable..
Jul 1 2019, 9:16 PM · Restricted Project, Restricted Project