[PATCH] D43231: [clang-format] Refactor ObjC tests

2018-02-13 Thread Jacek Olesiak via Phabricator via cfe-commits
jolesiak created this revision. Herald added subscribers: cfe-commits, klimek. Simplifies ObjC style tests. Repository: rC Clang https://reviews.llvm.org/D43231 Files: unittests/Format/FormatTestObjC.cpp Index: unittests/Format/FormatTestObjC.cpp

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > Do you have a reference to style guides recommending any of this? Unfortunately not... I think this is a really advanced topic, and often not recommended way to format code at all (e.g. "if you need a block, you may as well use a function"). I can find the current

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added reviewers: hokein, ioeric, sammccall. Herald added subscribers: jkorous-apple, klimek. Some of the existing structs had primimtive fields that were not explicitly initialized on construction. After this commit every struct consistently sets

[PATCH] D43124: [clang-format] Improve ObjC headers detection

2018-02-13 Thread Jacek Olesiak via Phabricator via cfe-commits
jolesiak planned changes to this revision. jolesiak added a comment. I'll add a few test cases after submitting https://reviews.llvm.org/D43231. Repository: rC Clang https://reviews.llvm.org/D43124 ___ cfe-commits mailing list

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz marked 4 inline comments as done. Typz added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:1130 +if (FormatTok->Tok.is(tok::l_brace)) { + if (Style.BraceWrapping.AfterObjCDeclaration) +addUnwrappedLine();

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek, benhamilton. ObjC defines `@autoreleasepool` and `@synchronized` control blocks. These used to be formatted according to the `AfterObjCDeclaration` brace- wrapping flag, which is not very consistent. This patch changes

[clang-tools-extra] r324993 - [clangd] Remove an already-done FIXME, NFC

2018-02-13 Thread Haojian Wu via cfe-commits
Author: hokein Date: Tue Feb 13 01:56:45 2018 New Revision: 324993 URL: http://llvm.org/viewvc/llvm-project?rev=324993=rev Log: [clangd] Remove an already-done FIXME, NFC Modified: clang-tools-extra/trunk/clangd/index/Index.h Modified: clang-tools-extra/trunk/clangd/index/Index.h URL:

r324995 - [clang-format] Support text proto extensions

2018-02-13 Thread Krasimir Georgiev via cfe-commits
Author: krasimir Date: Tue Feb 13 02:20:39 2018 New Revision: 324995 URL: http://llvm.org/viewvc/llvm-project?rev=324995=rev Log: [clang-format] Support text proto extensions Summary: This adds support for text proto extensions, like: ``` msg { [type.type/ext] { key: value } } ```

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 Thread Jan Korous via Phabricator via cfe-commits
jkorous-apple added inline comments. Comment at: clangd/Protocol.h:190 /// (see exit notification) its process. - llvm::Optional processId; + llvm::Optional processId = 0; Sorry if I am asking stupid question but since the type is Optional<> isn't it's

[PATCH] D43231: [clang-format] Refactor ObjC tests

2018-02-13 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir added a comment. I don't believe this is needed: test fails before would fail at the line where test instance is checked, and after they will fail at the checkLanguage function. Repository: rC Clang https://reviews.llvm.org/D43231 ___

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. This is done as discussed in https://reviews.llvm.org/D43114. But I can instead add a new `AfterObjCSpecialBlock` brace wrapping flag. Repository: rC Clang https://reviews.llvm.org/D43232 ___ cfe-commits mailing list

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. This commit makes potential UB less likely. Logical errors (i.e. forgot to initialize some fields) will still be there. Sending this for review to make sure everyone agrees this is a good thing. Repository: rCTE Clang Tools Extra

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 134019. ilya-biryukov added a comment. - Removed initializers for Optional<> fields, they are not problematic Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43230 Files: clangd/Protocol.h Index: clangd/Protocol.h

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. I think it's useful to explicit initialize non-trivial types like `FileChangeType`. But I think it's safe and probably easier to rely on default values of primitive types like int, bool etc. Explicitly setting default values is probably fine (so this change LGTM), but

[PATCH] D35894: [clangd] Code hover for Clangd

2018-02-13 Thread Simon Marchi via Phabricator via cfe-commits
simark added a comment. > I'm not aware of the code that pretty-prints macros. There's a code that > dumps debug output, but it's not gonna help in that case. > Alternatively, we could simply print the macro name and not print the > definition. That's super-simple and is in line with hovers

[PATCH] D43229: [clangd] Enable snippet completion based on client capabilities.

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added reviewers: hokein, ioeric, sammccall. Herald added subscribers: jkorous-apple, klimek. And remove -enable-snippets flag. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43229 Files: clangd/ClangdLSPServer.cpp

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. To me none of these options make sense. For the case you are describing, I agree that the current behavior is not ideal, but neither are any of the alternatives. However, I think that's fine. We'll just format those cases in a somewhat weird way and users can either

[PATCH] D43226: __threading_support: Remove (void) in favor of ().

2018-02-13 Thread Bruce Mitchener via Phabricator via cfe-commits
brucem created this revision. brucem added reviewers: mclow.lists, EricWF. This fixes a clang-tidy warning when building something that uses this file. Repository: rCXX libc++ https://reviews.llvm.org/D43226 Files: include/__threading_support Index: include/__threading_support

[clang-tools-extra] r324992 - [clangd] SymbolLocation only covers symbol name.

2018-02-13 Thread Haojian Wu via cfe-commits
Author: hokein Date: Tue Feb 13 01:53:50 2018 New Revision: 324992 URL: http://llvm.org/viewvc/llvm-project?rev=324992=rev Log: [clangd] SymbolLocation only covers symbol name. Summary: * Change the offset range to half-open, [start, end). * Fix a few fixmes. Reviewers: sammccall Subscribers:

[PATCH] D43223: [clang-tidy] Update fuchsia-multiple-inheritance to not fail

2018-02-13 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D43223 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43224: Fix typos.

2018-02-13 Thread Bruce Mitchener via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL324989: Fix typos. (authored by brucem, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D43224 Files:

[PATCH] D43127: [clangd] Stop exposing Futures from ClangdServer operations.

2018-02-13 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 Comment at: unittests/clangd/ClangdTests.cpp:783 public: -NoConcurrentAccessDiagConsumer(std::promise StartSecondReparse) -:

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, ioeric, hokein. Herald added subscribers: jkorous-apple, klimek. As a consequence, all LSP operations are now handled asynchronously, i.e. they never block the main processing thread. However, if -run-synchronously

r324991 - Fix for PR32992. Static const classes not exported.

2018-02-13 Thread Hans Wennborg via cfe-commits
Author: hans Date: Tue Feb 13 01:19:43 2018 New Revision: 324991 URL: http://llvm.org/viewvc/llvm-project?rev=324991=rev Log: Fix for PR32992. Static const classes not exported. Patch by zahiraam! Differential Revision: https://reviews.llvm.org/D42968 Modified:

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

2018-02-13 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 134005. https://reviews.llvm.org/D43120 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/ThrowKeywordMissingCheck.cpp clang-tidy/misc/ThrowKeywordMissingCheck.h docs/ReleaseNotes.rst

[PATCH] D43197: [OpenMP] Add flag for linking runtime bitcode library

2018-02-13 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added inline comments. Comment at: test/Driver/openmp-offload-gpu.c:150 +/// bitcode library that will be found via the LIBRARY_PATH. +// RUN: touch /tmp/libomptarget-nvptx-sm_60.bc +// RUN: LIBRARY_PATH=/tmp %clang -### -fopenmp=libomp

[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning

2018-02-13 Thread Oren Ben Simhon via Phabricator via cfe-commits
oren_ben_simhon added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D41880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r324988 - [DebugInfo] Avoid name conflict of generated VLA expression variable.

2018-02-13 Thread Sander de Smalen via cfe-commits
Author: s.desmalen Date: Mon Feb 12 23:49:34 2018 New Revision: 324988 URL: http://llvm.org/viewvc/llvm-project?rev=324988=rev Log: [DebugInfo] Avoid name conflict of generated VLA expression variable. Summary: This patch also adds the 'DW_AT_artificial' flag to the generated variable.

[libcxx] r324989 - Fix typos.

2018-02-13 Thread Bruce Mitchener via cfe-commits
Author: brucem Date: Tue Feb 13 00:12:00 2018 New Revision: 324989 URL: http://llvm.org/viewvc/llvm-project?rev=324989=rev Log: Fix typos. Reviewers: mclow.lists, EricWF Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D43224 Modified:

[PATCH] D43127: [clangd] Stop exposing Futures from ClangdServer operations.

2018-02-13 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL324990: [clangd] Stop exposing Futures from ClangdServer operations. (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[clang-tools-extra] r324990 - [clangd] Stop exposing Futures from ClangdServer operations.

2018-02-13 Thread Sam McCall via cfe-commits
Author: sammccall Date: Tue Feb 13 00:59:23 2018 New Revision: 324990 URL: http://llvm.org/viewvc/llvm-project?rev=324990=rev Log: [clangd] Stop exposing Futures from ClangdServer operations. Summary: LSP has asynchronous semantics, being able to block on an async operation completing is

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. Do you have a reference to style guides recommending any of this? Repository: rC Clang https://reviews.llvm.org/D43183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43167: Fix incorrect indentation.

2018-02-13 Thread Bruce Mitchener via Phabricator via cfe-commits
brucem updated this revision to Diff 134002. brucem added a comment. Rebased forward. Repository: rCXX libc++ https://reviews.llvm.org/D43167 Files: include/ios Index: include/ios === --- include/ios +++ include/ios @@

[PATCH] D42983: [clang-tidy] Add readability-simd-intrinsics check.

2018-02-13 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:46 + + static const llvm::StringMap Mapping{ +// [simd.alg] consider using `llvm::StringSwitch`? Comment at:

[PATCH] D43182: [clangd] SymbolLocation only covers symbol name.

2018-02-13 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. hokein marked 2 inline comments as done. Closed by commit rL324992: [clangd] SymbolLocation only covers symbol name. (authored by hokein, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D43182: [clangd] SymbolLocation only covers symbol name.

2018-02-13 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE324992: [clangd] SymbolLocation only covers symbol name. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D43182?vs=133839=134008#toc Repository: rCTE Clang

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

2018-02-13 Thread Simon Dardis via Phabricator via cfe-commits
sdardis added a comment. Yes. Something like LIBUNWIND_TEST_CFLAGS mapping to config.test_cflags, which libunwind/test/libunwind/test/config.py then appends when building the tests. Thanks. Repository: rUNW libunwind https://reviews.llvm.org/D39074

[PATCH] D43180: [clang-format] Support text proto extensions

2018-02-13 Thread Krasimir Georgiev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC324995: [clang-format] Support text proto extensions (authored by krasimir, committed by ). Changed prior to commit: https://reviews.llvm.org/D43180?vs=133872=134013#toc Repository: rC Clang

r325011 - An updated test to show the current warnings produced for implicit conversions from 'double' to 'float'.

2018-02-13 Thread Andrew V. Tischenko via cfe-commits
Author: avt77 Date: Tue Feb 13 07:20:29 2018 New Revision: 325011 URL: http://llvm.org/viewvc/llvm-project?rev=325011=rev Log: An updated test to show the current warnings produced for implicit conversions from 'double' to 'float'. Modified: cfe/trunk/test/Sema/conversion.c Modified:

[clang-tools-extra] r325015 - [clang-tidy] Update fuchsia-multiple-inheritance to not fail

2018-02-13 Thread Julie Hockett via cfe-commits
Author: juliehockett Date: Tue Feb 13 07:40:40 2018 New Revision: 325015 URL: http://llvm.org/viewvc/llvm-project?rev=325015=rev Log: [clang-tidy] Update fuchsia-multiple-inheritance to not fail Updating the fuchsia-multiple-inheritance to gracefully handle unknown record types (e.g. templatized

[PATCH] D42624: [clang-tidy] Add a utility Matcher to match the next statement within a statement sequence

2018-02-13 Thread Tristan Bourvon via Phabricator via cfe-commits
tbourvon added inline comments. Comment at: clang-tidy/utils/Matchers.h:16 +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Analysis/CFG.h" aaron.ballman wrote: > This will require linking in the clangAnalysis library as well; are we sure > we

[PATCH] D39571: [clangd] DidChangeConfiguration Notification

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In https://reviews.llvm.org/D39571#1005926, @malaperle wrote: > I haven't looked at the newest patch yet but I gave it a quick try and saw > something odd. If I change the configuration to something invalid (say I > specify the path to a CMakeLists.txt), then I

[PATCH] D42623: [clang-tidy] Add a Lexer util to get the source text of a statement

2018-02-13 Thread Tristan Bourvon via Phabricator via cfe-commits
tbourvon abandoned this revision. tbourvon added inline comments. Comment at: clang-tidy/utils/LexerUtils.h:26 +/// Get source code text for statement. +Optional getStmtText(const Stmt* Statement, const SourceManager& SM); + alexfh wrote: > aaron.ballman wrote:

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-13 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment. In https://reviews.llvm.org/D43108#1005904, @nruslan wrote: > @hans: One real-world example is when it is used to compile UEFI code using > PE/COFF targets natively. Obviously, UEFI uses ABI which is basically almost > the same as MS ABI, except that chkstk is not used.

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. In https://reviews.llvm.org/D43183#1006224, @Typz wrote: > It is explicitly documented in google style guide: > https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements > : > > > case blocks in switch statements can have curly braces or not,

[PATCH] D43181: [CodeGen] Initialize large arrays by copying from a global

2018-02-13 Thread Ivan Kosarev via Phabricator via cfe-commits
kosarev added inline comments. Comment at: lib/CodeGen/CGExprAgg.cpp:421 + // with explicit initializers should be large enough. + if (NumInitElements > 8 && elementType->isBuiltinType()) { +CodeGen::CodeGenModule = CGF.CGM; rjmccall wrote: > Is there a

[PATCH] D43240: [OpenCL] Fix __enqueue_block for block with captures

2018-02-13 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision. yaxunl added reviewers: Anastasia, bader. The following test case causes issue with codegen of __enqueue_block void (^block)(void) = ^{ callee(id, out); }; enqueue_kernel(queue, 0, ndrange, block); Clang first does codegen for block expression in the first

[PATCH] D43171: [AMDGPU] Change constant addr space to 4 for clang

2018-02-13 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments. Comment at: test/CodeGenOpenCL/address-spaces.cl:37 +// SPIR: i32 addrspace(2)* %arg +// GIZ: i32 addrspace(4)* %arg void f__c(__constant int *arg) {} t-tye wrote: > Suggest using the same name across all the OpenCL tests as some

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > We'll just format those cases in a somewhat weird way and users can either > accept that or change their code to not need it. I think we have a really diverging opinion on this. From my experience, people will mostly accept the output of the formatter without question :

[PATCH] D43181: [CodeGen] Initialize large arrays by copying from a global

2018-02-13 Thread Ivan Kosarev via Phabricator via cfe-commits
kosarev updated this revision to Diff 134042. kosarev added a comment. Improved as suggested to cover all trivially-copyable types. https://reviews.llvm.org/D43181 Files: lib/CodeGen/CGExprAgg.cpp test/CodeGen/init.c Index: test/CodeGen/init.c

[PATCH] D43187: [AST] Refine the condition for element-dependent array fillers

2018-02-13 Thread Ivan Kosarev via Phabricator via cfe-commits
kosarev added a comment. John, maybe you can review this or suggest some other reviewers? Thanks. Repository: rL LLVM https://reviews.llvm.org/D43187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38639: [clangd] #include statements support for Open definition

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:103 + +if (SourceMgr.isInMainFile(FilenameRange.getAsRange().getBegin())) { + // Only inclusion directives in the main file make sense. The user cannot NIT: replace

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. It is explicitly documented in google style guide: https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements : > case blocks in switch statements can have curly braces or not, depending on > your preference. If you do include curly braces they should

[PATCH] D43223: [clang-tidy] Update fuchsia-multiple-inheritance to not fail

2018-02-13 Thread Julie Hockett via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE325015: [clang-tidy] Update fuchsia-multiple-inheritance to not fail (authored by juliehockett, committed by ). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43223 Files:

[PATCH] D43167: Fix incorrect indentation.

2018-02-13 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. This is fine. Repository: rCXX libc++ https://reviews.llvm.org/D43167 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. In https://reviews.llvm.org/D43183#1006170, @Typz wrote: > > We'll just format those cases in a somewhat weird way and users can either > > accept that or change their code to not need it. > > I think we have a really diverging opinion on this. From my experience, >

[PATCH] D43231: [clang-format] Refactor ObjC tests

2018-02-13 Thread Jacek Olesiak via Phabricator via cfe-commits
jolesiak abandoned this revision. jolesiak added a comment. In https://reviews.llvm.org/D43231#1006123, @krasimir wrote: > I don't believe this is needed: test fails before would fail at the line > where test instance is checked, and after they will fail at the checkLanguage > function.

[PATCH] D35894: [clangd] Code hover for Clangd

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In https://reviews.llvm.org/D35894#1006124, @simark wrote: > Is there a way to get the macro name from the MacroInfo object? I couldn't > find any, so the solution I'm considering is to make > `DeclarationAndMacrosFinder::takeMacroInfos` return an >

[PATCH] D42624: [clang-tidy] Add a utility Matcher to match the next statement within a statement sequence

2018-02-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/utils/Matchers.h:16 +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Analysis/CFG.h" tbourvon wrote: > aaron.ballman wrote: > > This will require linking in the clangAnalysis library as

[PATCH] D42983: [clang-tidy] Add readability-simd-intrinsics check.

2018-02-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked 8 inline comments as done. MaskRay added inline comments. Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:46 + + static const llvm::StringMap Mapping{ +// [simd.alg] hokein wrote: > consider using `llvm::StringSwitch`? The list is

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 134080. ilya-biryukov added a comment. - Capture only needed vars in lambda, not everything. - Rebase onto head. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43227 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.cpp:194 +if (!Replacements) + return replyError(ErrorCode::InternalError, +llvm::toString(Replacements.takeError())); ioeric wrote: > nit: since

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clangd/Protocol.h:80 struct Position { + Position() = default; + Position(int line, int character) : line(line), character(character) {} I'd lean to making callers initialize field-by-field here rather than provide

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Hum, not sure I fully got your proposal. So you actually mean that clang-format you format like this (with 4-space indent for clarity): switch (x) { case 0: break; case 1: { foo(); break; } case 2: { foo(); } break; case 3: {

[libcxxabi] r325022 - [demangler] Rewrite parse_nested_name in the new style.

2018-02-13 Thread Erik Pilkington via cfe-commits
Author: epilk Date: Tue Feb 13 09:09:03 2018 New Revision: 325022 URL: http://llvm.org/viewvc/llvm-project?rev=325022=rev Log: [demangler] Rewrite parse_nested_name in the new style. Modified: libcxxabi/trunk/src/cxa_demangle.cpp Modified: libcxxabi/trunk/src/cxa_demangle.cpp URL:

[libcxxabi] r325023 - [demangler] Support for inheriting constructors.

2018-02-13 Thread Erik Pilkington via cfe-commits
Author: epilk Date: Tue Feb 13 09:09:07 2018 New Revision: 325023 URL: http://llvm.org/viewvc/llvm-project?rev=325023=rev Log: [demangler] Support for inheriting constructors. Fixes PR33223. Modified: libcxxabi/trunk/src/cxa_demangle.cpp libcxxabi/trunk/test/test_demangle.pass.cpp

[libcxx] r325028 - Make the ctype_byname::widen test cases pass on FreeBSD.

2018-02-13 Thread Dimitry Andric via cfe-commits
Author: dim Date: Tue Feb 13 09:43:24 2018 New Revision: 325028 URL: http://llvm.org/viewvc/llvm-project?rev=325028=rev Log: Make the ctype_byname::widen test cases pass on FreeBSD. Modified:

[PATCH] D42606: [Coroutines] Use allocator overload when available

2018-02-13 Thread Gor Nishanov via Phabricator via cfe-commits
GorNishanov added inline comments. Comment at: lib/Sema/SemaCoroutine.cpp:1062 + // an argument list." + for (auto *PD : FD.parameters()) { +if (PD->getType()->isDependentType()) GorNishanov wrote: > This does not implement TS specified behavior for non

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

2018-02-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. I apologize for not noticing this important detail earlier -- I think this check should live under `bugprone` rather than `misc`. Other than where it lives (and the

[PATCH] D43246: [clangd] Assert path is absolute when assigning to URIForFile.

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added reviewers: hokein, ioeric, sammccall. Herald added subscribers: jkorous-apple, klimek. The assertion will point directly to misbehaving code, so that debugging related problems (like the one fixed by r325029) is easier. Repository: rCTE

[clang-tools-extra] r325024 - [clangd] Log if CWD could not be changed. NFC.

2018-02-13 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov Date: Tue Feb 13 09:15:06 2018 New Revision: 325024 URL: http://llvm.org/viewvc/llvm-project?rev=325024=rev Log: [clangd] Log if CWD could not be changed. NFC. Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp clang-tools-extra/trunk/clangd/CodeComplete.cpp

[libcxx] r325027 - Put type attributes after class keyword

2018-02-13 Thread Dimitry Andric via cfe-commits
Author: dim Date: Tue Feb 13 09:40:59 2018 New Revision: 325027 URL: http://llvm.org/viewvc/llvm-project?rev=325027=rev Log: Put type attributes after class keyword Summary: Compiling `` in C++17 or higher mode results in: ``` functional:2500:1: warning: attribute '__visibility__' is ignored,

[clang-tools-extra] r325029 - [clangd] Fixed findDefinitions to always return absolute paths.

2018-02-13 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov Date: Tue Feb 13 09:47:16 2018 New Revision: 325029 URL: http://llvm.org/viewvc/llvm-project?rev=325029=rev Log: [clangd] Fixed findDefinitions to always return absolute paths. Relative paths could be returned in some cases, e.g. when relative path is used in compilation

[PATCH] D38639: [clangd] #include statements support for Open definition

2018-02-13 Thread Marc-Andre Laperle via Phabricator via cfe-commits
malaperle added inline comments. Comment at: unittests/clangd/XRefsTests.cpp:53 +class IgnoreDiagnostics : public DiagnosticsConsumer { + void onDiagnosticsReady( ilya-biryukov wrote: > NIT: remove this class, use `IgnoreDiagnostics` from `Compiler.h`

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In https://reviews.llvm.org/D43230#1006104, @ioeric wrote: > But I think it's safe and probably easier to rely on default values of > primitive types like int, bool etc It's not always safe, as primitive types are sometimes left uninitialized (e.g. when

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 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. Yup, I got bitten recently from some of our plain-c-style structs with no default initializers (in Index). Definitely a fan of this change. Main downside is you can't use aggregate

[PATCH] D43246: [clangd] Assert path is absolute when assigning to URIForFile.

2018-02-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. I think another option to prevent the bug in r325029 from happening would be providing a canonical approach for retrieving (absolute) paths from `FileManager`. We already have code in symbol collector that does this, and we have similar code in XRefs.cpp now. We should

[PATCH] D42995: [ThinLTO] Ignore object files with empty ThinLTO index

2018-02-13 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka planned changes to this revision. vitalybuka added a comment. https://reviews.llvm.org/D42995 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r325021 - [clangd] Remove the RealFS layer from test VFS. NFC.

2018-02-13 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov Date: Tue Feb 13 09:08:13 2018 New Revision: 325021 URL: http://llvm.org/viewvc/llvm-project?rev=325021=rev Log: [clangd] Remove the RealFS layer from test VFS. NFC. It was required before because preambles could only be created on disk. All tests use in-memory preambles now.

[PATCH] D42606: [Coroutines] Use allocator overload when available

2018-02-13 Thread Gor Nishanov via Phabricator via cfe-commits
GorNishanov added inline comments. Comment at: lib/Sema/SemaCoroutine.cpp:1062 + // an argument list." + for (auto *PD : FD.parameters()) { +if (PD->getType()->isDependentType()) This does not implement TS specified behavior for non static member

r325031 - [AMDGPU] Change constant addr space to 4

2018-02-13 Thread Yaxun Liu via cfe-commits
Author: yaxunl Date: Tue Feb 13 10:01:21 2018 New Revision: 325031 URL: http://llvm.org/viewvc/llvm-project?rev=325031=rev Log: [AMDGPU] Change constant addr space to 4 Differential Revision: https://reviews.llvm.org/D43171 Added: cfe/trunk/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl Removed:

[PATCH] D43171: [AMDGPU] Change constant addr space to 4 for clang

2018-02-13 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL325031: [AMDGPU] Change constant addr space to 4 (authored by yaxunl, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D43171: [AMDGPU] Change constant addr space to 4 for clang

2018-02-13 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC325031: [AMDGPU] Change constant addr space to 4 (authored by yaxunl, committed by ). Changed prior to commit: https://reviews.llvm.org/D43171?vs=133802=134071#toc Repository: rC Clang

[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line

2018-02-13 Thread Russell McClellan via Phabricator via cfe-commits
russellmcc added a comment. Ping! I believe all feedback has been addressed - further consideration would be much appreciated. https://reviews.llvm.org/D40988 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. In https://reviews.llvm.org/D43230#1006469, @ilya-biryukov wrote: > In https://reviews.llvm.org/D43230#1006104, @ioeric wrote: > > > But I think it's safe and probably easier to rely on default values of > > primitive types like int, bool etc > > > It's not always safe,

[PATCH] D43209: Put type attributes after class keyword

2018-02-13 Thread Dimitry Andric via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCXX325027: Put type attributes after class keyword (authored by dim, committed by ). Changed prior to commit: https://reviews.llvm.org/D43209?vs=133932=134065#toc Repository: rCXX libc++

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clangd/Protocol.h:190 /// (see exit notification) its process. - llvm::Optional processId; + llvm::Optional processId = 0; jkorous-apple wrote: > Sorry if I am asking stupid question but since the type is

[PATCH] D42983: [clang-tidy] Add readability-simd-intrinsics check.

2018-02-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 134076. MaskRay marked an inline comment as done. MaskRay added a comment. Remove UseStdExperimental Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42983 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/ClangdLSPServer.cpp:194 +if (!Replacements) + return replyError(ErrorCode::InternalError, +llvm::toString(Replacements.takeError())); nit: since we are not spelling out

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-13 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. LG, suggest a tweak to capture() though. I wonder whether we want to introduce `using Callback = UniqueFunction` for readability at some point... Comment

[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning

2018-02-13 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:2007 + +bool Sema::CheckAttrNoArgs(const AttributeList ) { + if (!checkAttributeNumArgs(*this, Attr, 0)) { Wy did this get renamed? Comment at:

r325040 - Update StmtProfile.cpp to handle zero template arguments.

2018-02-13 Thread Richard Trieu via cfe-commits
Author: rtrieu Date: Tue Feb 13 11:53:40 2018 New Revision: 325040 URL: http://llvm.org/viewvc/llvm-project?rev=325040=rev Log: Update StmtProfile.cpp to handle zero template arguments. Treat having no templates arguments differently than having zero template arguments when profiling. Modified:

[PATCH] D39571: [clangd] DidChangeConfiguration Notification

2018-02-13 Thread Simon Marchi via Phabricator via cfe-commits
simark updated this revision to Diff 134097. simark added a comment. Add tests, work in progress Can you take a look at the "TEST(DidChangeConfiguration, DifferentDeclaration)" test, and tell me if you see anything wrong with it? It seems to me like changes in command line arguments (adding

[PATCH] D43221: Teach Wreturn-type, Wunreachable-code, and alpha.deadcode.UnreachableCode to treat __assume(0) like __builtin_unreachable.

2018-02-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm It looks like we already generate llvm.assume() for these and that eventually behaves the same as the unreachable instruction. Neat. :) https://reviews.llvm.org/D43221

[PATCH] D43248: [Attr] Fix printing of parameter indices in attributes

2018-02-13 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny created this revision. jdenny added reviewers: aaron.ballman, hfinkel. Parameter indices in some attributes (argument_with_type_tag, pointer_with_type_tag, nonnull, ownership_takes, ownership_holds, and ownership_returns) are specified in source as one-origin including any this

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/ClangdLSPServer.cpp:194 +if (!Replacements) + return replyError(ErrorCode::InternalError, +llvm::toString(Replacements.takeError())); ilya-biryukov wrote: > ioeric

[PATCH] D43181: [CodeGen] Initialize large arrays by copying from a global

2018-02-13 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGExprAgg.cpp:421 + // with explicit initializers should be large enough. + if (NumInitElements > 8 && elementType->isBuiltinType()) { +CodeGen::CodeGenModule = CGF.CGM; kosarev wrote: > rjmccall

[PATCH] D39571: [clangd] DidChangeConfiguration Notification

2018-02-13 Thread Simon Marchi via Phabricator via cfe-commits
simark added a comment. I think I managed to make some tests by using the `MockCompilationDatabase`. Basically with some code like: #ifndef MACRO static void func () {} // 1 #else static void func () {} // 2 #endif and these steps: 1. Server.addDocument(...) 2.

[PATCH] D43187: [AST] Refine the condition for element-dependent array fillers

2018-02-13 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Repository: rL LLVM https://reviews.llvm.org/D43187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43246: [clangd] Assert path is absolute when assigning to URIForFile.

2018-02-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clangd/Protocol.h:52 struct URIForFile { + URIForFile() = default; I don't like how the API changes here take us further away from the other structs in this file. Why does this one enforce invariants, when the

  1   2   >