xazax.hun added a comment.
In https://reviews.llvm.org/D34512#856184, @dcoughlin wrote:
> In either case, the important scenario I think we should support is choosing
> at a call site to a C function the most likely definition of the called
> function, based on number and type of parameters,
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
One nit, otherwise LGTM! Thanks for fixing this!
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:938
llvm::APSInt Multiplicand(rightI.getBitWidth(),
xazax.hun added a comment.
Is this blocked on something?
https://reviews.llvm.org/D32981
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
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
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
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 added a comment.
In https://reviews.llvm.org/D30691#878711, @r.stahl wrote:
> If either of the FullSourceLocs is a MacroID, the call
> SM.getFileEntryForID(XL.getFileID()) will return a null pointer. The null
> pointer will crash the program when attempting to call ->getName() on it.
xazax.hun updated this revision to Diff 116502.
xazax.hun added a comment.
Herald added a subscriber: baloghadamsoftware.
- Fixed an issue with source locations
- Updated to latest trunk
https://reviews.llvm.org/D30691
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
xazax.hun added a comment.
In https://reviews.llvm.org/D30691#878830, @r.stahl wrote:
> For my purposes I replaced the return statement of the
> compareCrossTUSourceLocs function with:
>
> return XL.getFileID() < YL.getFileID();
>
>
> A more correct fix would create only one unique diagnostic
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM!
https://reviews.llvm.org/D37025
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
Generally, it looks good to me. Though it looks like some of the cases covered
in the code do not have corresponding tests (e.g.: the parenexprs).
I think this approach is good in a
xazax.hun added a comment.
In https://reviews.llvm.org/D37023#853941, @NoQ wrote:
> Thank you for the review!
>
> > Though it looks like some of the cases covered in the code do not have
> > corresponding tests (e.g.: the parenexprs).
>
> These are covered by tests in
xazax.hun added a comment.
In https://reviews.llvm.org/D34512#854729, @dcoughlin wrote:
> Except for some drive-by nits, this is a high-level review.
>
> I have some high-level questions about the design of the library:
>
> 1. **How do you intend to handle the case when there are multiple
xazax.hun updated this revision to Diff 113740.
xazax.hun marked 19 inline comments as done.
xazax.hun added a comment.
- Make the API capable of using custom lookup strategies for CTU
- Fix review comments
https://reviews.llvm.org/D34512
Files:
include/clang/Basic/AllDiagnostics.h
xazax.hun added inline comments.
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:82
+size_t Pos = Line.find(" ");
+StringRef LineRef{Line};
+if (Pos > 0 && Pos != std::string::npos) {
danielmarjamaki wrote:
> LineRef can be const
StringRef is an
xazax.hun created this revision.
Herald added subscribers: baloghadamsoftware, whisperity.
Unfortunately, currently, the analyzer core sets the checker name after the
constructor was already run. So if we set the BugType in the constructor, the
output plist will not contain a checker name.
xazax.hun updated this revision to Diff 113088.
xazax.hun marked 2 inline comments as done.
xazax.hun added a comment.
- Updates according to review comments.
https://reviews.llvm.org/D34512
Files:
include/clang/Basic/AllDiagnostics.h
include/clang/Basic/CMakeLists.txt
xazax.hun added a comment.
In https://reviews.llvm.org/D37437#860311, @NoQ wrote:
> Cool. Thanks!
>
> > In the future probably it would be better to alter the signature of the
> > checkers' constructor to set the name in the constructor so it is possible
> > to create the BugType eagerly.
>
>
xazax.hun updated this revision to Diff 113837.
xazax.hun retitled this revision from "[analyzer] Fix SimpleStreamChecker's
output plist not containing the checker name" to "[analyzer] Fix some checker's
output plist not containing the checker name".
xazax.hun edited the summary of this
xazax.hun created this revision.
Herald added subscribers: baloghadamsoftware, whisperity.
The users of CallDescription no longer need to make sure that the called
function is a C function. This makes the API usage easier and it conservatively
will never match ObjC Messages. In the future, this
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
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 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
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 inline comments.
Comment at: lib/StaticAnalyzer/Core/LoopUnrolling.cpp:100
declRefExpr(to(varDecl(VarNodeMatcher)),
binaryOperator(anyOf(hasOperatorName("="), hasOperatorName("+="),
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
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:
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
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
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 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
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,
xazax.hun added a comment.
I agree it might be useful to expose this matcher to everybody.
https://reviews.llvm.org/D38921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM!
https://reviews.llvm.org/D39711
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM!
https://reviews.llvm.org/D39372
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun accepted this revision.
xazax.hun added a comment.
LGTM!
https://reviews.llvm.org/D39803
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
xazax.hun added a reviewer: doug.gregor.
xazax.hun added a comment.
Doug added anonymous structure handling, added as a reviewer in case he wants
to have a look.
https://reviews.llvm.org/D39886
___
cfe-commits mailing list
xazax.hun accepted this revision.
xazax.hun added a comment.
LGTM!
https://reviews.llvm.org/D39800
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
xazax.hun added a comment.
In https://reviews.llvm.org/D38171#927046, @alexfh wrote:
> And, btw, sorry for the long delay. I've been on travelling / on vacation for
> the last few weeks.
No problem. Will you have some time to think about the overall concept?
Implementation and test wise it
xazax.hun accepted this revision.
xazax.hun added a comment.
I found some nit, otherwise LG!
I think you should includ the context in the patches, that makes them reviewing
much easier. See:
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
Found one possible problem. Once fixed, LG!
Comment at: clang-tidy/hicpp/HICPPTidyModule.cpp:33
+#include "../performance/MoveConstArgCheck.h"
+#include
xazax.hun added a subscriber: NoQ.
xazax.hun added inline comments.
Comment at: docs/ReleaseNotes.rst:261
+- Static Analyzer can now properly detect and diagnose unary pre-/post-
+ increment/decrement on an uninitialized values.
+
lebedev.ri wrote:
> JonasToth
xazax.hun added a comment.
@dcoughlin do you have some input on this?
https://reviews.llvm.org/D37437
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
xazax.hun added a comment.
In https://reviews.llvm.org/D39722#933828, @aaron.ballman wrote:
> In https://reviews.llvm.org/D39722#933699, @a.sidorin wrote:
>
> > Hello Takafumi,
> >
> > This is almost OK to me but there is an inline comment we need to resolve
> > in order to avoid Windows
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LG!
https://reviews.llvm.org/D40388
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp:64
+if (const UnaryOperator *U = dyn_cast(StoreE)) {
+ str = "The expression of the unary operator is an uninitialized value. "
+"The computed value will
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp:64
+if (const UnaryOperator *U = dyn_cast(StoreE)) {
+ str = "The expression of the unary operator is an uninitialized value. "
+"The computed value will
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
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
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
xazax.hun added a comment.
I found some nits, but overall I think this is getting close.
Comment at: lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:76
range = CL->getSourceRange();
- }
- else if (const AllocaRegion* AR = dyn_cast(R)) {
+ } else if (const
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
A nit, otherwise LG!
Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:693
SMng);
+ } else if (Optional BE = P.getAs()) {
+S
xazax.hun added a comment.
In https://reviews.llvm.org/D39049#910482, @NoQ wrote:
> // TODO: once the constraint manager is smart enough to handle non
> simplified
> // symbolic expressions remove this function. Note that this can not be
> used in
> // the constraint manager as is, since
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM! But wait for @dcoughlin, @zaks.anna , or @NoQ before commit.
https://reviews.llvm.org/D37187
___
cfe-commits mailing list
xazax.hun requested changes to this revision.
xazax.hun added a comment.
This revision now requires changes to proceed.
In https://reviews.llvm.org/D39049#928620, @danielmarjamaki wrote:
> > So what are the arguments that are passed to getSimplifiedOffset() in
> that case? 0? That does not
xazax.hun requested changes to this revision.
xazax.hun added a comment.
This revision now requires changes to proceed.
The checker documentation should be updated as well.
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2251
+// will generate TypeTraitExpr <...> 'int'
xazax.hun added a comment.
In https://reviews.llvm.org/D39049#926597, @danielmarjamaki wrote:
> > Could you do a similar analysis that I did above to check why does this not
> > work for the multidimensional case? (I.e.: checking what constraints are
> > generated and what the analyzer does
xazax.hun added a comment.
In https://reviews.llvm.org/D38171#909346, @leanil wrote:
> In https://reviews.llvm.org/D38171#901427, @xazax.hun wrote:
>
> > One problem to think about when we add all clang-diagnostic as "first or
> > second" class citizen, `checkes=*` might now enable all the
xazax.hun added inline comments.
Comment at: include/clang/Sema/TemplateInstCallback.h:44
+template
+void initialize(TemplateInstantiationCallbackPtrs& Callbacks_, const Sema
)
+{
Nit: this file should be clang formatted, it does not follow the LLVM style.
xazax.hun added inline comments.
Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:382
+DescFile<"CheckSecuritySyntaxOnly.cpp">;
+ def DeprecatedBufferHandling : Checker<"DeprecatedBufferHandling">,
+HelpText<"Warn on uses of deprecated buffer manipulating
xazax.hun added a comment.
In https://reviews.llvm.org/D35109#926078, @baloghadamsoftware wrote:
> So still the options are to fix it in the checker or fix it in the engine
> (the max/4 or the type extension solution), but leaving it unfixed is not an
> option. I am open to any solution, but
xazax.hun updated this revision to Diff 121477.
xazax.hun added a comment.
- Dominic said he no longer have time to continue with this patch, so I
commandeered this revision
- Skip template instantiations
- Do not attempt fix macro expansions
- Do not attempt fix type aliases and typedef types
-
xazax.hun created this revision.
Herald added subscribers: baloghadamsoftware, whisperity.
Currently relative paths wasn't supperted by run-clang-tidy.py
I added the support, however I did not find any existing tests. Is it ok for
this to land without a test or should I introduce one?
xazax.hun marked 5 inline comments as done.
xazax.hun added a comment.
Two problems are not resolved. One is Aaron prefers to query some infor from
the AST instead of relexing. The second is providing base initializers in the
wrong order.
I think there are other checks that do relexing in some
xazax.hun added a comment.
Also, bugprone might be a better module to put this?
https://reviews.llvm.org/D33722
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
xazax.hun updated this revision to Diff 121698.
xazax.hun added a comment.
- Do not warn for NonCopyable bases
- Remove lexing
https://reviews.llvm.org/D33722
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/CopyConstructorInitCheck.cpp
clang-tidy/misc/CopyConstructorInitCheck.h
xazax.hun added a comment.
In https://reviews.llvm.org/D33722#915440, @aaron.ballman wrote:
> In the other cases, it is not clear that the re-lexed information should be
> carried in the AST. In this case, I think it's pretty clear that the AST
> should carry this information. Further, I don't
xazax.hun added inline comments.
Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:382
+DescFile<"CheckSecuritySyntaxOnly.cpp">;
+ def DeprecatedBufferHandling : Checker<"DeprecatedBufferHandling">,
+HelpText<"Warn on uses of deprecated buffer manipulating
xazax.hun added inline comments.
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:41
+static const llvm::StringSet<> KnownBannedMacroNames = {"EAGAIN",
"EWOULDBLOCK",
+ "SIGCLD", "SIGCHLD"};
Is this
xazax.hun abandoned this revision.
xazax.hun added a comment.
In https://reviews.llvm.org/D39551#914360, @dcoughlin wrote:
> I believe that the intent of `__builtin_debugtrap()` is to provide an
> in-source mechanism so that the developer can trap into the debugger and then
> continue
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
This looks like a great addition! Apart from some nits, LGTM.
Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:487
+
+// result >= constant
+
xazax.hun added a comment.
In https://reviews.llvm.org/D39711#917433, @dcoughlin wrote:
> @xazax.hun Would you be willing to take a look?
Sure, I basically agree with George.
Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:587
if (TrackedType &&
+
xazax.hun updated this revision to Diff 121886.
xazax.hun marked 2 inline comments as done.
xazax.hun added a comment.
- Fix doc comments that I overlooked earlier
https://reviews.llvm.org/D33722
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/CopyConstructorInitCheck.cpp
xazax.hun added a comment.
In https://reviews.llvm.org/D33722#916990, @aaron.ballman wrote:
> In https://reviews.llvm.org/D33722#916540, @xazax.hun wrote:
>
> > Also, bugprone might be a better module to put this?
>
>
> I don't have strong opinions on misc vs bugprone (they're both effectively
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:587
if (TrackedType &&
+ !isa(CE) &&
!ASTCtxt.canAssignObjCInterfaces(DestObjectPtrType, *TrackedType) &&
george.karpenkov wrote:
> dcoughlin
xazax.hun created this revision.
Herald added subscribers: szepet, baloghadamsoftware, whisperity.
Add documentation to the recently added issue hash dumping function.
Repository:
rL LLVM
https://reviews.llvm.org/D39543
Files:
docs/analyzer/DebugChecks.rst
Index:
xazax.hun added inline comments.
Comment at: lib/Analysis/BodyFarm.cpp:415
CallbackRecordDecl, CallArgs);
- } else {
+ } else if (Callback->getType()->isRValueReferenceType()
+ ||
xazax.hun added a comment.
In https://reviews.llvm.org/D38844#911735, @NoQ wrote:
> Hey, i just recalled that we have documentation for `ExprInspection`
> functions in `docs/analyzer/DebugChecks.rst`, you may want to add your
> function there as well :)
Indeed, thanks for pointing this out!
xazax.hun updated this revision to Diff 121290.
xazax.hun marked 2 inline comments as done.
https://reviews.llvm.org/D39543
Files:
docs/analyzer/DebugChecks.rst
Index: docs/analyzer/DebugChecks.rst
===
---
xazax.hun added inline comments.
Comment at: docs/analyzer/DebugChecks.rst:255
+ int x = 1;
+ clang_analyzer_hashDump(x); // Hashed string of x on stderr.
+}
NoQ wrote:
> Unlike `printState` and like all other functions, your function doesn't dump
xazax.hun created this revision.
Herald added subscribers: szepet, baloghadamsoftware, whisperity.
For some reason, `__builtin_debugtrap` is not a sink for the analyzer. I also
added some test cases to demonstrate that `__builtin_trap` and
`__builtin_unreachable` are handled properly. The
xazax.hun added a comment.
In the meantime, I found this discussion:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20121015/153735.html
It looks like the reasoning behind this intrinsic not being noreturn is that
the user might continue the execution in the debugger.
I think the main
xazax.hun updated this revision to Diff 121885.
xazax.hun marked 3 inline comments as done.
xazax.hun added a comment.
- Fix review comments
https://reviews.llvm.org/D33722
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/CopyConstructorInitCheck.cpp
xazax.hun updated this revision to Diff 121700.
xazax.hun added a comment.
- Rebased on ToT
https://reviews.llvm.org/D30691
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM!
Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:633
+// modeled as short-circuit in Clang CFG but this is incorrect.
+StmtNodeBuilder Bldr(Pred, Dst,
xazax.hun added a comment.
In https://reviews.llvm.org/D35109#916617, @NoQ wrote:
> A breakthrough with credit going to Devin: Note that whenever we're not
> dealing with `>`/`<`/`<=`/`>=` (but only with additive ops and `==` and `!=`,
> and we have everything of the same type) we can
xazax.hun added inline comments.
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:59
+ if (!BeginLoc.isMacroID()) {
+Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+ "s");
xazax.hun added inline comments.
Comment at: lib/AST/ASTImporter.cpp:5877
+ DeclarationName Name = Importer.Import(E->getName());
+ if (E->getName().isEmpty() && Name.isEmpty())
+return nullptr;
Is this condition correct?
https://reviews.llvm.org/D38694
xazax.hun added a comment.
I like the idea of adding those assertions but a bit worried about the other
changes. Basically (if I get this right), we are masking the issues here and I
am a bit afraid that they will get forgotten. I think it would be nice to at
least add a FIXME that this path
xazax.hun added a comment.
This does not support memberExprs as condition variables right now.
What happens if you have something like this:
struct X {
void f(int a) {
while(a < i) {
--i;
}
}
int i;
};
I think you could extend the test cases with some classes.
xazax.hun added a comment.
In https://reviews.llvm.org/D40939#948252, @NoQ wrote:
> Like, it's not the situation when we couldn't figure out the type - it would
> have been null in that case. Here we know exactly that the type is void.
Oh, thank you for the clarification!
Repository:
rC
xazax.hun added inline comments.
Comment at: clang-tidy/misc/InfiniteLoopCheck.cpp:121
+
+ Stmt *FunctionBody = nullptr;
+ if (ContainingLambda)
This could be pointer to const, right?
https://reviews.llvm.org/D40937
xazax.hun updated this revision to Diff 125986.
xazax.hun added a comment.
Herald added a subscriber: rnkovacs.
- @gerazo addressed some review comments in scan-build-py.
https://reviews.llvm.org/D30691
Files:
include/clang/AST/ASTContext.h
xazax.hun added inline comments.
Comment at: test/clang-tidy/modernize-replace-uncaught-exception.cpp:41
+
+ res = uncaught_exception();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: 'std::uncaught_exception' is
deprecated, use 'std::uncaught_exceptions' instead
xazax.hun added a comment.
I like the idea of making functions that can be static, static. Could you
update the usages of this function as well?
https://reviews.llvm.org/D39372
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun added a comment.
I like the idea of making `ProgramState::getSVal(const MemRegion *)` symmetric
to `ProgramState::getSVal(Loc)`.
Just some philosophical questions which should probably be addressed in the
future:
But also I wonder, should we maintain all these overloads? I mean, a
xazax.hun created this revision.
Herald added subscribers: szepet, baloghadamsoftware, whisperity.
The analyzer did not return an UndefVal in case a negative value was left
shifted.
I also added altered the UndefResultChecker to emit a clear warning in this
case.
Repository:
rL LLVM
201 - 300 of 1407 matches
Mail list logo
