Page MenuHomePhabricator

LegalizeAdulthood (Richard)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 15 2013, 12:45 PM (462 w, 4 d)

Recent Activity

Yesterday

LegalizeAdulthood updated the diff for D125622: [clang-tidy] Reject invalid enum initializers in C files.
  • Add C++ test case for acceptable operator, initializer
Fri, May 27, 5:55 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D125622: [clang-tidy] Reject invalid enum initializers in C files.
Fri, May 27, 5:52 PM · Restricted Project, Restricted Project
LegalizeAdulthood updated the diff for D125622: [clang-tidy] Reject invalid enum initializers in C files.
  • Update from review comments
  • Disallow operator, unless inside parentheses
Fri, May 27, 5:43 PM · Restricted Project, Restricted Project

Thu, May 26

LegalizeAdulthood requested review of D126495: [clang-tidy] Organize test docs into subdirectories by module (NFC).
Thu, May 26, 12:18 PM · Restricted Project, Restricted Project

Mon, May 23

LegalizeAdulthood added inline comments to D126247: `readability-indentifier-naming` resolution order and examples.
Mon, May 23, 8:33 PM · Restricted Project, Restricted Project
LegalizeAdulthood committed rG89e663c4f83a: [clang-tidy] Improve add_new_check.py to recognize more checks (authored by LegalizeAdulthood).
[clang-tidy] Improve add_new_check.py to recognize more checks
Mon, May 23, 8:49 AM · Restricted Project, Restricted Project
LegalizeAdulthood closed D126134: [clang-tidy] Improve add_new_check.py to recognize more checks.
Mon, May 23, 8:48 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D126134: [clang-tidy] Improve add_new_check.py to recognize more checks.
Mon, May 23, 8:17 AM · Restricted Project, Restricted Project
LegalizeAdulthood updated the diff for D126134: [clang-tidy] Improve add_new_check.py to recognize more checks.

Update from review comments

Mon, May 23, 8:15 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D126134: [clang-tidy] Improve add_new_check.py to recognize more checks.
Mon, May 23, 8:05 AM · Restricted Project, Restricted Project
LegalizeAdulthood added a comment to D126134: [clang-tidy] Improve add_new_check.py to recognize more checks.

Another observation:
If some new pattern comes up and fixits aren't recognized for a check, it might be better
to switch to a whitelist for checks with fixits rather than going crazier on the file scraping.

Mon, May 23, 7:59 AM · Restricted Project, Restricted Project
LegalizeAdulthood accepted D126162: [clang-tidy] Extend SimplifyBooleanExpr demorgan support..

LGTM

Mon, May 23, 7:50 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D126134: [clang-tidy] Improve add_new_check.py to recognize more checks.
Mon, May 23, 7:40 AM · Restricted Project, Restricted Project

Sat, May 21

LegalizeAdulthood requested review of D126134: [clang-tidy] Improve add_new_check.py to recognize more checks.
Sat, May 21, 4:38 PM · Restricted Project, Restricted Project
LegalizeAdulthood accepted D125771: [clang-tidy] Add a useful note about -std=c++11-or-later.
Sat, May 21, 1:01 PM · Restricted Project, Restricted Project

Fri, May 20

LegalizeAdulthood added inline comments to D125771: [clang-tidy] Add a useful note about -std=c++11-or-later.
Fri, May 20, 1:59 PM · Restricted Project, Restricted Project
Herald added a project to D117460: [clang-tidy][NFC] Reduce map lookups in IncludeSorter: Restricted Project.

Are we waiting on some sort of benchmark before accepting/rejecting this change?

Fri, May 20, 10:22 AM · Restricted Project, Restricted Project
LegalizeAdulthood accepted D117409: [clang-tidy] Move IncludeInserter into ClangTidyContext.

If the only thing holding this up is my comments about following the Law of Demeter, I don't think that should block submitting this change.

Fri, May 20, 10:20 AM · Restricted Project, Restricted Project
LegalizeAdulthood accepted D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks.

LGTM

Fri, May 20, 10:16 AM · Restricted Project, Restricted Project
LegalizeAdulthood accepted D125769: [clang-tidy] Introduce the CheckHeaderFile option to modernize-deprecated-headers.

LGTM
Address one nit and ship.

Fri, May 20, 10:12 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D125769: [clang-tidy] Introduce the CheckHeaderFile option to modernize-deprecated-headers.
Fri, May 20, 10:12 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D125771: [clang-tidy] Add a useful note about -std=c++11-or-later.
Fri, May 20, 9:48 AM · Restricted Project, Restricted Project

Thu, May 19

LegalizeAdulthood added inline comments to D125769: [clang-tidy] Introduce the CheckHeaderFile option to modernize-deprecated-headers.
Thu, May 19, 10:07 AM · Restricted Project, Restricted Project
LegalizeAdulthood added a comment to D125771: [clang-tidy] Add a useful note about -std=c++11-or-later.

Tbh the whole testing infrastructure for clang-tidy is a mess. When I have wanted to verify fixes and diagnostics for header files, I find manually invoking clang-tidy is better than using the check_clang_tidy script.
I don't have the time right now, but a verify mode like with clang is something that we should move to in the future.

Thu, May 19, 10:02 AM · Restricted Project, Restricted Project

Wed, May 18

LegalizeAdulthood added inline comments to D121372: [clang-tidy][docs][NFC] Update URL and docs of PostfixOperatorCheck.
Wed, May 18, 11:59 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D121372: [clang-tidy][docs][NFC] Update URL and docs of PostfixOperatorCheck.
Wed, May 18, 11:58 PM · Restricted Project, Restricted Project
LegalizeAdulthood added a comment to D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr.

LGTM! Thanks for taking my idea and implementing it much better than what I had :).

Cheers.
Can I ask why you feel that this should be behind an option?

Wed, May 18, 2:10 PM · Restricted Project, Restricted Project
LegalizeAdulthood accepted D125770: [clang-tidy] modernize-deprecated-headers should ignore system headers.

LGTM

Wed, May 18, 2:05 PM · Restricted Project, Restricted Project
LegalizeAdulthood abandoned D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem.

Discarding in favor of Nathan James's improved implementation.

Wed, May 18, 11:15 AM · Restricted Project, Restricted Project
LegalizeAdulthood accepted D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr.

LGTM! Thanks for taking my idea and implementing it much better than what I had :).

Wed, May 18, 11:15 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D125770: [clang-tidy] modernize-deprecated-headers should ignore system headers.
Wed, May 18, 8:45 AM · Restricted Project, Restricted Project
LegalizeAdulthood accepted D125885: [clang-tidy] bugprone-argument-comment: Ignore calls to user-defined literals.

LGTM

Wed, May 18, 8:27 AM · Restricted Project, Restricted Project
LegalizeAdulthood accepted D125877: [clang-tidy] Fix readability-simplify-boolean-expr crash with implicit cast in return..

Another good catch!

Wed, May 18, 8:25 AM · Restricted Project, Restricted Project
LegalizeAdulthood accepted D125874: [clang-tidy] Fix readability-simplify-boolean-expr when Ifs have an init statement or condition variable.

Good catch!

Wed, May 18, 8:23 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D125770: [clang-tidy] modernize-deprecated-headers should ignore system headers.
Wed, May 18, 8:21 AM · Restricted Project, Restricted Project

Tue, May 17

LegalizeAdulthood added a comment to D125026: [clang-tidy][NFC] Reimplement SimplifyBooleanExpr with RecursiveASTVisitors.

Just for my own edification, how did you know/suspect that a pure visitor would be faster than matchers?

Tue, May 17, 10:07 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124918: [clang-tidy] Add a new check for non-trivial unused variables..
Tue, May 17, 10:00 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D125770: [clang-tidy] modernize-deprecated-headers should ignore system headers.
Tue, May 17, 9:58 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D125769: [clang-tidy] Introduce the CheckHeaderFile option to modernize-deprecated-headers.
Tue, May 17, 9:51 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks.
Tue, May 17, 9:38 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124918: [clang-tidy] Add a new check for non-trivial unused variables..
Tue, May 17, 9:28 AM · Restricted Project, Restricted Project
LegalizeAdulthood added a comment to D125771: [clang-tidy] Add a useful note about -std=c++11-or-later.

I thought there wasn't any support for validating fixits applied to header files?

Tue, May 17, 9:07 AM · Restricted Project, Restricted Project

Mon, May 16

LegalizeAdulthood added inline comments to D125622: [clang-tidy] Reject invalid enum initializers in C files.
Mon, May 16, 9:38 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D125622: [clang-tidy] Reject invalid enum initializers in C files.
Mon, May 16, 10:12 AM · Restricted Project, Restricted Project

Sat, May 14

LegalizeAdulthood requested review of D125622: [clang-tidy] Reject invalid enum initializers in C files.
Sat, May 14, 7:49 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Sat, May 14, 5:28 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Sat, May 14, 1:48 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Sat, May 14, 1:47 PM · Restricted Project, Restricted Project
LegalizeAdulthood committed rG9d99cf59a151: [clang-tidy] Restore test parameter operator<< function (NFC) (authored by LegalizeAdulthood).
[clang-tidy] Restore test parameter operator<< function (NFC)
Sat, May 14, 1:05 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124918: [clang-tidy] Add a new check for non-trivial unused variables..
Sat, May 14, 12:27 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124341: [clang-tidy][NFC] Replace many instances of std::string where a StringRef would suffice..
Sat, May 14, 11:56 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Sat, May 14, 11:50 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Sat, May 14, 11:27 AM · Restricted Project, Restricted Project

Fri, May 13

LegalizeAdulthood committed rG512273833136: [clang-tidy] Support expressions of literals in modernize-macro-to-enum (authored by LegalizeAdulthood).
[clang-tidy] Support expressions of literals in modernize-macro-to-enum
Fri, May 13, 5:47 PM · Restricted Project, Restricted Project
LegalizeAdulthood closed D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Fri, May 13, 5:46 PM · Restricted Project, Restricted Project
LegalizeAdulthood updated the diff for D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.

Update documentation from review comments

Fri, May 13, 2:13 PM · Restricted Project, Restricted Project

Mon, May 9

LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Mon, May 9, 8:32 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Mon, May 9, 8:29 AM · Restricted Project, Restricted Project

Sun, May 8

LegalizeAdulthood added a comment to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.

OK, so thinking about this review a little more, I propose this:

  • Take the check as is, but document that the initializing expressions may result in an invalid enum, particularly for C which restricts the underlying type to be int
  • Create a subsequent commit that rejects the enums where the language is C and the initializing expression is a value larger than an int by rejecting any macro where any integer token in the expression is larger than an int
  • Create an additional subsequent commit that not only matches the expression but also computes the value and checks it for range.
Sun, May 8, 7:38 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Sun, May 8, 7:24 PM · Restricted Project, Restricted Project

Mon, May 2

LegalizeAdulthood added a comment to D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem.

I've done my own implementation, but its definitely over engineered, WDYT? D124806

Mon, May 2, 6:20 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Mon, May 2, 1:19 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Mon, May 2, 1:11 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem.
Mon, May 2, 9:55 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Mon, May 2, 9:44 AM · Restricted Project, Restricted Project
LegalizeAdulthood added a comment to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.

To clarify my previous comments, I'm fine punting on the edge cases until user reports come in, so don't let them block this review if you feel strongly about not supporting them. But when user reports start coming in, at some point I might start asking to replace the custom parser with calling into the clangParse library through some invented utility interface so that we don't have to deal with a long tail of bug reports.

Yeah, I think punting on edge cases is the right thing to do here. As I say,
the worst that happens is your macro isn't converted automatically when you
could convert it manually.

I largely agree, but I've found cases where we'll convert correct code to incorrect code, so it's a bit stronger than that.

Mon, May 2, 9:15 AM · Restricted Project, Restricted Project

Sun, May 1

LegalizeAdulthood added inline comments to D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem.
Sun, May 1, 1:56 PM · Restricted Project, Restricted Project
LegalizeAdulthood added a comment to D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem.

I also noticed that some of the simplifications being performed resulted in !(x || !y) coming out of other parts of the simplifier, so it should be the case that running this check on your code should result in no further changes to your code if you run the check again.

Sun, May 1, 1:54 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem.
Sun, May 1, 11:26 AM · Restricted Project, Restricted Project
LegalizeAdulthood added a comment to D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem.

Testing on VTK revealed this change:

-  this->SliceMapper->SetBackground((this->Background &&
-    !(this->SliceFacesCamera && this->InternalResampleToScreenPixels &&
-      !this->SeparateWindowLevelOperation)));
+  this->SliceMapper->SetBackground(
+      (this->Background &&
+       (!this->SliceFacesCamera && this->InternalResampleToScreenPixels ||
+        this->SeparateWindowLevelOperation)));

Which is incorrect. So more improvement is needed for the case of !(x && y && !z) or !(x || y || !z) is encountered.

Sun, May 1, 11:07 AM · Restricted Project, Restricted Project

Sat, Apr 30

LegalizeAdulthood added a comment to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.

FYI, once nice addition of the parsing of macro bodies is that it paves the way for
a modernize-macro-to-function check that converts function-like macros that
compute values to template functions. Once this change has landed, I'll be putting
up a review for that.

Sat, Apr 30, 9:23 PM · Restricted Project, Restricted Project
LegalizeAdulthood updated the diff for D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.

Recognize comma operator expressions

Sat, Apr 30, 9:04 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Sat, Apr 30, 9:03 PM · Restricted Project, Restricted Project
LegalizeAdulthood added a reviewer for D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum: njames93.
Sat, Apr 30, 11:22 AM · Restricted Project, Restricted Project

Fri, Apr 29

LegalizeAdulthood updated the summary of D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem.
Fri, Apr 29, 2:09 PM · Restricted Project, Restricted Project
LegalizeAdulthood updated the diff for D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem.

Surround replacements with parens

Fri, Apr 29, 2:08 PM · Restricted Project, Restricted Project
LegalizeAdulthood added a comment to D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem.

General question:

There is the question of whether or not the replacement should have ()s around the whole
thing because we don't analyze the context to see if the change in precedence between
operator! and operator&& or operator|| makes any difference.

Fri, Apr 29, 1:56 PM · Restricted Project, Restricted Project
LegalizeAdulthood updated the diff for D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.

Update from review comments

Fri, Apr 29, 1:44 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Fri, Apr 29, 1:19 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Fri, Apr 29, 1:02 PM · Restricted Project, Restricted Project
LegalizeAdulthood added a comment to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.

To clarify my previous comments, I'm fine punting on the edge cases until user reports come in, so don't let them block this review if you feel strongly about not supporting them. But when user reports start coming in, at some point I might start asking to replace the custom parser with calling into the clangParse library through some invented utility interface so that we don't have to deal with a long tail of bug reports.

Fri, Apr 29, 1:00 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Fri, Apr 29, 12:40 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Fri, Apr 29, 10:45 AM · Restricted Project, Restricted Project
LegalizeAdulthood updated the diff for D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem.

Sort changes by check name in docs

Fri, Apr 29, 10:29 AM · Restricted Project, Restricted Project

Thu, Apr 28

LegalizeAdulthood added a comment to D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem.

General question:

Thu, Apr 28, 5:32 PM · Restricted Project, Restricted Project
LegalizeAdulthood requested review of D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem.
Thu, Apr 28, 5:30 PM · Restricted Project, Restricted Project

Apr 27 2022

LegalizeAdulthood updated the diff for D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.

Update documentation

Apr 27 2022, 11:19 PM · Restricted Project, Restricted Project
LegalizeAdulthood updated the diff for D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.

Add file block comment

Apr 27 2022, 6:59 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124341: [clang-tidy][NFC] Replace many instances of std::string where a StringRef would suffice..
Apr 27 2022, 10:28 AM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Apr 27 2022, 8:35 AM · Restricted Project, Restricted Project

Apr 26 2022

LegalizeAdulthood updated the diff for D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.

Inline Variable

Apr 26 2022, 11:49 PM · Restricted Project, Restricted Project
LegalizeAdulthood updated the diff for D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
  • Add banner block comment for new files
  • Extract Functions to eliminate duplication
Apr 26 2022, 11:47 PM · Restricted Project, Restricted Project
LegalizeAdulthood added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Apr 26 2022, 8:32 PM · Restricted Project, Restricted Project
LegalizeAdulthood set the repository for D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum to rG LLVM Github Monorepo.
Apr 26 2022, 8:29 PM · Restricted Project, Restricted Project
LegalizeAdulthood requested review of D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Apr 26 2022, 8:29 PM · Restricted Project, Restricted Project
LegalizeAdulthood committed rG693246e03f28: [clang-tidy] Modernize-macro-to-enum should skip macros used in other macros (authored by LegalizeAdulthood).
[clang-tidy] Modernize-macro-to-enum should skip macros used in other macros
Apr 26 2022, 8:10 PM · Restricted Project, Restricted Project
LegalizeAdulthood closed D124316: [clang-tidy] Modernize-macro-to-enum should skip macros used in other macros.
Apr 26 2022, 8:10 PM · Restricted Project, Restricted Project
LegalizeAdulthood updated the diff for D124316: [clang-tidy] Modernize-macro-to-enum should skip macros used in other macros.

Update from review comments

Apr 26 2022, 8:07 PM · Restricted Project, Restricted Project

Apr 25 2022

LegalizeAdulthood added a comment to D124316: [clang-tidy] Modernize-macro-to-enum should skip macros used in other macros.

Gentle ping.

Apr 25 2022, 1:39 PM · Restricted Project, Restricted Project

Apr 22 2022

LegalizeAdulthood requested review of D124316: [clang-tidy] Modernize-macro-to-enum should skip macros used in other macros.
Apr 22 2022, 6:41 PM · Restricted Project, Restricted Project