[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

2018-11-07 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Sorry, had a leftover draft which I forgot to clean up. Edited in Phabricator now. Repository: rC Clang https://reviews.llvm.org/D52674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

2018-11-09 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In https://reviews.llvm.org/D52674#1293180, @rjmccall wrote: > In https://reviews.llvm.org/D52674#1291284, @smeenai wrote: > > > In https://reviews.llvm.org/D52674#1281747, @rjmccall wrote: > > > > > In https://reviews.llvm.org/D52674#1271814, @smeenai wrote: > > > > > >

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

2018-11-13 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai updated this revision to Diff 173965. smeenai added a comment. Stateful approach Repository: rC Clang https://reviews.llvm.org/D52674 Files: lib/AST/MicrosoftMangle.cpp test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm Index: test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

2018-11-13 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. @rnk pointed out on IRC that the MicrosoftCXXNameMangler is actually specifically designed to manage the mangling of only a single name, in which case adding state to it for handling RTTI seems like a natural approach. @rjmccall, what do you think? I think this is much

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

2018-11-13 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In https://reviews.llvm.org/D52674#1297879, @rjmccall wrote: > I'm not worried about the mangler being re-used for multiple declarations, > I'm worried about a global flag changing how we mangle all components of a > type when we only mean to change it at the top level.

[PATCH] D54536: [AST] Fix typo in MicrosoftMangle

2018-11-14 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai created this revision. smeenai added reviewers: majnemer, rnk. I noticed that we were spelling it as Artifical instead of Artificial when working on the mangling for Obj-C RTTI. I'm putting this up for review instead of committing it directly because I'm not 100% sure what the policy here

[PATCH] D54536: [AST] Fix typo in MicrosoftMangle

2018-11-14 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC346882: [AST] Fix typo in MicrosoftMangle (authored by smeenai, committed by ). Changed prior to commit: https://reviews.llvm.org/D54536?vs=174064&id=174075#toc Repository: rC Clang https://reviews.

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

2018-11-15 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai planned changes to this revision. smeenai added a comment. In https://reviews.llvm.org/D52674#1298115, @rjmccall wrote: > In https://reviews.llvm.org/D52674#1297893, @smeenai wrote: > > > In https://reviews.llvm.org/D52674#1297879, @rjmccall wrote: > > > > > I'm not worried about the mang

[PATCH] D52956: Support `-fno-visibility-inlines-hidden`

2019-02-25 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. I tried committing this for you, but it doesn't apply to master. Could you rebase? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52956/new/ https://reviews.llvm.org/D52956 ___ cfe-commits ma

[PATCH] D58537: lib/Header: Simplify CMakeLists.txt

2019-02-27 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai accepted this revision. smeenai added a comment. LGTM, thanks! Comment at: clang/lib/Headers/CMakeLists.txt:136 list(APPEND out_files ${dst}) + # The list function only updates out_files in the current scope. We need + # call set in order to also update the variab

[PATCH] D58791: [build] Rename clang-headers to clang-resource-headers

2019-02-28 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai created this revision. smeenai added reviewers: beanz, phosek, tstellar, rnk, dim. Herald added subscribers: openmp-commits, Sanitizers, jdoerfert, javed.absar, mgorny. Herald added a reviewer: serge-sans-paille. Herald added projects: clang, Sanitizers, LLDB, OpenMP, LLVM. The current in

[PATCH] D58537: lib/Header: Simplify CMakeLists.txt

2019-03-01 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. @tstellar are you planning to land this soon? It'll conflict with D58791 , but I'm not planning to land that for another few days, so I can rebase on top of this one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D58862: [cmake] Create exports for umbrella library targets

2019-03-01 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai created this revision. smeenai added reviewers: beanz, phosek. Herald added a subscriber: mgorny. Herald added projects: clang, LLVM. When using the umbrella llvm-libraries and clang-libraries targets, we should export all library targets, otherwise they'll be part of our distribution but

[PATCH] D58791: [build] Rename clang-headers to clang-resource-headers

2019-03-01 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai updated this revision to Diff 188988. smeenai added a comment. Release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58791/new/ https://reviews.llvm.org/D58791 Files: clang-tools-extra/clang-tidy/tool/CMakeLists.txt clang-tools-e

[PATCH] D58791: [build] Rename clang-headers to clang-resource-headers

2019-03-01 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. I added an entry to the release notes. The release notes change only covers the changes in this diff. My plan is to land this, wait for a few days (so that users of the target get broken in an obvious way), and then add the new install-clang-headers target (for installi

[PATCH] D58791: [build] Rename clang-headers to clang-resource-headers

2019-03-01 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In D58791#1415496 , @tstellar wrote: > This looks good, but could you also add an entry in the ReleaseNotes for this. Will do – thanks for the reminder :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D58791: [build] Rename clang-headers to clang-resource-headers

2019-03-01 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai updated this revision to Diff 189014. smeenai added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58791/new/ https://reviews.llvm.org/D58791 Files: clang-tools-extra/clang-tidy/tool/CMakeLists.txt clang-tools-extra/te

[PATCH] D58751: Order File Instrumentation: add clang support for -forder-file-instrumentation

2019-03-04 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:5312 + CmdArgs.push_back("-forder-file-instrumentation"); + // Enable order file instrumentation when ThinLTO is not on. When ThinLTO is + // on, we need to pass these flags as linker flags an

[PATCH] D58317: [clang] Add install targets for API headers

2019-03-04 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai updated this revision to Diff 189199. smeenai retitled this revision from "[clang] Add install targets for library headers" to "[clang] Add install targets for API headers". smeenai edited the summary of this revision. smeenai added a comment. Update target name Repository: rG LLVM Gi

[PATCH] D58317: [clang] Add install targets for API headers

2019-03-04 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. @phosek This is using the clang-headers name now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58317/new/ https://reviews.llvm.org/D58317 ___ cfe-commits mailing list cfe-comm

[PATCH] D58862: [cmake] Create exports for umbrella library targets

2019-03-04 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL355354: [cmake] Create exports for umbrella library targets (authored by smeenai, committed by ). Changed prior to commit: https://reviews.llvm.org/D58862?vs=189019&id=189229#toc Repository: rL LLVM

[PATCH] D58791: [build] Rename clang-headers to clang-resource-headers

2019-03-05 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL355340: [build] Rename clang-headers to clang-resource-headers (authored by smeenai, committed by ). Herald added a subscriber: delcypher. Changed prior to commit: https://reviews.llvm.org/D58791?vs=189

[PATCH] D59013: [CMake][runtimes] Move libunwind, libc++abi and libc++ to lib/ and include/

2019-03-06 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:89 + P.assign(D.Dir); + llvm::sys::path::append(P, "..", "lib", D.getTargetTriple(), "lib"); + if (getVFS().exists(P)) Is this supposed to append lib twice? Comment at

[PATCH] D59013: [CMake][runtimes] Move libunwind, libc++abi and libc++ to lib/ and include/

2019-03-06 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments. Comment at: clang/test/Driver/linux-per-target-runtime-dir.c:13 // CHECK-PER-TARGET-RUNTIME: "-isysroot" "[[SYSROOT:[^"]+]]" // CHECK-PER-TARGET-RUNTIME: "-internal-isystem" "[[RESDIR]]/include/c++/v1" // CHECK-PER-TARGET-RUNTIME: "-internal-isys

[PATCH] D59013: [CMake][runtimes] Move libunwind, libc++abi and libc++ to lib/ and include/

2019-03-07 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: libcxx/CMakeLists.txt:420-421 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) - set(DEFAULT_INSTALL_PREFIX lib${LLVM_LIBDIR_SUFFIX}/clang/

[PATCH] D58921: [CMake] Tell libc++ that we're using compiler-rt on Apple platforms

2019-03-08 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Is this to work around https://github.com/llvm/llvm-project/blob/f84083b4dbb1ddb6d2783400f11121f490cdb5a8/libcxx/lib/CMakeLists.txt#L312? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58921/new/ https://reviews.llvm.org/D58921 ___

[PATCH] D58317: [clang] Add install targets for API headers

2019-03-08 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Ping (although I'm not planning to land this until Monday). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58317/new/ https://reviews.llvm.org/D58317 ___ cfe-commits mailing lis

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

2019-03-08 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments. Comment at: clang/test/Driver/linux-per-target-runtime-dir.c:15 // CHECK-PER-TARGET-RUNTIME: "--sysroot=[[SYSROOT]]" +// CHECK-PER-TARGET-RUNTIME: "-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-linux-gnu" // CHECK-PER-TARGET-RUNTIME: "-L[[RE

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

2019-03-08 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments. Comment at: clang/test/Driver/linux-per-target-runtime-dir.c:15 // CHECK-PER-TARGET-RUNTIME: "--sysroot=[[SYSROOT]]" +// CHECK-PER-TARGET-RUNTIME: "-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-linux-gnu" // CHECK-PER-TARGET-RUNTIME: "-L[[RE

[PATCH] D58317: [clang] Add install targets for API headers

2019-03-11 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL355853: [clang] Add install targets for API headers (authored by smeenai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.

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

2019-03-11 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM, though you may wanna wait for @jdenny too. In D59168#1423587 , @phosek wrote: > The layout currently looks as follows: > > compiler-rt: >

[PATCH] D59797: [COFF] Reorder fields in Chunk and SectionChunk to reduce their size

2019-03-25 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments. Comment at: lld/COFF/Chunks.cpp:47 +namespace { +// This class exists just for the purpose of calculating the expected size of pcc wrote: > rnk wrote: > > ruiu wrote: > > > rnk wrote: > > > > ruiu wrote: > > > > > rnk wrote: > > >

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-27 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Do you need someone to commit this for you? Comment at: clang/test/CodeGenObjC/nontrivial-c-struct-within-struct-name.m:1 +// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks -fobjc-runtime=ios-11.0 -emit-llvm -o - -DUSESTRUCT -I %S/Input

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-27 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Also, it might be worth adding a third level of struct to the test, to show that it handles arbitrary nesting correctly (which it does). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59873/new/ https://reviews.llvm.org/D59

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-28 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Looks good. I'll commit this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59873/new/ https://reviews.llvm.org/D59873 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-28 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL357184: [CodeGen] Add additional mangling for struct members of non trivial structs (authored by smeenai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pri

[PATCH] D58537: lib/Header: Simplify CMakeLists.txt

2019-04-22 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. @tstellar ping. Someone appears to be running into this on the CMake mailing list too: https://cmake.org/pipermail/cmake/2019-April/069359.html Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58537/new/ https://reviews.llvm.org/D58537 __

[PATCH] D61054: lib/Header: Fix Visual Studio builds

2019-04-24 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM This would break if someone does a debug and release build simultaneously, the copies occur simultaneously and the filesystem gets unhappy, That seems fairly contrived though, and this

[PATCH] D61118: [MinGW] Fix dllexport of explicit template instantiation

2019-04-25 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. LGTM, though I'd wait for Hans and/or Reid too. I like the GCC/MinGW behavior here much more than the MSVC behavior. Having to mark the definitions only in the explicit instantiations case (for MSVC) whereas everything else requires you to mark the declarations is a wei

[PATCH] D61121: [Windows] Separate elements in -print-search-dirs with semicolons

2019-04-25 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. I feel something like `llvm::sys::path::PathListSeparator` would be nicer, since AFAIK any path list should be separated with semicolons on Windows and colons everywhere else. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61121/new/ htt

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

2019-04-30 Thread Shoaib Meenai via Phabricator via cfe-commits
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 cfe-commits@lists.llv

[PATCH] D49587: [CMake] Support exporting runtimes using the CMake export

2019-05-06 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments. Comment at: libunwind/src/CMakeLists.txt:160 if (LIBUNWIND_INSTALL_LIBRARY) - install(TARGETS ${LIBUNWIND_INSTALL_TARGETS} + if(libcxxabi IN_LIST LLVM_RUNTIME_DISTRIBUTION_COMPONENTS) +set(export_to_llvmruntimes EXPORT LLVMRuntimes) -

[PATCH] D61615: [COFF] Use COFF stubs for extern_weak functions

2019-05-07 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai accepted this revision. smeenai added a comment. LGTM, neat! Comment at: llvm/test/CodeGen/X86/extern_weak.ll:20-21 +; DARWIN-LABEL: _bar: +; DARWIN: cmpl $0, L_foo$non_lazy_ptr +; DARWIN: jmp _foo ## TAILCALL + I'm not parsing this too well (it's not q

[PATCH] D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`.

2018-09-21 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In https://reviews.llvm.org/D52344#1242530, @rjmccall wrote: > In https://reviews.llvm.org/D52344#1242451, @kristina wrote: > > > Would you be okay with me renaming `cfstring` to `.cfstring` for ELF so > > it's in line with ELF section naming conventions (I'm not entirel

[PATCH] D52581: [AST] Revert mangling changes from r339428

2018-09-26 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai created this revision. smeenai added reviewers: compnerd, rjmccall, theraven, DHowett-MSFT. As discussed in https://reviews.llvm.org/D50144, we want Obj-C classes to have the same mangling as C++ structs, to support headers like the following: #ifdef __OBJC__ @class I; #else struc

[PATCH] D52581: [AST] Revert mangling changes from r339428

2018-09-27 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In https://reviews.llvm.org/D52581#1247409, @theraven wrote: > > I would have done the same for the GNUstep RTTI here, except I don't > > actually > > see the code for that anywhere, and no tests seem to break either, so I > > believe it's not upstreamed yet. > > I'm n

[PATCH] D52581: [AST] Revert mangling changes from r339428

2018-09-27 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a subscriber: rnk. smeenai added a comment. Adding @rnk, since this'll touch MS ABI mangling. For context, we want `struct X` to have the same mangling as `@interface X` normally, but we want to be able to distinguish them for the purpose of exception handling. See this diff's sum

[PATCH] D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types

2018-09-27 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments. Comment at: lib/CodeGen/CGObjCMac.cpp:7457-7460 CGObjCNonFragileABIMac::GetEHType(QualType T) { // There's a particular fixed type info for 'id'. if (T->isObjCIdType() || T->isObjCQualifiedIdType()) { +if (CGM.getTriple().isWindowsMSVCEnv

[PATCH] D51714: CMake: Deprecate using llvm-config to detect llvm installation

2018-09-27 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Is there anything holding this up? Repository: rC Clang https://reviews.llvm.org/D51714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52664: Update CMakeLists.txt snippet so that example compiles

2018-09-28 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Yep, that's my doing (https://reviews.llvm.org/rL319840). I didn't think about the documentation, whoops. Repository: rC Clang https://reviews.llvm.org/D52664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

2018-09-28 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai created this revision. smeenai added reviewers: compnerd, DHowett-MSFT, rjmccall, rnk, theraven. Herald added a subscriber: erik.pilkington. Obj-C classes are mangled as C++ structs with the same name (in both the Itanium and the Microsoft ABIs), but we want to be able to distinguish them

[PATCH] D52581: [AST] Revert mangling changes from r339428

2018-09-28 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. https://reviews.llvm.org/D52674 adds the RTTI Obj-C discriminator. I put it up as a separate change so it could be reviewed independently and more thoroughly. Repository: rC Clang https://reviews.llvm.org/D52581 ___ cfe-

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

2018-09-28 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai updated this revision to Diff 167555. smeenai added a comment. arc fail Repository: rC Clang https://reviews.llvm.org/D52674 Files: lib/AST/MicrosoftMangle.cpp test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm Index: test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm ===

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

2018-10-01 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In https://reviews.llvm.org/D52674#1251419, @rjmccall wrote: > Conceptually this seems fine, but I think it would be good to stop and make > sure we're using a consistent style when mangling extensions. Currently it > feels like every patch to add a Clang extension to

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

2018-10-02 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai planned changes to this revision. smeenai added a comment. In https://reviews.llvm.org/D52674#1251931, @rjmccall wrote: > In https://reviews.llvm.org/D52674#1251439, @smeenai wrote: > > > In https://reviews.llvm.org/D52674#1251419, @rjmccall wrote: > > > > > Conceptually this seems fine,

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

2018-10-02 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai requested review of this revision. smeenai added a comment. Actually, I take that back ... I just misread the stack trace. There are a bunch of hops between the `mangleCXXRTTI` call and the ultimate mangling function: MicrosoftMangleContextImpl::mangleCXXRTTI(QualType, raw_ostream &)

[PATCH] D52843: Update Clang Windows getting started docs

2018-10-03 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments. Comment at: clang/www/get_started.html:197 +If you are using Visual Studio 2017: + cmake -G "Visual Studio 15 2017 Win64" ..\llvm +This will generate x64 binaries by default, which should perform better. Does the Win64

[PATCH] D52581: [AST] Revert mangling changes from r339428

2018-10-04 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC343808: [AST] Revert mangling changes from r339428 (authored by smeenai, committed by ). Changed prior to commit: https://reviews.llvm.org/D52581?vs=167219&id=168354#toc Repository: rC Clang https:/

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

2018-10-04 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In https://reviews.llvm.org/D52674#1253408, @rjmccall wrote: > In https://reviews.llvm.org/D52674#1253401, @smeenai wrote: > > > Actually, I take that back ... I just misread the stack trace. > > > > There are a bunch of hops between the `mangleCXXRTTI` call and the ultim

[PATCH] D52990: [MinGW] Allow using ubsan

2018-10-08 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments. Comment at: lib/Driver/ToolChains/MinGW.cpp:266 + // directives in the object files, but the static library needs + // -lpsapi unless the sanitizer was built targeting >= win7. + CmdArgs.push_back("-lpsapi"); -

[PATCH] D52990: [MinGW] Allow using ubsan

2018-10-08 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments. Comment at: lib/Driver/ToolChains/MinGW.cpp:266 + // directives in the object files, but the static library needs + // -lpsapi unless the sanitizer was built targeting >= win7. + CmdArgs.push_back("-lpsapi"); -

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

2019-02-01 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added subscribers: hans, smeenai. smeenai added reviewers: rnk, efriedma. smeenai added a comment. +@hans for the release branch. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57636/new/ https://reviews.llvm.org/D57636 _

[PATCH] D58204: CMake: Fix stand-alone clang builds since r353268

2019-02-13 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments. Comment at: clang/lib/Basic/CMakeLists.txt:13 -set(generate_vcs_version_script "${LLVM_CMAKE_PATH}/GenerateVersionFromVCS.cmake") +set(generate_vcs_version_script "${LLVM_CMAKE_DIR}/GenerateVersionFromVCS.cmake") Will this wor

[PATCH] D58204: CMake: Fix stand-alone clang builds since r353268

2019-02-13 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. This could also be improved by teaching clang to use LLVM_BUILD_MAIN_SRC_DIR when that's available. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58204/new/ https://reviews.llvm.org/D58204 __

[PATCH] D58204: CMake: Fix stand-alone clang builds since r353268

2019-02-13 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai accepted this revision. smeenai added a comment. LGTM. There's more clean up that could be done here, but this addresses the immediate issue. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58204/new/ https://reviews.llvm.org/D58204

[PATCH] D58268: [clang] Create install targets for non-shared libraries

2019-02-14 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai created this revision. smeenai added reviewers: beanz, phosek. Herald added a subscriber: mgorny. Herald added a project: clang. I don't see a reason for these to not have install targets created, which in turn allows them to be bundled in distributions. This doesn't affect the "install" t

[PATCH] D58269: [clang] Add build and install targets for clang libraries

2019-02-14 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai created this revision. smeenai added reviewers: beanz, phosek. Herald added a subscriber: mgorny. Herald added a project: clang. This is modeled after the existing llvm-libraries target. It's a convenient way to include all clang libraries in a distribution. This differs slightly from the

[PATCH] D58269: [clang] Add build and install targets for clang libraries

2019-02-14 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai updated this revision to Diff 186957. smeenai added a comment. Fix condition Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58269/new/ https://reviews.llvm.org/D58269 Files: clang/CMakeLists.txt clang/cmake/modules/AddClang.cmake Inde

[PATCH] D58269: [clang] Add build and install targets for clang libraries

2019-02-14 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. A couple of things I noticed while writing this patch: - clang guards its install target creation with `CMAKE_CONFIGURATION_TYPES`, whereas LLVM uses `LLVM_ENABLE_IDE`. Should clang be switched over to be consistent with LLVM? - I realize we create the `install-clang-li

[PATCH] D58268: [clang] Create install targets for non-shared libraries

2019-02-15 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC354140: [clang] Create install targets for non-shared libraries (authored by smeenai, committed by ). Changed prior to commit: https://reviews.llvm.org/D58268?vs=186955&id=187018#toc Repository: rC C

[PATCH] D58269: [clang] Add build and install targets for clang libraries

2019-02-15 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC354141: [clang] Add build and install targets for clang libraries (authored by smeenai, committed by ). Changed prior to commit: https://reviews.llvm.org/D58269?vs=186957&id=187019#toc Repository: rC

[PATCH] D58284: [clang] Switch to LLVM_ENABLE_IDE

2019-02-15 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai created this revision. smeenai added reviewers: beanz, phosek. Herald added subscribers: jdoerfert, arphaman, mgorny. Herald added a project: clang. r344555 switched LLVM to guarding install targets with LLVM_ENABLE_IDE instead of CMAKE_CONFIGURATION_TYPES, which expresses the intent more

[PATCH] D58284: [clang] Switch to LLVM_ENABLE_IDE

2019-02-15 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a subscriber: lebedev.ri. smeenai added a comment. CC @lebedev.ri, since I believe you raised some issues during the corresponding LLVM change but those were addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58284/new/ https:/

[PATCH] D58284: [clang] Switch to LLVM_ENABLE_IDE

2019-02-15 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In D58284#1399446 , @lebedev.ri wrote: > In D58284#1399443 , @smeenai wrote: > > > CC @lebedev.ri, since I believe you raised some issues during the > > corresponding LLVM change but those

[PATCH] D58317: [clang] Add install targets for development headers

2019-02-15 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. I'm not entirely happy with the name clang-dev-headers, and am open to suggestions. It's unfortunate clang-headers was already taken for something different, but renaming that target or increasing its scope seems bad for existing users. Other possibilities I thought of

[PATCH] D58317: [clang] Add install targets for development headers

2019-02-15 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai created this revision. smeenai added reviewers: beanz, phosek. Herald added subscribers: jdoerfert, mgorny. Herald added a project: clang. smeenai added a comment. I'm not entirely happy with the name clang-dev-headers, and am open to suggestions. It's unfortunate clang-headers was alread

[PATCH] D58480: [clang] Add CMake target for installing clang's CMake exports

2019-02-20 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai created this revision. smeenai added reviewers: beanz, phosek. Herald added a subscriber: mgorny. Herald added a project: clang. smeenai retitled this revision from "[clang] Add CMake target for clang's CMake exports" to "[clang] Add CMake target for installing clang's CMake exports". Thi

[PATCH] D58284: [clang] Switch to LLVM_ENABLE_IDE

2019-02-20 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL354525: [clang] Switch to LLVM_ENABLE_IDE (authored by smeenai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D

[PATCH] D58317: [clang] Add install targets for development headers

2019-02-20 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In D58317#1404961 , @phosek wrote: > In D58317#1400223 , @smeenai wrote: > > > I'm not entirely happy with the name clang-dev-headers, and am open to > > suggestions. It's unfortunate clang

[PATCH] D58480: [clang] Add CMake target for installing clang's CMake exports

2019-02-20 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL354527: [clang] Add CMake target for installing clang's CMake exports (authored by smeenai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D58317: [clang] Add install targets for library headers

2019-02-20 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai updated this revision to Diff 187697. smeenai added a comment. Switch to clang-library-headers pending cfe-dev discussion Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58317/new/ https://reviews.llvm.org/D58317 Files: clang/CMakeLists.tx

[PATCH] D58317: [clang] Add install targets for library headers

2019-02-20 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai updated this revision to Diff 187698. smeenai retitled this revision from "[clang] Add install targets for development headers" to "[clang] Add install targets for library headers". smeenai edited the summary of this revision. smeenai added a comment. Update description Repository: rG

[PATCH] D52956: Support `-fno-visibility-inlines-hidden`

2019-02-21 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. You should ask the reviewers to commit it in your behalf. You can request commit access after you've had a few patches accepted and committed. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52956/new/ https://reviews.llvm.org/D52956 __

[PATCH] D58537: lib/Header: Simplify CMakeLists.txt

2019-02-21 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai accepted this revision. smeenai added reviewers: beanz, phosek. smeenai added a comment. This revision is now accepted and ready to land. LGTM, though I generally prefer functions to macros. Comment at: clang/lib/Headers/CMakeLists.txt:127 -# Generate arm_neon.h -clan

[PATCH] D37182: [libcxx] Special visibility macros for the experimental library

2017-08-29 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added reviewers: mclow.lists, compnerd. smeenai resigned from this revision. smeenai added a comment. It's really cool that you're getting the filesystem library to work on Windows :) This looks reasonable to me; it's the only way I can think of to get the experimental library working o

[PATCH] D28212: typeinfo: provide a partial implementation for Win32

2017-09-12 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai updated this revision to Diff 114957. smeenai added a comment. Rebase https://reviews.llvm.org/D28212 Files: include/typeinfo src/support/runtime/exception_msvc.ipp src/typeinfo.cpp Index: src/typeinfo.cpp === --- sr

[PATCH] D28212: typeinfo: provide a partial implementation for Win32

2017-09-12 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Rebased and tested on Windows again. I'd like to see this patch get through to reduce the dependencies on the `vcruntime` headers, since those end up pulling in lots of cruft and can cause some nasty conflicts. (Similarly, I'll be trying to remove dependencies on `Wind

[PATCH] D36720: [libc++] Prevent stale site configuration headers

2017-09-14 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments. Comment at: CMakeLists.txt:633 + if (EXISTS "${site_config_path}") +file(REMOVE "${site_config_path}") + endif() EricWF wrote: > Maybe print a warning or a message here? While it seems useful to > re-configure and remove the

[PATCH] D36720: [libc++] Prevent stale site configuration headers

2017-09-14 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL313284: [libc++] Prevent stale site configuration headers (authored by smeenai). Changed prior to commit: https://reviews.llvm.org/D36720?vs=111096&id=115250#toc Repository: rL LLVM https://reviews.

[PATCH] D36713: [libc++] Add a persistent way to disable availability

2017-09-14 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. @mclow.lists, any final verdict here? I ended up doing this differently for my internal use case, so if you think this isn't generally useful, I'm happy to abandon. https://reviews.llvm.org/D36713 ___ cfe-commits mailing l

[PATCH] D31363: [libc++] Remove cmake glob for source files

2017-09-14 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai updated this revision to Diff 115259. smeenai added a comment. Herald added a subscriber: fedor.sergeev. Address comments https://reviews.llvm.org/D31363 Files: benchmarks/CMakeLists.txt lib/CMakeLists.txt Index: lib/CMakeLists.txt =

[PATCH] D31363: [libc++] Remove cmake glob for source files

2017-09-14 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. This depends on https://reviews.llvm.org/D37859 for the `SOURCE_DIR` parameter to `llvm_check_source_file_list`. https://reviews.llvm.org/D31363 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D28212: typeinfo: provide a partial implementation for Win32

2017-09-14 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. @compnerd, @EricWF and I discussed this a bit on IRC yesterday. My motivation here is that I'm using libc++ with other headers that clash badly with the vcruntime headers, which prevents me from using libc++'s `_LIBCPP_ABI_MICROSOFT` support. Reducing libc++'s dependenc

[PATCH] D35388: [libc++] Give extern templates default visibility on gcc

2017-09-14 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Good point. It looks like clang actually ignores the attributes in that case as well; it just doesn't warn you about it :D Take a look at https://godbolt.org/g/CJTD6c to see what I mean. Note the `.hidden c::f()` in both the gcc and clang outputs. https://reviews.llvm

[PATCH] D35388: [libc++] Give extern templates default visibility on gcc

2017-09-14 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Filed https://bugs.llvm.org/show_bug.cgi?id=34614 about the silent attribute ignoring. https://reviews.llvm.org/D35388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D28212: typeinfo: provide a partial implementation for Win32

2017-09-14 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added inline comments. Comment at: include/typeinfo:100 +int __compare(const struct type_info &__rhs); +#endif // _LIBCPP_ABI_MICROSOFT + Drop the `struct`; it causes compilation errors. This needs to be marked `const`. Comment at:

[PATCH] D64579: [clang-shlib] Fix clang-shlib for PRIVATE dependencies

2019-07-11 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai created this revision. smeenai added reviewers: beanz, compnerd, phosek. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. Any static library with a PRIVATE dependency ends up with a $ generator expression in its INTERFACE_LINK_LIBRARIES, which won't be evaluate

[PATCH] D64579: [clang-shlib] Fix clang-shlib for PRIVATE dependencies

2019-07-11 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. @beanz, what do you think of this? I think it's kinda awful, but I can't think of a better alternative short of upgrading to CMake 3.12 (as detailed in my comment). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64579/new/

[PATCH] D64579: [clang-shlib] Fix clang-shlib for PRIVATE dependencies

2019-07-11 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In D64579#1581172 , @phosek wrote: > While upgrading to newer CMake would be nice, I think it's unlikely that we > could move all the way to 3.12 since that version was only released a year > ago and still isn't available in most

[PATCH] D64579: [clang-shlib] Fix clang-shlib for PRIVATE dependencies

2019-07-11 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In D64579#1581194 , @smeenai wrote: > In D64579#1581172 , @phosek wrote: > > > While upgrading to newer CMake would be nice, I think it's unlikely that we > > could move all the way to 3.12

<    1   2   3   4   5   6   7   8   >