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

2019-07-12 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-07-12 Thread Erik Pilkington via Phabricator via cfe-commits
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:

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

2019-07-11 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-07-08 Thread Erik Pilkington via Phabricator via cfe-commits
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] D63912: [ObjC] Add a warning for implicit conversions of a constant non-boolean value to BOOL

2019-07-08 Thread Erik Pilkington via Phabricator via cfe-commits
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:

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

2019-07-08 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment. In D63856#1561160 , @rjmccall wrote: > In D63856#1561132 , @erik.pilkington > wrote: > > > In D63856#1561112 , @rjmccall > > wrote: > > >

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

2019-07-08 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-07-08 Thread Erik Pilkington via Phabricator via cfe-commits
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

[PATCH] D63954: Add lifetime categories attributes

2019-06-28 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-06-27 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-06-27 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment. In D63856#1561112 , @rjmccall wrote: > In D63856#1560213 , @erik.pilkington > wrote: > > > In D63856#1560180 , @rjmccall > > wrote: > > >

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

2019-06-27 Thread Erik Pilkington via Phabricator via cfe-commits
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`,

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

2019-06-26 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-06-24 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-06-24 Thread Erik Pilkington via Phabricator via cfe-commits
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:

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

2019-06-19 Thread Erik Pilkington via Phabricator via cfe-commits
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:

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

2019-06-19 Thread Erik Pilkington via Phabricator via cfe-commits
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] D62960: SVE opaque type for C intrinsics demo

2019-06-19 Thread Erik Pilkington via Phabricator via cfe-commits
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

2019-06-13 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-06-13 Thread Erik Pilkington via Phabricator via cfe-commits
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:

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

2019-06-11 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-06-10 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-06-05 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-06-05 Thread Erik Pilkington via Phabricator via cfe-commits
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:

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

2019-06-03 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-06-03 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-05-15 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-05-15 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-05-10 Thread Erik Pilkington via Phabricator via cfe-commits
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] D61803: [CodeGen][ObjC] Emit invoke instead of call to call `objc_release` when necessary.

2019-05-10 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added inline comments. Comment at: lib/CodeGen/CGObjC.cpp:2634-2646 + ASTContext = getContext(); + const ImplicitParamDecl *paramDecl = + ImplicitParamDecl::Create(Ctx, nullptr, SourceLocation(), nullptr, +Ctx.VoidPtrTy,

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

2019-05-09 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-05-09 Thread Erik Pilkington via Phabricator via cfe-commits
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:

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

2019-05-09 Thread Erik Pilkington via Phabricator via cfe-commits
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 && +

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

2019-05-09 Thread Erik Pilkington via Phabricator via cfe-commits
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

2019-05-09 Thread Erik Pilkington via Phabricator via cfe-commits
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

2019-05-06 Thread Erik Pilkington via Phabricator via cfe-commits
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 >

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

2019-05-03 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-05-03 Thread Erik Pilkington via Phabricator via cfe-commits
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.

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

2019-05-03 Thread Erik Pilkington via Phabricator via cfe-commits
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

2019-05-02 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-05-01 Thread Erik Pilkington via Phabricator via cfe-commits
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

2019-04-30 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-04-30 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-04-30 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-04-30 Thread Erik Pilkington via Phabricator via cfe-commits
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] D52521: [Sema] DR727: Ensure we pick up enclosing template instantiation arguments for a class-scope explicit specialization.

2019-04-26 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-04-26 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-04-26 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-04-26 Thread Erik Pilkington via Phabricator via cfe-commits
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() {

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

2019-04-26 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment. In D61147#1479956 , @rjmccall wrote: > In D61147#1479940 , @erik.pilkington > wrote: > > > In D61147#1479932 , @rjmccall > > wrote: > > >

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

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
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.

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

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
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] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
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 > > >

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

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
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

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-04-19 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-04-17 Thread Erik Pilkington via Phabricator via cfe-commits
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] D60736: [Sema][ObjC] Don't warn about a block implicitly retaining self if the block is marked noescape

2019-04-17 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-04-15 Thread Erik Pilkington via Phabricator via cfe-commits
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] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

2019-04-11 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-04-11 Thread Erik Pilkington via Phabricator via cfe-commits
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,

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

2019-04-10 Thread Erik Pilkington via Phabricator via cfe-commits
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:

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

2019-04-10 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-04-03 Thread Erik Pilkington via Phabricator via cfe-commits
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,

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

2019-04-01 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-04-01 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-03-29 Thread Erik Pilkington via Phabricator via cfe-commits
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 >

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

2019-03-28 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-03-26 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-03-26 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-03-26 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-03-26 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-03-22 Thread Erik Pilkington via Phabricator via cfe-commits
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

2019-03-21 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-03-18 Thread Erik Pilkington via Phabricator via cfe-commits
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] D59394: [Sema] De-duplicate some availability checking logic

2019-03-18 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-03-15 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-03-15 Thread Erik Pilkington via Phabricator via cfe-commits
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:

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

2019-03-14 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-03-14 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment. In D58514#1429713 , @rjmccall wrote: > In D58514#1429652 , @erik.pilkington > wrote: > > > In D58514#1429606 , @rjmccall > > wrote: > > >

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

2019-03-14 Thread Erik Pilkington via Phabricator via cfe-commits
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,

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

2019-03-13 Thread Erik Pilkington via Phabricator via cfe-commits
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:

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

2019-03-13 Thread Erik Pilkington via Phabricator via cfe-commits
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] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

2019-03-13 Thread Erik Pilkington via Phabricator via cfe-commits
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] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

2019-03-13 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-03-13 Thread Erik Pilkington via Phabricator via cfe-commits
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)

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

2019-03-13 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-03-12 Thread Erik Pilkington via Phabricator via cfe-commits
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.

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

2019-03-11 Thread Erik Pilkington via Phabricator via cfe-commits
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] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

2019-02-28 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-02-27 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-02-27 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-02-27 Thread Erik Pilkington via Phabricator via cfe-commits
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

2019-02-27 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-02-27 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-02-14 Thread Erik Pilkington via Phabricator via cfe-commits
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] D57270: [ObjC] Fix non-canonical types preventing type arguments substitution.

2019-02-14 Thread Erik Pilkington via Phabricator via cfe-commits
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

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

2019-02-14 Thread Erik Pilkington via Phabricator via cfe-commits
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

  1   2   3   4   5   >