[PATCH] D53787: [Sema] Use proper visibility for global new and delete declarations

2018-10-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In https://reviews.llvm.org/D53787#1279913, @phosek wrote: > In https://reviews.llvm.org/D53787#1279899, @rsmith wrote: > > > Replacing the global new and delete is supposed to be a whole-program > > operation (you only get one global allocator). Otherwise you couldn't

[PATCH] D53654: [clang] Improve ctor initializer completions.

2018-10-30 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:813 +static void AddTemplateParameterChunks(ASTContext , + const PrintingPolicy , We don't seem to need this fwd-decl anymore. Remove it?

[PATCH] D53651: [clangd] Use thread pool for background indexing.

2018-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 171655. kadircet marked 14 inline comments as done. kadircet added a comment. - Address comments && Use ThreadPool Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53651 Files: clangd/Threading.cpp clangd/Threading.h

[PATCH] D53651: [clangd] Use thread pool for background indexing.

2018-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clangd/Threading.cpp:102 +void setThreadPriority(std::thread , ThreadPriority Priority) { +#ifdef HAVE_PTHREAD_H + sched_param priority; sammccall wrote: > don't you also need to actually include it? Turns out it was

[PATCH] D53651: [clangd] Use thread pool for background indexing.

2018-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 171658. kadircet added a comment. - Delete outdated comment. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53651 Files: clangd/Threading.cpp clangd/Threading.h clangd/index/Background.cpp clangd/index/Background.h Index:

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

2018-10-30 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added a comment. In https://reviews.llvm.org/D53738#1279069, @rjmccall wrote: > > The important difference would be that we separate the semantics of > > performing the conversion and the operation. I understand that adding a new > > type could be a bit more involved than baking the

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-30 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Great stuff! Just nits (though a few of them; this is a tricky patch!). Comment at: clangd/index/Background.cpp:122 +inline BackgroundIndex::FileDigest

[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-10-30 Thread Filipe Cabecinhas via Phabricator via cfe-commits
filcab updated this revision to Diff 171660. filcab added a comment. Update with name change, using rjmccall's suggestion Repository: rC Clang https://reviews.llvm.org/D52615 Files: include/clang/Driver/Options.td include/clang/Driver/SanitizerArgs.h

[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-10-30 Thread Filipe Cabecinhas via Phabricator via cfe-commits
filcab updated this revision to Diff 171661. filcab added a comment. Missed the change in some places Repository: rC Clang https://reviews.llvm.org/D52615 Files: docs/ClangCommandLineReference.rst docs/UsersManual.rst include/clang/Driver/Options.td

[PATCH] D53830: [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Hi astrelni, my 2cents. Please upload the patch will full context (i believe `diff -U 9`, but check the man pages if that doesn't work :D) Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:32 + + // a *= b; a /= b; +

[PATCH] D53651: [clangd] Use thread pool for background indexing.

2018-10-30 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Nice! Sorry about the back and forth with threadpool... Comment at: clangd/index/Background.cpp:35 + assert(ThreadPoolSize > 0 && "Thread pool size can't be zero.");

[PATCH] D53809: Fix invalid address space generation for clk_event_t

2018-10-30 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia accepted this revision. Anastasia added a comment. This revision is now accepted and ready to land. LGTM. Thanks! Could you please add prefix `[OpenCL]` in the final commit title. Repository: rC Clang https://reviews.llvm.org/D53809

[PATCH] D43783: [OpenCL] Remove block invoke function from emitted block literal struct

2018-10-30 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment. In https://reviews.llvm.org/D43783#1272001, @AlexeySotkin wrote: > In https://reviews.llvm.org/D43783#1215573, @Anastasia wrote: > > > In https://reviews.llvm.org/D43783#1212485, @yaxunl wrote: > > > > > In https://reviews.llvm.org/D43783#1204353, @svenvh wrote: > > >

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked 9 inline comments as done. aaron.ballman added inline comments. Comment at: Analysis/diagnostics/sarif-diagnostics-taint-test.c:18 +// Test the basics for sanity. +// CHECK: sarifLog['version'].startswith("2.0.0") +// CHECK:

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 171685. aaron.ballman added a comment. Updated based on review feedback -- removed the `Sarif` path generation scheme as it isn't currently needed, and replaced a FIXME with a better comment. Testing remains an open question, however.

[PATCH] D53716: [AST] Only store data for the NRVO candidate in ReturnStmt if needed.

2018-10-30 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL345605: [AST] Only store data for the NRVO candidate in ReturnStmt if needed (authored by brunoricci, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D53716: [AST] Only store data for the NRVO candidate in ReturnStmt if needed.

2018-10-30 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC345605: [AST] Only store data for the NRVO candidate in ReturnStmt if needed (authored by brunoricci, committed by ). Changed prior to commit: https://reviews.llvm.org/D53716?vs=171131=171696#toc

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst:30 + +.. option:: IgnoreLocationless + JonasToth wrote: > I think the another option name would fit better: how about > `IgnoreCommandLineMacros` or the like?

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-30 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/Background.cpp:311 SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs())); - // FIXME: partition the symbols by file rather than TU, to avoid duplication. - IndexedSymbols.update(AbsolutePath, -

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM, @aaron.ballman do you have more comments? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53817 ___ cfe-commits

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

2018-10-30 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! I only wonder if this should be on by default or guarded by a config option. I do not have strong feelings about any of the options though. Repository: rC Clang

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-30 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. Comment at: clangd/index/Background.cpp:311 SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs())); - // FIXME: partition the symbols by file rather than TU, to avoid duplication. -

[PATCH] D53654: [clang] Improve ctor initializer completions.

2018-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 171703. kadircet marked 8 inline comments as done. kadircet added a comment. - Address comments. Repository: rC Clang https://reviews.llvm.org/D53654 Files: lib/Sema/SemaCodeComplete.cpp test/CodeCompletion/ctor-initializer.cpp

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

2018-10-30 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added a comment. So I ran `llvm/tools/clang/tools/extra/clang-tidy/tool/run-clang-tidy.py` The bare version produces `65664` unique findings. The version restricted to `cppcoreguidelines-narrowing-conversions` produces `4323` unique findings. That's about `+7%` of findings. I can post

[PATCH] D53654: [clang] Improve ctor initializer completions.

2018-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:821 + +DeclContext::lookup_result getConstructorResults(ASTContext , + const CXXRecordDecl *Record, ilya-biryukov wrote: > There's a

[PATCH] D53651: [clangd] Use thread pool for background indexing.

2018-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL345590: [clangd] Use thread pool for background indexing. (authored by kadircet, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D53651 Files:

r345594 - [clang] Move two utility functions into SourceManager

2018-10-30 Thread Roman Lebedev via cfe-commits
Author: lebedevri Date: Tue Oct 30 05:37:16 2018 New Revision: 345594 URL: http://llvm.org/viewvc/llvm-project?rev=345594=rev Log: [clang] Move two utility functions into SourceManager Summary: So we can keep that not-so-great logic in one place. Reviewers: rsmith, aaron.ballman Reviewed By:

[PATCH] D53837: [clang] Move two utility functions into SourceManager

2018-10-30 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL345594: [clang] Move two utility functions into SourceManager (authored by lebedevri, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D53837: [clang] Move two utility functions into SourceManager

2018-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D53837#1279891, @rsmith wrote: > Thanks, LG. I bet there's a bunch more places we could change to call these > two. Thank you for the review. Repository: rL LLVM https://reviews.llvm.org/D53837

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-30 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 171684. ioeric marked 11 inline comments as done. ioeric added a comment. - address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53433 Files: clangd/index/Background.cpp clangd/index/Background.h

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 2 inline comments as done. ymandel added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

2018-10-30 Thread Nikolai Kosjar via Phabricator via cfe-commits
nik created this revision. Herald added subscribers: cfe-commits, arphaman. If a header file was processed for the second time, we could end up with a wrong conditional stack and skipped ranges: In the particular example, if the header guard is evaluated the second time and it is decided to skip

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 3 inline comments as done. ymandel added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler

[PATCH] D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496)

2018-10-30 Thread Hans Wennborg via Phabricator via cfe-commits
hans created this revision. hans added reviewers: rnk, thakis, takuto.ikuta. In the course of https://reviews.llvm.org/D51340, @takuto.ikuta discovered that Clang fails to put dllexport/import attributes on static locals during template instantiation. For regular functions, this happens in

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

2018-10-30 Thread Donát Nagy via Phabricator via cfe-commits
donat.nagy updated this revision to Diff 171678. donat.nagy added a comment. Use APFloat to determine precision of floating point type Additionally, fix a typo in the tests. Repository: rC Clang https://reviews.llvm.org/D52730 Files: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") +<<

r345605 - [AST] Only store data for the NRVO candidate in ReturnStmt if needed

2018-10-30 Thread Bruno Ricci via cfe-commits
Author: brunoricci Date: Tue Oct 30 07:40:49 2018 New Revision: 345605 URL: http://llvm.org/viewvc/llvm-project?rev=345605=rev Log: [AST] Only store data for the NRVO candidate in ReturnStmt if needed Only store the NRVO candidate if needed in ReturnStmt. A good chuck of all of the ReturnStmt

r345591 - [CodeGen] Disable the machine verifier on a ThinLTO test

2018-10-30 Thread Francis Visoiu Mistrih via cfe-commits
Author: thegameg Date: Tue Oct 30 05:18:33 2018 New Revision: 345591 URL: http://llvm.org/viewvc/llvm-project?rev=345591=rev Log: [CodeGen] Disable the machine verifier on a ThinLTO test This allows us to turn the machine verifier on by default on X86. Modified:

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

2018-10-30 Thread Donát Nagy via Phabricator via cfe-commits
donat.nagy marked 7 inline comments as done. donat.nagy added a comment. I found a solution for determining the precision value of a floating point type. Is this acceptable? Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:163 + +switch (FloatingSize) { +

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

2018-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 171688. lebedev.ri added a comment. Rebased, NFC. Repository: rC Clang https://reviews.llvm.org/D50250 Files: docs/ReleaseNotes.rst docs/UndefinedBehaviorSanitizer.rst include/clang/Basic/Sanitizers.def lib/CodeGen/CGExprScalar.cpp

[clang-tools-extra] r345590 - [clangd] Use thread pool for background indexing.

2018-10-30 Thread Kadir Cetinkaya via cfe-commits
Author: kadircet Date: Tue Oct 30 05:13:27 2018 New Revision: 345590 URL: http://llvm.org/viewvc/llvm-project?rev=345590=rev Log: [clangd] Use thread pool for background indexing. Reviewers: sammccall, ioeric Reviewed By: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, jfb,

[PATCH] D53651: [clangd] Use thread pool for background indexing.

2018-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 171676. kadircet marked an inline comment as done. kadircet added a comment. - Format the code. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53651 Files: clangd/Threading.cpp clangd/Threading.h clangd/index/Background.cpp

r345597 - [AST] Only store the needed data in WhileStmt

2018-10-30 Thread Bruno Ricci via cfe-commits
Author: brunoricci Date: Tue Oct 30 06:42:41 2018 New Revision: 345597 URL: http://llvm.org/viewvc/llvm-project?rev=345597=rev Log: [AST] Only store the needed data in WhileStmt Don't store the data for the condition variable if not needed. This cuts the size of WhileStmt by up to a pointer. The

[PATCH] D53715: [AST] Only store the needed data in WhileStmt.

2018-10-30 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL345597: [AST] Only store the needed data in WhileStmt (authored by brunoricci, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

2018-10-30 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 171692. gchatelet added a comment. - Add more checks, disable bool implicit casts warning, enable ternary operator warnings. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 Files:

[PATCH] D53651: [clangd] Use thread pool for background indexing.

2018-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 171675. kadircet marked 2 inline comments as done. kadircet added a comment. - Add comment. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53651 Files: clangd/Threading.cpp clangd/Threading.h clangd/index/Background.cpp

[PATCH] D53651: [clangd] Use thread pool for background indexing.

2018-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clangd/Threading.cpp:101 +void setThreadPriority(std::thread , ThreadPriority Priority) { +#ifdef HAVE_PTHREAD_H ilya-biryukov wrote: > Maybe put this helper into `llvm/Support/Threading.h`? We talked with Sam about

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 171683. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Better option name. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53817 Files: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp

[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

2018-10-30 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. I think this roughly looks fine. Krasimir, any thoughts? Comment at: unittests/Format/FormatTest.cpp:11854 + // case above. + { +auto Style = getGoogleStyle(); No need to use a scope here. Feel free to redefine Style. If in fact

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

2018-10-30 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added a comment. So I've updated the code to add more narrowing conversions. It now tests constant expressions against receiving type, do not warn about implicit bool casting, handles ternary operator with constant expressions. I ran it on our code base: results look legit. I'll ping

[PATCH] D53856: [analyzer] Put llvm.Conventions back in alpha

2018-10-30 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. In https://reviews.llvm.org/D53856#1279887, @NoQ wrote: > This might be also covered by @rnkovacs's string buffer escape checker - > either already or eventually, it'll become just yet another string view API > that the checker supports. I thought about that too,

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 171700. ymandel added a comment. Reworded diagnostic message and corresponding documentation. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D53817#1280405, @JonasToth wrote: > LGTM Thank you for the review! > @aaron.ballman do you have more comments? If there are no more comments, i'll land this in a few (~3) hours. Repository: rCTE Clang Tools Extra

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: docs/ReleaseNotes.rst:77 - New :doc:`abseil-duration-factory-float ` check. JonasToth wrote: > that seems to be unrelated sry for noise, this was part of the history diffs! Repository: rCTE Clang Tools Extra

[PATCH] D53648: [clangd] Only log ignored diagnostics with -log=verbose.

2018-10-30 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Sorry for the delay, it was open in my browser for days... Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53648 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D53871: [OpenCL] Allow clk_event_t comparisons

2018-10-30 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh created this revision. svenvh added a reviewer: mantognini. Herald added subscribers: cfe-commits, Anastasia, yaxunl. Also rename `invalid-clk-events-cl2.0.cl` to `clk_event_t.cl` and repurpose it to include both positive and negative clk_event_t tests. Repository: rC Clang

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

2018-10-30 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington updated this revision to Diff 171719. erik.pilkington marked 7 inline comments as done. erik.pilkington added a comment. Address review comments. Thanks! https://reviews.llvm.org/D53522 Files: clang/include/clang/Lex/ModuleMap.h clang/lib/Frontend/DependencyFile.cpp

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

2018-10-30 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added inline comments. Comment at: clang/include/clang/Lex/ModuleMap.h:649-650 + ///This can differ from \c Header's name due to symlinks. void addHeader(Module *Mod, Module::Header Header, - ModuleHeaderRole Role, bool Imported =

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") +<<

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

2018-10-30 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:14 #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "llvm/ADT/APInt.h" +#include "llvm/ADT/APSInt.h" Is `APInt` used anywhere? Repository: rCTE

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:25 // FIXME: Check double -> float truncation. Pay attention to casts: void NarrowingConversionsCheck::registerMatchers(MatchFinder *Finder) { I think

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

2018-10-30 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added a comment. Here are 15 random ones from **llvm** synced at `8f9fb8bab2e9b5b27fe40d700d2abe967b99fbb5`. lib/Target/ARM/AsmParser/ARMAsmParser.cpp:3802:31: warning: narrowing conversion from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

2018-10-30 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 171717. gchatelet marked 4 inline comments as done. gchatelet added a comment. - Remove leftover Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 Files: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: Analysis/diagnostics/sarif-diagnostics-taint-test.c:18 +// Test the basics for sanity. +// CHECK: sarifLog['version'].startswith("2.0.0") +// CHECK: sarifLog['runs'][0]['tool']['fullName'] == "clang static analyzer"

[PATCH] D53697: [ASTImporter][Structural Eq] Check for isBeingDefined

2018-10-30 Thread Gabor Marton via Phabricator via cfe-commits
martong marked 2 inline comments as done. martong added a comment. > I wonder if it is possible to get into situation where non-equivalent decls > are marked equivalent with this patch? If yes, we can create a mapping > between decls being imported and original decls as an alternative solution.

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-30 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL345610: [clang-tidy] cppcoreguidelines-macro-usage: print macro names (authored by lebedevri, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[clang-tools-extra] r345610 - [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-30 Thread Roman Lebedev via cfe-commits
Author: lebedevri Date: Tue Oct 30 08:52:36 2018 New Revision: 345610 URL: http://llvm.org/viewvc/llvm-project?rev=345610=rev Log: [clang-tidy] cppcoreguidelines-macro-usage: print macro names Summary: The macro may not have location (or more generally, the location may not exist), e.g. if it

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. I much prefer this version. We've had the same problem with diffing plist output. One thing we have learned is using a FileCheck was definitely a bad idea, as it leads to

[PATCH] D53648: [clangd] Only log ignored diagnostics with -log=verbose.

2018-10-30 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53648 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 171712. aaron.ballman added a comment. Switched to using diff for testing. https://reviews.llvm.org/D53814 Files: Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif Analysis/diagnostics/sarif-diagnostics-taint-test.c

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D53697: [ASTImporter][Structural Eq] Check for isBeingDefined

2018-10-30 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 171723. martong marked 2 inline comments as done. martong added a comment. - Remove unrelated test Repository: rC Clang https://reviews.llvm.org/D53697 Files: lib/AST/ASTStructuralEquivalence.cpp unittests/AST/ASTImporterTest.cpp Index:

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth requested changes to this revision. JonasToth added a comment. This revision now requires changes to proceed. because this now diverged quite a bit i will revert the lgtm for now and take a longer look at the new patch. The numbers you reported sound good. Comment

r345609 - [OPENMP] Support for mapping of the lambdas in target regions.

2018-10-30 Thread Alexey Bataev via cfe-commits
Author: abataev Date: Tue Oct 30 08:50:12 2018 New Revision: 345609 URL: http://llvm.org/viewvc/llvm-project?rev=345609=rev Log: [OPENMP] Support for mapping of the lambdas in target regions. Added support for mapping of lambdas in the target regions. It scans all the captures by reference in

[PATCH] D53725: [CodeGen] Move `emitConstant` from ScalarExprEmitter to CodeGenFunction. NFC.

2018-10-30 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. In https://reviews.llvm.org/D53725#1278067, @rjmccall wrote: > This should at least be named `emitScalarConstant`. Agree. I've just capitalized 'e' as it looks like the majority of `Emit...` methods are capitalized that way. https://reviews.llvm.org/D53725

r345628 - Add the ability to output static analysis results to SARIF.

2018-10-30 Thread Aaron Ballman via cfe-commits
Author: aaronballman Date: Tue Oct 30 11:55:38 2018 New Revision: 345628 URL: http://llvm.org/viewvc/llvm-project?rev=345628=rev Log: Add the ability to output static analysis results to SARIF. This allows users to specify SARIF (https://github.com/oasis-tcs/sarif-spec) as the output from the

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") +<<

r345649 - Changing the command line parameters sent to diff for this test.

2018-10-30 Thread Aaron Ballman via cfe-commits
Author: aaronballman Date: Tue Oct 30 13:55:18 2018 New Revision: 345649 URL: http://llvm.org/viewvc/llvm-project?rev=345649=rev Log: Changing the command line parameters sent to diff for this test. On some systems, -U 1 was being interpreted as -U -1. Trying -U1 to see if that's the

[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

2018-10-30 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Any ideas for better warning message? Except for the warning text, I see this patch as ready. https://reviews.llvm.org/D52949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D53850: Declares __cpu_model as hidden symbol

2018-10-30 Thread Haibo Huang via Phabricator via cfe-commits
hhb updated this revision to Diff 171789. https://reviews.llvm.org/D53850 Files: lib/CodeGen/CGBuiltin.cpp test/CodeGen/builtin-cpu-is.c test/CodeGen/builtin-cpu-supports.c Index: test/CodeGen/builtin-cpu-supports.c === ---

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Sorry for responding so late, but i thought i get a shot to look at it. Here is the mail from Douglas Yung who was so kind to isolate the breakage and find a reproducer. If you have the chance please take a look at it. Hi Jonas, I have attached a bzipped

r345635 - Speculatively attempt to fix a failing testbot.

2018-10-30 Thread Aaron Ballman via cfe-commits
Author: aaronballman Date: Tue Oct 30 12:49:17 2018 New Revision: 345635 URL: http://llvm.org/viewvc/llvm-project?rev=345635=rev Log: Speculatively attempt to fix a failing testbot. A testbot ( http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/54690/) was failing with a

[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp:29 +AST_MATCHER(NamespaceAliasDecl, isAliasedToStd) { + if (const auto *AN = Node.getAliasedNamespace()) { +// If this aliases to an actual namespace, check if its std.

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") +<<

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

2018-10-30 Thread Davide Italiano via Phabricator via cfe-commits
davide added subscribers: shafik, davide. davide added a comment. This patch broke lldb, at least on MacOS. You can reproduce the issue building lldb, applying your patch and then running $ ./lldb-dotest -p TestCModules I'm going to revert this to unblock folks, but don't hesitate to ping me

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. I think this is getting really close! One question: would it make more sense to name this `readability-const-type-return` or `readability-const-return-type` instead? It's not that the functions return a const *value* that's the issue, it's that the declared

[PATCH] D53860: [SemaCXX] Don't check base's dtor is accessible

2018-10-30 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. http://wg21.link/p0968r0#2227 says: The destructor for each element of class type is potentially invoked (15.4 [class.dtor]) from the context where the aggregate initialization occurs. [Note: This provision ensures that destructors can be called for fully-constructed

[PATCH] D53475: Create ConstantExpr class

2018-10-30 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment. Ping? Repository: rC Clang https://reviews.llvm.org/D53475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r345637 - NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

2018-10-30 Thread Erik Pilkington via cfe-commits
Author: epilk Date: Tue Oct 30 13:31:30 2018 New Revision: 345637 URL: http://llvm.org/viewvc/llvm-project?rev=345637=rev Log: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects) We haven't supported compiling ObjC1 for a long time (and never will again), so there isn't any

[PATCH] D53850: Declares __cpu_model as hidden symbol

2018-10-30 Thread Haibo Huang via Phabricator via cfe-commits
hhb updated this revision to Diff 171748. https://reviews.llvm.org/D53850 Files: lib/CodeGen/CGBuiltin.cpp test/CodeGen/builtin-cpu-is.c test/CodeGen/builtin-cpu-supports.c Index: test/CodeGen/builtin-cpu-supports.c === ---

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

2018-10-30 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added a comment. In https://reviews.llvm.org/D53524#1279288, @tejohnson wrote: > In https://reviews.llvm.org/D53524#1276038, @pcc wrote: > > > In https://reviews.llvm.org/D53524#1274505, @tejohnson wrote: > > > > > In https://reviews.llvm.org/D53524#1271387, @tejohnson wrote: > > > > >

[PATCH] D53725: [CodeGen] Move `emitConstant` from ScalarExprEmitter to CodeGenFunction. NFC.

2018-10-30 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai updated this revision to Diff 171751. vsapsai added a comment. - Rename `EmitConstant` to `EmitScalarConstant`. https://reviews.llvm.org/D53725 Files: clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CGExprScalar.cpp clang/lib/CodeGen/CodeGenFunction.h Index:

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision. aaron.ballman added a comment. In https://reviews.llvm.org/D53814#1280581, @george.karpenkov wrote: > I much prefer this version. > We've had the same problem with diffing plist output. > One thing we have learned is using a FileCheck was definitely a bad

[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace

2018-10-30 Thread Julie Hockett via Phabricator via cfe-commits
juliehockett created this revision. juliehockett added reviewers: aaron.ballman, hokein, ilya-biryukov. juliehockett added a project: clang-tools-extra. Herald added subscribers: xazax.hun, mgorny. Adds a checker to warn against using the std namespace, as Zircon's kernel lib++ policy does not

[PATCH] D53860: [SemaCXX] Don't check base's dtor is accessible

2018-10-30 Thread Taewook Oh via Phabricator via cfe-commits
twoh added a comment. I think the context is Derived here. My understanding of http://wg21.link/p0968r0#2227 (in this patch's context) is that when Derived is aggregate initialized, the destructor for each element of Base is potentially invoked as well. For me it seems that the bug is

r345646 - NFC: Merge KEYOBJC and KEYARC

2018-10-30 Thread Erik Pilkington via cfe-commits
Author: epilk Date: Tue Oct 30 13:51:28 2018 New Revision: 345646 URL: http://llvm.org/viewvc/llvm-project?rev=345646=rev Log: NFC: Merge KEYOBJC and KEYARC We used to only define ARC keywords in -fobjc-arc mode, but now that we define them in ObjC mode, there isn't any reason to keep them

r345630 - Fixing some build bot failures from r345628; NFC intended.

2018-10-30 Thread Aaron Ballman via cfe-commits
Author: aaronballman Date: Tue Oct 30 12:06:58 2018 New Revision: 345630 URL: http://llvm.org/viewvc/llvm-project?rev=345630=rev Log: Fixing some build bot failures from r345628; NFC intended. Modified: cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp Modified:

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 171765. ymandel marked 2 inline comments as done. ymandel added a comment. Add comment to field. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D53547: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

2018-10-30 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE345637: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects) (authored by epilk, committed by ). Changed prior to commit: https://reviews.llvm.org/D53547?vs=170703=171778#toc

[clang-tools-extra] r345637 - NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

2018-10-30 Thread Erik Pilkington via cfe-commits
Author: epilk Date: Tue Oct 30 13:31:30 2018 New Revision: 345637 URL: http://llvm.org/viewvc/llvm-project?rev=345637=rev Log: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects) We haven't supported compiling ObjC1 for a long time (and never will again), so there isn't any

  1   2   >