Page MenuHomePhabricator

njames93 (Nathan James)
Dodd

Projects

User does not belong to any projects.

User Details

User Since
Dec 23 2019, 11:05 AM (76 w, 5 d)

Recent Activity

Sat, May 29

njames93 added a comment to D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias.

Is it worth using regex to ignore some operators:
*::operator bool to ignore all bool conversions etc.

Sat, May 29, 12:08 AM · Restricted Project

Sun, May 16

njames93 added a comment to D102576: [clang-tidy] cppcoreguidelines-avoid-do-while: a new check.

This is going to be very noisey for codebases that use a do...while macro:

#define MACRO() do { Something(); With(); Multiple(); Statements(); } while (false)
Sun, May 16, 11:24 AM · Restricted Project

May 13 2021

njames93 added a comment to D102369: [ASTMatchers][NFC] Remove runtime checks where compile time is sufficient.

I'm not opposed, but I don't know that this is really an improvement. We've gone from a pretty simple code pattern to needing to spell out the type twice with a type_trait, and all that we save is a call to isa<> within the casting operation. While I agree this is strictly executing less code, it's morally the same as the usual antipattern of "isa followed by cast should be a dyn_cast". Is there evidence that this is a big performance win?

May 13 2021, 4:28 AM · Restricted Project

May 12 2021

njames93 requested review of D102369: [ASTMatchers][NFC] Remove runtime checks where compile time is sufficient.
May 12 2021, 2:18 PM · Restricted Project
njames93 accepted D102337: [clang-tidy] Fix test that requires Windows platofrm.

LGTM.

May 12 2021, 1:26 PM · Restricted Project
njames93 added a comment to D86671: [clang-tidy] Add new case type to check variables with Hungarian notation.

Hi @njames93:
Could you do me a favor? Because it is my first patch, something I'm not sure. I'm confused about can I land this patch now? I read the "LLVM Code-Review Policy and Practices" document, it said patches can be landed if received a LGTM, but seems you are still reviewing.

May 12 2021, 10:23 AM · Restricted Project, Restricted Project, Restricted Project
njames93 accepted D97567: [clang-tidy] performance-* checks: Also allow allow member expressions to be used in a const manner..

LGTM, but with a very minor nit.

May 12 2021, 8:28 AM · Restricted Project, Restricted Project
njames93 added a comment to D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check.

Whats the intended behaviour for derived classes and their destructors? Can test be added to demonstrate that behaviour?

May 12 2021, 8:17 AM · Restricted Project
njames93 committed rG4c59ab34f7bd: [clang-tidy][NFC] Simplify a lot of bugprone-sizeof-expression matchers (authored by njames93).
[clang-tidy][NFC] Simplify a lot of bugprone-sizeof-expression matchers
May 12 2021, 5:19 AM
njames93 closed D101614: [clang-tidy][NFC] Simplify a lot of bugprone-sizeof-expression matchers.
May 12 2021, 5:18 AM · Restricted Project
njames93 added a comment to D102322: [clang-tidy] Add '-target' CLI option to override target triple.

This change doesn't seem to accomplish anything. We can already set the target via the compile command.
This can be done in the compilation database directly or by using the extra-arg and extra-arg-before command line arguments.
If you're not using a compilation database, then you can pass the compile command to clang-tidy after an empty -- argument.

May 12 2021, 5:08 AM · Restricted Project
njames93 committed rG163325086c35: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule (authored by h-joo).
[clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule
May 12 2021, 4:57 AM
njames93 closed D101239: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule.
May 12 2021, 4:57 AM · Restricted Project

May 10 2021

njames93 added a comment to D101239: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule.

LGTM, thanks.

@njames93, I appreciate your time for the review. May I ask one more thing? I do not have commit rights. Would it be possible for you to make the commit? Thank you!!

No problem, Can you please provide your GitHub username and email so I can mark you as the author of the patch.

May 10 2021, 1:50 AM · Restricted Project

May 9 2021

njames93 accepted D101239: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule.

LGTM, thanks.

May 9 2021, 8:54 AM · Restricted Project

May 8 2021

njames93 added a comment to D101239: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule.

Whats the expected behaviour of sugar types. Can tests be added that demonstrate the behaviour.
I'd argue these cases shouldn't be warned on if IgnoreArrays is enabled and If that behaviour isn't observed currently it should be addressed.

May 8 2021, 10:45 AM · Restricted Project

May 5 2021

njames93 added inline comments to D101259: [clang-tidy] Fix cppcoreguidelines-pro-type-vararg false positives with __builtin_ms_va_list.
May 5 2021, 6:29 AM · Restricted Project

May 4 2021

njames93 committed rG61dc0f2b593d: [Format] Don't sort includes if DisableFormat is true (authored by njames93).
[Format] Don't sort includes if DisableFormat is true
May 4 2021, 11:04 AM
njames93 closed D101628: [Format] Don't sort includes if DisableFormat is true.
May 4 2021, 11:04 AM · Restricted Project, Restricted Project
njames93 committed rGe1c729c56829: [clang-tidy][NFC] Update tests and Default options to use boolean value (authored by njames93).
[clang-tidy][NFC] Update tests and Default options to use boolean value
May 4 2021, 10:18 AM
njames93 closed D101721: [clang-tidy][NFC] Update tests and Default options to use boolean value.
May 4 2021, 10:18 AM · Restricted Project
njames93 updated the diff for D101628: [Format] Don't sort includes if DisableFormat is true.

Add test case demonstrating new behvaiour.

May 4 2021, 9:34 AM · Restricted Project, Restricted Project

May 3 2021

njames93 added a comment to D96215: [clang-tidy] Aliasing: Add support for lambda captures..
In D96215#2550227, @NoQ wrote:

Umm, looks like we're both missing the elephant in the room: passing a variable into a function by reference.

int &hidden_reference(int &x) {
  return x;
}

void test_hidden_reference() {
  int x = 0;
  int &y = hidden_reference(x);
  for (; x < 10; ++y) { // Warns ¯\_(ツ)_/¯
  }
}

With this taken care of, I hope structured bindings will be handled automatically because whatever's unwrapped couldn't have obtained a reference to our local variable without going through a function first (eg., the constructor). Unless there's some aggregate magic going on... but in this case, again, we have to take care of aggregate magic and structured bindings aren't at fault.

May 3 2021, 11:06 PM · Restricted Project
njames93 added a comment to D101628: [Format] Don't sort includes if DisableFormat is true.

Is it testable?

May 3 2021, 3:03 PM · Restricted Project, Restricted Project
njames93 added a comment to D101721: [clang-tidy][NFC] Update tests and Default options to use boolean value.

That makes sense to me. Should we file a bug to suggest adding the deprecation warning in Clang 14(?) and planned removal in Clang 16(?) so that we don't lose track of this? (I have no firm opinion about which versions we decide to start deprecating and remove so long as they're not disruptive.)

May 3 2021, 10:58 AM · Restricted Project
njames93 added inline comments to D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck..
May 3 2021, 10:48 AM · Restricted Project
njames93 added a comment to D101721: [clang-tidy][NFC] Update tests and Default options to use boolean value.

However in a few years once we can be confident most users are using clang-tidy-11 or newer, it may be wise to drop support for 0 and 1 in order to be inline with yaml completely.

I think if we want to go that route (which seems sensible to me), we should start warning on using anything but true/false as being deprecated. WDYT?

That's sort of the plan, however we shouldn't make that change right away as there's no point in issuing warnings at this time. As configurations are checked in there is likely to be people still using 10 and previous, which don't support the new spelling. This means the config can't be updated and users with newer clang-tidy versions will get a warning they can't silence.

May 3 2021, 5:40 AM · Restricted Project
njames93 added a comment to D101721: [clang-tidy][NFC] Update tests and Default options to use boolean value.

LGTM! One thing I'd like to be sure of though -- do we still have at least one test that's showing you can use false/0 and true/1/nonzero interchangeably? If not, we should probably have one that shows which "alternate forms" are accepted.

clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp::CheckOptionsValidation::ValidIntOptions contains a test which supports 0 and 1.
However in a few years once we can be confident most users are using clang-tidy-11 or newer, it may be wise to drop support for 0 and 1 in order to be inline with yaml completely.

May 3 2021, 4:48 AM · Restricted Project
njames93 committed rG53df522a0c53: [clang-tidy][NFC] Short circuit getting enum options suggestions. (authored by njames93).
[clang-tidy][NFC] Short circuit getting enum options suggestions.
May 3 2021, 3:21 AM
njames93 added inline comments to D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check.
May 3 2021, 3:17 AM · Restricted Project

May 2 2021

njames93 requested review of D101721: [clang-tidy][NFC] Update tests and Default options to use boolean value.
May 2 2021, 5:25 AM · Restricted Project
njames93 added reviewers for D101239: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule: aaron.ballman, njames93.
May 2 2021, 3:07 AM · Restricted Project
njames93 added inline comments to D86671: [clang-tidy] Add new case type to check variables with Hungarian notation.
May 2 2021, 1:37 AM · Restricted Project, Restricted Project, Restricted Project

May 1 2021

njames93 updated the diff for D101614: [clang-tidy][NFC] Simplify a lot of bugprone-sizeof-expression matchers.

Update.

May 1 2021, 1:28 AM · Restricted Project
njames93 added a comment to D86671: [clang-tidy] Add new case type to check variables with Hungarian notation.

Sorry, I thought this landed months ago. I'll take a proper look again next week.

May 1 2021, 1:28 AM · Restricted Project, Restricted Project, Restricted Project
njames93 committed rG172a8016788c: [clang-tidy][NFC] Remove redudnant expr and qualType matchers from bugprone… (authored by njames93).
[clang-tidy][NFC] Remove redudnant expr and qualType matchers from bugprone…
May 1 2021, 12:54 AM

Apr 30 2021

njames93 retitled D101239: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule from Enable the use of IgnoreArray flag in pro-type-member-init rule to [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule.
Apr 30 2021, 1:26 PM · Restricted Project
njames93 requested review of D101628: [Format] Don't sort includes if DisableFormat is true.
Apr 30 2021, 6:58 AM · Restricted Project, Restricted Project
njames93 updated the diff for D101617: [clang-tidy] Tweak diag ranges for bugprone-sizeof-expression.

Fix clangd test cases failing.

Apr 30 2021, 6:41 AM · Restricted Project
njames93 requested review of D101624: [clang-tidy] Make performance-inefficient-vector-operation work on members.
Apr 30 2021, 6:32 AM · Restricted Project
njames93 committed rG681503708594: [clangd][NFC] Remove unnecessary string captures in lambdas. (authored by njames93).
[clangd][NFC] Remove unnecessary string captures in lambdas.
Apr 30 2021, 5:28 AM
njames93 closed D101611: [clangd][NFC] Remove unnecessary string captures in lambdas..
Apr 30 2021, 5:27 AM · Restricted Project
njames93 requested review of D101617: [clang-tidy] Tweak diag ranges for bugprone-sizeof-expression.
Apr 30 2021, 5:26 AM · Restricted Project
njames93 requested review of D101614: [clang-tidy][NFC] Simplify a lot of bugprone-sizeof-expression matchers.
Apr 30 2021, 4:05 AM · Restricted Project
njames93 requested review of D101611: [clangd][NFC] Remove unnecessary string captures in lambdas..
Apr 30 2021, 2:49 AM · Restricted Project

Apr 28 2021

njames93 updated the diff for D97955: [clang-tidy] Refactor loop-convert to bring most of the checking into matchers.

Remove unnecessary test case.

Apr 28 2021, 4:27 PM · Restricted Project, Restricted Project
njames93 committed rGc3846bcfe1cc: [clangd][NFC] Reserve storage when creating semantic token encoding. (authored by njames93).
[clangd][NFC] Reserve storage when creating semantic token encoding.
Apr 28 2021, 2:40 PM
njames93 closed D101461: [clangd][NFC] Reserve storage when creating semantic token encoding..
Apr 28 2021, 2:40 PM · Restricted Project
njames93 requested review of D101461: [clangd][NFC] Reserve storage when creating semantic token encoding..
Apr 28 2021, 7:42 AM · Restricted Project
njames93 updated the diff for D97121: [clang-tidy] Add a Standalone diagnostics mode to clang-tidy.

Rebase and fix up new checks added and changes to tests.

Apr 28 2021, 5:47 AM · Restricted Project, Restricted Project, Restricted Project
Herald added a project to D97955: [clang-tidy] Refactor loop-convert to bring most of the checking into matchers: Restricted Project.

Ping?

Apr 28 2021, 5:31 AM · Restricted Project, Restricted Project
njames93 committed rG858a9583e1fe: [clang-query] Add check to prevent setting srcloc when no introspection is… (authored by njames93).
[clang-query] Add check to prevent setting srcloc when no introspection is…
Apr 28 2021, 3:22 AM
njames93 closed D101365: [clang-query] Add check to prevent setting srcloc when no introspection is available..
Apr 28 2021, 3:21 AM · Restricted Project

Apr 27 2021

njames93 added inline comments to D101259: [clang-tidy] Fix cppcoreguidelines-pro-type-vararg false positives with __builtin_ms_va_list.
Apr 27 2021, 11:32 PM · Restricted Project
njames93 added inline comments to D101275: [clangd] Hide inlay hints capability behind a command-line flag.
Apr 27 2021, 4:31 PM · Restricted Project
njames93 updated the diff for D101365: [clang-query] Add check to prevent setting srcloc when no introspection is available..

Nit.

Apr 27 2021, 4:26 PM · Restricted Project
njames93 updated the diff for D101365: [clang-query] Add check to prevent setting srcloc when no introspection is available..

Fix up tests(probably, see what premerge says)

Apr 27 2021, 10:23 AM · Restricted Project
njames93 updated the diff for D101365: [clang-query] Add check to prevent setting srcloc when no introspection is available..

Cleanup.

Apr 27 2021, 7:18 AM · Restricted Project
njames93 requested review of D101365: [clang-query] Add check to prevent setting srcloc when no introspection is available..
Apr 27 2021, 7:11 AM · Restricted Project

Apr 24 2021

njames93 accepted D101049: [AST] Add DeclarationNameInfo to node introspection.
Apr 24 2021, 11:45 PM · Restricted Project
njames93 abandoned D100638: [AST][Introspection] Avoid creating temporary strings when comparing LocationCalls..
Apr 24 2021, 11:39 PM · Restricted Project

Apr 23 2021

njames93 added inline comments to D93325: Add srcloc output to clang-query.
Apr 23 2021, 8:35 AM · Restricted Project, Restricted Project
njames93 added inline comments to D101049: [AST] Add DeclarationNameInfo to node introspection.
Apr 23 2021, 8:15 AM · Restricted Project
njames93 accepted D101054: [AST] Sort introspection results without instantiating other data.

This I suppose is a good fast way to sort these.

Apr 23 2021, 8:09 AM · Restricted Project

Apr 21 2021

njames93 added a comment to D99924: [clang-tidy] Avoid bugprone-macro-parentheses warnings after goto argument.

Adding the CFE mailing list, which was not on the review previously.

Apr 21 2021, 11:23 PM · Restricted Project
njames93 accepted D100712: [AST] Add NestedNameSpecifierLoc accessors to node introspection.
Apr 21 2021, 10:43 PM · Restricted Project
njames93 added a comment to D96286: [clangd][NFC] Change TidyProvider cache to use a linked list approach.

Ping?

Apr 21 2021, 1:25 AM · Restricted Project, Restricted Project
njames93 retitled D96286: [clangd][NFC] Change TidyProvider cache to use a linked list approach from [clangd] Change TidyProvider cache to use a linked list approach to [clangd][NFC] Change TidyProvider cache to use a linked list approach.
Apr 21 2021, 1:25 AM · Restricted Project, Restricted Project

Apr 20 2021

njames93 committed rG6b4e8f82a3f8: [clangd] Use dirty filesystem when performing cross file tweaks (authored by njames93).
[clangd] Use dirty filesystem when performing cross file tweaks
Apr 20 2021, 9:14 AM
njames93 closed D93978: [clangd] Use dirty filesystem when performing cross file tweaks.
Apr 20 2021, 9:13 AM · Restricted Project, Restricted Project

Apr 19 2021

njames93 accepted D100723: [AST] Fix comparison to of SourceRanges in container.
Apr 19 2021, 1:11 PM · Restricted Project
njames93 accepted D100720: [AST] Update introspection API to use const-ref for copyable types.

LG with 1 nit.

Apr 19 2021, 10:57 AM · Restricted Project
njames93 updated the diff for D93978: [clangd] Use dirty filesystem when performing cross file tweaks.

Fix up compilation failures and test failures.

Apr 19 2021, 7:29 AM · Restricted Project, Restricted Project

Apr 18 2021

njames93 added a comment to D100731: [clangd] Omit parameter hint for setter functions.

I'm not sure how related this is, but Hover has methods that try to detect setter functions in class to give them a trivial description, would that be of any help here?

Apr 18 2021, 2:04 PM · Restricted Project
njames93 committed rGdb75db85f231: [Introspection] Dont emit json if unchanged. (authored by njames93).
[Introspection] Dont emit json if unchanged.
Apr 18 2021, 12:22 PM
njames93 closed D100719: [Introspection] Dont emit json if unchanged..
Apr 18 2021, 12:22 PM · Restricted Project
njames93 added inline comments to D100712: [AST] Add NestedNameSpecifierLoc accessors to node introspection.
Apr 18 2021, 12:14 PM · Restricted Project
njames93 added inline comments to D100723: [AST] Fix comparison to of SourceRanges in container.
Apr 18 2021, 8:37 AM · Restricted Project
njames93 added inline comments to D100712: [AST] Add NestedNameSpecifierLoc accessors to node introspection.
Apr 18 2021, 2:15 AM · Restricted Project
njames93 requested review of D100719: [Introspection] Dont emit json if unchanged..
Apr 18 2021, 2:04 AM · Restricted Project
njames93 committed rGa0898f0cecc7: [AST][Introspection][NFC] Remove unnecessary temporary strings. (authored by njames93).
[AST][Introspection][NFC] Remove unnecessary temporary strings.
Apr 18 2021, 1:25 AM

Apr 17 2021

njames93 updated the diff for D100638: [AST][Introspection] Avoid creating temporary strings when comparing LocationCalls..

Rebase and remove Args from comparison.
Add quick check for skippig common prefix calls.

Apr 17 2021, 11:50 PM · Restricted Project
njames93 updated the diff for D93978: [clangd] Use dirty filesystem when performing cross file tweaks.

Address comments.

Apr 17 2021, 4:50 AM · Restricted Project, Restricted Project
njames93 accepted D100516: [AST] Add TypeLoc support to node introspection.

LGTM, just a few nits before landing please.

Apr 17 2021, 3:51 AM · Restricted Project
njames93 accepted D100688: [AST] Remove args from LocationCall.
Apr 17 2021, 3:23 AM · Restricted Project

Apr 16 2021

njames93 added inline comments to D100638: [AST][Introspection] Avoid creating temporary strings when comparing LocationCalls..
Apr 16 2021, 5:06 AM · Restricted Project
njames93 requested review of D100638: [AST][Introspection] Avoid creating temporary strings when comparing LocationCalls..
Apr 16 2021, 3:51 AM · Restricted Project

Apr 15 2021

njames93 added inline comments to D100516: [AST] Add TypeLoc support to node introspection.
Apr 15 2021, 11:08 PM · Restricted Project
njames93 added inline comments to D100516: [AST] Add TypeLoc support to node introspection.
Apr 15 2021, 2:39 PM · Restricted Project
njames93 accepted D100548: [AST] Fix location call storage with common last-invocation.

LGTM

Apr 15 2021, 2:28 PM · Restricted Project
njames93 committed rGf019e5f73ed7: [AST][Introspection] Add a check to detect if introspection is supported. (authored by njames93).
[AST][Introspection] Add a check to detect if introspection is supported.
Apr 15 2021, 2:22 PM
njames93 closed D100530: [AST][Introspection] Add a check to detect if introspection is supported..
Apr 15 2021, 2:22 PM · Restricted Project
njames93 committed rG542e7806e610: [AST] Add a print method to Introspection LocationCall (authored by njames93).
[AST] Add a print method to Introspection LocationCall
Apr 15 2021, 2:19 PM
njames93 closed D100423: [AST] Add a print method to Introspection LocationCall.
Apr 15 2021, 2:18 PM · Restricted Project
njames93 updated the diff for D100423: [AST] Add a print method to Introspection LocationCall.

Tweak implementaion to make the print a static method of FormatterCpp.

Apr 15 2021, 5:12 AM · Restricted Project
njames93 requested review of D100530: [AST][Introspection] Add a check to detect if introspection is supported..
Apr 15 2021, 12:38 AM · Restricted Project

Apr 14 2021

njames93 committed rG6890f302f587: [AST][Introspection] Fix args not being set. (authored by njames93).
[AST][Introspection] Fix args not being set.
Apr 14 2021, 4:20 PM
njames93 committed rGbfb6c2874be8: [AST][NFC] Remove temporary ASTTU file from Introspection generation. (authored by njames93).
[AST][NFC] Remove temporary ASTTU file from Introspection generation.
Apr 14 2021, 4:15 PM
njames93 closed D100343: [AST][NFC] Remove temporary ASTTU file from Introspection generation..
Apr 14 2021, 4:15 PM · Restricted Project
njames93 committed rGb23abbeab1d7: [AST] Use IntrusiveRefCntPtr for Introspection LocationCall. (authored by njames93).
[AST] Use IntrusiveRefCntPtr for Introspection LocationCall.
Apr 14 2021, 4:12 PM