Re: [clang-tools-extra] r330492 - [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

I see Alex already got it, but in the future, that kind of trivial test fix for a failing test is fine to just land, and it is more important to get the bots healthy. =] On Fri, Apr 20, 2018, 22:14 Yan Zhang via cfe-commits < cfe-commits@lists.llvm.org> wrote: > https://reviews.llvm.org/D45912

[PATCH] D45776: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py

zinovy.nis updated this revision to Diff 143436. zinovy.nis marked 3 inline comments as done. zinovy.nis added a comment. - Applied the changes suggested by Alexander. https://reviews.llvm.org/D45776 Files: docs/clang-tidy/index.rst test/clang-tidy/check_clang_tidy.cpp

[PATCH] D44932: [CodeComplete] Fix completion in the middle of ident in ctor lists.

ilya-biryukov updated this revision to Diff 143435. ilya-biryukov added a comment. - Fixed failing assert on the end of file. Added a test for that. Repository: rC Clang https://reviews.llvm.org/D44932 Files: lib/Lex/Lexer.cpp test/CodeCompletion/ctor-initializer.cpp

[PATCH] D44932: [CodeComplete] Fix completion in the middle of ident in ctor lists.

ilya-biryukov added inline comments. Comment at: lib/Lex/Lexer.cpp:1667-1668 + assert(CurPtr < BufferEnd && "eof at completion point"); + while (isIdentifierBody(*CurPtr)) +++CurPtr; + BufferPtr = CurPtr; aaron.ballman wrote: > You should

[PATCH] D45839: [analyzer] Add support for WebKit "unified sources".

george.karpenkov added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h:144 +// includes the full path. +if (SM.getFilename(IL).contains("UnifiedSource")) { + StringRef Name = SM.getFilename(SL); NoQ

[PATCH] D44557: [analyzer] CStringChecker.cpp - Code refactoring on bug report.

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

[PATCH] D45682: [analyzer] Move `TaintBugVisitor` from `GenericTaintChecker.cpp` to `BugReporterVisitors.h`.

george.karpenkov added a comment. I'm new to the taint visitor, but I am quite confused by your change description. > and many checkers rely on it How can other checkers rely on it if it's private to the taint checker? Also, it's probably to explicitly include BugReporterVisitors.h in the

[PATCH] D45517: [analyzer] WIP: False positive refutation with Z3

rnkovacs added a comment. In https://reviews.llvm.org/D45517#1074057, @NoQ wrote: > > The visitor currently checks states appearing as block edges in the > > exploded graph. The first idea was to filter states based on the shape of > > the exploded graph, by checking the number of successors

[PATCH] D45517: [analyzer] WIP: False positive refutation with Z3

rnkovacs added a comment. In https://reviews.llvm.org/D45517#1074057, @NoQ wrote: > > The visitor currently checks states appearing as block edges in the > > exploded graph. The first idea was to filter states based on the shape of > > the exploded graph, by checking the number of successors

[PATCH] D45920: [analyzer] Move RangeSet related declarations into the RangedConstraintManager header.

george.karpenkov added a comment. Another approach would be to instead teach `RangedConstraintManager` to convert it's constraints to Z3. That would be an unwanted dependency, but the change would be much smaller, and the internals of the solver would not have to be exposed. @NoQ thoughts?

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

rjmccall added a comment. This does not address my review. My review is suggesting that we avoid this issue completely by fixing IRGen to use an external linkage for internal declarations in your emission mode. That would allow you to just emit the module ctors as truly internal in the first

[PATCH] D45807: [libclang] Fix test LibclangReparseTest.FileName when TMPDIR is set to a symbolic link

This revision was automatically updated to reflect the committed changes. Closed by commit rC330507: [libclang] Fix LibclangReparseTest.FileName when TMPDIR is set to a symlink (authored by petr.pavlu, committed by ). Repository: rC Clang https://reviews.llvm.org/D45807 Files:

r330507 - [libclang] Fix LibclangReparseTest.FileName when TMPDIR is set to a symlink

Author: petr.pavlu Date: Sat Apr 21 07:35:18 2018 New Revision: 330507 URL: http://llvm.org/viewvc/llvm-project?rev=330507=rev Log: [libclang] Fix LibclangReparseTest.FileName when TMPDIR is set to a symlink Fix testing of clang_File_tryGetRealPathName() in LibclangReparseTest.FileName when

[clang-tools-extra] r330509 - [clang-apply-replacements] Make clang-apply-replacements installable

Author: zinovy.nis Date: Sat Apr 21 08:01:33 2018 New Revision: 330509 URL: http://llvm.org/viewvc/llvm-project?rev=330509=rev Log: [clang-apply-replacements] Make clang-apply-replacements installable Add a new target for install: install-clang-apply-replacements. So if you need clang-tidy and

[PATCH] D45920: [analyzer] Move RangeSet related declarations into the RangedConstraintManager header.

rnkovacs created this revision. rnkovacs added reviewers: NoQ, george.karpenkov, dcoughlin. Herald added subscribers: dkrupp, a.sidorin, szepet, baloghadamsoftware, xazax.hun, whisperity. I could also move `RangedConstraintManager.h` under `include/` if you agree as it seems slightly out of

[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor

rjmccall added inline comments. Herald added a reviewer: javed.absar. Comment at: lib/CodeGen/CodeGenFunction.h:847 +CurrentCleanupStackDepth = C; + } + You don't need (or want) these accessors, I think; this is just private state of the CGF object, and

[PATCH] D45451: [ItaniumMangle] Undeduced auto type doesn't belong in the substitution table

rjmccall added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:2342 + if (isa(Ty)) +return false; return true; rjmccall wrote: > I agree with your analysis that this shouldn't be a substitution candidate. > However, I think this probably

[PATCH] D45451: [ItaniumMangle] Undeduced auto type doesn't belong in the substitution table

rjmccall added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:2342 + if (isa(Ty)) +return false; return true; rjmccall wrote: > rjmccall wrote: > > I agree with your analysis that this shouldn't be a substitution candidate. > > However, I

[PATCH] D45776: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py

This revision was automatically updated to reflect the committed changes. Closed by commit rL330511: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py (authored by zinovy.nis, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[clang-tools-extra] r330511 - [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py

Author: zinovy.nis Date: Sat Apr 21 08:23:56 2018 New Revision: 330511 URL: http://llvm.org/viewvc/llvm-project?rev=330511=rev Log: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py The patch introduces a new command line option '-check-suffix' for check_clang_tidy.py to allow

[PATCH] D45718: Allow registering custom statically-linked analyzer checkers

george.karpenkov added a comment. Tests would be nice, but on the other hand AFAIK we don't have unit tests for the analyzer. Maybe it's time we add those. Repository: rC Clang https://reviews.llvm.org/D45718 ___ cfe-commits mailing list

[PATCH] D45920: [analyzer] Move RangeSet related declarations into the RangedConstraintManager header.

george.karpenkov added a comment. > I could also move RangedConstraintManager.h under include We probably don't want to do that: currently it can only be imported by files in `Core`, and we probably should keep it that way Comment at:

[PATCH] D45898: [SemaCXX] Mark destructor as referenced

rjmccall added a reviewer: doug.gregor. rjmccall added inline comments. Comment at: lib/Sema/SemaInit.cpp:7074 + if (RD->hasDefinition() && RD->hasNonTrivialDestructor()) +S.MarkFunctionReferenced(E->getExprLoc(), S.LookupDestructor(RD)); +

[PATCH] D45160: [clang-apply-replacements] Make clang-apply-replacements installable

This revision was automatically updated to reflect the committed changes. Closed by commit rL330509: [clang-apply-replacements] Make clang-apply-replacements installable (authored by zinovy.nis, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D45900: CodeGen: Fix invalid bitcast for lifetime.start/end

rjmccall added a comment. These functions must predate the addition of CreateLifetimeStart and CreateLifetimeEnd methods on IRBuilder; we should just be calling those. https://reviews.llvm.org/D45900 ___ cfe-commits mailing list

[PATCH] D45517: [analyzer] WIP: False positive refutation with Z3

rnkovacs updated this revision to Diff 143440. https://reviews.llvm.org/D45517 Files: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h

[PATCH] D45517: [analyzer] WIP: False positive refutation with Z3

george.karpenkov added inline comments. Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:284 + /// \sa shouldPostProcessBugReports + Optional PostProcessBugReports; + The option name should be more self-explanatory, post-processing in general

Re: [clang-tools-extra] r330492 - [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

Sorry I was out today. What is the new error? Can u send it to me? Best regards Yan Zhang > On Apr 21, 2018, at 16:32, Chandler Carruth wrote: > > Ok, this still isn't fixed a day later and over half our build bots are red > because of it. =/ I tried just applying the

Re: [clang-tools-extra] r330492 - [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

Should be able to reproduce it by patching it in and running the tests yourself? Nothing configuration specific here. Still, no hurry. On Sat, Apr 21, 2018 at 6:02 PM Yan Zhang via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Sorry I was out today. What is the new error? Can u send it to

[PATCH] D45921: Add getDeserializationListener to ASTReader

yamaguchi created this revision. yamaguchi added reviewers: v.g.vassilev, rsmith, dblaikie, thakis. We need to know if ASTReader already has a DeserializationListner or not, and this also helps to create a multiplexing deserialization listener if there is one already attached.

[PATCH] D45776: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D45776 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [clang-tools-extra] r330492 - [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

Ok, this still isn't fixed a day later and over half our build bots are red because of it. =/ I tried just applying the patch, and it doesn't seem to fully fix the test as it results in a different error... I've reverted in r330528 for now so that our bots are green. =] Feel free to re-land once

r330530 - Add tests for llvm-bcanalyzer stream types

Author: modocache Date: Sat Apr 21 17:04:35 2018 New Revision: 330530 URL: http://llvm.org/viewvc/llvm-project?rev=330530=rev Log: Add tests for llvm-bcanalyzer stream types Summary: Add tests for the improved stream type detection added to `llvm-bcanalyzer` in https://reviews.llvm.org/D41979.

[PATCH] D41980: Add tests for llvm-bcanalyzer stream types

This revision was automatically updated to reflect the committed changes. Closed by commit rC330530: Add tests for llvm-bcanalyzer stream types (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D41980?vs=143458=143464#toc Repository: rC Clang

[clang-tools-extra] r330528 - Revert r330492: [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

Author: chandlerc Date: Sat Apr 21 16:27:34 2018 New Revision: 330528 URL: http://llvm.org/viewvc/llvm-project?rev=330528=rev Log: Revert r330492: [clang-tidy] add new check to find out objc ivars which do not have prefix '_' This commit has been breaking most bots for a day now. There is a fix

Re: [clang-tools-extra] r330492 - [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

Hmm I have tested it locally before with 64-bit macOS and everything looks good. Best regards Yan Zhang > On Apr 21, 2018, at 18:15, Chandler Carruth wrote: > > Should be able to reproduce it by patching it in and running the tests > yourself? Nothing configuration

[PATCH] D41980: Add tests for llvm-bcanalyzer stream types

modocache updated this revision to Diff 143458. modocache added a comment. Herald added a reviewer: george.karpenkov. Added `-fmodule-format=raw`. Repository: rC Clang https://reviews.llvm.org/D41980 Files: test/Misc/serialized-diags-bcanalyzer.c test/PCH/include-stream-type.cpp