This revision was automatically updated to reflect the committed changes.
Closed by commit rL343528: [clang-tidy] Build it even without static analyzer
(authored by steveire, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52334?vs=167
steveire added a comment.
I'll take care of it in a few days and commit it myself. Thanks!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/ma
aaron.ballman reopened this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D52334#1250439, @aaron.ballman wrote:
> I've commit as r343415, thank you for the patch!
I've reverted in r343418 as the commit broke some bots.
htt
aaron.ballman closed this revision.
aaron.ballman added a comment.
I've commit as r343415, thank you for the patch!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM! If you need this merged on your behalf, I can do so tomorrow or Monday,
unless someone else gets to it first.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.or
steveire marked an inline comment as done.
steveire added a comment.
> Once I'm happy, I will accept it. If @alexfh has further comments, then they
> can be addressed at that time (pre or post commit).
That's very reasonable, thanks.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.o
steveire updated this revision to Diff 167619.
steveire added a comment.
Doc fix
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
Files:
CMakeLists.txt
clang-tidy/CMakeLists.txt
clang-tidy/ClangTidy.cpp
clang-tidy/plugin/CMakeLists.txt
clang-tidy/plugin/ClangTidyP
steveire updated this revision to Diff 167618.
steveire added a comment.
Format docs
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
Files:
CMakeLists.txt
clang-tidy/CMakeLists.txt
clang-tidy/ClangTidy.cpp
clang-tidy/plugin/CMakeLists.txt
clang-tidy/plugin/ClangT
aaron.ballman added a comment.
In https://reviews.llvm.org/D52334#1250166, @steveire wrote:
> > I think a reasonable place would be docs/clang-tidy/index.rst in the
> > "Getting Involved" area.
>
> Thanks, that's at least actionable, but not very specific. I've added docs
> now. If they don't s
steveire marked 2 inline comments as done.
steveire added a comment.
> I think a reasonable place would be docs/clang-tidy/index.rst in the "Getting
> Involved" area.
That's at least actionable, but not very specific. I've added docs now. If they
don't say what you want them to say, then please
steveire updated this revision to Diff 167615.
steveire added a comment.
Herald added a subscriber: arphaman.
Docs
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
Files:
CMakeLists.txt
clang-tidy/CMakeLists.txt
clang-tidy/ClangTidy.cpp
clang-tidy/plugin/CMakeLists.
aaron.ballman added a comment.
In https://reviews.llvm.org/D52334#1249875, @steveire wrote:
> @aaron.ballman Happy to add a note to the docs, but your request leaves me
> wondering where?
>
> AFAIK, the fact that `clang-tidy` wasn't built at all when using
> `CLANG_ENABLE_STATIC_ANALYZER` is no
steveire marked 2 inline comments as done.
steveire added a comment.
@aaron.ballman Happy to add a note to the docs, but your request leaves me
wondering where?
AFAIK, the fact that `clang-tidy` wasn't built at all when using
`CLANG_ENABLE_STATIC_ANALYZER` is not documented, so I can't just upd
steveire updated this revision to Diff 167584.
steveire added a comment.
Whitespace etc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
Files:
CMakeLists.txt
clang-tidy/CMakeLists.txt
clang-tidy/ClangTidy.cpp
clang-tidy/plugin/CMakeLists.txt
clang-tidy/plugin/Cla
aaron.ballman added a comment.
I think that we need some sort of developer documentation change that explains
1) that this option exists when configuring clang-tidy, and 2) that this
impacts the MPI module as well as the static analyzer.
Comment at: clang-tidy/CMakeLists.txt:
JonasToth added a comment.
In https://reviews.llvm.org/D52334#1247809, @steveire wrote:
> @JonasToth Once again - `clangStaticAnalyzerCheckers` is not `clangAnalysis`.
> Also, that commit changes the `mpi` plugin, which is excluded by this patch.
I pinged because of the MPI thing, i did not lo
steveire added a comment.
@JonasToth Once again - `clangStaticAnalyzerCheckers` is not `clangAnalysis`.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.o
JonasToth added a comment.
There has been a commit that seems related: https://reviews.llvm.org/rL343168
@steveire could you please take a look at how it affects this patch?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
___
c
JonasToth added a comment.
Please wait for explicit approval from @alexfh
Am 26.09.2018 um 13:05 schrieb Stephen Kelly via Phabricator:
> steveire added a comment.
>
> Can I go ahead and merge this now?
>
> Repository:
>
> rCTE Clang Tools Extra
>
> https://reviews.llvm.org/D52334
Reposi
lebedev.ri added a comment.
In https://reviews.llvm.org/D52334#1246356, @steveire wrote:
> Can I go ahead and merge this now?
In https://reviews.llvm.org/D52334#1242813, @lebedev.ri wrote:
> `#ifdef` hell is usually messy and is a source of problems.
> May i ask what is the motivation for t
steveire added a comment.
Can I go ahead and merge this now?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
steveire marked an inline comment as done.
steveire added inline comments.
Comment at: test/CMakeLists.txt:69
-clang-tidy
-)
-endif()
+ clang-tidy
+ )
alexfh wrote:
> There are some clang-tidy tests for the static analyzer integration (at least
> tes
steveire updated this revision to Diff 166825.
steveire added a comment.
Enable tests
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
Files:
CMakeLists.txt
clang-tidy/CMakeLists.txt
clang-tidy/ClangTidy.cpp
clang-tidy/plugin/CMakeLists.txt
clang-tidy/plugin/Clang
alexfh added a comment.
> ! In https://reviews.llvm.org/D52334#1242955, @JonasToth wrote:
> ... to me it makes sense to have clang-tidy without CSA.
Yep, it seems reasonable.
Comment at: test/CMakeLists.txt:69
-clang-tidy
-)
-endif()
+ clang-tidy
+ )
-
steveire updated this revision to Diff 166644.
steveire marked 2 inline comments as done.
steveire added a comment.
Handle tests
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
Files:
CMakeLists.txt
clang-tidy/CMakeLists.txt
clang-tidy/ClangTidy.cpp
clang-tidy/plug
steveire added a comment.
Actually, I had not run the tests. Thanks for the reminder there. I extended
the patch to enable the tests even if CSA is not available.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
___
cfe-commits
JonasToth added a comment.
In https://reviews.llvm.org/D52334#1242881, @steveire wrote:
> Thanks, that at least makes it more obvious where you are getting confused.
>
> See `tools/clang/lib/CMakeLists.txt`. It contains:
>
> add_subdirectory(Analysis)
> # ...
> if(CLANG_ENABLE_STATIC_ANALYZ
steveire added a comment.
Thanks, that at least makes it more obvious where you are getting confused.
See `tools/clang/lib/CMakeLists.txt`. It contains:
add_subdirectory(Analysis)
...
===
if(CLANG_ENABLE_STATIC_ANALYZER)
add_subdirectory(StaticAnalyzer)
endif()
1. That is: Analysis and St
JonasToth added a comment.
>> Some of the clang-tidy stuff relies on Analysis/* from clang as well, e.g.
>> the CFG class. Is this still included in builds with CSA off?
>
> The `Analysis` includes are ifdef'd out in the patch. Have you read the patch?
Yes I did read the patch. `bugprone-use-af
steveire added a comment.
> But that clang-tidy is deactivated totally makes that impossible :)
Yes. That should be clear by reading the patch.
> Some of the clang-tidy stuff relies on Analysis/* from clang as well, e.g.
> the CFG class. Is this still included in builds with CSA off?
The `Anal
JonasToth added a comment.
In https://reviews.llvm.org/D52334#1242811, @steveire wrote:
> @JonasToth Sorry, I don't know what's unclear. I'm so surprised by your
> question that I think maybe I'm missing something. I thought the commit
> message and the patch itself are clear. Am I missing some
lebedev.ri added a comment.
`#ifdef` hell is usually messy and is a source of problems.
May i ask what is the motivation for this change?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52334
___
cfe-commits mailing list
cfe-commits@
steveire added a comment.
@JonasToth Sorry, I don't know what's unclear. I'm so surprised by your
question that I think maybe I'm missing something. I thought the commit message
and the patch itself are clear. Am I missing something?
Currently you can only build clang-tidy if you build the stat
JonasToth added a comment.
Do builds even work properly if the CSA is not build right now?
Comment at: clang-tidy/tool/CMakeLists.txt:32
clangTidyModernizeModule
- clangTidyMPIModule
clangTidyObjCModule
Is the MPI module remove from the list of available
34 matches
Mail list logo
