[PATCH] D92101: [Clang][Sema] Attempt to fix CTAD faulty copy of non-local typedefs

martong updated this revision to Diff 308335. martong marked 2 inline comments as done. martong added a comment. - Check for dependent DeclContext - Remove "dump()" calls from tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92101/new/

[PATCH] D92307: [analyzer][StdLibraryFunctionsChecker] Fix typos in summaries of mmap and mmap64

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Looks good, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92307/new/ https://reviews.llvm.org/D92307

[PATCH] D92307: [analyzer][StdLibraryFunctionsChecker] Fix typos in summaries of mmap and mmap64

martong added a comment. In D92307#2422468 , @vsavchenko wrote: > That's a good fix! > How did this happen though that max value of `off_t` was even used for `fd`. > Seems pretty odd! I used a semi-automated approach to create these summaries from

[PATCH] D92109: [ASTImporter] Support import of CXXDeductionGuideDecl

martong added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:5900 + ASSERT_TRUE(ToD); + EXPECT_EQ(FromD->isCopyDeductionCandidate(), ToD->isCopyDeductionCandidate()); + // Check that the deduced class template is also imported.

[PATCH] D92109: [ASTImporter] Support import of CXXDeductionGuideDecl

martong updated this revision to Diff 307988. martong marked 2 inline comments as done. martong added a comment. - Update test to match the copy deduction guide Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92109/new/

[PATCH] D92209: [ASTImporter] Support CXXDeductionGuideDecl with local typedef

martong created this revision. martong added reviewers: teemperor, balazske. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: clang. martong requested review of this revision.

[PATCH] D92109: [ASTImporter] Support import of CXXDeductionGuideDecl

martong added a comment. Thanks for the review! Your comment made me think about whether we handle the deduced template class properly (`getDeducedTemplate`). Then I realized we do import that when we import the `DeclarationName`. Anyway I added a check for that in the test. Repository: rG

[PATCH] D92109: [ASTImporter] Support import of CXXDeductionGuideDecl

martong updated this revision to Diff 307848. martong added a comment. - Add test for user-defined ctad - Handle IsCopyDeductionCandidate - Fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92109/new/ https://reviews.llvm.org/D92109 Files:

[PATCH] D92109: [ASTImporter] Support import of CXXDeductionGuideDecl

martong created this revision. martong added reviewers: teemperor, shafik, balazske. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a project: clang. martong requested review of this revision. CXXDeductionGuideDecl

[PATCH] D92101: [Clang][Sema] Attempt to fix CTAD faulty copy of non-local typedefs

martong added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:5893 + // in the global scope. + EXPECT_EQ(Param->getType()->getAs()->getDecl(), Typedef); +} Note, this "expectation" fails in baseline. Repository: rG LLVM Github Monorepo

[PATCH] D92101: [Clang][Sema] Attempt to fix CTAD faulty copy of non-local typedefs

martong created this revision. martong added a reviewer: rsmith. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: shafik. Herald added a project: clang. martong requested review of this revision.

[PATCH] D91104: APINotes: add property models for YAML attributes

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Thanks, looks good to me! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91104/new/ https://reviews.llvm.org/D91104

[PATCH] D91104: APINotes: add property models for YAML attributes

martong added inline comments. Comment at: clang/include/clang/APINotes/Types.h:60 + /// Whether SwiftPrivate was specified. + unsigned SwiftPrivateSpecified : 1; + I was wondering whether Swift specific properties are meaningful to all descendants of

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Looks good to me now! Thanks for addressing my concerns. Comment at: clang/tools/apinotes-test/APINotesTest.cpp:15 + +static

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

martong added inline comments. Comment at: clang/include/clang/APINotes/Types.h:25 +/// auditing. +enum class EnumExtensibilityKind { + None, compnerd wrote: > martong wrote: > > compnerd wrote: > > > martong wrote: > > > > compnerd wrote: > > > > > martong

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

martong added inline comments. Comment at: clang/lib/APINotes/APINotesYAMLCompiler.cpp:33 +namespace yaml { +template <> +struct ScalarEnumerationTraits { Could you please clang-format the code? The `Lint: Pre-merge checks` are all over the code and makes the

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

martong added a comment. In D88859#2359399 , @compnerd wrote: >> I'd like to have maximum flexibility from the submitter by willing to change >> the details of the implementation > > I don't think that is a fair expectation - there are plenty of times

[PATCH] D63279: [Analyzer] Unroll for-loops where the upper boundary is a variable with know value

martong added a comment. FYI, I am working on repeating and reevaluating Peter's measurements with the current master. And if possible then I'd like to extend the measures for new C and C++ projects. I am planning to

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

martong added a comment. In D88859#2354377 , @gribozavr2 wrote: >> I am having a hard time to accept "this is how it is implemented in our >> fork" as a technical argument. Besides, I am not sure how could the Clang >> community benefit about being

[PATCH] D90157: [analyzer] Rework SValBuilder::evalCast function into maintainable and clear way

martong added a comment. > Now I'm going to make unittests for SValBuilder::evalCast. Let me ask you to > offer me some examples of casts to check. As many cases we can collect as > robust the test would be. E.g. This is not going to be easy. I guess we should cover both implicit and explicit

[PATCH] D89528: [clang][test] Fix prefix operator++ signature in iterators

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Ok, thanks for the context. LG! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89528/new/

[PATCH] D89318: [ASTImporter] Fix crash caused by unset AttributeSpellingListIndex

martong added a comment. Unfortunately, I could not reproduce the mentioned crash on our macOS machine. The mentioned test just passed with the output below. I gave up. myuser@msmarple ~/llvm3/build/release_assert $ /usr/local/Frameworks/Python.framework/Versions/3.8/bin/python3.8

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

martong added a reviewer: NoQ. martong added a subscriber: NoQ. martong added a comment. Herald added a subscriber: Charusso. Adding @NoQ as a reviewer, he might have some valuable comments, as he is very keen on this feature. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

martong added a comment. > as it is something which is actually in production already > ... > At the very least we need it for compatibility - this is already a shipping > feature. I value that you guys have a prototype on a fork that is being used in production. But, upstreaming and these

[PATCH] D89528: [clang][test] Fix prefix operator++ signature in iterators

martong added a comment. What is the context here? Did it cause any crash/bug or were you just reading through the code for a good night sleep? :D Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89528/new/ https://reviews.llvm.org/D89528

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

martong added a comment. Seems like this patch handles already existing attributes in Clang, so this might not be affected by the concerns brought up here by James: http://lists.llvm.org/pipermail/cfe-dev/2020-October/066996.html Comment at:

[PATCH] D63640: [clang] Improve Serialization/Imporing of APValues

martong added a comment. In D63640#2331779 , @Tyker wrote: > In D63640#2331734 , @martong wrote: > >> In D63640#2331410 , @rsmith wrote: >> >>> Reverse ping: I have a patch

[PATCH] D63640: [clang] Improve Serialization/Imporing of APValues

martong added a comment. In D63640#2331410 , @rsmith wrote: > Reverse ping: I have a patch implementing class type non-type template > parameters that's blocked on this landing. If you won't have time to address > @martong's comments soon, do you mind

[PATCH] D89318: [ASTImporter] Fix crash caused by unset AttributeSpellingListIndex

martong added a comment. Ups, sorry for the break and thanks for the notice! I am going to investigate. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89318/new/ https://reviews.llvm.org/D89318 ___

[PATCH] D89318: [ASTImporter] Fix crash caused by unset AttributeSpellingListIndex

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. martong marked an inline comment as done. Closed by commit rG73c6beb2f705: [ASTImporter] Fix crash caused by unset AttributeSpellingListIndex (authored by martong).

[PATCH] D89318: [ASTImporter] Fix crash caused by unset AttributeSpellingListIndex

martong marked an inline comment as done. martong added a comment. Thanks for the reivew! Comment at: clang/lib/AST/ASTImporter.cpp:8109 + FromAttr->getAttributeSpellingListIndex()); + ToAttr->setPackExpansion(FromAttr->isPackExpansion()); +

[PATCH] D89319: [ASTImporter] Fix crash caused by unimported type of FromatAttr

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. martong marked an inline comment as done. Closed by commit rGdd965711c9f0: [ASTImporter] Fix crash caused by unimported type of FromatAttr (authored by martong).

[PATCH] D89319: [ASTImporter] Fix crash caused by unimported type of FromatAttr

martong added a comment. Thanks for the review! Comment at: clang/test/ASTMerge/attr/Inputs/FormatAttr.cpp:2 +int foo(const char * fmt, ...) +__attribute__ ((__format__ (__scanf__, 1, 2))); teemperor wrote: > (Not sure if we care about that in tests, but

[PATCH] D89319: [ASTImporter] Fix crash caused by unimported type of FromatAttr

martong created this revision. martong added reviewers: teemperor, shafik. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a project: clang. martong requested review of this revision. During the import of FormatAttrs

[PATCH] D89318: [ASTImporter] Fix crash caused by unset AttributeSpellingListIndex

martong created this revision. martong added reviewers: teemperor, shafik. Herald added subscribers: cfe-commits, pengfei, gamesh411, Szelethus, arphaman, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a project: clang. martong requested review of this revision. During the

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG007dd12d5468: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl (authored by martong). Repository: rG LLVM Github

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

martong added a comment. > Currently, we are totally inconsistent about the diagnostics. Either we > should emit a diagnostic before all return false or we should not ever emit > any diags. The diagnostics in their current form are misleading, because > there could be many notes missing. I am

[PATCH] D63640: [clang] Improve Serialization/Imporing of APValues

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:9010 + ToPath[Idx] = + cast(const_cast(ImpDecl.get())); +} Tyker wrote: > rsmith wrote: > > We want the path in an `APValue` to be canonical, but importing a canonical >

[PATCH] D63640: [clang] Improve Serialization/Imporing of APValues

martong requested changes to this revision. martong added a comment. This revision now requires changes to proceed. Sorry for the late review, I just noticed something which is not a logical error, but we could make the ASTImporter code much cleaner. Comment at:

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

martong updated this revision to Diff 295800. martong added a comment. - Delegate to BitWidth Expr matching in case of BitFields Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88665/new/ https://reviews.llvm.org/D88665 Files:

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

martong added a comment. > In D88665#2307562 , @shafik wrote: > >> So was the bug we were saying there were falsely equivalent or falsely not >> equivalent? > > I think the bug is the crash :) Yes. More specifically, the call of `getBitWidthValue()`

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

martong added a comment. The added test causes the below assertion in the baseline (w/o the fix): ASTTests: ../../git/llvm-project/clang/lib/AST/ExprConstant.cpp:14543: llvm::APSInt clang::Expr::EvaluateKnownConstInt(const clang::ASTContext&, llvm::SmallVectorImpl >*) const: Assertion

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

martong created this revision. martong added reviewers: balazske, teemperor, shafik, a_sidorin. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a project: clang. martong requested review of this revision. Repository:

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

martong added a comment. In D88019#2296337 , @steakhal wrote: > In D88019#2291953 , @steakhal wrote: > >> What are our options mitigating anything similar happening in the future? >> >> This way any change touching

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

martong added a comment. > In this example, it cast to the `unsigned char` (which is the type of the > stored value of `**b`, stored at `#1`) instead of the static type of the > access (the type of `**b` is `char`at `#2`). Possible typo in the summary. At `#2` the type should be `char *`

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

martong added a comment. Beware, Phabricator ruins the visual experience of this nice analysis. E.g `//char ***//` is visible as an italic `char *`. > I think we should have a symbolic cast back to the static type before doing > anything with the SVal (iff the BaseKind differs). > If we do

[PATCH] D88100: [analyzer][StdLibraryFunctionsChecker] Separate the signature from the summaries

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG11d2e63ab006: [analyzer][StdLibraryFunctionsChecker] Separate the signature from the summaries (authored by martong). Repository: rG LLVM Github

[PATCH] D88092: [analyzer][StdLibraryFunctionsChecker] Fix getline/getdelim signatures

This revision was automatically updated to reflect the committed changes. Closed by commit rGd63a945a1304: [analyzer][StdLibraryFunctionsChecker] Fix getline/getdelim signatures (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88092: [analyzer][StdLibraryFunctionsChecker] Fix getline/getdelim signatures

martong added a comment. In D88092#2289312 , @Szelethus wrote: > A joy of reviewing C++ code is that you get to marvel in all the great things > the language has, without having to pull fistfuls of hair out to get get to > that point. These patches are

[PATCH] D88100: [analyzer][StdLibraryFunctionsChecker] Separate the signature from the summaries

martong added a comment. Thanks for the review! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88100/new/ https://reviews.llvm.org/D88100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D88100: [analyzer][StdLibraryFunctionsChecker] Separate the signature from the summaries

martong created this revision. martong added reviewers: balazske, Szelethus, steakhal, NoQ, vsavchenko. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity. Herald

[PATCH] D88092: [analyzer][StdLibraryFunctionsChecker] Fix getline/getdelim signatures

martong created this revision. martong added reviewers: balazske, Szelethus, steakhal, NoQ, vsavchenko. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity. Herald

[PATCH] D77792: [analyzer] Extend constraint manager to be able to compare simple SymSymExprs

martong added a reviewer: vsavchenko. martong added a comment. Adding Valeriy as a reviewer. He's been working with the solver recently, so his insights might be really useful here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77792/new/ https://reviews.llvm.org/D77792

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0c4f91f84b2e: [analyzer][solver] Fix issue with symbol non-equality tracking (authored by martong). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

martong updated this revision to Diff 293168. martong marked an inline comment as done. martong added a comment. - Avoid same pattern when checking whether the assumption is infeasible Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88019/new/

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

martong added a comment. Thanks for the quick review! I updated according to your suggestion, so we avoid the same pattern. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1350-1371 ProgramStateRef trackEQ(ProgramStateRef State, SymbolRef Sym,

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

martong added a comment. @steakhal, thank you for your time and huge effort in debugging this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88019/new/ https://reviews.llvm.org/D88019 ___ cfe-commits

[PATCH] D82445: [analyzer][solver] Track symbol equivalence

martong added a comment. We came up with a quick fix, let us know what you think: https://reviews.llvm.org/D88019 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82445/new/ https://reviews.llvm.org/D82445

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

martong created this revision. martong added reviewers: vsavchenko, steakhal, NoQ. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity. Herald added a

[PATCH] D87785: [analyzer][StdLibraryFunctionsChecker] Fix a BufferSize constraint crash

martong abandoned this revision. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:694 // execution continues on a code whose behaviour is undefined. assert(SuccessSt); NewState = SuccessSt;

[PATCH] D82445: [analyzer][solver] Track symbol equivalence

martong added a subscriber: steakhal. martong added a comment. Hi Valery, Together with @steakhal we have found a serious issue. The below code crashes if you compile with `-DEXPENSIVE_CHECKS`. The analyzer goes on an unfeasible path, the State has a contradiction. We think that

[PATCH] D87785: [analyzer][StdLibraryFunctionsChecker] Fix a BufferSize constraint crash

martong added a comment. Alright, I refactored the change a bit. A Constraint::assume works similarly to an engine DualAssume. Plus I added `isApplicable` to check if it is meaningful to call the assume at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D87785: [analyzer][StdLibraryFunctionsChecker] Fix a BufferSize constraint crash

martong updated this revision to Diff 292550. martong added a comment. - apply -> assume, Add isApplicable Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87785/new/ https://reviews.llvm.org/D87785 Files:

[PATCH] D79432: [analyzer] StdLibraryFunctionsChecker: Add summaries for libc

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1123-1124 + "abs", Summary(ArgTypes{IntTy}, RetType{IntTy}, EvalCallAsPure) + .Case({ArgumentCondition(0, WithinRange, SingleValue(0)), +

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. In D86660#2278025 , @shafik wrote: > We are going to move forward with this approach (after dealing with the > multi-dimensional array case)

[PATCH] D87785: [analyzer][StdLibraryFunctionsChecker] Fix a BufferSize constraint crash

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:694 // execution continues on a code whose behaviour is undefined. assert(SuccessSt); NewState = SuccessSt; This is where we crashed

[PATCH] D87785: [analyzer][StdLibraryFunctionsChecker] Fix a BufferSize constraint crash

martong created this revision. martong added reviewers: steakhal, balazske, Szelethus, NoQ. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity. Herald added a

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

martong added a comment. > Actually, the test added here could have catched that regression. Correction: It is the new BufferSize argument constraint in `fread`'s summary and the existing test `test_fread_uninitialized` in `std-c-library-functions.c` that could have catched that regression.

[PATCH] D87683: [clang-tidy] Crash fix for bugprone-misplaced-pointer-arithmetic-in-alloc

martong added a comment. In D87683#2274663 , @baloghadamsoftware wrote: > In D87683#2274569 , @njames93 wrote: > >> Please fix the test case first, can't call `operator new(unsigned long, >> void*)` with an

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

This revision was automatically updated to reflect the committed changes. Closed by commit rGa012bc4c42e4: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite (authored by martong). Changed prior to commit: https://reviews.llvm.org/D87081?vs=291606=291919#toc

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

martong added a comment. There is a blatant regression we introduced in D87240 . Actually, the test added here could have catched that regression. See https://reviews.llvm.org/D87240#inline-812062 I am putting back the modeling dependency with this patch.

[PATCH] D87240: [analyzer][StdLibraryFunctionsChecker] Have proper weak dependencies

martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:352 HelpText<"Improve modeling of the C standard library functions">, - Dependencies<[CallAndMessageModeling]>, CheckerOptions<[ Umm, we should not have

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

martong updated this revision to Diff 291606. martong added a comment. - Add tests to verify compatibility of the two checkers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87081/new/ https://reviews.llvm.org/D87081 Files:

[PATCH] D87519: [analyzer][Liveness][NFC] Enqueue the CFGBlocks post-order

martong added inline comments. Comment at: clang/lib/Analysis/LiveVariables.cpp:522 - // FIXME: we should enqueue using post order. - for (const CFGBlock *B : cfg->nodes()) { + for (const CFGBlock *B : *AC.getAnalysis()) { worklist.enqueueBlock(B);

[PATCH] D87518: [analyzer][Liveness][NFC] Remove an unneeded pass to collect variables that appear in an assignment

martong added inline comments. Comment at: clang/include/clang/Analysis/CFG.h:1311 + + llvm::iterator_range nodes() { return *this; } + llvm::iterator_range const_nodes() const { return *this; } martong wrote: > Do we convert `this` to `CFGBlock *Entry` here?

[PATCH] D87518: [analyzer][Liveness][NFC] Remove an unneeded pass to collect variables that appear in an assignment

martong added a comment. > This problem only exists if we traverse a CFGBlock in order. And Liveness in > fact does it reverse order. So a distinct pass is indeed unnecessary, we can > note the appearance of the assignment by the time we reach the variable. Should this patch depend on

[PATCH] D87527: [ASTMatchers] Fix `hasBody` for the descendants of `FunctionDecl`

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. > Unfortunately template specializations do not catch the descendants of the > class for which the template is specialized. Therefore it does not work > correcly for the descendants of

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa97648b93846: [analyzer][StdLibraryFunctionsChecker] Add better diagnostics (authored by martong). Changed prior to commit:

[PATCH] D87239: [analyzer][StdLibraryFunctionsChecker] Remove strcasecmp

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb7586afc4dcd: [analyzer][StdLibraryFunctionsChecker] Remove strcasecmp (authored by martong). Changed prior to commit:

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

martong added a comment. In D79431#2265155 , @Szelethus wrote: > In D79431#2263693 , @martong wrote: > >> In D79431#2263690 , @martong wrote: >> >>> What if we'd add a

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

martong added a comment. In D79431#2263690 , @martong wrote: > In D79431#2215610 , @Szelethus wrote: > >> Ah, okay, silly me. Didn't catch that. Then the message is OK for now. >> >> edit: Describing //how// the

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

martong added a comment. In D79431#2215610 , @Szelethus wrote: > Ah, okay, silly me. Didn't catch that. Then the message is OK for now. > > edit: Describing //how// the violation happened might be a valuable for > development purposes as well. What if

[PATCH] D87239: [analyzer][StdLibraryFunctionsChecker] Remove strcasecmp

martong added a comment. @steakhal Ping. > I completely agree with you. So, how should we proceed? :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87239/new/ https://reviews.llvm.org/D87239 ___

[PATCH] D76604: [Analyzer] Model `size()` member function of containers

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. This basically looks good to me. Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:482-483 +// of the container (the difference between its `begin()`

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

martong accepted this revision. martong added a comment. LGTM! Thanks for the clarification and the example you gave. (I agree with @steakhal and I wish if we could get rid of the many lines not-descriptive plist stuff, but that is rather unrelated) CHANGES SINCE LAST ACTION

[PATCH] D86445: [analyzer][RFC] Simplify MetadataSymbol representation and resolve a CStringChecker FIXME

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2384-2385 if (SuperRegions.count(MR)) { Entries = F.remove(Entries, MR); + Entries = F.add(Entries, MR, UnknownVal()); continue; martong

[PATCH] D86445: [analyzer][RFC] Simplify MetadataSymbol representation and resolve a CStringChecker FIXME

martong added a comment. > But if the string is invalidated (or the new length is completely unknown), > **do not remove** the length from the state; instead, set it to > SymbolConjured. Where exactly do you implement the above? > When strlen(R) is used for the first time on a region R,

[PATCH] D87239: [analyzer][StdLibraryFunctionsChecker] Remove strcasecmp

martong marked an inline comment as done. martong added a comment. In D87239#2259345 , @steakhal wrote: > I completely agree with you. > I plan to further refactor the CStringChecker, but the related patches are > pretty much stuck :D > > I think this

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

martong added a comment. In D87081#2258637 , @Szelethus wrote: > The patch looks great, in fact, it demonstrates how well thought out your > summary crafting machinery is. > > In D87081#2258579 , @martong wrote: >

[PATCH] D87240: [analyzer][StdLibraryFunctionsChecker] Have proper weak dependencies

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8248c2af9497: [analyzer][StdLibraryFunctionsChecker] Have proper weak dependencies (authored by martong). Changed prior to commit:

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

This revision was automatically updated to reflect the committed changes. Closed by commit rGd01280587d97: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions (authored by martong). Changed prior to commit: https://reviews.llvm.org/D84415?vs=289970=290296#toc

[PATCH] D87240: [analyzer][StdLibraryFunctionsChecker] Have proper weak dependencies

martong created this revision. martong added reviewers: Szelethus, balazske, NoQ, vsavchenko. Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity. Herald

[PATCH] D87239: [analyzer][StdLibraryFunctionsChecker] Remove strcasecmp

martong created this revision. martong added reviewers: Szelethus, balazske, NoQ, vsavchenko. Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity. Herald

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

martong added a comment. In D87081#2256636 , @balazske wrote: > This checker will make an additional assumption on `fread` and `fwrite` with > the ReturnValueCondition. There is nothing new in that. This assumption described by the `.Case` has been

[PATCH] D84248: [analyzer][StdLibraryFunctionsChecker] Add POSIX time handling functions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf0b9dbcfc7ba: [analyzer][StdLibraryFunctionsChecker] Add POSIX time handling functions (authored by martong). Repository: rG LLVM Github Monorepo

[PATCH] D84248: [analyzer][StdLibraryFunctionsChecker] Add POSIX time handling functions

martong added a comment. In D84248#2215519 , @Szelethus wrote: > Lets make sure we invite the wider community to see whats going on. > Otherwise, LGTM! I am committing this because it already has two accepts, plus I am //very// confident with the

[PATCH] D84248: [analyzer][StdLibraryFunctionsChecker] Add POSIX time handling functions

martong updated this revision to Diff 289982. martong added a comment. - Rebase to master, ie using optionals everywhere Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84248/new/ https://reviews.llvm.org/D84248 Files:

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

martong added a comment. Polite ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84415/new/ https://reviews.llvm.org/D84415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

martong updated this revision to Diff 289970. martong added a comment. - Rebase to master Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84415/new/ https://reviews.llvm.org/D84415 Files:

[PATCH] D87138: [analyzer] Introduce refactoring of PthreadLockChecker

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. This seems to be a total non-functional-change. Please include [NFC] next time with a similar refactoring. Otherwise, Looks good to me, thanks! Repository: rG LLVM Github Monorepo

<    5   6   7   8   9   10   11   12   13   14   >