[PATCH] D47632: [ASTImporter] Refactor Decl creation

a.sidorin added a comment. Hi Gabor! I like the change but there are also some questions. Comment at: lib/AST/ASTImporter.cpp:1659 + AccessSpecDecl *ToD; + std::tie(ToD, AlreadyImported) = CreateDecl( + D, Importer.getToContext(), D->getAccess(), DC, Loc, ColonLoc); ---

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

a.sidorin accepted this revision. a.sidorin added a comment. This revision is now accepted and ready to land. Thank you! When committing, please change the commit message (the current review description is out-of-date) and mention the reformatting done. Repository: rC Clang https://reviews.

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

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

[PATCH] D46944: [analyzer] Use sufficiently large types for index/size calculation.

a.sidorin accepted this revision. a.sidorin added a comment. This revision is now accepted and ready to land. Hi Bevin, The patch looks good to me. But let's wait for maintainers to approve it. @NoQ , could you take a look? https://reviews.llvm.org/D46944 ___

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

a.sidorin accepted this revision. a.sidorin added a comment. This revision is now accepted and ready to land. Looks good, thank you! Repository: rC Clang https://reviews.llvm.org/D48631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D48628: [AST] Structural equivalence of methods

a.sidorin requested changes to this revision. a.sidorin added a comment. This revision now requires changes to proceed. Hi Balázs, I think that the test changes unrelated to C++ method equivalence should be moved into a separate patch. Comment at: lib/AST/ASTStructuralEquival

[PATCH] D47632: [ASTImporter] Refactor Decl creation

a.sidorin added a comment. Hi Gabor, Let's agree on `getImportedOrCreateDecl()` :) I think it is informative enough but is still not too long. Repository: rC Clang https://reviews.llvm.org/D47632 ___ cfe-commits mailing list cfe-commits@lists.l

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

a_sidorin added a comment. Hello Gabor, The change looks reasonable to me. But could you please check if we can have some code unification with structural matching? Comment at: lib/AST/ASTImporter.cpp:2085 } + } else { +if (!IsStructuralMatch

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

a.sidorin added a comment. Hello Gabor, 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? Repo

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

a.sidorin added a comment. Whoops, sorry Balázs. Didn't look at the review author :( Repository: rC Clang https://reviews.llvm.org/D48722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

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

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

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

a_sidorin added a comment. Hello Rafael. This change is good, just some cleanup is needed. Comment at: lib/AST/ASTImporter.cpp:2559 D->isImplicit()); +ToFunction->setRangeEnd(Importer.Import(D->getLocEnd())); } else if (auto *F

[PATCH] D48628: [AST] Structural equivalence of methods

a_sidorin accepted this revision. a_sidorin added a comment. LG with a nit. Thank you! Comment at: unittests/AST/StructuralEquivalenceTest.cpp:489 + +TEST_F(StructuralEquivalenceRecordTest, DISABLED_Methods) { + auto t = makeNamedDecls( Could you add a comment

[PATCH] D47632: [ASTImporter] Refactor Decl creation

a_sidorin added a comment. Hi Gabor, I like the new syntax. There are some comments inline; most of them are just stylish. Comment at: lib/AST/ASTImporter.cpp:94 + void updateAttrAndFlags(const Decl *From, Decl *To) { +// check if some flags or attrs are new in 'From' and

[PATCH] D48628: [AST] Structural equivalence of methods

a_sidorin added inline comments. Comment at: unittests/AST/StructuralEquivalenceTest.cpp:489 + +TEST_F(StructuralEquivalenceRecordTest, DISABLED_Methods) { + auto t = makeNamedDecls( balazske wrote: > a_sidorin wrote: > > Could you add a comment why this test is

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

a_sidorin accepted this revision. a_sidorin added a comment. LGTM too. Thank you! Repository: rC Clang https://reviews.llvm.org/D48941 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

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

a.sidorin accepted this revision. a.sidorin added a comment. Hello Zoltán, Sorry for the delay. I think the patch is fine, just some minor nits inline. Comment at: unittests/AST/ASTImporterTest.cpp:234 +assert(ToAST); +createVirtualFileIfNeeded(ToAST.get(), It->FileNam

[PATCH] D47632: [ASTImporter] Refactor Decl creation

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. Thank you Gabor! I apologize for the delay in reviewing patches. Repository: rC Clang https://reviews.llvm.org/D47632 ___ cfe-commits m

[PATCH] D49235: [ASTImporter] Import described template (if any) of function.

a_sidorin added a comment. Hello Balasz, This looks mostly good but I have a question inline. Comment at: lib/AST/ASTImporter.cpp:2715 +if (auto *ToFT = dyn_cast(Importer.Import(FromFT))) + ToFunction->setDescribedFunctionTemplate(ToFT); +else The

[PATCH] D49245: [ASTImporter] Import implicit methods of existing class.

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. LGTM. Just some stylish nits. To resolve this issue, I used `Sema::DeclareImplicit...` methods. But I like this approach much more because it doesn't allows to forget different kinds of

[PATCH] D49293: [ASTImporter] Add support for import of CXXInheritedCtorInitExpr.

a_sidorin added a comment. Adding new nodes is always welcome. Comment at: lib/AST/ASTImporter.cpp:6741 + + auto *Ctor = dyn_cast(Importer.Import( + E->getConstructor())); cast_or_null? Repository: rC Clang https://reviews.llvm.org/D49293

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

a_sidorin added a comment. Hi Gabor, Could you provide some tests for the issue? Repository: rC Clang https://reviews.llvm.org/D49300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

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

a_sidorin added a comment. Hi Gabor, The change is OK but I have some questions regarding tests. Comment at: unittests/AST/ASTImporterTest.cpp:2495 + FirstDeclMatcher().match(ToTU, fieldDecl(hasName("entry1"))); + auto getRecordDecl = [](FieldDecl *FD) { +auto *ET = c

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

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. Hi Gabor, Thank you for this explanation, it sounds reasonable. Ithink it should be placed into the commit message or into the comment somewhere. Repository: rC Clang https://review

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

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. Thank you Gabor! Repository: rC Clang https://reviews.llvm.org/D49296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lis

[PATCH] D49293: [ASTImporter] Add support for import of CXXInheritedCtorInitExpr.

a_sidorin added inline comments. Comment at: lib/AST/ASTImporter.cpp:6741 + + auto *Ctor = dyn_cast(Importer.Import( + E->getConstructor())); balazske wrote: > a_sidorin wrote: > > cast_or_null? > dyn_cast_or_null: Import may return nullptr, but if not, the

[PATCH] D49235: [ASTImporter] Import described template (if any) of function.

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

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

a_sidorin added a comment. Hello Balasz, In my opinion, importing and setting the attributes should be handled by the stuff used in InitializeImportedDecl(). Can we extend it or reuse the code? It will allow us not to miss the required actions while importing a new function too. Repository:

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

a_sidorin added a comment. Ouch! Sorry, Gabor -_\\ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68634/new/ https://reviews.llvm.org/D68634 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D78000: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)

a_sidorin accepted this revision. a_sidorin added a comment. Hello Shafik! The patch looks good, I only have a few stylish nits. Comment at: clang/unittests/AST/ASTImporterTest.cpp:5964 + new SourceWithCompletedTagList(CompletedTags); + clang::ASTContext &context = FromTU

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

a.sidorin added a comment. Hello Takafumi, This is almost OK to me but there is an inline comment we need to resolve in order to avoid Windows buildbot failures. In addition, as Gabor pointed, when we add a new matcher, we need to update matcher documentation as well. To update the docs, you sh

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

a.sidorin added a comment. Oh, sorry! I was thinking that a matcher is still in ASTMatchers.h (in previous revisions it was there). If matcher is internal (current revision), there is no need to update docs. https://reviews.llvm.org/D39722 ___ cfe

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

a.sidorin added a comment. LGTM, thank you! Comment at: unittests/AST/ASTImporterTest.cpp:566 + typeTraitExpr( + hasType(asString("_Bool")) + )));

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

a.sidorin accepted this revision. a.sidorin added a comment. This revision is now accepted and ready to land. Hello Peter, This looks good to me. But could you please check if test works if we add `-fms-compatibility` and `-fdelayed-template-parsing` to `Args`? Comment at: un

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

a.sidorin accepted this revision. a.sidorin added a comment. Thank you! https://reviews.llvm.org/D39722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38843: [ASTImporter] Support importing CXXPseudoDestructorExpr

a.sidorin accepted this revision. a.sidorin added a comment. This revision is now accepted and ready to land. Looks good, thank you! https://reviews.llvm.org/D38843 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

This revision was automatically updated to reflect the committed changes. Closed by commit rL318998: [ASTImporter] Support TypeTraitExpr (authored by a.sidorin). Changed prior to commit: https://reviews.llvm.org/D39722?vs=124286&id=124309#toc Repository: rL LLVM https://reviews.llvm.org/D39

[PATCH] D38843: [ASTImporter] Support importing CXXPseudoDestructorExpr

This revision was automatically updated to reflect the committed changes. Closed by commit rL319015: [ASTImporter] Support importing CXXPseudoDestructorExpr (authored by a.sidorin). Changed prior to commit: https://reviews.llvm.org/D38843?vs=124173&id=124338#toc Repository: rL LLVM https://

[PATCH] D37806: [analyzer] PthreadLock: Fix return values of XNU lock functions.

a.sidorin accepted this revision. a.sidorin added a comment. This revision is now accepted and ready to land. Looks good to me. Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:271 } -assert(lockFail && lockSucc); -C.addTransition(lockFail); - +//

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

a.sidorin added a comment. Hello Peter. Please set the dependencies for the patch - it cannot be applied clearly and even if I add ImportTemplateArgumentListInfo, tests still fail - looks like FunctionTemplateDecl patch should be applied first. https://reviews.llvm.org/D38845 __

[PATCH] D40715: [analyser] different.LabelInsideSwitch checker implementation

a.sidorin added a comment. Hello Alexey, Thank you for the patch. I have made a preliminary review and will add some other reviewers. You can find my comments inline. Comment at: lib/StaticAnalyzer/Checkers/LabelInsideSwitchChecker.cpp:1 +#include "clang/AST/StmtVisitor.h" +#

[PATCH] D27181: [ASTImporter] Support for importing UsingDecl and UsingShadowDecl

This revision was automatically updated to reflect the committed changes. Closed by commit rL319632: [ASTImporter] Add unit tests for UsingDecl and UsingShadowDecl (authored by a.sidorin). Changed prior to commit: https://reviews.llvm.org/D27181?vs=79488&id=125290#toc Repository: rL LLVM ht

[PATCH] D27181: [ASTImporter] Support for importing UsingDecl and UsingShadowDecl

a.sidorin added a comment. Hello Kareem, While the functionality of this patch is already implemented in my already merged patch, I added your tests into the repo. Thank you! https://reviews.llvm.org/D27181 ___ cfe-commits mailing list cfe-commits

[PATCH] D38694: [ASTImporter] Support importing CXXUnresolvedConstructExpr and UnresolvedLookupExpr

a.sidorin added inline comments. Comment at: lib/AST/ASTImporter.cpp:5877 + DeclarationName Name = Importer.Import(E->getName()); + if (E->getName().isEmpty() && Name.isEmpty()) +return nullptr; xazax.hun wrote: > Is this condition correct? Looks like it sh

[PATCH] D40841: [analyzer] Fix a crash on C++17 AST for non-trivial construction into a trivial brace initializer.

a.sidorin added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:288 +// this-region of the parent stack frame). +if (dyn_cast_or_null(LCtx->getParentMap().getParent(CE))) { + MemRegionManager &MRMgr = getSValBuilder().getRegionManager(); --

[PATCH] D40715: [analyser] different.LabelInsideSwitch checker implementation

a.sidorin added a comment. Hello Alexey, Thank you for the update. The code looks much cleaner now. Comment at: lib/StaticAnalyzer/Checkers/LabelInsideSwitchChecker.cpp:115 + +namespace clang { + namespace ento { alexey.knyshev wrote: > a.sidorin wrote: > > Y

[PATCH] D121176: [ASTStructuralEquivalence] Add support for comparing ObjCCategoryDecl.

a_sidorin added a comment. Hi Volodymyr, Unfortunately, I'm not familiar with Objective-C and its frontend but I have a question. Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:1989 + ++ParamT1, ++ParamT2) { +if (!IsStructurallyEquivalent(Context, *ParamT1, *

[PATCH] D66336: [ASTImporter] Add development internals docs

a_sidorin added inline comments. Comment at: clang/docs/InternalsManual.rst:1470 +*templated* class (the ``CXXRecordDecl``) of a ``ClassTemplateDecl`` with +``ClassTemplateDecl::getTemplatedDec()``. And we can get back a pointer of the +"described" class template from the *templa

[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: cl

[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 htt

[PATCH] D66866: [ASTImporter] At import of records re-order indirect fields too.

a_sidorin accepted this revision. a_sidorin added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66866/new/ https://reviews.llvm.org/D66866 ___ cfe-commits mailing list cfe-commits@list

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

a_sidorin accepted this revision. a_sidorin added a comment. Looks good, thank you for addressing the comments! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66538/new/ https://reviews.llvm.org/D66538

[PATCH] D66336: [ASTImporter] Add development internals docs

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66336/new/ https://reviews.llvm.org/D66336 ___

[PATCH] D37023: [analyzer] Fix bugreporter::getDerefExpr() again.

a.sidorin added a comment. Hi Artem, I have a question after quick look. The original code considered `ParenExpr`s but I cannot find nothing paren-related in the patch. Is case `(x->y).z` handled as expected? https://reviews.llvm.org/D37023 ___ cf

[PATCH] D37023: [analyzer] Fix bugreporter::getDerefExpr() again.

a.sidorin added a comment. Sorry, missed that. https://reviews.llvm.org/D37023 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70819: [ASTImporter] Support functions with placeholder return types ...

a_sidorin added a comment. Hello Gabor, That's an interesting case, thank for fixing this! Comment at: clang/lib/AST/ASTImporter.cpp:3008 +// which is equal to the given DC. +bool isAncestorDeclContextOf(DeclContext *DC, Decl *D) { + DeclContext *DCi = D->getDeclContext(); ---

[PATCH] D70819: [ASTImporter] Support functions with placeholder return types ...

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. LGTM, thanks! Let's wait for Shafik's comments before committing this. Comment at: clang/lib/AST/ASTImporter.cpp:48 #include "clang/Basic/Builtins.h" +#include "clang/

[PATCH] D71020: [ASTImporter] Friend class decl should not be visible in its context

a_sidorin added a comment. Hi Gabor, This patch looks mostly good to me. Thanks! Comment at: clang/lib/AST/ASTImporter.cpp:334 + FromDC->lookup(FromNamed->getDeclName()); + if (std::find(FromLookup.begin(), FromLookup.end(), FromNamed) != + Fr

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

a_sidorin added a comment. Finally, this FIXME is addressed. Thanks! Are you going to add any tests in the following patches? It looks almost good to me, but I have a question inline. Comment at: clang/lib/AST/ASTImporter.cpp:7914 + +#define IMPORT_TYPE_LOC(NAME)

[PATCH] D71020: [ASTImporter] Friend class decl should not be visible in its context

a_sidorin added a comment. Hi Gabor, I have an inline comment for the patch. Otherwise, looks good. Comment at: clang/unittests/AST/ASTImporterTest.cpp:252 + QualType Ty = FD->getFriendType()->getType().getCanonicalType(); + if (isa(Ty)) +Ty = cast(Ty)->getNamedType(); --

[PATCH] D71020: [ASTImporter] Friend class decl should not be visible in its context

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

[PATCH] D75732: [ASTImporter] Added visibility check for variable templates.

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75732/new/ https://reviews.llvm.org/D75732

[PATCH] D75740: [ASTImporter] Corrected import of repeated friend declarations.

a_sidorin added a comment. Hello Balazs, This look almost good to me except some comments inline. Comment at: clang/lib/AST/ASTImporter.cpp:3635 +static std::tuple +getFriendCountAndPosition(FriendDecl *FD) { It's better to turn the tuple into a named struct

[PATCH] D74542: [ASTImporter] Prevent the ASTImporter from creating multiple main FileIDs.

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

[PATCH] D74554: [ASTImporter] Added visibility check for scoped enums.

a_sidorin accepted this revision. a_sidorin added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:4870 + Decl *ToTU = getToTuDecl( + R"( + enum class E; Maybe it's better to use just non-raw string literals here? ``` Decl *ToTU = g

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. The patch looks good. Thanks! I would prefer to see more tests for other TypeLoc cases, but I think they could be added in the following patches. @shafik Shafik, do you have any suggestio

[PATCH] D43967: [ASTImporter] Add test helper Fixture

a.sidorin added a comment. Hi Gabor! This looks much better. Comment at: unittests/AST/ASTImporterTest.cpp:1000 + R"( + template + struct X {}; In C++, names started with underscore are reserved for implementation so it's undesirable to use them

[PATCH] D43967: [ASTImporter] Add test helper Fixture

a.sidorin accepted this revision. a.sidorin added a comment. This revision is now accepted and ready to land. This looks good to me. Thank you! @xazax.hun Gabor, could you please take a look? Repository: rC Clang https://reviews.llvm.org/D43967 _

[PATCH] D45241: [analyzer] Invalidate union regions properly. Don't hesitate to load the default binding later.

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

[PATCH] D44079: [ASTImporter] Allow testing of import sequences; fix import of typedefs for anonymous decls

a.sidorin updated this revision to Diff 141537. a.sidorin added a comment. Rebase to the latest master; add a suggestion from Adam Balogh (thanks!). Repository: rC Clang https://reviews.llvm.org/D44079 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp test/Analysis/Inputs/

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

a.sidorin added a comment. Herald added a subscriber: martong. Hi Peter, The changes needed for rebase are too large - they contain both fixes for compilation failures and fixes for test errors. Could you please upgrade the patch? https://reviews.llvm.org/D38845

[PATCH] D45416: [analyzer] ExprEngine: model GCC inline asm rvalue cast outputs

a.sidorin created this revision. a.sidorin added reviewers: NoQ, dcoughlin, xazax.hun. Herald added subscribers: cfe-commits, rnkovacs, szepet. Herald added a reviewer: george.karpenkov. Despite the fact that cast expressions return rvalues, GCC still handles such outputs as lvalues when compili

[PATCH] D45417: [analyzer] Don't crash on printing ConcreteInt of size >64 bits

a.sidorin created this revision. a.sidorin added reviewers: NoQ, dcoughlin, xazax.hun. Herald added subscribers: cfe-commits, rnkovacs, szepet. Herald added a reviewer: george.karpenkov. Printing of ConcreteInts with size >64 bits resulted in assertion failure in get[Z|S]ExtValue() :`getActiveBit

[PATCH] D45416: [analyzer] ExprEngine: model GCC inline asm rvalue cast outputs

a.sidorin added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:3082 +if (X.isUnknown()) { + // The value being casted to rvalue can be garbage-collected after + // the cast is modeled. Try to recover the memory region being casted --

[PATCH] D45416: [analyzer] ExprEngine: model GCC inline asm rvalue cast outputs

a.sidorin added a comment. > The ultimate solution would probably be to add a fake cloned asm statement to > the CFG (instead of the real asm statement) that would point to the correct > output child-expression(s) that are untouched themselves but simply have > their noop casts removed. I have

[PATCH] D45417: [analyzer] Don't crash on printing ConcreteInt of size >64 bits

a.sidorin updated this revision to Diff 141869. a.sidorin edited the summary of this revision. a.sidorin added a comment. After thinking a bit more, I have removed the FIXME at all: dumping SVal is an extremely rare event so this shouldn't affect the performance. Repository: rC Clang https:/

[PATCH] D45417: [analyzer] Don't crash on printing ConcreteInt of size >64 bits

a.sidorin updated this revision to Diff 141870. a.sidorin added a comment. Renamed the test file. Repository: rC Clang https://reviews.llvm.org/D45417 Files: lib/StaticAnalyzer/Core/SVals.cpp test/Analysis/sval-dump-int128.c Index: test/Analysis/sval-dump-int128.c =

[PATCH] D45416: [analyzer] ExprEngine: model GCC inline asm rvalue cast outputs

a.sidorin added a comment. Maybe we should just remove the condition and leave a FIXME? Repository: rC Clang https://reviews.llvm.org/D45416 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D39886: [ASTImporter] Fix wrong conflict detections for unnamed structures

a.sidorin added a comment. Hello Takafumi, Thank you for this patch. Looks like you're trying to disable lookup for similar structures if the structure is anonymous but there are two things I'm worrying about this solution. 1. Are import conflicts for anonymous structures resolved correctly? 2

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

a.sidorin added a comment. Hello Takafumi, Thank you for this patch! I feel positive about it. You can find my comments inline. Comment at: lib/AST/ASTImporter.cpp:5540 + for(auto FromArg : E->getArgs()) { +TypeSourceInfo *ToTI = Importer.Import(FromArg); +ToArgVec.p

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

a.sidorin added a comment. Hello Peter, Looks mostly good, but there are some minor comments. Comment at: lib/AST/ASTImporter.cpp:5500 + + TemplateArgumentListInfo ToTAInfo; + TemplateArgumentListInfo *ResInfo = nullptr; xazax.hun wrote: > szepet wrote: > >

[PATCH] D37806: [analyzer] PthreadLock: Fix return values of XNU lock functions.

a.sidorin added a comment. Hi Artem, Sorry for long delay for reviews. Unfortunately, hospital is a bad place to do a code review and broken hand is a bad review assistant. This patch looks good to me, I have just a minor comment nit. Comment at: lib/StaticAnalyzer/Checkers/

[PATCH] D37812: [analyzer] PthreadLock: Escape the pointers.

a.sidorin added a comment. Hi Artem. The patch looks mostly good, but I have an inline question. Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:588 +if (Call->isInSystemHeader()) + IsLibraryFunction = true; + } Do we think that only sy

[PATCH] D37809: [analyzer] PthreadLock: Refactor, use PostCall API. NFC.

a.sidorin added a comment. I like this refactoring. I wrote some things that are not clear for me inline. Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:107 + void TryXNULock(const CallEvent &Call, CheckerContext &C) const; + void AcquireLockAux(const CallEven

[PATCH] D38843: [ASTImporter] Support importing CXXPseudoDestructorExpr

a.sidorin added a comment. Hello Peter, Patch is almost OK but there are some minor issues. Comment at: lib/AST/ASTImporter.cpp:5549 + Expr *BaseE = Importer.Import(E->getBase()); + if (!BaseE) +return nullptr; szepet wrote: > xazax.hun wrote: > > Does `

[PATCH] D38694: [ASTImporter] Support importing CXXUnresolvedConstructExpr and UnresolvedLookupExpr

a.sidorin added a comment. Hi Peter, Thank you for the patch. You can find some comments inline. Comment at: lib/AST/ASTImporter.cpp:5476 + + for (unsigned ai = 0, ae = NumArgs; ai != ae; ++ai) { +Expr *FromArg = CE->getArg(ai); xazax.hun wrote: > Use upp

[PATCH] D40073: [Analyzer] Non-determinism: don't sort indirect goto LabelDecl's by addresses

a.sidorin added inline comments. Comment at: test/Analysis/diagnostics/goto-label-determinism.cpp:2 +// RUN: %clang_analyze_cc1 -triple arm-unknown-linux-gnueabi -w -analyzer-checker=debug.ExprInspection %s -verify +// RUN: for i in {1..100}; do %clang_analyze_cc1 -triple arm-u

[PATCH] D39886: [ASTImporter] Fix wrong conflict detections for unnamed structures

a.sidorin accepted this revision. a.sidorin added a comment. This revision is now accepted and ready to land. Hello Takafumi, I think you are correct. So, these are not unnamed structures that need conflict resolution but declarations that instantiate the type. Therefore, we can omit both looku

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

a.sidorin accepted this revision. a.sidorin added a comment. This revision is now accepted and ready to land. LGTM, thank you! https://reviews.llvm.org/D39722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

[PATCH] D40073: [Analyzer] Non-determinism: don't sort indirect goto LabelDecl's by addresses

a.sidorin added a comment. Thank you, Devin! Ilya doesn't have commit access so please commit the patch (or leave it for me, I'm going to commit some patches tomorrow). By the way, is there a common way to write tests for non-determinism for LLVM test suite? https://reviews.llvm.org/D40073

[PATCH] D39886: [ASTImporter] Fix wrong conflict detections for unnamed structures

a.sidorin requested changes to this revision. a.sidorin added a comment. This revision now requires changes to proceed. Hello Takafumi, Could you investigate the patch https://reviews.llvm.org/D30876? It should fix same issue. However, it also handles conflict resolution. I accidentally forgot

[PATCH] D40073: [Analyzer] Non-determinism: don't sort indirect goto LabelDecl's by addresses

This revision was automatically updated to reflect the committed changes. Closed by commit rL318750: [Analyzer] Non-determinism: stable iteration on indirect goto LabelDecl's (authored by a.sidorin). Changed prior to commit: https://reviews.llvm.org/D40073?vs=123558&id=123750#toc Repository:

[PATCH] D39886: [ASTImporter] Fix wrong conflict detections for unnamed structures

a.sidorin added a comment. No need to apologize. Thank you for your work anyway. Even if the bug-fixing part of this patch will be omitted, I'd still like to add your tests into the test suite. https://reviews.llvm.org/D39886 ___ cfe-commits maili

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

a.sidorin added a comment. Hello Aaron, This patch is OK for me but could you please take a look at ASTMatchers changes? https://reviews.llvm.org/D39722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/l

[PATCH] D32751: [ASTImporter] Support new kinds of declarations (mostly Using*)

a.sidorin closed this revision. a.sidorin added a comment. Closed with https://reviews.llvm.org/rL318776 (forgot Differential Revision, sorry). https://reviews.llvm.org/D32751 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

a.sidorin added inline comments. Comment at: lib/AST/ASTImporter.cpp:5622 + SmallVector ToArgVec; + for (auto FromArg : E->getArgs()) { +TypeSourceInfo *ToTI = Importer.Import(FromArg); aaron.ballman wrote: > `const auto *`? By the way, you can replace the

[PATCH] D27773: [analyzer] Add checker modeling gtest APIs.

a.sidorin added a comment. Thank you Devin! Comment at: lib/StaticAnalyzer/Checkers/GTestChecker.cpp:30 +// +// The gtest unit testing API provides macros for assertions that that expand +// into an if statement that calls a series of constructors and returns "

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

a.sidorin added a comment. Thank you for your work, Zoltán! Did you checked if same warnings may be emitted by another checkers? For example, ArrayBoundChecker may warn if index is tainted. I also have some comments below. Comment at: lib/StaticAnalyzer/Checkers/DirtyScalarChe

[PATCH] D26753: ASTImporter: improve support for C++ templates

a.sidorin added a comment. I didn't notice this failure but I'll recheck this. Thank you Kareem! https://reviews.llvm.org/D26753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27180: Testbed and skeleton of a new expression parser

a.sidorin added a comment. Sorry for being late :( Comment at: cfe/trunk/tools/clang-import-test/clang-import-test.cpp:99 +llvm::errs() << LineString << '\n'; +std::string Space(LocColumn, ' '); +llvm::errs() << Space.c_str() << '\n'; I still think

<    1   2   3   4   5   >