[PATCH] D42768: AST: add an extension to support SwiftCC on MS ABI

compnerd added a comment. Ah, yeah, I'm just filling in the test cases for those two cases. Repository: rC Clang https://reviews.llvm.org/D42768 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D42768: AST: add an extension to support SwiftCC on MS ABI

compnerd updated this revision to Diff 132325. compnerd added a comment. Handle namespaces properly Repository: rC Clang https://reviews.llvm.org/D42768 Files: lib/AST/MicrosoftMangle.cpp test/CodeGenCXX/msabi-swiftcall-cc.cpp Index: test/CodeGenCXX/msabi-swiftcall-cc.cpp =

[PATCH] D42768: AST: add an extension to support SwiftCC on MS ABI

compnerd updated this revision to Diff 132328. compnerd added a comment. Add test case for non-anonymous non-__Swift::__swift_cc namespace. Repository: rC Clang https://reviews.llvm.org/D42768 Files: lib/AST/MicrosoftMangle.cpp test/CodeGenCXX/msabi-swiftcall-cc.cpp Index: test/CodeGen

[PATCH] D42768: AST: add an extension to support SwiftCC on MS ABI

compnerd updated this revision to Diff 132528. compnerd added a comment. Handle the non-top-level decl case Repository: rC Clang https://reviews.llvm.org/D42768 Files: lib/AST/MicrosoftMangle.cpp test/CodeGenCXX/msabi-swiftcall-cc.cpp Index: test/CodeGenCXX/msabi-swiftcall-cc.cpp ==

[PATCH] D42768: AST: add an extension to support SwiftCC on MS ABI

compnerd added a comment. @rnk thats not a bad idea. However, I had implemented it slightly differently. I mangled it as if it was a PMF: so you get `__Swift::__swift_cc::*` as the type. Repository: rC Clang https://reviews.llvm.org/D42768 __

[PATCH] D42758: Support `#pragma comment(lib, "name")` in the frontend for ELF

compnerd marked 2 inline comments as done. compnerd added a comment. @probinson it would be pretty cool if we could get the PS4 environment to share the same linker options implementation. What other options do you guys need for this to be a viable approach? Repository: rC Clang https://re

[PATCH] D42768: AST: add an extension to support SwiftCC on MS ABI

compnerd added inline comments. Comment at: lib/AST/MicrosoftMangle.cpp:1061 +if (Ty->getAs()->getCallConv() == CC_Swift) + for (const char *NS : {"__swift_cc", "__Swift"}) +mangleSourceName(NS); rsmith wrote: > Do we really need both of these qu

[PATCH] D42768: AST: add an extension to support SwiftCC on MS ABI

compnerd added inline comments. Comment at: lib/AST/MicrosoftMangle.cpp:1717 +Out << '8'; +for (const char *NS : {"__swift_cc", "__Swift"}) + mangleSourceName(NS); erichkeane wrote: > compnerd wrote: > > erichkeane wrote: > > > rsmith wro

[PATCH] D42758: Support `#pragma comment(lib, "name")` in the frontend for ELF

compnerd updated this revision to Diff 132853. compnerd added a comment. clang-format missed line, simplify some checks Repository: rC Clang https://reviews.llvm.org/D42758 Files: lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h lib/Parse/ParsePragma.cpp test/CodeGen/elf-lin

[PATCH] D29868: Recover more gracefully when __declspec is not supported as a keyword

compnerd added inline comments. Comment at: lib/Parse/ParseDecl.cpp:2973 + // recognize that scenario and recover gracefully. + if (!getLangOpts().MicrosoftExt && Tok.is(tok::identifier) && + Tok.getIdentifierInfo()->getName().equals("__declspec")) { --

[PATCH] D29868: Recover more gracefully when __declspec is not supported as a keyword

compnerd added inline comments. Comment at: lib/Parse/ParseDecl.cpp:2989 + + Diag(Loc, diag::err_ms_attributes_not_enabled); + continue; aaron.ballman wrote: > compnerd wrote: > > I think that we want to emit the diagnostic even if there is no p

[PATCH] D29967: Get class property selectors from property decl if it exists

compnerd requested changes to this revision. compnerd added a comment. This revision now requires changes to proceed. Please add a test for this. Comment at: lib/Sema/SemaExprObjC.cpp:1989 + Selector SetterSel; + if (ObjCPropertyDecl *PD = IFace->FindPropertyDeclaration( +

[PATCH] D29967: Get class property selectors from property decl if it exists

compnerd added a comment. I think Im misunderstanding something. How does the test actually test what you are changing? https://reviews.llvm.org/D29967 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/l

[PATCH] D29967: Get class property selectors from property decl if it exists

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. Ah, I had missed the `-verify` option on the test. Yes, that makes sense. Ternary may have flowed the conditional code better. Do you need someone to commit this on your behalf? https

[PATCH] D29967: Get class property selectors from property decl if it exists

compnerd closed this revision. compnerd added a comment. SVN r295683 https://reviews.llvm.org/D29967 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27360: [clang] Fix D26214: Move error handling out of MC and to the callers.

compnerd added a comment. I think that the argument parsing should really be hoisted into the driver rather than pushed down into the assembler. The driver can parse and validate the options before passing them down, so when cc1as gets it, it will simply set the value. The newly introduced di

[PATCH] D27384: [libclang] Restore clang_getNumTemplateArguments/clang_getTemplateArgumentAsType functionality

compnerd added inline comments. Comment at: tools/libclang/CXType.cpp:151-153 + if (A.getKind() != TemplateArgument::Type) +return MakeCXType(QualType(), TU); + return MakeCXType(A.getAsType(), TU); Isn't this more compact as: return MakeCXType(A.getKi

[PATCH] D26949: [libc++abi] Clean up visibility

compnerd added a comment. I really like the clean up this does. It removes the unnecessary usage of the GCC pragmas and cleans up the definitions by using the headers more properly. Comment at: src/abort_message.h:19 -__attribute__((visibility("hidden"), noreturn)) +_LIBCXX

[PATCH] D25417: [libcxxabi] Fix alignment of allocated exceptions in 32 bit builds

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. I thought we had gotten this merged already. Oops. LGTM with the minor tweak for windows. Comment at: src/fallback_malloc.cpp:206 +return fallback_malloc(size); }

[PATCH] D25686: [Driver] Improve support for Gentoo arm*-hardfloat-*-*eabi triples

compnerd added a comment. Please don't canonicalize the triple that way. The behavior of `armv7-unknown-linux-gnueabi` is different from `armv7-unknown-linux-gnueabihf` which is different from `armv7-unknown-linux-gnueabihf -mfloat-abi=hard`. I agree that this is absolutely terrible and unexp

[PATCH] D27360: [clang] Fix D26214: Move error handling out of MC and to the callers.

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. Im not the biggest fan of this. However, having some validation is probably better than not. We can come up with a better way to address this in the future when needed I suppose. https

[PATCH] D32192: Enable leak sanitizer builds for darwin

compnerd requested changes to this revision. compnerd added a comment. This revision now requires changes to proceed. Please add a test case. The change itself looks reasonable. https://reviews.llvm.org/D32192 ___ cfe-commits mailing list cfe-commi

[PATCH] D31568: Add Python 3 support to clang.cindex

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. I think it would've been nice to split this up into the changes for map/filter rather than group it together. But sure, this looks reasonable. Repository: rL LLVM https://reviews.llvm

[PATCH] D18478: python bindings: expose the clang version string

compnerd accepted this revision. compnerd added a subscriber: jbcoe. compnerd added a comment. This revision is now accepted and ready to land. This would ideally wait for the change that @jbcoe has in the works to enable python 3, but the change itself is fine. https://reviews.llvm.org/D18478

[PATCH] D32427: Fix float abi for SUSE ARM triples

compnerd added a comment. I think that you should mutate the environment in the canonicalisation phase of the triple. That will allow you to use `armv7-suse-linux-gnueabi` and `armv7-suse-linux-gnueabihf` in the frontend, but have the backend always get `armv7-suse-linux-gnueabihf`. https://

[PATCH] D32269: [Driver] Add iSOFTLinux to GNU ToolChains X86Triple

compnerd requested changes to this revision. compnerd added a comment. This revision now requires changes to proceed. Is this to actually get the correct GCC search dir? Your test doesnt really test anything AFAICT, as it is just invoking clang with a target that it would accept anyways. Repo

[PATCH] D32109: [Driver] Limit .exe extension addition to Windows hosts

compnerd requested changes to this revision. compnerd added a comment. Why not always replace the extension? Windows doesnt require the .exe suffix IIRC. https://reviews.llvm.org/D32109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:

[PATCH] D32988: [libc++] Refactor Windows support headers.

compnerd requested changes to this revision. compnerd added inline comments. Comment at: include/__config:232-235 +#ifndef NOMINMAX +#define NOMINMAX +#endif + bcraig wrote: > I can see this helping when we are build libc++, but I don't think it helps > client a

[PATCH] D33082: Fix Libc++ build with MinGW64

compnerd requested changes to this revision. compnerd added inline comments. This revision now requires changes to proceed. Comment at: include/__locale:370 static const mask blank = _ISblank; -#elif defined(_LIBCPP_MSVCRT) +#elif defined(_LIBCPP_WIN32API) typedef unsi

[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows

compnerd requested changes to this revision. compnerd added a comment. This revision now requires changes to proceed. I think that we should sink the `min`/`max` checks into `__undef_macros`. I don't like the idea of littering that check everywhere. Comment at: include/__conf

[PATCH] D33082: Fix Libc++ build with MinGW64

compnerd added a comment. Sure, a `_LIBCPP_MSVCRT_LIKE` WFM. I just want to make sure that we don''t conflate the underlying libc implementation with the Win32 API set. https://reviews.llvm.org/D33082 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal

compnerd requested changes to this revision. compnerd added inline comments. This revision now requires changes to proceed. Comment at: cmake/caches/BaremetalARM.cmake:1 +set(LLVM_TARGETS_TO_BUILD ARM CACHE STRING "") + Please rename this file to `BareMetalARMv6.

[PATCH] D33385: __cxa_demangle: Fix constructor cv qualifier handling

compnerd added a comment. Can you please ensure that you cross-port this into `llvm/lib/Demangle/ItaniumDemangle.cpp`? https://reviews.llvm.org/D33385 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler

compnerd requested changes to this revision. compnerd added inline comments. This revision now requires changes to proceed. Comment at: src/cxa_demangle.cpp:3036 break; -if (db.names.size() < 2) +if (k1 <= k0)

[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal

compnerd added inline comments. Comment at: cmake/caches/BaremetalARM.cmake:1 +set(LLVM_TARGETS_TO_BUILD ARM CACHE STRING "") + jroelofs wrote: > compnerd wrote: > > Please rename this file to `BareMetalARMv6.cmake`. (I'm interested in the > > suffix primarily)

[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal

compnerd added inline comments. Comment at: lib/Driver/ToolChains/BareMetal.cpp:68 + SmallString<128> Dir(getDriver().ResourceDir); + llvm::sys::path::append(Dir, "lib", "baremetal"); + return Dir.str(); jroelofs wrote: > compnerd wrote: > > jroelofs wrote: >

[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler

compnerd added inline comments. Comment at: src/cxa_demangle.cpp:3036 break; -if (db.names.size() < 2) +if (k1 <= k0) return first; erik.pilkington wrote: > compnerd wrote:

[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. This is a pretty straightforward cleanup now, which adds additional functionality by deferring work to CMake. There are a couple of minor points about inconsistent quoting but this seems

[PATCH] D111457: [clang][test] Add lit helper for windows paths

compnerd requested changes to this revision. compnerd added inline comments. This revision now requires changes to proceed. Comment at: clang/test/lit.cfg.py:60 +if platform.system() == 'Windows': +root_sep = 'C:\\' +else: This isn't really a separator, this

[PATCH] D116722: [clang] Verify ssp buffer size is a valid integer

compnerd added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3204 if (StackProtectorLevel) { -CmdArgs.push_back("-stack-protector-buffer-size"); -// FIXME: Verify the argument is a valid integer. -CmdArgs.push_back(Args.MakeArgS

[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

compnerd added inline comments. Comment at: clang/cmake/modules/AddClang.cmake:127 + LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX} + ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX} + RUNTIME DESTINATION ${CMAKE_INSTAL

[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

compnerd added inline comments. Comment at: clang/cmake/modules/AddClang.cmake:127 + LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX} + ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX} + RUNTIME DESTINATION ${CMAKE_INSTAL

[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

compnerd added inline comments. Comment at: clang/cmake/modules/AddClang.cmake:127 + LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX} + ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX} + RUNTIME DESTINATION ${CMAKE_INSTAL

[PATCH] D92797: APINotes: add initial stub of APINotesWriter

compnerd added a comment. Gentle post-holiday ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92797/new/ https://reviews.llvm.org/D92797 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D94364: [clang] Allow specifying the aapcs and aapcs-vfp for windows on arm

compnerd added a comment. Can you please add some more context around the motivation for this change? It seems ... unnecessary as everything is already AAPCS/AAPCS-VFP. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94364/new/ https://reviews.llvm

[PATCH] D92797: APINotes: add initial stub of APINotesWriter

compnerd added a comment. Ping x 2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92797/new/ https://reviews.llvm.org/D92797 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D94364: [clang] Allow specifying the aapcs and aapcs-vfp for windows on arm

compnerd added a comment. I think that it makes more sense to ignore the `aapcs-vfp` calling convention attribute (returning `CCCR_Ignore`), and warn on the `aapcs`. The redundant calling convention attribute does nothing, and I think that simply ignoring it would solve your issue. However, c

[PATCH] D92797: APINotes: add initial stub of APINotesWriter

compnerd added a comment. Ping x 3 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92797/new/ https://reviews.llvm.org/D92797 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D92797: APINotes: add initial stub of APINotesWriter

compnerd added a comment. Welcome back, hope you had a good time off. Thanks for the review! Comment at: clang/lib/APINotes/APINotesWriter.cpp:35 + bool SwiftImportAsMember = false; +#endif + martong wrote: > compnerd wrote: > > Please ignore this `#if`-defed

[PATCH] D95053: [Demangle] Support demangling Swift calling convention in MS demangler.

compnerd accepted this revision. compnerd added a subscriber: rnk. compnerd added a comment. This revision is now accepted and ready to land. Perhaps @rnk has any thoughts, but this seems very straightforward. Thanks for fixing this! Comment at: llvm/test/Demangle/ms-mangle.te

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

compnerd created this revision. compnerd added a reviewer: aaron.ballman. Herald added a subscriber: mgorny. compnerd requested review of this revision. Herald added a project: clang-tools-extra. This introduces a new check, readability-containter-data-pointer. This check is meant to catch the ca

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

compnerd updated this revision to Diff 369331. compnerd added a comment. Reflow the text using clang-format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108893/new/ https://reviews.llvm.org/D108893 Files: clang-tools-extra/clang-tidy/readabili

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

compnerd updated this revision to Diff 369340. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108893/new/ https://reviews.llvm.org/D108893 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-tidy/readability/Con

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

compnerd added a comment. In D108893#2971410 , @Eugene.Zelenko wrote: > Thank you for implementing 26817 > ! But shouldn't this check > belong to `modernize` module? Oh, I was unaware of the PR, I'll tag th

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

compnerd added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:18 +namespace tidy { +namespace readability { +ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name, Eugene.Zelenko wrote: > compner

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

compnerd added a comment. Hmm, one case that doesn't currently get handled properly is the following test case: c++ template void f(const T *); void g(const std::vector **v) { f(&(**v)[0]); } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

compnerd updated this revision to Diff 369343. compnerd edited the summary of this revision. compnerd added a comment. Add some documentation and release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108893/new/ https://reviews.llvm.org/D108

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

compnerd marked 2 inline comments as done. compnerd added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:87 + + Finds cases where code could use ``data`` rather than the address of an element. + Eugene.Zelenko wrote: > It'll be good idea t

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

compnerd updated this revision to Diff 369453. compnerd marked 2 inline comments as done. compnerd added a comment. Update release notes to incorporate feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108893/new/ https://reviews.llvm.org/D10

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

compnerd added a comment. @Eugene.Zelenko I'd like to have @aaron.ballman weigh in on the naming, and assuming that he's okay with `modernize.container-data-pointer`, I'll change it to that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108893/new

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

compnerd marked 7 inline comments as done. compnerd added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:63 +hasType(Container), +hasType(

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

compnerd updated this revision to Diff 372285. compnerd added a comment. Address review feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108893/new/ https://reviews.llvm.org/D108893 Files: clang-tools-extra/clang-tidy/readability/CMakeList

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

compnerd marked an inline comment as done. compnerd added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp:104 + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer i

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

This revision was automatically updated to reflect the committed changes. compnerd marked an inline comment as done. Closed by commit rGd0d9e6f0849b: clang-tidy: introduce readability-containter-data-pointer check (authored by compnerd). Changed prior to commit: https://reviews.llvm.org/D108893

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

compnerd added a comment. @Eugene.Zelenko - sorry, I didn't see the additional comments before the commit. I'm happy to do a follow up depending on the resolution. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108893/new/ https://reviews.llvm.org

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

compnerd added a comment. @thakis - thanks, seems that I had a part of the change sitting in my stash ... I had added a `-NOT` to verify the behaviour, and forgot to remove it. I'll revert the revert with the fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

<    2   3   4   5   6   7