Re: [PATCH] D17926: [clang-tidy] Don't delete unused parameter in class override method in anonymous namespace.

[Haojian Wu via cfe-commits]

hokein updated this revision to Diff 52332.
hokein added a comment.

Code format.


http://reviews.llvm.org/D17926

Files:
  clang-tidy/misc/UnusedParametersCheck.cpp
  test/clang-tidy/misc-unused-parameters.cpp

Index: test/clang-tidy/misc-unused-parameters.cpp
===
--- test/clang-tidy/misc-unused-parameters.cpp
+++ test/clang-tidy/misc-unused-parameters.cpp
@@ -127,6 +127,16 @@
   useFunction(&C::h);
 }
 
+class Base {
+  virtual void f(int i);
+};
+
+class Derived : public Base {
+  void f(int i) override {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning
+// CHECK-FIXES: void f(int  /*i*/) override {}
+};
+
 } // end namespace
 
 template  void someFunctionTemplate(T b, T e) { (void)b; (void)e; }
Index: clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tidy/misc/UnusedParametersCheck.cpp
@@ -16,6 +16,13 @@
 
 namespace clang {
 namespace tidy {
+namespace {
+bool isOverrideMethod(const FunctionDecl *Function) {
+  if (const auto *MD = dyn_cast(Function))
+return MD->size_overridden_methods() > 0 || MD->hasAttr();
+  return false;
+}
+} // namespace
 
 void UnusedParametersCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(functionDecl().bind("function"), this);
@@ -63,20 +70,15 @@
   auto MyDiag = diag(Param->getLocation(), "parameter '%0' is unused")
 << Param->getName();
 
-  auto UsedByRef = [&] {
-return !ast_matchers::match(
-decl(hasDescendant(
-declRefExpr(to(equalsNode(Function)),
-unless(hasAncestor(
-
callExpr(callee(equalsNode(Function,
-*Result.Context->getTranslationUnitDecl(), *Result.Context)
-.empty();
-  };
+  auto DeclRefExpr =
+  declRefExpr(to(equalsNode(Function)),
+  unless(hasAncestor(callExpr(callee(equalsNode(Function));
 
   // Comment out parameter name for non-local functions.
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
-  UsedByRef()) {
+  !ast_matchers::match(DeclRefExpr, *Result.Context).empty() ||
+  isOverrideMethod(Function)) {
 SourceRange RemovalRange(Param->getLocation(), Param->getLocEnd());
 // Note: We always add a space before the '/*' to not accidentally create a
 // '*/*' for pointer types, which doesn't start a comment. clang-format 
will


Index: test/clang-tidy/misc-unused-parameters.cpp
===
--- test/clang-tidy/misc-unused-parameters.cpp
+++ test/clang-tidy/misc-unused-parameters.cpp
@@ -127,6 +127,16 @@
   useFunction(&C::h);
 }
 
+class Base {
+  virtual void f(int i);
+};
+
+class Derived : public Base {
+  void f(int i) override {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning
+// CHECK-FIXES: void f(int  /*i*/) override {}
+};
+
 } // end namespace
 
 template  void someFunctionTemplate(T b, T e) { (void)b; (void)e; }
Index: clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tidy/misc/UnusedParametersCheck.cpp
@@ -16,6 +16,13 @@
 
 namespace clang {
 namespace tidy {
+namespace {
+bool isOverrideMethod(const FunctionDecl *Function) {
+  if (const auto *MD = dyn_cast(Function))
+return MD->size_overridden_methods() > 0 || MD->hasAttr();
+  return false;
+}
+} // namespace
 
 void UnusedParametersCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(functionDecl().bind("function"), this);
@@ -63,20 +70,15 @@
   auto MyDiag = diag(Param->getLocation(), "parameter '%0' is unused")
 << Param->getName();
 
-  auto UsedByRef = [&] {
-return !ast_matchers::match(
-decl(hasDescendant(
-declRefExpr(to(equalsNode(Function)),
-unless(hasAncestor(
-callExpr(callee(equalsNode(Function,
-*Result.Context->getTranslationUnitDecl(), *Result.Context)
-.empty();
-  };
+  auto DeclRefExpr =
+  declRefExpr(to(equalsNode(Function)),
+  unless(hasAncestor(callExpr(callee(equalsNode(Function));
 
   // Comment out parameter name for non-local functions.
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
-  UsedByRef()) {
+  !ast_matchers::match(DeclRefExpr, *Result.Context).empty() ||
+  isOverrideMethod(Function)) {
 SourceRange RemovalRange(Param->getLocation(), Param->getLocEnd());
 // Note: We always add a space before the '/*' to not accidentally create a
 // '*/*' for pointer types, which doesn't start a comment. clang-format will

[clang-tools-extra] r265117 - [clang-tidy] Don't delete unused parameter in class override method in anonymous namespace.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Fri Apr  1 02:57:30 2016
New Revision: 265117

URL: http://llvm.org/viewvc/llvm-project?rev=265117&view=rev
Log:
[clang-tidy] Don't delete unused parameter in class override method in 
anonymous namespace.

Summary: Fixes PR26740.

Reviewers: alexfh

Subscribers: cfe-commits

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

Modified:
clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp?rev=265117&r1=265116&r2=265117&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp Fri Apr  
1 02:57:30 2016
@@ -16,6 +16,13 @@ using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
+namespace {
+bool isOverrideMethod(const FunctionDecl *Function) {
+  if (const auto *MD = dyn_cast(Function))
+return MD->size_overridden_methods() > 0 || MD->hasAttr();
+  return false;
+}
+} // namespace
 
 void UnusedParametersCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(functionDecl().bind("function"), this);
@@ -63,20 +70,15 @@ void UnusedParametersCheck::warnOnUnused
   auto MyDiag = diag(Param->getLocation(), "parameter '%0' is unused")
 << Param->getName();
 
-  auto UsedByRef = [&] {
-return !ast_matchers::match(
-decl(hasDescendant(
-declRefExpr(to(equalsNode(Function)),
-unless(hasAncestor(
-
callExpr(callee(equalsNode(Function,
-*Result.Context->getTranslationUnitDecl(), *Result.Context)
-.empty();
-  };
+  auto DeclRefExpr =
+  declRefExpr(to(equalsNode(Function)),
+  unless(hasAncestor(callExpr(callee(equalsNode(Function));
 
   // Comment out parameter name for non-local functions.
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
-  UsedByRef()) {
+  !ast_matchers::match(DeclRefExpr, *Result.Context).empty() ||
+  isOverrideMethod(Function)) {
 SourceRange RemovalRange(Param->getLocation(), Param->getLocEnd());
 // Note: We always add a space before the '/*' to not accidentally create a
 // '*/*' for pointer types, which doesn't start a comment. clang-format 
will

Modified: clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp?rev=265117&r1=265116&r2=265117&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp Fri Apr  
1 02:57:30 2016
@@ -127,6 +127,16 @@ void someMoreCallSites() {
   useFunction(&C::h);
 }
 
+class Base {
+  virtual void f(int i);
+};
+
+class Derived : public Base {
+  void f(int i) override {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning
+// CHECK-FIXES: void f(int  /*i*/) override {}
+};
+
 } // end namespace
 
 template  void someFunctionTemplate(T b, T e) { (void)b; (void)e; }


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

Re: [PATCH] D17926: [clang-tidy] Don't delete unused parameter in class override method in anonymous namespace.

[Haojian Wu via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL265117: [clang-tidy] Don't delete unused parameter in class 
override method in… (authored by hokein).

Changed prior to commit:
  http://reviews.llvm.org/D17926?vs=52332&id=52336#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D17926

Files:
  clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp

Index: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -16,6 +16,13 @@
 
 namespace clang {
 namespace tidy {
+namespace {
+bool isOverrideMethod(const FunctionDecl *Function) {
+  if (const auto *MD = dyn_cast(Function))
+return MD->size_overridden_methods() > 0 || MD->hasAttr();
+  return false;
+}
+} // namespace
 
 void UnusedParametersCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(functionDecl().bind("function"), this);
@@ -63,20 +70,15 @@
   auto MyDiag = diag(Param->getLocation(), "parameter '%0' is unused")
 << Param->getName();
 
-  auto UsedByRef = [&] {
-return !ast_matchers::match(
-decl(hasDescendant(
-declRefExpr(to(equalsNode(Function)),
-unless(hasAncestor(
-
callExpr(callee(equalsNode(Function,
-*Result.Context->getTranslationUnitDecl(), *Result.Context)
-.empty();
-  };
+  auto DeclRefExpr =
+  declRefExpr(to(equalsNode(Function)),
+  unless(hasAncestor(callExpr(callee(equalsNode(Function));
 
   // Comment out parameter name for non-local functions.
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
-  UsedByRef()) {
+  !ast_matchers::match(DeclRefExpr, *Result.Context).empty() ||
+  isOverrideMethod(Function)) {
 SourceRange RemovalRange(Param->getLocation(), Param->getLocEnd());
 // Note: We always add a space before the '/*' to not accidentally create a
 // '*/*' for pointer types, which doesn't start a comment. clang-format 
will
Index: clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
@@ -127,6 +127,16 @@
   useFunction(&C::h);
 }
 
+class Base {
+  virtual void f(int i);
+};
+
+class Derived : public Base {
+  void f(int i) override {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning
+// CHECK-FIXES: void f(int  /*i*/) override {}
+};
+
 } // end namespace
 
 template  void someFunctionTemplate(T b, T e) { (void)b; (void)e; }


Index: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -16,6 +16,13 @@
 
 namespace clang {
 namespace tidy {
+namespace {
+bool isOverrideMethod(const FunctionDecl *Function) {
+  if (const auto *MD = dyn_cast(Function))
+return MD->size_overridden_methods() > 0 || MD->hasAttr();
+  return false;
+}
+} // namespace
 
 void UnusedParametersCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(functionDecl().bind("function"), this);
@@ -63,20 +70,15 @@
   auto MyDiag = diag(Param->getLocation(), "parameter '%0' is unused")
 << Param->getName();
 
-  auto UsedByRef = [&] {
-return !ast_matchers::match(
-decl(hasDescendant(
-declRefExpr(to(equalsNode(Function)),
-unless(hasAncestor(
-callExpr(callee(equalsNode(Function,
-*Result.Context->getTranslationUnitDecl(), *Result.Context)
-.empty();
-  };
+  auto DeclRefExpr =
+  declRefExpr(to(equalsNode(Function)),
+  unless(hasAncestor(callExpr(callee(equalsNode(Function));
 
   // Comment out parameter name for non-local functions.
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
-  UsedByRef()) {
+  !ast_matchers::match(DeclRefExpr, *Result.Context).empty() ||
+  isOverrideMethod(Function)) {
 SourceRange RemovalRange(Param->getLocation(), Param->getLocEnd());
 // Note: We always add a space before the '/*' to not accidentally create a
 // '*/*' for pointer types, which doesn't start a comment. clang-format will
Index: clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
==

Re: [PATCH] D17926: [clang-tidy] Don't delete unused parameter in class override method in anonymous namespace.

[Haojian Wu via cfe-commits]

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

Repository:
  rL LLVM

http://reviews.llvm.org/D17926



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

Re: [PATCH] D18110: [OpenMP] Fix SEMA bug in the capture of global variables in template functions.

[Alexey Bataev via cfe-commits]

ABataev added inline comments.


Comment at: lib/Sema/SemaOpenMP.cpp:816-822
@@ -801,6 +815,9 @@
+
+  // A DSA refers to this captured region if the parent contexts match.
+  auto *ParentContext = RSI->TheCapturedDecl->getParent();
   for (auto I = Stack.rbegin(), EE = Stack.rend(); I != EE; ++I)
-if (I->CurScope == S)
+if (I->ParentDeclContext == ParentContext)
   return I->Directive;
   return OMPD_unknown;
 }
 

sfantao wrote:
> ABataev wrote:
> > Actually, I think, we need to rework the whole IsOpenMPCapturedByRef() 
> > function. I'm not sure that it is even required. It looks like some 
> > optimization hack in the frontend. I'm against any not-necessary 
> > optimizations in frontend. If scalar value is a firstprivate, it must be 
> > handled in codegen, not by handling it by copy.
> 'IsOpenMPCapturedByRef' goal is to change the signature of the outlined 
> function. We don't want to have reference types in target region arguments 
> unless they are really required. We have seen performance being greatly 
> affected just because of that. Of course a consequence of this is to have 
> variables that become first private.
> 
> The current implementation of OpenMP firstprivate doesn't change the the 
> function signatures, only the codegen inside the region is affected. So, this 
> is not a complete overlap.
> 
> With this, I am not saying that this cannot be done in codegen. The reason I 
> am doing it here is your initial guideline that we should attempt to fix most 
> of the things in Sema and avoid complicating codegen which is a more 
> sensitive part.
> 
> Doing this in Codegen would require changing the implementation of 
> `GenerateOpenMPCapturedVars` and `GenerateOpenMPCapturedStmtFunction`. We 
> wouldn't need the tracking of the context anymore (checking the directive 
> would be sufficient), but we would still need to see what't in the map 
> clauses.
> 
> So, do you think we should change this?
I think we should not dot it at all for now. Performance is an important thing, 
of course, but at first we must have just working solution and only after that 
we will think about performance.
Early optimization leads to many troubles. The code becomes very hard to 
understand. It just increases the time of the development.
I suggest to remove all optimizations for now and emit the code as is, before 
we have a working solution.
Also, this function may lead to incorrect codegen. You don't check that 
CapturedRegion is actually an OpenMP region. If we have non-OpenMP cpatured 
region between OpenMP regions it will lead to incorrect code.


http://reviews.llvm.org/D18110



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

r265121 - [OPENMP 4.5] Allow data members as loop counters in loop-based

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Apr  1 04:23:34 2016
New Revision: 265121

URL: http://llvm.org/viewvc/llvm-project?rev=265121&view=rev
Log:
[OPENMP 4.5] Allow data members as loop counters in loop-based
directives.

OpenMP 4.5 allows privatization of non-static data members in non-static
member functions. Patch allows to use and implicit privatization of data
members used as counters in loop-based directives.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/for_lastprivate_codegen.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=265121&r1=265120&r2=265121&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Fri Apr  1 04:23:34 2016
@@ -90,7 +90,8 @@ private:
   };
   typedef llvm::DenseMap DeclSAMapTy;
   typedef llvm::DenseMap AlignedMapTy;
-  typedef llvm::DenseMap LoopControlVariablesMapTy;
+  typedef std::pair LCDeclInfo;
+  typedef llvm::DenseMap LoopControlVariablesMapTy;
   typedef llvm::DenseMap MappedDeclsTy;
   typedef llvm::StringMap>
   CriticalsWithHintsTy;
@@ -183,17 +184,17 @@ public:
   Expr *addUniqueAligned(ValueDecl *D, Expr *NewDE);
 
   /// \brief Register specified variable as loop control variable.
-  void addLoopControlVariable(ValueDecl *D);
+  void addLoopControlVariable(ValueDecl *D, VarDecl *Capture);
   /// \brief Check if the specified variable is a loop control variable for
   /// current region.
   /// \return The index of the loop control variable in the list of associated
   /// for-loops (from outer to inner).
-  unsigned isLoopControlVariable(ValueDecl *D);
+  LCDeclInfo isLoopControlVariable(ValueDecl *D);
   /// \brief Check if the specified variable is a loop control variable for
   /// parent region.
   /// \return The index of the loop control variable in the list of associated
   /// for-loops (from outer to inner).
-  unsigned isParentLoopControlVariable(ValueDecl *D);
+  LCDeclInfo isParentLoopControlVariable(ValueDecl *D);
   /// \brief Get the loop control variable for the I-th loop (or nullptr) in
   /// parent directive.
   ValueDecl *getParentLoopControlVariable(unsigned I);
@@ -522,24 +523,26 @@ Expr *DSAStackTy::addUniqueAligned(Value
   return nullptr;
 }
 
-void DSAStackTy::addLoopControlVariable(ValueDecl *D) {
+void DSAStackTy::addLoopControlVariable(ValueDecl *D, VarDecl *Capture) {
   assert(Stack.size() > 1 && "Data-sharing attributes stack is empty");
   D = getCanonicalDecl(D);
-  Stack.back().LCVMap.insert(std::make_pair(D, Stack.back().LCVMap.size() + 
1));
+  Stack.back().LCVMap.insert(
+  std::make_pair(D, LCDeclInfo(Stack.back().LCVMap.size() + 1, Capture)));
 }
 
-unsigned DSAStackTy::isLoopControlVariable(ValueDecl *D) {
+DSAStackTy::LCDeclInfo DSAStackTy::isLoopControlVariable(ValueDecl *D) {
   assert(Stack.size() > 1 && "Data-sharing attributes stack is empty");
   D = getCanonicalDecl(D);
-  return Stack.back().LCVMap.count(D) > 0 ? Stack.back().LCVMap[D] : 0;
+  return Stack.back().LCVMap.count(D) > 0 ? Stack.back().LCVMap[D]
+  : LCDeclInfo(0, nullptr);
 }
 
-unsigned DSAStackTy::isParentLoopControlVariable(ValueDecl *D) {
+DSAStackTy::LCDeclInfo DSAStackTy::isParentLoopControlVariable(ValueDecl *D) {
   assert(Stack.size() > 2 && "Data-sharing attributes stack is empty");
   D = getCanonicalDecl(D);
   return Stack[Stack.size() - 2].LCVMap.count(D) > 0
  ? Stack[Stack.size() - 2].LCVMap[D]
- : 0;
+ : LCDeclInfo(0, nullptr);
 }
 
 ValueDecl *DSAStackTy::getParentLoopControlVariable(unsigned I) {
@@ -547,7 +550,7 @@ ValueDecl *DSAStackTy::getParentLoopCont
   if (Stack[Stack.size() - 2].LCVMap.size() < I)
 return nullptr;
   for (auto &Pair : Stack[Stack.size() - 2].LCVMap) {
-if (Pair.second == I)
+if (Pair.second.first == I)
   return Pair.first;
   }
   return nullptr;
@@ -928,11 +931,12 @@ VarDecl *Sema::IsOpenMPCapturedDecl(Valu
   if (DSAStack->getCurrentDirective() != OMPD_unknown &&
   (!DSAStack->isClauseParsingMode() ||
DSAStack->getParentDirective() != OMPD_unknown)) {
-if (DSAStack->isLoopControlVariable(D) ||
+auto &&Info = DSAStack->isLoopControlVariable(D);
+if (Info.first ||
 (VD && VD->hasLocalStorage() &&
  isParallelOrTaskRegion(DSAStack->getCurrentDirective())) ||
 (VD && DSAStack->isForceVarCapturing()))
-  return VD;
+  return VD ? VD : Info.second;
 auto DVarPrivate = DSAStack->getTopDSA(D, DSAStack->isClauseParsingMode());
 if (DVarPrivate.CKind != OMPC_unknown && 
isOpenMPPrivate(DVarPrivate.CKind))
   return VD ? VD : cast(DVarPrivate.PrivateCopy->getDecl());
@@ -3242,33 +3246,29 @@ class OpenMPIterationSpaceChecker {
   /// \brief A source location for referring to increment later.
   SourceRange IncrementSrcRange;
   /// \b

r265123 - [OPENMP] Avoid useless recursive calls in getDSA if it is called in a loop, NFC

[Dmitry Polukhin via cfe-commits]

Author: dpolukhin
Date: Fri Apr  1 04:52:30 2016
New Revision: 265123

URL: http://llvm.org/viewvc/llvm-project?rev=265123&view=rev
Log:
[OPENMP] Avoid useless recursive calls in getDSA if it is called in a loop, NFC

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

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=265123&r1=265122&r2=265123&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Fri Apr  1 04:52:30 2016
@@ -141,7 +141,7 @@ private:
 
   typedef SmallVector::reverse_iterator reverse_iterator;
 
-  DSAVarData getDSA(StackTy::reverse_iterator Iter, ValueDecl *D);
+  DSAVarData getDSA(StackTy::reverse_iterator& Iter, ValueDecl *D);
 
   /// \brief Checks if the variable is a local for OpenMP region.
   bool isOpenMPLocal(VarDecl *D, StackTy::reverse_iterator Iter);
@@ -396,7 +396,7 @@ static ValueDecl *getCanonicalDecl(Value
   return D;
 }
 
-DSAStackTy::DSAVarData DSAStackTy::getDSA(StackTy::reverse_iterator Iter,
+DSAStackTy::DSAVarData DSAStackTy::getDSA(StackTy::reverse_iterator& Iter,
   ValueDecl *D) {
   D = getCanonicalDecl(D);
   auto *VD = dyn_cast(D);
@@ -505,7 +505,7 @@ DSAStackTy::DSAVarData DSAStackTy::getDS
   //  For constructs other than task, if no default clause is present, these
   //  variables inherit their data-sharing attributes from the enclosing
   //  context.
-  return getDSA(std::next(Iter), D);
+  return getDSA(++Iter, D);
 }
 
 Expr *DSAStackTy::addUniqueAligned(ValueDecl *D, Expr *NewDE) {
@@ -722,7 +722,7 @@ DSAStackTy::DSAVarData DSAStackTy::hasDS
   bool FromParent) {
   D = getCanonicalDecl(D);
   auto StartI = std::next(Stack.rbegin());
-  auto EndI = std::prev(Stack.rend());
+  auto EndI = Stack.rend();
   if (FromParent && StartI != EndI) {
 StartI = std::next(StartI);
   }
@@ -742,7 +742,7 @@ DSAStackTy::hasInnermostDSA(ValueDecl *D
 DirectivesPredicate DPred, bool FromParent) {
   D = getCanonicalDecl(D);
   auto StartI = std::next(Stack.rbegin());
-  auto EndI = std::prev(Stack.rend());
+  auto EndI = Stack.rend();
   if (FromParent && StartI != EndI) {
 StartI = std::next(StartI);
   }


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

r265125 - [Lexer] Don't read out of bounds if a conflict marker is at the end of a file

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Fri Apr  1 04:58:45 2016
New Revision: 265125

URL: http://llvm.org/viewvc/llvm-project?rev=265125&view=rev
Log:
[Lexer] Don't read out of bounds if a conflict marker is at the end of a file

This can happen as we look for '' while scanning tokens but then expect
'\n' to tell apart perforce from diff3 conflict markers. Just harden
the pointer arithmetic.

Found by libfuzzer + asan!

Added:
cfe/trunk/test/Lexer/eof-conflict-marker.c
Modified:
cfe/trunk/lib/Lex/Lexer.cpp

Modified: cfe/trunk/lib/Lex/Lexer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Lexer.cpp?rev=265125&r1=265124&r2=265125&view=diff
==
--- cfe/trunk/lib/Lex/Lexer.cpp (original)
+++ cfe/trunk/lib/Lex/Lexer.cpp Fri Apr  1 04:58:45 2016
@@ -2610,7 +2610,7 @@ static const char *FindConflictEnd(const
ConflictMarkerKind CMK) {
   const char *Terminator = CMK == CMK_Perforce ? "\n" : ">>>";
   size_t TermLen = CMK == CMK_Perforce ? 5 : 7;
-  StringRef RestOfBuffer(CurPtr+TermLen, BufferEnd-CurPtr-TermLen);
+  auto RestOfBuffer = StringRef(CurPtr, BufferEnd - CurPtr).substr(TermLen);
   size_t Pos = RestOfBuffer.find(Terminator);
   while (Pos != StringRef::npos) {
 // Must occur at start of line.

Added: cfe/trunk/test/Lexer/eof-conflict-marker.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/eof-conflict-marker.c?rev=265125&view=auto
==
--- cfe/trunk/test/Lexer/eof-conflict-marker.c (added)
+++ cfe/trunk/test/Lexer/eof-conflict-marker.c Fri Apr  1 04:58:45 2016
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+// vim: set binary noeol:
+
+// This file intentionally ends without a \n on the last line.  Make sure your
+// editor doesn't add one.
+
+ ORIGINAL
+// expected-error@-1 {{version control conflict marker in file}}
+
+// expected-error@-1 {{expected identifier or '('}}
+
\ No newline at end of file


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

r265126 - [Lexer] Let the compiler infer string lengths. No functionality change intended.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Fri Apr  1 05:04:07 2016
New Revision: 265126

URL: http://llvm.org/viewvc/llvm-project?rev=265126&view=rev
Log:
[Lexer] Let the compiler infer string lengths. No functionality change intended.

Modified:
cfe/trunk/lib/Lex/Lexer.cpp

Modified: cfe/trunk/lib/Lex/Lexer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Lexer.cpp?rev=265126&r1=265125&r2=265126&view=diff
==
--- cfe/trunk/lib/Lex/Lexer.cpp (original)
+++ cfe/trunk/lib/Lex/Lexer.cpp Fri Apr  1 05:04:07 2016
@@ -2636,8 +2636,8 @@ bool Lexer::IsStartOfConflictMarker(cons
 return false;
   
   // Check to see if we have <<< or .
-  if ((BufferEnd-CurPtr < 8 || StringRef(CurPtr, 7) != "<<<") &&
-  (BufferEnd-CurPtr < 6 || StringRef(CurPtr, 5) != " "))
+  if (!StringRef(CurPtr, BufferEnd - CurPtr).startswith("<<<") &&
+  !StringRef(CurPtr, BufferEnd - CurPtr).startswith(" "))
 return false;
 
   // If we have a situation where we don't care about conflict markers, ignore


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

r265127 - [OPENMP] Fixed documentation category for 'declare simd' attribute, NFC.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Apr  1 05:12:06 2016
New Revision: 265127

URL: http://llvm.org/viewvc/llvm-project?rev=265127&view=rev
Log:
[OPENMP] Fixed documentation category for 'declare simd' attribute, NFC.

Modified:
cfe/trunk/include/clang/Basic/AttrDocs.td

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=265127&r1=265126&r2=265127&view=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Fri Apr  1 05:12:06 2016
@@ -1900,7 +1900,7 @@ arguments, with arbitrary offsets.
 }
 
 def OMPDeclareSimdDocs : Documentation {
-  let Category = DocCatStmt;
+  let Category = DocCatFunction;
   let Heading = "#pragma omp declare simd";
   let Content = [{
 The `declare simd` construct can be applied to a function to enable the 
creation


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

Re: [PATCH] D18542: [OPENMP] Parsing and Sema support for 'omp declare target' directive

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin updated this revision to Diff 52349.
DmitryPolukhin marked an inline comment as done.
DmitryPolukhin added a comment.

- implemented ad hoc solution for printing
- added documentation for the attrbute
- reabse

In http://reviews.llvm.org/D18542#388241, @aaron.ballman wrote:

> If you don't feel like doing all that design work (which I'm not attempting 
> to sign you up for!), I think the ad hoc solution may be preferable to the 
> proposed approach. Do you intend to add several more pragmas that need this 
> same sort of behavior, or is this a one-off?


I don't expect more pragmas like declare target and grouping mechanism design 
might be overkill for my case. So I implemented ad hoc solution that is much 
shorter. PTAL!


http://reviews.llvm.org/D18542

Files:
  include/clang/AST/ASTMutationListener.h
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/OpenMPKinds.def
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTWriter.h
  lib/AST/ASTContext.cpp
  lib/AST/DeclPrinter.cpp
  lib/Basic/OpenMPKinds.cpp
  lib/Frontend/MultiplexConsumer.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Serialization/ASTCommon.h
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/OpenMP/declare_target_ast_print.cpp
  test/OpenMP/declare_target_messages.cpp

Index: test/OpenMP/declare_target_messages.cpp
===
--- /dev/null
+++ test/OpenMP/declare_target_messages.cpp
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify -fopenmp -fnoopenmp-use-tls -ferror-limit 100 -o - %s
+
+#pragma omp end declare target // expected-error {{unexpected OpenMP directive '#pragma omp end declare target'}}
+
+int a, b; // expected-warning {{declaration is not declared in any declare target region}}
+__thread int t; // expected-note {{defined as threadprivate or thread local}}
+#pragma omp declare target private(a) // expected-warning {{extra tokens at the end of '#pragma omp declare target' are ignored}}
+void f();
+#pragma omp end declare target shared(a) // expected-warning {{extra tokens at the end of '#pragma omp end declare target' are ignored}}
+void c(); // expected-warning {{declaration is not declared in any declare target region}}
+
+extern int b;
+
+struct NonT {
+  int a;
+};
+
+typedef int sint;
+
+#pragma omp declare target // expected-note {{to match this '#pragma omp declare target'}}
+#pragma omp threadprivate(a) // expected-note {{defined as threadprivate or thread local}}
+extern int b;
+int g;
+
+struct T { // expected-note {{mappable type cannot be polymorphic}}
+  int a;
+  virtual int method();
+};
+
+class VC { // expected-note {{mappable type cannot be polymorphic}}
+  T member;
+  NonT member1;
+  public:
+virtual int method() { T a; return 0; } // expected-error {{type 'T' is not mappable to target}}
+};
+
+struct C {
+  NonT a;
+  sint b;
+  int method();
+  int method1();
+};
+
+int C::method1() {
+  return 0;
+}
+
+void foo() {
+  a = 0; // expected-error {{threadprivate variables cannot be used in target constructs}}
+  b = 0; // expected-note {{used here}}
+  t = 1; // expected-error {{threadprivate variables cannot be used in target constructs}}
+  C object;
+  VC object1; // expected-error {{type 'VC' is not mappable to target}}
+  g = object.method();
+  g += object.method1();
+  g += object1.method();
+  f();
+  c(); // expected-note {{used here}}
+}
+#pragma omp declare target // expected-error {{expected '#pragma omp end declare target'}}
+void foo1() {}
+#pragma omp end declare target
+#pragma omp end declare target // expected-error {{unexpected OpenMP directive '#pragma omp end declare target'}}
+
+int C::method() {
+  return 0;
+}
+
+struct S {
+#pragma omp declare target // expected-error {{directive must be at file or namespace scope}}
+  int v;
+#pragma omp end declare target // expected-error {{unexpected OpenMP directive '#pragma omp end declare target'}}
+};
+
+int main (int argc, char **argv) {
+#pragma omp declare target // expected-error {{unexpected OpenMP directive '#pragma omp declare target'}}
+  int v;
+#pragma omp end declare target // expected-error {{unexpected OpenMP directive '#pragma omp end declare target'}}
+  foo();
+  return (0);
+}
+
+namespace {
+#pragma omp declare target // expected-note {{to match this '#pragma omp declare target'}}
+  int x;
+} //  expected-error {{expected '#pragma omp end declare target'}}
+#pragma omp end declare target // expected-error {{unexpected OpenMP directive '#pragma omp end declare target'}}
+
+#pragma omp declare target // expected-error {{expected '#pragma omp end declare target'}} expected-note {{to match this '#pragma omp declare targe

Re: [PATCH] D18649: [clang-tidy] cppcoreguidelines-interfaces-global-init

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

In http://reviews.llvm.org/D18649#389294, @courbet wrote:

> In http://reviews.llvm.org/D18649#388337, @Eugene.Zelenko wrote:
>
> > Please mention new check in docs/ReleaseNotes.rst.
>
>
> That should be in a different commit, right ? Release notes are in a 
> different repo (forgive my ignorance, this is my first patch).


Now they are in the same repo (clang-tools-extra, docs/ReleaseNotes.rst).


http://reviews.llvm.org/D18649



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

Re: [PATCH] D10834: Added functions to retrieve information about whether a vardecl is local in libclang and its python bindings.

[guibufolo+l...@gmail.com via cfe-commits]

RedX2501 updated this revision to Diff 52358.
RedX2501 added a comment.

Fixed compiler warnings.


http://reviews.llvm.org/D10834

Files:
  bindings/python/clang/cindex.py
  bindings/python/tests/cindex/test_cursor.py
  include/clang-c/Index.h
  test/Index/islocalvardecl.cpp
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp
  tools/libclang/libclang.exports

Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -33,6 +33,7 @@
 clang_Cursor_isNull
 clang_Cursor_isObjCOptional
 clang_Cursor_isVariadic
+clang_Cursor_isLocalVarDecl
 clang_Cursor_getModule
 clang_Cursor_getStorageClass
 clang_File_isEqual
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -6921,6 +6921,19 @@
   return 0;
 }
 
+unsigned clang_Cursor_isLocalVarDecl(CXCursor C){
+  if (C.kind != CXCursor_VarDecl) {
+return 0;
+  }
+
+  const Decl *D = getCursorDecl(C);
+  if (const VarDecl *VD = dyn_cast(D)) {
+		return VD->isLocalVarDecl();
+	}
+
+	return 0;
+}
+
 CXSourceRange clang_Cursor_getCommentRange(CXCursor C) {
   if (!clang_isDeclaration(C.kind))
 return clang_getNullRange();
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -1276,6 +1276,22 @@
 }
 
 /**/
+/* Local VarKind testing. */
+/**/
+
+static enum CXChildVisitResult PrintIsLocalVarDecl(CXCursor C, CXCursor p,
+   CXClientData d){
+
+  if (clang_getCursorKind(C) != CXCursor_VarDecl)
+return CXChildVisit_Recurse;
+
+  PrintCursor(C, NULL);
+  printf(" IsLocalVarDecl=%d\n", clang_Cursor_isLocalVarDecl(C));
+
+  return CXChildVisit_Recurse;
+}
+
+/**/
 /* Typekind testing.  */
 /**/
 
@@ -4150,10 +4166,12 @@
   fprintf(stderr,
 "   c-index-test -test-print-linkage-source {}*\n"
 "   c-index-test -test-print-visibility {}*\n"
+"   c-index-test -test-print-is-local-var {}*\n"
 "   c-index-test -test-print-type {}*\n"
 "   c-index-test -test-print-type-size {}*\n"
 "   c-index-test -test-print-bitwidth {}*\n"
 "   c-index-test -test-print-type-declaration {}*\n"
+  fprintf(stderr,
 "   c-index-test -print-usr [ {}]*\n"
 "   c-index-test -print-usr-file \n"
 "   c-index-test -write-pch  \n");
@@ -4241,6 +4259,9 @@
   else if (argc > 2 && strcmp(argv[1], "-test-print-visibility") == 0)
 return perform_test_load_source(argc - 2, argv + 2, "all", PrintVisibility,
 NULL);
+	else if (argc > 2 && strcmp(argv[1], "-test-print-is-local-var") == 0)
+return perform_test_load_source(argc - 2, argv + 2, "all", PrintIsLocalVarDecl,
+NULL);
   else if (argc > 2 && strcmp(argv[1], "-test-print-type") == 0)
 return perform_test_load_source(argc - 2, argv + 2, "all",
 PrintType, 0);
Index: test/Index/islocalvardecl.cpp
===
--- /dev/null
+++ test/Index/islocalvardecl.cpp
@@ -0,0 +1,32 @@
+// RUN: c-index-test -test-print-local-var-kind %s | FileCheck %s
+
+extern "C" {
+  int var0;
+  static int var1;
+
+  void func(void){
+static int var2;
+int var3;
+  }
+};
+
+int var4;
+
+class Classy {
+  static int var5;
+
+  void member(){
+int var6;
+static int var7;
+  }
+};
+
+// CHECK: VarDecl=var0:3:5 (Definition) IsLocalVarDecl=0
+// CHECK: VarDecl=var1:4:12 (Definition) IsLocalVarDecl=0
+// CHECK: VarDecl=var2:7:14 (Definition) IsLocalVarDecl=1
+// CHECK: VarDecl=var3:8:7 (Definition) IsLocalVarDecl=1
+// CHECK: VarDecl=var4:13:5 (Definition) IsLocalVarDecl=0
+// CHECK: VarDecl=var5:17:12 IsLocalVarDecl=0
+// CHECK: VarDecl=var6:20:9 (Definition) IsLocalVarDecl=1
+// CHECK: VarDecl=var7:21:16 (Definition) IsLocalVarDecl=1
+
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -3882,6 +3882,11 @@
 CINDEX_LINKAGE unsigned clang_Cursor_isVariadic(CXCursor C);
 
 /**
+ * \brief Returns non-zero if the Cursor refers to a local VarDecl.
+ */
+CINDEX_LINKAGE unsigned clang_Cursor_isLocalVarDecl(CXCursor C);
+
+/**
  * \brief Given a cursor that represents a declaration, return the associ

[PATCH] D18695: [clang-tidy] Reduce false-positive ratio in misc-suspicious-missing-comma check.

[Etienne Bergeron via cfe-commits]

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

This patch is adding detection of common string literal patterns
that should not trigger warnings.

  [*] Add a limit on the number of concatenated token,
  [*] Add support for parenthese sequence of tokens,
  [*] Add detection of valid indentation.

As an example, this code will no longer trigger a warning:
```
const char* Array[] = {
  "first literal"
"indented literal"
"indented literal",
  "second literal",
  [...]
```

http://reviews.llvm.org/D18695

Files:
  clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
  clang-tidy/misc/SuspiciousMissingCommaCheck.h
  test/clang-tidy/misc-suspicious-missing-comma.cpp

Index: test/clang-tidy/misc-suspicious-missing-comma.cpp
===
--- test/clang-tidy/misc-suspicious-missing-comma.cpp
+++ test/clang-tidy/misc-suspicious-missing-comma.cpp
@@ -15,7 +15,8 @@
   L"Red", L"Yellow", L"Blue", L"Green", L"Purple", L"Rose", L"White", L"Black"
 };
 
-// The following array should not trigger any warnings.
+// The following array should not trigger any warnings. There is more than 5
+// elements, but they are all concatenated string literals.
 const char* HttpCommands[] = {
   "GET / HTTP/1.0\r\n"
   "\r\n",
@@ -26,9 +27,56 @@
   "GET /favicon.ico HTTP/1.0\r\n"
   "header: dummy"
   "\r\n",
+
+  "GET /index.html-en HTTP/1.0\r\n"
+  "\r\n",
+
+  "GET /index.html-fr HTTP/1.0\r\n"
+  "\r\n",
+
+  "GET /index.html-es HTTP/1.0\r\n"
+  "\r\n",
 };
 
 // This array is too small to trigger a warning.
 const char* SmallArray[] = {
   "a" "b", "c"
 };
+
+// Parentheses should be enough to avoid warnings.
+const char* ParentheseArray[] = {
+  ("a" "b"), "c",
+  ("d"
+   "e"
+   "f"),
+  "g", "h", "i", "j", "k", "l"
+};
+
+// Indentation should be enough to avoid warnings.
+const char* CorrectlyIndentedArray[] = {
+  "This is a long message "
+  "which is spanning over multiple lines."
+  "And this should be fine.",
+  "a", "b", "c", "d", "e", "f",
+  "g", "h", "i", "j", "k", "l"
+};
+
+const char* IncorrectlyIndentedArray[] = {
+  "This is a long message "
+  "which is spanning over multiple lines."
+  "And this should be fine.",
+  "a", "b", "c", "d", "e", "f",
+  "g", "h", "i", "j", "k", "l"
+};
+// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: suspicious string literal, probably missing a comma [misc-suspicious-missing-comma]
+
+const char* ManyConcatenatedTokensArray[] = {
+  "Dummy line",
+  "Dummy line",
+  "a" "b" "c" "d" "e" "f",
+  "g" "h" "i" "j" "k" "l",
+  "Dummy line",
+  "Dummy line",
+  "Dummy line",
+  "Dummy line",
+};
Index: clang-tidy/misc/SuspiciousMissingCommaCheck.h
===
--- clang-tidy/misc/SuspiciousMissingCommaCheck.h
+++ clang-tidy/misc/SuspiciousMissingCommaCheck.h
@@ -32,6 +32,8 @@
   const unsigned SizeThreshold;
   // Maximal threshold ratio of suspicious string literals to be considered.
   const double RatioThreshold;
+  // Maximal number of concatenated tokens.
+  const unsigned MaxConcatenatedTokens;
 };
 
 } // namespace misc
Index: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
===
--- clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
+++ clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
@@ -19,22 +19,65 @@
 
 namespace {
 
+bool isConcatenatedLiteralsOnPurpose(ASTContext* Ctx,
+ const StringLiteral* Lit) {
+  // String literals surrounded by parentheses are assumed to be on purpose.
+  //i.e.:  const char* Array[] = { ("a" "b" "c"), "d", [...] };
+  auto Parents = Ctx->getParents(*Lit);
+  if (Parents.size() == 1 && Parents[0].get() != nullptr)
+return true;
+
+  // Appropriately indented string literals are assumed to be on purpose.
+  // The following frequent indentation is accepted:
+  // const char* Array[] = {
+  //   "first literal"
+  //   "indented literal"
+  //   "indented literal",
+  //   "second literal",
+  //   [...]
+  // };
+  const SourceManager& SM = Ctx->getSourceManager();
+  bool IndentedCorrectly = true;
+  SourceLocation FirstToken = Lit->getStrTokenLoc(0);
+  FileID BaseFID = SM.getFileID(FirstToken);
+  unsigned int BaseIndent = SM.getSpellingColumnNumber(FirstToken);
+  unsigned int BaseLine = SM.getSpellingLineNumber(FirstToken);
+  for (unsigned int TokNum = 1; TokNum < Lit->getNumConcatenated(); ++ TokNum) {
+SourceLocation Token = Lit->getStrTokenLoc(TokNum);
+FileID FID = SM.getFileID(Token);
+unsigned int Indent = SM.getSpellingColumnNumber(Token);
+unsigned int Line = SM.getSpellingLineNumber(Token);
+if (FID != BaseFID || Line != BaseLine + TokNum || Indent <= BaseIndent) {
+  IndentedCorrectly = false;
+  break;
+}
+  }
+  if (IndentedCorrectly)
+return true;
+
+  // There is no pattern recognized

Re: [PATCH] D18649: [clang-tidy] cppcoreguidelines-interfaces-global-init

[Clement Courbet via cfe-commits]

courbet updated this revision to Diff 52353.
courbet added a comment.

Updated release notes.


http://reviews.llvm.org/D18649

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.cpp
  clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-interfaces-global-init.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp

Index: test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp
@@ -0,0 +1,64 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-interfaces-global-init %t
+
+constexpr int makesInt() { return 3; }
+constexpr int takesInt(int i) { return i + 1; }
+constexpr int takesIntPtr(int *i) { return *i; }
+
+extern int ExternGlobal;
+static int GlobalScopeBadInit1 = ExternGlobal;
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with
+// non-const expression depending on static variable 'ExternGlobal'
+static int GlobalScopeBadInit2 = takesInt(ExternGlobal);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with
+// non-const expression depending on static variable 'ExternGlobal'
+static int GlobalScopeBadInit3 = takesIntPtr(&ExternGlobal);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with
+// non-const expression depending on static variable 'ExternGlobal'
+static int GlobalScopeBadInit4 = 3 * (ExternGlobal + 2);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with
+// non-const expression depending on static variable 'ExternGlobal'
+
+namespace ns {
+static int NamespaceScope = makesInt();
+static int NamespaceScopeBadInit = takesInt(ExternGlobal);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with
+// non-const expression depending on static variable 'ExternGlobal'
+
+struct A {
+  static int ClassScope;
+  static int ClassScopeBadInit;
+};
+
+int A::ClassScopeBadInit = takesInt(ExternGlobal);
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: initializing static variable with
+// non-const expression depending on static variable 'ExternGlobal'
+
+static int FromClassBadInit = takesInt(A::ClassScope);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with
+// non-const expression depending on static variable 'ClassScope'
+} // namspace ns
+
+void f() {
+  // This is fine, it's executed after dynamic initilization occurs.
+  static int G = takesInt(ExternGlobal);
+}
+
+// Defined global variables are fine:
+static int GlobalScope = makesInt();
+static int GlobalScopeBadInit = takesInt(GlobalScope);
+static int GlobalScope2 = takesInt(ns::NamespaceScope);
+
+// Enums are fine.
+enum Enum { kEnumValue = 1 };
+static int GlobalScopeFromEnum = takesInt(kEnumValue);
+
+// Leave constexprs alone.
+extern constexpr int GlobalScopeConstexpr = makesInt();
+static constexpr int GlobalScopeConstexprOk =
+takesInt(GlobalScopeConstexpr);
+
+// This is a pretty common instance: People are usually not using constexpr, but
+// this is what they should write:
+static constexpr const char kValue[] = "value";
+constexpr int Fingerprint(const char *value) { return 0; }
+static int kFingerprint = Fingerprint(kValue);
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
cert-fio38-c (redirects to misc-non-copyable-objects) 
cert-flp30-c
cert-oop11-cpp (redirects to misc-move-constructor-init) 
+   cppcoreguidelines-interfaces-global-init
cppcoreguidelines-pro-bounds-array-to-pointer-decay
cppcoreguidelines-pro-bounds-constant-array-index
cppcoreguidelines-pro-bounds-pointer-arithmetic
Index: docs/clang-tidy/checks/cppcoreguidelines-interfaces-global-init.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-interfaces-global-init.rst
@@ -0,0 +1,11 @@
+.. title:: clang-tidy - cppcoreguidelines-interfaces-global-init
+
+cppcoreguidelines-interfaces-global-init
+
+
+This check flags initializers of globals that access extern objects,
+and therefore can lead to order-of-initialization problems.
+
+This rule is part of the "Interfaces" profile of the C++ Core Guidelines, see
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Ri-global-init
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -153,6 +153,13 @@
 
   Finds unnecessary string initializations.
 
+- New `cppcoreguidelines-interfaces-

Re: [PATCH] D18649: [clang-tidy] cppcoreguidelines-interfaces-global-init

[Clement Courbet via cfe-commits]

courbet added a comment.

In http://reviews.llvm.org/D18649#388337, @Eugene.Zelenko wrote:

> Please mention new check in docs/ReleaseNotes.rst.


That should be in a different commit, right ? Release notes are in a different 
repo (forgive my ignorance, this is my first patch).


http://reviews.llvm.org/D18649



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

Re: [PATCH] D18695: [clang-tidy] Reduce false-positive ratio in misc-suspicious-missing-comma check.

[Etienne Bergeron via cfe-commits]

etienneb updated this revision to Diff 52365.
etienneb added a comment.

Simplification and nits.


http://reviews.llvm.org/D18695

Files:
  clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
  clang-tidy/misc/SuspiciousMissingCommaCheck.h
  test/clang-tidy/misc-suspicious-missing-comma.cpp

Index: test/clang-tidy/misc-suspicious-missing-comma.cpp
===
--- test/clang-tidy/misc-suspicious-missing-comma.cpp
+++ test/clang-tidy/misc-suspicious-missing-comma.cpp
@@ -15,7 +15,8 @@
   L"Red", L"Yellow", L"Blue", L"Green", L"Purple", L"Rose", L"White", L"Black"
 };
 
-// The following array should not trigger any warnings.
+// The following array should not trigger any warnings. There is more than 5
+// elements, but they are all concatenated string literals.
 const char* HttpCommands[] = {
   "GET / HTTP/1.0\r\n"
   "\r\n",
@@ -26,9 +27,56 @@
   "GET /favicon.ico HTTP/1.0\r\n"
   "header: dummy"
   "\r\n",
+
+  "GET /index.html-en HTTP/1.0\r\n"
+  "\r\n",
+
+  "GET /index.html-fr HTTP/1.0\r\n"
+  "\r\n",
+
+  "GET /index.html-es HTTP/1.0\r\n"
+  "\r\n",
 };
 
 // This array is too small to trigger a warning.
 const char* SmallArray[] = {
   "a" "b", "c"
 };
+
+// Parentheses should be enough to avoid warnings.
+const char* ParentheseArray[] = {
+  ("a" "b"), "c",
+  ("d"
+   "e"
+   "f"),
+  "g", "h", "i", "j", "k", "l"
+};
+
+// Indentation should be enough to avoid warnings.
+const char* CorrectlyIndentedArray[] = {
+  "This is a long message "
+  "which is spanning over multiple lines."
+  "And this should be fine.",
+  "a", "b", "c", "d", "e", "f",
+  "g", "h", "i", "j", "k", "l"
+};
+
+const char* IncorrectlyIndentedArray[] = {
+  "This is a long message "
+  "which is spanning over multiple lines."
+  "And this should be fine.",
+  "a", "b", "c", "d", "e", "f",
+  "g", "h", "i", "j", "k", "l"
+};
+// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: suspicious string literal, probably missing a comma [misc-suspicious-missing-comma]
+
+const char* TooManyConcatenatedTokensArray[] = {
+  "Dummy line",
+  "Dummy line",
+  "a" "b" "c" "d" "e" "f",
+  "g" "h" "i" "j" "k" "l",
+  "Dummy line",
+  "Dummy line",
+  "Dummy line",
+  "Dummy line",
+};
Index: clang-tidy/misc/SuspiciousMissingCommaCheck.h
===
--- clang-tidy/misc/SuspiciousMissingCommaCheck.h
+++ clang-tidy/misc/SuspiciousMissingCommaCheck.h
@@ -32,6 +32,8 @@
   const unsigned SizeThreshold;
   // Maximal threshold ratio of suspicious string literals to be considered.
   const double RatioThreshold;
+  // Maximal number of concatenated tokens.
+  const unsigned MaxConcatenatedTokens;
 };
 
 } // namespace misc
Index: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
===
--- clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
+++ clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
@@ -19,27 +19,72 @@
 
 namespace {
 
-AST_MATCHER(StringLiteral, isConcatenatedLiteral) {
-  return Node.getNumConcatenated() > 1;
+bool isConcatenatedLiteralsOnPurpose(ASTContext* Ctx,
+ const StringLiteral* Lit) {
+  // String literals surrounded by parentheses are assumed to be on purpose.
+  //i.e.:  const char* Array[] = { ("a" "b" "c"), "d", [...] };
+  auto Parents = Ctx->getParents(*Lit);
+  if (Parents.size() == 1 && Parents[0].get() != nullptr)
+return true;
+
+  // Appropriately indented string literals are assumed to be on purpose.
+  // The following frequent indentation is accepted:
+  // const char* Array[] = {
+  //   "first literal"
+  //   "indented literal"
+  //   "indented literal",
+  //   "second literal",
+  //   [...]
+  // };
+  const SourceManager& SM = Ctx->getSourceManager();
+  bool IndentedCorrectly = true;
+  SourceLocation FirstToken = Lit->getStrTokenLoc(0);
+  FileID BaseFID = SM.getFileID(FirstToken);
+  unsigned int BaseIndent = SM.getSpellingColumnNumber(FirstToken);
+  unsigned int BaseLine = SM.getSpellingLineNumber(FirstToken);
+  for (unsigned int TokNum = 1; TokNum < Lit->getNumConcatenated(); ++ TokNum) {
+SourceLocation Token = Lit->getStrTokenLoc(TokNum);
+FileID FID = SM.getFileID(Token);
+unsigned int Indent = SM.getSpellingColumnNumber(Token);
+unsigned int Line = SM.getSpellingLineNumber(Token);
+if (FID != BaseFID || Line != BaseLine + TokNum || Indent <= BaseIndent) {
+  IndentedCorrectly = false;
+  break;
+}
+  }
+  if (IndentedCorrectly)
+return true;
+
+  // There is no pattern recognized by the checker, assume it's not on purpose.
+  return false;
+}
+
+AST_MATCHER_P(StringLiteral, isConcatenatedLiteral,
+  unsigned, MaxConcatenatedTokens) {
+  return Node.getNumConcatenated() > 1 &&
+ Node.getNumConcatenated() < MaxConcatenatedTokens &&
+ !isConcatenatedLiteralsOnPurpose(&Finder->getASTContext(), &Nod

Re: [PATCH] D18649: [clang-tidy] cppcoreguidelines-interfaces-global-init

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Thank you for working on the new clang-tidy check!

We usually recommend authors to run their checks on a large code base to ensure 
it doesn't crash and doesn't generate obvious false positives. It would be 
nice, if you could provide a quick summary of such a run (total number of hits, 
number of what seems to be a false positive in a sample of ~100).



Comment at: clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.cpp:41
@@ +40,3 @@
+  // For now assume that people who write macros know what they're doing.
+  if (Var->getLocation().isMacroID()) {
+return;

nit: No need for braces around single-line if/for/... bodies.


Comment at: clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.h:20
@@ +19,3 @@
+/// Flags possible initialization order issues of static variables.
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-interfaces-global-init.html
+class InterfacesGlobalInitCheck : public ClangTidyCheck {

I'd leave the "For user-facing documentation, see ..." phrase that the 
add_new_check.py script adds.


Comment at: test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp:37
@@ +36,3 @@
+static int FromClassBadInit = takesInt(A::ClassScope);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with
+// non-const expression depending on static variable 'ClassScope'

These should be on a single line. I guess, your clang-format doesn't use 
-style=file by default and doesn't understand the local style configured for 
the test/ directory. Otherwise it wouldn't split the lines.


http://reviews.llvm.org/D18649



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

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Samuel Benzaquen via cfe-commits]

sbenza added inline comments.


Comment at: clang-tidy/misc/AssignOperatorCheck.cpp:63
@@ +62,3 @@
+
+  Finder->addMatcher(returnStmt(IsBadReturnStatement, 
hasAncestor(IsGoodAssign))
+ .bind("returnStmt"),

I dislike these uses of hasAnscestor. They are kind of slow.
But more importantly, they break with nested functions/types.
This particular example is not checking that the return statement is from the 
assignment operator, only that it is within it. For example, it would match a 
lambda.
I think this would trip the check:

F& operator=(const F& o) {
  std::copy_if(o.begin(), o.end(), begin(), [](V v) { return v > 0; });
  return *this;
}


Comment at: clang-tidy/misc/AssignOperatorCheck.cpp:69
@@ +68,3 @@
+void AssignOperatorCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Method = Result.Nodes.getNodeAs("method");
+  const auto *RetStmt = Result.Nodes.getNodeAs("returnStmt");

Move this closer to where it is used.


Comment at: clang-tidy/misc/AssignOperatorCheck.cpp:70
@@ +69,3 @@
+  const auto *Method = Result.Nodes.getNodeAs("method");
+  const auto *RetStmt = Result.Nodes.getNodeAs("returnStmt");
+  if (RetStmt) {

Move this into the if () statement.


Comment at: clang-tidy/misc/AssignOperatorCheck.h:19
@@ +18,3 @@
+
+/// Finds declarations of assignment operators with the wrong return and/or
+/// argument types.

This does not talk about the return statement, only the return type.


Comment at: test/clang-tidy/misc-assign-operator.cpp:16
@@ +15,3 @@
+  AlsoGood& operator=(AlsoGood);
+};
+

This is a very common C++98 way of implementing copy-and-swap with copy elision 
support.
You do: `T& operator=(T t) { swap(t); return *this; }`
And it will avoid the copy if the argument is already a temporary due to copy 
elision on the caller.


Comment at: test/clang-tidy/misc-assign-operator.cpp:69
@@ +68,3 @@
+n = rhs.n;
+return *this;
+  }

This case is not a bad return statement, so it should not be in this class.


http://reviews.llvm.org/D18265



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

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: test/clang-tidy/misc-assign-operator.cpp:16
@@ +15,3 @@
+  AlsoGood& operator=(AlsoGood);
+};
+

sbenza wrote:
> This is a very common C++98 way of implementing copy-and-swap with copy 
> elision support.
> You do: `T& operator=(T t) { swap(t); return *this; }`
> And it will avoid the copy if the argument is already a temporary due to copy 
> elision on the caller.
I wasn't arguing that it wasn't useful, but this check is also registered as 
cppcoreguidelines-c-copy-assignment-signature, and so we need to make sure that 
we aren't breaking that check. Basically, this can be resolved by looking at 
the spelling of the check and deciding whether to diagnose this particular case 
or not and add appropriate tests. (We do this for a few checks shared with CERT 
rules as well.)


http://reviews.llvm.org/D18265



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

Re: [PATCH] D18649: [clang-tidy] cppcoreguidelines-interfaces-global-init

[Etienne Bergeron via cfe-commits]

etienneb added a subscriber: etienneb.
etienneb added a comment.

some nits.



Comment at: clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.cpp:21
@@ +20,3 @@
+void InterfacesGlobalInitCheck::registerMatchers(MatchFinder *Finder) {
+  auto IsStaticGlobal =
+  allOf(hasGlobalStorage(),

nit: const (and below)


Comment at: test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp:42
@@ +41,3 @@
+void f() {
+  // This is fine, it's executed after dynamic initilization occurs.
+  static int G = takesInt(ExternGlobal);

nit: initilization -> initialization


http://reviews.llvm.org/D18649



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

Re: [PATCH] D18649: [clang-tidy] cppcoreguidelines-interfaces-global-init

[Clement Courbet via cfe-commits]

courbet removed a subscriber: etienneb.
courbet updated this revision to Diff 52366.
courbet marked 2 inline comments as done.
courbet added a comment.

Style fixes.


http://reviews.llvm.org/D18649

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.cpp
  clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-interfaces-global-init.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp

Index: test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp
@@ -0,0 +1,57 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-interfaces-global-init %t
+
+constexpr int makesInt() { return 3; }
+constexpr int takesInt(int i) { return i + 1; }
+constexpr int takesIntPtr(int *i) { return *i; }
+
+extern int ExternGlobal;
+static int GlobalScopeBadInit1 = ExternGlobal;
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with non-const expression depending on static variable 'ExternGlobal'
+static int GlobalScopeBadInit2 = takesInt(ExternGlobal);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with non-const expression depending on static variable 'ExternGlobal'
+static int GlobalScopeBadInit3 = takesIntPtr(&ExternGlobal);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with non-const expression depending on static variable 'ExternGlobal'
+static int GlobalScopeBadInit4 = 3 * (ExternGlobal + 2);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with non-const expression depending on static variable 'ExternGlobal'
+
+namespace ns {
+static int NamespaceScope = makesInt();
+static int NamespaceScopeBadInit = takesInt(ExternGlobal);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with non-const expression depending on static variable 'ExternGlobal'
+
+struct A {
+  static int ClassScope;
+  static int ClassScopeBadInit;
+};
+
+int A::ClassScopeBadInit = takesInt(ExternGlobal);
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: initializing static variable with non-const expression depending on static variable 'ExternGlobal'
+
+static int FromClassBadInit = takesInt(A::ClassScope);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with non-const expression depending on static variable 'ClassScope'
+} // namspace ns
+
+void f() {
+  // This is fine, it's executed after dynamic initilization occurs.
+  static int G = takesInt(ExternGlobal);
+}
+
+// Defined global variables are fine:
+static int GlobalScope = makesInt();
+static int GlobalScopeBadInit = takesInt(GlobalScope);
+static int GlobalScope2 = takesInt(ns::NamespaceScope);
+
+// Enums are fine.
+enum Enum { kEnumValue = 1 };
+static int GlobalScopeFromEnum = takesInt(kEnumValue);
+
+// Leave constexprs alone.
+extern constexpr int GlobalScopeConstexpr = makesInt();
+static constexpr int GlobalScopeConstexprOk =
+takesInt(GlobalScopeConstexpr);
+
+// This is a pretty common instance: People are usually not using constexpr, but
+// this is what they should write:
+static constexpr const char kValue[] = "value";
+constexpr int Fingerprint(const char *value) { return 0; }
+static int kFingerprint = Fingerprint(kValue);
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
cert-fio38-c (redirects to misc-non-copyable-objects) 
cert-flp30-c
cert-oop11-cpp (redirects to misc-move-constructor-init) 
+   cppcoreguidelines-interfaces-global-init
cppcoreguidelines-pro-bounds-array-to-pointer-decay
cppcoreguidelines-pro-bounds-constant-array-index
cppcoreguidelines-pro-bounds-pointer-arithmetic
Index: docs/clang-tidy/checks/cppcoreguidelines-interfaces-global-init.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-interfaces-global-init.rst
@@ -0,0 +1,11 @@
+.. title:: clang-tidy - cppcoreguidelines-interfaces-global-init
+
+cppcoreguidelines-interfaces-global-init
+
+
+This check flags initializers of globals that access extern objects,
+and therefore can lead to order-of-initialization problems.
+
+This rule is part of the "Interfaces" profile of the C++ Core Guidelines, see
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Ri-global-init
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -153,6 +153,13 @@
 
   Finds unnecessary string initializati

Re: [PATCH] D18649: [clang-tidy] cppcoreguidelines-interfaces-global-init

[Clement Courbet via cfe-commits]

courbet added a subscriber: etienneb.
courbet added a comment.

Looks like there's a race condition in phabricator.



Comment at: test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp:37
@@ +36,3 @@
+static int FromClassBadInit = takesInt(A::ClassScope);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with
+// non-const expression depending on static variable 'ClassScope'

alexfh wrote:
> These should be on a single line. I guess, your clang-format doesn't use 
> -style=file by default and doesn't understand the local style configured for 
> the test/ directory. Otherwise it wouldn't split the lines.
Thanks for the hint. I was using -style=LLVM indeed.


http://reviews.llvm.org/D18649



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

Re: [PATCH] D10834: Added functions to retrieve information about whether a vardecl is local in libclang and its python bindings.

[guibufolo+l...@gmail.com via cfe-commits]

RedX2501 updated this revision to Diff 52367.
RedX2501 added a comment.

Fix compilation error introduced due to breaking a long string.
Fixed indentation.


http://reviews.llvm.org/D10834

Files:
  bindings/python/clang/cindex.py
  bindings/python/tests/cindex/test_cursor.py
  include/clang-c/Index.h
  test/Index/islocalvardecl.cpp
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp
  tools/libclang/libclang.exports

Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -33,6 +33,7 @@
 clang_Cursor_isNull
 clang_Cursor_isObjCOptional
 clang_Cursor_isVariadic
+clang_Cursor_isLocalVarDecl
 clang_Cursor_getModule
 clang_Cursor_getStorageClass
 clang_File_isEqual
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -6921,6 +6921,19 @@
   return 0;
 }
 
+unsigned clang_Cursor_isLocalVarDecl(CXCursor C){
+  if (C.kind != CXCursor_VarDecl) {
+return 0;
+  }
+
+  const Decl *D = getCursorDecl(C);
+  if (const VarDecl *VD = dyn_cast(D)) {
+return VD->isLocalVarDecl();
+  }
+
+  return 0;
+}
+
 CXSourceRange clang_Cursor_getCommentRange(CXCursor C) {
   if (!clang_isDeclaration(C.kind))
 return clang_getNullRange();
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -1276,6 +1276,22 @@
 }
 
 /**/
+/* Local VarKind testing. */
+/**/
+
+static enum CXChildVisitResult PrintIsLocalVarDecl(CXCursor C, CXCursor p,
+   CXClientData d){
+
+  if (clang_getCursorKind(C) != CXCursor_VarDecl)
+return CXChildVisit_Recurse;
+
+  PrintCursor(C, NULL);
+  printf(" IsLocalVarDecl=%d\n", clang_Cursor_isLocalVarDecl(C));
+
+  return CXChildVisit_Recurse;
+}
+
+/**/
 /* Typekind testing.  */
 /**/
 
@@ -4150,10 +4166,12 @@
   fprintf(stderr,
 "   c-index-test -test-print-linkage-source {}*\n"
 "   c-index-test -test-print-visibility {}*\n"
+"   c-index-test -test-print-is-local-var {}*\n"
 "   c-index-test -test-print-type {}*\n"
 "   c-index-test -test-print-type-size {}*\n"
 "   c-index-test -test-print-bitwidth {}*\n"
-"   c-index-test -test-print-type-declaration {}*\n"
+"   c-index-test -test-print-type-declaration {}*\n");
+  fprintf(stderr,
 "   c-index-test -print-usr [ {}]*\n"
 "   c-index-test -print-usr-file \n"
 "   c-index-test -write-pch  \n");
@@ -4241,6 +4259,9 @@
   else if (argc > 2 && strcmp(argv[1], "-test-print-visibility") == 0)
 return perform_test_load_source(argc - 2, argv + 2, "all", PrintVisibility,
 NULL);
+  else if (argc > 2 && strcmp(argv[1], "-test-print-is-local-var") == 0)
+return perform_test_load_source(argc - 2, argv + 2, "all", PrintIsLocalVarDecl,
+NULL);
   else if (argc > 2 && strcmp(argv[1], "-test-print-type") == 0)
 return perform_test_load_source(argc - 2, argv + 2, "all",
 PrintType, 0);
Index: test/Index/islocalvardecl.cpp
===
--- /dev/null
+++ test/Index/islocalvardecl.cpp
@@ -0,0 +1,32 @@
+// RUN: c-index-test -test-print-local-var-kind %s | FileCheck %s
+
+extern "C" {
+  int var0;
+  static int var1;
+
+  void func(void){
+static int var2;
+int var3;
+  }
+};
+
+int var4;
+
+class Classy {
+  static int var5;
+
+  void member(){
+int var6;
+static int var7;
+  }
+};
+
+// CHECK: VarDecl=var0:3:5 (Definition) IsLocalVarDecl=0
+// CHECK: VarDecl=var1:4:12 (Definition) IsLocalVarDecl=0
+// CHECK: VarDecl=var2:7:14 (Definition) IsLocalVarDecl=1
+// CHECK: VarDecl=var3:8:7 (Definition) IsLocalVarDecl=1
+// CHECK: VarDecl=var4:13:5 (Definition) IsLocalVarDecl=0
+// CHECK: VarDecl=var5:17:12 IsLocalVarDecl=0
+// CHECK: VarDecl=var6:20:9 (Definition) IsLocalVarDecl=1
+// CHECK: VarDecl=var7:21:16 (Definition) IsLocalVarDecl=1
+
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -3882,6 +3882,11 @@
 CINDEX_LINKAGE unsigned clang_Cursor_isVariadic(CXCursor C);
 
 /**
+ * \brief Returns non-zero if the Cursor refers to a local VarDecl.
+ */
+CINDEX_LINKAGE unsi

Re: [PATCH] D18649: [clang-tidy] cppcoreguidelines-interfaces-global-init

[Etienne Bergeron via cfe-commits]

etienneb added a comment.

more nits, sorry didn't saw them first time.



Comment at: test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp:32
@@ +31,3 @@
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with 
non-const expression depending on static variable 'ClassScope'
+} // namspace ns
+

nit: namspace -> namespace


Comment at: test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp:43
@@ +42,3 @@
+static int GlobalScope2 = takesInt(ns::NamespaceScope);
+
+// Enums are fine.

nit: remove empty line here.


http://reviews.llvm.org/D18649



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

Re: [PATCH] D18695: [clang-tidy] Reduce false-positive ratio in misc-suspicious-missing-comma check.

[Alexander Kornienko via cfe-commits]

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

Looks good!


http://reviews.llvm.org/D18695



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

Re: [PATCH] D18695: [clang-tidy] Reduce false-positive ratio in misc-suspicious-missing-comma check.

[Etienne Bergeron via cfe-commits]

etienneb added a comment.

thanks alex, I'll give a day or two to people to jump in.
If no other comments, I'll land it.


http://reviews.llvm.org/D18695



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

Re: [PATCH] D18649: [clang-tidy] cppcoreguidelines-interfaces-global-init

[Clement Courbet via cfe-commits]

courbet updated this revision to Diff 52370.
courbet marked 3 inline comments as done.
courbet added a comment.

Constness + typos.


http://reviews.llvm.org/D18649

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.cpp
  clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-interfaces-global-init.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp

Index: test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp
@@ -0,0 +1,57 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-interfaces-global-init %t
+
+constexpr int makesInt() { return 3; }
+constexpr int takesInt(int i) { return i + 1; }
+constexpr int takesIntPtr(int *i) { return *i; }
+
+extern int ExternGlobal;
+static int GlobalScopeBadInit1 = ExternGlobal;
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with non-const expression depending on static variable 'ExternGlobal'
+static int GlobalScopeBadInit2 = takesInt(ExternGlobal);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with non-const expression depending on static variable 'ExternGlobal'
+static int GlobalScopeBadInit3 = takesIntPtr(&ExternGlobal);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with non-const expression depending on static variable 'ExternGlobal'
+static int GlobalScopeBadInit4 = 3 * (ExternGlobal + 2);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with non-const expression depending on static variable 'ExternGlobal'
+
+namespace ns {
+static int NamespaceScope = makesInt();
+static int NamespaceScopeBadInit = takesInt(ExternGlobal);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with non-const expression depending on static variable 'ExternGlobal'
+
+struct A {
+  static int ClassScope;
+  static int ClassScopeBadInit;
+};
+
+int A::ClassScopeBadInit = takesInt(ExternGlobal);
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: initializing static variable with non-const expression depending on static variable 'ExternGlobal'
+
+static int FromClassBadInit = takesInt(A::ClassScope);
+// CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing static variable with non-const expression depending on static variable 'ClassScope'
+} // namspace ns
+
+void f() {
+  // This is fine, it's executed after dynamic initialization occurs.
+  static int G = takesInt(ExternGlobal);
+}
+
+// Defined global variables are fine:
+static int GlobalScope = makesInt();
+static int GlobalScopeBadInit = takesInt(GlobalScope);
+static int GlobalScope2 = takesInt(ns::NamespaceScope);
+
+// Enums are fine.
+enum Enum { kEnumValue = 1 };
+static int GlobalScopeFromEnum = takesInt(kEnumValue);
+
+// Leave constexprs alone.
+extern constexpr int GlobalScopeConstexpr = makesInt();
+static constexpr int GlobalScopeConstexprOk =
+takesInt(GlobalScopeConstexpr);
+
+// This is a pretty common instance: People are usually not using constexpr, but
+// this is what they should write:
+static constexpr const char kValue[] = "value";
+constexpr int Fingerprint(const char *value) { return 0; }
+static int kFingerprint = Fingerprint(kValue);
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
cert-fio38-c (redirects to misc-non-copyable-objects) 
cert-flp30-c
cert-oop11-cpp (redirects to misc-move-constructor-init) 
+   cppcoreguidelines-interfaces-global-init
cppcoreguidelines-pro-bounds-array-to-pointer-decay
cppcoreguidelines-pro-bounds-constant-array-index
cppcoreguidelines-pro-bounds-pointer-arithmetic
Index: docs/clang-tidy/checks/cppcoreguidelines-interfaces-global-init.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-interfaces-global-init.rst
@@ -0,0 +1,11 @@
+.. title:: clang-tidy - cppcoreguidelines-interfaces-global-init
+
+cppcoreguidelines-interfaces-global-init
+
+
+This check flags initializers of globals that access extern objects,
+and therefore can lead to order-of-initialization problems.
+
+This rule is part of the "Interfaces" profile of the C++ Core Guidelines, see
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Ri-global-init
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -153,6 +153,13 @@
 
   Finds unnecessary string initializations.
 
+- New `cppcoreguidelines-

Re: [PATCH] D18442: A clang-tidy check for std:accumulate.

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Sorry for the delay (mostly due to the holidays here).

The check looks very useful, at least, the pattern is hard to spot by manual 
inspection. A few comments though, mostly style-related.



Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:21
@@ +20,3 @@
+/// Returns the value_type for an InputIterator type.
+static QualType getInputIteratorValueType(const Type &IteratorType,
+  const ASTContext &context) {

I suspect, a significant part of this could be done in the matcher. I might be 
wrong, but it seems worth trying. The resulting code is usually much shorter 
and cleaner.


Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:57
@@ +56,3 @@
+  //  - a bigger int, regardless of signedness.
+  //  - FIXME: should it be a warning to fold into floating point ?
+  if (ValueType.isInteger()) {

nit: no space needed before the question mark.


Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:101
@@ +100,3 @@
+  getInputIteratorValueType(*Iterator->getType(), *Result.Context);
+  if (ValueType.isNull()) {
+return;

nit: No braces needed for one line `if` bodies. Here and elsewhere.


Comment at: test/clang-tidy/misc-fold-init-type.cpp:17
@@ +16,3 @@
+  return std::accumulate(a, a + 1, 0);
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type
+  // 'int' might result in loss of precision [misc-fold-init-type]

The CHECK lines shouldn't be broken. Looks like your clang-format doesn't use 
-style=file by default.


Comment at: test/clang-tidy/misc-fold-init-type.cpp:24
@@ +23,3 @@
+  return std::accumulate(it, it, 0);
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type
+  // 'int' might result in loss of precision [misc-fold-init-type]

We usually specify each unique message completely once and truncate static 
parts of all other patterns to a make the test more readable. E.g. here I would 
remove everything after 'int'.


http://reviews.llvm.org/D18442



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

Re: [PATCH] D18180: [clang-tidy] Add a check to detect static definitions in anonymous namespace.

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Sorry for the delay. A few minor issues.



Comment at: 
clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp:46
@@ +45,3 @@
+  auto Diag = diag(Def->getLocation(), "'%0' is a static definition in "
+   "anonymous namespace")
+  << Def->getName();

I'd expand the message a bit: "; static is redundant here".


Comment at: 
docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst:8
@@ +7,3 @@
+
+Static is duplicated in this case because anonymous namespace is only visible 
in
+current transform unit.

nit: in _the_ current _translation_ unit.


Comment at: 
test/clang-tidy/readability-static-definition-in-anonymous-namespace.cpp:1
@@ +1,2 @@
+// RUN: %check_clang_tidy %s 
readability-static-definition-in-anonymous-namespace %t
+

Please add a couple of tests for different cases with macros.


Comment at: 
test/clang-tidy/readability-static-definition-in-anonymous-namespace.cpp:9
@@ +8,3 @@
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'c' is a static definition in 
anonymous namespace [readability-static-definition-in-anonymous-namespace]
+// CHECK-FIXES: int c = 1;
+static const int d = 1;

This check is not helpful, it will match `static int c = 1;` as well, since 
it's not anchored to the line start. Should be:
`CHECK-FIXES: {{^}}int c = 1;`. Same in other checks.


Repository:
  rL LLVM

http://reviews.llvm.org/D18180



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

Re: [PATCH] D18694: [ClangTidy] Add an 'explain-checks' option to diagnose where each checks comes from.

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Nice! I like the idea, but we need to figure out a bunch of details (see the 
comments inline).



Comment at: clang-tidy/ClangTidyOptions.cpp:264
@@ +263,3 @@
+  ParsedOptions->CheckSources[*ParsedOptions->Checks] =
+  std::string(ConfigFile.c_str());
+}

`ConfigFile.str()` should also work.


Comment at: clang-tidy/tool/ClangTidyMain.cpp:131
@@ -130,1 +130,3 @@
 
+static cl::opt ExplainChecks("explain-checks", cl::desc(R"(
+explains where each check comes from, i.e. command line, .clang-tidy

I would expect something different from an option named '-explain-checks', e.g. 
some kind of a short documentation on each check or the like. Something like 
`explain-config` or `describe-config` would be closer to what the option does, 
I think.


Comment at: clang-tidy/tool/ClangTidyMain.cpp:132
@@ +131,3 @@
+static cl::opt ExplainChecks("explain-checks", cl::desc(R"(
+explains where each check comes from, i.e. command line, .clang-tidy
+configuration file.

How about `for each enabled check explains, where it is enabled, i.e. in 
clang-tidy binary, command line or a specific configuration file`?


Comment at: clang-tidy/tool/ClangTidyMain.cpp:265
@@ -258,2 +264,3 @@
   DefaultOptions.Checks = DefaultChecks;
+  DefaultOptions.CheckSources[DefaultChecks] = "ClangTidy binary";
   DefaultOptions.WarningsAsErrors = "";

s/ClangTidy/clang-tidy/


Comment at: clang-tidy/tool/ClangTidyMain.cpp:278
@@ -270,1 +277,3 @@
 OverrideOptions.Checks = Checks;
+OverrideOptions.CheckSources[Checks] = "command-line argument 'checks'";
+  }

s/argument 'checks'/option '-checks'/


Comment at: clang-tidy/tool/ClangTidyMain.cpp:294
@@ +293,3 @@
+ParsedConfig->CheckSources[*ParsedConfig->Checks] =
+"command-line argument 'config'";
+  }

s/argument 'config'/option '-config'/


Comment at: clang-tidy/tool/ClangTidyMain.cpp:329
@@ +328,3 @@
+for (const std::string& Check : EnabledChecks) {
+  for (const ClangTidyOptions::StringPair &CheckSource:
+   EffectiveOptions.CheckSources) {

I'm not sure I understand how this should work in case a check is:
1. enabled in binary (e.g. `-*,a`)
2. disabled in the config file (e.g. `-a`)
3. enabled again on the command line (e.g. `a`)

clang-tidy should say 'command line' in this case, but i'm not sure what your 
implementation will do, since it doesn't seem to keep the order of the 
configurations.


Comment at: clang-tidy/tool/ClangTidyMain.cpp:331
@@ +330,3 @@
+   EffectiveOptions.CheckSources) {
+GlobList filter(CheckSource.first);
+if (filter.contains(Check)) {

nit: Filter

You could also write `if (GlobList(CheckSource.first).contains(Check)) {`


Comment at: clang-tidy/tool/ClangTidyMain.cpp:333
@@ +332,3 @@
+if (filter.contains(Check)) {
+  llvm::outs() << "'" << Check << "' comes from " << CheckSource.second
+   << ".\n";

s/comes from/is enabled in the/


Repository:
  rL LLVM

http://reviews.llvm.org/D18694



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

Re: [PATCH] D18369: [OpenCL] Upstreaming khronos OpenCL 1.2/2.0 header files.

[Yaxun Liu via cfe-commits]

yaxunl added a comment.

Anastasia/Alexey/Xiuli,

Do you agree that we should have one single opencl.h instead of headers for 
different OpenCL versions?

Since most 1.2 builtins are included in 2.0. I think this is doable.

If no objection, I will try to merge them into one header first then addressing 
other issues.

Thanks.


http://reviews.llvm.org/D18369



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

Re: [PATCH] D18649: [clang-tidy] cppcoreguidelines-interfaces-global-init

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.cpp:46
@@ +45,3 @@
+   "initializing static variable with non-const expression depending on "
+   "static variable '%0'.")
+  << Referencee->getName();

nit: no trailing period needed in diagnostic messages


Comment at: clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.cpp:47
@@ +46,3 @@
+   "static variable '%0'.")
+  << Referencee->getName();
+}

This might work even better without `->getName()`. Not sure, but it makes sense 
to try.


Comment at: 
docs/clang-tidy/checks/cppcoreguidelines-interfaces-global-init.rst:7
@@ +6,3 @@
+This check flags initializers of globals that access extern objects,
+and therefore can lead to order-of-initialization problems.
+

What about indirect access of other globals, e.g. via function calls? We 
obviously can't analyze functions defined in a different translation unit, and 
I'm not sure we need to analyze even those defined in the same TU (since it may 
be imprecise and quite expensive), but maybe we should use some heuristic here 
(the most agressive would be to flag all initializers containing function 
calls, but that might be rather noisy).


http://reviews.llvm.org/D18649



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

Re: [PATCH] D18369: [OpenCL] Upstreaming khronos OpenCL 1.2/2.0 header files.

[Yaxun Liu via cfe-commits]

yaxunl added a comment.

One of the difference between opencl-12.cl and opencl-20.cl is opencl-12.cl 
defines

  #define const_func __attribute__((const))
  #define readonly __attribute__((pure))

and uses them for many functions, e.g.

  float const_func __attribute__((overloadable)) acos(float);

I think this is a nice feature for performance. However surprisingly 
opencl-20.cl does not do that.

I suggest to keep these attributes in the merged file. What do you think? 
Thanks.


http://reviews.llvm.org/D18369



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

Re: [PATCH] D18110: [OpenMP] Fix SEMA bug in the capture of global variables in template functions.

[Samuel Antao via cfe-commits]

sfantao added a comment.

Hi Alexey,



Comment at: lib/Sema/SemaOpenMP.cpp:816-822
@@ -801,6 +815,9 @@
+
+  // A DSA refers to this captured region if the parent contexts match.
+  auto *ParentContext = RSI->TheCapturedDecl->getParent();
   for (auto I = Stack.rbegin(), EE = Stack.rend(); I != EE; ++I)
-if (I->CurScope == S)
+if (I->ParentDeclContext == ParentContext)
   return I->Directive;
   return OMPD_unknown;
 }
 

ABataev wrote:
> sfantao wrote:
> > ABataev wrote:
> > > Actually, I think, we need to rework the whole IsOpenMPCapturedByRef() 
> > > function. I'm not sure that it is even required. It looks like some 
> > > optimization hack in the frontend. I'm against any not-necessary 
> > > optimizations in frontend. If scalar value is a firstprivate, it must be 
> > > handled in codegen, not by handling it by copy.
> > 'IsOpenMPCapturedByRef' goal is to change the signature of the outlined 
> > function. We don't want to have reference types in target region arguments 
> > unless they are really required. We have seen performance being greatly 
> > affected just because of that. Of course a consequence of this is to have 
> > variables that become first private.
> > 
> > The current implementation of OpenMP firstprivate doesn't change the the 
> > function signatures, only the codegen inside the region is affected. So, 
> > this is not a complete overlap.
> > 
> > With this, I am not saying that this cannot be done in codegen. The reason 
> > I am doing it here is your initial guideline that we should attempt to fix 
> > most of the things in Sema and avoid complicating codegen which is a more 
> > sensitive part.
> > 
> > Doing this in Codegen would require changing the implementation of 
> > `GenerateOpenMPCapturedVars` and `GenerateOpenMPCapturedStmtFunction`. We 
> > wouldn't need the tracking of the context anymore (checking the directive 
> > would be sufficient), but we would still need to see what't in the map 
> > clauses.
> > 
> > So, do you think we should change this?
> I think we should not dot it at all for now. Performance is an important 
> thing, of course, but at first we must have just working solution and only 
> after that we will think about performance.
> Early optimization leads to many troubles. The code becomes very hard to 
> understand. It just increases the time of the development.
> I suggest to remove all optimizations for now and emit the code as is, before 
> we have a working solution.
> Also, this function may lead to incorrect codegen. You don't check that 
> CapturedRegion is actually an OpenMP region. If we have non-OpenMP cpatured 
> region between OpenMP regions it will lead to incorrect code.
I don't agree. As you know, several components on the codegen depend on how 
things are captured. So in my view, identifying the best possible 
implementation (including performance-wise) as early as possible is in our best 
interest to avoid major refactoring down the road.

All the offloading implementation that we have working today in the trunk is 
relying on this already. So, by rolling back to capture by reference 
everything, will require to refactor all the offloading code/patches/regression 
tests just to have to redo them again in the near future because of the issues 
we have identified already. 

I'd rather spend that effort to have the capture by copy to work in a 
acceptable way and I can move this logic to codegen if you think that is the 
best way to do it. Note that will require checking the expressions of the map 
clause in `GenerateOpenMPCapturedVars` and 
`GenerateOpenMPCapturedStmtFunction`, something that Sema will have to do 
anyway.

Let me know your thoughts. 




http://reviews.llvm.org/D18110



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

Re: [PATCH] D18136: boost-use-to-string check

[Piotr Padlewski via cfe-commits]

Prazek updated the summary for this revision.
Prazek updated this revision to Diff 52378.
Prazek added a comment.

Updated ReleaseNotes and also fixed bug.
After lgtm please lgtm also this http://reviews.llvm.org/D18274 because I want 
to send them together, but in separate commits.


http://reviews.llvm.org/D18136

Files:
  clang-tidy/boost/BoostTidyModule.cpp
  clang-tidy/boost/CMakeLists.txt
  clang-tidy/boost/UseToStringCheck.cpp
  clang-tidy/boost/UseToStringCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/boost-use-to-string.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/boost-use-to-string.cpp

Index: test/clang-tidy/boost-use-to-string.cpp
===
--- /dev/null
+++ test/clang-tidy/boost-use-to-string.cpp
@@ -0,0 +1,146 @@
+// RUN: %check_clang_tidy %s boost-use-to-string %t
+
+
+namespace std {
+
+template  class basic_string {};
+
+using string = basic_string;
+using wstring = basic_string;
+}
+
+namespace boost {
+template 
+T lexical_cast(const V&) {
+  return T();
+};
+}
+
+struct my_weird_type {};
+
+std::string fun(const std::string &) {}
+
+void test_to_string1() {
+
+  auto xa = boost::lexical_cast(5);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::to_string instead of boost::lexical_cast [boost-use-to-string]
+// CHECK-FIXES: auto xa = std::to_string(5);
+
+ auto z = boost::lexical_cast(42LL);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use std::to_string {{..}}
+// CHECK-FIXES: auto z = std::to_string(42LL);
+
+  // this should not trigger
+  fun(boost::lexical_cast(42.0));
+  auto non = boost::lexical_cast(42);
+  boost::lexical_cast("12");
+}
+
+void test_to_string2() {
+  int a;
+  long b;
+  long long c;
+  unsigned d;
+  unsigned long e;
+  unsigned long long f;
+  float g;
+  double h;
+  long double i;
+  bool j;
+
+  fun(boost::lexical_cast(a));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::to_string {{..}}
+// CHECK-FIXES: fun(std::to_string(a));
+  fun(boost::lexical_cast(b));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::to_string {{..}}
+// CHECK-FIXES: fun(std::to_string(b));
+  fun(boost::lexical_cast(c));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::to_string {{..}}
+// CHECK-FIXES: fun(std::to_string(c));
+  fun(boost::lexical_cast(d));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::to_string {{..}}
+// CHECK-FIXES: fun(std::to_string(d));
+  fun(boost::lexical_cast(e));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::to_string {{..}}
+// CHECK-FIXES: fun(std::to_string(e));
+  fun(boost::lexical_cast(f));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::to_string {{..}}
+// CHECK-FIXES: fun(std::to_string(f));
+
+  // No change for floating numbers.
+  fun(boost::lexical_cast(g));
+  fun(boost::lexical_cast(h));
+  fun(boost::lexical_cast(i));
+  // And bool.
+  fun(boost::lexical_cast(j));
+}
+
+std::string fun(const std::wstring &) {}
+
+void test_to_wstring() {
+  int a;
+  long b;
+  long long c;
+  unsigned d;
+  unsigned long e;
+  unsigned long long f;
+  float g;
+  double h;
+  long double i;
+  bool j;
+
+  fun(boost::lexical_cast(a));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::to_wstring instead of boost::lexical_cast [boost-use-to-string]
+// CHECK-FIXES: fun(std::to_wstring(a));
+  fun(boost::lexical_cast(b));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::to_wstring {{..}}
+// CHECK-FIXES: fun(std::to_wstring(b));
+  fun(boost::lexical_cast(c));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::to_wstring {{..}}
+// CHECK-FIXES: fun(std::to_wstring(c));
+  fun(boost::lexical_cast(d));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::to_wstring {{..}}
+// CHECK-FIXES: fun(std::to_wstring(d));
+  fun(boost::lexical_cast(e));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::to_wstring {{..}}
+// CHECK-FIXES: fun(std::to_wstring(e));
+  fun(boost::lexical_cast(f));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::to_wstring {{..}}
+// CHECK-FIXES: fun(std::to_wstring(f));
+
+  // No change for floating numbers
+  fun(boost::lexical_cast(g));
+  fun(boost::lexical_cast(h));
+  fun(boost::lexical_cast(i));
+  // and bool.
+  fun(boost::lexical_cast(j));
+}
+
+const auto glob = boost::lexical_cast(42);
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use std::to_string{{..}}
+// CHECK-FIXES: const auto glob = std::to_string(42);
+
+template 
+void string_as_T(T t = T()) {
+  boost::lexical_cast(42);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use std::to_string{{..}}
+// CHECK-FIXES: std::to_string(42);
+
+  boost::lexical_cast(42);
+  string_as_T(boost::lexical_cast(42));
+  auto p = boost::lexical_cast(42);
+  auto p2 = (T)boost::lexical_cast(42);
+  auto p3 = static_cast(boost::lexical_cast(42));
+}
+
+
+void no_warnings() {
+  fun(boost::lexical_cast("abc"));
+  fun(boost::lexical_cast("abc"));
+  fun(boost::lexical_cast(my_weird_type{}));
+  string_as_T();
+  string_as_T();
+}
+
+
+
Index: docs/cla

Re: [PATCH] D18136: boost-use-to-string check

[Piotr Padlewski via cfe-commits]

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

http://reviews.llvm.org/D18136



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

r265146 - [OpenCL] Moved nosvm attribute handling in Sema to other OpenCL attrs

[Anastasia Stulova via cfe-commits]

Author: stulova
Date: Fri Apr  1 11:05:09 2016
New Revision: 265146

URL: http://llvm.org/viewvc/llvm-project?rev=265146&view=rev
Log:
[OpenCL] Moved nosvm attribute handling in Sema to other OpenCL attrs


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

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=265146&r1=265145&r2=265146&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Apr  1 11:05:09 2016
@@ -5706,6 +5706,9 @@ static void ProcessDeclAttribute(Sema &S
   case AttributeList::AT_OpenCLAccess:
 handleOpenCLAccessAttr(S, D, Attr);
 break;
+  case AttributeList::AT_OpenCLNoSVM:
+handleOpenCLNoSVMAttr(S, D, Attr);
+break;
   case AttributeList::AT_SwiftContext:
 handleParameterABIAttr(S, D, Attr, ParameterABI::SwiftContext);
 break;
@@ -5715,9 +5718,6 @@ static void ProcessDeclAttribute(Sema &S
   case AttributeList::AT_SwiftIndirectResult:
 handleParameterABIAttr(S, D, Attr, ParameterABI::SwiftIndirectResult);
 break;
-  case AttributeList::AT_OpenCLNoSVM:
-handleOpenCLNoSVMAttr(S, D, Attr);
-break;
   case AttributeList::AT_InternalLinkage:
 handleInternalLinkageAttr(S, D, Attr);
 break;


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

Re: [PATCH] D18110: [OpenMP] Fix SEMA bug in the capture of global variables in template functions.

[Carlo Bertolli via cfe-commits]

carlo.bertolli added a comment.

Hi

If I understand correctly the problem, I would like to add something on top of 
Samuel's comment.
My understanding is that Alexey is suggesting that we pass a reference type to 
kernels for every pointer input parameter, regardless of the actual type and 
data sharing attribute of the related variable. Ignore the remained of this 
comment if this is not the case.

In my viewpoint, this violates the basic logic for which target firstprivate 
was introduced in the OpenMP specification: to avoid having to pass to GPU 
kernels a reference to every pointer variable. In fact, having to pass a 
reference results in ptxas generating an additional register for each input 
parameter. We saw significant performance differences in this respect, and 
reported about this in a paper at SC15. This has been reported by various 
members of the OpenMP committee multiple times as the reason why the target 
firstprivate logic is defined the way it is.

In conclusion, I do not see this patch as an optimization, but as a way of 
correctly implementing what the OpenMP specification clearly state. Any other 
implementation is wrong, not a simpler, non optimized version.


http://reviews.llvm.org/D18110



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

Re: [PATCH] D18196: [CodeGen] Emit lifetime.end intrinsic after destructor call in landing pad

[John McCall via cfe-commits]

rjmccall added a comment.

LGTM.


http://reviews.llvm.org/D18196



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

Re: [PATCH] D18567: Block: Fix a crash when we have type attributes or qualifiers with omitted return type.

[Anastasia Stulova via cfe-commits]

Anastasia added inline comments.


Comment at: test/SemaOpenCL/invalid-block.cl:8-9
@@ -8,4 +7,4 @@
+  int (^bl1)() = ^() {return 1;}; // expected-error{{invalid block variable 
declaration - must be const qualified}}
   int (^const bl2)(); // expected-error{{invalid block variable declaration - 
must be initialized}}
-  int (^const bl3)() = ^const(){
-  };
+  int (^const bl3)() = ^(){return 1;};
 }

manmanren wrote:
> Yes, John is right. The DependentTy is never replaced if wrapped with an 
> Attributed type or a qualified type.
Ah, I see. And now the error occurs because you are invalidating/removing the 
attributes?


http://reviews.llvm.org/D18567



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

[PATCH] D18698: [C11] PR24451: Allow restrict _Atomic pointers

[Denis Zobnin via cfe-commits]

d.zobnin.bugzilla created this revision.
d.zobnin.bugzilla added a reviewer: rsmith.
d.zobnin.bugzilla added a subscriber: cfe-commits.

Treat _Atomic pointers as pointer types (check value type of AtomicType) in 
case of handling the "restrict" qualifier. Accept such code (so does GCC):

int * restrict _Atomic p;

http://reviews.llvm.org/D18698

Files:
  lib/Sema/SemaType.cpp
  test/Misc/ast-dump-restrict-atomic.c

Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1663,12 +1663,18 @@
   if (T.isNull())
 return QualType();
 
+  QualType OriginalType = T;
+
   // Enforce C99 6.7.3p2: "Types other than pointer types derived from
   // object or incomplete types shall not be restrict-qualified."
   if (Qs.hasRestrict()) {
 unsigned DiagID = 0;
 QualType ProblemTy;
 
+// Handle code like "int * _Atomic restrict p".
+if (const AtomicType *AtomicT = T->getAs())
+  T = AtomicT->getValueType();
+
 if (T->isAnyPointerType() || T->isReferenceType() ||
 T->isMemberPointerType()) {
   QualType EltTy;
@@ -1696,7 +1702,7 @@
 }
   }
 
-  return Context.getQualifiedType(T, Qs);
+  return Context.getQualifiedType(OriginalType, Qs);
 }
 
 QualType Sema::BuildQualifiedType(QualType T, SourceLocation Loc,
Index: test/Misc/ast-dump-restrict-atomic.c
===
--- test/Misc/ast-dump-restrict-atomic.c
+++ test/Misc/ast-dump-restrict-atomic.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c11 -ast-dump %s | FileCheck %s
+// RUN: %clang_cc1 -std=c11 -verify %s
+// expected-no-diagnostics
+
+int * _Atomic restrict a;
+int * restrict _Atomic b;
+int _Atomic * restrict c;
+volatile _Atomic int * restrict _Atomic const * restrict d;
+
+// CHECK: a '__restrict _Atomic(int *)'
+// CHECK: b '__restrict _Atomic(int *)'
+// CHECK: c '_Atomic(int) *__restrict'
+// CHECK: d 'const __restrict _Atomic(volatile _Atomic(int) *) *__restrict'


Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1663,12 +1663,18 @@
   if (T.isNull())
 return QualType();
 
+  QualType OriginalType = T;
+
   // Enforce C99 6.7.3p2: "Types other than pointer types derived from
   // object or incomplete types shall not be restrict-qualified."
   if (Qs.hasRestrict()) {
 unsigned DiagID = 0;
 QualType ProblemTy;
 
+// Handle code like "int * _Atomic restrict p".
+if (const AtomicType *AtomicT = T->getAs())
+  T = AtomicT->getValueType();
+
 if (T->isAnyPointerType() || T->isReferenceType() ||
 T->isMemberPointerType()) {
   QualType EltTy;
@@ -1696,7 +1702,7 @@
 }
   }
 
-  return Context.getQualifiedType(T, Qs);
+  return Context.getQualifiedType(OriginalType, Qs);
 }
 
 QualType Sema::BuildQualifiedType(QualType T, SourceLocation Loc,
Index: test/Misc/ast-dump-restrict-atomic.c
===
--- test/Misc/ast-dump-restrict-atomic.c
+++ test/Misc/ast-dump-restrict-atomic.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c11 -ast-dump %s | FileCheck %s
+// RUN: %clang_cc1 -std=c11 -verify %s
+// expected-no-diagnostics
+
+int * _Atomic restrict a;
+int * restrict _Atomic b;
+int _Atomic * restrict c;
+volatile _Atomic int * restrict _Atomic const * restrict d;
+
+// CHECK: a '__restrict _Atomic(int *)'
+// CHECK: b '__restrict _Atomic(int *)'
+// CHECK: c '_Atomic(int) *__restrict'
+// CHECK: d 'const __restrict _Atomic(volatile _Atomic(int) *) *__restrict'
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18369: [OpenCL] Upstreaming khronos OpenCL 1.2/2.0 header files.

[Anastasia Stulova via cfe-commits]

Anastasia added a comment.

In http://reviews.llvm.org/D18369#389469, @yaxunl wrote:

> One of the difference between opencl-12.cl and opencl-20.cl is opencl-12.cl 
> defines
>
>   #define const_func __attribute__((const))
>   #define readonly __attribute__((pure))
>   
>
> and uses them for many functions, e.g.
>
>   float const_func __attribute__((overloadable)) acos(float);
>   
>
> I think this is a nice feature for performance. However surprisingly 
> opencl-20.cl does not do that.
>
> I suggest to keep these attributes in the merged file. What do you think? 
> Thanks.


I think it's OK to have them them in. But if we keep this, could we use "__" 
prefix to avoid possible clashes with the custom code identifiers (the same 
should apply to any other identifiers which we declare in this header that is 
not specified in Spec).

> Do you agree that we should have one single opencl.h instead of headers for 
> different OpenCL versions?


The only concern about having one header would be its parsing speed due to 
large size. But I believe that the proportion of CL1.2 and CL2.0 declarations 
isn't really large. So perhaps we won't gain much from separating into multiple.

> >   What about OpenCL 1.1 header? Ideally it would be nice to have them in 
> > too!

> 




> We can add it later after we done with 1.2 and 2.0 headers.


Since CL1.2 and CL2.0 are derived from CL1.1, it would make sense to fix the 
declarations (header) for CL1.1 too.


http://reviews.llvm.org/D18369



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

[PATCH] D18700: [Inline asm][GCC compatibility] Handle %v-prefixed code in inline assembly

[Denis Zobnin via cfe-commits]

d.zobnin.bugzilla created this revision.
d.zobnin.bugzilla added a reviewer: echristo.
d.zobnin.bugzilla added a subscriber: cfe-commits.

Handle an inline assembly feature of GCC: code prefixed with "%v", e. g. 
"%vpcmpestri" is transformed into "vpcmpestri" instruction if target supports 
AVX and into "pcmpestri" otherwise.

Given the code:
```
void f(void* arg)
{
  __asm__ ("%vpcmpestri $0, (%1), %2"
   : "=c"(arg)
   : "r"(arg), "x"(arg));
}
```

"gcc -c test.c -o test.o" produces

```
movq   -0x10(%rbp),%xmm0
pcmpestri $0x0,(%rax),%xmm0
```

While "gcc -c -march=corei7-avx test.c -o test.o" produces

```
vmovq  %rdx,%xmm0
vpcmpestri $0x0,(%rax),%xmm0
```

http://reviews.llvm.org/D18700

Files:
  lib/AST/Stmt.cpp
  test/CodeGen/avx-inline-asm.c

Index: lib/AST/Stmt.cpp
===
--- lib/AST/Stmt.cpp
+++ lib/AST/Stmt.cpp
@@ -632,6 +632,14 @@
 
   CurPtr = NameEnd+1;
   continue;
+} else if (*Begin == 'v') {
+  // GCC accepts code staring with "%v", e. g. "%vpcmpestri" and transforms
+  // it into "vpcmpestri" instruction if target processor supports AVX and
+  // into "pcmpestri" otherwise.
+  if (C.getTargetInfo().hasFeature("avx"))
+CurStringPiece = "v" + CurStringPiece;
+  CurStringPiece += EscapedChar;
+  continue;
 }
 
 DiagOffs = CurPtr-StrStart-1;
Index: test/CodeGen/avx-inline-asm.c
===
--- test/CodeGen/avx-inline-asm.c
+++ test/CodeGen/avx-inline-asm.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -target-feature +avx 
-emit-llvm -S %s -o - | FileCheck %s -check-prefix=CHECK-AVX
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -emit-llvm -S %s -o - | 
FileCheck %s
+
+void f(void* arg)
+{
+  __asm__ ("%vpcmpestri $0, (%1), %2"
+   : "=c"(arg)
+   : "r"(arg), "x"(arg));
+
+  // CHECK: pcmpestri
+  // CHECK-AVX: vpcmpestri
+}


Index: lib/AST/Stmt.cpp
===
--- lib/AST/Stmt.cpp
+++ lib/AST/Stmt.cpp
@@ -632,6 +632,14 @@
 
   CurPtr = NameEnd+1;
   continue;
+} else if (*Begin == 'v') {
+  // GCC accepts code staring with "%v", e. g. "%vpcmpestri" and transforms
+  // it into "vpcmpestri" instruction if target processor supports AVX and
+  // into "pcmpestri" otherwise.
+  if (C.getTargetInfo().hasFeature("avx"))
+CurStringPiece = "v" + CurStringPiece;
+  CurStringPiece += EscapedChar;
+  continue;
 }
 
 DiagOffs = CurPtr-StrStart-1;
Index: test/CodeGen/avx-inline-asm.c
===
--- test/CodeGen/avx-inline-asm.c
+++ test/CodeGen/avx-inline-asm.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -target-feature +avx -emit-llvm -S %s -o - | FileCheck %s -check-prefix=CHECK-AVX
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -emit-llvm -S %s -o - | FileCheck %s
+
+void f(void* arg)
+{
+  __asm__ ("%vpcmpestri $0, (%1), %2"
+   : "=c"(arg)
+   : "r"(arg), "x"(arg));
+
+  // CHECK: pcmpestri
+  // CHECK-AVX: vpcmpestri
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r265162 - [CrashReproducer] Add -fmodule-cache-path to reproducer script

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Fri Apr  1 12:39:08 2016
New Revision: 265162

URL: http://llvm.org/viewvc/llvm-project?rev=265162&view=rev
Log:
[CrashReproducer] Add -fmodule-cache-path to reproducer script

The cc1 invocation in the reproducer script should contain a valid path in
-fmodule-cache-path; for that reuse ".cache/module" dir we already
use to dump the vfs and modules.

Modified:
cfe/trunk/lib/Driver/Job.cpp
cfe/trunk/test/Modules/crash-vfs-path-emptydir-entries.m
cfe/trunk/test/Modules/crash-vfs-path-symlink-component.m
cfe/trunk/test/Modules/crash-vfs-path-symlink-topheader.m
cfe/trunk/test/Modules/crash-vfs-path-traversal.m
cfe/trunk/test/Modules/crash-vfs-relative-overlay.m

Modified: cfe/trunk/lib/Driver/Job.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Job.cpp?rev=265162&r1=265161&r2=265162&view=diff
==
--- cfe/trunk/lib/Driver/Job.cpp (original)
+++ cfe/trunk/lib/Driver/Job.cpp Fri Apr  1 12:39:08 2016
@@ -18,6 +18,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -194,6 +195,18 @@ void Command::Print(raw_ostream &OS, con
 printArg(OS, "-ivfsoverlay", Quote);
 OS << ' ';
 printArg(OS, CrashInfo->VFSPath.str().c_str(), Quote);
+
+// Insert -fmodules-cache-path and use the relative module directory
+// .cache/vfs/modules where we already dumped the modules.
+SmallString<128> RelModCacheDir = llvm::sys::path::parent_path(
+llvm::sys::path::parent_path(CrashInfo->VFSPath));
+llvm::sys::path::append(RelModCacheDir, "modules");
+
+std::string ModCachePath = "-fmodules-cache-path=";
+ModCachePath.append(RelModCacheDir.c_str());
+
+OS << ' ';
+printArg(OS, ModCachePath.c_str(), Quote);
   }
 
   if (ResponseFile != nullptr) {

Modified: cfe/trunk/test/Modules/crash-vfs-path-emptydir-entries.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/crash-vfs-path-emptydir-entries.m?rev=265162&r1=265161&r2=265162&view=diff
==
--- cfe/trunk/test/Modules/crash-vfs-path-emptydir-entries.m (original)
+++ cfe/trunk/test/Modules/crash-vfs-path-emptydir-entries.m Fri Apr  1 
12:39:08 2016
@@ -37,6 +37,7 @@
 // CHECKSH-NOT: "-fmodules-cache-path="
 // CHECKSH: "crash-vfs-{{[^ ]*}}.m"
 // CHECKSH: "-ivfsoverlay" "crash-vfs-{{[^ ]*}}.cache/vfs/vfs.yaml"
+// CHECKSH: "-fmodules-cache-path=crash-vfs-{{[^ ]*}}.cache/modules"
 
 // CHECKYAML: 'type': 'directory',
 // CHECKYAML: 'name': "",

Modified: cfe/trunk/test/Modules/crash-vfs-path-symlink-component.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/crash-vfs-path-symlink-component.m?rev=265162&r1=265161&r2=265162&view=diff
==
--- cfe/trunk/test/Modules/crash-vfs-path-symlink-component.m (original)
+++ cfe/trunk/test/Modules/crash-vfs-path-symlink-component.m Fri Apr  1 
12:39:08 2016
@@ -38,6 +38,7 @@
 // CHECKSH-NOT: "-fmodules-cache-path="
 // CHECKSH: "crash-vfs-{{[^ ]*}}.m"
 // CHECKSH: "-ivfsoverlay" "crash-vfs-{{[^ ]*}}.cache/vfs/vfs.yaml"
+// CHECKSH: "-fmodules-cache-path=crash-vfs-{{[^ ]*}}.cache/modules"
 
 // CHECKYAML: 'type': 'directory'
 // CHECKYAML: 'name': "/[[PATH:.*]]/i/usr/include",

Modified: cfe/trunk/test/Modules/crash-vfs-path-symlink-topheader.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/crash-vfs-path-symlink-topheader.m?rev=265162&r1=265161&r2=265162&view=diff
==
--- cfe/trunk/test/Modules/crash-vfs-path-symlink-topheader.m (original)
+++ cfe/trunk/test/Modules/crash-vfs-path-symlink-topheader.m Fri Apr  1 
12:39:08 2016
@@ -39,6 +39,7 @@
 // CHECKSH-NOT: "-fmodules-cache-path="
 // CHECKSH: "crash-vfs-{{[^ ]*}}.m"
 // CHECKSH: "-ivfsoverlay" "crash-vfs-{{[^ ]*}}.cache/vfs/vfs.yaml"
+// CHECKSH: "-fmodules-cache-path=crash-vfs-{{[^ ]*}}.cache/modules"
 
 // CHECKYAML: 'type': 'directory',
 // CHECKYAML: 'name': "",

Modified: cfe/trunk/test/Modules/crash-vfs-path-traversal.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/crash-vfs-path-traversal.m?rev=265162&r1=265161&r2=265162&view=diff
==
--- cfe/trunk/test/Modules/crash-vfs-path-traversal.m (original)
+++ cfe/trunk/test/Modules/crash-vfs-path-traversal.m Fri Apr  1 12:39:08 2016
@@ -35,6 +35,7 @@
 // CHECKSH-NOT: "-fmodules-cache-path="
 // CHECKSH: "crash-vfs-{{[^ ]*}}.m"
 // CHECKSH: "-ivfsoverlay" "crash-vfs-{{[^ ]*}}.cache/vfs/vfs.yaml"
+// CHECKSH: "-fmodules-cache-path=crash-vfs-{{[^ ]*}}.cache/modules"
 
 // CHECKYAML: 'type': 'directory'
 // CHECKYAML: 'name':

Re: [PATCH] D18565: Implement an "I'm dtrace, please retain all debug types" option.

[Adrian Prantl via cfe-commits]

> On Mar 31, 2016, at 5:35 PM, David Blaikie  wrote:
> 
> 
> 
> On Wed, Mar 30, 2016 at 10:49 AM, Adrian Prantl  > wrote:
> 
>> On Mar 29, 2016, at 10:06 PM, David Blaikie > > wrote:
>> 
>> 
>> 
>> On Tue, Mar 29, 2016 at 12:03 PM, Adrian Prantl via cfe-commits 
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> 
>> > On Mar 29, 2016, at 12:00 PM, Joerg Sonnenberger > > > wrote:
>> >
>> > On Tue, Mar 29, 2016 at 06:47:24PM +, Adrian Prantl via cfe-commits 
>> > wrote:
>> >> This code in this patch listens to the driver option -gfull, and lowers 
>> >> it to the new cc1 option -debug-retain-types (1).
>> >> When -debug-retain-types is present, CGDebugInfo will retain every(2) 
>> >> type it creates.
>> >
>> > Is there a good reason for calling it -gfull? I would find something
>> > -gall-types or -gretain-all-types to make a lot more sense. This should
>> > be orthogonal to other options like providing only line tables?
>> 
>> My thinking was this:
>> The driver already supports -gfull, but it doesn’t do anything.
>> This patch can be considered a first step towards making -gfull behave as 
>> expected.
>> Eventually it should emit debug info for *all* types.
>> 
>> Seems somewhat problematic to half implement it, though. (admittedly we're 
>> just silently ignoring it right now)
> 
> I don’t think this is problematic at all. This is incremental development.
> 
> It strikes me as a strange increment. Implementing full -gfull doesn't seem 
> like it would take much time to implement, etc.
>> 
>> & is 'real' -gfull what dtrace really wants? (seems it isn't - since clang's 
>> never really implemented it?)
> 
> Admitted, ‘real' -gfull is probably more than it absolutely needs.
> 
> If just retaining referenced types is all it needs, yeah, it seems -gfull 
> would be rather beyond that.
>  
> 
>> Emitting all types referenced by used (even if later optimized away) code 
>> seems like the thing? -greferenced? or maybe a -f flag? Not sure.
> 
> I don’t see a compelling case for adding another driver option to the already 
> confusing zoo of existing driver options.
> 
> My point is I think a -gfull that does something other than full would 
> possibly be more confusing than not.

Point taken. Let’s surface this under a separate option instead. We can call it 
“-greferenced” to fit between -gfull and -gused.

thanks,
adrian
>  
> Note that we currently also accept a -gused option which according to the 
> driver code is supposed to be the opposite of -gfull.
> 
> Are -gused/-gfull meant to toggle each other?
> 
> Huh, seems they're not general GCC flags, they're Darwin things - I didn't 
> know that.
> 
> Looks like GCC usually spells this -f[no-]eliminate-unused-debug-types. But 
> doesn't seem to have an intermediate version that would be what you're going 
> for.
>  
> Adding a -greferenced option IMO will only make this more confusion instead 
> of helping.
> My suggestion is to have -gfull (also) activate -debug-retain-types. In the 
> somewhat hypothetical scenario that someone implements a more comprehensive 
> version of -gfull we should revisit this and analyze whether the resulting 
> debug information is really too large to be practical, and if we conclude 
> that this is a problem, we can still decide to expose -debug-retain-types to 
> the driver with a new separate option.
> 
> I would be concerned about breaking other existing users that may grow once 
> we support the flag. (& perhaps inconsistency between GCC and Clang, but 
> inconsistency already exists there of course)


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

[PATCH] D18703: [clang-tidy] Add new checker for comparison with runtime string functions.

[Etienne Bergeron via cfe-commits]

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

This checker is validating suspicious usage of string compare functions.

Example:
```
  if (strcmp(...))   // Implicitly compare to zero
  if (!strcmp(...))  // Won't warn
  if (strcmp(...) != 0)  // Won't warn
```

This patch was checker over large amount of code.
There is three checks:
  [*] Implicit comparator to zero (coding-style, many warnings found),
  [*] Suspicious implicit cast to non-integral (bugs!?, almost none found),
  [*] Comparison to suspicious constant (bugs!?, found two cases),

Example:
[[https://github.com/kylepjohnson/sigma/blob/master/sigma/native-installers/debian/dependencies/files/opt/sigma/E/HEURISTICS/che_to_precgen.c
 |
https://github.com/kylepjohnson/sigma/blob/master/sigma/native-installers/debian/dependencies/files/opt/sigma/E/HEURISTICS/che_to_precgen.c]]

```
  else if(strcmp(id, "select") == 0)
  {
 array->array[i].key1 = 25;
  }
  else if(strcmp(id, "sk") == 28)  // BUG!?
  {
 array->array[i].key1 = 20;
  }
```

http://reviews.llvm.org/D18703

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/SuspiciousStringCompareCheck.cpp
  clang-tidy/misc/SuspiciousStringCompareCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-suspicious-string-compare.rst
  test/clang-tidy/misc-suspicious-string-compare.cpp

Index: test/clang-tidy/misc-suspicious-string-compare.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-suspicious-string-compare.cpp
@@ -0,0 +1,298 @@
+// RUN: %check_clang_tidy %s misc-suspicious-string-compare %t
+
+typedef __SIZE_TYPE__ size;
+
+struct locale_t {
+  void* dummy;
+} locale;
+
+static const char A[] = "abc";
+static const unsigned char U[] = "abc";
+static const unsigned char V[] = "xyz";
+static const wchar_t W[] = L"abc";
+
+int memcmp(const void *, const void *, size);
+int wmemcmp(const wchar_t *, const wchar_t *, size);
+int memicmp(const void *, const void *, size);
+int _memicmp(const void *, const void *, size);
+int _memicmp_l(const void *, const void *, size, locale_t);
+
+int strcmp(const char *, const char *);
+int strncmp(const char *, const char *, size);
+int strcasecmp(const char *, const char *);
+int strncasecmp(const char *, const char *, size);
+int stricmp(const char *, const char *);
+int strcmpi(const char *, const char *);
+int strnicmp(const char *, const char *, size);
+int _stricmp(const char *, const char * );
+int _strnicmp(const char *, const char *, size);
+int _stricmp_l(const char *, const char *, locale_t);
+int _strnicmp_l(const char *, const char *, size, locale_t);
+
+int wcscmp(const wchar_t *, const wchar_t *);
+int wcsncmp(const wchar_t *, const wchar_t *, size);
+int wcscasecmp(const wchar_t *, const wchar_t *);
+int wcsicmp(const wchar_t *, const wchar_t *);
+int wcsnicmp(const wchar_t *, const wchar_t *, size);
+int _wcsicmp(const wchar_t *, const wchar_t *);
+int _wcsnicmp(const wchar_t *, const wchar_t *, size);
+int _wcsicmp_l(const wchar_t *, const wchar_t *, locale_t);
+int _wcsnicmp_l(const wchar_t *, const wchar_t *, size, locale_t);
+
+int _mbscmp(const unsigned char *, const unsigned char *);
+int _mbsncmp(const unsigned char *, const unsigned char *, size);
+int _mbsnbcmp(const unsigned char *, const unsigned char *, size);
+int _mbsnbicmp(const unsigned char *, const unsigned char *, size);
+int _mbsicmp(const unsigned char *, const unsigned char *);
+int _mbsnicmp(const unsigned char *, const unsigned char *, size);
+int _mbscmp_l(const unsigned char *, const unsigned char *, locale_t);
+int _mbsncmp_l(const unsigned char *, const unsigned char *, size, locale_t);
+int _mbsicmp_l(const unsigned char *, const unsigned char *, locale_t);
+int _mbsnicmp_l(const unsigned char *, const unsigned char *, size, locale_t);
+int _mbsnbcmp_l(const unsigned char *, const unsigned char *, size, locale_t);
+int _mbsnbicmp_l(const unsigned char *, const unsigned char *, size, locale_t);
+
+int test_warning_patterns() {
+  if (strcmp(A, "a"))
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is called without explicitly comparing result [misc-suspicious-string-compare]
+  // CHECK-FIXES: if (strcmp(A, "a") != 0)
+
+  if (!strcmp(A, "a"))
+return 0;
+
+  if (!strcmp(A, "a") ||
+  strcmp(A, "b"))
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is called without explicitly comparing result [misc-suspicious-string-compare]
+  // CHECK-FIXES: strcmp(A, "b") != 0)
+
+  if (strcmp(A, "a") == 1)
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is compared to a suspicious constant [misc-suspicious-string-compare]
+
+  if (strcmp(A, "a") == -1)
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is compared to a suspicious

Re: [PATCH] D18567: Block: Fix a crash when we have type attributes or qualifiers with omitted return type.

[Manman Ren via cfe-commits]

manmanren updated this revision to Diff 52398.
manmanren added a comment.

Addressing review comments.


http://reviews.llvm.org/D18567

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaType.cpp
  test/SemaObjC/block-omitted-return-type.m
  test/SemaOpenCL/invalid-block.cl

Index: test/SemaObjC/block-omitted-return-type.m
===
--- /dev/null
+++ test/SemaObjC/block-omitted-return-type.m
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 %s -fblocks -verify -fsyntax-only
+
+@interface NSObject
+@end
+
+@interface Test : NSObject
+- (void)test;
+@end
+
+@implementation Test
+- (void)test
+{
+  void (^simpleBlock)() = ^ _Nonnull { //expected-warning {{attribute '_Nonnull' ignored, because it cannot be applied to omitted return type}}
+return;
+  };
+  void (^simpleBlock2)() = ^ _Nonnull void { //expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'void'}}
+return;
+  };
+  void (^simpleBlock3)() = ^ _Nonnull (void) {  //expected-warning {{attribute '_Nonnull' ignored, because it cannot be applied to omitted return type}}
+return;
+  };
+
+  void (^simpleBlock4)() = ^ const { //expected-warning {{'const' qualifier on omitted return type '' has no effect}}
+return;
+  };
+  void (^simpleBlock5)() = ^ const void { //expected-error {{incompatible block pointer types initializing 'void (^)()' with an expression of type 'const void (^)(void)'}}
+return;
+  };
+  void (^simpleBlock6)() = ^ const (void) { //expected-warning {{'const' qualifier on omitted return type '' has no effect}}
+return;
+  };
+  void (^simpleBlock7)() = ^ _Nonnull __attribute__((align_value(128))) _Nullable const (void) { // expected-warning {{attribute '_Nullable' ignored, because it cannot be applied to omitted return type}} \
+// expected-warning {{attribute '_Nonnull' ignored, because it cannot be applied to omitted return type}} \
+// expected-warning {{'const' qualifier on omitted return type '' has no effect}} \
+// expected-warning {{'align_value' attribute only applies to variables and typedefs}}
+return;
+  };
+  void (^simpleBlock9)() = ^ __attribute__ ((align_value(128))) _Nonnull const (void) { // expected-warning {{attribute '_Nonnull' ignored, because it cannot be applied to omitted return type}} \
+// expected-warning {{'const' qualifier on omitted return type '' has no effect}} \
+// expected-warning {{'align_value' attribute only applies to variables and typedefs}}
+return;
+  };
+}
+@end
Index: test/SemaOpenCL/invalid-block.cl
===
--- test/SemaOpenCL/invalid-block.cl
+++ test/SemaOpenCL/invalid-block.cl
@@ -4,16 +4,15 @@
 
 // All blocks declarations must be const qualified and initialized.
 void f1() {
-  int (^bl1)() = ^() {}; // expected-error{{invalid block variable declaration - must be const qualified}}
+  int (^bl1)() = ^() {return 1;}; // expected-error{{invalid block variable declaration - must be const qualified}}
   int (^const bl2)(); // expected-error{{invalid block variable declaration - must be initialized}}
-  int (^const bl3)() = ^const(){
-  };
+  int (^const bl3)() = ^(){return 1;};
 }
 
 // A block with extern storage class is not allowed.
-extern int (^const bl)() = ^const(){}; // expected-error{{invalid block variable declaration - using 'extern' storage class is disallowed}}
+extern int (^const bl)() = ^(){return 1;}; // expected-error{{invalid block variable declaration - using 'extern' storage class is disallowed}}
 void f2() {
-  extern int (^const bl)() = ^const(){}; // expected-error{{invalid block variable declaration - using 'extern' storage class is disallowed}}
+  extern int (^const bl)() = ^(){return 1;}; // expected-error{{invalid block variable declaration - using 'extern' storage class is disallowed}}
 }
 
 // A block cannot be the return value of a function.
@@ -29,22 +28,22 @@
 }
 
 // A block with variadic argument is not allowed.
-int (^const bl)(int, ...) = ^const int(int I, ...) { // expected-error {{invalid block prototype, variadic arguments are not allowed in OpenCL}}
+int (^const bl)(int, ...) = ^int(int I, ...) { // expected-error {{invalid block prototype, variadic arguments are not allowed in OpenCL}}
   return 0;
 };
 
 // A block can't be used to declare an array
 typedef int (^const bl1_t)(int);
 void f5(int i) {
-  bl1_t bl1 = ^const(int i) {return 1;};
-  bl1_t bl2 = ^const(int i) {return 2;};
+  bl1_t bl1 = ^(int i) {return 1;};
+  bl1_t bl2 = ^(int i) {return 2;};
   bl1_t arr[] = {bl1, bl2}; // expected-error {{array of 'bl1_t' (aka 'int (^const)(int)') type is invalid in OpenCL}}
   int tmp = i ? bl1(i)  // expected-error {{block type cannot be used as expression in ternary expression in OpenCL}}
   : bl2(i); // expected-error {{block type cannot be used as expression in ternary expression in OpenCL}}
 }
 
 void f6(bl1_t * bl_ptr) {
-

Re: [PATCH] D18698: [C11] PR24451: Allow restrict _Atomic pointers

[Richard Smith via cfe-commits]

rsmith added a comment.

I'm not convinced this change is correct. There are two possibilities for what 
`int *_Atomic restrict` could mean:

1. `_Atomic(int *restrict)`: this seems to be ill-formed by 6.7.2.4/3, because 
the type name in an atomic type specifier cannot be a qualified type
2. `_Atomic(int *) restrict`: this seems to be ill-formed by 6.7.3/2, because 
(by 6.2.5) an atomic type is not a pointer type

If you're trying to simulate a GCC extension, this should at least have a 
corresponding `ExtWarn`, but you'll also need to make the case that the 
extension is useful.


http://reviews.llvm.org/D18698



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

Re: [PATCH] D18703: [clang-tidy] Add new checker for comparison with runtime string functions.

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko added a comment.

Implicit conversions to bool should be handled by 
readability-implicit-bool-cast.


http://reviews.llvm.org/D18703



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

Re: [PATCH] D18703: [clang-tidy] Add new checker for comparison with runtime string functions.

[Etienne Bergeron via cfe-commits]

etienneb added a comment.

As pointed by Eugene,

The following code seems to produce multiple errors.

  int foo() {
if (strcmp(A, "a") == true)
  return 0;
return 1;
  }

Results:

  /home/etienneb/examples/test.cc:56:7: warning: function 'strcmp' is compared 
to a suspicious constant [misc-suspicious-string-compare]
if (strcmp(A, "a") == true)
^
  /home/etienneb/examples/test.cc:56:25: warning: implicit cast bool -> 'int' 
[readability-implicit-bool-cast]
if (strcmp(A, "a") == true)
  ^
  /home/etienneb/examples/test.cc:56:25: warning: redundant boolean literal 
supplied to boolean operator [readability-simplify-boolean-expr]
if (strcmp(A, "a") == true)
  ^
static_cast(strcmp(A, "a"))

On my side, I'm not against to keep it that way.
The //implicit cast bool -> 'int'// warning is often too noisy and is often 
ignored by programmers.
The other message sounds more specific and may ring a bell.

WDYT?


http://reviews.llvm.org/D18703



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

Re: [PATCH] D18703: [clang-tidy] Add new checker for comparison with runtime string functions.

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added inline comments.


Comment at: docs/clang-tidy/checks/misc-suspicious-string-compare.rst:12
@@ +11,3 @@
+Example:
+  if (strcmp(...))   // Implicitly compare to zero
+  if (!strcmp(...))  // Won't warn

I don't know documentation markup well, but looks like you need to add ".. 
code-block:: c++" before code snippet. See performance-implicit-cast-in-loop 
for example.


http://reviews.llvm.org/D18703



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

Re: [PATCH] D18703: [clang-tidy] Add new checker for comparison with runtime string functions.

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a comment.

In http://reviews.llvm.org/D18703#389706, @etienneb wrote:

> As pointed by Eugene,
>
> The following code seems to produce multiple errors.
>
>   int foo() {
> if (strcmp(A, "a") == true)
>   return 0;
> return 1;
>   }
>
>
> Results:
>
>   /home/etienneb/examples/test.cc:56:7: warning: function 'strcmp' is 
> compared to a suspicious constant [misc-suspicious-string-compare]
> if (strcmp(A, "a") == true)
> ^
>   /home/etienneb/examples/test.cc:56:25: warning: implicit cast bool -> 'int' 
> [readability-implicit-bool-cast]
> if (strcmp(A, "a") == true)
>   ^
>   /home/etienneb/examples/test.cc:56:25: warning: redundant boolean literal 
> supplied to boolean operator [readability-simplify-boolean-expr]
> if (strcmp(A, "a") == true)
>   ^
> static_cast(strcmp(A, "a"))
>
>
> On my side, I'm not against to keep it that way.
>  The //implicit cast bool -> 'int'// warning is often too noisy and is often 
> ignored by programmers.
>  The other message sounds more specific and may ring a bell.
>
> WDYT?


Actually I meant "if (strcmp(...))" statement when I refer to 
readability-implicit-bool-cast.


http://reviews.llvm.org/D18703



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

Re: r265038 - Diagnostics: remove dodgy handler for bitcode inlineasm diagnostics.

[Tim Northover via cfe-commits]

> On 31 Mar 2016, at 16:51, Steven Wu  wrote:
> 
> The original handler is not there to workaround a crash. It is to avoid 
> generate a crash report (report_fatal_error) when the input bitcode contains 
> invalid assembly. 

Ah, I see now. That test needs tweaking too, since it doesn't actually produce 
the crash report even after my change (probably because it's a %clang_cc1 test).

How does the patch attached here look? I originally tried reusing the real 
InlineAsmDiagHandler but that was disastrous (it needs a real AST I think, 
which obviously doesn't exist when compiling a .bc file).

Cheers.

Tim.



asm-diagnostics.diff
Description: Binary data
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r265177 - Diagnose missing macro argument following charize operator.

[Andy Gibbs via cfe-commits]

Author: andyg
Date: Fri Apr  1 14:02:20 2016
New Revision: 265177

URL: http://llvm.org/viewvc/llvm-project?rev=265177&view=rev
Log:
Diagnose missing macro argument following charize operator.

For completeness, add a test-case for the equivalent stringize operator
diagnostic too.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/test/Parser/MicrosoftExtensions.c
cfe/trunk/test/Preprocessor/stringize_misc.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=265177&r1=265176&r2=265177&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Fri Apr  1 14:02:20 2016
@@ -387,7 +387,7 @@ def err_pp_expected_comma_in_arg_list :
 def err_pp_duplicate_name_in_arg_list : Error<
   "duplicate macro parameter name %0">;
 def err_pp_stringize_not_parameter : Error<
-  "'#' is not followed by a macro parameter">;
+  "'%select{#|#@}0' is not followed by a macro parameter">;
 def err_pp_malformed_ident : Error<"invalid #ident directive">;
 def err_pp_unterminated_conditional : Error<
   "unterminated conditional directive">;

Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=265177&r1=265176&r2=265177&view=diff
==
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Fri Apr  1 14:02:20 2016
@@ -2151,7 +2151,7 @@ void Preprocessor::HandleDefineDirective
 while (Tok.isNot(tok::eod)) {
   LastTok = Tok;
 
-  if (Tok.isNot(tok::hash) && Tok.isNot(tok::hashhash)) {
+  if (!Tok.isOneOf(tok::hash, tok::hashat, tok::hashhash)) {
 MI->AddTokenToBody(Tok);
 
 // Get the next token of the macro.
@@ -2210,7 +2210,8 @@ void Preprocessor::HandleDefineDirective
   MI->AddTokenToBody(LastTok);
   continue;
 } else {
-  Diag(Tok, diag::err_pp_stringize_not_parameter);
+  Diag(Tok, diag::err_pp_stringize_not_parameter)
+<< LastTok.is(tok::hashat);
 
   // Disable __VA_ARGS__ again.
   Ident__VA_ARGS__->setIsPoisoned(true);

Modified: cfe/trunk/test/Parser/MicrosoftExtensions.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/MicrosoftExtensions.c?rev=265177&r1=265176&r2=265177&view=diff
==
--- cfe/trunk/test/Parser/MicrosoftExtensions.c (original)
+++ cfe/trunk/test/Parser/MicrosoftExtensions.c Fri Apr  1 14:02:20 2016
@@ -35,6 +35,9 @@ void test_ms_alignof_alias(void) {
 /* Charify extension. */
 #define FOO(x) #@x
 char x = FOO(a);
+#define HASHAT #@
+#define MISSING_ARG(x) #@
+/* expected-error@-1 {{'#@' is not followed by a macro parameter}} */
 
 typedef enum E { e1 };
 

Modified: cfe/trunk/test/Preprocessor/stringize_misc.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/stringize_misc.c?rev=265177&r1=265176&r2=265177&view=diff
==
--- cfe/trunk/test/Preprocessor/stringize_misc.c (original)
+++ cfe/trunk/test/Preprocessor/stringize_misc.c Fri Apr  1 14:02:20 2016
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -E %s | FileCheck -strict-whitespace %s
+#ifdef TEST1
+// RUN: %clang_cc1 -E %s -DTEST1 | FileCheck -strict-whitespace %s
 
 #define M(x, y) #x #y
 
@@ -28,3 +29,13 @@ START_END( {a=1 , b=2;} ) /* braces are
 M(a COMMA b, (a, b)) 
 // CHECK: "a COMMA b" "(a, b)"
 
+#endif
+
+#ifdef TEST2
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST2
+
+#define HASH #
+#define INVALID() #
+// expected-error@-1{{'#' is not followed by a macro parameter}}
+
+#endif


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

Re: [PATCH] D18703: [clang-tidy] Add new checker for comparison with runtime string functions.

[Etienne Bergeron via cfe-commits]

etienneb added a comment.

You're right, both are outputted.

  /home/etienneb/examples/test.cc:56:7: warning: function 'strcmp' is called 
without explicitly comparing result [misc-suspicious-string-compare]
if (strcmp(A, "a"))
^
   != 0
  /home/etienneb/examples/test.cc:56:7: warning: implicit cast 'int' -> bool 
[readability-implicit-bool-cast]
if (strcmp(A, "a"))
^
   != 0


http://reviews.llvm.org/D18703



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

Re: r265038 - Diagnostics: remove dodgy handler for bitcode inlineasm diagnostics.

[Steven Wu via cfe-commits]

> On Apr 1, 2016, at 12:04 PM, Tim Northover  wrote:
> 
> 
>> On 31 Mar 2016, at 16:51, Steven Wu  wrote:
>> 
>> The original handler is not there to workaround a crash. It is to avoid 
>> generate a crash report (report_fatal_error) when the input bitcode contains 
>> invalid assembly. 
> 
> Ah, I see now. That test needs tweaking too, since it doesn't actually 
> produce the crash report even after my change (probably because it's a 
> %clang_cc1 test).
Probably need assertion build or some cmake setting to actually show the crash 
log. 
> 
> How does the patch attached here look? I originally tried reusing the real 
> InlineAsmDiagHandler but that was disastrous (it needs a real AST I think, 
> which obviously doesn't exist when compiling a .bc file).
Generally good. How about only report error when SM.getKind() == DK_Error?

Steven

> 
> Cheers.
> 
> Tim.
> 
> 

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

[PATCH] D18709: Add copyright notice to modulemap files

[Yunzhong Gao via cfe-commits]

ygao created this revision.
ygao added subscribers: cfe-commits, rsmith.

Hi,
It seems that the module.modulemap file in the lib/Headers directory is missing 
the LLVM copyright notice.
This patch adds the copyright notice just like the rest of the files in this 
directory.
Could you review that is acceptable?

Thanks,
- Gao

http://reviews.llvm.org/D18709

Files:
  lib/Headers/module.modulemap

Index: lib/Headers/module.modulemap
===
--- lib/Headers/module.modulemap
+++ lib/Headers/module.modulemap
@@ -1,3 +1,26 @@
+/*=== module.modulemap - intrinsics module map -===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ *===---===
+ */
+
 module _Builtin_intrinsics [system] [extern_c] {
   explicit module altivec {
 requires altivec


Index: lib/Headers/module.modulemap
===
--- lib/Headers/module.modulemap
+++ lib/Headers/module.modulemap
@@ -1,3 +1,26 @@
+/*=== module.modulemap - intrinsics module map -===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ *===---===
+ */
+
 module _Builtin_intrinsics [system] [extern_c] {
   explicit module altivec {
 requires altivec
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17963: [OPENMP] Codegen for teams directive for NVPTX

[Carlo Bertolli via cfe-commits]

carlo.bertolli updated this revision to Diff 52416.
carlo.bertolli added a comment.

[OPENMP] Even though this patch was already accepted in its previous form, 
comments on depending patch http://reviews.llvm.org/D18286 
(http://reviews.llvm.org/D18286) revealed that a new approach for this patch 
was necessary. Instead of committing something that I have to change anyway 
later on, I decided to provide a new version of this base patch. Please review 
it again and let me know about any comments you may have.


Repository:
  rL LLVM

http://reviews.llvm.org/D17963

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CGStmtOpenMP.cpp
  test/OpenMP/nvptx_teams_codegen.cpp

Index: test/OpenMP/nvptx_teams_codegen.cpp
===
--- /dev/null
+++ test/OpenMP/nvptx_teams_codegen.cpp
@@ -0,0 +1,136 @@
+// Test target codegen - host bc file has to be created first.
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fomptargets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -fomptargets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fomp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s --check-prefix CK1 --check-prefix CK1-64
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -x c++ -triple i386-unknown-unknown -fomptargets=nvptx-nvidia-cuda -emit-llvm-bc %s -o %t-x86-host.bc
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -x c++ -triple nvptx-unknown-unknown -fomptargets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fomp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s --check-prefix CK1 --check-prefix CK1-32
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+#ifdef CK1
+
+template 
+int tmain(T argc) {
+#pragma omp target
+#pragma omp teams
+  argc = 0;
+  return 0;
+}
+
+
+int main (int argc, char **argv) {
+#pragma omp target
+#pragma omp teams
+  {
+  argc = 0;
+  }
+  return tmain(argv);
+}
+
+// only nvptx side: do not outline teams region and do not call fork_teams
+// CK1:  define {{.*}}void @{{[^,]+}}(i{{[0-9]+}} [[ARGC:%.+]])
+// CK1:  {{.+}} = alloca i{{[0-9]+}}*,
+// CK1:  {{.+}} = alloca i{{[0-9]+}}*,
+// CK1:  [[ARGCADDR_PTR:%.+]] = alloca i{{[0-9]+}}*,
+// CK1:  [[ARGCADDR:%.+]] = alloca i{{[0-9]+}},
+// CK1:  store {{.+}} 0, {{.+}},
+// CK1:  store i{{[0-9]+}} [[ARGC]], i{{[0-9]+}}* [[ARGCADDR]],
+// CK1-64:  [[CONV:%.+]] = bitcast i{{[0-9]+}}* [[ARGCADDR]] to i{{[0-9]+}}*
+// CK1-64:  store i{{[0-9]+}}* [[CONV]], i{{[0-9]+}}** [[ARGCADDR_PTR]],
+// CK1-32:  store i{{[0-9]+}}* [[ARGCADDR]], i{{[0-9]+}}** [[ARGCADDR_PTR]],
+// CK1:  [[ARGCADDR_PTR_REF:%.+]] = load i{{[0-9]+}}*, i{{[0-9]+}}** [[ARGCADDR_PTR]],
+// CK1:  store i{{[0-9]+}} 0, i{{[0-9]+}}* [[ARGCADDR_PTR_REF]],
+// CK1-NOT: call {{.*}}void (%ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_teams(
+// CK1:  ret void
+// CK1-NEXT: }
+
+// target region in template
+// CK1: define {{.*}}void @{{[^,]+}}(i{{.+}}***{{.+}} [[ARGC:%.+]])
+// CK1: [[ARGCADDR_PTR:%.+]] = alloca i{{.+}}***,
+// CK1: [[ARGCADDR:%.+]] = alloca i{{.+}}***,
+// CK1: store i{{.+}}*** [[ARGC]], i{{.+}} [[ARGCADDR]]
+// CK1: [[ARGCADDR_REF:%.+]] = load i{{.+}}***, i{{.+}} [[ARGCADDR]],
+// CK1: store i8*** [[ARGCADDR_REF]], i8 [[ARGCADDR_PTR]],
+// CK1: [[ARGCADDR_PTR_REF:%.+]] = load i{{.+}}***, i{{.+}} [[ARGCADDR_PTR]],
+// CK1: store i{{[0-9]+}}** null, i{{[0-9]+}}*** [[ARGCADDR_PTR_REF]],
+// CK1-NOT: call {{.*}}void (%ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_teams(
+// CK1:  ret void
+// CK1-NEXT: }
+
+
+#endif // CK1
+
+// Test target codegen - host bc file has to be created first.
+// RUN: %clang_cc1 -DCK2 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fomptargets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -DCK2 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -fomptargets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fomp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s --check-prefix CK2 --check-prefix CK2-64
+// RUN: %clang_cc1 -DCK2 -verify -fopenmp -x c++ -triple i386-unknown-unknown -fomptargets=nvptx-nvidia-cuda -emit-llvm-bc %s -o %t-x86-host.bc
+// RUN: %clang_cc1 -DCK2 -verify -fopenmp -x c++ -triple nvptx-unknown-unknown -fomptargets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fomp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s --check-prefix CK2 --check-prefix CK2-32
+// expected-no-diagnostics
+#ifdef CK2
+
+template 
+int tmain(T argc) {
+  int a = 10;
+  int b = 5;
+#pragma omp target
+#pragma omp teams num_teams(a) thread_limit(b)
+  {
+  argc = 0;
+  }
+  return 0;
+}
+
+int main (int argc, char **argv) {
+  int a = 20;
+  int b = 5;
+#pragma omp target
+#pragma omp teams num_teams(a) thread_limit(b)
+  {
+  argc = 0;
+  

Re: [PATCH] D18703: [clang-tidy] Add new checker for comparison with runtime string functions.

[Etienne Bergeron via cfe-commits]

etienneb updated this revision to Diff 52420.
etienneb added a comment.

Fix support for C.
Adding test for C99 code.


http://reviews.llvm.org/D18703

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/SuspiciousStringCompareCheck.cpp
  clang-tidy/misc/SuspiciousStringCompareCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-suspicious-string-compare.rst
  test/clang-tidy/misc-suspicious-string-compare.c
  test/clang-tidy/misc-suspicious-string-compare.cpp

Index: test/clang-tidy/misc-suspicious-string-compare.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-suspicious-string-compare.cpp
@@ -0,0 +1,298 @@
+// RUN: %check_clang_tidy %s misc-suspicious-string-compare %t
+
+typedef __SIZE_TYPE__ size;
+
+struct locale_t {
+  void* dummy;
+} locale;
+
+static const char A[] = "abc";
+static const unsigned char U[] = "abc";
+static const unsigned char V[] = "xyz";
+static const wchar_t W[] = L"abc";
+
+int memcmp(const void *, const void *, size);
+int wmemcmp(const wchar_t *, const wchar_t *, size);
+int memicmp(const void *, const void *, size);
+int _memicmp(const void *, const void *, size);
+int _memicmp_l(const void *, const void *, size, locale_t);
+
+int strcmp(const char *, const char *);
+int strncmp(const char *, const char *, size);
+int strcasecmp(const char *, const char *);
+int strncasecmp(const char *, const char *, size);
+int stricmp(const char *, const char *);
+int strcmpi(const char *, const char *);
+int strnicmp(const char *, const char *, size);
+int _stricmp(const char *, const char * );
+int _strnicmp(const char *, const char *, size);
+int _stricmp_l(const char *, const char *, locale_t);
+int _strnicmp_l(const char *, const char *, size, locale_t);
+
+int wcscmp(const wchar_t *, const wchar_t *);
+int wcsncmp(const wchar_t *, const wchar_t *, size);
+int wcscasecmp(const wchar_t *, const wchar_t *);
+int wcsicmp(const wchar_t *, const wchar_t *);
+int wcsnicmp(const wchar_t *, const wchar_t *, size);
+int _wcsicmp(const wchar_t *, const wchar_t *);
+int _wcsnicmp(const wchar_t *, const wchar_t *, size);
+int _wcsicmp_l(const wchar_t *, const wchar_t *, locale_t);
+int _wcsnicmp_l(const wchar_t *, const wchar_t *, size, locale_t);
+
+int _mbscmp(const unsigned char *, const unsigned char *);
+int _mbsncmp(const unsigned char *, const unsigned char *, size);
+int _mbsnbcmp(const unsigned char *, const unsigned char *, size);
+int _mbsnbicmp(const unsigned char *, const unsigned char *, size);
+int _mbsicmp(const unsigned char *, const unsigned char *);
+int _mbsnicmp(const unsigned char *, const unsigned char *, size);
+int _mbscmp_l(const unsigned char *, const unsigned char *, locale_t);
+int _mbsncmp_l(const unsigned char *, const unsigned char *, size, locale_t);
+int _mbsicmp_l(const unsigned char *, const unsigned char *, locale_t);
+int _mbsnicmp_l(const unsigned char *, const unsigned char *, size, locale_t);
+int _mbsnbcmp_l(const unsigned char *, const unsigned char *, size, locale_t);
+int _mbsnbicmp_l(const unsigned char *, const unsigned char *, size, locale_t);
+
+int test_warning_patterns() {
+  if (strcmp(A, "a"))
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is called without explicitly comparing result [misc-suspicious-string-compare]
+  // CHECK-FIXES: if (strcmp(A, "a") != 0)
+
+  if (!strcmp(A, "a") ||
+  strcmp(A, "b"))
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is called without explicitly comparing result [misc-suspicious-string-compare]
+  // CHECK-FIXES: strcmp(A, "b") != 0)
+
+  if (strcmp(A, "a") == 1)
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is compared to a suspicious constant [misc-suspicious-string-compare]
+
+  if (strcmp(A, "a") == -1)
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is compared to a suspicious constant [misc-suspicious-string-compare]
+
+  if (strcmp(A, "a") == true)
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is compared to a suspicious constant [misc-suspicious-string-compare]
+
+  if (strcmp(A, "a") < '0')
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is compared to a suspicious constant [misc-suspicious-string-compare]
+
+  if (strcmp(A, "a") < 0.)
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' has suspicious implicit cast [misc-suspicious-string-compare]
+}
+
+int test_valid_patterns() {
+  // The following cases are valid.
+  if (strcmp(A, "a") < 0)
+return 0;
+  if (strcmp(A, "a") == 0)
+return 0;
+  if (strcmp(A, "a") <= 0)
+return 0;
+
+  if (wcscmp(W, L"a") < 0)
+return 0;
+  if (wcscmp(W, L"a") == 0)
+return 0;
+  if (wcscmp(W, L"a") <= 0)
+return 0;
+
+  if (!strcmp(A, "a"))
+return 0;
+
+  return 1;
+}
+
+int test_implicit_compare_

Re: [PATCH] D18703: [clang-tidy] Add new checker for comparison with runtime string functions.

[Etienne Bergeron via cfe-commits]

etienneb updated this revision to Diff 52421.
etienneb marked an inline comment as done.
etienneb added a comment.

nits and formatting


http://reviews.llvm.org/D18703

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/SuspiciousStringCompareCheck.cpp
  clang-tidy/misc/SuspiciousStringCompareCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-suspicious-string-compare.rst
  test/clang-tidy/misc-suspicious-string-compare.c
  test/clang-tidy/misc-suspicious-string-compare.cpp

Index: test/clang-tidy/misc-suspicious-string-compare.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-suspicious-string-compare.cpp
@@ -0,0 +1,298 @@
+// RUN: %check_clang_tidy %s misc-suspicious-string-compare %t
+
+typedef __SIZE_TYPE__ size;
+
+struct locale_t {
+  void* dummy;
+} locale;
+
+static const char A[] = "abc";
+static const unsigned char U[] = "abc";
+static const unsigned char V[] = "xyz";
+static const wchar_t W[] = L"abc";
+
+int memcmp(const void *, const void *, size);
+int wmemcmp(const wchar_t *, const wchar_t *, size);
+int memicmp(const void *, const void *, size);
+int _memicmp(const void *, const void *, size);
+int _memicmp_l(const void *, const void *, size, locale_t);
+
+int strcmp(const char *, const char *);
+int strncmp(const char *, const char *, size);
+int strcasecmp(const char *, const char *);
+int strncasecmp(const char *, const char *, size);
+int stricmp(const char *, const char *);
+int strcmpi(const char *, const char *);
+int strnicmp(const char *, const char *, size);
+int _stricmp(const char *, const char * );
+int _strnicmp(const char *, const char *, size);
+int _stricmp_l(const char *, const char *, locale_t);
+int _strnicmp_l(const char *, const char *, size, locale_t);
+
+int wcscmp(const wchar_t *, const wchar_t *);
+int wcsncmp(const wchar_t *, const wchar_t *, size);
+int wcscasecmp(const wchar_t *, const wchar_t *);
+int wcsicmp(const wchar_t *, const wchar_t *);
+int wcsnicmp(const wchar_t *, const wchar_t *, size);
+int _wcsicmp(const wchar_t *, const wchar_t *);
+int _wcsnicmp(const wchar_t *, const wchar_t *, size);
+int _wcsicmp_l(const wchar_t *, const wchar_t *, locale_t);
+int _wcsnicmp_l(const wchar_t *, const wchar_t *, size, locale_t);
+
+int _mbscmp(const unsigned char *, const unsigned char *);
+int _mbsncmp(const unsigned char *, const unsigned char *, size);
+int _mbsnbcmp(const unsigned char *, const unsigned char *, size);
+int _mbsnbicmp(const unsigned char *, const unsigned char *, size);
+int _mbsicmp(const unsigned char *, const unsigned char *);
+int _mbsnicmp(const unsigned char *, const unsigned char *, size);
+int _mbscmp_l(const unsigned char *, const unsigned char *, locale_t);
+int _mbsncmp_l(const unsigned char *, const unsigned char *, size, locale_t);
+int _mbsicmp_l(const unsigned char *, const unsigned char *, locale_t);
+int _mbsnicmp_l(const unsigned char *, const unsigned char *, size, locale_t);
+int _mbsnbcmp_l(const unsigned char *, const unsigned char *, size, locale_t);
+int _mbsnbicmp_l(const unsigned char *, const unsigned char *, size, locale_t);
+
+int test_warning_patterns() {
+  if (strcmp(A, "a"))
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is called without explicitly comparing result [misc-suspicious-string-compare]
+  // CHECK-FIXES: if (strcmp(A, "a") != 0)
+
+  if (!strcmp(A, "a") ||
+  strcmp(A, "b"))
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is called without explicitly comparing result [misc-suspicious-string-compare]
+  // CHECK-FIXES: strcmp(A, "b") != 0)
+
+  if (strcmp(A, "a") == 1)
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is compared to a suspicious constant [misc-suspicious-string-compare]
+
+  if (strcmp(A, "a") == -1)
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is compared to a suspicious constant [misc-suspicious-string-compare]
+
+  if (strcmp(A, "a") == true)
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is compared to a suspicious constant [misc-suspicious-string-compare]
+
+  if (strcmp(A, "a") < '0')
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is compared to a suspicious constant [misc-suspicious-string-compare]
+
+  if (strcmp(A, "a") < 0.)
+return 0;
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' has suspicious implicit cast [misc-suspicious-string-compare]
+}
+
+int test_valid_patterns() {
+  // The following cases are valid.
+  if (strcmp(A, "a") < 0)
+return 0;
+  if (strcmp(A, "a") == 0)
+return 0;
+  if (strcmp(A, "a") <= 0)
+return 0;
+
+  if (wcscmp(W, L"a") < 0)
+return 0;
+  if (wcscmp(W, L"a") == 0)
+return 0;
+  if (wcscmp(W, L"a") <= 0)
+return 0;
+
+  if (!strcmp(A, "a"))
+return 0;
+
+  return 1;
+}
+
+int test

Re: [PATCH] D18703: [clang-tidy] Add new checker for comparison with runtime string functions.

[Etienne Bergeron via cfe-commits]

etienneb added a comment.

Eugene, I double checked and the implicit bool version doesn't trigger in C.
This check is applied in C and C++.
(note: I needed to add code to fix C)



Comment at: docs/clang-tidy/checks/misc-suspicious-string-compare.rst:13
@@ +12,3 @@
+.. code:: c++
+if (strcmp(...))   // Implicitly compare to zero
+if (!strcmp(...))  // Won't warn

Any idea how to validate the look after formatting?


http://reviews.llvm.org/D18703



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

Re: [PATCH] D17412: PR19957: [OpenCL] incorrectly accepts implicit address space conversion with ternary operator

[Yaxun Liu via cfe-commits]

yaxunl marked 2 inline comments as done.


Comment at: test/CodeGenOpenCL/address-spaces-conversions.cl:1
@@ -1,2 +1,2 @@
 // RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 
-ffake-address-space-map -cl-std=CL2.0 -emit-llvm -o - | FileCheck %s
 

Anastasia wrote:
> Cool, thanks! Could you insert a link to the new review here if possible.
I looked into the ICE when -ffake-address-space-map is not set.

when -ffake-address-space-map is set, address space 0-4 mapped to 0-4. When it 
is not set, it maps to values defined by target. x86 uses DefaultAddrSpaceMap 
which maps 0-4 all to 0. This causes assertion for testcase like

  global int *a;
  generic void *b = a;

A review is opened:

http://reviews.llvm.org/D18713


http://reviews.llvm.org/D17412



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

Re: [PATCH] D18565: Implement an "I'm dtrace, please retain all debug types" option.

[David Blaikie via cfe-commits]

On Fri, Apr 1, 2016 at 10:54 AM, Adrian Prantl  wrote:

>
> On Mar 31, 2016, at 5:35 PM, David Blaikie  wrote:
>
>
>
> On Wed, Mar 30, 2016 at 10:49 AM, Adrian Prantl  wrote:
>
>>
>> On Mar 29, 2016, at 10:06 PM, David Blaikie  wrote:
>>
>>
>>
>> On Tue, Mar 29, 2016 at 12:03 PM, Adrian Prantl via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>>
>>> > On Mar 29, 2016, at 12:00 PM, Joerg Sonnenberger <
>>> jo...@britannica.bec.de> wrote:
>>> >
>>> > On Tue, Mar 29, 2016 at 06:47:24PM +, Adrian Prantl via
>>> cfe-commits wrote:
>>> >> This code in this patch listens to the driver option -gfull, and
>>> lowers it to the new cc1 option -debug-retain-types (1).
>>> >> When -debug-retain-types is present, CGDebugInfo will retain every(2)
>>> type it creates.
>>> >
>>> > Is there a good reason for calling it -gfull? I would find something
>>> > -gall-types or -gretain-all-types to make a lot more sense. This should
>>> > be orthogonal to other options like providing only line tables?
>>>
>>> My thinking was this:
>>> The driver already supports -gfull, but it doesn’t do anything.
>>> This patch can be considered a first step towards making -gfull behave
>>> as expected.
>>> Eventually it should emit debug info for *all* types.
>>>
>>
>> Seems somewhat problematic to half implement it, though. (admittedly
>> we're just silently ignoring it right now)
>>
>>
>> I don’t think this is problematic at all. This is incremental development.
>>
>
> It strikes me as a strange increment. Implementing full -gfull doesn't
> seem like it would take much time to implement, etc.
>
>>
>> & is 'real' -gfull what dtrace really wants? (seems it isn't - since
>> clang's never really implemented it?)
>>
>>
>> Admitted, ‘real' -gfull is probably more than it absolutely needs.
>>
>
> If just retaining referenced types is all it needs, yeah, it seems -gfull
> would be rather beyond that.
>
>
>>
>> Emitting all types referenced by used (even if later optimized away) code
>> seems like the thing? -greferenced? or maybe a -f flag? Not sure.
>>
>>
>> I don’t see a compelling case for adding another driver option to the
>> already confusing zoo of existing driver options.
>>
>
> My point is I think a -gfull that does something other than full would
> possibly be more confusing than not.
>
>
> Point taken. Let’s surface this under a separate option instead. We can
> call it “-greferenced” to fit between -gfull and -gused.
>

After seeing that -gfull/-gused are Darwin-only extensions in GCC, and if
we're introducing a new one anyway - should we be compatible with those
(that don't do anything in Clang anyway), or use something more like the -f
syntax used in GCC? It seems like we may want to avoid more -gX options
because they're often orthogonal (Google's recently had trouble with
-gsplit-dwarf + -gmlt, for example (build system gets confused when you
remove the output file it asked for... ))

- Dave


>
> thanks,
> adrian
>
>
>
>> Note that we currently also accept a -gused option which according to the
>> driver code is supposed to be the opposite of -gfull.
>>
>
> Are -gused/-gfull meant to toggle each other?
>
> Huh, seems they're not general GCC flags, they're Darwin things - I didn't
> know that.
>
> Looks like GCC usually spells this -f[no-]eliminate-unused-debug-types.
> But doesn't seem to have an intermediate version that would be what you're
> going for.
>
>
>> Adding a -greferenced option IMO will only make this more confusion
>> instead of helping.
>> My suggestion is to have -gfull (also) activate -debug-retain-types. In
>> the somewhat hypothetical scenario that someone implements a more
>> comprehensive version of -gfull we should revisit this and analyze whether
>> the resulting debug information is really too large to be practical, and if
>> we conclude that this is a problem, we can still decide to expose
>> -debug-retain-types to the driver with a new separate option.
>>
>
> I would be concerned about breaking other existing users that may grow
> once we support the flag. (& perhaps inconsistency between GCC and Clang,
> but inconsistency already exists there of course)
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18637: Extract key to avoid preemptive mallocs in insert/emplace in associative containers

[Duncan P. N. Exon Smith via cfe-commits]

LGTM.  Thanks for picking this up (and sorry for getting distracted)!

> On 2016-Mar-30, at 21:15, Eric Fiselier  wrote:
> 
> EricWF created this revision.
> EricWF added reviewers: mclow.lists, dexonsmith.
> EricWF added a subscriber: cfe-commits.
> 
> This patch applies Duncan's work on __hash_table to __tree.
> 
> http://reviews.llvm.org/D18637
> 
> Files:
>  include/__hash_table
>  include/__tree
>  include/__tuple
>  include/type_traits
>  
> test/std/containers/associative/map/map.modifiers/insert_allocator_requirements.pass.cpp
>  
> test/std/containers/associative/map/map.modifiers/insert_and_emplace_allocator_requirements.pass.cpp
>  test/std/containers/associative/set/insert_allocator_requirements.pass.cpp
>  
> test/std/containers/associative/set/insert_and_emplace_allocator_requirements.pass.cpp
> 
> 

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

Re: [PATCH] D18637: Extract key to avoid preemptive mallocs in insert/emplace in associative containers

[Duncan P. N. Exon Smith via cfe-commits]

dexonsmith added a subscriber: dexonsmith.
dexonsmith added a comment.

LGTM.  Thanks for picking this up (and sorry for getting distracted)!


http://reviews.llvm.org/D18637



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

Re: [PATCH] D18713: [OpenCL] Generate bitcast when target address space does not change.

[John McCall via cfe-commits]

rjmccall added inline comments.


Comment at: lib/CodeGen/CGExprScalar.cpp:1421
@@ -1415,1 +1420,3 @@
+else
+  return Builder.CreateBitCast(Src, DstTy);
   }

This is just CreatePointerBitCastOrAddrSpaceCast.


http://reviews.llvm.org/D18713



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

Re: [PATCH] D17933: Set MaxAtomicInlineWidth properly for i386, i486, and x86-64 cpus without cmpxchg16b.

[James Y Knight via cfe-commits]

jyknight marked an inline comment as done.


Comment at: test/CodeGen/atomic-ops.c:1
@@ -1,2 +1,2 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - -ffreestanding -ffake-address-space-map 
-triple=i686-apple-darwin9 | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -ffreestanding -ffake-address-space-map 
-triple=i686-apple-darwin9 -target-cpu i686 | FileCheck %s
 // REQUIRES: x86-registered-target

rsmith wrote:
> Why do you need a -target-cpu i686 in addition to the i686 triple?
The Driver code is what's responsible for parsing the default target cpu out of 
the triple. So if you invoke CC1 yourself, and don't pass -target-cpu, it 
treats i686-* triples as still targeting i386.


Comment at: test/Preprocessor/arm-target-features.c:108-118
@@ -107,2 +107,13 @@
 // V8M_BASELINE-NOT: __ARM_FP 0x{{.*}}
-// V8M_BASELINE-NOT: __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1
+// V8M_BASELINE: #define __GCC_ATOMIC_BOOL_LOCK_FREE 2
+// V8M_BASELINE: #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 2
+// V8M_BASELINE: #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 2
+// V8M_BASELINE: #define __GCC_ATOMIC_CHAR_LOCK_FREE 2
+// V8M_BASELINE: #define __GCC_ATOMIC_INT_LOCK_FREE 2
+// V8M_BASELINE: #define __GCC_ATOMIC_LLONG_LOCK_FREE 1
+// V8M_BASELINE: #define __GCC_ATOMIC_LONG_LOCK_FREE 2
+// V8M_BASELINE: #define __GCC_ATOMIC_POINTER_LOCK_FREE 2
+// V8M_BASELINE: #define __GCC_ATOMIC_SHORT_LOCK_FREE 2
+// V8M_BASELINE: #define __GCC_ATOMIC_TEST_AND_SET_TRUEVAL 1
+// V8M_BASELINE: #define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 2
+// V8M_BASELINE: __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1

rsmith wrote:
> Maybe drop the #define on these lines for consistency with the surrounding 
> tests?
Did the inverse (cleaned up the surrounding lines) in r265187.


Comment at: test/Preprocessor/init.c:3295
@@ -3292,1 +3294,3 @@
+// MIPSN32BE: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
+// MIPSN32BE: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 1
 // MIPSN32BE: #define __GNUC_MINOR__ 2

rsmith wrote:
> Can you add a `MIPSN32BE-NOT: ` for the 16 byte form? Likewise for the below 
> cases.
Done here and everywhere.


http://reviews.llvm.org/D17933



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

Re: [PATCH] D17933: Set MaxAtomicInlineWidth properly for i386, i486, and x86-64 cpus without cmpxchg16b.

[James Y Knight via cfe-commits]

jyknight updated this revision to Diff 52430.
jyknight added a comment.

Review tweaks.


http://reviews.llvm.org/D17933

Files:
  lib/Basic/Targets.cpp
  lib/Frontend/InitPreprocessor.cpp
  test/CodeGen/atomic-ops.c
  test/CodeGen/ms-volatile.c
  test/CodeGenCXX/atomicinit.cpp
  test/OpenMP/atomic_capture_codegen.cpp
  test/OpenMP/atomic_read_codegen.c
  test/OpenMP/atomic_update_codegen.cpp
  test/OpenMP/atomic_write_codegen.c
  test/Preprocessor/arm-target-features.c
  test/Preprocessor/init.c
  test/Preprocessor/predefined-arch-macros.c
  test/Preprocessor/predefined-macros.c
  test/Preprocessor/x86_target_features.c
  test/Sema/atomic-ops.c

Index: test/Sema/atomic-ops.c
===
--- test/Sema/atomic-ops.c
+++ test/Sema/atomic-ops.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -ffreestanding -fsyntax-only -triple=i686-linux-gnu -std=c11
+// RUN: %clang_cc1 %s -verify -ffreestanding -fsyntax-only -triple=i686-linux-gnu -target-cpu i686 -std=c11
 
 // Basic parsing/Sema tests for __c11_atomic_*
 
@@ -502,5 +502,3 @@
   (void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_acq_rel, memory_order_relaxed);
   (void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, memory_order_relaxed);
 }
-
-
Index: test/Preprocessor/x86_target_features.c
===
--- test/Preprocessor/x86_target_features.c
+++ test/Preprocessor/x86_target_features.c
@@ -291,8 +291,13 @@
 
 // NOTBM-NOT: #define __TBM__ 1
 
-// RUN: %clang -target i386-unknown-unknown -march=pentiumpro -mcx16 -x c -E -dM -o - %s | FileCheck -match-full-lines --check-prefix=MCX16 %s
+// RUN: %clang -target i386-unknown-unknown -march=pentiumpro -mcx16 -x c -E -dM -o - %s | FileCheck -match-full-lines --check-prefix=MCX16-386 %s
 
+// MCX16-386-NOT: __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16
+
+// RUN: %clang -target x86_64-unknown-unknown -mcx16 -x c -E -dM -o - %s | FileCheck -match-full-lines --check-prefix=MCX16 %s
+
+// MCX16: #define __GCC_ATOMIC_LLONG_LOCK_FREE 2
 // MCX16: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 1
 
 // RUN: %clang -target i386-unknown-unknown -march=atom -mprfchw -x c -E -dM -o - %s | FileCheck -match-full-lines --check-prefix=PRFCHW %s
Index: test/Preprocessor/predefined-macros.c
===
--- test/Preprocessor/predefined-macros.c
+++ test/Preprocessor/predefined-macros.c
@@ -102,47 +102,117 @@
 // RUN: %clang_cc1 %s -E -dM -o - -triple i686 -target-cpu i386 \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-SYNC_CAS_I386
 // CHECK-SYNC_CAS_I386-NOT: __GCC_HAVE_SYNC_COMPARE_AND_SWAP
+// CHECK-SYNC_CAS_I386-NOT: __GCC_ATOMIC_{{.*}}_LOCK_FREE 2
 //
 // RUN: %clang_cc1 %s -E -dM -o - -triple i686 -target-cpu i486 \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-SYNC_CAS_I486
+// CHECK-SYNC_CAS_I486: #define __GCC_ATOMIC_BOOL_LOCK_FREE 2
+// CHECK-SYNC_CAS_I486: #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 2
+// CHECK-SYNC_CAS_I486: #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 2
+// CHECK-SYNC_CAS_I486: #define __GCC_ATOMIC_CHAR_LOCK_FREE 2
+// CHECK-SYNC_CAS_I486: #define __GCC_ATOMIC_INT_LOCK_FREE 2
+// CHECK-SYNC_CAS_I486: #define __GCC_ATOMIC_LLONG_LOCK_FREE 1
+// CHECK-SYNC_CAS_I486: #define __GCC_ATOMIC_LONG_LOCK_FREE 2
+// CHECK-SYNC_CAS_I486: #define __GCC_ATOMIC_POINTER_LOCK_FREE 2
+// CHECK-SYNC_CAS_I486: #define __GCC_ATOMIC_SHORT_LOCK_FREE 2
+// CHECK-SYNC_CAS_I486: #define __GCC_ATOMIC_TEST_AND_SET_TRUEVAL 1
+// CHECK-SYNC_CAS_I486: #define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 2
 // CHECK-SYNC_CAS_I486: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
 // CHECK-SYNC_CAS_I486: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
 // CHECK-SYNC_CAS_I486: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
 // CHECK-SYNC_CAS_I486-NOT: __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8
+// CHECK-SYNC_CAS_I486-NOT: __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16
 //
+
+// FIXME: The value of LLONG_LOCK_FREE on i586 is wrong: clang is
+// using the alignment of "long long" (only 32bit for a 64bit value)
+// to disqualify it from being lock free. But the semantics of this
+// define are supposed to tell you if "_Atomic long long" is lock free
+// -- and it is.
+
 // RUN: %clang_cc1 %s -E -dM -o - -triple i686 -target-cpu i586 \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-SYNC_CAS_I586
+// CHECK-SYNC_CAS_I586: #define __GCC_ATOMIC_BOOL_LOCK_FREE 2
+// CHECK-SYNC_CAS_I586: #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 2
+// CHECK-SYNC_CAS_I586: #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 2
+// CHECK-SYNC_CAS_I586: #define __GCC_ATOMIC_CHAR_LOCK_FREE 2
+// CHECK-SYNC_CAS_I586: #define __GCC_ATOMIC_INT_LOCK_FREE 2
+// CHECK-SYNC_CAS_I586: #define __GCC_ATOMIC_LLONG_LOCK_FREE 1
+// CHECK-SYNC_CAS_I586: #define __GCC_ATOMIC_LONG_LOCK_FREE 2
+// CHECK-SYNC_CAS_I586: #define __GCC_ATOMIC_POINTER_LOCK_FREE 2
+// CHECK-SYNC_CAS_I586: #define __GCC_ATOMIC_SHORT_LOC

Re: [RFC] [PATCH] Announcing new Clang Diagnostic Personalities

[Richard Smith via cfe-commits]

Wonderful work, I look forward to it landing in SVN.

Only one concern: the choice to select the personality during compilation
seems to create problems:

 * We won't have an easy way to check that changes that add / change
diagnostics correctly update all the personalities
 * Distributions would presumably pick one personality for their users,
restricting user choice
 * The unit tests hard-code expected diagnostic output for a particular
personality, so would fail on other personalities.

I think the solution should be completely clear: please also update the
build system to always build the compiler five times, one for each
personality, and likewise make four additional copies of the test suite,
one for each new personality. This will increase build time and developer
overhead, but it's obviously worthwhile.

On Fri, Apr 1, 2016 at 12:32 PM, Chris Bieneman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Hello Clang Community!
>
> I’m writing as a representative of the self-termed "Clang Diagnostic
> Innovation” (CDI) working group to share with you all our newest feature.
>
> Clang has long been known for its excellent diagnostic messages, and for
> providing meaningful and actionable errors and warnings even in the most
> extreme cases. We set for ourselves the difficult challenge of trying to
> figure out how to get Clang’s diagnostic messages to have a stronger impact
> on our users and to help them grow as engineers.
>
> We spent months researching and laboring over how to truly influence our
> users and found there really isn’t a silver bullet. Each individual
> engineer is motivated and inspired by something different. In short, there
> is no one-size-fits-all solution here. To maximize our reach, we have come
> up with a new and imaginative solution that we call Clang Diagnostic
> Personalities.
>
> Clang Diagnostic Personalities are special modes enabled at configuration
> time that translate Clang diagnostic strings into more engaging messages
> that will reverberate with our users, we hope, inspiring them to write
> better code. Because each person is inspired by different influences we’ve
> provided a variety of personalities as part of our initial implementation,
> and we want to encourage members of the Clang community to contribute their
> own unique personalities.
>
> These diagnostic personalities allow us to reach our users on a much more
> personal and visceral level.
>
> With our new Snark personality we ensure that our users feel each failure
> with a very personal twinge. It seeks to prod our engineers by not sparing
> the stick or holding back at all.
>
> Our new Mr. T personality ensures that “The jibba jabba stops here!” and
> uses subtle intimidation to motivate engineers to learn from their mistakes
> and become better people as well as better engineers.
>
> The new Star Trek personality inspires us all to imagine a Gene
> Roddenberry future utopia, and it helps us get there by making us feel like
> we already are.
>
> Lastly da Chicago personality keeps da wiseguys in line and speaks da
> language of da people. Da Bulls!
>
> I’ve attached patches for the basic clang support as well as for the
> various personalities for review and consideration by the wider community.
>
> Thank you,
>
> -Chris
>
> CDI Working Group Members:
> Chris Bieneman
> Justin Bogner
> Pete Cooper
> Dallas De Atley
> Kevin Enderby
> Louis Gerbarg
> Jim Grosbach
> Lang Hames
> Michael Ilseman
> Nick Kledzik
> Alex Rosenberg
> Marc Schifer
>
>
> ___
> 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] D18713: [OpenCL] Generate bitcast when target address space does not change.

[Yaxun Liu via cfe-commits]

yaxunl updated the summary for this revision.
yaxunl updated this revision to Diff 52431.
yaxunl added a comment.

Use CreatePointerBitCastOrAddrSpaceCast as John suggested.


http://reviews.llvm.org/D18713

Files:
  lib/CodeGen/CGExprScalar.cpp
  test/CodeGenOpenCL/2016-04-01-addrcast.cl

Index: test/CodeGenOpenCL/2016-04-01-addrcast.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/2016-04-01-addrcast.cl
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 -cl-std=CL2.0 
-emit-llvm -o - | FileCheck %s
+
+// When -ffake-address-space-map is not used, all address space mapped to 0 
for x86/x86-64, a bitcast should be generated instead of addrspacecast.
+void foo() {
+  global int *a;
+  generic void *b = a;
+  // CHECK: bitcast 
+  // CHECK-NOT: addrspacecast
+}
Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -1411,7 +1411,10 @@
   }
   case CK_AddressSpaceConversion: {
 Value *Src = Visit(const_cast(E));
-return Builder.CreateAddrSpaceCast(Src, ConvertType(DestTy));
+// Since target may map different address spaces in AST to the same address
+// space, an address space conversion may end up as a bitcast.
+return Builder.CreatePointerBitCastOrAddrSpaceCast(Src,
+   ConvertType(DestTy));
   }
   case CK_AtomicToNonAtomic:
   case CK_NonAtomicToAtomic:


Index: test/CodeGenOpenCL/2016-04-01-addrcast.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/2016-04-01-addrcast.cl
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 -cl-std=CL2.0 -emit-llvm -o - | FileCheck %s
+
+// When -ffake-address-space-map is not used, all address space mapped to 0 for x86/x86-64, a bitcast should be generated instead of addrspacecast.
+void foo() {
+  global int *a;
+  generic void *b = a;
+  // CHECK: bitcast 
+  // CHECK-NOT: addrspacecast
+}
Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -1411,7 +1411,10 @@
   }
   case CK_AddressSpaceConversion: {
 Value *Src = Visit(const_cast(E));
-return Builder.CreateAddrSpaceCast(Src, ConvertType(DestTy));
+// Since target may map different address spaces in AST to the same address
+// space, an address space conversion may end up as a bitcast.
+return Builder.CreatePointerBitCastOrAddrSpaceCast(Src,
+   ConvertType(DestTy));
   }
   case CK_AtomicToNonAtomic:
   case CK_NonAtomicToAtomic:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18713: [OpenCL] Generate bitcast when target address space does not change.

[John McCall via cfe-commits]

rjmccall added a comment.

LGTM, thanks!


http://reviews.llvm.org/D18713



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

Re: [PATCH] D17392: Embed bitcode in object file (clang cc1 part)

[Steven Wu via cfe-commits]

steven_wu updated this revision to Diff 52435.
steven_wu added a comment.

Address the feedback from Richard:
Break -fembed-bitcode option into multiple -fembed-bitcode= options.
-fembed-bitcode=all will embed both bitcode and commandline and 
-fembed-bitcode=bitcode will embed only the bitcode in the object file.


http://reviews.llvm.org/D17392

Files:
  include/clang/CodeGen/BackendUtil.h
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CodeGenAction.cpp
  lib/Driver/Driver.cpp
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Driver/embed-bitcode.c
  test/Frontend/embed-bitcode.ll

Index: test/Frontend/embed-bitcode.ll
===
--- /dev/null
+++ test/Frontend/embed-bitcode.ll
@@ -0,0 +1,57 @@
+; check .ll input
+; RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -emit-llvm \
+; RUN:-fembed-bitcode=all -x ir %s -o - \
+; RUN:| FileCheck %s
+; RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -emit-llvm \
+; RUN:-fembed-bitcode=marker -x ir %s -o - \
+; RUN:| FileCheck %s -check-prefix=CHECK-MARKER
+; RUN: %clang_cc1 -triple aarch64-unknown-linux-gnueabi -emit-llvm \
+; RUN:-fembed-bitcode=all -x ir %s -o - \
+; RUN:| FileCheck %s -check-prefix=CHECK-ELF
+
+; check .bc input
+; RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -emit-llvm-bc \
+; RUN:-x ir %s -o %t.bc
+; RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -emit-llvm \
+; RUN:-fembed-bitcode=all -x ir %t.bc -o - \
+; RUN:| FileCheck %s
+; RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -emit-llvm \
+; RUN:-fembed-bitcode=bitcode -x ir %t.bc -o - \
+; RUN:| FileCheck %s -check-prefix=CHECK-ONLY-BITCODE
+; RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -emit-llvm \
+; RUN:-fembed-bitcode=marker -x ir %t.bc -o - \
+; RUN:| FileCheck %s -check-prefix=CHECK-MARKER
+
+; run through -fembed-bitcode twice and make sure it doesn't crash
+; RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -emit-llvm-bc \
+; RUN:-fembed-bitcode=all -x ir %s -o - \
+; RUN: | %clang_cc1 -triple thumbv7-apple-ios8.0.0 -emit-llvm \
+; RUN:-fembed-bitcode=all -x ir - -o /dev/null
+
+; check the magic number of bitcode at the beginning of the string
+; CHECK: @llvm.embedded.module
+; CHECK: c"\DE\C0\17\0B
+; CHECK: section "__LLVM,__bitcode"
+; CHECK: @llvm.cmdline
+; CHECK: section "__LLVM,__cmdline"
+
+; CHECK-ELF: @llvm.embedded.module
+; CHECK-ELF: section ".llvmbc"
+; CHECK-ELF: @llvm.cmdline
+; CHECK-ELF: section ".llvmcmd"
+
+; CHECK-ONLY-BITCODE: @llvm.embedded.module
+; CHECK-ONLY-BITCODE: c"\DE\C0\17\0B
+; CHECK-ONLY-BITCODE: section "__LLVM,__bitcode"
+; CHECK-ONLY-BITCODE-NOT: @llvm.cmdline
+; CHECK-ONLY-BITCODE-NOT: section "__LLVM,__cmdline"
+
+; CHECK-MARKER: @llvm.embedded.module
+; CHECK-MARKER: constant [0 x i8] zeroinitializer
+; CHECK-MARKER: section "__LLVM,__bitcode"
+; CHECK-MARKER: @llvm.cmdline
+; CHECK-MARKER: section "__LLVM,__cmdline"
+
+define i32 @f0() {
+  ret i32 0
+}
Index: test/Driver/embed-bitcode.c
===
--- test/Driver/embed-bitcode.c
+++ test/Driver/embed-bitcode.c
@@ -7,28 +7,35 @@
 // CHECK-CC: -emit-llvm-bc
 // CHECK-CC: -cc1
 // CHECK-CC: -emit-obj
-// CHECK-CC: -fembed-bitcode
+// CHECK-CC: -fembed-bitcode=all
 
+// RUN: %clang %s -c -fembed-bitcode=bitcode -fintegrated-as 2>&1 -### | FileCheck %s -check-prefix=CHECK-BITCODE
+// CHECK-BITCODE: -cc1
+// CHECK-BITCODE: -emit-llvm-bc
+// CHECK-BITCODE: -cc1
+// CHECK-BITCODE: -emit-obj
+// CHECK-BITCODE: -fembed-bitcode=bitcode
+//
 // RUN: %clang %s -c -save-temps -fembed-bitcode -fintegrated-as 2>&1 -### | FileCheck %s -check-prefix=CHECK-SAVE-TEMP
 // CHECK-SAVE-TEMP: -cc1
 // CHECK-SAVE-TEMP: -E
 // CHECK-SAVE-TEMP: -cc1
 // CHECK-SAVE-TEMP: -emit-llvm-bc
 // CHECK-SAVE-TEMP: -cc1
 // CHECK-SAVE-TEMP: -S
-// CHECK-SAVE-TEMP: -fembed-bitcode
+// CHECK-SAVE-TEMP: -fembed-bitcode=all
 // CHECK-SAVE-TEMP: -cc1as
 
 // RUN: %clang -c %s -flto -fembed-bitcode 2>&1 -### | FileCheck %s -check-prefix=CHECK-LTO
 // CHECK-LTO: -cc1
 // CHECK-LTO: -emit-llvm-bc
 // CHECK-LTO-NOT: warning: argument unused during compilation: '-fembed-bitcode'
 // CHECK-LTO-NOT: -cc1
-// CHECK-LTO-NOT: -fembed-bitcode
+// CHECK-LTO-NOT: -fembed-bitcode=all
 
 // RUN: %clang -c %s -fembed-bitcode-marker -fintegrated-as 2>&1 -### | FileCheck %s -check-prefix=CHECK-MARKER
 // CHECK-MARKER: -cc1
 // CHECK-MARKER: -emit-obj
-// CHECK-MARKER: -fembed-bitcode-marker
+// CHECK-MARKER: -fembed-bitcode=marker
 // CHECK-MARKER-NOT: -cc1
 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -646,6 +646,47 @@
   }
 }
   }
+	// Handle -fembed-bitcode option.
+  if (Arg *A = Args.getLa

r265195 - [modules] Start moving the code for encoding AST records out of ASTWriter into

[Richard Smith via cfe-commits]

Author: rsmith
Date: Fri Apr  1 17:52:03 2016
New Revision: 265195

URL: http://llvm.org/viewvc/llvm-project?rev=265195&view=rev
Log:
[modules] Start moving the code for encoding AST records out of ASTWriter into
a separate class. The goal is for this class to have a separate lifetime from
the AST writer so that it can meaningfully track pending statement nodes and
context for more compact encoding of various types.

Modified:
cfe/trunk/include/clang/Serialization/ASTWriter.h
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp

Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=265195&r1=265194&r2=265195&view=diff
==
--- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTWriter.h Fri Apr  1 17:52:03 2016
@@ -90,6 +90,7 @@ public:
 
   friend class ASTDeclWriter;
   friend class ASTStmtWriter;
+  friend class ASTRecordWriter;
 private:
   /// \brief Map that provides the ID numbers of each type within the
   /// output stream, plus those deserialized from a chained PCH.
@@ -523,7 +524,6 @@ private:
   void WriteReferencedSelectorsPool(Sema &SemaRef);
   void WriteIdentifierTable(Preprocessor &PP, IdentifierResolver &IdResolver,
 bool IsModule);
-  void WriteAttributes(ArrayRef Attrs, RecordDataImpl &Record);
   void WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord);
   void WriteDeclContextVisibleUpdate(const DeclContext *DC);
   void WriteFPPragmaOptions(const FPOptions &Opts);
@@ -555,7 +555,7 @@ private:
 
   void WriteDeclAbbrevs();
   void WriteDecl(ASTContext &Context, Decl *D);
-  void AddFunctionDefinition(const FunctionDecl *FD, RecordData &Record);
+  void AddFunctionDefinition(const FunctionDecl *FD, RecordDataImpl &Record);
 
   uint64_t WriteASTCore(Sema &SemaRef,
 StringRef isysroot, const std::string &OutputFile,
@@ -684,6 +684,8 @@ public:
   /// declaration.
   serialization::DeclID getDeclID(const Decl *D);
 
+  void AddAttributes(ArrayRef Attrs, RecordDataImpl &Record);
+
   /// \brief Emit a declaration name.
   void AddDeclarationName(DeclarationName Name, RecordDataImpl &Record);
   void AddDeclarationNameLoc(const DeclarationNameLoc &DNLoc,
@@ -864,6 +866,219 @@ public:
   const RecordDecl *Record) override;
 };
 
+/// \brief An object for streaming information to a record.
+class ASTRecordWriter {
+  ASTWriter *Writer;
+  ASTWriter::RecordDataImpl *Record;
+
+public:
+  /// Construct a ASTRecordWriter that uses the default encoding scheme.
+  ASTRecordWriter(ASTWriter &Writer, ASTWriter::RecordDataImpl &Record)
+  : Writer(&Writer), Record(&Record) {}
+
+  /// Construct a ASTRecordWriter that uses the same encoding scheme as another
+  /// ASTRecordWriter.
+  ASTRecordWriter(ASTRecordWriter &Parent, ASTWriter::RecordDataImpl &Record)
+  : Writer(Parent.Writer), Record(&Record) {}
+
+  /// \brief Extract the underlying record storage.
+  ASTWriter::RecordDataImpl &getRecordData() const { return *Record; }
+
+  /// \brief Minimal vector-like interface.
+  /// @{
+  void push_back(uint64_t N) { Record->push_back(N); }
+  template
+  void append(InputIterator begin, InputIterator end) {
+Record->append(begin, end);
+  }
+  bool empty() const { return Record->empty(); }
+  size_t size() const { return Record->size(); }
+  uint64_t &operator[](size_t N) { return (*Record)[N]; }
+  /// @}
+
+
+  /// \brief Emit the record to the stream, and return its offset.
+  // FIXME: Allow record producers to suggest Abbrevs.
+  uint64_t Emit(unsigned Code, unsigned Abbrev = 0) {
+uint64_t Offset = Writer->Stream.GetCurrentBitNo();
+Writer->Stream.EmitRecord(Code, *Record);
+return Offset;
+  }
+
+
+  /// \brief Emit a source location.
+  void AddSourceLocation(SourceLocation Loc) {
+return Writer->AddSourceLocation(Loc, *Record);
+  }
+
+  /// \brief Emit a source range.
+  void AddSourceRange(SourceRange Range) {
+return Writer->AddSourceRange(Range, *Record);
+  }
+
+  /// \brief Emit an integral value.
+  void AddAPInt(const llvm::APInt &Value) {
+return Writer->AddAPInt(Value, *Record);
+  }
+
+  /// \brief Emit a signed integral value.
+  void AddAPSInt(const llvm::APSInt &Value) {
+return Writer->AddAPSInt(Value, *Record);
+  }
+
+  /// \brief Emit a floating-point value.
+  void AddAPFloat(const llvm::APFloat &Value) {
+return Writer->AddAPFloat(Value, *Record);
+  }
+
+  /// \brief Emit a reference to an identifier.
+  void AddIdentifierRef(const IdentifierInfo *II) {
+return Writer->AddIdentifierRef(II, *Record);
+  }
+
+  /// \brief Emit a Selector (which is a smart pointer reference).
+  void AddSelectorRef(Selector S) {
+return 

r265197 - [CodeGen] Emit lifetime.end intrinsic after objects are destructed in

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Fri Apr  1 17:58:55 2016
New Revision: 265197

URL: http://llvm.org/viewvc/llvm-project?rev=265197&view=rev
Log:
[CodeGen] Emit lifetime.end intrinsic after objects are destructed in
landing pads.

Previously, lifetime.end intrinsics were inserted only on normal control
flows. This prevented StackColoring from merging stack slots for objects
that were destroyed on the exception handling control flow since it
couldn't tell their lifetime ranges were disjoint. This patch fixes
code-gen to emit the intrinsic on both control flows.

rdar://problem/22181976

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

Modified:
cfe/trunk/lib/CodeGen/CGCleanup.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/EHScopeStack.h
cfe/trunk/test/CodeGenCXX/destructors.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp

Modified: cfe/trunk/lib/CodeGen/CGCleanup.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCleanup.cpp?rev=265197&r1=265196&r2=265197&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCleanup.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCleanup.cpp Fri Apr  1 17:58:55 2016
@@ -157,6 +157,20 @@ bool EHScopeStack::containsOnlyLifetimeM
   return true;
 }
 
+bool EHScopeStack::requiresLandingPad() const {
+  for (stable_iterator si = getInnermostEHScope(); si != stable_end(); ) {
+// Skip lifetime markers.
+if (auto *cleanup = dyn_cast(&*find(si)))
+  if (cleanup->isLifetimeMarker()) {
+si = cleanup->getEnclosingEHScope();
+continue;
+  }
+return true;
+  }
+
+  return false;
+}
+
 EHScopeStack::stable_iterator
 EHScopeStack::getInnermostActiveNormalCleanup() const {
   for (stable_iterator si = getInnermostNormalCleanup(), se = stable_end();

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=265197&r1=265196&r2=265197&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Fri Apr  1 17:58:55 2016
@@ -1388,7 +1388,7 @@ void CodeGenFunction::EmitAutoVarCleanup
   // Make sure we call @llvm.lifetime.end.  This needs to happen
   // *last*, so the cleanup needs to be pushed *first*.
   if (emission.useLifetimeMarkers()) {
-EHStack.pushCleanup(NormalCleanup,
+EHStack.pushCleanup(NormalAndEHCleanup,
  emission.getAllocatedAddress(),
  emission.getSizeForLifetimeMarkers());
 EHCleanupScope &cleanup = cast(*EHStack.begin());

Modified: cfe/trunk/lib/CodeGen/EHScopeStack.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/EHScopeStack.h?rev=265197&r1=265196&r2=265197&view=diff
==
--- cfe/trunk/lib/CodeGen/EHScopeStack.h (original)
+++ cfe/trunk/lib/CodeGen/EHScopeStack.h Fri Apr  1 17:58:55 2016
@@ -341,9 +341,7 @@ public:
   /// Determines whether the exception-scopes stack is empty.
   bool empty() const { return StartOfData == EndOfBuffer; }
 
-  bool requiresLandingPad() const {
-return InnermostEHScope != stable_end();
-  }
+  bool requiresLandingPad() const;
 
   /// Determines whether there are any normal cleanups on the stack.
   bool hasNormalCleanups() const {

Modified: cfe/trunk/test/CodeGenCXX/destructors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/destructors.cpp?rev=265197&r1=265196&r2=265197&view=diff
==
--- cfe/trunk/test/CodeGenCXX/destructors.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/destructors.cpp Fri Apr  1 17:58:55 2016
@@ -4,6 +4,9 @@
 // RUN: FileCheck --check-prefix=CHECK3 --input-file=%t %s
 // RUN: FileCheck --check-prefix=CHECK4 --input-file=%t %s
 // RUN: FileCheck --check-prefix=CHECK5 --input-file=%t %s
+// RUN: %clang_cc1 %s -triple x86_64-apple-darwin10 -emit-llvm -o - 
-fcxx-exceptions -fexceptions -O1 -disable-llvm-optzns -std=c++11 > %t2
+// RUN: FileCheck --check-prefix=CHECK6 --input-file=%t2 %s
+// REQUIRES: asserts
 
 struct A {
   int a;
@@ -428,3 +431,64 @@ namespace test10 {
 return true;
   }
 }
+
+#if __cplusplus >= 201103L
+namespace test11 {
+
+// Check that lifetime.end is emitted in the landing pad.
+
+// CHECK6-LABEL: define void @_ZN6test1115testLifetimeEndEi(
+// CHECK6: entry:
+// CHECK6: [[T1:%[a-z0-9]+]] = alloca %"struct.test11::S1"
+// CHECK6: [[T2:%[a-z0-9]+]] = alloca %"struct.test11::S1"
+// CHECK6: [[T3:%[a-z0-9]+]] = alloca %"struct.test11::S1"
+
+// CHECK6: {{^}}invoke.cont
+// CHECK6: call void @_ZN6test112S1D1Ev(%"struct.test11::S1"* [[T1]])
+// CHECK6: [[BC1:%[a-z0-9]+]] = bitcast %"struct.test11::S1"* [[T1]] to i8*
+// CHECK6: call void @llvm.lifetime.end(i64 32, i8* [[BC1]])
+// CHECK6: {{^}}lpa

Re: [PATCH] D18196: [CodeGen] Emit lifetime.end intrinsic after destructor call in landing pad

[Akira Hatanaka via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL265197: [CodeGen] Emit lifetime.end intrinsic after objects 
are destructed in (authored by ahatanak).

Changed prior to commit:
  http://reviews.llvm.org/D18196?vs=52110&id=52440#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D18196

Files:
  cfe/trunk/lib/CodeGen/CGCleanup.cpp
  cfe/trunk/lib/CodeGen/CGDecl.cpp
  cfe/trunk/lib/CodeGen/EHScopeStack.h
  cfe/trunk/test/CodeGenCXX/destructors.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp

Index: cfe/trunk/test/CodeGenCXX/destructors.cpp
===
--- cfe/trunk/test/CodeGenCXX/destructors.cpp
+++ cfe/trunk/test/CodeGenCXX/destructors.cpp
@@ -4,6 +4,9 @@
 // RUN: FileCheck --check-prefix=CHECK3 --input-file=%t %s
 // RUN: FileCheck --check-prefix=CHECK4 --input-file=%t %s
 // RUN: FileCheck --check-prefix=CHECK5 --input-file=%t %s
+// RUN: %clang_cc1 %s -triple x86_64-apple-darwin10 -emit-llvm -o - -fcxx-exceptions -fexceptions -O1 -disable-llvm-optzns -std=c++11 > %t2
+// RUN: FileCheck --check-prefix=CHECK6 --input-file=%t2 %s
+// REQUIRES: asserts
 
 struct A {
   int a;
@@ -428,3 +431,64 @@
 return true;
   }
 }
+
+#if __cplusplus >= 201103L
+namespace test11 {
+
+// Check that lifetime.end is emitted in the landing pad.
+
+// CHECK6-LABEL: define void @_ZN6test1115testLifetimeEndEi(
+// CHECK6: entry:
+// CHECK6: [[T1:%[a-z0-9]+]] = alloca %"struct.test11::S1"
+// CHECK6: [[T2:%[a-z0-9]+]] = alloca %"struct.test11::S1"
+// CHECK6: [[T3:%[a-z0-9]+]] = alloca %"struct.test11::S1"
+
+// CHECK6: {{^}}invoke.cont
+// CHECK6: call void @_ZN6test112S1D1Ev(%"struct.test11::S1"* [[T1]])
+// CHECK6: [[BC1:%[a-z0-9]+]] = bitcast %"struct.test11::S1"* [[T1]] to i8*
+// CHECK6: call void @llvm.lifetime.end(i64 32, i8* [[BC1]])
+// CHECK6: {{^}}lpad
+// CHECK6: call void @_ZN6test112S1D1Ev(%"struct.test11::S1"* [[T1]])
+// CHECK6: [[BC2:%[a-z0-9]+]] = bitcast %"struct.test11::S1"* [[T1]] to i8*
+// CHECK6: call void @llvm.lifetime.end(i64 32, i8* [[BC2]])
+
+// CHECK6: {{^}}invoke.cont
+// CHECK6: call void @_ZN6test112S1D1Ev(%"struct.test11::S1"* [[T2]])
+// CHECK6: [[BC3:%[a-z0-9]+]] = bitcast %"struct.test11::S1"* [[T2]] to i8*
+// CHECK6: call void @llvm.lifetime.end(i64 32, i8* [[BC3]])
+// CHECK6: {{^}}lpad
+// CHECK6: call void @_ZN6test112S1D1Ev(%"struct.test11::S1"* [[T2]])
+// CHECK6: [[BC4:%[a-z0-9]+]] = bitcast %"struct.test11::S1"* [[T2]] to i8*
+// CHECK6: call void @llvm.lifetime.end(i64 32, i8* [[BC4]])
+
+// CHECK6: {{^}}invoke.cont
+// CHECK6: call void @_ZN6test112S1D1Ev(%"struct.test11::S1"* [[T3]])
+// CHECK6: [[BC5:%[a-z0-9]+]] = bitcast %"struct.test11::S1"* [[T3]] to i8*
+// CHECK6: call void @llvm.lifetime.end(i64 32, i8* [[BC5]])
+// CHECK6: {{^}}lpad
+// CHECK6: call void @_ZN6test112S1D1Ev(%"struct.test11::S1"* [[T3]])
+// CHECK6: [[BC6:%[a-z0-9]+]] = bitcast %"struct.test11::S1"* [[T3]] to i8*
+// CHECK6: call void @llvm.lifetime.end(i64 32, i8* [[BC6]])
+
+  struct S1 {
+~S1();
+int a[8];
+  };
+
+  void func1(S1 &) noexcept(false);
+
+  void testLifetimeEnd(int n) {
+if (n < 10) {
+  S1 t1;
+  func1(t1);
+} else if (n < 100) {
+  S1 t2;
+  func1(t2);
+} else if (n < 1000) {
+  S1 t3;
+  func1(t3);
+}
+  }
+
+}
+#endif
Index: cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
===
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple=i386-pc-win32 -mconstructor-aliases -fexceptions -fcxx-exceptions -fno-rtti | FileCheck -check-prefix WIN32 %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -O3 -disable-llvm-optzns %s -o - -triple=i386-pc-win32 -mconstructor-aliases -fexceptions -fcxx-exceptions -fno-rtti | FileCheck -check-prefix WIN32 -check-prefix WIN32-LIFETIME %s
 
 struct A {
   A();
@@ -206,3 +207,36 @@
 // WIN32: cleanuppad
 // WIN32: call x86_thiscallcc void @"\01??1D@noexcept_false_dtor@@QAE@XZ"(%"struct.noexcept_false_dtor::D"* %{{.*}})
 // WIN32: cleanupret
+
+namespace lifetime_marker {
+struct C {
+  ~C();
+};
+void g();
+void f() {
+  C c;
+  g();
+}
+
+// WIN32-LIFETIME-LABEL: define void @"\01?f@lifetime_marker@@YAXXZ"()
+// WIN32-LIFETIME: %[[c:.*]] = alloca %"struct.lifetime_marker::C"
+// WIN32-LIFETIME: %[[bc0:.*]] = bitcast %"struct.lifetime_marker::C"* %c to i8*
+// WIN32-LIFETIME: call void @llvm.lifetime.start(i64 1, i8* %[[bc0]])
+// WIN32-LIFETIME: invoke void @"\01?g@lifetime_marker@@YAXXZ"()
+// WIN32-LIFETIME-NEXT: to label %[[cont:[^ ]*]] unwind label %[[lpad0:[^ ]*]]
+//
+// WIN32-LIFETIME: [[cont]]:
+// WIN32-LIFETIME: call x86_thiscallcc void @"\01??1C@lifetime_marker@@QAE@XZ"({{.*}})
+// WIN32-LIFETIME: %[[bc1:.*]] = bitcast %"struct.lifetime_marker::C"* %[[c]] to i8*
+// WIN32-LIFETIME: 

[PATCH] D18708: Set C99 as default C Standard for PS4 target

[Yung, Douglas via cfe-commits]

Forwarding to cfe-commits as I don't think I saw it appear there.

> dyung created this revision.
> 
> On the PS4, the default C standard is C99 which differs from the
> current default of C11. This patch makes the default C99 when targeting
> the PS4.
> 
> http://reviews.llvm.org/D18708
> 
> Files:
>   include/clang/Frontend/CompilerInvocation.h
>   lib/Frontend/CompilerInvocation.cpp
>   test/Driver/ps4-cpu-defaults.cpp
>   test/Driver/ps4-misc-defaults.cpp
> 
> Index: test/Driver/ps4-misc-defaults.cpp
> ===
> --- test/Driver/ps4-misc-defaults.cpp
> +++ test/Driver/ps4-misc-defaults.cpp
> @@ -4,3 +4,7 @@
>  // RUN: %clang -target x86_64-scei-ps4 -c %s -### 2>&1 | FileCheck %s
> // CHECK: "-target-cpu" "btver2"
>  // CHECK-NOT: exceptions
> +
> +// Check that the PS4 defaults to C99 when compiling C files // RUN:
> +%clang -target x86_64-scei-ps4 -E -x c -dM %s | FileCheck
> +-check-prefix=CHECK-CSTD %s // CHECK-CSTD: __STDC_VERSION__ 199901L
> Index: test/Driver/ps4-cpu-defaults.cpp
> ===
> --- test/Driver/ps4-cpu-defaults.cpp
> +++ test/Driver/ps4-cpu-defaults.cpp
> @@ -1,6 +0,0 @@
> -// Check that on the PS4 we default to:
> -// -target-cpu btver2 and no exceptions
> -
> -// RUN: %clang -target x86_64-scei-ps4 -c %s -### 2>&1 | FileCheck %s
> -// CHECK: "-target-cpu" "btver2"
> -// CHECK-NOT: exceptions
> Index: lib/Frontend/CompilerInvocation.cpp
> ===
> --- lib/Frontend/CompilerInvocation.cpp
> +++ lib/Frontend/CompilerInvocation.cpp
> @@ -1355,6 +1355,7 @@
>  }
> 
>  void CompilerInvocation::setLangDefaults(LangOptions &Opts, InputKind
> IK,
> + const llvm::Triple &T,
>   LangStandard::Kind LangStd) {
>// Set some properties which depend solely on the input kind; it
> would be nice
>// to move these to the language standard, and have the driver
> resolve the @@ -1387,7 +1388,11 @@
>  case IK_PreprocessedC:
>  case IK_ObjC:
>  case IK_PreprocessedObjC:
> -  LangStd = LangStandard::lang_gnu11;
> +  // The PS4 uses C99 as the default C standard.
> +  if (T.isPS4())
> +LangStd = LangStandard::lang_gnu99;
> +  else
> +LangStd = LangStandard::lang_gnu11;
>break;
>  case IK_CXX:
>  case IK_PreprocessedCXX:
> @@ -1541,7 +1546,8 @@
>LangStd = OpenCLLangStd;
>}
> 
> -  CompilerInvocation::setLangDefaults(Opts, IK, LangStd);
> +  llvm::Triple T(TargetOpts.Triple);
> +  CompilerInvocation::setLangDefaults(Opts, IK, T, LangStd);
> 
>// We abuse '-f[no-]gnu-keywords' to force overriding all GNU-
> extension
>// keywords. This behavior is provided by GCC's poorly named '-fasm'
> flag, @@ -1858,7 +1864,6 @@
>// Provide diagnostic when a given target is not expected to be an
> OpenMP
>// device or host.
>if (Opts.OpenMP && !Opts.OpenMPIsDevice) {
> -llvm::Triple T(TargetOpts.Triple);
>  switch (T.getArch()) {
>  default:
>break;
> Index: include/clang/Frontend/CompilerInvocation.h
> ===
> --- include/clang/Frontend/CompilerInvocation.h
> +++ include/clang/Frontend/CompilerInvocation.h
> @@ -153,8 +153,10 @@
>///
>/// \param Opts - The LangOptions object to set up.
>/// \param IK - The input language.
> +  /// \param T - The target triple.
>/// \param LangStd - The input language standard.
>static void setLangDefaults(LangOptions &Opts, InputKind IK,
> +   const llvm::Triple &T,
> LangStandard::Kind LangStd =
> LangStandard::lang_unspecified);
> 
>/// \brief Retrieve a module hash string that is suitable for
> uniquely
> 

Index: test/Driver/ps4-misc-defaults.cpp
===
--- test/Driver/ps4-misc-defaults.cpp
+++ test/Driver/ps4-misc-defaults.cpp
@@ -4,3 +4,7 @@
 // RUN: %clang -target x86_64-scei-ps4 -c %s -### 2>&1 | FileCheck %s
 // CHECK: "-target-cpu" "btver2"
 // CHECK-NOT: exceptions
+
+// Check that the PS4 defaults to C99 when compiling C files
+// RUN: %clang -target x86_64-scei-ps4 -E -x c -dM %s | FileCheck -check-prefix=CHECK-CSTD %s
+// CHECK-CSTD: __STDC_VERSION__ 199901L
Index: test/Driver/ps4-cpu-defaults.cpp
===
--- test/Driver/ps4-cpu-defaults.cpp
+++ test/Driver/ps4-cpu-defaults.cpp
@@ -1,6 +0,0 @@
-// Check that on the PS4 we default to:
-// -target-cpu btver2 and no exceptions
-
-// RUN: %clang -target x86_64-scei-ps4 -c %s -### 2>&1 | FileCheck %s
-// CHECK: "-target-cpu" "btver2"
-// CHECK-NOT: exceptions
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Front

[PATCH] D18717: [Clang-tidy] Improve checks documentation consistency

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko created this revision.
Eugene.Zelenko added reviewers: alexfh, aaron.ballman, LegalizeAdulthood.
Eugene.Zelenko added a subscriber: cfe-commits.
Eugene.Zelenko set the repository for this revision to rL LLVM.

I tried to make using of `` and ` more consistent. Two Google checks changed to 
aliases.

I'm not sure about using :option: for check options.

Repository:
  rL LLVM

http://reviews.llvm.org/D18717

Files:
  docs/clang-tidy/checks/cert-err52-cpp.rst
  docs/clang-tidy/checks/cert-err58-cpp.rst
  docs/clang-tidy/checks/cert-flp30-c.rst
  docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
  docs/clang-tidy/checks/google-build-explicit-make-pair.rst
  docs/clang-tidy/checks/google-build-namespaces.rst
  docs/clang-tidy/checks/google-build-using-namespace.rst
  docs/clang-tidy/checks/google-global-names-in-headers.rst
  docs/clang-tidy/checks/google-readability-braces-around-statements.rst
  docs/clang-tidy/checks/google-readability-casting.rst
  docs/clang-tidy/checks/google-readability-function-size.rst
  docs/clang-tidy/checks/google-readability-todo.rst
  docs/clang-tidy/checks/google-runtime-int.rst
  docs/clang-tidy/checks/google-runtime-member-string-references.rst
  docs/clang-tidy/checks/google-runtime-memset.rst
  docs/clang-tidy/checks/google-runtime-operator.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-assert-side-effect.rst
  docs/clang-tidy/checks/misc-bool-pointer-implicit-conversion.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-suspicious-semicolon.rst
  docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
  docs/clang-tidy/checks/misc-unused-parameters.rst
  docs/clang-tidy/checks/modernize-loop-convert.rst
  docs/clang-tidy/checks/modernize-pass-by-value.rst
  docs/clang-tidy/checks/modernize-use-auto.rst
  docs/clang-tidy/checks/modernize-use-nullptr.rst
  docs/clang-tidy/checks/performance-implicit-cast-in-loop.rst
  docs/clang-tidy/checks/readability-braces-around-statements.rst
  docs/clang-tidy/checks/readability-function-size.rst
  docs/clang-tidy/checks/readability-identifier-naming.rst
  docs/clang-tidy/checks/readability-named-parameter.rst

Index: docs/clang-tidy/checks/google-readability-todo.rst
===
--- docs/clang-tidy/checks/google-readability-todo.rst
+++ docs/clang-tidy/checks/google-readability-todo.rst
@@ -3,7 +3,6 @@
 google-readability-todo
 ===
 
-
 Finds TODO comments without a username or bug number.
 
-Corresponding cpplint.py check: 'readability/todo'
+Corresponding cpplint.py check: `readability/todo`
Index: docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
===
--- docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
+++ docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
@@ -8,9 +8,8 @@
 are out of bounds (for ``std::array``). For out-of-bounds checking of static
 arrays, see the clang-diagnostic-array-bounds check.
 
-The check can generate fixes after the option
-``cppcoreguidelines-pro-bounds-constant-array-index.GslHeader`` has been set to
-the name of the include file that contains ``gsl::at()``, e.g. ``"gsl/gsl.h"``.
+The check can generate fixes after the option :option:`GslHeader` has been set
+to the name of the include file that contains ``gsl::at()``, e.g. `"gsl/gsl.h"`.
 
 This rule is part of the "Bounds safety" profile of the C++ Core Guidelines, see
 https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Pro-bounds-arrayindex.
Index: docs/clang-tidy/checks/cert-flp30-c.rst
===
--- docs/clang-tidy/checks/cert-flp30-c.rst
+++ docs/clang-tidy/checks/cert-flp30-c.rst
@@ -3,8 +3,8 @@
 cert-flp30-c
 
 
-This check flags ``for`` loops where the induction expression has a floating-
-point type.
+This check flags ``for`` loops where the induction expression has a
+floating-point type.
 
 This check corresponds to the CERT C Coding Standard rule
 `FLP30-C. Do not use floating-point variables as loop counters
Index: docs/clang-tidy/checks/misc-bool-pointer-implicit-conversion.rst
===
--- docs/clang-tidy/checks/misc-bool-pointer-implicit-conversion.rst
+++ docs/clang-tidy/checks/misc-bool-pointer-implicit-conversion.rst
@@ -3,10 +3,9 @@
 misc-bool-pointer-implicit-conversion
 =
 
+Checks for conditions based on implicit conversion from a ``bool`` pointer to
+``bool``.
 
-Checks for conditions based on implicit conversion from a bool pointer to
-bool.
-
 Example:
 
 .. code:: c++
Index: docs/clang-tidy/checks/google-build-name

r265201 - [Objective-C] Introduce objc_runtime_visible attribute.

[Douglas Gregor via cfe-commits]

Author: dgregor
Date: Fri Apr  1 18:23:52 2016
New Revision: 265201

URL: http://llvm.org/viewvc/llvm-project?rev=265201&view=rev
Log:
[Objective-C] Introduce objc_runtime_visible attribute.

The objc_runtime_visible attribute deals with an odd corner case where
a particular Objective-C class is known to the Objective-C runtime
(and, therefore, accessible by name) but its symbol has been hidden
for some reason. For such classes, teach CodeGen to use
objc_lookUpClass to retrieve the Class object, rather than referencing
the class symbol directly.

Classes annotated with objc_runtime_visible have two major limitations
that fall out from places where Objective-C metadata needs to refer to
the class (or metaclass) symbol directly:

* One cannot implement a subclass of an objc_runtime_visible class.
* One cannot implement a category on an objc_runtime_visible class.

Implements rdar://problem/25494092.

Added:
cfe/trunk/test/CodeGenObjC/attr-objc-runtime-visible.m
cfe/trunk/test/SemaObjC/attr-objc-runtime-visible.m
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/CodeGen/CGObjCMac.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaDeclObjC.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=265201&r1=265200&r2=265201&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Fri Apr  1 18:23:52 2016
@@ -1241,6 +1241,12 @@ def ObjCRuntimeName : Attr {
   let Documentation = [ObjCRuntimeNameDocs];
 }
 
+def ObjCRuntimeVisible : Attr {
+  let Spellings = [GNU<"objc_runtime_visible">];
+  let Subjects = SubjectList<[ObjCInterface], ErrorDiag>;
+  let Documentation = [ObjCRuntimeVisibleDocs];
+}
+
 def ObjCBoxable : Attr {
   let Spellings = [GNU<"objc_boxable">];
   let Subjects = SubjectList<[Record], ErrorDiag, "ExpectedStructOrUnion">;

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=265201&r1=265200&r2=265201&view=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Fri Apr  1 18:23:52 2016
@@ -612,6 +612,13 @@ can only be placed before an @protocol o
 }];
 }
 
+def ObjCRuntimeVisibleDocs : Documentation {
+let Category = DocCatFunction;
+let Content = [{
+This attribute specifies that the Objective-C class to which it applies is 
visible to the Objective-C runtime but not to the linker. Classes annotated 
with this attribute cannot be subclassed and cannot have categories defined for 
them.
+}];
+}
+
 def ObjCBoxableDocs : Documentation {
 let Category = DocCatFunction;
 let Content = [{

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=265201&r1=265200&r2=265201&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Apr  1 18:23:52 
2016
@@ -715,6 +715,12 @@ def err_objc_root_class_subclass : Error
 def warn_objc_root_class_missing : Warning<
   "class %0 defined without specifying a base class">,
   InGroup;
+def err_objc_runtime_visible_category : Error<
+  "cannot implement a category for class %0 that is only visible via the "
+  "Objective-C runtime">;
+def err_objc_runtime_visible_subclass : Error<
+  "cannot implement subclass %0 of a superclass %1 that is only visible via 
the "
+  "Objective-C runtime">;
 def note_objc_needs_superclass : Note<
   "add a super class to fix this problem">;
 def warn_dup_category_def : Warning<

Modified: cfe/trunk/lib/CodeGen/CGObjCMac.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCMac.cpp?rev=265201&r1=265200&r2=265201&view=diff
==
--- cfe/trunk/lib/CodeGen/CGObjCMac.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjCMac.cpp Fri Apr  1 18:23:52 2016
@@ -349,6 +349,20 @@ public:
 return CGM.CreateRuntimeFunction(FTy, "objc_enumerationMutation");
   }
 
+  llvm::Constant *getLookUpClassFn() {
+CodeGen::CodeGenTypes &Types = CGM.getTypes();
+ASTContext &Ctx = CGM.getContext();
+// Class objc_lookUpClass (const char *)
+SmallVector Params;
+Params.push_back(
+  Ctx.getCanonicalType(Ctx.getPointerType(Ctx.CharTy.withConst(;
+llvm::FunctionType *FTy =
+Types.GetFunctionType(Types.arrangeBuiltinFunctionDeclaration(
+Ctx.getCanonical

Re: [PATCH] D18073: Add memory allocating functions

[Alexander Riccio via cfe-commits]

ariccio added a comment.

I'm sorry, I'm confused.

As an example, for `_wcsdup_dbg`, I've added `testWinWcsdupDbg`, 
`testWinWcsdupDbgContentIsDefined`, and `testWinWideDbgLeakWithinReturn`. Which 
ones should I drop for that API?


http://reviews.llvm.org/D18073



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

Re: r263191 - Preserve ExtParameterInfos into CGFunctionInfo.

[John McCall via cfe-commits]

> On Apr 1, 2016, at 3:50 PM, Nico Weber  wrote:
> On Thu, Mar 10, 2016 at 8:30 PM, John McCall via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Author: rjmccall
> Date: Thu Mar 10 22:30:31 2016
> New Revision: 263191
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=263191&view=rev 
> 
> Log:
> Preserve ExtParameterInfos into CGFunctionInfo.
> 
> As part of this, make the function-arrangement interfaces
> a little simpler and more semantic.
> 
> NFC.
> 
> Modified:
> cfe/trunk/include/clang/AST/CanonicalType.h
> cfe/trunk/include/clang/CodeGen/CGFunctionInfo.h
> cfe/trunk/lib/CodeGen/CGAtomic.cpp
> cfe/trunk/lib/CodeGen/CGBlocks.cpp
> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> cfe/trunk/lib/CodeGen/CGCall.cpp
> cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
> cfe/trunk/lib/CodeGen/CGException.cpp
> cfe/trunk/lib/CodeGen/CGExpr.cpp
> cfe/trunk/lib/CodeGen/CGObjC.cpp
> cfe/trunk/lib/CodeGen/CGObjCMac.cpp
> cfe/trunk/lib/CodeGen/CGObjCRuntime.cpp
> cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
> cfe/trunk/lib/CodeGen/CGStmt.cpp
> cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
> cfe/trunk/lib/CodeGen/CodeGenABITypes.cpp
> cfe/trunk/lib/CodeGen/CodeGenTypes.h
> cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
> 
> Modified: cfe/trunk/include/clang/AST/CanonicalType.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CanonicalType.h?rev=263191&r1=263190&r2=263191&view=diff
>  
> 
> ==
> --- cfe/trunk/include/clang/AST/CanonicalType.h (original)
> +++ cfe/trunk/include/clang/AST/CanonicalType.h Thu Mar 10 22:30:31 2016
> @@ -484,6 +484,9 @@ struct CanProxyAdaptorLLVM_CLANG_CANPROXY_TYPE_ACCESSOR(getReturnType)
>LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(FunctionType::ExtInfo, getExtInfo)
>LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(unsigned, getNumParams)
> +  LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(bool, hasExtParameterInfos)
> +  LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(
> +ArrayRef, 
> getExtParameterInfos)
>CanQualType getParamType(unsigned i) const {
>  return CanQualType::CreateUnsafe(this->getTypePtr()->getParamType(i));
>}
> 
> Modified: cfe/trunk/include/clang/CodeGen/CGFunctionInfo.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CodeGen/CGFunctionInfo.h?rev=263191&r1=263190&r2=263191&view=diff
>  
> 
> ==
> --- cfe/trunk/include/clang/CodeGen/CGFunctionInfo.h (original)
> +++ cfe/trunk/include/clang/CodeGen/CGFunctionInfo.h Thu Mar 10 22:30:31 2016
> @@ -343,8 +343,10 @@ struct CGFunctionInfoArgInfo {
>  /// function definition.
>  class CGFunctionInfo final
>  : public llvm::FoldingSetNode,
> -  private llvm::TrailingObjects {
> +  private llvm::TrailingObjects +FunctionProtoType::ExtParameterInfo> {
>typedef CGFunctionInfoArgInfo ArgInfo;
> +  typedef FunctionProtoType::ExtParameterInfo ExtParameterInfo;
> 
>/// The LLVM::CallingConv to use for this function (as specified by the
>/// user).
> @@ -378,7 +380,8 @@ class CGFunctionInfo final
>/// The struct representing all arguments passed in memory.  Only used when
>/// passing non-trivial types with inalloca.  Not part of the profile.
>llvm::StructType *ArgStruct;
> -  unsigned ArgStructAlign;
> +  unsigned ArgStructAlign : 31;
> +  unsigned HasExtParameterInfos : 1;
> 
>unsigned NumArgs;
> 
> @@ -389,7 +392,19 @@ class CGFunctionInfo final
>  return getTrailingObjects();
>}
> 
> -  size_t numTrailingObjects(OverloadToken) { return NumArgs + 1; }
> +  ExtParameterInfo *getExtParameterInfosBuffer() {
> +return getTrailingObjects();
> +  }
> +  const ExtParameterInfo *getExtParameterInfosBuffer() const{
> +return getTrailingObjects();
> +  }
> +
> +  size_t numTrailingObjects(OverloadToken) const {
> +return NumArgs + 1;
> +  }
> +  size_t numTrailingObjects(OverloadToken) const {
> +return (HasExtParameterInfos ? NumArgs : 0);
> +  }
>friend class TrailingObjects;
> 
>CGFunctionInfo() : Required(RequiredArgs::All) {}
> @@ -399,6 +414,7 @@ public:
>  bool instanceMethod,
>  bool chainCall,
>  const FunctionType::ExtInfo &extInfo,
> +ArrayRef paramInfos,
>  CanQualType resultType,
>  ArrayRef argTypes,
>  RequiredArgs required);
> @@ -472,6 +488,16 @@ public:
>ABIArg

Re: [PATCH] D18708: Set C99 as default C Standard for PS4 target

[Yunzhong Gao via cfe-commits]

ygao added a subscriber: ygao.


Comment at: test/Driver/ps4-misc-defaults.cpp:10
@@ +9,2 @@
+// RUN: %clang -target x86_64-scei-ps4 -E -x c -dM %s | FileCheck 
-check-prefix=CHECK-CSTD %s
+// CHECK-CSTD: __STDC_VERSION__ 199901L

It seems to me that this part of the test, exercising "-E", should be placed 
under test/Preprocessor, instead of test/Driver.


http://reviews.llvm.org/D18708



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

Re: [PATCH] D18708: Set C99 as default C Standard for PS4 target

[Richard Smith via cfe-commits]

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

What's the motivation for this change? Clang's C11 mode should accept all code 
accepted by its C99 mode (and conversely, most or perhaps all of the C11 
language features are accepted by default in C99 mode as an extension). Is the 
problem the C11 standard library extensions?


http://reviews.llvm.org/D18708



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

Re: [PATCH] D17987: [clang-tidy] Extension of checker misc-misplaced-widening-cast

[Alexander Kornienko via cfe-commits]

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

Looks good! Thank you for addressing the comments.



Comment at: clang-tidy/misc/MisplacedWideningCastCheck.cpp:117
@@ +116,3 @@
+static llvm::SmallDenseMap createRelativeCharSizesMap() {
+  llvm::SmallDenseMap Result(6);
+  Result[BuiltinType::UChar] = 1;

baloghadamsoftware wrote:
> I changed to SmallDenseMap and its lookup() member function is nice. However 
> it unfortunately lacks an initializer_list constructor.
It has a constructor from two iterators, but I'm not sure it'll result in a 
significantly better code.


Comment at: clang-tidy/misc/MisplacedWideningCastCheck.cpp:188
@@ -114,1 +187,3 @@
+if (CastBuiltinType && CalcBuiltinType &&
+!isFirstWider(CastBuiltinType->getKind(), CalcBuiltinType->getKind()))
   return;

baloghadamsoftware wrote:
> Is this an LLVM style rule? I always learned that it is the safest to use 
> braces even for single-line if bodies.
http://llvm.org/docs/CodingStandards.html is surprisingly inconsistent in this 
regard, but "no braces around single-line if/for/... bodies" is a more common 
style in LLVM and, in particular, in clang-tidy code. The problems this style 
might lead to (statements indented as-if they were under if/for/..., but they 
actually aren't) are mitigated by the wide use of clang-format.


http://reviews.llvm.org/D17987



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

Re: [PATCH] D18717: [Clang-tidy] Improve checks documentation consistency

[Alexander Kornienko via cfe-commits]

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

Awesome! Thanks!


Repository:
  rL LLVM

http://reviews.llvm.org/D18717



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

Re: [PATCH] D18708: Set C99 as default C Standard for PS4 target

[Douglas Yung via cfe-commits]

dyung added a comment.

In http://reviews.llvm.org/D18708#390132, @rsmith wrote:

> What's the motivation for this change? Clang's C11 mode should accept all 
> code accepted by its C99 mode (and conversely, most or perhaps all of the C11 
> language features are accepted by default in C99 mode as an extension). Is 
> the problem the C11 standard library extensions?


From my understanding, there are 2 issues that block us. The first is that we 
currently do not ship all of the header files for C11. The second is that we do 
not yet fully have C11 library support yet for our platform.


http://reviews.llvm.org/D18708



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

Re: [PATCH] D18717: [Clang-tidy] Improve checks documentation consistency

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Also, could you, please, next time generate diffs with full context as 
documented in 
http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface 
(or use arcanist, which does this automatically)? This sometimes (not in this 
case) saves reviewers manual work of finding the relevant source files and 
looking into them in order to see the context. Thanks!


Repository:
  rL LLVM

http://reviews.llvm.org/D18717



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

Re: [PATCH] D18717: [Clang-tidy] Improve checks documentation consistency

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a comment.

In http://reviews.llvm.org/D18717#390151, @alexfh wrote:

> Also, could you, please, next time generate diffs with full context as 
> documented in 
> http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
>  (or use arcanist, which does this automatically)? This sometimes (not in 
> this case) saves reviewers manual work of finding the relevant source files 
> and looking into them in order to see the context. Thanks!


I'll try to not forget next time :-)


Repository:
  rL LLVM

http://reviews.llvm.org/D18717



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

[clang-tools-extra] r265205 - [Clang-tidy] Improve checks documentation consistency.

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Fri Apr  1 20:07:18 2016
New Revision: 265205

URL: http://llvm.org/viewvc/llvm-project?rev=265205&view=rev
Log:
[Clang-tidy] Improve checks documentation consistency.

Differential revision: http://reviews.llvm.org/D18717

Modified:
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err52-cpp.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err58-cpp.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-flp30-c.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/google-build-explicit-make-pair.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/google-build-namespaces.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/google-build-using-namespace.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/google-global-names-in-headers.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/google-readability-braces-around-statements.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/google-readability-casting.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/google-readability-function-size.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/google-readability-todo.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-int.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-member-string-references.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-memset.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-operator.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-assert-side-effect.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/misc-bool-pointer-implicit-conversion.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-suspicious-semicolon.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-loop-convert.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-pass-by-value.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-auto.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-nullptr.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/performance-implicit-cast-in-loop.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-braces-around-statements.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-identifier-naming.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-named-parameter.rst

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err52-cpp.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err52-cpp.rst?rev=265205&r1=265204&r2=265205&view=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err52-cpp.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err52-cpp.rst Fri Apr  
1 20:07:18 2016
@@ -3,7 +3,7 @@
 cert-err52-cpp
 ==
 
-This check flags all call expressions involving setjmp() and longjmp().
+This check flags all call expressions involving ``setjmp()`` and ``longjmp()``.
 
 This check corresponds to the CERT C++ Coding Standard rule
 `ERR52-CPP. Do not use setjmp() or longjmp()

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err58-cpp.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err58-cpp.rst?rev=265205&r1=265204&r2=265205&view=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err58-cpp.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err58-cpp.rst Fri Apr  
1 20:07:18 2016
@@ -3,8 +3,8 @@
 cert-err58-cpp
 ==
 
-This check flags all static or thread_local variable declarations where the
-constructor for the object may throw an exception.
+This check flags all ``static`` or ``thread_local`` variable declarations where
+the constructor for the object may throw an exception.
 
 This check corresponds to the CERT C++ Coding Standard rule
 `ERR58-CPP. Constructors of objects with static or thread storage duration 
must not throw exceptions

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/cert-flp30-c.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cert-flp30-c.rst?re

Re: [PATCH] D18717: [Clang-tidy] Improve checks documentation consistency

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL265205: [Clang-tidy] Improve checks documentation 
consistency. (authored by eugenezelenko).

Changed prior to commit:
  http://reviews.llvm.org/D18717?vs=52441&id=52451#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D18717

Files:
  clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err52-cpp.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err58-cpp.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/cert-flp30-c.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/google-build-explicit-make-pair.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/google-build-namespaces.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/google-build-using-namespace.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/google-global-names-in-headers.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/google-readability-braces-around-statements.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/google-readability-casting.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/google-readability-function-size.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/google-readability-todo.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-int.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-member-string-references.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-memset.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-operator.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/misc-assert-side-effect.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-bool-pointer-implicit-conversion.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-suspicious-semicolon.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-loop-convert.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-pass-by-value.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-auto.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-nullptr.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/performance-implicit-cast-in-loop.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-braces-around-statements.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-identifier-naming.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/readability-named-parameter.rst

Index: clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err52-cpp.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err52-cpp.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err52-cpp.rst
@@ -3,7 +3,7 @@
 cert-err52-cpp
 ==
 
-This check flags all call expressions involving setjmp() and longjmp().
+This check flags all call expressions involving ``setjmp()`` and ``longjmp()``.
 
 This check corresponds to the CERT C++ Coding Standard rule
 `ERR52-CPP. Do not use setjmp() or longjmp()
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-braces-around-statements.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-braces-around-statements.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-braces-around-statements.rst
@@ -3,6 +3,8 @@
 readability-braces-around-statements
 
 
+`google-readability-braces-around-statements` redirects here as an alias for
+this check.
 
 Checks that bodies of ``if`` statements and loops (``for``, ``range-for``,
 ``do-while``, and ``while``) are inside braces
@@ -22,8 +24,8 @@
 statement;
   }
 
-Additionally, one can define an option ``ShortStatementLines`` defining the
-minimal number of lines that the statement should have in order to trigger
+Additionally, one can define an option :option:`ShortStatementLines` defining
+the minimal number of lines that the statement should have in order to trigger
 this check.
 
 The number of lines is counted from the end of condition or initial keyword
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst
+++ clang-tools-ext

Re: [PATCH] D18708: Set C99 as default C Standard for PS4 target

[Richard Smith via cfe-commits]

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

In http://reviews.llvm.org/D18708#390150, @dyung wrote:

> From my understanding, there are 2 issues that block us. The first is that we 
> currently do not ship all of the header files for C11. The second is that we 
> do not yet fully have C11 library support yet for our platform.


How do things go wrong if Clang is used in C11 mode against a C99 library? Do 
you have code that conditionally uses C11 library features if they're 
available? (And thanks for explaining, by the way; this is useful information 
for when we next consider changing the default language mode -- we'll likely be 
changing the default C++ language mode soon.)

Anyway, it seems reasonable for the PS4 toolchain to control its defaults here, 
and the patch itself LGTM.


http://reviews.llvm.org/D18708



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

Re: [PATCH] D15332: new clang-tidy checker readability-non-const-parameter

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: clang-tidy/readability/NonConstParameterCheck.cpp:44
@@ +43,3 @@
+addParm(Parm);
+  } else if (const CXXConstructorDecl *Ctor =
+ Result.Nodes.getNodeAs("Ctor")) {

`const auto *Ctor`


Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:7
@@ +6,3 @@
+Finds function pointer parameters that should be const. When const is used
+properly, many mistakes can be avoided. Advantages when using const properly:
+ * avoid that data is unintentionally modified

That now sounds ambiguous (is it about parameters of a pointer to a function 
type?). I'd say "The check finds function parameters of a pointer type that 
could be changed to point to a constant type instead." or something similar.


Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:8
@@ +7,3 @@
+properly, many mistakes can be avoided. Advantages when using const properly:
+ * avoid that data is unintentionally modified
+ * get additional warnings such as using uninitialized data

"prevent unintentional modification of data"


Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:10
@@ +9,3 @@
+ * get additional warnings such as using uninitialized data
+ * easier for developers to see possible side effects
+

"make it easier for developers ..."


Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:12
@@ +11,3 @@
+
+clang-tidy is not strict about constness, it only warns when the constness will
+make the function interface safer.

s/clang-tidy/This check/


Comment at: test/clang-tidy/readability-non-const-parameter.cpp:244
@@ +243,3 @@
+  // CHECK-MESSAGES: :[[@LINE+1]]:21: warning: parameter 'p' can be const
+  void doStuff(int *p) {
+  // CHECK-FIXES: {{^}}  void doStuff(const int *p) {{{$}}

Please add a test to ensure that overridden methods are not modified (since you 
would need to update the base class' method and then all the other overrides.


Comment at: test/clang-tidy/readability-non-const-parameter.cpp:251
@@ +250,2 @@
+};
+

The check can break code taking a pointer to a function it modifies:

  typedef int (*fn)(int *);
  int f(int *p) {return *p; }
  fn fp = &f;

It's not always possible to detect, when this happens (for example, a pointer 
to the function could be taken in a different translation unit), but we should 
at least keep this case in mind. Please add a test case for this.


http://reviews.llvm.org/D15332



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

Re: [PATCH] D18708: Set C99 as default C Standard for PS4 target

[Douglas Yung via cfe-commits]

dyung added a comment.

In http://reviews.llvm.org/D18708#390166, @rsmith wrote:

> In http://reviews.llvm.org/D18708#390150, @dyung wrote:
>
> > From my understanding, there are 2 issues that block us. The first is that 
> > we currently do not ship all of the header files for C11. The second is 
> > that we do not yet fully have C11 library support yet for our platform.
>
>
> How do things go wrong if Clang is used in C11 mode against a C99 library? Do 
> you have code that conditionally uses C11 library features if they're 
> available? (And thanks for explaining, by the way; this is useful information 
> for when we next consider changing the default language mode -- we'll likely 
> be changing the default C++ language mode soon.)
>
> Anyway, it seems reasonable for the PS4 toolchain to control its defaults 
> here, and the patch itself LGTM.


My understanding is that we currently do not ship all of the headers required 
for C11, so if users tried to use anything from those, they would be unable to 
do so. I am also under the impression that C11 requires some library support 
which we do not yet provide.

Thanks for the quick review!


http://reviews.llvm.org/D18708



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

Re: [PATCH] D17043: Check that the result of a library call w/o side effects is used

[Alexander Kornienko via cfe-commits]

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

What's the status of this patch? Do you still want to continue working on it or 
are you fine with the warn_unused_result/nodiscard-based solution?


http://reviews.llvm.org/D17043



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

Re: [PATCH] D15332: new clang-tidy checker readability-non-const-parameter

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a comment.

Please also mention check in  docs/ReleaseNotes.rst.



Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:15
@@ +14,3 @@
+
+  // warning here; the declaration "const char *p" would make the function
+  // interface safer.

I think you should add ".. code-block:: c++" before code block.



http://reviews.llvm.org/D15332



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

Re: [PATCH] D18136: boost-use-to-string check

[Alexander Kornienko via cfe-commits]

alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-tidy/boost/UseToStringCheck.cpp:55
@@ +54,3 @@
+void UseToStringCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedToString = Result.Nodes.getNodeAs("to_string");
+  auto CharType =

"Matched" isn't useful here and "ToString" is misleading. I'd name this 
`LexicalCastCall` or just `Call`.


Comment at: clang-tidy/boost/UseToStringCheck.cpp:59
@@ +58,3 @@
+
+  if (CharType.isNull())
+return;

When can `CharType` be `isNull()`? Do you have a test case for this?


Comment at: clang-tidy/boost/UseToStringCheck.cpp:64
@@ +63,3 @@
+  CharType->isSpecificBuiltinType(BuiltinType::Char_U)) {
+apply(MatchedToString,
+  "use std::to_string instead of boost::lexical_cast",

I'd rewrite this code as:

  StringRef StringType;
  if (CharType->isSpecificBuiltinType(BuiltinType::Char_S) ||
  CharType->isSpecificBuiltinType(BuiltinType::Char_U))
StringType = "string";
  else if (CharType->isSpecificBuiltinType(BuiltinType::WChar_S) ||
   CharType->isSpecificBuiltinType(BuiltinType::WChar_U))
StringType = "wstring";
  else
return;

  diag(Call->getLocStart(), "use std::to_%0 instead of 
boost::lexical_cast")
  << StringType << FixItHint::CreateReplacement(
  CharSourceRange::getCharRange(Call->getLocStart(),
 Call->getArg(0)->getExprLoc()),
  (llvm::Twine("std::to_") + StringType).str());


Comment at: clang-tidy/boost/UseToStringCheck.cpp:68
@@ +67,3 @@
+  }
+  // Is CharType 'wchar_t'.
+  else if (CharType->isSpecificBuiltinType(BuiltinType::WChar_S) ||

Please move this comment inside the `if` body. As it is now, it breaks the 
formatting.


Comment at: docs/clang-tidy/checks/boost-use-to-string.rst:6
@@ +5,3 @@
+
+This check finds conversion from integer type like int to std::string or
+std::wstring using boost::lexical_cast, and replace it to calls of

Please enclose inline code snippets in double backquotes (`int`, `std::string`, 
etc.).


http://reviews.llvm.org/D18136



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

Re: [PATCH] D18191: [clang-tidy] Add check for function parameters that are const& to builtin types

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Adding Samuel, who's written a similar check internally and might want to 
upstream it or suggest improvements to this patch.


http://reviews.llvm.org/D18191



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

Re: [PATCH] D15032: [clang-tidy] new checker cppcoreguidelines-pro-lifetime

[Alexander Kornienko via cfe-commits]

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

FYI, I'm waiting with reviewing this change until 
http://reviews.llvm.org/D15031 is landed, since it can affect this patch.


http://reviews.llvm.org/D15032



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

Re: [PATCH] D16248: [Clang-tidy] rename misc-inefficient-algorithm to performance-inefficient-algorithm

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Please rebase the patch and add full context to the diffs (see 
http://llvm.org/docs/Phabricator.html).



Comment at: docs/clang-tidy/checks/misc-inefficient-algorithm.rst:4
@@ -5,1 +3,3 @@
+.. meta::
+   :http-equiv=refresh: 5;URL=performance-inefficient-algorithm.html
 

alexfh wrote:
> We need to change the add_new_check.py script to exclude obsolete check names 
> from the list (it could exclude all files marked `:orphan:`). Tell me, if you 
> need help with this.
No need to do this, since the script already implements an alternative solution.


Comment at: docs/clang-tidy/checks/performance-inefficient-algorithm.rst:3
@@ -2,3 +2,3 @@
 
-misc-inefficient-algorithm
+performance-inefficient-algorithm
 ==

xazax.hun wrote:
> Eugene.Zelenko wrote:
> > alexfh wrote:
> > > After reading this check name a few times, I found it too generic (one 
> > > may think that this is a generic algorithm-level code profiler ;). I 
> > > think, we need to rename it to `performance-inefficient-lookup-algorithm` 
> > > or `performance-inefficient-search-algorithm`, since we're changing the 
> > > name anyway.
> > I think will be better to keep generic name, since other algorithms could 
> > be added later.
> That is an interesting question whether it is better to have more general 
> check names and make checkers do more stuff or have more specific names and 
> split functionality between more checks. It would be awesome to have a policy 
> on that. A good benchmark whether two checks should be implemented by the 
> same checker is to think about whether there are cases when the user might 
> enable only one of the checks, not both. 
> A good benchmark whether two checks should be implemented by the same checker 
> is to think about whether there are cases when the user might enable only one 
> of the checks, not both.

In this case the check detects one consistent set of patterns (non-member 
lookup algorithms used on containers that implement better alternatives). I 
don't think we'll want to add something unrelated to this check, so I'd better 
make the name more specific and create another check, when we have a different 
set of patterns.


http://reviews.llvm.org/D16248



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

Re: [PATCH] D18575: [clang-tidy] New checker to replace deprecated throw() specifications

[Alexander Kornienko via cfe-commits]

alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:16
@@ +15,3 @@
+
+// FIXME: Should this be moved to ASTMatchers.h?
+namespace ast_matchers {

Yes, it might make sense to move it to ASTMatchers.h.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:31
@@ +30,3 @@
+///   matches the declarations of h, i, and j, but not f, g or k.
+AST_MATCHER(FunctionDecl, isDynamicException) {
+  const auto *FnTy = Node.getType()->getAs();

Please rename the matcher to `hasDynamicExceptionSpec()`.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:35
@@ +34,3 @@
+return false;
+  return isDynamicExceptionSpec(FnTy->getExceptionSpecType());
+}

  if (const auto *FnTy = Node.getType()->getAs())
return FnTy->hasDynamicExceptionSpec();
  return false;


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:61
@@ +60,3 @@
+  CurrentLoc =
+  Lexer::GetBeginningOfToken(CurrentLoc, SM, Context.getLangOpts());
+  if (!Lexer::getRawToken(CurrentLoc, Tok, SM, Context.getLangOpts()) &&

I suspect that repeated calls to the GetBeginningOfToken method might be rather 
expensive. An alternative (and possibly more efficient) approach would be to 
re-lex the range from the start of the function declaration to the current 
location (in the forward direction) and then just operate on the array of 
tokens. See UseOverrideCheck.cpp, for example. If you decide to go that way, we 
could move the ParseTokens function to clang-tidy/utils/LexerUtils.{h,cpp}.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:84
@@ +83,3 @@
+  Finder->addMatcher(
+  ast_matchers::functionDecl(ast_matchers::isDynamicException())
+  .bind("functionDecl"),

We usually add `using namespace ast_matchers;` to make matchers more readable.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:110
@@ +109,3 @@
+  SimpleReverseLexer Lexer{Context, SM, BeginLoc, CurrentLoc};
+  Token &Tok = Lexer.getToken();
+  unsigned TokenLength{0};

I don't like this use of `Tok` as an alias of `Lexer::Tok`. It makes the code 
using it harder to understand.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:123
@@ +122,3 @@
+return;
+  // if (!isBefore(BeginLoc, Tok.getLocation())) return;
+  bool empty = true;

Please remove the commented-out code or uncomment it, if it's needed.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:156
@@ +155,3 @@
+auto Len = endInfo.second - beginInfo.second + TokenLength;
+diag(FuncDecl->getLocation(), "function '%0': Replace dynamic exception "
+  "specifications '%1' with '%2'")

The format of the message is not consistent with other messages. I'd suggest 
something along the lines of: "function '%0' uses dynamic exception 
specification '%1'; use '%2' instead".


Comment at: clang-tidy/modernize/UseNoexceptCheck.h:35
@@ +34,3 @@
+public:
+  UseNoexceptCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context),

As soon as a constructor starts doing something but just calling the base 
constructor (and sometimes before that), we usually move it to a .cpp file.

Please also move the `storeOptions` definition to the .cpp file.


http://reviews.llvm.org/D18575



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