[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

martong added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:26 + + if (NewExpr->getNumPlacementArgs() > 0) +return; Perhaps we should add in the docs that placement new is not supported. Or add a fixme here. Anyw

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

martong added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:38 +return; + + ASTContext &Context = D->getASTContext(); Would it make sense to early return here if the language dialect is >= C++17 ? =

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

martong added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + martong wrote:

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

martong added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + aaron.ballman w

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

martong added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + martong wrote:

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

martong added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + aaron.ballman w

[PATCH] D66951: [ASTImporter] Add comprehensive tests for ODR violation handling strategies

This revision was automatically updated to reflect the committed changes. Closed by commit rL372564: [ASTImporter][NFC] Add comprehensive tests for ODR violation handling strategies (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Change

[PATCH] D66951: [ASTImporter] Add comprehensive tests for ODR violation handling strategies

martong added a comment. Trying to fix in svn commit 372633. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66951/new/ https://reviews.llvm.org/D66951 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

[PATCH] D66951: [ASTImporter] Add comprehensive tests for ODR violation handling strategies

martong added a comment. 2nd attempt to fix the windows build errors: 372646 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66951/new/ https://reviews.llvm.org/D66951 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D46019: [ASTImporter] Fix isa cast assert

martong created this revision. martong added reviewers: a.sidorin, xazax.hun, szepet. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Do early return if we can't import the found decl for a member expr. This follows the pre-existing scheme, e.g with E->getMemberDecl(). Repository: rC

[PATCH] D46019: [ASTImporter] Fix isa cast assert

martong added a comment. Hi Aleksei, Thanks for the review. We faced this assert during the CTU analysis of protobuf. We tried hard to synthesize a minimal test example both by hand and by executing creduce on multiple files. Unfortunately, we were unable to reduce to such a minimal example,

[PATCH] D38845: [ASTImporter] Support importing UnresolvedMemberExpr, DependentNameType, DependentScopeDeclRefExpr

martong added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:1556 + // Converting code texts into TUs + std::transform(Codes.begin(), Codes.end(), std::back_inserter(FromTUs), + [this](StringRef Code) { Instead of having 4 `FromT

[PATCH] D46019: [ASTImporter] Fix isa cast assert

martong added a comment. Guys, I still don't have commit rights, could you please help me with the commit? (I think one more good quality patch and then I could request commit rights for myself ...) Repository: rC Clang https://reviews.llvm.org/D46019

[PATCH] D46353: [ASTImporter] Extend lookup logic in class templates

martong created this revision. martong added reviewers: a.sidorin, xazax.hun, szepet. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. During import of a class template, lookup may find a forward declaration and structural match falsely reports equivalency in between a fwd decl and a def

[PATCH] D46353: [ASTImporter] Extend lookup logic in class templates

martong added a comment. > During import of a class template, lookup may find a forward declaration and > structural match falsely reports equivalency in between a fwd decl and a > definition. This can happen when the class to be imported does not have any data members. Structural equivalency

[PATCH] D46019: [ASTImporter] Fix isa cast assert

martong added a comment. ping... @xazax.hun could you please help me with the commit? Repository: rC Clang https://reviews.llvm.org/D46019 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D38845: [ASTImporter] Support importing UnresolvedMemberExpr, DependentNameType, DependentScopeDeclRefExpr

martong accepted this revision. martong added a comment. LGTM! Comment at: unittests/AST/ASTImporterTest.cpp:1505 ::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); +const internal::VariadicDynCastAllOfMatcher +dependentScopeDeclRefExpr; ---

[PATCH] D38845: [ASTImporter] Support importing UnresolvedMemberExpr, DependentNameType, DependentScopeDeclRefExpr

martong added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:1532 +dependentNameType; +TEST(ImportExpr, DependentNameType) { + MatchVerifier Verifier; Perhaps a newline before would make it more independent from the Matcher, as you did it at

[PATCH] D46353: [ASTImporter] Extend lookup logic in class templates

martong added a comment. > should we add this new declaration to the redeclaration chain like we do it > for RecordDecls? I think, on a long term we should. Otherwise we could loose e.g. C++11 attributes which are attached to the forward declaration only. However, I'd do that as a separate comm

[PATCH] D46398: [ASTImporterTest] Fix potential use-after-free

martong added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:211 StringRef Code; StringRef FileName; std::unique_ptr Unit; Can't we have the same problem with FileName? Perhaps an other alternative would be to make the members real

[PATCH] D47534: [ASTImporter] Add new tests about templated-described swing

martong updated this revision to Diff 152660. martong marked 2 inline comments as done. martong added a comment. - Clang format the test code snippet. Repository: rC Clang https://reviews.llvm.org/D47534 Files: unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp ==

[PATCH] D47534: [ASTImporter] Add new tests about templated-described swing

martong added a comment. I addressed the comments, thanks for the review! Repository: rC Clang https://reviews.llvm.org/D47534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47534: [ASTImporter] Add new tests about templated-described swing

This revision was automatically updated to reflect the committed changes. Closed by commit rL335455: [ASTImporter] Add new tests about templated-described swing (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47534 F

[PATCH] D47367: [ASTImporter] Add ms compatibility to tests

martong updated this revision to Diff 152674. martong added a comment. - Update commit comment and fix broken format in a comment. Repository: rC Clang https://reviews.llvm.org/D47367 Files: unittests/AST/ASTImporterTest.cpp unittests/AST/Language.cpp unittests/AST/Language.h Index: u

[PATCH] D47367: [ASTImporter] Add ms compatibility to tests

This revision was automatically updated to reflect the committed changes. Closed by commit rC335464: [ASTImporter] Add ms compatibility to tests which use the TestBase (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D47367?vs=152674&id=152675#toc Reposit

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong updated this revision to Diff 152691. martong marked 3 inline comments as done. martong added a comment. - Address review comments Repository: rC Clang https://reviews.llvm.org/D47532 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp lib/AST/DeclBase.cpp test/AST

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:2021 + +TEST_P(ImportFriendFunctions, + DISABLED_ImportFriendFunctionRedeclChainDefWithClass_ImportTheProto) { a_sidorin wrote: > Could you add comments why these tests are disable

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

This revision was automatically updated to reflect the committed changes. Closed by commit rC335480: [ASTImporter] Import the whole redecl chain of functions (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D47532?vs=152691&id=152693#toc Repository: rC

[PATCH] D47632: [ASTImporter] Refactor Decl creation

martong updated this revision to Diff 152703. martong added a comment. - Rebase from master. Repository: rC Clang https://reviews.llvm.org/D47632 Files: include/clang/AST/ASTImporter.h include/clang/AST/DeclBase.h lib/AST/ASTImporter.cpp lib/AST/ExternalASTMerger.cpp Index: lib/AST/

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong added a comment. @labath Sure, looking into it. Repository: rC Clang https://reviews.llvm.org/D47532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong added a comment. This is not trivial to fix. Reverting until we can reproduce and fix it. Reverted with commit: r335491 Repository: rC Clang https://reviews.llvm.org/D47532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

This revision was automatically updated to reflect the committed changes. Closed by commit rC335600: [ASTImporter] Use InjectedClassNameType at import of templated record. (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D47450?vs=152634&id=152880#toc Rep

[PATCH] D47632: [ASTImporter] Refactor Decl creation

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:1659 + AccessSpecDecl *ToD; + std::tie(ToD, AlreadyImported) = CreateDecl( + D, Importer.getToContext(), D->getAccess(), DC, Loc, ColonLoc); a.sidorin wrote: > As I see, all usage samples r

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong added a comment. The broken lldb tests are fixed with a minor change. We no longer load the Decls from the external source during the call of `DeclContext::containsDecl`. A new function `DeclContext::containsDeclAndLoad` is added which does a load and calls `containsDecl`. Re-apply comm

[PATCH] D47459: [ASTImporter] Eliminated some unittest warnings.

This revision was automatically updated to reflect the committed changes. Closed by commit rL335959: [ASTImporter] Eliminated some unittest warnings. (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47459 Files: cfe

[PATCH] D48631: [ASTImporter] Added import of CXXStdInitializerListExpr

This revision was automatically updated to reflect the committed changes. Closed by commit rL335968: [ASTImporter] Added import of CXXStdInitializerListExpr (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D48631 Files

[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types

martong created this revision. martong added reviewers: a.sidorin, balazske, r.stahl. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Currently, anonymous types are merged into the same redecl chain even if they are structurally inequivalent. This results that global objects are not impor

[PATCH] D47632: [ASTImporter] Refactor Decl creation

martong added a comment. I realized that, this patch brakes 3 lldb tests ATM: - `TestTopLevelExprs.py`. If https://reviews.llvm.org/D48722 was merged then this test would not be broken. - `TestPersistentTypes.py` If https://reviews.llvm.org/D48773 was merged then this test would not be broken.

[PATCH] D48722: [ASTImporter] Update isUsed flag at Decl import.

martong added a comment. > I have a strong feeling of duplication with attribute and flags merging move > in https://reviews.llvm.org/D47632. Maybe it is better to be resolved in that > review by using the same code for attr/flag merging for both newly-created > and mapped decls? Ok, then I'll

[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types

martong updated this revision to Diff 153917. martong marked an inline comment as done. martong added a comment. Remove redundant code and use only StructurlaEquivalence Repository: rC Clang https://reviews.llvm.org/D48773 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp

[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:2085 } + } else { +if (!IsStructuralMatch(D, FoundRecord, false)) a_sidorin wrote: > Is it possible to use the added code for the entire condition `if (auto > *F

[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types

martong marked an inline comment as done. martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:2085 } + } else { +if (!IsStructuralMatch(D, FoundRecord, false)) balazske wrote: > martong wrote: > > a_sidorin wrote:

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:2559 D->isImplicit()); +ToFunction->setRangeEnd(Importer.Import(D->getLocEnd())); } else if (auto *FromConversion = dyn_cast(D)) { Why don't we n

[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types

martong updated this revision to Diff 154187. martong added a comment. `PrevDecl` is set after the `!SearchName` branch, as it had been done before. This way original behaviour is kept as much as possible. Repository: rC Clang https://reviews.llvm.org/D48773 Files: lib/AST/ASTImporter.cpp

[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types

This revision was automatically updated to reflect the committed changes. Closed by commit rC336332: [ASTImporter] Fix import of objects with anonymous types (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D48773?vs=154187&id=154192#toc Repository: rC

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang https://reviews.llvm.org/D48941 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D47632: [ASTImporter] Refactor Decl creation

martong updated this revision to Diff 154418. martong added a comment. - Rebase from master - Enable some disabled tests - Update the isUsed flag - Add a new config `Minimal` to the structural eq check, so lldb tests can pass now - Return with a bool value and use LLVM_NODISCARD in CreateDecl - R

[PATCH] D47632: [ASTImporter] Refactor Decl creation

martong updated this revision to Diff 154421. martong added a comment. Fix indentation Repository: rC Clang https://reviews.llvm.org/D47632 Files: include/clang/AST/ASTImporter.h include/clang/AST/ASTStructuralEquivalence.h include/clang/AST/DeclBase.h lib/AST/ASTImporter.cpp lib/A

[PATCH] D47632: [ASTImporter] Refactor Decl creation

martong updated this revision to Diff 154582. martong marked 6 inline comments as done. martong added a comment. Address review comments Repository: rC Clang https://reviews.llvm.org/D47632 Files: include/clang/AST/ASTImporter.h include/clang/AST/ASTStructuralEquivalence.h include/clan

[PATCH] D47632: [ASTImporter] Refactor Decl creation

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:161 + ToD->IdentifierNamespace = FromD->IdentifierNamespace; + if (FromD->hasAttrs()) +for (const Attr *FromAttr : FromD->getAttrs()) a_sidorin wrote: > Should we move the below

[PATCH] D47946: [ASTImporter] Fix infinite recursion on function import with struct definition in parameters

martong added a comment. Ping. https://reviews.llvm.org/D47946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

martong added a comment. Ping Repository: rC Clang https://reviews.llvm.org/D44100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47946: [ASTImporter] Fix infinite recursion on function import with struct definition in parameters

martong added a comment. @gerazo, Do you have commit rights, or should I help with the commit? https://reviews.llvm.org/D47946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47632: [ASTImporter] Refactor Decl creation

martong added a comment. Hi Aleksei, could you pleas take an other quick look, there were only minor changes since your last comments. Thanks! Repository: rC Clang https://reviews.llvm.org/D47632 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D47632: [ASTImporter] Refactor Decl creation

This revision was automatically updated to reflect the committed changes. Closed by commit rL336896: [ASTImporter] Refactor Decl creation (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47632?vs=154582&id=155134#t

[PATCH] D48722: [ASTImporter] Update isUsed flag at Decl import.

martong abandoned this revision. martong added a comment. Closed in favor of https://reviews.llvm.org/D47632 Repository: rC Clang https://reviews.llvm.org/D48722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

[PATCH] D47946: [ASTImporter] Fix infinite recursion on function import with struct definition in parameters

This revision was automatically updated to reflect the committed changes. Closed by commit rL336898: [ASTImporter] Fix infinite recursion on function import with struct definition… (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://re

[PATCH] D47632: [ASTImporter] Refactor Decl creation

martong added a comment. > I apologize for the delay in reviewing patches. There is no need to apologize. On the contrary, we (me and my colleges at Ericsson) would like to thank you for the effort you had put into reviewing these patches. This period was hard, we provided so many patches latel

[PATCH] D49296: [ASTImporter] Fix import of unnamed structs

martong created this revision. martong added reviewers: a.sidorin, a_sidorin, balazske, gerazo. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. https://reviews.llvm.org/D48773 simplified ASTImporter nicely, but it introduced a new error: Unnamed structs are not imported correctly, if the

[PATCH] D49300: [ASTImporter] Fix poisonous structural equivalence cache

martong created this revision. martong added reviewers: a.sidorin, a_sidorin, r.stahl. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Implementation functions call into the member functions of ASTStructuralEquivalence, thus they can falsely alter the DeclsToCheck state (they add decls).

[PATCH] D49296: [ASTImporter] Fix import of unnamed structs

martong updated this revision to Diff 155660. martong marked 6 inline comments as done. martong added a comment. Address review comments Repository: rC Clang https://reviews.llvm.org/D49296 Files: lib/AST/ASTStructuralEquivalence.cpp unittests/AST/ASTImporterTest.cpp unittests/AST/Stru

[PATCH] D49296: [ASTImporter] Fix import of unnamed structs

martong added inline comments. Comment at: lib/AST/ASTStructuralEquivalence.cpp:913 - if (D1->isAnonymousStructOrUnion() && D2->isAnonymousStructOrUnion()) { + if (!D1->getDeclName() && !D2->getDeclName()) { // If both anonymous structs/unions are in a record context, ma

[PATCH] D49300: [ASTImporter] Fix poisonous structural equivalence cache

martong added a comment. Hi Aleksei, Unfortunately, it seems that it is very to hard synthesize a lit or unit test for this fix. I have found this flaw during the CTU analysis of Bitcoin, where the AST was immense. Because of the lack of tests, now I am trying to persuade you based on some th

[PATCH] D49296: [ASTImporter] Fix import of unnamed structs

martong updated this revision to Diff 155678. martong added a comment. RecordDecl* -> RecordDecl * Repository: rC Clang https://reviews.llvm.org/D49296 Files: lib/AST/ASTStructuralEquivalence.cpp unittests/AST/ASTImporterTest.cpp unittests/AST/StructuralEquivalenceTest.cpp Index: unit

[PATCH] D49296: [ASTImporter] Fix import of unnamed structs

martong updated this revision to Diff 155835. martong marked an inline comment as done. martong added a comment. Remove unneeded assert Repository: rC Clang https://reviews.llvm.org/D49296 Files: lib/AST/ASTStructuralEquivalence.cpp unittests/AST/ASTImporterTest.cpp unittests/AST/Struc

[PATCH] D49296: [ASTImporter] Fix import of unnamed structs

This revision was automatically updated to reflect the committed changes. Closed by commit rL337267: [ASTImporter] Fix import of unnamed structs (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D49296 Files: cfe/trun

[PATCH] D49300: [ASTImporter] Fix poisonous structural equivalence cache

martong added a comment. @a_sidorin Ok, thanks Aleksei for the review. I added the explanation as file comments into `StructuralEquivalence.cpp`. Repository: rC Clang https://reviews.llvm.org/D49300 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D49300: [ASTImporter] Fix poisonous structural equivalence cache

martong updated this revision to Diff 155858. martong added a comment. - Add comment about structural eq and BFS Repository: rC Clang https://reviews.llvm.org/D49300 Files: include/clang/AST/ASTStructuralEquivalence.h lib/AST/ASTImporter.cpp lib/AST/ASTStructuralEquivalence.cpp lib/S

[PATCH] D49300: [ASTImporter] Fix poisonous structural equivalence cache

This revision was automatically updated to reflect the committed changes. Closed by commit rC337275: [ASTImporter] Fix poisonous structural equivalence cache (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D49300?vs=155858&id=155863#toc Repository: rC

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

martong added a comment. Just added a new entry to our roadmap: https://github.com/Ericsson/clang/issues/435 Repository: rC Clang https://reviews.llvm.org/D30691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-b

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

martong added a comment. @NoQ , @dkrupp CallEvent::getRuntimeDefinition is overwritten in - AnyFunctionCall - CXXInstanceCall - CXXMemberCall - CXXDestructorCall - ObjCMethodCall AnyFunctionCall handles the CTU logic. CXXInstanceCall calls into AnyFunctionCall if the function is not virtual. If

[PATCH] D68634: [ASTImporter] Imported FunctionDecls inherit attributes from existing functions

martong created this revision. martong added reviewers: a_sidorin, balazske, dkrupp, vbridgers. Herald added subscribers: cfe-commits, teemperor, gamesh411, Szelethus, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: clang. This way some CTU f

[PATCH] D68634: [ASTImporter] Imported FunctionDecls inherit attributes from existing functions

martong marked an inline comment as done. martong added a comment. In D68634#1700591 , @a_sidorin wrote: > Hello Balasz, > In my opinion, importing and setting the attributes should be handled by the > stuff used in InitializeImportedDecl(). Can we exten

[PATCH] D68634: [ASTImporter] Imported FunctionDecls inherit attributes from existing functions

martong added a comment. In D68634#1701192 , @martong wrote: > In D68634#1700591 , @a_sidorin wrote: > > > Hello Balasz, > > In my opinion, importing and setting the attributes should be handled by > > the stuff u

[PATCH] D69566: [ASTImporter] Add support for BuiltinTemplateDecl

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! Thanks! Comment at: clang/lib/AST/ASTImporter.cpp:4479 + } + assert(ToD && "BuiltinTemplateDecl of unsupported kind!"); + Importer.MapImported(D, ToD); ---

[PATCH] D69360: [NFC] Refactor representation of materialized temporaries

martong added inline comments. Herald added a subscriber: rnkovacs. Comment at: clang/lib/AST/ASTImporter.cpp:7004 + // FIXME: Should ManglingNumber get numbers associated with 'to' context? + auto* To = MaterializeTemporaryDecl::Create(Temporary, ExtendingDecl, +

[PATCH] D79704: [Analyzer] [NFC] Parameter Regions

martong added a comment. Seems like many changes could be simplified or absolutely not needed in this patch if ParamRegion (or with a better name ParamVarRegion) was derived from VarRegion. Is there any difficulties in that derivation? Comment at: clang/include/clang/StaticA

[PATCH] D80286: [Analyzer] Allow creation of stack frame for functions without definition

martong added a comment. > Retrieving the parameter location of functions was disabled because it may > causes crashes due to the fact that functions may have multiple declarations > and without definition it is difficult to ensure that always the same > declration is used. Now parameters are s

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

martong added a comment. Hi Endre, just checked the latest update. Overall looks good to me, but found some nits. Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:260 +/// that produce the AST used for analysis. +StringRef OnDemandParsingDatabase; + -

[PATCH] D79704: [Analyzer] [NFC] Parameter Regions

martong added a comment. Thanks for addressing my comments! Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:309 + /// Get the lvalue for a parameter. + Loc getLValue(const Expr *Call, unsigned Index, +const LocationContext *LC

[PATCH] D80286: [Analyzer] Allow creation of stack frame for functions without definition

martong added inline comments. Comment at: clang/unittests/StaticAnalyzer/ParamRegionTest.cpp:22 +const ParmVarDecl *PVD) { +for (const auto *D2: PVD->redecls()) { + const auto *PVD2 = cast(D2); I am concerned about the re

[PATCH] D77474: [analyzer][MallocChecker] Make NewDeleteLeaks depend on DynamicMemoryModeling rather than NewDelete

martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:194 + const ProgramPointTag *Tag = nullptr) { +// Say this 3 times fast. +State = State ? State : getState(); I like the joke,

[PATCH] D78099: [analyzer][RetainCount] Tie diagnostics to osx.cocoa.RetainCount rather then RetainCountBase, for the most part

martong requested changes to this revision. martong added a comment. This revision now requires changes to proceed. Found a small mistake that should be corrected, then will be good to me. Comment at: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp:

[PATCH] D78126: [analyzer][NFC] Don't allow dependency checkers to emit diagnostics

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2132 + + assert(!isDependency(Registry, bt.getCheckerName()) && + "Some checkers depend on this one

[PATCH] D77474: [analyzer][MallocChecker] Make NewDeleteLeaks depend on DynamicMemoryModeling rather than NewDelete

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Looks good. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2024 + if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) { +C.a

[PATCH] D78099: [analyzer][RetainCount] Tie diagnostics to osx.cocoa.RetainCount rather then RetainCountBase, for the most part

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LG! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78099/new/ https://reviews.llvm.org/D78099 ___ cfe-commits mailing list cfe-commits@

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:435 + /// If the call returns a C++ record type then the region of its return value + // can be retrieved from its construction context. + Optional getReturnValueUnder

[PATCH] D77148: [analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved

martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1013 +addToFunctionSummaryMap( +"__buf_size_arg_constraint_mul", +Summary(ArgTypes{ConstVoidPtrTy, SizeTy, Si

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:747 + : *FilePtrTy) +: None; + balazske wrote: > The `Optional` can be l

[PATCH] D77148: [analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved

martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1013 +addToFunctionSummaryMap( +"__buf_size_arg_constraint_mul", +Summary(ArgTypes{ConstVoidPtrTy, SizeTy, Si

[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

martong updated this revision to Diff 266287. martong marked 4 inline comments as done. martong added a comment. - Add assert - Create the BufferSize arg constraints in a more readable way Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77066/new/ ht

[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:249-250 + // cannot apply the constraint. Actually, other checkers like + // CallAndMessage should catch this situation earlier, because we call a + // func

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

martong added inline comments. Comment at: clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp:24 +public: + void checkPostCall(const CallEvent &Call, CheckerContext &C) const { +if (!Call.getOriginExpr()) I assume this tests this call expre

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

martong added inline comments. Comment at: clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp:28 + +Optional RetVal = Call.getReturnValueUnderConstruction(0); +assert(RetVal); How do we know that `0` is the proper block count? CHANGES

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

martong added a comment. In this version of the patch you are using `handleConstructionContext` to "retrieve" or read an SVal for the construction. However, it seems like that function was designed to store values in the GDM for individual construction context items. To mix a read-only function

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

martong added a comment. In D80366#2059765 , @martong wrote: > In this version of the patch you are using `handleConstructionContext` to > "retrieve" or read an SVal for the construction. However, it seems like that > function was designed to store value

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

martong updated this revision to Diff 266834. martong marked an inline comment as done. martong added a comment. - Rebase to master Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80016/new/ https://reviews.llvm.org/D80016 Files: clang/lib/StaticA

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

martong updated this revision to Diff 266858. martong marked 2 inline comments as done. martong added a comment. - Add `if (FileTy)` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80016/new/ https://reviews.llvm.org/D80016 Files: clang/lib/Static

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:747 + : *FilePtrTy) +: None; + balazske wrote: > martong wrote: > > balazske wrote: > > > The `Optional` can be le

[PATCH] D77658: [analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints

martong updated this revision to Diff 266879. martong marked an inline comment as done. martong added a comment. - Rebase to master Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77658/new/ https://reviews.llvm.org/D77658 Files: clang/lib/StaticA

<    2   3   4   5   6   7   8   9   10   11   >