[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 clang/lib/CodeGen/CGDeclCX

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

erik.pilkington added a comment. In D61165#1485514 , @rjmccall wrote: > 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

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

erik.pilkington added a comment. In D61165#1487328 , @rjmccall wrote: > In D61165#1487100 , @erik.pilkington > wrote: > > > It seems like the most common sense interpretation here is to just treat > > the initiali

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

erik.pilkington added a comment. In D61165#1488657 , @rjmccall wrote: > In D61165#1488553 , @erik.pilkington > wrote: > > > In D61165#1487328 , @rjmccall > > wrote: > > > >

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

erik.pilkington added a comment. In D61165#1490168 , @rjmccall wrote: > I think the intuitive rule is that initialization is complete when the > full-expression performing the initialization is complete because that's the > normal unit of sequencing. No

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

erik.pilkington added a comment. In D61165#1490608 , @rjmccall wrote: > The flip side of that argument is that (1) there aren't very many users right > now and (2) it's much easier to start conservative and then weaken the rule > than it will be to stren

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

erik.pilkington added a comment. Duncan pointed out eel.is/c++draft/class.init#class.base.init-13, which appears to claim that initialization ends with the execution of the constructor, excluding temporary destructors. > (13) In a non-delegating constructor, initialization proceeds in the > fo

[PATCH] D52253: Fix an assert in the implementation of -Wquoted-include-in-framework-header

erik.pilkington created this revision. erik.pilkington added a reviewer: bruno. Herald added a subscriber: dexonsmith. rdar://43692300 Repository: rC Clang https://reviews.llvm.org/D52253 Files: clang/lib/Lex/HeaderSearch.cpp clang/test/Frontend/Inputs/Radar43692300/Headers/Headers/File.

[PATCH] D52253: Fix an assert in the implementation of -Wquoted-include-in-framework-header

erik.pilkington added inline comments. Comment at: clang/lib/Lex/HeaderSearch.cpp:670 if (IsIncludeeInFramework) { NewInclude += StringRef(ToFramework).drop_back(10); // drop .framework NewInclude += "/"; Crash was here, if ToFramework is empty

[PATCH] D52253: Fix an assert in the implementation of -Wquoted-include-in-framework-header

erik.pilkington updated this revision to Diff 166070. erik.pilkington added a comment. Sure, the new patch moves the test to double-quotes.m and gives it a more meaningful name. Thanks! https://reviews.llvm.org/D52253 Files: clang/lib/Lex/HeaderSearch.cpp clang/test/Modules/Inputs/double-

[PATCH] D52271: [Sema] Ensure that we retain __restrict qualifiers when substituting a reference type.

erik.pilkington created this revision. erik.pilkington added reviewers: rsmith, rjmccall. Herald added a subscriber: dexonsmith. Fixes rdar://43760099. Thanks for taking a look! Repository: rC Clang https://reviews.llvm.org/D52271 Files: clang/lib/Sema/TreeTransform.h clang/test/SemaCXX/

[PATCH] D52271: [Sema] Ensure that we retain __restrict qualifiers when substituting a reference type.

erik.pilkington updated this revision to Diff 166224. erik.pilkington added a comment. Sure, the new patch just preserves __restrict qualifiers. Thanks! https://reviews.llvm.org/D52271 Files: clang/lib/Sema/TreeTransform.h clang/test/SemaCXX/subst-restrict.cpp Index: clang/test/SemaCXX/su

[PATCH] D52339: Support enums with a fixed underlying type in all language modes

erik.pilkington created this revision. erik.pilkington added reviewers: rsmith, rjmccall. Herald added a subscriber: dexonsmith. A significant number of internal C users have been complaining about the lack of support for fixed enums. We already support this in Objective-C mode, as well as in C

[PATCH] D52339: Support enums with a fixed underlying type in all language modes

erik.pilkington added a comment. In https://reviews.llvm.org/D52339#1242126, @aaron.ballman wrote: > In https://reviews.llvm.org/D52339#1242118, @jfb wrote: > > > In https://reviews.llvm.org/D52339#1242103, @aaron.ballman wrote: > > > > > In https://reviews.llvm.org/D52339#1242086, @jfb wrote: >

[PATCH] D52339: Support enums with a fixed underlying type in all language modes

erik.pilkington added inline comments. Comment at: clang/include/clang/Basic/Features.def:90 FEATURE(objc_default_synthesize_properties, LangOpts.ObjC2) -FEATURE(objc_fixed_enum, LangOpts.ObjC2) +FEATURE(objc_fixed_enum, true) FEATURE(objc_instancetype, LangOpts.ObjC2)

[PATCH] D52339: Support enums with a fixed underlying type in all language modes

erik.pilkington added a comment. > There's no reason to make this an `ObjC2`-only feature; we should probably > eliminate that distinction throughout the compiler. Sure, I was just writing a proposal to do that: http://lists.llvm.org/pipermail/cfe-dev/2018-September/059468.html Repository:

[PATCH] D52339: Support enums with a fixed underlying type in all language modes

erik.pilkington added reviewers: jlebar, Anastasia. erik.pilkington added subscribers: jlebar, Anastasia. erik.pilkington added inline comments. Comment at: clang/include/clang/Basic/Features.def:233 EXTENSION(cxx_variadic_templates, LangOpts.CPlusPlus) +EXTENSION(cxx_fixed_enum

[PATCH] D52521: [Sema] DR727: Ensure we pick up enclosing template instantiation arguments for a class-scope explicit specialization.

erik.pilkington created this revision. erik.pilkington added reviewers: rsmith, rjmccall. Herald added a subscriber: dexonsmith. Previously, we stopped early on a class-scope explicit specialization, leading us to lose enclosing levels of template arguments. I'm not entirely convinced that this

[PATCH] D52521: [Sema] DR727: Ensure we pick up enclosing template instantiation arguments for a class-scope explicit specialization.

erik.pilkington added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:143 TSK_ExplicitSpecialization && !Function->getClassScopeSpecializationPattern())) break; It loo

[PATCH] D52574: NFC: Fix some darwin linker warnings introduced in r338385

erik.pilkington created this revision. erik.pilkington added reviewers: xbolva00, kristina, asb. Herald added subscribers: kadircet, jocewei, PkmX, dexonsmith, the_o, brucehoult, MartinMosbeck, rogfer01, mgrang, edward-jones, zzheng, jrtc27, ioeric, niosHD, sabuasal, apazos, simoncook, johnrusso,

[PATCH] D52400: Improve -Wshadow warnings with enumerators

erik.pilkington added inline comments. Comment at: lib/Sema/SemaDecl.cpp:16320 +// Check for other kinds of shadowing not already handled. +CheckShadow(New, PrevDecl, R); + I don't think we should do this for scoped enums in C++, i.e. this should be fine

[PATCH] D52339: Support enums with a fixed underlying type in all language modes

erik.pilkington added a comment. Ping! This doesn't really affect CUDA or OpenCL more than any change to the compiler, so I don't think it makes sense to block this until we hear from them. Repository: rC Clang https://reviews.llvm.org/D52339 __

[PATCH] D52339: Support enums with a fixed underlying type in all language modes

erik.pilkington added a comment. In https://reviews.llvm.org/D52339#1249457, @aaron.ballman wrote: > However, short of that concern, this LGTM. I've spoken with the paper author > for the WG14 paper and I think this approach fits their general proposal, so > I don't think we'll have divergence

[PATCH] D52675: [clang] Properly apply attributes on explicit instantiations of static data members

erik.pilkington added reviewers: rsmith, rjmccall. erik.pilkington added a comment. Add some reviewers. Repository: rC Clang https://reviews.llvm.org/D52675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[PATCH] D52791: [Diagnostics] Check for misleading variable declarations

erik.pilkington added a comment. Thanks for working on this! Comment at: include/clang/Basic/DiagnosticSemaKinds.td:318-320 +def warn_misleading_var_type_decl : Warning< + "misleading variable declaration, supposed to be a pointer type instead ?">, + InGroup>; ---

[PATCH] D52521: [Sema] DR727: Ensure we pick up enclosing template instantiation arguments for a class-scope explicit specialization.

erik.pilkington updated this revision to Diff 168352. erik.pilkington added a comment. Address @rsmith's review comments. https://reviews.llvm.org/D52521 Files: clang/include/clang/AST/DeclTemplate.h clang/lib/Sema/SemaTemplateInstantiate.cpp clang/test/SemaCXX/member-spec-dr727.cpp Inde

[PATCH] D52521: [Sema] DR727: Ensure we pick up enclosing template instantiation arguments for a class-scope explicit specialization.

erik.pilkington added inline comments. Comment at: clang/include/clang/AST/DeclTemplate.h:1771-1779 + void setIsSpecializedMember(bool V = true) { IsSpecializedMember = V; } + + /// If this class template specialization was declared at class scope (DR727). + /// For example,

[PATCH] D57634: Support for GCC's `__builtin_va_arg_pack{,_len}`

erik.pilkington created this revision. erik.pilkington added reviewers: rsmith, jfb, aaron.ballman. Herald added subscribers: kristina, dexonsmith, jkorous. Herald added a project: clang. You can read more about these attributes here: https://gcc.gnu.org/onlinedocs/gcc-4.4.2/gcc/Constructing-Call

[PATCH] D57634: Support for GCC's `__builtin_va_arg_pack{,_len}`

erik.pilkington abandoned this revision. erik.pilkington added a comment. Tentatively marking as abandoned, Eli suggested a different approach in the LLVM review which I'm going to look into. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57634/new/ https://revie

[PATCH] D57712: [SemaObjC] Don't infer the availabilty of messages to +new from -init if the receiver has Class type

erik.pilkington created this revision. erik.pilkington added a reviewer: arphaman. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. Fixes rdar://47713266 Thanks for taking a look! Erik Repository: rC Clang https://reviews.llvm.org/D57712 Files: clang/lib/Sema/

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

erik.pilkington created this revision. erik.pilkington added reviewers: aaron.ballman, rjmccall, eli.friedman. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. This attribute applies to declarations of C stdlib functions (sprintf, memcpy...) that have known fortified

[PATCH] D57936: [CodeGenObjC] When available, emit a direct call to objc_alloc_init(cls) instead of [objc_alloc(cls) init]

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, pete, ahatanak. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. This provides a code size win on the caller side, since the `init` message send is done in the runtime function. rdar://

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

erik.pilkington updated this revision to Diff 186013. erik.pilkington marked 5 inline comments as done. erik.pilkington added a comment. Address John's comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57918/new/ https://reviews.llvm.org/D57918 Files: clang/include/clang/Basic/

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

erik.pilkington added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:986 + } + }]; +} rjmccall wrote: > Probably worth clarifying somewhere in here that only a specific set of > stdlib functions are supported. I don't know that it's import

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

erik.pilkington added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:987 + +Only a specific set of standard library functions are supported: + - memcpy aaron.ballman wrote: > And we don't have Annex K versions for any of these, like memset_s(

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

erik.pilkington updated this revision to Diff 186028. erik.pilkington marked 2 inline comments as done. erik.pilkington added a comment. Describe what the arguments do in the docs. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57918/new/ https://reviews.llvm.org/D57918 Files: clang/i

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

erik.pilkington marked an inline comment as done. erik.pilkington added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:987-989 +The first argument to the attribute is the type passed to +`__builtin_object_size`, and the second is the flag that the fortified fo

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

erik.pilkington updated this revision to Diff 186300. erik.pilkington added a comment. Link to the GCC docs for `__builitn_object_size`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57918/new/ https://reviews.llvm.org/D57918 Files: clang/include/clang/Basic/Attr.td clang/include/c

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

erik.pilkington marked 8 inline comments as done. erik.pilkington added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:987-989 +The first argument to the attribute is the type passed to +`__builtin_object_size`, and the second is the flag that the fortified fo

[PATCH] D58145: [Sema] Fix a bogus -Wconversion warning

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, rsmith, nickdesaulniers. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. DiagnoseFloatingImpCast expects an integer type, but the changes introduced in D50467

[PATCH] D58152: [Sema] Delay checking whether objc_designated_initializer is being applied to an init method

erik.pilkington created this revision. erik.pilkington added reviewers: aaron.ballman, arphaman, erichkeane. Herald added subscribers: jdoerfert, dexonsmith, jkorous. Herald added a project: clang. We ran into some source breaking changes to do with the attribute reordering changes that recently

[PATCH] D58152: [Sema] Delay checking whether objc_designated_initializer is being applied to an init method

erik.pilkington added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7258-7267 + // Do this check after processing D's attributes because the attribute + // objc_method_family can change whether the given method is in the init + // family, and it can be applied a

[PATCH] D58152: [Sema] Delay checking whether objc_designated_initializer is being applied to an init method

erik.pilkington updated this revision to Diff 186712. erik.pilkington marked an inline comment as done. erik.pilkington added a comment. Remove an `auto`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58152/new/ https://reviews.llvm.org/D58152 Files: clang/include/clang/Basic/Attr.td

[PATCH] D58145: [Sema] Fix a bogus -Wconversion warning

erik.pilkington updated this revision to Diff 186774. erik.pilkington marked 2 inline comments as done. erik.pilkington added a comment. I was taking a final look at this, and I noticed that we were sending the wrong arguments to `DiagnoseFloatingImpCast`. The expression argument is meant to be

[PATCH] D57936: [CodeGenObjC] When available, emit a direct call to objc_alloc_init(cls) instead of [objc_alloc(cls) init]

erik.pilkington updated this revision to Diff 186784. erik.pilkington marked an inline comment as done. erik.pilkington added a comment. Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57936/new/ https://reviews.llvm.org/D57936 Files: clang/include/clang/Basic/

[PATCH] D57936: [CodeGenObjC] When available, emit a direct call to objc_alloc_init(cls) instead of [objc_alloc(cls) init]

erik.pilkington marked 2 inline comments as done. erik.pilkington added inline comments. Comment at: clang/lib/CodeGen/CGObjC.cpp:459 + llvm::Value *Receiver = + CGF.CGM.getObjCRuntime().GetClass(CGF, ObjTy->getInterface()); + return CGF.EmitObjCAllocInit(Receiver, CGF.Con

[PATCH] D58254: [Sema] Diagnose floating point conversions based on target semantics

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, nickdesaulniers. Herald added subscribers: jdoerfert, dexonsmith, jkorous. ...instead of just comparing rank. Also, fix a bad warning about `_Float16`, since its declared out of order in BuiltinTypes.def, meaning co

[PATCH] D58145: [Sema] Fix a bogus -Wconversion warning

erik.pilkington marked an inline comment as done. erik.pilkington added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:10634 + // warning. + else if (ResultBT->isFloatingPoint() && ResultBT->getKind() < RBT->getKind() && + // We don't want to warn for s

[PATCH] D57270: [ObjC] Fix non-canonical types preventing type arguments substitution.

erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57270/new/ https://reviews.llvm.org/D57270 ___ cfe-commits mailing

[PATCH] D57075: [ObjC] For type substitution in generics use a regular recursive type visitor.

erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land. LGTM too, I agree this should be NFC with the parent commit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57075/new/ https://reviews.llvm.org/D57075

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

erik.pilkington created this revision. erik.pilkington added reviewers: rsmith, jfb, rjmccall. Herald added subscribers: llvm-commits, kristina, arphaman, dexonsmith, jkorous, hiraditya. Herald added projects: clang, LLVM. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0476r2.html This

[PATCH] D62643: [CodeGen][ObjC] Convert '[self alloc]' in a class method to 'objc_alloc(self)'

erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land. I thinks this looks good, aside from a null concern. Comment at: lib/CodeGen/CGObjC.cpp:460 + const Expr *SelfInClassMethod = nullptr; + if (const auto *C

[PATCH] D62831: [CodeGen][ObjC] Add attribute "arc_retain_agnostic" to ObjC globals that are retain-agnostic

erik.pilkington added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:1441 + cast(literal)->addAttribute("arc_retain_agnostic"); + Can't you just declare `literal` as a GlobalVariable instead of immediately casting it? Comment at: lib

[PATCH] D62831: [CodeGen][ObjC] Add attribute "arc_retain_agnostic" to ObjC globals that are retain-agnostic

erik.pilkington added a comment. I think this looks good from a clang perspective. No reason to land it before the dust settles on D62433 though. Do you think we can also add this attribute (or something like it) onto allocas of `_NSConcreteStackBlock`s? They a

[PATCH] D62960: SVE opaque type for C intrinsics demo

erik.pilkington added inline comments. Comment at: lib/AST/ItaniumMangle.cpp:2680 +break; +#include "clang/Basic/AArch64SVEACLETypes.def" } jfb wrote: > @rjmccall you probably should review this part. Sorry for the drive by comment, but: All of these mangl

[PATCH] D63176: [SemaObjC] Infer availability of stuff declared in categories from the availability of the enclosing category

erik.pilkington created this revision. erik.pilkington added reviewers: arphaman, steven_wu. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. These extend the class, but (unlike methods in @interfaces) can be referenced without mentioning the the category, so we don't

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

erik.pilkington updated this revision to Diff 204609. erik.pilkington marked 46 inline comments as done. erik.pilkington added a comment. Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62825/new/ https://reviews.llvm.org/D62825 Files: clang/include/clang-c/Ind

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

erik.pilkington added a comment. In D62825#1528329 , @rsmith wrote: > Can we also use the same bit reader/writer implementation to generalize the > current implementation of `__builtin_memcpy` and friends? (I don't think we > can remove the existing impl

[PATCH] D62960: SVE opaque type for C intrinsics demo

erik.pilkington added inline comments. Comment at: lib/AST/ItaniumMangle.cpp:2680 +break; +#include "clang/Basic/AArch64SVEACLETypes.def" } rsandifo-arm wrote: > erik.pilkington wrote: > > jfb wrote: > > > @rjmccall you probably should review this part. >

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

erik.pilkington updated this revision to Diff 205713. erik.pilkington marked 16 inline comments as done. erik.pilkington added a comment. Address review comments, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62825/new/ https://reviews.llvm.org/D62825 Files: clang/include/cla

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

erik.pilkington added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:5461-5469 +case APValue::LValue: { + LValue LVal; + LVal.setFrom(Info.Ctx, Val); + APValue RVal; + if (!handleLValueToRValueConversion(Info, BCE, Ty.withConst(), +

[PATCH] D64600: [ObjC] Add an attribute that "clamps" signed char BOOLs to {0,1}

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, aaron.ballman, steven_wu. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. This attribute can be applied to typedefs of BOOL in Objective-C. It causes loads, stores, and casts to BOOL to

[PATCH] D64600: [ObjC] Add an attribute that "clamps" signed char BOOLs to {0,1}

erik.pilkington added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3279 +def ObjCClampingBool : TypeAttr { + let Spellings = [Clang<"objc_clamping_bool">]; + let Subjects = SubjectList<[TypedefName]>; aaron.ballman wrote: > Is there a desire f

[PATCH] D64600: [ObjC] Add an attribute that "clamps" signed char BOOLs to {0,1}

erik.pilkington updated this revision to Diff 209603. erik.pilkington marked 2 inline comments as done. erik.pilkington added a comment. Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64600/new/ https://reviews.llvm.org/D64600 Files: clang/include/clang/Basic/

[PATCH] D65022: [Sema] Always instantiate the initializer of a variable template with undeduced type (8.0 regression)

erik.pilkington created this revision. erik.pilkington added reviewers: rsmith, rjmccall. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. This patch fixes a regression introduced in r359947. Here: template struct Outer { template static constexpr auto x = 1;

[PATCH] D65022: [Sema] Always instantiate the initializer of a variable template with undeduced type (8.0 regression)

erik.pilkington updated this revision to Diff 212026. erik.pilkington added a comment. Fix the type of the MemberExpr/DeclRefExpr after instantiating the variable initializer. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65022/new/ https://reviews.llvm.org/D65022 Files: clang/lib/S

[PATCH] D65359: [Sema] Map from a variable template specialization in a pattern to a variable template specialization in an instantiation properly

erik.pilkington created this revision. erik.pilkington added a reviewer: rsmith. Herald added subscribers: dexonsmith, jkorous. Previously, when instantiating `S::mf` below, we would emit a reference to the declaration of S::v, when we really ought to make a new declaration for `v` in the templa

[PATCH] D65022: [Sema] Always instantiate the initializer of a variable template with undeduced type (8.0 regression)

erik.pilkington updated this revision to Diff 212470. erik.pilkington added a comment. Rebase on top of r367367. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65022/new/ https://reviews.llvm.org/D65022 Files: clang/lib/Sema/SemaExpr.cpp clang/lib/Sema/SemaExprMember.cpp clang/tes

[PATCH] D65665: Support for attributes on @class declarations

erik.pilkington created this revision. erik.pilkington added reviewers: aaron.ballman, arphaman, rjmccall. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. This is useful to make availability checking work with forward declarations, but there also seem to be other att

[PATCH] D27165: Add format_dynamic_key_arg attribute to improve "-Wformat" warnings for functions that load the formatting string dynamically based on a key value

erik.pilkington commandeered this revision. erik.pilkington added a reviewer: arphaman. erik.pilkington added a comment. Herald added subscribers: dexonsmith, jkorous. Herald added a project: LLVM. Stealing this (with Alex's permission). I think it makes more sense to add a special case for the m

[PATCH] D27165: Add format_dynamic_key_arg attribute to improve "-Wformat" warnings for functions that load the formatting string dynamically based on a key value

erik.pilkington updated this revision to Diff 214925. erik.pilkington added a comment. Special case localizedStringForKey instead of using an attribute. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D27165/new/ https://reviews.llvm.org/D27165 Files: clang/include/clang/AST/FormatStrin

[PATCH] D27165: Add format_dynamic_key_arg attribute to improve "-Wformat" warnings for functions that load the formatting string dynamically based on a key value

erik.pilkington updated this revision to Diff 214977. erik.pilkington added a comment. Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D27165/new/ https://reviews.llvm.org/D27165 Files: clang/include/clang/AST/FormatString.h clang/include/clang/Basic/Identifie

[PATCH] D65022: [Sema] Always instantiate the initializer of a variable template with undeduced type (8.0 regression)

erik.pilkington added a comment. Hmm, looks like this patch fails to fix the case where we're referencing the variable in an non-odr use context, since we don't even bother to instantiate the initializer here: template struct S { template static constexpr auto x = 43; }; in

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

erik.pilkington updated this revision to Diff 206304. erik.pilkington marked 10 inline comments as done. erik.pilkington added a comment. Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62825/new/ https://reviews.llvm.org/D62825 Files: clang/include/clang-c/Ind

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

erik.pilkington added inline comments. Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:223 +def note_constexpr_bit_cast_invalid_type : Note< + "cannot constexpr evaluate a bit_cast with a " + "%select{union|pointer|member pointer|volatile|struct with a reference me

[PATCH] D63856: [ObjC] Add a -Wtautological-compare warning for BOOL

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, steven_wu, rsmith. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. On macOS, BOOL is a typedef for signed char, but it should never hold a value that isn't 1 or 0. Any code that expects

[PATCH] D63856: [ObjC] Add a -Wtautological-compare warning for BOOL

erik.pilkington added a comment. In D63856#1560180 , @rjmccall wrote: > This only applies to relational operators, right? I'm a little uncomfortable > with calling this "tautological" since it's not like it's *undefined > behavior* to have `(BOOL) 2`, i

[PATCH] D63856: [ObjC] Add a -Wtautological-compare warning for BOOL

erik.pilkington added a comment. In D63856#1561112 , @rjmccall wrote: > In D63856#1560213 , @erik.pilkington > wrote: > > > In D63856#1560180 , @rjmccall > > wrote: > > > >

[PATCH] D63912: [ObjC] Add a warning for implicit conversions of a constant non-boolean value to BOOL

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, steven_wu. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. erik.pilkington added a reviewer: arphaman. On macOS, BOOL is a typedef for signed char, but it should never hold a value that

[PATCH] D63954: Add lifetime categories attributes

erik.pilkington added a comment. > We explicitly allow to add an annotation after > the definition of the class to allow adding annotations > to external source from by the user, e.g. > > #include > > namespace std { > template > class [[gsl::Owner(T)]] vector; > } Wait, does tha

[PATCH] D63912: [ObjC] Add a warning for implicit conversions of a constant non-boolean value to BOOL

erik.pilkington added a comment. Ping! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63912/new/ https://reviews.llvm.org/D63912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[PATCH] D63856: [ObjC] Add a -Wtautological-compare warning for BOOL

erik.pilkington added a comment. Ping! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63856/new/ https://reviews.llvm.org/D63856 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[PATCH] D63856: [ObjC] Add a -Wtautological-compare warning for BOOL

erik.pilkington added a comment. In D63856#1561160 , @rjmccall wrote: > In D63856#1561132 , @erik.pilkington > wrote: > > > In D63856#1561112 , @rjmccall > > wrote: > > > >

[PATCH] D63912: [ObjC] Add a warning for implicit conversions of a constant non-boolean value to BOOL

erik.pilkington updated this revision to Diff 208545. erik.pilkington marked 2 inline comments as done. erik.pilkington added a comment. Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63912/new/ https://reviews.llvm.org/D63912 Files: clang/include/clang/Basic/

[PATCH] D63912: [ObjC] Add a warning for implicit conversions of a constant non-boolean value to BOOL

erik.pilkington added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3251 + "the only well defined values for BOOL are YES and NO">, + InGroup; rjmccall wrote: > The `from` seems awkward here; `%0` is the number, not the type.

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

erik.pilkington updated this revision to Diff 198916. erik.pilkington marked 6 inline comments as done. erik.pilkington added a comment. Address review comments. Also remove the special case where we wouldn't check a destructor for an array in `-fno-exceptions` mode. This seems inconsistent with

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

erik.pilkington added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3897-3900 +This works in almost all cases, but if ``no_destroy`` is applied to a ``static`` +or ``thread_local`` local builtin array variable and exceptions are enabled, the +destructor is

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

erik.pilkington marked 2 inline comments as done. erik.pilkington added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:13121-13122 + if (VD->isNoDestroy(getASTContext()) && + !(VD->getType()->isArrayType() && getLangOpts().Exceptions && +VD->isStaticLoc

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

erik.pilkington updated this revision to Diff 198944. erik.pilkington marked an inline comment as done. erik.pilkington added a comment. Rename `hasAccessibleDestructor`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61165/new/ https://reviews.llvm.org/D61165 Files: clang/include/cla

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

erik.pilkington added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:13121-13122 + if (VD->isNoDestroy(getASTContext()) && + !(VD->getType()->isArrayType() && getLangOpts().Exceptions && +VD->isStaticLocal())) return; rjmccall wrot

[PATCH] D61803: [CodeGen][ObjC] Emit invoke instead of call to call `objc_release` when necessary.

erik.pilkington added inline comments. Comment at: lib/CodeGen/CGObjC.cpp:2634-2646 + ASTContext &Ctx = getContext(); + const ImplicitParamDecl *paramDecl = + ImplicitParamDecl::Create(Ctx, nullptr, SourceLocation(), nullptr, +Ctx.VoidPtrTy,

[PATCH] D61803: [CodeGen][ObjC] Emit invoke instead of call to call `objc_release` when necessary.

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/D61803/new/ https://reviews.llvm.org/D61803 ___

[PATCH] D61957: [CodeGenObjC] invoke objc_autorelease, objc_retain when necessary

erik.pilkington created this revision. erik.pilkington added reviewers: ahatanak, rjmccall. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. Any of these methods can be overridden, so we always need to conservatively `invoke` these calls. Repository: rC Clang htt

[PATCH] D61722: [AST] Add RecoveryExpr; produce it on overload resolution failure and missing member.

erik.pilkington added a reviewer: erik.pilkington. erik.pilkington added inline comments. Herald added a subscriber: dexonsmith. Comment at: include/clang/AST/BuiltinTypes.def:265 +// a template. +BUILTIN_TYPE(Recovery, RecoveryTy) + Why are you creating a new ty

[PATCH] D55628: Add support for "labels" on push/pop directives in #pragma clang attribute

erik.pilkington created this revision. erik.pilkington added reviewers: arphaman, aaron.ballman. Herald added subscribers: dexonsmith, jkorous. One problem with defining macros that expand to _Pragma("clang attribute")` is that they don't nest very well: // In some header... #define ASSUME_X

[PATCH] D55628: Add support for "labels" on push/pop directives in #pragma clang attribute

erik.pilkington marked an inline comment as done. erik.pilkington added inline comments. Comment at: clang/test/Sema/pragma-attribute-label.c:7 + +#pragma clang attribute pop // expected-error{{'#pragma clang attribute pop' with no matching '#pragma clang attribute push'}} +#pra

[PATCH] D55628: Add support for "labels" on push/pop directives in #pragma clang attribute

erik.pilkington updated this revision to Diff 178254. erik.pilkington marked 7 inline comments as done. erik.pilkington added a comment. Address @aaron.ballman comments. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55628/new/ https://reviews.llvm.org/D55628 Files: clang/docs

[PATCH] D55628: Add support for "labels" on push/pop directives in #pragma clang attribute

erik.pilkington added inline comments. Comment at: clang/docs/LanguageExtensions.rst:2667 +Because multiple push directives can be nested, if you're writing a macro that +expands to ``_Pragma("clang attribute")`` it's good hygiene to add a label to +your push/pop directives. A po

[PATCH] D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, ahatanak, jordan_rose, aaron.ballman. Herald added subscribers: dexonsmith, jkorous. This attribute, called "objc_externally_retained", exposes clang's notion of pseudo-`__strong` variables in ARC. Pseudo-strong var

[PATCH] D55628: Add support for "labels" on push/pop directives in #pragma clang attribute

erik.pilkington updated this revision to Diff 178837. erik.pilkington added a comment. After looking through some users of `#pragma clang attribute`, I don't think that the begin/end solution is what we want here. Some users of this attribute write macros that can expand to different attributes

<    1   2   3   4   5   6   >