[PATCH] D46131: Add Microsoft Mangling for OpenCL Half Type

2018-04-26 Thread Vince Bridgers via Phabricator via cfe-commits
vbridgers created this revision. vbridgers added reviewers: rnk, Anastasia, erichkeane. vbridgers added a project: clang. Herald added subscribers: cfe-commits, yaxunl. Half-type mangling is accomplished following the method introduced by Erich Keane for mangling _Float16. Updated the half.cl LIT

[PATCH] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

2020-05-31 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers created this revision. vabridgers added reviewers: balazske, NoQ, martong, baloghadamsoftware, Szelethus, gamesh411. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun. Herald added a project: clang. vabri

[PATCH] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

2020-05-31 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Not sure this is "correct", but it passes LITs with the new case, and it will at least get the discussion started :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80903/new/ https://reviews.llvm.org/D80903 __

[PATCH] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

2020-05-31 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 267534. vabridgers added a comment. Address some typos Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80903/new/ https://reviews.llvm.org/D80903 Files: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp

[PATCH] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

2020-06-01 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:114-115 // Convert the array length to size_t. NonLoc IndexLength = SVB.evalCast(SizeD, SizeTy, SizeE->getType()).castAs(); // Multiply the array length by t

[PATCH] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

2020-06-02 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 267882. vabridgers added a comment. Address comments from Artem and Balazs. This change just avoids the crash for now until a proper fix can be made. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80903/new/

[PATCH] D83970: [ASTImporter] Refactor ASTImporter to support custom downstream tests

2020-07-16 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers created this revision. vabridgers added a reviewer: martong. Herald added subscribers: cfe-commits, teemperor, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: clang. The purpose of this change is to do a small refactoring of code in

[PATCH] D83970: [ASTImporter] Refactor ASTImporter to support custom downstream tests

2020-07-16 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. @shafik, Thanks! I'll wait 'til Gabor gets a chance to review before landing. Best! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83970/new/ https://reviews.llvm.org/D83970 ___

[PATCH] D83992: [ASTImporter] Add Visitor for TypedefNameDecl's

2020-07-16 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers created this revision. vabridgers added a reviewer: martong. Herald added subscribers: cfe-commits, teemperor, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a project: clang. We found a case where Typedef Name Declarations were not being added correctly when importing built

[PATCH] D83992: [ASTImporter] Add Visitor for TypedefNameDecl's

2020-07-16 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 278630. vabridgers added a comment. Fix lint pre-merge check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83992/new/ https://reviews.llvm.org/D83992 Files: clang/lib/AST/ASTImporterLookupTable.cpp Inde

[PATCH] D83992: [ASTImporter] Add Visitor for TypedefNameDecl's

2020-07-21 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. 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! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D83970: [ASTImporter] Refactor ASTImporter to support custom downstream tests

2020-07-21 Thread Vince Bridgers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG20157410862d: [ASTImporter] Refactor ASTImporter to support custom downstream tests (authored by vabridgers, committed by einvbri ). Repository: rG LLVM Github Monorepo

[PATCH] D83992: [ASTImporter] Add Visitor for TypedefNameDecl's

2020-07-25 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 280700. vabridgers added a comment. Adding negative test case that exposes the original problem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83992/new/ https://reviews.llvm.org/D83992 Files: clang/lib/A

[PATCH] D84898: clang-tidy] Add new checker for complex conditions with no meaning

2020-07-29 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers created this revision. Herald added subscribers: cfe-commits, phosek, Charusso, mgorny. Herald added a project: clang. vabridgers requested review of this revision. This checker finds cases where relational expressions have no meaning. For example, (x <= y <= z) has no meaning, but just

[PATCH] D84898: clang-tidy] Add new checker for complex conditions with no meaning

2020-07-29 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 281769. vabridgers added a comment. Correct some simple mistakes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84898/new/ https://reviews.llvm.org/D84898 Files: clang-tools-extra/clang-tidy/misc/CMakeList

[PATCH] D84898: clang-tidy] Add new checker for complex conditions with no meaning

2020-07-29 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 281770. vabridgers added a comment. Another update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84898/new/ https://reviews.llvm.org/D84898 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt clang-

[PATCH] D84898: clang-tidy] Add new checker for complex conditions with no meaning

2020-07-29 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. I believe this is ready for review. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84898/new/ https://reviews.llvm.org/D84898 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D84898: [clang-tidy] Add new checker for complex conditions with no meaning

2020-07-30 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. 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

[PATCH] D84898: [clang-tidy] Add new checker for complex conditions with no meaning

2020-07-30 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 281937. vabridgers marked 6 inline comments as done. vabridgers added a comment. Address most review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84898/new/ https://reviews.llvm.org/D84898 Files:

[PATCH] D84898: [clang-tidy] Add new checker for complex conditions with no meaning

2020-07-30 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. I believe all review comments have been address, except for the discussion on implementing this in the CFE or as a tidy check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84898/new/ https://reviews.llvm.org/D84898 __

[PATCH] D84898: [clang-tidy] Add new checker for complex conditions with no meaning

2020-07-30 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 281943. vabridgers added a comment. Address backticks in rst files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84898/new/ https://reviews.llvm.org/D84898 Files: clang-tools-extra/clang-tidy/bugprone/Bug

[PATCH] D84898: [clang-tidy] Add new checker for complex conditions with no meaning

2020-07-30 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers marked 2 inline comments as done. vabridgers added a comment. Thanks Eugene, I think the backtick issue has been addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84898/new/ https://reviews.llvm.org/D84898 ___

[PATCH] D77507: [clangd] Fix HitMapping assertion in Tokens.cpp

2020-04-05 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers created this revision. vabridgers added reviewers: ilya-biryukov, sammccall. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Extend test cases for tokens, and remove assertion that is unneeded and hitting in Tokens.

[PATCH] D77507: [clangd] Fix HitMapping assertion in Tokens.cpp

2020-04-05 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 255193. vabridgers added a comment. Remove extraneous test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77507/new/ https://reviews.llvm.org/D77507 Files: clang/lib/Tooling/Syntax/Tokens.cpp clang/

[PATCH] D77507: [clangd] Fix HitMapping assertion in Tokens.cpp

2020-04-06 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Thank you for the comments. I'll keep looking at this to find a proper fix, but any hints you may have would be greatly appreciated (debugging tips, strategies, areas of code to focus on). This code is new to me, so I may not be as efficient at tracking this down as

[PATCH] D77507: [clangd] Fix HitMapping assertion in Tokens.cpp

2020-04-07 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. ayup, no problem and thanks for the fix. I'll abandon this change. Best! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77507/new/ https://reviews.llvm.org/D77507 ___ cfe-com

[PATCH] D77721: [ASTImporter] Add support for importing fixed point literals

2020-04-08 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. 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! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D57226: [Fixed Point] [AST] Add an AST serialization code for fixed-point literals.

2020-04-08 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. https://reviews.llvm.org/D77721 depends on this serialization change for fixed point literals. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57226/new/ https://reviews.llvm.org/D57226 ___

[PATCH] D77721: [ASTImporter] Add support for importing fixed point literals

2020-04-08 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers created this revision. vabridgers added reviewers: martong, leonardchan, ebevhan. Herald added subscribers: cfe-commits, teemperor, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: clang. vabridgers added a comment. Landing this chan

[PATCH] D77721: [ASTImporter] Add support for importing fixed point literals

2020-04-08 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. @balazske, Thank you for the comments. I'll address and repost the review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77721/new/ https://reviews.llvm.org/D77721 ___ cfe-c

[PATCH] D77721: [ASTImporter] Add support for importing fixed point literals

2020-04-08 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 256133. vabridgers added a comment. Remove extraneous code :/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77721/new/ https://reviews.llvm.org/D77721 Files: clang/include/clang/AST/Expr.h clang/include

[PATCH] D77721: [ASTImporter] Add support for importing fixed point literals

2020-04-08 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 256131. vabridgers added a comment. Addressed comments from @balazske. Thanks for the tips and useful starting point from @martong Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77721/new/ https://reviews.llv

[PATCH] D77721: [ASTImporter] Add support for importing fixed point literals

2020-04-09 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 256234. vabridgers added a comment. Address comment from @balazske Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77721/new/ https://reviews.llvm.org/D77721 Files: clang/include/clang/ASTMatchers/ASTMatche

[PATCH] D77721: [ASTImporter] Add support for importing fixed point literals

2020-04-09 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers marked an inline comment as done. vabridgers added a comment. Ahhh yes, I see. I can get this done while we're waiting on https://reviews.llvm.org/D57226 to land. Thanks Gabor!! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77721/new/ h

[PATCH] D77721: [ASTImporter] Add support for importing fixed point literals

2020-04-09 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 256339. vabridgers added a comment. Incorporate Gabor's suggestion for improving test coverage Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77721/new/ https://reviews.llvm.org/D77721 Files: clang/include

[PATCH] D50256: [Analyzer] Basic support for multiplication and division in the constraint manager (for == and != only)

2020-04-09 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. ping! Any chance of this patch being accepted? This patch can help some SA false positives. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D50256/new/ https://reviews.llvm.org/D50256 ___ cfe-commits mailing list c

[PATCH] D49074: [Analyzer] Basic support for multiplication and division in the constraint manager

2020-04-09 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. ping! Any chance of this patch being accepted? This patch can help some SA false positives. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49074/new/ https://reviews.llvm.org/D49074 ___ cfe-commits mailing list c

[PATCH] D57226: [Fixed Point] [AST] Add an AST serialization code for fixed-point literals.

2020-04-13 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 257183. vabridgers added a comment. Address comments from rjmccall Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57226/new/ https://reviews.llvm.org/D57226 Files: clang/include/clang/AST/Expr.h clang/in

[PATCH] D57226: [Fixed Point] [AST] Add an AST serialization code for fixed-point literals.

2020-04-14 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Thank you for the comments. Please do let me know when this patch is ready to land. Best! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57226/new/ https://reviews.llvm.org/D57226 _

[PATCH] D57226: [Fixed Point] [AST] Add an AST serialization code for fixed-point literals.

2020-04-14 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 257287. vabridgers added a comment. update based on comments from rjmccall Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57226/new/ https://reviews.llvm.org/D57226 Files: clang/include/clang/AST/Expr.h

[PATCH] D77721: [ASTImporter] Add support for importing fixed point literals

2020-04-14 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Do these changes look ok to land? https://reviews.llvm.org/D57226 is pushed. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77721/new/ https://reviews.llvm.org/D77721 __

[PATCH] D57226: [Fixed Point] [AST] Add an AST serialization code for fixed-point literals.

2020-04-14 Thread Vince Bridgers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG161fc1d9118f: [Fixed Point] [AST] Add an AST serialization code for fixed-point literals. (authored by vabridgers, committed by einvbri ). Repository: rG LLVM Github Mono

[PATCH] D77721: [ASTImporter] Add support for importing fixed point literals

2020-04-15 Thread Vince Bridgers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG789215dc0db1: [ASTImporter] Add support for importing fixed point literals (authored by vabridgers, committed by einvbri ). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

2020-06-04 Thread Vince Bridgers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGbd425825411a: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker (authored by vabridgers, committed by einvbri ). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

2020-07-01 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. 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

[PATCH] D83006: [ASTImporter] Add unittest case for friend decl import

2020-07-01 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers created this revision. vabridgers added a reviewer: martong. Herald added subscribers: cfe-commits, teemperor, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: clang. vabridgers updated this revision to Diff 274945. vabridgers added a

[PATCH] D83006: [ASTImporter] Add unittest case for friend decl import

2020-07-01 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 274945. vabridgers added a comment. Updated commit header. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83006/new/ https://reviews.llvm.org/D83006 Files: clang/unittests/AST/ASTImporterTest.cpp Index:

[PATCH] D83006: [ASTImporter] Add unittest case for friend decl import

2020-07-01 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D83006: [ASTImporter] Add unittest case for friend decl import

2020-07-01 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 274961. vabridgers added a comment. update for pre-merge lint checks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83006/new/ https://reviews.llvm.org/D83006 Files: clang/unittests/AST/ASTImporterTest.cpp

[PATCH] D83009: [clang][Serialization] Don't duplicate the body of LambdaExpr during deserialization

2020-07-01 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. I cherry picked this change and retried the case that was failing. This change addresses the issue I was seeing. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83009/new/ https://reviews.llvm.org/D83009 __

[PATCH] D83006: [ASTImporter] Add unittest case for friend decl import

2020-07-02 Thread Vince Bridgers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG59f1bf46f8c2: [ASTImporter] Add unittest case for friend decl import (authored by vabridgers, committed by einvbri ). Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D82882: [ASTImporter] Fix AST import crash for a friend decl

2020-06-30 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers created this revision. vabridgers added a reviewer: martong. Herald added subscribers: cfe-commits, teemperor, rnkovacs, kristof.beyls. Herald added a reviewer: a.sidorin. Herald added a project: clang. Running CTU testing, we found that VisitFriendDecl in ASTImporterLookup.cpp was not

[PATCH] D82882: [ASTImporter] Fix AST import crash for a friend decl

2020-06-30 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 274542. vabridgers added a comment. fix pre-merge checks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82882/new/ https://reviews.llvm.org/D82882 Files: clang/lib/AST/ASTImporterLookupTable.cpp Index: c

[PATCH] D82882: [ASTImporter] Fix AST import crash for a friend decl

2020-06-30 Thread Vince Bridgers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGecae672ac2ac: [ASTImporter] Fix AST import crash for a friend decl (authored by vabridgers, committed by einvbri ). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D84898: [clang-tidy] Add new checker for complex conditions with no meaning

2020-08-01 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. @njames93, I'll take a crack at implementing a cfe diagnostic for this, see how far I get. Cheers :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84898/new/ https://reviews.llvm.org/D84898

[PATCH] D84898: [clang-tidy] Add new checker for complex conditions with no meaning

2020-08-01 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-02 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. vabridgers requested review of this revision. This changes add a new warning named -Wcompare-op-parentheses that's part of the -Wparentheses diagnostic group. This diagnostic produces a warning

[PATCH] D84898: [clang-tidy] Add new checker for complex conditions with no meaning

2020-08-02 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. 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. Repository: rG LLVM Gith

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-02 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. This is an implementation in the CFE, after submitting and getting comments on https://reviews.llvm.org/D84898. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85097/new/ https://reviews.llvm.org/D85097 _

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-02 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. 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! Repository: rG LLVM Github Monorepo CHA

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-02 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 282471. vabridgers marked 2 inline comments as done. vabridgers added a comment. Address simpler issues brought up during review so far. Tests to be refactored, and a fixit added. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-02 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. 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! Comment at: clang/docs/DiagnosticsReference.rst:2853 +-Wcompare-no

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-02 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added inline comments. Comment at: clang/test/Misc/warning-wall.c:6 + CHECK - + NEXT : -Wchar - subscri

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-02 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 282475. vabridgers added a comment. back out last unwanted changes from clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85097/new/ https://reviews.llvm.org/D85097 Files: clang/docs/DiagnosticsR

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-02 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:14010 + << Bop->getSourceRange() << OpLoc; + SuggestParentheses(Self, Bop->getOperatorLoc(), + Self.PDiag(diag::note_precedence_silence) vabridgers wrote: > lebede

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-02 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 282484. vabridgers added a comment. refactor test cases per comment from Arthur Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85097/new/ https://reviews.llvm.org/D85097 Files: clang/docs/DiagnosticsRefere

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-02 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. 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

[PATCH] D85157: [Sema] Add casting check for integer to fixed point conversions

2020-08-03 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. vabridgers requested review of this revision. This change squelches the warning for a cast from integer to fixed point conversions. Repository: rG LLVM Github Monorepo https://reviews.llvm

[PATCH] D85157: [Sema] Add casting check for integer to fixed point conversions

2020-08-03 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 282775. vabridgers added a comment. improve the commit message detail Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85157/new/ https://reviews.llvm.org/D85157 Files: clang/lib/Sema/SemaCast.cpp clang/te

[PATCH] D85157: [Sema] Add casting check for integer to fixed point conversions

2020-08-03 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers marked 5 inline comments as done. vabridgers added a comment. 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. Comment at: clang/lib/Sema/SemaCast.cpp:2660

[PATCH] D85157: [Sema] Add casting check for integer to fixed point conversions

2020-08-03 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 282776. vabridgers marked 2 inline comments as done. vabridgers added a comment. remove -DFIXED_POINT from lit test, since it's not needed in this casting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85157/n

[PATCH] D85157: [Sema] Add casting check for integer to fixed point conversions

2020-08-03 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added inline comments. Comment at: clang/test/Sema/warn-bad-function-cast.c:49 +#ifdef FIXED_POINT + (void)(_Fract) if1(); // no warning +#endif bjope wrote: > bjope wrote: > > bjope wrote: > > > This should be added before the line saying `/* All fol

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-04 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 282852. vabridgers added a comment. isRelationalOp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85097/new/ https://reviews.llvm.org/D85097 Files: clang/docs/DiagnosticsReference.rst clang/include/clang

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-04 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 282854. vabridgers added a comment. fix misc test formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85097/new/ https://reviews.llvm.org/D85097 Files: clang/docs/DiagnosticsReference.rst clang/inc

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-04 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers marked 2 inline comments as done. vabridgers added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:14047 +static bool isComparisonOpSamePrecedence(BinaryOperatorKind Opc) { + switch (Opc) { njames93 wrote: > Quuxplusone wrote: > > Same prec

[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

2020-08-04 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 282902. vabridgers edited the summary of this revision. vabridgers added a comment. ok, I think it's all sorted out now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85157/new/ https://reviews.llvm.org/D851

[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

2020-08-04 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a subscriber: bevinh. vabridgers added a comment. ok, I think it's all sorted out now. Thanks @bevinh for the desk review. Let's start at the beginning again :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85157/new/ https://revi

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-05 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 283447. vabridgers added a comment. 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 Repository: rG LLVM Github Mo

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-05 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. 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 o

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-06 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:14034 + << FixItHint::CreateInsertion(LHSExpr->getEndLoc(), ") && ") + << FixItHint::CreateInsertion(LHSBO->getRHS()->getBeginLoc(), "(y ") + << FixItHint::CreateInsertion(RHSExpr->getEndLoc(

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-06 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 283780. vabridgers added a comment. use source from expression in fixit s/seperate/separate/ address some chained comparison ambiguities outside of original scope of changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-06 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

2020-08-06 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Ping! Ok to land? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85157/new/ https://reviews.llvm.org/D85157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://list

[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

2020-08-07 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 283858. vabridgers added a comment. address Bjorn's comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85157/new/ https://reviews.llvm.org/D85157 Files: clang/lib/Sema/SemaCast.cpp clang/test/Sema/wa

[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

2020-08-07 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers marked an inline comment as done. vabridgers added inline comments. Comment at: clang/test/Sema/warn-bad-function-cast.c:49 +#ifdef FIXED_POINT + (void)(_Fract) if1(); // no warning +#endif bjope wrote: > bjope wrote: > > vabridgers wrote: > > > bjope

[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

2020-08-07 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers marked 4 inline comments as done. vabridgers added a comment. All comments marked as done. ok to land? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85157/new/ https://reviews.llvm.org/D85157

[PATCH] D129269: [analyzer] Fix use of length in CStringChecker

2022-07-07 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers created this revision. vabridgers added reviewers: martong, steakhal. Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All. va

[PATCH] D129269: [analyzer] Fix use of length in CStringChecker

2022-07-07 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Thanks Balazs, you mean something like this correct? void strcpy_no_overflow_2(char *y) { char x[3]; strcpy(x, "12\0"); // this produces a warning, but should not. } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D129269: [analyzer] Fix use of length in CStringChecker

2022-07-07 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 443101. vabridgers added a comment. a proposal to handle embedded null case caught by @steakhal Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129269/new/ https://reviews.llvm.org/D129269 Files: clang/lib/

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

2022-03-25 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers created this revision. vabridgers added reviewers: NoQ, steakhal, martong. Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a project: All. vabridgers requested review

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

2022-03-26 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Hi @NoQ, good question :) When I looked into the existing SimpleSValBuilder.cpp code, I found a few places that did width modifications so I concluded this was the correct and accepted way to handle this. A few notes to help decide if my suggestion is correct or is t

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

2022-03-26 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Apparently, it's up to an implementation to determine the specific relations that can be computed between different address spaces. Perhaps a better way to deal with this for now, to avoid crashes, is follow the DereferenceChecker model. That checker punts on checkin

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

2022-03-28 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Ahhh, gotcha. I'll run that down and report back. Thanks ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122513/new/ https://reviews.llvm.org/D122513 ___ cfe-commits mailing l

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

2022-03-28 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. I think I got it, looks like we're losing the width information at line 685, 686 below. Looks like I need to adjust the bitwidth of V before returning. clang/lib/StaticAnalyzer/Core/SValBuilder.cpp 671 SVal SValBuilder::evalCastSubKind(loc::ConcreteInt V, QualType

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

2022-03-28 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 418749. vabridgers added a comment. Come up with a more principaled fix, thanks @NoQ :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122513/new/ https://reviews.llvm.org/D122513 Files: clang/lib/StaticAn

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

2022-03-29 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 418996. vabridgers added a comment. fix typo in test - ready to land Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122513/new/ https://reviews.llvm.org/D122513 Files: clang/lib/StaticAnalyzer/Core/SValBui

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

2022-03-29 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers marked an inline comment as done. vabridgers added inline comments. Comment at: clang/test/Analysis/addrspace-null.c:19 + +#if defined(AMDGCN) +// this crashes NoQ wrote: > Shouldn't this be `AMDGCN_TRIPLE`? fixed, thank you. Repository: rG LLVM Gi

[PATCH] D118050: [analyzer] Different address spaces cannot overlap

2022-03-29 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 419013. vabridgers added a comment. Herald added a project: All. rebase, check ci Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118050/new/ https://reviews.llvm.org/D118050 Files: clang/lib/StaticAnalyzer

[PATCH] D118050: [analyzer] Different address spaces cannot overlap

2022-03-31 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 419438. vabridgers added a comment. add NDEBUG around assert Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118050/new/ https://reviews.llvm.org/D118050 Files: clang/lib/StaticAnalyzer/Checkers/CStringChec

[PATCH] D118050: [analyzer] Different address spaces cannot overlap

2022-03-31 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. I added #ifndef NDEBUG around the assert since I'm not sure if downstream consumers that use different pointer sizes by address space will hit this assert. Our downstream target causes this assert to fire, so I will be continuing to find and fix these issues, submitt

  1   2   3   >