[clang-tools-extra] r247261 - [clang-tidy] Add inconsistent declaration parameter name check

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Thu Sep 10 05:07:11 2015
New Revision: 247261

URL: http://llvm.org/viewvc/llvm-project?rev=247261=rev
Log:
[clang-tidy] Add inconsistent declaration parameter name check

This is first of series of patches, porting code from my project colobot-lint,
as I mentioned recently in cfe-dev mailing list.

This patch adds a new check in readability module:
readability-inconsistent-declaration-parameter-name. I also added appropriate
testcases and documentation.

I chose readability module, as it seems it is the best place for it.

I think I followed the rules of LLVM coding guideline, but I may have missed
something, as I usually use other code formatting style.

http://reviews.llvm.org/D12462

Patch by Piotr Dziwinski!

Added:

clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp

clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst

clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt?rev=247261=247260=247261=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt Thu Sep 10 
05:07:11 2015
@@ -6,6 +6,7 @@ add_clang_library(clangTidyReadabilityMo
   ElseAfterReturnCheck.cpp
   FunctionSizeCheck.cpp
   IdentifierNamingCheck.cpp
+  InconsistentDeclarationParameterNameCheck.cpp
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
   ReadabilityTidyModule.cpp

Added: 
clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp?rev=247261=auto
==
--- 
clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
 (added)
+++ 
clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
 Thu Sep 10 05:07:11 2015
@@ -0,0 +1,336 @@
+//===--- InconsistentDeclarationParameterNameCheck.cpp - clang-tidy---===//
+//
+//   The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "InconsistentDeclarationParameterNameCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+using namespace ast_matchers;
+
+namespace {
+
+AST_MATCHER(FunctionDecl, hasOtherDeclarations) {
+  auto It = Node.redecls_begin();
+  auto EndIt = Node.redecls_end();
+
+  if (It == EndIt)
+return false;
+
+  ++It;
+  return It != EndIt;
+}
+
+struct DifferingParamInfo {
+  DifferingParamInfo(StringRef SourceName, StringRef OtherName,
+ SourceRange OtherNameRange, bool GenerateFixItHint)
+  : SourceName(SourceName), OtherName(OtherName),
+OtherNameRange(OtherNameRange), GenerateFixItHint(GenerateFixItHint) {}
+
+  StringRef SourceName;
+  StringRef OtherName;
+  SourceRange OtherNameRange;
+  bool GenerateFixItHint;
+};
+
+using DifferingParamsContainer = llvm::SmallVector;
+
+struct InconsistentDeclarationInfo {
+  InconsistentDeclarationInfo(SourceLocation DeclarationLocation,
+  DifferingParamsContainer &)
+  : DeclarationLocation(DeclarationLocation),
+DifferingParams(std::move(DifferingParams)) {}
+
+  SourceLocation DeclarationLocation;
+  DifferingParamsContainer DifferingParams;
+};
+
+using InconsistentDeclarationsContainer =
+llvm::SmallVector;
+
+bool checkIfFixItHintIsApplicable(
+const FunctionDecl *ParameterSourceDeclaration,
+const ParmVarDecl *SourceParam, const FunctionDecl *OriginalDeclaration) {
+  // Assumptions with regard to function declarations/definition:
+  //  * If both function declaration and definition are seen, assume that
+  //  definition is most up-to-date, and use it to generate replacements.
+  //  * If only function declarations are seen, there is no easy way to tell
+  //  which is up-to-date and which is not, so don't do anything.

Re: [PATCH] D12734: Another patch for modernize-loop-convert.

[Manuel Klimek via cfe-commits]

klimek added inline comments.


Comment at: clang-tidy/modernize/LoopConvertUtils.cpp:765-766
@@ -764,2 +764,4 @@
   // exactly one usage.
-  addUsage(Usage(nullptr, false, C->getLocation()));
+  // We are using the 'IsArrow' field of Usage to store if we have to add
+  // a '&' to capture the element by reference.
+  addUsage(

I think when the comment is on the Usage member, it's fine to delete it here, 
as it'll only get out of sync.


Comment at: clang-tidy/modernize/LoopConvertUtils.h:203-206
@@ -200,2 +202,6 @@
   const Expr *Expression;
+  // Indicates whether this is an access to a member through the arrow operator
+  // on pointers or iterators. In case of lambda captures, it indicates whether
+  // the capture was done by value.
   bool IsArrow;
+  // Range that covers this usage.

I think this now really needs a different name, or we should put it into an 
enum if we can't find a good bool name that fits both the lambda capture and 
member expression case.


http://reviews.llvm.org/D12734



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

Re: [Diffusion] rL247251: [OPENMP] Outlined function for parallel and other regions with list of…

[NAKAMURA Takumi via cfe-commits]

chapuni added subscribers: cfe-commits, chapuni.
chapuni added a comment.

See also; http://bb.pgr.jp/builders/clang-i686-cygwin-RA-centos6/builds/20524


/cfe/trunk/test/OpenMP/parallel_codegen.cpp:99 Seems it is incompatible to 
32-bit targets, for example, i686-linux, i686-cygwin.

  define internal void @.omp_outlined..1(i32* noalias %.global_tid., i32* 
noalias %.bound_tid., i8*** dereferenceable(4) %argc) #1 personality i8* 
bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {

Users:
  ABataev (Author)

http://reviews.llvm.org/rL247251



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

Re: [Diffusion] rL247251: [OPENMP] Outlined function for parallel and other regions with list of…

[Bataev, Alexey via cfe-commits]

Yes, I know. Fixing it right now. Will be fixed in few minutes, thanks.

Best regards,
Alexey Bataev
=
Software Engineer
Intel Compiler Team

10.09.2015 11:46, NAKAMURA Takumi пишет:

chapuni added subscribers: cfe-commits, chapuni.
chapuni added a comment.

See also; http://bb.pgr.jp/builders/clang-i686-cygwin-RA-centos6/builds/20524


/cfe/trunk/test/OpenMP/parallel_codegen.cpp:99 Seems it is incompatible to 
32-bit targets, for example, i686-linux, i686-cygwin.

   define internal void @.omp_outlined..1(i32* noalias %.global_tid., i32* 
noalias %.bound_tid., i8*** dereferenceable(4) %argc) #1 personality i8* 
bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {

Users:
   ABataev (Author)

http://reviews.llvm.org/rL247251





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

Re: [PATCH] D11797: [LIbClang] Report the named type for ElaboratedType

[Manuel Klimek via cfe-commits]

klimek added a comment.

Ok, so just for my understanding: the nested name specifier in the changed 
tests canonicaltype is *not* the user-written type (thus, what the elaborated 
type would get us), but the full nested name specifier of the canonical type?


http://reviews.llvm.org/D11797



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

Re: [PATCH] D1623: Support __builtin_ms_va_list.

[Charles Davis via cfe-commits]

cdavis5x updated this revision to Diff 34415.
cdavis5x added a comment.

- Rebase onto http://reviews.llvm.org/rL247238.
- Run `clang-format` on this patch.
- Push `va_arg` emission down into the `ABIInfo` hierarchy.

I threaded the `IsMS` parameter from the `VAArgExpr` through the
`EmitVAArg` methods. I'm not entirely happy about this design; I'm open
to suggestions. One thing this does help, though, is supporting this
on other architectures (ARM, maybe?).


http://reviews.llvm.org/D1623

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/Expr.h
  include/clang/Basic/BuiltinsX86.def
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/TargetInfo.h
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/ASTDiagnostic.cpp
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets.cpp
  lib/CodeGen/ABIInfo.h
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaExpr.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/CodeGen/ms_abi.c
  test/PCH/Inputs/va_arg.h
  test/PCH/va_arg.c
  test/PCH/va_arg.cpp
  test/PCH/va_arg.h
  test/Sema/varargs-win64.c
  test/Sema/varargs-x86-32.c
  test/Sema/varargs-x86-64.c
  test/SemaTemplate/instantiate-expr-3.cpp

Index: test/SemaTemplate/instantiate-expr-3.cpp
===
--- test/SemaTemplate/instantiate-expr-3.cpp
+++ test/SemaTemplate/instantiate-expr-3.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
 
 // -
 // Imaginary literals
@@ -108,12 +108,41 @@
 struct VaArg1 {
   void f(int n, ...) {
 VaList va;
-__builtin_va_start(va, n); // expected-error{{int}}
+__builtin_va_start(va, n); // expected-error{{int}} expected-error{{char *}}
 for (int i = 0; i != n; ++i)
   (void)__builtin_va_arg(va, ArgType); // expected-error{{int}}
-__builtin_va_end(va); // expected-error{{int}}
+__builtin_va_end(va);  // expected-error{{int}} expected-error{{char *}}
   }
 };
 
 template struct VaArg1<__builtin_va_list, int>;
+template struct VaArg1<__builtin_ms_va_list, int>; // expected-note{{instantiation}}
 template struct VaArg1; // expected-note{{instantiation}}
+
+template 
+struct VaArg2 {
+  void __attribute__((ms_abi)) f(int n, ...) {
+__builtin_ms_va_list va;
+__builtin_ms_va_start(va, n);
+for (int i = 0; i != n; ++i)
+  (void)__builtin_va_arg(va, ArgType);
+__builtin_ms_va_end(va);
+  }
+};
+
+template struct VaArg2;
+
+template 
+struct VaArg3 {
+  void __attribute__((ms_abi)) f(int n, ...) {
+VaList va;
+__builtin_ms_va_start(va, n); // expected-error{{int}} expected-error{{__va_list_tag}}
+for (int i = 0; i != n; ++i)
+  (void)__builtin_va_arg(va, ArgType); // expected-error{{int}}
+__builtin_ms_va_end(va);   // expected-error{{int}} expected-error{{__va_list_tag}}
+  }
+};
+
+template struct VaArg3<__builtin_ms_va_list, int>;
+template struct VaArg3<__builtin_va_list, int>; // expected-note{{instantiation}}
+template struct VaArg3;   // expected-note{{instantiation}}
Index: test/Sema/varargs-x86-64.c
===
--- test/Sema/varargs-x86-64.c
+++ test/Sema/varargs-x86-64.c
@@ -6,3 +6,77 @@
   (void)__builtin_va_arg(args2, int); // expected-error {{first argument to 'va_arg' is of type 'const __builtin_va_list' and not 'va_list'}}
 }
 
+void f2(int a, ...) {
+  __builtin_ms_va_list ap;
+  __builtin_ms_va_start(ap, a); // expected-error {{'__builtin_ms_va_start' used in System V ABI function}}
+}
+
+void __attribute__((ms_abi)) g1(int a) {
+  __builtin_ms_va_list ap;
+
+  __builtin_ms_va_start(ap, a, a); // expected-error {{too many arguments to function}}
+  __builtin_ms_va_start(ap, a);// expected-error {{'va_start' used in function with fixed args}}
+}
+
+void __attribute__((ms_abi)) g2(int a, int b, ...) {
+  __builtin_ms_va_list ap;
+
+  __builtin_ms_va_start(ap, 10); // expected-warning {{second parameter of 'va_start' not last named argument}}
+  __builtin_ms_va_start(ap, a);  // expected-warning {{second parameter of 'va_start' not last named argument}}
+  __builtin_ms_va_start(ap, b);
+}
+
+void __attribute__((ms_abi)) g3(float a, ...) {
+  __builtin_ms_va_list ap;
+
+  __builtin_ms_va_start(ap, a);
+  __builtin_ms_va_start(ap, (a));
+}
+
+void __attribute__((ms_abi)) g5() {
+  __builtin_ms_va_list ap;
+  __builtin_ms_va_start(ap, ap); // expected-error 

Re: Preventing several replacements on a macro call.

[Ted Kremenek via cfe-commits]

+ Argyrios

Hi Angel,

I believe Argyrios is the original author of the code in question, as this 
looks related to the Objective-C “modernizer” migrator he wrote a while back.  
This code started life on our internal development branch at Apple related to 
an Xcode feature we were doing, and I did the work on pushing it back to open 
source trunk.  Argyrios is the best one to answer your technical question here.

Ted

> On Sep 9, 2015, at 6:05 AM, Angel Garcia  wrote:
> 
> +cfe-commits
> 
> On Tue, Sep 8, 2015 at 6:56 PM, Angel Garcia  > wrote:
> Hi Ted,
> 
> I was working on a clang-tidy check, and today I discovered that it was 
> unable to do several replacements in different arguments of the same macro 
> call. At first, I thought it was a bug, and trying to figure out why this was 
> happening, I found that the replacements were rejected in 
> lib/Edit/EditedSource.cpp:46, where there is a comment that says "Trying to 
> write in a macro argument input that has already been written for another 
> argument of the same macro". This comment means that this changes are 
> rejected on purpose.
> 
> At the commit history, I saw that you had commited 
>  this code (that's why I am asking you). Do 
> you think that there is a way around this? I don't really understand why 
> there is a particular case for the macros here, and understanding it would 
> help me to decide whether I should give up on trying to make this work, or 
> try to find a "better" solution.
> 
> Thanks and sorry for the inconvenience,
> Angel
> 

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

Re: [PATCH] D12734: Another patch for modernize-loop-convert.

[Manuel Klimek via cfe-commits]

klimek added inline comments.


Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:458
@@ -438,2 +457,3 @@
 // variable.
 for (const auto  : Usages) {
+  std::string ReplaceText;

I'd call that 'Usage' here, as it's not an iterator.


Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:460-465
@@ -441,1 +459,8 @@
+  std::string ReplaceText;
+  if (I.Expression) {
+ReplaceText = I.IsArrow ? VarName + "." : VarName;
+  } else {
+// Lambda capture: IsArrow means that the index is captured by value.
+ReplaceText = I.IsArrow ? "&" + VarName : VarName;
+  }
   TUInfo->getReplacedVars().insert(std::make_pair(TheLoop, IndexVar));

We need to document the members of Usage better: It seems unclear why the 
'else' part goes into a lambda capture, or why in the if() path adding a '.' 
will ever help. Examples would help in comments here, I think.


Comment at: clang-tidy/modernize/LoopConvertUtils.cpp:765-766
@@ -764,2 +764,4 @@
   // exactly one usage.
-  addUsage(Usage(nullptr, false, C->getLocation()));
+  // We are using the 'IsArrow' field of Usage to store if we have to add
+  // a '&' to capture the element by reference.
+  addUsage(

This needs to be a comment at the IsArrow field of Usage.


Comment at: test/clang-tidy/modernize-loop-convert-basic.cpp:504
@@ -502,1 +503,3 @@
 
+void constPseudo() {
+  int sum = 0;

why Pseudo?


http://reviews.llvm.org/D12734



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

Re: [PATCH] D10414: Attach function attribute "arm-restrict-it" instead of passing arm-restrict-it as a backend-option

[James Molloy via cfe-commits]

jmolloy added a subscriber: jmolloy.
jmolloy added a comment.

Hi Akira,

I'm sorry to be contrary (and I missed the discussion on Tuesday because I was 
away on vacation) but I think there *is* a usecase for -mno-restrict-it to 
work, and I would hate to see it broken.

Non-restricted IT blocks are indeed deprecated for ARMv8 in the ARMARM. But 
there are circumstances where you may still want to emit them - the biggest 
example being you're compiling for a CPU microarchitecture that you *know* 
doesn't have a performance penalty on non-restricted IT blocks. Restricted IT 
blocks can pessimize code quite badly in some circumstances, and allowing 
people to turn it off for their target if needed is very important, IMO.

Cheers,

James


http://reviews.llvm.org/D10414



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

Re: [PATCH] D12732: Add a deprecation notice to the clang-modernize documentation.

[Manuel Klimek via cfe-commits]

klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

LG


http://reviews.llvm.org/D12732



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

Re: [PATCH] D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check

[Alexander Kornienko via cfe-commits]

alexfh closed this revision.
alexfh added a comment.

Committed revision 247261.

Thank you again for the new awesome check!


http://reviews.llvm.org/D12462



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

Re: [Diffusion] rL247251: [OPENMP] Outlined function for parallel and other regions with list of…

[Alexey Bataev via cfe-commits]

ABataev added a subscriber: ABataev.
ABataev added a comment.

Yes, I know. Fixing it right now. Will be fixed in few minutes, thanks.

Best regards,

Alexey Bataev
=

Software Engineer
Intel Compiler Team

10.09.2015 11:46, NAKAMURA Takumi пишет:

> chapuni added subscribers: cfe-commits, chapuni.

>  chapuni added a comment.

> 

> See also; http://bb.pgr.jp/builders/clang-i686-cygwin-RA-centos6/builds/20524

> 

> /cfe/trunk/test/OpenMP/parallel_codegen.cpp:99 Seems it is incompatible to 
> 32-bit targets, for example, i686-linux, i686-cygwin.

> 

>   define internal void @.omp_outlined..1(i32* noalias %.global_tid., i32* 
> noalias %.bound_tid., i8*** dereferenceable(4) %argc) #1 personality i8* 
> bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {

> 

> Users:

> 

>   ABataev (Author)

> 

> http://reviews.llvm.org/rL247251



Users:
  ABataev (Author)

http://reviews.llvm.org/rL247251



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

r247255 - [OPENMP] Fix test incompatibility with 32-bit platforms

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Thu Sep 10 04:06:59 2015
New Revision: 247255

URL: http://llvm.org/viewvc/llvm-project?rev=247255=rev
Log:
[OPENMP] Fix test incompatibility with 32-bit platforms

Modified:
cfe/trunk/test/OpenMP/parallel_codegen.cpp

Modified: cfe/trunk/test/OpenMP/parallel_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/parallel_codegen.cpp?rev=247255=247254=247255=diff
==
--- cfe/trunk/test/OpenMP/parallel_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/parallel_codegen.cpp Thu Sep 10 04:06:59 2015
@@ -96,7 +96,7 @@ int main (int argc, char **argv) {
 // CHECK-DEBUG-NEXT:  ret i32 0
 // CHECK-DEBUG-NEXT:  }
 
-// CHECK:   define internal {{.*}}void [[OMP_OUTLINED]](i32* noalias 
%.global_tid., i32* noalias %.bound_tid., i8*** dereferenceable(8) %argc)
+// CHECK:   define internal {{.*}}void [[OMP_OUTLINED]](i32* noalias 
%.global_tid., i32* noalias %.bound_tid., i8*** dereferenceable({{4|8}}) %argc)
 // CHECK:   store i8*** %argc, i8 [[ARGC_PTR_ADDR:%.+]],
 // CHECK-NEXT:  [[ARGC_REF:%.+]] = load i8***, i8 [[ARGC_PTR_ADDR]]
 // CHECK-NEXT:  [[ARGC:%.+]] = load i8**, i8*** [[ARGC_REF]]
@@ -106,7 +106,7 @@ int main (int argc, char **argv) {
 // CHECK:   call {{.*}}void @{{.+terminate.*|abort}}(
 // CHECK-NEXT:  unreachable
 // CHECK-NEXT:  }
-// CHECK-DEBUG:   define internal void [[OMP_OUTLINED]](i32* noalias 
%.global_tid., i32* noalias %.bound_tid., i8*** dereferenceable(8) %argc)
+// CHECK-DEBUG:   define internal void [[OMP_OUTLINED]](i32* noalias 
%.global_tid., i32* noalias %.bound_tid., i8*** dereferenceable({{4|8}}) %argc)
 // CHECK-DEBUG:   store i8*** %argc, i8 [[ARGC_PTR_ADDR:%.+]],
 // CHECK-DEBUG:  [[ARGC_REF:%.+]] = load i8***, i8 [[ARGC_PTR_ADDR]]
 // CHECK-DEBUG-NEXT:  [[ARGC:%.+]] = load i8**, i8*** [[ARGC_REF]]


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

Re: [PATCH] D12734: Another patch for modernize-loop-convert.

[Angel Garcia via cfe-commits]

angelgarcia updated this revision to Diff 34422.
angelgarcia marked 4 inline comments as done.
angelgarcia added a comment.

Document Usage's fields and other comments.


http://reviews.llvm.org/D12734

Files:
  clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tidy/modernize/LoopConvertCheck.h
  clang-tidy/modernize/LoopConvertUtils.cpp
  clang-tidy/modernize/LoopConvertUtils.h
  test/clang-tidy/Inputs/modernize-loop-convert/structures.h
  test/clang-tidy/modernize-loop-convert-basic.cpp
  test/clang-tidy/modernize-loop-convert-extra.cpp
  test/clang-tidy/modernize-loop-convert-negative.cpp

Index: test/clang-tidy/modernize-loop-convert-negative.cpp
===
--- test/clang-tidy/modernize-loop-convert-negative.cpp
+++ test/clang-tidy/modernize-loop-convert-negative.cpp
@@ -290,7 +290,6 @@
 dependent v;
 dependent *pv;
 
-transparent cv;
 int Sum = 0;
 
 // Checks for the Index start and end:
@@ -473,3 +472,41 @@
 }
 
 } // namespace NegativeMultiEndCall
+
+namespace NoUsages {
+
+const int N = 6;
+int arr[N] = {1, 2, 3, 4, 5, 6};
+S s;
+dependent v;
+int Count = 0;
+
+void foo();
+
+void f() {
+  for (int I = 0; I < N; ++I) {}
+  for (int I = 0; I < N; ++I)
+printf("Hello world\n");
+  for (int I = 0; I < N; ++I)
+++Count;
+  for (int I = 0; I < N; ++I)
+foo();
+
+  for (S::iterator I = s.begin(), E = s.end(); I != E; ++I) {}
+  for (S::iterator I = s.begin(), E = s.end(); I != E; ++I)
+printf("Hello world\n");
+  for (S::iterator I = s.begin(), E = s.end(); I != E; ++I)
+++Count;
+  for (S::iterator I = s.begin(), E = s.end(); I != E; ++I)
+foo();
+
+  for (int I = 0; I < v.size(); ++I) {}
+  for (int I = 0; I < v.size(); ++I)
+printf("Hello world\n");
+  for (int I = 0; I < v.size(); ++I)
+++Count;
+  for (int I = 0; I < v.size(); ++I)
+foo();
+}
+
+} // namespace NoUsages
Index: test/clang-tidy/modernize-loop-convert-extra.cpp
===
--- test/clang-tidy/modernize-loop-convert-extra.cpp
+++ test/clang-tidy/modernize-loop-convert-extra.cpp
@@ -14,10 +14,9 @@
 int b = arr[i][a];
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & elem : arr) {
+  // CHECK-FIXES: for (auto & elem : arr)
   // CHECK-FIXES-NEXT: int a = 0;
   // CHECK-FIXES-NEXT: int b = elem[a];
-  // CHECK-FIXES-NEXT: }
 
   for (int j = 0; j < M; ++j) {
 int a = 0;
@@ -121,16 +120,16 @@
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto alias : IntArr)
-  // CHECK-FIXES-NEXT: if (alias) {
+  // CHECK-FIXES-NEXT: if (alias)
 
   for (unsigned i = 0; i < N; ++i) {
 while (int alias = IntArr[i]) {
   sideEffect(alias);
 }
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto alias : IntArr)
-  // CHECK-FIXES-NEXT: while (alias) {
+  // CHECK-FIXES-NEXT: while (alias)
 
   for (unsigned i = 0; i < N; ++i) {
 switch (int alias = IntArr[i]) {
@@ -140,32 +139,32 @@
   }
   // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto alias : IntArr)
-  // CHECK-FIXES-NEXT: switch (alias) {
+  // CHECK-FIXES-NEXT: switch (alias)
 
   for (unsigned i = 0; i < N; ++i) {
 for (int alias = IntArr[i]; alias < N; ++alias) {
   sideEffect(alias);
 }
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto alias : IntArr)
-  // CHECK-FIXES-NEXT: for (; alias < N; ++alias) {
+  // CHECK-FIXES-NEXT: for (; alias < N; ++alias)
 
   for (unsigned i = 0; i < N; ++i) {
 for (unsigned j = 0; int alias = IntArr[i]; ++j) {
   sideEffect(alias);
 }
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto alias : IntArr)
-  // CHECK-FIXES-NEXT: for (unsigned j = 0; alias; ++j) {
+  // CHECK-FIXES-NEXT: for (unsigned j = 0; alias; ++j)
 
   struct IntRef { IntRef(const int& i); };
   for (int i = 0; i < N; ++i) {
 IntRef Int(IntArr[i]);
   }
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & elem : IntArr) {
+  // CHECK-FIXES: for (auto & elem : IntArr)
   // CHECK-FIXES-NEXT: IntRef Int(elem);
 }
 
@@ -288,7 +287,7 @@
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & DEFs_it : DEFs)
-  // CHECK-FIXES-NEXT: if (DEFs_it == DEF) {
+  // CHECK-FIXES-NEXT: if (DEFs_it == DEF)
   // CHECK-FIXES-NEXT: printf("I found %d\n", DEFs_it);
 }
 
@@ -315,8 +314,8 @@
   T Vals;
   // Using the name "Val", although it is the name of an existing struct, is
   // safe in this loop since it will only exist within this scope.
-  for (T::iterator it = Vals.begin(), e = Vals.end(); it != e; ++it) {
-  }
+  for 

r247260 - [OPENMP] Propagate alignment from original variables to the private copies.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Thu Sep 10 04:48:30 2015
New Revision: 247260

URL: http://llvm.org/viewvc/llvm-project?rev=247260=rev
Log:
[OPENMP] Propagate alignment from original variables to the private copies.
Currently private copies of captured variables have default alignment. Patch 
makes private variables to have same alignment as original variables.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/for_lastprivate_codegen.cpp
cfe/trunk/test/OpenMP/parallel_copyin_codegen.cpp
cfe/trunk/test/OpenMP/parallel_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/parallel_private_codegen.cpp
cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=247260=247259=247260=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Thu Sep 10 04:48:30 2015
@@ -452,12 +452,17 @@ bool DSAStackTy::isOpenMPLocal(VarDecl *
 
 /// \brief Build a variable declaration for OpenMP loop iteration variable.
 static VarDecl *buildVarDecl(Sema , SourceLocation Loc, QualType Type,
- StringRef Name) {
+ StringRef Name, const AttrVec *Attrs = nullptr) {
   DeclContext *DC = SemaRef.CurContext;
   IdentifierInfo *II = ().get(Name);
   TypeSourceInfo *TInfo = SemaRef.Context.getTrivialTypeSourceInfo(Type, Loc);
   VarDecl *Decl =
   VarDecl::Create(SemaRef.Context, DC, Loc, Loc, II, Type, TInfo, SC_None);
+  if (Attrs) {
+for (specific_attr_iterator I(Attrs->begin()), 
E(Attrs->end());
+ I != E; ++I)
+  Decl->addAttr(*I);
+  }
   Decl->setImplicit();
   return Decl;
 }
@@ -723,9 +728,9 @@ void Sema::EndOpenMPDSABlock(Stmt *CurDi
 // by the address of the new private variable in CodeGen. This new
 // variable is not added to IdResolver, so the code in the OpenMP
 // region uses original variable for proper diagnostics.
-auto *VDPrivate =
-buildVarDecl(*this, DE->getExprLoc(), 
Type.getUnqualifiedType(),
- VD->getName());
+auto *VDPrivate = buildVarDecl(
+*this, DE->getExprLoc(), Type.getUnqualifiedType(),
+VD->getName(), VD->hasAttrs() ? >getAttrs() : nullptr);
 ActOnUninitializedDecl(VDPrivate, /*TypeMayContainAuto=*/false);
 if (VDPrivate->isInvalidDecl())
   continue;
@@ -2732,6 +2737,8 @@ public:
 NewVD->setPreviousDeclInSameBlockScope(
 VD->isPreviousDeclInSameBlockScope());
 VD->getDeclContext()->addHiddenDecl(NewVD);
+if (VD->hasAttrs())
+  NewVD->setAttrs(VD->getAttrs());
 transformedLocalDecl(VD, NewVD);
 return NewVD;
   }
@@ -2911,7 +2918,9 @@ Expr *OpenMPIterationSpaceChecker::Build
 Expr *OpenMPIterationSpaceChecker::BuildPrivateCounterVar() const {
   if (Var && !Var->isInvalidDecl()) {
 auto Type = Var->getType().getNonReferenceType();
-auto *PrivateVar = buildVarDecl(SemaRef, DefaultLoc, Type, Var->getName());
+auto *PrivateVar =
+buildVarDecl(SemaRef, DefaultLoc, Type, Var->getName(),
+ Var->hasAttrs() ? >getAttrs() : nullptr);
 if (PrivateVar->isInvalidDecl())
   return nullptr;
 return buildDeclRefExpr(SemaRef, PrivateVar, Type, DefaultLoc);
@@ -5658,7 +5667,8 @@ OMPClause *Sema::ActOnOpenMPPrivateClaus
 // IdResolver, so the code in the OpenMP region uses original variable for
 // proper diagnostics.
 Type = Type.getUnqualifiedType();
-auto VDPrivate = buildVarDecl(*this, DE->getExprLoc(), Type, 
VD->getName());
+auto VDPrivate = buildVarDecl(*this, DE->getExprLoc(), Type, VD->getName(),
+  VD->hasAttrs() ? >getAttrs() : nullptr);
 ActOnUninitializedDecl(VDPrivate, /*TypeMayContainAuto=*/false);
 if (VDPrivate->isInvalidDecl())
   continue;
@@ -5861,7 +5871,8 @@ OMPClause *Sema::ActOnOpenMPFirstprivate
 }
 
 Type = Type.getUnqualifiedType();
-auto VDPrivate = buildVarDecl(*this, ELoc, Type, VD->getName());
+auto VDPrivate = buildVarDecl(*this, ELoc, Type, VD->getName(),
+  VD->hasAttrs() ? >getAttrs() : nullptr);
 // Generate helper private variable and initialize it with the value of the
 // original variable. The address of the original variable is replaced by
 // the address of the new private variable in the CodeGen. This new 
variable
@@ -6017,11 +6028,13 @@ OMPClause *Sema::ActOnOpenMPLastprivateC
 //  operator for the class type.
 Type = Context.getBaseElementType(Type).getNonReferenceType();
 auto *SrcVD = buildVarDecl(*this, DE->getLocStart(),
-   Type.getUnqualifiedType(), ".lastprivate.src");
+   

Re: PATCH: Expose the 'file' that is associated with a compile database command

[Manuel Klimek via cfe-commits]

@@ -179,11 +185,13 @@ public:
   /// \param Directory The base directory used in the
FixedCompilationDatabase.
   static FixedCompilationDatabase *loadFromCommandLine(int ,
const char *const
*Argv,
-   Twine Directory =
".");
+   Twine Directory =
".",
+   Twine File =
Twine());

A fixed compilation database returns the same command lines for all files,
thus having a file in the function seems strange.

What exactly is the use case? So far, the compilation database has been
designed for 2 use cases:
1. for a file, get the compile commands; the user already knows the file,
no need to get the file
2. get all compile commands; for that, we have the getAllFiles() method, so
a user can get all known files (for compilation databases that support
that), and then get the compile command line.

Thoughts?
/Manuel

On Wed, Sep 9, 2015 at 9:36 PM Argyrios Kyrtzidis 
wrote:

> Hi,
>
> The attached patch exposes the ‘file’ entry in a compilation database
> command, via the CompileCommand structure.
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r247258 - Add a deprecation notice to the clang-modernize documentation.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Thu Sep 10 04:42:01 2015
New Revision: 247258

URL: http://llvm.org/viewvc/llvm-project?rev=247258=rev
Log:
Add a deprecation notice to the clang-modernize documentation.

Summary:
Add a deprecation notice to the clang-modernize documentation. Remove
the reference to the external JIRA tracker.

Reviewers: revane, klimek

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D12732

Modified:
clang-tools-extra/trunk/docs/clang-modernize.rst

Modified: clang-tools-extra/trunk/docs/clang-modernize.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-modernize.rst?rev=247258=247257=247258=diff
==
--- clang-tools-extra/trunk/docs/clang-modernize.rst (original)
+++ clang-tools-extra/trunk/docs/clang-modernize.rst Thu Sep 10 04:42:01 2015
@@ -1,5 +1,14 @@
 .. index:: clang-modernize
 
+.. note::
+
+   **Deprecation**
+
+   As of September 2015 all :program:`clang-modernize` transforms have been
+   ported to :doc:`clang-tidy/index`. :program:`clang-modernize` is deprecated
+   and is going to be removed soon.
+
+
 ==
 Clang C++ Modernizer User's Manual
 ==
@@ -81,34 +90,6 @@ situation you are dealing with.
 .. _Building LLVM with CMake: http://llvm.org/docs/CMake.html
 .. _Clang Tools Documentation: http://clang.llvm.org/docs/ClangTools.html
 
-Getting Involved
-
-
-If you find a bug
-
-.. raw:: html
-
-  
-  https://cpp11-migrate.atlassian.net/s/en_USpfg3b3-1988229788/6095/34/1.4.0-m2/_/download/batch/com.atlassian.jira.collector.plugin.jira-issue-collector-plugin:issuecollector/com.atlassian.jira.collector.plugin.jira-issue-collector-plugin:issuecollector.js?collectorId=50813874";>
-  window.ATL_JQ_PAGE_PROPS =  {
-"triggerFunction": function(showCollectorDialog) {
-  //Requries that jQuery is available! 
-  jQuery("#logbug").click(function(e) {
-e.preventDefault();
-showCollectorDialog();
-  });
-}};
-  
-
-Bugs and feature development of the Modernizer are tracked at
-http://cpp11-migrate.atlassian.net. If you want to get involved the front page
-shows a list of outstanding issues or you can browse around the project to get
-familiar. To take on issues or contribute feature requests and/or bug reports
-you need to sign up for an account from the `log in page`_. An account also
-lets you sign up for notifications on issues or vote for unassigned issues to
-be completed.
-
-.. _log in page: https://cpp11-migrate.atlassian.net/login
 
 .. _transforms:
 


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

Re: [PATCH] D12734: Another patch for modernize-loop-convert.

[Angel Garcia via cfe-commits]

angelgarcia updated this revision to Diff 34428.
angelgarcia added a comment.

Comment the enumerators.

> Do we need default?


I think so. We need to set the cases that do not fall in any of these 
categories to something, and I think that using one of the other three as the 
default kind would be confusing. But maybe there is a better name than just 
UK_Default. Any ideas?


http://reviews.llvm.org/D12734

Files:
  clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tidy/modernize/LoopConvertCheck.h
  clang-tidy/modernize/LoopConvertUtils.cpp
  clang-tidy/modernize/LoopConvertUtils.h
  test/clang-tidy/Inputs/modernize-loop-convert/structures.h
  test/clang-tidy/modernize-loop-convert-basic.cpp
  test/clang-tidy/modernize-loop-convert-extra.cpp
  test/clang-tidy/modernize-loop-convert-negative.cpp

Index: test/clang-tidy/modernize-loop-convert-negative.cpp
===
--- test/clang-tidy/modernize-loop-convert-negative.cpp
+++ test/clang-tidy/modernize-loop-convert-negative.cpp
@@ -290,7 +290,6 @@
 dependent v;
 dependent *pv;
 
-transparent cv;
 int Sum = 0;
 
 // Checks for the Index start and end:
@@ -473,3 +472,41 @@
 }
 
 } // namespace NegativeMultiEndCall
+
+namespace NoUsages {
+
+const int N = 6;
+int arr[N] = {1, 2, 3, 4, 5, 6};
+S s;
+dependent v;
+int Count = 0;
+
+void foo();
+
+void f() {
+  for (int I = 0; I < N; ++I) {}
+  for (int I = 0; I < N; ++I)
+printf("Hello world\n");
+  for (int I = 0; I < N; ++I)
+++Count;
+  for (int I = 0; I < N; ++I)
+foo();
+
+  for (S::iterator I = s.begin(), E = s.end(); I != E; ++I) {}
+  for (S::iterator I = s.begin(), E = s.end(); I != E; ++I)
+printf("Hello world\n");
+  for (S::iterator I = s.begin(), E = s.end(); I != E; ++I)
+++Count;
+  for (S::iterator I = s.begin(), E = s.end(); I != E; ++I)
+foo();
+
+  for (int I = 0; I < v.size(); ++I) {}
+  for (int I = 0; I < v.size(); ++I)
+printf("Hello world\n");
+  for (int I = 0; I < v.size(); ++I)
+++Count;
+  for (int I = 0; I < v.size(); ++I)
+foo();
+}
+
+} // namespace NoUsages
Index: test/clang-tidy/modernize-loop-convert-extra.cpp
===
--- test/clang-tidy/modernize-loop-convert-extra.cpp
+++ test/clang-tidy/modernize-loop-convert-extra.cpp
@@ -14,10 +14,9 @@
 int b = arr[i][a];
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & elem : arr) {
+  // CHECK-FIXES: for (auto & elem : arr)
   // CHECK-FIXES-NEXT: int a = 0;
   // CHECK-FIXES-NEXT: int b = elem[a];
-  // CHECK-FIXES-NEXT: }
 
   for (int j = 0; j < M; ++j) {
 int a = 0;
@@ -121,16 +120,16 @@
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto alias : IntArr)
-  // CHECK-FIXES-NEXT: if (alias) {
+  // CHECK-FIXES-NEXT: if (alias)
 
   for (unsigned i = 0; i < N; ++i) {
 while (int alias = IntArr[i]) {
   sideEffect(alias);
 }
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto alias : IntArr)
-  // CHECK-FIXES-NEXT: while (alias) {
+  // CHECK-FIXES-NEXT: while (alias)
 
   for (unsigned i = 0; i < N; ++i) {
 switch (int alias = IntArr[i]) {
@@ -140,32 +139,32 @@
   }
   // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto alias : IntArr)
-  // CHECK-FIXES-NEXT: switch (alias) {
+  // CHECK-FIXES-NEXT: switch (alias)
 
   for (unsigned i = 0; i < N; ++i) {
 for (int alias = IntArr[i]; alias < N; ++alias) {
   sideEffect(alias);
 }
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto alias : IntArr)
-  // CHECK-FIXES-NEXT: for (; alias < N; ++alias) {
+  // CHECK-FIXES-NEXT: for (; alias < N; ++alias)
 
   for (unsigned i = 0; i < N; ++i) {
 for (unsigned j = 0; int alias = IntArr[i]; ++j) {
   sideEffect(alias);
 }
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto alias : IntArr)
-  // CHECK-FIXES-NEXT: for (unsigned j = 0; alias; ++j) {
+  // CHECK-FIXES-NEXT: for (unsigned j = 0; alias; ++j)
 
   struct IntRef { IntRef(const int& i); };
   for (int i = 0; i < N; ++i) {
 IntRef Int(IntArr[i]);
   }
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & elem : IntArr) {
+  // CHECK-FIXES: for (auto & elem : IntArr)
   // CHECK-FIXES-NEXT: IntRef Int(elem);
 }
 
@@ -288,7 +287,7 @@
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & DEFs_it : DEFs)
-  // CHECK-FIXES-NEXT: if (DEFs_it == DEF) {
+  // CHECK-FIXES-NEXT: if (DEFs_it == DEF)
   // CHECK-FIXES-NEXT: printf("I found %d\n", DEFs_it);
 }
 
@@ -315,8 +314,8 @@
   T Vals;
   // Using the name 

r247277 - AVX-512: Changed nidx parameter in extractf64/32 intrinsic from i8 to i32 according to the Intel Spec

[Igor Breger via cfe-commits]

Author: ibreger
Date: Thu Sep 10 07:55:54 2015
New Revision: 247277

URL: http://llvm.org/viewvc/llvm-project?rev=247277=rev
Log:
AVX-512: Changed nidx parameter in extractf64/32 intrinsic from i8 to i32 
according to the Intel Spec

Differential Revision: http://reviews.llvm.org/D12752

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=247277=247276=247277=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Thu Sep 10 07:55:54 2015
@@ -1032,8 +1032,8 @@ TARGET_BUILTIN(__builtin_ia32_vpermt2var
 TARGET_BUILTIN(__builtin_ia32_vpermt2varpd512_mask, "V8dV8LLiV8dV8dUc", "", 
"avx512f")
 TARGET_BUILTIN(__builtin_ia32_alignq512_mask, "V8LLiV8LLiV8LLiIiV8LLiUc", "", 
"avx512f")
 TARGET_BUILTIN(__builtin_ia32_alignd512_mask, "V16iV16iV16iIiV16iUs", "", 
"avx512f")
-TARGET_BUILTIN(__builtin_ia32_extractf64x4_mask, "V4dV8dIcV4dUc", "", 
"avx512f")
-TARGET_BUILTIN(__builtin_ia32_extractf32x4_mask, "V4fV16fIcV4fUc", "", 
"avx512f")
+TARGET_BUILTIN(__builtin_ia32_extractf64x4_mask, "V4dV8dIiV4dUc", "", 
"avx512f")
+TARGET_BUILTIN(__builtin_ia32_extractf32x4_mask, "V4fV16fIiV4fUc", "", 
"avx512f")
 
 TARGET_BUILTIN(__builtin_ia32_gathersiv8df, "V8dV8dvC*V8iUcIi", "", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_gathersiv16sf, "V16fV16fvC*UsIi", "", "avx512f")


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

r247273 - [OPENMP] Generate threadprivates as TLS variables by default.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Thu Sep 10 07:06:58 2015
New Revision: 247273

URL: http://llvm.org/viewvc/llvm-project?rev=247273=rev
Log:
[OPENMP] Generate threadprivates as TLS variables by default.
If target supports TLS all threadprivates are generated as TLS. If target does 
not support TLS, use runtime calls for proper codegen of threadprivate 
variables.

Added:
cfe/trunk/test/OpenMP/driver.c   (with props)
Modified:
cfe/trunk/lib/Driver/Tools.cpp

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=247273=247272=247273=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Thu Sep 10 07:06:58 2015
@@ -4205,10 +4205,7 @@ void Clang::ConstructJob(Compilation ,
   // given, decide a default based on the target. Otherwise rely on the
   // options and pass the right information to the frontend.
   if (!Args.hasFlag(options::OPT_fopenmp_use_tls,
-options::OPT_fnoopenmp_use_tls,
-getToolChain().getArch() == llvm::Triple::ppc ||
-getToolChain().getArch() == llvm::Triple::ppc64 ||
-getToolChain().getArch() == llvm::Triple::ppc64le))
+options::OPT_fnoopenmp_use_tls, /*Default=*/true))
 CmdArgs.push_back("-fnoopenmp-use-tls");
   break;
 default:

Added: cfe/trunk/test/OpenMP/driver.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/driver.c?rev=247273=auto
==
--- cfe/trunk/test/OpenMP/driver.c (added)
+++ cfe/trunk/test/OpenMP/driver.c Thu Sep 10 07:06:58 2015
@@ -0,0 +1,10 @@
+// Test that by default -fnoopenmp-use-tls is passed to frontend.
+//
+// RUN: %clang %s -### -o %t.o 2>&1 -fopenmp=libomp | FileCheck 
--check-prefix=CHECK-DEFAULT %s
+// CHECK-DEFAULT: -cc1
+// CHECK-DEFAULT-NOT: -fnoopenmp-use-tls
+//
+// RUN: %clang %s -### -o %t.o 2>&1 -fopenmp=libomp -fnoopenmp-use-tls | 
FileCheck --check-prefix=CHECK-NO-TLS %s
+// CHECK-NO-TLS: -cc1
+// CHECK-NO-TLS-SAME: -fnoopenmp-use-tls
+//

Propchange: cfe/trunk/test/OpenMP/driver.c
--
svn:eol-style = native

Propchange: cfe/trunk/test/OpenMP/driver.c
--
svn:keywords = Author Date Id Rev URL

Propchange: cfe/trunk/test/OpenMP/driver.c
--
svn:mime-type = text/plain


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

D12689: [libc++][static linking] std streams are not initialized prior to their use in static object constructors

[Evgeny Astigeevich via cfe-commits]

Ping.
Could anyone review the patch?

Kind regards,
Evgeny Astigeevich

-Original Message-
From: Evgeny Astigeevich [mailto:evgeny.astigeev...@arm.com]
Sent: 08 September 2015 11:00
To: Evgeny Astigeevich
Cc: cfe-commits@lists.llvm.org
Subject: [PATCH] D12689: [libc++][static linking] std streams are not 
initialized prior to their use in static object constructors

eastig created this revision.
eastig added a subscriber: cfe-commits.

When an executable (e.g. for bare metal environment) is statically linked  with 
libc++ the following code might not work as expected:

```
#include 

class Foo {
public:
  Foo(int n) {
std::cout << "Hello World - " << n << std::endl;
  }
};

Foo f(5);

int main() {
  return 0;
}
```
The program hangs or crashes because 'std::cout' is not initialized before 'Foo 
f(5)'.

The standard says:

> 27.3 Standard iostream objects
> Header  synopsis
> (2) ... The objects are constructed, and the associations are
> established at some time prior to or during first time an object of
> class basic_ios::Init is constructed, and in any case before 
> the body of main begins execution [264]. The objects are not destroyed during 
> program execution [265].
>

And footnote 265 says:

> Constructors and destructors for static objects can access these
> objects to read input from stdin or write output to stdout or stderr.

The similar issue was raised more than year ago. A patch was proposed but not 
committed. See discussion of it here:

[[ 
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130304/075363.html 
| initial patch ]] [[ 
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130325/077130.html 
| review, suggestion for #ifdef ]]

The proposed patch caused initialization/deinitialization of the std streams as 
many times as the static Init objects were created.
The current patch fixes this issue. A number of uses is counted by means of 
__shared_count. It is used instead of a 'int' variable because it is 
thread-safe. The standard allows multi-threaded initialization of static 
objects from different compilation modules.



http://reviews.llvm.org/D12689

Files:
  include/ios
  src/iostream.cpp


-- IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium.  Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered 
in England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, 
Registered in England & Wales, Company No:  2548782
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12725: [analyzer] A fix for substraction of an integer from a pointer.

[Artem Dergachev via cfe-commits]

NoQ added a comment.

Thanks, fixed :)


http://reviews.llvm.org/D12725



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

Re: [PATCH] D12725: [analyzer] A fix for substraction of an integer from a pointer.

[Artem Dergachev via cfe-commits]

NoQ updated this revision to Diff 34423.
NoQ added a comment.

Make the tests easier to understand.


http://reviews.llvm.org/D12725

Files:
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/ptr-arith.c

Index: test/Analysis/ptr-arith.c
===
--- test/Analysis/ptr-arith.c
+++ test/Analysis/ptr-arith.c
@@ -296,3 +296,20 @@
   clang_analyzer_eval([i].x < [i].y);// expected-warning{{TRUE}}
 }
 
+void negativeIndex(char *str) {
+  *(str + 1) = 'a';
+  clang_analyzer_eval(*(str + 1) == 'a'); // expected-warning{{TRUE}}
+  clang_analyzer_eval(*(str - 1) == 'a'); // expected-warning{{UNKNOWN}}
+
+  char *ptr1 = str - 1;
+  clang_analyzer_eval(*ptr1 == 'a'); // expected-warning{{UNKNOWN}}
+
+  char *ptr2 = str;
+  ptr2 -= 1;
+  clang_analyzer_eval(*ptr2 == 'a'); // expected-warning{{UNKNOWN}}
+
+  char *ptr3 = str;
+  --ptr3;
+  clang_analyzer_eval(*ptr3 == 'a'); // expected-warning{{UNKNOWN}}
+}
+
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -911,8 +911,9 @@
   elementType = elemReg->getElementType();
 }
 else if (isa(region)) {
+  assert(op == BO_Add || op == BO_Sub);
+  index = (op == BO_Add) ? rhs : evalMinus(rhs);
   superR = region;
-  index = rhs;
   if (resultTy->isAnyPointerType())
 elementType = resultTy->getPointeeType();
 }


Index: test/Analysis/ptr-arith.c
===
--- test/Analysis/ptr-arith.c
+++ test/Analysis/ptr-arith.c
@@ -296,3 +296,20 @@
   clang_analyzer_eval([i].x < [i].y);// expected-warning{{TRUE}}
 }
 
+void negativeIndex(char *str) {
+  *(str + 1) = 'a';
+  clang_analyzer_eval(*(str + 1) == 'a'); // expected-warning{{TRUE}}
+  clang_analyzer_eval(*(str - 1) == 'a'); // expected-warning{{UNKNOWN}}
+
+  char *ptr1 = str - 1;
+  clang_analyzer_eval(*ptr1 == 'a'); // expected-warning{{UNKNOWN}}
+
+  char *ptr2 = str;
+  ptr2 -= 1;
+  clang_analyzer_eval(*ptr2 == 'a'); // expected-warning{{UNKNOWN}}
+
+  char *ptr3 = str;
+  --ptr3;
+  clang_analyzer_eval(*ptr3 == 'a'); // expected-warning{{UNKNOWN}}
+}
+
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -911,8 +911,9 @@
   elementType = elemReg->getElementType();
 }
 else if (isa(region)) {
+  assert(op == BO_Add || op == BO_Sub);
+  index = (op == BO_Add) ? rhs : evalMinus(rhs);
   superR = region;
-  index = rhs;
   if (resultTy->isAnyPointerType())
 elementType = resultTy->getPointeeType();
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12689: [libc++][static linking] std streams are not initialized prior to their use in static object constructors

[Reid Kleckner via cfe-commits]

rnk added a subscriber: rnk.
rnk added a comment.

I think a better approach would be to use `__attribute__((init_priority(101)))` 
on Linux and `#pragma init_seg(lib)` on Windows to ensure that libc++'s 
iostream initializer runs earlier.


http://reviews.llvm.org/D12689



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

[PATCH] D12759: [clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stlcontainers.

[Alexander Kornienko via cfe-commits]

alexfh created this revision.
alexfh added a reviewer: djasper.
alexfh added a subscriber: cfe-commits.

sizeof(some_std_string) is likely to be an error. This check finds this
pattern and suggests using .size() instead.

http://reviews.llvm.org/D12759

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/SizeofContainerCheck.cpp
  clang-tidy/misc/SizeofContainerCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-sizeof-container.rst
  test/clang-tidy/misc-sizeof-container.cpp

Index: test/clang-tidy/misc-sizeof-container.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-sizeof-container.cpp
@@ -0,0 +1,42 @@
+// RUN: %python %S/check_clang_tidy.py %s misc-sizeof-container %t
+
+namespace std {
+template 
+class basic_string {};
+
+template 
+basic_string operator+(const basic_string &, const T *);
+
+typedef basic_string string;
+
+template 
+class vector {};
+}
+
+class string {};
+
+void f() {
+  string s1;
+  std::string s2;
+  std::vector v;
+
+  int a = 42 + sizeof(s1);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: sizeof() doesn't return the size of the container. Did you mean .size()? [misc-sizeof-container]
+// CHECK-FIXES: int a = 42 + s1.size();
+  a = 123 * sizeof(s2);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: sizeof() doesn't return the size
+// CHECK-FIXES: a = 123 * s2.size();
+  a = 45 + sizeof(s2 + "asdf");
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: sizeof() doesn't return the size
+// CHECK-FIXES: a = 45 + (s2 + "asdf").size();
+  a = sizeof(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: sizeof() doesn't return the size
+// CHECK-FIXES: a = v.size();
+
+  a = sizeof(a);
+  a = sizeof(int);
+  a = sizeof(std::string);
+  a = sizeof(std::vector);
+
+  (void)a;
+}
Index: docs/clang-tidy/checks/misc-sizeof-container.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-sizeof-container.rst
@@ -0,0 +1,17 @@
+misc-sizeof-container
+=
+
+The check finds usages of ``sizeof`` on expressions of STL container types. Most
+likely the user wanted to use ``.size()`` instead.
+
+Currently only ``std::string`` and ``std::vector`` are supported.
+
+Examples:
+
+.. code:: c++
+
+  std::string s;
+  int a = 47 + sizeof(s); // warning: sizeof() doesn't return the size of the container. Did you mean .size()?
+  // The suggested fix is: int a = 47 + s.size();
+
+  int b = sizeof(std::string); // no warning, probably intended.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -31,6 +31,7 @@
misc-macro-repeated-side-effects
misc-move-constructor-init
misc-noexcept-move-constructor
+   misc-sizeof-container
misc-static-assert
misc-swapped-arguments
misc-undelegated-constructor
@@ -54,4 +55,4 @@
readability-named-parameter
readability-redundant-smartptr-get
readability-redundant-string-cstr
-   readability-simplify-boolean-expr
\ No newline at end of file
+   readability-simplify-boolean-expr
Index: clang-tidy/misc/SizeofContainerCheck.h
===
--- /dev/null
+++ clang-tidy/misc/SizeofContainerCheck.h
@@ -0,0 +1,35 @@
+//===--- SizeofContainerCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SIZEOF_CONTAINER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SIZEOF_CONTAINER_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// Find usages of sizeof on expressions of STL container types. Most likely the
+/// user wanted to use `.size()` instead.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-sizeof-container.html
+class SizeofContainerCheck : public ClangTidyCheck {
+public:
+  SizeofContainerCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SIZEOF_CONTAINER_H
+
Index: clang-tidy/misc/SizeofContainerCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/SizeofContainerCheck.cpp
@@ -0,0 +1,76 @@
+//===--- SizeofContainerCheck.cpp - clang-tidy-===//
+//
+// The LLVM Compiler 

Re: [PATCH] D12759: [clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stlcontainers.

[Alexander Kornienko via cfe-commits]

alexfh updated this revision to Diff 34441.
alexfh added a comment.

Ignore template instantiations.


http://reviews.llvm.org/D12759

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/SizeofContainerCheck.cpp
  clang-tidy/misc/SizeofContainerCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-sizeof-container.rst
  test/clang-tidy/misc-sizeof-container.cpp

Index: test/clang-tidy/misc-sizeof-container.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-sizeof-container.cpp
@@ -0,0 +1,51 @@
+// RUN: %python %S/check_clang_tidy.py %s misc-sizeof-container %t
+
+namespace std {
+template 
+class basic_string {};
+
+template 
+basic_string operator+(const basic_string &, const T *);
+
+typedef basic_string string;
+
+template 
+class vector {};
+}
+
+class string {};
+
+template
+void g(T t) {
+  (void)sizeof(t);
+}
+
+void f() {
+  string s1;
+  std::string s2;
+  std::vector v;
+
+  int a = 42 + sizeof(s1);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: sizeof() doesn't return the size of the container. Did you mean .size()? [misc-sizeof-container]
+// CHECK-FIXES: int a = 42 + s1.size();
+  a = 123 * sizeof(s2);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: sizeof() doesn't return the size
+// CHECK-FIXES: a = 123 * s2.size();
+  a = 45 + sizeof(s2 + "asdf");
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: sizeof() doesn't return the size
+// CHECK-FIXES: a = 45 + (s2 + "asdf").size();
+  a = sizeof(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: sizeof() doesn't return the size
+// CHECK-FIXES: a = v.size();
+
+  a = sizeof(a);
+  a = sizeof(int);
+  a = sizeof(std::string);
+  a = sizeof(std::vector);
+
+  g(s1);
+  g(s2);
+  g(v);
+
+  (void)a;
+}
Index: docs/clang-tidy/checks/misc-sizeof-container.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-sizeof-container.rst
@@ -0,0 +1,17 @@
+misc-sizeof-container
+=
+
+The check finds usages of ``sizeof`` on expressions of STL container types. Most
+likely the user wanted to use ``.size()`` instead.
+
+Currently only ``std::string`` and ``std::vector`` are supported.
+
+Examples:
+
+.. code:: c++
+
+  std::string s;
+  int a = 47 + sizeof(s); // warning: sizeof() doesn't return the size of the container. Did you mean .size()?
+  // The suggested fix is: int a = 47 + s.size();
+
+  int b = sizeof(std::string); // no warning, probably intended.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -31,6 +31,7 @@
misc-macro-repeated-side-effects
misc-move-constructor-init
misc-noexcept-move-constructor
+   misc-sizeof-container
misc-static-assert
misc-swapped-arguments
misc-undelegated-constructor
@@ -54,4 +55,4 @@
readability-named-parameter
readability-redundant-smartptr-get
readability-redundant-string-cstr
-   readability-simplify-boolean-expr
\ No newline at end of file
+   readability-simplify-boolean-expr
Index: clang-tidy/misc/SizeofContainerCheck.h
===
--- /dev/null
+++ clang-tidy/misc/SizeofContainerCheck.h
@@ -0,0 +1,35 @@
+//===--- SizeofContainerCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SIZEOF_CONTAINER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SIZEOF_CONTAINER_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// Find usages of sizeof on expressions of STL container types. Most likely the
+/// user wanted to use `.size()` instead.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-sizeof-container.html
+class SizeofContainerCheck : public ClangTidyCheck {
+public:
+  SizeofContainerCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SIZEOF_CONTAINER_H
+
Index: clang-tidy/misc/SizeofContainerCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/SizeofContainerCheck.cpp
@@ -0,0 +1,77 @@
+//===--- SizeofContainerCheck.cpp - clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is 

Re: [PATCH] D12759: [clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stlcontainers.

[Aaron Ballman via cfe-commits]

aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a comment.

Great idea for a checker! Some comments below.



Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:36
@@ +35,3 @@
+  Finder->addMatcher(
+  expr(sizeOfExpr(
+   has(expr(hasType(hasCanonicalType(hasDeclaration(recordDecl(

Do you need the expr() matcher?


Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:38
@@ +37,3 @@
+   has(expr(hasType(hasCanonicalType(hasDeclaration(recordDecl(
+   matchesName("std::(basic_string|vector)|::string")
+  .bind("sizeof"),

I think that this should be checking for more than just basic_string and 
vector. Why not other containers? For instance, anything that exposes a member 
function with a size() function seems like it would be reasonable.


Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:48
@@ +47,3 @@
+  SourceLocation SizeOfLoc = SizeOf->getLocStart();
+  auto Diag = diag(SizeOfLoc, "sizeof() doesn't return the size of the "
+  "container. Did you mean .size()?");

I think the period should be replaced with a semicolon (it's not a sentence, so 
the period is also wrong). e.g. (with some wording tweaks),

"sizeof() does not return the size of the container; did you mean to call 
size()?"

Also, is this actually emitting the diagnostic? If so, a comment would be good 
explaining why the early return for macros is located where it is, so no one 
does a drive-by "fix."


http://reviews.llvm.org/D12759



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

Re: [PATCH] D12759: [clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stlcontainers.

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

I noticed a few more things, but mostly nitpicky at this point.



Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:22
@@ +21,3 @@
+bool needsParens(const Expr *E) {
+  E = E->IgnoreImpCasts();
+  if (isa(E) || isa(E))

Should we also ignore parens for when people do crazy things, like 
sizeofsome_string+other_string?


Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:25
@@ +24,3 @@
+return true;
+  if (const auto *Op = dyn_cast(E)) {
+return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call &&

I missed this earlier, but what about other 2-arg overloaded operators, like 
placement operator new & operator delete, or operator delete with size (and 
array forms)?


Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:38
@@ +37,3 @@
+   sizeOfExpr(has(expr(hasType(hasCanonicalType(hasDeclaration(
+   recordDecl(matchesName("^(::std::|::string)"),
+  hasMethod(methodDecl(hasName("size"), isPublic(),

Why is "|::string" needed?


http://reviews.llvm.org/D12759



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

Re: [PATCH] D10414: Attach function attribute "arm-restrict-it" instead of passing arm-restrict-it as a backend-option

[Jim Grosbach via cfe-commits]

grosbach added a comment.

In http://reviews.llvm.org/D10414#243056, @jmolloy wrote:

> Non-restricted IT blocks are indeed deprecated for ARMv8 in the ARMARM. But 
> there are circumstances where you may still want to emit them - the biggest 
> example being you're compiling for a CPU microarchitecture that you *know* 
> doesn't have a performance penalty on non-restricted IT blocks. Restricted IT 
> blocks can pessimize code quite badly in some circumstances, and allowing 
> people to turn it off for their target if needed is very important, IMO.


Bother, email response isn't showing up in Phab. Reposting here so there's a 
record. Sorry for the duplication on-list.

If such microarchitectures exist, shouldn’t they be represented properly as a 
CPU in the backend and get the right setting by default?


http://reviews.llvm.org/D10414



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

Re: [PATCH] D12759: [clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stlcontainers.

[Alexander Kornienko via cfe-commits]

alexfh updated this revision to Diff 34446.
alexfh added a comment.

Match a broader set of containers. Updated diagnostic message. Added tests.


http://reviews.llvm.org/D12759

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/SizeofContainerCheck.cpp
  clang-tidy/misc/SizeofContainerCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-sizeof-container.rst
  test/clang-tidy/misc-sizeof-container.cpp

Index: test/clang-tidy/misc-sizeof-container.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-sizeof-container.cpp
@@ -0,0 +1,78 @@
+// RUN: %python %S/check_clang_tidy.py %s misc-sizeof-container %t
+
+namespace std {
+
+typedef unsigned int size_t;
+
+template 
+struct basic_string {
+  size_t size() const;
+};
+
+template 
+basic_string operator+(const basic_string &, const T *);
+
+typedef basic_string string;
+
+template 
+struct vector {
+  size_t size() const;
+};
+
+class fake_container1 {
+  size_t size() const; // non-public
+};
+
+struct fake_container2 {
+  size_t size(); // non-const
+};
+
+}
+
+struct string {
+  std::size_t size() const;
+};
+
+template
+void g(T t) {
+  (void)sizeof(t);
+}
+
+void f() {
+  string s1;
+  std::string s2;
+  std::vector v;
+
+  int a = 42 + sizeof(s1);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: sizeof() doesn't return the size of the container; did you mean .size()? [misc-sizeof-container]
+// CHECK-FIXES: int a = 42 + s1.size();
+  a = 123 * sizeof(s2);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: sizeof() doesn't return the size
+// CHECK-FIXES: a = 123 * s2.size();
+  a = 45 + sizeof(s2 + "asdf");
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: sizeof() doesn't return the size
+// CHECK-FIXES: a = 45 + (s2 + "asdf").size();
+  a = sizeof(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: sizeof() doesn't return the size
+// CHECK-FIXES: a = v.size();
+  a = sizeof(std::vector{});
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: sizeof() doesn't return the size
+// CHECK-FIXES: a = std::vector{}.size();
+
+  a = sizeof(a);
+  a = sizeof(int);
+  a = sizeof(std::string);
+  a = sizeof(std::vector);
+
+  g(s1);
+  g(s2);
+  g(v);
+
+  std::fake_container1 f1;
+  std::fake_container2 f2;
+
+  a = sizeof(f1);
+  a = sizeof(f2);
+
+  (void)a;
+}
Index: docs/clang-tidy/checks/misc-sizeof-container.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-sizeof-container.rst
@@ -0,0 +1,17 @@
+misc-sizeof-container
+=
+
+The check finds usages of ``sizeof`` on expressions of STL container types. Most
+likely the user wanted to use ``.size()`` instead.
+
+Currently only ``std::string`` and ``std::vector`` are supported.
+
+Examples:
+
+.. code:: c++
+
+  std::string s;
+  int a = 47 + sizeof(s); // warning: sizeof() doesn't return the size of the container. Did you mean .size()?
+  // The suggested fix is: int a = 47 + s.size();
+
+  int b = sizeof(std::string); // no warning, probably intended.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -31,6 +31,7 @@
misc-macro-repeated-side-effects
misc-move-constructor-init
misc-noexcept-move-constructor
+   misc-sizeof-container
misc-static-assert
misc-swapped-arguments
misc-undelegated-constructor
@@ -54,4 +55,4 @@
readability-named-parameter
readability-redundant-smartptr-get
readability-redundant-string-cstr
-   readability-simplify-boolean-expr
\ No newline at end of file
+   readability-simplify-boolean-expr
Index: clang-tidy/misc/SizeofContainerCheck.h
===
--- /dev/null
+++ clang-tidy/misc/SizeofContainerCheck.h
@@ -0,0 +1,35 @@
+//===--- SizeofContainerCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SIZEOF_CONTAINER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SIZEOF_CONTAINER_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// Find usages of sizeof on expressions of STL container types. Most likely the
+/// user wanted to use `.size()` instead.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-sizeof-container.html
+class SizeofContainerCheck : public ClangTidyCheck {
+public:
+  SizeofContainerCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void 

Re: [PATCH] D12759: [clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stlcontainers.

[Alexander Kornienko via cfe-commits]

alexfh marked 3 inline comments as done.


Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:37
@@ +36,3 @@
+  expr(unless(isInTemplateInstantiation()),
+   sizeOfExpr(
+   has(expr(hasType(hasCanonicalType(hasDeclaration(recordDecl(

Yes, I do. `sizeOfExpr().bind("...")` doesn't compile for some reason.

```
tools/clang/tools/extra/clang-tidy/misc/SizeofContainerCheck.cpp:24:35: error: 
no member named 'bind' in 'clang::ast_matchers::internal::Matcher'
  .bind("expr"))).bind("sizeof"),
  ~~~ ^
```


Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:39
@@ +38,3 @@
+   has(expr(hasType(hasCanonicalType(hasDeclaration(recordDecl(
+   matchesName("std::(basic_string|vector)|::string")
+  .bind("sizeof"),

I'd like to limit this to the STL containers first. So "any recordDecl named 
std::.* with a .size() method" might work.


Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:49
@@ +48,3 @@
+  SourceLocation SizeOfLoc = SizeOf->getLocStart();
+  auto Diag = diag(SizeOfLoc, "sizeof() doesn't return the size of the "
+  "container. Did you mean .size()?");

1. Done.
2. It's actually emitting the diagnostic. And I thought that the comment on 
line 52 below explains what happens in enough detail (`// Don't generate fixes 
for macros.`). If something is still unclear, can you explain what exactly?


http://reviews.llvm.org/D12759



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

r247286 - Properly close documentation /code blocks with /endcode.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Thu Sep 10 10:13:22 2015
New Revision: 247286

URL: http://llvm.org/viewvc/llvm-project?rev=247286=rev
Log:
Properly close documentation /code blocks with /endcode.

Modified:
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=247286=247285=247286=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Thu Sep 10 10:13:22 2015
@@ -1870,7 +1870,7 @@ AST_MATCHER_P_OVERLOAD(CXXRecordDecl, is
 /// \code
 ///   class A { void func(); };
 ///   class B { void member(); };
-/// \code
+/// \endcode
 ///
 /// \c recordDecl(hasMethod(hasName("func"))) matches the declaration of \c A
 /// but not \c B.
@@ -2319,7 +2319,7 @@ AST_MATCHER_P(QualType, references, inte
 ///   typedef int _ref;
 ///   int a;
 ///   int_ref b = a;
-/// \code
+/// \endcode
 ///
 /// \c varDecl(hasType(qualType(referenceType()) will not match the
 /// declaration of b but \c
@@ -3090,6 +3090,7 @@ AST_MATCHER_P(UnaryOperator, hasUnaryOpe
 /// \code
 /// class URL { URL(string); };
 /// URL url = "a string";
+/// \endcode
 AST_MATCHER_P(CastExpr, hasSourceExpression,
   internal::Matcher, InnerMatcher) {
   const Expr* const SubExpression = Node.getSubExpr();
@@ -3854,7 +3855,7 @@ AST_TYPE_MATCHER(TypedefType, typedefTyp
 ///
 ///   template class C;  // A
 ///   C var;// B
-/// \code
+/// \endcode
 ///
 /// \c templateSpecializationType() matches the type of the explicit
 /// instantiation in \c A and the type of the variable declaration in \c B.
@@ -3879,7 +3880,7 @@ AST_TYPE_MATCHER(UnaryTransformType, una
 ///
 ///   C c;
 ///   S s;
-/// \code
+/// \endcode
 ///
 /// \c recordType() matches the type of the variable declarations of both \c c
 /// and \c s.
@@ -3899,7 +3900,7 @@ AST_TYPE_MATCHER(RecordType, recordType)
 ///
 ///   class C c;
 ///   N::M::D d;
-/// \code
+/// \endcode
 ///
 /// \c elaboratedType() matches the type of the variable declarations of both
 /// \c c and \c d.
@@ -3916,7 +3917,7 @@ AST_TYPE_MATCHER(ElaboratedType, elabora
 /// }
 ///   }
 ///   N::M::D d;
-/// \code
+/// \endcode
 ///
 /// \c elaboratedType(hasQualifier(hasPrefix(specifiesNamespace(hasName("N"
 /// matches the type of the variable declaration of \c d.
@@ -3938,7 +3939,7 @@ AST_MATCHER_P(ElaboratedType, hasQualifi
 /// }
 ///   }
 ///   N::M::D d;
-/// \code
+/// \endcode
 ///
 /// \c elaboratedType(namesType(recordType(
 /// hasDeclaration(namedDecl(hasName("D")) matches the type of the variable
@@ -3957,7 +3958,7 @@ AST_MATCHER_P(ElaboratedType, namesType,
 ///   void F(T t) {
 /// int i = 1 + t;
 ///   }
-/// \code
+/// \endcode
 ///
 /// \c substTemplateTypeParmType() matches the type of 't' but not '1'
 AST_TYPE_MATCHER(SubstTemplateTypeParmType, substTemplateTypeParmType);
@@ -3972,7 +3973,7 @@ AST_TYPE_MATCHER(SubstTemplateTypeParmTy
 ///   class D {};
 /// }
 ///   }
-/// \code
+/// \endcode
 ///
 /// \c recordDecl(hasDeclContext(namedDecl(hasName("M" matches the
 /// declaration of \c class \c D.


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

Re: [PATCH] D12759: [clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stlcontainers.

[Aaron Ballman via cfe-commits]

On Thu, Sep 10, 2015 at 10:54 AM, Alexander Kornienko  wrote:
> alexfh marked 3 inline comments as done.
>
> 
> Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:37
> @@ +36,3 @@
> +  expr(unless(isInTemplateInstantiation()),
> +   sizeOfExpr(
> +   has(expr(hasType(hasCanonicalType(hasDeclaration(recordDecl(
> 
> Yes, I do. `sizeOfExpr().bind("...")` doesn't compile for some reason.
>
> ```
> tools/clang/tools/extra/clang-tidy/misc/SizeofContainerCheck.cpp:24:35: 
> error: no member named 'bind' in 
> 'clang::ast_matchers::internal::Matcher'
>   .bind("expr"))).bind("sizeof"),
>   ~~~ ^
> ```

That's kind of strange, but okay!

> 
> Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:39
> @@ +38,3 @@
> +   has(expr(hasType(hasCanonicalType(hasDeclaration(recordDecl(
> +   matchesName("std::(basic_string|vector)|::string")
> +  .bind("sizeof"),
> 
> I'd like to limit this to the STL containers first. So "any recordDecl named 
> std::.* with a .size() method" might work.

Sounds reasonable to me.

> 
> Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:49
> @@ +48,3 @@
> +  SourceLocation SizeOfLoc = SizeOf->getLocStart();
> +  auto Diag = diag(SizeOfLoc, "sizeof() doesn't return the size of the "
> +  "container. Did you mean .size()?");
> 
> 1. Done.
> 2. It's actually emitting the diagnostic. And I thought that the comment on 
> line 52 below explains what happens in enough detail (`// Don't generate 
> fixes for macros.`). If something is still unclear, can you explain what 
> exactly?

It's not intuitive that creating the local variable actually emits the
diagnostic, so it seems like you would be able to hoist the early
return up above the local declaration when in fact that would remove
the diagnostic entirely. The comment about generating fixes suggests
an additional diagnostic, at least to me.

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

Re: r247233 - EmitRecord* API change: accepts ArrayRef instead of a SmallVector (NFC)

[David Blaikie via cfe-commits]

On Thu, Sep 10, 2015 at 9:00 AM, Mehdi Amini  wrote:

>
> On Sep 9, 2015, at 7:06 PM, David Blaikie  wrote:
>
>
>
> On Wed, Sep 9, 2015 at 6:46 PM, Mehdi Amini via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: mehdi_amini
>> Date: Wed Sep  9 20:46:39 2015
>> New Revision: 247233
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=247233=rev
>> Log:
>> EmitRecord* API change: accepts ArrayRef instead of a SmallVector (NFC)
>>
>> This reapply a variant commit r247179 after post-commit review from
>> D.Blaikie.
>> Hopefully I got it right this time: lifetime of initializer list ends
>> as with any expression, which make invalid the pattern:
>>
>> ArrayRef Arr = { 1, 2, 3, 4};
>>
>> Just like StringRef, ArrayRef shouldn't be used to initialize local
>> variable but only as function argument.
>>
>
> Looks pretty reasonable - I'll mention it again, just in case: removing
> the named variables and just putting the init lists directly in the call
> might be as (or more) readable - might be worth giving it a go & running it
> through clang-format to see what you think.
>
>
> Here is an example, let me know what do you think:
>
>   {
> RecordData::value_type Record[] = {METADATA, VERSION_MAJOR,
> VERSION_MINOR,
>CLANG_VERSION_MAJOR,
> CLANG_VERSION_MINOR,
>!isysroot.empty(),
> IncludeTimestamps,
>ASTHasCompilerErrors};
> Stream.EmitRecordWithBlob(MetadataAbbrevCode, Record,
>   getClangFullRepositoryVersion());
>   }
>   Stream.EmitRecordWithBlob(
>   MetadataAbbrevCode,
>   (uint64_t[]){METADATA, VERSION_MAJOR, VERSION_MINOR,
> CLANG_VERSION_MAJOR,
>

Why the cast (uint64_t[])? I'm vaguely surprised that even compiles... ?

I would imagine it'd be passed as an init list, then turned into an
ArrayRef from there... but I guess not?


>CLANG_VERSION_MINOR, !isysroot.empty(),
> IncludeTimestamps,
>ASTHasCompilerErrors},
>   getClangFullRepositoryVersion());
>
> Thanks,
>
> —
> Mehdi
>
>
>
>
>
>
>
> - Dave
>
>
>>
>> From: Mehdi Amini 
>>
>> Modified:
>> cfe/trunk/include/clang/Serialization/ASTWriter.h
>> cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp
>> cfe/trunk/lib/Serialization/ASTWriter.cpp
>> cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
>>
>> Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=247233=247232=247233=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
>> +++ cfe/trunk/include/clang/Serialization/ASTWriter.h Wed Sep  9 20:46:39
>> 2015
>> @@ -84,6 +84,7 @@ class ASTWriter : public ASTDeserializat
>>  public:
>>typedef SmallVector RecordData;
>>typedef SmallVectorImpl RecordDataImpl;
>> +  typedef ArrayRef RecordDataRef;
>>
>>friend class ASTDeclWriter;
>>friend class ASTStmtWriter;
>> @@ -756,7 +757,7 @@ public:
>>void AddPath(StringRef Path, RecordDataImpl );
>>
>>/// \brief Emit the current record with the given path as a blob.
>> -  void EmitRecordWithPath(unsigned Abbrev, RecordDataImpl ,
>> +  void EmitRecordWithPath(unsigned Abbrev, RecordDataRef Record,
>>StringRef Path);
>>
>>/// \brief Add a version tuple to the given record
>>
>> Modified: cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp?rev=247233=247232=247233=diff
>>
>> ==
>> --- cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp (original)
>> +++ cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp Wed Sep  9
>> 20:46:39 2015
>> @@ -51,6 +51,7 @@ public:
>>
>>  typedef SmallVector RecordData;
>>  typedef SmallVectorImpl RecordDataImpl;
>> +typedef ArrayRef RecordDataRef;
>>
>>  class SDiagsWriter;
>>
>> @@ -393,13 +394,9 @@ unsigned SDiagsWriter::getEmitFile(const
>>
>>// Lazily generate the record for the file.
>>entry = State->Files.size();
>> -  RecordData Record;
>> -  Record.push_back(RECORD_FILENAME);
>> -  Record.push_back(entry);
>> -  Record.push_back(0); // For legacy.
>> -  Record.push_back(0); // For legacy.
>>StringRef Name(FileName);
>> -  Record.push_back(Name.size());
>> +  RecordData::value_type Record[] = {RECORD_FILENAME, entry, 0 /* For
>> legacy */,
>> + 0 /* For legacy */, Name.size()};
>>State->Stream.EmitRecordWithBlob(State->Abbrevs.get(RECORD_FILENAME),
>> Record,
>> Name);
>>
>> @@ -531,14 +528,11 @@ void 

Re: [PATCH] D12759: [clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stlcontainers.

[Aaron Ballman via cfe-commits]

On Thu, Sep 10, 2015 at 12:14 PM, Alexander Kornienko  wrote:
> On Thu, Sep 10, 2015 at 5:22 PM, Aaron Ballman 
> wrote:
>>
>> > 
>> > Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:49
>> > @@ +48,3 @@
>> > +  SourceLocation SizeOfLoc = SizeOf->getLocStart();
>> > +  auto Diag = diag(SizeOfLoc, "sizeof() doesn't return the size of the
>> > "
>> > +  "container. Did you mean .size()?");
>> > 
>> > 1. Done.
>> > 2. It's actually emitting the diagnostic. And I thought that the comment
>> > on line 52 below explains what happens in enough detail (`// Don't generate
>> > fixes for macros.`). If something is still unclear, can you explain what
>> > exactly?
>>
>> It's not intuitive that creating the local variable actually emits the
>> diagnostic, so it seems like you would be able to hoist the early
>> return up above the local declaration when in fact that would remove
>> the diagnostic entirely. The comment about generating fixes suggests
>> an additional diagnostic, at least to me.
>
>
> This side effect of the diag() method is one of the core things in the
> clang-tidy API for checks. The same pattern is used with other Diag/diag
> methods in clang that produce various DiagnosticBuilders, and so far I
> didn't see problems with it being misleading. So it shouldn't need a comment
> at each usage, imho. May be a comment at the method definition needs to
> cover this aspect as well, if it doesn't.

Yeah, I think what threw me off was the local variable declaration
more than the diag function call. In hindsight, this behavior makes
sense. We'll chalk it up to an education issue on my part, with no
changes needed.

Thanks!

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

Re: [PATCH] D12689: [libc++][static linking] std streams are not initialized prior to their use in static object constructors

[Evgeny Astigeevich via cfe-commits]

eastig added a comment.

In http://reviews.llvm.org/D12689#243295, @rnk wrote:

> I think a better approach would be to use 
> `__attribute__((init_priority(101)))` on Linux and `#pragma init_seg(lib)` on 
> Windows to ensure that libc++'s iostream initializer runs earlier.


Thank you for advice.

I found this discussion of init_priority: Clarification of attribute 
init_priority
https://gcc.gnu.org/ml/gcc-help/2011-05/msg00220.html

It looks like it is not reliable method 
(https://gcc.gnu.org/ml/gcc-help/2011-05/msg00221.html):

> Note that although the documentation doesn't seem to mention it, the

>  init_priority attribute only works correctly when using the GNU linker

>  or gold.


Gcc libc++ does not use it.


http://reviews.llvm.org/D12689



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

r247303 - Debug Info: Remove an unnecessary debug type visitor.

[Adrian Prantl via cfe-commits]

Author: adrian
Date: Thu Sep 10 12:13:31 2015
New Revision: 247303

URL: http://llvm.org/viewvc/llvm-project?rev=247303=rev
Log:
Debug Info: Remove an unnecessary debug type visitor.
Thanks to dblaikie for spotting this.

Modified:
cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
cfe/trunk/test/Modules/ModuleDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=247303=247302=247303=diff
==
--- cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp (original)
+++ cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp Thu Sep 10 
12:13:31 2015
@@ -70,13 +70,6 @@ class PCHContainerGenerator : public AST
   return true;
 }
 
-bool VisitValueDecl(ValueDecl *D) {
-  QualType QualTy = D->getType();
-  if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))
-DI.getOrCreateStandaloneType(QualTy, D->getLocation());
-  return true;
-}
-
 bool VisitObjCInterfaceDecl(ObjCInterfaceDecl *D) {
   QualType QualTy(D->getTypeForDecl(), 0);
   if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))

Modified: cfe/trunk/test/Modules/ModuleDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=247303=247302=247303=diff
==
--- cfe/trunk/test/Modules/ModuleDebugInfo.cpp (original)
+++ cfe/trunk/test/Modules/ModuleDebugInfo.cpp Thu Sep 10 12:13:31 2015
@@ -7,10 +7,12 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -triple %itanium_abi_triple -x objective-c++ -std=c++11 -g 
-fmodules -fmodule-format=obj -fimplicit-module-maps -DMODULES 
-fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o %t.ll -mllvm 
-debug-only=pchcontainer &>%t-mod.ll
 // RUN: cat %t-mod.ll | FileCheck %s
+// RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s
 
 // PCH:
 // RUN: %clang_cc1 -triple %itanium_abi_triple -x c++ -std=c++11 -emit-pch 
-fmodule-format=obj -I %S/Inputs -o %t.pch %S/Inputs/DebugCXX.h -mllvm 
-debug-only=pchcontainer &>%t-pch.ll
 // RUN: cat %t-pch.ll | FileCheck %s
+// RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s
 
 #ifdef MODULES
 @import DebugCXX;
@@ -30,12 +32,11 @@
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
 // CHECK-SAME: name: "Template"
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIfNS_6traitsIf")
-// CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template"
-// CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIlNS_6traitsIl")
 // CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A"
 // CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE")
 // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "FloatInstatiation"
 // no mangled name here yet.
 // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
 // no mangled name here yet.
+
+// CHECK-NEG-NOT: "_ZTSN8DebugCXX8TemplateIlNS_6traitsIl"


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

r247307 - [CUDA] Allow trivial constructors as initializer for __shared__ variables.

[Artem Belevich via cfe-commits]

Author: tra
Date: Thu Sep 10 12:26:58 2015
New Revision: 247307

URL: http://llvm.org/viewvc/llvm-project?rev=247307=rev
Log:
[CUDA] Allow trivial constructors as initializer for __shared__ variables.

Differential Revision: http://reviews.llvm.org/D12739

Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/test/CodeGenCUDA/address-spaces.cu

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=247307=247306=247307=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu Sep 10 12:26:58 2015
@@ -2165,8 +2165,10 @@ void CodeGenModule::EmitGlobalVarDefinit
   if (getLangOpts().CPlusPlus && getLangOpts().CUDAIsDevice
   && D->hasAttr()) {
 if (InitExpr) {
-  Error(D->getLocation(),
-"__shared__ variable cannot have an initialization.");
+  const auto *C = dyn_cast(InitExpr);
+  if (C == nullptr || !C->getConstructor()->hasTrivialBody())
+Error(D->getLocation(),
+  "__shared__ variable cannot have an initialization.");
 }
 Init = llvm::UndefValue::get(getTypes().ConvertType(ASTTy));
   } else if (!InitExpr) {

Modified: cfe/trunk/test/CodeGenCUDA/address-spaces.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCUDA/address-spaces.cu?rev=247307=247306=247307=diff
==
--- cfe/trunk/test/CodeGenCUDA/address-spaces.cu (original)
+++ cfe/trunk/test/CodeGenCUDA/address-spaces.cu Thu Sep 10 12:26:58 2015
@@ -25,6 +25,8 @@ struct MyStruct {
 // CHECK: @_ZZ5func3vE1a = internal addrspace(3) global float 0.00e+00
 // CHECK: @_ZZ5func4vE1a = internal addrspace(3) global float 0.00e+00
 // CHECK: @b = addrspace(3) global float undef
+// CHECK: @c = addrspace(3) global %struct.c undef
+// CHECK  @d = addrspace(3) global %struct.d undef
 
 __device__ void foo() {
   // CHECK: load i32, i32* addrspacecast (i32 addrspace(1)* @i to i32*)
@@ -117,3 +119,15 @@ __device__ int construct_shared_struct()
   return t.getData();
 // CHECK: call i32 @_ZN14StructWithCtor7getDataEv(%struct.StructWithCtor* 
addrspacecast (%struct.StructWithCtor addrspace(3)* 
@_ZZ23construct_shared_structvE1t to %struct.StructWithCtor*))
 }
+
+// Make sure we allow __shared__ structures with default or empty constructors.
+struct c {
+  int i;
+};
+__shared__ struct c c;
+
+struct d {
+  int i;
+  d() {}
+};
+__shared__ struct d d;


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

Re: [PATCH] D12759: [clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stlcontainers.

[Alexander Kornienko via cfe-commits]

On Thu, Sep 10, 2015 at 5:22 PM, Aaron Ballman 
wrote:

> > 
> > Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:49
> > @@ +48,3 @@
> > +  SourceLocation SizeOfLoc = SizeOf->getLocStart();
> > +  auto Diag = diag(SizeOfLoc, "sizeof() doesn't return the size of the "
> > +  "container. Did you mean .size()?");
> > 
> > 1. Done.
> > 2. It's actually emitting the diagnostic. And I thought that the comment
> on line 52 below explains what happens in enough detail (`// Don't generate
> fixes for macros.`). If something is still unclear, can you explain what
> exactly?
>
> It's not intuitive that creating the local variable actually emits the
> diagnostic, so it seems like you would be able to hoist the early
> return up above the local declaration when in fact that would remove
> the diagnostic entirely. The comment about generating fixes suggests
> an additional diagnostic, at least to me.
>

This side effect of the diag() method is one of the core things in the
clang-tidy API for checks. The same pattern is used with other Diag/diag
methods in clang that produce various DiagnosticBuilders, and so far I
didn't see problems with it being misleading. So it shouldn't need a
comment at each usage, imho. May be a comment at the method definition
needs to cover this aspect as well, if it doesn't.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12381: [Static Analyzer] Merge the Objective-C Generics Checker into Dynamic Type Propagation checker.

[Gábor Horváth via cfe-commits]

xazax.hun added a comment.

In http://reviews.llvm.org/D12381#241709, @xazax.hun wrote:

> There are several fallouts from this review, so I decided to split this patch 
> up the following way:
>
> 1. I created a patch to incorporate the result of this review into 
> ObjCGenericsChecker: http://reviews.llvm.org/D12701
> 2. I will created a separate patch to purge the dynamic type information from 
> the GDM for dead symbols.


The second patch is available here: http://reviews.llvm.org/D12767

> 3. Once the former two patch is accepted I will rebase this patch on the top 
> of those, so this will only contain minimal changes required for the merge.



http://reviews.llvm.org/D12381



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

Re: r247303 - Debug Info: Remove an unnecessary debug type visitor.

[Adrian Prantl via cfe-commits]

> On Sep 10, 2015, at 10:19 AM, David Blaikie  wrote:
> 
> 
> 
> On Thu, Sep 10, 2015 at 10:18 AM, David Blaikie  > wrote:
> 
> 
> On Thu, Sep 10, 2015 at 10:13 AM, Adrian Prantl via cfe-commits 
> > wrote:
> Author: adrian
> Date: Thu Sep 10 12:13:31 2015
> New Revision: 247303
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=247303=rev 
> 
> Log:
> Debug Info: Remove an unnecessary debug type visitor.
> Thanks to dblaikie for spotting this.
> 
> Modified:
> cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
> cfe/trunk/test/Modules/ModuleDebugInfo.cpp
> 
> Modified: cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=247303=247302=247303=diff
>  
> 
> ==
> --- cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp (original)
> +++ cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp Thu Sep 10 
> 12:13:31 2015
> @@ -70,13 +70,6 @@ class PCHContainerGenerator : public AST
>return true;
>  }
> 
> -bool VisitValueDecl(ValueDecl *D) {
> -  QualType QualTy = D->getType();
> -  if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))
> -DI.getOrCreateStandaloneType(QualTy, D->getLocation());
> -  return true;
> -}
> -
>  bool VisitObjCInterfaceDecl(ObjCInterfaceDecl *D) {
>QualType QualTy(D->getTypeForDecl(), 0);
>if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))
> 
> Modified: cfe/trunk/test/Modules/ModuleDebugInfo.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=247303=247302=247303=diff
>  
> 
> ==
> --- cfe/trunk/test/Modules/ModuleDebugInfo.cpp (original)
> +++ cfe/trunk/test/Modules/ModuleDebugInfo.cpp Thu Sep 10 12:13:31 2015
> @@ -7,10 +7,12 @@
>  // RUN: rm -rf %t
>  // RUN: %clang_cc1 -triple %itanium_abi_triple -x objective-c++ -std=c++11 
> -g -fmodules -fmodule-format=obj -fimplicit-module-maps -DMODULES 
> -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o %t.ll -mllvm 
> -debug-only=pchcontainer &>%t-mod.ll
>  // RUN: cat %t-mod.ll | FileCheck %s
> +// RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s
> 
>  // PCH:
>  // RUN: %clang_cc1 -triple %itanium_abi_triple -x c++ -std=c++11 -emit-pch 
> -fmodule-format=obj -I %S/Inputs -o %t.pch %S/Inputs/DebugCXX.h -mllvm 
> -debug-only=pchcontainer &>%t-pch.ll
>  // RUN: cat %t-pch.ll | FileCheck %s
> +// RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s
> 
>  #ifdef MODULES
>  @import DebugCXX;
> @@ -30,12 +32,11 @@
>  // CHECK: !DICompositeType(tag: DW_TAG_class_type,
>  // CHECK-SAME: name: "Template"
>  // CHECK-SAME: identifier: 
> "_ZTSN8DebugCXX8TemplateIfNS_6traitsIf")
> -// CHECK: !DICompositeType(tag: DW_TAG_class_type,
> -// CHECK-SAME: name: "Template"
> -// CHECK-SAME: identifier: 
> "_ZTSN8DebugCXX8TemplateIlNS_6traitsIl")
>  // CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A"
>  // CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE")
>  // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "FloatInstatiation"
>  // no mangled name here yet.
>  // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
>  // no mangled name here yet.
> +
> +// CHECK-NEG-NOT: "_ZTSN8DebugCXX8TemplateIlNS_6traitsIl"
> 
> Rather than using a separate check - maybe do the same sort of thing we do 
> for DWARF testing, CHECK-NOT between each DICompositeType, to ensure we only 
> get the types we intended? (including a CHECK-NOT at the end to ensure there 
> aren't any trailing ones)
> 
> But also: How does the current implementation avoid emitting this type? I 
> thought it visited all the type decls, even those not immediately in the 
> module? (you mentioned that was something that you were planning to address 
> in a future patch) Is that not the case? Is this now sufficient to only emit 
> the decls immediately in this module?

It transitively emits all types that are showing up in a type declaration in 
the module. This type is only instantiated inside a variable declaration.

> (Oh, I guess what might be missing is types referenced from /types/ in this 
> module - types referenced from variable decls only are addressed by this 
> patch, but you still don't want to include a full 

Re: [PATCH] D10414: Attach function attribute "arm-restrict-it" instead of passing arm-restrict-it as a backend-option

[James Molloy via cfe-commits]

jmolloy added a comment.

Hi Jim,

In an ideal world, yes. However there's no guarantee that all ARM implementors 
will (a) be able to commit to LLVM or (b) use ToT. Perhaps they're building a 
project that uses clang, or a specific version of clang, and this tuning option 
makes things go faster on their architecture.

I'm not arguing about the defaults, just about the breakage of -mno-restrict-it.

Cheers,

James


http://reviews.llvm.org/D10414



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

Re: r247233 - EmitRecord* API change: accepts ArrayRef instead of a SmallVector (NFC)

[Mehdi Amini via cfe-commits]

> On Sep 9, 2015, at 7:06 PM, David Blaikie  wrote:
> 
> 
> 
> On Wed, Sep 9, 2015 at 6:46 PM, Mehdi Amini via cfe-commits 
> > wrote:
> Author: mehdi_amini
> Date: Wed Sep  9 20:46:39 2015
> New Revision: 247233
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=247233=rev 
> 
> Log:
> EmitRecord* API change: accepts ArrayRef instead of a SmallVector (NFC)
> 
> This reapply a variant commit r247179 after post-commit review from
> D.Blaikie.
> Hopefully I got it right this time: lifetime of initializer list ends
> as with any expression, which make invalid the pattern:
> 
> ArrayRef Arr = { 1, 2, 3, 4};
> 
> Just like StringRef, ArrayRef shouldn't be used to initialize local
> variable but only as function argument.
> 
> Looks pretty reasonable - I'll mention it again, just in case: removing the 
> named variables and just putting the init lists directly in the call might be 
> as (or more) readable - might be worth giving it a go & running it through 
> clang-format to see what you think.

Here is an example, let me know what do you think:

  {
RecordData::value_type Record[] = {METADATA, VERSION_MAJOR, VERSION_MINOR,
   CLANG_VERSION_MAJOR, CLANG_VERSION_MINOR,
   !isysroot.empty(), IncludeTimestamps,
   ASTHasCompilerErrors};
Stream.EmitRecordWithBlob(MetadataAbbrevCode, Record,
  getClangFullRepositoryVersion());
  }
  Stream.EmitRecordWithBlob(
  MetadataAbbrevCode,
  (uint64_t[]){METADATA, VERSION_MAJOR, VERSION_MINOR, CLANG_VERSION_MAJOR,
   CLANG_VERSION_MINOR, !isysroot.empty(), IncludeTimestamps,
   ASTHasCompilerErrors},
  getClangFullRepositoryVersion());

Thanks,

— 
Mehdi






> 
> - Dave
>  
> 
> From: Mehdi Amini >
> 
> Modified:
> cfe/trunk/include/clang/Serialization/ASTWriter.h
> cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp
> cfe/trunk/lib/Serialization/ASTWriter.cpp
> cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
> 
> Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=247233=247232=247233=diff
>  
> 
> ==
> --- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
> +++ cfe/trunk/include/clang/Serialization/ASTWriter.h Wed Sep  9 20:46:39 2015
> @@ -84,6 +84,7 @@ class ASTWriter : public ASTDeserializat
>  public:
>typedef SmallVector RecordData;
>typedef SmallVectorImpl RecordDataImpl;
> +  typedef ArrayRef RecordDataRef;
> 
>friend class ASTDeclWriter;
>friend class ASTStmtWriter;
> @@ -756,7 +757,7 @@ public:
>void AddPath(StringRef Path, RecordDataImpl );
> 
>/// \brief Emit the current record with the given path as a blob.
> -  void EmitRecordWithPath(unsigned Abbrev, RecordDataImpl ,
> +  void EmitRecordWithPath(unsigned Abbrev, RecordDataRef Record,
>StringRef Path);
> 
>/// \brief Add a version tuple to the given record
> 
> Modified: cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp?rev=247233=247232=247233=diff
>  
> 
> ==
> --- cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp (original)
> +++ cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp Wed Sep  9 
> 20:46:39 2015
> @@ -51,6 +51,7 @@ public:
> 
>  typedef SmallVector RecordData;
>  typedef SmallVectorImpl RecordDataImpl;
> +typedef ArrayRef RecordDataRef;
> 
>  class SDiagsWriter;
> 
> @@ -393,13 +394,9 @@ unsigned SDiagsWriter::getEmitFile(const
> 
>// Lazily generate the record for the file.
>entry = State->Files.size();
> -  RecordData Record;
> -  Record.push_back(RECORD_FILENAME);
> -  Record.push_back(entry);
> -  Record.push_back(0); // For legacy.
> -  Record.push_back(0); // For legacy.
>StringRef Name(FileName);
> -  Record.push_back(Name.size());
> +  RecordData::value_type Record[] = {RECORD_FILENAME, entry, 0 /* For legacy 
> */,
> + 0 /* For legacy */, Name.size()};
>State->Stream.EmitRecordWithBlob(State->Abbrevs.get(RECORD_FILENAME), 
> Record,
> Name);
> 
> @@ -531,14 +528,11 @@ void SDiagsWriter::EmitBlockInfoBlock()
> 
>  

Re: r247233 - EmitRecord* API change: accepts ArrayRef instead of a SmallVector (NFC)

[Mehdi Amini via cfe-commits]

> On Sep 10, 2015, at 9:02 AM, David Blaikie  wrote:
> 
> 
> 
> On Thu, Sep 10, 2015 at 9:00 AM, Mehdi Amini  > wrote:
> 
>> On Sep 9, 2015, at 7:06 PM, David Blaikie > > wrote:
>> 
>> 
>> 
>> On Wed, Sep 9, 2015 at 6:46 PM, Mehdi Amini via cfe-commits 
>> > wrote:
>> Author: mehdi_amini
>> Date: Wed Sep  9 20:46:39 2015
>> New Revision: 247233
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=247233=rev 
>> 
>> Log:
>> EmitRecord* API change: accepts ArrayRef instead of a SmallVector (NFC)
>> 
>> This reapply a variant commit r247179 after post-commit review from
>> D.Blaikie.
>> Hopefully I got it right this time: lifetime of initializer list ends
>> as with any expression, which make invalid the pattern:
>> 
>> ArrayRef Arr = { 1, 2, 3, 4};
>> 
>> Just like StringRef, ArrayRef shouldn't be used to initialize local
>> variable but only as function argument.
>> 
>> Looks pretty reasonable - I'll mention it again, just in case: removing the 
>> named variables and just putting the init lists directly in the call might 
>> be as (or more) readable - might be worth giving it a go & running it 
>> through clang-format to see what you think.
> 
> Here is an example, let me know what do you think:
> 
>   {
> RecordData::value_type Record[] = {METADATA, VERSION_MAJOR, VERSION_MINOR,
>CLANG_VERSION_MAJOR, 
> CLANG_VERSION_MINOR,
>!isysroot.empty(), IncludeTimestamps,
>ASTHasCompilerErrors};
> Stream.EmitRecordWithBlob(MetadataAbbrevCode, Record,
>   getClangFullRepositoryVersion());
>   }
>   Stream.EmitRecordWithBlob(
>   MetadataAbbrevCode,
>   (uint64_t[]){METADATA, VERSION_MAJOR, VERSION_MINOR, 
> CLANG_VERSION_MAJOR,
> 
> Why the cast (uint64_t[])? I'm vaguely surprised that even compiles... ?
> 
> I would imagine it'd be passed as an init list, then turned into an ArrayRef 
> from there... but I guess not?


Might be more clear with the callee:

  template 
  void EmitRecordWithBlob(unsigned Abbrev, const Container ,
  StringRef Blob) {
EmitRecordWithAbbrevImpl(Abbrev, makeArrayRef(Vals), Blob, None);
  }

The template can’t be deduced without the cast.

— 
Mehdi






>  
>CLANG_VERSION_MINOR, !isysroot.empty(), IncludeTimestamps,
>ASTHasCompilerErrors},
>   getClangFullRepositoryVersion());
> 
> Thanks,
> 
> — 
> Mehdi
> 
> 
> 
> 
> 
> 
>> 
>> - Dave
>>  
>> 
>> From: Mehdi Amini >
>> 
>> Modified:
>> cfe/trunk/include/clang/Serialization/ASTWriter.h
>> cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp
>> cfe/trunk/lib/Serialization/ASTWriter.cpp
>> cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
>> 
>> Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=247233=247232=247233=diff
>>  
>> 
>> ==
>> --- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
>> +++ cfe/trunk/include/clang/Serialization/ASTWriter.h Wed Sep  9 20:46:39 
>> 2015
>> @@ -84,6 +84,7 @@ class ASTWriter : public ASTDeserializat
>>  public:
>>typedef SmallVector RecordData;
>>typedef SmallVectorImpl RecordDataImpl;
>> +  typedef ArrayRef RecordDataRef;
>> 
>>friend class ASTDeclWriter;
>>friend class ASTStmtWriter;
>> @@ -756,7 +757,7 @@ public:
>>void AddPath(StringRef Path, RecordDataImpl );
>> 
>>/// \brief Emit the current record with the given path as a blob.
>> -  void EmitRecordWithPath(unsigned Abbrev, RecordDataImpl ,
>> +  void EmitRecordWithPath(unsigned Abbrev, RecordDataRef Record,
>>StringRef Path);
>> 
>>/// \brief Add a version tuple to the given record
>> 
>> Modified: cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp?rev=247233=247232=247233=diff
>>  
>> 
>> ==
>> --- cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp (original)
>> +++ cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp Wed Sep  9 
>> 20:46:39 2015
>> @@ -51,6 +51,7 @@ public:
>> 
>>  typedef SmallVector 

Re: [PATCH] D12759: [clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stlcontainers.

[Aaron Ballman via cfe-commits]

On Thu, Sep 10, 2015 at 12:04 PM, Alexander Kornienko  wrote:
> alexfh marked 5 inline comments as done.
>
> 
> Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:23
> @@ +22,3 @@
> +  E = E->IgnoreImpCasts();
> +  if (isa(E) || isa(E))
> +return true;
> 
> I don't think we need to remove anything beyond the most external pair of 
> parentheses.

That's true; the extra parens would just remain as they are, we
wouldn't need to add more.

>
> 
> Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:26
> @@ +25,3 @@
> +  if (const auto *Op = dyn_cast(E)) {
> +return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call &&
> +   Op->getOperator() != OO_Subscript;
> 
> Do you have an example of an expression that will break when a `.size()` is 
> appended to it? Note, that it should be an expression of a class type.

Nope, everything I can think of is already covered it seems.

>
> 
> Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:39
> @@ +38,3 @@
> +recordDecl(matchesName("^(::std::|::string)"),
> +   hasMethod(methodDecl(hasName("size"), 
> isPublic(),
> +isConst()))
> 
> Needed for code bases that use a std::string-like string class defined in the 
> global namespace. Maybe we need a configuration option for custom container 
> regexps. But this will likely be a follow up.

Follow-up makes sense to me. Perhaps the option can simply be
"anything with a size() member function" vs "only STL containers".

Thanks, with that, LGTM!

~Aaron

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

r247302 - Re-commit r247218: "Fix Clang-tidy misc-use-override warnings, other minor fixes"

[Hans Wennborg via cfe-commits]

Author: hans
Date: Thu Sep 10 12:07:54 2015
New Revision: 247302

URL: http://llvm.org/viewvc/llvm-project?rev=247302=rev
Log:
Re-commit r247218: "Fix Clang-tidy misc-use-override warnings, other minor 
fixes"

This never broke the build; it was the LLVM side, r247216, that caused problems.

Modified:
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
cfe/trunk/lib/Driver/ToolChains.h
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/lib/Frontend/PCHContainerOperations.cpp

Modified: cfe/trunk/lib/Basic/Targets.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=247302=247301=247302=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Thu Sep 10 12:07:54 2015
@@ -30,6 +30,7 @@
 #include "llvm/Support/TargetParser.h"
 #include 
 #include 
+
 using namespace clang;
 
 
//===--===//
@@ -739,7 +740,7 @@ namespace {
 template 
 class WebAssemblyOSTargetInfo : public OSTargetInfo {
   void getOSDefines(const LangOptions , const llvm::Triple ,
-MacroBuilder ) const override final {
+MacroBuilder ) const final {
 // A common platform macro.
 if (Opts.POSIXThreads)
   Builder.defineMacro("_REENTRANT");
@@ -749,7 +750,7 @@ class WebAssemblyOSTargetInfo : public O
   }
 
   // As an optimization, group static init code together in a section.
-  const char *getStaticInitSectionSpecifier() const override final {
+  const char *getStaticInitSectionSpecifier() const final {
 return ".text.__startup";
   }
 
@@ -7011,13 +7012,13 @@ private:
   Features["simd128"] = true;
 return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
   }
-  bool hasFeature(StringRef Feature) const override final {
+  bool hasFeature(StringRef Feature) const final {
 return llvm::StringSwitch(Feature)
 .Case("simd128", SIMDLevel >= SIMD128)
 .Default(false);
   }
   bool handleTargetFeatures(std::vector ,
-DiagnosticsEngine ) override final {
+DiagnosticsEngine ) final {
 for (const auto  : Features) {
   if (Feature == "+simd128") {
 SIMDLevel = std::max(SIMDLevel, SIMD128);
@@ -7034,7 +7035,7 @@ private:
 }
 return true;
   }
-  bool setCPU(const std::string ) override final {
+  bool setCPU(const std::string ) final {
 return llvm::StringSwitch(Name)
   .Case("mvp",   true)
   .Case("bleeding-edge", true)
@@ -7042,32 +7043,32 @@ private:
   .Default(false);
   }
   void getTargetBuiltins(const Builtin::Info *,
- unsigned ) const override final {
+ unsigned ) const final {
 Records = BuiltinInfo;
 NumRecords = clang::WebAssembly::LastTSBuiltin - Builtin::FirstTSBuiltin;
   }
-  BuiltinVaListKind getBuiltinVaListKind() const override final {
+  BuiltinVaListKind getBuiltinVaListKind() const final {
 // TODO: Implement va_list properly.
 return VoidPtrBuiltinVaList;
   }
   void getGCCRegNames(const char *const *,
-  unsigned ) const override final {
+  unsigned ) const final {
 Names = nullptr;
 NumNames = 0;
   }
   void getGCCRegAliases(const GCCRegAlias *,
-unsigned ) const override final {
+unsigned ) const final {
 Aliases = nullptr;
 NumAliases = 0;
   }
   bool
   validateAsmConstraint(const char *,
-TargetInfo::ConstraintInfo ) const override final 
{
+TargetInfo::ConstraintInfo ) const final {
 return false;
   }
-  const char *getClobbers() const override final { return ""; }
-  bool isCLZForZeroUndef() const override final { return false; }
-  bool hasInt128Type() const override final { return true; }
+  const char *getClobbers() const final { return ""; }
+  bool isCLZForZeroUndef() const final { return false; }
+  bool hasInt128Type() const final { return true; }
 };
 
 const Builtin::Info WebAssemblyTargetInfo::BuiltinInfo[] = {

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=247302=247301=247302=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Thu Sep 10 12:07:54 2015
@@ -59,7 +59,7 @@ public:
   virtual const VarDecl *getThreadIDVariable() const = 0;
 
   /// \brief Emit the captured statement body.
-  virtual void EmitBody(CodeGenFunction , const Stmt *S) override;
+  void EmitBody(CodeGenFunction , const Stmt *S) override;
 
   /// \brief Get 

Re: r247303 - Debug Info: Remove an unnecessary debug type visitor.

[David Blaikie via cfe-commits]

On Thu, Sep 10, 2015 at 10:13 AM, Adrian Prantl via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: adrian
> Date: Thu Sep 10 12:13:31 2015
> New Revision: 247303
>
> URL: http://llvm.org/viewvc/llvm-project?rev=247303=rev
> Log:
> Debug Info: Remove an unnecessary debug type visitor.
> Thanks to dblaikie for spotting this.
>
> Modified:
> cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
> cfe/trunk/test/Modules/ModuleDebugInfo.cpp
>
> Modified: cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=247303=247302=247303=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp (original)
> +++ cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp Thu Sep 10
> 12:13:31 2015
> @@ -70,13 +70,6 @@ class PCHContainerGenerator : public AST
>return true;
>  }
>
> -bool VisitValueDecl(ValueDecl *D) {
> -  QualType QualTy = D->getType();
> -  if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))
> -DI.getOrCreateStandaloneType(QualTy, D->getLocation());
> -  return true;
> -}
> -
>  bool VisitObjCInterfaceDecl(ObjCInterfaceDecl *D) {
>QualType QualTy(D->getTypeForDecl(), 0);
>if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))
>
> Modified: cfe/trunk/test/Modules/ModuleDebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=247303=247302=247303=diff
>
> ==
> --- cfe/trunk/test/Modules/ModuleDebugInfo.cpp (original)
> +++ cfe/trunk/test/Modules/ModuleDebugInfo.cpp Thu Sep 10 12:13:31 2015
> @@ -7,10 +7,12 @@
>  // RUN: rm -rf %t
>  // RUN: %clang_cc1 -triple %itanium_abi_triple -x objective-c++
> -std=c++11 -g -fmodules -fmodule-format=obj -fimplicit-module-maps
> -DMODULES -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o %t.ll
> -mllvm -debug-only=pchcontainer &>%t-mod.ll
>  // RUN: cat %t-mod.ll | FileCheck %s
> +// RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s
>
>  // PCH:
>  // RUN: %clang_cc1 -triple %itanium_abi_triple -x c++ -std=c++11
> -emit-pch -fmodule-format=obj -I %S/Inputs -o %t.pch %S/Inputs/DebugCXX.h
> -mllvm -debug-only=pchcontainer &>%t-pch.ll
>  // RUN: cat %t-pch.ll | FileCheck %s
> +// RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s
>
>  #ifdef MODULES
>  @import DebugCXX;
> @@ -30,12 +32,11 @@
>  // CHECK: !DICompositeType(tag: DW_TAG_class_type,
>  // CHECK-SAME: name: "Template >"
>  // CHECK-SAME: identifier:
> "_ZTSN8DebugCXX8TemplateIfNS_6traitsIf")
> -// CHECK: !DICompositeType(tag: DW_TAG_class_type,
> -// CHECK-SAME: name: "Template"
> -// CHECK-SAME: identifier:
> "_ZTSN8DebugCXX8TemplateIlNS_6traitsIl")
>  // CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A"
>  // CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE")
>  // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "FloatInstatiation"
>  // no mangled name here yet.
>  // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
>  // no mangled name here yet.
> +
> +// CHECK-NEG-NOT: "_ZTSN8DebugCXX8TemplateIlNS_6traitsIl"
>

Rather than using a separate check - maybe do the same sort of thing we do
for DWARF testing, CHECK-NOT between each DICompositeType, to ensure we
only get the types we intended? (including a CHECK-NOT at the end to ensure
there aren't any trailing ones)

But also: How does the current implementation avoid emitting this type? I
thought it visited all the type decls, even those not immediately in the
module? (you mentioned that was something that you were planning to address
in a future patch) Is that not the case? Is this now sufficient to only
emit the decls immediately in this module?


>
>
> ___
> 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: [PATCH] D12695: [Driver] Use UniversalCRT on Windows if available

[Reid Kleckner via cfe-commits]

rnk added a comment.

There's a bunch of whitespace issues that clang-format can resolve. We also 
have the convention that variables are StudlyCaps, which isn't followed in a 
few places.

Do you need someone to commit this for you? If so, I can patch this in, test it 
manually, and deal with the style stuff if you deal with the VSDir part.



Comment at: lib/Driver/MSVCToolChain.cpp:293
@@ +292,3 @@
+// specific header files. If not, they are probably shipped with Universal CRT.
+bool clang::driver::toolchains::MSVCToolChain::useUniversalCRT() const
+{

This should really take VSDir as a parameter instead of recomputing it, since 
all the callers are already computing it.


http://reviews.llvm.org/D12695



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

Re: [PATCH] D12695: [Driver] Use UniversalCRT on Windows if available

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

In http://reviews.llvm.org/D12695#243552, @rnk wrote:

> There's a bunch of whitespace issues that clang-format can resolve.


Thanks! I'll reformat it.

> We also have the convention that variables are StudlyCaps, which isn't 
> followed in a few places.


I've tried to follow the style of the source file. Do you think I should force 
this rule here?

> Do you need someone to commit this for you? If so, I can patch this in, test 
> it manually, and deal with the style stuff if you deal with the VSDir part.


I'd really appreciate it. I'll prepare a new version in a few minutes.


http://reviews.llvm.org/D12695



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

[PATCH] D12769: [analyzer] Update SATestBuild.py to set -isysroot for preprocessed files

[Devin Coughlin via cfe-commits]

dcoughlin created this revision.
dcoughlin added reviewers: zaks.anna, xazax.hun.
dcoughlin added a subscriber: cfe-commits.

Update the static analyzer buildbot script to set -isysroot to the OS X SDK 
path when analyzing preprocessed files.

http://reviews.llvm.org/D12769

Files:
  utils/analyzer/SATestBuild.py

Index: utils/analyzer/SATestBuild.py
===
--- utils/analyzer/SATestBuild.py
+++ utils/analyzer/SATestBuild.py
@@ -52,7 +52,7 @@
 import time
 import plistlib
 import argparse
-from subprocess import check_call, CalledProcessError
+from subprocess import check_call, check_output, CalledProcessError
 
 #--
 # Helper functions.
@@ -255,14 +255,31 @@
 return True
 return False
 
+# Get the path to the SDK for the given SDK name. Returns None if
+# the path cannot be determined.
+def getSDKPath(SDKName):
+if which("xcrun") is None:
+return None
+
+Cmd = "xcrun --sdk " + SDKName + " --show-sdk-path"
+return check_output(Cmd, shell=True).rstrip()
+
 # Run analysis on a set of preprocessed files.
 def runAnalyzePreprocessed(Dir, SBOutputDir, Mode):
 if os.path.exists(os.path.join(Dir, BuildScript)):
 print "Error: The preprocessed files project should not contain %s" % \
BuildScript
 raise Exception()
 
-CmdPrefix = Clang + " -cc1 -analyze -analyzer-output=plist -w "
+CmdPrefix = Clang + " "
+
+# For now, we assume the preprocessed files should be analyzed
+# with the OS X SDK.
+SDKPath = getSDKPath("macosx")
+if SDKPath is not None:
+  CmdPrefix += "-cc1 -isysroot " + SDKPath + " "
+
+CmdPrefix += "-analyze -analyzer-output=plist -w "
 CmdPrefix += "-analyzer-checker=" + Checkers +" -fcxx-exceptions -fblocks "
 
 if (Mode == 2) :


Index: utils/analyzer/SATestBuild.py
===
--- utils/analyzer/SATestBuild.py
+++ utils/analyzer/SATestBuild.py
@@ -52,7 +52,7 @@
 import time
 import plistlib
 import argparse
-from subprocess import check_call, CalledProcessError
+from subprocess import check_call, check_output, CalledProcessError
 
 #--
 # Helper functions.
@@ -255,14 +255,31 @@
 return True
 return False
 
+# Get the path to the SDK for the given SDK name. Returns None if
+# the path cannot be determined.
+def getSDKPath(SDKName):
+if which("xcrun") is None:
+return None
+
+Cmd = "xcrun --sdk " + SDKName + " --show-sdk-path"
+return check_output(Cmd, shell=True).rstrip()
+
 # Run analysis on a set of preprocessed files.
 def runAnalyzePreprocessed(Dir, SBOutputDir, Mode):
 if os.path.exists(os.path.join(Dir, BuildScript)):
 print "Error: The preprocessed files project should not contain %s" % \
BuildScript
 raise Exception()
 
-CmdPrefix = Clang + " -cc1 -analyze -analyzer-output=plist -w "
+CmdPrefix = Clang + " "
+
+# For now, we assume the preprocessed files should be analyzed
+# with the OS X SDK.
+SDKPath = getSDKPath("macosx")
+if SDKPath is not None:
+  CmdPrefix += "-cc1 -isysroot " + SDKPath + " "
+
+CmdPrefix += "-analyze -analyzer-output=plist -w "
 CmdPrefix += "-analyzer-checker=" + Checkers +" -fcxx-exceptions -fblocks "
 
 if (Mode == 2) :
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12695: [Driver] Use UniversalCRT on Windows if available

[Igor Kudrin via cfe-commits]

ikudrin marked an inline comment as done.
ikudrin added a comment.

In http://reviews.llvm.org/D12695#243320, @rnk wrote:

> FYI @ruiu is moving this code to LLVM in http://reviews.llvm.org/D12604.


Thanks. I think I'll move my changes to the new place when he's finished.


http://reviews.llvm.org/D12695



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

Re: [PATCH] D11664: [CUDA] Implemented additional processing steps needed to link with CUDA libdevice bitcode.

[Artem Belevich via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL247317: [CUDA] Postprocess bitcode linked in during 
device-side CUDA compilation. (authored by tra).

Changed prior to commit:
  http://reviews.llvm.org/D11664?vs=34467=34470#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D11664

Files:
  cfe/trunk/include/clang/Basic/LangOptions.def
  cfe/trunk/include/clang/Driver/CC1Options.td
  cfe/trunk/lib/CodeGen/CodeGenAction.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGenCUDA/Inputs/device-code.ll
  cfe/trunk/test/CodeGenCUDA/link-device-bitcode.cu

Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -1406,6 +1406,9 @@
   if (Args.hasArg(OPT_fcuda_is_device))
 Opts.CUDAIsDevice = 1;
 
+  if (Args.hasArg(OPT_fcuda_uses_libdevice))
+Opts.CUDAUsesLibDevice = 1;
+
   if (Args.hasArg(OPT_fcuda_allow_host_calls_from_host_device))
 Opts.CUDAAllowHostCallsFromHostDevice = 1;
 
Index: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp
@@ -159,7 +159,12 @@
   if (LinkModule) {
 if (Linker::LinkModules(
 M, LinkModule.get(),
-[=](const DiagnosticInfo ) { linkerDiagnosticHandler(DI); }))
+[=](const DiagnosticInfo ) { linkerDiagnosticHandler(DI); },
+(LangOpts.CUDA && LangOpts.CUDAIsDevice &&
+ LangOpts.CUDAUsesLibDevice)
+? (Linker::Flags::LinkOnlyNeeded |
+   Linker::Flags::InternalizeLinkedSymbols)
+: Linker::Flags::None))
   return;
   }
 
Index: cfe/trunk/include/clang/Driver/CC1Options.td
===
--- cfe/trunk/include/clang/Driver/CC1Options.td
+++ cfe/trunk/include/clang/Driver/CC1Options.td
@@ -659,6 +659,8 @@
   HelpText<"Disable all cross-target (host, device, etc.) call checks in CUDA">;
 def fcuda_include_gpubinary : Separate<["-"], "fcuda-include-gpubinary">,
   HelpText<"Incorporate CUDA device-side binary into host object file.">;
+def fcuda_uses_libdevice : Flag<["-"], "fcuda-uses-libdevice">,
+  HelpText<"Selectively link and internalize bitcode.">;
 
 } // let Flags = [CC1Option]
 
Index: cfe/trunk/include/clang/Basic/LangOptions.def
===
--- cfe/trunk/include/clang/Basic/LangOptions.def
+++ cfe/trunk/include/clang/Basic/LangOptions.def
@@ -166,6 +166,7 @@
 LANGOPT(CUDAIsDevice  , 1, 0, "Compiling for CUDA device")
 LANGOPT(CUDAAllowHostCallsFromHostDevice, 1, 0, "Allow host device functions to call host functions")
 LANGOPT(CUDADisableTargetCallChecks, 1, 0, "Disable checks for call targets (host, device, etc.)")
+LANGOPT(CUDAUsesLibDevice , 1, 0, "Selectively link and internalize bitcode.")
 
 LANGOPT(AssumeSaneOperatorNew , 1, 1, "implicit __attribute__((malloc)) for C++'s new operators")
 LANGOPT(SizedDeallocation , 1, 0, "enable sized deallocation functions")
Index: cfe/trunk/test/CodeGenCUDA/Inputs/device-code.ll
===
--- cfe/trunk/test/CodeGenCUDA/Inputs/device-code.ll
+++ cfe/trunk/test/CodeGenCUDA/Inputs/device-code.ll
@@ -0,0 +1,38 @@
+; Simple bit of IR to mimic CUDA's libdevice. We want to be
+; able to link with it and we need to make sure all __nvvm_reflect
+; calls are eliminated by the time PTX has been produced.
+
+target triple = "nvptx-unknown-cuda"
+
+declare i32 @__nvvm_reflect(i8*)
+
+@"$str" = private addrspace(1) constant [8 x i8] c"USE_MUL\00"
+
+define void @unused_subfunc(float %a) {
+   ret void
+}
+
+define void @used_subfunc(float %a) {
+   ret void
+}
+
+define float @_Z17device_mul_or_addff(float %a, float %b) {
+  %reflect = call i32 @__nvvm_reflect(i8* addrspacecast (i8 addrspace(1)* getelementptr inbounds ([8 x i8], [8 x i8] addrspace(1)* @"$str", i32 0, i32 0) to i8*))
+  %cmp = icmp ne i32 %reflect, 0
+  br i1 %cmp, label %use_mul, label %use_add
+
+use_mul:
+  %ret1 = fmul float %a, %b
+  br label %exit
+
+use_add:
+  %ret2 = fadd float %a, %b
+  br label %exit
+
+exit:
+  %ret = phi float [%ret1, %use_mul], [%ret2, %use_add]
+
+  call void @used_subfunc(float %ret)
+
+  ret float %ret
+}
Index: cfe/trunk/test/CodeGenCUDA/link-device-bitcode.cu
===
--- cfe/trunk/test/CodeGenCUDA/link-device-bitcode.cu
+++ cfe/trunk/test/CodeGenCUDA/link-device-bitcode.cu
@@ -0,0 +1,56 @@
+// Test for linking with CUDA's libdevice as outlined in
+// http://llvm.org/docs/NVPTXUsage.html#linking-with-libdevice
+//
+// REQUIRES: nvptx-registered-target
+//
+// Prepare bitcode 

Re: [PATCH] D12769: [analyzer] Update SATestBuild.py to set -isysroot for preprocessed files

[Devin Coughlin via cfe-commits]

dcoughlin updated this revision to Diff 34471.
dcoughlin added a comment.

Pass -cc1 to clang even when SDK path is not found.


http://reviews.llvm.org/D12769

Files:
  utils/analyzer/SATestBuild.py

Index: utils/analyzer/SATestBuild.py
===
--- utils/analyzer/SATestBuild.py
+++ utils/analyzer/SATestBuild.py
@@ -52,7 +52,7 @@
 import time
 import plistlib
 import argparse
-from subprocess import check_call, CalledProcessError
+from subprocess import check_call, check_output, CalledProcessError
 
 #--
 # Helper functions.
@@ -255,14 +255,31 @@
 return True
 return False
 
+# Get the path to the SDK for the given SDK name. Returns None if
+# the path cannot be determined.
+def getSDKPath(SDKName):
+if which("xcrun") is None:
+return None
+
+Cmd = "xcrun --sdk " + SDKName + " --show-sdk-path"
+return check_output(Cmd, shell=True).rstrip()
+
 # Run analysis on a set of preprocessed files.
 def runAnalyzePreprocessed(Dir, SBOutputDir, Mode):
 if os.path.exists(os.path.join(Dir, BuildScript)):
 print "Error: The preprocessed files project should not contain %s" % \
BuildScript
 raise Exception()
 
-CmdPrefix = Clang + " -cc1 -analyze -analyzer-output=plist -w "
+CmdPrefix = Clang + " -cc1 "
+
+# For now, we assume the preprocessed files should be analyzed
+# with the OS X SDK.
+SDKPath = getSDKPath("macosx")
+if SDKPath is not None:
+  CmdPrefix += "-isysroot " + SDKPath + " "
+
+CmdPrefix += "-analyze -analyzer-output=plist -w "
 CmdPrefix += "-analyzer-checker=" + Checkers +" -fcxx-exceptions -fblocks "
 
 if (Mode == 2) :


Index: utils/analyzer/SATestBuild.py
===
--- utils/analyzer/SATestBuild.py
+++ utils/analyzer/SATestBuild.py
@@ -52,7 +52,7 @@
 import time
 import plistlib
 import argparse
-from subprocess import check_call, CalledProcessError
+from subprocess import check_call, check_output, CalledProcessError
 
 #--
 # Helper functions.
@@ -255,14 +255,31 @@
 return True
 return False
 
+# Get the path to the SDK for the given SDK name. Returns None if
+# the path cannot be determined.
+def getSDKPath(SDKName):
+if which("xcrun") is None:
+return None
+
+Cmd = "xcrun --sdk " + SDKName + " --show-sdk-path"
+return check_output(Cmd, shell=True).rstrip()
+
 # Run analysis on a set of preprocessed files.
 def runAnalyzePreprocessed(Dir, SBOutputDir, Mode):
 if os.path.exists(os.path.join(Dir, BuildScript)):
 print "Error: The preprocessed files project should not contain %s" % \
BuildScript
 raise Exception()
 
-CmdPrefix = Clang + " -cc1 -analyze -analyzer-output=plist -w "
+CmdPrefix = Clang + " -cc1 "
+
+# For now, we assume the preprocessed files should be analyzed
+# with the OS X SDK.
+SDKPath = getSDKPath("macosx")
+if SDKPath is not None:
+  CmdPrefix += "-isysroot " + SDKPath + " "
+
+CmdPrefix += "-analyze -analyzer-output=plist -w "
 CmdPrefix += "-analyzer-checker=" + Checkers +" -fcxx-exceptions -fblocks "
 
 if (Mode == 2) :
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r247233 - EmitRecord* API change: accepts ArrayRef instead of a SmallVector (NFC)

[David Blaikie via cfe-commits]

On Thu, Sep 10, 2015 at 11:39 AM, Richard Smith 
wrote:

> On Thu, Sep 10, 2015 at 9:13 AM, David Blaikie via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>>
>>
>> On Thu, Sep 10, 2015 at 9:07 AM, Mehdi Amini 
>> wrote:
>>
>>>
>>> On Sep 10, 2015, at 9:02 AM, David Blaikie  wrote:
>>>
>>>
>>>
>>> On Thu, Sep 10, 2015 at 9:00 AM, Mehdi Amini 
>>> wrote:
>>>

 On Sep 9, 2015, at 7:06 PM, David Blaikie  wrote:



 On Wed, Sep 9, 2015 at 6:46 PM, Mehdi Amini via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

> Author: mehdi_amini
> Date: Wed Sep  9 20:46:39 2015
> New Revision: 247233
>
> URL: http://llvm.org/viewvc/llvm-project?rev=247233=rev
> Log:
> EmitRecord* API change: accepts ArrayRef instead of a SmallVector (NFC)
>
> This reapply a variant commit r247179 after post-commit review from
> D.Blaikie.
> Hopefully I got it right this time: lifetime of initializer list ends
> as with any expression, which make invalid the pattern:
>
> ArrayRef Arr = { 1, 2, 3, 4};
>
> Just like StringRef, ArrayRef shouldn't be used to initialize local
> variable but only as function argument.
>

 Looks pretty reasonable - I'll mention it again, just in case: removing
 the named variables and just putting the init lists directly in the call
 might be as (or more) readable - might be worth giving it a go & running it
 through clang-format to see what you think.


 Here is an example, let me know what do you think:

   {
 RecordData::value_type Record[] = {METADATA, VERSION_MAJOR,
 VERSION_MINOR,
CLANG_VERSION_MAJOR,
 CLANG_VERSION_MINOR,
!isysroot.empty(),
 IncludeTimestamps,
ASTHasCompilerErrors};
 Stream.EmitRecordWithBlob(MetadataAbbrevCode, Record,
   getClangFullRepositoryVersion());
   }
   Stream.EmitRecordWithBlob(
   MetadataAbbrevCode,
   (uint64_t[]){METADATA, VERSION_MAJOR, VERSION_MINOR,
 CLANG_VERSION_MAJOR,

>>>
>>> Why the cast (uint64_t[])? I'm vaguely surprised that even compiles... ?
>>>
>>> This is a GNU extension (C99 compound literals in C++). It won't compile
> on MSVC.
>

Ah, good to know - thanks for the catch.


> I would imagine it'd be passed as an init list, then turned into an
>>> ArrayRef from there... but I guess not?
>>>
>>>
>>>
>>> Might be more clear with the callee:
>>>
>>>   template 
>>>   void EmitRecordWithBlob(unsigned Abbrev, const Container ,
>>>   StringRef Blob) {
>>> EmitRecordWithAbbrevImpl(Abbrev, makeArrayRef(Vals), Blob, None);
>>>   }
>>>
>>> The template can’t be deduced without the cast.
>>>
>>
>> Yeah, curious though. Another hole in perfect forwarding I suppose (not
>> that this ^ is perfect forwarding, but even with perfect forwarding it
>> doesn't cope well)
>>
>> Yeah, the cast is rather unfortunate. Hrm.
>>
>> Ah well - probably just as good to leave it as-is for now. If someone has
>> a flash of inspiration later & figures out a way to make it better, so be
>> it.
>>
>
> The way to handle this is to add another EmitRecord overload that takes a
> std::initializer_list.
>

Except the integer types aren't known - which has been one of the wrinkles
with this whole code (or at least the template is trying to allow arrays of
different integer types - but perhaps that's not important for the init
list/literal case?)


>
>
>>> —
>>> Mehdi
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
CLANG_VERSION_MINOR, !isysroot.empty(),
 IncludeTimestamps,
ASTHasCompilerErrors},
   getClangFullRepositoryVersion());

 Thanks,

 —
 Mehdi







 - Dave


>
> From: Mehdi Amini 
>
> Modified:
> cfe/trunk/include/clang/Serialization/ASTWriter.h
> cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp
> cfe/trunk/lib/Serialization/ASTWriter.cpp
> cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
>
> Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=247233=247232=247233=diff
>
> ==
> --- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
> +++ cfe/trunk/include/clang/Serialization/ASTWriter.h Wed Sep  9
> 20:46:39 2015
> @@ -84,6 +84,7 @@ class ASTWriter : public ASTDeserializat
>  public:
>typedef SmallVector RecordData;
>typedef SmallVectorImpl 

Re: [PATCH] D12689: [libc++][static linking] std streams are not initialized prior to their use in static object constructors

[Richard Smith via cfe-commits]

rsmith added a subscriber: rsmith.
rsmith added a comment.

Can we instead fix this in Clang by ensuring that libc++ is put at the right 
position in the static link order so that its initializers run first? libc++'s 
avoidance of running iostreams init code from every translation unit is a 
significant startup performance feature, and we shouldn't abandon it unless we 
really have to.


http://reviews.llvm.org/D12689



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

Re: [PATCH] D12769: [analyzer] Update SATestBuild.py to set -isysroot for preprocessed files

[Devin Coughlin via cfe-commits]

dcoughlin added inline comments.


Comment at: utils/analyzer/SATestBuild.py:277
@@ +276,3 @@
+# For now, we assume the preprocessed files should be analyzed
+# with the OS X SDK.
+SDKPath = getSDKPath("macosx")

xazax.hun wrote:
> I think it might be better to check if the host os is OS X. This way one 
> might be able to check preprocessed files on other linux.
The patch checks for the existence of xcrun in the path and doesn't set 
-isysroot if that is not present, so this should continue to work on Linux and 
cygwin.


http://reviews.llvm.org/D12769



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

Re: PATCH: Expose the 'file' that is associated with a compile database command

[Argyrios Kyrtzidis via cfe-commits]

> On Sep 10, 2015, at 1:48 AM, Manuel Klimek  wrote:
> 
> @@ -179,11 +185,13 @@ public:
>/// \param Directory The base directory used in the 
> FixedCompilationDatabase.
>static FixedCompilationDatabase *loadFromCommandLine(int ,
> const char *const 
> *Argv,
> -   Twine Directory = 
> ".");
> +   Twine Directory = ".",
> +   Twine File = Twine());
>  
> A fixed compilation database returns the same command lines for all files, 
> thus having a file in the function seems strange.

Ah ok, thanks for clarifying.

> 
> What exactly is the use case? So far, the compilation database has been 
> designed for 2 use cases:
> 1. for a file, get the compile commands; the user already knows the file, no 
> need to get the file
> 2. get all compile commands; for that, we have the getAllFiles() method, so a 
> user can get all known files (for compilation databases that support that), 
> and then get the compile command line.

It’s #2, I want to get all compile commands. But it seems really strange to me 
that the ‘file’ starts as a property of the compile command in the json file 
but then it gets dropped and I need to do work to re-associate the files with 
the compile commands again.
I need to get a list of all the files and then for each one do a lookup to get 
the associated commands. I then have to maintain this association myself, 
passing a command along with its file separately or the structure that keeps 
track of the association.

It seems simpler to me to include the file that was associated with the command 
(if the compilation database supports that) along with the command, is there a 
downside I’m missing ?

> 
> Thoughts?
> /Manuel
> 
> On Wed, Sep 9, 2015 at 9:36 PM Argyrios Kyrtzidis  > wrote:
> Hi,
> 
> The attached patch exposes the ‘file’ entry in a compilation database 
> command, via the CompileCommand structure.
> 

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

Re: r247233 - EmitRecord* API change: accepts ArrayRef instead of a SmallVector (NFC)

[Richard Smith via cfe-commits]

On Thu, Sep 10, 2015 at 9:13 AM, David Blaikie via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
>
> On Thu, Sep 10, 2015 at 9:07 AM, Mehdi Amini 
> wrote:
>
>>
>> On Sep 10, 2015, at 9:02 AM, David Blaikie  wrote:
>>
>>
>>
>> On Thu, Sep 10, 2015 at 9:00 AM, Mehdi Amini 
>> wrote:
>>
>>>
>>> On Sep 9, 2015, at 7:06 PM, David Blaikie  wrote:
>>>
>>>
>>>
>>> On Wed, Sep 9, 2015 at 6:46 PM, Mehdi Amini via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: mehdi_amini
 Date: Wed Sep  9 20:46:39 2015
 New Revision: 247233

 URL: http://llvm.org/viewvc/llvm-project?rev=247233=rev
 Log:
 EmitRecord* API change: accepts ArrayRef instead of a SmallVector (NFC)

 This reapply a variant commit r247179 after post-commit review from
 D.Blaikie.
 Hopefully I got it right this time: lifetime of initializer list ends
 as with any expression, which make invalid the pattern:

 ArrayRef Arr = { 1, 2, 3, 4};

 Just like StringRef, ArrayRef shouldn't be used to initialize local
 variable but only as function argument.

>>>
>>> Looks pretty reasonable - I'll mention it again, just in case: removing
>>> the named variables and just putting the init lists directly in the call
>>> might be as (or more) readable - might be worth giving it a go & running it
>>> through clang-format to see what you think.
>>>
>>>
>>> Here is an example, let me know what do you think:
>>>
>>>   {
>>> RecordData::value_type Record[] = {METADATA, VERSION_MAJOR,
>>> VERSION_MINOR,
>>>CLANG_VERSION_MAJOR,
>>> CLANG_VERSION_MINOR,
>>>!isysroot.empty(),
>>> IncludeTimestamps,
>>>ASTHasCompilerErrors};
>>> Stream.EmitRecordWithBlob(MetadataAbbrevCode, Record,
>>>   getClangFullRepositoryVersion());
>>>   }
>>>   Stream.EmitRecordWithBlob(
>>>   MetadataAbbrevCode,
>>>   (uint64_t[]){METADATA, VERSION_MAJOR, VERSION_MINOR,
>>> CLANG_VERSION_MAJOR,
>>>
>>
>> Why the cast (uint64_t[])? I'm vaguely surprised that even compiles... ?
>>
>> This is a GNU extension (C99 compound literals in C++). It won't compile
on MSVC.

> I would imagine it'd be passed as an init list, then turned into an
>> ArrayRef from there... but I guess not?
>>
>>
>>
>> Might be more clear with the callee:
>>
>>   template 
>>   void EmitRecordWithBlob(unsigned Abbrev, const Container ,
>>   StringRef Blob) {
>> EmitRecordWithAbbrevImpl(Abbrev, makeArrayRef(Vals), Blob, None);
>>   }
>>
>> The template can’t be deduced without the cast.
>>
>
> Yeah, curious though. Another hole in perfect forwarding I suppose (not
> that this ^ is perfect forwarding, but even with perfect forwarding it
> doesn't cope well)
>
> Yeah, the cast is rather unfortunate. Hrm.
>
> Ah well - probably just as good to leave it as-is for now. If someone has
> a flash of inspiration later & figures out a way to make it better, so be
> it.
>

The way to handle this is to add another EmitRecord overload that takes a
std::initializer_list.


>> —
>> Mehdi
>>
>>
>>
>>
>>
>>
>>
>>
>>>CLANG_VERSION_MINOR, !isysroot.empty(),
>>> IncludeTimestamps,
>>>ASTHasCompilerErrors},
>>>   getClangFullRepositoryVersion());
>>>
>>> Thanks,
>>>
>>> —
>>> Mehdi
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> - Dave
>>>
>>>

 From: Mehdi Amini 

 Modified:
 cfe/trunk/include/clang/Serialization/ASTWriter.h
 cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp
 cfe/trunk/lib/Serialization/ASTWriter.cpp
 cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp

 Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=247233=247232=247233=diff

 ==
 --- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
 +++ cfe/trunk/include/clang/Serialization/ASTWriter.h Wed Sep  9
 20:46:39 2015
 @@ -84,6 +84,7 @@ class ASTWriter : public ASTDeserializat
  public:
typedef SmallVector RecordData;
typedef SmallVectorImpl RecordDataImpl;
 +  typedef ArrayRef RecordDataRef;

friend class ASTDeclWriter;
friend class ASTStmtWriter;
 @@ -756,7 +757,7 @@ public:
void AddPath(StringRef Path, RecordDataImpl );

/// \brief Emit the current record with the given path as a blob.
 -  void EmitRecordWithPath(unsigned Abbrev, RecordDataImpl ,
 +  void EmitRecordWithPath(unsigned Abbrev, RecordDataRef Record,
StringRef Path);

/// \brief Add a version tuple to 

Re: Preventing several replacements on a macro call.

[Argyrios Kyrtzidis via cfe-commits]

Hi Angel,

This part of the code is conservative because it is not clear if accepting the 
change in all the different places where the macro argument is expanded is 
acceptable or not.
For a contrived example, let’s say you have this macro definition:

#define MY_MAC(x) foo(x) + bar(x)

and you use it like this:

int a = MY_MAC(b);

which expands to this:

int a = foo(b) + bar(b);

Now suppose you want to find all places where “foo(b)” is used and change it to 
“foo(b.c)”. You walk the AST and find “foo(b)” from the macro expansion and you 
change ‘b’ to ‘b.c’. This change will apply to the macro argument:

int a = MY_MAC(b.c);

But this now expands like this:

int a = foo(b.c) + bar(b.c)

And you unintentionally also changed the ‘bar’ call.


Now, ideally we would keep track of all such changes and if you tried to change 
the ‘b’ in both “foo(b)” and “bar(b)” we would automatically accept it; but 
this is a bit complicated.
In the meantime we could add a ‘knob' to control this behavior. We could have a 
field in Commit object that you set to true to indicate that it is ok to accept 
a change in a macro argument that expands in multiple places, and also for 
convenience add such a knob to EditedSource object for accepting in all commits 
without needing to set it for each commit separately.

What do you think ?

> On Sep 9, 2015, at 6:05 AM, Angel Garcia via cfe-commits 
>  wrote:
> 
> +cfe-commits
> 
> On Tue, Sep 8, 2015 at 6:56 PM, Angel Garcia  > wrote:
> Hi Ted,
> 
> I was working on a clang-tidy check, and today I discovered that it was 
> unable to do several replacements in different arguments of the same macro 
> call. At first, I thought it was a bug, and trying to figure out why this was 
> happening, I found that the replacements were rejected in 
> lib/Edit/EditedSource.cpp:46, where there is a comment that says "Trying to 
> write in a macro argument input that has already been written for another 
> argument of the same macro". This comment means that this changes are 
> rejected on purpose.
> 
> At the commit history, I saw that you had commited 
>  this code (that's why I am asking you). Do 
> you think that there is a way around this? I don't really understand why 
> there is a particular case for the macros here, and understanding it would 
> help me to decide whether I should give up on trying to make this work, or 
> try to find a "better" solution.
> 
> Thanks and sorry for the inconvenience,
> Angel
> 
> ___
> 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: [PATCH] D12785: Document __builtin_nontemporal_load and __builtin_nontemporal_store.

[Richard Smith via cfe-commits]

rsmith added inline comments.


Comment at: docs/LanguageExtensions.rst:1802-1807
@@ +1801,8 @@
+
+For example, on AArch64 in the following code::
+
+  LDR X1, [X2]
+  LDNP X3, X4, [X1]
+
+the ``LDNP`` might be executed before the ``LDR``. In this case the load would
+be performed from a wrong address (see 6.3.8 in `Programmer's Guide for ARMv8-A

This seems to make the feature essentially useless, since you cannot guarantee 
that the address register is set up sufficiently far before the non-temporal 
load. Should the compiler not be required to insert the necessary barrier 
itself in this case?


http://reviews.llvm.org/D12785



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

Re: [PATCH] D12695: [Driver] Use UniversalCRT on Windows if available

[Reid Kleckner via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL247362: [Driver] Use UniversalCRT on Windows if available 
(authored by rnk).

Changed prior to commit:
  http://reviews.llvm.org/D12695?vs=34475=34515#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D12695

Files:
  cfe/trunk/lib/Driver/MSVCToolChain.cpp
  cfe/trunk/lib/Driver/ToolChains.h
  cfe/trunk/lib/Driver/Tools.cpp

Index: cfe/trunk/lib/Driver/MSVCToolChain.cpp
===
--- cfe/trunk/lib/Driver/MSVCToolChain.cpp
+++ cfe/trunk/lib/Driver/MSVCToolChain.cpp
@@ -205,6 +205,21 @@
 #endif // USE_WIN32
 }
 
+// Convert LLVM's ArchType
+// to the corresponding name of Windows SDK libraries subfolder
+static StringRef getWindowsSDKArch(llvm::Triple::ArchType Arch) {
+  switch (Arch) {
+  case llvm::Triple::x86:
+return "x86";
+  case llvm::Triple::x86_64:
+return "x64";
+  case llvm::Triple::arm:
+return "arm";
+  default:
+return "";
+  }
+}
+
 /// \brief Get Windows SDK installation directory.
 bool MSVCToolChain::getWindowsSDKDir(std::string , int ,
  int ) const {
@@ -263,26 +278,75 @@
 if (!found)
   return false;
 
-llvm::sys::path::append(libPath, "um");
-switch (getArch()) {
-case llvm::Triple::x86:
-  llvm::sys::path::append(libPath, "x86");
-  break;
-case llvm::Triple::x86_64:
-  llvm::sys::path::append(libPath, "x64");
-  break;
-case llvm::Triple::arm:
-  llvm::sys::path::append(libPath, "arm");
-  break;
-default:
+const StringRef archName = getWindowsSDKArch(getArch());
+if (archName.empty())
   return false;
-}
+llvm::sys::path::append(libPath, "um", archName);
   }
 
   path = libPath.str();
   return true;
 }
 
+// Check if the Include path of a specified version of Visual Studio contains
+// specific header files. If not, they are probably shipped with Universal CRT.
+bool clang::driver::toolchains::MSVCToolChain::useUniversalCRT(
+std::string ) const {
+  llvm::SmallString<128> TestPath(VisualStudioDir);
+  llvm::sys::path::append(TestPath, "VC\\include\\stdlib.h");
+
+  return !llvm::sys::fs::exists(TestPath);
+}
+
+bool MSVCToolChain::getUniversalCRTSdkDir(std::string ,
+  std::string ) const {
+  // vcvarsqueryregistry.bat for Visual Studio 2015 queries the registry
+  // for the specific key "KitsRoot10". So do we.
+  if (!getSystemRegistryString(
+  "SOFTWARE\\Microsoft\\Windows Kits\\Installed Roots", "KitsRoot10",
+  Path, nullptr))
+return false;
+
+  UCRTVersion.clear();
+
+  // Find the most recent version of Universal CRT.
+  // vcvarsqueryregistry.bat sorts entries in the include directory by names and
+  // uses the last one of the list.
+  // So we compare entry names lexicographically to find the greatest one.
+  std::error_code EC;
+  llvm::SmallString<128> IncludePath(Path);
+  llvm::sys::path::append(IncludePath, "Include");
+  for (llvm::sys::fs::directory_iterator DirIt(IncludePath, EC), DirEnd;
+   DirIt != DirEnd && !EC; DirIt.increment(EC)) {
+if (!llvm::sys::fs::is_directory(DirIt->path()))
+  continue;
+StringRef CandidateName = llvm::sys::path::filename(DirIt->path());
+if (CandidateName > UCRTVersion)
+  UCRTVersion = CandidateName;
+  }
+
+  return !UCRTVersion.empty();
+}
+
+bool MSVCToolChain::getUniversalCRTLibraryPath(std::string ) const {
+  std::string UniversalCRTSdkPath;
+  std::string UCRTVersion;
+
+  Path.clear();
+  if (!getUniversalCRTSdkDir(UniversalCRTSdkPath, UCRTVersion))
+return false;
+
+  StringRef ArchName = getWindowsSDKArch(getArch());
+  if (ArchName.empty())
+return false;
+
+  llvm::SmallString<128> LibPath(UniversalCRTSdkPath);
+  llvm::sys::path::append(LibPath, "Lib", UCRTVersion, "ucrt", ArchName);
+
+  Path = LibPath.str();
+  return true;
+}
+
 // Get the location to use for Visual Studio binaries.  The location priority
 // is: %VCINSTALLDIR% > %PATH% > newest copy of Visual Studio installed on
 // system (as reported by the registry).
@@ -460,6 +524,17 @@
   if (getVisualStudioInstallDir(VSDir)) {
 AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, VSDir, "VC\\include");
 
+if (useUniversalCRT(VSDir)) {
+  std::string UniversalCRTSdkPath;
+  std::string UCRTVersion;
+  if (getUniversalCRTSdkDir(UniversalCRTSdkPath, UCRTVersion)) {
+llvm::SmallString<128> UCRTIncludePath(UniversalCRTSdkPath);
+llvm::sys::path::append(UCRTIncludePath, "Include", UCRTVersion,
+"ucrt");
+addSystemInclude(DriverArgs, CC1Args, UCRTIncludePath);
+  }
+}
+
 std::string WindowsSDKDir;
 int major, minor;
 if (getWindowsSDKDir(WindowsSDKDir, major, minor)) {
Index: cfe/trunk/lib/Driver/ToolChains.h
===
--- 

r247362 - [Driver] Use UniversalCRT on Windows if available

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Thu Sep 10 19:09:39 2015
New Revision: 247362

URL: http://llvm.org/viewvc/llvm-project?rev=247362=rev
Log:
[Driver] Use UniversalCRT on Windows if available

Summary:
With Visual Studio 2015 release, a part of runtime library was extracted
and now comes with Windows Kits. This patch enables clang to use
Universal CRT library if  %INCLUDE or %LIB environment varaibles are not
specified.

See also https://llvm.org/bugs/show_bug.cgi?id=24741

Patch by Igor Kudrin

Reviewers: zturner, hans, rnk

Subscribers: ruiu, cfe-commits

Differential Revision: http://reviews.llvm.org/D12695

Modified:
cfe/trunk/lib/Driver/MSVCToolChain.cpp
cfe/trunk/lib/Driver/ToolChains.h
cfe/trunk/lib/Driver/Tools.cpp

Modified: cfe/trunk/lib/Driver/MSVCToolChain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/MSVCToolChain.cpp?rev=247362=247361=247362=diff
==
--- cfe/trunk/lib/Driver/MSVCToolChain.cpp (original)
+++ cfe/trunk/lib/Driver/MSVCToolChain.cpp Thu Sep 10 19:09:39 2015
@@ -205,6 +205,21 @@ static bool getSystemRegistryString(cons
 #endif // USE_WIN32
 }
 
+// Convert LLVM's ArchType
+// to the corresponding name of Windows SDK libraries subfolder
+static StringRef getWindowsSDKArch(llvm::Triple::ArchType Arch) {
+  switch (Arch) {
+  case llvm::Triple::x86:
+return "x86";
+  case llvm::Triple::x86_64:
+return "x64";
+  case llvm::Triple::arm:
+return "arm";
+  default:
+return "";
+  }
+}
+
 /// \brief Get Windows SDK installation directory.
 bool MSVCToolChain::getWindowsSDKDir(std::string , int ,
  int ) const {
@@ -263,26 +278,75 @@ bool MSVCToolChain::getWindowsSDKLibrary
 if (!found)
   return false;
 
-llvm::sys::path::append(libPath, "um");
-switch (getArch()) {
-case llvm::Triple::x86:
-  llvm::sys::path::append(libPath, "x86");
-  break;
-case llvm::Triple::x86_64:
-  llvm::sys::path::append(libPath, "x64");
-  break;
-case llvm::Triple::arm:
-  llvm::sys::path::append(libPath, "arm");
-  break;
-default:
+const StringRef archName = getWindowsSDKArch(getArch());
+if (archName.empty())
   return false;
-}
+llvm::sys::path::append(libPath, "um", archName);
   }
 
   path = libPath.str();
   return true;
 }
 
+// Check if the Include path of a specified version of Visual Studio contains
+// specific header files. If not, they are probably shipped with Universal CRT.
+bool clang::driver::toolchains::MSVCToolChain::useUniversalCRT(
+std::string ) const {
+  llvm::SmallString<128> TestPath(VisualStudioDir);
+  llvm::sys::path::append(TestPath, "VC\\include\\stdlib.h");
+
+  return !llvm::sys::fs::exists(TestPath);
+}
+
+bool MSVCToolChain::getUniversalCRTSdkDir(std::string ,
+  std::string ) const {
+  // vcvarsqueryregistry.bat for Visual Studio 2015 queries the registry
+  // for the specific key "KitsRoot10". So do we.
+  if (!getSystemRegistryString(
+  "SOFTWARE\\Microsoft\\Windows Kits\\Installed Roots", "KitsRoot10",
+  Path, nullptr))
+return false;
+
+  UCRTVersion.clear();
+
+  // Find the most recent version of Universal CRT.
+  // vcvarsqueryregistry.bat sorts entries in the include directory by names 
and
+  // uses the last one of the list.
+  // So we compare entry names lexicographically to find the greatest one.
+  std::error_code EC;
+  llvm::SmallString<128> IncludePath(Path);
+  llvm::sys::path::append(IncludePath, "Include");
+  for (llvm::sys::fs::directory_iterator DirIt(IncludePath, EC), DirEnd;
+   DirIt != DirEnd && !EC; DirIt.increment(EC)) {
+if (!llvm::sys::fs::is_directory(DirIt->path()))
+  continue;
+StringRef CandidateName = llvm::sys::path::filename(DirIt->path());
+if (CandidateName > UCRTVersion)
+  UCRTVersion = CandidateName;
+  }
+
+  return !UCRTVersion.empty();
+}
+
+bool MSVCToolChain::getUniversalCRTLibraryPath(std::string ) const {
+  std::string UniversalCRTSdkPath;
+  std::string UCRTVersion;
+
+  Path.clear();
+  if (!getUniversalCRTSdkDir(UniversalCRTSdkPath, UCRTVersion))
+return false;
+
+  StringRef ArchName = getWindowsSDKArch(getArch());
+  if (ArchName.empty())
+return false;
+
+  llvm::SmallString<128> LibPath(UniversalCRTSdkPath);
+  llvm::sys::path::append(LibPath, "Lib", UCRTVersion, "ucrt", ArchName);
+
+  Path = LibPath.str();
+  return true;
+}
+
 // Get the location to use for Visual Studio binaries.  The location priority
 // is: %VCINSTALLDIR% > %PATH% > newest copy of Visual Studio installed on
 // system (as reported by the registry).
@@ -460,6 +524,17 @@ void MSVCToolChain::AddClangSystemInclud
   if (getVisualStudioInstallDir(VSDir)) {
 AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, VSDir, "VC\\include");
 
+if (useUniversalCRT(VSDir)) {
+  std::string UniversalCRTSdkPath;
+  std::string 

Re: [PATCH] D11737: Add -linker (and -linker=) alias for -fuse-ld=

[Filipe Cabecinhas via cfe-commits]

filcab abandoned this revision.
filcab added a comment.

We don't need this anymore, since we're making some internal changes.
Thank you for the feedback!



Comment at: include/clang/Driver/Options.td:1853-1854
@@ -1853,1 +1852,4 @@
+def fuse_ld_EQ : Joined<["-"], "fuse-ld=">, HelpText<"Use linker ">, 
Group;
+def linker : Separate<["-"], "linker">, Alias, 
MetaVarName<"">;
+def linker_EQ : Joined<["-"], "linker=">, Alias;
 

echristo wrote:
> Any reason to have both options?
I went and double-checked our docs. We should only need one.


Comment at: include/clang/Driver/Options.td:1853
@@ -1853,1 +1852,3 @@
+def fuse_ld_EQ : Joined<["-"], "fuse-ld=">, HelpText<"Use linker ">, 
Group;
+def linker_EQ : Joined<["-"], "linker=">, Alias, 
MetaVarName<"">;
 

echristo wrote:
> thakis wrote:
> > Any reason to have another alias for this at all? Does gcc understand this 
> > spelling? If not, could you limit this flag to PS4 targets? (I'm guessing 
> > you need it for PS4 targets for some reason.)
> Any reason? (And I'm not sure how to limit it btw on target).
Same here, no idea how to limit the flag to a specific target.


Comment at: test/Driver/fuse-ld.c:27-33
@@ -26,1 +26,9 @@
 
+// -linker= is an alias to fuse-ld. Don't need to retry all combinations
+// RUN: %clang %s -### -linker gold \
+// RUN: --sysroot=%S/Inputs/basic_freebsd_tree \
+// RUN: -target x86_64-unknown-freebsd \
+// RUN: -B%S/Inputs/basic_freebsd_tree/usr/bin 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=CHECK-FREEBSD-GOLD
+// CHECK-FREEBSD-LINKER-GOLD: Inputs/basic_freebsd_tree/usr/bin{{/|\\+}}ld.gold
+

echristo wrote:
> Bet this doesn't work :)
Derp. Sorry.


Comment at: test/Driver/fuse-ld.c:32-33
@@ +31,4 @@
+// RUN: -B%S/Inputs/basic_freebsd_tree/usr/bin 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=CHECK-FREEBSD-GOLD
+// CHECK-FREEBSD-LINKER-GOLD: Inputs/basic_freebsd_tree/usr/bin{{/|\\+}}ld.gold
+

pcc wrote:
> Prefix mismatch here.
Ah yes. Thanks!


http://reviews.llvm.org/D11737



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

Re: [PATCH] D12652: [Static Analyzer] Lambda support.

[Gábor Horváth via cfe-commits]

xazax.hun updated this revision to Diff 34498.
xazax.hun added a comment.

- Updated to latest trunk
- Added test for the defensive inline check heuristic case
- Added test for diagnostic within a lambda body
- Added test for path notes


http://reviews.llvm.org/D12652

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/MemRegion.cpp
  test/Analysis/dead-stores.cpp
  test/Analysis/lambda-notes.cpp
  test/Analysis/lambdas.cpp
  test/Analysis/temporaries.cpp

Index: test/Analysis/temporaries.cpp
===
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -299,13 +299,7 @@
   void testRecursiveFramesStart() { testRecursiveFrames(false); }
 
   void testLambdas() {
-// This is the test we would like to write:
-// []() { check(NoReturnDtor()); } != nullptr || check(Dtor());
-// But currently the analyzer stops when it encounters a lambda:
-[] {};
-// The CFG for this now looks correct, but we still do not reach the line
-// below.
-clang_analyzer_warnIfReached(); // FIXME: Should warn.
+[]() { check(NoReturnDtor()); } != nullptr || check(Dtor());
   }
 
   void testGnuExpressionStatements(int v) {
Index: test/Analysis/lambdas.cpp
===
--- test/Analysis/lambdas.cpp
+++ test/Analysis/lambdas.cpp
@@ -1,9 +1,181 @@
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -analyze -analyzer-checker=debug.DumpCFG %s > %t 2>&1
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config inline-lambdas=true -verify %s 
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -analyze -analyzer-checker=core,debug.DumpCFG -analyzer-config inline-lambdas=true %s > %t 2>&1
 // RUN: FileCheck --input-file=%t %s
 
+void clang_analyzer_warnIfReached();
+void clang_analyzer_eval(int);
+
 struct X { X(const X&); };
 void f(X x) { (void) [x]{}; }
 
+
+// Lambda semantics tests.
+
+void basicCapture() {
+  int i = 5;
+  [i]() mutable {
+// clang_analyzer_eval does nothing in inlined functions.
+if (i != 5)
+  clang_analyzer_warnIfReached();
+++i;
+  }();
+  [] {
+if (i != 5)
+  clang_analyzer_warnIfReached();
+  }();
+  [] {
+if (i != 5)
+  clang_analyzer_warnIfReached();
+i++;
+  }();
+  clang_analyzer_eval(i == 6); // expected-warning{{TRUE}}
+}
+
+void deferredLambdaCall() {
+  int i = 5;
+  auto l1 = [i]() mutable {
+if (i != 5)
+  clang_analyzer_warnIfReached();
+++i;
+  };
+  auto l2 = [] {
+if (i != 5)
+  clang_analyzer_warnIfReached();
+  };
+  auto l3 = [] {
+if (i != 5)
+  clang_analyzer_warnIfReached();
+i++;
+  };
+  l1();
+  l2();
+  l3();
+  clang_analyzer_eval(i == 6); // expected-warning{{TRUE}}
+}
+
+void multipleCaptures() {
+  int i = 5, j = 5;
+  [i, ]() mutable {
+if (i != 5 && j != 5)
+  clang_analyzer_warnIfReached();
+++i;
+++j;
+  }();
+  clang_analyzer_eval(i == 5); // expected-warning{{TRUE}}
+  clang_analyzer_eval(j == 6); // expected-warning{{TRUE}}
+  [=]() mutable {
+if (i != 5 && j != 6)
+  clang_analyzer_warnIfReached();
+++i;
+++j;
+  }();
+  clang_analyzer_eval(i == 5); // expected-warning{{TRUE}}
+  clang_analyzer_eval(j == 6); // expected-warning{{TRUE}}
+  [&]() mutable {
+if (i != 5 && j != 6)
+  clang_analyzer_warnIfReached();
+++i;
+++j;
+  }();
+  clang_analyzer_eval(i == 6); // expected-warning{{TRUE}}
+  clang_analyzer_eval(j == 7); // expected-warning{{TRUE}}
+}
+
+void testReturnValue() {
+  int i = 5;
+  auto l = [i] (int a) {
+return i + a;
+  };
+  int b = l(3);
+  clang_analyzer_eval(b == 8); // expected-warning{{TRUE}}
+}
+
+// Nested lambdas.
+
+void testNestedLambdas() {
+  int i = 5;
+  auto l = [i]() mutable {
+[]() {
+  ++i;
+}();
+if (i != 6)
+  clang_analyzer_warnIfReached();
+  };
+  l();
+  clang_analyzer_eval(i == 5); // expected-warning{{TRUE}}
+}
+
+// Captured this.
+
+class RandomClass {
+  int i;
+
+  void captureFields() {
+i = 5;
+[this]() {
+  // clang_analyzer_eval does nothing in inlined functions.
+  if (i != 5)
+clang_analyzer_warnIfReached();
+  ++i;
+}();
+clang_analyzer_eval(i == 6); // expected-warning{{TRUE}}
+  }
+};
+
+
+// Nested this capture.
+
+class RandomClass2 {
+  int i;
+
+  void captureFields() {
+i = 5;
+[this]() {
+  // clang_analyzer_eval does nothing in inlined functions.
+  if (i != 5)
+clang_analyzer_warnIfReached();
+  ++i;
+  [this]() {
+// clang_analyzer_eval does nothing in inlined functions.
+if (i != 6)
+  clang_analyzer_warnIfReached();
+++i;
+  }();
+}();
+   

Re: [PATCH] D12087: always_inline codegen rewrite

[Evgeniy Stepanov via cfe-commits]

eugenis added a comment.

I'm going to commit this tomorrow unless someone speaks up.


Repository:
  rL LLVM

http://reviews.llvm.org/D12087



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

Re: [PATCH] D12785: Document __builtin_nontemporal_load and __builtin_nontemporal_store.

[Michael Zolotukhin via cfe-commits]

mzolotukhin added inline comments.


Comment at: docs/LanguageExtensions.rst:1802-1807
@@ +1801,8 @@
+
+For example, on AArch64 in the following code::
+
+  LDR X1, [X2]
+  LDNP X3, X4, [X1]
+
+the ``LDNP`` might be executed before the ``LDR``. In this case the load would
+be performed from a wrong address (see 6.3.8 in `Programmer's Guide for ARMv8-A

rsmith wrote:
> This seems to make the feature essentially useless, since you cannot 
> guarantee that the address register is set up sufficiently far before the 
> non-temporal load. Should the compiler not be required to insert the 
> necessary barrier itself in this case?
Yes, we can require targets to only use corresponding NT instructions when it's 
safe, and then remove this remark from the documentation. For ARM64 that would 
mean either not to emit LDNP at all, or conservatively emit barriers before 
each LDNP (which probably removes all performance benefits of using it) - that 
is, yes, non-temporal loads would be useless on this target.

But I think we want to keep the builtin for NT-load, as it's a generic feature, 
not ARM64 specific. It can be used on other targets - e.g. we can use this in 
x86 stream builtins, and hopefully simplify their current implementation. I 
don't know about non-temporal operations on other targets, but if there are 
others, they can use it too right out of the box.


http://reviews.llvm.org/D12785



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

Re: r247369 - Module Debugging: Emit forward declarations for types that are defined in

[David Blaikie via cfe-commits]

On Thu, Sep 10, 2015 at 6:03 PM, Adrian Prantl via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: adrian
> Date: Thu Sep 10 20:03:26 2015
> New Revision: 247369
>
> URL: http://llvm.org/viewvc/llvm-project?rev=247369=rev
> Log:
> Module Debugging: Emit forward declarations for types that are defined in
> clang modules, if -dwarf-ext-refs (DebugTypesExtRefs) is specified.
>

This change seems to have a lot more code in it than I was expecting...

I was rather expecting something a lot like the flimit-debug-info support.
Specifically, I would've expected one more conditional added to
CGDebugInfo::shouldOmitDefinition.

Why the extra complexity?

I guess part of it is to be able to omit definitions of things other than
record types - is there much value in that? (especially typedefs - it seems
like a typedef is too small to benefit from a declaration (even if we could
emit one)?)


>
> Added:
> cfe/trunk/test/Modules/ExtDebugInfo.cpp
> cfe/trunk/test/Modules/ExtDebugInfo.m
> Modified:
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> cfe/trunk/lib/CodeGen/CGDebugInfo.h
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=247369=247368=247369=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Sep 10 20:03:26 2015
> @@ -148,7 +148,9 @@ void CGDebugInfo::setLocation(SourceLoca
>  }
>
>  llvm::DIScope *CGDebugInfo::getDeclContextDescriptor(const Decl *D) {
> -  return getContextDescriptor(cast(D->getDeclContext()), TheCU);
> +  llvm::DIScope *Mod = getParentModuleOrNull(D);
> +  return getContextDescriptor(cast(D->getDeclContext()),
> +  Mod ? Mod : TheCU);
>  }
>
>  llvm::DIScope *CGDebugInfo::getContextDescriptor(const Decl *Context,
> @@ -1448,6 +1450,9 @@ void CGDebugInfo::completeRequiredType(c
>  if (CXXDecl->isDynamicClass())
>return;
>
> +  if (DebugTypeExtRefs && RD->isFromASTFile())
> +return;
> +
>QualType Ty = CGM.getContext().getRecordType(RD);
>llvm::DIType *T = getTypeOrNull(Ty);
>if (T && T->isForwardDecl())
> @@ -1669,9 +1674,9 @@ CGDebugInfo::getOrCreateModuleRef(Extern
>TheCU->getSourceLanguage(), internString(Mod.ModuleName),
>internString(Mod.Path), TheCU->getProducer(), true, StringRef(), 0,
>internString(Mod.ASTFile), llvm::DIBuilder::FullDebug,
> Mod.Signature);
> -  llvm::DIModule *M =
> -  DIB.createModule(CU, Mod.ModuleName, ConfigMacros,
> internString(Mod.Path),
> -   internString(CGM.getHeaderSearchOpts().Sysroot));
> +  llvm::DIModule *M = DIB.createModule(
> +  CU, Mod.ModuleName, ConfigMacros, internString(Mod.Path),
> +  internString(CGM.getHeaderSearchOpts().Sysroot));
>DIB.finalize();
>ModRef.reset(M);
>return M;
> @@ -2081,9 +2086,16 @@ llvm::DIType *CGDebugInfo::getOrCreateTy
>if (auto *T = getTypeOrNull(Ty))
>  return T;
>
> +  llvm::DIType *Res = nullptr;
> +  if (DebugTypeExtRefs)
> +// Make a forward declaration of an external type.
> +Res = getTypeExtRefOrNull(Ty, Unit);
> +
>// Otherwise create the type.
> -  llvm::DIType *Res = CreateTypeNode(Ty, Unit);
> -  void *TyPtr = Ty.getAsOpaquePtr();
> +  if (!Res)
> +Res = CreateTypeNode(Ty, Unit);
> +
> +  void* TyPtr = Ty.getAsOpaquePtr();
>
>// And update the type cache.
>TypeCache[TyPtr].reset(Res);
> @@ -2115,6 +2127,123 @@ ObjCInterfaceDecl *CGDebugInfo::getObjCI
>}
>  }
>
> +llvm::DIModule *CGDebugInfo::getParentModuleOrNull(const Decl *D) {
> +  if (!DebugTypeExtRefs || !D || !D->isFromASTFile())
> +return nullptr;
> +
> +  llvm::DIModule *ModuleRef = nullptr;
> +  auto *Reader = CGM.getContext().getExternalSource();
> +  auto Idx = D->getOwningModuleID();
> +  auto Info = Reader->getSourceDescriptor(Idx);
> +  if (Info)
> +ModuleRef = getOrCreateModuleRef(*Info);
> +  return ModuleRef;
> +}
> +
> +llvm::DIType *CGDebugInfo::getTypeExtRefOrNull(QualType Ty, llvm::DIFile
> *F,
> +   bool Anchored) {
> +  assert(DebugTypeExtRefs && "module debugging only");
> +  Decl *TyDecl = nullptr;
> +  StringRef Name;
> +  SmallString<256> UID;
> +  unsigned Tag = 0;
> +
> +  // Handle all types that have a declaration.
> +  switch (Ty->getTypeClass()) {
> +  case Type::Typedef: {
> +TyDecl = cast(Ty)->getDecl();
> +if (!TyDecl->isFromASTFile())
> +  return nullptr;
> +
> +// A typedef will anchor a type in the module.
> +if (auto *TD = dyn_cast(TyDecl)) {
> +  // This is a working around the fact that LLVM does not allow
> +  // typedefs to be forward declarations.
> +  QualType Ty = TD->getUnderlyingType();
> +  Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());
> +  if (auto *AnchoredTy = getTypeExtRefOrNull(Ty, F,
> /*Anchored=*/true)) {

Re: [PATCH] D11279: Initial patch for PS4 toolchain

[Filipe Cabecinhas via cfe-commits]

filcab updated this revision to Diff 34522.
filcab marked 12 inline comments as done.
filcab added a comment.
This revision is now accepted and ready to land.

Addressed review comments.


http://reviews.llvm.org/D11279

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/DiagnosticGroups.td
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp
  lib/Driver/Tools.h
  lib/Frontend/InitHeaderSearch.cpp
  test/Driver/Inputs/scei-ps4_tree/target/include/.keep
  test/Driver/Inputs/scei-ps4_tree/target/include_common/.keep
  test/Driver/debug-options.c
  test/Driver/ps4-header-search.c
  test/Driver/ps4-linker-non-win.c
  test/Driver/ps4-linker-win.c
  test/Driver/ps4-pic.c
  test/Driver/ps4-sdk-root.c
  test/Driver/rtti-options.cpp
  test/Driver/stack-protector.c

Index: test/Driver/stack-protector.c
===
--- test/Driver/stack-protector.c
+++ test/Driver/stack-protector.c
@@ -23,3 +23,12 @@
 // RUN: %clang -fstack-protector-all -### %s 2>&1 | FileCheck %s -check-prefix=SSP-ALL
 // SSP-ALL: "-stack-protector" "3"
 // SSP-ALL-NOT: "-stack-protector-buffer-size" 
+
+// RUN: %clang -target x86_64-scei-ps4 -### %s 2>&1 | FileCheck %s -check-prefix=SSP-PS4
+// RUN: %clang -target x86_64-scei-ps4 -fstack-protector -### %s 2>&1 | FileCheck %s -check-prefix=SSP-PS4
+// SSP-PS4: "-stack-protector" "2"
+// SSP-PS4-NOT: "-stack-protector-buffer-size"
+
+// RUN: %clang -target x86_64-scei-ps4 -fstack-protector --param ssp-buffer-size=16 -### %s 2>&1 | FileCheck %s -check-prefix=SSP-PS4-BUF
+// SSP-PS4-BUF: "-stack-protector" "2"
+// SSP-PS4-BUF: "-stack-protector-buffer-size" "16"
Index: test/Driver/rtti-options.cpp
===
--- test/Driver/rtti-options.cpp
+++ test/Driver/rtti-options.cpp
@@ -34,7 +34,8 @@
 // RUN: %clang -x c++ -### -c -target x86_64-unknown-unknown -fexceptions %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 
 // -frtti + exceptions
-// RUN: %clang -### -c -target x86_64-scei-ps4 -fcxx-exceptions -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
+// ps4 needs -nostdinc due to having warnings if the SDK is not installed
+// RUN: %clang -### -c -nostdinc -target x86_64-scei-ps4 -fcxx-exceptions -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fcxx-exceptions -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 
 // -f{no-,}rtti/default
Index: test/Driver/ps4-sdk-root.c
===
--- /dev/null
+++ test/Driver/ps4-sdk-root.c
@@ -0,0 +1,48 @@
+// REQUIRES: x86-registered-target
+
+// Check that ps4-clang doesn't report a warning message when locating
+// system header files (either by looking at the value of SCE_PS4_SDK_DIR
+// or relative to the location of the compiler driver), if "-nostdinc",
+// "--sysroot" or "-isysroot" option is specified on the command line.
+// Otherwise, check that ps4-clang reports a warning.
+
+// Check that clang doesn't report a warning message when locating
+// system libraries (either by looking at the value of SCE_PS4_SDK_DIR
+// or relative to the location of the compiler driver), if "-c", "-S", "-E",
+// "--sysroot", "-nostdlib" or "-nodefaultlibs" option is specified on
+// the command line.
+// Otherwise, check that ps4-clang reports a warning.
+
+// setting up SCE_PS4_SDK_DIR to existing location, which is not a PS4 SDK.
+// RUN: env SCE_PS4_SDK_DIR=.. %clang -### -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=WARN-SYS-LIBS -check-prefix=NO-WARN %s
+
+// RUN: env SCE_PS4_SDK_DIR=.. %clang -### -c -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=NO-WARN %s
+// RUN: env SCE_PS4_SDK_DIR=.. %clang -### -S -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=NO-WARN %s
+// RUN: env SCE_PS4_SDK_DIR=.. %clang -### -E -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=NO-WARN %s
+// RUN: env SCE_PS4_SDK_DIR=.. %clang -### -emit-ast -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=NO-WARN %s
+// RUN: env SCE_PS4_SDK_DIR=.. %clang -### -isysroot foo -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-ISYSROOT -check-prefix=WARN-SYS-LIBS -check-prefix=NO-WARN %s
+
+// RUN: env SCE_PS4_SDK_DIR=.. %clang -### -c -nostdinc -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=NO-WARN %s
+// RUN: env SCE_PS4_SDK_DIR=.. %clang -### -S -nostdinc -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=NO-WARN %s
+// RUN: env SCE_PS4_SDK_DIR=.. %clang -### -E -nostdinc -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=NO-WARN %s
+// RUN: env SCE_PS4_SDK_DIR=.. %clang -### -emit-ast -nostdinc -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=NO-WARN %s
+

Re: [PATCH] D12785: Document __builtin_nontemporal_load and __builtin_nontemporal_store.

[Mikhail Zolotukhin via cfe-commits]

> On Sep 10, 2015, at 6:29 PM, Richard Smith  wrote:
> 
> On Thu, Sep 10, 2015 at 5:57 PM, Michael Zolotukhin  > wrote:
> mzolotukhin added inline comments.
> 
> 
> Comment at: docs/LanguageExtensions.rst:1802-1807
> @@ +1801,8 @@
> +
> +For example, on AArch64 in the following code::
> +
> +  LDR X1, [X2]
> +  LDNP X3, X4, [X1]
> +
> +the ``LDNP`` might be executed before the ``LDR``. In this case the load 
> would
> +be performed from a wrong address (see 6.3.8 in `Programmer's Guide for 
> ARMv8-A
> 
> rsmith wrote:
> > This seems to make the feature essentially useless, since you cannot 
> > guarantee that the address register is set up sufficiently far before the 
> > non-temporal load. Should the compiler not be required to insert the 
> > necessary barrier itself in this case?
> Yes, we can require targets to only use corresponding NT instructions when 
> it's safe, and then remove this remark from the documentation. For ARM64 that 
> would mean either not to emit LDNP at all, or conservatively emit barriers 
> before each LDNP (which probably removes all performance benefits of using 
> it) - that is, yes, non-temporal loads would be useless on this target.
> 
> I think this should already be the case -- according to the definition of 
> !nontemporal in the LangRef 
> (http://llvm.org/docs/LangRef.html#load-instruction 
> ), using an LDNP without 
> an accompanying barrier would not be correct on AArch64, as it does not have 
> the right semantics.
I removed the paragraph in updated patch.
>  
> But I think we want to keep the builtin for NT-load, as it's a generic 
> feature, not ARM64 specific. It can be used on other targets - e.g. we can 
> use this in x86 stream builtins, and hopefully simplify their current 
> implementation. I don't know about non-temporal operations on other targets, 
> but if there are others, they can use it too right out of the box.
> 
> Yes, I'm not arguing for removing the builtin, just that the AArch64 backend 
> needs to be very careful when mapping it to LDNP, because that will 
> frequently not be correct.
Yes, that's true, and that was the reason why we only implemented STNP for now.

Michael

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

r247374 - Docs: Document __builtin_nontemporal_load and __builtin_nontemporal_store.

[Michael Zolotukhin via cfe-commits]

Author: mzolotukhin
Date: Thu Sep 10 21:01:15 2015
New Revision: 247374

URL: http://llvm.org/viewvc/llvm-project?rev=247374=rev
Log:
Docs: Document __builtin_nontemporal_load and __builtin_nontemporal_store.

Summary:
In r247104 I added the builtins for generating non-temporal memory operations,
but now I realized that they lack documentation. This patch adds some.

Differential Revision: http://reviews.llvm.org/D12785

Modified:
cfe/trunk/docs/LanguageExtensions.rst

Modified: cfe/trunk/docs/LanguageExtensions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=247374=247373=247374=diff
==
--- cfe/trunk/docs/LanguageExtensions.rst (original)
+++ cfe/trunk/docs/LanguageExtensions.rst Thu Sep 10 21:01:15 2015
@@ -1779,6 +1779,26 @@ care should be exercised.
 For these reasons the higher level atomic primitives should be preferred where
 possible.
 
+Non-temporal load/store builtins
+
+
+Clang provides overloaded builtins allowing generation of non-temporal memory
+accesses.
+
+.. code-block:: c
+
+  T __builtin_nontemporal_load(T *addr);
+  void __builtin_nontemporal_store(T value, T *addr);
+
+The types ``T`` currently supported are:
+
+* Integer types.
+* Floating-point types.
+* Vector types.
+
+Note that the compiler does not guarantee that non-temporal loads or stores
+will be used.
+
 Non-standard C++11 Attributes
 =
 


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

Re: [PATCH] D12785: Document __builtin_nontemporal_load and __builtin_nontemporal_store.

[Michael Zolotukhin via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL247374: Docs: Document __builtin_nontemporal_load and 
__builtin_nontemporal_store. (authored by mzolotukhin).

Changed prior to commit:
  http://reviews.llvm.org/D12785?vs=34521=34524#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D12785

Files:
  cfe/trunk/docs/LanguageExtensions.rst

Index: cfe/trunk/docs/LanguageExtensions.rst
===
--- cfe/trunk/docs/LanguageExtensions.rst
+++ cfe/trunk/docs/LanguageExtensions.rst
@@ -1779,6 +1779,26 @@
 For these reasons the higher level atomic primitives should be preferred where
 possible.
 
+Non-temporal load/store builtins
+
+
+Clang provides overloaded builtins allowing generation of non-temporal memory
+accesses.
+
+.. code-block:: c
+
+  T __builtin_nontemporal_load(T *addr);
+  void __builtin_nontemporal_store(T value, T *addr);
+
+The types ``T`` currently supported are:
+
+* Integer types.
+* Floating-point types.
+* Vector types.
+
+Note that the compiler does not guarantee that non-temporal loads or stores
+will be used.
+
 Non-standard C++11 Attributes
 =
 


Index: cfe/trunk/docs/LanguageExtensions.rst
===
--- cfe/trunk/docs/LanguageExtensions.rst
+++ cfe/trunk/docs/LanguageExtensions.rst
@@ -1779,6 +1779,26 @@
 For these reasons the higher level atomic primitives should be preferred where
 possible.
 
+Non-temporal load/store builtins
+
+
+Clang provides overloaded builtins allowing generation of non-temporal memory
+accesses.
+
+.. code-block:: c
+
+  T __builtin_nontemporal_load(T *addr);
+  void __builtin_nontemporal_store(T value, T *addr);
+
+The types ``T`` currently supported are:
+
+* Integer types.
+* Floating-point types.
+* Vector types.
+
+Note that the compiler does not guarantee that non-temporal loads or stores
+will be used.
+
 Non-standard C++11 Attributes
 =
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r247351 - Handle '-r' option properly

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Thu Sep 10 17:31:45 2015
New Revision: 247351

URL: http://llvm.org/viewvc/llvm-project?rev=247351=rev
Log:
Handle '-r' option properly

Summary:
This fixs the bug
https://llvm.org/bugs/show_bug.cgi?id=12587

Patch by Yunlian Jiang

Reviewers: Bigcheese, rnk

Differential Revision: http://reviews.llvm.org/D10279

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/test/Driver/Xlinker-args.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=247351=247350=247351=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Sep 10 17:31:45 2015
@@ -1609,7 +1609,7 @@ def resource_dir_EQ : Joined<["-"], "res
   Alias;
 def rpath : Separate<["-"], "rpath">, Flags<[LinkerInput]>;
 def rtlib_EQ : Joined<["-", "--"], "rtlib=">;
-def r : Flag<["-"], "r">;
+def r : Flag<["-"], "r">, Flags<[LinkerInput,NoArgumentUnused]>;
 def save_temps_EQ : Joined<["-", "--"], "save-temps=">, Flags<[DriverOption]>,
   HelpText<"Save intermediate compilation results.">;
 def save_temps : Flag<["-", "--"], "save-temps">, Flags<[DriverOption]>,

Modified: cfe/trunk/test/Driver/Xlinker-args.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Xlinker-args.c?rev=247351=247350=247351=diff
==
--- cfe/trunk/test/Driver/Xlinker-args.c (original)
+++ cfe/trunk/test/Driver/Xlinker-args.c Thu Sep 10 17:31:45 2015
@@ -3,17 +3,17 @@
 
 // RUN: %clang -target i386-apple-darwin9 -### \
 // RUN:   -Xlinker one -Xlinker --no-demangle \
-// RUN:   -Wl,two,--no-demangle,three -Xlinker four -z five %s 2> %t
+// RUN:   -Wl,two,--no-demangle,three -Xlinker four -z five -r %s 2> %t
 // RUN: FileCheck -check-prefix=DARWIN < %t %s
 //
 // RUN: %clang -target x86_64-pc-linux-gnu -### \
 // RUN:   -Xlinker one -Xlinker --no-demangle \
-// RUN:   -Wl,two,--no-demangle,three -Xlinker four -z five %s 2> %t
+// RUN:   -Wl,two,--no-demangle,three -Xlinker four -z five -r %s 2> %t
 // RUN: FileCheck -check-prefix=LINUX < %t %s
 //
 // DARWIN-NOT: --no-demangle
-// DARWIN: "one" "two" "three" "four" "-z" "five"
-// LINUX: "--no-demangle" "one" "two" "three" "four" "-z" "five"
+// DARWIN: "one" "two" "three" "four" "-z" "five" "-r"
+// LINUX: "--no-demangle" "one" "two" "three" "four" "-z" "five" "-r"
 
 // Check that we forward '-Xlinker' and '-Wl,' on Windows.
 // RUN: %clang -target i686-pc-win32 -### \


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

Re: [PATCH] D12785: Document __builtin_nontemporal_load and __builtin_nontemporal_store.

[Richard Smith via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

LGTM



Comment at: docs/LanguageExtensions.rst:1784
@@ +1783,3 @@
+
+Clang provides overloaded builtins allowing to generate non-temporal memory
+accesses.

to generate -> generation of


Comment at: docs/LanguageExtensions.rst:1799
@@ +1798,3 @@
+Note that the compiler does not guarantee that non-temporal loads or stores
+would be used.
+

would -> will


http://reviews.llvm.org/D12785



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

Re: [PATCH] D12402: PR24595: clang-cl fails to compile vswriter.h header from Windows SDK 8.1 in 32 bit mode

[Reid Kleckner via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm, thanks!



Comment at: test/CodeGenCXX/ctor-dtor-alias.cpp:170
@@ -169,3 +169,3 @@
   // it has a different calling conversion.
-  // CHECK4: call void @_ZN5test93barD2Ev
+  // CHECK4: call void @_ZN6test103barD2Ev
   bar ptr;

Isn't this still in the test9 namespace? Shouldn't this change be reverted?


Comment at: test/CodeGenCXX/microsoft-abi-structors.cpp:411
@@ -410,3 @@
-
-// FIXME: This should be x86_thiscallcc.  MSVC ignores explicit CCs on 
structors.
-// CHECK: define %"struct.test1::B"* @"\01??0B@test1@@QAA@PAF@Z"

Hah, looks like we encountered this long ago and didn't go back for it. :)


http://reviews.llvm.org/D12402



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

Re: r247233 - EmitRecord* API change: accepts ArrayRef instead of a SmallVector (NFC)

[David Blaikie via cfe-commits]

On Thu, Sep 10, 2015 at 1:34 PM, Richard Smith 
wrote:

> On Thu, Sep 10, 2015 at 11:46 AM, David Blaikie 
> wrote:
>
>>
>>
>> On Thu, Sep 10, 2015 at 11:39 AM, Richard Smith 
>> wrote:
>>
>>> On Thu, Sep 10, 2015 at 9:13 AM, David Blaikie via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>


 On Thu, Sep 10, 2015 at 9:07 AM, Mehdi Amini 
 wrote:

>
> On Sep 10, 2015, at 9:02 AM, David Blaikie  wrote:
>
>
>
> On Thu, Sep 10, 2015 at 9:00 AM, Mehdi Amini 
> wrote:
>
>>
>> On Sep 9, 2015, at 7:06 PM, David Blaikie  wrote:
>>
>>
>>
>> On Wed, Sep 9, 2015 at 6:46 PM, Mehdi Amini via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: mehdi_amini
>>> Date: Wed Sep  9 20:46:39 2015
>>> New Revision: 247233
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=247233=rev
>>> Log:
>>> EmitRecord* API change: accepts ArrayRef instead of a SmallVector
>>> (NFC)
>>>
>>> This reapply a variant commit r247179 after post-commit review from
>>> D.Blaikie.
>>> Hopefully I got it right this time: lifetime of initializer list ends
>>> as with any expression, which make invalid the pattern:
>>>
>>> ArrayRef Arr = { 1, 2, 3, 4};
>>>
>>> Just like StringRef, ArrayRef shouldn't be used to initialize local
>>> variable but only as function argument.
>>>
>>
>> Looks pretty reasonable - I'll mention it again, just in case:
>> removing the named variables and just putting the init lists directly in
>> the call might be as (or more) readable - might be worth giving it a go &
>> running it through clang-format to see what you think.
>>
>>
>> Here is an example, let me know what do you think:
>>
>>   {
>> RecordData::value_type Record[] = {METADATA, VERSION_MAJOR,
>> VERSION_MINOR,
>>CLANG_VERSION_MAJOR,
>> CLANG_VERSION_MINOR,
>>!isysroot.empty(),
>> IncludeTimestamps,
>>ASTHasCompilerErrors};
>> Stream.EmitRecordWithBlob(MetadataAbbrevCode, Record,
>>   getClangFullRepositoryVersion());
>>   }
>>   Stream.EmitRecordWithBlob(
>>   MetadataAbbrevCode,
>>   (uint64_t[]){METADATA, VERSION_MAJOR, VERSION_MINOR,
>> CLANG_VERSION_MAJOR,
>>
>
> Why the cast (uint64_t[])? I'm vaguely surprised that even compiles...
> ?
>
> This is a GNU extension (C99 compound literals in C++). It won't
>>> compile on MSVC.
>>>
>>
>> Ah, good to know - thanks for the catch.
>>
>>
>>> I would imagine it'd be passed as an init list, then turned into an
> ArrayRef from there... but I guess not?
>
>
>
> Might be more clear with the callee:
>
>   template 
>   void EmitRecordWithBlob(unsigned Abbrev, const Container ,
>   StringRef Blob) {
> EmitRecordWithAbbrevImpl(Abbrev, makeArrayRef(Vals), Blob, None);
>   }
>
> The template can’t be deduced without the cast.
>

 Yeah, curious though. Another hole in perfect forwarding I suppose (not
 that this ^ is perfect forwarding, but even with perfect forwarding it
 doesn't cope well)

 Yeah, the cast is rather unfortunate. Hrm.

 Ah well - probably just as good to leave it as-is for now. If someone
 has a flash of inspiration later & figures out a way to make it better, so
 be it.

>>>
>>> The way to handle this is to add another EmitRecord overload that takes
>>> a std::initializer_list.
>>>
>>
>> Except the integer types aren't known - which has been one of the
>> wrinkles with this whole code (or at least the template is trying to allow
>> arrays of different integer types - but perhaps that's not important for
>> the init list/literal case?)
>>
>
> While the bitstream writer allows any unsigned type, the bitstream reader
> hardcodes uint64_t, so types larger than uint64_t won't round-trip and
> don't need to be supported. And we don't need to support smaller types
> either, because (1) that will cause template argument deduction problems
> and (2) we are supporting a literal braced-init-list, not some pre-built
> container of unsigned integers, so it's not really too much of an
> imposition to perform the conversion to uint64_t in the caller. So
> hardcoding uint64_t in the writer for the braced-init-list case doesn't
> seem like a problem to me.
>

Yep, I was slowly getting there. Thanks for the clarity (I don't know the
bitcode reader that well, etc) - I imagine the constants used in the init
lists are usually int64_t anyway.

If you have a literal init list of ints and pass it 

Re: [PATCH] D10018: C11 _Bool bitfield diagnostic

[Rachel Craik via cfe-commits]

rcraik added inline comments.


Comment at: lib/Sema/SemaDecl.cpp:12586
@@ -12585,3 +12585,3 @@
   if (!FieldTy->isDependentType()) {
 uint64_t TypeSize = Context.getTypeSize(FieldTy);
 if (Value.getZExtValue() > TypeSize) {

hubert.reinterpretcast wrote:
> rsmith wrote:
> > I think the right way to fix this is to call `getIntWidth` here instead of 
> > `getTypeSize`, and finesse our error message to clarify that we're talking 
> > about the width of the type (the number of value bits) rather than the size 
> > of the type (the number of storage bits).
> The implementation of `getIntWidth` currently makes this consideration moot 
> at this time, but should this extend to C89 (aside from the `_Bool` 
> extension)?
I think we have three options (the special case for _Bool bitfields being 
removed in each case):
  # change `getTypeSize` to `getIntWidth` and leave the rest of the checks 
as-is 
  # change `getTypeSize` to `getIntWidth` and update the C/MS diagnostic to 
either `ExtWarn` or `Warning`  (for some or all language levels)
  # leave as `getTypeSize` for lower language levels

Opinions?


http://reviews.llvm.org/D10018



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

Re: r247233 - EmitRecord* API change: accepts ArrayRef instead of a SmallVector (NFC)

[Richard Smith via cfe-commits]

On Thu, Sep 10, 2015 at 6:26 PM, David Blaikie  wrote:

>
>
> On Thu, Sep 10, 2015 at 1:34 PM, Richard Smith 
> wrote:
>
>> On Thu, Sep 10, 2015 at 11:46 AM, David Blaikie 
>> wrote:
>>
>>>
>>>
>>> On Thu, Sep 10, 2015 at 11:39 AM, Richard Smith 
>>> wrote:
>>>
 On Thu, Sep 10, 2015 at 9:13 AM, David Blaikie via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

>
>
> On Thu, Sep 10, 2015 at 9:07 AM, Mehdi Amini 
> wrote:
>
>>
>> On Sep 10, 2015, at 9:02 AM, David Blaikie 
>> wrote:
>>
>>
>>
>> On Thu, Sep 10, 2015 at 9:00 AM, Mehdi Amini 
>> wrote:
>>
>>>
>>> On Sep 9, 2015, at 7:06 PM, David Blaikie 
>>> wrote:
>>>
>>>
>>>
>>> On Wed, Sep 9, 2015 at 6:46 PM, Mehdi Amini via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: mehdi_amini
 Date: Wed Sep  9 20:46:39 2015
 New Revision: 247233

 URL: http://llvm.org/viewvc/llvm-project?rev=247233=rev
 Log:
 EmitRecord* API change: accepts ArrayRef instead of a SmallVector
 (NFC)

 This reapply a variant commit r247179 after post-commit review from
 D.Blaikie.
 Hopefully I got it right this time: lifetime of initializer list
 ends
 as with any expression, which make invalid the pattern:

 ArrayRef Arr = { 1, 2, 3, 4};

 Just like StringRef, ArrayRef shouldn't be used to initialize local
 variable but only as function argument.

>>>
>>> Looks pretty reasonable - I'll mention it again, just in case:
>>> removing the named variables and just putting the init lists directly in
>>> the call might be as (or more) readable - might be worth giving it a go 
>>> &
>>> running it through clang-format to see what you think.
>>>
>>>
>>> Here is an example, let me know what do you think:
>>>
>>>   {
>>> RecordData::value_type Record[] = {METADATA, VERSION_MAJOR,
>>> VERSION_MINOR,
>>>CLANG_VERSION_MAJOR,
>>> CLANG_VERSION_MINOR,
>>>!isysroot.empty(),
>>> IncludeTimestamps,
>>>ASTHasCompilerErrors};
>>> Stream.EmitRecordWithBlob(MetadataAbbrevCode, Record,
>>>   getClangFullRepositoryVersion());
>>>   }
>>>   Stream.EmitRecordWithBlob(
>>>   MetadataAbbrevCode,
>>>   (uint64_t[]){METADATA, VERSION_MAJOR, VERSION_MINOR,
>>> CLANG_VERSION_MAJOR,
>>>
>>
>> Why the cast (uint64_t[])? I'm vaguely surprised that even
>> compiles... ?
>>
>> This is a GNU extension (C99 compound literals in C++). It won't
 compile on MSVC.

>>>
>>> Ah, good to know - thanks for the catch.
>>>
>>>
 I would imagine it'd be passed as an init list, then turned into an
>> ArrayRef from there... but I guess not?
>>
>>
>>
>> Might be more clear with the callee:
>>
>>   template 
>>   void EmitRecordWithBlob(unsigned Abbrev, const Container ,
>>   StringRef Blob) {
>> EmitRecordWithAbbrevImpl(Abbrev, makeArrayRef(Vals), Blob, None);
>>   }
>>
>> The template can’t be deduced without the cast.
>>
>
> Yeah, curious though. Another hole in perfect forwarding I suppose
> (not that this ^ is perfect forwarding, but even with perfect forwarding 
> it
> doesn't cope well)
>
> Yeah, the cast is rather unfortunate. Hrm.
>
> Ah well - probably just as good to leave it as-is for now. If someone
> has a flash of inspiration later & figures out a way to make it better, so
> be it.
>

 The way to handle this is to add another EmitRecord overload that takes
 a std::initializer_list.

>>>
>>> Except the integer types aren't known - which has been one of the
>>> wrinkles with this whole code (or at least the template is trying to allow
>>> arrays of different integer types - but perhaps that's not important for
>>> the init list/literal case?)
>>>
>>
>> While the bitstream writer allows any unsigned type, the bitstream reader
>> hardcodes uint64_t, so types larger than uint64_t won't round-trip and
>> don't need to be supported. And we don't need to support smaller types
>> either, because (1) that will cause template argument deduction problems
>> and (2) we are supporting a literal braced-init-list, not some pre-built
>> container of unsigned integers, so it's not really too much of an
>> imposition to perform the conversion to uint64_t in the caller. So
>> hardcoding uint64_t in the writer for the braced-init-list case doesn't
>> seem 

Re: [PATCH] D12652: [Static Analyzer] Lambda support.

[Jordan Rose via cfe-commits]

jordan_rose added a comment.

A  few comments coming late…



Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:515-517
@@ -511,1 +514,5 @@
 
+  /// Returns true if lambdas should be inlined. Otherwise a sink node will be
+  /// generated each time a LambdaExpr is visited.
+  bool shouldInlineLambdas();
+

"inline" is kind of a misnomer, since we may not actually inline lambdas. I 
would have suggested "model lambdas" or "lambda support".


Comment at: lib/StaticAnalyzer/Core/MemRegion.cpp:740-741
@@ -739,3 +739,4 @@
   const DeclContext *DC,
-  const VarDecl *VD) {
+  const VarDecl *VD,
+  MemRegionManager *Mmgr) {
   while (LC) {

Why the extra parameter?


http://reviews.llvm.org/D12652



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

Re: [PATCH] D12785: Document __builtin_nontemporal_load and __builtin_nontemporal_store.

[Richard Smith via cfe-commits]

On Thu, Sep 10, 2015 at 5:57 PM, Michael Zolotukhin 
wrote:

> mzolotukhin added inline comments.
>
> 
> Comment at: docs/LanguageExtensions.rst:1802-1807
> @@ +1801,8 @@
> +
> +For example, on AArch64 in the following code::
> +
> +  LDR X1, [X2]
> +  LDNP X3, X4, [X1]
> +
> +the ``LDNP`` might be executed before the ``LDR``. In this case the load
> would
> +be performed from a wrong address (see 6.3.8 in `Programmer's Guide for
> ARMv8-A
> 
> rsmith wrote:
> > This seems to make the feature essentially useless, since you cannot
> guarantee that the address register is set up sufficiently far before the
> non-temporal load. Should the compiler not be required to insert the
> necessary barrier itself in this case?
> Yes, we can require targets to only use corresponding NT instructions when
> it's safe, and then remove this remark from the documentation. For ARM64
> that would mean either not to emit LDNP at all, or conservatively emit
> barriers before each LDNP (which probably removes all performance benefits
> of using it) - that is, yes, non-temporal loads would be useless on this
> target.
>

I think this should already be the case -- according to the definition of
!nontemporal in the LangRef (
http://llvm.org/docs/LangRef.html#load-instruction), using an LDNP without
an accompanying barrier would not be correct on AArch64, as it does not
have the right semantics.


> But I think we want to keep the builtin for NT-load, as it's a generic
> feature, not ARM64 specific. It can be used on other targets - e.g. we can
> use this in x86 stream builtins, and hopefully simplify their current
> implementation. I don't know about non-temporal operations on other
> targets, but if there are others, they can use it too right out of the box.


Yes, I'm not arguing for removing the builtin, just that the AArch64
backend needs to be very careful when mapping it to LDNP, because that will
frequently not be correct.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r247319 - Add a getDeclContextDescriptor() helper function to CGDebugInfo. (NFC)

[David Blaikie via cfe-commits]

On Thu, Sep 10, 2015 at 11:39 AM, Adrian Prantl via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: adrian
> Date: Thu Sep 10 13:39:45 2015
> New Revision: 247319
>
> URL: http://llvm.org/viewvc/llvm-project?rev=247319=rev
> Log:
> Add a getDeclContextDescriptor() helper function to CGDebugInfo. (NFC)
>

I might've found this easier to review with a more detailed description (&
possibly splitting the patch in two) - mentioning that this is the same as
getContextDescriptor, but accesses the DeclContext first and updating all
the callers that did this explicitly to use the utility. Second patch to
add the extra functionality of being able to specify a default other than
"TheCU".

All good though,

- Dave


>
> Modified:
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> cfe/trunk/lib/CodeGen/CGDebugInfo.h
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=247319=247318=247319=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Sep 10 13:39:45 2015
> @@ -147,9 +147,14 @@ void CGDebugInfo::setLocation(SourceLoca
>}
>  }
>
> -llvm::DIScope *CGDebugInfo::getContextDescriptor(const Decl *Context) {
> +llvm::DIScope *CGDebugInfo::getDeclContextDescriptor(const Decl *D) {
> +  return getContextDescriptor(cast(D->getDeclContext()), TheCU);
> +}
> +
> +llvm::DIScope *CGDebugInfo::getContextDescriptor(const Decl *Context,
> + llvm::DIScope *Default) {
>if (!Context)
> -return TheCU;
> +return Default;
>
>auto I = RegionMap.find(Context);
>if (I != RegionMap.end()) {
> @@ -165,7 +170,7 @@ llvm::DIScope *CGDebugInfo::getContextDe
>  if (!RDecl->isDependentType())
>return getOrCreateType(CGM.getContext().getTypeDeclType(RDecl),
>   getOrCreateMainFile());
> -  return TheCU;
> +  return Default;
>  }
>
>  StringRef CGDebugInfo::getFunctionName(const FunctionDecl *FD) {
> @@ -770,7 +775,7 @@ llvm::DIType *CGDebugInfo::CreateType(co
>SourceLocation Loc = AliasDecl->getLocation();
>return DBuilder.createTypedef(
>Src, internString(OS.str()), getOrCreateFile(Loc),
> getLineNumber(Loc),
> -  getContextDescriptor(cast(AliasDecl->getDeclContext(;
> +  getDeclContextDescriptor(AliasDecl));
>  }
>
>  llvm::DIType *CGDebugInfo::CreateType(const TypedefType *Ty,
> @@ -783,7 +788,7 @@ llvm::DIType *CGDebugInfo::CreateType(co
>return DBuilder.createTypedef(
>getOrCreateType(Ty->getDecl()->getUnderlyingType(), Unit),
>Ty->getDecl()->getName(), getOrCreateFile(Loc), getLineNumber(Loc),
> -  getContextDescriptor(cast(Ty->getDecl()->getDeclContext(;
> +  getDeclContextDescriptor(Ty->getDecl()));
>  }
>
>  llvm::DIType *CGDebugInfo::CreateType(const FunctionType *Ty,
> @@ -1510,8 +1515,7 @@ llvm::DIType *CGDebugInfo::CreateType(co
>llvm::DIType *T = cast_or_null(getTypeOrNull(QualType(Ty,
> 0)));
>if (T || shouldOmitDefinition(DebugKind, RD, CGM.getLangOpts())) {
>  if (!T)
> -  T = getOrCreateRecordFwdDecl(
> -  Ty, getContextDescriptor(cast(RD->getDeclContext(;
> +  T = getOrCreateRecordFwdDecl(Ty, getDeclContextDescriptor(RD));
>  return T;
>}
>
> @@ -1939,8 +1943,7 @@ llvm::DIType *CGDebugInfo::CreateEnumTyp
>// If this is just a forward declaration, construct an appropriately
>// marked node and just return it.
>if (!ED->getDefinition()) {
> -llvm::DIScope *EDContext =
> -getContextDescriptor(cast(ED->getDeclContext()));
> +llvm::DIScope *EDContext = getDeclContextDescriptor(ED);
>  llvm::DIFile *DefUnit = getOrCreateFile(ED->getLocation());
>  unsigned Line = getLineNumber(ED->getLocation());
>  StringRef EDName = ED->getName();
> @@ -1980,8 +1983,7 @@ llvm::DIType *CGDebugInfo::CreateTypeDef
>
>llvm::DIFile *DefUnit = getOrCreateFile(ED->getLocation());
>unsigned Line = getLineNumber(ED->getLocation());
> -  llvm::DIScope *EnumContext =
> -  getContextDescriptor(cast(ED->getDeclContext()));
> +  llvm::DIScope *EnumContext = getDeclContextDescriptor(ED);
>llvm::DIType *ClassTy =
>ED->isFixed() ? getOrCreateType(ED->getIntegerType(), DefUnit) :
> nullptr;
>return DBuilder.createEnumerationType(EnumContext, ED->getName(),
> DefUnit,
> @@ -2228,8 +2230,7 @@ llvm::DICompositeType *CGDebugInfo::Crea
>unsigned Line = getLineNumber(RD->getLocation());
>StringRef RDName = getClassName(RD);
>
> -  llvm::DIScope *RDContext =
> -  getContextDescriptor(cast(RD->getDeclContext()));
> +  llvm::DIScope *RDContext = getDeclContextDescriptor(RD);
>
>// If we ended up creating the type during the context chain
> construction,
>// just return that.
> @@ -2326,7 +2327,7 @@ void CGDebugInfo::collectFunctionDeclPro
>

Re: [PATCH] D12785: Document __builtin_nontemporal_load and __builtin_nontemporal_store.

[Michael Zolotukhin via cfe-commits]

mzolotukhin updated this revision to Diff 34521.
mzolotukhin added a comment.

- Remove paragraph about changing program behavior (since we shouldn't change 
it anyway).


http://reviews.llvm.org/D12785

Files:
  docs/LanguageExtensions.rst

Index: docs/LanguageExtensions.rst
===
--- docs/LanguageExtensions.rst
+++ docs/LanguageExtensions.rst
@@ -1778,6 +1778,26 @@
 For these reasons the higher level atomic primitives should be preferred where
 possible.
 
+Non-temporal load/store builtins
+
+
+Clang provides overloaded builtins allowing to generate non-temporal memory
+accesses.
+
+.. code-block:: c
+
+  T __builtin_nontemporal_load(T *addr);
+  void __builtin_nontemporal_store(T value, T *addr);
+
+The types ``T`` currently supported are:
+
+* Integer types.
+* Floating-point types.
+* Vector types.
+
+Note that the compiler does not guarantee that non-temporal loads or stores
+would be used.
+
 Non-standard C++11 Attributes
 =
 


Index: docs/LanguageExtensions.rst
===
--- docs/LanguageExtensions.rst
+++ docs/LanguageExtensions.rst
@@ -1778,6 +1778,26 @@
 For these reasons the higher level atomic primitives should be preferred where
 possible.
 
+Non-temporal load/store builtins
+
+
+Clang provides overloaded builtins allowing to generate non-temporal memory
+accesses.
+
+.. code-block:: c
+
+  T __builtin_nontemporal_load(T *addr);
+  void __builtin_nontemporal_store(T value, T *addr);
+
+The types ``T`` currently supported are:
+
+* Integer types.
+* Floating-point types.
+* Vector types.
+
+Note that the compiler does not guarantee that non-temporal loads or stores
+would be used.
+
 Non-standard C++11 Attributes
 =
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12652: [Static Analyzer] Lambda support.

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

Have you tested this on a large codebase that uses lambdas? When do you think 
we should turn this on by default?

Please, add test cases that demonstrate what happens when an issue is reported 
within a lambda and to check if inlined defensive checks work.

(As a follow up to this patch, we may need to teach LiveVariables.cpp and 
UninitializedValues.cpp about lambdas. For example, to address issues like this 
one: https://llvm.org/bugs/show_bug.cgi?id=22834)


http://reviews.llvm.org/D12652



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

Re: [PATCH] D12652: [Static Analyzer] Lambda support.

[Anna Zaks via cfe-commits]

zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.

Please, do turn on by default.
LGTM


http://reviews.llvm.org/D12652



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

Re: [PATCH] D12780: [analyzer] Add generateErrorNode() APIs to CheckerContext

[Anna Zaks via cfe-commits]

zaks.anna added inline comments.


Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:229
@@ -228,2 +228,3 @@
+  /// checkers should use generateErrorNode() instead.
   ExplodedNode *generateSink(ProgramStateRef State = nullptr,
  ExplodedNode *Pred = nullptr,

Most likely there are not much uses of this left and to avoid confusion we 
could require State and Pred inputs. What do you think?


Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:244
@@ +243,3 @@
+  const ProgramPointTag *Tag = nullptr) {
+return generateSink(State, /*Pred=*/nullptr,
+   (Tag ? Tag : Location.getTag()));

Please, use a non-null Pred for clarity


Comment at: lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:89
@@ -88,3 +88,3 @@
  CheckerContext ) const {
-  ExplodedNode *N = C.getPredecessor();
+  ExplodedNode *N = C.generateNonFatalErrorNode();
   const LocationContext *LC = N->getLocationContext();

We should not generate a transition on an early exit here..


Comment at: lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:116
@@ -115,3 +115,3 @@
  CheckerContext ) const {
-  ExplodedNode *N = C.getPredecessor();
+  ExplodedNode *N = C.generateNonFatalErrorNode();
   const LocationContext *LC = N->getLocationContext();

Same here, no reason to generate a transition unless we are ready to report a 
bug. I'just pass C.generateNonFatalErrorNode() to  llvm::make_unique.


Comment at: lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp:53
@@ -52,3 +52,3 @@
 
-  if (ExplodedNode *N = C.addTransition()) {
+  if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
 if (!BT)

Can this ever fail? In some cases we just assume it won't in others we tests..

Maybe it only fails when we cache out?


http://reviews.llvm.org/D12780



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

Re: [PATCH] D12780: [analyzer] Add generateErrorNode() APIs to CheckerContext

[Jordan Rose via cfe-commits]

jordan_rose added inline comments.


Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:321
@@ +320,3 @@
+// sink, we assume that a client requesting a transition to a state that is
+// the same as the predecessor state has made a mistake. We return the
+// predecessor and rather than cache out.

What does "has made a mistake" mean? What is the mistake and how will they 
discover it?


http://reviews.llvm.org/D12780



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

r247373 - [sema] Fix assertion hit when using libclang to index a particular C++ snippet involving templates.

[Argyrios Kyrtzidis via cfe-commits]

Author: akirtzidis
Date: Thu Sep 10 20:44:56 2015
New Revision: 247373

URL: http://llvm.org/viewvc/llvm-project?rev=247373=rev
Log:
[sema] Fix assertion hit when using libclang to index a particular C++ snippet 
involving templates.

Assertion hit was in ClassTemplateSpecializationDecl::getSourceRange().

Modified:
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/Index/index-file.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=247373=247372=247373=diff
==
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Thu Sep 10 20:44:56 2015
@@ -7386,11 +7386,16 @@ Sema::ActOnExplicitInstantiation(Scope *
   }
 }
 
+// Set the template specialization kind. Make sure it is set before
+// instantiating the members which will trigger ASTConsumer callbacks.
+Specialization->setTemplateSpecializationKind(TSK);
 InstantiateClassTemplateSpecializationMembers(TemplateNameLoc, Def, TSK);
+  } else {
+
+// Set the template specialization kind.
+Specialization->setTemplateSpecializationKind(TSK);
   }
 
-  // Set the template specialization kind.
-  Specialization->setTemplateSpecializationKind(TSK);
   return Specialization;
 }
 

Modified: cfe/trunk/test/Index/index-file.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/index-file.cpp?rev=247373=247372=247373=diff
==
--- cfe/trunk/test/Index/index-file.cpp (original)
+++ cfe/trunk/test/Index/index-file.cpp Thu Sep 10 20:44:56 2015
@@ -15,9 +15,19 @@ void tfoo() {}
 void tfoo() {}
 }
 
+namespace crash1 {
+template class A {
+  A(A &) = delete;
+  void meth();
+};
+template <> void A::meth();
+template class A;
+}
+
 // RUN: c-index-test -index-file %s > %t
 // RUN: FileCheck %s -input-file=%t
 
 // CHECK: [indexDeclaration]: kind: type-alias | name: MyTypeAlias | {{.*}} | 
loc: 1:7
 // CHECK: [indexDeclaration]: kind: struct-template-spec | name: TS | {{.*}} | 
loc: 11:8
 // CHECK: [indexDeclaration]: kind: function-template-spec | name: tfoo | 
{{.*}} | loc: 15:6
+// CHECK: [indexDeclaration]: kind: c++-instance-method | name: meth | {{.*}} 
| loc: 23:26


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

r247346 - [MS ABI] Make member pointers return true for isIncompleteType

[David Majnemer via cfe-commits]

Author: majnemer
Date: Thu Sep 10 16:52:00 2015
New Revision: 247346

URL: http://llvm.org/viewvc/llvm-project?rev=247346=rev
Log:
[MS ABI] Make member pointers return true for isIncompleteType

The type of a member pointer is incomplete if it has no inheritance
model.  This lets us reuse more general logic already embedded in clang.

Modified:
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/CodeGen/CGCXXABI.h
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaType.cpp

Modified: cfe/trunk/lib/AST/Type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=247346=247345=247346=diff
==
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Thu Sep 10 16:52:00 2015
@@ -22,6 +22,7 @@
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeVisitor.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/TargetInfo.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/raw_ostream.h"
@@ -1899,6 +1900,28 @@ bool Type::isIncompleteType(NamedDecl **
   case IncompleteArray:
 // An array of unknown size is an incomplete type (C99 6.2.5p22).
 return true;
+  case MemberPointer: {
+// Member pointers in the MS ABI have special behavior in
+// RequireCompleteType: they attach a MSInheritanceAttr to the 
CXXRecordDecl
+// to indicate which inheritance model to use.
+auto *MPTy = cast(CanonicalType);
+const Type *ClassTy = MPTy->getClass();
+// Member pointers with dependent class types don't get special treatment.
+if (ClassTy->isDependentType())
+  return false;
+const CXXRecordDecl *RD = ClassTy->getAsCXXRecordDecl();
+ASTContext  = RD->getASTContext();
+// Member pointers not in the MS ABI don't get special treatment.
+if (!Context.getTargetInfo().getCXXABI().isMicrosoft())
+  return false;
+// The inheritance attribute might only be present on the most recent
+// CXXRecordDecl, use that one.
+RD = RD->getMostRecentDecl();
+// Nothing interesting to do if the inheritance attribute is already set.
+if (RD->hasAttr())
+  return false;
+return true;
+  }
   case ObjCObject:
 return cast(CanonicalType)->getBaseType()
  ->isIncompleteType(Def);

Modified: cfe/trunk/lib/CodeGen/CGCXXABI.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.h?rev=247346=247345=247346=diff
==
--- cfe/trunk/lib/CodeGen/CGCXXABI.h (original)
+++ cfe/trunk/lib/CodeGen/CGCXXABI.h Thu Sep 10 16:52:00 2015
@@ -174,10 +174,6 @@ public:
 return true;
   }
 
-  virtual bool isTypeInfoCalculable(QualType Ty) const {
-return !Ty->isIncompleteType();
-  }
-
   /// Create a null member pointer of the given type.
   virtual llvm::Constant *EmitNullMemberPointer(const MemberPointerType *MPT);
 

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=247346=247345=247346=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Thu Sep 10 16:52:00 2015
@@ -1586,7 +1586,7 @@ void CodeGenModule::ConstructAttributeLi
 
   if (const auto *RefTy = RetTy->getAs()) {
 QualType PTy = RefTy->getPointeeType();
-if (getCXXABI().isTypeInfoCalculable(PTy) && PTy->isConstantSizeType())
+if (!PTy->isIncompleteType() && PTy->isConstantSizeType())
   RetAttrs.addDereferenceableAttr(getContext().getTypeSizeInChars(PTy)
 .getQuantity());
 else if (getContext().getTargetAddressSpace(PTy) == 0)
@@ -1698,7 +1698,7 @@ void CodeGenModule::ConstructAttributeLi
 
 if (const auto *RefTy = ParamType->getAs()) {
   QualType PTy = RefTy->getPointeeType();
-  if (getCXXABI().isTypeInfoCalculable(PTy) && PTy->isConstantSizeType())
+  if (!PTy->isIncompleteType() && PTy->isConstantSizeType())
 Attrs.addDereferenceableAttr(getContext().getTypeSizeInChars(PTy)
.getQuantity());
   else if (getContext().getTargetAddressSpace(PTy) == 0)

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=247346=247345=247346=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Sep 10 16:52:00 2015
@@ -1906,9 +1906,8 @@ llvm::DIType *CGDebugInfo::CreateType(co
 
 llvm::DIType *CGDebugInfo::CreateType(const 

[PATCH] D12785: Document __builtin_nontemporal_load and __builtin_nontemporal_store.

[Michael Zolotukhin via cfe-commits]

mzolotukhin created this revision.
mzolotukhin added reviewers: hfinkel, rsmith.
mzolotukhin added a subscriber: cfe-commits.

In r247104 I added the builtins for generating non-temporal memory operations,
but now I realized that they lack documentation. This patch adds some.

http://reviews.llvm.org/D12785

Files:
  docs/LanguageExtensions.rst

Index: docs/LanguageExtensions.rst
===
--- docs/LanguageExtensions.rst
+++ docs/LanguageExtensions.rst
@@ -1778,6 +1778,36 @@
 For these reasons the higher level atomic primitives should be preferred where
 possible.
 
+Non-temporal load/store builtins
+
+
+Clang provides overloaded builtins allowing to generate non-temporal memory
+accesses.
+
+.. code-block:: c
+
+  T __builtin_nontemporal_load(T *addr);
+  void __builtin_nontemporal_store(T value, T *addr);
+
+The types ``T`` currently supported are:
+
+* Integer types.
+* Floating-point types
+* Vector types.
+
+Note that the compiler does not guarantee that non-temporal loads or stores
+would be used.  Also, using non-temporal loads and stores might change program
+semantics on some targets, so the builtins should be used with care.
+
+For example, on AArch64 in the following code::
+
+  LDR X1, [X2]
+  LDNP X3, X4, [X1]
+
+the ``LDNP`` might be executed before the ``LDR``. In this case the load would
+be performed from a wrong address (see 6.3.8 in `Programmer's Guide for ARMv8-A
+`_).
+
 Non-standard C++11 Attributes
 =
 


Index: docs/LanguageExtensions.rst
===
--- docs/LanguageExtensions.rst
+++ docs/LanguageExtensions.rst
@@ -1778,6 +1778,36 @@
 For these reasons the higher level atomic primitives should be preferred where
 possible.
 
+Non-temporal load/store builtins
+
+
+Clang provides overloaded builtins allowing to generate non-temporal memory
+accesses.
+
+.. code-block:: c
+
+  T __builtin_nontemporal_load(T *addr);
+  void __builtin_nontemporal_store(T value, T *addr);
+
+The types ``T`` currently supported are:
+
+* Integer types.
+* Floating-point types
+* Vector types.
+
+Note that the compiler does not guarantee that non-temporal loads or stores
+would be used.  Also, using non-temporal loads and stores might change program
+semantics on some targets, so the builtins should be used with care.
+
+For example, on AArch64 in the following code::
+
+  LDR X1, [X2]
+  LDNP X3, X4, [X1]
+
+the ``LDNP`` might be executed before the ``LDR``. In this case the load would
+be performed from a wrong address (see 6.3.8 in `Programmer's Guide for ARMv8-A
+`_).
+
 Non-standard C++11 Attributes
 =
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r247367 - [MS ABI] Remove another call to RequireCompleteType

[David Majnemer via cfe-commits]

Author: majnemer
Date: Thu Sep 10 19:53:15 2015
New Revision: 247367

URL: http://llvm.org/viewvc/llvm-project?rev=247367=rev
Log:
[MS ABI] Remove another call to RequireCompleteType

I cannot come up with a testcase which would rely on this call to
RequireCompleteType, I believe that it is superfluous given the current
state of clang.

Modified:
cfe/trunk/lib/Sema/SemaCast.cpp

Modified: cfe/trunk/lib/Sema/SemaCast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=247367=247366=247367=diff
==
--- cfe/trunk/lib/Sema/SemaCast.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCast.cpp Thu Sep 10 19:53:15 2015
@@ -1496,10 +1496,6 @@ TryStaticImplicitCast(Sema , ExprRe
   msg = 0;
   return TC_Failed;
 }
-  } else if (DestType->isMemberPointerType()) {
-if (Self.Context.getTargetInfo().getCXXABI().isMicrosoft()) {
-  Self.RequireCompleteType(OpRange.getBegin(), DestType, 0);
-}
   }
 
   InitializedEntity Entity = InitializedEntity::InitializeTemporary(DestType);


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

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

[John McCall via cfe-commits]

rjmccall added a comment.

Sorry for putting off the final review on this; I was heads-down trying to get 
the alignment patch done.  It's looking good; obviously you'll need to update 
it to work with Addresses properly, but hopefully that won't be too much of a 
problem.

When you do, maybe you should start a new review; I think there's some way to 
do that in Phabricator that ties it to the old one.  Phabricator seems to not 
be very happy with the extent to which the code has changed, and the old 
comments now just make it harder to review the current patch.


http://reviews.llvm.org/D11361



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

Re: [PATCH] D12221: [RFC] Introduce `__attribute__((nontemporal))`.

[Michael Zolotukhin via cfe-commits]

mzolotukhin abandoned this revision.
mzolotukhin added a comment.

We decided to go with an alternative way - with builtins instead of type 
attributes. The corresponding patch is http://reviews.llvm.org/D12313, and it's 
already reviewed and committed.


http://reviews.llvm.org/D12221



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

r247380 - [opaque pointer type] update test cases for explicit pointee types on global aliases

[David Blaikie via cfe-commits]

Author: dblaikie
Date: Thu Sep 10 22:22:18 2015
New Revision: 247380

URL: http://llvm.org/viewvc/llvm-project?rev=247380=rev
Log:
[opaque pointer type] update test cases for explicit pointee types on global 
aliases

Modified:
cfe/trunk/test/CodeGen/alias.c
cfe/trunk/test/CodeGen/attributes.c
cfe/trunk/test/CodeGen/hidden-alias-to-internal-function.c
cfe/trunk/test/CodeGen/pragma-weak.c
cfe/trunk/test/CodeGenCXX/attr.cpp
cfe/trunk/test/CodeGenCXX/constructor-alias.cpp
cfe/trunk/test/CodeGenCXX/ctor-dtor-alias.cpp
cfe/trunk/test/CodeGenCXX/cxx11-thread-local-reference.cpp
cfe/trunk/test/CodeGenCXX/cxx11-thread-local.cpp
cfe/trunk/test/CodeGenCXX/destructors.cpp
cfe/trunk/test/CodeGenCXX/dllexport-alias.cpp
cfe/trunk/test/CodeGenCXX/dllexport.cpp
cfe/trunk/test/CodeGenCXX/extern-c.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-structors-alias.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp
cfe/trunk/test/CodeGenCXX/new-alias.cpp
cfe/trunk/test/CodeGenCXX/virtual-destructor-calls.cpp
cfe/trunk/test/OpenMP/threadprivate_codegen.cpp

Modified: cfe/trunk/test/CodeGen/alias.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/alias.c?rev=247380=247379=247380=diff
==
--- cfe/trunk/test/CodeGen/alias.c (original)
+++ cfe/trunk/test/CodeGen/alias.c Thu Sep 10 22:22:18 2015
@@ -21,20 +21,20 @@ const int wacom_usb_ids[] = {1, 1, 2, 3,
 // CHECKASM-DAG: .globl wacom_usb_ids
 // CHECKASM-DAG: .size wacom_usb_ids, 32
 extern const int __mod_usb_device_table __attribute__ 
((alias("wacom_usb_ids")));
-// CHECKBASIC-DAG: @__mod_usb_device_table = alias getelementptr inbounds ([8 
x i32], [8 x i32]* @wacom_usb_ids, i32 0, i32 0)
+// CHECKBASIC-DAG: @__mod_usb_device_table = alias i32, getelementptr inbounds 
([8 x i32], [8 x i32]* @wacom_usb_ids, i32 0, i32 0)
 // CHECKASM-DAG: .globl __mod_usb_device_table
 // CHECKASM-DAG: __mod_usb_device_table = wacom_usb_ids
 // CHECKASM-DAG-NOT: .size __mod_usb_device_table
 
 extern int g1;
 extern int g1 __attribute((alias("g0")));
-// CHECKBASIC-DAG: @g1 = alias i32* @g0
+// CHECKBASIC-DAG: @g1 = alias i32, i32* @g0
 // CHECKASM-DAG: .globl g1
 // CHECKASM-DAG: g1 = g0
 // CHECKASM-DAG-NOT: .size g1
 
 extern __thread int __libc_errno __attribute__ ((alias ("TL_WITH_ALIAS")));
-// CHECKBASIC-DAG: @__libc_errno = thread_local alias i32* @TL_WITH_ALIAS
+// CHECKBASIC-DAG: @__libc_errno = thread_local alias i32, i32* @TL_WITH_ALIAS
 // CHECKASM-DAG: .globl __libc_errno
 // CHECKASM-DAG: __libc_errno = TL_WITH_ALIAS
 // CHECKASM-DAG-NOT: .size __libc_errno
@@ -42,10 +42,10 @@ extern __thread int __libc_errno __attri
 void f0(void) { }
 extern void f1(void);
 extern void f1(void) __attribute((alias("f0")));
-// CHECKBASIC-DAG: @f1 = alias void ()* @f0
-// CHECKBASIC-DAG: @test8_foo = weak alias bitcast (void ()* @test8_bar to 
void (...)*)
-// CHECKBASIC-DAG: @test8_zed = alias bitcast (void ()* @test8_bar to void 
(...)*)
-// CHECKBASIC-DAG: @test9_zed = alias void ()* @test9_bar
+// CHECKBASIC-DAG: @f1 = alias void (), void ()* @f0
+// CHECKBASIC-DAG: @test8_foo = weak alias void (...), bitcast (void ()* 
@test8_bar to void (...)*)
+// CHECKBASIC-DAG: @test8_zed = alias void (...), bitcast (void ()* @test8_bar 
to void (...)*)
+// CHECKBASIC-DAG: @test9_zed = alias void (), void ()* @test9_bar
 // CHECKBASIC: define void @f0() [[NUW:#[0-9]+]] {
 
 // Make sure that aliases cause referenced values to be emitted.
@@ -65,7 +65,7 @@ static int inner(int a) { return 0; }
 static int inner_weak(int a) { return 0; }
 extern __typeof(inner) inner_a __attribute__((alias("inner")));
 static __typeof(inner_weak) inner_weak_a __attribute__((weakref, 
alias("inner_weak")));
-// CHECKCC: @inner_a = alias i32 (i32)* @inner
+// CHECKCC: @inner_a = alias i32 (i32), i32 (i32)* @inner
 // CHECKCC: define internal arm_aapcs_vfpcc i32 @inner(i32 %a) [[NUW:#[0-9]+]] 
{
 
 int outer(int a) { return inner(a); }

Modified: cfe/trunk/test/CodeGen/attributes.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/attributes.c?rev=247380=247379=247380=diff
==
--- cfe/trunk/test/CodeGen/attributes.c (original)
+++ cfe/trunk/test/CodeGen/attributes.c Thu Sep 10 22:22:18 2015
@@ -26,7 +26,7 @@ int t6 __attribute__((visibility("protec
 // CHECK: @t12 = global i32 0, section "SECT"
 int t12 __attribute__((section("SECT")));
 
-// CHECK: @t9 = weak alias bitcast (void ()* @__t8 to void (...)*)
+// CHECK: @t9 = weak alias void (...), bitcast (void ()* @__t8 to void (...)*)
 void __t8() {}
 void t9() __attribute__((weak, alias("__t8")));
 

Modified: cfe/trunk/test/CodeGen/hidden-alias-to-internal-function.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/hidden-alias-to-internal-function.c?rev=247380=247379=247380=diff

r247384 - [modules] Don't load files specified by -fmodule-file= when modules are

[Richard Smith via cfe-commits]

Author: rsmith
Date: Thu Sep 10 22:58:07 2015
New Revision: 247384

URL: http://llvm.org/viewvc/llvm-project?rev=247384=rev
Log:
[modules] Don't load files specified by -fmodule-file= when modules are
disabled. (We still allow this via -cc1 / -Xclang, primarily for testing.)

Modified:
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/test/Driver/modules.m

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=247384=247383=247384=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Thu Sep 10 22:58:07 2015
@@ -4461,7 +4461,10 @@ void Clang::ConstructJob(Compilation ,
   Args.AddAllArgs(CmdArgs, options::OPT_fmodule_map_file);
 
   // -fmodule-file can be used to specify files containing precompiled modules.
-  Args.AddAllArgs(CmdArgs, options::OPT_fmodule_file);
+  if (HaveModules)
+Args.AddAllArgs(CmdArgs, options::OPT_fmodule_file);
+  else
+Args.ClaimAllArgs(options::OPT_fmodule_file);
 
   // -fmodule-cache-path specifies where our implicitly-built module files
   // should be written.

Modified: cfe/trunk/test/Driver/modules.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/modules.m?rev=247384=247383=247384=diff
==
--- cfe/trunk/test/Driver/modules.m (original)
+++ cfe/trunk/test/Driver/modules.m Thu Sep 10 22:58:07 2015
@@ -37,3 +37,13 @@
 // CHECK-MODULE-MAP-FILES: "-fmodules"
 // CHECK-MODULE-MAP-FILES: "-fmodule-map-file=foo.map"
 // CHECK-MODULE-MAP-FILES: "-fmodule-map-file=bar.map"
+
+// RUN: %clang -fmodules -fmodule-file=foo.pcm -fmodule-file=bar.pcm -### %s 
2>&1 | FileCheck -check-prefix=CHECK-MODULE-FILES %s
+// CHECK-MODULE-FILES: "-fmodules"
+// CHECK-MODULE-FILES: "-fmodule-file=foo.pcm"
+// CHECK-MODULE-FILES: "-fmodule-file=bar.pcm"
+
+// RUN: %clang -fno-modules -fmodule-file=foo.pcm -fmodule-file=bar.pcm -### 
%s 2>&1 | FileCheck -check-prefix=CHECK-NO-MODULE-FILES %s
+// CHECK-NO-MODULE-FILES-NOT: "-fmodules"
+// CHECK-NO-MODULE-FILES-NOT: "-fmodule-file=foo.pcm"
+// CHECK-NO-MODULE-FILES-NOT: "-fmodule-file=bar.pcm"


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

Re: r246985 - Compute and preserve alignment more faithfully in IR-generation.

[Chandler Carruth via cfe-commits]

On Thu, Sep 10, 2015 at 7:27 PM John McCall  wrote:

> On Sep 10, 2015, at 16:09, Chandler Carruth  wrote:
>
>
> On Thu, Sep 10, 2015 at 3:26 PM John McCall  wrote:
>
>> On Sep 10, 2015, at 3:22 PM, Chandler Carruth 
>> wrote:
>>
>> I've reproduced this with the same technique.
>>
>> John, let me know if you need help debugging this, but this is blocking a
>> ton of stuff for us so I'm going to revert for now.
>>
>>
>> Please just give me a chance to fix; it is probably straightforward, and
>> the change is invasive enough that you’ll have to revert the last three
>> days of changes to IRGen.
>>
>
> I mean, I'd love to avoid the revert, but were totally hosed at the moment
> by this change (perhaps our fault for so much code managing to use a silly
> jpeg library). I'm going to at least start working on teasing this stuff
> apart in parallel, and hopefully you'll have a fix soon. While we're going
> to need to revert to green before too long, I'll update here before doing
> anything. Also happy to chat on IRC if there is anything I can do to help.
>
>
> For those following along at home, we tracked this down to some assembly
> that's not following the x86-64 ABI correctly.
>
> Clang is emitting a pair of adjacent 32-bit loads. With my patch, the
> first is now marked with alignment 8; LLVM deduces that the loads can be
> combined. Later, the first value is passed as an argument to a function,
> and LLVM places the combined load in the argument register without clearing
> the high bits; this is legal as those bits have no guaranteed value under
> the ABI. The callee, which is written in assembly, then uses the full
> register as an index into an array without zero-extending it first.
>

Another note is that I'm going to keep an eye out for any other assembly we
run into making similar assumptions, but after digging into the project, it
appears that most of this is likely a historical error for a single type.

And I just want to say *many* thanks for the assistance understanding this
error John (and David, Art, Richard, and everyone else)... It was not at
all easy to really pinpoint the nature of this problem, and made more
confusing by the fact that the ABI seems silent about this (despite the
clear behavior of multiple implementations).

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

Re: [PATCH] D1623: Support __builtin_ms_va_list.

[Richard Smith via cfe-commits]

rsmith added inline comments.


Comment at: lib/Sema/SemaExpr.cpp:11664-11672
@@ -11662,1 +11663,11 @@
+
+  // It might be a __builtin_ms_va_list.
+  if (!E->isTypeDependent() && Context.getTargetInfo().hasBuiltinMSVaList()) {
+QualType MSVaListType = Context.getBuiltinMSVaListType();
+if (Context.hasSameType(MSVaListType, E->getType())) {
+  if (CheckForModifiableLvalue(E, BuiltinLoc, *this))
+return ExprError();
+  IsMS = true;
+}
+  }
 

If `__builtin_va_list` and `__builtin_ms_va_list` are the same type, this will 
set `IsMS` to true, which is not wrong per se but seems a bit surprising. 
(`IsMS` is the "I'm using an unnatural ABI" flag, and I think it'd be a bit 
better to not set it for normal `va_start` / `va_arg` / `va_end`, even when 
targeting the MS ABI. Thoughts?


http://reviews.llvm.org/D1623



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

r247375 - [modules] Slightly defang an assert that produces false-positives on the selfhost bot.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Thu Sep 10 21:22:03 2015
New Revision: 247375

URL: http://llvm.org/viewvc/llvm-project?rev=247375=rev
Log:
[modules] Slightly defang an assert that produces false-positives on the 
selfhost bot.

Modified:
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp

Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=247375=247374=247375=diff
==
--- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Thu Sep 10 21:22:03 2015
@@ -1522,20 +1522,21 @@ void ASTDeclWriter::VisitDeclContext(Dec
   Record.push_back(VisibleOffset);
 }
 
-/// \brief Is this a local declaration (that is, one that will be written to
-/// our AST file)? This is the case for declarations that are neither imported
-/// from another AST file nor predefined.
-static bool isLocalDecl(ASTWriter , const Decl *D) {
-  if (D->isFromASTFile())
-return false;
-  return W.getDeclID(D) >= NUM_PREDEF_DECL_IDS;
-}
-
 const Decl *ASTWriter::getFirstLocalDecl(const Decl *D) {
-  assert(isLocalDecl(*this, D) && "expected a local declaration");
+  /// \brief Is this a local declaration (that is, one that will be written to
+  /// our AST file)? This is the case for declarations that are neither 
imported
+  /// from another AST file nor predefined.
+  auto IsLocalDecl = [&](const Decl *D) -> bool {
+if (D->isFromASTFile())
+  return false;
+auto I = DeclIDs.find(D);
+return (I == DeclIDs.end() || I->second >= NUM_PREDEF_DECL_IDS);
+  };
+
+  assert(IsLocalDecl(D) && "expected a local declaration");
 
   const Decl *Canon = D->getCanonicalDecl();
-  if (isLocalDecl(*this, Canon))
+  if (IsLocalDecl(Canon))
 return Canon;
 
   const Decl * = FirstLocalDeclCache[Canon];
@@ -1543,7 +1544,7 @@ const Decl *ASTWriter::getFirstLocalDecl
 return CacheEntry;
 
   for (const Decl *Redecl = D; Redecl; Redecl = Redecl->getPreviousDecl())
-if (isLocalDecl(*this, Redecl))
+if (IsLocalDecl(Redecl))
   D = Redecl;
   return CacheEntry = D;
 }


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

Re: [PATCH] D12747: Implement [depr.c.headers]

[Richard Smith via cfe-commits]

rsmith added a comment.

Each of the foo.h files added here was svn cp'd from the corresponding cfoo 
file. The listed diffs are against the base file. Likewise, __nullptr was 
copied from cstddef.

(Please disregard the changes to lib/buildit.)


Repository:
  rL LLVM

http://reviews.llvm.org/D12747



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

Re: [PATCH] D12743: [CodeGen] Teach SimplifyPersonality about the updated LandingPadInst

[John McCall via cfe-commits]

rjmccall added a comment.

LGTM, thanks.


http://reviews.llvm.org/D12743



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

Re: [PATCH] D11815: Pass subtarget feature "force-align-stack"

[Akira Hatanaka via cfe-commits]

ahatanak added a comment.

OK, thanks. I'll go ahead and commit this patch and the llvm-side patch.


http://reviews.llvm.org/D11815



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

Re: [PATCH] D12402: PR24595: clang-cl fails to compile vswriter.h header from Windows SDK 8.1 in 32 bit mode

[Andrey Bokhanko via cfe-commits]

andreybokhanko updated this revision to Diff 34477.
andreybokhanko added a comment.

Reid,

Sorry for delayed reply. All your comments are fixed (it turned out that usage 
of DefaultCC instead of ToCC is the root of all evil; after I fixed this, all 
other problems went away).

Patch updated; please re-review.

Andrey


http://reviews.llvm.org/D12402

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/Sema.h
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenCXX/ctor-dtor-alias.cpp
  test/CodeGenCXX/microsoft-abi-structors.cpp
  test/SemaCXX/decl-microsoft-call-conv.cpp

Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2269,8 +2269,11 @@
 
   // Adjust the default free function calling convention to the default method
   // calling convention.
+  bool IsCtorOrDtor =
+  (Entity.getNameKind() == DeclarationName::CXXConstructorName) ||
+  (Entity.getNameKind() == DeclarationName::CXXDestructorName);
   if (T->isFunctionType())
-adjustMemberFunctionCC(T, /*IsStatic=*/false);
+adjustMemberFunctionCC(T, /*IsStatic=*/false, IsCtorOrDtor, Loc);
 
   return Context.getMemberPointerType(T, Class.getTypePtr());
 }
@@ -5842,25 +5845,43 @@
   return false;
 }
 
-void Sema::adjustMemberFunctionCC(QualType , bool IsStatic) {
+void Sema::adjustMemberFunctionCC(QualType , bool IsStatic, bool IsCtorOrDtor,
+  SourceLocation Loc) {
   FunctionTypeUnwrapper Unwrapped(*this, T);
   const FunctionType *FT = Unwrapped.get();
   bool IsVariadic = (isa(FT) &&
  cast(FT)->isVariadic());
-
-  // Only adjust types with the default convention.  For example, on Windows we
-  // should adjust a __cdecl type to __thiscall for instance methods, and a
-  // __thiscall type to __cdecl for static methods.
   CallingConv CurCC = FT->getCallConv();
-  CallingConv FromCC =
-  Context.getDefaultCallingConvention(IsVariadic, IsStatic);
   CallingConv ToCC = Context.getDefaultCallingConvention(IsVariadic, !IsStatic);
-  if (CurCC != FromCC || FromCC == ToCC)
-return;
 
-  if (hasExplicitCallingConv(T))
+  if (CurCC == ToCC)
 return;
 
+  // MS compiler ignores explicit calling convention attributes on structors. We
+  // should do the same.
+  if (Context.getTargetInfo().getCXXABI().isMicrosoft() && IsCtorOrDtor) {
+// Issue a warning on ignored calling convention -- except of __stdcall.
+// Again, this is what MS compiler does.
+if (CurCC != CC_X86StdCall)
+  Diag(Loc, diag::warn_cconv_structors)
+  << FunctionType::getNameForCallConv(CurCC);
+  // Default adjustment.
+  } else {
+// Only adjust types with the default convention.  For example, on Windows
+// we should adjust a __cdecl type to __thiscall for instance methods, and a
+// __thiscall type to __cdecl for static methods.
+CallingConv DefaultCC =
+Context.getDefaultCallingConvention(IsVariadic, IsStatic);
+
+if (!IsCtorOrDtor) {
+  if (CurCC != DefaultCC || DefaultCC == ToCC)
+return;
+
+  if (hasExplicitCallingConv(T))
+return;
+}
+  }
+
   FT = Context.adjustFunctionType(FT, FT->getExtInfo().withCallingConv(ToCC));
   QualType Wrapped = Unwrapped.wrap(*this, FT);
   T = Context.getAdjustedType(T, Wrapped);
Index: lib/Sema/DeclSpec.cpp
===
--- lib/Sema/DeclSpec.cpp
+++ lib/Sema/DeclSpec.cpp
@@ -351,6 +351,11 @@
   getName().OperatorFunctionId.Operator));
 }
 
+bool Declarator::isCtorOrDtor() {
+  return (getName().getKind() == UnqualifiedId::IK_ConstructorName) ||
+ (getName().getKind() == UnqualifiedId::IK_DestructorName);
+}
+
 bool DeclSpec::hasTagDefinition() const {
   if (!TypeSpecOwned)
 return false;
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -7244,7 +7244,8 @@
   << DeclSpec::getSpecifierName(TSCS);
 
   if (D.isFirstDeclarationOfMember())
-adjustMemberFunctionCC(R, D.isStaticMember());
+adjustMemberFunctionCC(R, D.isStaticMember(), D.isCtorOrDtor(),
+   D.getIdentifierLoc());
 
   bool isFriend = false;
   FunctionTemplateDecl *FunctionTemplate = nullptr;
Index: include/clang/Sema/DeclSpec.h
===
--- include/clang/Sema/DeclSpec.h
+++ include/clang/Sema/DeclSpec.h
@@ -2208,6 +2208,9 @@
   /// redeclaration time if the decl is static.
   bool isStaticMember();
 
+  /// Returns true if this declares a constructor or a destructor.
+  bool isCtorOrDtor();
+
   void setRedeclaration(bool Val) { Redeclaration = Val; }
   bool isRedeclaration() const { return Redeclaration; }
 };
Index: include/clang/Sema/Sema.h