[PATCH] D22391: [Sema] Add warning for implicitly casting a null constant to a non null pointer type

2017-02-03 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 87016. ahatanak marked 2 inline comments as done. ahatanak added a comment. Turning the warning on by default caused clang to issue warnings in many other cases, including Objective-C methods returning nil, which was something r240153 tried to avoid. If we

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2017-01-31 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL293678: [Sema] Transform a templated name before looking it up in (authored by ahatanak). Changed prior to commit: https://reviews.llvm.org/D24969?vs=82134=86474#toc Repository: rL LLVM

[PATCH] D25556: [Sema] Add variable captured by a block to the enclosing lambda's potential capture list

2017-02-02 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 86918. ahatanak added a comment. Rebase https://reviews.llvm.org/D25556 Files: include/clang/Sema/Sema.h lib/Sema/Sema.cpp lib/Sema/SemaExpr.cpp test/SemaObjCXX/blocks.mm Index: test/SemaObjCXX/blocks.mm

[PATCH] D29542: [TargetInfo] Adjust x86-32 atomic support to the CPU used

2017-02-06 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: lib/Basic/Targets.cpp:4244 +} else // allow locked atomics up to 4 bytes + MaxAtomicPromoteWidth = 32; + } mgorny wrote: > dim wrote: > > Are you purposefully not setting `MaxAtomicInlineWidth` here? (It

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

2017-01-24 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: lib/CodeGen/CodeGenFunction.h:217 + /// statements. + llvm::SmallVector LabelSeenStack; + ahatanak wrote: > rjmccall wrote: > > Shouldn't this be maintained by some existing scoping structure like > >

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

2017-01-24 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: lib/CodeGen/CodeGenFunction.h:217 + /// statements. + llvm::SmallVector LabelSeenStack; + rjmccall wrote: > Shouldn't this be maintained by some existing scoping structure like > LexicalScope? I think I

[PATCH] D22391: [Sema] Add warning for implicitly casting a null constant to a non null pointer type

2017-01-24 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:290 def NullableToNonNullConversion : DiagGroup<"nullable-to-nonnull-conversion">; +def NullConstToNonnull : DiagGroup<"null-const-to-nonnull">; def NullabilityCompletenessOnArrays :

[PATCH] D28050: [Clang][Driver] Clean up Clang::ConstructJob a little bit, NFC

2017-01-30 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. I think this patch is an improvement, but Clang::ConstructJob is still one giant function. Do you have ideas to improve readability of this function or plans to further reduce its size? Repository: rL LLVM https://reviews.llvm.org/D28050

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

2017-01-25 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL293106: [CodeGen] Suppress emission of lifetime markers if a label has been seen (authored by ahatanak). Changed prior to commit: https://reviews.llvm.org/D27680?vs=85774=85817#toc Repository: rL

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

2017-01-25 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 85774. ahatanak marked 2 inline comments as done. ahatanak added a comment. Yes, we can make VarBypassDetector detect variables declared after labels too if we want to put all the logic for disabling lifetime markers in one place.

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

2017-01-24 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 85643. ahatanak marked an inline comment as done. ahatanak added a comment. To decide whether lifetime markers should be disabled, check whether the current LexicalScope has labels instead of introducing a new structure that keeps track of labels seen in

[PATCH] D22391: [Sema] Add warning for implicitly casting a null constant to a non null pointer type

2017-01-24 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 85651. ahatanak added a reviewer: jordan_rose. ahatanak added a comment. Rebase. https://reviews.llvm.org/D22391 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/Sema.cpp

[PATCH] D30174: [Sema][ObjC] Warn about 'performSelector' calls with selectors that return record types

2017-02-21 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. I'm not sure how common it is to pass a function that doesn't return an object or void, I think it's OK to allow returning primitive types if you think being too strict would catch too many false positives. Comment at: lib/AST/DeclObjC.cpp:987

[PATCH] D30166: Honor __unaligned in codegen for declarations and expressions

2017-02-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: include/clang/AST/ASTContext.h:1910 +if (T.getQualifiers().hasUnaligned()) + TI.Align = 8; +return TI; Is it better to call TargetInfo::getCharWidth() instead of assigning a hardcoded number here?

[PATCH] D30174: [Sema][ObjC] Warn about 'performSelector' calls with selectors that return record types

2017-02-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Do we still issue a warning even when the struct can be returned in a register? For example, x86 can return a small struct (for example, a struct with one int field) in a single register, in which case it's fine to pass it to performSelector via @selector. If we

[PATCH] D30174: [Sema][ObjC] Warn about 'performSelector' calls with selectors that return record types

2017-02-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. In https://reviews.llvm.org/D30174#681843, @arphaman wrote: > Yes, we do. Primarily for the following reason: even if some target may > return a struct in a register, another target isn't guaranteed to do the same > thing. It's better to always warn about this rather

[PATCH] D29908: Disallow returning a __block variable via a move

2017-02-14 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Posting John's review comment, which was sent to the review I abandoned: "Hmm. The behavior I think we actually want here is that we should move out of the __block variable but not perform NRVO, essentially like we would if the variable were a parameter. The block

[PATCH] D29908: Disallow returning a __block variable via a move

2017-02-14 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. In https://reviews.llvm.org/D29908#676589, @ahatanak wrote: > Posting John's review comment, which was sent to the review I abandoned: > > "Hmm. The behavior I think we actually want here is that we should move out > of the __block variable but not perform NRVO,

[PATCH] D29908: Disallow returning a __block variable via a move

2017-02-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak created this revision. This patch disables moving a __block variable to return it from a function, which was causing a crash that occurred when compiling and executing the following code: #include #include #include class A { public: A(int x) : internal(x) {}

[PATCH] D29908: Disallow returning a __block variable via a move

2017-02-14 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. In https://reviews.llvm.org/D29908#676788, @rjmccall wrote: > Oh, I see. Just to clarify: it doesn't matter that the object "ret" used to > point to has been destructed, what matters is that "ret" now holds a null > reference because it's been moved out of. Yes,

[PATCH] D29908: Disallow returning a __block variable via a move

2017-02-14 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 88430. ahatanak added a comment. Update comment. https://reviews.llvm.org/D29908 Files: lib/Sema/SemaStmt.cpp test/SemaObjCXX/blocks.mm Index: test/SemaObjCXX/blocks.mm === ---

[PATCH] D29599: Clang Changes for alloc_align

2017-02-14 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. My question probably wasn't clear, but I wasn't sure how template functions in general (not just member functions) should be handled. I see a warning when the following function is compiled: template T __attribute__((alloc_align(1))) foo0(int a) { typedef

[PATCH] D29908: Disallow returning a __block variable via a move

2017-02-14 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL295150: [Sema] Disallow returning a __block variable via a move. (authored by ahatanak). Changed prior to commit: https://reviews.llvm.org/D29908?vs=88430=88486#toc Repository: rL LLVM

[PATCH] D29599: Clang Changes for alloc_align

2017-02-12 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Can this attribute be used on c++ template methods? Is the following code valid? template struct S { T foo(int a) __attribute__((alloc_align(1))); }; https://reviews.llvm.org/D29599 ___ cfe-commits mailing list

[PATCH] D29755: Cache FileID when translating diagnostics in PCH files

2017-02-12 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: lib/Frontend/ASTUnit.cpp:2551 continue; -FileID FID = SrcMgr.translateFile(FE); +FileID FID; +if (FE == CachedFE) { Since FID is always equal to CachedFID, I think you can simplify this a bit by

[PATCH] D29724: [Driver] Report available language standards on user error

2017-02-09 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Test case? Comment at: lib/Frontend/CompilerInvocation.cpp:1709 +Diags.Report(diag::note_drv_supported_value_with_description) + << Std.getName() << Std.getDescription(); + } Is it possible to change the

[PATCH] D28514: [CodeCompletion] Reset the hidden declaration obtained after lookup when caching UsingShadowDecls

2017-01-23 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak accepted this revision. ahatanak added a comment. This revision is now accepted and ready to land. Thanks LGTM. Repository: rL LLVM https://reviews.llvm.org/D28514 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D28467: [Sema] Add warning for unused lambda captures

2017-01-18 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. In https://reviews.llvm.org/D28467#649861, @krasin wrote: > This change makes Clang hardly incompatible with MSVC++. Consider the > following program: > > #include > > int main(void) { > const int kDelta = 1001; > auto g = [kDelta](int i) >

[PATCH] D25001: [Module] Merge function prototype with a non-prototype function declaration

2017-01-17 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Sorry for the delay in replying. Comment at: lib/Serialization/ASTReaderDecl.cpp:2715 return (FuncX->getLinkageInternal() == FuncY->getLinkageInternal()) && FuncX->getASTContext().hasSameType(FuncX->getType(), FuncY->getType()); }

[PATCH] D28705: [Sema] Fix bug in handling of designated initializer

2017-01-17 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL292245: [Sema] Fix bug in handling of designated initializer. (authored by ahatanak). Changed prior to commit: https://reviews.llvm.org/D28705?vs=84418=84714#toc Repository: rL LLVM

[PATCH] D28514: [CodeCompletion] Reset the hidden declaration obtained after lookup when caching UsingShadowDecls

2017-01-17 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:961 +// then incorrectly applied to the target declaration. This can be avoided +// by resetting the declaration that's being hidden. +if (Hiding && isa(Hiding)) I'm not sure

[PATCH] D28514: [CodeCompletion] Reset the hidden declaration obtained after lookup when caching UsingShadowDecls

2017-01-19 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Looks good to me, but one more comment/question. Comment at: lib/Sema/SemaLookup.cpp:3433 + // is not hidden by the using declaration. + if (isa(ND) && isa(D)) +continue; Do we have to check that ND's UsingDecl is

[PATCH] D25556: [Sema] Add variable captured by a block to the enclosing lambda's potential capture list

2017-01-19 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added reviewers: ABataev, malcolm.parsons. ahatanak added a comment. Add a few more people who have looked at this part of clang in the past to the reviewers list. https://reviews.llvm.org/D25556 ___ cfe-commits mailing list

[PATCH] D28514: [CodeCompletion] Reset the hidden declaration obtained after lookup when caching UsingShadowDecls

2017-01-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. This is not invalid: namespace Foo { class C { }; } namespace Bar { class C { }; } void foo1() { using Foo::C; { using Bar::C; } } Repository: rL LLVM https://reviews.llvm.org/D28514 ___

[PATCH] D28514: [CodeCompletion] Reset the hidden declaration obtained after lookup when caching UsingShadowDecls

2017-01-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. I had something like the following code in mind (which is not invalid). Does usingdecl Bar::C hide Foo::C ? namespace Foo { class C { }; } namespace Bar { class C { }; } using Foo::C; { using Bar::C; } Repository: rL LLVM

[PATCH] D28514: [CodeCompletion] Reset the hidden declaration obtained after lookup when caching UsingShadowDecls

2017-01-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. In the example I showed, wouldn't the DeclContext for all the UsingDecls and UsingShadowDecls be the FunctionDecl for foo1? Maybe you can check whether UsingShadowDecl::getUsingDecl() (which returns the UsingDecl which is tied to the UsingShadowDecl) is equal to the

[PATCH] D28514: [CodeCompletion] Reset the hidden declaration obtained after lookup when caching UsingShadowDecls

2017-01-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. If they are equal, the loop can continue because a UsingDecl doesn't hide a UsingShadowDecl that is tied to it. Repository: rL LLVM https://reviews.llvm.org/D28514 ___ cfe-commits mailing list

[PATCH] D30372: [Driver] Consolidate tools and toolchains by target platform. (NFC)

2017-02-26 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Have you considered using "include_directories" in CMakeLists.txt to avoid including "../Something.h"? https://reviews.llvm.org/D30372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30372: [Driver] Consolidate tools and toolchains by target platform. (NFC)

2017-02-27 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. In https://reviews.llvm.org/D30372#687054, @dlj wrote: > In https://reviews.llvm.org/D30372#686871, @ahatanak wrote: > > > Have you considered using "include_directories" in CMakeLists.txt to avoid > > including "../Something.h"? > > > I don't want to take that

[PATCH] D25556: [Sema] Add variable captured by a block to the enclosing lambda's potential capture list

2017-02-28 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL296584: [Sema] Add variable captured by a block to the enclosing lambda's (authored by ahatanak). Changed prior to commit: https://reviews.llvm.org/D25556?vs=86918=90123#toc Repository: rL LLVM

[PATCH] D26922: [ObjC++] Don't enter a C++ declarator context when the current context is an Objective-C declaration

2016-11-28 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. I wonder whether it is possible to avoid calling DeclScopeObj.EnterDeclaratorScope at ParseDecl.cpp:5317 instead of fixing Sema::ActOnCXXEnterDeclaratorScope? I believe it's calling EnterDeclaratorScope to enable name lookup when a namespace-member variable is

[PATCH] D26800: [Sema] Make __attribute__((notnull)) inheritable through function parameters

2016-11-28 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: test/Sema/nonnull.c:171 + +void pr31040(char *p __attribute__((nonnull))); +void pr31040(char *p) {} Is this not pr30828? https://reviews.llvm.org/D26800 ___ cfe-commits

[PATCH] D21453: Add support for attribute "overallocated"

2016-12-08 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: include/clang/AST/Decl.h:3250 + /// This is true if this struct ends with an array marked 'flexible_array'. + bool HasFlexibleArrayAttr : 1; + rsmith wrote: > How is this different from `HasFlexibleArrayMember`? Do

[PATCH] D21453: Add support for attribute "overallocated"

2016-12-09 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: include/clang/AST/Decl.h:3250 + /// This is true if this struct ends with an array marked 'flexible_array'. + bool HasFlexibleArrayAttr : 1; + ahatanak wrote: > rsmith wrote: > > How is this different from

[PATCH] D23096: [Sema] Pass CombineWithOuterScope = true to constructor of LocalInstantiationScope

2016-12-12 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 81171. ahatanak added a comment. Rebase and ping. https://reviews.llvm.org/D23096 Files: lib/Sema/SemaTemplate.cpp lib/Sema/SemaTemplateInstantiate.cpp lib/Sema/TreeTransform.h test/SemaCXX/vartemplate-lambda.cpp

[PATCH] D27478: Make ASTContext::getDeclAlign return the correct alignment for FunctionDecls

2016-12-12 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. This doesn't happen for objective-c block pointers. Sema::CheckCastAlign returns early when the type isn't a PointerType (BlockPointerType isn't a subclass of PointerType). https://reviews.llvm.org/D27478 ___ cfe-commits

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-12-12 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 81172. ahatanak added a comment. Rebase and ping. https://reviews.llvm.org/D24969 Files: lib/Sema/SemaTemplateInstantiateDecl.cpp lib/Sema/TreeTransform.h test/SemaCXX/destructor.cpp Index: test/SemaCXX/destructor.cpp

[PATCH] D27800: Add overload of TransformToPotentiallyEvaluated for TypeSourceInfo

2016-12-15 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Can you add a test case? https://reviews.llvm.org/D27800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27424: Add the diagnose_if attribute to clang.

2016-12-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: include/clang/Sema/Overload.h:754 +unsigned NumInlineBytesUsed; +llvm::AlignedCharArray InlineSpace; I guess you could use "void *" instead of

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

2016-12-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 81284. ahatanak added a comment. Address review comment. Use llvm::is_contained instead of std::find. https://reviews.llvm.org/D27680 Files: lib/CodeGen/CGDecl.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGStmt.cpp lib/CodeGen/CodeGenFunction.h

[PATCH] D27299: [Sema] C++11 opaque enums should avoid the "case value not in enumerated type" switch warning

2016-12-06 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak accepted this revision. ahatanak added a comment. This revision is now accepted and ready to land. That makes sense. Thanks for the explanation. Repository: rL LLVM https://reviews.llvm.org/D27299 ___ cfe-commits mailing list

[PATCH] D27478: Make ASTContext::getDeclAlign return the correct alignment for FunctionDecls

2016-12-06 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak created this revision. ahatanak added reviewers: rsmith, doug.gregor. ahatanak added a subscriber: cfe-commits. This patch silences an incorrect warning that is issued when a function pointer is cast to another function pointer type. The warning gets issued because alignments of the

[PATCH] D27478: Make ASTContext::getDeclAlign return the correct alignment for FunctionDecls

2016-12-06 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 80464. ahatanak added a comment. Call getTypeInfoImpl from getDeclAlign to get the alignment of functions. https://reviews.llvm.org/D27478 Files: lib/AST/ASTContext.cpp test/Sema/warn-cast-align.c Index: test/Sema/warn-cast-align.c

[PATCH] D27837: Add fix-it notes to the nullability consistency warning

2016-12-16 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8772 +def note_nullability_fix_it : Note< + "insert '%select{_Nonnull|_Nullable|_Null_unspecified}0' if the " + "%select{pointer|block pointer|member pointer|array parameter}1 "

[PATCH] D23096: [Sema] Pass CombineWithOuterScope = true to constructor of LocalInstantiationScope

2016-12-16 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL289990: [Sema] Transform the default arguments of a lambda expression when the (authored by ahatanak). Changed prior to commit: https://reviews.llvm.org/D23096?vs=81171=81787#toc Repository: rL LLVM

[PATCH] D21453: Add support for attribute "overallocated"

2016-12-12 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 81090. ahatanak added a comment. Address review comments. - Arrays marked "flexible_array" are now treated as flexible arrays. - __builtin_object_size returns a more accurate numbers for normal C99 flexible arrays (see test/CodeGen/object-size.c). Note

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

2016-12-12 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a subscriber: cfe-commits. ahatanak added a comment. Add cfe-commits. https://reviews.llvm.org/D27680 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27607: [ubsan] Treat ObjC's BOOL as if its range is always {0, 1}

2016-12-09 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:1221 +/// Check if \p Ty is defined as BOOL in a system header. In ObjC language +/// modes, it's safe to treat such a type as 'the builtin bool'. +static bool isObjCBool(QualType Ty, const SourceManager ,

[PATCH] D28231: -Wunreachable-code: Avoid multiple diagnostics that are triggered by the same source range and fix the unary operator fixit source range

2017-01-11 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak accepted this revision. ahatanak added a comment. This revision is now accepted and ready to land. Thanks, LGTM Repository: rL LLVM https://reviews.llvm.org/D28231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D28705: [Sema] Fix bug in handling of designated initializer

2017-01-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak created this revision. ahatanak added reviewers: rsmith, rnk, arphaman. ahatanak added a subscriber: cfe-commits. CheckDesignatedInitializer wasn't taking into account the base classes when computing the index for the field in the derived class, which caused the test case to crash

[PATCH] D28457: Add missing const qualifier to member function Decl::isLocalExternDecl()

2017-01-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak accepted this revision. ahatanak added a reviewer: ahatanak. ahatanak added a comment. This revision is now accepted and ready to land. This looks OK to commit. https://reviews.llvm.org/D28457 ___ cfe-commits mailing list

[PATCH] D28705: [Sema] Fix bug in handling of designated initializer

2017-01-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. In https://reviews.llvm.org/D28705#646088, @rnk wrote: > What happens with virtual bases? > > struct B { int x; }; > struct D : virtual B { int y; }; > void test() { D d = {1, .y = 2}; } A class with virtual base is not considered an aggregate, so it doesn't go

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

2017-01-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 84434. ahatanak added a comment. Remove lifetime markers completely for variables that are declared after a label was seen in a compound statement I tested this patch building llvm's test-suite and SPEC benchmarks with -Os. Surprisingly (maybe not so

[PATCH] D28705: [Sema] Fix bug in handling of designated initializer

2017-01-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 84418. ahatanak marked 2 inline comments as done. ahatanak added a comment. Address review comments. https://reviews.llvm.org/D28705 Files: lib/Sema/SemaInit.cpp test/SemaCXX/designated-initializers-base-class.cpp Index:

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

2017-01-09 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 83680. ahatanak added a comment. Herald added a subscriber: mgorny. I added an AST analysis pass that is run before IRGen and decides which VarDecls need their lifetimes to be extended or disabled. It only looks at VarDecls which have their addresses taken

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2017-01-09 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Richard, does the updated patch look OK? https://reviews.llvm.org/D24969 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-01-11 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. In https://reviews.llvm.org/D27680#642770, @rjmccall wrote: > Interesting. That's a pretty heavyweight solution, and you're still missing > switches, which have exactly the same "can jump into an arbitrary variable's > scope" behavior. In addition to missing the

[PATCH] D21099: [Sema] Teach -Wcast-align to look at the aligned attribute of the declared variables

2016-11-30 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL288267: [Sema] Teach -Wcast-align to look at the aligned attribute of the (authored by ahatanak). Changed prior to commit: https://reviews.llvm.org/D21099?vs=78594=79792#toc Repository: rL LLVM

[PATCH] D26922: [ObjC++] Don't enter a C++ declarator context when the current context is an Objective-C declaration

2016-12-01 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak accepted this revision. ahatanak added a comment. This revision is now accepted and ready to land. I think this is fine. I guess we can print a more helpful error message than "error: property requires fields to be named", but we can probably do it later. Repository: rL LLVM

[PATCH] D27304: [PATCH] [Sema][X86] Don't allow floating-point return types when SSE is disabled

2016-12-01 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. In https://reviews.llvm.org/D27304#610944, @thegameg wrote: > In https://reviews.llvm.org/D27304#610697, @joerg wrote: > > > I think this is the absolutely wrong place to put such logic. It really can > > not be anywhere but the backend. > > > I am aware of this. But

[PATCH] D26922: [ObjC++] Don't enter a C++ declarator context when the current context is an Objective-C declaration

2016-11-29 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Yes, I meant ParseDecl:5266. Reading the comment in ShouldEnterDeclaratorScope, it seemed to me that it shouldn't return true in this context (when parsing an objective-c). Also, the following code (which is not valid) crashes with ToT trunk, @property (nonatomic)

[PATCH] D27214: [ObjC] Encode type arguments in property information string constants

2016-11-30 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Do you know whether this patch would have any effect on the objective-c runtime? I suspect it wouldn't cause the runtime to crash, but I don't know how the runtime uses the encoded type information. Other than that, the changes made in this patch look good to me.

[PATCH] D27299: [Sema] C++11 opaque enums should avoid the "case value not in enumerated type" switch warning

2016-12-01 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: lib/Sema/SemaStmt.cpp:1165 if (!hasCasesNotInSwitch) SS->setAllEnumCasesCovered(); This function used to call setAllEnumCasesCovered() when parsing a switch statement with an opaque enum condition,

[PATCH] D28231: -Wunreachable-code: Avoid multiple diagnostics that are triggered by the same source range and fix the unary operator fixit source range

2017-01-05 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: lib/Analysis/ReachableCode.cpp:229 + if (SilenceableCondValNotSet && SilenceableCondVal->getBegin().isValid()) +*SilenceableCondVal = UO->getSourceRange(); + return UO->getOpcode() == UO_LNot && IsSubExprConfigValue;

[PATCH] D27478: Make ASTContext::getDeclAlign return the correct alignment for FunctionDecls

2017-01-06 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL291253: Make ASTContext::getDeclAlign return the correct alignment for (authored by ahatanak). Changed prior to commit: https://reviews.llvm.org/D27478?vs=80464=83378#toc Repository: rL LLVM

[PATCH] D28344: [AVR] Add support for the full set of inline asm constraints

2017-01-06 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Test case? Comment at: lib/Basic/Targets.cpp:8506 + case 'N': // Integer constant (Range: -1) +Info.setRequiresImmediate(-1); + case 'O': // Integer constant (Range: 8, 16, 24) Is this meant to fall through or do you

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

2017-01-03 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:917 + if (!InsertPt) +Builder.SetInsertPoint(BB, BB->begin()); + // If InsertPt is a terminator, insert it before InsertPt. ahatanak wrote: > rjmccall wrote: > > BB->begin() is

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

2017-01-03 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:917 + if (!InsertPt) +Builder.SetInsertPoint(BB, BB->begin()); + // If InsertPt is a terminator, insert it before InsertPt. rjmccall wrote: > BB->begin() is not necessarily a

[PATCH] D27478: Make ASTContext::getDeclAlign return the correct alignment for FunctionDecls

2017-01-03 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. ping https://reviews.llvm.org/D27478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-01-04 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. I found another problem with the current patch: it can generate IR that doesn't pass asan's use-after-scope check. For example, IRGen generates the following IR for function move_lifetime_start in lifetime2.c when this patch is applied: entry: %i = alloca i32,

[PATCH] D25556: [Sema] Add variable captured by a block to the enclosing lambda's potential capture list

2017-01-03 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. ping https://reviews.llvm.org/D25556 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28231: -Wunreachable-code: Avoid multiple diagnostics that are triggered by the same source range and fix the unary operator fixit source range

2017-01-03 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. If I change the condition to the following, if (!s->p || 1) clang suggests enclosing !s->p with a parenthesis, but the comment in ReachableCode.cpp says the parenthesis should enclose the integer literal. It seems like there is a contradiction here? Repository:

[PATCH] D28296: [ObjC] The declarator for a block literal should be a definition

2017-01-05 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak accepted this revision. ahatanak added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D28296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D25206: [Parser] Correct typo after lambda capture initializer is parsed

2016-12-19 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Thanks all for the review. Comment at: cfe/trunk/test/SemaCXX/lambda-expressions.cpp:572 +void foo1() { + auto s0 = S1{[name=]() {}}; // expected-error 2 {{expected expression}} + auto s1 = S1{[name=name]() {}}; // expected-error {{use of undeclared

[PATCH] D25206: [Parser] Correct typo after lambda capture initializer is parsed

2016-12-19 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL290156: [Parser] Correct typo after lambda capture initializer is parsed. (authored by ahatanak). Changed prior to commit: https://reviews.llvm.org/D25206?vs=75819=82052#toc Repository: rL LLVM

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-12-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 82120. ahatanak added a comment. Call TransformDeclarationNameInfo unconditionally to transform NameInfo rather than doing so only for destructors. I think this is a cleaner fix. https://reviews.llvm.org/D24969 Files:

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

2016-12-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 82142. ahatanak added a comment. Rebase and improve test cases. - Add "-mllvm -disable-llvm-optzns" to the RUN line. - Add a test case that shows lifetime.start of a variable isn't moved to the beginning of its scope when the goto leaves the scope.

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-12-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: lib/Sema/TreeTransform.h:8766-8767 NamedDecl *FirstQualifierInScope = nullptr; + DeclarationNameInfo MemberNameInfo = + getDerived().TransformDeclarationNameInfo(E->getMemberNameInfo()); rsmith wrote: >

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-12-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 82134. ahatanak marked 2 inline comments as done. ahatanak added a comment. Add code for error recovery. https://reviews.llvm.org/D24969 Files: lib/Sema/SemaTemplateInstantiateDecl.cpp lib/Sema/TreeTransform.h test/SemaCXX/destructor.cpp Index:

[PATCH] D27410: Always issue vtables when generating coverage instrumentation

2016-12-19 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: llvm/tools/clang/test/CodeGenCXX/vtable-coverage-gen.cpp:3 +// RUN: FileCheck %s < %t +// CHECK: @_ZTV8Category = linkonce_odr unnamed_addr constant {{.*}}, comdat, + I'm not sure I understood the purpose of this

[PATCH] D25556: [Sema] Add variable captured by a block to the enclosing lambda's potential capture list

2016-12-22 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 82394. ahatanak added a comment. Rebase. r286584 changed getCurLambda to optionally skip CapturedRegionScopeInfos. This patch changes it to skip BlockScopeInfos too. https://reviews.llvm.org/D25556 Files: include/clang/Sema/Sema.h lib/Sema/Sema.cpp

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

2016-12-22 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Ah, good idea. That sounds like a much simpler and less invasive approach. I agree that the performance impact would probably be small, and if it turns out to have a significant impact, we can reduce the number of times we move the lifetime.start (without moving it

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

2016-12-23 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 82421. ahatanak added a comment. When compiling for C, insert lifetime.start at the beginning of the current lexical scope, rather than moving it retroactively when a goto jumps backwards over the variable declaration. Also, insert lifetime.start at a

[PATCH] D15075: No error for conflict between inputs\outputs and clobber list

2016-12-27 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. In https://reviews.llvm.org/D15075#631237, @myatsina wrote: > In https://reviews.llvm.org/D15075#631207, @vitalybuka wrote: > > > These patches break asan tests: > >

[PATCH] D15075: No error for conflict between inputs\outputs and clobber list

2016-12-27 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. I guess it doesn't build because output constraints need "=" (e.g., "=D")? Also, I think all registers ("D", "S", and "c") should be in both the output and input operands list. You can probably declare new variables and use them in the output operands (e.g.,

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

2016-12-21 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 82279. ahatanak marked an inline comment as done. ahatanak added a comment. Remove Addr and Size from CallLifetimeEnd. https://reviews.llvm.org/D27680 Files: lib/CodeGen/CGDecl.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGStmt.cpp

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

2016-12-21 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. In https://reviews.llvm.org/D27680#628272, @rjmccall wrote: > Wouldn't it be simpler to just record an insertion point for the beginning of > the current lexical scope and insert the lifetime.begin there instead of at > the current IP? I'm not sure I understood your

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-12-19 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. ping https://reviews.llvm.org/D24969 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27478: Make ASTContext::getDeclAlign return the correct alignment for FunctionDecls

2016-12-19 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. ping https://reviews.llvm.org/D27478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27872: [APFloat] Switch from (PPCDoubleDoubleImpl, IEEEdouble) layout to (IEEEdouble, IEEEdouble)

2016-12-16 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: llvm/include/llvm/ADT/APFloat.h:773 + return U.IEEE.makeZero(Neg); +} else if (usesLayout(getSemantics())) { + return U.Double.makeZero(Neg); You don't need else after return.

  1   2   3   4   5   6   >