xazax.hun added a comment.
Generally looks good, I only wonder if this works well with inline namespaces.
Could you test?
Repository:
rC Clang
https://reviews.llvm.org/D48027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.l
xazax.hun accepted this revision.
xazax.hun added a comment.
Sorry for the delay, I think this is OK to commit.
As a possible improvement, I can imagine making it slightly stricter, e.g. you
could only skip QualifiedNames for inline namespaces and the beginning. Maybe
add support for staring wit
xazax.hun added a comment.
Oh, and thanks for working on this, this improvement was long overdue, but
everybody was busy with something else :)
https://reviews.llvm.org/D48027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llv
xazax.hun added inline comments.
Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:35
+ // it is not ture in C++17's template argument deduction.
+ if (!getLangOpts().CPlusPlus || getLangOpts().CPlusPlus17 ||
+ getLangOpts().CPlusPlus2a)
Isn't the chec
xazax.hun added a comment.
The analyzer can only reason about const variables this way, right? Maybe we
should only import the initializers for such variables to have better
performance? What do you think?
Also, I wonder what happens with user-defined classes. Will the analyzer
evaluate the co
xazax.hun added inline comments.
Comment at: include/clang/CrossTU/CrossTranslationUnit.h:117
- /// This function loads a function definition from an external AST
- ///file.
+ /// \brief This function loads a definition from an external AST file.
///
-
xazax.hun added a comment.
Looks good so far, some comments inline.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:58
+
+ auto *TypeDecl = TypedR->getValueType().getTypePtr()->getAsCXXRecordDecl();
+ if (TypeDecl->getName() != "basic_string")
--
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:73
+if (State->contains(TypedR)) {
+ const SymbolRef *StrBufferPtr = State->get(TypedR);
+ const Expr *Origin = Call.getOriginExpr();
xazax.hu
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:65
+ if (Call.isCalled(CStrFn)) {
+SymbolRef RawPtr = Call.getReturnValue().getAsSymbol();
+State = State->set(TypedR, RawPtr);
xazax.hun wrote:
>
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM!
https://reviews.llvm.org/D47135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LG!
Repository:
rC Clang
https://reviews.llvm.org/D47416
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/c
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2382
+// Reset the solver
+RefutationMgr.reset();
+ }
george.karpenkov wrote:
> george.karpenkov wrote:
> > (apologies in advance for nitpicking not on your code
xazax.hun added a comment.
@leanil : Could you add a test case where the warnings are explicitly disabled
in the compilation command and also one where only the aliased warning is
explicitly disabled?
In https://reviews.llvm.org/D38171#878814, @lebedev.ri wrote:
> I feel like the current handl
xazax.hun added a comment.
In https://reviews.llvm.org/D38171#878808, @alexfh wrote:
> András, that's definitely an interesting idea. However, it might be
> interesting to explore a more principled approach:
>
> 1. Make `clang-diagnostic-*` checks first-class citizens and take full
> control of
xazax.hun added a comment.
In https://reviews.llvm.org/D38171#880022, @lebedev.ri wrote:
> A mail [0] posted by @JonasToth is about the similar problem, i think we can
> continue there.
Great! Could you summarize your points there as well? Thanks in advance.
https://reviews.llvm.org/D38171
xazax.hun accepted this revision.
xazax.hun added a comment.
I think there was only one comment but that is already addressed in a dependent
revision. So I think this one is good as is.
https://reviews.llvm.org/D31538
___
cfe-commits mailing list
c
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM!
https://reviews.llvm.org/D38674
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
xazax.hun added a comment.
In https://reviews.llvm.org/D38675#891750, @danielmarjamaki wrote:
> > However, the checker seems to work with a low false positive rate. (<15 on
> > the LLVM, 6 effectively different)
>
> This does not sound like a low false positive rate to me. Could you describe
>
xazax.hun accepted this revision.
xazax.hun added a comment.
LGTM!
https://reviews.llvm.org/D31541
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
xazax.hun added a comment.
Did you link the correct bug in the description? The one you linked was closed
long ago.
https://reviews.llvm.org/D38702
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listin
xazax.hun added a comment.
Let's also summarize what do we have now and what do we want.
> I also think this sounds good, though I'm not quite sure it can be done in
> this patch. Anyway, we should definitely specify what we expect from
> first-class citizen checks. Please correct & extend the
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/IssueHash.cpp:39
+ // primary template.
+ if (const FunctionDecl *InstantiatedFrom =
+ Target->getInstantiatedFromMemberFunction())
martong wrote:
> Could we use here FunctionDecl::ge
xazax.hun added a comment.
In https://reviews.llvm.org/D38728#895669, @NoQ wrote:
> I think it would be great to split them into two different patches, to be
> able to easily see how the change in the hashing affects the tests (and maybe
> revert easily if something goes wrong).
So you would
xazax.hun added a comment.
In https://reviews.llvm.org/D34512#890537, @r.stahl wrote:
> In https://reviews.llvm.org/D34512#877643, @xazax.hun wrote:
>
> > - Unittests now creates temporary files at the correct location
> > - Temporary files are also cleaned up when the process is terminated
> > -
xazax.hun created this revision.
The function map generator tool always creates absolute path. The correct logic
to determine whether a path should be handled as absolute depends on the value
of the CrossTU directory. Added a unit test to avoid regressions.
https://reviews.llvm.org/D38842
Fil
xazax.hun updated this revision to Diff 118788.
xazax.hun added a comment.
- Rebase based on the dependent revision and minor cleanups
https://reviews.llvm.org/D38728
Files:
lib/StaticAnalyzer/Core/IssueHash.cpp
test/Analysis/bug_hash_test.cpp
test/Analysis/edges-new.mm
Index: test/Anal
xazax.hun added a comment.
Thanks for working on this, these additions look very useful to me.
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:124
+ case Stmt::CXXFunctionalCastExprClass:
+return cast(Left)->getTypeAsWritten() ==
You could merge
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM with a nit.
Comment at: unittests/AST/ASTImporterTest.cpp:100
+ // This might catch other cases.
+ Imported->dump(ToNothing);
I would elaborat
xazax.hun updated this revision to Diff 119141.
xazax.hun marked 2 inline comments as done.
xazax.hun added a comment.
Herald added a subscriber: szepet.
- Address review comments.
https://reviews.llvm.org/D37437
Files:
include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
lib/StaticAnaly
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:308
if (StOutBound && !StInBound) {
+if (!Filter.CheckCStringOutOfBounds)
+ return state;
zaks.anna wrote:
> This seems to be related to the change in the test
xazax.hun added inline comments.
Comment at: unittests/AST/ASTImporterTest.cpp:100
+ // This might catch other cases.
+ Imported->dump(ToNothing);
r.stahl wrote:
> xazax.hun wrote:
> > I would elaborate a bit more on the purpose of the code below.
> I will ne
xazax.hun updated this revision to Diff 119458.
xazax.hun marked an inline comment as done.
xazax.hun added a comment.
- Update the scan-build part to work correctly with the accepted version of
libCrossTU
https://reviews.llvm.org/D30691
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptio
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM!
https://reviews.llvm.org/D38922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM!
https://reviews.llvm.org/D39048
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/LoopUnrolling.cpp:100
declRefExpr(to(varDecl(VarNodeMatcher)),
binaryOperator(anyOf(hasOperatorName("="), hasOperatorName("+="),
hasOperatorName("/
xazax.hun added inline comments.
Comment at: lib/AST/ASTImporter.cpp:5500
+
+ TemplateArgumentListInfo ToTAInfo;
+ TemplateArgumentListInfo *ResInfo = nullptr;
According to phabricator this code is very similar to a snippet starting from
line 4524 and some cod
xazax.hun added inline comments.
Comment at: lib/AST/ASTImporter.cpp:5549
+ Expr *BaseE = Importer.Import(E->getBase());
+ if (!BaseE)
+return nullptr;
Does `E->getBase()` guaranteed to return non-null? What happens when this node
was constructed using Emp
xazax.hun added inline comments.
Comment at: lib/AST/ASTImporter.cpp:5476
+
+ for (unsigned ai = 0, ae = NumArgs; ai != ae; ++ai) {
+Expr *FromArg = CE->getArg(ai);
Use uppercase variable names.
Comment at: lib/AST/ASTImporter.cpp:5477
+
xazax.hun added a comment.
One problem to think about when we add all clang-diagnostic as "first or
second" class citizen, `checkes=*` might now enable all the warnings which make
no sense and might be surprising to the users. What do you think?
https://reviews.llvm.org/D38171
_
xazax.hun added a comment.
I checked what happens:
The checker would like to solve the following (I inspect the branch when x == 0
):
`((reg_$1) + 1) * 4 <= 0`
The `getSimplifiedOffsets` function kicks in and simplifies the expression
above to the following:
`(reg_$1) <= -1`
The analyzer a
xazax.hun added a comment.
LGTM!
https://reviews.llvm.org/D37187
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
xazax.hun added a subscriber: NoQ.
xazax.hun added a comment.
I think this change is very useful but it is also important to get these
changes right.
I think one of the main reason you did not get review comments yet is that it
is not easy to verify that these changes are sound.
In general, the
xazax.hun added inline comments.
Comment at: lib/AST/ASTImporter.cpp:5500
+
+ TemplateArgumentListInfo ToTAInfo;
+ TemplateArgumentListInfo *ResInfo = nullptr;
szepet wrote:
> xazax.hun wrote:
> > According to phabricator this code is very similar to a snippet
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:651-652
+ } else if (StoreSite->getLocation().getAs()) {
+os << "Reach the max loop limit.";
+os << " Assigning a conjured symbol";
+if (R->canPrintPretty()) {
--
xazax.hun added inline comments.
Comment at: clang-tidy/misc/MisplacedOperatorInStrlenInAllocCheck.cpp:30
+ Finder->addMatcher(
+ callExpr(callee(functionDecl(hasName("malloc"))),
+ hasArgument(0, anyOf(hasDescendant(BadUse), BadUse)))
Maybe i
xazax.hun added a comment.
I agree that we should not spend too much effort on making warnings from the
compiler and tidy disjunct.
https://reviews.llvm.org/D33537
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bi
xazax.hun abandoned this revision.
xazax.hun added a comment.
Since r316069 this is no longer relevant.
https://reviews.llvm.org/D26105
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:651-652
+ } else if (StoreSite->getLocation().getAs()) {
+os << "Reach the max loop limit.";
+os << " Assigning a conjured symbol";
+if (R->canPrintPretty()) {
--
xazax.hun updated this revision to Diff 120233.
xazax.hun added a comment.
Herald added a subscriber: szepet.
- Modify a test to trigger the assertion fail before the patch is applied.
https://reviews.llvm.org/D37470
Files:
lib/StaticAnalyzer/Core/CallEvent.cpp
test/Analysis/objc-message.m
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM! But wait for @aaron.ballman, @alexfh, or @hokein for the final word.
https://reviews.llvm.org/D38688
___
cfe-commits mailing list
cfe
xazax.hun added a comment.
Yeah, I would rather have the cleanups and do extra work in the visitor. But
lets wait what @NoQ thinks.
Repository:
rC Clang
https://reviews.llvm.org/D49568
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http
xazax.hun requested changes to this revision.
xazax.hun added a comment.
This revision now requires changes to proceed.
Some comments, mostly nits inline.
Comment at: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp:149
+C.addTransition(State);
return;
+ }
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp:207
- if (mayInvalidateBuffer(Call)) {
-if (const PtrSet *PS = State->get(ObjRegion)) {
- // Mark all pointer symbols associated with the deleted object released.
- c
xazax.hun added a comment.
Small comments inline.
Comment at: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp:181
- auto *TypeDecl = ObjRegion->getValueType()->getAsCXXRecordDecl();
- if (TypeDecl->getName() != "basic_string")
-return;
+for (unsigned I = 0, E =
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp:181
- auto *TypeDecl = ObjRegion->getValueType()->getAsCXXRecordDecl();
- if (TypeDecl->getName() != "basic_string")
-return;
+for (unsigned I = 0, E = FD->getNumParams();
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM!
https://reviews.llvm.org/D49656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
xazax.hun accepted this revision.
xazax.hun added a comment.
We could also print something about the ReturnStmt, like source location or
pretty print of its expressions so we can check that we picked the right one in
case we have multiple.
But consider this as an optional task if you have nothin
xazax.hun added a comment.
Overall looks good. Was this tested on large software? I would also be grateful
if you could run the regression tests with templight always being enabled to
see if they uncover any assertions/crashes.
Comment at: include/clang/Driver/CC1Options.td:5
xazax.hun added a comment.
In https://reviews.llvm.org/D5767#999143, @sabel83 wrote:
> 2. What do you mean by regression tests? We have run the clang-test target
> successfully on the patched code (which has the hook). Note that the hook
> this pull request provides is implemented as a ProgramA
xazax.hun added inline comments.
Comment at: lib/FrontendTool/ExecuteCompilerInvocation.cpp:261
+
+} //namespace
Nit: this should be `// namespace clang`
https://reviews.llvm.org/D5767
___
cfe-commits mailing list
xazax.hun added inline comments.
Comment at: lib/Sema/SemaTemplateInstantiate.cpp:646
}
+}
+
Use either `LLVM_FALLTHROUGH;` here or break to avoid compiler warnings.
https://reviews.llvm.org/D5767
___
cf
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: martong.
Looks good to me. Only found a few nits.
Comment at: lib/AST/ASTImporter.cpp:4296
// Create the declaration that is being templa
xazax.hun accepted this revision.
xazax.hun added a comment.
Thanks, this looks good to me! I will try this out soon and commit after that.
https://reviews.llvm.org/D5767
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/
xazax.hun added subscribers: dkrupp, whisperity.
xazax.hun added a comment.
@alexfh did you have any chance to think about this change?
https://reviews.llvm.org/D38171
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi
xazax.hun added a comment.
In https://reviews.llvm.org/D30691#1003514, @george.karpenkov wrote:
> Python code looks OK to me, I have one last request: could we have a small
> documentation how the whole thing is supposed work in integration, preferably
> on an available open-source project any
xazax.hun updated this revision to Diff 134431.
xazax.hun added a comment.
- Rebased to current ToT
- Fixed a problem that the scan-build-py used an old version of the ctu
configuration option
- Added a short guide how to use CTU
https://reviews.llvm.org/D30691
Files:
include/clang/StaticAna
xazax.hun added a comment.
In https://reviews.llvm.org/D30489#769916, @NoQ wrote:
> An idea. I believe the safest way to find the bugs you mentioned would be to
> replace extent-as-a-symbol with extent-as-a-trait.
>
> I.e., currently we construct `extent_$3{SymRegion{conj_$1{char *}}}`, assume
xazax.hun updated this revision to Diff 101695.
xazax.hun edited the summary of this revision.
xazax.hun added a comment.
- Migrate to use USR instead of mangled names. Name mangling related changes
are reverted.
- Better error handling in some cases.
- File paths containing spaces are now handle
xazax.hun marked 2 inline comments as done.
xazax.hun added a comment.
In https://reviews.llvm.org/D30691#731617, @zaks.anna wrote:
> I agree that scan-build or scan-build-py integration is an important issue to
> resolve here. What I envision is that users will just call scan-build and
> pass
xazax.hun added a comment.
While I have no objections, I am wondering whether this is the good way to
spend the performance budget. In particular, there are patches to generate more
symbolic expressions, that could also degrade the performance (but fix some
fixmes along the way).
https://revi
xazax.hun added a comment.
I do not see any test cases for this patch. Could you add them as well?
Are you sure that this representation is ok?
How do you handle the following case?
struct A {
A() {
X x;
x.virtualMethod(); // this virtual call is ok
}
}
int main() {
xazax.hun added a comment.
Note that when you update the differential revision you need to upload the
whole diff. Your diff now only contains the tests but not the code.
In https://reviews.llvm.org/D34275#785189, @wangxindsb wrote:
> > How do you handle the following case?
> >
> > struct A {
xazax.hun added a comment.
In https://reviews.llvm.org/D34275#785294, @wangxindsb wrote:
> > What about:
> >
> > struct A {
> > A() {
> > X x;
> > x.virtualMethod(); // this virtual call is ok
> > foo(); // should warn here
> > }
> > virtual foo();
> > }
> > i
xazax.hun created this revision.
xazax.hun added a project: clang-tools-extra.
Herald added a subscriber: whisperity.
Constexpr variable definitions should be ok in headers.
https://stackoverflow.com/questions/34445336/constexpr-global-constants-in-a-header-file-and-odr
Repository:
rL LLVM
h
xazax.hun created this revision.
Herald added a subscriber: whisperity.
This is how asserts are working right now. This way the semantics of
__builtin_assume will be identical to asserts.
I also moved the tests to another file.
Repository:
rL LLVM
https://reviews.llvm.org/D34502
Files:
l
xazax.hun created this revision.
Right now source locations from different translation units can not be
compared.
This is a problem for an upcoming feature in the Static Analyzer, the cross
translation unit support (https://reviews.llvm.org/D30691).
It would be great to be able to sort the sou
xazax.hun added inline comments.
Comment at: docs/clang-tidy/checks/readability-redundant-keyword.rst:8
+
+`extern` is redundant in function declarations
+
alexfh wrote:
> Could you explain, why you think `extern` is redundant in function
> declarations?
Just to
xazax.hun added a comment.
Note that, it is not easy to add a test case for this patch without the
https://reviews.llvm.org/D30691 being accepted.
Repository:
rL LLVM
https://reviews.llvm.org/D34506
___
cfe-commits mailing list
cfe-commits@lists
xazax.hun created this revision.
Herald added a subscriber: mgorny.
This patch introduces a class that can help to build tools that require cross
translation unit facilities.
This class allows function definitions to be loaded from external AST files
based on an index.
In order to use this funct
xazax.hun updated this revision to Diff 103571.
xazax.hun added a comment.
- Add a tool to dump USRs for function definitions. It can be used to create
index files.
https://reviews.llvm.org/D34512
Files:
include/clang/Tooling/CrossTranslationUnit.h
lib/Tooling/CMakeLists.txt
lib/Tooling/
xazax.hun added a comment.
Some of the CTU related analyzer independent parts are being factored out.
The review is ongoing here: https://reviews.llvm.org/D34512
Another small and independent part is under review here:
https://reviews.llvm.org/D34506
https://reviews.llvm.org/D30691
xazax.hun updated this revision to Diff 103707.
xazax.hun marked an inline comment as done.
xazax.hun edited the summary of this revision.
xazax.hun added a comment.
- Added test for the USR dumping tool.
- Added unit test for the CrossTranslationUnit class
- Added documentation for the API entry
xazax.hun added a comment.
Unfortunately, in order to make the Unit Test pass, I also had to modify the
AST Importer. Are you ok with having that change as part of this patch (Aleksei
might be a suitable person to review that part) or should I make a separate
differential for that?
https://re
xazax.hun added inline comments.
Comment at: include/clang/Tooling/CrossTranslationUnit.h:53-58
+ /// \p CrossTUDir directory, called \p IndexName. In case the declaration is
+ /// found in the index the corresponding AST file will be loaded and the
+ /// definition of the fun
xazax.hun updated this revision to Diff 103710.
xazax.hun added a comment.
- Removed an unrelated change
- Made the unit test more strict
https://reviews.llvm.org/D34512
Files:
include/clang/Tooling/CrossTranslationUnit.h
lib/AST/ASTImporter.cpp
lib/Tooling/CMakeLists.txt
lib/Tooling/Cr
xazax.hun added a comment.
In https://reviews.llvm.org/D34506#787971, @joerg wrote:
> I don't think it is a good idea to make this function non-transitive.
I think this is a good point. However, I am not entirely sure that it is
transitive right now. Check the "Both are in built-in buffers, bu
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
In the future, we might want to model the standard placement new and return a
symbol with the correct SpaceRegion (i.e.: the space region of the argument).
Comment at:
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
The code looks good to me. But the best way to verify these kinds of changes to
see how the results change on large projects after applying the patch.
https://reviews.llvm.org/D41250
xazax.hun added a comment.
In https://reviews.llvm.org/D40560#947514, @NoQ wrote:
> Replaced the live expression hack with a slightly better approach. It doesn't
> update the live variables analysis to take `CFGNewAllocator` into account,
> but at least tests now pass.
>
> In order to keep the
xazax.hun added a comment.
Just to be sure, this is just a refactoring to make this cleaner or you expect
this to have other effects as well, like better performance?
https://reviews.llvm.org/D41151
___
cfe-commits mailing list
cfe-commits@lists.ll
xazax.hun added a comment.
I think, while the analyzer is more suitable for certain kinds of checks that
require deep analysis, it is still useful to have quicker syntactic checks that
can easily identify problems that are the results of typos or incorrectly
modified copy and pasted code. I thi
xazax.hun added reviewers: NoQ, george.karpenkov.
xazax.hun added a comment.
In the tests there are multiple variants of the strcpy function guarded by
macros. Maybe we should run the tests multiple times to test all variants with
different defines?
https://reviews.llvm.org/D41384
_
xazax.hun added a comment.
Maybe `debug.AnalysisOrder` could be used to test the callback order
explicitly. This way the test could also serve as a documentation for the
callback order.
https://reviews.llvm.org/D41406
___
cfe-commits mailing list
xazax.hun added a comment.
Is it possible that this will hide other problems? Wouldn't it be better to run
the tests twice once with this argument and once without it?
Repository:
rC Clang
https://reviews.llvm.org/D41444
___
cfe-commits mailing
xazax.hun added a comment.
In https://reviews.llvm.org/D41444#960848, @a.sidorin wrote:
> In https://reviews.llvm.org/D41444#960841, @xazax.hun wrote:
>
> > Is it possible that this will hide other problems? Wouldn't it be better to
> > run the tests twice once with this argument and once withou
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM!
https://reviews.llvm.org/D41451
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
xazax.hun added a comment.
In https://reviews.llvm.org/D41444#960999, @a.sidorin wrote:
> Also, I still think we should rather not apply template-related patches until
> this testing is implemented. Gabor, Peter, do you agree?
Sure, I am fine with that.
Repository:
rC Clang
https://review
xazax.hun marked 6 inline comments as done.
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:372
+
+ cross_tu::CrossTranslationUnitContext &CTUCtx =
+ Engine->getCrossTranslationUnitContext();
dcoughlin wrote:
> Can this lo
xazax.hun updated this revision to Diff 127525.
xazax.hun marked an inline comment as not done.
xazax.hun added a comment.
- Address some review comments
- Rebased on ToT
https://reviews.llvm.org/D30691
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
include/clang/StaticAnalyzer/
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:407
+ if (!NaiveCTU.hasValue()) {
+NaiveCTU = getBooleanOption("experimental-enable-naive-ctu-analysis",
+/*Default=*/false);
This option
1 - 100 of 1413 matches
Mail list logo
