[PATCH] D42508: AST: support protocol conformances on id/class/interfaces in MS ABI

majnemer accepted this revision. majnemer added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D42508 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D42641: [MinGW] Emit typeinfo locally for dllimported classes without key functions

majnemer added inline comments. Comment at: lib/CodeGen/CGVTables.cpp:887-888 + if (CGM.getTriple().isWindowsGNUEnvironment() && RD->hasAttr()) +return true; + Maybe a comment like "VTables of classes declared as dllimport are always considered to be ext

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

majnemer added a comment. docs/LanguageExtensions.html should be updated to mention that we support this extension on all ELF targets. Repository: rC Clang https://reviews.llvm.org/D42758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D29912: [MS ABI] Correctly mangling vbase destructors

majnemer created this revision. They are a little bit of a special case in the mangling. They are always mangled without taking into account their virtual-ness of the destructor. They are also mangled to return void, unlike the actual destructor. This fixes PR31931. https://reviews.llvm.org/D29

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

majnemer added inline comments. Comment at: lib/Parse/ParseDecl.cpp:2989 + + Diag(Loc, diag::err_ms_attributes_not_enabled); + continue; aaron.ballman wrote: > compnerd wrote: > > aaron.ballman wrote: > > > compnerd wrote: > > > > I think that w

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

majnemer added inline comments. Comment at: lib/Parse/ParseDecl.cpp:2989 + + Diag(Loc, diag::err_ms_attributes_not_enabled); + continue; aaron.ballman wrote: > majnemer wrote: > > aaron.ballman wrote: > > > compnerd wrote: > > > > aaron.ballman

[PATCH] D29912: [MS ABI] Correctly mangling vbase destructors

This revision was automatically updated to reflect the committed changes. Closed by commit rL295010: [MS ABI] Correctly mangling vbase destructors (authored by majnemer). Changed prior to commit: https://reviews.llvm.org/D29912?vs=88258&id=88280#toc Repository: rL LLVM https://reviews.llvm.

[PATCH] D29930: Add `__is_direct_constructible` trait for checking safe reference initialization.

majnemer added inline comments. Comment at: include/clang/Basic/TokenKinds.def:418 TYPE_TRAIT_N(__is_nothrow_constructible, IsNothrowConstructible, KEYCXX) +TYPE_TRAIT_N(__is_direct_constructible, IsDirectConstructible, KEYCXX) Should this trait be grouped her

[PATCH] D30082: Fix assertion when generating debug information for deduced template specialization types.

majnemer added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:2479 +case Type::DeducedTemplateSpecialization: { + QualType DT = dyn_cast(T)->getDeducedType(); + assert(!DT.isNull() && "Undeduced types shouldn't reach here."); You are uncon

[PATCH] D30806: [nonnull] Teach Clang to attach the nonnull LLVM attribute to declarations and calls instead of just definitions, and then teach it to *not* attach such attributes even if the source c

majnemer added inline comments. Comment at: lib/AST/ASTContext.cpp:8786 +if (OverrideNonnull && OverrideNonnullArgs) + *OverrideNonnullArgs |= 1 << ArgTypes.size(); + `1U` to avoid overflow UB? Comment at: lib/CodeGen/CGCall.cpp:2082 +

[PATCH] D30809: [coroutines] Add codegen for await and yield expressions

majnemer added inline comments. Comment at: lib/CodeGen/CGCoroutine.cpp:26 +enum class AwaitKind { Init, Normal, Yield, Final }; +char const *AwaitKindStr[] = {"init", "await", "yield", "final"}; +} I'd move this into buildSuspendSuffixStr. Com

[PATCH] D30809: [coroutines] Add codegen for await and yield expressions

majnemer added inline comments. Comment at: lib/CodeGen/CGCoroutine.cpp:85 + unsigned No = 0; + const char* AwaitKindStr = 0; + switch (Kind) { I'd use a StringRef here. https://reviews.llvm.org/D30809 ___ cfe-c

[PATCH] D30809: [coroutines] Add codegen for await and yield expressions

majnemer added inline comments. Comment at: lib/CodeGen/CGCoroutine.cpp:85 + unsigned No = 0; + StringRef AwaitKindStr = 0; + switch (Kind) { I'd just let the default constructor do its thing. https://reviews.llvm.org/D30809 __

[PATCH] D27226: [MS ABI] Implement more of the Itanium mangling rules

majnemer created this revision. majnemer added a reviewer: rnk. majnemer added a subscriber: cfe-commits. We didn't implement one of the corner cases: a lambda which belongs to an initializer for a field. In this case, we need to mangle the field name into the lambda. This fixes PR31197. https

[PATCH] D27226: [MS ABI] Implement more of the Itanium mangling rules

majnemer updated this revision to Diff 79837. majnemer added a comment. - Address review comments https://reviews.llvm.org/D27226 Files: lib/AST/MicrosoftMangle.cpp test/CodeGenCXX/mangle-ms-cxx11.cpp Index: test/CodeGenCXX/mangle-ms-cxx11.cpp ==

[PATCH] D27226: [MS ABI] Implement more of the Itanium mangling rules

majnemer updated this revision to Diff 79848. majnemer marked an inline comment as done. majnemer added a comment. - Address review comments https://reviews.llvm.org/D27226 Files: lib/AST/MicrosoftMangle.cpp test/CodeGenCXX/mangle-ms-cxx11.cpp Index: test/CodeGenCXX/mangle-ms-cxx11.cpp ===

[PATCH] D27226: [MS ABI] Implement more of the Itanium mangling rules

majnemer updated this revision to Diff 80096. majnemer added a comment. - Simplify the mangling a little bit https://reviews.llvm.org/D27226 Files: lib/AST/MicrosoftMangle.cpp test/CodeGenCXX/mangle-ms-cxx11.cpp Index: test/CodeGenCXX/mangle-ms-cxx11.cpp ===

[PATCH] D27226: [MS ABI] Implement more of the Itanium mangling rules

This revision was automatically updated to reflect the committed changes. Closed by commit rL288826: [MS ABI] Implement more of the Itanium mangling rules (authored by majnemer). Changed prior to commit: https://reviews.llvm.org/D27226?vs=80096&id=80437#toc Repository: rL LLVM https://revie

[PATCH] D26846: __uuidof() and declspec(uuid("...")) should be allowed on enumeration types

majnemer added a comment. This LGTM but Aaron should give the go ahead. Repository: rL LLVM https://reviews.llvm.org/D26846 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27529: Correct Vectorcall Register passing and HVA Behavior

majnemer added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:1688 +for (auto &I : FI.arguments()) { + if(Count < 6) +I.info = reclassifyHvaArgType(I.type, State, I.info); Formatting. Comment at: lib/CodeGen/TargetInfo

[PATCH] D27529: Correct Vectorcall Register passing and HVA Behavior

majnemer added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:1688 +for (auto &I : FI.arguments()) { + if(Count < 6) +I.info = reclassifyHvaArgType(I.type, State, I.info); erichkeane wrote: > majnemer wrote: > > Formatting. > I don't see

[PATCH] D27529: Correct Vectorcall Register passing and HVA Behavior

majnemer added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:3871 +for (auto &I : FI.arguments()) { + if (Count < 6) +I.info = classify(I.type, FreeSSERegs, false, IsVectorCall, IsRegCall); erichkeane wrote: > majnemer wrote: > > majnem

[PATCH] D27486: Correct class-template deprecation behavior

majnemer added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:2355 Converted, nullptr); + if (auto *attr = ClassTemplate->getTemplatedDecl() + ->getAttr()) Please capital

[PATCH] D27680: [CodeGen] Move lifetime.start of a variable when goto jumps back past its declaration

majnemer added inline comments. Comment at: lib/CodeGen/CodeGenFunction.h:619 +bool hasLabel(const LabelDecl *LD) const { + return std::find(Labels.begin(), Labels.end(), LD) != Labels.end(); +} This can be written as `llvm::is_contained(Labels, LD);

[PATCH] D32435: clang-cl: Add support for /permissive-

majnemer requested changes to this revision. majnemer added a comment. This revision now requires changes to proceed. I don't think this is correct. GDR (of Microsoft) says the behavior is different: https://www.reddit.com/r/cpp/comments/5dh7j5/visual_c_introduces_permissive_for_conformance/da5f

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

majnemer added inline comments. Comment at: include/support/win32/msvc_builtin_support.h:33 + +_LIBCPP_ALWAYS_INLINE int __builtin_popcount(unsigned int x) +{ compnerd wrote: > I think I prefer the following implementation: > > _LIBCPP_ALWAYS_INLINE int __bu

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

majnemer requested changes to this revision. majnemer added a comment. This revision now requires changes to proceed. I have a question regarding how this work with respect to the dllimport semantics known by the linker. IIUC, we will now allow a program like: extern int __declspec(dllimport)

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

majnemer added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:2219 + Info.getLangOpts().CPlusPlus && !isForManglingOnly(Kind) && + Var->hasAttr()) // FIXME: Diagnostic! The summary and the bug both mention dllimport but

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

majnemer added inline comments. Comment at: clang/test/CodeGenCXX/dllimport.cpp:2 // RUN: %clang_cc1 -disable-noundef-analysis -triple i686-windows-msvc -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s -DMSABI -w | FileCheck --check-prefix=

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

majnemer added a comment. In D117569#3258307 , @zahiraam wrote: > In D117569#3257121 , @majnemer > wrote: > >> I have a question regarding how this work with respect to the dllimport >> semantics known by the li

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

majnemer added a comment. Your example is different from mine as it nests the constexpr variable inside the function rather than having it at translation-unit scope. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117569/new/ https://reviews.llvm.org/D117569

[PATCH] D114425: [clang] Add __builtin_bswap128

majnemer added a comment. OOC, how hard would it be to generalize this builtin a little? It is nice that we have builtins like `__builtin_add_overflow` which do the right thing regardless of their input. It seems like it would be nice if we started to expose more intrinsics which did the right

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

majnemer added a comment. This is looking great! Just a few more questions. What is the behavior with something like: thread_local int x = 2; int f() { return x; } I'm wondering if we need to move this logic

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

majnemer added a comment. In D115456#3217595 , @momo5502 wrote: > In D115456#3216811 , @majnemer > wrote: > >> This is looking great! Just a few more questions. >> >> What is the behavior with something like: >>

[PATCH] D114425: [clang] Add __builtin_bswap128

majnemer added a comment. In D114425#3217478 , @Quuxplusone wrote: > In D114425#3216802 , @majnemer > wrote: > >> OOC, how hard would it be to generalize this builtin a little? It is nice >> that we have builti

[PATCH] D116020: [clang][#52782] Bail on incomplete parameter type in stdcall name mangling

majnemer added a comment. I wonder if we should have different behavior for MSVC targets. If I do: class Incomplete; extern "C" int __stdcall Fn(int, Incomplete, long long); auto fnptr = &Fn; MSVC generates: EXTRN _Fn@12:PROC It appears that they skip over incomplete types. Should

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

majnemer accepted this revision. majnemer added a comment. This revision is now accepted and ready to land. Looks great! Please give others some time to review it as it is a holiday season... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115456/new/ https://reviews.llvm.org/D115456 __

[PATCH] D115456: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG072e2a7c67b7: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI (authored by momo5502, committed by majnemer). Repository: rG LLVM

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

majnemer added a comment. Is this a new feature in MSVC? Seems like it might be. If so, should it be predicated on `isCompatibleWithMSVC(1925)` or something? CHANGES SINCE LAS

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

majnemer added inline comments. Comment at: clang/lib/CodeGen/MicrosoftCXXABI.cpp:404 bool usesThreadWrapperFunction(const VarDecl *VD) const override { -return false; +return true; } Should this depend on the MSVC compatibility version? CHANGES S

<    1   2