[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

wmi added a comment. In D36562#1642403 , @chill wrote: > In D36562#1641930 , @wmi wrote: > > > In D36562#1639441 , @chill wrote: > > > > > Shouldn't we disable

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:308 +//===--===// +// Unsinged analyzer options.

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

MaskRay added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:308 +//===--===// +// Unsinged analyzer options.

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

NoQ added a comment. Until we do enough debugging to understand the nature of these false positives, i recommend silencing the checker when it has false positives. Later we can improve the granularity of our checkers, so that to be able to disable smaller portions of them, or make constraint

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

Szelethus added a comment. In D66721#1644514 , @NoQ wrote: > No-no, you're disabling the checkers here but you should be silencing them. > This will be crashing because modeling is disabled. > > I also recommend checker options, even if they apply to

[PATCH] D66723: [clangd] Add a distinct highlighting for local variables

nridge created this revision. nridge added reviewers: hokein, jvikstrom. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. It's useful to be able to distinguish local variables from namespace scope variables. Repository:

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

Charusso added a comment. In D66721#1644531 , @NoQ wrote: > Now you're silencing the *whole* checker. This is equivalent to passing an > `-analyzer-silence-checker` flag. I am silencing the *whole* checker if and only if a bitwise operation or a shift

[PATCH] D66722: [clang] Ensure that comment classes are trivially destructible

riccibruno updated this revision to Diff 217063. riccibruno added a comment. Also test that `DeclInfo` is trivially destructible. Thanks ! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66722/new/ https://reviews.llvm.org/D66722 Files:

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

NoQ added a comment. Now you're silencing the *whole* checker. This is equivalent to passing an `-analyzer-silence-checker` flag. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66721/new/ https://reviews.llvm.org/D66721 ___ cfe-commits

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

Charusso updated this revision to Diff 217062. Charusso marked 4 inline comments as done. Charusso edited the summary of this revision. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66721/new/ https://reviews.llvm.org/D66721 Files:

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:305 +"Whether the bitwise (and shift) operations should be checked.", +false) + NoQ wrote: > We didn't do enough debugging to

[PATCH] D66722: [clang] Ensure that comment classes are trivially destructible

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Regarding adding a test for `DeclInfo` -- SGTM as well. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66722/new/ https://reviews.llvm.org/D66722

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:305 +"Whether the bitwise (and shift) operations should be checked.", +false) + We didn't do enough debugging to demonstrate that

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

NoQ added a comment. No-no, you're disabling the checkers here but you should be silencing them. This will be crashing because modeling is disabled. I also recommend checker options, even if they apply to multiple checkers (@Szelethus mentioned that package options are a thing, you might use

[PATCH] D66722: [clang] Ensure that comment classes are trivially destructible

Mordante added a reviewer: gribozavr. Mordante added a subscriber: gribozavr. Mordante added a comment. Please also add a test for the `DeclInfo` struct. It isn't a `CommentNode` but also allocated with the `BumpPtrAllocator`. I also like @gribozavr's input. Repository: rC Clang CHANGES

[PATCH] D66722: [clang] Ensure that comment classes are trivially destructible

riccibruno created this revision. riccibruno added reviewers: aaron.ballman, Mordante. riccibruno added a project: clang. Herald added a subscriber: cfe-commits. As in D66646 , these classes are also allocated with a `BumpPtrAllocator`, and therefore should be

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. This patch introduces a new `analyzer-config` configuration:

[PATCH] D66706: [Wdocumentation] fixes an assertion failure with typedefed function and block pointer

gribozavr added inline comments. Comment at: clang/lib/AST/Comment.cpp:151 +static bool getFunctionTypeLoc(TypeLoc TL, FunctionTypeLoc , + bool testTypedefTypeLoc = false) { TypeLoc PrevTL; Mordante wrote: > gribozavr wrote: > >

[PATCH] D66700: [Wdocumentation] improve wording of a warning message

This revision was automatically updated to reflect the committed changes. Closed by commit rL369873: [Wdocumentation] improve wording of a warning message (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

r369873 - [Wdocumentation] improve wording of a warning message

Author: gribozavr Date: Sun Aug 25 11:20:18 2019 New Revision: 369873 URL: http://llvm.org/viewvc/llvm-project?rev=369873=rev Log: [Wdocumentation] improve wording of a warning message Based on @davezarzycki remarks in D64696 improved the wording of the warning message. Differential Revision:

[PATCH] D66669: [X86] Remove what little support we had for MPX

craig.topper updated this revision to Diff 217056. craig.topper added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Add to clang and llvm release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D9/new/

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

aganea updated this revision to Diff 217055. aganea marked 2 inline comments as done. aganea added a comment. As requested. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66556/new/ https://reviews.llvm.org/D66556 Files: lib/Lex/DependencyDirectivesSourceMinimizer.cpp

[PATCH] D66706: [Wdocumentation] fixes an assertion failure with typedefed function and block pointer

Mordante marked 2 inline comments as done. Mordante added inline comments. Comment at: clang/lib/AST/Comment.cpp:151 +static bool getFunctionTypeLoc(TypeLoc TL, FunctionTypeLoc , + bool testTypedefTypeLoc = false) { TypeLoc PrevTL;

[PATCH] D66706: [Wdocumentation] fixes an assertion failure with typedefed function and block pointer

gribozavr added inline comments. Comment at: clang/lib/AST/Comment.cpp:151 +static bool getFunctionTypeLoc(TypeLoc TL, FunctionTypeLoc , + bool testTypedefTypeLoc = false) { TypeLoc PrevTL; Why is the new functionality turned off

[PATCH] D66565: [analyzer] pr43036: Fix support for operator `sizeof...'.

a_sidorin accepted this revision. a_sidorin added a comment. LGTM++ Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66565/new/ https://reviews.llvm.org/D66565 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66715: [CFG] Add dumps for CFGElement and CFGElementRef

Szelethus added a comment. Mind that I'll probably commit without the unit test, last time I learned the hard way that the AST's lifetime ends by the time we retrieve the CFG. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66715/new/

[PATCH] D66711: [clang] Warning for non-final classes with final destructors

logan-5 updated this revision to Diff 217049. logan-5 added a comment. Addressed some feedback. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66711/new/ https://reviews.llvm.org/D66711 Files: clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

Szelethus added a comment. No worries, always happy to help! :) Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66014/new/ https://reviews.llvm.org/D66014 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66711: [clang] Warning for non-final classes with final destructors

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2204 "abstract class is marked '%select{final|sealed}0'">, InGroup; +def warn_final_destructor_nonfinal_class : Warning< + "class with destructor marked '%select{final|sealed}0'

[PATCH] D66538: [AST] AST structural equivalence to work internally with pairs.

a_sidorin added a comment. Hello Balasz, I have some comments inline. Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:1579 + D2 = D2->getCanonicalDecl(); + std::pair P = std::make_pair(D1, D2); + `std::pair P{D1, D2}`? Comment at:

[PATCH] D66711: [clang] Warning for non-final classes with final destructors

xbolva00 accepted this revision. xbolva00 added a comment. This revision is now accepted and ready to land. Looks fine Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66711/new/ https://reviews.llvm.org/D66711 ___

[PATCH] D66572: [analyzer] BugReporter Separation Ep.I.

Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:75 /// individual bug reports. class BugReport : public llvm::ilist_node { public: Shouldn't we make this an abstract class? CHANGES SINCE LAST

[PATCH] D66716: [analyzer] PR43102: Fix an assertion and an out-of-bounds error for diagnostic location construction

Szelethus created this revision. Szelethus added reviewers: NoQ, xazax.hun, baloghadamsoftware, Charusso, dcoughlin, rnkovacs, TWeaver. Szelethus added a project: clang. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity.

[PATCH] D66715: [CFG] Add dumps for CFGElement and CFGElementRef

Szelethus created this revision. Szelethus added reviewers: NoQ, xazax.hun, dcoughlin, rnkovacs, Charusso, baloghadamsoftware. Szelethus added a project: clang. Herald added subscribers: cfe-commits, gamesh411, dkrupp. Seems like we never had these, so here we go! I also did some refactoring as

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

chrish_ericsson_atx added a comment. @Szelethus, firstly, thank you for landing this change. I really appreciate it! Secondly, apologies for misspelling your name earlier. And lastly, thank you for your patient explanation of how to get the diffs updated correctly in a Phabricator review.

[PATCH] D66714: [analyzer] Don't run the analyzer for -analyzer-list-enabled-checkers

Szelethus created this revision. Szelethus added reviewers: NoQ, dcoughlin, baloghadamsoftware, Charusso, rnkovacs, xazax.hun. Szelethus added a project: clang. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity. Short and sweet.

[PATCH] D66646: Ensure that statements, expressions and types are trivially destructible with a static_assert

lebedev.ri accepted this revision. lebedev.ri added a subscriber: rsmith. lebedev.ri added a comment. This revision is now accepted and ready to land. SGTM but please wait a bit in case @aaron.ballman / @rsmith wants to comment. Thanks. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D66646: Ensure that statements, expressions and types are trivially destructible with a static_assert

riccibruno added a comment. > I was looking at SpecificBumpPtrAllocator, which knows it's type. > But if i look down the indirection, i think > AllocatorBase::Allocate()/AllocatorBase::Deallocate() is the place. I did look at that, but I think it won't work for two reasons: 1. AST nodes are

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

xbolva00 added a comment. Another bug found by -Wxor-used-as-pow :) https://github.com/christinaa/PcapPlusPlus/commit/4646a004f5168bcb78fe2dce78afa08d794c1450 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

jpakkane added a comment. I used stdint to replicate a real world use case as I'd imagine those types would match this search quite heavily. The tests already have one test for a typedeffed integer and one that is defined with a macro. If those are deemed sufficient, the stdint type can be

[PATCH] D66713: ContentCache: Drop getBuffer's dependency on SourceManager

dexonsmith updated this revision to Diff 217036. dexonsmith added a comment. (Updating the diff with full context.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66713/new/ https://reviews.llvm.org/D66713 Files: clang/include/clang/Basic/SourceManager.h

[PATCH] D66713: ContentCache: Drop getBuffer's dependency on SourceManager

dexonsmith created this revision. dexonsmith added reviewers: arphaman, bruno. Herald added a subscriber: ributzka. Refactor ContentCache::IsSystemFile to IsFileVolatile, checking SourceManager::userFilesAreVolatile at construction time. This is a step toward lowering ContentCache down from

[PATCH] D66712: [Driver] Enable ShadowCallStack, not SafeStack, by default on AArch64 Fuchsia

mcgrathr added reviewers: phosek, leonardchan, jakehehrlich. mcgrathr added a comment. This should land only after we've done our first stages of downstream roll-out and burn-in testing. But setting it up now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D66712: [Driver] Enable ShadowCallStack, not SafeStack, by default on AArch64 Fuchsia

mcgrathr created this revision. Herald added subscribers: cfe-commits, cryptoad, kristof.beyls, javed.absar. Herald added a project: clang. On AArch64, Fuchsia fully supports both SafeStack and ShadowCallStack ABIs. The latter is now preferred and will be the default. It's possible to enable