[PATCH] D53547: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, rsmith. Herald added a subscriber: dexonsmith. We haven't supported compiling ObjC1 for a long time (and never will again), so there isn't any reason to keep these separate. This patch replaces LangOpts::ObjC1 and

[PATCH] D53547: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

erik.pilkington updated this revision to Diff 170703. erik.pilkington marked 4 inline comments as done. erik.pilkington added a comment. Fix some spacing mistakes. Thanks! https://reviews.llvm.org/D53547 Files: clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp clang-tools-extra/

[PATCH] D53547: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

erik.pilkington added a subscriber: jingham. erik.pilkington added inline comments. Comment at: clang/lib/Basic/IdentifierTable.cpp:166-167 // in non-arc mode. - if (LangOpts.ObjC2 && (Flags & KEYARC)) return KS_Enabled; - if (LangOpts.ObjC2 && (Flags & KEYOBJC2)) return KS_

[PATCH] D53595: [C++17] Reject shadowing of capture by parameter in lambda

erik.pilkington added a comment. Thanks for working on this! Can you update www/cxx_dr_status.html too? Comment at: lib/Sema/SemaLambda.cpp:507 + bool Error = false; + if (getLangOpts().CPlusPlus17) { +// Resolution of CWG 2211 in C++17 renders shadowing ill-f

[PATCH] D53595: [C++17] Reject shadowing of capture by parameter in lambda

erik.pilkington added inline comments. Comment at: lib/Sema/SemaLambda.cpp:510 +for (const auto &Capture: Captures) { + if (Capture.Id && Capture.Id->getName() == Param->getName()) { +Error = true; Rakete wrote: > erik.pilkington

[PATCH] D53621: Support for groups of attributes in #pragma clang attribute

erik.pilkington created this revision. erik.pilkington added reviewers: aaron.ballman, arphaman. Herald added a subscriber: dexonsmith. This patch allows pushing an empty `#pragma clang attribute push`, then adding multiple attributes to it, then popping them all with `#pragma clang attribute po

[PATCH] D53621: Support for groups of attributes in #pragma clang attribute

erik.pilkington updated this revision to Diff 171218. erik.pilkington marked an inline comment as done. erik.pilkington added a comment. Add documentation and release notes, `{}`s. Thanks! https://reviews.llvm.org/D53621 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst

[PATCH] D53621: Support for groups of attributes in #pragma clang attribute

erik.pilkington marked an inline comment as done. erik.pilkington added inline comments. Comment at: clang/docs/LanguageExtensions.rst:2666 - #pragma clang attribute push(__attribute__((annotate("custom"))), apply_to = function) + #pragma clang attribute push (__attribute__(

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

erik.pilkington updated this revision to Diff 171719. erik.pilkington marked 7 inline comments as done. erik.pilkington added a comment. Address review comments. Thanks! https://reviews.llvm.org/D53522 Files: clang/include/clang/Lex/ModuleMap.h clang/lib/Frontend/DependencyFile.cpp clang/

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

erik.pilkington added inline comments. Comment at: clang/include/clang/Lex/ModuleMap.h:649-650 + ///This can differ from \c Header's name due to symlinks. void addHeader(Module *Mod, Module::Header Header, - ModuleHeaderRole Role, bool Imported = false

[PATCH] D53547: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

erik.pilkington added inline comments. Comment at: clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp:23 // this check should only be applied to ObjC sources. - if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) { + if (!getLangOpts().ObjC) { return

[PATCH] D56760: Add a new builtin: __builtin_dynamic_object_size

erik.pilkington created this revision. erik.pilkington added reviewers: george.burgess.iv, ahatanak, rjmccall, rsmith. Herald added subscribers: kristina, dexonsmith, jkorous. This builtin has the same UI as `__builtin_object_size`, but has the potential to be evaluated dynamically. It is meant t

[PATCH] D56802: [CodeGenObjC] Treat ivar offsets variables as constant if they refer to ivars of a direct subclass of NSObject with an @implementation

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, pete, ahatanak. Herald added subscribers: dexonsmith, jkorous. This patch was originally written by Pete Cooper. This is possible because the size of NSObject is effectively ABI, and will not change in the future.

[PATCH] D56802: [CodeGenObjC] Treat ivar offsets variables as constant if they refer to ivars of a direct subclass of NSObject with an @implementation

erik.pilkington updated this revision to Diff 182182. erik.pilkington added a comment. Address review comments: move the check to a function, and call it when emitting the ivar offset instead of when we emit the global. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56802/new/ h

[PATCH] D56816: [ObjC] Follow-up r350768 and allow the use of unavailable methods that are declared in a parent class from within the @implementation context

erik.pilkington added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:7374-7378 + (MD->isDefined() || + (MD->getClassInterface() && +MD->getClassInterface()->getSuperClass() && +MD->getClassInterface()->getSuperCla

[PATCH] D56816: [ObjC] Follow-up r350768 and allow the use of unavailable methods that are declared in a parent class from within the @implementation context

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

[PATCH] D56802: [CodeGenObjC] Treat ivar offsets variables as constant if they refer to ivars of a direct subclass of NSObject with an @implementation

erik.pilkington updated this revision to Diff 182214. erik.pilkington added a comment. Sure, that makes sense. The new patch makes the global constant again. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56802/new/ https://reviews.llvm.org/D56802 Files: clang/lib/CodeGen/CGObjCMac.cp

[PATCH] D56879: [Sema] Suppress a warning about a forward-declared fixed enum in C mode

erik.pilkington created this revision. erik.pilkington added reviewers: rsmith, aaron.ballman, arphaman. Herald added subscribers: dexonsmith, jkorous. As of r343360, we support fixed-enums in C. This lead to some warnings in project headers where a fixed enum is forward declared then later defin

[PATCH] D56879: [Sema] Suppress a warning about a forward-declared fixed enum in C mode

erik.pilkington updated this revision to Diff 182548. erik.pilkington marked an inline comment as done. erik.pilkington added a comment. Add a `-pedantic` RUN line. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56879/new/ https://reviews.llvm.org/D56879 Files: clang/lib/Sema/SemaDecl

[PATCH] D56892: Add a priority field to availability attributes to prioritize explicit attributes from declaration over attributes from '#pragma clang attribute'

erik.pilkington added a comment. This seems pretty reasonable to me. I agree that a more general mechanism to override #pca (/implicit) attributes would be pretty useful, but I guess there is no need to jump the gun on that. Comment at: include/clang/Sema/Sema.h:2471 +///

[PATCH] D56892: Add a priority field to availability attributes to prioritize explicit attributes from declaration over attributes from '#pragma clang attribute'

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

[PATCH] D57064: [Sema] Improve a -Warray-bounds diagnostic

erik.pilkington created this revision. erik.pilkington added reviewers: aaron.ballman, rsmith. Herald added subscribers: dexonsmith, jkorous. Fix a bug where we would compare array sizes with incompatible element types, and look through explicit casts. rdar://44800168 Thanks for taking a look!

[PATCH] D56760: Add a new builtin: __builtin_dynamic_object_size

erik.pilkington added a comment. In D56760#1367915 , @jfb wrote: > @rsmith, what do you think and how do you want to proceed? We think something > like what Erik implemented will catch things `_FORTIFY_SOURCE` currently > cannot. We agree there are valid

[PATCH] D56760: Add a new builtin: __builtin_dynamic_object_size

erik.pilkington added a comment. In D56760#1368216 , @rsmith wrote: > I don't see any evidence of that. Jakub said that modes 0-3 should stay > static, but that's in line with what we suggested. I don't think Jakub really said anything about whether we

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

erik.pilkington added inline comments. Comment at: clang/lib/AST/Type.cpp:1295 + + QualType VisitObjCObjectType(const ObjCObjectType *objType) { +if (!objType->isKindOfType()) Does this works with type sugar? i.e. previously calling `Ty->getAs()` would have

[PATCH] D57064: [Sema] Improve a -Warray-bounds diagnostic

erik.pilkington marked an inline comment as done. erik.pilkington added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:2092 + Optional getTypeSizeInCharsIfKnown(QualType Ty) const { +if (Ty->isIncompleteType() || Ty->isDependentType()) + return None;

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

erik.pilkington added a comment. In D52339#1377527 , @arphaman wrote: > @erik.pilkington the change to make `objc_fixed_enum` true in non-ObjC mode > turned out to be problematic for our adoption. Could you please fix it before > LLVM8 is wrapped up so t

[PATCH] D57476: [CodeGenObjC] Use an invoke instead of a call when calling `objc_alloc` or `objc_allocWithZone`

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, pete, ahatanak. Herald added subscribers: dexonsmith, jkorous. `objc_alloc` and `objc_allocWithZone` may throw exceptions if the underlying method does. If we're in a `@try` block, then make sure we emit an `invoke`

[PATCH] D57476: [CodeGenObjC] Use an invoke instead of a call when calling `objc_alloc` or `objc_allocWithZone`

erik.pilkington marked an inline comment as done. erik.pilkington added inline comments. Comment at: clang/test/CodeGenObjC/convert-messages-to-runtime-calls.m:43-44 +void check_invoke() { + // MSGS: {{call.*@objc_msgSend}} + // MSGS: {{call.*@objc_msgSend}} + // CALLS: {{invo

[PATCH] D57476: [CodeGenObjC] Use an invoke instead of a call when calling `objc_alloc` or `objc_allocWithZone`

erik.pilkington updated this revision to Diff 184381. erik.pilkington added a comment. Fix a mistake in the tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57476/new/ https://reviews.llvm.org/D57476 Files: clang/lib/CodeGen/CGObjC.cpp clang/test/CodeGenObjC/convert-messages-to

[PATCH] D57345: Make clang/test/Index/pch-from-libclang.c pass in more places

erik.pilkington resigned from this revision. erik.pilkington added a comment. Sorry, this really seems more like @arphaman's wheelhouse. I'll ping him to take a look though. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57345/new/ https://reviews.llvm.org/D57345

[PATCH] D54010: [CodeGen] Fix a crash when updating a zeroinitialize designated initializer

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, ahatanak. Herald added a subscriber: dexonsmith. LLVM IR, in it's infinite wisdom, doesn't relate `ConstantAggregate` and `ConstantAggregateZero` through inheritance, so make sure we handle both cases here. Fixes a

[PATCH] D54010: [CodeGen] Fix a crash when updating a zeroinitialize designated initializer

erik.pilkington updated this revision to Diff 172306. erik.pilkington marked an inline comment as done. erik.pilkington added a comment. Use `getAggregateElement`. Thanks! https://reviews.llvm.org/D54010 Files: clang/lib/CodeGen/CGExprConstant.cpp clang/test/CodeGen/designated-initializers.

[PATCH] D54010: [CodeGen] Fix a crash when updating a zeroinitialize designated initializer

erik.pilkington added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1142 + llvm_unreachable("Unexpected Base constant type!"); +} rjmccall wrote: > Is `getAggregateElement` not good enough here? No, `getAggregateElement` is perfect,

[PATCH] D54055: CXXNameMangler::mangleFunctionParam: fix assertion

erik.pilkington added a comment. Can you give an example of some code that triggered this with your patch applied? Even if it isn't a real reproducer right now, it would help to try to understand whats happening here. Repository: rC Clang https://reviews.llvm.org/D54055 _

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

erik.pilkington added a comment. Ping! https://reviews.llvm.org/D53522 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

erik.pilkington added a reviewer: erik.pilkington. erik.pilkington added a comment. Have you tried running creduce on the preprocessed source? We should really have a real reproducer for this, otherwise its really hard to tell what the underlying problem is. https://reviews.llvm.org/D54344

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

erik.pilkington added inline comments. Comment at: clang/lib/Lex/ModuleMap.cpp:1198-1203 +// If the header in the module map refers to a symlink, Header.Entry +// refers to the actual file. The callback should be notified of both. +if (!FullPathAsWritten.empty() && +

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

erik.pilkington added a comment. I have a few nits, but I think this looks about right. I reduced this testcase with creduce, can you use the minimized version? Also, I think it makes more sense to put the test into `test/CodeGenCXX` and verify the `-emit-llvm-only` output is correct with FileC

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

erik.pilkington accepted this revision. erik.pilkington added a comment. LGTM too, with one small fix in test. Thanks for working on this! Comment at: test/CodeGenCXX/attr-no-destroy-d54344.cpp:4 +// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu -D

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

erik.pilkington added a comment. In https://reviews.llvm.org/D54344#1294960, @kristina wrote: > In https://reviews.llvm.org/D54344#1294942, @erik.pilkington wrote: > > > LGTM too, with one small fix in test. Thanks for working on this! > > > Well, I figured since the assertion being tripped was (

[PATCH] D54414: [Sema] Make sure we substitute an instantiation-dependent default template parameter

erik.pilkington created this revision. erik.pilkington added a reviewer: rsmith. Herald added a subscriber: dexonsmith. Previously, we'd accept the attached test because we didn't substitute into `void_t` when we really ought to of. Fixes llvm.org/PR39623 Thanks! Repository: rC Clang https

[PATCH] D55039: [sema] Warn of mangling change if function parameters are noexcept.

erik.pilkington added a comment. It seems like this check isn't really doing enough to catch this break in full generality. For instance, the mangling of the following changes from C++14 to 17, but isn't diagnosed here: template struct something {}; void f(something) {} The right thing to

[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions

erik.pilkington added a comment. Hi Bruno, thanks for working on this! Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2370 + "use of this statement in a constexpr %select{function|constructor}0 " + "is incompatible with C++ standards before C++20">, + InGroup, Defaul

[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions

erik.pilkington added a comment. LGTM, but you should probably let @rsmith have the final word! Comment at: lib/Sema/SemaDeclCXX.cpp:1916-1919 +for (Stmt *SubStmt : S->children()) + if (SubStmt && + !CheckConstexprFunctionStmt(SemaRef, Dcl, SubStmt, ReturnStmt

[PATCH] D58744: [CodeGen] Fix some broken IR generated by -fsanitize=unsigned-integer-overflow

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, arphaman, ahatanak. Herald added subscribers: jdoerfert, jfb, dexonsmith, jkorous. Herald added a project: clang. I think the author of the function assumed that `GetInsertBlock()` wouldn't change from where `atomic

[PATCH] D58744: [CodeGen] Fix some broken IR generated by -fsanitize=unsigned-integer-overflow

erik.pilkington updated this revision to Diff 188641. erik.pilkington added a comment. Use FileCheck in the test, NFC. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58744/new/ https://reviews.llvm.org/D58744 Files: clang/lib/CodeGen/CGExprScalar.cpp clang/test/CodeGen/sanitize-atom

[PATCH] D58744: [CodeGen] Fix some broken IR generated by -fsanitize=unsigned-integer-overflow

erik.pilkington marked an inline comment as done. erik.pilkington added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2921 +atomicPHI->addIncoming(old, curBlock); +Builder.CreateCondBr(success, contBB, atomicOpBB); Builder.SetInsertPoint(contBB); --

[PATCH] D58744: [CodeGen] Fix some broken IR generated by -fsanitize=unsigned-integer-overflow

erik.pilkington updated this revision to Diff 188645. erik.pilkington added a comment. Use `atomicPHI->getParent()` instead of tracking the block. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58744/new/ https://reviews.llvm.org/D58744 Files: clang/lib/CodeGen/CGExprScalar.cpp clan

[PATCH] D58757: Add a version of the pass_object_size attribute that works with builtin_dynamic_object_size

erik.pilkington created this revision. erik.pilkington added reviewers: aaron.ballman, george.burgess.iv, rsmith. Herald added subscribers: jdoerfert, kristina, dexonsmith, jkorous. Herald added a project: clang. This attribute, named `pass_dynamic_object_size` has the same semantics as pass_obje

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

erik.pilkington created this revision. erik.pilkington added reviewers: george.burgess.iv, rsmith, aaron.ballman. Herald added subscribers: jdoerfert, dexonsmith, jkorous. Herald added a project: clang. These diagnose overflowing calls to subset of fortifiable functions. Some functions, like `spr

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

erik.pilkington added inline comments. Comment at: clang/test/Parser/objc-static-assert.mm:1 +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s + thakis wrote: > aaron.ballman wrote: > > thakis wrote: > > > aaron.ballman wrote: > > > > Can you try

[PATCH] D59282: [Parse] Parse '#pragma clang attribute' as an external-declaration

erik.pilkington created this revision. erik.pilkington added reviewers: aaron.ballman, arphaman, rsmith. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. Previously, we parsed it only in the top level, which excludes namespaces and `extern "C"` blocks. rdar://problem

[PATCH] D59327: [Sema] Fix a use-after-free of a _Nonnull ParsedAttr

erik.pilkington created this revision. erik.pilkington added reviewers: aaron.ballman, arphaman, rsmith. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. We were allocating the implicit attribute in the declarator's attribute pool, but putting into the declaration spe

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land. LGTM, after one more comment in Features.def :) Comment at: clang/include/clang/Basic/Features.def:121 FEATURE(objc_class_property, LangOpts.ObjC) +FEATURE

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

erik.pilkington added a subscriber: jyknight. erik.pilkington marked 2 inline comments as done. erik.pilkington added a comment. Herald added a subscriber: jdoerfert. I reverted this in r356103: Revert "Add a new attribute, fortify_stdlib" This reverts commit r353765. After talking with ou

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

erik.pilkington added inline comments. Comment at: clang/include/clang/Basic/Features.def:121 FEATURE(objc_class_property, LangOpts.ObjC) +FEATURE(objc_c_static_assert, true) +FEATURE(objc_cxx_static_assert, LangOpts.CPlusPlus11) thakis wrote: > erik.pilkington

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

erik.pilkington added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:319 +// If the parameter has a pass_object_size attribute, then we should use +// it's (potentially) more strict checking mode. Otherwise, conservatively +// assume type 0.

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

erik.pilkington updated this revision to Diff 190549. 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/D58797/new/ https://reviews.llvm.org/D58797 Files: clang/include/cla

[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

erik.pilkington added a comment. In D58514#1429606 , @rjmccall wrote: > There is no way in the existing ABI for copying a block to cause other > references to the block to become references to the heap block. We do do > that for `__block` variables, but

[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

erik.pilkington added a comment. In D58514#1429713 , @rjmccall wrote: > In D58514#1429652 , @erik.pilkington > wrote: > > > In D58514#1429606 , @rjmccall > > wrote: > > > >

[PATCH] D59394: [Sema] De-duplicate some availability checking logic

erik.pilkington created this revision. erik.pilkington added reviewers: rsmith, aaron.ballman, ldionne, dexonsmith. Herald added subscribers: jdoerfert, jkorous. Herald added a project: clang. Right now, we emit unavailable errors for calls to functions during overload resolution, and for referen

[PATCH] D58757: Add a version of the pass_object_size attribute that works with builtin_dynamic_object_size

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

[PATCH] D58757: Add a version of the pass_object_size attribute that works with builtin_dynamic_object_size

erik.pilkington added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1570 +def PassDynamicObjectSize : InheritableParamAttr { + let Spellings = [Clang<"pass_dynamic_object_size">]; + let Args = [IntArgument<"Type">]; aaron.ballman wrote: > Why u

[PATCH] D59394: [Sema] De-duplicate some availability checking logic

erik.pilkington requested review of this revision. erik.pilkington added a comment. In D59394#1430221 , @ldionne wrote: > I can confirm this fixed my original problem. It's also close to the patch I > attempted writing myself earlier this week -- but this

[PATCH] D59523: Thread Safety: also look at ObjC methods

erik.pilkington added a comment. I don't understand this code at all, but what about `BlockDecl`? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59523/new/ https://reviews.llvm.org/D59523 ___ cfe-commits mailing list

[PATCH] D59670: [Sema] Fix an assert when a block captures a constexpr local

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, rsmith. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. `MarkVarDeclODRUsed` indirectly calls `captureInBlock`, which creates a copy expression. The copy expression is insulated in it's

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

erik.pilkington marked 4 inline comments as done. erik.pilkington added a comment. In D58797#1438975 , @nickdesaulniers wrote: > This is causing false positive warnings for the Linux kernel: > https://github.com/ClangBuiltLinux/linux/issues/423 > :( > >

[PATCH] D59670: [Sema] Fix an assert when a block captures a constexpr local

erik.pilkington updated this revision to Diff 192300. erik.pilkington marked an inline comment as done. erik.pilkington added a comment. Add an assert. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59670/new/ https://reviews.llvm.org/D59670 Files: clang/include/clang/Sema/Sema.h cl

[PATCH] D59670: [Sema] Fix an assert when a block captures a constexpr local

erik.pilkington added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:15692 + MaybeODRUseExprSet LocalMaybeODRUseExprs; + std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs); + rjmccall wrote: > It looks like `SmallPtrSet`'s move constructor does actual

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

erik.pilkington added a comment. In D58797#1443411 , @efriedma wrote: > For that particular case, I think we could suppress the false positive using > DiagRuntimeBehavior. How many total false positives are we talking about, > and how many can the compi

[PATCH] D59670: [Sema] Fix an assert when a block captures a constexpr local

erik.pilkington closed this revision. erik.pilkington added a comment. Landed in r357040. (I forgot to write Differential revision:!) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59670/new/ https://reviews.llvm.org/D59670 ___ cfe-commits m

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

erik.pilkington added a comment. Ah, looks like the problem is we're sending a dependent expression to the constant evaluator, we should just bail out in that case. I'll fix this tomorrow morning, thanks for tracking this down! Repository: rC Clang CHANGES SINCE LAST ACTION https://review

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

erik.pilkington added a comment. In D58797#1447148 , @phosek wrote: > This is triggering a Clang assertion failure in Fuchsia build: > > clang/lib/AST/ExprConstant.cpp:5032: bool (anonymous > namespace)::ExprEvaluatorBase<(anonymous > namespace)::Point

[PATCH] D60099: [CodeGen] Fix a regression by emitting lambda expressions in EmitLValue

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, rsmith, ahatanak. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. Currently, we emit a "unsupported l-value" error for the lambda expression in the following: @interface X @proper

[PATCH] D60101: [Sema] Fix a use-after-deallocate of a ParsedAttr

erik.pilkington created this revision. erik.pilkington added a reviewer: aaron.ballman. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. `moveAttrFromListToList` only makes sense when moving an attribute to a list with a pool that's either equivalent, or has a shorter

[PATCH] D24639: [Sema] Warn when returning a lambda that captures a local variable by reference

erik.pilkington added a comment. In D24639#1453795 , @bgianfo wrote: > Is there a reason why this was never merged? > Has the functionality been folded in in some other revision? > > I saw a bug recently which would have been caught by exactly this, which

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

erik.pilkington created this revision. erik.pilkington added reviewers: aaron.ballman, rjmccall. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. We want to make `objc_nonlazy_class` apply to implementations, but ran into this. There doesn't seem to be any reason that

[PATCH] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, aaron.ballman. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. Fixes rdar://49523079 Repository: rC Clang https://reviews.llvm.org/D60544 Files: clang/include/clang/Basic/Attr.td

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

erik.pilkington marked an inline comment as done. erik.pilkington added inline comments. Comment at: clang/test/FixIt/fixit-pragma-attribute.cpp:19-20 #pragma clang attribute push (__attribute__((annotate("subRuleContradictions"))), apply_to = any(variable, variable(is_paramet

[PATCH] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

erik.pilkington added a comment. In D60544#1462869 , @rjmccall wrote: > Should we have a special `has_feature` test to check that this attribute is > allowed on implementations? We've done that in the past when expanding the > set of subjects for an att

[PATCH] D60736: [Sema][ObjC] Don't warn about a block implicitly retaining self if the block is marked noescape

erik.pilkington added a comment. Akira and I were just talking about an alternative approach to this: Keep a vector of pairs of BlockDecls and SourceLocations in the enclosing method's FunctionScopeInfo, and emit any unsuppressed diagnostics when popping the method. This would avoid having to

[PATCH] D60736: [Sema][ObjC] Don't warn about a block implicitly retaining self if the block is marked noescape

erik.pilkington added inline comments. Comment at: lib/Parse/ParseObjc.cpp:3696-3699 + + Actions.ActOnEndOfObjCMethodDef(); + // Clean up the remaining EOF token. Any reason not to do this check in `ActOnFinishFunctionBody` (which is called by ParseFunctionS

[PATCH] D60736: [Sema][ObjC] Don't warn about a block implicitly retaining self if the block is marked noescape

erik.pilkington accepted this revision. erik.pilkington added a comment. LGTM too, thanks! Comment at: lib/Sema/SemaDecl.cpp:13169 +do { + R = R || !CurBD->doesNotEscape(); + if (R) This can just be R = !CurBD->doesNotEscape(); Repository: rC

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

erik.pilkington added a comment. Wait, why is NumTypos incorrect here? I think its because we don't handle the typo on the line: `[self undeclaredMethod:undeclaredArg];`, even the following asserts now. Seems like the right fix would be to track down why we aren't handling the typo in the messa

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

erik.pilkington added a comment. > but don't want to annotate each unused ObjC method parameters with > `__attribute__((unused))` or insert `(void)unused_param` into the body of the > method to silence the warning since, unlike C/C++ functions, it's not > possible to comment out the unused para

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

erik.pilkington added a comment. > Yeah, I tend to think we should just suppress this unconditionally in > Objective-C. IMO this warning still makes sense for normal functions, or methods that are only declared in an @implementation. Adding a fix-it to cast to void in the function/method body

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

erik.pilkington added a comment. In D61147#1479530 , @ahatanak wrote: > In D61147#1479468 , @erik.pilkington > wrote: > > > > Yeah, I tend to think we should just suppress this unconditionally in > > > Objective-C

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

erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land. Huh, okay. I guess there are two separate bugs that are conspiring to crash the compiler here: we shouldn't correct a TypoExpr more than once, and we shouldn't correct a Typo

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

erik.pilkington created this revision. erik.pilkington added reviewers: rjmccall, rsmith, jfb. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. Previously, we didn't mark an array's element type's destructor referenced when it was annotated with no_destroy. This is no

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

erik.pilkington abandoned this revision. erik.pilkington added a comment. Herald added a subscriber: jkorous. Looks like @rsmith fixed this in r359266. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52521/new/ https://reviews.llvm.org/D52521 ___

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

erik.pilkington added a comment. In D61147#1479932 , @rjmccall wrote: > I do not think the ObjC version of this diagnostic is useful as it's been > explained here, and I would strongly recommend just not including such a > warning for the time being. W

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

erik.pilkington added a comment. In D61147#1479956 , @rjmccall wrote: > In D61147#1479940 , @erik.pilkington > wrote: > > > In D61147#1479932 , @rjmccall > > wrote: > > > >

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

erik.pilkington added a comment. JF, Michael, and I were talking about this offline, and we think that the right choice of semantics for the static local case is to call the destructors. struct HoldsResource { HoldsResource() { tryToAcquireItMayThrow(); } ~HoldsResource() { releaseIt()

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

erik.pilkington added a comment. By using `no_destroy`, you're saying that exit-time destructor doesn't matter because the OS will either reclaim the resources automatically, or its just doesn't matter since the process is going down. I don't think that implies that we can safely ignore the des

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

erik.pilkington planned changes to this revision. erik.pilkington added a comment. In D61165#1480976 , @rjmccall wrote: > I believe at least one of the goals of `nodestroy` is to allow the type to > potentially not provide a destructor at all, so if we're

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

erik.pilkington added a comment. No worries! It happens, I probably should have pinged this more aggressively. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52521/new/ https://reviews.llvm.org/D52521 ___ cfe-commits mailing list cfe-commits

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

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

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

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

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

erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61147/new/ https://reviews.llvm.org/D61147 ___

<    1   2   3   4   5   6   >