Re: [PATCH] D14980: PR18513: make gcc compatible layout for bit-fields with explicit aligned attribute

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin added a comment.

John and Richard,

I would like to proceed with this patch one way or another. If this patch 
cannot be accepted in upstream, I'll discard it. On the other hand I'm ready to 
improve this patch further if it is OK in principle but needs more work. Please 
let me know how you would like to proceed?

  Thanks,
  Dmitry


http://reviews.llvm.org/D14980



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

Re: [PATCH] D15888: [Analyzer] Change the default SA checkers for PS4

[Yury Gribov via cfe-commits]

ygribov added a subscriber: ygribov.
ygribov added a comment.

What's the problem with Vfork though?


http://reviews.llvm.org/D15888



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

[PATCH] D16047: [OpenCL] Add Sema checks for OpenCL 2.0

[Xiuli PAN via cfe-commits]

pxli168 created this revision.
pxli168 added reviewers: Anastasia, pekka.jaaskelainen.
pxli168 added subscribers: bader, cfe-commits.

Add Sema checks for opencl 2.0 new features: Block, pipe, atomic etc. Also fix 
some old Sema check like pointer to image.
This patch is based on bader's patch in SPIRV-1.0 branch. 

http://reviews.llvm.org/D16047

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenOpenCL/opencl_types.cl
  test/SemaOpenCL/invalid-block.cl
  test/SemaOpenCL/invalid-decl.cl

Index: test/SemaOpenCL/invalid-decl.cl
===
--- /dev/null
+++ test/SemaOpenCL/invalid-decl.cl
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -verify -cl-std=CL2.0 %s 
+typedef __attribute__(( ext_vector_type(4) )) int int4;
+#define ATOMIC_VAR_INIT 
+int; // expected-error {{declaration does not declare anything}}
+
+void test1(){
+  myfun(); // expected-error {{implicit declaration of function 'myfun' is invalid in OpenCL}}
+}
+
+void test2(image1d_t *i){} // expected-error {{pointer to image is invalid in OpenCL}}
+
+void test3() {
+  pipe int p; // expected-error {{pipe can only be used as a function parameter}}
+  image1d_t i; // expected-error {{image can only be used as a function parameter}}
+}
+
+void kernel test4() {
+  atomic_int guide = ATOMIC_VAR_INIT(42); // expected-error {{initialization of atomic variables is restricted to variables in global address space in opencl}}
+}
Index: test/SemaOpenCL/invalid-block.cl
===
--- /dev/null
+++ test/SemaOpenCL/invalid-block.cl
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -verify -fblocks -cl-std=CL2.0 %s
+
+int (^BlkVariadic)(int, ...) = ^int(int I, ...) { // expected-error {{invalid block prototype, variadic arguments are not allowed}}
+  return 0;
+};
+
+typedef int (^BlkInt)(int);
+void f1(int i) {
+  BlkInt B1 = ^int(int I) {return 1;};
+  BlkInt B2 = ^int(int I) {return 2;};
+  BlkInt Arr[] = {B1, B2}; // expected-error {{array of block is invalid in OpenCL}}
+  int tmp = i ? B1(i)  // expected-error {{blocks cannot be used as expressions in ternary expressions}}
+  : B2(i); // expected-error {{blocks cannot be used as expressions in ternary expressions}}
+}
+
+void f2(BlkInt *BlockPtr) {
+  BlkInt B = ^int(int I) {return 1;};
+  BlkInt *P = &B; // expected-error {{invalid argument type 'BlkInt' (aka 'int (^)(int)') to unary expression}}
+  B = *BlockPtr;  // expected-error {{dereferencing pointer of type '__generic BlkInt *' (aka 'int (^__generic *)(int)') is not allowed}}
+}
+
Index: test/CodeGenOpenCL/opencl_types.cl
===
--- test/CodeGenOpenCL/opencl_types.cl
+++ test/CodeGenOpenCL/opencl_types.cl
@@ -35,6 +35,3 @@
   fnc4smp(glb_smp);
 // CHECK: call {{.*}}void @fnc4smp(i32
 }
-
-void __attribute__((overloadable)) bad1(image1d_t *b, image2d_t *c, image2d_t *d) {}
-// CHECK-LABEL: @{{_Z4bad1P11ocl_image1dP11ocl_image2dS2_|"\\01\?bad1@@\$\$J0YAXPE?APAUocl_image1d@@PE?APAUocl_image2d@@1@Z"}}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2176,6 +2176,14 @@
 Diag(Loc, diag::warn_vla_used);
   }
 
+  // OpenCL v2.0 s6.12.5 -- The following Blocks features are currently not
+  // supported in OpenCL C: Arrays of Blocks.
+  if (getLangOpts().OpenCL && getLangOpts().OpenCLVersion >= 200 &&
+  Context.getBaseElementType(T)->isBlockPointerType()) {
+Diag(Loc, diag::err_opencl_invalid_block_array);
+return QualType();
+  }
+
   return T;
 }
 
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -6135,6 +6135,22 @@
   << Init->getSourceRange();
   }
 
+  // OpenCL v2.0 s6.13.1.1 -- This macro can only be used to initialize atomic
+  // objects that are declared in program scope in the global address space.
+  if (S.getLangOpts().OpenCL && S.getLangOpts().OpenCLVersion >= 200 &&
+  Entity.getType()->isAtomicType()) {
+Qualifiers TyQualifiers = Entity.getType().getQualifiers();
+bool HasGlobalAS = TyQualifiers.hasAddressSpace() &&
+   TyQualifiers.getAddressSpace() == LangAS::opencl_global;
+if (!HasGlobalAS && Entity.getKind() == InitializedEntity::EK_Variable &&
+Args.size() > 0) {
+  const Expr *Init = Args[0];
+  S.Diag(Init->getLocStart(), diag::err_opencl_atomic_init_addressspace)
+  << SourceRange(Entity.getDecl()->getLocStart(), Init->getLocEnd());
+  return ExprError();
+}
+  }
+
   // Diagnose cases where we initialize a pointer to an array temporary, and the
   // pointer obviously outlives the temporary.
   if (Args.size() == 1 && Args[0]->getType()->isArrayType() &&
Index: li

r257318 - AnalysisConsumer: use canonical decl for both lookup and store of

[Yury Gribov via cfe-commits]

Author: ygribov
Date: Mon Jan 11 03:38:48 2016
New Revision: 257318

URL: http://llvm.org/viewvc/llvm-project?rev=257318&view=rev
Log:
AnalysisConsumer: use canonical decl for both lookup and store of
visited decls.

Due to redeclarations, the function may have different declarations used
in CallExpr and in the definition. However, we need to use a unique
declaration for both store and lookup in VisitedCallees. This patch
fixes issues with analysis in topological order. A simple test is
included.

Patch by Alex Sidorin!

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

Added:
cfe/trunk/test/Analysis/inlining/analysis-order.c
Modified:
cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp?rev=257318&r1=257317&r2=257318&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp Mon Jan 11 
03:38:48 2016
@@ -496,10 +496,11 @@ void AnalysisConsumer::HandleDeclsCallGr
(Mgr->options.InliningMode == All ? nullptr : &VisitedCallees));
 
 // Add the visited callees to the global visited set.
-for (SetOfConstDecls::iterator I = VisitedCallees.begin(),
-   E = VisitedCallees.end(); I != E; ++I) {
-Visited.insert(*I);
-}
+for (const Decl *Callee : VisitedCallees)
+  // Decls from CallGraph are already canonical. But Decls coming from
+  // CallExprs may be not. We should canonicalize them manually.
+  Visited.insert(isa(Callee) ? Callee
+ : Callee->getCanonicalDecl());
 VisitedAsTopLevel.insert(D);
   }
 }

Added: cfe/trunk/test/Analysis/inlining/analysis-order.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/analysis-order.c?rev=257318&view=auto
==
--- cfe/trunk/test/Analysis/inlining/analysis-order.c (added)
+++ cfe/trunk/test/Analysis/inlining/analysis-order.c Mon Jan 11 03:38:48 2016
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core.builtin.NoReturnFunctions 
-analyzer-display-progress %s 2>&1 | FileCheck %s
+
+// Do not analyze test1() again because it was inlined
+void test1();
+
+void test2() {
+  test1();
+}
+
+void test1() {
+}
+
+// CHECK: analysis-order.c test2
+// CHECK-NEXT: analysis-order.c test1
+// CHECK-NEXT: analysis-order.c test2


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

Re: [PATCH] D15410: AnalysisConsumer: use canonical decl for both lookup and store of visited decls

[Yury Gribov via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL257318: AnalysisConsumer: use canonical decl for both lookup 
and store of (authored by ygribov).

Changed prior to commit:
  http://reviews.llvm.org/D15410?vs=43726&id=6#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D15410

Files:
  cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  cfe/trunk/test/Analysis/inlining/analysis-order.c

Index: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -496,10 +496,11 @@
(Mgr->options.InliningMode == All ? nullptr : &VisitedCallees));
 
 // Add the visited callees to the global visited set.
-for (SetOfConstDecls::iterator I = VisitedCallees.begin(),
-   E = VisitedCallees.end(); I != E; ++I) {
-Visited.insert(*I);
-}
+for (const Decl *Callee : VisitedCallees)
+  // Decls from CallGraph are already canonical. But Decls coming from
+  // CallExprs may be not. We should canonicalize them manually.
+  Visited.insert(isa(Callee) ? Callee
+ : Callee->getCanonicalDecl());
 VisitedAsTopLevel.insert(D);
   }
 }
Index: cfe/trunk/test/Analysis/inlining/analysis-order.c
===
--- cfe/trunk/test/Analysis/inlining/analysis-order.c
+++ cfe/trunk/test/Analysis/inlining/analysis-order.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core.builtin.NoReturnFunctions 
-analyzer-display-progress %s 2>&1 | FileCheck %s
+
+// Do not analyze test1() again because it was inlined
+void test1();
+
+void test2() {
+  test1();
+}
+
+void test1() {
+}
+
+// CHECK: analysis-order.c test2
+// CHECK-NEXT: analysis-order.c test1
+// CHECK-NEXT: analysis-order.c test2


Index: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -496,10 +496,11 @@
(Mgr->options.InliningMode == All ? nullptr : &VisitedCallees));
 
 // Add the visited callees to the global visited set.
-for (SetOfConstDecls::iterator I = VisitedCallees.begin(),
-   E = VisitedCallees.end(); I != E; ++I) {
-Visited.insert(*I);
-}
+for (const Decl *Callee : VisitedCallees)
+  // Decls from CallGraph are already canonical. But Decls coming from
+  // CallExprs may be not. We should canonicalize them manually.
+  Visited.insert(isa(Callee) ? Callee
+ : Callee->getCanonicalDecl());
 VisitedAsTopLevel.insert(D);
   }
 }
Index: cfe/trunk/test/Analysis/inlining/analysis-order.c
===
--- cfe/trunk/test/Analysis/inlining/analysis-order.c
+++ cfe/trunk/test/Analysis/inlining/analysis-order.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core.builtin.NoReturnFunctions -analyzer-display-progress %s 2>&1 | FileCheck %s
+
+// Do not analyze test1() again because it was inlined
+void test1();
+
+void test2() {
+  test1();
+}
+
+void test1() {
+}
+
+// CHECK: analysis-order.c test2
+// CHECK-NEXT: analysis-order.c test1
+// CHECK-NEXT: analysis-order.c test2
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15729: Load compiler plugins in ASTUnit, too

[Kevin Funk via cfe-commits]

kfunk updated this revision to Diff 9.
kfunk added a comment.

Update, add (non-working) test

Just uploading my WIP patch, now that the branching comes close. I added a 
test, unfortunately it doesn't do what I expect.
Please see the FIXME.

Didn't have the time to investigate yet, so any hints welcome!


http://reviews.llvm.org/D15729

Files:
  include/clang/Frontend/CompilerInstance.h
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/CompilerInstance.cpp
  lib/FrontendTool/ExecuteCompilerInvocation.cpp
  unittests/Frontend/CMakeLists.txt
  unittests/Frontend/FrontendActionTest.cpp
  unittests/Frontend/PrintFunctionNamesTestPlugin/CMakeLists.txt
  
unittests/Frontend/PrintFunctionNamesTestPlugin/PrintFunctionNamesTestPlugin.cpp

Index: unittests/Frontend/PrintFunctionNamesTestPlugin/PrintFunctionNamesTestPlugin.cpp
===
--- /dev/null
+++ unittests/Frontend/PrintFunctionNamesTestPlugin/PrintFunctionNamesTestPlugin.cpp
@@ -0,0 +1,123 @@
+//===- PrintFunctionNames.cpp -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Example clang plugin which simply prints the names of all the top-level decls
+// in the input file.
+//
+//===--===//
+
+#include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/AST/AST.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Sema/Sema.h"
+#include "llvm/Support/raw_ostream.h"
+using namespace clang;
+
+namespace {
+
+class PrintFunctionsConsumer : public ASTConsumer {
+  CompilerInstance &Instance;
+  std::set ParsedTemplates;
+
+public:
+  PrintFunctionsConsumer(CompilerInstance &Instance,
+ std::set ParsedTemplates)
+  : Instance(Instance), ParsedTemplates(ParsedTemplates) {}
+
+  bool HandleTopLevelDecl(DeclGroupRef DG) override {
+for (DeclGroupRef::iterator i = DG.begin(), e = DG.end(); i != e; ++i) {
+  const Decl *D = *i;
+  if (const NamedDecl *ND = dyn_cast(D))
+llvm::errs() << "top-level-decl: \"" << ND->getNameAsString() << "\"\n";
+}
+
+return true;
+  }
+
+  void HandleTranslationUnit(ASTContext& context) override {
+if (!Instance.getLangOpts().DelayedTemplateParsing)
+  return;
+
+// This demonstrates how to force instantiation of some templates in
+// -fdelayed-template-parsing mode. (Note: Doing this unconditionally for
+// all templates is similar to not using -fdelayed-template-parsig in the
+// first place.)
+// The advantage of doing this in HandleTranslationUnit() is that all
+// codegen (when using -add-plugin) is completely finished and this can't
+// affect the compiler output.
+struct Visitor : public RecursiveASTVisitor {
+  const std::set &ParsedTemplates;
+  Visitor(const std::set &ParsedTemplates)
+  : ParsedTemplates(ParsedTemplates) {}
+  bool VisitFunctionDecl(FunctionDecl *FD) {
+if (FD->isLateTemplateParsed() &&
+ParsedTemplates.count(FD->getNameAsString()))
+  LateParsedDecls.insert(FD);
+return true;
+  }
+
+  std::set LateParsedDecls;
+} v(ParsedTemplates);
+v.TraverseDecl(context.getTranslationUnitDecl());
+clang::Sema &sema = Instance.getSema();
+for (const FunctionDecl *FD : v.LateParsedDecls) {
+  clang::LateParsedTemplate* LPT = sema.LateParsedTemplateMap.lookup(FD);
+  sema.LateTemplateParser(sema.OpaqueParser, *LPT);
+  llvm::errs() << "late-parsed-decl: \"" << FD->getNameAsString() << "\"\n";
+}
+  }
+};
+
+class PrintFunctionNamesAction : public PluginASTAction {
+  std::set ParsedTemplates;
+protected:
+  std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
+ llvm::StringRef) override {
+return llvm::make_unique(CI, ParsedTemplates);
+  }
+
+  bool ParseArgs(const CompilerInstance &CI,
+ const std::vector &args) override {
+for (unsigned i = 0, e = args.size(); i != e; ++i) {
+  llvm::errs() << "PrintFunctionNames arg = " << args[i] << "\n";
+
+  // Example error handling.
+  DiagnosticsEngine &D = CI.getDiagnostics();
+  if (args[i] == "-an-error") {
+unsigned DiagID = D.getCustomDiagID(DiagnosticsEngine::Error,
+"invalid argument '%0'");
+D.Report(DiagID) << args[i];
+return false;
+  } else if (args[i] == "-parse-template") {
+if (i + 1 >= e) {
+  D.Report(D.getCustomDiagID(DiagnosticsEngine::Error,
+ "missing -parse-template argument

Re: [PATCH] D12901: [Static Analyzer] Assertion "System is over constrained" after truncating 64 bits integers to 32 bits. (PR25078)

[pierre gousseau via cfe-commits]

pgousseau added a comment.

In http://reviews.llvm.org/D12901#320680, @zaks.anna wrote:

> > This patch also fixes a bug in 'RangeSet::pin' causing single value ranges 
> > to not be considered conventionally ordered.
>
>
> Can that fix be submitted as a separate patch? Is there a test for it?


Yes I will look at creating a separate review for it.
Tests at lines 81, 111, 131 fail without the fix to RangeSet::pin.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:351
@@ -351,1 +350,3 @@
   }
+  case CK_IntegralCast: {
+// Delegate to SValBuilder to process.

zaks.anna wrote:
> SValBuilder::evalCast and SimpleSValBuilder::evalCastFromNonLoc perform a lot 
> of special casing. I am not sure we are not loosing anything if we bypass 
> them. For example, there is special handling of Booleans. We might want to 
> add this smarter handling of the integral conversions inside 
> SimpleSValBuilder::evalCastFromNonLoc, where you see the comment starting 
> with "If the types are the same or both are integers, ignore the cast."
Yes I initially looked at making the change in 'evalCastFromNonLoc' but the 
problem is that the change requires access to the ProgramState, so to avoid 
changing the interface of evalCast (used in around 50 places) I made the change 
here.
Looking at evalCast it does not seem to add any special handling to casts of 
type 'CK_IntegralCast' before calling 'evalCastFromNonLoc'?
Booleans casts should be associated with another type of cast than 
'CK_IntegralCast' and the new 'evalIntegralCast' will call 'evalCast' if it 
does not detect a truncation so it should be ok, what do you think?


http://reviews.llvm.org/D12901



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

Re: [PATCH] D15743: Fix assert hit when tree-transforming template template parameter packs.

[Manuel Klimek via cfe-commits]

klimek updated this revision to Diff 44451.
klimek added a comment.

Expand test.


http://reviews.llvm.org/D15743

Files:
  lib/Sema/TreeTransform.h
  test/SemaTemplate/temp_arg_template.cpp

Index: test/SemaTemplate/temp_arg_template.cpp
===
--- test/SemaTemplate/temp_arg_template.cpp
+++ test/SemaTemplate/temp_arg_template.cpp
@@ -75,7 +75,11 @@
 // expected-warning@-2 {{variadic templates are a C++11 extension}}
 #endif
 
-struct template_tuple {};
+struct template_tuple {
+#if __cplusplus >= 201103L
+  static constexpr int N = sizeof...(Templates);
+#endif
+};
 template 
 struct identity {};
 template  class... Templates>
@@ -85,6 +89,12 @@
 
 template_tuple f7() {}
 
+#if __cplusplus >= 201103L
+struct S : public template_tuple {
+  static_assert(N == 2, "Number of template arguments incorrect");
+};
+#endif
+
 void foo() {
   f7();
 }
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -3583,7 +3583,7 @@
   case TemplateArgument::Template:
   case TemplateArgument::TemplateExpansion: {
 NestedNameSpecifierLocBuilder Builder;
-TemplateName Template = Arg.getAsTemplate();
+TemplateName Template = Arg.getAsTemplateOrTemplatePattern();
 if (DependentTemplateName *DTN = Template.getAsDependentTemplateName())
   Builder.MakeTrivial(SemaRef.Context, DTN->getQualifier(), Loc);
 else if (QualifiedTemplateName *QTN = 
Template.getAsQualifiedTemplateName())


Index: test/SemaTemplate/temp_arg_template.cpp
===
--- test/SemaTemplate/temp_arg_template.cpp
+++ test/SemaTemplate/temp_arg_template.cpp
@@ -75,7 +75,11 @@
 // expected-warning@-2 {{variadic templates are a C++11 extension}}
 #endif
 
-struct template_tuple {};
+struct template_tuple {
+#if __cplusplus >= 201103L
+  static constexpr int N = sizeof...(Templates);
+#endif
+};
 template 
 struct identity {};
 template  class... Templates>
@@ -85,6 +89,12 @@
 
 template_tuple f7() {}
 
+#if __cplusplus >= 201103L
+struct S : public template_tuple {
+  static_assert(N == 2, "Number of template arguments incorrect");
+};
+#endif
+
 void foo() {
   f7();
 }
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -3583,7 +3583,7 @@
   case TemplateArgument::Template:
   case TemplateArgument::TemplateExpansion: {
 NestedNameSpecifierLocBuilder Builder;
-TemplateName Template = Arg.getAsTemplate();
+TemplateName Template = Arg.getAsTemplateOrTemplatePattern();
 if (DependentTemplateName *DTN = Template.getAsDependentTemplateName())
   Builder.MakeTrivial(SemaRef.Context, DTN->getQualifier(), Loc);
 else if (QualifiedTemplateName *QTN = Template.getAsQualifiedTemplateName())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15743: Fix assert hit when tree-transforming template template parameter packs.

[Manuel Klimek via cfe-commits]

klimek added a reviewer: bkramer.


Comment at: test/SemaTemplate/temp_arg_template.cpp:80
@@ +79,3 @@
+#if __cplusplus >= 201103L
+  static constexpr int N = sizeof...(Templates);
+#endif

dblaikie wrote:
> It would be good if we tested for some specific behavior here other than 
> "don't crash" (doing anything other than crashing is a bit underspecified) - 
> maybe we should be testing that N has the right value?
> 
> (don't have quite enough context on the fix itself to sign off on it though, 
> unfortunately :/)
Done.


http://reviews.llvm.org/D15743



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

[clang-tools-extra] r257320 - [clang-tidy] Fix a false positive in google-runtime-memset

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Mon Jan 11 04:26:29 2016
New Revision: 257320

URL: http://llvm.org/viewvc/llvm-project?rev=257320&view=rev
Log:
[clang-tidy] Fix a false positive in google-runtime-memset

google-runtime-memset no longer issues a warning if the fill char value
is known to be an invalid fill char count.

Namely, it no longer warns for these:

  memset(p, 0, 0);
  memset(p, -1, 0);

In both cases, swapping the last two args would either be useless (there is
no actual bug) or wrong (it would introduce a bug).

Patch by Matt Armstrong!

Modified:
clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp

clang-tools-extra/trunk/test/clang-tidy/google-runtime-memset-zero-length.cpp

Modified: clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp?rev=257320&r1=257319&r2=257320&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp Mon Jan 
11 04:26:29 2016
@@ -51,25 +51,29 @@ static StringRef getAsString(const Match
 void MemsetZeroLengthCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Call = Result.Nodes.getNodeAs("decl");
 
+  // Note, this is:
+  // void *memset(void *buffer, int fill_char, size_t byte_count);
+  // Arg1 is fill_char, Arg2 is byte_count.
   const Expr *Arg1 = Call->getArg(1);
   const Expr *Arg2 = Call->getArg(2);
 
-  // Try to evaluate the second argument so we can also find values that are 
not
-  // just literals.
+  // Return if `byte_count` is not zero at compile time.
   llvm::APSInt Value1, Value2;
   if (Arg2->isValueDependent() ||
   !Arg2->EvaluateAsInt(Value2, *Result.Context) || Value2 != 0)
 return;
 
-  // If both arguments evaluate to zero emit a warning without fix suggestions.
+  // Return if `fill_char` is known to be zero or negative at compile
+  // time. In these cases, swapping the args would be a nop, or
+  // introduce a definite bug. The code is likely correct.
   if (!Arg1->isValueDependent() &&
   Arg1->EvaluateAsInt(Value1, *Result.Context) &&
-  (Value1 == 0 || Value1.isNegative())) {
-diag(Call->getLocStart(), "memset of size zero");
+  (Value1 == 0 || Value1.isNegative()))
 return;
-  }
 
-  // Emit a warning and fix-its to swap the arguments.
+  // `byte_count` is known to be zero at compile time, and `fill_char` is
+  // either not known or known to be a positive integer. Emit a warning
+  // and fix-its to swap the arguments.
   auto D = diag(Call->getLocStart(),
 "memset of size zero, potentially swapped arguments");
   SourceRange LHSRange = Arg1->getSourceRange();

Modified: 
clang-tools-extra/trunk/test/clang-tidy/google-runtime-memset-zero-length.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-runtime-memset-zero-length.cpp?rev=257320&r1=257319&r2=257320&view=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/google-runtime-memset-zero-length.cpp 
(original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/google-runtime-memset-zero-length.cpp 
Mon Jan 11 04:26:29 2016
@@ -48,13 +48,15 @@ void foo(void *a, int xsize, int ysize)
 
   memset(a, -1, sizeof(int));
   memset(a, 0xcd, 1);
+
+  // Don't warn when the fill char and the length are both known to be
+  // zero.  No bug is possible.
+  memset(a, 0, v);
   memset(a, v, 0);
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero
-// CHECK-FIXES: memset(a, v, 0);
 
+  // -1 is clearly not a length by virtue of being negative, so no warning
+  // despite v == 0.
   memset(a, -1, v);
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero
-// CHECK-FIXES: memset(a, -1, v);
 
   memtmpl<0, int>();
 }


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

r257325 - clang-format: Slightly row back on r257257.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Mon Jan 11 05:01:05 2016
New Revision: 257325

URL: http://llvm.org/viewvc/llvm-project?rev=257325&view=rev
Log:
clang-format: Slightly row back on r257257.

r257257 change the way clang-format enforces line breaks after a
templated type has been line-wrapped. This was to fix an incorrect line
break if BinPackParameters is set to false. However, it also leads to
an unwanted line break in a different case. Thus, for now, only do this
when BinPackParameters is false. This isn't ideal yet, but helps us
until we have a better solution.

With BinPackParameters:
Before:
  void fff(aaa aa);

After:
  void fff(
  aaa
  aa);

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

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=257325&r1=257324&r2=257325&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Mon Jan 11 05:01:05 2016
@@ -151,7 +151,11 @@ bool ContinuationIndenter::mustBreak(con
 return true;
   if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) ||
(Previous.is(TT_TemplateCloser) && Current.is(TT_StartOfName) &&
-Previous.NestingLevel == 1) ||
+// FIXME: This is a temporary workaround for the case where 
clang-format
+// sets BreakBeforeParameter to avoid bin packing and this creates a
+// completely unnecessary line break after a template type that isn't
+// line-wrapped.
+(Previous.NestingLevel == 1 || Style.BinPackParameters)) ||
(Style.BreakBeforeTernaryOperators && Current.is(TT_ConditionalExpr) &&
 Previous.isNot(tok::question)) ||
(!Style.BreakBeforeTernaryOperators &&

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=257325&r1=257324&r2=257325&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jan 11 05:01:05 2016
@@ -4062,10 +4062,23 @@ TEST_F(FormatTest, FormatsDeclarationsOn
"   int ,\n"
"   int aaa) {}",
NoBinPacking);
+
   NoBinPacking.AllowAllParametersOfDeclarationOnNextLine = false;
   verifyFormat("void aa(,\n"
"vector bbb);",
NoBinPacking);
+  // FIXME: This behavior difference is probably not wanted. However, currently
+  // we cannot distinguish BreakBeforeParameter being set because of the 
wrapped
+  // template arguments from BreakBeforeParameter being set because of the
+  // one-per-line formatting.
+  verifyFormat(
+  "void fff(aaa aa);",
+  NoBinPacking);
+  verifyFormat(
+  "void fff(\n"
+  "aaa\n"
+  "aa);");
 }
 
 TEST_F(FormatTest, FormatsOneParameterPerLineIfNecessary) {


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

r257324 - clang-format: [JS] Improve line-flow when calling functions on array literals.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Mon Jan 11 05:00:58 2016
New Revision: 257324

URL: http://llvm.org/viewvc/llvm-project?rev=257324&view=rev
Log:
clang-format: [JS] Improve line-flow when calling functions on array literals.

Before:
  return [
aa
  ].aaa(function() {
 //
   })
   .bb();

After:
  return [aa]
  .aaa(function() {
//
  })
  .bb();

Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=257324&r1=257323&r2=257324&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Mon Jan 11 05:00:58 2016
@@ -178,13 +178,15 @@ bool ContinuationIndenter::mustBreak(con
 return true;
 
   unsigned NewLineColumn = getNewLineColumn(State);
-  if (State.Column <= NewLineColumn)
-return false;
-
   if (Current.isMemberAccess() &&
-  State.Column + getLengthToNextOperator(Current) > Style.ColumnLimit)
+  State.Column + getLengthToNextOperator(Current) > Style.ColumnLimit &&
+  (State.Column > NewLineColumn ||
+   Current.NestingLevel < State.StartOfLineLevel))
 return true;
 
+  if (State.Column <= NewLineColumn)
+return false;
+
   if (Style.AlwaysBreakBeforeMultilineStrings &&
   (NewLineColumn == State.FirstIndent + Style.ContinuationIndentWidth ||
Previous.is(tok::comma) || Current.NestingLevel < 2) &&

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=257324&r1=257323&r2=257324&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Mon Jan 11 05:00:58 2016
@@ -530,6 +530,12 @@ TEST_F(FormatTestJS, MultipleFunctionLit
   verifyFormat("getSomeLongPromise()\n"
".then(function(value) { body(); })\n"
".thenCatch(function(error) { body(); });");
+
+  verifyFormat("return [aa]\n"
+   ".aaa(function() {\n"
+   "  //\n"
+   "})\n"
+   ".bb();");
 }
 
 TEST_F(FormatTestJS, ArrowFunctions) {


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

Re: [PATCH] D16044: getVariableName() for MemRegion

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

xazax.hun added inline comments.


Comment at: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:643
@@ +642,3 @@
+  const clang::ento::MemRegion *R = this;
+  llvm::SmallString<200> buf;
+  llvm::raw_svector_ostream os(buf);

Do you think 200 is a reasonable size here? In case the namespaces are not 
included in the printed name, I think it is very rare that a name would be 
longer than 100 characters.


Comment at: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:658
@@ +657,3 @@
+
+R = R->getAs()->getSuperRegion();
+  }

Can you use ER here instead of additional getAs?


http://reviews.llvm.org/D16044



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

Re: [PATCH] D15743: Fix assert hit when tree-transforming template template parameter packs.

[Benjamin Kramer via cfe-commits]

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

This looks good. getAsTemplateOrTemplatePattern mirrors the existing switch in 
TreeTransform.


http://reviews.llvm.org/D15743



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

Re: [PATCH] D15120: Add support for __float128 type to be used by targets that support it

[Nemanja Ivanovic via cfe-commits]

nemanjai updated the summary for this revision.
nemanjai updated this revision to Diff 44459.
nemanjai added a comment.

Addressed the review comments:

- Added the requested test cases
- Disabled promotion of long double to __float128
- Disabled operations between long double and __float128 if they have different 
representations


Repository:
  rL LLVM

http://reviews.llvm.org/D15120

Files:
  bindings/python/clang/cindex.py
  include/clang-c/Index.h
  include/clang/AST/ASTContext.h
  include/clang/AST/BuiltinTypes.def
  include/clang/AST/Type.h
  include/clang/AST/TypeLoc.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/Specifiers.h
  include/clang/Basic/TargetInfo.h
  include/clang/Basic/TokenKinds.def
  include/clang/Driver/Options.td
  include/clang/Lex/LiteralSupport.h
  include/clang/Sema/DeclSpec.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/AST/NSAPI.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/Type.cpp
  lib/AST/TypeLoc.cpp
  lib/Analysis/PrintfFormatString.cpp
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Format/FormatToken.cpp
  lib/Frontend/InitPreprocessor.cpp
  lib/Index/USRGeneration.cpp
  lib/Lex/LiteralSupport.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseTentative.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateVariadic.cpp
  lib/Sema/SemaType.cpp
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReader.cpp
  test/CodeGenCXX/float128-declarations.cpp
  test/Preprocessor/init.c
  test/Sema/128bitfloat.cpp
  test/Sema/float128-ld-incompatibility.cpp
  test/SemaCXX/deleted-operator.cpp
  test/SemaCXX/overloaded-builtin-operators.cpp
  tools/libclang/CXType.cpp

Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -51,6 +51,7 @@
 BTCASE(Float);
 BTCASE(Double);
 BTCASE(LongDouble);
+BTCASE(Float128);
 BTCASE(NullPtr);
 BTCASE(Overload);
 BTCASE(Dependent);
@@ -466,6 +467,7 @@
 TKIND(Float);
 TKIND(Double);
 TKIND(LongDouble);
+TKIND(Float128);
 TKIND(NullPtr);
 TKIND(Overload);
 TKIND(Dependent);
Index: test/SemaCXX/overloaded-builtin-operators.cpp
===
--- test/SemaCXX/overloaded-builtin-operators.cpp
+++ test/SemaCXX/overloaded-builtin-operators.cpp
@@ -183,7 +183,7 @@
   // FIXME: lots of candidates here!
   (void)(1.0f * a); // expected-error{{ambiguous}} \
 // expected-note 4{{candidate}} \
-// expected-note {{remaining 117 candidates omitted; pass -fshow-overloads=all to show them}}
+// expected-note {{remaining 140 candidates omitted; pass -fshow-overloads=all to show them}}
 }
 
 // pr5432
Index: test/SemaCXX/deleted-operator.cpp
===
--- test/SemaCXX/deleted-operator.cpp
+++ test/SemaCXX/deleted-operator.cpp
@@ -9,7 +9,7 @@
   PR10757 a1;
   // FIXME: We get a ridiculous number of "built-in candidate" notes here...
   if(~a1) {} // expected-error {{overload resolution selected deleted operator}} expected-note 8 {{built-in candidate}}
-  if(a1==a1) {} // expected-error {{overload resolution selected deleted operator}} expected-note 121 {{built-in candidate}}
+  if(a1==a1) {} // expected-error {{overload resolution selected deleted operator}} expected-note 144 {{built-in candidate}}
 }
 
 struct DelOpDel {
Index: test/Sema/float128-ld-incompatibility.cpp
===
--- test/Sema/float128-ld-incompatibility.cpp
+++ test/Sema/float128-ld-incompatibility.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 \
+// RUN: -triple powerpc64le-unknown-linux-gnu -target-cpu pwr7 \
+// RUN: -target-feature +float128 %s
+
+__float128 qf();
+long double ldf();
+
+long double ld{qf()}; // expected-error {{non-constant-expression cannot be narrowed from type '__float128' to 'long double' in initializer list}} \
+ expected-note {{insert an explicit cast to silence this issue}}
+__float128 q{ldf()};  // passes
+
+auto test1(__float128 q, long double ld) -> decltype(q + ld) { // expected-error {{invalid operands to binary expression ('__float128' and 'long double')}}
+  return q + ld;  // expected-error {{invalid operands to binary expression ('__float128' and 'long double')}}
+}
+
+auto test2(long double a, __float128 b) -> decltype(a + b) { // expected-error {{invalid operands to binary expression ('long double' and '__float128')}}
+  return a + b;   

Re: [PATCH] D15743: Fix assert hit when tree-transforming template template parameter packs.

[Manuel Klimek via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL257326: Fix assert hit when tree-transforming template 
template parameter packs. (authored by klimek).

Changed prior to commit:
  http://reviews.llvm.org/D15743?vs=44451&id=44460#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D15743

Files:
  cfe/trunk/lib/Sema/TreeTransform.h
  cfe/trunk/test/SemaTemplate/temp_arg_template.cpp

Index: cfe/trunk/test/SemaTemplate/temp_arg_template.cpp
===
--- cfe/trunk/test/SemaTemplate/temp_arg_template.cpp
+++ cfe/trunk/test/SemaTemplate/temp_arg_template.cpp
@@ -75,7 +75,11 @@
 // expected-warning@-2 {{variadic templates are a C++11 extension}}
 #endif
 
-struct template_tuple {};
+struct template_tuple {
+#if __cplusplus >= 201103L
+  static constexpr int N = sizeof...(Templates);
+#endif
+};
 template 
 struct identity {};
 template  class... Templates>
@@ -85,6 +89,12 @@
 
 template_tuple f7() {}
 
+#if __cplusplus >= 201103L
+struct S : public template_tuple {
+  static_assert(N == 2, "Number of template arguments incorrect");
+};
+#endif
+
 void foo() {
   f7();
 }
Index: cfe/trunk/lib/Sema/TreeTransform.h
===
--- cfe/trunk/lib/Sema/TreeTransform.h
+++ cfe/trunk/lib/Sema/TreeTransform.h
@@ -3583,7 +3583,7 @@
   case TemplateArgument::Template:
   case TemplateArgument::TemplateExpansion: {
 NestedNameSpecifierLocBuilder Builder;
-TemplateName Template = Arg.getAsTemplate();
+TemplateName Template = Arg.getAsTemplateOrTemplatePattern();
 if (DependentTemplateName *DTN = Template.getAsDependentTemplateName())
   Builder.MakeTrivial(SemaRef.Context, DTN->getQualifier(), Loc);
 else if (QualifiedTemplateName *QTN = 
Template.getAsQualifiedTemplateName())


Index: cfe/trunk/test/SemaTemplate/temp_arg_template.cpp
===
--- cfe/trunk/test/SemaTemplate/temp_arg_template.cpp
+++ cfe/trunk/test/SemaTemplate/temp_arg_template.cpp
@@ -75,7 +75,11 @@
 // expected-warning@-2 {{variadic templates are a C++11 extension}}
 #endif
 
-struct template_tuple {};
+struct template_tuple {
+#if __cplusplus >= 201103L
+  static constexpr int N = sizeof...(Templates);
+#endif
+};
 template 
 struct identity {};
 template  class... Templates>
@@ -85,6 +89,12 @@
 
 template_tuple f7() {}
 
+#if __cplusplus >= 201103L
+struct S : public template_tuple {
+  static_assert(N == 2, "Number of template arguments incorrect");
+};
+#endif
+
 void foo() {
   f7();
 }
Index: cfe/trunk/lib/Sema/TreeTransform.h
===
--- cfe/trunk/lib/Sema/TreeTransform.h
+++ cfe/trunk/lib/Sema/TreeTransform.h
@@ -3583,7 +3583,7 @@
   case TemplateArgument::Template:
   case TemplateArgument::TemplateExpansion: {
 NestedNameSpecifierLocBuilder Builder;
-TemplateName Template = Arg.getAsTemplate();
+TemplateName Template = Arg.getAsTemplateOrTemplatePattern();
 if (DependentTemplateName *DTN = Template.getAsDependentTemplateName())
   Builder.MakeTrivial(SemaRef.Context, DTN->getQualifier(), Loc);
 else if (QualifiedTemplateName *QTN = Template.getAsQualifiedTemplateName())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r257326 - Fix assert hit when tree-transforming template template parameter packs.

[Manuel Klimek via cfe-commits]

Author: klimek
Date: Mon Jan 11 05:39:00 2016
New Revision: 257326

URL: http://llvm.org/viewvc/llvm-project?rev=257326&view=rev
Log:
Fix assert hit when tree-transforming template template parameter packs.

Covers significantly more code in the template template pack argument
test and fixes the resulting assert problem.

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

Modified:
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/test/SemaTemplate/temp_arg_template.cpp

Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=257326&r1=257325&r2=257326&view=diff
==
--- cfe/trunk/lib/Sema/TreeTransform.h (original)
+++ cfe/trunk/lib/Sema/TreeTransform.h Mon Jan 11 05:39:00 2016
@@ -3583,7 +3583,7 @@ void TreeTransform::InventTempl
   case TemplateArgument::Template:
   case TemplateArgument::TemplateExpansion: {
 NestedNameSpecifierLocBuilder Builder;
-TemplateName Template = Arg.getAsTemplate();
+TemplateName Template = Arg.getAsTemplateOrTemplatePattern();
 if (DependentTemplateName *DTN = Template.getAsDependentTemplateName())
   Builder.MakeTrivial(SemaRef.Context, DTN->getQualifier(), Loc);
 else if (QualifiedTemplateName *QTN = 
Template.getAsQualifiedTemplateName())

Modified: cfe/trunk/test/SemaTemplate/temp_arg_template.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/temp_arg_template.cpp?rev=257326&r1=257325&r2=257326&view=diff
==
--- cfe/trunk/test/SemaTemplate/temp_arg_template.cpp (original)
+++ cfe/trunk/test/SemaTemplate/temp_arg_template.cpp Mon Jan 11 05:39:00 2016
@@ -75,7 +75,11 @@ template  class... T
 // expected-warning@-2 {{variadic templates are a C++11 extension}}
 #endif
 
-struct template_tuple {};
+struct template_tuple {
+#if __cplusplus >= 201103L
+  static constexpr int N = sizeof...(Templates);
+#endif
+};
 template 
 struct identity {};
 template  class... Templates>
@@ -85,6 +89,12 @@ template  class... T
 
 template_tuple f7() {}
 
+#if __cplusplus >= 201103L
+struct S : public template_tuple {
+  static_assert(N == 2, "Number of template arguments incorrect");
+};
+#endif
+
 void foo() {
   f7();
 }


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

[PATCH] D16056: [Gold] Pass -mllvm options to the gold plugin

[James Molloy via cfe-commits]

jmolloy created this revision.
jmolloy added a reviewer: rafael.
jmolloy added a subscriber: cfe-commits.
jmolloy set the repository for this revision to rL LLVM.
Herald added a subscriber: joker.eph.

The gold plugin can take LLVM options, but the syntax is confusing: 
-Wl,-plugin-opt= or -Xlinker -plugin-opt=.

Instead, pass -mllvm options through to Gold so the obvious syntax works.

Repository:
  rL LLVM

http://reviews.llvm.org/D16056

Files:
  lib/Driver/Tools.cpp
  test/Driver/gold-lto.c

Index: test/Driver/gold-lto.c
===
--- test/Driver/gold-lto.c
+++ test/Driver/gold-lto.c
@@ -16,11 +16,14 @@
 // CHECK-X86-64-COREI7: "-plugin-opt=foo"
 //
 // RUN: %clang -target arm-unknown-linux -### %t.o -flto 2>&1 \
-// RUN: -march=armv7a -Wl,-plugin-opt=foo -O0 \
+// RUN: -march=armv7a -Wl,-plugin-opt=foo -O0 -mllvm -bar=baz \
+// RUN: -mllvm -test-option \
 // RUN: | FileCheck %s --check-prefix=CHECK-ARM-V7A
 // CHECK-ARM-V7A: "-plugin" "{{.*}}/LLVMgold.so"
 // CHECK-ARM-V7A: "-plugin-opt=mcpu=cortex-a8"
 // CHECK-ARM-V7A: "-plugin-opt=O0"
+// CHECK-ARM-V7A: "-plugin-opt=-bar=baz"
+// CHECK-ARM-V7A: "-plugin-opt=-test-option"
 // CHECK-ARM-V7A: "-plugin-opt=foo"
 //
 // RUN: %clang -target i686-linux-android -### %t.o -flto 2>&1 \
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -1817,6 +1817,12 @@
 
   if (IsThinLTO)
 CmdArgs.push_back("-plugin-opt=thinlto");
+
+  // Claim and pass through -mllvm options to the Gold plugin.
+  for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
+A->claim();
+CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=") + 
A->getValue(0)));
+  }
 }
 
 /// This is a helper function for validating the optional refinement step


Index: test/Driver/gold-lto.c
===
--- test/Driver/gold-lto.c
+++ test/Driver/gold-lto.c
@@ -16,11 +16,14 @@
 // CHECK-X86-64-COREI7: "-plugin-opt=foo"
 //
 // RUN: %clang -target arm-unknown-linux -### %t.o -flto 2>&1 \
-// RUN: -march=armv7a -Wl,-plugin-opt=foo -O0 \
+// RUN: -march=armv7a -Wl,-plugin-opt=foo -O0 -mllvm -bar=baz \
+// RUN: -mllvm -test-option \
 // RUN: | FileCheck %s --check-prefix=CHECK-ARM-V7A
 // CHECK-ARM-V7A: "-plugin" "{{.*}}/LLVMgold.so"
 // CHECK-ARM-V7A: "-plugin-opt=mcpu=cortex-a8"
 // CHECK-ARM-V7A: "-plugin-opt=O0"
+// CHECK-ARM-V7A: "-plugin-opt=-bar=baz"
+// CHECK-ARM-V7A: "-plugin-opt=-test-option"
 // CHECK-ARM-V7A: "-plugin-opt=foo"
 //
 // RUN: %clang -target i686-linux-android -### %t.o -flto 2>&1 \
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -1817,6 +1817,12 @@
 
   if (IsThinLTO)
 CmdArgs.push_back("-plugin-opt=thinlto");
+
+  // Claim and pass through -mllvm options to the Gold plugin.
+  for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
+A->claim();
+CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=") + A->getValue(0)));
+  }
 }
 
 /// This is a helper function for validating the optional refinement step
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r257327 - [WebAssembly] Fix a typo in a comment.

[Dan Gohman via cfe-commits]

Author: djg
Date: Mon Jan 11 05:49:44 2016
New Revision: 257327

URL: http://llvm.org/viewvc/llvm-project?rev=257327&view=rev
Log:
[WebAssembly] Fix a typo in a comment.

Modified:
cfe/trunk/test/Driver/wasm-toolchain.c

Modified: cfe/trunk/test/Driver/wasm-toolchain.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/wasm-toolchain.c?rev=257327&r1=257326&r2=257327&view=diff
==
--- cfe/trunk/test/Driver/wasm-toolchain.c (original)
+++ cfe/trunk/test/Driver/wasm-toolchain.c Mon Jan 11 05:49:44 2016
@@ -18,7 +18,7 @@
 // NO_DATA_SECTIONS-NOT: data-sections
 
 // Ditto, but ensure that a user -fvisibility=default disables the default
-// -fvisibilt=hidden.
+// -fvisibility=hidden.
 
 // RUN: %clang %s -### -target wasm32-unknown-unknown -fvisibility=default 
2>&1 | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
 // FVISIBILITY_DEFAULT-NOT: hidden


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

r257330 - clang-format: Fix overloading "operator, " definitions more thoroughly.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Mon Jan 11 06:55:33 2016
New Revision: 257330

URL: http://llvm.org/viewvc/llvm-project?rev=257330&view=rev
Log:
clang-format: Fix overloading "operator," definitions more thoroughly.

Before:
  aa operator,(a &
   aa) const;
After:
  aa operator,(
  a &aa) const;

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

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=257330&r1=257329&r2=257330&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Jan 11 06:55:33 2016
@@ -119,7 +119,9 @@ private:
   }
 }
 
-if (Left->Previous &&
+if (Left->is(TT_OverloadedOperatorLParen)) {
+  Contexts.back().IsExpression = false;
+} else if (Left->Previous &&
 (Left->Previous->isOneOf(tok::kw_static_assert, tok::kw_decltype,
  tok::kw_if, tok::kw_while, tok::l_paren,
  tok::comma) ||
@@ -132,9 +134,7 @@ private:
   // This is a parameter list of a lambda expression.
   Contexts.back().IsExpression = false;
 } else if (Line.InPPDirective &&
-   (!Left->Previous ||
-!Left->Previous->isOneOf(tok::identifier,
- TT_OverloadedOperator))) {
+   (!Left->Previous || !Left->Previous->is(tok::identifier))) {
   Contexts.back().IsExpression = true;
 } else if (Contexts[Contexts.size() - 2].CaretFound) {
   // This is the parameter list of an ObjC block.

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=257330&r1=257329&r2=257330&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jan 11 06:55:33 2016
@@ -5492,7 +5492,7 @@ TEST_F(FormatTest, UnderstandsOverloaded
   verifyFormat("template \n"
"AAA operator/(const AAA &a, BBB &b);");
   verifyFormat("aa operator,(\n"
-   "a) 
const;");
+   "a &aa) 
const;");
 
   verifyFormat(
   "ostream &operator<<(ostream &OutputStream,\n"


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

[PATCH] D16058: [clang-format] Fix comment aligning when there are changes within the comment

[Benjamin Kramer via cfe-commits]

bkramer created this revision.
bkramer added a reviewer: djasper.
bkramer added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

As soon as a comment had whitespace changes inside of the token, we
couldn't identify the whole comment as a trailing comment anymore and
alignment stopped working. Add a new boolean to Change for this special
case and fix trailing comment identification to use it.

Before this fix

int xy;  // a
int z;   //b

became

int xy;  // a
int z;  // b

with this patch we immediately get to:

int xy;  // a
int z;   // b

http://reviews.llvm.org/D16058

Files:
  lib/Format/WhitespaceManager.cpp
  lib/Format/WhitespaceManager.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1022,6 +1022,10 @@
"  lineWith(); // comment\n"
"  // at start\n"
"}"));
+  EXPECT_EQ("int xy; // a\n"
+"int z;  // b",
+format("int xy;// a\n"
+   "int z;//b"));
 
   verifyFormat("#define A  \\\n"
"  int i; /* i */   \\\n"
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -109,7 +109,8 @@
unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
unsigned NewlinesBefore, StringRef PreviousLinePostfix,
StringRef CurrentLinePrefix, tok::TokenKind Kind,
-   bool ContinuesPPDirective, bool IsStartOfDeclName);
+   bool ContinuesPPDirective, bool IsStartOfDeclName,
+   bool IsTrailingCommentContinuation);
 
 bool CreateReplacement;
 // Changes might be in the middle of a token, so we cannot just keep the
@@ -139,6 +140,10 @@
 // comments. Uncompensated negative offset is truncated to 0.
 int Spaces;
 
+// If this change is inside of a line comment but not at the start of such a
+// comment.
+bool IsTrailingCommentContinuation;
+
 // \c IsTrailingComment, \c TokenLength, \c PreviousEndOfTokenColumn and
 // \c EscapedNewlineColumn will be calculated in
 // \c calculateLineBreakInformation.
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -30,17 +30,19 @@
 unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
 unsigned NewlinesBefore, StringRef PreviousLinePostfix,
 StringRef CurrentLinePrefix, tok::TokenKind Kind, bool ContinuesPPDirective,
-bool IsStartOfDeclName)
+bool IsStartOfDeclName, bool IsTrailingCommentContinuation)
 : CreateReplacement(CreateReplacement),
   OriginalWhitespaceRange(OriginalWhitespaceRange),
   StartOfTokenColumn(StartOfTokenColumn), NewlinesBefore(NewlinesBefore),
   PreviousLinePostfix(PreviousLinePostfix),
   CurrentLinePrefix(CurrentLinePrefix), Kind(Kind),
   ContinuesPPDirective(ContinuesPPDirective),
   IsStartOfDeclName(IsStartOfDeclName), IndentLevel(IndentLevel),
-  Spaces(Spaces), IsTrailingComment(false), TokenLength(0),
-  PreviousEndOfTokenColumn(0), EscapedNewlineColumn(0),
-  StartOfBlockComment(nullptr), IndentationOffset(0) {}
+  Spaces(Spaces),
+  IsTrailingCommentContinuation(IsTrailingCommentContinuation),
+  IsTrailingComment(false), TokenLength(0), PreviousEndOfTokenColumn(0),
+  EscapedNewlineColumn(0), StartOfBlockComment(nullptr),
+  IndentationOffset(0) {}
 
 void WhitespaceManager::reset() {
   Changes.clear();
@@ -58,7 +60,8 @@
   Change(/*CreateReplacement=*/true, Tok.WhitespaceRange, IndentLevel,
  Spaces, StartOfTokenColumn, Newlines, "", "", Tok.Tok.getKind(),
  InPPDirective && !Tok.IsFirst,
- Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+ Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+ /*IsTrailingCommentContinuation=*/false));
 }
 
 void WhitespaceManager::addUntouchableToken(const FormatToken &Tok,
@@ -69,7 +72,8 @@
   /*CreateReplacement=*/false, Tok.WhitespaceRange, /*IndentLevel=*/0,
   /*Spaces=*/0, Tok.OriginalColumn, Tok.NewlinesBefore, "", "",
   Tok.Tok.getKind(), InPPDirective && !Tok.IsFirst,
-  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+  /*IsTrailingCommentContinuation=*/false));
 }
 
 void WhitespaceManager::replaceWhitespaceInToken(
@@ -82,15 +86,11 @@
   Changes.push_back(Change(
   true, SourceRange(Start, Start.getLocWithOffset(ReplaceChars)),
   IndentLevel, Spaces, std::max(0, Spaces), Newline

Re: [PATCH] D16058: [clang-format] Fix comment aligning when there are changes within the comment

[Daniel Jasper via cfe-commits]

djasper added inline comments.


Comment at: lib/Format/WhitespaceManager.h:145
@@ +144,3 @@
+// comment.
+bool IsTrailingCommentContinuation;
+

I find the term "continuation" a bit confusing here. How about something like 
"IsInsideToken"?


http://reviews.llvm.org/D16058



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

Re: [PATCH] D16058: [clang-format] Fix comment aligning when there are changes within the comment

[Manuel Klimek via cfe-commits]

klimek added inline comments.


Comment at: lib/Format/WhitespaceManager.h:145
@@ +144,3 @@
+// comment.
+bool IsTrailingCommentContinuation;
+

djasper wrote:
> I find the term "continuation" a bit confusing here. How about something like 
> "IsInsideToken"?
"Token" sounds like it could be true for things outside comments. Perhaps 
"InComment"?


http://reviews.llvm.org/D16058



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

Re: [PATCH] D16058: [clang-format] Fix comment aligning when there are changes within the comment

[Benjamin Kramer via cfe-commits]

bkramer updated this revision to Diff 44472.
bkramer added a comment.

Why not both?


http://reviews.llvm.org/D16058

Files:
  lib/Format/WhitespaceManager.cpp
  lib/Format/WhitespaceManager.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1022,6 +1022,10 @@
"  lineWith(); // comment\n"
"  // at start\n"
"}"));
+  EXPECT_EQ("int xy; // a\n"
+"int z;  // b",
+format("int xy;// a\n"
+   "int z;//b"));
 
   verifyFormat("#define A  \\\n"
"  int i; /* i */   \\\n"
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -109,7 +109,8 @@
unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
unsigned NewlinesBefore, StringRef PreviousLinePostfix,
StringRef CurrentLinePrefix, tok::TokenKind Kind,
-   bool ContinuesPPDirective, bool IsStartOfDeclName);
+   bool ContinuesPPDirective, bool IsStartOfDeclName,
+   bool IsInsideTrailingCommentToken);
 
 bool CreateReplacement;
 // Changes might be in the middle of a token, so we cannot just keep the
@@ -139,6 +140,10 @@
 // comments. Uncompensated negative offset is truncated to 0.
 int Spaces;
 
+// If this change is inside of a line comment but not at the start of such a
+// comment.
+bool IsInsideTrailingCommentToken;
+
 // \c IsTrailingComment, \c TokenLength, \c PreviousEndOfTokenColumn and
 // \c EscapedNewlineColumn will be calculated in
 // \c calculateLineBreakInformation.
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -30,17 +30,19 @@
 unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
 unsigned NewlinesBefore, StringRef PreviousLinePostfix,
 StringRef CurrentLinePrefix, tok::TokenKind Kind, bool ContinuesPPDirective,
-bool IsStartOfDeclName)
+bool IsStartOfDeclName, bool IsInsideTrailingCommentToken)
 : CreateReplacement(CreateReplacement),
   OriginalWhitespaceRange(OriginalWhitespaceRange),
   StartOfTokenColumn(StartOfTokenColumn), NewlinesBefore(NewlinesBefore),
   PreviousLinePostfix(PreviousLinePostfix),
   CurrentLinePrefix(CurrentLinePrefix), Kind(Kind),
   ContinuesPPDirective(ContinuesPPDirective),
   IsStartOfDeclName(IsStartOfDeclName), IndentLevel(IndentLevel),
-  Spaces(Spaces), IsTrailingComment(false), TokenLength(0),
-  PreviousEndOfTokenColumn(0), EscapedNewlineColumn(0),
-  StartOfBlockComment(nullptr), IndentationOffset(0) {}
+  Spaces(Spaces),
+  IsInsideTrailingCommentToken(IsTrailingCommentContinuation),
+  IsTrailingComment(false), TokenLength(0), PreviousEndOfTokenColumn(0),
+  EscapedNewlineColumn(0), StartOfBlockComment(nullptr),
+  IndentationOffset(0) {}
 
 void WhitespaceManager::reset() {
   Changes.clear();
@@ -58,7 +60,8 @@
   Change(/*CreateReplacement=*/true, Tok.WhitespaceRange, IndentLevel,
  Spaces, StartOfTokenColumn, Newlines, "", "", Tok.Tok.getKind(),
  InPPDirective && !Tok.IsFirst,
- Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+ Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+ /*IsInsideTrailingCommentToken=*/false));
 }
 
 void WhitespaceManager::addUntouchableToken(const FormatToken &Tok,
@@ -69,7 +72,8 @@
   /*CreateReplacement=*/false, Tok.WhitespaceRange, /*IndentLevel=*/0,
   /*Spaces=*/0, Tok.OriginalColumn, Tok.NewlinesBefore, "", "",
   Tok.Tok.getKind(), InPPDirective && !Tok.IsFirst,
-  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+  /*IsInsideTrailingCommentToken=*/false));
 }
 
 void WhitespaceManager::replaceWhitespaceInToken(
@@ -82,15 +86,11 @@
   Changes.push_back(Change(
   true, SourceRange(Start, Start.getLocWithOffset(ReplaceChars)),
   IndentLevel, Spaces, std::max(0, Spaces), Newlines, PreviousPostfix,
-  CurrentPrefix,
-  // If we don't add a newline this change doesn't start a comment. Thus,
-  // when we align line comments, we don't need to treat this change as one.
-  // FIXME: We still need to take this change in account to properly
-  // calculate the new length of the comment and to calculate the changes
-  // for which to do the alignment when aligning comments.
-  Tok.is(TT_LineComment) && Newlines > 0 ? tok::commen

Re: [PATCH] D16058: [clang-format] Fix comment aligning when there are changes within the comment

[Daniel Jasper via cfe-commits]

djasper added inline comments.


Comment at: lib/Format/WhitespaceManager.h:145
@@ +144,3 @@
+// comment.
+bool IsTrailingCommentContinuation;
+

klimek wrote:
> djasper wrote:
> > I find the term "continuation" a bit confusing here. How about something 
> > like "IsInsideToken"?
> "Token" sounds like it could be true for things outside comments. Perhaps 
> "InComment"?
So why not store the more generic information here? We already have the extra 
information whether this is a line comment or not stored in the token kind.


http://reviews.llvm.org/D16058



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

Re: [PATCH] D15944: [OpenMP] Parsing and sema support for target update directive

[Alexey Bataev via cfe-commits]

ABataev added a comment.

Kelvin, thanks for the patch!
One more comment: what about nesting of regions? You need to add a 
corresponding code and tests



Comment at: include/clang/AST/OpenMPClause.h:3196
@@ -3195,1 +3195,3 @@
 
+/// \brief This represents clause 'from' in the '#pragma omp ...'
+/// directives.

New clauses must be added in separate patches after commit of the new directive.


Comment at: lib/CodeGen/CGStmtOpenMP.cpp:2685
@@ +2684,3 @@
+const OMPTargetUpdateDirective &S) {
+  llvm_unreachable("CodeGen for 'omp target update' is not supported yet.");
+}

Maybe just do not emit anything, to not crash a compiler.


Comment at: lib/Parse/ParseOpenMP.cpp:71-74
@@ -69,3 +70,6 @@
  !P.getPreprocessor().getSpelling(Tok).compare("point")) ||
-((i == 1) && 
!P.getPreprocessor().getSpelling(Tok).compare("data"));
+((i == 1) &&
+ !P.getPreprocessor().getSpelling(Tok).compare("data")) ||
+((i == 2) &&
+ !P.getPreprocessor().getSpelling(Tok).compare("update"));
   } else {

Probably, we need to add local enumeric for these constants (0, 1, 2 etc.) 


http://reviews.llvm.org/D15944



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

[PATCH] D16062: [analyzer] Rename kind-enumeration values of SVal, SymExpr, MemRegion classes, for consistency.

[Artem Dergachev via cfe-commits]

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

Based on discussion in D15448.

- For every sub-class `C`, its kind in the relevant enumeration is `CKind` (or 
`C##Kind` in preprocessor-ish terms), eg:
  `MemRegionKind` -> `MemRegionValKind`
  `RegionValueKind` -> `SymbolRegionValueKind`
  `CastSymbolKind` -> `SymbolCastKind`
  `SymIntKind` -> `SymIntExprKind`

- `MemSpaceRegion` is now an abstract base and no longer occupies 
`GenericMemSpaceRegionKind`. Instead, a new class, `CodeSpaceRegion`, is 
introduced for handling the unique use case for `MemSpaceRegion` as "the 
generic memory space" (when it represents a memory space that holds all 
executable code).

- `BEG_` prefixes in memory region kind ranges are renamed to `BEGIN_` for 
consisitency with symbol kind ranges (pro: not about begging, con: two extra 
characters to type).

http://reviews.llvm.org/D16062

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
  lib/StaticAnalyzer/Core/MemRegion.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/SVals.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Store.cpp
  lib/StaticAnalyzer/Core/SymbolManager.cpp

Index: lib/StaticAnalyzer/Core/SymbolManager.cpp
===
--- lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -115,22 +115,22 @@
   const SymExpr *SE = itr.pop_back_val();
 
   switch (SE->getKind()) {
-case SymExpr::RegionValueKind:
-case SymExpr::ConjuredKind:
-case SymExpr::DerivedKind:
-case SymExpr::ExtentKind:
-case SymExpr::MetadataKind:
+case SymExpr::SymbolRegionValueKind:
+case SymExpr::SymbolConjuredKind:
+case SymExpr::SymbolDerivedKind:
+case SymExpr::SymbolExtentKind:
+case SymExpr::SymbolMetadataKind:
   return;
-case SymExpr::CastSymbolKind:
+case SymExpr::SymbolCastKind:
   itr.push_back(cast(SE)->getOperand());
   return;
-case SymExpr::SymIntKind:
+case SymExpr::SymIntExprKind:
   itr.push_back(cast(SE)->getLHS());
   return;
-case SymExpr::IntSymKind:
+case SymExpr::IntSymExprKind:
   itr.push_back(cast(SE)->getRHS());
   return;
-case SymExpr::SymSymKind: {
+case SymExpr::SymSymExprKind: {
   const SymSymExpr *x = cast(SE);
   itr.push_back(x->getLHS());
   itr.push_back(x->getRHS());
@@ -458,35 +458,35 @@
   bool KnownLive;
 
   switch (sym->getKind()) {
-  case SymExpr::RegionValueKind:
+  case SymExpr::SymbolRegionValueKind:
 KnownLive = isLiveRegion(cast(sym)->getRegion());
 break;
-  case SymExpr::ConjuredKind:
+  case SymExpr::SymbolConjuredKind:
 KnownLive = false;
 break;
-  case SymExpr::DerivedKind:
+  case SymExpr::SymbolDerivedKind:
 KnownLive = isLive(cast(sym)->getParentSymbol());
 break;
-  case SymExpr::ExtentKind:
+  case SymExpr::SymbolExtentKind:
 KnownLive = isLiveRegion(cast(sym)->getRegion());
 break;
-  case SymExpr::MetadataKind:
+  case SymExpr::SymbolMetadataKind:
 KnownLive = MetadataInUse.count(sym) &&
 isLiveRegion(cast(sym)->getRegion());
 if (KnownLive)
   MetadataInUse.erase(sym);
 break;
-  case SymExpr::SymIntKind:
+  case SymExpr::SymIntExprKind:
 KnownLive = isLive(cast(sym)->getLHS());
 break;
-  case SymExpr::IntSymKind:
+  case SymExpr::IntSymExprKind:
 KnownLive = isLive(cast(sym)->getRHS());
 break;
-  case SymExpr::SymSymKind:
+  case SymExpr::SymSymExprKind:
 KnownLive = isLive(cast(sym)->getLHS()) &&
 isLive(cast(sym)->getRHS());
 break;
-  case SymExpr::CastSymbolKind:
+  case SymExpr::SymbolCastKind:
 KnownLive = isLive(cast(sym)->getOperand());
 break;
   }
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -100,7 +100,7 @@
   // Process region cast according to the kind of the region being cast.
   switch (R->getKind()) {
 case MemRegion::CXXThisRegionKind:
-case MemRegion::GenericMemSpaceRegionKind:
+case MemRegion::CodeSpaceRegionKind:
 case MemRegion::StackLocalsSpaceRegionKind:
 case MemRegion::StackArgumentsSpaceRegionKind:
 case MemRegion::HeapSpaceRegionKind:
@@ -393,7 +393,7 @@
   const MemRegion* BaseR = nullptr;
 
   switch (BaseL.getSubKind()) {
-  case loc::MemRegionKind:
+  case loc::MemRegionValKind:
 BaseR = BaseL.castAs().getRegion();
 break;
 
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -141,7 +141,7 @@
   

Re: [PATCH] D15443: Fix getLocEnd for function declarations with exception specification.

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In http://reviews.llvm.org/D15443#323043, @adek05 wrote:

> I think this won't work. If there is a problem with the expression inside 
> `throw` or `noexcept` specifier, it will be highlighted inside the parens, 
> never trying to actually access the end location of the function declaration.
>
> It would be enough if there is a warning/error which would highlight the 
> whole function declaration, because we could check whether it points at `;` 
> or the character in front of it, but I don't know about such warning and I 
> don't know how to smartly look for it.


I think the best way to proceed then is with a test in SourceLocationTest.cpp 
in the AST unit test suite.


http://reviews.llvm.org/D15443



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

Re: [PATCH] D15796: [PATCH] clang-tidy documentation redirects

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

Ping


http://reviews.llvm.org/D15796



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

Re: [PATCH] D16044: getVariableName() for MemRegion

[Alexander Droste via cfe-commits]

Alexander_Droste updated this revision to Diff 44476.
Alexander_Droste added a comment.

- reduced buffer size for variable name
- simplified `R->getAs()->getSuperRegion()` to 
`ER->getSuperRegion()`.


http://reviews.llvm.org/D16044

Files:
  tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp

Index: tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -187,6 +187,14 @@
   template const RegionTy* getAs() const;
 
   virtual bool isBoundable() const { return false; }
+
+  /// Get variable name for memory region. The name is obtained from
+  /// the variable/field declaration retrieved from the memory region.
+  /// Regions that point to an element of an array are returned as: "arr[0]".
+  /// Regions that point to a struct are returned as: "st.var".
+  ///
+  /// \returns variable name for memory region
+  std::string getVariableName() const;
 };
 
 /// MemSpaceRegion - A memory region that represents a "memory space";
Index: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -635,6 +635,45 @@
   superRegion->printPrettyAsExpr(os);
 }
 
+std::string MemRegion::getVariableName() const {
+  std::string VariableName{""};
+  std::string ArrayIndices{""};
+
+  const clang::ento::MemRegion *R = this;
+  llvm::SmallString<50> buf;
+  llvm::raw_svector_ostream os(buf);
+
+  // Obtain array indices to add them to the variable name.
+  const clang::ento::ElementRegion *ER = nullptr;
+  while ((ER = R->getAs())) {
+llvm::APSInt IndexInArray =
+ER->getIndex().getAs()->getValue();
+
+llvm::SmallString<2> intValAsString;
+IndexInArray.toString(intValAsString);
+std::string idx{intValAsString.begin(), intValAsString.end()};
+
+ArrayIndices = "[" + idx + "]" + ArrayIndices;
+
+R = ER->getSuperRegion();
+  }
+
+  // Get variable name.
+  if (R && R->canPrintPretty()) {
+R->printPretty(os);
+VariableName += os.str();
+  }
+
+  // Combine variable name with indices.
+  if (VariableName.size() && VariableName.back() == '\'') {
+VariableName.insert(VariableName.size() - 1, ArrayIndices);
+  } else {
+VariableName += ArrayIndices;
+  }
+
+  return VariableName;
+}
+
 
//===--===//
 // MemRegionManager methods.
 
//===--===//


Index: tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -187,6 +187,14 @@
   template const RegionTy* getAs() const;
 
   virtual bool isBoundable() const { return false; }
+
+  /// Get variable name for memory region. The name is obtained from
+  /// the variable/field declaration retrieved from the memory region.
+  /// Regions that point to an element of an array are returned as: "arr[0]".
+  /// Regions that point to a struct are returned as: "st.var".
+  ///
+  /// \returns variable name for memory region
+  std::string getVariableName() const;
 };
 
 /// MemSpaceRegion - A memory region that represents a "memory space";
Index: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -635,6 +635,45 @@
   superRegion->printPrettyAsExpr(os);
 }
 
+std::string MemRegion::getVariableName() const {
+  std::string VariableName{""};
+  std::string ArrayIndices{""};
+
+  const clang::ento::MemRegion *R = this;
+  llvm::SmallString<50> buf;
+  llvm::raw_svector_ostream os(buf);
+
+  // Obtain array indices to add them to the variable name.
+  const clang::ento::ElementRegion *ER = nullptr;
+  while ((ER = R->getAs())) {
+llvm::APSInt IndexInArray =
+ER->getIndex().getAs()->getValue();
+
+llvm::SmallString<2> intValAsString;
+IndexInArray.toString(intValAsString);
+std::string idx{intValAsString.begin(), intValAsString.end()};
+
+ArrayIndices = "[" + idx + "]" + ArrayIndices;
+
+R = ER->getSuperRegion();
+  }
+
+  // Get variable name.
+  if (R && R->canPrintPretty()) {
+R->printPretty(os);
+VariableName += os.str();
+  }
+
+  // Combine variable name with indices.
+  if (VariableName.size() && VariableName.back() == '\'') {
+VariableName.insert(VariableName.size() - 1, ArrayIndices);
+  } el

Re: [PATCH] D15448: [analyzer] SVal Visitor.

[Artem Dergachev via cfe-commits]

NoQ updated this revision to Diff 44475.
NoQ marked 5 inline comments as done.
NoQ added a comment.

Renamed the kinds for consistency (review http://reviews.llvm.org/D16062), this 
diff is updated to use the new naming convention. The 'kind' column gets 
removed from the def-files.


http://reviews.llvm.org/D15448

Files:
  docs/analyzer/DebugChecks.rst
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def
  include/clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/Symbols.def
  lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  test/Analysis/explain-svals.cpp

Index: test/Analysis/explain-svals.cpp
===
--- /dev/null
+++ test/Analysis/explain-svals.cpp
@@ -0,0 +1,98 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core.builtin,debug.ExprInspection,unix.cstring -verify %s
+
+typedef unsigned long size_t;
+
+struct S {
+  struct S3 {
+int y[10];
+  };
+  struct S2 : S3 {
+int *x;
+  } s2[10];
+  int z;
+};
+
+
+void clang_analyzer_explain(int);
+void clang_analyzer_explain(void *);
+void clang_analyzer_explain(S);
+
+size_t clang_analyzer_getExtent(void *);
+
+size_t strlen(const char *);
+
+int conjure();
+S conjure_S();
+
+int glob;
+static int stat_glob;
+void *glob_ptr;
+
+// Test strings are regex'ed because we need to match exact string
+// rather than a substring.
+
+void test_1(int param, void *ptr) {
+  clang_analyzer_explain(&glob); // expected-warning-re^pointer to global variable 'glob'$
+  clang_analyzer_explain(param); // expected-warning-re^argument 'param'$
+  clang_analyzer_explain(ptr); // expected-warning-re^argument 'ptr'$
+  if (param == 42)
+clang_analyzer_explain(param); // expected-warning-re^signed 32-bit integer '42'$
+}
+
+void test_2(char *ptr, int ext) {
+  clang_analyzer_explain((void *) "asdf"); // expected-warning-re^pointer to element of type 'char' with index 0 of string literal "asdf"$
+  clang_analyzer_explain(strlen(ptr)); // expected-warning-re^metadata of type 'unsigned long' tied to pointee of argument 'ptr'$
+  clang_analyzer_explain(conjure()); // expected-warning-re^symbol of type 'int' conjured at statement 'conjure\(\)'$
+  clang_analyzer_explain(glob); // expected-warning-re^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob'$
+  clang_analyzer_explain(glob_ptr); // expected-warning-re^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob_ptr'$
+  clang_analyzer_explain(clang_analyzer_getExtent(ptr)); // expected-warning-re^extent of pointee of argument 'ptr'$
+  int *x = new int[ext];
+  clang_analyzer_explain(x); // expected-warning-re^pointer to element of type 'int' with index 0 of pointee of symbol of type 'int \*' conjured at statement 'new int \[ext\]'$
+  // Sic! What gets computed is the extent of the element-region.
+  clang_analyzer_explain(clang_analyzer_getExtent(x)); // expected-warning-re^signed 32-bit integer '4'$
+  delete[] x;
+}
+
+void test_3(S s) {
+  clang_analyzer_explain(&s); // expected-warning-re^pointer to parameter 's'$
+  clang_analyzer_explain(s.z); // expected-warning-re^initial value of field 'z' of parameter 's'$
+  clang_analyzer_explain(&s.s2[5].y[3]); // expected-warning-re^pointer to element of type 'int' with index 3 of field 'y' of base object 'S::S3' inside element of type 'struct S::S2' with index 5 of field 's2' of parameter 's'$
+  if (!s.s2[7].x) {
+clang_analyzer_explain(s.s2[7].x); // expected-warning-re^concrete memory address '0'$
+// FIXME: we need to be explaining '1' rather than '0' here; not explainer bug.
+clang_analyzer_explain(s.s2[7].x + 1); // expected-warning-re^concrete memory address '0'$
+  }
+}
+
+void test_4(int x, int y) {
+  int z;
+  static int stat;
+  clang_analyzer_explain(x + 1); // expected-warning-re^\(argument 'x'\) \+ 1$
+  clang_analyzer_explain(1 + y); // expected-warning-re^\(argument 'y'\) \+ 1$
+  clang_analyzer_explain(x + y); // expected-warning-re^unknown value$
+  clang_analyzer_explain(z); // expected-warning-re^undefined value$
+  clang_analyzer_explain(&z); // expected-warning-re^pointer to local variable 'z'$
+  clang_analyzer_explain(stat); // expected-warning-re^signed 32-bit integer '0'$
+  clang_analyzer_explain(&stat); // expected-warning-re^pointer to static local variable 'stat'$
+  clang_analyzer_explain(stat_glob); // expecte

Re: [PATCH] D16044: getVariableName() for MemRegion

[Alexander Droste via cfe-commits]

Alexander_Droste marked 2 inline comments as done.


Comment at: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:643
@@ +642,3 @@
+  const clang::ento::MemRegion *R = this;
+  llvm::SmallString<50> buf;
+  llvm::raw_svector_ostream os(buf);

You're right, 200 was a bit overambitious. I reduced it to 50.


http://reviews.llvm.org/D16044



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

Re: [PATCH] D15796: [PATCH] clang-tidy documentation redirects

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Sorry for the delay and thanks for pinging me.



Comment at: docs/clang-tidy/checks/cert-dcl54-cpp.rst:6
@@ +5,3 @@
+cert-dcl54-cpp
+==
+

aaron.ballman wrote:
> > We need to teach add_new_check.py to retain (or automatically generate) 
> > these comments then.
> 
> I have not touched add_new_check.py for this because add_new_check.py knows 
> nothing about check aliases anyway. Whenever we update add_new_check.py to 
> generate checker aliases directly, that would be a good time to do this work.
add_new_check.py should do something reasonable with list.rst right away, 
otherwise the `(redirects to ...)` comments will be lost on each run of the 
script, which will result in annoying manual work of reverting these changes. 
Do you need help implementing alias handling in the add_new_check.py script?


Comment at: docs/clang-tidy/checks/cert-dcl54-cpp.rst:11
@@ +10,1 @@
+information.
\ No newline at end of file


nit: Could you add newlines at the end of files to remove this annoying line?


http://reviews.llvm.org/D15796



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

Re: [PATCH] D15796: [PATCH] clang-tidy documentation redirects

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: docs/clang-tidy/checks/cert-dcl54-cpp.rst:6
@@ +5,3 @@
+cert-dcl54-cpp
+==
+

alexfh wrote:
> aaron.ballman wrote:
> > > We need to teach add_new_check.py to retain (or automatically generate) 
> > > these comments then.
> > 
> > I have not touched add_new_check.py for this because add_new_check.py knows 
> > nothing about check aliases anyway. Whenever we update add_new_check.py to 
> > generate checker aliases directly, that would be a good time to do this 
> > work.
> add_new_check.py should do something reasonable with list.rst right away, 
> otherwise the `(redirects to ...)` comments will be lost on each run of the 
> script, which will result in annoying manual work of reverting these changes. 
> Do you need help implementing alias handling in the add_new_check.py script?
Oye, that's a good point. If you don't mind tackling the Python side of things, 
that would be great.


Comment at: docs/clang-tidy/checks/cert-dcl54-cpp.rst:11
@@ +10,1 @@
+information.
\ No newline at end of file


alexfh wrote:
> nit: Could you add newlines at the end of files to remove this annoying line?
Will do.


http://reviews.llvm.org/D15796



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

Re: [PATCH] D15796: [PATCH] clang-tidy documentation redirects

[Aaron Ballman via cfe-commits]

aaron.ballman updated this revision to Diff 44492.
aaron.ballman added a comment.

Adding newlines to the end of some files, per feedback.


http://reviews.llvm.org/D15796

Files:
  docs/clang-tidy/checks/cert-dcl03-c.rst
  docs/clang-tidy/checks/cert-dcl54-cpp.rst
  docs/clang-tidy/checks/cert-dcl59-cpp.rst
  docs/clang-tidy/checks/cert-err61-cpp.rst
  docs/clang-tidy/checks/cert-fio38-c.rst
  docs/clang-tidy/checks/cert-oop11-cpp.rst
  docs/clang-tidy/checks/google-build-namespaces.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-move-constructor-init.rst
  docs/clang-tidy/checks/misc-new-delete-overloads.rst
  docs/clang-tidy/checks/misc-non-copyable-objects.rst
  docs/clang-tidy/checks/misc-static-assert.rst
  docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst

Index: docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
===
--- docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
+++ docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
@@ -3,6 +3,8 @@
 misc-throw-by-value-catch-by-reference
 ==
 
+"cert-err61-cpp" redirects here as an alias for this checker.
+
 Finds violations of the rule "Throw by value, catch by reference" presented for example in "C++ Coding Standards" by H. Sutter and A. Alexandrescu. This check also has the option to find violations of the rule "Throw anonymous temporaries" (https://www.securecoding.cert.org/confluence/display/cplusplus/ERR09-CPP.+Throw+anonymous+temporaries). The option is named "CheckThrowTemporaries" and it's on by default.
 
 Exceptions:
Index: docs/clang-tidy/checks/misc-static-assert.rst
===
--- docs/clang-tidy/checks/misc-static-assert.rst
+++ docs/clang-tidy/checks/misc-static-assert.rst
@@ -3,6 +3,7 @@
 misc-static-assert
 ==
 
+"cert-dcl03-c" redirects here as an alias for this checker.
 
 Replaces ``assert()`` with ``static_assert()`` if the condition is evaluatable
 at compile time.
Index: docs/clang-tidy/checks/misc-non-copyable-objects.rst
===
--- docs/clang-tidy/checks/misc-non-copyable-objects.rst
+++ docs/clang-tidy/checks/misc-non-copyable-objects.rst
@@ -3,6 +3,8 @@
 misc-non-copyable-objects
 =
 
+"cert-fio38-c" redirects here as an alias for this checker.
+
 The check flags dereferences and non-pointer declarations of objects that are
 not meant to be passed by value, such as C FILE objects or POSIX
 pthread_mutex_t objects.
Index: docs/clang-tidy/checks/misc-new-delete-overloads.rst
===
--- docs/clang-tidy/checks/misc-new-delete-overloads.rst
+++ docs/clang-tidy/checks/misc-new-delete-overloads.rst
@@ -3,6 +3,8 @@
 misc-new-delete-overloads
 =
 
+"cert-dcl54-cpp" redirects here as an alias for this checker.
+
 The check flags overloaded operator new() and operator delete() functions that
 do not have a corresponding free store function defined within the same scope.
 For instance, the check will flag a class implementation of a non-placement
Index: docs/clang-tidy/checks/misc-move-constructor-init.rst
===
--- docs/clang-tidy/checks/misc-move-constructor-init.rst
+++ docs/clang-tidy/checks/misc-move-constructor-init.rst
@@ -3,6 +3,7 @@
 misc-move-constructor-init
 ==
 
+"cert-oop11-cpp" redirects here as an alias for this checker.
 
 The check flags user-defined move constructors that have a ctor-initializer
 initializing a member or base class through a copy constructor instead of a
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,9 +5,15 @@
 
 .. toctree::   
cert-dcl50-cpp
+   cert-dcl54-cpp (redirects to misc-new-delete-overloads) 
+   cert-dcl59-cpp (redirects to google-build-namespaces) 
cert-err52-cpp
cert-err58-cpp
cert-err60-cpp
+   cert-err61-cpp (redirects to misc-throw-by-value-catch-by-reference) 
+   cert-oop11-cpp (redirects to misc-move-constructor-init) 
+   cert-dcl03-c (redirects to misc-static-assert) 
+   cert-fio38-c (redirects to misc-non-copyable-objects) 
cppcoreguidelines-pro-bounds-array-to-pointer-decay
cppcoreguidelines-pro-bounds-constant-array-index
cppcoreguidelines-pro-bounds-pointer-arithmetic
Index: docs/clang-tidy/checks/google-build-namespaces.rst
===
--- docs/clang-tidy/checks/google-build-namespaces.rst
+++ docs/clang-tidy/checks/google-build-namespaces.rst
@@ -3,6 +3,7 @@
 google-build-namespaces
 ===
 
+"cert-dcl59-cpp" redirects here as an alias for t

Re: [PATCH] D15796: [PATCH] clang-tidy documentation redirects

[Aaron Ballman via cfe-commits]

aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

http://reviews.llvm.org/D15796



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

[PATCH] D16063: [Analyzer] Use a wider integer type for an array index

[Aleksei Sidorin via cfe-commits]

a.sidorin created this revision.
a.sidorin added reviewers: zaks.anna, dcoughlin, xazax.hun.
a.sidorin added a subscriber: cfe-commits.
a.sidorin set the repository for this revision to rL LLVM.

Currently, a default index type in CSA is 'int'. However, this assumption seems 
to be incorrect for 64-bit platforms where index may be 64-bit as well as the 
array size. For example, it leads to unexpected overflows while performing 
pointer arithmetics. Moreover, PointerDiffType and 'int' cannot be used as a 
common array index type because arrays may have size (and indexes) more than 
PTRDIFF_MAX but less than SIZE_MAX. Since maximum array size for 64 bits is 
SIZE_MAX/8, I propose to use 'long long' as a common index type, although it 
looks a bit hacky for 32 bit.

Repository:
  rL LLVM

http://reviews.llvm.org/D16063

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  test/Analysis/index-type.c

Index: test/Analysis/index-type.c
===
--- /dev/null
+++ test/Analysis/index-type.c
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze 
-analyzer-checker=core,alpha.security.ArrayBoundV2 -verify %s
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze 
-analyzer-checker=core,alpha.security.ArrayBoundV2 -DM32 -verify %s
+// expected-no-diagnostics
+
+#define UINT_MAX (~0u)
+
+#ifdef M32
+
+#define X86_ARRAY_SIZE UINT_MAX/2 + 4
+
+void testIndexTooBig() {
+  char arr[X86_ARRAY_SIZE];
+  char *ptr = arr + UINT_MAX/2;
+  ptr += 2;  // index shouldn't overflow
+  *ptr = 42; // no-warning
+}
+
+#else // 64-bit tests
+
+#define ARRAY_SIZE 0x1
+
+void testIndexOverflow64() {
+  char arr[ARRAY_SIZE];
+  char *ptr = arr + UINT_MAX/2;
+  ptr += 2;  // don't overflow 64-bit index
+  *ptr = 42; // no-warning
+}
+
+#define ULONG_MAX (~0ul)
+#define BIG_ARRAY_SIZE 0x9000
+void testIndexTooBig64() {
+  char arr[ULONG_MAX/8-1];
+  char *ptr = arr + ULONG_MAX;
+  ptr += 2;  // don't overflow 64-bit index
+  *ptr = 42; // no-warning
+}
+
+#endif
Index: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -65,7 +65,7 @@
   SymMgr(context, BasicVals, alloc),
   MemMgr(context, alloc),
   StateMgr(stateMgr),
-  ArrayIndexTy(context.IntTy),
+  ArrayIndexTy(context.LongLongTy),
   ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}
 
   virtual ~SValBuilder() {}


Index: test/Analysis/index-type.c
===
--- /dev/null
+++ test/Analysis/index-type.c
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze -analyzer-checker=core,alpha.security.ArrayBoundV2 -verify %s
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core,alpha.security.ArrayBoundV2 -DM32 -verify %s
+// expected-no-diagnostics
+
+#define UINT_MAX (~0u)
+
+#ifdef M32
+
+#define X86_ARRAY_SIZE UINT_MAX/2 + 4
+
+void testIndexTooBig() {
+  char arr[X86_ARRAY_SIZE];
+  char *ptr = arr + UINT_MAX/2;
+  ptr += 2;  // index shouldn't overflow
+  *ptr = 42; // no-warning
+}
+
+#else // 64-bit tests
+
+#define ARRAY_SIZE 0x1
+
+void testIndexOverflow64() {
+  char arr[ARRAY_SIZE];
+  char *ptr = arr + UINT_MAX/2;
+  ptr += 2;  // don't overflow 64-bit index
+  *ptr = 42; // no-warning
+}
+
+#define ULONG_MAX (~0ul)
+#define BIG_ARRAY_SIZE 0x9000
+void testIndexTooBig64() {
+  char arr[ULONG_MAX/8-1];
+  char *ptr = arr + ULONG_MAX;
+  ptr += 2;  // don't overflow 64-bit index
+  *ptr = 42; // no-warning
+}
+
+#endif
Index: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -65,7 +65,7 @@
   SymMgr(context, BasicVals, alloc),
   MemMgr(context, alloc),
   StateMgr(stateMgr),
-  ArrayIndexTy(context.IntTy),
+  ArrayIndexTy(context.LongLongTy),
   ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}
 
   virtual ~SValBuilder() {}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15862: A possible direction for fixing https://llvm.org/bugs/show_bug.cgi?id=25973.

[Marshall Clow via cfe-commits]

mclow.lists updated this revision to Diff 44496.
mclow.lists added a comment.

Updated to address Tim's comments.


http://reviews.llvm.org/D15862

Files:
  include/algorithm
  include/iterator
  include/string
  
test/std/strings/basic.string/string.modifiers/string_append/initializer_list.pass.cpp
  test/std/strings/basic.string/string.modifiers/string_append/iterator.pass.cpp
  test/std/strings/basic.string/string.modifiers/string_append/pointer.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_append/pointer_size.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_append/push_back.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_append/size_char.pass.cpp
  test/std/strings/basic.string/string.modifiers/string_append/string.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_append/string_size_size.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_assign/initializer_list.pass.cpp
  test/std/strings/basic.string/string.modifiers/string_assign/iterator.pass.cpp
  test/std/strings/basic.string/string.modifiers/string_assign/pointer.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_assign/pointer_size.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_assign/rv_string.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_assign/size_char.pass.cpp
  test/std/strings/basic.string/string.modifiers/string_assign/string.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_assign/string_size_size.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_insert/iter_char.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_insert/iter_initializer_list.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_insert/iter_iter_iter.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_insert/iter_size_char.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_insert/size_pointer.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_insert/size_pointer_size.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_insert/size_size_char.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_insert/size_string.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_insert/size_string_size_size.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_replace/iter_iter_initializer_list.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_replace/iter_iter_iter_iter.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_replace/iter_iter_pointer.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_replace/iter_iter_pointer_size.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_replace/iter_iter_size_char.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_replace/iter_iter_string.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_replace/size_size_pointer.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_replace/size_size_pointer_size.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_replace/size_size_size_char.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_replace/size_size_string.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_replace/size_size_string_size_size.pass.cpp
  test/support/test_iterators.h

Index: test/std/strings/basic.string/string.modifiers/string_replace/size_size_string_size_size.pass.cpp
===
--- test/std/strings/basic.string/string.modifiers/string_replace/size_size_string_size_size.pass.cpp
+++ test/std/strings/basic.string/string.modifiers/string_replace/size_size_string_size_size.pass.cpp
@@ -5903,7 +5903,7 @@
 test54();
 test55();
 }
-#if __cplusplus >= 201103L
+#if TEST_STD_VER >= 11
 {
 typedef std::basic_string, min_allocator> S;
 test0();
Index: test/std/strings/basic.string/string.modifiers/string_replace/size_size_string.pass.cpp
===
--- test/std/strings/basic.string/string.modifiers/string_replace/size_size_string.pass.cpp
+++ test/std/strings/basic.string/string.modifiers/string_replace/size_size_string.pass.cpp
@@ -362,7 +362,7 @@
 test1();
 test2();
 }
-#if __cplusplus >= 201103L
+#if TEST_STD_VER >= 11
 {
 typedef std::basic_string, min_allocator> S;
 test0();
Index: test/std/strings/basic.string/string.modifiers/string_replace/size_size_size_char.pass.cpp
===
--- test/std/strings/basic.string/string.modifiers/string_replace/size_size_size_char.pass.cpp
+++ test/std/strings/basic.string/string.modifiers/string_replace/size_size_size_char.pass.cpp
@@ -364,7 +364,7 @@
 test1();
 test2();
 }
-#if __cplusplus >= 201103L
+#if TEST_STD_VER >= 11
 {
 typedef s

Re: [PATCH] D15862: A possible direction for fixing https://llvm.org/bugs/show_bug.cgi?id=25973.

[Marshall Clow via cfe-commits]

mclow.lists marked 2 inline comments as done.


Comment at: include/string:2677-2678
@@ +2676,4 @@
+#endif
+for (; __first != __last; ++__first)
+push_back(*__first);
+

tcanens wrote:
> If an exception is thrown after a `push_back()` causes reallocation, existing 
> iterators/pointers/references would have been invalidated, and the `catch` 
> block can't do anything about it.
> 
> It looks like a temporary string is also needed here.
Dang, you're right. Thanks.


http://reviews.llvm.org/D15862



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

Re: [PATCH] D14277: [Analyzer] Make referenced SymbolMetadata live even if its region is dead

[Aleksei Sidorin via cfe-commits]

a.sidorin added a comment.

Ping?


Repository:
  rL LLVM

http://reviews.llvm.org/D14277



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

[PATCH] D16065: Fix infinite recursion for invalid declaration

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin created this revision.
DmitryPolukhin added a reviewer: rsmith.
DmitryPolukhin added subscribers: cfe-commits, kcc.

Fix for a case found by fuzzing PR23057 (comment #25 
https://llvm.org/bugs/show_bug.cgi?id=23057#c25).

http://reviews.llvm.org/D16065

Files:
  lib/AST/Decl.cpp
  test/SemaCXX/linkage-invalid-decl.cpp

Index: test/SemaCXX/linkage-invalid-decl.cpp
===
--- /dev/null
+++ test/SemaCXX/linkage-invalid-decl.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// This invalid declaration used to call infinite recursion in linkage
+// calculation for enum as a function argument.
+inline foo(A)(enum E;
+// expected-error@-1 {{unknown type name 'foo'}}
+// expected-error@-2 {{ISO C++ forbids forward references to 'enum' types}}
+// expected-error@-3 {{expected ')'}}
+// expected-note@-4 {{to match this '('}}
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -1184,7 +1184,7 @@
 return LinkageInfo::none();
 
   const Decl *OuterD = getOutermostFuncOrBlockContext(D);
-  if (!OuterD)
+  if (!OuterD || OuterD->isInvalidDecl())
 return LinkageInfo::none();
 
   LinkageInfo LV;


Index: test/SemaCXX/linkage-invalid-decl.cpp
===
--- /dev/null
+++ test/SemaCXX/linkage-invalid-decl.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// This invalid declaration used to call infinite recursion in linkage
+// calculation for enum as a function argument.
+inline foo(A)(enum E;
+// expected-error@-1 {{unknown type name 'foo'}}
+// expected-error@-2 {{ISO C++ forbids forward references to 'enum' types}}
+// expected-error@-3 {{expected ')'}}
+// expected-note@-4 {{to match this '('}}
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -1184,7 +1184,7 @@
 return LinkageInfo::none();
 
   const Decl *OuterD = getOutermostFuncOrBlockContext(D);
-  if (!OuterD)
+  if (!OuterD || OuterD->isInvalidDecl())
 return LinkageInfo::none();
 
   LinkageInfo LV;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16063: [Analyzer] Use a wider integer type for an array index

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

xazax.hun added a comment.

How is this array index type used? Should it support negative values? If the 
answer is no, ASTContext has a getSizeType method. If it is yes, it also has a 
getPointerDiffType method (although it returns a QualType not a CanQualType). 
Is there a reason not to use them?


Repository:
  rL LLVM

http://reviews.llvm.org/D16063



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

Re: [PATCH] D15796: [PATCH] clang-tidy documentation redirects

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: docs/clang-tidy/checks/cert-dcl54-cpp.rst:7
@@ +6,3 @@
+==
+
+The cert-dcl54-cpp checker is an alias, please see

This seems to do what we need (full file is here {F1298675}):

```
diff --git a/clang-tidy/add_new_check.py b/clang-tidy/add_new_check.py
index 6ecc89d..6000616 100755
--- a/clang-tidy/add_new_check.py
+++ b/clang-tidy/add_new_check.py
@@ -215,15 +215,25 @@ void awesome_f2();
 
 # Recreates the list of checks in the docs/clang-tidy/checks directory.
 def update_checks_list(module_path):
-  filename = os.path.normpath(
-  os.path.join(module_path, '../../docs/clang-tidy/checks/list.rst'))
+  docs_dir = os.path.join(module_path, '../../docs/clang-tidy/checks')
+  filename = os.path.normpath(os.path.join(docs_dir, 'list.rst'))
   with open(filename, 'r') as f:
 lines = f.readlines()
-
-  checks = map(lambda s: '   ' + s.replace('.rst', '\n'),
-   filter(lambda s: s.endswith('.rst') and s != 'list.rst',
-  os.listdir(os.path.join(module_path, 
'../../docs/clang-tidy/checks'
-  checks.sort()
+  doc_files = filter(
+  lambda s: s.endswith('.rst') and s != 'list.rst',
+  os.listdir(docs_dir))
+  doc_files.sort()
+
+  def format_link(doc_file):
+check_name = doc_file.replace('.rst', '')
+with open(os.path.join(docs_dir, doc_file), 'r') as doc:
+  match = re.search('.*:http-equiv=refresh: \d+;URL=(.*).html.*', 
doc.read())
+  if match:
+return '   %(check)s (redirects to %(target)s) <%(check)s>\n' % {
+'check' : check_name, 'target' : match.group(1) }
+  return '   %s\n' % check_name
+
+  checks = map(format_link, doc_files)
 
   print('Updating %s...' % filename)
   with open(filename, 'wb') as f:
```

Can you include this in the patch?


http://reviews.llvm.org/D15796



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

Re: [PATCH] D15796: [PATCH] clang-tidy documentation redirects

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: docs/clang-tidy/checks/cert-dcl54-cpp.rst:7
@@ +6,3 @@
+==
+
+The cert-dcl54-cpp checker is an alias, please see

alexfh wrote:
> This seems to do what we need (full file is here {F1298675}):
> 
> ```
> diff --git a/clang-tidy/add_new_check.py b/clang-tidy/add_new_check.py
> index 6ecc89d..6000616 100755
> --- a/clang-tidy/add_new_check.py
> +++ b/clang-tidy/add_new_check.py
> @@ -215,15 +215,25 @@ void awesome_f2();
>  
>  # Recreates the list of checks in the docs/clang-tidy/checks directory.
>  def update_checks_list(module_path):
> -  filename = os.path.normpath(
> -  os.path.join(module_path, '../../docs/clang-tidy/checks/list.rst'))
> +  docs_dir = os.path.join(module_path, '../../docs/clang-tidy/checks')
> +  filename = os.path.normpath(os.path.join(docs_dir, 'list.rst'))
>with open(filename, 'r') as f:
>  lines = f.readlines()
> -
> -  checks = map(lambda s: '   ' + s.replace('.rst', '\n'),
> -   filter(lambda s: s.endswith('.rst') and s != 'list.rst',
> -  os.listdir(os.path.join(module_path, 
> '../../docs/clang-tidy/checks'
> -  checks.sort()
> +  doc_files = filter(
> +  lambda s: s.endswith('.rst') and s != 'list.rst',
> +  os.listdir(docs_dir))
> +  doc_files.sort()
> +
> +  def format_link(doc_file):
> +check_name = doc_file.replace('.rst', '')
> +with open(os.path.join(docs_dir, doc_file), 'r') as doc:
> +  match = re.search('.*:http-equiv=refresh: \d+;URL=(.*).html.*', 
> doc.read())
> +  if match:
> +return '   %(check)s (redirects to %(target)s) <%(check)s>\n' % {
> +'check' : check_name, 'target' : match.group(1) }
> +  return '   %s\n' % check_name
> +
> +  checks = map(format_link, doc_files)
>  
>print('Updating %s...' % filename)
>with open(filename, 'wb') as f:
> ```
> 
> Can you include this in the patch?
(and please re-generate the list using the script)


http://reviews.llvm.org/D15796



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

[PATCH] D16066: Add BeforeWhileInDoWhile BraceWrapping option

[Eric Baker via cfe-commits]

ebaker355 created this revision.
ebaker355 added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

Adds a new BraceWrapping option called BeforeWhileInDoWhile.

Code like this:

do {
// Some code
} while (1);

becomes:

do {
// Some code
}
while (1);

I couldn't think of a better name for the option than BeforeWhileInDoWhile. I'm 
open to suggestions!

Also, this is my first attempt at submitting a patch for this project. If I've 
done anything wrong, please let me know. Thanks!

http://reviews.llvm.org/D16066

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6582,7 +6582,7 @@
"   b};");
 
   verifyNoCrash("a<,");
-  
+
   // No braced initializer here.
   verifyFormat("void f() {\n"
"  struct Dummy {};\n"
@@ -9823,6 +9823,7 @@
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterUnion);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeCatch);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeElse);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeWhileInDoWhile);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, IndentBraces);
 }
 
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1472,7 +1472,10 @@
 parseStructuralElement();
 --Line->Level;
   }
-
+  if (FormatTok->Tok.is(tok::kw_while) &&
+  Style.BraceWrapping.BeforeWhileInDoWhile) {
+addUnwrappedLine();
+  }
   // FIXME: Add error handling.
   if (!FormatTok->Tok.is(tok::kw_while)) {
 addUnwrappedLine();
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -347,6 +347,7 @@
 IO.mapOptional("AfterUnion", Wrapping.AfterUnion);
 IO.mapOptional("BeforeCatch", Wrapping.BeforeCatch);
 IO.mapOptional("BeforeElse", Wrapping.BeforeElse);
+IO.mapOptional("BeforeWhileInDoWhile", Wrapping.BeforeWhileInDoWhile);
 IO.mapOptional("IndentBraces", Wrapping.IndentBraces);
   }
 };
@@ -418,7 +419,7 @@
 return Style;
   FormatStyle Expanded = Style;
   Expanded.BraceWrapping = {false, false, false, false, false, false,
-false, false, false, false, false};
+false, false, false, false, false, false};
   switch (Style.BreakBeforeBraces) {
   case FormatStyle::BS_Linux:
 Expanded.BraceWrapping.AfterClass = true;
@@ -450,7 +451,7 @@
 break;
   case FormatStyle::BS_GNU:
 Expanded.BraceWrapping = {true, true, true, true, true, true,
-  true, true, true, true, true};
+  true, true, true, true, true, true};
 break;
   case FormatStyle::BS_WebKit:
 Expanded.BraceWrapping.AfterFunction = true;
@@ -487,7 +488,7 @@
   LLVMStyle.BreakBeforeTernaryOperators = true;
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
   LLVMStyle.BraceWrapping = {false, false, false, false, false, false,
- false, false, false, false, false};
+ false, false, false, false, false, false};
   LLVMStyle.BreakConstructorInitializersBeforeComma = false;
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.ColumnLimit = 80;
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -265,6 +265,8 @@
 bool BeforeCatch;
 /// \brief Wrap before \c else.
 bool BeforeElse;
+/// \brief Wrap before \c while in do...while.
+bool BeforeWhileInDoWhile;
 /// \brief Indent the wrapped braces themselves.
 bool IndentBraces;
   };
Index: docs/ClangFormatStyleOptions.rst
===
--- docs/ClangFormatStyleOptions.rst
+++ docs/ClangFormatStyleOptions.rst
@@ -324,6 +324,7 @@
   * ``bool AfterUnion`` Wrap union definitions.
   * ``bool BeforeCatch`` Wrap before ``catch``.
   * ``bool BeforeElse`` Wrap before ``else``.
+  * ``bool BeforeWhileInDoWhile`` Wrap before ``while`` in do...while.
   * ``bool IndentBraces`` Indent the wrapped braces themselves.
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16066: Add BeforeWhileInDoWhile BraceWrapping option

[Eric Baker via cfe-commits]

ebaker355 updated this revision to Diff 44506.
ebaker355 added a comment.

Remove unintended blank line change.


http://reviews.llvm.org/D16066

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9823,6 +9823,7 @@
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterUnion);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeCatch);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeElse);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeWhileInDoWhile);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, IndentBraces);
 }
 
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1472,7 +1472,10 @@
 parseStructuralElement();
 --Line->Level;
   }
-
+  if (FormatTok->Tok.is(tok::kw_while) &&
+  Style.BraceWrapping.BeforeWhileInDoWhile) {
+addUnwrappedLine();
+  }
   // FIXME: Add error handling.
   if (!FormatTok->Tok.is(tok::kw_while)) {
 addUnwrappedLine();
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -347,6 +347,7 @@
 IO.mapOptional("AfterUnion", Wrapping.AfterUnion);
 IO.mapOptional("BeforeCatch", Wrapping.BeforeCatch);
 IO.mapOptional("BeforeElse", Wrapping.BeforeElse);
+IO.mapOptional("BeforeWhileInDoWhile", Wrapping.BeforeWhileInDoWhile);
 IO.mapOptional("IndentBraces", Wrapping.IndentBraces);
   }
 };
@@ -418,7 +419,7 @@
 return Style;
   FormatStyle Expanded = Style;
   Expanded.BraceWrapping = {false, false, false, false, false, false,
-false, false, false, false, false};
+false, false, false, false, false, false};
   switch (Style.BreakBeforeBraces) {
   case FormatStyle::BS_Linux:
 Expanded.BraceWrapping.AfterClass = true;
@@ -450,7 +451,7 @@
 break;
   case FormatStyle::BS_GNU:
 Expanded.BraceWrapping = {true, true, true, true, true, true,
-  true, true, true, true, true};
+  true, true, true, true, true, true};
 break;
   case FormatStyle::BS_WebKit:
 Expanded.BraceWrapping.AfterFunction = true;
@@ -487,7 +488,7 @@
   LLVMStyle.BreakBeforeTernaryOperators = true;
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
   LLVMStyle.BraceWrapping = {false, false, false, false, false, false,
- false, false, false, false, false};
+ false, false, false, false, false, false};
   LLVMStyle.BreakConstructorInitializersBeforeComma = false;
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.ColumnLimit = 80;
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -265,6 +265,8 @@
 bool BeforeCatch;
 /// \brief Wrap before \c else.
 bool BeforeElse;
+/// \brief Wrap before \c while in do...while.
+bool BeforeWhileInDoWhile;
 /// \brief Indent the wrapped braces themselves.
 bool IndentBraces;
   };
Index: docs/ClangFormatStyleOptions.rst
===
--- docs/ClangFormatStyleOptions.rst
+++ docs/ClangFormatStyleOptions.rst
@@ -324,6 +324,7 @@
   * ``bool AfterUnion`` Wrap union definitions.
   * ``bool BeforeCatch`` Wrap before ``catch``.
   * ``bool BeforeElse`` Wrap before ``else``.
+  * ``bool BeforeWhileInDoWhile`` Wrap before ``while`` in do...while.
   * ``bool IndentBraces`` Indent the wrapped braces themselves.
 
 


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9823,6 +9823,7 @@
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterUnion);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeCatch);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeElse);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeWhileInDoWhile);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, IndentBraces);
 }
 
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1472,7 +1472,10 @@
 parseStructuralElement();
 --Line->Level;
   }
-
+  if (FormatTok->Tok.is(tok::kw_while) &&
+  Style.BraceWrapping.BeforeWhileInDoWhile) {
+addUnwrappedLine();
+  }
   // FIXME: Add error handling.
   if (!FormatTok->Tok.is(tok::kw_while)) {
 addUnwrappedLine();
Index: lib/Format/Format.cpp

Re: [PATCH] D16058: [clang-format] Fix comment aligning when there are changes within the comment

[Benjamin Kramer via cfe-commits]

bkramer updated this revision to Diff 44508.
bkramer added a comment.

- Renamed flag to IsInsideToken and enabled it for all in-token replacements.
- TokenLength now gets updated to contain all changes on the same line if 
they're in the same token.


http://reviews.llvm.org/D16058

Files:
  lib/Format/WhitespaceManager.cpp
  lib/Format/WhitespaceManager.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1022,6 +1022,15 @@
"  lineWith(); // comment\n"
"  // at start\n"
"}"));
+  EXPECT_EQ("int xy; // a\n"
+"int z;  // b",
+format("int xy;// a\n"
+   "int z;//b"));
+  EXPECT_EQ("int xy; // a\n"
+"int z; // bb",
+format("int xy;// a\n"
+   "int z;//bb",
+   getLLVMStyleWithColumns(12)));
 
   verifyFormat("#define A  \\\n"
"  int i; /* i */   \\\n"
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -109,7 +109,8 @@
unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
unsigned NewlinesBefore, StringRef PreviousLinePostfix,
StringRef CurrentLinePrefix, tok::TokenKind Kind,
-   bool ContinuesPPDirective, bool IsStartOfDeclName);
+   bool ContinuesPPDirective, bool IsStartOfDeclName,
+   bool IsInsideTrailingCommentToken);
 
 bool CreateReplacement;
 // Changes might be in the middle of a token, so we cannot just keep the
@@ -139,6 +140,9 @@
 // comments. Uncompensated negative offset is truncated to 0.
 int Spaces;
 
+// If this change is inside of a token but not at the start of the token.
+bool IsInsideToken;
+
 // \c IsTrailingComment, \c TokenLength, \c PreviousEndOfTokenColumn and
 // \c EscapedNewlineColumn will be calculated in
 // \c calculateLineBreakInformation.
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -30,16 +30,16 @@
 unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
 unsigned NewlinesBefore, StringRef PreviousLinePostfix,
 StringRef CurrentLinePrefix, tok::TokenKind Kind, bool ContinuesPPDirective,
-bool IsStartOfDeclName)
+bool IsStartOfDeclName, bool IsInsideToken)
 : CreateReplacement(CreateReplacement),
   OriginalWhitespaceRange(OriginalWhitespaceRange),
   StartOfTokenColumn(StartOfTokenColumn), NewlinesBefore(NewlinesBefore),
   PreviousLinePostfix(PreviousLinePostfix),
   CurrentLinePrefix(CurrentLinePrefix), Kind(Kind),
   ContinuesPPDirective(ContinuesPPDirective),
   IsStartOfDeclName(IsStartOfDeclName), IndentLevel(IndentLevel),
-  Spaces(Spaces), IsTrailingComment(false), TokenLength(0),
-  PreviousEndOfTokenColumn(0), EscapedNewlineColumn(0),
+  Spaces(Spaces), IsInsideToken(IsInsideToken), IsTrailingComment(false),
+  TokenLength(0), PreviousEndOfTokenColumn(0), EscapedNewlineColumn(0),
   StartOfBlockComment(nullptr), IndentationOffset(0) {}
 
 void WhitespaceManager::reset() {
@@ -58,7 +58,8 @@
   Change(/*CreateReplacement=*/true, Tok.WhitespaceRange, IndentLevel,
  Spaces, StartOfTokenColumn, Newlines, "", "", Tok.Tok.getKind(),
  InPPDirective && !Tok.IsFirst,
- Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+ Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+ /*IsInsideToken=*/false));
 }
 
 void WhitespaceManager::addUntouchableToken(const FormatToken &Tok,
@@ -69,7 +70,8 @@
   /*CreateReplacement=*/false, Tok.WhitespaceRange, /*IndentLevel=*/0,
   /*Spaces=*/0, Tok.OriginalColumn, Tok.NewlinesBefore, "", "",
   Tok.Tok.getKind(), InPPDirective && !Tok.IsFirst,
-  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+  /*IsInsideToken=*/false));
 }
 
 void WhitespaceManager::replaceWhitespaceInToken(
@@ -82,15 +84,10 @@
   Changes.push_back(Change(
   true, SourceRange(Start, Start.getLocWithOffset(ReplaceChars)),
   IndentLevel, Spaces, std::max(0, Spaces), Newlines, PreviousPostfix,
-  CurrentPrefix,
-  // If we don't add a newline this change doesn't start a comment. Thus,
-  // when we align line comments, we don't need to treat this change as one.
-  // FIXME: We still need to take this change in account to properly
-  // calculate the new length of the comment and t

Re: [PATCH] D15796: [PATCH] clang-tidy documentation redirects

[Aaron Ballman via cfe-commits]

aaron.ballman updated this revision to Diff 44507.
aaron.ballman marked 2 inline comments as done.
aaron.ballman added a comment.

Adds the add_new_check.py changes from Alex, regenerates list.rst from the 
Python script.


http://reviews.llvm.org/D15796

Files:
  clang-tidy/add_new_check.py
  docs/clang-tidy/checks/cert-dcl03-c.rst
  docs/clang-tidy/checks/cert-dcl54-cpp.rst
  docs/clang-tidy/checks/cert-dcl59-cpp.rst
  docs/clang-tidy/checks/cert-err61-cpp.rst
  docs/clang-tidy/checks/cert-fio38-c.rst
  docs/clang-tidy/checks/cert-oop11-cpp.rst
  docs/clang-tidy/checks/google-build-namespaces.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-move-constructor-init.rst
  docs/clang-tidy/checks/misc-new-delete-overloads.rst
  docs/clang-tidy/checks/misc-non-copyable-objects.rst
  docs/clang-tidy/checks/misc-static-assert.rst
  docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst

Index: docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
===
--- docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
+++ docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
@@ -3,6 +3,8 @@
 misc-throw-by-value-catch-by-reference
 ==
 
+"cert-err61-cpp" redirects here as an alias for this checker.
+
 Finds violations of the rule "Throw by value, catch by reference" presented for example in "C++ Coding Standards" by H. Sutter and A. Alexandrescu. This check also has the option to find violations of the rule "Throw anonymous temporaries" (https://www.securecoding.cert.org/confluence/display/cplusplus/ERR09-CPP.+Throw+anonymous+temporaries). The option is named "CheckThrowTemporaries" and it's on by default.
 
 Exceptions:
Index: docs/clang-tidy/checks/misc-static-assert.rst
===
--- docs/clang-tidy/checks/misc-static-assert.rst
+++ docs/clang-tidy/checks/misc-static-assert.rst
@@ -3,6 +3,7 @@
 misc-static-assert
 ==
 
+"cert-dcl03-c" redirects here as an alias for this checker.
 
 Replaces ``assert()`` with ``static_assert()`` if the condition is evaluatable
 at compile time.
Index: docs/clang-tidy/checks/misc-non-copyable-objects.rst
===
--- docs/clang-tidy/checks/misc-non-copyable-objects.rst
+++ docs/clang-tidy/checks/misc-non-copyable-objects.rst
@@ -3,6 +3,8 @@
 misc-non-copyable-objects
 =
 
+"cert-fio38-c" redirects here as an alias for this checker.
+
 The check flags dereferences and non-pointer declarations of objects that are
 not meant to be passed by value, such as C FILE objects or POSIX
 pthread_mutex_t objects.
Index: docs/clang-tidy/checks/misc-new-delete-overloads.rst
===
--- docs/clang-tidy/checks/misc-new-delete-overloads.rst
+++ docs/clang-tidy/checks/misc-new-delete-overloads.rst
@@ -3,6 +3,8 @@
 misc-new-delete-overloads
 =
 
+"cert-dcl54-cpp" redirects here as an alias for this checker.
+
 The check flags overloaded operator new() and operator delete() functions that
 do not have a corresponding free store function defined within the same scope.
 For instance, the check will flag a class implementation of a non-placement
Index: docs/clang-tidy/checks/misc-move-constructor-init.rst
===
--- docs/clang-tidy/checks/misc-move-constructor-init.rst
+++ docs/clang-tidy/checks/misc-move-constructor-init.rst
@@ -3,6 +3,7 @@
 misc-move-constructor-init
 ==
 
+"cert-oop11-cpp" redirects here as an alias for this checker.
 
 The check flags user-defined move constructors that have a ctor-initializer
 initializing a member or base class through a copy constructor instead of a
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,10 +4,16 @@
 =
 
 .. toctree::   
+   cert-dcl03-c (redirects to misc-static-assert) 
cert-dcl50-cpp
+   cert-dcl54-cpp (redirects to misc-new-delete-overloads) 
+   cert-dcl59-cpp (redirects to google-build-namespaces) 
cert-err52-cpp
cert-err58-cpp
cert-err60-cpp
+   cert-err61-cpp (redirects to misc-throw-by-value-catch-by-reference) 
+   cert-fio38-c (redirects to misc-non-copyable-objects) 
+   cert-oop11-cpp (redirects to misc-move-constructor-init) 
cppcoreguidelines-pro-bounds-array-to-pointer-decay
cppcoreguidelines-pro-bounds-constant-array-index
cppcoreguidelines-pro-bounds-pointer-arithmetic
Index: docs/clang-tidy/checks/google-build-namespaces.rst
===
--- docs/clang-tidy/checks/google-build-namespaces.rst
+++ docs/clang-tidy/checks/googl

Re: [PATCH] D16063: [Analyzer] Use a wider integer type for an array index

[Aleksei Sidorin via cfe-commits]

a.sidorin added a comment.

In http://reviews.llvm.org/D16063#323462, @xazax.hun wrote:

> How is this array index type used?


This type is used to make all the symbolic values for indices to have the same 
integer type (in SValBuilder).

In http://reviews.llvm.org/D16063#323462, @xazax.hun wrote:

> Should it support negative values? If the answer is no, ASTContext has a 
> getSizeType method.


They should support negative values because we are allowed to use negative 
index expressions.

In http://reviews.llvm.org/D16063#323462, @xazax.hun wrote:

> If it is yes, it also has a getPointerDiffType method (although it returns a 
> QualType not a CanQualType). Is there a reason not to use them?


As I described before, PtrDiffType is signed and is limited to SIZE_MAX/2. 
However, we are allowed to create arrays with the size more than SIZE_MAX/2 
(see testIndexTooBig() test for details). But we should not loose the ability 
to handle such arrays and their indices.That's why I selected a 64-bit 
arch-independent type.


Repository:
  rL LLVM

http://reviews.llvm.org/D16063



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

Re: [PATCH] D16058: [clang-format] Fix comment aligning when there are changes within the comment

[Daniel Jasper via cfe-commits]

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

I think this looks good, but I'd also like Manuel to take a look.


http://reviews.llvm.org/D16058



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

Re: [PATCH] D15914: [OpenCL] Pipe builtin functions

[Anastasia Stulova via cfe-commits]

Anastasia added inline comments.


Comment at: include/clang/Basic/Builtins.def:1260
@@ +1259,3 @@
+
+LANGBUILTIN(reserve_read_pipe, "i.", "tn", OCLC_LANG)
+LANGBUILTIN(reserve_write_pipe, "i.", "tn", OCLC_LANG)

Ok, this way it means variable number of arg which isn't right either. But I 
see some other builtins do the same.

I think a better approach would be to add pipe as a modifier and a way to 
represent any gentype in builtins declarations here to be able to specify a 
signature properly. Although it seems it won't be used for anything but 
documentation purposes.

Also adding the overloaded variant for read/write_pipes explicitly might be a 
good idea (similar to sync_fetch_and_add).

But I see that not all builtins follow this approach, so it's up to you.


Comment at: lib/CodeGen/CGBuiltin.cpp:2033
@@ +2032,3 @@
+  *Arg1 = EmitScalarExpr(E->getArg(1));
+llvm::Type *ReservedIDTy = ConvertType(getContext().OCLReserveIDTy);
+

Why do we need to modify user defined functions? Only builtins will have this 
extra parameter.

I think packet size would be useful for builtin implementation to know the 
number of bytes to be read/written. I don't see how to implement it correctly 
otherwise. As mentioned earlier relying on the metadata is not a conventional 
compilation approach and should be avoided if possible.


Comment at: lib/Sema/SemaChecking.cpp:268
@@ +267,3 @@
+// TODO:Refine OpenCLImageAccessAttr to OpenCLArgAccessAttr since pipe can use
+// it too
+static OpenCLImageAccessAttr *getOpenCLArgAccess(const Decl *D) {

Yes, they should. It seems however it has already been checked here?


Comment at: lib/Sema/SemaChecking.cpp:274
@@ +273,3 @@
+}
+
+/// Returns true if pipe element type is different from the pointer.

Makes sense!


Comment at: lib/Sema/SemaChecking.cpp:291
@@ +290,3 @@
+  bool isValid = true;
+  // TODO: For all pipe built-in read is for read_only?
+  bool ReadOnly = getFunctionName(Call).find("read") != StringRef::npos;

I agree the spec doesn't require that checking but I think it's just being 
imprecise in the description. If you are in doubt you can raise a bug with 
Khronos to clarify. This might result in additional latency.

I think it makes sense though to check all of them.


Comment at: test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl:4
@@ +3,3 @@
+void test(read_only pipe int p, global int* ptr){
+  int tmp;
+  read_pipe(tmp, p);// expected-error {{first argument to read_pipe must 
be a pipe type}}

I think we need to test all the builtins!


http://reviews.llvm.org/D15914



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

Re: [PATCH] D16063: [Analyzer] Use a wider integer type for an array index

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

xazax.hun added a comment.

In http://reviews.llvm.org/D16063#323513, @a.sidorin wrote:

> As I described before, PtrDiffType is signed and is limited to SIZE_MAX/2. 
> However, we are allowed to create arrays with the size more than SIZE_MAX/2 
> (see testIndexTooBig() test for details). But we should not loose the ability 
> to handle such arrays and their indices.That's why I selected a 64-bit 
> arch-independent type.


Oh, I see now, thanks. My only concern left is that, is it guaranteed that 
LongLongTy is available on all supported platforms including embedded ones?


Repository:
  rL LLVM

http://reviews.llvm.org/D16063



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

Re: [PATCH] D16040: [OpenCL] Refine OpenCLImageAccessAttr to OpenCLAccessAttr

[Anastasia Stulova via cfe-commits]

Anastasia added inline comments.


Comment at: lib/Sema/SemaDeclAttr.cpp:4934
@@ +4933,3 @@
+const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
+if (AccessAttr->isReadWrite()) {
+  if (DeclTy->isPipeType() ||

In the case of failure, where would destructor of AccessAttr be called?

Could we invoke it before exiting? Or alternatively it's possible to check an 
access kind based on string matching from Attr getName() and create AccessAttr 
later on success.


Comment at: lib/Sema/SemaDeclAttr.cpp:5694
@@ +5693,3 @@
+// attribute and may cause the same attribute to be added twice
+if (const ParmVarDecl *PDecl = llvm::dyn_cast(D)) {
+  if (PDecl->getType().getCanonicalType().getTypePtr()->isPipeType())

So where would the attribute check happen? I don't think the comment is very 
clear.


Comment at: lib/Sema/SemaType.cpp:6223
@@ +6222,3 @@
+   Sema &S) {
+  // OpenCL v2.0 s6.6 -- Access Qualifier can used only for image and pipe type
+  if (!(CurType->isImageType() || CurType->isPipeType())) {

remove one -


Comment at: test/SemaOpenCL/invalid-kernel-attrs.cl:31
@@ -30,3 +30,3 @@
   int __kernel x; // expected-error {{'__kernel' attribute only applies to 
functions}}
-  read_only int i; // expected-error {{'read_only' attribute only applies to 
parameters}}
-  __write_only int j; // expected-error {{'__write_only' attribute only 
applies to parameters}}
+  read_only image1d_t i; // expected-error {{'read_only' attribute only 
applies to parameters}}
+  __write_only image2d_t j; // expected-error {{'__write_only' attribute only 
applies to parameters}}

Strangely, I don't see this diagnostic being added in this patch.


http://reviews.llvm.org/D16040



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

Re: [PATCH] D15823: Support virtual-near-miss check.

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

A few minor comments.



Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:23
@@ +22,3 @@
+/// Finds out if the given method overrides some method.
+bool isOverrideMethod(const CXXMethodDecl *MD) {
+  return MD->size_overridden_methods() > 0 || MD->hasAttr();

Please mark this and other functions `static` to make them local to the 
translation unit.


Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:43
@@ +42,3 @@
+
+  // Check if return types are identical
+  if (Context->hasSameType(DerivedReturnTy, BaseReturnTy))

nit: Missing trailing period.


Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:68
@@ +67,3 @@
+  // The return types aren't either both pointers or references to a class 
type.
+  if (DTy.isNull()) {
+return false;

nit: No braces around one-line `if` bodies, please.


Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:90
@@ +89,3 @@
+// Check ambiguity.
+if (Paths.isAmbiguous(Context->getCanonicalType(BTy).getUnqualifiedType()))
+  return false;

I wonder whether `BTy.getCanonicalType().getUnqualifiedType()` will work here.


Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:99
@@ +98,3 @@
+for (const auto &Path : Paths) {
+  if (Path.Access == AS_public) {
+HasPublicAccess = true;

nit: No braces around one-line `if` bodies, please.


Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:188
@@ +187,3 @@
+  auto Iter = PossibleMap.find(Id);
+  if (Iter != PossibleMap.end()) {
+return Iter->second;

nit: No braces around one-line `if` bodies, please.


Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:203
@@ +202,3 @@
+  auto Iter = OverriddenMap.find(Key);
+  if (Iter != OverriddenMap.end()) {
+return Iter->second;

nit: No braces around one-line `if` bodies, please.


Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:258
@@ +257,3 @@
+ "'%0' has a similar name and the same "
+ "signature with virtual method '%1'. "
+ "Did you meant to override it?")

The diagnostic messages are not complete sentences. Please use a semicolon 
instead of the period and a lower-case letter after it. Also, s/signature 
with/signature as/ and s/meant/mean/. Overall it should be:
`method '%0' has a similar name and the same signature as method '%1'; did you 
mean to override it?`.


Comment at: test/clang-tidy/misc-virtual-near-miss.cpp:15
@@ +14,3 @@
+  void func2();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'Derived::func2' has a similar 
name and the same signature with virtual method 'Base::func'. Did you meant to 
override it?
+

Please remove the `. Did you mean to override it?` part from all CHECK lines 
except for the first one to make the test file easier to read. Maybe even cut 
the static test in the middle:

  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'Derived::func2' has {{.*}} 
'Base::func'


Comment at: test/clang-tidy/misc-virtual-near-miss.cpp:42
@@ +41,3 @@
+class Child : Father, Mother {
+public:
+  Child();

Please mark the inheritance explicitly `private` then to avoid questions.


http://reviews.llvm.org/D15823



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

Re: [PATCH] D16058: [clang-format] Fix comment aligning when there are changes within the comment

[Benjamin Kramer via cfe-commits]

bkramer updated this revision to Diff 44513.
bkramer added a comment.

- Moved newline check into replaceWhitespaceInToken
- Removed duplicated TokenLength computation


http://reviews.llvm.org/D16058

Files:
  lib/Format/WhitespaceManager.cpp
  lib/Format/WhitespaceManager.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1022,6 +1022,15 @@
"  lineWith(); // comment\n"
"  // at start\n"
"}"));
+  EXPECT_EQ("int xy; // a\n"
+"int z;  // b",
+format("int xy;// a\n"
+   "int z;//b"));
+  EXPECT_EQ("int xy; // a\n"
+"int z; // bb",
+format("int xy;// a\n"
+   "int z;//bb",
+   getLLVMStyleWithColumns(12)));
 
   verifyFormat("#define A  \\\n"
"  int i; /* i */   \\\n"
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -109,7 +109,8 @@
unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
unsigned NewlinesBefore, StringRef PreviousLinePostfix,
StringRef CurrentLinePrefix, tok::TokenKind Kind,
-   bool ContinuesPPDirective, bool IsStartOfDeclName);
+   bool ContinuesPPDirective, bool IsStartOfDeclName,
+   bool IsInsideTrailingCommentToken);
 
 bool CreateReplacement;
 // Changes might be in the middle of a token, so we cannot just keep the
@@ -139,6 +140,9 @@
 // comments. Uncompensated negative offset is truncated to 0.
 int Spaces;
 
+// If this change is inside of a token but not at the start of the token.
+bool IsInsideToken;
+
 // \c IsTrailingComment, \c TokenLength, \c PreviousEndOfTokenColumn and
 // \c EscapedNewlineColumn will be calculated in
 // \c calculateLineBreakInformation.
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -30,16 +30,16 @@
 unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
 unsigned NewlinesBefore, StringRef PreviousLinePostfix,
 StringRef CurrentLinePrefix, tok::TokenKind Kind, bool ContinuesPPDirective,
-bool IsStartOfDeclName)
+bool IsStartOfDeclName, bool IsInsideToken)
 : CreateReplacement(CreateReplacement),
   OriginalWhitespaceRange(OriginalWhitespaceRange),
   StartOfTokenColumn(StartOfTokenColumn), NewlinesBefore(NewlinesBefore),
   PreviousLinePostfix(PreviousLinePostfix),
   CurrentLinePrefix(CurrentLinePrefix), Kind(Kind),
   ContinuesPPDirective(ContinuesPPDirective),
   IsStartOfDeclName(IsStartOfDeclName), IndentLevel(IndentLevel),
-  Spaces(Spaces), IsTrailingComment(false), TokenLength(0),
-  PreviousEndOfTokenColumn(0), EscapedNewlineColumn(0),
+  Spaces(Spaces), IsInsideToken(IsInsideToken), IsTrailingComment(false),
+  TokenLength(0), PreviousEndOfTokenColumn(0), EscapedNewlineColumn(0),
   StartOfBlockComment(nullptr), IndentationOffset(0) {}
 
 void WhitespaceManager::reset() {
@@ -58,7 +58,8 @@
   Change(/*CreateReplacement=*/true, Tok.WhitespaceRange, IndentLevel,
  Spaces, StartOfTokenColumn, Newlines, "", "", Tok.Tok.getKind(),
  InPPDirective && !Tok.IsFirst,
- Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+ Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+ /*IsInsideToken=*/false));
 }
 
 void WhitespaceManager::addUntouchableToken(const FormatToken &Tok,
@@ -69,7 +70,8 @@
   /*CreateReplacement=*/false, Tok.WhitespaceRange, /*IndentLevel=*/0,
   /*Spaces=*/0, Tok.OriginalColumn, Tok.NewlinesBefore, "", "",
   Tok.Tok.getKind(), InPPDirective && !Tok.IsFirst,
-  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+  /*IsInsideToken=*/false));
 }
 
 void WhitespaceManager::replaceWhitespaceInToken(
@@ -82,15 +84,10 @@
   Changes.push_back(Change(
   true, SourceRange(Start, Start.getLocWithOffset(ReplaceChars)),
   IndentLevel, Spaces, std::max(0, Spaces), Newlines, PreviousPostfix,
-  CurrentPrefix,
-  // If we don't add a newline this change doesn't start a comment. Thus,
-  // when we align line comments, we don't need to treat this change as one.
-  // FIXME: We still need to take this change in account to properly
-  // calculate the new length of the comment and to calculate the changes
-  // for which to do the alignment when aligning comme

Re: [PATCH] D16058: [clang-format] Fix comment aligning when there are changes within the comment

[Manuel Klimek via cfe-commits]

klimek accepted this revision.
klimek added a reviewer: klimek.
klimek added a comment.

lg



Comment at: lib/Format/WhitespaceManager.h:113
@@ -113,1 +112,3 @@
+   bool ContinuesPPDirective, bool IsStartOfDeclName,
+   bool IsInsideTrailingCommentToken);
 

Forgot one replacement of the original name.


Comment at: lib/Format/WhitespaceManager.h:143
@@ -141,1 +142,3 @@
 
+// If this change is inside of a token but not at the start of the token.
+bool IsInsideToken;

... or starting a new line.


http://reviews.llvm.org/D16058



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

Re: [PATCH] D16041: Change vfs::FileSystem to be managed with std::shared_ptr

[David Blaikie via cfe-commits]

On Sun, Jan 10, 2016 at 11:42 PM, Owen Anderson via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> resistor created this revision.
> resistor added reviewers: chandlerc, bkramer, klimek.
> resistor added a subscriber: cfe-commits.
> resistor set the repository for this revision to rL LLVM.
> Herald added a subscriber: klimek.
>
> Managing it with IntrusiveRefCntPtr caused the virtual destructor not to
> be called properly.
>

Regardless of the broader discussion on this patch, I'm confused by why
this ^ would be the case. What is it that IntrusiveRefCntPtr is doing
that's causing problems with destruction? (& I'm all for changing this to
non-intrusive smart pointers over intrusive ones anyway, but I'd still like
to understand the extra motivation here)


>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D16041
>
> Files:
>   include/clang/Basic/FileManager.h
>   include/clang/Basic/VirtualFileSystem.h
>   include/clang/Driver/Driver.h
>   include/clang/Frontend/CompilerInstance.h
>   include/clang/Frontend/CompilerInvocation.h
>   include/clang/Tooling/Tooling.h
>   lib/Basic/FileManager.cpp
>   lib/Basic/VirtualFileSystem.cpp
>   lib/Driver/Driver.cpp
>   lib/Format/Format.cpp
>   lib/Frontend/ASTUnit.cpp
>   lib/Frontend/CompilerInstance.cpp
>   lib/Frontend/CompilerInvocation.cpp
>   lib/Frontend/FrontendAction.cpp
>   lib/Index/SimpleFormatContext.h
>   lib/StaticAnalyzer/Frontend/ModelInjector.cpp
>   lib/Tooling/Core/Replacement.cpp
>   lib/Tooling/Tooling.cpp
>   tools/clang-format/ClangFormat.cpp
>   unittests/Basic/VirtualFileSystemTest.cpp
>   unittests/Driver/ToolChainTest.cpp
>   unittests/Lex/PPCallbacksTest.cpp
>   unittests/Tooling/RewriterTestContext.h
>   unittests/Tooling/ToolingTest.cpp
>
>
> ___
> 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

r257341 - [clang-format] Fix comment aligning when there are changes within the comment

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Mon Jan 11 10:27:16 2016
New Revision: 257341

URL: http://llvm.org/viewvc/llvm-project?rev=257341&view=rev
Log:
[clang-format] Fix comment aligning when there are changes within the comment

As soon as a comment had whitespace changes inside of the token, we
couldn't identify the whole comment as a trailing comment anymore and
alignment stopped working. Add a new boolean to Change for this special
case and fix trailing comment identification to use it.

This also changes WhitespaceManager to sum the length of all Changes
inside of a token into the first Change.

Before this fix

  int xy;  // a
  int z;   //b

became

  int xy;  // a
  int z;  // b

with this patch we immediately get to:

  int xy;  // a
  int z;   // b

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

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

Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=257341&r1=257340&r2=257341&view=diff
==
--- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp Mon Jan 11 10:27:16 2016
@@ -30,7 +30,7 @@ WhitespaceManager::Change::Change(
 unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
 unsigned NewlinesBefore, StringRef PreviousLinePostfix,
 StringRef CurrentLinePrefix, tok::TokenKind Kind, bool 
ContinuesPPDirective,
-bool IsStartOfDeclName)
+bool IsStartOfDeclName, bool IsInsideToken)
 : CreateReplacement(CreateReplacement),
   OriginalWhitespaceRange(OriginalWhitespaceRange),
   StartOfTokenColumn(StartOfTokenColumn), NewlinesBefore(NewlinesBefore),
@@ -38,8 +38,8 @@ WhitespaceManager::Change::Change(
   CurrentLinePrefix(CurrentLinePrefix), Kind(Kind),
   ContinuesPPDirective(ContinuesPPDirective),
   IsStartOfDeclName(IsStartOfDeclName), IndentLevel(IndentLevel),
-  Spaces(Spaces), IsTrailingComment(false), TokenLength(0),
-  PreviousEndOfTokenColumn(0), EscapedNewlineColumn(0),
+  Spaces(Spaces), IsInsideToken(IsInsideToken), IsTrailingComment(false),
+  TokenLength(0), PreviousEndOfTokenColumn(0), EscapedNewlineColumn(0),
   StartOfBlockComment(nullptr), IndentationOffset(0) {}
 
 void WhitespaceManager::reset() {
@@ -58,7 +58,8 @@ void WhitespaceManager::replaceWhitespac
   Change(/*CreateReplacement=*/true, Tok.WhitespaceRange, IndentLevel,
  Spaces, StartOfTokenColumn, Newlines, "", "", Tok.Tok.getKind(),
  InPPDirective && !Tok.IsFirst,
- Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+ Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+ /*IsInsideToken=*/false));
 }
 
 void WhitespaceManager::addUntouchableToken(const FormatToken &Tok,
@@ -69,7 +70,8 @@ void WhitespaceManager::addUntouchableTo
   /*CreateReplacement=*/false, Tok.WhitespaceRange, /*IndentLevel=*/0,
   /*Spaces=*/0, Tok.OriginalColumn, Tok.NewlinesBefore, "", "",
   Tok.Tok.getKind(), InPPDirective && !Tok.IsFirst,
-  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+  /*IsInsideToken=*/false));
 }
 
 void WhitespaceManager::replaceWhitespaceInToken(
@@ -82,15 +84,10 @@ void WhitespaceManager::replaceWhitespac
   Changes.push_back(Change(
   true, SourceRange(Start, Start.getLocWithOffset(ReplaceChars)),
   IndentLevel, Spaces, std::max(0, Spaces), Newlines, PreviousPostfix,
-  CurrentPrefix,
-  // If we don't add a newline this change doesn't start a comment. Thus,
-  // when we align line comments, we don't need to treat this change as 
one.
-  // FIXME: We still need to take this change in account to properly
-  // calculate the new length of the comment and to calculate the changes
-  // for which to do the alignment when aligning comments.
-  Tok.is(TT_LineComment) && Newlines > 0 ? tok::comment : tok::unknown,
+  CurrentPrefix, Tok.is(TT_LineComment) ? tok::comment : tok::unknown,
   InPPDirective && !Tok.IsFirst,
-  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+  /*IsInsideToken=*/Newlines == 0));
 }
 
 const tooling::Replacements &WhitespaceManager::generateReplacements() {
@@ -110,6 +107,7 @@ const tooling::Replacements &WhitespaceM
 
 void WhitespaceManager::calculateLineBreakInformation() {
   Changes[0].PreviousEndOfTokenColumn = 0;
+  Change *LastOutsideTokenChange = &Changes[0];
   for (unsigned i = 1, e = Changes.size(); i != e; ++i) {
 unsigned OriginalWhitespaceStart =
 SourceMgr.getFileOffset(Changes[i].OriginalWhitespaceRange.getBegin());
@@ -120,11 +118,2

Re: [PATCH] D16058: [clang-format] Fix comment aligning when there are changes within the comment

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL257341: [clang-format] Fix comment aligning when there are 
changes within the comment (authored by d0k).

Changed prior to commit:
  http://reviews.llvm.org/D16058?vs=44513&id=44517#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D16058

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

Index: cfe/trunk/lib/Format/WhitespaceManager.h
===
--- cfe/trunk/lib/Format/WhitespaceManager.h
+++ cfe/trunk/lib/Format/WhitespaceManager.h
@@ -109,7 +109,8 @@
unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
unsigned NewlinesBefore, StringRef PreviousLinePostfix,
StringRef CurrentLinePrefix, tok::TokenKind Kind,
-   bool ContinuesPPDirective, bool IsStartOfDeclName);
+   bool ContinuesPPDirective, bool IsStartOfDeclName,
+   bool IsInsideToken);
 
 bool CreateReplacement;
 // Changes might be in the middle of a token, so we cannot just keep the
@@ -139,6 +140,10 @@
 // comments. Uncompensated negative offset is truncated to 0.
 int Spaces;
 
+// If this change is inside of a token but not at the start of the token or
+// directly after a newline.
+bool IsInsideToken;
+
 // \c IsTrailingComment, \c TokenLength, \c PreviousEndOfTokenColumn and
 // \c EscapedNewlineColumn will be calculated in
 // \c calculateLineBreakInformation.
Index: cfe/trunk/lib/Format/WhitespaceManager.cpp
===
--- cfe/trunk/lib/Format/WhitespaceManager.cpp
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp
@@ -30,16 +30,16 @@
 unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
 unsigned NewlinesBefore, StringRef PreviousLinePostfix,
 StringRef CurrentLinePrefix, tok::TokenKind Kind, bool ContinuesPPDirective,
-bool IsStartOfDeclName)
+bool IsStartOfDeclName, bool IsInsideToken)
 : CreateReplacement(CreateReplacement),
   OriginalWhitespaceRange(OriginalWhitespaceRange),
   StartOfTokenColumn(StartOfTokenColumn), NewlinesBefore(NewlinesBefore),
   PreviousLinePostfix(PreviousLinePostfix),
   CurrentLinePrefix(CurrentLinePrefix), Kind(Kind),
   ContinuesPPDirective(ContinuesPPDirective),
   IsStartOfDeclName(IsStartOfDeclName), IndentLevel(IndentLevel),
-  Spaces(Spaces), IsTrailingComment(false), TokenLength(0),
-  PreviousEndOfTokenColumn(0), EscapedNewlineColumn(0),
+  Spaces(Spaces), IsInsideToken(IsInsideToken), IsTrailingComment(false),
+  TokenLength(0), PreviousEndOfTokenColumn(0), EscapedNewlineColumn(0),
   StartOfBlockComment(nullptr), IndentationOffset(0) {}
 
 void WhitespaceManager::reset() {
@@ -58,7 +58,8 @@
   Change(/*CreateReplacement=*/true, Tok.WhitespaceRange, IndentLevel,
  Spaces, StartOfTokenColumn, Newlines, "", "", Tok.Tok.getKind(),
  InPPDirective && !Tok.IsFirst,
- Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+ Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+ /*IsInsideToken=*/false));
 }
 
 void WhitespaceManager::addUntouchableToken(const FormatToken &Tok,
@@ -69,7 +70,8 @@
   /*CreateReplacement=*/false, Tok.WhitespaceRange, /*IndentLevel=*/0,
   /*Spaces=*/0, Tok.OriginalColumn, Tok.NewlinesBefore, "", "",
   Tok.Tok.getKind(), InPPDirective && !Tok.IsFirst,
-  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+  /*IsInsideToken=*/false));
 }
 
 void WhitespaceManager::replaceWhitespaceInToken(
@@ -82,15 +84,10 @@
   Changes.push_back(Change(
   true, SourceRange(Start, Start.getLocWithOffset(ReplaceChars)),
   IndentLevel, Spaces, std::max(0, Spaces), Newlines, PreviousPostfix,
-  CurrentPrefix,
-  // If we don't add a newline this change doesn't start a comment. Thus,
-  // when we align line comments, we don't need to treat this change as one.
-  // FIXME: We still need to take this change in account to properly
-  // calculate the new length of the comment and to calculate the changes
-  // for which to do the alignment when aligning comments.
-  Tok.is(TT_LineComment) && Newlines > 0 ? tok::comment : tok::unknown,
+  CurrentPrefix, Tok.is(TT_LineComment) ? tok::comment : tok::unknown,
   InPPDirective && !Tok.IsFirst,
-  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName)));
+  Tok.is(TT_StartOfName) || Tok.is(TT_FunctionDeclarationName),
+  /*IsInsideToken=*/Newlines == 0));
 }
 
 const tooling::Replacements &WhitespaceManager::generateReplacements() {
@@ -110,6 +107,7 @@
 
 void WhitespaceManager::calculateLineBreakInformation() {
   Changes[0].PreviousEn

Re: [PATCH] D15796: [PATCH] clang-tidy documentation redirects

[Alexander Kornienko via cfe-commits]

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

LG. Thank you!


http://reviews.llvm.org/D15796



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

Re: [PATCH] D15173: [Preprocessor] Fix assertion in AnnotatePreviousCachedTokens

[Bruno Cardoso Lopes via cfe-commits]

bruno added a comment.

Ping!


http://reviews.llvm.org/D15173



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

[clang-tools-extra] r257347 - Add documentation redirects for clang-tidy checkers that are exposed under multiple checker names. Updates the Python script for adding checks to properly handle these al

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Mon Jan 11 10:48:26 2016
New Revision: 257347

URL: http://llvm.org/viewvc/llvm-project?rev=257347&view=rev
Log:
Add documentation redirects for clang-tidy checkers that are exposed under 
multiple checker names. Updates the Python script for adding checks to properly 
handle these aliases.

Added:
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl03-c.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl54-cpp.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl59-cpp.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err61-cpp.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-fio38-c.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-oop11-cpp.rst
Modified:
clang-tools-extra/trunk/clang-tidy/add_new_check.py
clang-tools-extra/trunk/docs/clang-tidy/checks/google-build-namespaces.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/misc-move-constructor-init.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-new-delete-overloads.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-non-copyable-objects.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-static-assert.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst

Modified: clang-tools-extra/trunk/clang-tidy/add_new_check.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/add_new_check.py?rev=257347&r1=257346&r2=257347&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/add_new_check.py (original)
+++ clang-tools-extra/trunk/clang-tidy/add_new_check.py Mon Jan 11 10:48:26 2016
@@ -215,15 +215,25 @@ void awesome_f2();
 
 # Recreates the list of checks in the docs/clang-tidy/checks directory.
 def update_checks_list(module_path):
-  filename = os.path.normpath(
-  os.path.join(module_path, '../../docs/clang-tidy/checks/list.rst'))
+  docs_dir = os.path.join(module_path, '../../docs/clang-tidy/checks')
+  filename = os.path.normpath(os.path.join(docs_dir, 'list.rst'))
   with open(filename, 'r') as f:
 lines = f.readlines()
+  doc_files = filter(
+  lambda s: s.endswith('.rst') and s != 'list.rst',
+  os.listdir(docs_dir))
+  doc_files.sort()
 
-  checks = map(lambda s: '   ' + s.replace('.rst', '\n'),
-   filter(lambda s: s.endswith('.rst') and s != 'list.rst',
-  os.listdir(os.path.join(module_path, 
'../../docs/clang-tidy/checks'
-  checks.sort()
+  def format_link(doc_file):
+check_name = doc_file.replace('.rst', '')
+with open(os.path.join(docs_dir, doc_file), 'r') as doc:
+  match = re.search('.*:http-equiv=refresh: \d+;URL=(.*).html.*', 
doc.read())
+  if match:
+return '   %(check)s (redirects to %(target)s) <%(check)s>\n' % {
+'check' : check_name, 'target' : match.group(1) }
+  return '   %s\n' % check_name
+
+  checks = map(format_link, doc_files)
 
   print('Updating %s...' % filename)
   with open(filename, 'wb') as f:

Added: clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl03-c.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl03-c.rst?rev=257347&view=auto
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl03-c.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl03-c.rst Mon Jan 11 
10:48:26 2016
@@ -0,0 +1,9 @@
+.. title:: clang-tidy - cert-dcl03-c
+.. meta::
+   :http-equiv=refresh: 5;URL=misc-static-assert.html
+
+cert-dcl03-c
+
+
+The cert-dcl03-c checker is an alias, please see
+`misc-static-assert `_ for more information.

Added: clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl54-cpp.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl54-cpp.rst?rev=257347&view=auto
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl54-cpp.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl54-cpp.rst Mon Jan 
11 10:48:26 2016
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - cert-dcl54-cpp
+.. meta::
+   :http-equiv=refresh: 5;URL=misc-new-delete-overloads.html
+
+cert-dcl54-cpp
+==
+
+The cert-dcl54-cpp checker is an alias, please see
+`misc-new-delete-overloads `_ for more
+information.

Added: clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl59-cpp.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl59-cpp.rst?rev=257347&view=auto
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl59-cpp.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl

Re: [PATCH] D15796: [PATCH] clang-tidy documentation redirects

[Aaron Ballman via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thanks! I've commit in r257347.


http://reviews.llvm.org/D15796



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

Re: [PATCH] D14980: PR18513: make gcc compatible layout for bit-fields with explicit aligned attribute

[John McCall via cfe-commits]

rjmccall added a comment.

Sorry, holidays.  I'm comfortable with taking this patch as-is.


http://reviews.llvm.org/D14980



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

Re: [PATCH] D15097: [Sema] Issue a warning for integer overflow in struct initializer

[Akira Hatanaka via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL257357: [Sema] Issue a warning for integer overflow in 
struct initializer (authored by ahatanak).

Changed prior to commit:
  http://reviews.llvm.org/D15097?vs=41451&id=44521#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D15097

Files:
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/Sema/integer-overflow.c

Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -7853,6 +7853,10 @@
 void Sema::CheckForIntOverflow (Expr *E) {
   if (isa(E->IgnoreParenCasts()))
 E->IgnoreParenCasts()->EvaluateForOverflow(Context);
+  else if (auto InitList = dyn_cast(E))
+for (Expr *E : InitList->inits())
+  if (isa(E->IgnoreParenCasts()))
+E->IgnoreParenCasts()->EvaluateForOverflow(Context);
 }
 
 namespace {
Index: cfe/trunk/test/Sema/integer-overflow.c
===
--- cfe/trunk/test/Sema/integer-overflow.c
+++ cfe/trunk/test/Sema/integer-overflow.c
@@ -145,3 +145,11 @@
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with 
type 'int'}}
   return ((4608 * 1024 * 1024) + ((uint64_t)(4608 * 1024 * 1024)));
 }
+
+struct s {
+  unsigned x;
+  unsigned y;
+} s = {
+  .y = 5,
+  .x = 4 * 1024 * 1024 * 1024  // expected-warning {{overflow in expression; 
result is 0 with type 'int'}}
+};


Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -7853,6 +7853,10 @@
 void Sema::CheckForIntOverflow (Expr *E) {
   if (isa(E->IgnoreParenCasts()))
 E->IgnoreParenCasts()->EvaluateForOverflow(Context);
+  else if (auto InitList = dyn_cast(E))
+for (Expr *E : InitList->inits())
+  if (isa(E->IgnoreParenCasts()))
+E->IgnoreParenCasts()->EvaluateForOverflow(Context);
 }
 
 namespace {
Index: cfe/trunk/test/Sema/integer-overflow.c
===
--- cfe/trunk/test/Sema/integer-overflow.c
+++ cfe/trunk/test/Sema/integer-overflow.c
@@ -145,3 +145,11 @@
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}}
   return ((4608 * 1024 * 1024) + ((uint64_t)(4608 * 1024 * 1024)));
 }
+
+struct s {
+  unsigned x;
+  unsigned y;
+} s = {
+  .y = 5,
+  .x = 4 * 1024 * 1024 * 1024  // expected-warning {{overflow in expression; result is 0 with type 'int'}}
+};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r257357 - [Sema] Issue a warning for integer overflow in struct initializer

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Mon Jan 11 11:22:01 2016
New Revision: 257357

URL: http://llvm.org/viewvc/llvm-project?rev=257357&view=rev
Log:
[Sema] Issue a warning for integer overflow in struct initializer

Clang wasn't issuing a warning when compiling the following code:

struct s {
  unsigned x;
} s = {
  .x = 4 * 1024 * 1024 * 1024
};

rdar://problem/23399683

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

Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/integer-overflow.c

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=257357&r1=257356&r2=257357&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Jan 11 11:22:01 2016
@@ -7853,6 +7853,10 @@ void Sema::CheckBoolLikeConversion(Expr
 void Sema::CheckForIntOverflow (Expr *E) {
   if (isa(E->IgnoreParenCasts()))
 E->IgnoreParenCasts()->EvaluateForOverflow(Context);
+  else if (auto InitList = dyn_cast(E))
+for (Expr *E : InitList->inits())
+  if (isa(E->IgnoreParenCasts()))
+E->IgnoreParenCasts()->EvaluateForOverflow(Context);
 }
 
 namespace {

Modified: cfe/trunk/test/Sema/integer-overflow.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/integer-overflow.c?rev=257357&r1=257356&r2=257357&view=diff
==
--- cfe/trunk/test/Sema/integer-overflow.c (original)
+++ cfe/trunk/test/Sema/integer-overflow.c Mon Jan 11 11:22:01 2016
@@ -145,3 +145,11 @@ uint64_t check_integer_overflows(int i)
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with 
type 'int'}}
   return ((4608 * 1024 * 1024) + ((uint64_t)(4608 * 1024 * 1024)));
 }
+
+struct s {
+  unsigned x;
+  unsigned y;
+} s = {
+  .y = 5,
+  .x = 4 * 1024 * 1024 * 1024  // expected-warning {{overflow in expression; 
result is 0 with type 'int'}}
+};


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

Re: [PATCH] D15450: Avoid double deletion in Clang driver.

[Serge Pavlov via cfe-commits]

Any feedback?

Thanks,
--Serge

2015-12-11 20:24 GMT+06:00 Serge Pavlov :

> sepavloff created this revision.
> sepavloff added a subscriber: cfe-commits.
>
> Llvm module object is shared between CodeGenerator and BackendConsumer,
> in both classes it is stored as std::unique_ptr, which is not a good
> design solution and can cause double deletion error. Usually it does
> not occur because in BackendConsumer::HandleTranslationUnit the
> ownership of CodeGenerator over the module is taken away. If however
> this method is not called, the module is deleted twice and compiler
> crashes.
>
> As the module owned by BackendConsumer is always the same as CodeGenerator
> has, local copy of llvm module can be removed from BackendGenerator.
>
> http://reviews.llvm.org/D15450
>
> Files:
>   lib/CodeGen/CodeGenAction.cpp
>
> Index: lib/CodeGen/CodeGenAction.cpp
> ===
> --- lib/CodeGen/CodeGenAction.cpp
> +++ lib/CodeGen/CodeGenAction.cpp
> @@ -73,7 +73,6 @@
>
>  std::unique_ptr Gen;
>
> -std::unique_ptr TheModule;
>  SmallVector>, 4>
>  LinkModules;
>
> @@ -97,7 +96,10 @@
>  this->LinkModules.push_back(
>  std::make_pair(I.first,
> std::unique_ptr(I.second)));
>  }
> -std::unique_ptr takeModule() { return
> std::move(TheModule); }
> +llvm::Module *getModule() const { return Gen->GetModule(); }
> +std::unique_ptr takeModule() {
> +  return std::unique_ptr(Gen->ReleaseModule());
> +}
>  void releaseLinkModules() {
>for (auto &I : LinkModules)
>  I.second.release();
> @@ -117,8 +119,6 @@
>
>Gen->Initialize(Ctx);
>
> -  TheModule.reset(Gen->GetModule());
> -
>if (llvm::TimePassesIsEnabled)
>  LLVMIRGeneration.stopTimer();
>  }
> @@ -165,27 +165,14 @@
>}
>
>// Silently ignore if we weren't initialized for some reason.
> -  if (!TheModule)
> -return;
> -
> -  // Make sure IR generation is happy with the module. This is
> released by
> -  // the module provider.
> -  llvm::Module *M = Gen->ReleaseModule();
> -  if (!M) {
> -// The module has been released by IR gen on failures, do not
> double
> -// free.
> -TheModule.release();
> +  if (!getModule())
>  return;
> -  }
> -
> -  assert(TheModule.get() == M &&
> - "Unexpected module change during IR generation");
>
>// Link LinkModule into this module if present, preserving its
> validity.
>for (auto &I : LinkModules) {
>  unsigned LinkFlags = I.first;
>  llvm::Module *LinkModule = I.second.get();
> -if (Linker::linkModules(*M, *LinkModule,
> +if (Linker::linkModules(*getModule(), *LinkModule,
>  [=](const DiagnosticInfo &DI) {
>linkerDiagnosticHandler(DI, LinkModule,
> Diags);
>  },
> @@ -195,7 +182,7 @@
>
>// Install an inline asm handler so that diagnostics get printed
> through
>// our diagnostics hooks.
> -  LLVMContext &Ctx = TheModule->getContext();
> +  LLVMContext &Ctx = getModule()->getContext();
>LLVMContext::InlineAsmDiagHandlerTy OldHandler =
>  Ctx.getInlineAsmDiagnosticHandler();
>void *OldContext = Ctx.getInlineAsmDiagnosticContext();
> @@ -208,7 +195,7 @@
>
>EmitBackendOutput(Diags, CodeGenOpts, TargetOpts, LangOpts,
>  C.getTargetInfo().getDataLayoutString(),
> -TheModule.get(), Action, AsmOutStream);
> +getModule(), Action, AsmOutStream);
>
>Ctx.setInlineAsmDiagnosticHandler(OldHandler, OldContext);
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r257357 - [Sema] Issue a warning for integer overflow in struct initializer

[Joerg Sonnenberger via cfe-commits]

On Mon, Jan 11, 2016 at 05:22:01PM -, Akira Hatanaka via cfe-commits wrote:
> Author: ahatanak
> Date: Mon Jan 11 11:22:01 2016
> New Revision: 257357
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=257357&view=rev
> Log:
> [Sema] Issue a warning for integer overflow in struct initializer
> 
> Clang wasn't issuing a warning when compiling the following code:
> 
> struct s {
>   unsigned x;
> } s = {
>   .x = 4 * 1024 * 1024 * 1024
> };

Does this trigger in dead branches or not? Read: can you make sure it
doesn't and add an appropiate case to the test, please?

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

Re: [PATCH] D16047: [OpenCL] Add Sema checks for OpenCL 2.0

[Pekka Jääskeläinen via cfe-commits]

pekka.jaaskelainen added inline comments.


Comment at: lib/Sema/SemaDecl.cpp:5733
@@ +5732,3 @@
+
+#if 0
+

Is this intentionally included in the patch? 


Comment at: lib/Sema/SemaDecl.cpp:6759
@@ +6758,3 @@
+
+#if 0
+  // OpenCL v2.0 s6.9.b

Ditto. Better not commit disabled code in the repository.


Comment at: lib/Sema/SemaDecl.cpp:7305
@@ -7211,3 +7304,3 @@
   return PtrPtrKernelParam;
-return PointeeType.getAddressSpace() == 0 ? PrivatePtrKernelParam
-  : PtrKernelParam;
+//TODO?
+//return PointeeType.getAddressSpace() == 0 ? PrivatePtrKernelParam

Ditto.


Comment at: lib/Sema/SemaExpr.cpp:6295
@@ +6294,3 @@
+  // OpenCL v2.0 s6.12.5 -- To support these behaviors, additional
+  // restrictions28 in addition to the above feature restrictions are: Blocks
+  // cannot be used as expressions of the ternary selection operator (?:).

-28


Comment at: lib/Sema/SemaExpr.cpp:6298
@@ +6297,3 @@
+  if (getLangOpts().OpenCL && getLangOpts().OpenCLVersion >= 200) {
+if (checkBlockType(*this, LHS.get()) | checkBlockType(*this, RHS.get()))
+  return QualType();

||


http://reviews.llvm.org/D16047



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

Re: [PATCH] D12901: [Static Analyzer] Assertion "System is over constrained" after truncating 64 bits integers to 32 bits. (PR25078)

[Anna Zaks via cfe-commits]

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

Sounds good. Please, split this into 2 patches (each fixing the separate 
problem) and commit.

Thanks!
Anna.


http://reviews.llvm.org/D12901



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

Re: [PATCH] D15914: [OpenCL] Pipe builtin functions

[Pekka Jääskeläinen via cfe-commits]

pekka.jaaskelainen added inline comments.


Comment at: lib/CodeGen/CGBuiltin.cpp:2033
@@ +2032,3 @@
+  *Arg1 = EmitScalarExpr(E->getArg(1));
+llvm::Type *ReservedIDTy = ConvertType(getContext().OCLReserveIDTy);
+

Anastasia wrote:
> Why do we need to modify user defined functions? Only builtins will have this 
> extra parameter.
> 
> I think packet size would be useful for builtin implementation to know the 
> number of bytes to be read/written. I don't see how to implement it correctly 
> otherwise. As mentioned earlier relying on the metadata is not a conventional 
> compilation approach and should be avoided if possible.
The pipe struct can have the packet size in its header before the actual ring 
buffer or whatever, which can be used as a fallback unless the compiler can 
optimize it to a larger access. Correct implementation thus doesn't require a 
"hidden parameter". Adding it as a compile time hidden constant argument should 
help the optimizers, that's of course true, but I don't think it's strictly 
required.

If you think having a special behavior for the built-ins calls isn't 
problematic, then fine, I'm not having so strong opinion on this.


http://reviews.llvm.org/D15914



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

Re: [PATCH] D15914: [OpenCL] Pipe builtin functions

[Anastasia Stulova via cfe-commits]

Anastasia added inline comments.


Comment at: lib/CodeGen/CGBuiltin.cpp:2033
@@ +2032,3 @@
+  *Arg1 = EmitScalarExpr(E->getArg(1));
+llvm::Type *ReservedIDTy = ConvertType(getContext().OCLReserveIDTy);
+

pekka.jaaskelainen wrote:
> Anastasia wrote:
> > Why do we need to modify user defined functions? Only builtins will have 
> > this extra parameter.
> > 
> > I think packet size would be useful for builtin implementation to know the 
> > number of bytes to be read/written. I don't see how to implement it 
> > correctly otherwise. As mentioned earlier relying on the metadata is not a 
> > conventional compilation approach and should be avoided if possible.
> The pipe struct can have the packet size in its header before the actual ring 
> buffer or whatever, which can be used as a fallback unless the compiler can 
> optimize it to a larger access. Correct implementation thus doesn't require a 
> "hidden parameter". Adding it as a compile time hidden constant argument 
> should help the optimizers, that's of course true, but I don't think it's 
> strictly required.
> 
> If you think having a special behavior for the built-ins calls isn't 
> problematic, then fine, I'm not having so strong opinion on this.
So where would this size be initialized/set? Note that host code might have 
different type sizes.

I am thinking that having a size parameter makes the generated function more 
generic since we lose information about the type here and recovering it might 
involve extra steps. I am currently not clear about how to do that.


http://reviews.llvm.org/D15914



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

Re: [PATCH] D15914: [OpenCL] Pipe builtin functions

[Pekka Jääskeläinen via cfe-commits]

pekka.jaaskelainen added inline comments.


Comment at: lib/CodeGen/CGBuiltin.cpp:2033
@@ +2032,3 @@
+  *Arg1 = EmitScalarExpr(E->getArg(1));
+llvm::Type *ReservedIDTy = ConvertType(getContext().OCLReserveIDTy);
+

Anastasia wrote:
> pekka.jaaskelainen wrote:
> > Anastasia wrote:
> > > Why do we need to modify user defined functions? Only builtins will have 
> > > this extra parameter.
> > > 
> > > I think packet size would be useful for builtin implementation to know 
> > > the number of bytes to be read/written. I don't see how to implement it 
> > > correctly otherwise. As mentioned earlier relying on the metadata is not 
> > > a conventional compilation approach and should be avoided if possible.
> > The pipe struct can have the packet size in its header before the actual 
> > ring buffer or whatever, which can be used as a fallback unless the 
> > compiler can optimize it to a larger access. Correct implementation thus 
> > doesn't require a "hidden parameter". Adding it as a compile time hidden 
> > constant argument should help the optimizers, that's of course true, but I 
> > don't think it's strictly required.
> > 
> > If you think having a special behavior for the built-ins calls isn't 
> > problematic, then fine, I'm not having so strong opinion on this.
> So where would this size be initialized/set? Note that host code might have 
> different type sizes.
> 
> I am thinking that having a size parameter makes the generated function more 
> generic since we lose information about the type here and recovering it might 
> involve extra steps. I am currently not clear about how to do that.
https://www.khronos.org/registry/cl/sdk/2.0/docs/man/xhtml/clCreatePipe.html 
sets the packet size in creation. We have a prototype implementation working 
using that. But that generic software version uses a byte loop which is 
painfully slow, which of course should be optimized to wide loads/stores 
whenever possible.

If we want to optimize it, it means inlining the builtin call and then deriving 
the size for the loop at the compile time after which it can be vectorized 
(assuming the packet fifo is aligned). Where this size constant comes from is 
the question under discussion -- you propose having it as an additional hidden 
parameter to the calls and I propose a metadata. Yours optimizes automatically 
but requires special casing (when calling a builtin, not user function, add 
this magic parameter), mine requires a new optimization that takes the MD in 
account and converts it to a constant after inlining the built-in.

I'm open for both as I do not really know how much trouble the special casing 
will bring and I appreciate that optimizations might just work, but in my 
understanding the size info is strictly not required, but very useful for 
optimization.


http://reviews.llvm.org/D15914



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

Re: [PATCH] D15363: [UBSan] Implement runtime suppressions (PR25066).

[Reid Kleckner via cfe-commits]

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

In http://reviews.llvm.org/D15363#321941, @samsonov wrote:

> Interesting. Do we run test/asan/TestCases/suppressions-interceptor.cc on
>  Windows? Seems so: I don't see an XFAIL there, and it also uses
>  suppressions, but with single quote, followed by double quote:
>
> %env_asan_opts=suppressions='"%t.supp"'
>
> Why does it work there?


We need two levels of quoting: one for the shell and one that makes it all the 
way through to ASan. The single quotes get taken out by ShLexer or bash 
depending on which is in use, and the inner double quotes escape the colons 
inside the Windows paths.

This seemed like the most logically consistent thing to me, even if it's ugly. 
Any better ideas for smuggling colons through ASan options?


Repository:
  rL LLVM

http://reviews.llvm.org/D15363



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

Re: [PATCH] D16062: [analyzer] Rename kind-enumeration values of SVal, SymExpr, MemRegion classes, for consistency.

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

> MemSpaceRegion is now an abstract base


What prevents it from being instantiated?


http://reviews.llvm.org/D16062



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

Re: [PATCH] D16041: Change vfs::FileSystem to be managed with std::shared_ptr

[Owen Anderson via cfe-commits]

> On Jan 11, 2016, at 8:25 AM, David Blaikie  wrote:
> 
> 
> 
> On Sun, Jan 10, 2016 at 11:42 PM, Owen Anderson via cfe-commits 
>  wrote:
> resistor created this revision.
> resistor added reviewers: chandlerc, bkramer, klimek.
> resistor added a subscriber: cfe-commits.
> resistor set the repository for this revision to rL LLVM.
> Herald added a subscriber: klimek.
> 
> Managing it with IntrusiveRefCntPtr caused the virtual destructor not to be 
> called properly.
> 
> Regardless of the broader discussion on this patch, I'm confused by why this 
> ^ would be the case. What is it that IntrusiveRefCntPtr is doing that's 
> causing problems with destruction? (& I'm all for changing this to 
> non-intrusive smart pointers over intrusive ones anyway, but I'd still like 
> to understand the extra motivation here)
>  

ThreadSafeRefCountedBase, which classes must inherit from in order to use 
IntrusiveRefCntPtr, does not handle virtual destructors properly.  For the 
non-thread safe version, there is RefCountBaseVPTR which solves this problem.  
An alternative solution would have been to add a corresponding 
ThreadSafeRefCountedBaseVPTR, but IMO at that point one might as well use 
shared_ptr since it’s 90% of the way there.

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

[libcxx] r257368 - Preemptively disable unsigned integer sanitization in 32 and 64 bit versions of __murmur2_or_cityhash. This lets people use the unsigned integer overflow checker in UBSAN w/o gettin

[Marshall Clow via cfe-commits]

Author: marshall
Date: Mon Jan 11 13:27:10 2016
New Revision: 257368

URL: http://llvm.org/viewvc/llvm-project?rev=257368&view=rev
Log:
Preemptively disable unsigned integer sanitization in 32 and 64 bit versions of 
__murmur2_or_cityhash. This lets people use the unsigned integer overflow 
checker in UBSAN w/o getting hits from libc++'s hash code (where the unsigned 
integer overflow is legal and deliberate)> Patch by @danielaustin. Reviewed as: 
http://reviews.llvm.org/D15973

Modified:
libcxx/trunk/include/__config
libcxx/trunk/include/memory

Modified: libcxx/trunk/include/__config
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=257368&r1=257367&r2=257368&view=diff
==
--- libcxx/trunk/include/__config (original)
+++ libcxx/trunk/include/__config Mon Jan 11 13:27:10 2016
@@ -597,6 +597,11 @@ namespace std {
 
 #define _LIBCPP_HAS_NO_ASAN
 
+// Allow for build-time disabling of unsigned integer sanitization
+#ifndef _LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK
+#define _LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK 
__attribute((no_sanitize("unsigned integer")))
+#endif 
+
 #endif // __clang__ || __GNUC__ || _MSC_VER || __IBMCPP__
 
 #ifndef _LIBCPP_HAS_NO_NOEXCEPT
@@ -826,6 +831,10 @@ extern "C" void __sanitizer_annotate_con
 #define _LIBCPP_HAS_NO_ATOMIC_HEADER
 #endif
 
+#ifndef _LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK
+#define _LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK
+#endif 
+
 #endif // __cplusplus
 
 #endif // _LIBCPP_CONFIG

Modified: libcxx/trunk/include/memory
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/memory?rev=257368&r1=257367&r2=257368&view=diff
==
--- libcxx/trunk/include/memory (original)
+++ libcxx/trunk/include/memory Mon Jan 11 13:27:10 2016
@@ -3194,7 +3194,7 @@ struct __murmur2_or_cityhash<_Size, 32>
 // murmur2
 template 
 _Size
-__murmur2_or_cityhash<_Size, 32>::operator()(const void* __key, _Size __len)
+__murmur2_or_cityhash<_Size, 32>::operator()(const void* __key, _Size __len) 
_LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK 
 {
 const _Size __m = 0x5bd1e995;
 const _Size __r = 24;
@@ -3344,7 +3344,7 @@ struct __murmur2_or_cityhash<_Size, 64>
 // cityhash64
 template 
 _Size
-__murmur2_or_cityhash<_Size, 64>::operator()(const void* __key, _Size __len)
+__murmur2_or_cityhash<_Size, 64>::operator()(const void* __key, _Size __len) 
_LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK 
 {
   const char* __s = static_cast(__key);
   if (__len <= 32) {


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

Re: r257357 - [Sema] Issue a warning for integer overflow in struct initializer

[Akira Hatanaka via cfe-commits]

It triggers in dead branches and so does an initialization of an integer 
variable declared in a function.

I’ll look into fixing it.

> On Jan 11, 2016, at 9:54 AM, Joerg Sonnenberger via cfe-commits 
>  wrote:
> 
> On Mon, Jan 11, 2016 at 05:22:01PM -, Akira Hatanaka via cfe-commits 
> wrote:
>> Author: ahatanak
>> Date: Mon Jan 11 11:22:01 2016
>> New Revision: 257357
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=257357&view=rev
>> Log:
>> [Sema] Issue a warning for integer overflow in struct initializer
>> 
>> Clang wasn't issuing a warning when compiling the following code:
>> 
>> struct s {
>>  unsigned x;
>> } s = {
>>  .x = 4 * 1024 * 1024 * 1024
>> };
> 
> Does this trigger in dead branches or not? Read: can you make sure it
> doesn't and add an appropiate case to the test, please?
> 
> Joerg
> ___
> 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] D15858: Warn undeclared identifiers in CUDA kernel calls

[Jason Henline via cfe-commits]

jhen updated this revision to Diff 44538.
jhen added a comment.

- Handle unexpanded parameter packs

  The changes for instantiation dependence also fix a bug with unexpanded 
parameter packs, so add a unit test for unexpanded parameter packs as well.


http://reviews.llvm.org/D15858

Files:
  include/clang/AST/Expr.h
  include/clang/AST/ExprCXX.h
  lib/AST/Expr.cpp
  test/SemaCUDA/cxx11-kernel-call.cu
  test/SemaCUDA/kernel-call.cu

Index: test/SemaCUDA/kernel-call.cu
===
--- test/SemaCUDA/kernel-call.cu
+++ test/SemaCUDA/kernel-call.cu
@@ -23,4 +23,6 @@
 
   int (*fp)(int) = h2;
   fp<<<1, 1>>>(42); // expected-error {{must have void return type}}
+
+  g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}}
 }
Index: test/SemaCUDA/cxx11-kernel-call.cu
===
--- /dev/null
+++ test/SemaCUDA/cxx11-kernel-call.cu
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+#include "Inputs/cuda.h"
+
+__global__ void k1() {}
+
+template void k1Wrapper() {
+  void (*f)() = [] { k1<<>>(); }; // expected-error {{initializer contains unexpanded parameter pack 'Dimensions'}}
+}
Index: lib/AST/Expr.cpp
===
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -1138,38 +1138,38 @@
 // Postfix Operators.
 //===--===//
 
-CallExpr::CallExpr(const ASTContext& C, StmtClass SC, Expr *fn,
-   unsigned NumPreArgs, ArrayRef args, QualType t,
+CallExpr::CallExpr(const ASTContext &C, StmtClass SC, Expr *fn,
+   ArrayRef preargs, ArrayRef args, QualType t,
ExprValueKind VK, SourceLocation rparenloc)
-  : Expr(SC, t, VK, OK_Ordinary,
- fn->isTypeDependent(),
- fn->isValueDependent(),
- fn->isInstantiationDependent(),
- fn->containsUnexpandedParameterPack()),
-NumArgs(args.size()) {
-
-  SubExprs = new (C) Stmt*[args.size()+PREARGS_START+NumPreArgs];
+: Expr(SC, t, VK, OK_Ordinary, fn->isTypeDependent(),
+   fn->isValueDependent(), fn->isInstantiationDependent(),
+   fn->containsUnexpandedParameterPack()),
+  NumArgs(args.size()) {
+
+  unsigned NumPreArgs = preargs.size();
+  SubExprs = new (C) Stmt *[args.size()+PREARGS_START+NumPreArgs];
   SubExprs[FN] = fn;
+  for (unsigned i = 0; i != NumPreArgs; ++i) {
+updateDependenciesFromArg(preargs[i]);
+SubExprs[i+PREARGS_START] = preargs[i];
+  }
   for (unsigned i = 0; i != args.size(); ++i) {
-if (args[i]->isTypeDependent())
-  ExprBits.TypeDependent = true;
-if (args[i]->isValueDependent())
-  ExprBits.ValueDependent = true;
-if (args[i]->isInstantiationDependent())
-  ExprBits.InstantiationDependent = true;
-if (args[i]->containsUnexpandedParameterPack())
-  ExprBits.ContainsUnexpandedParameterPack = true;
-
+updateDependenciesFromArg(args[i]);
 SubExprs[i+PREARGS_START+NumPreArgs] = args[i];
   }
 
   CallExprBits.NumPreArgs = NumPreArgs;
   RParenLoc = rparenloc;
 }
 
+CallExpr::CallExpr(const ASTContext &C, StmtClass SC, Expr *fn,
+   ArrayRef args, QualType t, ExprValueKind VK,
+   SourceLocation rparenloc)
+: CallExpr(C, SC, fn, ArrayRef(), args, t, VK, rparenloc) {}
+
 CallExpr::CallExpr(const ASTContext &C, Expr *fn, ArrayRef args,
QualType t, ExprValueKind VK, SourceLocation rparenloc)
-: CallExpr(C, CallExprClass, fn, /*NumPreArgs=*/0, args, t, VK, rparenloc) {
+: CallExpr(C, CallExprClass, fn, ArrayRef(), args, t, VK, rparenloc) {
 }
 
 CallExpr::CallExpr(const ASTContext &C, StmtClass SC, EmptyShell Empty)
@@ -1179,10 +1179,21 @@
EmptyShell Empty)
   : Expr(SC, Empty), SubExprs(nullptr), NumArgs(0) {
   // FIXME: Why do we allocate this?
-  SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs];
+  SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs]();
   CallExprBits.NumPreArgs = NumPreArgs;
 }
 
+void CallExpr::updateDependenciesFromArg(Expr *Arg) {
+  if (Arg->isTypeDependent())
+ExprBits.TypeDependent = true;
+  if (Arg->isValueDependent())
+ExprBits.ValueDependent = true;
+  if (Arg->isInstantiationDependent())
+ExprBits.InstantiationDependent = true;
+  if (Arg->containsUnexpandedParameterPack())
+ExprBits.ContainsUnexpandedParameterPack = true;
+}
+
 Decl *CallExpr::getCalleeDecl() {
   Expr *CEE = getCallee()->IgnoreParenImpCasts();
 
Index: include/clang/AST/ExprCXX.h
===
--- include/clang/AST/ExprCXX.h
+++ include/clang/AST/ExprCXX.h
@@ -66,8 +66,7 @@
   CXXOperatorCallExpr(ASTContext& C, OverloadedOperatorKind Op, Expr *fn,
   ArrayRef args, QualType t, ExprValueKind VK,
   SourceLocation operatorloc, bool fpContractable)

Re: [PATCH] D15921: [analyzer] Utility to match function calls.

[Anna Zaks via cfe-commits]

zaks.anna added inline comments.


Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:312
@@ +311,3 @@
+  /// calls.
+  bool isCalled(const CallEvent &Call, const CallDescription &CD);
+

The API is a bit awkward. Maybe it would be better if we make this a member of 
CallEvent as you've suggested initially?


http://reviews.llvm.org/D15921



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

r257383 - Fix -Wmicrosoft-enum-value warning

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Mon Jan 11 14:55:16 2016
New Revision: 257383

URL: http://llvm.org/viewvc/llvm-project?rev=257383&view=rev
Log:
Fix -Wmicrosoft-enum-value warning

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h?rev=257383&r1=257382&r2=257383&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h Mon Jan 11 14:55:16 2016
@@ -335,7 +335,7 @@ private:
 public:
   /// \brief Kind of a given entry. Currently, only target regions are
   /// supported.
-  enum OffloadingEntryInfoKinds {
+  enum OffloadingEntryInfoKinds : unsigned {
 // Entry is a target region.
 OFFLOAD_ENTRY_INFO_TARGET_REGION = 0,
 // Invalid entry info.


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

Re: [PATCH] D16062: [analyzer] Rename kind-enumeration values of SVal, SymExpr, MemRegion classes, for consistency.

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

Looks good to me. Thanks for making this more consistent!



Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:102
@@ -103,1 +101,3 @@
+BEGIN_TYPED_REGIONS,
+FunctionTextRegionKind = BEGIN_TYPED_REGIONS,
 BlockTextRegionKind,

While we're at it, what do you think about renaming FunctionTextRegion and 
BlockTextRegion to FunctionCodeRegion and BlockCodeRegion, respectively? The 
term 'code' is less jargony than 'text' and we already refer to BlockTextRegion 
as a 'code region' in BlockDataRegion.


Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:195
@@ -194,2 +194,3 @@
 protected:
   friend class MemRegionManager;
+

Following up on Anna's question: does MemSpaceRegion need to be a friend of 
MemRegionManager now?


http://reviews.llvm.org/D16062



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

Re: r257357 - [Sema] Issue a warning for integer overflow in struct initializer

[Joerg Sonnenberger via cfe-commits]

On Mon, Jan 11, 2016 at 12:28:36PM -0800, Akira Hatanaka via cfe-commits wrote:
> It triggers in dead branches and so does an initialization of an integer 
> variable declared in a function.

I'm more interested in the case of global initialisers here. There is a
long standing history of not checking properly for dead branches, which
makes some code noisy for no good reason.

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

[PATCH] D16078: Add an Action* member to InputInfo.

[Justin Lebar via cfe-commits]

jlebar created this revision.
jlebar added a reviewer: echristo.
jlebar added subscribers: cfe-commits, tra, jhen.
Herald added subscribers: dschuff, jfb.

The CUDA toolchain needs to know which Actions created which InputInfos,
because it needs to attach GPU archs to the various InputInfos.

http://reviews.llvm.org/D16078

Files:
  lib/Driver/Driver.cpp
  lib/Driver/InputInfo.h
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2980,7 +2980,7 @@
   ExtractArgs.push_back(OutFile);
 
   const char *Exec = Args.MakeArgString(TC.GetProgramPath("objcopy"));
-  InputInfo II(Output.getFilename(), types::TY_Object, Output.getFilename());
+  InputInfo II(types::TY_Object, Output.getFilename(), Output.getFilename());
 
   // First extract the dwo sections.
   C.addCommand(llvm::make_unique(JA, T, Exec, ExtractArgs, II));
@@ -8987,7 +8987,7 @@
const char *LinkingOutput) const {
   const toolchains::NaClToolChain &ToolChain =
   static_cast(getToolChain());
-  InputInfo NaClMacros(ToolChain.GetNaClArmMacrosPath(), types::TY_PP_Asm,
+  InputInfo NaClMacros(types::TY_PP_Asm, ToolChain.GetNaClArmMacrosPath(),
"nacl-arm-macros.s");
   InputInfoList NewInputs;
   NewInputs.push_back(NaClMacros);
Index: lib/Driver/InputInfo.h
===
--- lib/Driver/InputInfo.h
+++ lib/Driver/InputInfo.h
@@ -10,6 +10,7 @@
 #ifndef LLVM_CLANG_LIB_DRIVER_INPUTINFO_H
 #define LLVM_CLANG_LIB_DRIVER_INPUTINFO_H
 
+#include "clang/Driver/Action.h"
 #include "clang/Driver/Types.h"
 #include "llvm/Option/Arg.h"
 #include 
@@ -38,29 +39,47 @@
 const llvm::opt::Arg *InputArg;
   } Data;
   Class Kind;
+  const Action* Act;
   types::ID Type;
   const char *BaseInput;
 
-public:
-  InputInfo() {}
-  InputInfo(types::ID _Type, const char *_BaseInput)
-: Kind(Nothing), Type(_Type), BaseInput(_BaseInput) {
+  static types::ID GetActionType(const Action *A) {
+return A != nullptr ? A->getType() : types::TY_Nothing;
   }
-  InputInfo(const char *_Filename, types::ID _Type, const char *_BaseInput)
-: Kind(Filename), Type(_Type), BaseInput(_BaseInput) {
+
+public:
+  InputInfo() : InputInfo(nullptr, nullptr) {}
+  InputInfo(const Action *A, const char *_BaseInput)
+  : Kind(Nothing), Act(A), Type(GetActionType(A)), BaseInput(_BaseInput) {}
+
+  InputInfo(types::ID _Type, const char *_Filename, const char *_BaseInput)
+  : Kind(Filename), Act(nullptr), Type(_Type), BaseInput(_BaseInput) {
 Data.Filename = _Filename;
   }
-  InputInfo(const llvm::opt::Arg *_InputArg, types::ID _Type,
+  InputInfo(const Action *A, const char *_Filename, const char *_BaseInput)
+  : Kind(Filename), Act(A), Type(GetActionType(A)), BaseInput(_BaseInput) {
+Data.Filename = _Filename;
+  }
+
+  InputInfo(types::ID _Type, const llvm::opt::Arg *_InputArg,
 const char *_BaseInput)
-  : Kind(InputArg), Type(_Type), BaseInput(_BaseInput) {
+  : Kind(InputArg), Act(nullptr), Type(_Type), BaseInput(_BaseInput) {
+Data.InputArg = _InputArg;
+  }
+  InputInfo(const Action *A, const llvm::opt::Arg *_InputArg,
+const char *_BaseInput)
+  : Kind(InputArg), Act(A), Type(GetActionType(A)), BaseInput(_BaseInput) {
 Data.InputArg = _InputArg;
   }
 
   bool isNothing() const { return Kind == Nothing; }
   bool isFilename() const { return Kind == Filename; }
   bool isInputArg() const { return Kind == InputArg; }
   types::ID getType() const { return Type; }
   const char *getBaseInput() const { return BaseInput; }
+  /// The action for which this InputInfo was created.  May be null.
+  const Action *getAction() const { return Act; }
+  void setAction(const Action *A) { Act = A; }
 
   const char *getFilename() const {
 assert(isFilename() && "Invalid accessor.");
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1822,9 +1822,9 @@
 Input.claim();
 if (Input.getOption().matches(options::OPT_INPUT)) {
   const char *Name = Input.getValue();
-  return InputInfo(Name, A->getType(), Name);
+  return InputInfo(A, Name, /* BaseInput = */ Name);
 }
-return InputInfo(&Input, A->getType(), "");
+return InputInfo(A, &Input, /* BaseInput = */ "");
   }
 
   if (const BindArchAction *BAA = dyn_cast(A)) {
@@ -1899,11 +1899,11 @@
   // Determine the place to write output to, if any.
   InputInfo Result;
   if (JA->getType() == types::TY_Nothing)
-Result = InputInfo(A->getType(), BaseInput);
+Result = InputInfo(A, BaseInput);
   else
-Result = InputInfo(GetNamedOutputPath(C, *JA, BaseInput, BoundArch,
-  AtTopLevel, MultipleArchs),
-   A->getType(), BaseInput);
+Result = Inpu

[PATCH] D16080: [CUDA] Add tests for compiling CUDA files with -E.

[Justin Lebar via cfe-commits]

jlebar created this revision.
jlebar added a reviewer: tra.
jlebar added subscribers: jhen, cfe-commits.

http://reviews.llvm.org/D16080

Files:
  test/Driver/cuda-preprocess.cu

Index: test/Driver/cuda-preprocess.cu
===
--- /dev/null
+++ test/Driver/cuda-preprocess.cu
@@ -0,0 +1,29 @@
+// Tests CUDA compilation with -E.
+
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+#ifndef __CUDA_ARCH__
+clang_unittest_no_arch
+#else
+clang_unittest_cuda_arch __CUDA_ARCH__
+#endif
+
+// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix NOARCH %s
+// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 
--cuda-host-only %s 2>&1 \
+// RUN:   | FileCheck -check-prefix NOARCH %s
+// NOARCH: clang_unittest_no_arch
+
+// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 
--cuda-device-only %s 2>&1 \
+// RUN:   | FileCheck -check-prefix SM20 %s
+// SM20: clang_unittest_cuda_arch 200
+
+// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_30 
--cuda-device-only %s 2>&1 \
+// RUN:   | FileCheck -check-prefix SM30 %s
+// SM30: clang_unittest_cuda_arch 300
+
+// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 
--cuda-gpu-arch=sm_30 \
+// RUN:   --cuda-device-only %s 2>&1 \
+// RUN:   | FileCheck -check-prefix SM20 -check-prefix SM30 %s


Index: test/Driver/cuda-preprocess.cu
===
--- /dev/null
+++ test/Driver/cuda-preprocess.cu
@@ -0,0 +1,29 @@
+// Tests CUDA compilation with -E.
+
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+#ifndef __CUDA_ARCH__
+clang_unittest_no_arch
+#else
+clang_unittest_cuda_arch __CUDA_ARCH__
+#endif
+
+// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix NOARCH %s
+// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 --cuda-host-only %s 2>&1 \
+// RUN:   | FileCheck -check-prefix NOARCH %s
+// NOARCH: clang_unittest_no_arch
+
+// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 --cuda-device-only %s 2>&1 \
+// RUN:   | FileCheck -check-prefix SM20 %s
+// SM20: clang_unittest_cuda_arch 200
+
+// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_30 --cuda-device-only %s 2>&1 \
+// RUN:   | FileCheck -check-prefix SM30 %s
+// SM30: clang_unittest_cuda_arch 300
+
+// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 --cuda-gpu-arch=sm_30 \
+// RUN:   --cuda-device-only %s 2>&1 \
+// RUN:   | FileCheck -check-prefix SM20 -check-prefix SM30 %s
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D16081: [CUDA] Add test for compiling CUDA code with -S.

[Justin Lebar via cfe-commits]

jlebar created this revision.
jlebar added a reviewer: tra.
jlebar added subscribers: jhen, cfe-commits.

http://reviews.llvm.org/D16081

Files:
  test/Driver/cuda-options.cu
  test/Driver/cuda-output-asm.cu

Index: test/Driver/cuda-output-asm.cu
===
--- /dev/null
+++ test/Driver/cuda-output-asm.cu
@@ -0,0 +1,29 @@
+// Tests CUDA compilation with -S.
+
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix HOST -check-prefix SM20 %s
+// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-host-only -o foo.s %s 
2>&1 \
+// RUN:   | FileCheck -check-prefix HOST %s
+// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 \
+// RUN:   --cuda-device-only -o foo.s %s 2>&1 \
+// RUN:   | FileCheck -check-prefix SM20 %s
+// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 \
+// RUN:   --cuda-gpu-arch=sm_30 --cuda-device-only %s 2>&1 \
+// RUN:   | FileCheck -check-prefix SM20 -check-prefix SM30 %s
+
+// HOST-DAG: "-cc1" "-triple" "x86_64--linux-gnu"
+// SM20-DAG: "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// SM20-same: "-target-cpu" "sm_20"
+// SM30-DAG: "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// SM30-same: "-target-cpu" "sm_30"
+
+// RUN: %clang -### -S -target x86_64-linux-gnu -o foo.s %s 2>&1 \
+// RUN:   | FileCheck -check-prefix MULTIPLE-OUTPUT-FILES %s
+// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-device-only \
+// RUN:   --cuda-gpu-arch=sm_20 --cuda-gpu-arch=sm_30 -o foo.s %s 2>&1 \
+// RUN:   | FileCheck -check-prefix MULTIPLE-OUTPUT-FILES %s
+// MULTIPLE-OUTPUT-FILES: error: cannot specify -o when generating multiple 
output files
Index: test/Driver/cuda-options.cu
===
--- test/Driver/cuda-options.cu
+++ test/Driver/cuda-options.cu
@@ -39,13 +39,6 @@
 // RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
 // RUN:-check-prefix NOHOST -check-prefix NOLINK %s
 
-// Verify that with -S we compile host and device sides to assembly and
-// incorporate device code into the host side.
-// RUN: %clang -### -target x86_64-linux-gnu -S -c %s 2>&1 \
-// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
-// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
-// RUN:-check-prefix NOLINK %s
-
 // Verify that --cuda-gpu-arch option passes the correct GPU archtecture to
 // device compilation.
 // RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=sm_35 -c %s 2>&1 \


Index: test/Driver/cuda-output-asm.cu
===
--- /dev/null
+++ test/Driver/cuda-output-asm.cu
@@ -0,0 +1,29 @@
+// Tests CUDA compilation with -S.
+
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix HOST -check-prefix SM20 %s
+// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-host-only -o foo.s %s 2>&1 \
+// RUN:   | FileCheck -check-prefix HOST %s
+// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 \
+// RUN:   --cuda-device-only -o foo.s %s 2>&1 \
+// RUN:   | FileCheck -check-prefix SM20 %s
+// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 \
+// RUN:   --cuda-gpu-arch=sm_30 --cuda-device-only %s 2>&1 \
+// RUN:   | FileCheck -check-prefix SM20 -check-prefix SM30 %s
+
+// HOST-DAG: "-cc1" "-triple" "x86_64--linux-gnu"
+// SM20-DAG: "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// SM20-same: "-target-cpu" "sm_20"
+// SM30-DAG: "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// SM30-same: "-target-cpu" "sm_30"
+
+// RUN: %clang -### -S -target x86_64-linux-gnu -o foo.s %s 2>&1 \
+// RUN:   | FileCheck -check-prefix MULTIPLE-OUTPUT-FILES %s
+// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-device-only \
+// RUN:   --cuda-gpu-arch=sm_20 --cuda-gpu-arch=sm_30 -o foo.s %s 2>&1 \
+// RUN:   | FileCheck -check-prefix MULTIPLE-OUTPUT-FILES %s
+// MULTIPLE-OUTPUT-FILES: error: cannot specify -o when generating multiple output files
Index: test/Driver/cuda-options.cu
===
--- test/Driver/cuda-options.cu
+++ test/Driver/cuda-options.cu
@@ -39,13 +39,6 @@
 // RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
 // RUN:-check-prefix NOHOST -check-prefix NOLINK %s
 
-// Verify that with -S we compile host and device sides to assembly and
-// incorporate device code into the host side.
-// RUN: %clang -### -target x86_64-linux-gnu -S -c %s 2>&1 \
-// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
-// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
-// RUN:-check-prefix NOLINK %s
-
 // Verify that --cuda-gpu-a

[PATCH] D16079: [CUDA] Reject values for --cuda-gpu-arch that are not of the form /sm_\d+/.

[Justin Lebar via cfe-commits]

jlebar created this revision.
jlebar added a reviewer: tra.
jlebar added subscribers: echristo, jhen, cfe-commits.

http://reviews.llvm.org/D16079

Files:
  include/clang/Driver/Action.h
  lib/Driver/Action.cpp
  lib/Driver/Driver.cpp
  test/Driver/cuda-bad-arch.cu

Index: test/Driver/cuda-bad-arch.cu
===
--- /dev/null
+++ test/Driver/cuda-bad-arch.cu
@@ -0,0 +1,34 @@
+// Checks errors generated by passing a bad value for --cuda-gpu-arch.
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=compute_20 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix BAD %s
+// RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=foo_20 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix BAD %s
+// RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=sm20 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix BAD %s
+// RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=sm_abc -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix BAD %s
+// RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=sm_20a -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix BAD %s
+// RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=sm_a20 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix BAD %s
+// RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=ssm_20 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix BAD %s
+// RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=sm_ -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix BAD %s
+
+// BAD: error: Unsupported CUDA gpu architecture
+
+// RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=sm_2 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix OK %s
+// RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix OK %s
+// RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=sm_999 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix OK %s
+// RUN: %clang -### -target x86_64-linux-gnu -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix OK %s
+
+// OK-NOT: error: Unsupported CUDA gpu architecture
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1297,8 +1297,12 @@
 if (!A->getOption().matches(options::OPT_cuda_gpu_arch_EQ))
   continue;
 A->claim();
-if (GpuArchNames.insert(A->getValue()).second)
-  GpuArchList.push_back(A->getValue());
+
+const auto& Arch = A->getValue();
+if (!CudaDeviceAction::IsValidGpuArchName(Arch))
+  C.getDriver().Diag(clang::diag::err_drv_cuda_bad_gpu_arch) << Arch;
+else if (GpuArchNames.insert(Arch).second)
+  GpuArchList.push_back(Arch);
   }
 
   // Default to sm_20 which is the lowest common denominator for supported GPUs.
Index: lib/Driver/Action.cpp
===
--- lib/Driver/Action.cpp
+++ lib/Driver/Action.cpp
@@ -9,6 +9,7 @@
 
 #include "clang/Driver/Action.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Regex.h"
 #include 
 using namespace clang::driver;
 using namespace llvm::opt;
@@ -54,7 +55,14 @@
 CudaDeviceAction::CudaDeviceAction(Action *Input, const char *ArchName,
bool AtTopLevel)
 : Action(CudaDeviceClass, Input), GpuArchName(ArchName),
-  AtTopLevel(AtTopLevel) {}
+  AtTopLevel(AtTopLevel) {
+  assert(IsValidGpuArchName(GpuArchName));
+}
+
+bool CudaDeviceAction::IsValidGpuArchName(llvm::StringRef ArchName) {
+  static llvm::Regex RE("^sm_[0-9]+$");
+  return RE.match(ArchName);
+}
 
 void CudaHostAction::anchor() {}
 
Index: include/clang/Driver/Action.h
===
--- include/clang/Driver/Action.h
+++ include/clang/Driver/Action.h
@@ -15,6 +15,9 @@
 #include "llvm/ADT/SmallVector.h"
 
 namespace llvm {
+
+class StringRef;
+
 namespace opt {
   class Arg;
 }
@@ -133,7 +136,7 @@
 
 class CudaDeviceAction : public Action {
   virtual void anchor();
-  /// GPU architecture to bind -- e.g 'sm_35'.
+  /// GPU architecture to bind.  Always of the form /sm_\d+/.
   const char *GpuArchName;
   /// True when action results are not consumed by the host action (e.g when
   /// -fsyntax-only or --cuda-device-only options are used).
@@ -145,6 +148,8 @@
   const char *getGpuArchName() const { return GpuArchName; }
   bool isAtTopLevel() const { return AtTopLevel; }
 
+  static bool IsValidGpuArchName(llvm::StringRef ArchName);
+
   static bool classof(const Action *A) {
 return A->getKind() == CudaDeviceClass;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D16082: [CUDA] Invoke ptxas and fatbinary during compilation.

[Justin Lebar via cfe-commits]

jlebar created this revision.
jlebar added reviewers: tra, echristo.
jlebar added subscribers: jhen, cfe-commits.

Previously we compiled CUDA device code to PTX assembly and embedded
that asm as text in our host binary.  Now we compile to PTX assembly and
then invoke ptxas to assemble the PTX into a cubin file.  We gather the
ptx and cubin files for each of our --cuda-gpu-archs and combine them
using fatbinary, and then embed that into the host binary.

Adds two new command-line flags, -Xcuda_ptxas and -Xcuda_fatbinary,
which pass args down to the external tools.

http://reviews.llvm.org/D16082

Files:
  include/clang/Driver/Action.h
  include/clang/Driver/Options.td
  include/clang/Driver/Types.def
  lib/CodeGen/CGCUDANV.cpp
  lib/Driver/Action.cpp
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp
  lib/Driver/Tools.h
  lib/Driver/Types.cpp
  test/Driver/Inputs/CUDA/usr/local/cuda/bin/.keep
  test/Driver/cuda-external-tools.cu
  test/Driver/cuda-options.cu

Index: test/Driver/cuda-options.cu
===
--- test/Driver/cuda-options.cu
+++ test/Driver/cuda-options.cu
@@ -54,7 +54,7 @@
 // RUN:-check-prefix DEVICE2 -check-prefix DEVICE-SM35 \
 // RUN:-check-prefix DEVICE2-SM30 -check-prefix HOST \
 // RUN:-check-prefix HOST-NOSAVE -check-prefix INCLUDES-DEVICE \
-// RUN:-check-prefix INCLUDES-DEVICE2 -check-prefix NOLINK %s
+// RUN:-check-prefix NOLINK %s
 
 // Verify that device-side results are passed to the correct tool when
 // -save-temps is used.
@@ -85,10 +85,16 @@
 // DEVICE-NOSAVE-SAME: "-aux-triple" "x86_64--linux-gnu"
 // DEVICE-SAME: "-fcuda-is-device"
 // DEVICE-SM35-SAME: "-target-cpu" "sm_35"
-// DEVICE-SAME: "-o" "[[GPUBINARY1:[^"]*]]"
+// DEVICE-SAME: "-o" "[[PTXFILE:[^"]*]]"
 // DEVICE-NOSAVE-SAME: "-x" "cuda"
 // DEVICE-SAVE-SAME: "-x" "ir"
 
+// Match the call to ptxas (which assembles PTX to SASS).
+// DEVICE:ptxas
+// DEVICE-SM35-DAG: "--gpu-name" "sm_35"
+// DEVICE-DAG: "--output-file" "[[CUBINFILE:[^"]*]]"
+// DEVICE-DAG: "[[PTXFILE]]"
+
 // Match another device-side compilation.
 // DEVICE2: "-cc1" "-triple" "nvptx64-nvidia-cuda"
 // DEVICE2-SAME: "-aux-triple" "x86_64--linux-gnu"
@@ -101,6 +107,11 @@
 // NODEVICE-NOT: "-cc1" "-triple" "nvptx64-nvidia-cuda"
 // NODEVICE-SAME-NOT: "-fcuda-is-device"
 
+// INCLUDES-DEVICE:fatbinary
+// INCLUDES-DEVICE-DAG: "--create" "[[FATBINARY:[^"]*]]"
+// INCLUDES-DEVICE-DAG: "--image=profile=sm_{{[0-9]+}},file=[[CUBINFILE]]"
+// INCLUDES-DEVICE-DAG: "--image=profile=compute_{{[0-9]+}},file=[[PTXFILE]]"
+
 // Match host-side preprocessor job with -save-temps.
 // HOST-SAVE: "-cc1" "-triple" "x86_64--linux-gnu"
 // HOST-SAVE-SAME: "-aux-triple" "nvptx64-nvidia-cuda"
@@ -114,8 +125,7 @@
 // HOST-SAME: "-o" "[[HOSTOUTPUT:[^"]*]]"
 // HOST-NOSAVE-SAME: "-x" "cuda"
 // HOST-SAVE-SAME: "-x" "cuda-cpp-output"
-// INCLUDES-DEVICE-SAME: "-fcuda-include-gpubinary" "[[GPUBINARY1]]"
-// INCLUDES-DEVICE2-SAME: "-fcuda-include-gpubinary" "[[GPUBINARY2]]"
+// INCLUDES-DEVICE-SAME: "-fcuda-include-gpubinary" "[[FATBINARY]]"
 
 // Match external assembler that uses compilation output.
 // HOST-AS: "-o" "{{.*}}.o" "[[HOSTOUTPUT]]"
Index: test/Driver/cuda-external-tools.cu
===
--- /dev/null
+++ test/Driver/cuda-external-tools.cu
@@ -0,0 +1,66 @@
+// Tests that ptxas and fatbinary are correctly during CUDA compilation.
+//
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// Regular compile with -O2.
+// RUN: %clang -### -target x86_64-linux-gnu -O2 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT2 %s
+
+// Regular compile without -O.  This should result in us passing -O0 to ptxas.
+// RUN: %clang -### -target x86_64-linux-gnu -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT0 %s
+
+// Regular compile targeting sm_35.
+// RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=sm_35 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM35 %s
+
+// 32-bit compile.
+// RUN: %clang -### -target x86_32-linux-gnu -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH32 -check-prefix SM20 %s
+
+// Check -Xcuda-ptxas and -Xcuda-fatbinary
+// RUN: %clang -### -target x86_64-linux-gnu -c -Xcuda-ptxas -foo1 \
+// RUN:   -Xcuda-fatbinary -bar1 -Xcuda-ptxas -foo2 -Xcuda-fatbinary -bar2 %s 2>&1 \
+// RUN: | FileCheck -check-prefix SM20 -check-prefix PTXAS-EXTRA \
+// RUN:   -check-prefix FATBINARY-EXTRA %s
+
+// Match clang job that produces PTX assembly.
+// CHECK: "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// SM20: "-target-cpu" "sm_20"
+// SM35: "-target-cpu" "sm_35"
+// SM20: "-o" "[[PTXFILE:[^"]*]]"
+// SM35: "-o" "[[PTXFILE:[^"]*]]"
+
+// Match the call to ptxas (which assembles PTX to SASS).
+// CHECK:ptxas
+// ARCH64: "-m64"
+// ARCH

Re: [PATCH] D15539: [libcxxabi] Reducing stack usage of test

[Ben Craig via cfe-commits]

bcraig added a comment.

ping


http://reviews.llvm.org/D15539



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

Re: [PATCH] D15539: [libcxxabi] Reducing stack usage of test

[Jonathan Roelofs via cfe-commits]

jroelofs added a comment.

FOAD: Ball's in @mclow.lists's court, not mine. I don't feel comfortable 
signing off on this.


http://reviews.llvm.org/D15539



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

r257392 - PR26087: Use nonstandard MSVC extension for VS2015 as well.

[James Y Knight via cfe-commits]

Author: jyknight
Date: Mon Jan 11 16:00:22 2016
New Revision: 257392

URL: http://llvm.org/viewvc/llvm-project?rev=257392&view=rev
Log:
PR26087: Use nonstandard MSVC extension for VS2015 as well.

In r256564, I had conditioned the workaround in has_getDecl to only be
used for MSVC before the 2015 release, believing that 2015 could handle
the standard code. But, that was incorrect.

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

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h?rev=257392&r1=257391&r2=257392&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h Mon Jan 11 
16:00:22 2016
@@ -560,10 +560,10 @@ bool matchesFirstInPointerRange(const Ma
 
 // Metafunction to determine if type T has a member called
 // getDecl.
-#if defined(_MSC_VER) && (_MSC_VER < 1900) && !defined(__clang__)
-// For old versions of MSVC, we use a weird nonstandard __if_exists
-// statement, since before MSVC2015, it was not standards-conformant
-// enough to compile the usual code below.
+#if defined(_MSC_VER) && !defined(__clang__)
+// For MSVC, we use a weird nonstandard __if_exists statement, as it
+// is not standards-conformant enough to properly compile the standard
+// code below. (At least up through MSVC 2015 require this workaround)
 template  struct has_getDecl {
   __if_exists(T::getDecl) {
 enum { value = 1 };


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

[PATCH] D16087: Add some overview doxygen comments to lib/Format.

[Nico Weber via cfe-commits]

thakis created this revision.
thakis added a reviewer: djasper.
thakis added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

It always takes me a while to understand how clang-format's pieces fit together 
-- hopefully this will save me some time the next time I need it.

http://reviews.llvm.org/D16087

Files:
  lib/Format/ContinuationIndenter.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/TokenAnnotator.h
  lib/Format/UnwrappedLineFormatter.h
  lib/Format/UnwrappedLineParser.h

Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -58,6 +58,8 @@
 
 class FormatTokenSource;
 
+/// \brief Takes a sequence of tokens, and splits it into chunks of tokens
+/// that would be a single line each if there was no column limit.
 class UnwrappedLineParser {
 public:
   UnwrappedLineParser(const FormatStyle &Style,
Index: lib/Format/UnwrappedLineFormatter.h
===
--- lib/Format/UnwrappedLineFormatter.h
+++ lib/Format/UnwrappedLineFormatter.h
@@ -28,6 +28,9 @@
 class ContinuationIndenter;
 class WhitespaceManager;
 
+/// \brief Responsible for taking a list of logical lines, and for producing
+/// a set of whitespace adjustments to turn them into physical lines so
+/// that each physical line is below a column limit.
 class UnwrappedLineFormatter {
 public:
   UnwrappedLineFormatter(ContinuationIndenter *Indenter,
Index: lib/Format/TokenAnnotator.h
===
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -36,6 +36,7 @@
   LT_VirtualFunctionDecl
 };
 
+/// \brief Stores additional information for an \c UnwrappedLine.
 class AnnotatedLine {
 public:
   AnnotatedLine(const UnwrappedLine &Line)
@@ -148,7 +149,11 @@
   // FIXME: Can/should this be done in the UnwrappedLineParser?
   void setCommentLineLevels(SmallVectorImpl &Lines);
 
+  /// \brief Compute metadata of \c AnnotatedLine itself, like \c LineType.
   void annotate(AnnotatedLine &Line);
+
+  /// \brief For all the tokens in \c Line, compute if a line break is
+  /// needed for that token, and similar information.
   void calculateFormattingInformation(AnnotatedLine &Line);
 
 private:
Index: lib/Format/FormatToken.h
===
--- lib/Format/FormatToken.h
+++ lib/Format/FormatToken.h
@@ -107,8 +107,8 @@
 class TokenRole;
 class AnnotatedLine;
 
-/// \brief A wrapper around a \c Token storing information about the
-/// whitespace characters preceding it.
+/// \brief A wrapper around a \c Token storing additional information useful
+/// for formatting.
 struct FormatToken {
   FormatToken() {}
 
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -758,6 +758,7 @@
 
 namespace {
 
+/// \brief Responsible for splitting up a file into \c FormatTokens.
 class FormatTokenLexer {
 public:
   FormatTokenLexer(SourceManager &SourceMgr, FileID ID, FormatStyle &Style,
@@ -1417,6 +1418,15 @@
   }
 }
 
+/// \brief Responsible for taking a file and producing whitespace adjustments
+/// to reformat the file according to a desired style.
+///
+/// The pipeline used by \c Formatter:
+/// -# A \c FormatTokenLexer splits the file into raw tokens
+/// -# A \c UnwrappedLineParser converts those into logical lines
+/// -# A \c TokenAnnotator computes metadata for the tokens in each line
+/// -# A \c UnwrappedLineFormatter then produces whitespace adjustments
+///which result in the desired logical lines
 class Formatter : public UnwrappedLineConsumer {
 public:
   Formatter(const FormatStyle &Style, SourceManager &SourceMgr, FileID ID,
Index: lib/Format/ContinuationIndenter.h
===
--- lib/Format/ContinuationIndenter.h
+++ lib/Format/ContinuationIndenter.h
@@ -32,6 +32,8 @@
 struct ParenState;
 class WhitespaceManager;
 
+/// \brief Helper class for breaking a logical line into multiple physical
+/// lines.  Manages moving state from one physical line to the next.
 class ContinuationIndenter {
 public:
   /// \brief Constructs a \c ContinuationIndenter to format \p Line starting in
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16041: Change vfs::FileSystem to be managed with std::shared_ptr

[Benjamin Kramer via cfe-commits]

On Mon, Jan 11, 2016 at 8:08 PM, Owen Anderson  wrote:
>
>> On Jan 11, 2016, at 8:25 AM, David Blaikie  wrote:
>>
>>
>>
>> On Sun, Jan 10, 2016 at 11:42 PM, Owen Anderson via cfe-commits 
>>  wrote:
>> resistor created this revision.
>> resistor added reviewers: chandlerc, bkramer, klimek.
>> resistor added a subscriber: cfe-commits.
>> resistor set the repository for this revision to rL LLVM.
>> Herald added a subscriber: klimek.
>>
>> Managing it with IntrusiveRefCntPtr caused the virtual destructor not to be 
>> called properly.
>>
>> Regardless of the broader discussion on this patch, I'm confused by why this 
>> ^ would be the case. What is it that IntrusiveRefCntPtr is doing that's 
>> causing problems with destruction? (& I'm all for changing this to 
>> non-intrusive smart pointers over intrusive ones anyway, but I'd still like 
>> to understand the extra motivation here)
>>
>
> ThreadSafeRefCountedBase, which classes must inherit from in order to use 
> IntrusiveRefCntPtr, does not handle virtual destructors properly.  For the 
> non-thread safe version, there is RefCountBaseVPTR which solves this problem. 
>  An alternative solution would have been to add a corresponding 
> ThreadSafeRefCountedBaseVPTR, but IMO at that point one might as well use 
> shared_ptr since it’s 90% of the way there.

I find this surprising. ThreadSafeRefCountedBase calls
"delete static_cast(this)". As FileSystem has a
virtual dtor, polymorphic deletion should just work. Am I missing
something?

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

Re: [PATCH] D16041: Change vfs::FileSystem to be managed with std::shared_ptr

[Owen Anderson via cfe-commits]

> On Jan 11, 2016, at 2:13 PM, Benjamin Kramer  wrote:
> 
> On Mon, Jan 11, 2016 at 8:08 PM, Owen Anderson  > wrote:
>> 
>>> On Jan 11, 2016, at 8:25 AM, David Blaikie  wrote:
>>> 
>>> 
>>> 
>>> On Sun, Jan 10, 2016 at 11:42 PM, Owen Anderson via cfe-commits 
>>>  wrote:
>>> resistor created this revision.
>>> resistor added reviewers: chandlerc, bkramer, klimek.
>>> resistor added a subscriber: cfe-commits.
>>> resistor set the repository for this revision to rL LLVM.
>>> Herald added a subscriber: klimek.
>>> 
>>> Managing it with IntrusiveRefCntPtr caused the virtual destructor not to be 
>>> called properly.
>>> 
>>> Regardless of the broader discussion on this patch, I'm confused by why 
>>> this ^ would be the case. What is it that IntrusiveRefCntPtr is doing 
>>> that's causing problems with destruction? (& I'm all for changing this to 
>>> non-intrusive smart pointers over intrusive ones anyway, but I'd still like 
>>> to understand the extra motivation here)
>>> 
>> 
>> ThreadSafeRefCountedBase, which classes must inherit from in order to use 
>> IntrusiveRefCntPtr, does not handle virtual destructors properly.  For the 
>> non-thread safe version, there is RefCountBaseVPTR which solves this 
>> problem.  An alternative solution would have been to add a corresponding 
>> ThreadSafeRefCountedBaseVPTR, but IMO at that point one might as well use 
>> shared_ptr since it’s 90% of the way there.
> 
> I find this surprising. ThreadSafeRefCountedBase calls
> "delete static_cast(this)". As FileSystem has a
> virtual dtor, polymorphic deletion should just work. Am I missing
> something?

I’m not an expert on this stuff, but I’ll refer you to the difference between 
RefCountedBase and RefCountedBaseVPTR, and point out that 
ThreadSafeRefCountedBase is like the former rather than the latter.

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

Re: [PATCH] D16079: [CUDA] Reject values for --cuda-gpu-arch that are not of the form /sm_\d+/.

[Eric Christopher via cfe-commits]

echristo accepted this revision.
echristo added a reviewer: echristo.
echristo added a comment.
This revision is now accepted and ready to land.

LGTM.

-eric


http://reviews.llvm.org/D16079



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

Re: [PATCH] D16082: [CUDA] Invoke ptxas and fatbinary during compilation.

[Artem Belevich via cfe-commits]

tra added a comment.

Make sure it works with -save-temps and -fintegrated-as/-fno-integrated-as. 
They tend to throw wrenches into pipeline construction.



Comment at: lib/Driver/Driver.cpp:1380
@@ +1379,3 @@
+  C.MakeAction(DeviceActions, types::TY_CUDA_FATBIN),
+  /* GpuArchName = */ nullptr,
+  /* AtTopLevel = */ false);

So, you're treating GpuArchName==nullptr as a special case of DeviceAction for 
fatbin?
Perhaps it warrants its own CudaDeviceLinkAction?


Comment at: lib/Driver/Driver.cpp:1620-1625
@@ -1602,3 +1619,8 @@
   case phases::Assemble:
-return C.MakeAction(Input, types::TY_Object);
+// Assembling generates an object file, except when we're assembling CUDA,
+// in which case we get a cubin file.
+return C.MakeAction(
+std::move(Input), TC.getTriple().getOS() == llvm::Triple::CUDA
+  ? types::TY_CUDA_CUBIN
+  : types::TY_Object);
   }

cubin *is* an ELF object file. Do we really need a new type here or can we get 
by with TY_Object?


Comment at: lib/Driver/ToolChains.cpp:4193
@@ +4192,3 @@
+: Linux(D, Triple, Args) {
+  if (CudaInstallation.isValid()) {
+getProgramPaths().push_back(CudaInstallation.getBinPath());

unneeded {} here and in few more places throughout the patch.


Comment at: lib/Driver/ToolChains.cpp:4230
@@ -4224,3 +4229,3 @@
   // Skip this argument unless the architecture matches BoundArch
-  if (A->getValue(0) != StringRef(BoundArch))
+  if (!BoundArch || A->getValue(0) != StringRef(BoundArch))
 continue;

You may as well move it out of the loop and return early if BoundArch is 
nullptr.


Comment at: lib/Driver/Tools.cpp:10621-10625
@@ +10620,7 @@
+  ArgStringList CmdArgs;
+  if (TC.getTriple().isArch64Bit()) {
+CmdArgs.push_back("-m64");
+  } else {
+CmdArgs.push_back("-m32");
+  }
+

CmdArgs.push_back(TC.getTriple().isArch64Bit() ? "-m64" : "-m32");

or, even

ArgStringList CmdArgs = {TC.getTriple().isArch64Bit() ? "-m64" : "-m32"};

Same in Linker::ConstructJob below.


Comment at: lib/Driver/Tools.cpp:10677-10678
@@ +10676,4 @@
+std::string Arch = A->getGpuArchName();
+// We need to name pass an Arch of the form "sm_XX" for cubin files and
+// "compute_XX" for ptx.  CudaDeviceAction's getGpuArchName() is guaranteed
+// to match "sm_XX".

First line does not parse.

In general compute_XX does not necessarily match sm_XX.
[[ http://docs.nvidia.com/cuda/cuda-compiler-driver-nvcc/#ptxas-options | ptxas 
options ]] says that 

> Allowed values for this option: compute_20, compute_30, compute_35, 
> compute_50, compute_52; and sm_20, sm_21, sm_30, sm_32, sm_35, sm_50 and sm_52

Note that there's no compute_21, compute_32. You'll need sm_XX -> compute_YY 
map.




Comment at: lib/Driver/Tools.h:923
@@ +922,3 @@
+
+// Runs fatbinary, which is sort of a linker for NVPTX.
+class LLVM_LIBRARY_VISIBILITY Linker : public Tool {

Please add more details about what fatbin does.
".. which combines GPU object files and, optionally, PTX assembly into a single 
output file."


http://reviews.llvm.org/D16082



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

Re: [PATCH] D16082: [CUDA] Invoke ptxas and fatbinary during compilation.

[Eric Christopher via cfe-commits]

echristo added a comment.

One question inline, one nit, and one more question here: You've got a couple 
of checks inline for null names/architectures, where do you expect those to 
come from and can you test for them? Or, another question, is if they're 
multiple architectures shouldn't we be able to see all of the actions that 
arise from each gpu?

Or I'm missing something, either way an explanation is good. :)

-eric



Comment at: lib/Driver/Driver.cpp:1880
@@ +1879,3 @@
+// retrieve the Input's GPU arch.
+II.setAction(A);
+return II;

Weren't you just adding it as part of the InputInfo constructor in 16078?


Comment at: lib/Driver/ToolChains.cpp:4261
@@ -4255,2 +4260,3 @@
 
-  DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ), BoundArch);
+  if (BoundArch) {
+DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ), 
BoundArch);

Nit: No braces around single lines.


http://reviews.llvm.org/D16082



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

Re: [PATCH] D16078: Add an Action* member to InputInfo.

[Eric Christopher via cfe-commits]

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

Sure. Seems reasonable.

-eric


http://reviews.llvm.org/D16078



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

r257403 - When a tag is declared in prototype scope in C, if we've decided that it

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Jan 11 16:41:53 2016
New Revision: 257403

URL: http://llvm.org/viewvc/llvm-project?rev=257403&view=rev
Log:
When a tag is declared in prototype scope in C, if we've decided that it
redeclares an existing tag but are creating a new declaration anyway (because
it has attributes or changes the visibility of the name), don't warn that it
won't be visible outside the current scope. That's not true.

Also narrow down the set of cases where we create these extra declarations when
building modules; previously, all tag declarations but the first in a module
header would get this treatment if -fmodules-local-submodule-visibility. (This
isn't a functional change, but we try to avoid creating these extra
declarations whenever we can.)

Added:
cfe/trunk/test/Modules/tag-injection.c
Modified:
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/Sema/decl-in-prototype.c

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=257403&r1=257402&r2=257403&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Jan 11 16:41:53 2016
@@ -12316,7 +12316,8 @@ Decl *Sema::ActOnTag(Scope *S, unsigned
 } else if (TUK == TUK_Reference &&
(PrevTagDecl->getFriendObjectKind() ==
 Decl::FOK_Undeclared ||
-getOwningModule(PrevDecl) !=
+PP.getModuleContainingLocation(
+PrevDecl->getLocation()) !=
 PP.getModuleContainingLocation(KWLoc)) &&
SS.isEmpty()) {
   // This declaration is a reference to an existing entity, but
@@ -12326,8 +12327,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned
   // the declaration would have meant the same thing if no prior
   // declaration were found, that is, if it was found in the same
   // scope where we would have injected a declaration.
-  if (!getTagInjectionContext(CurContext)
-   ->getRedeclContext()
+  if (!getTagInjectionContext(CurContext)->getRedeclContext()

->Equals(PrevDecl->getDeclContext()->getRedeclContext()))
 return PrevTagDecl;
   // This is in the injected scope, create a new declaration in
@@ -12634,7 +12634,7 @@ CreateNewDecl:
 << Name;
 Invalid = true;
   }
-} else {
+} else if (!PrevDecl) {
   Diag(Loc, diag::warn_decl_in_param_list) << Context.getTagDeclType(New);
 }
 DeclsInPrototypeScope.push_back(New);

Added: cfe/trunk/test/Modules/tag-injection.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/tag-injection.c?rev=257403&view=auto
==
--- cfe/trunk/test/Modules/tag-injection.c (added)
+++ cfe/trunk/test/Modules/tag-injection.c Mon Jan 11 16:41:53 2016
@@ -0,0 +1,18 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo 'struct a;' > %t/a.h
+// RUN: echo 'struct b {}; void foo(struct b*);' > %t/b.h
+// RUN: echo 'module X { module a { header "a.h" } module b { header "b.h" } 
}' > %t/x.modulemap
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t 
-fmodule-map-file=%t/x.modulemap %s -I%t -verify
+
+#include "a.h"
+
+void f(struct a *p);
+
+// FIXME: We should warn that 'b' will not be visible outside of this function,
+// but we merge this 'b' with X.b's 'b' because we don't yet implement C's
+// "compatible types" rule.
+void g(struct b *p);
+
+struct b b; // expected-error {{definition of 'b' must be imported from module 
'X.b' before it is required}}
+// expected-note@b.h:1 {{here}}

Modified: cfe/trunk/test/Sema/decl-in-prototype.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/decl-in-prototype.c?rev=257403&r1=257402&r2=257403&view=diff
==
--- cfe/trunk/test/Sema/decl-in-prototype.c (original)
+++ cfe/trunk/test/Sema/decl-in-prototype.c Mon Jan 11 16:41:53 2016
@@ -35,3 +35,6 @@ void f6(struct z {int b;} c) { // expect
 void pr19018_1 (enum e19018 { qq } x); // expected-warning{{declaration of 
'enum e19018' will not be visible outside of this function}}
 enum e19018 qq; //expected-error{{tentative definition has type 'enum e19018' 
that is never completed}} \
 //expected-note{{forward declaration of 'enum e19018'}}
+
+// Only warn once, even if we create two declarations.
+void f(struct q *, struct __attribute__((aligned(4))) q *); // 
expected-warning {{will not be visible outside}}


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

Re: [PATCH] D15314: Fix a bug in unavailable checking

[Manman Ren via cfe-commits]

manmanren added a comment.

Ping :]


http://reviews.llvm.org/D15314



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