Page MenuHomePhabricator

vabridgers (Vince Bridgers)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 28 2019, 3:56 PM (37 w, 23 h)

Recent Activity

Fri, Aug 7

vabridgers added a comment to D85157: [Sema] Add casting check for fixed to fixed point conversions.

All comments marked as done. ok to land?

Fri, Aug 7, 3:10 AM · Restricted Project
vabridgers added inline comments to D85157: [Sema] Add casting check for fixed to fixed point conversions.
Fri, Aug 7, 3:08 AM · Restricted Project
vabridgers updated the diff for D85157: [Sema] Add casting check for fixed to fixed point conversions.

address Bjorn's comment

Fri, Aug 7, 3:07 AM · Restricted Project

Thu, Aug 6

vabridgers added a comment to D85157: [Sema] Add casting check for fixed to fixed point conversions.

Ping! Ok to land?

Thu, Aug 6, 6:03 PM · Restricted Project
vabridgers added a comment to D85097: [Sema] add warning for comparisons like 'x<=y<=z'.

Thanks for the recent comments. I just pushed a few improvements over the last patch that didn't comprehend latest comments from @rsmith and @Quuxplusone. I'll read through those carefully and address those.

Thu, Aug 6, 6:03 PM · Restricted Project
vabridgers updated the diff for D85097: [Sema] add warning for comparisons like 'x<=y<=z'.

use source from expression in fixit
s/seperate/separate/
address some chained comparison ambiguities outside of original scope of changes

Thu, Aug 6, 5:59 PM · Restricted Project
vabridgers added inline comments to D85097: [Sema] add warning for comparisons like 'x<=y<=z'.
Thu, Aug 6, 2:46 AM · Restricted Project

Wed, Aug 5

vabridgers added a comment to D85097: [Sema] add warning for comparisons like 'x<=y<=z'.

I added prototype fixits per request by Roman, updated the LIT test, and added an additional RUN line (one for -Wparentheses and one for -Wcompare-op-parentheses). Also filed https://bugs.llvm.org/show_bug.cgi?id=47010 to follow up on the FIXME cases at the bottom of the LIT since they are out of scope for this change. Thanks for the feedback and comments so far, I look forward to driving this change to completion.

Wed, Aug 5, 5:12 PM · Restricted Project
vabridgers updated the diff for D85097: [Sema] add warning for comparisons like 'x<=y<=z'.

added prototype fixits for review.
added additional RUN test case.
filed https://bugs.llvm.org/show_bug.cgi?id=47010 for other
warnings improvement post landing of this patch, after lgtm

Wed, Aug 5, 5:09 PM · Restricted Project

Tue, Aug 4

vabridgers updated subscribers of D85157: [Sema] Add casting check for fixed to fixed point conversions.

ok, I think it's all sorted out now. Thanks @bevinh for the desk review. Let's start at the beginning again :)

Tue, Aug 4, 6:44 AM · Restricted Project
vabridgers updated the diff for D85157: [Sema] Add casting check for fixed to fixed point conversions.

ok, I think it's all sorted out now.

Tue, Aug 4, 6:42 AM · Restricted Project
vabridgers updated the summary of D85157: [Sema] Add casting check for fixed to fixed point conversions.
Tue, Aug 4, 6:41 AM · Restricted Project
vabridgers retitled D85157: [Sema] Add casting check for fixed to fixed point conversions from [Sema] Add casting check for integer to fixed point conversions to [Sema] Add casting check for fixed to fixed point conversions.
Tue, Aug 4, 6:40 AM · Restricted Project
vabridgers updated the summary of D85157: [Sema] Add casting check for fixed to fixed point conversions.
Tue, Aug 4, 5:31 AM · Restricted Project
vabridgers updated the summary of D85157: [Sema] Add casting check for fixed to fixed point conversions.
Tue, Aug 4, 5:28 AM · Restricted Project
vabridgers updated the summary of D85157: [Sema] Add casting check for fixed to fixed point conversions.
Tue, Aug 4, 5:27 AM · Restricted Project
vabridgers added inline comments to D85097: [Sema] add warning for comparisons like 'x<=y<=z'.
Tue, Aug 4, 3:24 AM · Restricted Project
vabridgers updated the diff for D85097: [Sema] add warning for comparisons like 'x<=y<=z'.

fix misc test formatting

Tue, Aug 4, 3:22 AM · Restricted Project
vabridgers updated the diff for D85097: [Sema] add warning for comparisons like 'x<=y<=z'.

isRelationalOp

Tue, Aug 4, 3:16 AM · Restricted Project

Mon, Aug 3

vabridgers added inline comments to D85157: [Sema] Add casting check for fixed to fixed point conversions.
Mon, Aug 3, 5:41 PM · Restricted Project
vabridgers updated the diff for D85157: [Sema] Add casting check for fixed to fixed point conversions.

remove -DFIXED_POINT from lit test, since it's not needed in this casting

Mon, Aug 3, 5:40 PM · Restricted Project
vabridgers added a comment to D85157: [Sema] Add casting check for fixed to fixed point conversions.

I updated the commit header with more details since the first submission was obviously too terse. @bjope, I believe the update should address your comments.

Mon, Aug 3, 5:36 PM · Restricted Project
vabridgers updated the diff for D85157: [Sema] Add casting check for fixed to fixed point conversions.

improve the commit message detail

Mon, Aug 3, 5:33 PM · Restricted Project
vabridgers updated subscribers of D85157: [Sema] Add casting check for fixed to fixed point conversions.
Mon, Aug 3, 2:54 PM · Restricted Project
vabridgers added reviewers for D85157: [Sema] Add casting check for fixed to fixed point conversions: leonardchan, aaron.ballman.
Mon, Aug 3, 2:48 PM · Restricted Project
vabridgers requested review of D85157: [Sema] Add casting check for fixed to fixed point conversions.
Mon, Aug 3, 1:40 PM · Restricted Project

Sun, Aug 2

vabridgers added a comment to D85097: [Sema] add warning for comparisons like 'x<=y<=z'.

I believe I've addressed all comments so far. Looks like Arthur suggested some particular cases that are not currently covered, and are not covered by this change since I think addressing those issues are our of scope of my original intent. If this patch is otherwise acceptable, would the reviewers be ok accepting this patch on the condition of creating a bugzilla report to track those issues?

Sun, Aug 2, 4:29 PM · Restricted Project
vabridgers updated the diff for D85097: [Sema] add warning for comparisons like 'x<=y<=z'.

refactor test cases per comment from Arthur

Sun, Aug 2, 4:26 PM · Restricted Project
vabridgers added inline comments to D85097: [Sema] add warning for comparisons like 'x<=y<=z'.
Sun, Aug 2, 3:51 PM · Restricted Project
vabridgers updated the diff for D85097: [Sema] add warning for comparisons like 'x<=y<=z'.

back out last unwanted changes from clang-format

Sun, Aug 2, 1:27 PM · Restricted Project
vabridgers added inline comments to D85097: [Sema] add warning for comparisons like 'x<=y<=z'.
Sun, Aug 2, 12:57 PM · Restricted Project
vabridgers added a comment to D85097: [Sema] add warning for comparisons like 'x<=y<=z'.

Thanks for the comments. I posted an update for the simpler issues, working on refactoring the test cases and creating a fixit. Thanks for the good and actionable review comments!

Sun, Aug 2, 12:54 PM · Restricted Project
vabridgers updated the diff for D85097: [Sema] add warning for comparisons like 'x<=y<=z'.

Address simpler issues brought up during review so far.
Tests to be refactored, and a fixit added.

Sun, Aug 2, 12:53 PM · Restricted Project
vabridgers added a comment to D85097: [Sema] add warning for comparisons like 'x<=y<=z'.

Thank you for the comments @lebedev.ri and @Quuxplusone. I'll abandon the tidy approach (https://reviews.llvm.org/D84898) and work towards satisfying these review comments (and any others), driving towards acceptance. Best!

Sun, Aug 2, 11:25 AM · Restricted Project
vabridgers added a comment to D85097: [Sema] add warning for comparisons like 'x<=y<=z'.

This is an implementation in the CFE, after submitting and getting comments on https://reviews.llvm.org/D84898.

Sun, Aug 2, 11:03 AM · Restricted Project
vabridgers added reviewers for D85097: [Sema] add warning for comparisons like 'x<=y<=z': lebedev.ri, aaron.ballman, alexfh, hokein, njames93.
Sun, Aug 2, 10:50 AM · Restricted Project
vabridgers added a comment to D84898: [clang-tidy] Add new checker for complex conditions with no meaning.

I've posted https://reviews.llvm.org/D85097 to replace this review. https://reviews.llvm.org/D85097 implements this check in the CFE instead of as a tidy check per recommendation from @lebedev.ri . If acceptable, I'll abandon this review.

Sun, Aug 2, 10:49 AM · Restricted Project, Restricted Project
vabridgers requested review of D85097: [Sema] add warning for comparisons like 'x<=y<=z'.
Sun, Aug 2, 10:47 AM · Restricted Project

Sat, Aug 1

vabridgers added a comment to D84898: [clang-tidy] Add new checker for complex conditions with no meaning.

Looks like this can be implemented as a warning in the cfe (as @lebedev.ri suggested). I'll probably abandon this review, but will keep it active until I have the alternative cfe warning patch prepared.

Sat, Aug 1, 5:22 PM · Restricted Project, Restricted Project
vabridgers added a comment to D84898: [clang-tidy] Add new checker for complex conditions with no meaning.

@njames93, I'll take a crack at implementing a cfe diagnostic for this, see how far I get. Cheers :)

Sat, Aug 1, 1:42 PM · Restricted Project, Restricted Project

Thu, Jul 30

vabridgers added a comment to D84898: [clang-tidy] Add new checker for complex conditions with no meaning.

Thanks Eugene, I think the backtick issue has been addressed.

Thu, Jul 30, 8:38 AM · Restricted Project, Restricted Project
vabridgers updated the diff for D84898: [clang-tidy] Add new checker for complex conditions with no meaning.

Address backticks in rst files

Thu, Jul 30, 8:37 AM · Restricted Project, Restricted Project
vabridgers added a comment to D84898: [clang-tidy] Add new checker for complex conditions with no meaning.

I believe all review comments have been address, except for the discussion on implementing this in the CFE or as a tidy check.

Thu, Jul 30, 8:21 AM · Restricted Project, Restricted Project
vabridgers updated the diff for D84898: [clang-tidy] Add new checker for complex conditions with no meaning.

Address most review comments.

Thu, Jul 30, 8:20 AM · Restricted Project, Restricted Project
vabridgers updated subscribers of D84898: [clang-tidy] Add new checker for complex conditions with no meaning.
Thu, Jul 30, 2:33 AM · Restricted Project, Restricted Project
vabridgers added a comment to D84898: [clang-tidy] Add new checker for complex conditions with no meaning.

Thanks all for the prompt and actionable comments. I will address all comments directly and 1-1, but would like to bottom out on one point before driving this forward. @lebedev.ri commented this should be in the Clang front end rather than a tidy check. Can we reach a consensus on that comment before I proceed with either a tidy check renamed to bugprone (comment from @Eugene.Zelenko ) or implement this check in the front end, enabled only when all warnings are enabled? I had considered both approaches when I started this, and thought the tidy checker was the best first step.

Thu, Jul 30, 2:32 AM · Restricted Project, Restricted Project

Wed, Jul 29

vabridgers added a comment to D84898: [clang-tidy] Add new checker for complex conditions with no meaning.

I believe this is ready for review. Thanks!

Wed, Jul 29, 5:48 PM · Restricted Project, Restricted Project
vabridgers added a reviewer for D84898: [clang-tidy] Add new checker for complex conditions with no meaning: aaron.ballman.
Wed, Jul 29, 5:33 PM · Restricted Project, Restricted Project
vabridgers updated the diff for D84898: [clang-tidy] Add new checker for complex conditions with no meaning.

Another update

Wed, Jul 29, 5:31 PM · Restricted Project, Restricted Project
vabridgers updated the diff for D84898: [clang-tidy] Add new checker for complex conditions with no meaning.

Correct some simple mistakes

Wed, Jul 29, 5:29 PM · Restricted Project, Restricted Project
vabridgers requested review of D84898: [clang-tidy] Add new checker for complex conditions with no meaning.
Wed, Jul 29, 5:24 PM · Restricted Project, Restricted Project

Sat, Jul 25

vabridgers updated the diff for D83992: [ASTImporter] Add Visitor for TypedefNameDecl's.

Adding negative test case that exposes the original problem.

Sat, Jul 25, 3:20 PM · Restricted Project

Tue, Jul 21

einvbri <vince.a.bridgers@ericsson.com> committed rG20157410862d: [ASTImporter] Refactor ASTImporter to support custom downstream tests (authored by vabridgers).
[ASTImporter] Refactor ASTImporter to support custom downstream tests
Tue, Jul 21, 8:34 AM
vabridgers closed D83970: [ASTImporter] Refactor ASTImporter to support custom downstream tests.
Tue, Jul 21, 8:34 AM · Restricted Project
vabridgers added a comment to D83992: [ASTImporter] Add Visitor for TypedefNameDecl's.

I discussed with Gabor. We don't feel comfortable landing this without a covering negative test case, so I'll work on that a little more, see what I can come up with. Thanks Gabor!

Tue, Jul 21, 7:14 AM · Restricted Project

Fri, Jul 17

vabridgers updated subscribers of D83992: [ASTImporter] Add Visitor for TypedefNameDecl's.
Fri, Jul 17, 3:08 AM · Restricted Project
vabridgers updated subscribers of D83970: [ASTImporter] Refactor ASTImporter to support custom downstream tests.
Fri, Jul 17, 3:08 AM · Restricted Project
vabridgers updated subscribers of D83970: [ASTImporter] Refactor ASTImporter to support custom downstream tests.
Fri, Jul 17, 3:07 AM · Restricted Project

Thu, Jul 16

vabridgers updated the diff for D83992: [ASTImporter] Add Visitor for TypedefNameDecl's.

Fix lint pre-merge check

Thu, Jul 16, 4:40 PM · Restricted Project
Herald added a project to D83992: [ASTImporter] Add Visitor for TypedefNameDecl's: Restricted Project.
Thu, Jul 16, 3:58 PM · Restricted Project
vabridgers added a comment to D83970: [ASTImporter] Refactor ASTImporter to support custom downstream tests.

@shafik, Thanks! I'll wait 'til Gabor gets a chance to review before landing. Best!

Thu, Jul 16, 2:22 PM · Restricted Project
Herald added a project to D83970: [ASTImporter] Refactor ASTImporter to support custom downstream tests: Restricted Project.
Thu, Jul 16, 11:05 AM · Restricted Project

Jul 2 2020

einvbri <vince.a.bridgers@ericsson.com> committed rG59f1bf46f8c2: [ASTImporter] Add unittest case for friend decl import (authored by vabridgers).
[ASTImporter] Add unittest case for friend decl import
Jul 2 2020, 7:33 AM
vabridgers closed D83006: [ASTImporter] Add unittest case for friend decl import.
Jul 2 2020, 7:33 AM · Restricted Project

Jul 1 2020

vabridgers added a comment to D83009: [clang][Serialization] Don't duplicate the body of LambdaExpr during deserialization.

I cherry picked this change and retried the case that was failing. This change addresses the issue I was seeing. Thanks.

Jul 1 2020, 6:58 PM · Restricted Project
vabridgers updated the diff for D83006: [ASTImporter] Add unittest case for friend decl import.

update for pre-merge lint checks

Jul 1 2020, 5:18 PM · Restricted Project
vabridgers added a comment to D83006: [ASTImporter] Add unittest case for friend decl import.

I confirmed that a crash was seen when the change for https://reviews.llvm.org/D82882 was reverted, then the unittests pass with the change associated with https://reviews.llvm.org/D82882.

Jul 1 2020, 5:18 PM · Restricted Project
vabridgers updated the diff for D83006: [ASTImporter] Add unittest case for friend decl import.

Updated commit header.

Jul 1 2020, 4:13 PM · Restricted Project
vabridgers created D83006: [ASTImporter] Add unittest case for friend decl import.
Jul 1 2020, 4:13 PM · Restricted Project
vabridgers added a comment to D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases.

This patch addresses a crash we started seeing in our CTU analysis runs as a result of 05843dc6ab97a00cbde7aa4f08bf3692eb83109d. Gabor, maybe I can generate a regression test for the problems that 05843dc6ab97a00cbde7aa4f08bf3692eb83109d caused as a follow up for whatever is committed here?

Jul 1 2020, 4:17 AM · Restricted Project

Jun 30 2020

einvbri <vince.a.bridgers@ericsson.com> committed rGecae672ac2ac: [ASTImporter] Fix AST import crash for a friend decl (authored by vabridgers).
[ASTImporter] Fix AST import crash for a friend decl
Jun 30 2020, 2:10 PM
vabridgers closed D82882: [ASTImporter] Fix AST import crash for a friend decl.
Jun 30 2020, 2:09 PM · Restricted Project
vabridgers updated the diff for D82882: [ASTImporter] Fix AST import crash for a friend decl.

fix pre-merge checks

Jun 30 2020, 11:24 AM · Restricted Project
vabridgers created D82882: [ASTImporter] Fix AST import crash for a friend decl.
Jun 30 2020, 8:39 AM · Restricted Project

Jun 4 2020

einvbri <vince.a.bridgers@ericsson.com> committed rGbd425825411a: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker (authored by vabridgers).
[analyzer] Ignore calculated indices of <= 0 in VLASizeChecker
Jun 4 2020, 5:30 AM
vabridgers closed D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker.
Jun 4 2020, 5:29 AM · Restricted Project

Jun 2 2020

vabridgers updated the diff for D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker.

Address comments from Artem and Balazs. This change just avoids the crash for now until a proper fix can be made.

Jun 2 2020, 7:41 AM · Restricted Project

Jun 1 2020

vabridgers added inline comments to D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker.
Jun 1 2020, 5:21 PM · Restricted Project

May 31 2020

vabridgers updated the diff for D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker.

Address some typos

May 31 2020, 5:49 PM · Restricted Project
vabridgers added a comment to D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker.

Not sure this is "correct", but it passes LITs with the new case, and it will at least get the discussion started :)

May 31 2020, 5:33 PM · Restricted Project
vabridgers created D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker.
May 31 2020, 5:33 PM · Restricted Project

Apr 15 2020

einvbri <vince.a.bridgers@ericsson.com> committed rG789215dc0db1: [ASTImporter] Add support for importing fixed point literals (authored by vabridgers).
[ASTImporter] Add support for importing fixed point literals
Apr 15 2020, 8:46 AM
vabridgers closed D77721: [ASTImporter] Add support for importing fixed point literals.
Apr 15 2020, 8:45 AM · Restricted Project

Apr 14 2020

einvbri <vince.a.bridgers@ericsson.com> committed rG161fc1d9118f: [Fixed Point] [AST] Add an AST serialization code for fixed-point literals. (authored by vabridgers).
[Fixed Point] [AST] Add an AST serialization code for fixed-point literals.
Apr 14 2020, 11:51 AM
vabridgers closed D57226: [Fixed Point] [AST] Add an AST serialization code for fixed-point literals..
Apr 14 2020, 11:51 AM · Restricted Project
vabridgers added a comment to D77721: [ASTImporter] Add support for importing fixed point literals.

Do these changes look ok to land? https://reviews.llvm.org/D57226 is pushed. Thanks!

Apr 14 2020, 11:50 AM · Restricted Project
vabridgers updated the diff for D57226: [Fixed Point] [AST] Add an AST serialization code for fixed-point literals..

update based on comments from rjmccall

Apr 14 2020, 4:45 AM · Restricted Project
vabridgers added a comment to D57226: [Fixed Point] [AST] Add an AST serialization code for fixed-point literals..

Thank you for the comments. Please do let me know when this patch is ready to land. Best!

Apr 14 2020, 4:45 AM · Restricted Project

Apr 13 2020

vabridgers updated the diff for D57226: [Fixed Point] [AST] Add an AST serialization code for fixed-point literals..

Address comments from rjmccall

Apr 13 2020, 8:06 PM · Restricted Project

Apr 9 2020

vabridgers added a comment to D49074: [Analyzer] Basic support for multiplication and division in the constraint manager.

ping! Any chance of this patch being accepted? This patch can help some SA false positives.

Apr 9 2020, 4:02 PM
vabridgers added a comment to D50256: [Analyzer] Basic support for multiplication and division in the constraint manager (for == and != only).

ping! Any chance of this patch being accepted? This patch can help some SA false positives.

Apr 9 2020, 4:02 PM
vabridgers updated the diff for D77721: [ASTImporter] Add support for importing fixed point literals.

Incorporate Gabor's suggestion for improving test coverage

Apr 9 2020, 11:24 AM · Restricted Project
vabridgers added a comment to D77721: [ASTImporter] Add support for importing fixed point literals.

Ahhh yes, I see. I can get this done while we're waiting on https://reviews.llvm.org/D57226 to land. Thanks Gabor!!

Apr 9 2020, 10:17 AM · Restricted Project
vabridgers updated the diff for D77721: [ASTImporter] Add support for importing fixed point literals.

Address comment from @balazske

Apr 9 2020, 3:46 AM · Restricted Project

Apr 8 2020

vabridgers updated the diff for D77721: [ASTImporter] Add support for importing fixed point literals.

Remove extraneous code :/

Apr 8 2020, 4:18 PM · Restricted Project
vabridgers updated the diff for D77721: [ASTImporter] Add support for importing fixed point literals.

Addressed comments from @balazske.
Thanks for the tips and useful starting point from @martong

Apr 8 2020, 4:18 PM · Restricted Project
vabridgers added a comment to D77721: [ASTImporter] Add support for importing fixed point literals.

@balazske, Thank you for the comments. I'll address and repost the review.

Apr 8 2020, 7:02 AM · Restricted Project
vabridgers added a comment to D77721: [ASTImporter] Add support for importing fixed point literals.

Landing this change depends on https://reviews.llvm.org/D57226 to be pushed. Please review for now, and I'll be sure to push this only after https://reviews.llvm.org/D57226 is pushed. Thanks!

Apr 8 2020, 5:23 AM · Restricted Project
vabridgers created D77721: [ASTImporter] Add support for importing fixed point literals.
Apr 8 2020, 5:23 AM · Restricted Project
vabridgers added a comment to D57226: [Fixed Point] [AST] Add an AST serialization code for fixed-point literals..

https://reviews.llvm.org/D77721 depends on this serialization change for fixed point literals.

Apr 8 2020, 5:23 AM · Restricted Project