[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

thakis added inline comments. Comment at: clang-tools-extra/trunk/clangd/test/CMakeLists.txt:5 + # No tests for these, but we should still make sure they build. + clangd-indexer + dexp Since you're touching this anyway, there's now one test that calls

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

twoh added a comment. Friendly ping for comments. Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D38061/new/ https://reviews.llvm.org/D38061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

r359574 - [LibTooling] Change Transformer's TextGenerator to a partial function.

Author: ymandel Date: Tue Apr 30 09:48:33 2019 New Revision: 359574 URL: http://llvm.org/viewvc/llvm-project?rev=359574=rev Log: [LibTooling] Change Transformer's TextGenerator to a partial function. Summary: Changes the signature of the TextGenerator std::function to return an Expected instead

[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

tra added a comment. +1 to Hal's comments. @jdoerfert : > I'd even go as far as to argue that __clang_cuda_device_functions.h should > include the internal math.h wrapper to get all math functions. See also the > next comment. I'd argue other way around -- include

[PATCH] D60974: Clang IFSO driver action.

jakehehrlich added a comment. > Does llvm-elfabi consume your proposed Schema format? Has it landed yet? No, I just proposed it and explained my reasoning. If you wanted to add this format to TextAPI that would be acceptable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

phosek marked an inline comment as not done. phosek added a subscriber: rsmith. phosek added inline comments. Comment at: libunwind/CMakeLists.txt:190 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) - set(DEFAULT_INSTALL_PREFIX

[PATCH] D50515: Re-push "[Option] Fix PR37006 prefix choice in findNearest"

thakis added a comment. I think I figured it out. r359604 should fix the bug that caused this to be reverted twice. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D50515/new/ https://reviews.llvm.org/D50515 ___

[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

This revision was automatically updated to reflect the committed changes. ymandel marked an inline comment as done. Closed by commit rL359574: [LibTooling] Change Transformers TextGenerator to a partial function. (authored by ymandel, committed by ). Herald added a project: LLVM. Herald added a

[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

ymandel added a comment. In D61015#1484669 , @thakis wrote: > This breaks a test: > http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/12112/steps/check-llvm%20check-clang%20stage3%2Fmsan/logs/stdio > > [--] 1 test

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

phosek marked an inline comment as done. phosek added inline comments. Comment at: libunwind/CMakeLists.txt:190 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) - set(DEFAULT_INSTALL_PREFIX lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/) -

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

phosek added a comment. In D59168#1484467 , @jdenny wrote: > In D59168#1480801 , @phosek wrote: > > > In D59168#1480428 , @jdenny wrote: > > > > > It seems we have roughly

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

vsapsai added a comment. Thanks for the insightful explanation, Craig. As far as I understand, implementing intrinsics with builtins is possible but it is more complex and wasn't providing enough value to prefer it over inline assembly. If that is correct, I'd like to revive the abandoned

[PATCH] D61032: [clang] Avoid defining duplicate header search paths for libc++ on Darwin

ldionne abandoned this revision. ldionne added a comment. I decided to use a different approach and first refactor the header search logic from CC1 to the driver. A patch should come eventually when I've tested it properly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

hfinkel added a comment. In D60907#1483660 , @jdoerfert wrote: > In D60907#1483615 , @hfinkel wrote: > > > In D60907#1479370 , @gtbercea > > wrote: > > > > > In

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

sammccall marked 3 inline comments as done. sammccall added inline comments. Comment at: clang-tools-extra/trunk/clangd/unittests/lit.cfg.in:1 +@LIT_SITE_CFG_IN_HEADER@ +# This is a shim to run the gtest unittests in ../unittests using lit. MaskRay wrote: >

[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

jdoerfert added a comment. In D60907#1484529 , @hfinkel wrote: > In D60907#1483660 , @jdoerfert wrote: > > > In D60907#1483615 , @hfinkel wrote: > > > > > In D60907#1479370

[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

tra added a comment. In D60907#1484756 , @jdoerfert wrote: > I actually don't want to preinclude anything and my arguments are (mostly) > for the OpenMP offloading code path not necessarily Cuda. > Maybe to clarify, what I want is: > > 1. Make sure the

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

jdenny added a comment. In D59168#1484754 , @phosek wrote: > It's the `LIBCXX_ABI_VERSION` CMake option, see > https://github.com/llvm/llvm-project/blob/master/libcxx/CMakeLists.txt#L121 Thanks. Comment at:

r359598 - Add requires amdgpu-registered-target for amdgpu-float16.cpp

Author: yaxunl Date: Tue Apr 30 12:06:15 2019 New Revision: 359598 URL: http://llvm.org/viewvc/llvm-project?rev=359598=rev Log: Add requires amdgpu-registered-target for amdgpu-float16.cpp Modified: cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp Modified:

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

craig.topper added a comment. Your understanding is correct. As far as testing I think the existing testing in the original patches is sufficient. I'm not sure I understand how a target specific intrinsic that only works on x86 in the bitcode is substantially better than inline assembly. Do

[PATCH] D60848: [Parser] Avoid correcting delayed typos in array subscript multiple times.

vsapsai updated this revision to Diff 197421. vsapsai added a comment. - Add a test case with `-disable-free`. Thanks for the suggestion, Erik. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60848/new/ https://reviews.llvm.org/D60848 Files: clang/lib/Parse/ParseExpr.cpp

[PATCH] D61319: [PR41674] [OpenCL] Fix initialisation of this via pointer

kpet created this revision. kpet added reviewers: Anastasia, mikael. Herald added subscribers: cfe-commits, yaxunl. Herald added a project: clang. When the expression used to initialise this has a pointer type, check the address space of the pointee type instead of the pointer type to decide

[PATCH] D58060: Fix diagnostic for addr spaces in reference binding

Anastasia added a comment. @rjmccall or @ebevhan do you have any more feedback for this patch? Btw, it has now dependency with https://reviews.llvm.org/D61318 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58060/new/ https://reviews.llvm.org/D58060

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

thakis added inline comments. Comment at: clang-tools-extra/trunk/clangd/unittests/lit.cfg.in:1 +@LIT_SITE_CFG_IN_HEADER@ +# This is a shim to run the gtest unittests in ../unittests using lit. sammccall wrote: > MaskRay wrote: > > thakis wrote: > > > thakis

[PATCH] D50515: Re-push "[Option] Fix PR37006 prefix choice in findNearest"

thakis added a comment. vitalybuka told me that this should repro things locally: mkdir /tmp/bot cd /tmp/bot svn checkout https://llvm.org/svn/llvm-project/zorg BUILDBOT_CLOBBER= BUILDBOT_REVISION=YOUR_REV zorg/trunk/zorg/buildbot/builders/sanitizers/buildbot_fast.sh 2>&1 | tee

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel created this revision. ymandel added a reviewer: ilya-biryukov. Herald added a project: clang. This revision adds a new kind of rewrite rule, `CompositeRewriteRule`, which composes multiple subrules into a new rule that allows ordered-choice among its subrules. With this feature, users

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

dblaikie added a comment. In D38061#1484568 , @wenlei wrote: > In D38061#1484486 , @dblaikie wrote: > > > Seems like the right thing would be for the DWARF code that wants a > > rendered type name to pass its own

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

wenlei added a comment. In D38061#1484486 , @dblaikie wrote: > Seems like the right thing would be for the DWARF code that wants a rendered > type name to pass its own printing policy, rather than changing some > relatively global one. > > (though also

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

dblaikie added a comment. Seems like the right thing would be for the DWARF code that wants a rendered type name to pass its own printing policy, rather than changing some relatively global one. (though also I have my doubts about the whole approach - macro expansion can change the line

[PATCH] D59264: [Driver] Support compiler-rt crtbegin.o/crtend.o for Linux

This revision was automatically updated to reflect the committed changes. Closed by commit rL359603: [Driver] Support compiler-rt crtbegin.o/crtend.o for Linux (authored by phosek, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

r359603 - [Driver] Support compiler-rt crtbegin.o/crtend.o for Linux

Author: phosek Date: Tue Apr 30 12:35:14 2019 New Revision: 359603 URL: http://llvm.org/viewvc/llvm-project?rev=359603=rev Log: [Driver] Support compiler-rt crtbegin.o/crtend.o for Linux When compiler-rt is selected as the runtime library for Linux targets use its crtbegin.o/crtend.o

[PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

martong marked 2 inline comments as done. martong added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c:8 int b; -} FILE; +} MYFILE; In TestCmodules.py we have `import Darwin` and then `expr *fopen("/dev/zero",

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

MaskRay added inline comments. Comment at: clang-tools-extra/trunk/clangd/unittests/lit.cfg.in:1 +@LIT_SITE_CFG_IN_HEADER@ +# This is a shim to run the gtest unittests in ../unittests using lit. thakis wrote: > thakis wrote: > > Every other LLVM project puts the

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

jdenny added a comment. In D59168#1480801 , @phosek wrote: > In D59168#1480428 , @jdenny wrote: > > > It seems we have roughly the same situation for their ABIs. That is, for > > .so files, someone noted that

[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

thakis added a comment. This breaks a test: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/12112/steps/check-llvm%20check-clang%20stage3%2Fmsan/logs/stdio [--] 1 test from TransformerTest [ RUN ] TransformerTest.NodePartNameDeclRefFailure

[PATCH] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

leonardchan updated this revision to Diff 197416. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51329/new/ https://reviews.llvm.org/D51329 Files: clang/include/clang/AST/ASTContext.h clang/include/clang/AST/RecursiveASTVisitor.h

[PATCH] D61316: [clangd] Improvements to header mapping: more precise parsing of cppreference symbol pages.

sammccall updated this revision to Diff 197350. sammccall added a comment. fix std::chrono regression Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61316/new/ https://reviews.llvm.org/D61316 Files: clangd/StdSymbolMap.inc

[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Thanks! LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61015/new/ https://reviews.llvm.org/D61015

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

Tyker added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:2033 + + void setExplicitSpecifier(ExplicitSpecInfo ESI); + Tyker wrote: > Tyker wrote: > > rsmith wrote: > > > Generally we don't want to have setters in the AST; the AST is intended >

[PATCH] D50515: Re-push "[Option] Fix PR37006 prefix choice in findNearest"

thakis added a comment. Herald added a project: LLVM. I re-landed this to get access to the msan output, since the link above has long expired. Here's the new link: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/31821/steps/check-clang%20msan/logs/stdio Sadly, the bot

r359578 - [LibTooling] Fix broken test after r359574.

Author: ymandel Date: Tue Apr 30 10:24:36 2019 New Revision: 359578 URL: http://llvm.org/viewvc/llvm-project?rev=359578=rev Log: [LibTooling] Fix broken test after r359574. r359574 changed the way that failures are reported, which broke the test TransformerTest.NodePartNameDeclRefFailure which

r359594 - AMDGPU: Enable _Float16

Author: yaxunl Date: Tue Apr 30 11:35:37 2019 New Revision: 359594 URL: http://llvm.org/viewvc/llvm-project?rev=359594=rev Log: AMDGPU: Enable _Float16 Added: cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp Modified:

[PATCH] D61220: lib/Header: Fix Visual Studio builds try #2

smeenai accepted this revision. smeenai added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61220/new/ https://reviews.llvm.org/D61220 ___ cfe-commits mailing list

[PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

martong created this revision. martong added reviewers: shafik, teemperor, jingham, clayborg, a_sidorin. Herald added subscribers: lldb-commits, cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added projects: clang, LLDB. With LLDB we use

[PATCH] D61318: [Sema] Prevent binding references with mismatching address spaces to temporaries

Anastasia created this revision. Anastasia added reviewers: rjmccall, ebevhan. This is a straw-man idea for solving binding references with address spaces to temporaries. If the logic below makes sense I will apply it elsewhere in similar checks. Arbitrary address space references can't be

[PATCH] D61324: Make check-clang depend on the clang-check binary always

thakis created this revision. thakis added a reviewer: rnk. Herald added subscribers: llvm-commits, mgorny. Herald added a project: LLVM. check-clang (the target that runs all clang tests) used to only depend on clang-check (a binary like clang-tidy, clang-refactor, etc) if the static analyzer is

[PATCH] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

leonardchan updated this revision to Diff 197415. leonardchan marked 17 inline comments as done. leonardchan removed a reviewer: martong. Herald added a reviewer: martong. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51329/new/

[PATCH] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

leonardchan added inline comments. Comment at: clang/lib/Parse/ParseDecl.cpp:257-262 + bool AttrStartIsInMacro = + (StartLoc.isMacroID() && Lexer::isAtStartOfMacroExpansion( + StartLoc, SrcMgr, PP.getLangOpts())); + bool

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

rjmccall added a comment. IIRC, C says *members* are initialized as if they were in static storage, which might mean that their interior padding should be zeroed, but I don't think says anything about the padding in the enclosing aggregate. But I think zero-initializing padding is probably

[PATCH] D61142: Set LoopInterleaved in the PassManagerBuilder.

This revision was automatically updated to reflect the committed changes. Closed by commit rC359616: Set LoopInterleaved in the PassManagerBuilder. (authored by asbirlea, committed by ). Changed prior to commit: https://reviews.llvm.org/D61142?vs=196696=197445#toc Repository: rC Clang

r359623 - Remove two unnecessary wrappers of canPassInRegisters

Author: rnk Date: Tue Apr 30 15:23:20 2019 New Revision: 359623 URL: http://llvm.org/viewvc/llvm-project?rev=359623=rev Log: Remove two unnecessary wrappers of canPassInRegisters These extra layers aren't necessary. Modified: cfe/trunk/lib/CodeGen/CGCXXABI.cpp

[PATCH] D61357: SemaOverload: Complete candidates before emitting the error, to ensure diagnostics emitted (or suppressed) during completion don't interfere with the overload notes

dblaikie created this revision. dblaikie added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. Because diagnostics and their notes are not connected at the API level, if the error message for an overload is emitted, then the overload candidates are

[PATCH] D61357: SemaOverload: Complete candidates before emitting the error, to ensure diagnostics emitted (or suppressed) during completion don't interfere with the overload notes

dblaikie added a comment. Oh, @rsmith - if there's any better/different testing (if you can figure out how to reduce the test case down further, now that we know the cause - or if you'd like testing for other codepaths I've touched in this patch) I'm all ears. (also naming/API wrangling - my

[PATCH] D60974: Clang IFSO driver action.

jakehehrlich added a comment. The linker doesn't look at section permissions. In fact the only thing it even looks at the section index for is to check for alignment for copy relocations which is something most people don't even use. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D60974: Clang IFSO driver action.

plotfi added a comment. In D60974#1485611 , @jakehehrlich wrote: > Oh and "ifo" is a good name. I still would like to keep the clang flag as -emit-ifso; the intermediate unmerged files are analogous to object files but the final result should be

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

richard.townsend.arm added inline comments. Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1106 + +FI.getReturnInfo().setInReg(isAArch64 && !IsSizeGreaterThan128(RD)); I'm not sure what the IsSizeGreaterThan128 check is doing here - if the return type is

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

rnk added inline comments. Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:55-56 + bool passClassIndirect(const CXXRecordDecl *RD) const { +return !canCopyArgument(RD); + } These are both trivial wrappers for `RD->canPassInRegisters()` which has the real

[PATCH] D60974: Clang IFSO driver action.

plotfi added a comment. In D60974#1484744 , @jakehehrlich wrote: > > Does llvm-elfabi consume your proposed Schema format? Has it landed yet? > > No, I just proposed it and explained my reasoning. If you wanted to add this > format to TextAPI that would

[PATCH] D61274: [Sema][AST] Explicit visibility for OpenCL/CUDA kernels/variables

yaxunl added inline comments. Comment at: lib/AST/Decl.cpp:738 + (isa(D) && D->hasAttr()) || + (isa(D) && D->hasAttr())) { +Visibility Vis = LV.getVisibility(); we also need this for `__constant__` variables. Repository: rC Clang CHANGES SINCE

[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

hintonda updated this revision to Diff 197466. hintonda added a comment. - Refactor and fix additional dashes in error messages and tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61269/new/ https://reviews.llvm.org/D61269 Files:

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

axzhang added a comment. Pinging for visibility. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55044/new/ https://reviews.llvm.org/D55044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D59924: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

rjmccall added a subscriber: chandlerc. rjmccall added a comment. @chandlerc, I hate to do this to you, but licensing question. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59924/new/ https://reviews.llvm.org/D59924

[PATCH] D61350: [clang-tidy] New check calling out uses of +new in Objective-C code

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp:29 + +bool isMessageExpressionInsideMacro(const ObjCMessageExpr *Expr) { + SourceLocation ReceiverLocation = Expr->getReceiverRange().getBegin();

[PATCH] D61338: [WebAssembly] Use the "wasm32-wasi" triple in tests

sunfish created this revision. sunfish added reviewers: dschuff, sbc100, aheejin. Herald added a subscriber: jgravelle-google. Herald added a project: clang. Similar to https://reviews.llvm.org/D61334, update clang tests to use the "wasm32-wasi" triple, removing the "-musl" environment and

[PATCH] D60930: [codeview] Fix symbol names for dynamic initializers and atexit stubs

rjmccall added a comment. In Swift we basically shove *everything* through our corresponding linkage-entity structure, so I'm not opposed to the basic idea. That said, it's unfortunate that this needs to be raised to the AST level. Also, you have really been contributing to this project far

[PATCH] D61274: [Sema][AST] Explicit visibility for OpenCL/CUDA kernels/variables

rjmccall added a comment. Okay. So it sounds like this should either be a device-only rule, with no warning in mixed-mode languages like CUDA, or we should take a different approach. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61274/new/

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

mgrang updated this revision to Diff 197436. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60349/new/ https://reviews.llvm.org/D60349 Files: include/clang/AST/DeclCXX.h include/clang/CodeGen/CGFunctionInfo.h lib/CodeGen/CGCall.cpp lib/CodeGen/MicrosoftCXXABI.cpp

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

xbolva00 added inline comments. Comment at: clang/include/clang/AST/DeclBase.h:1534 /// 25 bits to fit in the remaining availible space. /// Note that this makes CXXConstructorDeclBitfields take Small typo + update comment CHANGES SINCE LAST ACTION

[PATCH] D61147: [Sema][ObjC] Disable -Wunused-parameter for ObjC methods

erik.pilkington accepted this revision. erik.pilkington 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/D61147/new/ https://reviews.llvm.org/D61147

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

Rakete added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:2579 +assert( +!ES.getExpr() || +CXXConstructorDeclBits.HasTrailingExplicitSpecifier && Your or needs parens or the disambiguation is wrong.

[PATCH] D60974: Clang IFSO driver action.

plotfi added a comment. In D60974#1485417 , @jakehehrlich wrote: > Background: > There are two parts to this. TextAPI is a public interface of textual > representations for these sorts of files that already exists in llvm. There > exist ELF and MachO

[PATCH] D60974: Clang IFSO driver action.

plotfi added a comment. In D60974#1485552 , @jakehehrlich wrote: > Well I think it should be a seperate tool but because the code inside > llvm-elfabi isn't a library yet (though it is written to be used like one) we > can do that by adding a symlink

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land. LGTM, I think this makes sense. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61280/new/ https://reviews.llvm.org/D61280

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

rjmccall added a comment. I don't think the implication is supposed to be that padding is zero-initialized or not depending on where in the aggregate it appears, but it doesn't really matter, I don't think we're arguing about the goal of this patch. Repository: rC Clang CHANGES SINCE LAST

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

rsmith added a comment. I'd like to understand more about the intended use cases of this functionality. What information do clients want? Comment at: clang/lib/Lex/Preprocessor.cpp:870-900 + TokenSource Source; do { +Source = TokenSource(); + switch

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

rsmith added a comment. This is great, thank you so much! Comment at: clang/include/clang/AST/DeclBase.h:1539-1541 +uint64_t NumCtorInitializers : 64 - NumDeclContextBits - +NumFunctionDeclBits - +/*Other used bits in CXXConstructorDecl*/ 3;

[PATCH] D61345: Allow 'CodeGenObjC/illegal-UTF8.m' test for 32-bit targets.

vvereschaka created this revision. vvereschaka added a project: clang. Herald added subscribers: cfe-commits, kristof.beyls, javed.absar. The 'CodeGenObjC/illegal-UTF8.m' get failed with Clang built with 32-bit targets only (as example ARM-only) with the following error: error: unable to create

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

efriedma added inline comments. Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1106 + +FI.getReturnInfo().setInReg(isAArch64 && !IsSizeGreaterThan128(RD)); richard.townsend.arm wrote: > I'm not sure what the IsSizeGreaterThan128 check is doing here - if the

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

efriedma added inline comments. Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1106 + +FI.getReturnInfo().setInReg(isAArch64 && !IsSizeGreaterThan128(RD)); richard.townsend.arm wrote: > efriedma wrote: > > richard.townsend.arm wrote: > > > I'm not sure what

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

vsapsai added a comment. Having in bitcode something like `@llvm.x86.encls.64` is better than inline assembly because we understand the meaning of the bitcode while we don't parse assembly and have a very limited understanding of what it is doing. The use case we need to support is basically

[PATCH] D61349: [clangd] Standard library mapping: prefer "primary" versions of functions over variants.

sammccall created this revision. sammccall added a reviewer: kadircet. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D61349 Files: clangd/StdSymbolMap.inc

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

rjmccall added a comment. Hmm. You know, there's another case where the destructor can be called even for a non-array: if constructing the object requires a temporary, I believe an exception thrown from that temporary's destructor is supposed to go back and destroy the variable. (This is,

[PATCH] D60974: Clang IFSO driver action.

plotfi added a comment. In D60974#1483479 , @jakehehrlich wrote: > In D60974#1483380 , @plotfi wrote: > > > Also, curious on the lack of denoting the sections and which symbols go in > > which sections? Do you

r359628 - Variable auto-init: don't initialize aggregate padding of all aggregates

Author: jfb Date: Tue Apr 30 15:56:53 2019 New Revision: 359628 URL: http://llvm.org/viewvc/llvm-project?rev=359628=rev Log: Variable auto-init: don't initialize aggregate padding of all aggregates Summary: C guarantees that brace-init with fewer initializers than members in the aggregate will

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

jfb added a comment. Follow-up test fix in http://llvm.org/r359636 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61280/new/ https://reviews.llvm.org/D61280 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D59924: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

jsji added inline comments. Comment at: lib/Headers/ppc_wrappers/mmintrin.h:3 + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal jsji wrote: >

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

richard.townsend.arm added inline comments. Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1106 + +FI.getReturnInfo().setInReg(isAArch64 && !IsSizeGreaterThan128(RD)); efriedma wrote: > richard.townsend.arm wrote: > > I'm not sure what the

[PATCH] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

leonardchan updated this revision to Diff 197454. leonardchan marked 2 inline comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51329/new/ https://reviews.llvm.org/D51329 Files: clang/include/clang/AST/ASTContext.h

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

erik.pilkington updated this revision to Diff 197453. erik.pilkington added a comment. Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61165/new/ https://reviews.llvm.org/D61165 Files: clang/include/clang/Basic/AttrDocs.td clang/lib/CodeGen/CGDeclCXX.cpp

[PATCH] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

leonardchan added inline comments. Comment at: clang/lib/Parse/ParseDecl.cpp:257-262 + bool AttrStartIsInMacro = + (StartLoc.isMacroID() && Lexer::isAtStartOfMacroExpansion( + StartLoc, SrcMgr, PP.getLangOpts())); + bool

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

This revision was automatically updated to reflect the committed changes. Closed by commit rC359628: Variable auto-init: dont initialize aggregate padding of all aggregates (authored by jfb, committed by ). Changed prior to commit: https://reviews.llvm.org/D61280?vs=197188=197476#toc

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

jfb added a comment. This seems uncontroversial, I'm happy to make follow-up change if you have suggestions. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61280/new/ https://reviews.llvm.org/D61280 ___ cfe-commits

[PATCH] D61147: [Sema][ObjC] Disable -Wunused-parameter for ObjC methods

ahatanak updated this revision to Diff 197469. ahatanak retitled this revision from "[Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods" to "[Sema][ObjC] Disable -Wunused-parameter for ObjC methods". ahatanak edited the summary of this

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

erik.pilkington updated this revision to Diff 197477. erik.pilkington added a comment. Add a try/catch block to the docs. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61165/new/ https://reviews.llvm.org/D61165 Files: clang/include/clang/Basic/AttrDocs.td

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

richard.townsend.arm added inline comments. Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1106 + +FI.getReturnInfo().setInReg(isAArch64 && !IsSizeGreaterThan128(RD)); efriedma wrote: > richard.townsend.arm wrote: > > efriedma wrote: > > >

[PATCH] D61338: [WebAssembly] Use the "wasm32-wasi" triple in tests

This revision was automatically updated to reflect the committed changes. Closed by commit rC359630: [WebAssembly] Use the wasm32-wasi triple in tests (authored by djg, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61338/new/

[PATCH] D61147: [Sema][ObjC] Disable -Wunused-parameter for ObjC methods

ahatanak added a comment. In D61147#1479932 , @rjmccall wrote: > I would also recommend that you go fix the warning to never fire on virtual > C++ methods. I tried building clang/llvm with -Wunused-parameter turned on and there are lots of places

[PATCH] D61318: [Sema] Prevent binding references with mismatching address spaces to temporaries

rjmccall added inline comments. Comment at: lib/Sema/SemaInit.cpp:4836 + if (T1Quals.hasAddressSpace()) { +if (!T1Quals.isAddressSpaceSupersetOf(cv1T1IgnoreAS.getQualifiers())) { + Sequence.SetFailed( Isn't `cv1T1IgnoreAS.getQualifiers()` always

r359636 - Fix auto-init test

Author: jfb Date: Tue Apr 30 16:27:28 2019 New Revision: 359636 URL: http://llvm.org/viewvc/llvm-project?rev=359636=rev Log: Fix auto-init test r359628 changed the initialization of padding to follow C, but I didn't update the C++ tests. Modified:

[PATCH] D60974: Clang IFSO driver action.

jakehehrlich added a comment. > Wanted to mention, from my testing with yaml2obj, I needed the section flags > (SHF_EXECINSTR, SHF_WRITE, SHF_ALLOC, etc) otherwise nm was spitting the > wrong thing. How are you going to handle those? I'm not sure what you mean. yaml2obj is a different tool

  1   2   >