[PATCH] D45555: Fix a link failure in applyChanges()

2018-04-12 Thread Heejin Ahn via Phabricator via cfe-commits
aheejin created this revision.
aheejin added a reviewer: jdemeule.
Herald added subscribers: cfe-commits, mgorny, klimek.

This call
(https://github.com/llvm-mirror/clang-tools-extra/blob/e6bfa666d96c0d3010bb7d572f6240424ebd1cff/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp#L228-L229)
to tooling::applyAtomicChanges() introduced in https://reviews.llvm.org/D43764 
(https://reviews.llvm.org/rL329813) caused
a undefined reference error when built with `-DBUILD_SHARED_LIB=ON`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D4

Files:
  clang-apply-replacements/CMakeLists.txt


Index: clang-apply-replacements/CMakeLists.txt
===
--- clang-apply-replacements/CMakeLists.txt
+++ clang-apply-replacements/CMakeLists.txt
@@ -10,6 +10,7 @@
   clangBasic
   clangRewrite
   clangToolingCore
+  clangToolingRefactor
   )
 
 include_directories(


Index: clang-apply-replacements/CMakeLists.txt
===
--- clang-apply-replacements/CMakeLists.txt
+++ clang-apply-replacements/CMakeLists.txt
@@ -10,6 +10,7 @@
   clangBasic
   clangRewrite
   clangToolingCore
+  clangToolingRefactor
   )
 
 include_directories(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r329882 - ObjCGNU: Fix empty v3 protocols being emitted two fields short

2018-04-12 Thread Shoaib Meenai via cfe-commits
This is missing an attribution of changes, right?

From: cfe-commits  on behalf of David 
Chisnall via cfe-commits 
Reply-To: David Chisnall 
Date: Wednesday, April 11, 2018 at 11:49 PM
To: "cfe-commits@lists.llvm.org" 
Subject: r329882 - ObjCGNU: Fix empty v3 protocols being emitted two fields 
short

Author: theraven
Date: Wed Apr 11 23:46:15 2018
New Revision: 329882

URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D329882-26view-3Drev&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=kOSrBx9BPwaPzisR4ZjTMKz416HW7XRohfwPXtT5O9E&s=69tkdQlTNiO-wOTuNHO7a35iqDVy9hW6Jrw-hSq_q6s&e=
Log:
ObjCGNU: Fix empty v3 protocols being emitted two fields short

Summary:
Protocols that were being referenced but could not be fully realized were being 
emitted without `properties`/`optional_properties`. Since all v3 protocols must 
be 9 processor words wide, the lack of these fields is catastrophic for the 
runtime.

As an example, the runtime cannot know 
[here](https://github.com/gnustep/libobjc2/blob/master/protocol.c#L73) that 
`properties` and `optional_properties` are invalid.

Reviewers: rjmccall, theraven

Reviewed By: rjmccall, theraven

Subscribers: cfe-commits

Tags: #clang

Differential Revision: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D45305&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=kOSrBx9BPwaPzisR4ZjTMKz416HW7XRohfwPXtT5O9E&s=JoyP68pCdLsffM8xX9NMx9VXafshpTnxECegbsdc4rc&e=

Added:
cfe/trunk/test/CodeGenObjC/gnu-empty-protocol-v3.m
Modified:
cfe/trunk/lib/CodeGen/CGObjCGNU.cpp

Modified: cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_CodeGen_CGObjCGNU.cpp-3Frev-3D329882-26r1-3D329881-26r2-3D329882-26view-3Ddiff&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=kOSrBx9BPwaPzisR4ZjTMKz416HW7XRohfwPXtT5O9E&s=v2_iviP5qFoN3SvBUJtLVbxpLsyewCRlyAbxv96yJiI&e=
==
--- cfe/trunk/lib/CodeGen/CGObjCGNU.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjCGNU.cpp Wed Apr 11 23:46:15 2018
@@ -1748,11 +1748,13 @@ CGObjCGNU::GenerateEmptyProtocol(const s
   llvm::ConstantInt::get(Int32Ty, ProtocolVersion), IdTy));
   Elements.add(MakeConstantString(ProtocolName, ".objc_protocol_name"));
-  Elements.add(ProtocolList);
-  Elements.add(MethodList);
-  Elements.add(MethodList);
-  Elements.add(MethodList);
-  Elements.add(MethodList);
+  Elements.add(ProtocolList); /* .protocol_list */
+  Elements.add(MethodList);   /* .instance_methods */
+  Elements.add(MethodList);   /* .class_methods */
+  Elements.add(MethodList);   /* .optional_instance_methods */
+  Elements.add(MethodList);   /* .optional_class_methods */
+  Elements.add(NULLPtr);  /* .properties */
+  Elements.add(NULLPtr);  /* .optional_properties */
   return Elements.finishAndCreateGlobal(".objc_protocol",
 CGM.getPointerAlign());
}

Added: cfe/trunk/test/CodeGenObjC/gnu-empty-protocol-v3.m
URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_CodeGenObjC_gnu-2Dempty-2Dprotocol-2Dv3.m-3Frev-3D329882-26view-3Dauto&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=kOSrBx9BPwaPzisR4ZjTMKz416HW7XRohfwPXtT5O9E&s=arR3-NaqV7bUw3lmGH8VA1UAMCgPZ2Xf4Zbn-tKJY-A&e=
==
--- cfe/trunk/test/CodeGenObjC/gnu-empty-protocol-v3.m (added)
+++ cfe/trunk/test/CodeGenObjC/gnu-empty-protocol-v3.m Wed Apr 11 23:46:15 2018
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -fobjc-runtime=gnustep-1.9 
-emit-llvm -o - %s | FileCheck %s
+
+@protocol X;
+
+__attribute__((objc_root_class))
+@interface Z 
+@end
+
+@implementation Z
+@end
+
+// CHECK:  @.objc_protocol_list = internal global { i8*, i32, [0 x i8*] } 
zeroinitializer, align 4
+// CHECK:  @.objc_method_list = internal global { i32, [0 x { i8*, i8* }] 
} zeroinitializer, align 4
+// CHECK:  @.objc_protocol_name = private unnamed_addr constant [2 x i8] 
c"X\00", align 1
+// CHECK:  @.objc_protocol = internal global { i8*, i8*, { i8*, i32, [0 x 
i8*] }*, { i32, [0 x { i8*, i8* }] }*, { i32, [0 x { i8*, i8* }] }*, { i32, [0 
x { i8*, i8* }] }*, { i32, [0 x { i8*, i8* }] }*, i8*, i8* } {
+// CHECK-SAME: i8* inttoptr (i32 3 to i8*),
+// CHECK-SAME: i8* getelementptr inbounds ([2 x i8], [2 x i8]* 
@.objc_protocol_name, i32 0, i32 0),
+// CHECK-SAME: { i8*, i32, [0 x i8*] }* @.objc_protocol_list,
+// CHECK-SAME: { i32, [0 x { i8*, i8* }] }* @.objc_method_list,
+// CHECK-SAME: { i32, [0 x { i8*, i8* }] }* @.objc_method_list,
+// CHECK-SAME: { i32, [0 x { i8*, i8* }] }* @.objc_method_list,
+// CHECK-SAME: { i32, [0 x { i8*, i8* }] }* @.objc_method_list,
+// CHECK-SAME: i8* null,
+// CHECK-SAME: i8* null
+// CHECK-

Re: [clang-tools-extra] r329873 - [clang-tidy] [modernize-use-auto] Get only a length of token, not the token itself

2018-04-12 Thread Roman Lebedev via cfe-commits
Test?

On Thu, Apr 12, 2018 at 8:41 AM, Zinovy Nis via cfe-commits
 wrote:
> Author: zinovy.nis
> Date: Wed Apr 11 22:41:24 2018
> New Revision: 329873
>
> URL: http://llvm.org/viewvc/llvm-project?rev=329873&view=rev
> Log:
> [clang-tidy] [modernize-use-auto] Get only a length of token, not the token 
> itself
>
>
> Modified:
> clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
>
> Modified: clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp?rev=329873&r1=329872&r2=329873&view=diff
> ==
> --- clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp (original)
> +++ clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp Wed Apr 11 
> 22:41:24 2018
> @@ -11,6 +11,7 @@
>  #include "clang/AST/ASTContext.h"
>  #include "clang/ASTMatchers/ASTMatchFinder.h"
>  #include "clang/ASTMatchers/ASTMatchers.h"
> +#include "clang/Lex/Lexer.h"
>  #include "clang/Tooling/FixIt.h"
>
>  using namespace clang;
> @@ -419,8 +420,8 @@ void UseAutoCheck::replaceExpr(
>SourceRange Range(Loc.getSourceRange());
>
>if (MinTypeNameLength != 0 &&
> -  tooling::fixit::getText(Loc.getSourceRange(), 
> FirstDecl->getASTContext())
> -  .size() < MinTypeNameLength)
> +  Lexer::MeasureTokenLength(Loc.getLocStart(), 
> Context->getSourceManager(),
> +getLangOpts()) < MinTypeNameLength)
>  return;
>
>auto Diag = diag(Range.getBegin(), Message);
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [clang-tools-extra] r329873 - [clang-tidy] [modernize-use-auto] Get only a length of token, not the token itself

2018-04-12 Thread Roman Lebedev via cfe-commits
Also, i guess you now can remove linking to clangTooling that was
required to unbreak the build.
I'm guessing clangLex contains Lexer::MeasureTokenLength() ?

On Thu, Apr 12, 2018 at 10:22 AM, Roman Lebedev  wrote:
> Test?
>
> On Thu, Apr 12, 2018 at 8:41 AM, Zinovy Nis via cfe-commits
>  wrote:
>> Author: zinovy.nis
>> Date: Wed Apr 11 22:41:24 2018
>> New Revision: 329873
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=329873&view=rev
>> Log:
>> [clang-tidy] [modernize-use-auto] Get only a length of token, not the token 
>> itself
>>
>>
>> Modified:
>> clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
>>
>> Modified: clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp?rev=329873&r1=329872&r2=329873&view=diff
>> ==
>> --- clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp (original)
>> +++ clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp Wed Apr 11 
>> 22:41:24 2018
>> @@ -11,6 +11,7 @@
>>  #include "clang/AST/ASTContext.h"
>>  #include "clang/ASTMatchers/ASTMatchFinder.h"
>>  #include "clang/ASTMatchers/ASTMatchers.h"
>> +#include "clang/Lex/Lexer.h"
>>  #include "clang/Tooling/FixIt.h"
>>
>>  using namespace clang;
>> @@ -419,8 +420,8 @@ void UseAutoCheck::replaceExpr(
>>SourceRange Range(Loc.getSourceRange());
>>
>>if (MinTypeNameLength != 0 &&
>> -  tooling::fixit::getText(Loc.getSourceRange(), 
>> FirstDecl->getASTContext())
>> -  .size() < MinTypeNameLength)
>> +  Lexer::MeasureTokenLength(Loc.getLocStart(), 
>> Context->getSourceManager(),
>> +getLangOpts()) < MinTypeNameLength)
>>  return;
>>
>>auto Diag = diag(Range.getBegin(), Message);
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [clang-tools-extra] r329873 - [clang-tidy] [modernize-use-auto] Get only a length of token, not the token itself

2018-04-12 Thread Zinovy Nis via cfe-commits
Yes, due to to incorrect token length returned by the old code, the test
was passing. I fixed it.

Hm, you are right, clangTooling is not requirted anymore.

чт, 12 апр. 2018 г. в 10:25, Roman Lebedev :

> Also, i guess you now can remove linking to clangTooling that was
> required to unbreak the build.
> I'm guessing clangLex contains Lexer::MeasureTokenLength() ?
>
> On Thu, Apr 12, 2018 at 10:22 AM, Roman Lebedev 
> wrote:
> > Test?
> >
> > On Thu, Apr 12, 2018 at 8:41 AM, Zinovy Nis via cfe-commits
> >  wrote:
> >> Author: zinovy.nis
> >> Date: Wed Apr 11 22:41:24 2018
> >> New Revision: 329873
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=329873&view=rev
> >> Log:
> >> [clang-tidy] [modernize-use-auto] Get only a length of token, not the
> token itself
> >>
> >>
> >> Modified:
> >> clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
> >>
> >> Modified: clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
> >> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp?rev=329873&r1=329872&r2=329873&view=diff
> >>
> ==
> >> --- clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
> (original)
> >> +++ clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp Wed
> Apr 11 22:41:24 2018
> >> @@ -11,6 +11,7 @@
> >>  #include "clang/AST/ASTContext.h"
> >>  #include "clang/ASTMatchers/ASTMatchFinder.h"
> >>  #include "clang/ASTMatchers/ASTMatchers.h"
> >> +#include "clang/Lex/Lexer.h"
> >>  #include "clang/Tooling/FixIt.h"
> >>
> >>  using namespace clang;
> >> @@ -419,8 +420,8 @@ void UseAutoCheck::replaceExpr(
> >>SourceRange Range(Loc.getSourceRange());
> >>
> >>if (MinTypeNameLength != 0 &&
> >> -  tooling::fixit::getText(Loc.getSourceRange(),
> FirstDecl->getASTContext())
> >> -  .size() < MinTypeNameLength)
> >> +  Lexer::MeasureTokenLength(Loc.getLocStart(),
> Context->getSourceManager(),
> >> +getLangOpts()) < MinTypeNameLength)
> >>  return;
> >>
> >>auto Diag = diag(Range.getBegin(), Message);
> >>
> >>
> >> ___
> >> cfe-commits mailing list
> >> cfe-commits@lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [clang-tools-extra] r329873 - [clang-tidy] [modernize-use-auto] Get only a length of token, not the token itself

2018-04-12 Thread Roman Lebedev via cfe-commits
On Thu, Apr 12, 2018 at 10:30 AM, Zinovy Nis  wrote:
> Yes, due to to incorrect token length returned by the old code, the test was
> passing. I fixed it.
Let me rephrase.
If i revert just the
clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp change,
will the corresponding tests in
clang-tools-extra/trunk/test/clang-tidy/modernize-use-auto-cast.cpp
break?
No, they won't. And that is a problem.

> Hm, you are right, clangTooling is not requirted anymore.
>
> чт, 12 апр. 2018 г. в 10:25, Roman Lebedev :
>>
>> Also, i guess you now can remove linking to clangTooling that was
>> required to unbreak the build.
>> I'm guessing clangLex contains Lexer::MeasureTokenLength() ?
>>
>> On Thu, Apr 12, 2018 at 10:22 AM, Roman Lebedev 
>> wrote:
>> > Test?
>> >
>> > On Thu, Apr 12, 2018 at 8:41 AM, Zinovy Nis via cfe-commits
>> >  wrote:
>> >> Author: zinovy.nis
>> >> Date: Wed Apr 11 22:41:24 2018
>> >> New Revision: 329873
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=329873&view=rev
>> >> Log:
>> >> [clang-tidy] [modernize-use-auto] Get only a length of token, not the
>> >> token itself
>> >>
>> >>
>> >> Modified:
>> >> clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
>> >>
>> >> Modified: clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
>> >> URL:
>> >> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp?rev=329873&r1=329872&r2=329873&view=diff
>> >>
>> >> ==
>> >> --- clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
>> >> (original)
>> >> +++ clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp Wed
>> >> Apr 11 22:41:24 2018
>> >> @@ -11,6 +11,7 @@
>> >>  #include "clang/AST/ASTContext.h"
>> >>  #include "clang/ASTMatchers/ASTMatchFinder.h"
>> >>  #include "clang/ASTMatchers/ASTMatchers.h"
>> >> +#include "clang/Lex/Lexer.h"
>> >>  #include "clang/Tooling/FixIt.h"
>> >>
>> >>  using namespace clang;
>> >> @@ -419,8 +420,8 @@ void UseAutoCheck::replaceExpr(
>> >>SourceRange Range(Loc.getSourceRange());
>> >>
>> >>if (MinTypeNameLength != 0 &&
>> >> -  tooling::fixit::getText(Loc.getSourceRange(),
>> >> FirstDecl->getASTContext())
>> >> -  .size() < MinTypeNameLength)
>> >> +  Lexer::MeasureTokenLength(Loc.getLocStart(),
>> >> Context->getSourceManager(),
>> >> +getLangOpts()) < MinTypeNameLength)
>> >>  return;
>> >>
>> >>auto Diag = diag(Range.getBegin(), Message);
>> >>
>> >>
>> >> ___
>> >> cfe-commits mailing list
>> >> cfe-commits@lists.llvm.org
>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r329882 - ObjCGNU: Fix empty v3 protocols being emitted two fields short

2018-04-12 Thread David Chisnall via cfe-commits
It does seem to be.  I used arc to apply the change, so I’m not sure what 
happened - I thought it normally set the author correctly.

David

> On 12 Apr 2018, at 08:09, Shoaib Meenai  wrote:
> 
> This is missing an attribution of changes, right?
>  
> From: cfe-commits  on behalf of David 
> Chisnall via cfe-commits 
> Reply-To: David Chisnall 
> Date: Wednesday, April 11, 2018 at 11:49 PM
> To: "cfe-commits@lists.llvm.org" 
> Subject: r329882 - ObjCGNU: Fix empty v3 protocols being emitted two fields 
> short
>  
> Author: theraven
> Date: Wed Apr 11 23:46:15 2018
> New Revision: 329882
>  
> URL: 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D329882-26view-3Drev&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=kOSrBx9BPwaPzisR4ZjTMKz416HW7XRohfwPXtT5O9E&s=69tkdQlTNiO-wOTuNHO7a35iqDVy9hW6Jrw-hSq_q6s&e=
> Log:
> ObjCGNU: Fix empty v3 protocols being emitted two fields short
>  
> Summary:
> Protocols that were being referenced but could not be fully realized were 
> being emitted without `properties`/`optional_properties`. Since all v3 
> protocols must be 9 processor words wide, the lack of these fields is 
> catastrophic for the runtime.
>  
> As an example, the runtime cannot know 
> [here](https://github.com/gnustep/libobjc2/blob/master/protocol.c#L73) that 
> `properties` and `optional_properties` are invalid.
>  
> Reviewers: rjmccall, theraven
>  
> Reviewed By: rjmccall, theraven
>  
> Subscribers: cfe-commits
>  
> Tags: #clang
>  
> Differential Revision: 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D45305&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=kOSrBx9BPwaPzisR4ZjTMKz416HW7XRohfwPXtT5O9E&s=JoyP68pCdLsffM8xX9NMx9VXafshpTnxECegbsdc4rc&e=
>  
> Added:
> cfe/trunk/test/CodeGenObjC/gnu-empty-protocol-v3.m
> Modified:
> cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
>  
> Modified: cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
> URL: 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_CodeGen_CGObjCGNU.cpp-3Frev-3D329882-26r1-3D329881-26r2-3D329882-26view-3Ddiff&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=kOSrBx9BPwaPzisR4ZjTMKz416HW7XRohfwPXtT5O9E&s=v2_iviP5qFoN3SvBUJtLVbxpLsyewCRlyAbxv96yJiI&e=
> ==
> --- cfe/trunk/lib/CodeGen/CGObjCGNU.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGObjCGNU.cpp Wed Apr 11 23:46:15 2018
> @@ -1748,11 +1748,13 @@ CGObjCGNU::GenerateEmptyProtocol(const s
>llvm::ConstantInt::get(Int32Ty, ProtocolVersion), IdTy));
>Elements.add(MakeConstantString(ProtocolName, ".objc_protocol_name"));
> -  Elements.add(ProtocolList);
> -  Elements.add(MethodList);
> -  Elements.add(MethodList);
> -  Elements.add(MethodList);
> -  Elements.add(MethodList);
> +  Elements.add(ProtocolList); /* .protocol_list */
> +  Elements.add(MethodList);   /* .instance_methods */
> +  Elements.add(MethodList);   /* .class_methods */
> +  Elements.add(MethodList);   /* .optional_instance_methods */
> +  Elements.add(MethodList);   /* .optional_class_methods */
> +  Elements.add(NULLPtr);  /* .properties */
> +  Elements.add(NULLPtr);  /* .optional_properties */
>return Elements.finishAndCreateGlobal(".objc_protocol",
>  CGM.getPointerAlign());
> }
>  
> Added: cfe/trunk/test/CodeGenObjC/gnu-empty-protocol-v3.m
> URL: 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_CodeGenObjC_gnu-2Dempty-2Dprotocol-2Dv3.m-3Frev-3D329882-26view-3Dauto&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=kOSrBx9BPwaPzisR4ZjTMKz416HW7XRohfwPXtT5O9E&s=arR3-NaqV7bUw3lmGH8VA1UAMCgPZ2Xf4Zbn-tKJY-A&e=
> ==
> --- cfe/trunk/test/CodeGenObjC/gnu-empty-protocol-v3.m (added)
> +++ cfe/trunk/test/CodeGenObjC/gnu-empty-protocol-v3.m Wed Apr 11 23:46:15 
> 2018
> @@ -0,0 +1,25 @@
> +// RUN: %clang_cc1 -triple i686-pc-linux-gnu -fobjc-runtime=gnustep-1.9 
> -emit-llvm -o - %s | FileCheck %s
> +
> +@protocol X;
> +
> +__attribute__((objc_root_class))
> +@interface Z 
> +@end
> +
> +@implementation Z
> +@end
> +
> +// CHECK:  @.objc_protocol_list = internal global { i8*, i32, [0 x i8*] 
> } zeroinitializer, align 4
> +// CHECK:  @.objc_method_list = internal global { i32, [0 x { i8*, i8* 
> }] } zeroinitializer, align 4
> +// CHECK:  @.objc_protocol_name = private unnamed_addr constant [2 x i8] 
> c"X\00", align 1
> +// CHECK:  @.objc_protocol = internal global { i8*, i8*, { i8*, i32, [0 
> x i8*] }*, { i32, [0 x { i8*, i8* }] }*, { i32, [0 x { i8*, i8* }] }*, { i32, 
> [0 x { i8*, i8* }] }*, { i32, [0 x { i8*, i8* }] }*, i8*, i8* } {
> +// CHECK-SAME: i8* inttoptr (i32 3 to i8*),
> +// CHECK-SAME: i8* getelementptr inbounds ([2 x i8], [2 x i8]* 
> @.objc_protocol_name, i32 0, i32 0),

Re: [clang-tools-extra] r329873 - [clang-tidy] [modernize-use-auto] Get only a length of token, not the token itself

2018-04-12 Thread Zinovy Nis via cfe-commits
Yes, a test that should fail, did not fail with old code. And it's a
problem)


чт, 12 апр. 2018 г. в 10:35, Roman Lebedev :

> On Thu, Apr 12, 2018 at 10:30 AM, Zinovy Nis  wrote:
> > Yes, due to to incorrect token length returned by the old code, the test
> was
> > passing. I fixed it.
> Let me rephrase.
> If i revert just the
> clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp change,
> will the corresponding tests in
> clang-tools-extra/trunk/test/clang-tidy/modernize-use-auto-cast.cpp
> break?
> No, they won't. And that is a problem.
>
> > Hm, you are right, clangTooling is not requirted anymore.
> >
> > чт, 12 апр. 2018 г. в 10:25, Roman Lebedev :
> >>
> >> Also, i guess you now can remove linking to clangTooling that was
> >> required to unbreak the build.
> >> I'm guessing clangLex contains Lexer::MeasureTokenLength() ?
> >>
> >> On Thu, Apr 12, 2018 at 10:22 AM, Roman Lebedev 
> >> wrote:
> >> > Test?
> >> >
> >> > On Thu, Apr 12, 2018 at 8:41 AM, Zinovy Nis via cfe-commits
> >> >  wrote:
> >> >> Author: zinovy.nis
> >> >> Date: Wed Apr 11 22:41:24 2018
> >> >> New Revision: 329873
> >> >>
> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=329873&view=rev
> >> >> Log:
> >> >> [clang-tidy] [modernize-use-auto] Get only a length of token, not the
> >> >> token itself
> >> >>
> >> >>
> >> >> Modified:
> >> >> clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
> >> >>
> >> >> Modified:
> clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
> >> >> URL:
> >> >>
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp?rev=329873&r1=329872&r2=329873&view=diff
> >> >>
> >> >>
> ==
> >> >> --- clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
> >> >> (original)
> >> >> +++ clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp Wed
> >> >> Apr 11 22:41:24 2018
> >> >> @@ -11,6 +11,7 @@
> >> >>  #include "clang/AST/ASTContext.h"
> >> >>  #include "clang/ASTMatchers/ASTMatchFinder.h"
> >> >>  #include "clang/ASTMatchers/ASTMatchers.h"
> >> >> +#include "clang/Lex/Lexer.h"
> >> >>  #include "clang/Tooling/FixIt.h"
> >> >>
> >> >>  using namespace clang;
> >> >> @@ -419,8 +420,8 @@ void UseAutoCheck::replaceExpr(
> >> >>SourceRange Range(Loc.getSourceRange());
> >> >>
> >> >>if (MinTypeNameLength != 0 &&
> >> >> -  tooling::fixit::getText(Loc.getSourceRange(),
> >> >> FirstDecl->getASTContext())
> >> >> -  .size() < MinTypeNameLength)
> >> >> +  Lexer::MeasureTokenLength(Loc.getLocStart(),
> >> >> Context->getSourceManager(),
> >> >> +getLangOpts()) < MinTypeNameLength)
> >> >>  return;
> >> >>
> >> >>auto Diag = diag(Range.getBegin(), Message);
> >> >>
> >> >>
> >> >> ___
> >> >> cfe-commits mailing list
> >> >> cfe-commits@lists.llvm.org
> >> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r329882 - ObjCGNU: Fix empty v3 protocols being emitted two fields short

2018-04-12 Thread Shoaib Meenai via cfe-commits
AFAIK you need to add the "Patch by …" line manually. I don't know if arc is 
supposed to preserve the original commit author information, though I would 
suspect the version control system itself plays a role there.

From: David Chisnall  on behalf of David Chisnall 

Date: Thursday, April 12, 2018 at 12:37 AM
To: Shoaib Meenai 
Cc: "cfe-commits@lists.llvm.org" 
Subject: Re: r329882 - ObjCGNU: Fix empty v3 protocols being emitted two fields 
short

It does seem to be.  I used arc to apply the change, so I’m not sure what 
happened - I thought it normally set the author correctly.

David

On 12 Apr 2018, at 08:09, Shoaib Meenai mailto:smee...@fb.com>> 
wrote:
This is missing an attribution of changes, right?

From: cfe-commits 
mailto:cfe-commits-boun...@lists.llvm.org>> 
on behalf of David Chisnall via cfe-commits 
mailto:cfe-commits@lists.llvm.org>>
Reply-To: David Chisnall mailto:csda...@swan.ac.uk>>
Date: Wednesday, April 11, 2018 at 11:49 PM
To: "cfe-commits@lists.llvm.org" 
mailto:cfe-commits@lists.llvm.org>>
Subject: r329882 - ObjCGNU: Fix empty v3 protocols being emitted two fields 
short

Author: theraven
Date: Wed Apr 11 23:46:15 2018
New Revision: 329882

URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D329882-26view-3Drev&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=kOSrBx9BPwaPzisR4ZjTMKz416HW7XRohfwPXtT5O9E&s=69tkdQlTNiO-wOTuNHO7a35iqDVy9hW6Jrw-hSq_q6s&e=
Log:
ObjCGNU: Fix empty v3 protocols being emitted two fields short

Summary:
Protocols that were being referenced but could not be fully realized were being 
emitted without `properties`/`optional_properties`. Since all v3 protocols must 
be 9 processor words wide, the lack of these fields is catastrophic for the 
runtime.

As an example, the runtime cannot know 
[here](https://github.com/gnustep/libobjc2/blob/master/protocol.c#L73) that 
`properties` and `optional_properties` are invalid.

Reviewers: rjmccall, theraven

Reviewed By: rjmccall, theraven

Subscribers: cfe-commits

Tags: #clang

Differential Revision: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D45305&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=kOSrBx9BPwaPzisR4ZjTMKz416HW7XRohfwPXtT5O9E&s=JoyP68pCdLsffM8xX9NMx9VXafshpTnxECegbsdc4rc&e=

Added:
 cfe/trunk/test/CodeGenObjC/gnu-empty-protocol-v3.m
Modified:
 cfe/trunk/lib/CodeGen/CGObjCGNU.cpp

Modified: cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_CodeGen_CGObjCGNU.cpp-3Frev-3D329882-26r1-3D329881-26r2-3D329882-26view-3Ddiff&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=kOSrBx9BPwaPzisR4ZjTMKz416HW7XRohfwPXtT5O9E&s=v2_iviP5qFoN3SvBUJtLVbxpLsyewCRlyAbxv96yJiI&e=
==
--- cfe/trunk/lib/CodeGen/CGObjCGNU.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjCGNU.cpp Wed Apr 11 23:46:15 2018
@@ -1748,11 +1748,13 @@ CGObjCGNU::GenerateEmptyProtocol(const s
llvm::ConstantInt::get(Int32Ty, ProtocolVersion), IdTy));
Elements.add(MakeConstantString(ProtocolName, ".objc_protocol_name"));
-  Elements.add(ProtocolList);
-  Elements.add(MethodList);
-  Elements.add(MethodList);
-  Elements.add(MethodList);
-  Elements.add(MethodList);
+  Elements.add(ProtocolList); /* .protocol_list */
+  Elements.add(MethodList);   /* .instance_methods */
+  Elements.add(MethodList);   /* .class_methods */
+  Elements.add(MethodList);   /* .optional_instance_methods */
+  Elements.add(MethodList);   /* .optional_class_methods */
+  Elements.add(NULLPtr);  /* .properties */
+  Elements.add(NULLPtr);  /* .optional_properties */
return Elements.finishAndCreateGlobal(".objc_protocol",
  CGM.getPointerAlign());
}

Added: cfe/trunk/test/CodeGenObjC/gnu-empty-protocol-v3.m
URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_CodeGenObjC_gnu-2Dempty-2Dprotocol-2Dv3.m-3Frev-3D329882-26view-3Dauto&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=kOSrBx9BPwaPzisR4ZjTMKz416HW7XRohfwPXtT5O9E&s=arR3-NaqV7bUw3lmGH8VA1UAMCgPZ2Xf4Zbn-tKJY-A&e=
==
--- cfe/trunk/test/CodeGenObjC/gnu-empty-protocol-v3.m (added)
+++ cfe/trunk/test/CodeGenObjC/gnu-empty-protocol-v3.m Wed Apr 11 23:46:15 2018
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -fobjc-runtime=gnustep-1.9 
-emit-llvm -o - %s | FileCheck %s
+
+@protocol X;
+
+__attribute__((objc_root_class))
+@interface Z 
+@end
+
+@implementation Z
+@end
+
+// CHECK:  @.objc_protocol_list = internal global { i8*, i32, [0 x i8*] } 
zeroinitializer, align 4
+// CHECK:  @.objc_method_list = internal global { i32, [0 x { i8*, i8* }] 
} zeroinitializer, align 4
+// CHECK:  @.objc_protocol_na

[PATCH] D45557: [Analyzer] Fix for SValBuilder expressions rearrangement

2018-04-12 Thread Balogh , Ádám via Phabricator via cfe-commits
baloghadamsoftware created this revision.
baloghadamsoftware added a reviewer: NoQ.
Herald added subscribers: dkrupp, a.sidorin, rnkovacs, szepet, xazax.hun, 
whisperity.
Herald added a reviewer: george.karpenkov.

Expression rearrangement in SValBuilder (see https://reviews.llvm.org/rL329780) 
crashes with an assert if the type of the integer is different from the type of 
the symbol. This fix adds a check that prevents rearrangement in such cases.


https://reviews.llvm.org/D45557

Files:
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/svalbuilder-rearrange-comparisons.c


Index: test/Analysis/svalbuilder-rearrange-comparisons.c
===
--- test/Analysis/svalbuilder-rearrange-comparisons.c
+++ test/Analysis/svalbuilder-rearrange-comparisons.c
@@ -929,3 +929,8 @@
 clang_analyzer_eval(n - 126 == m + 3); // expected-warning{{UNKNOWN}}
   }
 }
+
+int mixed_integer_types(int x, int y) {
+  short a = x - 1U;
+  return a - y;
+}
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -437,7 +437,10 @@
 // overflow bounds.
 static bool shouldRearrange(ProgramStateRef State, BinaryOperator::Opcode Op,
 SymbolRef Sym, llvm::APSInt Int, QualType Ty) {
+  BasicValueFactory &BV = State->getStateManager().getBasicVals();
+
   return Sym->getType() == Ty &&
+APSIntType(Int) == BV.getAPSIntType(Sym->getType()) &&
 (!BinaryOperator::isComparisonOp(Op) ||
  (isWithinConstantOverflowBounds(Sym, State) && 
   isWithinConstantOverflowBounds(Int)));


Index: test/Analysis/svalbuilder-rearrange-comparisons.c
===
--- test/Analysis/svalbuilder-rearrange-comparisons.c
+++ test/Analysis/svalbuilder-rearrange-comparisons.c
@@ -929,3 +929,8 @@
 clang_analyzer_eval(n - 126 == m + 3); // expected-warning{{UNKNOWN}}
   }
 }
+
+int mixed_integer_types(int x, int y) {
+  short a = x - 1U;
+  return a - y;
+}
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -437,7 +437,10 @@
 // overflow bounds.
 static bool shouldRearrange(ProgramStateRef State, BinaryOperator::Opcode Op,
 SymbolRef Sym, llvm::APSInt Int, QualType Ty) {
+  BasicValueFactory &BV = State->getStateManager().getBasicVals();
+
   return Sym->getType() == Ty &&
+APSIntType(Int) == BV.getAPSIntType(Sym->getType()) &&
 (!BinaryOperator::isComparisonOp(Op) ||
  (isWithinConstantOverflowBounds(Sym, State) && 
   isWithinConstantOverflowBounds(Int)));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-12 Thread Whisperity via Phabricator via cfe-commits
whisperity added a comment.

@NoQ Do you reckon these tests files are too long? Perhaps the one about this 
inheritance, that inheritance, diamond inheritance, etc. could be split into 
multiple files.




Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:317
 
+def CtorUninitializedMemberChecker: Checker<"CtorUninitializedMember">,
+ HelpText<"Reports uninitialized members in constructors">,

This always pokes me in the eye, but shouldn't this file be sorted 
alphabetically?



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:103
+  // - pointer/reference
+  // - fundamental object (int, double, etc.)
+  //   * the parent of each node is the object that contains it

I believe `fundamental object` should be rephrased as `of fundamental type` (as 
in: object that is of fundamental type), as the standard talks about 
//fundamental types//.



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:187
+
+  if (isCalledByConstructor(Context))
+return;

I think somewhere there should be a bit of reasoning why exactly these cases 
are ignored by the checker.



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:218
+
+  std::string WarningMsg = std::to_string(UninitFields.size()) +
+   " uninitialized field" +

Maybe one could use a Twine or a string builder to avoid all these unneccessary 
string instantiations? Or `std::string::append()`?



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:316
+
+// At this point the field is a fundamental type.
+if (isFundamentalUninit(V)) {

(See, you use //fundamental type// here.)



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:340
+
+// TODO As of writing this checker, there is very little support for unions in
+// the CSA. This function relies on a nonexisting implementation by assuming as

Please put `:` after `TODO`.



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:342
+// the CSA. This function relies on a nonexisting implementation by assuming as
+// little about it as possible.
+bool FindUninitializedFields::isUnionUninit(const TypedValueRegion *R) {

What does `it` refer to in this sentence?



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:421
+if (T->isUnionType()) {
+  // TODO does control ever reach here?
+  if (isUnionUninit(RT)) {

Please insert a `:` after `TODO` here too.



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:475
+  if (Chain.back()->getDecl()->getType()->isPointerType())
+return OS.str().drop_back(2);
+  return OS.str().drop_back(1);

Maybe this could be made more explicit (as opposed to a comment) by using 
[[http://llvm.org/doxygen/classllvm_1_1StringRef.html#a3f45a78650a72626cdf9210de0c6d701|`StringRef::rtrim(StringRef)`]],
 like this:

return OS.str().rtrim('.').rtrim("->");

How does this code behave if the `Chain` is empty and thus `OS` contains no 
string whatsoever? `drop_back` asserts if you want to drop more elements than 
the length of the string.



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:498
+Context.getStackFrame());
+  // getting 'this'
+  SVal This = Context.getState()->getSVal(ThisLoc);

Comment isn't formatted as full sentence.



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:501
+
+  // getting '*this'
+  SVal Object = Context.getState()->getSVal(This.castAs());

Neither here.



Comment at: test/Analysis/ctor-uninitialized-member.cpp:683
+// Note that the rules for unions are different in C++ and C.
+// http://lists.llvm.org/pipermail/cfe-dev/242017-March/42052912.html
+

NoQ wrote:
> I managed to find the thread, but this link doesn't work for me.
Confirmed, this link is a 404.


https://reviews.llvm.org/D45532



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-12 Thread Whisperity via Phabricator via cfe-commits
whisperity requested changes to this revision.
whisperity added subscribers: gsd, dkrupp, o.gyorgy.
whisperity added a comment.
This revision now requires changes to proceed.

Sorry, one comment has gone missing meanwhile, I'm still getting used to this 
interface and hit //Submit// early.




Comment at: test/Analysis/ctor-uninitialized-member-inheritance.cpp:24
+  : NonPolymorphicLeft1(int{}) {
+y = 420;
+z = 420;

The literal `420` is repeated //everywhere// in this file. I think this (the 
same value appearing over and over again) will make debugging bad if something 
goes haywire and one has to look at memory dumps, control-flow-graphs, etc.


https://reviews.llvm.org/D45532



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45561: NFC - Indentation fixes in predefined-arch-macros.c

2018-04-12 Thread Gabor Buella via Phabricator via cfe-commits
GBuella created this revision.
GBuella added a reviewer: craig.topper.
Herald added subscribers: cfe-commits, fedor.sergeev.

Consistently separating tests with empty lines.
Helps while navigating this file.


Repository:
  rC Clang

https://reviews.llvm.org/D45561

Files:
  test/Preprocessor/predefined-arch-macros.c

Index: test/Preprocessor/predefined-arch-macros.c
===
--- test/Preprocessor/predefined-arch-macros.c
+++ test/Preprocessor/predefined-arch-macros.c
@@ -1,5 +1,5 @@
 // Begin X86/GCC/Linux tests 
-//
+
 // RUN: %clang -march=i386 -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_I386_M32
@@ -11,7 +11,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_I386_M64
 // CHECK_I386_M64: error: {{.*}}
-//
+
 // RUN: %clang -march=i486 -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_I486_M32
@@ -25,7 +25,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_I486_M64
 // CHECK_I486_M64: error: {{.*}}
-//
+
 // RUN: %clang -march=i586 -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_I586_M32
@@ -42,7 +42,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_I586_M64
 // CHECK_I586_M64: error: {{.*}}
-//
+
 // RUN: %clang -march=pentium -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_PENTIUM_M32
@@ -59,7 +59,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_PENTIUM_M64
 // CHECK_PENTIUM_M64: error: {{.*}}
-//
+
 // RUN: %clang -march=pentium-mmx -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_PENTIUM_MMX_M32
@@ -79,7 +79,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_PENTIUM_MMX_M64
 // CHECK_PENTIUM_MMX_M64: error: {{.*}}
-//
+
 // RUN: %clang -march=winchip-c6 -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_WINCHIP_C6_M32
@@ -94,7 +94,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_WINCHIP_C6_M64
 // CHECK_WINCHIP_C6_M64: error: {{.*}}
-//
+
 // RUN: %clang -march=winchip2 -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_WINCHIP2_M32
@@ -110,7 +110,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_WINCHIP2_M64
 // CHECK_WINCHIP2_M64: error: {{.*}}
-//
+
 // RUN: %clang -march=c3 -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_C3_M32
@@ -126,7 +126,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_C3_M64
 // CHECK_C3_M64: error: {{.*}}
-//
+
 // RUN: %clang -march=c3-2 -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_C3_2_M32
@@ -146,7 +146,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_C3_2_M64
 // CHECK_C3_2_M64: error: {{.*}}
-//
+
 // RUN: %clang -march=i686 -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_I686_M32
@@ -163,7 +163,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_I686_M64
 // CHECK_I686_M64: error: {{.*}}
-//
+
 // RUN: %clang -march=pentiumpro -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_PENTIUMPRO_M32
@@ -180,7 +180,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_PENTIUMPRO_M64
 // CHECK_PENTIUMPRO_M64: error: {{.*}}
-//
+
 // RUN: %clang -march=pentium2 -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_PENTIUM2_M32
@@ -199,7 +199,7 @@
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_PENTIUM2_M64
 // CHECK_PENTIUM2_M64: error: {{.*}}
-//
+
 // RUN: %clang -march=pentium3 -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

2018-04-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

@alexfh i'm waiting for some comment from you. there was no comment added when 
you removed yourself as a reviewer,
so i *assume* the usual message of that time was meant - "removing from my 
review queue, re-add when others have accepted.".


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44602



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r329892 - [clang-apply-replacements] Don't forget to link to clangToolingRefactor

2018-04-12 Thread Roman Lebedev via cfe-commits
Author: lebedevri
Date: Thu Apr 12 03:01:20 2018
New Revision: 329892

URL: http://llvm.org/viewvc/llvm-project?rev=329892&view=rev
Log:
[clang-apply-replacements] Don't forget to link to clangToolingRefactor

Fixes build:

[1/3] Linking CXX shared library lib/libclangApplyReplacements.so.7svn
FAILED: lib/libclangApplyReplacements.so.7svn
<...>
/usr/local/bin/ld.lld: error: undefined symbol: 
clang::tooling::AtomicChange::replace(clang::SourceManager const&, 
clang::SourceLocation, unsigned int, llvm::StringRef)
>>> referenced by ApplyReplacements.cpp
>>>   
>>> tools/clang/tools/extra/clang-apply-replacements/CMakeFiles/clangApplyReplacements.dir/lib/Tooling/ApplyReplacements.cpp.o:(clang::replace::mergeAndDeduplicate(std::vector>>  std::allocator > const&, 
>>> std::vector>> std::allocator > const&, 
>>> llvm::DenseMap>> std::vector>> std::allocator >, 
>>> llvm::DenseMapInfo, 
>>> llvm::detail::DenseMapPair>> std::vector>> std::allocator > > >&, clang::SourceManager&))

/usr/local/bin/ld.lld: error: undefined symbol: 
clang::tooling::applyAtomicChanges[abi:cxx11](llvm::StringRef, llvm::StringRef, 
llvm::ArrayRef, clang::tooling::ApplyChangesSpec 
const&)
>>> referenced by ApplyReplacements.cpp
>>>   
>>> tools/clang/tools/extra/clang-apply-replacements/CMakeFiles/clangApplyReplacements.dir/lib/Tooling/ApplyReplacements.cpp.o:(clang::replace::applyChanges[abi:cxx11](llvm::StringRef,
>>>  std::vector>> std::allocator > const&, 
>>> clang::tooling::ApplyChangesSpec const&, clang::DiagnosticsEngine&))
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Refs. D43764, rL329813

Modified:
clang-tools-extra/trunk/clang-apply-replacements/CMakeLists.txt

Modified: clang-tools-extra/trunk/clang-apply-replacements/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-apply-replacements/CMakeLists.txt?rev=329892&r1=329891&r2=329892&view=diff
==
--- clang-tools-extra/trunk/clang-apply-replacements/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-apply-replacements/CMakeLists.txt Thu Apr 12 
03:01:20 2018
@@ -10,6 +10,7 @@ add_clang_library(clangApplyReplacements
   clangBasic
   clangRewrite
   clangToolingCore
+  clangToolingRefactor
   )
 
 include_directories(


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-04-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Third commit introducing linking errors.
Did no buildbot catch those?


Repository:
  rL LLVM

https://reviews.llvm.org/D43764



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r329894 - [clang-apply-replacements] Always initialize FormatStyle.

2018-04-12 Thread Benjamin Kramer via cfe-commits
Author: d0k
Date: Thu Apr 12 03:35:24 2018
New Revision: 329894

URL: http://llvm.org/viewvc/llvm-project?rev=329894&view=rev
Log:
[clang-apply-replacements] Always initialize FormatStyle.

The cleanup logic reads from this for cleanups even if reformatting is
not requested.

Found by msan.

Modified:

clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp

Modified: 
clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp?rev=329894&r1=329893&r2=329894&view=diff
==
--- 
clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
 (original)
+++ 
clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
 Thu Apr 12 03:35:24 2018
@@ -97,16 +97,13 @@ int main(int argc, char **argv) {
   IntrusiveRefCntPtr(new DiagnosticIDs()), DiagOpts.get());
 
   // Determine a formatting style from options.
-  format::FormatStyle FormatStyle;
-  if (DoFormat) {
-auto FormatStyleOrError =
-format::getStyle(FormatStyleOpt, FormatStyleConfig, "LLVM");
-if (!FormatStyleOrError) {
-  llvm::errs() << llvm::toString(FormatStyleOrError.takeError()) << "\n";
-  return 1;
-}
-FormatStyle = *FormatStyleOrError;
+  auto FormatStyleOrError =
+  format::getStyle(FormatStyleOpt, FormatStyleConfig, "LLVM");
+  if (!FormatStyleOrError) {
+llvm::errs() << llvm::toString(FormatStyleOrError.takeError()) << "\n";
+return 1;
   }
+  format::FormatStyle FormatStyle = std::move(*FormatStyleOrError);
 
   TUReplacements TURs;
   TUReplacementFiles TUFiles;


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45311: [X86] Introduce wbinvd intrinsic

2018-04-12 Thread Gabor Buella via Phabricator via cfe-commits
GBuella updated this revision to Diff 142142.
GBuella retitled this revision from "Introduce wbinvd intrinsic" to "[X86] 
Introduce wbinvd intrinsic".

https://reviews.llvm.org/D45311

Files:
  include/clang/Basic/BuiltinsX86.def
  lib/Headers/ia32intrin.h
  test/CodeGen/builtin-wbinvd.c


Index: test/CodeGen/builtin-wbinvd.c
===
--- /dev/null
+++ test/CodeGen/builtin-wbinvd.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 %s -ffreestanding -triple=x86_64-unknown-unknown -emit-llvm 
-o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 %s -ffreestanding -triple=i386-unknown-unknown -emit-llvm 
-o - -Wall -Werror | FileCheck %s
+
+#include 
+
+void test_wbinvd(void) {
+  //CHECK-LABEL: @test_wbinvd
+  //CHECK: call void @llvm.x86.wbinvd()
+  _wbinvd();
+}
Index: lib/Headers/ia32intrin.h
===
--- lib/Headers/ia32intrin.h
+++ lib/Headers/ia32intrin.h
@@ -70,4 +70,9 @@
 
 #define _rdpmc(A) __rdpmc(A)
 
+static __inline__ void __attribute__((__always_inline__, __nodebug__))
+_wbinvd(void) {
+  return __builtin_ia32_wbinvd();
+}
+
 #endif /* __IA32INTRIN_H */
Index: include/clang/Basic/BuiltinsX86.def
===
--- include/clang/Basic/BuiltinsX86.def
+++ include/clang/Basic/BuiltinsX86.def
@@ -679,7 +679,8 @@
 //CLWB
 TARGET_BUILTIN(__builtin_ia32_clwb, "vvC*", "", "clwb")
 
-//WBNOINVD
+//WB[NO]INVD
+TARGET_BUILTIN(__builtin_ia32_wbinvd, "v", "", "")
 TARGET_BUILTIN(__builtin_ia32_wbnoinvd, "v", "", "wbnoinvd")
 
 // ADX


Index: test/CodeGen/builtin-wbinvd.c
===
--- /dev/null
+++ test/CodeGen/builtin-wbinvd.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 %s -ffreestanding -triple=x86_64-unknown-unknown -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 %s -ffreestanding -triple=i386-unknown-unknown -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+#include 
+
+void test_wbinvd(void) {
+  //CHECK-LABEL: @test_wbinvd
+  //CHECK: call void @llvm.x86.wbinvd()
+  _wbinvd();
+}
Index: lib/Headers/ia32intrin.h
===
--- lib/Headers/ia32intrin.h
+++ lib/Headers/ia32intrin.h
@@ -70,4 +70,9 @@
 
 #define _rdpmc(A) __rdpmc(A)
 
+static __inline__ void __attribute__((__always_inline__, __nodebug__))
+_wbinvd(void) {
+  return __builtin_ia32_wbinvd();
+}
+
 #endif /* __IA32INTRIN_H */
Index: include/clang/Basic/BuiltinsX86.def
===
--- include/clang/Basic/BuiltinsX86.def
+++ include/clang/Basic/BuiltinsX86.def
@@ -679,7 +679,8 @@
 //CLWB
 TARGET_BUILTIN(__builtin_ia32_clwb, "vvC*", "", "clwb")
 
-//WBNOINVD
+//WB[NO]INVD
+TARGET_BUILTIN(__builtin_ia32_wbinvd, "v", "", "")
 TARGET_BUILTIN(__builtin_ia32_wbnoinvd, "v", "", "wbnoinvd")
 
 // ADX
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

2018-04-12 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment.

In https://reviews.llvm.org/D44602#1065331, @lebedev.ri wrote:

> @alexfh i'm waiting for some comment from you. there was no comment added 
> when you removed yourself as a reviewer,
>  so i *assume* the usual message of that time was meant - "removing from my 
> review queue, re-add when others have accepted.".


No, I just think that other reviewers can handle this. Feel free to commit as 
long as there are no open concerns.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44602



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45392: [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

2018-04-12 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments.



Comment at: test/clang-tidy/readability-identifier-naming-objc.m:8-9
+// RUN:   ]}' -- -fno-delayed-template-parsing \
+// RUN:   -I%S/Inputs/readability-identifier-naming \
+// RUN:   -isystem %S/Inputs/readability-identifier-naming/system
+

Wizard wrote:
> alexfh wrote:
> > It looks like these flags are not needed, since there are no #include 
> > directives in this test.
> So we won't worry about those flags when using this check in google codebase 
> right?
These flags are used by the other test to specify directories used  to search 
for #included headers. Since the test is copied to a temporary directory by the 
check_clang_tidy.py script and the headers stay in the source tree (see the 
test/clang-tidy/Inputs/readability-identifier-naming/ directory), the test 
can't just use relative include paths, instead we specify the full path using 
the -I and -isystem compiler options. In real use cases all compilation 
arguments (including the required `-I` and `-isystem` options) are taken from a 
compilation database.  No need to specify them in clang-tidy configuration.



Comment at: test/clang-tidy/readability-identifier-naming-objc.m:4
+// RUN:  [{key: readability-identifier-naming.ObjcIvarPrefix, value: '_'}]}' \
+// RUN: --
+

The `--` and the trailing backslash above can be removed as well.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45392



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45555: [clang-apply-replacements] Fix a link failure in applyChanges()

2018-04-12 Thread Heejin Ahn via Phabricator via cfe-commits
aheejin abandoned this revision.
aheejin added a comment.

Looks like the same patch https://reviews.llvm.org/rL329892 has already landed.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D4



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-04-12 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment.

In https://reviews.llvm.org/D43764#1065345, @lebedev.ri wrote:

> Third commit introducing linking errors.
>  Did no buildbot catch those?


2 buildbots sent me an email, but they were unrelated failures.


Repository:
  rL LLVM

https://reviews.llvm.org/D43764



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45555: [clang-apply-replacements] Fix a link failure in applyChanges()

2018-04-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In https://reviews.llvm.org/D4#1065387, @aheejin wrote:

> Looks like the same patch https://reviews.llvm.org/rL329892 has already 
> landed.


Sorry, somehow i missed this differential.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D4



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

2018-04-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In https://reviews.llvm.org/D44602#1065366, @alexfh wrote:

> In https://reviews.llvm.org/D44602#1065331, @lebedev.ri wrote:
>
> > @alexfh i'm waiting for some comment from you. there was no comment added 
> > when you removed yourself as a reviewer,
> >  so i *assume* the usual message of that time was meant - "removing from my 
> > review queue, re-add when others have accepted.".
>
>
> No, I just think that other reviewers can handle this. Feel free to commit as 
> long as there are no open concerns.


Ah, ok, i'll commit it then, thanks!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44602



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r329902 - [clang-tidy] readability-function-size: add VariableThreshold param.

2018-04-12 Thread Roman Lebedev via cfe-commits
Author: lebedevri
Date: Thu Apr 12 05:06:42 2018
New Revision: 329902

URL: http://llvm.org/viewvc/llvm-project?rev=329902&view=rev
Log:
[clang-tidy] readability-function-size: add VariableThreshold param.

Summary:
Pretty straight-forward, just count all the variable declarations in the 
function's body, and if more than the configured threshold - do complain.

Note that this continues perverse practice of disabling the new option by 
default.
I'm not certain where is the balance point between not being too noisy, and 
actually enforcing the good practice.
If we really want to not disable this by default, but also to not cause too 
many new warnings, we could default to 50 or so.
But that is a lot of variables too...

I was able to find one coding style referencing variable count:
  - https://www.kernel.org/doc/html/v4.15/process/coding-style.html#functions

> Another measure of the function is the number of local variables. They 
shouldn’t exceed 5-10, or you’re doing something wrong.

Reviewers: hokein, xazax.hun, JonasToth, aaron.ballman, alexfh

Reviewed By: aaron.ballman

Subscribers: kimgr, Eugene.Zelenko, rnkovacs, cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D44602

Added:

clang-tools-extra/trunk/test/clang-tidy/readability-function-size-variables-c++17.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst
clang-tools-extra/trunk/test/clang-tidy/readability-function-size.cpp

Modified: clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp?rev=329902&r1=329901&r2=329902&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp Thu 
Apr 12 05:06:42 2018
@@ -22,6 +22,21 @@ class FunctionASTVisitor : public Recurs
   using Base = RecursiveASTVisitor;
 
 public:
+  bool VisitVarDecl(VarDecl *VD) {
+// Do not count function params.
+// Do not count decomposition declarations (C++17's structured bindings).
+if (StructNesting == 0 &&
+!(isa(VD) || isa(VD)))
+  ++Info.Variables;
+return true;
+  }
+  bool VisitBindingDecl(BindingDecl *BD) {
+// Do count each of the bindings (in the decomposition declaration).
+if (StructNesting == 0)
+  ++Info.Variables;
+return true;
+  }
+
   bool TraverseStmt(Stmt *Node) {
 if (!Node)
   return Base::TraverseStmt(Node);
@@ -74,15 +89,38 @@ public:
 return true;
   }
 
+  bool TraverseLambdaExpr(LambdaExpr *Node) {
+++StructNesting;
+Base::TraverseLambdaExpr(Node);
+--StructNesting;
+return true;
+  }
+
+  bool TraverseCXXRecordDecl(CXXRecordDecl *Node) {
+++StructNesting;
+Base::TraverseCXXRecordDecl(Node);
+--StructNesting;
+return true;
+  }
+
+  bool TraverseStmtExpr(StmtExpr *SE) {
+++StructNesting;
+Base::TraverseStmtExpr(SE);
+--StructNesting;
+return true;
+  }
+
   struct FunctionInfo {
 unsigned Lines = 0;
 unsigned Statements = 0;
 unsigned Branches = 0;
 unsigned NestingThreshold = 0;
+unsigned Variables = 0;
 std::vector NestingThresholders;
   };
   FunctionInfo Info;
   std::vector TrackedParent;
+  unsigned StructNesting = 0;
   unsigned CurrentNestingLevel = 0;
 };
 
@@ -94,7 +132,8 @@ FunctionSizeCheck::FunctionSizeCheck(Str
   StatementThreshold(Options.get("StatementThreshold", 800U)),
   BranchThreshold(Options.get("BranchThreshold", -1U)),
   ParameterThreshold(Options.get("ParameterThreshold", -1U)),
-  NestingThreshold(Options.get("NestingThreshold", -1U)) {}
+  NestingThreshold(Options.get("NestingThreshold", -1U)),
+  VariableThreshold(Options.get("VariableThreshold", -1U)) {}
 
 void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "LineThreshold", LineThreshold);
@@ -102,6 +141,7 @@ void FunctionSizeCheck::storeOptions(Cla
   Options.store(Opts, "BranchThreshold", BranchThreshold);
   Options.store(Opts, "ParameterThreshold", ParameterThreshold);
   Options.store(Opts, "NestingThreshold", NestingThreshold);
+  Options.store(Opts, "VariableThreshold", VariableThreshold);
 }
 
 void FunctionSizeCheck::registerMatchers(MatchFinder *Finder) {
@@ -133,7 +173,7 @@ void FunctionSizeCheck::check(const Matc
   if (FI.Lines > LineThreshold || FI.Statements > StatementThreshold ||
   FI.Branches > BranchThreshold ||
   ActualNumberParameters > ParameterThreshold ||
-  !FI.NestingThresholders.empty()) {
+  !FI.NestingThresholders.empty() || FI.Variables >

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

2018-04-12 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE329902: [clang-tidy] readability-function-size: add 
VariableThreshold param. (authored by lebedevri, committed by ).

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44602

Files:
  clang-tidy/readability/FunctionSizeCheck.cpp
  clang-tidy/readability/FunctionSizeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/readability-function-size.rst
  test/clang-tidy/readability-function-size-variables-c++17.cpp
  test/clang-tidy/readability-function-size.cpp

Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -158,6 +158,11 @@
   `
   added.
 
+- Added `VariableThreshold` option to :doc:`readability-function-size
+  ` check
+
+  Flags functions that have more than a specified number of variables declared in the body.
+
 - Adding the missing bitwise assignment operations to 
   :doc:`hicpp-signed-bitwise `.
 
Index: docs/clang-tidy/checks/readability-function-size.rst
===
--- docs/clang-tidy/checks/readability-function-size.rst
+++ docs/clang-tidy/checks/readability-function-size.rst
@@ -36,3 +36,10 @@
 Flag compound statements which create next nesting level after
 `NestingThreshold`. This may differ significantly from the expected value
 for macro-heavy code. The default is `-1` (ignore the nesting level).
+
+.. option:: VariableThreshold
+
+   Flag functions exceeding this number of variables declared in the body.
+   The default is `-1` (ignore the number of variables).
+   Please note that function parameters and variables declared in lambdas,
+   GNU Statement Expressions, and nested class inline functions are not counted.
Index: test/clang-tidy/readability-function-size.cpp
===
--- test/clang-tidy/readability-function-size.cpp
+++ test/clang-tidy/readability-function-size.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-function-size %t -- -config='{CheckOptions: [{key: readability-function-size.LineThreshold, value: 0}, {key: readability-function-size.StatementThreshold, value: 0}, {key: readability-function-size.BranchThreshold, value: 0}, {key: readability-function-size.ParameterThreshold, value: 5}, {key: readability-function-size.NestingThreshold, value: 2}]}' -- -std=c++11
+// RUN: %check_clang_tidy %s readability-function-size %t -- -config='{CheckOptions: [{key: readability-function-size.LineThreshold, value: 0}, {key: readability-function-size.StatementThreshold, value: 0}, {key: readability-function-size.BranchThreshold, value: 0}, {key: readability-function-size.ParameterThreshold, value: 5}, {key: readability-function-size.NestingThreshold, value: 2}, {key: readability-function-size.VariableThreshold, value: 1}]}' -- -std=c++11
 
 // Bad formatting is intentional, don't run clang-format over the whole file!
 
@@ -64,7 +64,7 @@
 
 void baz0() { // 1
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'baz0' exceeds recommended size/complexity
-  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 27 lines including whitespace and comments (threshold 0)
+  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 28 lines including whitespace and comments (threshold 0)
   // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 9 statements (threshold 0)
   int a;
   { // 2
@@ -87,14 +87,15 @@
 }
   }
   macro()
-// CHECK-MESSAGES: :[[@LINE-1]]:3: note: nesting level 3 starts here (threshold 2)
-// CHECK-MESSAGES: :[[@LINE-28]]:25: note: expanded from macro 'macro'
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: note: nesting level 3 starts here (threshold 2)
+  // CHECK-MESSAGES: :[[@LINE-28]]:25: note: expanded from macro 'macro'
+  // CHECK-MESSAGES: :[[@LINE-27]]:6: note: 9 variables (threshold 1)
 }
 
 // check that nested if's are not reported. this was broken initially
 void nesting_if() { // 1
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'nesting_if' exceeds recommended size/complexity
-  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 22 lines including whitespace and comments (threshold 0)
+  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 23 lines including whitespace and comments (threshold 0)
   // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 18 statements (threshold 0)
   // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 6 branches (threshold 0)
   if (true) { // 2
@@ -114,12 +115,13 @@
   } else if (true) { // 2
  int j;
   }
+  // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 6 variables (threshold 1)
 }
 
 // however this should warn
 void bad_if_nesting() { // 1
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_if_nesting' exceeds recommended size/complexity
-// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 22 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 23 lines including whitespace and comments (threshold 0)
 // CHECK-MESSAGES:

[PATCH] D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition

2018-04-12 Thread Rafael Stahl via Phabricator via cfe-commits
r.stahl created this revision.
r.stahl added reviewers: xazax.hun, dcoughlin, a.sidorin, george.karpenkov.
Herald added subscribers: cfe-commits, rnkovacs, szepet.

In https://reviews.llvm.org/D30691 code was added to getRuntimeDefinition that 
does not handle the case when FD==nullptr.


Repository:
  rC Clang

https://reviews.llvm.org/D45564

Files:
  lib/StaticAnalyzer/Core/CallEvent.cpp


Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -387,31 +387,33 @@
 
 RuntimeDefinition AnyFunctionCall::getRuntimeDefinition() const {
   const FunctionDecl *FD = getDecl();
+  if (!FD) {
+return {};
+  }
+
   // Note that the AnalysisDeclContext will have the FunctionDecl with
   // the definition (if one exists).
-  if (FD) {
-AnalysisDeclContext *AD =
-  getLocationContext()->getAnalysisDeclContext()->
-  getManager()->getContext(FD);
-bool IsAutosynthesized;
-Stmt* Body = AD->getBody(IsAutosynthesized);
-DEBUG({
-if (IsAutosynthesized)
-  llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
-   << "\n";
-});
-if (Body) {
-  const Decl* Decl = AD->getDecl();
-  return RuntimeDefinition(Decl);
-}
+  AnalysisDeclContext *AD =
+getLocationContext()->getAnalysisDeclContext()->
+getManager()->getContext(FD);
+  bool IsAutosynthesized;
+  Stmt* Body = AD->getBody(IsAutosynthesized);
+  DEBUG({
+  if (IsAutosynthesized)
+llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
+ << "\n";
+  });
+  if (Body) {
+const Decl* Decl = AD->getDecl();
+return RuntimeDefinition(Decl);
   }
 
   SubEngine *Engine = getState()->getStateManager().getOwningEngine();
   AnalyzerOptions &Opts = Engine->getAnalysisManager().options;
 
   // Try to get CTU definition only if CTUDir is provided.
   if (!Opts.naiveCTUEnabled())
-return RuntimeDefinition();
+return {};
 
   cross_tu::CrossTranslationUnitContext &CTUCtx =
   *Engine->getCrossTranslationUnitContext();


Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -387,31 +387,33 @@
 
 RuntimeDefinition AnyFunctionCall::getRuntimeDefinition() const {
   const FunctionDecl *FD = getDecl();
+  if (!FD) {
+return {};
+  }
+
   // Note that the AnalysisDeclContext will have the FunctionDecl with
   // the definition (if one exists).
-  if (FD) {
-AnalysisDeclContext *AD =
-  getLocationContext()->getAnalysisDeclContext()->
-  getManager()->getContext(FD);
-bool IsAutosynthesized;
-Stmt* Body = AD->getBody(IsAutosynthesized);
-DEBUG({
-if (IsAutosynthesized)
-  llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
-   << "\n";
-});
-if (Body) {
-  const Decl* Decl = AD->getDecl();
-  return RuntimeDefinition(Decl);
-}
+  AnalysisDeclContext *AD =
+getLocationContext()->getAnalysisDeclContext()->
+getManager()->getContext(FD);
+  bool IsAutosynthesized;
+  Stmt* Body = AD->getBody(IsAutosynthesized);
+  DEBUG({
+  if (IsAutosynthesized)
+llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
+ << "\n";
+  });
+  if (Body) {
+const Decl* Decl = AD->getDecl();
+return RuntimeDefinition(Decl);
   }
 
   SubEngine *Engine = getState()->getStateManager().getOwningEngine();
   AnalyzerOptions &Opts = Engine->getAnalysisManager().options;
 
   // Try to get CTU definition only if CTUDir is provided.
   if (!Opts.naiveCTUEnabled())
-return RuntimeDefinition();
+return {};
 
   cross_tu::CrossTranslationUnitContext &CTUCtx =
   *Engine->getCrossTranslationUnitContext();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition

2018-04-12 Thread Rafael Stahl via Phabricator via cfe-commits
r.stahl added a comment.

I encountered this with a construct like this:

  struct S
  {
  void (*fp)();
  };
  
  int main()
  {
  struct S s;
  s.fp();
  }


Repository:
  rC Clang

https://reviews.llvm.org/D45564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r329904 - Allow [[maybe_unused]] on static data members; these are considered variables and the attribute should appertain to them.

2018-04-12 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Thu Apr 12 05:21:41 2018
New Revision: 329904

URL: http://llvm.org/viewvc/llvm-project?rev=329904&view=rev
Log:
Allow [[maybe_unused]] on static data members; these are considered variables 
and the attribute should appertain to them.

Patch by S. B. Tam.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/AttributeList.h
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=329904&r1=329903&r2=329904&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Apr 12 05:21:41 
2018
@@ -2834,8 +2834,7 @@ def warn_attribute_wrong_decl_type : War
   "|types and namespaces"
   "|variables, functions and classes"
   "|kernel functions"
-  "|non-K&R-style functions"
-  "|variables, functions, methods, types, enumerations, enumerators, labels, 
and non-static data members}1">,
+  "|non-K&R-style functions}1">,
   InGroup;
 def err_attribute_wrong_decl_type : Error;
 def warn_type_attribute_wrong_type : Warning<

Modified: cfe/trunk/include/clang/Sema/AttributeList.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=329904&r1=329903&r2=329904&view=diff
==
--- cfe/trunk/include/clang/Sema/AttributeList.h (original)
+++ cfe/trunk/include/clang/Sema/AttributeList.h Thu Apr 12 05:21:41 2018
@@ -928,7 +928,6 @@ enum AttributeDeclKind {
   ExpectedFunctionVariableOrClass,
   ExpectedKernelFunction,
   ExpectedFunctionWithProtoType,
-  ExpectedForMaybeUnused,
 };
 
 } // namespace clang

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=329904&r1=329903&r2=329904&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu Apr 12 05:21:41 2018
@@ -2042,16 +2042,6 @@ static void handleDependencyAttr(Sema &S
 static void handleUnusedAttr(Sema &S, Decl *D, const AttributeList &AL) {
   bool IsCXX17Attr = AL.isCXX11Attribute() && !AL.getScopeName();
 
-  if (IsCXX17Attr && isa(D)) {
-// The C++17 spelling of this attribute cannot be applied to a static data
-// member per [dcl.attr.unused]p2.
-if (cast(D)->isStaticDataMember()) {
-  S.Diag(AL.getLoc(), diag::warn_attribute_wrong_decl_type)
-  << AL.getName() << ExpectedForMaybeUnused;
-  return;
-}
-  }
-
   // If this is spelled as the standard C++17 attribute, but not in C++17, warn
   // about using it as an extension.
   if (!S.getLangOpts().CPlusPlus17 && IsCXX17Attr)

Modified: cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp?rev=329904&r1=329903&r2=329904&view=diff
==
--- cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp (original)
+++ cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp Thu Apr 12 
05:21:41 2018
@@ -2,7 +2,7 @@
 
 struct [[maybe_unused]] S {
   int I [[maybe_unused]];
-  static int SI [[maybe_unused]]; // expected-warning {{'maybe_unused' 
attribute only applies to variables, functions, methods, types, enumerations, 
enumerators, labels, and non-static data members}}
+  static int SI [[maybe_unused]];
 };
 
 enum [[maybe_unused]] E1 {


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45403: Make [[maybe_unused]] work with static data members

2018-04-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman added a comment.

I've commit in r329904, thank you for the patch!


https://reviews.llvm.org/D45403



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r329823 - bpf: accept all asm register names

2018-04-12 Thread Yonghong Song via cfe-commits
Author: yhs
Date: Wed Apr 11 09:08:00 2018
New Revision: 329823

URL: http://llvm.org/viewvc/llvm-project?rev=329823&view=rev
Log:
bpf: accept all asm register names

Sometimes when people compile bpf programs with
"clang ... -target bpf ...", the kernel header
files may contain host arch inline assembly codes
as in the patch https://patchwork.kernel.org/patch/10119683/
by Arnaldo Carvaldo de Melo.

The current workaround in the above patch
is to guard the inline assembly with "#ifndef __BPF__"
marco. So when __BPF__ is defined, these macros will
have no use.

Such a method is not extensible. As a matter of fact,
most of these inline assembly codes will be thrown away
at the end of clang compilation.

So for bpf target, this patch accepts all asm register
names in clang AST stage. The name will be checked
again during llc code generation if the inline assembly
code is indeed for bpf programs.

With this patch, the above "#ifndef __BPF__" is not needed
any more in https://patchwork.kernel.org/patch/10119683/.

Signed-off-by: Yonghong Song 

Modified:
cfe/trunk/include/clang/Basic/TargetInfo.h
cfe/trunk/lib/Basic/Targets/BPF.h

Modified: cfe/trunk/include/clang/Basic/TargetInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=329823&r1=329822&r2=329823&view=diff
==
--- cfe/trunk/include/clang/Basic/TargetInfo.h (original)
+++ cfe/trunk/include/clang/Basic/TargetInfo.h Wed Apr 11 09:08:00 2018
@@ -619,7 +619,7 @@ public:
   /// according to GCC.
   ///
   /// This is used by Sema for inline asm statements.
-  bool isValidGCCRegisterName(StringRef Name) const;
+  virtual bool isValidGCCRegisterName(StringRef Name) const;
 
   /// \brief Returns the "normalized" GCC register name.
   ///

Modified: cfe/trunk/lib/Basic/Targets/BPF.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/BPF.h?rev=329823&r1=329822&r2=329823&view=diff
==
--- cfe/trunk/lib/Basic/Targets/BPF.h (original)
+++ cfe/trunk/lib/Basic/Targets/BPF.h Wed Apr 11 09:08:00 2018
@@ -63,6 +63,7 @@ public:
 return TargetInfo::VoidPtrBuiltinVaList;
   }
 
+  bool isValidGCCRegisterName(StringRef Name) const override { return true; }
   ArrayRef getGCCRegNames() const override { return None; }
 
   bool validateAsmConstraint(const char *&Name,


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r329848 - [x86] wbnoinvd intrinsic

2018-04-12 Thread Gabor Buella via cfe-commits
Author: gbuella
Date: Wed Apr 11 13:09:09 2018
New Revision: 329848

URL: http://llvm.org/viewvc/llvm-project?rev=329848&view=rev
Log:
[x86] wbnoinvd intrinsic

The WBNOINVD instruction writes back all modified
cache lines in the processor’s internal cache to main memory
but does not invalidate (flush) the internal caches.

Reviewers: craig.topper, zvi, ashlykov

Reviewed By: craig.topper

Differential Revision: https://reviews.llvm.org/D43817

Added:
cfe/trunk/lib/Headers/wbnoinvdintrin.h
cfe/trunk/test/CodeGen/builtin-wbnoinvd.c
Modified:
cfe/trunk/docs/ClangCommandLineReference.rst
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Basic/Targets/X86.cpp
cfe/trunk/lib/Basic/Targets/X86.h
cfe/trunk/lib/Headers/CMakeLists.txt
cfe/trunk/lib/Headers/cpuid.h
cfe/trunk/lib/Headers/x86intrin.h
cfe/trunk/test/CodeGen/builtins-x86.c
cfe/trunk/test/Driver/x86-target-features.c
cfe/trunk/test/Preprocessor/predefined-arch-macros.c

Modified: cfe/trunk/docs/ClangCommandLineReference.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangCommandLineReference.rst?rev=329848&r1=329847&r2=329848&view=diff
==
--- cfe/trunk/docs/ClangCommandLineReference.rst (original)
+++ cfe/trunk/docs/ClangCommandLineReference.rst Wed Apr 11 13:09:09 2018
@@ -2484,6 +2484,8 @@ X86
 
 .. option:: -mvpclmulqdq, -mno-vpclmulqdq
 
+.. option:: -mwbnoinvd, -mno-wbnoinvd
+
 .. option:: -mx87, -m80387, -mno-x87
 
 .. option:: -mxop, -mno-xop

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=329848&r1=329847&r2=329848&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Wed Apr 11 13:09:09 2018
@@ -679,6 +679,9 @@ TARGET_BUILTIN(__builtin_ia32_clflushopt
 //CLWB
 TARGET_BUILTIN(__builtin_ia32_clwb, "vvC*", "", "clwb")
 
+//WBNOINVD
+TARGET_BUILTIN(__builtin_ia32_wbnoinvd, "v", "", "wbnoinvd")
+
 // ADX
 TARGET_BUILTIN(__builtin_ia32_addcarryx_u32, "UcUcUiUiUi*", "", "adx")
 TARGET_BUILTIN(__builtin_ia32_addcarry_u32, "UcUcUiUiUi*", "", "")

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=329848&r1=329847&r2=329848&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Wed Apr 11 13:09:09 2018
@@ -2598,6 +2598,8 @@ def mclflushopt : Flag<["-"], "mclflusho
 def mno_clflushopt : Flag<["-"], "mno-clflushopt">, 
Group;
 def mclwb : Flag<["-"], "mclwb">, Group;
 def mno_clwb : Flag<["-"], "mno-clwb">, Group;
+def mwbnoinvd : Flag<["-"], "mwbnoinvd">, Group;
+def mno_wbnoinvd : Flag<["-"], "mno-wbnoinvd">, Group;
 def mclzero : Flag<["-"], "mclzero">, Group;
 def mno_clzero : Flag<["-"], "mno-clzero">, Group;
 def mcx16 : Flag<["-"], "mcx16">, Group;

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=329848&r1=329847&r2=329848&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Wed Apr 11 13:09:09 2018
@@ -154,6 +154,8 @@ bool X86TargetInfo::initFeatureMap(
 break;
 
   case CK_IcelakeServer:
+setFeatureEnabledImpl(Features, "wbnoinvd", true);
+LLVM_FALLTHROUGH;
   case CK_IcelakeClient:
 setFeatureEnabledImpl(Features, "vaes", true);
 setFeatureEnabledImpl(Features, "gfni", true);
@@ -792,6 +794,8 @@ bool X86TargetInfo::handleTargetFeatures
   HasCLFLUSHOPT = true;
 } else if (Feature == "+clwb") {
   HasCLWB = true;
+} else if (Feature == "+wbnoinvd") {
+  HasWBNOINVD = true;
 } else if (Feature == "+prefetchwt1") {
   HasPREFETCHWT1 = true;
 } else if (Feature == "+clzero") {
@@ -1134,6 +1138,8 @@ void X86TargetInfo::getTargetDefines(con
 Builder.defineMacro("__CLFLUSHOPT__");
   if (HasCLWB)
 Builder.defineMacro("__CLWB__");
+  if (HasWBNOINVD)
+Builder.defineMacro("__WBNOINVD__");
   if (HasMPX)
 Builder.defineMacro("__MPX__");
   if (HasSHSTK)
@@ -1297,6 +1303,7 @@ bool X86TargetInfo::isValidFeatureName(S
   .Case("tbm", true)
   .Case("vaes", true)
   .Case("vpclmulqdq", true)
+  .Case("wbnoinvd", true)
   .Case("x87", true)
   .Case("xop", true)
   .Case("xsave", true)
@@ -1371,6 +1378,7 @@ bool X86TargetInfo::hasFeature(StringRef
   .Case("tbm", HasTBM)
   .Case("vaes", HasVAES)
   .Case("vpclmulqdq", HasVPCLMULQDQ)
+  .Case("wbnoinvd", HasWBNOINVD)
   .Case("x86", true)
   

[PATCH] D45498: [clang-format] Don't insert space between ObjC class and lightweight generic

2018-04-12 Thread Daniel Jasper via Phabricator via cfe-commits
djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.

Looks good.




Comment at: lib/Format/TokenAnnotator.cpp:2357
+  return false;
+else
+  // Protocol list -> space if configured. @interface Foo : Bar 

Don't use else after return.
https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return

I'd have a slight preference for writing:

  bool IsLightweightGeneric = 
  Right.MatchingParen && Right.MatchingParen->Next &&
  Right.MatchingParen->Next->is(tok::colon);
  return !IsLightweightGeneric && Style.ObjCSpaceBeforeProtocolList;

Then I think it might not even need a comment (or a shorter one).


Repository:
  rC Clang

https://reviews.llvm.org/D45498



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45521: [clang-format] Improve ObjC guessing heuristic by supporting all @keywords

2018-04-12 Thread Daniel Jasper via Phabricator via cfe-commits
djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.

Looks good.


Repository:
  rC Clang

https://reviews.llvm.org/D45521



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44970: [XRay][clang] Add flag to choose instrumentation bundles

2018-04-12 Thread Martin Pelikán via Phabricator via cfe-commits
pelikan accepted this revision.
pelikan added a comment.
This revision is now accepted and ready to land.

Most of my comments are minor enough I'd be OK if this went in.  But please 
consider them before committing.




Comment at: clang/include/clang/Driver/XRayArgs.h:29
   std::vector Modes;
+  XRayInstrSet XRayInstrumentationBundle;
   bool XRayInstrument = false;

Since the class already has "XRay" in its name, I would rename the member to 
just "InstrumentationBundle", just as most of the other members are.



Comment at: clang/include/clang/Frontend/CodeGenOptions.h:111
 
+  enum XRayInstrumentationTypes {
+XRay_Function,

Now I fail to spot where is this enum used.  IIUC this should work even when 
it's not here, as the code uses the things in namespace XRayInstrKind.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:469
 /// AlwaysEmitXRayCustomEvents - Return true if we should emit IR for calls to
 /// the __xray_customevent(...) builin calls, when doing XRay instrumentation.
 bool CodeGenFunction::AlwaysEmitXRayCustomEvents() const {

typo: builin -> builtin



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:471
 bool CodeGenFunction::AlwaysEmitXRayCustomEvents() const {
-  return CGM.getCodeGenOpts().XRayAlwaysEmitCustomEvents;
+  return CGM.getCodeGenOpts().XRayInstrumentFunctions &&
+ (CGM.getCodeGenOpts().XRayAlwaysEmitCustomEvents ||

I kind of don't like how the "-fxray-instrument" variable is called 
"XRayInstrumentFunctions" because that's not what it means any more.  I think 
in a later diff, we should clean this up.  Or maybe even clean up some of the 
old flags whose functionality has been superseded by this.  But the logic here 
is fine.

Same with the misleading "ShouldXRayInstrumentFunction()" which controls custom 
events too, and not just functions.



Comment at: clang/lib/Driver/XRayArgs.cpp:107-109
+  } else
+D.Diag(clang::diag::err_drv_invalid_value)
+<< "-fxray-instrumentation-bundle=" << P;

nitpick:  I'd rewrite it to 

  if (!Valid) {
D.Diag
continue; // or break or return
  }
  
  

but the current code logic is fine.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:457
+auto Mask = parseXRayInstrValue(B);
+if (Mask == 0)
+  if (B != "none")

did you mean: "(Mask == None)"?


https://reviews.llvm.org/D44970



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45004: [clang-format] Always indent wrapped Objective-C selector names

2018-04-12 Thread Daniel Jasper via Phabricator via cfe-commits
djasper accepted this revision.
djasper added inline comments.
This revision is now accepted and ready to land.



Comment at: unittests/Format/FormatTest.cpp:7666
   FormatStyle Style = getLLVMStyle();
+  // ObjC ignores IndentWrappedFunctionNames when wrapping methods.
   Style.IndentWrappedFunctionNames = false;

maybe: .. and always indents. ?


Repository:
  rC Clang

https://reviews.llvm.org/D45004



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45526: [clang-format] Prefer breaking after ObjC category close paren

2018-04-12 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment.

Both patch and comment inside patch say "break after closing paren" and yet 
that's not what the test or example in the description actually do. Why is that?


Repository:
  rC Clang

https://reviews.llvm.org/D45526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45237: [RISCV] Fix logic to check if frame pointer should be used

2018-04-12 Thread Alex Bradbury via Phabricator via cfe-commits
asb accepted this revision.
asb added a comment.
This revision is now accepted and ready to land.

Thanks, looks good to me.


https://reviews.llvm.org/D45237



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45284: [RISCV] More validations on the input value of -march=

2018-04-12 Thread Alex Bradbury via Phabricator via cfe-commits
asb added a comment.

I've added a few comments on tweaking the error messages based on your tests.




Comment at: test/Driver/riscv-arch.c:157
+// RV32-STR: error: invalid arch name 'rv32',
+// RV32-STR: string must begin with rv32 or rv64
+

But the given string does begin with rv32. 'string must begin with rv32{i,e,g} 
or rv64{i,g}'?



Comment at: test/Driver/riscv-arch.c:167
+// RV32-ORDER: error: invalid arch name 'rv32imcq',
+// RV32-ORDER: unsupported  canonical order of extension 'q'
+

I don't think this message is clear, specifically 'unsupported canonical 
order...'. How about 'error: standard extension not given in canonical order'



Comment at: test/Driver/riscv-arch.c:203
+// RV32SX-NAME: unsupported non-standard supervisor-level extension
+// RV32SX-NAME: name missing 'sx'
+

I think this would be clearer if you said "name missing after 'sx'"



Comment at: test/Driver/riscv-arch.c:208
+// RV32S-NAME: error: invalid arch name 'rv32is',
+// RV32S-NAME:unsupported standard supervisor-level extension name missing 's'
+

Ditto.



Comment at: test/Driver/riscv-arch.c:213
+// RV32ALL-NAME: error: invalid arch name 'rv32ix_s_sx',
+// RV32ALL-NAME: unsupported non-standard user-level extension name missing 'x'
+

Ditto.


https://reviews.llvm.org/D45284



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44888: [RISCV] Default enable linker relaxation and add -mrelax, -mno-relax flags

2018-04-12 Thread Alex Bradbury via Phabricator via cfe-commits
asb added a comment.

Looks good to me, just missing help text on the command line options.


Repository:
  rL LLVM

https://reviews.llvm.org/D44888



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45544: [AAch64] Add the __ARM_FEATURE_DOTPROD macro definition

2018-04-12 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment.

Hi, thanks for adding this.

Also wanted to confirm that macro __ARM_FEATURE_DOTPROD will defined/included 
in the next release of the ACLE.




Comment at: lib/Basic/Targets/AArch64.h:36
   unsigned HasFullFP16;
+  unsigned HasDotProd;
   llvm::AArch64::ArchKind ArchKind;

Does this need to be initialised (and set to false)?



Comment at: test/Preprocessor/aarch64-target-features.c:94
+// -dM %s -o - | FileCheck --check-prefix=CHECK-DOTPROD %s
+// CHECK-DOTPROD: __ARM_FEATURE_DOTPROD 1
+

Perhaps one additional check it's not enabled?


https://reviews.llvm.org/D45544



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45392: [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

2018-04-12 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment.

BTW, it looks like the 
http://clang.llvm.org/extra/clang-tidy/checks/objc-property-declaration.html 
check could also be replaced with readability-identifier-naming (not in this 
patch).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45392



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D45526: [clang-format] Prefer breaking after ObjC category close paren

2018-04-12 Thread Ben Hamilton via cfe-commits
Fair point. I'll update the diff description.

The net effect of the patch is to break after the closing paren; the way I
implemented that was to raise the penalty for breaking after the opening
paren.

On Thu, Apr 12, 2018, 06:37 Daniel Jasper via Phabricator <
revi...@reviews.llvm.org> wrote:

> djasper added a comment.
>
> Both patch and comment inside patch say "break after closing paren" and
> yet that's not what the test or example in the description actually do. Why
> is that?
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D45526
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45526: [clang-format] Prefer breaking after ObjC category close paren

2018-04-12 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment.

I understand that, but the test example does not break after the closing paren. 
It breaks after the subsequent "<".


Repository:
  rC Clang

https://reviews.llvm.org/D45526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45255: [CodeGen] Add an option to suppress output of llvm.ident

2018-04-12 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki updated this revision to Diff 142165.
miyuki added a comment.

Updated the test case


https://reviews.llvm.org/D45255

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/no-ident-version.c


Index: test/CodeGen/no-ident-version.c
===
--- /dev/null
+++ test/CodeGen/no-ident-version.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -Qn -emit-llvm -debug-info-kind=limited -o - %s | FileCheck 
%s
+
+// CHECK: define i32 @main()
+// CHECK-NOT: llvm.ident
+// CHECK-NOT: producer:
+int main(void) {}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1070,6 +1070,8 @@
   Opts.EmitCheckPathComponentsToStrip = getLastArgIntValue(
   Args, OPT_fsanitize_undefined_strip_path_components_EQ, 0, Diags);
 
+  Opts.EmitVersionIdentMetadata = Args.hasFlag(OPT_Qy, OPT_Qn, true);
+
   return Success;
 }
 
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4392,6 +4392,9 @@
 }
   }
 
+  if (!Args.hasFlag(options::OPT_Qy, options::OPT_Qn, true))
+CmdArgs.push_back("-Qn");
+
   // -fcommon is the default unless compiling kernel code or the target says so
   bool NoCommonDefault = KernelOrKext || isNoCommonDefault(RawTriple);
   if (!Args.hasFlag(options::OPT_fcommon, options::OPT_fno_common,
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -570,7 +570,8 @@
   if (DebugInfo)
 DebugInfo->finalize();
 
-  EmitVersionIdentMetadata();
+  if (getCodeGenOpts().EmitVersionIdentMetadata)
+EmitVersionIdentMetadata();
 
   EmitTargetMetadata();
 }
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -577,7 +577,8 @@
   remapDIPath(getCurrentDirname()),
   CSInfo,
   getSource(SM, SM.getMainFileID())),
-  Producer, LO.Optimize || CGOpts.PrepareForLTO || CGOpts.EmitSummaryIndex,
+  CGOpts.EmitVersionIdentMetadata ? Producer : "",
+  LO.Optimize || CGOpts.PrepareForLTO || CGOpts.EmitSummaryIndex,
   CGOpts.DwarfDebugFlags, RuntimeVers,
   CGOpts.EnableSplitDwarf ? "" : CGOpts.SplitDwarfFile, EmissionKind,
   0 /* DWOid */, CGOpts.SplitDwarfInlining, CGOpts.DebugInfoForProfiling,
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -68,6 +68,7 @@
  ///< Decl* various IR entities came from.
  ///< Only useful when running CodeGen as a
  ///< subroutine.
+CODEGENOPT(EmitVersionIdentMetadata , 1, 1) ///< Emit compiler version 
metadata.
 CODEGENOPT(EmitGcovArcs  , 1, 0) ///< Emit coverage data files, aka. GCDA.
 CODEGENOPT(EmitGcovNotes , 1, 0) ///< Emit coverage "notes" files, aka 
GCNO.
 CODEGENOPT(EmitOpenCLArgMetadata , 1, 0) ///< Emit OpenCL kernel arg metadata.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -396,7 +396,10 @@
 def Ofast : Joined<["-"], "Ofast">, Group, Flags<[CC1Option]>;
 def P : Flag<["-"], "P">, Flags<[CC1Option]>, Group,
   HelpText<"Disable linemarker output in -E mode">;
-def Qn : Flag<["-"], "Qn">, IgnoredGCCCompat;
+def Qy : Flag<["-"], "Qy">, Flags<[CC1Option]>,
+  HelpText<"Emit metadata containing compiler name and version">;
+def Qn : Flag<["-"], "Qn">, Flags<[CC1Option]>,
+  HelpText<"Do not emit metadata containing compiler name and version">;
 def Qunused_arguments : Flag<["-"], "Qunused-arguments">, Flags<[DriverOption, 
CoreOption]>,
   HelpText<"Don't emit warning for unused driver arguments">;
 def Q : Flag<["-"], "Q">, IgnoredGCCCompat;


Index: test/CodeGen/no-ident-version.c
===
--- /dev/null
+++ test/CodeGen/no-ident-version.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -Qn -emit-llvm -debug-info-kind=limited -o - %s | FileCheck %s
+
+// CHECK: define i32 @main()
+// CHECK-NOT: llvm.ident
+// CHECK-NOT: producer:
+int main(void) {}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++

[PATCH] D45255: [CodeGen] Add an option to suppress output of llvm.ident

2018-04-12 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki added a comment.

One use case for this is debugging: this flag makes it easier to find out which 
revisions of LLVM introduce changes in codegen and/or debug information without 
having to filter out .ident/producer metadata. Some users can also use this 
flag (or an equivalent, -fno-ident) to reduce code size: 
https://github.com/niXman/mingw-builds/issues/412 (.ident strings are embedded 
into each object file and don't get merged during linking, so the difference 
can be noticeable).


https://reviews.llvm.org/D45255



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45569: [Sema] Disable built-in increment operator for bool in overload resolution in C++17

2018-04-12 Thread Jan Korous via Phabricator via cfe-commits
jkorous-apple created this revision.
jkorous-apple added a reviewer: rsmith.
Herald added a subscriber: cfe-commits.

Following: https://llvm.org/svn/llvm-project/cfe/trunk@329804

For C++17 the wording of [over.built] p4 excluded bool:

For every pair (T , vq), where T is an arithmetic type other than bool, there 
exist
candidate operator functions of the form

  vq T & operator++(vq T &);
  T operator++(vq T &, int);


Repository:
  rC Clang

https://reviews.llvm.org/D45569

Files:
  Sema/SemaOverload.cpp
  SemaCXX/overloaded-builtin-operators-cxx17.cpp


Index: SemaCXX/overloaded-builtin-operators-cxx17.cpp
===
--- /dev/null
+++ SemaCXX/overloaded-builtin-operators-cxx17.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -fshow-overloads=best -verify -triple 
x86_64-linux-gnu -std=c++17 %s
+
+struct BoolRef {
+  operator bool&();
+};
+
+void foo(BoolRef br) {
+  // C++ [over.built]p3: Increment for bool got deprecated in C++17.
+  bool b = br++; // expected-error{{cannot increment value of type 'BoolRef'}}
+}
\ No newline at end of file
Index: Sema/SemaOverload.cpp
===
--- Sema/SemaOverload.cpp
+++ Sema/SemaOverload.cpp
@@ -7775,11 +7775,13 @@
 InitArithmeticTypes();
   }
 
+  // Increment is deprecated for bool since C++17.
+  //
   // C++ [over.built]p3:
   //
-  //   For every pair (T, VQ), where T is an arithmetic type, and VQ
-  //   is either volatile or empty, there exist candidate operator
-  //   functions of the form
+  //   For every pair (T, VQ), where T is an arithmetic type other
+  //   than bool, and VQ is either volatile or empty, there exist
+  //   candidate operator functions of the form
   //
   //   VQ T&  operator++(VQ T&);
   //   T  operator++(VQ T&, int);
@@ -7798,8 +7800,12 @@
 
 for (unsigned Arith = 0; Arith < NumArithmeticTypes; ++Arith) {
   const auto TypeOfT = ArithmeticTypes[Arith];
-  if (Op == OO_MinusMinus && TypeOfT == S.Context.BoolTy)
-continue;
+  if (TypeOfT == S.Context.BoolTy) {
+if (Op == OO_MinusMinus)
+  continue;
+if (Op == OO_PlusPlus && S.getLangOpts().CPlusPlus17)
+  continue;
+  }
   addPlusPlusMinusMinusStyleOverloads(
 TypeOfT,
 VisibleTypeConversionsQuals.hasVolatile(),


Index: SemaCXX/overloaded-builtin-operators-cxx17.cpp
===
--- /dev/null
+++ SemaCXX/overloaded-builtin-operators-cxx17.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -fshow-overloads=best -verify -triple x86_64-linux-gnu -std=c++17 %s
+
+struct BoolRef {
+  operator bool&();
+};
+
+void foo(BoolRef br) {
+  // C++ [over.built]p3: Increment for bool got deprecated in C++17.
+  bool b = br++; // expected-error{{cannot increment value of type 'BoolRef'}}
+}
\ No newline at end of file
Index: Sema/SemaOverload.cpp
===
--- Sema/SemaOverload.cpp
+++ Sema/SemaOverload.cpp
@@ -7775,11 +7775,13 @@
 InitArithmeticTypes();
   }
 
+  // Increment is deprecated for bool since C++17.
+  //
   // C++ [over.built]p3:
   //
-  //   For every pair (T, VQ), where T is an arithmetic type, and VQ
-  //   is either volatile or empty, there exist candidate operator
-  //   functions of the form
+  //   For every pair (T, VQ), where T is an arithmetic type other
+  //   than bool, and VQ is either volatile or empty, there exist
+  //   candidate operator functions of the form
   //
   //   VQ T&  operator++(VQ T&);
   //   T  operator++(VQ T&, int);
@@ -7798,8 +7800,12 @@
 
 for (unsigned Arith = 0; Arith < NumArithmeticTypes; ++Arith) {
   const auto TypeOfT = ArithmeticTypes[Arith];
-  if (Op == OO_MinusMinus && TypeOfT == S.Context.BoolTy)
-continue;
+  if (TypeOfT == S.Context.BoolTy) {
+if (Op == OO_MinusMinus)
+  continue;
+if (Op == OO_PlusPlus && S.getLangOpts().CPlusPlus17)
+  continue;
+  }
   addPlusPlusMinusMinusStyleOverloads(
 TypeOfT,
 VisibleTypeConversionsQuals.hasVolatile(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45255: [CodeGen] Add an option to suppress output of llvm.ident

2018-04-12 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits
rogfer01 added a comment.

There is a flag `-fno-ident` that has the same effect in GCC 
https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html#index-fno-ident 
although it involves also ignoring the `#ident`.

A quick look seems to suggest that #ident is just ignored in the usual PP 
callbacks used by the parser so in practice `-fno-ident` would be the same as 
`-Qn`


https://reviews.llvm.org/D45255



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r329911 - [OpenCL] Added -std/-cl-std=c++

2018-04-12 Thread Anastasia Stulova via cfe-commits
Author: stulova
Date: Thu Apr 12 07:17:04 2018
New Revision: 329911

URL: http://llvm.org/viewvc/llvm-project?rev=329911&view=rev
Log:
[OpenCL] Added -std/-cl-std=c++

This is std option for OpenCL C++ v1.0.

Differential Revision: https://reviews.llvm.org/D45363


Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/LangStandards.def
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/InitPreprocessor.cpp
cfe/trunk/test/Driver/autocomplete.c
cfe/trunk/test/Driver/opencl.cl
cfe/trunk/test/Driver/unknown-std.cl
cfe/trunk/test/Frontend/opencl.cl
cfe/trunk/test/Frontend/stdlang.c
cfe/trunk/test/Preprocessor/predefined-macros.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=329911&r1=329910&r2=329911&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Apr 12 07:17:04 
2018
@@ -7966,7 +7966,7 @@ def err_generic_sel_multi_match : Error<
 
 // Blocks
 def err_blocks_disable : Error<"blocks support disabled - compile with 
-fblocks"
-  " or %select{pick a deployment target that supports them|for OpenCL 2.0 or 
above}0">;
+  " or %select{pick a deployment target that supports them|for OpenCL 2.0}0">;
 def err_block_returning_array_function : Error<
   "block cannot return %select{array|function}0 type %1">;
 

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=329911&r1=329910&r2=329911&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Thu Apr 12 07:17:04 2018
@@ -188,7 +188,8 @@ ENUM_LANGOPT(DefaultCallingConv, Default
 LANGOPT(ShortEnums, 1, 0, "short enum types")
 
 LANGOPT(OpenCL, 1, 0, "OpenCL")
-LANGOPT(OpenCLVersion , 32, 0, "OpenCL version")
+LANGOPT(OpenCLVersion , 32, 0, "OpenCL C version")
+LANGOPT(OpenCLCPlusPlusVersion , 32, 0, "OpenCL C++ version")
 LANGOPT(NativeHalfType, 1, 0, "Native half type support")
 LANGOPT(NativeHalfArgsAndReturns, 1, 0, "Native half args and returns")
 LANGOPT(HalfArgsAndReturns, 1, 0, "half args and returns")

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=329911&r1=329910&r2=329911&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Apr 12 07:17:04 2018
@@ -514,7 +514,7 @@ def cl_mad_enable : Flag<["-"], "cl-mad-
 def cl_no_signed_zeros : Flag<["-"], "cl-no-signed-zeros">, 
Group, Flags<[CC1Option]>,
   HelpText<"OpenCL only. Allow use of less precise no signed zeros 
computations in the generated binary.">;
 def cl_std_EQ : Joined<["-"], "cl-std=">, Group, 
Flags<[CC1Option]>,
-  HelpText<"OpenCL language standard to compile for.">, 
Values<"cl,CL,cl1.1,CL1.1,cl1.2,CL1.2,cl2.0,CL2.0">;
+  HelpText<"OpenCL language standard to compile for.">, 
Values<"cl,CL,cl1.1,CL1.1,cl1.2,CL1.2,cl2.0,CL2.0,c++">;
 def cl_denorms_are_zero : Flag<["-"], "cl-denorms-are-zero">, 
Group, Flags<[CC1Option]>,
   HelpText<"OpenCL only. Allow denormals to be flushed to zero.">;
 def cl_fp32_correctly_rounded_divide_sqrt : Flag<["-"], 
"cl-fp32-correctly-rounded-divide-sqrt">, Group, 
Flags<[CC1Option]>,

Modified: cfe/trunk/include/clang/Frontend/LangStandards.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/LangStandards.def?rev=329911&r1=329910&r2=329911&view=diff
==
--- cfe/trunk/include/clang/Frontend/LangStandards.def (original)
+++ cfe/trunk/include/clang/Frontend/LangStandards.def Thu Apr 12 07:17:04 2018
@@ -155,6 +155,9 @@ LANGSTANDARD(opencl12, "cl1.2",
 LANGSTANDARD(opencl20, "cl2.0",
  OpenCL, "OpenCL 2.0",
  LineComment | C99 | Digraphs | HexFloat | OpenCL)
+LANGSTANDARD(openclcpp, "c++",
+ OpenCL, "OpenCL C++ 1.0",
+ LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | Digraphs | 
OpenCL)
 
 LANGSTANDARD_ALIAS_DEPR(opencl10, "CL")
 LANGSTANDARD_ALIAS_DEPR(opencl11, "CL1.1")

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=329911&r1=329910&r2=329911&view=diff
==
--- cfe/trunk/lib

Re: r329804 - [Sema] Fix built-in decrement operator overload resolution

2018-04-12 Thread Jan Korous via cfe-commits
Hi Richard,

Here you are:
https://reviews.llvm.org/D45569 

I am now thinking if it makes sense to output this warning for pre-17 standards:

warning: incrementing expression of type bool is deprecated and incompatible 
with C++17

Produced in:

SemaExpr.cpp

static QualType CheckIncrementDecrementOperand

// Increment of bool sets it to true, but is deprecated.
S.Diag(OpLoc, S.getLangOpts().CPlusPlus17 ? diag::ext_increment_bool
  : diag::warn_increment_bool)

What do you think?

Jan

> On 11 Apr 2018, at 15:19, Richard Smith  wrote:
> 
> While you're here... ++ should not be available for bool in C++17 onwards. :)
> 
> On Wed, 11 Apr 2018, 14:39 Jan Korous via cfe-commits, 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Author: jkorous
> Date: Wed Apr 11 06:36:29 2018
> New Revision: 329804
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=329804&view=rev 
> 
> Log:
> [Sema] Fix built-in decrement operator overload resolution
> 
> C++ [over.built] p4:
> 
> "For every pair (T, VQ), where T is an arithmetic type other than bool, and 
> VQ is either volatile or empty, there exist candidate operator functions of 
> the form
> 
>   VQ T&  operator--(VQ T&);
>   T  operator--(VQ T&, int);
> "
> The bool type is in position LastPromotedIntegralType in 
> BuiltinOperatorOverloadBuilder::getArithmeticType::ArithmeticTypes, but 
> addPlusPlusMinusMinusArithmeticOverloads() was expecting it at position 0.
> 
> Differential Revision: https://reviews.llvm.org/D44988 
> 
> 
> rdar://problem/34255516
> 
> Modified:
> cfe/trunk/lib/Sema/SemaOverload.cpp
> cfe/trunk/test/SemaCXX/overloaded-builtin-operators.cpp
> 
> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=329804&r1=329803&r2=329804&view=diff
>  
> 
> ==
> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Wed Apr 11 06:36:29 2018
> @@ -7796,10 +7796,12 @@ public:
>  if (!HasArithmeticOrEnumeralCandidateType)
>return;
> 
> -for (unsigned Arith = (Op == OO_PlusPlus? 0 : 1);
> - Arith < NumArithmeticTypes; ++Arith) {
> +for (unsigned Arith = 0; Arith < NumArithmeticTypes; ++Arith) {
> +  const auto TypeOfT = ArithmeticTypes[Arith];
> +  if (Op == OO_MinusMinus && TypeOfT == S.Context.BoolTy)
> +continue;
>addPlusPlusMinusMinusStyleOverloads(
> -ArithmeticTypes[Arith],
> +TypeOfT,
>  VisibleTypeConversionsQuals.hasVolatile(),
>  VisibleTypeConversionsQuals.hasRestrict());
>  }
> 
> Modified: cfe/trunk/test/SemaCXX/overloaded-builtin-operators.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/overloaded-builtin-operators.cpp?rev=329804&r1=329803&r2=329804&view=diff
>  
> 
> ==
> --- cfe/trunk/test/SemaCXX/overloaded-builtin-operators.cpp (original)
> +++ cfe/trunk/test/SemaCXX/overloaded-builtin-operators.cpp Wed Apr 11 
> 06:36:29 2018
> @@ -62,6 +62,10 @@ void f(Short s, Long l, Enum1 e1, Enum2
>// FIXME: should pass (void)static_cast(islong(e1 % e2));
>  }
> 
> +struct BoolRef {
> +  operator bool&();
> +};
> +
>  struct ShortRef { // expected-note{{candidate function (the implicit copy 
> assignment operator) not viable}}
>  #if __cplusplus >= 201103L // C++11 or later
>  // expected-note@-2 {{candidate function (the implicit move assignment 
> operator) not viable}}
> @@ -73,6 +77,10 @@ struct LongRef {
>operator volatile long&();
>  };
> 
> +struct FloatRef {
> +  operator float&();
> +};
> +
>  struct XpmfRef { // expected-note{{candidate function (the implicit copy 
> assignment operator) not viable}}
>  #if __cplusplus >= 201103L // C++11 or later
>  // expected-note@-2 {{candidate function (the implicit move assignment 
> operator) not viable}}
> @@ -84,13 +92,19 @@ struct E2Ref {
>operator E2&();
>  };
> 
> -void g(ShortRef sr, LongRef lr, E2Ref e2_ref, XpmfRef pmf_ref) {
> +void g(BoolRef br, ShortRef sr, LongRef lr, FloatRef fr, E2Ref e2_ref, 
> XpmfRef pmf_ref) {
>// C++ [over.built]p3
>short s1 = sr++;
> 
> -  // C++ [over.built]p3
> +  // C++ [over.built]p4
>long l1 = lr--;
> 
> +  // C++ [over.built]p4
> +  float f1 = fr--;
> +
> +  // C++ [over.built]p4
> +  bool b2 = br--; // expected-error{{cannot decrement value of type 
> 'BoolRef'}}
> +
>// C++ [over.built]p18
>short& sr1 = (sr *= lr);
>volati

[PATCH] D45513: [clangd] Add line and column number to the index symbol.

2018-04-12 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 142170.
hokein marked 5 inline comments as done.
hokein added a comment.

Address review comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45513

Files:
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolYAML.cpp
  unittests/clangd/Annotations.cpp
  unittests/clangd/Annotations.h
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -51,13 +51,24 @@
 MATCHER_P(IncludeHeader, P, "") {
   return arg.Detail && arg.Detail->IncludeHeader == P;
 }
-MATCHER_P(DeclRange, Offsets, "") {
-  return arg.CanonicalDeclaration.StartOffset == Offsets.first &&
-  arg.CanonicalDeclaration.EndOffset == Offsets.second;
-}
-MATCHER_P(DefRange, Offsets, "") {
-  return arg.Definition.StartOffset == Offsets.first &&
- arg.Definition.EndOffset == Offsets.second;
+MATCHER_P(DeclLoc, L, "") {
+  return arg.CanonicalDeclaration.Start == L.Start &&
+ arg.CanonicalDeclaration.End == L.End;
+}
+MATCHER_P(DeclRange, Pos, "") {
+  return std::tie(arg.CanonicalDeclaration.Start.Line,
+  arg.CanonicalDeclaration.Start.Column,
+  arg.CanonicalDeclaration.End.Line,
+  arg.CanonicalDeclaration.End.Column) ==
+ std::tie(Pos.start.line, Pos.start.character, Pos.end.line,
+  Pos.end.character);
+}
+MATCHER_P(DefRange, Pos, "") {
+  return std::tie(arg.Definition.Start.Line,
+  arg.Definition.Start.Column, arg.Definition.End.Line,
+  arg.Definition.End.Column) ==
+ std::tie(Pos.start.line, Pos.start.character, Pos.end.line,
+  Pos.end.character);
 }
 MATCHER_P(Refs, R, "") { return int(arg.References) == R; }
 
@@ -209,7 +220,7 @@
   )");
   runSymbolCollector(Header.code(), /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAreArray({AllOf(
-   QName("Tmpl"), DeclRange(Header.offsetRange()))}));
+   QName("Tmpl"), DeclRange(Header.range()))}));
 }
 
 TEST_F(SymbolCollectorTest, Locations) {
@@ -221,6 +232,9 @@
 
 // Declared in header, defined nowhere.
 extern int $zdecl[[Z]];
+
+void $foodecl[[fo\
+o]]();
   )cpp");
   Annotations Main(R"cpp(
 int $xdef[[X]] = 42;
@@ -234,13 +248,15 @@
   EXPECT_THAT(
   Symbols,
   UnorderedElementsAre(
-  AllOf(QName("X"), DeclRange(Header.offsetRange("xdecl")),
-DefRange(Main.offsetRange("xdef"))),
-  AllOf(QName("Cls"), DeclRange(Header.offsetRange("clsdecl")),
-DefRange(Main.offsetRange("clsdef"))),
-  AllOf(QName("print"), DeclRange(Header.offsetRange("printdecl")),
-DefRange(Main.offsetRange("printdef"))),
-  AllOf(QName("Z"), DeclRange(Header.offsetRange("zdecl");
+  AllOf(QName("X"), DeclRange(Header.range("xdecl")),
+DefRange(Main.range("xdef"))),
+  AllOf(QName("Cls"), DeclRange(Header.range("clsdecl")),
+DefRange(Main.range("clsdef"))),
+  AllOf(QName("print"), DeclRange(Header.range("printdecl")),
+DefRange(Main.range("printdef"))),
+  AllOf(QName("Z"), DeclRange(Header.range("zdecl"))),
+  AllOf(QName("foo"), DeclRange(Header.range("foodecl")))
+  ));
 }
 
 TEST_F(SymbolCollectorTest, References) {
@@ -365,9 +381,9 @@
   EXPECT_THAT(
   Symbols,
   UnorderedElementsAre(
-  AllOf(QName("abc_Test"), DeclRange(Header.offsetRange("expansion")),
+  AllOf(QName("abc_Test"), DeclRange(Header.range("expansion")),
 DeclURI(TestHeaderURI)),
-  AllOf(QName("Test"), DeclRange(Header.offsetRange("spelling")),
+  AllOf(QName("Test"), DeclRange(Header.range("spelling")),
 DeclURI(TestHeaderURI;
 }
 
@@ -382,7 +398,8 @@
  /*ExtraArgs=*/{"-DNAME=name"});
   EXPECT_THAT(Symbols,
   UnorderedElementsAre(AllOf(
-  QName("name"), DeclRange(Header.offsetRange("expansion")),
+  QName("name"),
+  DeclRange(Header.range("expansion")),
   DeclURI(TestHeaderURI;
 }
 
@@ -511,9 +528,9 @@
   Kind:Function
   Lang:Cpp
 CanonicalDeclaration:
-  StartOffset: 0
-  EndOffset:   1
   FileURI:file:///path/foo.h
+  Start: 4096 # Line 1, column 0
+  End: 4097 # Line 1, column 1
 CompletionLabel:'Foo1-label'
 CompletionFilterText:'filter'
 CompletionPlainInsertText:'plain'
@@ -531,25 +548,32 @@
   Kind:Function
   Lang:Cpp
 CanonicalDeclaration:
-  StartOffset: 10
-  EndOffset:   12
   FileURI:file:///path/bar.h
+  Start: 4096 # Line 1, column 0
+  End: 4097 # Lin

[PATCH] D45363: [OpenCL] Added -std/-cl-std=CL2.2/CLC++

2018-04-12 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329911: [OpenCL] Added -std/-cl-std=c++ (authored by 
stulova, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45363?vs=141975&id=142171#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45363

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/include/clang/Basic/LangOptions.def
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Frontend/LangStandards.def
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/Frontend/InitPreprocessor.cpp
  cfe/trunk/test/Driver/autocomplete.c
  cfe/trunk/test/Driver/opencl.cl
  cfe/trunk/test/Driver/unknown-std.cl
  cfe/trunk/test/Frontend/opencl.cl
  cfe/trunk/test/Frontend/stdlang.c
  cfe/trunk/test/Preprocessor/predefined-macros.c

Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -514,7 +514,7 @@
 def cl_no_signed_zeros : Flag<["-"], "cl-no-signed-zeros">, Group, Flags<[CC1Option]>,
   HelpText<"OpenCL only. Allow use of less precise no signed zeros computations in the generated binary.">;
 def cl_std_EQ : Joined<["-"], "cl-std=">, Group, Flags<[CC1Option]>,
-  HelpText<"OpenCL language standard to compile for.">, Values<"cl,CL,cl1.1,CL1.1,cl1.2,CL1.2,cl2.0,CL2.0">;
+  HelpText<"OpenCL language standard to compile for.">, Values<"cl,CL,cl1.1,CL1.1,cl1.2,CL1.2,cl2.0,CL2.0,c++">;
 def cl_denorms_are_zero : Flag<["-"], "cl-denorms-are-zero">, Group, Flags<[CC1Option]>,
   HelpText<"OpenCL only. Allow denormals to be flushed to zero.">;
 def cl_fp32_correctly_rounded_divide_sqrt : Flag<["-"], "cl-fp32-correctly-rounded-divide-sqrt">, Group, Flags<[CC1Option]>,
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7966,7 +7966,7 @@
 
 // Blocks
 def err_blocks_disable : Error<"blocks support disabled - compile with -fblocks"
-  " or %select{pick a deployment target that supports them|for OpenCL 2.0 or above}0">;
+  " or %select{pick a deployment target that supports them|for OpenCL 2.0}0">;
 def err_block_returning_array_function : Error<
   "block cannot return %select{array|function}0 type %1">;
 
Index: cfe/trunk/include/clang/Basic/LangOptions.def
===
--- cfe/trunk/include/clang/Basic/LangOptions.def
+++ cfe/trunk/include/clang/Basic/LangOptions.def
@@ -188,7 +188,8 @@
 LANGOPT(ShortEnums, 1, 0, "short enum types")
 
 LANGOPT(OpenCL, 1, 0, "OpenCL")
-LANGOPT(OpenCLVersion , 32, 0, "OpenCL version")
+LANGOPT(OpenCLVersion , 32, 0, "OpenCL C version")
+LANGOPT(OpenCLCPlusPlusVersion , 32, 0, "OpenCL C++ version")
 LANGOPT(NativeHalfType, 1, 0, "Native half type support")
 LANGOPT(NativeHalfArgsAndReturns, 1, 0, "Native half args and returns")
 LANGOPT(HalfArgsAndReturns, 1, 0, "half args and returns")
Index: cfe/trunk/include/clang/Frontend/LangStandards.def
===
--- cfe/trunk/include/clang/Frontend/LangStandards.def
+++ cfe/trunk/include/clang/Frontend/LangStandards.def
@@ -155,6 +155,9 @@
 LANGSTANDARD(opencl20, "cl2.0",
  OpenCL, "OpenCL 2.0",
  LineComment | C99 | Digraphs | HexFloat | OpenCL)
+LANGSTANDARD(openclcpp, "c++",
+ OpenCL, "OpenCL C++ 1.0",
+ LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | Digraphs | OpenCL)
 
 LANGSTANDARD_ALIAS_DEPR(opencl10, "CL")
 LANGSTANDARD_ALIAS_DEPR(opencl11, "CL1.1")
Index: cfe/trunk/test/Preprocessor/predefined-macros.c
===
--- cfe/trunk/test/Preprocessor/predefined-macros.c
+++ cfe/trunk/test/Preprocessor/predefined-macros.c
@@ -159,6 +159,8 @@
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-CL20
 // RUN: %clang_cc1 %s -E -dM -o - -x cl -cl-fast-relaxed-math \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-FRM
+// RUN: %clang_cc1 %s -E -dM -o - -x cl -cl-std=c++ \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-CLCPP10
 // CHECK-CL10: #define CL_VERSION_1_0 100
 // CHECK-CL10: #define CL_VERSION_1_1 110
 // CHECK-CL10: #define CL_VERSION_1_2 120
@@ -184,6 +186,10 @@
 // CHECK-CL20: #define __OPENCL_C_VERSION__ 200
 // CHECK-CL20-NOT: #define __FAST_RELAXED_MATH__ 1
 // CHECK-FRM: #define __FAST_RELAXED_MATH__ 1
+// CHECK-CLCPP10: #define __CL_CPP_VERSION_1_0__ 100
+// CHECK-CLCPP10: #define __OPENCL_CPP_VERSION__ 100
+// CHECK-CLCPP10-NOT: #define __FAST_RELAXED_MATH__ 1
+// CHECK-CLCPP10-NOT: #define __ENDIAN_LITTLE__ 1
 
 // RUN: %clang_cc1 %s

[PATCH] D45363: [OpenCL] Added -std/-cl-std=CL2.2/CLC++

2018-04-12 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC329911: [OpenCL] Added -std/-cl-std=c++ (authored by 
stulova, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D45363

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  include/clang/Frontend/LangStandards.def
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/InitPreprocessor.cpp
  test/Driver/autocomplete.c
  test/Driver/opencl.cl
  test/Driver/unknown-std.cl
  test/Frontend/opencl.cl
  test/Frontend/stdlang.c
  test/Preprocessor/predefined-macros.c

Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -426,39 +426,47 @@
 
   // OpenCL v1.0/1.1 s6.9, v1.2/2.0 s6.10: Preprocessor Directives and Macros.
   if (LangOpts.OpenCL) {
-// OpenCL v1.0 and v1.1 do not have a predefined macro to indicate the
-// language standard with which the program is compiled. __OPENCL_VERSION__
-// is for the OpenCL version supported by the OpenCL device, which is not
-// necessarily the language standard with which the program is compiled.
-// A shared OpenCL header file requires a macro to indicate the language
-// standard. As a workaround, __OPENCL_C_VERSION__ is defined for
-// OpenCL v1.0 and v1.1.
-switch (LangOpts.OpenCLVersion) {
-case 100:
-  Builder.defineMacro("__OPENCL_C_VERSION__", "100");
-  break;
-case 110:
-  Builder.defineMacro("__OPENCL_C_VERSION__", "110");
-  break;
-case 120:
-  Builder.defineMacro("__OPENCL_C_VERSION__", "120");
-  break;
-case 200:
-  Builder.defineMacro("__OPENCL_C_VERSION__", "200");
-  break;
-default:
-  llvm_unreachable("Unsupported OpenCL version");
-}
-Builder.defineMacro("CL_VERSION_1_0", "100");
-Builder.defineMacro("CL_VERSION_1_1", "110");
-Builder.defineMacro("CL_VERSION_1_2", "120");
-Builder.defineMacro("CL_VERSION_2_0", "200");
+if (LangOpts.CPlusPlus) {
+  if (LangOpts.OpenCLCPlusPlusVersion == 100)
+Builder.defineMacro("__OPENCL_CPP_VERSION__", "100");
+  else
+llvm_unreachable("Unsupported OpenCL C++ version");
+  Builder.defineMacro("__CL_CPP_VERSION_1_0__", "100");
+} else {
+  // OpenCL v1.0 and v1.1 do not have a predefined macro to indicate the
+  // language standard with which the program is compiled. __OPENCL_VERSION__
+  // is for the OpenCL version supported by the OpenCL device, which is not
+  // necessarily the language standard with which the program is compiled.
+  // A shared OpenCL header file requires a macro to indicate the language
+  // standard. As a workaround, __OPENCL_C_VERSION__ is defined for
+  // OpenCL v1.0 and v1.1.
+  switch (LangOpts.OpenCLVersion) {
+  case 100:
+Builder.defineMacro("__OPENCL_C_VERSION__", "100");
+break;
+  case 110:
+Builder.defineMacro("__OPENCL_C_VERSION__", "110");
+break;
+  case 120:
+Builder.defineMacro("__OPENCL_C_VERSION__", "120");
+break;
+  case 200:
+Builder.defineMacro("__OPENCL_C_VERSION__", "200");
+break;
+  default:
+llvm_unreachable("Unsupported OpenCL version");
+  }
+  Builder.defineMacro("CL_VERSION_1_0", "100");
+  Builder.defineMacro("CL_VERSION_1_1", "110");
+  Builder.defineMacro("CL_VERSION_1_2", "120");
+  Builder.defineMacro("CL_VERSION_2_0", "200");
 
-if (TI.isLittleEndian())
-  Builder.defineMacro("__ENDIAN_LITTLE__");
+  if (TI.isLittleEndian())
+Builder.defineMacro("__ENDIAN_LITTLE__");
 
-if (LangOpts.FastRelaxedMath)
-  Builder.defineMacro("__FAST_RELAXED_MATH__");
+  if (LangOpts.FastRelaxedMath)
+Builder.defineMacro("__FAST_RELAXED_MATH__");
+}
   }
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1872,6 +1872,8 @@
 Opts.OpenCLVersion = 120;
   else if (LangStd == LangStandard::lang_opencl20)
 Opts.OpenCLVersion = 200;
+  else if (LangStd == LangStandard::lang_openclcpp)
+Opts.OpenCLCPlusPlusVersion = 100;
 
   // OpenCL has some additional defaults.
   if (Opts.OpenCL) {
@@ -2063,6 +2065,7 @@
 .Cases("cl1.1", "CL1.1", LangStandard::lang_opencl11)
 .Cases("cl1.2", "CL1.2", LangStandard::lang_opencl12)
 .Cases("cl2.0", "CL2.0", LangStandard::lang_opencl20)
+.Case("c++", LangStandard::lang_openclcpp)
 .Default(LangStandard::lang_unspecified);
 
 if (OpenCLLangStd == LangStandard::lang_unspecified) {
@@ -2275,7 +2278,7 @@
   Opts.RTTI = Op

[PATCH] D45513: [clangd] Add line and column number to the index symbol.

2018-04-12 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clangd/index/Index.h:39
   // using half-open range, [StartOffset, EndOffset).
+  // FIXME(hokein): remove these fields in favor of Position.
   unsigned StartOffset = 0;

sammccall wrote:
> I don't think we should remove them, we'll just have the same problem in 
> reverse.
> Could position have have line/col/offset, and have Position Start, End?
As discussed offline, we decide to remove them as we don't have real use case 
of them (we could add them back when needed).
I removed all the references of them in clangd. And remove these fields in a 
separate patch.



Comment at: clangd/index/SymbolCollector.cpp:206
+  Result.EndPos.Line = Result.StartPos.Line;
+  Result.EndPos.Character = Result.StartPos.Character + TokenLength;
+

sammccall wrote:
> I don't think this works, tokens can be split across lines.
> 
> I believe you want to compute NameLoc.locWithOffset(TokenLength) and then 
> decompose that into line/col.
> (getLocForEndOfToken, confusingly, is different)
Good catch. Done, added a test.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45513



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45570: [XRay] [clang] use compiler-rt's symbol visibility rules

2018-04-12 Thread Martin Pelikán via Phabricator via cfe-commits
pelikan created this revision.
pelikan added a reviewer: dberris.

Depends on https://reviews.llvm.org/D38993.


Repository:
  rC Clang

https://reviews.llvm.org/D45570

Files:
  lib/Driver/ToolChains/CommonArgs.cpp


Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -706,7 +706,8 @@
   return !StaticRuntimes.empty() || !NonWholeStaticRuntimes.empty();
 }
 
-bool tools::addXRayRuntime(const ToolChain&TC, const ArgList &Args, 
ArgStringList &CmdArgs) {
+bool tools::addXRayRuntime(const ToolChain&TC, const ArgList &Args,
+   ArgStringList &CmdArgs) {
   if (Args.hasArg(options::OPT_shared))
 return false;
 
@@ -716,6 +717,10 @@
 for (const auto &Mode : TC.getXRayArgs().modeList())
   CmdArgs.push_back(TC.getCompilerRTArgString(Args, Mode, false));
 CmdArgs.push_back("-no-whole-archive");
+
+SmallString<128> XRay(TC.getCompilerRT(Args, "xray"));
+CmdArgs.push_back(Args.MakeArgString("--dynamic-list=" + XRay + ".syms"));
+CmdArgs.push_back(Args.MakeArgString("--version-script=" + XRay + 
".vers"));
 return true;
   }
 


Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -706,7 +706,8 @@
   return !StaticRuntimes.empty() || !NonWholeStaticRuntimes.empty();
 }
 
-bool tools::addXRayRuntime(const ToolChain&TC, const ArgList &Args, ArgStringList &CmdArgs) {
+bool tools::addXRayRuntime(const ToolChain&TC, const ArgList &Args,
+   ArgStringList &CmdArgs) {
   if (Args.hasArg(options::OPT_shared))
 return false;
 
@@ -716,6 +717,10 @@
 for (const auto &Mode : TC.getXRayArgs().modeList())
   CmdArgs.push_back(TC.getCompilerRTArgString(Args, Mode, false));
 CmdArgs.push_back("-no-whole-archive");
+
+SmallString<128> XRay(TC.getCompilerRT(Args, "xray"));
+CmdArgs.push_back(Args.MakeArgString("--dynamic-list=" + XRay + ".syms"));
+CmdArgs.push_back(Args.MakeArgString("--version-script=" + XRay + ".vers"));
 return true;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45513: [clangd] Add line and column number to the index symbol.

2018-04-12 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clangd/index/Index.h:27
 struct SymbolLocation {
+  struct Position {
+// Line position in a document (zero-based).

sammccall wrote:
> There are 4 of these per symbol, if we can keep line + character to 32 bits 
> we save 16 bytes per symbol.
> 
> That looks like it might still be ~10% of non-string size. WDYT?
> (12 bits for column and 20 bits for line seems like a reasonable guess)
after some offline discussion, I no longer think this is a good idea.
We should strive to keep memory usage reasonable with the current naive index 
implementation, but "giant vector of Symbol structs in memory" isn't a case to 
micro-optimize for in the long run.



Comment at: clangd/index/Index.h:32
+// Character offset on a line in a document (zero-based).
+int Character = 0;
+  };

sammccall wrote:
> Column?
> 
> LSP calls this "character" but this is nonstandard and I find it very 
> confusing with offset. 
We should document what this is an offset into: bytes, utf-16 code units, or 
unicode codepoints. (Or even grid offsets - glyphs and doublewidth are a thing)

Given that we intend to send it over LSP without reading the source, only 
utf-16 code units is really correct. Unicode codepoints is "nicer" and will 
give correct results in the BMP, while bytes will be correct for ASCII only.

I'd vote for making this utf-16 code units.

It's OK if the code populating it doesn't get this right (confuses bytes and 
code units) but add a fixme.



Comment at: clangd/index/Index.h:39
   // using half-open range, [StartOffset, EndOffset).
+  // FIXME(hokein): remove these fields in favor of Position.
   unsigned StartOffset = 0;

hokein wrote:
> sammccall wrote:
> > I don't think we should remove them, we'll just have the same problem in 
> > reverse.
> > Could position have have line/col/offset, and have Position Start, End?
> As discussed offline, we decide to remove them as we don't have real use case 
> of them (we could add them back when needed).
> I removed all the references of them in clangd. And remove these fields in a 
> separate patch.
After offline discussion: we don't actually plan to do math on these ever, we 
just send them to LSP clients.
So removing sounds fine. We can add later if there are clear use cases.



Comment at: clangd/index/Index.h:44
+  /// The symbol range, using half-open range [StartPos, EndPos).
+  Position StartPos;
+  Position EndPos;

Not sure about the abbreviation - maybe just start/end? Since offsets are going 
away.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45513



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45570: [XRay] [clang] use compiler-rt's symbol visibility rules

2018-04-12 Thread Martin Pelikán via Phabricator via cfe-commits
pelikan updated this revision to Diff 142174.
pelikan added a comment.

while there, clang-format the code I touched


Repository:
  rC Clang

https://reviews.llvm.org/D45570

Files:
  lib/Driver/ToolChains/CommonArgs.cpp


Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -706,16 +706,20 @@
   return !StaticRuntimes.empty() || !NonWholeStaticRuntimes.empty();
 }
 
-bool tools::addXRayRuntime(const ToolChain&TC, const ArgList &Args, 
ArgStringList &CmdArgs) {
-  if (Args.hasArg(options::OPT_shared))
-return false;
+bool tools::addXRayRuntime(const ToolChain &TC, const ArgList &Args,
+   ArgStringList &CmdArgs) {
+  if (Args.hasArg(options::OPT_shared)) return false;
 
   if (TC.getXRayArgs().needsXRayRt()) {
 CmdArgs.push_back("-whole-archive");
 CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
 for (const auto &Mode : TC.getXRayArgs().modeList())
   CmdArgs.push_back(TC.getCompilerRTArgString(Args, Mode, false));
 CmdArgs.push_back("-no-whole-archive");
+
+SmallString<128> XRay(TC.getCompilerRT(Args, "xray"));
+CmdArgs.push_back(Args.MakeArgString("--dynamic-list=" + XRay + ".syms"));
+CmdArgs.push_back(Args.MakeArgString("--version-script=" + XRay + 
".vers"));
 return true;
   }
 


Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -706,16 +706,20 @@
   return !StaticRuntimes.empty() || !NonWholeStaticRuntimes.empty();
 }
 
-bool tools::addXRayRuntime(const ToolChain&TC, const ArgList &Args, ArgStringList &CmdArgs) {
-  if (Args.hasArg(options::OPT_shared))
-return false;
+bool tools::addXRayRuntime(const ToolChain &TC, const ArgList &Args,
+   ArgStringList &CmdArgs) {
+  if (Args.hasArg(options::OPT_shared)) return false;
 
   if (TC.getXRayArgs().needsXRayRt()) {
 CmdArgs.push_back("-whole-archive");
 CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
 for (const auto &Mode : TC.getXRayArgs().modeList())
   CmdArgs.push_back(TC.getCompilerRTArgString(Args, Mode, false));
 CmdArgs.push_back("-no-whole-archive");
+
+SmallString<128> XRay(TC.getCompilerRT(Args, "xray"));
+CmdArgs.push_back(Args.MakeArgString("--dynamic-list=" + XRay + ".syms"));
+CmdArgs.push_back(Args.MakeArgString("--version-script=" + XRay + ".vers"));
 return true;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44882: [clangd] Implementation of workspace/symbol request

2018-04-12 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Still LG, thanks!
I'll look into the testing issue.




Comment at: clangd/ClangdLSPServer.cpp:113
+  auto KindVal = static_cast(Kind);
+  if (KindVal >= SymbolKindMin && KindVal <= SymbolKindMax)
+SupportedSymbolKinds.set(KindVal);

nit: the bounds checks at usage types, with explicit underlying type for the 
enum are inconsistent with what we do in other protocol enums, and seem to 
clutter the code.

All else equal, I'd prefer to have an enum/enum class with implicit type, and 
not have values that are out of the enum's range. It's a simpler model that 
matches the code we have, and doesn't need tricky casts to 
SymKindUnderlyingType. If we want to support clients that send higher numbers 
than we know about, we could either:
 - overload fromJSON(vector)&
 - just express the capabilities as vector and convert them to the enum 
(and check bounds) at this point.
WDYT?



Comment at: clangd/FindSymbols.h:26
+
+llvm::Expected> getWorkspaceSymbols(
+llvm::StringRef Query, int Limit, const SymbolIndex *const Index,

Would be nice to have a comment describing: query syntax, limit=0 special 
behavior.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44882: [clangd] Implementation of workspace/symbol request

2018-04-12 Thread Marc-Andre Laperle via Phabricator via cfe-commits
malaperle added a comment.

In https://reviews.llvm.org/D44882#1065622, @sammccall wrote:

> Still LG, thanks!
>  I'll look into the testing issue.


I thought about it after... I think it was because I was trying to test with 
std::unordered_map (to prevent multiple results) which needs std=c++11, I'll 
try with something else.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44882: [clangd] Implementation of workspace/symbol request

2018-04-12 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

In https://reviews.llvm.org/D44882#1065631, @malaperle wrote:

> In https://reviews.llvm.org/D44882#1065622, @sammccall wrote:
>
> > Still LG, thanks!
> >  I'll look into the testing issue.
>
>
> I thought about it after... I think it was because I was trying to test with 
> std::unordered_map (to prevent multiple results) which needs std=c++11, I'll 
> try with something else.


Worth a shot, but don't count on it - for c++ clang switched to using std=c++11 
by default a while ago.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r329914 - Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

2018-04-12 Thread Malcolm Parsons via cfe-commits
Author: malcolm.parsons
Date: Thu Apr 12 07:48:48 2018
New Revision: 329914

URL: http://llvm.org/viewvc/llvm-project?rev=329914&view=rev
Log:
Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

Summary:
This patch adds two new diagnostics, which are off by default:

**-Wreturn-std-move**

This diagnostic is enabled by `-Wreturn-std-move`, `-Wmove`, or `-Wall`.
Diagnose cases of `return x` or `throw x`, where `x` is the name of a local 
variable or parameter, in which a copy operation is performed when a move 
operation would have been available. The user probably expected a move, but 
they're not getting a move, perhaps because the type of "x" is different from 
the return type of the function.
A place where this comes up in the wild is `stdext::inplace_function` 
which implements conversion via a conversion operator rather than a converting 
constructor; see https://github.com/WG21-SG14/SG14/issues/125#issue-297201412
Another place where this has come up in the wild, but where the fix ended up 
being different, was

try { ... } catch (ExceptionType ex) {
throw ex;
}

where the appropriate fix in that case was to replace `throw ex;` with 
`throw;`, and incidentally to catch by reference instead of by value. (But one 
could contrive a scenario where the slicing was intentional, in which case 
throw-by-move would have been the appropriate fix after all.)
Another example (intentional slicing to a base class) is dissected in 
https://github.com/accuBayArea/Slides/blob/master/slides/2018-03-07.pdf

**-Wreturn-std-move-in-c++11**

This diagnostic is enabled only by the exact spelling 
`-Wreturn-std-move-in-c++11`.
Diagnose cases of "return x;" or "throw x;" which in this version of Clang *do* 
produce moves, but which prior to Clang 3.9 / GCC 5.1 produced copies instead. 
This is useful in codebases which care about portability to those older 
compilers.
The name "-in-c++11" is not technically correct; what caused the 
version-to-version change in behavior here was actually CWG 1579, not C++14. I 
think it's likely that codebases that need portability to GCC 4.9-and-earlier 
may understand "C++11" as a colloquialism for "older compilers." The wording of 
this diagnostic is based on feedback from @rsmith.

**Discussion**

Notice that this patch is kind of a negative-space version of Richard Trieu's 
`-Wpessimizing-move`. That diagnostic warns about cases of `return 
std::move(x)` that should be `return x` for speed. These diagnostics warn about 
cases of `return x` that should be `return std::move(x)` for speed. (The two 
diagnostics' bailiwicks do not overlap: we don't have to worry about a `return` 
statement flipping between the two states indefinitely.)

I propose to write a paper for San Diego that would relax the implicit-move 
rules so that in C++2a the user //would// see the moves they expect, and the 
diagnostic could be re-worded in a later version of Clang to suggest explicit 
`std::move` only "in C++17 and earlier." But in the meantime (and/or forever if 
that proposal is not well received), this diagnostic will be useful to detect 
accidental copy operations.

Reviewers: rtrieu, rsmith

Reviewed By: rsmith

Subscribers: lebedev.ri, Rakete, rsmith, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D43322

Patch by Arthur O'Dwyer.

Added:
cfe/trunk/test/SemaCXX/warn-return-std-move.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaStmt.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=329914&r1=329913&r2=329914&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Thu Apr 12 07:48:48 2018
@@ -383,7 +383,11 @@ def DeprecatedObjCIsaUsage : DiagGroup<"
 def ExplicitInitializeCall : DiagGroup<"explicit-initialize-call">;
 def Packed : DiagGroup<"packed">;
 def Padded : DiagGroup<"padded">;
+
 def PessimizingMove : DiagGroup<"pessimizing-move">;
+def ReturnStdMoveInCXX11 : DiagGroup<"return-std-move-in-c++11">;
+def ReturnStdMove : DiagGroup<"return-std-move">;
+
 def PointerArith : DiagGroup<"pointer-arith">;
 def PoundWarning : DiagGroup<"#warnings">;
 def PoundPragmaMessage : DiagGroup<"#pragma-messages">,
@@ -723,7 +727,12 @@ def IntToVoidPointerCast : DiagGroup<"in
 def IntToPointerCast : DiagGroup<"int-to-pointer-cast",
  [IntToVoidPointerCast]>;
 
-def Move : DiagGroup<"move", [PessimizingMove, RedundantMove, SelfMove]>;
+def Move : DiagGroup<"move", [
+PessimizingMove,
+RedundantMove,
+ReturnStdMove,
+SelfMove
+  ]>;
 
 def Extra : DiagGroup<"extra", [
 MissingFieldIniti

[PATCH] D43322: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

2018-04-12 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC329914: Diagnose cases of "return x" that should 
be "return std::move(x)" for efficiency (authored by malcolm.parsons, 
committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D43322

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaStmt.cpp
  test/SemaCXX/warn-return-std-move.cpp

Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -2905,7 +2905,8 @@
   if (VD->getKind() != Decl::Var &&
   !((CESK & CES_AllowParameters) && VD->getKind() == Decl::ParmVar))
 return false;
-  if (VD->isExceptionVariable()) return false;
+  if (!(CESK & CES_AllowExceptionVariables) && VD->isExceptionVariable())
+return false;
 
   // ...automatic...
   if (!VD->hasLocalStorage()) return false;
@@ -2937,6 +2938,11 @@
 /// returned lvalues as rvalues in certain cases (to prefer move construction),
 /// then falls back to treating them as lvalues if that failed.
 ///
+/// \param ConvertingConstructorsOnly If true, follow [class.copy]p32 and reject
+/// resolutions that find non-constructors, such as derived-to-base conversions
+/// or `operator T()&&` member functions. If false, do consider such
+/// conversion sequences.
+///
 /// \param Res We will fill this in if move-initialization was possible.
 /// If move-initialization is not possible, such that we must fall back to
 /// treating the operand as an lvalue, we will leave Res in its original
@@ -2946,6 +2952,7 @@
   const VarDecl *NRVOCandidate,
   QualType ResultType,
   Expr *&Value,
+  bool ConvertingConstructorsOnly,
   ExprResult &Res) {
   ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
 CK_NoOp, Value, VK_XValue);
@@ -2966,22 +2973,39 @@
   continue;
 
 FunctionDecl *FD = Step.Function.Function;
-if (isa(FD)) {
-  // C++14 [class.copy]p32:
-  // [...] If the first overload resolution fails or was not performed,
-  // or if the type of the first parameter of the selected constructor
-  // is not an rvalue reference to the object's type (possibly
-  // cv-qualified), overload resolution is performed again, considering
-  // the object as an lvalue.
-  const RValueReferenceType *RRefType =
-  FD->getParamDecl(0)->getType()->getAs();
-  if (!RRefType)
-break;
-  if (!S.Context.hasSameUnqualifiedType(RRefType->getPointeeType(),
-NRVOCandidate->getType()))
-break;
+if (ConvertingConstructorsOnly) {
+  if (isa(FD)) {
+// C++14 [class.copy]p32:
+// [...] If the first overload resolution fails or was not performed,
+// or if the type of the first parameter of the selected constructor
+// is not an rvalue reference to the object's type (possibly
+// cv-qualified), overload resolution is performed again, considering
+// the object as an lvalue.
+const RValueReferenceType *RRefType =
+FD->getParamDecl(0)->getType()->getAs();
+if (!RRefType)
+  break;
+if (!S.Context.hasSameUnqualifiedType(RRefType->getPointeeType(),
+  NRVOCandidate->getType()))
+  break;
+  } else {
+continue;
+  }
 } else {
-  continue;
+  if (isa(FD)) {
+// Check that overload resolution selected a constructor taking an
+// rvalue reference. If it selected an lvalue reference, then we
+// didn't need to cast this thing to an rvalue in the first place.
+if (!isa(FD->getParamDecl(0)->getType()))
+  break;
+  } else if (isa(FD)) {
+// Check that overload resolution selected a conversion operator
+// taking an rvalue reference.
+if (cast(FD)->getRefQualifier() != RQ_RValue)
+  break;
+  } else {
+continue;
+  }
 }
 
 // Promote "AsRvalue" to the heap, since we now need this
@@ -3019,13 +3043,82 @@
   ExprResult Res = ExprError();
 
   if (AllowNRVO) {
+bool AffectedByCWG1579 = false;
+
 if (!NRVOCandidate) {
   NRVOCandidate = getCopyElisionCandidate(ResultType, Value, CES_Default);
+  if (NRVOCandidate &&
+  !getDiagnostics().isIgnored(diag::warn_return_std_move_in_cxx11,
+  Value->getExprLoc())) {
+const VarDecl *NRVOCandidateInCXX11 =
+getCopyElisionCandidate(ResultType, Value, CES_FormerDefault);
+AffectedByCWG1579 = (!NRVOCandidateInCXX11);
+  }
 }
 
 if (NRVOCandidate) {
   TryMoveInitialization(*this, Entity, N

Re: [PATCH] D45526: [clang-format] Prefer breaking after ObjC category close paren

2018-04-12 Thread Ben Hamilton via cfe-commits
True. I will reword the description to clarify.

On Thu, Apr 12, 2018, 07:58 Daniel Jasper via Phabricator <
revi...@reviews.llvm.org> wrote:

> djasper added a comment.
>
> I understand that, but the test example does not break after the closing
> paren. It breaks after the subsequent "<".
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D45526
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45526: [clang-format] Prefer breaking after ObjC category close paren

2018-04-12 Thread Daniel Jasper via Phabricator via cfe-commits
djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.

Looks good.




Comment at: lib/Format/TokenAnnotator.cpp:2276
+  // In Objective-C type declarations, prefer breaking after the category's
+  // close paren instead of after the open paren.
+  if (Line.Type == LT_ObjCDecl && Left.is(tok::l_paren) && Left.Previous &&

This is still using the old wording.


Repository:
  rC Clang

https://reviews.llvm.org/D45526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45526: [clang-format] Do not break after ObjC category open paren

2018-04-12 Thread Ben Hamilton via Phabricator via cfe-commits
benhamilton updated this revision to Diff 142182.
benhamilton marked an inline comment as done.
benhamilton added a comment.

- Update comment.


Repository:
  rC Clang

https://reviews.llvm.org/D45526

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -334,6 +334,9 @@
"c, c,\n"
"c, c> {\n"
"}");
+  verifyFormat("@interface c (ccc) <\n"
+   "c> {\n"
+   "}");
   Style.ObjCBinPackProtocolList = FormatStyle::BPS_Never;
   verifyFormat("@interface d () <\n"
"d,\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2272,6 +2272,13 @@
   if (Left.is(tok::colon) && Left.is(TT_ObjCMethodExpr))
 return Line.MightBeFunctionDecl ? 50 : 500;
 
+  // In Objective-C type declarations, avoid breaking after the category's
+  // open paren (we'll prefer breaking after the protocol list's opening
+  // angle bracket, if present).
+  if (Line.Type == LT_ObjCDecl && Left.is(tok::l_paren) && Left.Previous &&
+  Left.Previous->isOneOf(tok::identifier, tok::greater))
+return 500;
+
   if (Left.is(tok::l_paren) && InFunctionDecl &&
   Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign)
 return 100;


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -334,6 +334,9 @@
"c, c,\n"
"c, c> {\n"
"}");
+  verifyFormat("@interface c (ccc) <\n"
+   "c> {\n"
+   "}");
   Style.ObjCBinPackProtocolList = FormatStyle::BPS_Never;
   verifyFormat("@interface d () <\n"
"d,\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2272,6 +2272,13 @@
   if (Left.is(tok::colon) && Left.is(TT_ObjCMethodExpr))
 return Line.MightBeFunctionDecl ? 50 : 500;
 
+  // In Objective-C type declarations, avoid breaking after the category's
+  // open paren (we'll prefer breaking after the protocol list's opening
+  // angle bracket, if present).
+  if (Line.Type == LT_ObjCDecl && Left.is(tok::l_paren) && Left.Previous &&
+  Left.Previous->isOneOf(tok::identifier, tok::greater))
+return 500;
+
   if (Left.is(tok::l_paren) && InFunctionDecl &&
   Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign)
 return 100;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45526: [clang-format] Do not break after ObjC category open paren

2018-04-12 Thread Ben Hamilton via Phabricator via cfe-commits
benhamilton added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:2276
+  // In Objective-C type declarations, prefer breaking after the category's
+  // close paren instead of after the open paren.
+  if (Line.Type == LT_ObjCDecl && Left.is(tok::l_paren) && Left.Previous &&

djasper wrote:
> This is still using the old wording.
Fixed, thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D45526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45004: [clang-format] Always indent wrapped Objective-C selector names

2018-04-12 Thread Ben Hamilton via Phabricator via cfe-commits
benhamilton updated this revision to Diff 142183.
benhamilton marked an inline comment as done.
benhamilton added a comment.

Update comment.


Repository:
  rC Clang

https://reviews.llvm.org/D45004

Files:
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/FormatTestObjC.cpp

Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -536,28 +536,25 @@
"ofSize:(size_t)height\n"
"  :(size_t)width;");
   Style.ColumnLimit = 40;
-  // Make sure selectors with 0, 1, or more arguments are not indented
-  // when IndentWrappedFunctionNames is false.
-  Style.IndentWrappedFunctionNames = false;
+  // Make sure selectors with 0, 1, or more arguments are indented when wrapped.
   verifyFormat("- (a)\n"
-   ";\n");
+   ";\n");
   verifyFormat("- (a)\n"
-   ":(int)a;\n");
+   ":(int)a;\n");
   verifyFormat("- (a)\n"
-   ":(int)a\n"
-   ":(int)a;\n");
+   ":(int)a\n"
+   ":(int)a;\n");
   verifyFormat("- (a)\n"
-   " aaa:(int)a\n"
-   ":(int)a;\n");
+   " aaa:(int)a\n"
+   ":(int)a;\n");
   verifyFormat("- (a)\n"
-   ":(int)a\n"
-   " aaa:(int)a;\n");
+   ":(int)a\n"
+   " aaa:(int)a;\n");
 
   // Continuation indent width should win over aligning colons if the function
   // name is long.
   Style = getGoogleStyle(FormatStyle::LK_ObjC);
   Style.ColumnLimit = 40;
-  Style.IndentWrappedFunctionNames = true;
   verifyFormat("- (void)shortf:(GTMFoo *)theFoo\n"
"dontAlignNamef:(NSRect)theRect {\n"
"}");
@@ -567,22 +564,6 @@
"   aShortf:(NSRect)theRect {\n"
"}");
 
-  // Make sure selectors with 0, 1, or more arguments are indented
-  // when IndentWrappedFunctionNames is true.
-  verifyFormat("- (a)\n"
-   ";\n");
-  verifyFormat("- (a)\n"
-   ":(int)a;\n");
-  verifyFormat("- (a)\n"
-   ":(int)a\n"
-   ":(int)a;\n");
-  verifyFormat("- (a)\n"
-   " aaa:(int)a\n"
-   ":(int)a;\n");
-  verifyFormat("- (a)\n"
-   ":(int)a\n"
-   " aaa:(int)a;\n");
-
   // Format pairs correctly.
   Style.ColumnLimit = 80;
   verifyFormat("- (void)drawRectOn:(id)surface\n"
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7663,16 +7663,18 @@
 
   // When the function name has to be wrapped.
   FormatStyle Style = getLLVMStyle();
+  // ObjC ignores IndentWrappedFunctionNames when wrapping methods
+  // and always indents instead.
   Style.IndentWrappedFunctionNames = false;
   verifyFormat("- (SomeLongType *)\n"
-   "veryLooongName:(NSString)aa\n"
-   "   anotherName:(NSString)bb {\n"
+   "veryLooongName:(NSString)aa\n"
+   "   anotherName:(NSString)bb {\n"
"}",
Style);
   Style.IndentWrappedFunctionNames = true;
   verifyFormat("- (SomeLongType *)\n"
-   "veryLooongName:(NSString)aa\n"
-   "   anotherName:(NSString)bb {\n"
+   "veryLooongName:(NSString)cc\n"
+   "   anotherName:(NSString)dd {\n"
"}",
Style);
 
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/Co

[PATCH] D41168: [X86] Lowering X86 avx512 sqrt intrinsics to IR

2018-04-12 Thread Tomasz Krupa via Phabricator via cfe-commits
tkrupa added a comment.

I'll wait with upstreaming this patch until there's an agreement on mask scalar 
approach.


Repository:
  rC Clang

https://reviews.llvm.org/D41168



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45004: [clang-format] Always indent wrapped Objective-C selector names

2018-04-12 Thread Ben Hamilton via Phabricator via cfe-commits
benhamilton added inline comments.



Comment at: unittests/Format/FormatTest.cpp:7666
   FormatStyle Style = getLLVMStyle();
+  // ObjC ignores IndentWrappedFunctionNames when wrapping methods.
   Style.IndentWrappedFunctionNames = false;

djasper wrote:
> maybe: .. and always indents. ?
Sure, done.


Repository:
  rC Clang

https://reviews.llvm.org/D45004



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45498: [clang-format] Don't insert space between ObjC class and lightweight generic

2018-04-12 Thread Ben Hamilton via Phabricator via cfe-commits
benhamilton updated this revision to Diff 142186.
benhamilton marked an inline comment as done.
benhamilton added a comment.

Avoid else after return. Clean up logic so it doesn't need a comment.


Repository:
  rC Clang

https://reviews.llvm.org/D45498

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -299,13 +299,13 @@
"+ (id)init;\n"
"@end");
 
-  verifyFormat("@interface Foo  : Bar  {\n"
+  verifyFormat("@interface Foo : Bar  {\n"
"  int _i;\n"
"}\n"
"+ (id)init;\n"
"@end");
 
-  verifyFormat("@interface Foo > : Xyzzy  {\n"
+  verifyFormat("@interface Foo> : Xyzzy  {\n"
"  int _i;\n"
"}\n"
"+ (id)init;\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2349,9 +2349,12 @@
: Style.SpacesInParentheses;
   if (Right.isOneOf(tok::semi, tok::comma))
 return false;
-  if (Right.is(tok::less) && Line.Type == LT_ObjCDecl &&
-  Style.ObjCSpaceBeforeProtocolList)
-return true;
+  if (Right.is(tok::less) && Line.Type == LT_ObjCDecl) {
+bool IsLightweightGeneric =
+Right.MatchingParen && Right.MatchingParen->Next &&
+Right.MatchingParen->Next->is(tok::colon);
+return !IsLightweightGeneric && Style.ObjCSpaceBeforeProtocolList;
+  }
   if (Right.is(tok::less) && Left.is(tok::kw_template))
 return Style.SpaceAfterTemplateKeyword;
   if (Left.isOneOf(tok::exclaim, tok::tilde))


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -299,13 +299,13 @@
"+ (id)init;\n"
"@end");
 
-  verifyFormat("@interface Foo  : Bar  {\n"
+  verifyFormat("@interface Foo : Bar  {\n"
"  int _i;\n"
"}\n"
"+ (id)init;\n"
"@end");
 
-  verifyFormat("@interface Foo > : Xyzzy  {\n"
+  verifyFormat("@interface Foo> : Xyzzy  {\n"
"  int _i;\n"
"}\n"
"+ (id)init;\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2349,9 +2349,12 @@
: Style.SpacesInParentheses;
   if (Right.isOneOf(tok::semi, tok::comma))
 return false;
-  if (Right.is(tok::less) && Line.Type == LT_ObjCDecl &&
-  Style.ObjCSpaceBeforeProtocolList)
-return true;
+  if (Right.is(tok::less) && Line.Type == LT_ObjCDecl) {
+bool IsLightweightGeneric =
+Right.MatchingParen && Right.MatchingParen->Next &&
+Right.MatchingParen->Next->is(tok::colon);
+return !IsLightweightGeneric && Style.ObjCSpaceBeforeProtocolList;
+  }
   if (Right.is(tok::less) && Left.is(tok::kw_template))
 return Style.SpaceAfterTemplateKeyword;
   if (Left.isOneOf(tok::exclaim, tok::tilde))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition

2018-04-12 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

We encountered the same problem but did not have time yet to submit the patch. 
We have literally the same fix internally, so it looks good to me. One minor 
style nit inline.

Could you add your repro as a regression test? You can also extend existing CTU 
tests just make sure to trigger the crash before the patch.

Thank you for the submission and the minimal reproducer.




Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:390
   const FunctionDecl *FD = getDecl();
+  if (!FD) {
+return {};

We usually do not write the braces for single statements.


Repository:
  rC Clang

https://reviews.llvm.org/D45564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r329917 - [clang-format] Don't insert space between ObjC class and lightweight generic

2018-04-12 Thread Ben Hamilton via cfe-commits
Author: benhamilton
Date: Thu Apr 12 08:11:51 2018
New Revision: 329917

URL: http://llvm.org/viewvc/llvm-project?rev=329917&view=rev
Log:
[clang-format] Don't insert space between ObjC class and lightweight generic

Summary:
In D45185, I added clang-format parser support for Objective-C
generics. However, I didn't touch the whitespace logic, so they
got the same space logic as Objective-C protocol lists.

In every example in the Apple SDK and in the documentation,
there is no space between the class name and the opening `<`
for the lightweight generic specification, so this diff
removes the space and updates the tests.

Test Plan: Tests updated. Ran tests with:
  % make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests

Reviewers: djasper, jolesiak

Reviewed By: djasper

Subscribers: klimek, cfe-commits

Differential Revision: https://reviews.llvm.org/D45498

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=329917&r1=329916&r2=329917&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Thu Apr 12 08:11:51 2018
@@ -2349,9 +2349,12 @@ bool TokenAnnotator::spaceRequiredBetwee
: Style.SpacesInParentheses;
   if (Right.isOneOf(tok::semi, tok::comma))
 return false;
-  if (Right.is(tok::less) && Line.Type == LT_ObjCDecl &&
-  Style.ObjCSpaceBeforeProtocolList)
-return true;
+  if (Right.is(tok::less) && Line.Type == LT_ObjCDecl) {
+bool IsLightweightGeneric =
+Right.MatchingParen && Right.MatchingParen->Next &&
+Right.MatchingParen->Next->is(tok::colon);
+return !IsLightweightGeneric && Style.ObjCSpaceBeforeProtocolList;
+  }
   if (Right.is(tok::less) && Left.is(tok::kw_template))
 return Style.SpaceAfterTemplateKeyword;
   if (Left.isOneOf(tok::exclaim, tok::tilde))

Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=329917&r1=329916&r2=329917&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Thu Apr 12 08:11:51 2018
@@ -299,13 +299,13 @@ TEST_F(FormatTestObjC, FormatObjCInterfa
"+ (id)init;\n"
"@end");
 
-  verifyFormat("@interface Foo  : Bar  {\n"
+  verifyFormat("@interface Foo : Bar  {\n"
"  int _i;\n"
"}\n"
"+ (id)init;\n"
"@end");
 
-  verifyFormat("@interface Foo > : Xyzzy  {\n"
+  verifyFormat("@interface Foo> : Xyzzy  {\n"
"  int _i;\n"
"}\n"
"+ (id)init;\n"


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r329916 - [clang-format] Always indent wrapped Objective-C selector names

2018-04-12 Thread Ben Hamilton via cfe-commits
Author: benhamilton
Date: Thu Apr 12 08:11:48 2018
New Revision: 329916

URL: http://llvm.org/viewvc/llvm-project?rev=329916&view=rev
Log:
[clang-format] Always indent wrapped Objective-C selector names

Summary:
Currently, indentation of Objective-C method names which are wrapped
onto the next line due to a long return type is controlled by the
style option `IndentWrappedFunctionNames`.

This diff changes the behavior so we always indent wrapped Objective-C
selector names.

NOTE: I partially reverted 
https://github.com/llvm-mirror/clang/commit/6159c0fbd1876c7f5f984b4830c664cc78f16e2e
 / rL242484, as it was causing wrapped selectors to be double-indented. Its 
tests in FormatTestObjC.cpp still pass.

Test Plan: Tests updated. Ran tests with:
  % make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests

Reviewers: djasper, jolesiak, stephanemoore, thakis

Reviewed By: djasper

Subscribers: stephanemoore, klimek, cfe-commits

Differential Revision: https://reviews.llvm.org/D45004

Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=329916&r1=329915&r2=329916&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Thu Apr 12 08:11:48 2018
@@ -26,6 +26,13 @@
 namespace clang {
 namespace format {
 
+// Returns true if a TT_SelectorName should be indented when wrapped,
+// false otherwise.
+static bool shouldIndentWrappedSelectorName(const FormatStyle &Style,
+LineType LineType) {
+  return Style.IndentWrappedFunctionNames || LineType == LT_ObjCMethodDecl;
+}
+
 // Returns the length of everything up to the first possible line break after
 // the ), ], } or > matching \c Tok.
 static unsigned getLengthToMatchingParen(const FormatToken &Tok) {
@@ -698,7 +705,7 @@ unsigned ContinuationIndenter::addTokenO
 State.Stack.back().AlignColons = false;
   } else {
 State.Stack.back().ColonPos =
-(Style.IndentWrappedFunctionNames
+(shouldIndentWrappedSelectorName(Style, State.Line->Type)
  ? std::max(State.Stack.back().Indent,
 State.FirstIndent + Style.ContinuationIndentWidth)
  : State.Stack.back().Indent) +
@@ -897,7 +904,7 @@ unsigned ContinuationIndenter::getNewLin
   if (NextNonComment->is(TT_SelectorName)) {
 if (!State.Stack.back().ObjCSelectorNameFound) {
   unsigned MinIndent = State.Stack.back().Indent;
-  if (Style.IndentWrappedFunctionNames)
+  if (shouldIndentWrappedSelectorName(Style, State.Line->Type))
 MinIndent = std::max(MinIndent,
  State.FirstIndent + 
Style.ContinuationIndentWidth);
   // If LongestObjCSelectorName is 0, we are indenting the first
@@ -1000,13 +1007,8 @@ unsigned ContinuationIndenter::moveState
   if (Current.isMemberAccess())
 State.Stack.back().StartOfFunctionCall =
 !Current.NextOperator ? 0 : State.Column;
-  if (Current.is(TT_SelectorName)) {
+  if (Current.is(TT_SelectorName))
 State.Stack.back().ObjCSelectorNameFound = true;
-if (Style.IndentWrappedFunctionNames) {
-  State.Stack.back().Indent =
-  State.FirstIndent + Style.ContinuationIndentWidth;
-}
-  }
   if (Current.is(TT_CtorInitializerColon) &&
   Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon) {
 // Indent 2 from the column, so:

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=329916&r1=329915&r2=329916&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Apr 12 08:11:48 2018
@@ -7678,16 +7678,18 @@ TEST_F(FormatTest, FormatForObjectiveCMe
 
   // When the function name has to be wrapped.
   FormatStyle Style = getLLVMStyle();
+  // ObjC ignores IndentWrappedFunctionNames when wrapping methods
+  // and always indents instead.
   Style.IndentWrappedFunctionNames = false;
   verifyFormat("- (SomeLongType *)\n"
-   "veryLooongName:(NSString)aa\n"
-   "   anotherName:(NSString)bb {\n"
+   "veryLooongName:(NSString)aa\n"
+   "   anotherName:(NSString)bb {\n"
"}",
Style);
   Style.IndentWrappedFunctionNames = true;
   verifyFormat("- (SomeLongType *)\n"
-   "veryLooongName:(NSString)aa\n"
- 

r329919 - [clang-format] Do not break after ObjC category open paren

2018-04-12 Thread Ben Hamilton via cfe-commits
Author: benhamilton
Date: Thu Apr 12 08:11:55 2018
New Revision: 329919

URL: http://llvm.org/viewvc/llvm-project?rev=329919&view=rev
Log:
[clang-format] Do not break after ObjC category open paren

Summary:
Previously, `clang-format` would break Objective-C
category extensions after the opening parenthesis to avoid
breaking the protocol list:

```
% echo "@interface c (ccc)  { }" | \
  clang-format -assume-filename=foo.h -style="{BasedOnStyle: llvm, \
  ColumnLimit: 40}"
@interface c (
ccc)  {
}
```

This looks fairly odd, as we could have kept the category extension
on the previous line.

Category extensions are a single item, so they are generally very
short compared to protocol lists. We should prefer breaking after the
opening `<` of the protocol list over breaking after the opening `(`
of the category extension.

With this diff, we now avoid breaking after the category extension's
open paren, which causes us to break after the protocol list's
open angle bracket:

```
% echo "@interface c (ccc)  { }" | \
  ./bin/clang-format -assume-filename=foo.h -style="{BasedOnStyle: llvm, \
  ColumnLimit: 40}"
@interface c (ccc) <
c> {
}
```

Test Plan: New test added. Confirmed test failed before diff and
  passed after diff by running:
  % make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests

Reviewers: djasper, jolesiak

Reviewed By: djasper

Subscribers: klimek, cfe-commits

Differential Revision: https://reviews.llvm.org/D45526

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=329919&r1=329918&r2=329919&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Thu Apr 12 08:11:55 2018
@@ -2272,6 +2272,13 @@ unsigned TokenAnnotator::splitPenalty(co
   if (Left.is(tok::colon) && Left.is(TT_ObjCMethodExpr))
 return Line.MightBeFunctionDecl ? 50 : 500;
 
+  // In Objective-C type declarations, avoid breaking after the category's
+  // open paren (we'll prefer breaking after the protocol list's opening
+  // angle bracket, if present).
+  if (Line.Type == LT_ObjCDecl && Left.is(tok::l_paren) && Left.Previous &&
+  Left.Previous->isOneOf(tok::identifier, tok::greater))
+return 500;
+
   if (Left.is(tok::l_paren) && InFunctionDecl &&
   Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign)
 return 100;

Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=329919&r1=329918&r2=329919&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Thu Apr 12 08:11:55 2018
@@ -334,6 +334,9 @@ TEST_F(FormatTestObjC, FormatObjCInterfa
"c, c,\n"
"c, c> {\n"
"}");
+  verifyFormat("@interface c (ccc) <\n"
+   "c> {\n"
+   "}");
   Style.ObjCBinPackProtocolList = FormatStyle::BPS_Never;
   verifyFormat("@interface d () <\n"
"d,\n"


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45521: [clang-format] Improve ObjC guessing heuristic by supporting all @keywords

2018-04-12 Thread Ben Hamilton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329918: [clang-format] Improve ObjC guessing heuristic by 
supporting all @keywords (authored by benhamilton, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D45521

Files:
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp


Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -1465,6 +1465,7 @@
 "NSAffineTransform",
 "NSArray",
 "NSAttributedString",
+"NSBlockOperation",
 "NSBundle",
 "NSCache",
 "NSCalendar",
@@ -1480,6 +1481,7 @@
 "NSIndexPath",
 "NSIndexSet",
 "NSInteger",
+"NSInvocationOperation",
 "NSLocale",
 "NSMapTable",
 "NSMutableArray",
@@ -1494,9 +1496,13 @@
 "NSNumber",
 "NSNumberFormatter",
 "NSObject",
+"NSOperation",
+"NSOperationQueue",
+"NSOperationQueuePriority",
 "NSOrderedSet",
 "NSPoint",
 "NSPointerArray",
+"NSQualityOfService",
 "NSRange",
 "NSRect",
 "NSRegularExpression",
@@ -1518,10 +1524,7 @@
   for (const FormatToken *FormatTok = Line->First; FormatTok;
FormatTok = FormatTok->Next) {
 if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
- (FormatTok->isObjCAtKeyword(tok::objc_interface) ||
-  FormatTok->isObjCAtKeyword(tok::objc_implementation) ||
-  FormatTok->isObjCAtKeyword(tok::objc_protocol) ||
-  FormatTok->isObjCAtKeyword(tok::objc_end) ||
+ (FormatTok->Tok.getObjCKeywordID() != tok::objc_not_keyword ||
   FormatTok->isOneOf(tok::numeric_constant, tok::l_square,
  tok::l_brace))) ||
 (FormatTok->Tok.isAnyIdentifier() &&
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -12118,6 +12118,12 @@
   EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.mm", ""));
   EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", ""));
   EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "@interface 
Foo\n@end\n"));
+  EXPECT_EQ(
+  FormatStyle::LK_ObjC,
+  guessLanguage("foo.h", "#define TRY(x, y) @try { x; } @finally { y; }"));
+  EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "#define AVAIL(x) @available(x, *))"));
+  EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "@class Foo;"));
   EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo", ""));
   EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo", "@interface 
Foo\n@end\n"));
   EXPECT_EQ(FormatStyle::LK_ObjC,


Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -1465,6 +1465,7 @@
 "NSAffineTransform",
 "NSArray",
 "NSAttributedString",
+"NSBlockOperation",
 "NSBundle",
 "NSCache",
 "NSCalendar",
@@ -1480,6 +1481,7 @@
 "NSIndexPath",
 "NSIndexSet",
 "NSInteger",
+"NSInvocationOperation",
 "NSLocale",
 "NSMapTable",
 "NSMutableArray",
@@ -1494,9 +1496,13 @@
 "NSNumber",
 "NSNumberFormatter",
 "NSObject",
+"NSOperation",
+"NSOperationQueue",
+"NSOperationQueuePriority",
 "NSOrderedSet",
 "NSPoint",
 "NSPointerArray",
+"NSQualityOfService",
 "NSRange",
 "NSRect",
 "NSRegularExpression",
@@ -1518,10 +1524,7 @@
   for (const FormatToken *FormatTok = Line->First; FormatTok;
FormatTok = FormatTok->Next) {
 if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
- (FormatTok->isObjCAtKeyword(tok::objc_interface) ||
-  FormatTok->isObjCAtKeyword(tok::objc_implementation) ||
-  FormatTok->isObjCAtKeyword(tok::objc_protocol) ||
-  FormatTok->isObjCAtKeyword(tok::objc_end) ||
+ (FormatTok->Tok.getObjCKeywordID() != tok::objc_not_keyword ||
   FormatTok->isOneOf(tok::numeric_constant, tok::l_square,
  tok::l_brace))) ||
 (FormatTok->Tok.isAnyIdentifier() &&
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -12118,6 +12118,12 @@
   EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.mm", ""));
   EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h

[PATCH] D45498: [clang-format] Don't insert space between ObjC class and lightweight generic

2018-04-12 Thread Ben Hamilton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329917: [clang-format] Don't insert space between ObjC 
class and lightweight generic (authored by benhamilton, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D45498

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTestObjC.cpp


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2349,9 +2349,12 @@
: Style.SpacesInParentheses;
   if (Right.isOneOf(tok::semi, tok::comma))
 return false;
-  if (Right.is(tok::less) && Line.Type == LT_ObjCDecl &&
-  Style.ObjCSpaceBeforeProtocolList)
-return true;
+  if (Right.is(tok::less) && Line.Type == LT_ObjCDecl) {
+bool IsLightweightGeneric =
+Right.MatchingParen && Right.MatchingParen->Next &&
+Right.MatchingParen->Next->is(tok::colon);
+return !IsLightweightGeneric && Style.ObjCSpaceBeforeProtocolList;
+  }
   if (Right.is(tok::less) && Left.is(tok::kw_template))
 return Style.SpaceAfterTemplateKeyword;
   if (Left.isOneOf(tok::exclaim, tok::tilde))
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -299,13 +299,13 @@
"+ (id)init;\n"
"@end");
 
-  verifyFormat("@interface Foo  : Bar  {\n"
+  verifyFormat("@interface Foo : Bar  {\n"
"  int _i;\n"
"}\n"
"+ (id)init;\n"
"@end");
 
-  verifyFormat("@interface Foo > : Xyzzy  {\n"
+  verifyFormat("@interface Foo> : Xyzzy  {\n"
"  int _i;\n"
"}\n"
"+ (id)init;\n"


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2349,9 +2349,12 @@
: Style.SpacesInParentheses;
   if (Right.isOneOf(tok::semi, tok::comma))
 return false;
-  if (Right.is(tok::less) && Line.Type == LT_ObjCDecl &&
-  Style.ObjCSpaceBeforeProtocolList)
-return true;
+  if (Right.is(tok::less) && Line.Type == LT_ObjCDecl) {
+bool IsLightweightGeneric =
+Right.MatchingParen && Right.MatchingParen->Next &&
+Right.MatchingParen->Next->is(tok::colon);
+return !IsLightweightGeneric && Style.ObjCSpaceBeforeProtocolList;
+  }
   if (Right.is(tok::less) && Left.is(tok::kw_template))
 return Style.SpaceAfterTemplateKeyword;
   if (Left.isOneOf(tok::exclaim, tok::tilde))
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -299,13 +299,13 @@
"+ (id)init;\n"
"@end");
 
-  verifyFormat("@interface Foo  : Bar  {\n"
+  verifyFormat("@interface Foo : Bar  {\n"
"  int _i;\n"
"}\n"
"+ (id)init;\n"
"@end");
 
-  verifyFormat("@interface Foo > : Xyzzy  {\n"
+  verifyFormat("@interface Foo> : Xyzzy  {\n"
"  int _i;\n"
"}\n"
"+ (id)init;\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r329918 - [clang-format] Improve ObjC guessing heuristic by supporting all @keywords

2018-04-12 Thread Ben Hamilton via cfe-commits
Author: benhamilton
Date: Thu Apr 12 08:11:53 2018
New Revision: 329918

URL: http://llvm.org/viewvc/llvm-project?rev=329918&view=rev
Log:
[clang-format] Improve ObjC guessing heuristic by supporting all @keywords

Summary:
This diff improves the Objective-C guessing heuristic by
replacing the hard-coded list of a subset of Objective-C @keywords
with a general check which supports all @keywords.

I also added a few more Foundation keywords which were missing from
the heuristic.

Test Plan: Unit tests updated. Ran tests with:
  % make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests

Reviewers: djasper, jolesiak

Reviewed By: djasper

Subscribers: klimek, cfe-commits

Differential Revision: https://reviews.llvm.org/D45521

Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=329918&r1=329917&r2=329918&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Thu Apr 12 08:11:53 2018
@@ -1465,6 +1465,7 @@ private:
 "NSAffineTransform",
 "NSArray",
 "NSAttributedString",
+"NSBlockOperation",
 "NSBundle",
 "NSCache",
 "NSCalendar",
@@ -1480,6 +1481,7 @@ private:
 "NSIndexPath",
 "NSIndexSet",
 "NSInteger",
+"NSInvocationOperation",
 "NSLocale",
 "NSMapTable",
 "NSMutableArray",
@@ -1494,9 +1496,13 @@ private:
 "NSNumber",
 "NSNumberFormatter",
 "NSObject",
+"NSOperation",
+"NSOperationQueue",
+"NSOperationQueuePriority",
 "NSOrderedSet",
 "NSPoint",
 "NSPointerArray",
+"NSQualityOfService",
 "NSRange",
 "NSRect",
 "NSRegularExpression",
@@ -1518,10 +1524,7 @@ private:
   for (const FormatToken *FormatTok = Line->First; FormatTok;
FormatTok = FormatTok->Next) {
 if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
- (FormatTok->isObjCAtKeyword(tok::objc_interface) ||
-  FormatTok->isObjCAtKeyword(tok::objc_implementation) ||
-  FormatTok->isObjCAtKeyword(tok::objc_protocol) ||
-  FormatTok->isObjCAtKeyword(tok::objc_end) ||
+ (FormatTok->Tok.getObjCKeywordID() != tok::objc_not_keyword ||
   FormatTok->isOneOf(tok::numeric_constant, tok::l_square,
  tok::l_brace))) ||
 (FormatTok->Tok.isAnyIdentifier() &&

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=329918&r1=329917&r2=329918&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Apr 12 08:11:53 2018
@@ -12118,6 +12118,12 @@ TEST_F(FormatTest, FileAndCode) {
   EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.mm", ""));
   EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", ""));
   EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "@interface 
Foo\n@end\n"));
+  EXPECT_EQ(
+  FormatStyle::LK_ObjC,
+  guessLanguage("foo.h", "#define TRY(x, y) @try { x; } @finally { y; }"));
+  EXPECT_EQ(FormatStyle::LK_ObjC,
+guessLanguage("foo.h", "#define AVAIL(x) @available(x, *))"));
+  EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo.h", "@class Foo;"));
   EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo", ""));
   EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo", "@interface 
Foo\n@end\n"));
   EXPECT_EQ(FormatStyle::LK_ObjC,


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45004: [clang-format] Always indent wrapped Objective-C selector names

2018-04-12 Thread Ben Hamilton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC329916: [clang-format] Always indent wrapped Objective-C 
selector names (authored by benhamilton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45004?vs=142183&id=142187#toc

Repository:
  rC Clang

https://reviews.llvm.org/D45004

Files:
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/FormatTestObjC.cpp

Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -26,6 +26,13 @@
 namespace clang {
 namespace format {
 
+// Returns true if a TT_SelectorName should be indented when wrapped,
+// false otherwise.
+static bool shouldIndentWrappedSelectorName(const FormatStyle &Style,
+LineType LineType) {
+  return Style.IndentWrappedFunctionNames || LineType == LT_ObjCMethodDecl;
+}
+
 // Returns the length of everything up to the first possible line break after
 // the ), ], } or > matching \c Tok.
 static unsigned getLengthToMatchingParen(const FormatToken &Tok) {
@@ -698,7 +705,7 @@
 State.Stack.back().AlignColons = false;
   } else {
 State.Stack.back().ColonPos =
-(Style.IndentWrappedFunctionNames
+(shouldIndentWrappedSelectorName(Style, State.Line->Type)
  ? std::max(State.Stack.back().Indent,
 State.FirstIndent + Style.ContinuationIndentWidth)
  : State.Stack.back().Indent) +
@@ -897,7 +904,7 @@
   if (NextNonComment->is(TT_SelectorName)) {
 if (!State.Stack.back().ObjCSelectorNameFound) {
   unsigned MinIndent = State.Stack.back().Indent;
-  if (Style.IndentWrappedFunctionNames)
+  if (shouldIndentWrappedSelectorName(Style, State.Line->Type))
 MinIndent = std::max(MinIndent,
  State.FirstIndent + Style.ContinuationIndentWidth);
   // If LongestObjCSelectorName is 0, we are indenting the first
@@ -1000,13 +1007,8 @@
   if (Current.isMemberAccess())
 State.Stack.back().StartOfFunctionCall =
 !Current.NextOperator ? 0 : State.Column;
-  if (Current.is(TT_SelectorName)) {
+  if (Current.is(TT_SelectorName))
 State.Stack.back().ObjCSelectorNameFound = true;
-if (Style.IndentWrappedFunctionNames) {
-  State.Stack.back().Indent =
-  State.FirstIndent + Style.ContinuationIndentWidth;
-}
-  }
   if (Current.is(TT_CtorInitializerColon) &&
   Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon) {
 // Indent 2 from the column, so:
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7678,16 +7678,18 @@
 
   // When the function name has to be wrapped.
   FormatStyle Style = getLLVMStyle();
+  // ObjC ignores IndentWrappedFunctionNames when wrapping methods
+  // and always indents instead.
   Style.IndentWrappedFunctionNames = false;
   verifyFormat("- (SomeLongType *)\n"
-   "veryLooongName:(NSString)aa\n"
-   "   anotherName:(NSString)bb {\n"
+   "veryLooongName:(NSString)aa\n"
+   "   anotherName:(NSString)bb {\n"
"}",
Style);
   Style.IndentWrappedFunctionNames = true;
   verifyFormat("- (SomeLongType *)\n"
-   "veryLooongName:(NSString)aa\n"
-   "   anotherName:(NSString)bb {\n"
+   "veryLooongName:(NSString)cc\n"
+   "   anotherName:(NSString)dd {\n"
"}",
Style);
 
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -536,28 +536,25 @@
"ofSize:(size_t)height\n"
"  :(size_t)width;");
   Style.ColumnLimit = 40;
-  // Make sure selectors with 0, 1, or more arguments are not indented
-  // when IndentWrappedFunctionNames is false.
-  Style.IndentWrappedFunctionNames = false;
+  // Make sure selectors with 0, 1, or more arguments are indented when wrapped.
   verifyFormat("- (a)\n"
-   ";\n");
+   ";\n");
   verifyFormat("- (a)\n"
-   ":(int)a;\n");
+   ":(int)a;\n");
   verifyFormat("- (a)\n"
-   "aa

[PATCH] D45526: [clang-format] Do not break after ObjC category open paren

2018-04-12 Thread Ben Hamilton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329919: [clang-format] Do not break after ObjC category open 
paren (authored by benhamilton, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D45526

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTestObjC.cpp


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2272,6 +2272,13 @@
   if (Left.is(tok::colon) && Left.is(TT_ObjCMethodExpr))
 return Line.MightBeFunctionDecl ? 50 : 500;
 
+  // In Objective-C type declarations, avoid breaking after the category's
+  // open paren (we'll prefer breaking after the protocol list's opening
+  // angle bracket, if present).
+  if (Line.Type == LT_ObjCDecl && Left.is(tok::l_paren) && Left.Previous &&
+  Left.Previous->isOneOf(tok::identifier, tok::greater))
+return 500;
+
   if (Left.is(tok::l_paren) && InFunctionDecl &&
   Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign)
 return 100;
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -334,6 +334,9 @@
"c, c,\n"
"c, c> {\n"
"}");
+  verifyFormat("@interface c (ccc) <\n"
+   "c> {\n"
+   "}");
   Style.ObjCBinPackProtocolList = FormatStyle::BPS_Never;
   verifyFormat("@interface d () <\n"
"d,\n"


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2272,6 +2272,13 @@
   if (Left.is(tok::colon) && Left.is(TT_ObjCMethodExpr))
 return Line.MightBeFunctionDecl ? 50 : 500;
 
+  // In Objective-C type declarations, avoid breaking after the category's
+  // open paren (we'll prefer breaking after the protocol list's opening
+  // angle bracket, if present).
+  if (Line.Type == LT_ObjCDecl && Left.is(tok::l_paren) && Left.Previous &&
+  Left.Previous->isOneOf(tok::identifier, tok::greater))
+return 500;
+
   if (Left.is(tok::l_paren) && InFunctionDecl &&
   Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign)
 return 100;
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -334,6 +334,9 @@
"c, c,\n"
"c, c> {\n"
"}");
+  verifyFormat("@interface c (ccc) <\n"
+   "c> {\n"
+   "}");
   Style.ObjCBinPackProtocolList = FormatStyle::BPS_Never;
   verifyFormat("@interface d () <\n"
"d,\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45458: MallocChecker refactoring of calls checkers

2018-04-12 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

In https://reviews.llvm.org/D45458#1062342, @NoQ wrote:

> @xazax.hun: do you think the approach you took in the `Valist` checker is 
> applicable here? Did you like how it ended up working? Cause i'd love to see 
> `CallDescription` and initializer lists used for readability here. And i'd 
> love to see branches replaced with map lookups. And i'm not sure if anybody 
> has written code that has all the stuff that i'd love to see.


Overall the `CallDescription` interface works really well if you do not need 
anything more advanced, e.g. supporting overloads. I can imagine that its 
current implementation is slightly less efficient since the lazy initialization 
logic of the identifiers are repeated for every identifier but that also makes 
it less bug-prone (do not have the implicit assumption that a set of 
identifiers should always exist together).

It would be great to extend the interface in the future, but one of the reasons 
I was reluctant to do so yet is performance. E.g.: having a simple check that a 
checker is interested only in global C functions is better than having this 
check repeated in every call description object in an array, or making it 
possible to do a StringSwitch like optimization with multiple statically known 
CallDescription objects would also be awesome.

But I think most of the time these tests would not be used on the hot paths 
anyways, so as long its current expressive power is sufficient, I prefer code 
that is utilizing this tool. It tends to make the code slightly shorter.


Repository:
  rC Clang

https://reviews.llvm.org/D45458



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45177: CStringChecker, check strlcpy/strlcat

2018-04-12 Thread David CARLIER via Phabricator via cfe-commits
devnexen added a comment.

ping


Repository:
  rC Clang

https://reviews.llvm.org/D45177



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45513: [clangd] Add line and column number to the index symbol.

2018-04-12 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

LG, with

- consider reverting the bitpacking stuff
- comment about utf-16
- clang-format :)




Comment at: clangd/index/SymbolCollector.cpp:202
+SymbolLocation::Position Pos;
+// Position is 0-based while source location is 1-based.
+Pos.Line = SM.getLineNumber(FileId, Offset) - 1;

nit: SourceManager is 1-based (or returns 1-based data here).
SourceLocation uses 0-based offsets, not 1-based line/column.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45513



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-12 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment.

Thank you for all your comments so far! I'll probably only be able to update 
the diff tomorrow (with me being in the GMT + 1 timezone).

> That's a lot of code, so it'll take me some time to understand what's going 
> on here. You might be able to help me by the large patch in smaller logical 
> chunks.

I know, and I tried to look for ways to split this checker into smaller logical 
chunks, but I didn't find a satisfying solution. I'll be sure to review the 
comments I have in the code so it's as easy as possible to understand what I 
did and why I did it. Also, I'd be delighted to help any way I can to guide you 
through the code! :)

> That said, your checker finds non-bugs, which is always risky. Sometimes the 
> user should be allowed to initialize the field after construction but before 
> the first use as a potentially important performance optimization. [...] If a 
> user wants to have his codebase "analyzer-clean", would he be able to 
> suppress the warning without changing the behavior of the program?

I didn't implement anything explicitly to suppress warnings, so if there is 
nothing in the CSA by default for it, I'm afraid this isn't possible (just yet).

My checker is mainly a tool to enforce the rule that every field should be 
initialized in a C++ object. While I see that this approach could result  
reduced performance, I think it's fine to say that those users who find this 
important (by 'this' I mean not initializing every field) should not enable 
this checker.

Nevertheless, I'd be happy to know what you think of this.

> Did you find any such intentionally uninitialized fields with your checker? 
> Were there many of those?

I ran the checker on some projects, but it's little difficult to analyze larger 
ones as this checker emits very important information in notes, and those are 
not yet part of the plist output. But it's coming: 
https://reviews.llvm.org/D45407! I'll be sure to answer these questions as soon 
as I can.




Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:187
+
+  if (isCalledByConstructor(Context))
+return;

whisperity wrote:
> I think somewhere there should be a bit of reasoning why exactly these cases 
> are ignored by the checker.
At this function's declaration I have some comments describing what this does 
and why it does it. Did you find it insufficient?



Comment at: test/Analysis/ctor-uninitialized-member-inheritance.cpp:24
+  : NonPolymorphicLeft1(int{}) {
+y = 420;
+z = 420;

whisperity wrote:
> The literal `420` is repeated //everywhere// in this file. I think this (the 
> same value appearing over and over again) will make debugging bad if 
> something goes haywire and one has to look at memory dumps, 
> control-flow-graphs, etc.
Would you say that I should rather use a different number for each test case?


https://reviews.llvm.org/D45532



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-12 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments.



Comment at: test/Analysis/ctor-uninitialized-member-inheritance.cpp:24
+  : NonPolymorphicLeft1(int{}) {
+y = 420;
+z = 420;

Szelethus wrote:
> whisperity wrote:
> > The literal `420` is repeated //everywhere// in this file. I think this 
> > (the same value appearing over and over again) will make debugging bad if 
> > something goes haywire and one has to look at memory dumps, 
> > control-flow-graphs, etc.
> Would you say that I should rather use a different number for each test case?
I mean, a different number for each variable within a test case.


https://reviews.llvm.org/D45532



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output

2018-04-12 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment.

Did you have some time to check on those programs? :)


Repository:
  rC Clang

https://reviews.llvm.org/D45407



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45513: [clangd] Add line and column number to the index symbol.

2018-04-12 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clangd/index/Index.h:32
+// Character offset on a line in a document (zero-based).
+int Character = 0;
+  };

sammccall wrote:
> sammccall wrote:
> > Column?
> > 
> > LSP calls this "character" but this is nonstandard and I find it very 
> > confusing with offset. 
> We should document what this is an offset into: bytes, utf-16 code units, or 
> unicode codepoints. (Or even grid offsets - glyphs and doublewidth are a 
> thing)
> 
> Given that we intend to send it over LSP without reading the source, only 
> utf-16 code units is really correct. Unicode codepoints is "nicer" and will 
> give correct results in the BMP, while bytes will be correct for ASCII only.
> 
> I'd vote for making this utf-16 code units.
> 
> It's OK if the code populating it doesn't get this right (confuses bytes and 
> code units) but add a fixme.
Done. Added FIXME.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45513



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-12 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:31
+/// every other element is a field, and the element that precedes it is the
+/// object that contains it.
+class FieldChainInfo {

As far as I understand, for every operation, the only relevant contribution of 
the non-last elements in the chain is the diagnostic message.
I wonder if you would build a string instead of a FieldRegion chain and only 
store the last FieldRegion would make this more explicit in the code. 



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:187
+
+  if (isCalledByConstructor(Context))
+return;

Szelethus wrote:
> whisperity wrote:
> > I think somewhere there should be a bit of reasoning why exactly these 
> > cases are ignored by the checker.
> At this function's declaration I have some comments describing what this does 
> and why it does it. Did you find it insufficient?
I am a bit surprised why these errors are not deduplicated by the analyzer. 
Maybe it would worth some investigation?



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:272
+// If Chain's value is None, that means that this function was called to
+// determine whether a union has any initialized fields. In this case we're not
+// collecting fields, we'd only like to know  whether the value contained in

I find the documentation and the name of the function below misleading. 
`isNonUnionUninit` tells me this function is not supposed to be called for 
unions but the documentation suggests otherwise.



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:283
+  bool ContainsUninitField = false;
+  Optional LocalChain = Chain;
+

Why do you copy here explicitly instead of taking the `Chain` argument by value?



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:292
+  if (LocalChain)
+LocalChain->push_back(FR);
+  if (isNonUnionUninit(FR, LocalChain))

Don't you need to pop somewhere?


https://reviews.llvm.org/D45532



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45513: [clangd] Add line and column number to the index symbol.

2018-04-12 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 142195.
hokein marked 3 inline comments as done.
hokein added a comment.

Update the patch, address remaining issues.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45513

Files:
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolYAML.cpp
  unittests/clangd/Annotations.cpp
  unittests/clangd/Annotations.h
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -51,13 +51,20 @@
 MATCHER_P(IncludeHeader, P, "") {
   return arg.Detail && arg.Detail->IncludeHeader == P;
 }
-MATCHER_P(DeclRange, Offsets, "") {
-  return arg.CanonicalDeclaration.StartOffset == Offsets.first &&
-  arg.CanonicalDeclaration.EndOffset == Offsets.second;
-}
-MATCHER_P(DefRange, Offsets, "") {
-  return arg.Definition.StartOffset == Offsets.first &&
- arg.Definition.EndOffset == Offsets.second;
+MATCHER_P(DeclRange, Pos, "") {
+  return std::tie(arg.CanonicalDeclaration.Start.Line,
+  arg.CanonicalDeclaration.Start.Column,
+  arg.CanonicalDeclaration.End.Line,
+  arg.CanonicalDeclaration.End.Column) ==
+ std::tie(Pos.start.line, Pos.start.character, Pos.end.line,
+  Pos.end.character);
+}
+MATCHER_P(DefRange, Pos, "") {
+  return std::tie(arg.Definition.Start.Line,
+  arg.Definition.Start.Column, arg.Definition.End.Line,
+  arg.Definition.End.Column) ==
+ std::tie(Pos.start.line, Pos.start.character, Pos.end.line,
+  Pos.end.character);
 }
 MATCHER_P(Refs, R, "") { return int(arg.References) == R; }
 
@@ -209,7 +216,7 @@
   )");
   runSymbolCollector(Header.code(), /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAreArray({AllOf(
-   QName("Tmpl"), DeclRange(Header.offsetRange()))}));
+   QName("Tmpl"), DeclRange(Header.range()))}));
 }
 
 TEST_F(SymbolCollectorTest, Locations) {
@@ -221,6 +228,9 @@
 
 // Declared in header, defined nowhere.
 extern int $zdecl[[Z]];
+
+void $foodecl[[fo\
+o]]();
   )cpp");
   Annotations Main(R"cpp(
 int $xdef[[X]] = 42;
@@ -234,13 +244,15 @@
   EXPECT_THAT(
   Symbols,
   UnorderedElementsAre(
-  AllOf(QName("X"), DeclRange(Header.offsetRange("xdecl")),
-DefRange(Main.offsetRange("xdef"))),
-  AllOf(QName("Cls"), DeclRange(Header.offsetRange("clsdecl")),
-DefRange(Main.offsetRange("clsdef"))),
-  AllOf(QName("print"), DeclRange(Header.offsetRange("printdecl")),
-DefRange(Main.offsetRange("printdef"))),
-  AllOf(QName("Z"), DeclRange(Header.offsetRange("zdecl");
+  AllOf(QName("X"), DeclRange(Header.range("xdecl")),
+DefRange(Main.range("xdef"))),
+  AllOf(QName("Cls"), DeclRange(Header.range("clsdecl")),
+DefRange(Main.range("clsdef"))),
+  AllOf(QName("print"), DeclRange(Header.range("printdecl")),
+DefRange(Main.range("printdef"))),
+  AllOf(QName("Z"), DeclRange(Header.range("zdecl"))),
+  AllOf(QName("foo"), DeclRange(Header.range("foodecl")))
+  ));
 }
 
 TEST_F(SymbolCollectorTest, References) {
@@ -365,9 +377,9 @@
   EXPECT_THAT(
   Symbols,
   UnorderedElementsAre(
-  AllOf(QName("abc_Test"), DeclRange(Header.offsetRange("expansion")),
+  AllOf(QName("abc_Test"), DeclRange(Header.range("expansion")),
 DeclURI(TestHeaderURI)),
-  AllOf(QName("Test"), DeclRange(Header.offsetRange("spelling")),
+  AllOf(QName("Test"), DeclRange(Header.range("spelling")),
 DeclURI(TestHeaderURI;
 }
 
@@ -382,7 +394,8 @@
  /*ExtraArgs=*/{"-DNAME=name"});
   EXPECT_THAT(Symbols,
   UnorderedElementsAre(AllOf(
-  QName("name"), DeclRange(Header.offsetRange("expansion")),
+  QName("name"),
+  DeclRange(Header.range("expansion")),
   DeclURI(TestHeaderURI;
 }
 
@@ -511,9 +524,13 @@
   Kind:Function
   Lang:Cpp
 CanonicalDeclaration:
-  StartOffset: 0
-  EndOffset:   1
   FileURI:file:///path/foo.h
+  Start:
+Line: 1
+Column: 0
+  End:
+Line: 1
+Column: 1
 CompletionLabel:'Foo1-label'
 CompletionFilterText:'filter'
 CompletionPlainInsertText:'plain'
@@ -531,17 +548,22 @@
   Kind:Function
   Lang:Cpp
 CanonicalDeclaration:
-  StartOffset: 10
-  EndOffset:   12
   FileURI:file:///path/bar.h
+  Start:
+Line: 1
+Column: 0
+  End:
+Line: 1
+Column: 1
 CompletionLabel:'Foo2-label'
 CompletionFilterText:'filter'
 CompletionPlainI

[PATCH] D45045: [DebugInfo] Generate debug information for labels.

2018-04-12 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment.

One question, but otherwise this looks fine.




Comment at: lib/CodeGen/CGStmt.cpp:535
+
+  // Emit debug info for the label only if it's reachable.
+  if (HaveInsertPoint()) {

Shouldn't it be added to the list of labels/variables of the DISubprogram 
regardless?


Repository:
  rC Clang

https://reviews.llvm.org/D45045



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44882: [clangd] Implementation of workspace/symbol request

2018-04-12 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

@malaperle, what's your plan of this patch? Are you going to land it before 
https://reviews.llvm.org/D45513? With the Line&Column info in the index, this 
patch could be simplified.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r329921 - [Test Fix] Fix broken test Index/comment-objc-parameterized-classes.m

2018-04-12 Thread Ben Hamilton via cfe-commits
Author: benhamilton
Date: Thu Apr 12 09:00:05 2018
New Revision: 329921

URL: http://llvm.org/viewvc/llvm-project?rev=329921&view=rev
Log:
[Test Fix] Fix broken test Index/comment-objc-parameterized-classes.m

I broke this test in D45498 when I changed the formatter to remove
spaces before Objective-C lightweight generics.

This fixes the test.

Test Plan:
  % make -j16 check-llvm-tools-llvm-lit && ./bin/llvm-lit -sv 
../llvm/tools/clang/test/Index/comment-objc-parameterized-classes.m

Modified:
cfe/trunk/test/Index/comment-objc-parameterized-classes.m

Modified: cfe/trunk/test/Index/comment-objc-parameterized-classes.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/comment-objc-parameterized-classes.m?rev=329921&r1=329920&r2=329921&view=diff
==
--- cfe/trunk/test/Index/comment-objc-parameterized-classes.m (original)
+++ cfe/trunk/test/Index/comment-objc-parameterized-classes.m Thu Apr 12 
09:00:05 2018
@@ -13,7 +13,7 @@
 @interface NSObject
 @end
 
-// CHECK: @interface A <__covariant T : id, U : NSObject *> 
: NSObject
+// CHECK: @interface A<__covariant T : id, U : NSObject *> 
: NSObject
 /// A
 @interface A<__covariant T : id, U : NSObject *> : NSObject
 @end


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44079: [ASTImporter] Allow testing of import sequences; fix import of typedefs for anonymous decls

2018-04-12 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.

Overall looks good, some minor nits inline.  If those can be resolved 
trivially, I am ok with committing this without another round of reviews.




Comment at: unittests/AST/ASTImporterTest.cpp:91
+static void createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName,
+  const std::string &Code) {
+  return createVirtualFileIfNeeded(

If we already touching this part, maybe better to take the code as a StringRef?



Comment at: unittests/AST/ASTImporterTest.cpp:356
+  ImportAction(StringRef FromFilename, StringRef ToFilename,
+   const std::string &DeclName)
+  : FromFilename(FromFilename), ToFilename(ToFilename),

Maybe using StringRef for DeclName too?



Comment at: unittests/AST/ASTImporterTest.cpp:416
+  AllASTUnits[Filename] = Found->getValue().createASTUnits(Filename);
+  BuiltASTs[Filename] = true;
+}

Do we need this? Can't we just say if a filename is in `AllASTUnits`, it is 
built?


Repository:
  rC Clang

https://reviews.llvm.org/D44079



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43578: -ftime-report switch support in Clang

2018-04-12 Thread Andrew V. Tischenko via Phabricator via cfe-commits
avt77 updated this revision to Diff 142198.
avt77 added a reviewer: davezarzycki.
avt77 added a comment.
Herald added a subscriber: mgorny.

I removed the dependence on TimePassesIsEnabled (as @davezarzycki sugested) and 
fixed the issue with failed test (tnx to @russell.gallop). As result the patch 
was redesigned.


https://reviews.llvm.org/D43578

Files:
  include/clang/Frontend/FrontendAction.h
  include/clang/Frontend/Utils.h
  include/clang/Lex/HeaderSearch.h
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Basic/CMakeLists.txt
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CodeGenAction.cpp
  lib/Frontend/ASTMerge.cpp
  lib/Frontend/CompilerInstance.cpp
  lib/Frontend/FrontendActions.cpp
  lib/Lex/HeaderSearch.cpp
  lib/Lex/PPMacroExpansion.cpp
  lib/Lex/Pragma.cpp
  lib/Parse/ParseTemplate.cpp
  lib/Parse/Parser.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaTemplate.cpp
  test/Frontend/ftime-report-template-decl.cpp

Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -0,0 +1,161 @@
+// RUN: %clang %s -S -o - -ftime-report  2>&1 | FileCheck %s
+// RUN: %clang %s -S -o - -fdelayed-template-parsing -DDELAYED_TEMPLATE_PARSING -ftime-report  2>&1 | FileCheck %s
+
+// Template function declarations
+template 
+void foo();
+template 
+void foo();
+
+// Template function definitions.
+template 
+void foo() {}
+
+// Template class (forward) declarations
+template 
+struct A;
+template 
+struct b;
+template 
+struct C;
+template 
+struct D;
+
+// Forward declarations with default parameters?
+template 
+class X1;
+template 
+class X2;
+
+// Forward declarations w/template template parameters
+template  class T>
+class TTP1;
+template  class>
+class TTP2;
+template  class T>
+class TTP5;
+
+// Forward declarations with non-type params
+template 
+class NTP0;
+template 
+class NTP1;
+template 
+class NTP2;
+template 
+class NTP3;
+template 
+class NTP4;
+template 
+class NTP5;
+template 
+class NTP6;
+template 
+class NTP7;
+
+// Template class declarations
+template 
+struct A {};
+template 
+struct B {};
+
+namespace PR6184 {
+namespace N {
+template 
+void bar(typename T::x);
+}
+
+template 
+void N::bar(typename T::x) {}
+}
+
+// This PR occurred only in template parsing mode.
+namespace PR17637 {
+template 
+struct L {
+  template 
+  struct O {
+template 
+static void Fun(U);
+  };
+};
+
+template 
+template 
+template 
+void L::O::Fun(U) {}
+
+void Instantiate() { L<0>::O::Fun(0); }
+}
+
+namespace explicit_partial_specializations {
+typedef char (&oneT)[1];
+typedef char (&twoT)[2];
+typedef char (&threeT)[3];
+typedef char (&fourT)[4];
+typedef char (&fiveT)[5];
+typedef char (&sixT)[6];
+
+char one[1];
+char two[2];
+char three[3];
+char four[4];
+char five[5];
+char six[6];
+
+template 
+struct bool_ { typedef int type; };
+template <>
+struct bool_ {};
+
+#define XCAT(x, y) x##y
+#define CAT(x, y) XCAT(x, y)
+#define sassert(_b_) bool_<(_b_)>::type CAT(var, __LINE__);
+
+template 
+struct L {
+  template 
+  struct O {
+template 
+static oneT Fun(U);
+  };
+};
+template 
+template 
+template 
+oneT L::O::Fun(U) { return one; }
+
+template <>
+template <>
+template 
+oneT L<0>::O::Fun(U) { return one; }
+
+void Instantiate() {
+  sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+}
+}
+
+template 
+struct Foo {
+  template 
+  using rebind_alloc = _Other;
+};
+template 
+struct _Wrap_alloc {
+  template 
+  using rebind_alloc = typename Foo<_Alloc>::template rebind_alloc<_Other>;
+  template 
+  using rebind = _Wrap_alloc;
+};
+_Wrap_alloc::rebind w;
+
+// CHECK:   = Clang Parser =
+// CHECK:   ---User Time---
+// CHECK:   Parse Top Level Decl
+// CHECK:   Parse Template
+// CHECK:   Parse Function Definition
+// CHECK:   PP Append Macro
+// CHECK:   Scope manipulation
+// CHECK:   PP Find Handler
+// CHECK:   Total
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -22,6 +22,7 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/ParsedTemplate.h"
@@ -32,6 +33,7 @@
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/ManagedStatic.h"
 
 #include 
 using namespace clang;
@@ -187,7 +189,6 @@
   }
 
   QualType ObjectType = ObjectTypePtr.get();
-
   LookupResult R(*this, TName, Name.getLocStart(), LookupOrdinaryName);
   LookupTemplateName(R, S, SS, ObjectType, EnteringContext,
  Memb

[PATCH] D43578: -ftime-report switch support in Clang

2018-04-12 Thread David Zarzycki via Phabricator via cfe-commits
davezarzycki added a comment.

It wasn't my suggestion. @thakis wrote: "We probably should have a separate 
bool in clang and key this off that and make -ftime-report set both."


https://reviews.llvm.org/D43578



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r329923 - [Hexagon] Enable auto-vectorization only when -fvectorize was given

2018-04-12 Thread Krzysztof Parzyszek via cfe-commits
Author: kparzysz
Date: Thu Apr 12 09:25:35 2018
New Revision: 329923

URL: http://llvm.org/viewvc/llvm-project?rev=329923&view=rev
Log:
[Hexagon] Enable auto-vectorization only when -fvectorize was given

Added:
cfe/trunk/test/Driver/hexagon-vectorize.c
Modified:
cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp?rev=329923&r1=329922&r2=329923&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp Thu Apr 12 09:25:35 2018
@@ -520,6 +520,13 @@ void HexagonToolChain::addClangTargetOpt
 CC1Args.push_back("-target-feature");
 CC1Args.push_back("+reserved-r19");
   }
+  if (Arg *A = DriverArgs.getLastArg(options::OPT_fvectorize,
+ options::OPT_fno_vectorize)) {
+if (A->getOption().matches(options::OPT_fvectorize)) {
+  CC1Args.push_back("-mllvm");
+  CC1Args.push_back("-hexagon-autohvx");
+}
+  }
 }
 
 void HexagonToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,

Added: cfe/trunk/test/Driver/hexagon-vectorize.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hexagon-vectorize.c?rev=329923&view=auto
==
--- cfe/trunk/test/Driver/hexagon-vectorize.c (added)
+++ cfe/trunk/test/Driver/hexagon-vectorize.c Thu Apr 12 09:25:35 2018
@@ -0,0 +1,7 @@
+// RUN: %clang -target hexagon -### %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-DEFAULT
+// RUN: %clang -target hexagon -fvectorize -### %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-VECTOR
+// RUN: %clang -target hexagon -fvectorize -fno-vectorize -### %s 2>&1 | 
FileCheck %s --check-prefix=CHECK-NOVECTOR
+
+// CHECK-DEFAULT-NOT: hexagon-autohvx
+// CHECK-VECTOR: "-mllvm" "-hexagon-autohvx"
+// CHECK-NOVECTOR-NOT: hexagon-autohvx


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r329924 - Correctly diagnose when a conversion function is declared with a type qualifier in the declaration specifiers rather than in the conversion type id. Fixes PR30595.

2018-04-12 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Thu Apr 12 09:41:55 2018
New Revision: 329924

URL: http://llvm.org/viewvc/llvm-project?rev=329924&view=rev
Log:
Correctly diagnose when a conversion function is declared with a type qualifier 
in the declaration specifiers rather than in the conversion type id. Fixes 
PR30595.

Modified:
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/SemaCXX/conversion-function.cpp

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=329924&r1=329923&r2=329924&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Apr 12 09:41:55 2018
@@ -8319,7 +8319,8 @@ void Sema::CheckConversionDeclarator(Dec
   QualType ConvType =
   GetTypeFromParser(D.getName().ConversionFunctionId, &ConvTSI);
 
-  if (D.getDeclSpec().hasTypeSpecifier() && !D.isInvalidType()) {
+  const DeclSpec &DS = D.getDeclSpec();
+  if (DS.hasTypeSpecifier() && !D.isInvalidType()) {
 // Conversion functions don't have return types, but the parser will
 // happily parse something like:
 //
@@ -8329,9 +8330,18 @@ void Sema::CheckConversionDeclarator(Dec
 //
 // The return type will be changed later anyway.
 Diag(D.getIdentifierLoc(), diag::err_conv_function_return_type)
-  << SourceRange(D.getDeclSpec().getTypeSpecTypeLoc())
+  << SourceRange(DS.getTypeSpecTypeLoc())
   << SourceRange(D.getIdentifierLoc());
 D.setInvalidType();
+  } else if (DS.getTypeQualifiers() && !D.isInvalidType()) {
+// It's also plausible that the user writes type qualifiers in the wrong
+// place, such as:
+//   struct S { const operator int(); };
+// FIXME: we could provide a fixit to move the qualifiers onto the
+// conversion type.
+Diag(D.getIdentifierLoc(), diag::err_conv_function_with_complex_decl)
+<< SourceRange(D.getIdentifierLoc()) << 0;
+D.setInvalidType();
   }
 
   const FunctionProtoType *Proto = R->getAs();
@@ -8444,12 +8454,12 @@ void Sema::CheckConversionDeclarator(Dec
 R = Context.getFunctionType(ConvType, None, Proto->getExtProtoInfo());
 
   // C++0x explicit conversion operators.
-  if (D.getDeclSpec().isExplicitSpecified())
-Diag(D.getDeclSpec().getExplicitSpecLoc(),
- getLangOpts().CPlusPlus11 ?
-   diag::warn_cxx98_compat_explicit_conversion_functions :
-   diag::ext_explicit_conversion_functions)
-  << SourceRange(D.getDeclSpec().getExplicitSpecLoc());
+  if (DS.isExplicitSpecified())
+Diag(DS.getExplicitSpecLoc(),
+ getLangOpts().CPlusPlus11
+ ? diag::warn_cxx98_compat_explicit_conversion_functions
+ : diag::ext_explicit_conversion_functions)
+<< SourceRange(DS.getExplicitSpecLoc());
 }
 
 /// ActOnConversionDeclarator - Called by ActOnDeclarator to complete

Modified: cfe/trunk/test/SemaCXX/conversion-function.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/conversion-function.cpp?rev=329924&r1=329923&r2=329924&view=diff
==
--- cfe/trunk/test/SemaCXX/conversion-function.cpp (original)
+++ cfe/trunk/test/SemaCXX/conversion-function.cpp Thu Apr 12 09:41:55 2018
@@ -444,3 +444,13 @@ namespace PR18234 {
   bool k1 = e == A::e; // expected-error {{no member named 'e'}}
   bool k2 = e.n == 0;
 }
+
+namespace PR30595 {
+struct S {
+  const operator int(); // expected-error {{cannot specify any part of a 
return type in the declaration of a conversion function; put the complete type 
after 'operator'}}
+  const operator int() const; // expected-error {{cannot specify any part of a 
return type}}
+  volatile const operator int(); // expected-error {{cannot specify any part 
of a return type}}
+
+  operator const int() const;
+};
+}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [clang-tools-extra] r329873 - [clang-tidy] [modernize-use-auto] Get only a length of token, not the token itself

2018-04-12 Thread Alexander Kornienko via cfe-commits
On Thu, Apr 12, 2018 at 7:44 AM Zinovy Nis via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: zinovy.nis
> Date: Wed Apr 11 22:41:24 2018
> New Revision: 329873
>
> URL: http://llvm.org/viewvc/llvm-project?rev=329873&view=rev
> Log:
> [clang-tidy] [modernize-use-auto] Get only a length of token, not the
> token itself
>
>
> Modified:
> clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
>
> Modified: clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp?rev=329873&r1=329872&r2=329873&view=diff
>
> ==
> --- clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
> (original)
> +++ clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp Wed Apr
> 11 22:41:24 2018
> @@ -11,6 +11,7 @@
>  #include "clang/AST/ASTContext.h"
>  #include "clang/ASTMatchers/ASTMatchFinder.h"
>  #include "clang/ASTMatchers/ASTMatchers.h"
> +#include "clang/Lex/Lexer.h"
>  #include "clang/Tooling/FixIt.h"
>
>  using namespace clang;
> @@ -419,8 +420,8 @@ void UseAutoCheck::replaceExpr(
>SourceRange Range(Loc.getSourceRange());
>
>if (MinTypeNameLength != 0 &&
> -  tooling::fixit::getText(Loc.getSourceRange(),
> FirstDecl->getASTContext())
> -  .size() < MinTypeNameLength)
> +  Lexer::MeasureTokenLength(Loc.getLocStart(),
> Context->getSourceManager(),
> +getLangOpts()) < MinTypeNameLength)
>

What about type names consisting of multiple tokens (`unsigned long long`,
`struct VeryLongStructName`, etc.)?


>  return;
>
>auto Diag = diag(Range.getBegin(), Message);
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-12 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:187
+
+  if (isCalledByConstructor(Context))
+return;

xazax.hun wrote:
> Szelethus wrote:
> > whisperity wrote:
> > > I think somewhere there should be a bit of reasoning why exactly these 
> > > cases are ignored by the checker.
> > At this function's declaration I have some comments describing what this 
> > does and why it does it. Did you find it insufficient?
> I am a bit surprised why these errors are not deduplicated by the analyzer. 
> Maybe it would worth some investigation?
Copied from `BugReport`s constructor documentation  for uniqueing:
>The reports that have the same report location, description, bug type, and 
>ranges are uniqued - only one of the equivalent reports will be presented to 
>the user. [...]
For this code snippet:
```
struct A {
   struct B {
  int x, y;
  
  B() {}
   } b;
   int w;

   A() {
  b = B();
   }
};
```
the warning message after a call to `A::A()` would be "3 uninitialized 
fields[...]", and for `B::B()` inside `A`s constructor would be "2 
uninitialized fields[...]", so it wouldn't be filtered out.



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:292
+  if (LocalChain)
+LocalChain->push_back(FR);
+  if (isNonUnionUninit(FR, LocalChain))

xazax.hun wrote:
> Don't you need to pop somewhere?
Nice catch! This isn't covered by tests, and would most probably cause an 
incorrect note message. I'll be sure to fix it.


https://reviews.llvm.org/D45532



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45578: Add a command line option 'fregister_dtor_with_atexit' to register destructor functions annotated with __attribute__((destructor)) using __cxa_atexit or atexit.

2018-04-12 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak created this revision.
ahatanak added reviewers: arphaman, steven_wu, rjmccall.

The command line option makes IRGen register destructor functions
annotated with __attribute__((destructor)) calling __cxa_atexit in a
synthesized constructor function instead of emitting references to
the destructor functions in a special function (__mod_term_funcs on
Darwin).

  

The primary reason for adding this option is that we are planning to
deprecate __mod_term_funcs section on Darwin in the future. This feature
is enabled by default on Darwin and can be disabled using command line
option 'fno_register_dtor_with_atexit'.

  

rdar://problem/33887655


Repository:
  rC Clang

https://reviews.llvm.org/D45578

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/constructor-attribute.c
  test/Driver/cxa-atexit.cpp
  test/Driver/rewrite-legacy-objc.m
  test/Driver/rewrite-objc.m

Index: test/Driver/rewrite-objc.m
===
--- test/Driver/rewrite-objc.m
+++ test/Driver/rewrite-objc.m
@@ -3,4 +3,4 @@
 // TEST0: clang{{.*}}" "-cc1"
 // TEST0: "-rewrite-objc"
 // FIXME: CHECK-NOT is broken somehow, it doesn't work here. Check adjacency instead.
-// TEST0: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fobjc-runtime=macosx" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fexceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
+// TEST0: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fregister-dtor-with-atexit" "-fobjc-runtime=macosx" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fexceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
Index: test/Driver/rewrite-legacy-objc.m
===
--- test/Driver/rewrite-legacy-objc.m
+++ test/Driver/rewrite-legacy-objc.m
@@ -3,11 +3,11 @@
 // TEST0: clang{{.*}}" "-cc1"
 // TEST0: "-rewrite-objc"
 // FIXME: CHECK-NOT is broken somehow, it doesn't work here. Check adjacency instead.
-// TEST0: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fobjc-runtime=macosx-fragile" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fexceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
+// TEST0: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fregister-dtor-with-atexit" "-fobjc-runtime=macosx-fragile" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fexceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
 // TEST0: rewrite-legacy-objc.m"
 // RUN: %clang -no-canonical-prefixes -target i386-apple-macosx10.9.0 -rewrite-legacy-objc %s -o - -### 2>&1 | \
 // RUN:   FileCheck -check-prefix=TEST1 %s
 // RUN: %clang -no-canonical-prefixes -target i386-apple-macosx10.6.0 -rewrite-legacy-objc %s -o - -### 2>&1 | \
 // RUN:   FileCheck -check-prefix=TEST2 %s
-// TEST1: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fobjc-runtime=macosx-fragile" "-fobjc-subscripting-legacy-runtime" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
-// TEST2: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fobjc-runtime=macosx-fragile" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
+// TEST1: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fregister-dtor-with-atexit" "-fobjc-runtime=macosx-fragile" "-fobjc-subscripting-legacy-runtime" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
+// TEST2: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fregister-dtor-with-atexit" "-fobjc-runtime=macosx-fragile" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
Index: test/Driver/cxa-atexit.cpp
===
--- test/Driver/cxa-atexit.cpp
+++ test/Driver/cxa-atexit.cpp
@@ -25,3 +25,18 @@
 // CHECK-MTI: "-fno-use-cxa-atexit"
 // CHECK-MIPS-NOT: "-fno-use-cxa-atexit"
 
+// RUN: %clang -target x86_64-apple-darwin -fregister-dtor-with-atexit -fno-register-dtor-with-atexit -c -### %s 2>&1 | \
+// RUN: FileCheck --check-prefix=WITHOUTATEXIT %s
+// RUN: %clang -target x86_64-apple-darwin -fno-register-dtor-with-atexit -fregister-dtor-with-atexit -c -### %s 2>&1 | \
+// RUN: FileCheck --check-prefix=WITHATEXIT %s
+// RUN: 

[PATCH] D45561: NFC - Indentation fixes in predefined-arch-macros.c

2018-04-12 Thread Craig Topper via Phabricator via cfe-commits
craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D45561



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


  1   2   >