a.sidorin added a comment.
Hi Gabor. One of the bugs fixed in this patch is still present in master, other
two are already fixed.
Comment at: lib/AST/ASTImporter.cpp:2749
// Create the imported function.
+ SourceLocation StartLoc = Importer.Import(D->getInnerLocStart());
a.sidorin added a comment.
This should be fixed in r269693.
https://reviews.llvm.org/D6549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
a.sidorin added a comment.
Hi Artem,
Thank you for adding me, I missed this patch. I have few comments below. If you
(and Vlad) can wait for two or three days, I will re-check the place I'm
worrying about and post the results.
Comment at:
a.sidorin added a comment.
I got it. I have hard-coded paths in CHECK-lines so these tests are passed on
my machine but not on other. Thank you Kareem!
https://reviews.llvm.org/D26753
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
a.sidorin added a comment.
Amazing work, Dominic. That's what I wanted to test for long time. But,
personally, I'm not happy with massive changes in tests.
1. I don't think that we need to change run line for tests if they pass with
both managers. These changes are pretty noisy,
2. If Z3 is
a.sidorin added a comment.
Main revisions: https://reviews.llvm.org/rL292776,
https://reviews.llvm.org/rL292778. Sorry for not mentioning them in
Differential Revision.
Repository:
rL LLVM
https://reviews.llvm.org/D26753
___
cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL292779: ASTImporter: quick test fix (authored by a.sidorin).
Changed prior to commit:
https://reviews.llvm.org/D26753?vs=79054=85332#toc
Repository:
rL LLVM
https://reviews.llvm.org/D26753
Files:
This revision was automatically updated to reflect the committed changes.
Closed by commit rL295644: [analyzer] Do not duplicate call graph nodes for
functions that have definition… (authored by a.sidorin).
Changed prior to commit:
https://reviews.llvm.org/D29643?vs=87388=89089#toc
a.sidorin added a comment.
Anna, I will commit. Thank you!
https://reviews.llvm.org/D29643
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
a.sidorin added a comment.
This patch lacks tests. If you add at least minimal test case (I'm not familiar
with ObjC and its front-end, unfortunately), I will no have any concerns. Also
adding Sean.
https://reviews.llvm.org/D6550
___
cfe-commits
a.sidorin added a comment.
Hello Zoltan. Sorry, I'm a bit busy now. Here are my thoughts about the design.
1. I think we should not add new warnings to GenericTaintChecker. Instead, it
is better to move warnings out of it. After that it will become just a plugin
used by other checkers. Such
a.sidorin added a comment.
Sorry for the delay, I have commented inline.
For me, this patch looks like a strict improvement. I think, after some clean
up this can be accepted.
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:442
+
+const RecordDecl *RD =
a.sidorin added a comment.
Hello Kareem.
There are some tests in "class-template" dir that may serve as example. You
don't need a warning for a function that was found: you should just return
found declaration. There is already a warning for a declaration that is defined
in multiple TUs. You
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
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:
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
a.sidorin accepted this revision.
a.sidorin added a comment.
This revision is now accepted and ready to land.
LGTM, thanks!
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2577
if (FName == "postEvent" &&
FD->getQualifiedNameAsString() ==
a.sidorin added a comment.
That's excellent. Thank you for this work, Sean!
Comment at: tools/clang-import-test/CMakeLists.txt:19
+ )
\ No newline at end of file
Please add a newline here.
Comment at:
a.sidorin added a comment.
Thank you Kareem, It looks mostly good, but I'd like to have some functional
tests in ASTMerge for this patch.
Comment at: lib/AST/ASTImporter.cpp:4299
+ if (ImportDeclParts(D, DC, LexicalDC, Name, AlreadyImported, Loc))
+return NULL;
+
a.sidorin added a comment.
Kareem, I have re-checked it and I cannot see the failure. But I'm not going to
commit it until NY holidays end (and, anyway, I will not commit it if somebody
has failing tests). Could you describe your configuration?
https://reviews.llvm.org/D26753
a.sidorin added a comment.
Looks good for me, but I'm not a reviewer. Thank you Devin!
Comment at: test/Driver/analyzer-target-enabled-checkers.cpp:7
// CHECK-DARWIN: "-analyzer-checker=core"
+// CHECK-DARWIN-SAME: "-analyzer-checker=apiModeling"
// CHECK-DARWIN-SAME:
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
a.sidorin added a comment.
Looks good now, thanks!
Repository:
rL LLVM
https://reviews.llvm.org/D27180
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
a.sidorin added a comment.
Looks useful and mostly good. A small advice is in inline comments.
Comment at: lib/StaticAnalyzer/Core/BugReporter.cpp:3294
+ // Find the node's current statement in the CFG.
+ // FIXME: CFG lookup should be made easier.
+ const CFG =
a.sidorin added inline comments.
Comment at: lib/StaticAnalyzer/Core/BugReporter.cpp:3363
+// we're post-dominated by.
+// FIXME: This is far from enough to handle the incomplete analysis case.
+// We may be post-dominated in subsequent blocks, or even
a.sidorin added a comment.
Hello Daniel,
Your patch looks mostly good to me. I have only minor naming comments.
Comment at: lib/StaticAnalyzer/Core/CheckerHelpers.cpp:99
+bool clang::ento::isExprResultConformsComparisonRule(CheckerContext ,
+
a.sidorin added inline comments.
Comment at: lib/StaticAnalyzer/Core/CheckerHelpers.cpp:99
+bool clang::ento::isExprResultConformsComparisonRule(CheckerContext ,
+ BinaryOperatorKind BOK,
+
a.sidorin accepted this revision.
a.sidorin added a comment.
Looks excellent now. Thank you Sean!
Repository:
rL LLVM
https://reviews.llvm.org/D30435
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
a.sidorin added inline comments.
Comment at: lib/AST/ASTImporter.cpp:4305
+ DeclarationNameInfo NameInfo(Name,
Importer.Import(D->getNameInfo().getLoc()));
+ ImportDeclarationNameLoc(D->getNameInfo(), NameInfo);
+
spyffe wrote:
> I've seen this pattern
a.sidorin added a comment.
Thank you Devin. Should we submit this to 4.0? I guess there are not many users
of both CSA and OpenMP but PR you pointed is already the second report about
this issue I see.
Repository:
rL LLVM
https://reviews.llvm.org/D30565
This revision was automatically updated to reflect the committed changes.
Closed by commit rL296884: [Analyzer] Terminate analysis on OpenMP code instead
of assertion crash (authored by a.sidorin).
Changed prior to commit:
https://reviews.llvm.org/D30565?vs=90441=90499#toc
Repository:
rL
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/D30831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
a.sidorin added a comment.
I have no any objection on this change.
Repository:
rL LLVM
https://reviews.llvm.org/D30798
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
a.sidorin added a comment.
Hello Zoltan,
Thank you for the patch. There is an inline comment.
Comment at: lib/AST/ASTImporter.cpp:5221
IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(I));
-if (!ToII)
- return nullptr;
+// ToII is nullptr when no
a.sidorin added a comment.
Hello Sean,
It is good to have the ability of recursive lookup. But for me (and I'm sorry),
the code becomes very hard to understand: I need to track if the Decl/DC is in
the source AST or in the AST being imported, if it was imported or if was in
the AST before
a.sidorin added a comment.
Looks like a right fix.
Comment at: lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp:93
// Warn when there is widening cast.
unsigned ToWidth = Ctx.getTypeInfo(ToPointeeTy).Width;
NoQ wrote:
> I think we should move
a.sidorin created this revision.
ExprEngine assumes that OpenMP statements should never appear in CFG. However,
current CFG doesn't know anything about OpenMP and passes such statements as
CFG nodes causing "UNREACHABLE executed!" crashes. Since I have no ideas on
OpenMP implementation in
a.sidorin added a comment.
`git blame` shows that OMP* statements were added to the switch block by
different authors while OpenMP support in clang was implemented. Looks like
they were put to "Should not appear" section instead of "Unsupported" by
mistake.
https://reviews.llvm.org/D30565
a.sidorin added a comment.
Hi Artem! Thank you for this patch. It looks very promising, but I have some
questions and remarks.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:187
const Expr *Result) {
- SVal V =
a.sidorin accepted this revision.
a.sidorin added a comment.
This revision is now accepted and ready to land.
Looks good.
https://reviews.llvm.org/D35674
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
a.sidorin added a comment.
Oh, I see now. I was wrongly thinking that these patches do the same thing and
we're duplicating the work. Thank you very much for your explanation, Devin.
Repository:
rL LLVM
https://reviews.llvm.org/D16403
___
a.sidorin added inline comments.
Comment at: lib/AST/ASTContext.cpp:1481
+ assert(!FD->hasBody() && "FD has a definition in current translation unit!");
+ if (!FD->getType()->getAs())
+return nullptr; // Cannot even mangle that.
xazax.hun wrote:
>
a.sidorin accepted this revision.
a.sidorin added a comment.
Refactoring changes are always appreciated. I only have a minor naming nit.
Comment at: include/clang/AST/ASTStructuralEquivalence.h:33
+ /// AST contexts for which we are checking structural equivalence.
+
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
___
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
a.sidorin updated this revision to Diff 98441.
a.sidorin added a reviewer: karkhaz.
a.sidorin added a comment.
Address review comments; add the context.
https://reviews.llvm.org/D32751
Files:
lib/AST/ASTImporter.cpp
test/ASTMerge/namespace/Inputs/namespace1.cpp
a.sidorin accepted this revision.
a.sidorin added a comment.
This revision is now accepted and ready to land.
Looks good!
Comment at: lib/AST/ASTImporter.cpp:1757
D2->setAnonymousStructOrUnion(true);
+if (PrevDecl) {
+ D2->setPreviousDecl(PrevDecl);
a.sidorin accepted this revision.
a.sidorin added a comment.
Ok, I hope this will work. Anyway, we can always revert this patch in case of
any problems.
https://reviews.llvm.org/D34277
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
a.sidorin added a comment.
Hi Matthias,
I have posted a comment about review duplication (more than a year ago!) in
your review but you haven't answered. So, all this time we were thinking that
you do separate non-related work.
@dcoughlin As a reviewer of both patches - could you tell us what's
a.sidorin updated this revision to Diff 99668.
a.sidorin added a comment.
Replaced cast<> with cast_or_null<>.
https://reviews.llvm.org/D32751
Files:
lib/AST/ASTImporter.cpp
test/ASTMerge/namespace/Inputs/namespace1.cpp
test/ASTMerge/namespace/Inputs/namespace2.cpp
a.sidorin updated this revision to Diff 99669.
a.sidorin added a comment.
Removed accidentally duplicated comment.
https://reviews.llvm.org/D32751
Files:
lib/AST/ASTImporter.cpp
test/ASTMerge/namespace/Inputs/namespace1.cpp
test/ASTMerge/namespace/Inputs/namespace2.cpp
a.sidorin added inline comments.
Comment at: lib/AST/ASTImporter.cpp:1311
+ EmptyDecl *ToD = EmptyDecl::Create(Importer.getToContext(), DC, Loc);
+ ToD->setLexicalDeclContext(LexicalDC);
+ LexicalDC->addDeclInternal(ToD);
xazax.hun wrote:
> Don't we need an
a.sidorin added a comment.
Hello Peter,
`if (!ToDecl) return nullptr;` is used if original node is always non-null.
`if(!ToDecl && FromDecl) return nullptr;` is used if original node can be null.
If the imported node is null, the result of import is null as well so such
import is OK.
a.sidorin added a comment.
Hi Artem,
Could you tell what code bases did you use to collect your statistics? I'll try
to check the patch on our code bases. I think we should be careful about
default settings. Maybe we should introduce another UMK_* for deeper analysis
instead?
a.sidorin added a comment.
Will anybody object if I commit this change without a test? This bug seems to
be pretty obvious but, unfortunately, I'm not familiar with Objective C.
https://reviews.llvm.org/D6550
___
cfe-commits mailing list
a.sidorin added a comment.
Thank you Sean, I'll try.
https://reviews.llvm.org/D6550
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
a.sidorin accepted this revision.
a.sidorin added a comment.
Hello Daniel & Gabor,
Thank you very much for your work. This patch looks good to me but I think such
a change should also be approved by maintainers.
https://reviews.llvm.org/D30691
___
a.sidorin updated this revision to Diff 102679.
a.sidorin marked an inline comment as done.
a.sidorin added a comment.
Herald added a subscriber: kristof.beyls.
Add a FIXME.
https://reviews.llvm.org/D32751
Files:
lib/AST/ASTImporter.cpp
test/ASTMerge/namespace/Inputs/namespace1.cpp
a.sidorin added inline comments.
Comment at: lib/AST/ASTImporter.cpp:2993
+ return nullptr;
+ }
+
szepet wrote:
> nit: As I see these cases typically handled in the way:
>
> ```
> FrPattern = .;
> ToPattern = ..;
> if(FrPattern && !ToPattern)
> ```
> Just
a.sidorin added a comment.
Hi Artem. It is a good point but I think we should have a literal for this
instead.
https://reviews.llvm.org/D32702
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
a.sidorin accepted this revision.
a.sidorin added a comment.
Thank you!
https://reviews.llvm.org/D32702
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
a.sidorin created this revision.
Herald added subscribers: rengolin, aemerson.
Support new AST nodes:
- UnresolvedUsingType
- EmptyDecl
- NamespaceAliasDecl
- UsingDecl
- UsingShadowDecl
- UsingDirectiveDecl
- UnresolvedUsingValueDecl
- UnresolvedUsingTypenameDecl
Refactor error handling in
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?
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);
-
+
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:
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
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
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=124309#toc
Repository:
rL LLVM
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
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
___
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
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=124338#toc
Repository:
rL LLVM
a.sidorin updated this revision to Diff 127283.
a.sidorin added a comment.
Fixed sanity check.
Repository:
rC Clang
https://reviews.llvm.org/D38694
Files:
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
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
a.sidorin added a comment.
Hi Artem. This patch looks OK, just stylish issues.
Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:112
+ // It means that we cannot handle construction into null or garbage pointers.
+ // Such cosntructors need to be handled by checkers to
a.sidorin added a comment.
This thing is very similar to https://reviews.llvm.org/D19979. Do we really
need to create a separate LoopContext or we can reuse ScopeContext instead?
https://reviews.llvm.org/D41151
___
cfe-commits mailing list
a.sidorin added a comment.
This patch still depends on scope implementation in CFG. There is no final
implementation; after initial implementation is done, I'll update the patch.
https://reviews.llvm.org/D19979
___
cfe-commits mailing list
a.sidorin updated this revision to Diff 127375.
a.sidorin added reviewers: xazax.hun, szepet.
a.sidorin added a comment.
Herald added a subscriber: rnkovacs.
Removed already fixed stuff, added a test for remaining.
Repository:
rC Clang
https://reviews.llvm.org/D6550
Files:
a.sidorin added a comment.
Hi Alexey,
This commit strongly needs testing on some real code. I cannot predict the TP
rate of this checker now.
Regarding implementation, you can find some remarks inline.
Comment at: lib/StaticAnalyzer/Checkers/CallArgsOrderChecker.cpp:31
+
a.sidorin accepted this revision.
a.sidorin added a comment.
This revision is now accepted and ready to land.
Looks good, just a minor nit.
Comment at: test/Analysis/NewDelete-custom.cpp:7
-#if !LEAKS
+#if !(LEAKS && !ALLOCATOR_INLINING)
// expected-no-diagnostics
a.sidorin accepted this revision.
a.sidorin added a comment.
This revision is now accepted and ready to land.
Looks good!
Repository:
rC Clang
https://reviews.llvm.org/D41409
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
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:
> >
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);
+
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:
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
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
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
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 , CheckerContext ) const;
+ void AcquireLockAux(const CallEvent ,
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
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
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
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=123750#toc
Repository:
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
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
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
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
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
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 = getSValBuilder().getRegionManager();
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
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:
> >
1 - 100 of 462 matches
Mail list logo
