Re: r349063 - [CodeComplete] Adhere to LLVM naming style in CodeCompletionTest. NFC

Hi Ilya, I'm not sure which one of the commits in that series is to blame, but ARM bots are broken due to a failure in CodeCompleteTest.cpp, most recent logs are available here: http://lab.llvm.org:8011/builders/clang-cmake-armv7-quick/builds/6090/steps/ninja%20check%201/logs/FAIL%3A%20Clang-Unit

[PATCH] D54834: [analyzer][MallocChecker] Improve warning messages on double-delete errors

Szelethus added a comment. Interesting, I've been watching the bots closely, but got no mail after a while. I'm not sure what the cause is, so I'll revert one-by-one. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54834/new/ https://reviews.llvm.org/D54834 ___

[PATCH] D54918: [analyzer] Apply clang-format to GenericTaintChecker.cpp

Szelethus added a comment. @boga95 Do you have commit access, or need this to be commited on your behalf? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54918/new/ https://reviews.llvm.org/D54918 ___ cfe-commits maili

[PATCH] D38675: [analyzer] MoveChecker Pt.10: Move the checker out of alpha state.

Szelethus added a comment. A little late to the party, but I don't see anything against this. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D38675/new/ https://reviews.llvm.org/D38675 ___ cfe-commits mailing list cfe-

[PATCH] D55280: [CTU] Make loadExternalAST return with non nullptr on success

martong added a comment. Ping @a_sidorin Could you please take another look and consider my previous comment? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55280/new/ https://reviews.llvm.org/D55280 ___ cfe-commits

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

martong added a comment. Ping @shafik, I have addressed you comments, could you please take another look? If you don't have any objections, could you please approve? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53699/new/ https://reviews.llvm.org/D53699

[PATCH] D55428: [Docs] Expand -fstack-protector and -fstack-protector-all info

This revision was automatically updated to reflect the committed changes. Closed by commit rL349335: [Docs] Expand -fstack-protector and -fstack-protector-all (authored by carwil, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D55428?v

[PATCH] D55640: [clang-tidy] Implement a check for large Objective-C type encodings 🔍

hokein added inline comments. Comment at: clang-tidy/objc/TypeEncodingSizeCheck.cpp:36 + objcPropertyDecl(unless(isExpansionInSystemHeader()), + anyOf(hasAncestor(objcInterfaceDecl().bind("interface")), + hasAncestor(objcCat

r349336 - Reverting bitfield size to attempt to fix a windows buildbot

Author: szelethus Date: Mon Dec 17 02:31:35 2018 New Revision: 349336 URL: http://llvm.org/viewvc/llvm-project?rev=349336&view=rev Log: Reverting bitfield size to attempt to fix a windows buildbot Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Modified: cfe/trunk/lib/Stati

[PATCH] D55437: [Index] Index declarations in lambda expression.

hokein updated this revision to Diff 178437. hokein added a comment. Update the patch, add comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55437/new/ https://reviews.llvm.org/D55437 Files: lib/Index/IndexBody.cpp test/Index/cxx11-lambdas.cpp Index:

[libunwind] r349339 - [AArch64][libunwind] Unwinding support for return address signing with B Key

Author: lukecheeseman Date: Mon Dec 17 03:43:24 2018 New Revision: 349339 URL: http://llvm.org/viewvc/llvm-project?rev=349339&view=rev Log: [AArch64][libunwind] Unwinding support for return address signing with B Key - Support for the case where the return address has been signed with the B key -

r349340 - Revert rC349281 '[analyzer][MallocChecker][NFC] Document and reorganize some functions'

Author: szelethus Date: Mon Dec 17 04:07:57 2018 New Revision: 349340 URL: http://llvm.org/viewvc/llvm-project?rev=349340&view=rev Log: Revert rC349281 '[analyzer][MallocChecker][NFC] Document and reorganize some functions' Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

[clang-tools-extra] r349341 - Revert rCTE349288 'Fix a lit test failure after MallocChecker changes'

Author: szelethus Date: Mon Dec 17 04:08:39 2018 New Revision: 349341 URL: http://llvm.org/viewvc/llvm-project?rev=349341&view=rev Log: Revert rCTE349288 'Fix a lit test failure after MallocChecker changes' Modified: clang-tools-extra/trunk/test/clang-tidy/static-analyzer.cpp Modified: clang

[PATCH] D54834: [analyzer][MallocChecker] Improve warning messages on double-delete errors

Szelethus reopened this revision. Szelethus added a comment. This revision is now accepted and ready to land. Reverted in rC349340 . With the wrong revision name... oops... Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54834/new

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

ilya-biryukov added a comment. Good point, we should definitely state it's a clangd-specific extension. We could come up with a naming scheme for our extensions. I would propose something like `clangd:$methodName`, i.e. the current extensions would be `clangd:textDocument/fileStatus`. WDYT? R

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clangd/index/Background.h:121 bool ShouldStop = false; - std::deque Queue; + std::deque> Tasks; std::vector ThreadPool; // FIXME: Abstr

r349342 - Fix "enumeral mismatch in conditional expression" gcc7 warnings. NFCI.

Author: rksimon Date: Mon Dec 17 04:17:37 2018 New Revision: 349342 URL: http://llvm.org/viewvc/llvm-project?rev=349342&view=rev Log: Fix "enumeral mismatch in conditional expression" gcc7 warnings. NFCI. Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mo

[PATCH] D55417: [clangd] Change disk-backed storage to be atomic

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM with a few nits Comment at: clangd/index/BackgroundIndexStorage.cpp:39 +template +llvm::Error writeAtomically(llvm::StringRef OutPath, WriterFunction &&Wr

r349343 - Fix "enumeral mismatch in conditional expression" gcc7 warning. NFCI.

Author: rksimon Date: Mon Dec 17 04:25:42 2018 New Revision: 349343 URL: http://llvm.org/viewvc/llvm-project?rev=349343&view=rev Log: Fix "enumeral mismatch in conditional expression" gcc7 warning. NFCI. Modified: cfe/trunk/lib/Sema/SemaStmtAttr.cpp Modified: cfe/trunk/lib/Sema/SemaStmtAttr.

r349344 - Revert rC349281 '[analyzer][MallocChecker][NFC] Document and reorganize some functions'

Author: szelethus Date: Mon Dec 17 04:25:48 2018 New Revision: 349344 URL: http://llvm.org/viewvc/llvm-project?rev=349344&view=rev Log: Revert rC349281 '[analyzer][MallocChecker][NFC] Document and reorganize some functions' Accidentally commited earlier with the same commit title, but really it

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

kadircet updated this revision to Diff 178443. kadircet marked an inline comment as done. kadircet added a comment. - Revert name from Tasks to Queue Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55315/new/ https://reviews.llvm.org/D55315 Files:

[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

Szelethus reopened this revision. Szelethus added a comment. This revision is now accepted and ready to land. Reverted in rC349344 . Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54823/new/ https://reviews.llvm.org/D54823

r349335 - [Docs] Expand -fstack-protector and -fstack-protector-all

Author: carwil Date: Mon Dec 17 02:11:35 2018 New Revision: 349335 URL: http://llvm.org/viewvc/llvm-project?rev=349335&view=rev Log: [Docs] Expand -fstack-protector and -fstack-protector-all Improve the description of these command line options by providing specific heuristic information, as outl

[clang-tools-extra] r349345 - [clangd] Only reduce priority of a thread for indexing.

Author: kadircet Date: Mon Dec 17 04:30:27 2018 New Revision: 349345 URL: http://llvm.org/viewvc/llvm-project?rev=349345&view=rev Log: [clangd] Only reduce priority of a thread for indexing. Summary: We'll soon have tasks pending for reading shards from disk, we want them to have normal priority.

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE349345: [clangd] Only reduce priority of a thread for indexing. (authored by kadircet, committed by ). Changed prior to commit: https://reviews.llvm.org/D55315?vs=178443&id=178445#toc Repository: r

[clang-tools-extra] r349348 - [clangd] Change diskbackedstorage to be atomic

Author: kadircet Date: Mon Dec 17 04:38:22 2018 New Revision: 349348 URL: http://llvm.org/viewvc/llvm-project?rev=349348&view=rev Log: [clangd] Change diskbackedstorage to be atomic Summary: There was a chance that multiple clangd instances could try to write same shard, in which case we would ge

[PATCH] D55417: [clangd] Change disk-backed storage to be atomic

kadircet updated this revision to Diff 178448. kadircet marked 3 inline comments as done. kadircet added a comment. - Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55417/new/ https://reviews.llvm.org/D55417 Files: clangd/index/Ba

[PATCH] D55417: [clangd] Change disk-backed storage to be atomic

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE349348: [clangd] Change diskbackedstorage to be atomic (authored by kadircet, committed by ). Changed prior to commit: https://reviews.llvm.org/D55417?vs=178448&id=178449#toc Repository: rCTE Clang

[PATCH] D53655: [ASTImporter] Fix redecl chain of classes and class templates

This revision was automatically updated to reflect the committed changes. Closed by commit rC349349: [ASTImporter] Fix redecl chain of classes and class templates (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D53655?vs=175641&id=178450#toc Repository:

r349349 - [ASTImporter] Fix redecl chain of classes and class templates

Author: martong Date: Mon Dec 17 04:42:12 2018 New Revision: 349349 URL: http://llvm.org/viewvc/llvm-project?rev=349349&view=rev Log: [ASTImporter] Fix redecl chain of classes and class templates Summary: The crux of the issue that is being fixed is that lookup could not find previous decls of a

[PATCH] D55683: [analyzer] Tests for scan-build?

aaron.ballman added a comment. Could we make use of scan-build `-o` to specify the exact output location, in conjunction with `stable-report-filename=true` to remove the non-determinism? Also, I like the idea of maybe someday making scan-build-py test the same file. However, it seems like that

[PATCH] D53708: [ASTImporter] Add importer specific lookup

martong updated this revision to Diff 178452. martong added a comment. - Rebase to master (trunk) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53708/new/ https://reviews.llvm.org/D53708 Files: include/clang/AST/ASTImporter.h include/clang/AST/ASTImporterLoo

[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

Szelethus added a comment. Lit test failures are gone for Windows after reverting this. Will probably deal with this revision after 8.0.0. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54823/new/ https://reviews.llvm.org/D54823

[PATCH] D55656: [OpenCL] Address space for default class members

Anastasia added inline comments. Comment at: lib/AST/ASTContext.cpp:2781 + + return getAddrSpaceQualType(NewT, Orig.getAddressSpace()); } rjmccall wrote: > You're trying to handle a method qualifier, not a type a functions that are > themselves in some non-sta

[PATCH] D53708: [ASTImporter] Add importer specific lookup

riccibruno added a comment. This is a perhaps a naive comment, but why is `localUncachedLookup` used instead of `noload_lookup` ? There is a fixme dating from 2013 stating that `noload_lookup` should be used instead. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D5

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

hokein updated this revision to Diff 178455. hokein added a comment. Update: - avoid using terms that C++ programmers are unfamiliar with. - textDocument/fileStatus => textDocument/clangd.fileStatus Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D5536

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

hokein added a comment. > Sorry if I missed any important design discussions here, but wanted to clear > up what information we are trying to convey to the user with the status > messages? > E.g. why seeing "building preamble", "building file" or "queued" in the > status bar can be useful to t

r349351 - [ASTImporter] Add importer specific lookup

Author: martong Date: Mon Dec 17 05:53:12 2018 New Revision: 349351 URL: http://llvm.org/viewvc/llvm-project?rev=349351&view=rev Log: [ASTImporter] Add importer specific lookup Summary: There are certain cases when normal C/C++ lookup (localUncachedLookup) does not find AST nodes. E.g.: Example

[PATCH] D53708: [ASTImporter] Add importer specific lookup

This revision was automatically updated to reflect the committed changes. Closed by commit rL349351: [ASTImporter] Add importer specific lookup (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D53708: [ASTImporter] Add importer specific lookup

martong added a comment. > This is a perhaps a naive comment, but why is localUncachedLookup used instead of noload_lookup ? There is a fixme dating from 2013 stating that noload_lookup should be used instead. This is not a naive question. I was wondering myself on the very same thing when I st

[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

Szelethus added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51866/new/ https://reviews.llvm.org/D51866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:2871-2872 /// distribute: attempt to distribute loop if State == Enable + /// pipeline: disable pipelining loop if State == Disable + /// pipeline_ii_count: try to pipeline loop for only 'Value' val

[PATCH] D55741: Implementation Feature Test Macros for P0722R3

ckennelly added a comment. Yes, I need someone to commit for me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55741/new/ https://reviews.llvm.org/D55741 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/

[PATCH] D55719: [OpenMP] parsing and sema support for 'close' map-type-modifier

ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55719/new/ https://reviews.llvm.org/D55719 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

ilya-biryukov added a comment. Probably the most important suggestion from my side is trying to clearly separate the `enqueue` calls (which actually schedule rebuilds of TUs) using clang and the `loadShards` function. The index loading should be fast, so I assume we won't win much latency by sc

[PATCH] D53708: [ASTImporter] Add importer specific lookup

riccibruno added a comment. In D53708#1332868 , @martong wrote: > > This is a perhaps a naive comment, but why is localUncachedLookup used > > instead of noload_lookup ? There is a fixme dating from 2013 stating > > that noload_lookup should be used inst

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

martong added a comment. Alexey, simply put, the crux of the problem is this: The imported decl context is different than what we get by calling the `getDeclContext()` on an imported field/friend of the "from" decl context. With other words: `import(cast(FromDC)` gives us a different value than

[PATCH] D53708: [ASTImporter] Add importer specific lookup

martong added a comment. In D53708#1332922 , @riccibruno wrote: > In D53708#1332868 , @martong wrote: > > > > This is a perhaps a naive comment, but why is localUncachedLookup used > > > instead of noload_lookup ?

r349357 - Build ASTImporterTest.cpp with /bigobj on MSVC builds to keep llvm-clang-x86_64-expensive-checks-win buildbot happy

Author: rksimon Date: Mon Dec 17 07:14:08 2018 New Revision: 349357 URL: http://llvm.org/viewvc/llvm-project?rev=349357&view=rev Log: Build ASTImporterTest.cpp with /bigobj on MSVC builds to keep llvm-clang-x86_64-expensive-checks-win buildbot happy Modified: cfe/trunk/unittests/AST/CMakeLis

[PATCH] D53708: [ASTImporter] Add importer specific lookup

riccibruno added a comment. > Exactly. And this seems to be an important feature in ASTImporter, because > this makes it possible that we can chain into the redecl chain a newly > imported AST node to existing and structurally equivalent > nodes. (The > existing nodes are found by the lookup an

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

alexey.lapshin marked 2 inline comments as done. alexey.lapshin added a comment. Ok, I will address all issues. Comment at: include/clang/Basic/Attr.td:2871-2872 /// distribute: attempt to distribute loop if State == Enable + /// pipeline: disable pipelining loop if State =

[PATCH] D55765: Fix use-after-free bug in Tooling.

ymandel created this revision. ymandel added a reviewer: alexfh. Herald added a subscriber: cfe-commits. `buildASTFromCodeWithArgs()` was creating a memory buffer referencing a stack-allocated string. This diff changes the implementation to copy the code string into the memory buffer so that said

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

ebevhan added inline comments. Comment at: lib/CodeGen/CGCall.cpp:77 + if (MD) +RecTy = Context.getAddrSpaceQualType(RecTy, MD->getType().getAddressSpace()); return Context.getPointerType(CanQualType::CreateUnsafe(RecTy)); I'm a bit late to the party, bu

[PATCH] D55765: Fix use-after-free bug in Tooling.

ymandel updated this revision to Diff 178468. ymandel added a comment. Change buildAST functions to take a StringRef instead of a Twine. In practice, code is almost never constructed on the fly using a Twine, so StringRef is simpler and avoids needing a copy when constructing the MemBuffer. Re

Re: r349053 - [CodeComplete] Fill preferred type on binary expressions

Hi Douglas, Reid, Sorry for the breakage, I'll take a look at the bug. On Sat, Dec 15, 2018 at 12:18 AM wrote: > Hi, I think this test is still broken on Windows when you are building a > cross compiler as we at Sony do. I have put the details in PR40033. Can > somebody take a look? > > > > Do

[PATCH] D55765: Fix use-after-free bug in Tooling.

ymandel updated this revision to Diff 178470. ymandel added a comment. Change header correspondingly. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55765/new/ https://reviews.llvm.org/D55765 Files: include/clang/Tooling/Tooling.h lib/Tooling/Tooling.cpp In

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

Szelethus added a comment. > I still expect some skeletons to fall out of the closet This patch doesn't handle -analyzer-disable-checker, which leads to assertation failures in later pathes. Since the way which checker should/shouldnt be enabled is implemented rather poorly, I'll probably try t

[PATCH] D55770: [clangd] BackgroundIndex rebuilds symbol index periodically.

ioeric created this revision. ioeric added reviewers: ilya-biryukov, kadircet. Herald added subscribers: cfe-commits, jfb, arphaman, jkorous, MaskRay. Currently, background index rebuilds symbol index on every indexed file, which can be inefficient. This patch makes it only rebuild symbol index p

[PATCH] D55771: [AST] Store the callee and argument expressions of CallExpr in a trailing array.

riccibruno created this revision. riccibruno added a reviewer: rjmccall. riccibruno added a project: clang. Herald added subscribers: cfe-commits, javed.absar. Herald added a reviewer: shafik. Since `CallExpr::setNumArgs` has been removed, it is now possible to store the callee expression and the

[PATCH] D54676: [AST] Pack CallExpr

riccibruno abandoned this revision. riccibruno added a comment. Abandoned in favor of D55771 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54676/new/ https://reviews.llvm.org/D54676

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

aprantl added inline comments. Comment at: lib/AST/ASTImporter.cpp:1672 // FIXME: Handle this case somehow better. - consumeError(ImportedOrErr.takeError()); + else +consumeError(ImportedOrErr.takeError()); this else is redundant. =

r349362 - [CodeComplete] Fix test failure on different host and target configs

Author: ibiryukov Date: Mon Dec 17 08:37:52 2018 New Revision: 349362 URL: http://llvm.org/viewvc/llvm-project?rev=349362&view=rev Log: [CodeComplete] Fix test failure on different host and target configs This should fix PR40033. Modified: cfe/trunk/unittests/Sema/CodeCompleteTest.cpp Modif

[PATCH] D54918: [analyzer] Apply clang-format to GenericTaintChecker.cpp

boga95 added a comment. I don't have commit access. Can you please do it on my behalf? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54918/new/ https://reviews.llvm.org/D54918 ___ cfe-commits mailing list cfe-commits

[PATCH] D53928: Enable builtins necessary for SLEEF [AArch64] vectorized trigonometry libm functions

steleman updated this revision to Diff 178477. steleman added a comment. Herald added a subscriber: arphaman. Updated version of this changeset/patch, as per Renato's latest comments from D53927 (https://reviews.llvm.org/D53927). New benchmark comparing SLEEF pe

[PATCH] D54834: [analyzer][MallocChecker] Improve warning messages on double-delete errors

NoQ added a comment. In D54834#1332679 , @Szelethus wrote: > Interesting, I've been watching the bots closely, but got no mail after a > while. I'm not sure what the cause is, so I'll revert one-by-one. One of the common reasons for that is that the bui

[PATCH] D55712: [Driver][PS4] Do not implicitly link against asan or ubsan if -nostdlib or -nodefaultlibs on PS4.

probinson added a comment. LGTM but I will defer to @filcab as he is more familiar with the area. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55712/new/ https://reviews.llvm.org/D55712 ___ cfe-commits mailing list

[PATCH] D55437: [Index] Index declarations in lambda expression.

ilya-biryukov added a comment. Update after investigating with @hokein offline: the `RecursiveASTVisitor` has custom code to visit lambda parameters and return type, but it fallback to visiting typelocs when both parameters and a return type are specified. Unfortunately this fallback does not w

Re: r349063 - [CodeComplete] Adhere to LLVM naming style in CodeCompletionTest. NFC

Hi Yvan, sorry for the inconvenience. I believe this is the same as https://llvm.org/PR40033, should be fixed by r349362 (the build hasn't finished yet). I'll double check it's fixed tomorrow and make sure to take another look if not. On Mon, Dec 17, 2018 at 10:46 AM Yvan Roux wrote: > Hi Ilya,

[PATCH] D54176: [PGO] clang part of change for context-sensitive PGO.

lebedev.ri added a comment. Clang reviews should mainly go to cfe-commits, not llvm-commits. (all that will happen automagically if one sets the correct repo for the differential..) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54176/new/ https://reviews.llvm.or

[PATCH] D55770: [clangd] BackgroundIndex rebuilds symbol index periodically.

kadircet added inline comments. Comment at: clangd/index/Background.cpp:464 + if (BuildIndexPeriodMs > 0) +SymbolsUpdatedSinceLastIndex = true; + else why not simply check if `BuildIndexPeriodMs` has passed and issue rebuild here? That way we won't spawn a

[PATCH] D55775: [Driver] Don't override '-march' when using '-arch x86_64h'

thegameg created this revision. thegameg added reviewers: vsk, dexonsmith, ab, t.p.northover, Gerolf. On Darwin, using '-arch x86_64h' would always override the option passed through '-march'. This patch allows users to use '-march' with x86_64h, while keeping the default to 'core-avx2' https

[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

steven_wu accepted this revision. steven_wu added a comment. This revision is now accepted and ready to land. Other than a small design choice commented inline, LGTM. Comment at: include/clang/Driver/DarwinSDKInfo.h:36 +/// SDK has no SDKSettings.json, or a valid \c DarwinSDKIn

[PATCH] D55685: Update Microsoft name mangling scheme for exception specifiers in the type system

zahen updated this revision to Diff 178490. zahen added a comment. Added support for msvc minor version as requested. Tied the updated mangling scheme to C++17 & compatibility mode > 1912 (15.5). Added additional tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55685/new/ https:/

[PATCH] D55749: [Driver] Automatically enable -munwind-tables if -fseh-exceptions is enabled

rnk added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:3929 + options::OPT_fno_asynchronous_unwind_tables, + (TC.IsUnwindTablesDefault(Args) || + (ExceptionArg && Can you put this logic in the Windows or mingw toolchain instead by

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:2871-2872 /// distribute: attempt to distribute loop if State == Enable + /// pipeline: disable pipelining loop if State == Disable + /// pipeline_ii_count: try to pipeline loop for only 'Value' val

[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

arphaman marked an inline comment as done. arphaman added inline comments. Comment at: include/clang/Driver/DarwinSDKInfo.h:36 +/// SDK has no SDKSettings.json, or a valid \c DarwinSDKInfo otherwise. +Expected> parseDarwinSDKInfo(llvm::vfs::FileSystem &VFS, +

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

alexey.lapshin marked an inline comment as done. alexey.lapshin added inline comments. Comment at: include/clang/Basic/Attr.td:2871-2872 /// distribute: attempt to distribute loop if State == Enable + /// pipeline: disable pipelining loop if State == Disable + /// pipeline_i

[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

steven_wu added inline comments. Comment at: include/clang/Driver/DarwinSDKInfo.h:36 +/// SDK has no SDKSettings.json, or a valid \c DarwinSDKInfo otherwise. +Expected> parseDarwinSDKInfo(llvm::vfs::FileSystem &VFS, + StringRef

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:2871-2872 /// distribute: attempt to distribute loop if State == Enable + /// pipeline: disable pipelining loop if State == Disable + /// pipeline_ii_count: try to pipeline loop for only 'Value' val

[PATCH] D55770: [clangd] BackgroundIndex rebuilds symbol index periodically.

dblaikie added a comment. (from the peanut gallery): Usually what I'd expect here is "reindexing will occur X milliseconds after a write, covering anything written up until that point" - so it doesn't reindex unnecessarily if the files are unmodified/sitting idle, and it doesn't thrash reindexi

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

leonardchan added a comment. In D53738#1326071 , @rjmccall wrote: > I'm fine with making this change under the assumption that we've gotten the > language rule right. Even if that weren't abstractly reasonable for general > language work — and I do thin

[PATCH] D55731: [darwin][arm64] use the "cyclone" CPU for Darwin even when `-arch` is not specified

ab accepted this revision. ab added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55731/new/ https://reviews.llvm.org/D55731 ___ cfe-commits m

[PATCH] D54604: Automatic variable initialization

rsmith added a comment. Thanks, you've addressed my comments to my satisfaction. (Not approving as I've not done a detailed review of the implementation, only of the approach, but it looks like your review with @pcc is nearly complete.) In D54604#1330356

r349380 - [darwin] parse the SDK settings from SDKSettings.json if it exists and

Author: arphaman Date: Mon Dec 17 11:19:15 2018 New Revision: 349380 URL: http://llvm.org/viewvc/llvm-project?rev=349380&view=rev Log: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend This commit adds support for re

[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

This revision was automatically updated to reflect the committed changes. Closed by commit rC349380: [darwin] parse the SDK settings from SDKSettings.json if it exists and (authored by arphaman, committed by ). Changed prior to commit: https://reviews.llvm.org/D55673?vs=178309&id=178501#toc Re

[PATCH] D55775: [Driver] Don't override '-march' when using '-arch x86_64h'

Gerolf added reviewers: ahatanak, qcolombet. Gerolf added a comment. + Akira, Quentin for their driver / x86_64h experience for a quick double check. Fwiw, LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55775/new/ https://reviews.llvm.org/D55775

[PATCH] D55775: [Driver] Don't override '-march' when using '-arch x86_64h'

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55775/new/ https://reviews.llvm.org/D55775 ___ cfe-commits mailing list cfe-commi

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

hfinkel added inline comments. Comment at: include/clang/Basic/Attr.td:2871-2872 /// distribute: attempt to distribute loop if State == Enable + /// pipeline: disable pipelining loop if State == Disable + /// pipeline_ii_count: try to pipeline loop for only 'Value' value -

[PATCH] D55775: [Driver] Don't override '-march' when using '-arch x86_64h'

This revision was automatically updated to reflect the committed changes. Closed by commit rL349381: [Driver] Don't override '-march' when using '-arch x86_64h' (authored by thegameg, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D557

r349381 - [Driver] Don't override '-march' when using '-arch x86_64h'

Author: thegameg Date: Mon Dec 17 11:29:27 2018 New Revision: 349381 URL: http://llvm.org/viewvc/llvm-project?rev=349381&view=rev Log: [Driver] Don't override '-march' when using '-arch x86_64h' On Darwin, using '-arch x86_64h' would always override the option passed through '-march'. This patch

r349382 - [darwin][arm64] use the "cyclone" CPU for Darwin even when `-arch`

Author: arphaman Date: Mon Dec 17 11:30:46 2018 New Revision: 349382 URL: http://llvm.org/viewvc/llvm-project?rev=349382&view=rev Log: [darwin][arm64] use the "cyclone" CPU for Darwin even when `-arch` is not specified The -target option allows the user to specify the build target using LLVM trip

[PATCH] D55731: [darwin][arm64] use the "cyclone" CPU for Darwin even when `-arch` is not specified

This revision was automatically updated to reflect the committed changes. Closed by commit rC349382: [darwin][arm64] use the "cyclone" CPU for Darwin even when `-arch` (authored by arphaman, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55731/new/

Re: r349212 - Mangle calling conventions into function pointer types where GCC does

I sent https://reviews.llvm.org/D55781 and cc'd the original reviewers so we can decide if it's worth the cost. On Fri, Dec 14, 2018 at 5:17 PM Richard Smith wrote: > Should this change be disableable by -fclang-abi-compat=7 or below? > > On Fri, 14 Dec 2018, 15:46 Reid Kleckner via cfe-commits

[PATCH] D55781: Make CC mangling conditional on the ABI version

rnk created this revision. rnk added reviewers: rjmccall, rsmith. Herald added a subscriber: krytarowski. https://reviews.llvm.org/D55781 Files: clang/lib/AST/ItaniumMangle.cpp clang/test/CodeGenCXX/mangle-win-ccs-old.cpp Index: clang/test/CodeGenCXX/mangle-win-ccs-old.cpp ==

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

rsmith added a comment. I think the ideal solution would be something like: If a variable produced by a macro expansion shadows a variable not produced by a macro expansion, do not warn immediately. Instead, warn if we see a use of the inner variable in a context that is not produced by the sam

[PATCH] D55781: Make CC mangling conditional on the ABI version

rnk added a comment. @rsmith asked if we should make this mangling conditional on the ABI version. I implemented it here, let me know if you think it's worth the extra code complexity. Our mingw ABI hasn't been particularly stable, but I think going forward we'll want to start mangling new call

[PATCH] D55765: Fix use-after-free bug in Tooling.

ymandel updated this revision to Diff 178507. ymandel added a comment. Update test that was calling builAST with an actual Twine. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55765/new/ https://reviews.llvm.org/D55765 Files: include/clang/Tooling/Tooling.h

[PATCH] D55662: [Sema][ObjC] Do not warn about repeated uses of weak variables when the variables are accessed in an unevaluated context.

rjmccall added a comment. Do you have an example, and is there a reasonable place to call `CheckPlaceholderExpr` on those paths? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55662/new/ https://reviews.llvm.org/D55662 _

r349384 - [NFC] Test commit: tweak whitespace in comment

Author: cpplearner Date: Mon Dec 17 11:53:22 2018 New Revision: 349384 URL: http://llvm.org/viewvc/llvm-project?rev=349384&view=rev Log: [NFC] Test commit: tweak whitespace in comment Modified: cfe/trunk/test/CXX/temp/temp.param/p3.cpp Modified: cfe/trunk/test/CXX/temp/temp.param/p3.cpp URL:

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

rjmccall added a comment. In D53738#1333276 , @leonardchan wrote: > In D53738#1326071 , @rjmccall wrote: > > > I'm fine with making this change under the assumption that we've gotten the > > language rule right. E

  1   2   >