[PATCH] D75431: [analyzer][NFC] Merge checkNewAllocator's paramaters into CXXAllocatorCall

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:905 + SVal getObjectUnderConstruction(ProgramStateRef State) const { +return ExprEngine::getObjectUnderConstruction(State, getOriginExpr(), Why does

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. Must have been annoying, thanks a lot! Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:424 /// new-expression that goes before the constructor. - void processNewAllocation(const CXXNewExpr *NE, CheckerContext

[PATCH] D76211: OpenMP Metadirective with user defined condition

2020-03-15 Thread Alok Mishra via Phabricator via cfe-commits
alokmishra.besu created this revision. alokmishra.besu added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, martong, jfb, arphaman, guansong, jholewinski. Herald added a project: clang. jdoerfert added a comment. I think you forgot to add the tests to the commit :) This is a

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:713 + StringRef Name; + if (const auto *DRE = dyn_cast(ContE->IgnoreParenCasts())) { +Name = DRE->getDecl()->getName(); Hmm, i think you should instead look at

[PATCH] D68165: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. Looks wonderful, thanks! Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:355 + template + void checkRealloc(CheckerContext , const CallExpr *CE, +ProgramStateRef State) const;

[PATCH] D76211: OpenMP Metadirective with user defined condition

2020-03-15 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment. I think you forgot to add the tests to the commit :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76211/new/ https://reviews.llvm.org/D76211 ___ cfe-commits mailing list

[PATCH] D73898: [analyzer] StdLibraryFunctionsChecker: Add argument constraints

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. Looks great as long as other reviewers are happy, thanks! Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:409 - Optional FoundSummary = findFunctionSummary(FD, CE, C); + for (const

[PATCH] D75063: [analyzer] StdLibraryFunctionsChecker: Add NotNull Arg Constraint

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. This is basically a shorthand for "outside [0, 0]", right? I don't mind ^.^ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75063/new/

[PATCH] D76118: [Coroutines] Do not evaluate InitListExpr of a co_return

2020-03-15 Thread JunMa via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG53c2e10fb8a6: [Coroutines] Do not evaluate InitListExpr of a co_return (authored by junparser). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github

[clang] 53c2e10 - [Coroutines] Do not evaluate InitListExpr of a co_return

2020-03-15 Thread Jun Ma via cfe-commits
Author: Jun Ma Date: 2020-03-16T12:42:44+08:00 New Revision: 53c2e10fb8a606aa0cb092dda1219603cc3017cd URL: https://github.com/llvm/llvm-project/commit/53c2e10fb8a606aa0cb092dda1219603cc3017cd DIFF: https://github.com/llvm/llvm-project/commit/53c2e10fb8a606aa0cb092dda1219603cc3017cd.diff LOG:

[PATCH] D75529: [analyzer] Limit UCharMax to min of max uchar or max int

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG0eba5dc80fb0: [analyzer] Fix modeling some library functions when UCHAR_MAX INT_MAX. (authored by dergachev.a). Changed prior to commit: https://reviews.llvm.org/D75529?vs=250109=250464#toc

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h:217 +mgr.template registerChecker(); } Szelethus wrote: > baloghadamsoftware wrote: > > Why do we need `MGR` here as template parameter? Is this

[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:289 + NameRuleMap CustomPropagations{ + {"c_str", {"std::__cxx11::basic_string", {{0}, {ReturnValueIndex, + {"data", {"std::__cxx11::basic_string", {{0},

[PATCH] D75529: [analyzer] Limit UCharMax to min of max uchar or max int

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D75529#1921959 , @vabridgers wrote: > @Charusso -- I do not have commit access, but I will request. Thank you. Whoops, i accidentally landed this patch because i misread your statement as if you're asking to land the patch for

[clang] 0eba5dc - [analyzer] Fix modeling some library functions when UCHAR_MAX > INT_MAX.

2020-03-15 Thread Artem Dergachev via cfe-commits
Author: Artem Dergachev Date: 2020-03-16T07:16:44+03:00 New Revision: 0eba5dc80fb0a65b8bad00998c390ce3ec0c430f URL: https://github.com/llvm/llvm-project/commit/0eba5dc80fb0a65b8bad00998c390ce3ec0c430f DIFF:

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. I like this! Please address the remaining nits^^ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:102 + /// Given a range, should the argument stay

[PATCH] D75271: [analyzer][NFC] Change LangOptions to CheckerManager in the shouldRegister* functions.

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. Great, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75271/new/ https://reviews.llvm.org/D75271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D74615: [Analyzer] Add visitor to track iterator invalidation

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D74615#1917289 , @Szelethus wrote: > You may have explained it in the summary, and I didn't get it, but why isn't > putting a `NoteTag` in `invalidateAllIteratorPositions` sufficient? We could > state something like `All

[PATCH] D75851: [Analyzer][StreamChecker] Added evaluation of fseek.

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:470 - // Make the return value accordingly to the error. - State = State->assume(RetVal, (SS->*IsOfError)()); - assert(State && "Return value should not be constrained already."); -

[PATCH] D74541: [Analyzer] Use note tags to track iterator increments and decrements

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:489-490 +StringRef ChangeText = + ((Op == OO_Plus || Op == OO_PlusEqual) != (ChangeVal <= 0)) ? + "incremented" : "decremented"; +const NoteTag *ChangeTag =

[PATCH] D73521: [analyzer] add-new-checker.py: Introduction

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D73521#1920130 , @Charusso wrote: > - The script actually creates a real world (hidden) checker. > - This checker always made with the build invocation. > - Its test file always made with the build invocation. Mm, ok, but

[PATCH] D75677: [Analyzer] Only add iterator note tags to the operations of the affected iterators

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp:107-108 + BR.markInteresting(Iter); + if (const auto = Iter.getAs()) { +BR.markInteresting(LCV->getRegion()); + } What

[PATCH] D73521: [analyzer] add-new-checker.py: Introduction

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/utils/analyzer/add-new-checker.py:83 +except OSError as e: +print('[error] llvm-tblgen is not found, specify it explicitly.') +exit() Let's tell the user politely how to specify `llvm-tblgen`.

[PATCH] D74541: [Analyzer] Use note tags to track iterator increments and decrements

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:489-490 +StringRef ChangeText = + ((Op == OO_Plus || Op == OO_PlusEqual) != (ChangeVal <= 0)) ? + "incremented" : "decremented"; +const NoteTag *ChangeTag =

[PATCH] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. It looks like for now there's no way to put the stream into an error state, right? Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:33-45 + enum KindTy { +Opened, /// Stream is opened. +Closed, /// Closed stream (an invalid stream

[PATCH] D75514: [Analyzer] Only add container note tags to the operations of the affected container

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. Wonderful, thank you! Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:726 +<< Text; +return std::string(Out.str()); + }); I suspect you can remove the explicit

[PATCH] D75678: [analyzer] Skip analysis of inherited ctor as top-level function

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:511 + // + // Also, we cannot model the parameters. CXXInheritedCtorInitExpr doesn't + // take arguments and doesn't model parameter initialization because there is

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

2020-03-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h:25 -/// Get the stored dynamic size for the region \p MR. +/// \returns The stored dynamic size for the region \p MR. DefinedOrUnknownSVal

[PATCH] D57497: [RISCV] Passing small data limitation value to RISCV backend

2020-03-15 Thread Shiva Chen via Phabricator via cfe-commits
shiva0217 added a comment. In D57497#1921497 , @lenary wrote: > In D57497#1920870 , @shiva0217 wrote: > > > In D57497#1920097 , @apazos wrote: > > > > > Thanks Shiva, I

[PATCH] D76196: [ASTMatchers] Extend hasReturnValue to GNU StmtExpr

2020-03-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D76196#1923524 , @aaron.ballman wrote: > I don't think this is a natural fit for the functionality. A statement > expression doesn't have a return value so much as it is a value expression > that can contain multiple

[PATCH] D76204: [Fuchsia] Include llvm-gsymutil tool in the Fuchsia toolchain

2020-03-15 Thread Petr Hosek via Phabricator via cfe-commits
phosek created this revision. phosek added a reviewer: leonardchan. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. This tool is used for generating and manipulating GSYM files. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D76204 Files:

[PATCH] D76196: [ASTMatchers] Extend hasReturnValue to GNU StmtExpr

2020-03-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. I don't think this is a natural fit for the functionality. A statement expression doesn't have a return value so much as it is a value expression that can contain multiple statements. To me, at least, this is a bit like saying the comma operator has a return

[clang] a1e940b - [Driver][test] Add a specific test file for -fmerge-all-constants

2020-03-15 Thread Fangrui Song via cfe-commits
Author: Fangrui Song Date: 2020-03-15T13:11:49-07:00 New Revision: a1e940b1853a69b14b1f66952256e8cb16e6a0aa URL: https://github.com/llvm/llvm-project/commit/a1e940b1853a69b14b1f66952256e8cb16e6a0aa DIFF: https://github.com/llvm/llvm-project/commit/a1e940b1853a69b14b1f66952256e8cb16e6a0aa.diff

[PATCH] D76197: clang-format: Use block indentation for braced initializations

2020-03-15 Thread Daan De Meyer via Phabricator via cfe-commits
DaanDeMeyer updated this revision to Diff 250429. DaanDeMeyer added a comment. New implementation that breaks fewer tests. It seems to be expected that block kind is `BK_Unknown` for C struct braced init lists. Is that supposed to be the case? There's still quite some tests failures but most

[clang] 5cc9dea - [tblgen] Remove unused private field. NFC.

2020-03-15 Thread Benjamin Kramer via cfe-commits
Author: Benjamin Kramer Date: 2020-03-15T16:51:22+01:00 New Revision: 5cc9dea78a3b6c01849b03014dc9e684eeaf5d94 URL: https://github.com/llvm/llvm-project/commit/5cc9dea78a3b6c01849b03014dc9e684eeaf5d94 DIFF:

[PATCH] D76196: [ASTMatchers] Extend hasReturnValue to GNU StmtExpr

2020-03-15 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, klimek. Herald added a project: clang. Herald added a subscriber: cfe-commits. Extend hasReturnValue to match the last statement in a gnu statement expression if the last statement is an expression. Repository: rG LLVM

[PATCH] D75298: [Clang][SVE] Parse builtin type string for scalable vectors

2020-03-15 Thread Sander de Smalen via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5087ace65197: [Clang][SVE] Parse builtin type string for scalable vectors (authored by sdesmalen). Changed prior to commit: https://reviews.llvm.org/D75298?vs=249966=250421#toc Repository: rG LLVM

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

2020-03-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: lebedev.ri, JonasToth, ldionne. aaron.ballman added a comment. Herald added a subscriber: dexonsmith. In D74692#1923315 , @zinovy.nis wrote: > In D74692#1923191 , @Quuxplusone >

[clang] 5087ace - [Clang][SVE] Parse builtin type string for scalable vectors

2020-03-15 Thread Sander de Smalen via cfe-commits
Author: Sander de Smalen Date: 2020-03-15T14:34:52Z New Revision: 5087ace65197471c07b78d16e3d599187c442cbf URL: https://github.com/llvm/llvm-project/commit/5087ace65197471c07b78d16e3d599187c442cbf DIFF: https://github.com/llvm/llvm-project/commit/5087ace65197471c07b78d16e3d599187c442cbf.diff

[PATCH] D76181: [AVR] Add support for the -mdouble=x flag

2020-03-15 Thread Ayke via Phabricator via cfe-commits
aykevl marked 3 inline comments as done. aykevl added a comment. In D76181#1923176 , @MaskRay wrote: > The GCC side commits can be found on > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92055 > So it seems that we will have both

[PATCH] D76181: [AVR] Add support for the -mdouble=x flag

2020-03-15 Thread Ayke via Phabricator via cfe-commits
aykevl updated this revision to Diff 250418. aykevl added a comment. Fixed nits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76181/new/ https://reviews.llvm.org/D76181 Files: clang/include/clang/Basic/LangOptions.def

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

2020-03-15 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis added a comment. In D74692#1923191 , @Quuxplusone wrote: > I still think this entire patch is misguided; there's no reason to make the > note for `const std::string s; std::move(s)` any longer than the note for > `int i; std::move(i)` or

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-15 Thread Raul Tambre via Phabricator via cfe-commits
tambre added a comment. Your help here and over on CMake's side has been very helpful. Thank you! I'll @ you on CMake's side if I need any help while working on CUDA support. Hopefully you won't mind. :) I'm progressing on this and hope to have initial support in a mergeable state within two

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-15 Thread Raul Tambre via Phabricator via cfe-commits
tambre added a comment. In D75811#1923278 , @csigg wrote: > > I'll be adding a `CUDA_ROOT` option to CMake that will be passed to clang > > as `--cuda-path`. > > I'm not familiar with CMake and whether that option is picked up from an > environment

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-15 Thread Christian Sigg via Phabricator via cfe-commits
csigg added a comment. > I've gone with the approach of trying the architectures in the most recent > non-deprecated order – sm_52, sm_30. I'm curious why you added sm_52 (I'm currently writing bazel rules for better CUDA support, and I'm using just sm_30 because that's been nvcc's default for

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

2020-03-15 Thread Christian Sigg via Phabricator via cfe-commits
csigg added a comment. > I'll be adding a `CUDA_ROOT` option to CMake that will be passed to clang as > `--cuda-path`. I'm not familiar with CMake and whether that option is picked up from an environment variable, but on Windows that environment variable that the CUDA installer sets is