[libcxx] r278319 - Refactor test archetypes implementation.

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Thu Aug 11 02:04:14 2016
New Revision: 278319

URL: http://llvm.org/viewvc/llvm-project?rev=278319&view=rev
Log:
Refactor test archetypes implementation.

Added:
libcxx/trunk/test/support/archetypes.ipp
Modified:
libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/U_V.pass.cpp

libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/const_first_const_second.pass.cpp

libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/const_pair_U_V.pass.cpp
libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/default.pass.cpp

libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/rv_pair_U_V.pass.cpp

libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/special_member_generation_test.pass.cpp
libcxx/trunk/test/support/archetypes.hpp

Modified: libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/U_V.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/U_V.pass.cpp?rev=278319&r1=278318&r2=278319&view=diff
==
--- libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/U_V.pass.cpp 
(original)
+++ libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/U_V.pass.cpp Thu 
Aug 11 02:04:14 2016
@@ -22,6 +22,7 @@
 
 #include "archetypes.hpp"
 #include "test_convertible.hpp"
+using namespace ImplicitTypes; // Get implicitly archetypes
 
 template 
@@ -58,20 +59,20 @@ int main()
 // Test non-const lvalue and rvalue types
 test_sfinae();
 test_sfinae();
-test_sfinae();
-test_sfinae();
+test_sfinae();
+test_sfinae();
 test_sfinae();
 test_sfinae();
-test_sfinae();
-test_sfinae();
+test_sfinae();
+test_sfinae();
 test_sfinae();
 test_sfinae();
-test_sfinae();
-test_sfinae();
+test_sfinae();
+test_sfinae();
 test_sfinae();
 test_sfinae();
-test_sfinae();
-test_sfinae();
+test_sfinae();
+test_sfinae();
 }
 {
 // Test converting types
@@ -79,10 +80,10 @@ int main()
 test_sfinae();
 test_sfinae();
 test_sfinae();
-test_sfinae();
-test_sfinae();
-test_sfinae();
-test_sfinae();
+test_sfinae();
+test_sfinae();
+test_sfinae();
+test_sfinae();
 }
 #if TEST_STD_VER > 11
 { // explicit constexpr test

Modified: 
libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/const_first_const_second.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/const_first_const_second.pass.cpp?rev=278319&r1=278318&r2=278319&view=diff
==
--- 
libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/const_first_const_second.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/const_first_const_second.pass.cpp
 Thu Aug 11 02:04:14 2016
@@ -20,6 +20,7 @@
 
 #include "archetypes.hpp"
 #include "test_convertible.hpp"
+using namespace ImplicitTypes; // Get implicitly archetypes
 
 struct ExplicitT {
   constexpr explicit ExplicitT(int x) : value(x) {}
@@ -62,13 +63,13 @@ int main()
 }
 {
 test_sfinae();
-test_sfinae();
+test_sfinae();
 test_sfinae();
-test_sfinae();
+test_sfinae();
 test_sfinae();
-test_sfinae();
+test_sfinae();
 test_sfinae();
-test_sfinae();
+test_sfinae();
 }
 #if TEST_STD_VER > 11
 {

Modified: 
libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/const_pair_U_V.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/const_pair_U_V.pass.cpp?rev=278319&r1=278318&r2=278319&view=diff
==
--- 
libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/const_pair_U_V.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/const_pair_U_V.pass.cpp
 Thu Aug 11 02:04:14 2016
@@ -20,6 +20,7 @@
 
 #include "archetypes.hpp"
 #include "test_convertible.hpp"
+using namespace ImplicitTypes; // Get implicitly archetypes
 
 template 
@@ -79,27 +80,27 @@ int main()
 test_pair_const();
 test_pair_const();
 
-test_pair_const(); // copy 
construction
-test_pair_const();
-test_pair_const();
-test_pair_const();
-test_pair_const();
+test_pair_const(); 
// copy construction
+test_pair_const();
+test_pair_const();
+test_pair_const();
+test_pair_const();
 
 test_pair_const(); // copy construction
 test_pair_const();
 test_pair_const();
 
-test_pair_const(); // copy 
construction
-test_pair_const();
-  

Re: [PATCH] D22782: Added 'inline' attribute to __init to inline the basic_string's constructor

[Eric Fiselier via cfe-commits]

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

I would love to see a benchmark with this, but I've done enough investigating 
on my own that I *know* this patch is beneficial.


https://reviews.llvm.org/D22782



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

[libcxx] r278323 - Unbreak C++03 build.

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Thu Aug 11 03:15:35 2016
New Revision: 278323

URL: http://llvm.org/viewvc/llvm-project?rev=278323&view=rev
Log:
Unbreak C++03 build.

Modified:
libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/default.pass.cpp

Modified: 
libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/default.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/default.pass.cpp?rev=278323&r1=278322&r2=278323&view=diff
==
--- libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/default.pass.cpp 
(original)
+++ libcxx/trunk/test/std/utilities/utility/pairs/pairs.pair/default.pass.cpp 
Thu Aug 11 03:15:35 2016
@@ -28,7 +28,6 @@
 
 #include "test_macros.h"
 #include "archetypes.hpp"
-using namespace ImplicitTypes; // Get implicitly archetypes
 
 int main()
 {
@@ -46,6 +45,7 @@ int main()
 static_assert(p.second == nullptr, "");
 }
 {
+using NoDefault = ImplicitTypes::NoDefault;
 using P = std::pair;
 static_assert(!std::is_default_constructible::value, "");
 using P2 = std::pair;


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

[clang-tools-extra] r278324 - [clang-tidy] fix readability-else-after-return test

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Thu Aug 11 03:18:39 2016
New Revision: 278324

URL: http://llvm.org/viewvc/llvm-project?rev=278324&view=rev
Log:
[clang-tidy] fix readability-else-after-return test

As pointed by Yung Douglas, exceptions in
test/clang-tidy/readability-else-after-return.cpp are causing PS4 bots to be
red, because exceptions aren't enabled by default on PS4 target. This patch is a
fix.

Modified:
clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp

Modified: 
clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp?rev=278324&r1=278323&r2=278324&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp 
Thu Aug 11 03:18:39 2016
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-else-after-return %t
+// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -std=c++11 
-fexceptions
 
 void f(int a) {
   if (a > 0)


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

Re: [PATCH] D22666: Frontend: Fix mcount inlining bug

[Honggyu Kim via cfe-commits]

honggyu.kim added a comment.

In https://reviews.llvm.org/D22666#509904, @compnerd wrote:

> Theres a frontend mangler and a backend mangler.  You should try this on 
> Windows GNU environments :-).


Thanks for the confirmation :)
If so, would it be okay if I put one more backslash for each mcount name with 
'\0' prefix? It shows the output properly with '\0' in IR generation. For 
example,

  @@ -4920,7 +4920,7 @@ public:
   if (Triple.getOS() == llvm::Triple::Linux ||
   Triple.getOS() == llvm::Triple::UnknownOS)
 this->MCountName =
  -  Opts.EABIVersion == "gnu" ? "\01__gnu_mcount_nc" : "\01mcount";
  +  Opts.EABIVersion == "gnu" ? "\\01__gnu_mcount_nc" : "\\01mcount";
 }
  
 StringRef getABI() const override { return ABI; }

`test-mcount.c` test above can be passed with this modification.


https://reviews.llvm.org/D22666



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

Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values

[Andrey Bokhanko via cfe-commits]

andreybokhanko added a subscriber: andreybokhanko.
andreybokhanko added a comment.

@uweigand Ulrich, any objections here? If not, I will commit this patch 
tomorrow (as the author, Vladimir, doesn't have commit access yet).

Andrey


Repository:
  rL LLVM

https://reviews.llvm.org/D21678



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

[PATCH] D23397: [clang-rename] cleanup `auto` usages

[Kirill Bobyrev via cfe-commits]

omtcyfz created this revision.
omtcyfz added reviewers: alexfh, klimek.
omtcyfz added a subscriber: cfe-commits.

As Alexander pointed out, LLVM Coding Standards are more conservative about 
using `auto`, i.e. it should be used in the following situations:

* When the type is //obvious//, i.e. explicitly mentioned in the same 
expression. For example `if (const clang::FieldDecl *FieldDecl = 
Initializer->getMember())`
* When the type is totally non-obvious and one iterates over something. For 
example `for (const auto &CurrDecl : 
Context.getTranslationUnitDecl()->decls())`.

Otherwise the type should be explicitly stated.

https://reviews.llvm.org/D23397

Files:
  clang-rename/RenamingAction.cpp
  clang-rename/USRFinder.cpp
  clang-rename/USRFindingAction.cpp
  clang-rename/USRLocFinder.cpp

Index: clang-rename/USRLocFinder.cpp
===
--- clang-rename/USRLocFinder.cpp
+++ clang-rename/USRLocFinder.cpp
@@ -67,7 +67,7 @@
   // Expression visitors:
 
   bool VisitDeclRefExpr(const DeclRefExpr *Expr) {
-const auto *Decl = Expr->getFoundDecl();
+const NamedDecl *Decl = Expr->getFoundDecl();
 
 if (USRSet.find(getUSRForDecl(Decl)) != USRSet.end()) {
   const SourceManager &Manager = Decl->getASTContext().getSourceManager();
@@ -79,7 +79,7 @@
   }
 
   bool VisitMemberExpr(const MemberExpr *Expr) {
-const auto *Decl = Expr->getFoundDecl().getDecl();
+const NamedDecl *Decl = Expr->getFoundDecl().getDecl();
 if (USRSet.find(getUSRForDecl(Decl)) != USRSet.end()) {
   const SourceManager &Manager = Decl->getASTContext().getSourceManager();
   SourceLocation Location = Manager.getSpellingLoc(Expr->getMemberLoc());
@@ -116,7 +116,8 @@
   // Namespace traversal:
   void handleNestedNameSpecifierLoc(NestedNameSpecifierLoc NameLoc) {
 while (NameLoc) {
-  const auto *Decl = NameLoc.getNestedNameSpecifier()->getAsNamespace();
+  const NamespaceDecl *Decl =
+  NameLoc.getNestedNameSpecifier()->getAsNamespace();
   if (Decl && USRSet.find(getUSRForDecl(Decl)) != USRSet.end()) {
 checkAndAddLocation(NameLoc.getLocalBeginLoc());
   }
@@ -126,9 +127,10 @@
 
 private:
   void checkAndAddLocation(SourceLocation Loc) {
-const auto BeginLoc = Loc;
-const auto EndLoc = Lexer::getLocForEndOfToken(
-BeginLoc, 0, Context.getSourceManager(), Context.getLangOpts());
+const SourceLocation BeginLoc = Loc,
+ EndLoc = Lexer::getLocForEndOfToken(
+ BeginLoc, 0, Context.getSourceManager(),
+ Context.getLangOpts());
 StringRef TokenName =
 Lexer::getSourceText(CharSourceRange::getTokenRange(BeginLoc, EndLoc),
  Context.getSourceManager(), Context.getLangOpts());
Index: clang-rename/USRFindingAction.cpp
===
--- clang-rename/USRFindingAction.cpp
+++ clang-rename/USRFindingAction.cpp
@@ -116,14 +116,14 @@
 
   void addUSRsOfOverridenFunctions(const CXXMethodDecl *MethodDecl) {
 USRSet.insert(getUSRForDecl(MethodDecl));
-for (auto &OverriddenMethod : MethodDecl->overridden_methods()) {
+for (const auto &OverriddenMethod : MethodDecl->overridden_methods()) {
   // Recursively visit each OverridenMethod.
   addUSRsOfOverridenFunctions(OverriddenMethod);
 }
   }
 
   bool checkIfOverriddenFunctionAscends(const CXXMethodDecl *MethodDecl) {
-for (auto &OverriddenMethod : MethodDecl->overridden_methods()) {
+for (const auto &OverriddenMethod : MethodDecl->overridden_methods()) {
   if (USRSet.find(getUSRForDecl(OverriddenMethod)) != USRSet.end()) {
 return true;
   }
@@ -143,10 +143,11 @@
 
 struct NamedDeclFindingConsumer : public ASTConsumer {
   void HandleTranslationUnit(ASTContext &Context) override {
-const auto &SourceMgr = Context.getSourceManager();
+const SourceManager &SourceMgr = Context.getSourceManager();
 // The file we look for the USR in will always be the main source file.
-const auto Point = SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID())
-   .getLocWithOffset(SymbolOffset);
+const SourceLocation Point =
+SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID())
+.getLocWithOffset(SymbolOffset);
 if (!Point.isValid())
   return;
 const NamedDecl *FoundDecl = nullptr;
Index: clang-rename/USRFinder.cpp
===
--- clang-rename/USRFinder.cpp
+++ clang-rename/USRFinder.cpp
@@ -60,23 +60,24 @@
   // Expression visitors:
 
   bool VisitDeclRefExpr(const DeclRefExpr *Expr) {
-const auto *Decl = Expr->getFoundDecl();
+const NamedDecl *Decl = Expr->getFoundDecl();
 return setResult(Decl, Expr->getLocation(),
  Decl->getNameAsString().length());
   }
 
   bool VisitMemberExpr(const MemberExpr

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

[Daniel Jasper via cfe-commits]

djasper added a comment.

Sorry :(... Review is easy enough.. Feel free to ping me more often in the 
future.



Comment at: lib/Format/WhitespaceManager.cpp:95
@@ -97,2 +94,3 @@
   std::sort(Changes.begin(), Changes.end(), Change::IsBeforeInFile(SourceMgr));
+  calculateScopeLevel();
   calculateLineBreakInformation();

bmharper wrote:
> berenm wrote:
> > Maybe we could spare the computation if we aren't going to align anything?
> > 
> > Is it better for clarity to always compute additional information? @djasper 
> > what's the Clang way to do?
> > 
> That's a good point. One certainly could elide that if alignment was turned 
> off. I think so long as it was mentioned in the comments of the ScopeLevel 
> member variable, it would be OK to do so. However, I'll also just defer this 
> decision to @djasper.
Yeah, just avoid unnecessary work.


Comment at: lib/Format/WhitespaceManager.h:173
@@ -165,1 +172,3 @@
+// down to -1.
+int ScopeLevel;
   };

NestingLevel does include braces, generally. However, there are two types:
- Braced initializers: These should just work as is.
- Braces that open blocks: Here, child lines are created and so the tokens 
within a block restart from NestingLevel 0. However, taking that NestingLevel 
in combination with the Level of the AnnotatedLine should work.

I think reusing that is highly preferable over implementing yet another 
parentheses counting.


Repository:
  rL LLVM

https://reviews.llvm.org/D21279



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

[PATCH] D23400: Implement Use-Define Chain.

[Vassil Vassilev via cfe-commits]

v.g.vassilev created this revision.
v.g.vassilev added subscribers: teemperor, dergachev.a, krememek, cfe-commits.
v.g.vassilev set the repository for this revision to rL LLVM.

As a result of a few internal discussions, I'd like to continue the work done 
in this ancient thread 
(http://lists.llvm.org/pipermail/cfe-dev/2009-September/006308.html) by Simone 
Pellegrini.

The current state of the patch is its latest revision from 2009. I will do the 
minimal changes to get it to compile. I'll try to address the comments that 
were not addressed. Once this is done, we can start a formal review procedure. 
I believe our suspicious clone detection infrastructure would benefit from the 
AST level Use-Define Chains. Also, a few SA passes could make use of this 
concept. 

Repository:
  rL LLVM

https://reviews.llvm.org/D23400

Files:
  include/clang/Analysis/DefUse.h
  lib/Analysis/CMakeLists.txt
  lib/Analysis/DefUse.cpp

Index: lib/Analysis/DefUse.cpp
===
--- /dev/null
+++ lib/Analysis/DefUse.cpp
@@ -0,0 +1,257 @@
+//===-- clang/Analysis/DefUse.h - DefUse analysis ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file contains the declaration of DefUse analysis headers
+//
+//===--===//
+
+#ifndef LLVM_CLANG_DEFUSE_H
+#define LLVM_CLANG_DEFUSE_H
+
+#include "llvm/ADT/DenseMap.h"
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ParentMap.h"
+#include "clang/AST/StmtVisitor.h"
+
+namespace clang {
+
+// forward definitions
+class CFG;
+class CFGBlock;
+
+class DefUse;
+
+namespace defuse {
+//===- DefUseNode -===//
+// Keeps the information for a particular 'definition' or 'use' of a variable
+// The structure is needed because a defuse chain can contains variable declarations
+// as well as variable reference. As a DeclStmt can contains several declarations
+// to address a particular variable we need to store its VarDecl or DeclRefExp
+class DefUseNode {
+public:
+  enum NodeKind {
+VarDecl, VarRef
+  };
+  enum UseKind {
+Use, Def, UseDef
+  };
+
+  DefUseNode(clang::VarDecl const* decl) :
+var_decl(decl), kind(VarDecl), usage(Def) {
+  }
+  DefUseNode(DeclRefExpr const* ref, UseKind u = Use) :
+var_ref(ref), kind(VarRef), usage(u) {
+  }
+
+  clang::VarDecl const* getDecl() const;
+
+  NodeKind const& getKind() const {
+return kind;
+  }
+  UseKind const& getUse() const {
+return usage;
+  }
+
+  clang::VarDecl const* getVarDecl() const {
+assert(kind==VarDecl);
+return var_decl;
+  }
+  DeclRefExpr const* getVarRef() const {
+assert(kind==VarRef);
+return var_ref;
+  }
+
+  bool operator==(DefUseNode const& n) const;
+
+private:
+  // a def-use node can be either a VarDecl or a DeclRefExpr
+  union {
+clang::VarDecl const* var_decl;
+DeclRefExpr const* var_ref;
+  };
+  NodeKind kind;
+  UseKind usage;
+};
+
+//===- Typedefs -===//
+class DefUseBlock;
+class VarDeclMap;
+typedef std::vector DefUseData;
+typedef llvm::DenseMap VarRefBlockMap;
+typedef std::vector DefUseVect;
+typedef std::vector VarRefsVect;
+
+class DefUseHelper;
+
+//===- DefUseHelper -===//
+
+class DefUseHelper : public StmtVisitor {
+  class DefUseHelperImpl;
+  DefUseHelperImpl* pimpl;
+
+  void InitializeValues(DefUseData* data, VarRefBlockMap* bm, VarDeclMap* dm);
+
+  friend class clang::DefUse;
+public:
+  DefUseNode::UseKind current_use;
+  DefUseHelper();
+
+  virtual void HandleDeclRefExpr(DeclRefExpr *DR); // remember to call the
+  // super class implementation of the method
+  void HandleDeclStmt(DeclStmt *DS);
+
+  virtual void HandleBinaryOperator(BinaryOperator* B);
+  virtual void HandleConditionalOperator(ConditionalOperator* C);
+  virtual void HandleCallExpr(CallExpr* C);
+  virtual void HandleUnaryOperator(UnaryOperator* U);
+  virtual void HandleArraySubscriptExpr(ArraySubscriptExpr* AS);
+
+  void VisitDeclRefExpr(DeclRefExpr *DR) {
+return HandleDeclRefExpr(DR);
+  }
+  void VisitDeclStmt(DeclStmt *DS) {
+return HandleDeclStmt(DS);
+  }
+  void VisitBinaryOperator(BinaryOperator* B) {
+return HandleBinaryOperator(B);
+  }
+  void VisitConditionalOperator(ConditionalOperator* C) {
+return HandleConditionalOperator(C);
+  }
+  void VisitCallExpr(CallExpr* C) {
+return HandleCallExpr(C);
+  }
+  void VisitUnaryOperator(UnaryOperator* U) {
+return HandleUnaryOperator(U);
+  }
+  void VisitArraySubscriptExpr(ArraySubscriptExpr* AS) {
+return HandleArraySubscriptExpr(AS);
+  }
+
+  void Vis

Re: r278264 - Reapply [Sema] Add sizeof diagnostics for bzero

[Joerg Sonnenberger via cfe-commits]

On Wed, Aug 10, 2016 at 02:36:01PM -0700, Bruno Cardoso Lopes wrote:
> Hi Joerg,
> 
> > Given that bzero is pretty much non-standard at this point, I strongly
> > dislike the approach of checking only the function name.
> 
> Thanks for the feedback. bzero it's widely used though, I think it's a
> usability win to perform a diagnostic here, despite bzero being
> non-standard.

Oh, I don't disagree that it is useful. Some projects still have a
ridiculous amount of non-standard bzero and bcmp invocations.

> 
> Is there any additional reason why you dislike it or additional checks
> you suggest? IIUC, Sema::CheckMemaccessArguments already checks if the
> number of arguments is the same and if they are indeed what they're
> supposed to be.

It checks the number of arguments, but that's about it. It doesn't for
example check that the first argument is a pointer.

> 
> If deprecation is the main issue here, shouldn't clang emit warnings
> to mention 'bzero' deprecation then?

It's not up to clang decide what the runtime environment provides or
deprecates. That said, bzero is no longer even part of SUSv4.

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

Re: [PATCH] D23316: [analyzer] Fixed the false-positives caused by macro generated code.

[Kirill Bobyrev via cfe-commits]

omtcyfz added a subscriber: omtcyfz.


Comment at: lib/Analysis/CloneDetection.cpp:436
@@ +435,3 @@
+if (IsInMacro) {
+  Signature.Complexity = 0;
+}

Do I understand correctly that a code generated by a macro doesn't affect 
"complexity" at all then?

```
TEST_F(QueryParserTest, Complete) {
  std::vector Comps =
  QueryParser::complete("", 0, QS);
  ASSERT_EQ(6u, Comps.size());
  EXPECT_EQ("help ", Comps[0].TypedText);
  EXPECT_EQ("help", Comps[0].DisplayText);
  EXPECT_EQ("let ", Comps[1].TypedText);
  EXPECT_EQ("let", Comps[1].DisplayText);
  EXPECT_EQ("match ", Comps[2].TypedText);
  EXPECT_EQ("match", Comps[2].DisplayText);
  EXPECT_EQ("set ", Comps[3].TypedText);
  EXPECT_EQ("set", Comps[3].DisplayText);
  EXPECT_EQ("unlet ", Comps[4].TypedText);
  EXPECT_EQ("unlet", Comps[4].DisplayText);
  EXPECT_EQ("quit", Comps[5].DisplayText);
  EXPECT_EQ("quit ", Comps[5].TypedText);

  Comps = QueryParser::complete("set o", 5, QS);
  ASSERT_EQ(1u, Comps.size());
  EXPECT_EQ("utput ", Comps[0].TypedText);
  EXPECT_EQ("output", Comps[0].DisplayText);

  Comps = QueryParser::complete("match while", 11, QS);
  ASSERT_EQ(1u, Comps.size());
  EXPECT_EQ("Stmt(", Comps[0].TypedText);
  EXPECT_EQ("Matcher whileStmt(Matcher...)",
Comps[0].DisplayText);
}
```

This is an actual piece of code from 
`extra/unittests/clang-query/QueryParserTest.cpp`. Yes, it is a test, but it 
still is a nice example of how many macros can be found in code (especially if 
we are talking about pure C or some weird C++).

Thus, I think it is reasonable to treat macro invocation as a `1`-"complexity" 
node.


https://reviews.llvm.org/D23316



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

Re: [PATCH] D23316: [analyzer] Fixed the false-positives caused by macro generated code.

[Artem Dergachev via cfe-commits]

NoQ added inline comments.


Comment at: lib/Analysis/CloneDetection.cpp:436
@@ +435,3 @@
+if (IsInMacro) {
+  Signature.Complexity = 0;
+}

omtcyfz wrote:
> Do I understand correctly that a code generated by a macro doesn't affect 
> "complexity" at all then?
> 
> ```
> TEST_F(QueryParserTest, Complete) {
>   std::vector Comps =
>   QueryParser::complete("", 0, QS);
>   ASSERT_EQ(6u, Comps.size());
>   EXPECT_EQ("help ", Comps[0].TypedText);
>   EXPECT_EQ("help", Comps[0].DisplayText);
>   EXPECT_EQ("let ", Comps[1].TypedText);
>   EXPECT_EQ("let", Comps[1].DisplayText);
>   EXPECT_EQ("match ", Comps[2].TypedText);
>   EXPECT_EQ("match", Comps[2].DisplayText);
>   EXPECT_EQ("set ", Comps[3].TypedText);
>   EXPECT_EQ("set", Comps[3].DisplayText);
>   EXPECT_EQ("unlet ", Comps[4].TypedText);
>   EXPECT_EQ("unlet", Comps[4].DisplayText);
>   EXPECT_EQ("quit", Comps[5].DisplayText);
>   EXPECT_EQ("quit ", Comps[5].TypedText);
> 
>   Comps = QueryParser::complete("set o", 5, QS);
>   ASSERT_EQ(1u, Comps.size());
>   EXPECT_EQ("utput ", Comps[0].TypedText);
>   EXPECT_EQ("output", Comps[0].DisplayText);
> 
>   Comps = QueryParser::complete("match while", 11, QS);
>   ASSERT_EQ(1u, Comps.size());
>   EXPECT_EQ("Stmt(", Comps[0].TypedText);
>   EXPECT_EQ("Matcher whileStmt(Matcher...)",
> Comps[0].DisplayText);
> }
> ```
> 
> This is an actual piece of code from 
> `extra/unittests/clang-query/QueryParserTest.cpp`. Yes, it is a test, but it 
> still is a nice example of how many macros can be found in code (especially 
> if we are talking about pure C or some weird C++).
> 
> Thus, I think it is reasonable to treat macro invocation as a 
> `1`-"complexity" node.
This "0" is not for the macro itself, but for the statements into which it 
expands. Macro itself is not a statement. If we put "1" here, it would produce 
a lot more complexity than you want.

That said, it's a good idea to treat every macro as a "complexity-1" statement, 
just need to figure out how to implement that correctly :)

Perhaps scan the source range of the sequence for how many different macro 
expansions are included, and add that number to complexity(?)


https://reviews.llvm.org/D23316



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

Re: [PATCH] D23316: [analyzer] Fixed the false-positives caused by macro generated code.

[Kirill Bobyrev via cfe-commits]

omtcyfz added inline comments.


Comment at: lib/Analysis/CloneDetection.cpp:436
@@ +435,3 @@
+if (IsInMacro) {
+  Signature.Complexity = 0;
+}

NoQ wrote:
> omtcyfz wrote:
> > Do I understand correctly that a code generated by a macro doesn't affect 
> > "complexity" at all then?
> > 
> > ```
> > TEST_F(QueryParserTest, Complete) {
> >   std::vector Comps =
> >   QueryParser::complete("", 0, QS);
> >   ASSERT_EQ(6u, Comps.size());
> >   EXPECT_EQ("help ", Comps[0].TypedText);
> >   EXPECT_EQ("help", Comps[0].DisplayText);
> >   EXPECT_EQ("let ", Comps[1].TypedText);
> >   EXPECT_EQ("let", Comps[1].DisplayText);
> >   EXPECT_EQ("match ", Comps[2].TypedText);
> >   EXPECT_EQ("match", Comps[2].DisplayText);
> >   EXPECT_EQ("set ", Comps[3].TypedText);
> >   EXPECT_EQ("set", Comps[3].DisplayText);
> >   EXPECT_EQ("unlet ", Comps[4].TypedText);
> >   EXPECT_EQ("unlet", Comps[4].DisplayText);
> >   EXPECT_EQ("quit", Comps[5].DisplayText);
> >   EXPECT_EQ("quit ", Comps[5].TypedText);
> > 
> >   Comps = QueryParser::complete("set o", 5, QS);
> >   ASSERT_EQ(1u, Comps.size());
> >   EXPECT_EQ("utput ", Comps[0].TypedText);
> >   EXPECT_EQ("output", Comps[0].DisplayText);
> > 
> >   Comps = QueryParser::complete("match while", 11, QS);
> >   ASSERT_EQ(1u, Comps.size());
> >   EXPECT_EQ("Stmt(", Comps[0].TypedText);
> >   EXPECT_EQ("Matcher whileStmt(Matcher...)",
> > Comps[0].DisplayText);
> > }
> > ```
> > 
> > This is an actual piece of code from 
> > `extra/unittests/clang-query/QueryParserTest.cpp`. Yes, it is a test, but 
> > it still is a nice example of how many macros can be found in code 
> > (especially if we are talking about pure C or some weird C++).
> > 
> > Thus, I think it is reasonable to treat macro invocation as a 
> > `1`-"complexity" node.
> This "0" is not for the macro itself, but for the statements into which it 
> expands. Macro itself is not a statement. If we put "1" here, it would 
> produce a lot more complexity than you want.
> 
> That said, it's a good idea to treat every macro as a "complexity-1" 
> statement, just need to figure out how to implement that correctly :)
> 
> Perhaps scan the source range of the sequence for how many different macro 
> expansions are included, and add that number to complexity(?)
> This "0" is not for the macro itself, but for the statements into which it 
> expands. Macro itself is not a statement. If we put "1" here, it would 
> produce a lot more complexity than you want.

Sure, I understand that, this is why I didn't suggest putting `1` there.

> Perhaps scan the source range of the sequence for how many different macro 
> expansions are included, and add that number to complexity(?)

Yes, this is exactly the solution that would work. Since macros aren't in the 
AST we'd need to get through SourceRange anyway.


https://reviews.llvm.org/D23316



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

Re: [PATCH] D23316: [analyzer] Fixed the false-positives caused by macro generated code.

[Kirill Bobyrev via cfe-commits]

omtcyfz added inline comments.


Comment at: lib/Analysis/CloneDetection.cpp:436
@@ +435,3 @@
+if (IsInMacro) {
+  Signature.Complexity = 0;
+}

omtcyfz wrote:
> NoQ wrote:
> > omtcyfz wrote:
> > > Do I understand correctly that a code generated by a macro doesn't affect 
> > > "complexity" at all then?
> > > 
> > > ```
> > > TEST_F(QueryParserTest, Complete) {
> > >   std::vector Comps =
> > >   QueryParser::complete("", 0, QS);
> > >   ASSERT_EQ(6u, Comps.size());
> > >   EXPECT_EQ("help ", Comps[0].TypedText);
> > >   EXPECT_EQ("help", Comps[0].DisplayText);
> > >   EXPECT_EQ("let ", Comps[1].TypedText);
> > >   EXPECT_EQ("let", Comps[1].DisplayText);
> > >   EXPECT_EQ("match ", Comps[2].TypedText);
> > >   EXPECT_EQ("match", Comps[2].DisplayText);
> > >   EXPECT_EQ("set ", Comps[3].TypedText);
> > >   EXPECT_EQ("set", Comps[3].DisplayText);
> > >   EXPECT_EQ("unlet ", Comps[4].TypedText);
> > >   EXPECT_EQ("unlet", Comps[4].DisplayText);
> > >   EXPECT_EQ("quit", Comps[5].DisplayText);
> > >   EXPECT_EQ("quit ", Comps[5].TypedText);
> > > 
> > >   Comps = QueryParser::complete("set o", 5, QS);
> > >   ASSERT_EQ(1u, Comps.size());
> > >   EXPECT_EQ("utput ", Comps[0].TypedText);
> > >   EXPECT_EQ("output", Comps[0].DisplayText);
> > > 
> > >   Comps = QueryParser::complete("match while", 11, QS);
> > >   ASSERT_EQ(1u, Comps.size());
> > >   EXPECT_EQ("Stmt(", Comps[0].TypedText);
> > >   EXPECT_EQ("Matcher whileStmt(Matcher...)",
> > > Comps[0].DisplayText);
> > > }
> > > ```
> > > 
> > > This is an actual piece of code from 
> > > `extra/unittests/clang-query/QueryParserTest.cpp`. Yes, it is a test, but 
> > > it still is a nice example of how many macros can be found in code 
> > > (especially if we are talking about pure C or some weird C++).
> > > 
> > > Thus, I think it is reasonable to treat macro invocation as a 
> > > `1`-"complexity" node.
> > This "0" is not for the macro itself, but for the statements into which it 
> > expands. Macro itself is not a statement. If we put "1" here, it would 
> > produce a lot more complexity than you want.
> > 
> > That said, it's a good idea to treat every macro as a "complexity-1" 
> > statement, just need to figure out how to implement that correctly :)
> > 
> > Perhaps scan the source range of the sequence for how many different macro 
> > expansions are included, and add that number to complexity(?)
> > This "0" is not for the macro itself, but for the statements into which it 
> > expands. Macro itself is not a statement. If we put "1" here, it would 
> > produce a lot more complexity than you want.
> 
> Sure, I understand that, this is why I didn't suggest putting `1` there.
> 
> > Perhaps scan the source range of the sequence for how many different macro 
> > expansions are included, and add that number to complexity(?)
> 
> Yes, this is exactly the solution that would work. Since macros aren't in the 
> AST we'd need to get through SourceRange anyway.
Though, it has to be optimized in order to prevent parsing a SourceLocation 
multiple times.


https://reviews.llvm.org/D23316



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

Re: [PATCH] D23316: [analyzer] Fixed the false-positives caused by macro generated code.

[Kirill Bobyrev via cfe-commits]

omtcyfz added inline comments.


Comment at: lib/Analysis/CloneDetection.cpp:436
@@ +435,3 @@
+if (IsInMacro) {
+  Signature.Complexity = 0;
+}

omtcyfz wrote:
> omtcyfz wrote:
> > NoQ wrote:
> > > omtcyfz wrote:
> > > > Do I understand correctly that a code generated by a macro doesn't 
> > > > affect "complexity" at all then?
> > > > 
> > > > ```
> > > > TEST_F(QueryParserTest, Complete) {
> > > >   std::vector Comps =
> > > >   QueryParser::complete("", 0, QS);
> > > >   ASSERT_EQ(6u, Comps.size());
> > > >   EXPECT_EQ("help ", Comps[0].TypedText);
> > > >   EXPECT_EQ("help", Comps[0].DisplayText);
> > > >   EXPECT_EQ("let ", Comps[1].TypedText);
> > > >   EXPECT_EQ("let", Comps[1].DisplayText);
> > > >   EXPECT_EQ("match ", Comps[2].TypedText);
> > > >   EXPECT_EQ("match", Comps[2].DisplayText);
> > > >   EXPECT_EQ("set ", Comps[3].TypedText);
> > > >   EXPECT_EQ("set", Comps[3].DisplayText);
> > > >   EXPECT_EQ("unlet ", Comps[4].TypedText);
> > > >   EXPECT_EQ("unlet", Comps[4].DisplayText);
> > > >   EXPECT_EQ("quit", Comps[5].DisplayText);
> > > >   EXPECT_EQ("quit ", Comps[5].TypedText);
> > > > 
> > > >   Comps = QueryParser::complete("set o", 5, QS);
> > > >   ASSERT_EQ(1u, Comps.size());
> > > >   EXPECT_EQ("utput ", Comps[0].TypedText);
> > > >   EXPECT_EQ("output", Comps[0].DisplayText);
> > > > 
> > > >   Comps = QueryParser::complete("match while", 11, QS);
> > > >   ASSERT_EQ(1u, Comps.size());
> > > >   EXPECT_EQ("Stmt(", Comps[0].TypedText);
> > > >   EXPECT_EQ("Matcher whileStmt(Matcher...)",
> > > > Comps[0].DisplayText);
> > > > }
> > > > ```
> > > > 
> > > > This is an actual piece of code from 
> > > > `extra/unittests/clang-query/QueryParserTest.cpp`. Yes, it is a test, 
> > > > but it still is a nice example of how many macros can be found in code 
> > > > (especially if we are talking about pure C or some weird C++).
> > > > 
> > > > Thus, I think it is reasonable to treat macro invocation as a 
> > > > `1`-"complexity" node.
> > > This "0" is not for the macro itself, but for the statements into which 
> > > it expands. Macro itself is not a statement. If we put "1" here, it would 
> > > produce a lot more complexity than you want.
> > > 
> > > That said, it's a good idea to treat every macro as a "complexity-1" 
> > > statement, just need to figure out how to implement that correctly :)
> > > 
> > > Perhaps scan the source range of the sequence for how many different 
> > > macro expansions are included, and add that number to complexity(?)
> > > This "0" is not for the macro itself, but for the statements into which 
> > > it expands. Macro itself is not a statement. If we put "1" here, it would 
> > > produce a lot more complexity than you want.
> > 
> > Sure, I understand that, this is why I didn't suggest putting `1` there.
> > 
> > > Perhaps scan the source range of the sequence for how many different 
> > > macro expansions are included, and add that number to complexity(?)
> > 
> > Yes, this is exactly the solution that would work. Since macros aren't in 
> > the AST we'd need to get through SourceRange anyway.
> Though, it has to be optimized in order to prevent parsing a SourceLocation 
> multiple times.
*visiting each SourceLocation


https://reviews.llvm.org/D23316



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

Re: [PATCH] D23387: [Analyzer] Report found fields order in PaddingChecker.

[Ben Craig via cfe-commits]

bcraig added a comment.

Note: In the future, try to provide more context lines in the patch.  
http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

I'm mostly fine with the algorithmic changes.  I'm not thrilled with the 
formatting of the error message, but I'm not sure if much can be done about it 
without causing tons of work.

Here's my wishlist items...

1. List each field on an individual line, preferably with the type mentioned as 
well.
2. Get the field ordering information into a note or fixit.
3. Provide a more stable ("stable" as in stable sort) field ordering.  I've got 
an inline comment describing what I mean with that.

I'm not sure if my first two items are possible without large overhauls.  The 
third isn't a big deal either.

So to recap, I'm strongly in favor of the idea.  I'm weakly in favor of the 
implementation.  I'll give you a shot to polish things a bit if you have the 
inclination.  If you don't, then I won't let my idea of a perfect (and 
expensive) implementation stand in the way of this good implementation.


https://reviews.llvm.org/D23387



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

Re: [PATCH] D23387: [Analyzer] Report found fields order in PaddingChecker.

[Ben Craig via cfe-commits]

bcraig added a comment.

I am kind of interested in what the warning message for clang::Sema in 
clang/include/clang/Sema/Sema.h looks like.  When I last analyzed that file, 
there was a bunch of padding.  It's a very long class though, so it can stress 
the usability of your messages.


https://reviews.llvm.org/D23387



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

Re: [PATCH] D23361: [OpenCL] AMDGCN: Fix size_t type

[Yaxun Liu via cfe-commits]

yaxunl updated this revision to Diff 67687.
yaxunl added a comment.

Revised by Matt's comments.


https://reviews.llvm.org/D23361

Files:
  include/clang/Basic/TargetInfo.h
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets.cpp
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGenOpenCL/size_t.cl

Index: test/CodeGenOpenCL/size_t.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/size_t.cl
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -emit-llvm -O0 -triple spir-unknown-unknown -o - | FileCheck --check-prefix=SZ32 %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -emit-llvm -O0 -triple spir64-unknown-unknown -o - | FileCheck --check-prefix=SZ64 %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -emit-llvm -O0 -triple amdgcn-- -o - | FileCheck --check-prefix=SZ64 %s
+
+//SZ32: define{{.*}} i32 @test_private(i8* %x)
+//SZ32: ptrtoint i8* %{{.*}} to i32
+//SZ64: define{{.*}} i64 @test_private(i8* %x)
+//SZ64: ptrtoint i8* %{{.*}} to i64
+size_t test_private(private char* x) {
+  return (size_t)x;
+}
+
+//SZ32: define{{.*}} i32 @test_global(i8 addrspace(1)* %x)
+//SZ32: ptrtoint i8 addrspace(1)* %{{.*}} to i32
+//SZ64: define{{.*}} i64 @test_global(i8 addrspace(1)* %x)
+//SZ64: ptrtoint i8 addrspace(1)* %{{.*}} to i64
+intptr_t test_global(global char* x) {
+  return (intptr_t)x;
+}
+
+//SZ32: define{{.*}} i32 @test_constant(i8 addrspace(2)* %x)
+//SZ32: ptrtoint i8 addrspace(2)* %{{.*}} to i32
+//SZ64: define{{.*}} i64 @test_constant(i8 addrspace(2)* %x)
+//SZ64: ptrtoint i8 addrspace(2)* %{{.*}} to i64
+uintptr_t test_constant(constant char* x) {
+  return (uintptr_t)x;
+}
+
+//SZ32: define{{.*}} i32 @test_local(i8 addrspace(3)* %x, i8 addrspace(3)* %y)
+//SZ32: ptrtoint i8 addrspace(3)* %{{.*}} to i32
+//SZ32: ptrtoint i8 addrspace(3)* %{{.*}} to i32
+//SZ64: define{{.*}} i64 @test_local(i8 addrspace(3)* %x, i8 addrspace(3)* %y)
+//SZ64: ptrtoint i8 addrspace(3)* %{{.*}} to i64
+//SZ64: ptrtoint i8 addrspace(3)* %{{.*}} to i64
+ptrdiff_t test_local(local char* x, local char *y) {
+  return x - y;
+}
+
+//SZ32: define{{.*}} i32 @test_generic(i8 addrspace(4)* %x)
+//SZ32: ptrtoint i8 addrspace(4)* %{{.*}} to i32
+//SZ64: define{{.*}} i64 @test_generic(i8 addrspace(4)* %x)
+//SZ64: ptrtoint i8 addrspace(4)* %{{.*}} to i64
+size_t test_generic(generic char* x) {
+  return (size_t)x;
+}
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -104,7 +104,8 @@
   IntAlignInBytes =
 C.toCharUnitsFromBits(C.getTargetInfo().getIntAlign()).getQuantity();
   IntTy = llvm::IntegerType::get(LLVMContext, C.getTargetInfo().getIntWidth());
-  IntPtrTy = llvm::IntegerType::get(LLVMContext, PointerWidthInBits);
+  IntPtrTy = llvm::IntegerType::get(LLVMContext, LangOpts.OpenCL ?
+  C.getTargetInfo().getOpenCLMaxPointerWidth() : PointerWidthInBits);
   Int8PtrTy = Int8Ty->getPointerTo(0);
   Int8PtrPtrTy = Int8PtrTy->getPointerTo(0);
 
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -2004,6 +2004,10 @@
 }
   }
 
+  uint64_t getOpenCLMaxPointerWidth() const override {
+return getTriple().getArch() == llvm::Triple::amdgcn ? 64 : 32;
+  }
+
   const char * getClobbers() const override {
 return "";
   }
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -306,8 +306,9 @@
 }
 LongDoubleWidth = LongDoubleAlign = 128;
 
-assert(PointerWidth == 32 || PointerWidth == 64);
-bool Is32BitArch = PointerWidth == 32;
+unsigned MaxPointerWidth = getOpenCLMaxPointerWidth();
+assert(MaxPointerWidth == 32 || MaxPointerWidth == 64);
+bool Is32BitArch = MaxPointerWidth == 32;
 SizeType = Is32BitArch ? UnsignedInt : UnsignedLong;
 PtrDiffType = Is32BitArch ? SignedInt : SignedLong;
 IntPtrType = Is32BitArch ? SignedInt : SignedLong;
Index: include/clang/Basic/TargetInfo.h
===
--- include/clang/Basic/TargetInfo.h
+++ include/clang/Basic/TargetInfo.h
@@ -294,6 +294,11 @@
 return AddrSpace == 0 ? PointerAlign : getPointerAlignV(AddrSpace);
   }
 
+  /// \brief Return the maximum width of pointers for OpenCL on this target.
+  virtual uint64_t getOpenCLMaxPointerWidth() const {
+return PointerWidth;
+  }
+
   /// \brief Return the size of '_Bool' and C++ 'bool' for this target, in bits.
   unsigned getBoolWidth() const { return BoolWidth; }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22515: Added false-positive filter for unintended hash code collisions to the CloneDetector.

[Raphael Isemann via cfe-commits]

teemperor updated this revision to Diff 67691.
teemperor added a comment.

- Rebased the patch to the newest code base.

This patch is live again as it turned out that the CloneDetector can't handle 
large code bases without it. The simpler approach that we implemented instead 
consumed too much memory (e.g. at least 15 GB for analyzing SQLite) because the 
large data vectors and the quadratic space complexity of the sub-sequence 
search didn't play together very well.


https://reviews.llvm.org/D22515

Files:
  include/clang/Analysis/CloneDetection.h
  lib/Analysis/CloneDetection.cpp
  lib/StaticAnalyzer/Checkers/CloneChecker.cpp

Index: lib/StaticAnalyzer/Checkers/CloneChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CloneChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CloneChecker.cpp
@@ -51,15 +51,6 @@
  "Related code clone is here.");
 
 for (CloneDetector::CloneGroup &Group : CloneGroups) {
-  // For readability reasons we sort the clones by line numbers.
-  std::sort(Group.Sequences.begin(), Group.Sequences.end(),
-[&SM](const StmtSequence &LHS, const StmtSequence &RHS) {
-  return SM.isBeforeInTranslationUnit(LHS.getStartLoc(),
-  RHS.getStartLoc()) &&
- SM.isBeforeInTranslationUnit(LHS.getEndLoc(),
-  RHS.getEndLoc());
-});
-
   // We group the clones by printing the first as a warning and all others
   // as a note.
   DiagEngine.Report(Group.Sequences.front().getStartLoc(), WarnID);
Index: lib/Analysis/CloneDetection.cpp
===
--- lib/Analysis/CloneDetection.cpp
+++ lib/Analysis/CloneDetection.cpp
@@ -243,46 +243,35 @@
 /// This class defines what a code clone is: If it collects for two statements
 /// the same data, then those two statements are considered to be clones of each
 /// other.
-class StmtDataCollector : public ConstStmtVisitor {
+///
+/// All collected data is forwarded to the given data consumer of the type T.
+/// The data consumer class needs to provide a member method with the signature:
+///   write(const char *Data, size_t Size)
+template 
+class StmtDataCollector : public ConstStmtVisitor> {
 
   ASTContext &Context;
-  std::vector &CollectedData;
+  /// \brief The data sink to which all data is forwarded.
+  T &DataConsumer;
 
 public:
   /// \brief Collects data of the given Stmt.
   /// \param S The given statement.
   /// \param Context The ASTContext of S.
-  /// \param D The given data vector to which all collected data is appended.
-  StmtDataCollector(const Stmt *S, ASTContext &Context,
-std::vector &D)
-  : Context(Context), CollectedData(D) {
-Visit(S);
+  /// \param D The data sink to which all data is forwarded.
+  StmtDataCollector(const Stmt *S, ASTContext &Context, T &DataConsumer)
+  : Context(Context), DataConsumer(DataConsumer) {
+this->Visit(S);
   }
 
   // Below are utility methods for appending different data to the vector.
 
   void addData(CloneDetector::DataPiece Integer) {
-CollectedData.push_back(Integer);
+DataConsumer.write(reinterpret_cast(&Integer), sizeof(Integer));
   }
 
-  // FIXME: The functions below add long strings to the data vector which are
-  // probably not good for performance. Replace the strings with pointer values
-  // or a some other unique integer.
-
   void addData(llvm::StringRef Str) {
-if (Str.empty())
-  return;
-
-const size_t OldSize = CollectedData.size();
-
-const size_t PieceSize = sizeof(CloneDetector::DataPiece);
-// Calculate how many vector units we need to accomodate all string bytes.
-size_t RoundedUpPieceNumber = (Str.size() + PieceSize - 1) / PieceSize;
-// Allocate space for the string in the data vector.
-CollectedData.resize(CollectedData.size() + RoundedUpPieceNumber);
-
-// Copy the string to the allocated space at the end of the vector.
-std::memcpy(CollectedData.data() + OldSize, Str.data(), Str.size());
+DataConsumer.write(Str.data(), Str.size());
   }
 
   void addData(const QualType &QT) { addData(QT.getAsString()); }
@@ -426,8 +415,10 @@
 // Create an empty signature that will be filled in this method.
 CloneDetector::CloneSignature Signature;
 
+llvm::hash_stream Hash;
+
 // Collect all relevant data from S and put it into the empty signature.
-StmtDataCollector(S, Context, Signature.Data);
+StmtDataCollector(S, Context, Hash);
 
 // If this code is generated by a macro, it won't increase the complexity
 // level of it's containing clone. This prevents false-positives caused by
@@ -457,7 +448,8 @@
   auto ChildSignature = generateSignatures(Child);
 
   // Add the collected data to the signature of the curren

Re: [PATCH] D15227: [analyzer] Valist checkers.

[Artem Dergachev via cfe-commits]

NoQ added a comment.

The checker's in great shape! I see a few minor things, but that's it.

The checks are split into two sections ("uninitialized" and "unterminated"), 
but there seem to be more auxiliary checks provided (eg. "copies into itself") 
that are on for both checkers, do you think you need to add more flags to make 
the checker more flexible, or maybe just merge everything into a single checker?

Because plist tests are so heavy: if all you wanted to test was how your bug 
reporter visitor works, did you consider using `-analyzer-output=text` instead? 
(this is not the default output mode; it has `note:` diagnostics displaying the 
path, which you can catch with `expected-note{{}}`). Also, you could probably 
write special tests for the visitor, instead of testing the visitors on all 
tests, regardless of text/plist. But i'm merely sharing this hint, i don't 
think it's worth rewriting once we already have plist tests.



Comment at: lib/StaticAnalyzer/Checkers/ValistChecker.cpp:177
@@ +176,3 @@
+  if (ExplodedNode *N = C.addTransition(State))
+reportLeakedVALists(LeakedVALists, "Initialized va_list", " is leaked", C,
+N);

dcoughlin wrote:
> Are there tests for this diagnostic? I don't see any.
In the other file, first few tests in `valist-unterminated.c`.


Comment at: lib/StaticAnalyzer/Checkers/ValistChecker.cpp:185
@@ +184,3 @@
+  const auto *TReg = dyn_cast_or_null(Reg);
+  const auto *EReg = dyn_cast_or_null(TReg);
+  return EReg ? EReg->getSuperRegion() : TReg;

At first, i thought that you're trying to strip casts from symbolic pointers 
here. But then i figured out that you are stripping an implementation detail 
here - as even `VarRegion`-based VAs reach here in the form of "`element{va,0 
S64b,struct __va_list_tag}`". I've a feeling this deserves a comment.


Comment at: lib/StaticAnalyzer/Checkers/ValistChecker.cpp:254
@@ +253,3 @@
+const Stmt *StartCallStmt = nullptr;
+if (Optional Exit = P.getAs())
+  StartCallStmt = Exit->getCalleeContext()->getCallSite();

Does `static const Stmt *PathDiagnosticLocation::getStmt(const ExplodedNode *)` 
work for you? Because i guess it was specifically designed for that purpose. 
Here and in one more place where this code is duplicated.


Comment at: test/Analysis/valist-uninitialized.c:64
@@ +63,3 @@
+  (void)va_arg(*y, int); //expected-warning{{va_arg() is called on an 
uninitialized va_list}}
+} // no-warning 
+

Trailing whitespace :)


Comment at: test/Analysis/valist-uninitialized.c:98
@@ +97,3 @@
+  va_start(va, fst);
+  va_copy(va, va); // expected-warning{{va_list 'va' is copied onto itself}} 
+  va_end(va);

A bit more trailing whitespace here and below.


Comment at: test/Analysis/valist-uninitialized.c:178
@@ +177,3 @@
+  va_list va;
+  some_library_function(n, va);
+} //no-warning

> Like the compiler, the static analyzer treats some functions differently if
> they come from a system header -- for example, //it is assumed that system//
> functions do not arbitrarily free() their parameters//, and that some bugs
> found in system headers cannot be fixed by the user and should be
> suppressed.

Perhaps library functions can be assumed to never `va_end()` `va_list`s either. 
The idea behind the trick, which is also used in, say, `SimpleStreamChecker`, 
is that we know all library functions, we can list all library functions that 
`va_end()` `va_list`s, so it's safe to assume that all other library functions 
do not `va_end()` `va_list`s (completely unlike other, non-standard functions 
with unavailable bodies).

So i think this test should warn, and it shouldn't be hard to fix. That said, 
some checkers (eg. `PthreadLockChecker`) do not implement this trick, so it's 
not critical, but it may give you slightly more positives.


Comment at: test/Analysis/valist-unterminated.c:42
@@ +41,3 @@
+}
+  //FIXME: this should NOT cause a warning
+

Wow, these tests are mind-breaking :))

Is it ok if i ask to put the FIXME above the test or above no-warning or into 
function name? Because it took some time for me to figure out that `f7` is not 
FIXME, being used to the traditional positioning of the FIXME comment><


https://reviews.llvm.org/D15227



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

Re: [PATCH] D22782: Added 'inline' attribute to __init to inline the basic_string's constructor

[David Blaikie via cfe-commits]

On Wed, Aug 10, 2016 at 5:39 PM Eric Fiselier  wrote:

> On Mon, Aug 8, 2016 at 9:32 AM, David Blaikie via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> I'm still (as per another similar thread) a bit concerned this is working
>> around a compiler optimizer bug - I'd love to see a standalone example of
>> this behavior (that adding the inline keyword to an already
>> inline/available definition is what's causing the inlining) so we can look
>> at what the compiler might be doing wrong (& consider fixing that as the
>> root cause instead of having to change every instance of this problem) -
>> preferably an example without std::string/headers/etc (so, preprocessed and
>> reduced).
>>
>
> Normally I would agree that this is an optimizer bug, but std::string is
> externally instantiated.
> It seems reasonable for Clang *not* to inline externally instantiated
> templates unless requested.
>

Sort of - but it does have the ability to create what it describes as
"available externally" definitions that can be used for inlining (& other
interprocedural analysis), but don't need to be emitted into the final
object if a call to them remains (ie: that call can be resolved by an
external definition)


> Simply removing the extern template declaration will resolve these
> inlining issues.
>
> Here is the standalone example you requested: https://godbolt.org/g/aVwEMR
>

Simplifying the test case a bit (while still preserving the behavior):

template 
struct string {
  ~string();
};

template 
string::~string() {}

// Remove this line to see the difference
#ifdef EXTERN
extern template struct string;
#endif

template 
void sink(string);

int main() {
  sink(string());
}

I still see the big difference (the use of exception handling to ensure the
dtor is called, etc) in this example when EXTERN is defined.

If I move the definition of ~string into the string class definition, the
difference goes away - the dtor call and associated exception handling is
optimized away.

& yes, adding 'inline' to the out-of-line definition produces the same
behavior. And in both cases Clang does the right thing of using an
available_externally definition, not a comdat/linkonce_odr definition.
(this is why I was confused by the original bug report - the existence or
absence of a definition of the function in the resulting IR or object file
isn't proof that the compiler didn't have the information necessary to
optimize the code)

Perhaps this is a language limitation that the out of line definition can't
be implicitly instantiated in this situation? Or we try not to do so for
some reason.

(Richard?)

I take it the always_inline part of this is necessary for the whole libc++
ABI strategy? (something I've never quite wrapped my head around the
details of)

>
>
>>
>> But up to you folks maintaining libc++ whether this goes in of course,
>> just my 2c.
>>
>> On Wed, Aug 3, 2016 at 5:23 AM Aditya Kumar via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> hiraditya added a comment.
>>>
>>> In https://reviews.llvm.org/D22782#504416, @EricWF wrote:
>>>
>>> > The change itself LGTM, although we probably want to inline the
>>> forward/input iterator __init's as well.
>>> >
>>> > However I would like to see a small benchmark that demonstrates the
>>> performance change. Please try and write the benchmark using Google
>>> Benchmark.
>>> >  Some helpful links:
>>> >
>>> > - http://libcxx.llvm.org/docs/TestingLibcxx.html#building-benchmarks
>>> > - http://github.com/google/benchmark
>>>
>>>
>>> Sure,
>>> We'll come up with a synthetic benchmark to expose performance
>>> improvements.
>>>
>>> Thanks,
>>>
>>>
>>> https://reviews.llvm.org/D22782
>>>
>>>
>>>
>>> ___
>>> 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
>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23168: emit_DW_AT_noreturn flag

[Victor via cfe-commits]

vleschuk updated this revision to Diff 67694.
vleschuk added a comment.

Add IR tests


https://reviews.llvm.org/D23168

Files:
  lib/AST/Decl.cpp
  lib/CodeGen/CGDebugInfo.cpp
  test/Frontend/dinoreturn.c
  test/Frontend/dinoreturn.cpp
  test/Frontend/dinoreturn.m

Index: test/Frontend/dinoreturn.m
===
--- /dev/null
+++ test/Frontend/dinoreturn.m
@@ -0,0 +1,5 @@
+// RUN: %clang %s -c -emit-llvm -S -g -o - | FileCheck %s
+// CHECK: DIFlagNoReturn 
+__attribute__ ((noreturn)) void f() {
+  exit(0);
+}
Index: test/Frontend/dinoreturn.cpp
===
--- /dev/null
+++ test/Frontend/dinoreturn.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang %s -c -std=c++11 -x c++ -emit-llvm -S -g -o - | FileCheck %s
+// CHECK: DIFlagNoReturn 
+[[ noreturn ]] void f() {
+  throw 1;
+}
Index: test/Frontend/dinoreturn.c
===
--- /dev/null
+++ test/Frontend/dinoreturn.c
@@ -0,0 +1,6 @@
+// RUN: %clang %s -c -std=c11 -emit-llvm -S -g -o - | FileCheck %s
+// CHECK: DIFlagNoReturn 
+#include 
+_Noreturn void f() {
+  exit(0);
+}
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2641,6 +2641,9 @@
   llvm::DIScope *Mod = getParentModuleOrNull(RDecl);
   FDContext = getContextDescriptor(RDecl, Mod ? Mod : TheCU);
 }
+// Check if it is a noreturn-marked function
+if (FD->isNoReturn())
+  Flags |= llvm::DINode::FlagNoReturn;
 // Collect template parameters.
 TParamsArray = CollectFunctionTemplateParams(FD, Unit);
   }
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2653,9 +2653,13 @@
 }
 
 bool FunctionDecl::isNoReturn() const {
-  return hasAttr() || hasAttr() ||
- hasAttr() ||
- getType()->getAs()->getNoReturnAttr();
+  bool HasNoReturnAttr = hasAttr() || 
hasAttr()
+ || hasAttr();
+  const auto *FuncType = getType()->getAs();
+  bool TypeHasNoReturnAttr = false;
+  if (FuncType)
+TypeHasNoReturnAttr = FuncType->getNoReturnAttr();
+  return HasNoReturnAttr || TypeHasNoReturnAttr;
 }
 
 void


Index: test/Frontend/dinoreturn.m
===
--- /dev/null
+++ test/Frontend/dinoreturn.m
@@ -0,0 +1,5 @@
+// RUN: %clang %s -c -emit-llvm -S -g -o - | FileCheck %s
+// CHECK: DIFlagNoReturn 
+__attribute__ ((noreturn)) void f() {
+  exit(0);
+}
Index: test/Frontend/dinoreturn.cpp
===
--- /dev/null
+++ test/Frontend/dinoreturn.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang %s -c -std=c++11 -x c++ -emit-llvm -S -g -o - | FileCheck %s
+// CHECK: DIFlagNoReturn 
+[[ noreturn ]] void f() {
+  throw 1;
+}
Index: test/Frontend/dinoreturn.c
===
--- /dev/null
+++ test/Frontend/dinoreturn.c
@@ -0,0 +1,6 @@
+// RUN: %clang %s -c -std=c11 -emit-llvm -S -g -o - | FileCheck %s
+// CHECK: DIFlagNoReturn 
+#include 
+_Noreturn void f() {
+  exit(0);
+}
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2641,6 +2641,9 @@
   llvm::DIScope *Mod = getParentModuleOrNull(RDecl);
   FDContext = getContextDescriptor(RDecl, Mod ? Mod : TheCU);
 }
+// Check if it is a noreturn-marked function
+if (FD->isNoReturn())
+  Flags |= llvm::DINode::FlagNoReturn;
 // Collect template parameters.
 TParamsArray = CollectFunctionTemplateParams(FD, Unit);
   }
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2653,9 +2653,13 @@
 }
 
 bool FunctionDecl::isNoReturn() const {
-  return hasAttr() || hasAttr() ||
- hasAttr() ||
- getType()->getAs()->getNoReturnAttr();
+  bool HasNoReturnAttr = hasAttr() || hasAttr()
+ || hasAttr();
+  const auto *FuncType = getType()->getAs();
+  bool TypeHasNoReturnAttr = false;
+  if (FuncType)
+TypeHasNoReturnAttr = FuncType->getNoReturnAttr();
+  return HasNoReturnAttr || TypeHasNoReturnAttr;
 }
 
 void
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22782: Added 'inline' attribute to __init to inline the basic_string's constructor

[Sebastian Pop via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL278356: Add 'inline' attribute to __init to inline the 
basic_string's constructor (authored by spop).

Changed prior to commit:
  https://reviews.llvm.org/D22782?vs=67597&id=67699#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D22782

Files:
  libcxx/trunk/include/string

Index: libcxx/trunk/include/string
===
--- libcxx/trunk/include/string
+++ libcxx/trunk/include/string
@@ -1442,6 +1442,7 @@
 }
 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 void
 basic_string<_CharT, _Traits, _Allocator>::__init(const value_type* __s, 
size_type __sz, size_type __reserve)
 {
@@ -1466,6 +1467,7 @@
 }
 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 void
 basic_string<_CharT, _Traits, _Allocator>::__init(const value_type* __s, 
size_type __sz)
 {
@@ -1603,6 +1605,7 @@
 #endif  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 void
 basic_string<_CharT, _Traits, _Allocator>::__init(size_type __n, value_type 
__c)
 {
@@ -1699,6 +1702,7 @@
 
 template 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 typename enable_if
 <
 __is_exactly_input_iterator<_InputIterator>::value,
@@ -1726,6 +1730,7 @@
 
 template 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 typename enable_if
 <
 __is_forward_iterator<_ForwardIterator>::value,


Index: libcxx/trunk/include/string
===
--- libcxx/trunk/include/string
+++ libcxx/trunk/include/string
@@ -1442,6 +1442,7 @@
 }
 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 void
 basic_string<_CharT, _Traits, _Allocator>::__init(const value_type* __s, size_type __sz, size_type __reserve)
 {
@@ -1466,6 +1467,7 @@
 }
 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 void
 basic_string<_CharT, _Traits, _Allocator>::__init(const value_type* __s, size_type __sz)
 {
@@ -1603,6 +1605,7 @@
 #endif  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 void
 basic_string<_CharT, _Traits, _Allocator>::__init(size_type __n, value_type __c)
 {
@@ -1699,6 +1702,7 @@
 
 template 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 typename enable_if
 <
 __is_exactly_input_iterator<_InputIterator>::value,
@@ -1726,6 +1730,7 @@
 
 template 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 typename enable_if
 <
 __is_forward_iterator<_ForwardIterator>::value,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D23409: Make clang-tidy work with clang-cl

[Zachary Turner via cfe-commits]

zturner created this revision.
zturner added reviewers: alexfh, djasper, rnk.
zturner added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

Attempting to run clang-tidy with the clang-cl driver results in a number of 
warnings and errors that make the tool unusable.  The two main issues, and the 
solutions implemented here are:

  # Command lines are tokenized differently on Windows and non-windows 
platforms.  Using standard tokenization on Windows leads to -- among other 
things -- all path separators being stripped from paths.  Obviously this won't 
work, so the fix implemented here is to use the correct windows command line 
tokenizer when clang-tidy is running on Windows.
  # The underlying clang driver does not attempt to auto-detect driver mode 
(CL, GCC, etc) from the program name.  It relies only on the existence of a 
`--driver-mode=` option.  We already have a function to detect the driver 
mode based on the program name, so the solution implemented here is to use the 
program name as a default, which will be overridden by the existence of a 
`--driver-mode` option.

With this patch, clang-tidy runs with clang-cl.

```
D:\src\llvm\tools\lldb>d:\src\llvmbuild\ninja\bin\clang-tidy.exe source\lldb.cpp
1448 warnings generated.
warning: argument unused during compilation: '-mincremental-linker-compatible' 
[clang-diagnostic-unused-command-line-argument]
warning: unknown argument ignored in clang-cl: 
'-resource-dir=d:\src\llvmbuild\ninja\bin\..\lib\clang\4.0.0' 
[clang-diagnostic-unk
nown-argument]
D:\src\llvm\tools\lldb\source\lldb.cpp:71:24: warning: invalid case style for 
variable 'g_version_str' [readability-identifier-nam
ing]
static std::string g_version_str;
   ^
D:\src\llvm\tools\lldb\source\lldb.cpp:76:22: warning: invalid case style for 
variable 'lldb_repo' [readability-identifier-naming]

const char * lldb_repo = GetLLDBRepository();
 ^
D:\src\llvm\tools\lldb\source\lldb.cpp:83:21: warning: invalid case style for 
variable 'lldb_rev' [readability-identifier-naming]
const char *lldb_rev = GetLLDBRevision();
^
D:\src\llvm\tools\lldb\source\lldb.cpp:89:21: warning: invalid case style for 
variable 'clang_rev' [readability-identifier-naming]

std::string clang_rev (clang::getClangRevision());
^
D:\src\llvm\tools\lldb\source\lldb.cpp:95:21: warning: invalid case style for 
variable 'llvm_rev' [readability-identifier-naming]
std::string llvm_rev (clang::getLLVMRevision());
^
Suppressed 1441 warnings (1441 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use 
-system-headers to display errors from system headers as
well.
```

https://reviews.llvm.org/D23409

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  lib/Tooling/JSONCompilationDatabase.cpp

Index: lib/Tooling/JSONCompilationDatabase.cpp
===
--- lib/Tooling/JSONCompilationDatabase.cpp
+++ lib/Tooling/JSONCompilationDatabase.cpp
@@ -16,7 +16,10 @@
 #include "clang/Tooling/CompilationDatabasePluginRegistry.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Allocator.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/StringSaver.h"
 #include 
 
 namespace clang {
@@ -113,8 +116,17 @@
 
 std::vector unescapeCommandLine(
 StringRef EscapedCommandLine) {
+#if defined(LLVM_ON_WIN32)
+  llvm::BumpPtrAllocator Alloc;
+  llvm::StringSaver Saver(Alloc);
+  llvm::SmallVector T;
+  llvm::cl::TokenizeWindowsCommandLine(EscapedCommandLine, Saver, T);
+  std::vector Result(T.begin(), T.end());
+  return Result;
+#else
   CommandLineArgumentParser parser(EscapedCommandLine);
   return parser.parse();
+#endif
 }
 
 class JSONCompilationDatabasePlugin : public CompilationDatabasePlugin {
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -88,31 +88,39 @@
   llvm::DeleteContainerSeconds(ToolChains);
 }
 
-void Driver::ParseDriverMode(ArrayRef Args) {
-  const std::string OptName =
-  getOpts().getOption(options::OPT_driver_mode).getPrefixedName();
+void Driver::ParseDriverMode(StringRef ProgramName,
+ ArrayRef Args) {
+  auto Default = ToolChain::getTargetAndModeFromProgramName(ProgramName);
+  StringRef DefaultMode(Default.second);
+  setDriverModeFromOption(DefaultMode);
 
   for (const char *ArgPtr : Args) {
 // Ingore nullptrs, they are response file's EOL markers
 if (ArgPtr == nullptr)
   continue;
 const StringRef Arg = ArgPtr;
-if (!Arg.startswith(OptName))
-  continue;
+setDriverModeFromOption(Arg);
+  }
+}
+
+void Driver::setDriverModeFromOption(StringRef Opt) {
+  const std::string OptName =
+  getOpts().getOption(options::OP

[libcxx] r278357 - test: relax the FS test a slight bit to be more reliable

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Thu Aug 11 11:58:12 2016
New Revision: 278357

URL: http://llvm.org/viewvc/llvm-project?rev=278357&view=rev
Log:
test: relax the FS test a slight bit to be more reliable

Some filesystems track atime always.  This relaxes the test to accept either a
filesystem which does not accurately track atime or does track the atime
accurately.  This allows the test to pass on filesystems mounted with
`strictatime` on Linux or on macOS.

Modified:

libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp

Modified: 
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp?rev=278357&r1=278356&r2=278357&view=diff
==
--- 
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
 Thu Aug 11 11:58:12 2016
@@ -158,7 +158,8 @@ TEST_CASE(get_last_write_time_dynamic_en
 
 TEST_CHECK(ftime2 > ftime);
 TEST_CHECK(dtime2 > dtime);
-TEST_CHECK(LastAccessTime(file) == file_access_time);
+TEST_CHECK(LastAccessTime(file) == file_access_time ||
+   LastAccessTime(file) == Clock::to_time_t(ftime2));
 TEST_CHECK(LastAccessTime(dir) == dir_access_time);
 }
 


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

Re: [PATCH] D23316: [analyzer] Fixed the false-positives caused by macro generated code.

[Artem Dergachev via cfe-commits]

NoQ added a comment.

No comments of my own, the patch looks good :)



Comment at: lib/Analysis/CloneDetection.cpp:436
@@ +435,3 @@
+if (IsInMacro) {
+  Signature.Complexity = 0;
+}

omtcyfz wrote:
> omtcyfz wrote:
> > omtcyfz wrote:
> > > NoQ wrote:
> > > > omtcyfz wrote:
> > > > > Do I understand correctly that a code generated by a macro doesn't 
> > > > > affect "complexity" at all then?
> > > > > 
> > > > > ```
> > > > > TEST_F(QueryParserTest, Complete) {
> > > > >   std::vector Comps =
> > > > >   QueryParser::complete("", 0, QS);
> > > > >   ASSERT_EQ(6u, Comps.size());
> > > > >   EXPECT_EQ("help ", Comps[0].TypedText);
> > > > >   EXPECT_EQ("help", Comps[0].DisplayText);
> > > > >   EXPECT_EQ("let ", Comps[1].TypedText);
> > > > >   EXPECT_EQ("let", Comps[1].DisplayText);
> > > > >   EXPECT_EQ("match ", Comps[2].TypedText);
> > > > >   EXPECT_EQ("match", Comps[2].DisplayText);
> > > > >   EXPECT_EQ("set ", Comps[3].TypedText);
> > > > >   EXPECT_EQ("set", Comps[3].DisplayText);
> > > > >   EXPECT_EQ("unlet ", Comps[4].TypedText);
> > > > >   EXPECT_EQ("unlet", Comps[4].DisplayText);
> > > > >   EXPECT_EQ("quit", Comps[5].DisplayText);
> > > > >   EXPECT_EQ("quit ", Comps[5].TypedText);
> > > > > 
> > > > >   Comps = QueryParser::complete("set o", 5, QS);
> > > > >   ASSERT_EQ(1u, Comps.size());
> > > > >   EXPECT_EQ("utput ", Comps[0].TypedText);
> > > > >   EXPECT_EQ("output", Comps[0].DisplayText);
> > > > > 
> > > > >   Comps = QueryParser::complete("match while", 11, QS);
> > > > >   ASSERT_EQ(1u, Comps.size());
> > > > >   EXPECT_EQ("Stmt(", Comps[0].TypedText);
> > > > >   EXPECT_EQ("Matcher whileStmt(Matcher...)",
> > > > > Comps[0].DisplayText);
> > > > > }
> > > > > ```
> > > > > 
> > > > > This is an actual piece of code from 
> > > > > `extra/unittests/clang-query/QueryParserTest.cpp`. Yes, it is a test, 
> > > > > but it still is a nice example of how many macros can be found in 
> > > > > code (especially if we are talking about pure C or some weird C++).
> > > > > 
> > > > > Thus, I think it is reasonable to treat macro invocation as a 
> > > > > `1`-"complexity" node.
> > > > This "0" is not for the macro itself, but for the statements into which 
> > > > it expands. Macro itself is not a statement. If we put "1" here, it 
> > > > would produce a lot more complexity than you want.
> > > > 
> > > > That said, it's a good idea to treat every macro as a "complexity-1" 
> > > > statement, just need to figure out how to implement that correctly :)
> > > > 
> > > > Perhaps scan the source range of the sequence for how many different 
> > > > macro expansions are included, and add that number to complexity(?)
> > > > This "0" is not for the macro itself, but for the statements into which 
> > > > it expands. Macro itself is not a statement. If we put "1" here, it 
> > > > would produce a lot more complexity than you want.
> > > 
> > > Sure, I understand that, this is why I didn't suggest putting `1` there.
> > > 
> > > > Perhaps scan the source range of the sequence for how many different 
> > > > macro expansions are included, and add that number to complexity(?)
> > > 
> > > Yes, this is exactly the solution that would work. Since macros aren't in 
> > > the AST we'd need to get through SourceRange anyway.
> > Though, it has to be optimized in order to prevent parsing a SourceLocation 
> > multiple times.
> *visiting each SourceLocation
Yeah, as a rough approximation we could count macro expansions within the 
current statement's children...


Comment at: test/Analysis/copypaste/macro-complexity.cpp:19
@@ +18,3 @@
+// Prevents that this tests won't accidentially pass because the code inside
+// the macro isn't complex enough.
+int foo(int a, int b) {  // expected-warning{{Detected code clone.}}

v.g.vassilev wrote:
> I am not sure I understand this comment. Could you reword?
Without macros, the same code would constitute a complex clone.
Wrapping code into macros reduces complexity of the code.
This tests the test above.

^(tried out some reword-ings)


https://reviews.llvm.org/D23316



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

Re: [PATCH] D22782: Added 'inline' attribute to __init to inline the basic_string's constructor

[Sebastian Pop via cfe-commits]

sebpop added a comment.

In https://reviews.llvm.org/D22782#512337, @EricWF wrote:

> I would love to see a benchmark with this, but I've done enough investigating 
> on my own that I *know* this patch is beneficial.


This patch was motivated by perf analysis we did on a proprietary benchmark in 
which we have seen a reduction of about 1 billion instructions (out of 10B) on 
x86_64-linux and aarch64-linux.


Repository:
  rL LLVM

https://reviews.llvm.org/D22782



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

[libcxx] r278356 - Add 'inline' attribute to __init to inline the basic_string's constructor

[Sebastian Pop via cfe-commits]

Author: spop
Date: Thu Aug 11 11:51:48 2016
New Revision: 278356

URL: http://llvm.org/viewvc/llvm-project?rev=278356&view=rev
Log:
Add 'inline' attribute to __init to inline the basic_string's constructor

basic_string's constructor calls init which was not getting inlined.  This
prevented optimization of const string as init would appear as a call in between
a string's def and use.

Patch by Laxman Sole and Aditya Kumar.

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

Modified:
libcxx/trunk/include/string

Modified: libcxx/trunk/include/string
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=278356&r1=278355&r2=278356&view=diff
==
--- libcxx/trunk/include/string (original)
+++ libcxx/trunk/include/string Thu Aug 11 11:51:48 2016
@@ -1442,6 +1442,7 @@ basic_string<_CharT, _Traits, _Allocator
 }
 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 void
 basic_string<_CharT, _Traits, _Allocator>::__init(const value_type* __s, 
size_type __sz, size_type __reserve)
 {
@@ -1466,6 +1467,7 @@ basic_string<_CharT, _Traits, _Allocator
 }
 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 void
 basic_string<_CharT, _Traits, _Allocator>::__init(const value_type* __s, 
size_type __sz)
 {
@@ -1603,6 +1605,7 @@ basic_string<_CharT, _Traits, _Allocator
 #endif  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 void
 basic_string<_CharT, _Traits, _Allocator>::__init(size_type __n, value_type 
__c)
 {
@@ -1699,6 +1702,7 @@ basic_string<_CharT, _Traits, _Allocator
 
 template 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 typename enable_if
 <
 __is_exactly_input_iterator<_InputIterator>::value,
@@ -1726,6 +1730,7 @@ basic_string<_CharT, _Traits, _Allocator
 
 template 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 typename enable_if
 <
 __is_forward_iterator<_ForwardIterator>::value,


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

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

[Aaron Ballman via cfe-commits]

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

Patch generally LGTM, though I wonder if there's a way we can add test coverage 
for this.


https://reviews.llvm.org/D23409



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

Re: [PATCH] D23361: [OpenCL] AMDGCN: Fix size_t type

[Anastasia Stulova via cfe-commits]

Anastasia added a comment.

I think Clang is supposed to generate the IR specific to the target 
architecture. It seems strange to ignore the pointer size. I am not sure if it 
might lead to some issues for the backends.



Comment at: lib/CodeGen/CodeGenModule.cpp:107
@@ -106,2 +106,3 @@
   IntTy = llvm::IntegerType::get(LLVMContext, C.getTargetInfo().getIntWidth());
-  IntPtrTy = llvm::IntegerType::get(LLVMContext, PointerWidthInBits);
+  IntPtrTy = llvm::IntegerType::get(LLVMContext, LangOpts.OpenCL ?
+  C.getTargetInfo().getOpenCLMaxPointerWidth() : PointerWidthInBits);

It seems wrong to call OpenCL specific function in generic code path?


https://reviews.llvm.org/D23361



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

Re: [PATCH] D23322: [OpenCL] AMDGPU: Add extensions cl_amd_media_ops and cl_amd_media_ops2

[Anastasia Stulova via cfe-commits]

Anastasia added a comment.

Is this related to our discussion on cfe-dev about the extensions and also the 
earlier review you have created: https://reviews.llvm.org/D21698?


https://reviews.llvm.org/D23322



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

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

[Reid Kleckner via cfe-commits]

rnk added inline comments.


Comment at: lib/Driver/Driver.cpp:93
@@ +92,3 @@
+ ArrayRef Args) {
+  auto Default = ToolChain::getTargetAndModeFromProgramName(ProgramName);
+  StringRef DefaultMode(Default.second);

Why not change ToolInvocation::run() to behave more like clang's main? I'd 
rather not do this twice, mostly for consistency with the regular driver, not 
because it's inefficient.


Comment at: lib/Tooling/JSONCompilationDatabase.cpp:119
@@ -115,1 +118,3 @@
 StringRef EscapedCommandLine) {
+#if defined(LLVM_ON_WIN32)
+  llvm::BumpPtrAllocator Alloc;

It would be nice if the JSON file just told us which quoting mechanism it was 
using. You can imagine building the compilation database on one system and 
sending it off to another for indexing.


Comment at: lib/Tooling/JSONCompilationDatabase.cpp:127
@@ -116,2 +126,3 @@
+#else
   CommandLineArgumentParser parser(EscapedCommandLine);
   return parser.parse();

I bet we could replace this with TokenizeGNUCommandLine some other day.


https://reviews.llvm.org/D23409



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

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

[Zachary Turner via cfe-commits]

zturner added a comment.

In https://reviews.llvm.org/D23409#512720, @aaron.ballman wrote:

> Patch generally LGTM, though I wonder if there's a way we can add test 
> coverage for this.


I'd imagine we can just enable the clang-tidy test suite on Windows (I'm 
assuming it's currently disabled).  I'll look into it.


https://reviews.llvm.org/D23409



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

Re: [PATCH] D23168: emit_DW_AT_noreturn flag

[Adrian Prantl via cfe-commits]

aprantl added inline comments.


Comment at: test/Frontend/dinoreturn.c:2
@@ +1,3 @@
+// RUN: %clang %s -c -std=c11 -emit-llvm -S -g -o - | FileCheck %s
+// CHECK: DIFlagNoReturn 
+#include 

Please check for more context. For example:
  CHECK: !DISubprogram({{.*}}name: "f"{{.*}}flags: DIFlagNoreturn
would be good here.


https://reviews.llvm.org/D23168



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

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

[Zachary Turner via cfe-commits]

zturner added inline comments.


Comment at: lib/Tooling/JSONCompilationDatabase.cpp:119
@@ -115,1 +118,3 @@
 StringRef EscapedCommandLine) {
+#if defined(LLVM_ON_WIN32)
+  llvm::BumpPtrAllocator Alloc;

rnk wrote:
> It would be nice if the JSON file just told us which quoting mechanism it was 
> using. You can imagine building the compilation database on one system and 
> sending it off to another for indexing.
Correct me if I've got this wrong but:

1. If you had a compile command database generated on non-Windows, and you used 
it on Windows, then it would necessarily be referring to something other than 
clang-cl right?  In which case, we don't support running clang in non-cl mode 
on Windows.

2. If you had a compile command database generated on Windows, then the only 
supported configuration would involve clang-cl, in which case you couldn't use 
it on non-Windows.

Is there a supported use case where a non-Windows compile database would be 
used on Windows or vice versa?


https://reviews.llvm.org/D23409



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

Re: [PATCH] D23167: emit_DW_AT_noreturn flag

[Adrian Prantl via cfe-commits]

aprantl added a comment.

You are adding a new constant to the LLVM IR, so there should be a round-trip 
test that tests the LLVM IR parser, bitcode writer, bitcode reader, and LLVM IR 
printer to make sure we're handling the new constant correctly.

One easy way to do this is by adding the new flag to the already existing test 
in test/IR/disubprogram.ll.


https://reviews.llvm.org/D23167



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

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

[Zachary Turner via cfe-commits]

zturner added inline comments.


Comment at: lib/Driver/Driver.cpp:93
@@ +92,3 @@
+ ArrayRef Args) {
+  auto Default = ToolChain::getTargetAndModeFromProgramName(ProgramName);
+  StringRef DefaultMode(Default.second);

rnk wrote:
> Why not change ToolInvocation::run() to behave more like clang's main? I'd 
> rather not do this twice, mostly for consistency with the regular driver, not 
> because it's inefficient.
I am very inexperienced with clang's driver model, but it seems to me like 
putting it here is actually the *correct* way, and what clang is doing is sort 
of a hackish workaround for the fact that it needs to parse response files the 
way it does.

If someone else comes along and builds another tool based off of clang, we 
don't want them to have to remember to do all this every single time.  Seems 
like the Driver should "just work", and if a particular tool (such as clang) 
needs to do something funky, that's on the tool.

Thoughts?


https://reviews.llvm.org/D23409



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

Re: [PATCH] D23322: [OpenCL] AMDGPU: Add extensions cl_amd_media_ops and cl_amd_media_ops2

[Yaxun Liu via cfe-commits]

yaxunl added a comment.

In https://reviews.llvm.org/D23322#512732, @Anastasia wrote:

> Is this related to our discussion on cfe-dev about the extensions and also 
> the earlier review you have created: https://reviews.llvm.org/D21698?


Since that feature takes time to implement, we decide to add our extensions to 
Clang directly first.


https://reviews.llvm.org/D23322



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

Re: [PATCH] D23387: [Analyzer] Report found fields order in PaddingChecker.

[Alexander Shaposhnikov via cfe-commits]

alexshap added a comment.

Thanks, your points regarding better formatting of the diagnostics / moving it 
into a note or fixit  and using more stable sort 
are valid, i will update this diff today or tomorrow. Regarding the type name - 
i am not sure - for example for template specializations
(for instance think about the standard containers) the full name of the type 
might be really long - not sure if it's really useful to print it here.


https://reviews.llvm.org/D23387



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

Re: r276900 - [Sema] Teach getCurrentThisType to reconize lambda in in-class initializer

[Richard Smith via cfe-commits]

Yes, this should be merged. It's not quite right, but it's certainly better
than what we had before.

On Mon, Aug 8, 2016 at 1:34 PM, Hans Wennborg  wrote:

> Richard: ping?
>
> On Wed, Jul 27, 2016 at 4:46 PM, Hans Wennborg  wrote:
> > Should this be merged to 3.9?
> >
> > Thanks,
> > Hans
> >
> > On Wed, Jul 27, 2016 at 11:25 AM, Erik Pilkington via cfe-commits
> >  wrote:
> >> Author: epilk
> >> Date: Wed Jul 27 13:25:10 2016
> >> New Revision: 276900
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=276900&view=rev
> >> Log:
> >> [Sema] Teach getCurrentThisType to reconize lambda in in-class
> initializer
> >>
> >> Fixes PR27994, a crash on valid.
> >>
> >> Differential revision: https://reviews.llvm.org/D21145
> >>
> >> Modified:
> >> cfe/trunk/lib/Sema/SemaExprCXX.cpp
> >> cfe/trunk/test/SemaCXX/lambda-expressions.cpp
> >>
> >> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
> >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaExprCXX.cpp?rev=276900&r1=276899&r2=276900&view=diff
> >> 
> ==
> >> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
> >> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Jul 27 13:25:10 2016
> >> @@ -961,32 +961,26 @@ static QualType adjustCVQualifiersForCXX
> >>  QualType Sema::getCurrentThisType() {
> >>DeclContext *DC = getFunctionLevelDeclContext();
> >>QualType ThisTy = CXXThisTypeOverride;
> >> +
> >>if (CXXMethodDecl *method = dyn_cast(DC)) {
> >>  if (method && method->isInstance())
> >>ThisTy = method->getThisType(Context);
> >>}
> >> -  if (ThisTy.isNull()) {
> >> -if (isGenericLambdaCallOperatorSpecialization(CurContext) &&
> >> -CurContext->getParent()->getParent()->isRecord()) {
> >> -  // This is a generic lambda call operator that is being
> instantiated
> >> -  // within a default initializer - so use the enclosing class as
> 'this'.
> >> -  // There is no enclosing member function to retrieve the 'this'
> pointer
> >> -  // from.
> >> -
> >> -  // FIXME: This looks wrong. If we're in a lambda within a lambda
> within a
> >> -  // default member initializer, we need to recurse up more
> parents to find
> >> -  // the right context. Looks like we should be walking up to the
> parent of
> >> -  // the closure type, checking whether that is itself a lambda,
> and if so,
> >> -  // recursing, until we reach a class or a function that isn't a
> lambda
> >> -  // call operator. And we should accumulate the constness of
> *this on the
> >> -  // way.
> >> -
> >> -  QualType ClassTy = Context.getTypeDeclType(
> >> -  cast(CurContext->getParent()->getParent()));
> >> -  // There are no cv-qualifiers for 'this' within default
> initializers,
> >> -  // per [expr.prim.general]p4.
> >> -  ThisTy = Context.getPointerType(ClassTy);
> >> -}
> >> +
> >> +  if (ThisTy.isNull() && isLambdaCallOperator(CurContext) &&
> >> +  !ActiveTemplateInstantiations.empty()) {
> >> +
> >> +assert(isa(DC) &&
> >> +   "Trying to get 'this' type from static method?");
> >> +
> >> +// This is a lambda call operator that is being instantiated as a
> default
> >> +// initializer. DC must point to the enclosing class type, so we
> can recover
> >> +// the 'this' type from it.
> >> +
> >> +QualType ClassTy = Context.getTypeDeclType(cast<
> CXXRecordDecl>(DC));
> >> +// There are no cv-qualifiers for 'this' within default
> initializers,
> >> +// per [expr.prim.general]p4.
>

This is wrong in C++17 onwards: if *this is captured by value by a
non-mutable lambda, it should become const-qualified.


> >> +ThisTy = Context.getPointerType(ClassTy);
> >>}
> >>
> >>// If we are within a lambda's call operator, the cv-qualifiers of
> 'this'
> >>
> >> Modified: cfe/trunk/test/SemaCXX/lambda-expressions.cpp
> >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> SemaCXX/lambda-expressions.cpp?rev=276900&r1=276899&r2=276900&view=diff
> >> 
> ==
> >> --- cfe/trunk/test/SemaCXX/lambda-expressions.cpp (original)
> >> +++ cfe/trunk/test/SemaCXX/lambda-expressions.cpp Wed Jul 27 13:25:10
> 2016
> >> @@ -1,5 +1,4 @@
> >> -// RUN: %clang_cc1 -std=c++11 -Wno-unused-value -fsyntax-only -verify
> -fblocks %s
> >> -// RUN: %clang_cc1 -std=c++1y -Wno-unused-value -fsyntax-only -verify
> -fblocks %s
> >> +// RUN: %clang_cc1 -std=c++14 -Wno-unused-value -fsyntax-only -verify
> -fblocks %s
> >>
> >>  namespace std { class type_info; };
> >>
> >> @@ -499,3 +498,30 @@ void foo() {
> >>};
> >>  }
> >>  }
> >> +
> >> +namespace PR27994 {
> >> +struct A { template  A(T); };
> >> +
> >> +template 
> >> +struct B {
> >> +  int x;
> >> +  A a = [&] { int y = x; };
> >> +  A b = [&] { [&] { [&] { int y = x; }; }; };
> >> +  A d = [&](auto param) { int y = x; };
> >> +  A e = [&](auto para

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

[Zachary Turner via cfe-commits]

zturner added a comment.

In https://reviews.llvm.org/D23409#512745, @zturner wrote:

> In https://reviews.llvm.org/D23409#512720, @aaron.ballman wrote:
>
> > Patch generally LGTM, though I wonder if there's a way we can add test 
> > coverage for this.
>
>
> I'd imagine we can just enable the clang-tidy test suite on Windows (I'm 
> assuming it's currently disabled).  I'll look into it.


Actually yea I'm not sure how to go about doing this.  All the existing tests 
are written to use GCC style command line, and it seems unreasonable to add a 
cl style command line variant of every single test.  maybe alexfh@ or someone 
has a suggestion.


https://reviews.llvm.org/D23409



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

Re: r277522 - [CFG] Fix crash finding destructor of lifetime-extended temporary.

[Richard Smith via cfe-commits]

Well, the whole approach here is still wrong (per the FIXME in
CFGBuilder::addLocalScopeForVarDecl) but this at least makes it consistent.
Let's take this for 3.9 to at least stem the bleeding.

On Tue, Aug 9, 2016 at 11:57 AM, Hans Wennborg  wrote:

> For the record, this was for PR28666.
>
> Richard: what do you think about merging this to 3.9?
>
> On Tue, Aug 2, 2016 at 2:07 PM, Devin Coughlin via cfe-commits
>  wrote:
> > Author: dcoughlin
> > Date: Tue Aug  2 16:07:23 2016
> > New Revision: 277522
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=277522&view=rev
> > Log:
> > [CFG] Fix crash finding destructor of lifetime-extended temporary.
> >
> > Fix a crash under -Wthread-safety when finding the destructor for a
> > lifetime-extending reference.
> >
> > A patch by Nandor Licker!
> >
> > Differential Revision: https://reviews.llvm.org/D22419
> >
> > Modified:
> > cfe/trunk/lib/Analysis/CFG.cpp
> > cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
> >
> > Modified: cfe/trunk/lib/Analysis/CFG.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/
> Analysis/CFG.cpp?rev=277522&r1=277521&r2=277522&view=diff
> > 
> ==
> > --- cfe/trunk/lib/Analysis/CFG.cpp (original)
> > +++ cfe/trunk/lib/Analysis/CFG.cpp Tue Aug  2 16:07:23 2016
> > @@ -3902,7 +3902,17 @@ CFGImplicitDtor::getDestructorDecl(ASTCo
> >  case CFGElement::AutomaticObjectDtor: {
> >const VarDecl *var = castAs().getVarDecl();
> >QualType ty = var->getType();
> > -  ty = ty.getNonReferenceType();
> > +
> > +  // FIXME: See CFGBuilder::addLocalScopeForVarDecl.
> > +  //
> > +  // Lifetime-extending constructs are handled here. This works for
> a single
> > +  // temporary in an initializer expression.
> > +  if (ty->isReferenceType()) {
> > +if (const Expr *Init = var->getInit()) {
> > +  ty = getReferenceInitTemporaryType(astContext, Init);
> > +}
> > +  }
> > +
> >while (const ArrayType *arrayType =
> astContext.getAsArrayType(ty)) {
> >  ty = arrayType->getElementType();
> >}
> >
> > Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> SemaCXX/warn-thread-safety-analysis.cpp?rev=277522&r1=
> 277521&r2=277522&view=diff
> > 
> ==
> > --- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
> > +++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Tue Aug  2
> 16:07:23 2016
> > @@ -5160,6 +5160,21 @@ void test3() {
> >  }  // end namespace  GlobalAcquiredBeforeAfterTest
> >
> >
> > +namespace LifetimeExtensionText {
> > +
> > +struct Holder {
> > +  virtual ~Holder() throw() {}
> > +  int i = 0;
> > +};
> > +
> > +void test() {
> > +  // Should not crash.
> > +  const auto &value = Holder().i;
> > +}
> > +
> > +} // end namespace LifetimeExtensionTest
> > +
> > +
> >  namespace LockableUnions {
> >
> >  union LOCKABLE MutexUnion {
> >
> >
> > ___
> > 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] D23322: [OpenCL] AMDGPU: Add extensions cl_amd_media_ops and cl_amd_media_ops2

[Anastasia Stulova via cfe-commits]

Anastasia added a comment.

Ok, sure. Is the plan to refactor this bit in case we implement the generic 
support later then?

It seems fine, although I can't check much without any documentation. Is there 
any reference available online?


https://reviews.llvm.org/D23322



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

Re: [PATCH] D23322: [OpenCL] AMDGPU: Add extensions cl_amd_media_ops and cl_amd_media_ops2

[Yaxun Liu via cfe-commits]

yaxunl added a comment.

In https://reviews.llvm.org/D23322#512823, @Anastasia wrote:

> Ok, sure. Is the plan to refactor this bit in case we implement the generic 
> support later then?
>
> It seems fine, although I can't check much without any documentation. Is 
> there any reference available online?


Yes we will refactor it after the generic support is implemented.

The documentations are here:
https://www.khronos.org/registry/cl/extensions/amd/cl_amd_media_ops.txt
https://www.khronos.org/registry/cl/extensions/amd/cl_amd_media_ops2.txt


https://reviews.llvm.org/D23322



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

Re: r276900 - [Sema] Teach getCurrentThisType to reconize lambda in in-class initializer

[Hans Wennborg via cfe-commits]

Merged in r278374.

Thanks,
Hans

On Thu, Aug 11, 2016 at 11:07 AM, Richard Smith  wrote:
> Yes, this should be merged. It's not quite right, but it's certainly better
> than what we had before.
>
> On Mon, Aug 8, 2016 at 1:34 PM, Hans Wennborg  wrote:
>>
>> Richard: ping?
>>
>> On Wed, Jul 27, 2016 at 4:46 PM, Hans Wennborg  wrote:
>> > Should this be merged to 3.9?
>> >
>> > Thanks,
>> > Hans
>> >
>> > On Wed, Jul 27, 2016 at 11:25 AM, Erik Pilkington via cfe-commits
>> >  wrote:
>> >> Author: epilk
>> >> Date: Wed Jul 27 13:25:10 2016
>> >> New Revision: 276900
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=276900&view=rev
>> >> Log:
>> >> [Sema] Teach getCurrentThisType to reconize lambda in in-class
>> >> initializer
>> >>
>> >> Fixes PR27994, a crash on valid.
>> >>
>> >> Differential revision: https://reviews.llvm.org/D21145
>> >>
>> >> Modified:
>> >> cfe/trunk/lib/Sema/SemaExprCXX.cpp
>> >> cfe/trunk/test/SemaCXX/lambda-expressions.cpp
>> >>
>> >> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
>> >> URL:
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=276900&r1=276899&r2=276900&view=diff
>> >>
>> >> ==
>> >> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
>> >> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Jul 27 13:25:10 2016
>> >> @@ -961,32 +961,26 @@ static QualType adjustCVQualifiersForCXX
>> >>  QualType Sema::getCurrentThisType() {
>> >>DeclContext *DC = getFunctionLevelDeclContext();
>> >>QualType ThisTy = CXXThisTypeOverride;
>> >> +
>> >>if (CXXMethodDecl *method = dyn_cast(DC)) {
>> >>  if (method && method->isInstance())
>> >>ThisTy = method->getThisType(Context);
>> >>}
>> >> -  if (ThisTy.isNull()) {
>> >> -if (isGenericLambdaCallOperatorSpecialization(CurContext) &&
>> >> -CurContext->getParent()->getParent()->isRecord()) {
>> >> -  // This is a generic lambda call operator that is being
>> >> instantiated
>> >> -  // within a default initializer - so use the enclosing class as
>> >> 'this'.
>> >> -  // There is no enclosing member function to retrieve the 'this'
>> >> pointer
>> >> -  // from.
>> >> -
>> >> -  // FIXME: This looks wrong. If we're in a lambda within a lambda
>> >> within a
>> >> -  // default member initializer, we need to recurse up more
>> >> parents to find
>> >> -  // the right context. Looks like we should be walking up to the
>> >> parent of
>> >> -  // the closure type, checking whether that is itself a lambda,
>> >> and if so,
>> >> -  // recursing, until we reach a class or a function that isn't a
>> >> lambda
>> >> -  // call operator. And we should accumulate the constness of
>> >> *this on the
>> >> -  // way.
>> >> -
>> >> -  QualType ClassTy = Context.getTypeDeclType(
>> >> -  cast(CurContext->getParent()->getParent()));
>> >> -  // There are no cv-qualifiers for 'this' within default
>> >> initializers,
>> >> -  // per [expr.prim.general]p4.
>> >> -  ThisTy = Context.getPointerType(ClassTy);
>> >> -}
>> >> +
>> >> +  if (ThisTy.isNull() && isLambdaCallOperator(CurContext) &&
>> >> +  !ActiveTemplateInstantiations.empty()) {
>> >> +
>> >> +assert(isa(DC) &&
>> >> +   "Trying to get 'this' type from static method?");
>> >> +
>> >> +// This is a lambda call operator that is being instantiated as a
>> >> default
>> >> +// initializer. DC must point to the enclosing class type, so we
>> >> can recover
>> >> +// the 'this' type from it.
>> >> +
>> >> +QualType ClassTy =
>> >> Context.getTypeDeclType(cast(DC));
>> >> +// There are no cv-qualifiers for 'this' within default
>> >> initializers,
>> >> +// per [expr.prim.general]p4.
>
>
> This is wrong in C++17 onwards: if *this is captured by value by a
> non-mutable lambda, it should become const-qualified.
>
>>
>> >> +ThisTy = Context.getPointerType(ClassTy);
>> >>}
>> >>
>> >>// If we are within a lambda's call operator, the cv-qualifiers of
>> >> 'this'
>> >>
>> >> Modified: cfe/trunk/test/SemaCXX/lambda-expressions.cpp
>> >> URL:
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/lambda-expressions.cpp?rev=276900&r1=276899&r2=276900&view=diff
>> >>
>> >> ==
>> >> --- cfe/trunk/test/SemaCXX/lambda-expressions.cpp (original)
>> >> +++ cfe/trunk/test/SemaCXX/lambda-expressions.cpp Wed Jul 27 13:25:10
>> >> 2016
>> >> @@ -1,5 +1,4 @@
>> >> -// RUN: %clang_cc1 -std=c++11 -Wno-unused-value -fsyntax-only -verify
>> >> -fblocks %s
>> >> -// RUN: %clang_cc1 -std=c++1y -Wno-unused-value -fsyntax-only -verify
>> >> -fblocks %s
>> >> +// RUN: %clang_cc1 -std=c++14 -Wno-unused-value -fsyntax-only -verify
>> >> -fblocks %s
>> >>
>> >>  namespace std { class type_info; };
>> >>
>> >> @@ -499,3 +498,30 @@ void foo() {
>> >>};
>> >>  }
>> >>  }
>> 

Re: [PATCH] D23361: [OpenCL] AMDGCN: Fix size_t type

[Yaxun Liu via cfe-commits]

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

In https://reviews.llvm.org/D23361#512729, @Anastasia wrote:

> I think Clang is supposed to generate the IR specific to the target 
> architecture. It seems strange to ignore the pointer size. I am not sure if 
> it might lead to some issues for the backends.


This change only affects AMDGCN target which has different pointer size for 
different address spaces. There is no change for other targets.

For AMDGCN target, since there can be only one definition for size_t type, it 
has to be 64 bit to accommodate all pointers.

For operations not involving pointers, this is fine since it does not matter 
whether size_t is i32 or i64.

For operations involving pointers, it only affects ptrtoint instruction, which 
allows a pointer to be casted to any integer. If the integer size is larger 
than the pointer size, it is zero extended. If the integer is smaller than the 
pointer size, it is truncated.

When clang casts a pointer to size_t, there are two cases:

1. non-private pointers: before this change, it was casted to i32, which is 
wrong. now it is casted to i64 correctly.

2. private pointer: before this change, it was casted to i32, now it is casted 
to i64. It is still correct. Backend should be able to optimize these 
instructions.


https://reviews.llvm.org/D23361



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

Re: [PATCH] D22515: [analyzer] Added custom hashing to the CloneDetector.

[Raphael Isemann via cfe-commits]

teemperor retitled this revision from "Added false-positive filter for 
unintended hash code collisions to the CloneDetector." to "[analyzer] Added 
custom hashing to the CloneDetector.".
teemperor updated the summary for this revision.
teemperor updated this revision to Diff 67715.
teemperor added a comment.

- Fixed some formatting problems.
- Updated Title/Summary


https://reviews.llvm.org/D22515

Files:
  include/clang/Analysis/CloneDetection.h
  lib/Analysis/CloneDetection.cpp
  lib/StaticAnalyzer/Checkers/CloneChecker.cpp

Index: lib/StaticAnalyzer/Checkers/CloneChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CloneChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CloneChecker.cpp
@@ -51,15 +51,6 @@
  "Related code clone is here.");
 
 for (CloneDetector::CloneGroup &Group : CloneGroups) {
-  // For readability reasons we sort the clones by line numbers.
-  std::sort(Group.Sequences.begin(), Group.Sequences.end(),
-[&SM](const StmtSequence &LHS, const StmtSequence &RHS) {
-  return SM.isBeforeInTranslationUnit(LHS.getStartLoc(),
-  RHS.getStartLoc()) &&
- SM.isBeforeInTranslationUnit(LHS.getEndLoc(),
-  RHS.getEndLoc());
-});
-
   // We group the clones by printing the first as a warning and all others
   // as a note.
   DiagEngine.Report(Group.Sequences.front().getStartLoc(), WarnID);
Index: lib/Analysis/CloneDetection.cpp
===
--- lib/Analysis/CloneDetection.cpp
+++ lib/Analysis/CloneDetection.cpp
@@ -243,46 +243,35 @@
 /// This class defines what a code clone is: If it collects for two statements
 /// the same data, then those two statements are considered to be clones of each
 /// other.
-class StmtDataCollector : public ConstStmtVisitor {
+///
+/// All collected data is forwarded to the given data consumer of the type T.
+/// The data consumer class needs to provide a member method with the signature:
+///   write(const char *Data, size_t Size)
+template 
+class StmtDataCollector : public ConstStmtVisitor> {
 
   ASTContext &Context;
-  std::vector &CollectedData;
+  /// \brief The data sink to which all data is forwarded.
+  T &DataConsumer;
 
 public:
   /// \brief Collects data of the given Stmt.
   /// \param S The given statement.
   /// \param Context The ASTContext of S.
-  /// \param D The given data vector to which all collected data is appended.
-  StmtDataCollector(const Stmt *S, ASTContext &Context,
-std::vector &D)
-  : Context(Context), CollectedData(D) {
-Visit(S);
+  /// \param D The data sink to which all data is forwarded.
+  StmtDataCollector(const Stmt *S, ASTContext &Context, T &DataConsumer)
+  : Context(Context), DataConsumer(DataConsumer) {
+this->Visit(S);
   }
 
   // Below are utility methods for appending different data to the vector.
 
   void addData(CloneDetector::DataPiece Integer) {
-CollectedData.push_back(Integer);
+DataConsumer.write(reinterpret_cast(&Integer), sizeof(Integer));
   }
 
-  // FIXME: The functions below add long strings to the data vector which are
-  // probably not good for performance. Replace the strings with pointer values
-  // or a some other unique integer.
-
   void addData(llvm::StringRef Str) {
-if (Str.empty())
-  return;
-
-const size_t OldSize = CollectedData.size();
-
-const size_t PieceSize = sizeof(CloneDetector::DataPiece);
-// Calculate how many vector units we need to accomodate all string bytes.
-size_t RoundedUpPieceNumber = (Str.size() + PieceSize - 1) / PieceSize;
-// Allocate space for the string in the data vector.
-CollectedData.resize(CollectedData.size() + RoundedUpPieceNumber);
-
-// Copy the string to the allocated space at the end of the vector.
-std::memcpy(CollectedData.data() + OldSize, Str.data(), Str.size());
+DataConsumer.write(Str.data(), Str.size());
   }
 
   void addData(const QualType &QT) { addData(QT.getAsString()); }
@@ -426,8 +415,10 @@
 // Create an empty signature that will be filled in this method.
 CloneDetector::CloneSignature Signature;
 
+llvm::hash_stream Hash;
+
 // Collect all relevant data from S and put it into the empty signature.
-StmtDataCollector(S, Context, Signature.Data);
+StmtDataCollector(S, Context, Hash);
 
 // If this code is generated by a macro, it won't increase the complexity
 // level of it's containing clone. This prevents false-positives caused by
@@ -457,7 +448,8 @@
   auto ChildSignature = generateSignatures(Child);
 
   // Add the collected data to the signature of the current statement.
-  Signature.add(ChildSignature);
+  Signature.Complexity += ChildSignature.Complex

Re: r277522 - [CFG] Fix crash finding destructor of lifetime-extended temporary.

[Hans Wennborg via cfe-commits]

Merged in r278376.

Thanks,
Hans

On Thu, Aug 11, 2016 at 11:13 AM, Richard Smith  wrote:
> Well, the whole approach here is still wrong (per the FIXME in
> CFGBuilder::addLocalScopeForVarDecl) but this at least makes it consistent.
> Let's take this for 3.9 to at least stem the bleeding.
>
>
> On Tue, Aug 9, 2016 at 11:57 AM, Hans Wennborg  wrote:
>>
>> For the record, this was for PR28666.
>>
>> Richard: what do you think about merging this to 3.9?
>>
>> On Tue, Aug 2, 2016 at 2:07 PM, Devin Coughlin via cfe-commits
>>  wrote:
>> > Author: dcoughlin
>> > Date: Tue Aug  2 16:07:23 2016
>> > New Revision: 277522
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=277522&view=rev
>> > Log:
>> > [CFG] Fix crash finding destructor of lifetime-extended temporary.
>> >
>> > Fix a crash under -Wthread-safety when finding the destructor for a
>> > lifetime-extending reference.
>> >
>> > A patch by Nandor Licker!
>> >
>> > Differential Revision: https://reviews.llvm.org/D22419
>> >
>> > Modified:
>> > cfe/trunk/lib/Analysis/CFG.cpp
>> > cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
>> >
>> > Modified: cfe/trunk/lib/Analysis/CFG.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=277522&r1=277521&r2=277522&view=diff
>> >
>> > ==
>> > --- cfe/trunk/lib/Analysis/CFG.cpp (original)
>> > +++ cfe/trunk/lib/Analysis/CFG.cpp Tue Aug  2 16:07:23 2016
>> > @@ -3902,7 +3902,17 @@ CFGImplicitDtor::getDestructorDecl(ASTCo
>> >  case CFGElement::AutomaticObjectDtor: {
>> >const VarDecl *var = castAs().getVarDecl();
>> >QualType ty = var->getType();
>> > -  ty = ty.getNonReferenceType();
>> > +
>> > +  // FIXME: See CFGBuilder::addLocalScopeForVarDecl.
>> > +  //
>> > +  // Lifetime-extending constructs are handled here. This works for
>> > a single
>> > +  // temporary in an initializer expression.
>> > +  if (ty->isReferenceType()) {
>> > +if (const Expr *Init = var->getInit()) {
>> > +  ty = getReferenceInitTemporaryType(astContext, Init);
>> > +}
>> > +  }
>> > +
>> >while (const ArrayType *arrayType =
>> > astContext.getAsArrayType(ty)) {
>> >  ty = arrayType->getElementType();
>> >}
>> >
>> > Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=277522&r1=277521&r2=277522&view=diff
>> >
>> > ==
>> > --- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
>> > +++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Tue Aug  2
>> > 16:07:23 2016
>> > @@ -5160,6 +5160,21 @@ void test3() {
>> >  }  // end namespace  GlobalAcquiredBeforeAfterTest
>> >
>> >
>> > +namespace LifetimeExtensionText {
>> > +
>> > +struct Holder {
>> > +  virtual ~Holder() throw() {}
>> > +  int i = 0;
>> > +};
>> > +
>> > +void test() {
>> > +  // Should not crash.
>> > +  const auto &value = Holder().i;
>> > +}
>> > +
>> > +} // end namespace LifetimeExtensionTest
>> > +
>> > +
>> >  namespace LockableUnions {
>> >
>> >  union LOCKABLE MutexUnion {
>> >
>> >
>> > ___
>> > 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

r278379 - [Sema] Add more strict check for sizeof diagnostics for bzero

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Thu Aug 11 13:33:15 2016
New Revision: 278379

URL: http://llvm.org/viewvc/llvm-project?rev=278379&view=rev
Log:
[Sema] Add more strict check for sizeof diagnostics for bzero

Follow-up from r278264 after Joerg's feedback.

Since bzero is not standard, be more strict: also check if the first
argument is a pointer, which harden the check for when it does not come
originally from a builtin.

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

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=278379&r1=278378&r2=278379&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Aug 11 13:33:15 2016
@@ -6199,6 +6199,13 @@ void Sema::CheckMemaccessArguments(const
   const Expr *SizeOfArg = getSizeOfExprArg(LenExpr);
   llvm::FoldingSetNodeID SizeOfArgID;
 
+  // Although widely used, 'bzero' is not a standard function. Be more strict
+  // with the argument types before allowing diagnostics and only allow the
+  // form bzero(ptr, sizeof(...)).
+  QualType FirstArgTy = Call->getArg(0)->IgnoreParenImpCasts()->getType();
+  if (BId == Builtin::BIbzero && !FirstArgTy->getAs())
+return;
+
   for (unsigned ArgIdx = 0; ArgIdx != LastArg; ++ArgIdx) {
 const Expr *Dest = Call->getArg(ArgIdx)->IgnoreParenImpCasts();
 SourceRange ArgRange = Call->getArg(ArgIdx)->getSourceRange();


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

Re: [PATCH] D23125: Modules: add command line option to support loading prebuilt modules on demand, without parsing any module map

[Manman Ren via cfe-commits]

manmanren added inline comments.


Comment at: lib/Frontend/CompilerInstance.cpp:1438-1450
@@ -1437,3 +1437,15 @@
 // Search for a module with the given name.
 Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
-if (!Module) {
+HeaderSearchOptions &HSOpts =
+PP->getHeaderSearchInfo().getHeaderSearchOpts();
+
+std::string ModuleFileName;
+bool LoadFromPrebuiltModulePath = false;
+if (Module) {
+  ModuleFileName = PP->getHeaderSearchInfo().getModuleFileName(Module);
+} else if (!HSOpts.PrebuiltModulePath.empty()) {
+  // Try to load the module from the prebuilt module path.
+  ModuleFileName =
+  PP->getHeaderSearchInfo().getModuleFileName(ModuleName, "", true);
+  LoadFromPrebuiltModulePath = true;
+} else {

bruno wrote:
> manmanren wrote:
> > rsmith wrote:
> > > It looks like the logic here is:
> > > 
> > >  * if we found the module in a module map, then look for it in the module 
> > > cache
> > >  * otherwise, if we have a prebuilt module path, then look for it there
> > >  * otherwise, error
> > > 
> > > I think that may be surprising; if I explicitly load a module map, 
> > > perhaps via `-fmodule-map-file=`, and supply a prebuilt form of that 
> > > module in the prebuilt modules path, I'd expect my prebuilt form to still 
> > > be used.
> > > 
> > > Perhaps instead we could first search the prebuilt module path for an 
> > > existing .pcm file, and if that fails and we have a `Module` then look 
> > > for it in the module cache?
> > I was wondering about the priority of loading from prebuilt module path vs. 
> > loading from module cache. Your suggestion makes sense.
> +1 to Richard's suggestion here.
> 
> I wonder about the workflow on generating such modules. Suppose I want to 
> build some modules to re-use later with -fprebuilt-module-path=. RIght now, 
> we get a module cache with hash in the names unless we disable hash 
> computation via cc1 option -fdisable-module-hash. Should we assume that the 
> user will have to copy out the cache dir and rename the modules OR that it 
> will use -fdisable-module-hash instead? Should this option also try to read 
> out modules with a hash in the name? Should we promote -fdisable-module-hash 
> to a driver option as well?
> 
> @Manman, do you have any workflow in mind? @Richard, do you have any feedback 
> in this respect?
> 
> In the crash reproducer we often have to rebuild the modules when running the 
> script in a different machine because of the paths used to hash the modules, 
> it would be nice (depending on the resulting design) if the crash reproducer 
> could use -fprebuilt-module-path in order to try to re-use the pre-built 
> modules resulting from the crash. 
If the modules are built implicitly with module hash, to load the module files 
as prebuilt modules, the user needs to copy out the cache dir and rename the 
modules. The user can choose to emit the modules explicitly or disable module 
hash when building the modules.


Comment at: lib/Frontend/CompilerInstance.cpp:1497-1505
@@ +1496,11 @@
+  if (LoadFromPrebuiltModulePath && !Module) {
+// Create a Module if we are using the prebuilt module path.
+Module = PP->getHeaderSearchInfo().getModuleMap().findOrCreateModule(
+ModuleName, nullptr, false/*IsFramework*/,
+false/*IsExplicit*/).first;
+// FIXME: Since we are mostly prebuilding system modules, we set
+// IsSystem to true here. This is not always the correct choice,
+// and IsSystem can affect diagnostics.
+Module->IsSystem = true;
+Module->IsFromModuleFile = true;
+  }

manmanren wrote:
> rsmith wrote:
> > The module file will have provided a `Module` instance, if it is in fact a 
> > module file for the requested module. Instead of inventing a `Module` here, 
> > can you check that one exists and produce an error if not?
> This is nice to know. I didn't realize we construct a Module when calling 
> ReadAST. Can you be more specific on how to check if a Module exists?
I assumed ReadAST only constructed Module instances for submodules, because it 
is inside ReadSubmoduleBlock. It turns out it also constructed Module instance 
for the top level module. I will update the patch.


https://reviews.llvm.org/D23125



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

Re: r278264 - Reapply [Sema] Add sizeof diagnostics for bzero

[Bruno Cardoso Lopes via cfe-commits]

On Thu, Aug 11, 2016 at 5:43 AM, Joerg Sonnenberger via cfe-commits
 wrote:
>
> On Wed, Aug 10, 2016 at 02:36:01PM -0700, Bruno Cardoso Lopes wrote:
> > Hi Joerg,
> >
> > > Given that bzero is pretty much non-standard at this point, I strongly
> > > dislike the approach of checking only the function name.
> >
> > Thanks for the feedback. bzero it's widely used though, I think it's a
> > usability win to perform a diagnostic here, despite bzero being
> > non-standard.
>
> Oh, I don't disagree that it is useful. Some projects still have a
> ridiculous amount of non-standard bzero and bcmp invocations.
>
> >
> > Is there any additional reason why you dislike it or additional checks
> > you suggest? IIUC, Sema::CheckMemaccessArguments already checks if the
> > number of arguments is the same and if they are indeed what they're
> > supposed to be.
>
> It checks the number of arguments, but that's about it. It doesn't for
> example check that the first argument is a pointer.

Right, it does not specifically check it's the first. Fixed in r278379.

-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r278382 - [analyzer] Teach RetainCountChecker about CVFooRetain

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Thu Aug 11 13:41:29 2016
New Revision: 278382

URL: http://llvm.org/viewvc/llvm-project?rev=278382&view=rev
Log:
[analyzer] Teach RetainCountChecker about CVFooRetain

Change the retain count checker to treat CoreFoundation-style "CV"-prefixed
reference types from CoreVideo similarly to CoreGraphics types. With this
change, we treat CVFooRetain() on a CVFooRef type as a retain. CVFooRelease()
APIs are annotated as consuming their parameter, so this change prevents false
positives about incorrect decrements of reference counts.



Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
cfe/trunk/test/Analysis/retain-release.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=278382&r1=278381&r2=278382&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Thu Aug 11 
13:41:29 2016
@@ -1171,8 +1171,9 @@ RetainSummaryManager::getFunctionSummary
 break;
   }
 
-  // For CoreGraphics ('CG') types.
-  if (cocoa::isRefType(RetTy, "CG", FName)) {
+  // For CoreGraphics ('CG') and CoreVideo ('CV') types.
+  if (cocoa::isRefType(RetTy, "CG", FName) ||
+  cocoa::isRefType(RetTy, "CV", FName)) {
 if (isRetain(FD, FName))
   S = getUnarySummary(FT, cfretain);
 else
@@ -3372,12 +3373,13 @@ bool RetainCountChecker::evalCall(const
 // Handle: id NSMakeCollectable(CFTypeRef)
 canEval = II->isStr("NSMakeCollectable");
   } else if (ResultTy->isPointerType()) {
-// Handle: (CF|CG)Retain
+// Handle: (CF|CG|CV)Retain
 // CFAutorelease
 // CFMakeCollectable
 // It's okay to be a little sloppy here (CGMakeCollectable doesn't exist).
 if (cocoa::isRefType(ResultTy, "CF", FName) ||
-cocoa::isRefType(ResultTy, "CG", FName)) {
+cocoa::isRefType(ResultTy, "CG", FName) ||
+cocoa::isRefType(ResultTy, "CV", FName)) {
   canEval = isRetain(FD, FName) || isAutorelease(FD, FName) ||
 isMakeCollectable(FD, FName);
 }

Modified: cfe/trunk/test/Analysis/retain-release.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release.m?rev=278382&r1=278381&r2=278382&view=diff
==
--- cfe/trunk/test/Analysis/retain-release.m (original)
+++ cfe/trunk/test/Analysis/retain-release.m Thu Aug 11 13:41:29 2016
@@ -1247,6 +1247,26 @@ CVReturn rdar_7283567_2(CFAllocatorRef a
   pixelBufferAttributes, pixelBufferOut) ;
 }
 
+#pragma clang arc_cf_code_audited begin
+typedef struct SomeOpaqueStruct *CMSampleBufferRef;
+CVImageBufferRef _Nonnull CMSampleBufferGetImageBuffer(CMSampleBufferRef 
_Nonnull sbuf);
+#pragma clang arc_cf_code_audited end
+
+CVBufferRef _Nullable CVBufferRetain(CVBufferRef _Nullable buffer);
+void CVBufferRelease(CF_CONSUMED CVBufferRef _Nullable buffer);
+
+void testCVPrefixRetain(CMSampleBufferRef sbuf) {
+  // Make sure RetainCountChecker treats CVFooRetain() as a CF-style retain.
+  CVPixelBufferRef pixelBuf = CMSampleBufferGetImageBuffer(sbuf);
+  CVBufferRetain(pixelBuf);
+  CVBufferRelease(pixelBuf); // no-warning
+
+
+  // Make sure result of CVFooRetain() is the same as its argument.
+  CVPixelBufferRef pixelBufAlias = CVBufferRetain(pixelBuf);
+  CVBufferRelease(pixelBufAlias); // no-warning
+}
+
 
//===--===//
 //  False leak associated with 
 //  CGBitmapContextCreateWithData


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

Re: [PATCH] D23003: [ObjC Availability] Warn upon unguarded use of partially available declaration

[Erik Pilkington via cfe-commits]

erik.pilkington updated this revision to Diff 67713.
erik.pilkington added a comment.

This new patch adds some testcases for templates, and simplifies some control 
flow in `TraverseIfStmt`. This patch also removes diagnostics for instantiated 
templates, instead just using the pattern. This means that a dependent type 
that was resolved during instantiation can refer to an unavailable declaration. 
For example, the following now compiles cleanly (when we want a diagnostic):

  void f(int);
  void f(char) __attribute__((availability(macos, introduced=10.12)));
  
  template  void use_f(T p) { f(p); }
  
  template void use_f();

This is done so containers, such as `vector` can be used 
safely provided `partially_available` is safe at the point of instantiation. I 
think the way to improve this is in `Sema::getVersionForDecl()`. There we could 
look at function and template parameters to determine the best base version, 
then diagnose against it.


https://reviews.llvm.org/D23003

Files:
  include/clang/AST/Stmt.h
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/ScopeInfo.h
  include/clang/Sema/Sema.h
  lib/AST/Stmt.cpp
  lib/Sema/JumpDiagnostics.cpp
  lib/Sema/ScopeInfo.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaStmt.cpp
  test/Parser/objc-available.m
  test/Sema/attr-availability.c
  test/SemaObjC/attr-availability.m
  test/SemaObjC/property-deprecated-warning.m
  test/SemaObjC/unguarded-availability.m

Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -0,0 +1,180 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx-10.9 -Wunguarded-availability -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -xobjective-c++ -DOBJCPP -triple x86_64-apple-macosx-10.9 -Wunguarded-availability -fblocks -fsyntax-only -verify %s
+
+#define AVAILABLE_10_0  __attribute__((availability(macos, introduced = 10.0)))
+#define AVAILABLE_10_11 __attribute__((availability(macos, introduced = 10.11)))
+#define AVAILABLE_10_12 __attribute__((availability(macos, introduced = 10.12)))
+
+int func_10_11() AVAILABLE_10_11; // expected-note 4 {{'func_10_11' has been explicitly marked partial here}}
+
+#ifdef OBJCPP
+// expected-note@+2 {{marked partial here}}
+#endif
+int func_10_12() AVAILABLE_10_12; // expected-note 5 {{'func_10_12' has been explicitly marked partial here}}
+
+int func_10_0() AVAILABLE_10_0;
+
+void use_func() {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+
+  if (@available(macos 10.11, *))
+func_10_11();
+  else
+func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
+
+void defn_10_11() AVAILABLE_10_11;
+
+void defn_10_11() {
+  func_10_11();
+}
+
+void nested_ifs() {
+  if (@available(macos 10.12, *)) {
+if (@available(macos 10.10, *)) {
+  func_10_12();
+} else {
+  func_10_12();
+}
+  } else {
+func_10_12(); // expected-warning{{'func_10_12' is only available on macOS 10.12 or newer}} expected-note{{enclose 'func_10_12' in an @available check to silence this warning}}
+  }
+}
+
+void star_case() {
+  if (@available(ios 9, *)) {
+func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+func_10_0();
+  } else
+func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+
+  if (@available(macos 10.11, *)) {
+if (@available(ios 8, *)) {
+  func_10_11();
+  func_10_12(); // expected-warning{{'func_10_12' is only available on macOS 10.12 or newer}} expected-note{{enclose}}
+} else {
+  func_10_11();
+  func_10_12(); // expected-warning{{'func_10_12' is only available on macOS 10.12 or newer}} expected-note{{enclose}}
+}
+  }
+}
+
+typedef int int_10_11 AVAILABLE_10_11; // expected-note {{'int_10_11' has been explicitly marked partial here}}
+#ifdef OBJCPP
+// expected-note@+2 {{marked partial here}}
+#endif
+typedef int int_10_12 AVAILABLE_10_12; // expected-note 3 {{'int_10_12' has been explicitly marked partial here}}
+
+void use_typedef() {
+  int_10_11 x; // expected-warning{{'int_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'int_10_11' in an @available check to silence this warning}}
+}
+
+__attribute__((objc_root_class))
+AVAILABLE_10_11 @interface Class_10_11 {
+  int_10_11 foo;
+  int_10_12 bar; // expected-warning {{'int_10_12' is partial: introduced in macOS 10.

[libcxx] r278387 - Remove test for the sign of a NaN - doesn't work on MIPS, not strictly legal. Fixes bug 28936

[Marshall Clow via cfe-commits]

Author: marshall
Date: Thu Aug 11 13:46:24 2016
New Revision: 278387

URL: http://llvm.org/viewvc/llvm-project?rev=278387&view=rev
Log:
Remove test for the sign of a NaN - doesn't work on MIPS, not strictly legal. 
Fixes bug 28936

Modified:

libcxx/trunk/test/std/numerics/complex.number/complex.transcendentals/asin.pass.cpp

Modified: 
libcxx/trunk/test/std/numerics/complex.number/complex.transcendentals/asin.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/numerics/complex.number/complex.transcendentals/asin.pass.cpp?rev=278387&r1=278386&r2=278387&view=diff
==
--- 
libcxx/trunk/test/std/numerics/complex.number/complex.transcendentals/asin.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/numerics/complex.number/complex.transcendentals/asin.pass.cpp
 Thu Aug 11 13:46:24 2016
@@ -91,7 +91,6 @@ void test_edges()
 {
 assert(std::isnan(r.real()));
 assert(std::isnan(r.imag()));
-assert(std::signbit(testcases[i].imag()) == 
std::signbit(r.imag()));
 }
 else if (std::isnan(testcases[i].real()) && 
std::isinf(testcases[i].imag()))
 {


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

Re: [libcxx] r278191 - test/hard_link_count(): Fix test on darwin

[Adrian Prantl via cfe-commits]

I'm just curious: Is libcxx generally supposed to merely pass through what the 
operating system is returning (as in this case) or is is supposed to provide a 
common abstraction that behaves the same on all platforms?

-- adrian

> On Aug 10, 2016, at 8:23 PM, Eric Fiselier via cfe-commits 
>  wrote:
> 
> Thanks Matthias. My apoligies, I lost track of this patch.
> 
> On Tue, Aug 9, 2016 at 7:31 PM, Bruno Cardoso Lopes via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Thanks Matthias!
> 
> On Tue, Aug 9, 2016 at 6:02 PM, Matthias Braun via cfe-commits
> mailto:cfe-commits@lists.llvm.org>> wrote:
> > Author: matze
> > Date: Tue Aug  9 20:02:28 2016
> > New Revision: 278191
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=278191&view=rev 
> > 
> > Log:
> > test/hard_link_count(): Fix test on darwin
> >
> > The hard link count that stat reports are different between normal hfs and 
> > the
> > case sensitive variant. Accept both.
> >
> > Modified:
> > 
> > libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.hard_lk_ct/hard_link_count.pass.cpp
> >
> > Modified: 
> > libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.hard_lk_ct/hard_link_count.pass.cpp
> > URL: 
> > http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.hard_lk_ct/hard_link_count.pass.cpp?rev=278191&r1=278190&r2=278191&view=diff
> >  
> > 
> > ==
> > --- 
> > libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.hard_lk_ct/hard_link_count.pass.cpp
> >  (original)
> > +++ 
> > libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.hard_lk_ct/hard_link_count.pass.cpp
> >  Tue Aug  9 20:02:28 2016
> > @@ -45,18 +45,27 @@ TEST_CASE(hard_link_count_for_file)
> >
> >  TEST_CASE(hard_link_count_for_directory)
> >  {
> > -uintmax_t DirExpect = 3;
> > -uintmax_t Dir3Expect = 2;
> > +uintmax_t DirExpect = 3; // hard link from . .. and Dir2
> > +uintmax_t Dir3Expect = 2; // hard link from . ..
> > +uintmax_t DirExpectAlt = DirExpect;
> > +uintmax_t Dir3ExpectAlt = Dir3Expect;
> >  #if defined(__APPLE__)
> > -DirExpect += 2;
> > -Dir3Expect += 1;
> > +// Filesystems formatted with case sensitive hfs+ behave unixish as
> > +// expected. Normal hfs+ filesystems report the number of directory
> > +// entries instead.
> > +DirExpectAlt = 5; // .  ..  Dir2  file1  file2
> > +Dir3Expect = 3; // .  ..  file5
> >  #endif
> > -TEST_CHECK(hard_link_count(StaticEnv::Dir) == DirExpect);
> > -TEST_CHECK(hard_link_count(StaticEnv::Dir3) == Dir3Expect);
> > +TEST_CHECK(hard_link_count(StaticEnv::Dir) == DirExpect ||
> > +   hard_link_count(StaticEnv::Dir) == DirExpectAlt);
> > +TEST_CHECK(hard_link_count(StaticEnv::Dir3) == Dir3Expect ||
> > +   hard_link_count(StaticEnv::Dir3) == Dir3ExpectAlt);
> >
> >  std::error_code ec;
> > -TEST_CHECK(hard_link_count(StaticEnv::Dir, ec) == DirExpect);
> > -TEST_CHECK(hard_link_count(StaticEnv::Dir3, ec) == Dir3Expect);
> > +TEST_CHECK(hard_link_count(StaticEnv::Dir, ec) == DirExpect ||
> > +   hard_link_count(StaticEnv::Dir, ec) == DirExpectAlt);
> > +TEST_CHECK(hard_link_count(StaticEnv::Dir3, ec) == Dir3Expect ||
> > +   hard_link_count(StaticEnv::Dir3, ec) == Dir3ExpectAlt);
> >  }
> >  TEST_CASE(hard_link_count_increments_test)
> >  {
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org 
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits 
> > 
> 
> 
> 
> --
> Bruno Cardoso Lopes
> http://www.brunocardoso.cc 
> ___
> 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

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

Re: [PATCH] D23125: Modules: add command line option to support loading prebuilt modules on demand, without parsing any module map

[Manman Ren via cfe-commits]

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

Addressing Richard's comments.


https://reviews.llvm.org/D23125

Files:
  docs/Modules.rst
  include/clang/Driver/Options.td
  include/clang/Lex/HeaderSearch.h
  include/clang/Lex/HeaderSearchOptions.h
  include/clang/Serialization/Module.h
  lib/Driver/Tools.cpp
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/CompilerInstance.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  lib/Lex/HeaderSearch.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ModuleManager.cpp
  test/Driver/modules.m
  test/Modules/Inputs/prebuilt-module/a.h
  test/Modules/Inputs/prebuilt-module/module.modulemap
  test/Modules/prebuilt-module.m

Index: test/Modules/prebuilt-module.m
===
--- test/Modules/prebuilt-module.m
+++ test/Modules/prebuilt-module.m
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+//
+// RUN: %clang_cc1 -fmodules -x objective-c -I %S/Inputs/prebuilt-module -triple %itanium_abi_triple -emit-module %S/Inputs/prebuilt-module/module.modulemap -fmodule-name=prebuilt -o %t/prebuilt.pcm
+// RUN: %clang_cc1 -fmodules -fprebuilt-module-path=%t/ -fdisable-module-hash %s -verify
+
+// expected-no-diagnostics
+@import prebuilt;
+int test() {
+  return a;
+}
Index: test/Modules/Inputs/prebuilt-module/module.modulemap
===
--- test/Modules/Inputs/prebuilt-module/module.modulemap
+++ test/Modules/Inputs/prebuilt-module/module.modulemap
@@ -0,0 +1 @@
+module prebuilt { header "a.h" }
Index: test/Modules/Inputs/prebuilt-module/a.h
===
--- test/Modules/Inputs/prebuilt-module/a.h
+++ test/Modules/Inputs/prebuilt-module/a.h
@@ -0,0 +1 @@
+const int a = 1;
Index: test/Driver/modules.m
===
--- test/Driver/modules.m
+++ test/Driver/modules.m
@@ -39,6 +39,13 @@
 // RUN: %clang -fmodules-disable-diagnostic-validation -### %s 2>&1 | FileCheck -check-prefix=MODULES_DISABLE_DIAGNOSTIC_VALIDATION %s
 // MODULES_DISABLE_DIAGNOSTIC_VALIDATION: -fmodules-disable-diagnostic-validation
 
+// RUN: %clang -fmodules -### %s 2>&1 | FileCheck -check-prefix=MODULES_PREBUILT_PATH_DEFAULT %s
+// MODULES_PREBUILT_PATH_DEFAULT-NOT: -fprebuilt-module-path
+
+// RUN: %clang -fmodules -fprebuilt-module-path=foo -fprebuilt-module-path=bar -### %s 2>&1 | FileCheck -check-prefix=MODULES_PREBUILT_PATH %s
+// MODULES_PREBUILT_PATH: "-fprebuilt-module-path=foo"
+// MODULES_PREBUILT_PATH: "-fprebuilt-module-path=bar"
+
 // RUN: %clang -fmodules -fmodule-map-file=foo.map -fmodule-map-file=bar.map -### %s 2>&1 | FileCheck -check-prefix=CHECK-MODULE-MAP-FILES %s
 // CHECK-MODULE-MAP-FILES: "-fmodules"
 // CHECK-MODULE-MAP-FILES: "-fmodule-map-file=foo.map"
Index: lib/Serialization/ModuleManager.cpp
===
--- lib/Serialization/ModuleManager.cpp
+++ lib/Serialization/ModuleManager.cpp
@@ -66,7 +66,7 @@
   // Look for the file entry. This only fails if the expected size or
   // modification time differ.
   const FileEntry *Entry;
-  if (Type == MK_ExplicitModule) {
+  if (Type == MK_ExplicitModule || Type == MK_PrebuiltModule) {
 // If we're not expecting to pull this file out of the module cache, it
 // might have a different mtime due to being moved across filesystems in
 // a distributed build. The size must still match, though. (As must the
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -1384,7 +1384,7 @@
 // any other module's anonymous namespaces, so don't attach the anonymous
 // namespace at all.
 NamespaceDecl *Anon = cast(Reader.GetDecl(AnonNamespace));
-if (F.Kind != MK_ImplicitModule && F.Kind != MK_ExplicitModule)
+if (!F.isModule())
   D->setAnonymousNamespace(Anon);
   }
 }
@@ -3747,8 +3747,7 @@
   // Each module has its own anonymous namespace, which is disjoint from
   // any other module's anonymous namespaces, so don't attach the anonymous
   // namespace at all.
-  if (ModuleFile.Kind != MK_ImplicitModule &&
-  ModuleFile.Kind != MK_ExplicitModule) {
+  if (!ModuleFile.isModule()) {
 if (TranslationUnitDecl *TU = dyn_cast(D))
   TU->setAnonymousNamespace(Anon);
 else
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -1312,8 +1312,7 @@
 SrcMgr::CharacteristicKind
   FileCharacter = (SrcMgr::CharacteristicKind)Record[2];
 SourceLocation IncludeLoc = ReadSourceLocation(*F, Record[1]);
-if (IncludeLoc.isInvalid() &&
-(F->Kind == MK

r278393 - Pass in frame pointer omitting compiler flags for CloudABI as well.

[Ed Schouten via cfe-commits]

Author: ed
Date: Thu Aug 11 14:23:30 2016
New Revision: 278393

URL: http://llvm.org/viewvc/llvm-project?rev=278393&view=rev
Log:
Pass in frame pointer omitting compiler flags for CloudABI as well.

On Linux we pass in -fomit-frame-pointer flags (and similar)
automatically if optimization is enabled. Let's do the same thing on
CloudABI. Without this, Clang seems to run out of registers quite
quickly while trying to build code with inline assembly.


Modified:
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/test/Driver/frame-pointer-elim.c

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=278393&r1=278392&r2=278393&view=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Thu Aug 11 14:23:30 2016
@@ -3242,7 +3242,7 @@ static bool shouldUseFramePointerForTarg
 break;
   }
 
-  if (Triple.isOSLinux()) {
+  if (Triple.isOSLinux() || Triple.getOS() == llvm::Triple::CloudABI) {
 switch (Triple.getArch()) {
 // Don't use a frame pointer on linux if optimizing for certain targets.
 case llvm::Triple::mips64:

Modified: cfe/trunk/test/Driver/frame-pointer-elim.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/frame-pointer-elim.c?rev=278393&r1=278392&r2=278393&view=diff
==
--- cfe/trunk/test/Driver/frame-pointer-elim.c (original)
+++ cfe/trunk/test/Driver/frame-pointer-elim.c Thu Aug 11 14:23:30 2016
@@ -8,6 +8,15 @@
 // RUN:   FileCheck --check-prefix=LINUX %s
 // LINUX-NOT: "-momit-leaf-frame-pointer"
 
+// CloudABI follows the same rules as Linux.
+// RUN: %clang -### -target x86_64-unknown-cloudabi -S -O1 %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=CLOUDABI-OPT %s
+// CLOUDABI-OPT: "-momit-leaf-frame-pointer"
+
+// RUN: %clang -### -target x86_64-unknown-cloudabi -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=CLOUDABI %s
+// CLOUDABI-NOT: "-momit-leaf-frame-pointer"
+
 // Darwin disables omitting the leaf frame pointer even under optimization
 // unless the command lines are given.
 // RUN: %clang -### -target i386-apple-darwin -S %s 2>&1 | \


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

[PATCH] D23418: [analyzer] Added a pass architecture to the CloneDetector

[Raphael Isemann via cfe-commits]

teemperor created this revision.
teemperor added reviewers: v.g.vassilev, NoQ, zaks.anna.
teemperor added a subscriber: cfe-commits.

https://reviews.llvm.org/D23418

Files:
  include/clang/Analysis/CloneDetection.h
  lib/Analysis/CloneDetection.cpp
  lib/StaticAnalyzer/Checkers/CloneChecker.cpp

Index: lib/StaticAnalyzer/Checkers/CloneChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CloneChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CloneChecker.cpp
@@ -36,11 +36,9 @@
  AnalysisManager &Mgr, BugReporter &BR) const;
 
   /// \brief Reports all clones to the user.
-  void reportClones(SourceManager &SM, AnalysisManager &Mgr,
-int MinComplexity) const {
-
-std::vector CloneGroups;
-CloneDetector.findClones(CloneGroups, MinComplexity);
+  void
+  reportClones(SourceManager &SM, AnalysisManager &Mgr,
+   const std::vector &Clones) const {
 
 DiagnosticsEngine &DiagEngine = Mgr.getDiagnostic();
 
@@ -50,7 +48,7 @@
 unsigned NoteID = DiagEngine.getCustomDiagID(DiagnosticsEngine::Note,
  "Related code clone is here.");
 
-for (CloneDetector::CloneGroup &Group : CloneGroups) {
+for (const CloneDetector::CloneGroup &Group : Clones) {
   // We group the clones by printing the first as a warning and all others
   // as a note.
   DiagEngine.Report(Group.Sequences.front().getStartLoc(), WarnID);
@@ -62,11 +60,37 @@
 
   /// \brief Reports only suspicious clones to the user along with informaton
   ///that explain why they are suspicious.
-  void reportSuspiciousClones(SourceManager &SM, AnalysisManager &Mgr,
-  int MinComplexity) const {
-
-std::vector Clones;
-CloneDetector.findSuspiciousClones(Clones, MinComplexity);
+  void reportSuspiciousClones(
+  SourceManager &SM, AnalysisManager &Mgr,
+  const std::vector &Clones) const {
+
+std::vector Pairs;
+
+for (const CloneDetector::CloneGroup &Group : Clones) {
+  for (unsigned i = 0; i < Group.Sequences.size(); ++i) {
+VariablePattern PatternA(Group.Sequences[i]);
+
+for (unsigned j = i + 1; j < Group.Sequences.size(); ++j) {
+  VariablePattern PatternB(Group.Sequences[j]);
+
+  VariablePattern::SuspiciousClonePair ClonePair;
+  // For now, we only report clones which break the variable pattern
+  // just
+  // once because multiple differences in a pattern are an indicator
+  // that
+  // those differences are maybe intended (e.g. because it's actually
+  // a different algorithm).
+  // TODO: In very big clones even multiple variables can be unintended,
+  // so replacing this number with a percentage could better handle such
+  // cases. On the other hand it could increase the false-positive rate
+  // for all clones if the percentage is too high.
+  if (PatternA.getPatternDifferences(PatternB, &ClonePair) == 1) {
+Pairs.push_back(ClonePair);
+break;
+  }
+}
+  }
+}
 
 DiagnosticsEngine &DiagEngine = Mgr.getDiagnostic();
 
@@ -81,7 +105,7 @@
 DiagnosticsEngine::Note, "Or maybe you wanted to use %0 here in this "
  "similar piece of code?");
 
-for (CloneDetector::SuspiciousClonePair &Pair : Clones) {
+for (VariablePattern::SuspiciousClonePair &Pair : Pairs) {
   // The first clone always has a suggestion and we report it to the user
   // along with the place where the suggestion should be used.
   DiagEngine.Report(Pair.FirstClone.Location.getBegin(), WarnID)
@@ -118,6 +142,8 @@
   // At this point, every statement in the translation unit has been analyzed by
   // the CloneDetector. The only thing left to do is to report the found clones.
 
+  SourceManager &SM = BR.getSourceManager();
+
   int MinComplexity = Mgr.getAnalyzerOptions().getOptionAsInteger(
   "MinimumCloneComplexity", 10, this);
   assert(MinComplexity >= 0);
@@ -128,11 +154,24 @@
   bool ReportNormalClones = Mgr.getAnalyzerOptions().getBooleanOption(
   "ReportNormalClones", true, this);
 
+  std::vector AllCloneGroups;
+  CloneDetector::PassList Passes = {
+  new MinComplexityPass(MinComplexity), new MinGroupSizePass(2),
+  new VerifyCloneHashesPass(), new OnlyLargestClonePass()};
+  CloneDetector.findClones(AllCloneGroups, Passes);
+
   if (ReportSuspiciousClones)
-reportSuspiciousClones(BR.getSourceManager(), Mgr, MinComplexity);
+reportSuspiciousClones(SM, Mgr, AllCloneGroups);
+
+  CloneDetector::PassList OnlyMatchingVariablePatterns = {
+  new MinGroupSizePass(2), new MatchingVariablePatternPass()};
+
+  std::vector CloneGroups;
+  CloneDetector::applyPasses(AllCloneGroups, CloneGroups,
+ OnlyMatchingVariablePatterns);
 
   if (ReportNormal

Re: [PATCH] D23418: [analyzer] Added a pass architecture to the CloneDetector

[Raphael Isemann via cfe-commits]

teemperor planned changes to this revision.
teemperor added a comment.

- Add description
- Make sure functions in CloneDetection.cpp are in a more logical order.


https://reviews.llvm.org/D23418



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

[PATCH] D23420: libcxx: Fix libcxx test on aarch64 with libunwind

[Adhemerval Zanella via cfe-commits]

zatrazz created this revision.
zatrazz added reviewers: jroelofs, danalbert.
zatrazz added subscribers: rengolin, cfe-commits.
Herald added a subscriber: aemerson.

Some tests uses 'long double' to/from conversions and for some targets
they are provided by compiler runtime (either compiler-rt or libgcc).
However when building libcxx with linunwinder current test configuration
at target_info.py do not include the required libraries, as:

  not llvm_unwinder:
[-lgcc_s] [-lgcc] [...] [-lgcc_s] [-lgcc]

  llvm_unwinder
[-lunwind] [-ldl]

This causes some tests build issues with missing symbols on aarch64,
for instance, where 'long double' is a binary float with 128-bits with
mostly of internal operations being provided by software routines.

This patch changes how to define the default linker flags by:

  not llvm_unwinder:
[-lgcc_s] [-lgcc]

  llvm_unwinder
[-lunwind] [-ldl] [-lgcc_s] [-lgcc]

I checked and aarch64 and x86_64 with libcxx and libunwind (with and without
LIBCXXABI_USE_LLVM_UNWINDER).

https://reviews.llvm.org/D23420

Files:
  test/libcxx/test/target_info.py

Index: test/libcxx/test/target_info.py
===
--- test/libcxx/test/target_info.py
+++ test/libcxx/test/target_info.py
@@ -170,17 +170,14 @@
 llvm_unwinder = self.full_config.get_lit_bool('llvm_unwinder', False)
 shared_libcxx = self.full_config.get_lit_bool('enable_shared', True)
 flags += ['-lm']
-if not llvm_unwinder:
-flags += ['-lgcc_s', '-lgcc']
 if enable_threads:
 flags += ['-lpthread']
 if not shared_libcxx:
   flags += ['-lrt']
 flags += ['-lc']
 if llvm_unwinder:
 flags += ['-lunwind', '-ldl']
-else:
-flags += ['-lgcc_s', '-lgcc']
+flags += ['-lgcc_s', '-lgcc']
 use_libatomic = self.full_config.get_lit_bool('use_libatomic', False)
 if use_libatomic:
 flags += ['-latomic']


Index: test/libcxx/test/target_info.py
===
--- test/libcxx/test/target_info.py
+++ test/libcxx/test/target_info.py
@@ -170,17 +170,14 @@
 llvm_unwinder = self.full_config.get_lit_bool('llvm_unwinder', False)
 shared_libcxx = self.full_config.get_lit_bool('enable_shared', True)
 flags += ['-lm']
-if not llvm_unwinder:
-flags += ['-lgcc_s', '-lgcc']
 if enable_threads:
 flags += ['-lpthread']
 if not shared_libcxx:
   flags += ['-lrt']
 flags += ['-lc']
 if llvm_unwinder:
 flags += ['-lunwind', '-ldl']
-else:
-flags += ['-lgcc_s', '-lgcc']
+flags += ['-lgcc_s', '-lgcc']
 use_libatomic = self.full_config.get_lit_bool('use_libatomic', False)
 if use_libatomic:
 flags += ['-latomic']
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r278395 - Don't enable PIE on i686-unknown-cloudabi.

[Ed Schouten via cfe-commits]

Author: ed
Date: Thu Aug 11 15:03:22 2016
New Revision: 278395

URL: http://llvm.org/viewvc/llvm-project?rev=278395&view=rev
Log:
Don't enable PIE on i686-unknown-cloudabi.

We're only going to provide support for using PIE on architectures that
provide PC-relative addressing. i686 is not one of those, so add the
necessary bits for only passing in -pie -zrelro conditionally.

Modified:
cfe/trunk/lib/Driver/ToolChains.cpp
cfe/trunk/lib/Driver/ToolChains.h
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/test/Driver/cloudabi.c
cfe/trunk/test/Driver/cloudabi.cpp

Modified: cfe/trunk/lib/Driver/ToolChains.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=278395&r1=278394&r2=278395&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains.cpp Thu Aug 11 15:03:22 2016
@@ -3344,6 +3344,19 @@ Tool *CloudABI::buildLinker() const {
   return new tools::cloudabi::Linker(*this);
 }
 
+bool CloudABI::isPIEDefault() const {
+  // Only enable PIE on architectures that support PC-relative
+  // addressing. PC-relative addressing is required, as the process
+  // startup code must be able to relocate itself.
+  switch (getTriple().getArch()) {
+  case llvm::Triple::aarch64:
+  case llvm::Triple::x86_64:
+return true;
+  default:
+return false;
+  }
+}
+
 SanitizerMask CloudABI::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::SafeStack;

Modified: cfe/trunk/lib/Driver/ToolChains.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.h?rev=278395&r1=278394&r2=278395&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains.h (original)
+++ cfe/trunk/lib/Driver/ToolChains.h Thu Aug 11 15:03:22 2016
@@ -632,8 +632,7 @@ public:
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) const override;
 
-  bool isPIEDefault() const override { return true; }
-
+  bool isPIEDefault() const override;
   SanitizerMask getSupportedSanitizers() const override;
   SanitizerMask getDefaultSanitizers() const override;
 

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=278395&r1=278394&r2=278395&view=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Thu Aug 11 15:03:22 2016
@@ -7523,11 +7523,13 @@ void cloudabi::Linker::ConstructJob(Comp
 
   // CloudABI only supports static linkage.
   CmdArgs.push_back("-Bstatic");
-
-  // CloudABI uses Position Independent Executables exclusively.
-  CmdArgs.push_back("-pie");
   CmdArgs.push_back("--no-dynamic-linker");
-  CmdArgs.push_back("-zrelro");
+
+  // Provide PIE linker flags in case PIE is default for the architecture.
+  if (ToolChain.isPIEDefault()) {
+CmdArgs.push_back("-pie");
+CmdArgs.push_back("-zrelro");
+  }
 
   CmdArgs.push_back("--eh-frame-hdr");
   CmdArgs.push_back("--gc-sections");

Modified: cfe/trunk/test/Driver/cloudabi.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cloudabi.c?rev=278395&r1=278394&r2=278395&view=diff
==
--- cfe/trunk/test/Driver/cloudabi.c (original)
+++ cfe/trunk/test/Driver/cloudabi.c Thu Aug 11 15:03:22 2016
@@ -1,8 +1,14 @@
 // RUN: %clang %s -### -target x86_64-unknown-cloudabi 2>&1 | FileCheck %s 
-check-prefix=SAFESTACK
 // SAFESTACK: "-cc1" "-triple" "x86_64-unknown-cloudabi" {{.*}} 
"-ffunction-sections" "-fdata-sections" {{.*}} "-fsanitize=safe-stack"
-// SAFESTACK: "-Bstatic" "-pie" "--no-dynamic-linker" "-zrelro" 
"--eh-frame-hdr" "--gc-sections" "-o" "a.out" "crt0.o" "crtbegin.o" "{{.*}}" 
"{{.*}}" "-lc" "-lcompiler_rt" "crtend.o"
+// SAFESTACK: "-Bstatic" "--no-dynamic-linker" "-pie" "-zrelro" 
"--eh-frame-hdr" "--gc-sections" "-o" "a.out" "crt0.o" "crtbegin.o" "{{.*}}" 
"{{.*}}" "-lc" "-lcompiler_rt" "crtend.o"
 
 // RUN: %clang %s -### -target x86_64-unknown-cloudabi 
-fno-sanitize=safe-stack 2>&1 | FileCheck %s -check-prefix=NOSAFESTACK
 // NOSAFESTACK: "-cc1" "-triple" "x86_64-unknown-cloudabi" {{.*}} 
"-ffunction-sections" "-fdata-sections"
 // NOSAFESTACK-NOT: "-fsanitize=safe-stack"
-// NOSAFESTACK: "-Bstatic" "-pie" "--no-dynamic-linker" "-zrelro" 
"--eh-frame-hdr" "--gc-sections" "-o" "a.out" "crt0.o" "crtbegin.o" "{{.*}}" 
"{{.*}}" "-lc" "-lcompiler_rt" "crtend.o"
+// NOSAFESTACK: "-Bstatic" "--no-dynamic-linker" "-pie" "-zrelro" 
"--eh-frame-hdr" "--gc-sections" "-o" "a.out" "crt0.o" "crtbegin.o" "{{.*}}" 
"{{.*}}" "-lc" "-lcompiler_rt" "crtend.o"
+
+// PIE shouldn't be enabled on i686. Just on architectures that provide
+// PC-relative addressing.
+// RUN: %cla

[PATCH] D23422: [VFS] Add 'ignore-non-existent-contents' field to YAML

[Bruno Cardoso Lopes via cfe-commits]

bruno created this revision.
bruno added a reviewer: benlangmuir.
bruno added a subscriber: cfe-commits.

Add 'ignore-non-existent-contents' to tell the VFS whether an invalid path
obtained via 'external-contents' should cause iteration on the VFS to stop.

If 'true', the VFS should ignore the entry and continue with the next. Allows
YAML files to be shared across multiple compiler invocations regardless of
prior existent paths in 'external-contents'. This global value is overridable
on a per-file basis.

This only adds the parsing part. Logic to actually use it will come next.

https://reviews.llvm.org/D23422

Files:
  include/clang/Basic/VirtualFileSystem.h
  lib/Basic/VirtualFileSystem.cpp
  test/VFS/Inputs/vfsoverlay2.yaml

Index: test/VFS/Inputs/vfsoverlay2.yaml
===
--- test/VFS/Inputs/vfsoverlay2.yaml
+++ test/VFS/Inputs/vfsoverlay2.yaml
@@ -1,5 +1,6 @@
 {
   'version': 0,
+  'ignore-non-existent-contents': false,
   'roots': [
 { 'name': 'OUT_DIR', 'type': 'directory',
   'contents': [
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -801,6 +801,7 @@
 ///   'case-sensitive': 
 ///   'use-external-names': 
 ///   'overlay-relative': 
+///   'ignore-non-existent-contents': 
 ///
 /// Virtual directories are represented as
 /// \verbatim
@@ -860,6 +861,14 @@
   /// \brief Whether to use to use the value of 'external-contents' for the
   /// names of files.  This global value is overridable on a per-file basis.
   bool UseExternalNames = true;
+
+  /// \brief Whether an invalid path obtained via 'external-contents' should
+  /// cause iteration on the VFS to stop. If 'true', the VFS should ignore
+  /// the entry and continue with the next. Allows YAML files to be shared
+  /// across multiple compiler invocations regardless of prior existent
+  /// paths in 'external-contents'. This global value is overridable on a
+  /// per-file basis.
+  bool IgnoreNonExistentContents = true;
   /// @}
 
   /// Virtual file paths and external files could be canonicalized without "..",
@@ -1301,6 +1310,7 @@
   KeyStatusPair("case-sensitive", false),
   KeyStatusPair("use-external-names", false),
   KeyStatusPair("overlay-relative", false),
+  KeyStatusPair("ignore-non-existent-contents", false),
   KeyStatusPair("roots", true),
 };
 
@@ -1359,6 +1369,9 @@
   } else if (Key == "use-external-names") {
 if (!parseScalarBool(I->getValue(), FS->UseExternalNames))
   return false;
+  } else if (Key == "ignore-non-existent-contents") {
+if (!parseScalarBool(I->getValue(), FS->IgnoreNonExistentContents))
+  return false;
   } else {
 llvm_unreachable("key missing from Keys");
   }
@@ -1619,7 +1632,7 @@
   JSONWriter(llvm::raw_ostream &OS) : OS(OS) {}
   void write(ArrayRef Entries, Optional UseExternalNames,
  Optional IsCaseSensitive, Optional IsOverlayRelative,
- StringRef OverlayDir);
+ Optional IgnoreNonExistentContents, StringRef OverlayDir);
 };
 }
 
@@ -1675,6 +1688,7 @@
Optional UseExternalNames,
Optional IsCaseSensitive,
Optional IsOverlayRelative,
+   Optional IgnoreNonExistentContents,
StringRef OverlayDir) {
   using namespace llvm::sys;
 
@@ -1692,6 +1706,9 @@
 OS << "  'overlay-relative': '"
<< (UseOverlayRelative ? "true" : "false") << "',\n";
   }
+  if (IgnoreNonExistentContents.hasValue())
+OS << "  'ignore-non-existent-contents': '"
+   << (IgnoreNonExistentContents.getValue() ? "true" : "false") << "',\n";
   OS << "  'roots': [\n";
 
   if (!Entries.empty()) {
@@ -1748,7 +1765,8 @@
   });
 
   JSONWriter(OS).write(Mappings, UseExternalNames, IsCaseSensitive,
-   IsOverlayRelative, OverlayDir);
+   IsOverlayRelative, IgnoreNonExistentContents,
+   OverlayDir);
 }
 
 VFSFromYamlDirIterImpl::VFSFromYamlDirIterImpl(
Index: include/clang/Basic/VirtualFileSystem.h
===
--- include/clang/Basic/VirtualFileSystem.h
+++ include/clang/Basic/VirtualFileSystem.h
@@ -340,6 +340,7 @@
   Optional IsCaseSensitive;
   Optional IsOverlayRelative;
   Optional UseExternalNames;
+  Optional IgnoreNonExistentContents;
   std::string OverlayDir;
 
 public:
@@ -351,6 +352,9 @@
   void setUseExternalNames(bool UseExtNames) {
 UseExternalNames = UseExtNames;
   }
+  void setIgnoreNonExistentContents(bool IgnoreContents) {
+IgnoreNonExistentContents = IgnoreContents;
+  }
   void setOverlayDir(StringRef OverlayDirectory) {
 IsOverlayRelative = true;
 OverlayDir.assign(OverlayDirectory.str());
___
cfe-com

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

In https://reviews.llvm.org/D23409#512819, @zturner wrote:

> In https://reviews.llvm.org/D23409#512745, @zturner wrote:
>
> > In https://reviews.llvm.org/D23409#512720, @aaron.ballman wrote:
> >
> > > Patch generally LGTM, though I wonder if there's a way we can add test 
> > > coverage for this.
> >
> >
> > I'd imagine we can just enable the clang-tidy test suite on Windows (I'm 
> > assuming it's currently disabled).  I'll look into it.
>
>
> Actually yea I'm not sure how to go about doing this.  All the existing tests 
> are written to use GCC style command line, and it seems unreasonable to add a 
> cl style command line variant of every single test.  maybe alexfh@ or someone 
> has a suggestion.


I think a single test for cl-style command line variant should be enough, if it 
verifies different aspects of the transformation of the command line (`/` 
prefix for flags, `\` path separators, different quoting, what else?).


https://reviews.llvm.org/D23409



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

Re: r276900 - [Sema] Teach getCurrentThisType to reconize lambda in in-class initializer

[Faisal Vali via cfe-commits]

On Thu, Aug 11, 2016 at 1:07 PM, Richard Smith  wrote:
> Yes, this should be merged. It's not quite right, but it's certainly better
> than what we had before.

I would agree  that the comment is broken - but, unless i'm missing
something subtle, the subsequent logic and call to
adjustCVQualifiersForCXXThisWithinLambda should handle the scenario
you describe.

>
> On Mon, Aug 8, 2016 at 1:34 PM, Hans Wennborg  wrote:
>>
>> Richard: ping?
>>
>> On Wed, Jul 27, 2016 at 4:46 PM, Hans Wennborg  wrote:
>> > Should this be merged to 3.9?
>> >
>> > Thanks,
>> > Hans
>> >
>> > On Wed, Jul 27, 2016 at 11:25 AM, Erik Pilkington via cfe-commits
>> >  wrote:
>> >> Author: epilk
>> >> Date: Wed Jul 27 13:25:10 2016
>> >> New Revision: 276900
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=276900&view=rev
>> >> Log:
>> >> [Sema] Teach getCurrentThisType to reconize lambda in in-class
>> >> initializer
>> >>
>> >> Fixes PR27994, a crash on valid.
>> >>
>> >> Differential revision: https://reviews.llvm.org/D21145
>> >>
>> >> Modified:
>> >> cfe/trunk/lib/Sema/SemaExprCXX.cpp
>> >> cfe/trunk/test/SemaCXX/lambda-expressions.cpp
>> >>
>> >> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
>> >> URL:
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=276900&r1=276899&r2=276900&view=diff
>> >>
>> >> ==
>> >> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
>> >> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Jul 27 13:25:10 2016
>> >> @@ -961,32 +961,26 @@ static QualType adjustCVQualifiersForCXX
>> >>  QualType Sema::getCurrentThisType() {
>> >>DeclContext *DC = getFunctionLevelDeclContext();
>> >>QualType ThisTy = CXXThisTypeOverride;
>> >> +
>> >>if (CXXMethodDecl *method = dyn_cast(DC)) {
>> >>  if (method && method->isInstance())
>> >>ThisTy = method->getThisType(Context);
>> >>}
>> >> -  if (ThisTy.isNull()) {
>> >> -if (isGenericLambdaCallOperatorSpecialization(CurContext) &&
>> >> -CurContext->getParent()->getParent()->isRecord()) {
>> >> -  // This is a generic lambda call operator that is being
>> >> instantiated
>> >> -  // within a default initializer - so use the enclosing class as
>> >> 'this'.
>> >> -  // There is no enclosing member function to retrieve the 'this'
>> >> pointer
>> >> -  // from.
>> >> -
>> >> -  // FIXME: This looks wrong. If we're in a lambda within a lambda
>> >> within a
>> >> -  // default member initializer, we need to recurse up more
>> >> parents to find
>> >> -  // the right context. Looks like we should be walking up to the
>> >> parent of
>> >> -  // the closure type, checking whether that is itself a lambda,
>> >> and if so,
>> >> -  // recursing, until we reach a class or a function that isn't a
>> >> lambda
>> >> -  // call operator. And we should accumulate the constness of
>> >> *this on the
>> >> -  // way.
>> >> -
>> >> -  QualType ClassTy = Context.getTypeDeclType(
>> >> -  cast(CurContext->getParent()->getParent()));
>> >> -  // There are no cv-qualifiers for 'this' within default
>> >> initializers,
>> >> -  // per [expr.prim.general]p4.
>> >> -  ThisTy = Context.getPointerType(ClassTy);
>> >> -}
>> >> +
>> >> +  if (ThisTy.isNull() && isLambdaCallOperator(CurContext) &&
>> >> +  !ActiveTemplateInstantiations.empty()) {
>> >> +
>> >> +assert(isa(DC) &&
>> >> +   "Trying to get 'this' type from static method?");
>> >> +
>> >> +// This is a lambda call operator that is being instantiated as a
>> >> default
>> >> +// initializer. DC must point to the enclosing class type, so we
>> >> can recover
>> >> +// the 'this' type from it.
>> >> +
>> >> +QualType ClassTy =
>> >> Context.getTypeDeclType(cast(DC));
>> >> +// There are no cv-qualifiers for 'this' within default
>> >> initializers,
>> >> +// per [expr.prim.general]p4.
>
>
> This is wrong in C++17 onwards: if *this is captured by value by a
> non-mutable lambda, it should become const-qualified.
>
>>
>> >> +ThisTy = Context.getPointerType(ClassTy);
>> >>}
>> >>
>> >>// If we are within a lambda's call operator, the cv-qualifiers of
>> >> 'this'
>> >>
>> >> Modified: cfe/trunk/test/SemaCXX/lambda-expressions.cpp
>> >> URL:
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/lambda-expressions.cpp?rev=276900&r1=276899&r2=276900&view=diff
>> >>
>> >> ==
>> >> --- cfe/trunk/test/SemaCXX/lambda-expressions.cpp (original)
>> >> +++ cfe/trunk/test/SemaCXX/lambda-expressions.cpp Wed Jul 27 13:25:10
>> >> 2016
>> >> @@ -1,5 +1,4 @@
>> >> -// RUN: %clang_cc1 -std=c++11 -Wno-unused-value -fsyntax-only -verify
>> >> -fblocks %s
>> >> -// RUN: %clang_cc1 -std=c++1y -Wno-unused-value -fsyntax-only -verify
>> >> -fblocks %s
>> >> +// RUN: %clang_cc1 -std=c++14 -Wno-unused-value 

Re: r278395 - Don't enable PIE on i686-unknown-cloudabi.

[Joerg Sonnenberger via cfe-commits]

On Thu, Aug 11, 2016 at 08:03:22PM -, Ed Schouten via cfe-commits wrote:
> Modified: cfe/trunk/lib/Driver/ToolChains.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=278395&r1=278394&r2=278395&view=diff
> ==
> --- cfe/trunk/lib/Driver/ToolChains.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains.cpp Thu Aug 11 15:03:22 2016
> @@ -3344,6 +3344,19 @@ Tool *CloudABI::buildLinker() const {
>return new tools::cloudabi::Linker(*this);
>  }
>  
> +bool CloudABI::isPIEDefault() const {
> +  // Only enable PIE on architectures that support PC-relative
> +  // addressing. PC-relative addressing is required, as the process
> +  // startup code must be able to relocate itself.
> +  switch (getTriple().getArch()) {
> +  case llvm::Triple::aarch64:
> +  case llvm::Triple::x86_64:
> +return true;
> +  default:
> +return false;
> +  }
> +}
> +
>  SanitizerMask CloudABI::getSupportedSanitizers() const {
>SanitizerMask Res = ToolChain::getSupportedSanitizers();
>Res |= SanitizerKind::SafeStack;
> 

This comment doesn't make sense to me. x86 requires a relative call per
function to effectively compute EIP. The only difference for relocation
purposes is that GP register. All other questions are identical between
x86 and x86_64.

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

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

May I ask you to upload patches with full diff context next time? I know, it's 
not directly supported by TortoiseGit, but there are at least two other 
reasonable ways of generating full diffs for Phabricator: 
http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface 
and 
http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-command-line.

Wrt testing, I think, apart from a new clang-tidy test, the changes to Driver 
and Tooling need separate tests as well.



Comment at: lib/Tooling/JSONCompilationDatabase.cpp:119
@@ -115,1 +118,3 @@
 StringRef EscapedCommandLine) {
+#if defined(LLVM_ON_WIN32)
+  llvm::BumpPtrAllocator Alloc;

zturner wrote:
> rnk wrote:
> > It would be nice if the JSON file just told us which quoting mechanism it 
> > was using. You can imagine building the compilation database on one system 
> > and sending it off to another for indexing.
> Correct me if I've got this wrong but:
> 
> 1. If you had a compile command database generated on non-Windows, and you 
> used it on Windows, then it would necessarily be referring to something other 
> than clang-cl right?  In which case, we don't support running clang in non-cl 
> mode on Windows.
> 
> 2. If you had a compile command database generated on Windows, then the only 
> supported configuration would involve clang-cl, in which case you couldn't 
> use it on non-Windows.
> 
> Is there a supported use case where a non-Windows compile database would be 
> used on Windows or vice versa?
> we don't support running clang in non-cl mode on Windows.

I think, clang in non-cl mode is functional on Windows to a certain degree 
(mingw-compatibility or something like this). Artificially preventing it from 
working doesn't seem to be right.


https://reviews.llvm.org/D23409



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

Re: [PATCH] D23423: [Clang-tidy] Comparison Function Address

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a comment.

Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).

If I'm not mistaken, GCC or Clang has warning that result of comparison is 
always true for such situations.



Comment at: clang-tidy/misc/ComparisonFunctionAddressCheck.h:19
@@ +18,3 @@
+
+/// This Checker gives a warning if address of a function is compared.
+/// For example: the programmer wants to write getc()==0 but writes getc==0.

Please lowercase Checker. Add add spaces across == below.


Comment at: docs/clang-tidy/checks/misc-comparison-function-address.rst:6
@@ +5,3 @@
+
+This Checker gives a warning if address of a function is compared.
+For example: the programmer wants to write ``getc()==0`` but writes ``getc==0``

Please lowercase Checker.


Comment at: docs/clang-tidy/checks/misc-comparison-function-address.rst:7
@@ +6,3 @@
+This Checker gives a warning if address of a function is compared.
+For example: the programmer wants to write ``getc()==0`` but writes ``getc==0``
+

Please add space across ==. Please also run Clang-format over examples code.


Comment at: docs/clang-tidy/checks/misc-comparison-function-address.rst:18
@@ +17,3 @@
+
+as a general rule, function pointers can be compared to other function 
pointers, function, 0, nullptr
+functions can be compared only against function pointers

Please highlight nullptr with ``.


Comment at: test/clang-tidy/misc-comparison-function-address.cpp:16
@@ +15,3 @@
+
+
+void test_function_warning() {

Unnecessary empty line.


Comment at: test/clang-tidy/misc-comparison-function-address.cpp:30
@@ +29,3 @@
+}
+
+

Unnecessary empty line.


Repository:
  rL LLVM

https://reviews.llvm.org/D23423



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

Re: [PATCH] D23423: [Clang-tidy] Comparison Function Address

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a comment.

Clang-tidy terminology use //checks//, not //checkers//.


Repository:
  rL LLVM

https://reviews.llvm.org/D23423



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

[PATCH] D23427: [Clang-tidy] Comparison Misuse

[Benedek Kiss via cfe-commits]

falho created this revision.
falho added reviewers: xazax.hun, o.gyorgy, alexfh, aaron.ballman, etienneb, 
hokein, Prazek.
falho added a subscriber: cfe-commits.
falho set the repository for this revision to rL LLVM.

This checker warns for the misuse of comparison operators
- char* is compared to a string literal
- inequality operator usage for NULL

Repository:
  rL LLVM

https://reviews.llvm.org/D23427

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/ComparisonMisuseCheck.cpp
  clang-tidy/misc/ComparisonMisuseCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-comparison-misuse.rst
  test/clang-tidy/misc-comparison-misuse.cpp

Index: test/clang-tidy/misc-comparison-misuse.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-comparison-misuse.cpp
@@ -0,0 +1,20 @@
+// RUN: %check_clang_tidy %s misc-comparison-misuse %t
+
+#define NULL __null
+
+bool test_pointer_to_literal(const char *my){
+  bool b = (my=="mystring");
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: char* is compared to a string literal [misc-comparison-misuse]
+  return "mystring"==my;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: char* is compared to a string literal [misc-comparison-misuse]
+}
+
+void test_null_to_pointer(int *p){
+  if (NULL>=p);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: comparison to nullptr [misc-comparison-misuse]
+  
+  if (NULL==p);
+  
+  if (NULL!=p);
+}
+
Index: docs/clang-tidy/checks/misc-comparison-misuse.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-comparison-misuse.rst
@@ -0,0 +1,25 @@
+.. title:: clang-tidy - misc-comparison-misuse
+
+misc-comparison-misuse
+==
+
+This checker reports errors related to the misuse of the comparison operator.
+It should warn for the following cases:
+
+Case 1:
+  ``char*`` is compared to a string literal.
+
+.. code-block::
+   bool isMyString(const char * my){
+return "mystring"==my;//error. comparing pointer to string literal
+   }
+
+
+Case 2:
+  Inequality operator usage for ``NULL``.
+
+.. code-block:: c++
+   void(int * p){
+ if (NULL>=p)//error, use only NULL==p, NULL!=p
+   }
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -55,6 +55,7 @@
misc-argument-comment
misc-assert-side-effect
misc-bool-pointer-implicit-conversion
+   misc-comparison-misuse
misc-dangling-handle
misc-definitions-in-headers
misc-fold-init-type
Index: clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -15,6 +15,7 @@
 #include "MisplacedConstCheck.h"
 #include "UnconventionalAssignOperatorCheck.h"
 #include "BoolPointerImplicitConversionCheck.h"
+#include "ComparisonMisuseCheck.h"
 #include "DanglingHandleCheck.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "FoldInitTypeCheck.h"
@@ -68,6 +69,8 @@
 "misc-unconventional-assign-operator");
 CheckFactories.registerCheck(
 "misc-bool-pointer-implicit-conversion");
+CheckFactories.registerCheck(
+"misc-comparison-misuse");
 CheckFactories.registerCheck(
 "misc-dangling-handle");
 CheckFactories.registerCheck(
Index: clang-tidy/misc/ComparisonMisuseCheck.h
===
--- /dev/null
+++ clang-tidy/misc/ComparisonMisuseCheck.h
@@ -0,0 +1,39 @@
+//===--- ComparisonMisuseCheck.h - clang-tidy*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_COMPARISON_MISUSE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_COMPARISON_MISUSE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This checker reports errors related to the misuse of the comparison operator.
+/// It should warn for the following cases:
+///   - strcmp,strncmp,memcmp misuse.
+///   - char* is compared to a string literal
+///   - inequality operator usage for NULL
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-comparison-misuse.html
+class ComparisonMisuseCheck : public ClangTidyCheck {
+public:
+  ComparisonMisuseCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) over

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

[Zachary Turner via cfe-commits]

zturner added a comment.

In https://reviews.llvm.org/D23409#513017, @alexfh wrote:

> May I ask you to upload patches with full diff context next time? I know, 
> it's not directly supported by TortoiseGit, but there are at least two other 
> reasonable ways of generating full diffs for Phabricator: 
> http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
>  and 
> http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-command-line.
>
> Wrt testing, I think, apart from a new clang-tidy test, the changes to Driver 
> and Tooling need separate tests as well.


I'm unsure what to actually test in the Driver and Tooling tests.  For example, 
in Tooling we just change from using one function to using another function.  
And that function is already tested in the LLVM test suite.  It seems redundant 
to add another test that is for all intents and purposes equivalent to "Does 
LLVM's windows command line parser work correctly?", which is already tested.

How necessary is this?

For the clang-tidy test, I will need to make it only enabled on Windows, since 
other platforms are not going to be building clang-cl.  I think I should be 
able to figure that out.


https://reviews.llvm.org/D23409



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

Re: [PATCH] D23420: libcxx: Fix libcxx test on aarch64 with libunwind

[Asiri Rathnayake via cfe-commits]

rmaprath added a subscriber: rmaprath.
rmaprath added a comment.

I too noticed this very recently (on our non-bare-metal builds), and wondered 
why these dependencies are not included by default. Thanks for looking into it. 
I'll let someone familiar with the details approve.


https://reviews.llvm.org/D23420



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

Re: [PATCH] D22346: [Clang-tidy] CERT-MSC50-CPP (std:rand() )

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a comment.

Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).



Comment at: docs/clang-tidy/checks/cert-msc50-cpp.rst:4
@@ +3,3 @@
+cert-msc-50
+===
+

Should be same length as section name above.


Comment at: docs/clang-tidy/checks/cert-msc50-cpp.rst:6
@@ +5,2 @@
+
+Pseudorandom number generators use mathematical algorithms to produce a 
sequence of numbers with good statistical properties, but the numbers produced 
are not genuinely random. This checker warns for the usage of std::rand().

Please use check and highlight std::rand() with ``.


Repository:
  rL LLVM

https://reviews.llvm.org/D22346



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

Re: [PATCH] D23421: [Clang-tidy] CERT-MSC53-CPP (checker for std namespace modification)

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this check! There is a slight problem, however, in 
that the check as-written will flag false positives because there are times 
when it is permissible to modify the std namespace. The important bit of the 
CERT requirement is "Do not add declarations or definitions to the standard 
namespaces std or posix, or to a namespace contained therein, except for a 
template specialization that depends on a user-defined type that meets the 
standard library requirements for the original template." So the part that's 
missing from this check is excluding template specializations that depend on 
user-defined types. For instance, the following should be valid:

  namespace std {
  template <>
  struct is_error_code_enum : std::true_type {};
  }

(This comes from Error.h in LLVM.)



Comment at: clang-tidy/cert/CERTTidyModule.cpp:37-38
@@ -35,2 +36,4 @@
 // DCL
+CheckFactories.registerCheck(
+"cert-msc53-cpp");
 CheckFactories.registerCheck(

Should put this under a `// MSC` comment rather than the `// DCL` one.


Comment at: clang-tidy/cert/DontModifyStdNamespaceCheck.cpp:33
@@ +32,3 @@
+
+  diag(Nmspc->getLocation(), "Modification of " + namespaceName +
+ " namespace can result to undefined 
behavior");

Should use format specifiers instead of building the string manually. e.g., 
using %0. Also, diagnostics should start with a lowercase letter and "to" 
should be "in".


Comment at: clang-tidy/cert/DontModifyStdNamespaceCheck.h:19
@@ +18,3 @@
+
+/// Modification of the std or posix namespace can result to undefined 
behavior.
+/// This checker warns for such modifications.

s/to/in


Comment at: docs/clang-tidy/checks/list.rst:12
@@ -11,2 +11,3 @@
cert-dcl59-cpp (redirects to google-build-namespaces) 
+   cert-msc53-cpp
cert-env33-c

Should keep this in alphabetical order.


Repository:
  rL LLVM

https://reviews.llvm.org/D23421



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

Re: [PATCH] D23427: [Clang-tidy] Comparison Misuse

[Eugene Zelenko via cfe-commits]

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

Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).

Please run Clang-format on newly added files. Test case is definitely needs it.



Comment at: clang-tidy/misc/ComparisonMisuseCheck.h:19
@@ +18,3 @@
+
+/// This checker reports errors related to the misuse of the comparison 
operator.
+/// It should warn for the following cases:

Check, please.


Comment at: clang-tidy/misc/ComparisonMisuseCheck.h:21
@@ +20,3 @@
+/// It should warn for the following cases:
+///   - strcmp,strncmp,memcmp misuse.
+///   - char* is compared to a string literal

Spaces after commas,


Comment at: docs/clang-tidy/checks/misc-comparison-misuse.rst:6
@@ +5,3 @@
+
+This checker reports errors related to the misuse of the comparison operator.
+It should warn for the following cases:

Check, please.


Comment at: docs/clang-tidy/checks/misc-comparison-misuse.rst:13
@@ +12,3 @@
+.. code-block::
+   bool isMyString(const char * my){
+return "mystring"==my;//error. comparing pointer to string literal

Clang-format examples, please.


Repository:
  rL LLVM

https://reviews.llvm.org/D23427



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

Re: [PATCH] D23427: [Clang-tidy] Comparison Misuse

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added inline comments.


Comment at: clang-tidy/misc/ComparisonMisuseCheck.cpp:21
@@ +20,3 @@
+void ComparisonMisuseCheck::registerMatchers(MatchFinder *Finder) {
+
+  Finder->addMatcher(

Please remove this line.


Repository:
  rL LLVM

https://reviews.llvm.org/D23427



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

Re: [PATCH] D23423: [Clang-tidy] Comparison Function Address

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a comment.

This check looks like specific case of https://reviews.llvm.org/D23427. May be 
they should be merged?


Repository:
  rL LLVM

https://reviews.llvm.org/D23423



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

Re: [PATCH] D23422: [VFS] Add 'ignore-non-existent-contents' field to YAML

[Ben Langmuir via cfe-commits]

benlangmuir added a comment.

This should have a test for the writer change.  Otherwise LGTM.


https://reviews.llvm.org/D23422



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

[libcxx] r278425 - Merging r278387:

[Hans Wennborg via cfe-commits]

Author: hans
Date: Thu Aug 11 16:47:28 2016
New Revision: 278425

URL: http://llvm.org/viewvc/llvm-project?rev=278425&view=rev
Log:
Merging r278387:

r278387 | marshall | 2016-08-11 11:46:24 -0700 (Thu, 11 Aug 2016) | 1 line

Remove test for the sign of a NaN - doesn't work on MIPS, not strictly legal. 
Fixes bug 28936


Modified:
libcxx/branches/release_39/   (props changed)

libcxx/branches/release_39/test/std/numerics/complex.number/complex.transcendentals/asin.pass.cpp

Propchange: libcxx/branches/release_39/
--
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Aug 11 16:47:28 2016
@@ -1 +1,2 @@
 /libcxx/branches/apple:136569-137939
+/libcxx/trunk:278387

Modified: 
libcxx/branches/release_39/test/std/numerics/complex.number/complex.transcendentals/asin.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/branches/release_39/test/std/numerics/complex.number/complex.transcendentals/asin.pass.cpp?rev=278425&r1=278424&r2=278425&view=diff
==
--- 
libcxx/branches/release_39/test/std/numerics/complex.number/complex.transcendentals/asin.pass.cpp
 (original)
+++ 
libcxx/branches/release_39/test/std/numerics/complex.number/complex.transcendentals/asin.pass.cpp
 Thu Aug 11 16:47:28 2016
@@ -91,7 +91,6 @@ void test_edges()
 {
 assert(std::isnan(r.real()));
 assert(std::isnan(r.imag()));
-assert(std::signbit(testcases[i].imag()) == 
std::signbit(r.imag()));
 }
 else if (std::isnan(testcases[i].real()) && 
std::isinf(testcases[i].imag()))
 {


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

Re: [libcxx] r278387 - Remove test for the sign of a NaN - doesn't work on MIPS, not strictly legal. Fixes bug 28936

[Hans Wennborg via cfe-commits]

Merged to 3.9 in r278425, as the PR was marked as a release blocker.

On Thu, Aug 11, 2016 at 11:46 AM, Marshall Clow via cfe-commits
 wrote:
> Author: marshall
> Date: Thu Aug 11 13:46:24 2016
> New Revision: 278387
>
> URL: http://llvm.org/viewvc/llvm-project?rev=278387&view=rev
> Log:
> Remove test for the sign of a NaN - doesn't work on MIPS, not strictly legal. 
> Fixes bug 28936
>
> Modified:
> 
> libcxx/trunk/test/std/numerics/complex.number/complex.transcendentals/asin.pass.cpp
>
> Modified: 
> libcxx/trunk/test/std/numerics/complex.number/complex.transcendentals/asin.pass.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/numerics/complex.number/complex.transcendentals/asin.pass.cpp?rev=278387&r1=278386&r2=278387&view=diff
> ==
> --- 
> libcxx/trunk/test/std/numerics/complex.number/complex.transcendentals/asin.pass.cpp
>  (original)
> +++ 
> libcxx/trunk/test/std/numerics/complex.number/complex.transcendentals/asin.pass.cpp
>  Thu Aug 11 13:46:24 2016
> @@ -91,7 +91,6 @@ void test_edges()
>  {
>  assert(std::isnan(r.real()));
>  assert(std::isnan(r.imag()));
> -assert(std::signbit(testcases[i].imag()) == 
> std::signbit(r.imag()));
>  }
>  else if (std::isnan(testcases[i].real()) && 
> std::isinf(testcases[i].imag()))
>  {
>
>
> ___
> 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] D23420: libcxx: Fix libcxx test on aarch64 with libunwind

[Eric Fiselier via cfe-commits]

EricWF added a subscriber: EricWF.
EricWF added a reviewer: EricWF.
EricWF added a comment.

Urg... Not this again. This link order is important. I can't remember *why*, 
but libgcc has to appear both before and after `pthread` and `libc` IIRC.  
Either way the intention here is to mimic the output of Clangs driver. For 
example the order Clang generates on Linux is:

  // clang++ -### -pthread -stdlib=libc++ -xc++ - < /dev/null
  "-lc++" "-lm" "-lgcc_s" "-lgcc" "-lpthread" "-lc" "-lgcc_s" "-lgcc"

What link order does the compiler generate on your platform?


https://reviews.llvm.org/D23420



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

[libcxx] r278426 - Merging r278282:

[Hans Wennborg via cfe-commits]

Author: hans
Date: Thu Aug 11 16:48:37 2016
New Revision: 278426

URL: http://llvm.org/viewvc/llvm-project?rev=278426&view=rev
Log:
Merging r278282:

r278282 | marshall | 2016-08-10 13:04:46 -0700 (Wed, 10 Aug 2016) | 1 line

std:: quailfy the calls for cend/crend/cbegin/cend. Fixes bug 28927.


Added:

libcxx/branches/release_39/test/std/iterators/iterator.range/begin-end.fail.cpp
  - copied unchanged from r278282, 
libcxx/trunk/test/std/iterators/iterator.range/begin-end.fail.cpp
Modified:
libcxx/branches/release_39/   (props changed)
libcxx/branches/release_39/include/iterator

Propchange: libcxx/branches/release_39/
--
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Aug 11 16:48:37 2016
@@ -1,2 +1,2 @@
 /libcxx/branches/apple:136569-137939
-/libcxx/trunk:278387
+/libcxx/trunk:278282,278387

Modified: libcxx/branches/release_39/include/iterator
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/branches/release_39/include/iterator?rev=278426&r1=278425&r2=278426&view=diff
==
--- libcxx/branches/release_39/include/iterator (original)
+++ libcxx/branches/release_39/include/iterator Thu Aug 11 16:48:37 2016
@@ -1632,16 +1632,16 @@ reverse_iterator rend(initia
 
 template 
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
-auto cbegin(const _Cp& __c) -> decltype(begin(__c))
+auto cbegin(const _Cp& __c) -> decltype(_VSTD::begin(__c))
 {
-return begin(__c);
+return _VSTD::begin(__c);
 }
 
 template 
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
-auto cend(const _Cp& __c) -> decltype(end(__c))
+auto cend(const _Cp& __c) -> decltype(_VSTD::end(__c))
 {
-return end(__c);
+return _VSTD::end(__c);
 }
 
 template 
@@ -1674,16 +1674,16 @@ auto rend(const _Cp& __c) -> decltype(__
 
 template 
 inline _LIBCPP_INLINE_VISIBILITY
-auto crbegin(const _Cp& __c) -> decltype(rbegin(__c))
+auto crbegin(const _Cp& __c) -> decltype(_VSTD::rbegin(__c))
 {
-return rbegin(__c);
+return _VSTD::rbegin(__c);
 }
 
 template 
 inline _LIBCPP_INLINE_VISIBILITY
-auto crend(const _Cp& __c) -> decltype(rend(__c))
+auto crend(const _Cp& __c) -> decltype(_VSTD::rend(__c))
 {
-return rend(__c);
+return _VSTD::rend(__c);
 }
 
 #endif


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

Re: [libcxx] r278282 - std:: quailfy the calls for cend/crend/cbegin/cend. Fixes bug 28927.

[Hans Wennborg via cfe-commits]

I've gone ahead and merged it in r278426.

On Wed, Aug 10, 2016 at 5:19 PM, Eric Fiselier  wrote:
> I would support merging this change.
>
> /Eric
>
> On Wed, Aug 10, 2016 at 3:20 PM, Hans Wennborg via cfe-commits
>  wrote:
>>
>> Should we merge this to 3.9? Looks like a straight-forward fix for
>> something that was hit by real code.
>>
>> Thanks,
>> Hans
>>
>> On Wed, Aug 10, 2016 at 1:04 PM, Marshall Clow via cfe-commits
>>  wrote:
>> > Author: marshall
>> > Date: Wed Aug 10 15:04:46 2016
>> > New Revision: 278282
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=278282&view=rev
>> > Log:
>> > std:: quailfy the calls for cend/crend/cbegin/cend. Fixes bug 28927.
>> >
>> > Added:
>> > libcxx/trunk/test/std/iterators/iterator.range/begin-end.fail.cpp
>> > Modified:
>> > libcxx/trunk/include/iterator
>> >
>> > Modified: libcxx/trunk/include/iterator
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/iterator?rev=278282&r1=278281&r2=278282&view=diff
>> >
>> > ==
>> > --- libcxx/trunk/include/iterator (original)
>> > +++ libcxx/trunk/include/iterator Wed Aug 10 15:04:46 2016
>> > @@ -1632,16 +1632,16 @@ reverse_iterator rend(initia
>> >
>> >  template 
>> >  inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
>> > -auto cbegin(const _Cp& __c) -> decltype(begin(__c))
>> > +auto cbegin(const _Cp& __c) -> decltype(_VSTD::begin(__c))
>> >  {
>> > -return begin(__c);
>> > +return _VSTD::begin(__c);
>> >  }
>> >
>> >  template 
>> >  inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
>> > -auto cend(const _Cp& __c) -> decltype(end(__c))
>> > +auto cend(const _Cp& __c) -> decltype(_VSTD::end(__c))
>> >  {
>> > -return end(__c);
>> > +return _VSTD::end(__c);
>> >  }
>> >
>> >  template 
>> > @@ -1674,16 +1674,16 @@ auto rend(const _Cp& __c) -> decltype(__
>> >
>> >  template 
>> >  inline _LIBCPP_INLINE_VISIBILITY
>> > -auto crbegin(const _Cp& __c) -> decltype(rbegin(__c))
>> > +auto crbegin(const _Cp& __c) -> decltype(_VSTD::rbegin(__c))
>> >  {
>> > -return rbegin(__c);
>> > +return _VSTD::rbegin(__c);
>> >  }
>> >
>> >  template 
>> >  inline _LIBCPP_INLINE_VISIBILITY
>> > -auto crend(const _Cp& __c) -> decltype(rend(__c))
>> > +auto crend(const _Cp& __c) -> decltype(_VSTD::rend(__c))
>> >  {
>> > -return rend(__c);
>> > +return _VSTD::rend(__c);
>> >  }
>> >
>> >  #endif
>> >
>> > Added: libcxx/trunk/test/std/iterators/iterator.range/begin-end.fail.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/iterators/iterator.range/begin-end.fail.cpp?rev=278282&view=auto
>> >
>> > ==
>> > --- libcxx/trunk/test/std/iterators/iterator.range/begin-end.fail.cpp
>> > (added)
>> > +++ libcxx/trunk/test/std/iterators/iterator.range/begin-end.fail.cpp
>> > Wed Aug 10 15:04:46 2016
>> > @@ -0,0 +1,51 @@
>> >
>> > +//===--===//
>> > +//
>> > +// The LLVM Compiler Infrastructure
>> > +//
>> > +// This file is dual licensed under the MIT and the University of
>> > Illinois Open
>> > +// Source Licenses. See LICENSE.TXT for details.
>> > +//
>> >
>> > +//===--===//
>> > +
>> > +#include "test_macros.h"
>> > +
>> > +#if TEST_STD_VER < 11
>> > +#error
>> > +#else
>> > +
>> > +// 
>> > +// template  auto begin(C& c) -> decltype(c.begin());
>> > +// template  auto begin(const C& c) -> decltype(c.begin());
>> > +// template  auto end(C& c) -> decltype(c.end());
>> > +// template  auto end(const C& c) -> decltype(c.end());
>> > +// template  reverse_iterator
>> > rbegin(initializer_list il);
>> > +// template  reverse_iterator
>> > rend(initializer_list il);
>> > +
>> > +
>> > +#include 
>> > +#include 
>> > +
>> > +namespace Foo {
>> > +   struct FakeContainer {};
>> > +   typedef int FakeIter;
>> > +
>> > +   FakeIter begin(const FakeContainer &)   { return 1; }
>> > +   FakeIter end  (const FakeContainer &)   { return 2; }
>> > +   FakeIter rbegin(const FakeContainer &)  { return 3; }
>> > +   FakeIter rend  (const FakeContainer &)  { return 4; }
>> > +
>> > +   FakeIter cbegin(const FakeContainer &)  { return 11; }
>> > +   FakeIter cend  (const FakeContainer &)  { return 12; }
>> > +   FakeIter crbegin(const FakeContainer &) { return 13; }
>> > +   FakeIter crend  (const FakeContainer &) { return 14; }
>> > +}
>> > +
>> > +
>> > +int main(){
>> > +// Bug #28927 - shouldn't find these via ADL
>> > +   (void) std::cbegin (Foo::FakeContainer());
>> > +   (void) std::cend   (Foo::FakeContainer());
>> > +   (void) std::crbegin(Foo::FakeContainer());
>> > +   (void) std::crend  (Foo::FakeContainer());
>> > +}
>> > +#endif
>> >
>> >
>> > ___

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

[Zachary Turner via cfe-commits]

zturner updated this revision to Diff 67751.
zturner added a comment.

I added a test for the clang Driver changes.

It seems I'm still having some trouble writing the clang-cl test for 
clang-tidy.  It depends on having a different compilation database that uses 
clang-cl instead of clang.  I found one set of tests called 
`clang-tidy-run-with-database.cpp` but it requires shell, which will make it 
not work on Windows, which is the main place it should be tested in the first 
place.


https://reviews.llvm.org/D23409

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  lib/Tooling/JSONCompilationDatabase.cpp
  unittests/Driver/ToolChainTest.cpp

Index: unittests/Driver/ToolChainTest.cpp
===
--- unittests/Driver/ToolChainTest.cpp
+++ unittests/Driver/ToolChainTest.cpp
@@ -117,4 +117,29 @@
 S);
 }
 
+TEST(ToolChainTest, DefaultDriverMode) {
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
+  struct TestDiagnosticConsumer : public DiagnosticConsumer {};
+  DiagnosticsEngine Diags(DiagID, &*DiagOpts, new TestDiagnosticConsumer);
+  IntrusiveRefCntPtr InMemoryFileSystem(
+  new vfs::InMemoryFileSystem);
+
+  Driver CCDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags,
+  InMemoryFileSystem);
+  Driver CXXDriver("/home/test/bin/clang++", "arm-linux-gnueabi", Diags,
+   InMemoryFileSystem);
+  Driver CLDriver("/home/test/bin/clang-cl", "arm-linux-gnueabi", Diags,
+  InMemoryFileSystem);
+
+  std::unique_ptr CC(CCDriver.BuildCompilation({"foo.cpp"}));
+  std::unique_ptr CXX(CXXDriver.BuildCompilation({"foo.cpp"}));
+  std::unique_ptr CL(CLDriver.BuildCompilation({"foo.cpp"}));
+
+  EXPECT_TRUE(CCDriver.CCCIsCC());
+  EXPECT_TRUE(CXXDriver.CCCIsCXX());
+  EXPECT_TRUE(CLDriver.IsCLMode());
+}
+
 } // end anonymous namespace
Index: lib/Tooling/JSONCompilationDatabase.cpp
===
--- lib/Tooling/JSONCompilationDatabase.cpp
+++ lib/Tooling/JSONCompilationDatabase.cpp
@@ -16,7 +16,10 @@
 #include "clang/Tooling/CompilationDatabasePluginRegistry.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Allocator.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/StringSaver.h"
 #include 
 
 namespace clang {
@@ -113,8 +116,17 @@
 
 std::vector unescapeCommandLine(
 StringRef EscapedCommandLine) {
+#if defined(LLVM_ON_WIN32)
+  llvm::BumpPtrAllocator Alloc;
+  llvm::StringSaver Saver(Alloc);
+  llvm::SmallVector T;
+  llvm::cl::TokenizeWindowsCommandLine(EscapedCommandLine, Saver, T);
+  std::vector Result(T.begin(), T.end());
+  return Result;
+#else
   CommandLineArgumentParser parser(EscapedCommandLine);
   return parser.parse();
+#endif
 }
 
 class JSONCompilationDatabasePlugin : public CompilationDatabasePlugin {
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -88,31 +88,39 @@
   llvm::DeleteContainerSeconds(ToolChains);
 }
 
-void Driver::ParseDriverMode(ArrayRef Args) {
-  const std::string OptName =
-  getOpts().getOption(options::OPT_driver_mode).getPrefixedName();
+void Driver::ParseDriverMode(StringRef ProgramName,
+ ArrayRef Args) {
+  auto Default = ToolChain::getTargetAndModeFromProgramName(ProgramName);
+  StringRef DefaultMode(Default.second);
+  setDriverModeFromOption(DefaultMode);
 
   for (const char *ArgPtr : Args) {
 // Ingore nullptrs, they are response file's EOL markers
 if (ArgPtr == nullptr)
   continue;
 const StringRef Arg = ArgPtr;
-if (!Arg.startswith(OptName))
-  continue;
+setDriverModeFromOption(Arg);
+  }
+}
+
+void Driver::setDriverModeFromOption(StringRef Opt) {
+  const std::string OptName =
+  getOpts().getOption(options::OPT_driver_mode).getPrefixedName();
+  if (!Opt.startswith(OptName))
+return;
+  StringRef Value = Opt.drop_front(OptName.size());
 
-const StringRef Value = Arg.drop_front(OptName.size());
-const unsigned M = llvm::StringSwitch(Value)
-   .Case("gcc", GCCMode)
-   .Case("g++", GXXMode)
-   .Case("cpp", CPPMode)
-   .Case("cl", CLMode)
-   .Default(~0U);
+  const unsigned M = llvm::StringSwitch(Value)
+ .Case("gcc", GCCMode)
+ .Case("g++", GXXMode)
+ .Case("cpp", CPPMode)
+ .Case("cl", CLMode)
+ .Default(~0U);
 
-if (M != ~0U)
-  Mode = static_cast(M);
-else
-  Diag(diag::err_drv_unsupported_option_argument) << OptName << Value;
-  }
+  if (M != ~0U)
+Mode = static_cast(M);
+  el

Re: r276900 - [Sema] Teach getCurrentThisType to reconize lambda in in-class initializer

[Richard Smith via cfe-commits]

On Thu, Aug 11, 2016 at 1:38 PM, Faisal Vali  wrote:

> On Thu, Aug 11, 2016 at 1:07 PM, Richard Smith 
> wrote:
> > Yes, this should be merged. It's not quite right, but it's certainly
> better
> > than what we had before.
>
> I would agree  that the comment is broken - but, unless i'm missing
> something subtle, the subsequent logic and call to
> adjustCVQualifiersForCXXThisWithinLambda should handle the scenario
> you describe.


Ah, thanks, I agree. Looks like it's just the comment that's slightly
misleading.


> >
> > On Mon, Aug 8, 2016 at 1:34 PM, Hans Wennborg  wrote:
> >>
> >> Richard: ping?
> >>
> >> On Wed, Jul 27, 2016 at 4:46 PM, Hans Wennborg 
> wrote:
> >> > Should this be merged to 3.9?
> >> >
> >> > Thanks,
> >> > Hans
> >> >
> >> > On Wed, Jul 27, 2016 at 11:25 AM, Erik Pilkington via cfe-commits
> >> >  wrote:
> >> >> Author: epilk
> >> >> Date: Wed Jul 27 13:25:10 2016
> >> >> New Revision: 276900
> >> >>
> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=276900&view=rev
> >> >> Log:
> >> >> [Sema] Teach getCurrentThisType to reconize lambda in in-class
> >> >> initializer
> >> >>
> >> >> Fixes PR27994, a crash on valid.
> >> >>
> >> >> Differential revision: https://reviews.llvm.org/D21145
> >> >>
> >> >> Modified:
> >> >> cfe/trunk/lib/Sema/SemaExprCXX.cpp
> >> >> cfe/trunk/test/SemaCXX/lambda-expressions.cpp
> >> >>
> >> >> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
> >> >> URL:
> >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaExprCXX.cpp?rev=276900&r1=276899&r2=276900&view=diff
> >> >>
> >> >> 
> ==
> >> >> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
> >> >> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Jul 27 13:25:10 2016
> >> >> @@ -961,32 +961,26 @@ static QualType adjustCVQualifiersForCXX
> >> >>  QualType Sema::getCurrentThisType() {
> >> >>DeclContext *DC = getFunctionLevelDeclContext();
> >> >>QualType ThisTy = CXXThisTypeOverride;
> >> >> +
> >> >>if (CXXMethodDecl *method = dyn_cast(DC)) {
> >> >>  if (method && method->isInstance())
> >> >>ThisTy = method->getThisType(Context);
> >> >>}
> >> >> -  if (ThisTy.isNull()) {
> >> >> -if (isGenericLambdaCallOperatorSpecialization(CurContext) &&
> >> >> -CurContext->getParent()->getParent()->isRecord()) {
> >> >> -  // This is a generic lambda call operator that is being
> >> >> instantiated
> >> >> -  // within a default initializer - so use the enclosing class
> as
> >> >> 'this'.
> >> >> -  // There is no enclosing member function to retrieve the
> 'this'
> >> >> pointer
> >> >> -  // from.
> >> >> -
> >> >> -  // FIXME: This looks wrong. If we're in a lambda within a
> lambda
> >> >> within a
> >> >> -  // default member initializer, we need to recurse up more
> >> >> parents to find
> >> >> -  // the right context. Looks like we should be walking up to
> the
> >> >> parent of
> >> >> -  // the closure type, checking whether that is itself a lambda,
> >> >> and if so,
> >> >> -  // recursing, until we reach a class or a function that isn't
> a
> >> >> lambda
> >> >> -  // call operator. And we should accumulate the constness of
> >> >> *this on the
> >> >> -  // way.
> >> >> -
> >> >> -  QualType ClassTy = Context.getTypeDeclType(
> >> >> -  cast(CurContext->getParent()->
> getParent()));
> >> >> -  // There are no cv-qualifiers for 'this' within default
> >> >> initializers,
> >> >> -  // per [expr.prim.general]p4.
> >> >> -  ThisTy = Context.getPointerType(ClassTy);
> >> >> -}
> >> >> +
> >> >> +  if (ThisTy.isNull() && isLambdaCallOperator(CurContext) &&
> >> >> +  !ActiveTemplateInstantiations.empty()) {
> >> >> +
> >> >> +assert(isa(DC) &&
> >> >> +   "Trying to get 'this' type from static method?");
> >> >> +
> >> >> +// This is a lambda call operator that is being instantiated as
> a
> >> >> default
> >> >> +// initializer. DC must point to the enclosing class type, so we
> >> >> can recover
> >> >> +// the 'this' type from it.
> >> >> +
> >> >> +QualType ClassTy =
> >> >> Context.getTypeDeclType(cast(DC));
> >> >> +// There are no cv-qualifiers for 'this' within default
> >> >> initializers,
> >> >> +// per [expr.prim.general]p4.
> >
> >
> > This is wrong in C++17 onwards: if *this is captured by value by a
> > non-mutable lambda, it should become const-qualified.
> >
> >>
> >> >> +ThisTy = Context.getPointerType(ClassTy);
> >> >>}
> >> >>
> >> >>// If we are within a lambda's call operator, the cv-qualifiers of
> >> >> 'this'
> >> >>
> >> >> Modified: cfe/trunk/test/SemaCXX/lambda-expressions.cpp
> >> >> URL:
> >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> SemaCXX/lambda-expressions.cpp?rev=276900&r1=276899&r2=276900&view=diff
> >> >>
> >> >> 
> ==
> >> >> 

[PATCH] D23429: [CUDA] Place GPU binary into .nv_fatbin section and align it by 8.

[Artem Belevich via cfe-commits]

tra created this revision.
tra added a reviewer: jlebar.
tra added a subscriber: cfe-commits.

This matches the way nvcc encapsulates GPU binaries into host object file.
Now cuobjdump can deal with clang-compiled object files.



https://reviews.llvm.org/D23429

Files:
  lib/CodeGen/CGCUDANV.cpp
  test/CodeGenCUDA/device-stub.cu

Index: test/CodeGenCUDA/device-stub.cu
===
--- test/CodeGenCUDA/device-stub.cu
+++ test/CodeGenCUDA/device-stub.cu
@@ -45,10 +45,12 @@
 // * constant unnamed string with the kernel name
 // CHECK: private unnamed_addr constant{{.*}}kernelfunc{{.*}}\00"
 // * constant unnamed string with GPU binary
-// CHECK: private unnamed_addr constant{{.*}}\00"
+// CHECK: private unnamed_addr constant{{.*GPU binary would be here.*}}\00"
+// CHECK-SAME: section ".nv_fatbin", align 8
 // * constant struct that wraps GPU binary
 // CHECK: @__cuda_fatbin_wrapper = internal constant { i32, i32, i8*, i8* } 
-// CHECK:   { i32 1180844977, i32 1, {{.*}}, i8* null }
+// CHECK-SAME: { i32 1180844977, i32 1, {{.*}}, i8* null }
+// CHECK-SAME: section ".nvFatBinSegment"
 // * variable to save GPU binary handle after initialization
 // CHECK: @__cuda_gpubin_handle = internal global i8** null
 // * Make sure our constructor/destructor was added to global ctor/dtor list.
Index: lib/CodeGen/CGCUDANV.cpp
===
--- lib/CodeGen/CGCUDANV.cpp
+++ lib/CodeGen/CGCUDANV.cpp
@@ -53,12 +53,20 @@
   /// Helper function that generates a constant string and returns a pointer to
   /// the start of the string.  The result of this function can be used 
anywhere
   /// where the C code specifies const char*.
-  llvm::Constant *makeConstantString(const std::string &Str,
- const std::string &Name = "",
+  llvm::Constant *makeConstantString(const StringRef Str,
+ const StringRef Name = "",
+ const StringRef SectionName = "",
  unsigned Alignment = 0) {
 llvm::Constant *Zeros[] = {llvm::ConstantInt::get(SizeTy, 0),
llvm::ConstantInt::get(SizeTy, 0)};
-auto ConstStr = CGM.GetAddrOfConstantCString(Str, Name.c_str());
+auto ConstStr = CGM.GetAddrOfConstantCString(Str, Name.data());
+llvm::GlobalVariable *GV =
+cast(ConstStr.getPointer());
+if (!SectionName.empty())
+  GV->setSection(SectionName);
+if (Alignment)
+  GV->setAlignment(Alignment);
+
 return llvm::ConstantExpr::getGetElementPtr(ConstStr.getElementType(),
 ConstStr.getPointer(), Zeros);
  }
@@ -285,7 +293,8 @@
 llvm::Constant *Values[] = {
 llvm::ConstantInt::get(IntTy, 0x466243b1), // Fatbin wrapper magic.
 llvm::ConstantInt::get(IntTy, 1),  // Fatbin version.
-makeConstantString(GpuBinaryOrErr.get()->getBuffer(), "", 16), // Data.
+makeConstantString(GpuBinaryOrErr.get()->getBuffer(), // Data.
+   "", ".nv_fatbin", 8),  //
 llvm::ConstantPointerNull::get(VoidPtrTy)}; // Unused in fatbin v1.
 llvm::GlobalVariable *FatbinWrapper = new llvm::GlobalVariable(
 TheModule, FatbinWrapperTy, true, llvm::GlobalValue::InternalLinkage,


Index: test/CodeGenCUDA/device-stub.cu
===
--- test/CodeGenCUDA/device-stub.cu
+++ test/CodeGenCUDA/device-stub.cu
@@ -45,10 +45,12 @@
 // * constant unnamed string with the kernel name
 // CHECK: private unnamed_addr constant{{.*}}kernelfunc{{.*}}\00"
 // * constant unnamed string with GPU binary
-// CHECK: private unnamed_addr constant{{.*}}\00"
+// CHECK: private unnamed_addr constant{{.*GPU binary would be here.*}}\00"
+// CHECK-SAME: section ".nv_fatbin", align 8
 // * constant struct that wraps GPU binary
 // CHECK: @__cuda_fatbin_wrapper = internal constant { i32, i32, i8*, i8* } 
-// CHECK:   { i32 1180844977, i32 1, {{.*}}, i8* null }
+// CHECK-SAME: { i32 1180844977, i32 1, {{.*}}, i8* null }
+// CHECK-SAME: section ".nvFatBinSegment"
 // * variable to save GPU binary handle after initialization
 // CHECK: @__cuda_gpubin_handle = internal global i8** null
 // * Make sure our constructor/destructor was added to global ctor/dtor list.
Index: lib/CodeGen/CGCUDANV.cpp
===
--- lib/CodeGen/CGCUDANV.cpp
+++ lib/CodeGen/CGCUDANV.cpp
@@ -53,12 +53,20 @@
   /// Helper function that generates a constant string and returns a pointer to
   /// the start of the string.  The result of this function can be used anywhere
   /// where the C code specifies const char*.
-  llvm::Constant *makeConstantString(const std::string &Str,
- const std::string &Name = "",
+  llvm::Constant *makeConstantString(const

r278435 - P0217R3: Perform semantic checks and initialization for the bindings in a

[Richard Smith via cfe-commits]

Author: rsmith
Date: Thu Aug 11 17:25:46 2016
New Revision: 278435

URL: http://llvm.org/viewvc/llvm-project?rev=278435&view=rev
Log:
P0217R3: Perform semantic checks and initialization for the bindings in a
decomposition declaration for arrays, aggregate-like structs, tuple-like
types, and (as an extension) for complex and vector types.

Added:
cfe/trunk/test/CXX/dcl.decl/dcl.decomp/
cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p2.cpp
cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p3.cpp
cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p4.cpp
Modified:
cfe/trunk/include/clang/AST/CXXInheritance.h
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/include/clang/AST/UnresolvedSet.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Initialization.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/ASTDumper.cpp
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/AST/ExprClassification.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprMember.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/Parser/cxx1z-decomposition.cpp
cfe/trunk/test/SemaCXX/cxx1z-decomposition.cpp

Modified: cfe/trunk/include/clang/AST/CXXInheritance.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CXXInheritance.h?rev=278435&r1=278434&r2=278435&view=diff
==
--- cfe/trunk/include/clang/AST/CXXInheritance.h (original)
+++ cfe/trunk/include/clang/AST/CXXInheritance.h Thu Aug 11 17:25:46 2016
@@ -172,7 +172,7 @@ public:
   /// paths for a derived-to-base search.
   explicit CXXBasePaths(bool FindAmbiguities = true, bool RecordPaths = true,
 bool DetectVirtual = true)
-  : FindAmbiguities(FindAmbiguities), RecordPaths(RecordPaths),
+  : Origin(), FindAmbiguities(FindAmbiguities), RecordPaths(RecordPaths),
 DetectVirtual(DetectVirtual), DetectedVirtual(nullptr),
 NumDeclsFound(0) {}
 

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=278435&r1=278434&r2=278435&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Thu Aug 11 17:25:46 2016
@@ -251,7 +251,7 @@ public:
   // FIXME: Deprecated, move clients to getName().
   std::string getNameAsString() const { return Name.getAsString(); }
 
-  void printName(raw_ostream &os) const { os << Name; }
+  virtual void printName(raw_ostream &os) const;
 
   /// getDeclName - Get the actual, stored name of the declaration,
   /// which may be a special name.
@@ -1025,7 +1025,7 @@ public:
   ///   void foo() { int x; static int y; extern int z; }
   ///
   bool isLocalVarDecl() const {
-if (getKind() != Decl::Var)
+if (getKind() != Decl::Var && getKind() != Decl::Decomposition)
   return false;
 if (const DeclContext *DC = getLexicalDeclContext())
   return DC->getRedeclContext()->isFunctionOrMethod();
@@ -1040,7 +1040,7 @@ public:
   /// isFunctionOrMethodVarDecl - Similar to isLocalVarDecl, but
   /// excludes variables declared in blocks.
   bool isFunctionOrMethodVarDecl() const {
-if (getKind() != Decl::Var)
+if (getKind() != Decl::Var && getKind() != Decl::Decomposition)
   return false;
 const DeclContext *DC = getLexicalDeclContext()->getRedeclContext();
 return DC->isFunctionOrMethod() && DC->getDeclKind() != Decl::Block;

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=278435&r1=278434&r2=278435&view=diff
==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Thu Aug 11 17:25:46 2016
@@ -1147,6 +1147,12 @@ public:
   /// \note This does NOT include a check for union-ness.
   bool isEmpty() const { return data().Empty; }
 
+  /// \brief Determine whether this class has direct non-static data members.
+  bool hasDirectFields() const {
+auto &D = data();
+return D.HasPublicFields || D.HasProtectedFields || D.HasPrivateFields;
+  }
+
   /// Whether this class is polymorphic (C++ [class.virtual]),
   /// which means that the class contains or inherits a virtual function.
   bool isPolymorphic() const { return data().Polymorphic; }
@@ -3451,6 +3457,8 @@ public:
 return llvm::makeArrayRef(getTrailingObjects(), 
NumBindings);
   }
 
+  void printName(raw_ostream &os) const override;
+
   static bool cl

Re: [PATCH] D23341: [CUDA] Include CUDA headers before anything else.

[Artem Belevich via cfe-commits]

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

OK. This should help with PR26966.


https://reviews.llvm.org/D23341



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

Re: [PATCH] D23433: [Basic] Add missing `const` qualifier (NFC)

[Vedant Kumar via cfe-commits]

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

Makes sense. isMacroID and isInSystemHeader are both already marked const.


https://reviews.llvm.org/D23433



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

Re: [PATCH] D23429: [CUDA] Place GPU binary into .nv_fatbin section and align it by 8.

[Justin Lebar via cfe-commits]

jlebar added inline comments.


Comment at: lib/CodeGen/CGCUDANV.cpp:62
@@ -60,2 +61,3 @@
llvm::ConstantInt::get(SizeTy, 0)};
-auto ConstStr = CGM.GetAddrOfConstantCString(Str, Name.c_str());
+auto ConstStr = CGM.GetAddrOfConstantCString(Str, Name.data());
+llvm::GlobalVariable *GV =

StringRefs aren't necessarily null-terminated.  :)


https://reviews.llvm.org/D23429



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

[PATCH] D23433: [Basic] Add missing `const` qualifier (NFC)

[Visoiu Mistrih Francis via cfe-commits]

thegameg created this revision.
thegameg added reviewers: joker-eph, vsk, dblaikie.
thegameg added a subscriber: cfe-commits.

The member function is a predicate, and doesn't apply any changes on the object.

https://reviews.llvm.org/D23433

Files:
  include/clang/Basic/SourceManager.h

Index: include/clang/Basic/SourceManager.h
===
--- include/clang/Basic/SourceManager.h
+++ include/clang/Basic/SourceManager.h
@@ -1357,7 +1357,7 @@
   }
 
   /// \brief Returns whether \p Loc is expanded from a macro in a system 
header.
-  bool isInSystemMacro(SourceLocation loc) {
+  bool isInSystemMacro(SourceLocation loc) const {
 return loc.isMacroID() && isInSystemHeader(getSpellingLoc(loc));
   }
 


Index: include/clang/Basic/SourceManager.h
===
--- include/clang/Basic/SourceManager.h
+++ include/clang/Basic/SourceManager.h
@@ -1357,7 +1357,7 @@
   }
 
   /// \brief Returns whether \p Loc is expanded from a macro in a system header.
-  bool isInSystemMacro(SourceLocation loc) {
+  bool isInSystemMacro(SourceLocation loc) const {
 return loc.isMacroID() && isInSystemHeader(getSpellingLoc(loc));
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23385: Implement __has_constant_initializer(obj) expression traits.

[Eric Fiselier via cfe-commits]

EricWF updated this revision to Diff 67768.
EricWF added a comment.

Re-flow comments because I suck at English.


https://reviews.llvm.org/D23385

Files:
  include/clang/AST/Expr.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/ExpressionTraits.h
  include/clang/Basic/TokenKinds.def
  lib/AST/Expr.cpp
  lib/AST/StmtPrinter.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/expression-traits.cpp

Index: test/SemaCXX/expression-traits.cpp
===
--- test/SemaCXX/expression-traits.cpp
+++ test/SemaCXX/expression-traits.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -fcxx-exceptions %s
-
+// RUN: %clang_cc1 -fsyntax-only -verify -fcxx-exceptions -std=c++14 %s
 //
 // Tests for "expression traits" intrinsics such as __is_lvalue_expr.
 //
@@ -617,3 +617,219 @@
 {
 check_temp_param_6<3,AnInt>();
 }
+
+//===//
+// __has_constant_initializer
+//===//
+struct PODType {
+int value;
+int value2;
+};
+#if __cplusplus >= 201103L
+struct LitType {
+constexpr LitType() : value(0) {}
+constexpr LitType(int x) : value(x) {}
+LitType(void*) : value(-1) {}
+int value;
+};
+#endif
+
+struct NonLit {
+#if __cplusplus >= 201402L
+constexpr NonLit() : value(0) {}
+constexpr NonLit(int x ) : value(x) {}
+#else
+NonLit() : value(0) {}
+NonLit(int x ) : value(x) {}
+#endif
+NonLit(void*) : value(-1) {}
+~NonLit() {}
+int value;
+};
+
+struct StoresNonLit {
+#if __cplusplus >= 201402L
+constexpr StoresNonLit() : obj() {}
+constexpr StoresNonLit(int x) : obj(x) {}
+#else
+StoresNonLit() : obj() {}
+StoresNonLit(int x) : obj(x) {}
+#endif
+StoresNonLit(void* p) : obj(p) {}
+NonLit obj;
+};
+
+const bool NonLitHasConstInit =
+#if __cplusplus >= 201402L
+true;
+#else
+false;
+#endif
+
+// Test diagnostics when the argument does not refer to a named identifier
+void check_is_constant_init_bogus()
+{
+(void)__has_constant_initializer(42); // expected-error {{does not reference a named variable}}
+(void)__has_constant_initializer(ReturnInt()); // expected-error {{does not reference a named variable}}
+(void)__has_constant_initializer(42, 43); // expected-error {{does not reference a named variable}}
+}
+
+// [basic.start.static]p2.1
+// if each full-expression (including implicit conversions) that appears in
+// the initializer of a reference with static or thread storage duration is
+// a constant expression (5.20) and the reference is bound to a glvalue
+// designating an object with static storage duration, to a temporary object
+// (see 12.2) or subobject thereof, or to a function;
+
+// Test binding to a static glvalue
+const int glvalue_int = 42;
+const int glvalue_int2 = ReturnInt();
+const int& glvalue_ref = glvalue_int;
+const int& glvalue_ref2 = glvalue_int2;
+static_assert(__has_constant_initializer(glvalue_ref), "");
+static_assert(__has_constant_initializer(glvalue_ref2), "");
+
+__thread const int& glvalue_ref_tl = glvalue_int;
+static_assert(__has_constant_initializer(glvalue_ref_tl), "");
+
+void test_basic_start_static_2_1() {
+const int non_global = 42;
+const int& non_global_ref = non_global;
+static_assert(!__has_constant_initializer(non_global), "automatic variables never have const init");
+static_assert(!__has_constant_initializer(non_global_ref), "automatic variables never have const init");
+
+static const int& local_init = non_global;
+static_assert(!__has_constant_initializer(local_init), "init must be static glvalue");
+static const int& global_init = glvalue_int;
+static_assert(__has_constant_initializer(global_init), "");
+static const int& temp_init = 42;
+static_assert(__has_constant_initializer(temp_init), "");
+#if 0
+/// FIXME: Why is this failing?
+__thread const int& tl_init = 42;
+static_assert(__has_constant_initializer(tl_init), "");
+#endif
+}
+
+const int& temp_ref = 42;
+const int& temp_ref2 = ReturnInt();
+static_assert(__has_constant_initializer(temp_ref), "");
+static_assert(!__has_constant_initializer(temp_ref2), "");
+
+const NonLit& nl_temp_ref = 42;
+static_assert(!__has_constant_initializer(nl_temp_ref), "");
+
+#if __cplusplus >= 201103L
+const LitType& lit_temp_ref = 42;
+static_assert(__has_constant_initializer(lit_temp_ref), "");
+
+const int& subobj_ref = LitType{}.value;
+static_assert(__has_constant_initializer(subobj_ref), "");
+#endif
+
+const int& nl_subobj_ref = NonLit().value;
+static_assert(!__has_constant_initializer(nl_subobj_ref), "");
+
+// [basic.start.static]p2.2
+// if an object with static or thread storage duration is initialized by a
+// constructor call, and if the initialization full-expression is a con

Re: [PATCH] D23385: Implement __has_constant_initializer(obj) expression traits.

[Jonathan Roelofs via cfe-commits]

jroelofs added a subscriber: jroelofs.


Comment at: lib/AST/Expr.cpp:2656
@@ +2655,3 @@
+AllowNonLiteral)) {
+if (!CE->getNumArgs()) return true;
+unsigned numArgs = CE->getNumArgs();

no need for this `if`.

Also, I think the `for` should be written:

```
for (auto *Arg : CE->arguments())
  if (Arg->isConstantInitializer(Ctx, false, Culprit))
return false;
```


Comment at: lib/Sema/SemaExprCXX.cpp:4775
@@ +4774,3 @@
+  // a 'constant initializer'.
+  else if ((VD->hasGlobalStorage() ||
+  VD->getTLSKind() != VarDecl::TLS_None) && VD->hasInit()) {

no else after return.

Also, what do you want this to do for `ParmVarDecl`s that happen to have an 
initializer? i.e:

```
void foo(int i = 45) {}
```


https://reviews.llvm.org/D23385



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

Re: [PATCH] D23385: Implement __has_constant_initializer(obj) expression traits.

[Jonathan Roelofs via cfe-commits]

jroelofs added a comment.

One more thing: need to add docs to `clang/docs/LanguageExtensions.rst`.


https://reviews.llvm.org/D23385



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

[PATCH] D23434: Don't allow llvm-include-order to intermingle includes from different files.

[Zachary Turner via cfe-commits]

zturner created this revision.
zturner added reviewers: alexfh, djasper.
zturner added a subscriber: cfe-commits.

See llvm.org/pr28943 for some context.

I'm not sure if this is the right fix, but it seems to pass the test suite for 
me.

https://reviews.llvm.org/D23434

Files:
  clang-tidy/llvm/IncludeOrderCheck.cpp
  test/clang-tidy/Inputs/Headers/cross-file-a.h
  test/clang-tidy/Inputs/Headers/cross-file-b.h
  test/clang-tidy/Inputs/Headers/cross-file-c.h
  test/clang-tidy/llvm-include-order.cpp

Index: test/clang-tidy/llvm-include-order.cpp
===
--- test/clang-tidy/llvm-include-order.cpp
+++ test/clang-tidy/llvm-include-order.cpp
@@ -35,3 +35,8 @@
 
 // CHECK-FIXES: #include "a.h"
 // CHECK-FIXES-NEXT: #include "b.h"
+
+// CHECK-MESSAGES-NOT: [[@LINE+1]]:1: warning: #includes are not sorted 
properly
+#include "cross-file-a.h"
+
+#include "cross-file-c.h"
Index: test/clang-tidy/Inputs/Headers/cross-file-c.h
===
--- /dev/null
+++ test/clang-tidy/Inputs/Headers/cross-file-c.h
@@ -0,0 +1,2 @@
+
+#include "cross-file-b.h"
Index: clang-tidy/llvm/IncludeOrderCheck.cpp
===
--- clang-tidy/llvm/IncludeOrderCheck.cpp
+++ clang-tidy/llvm/IncludeOrderCheck.cpp
@@ -73,6 +73,9 @@
 SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
 bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File,
 StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+  if (SM.getFileID(HashLoc) != SM.getMainFileID())
+return;
+
   // We recognize the first include as a special main module header and want
   // to leave it in the top position.
   IncludeDirective ID = {HashLoc, FilenameRange, FileName, IsAngled, false};


Index: test/clang-tidy/llvm-include-order.cpp
===
--- test/clang-tidy/llvm-include-order.cpp
+++ test/clang-tidy/llvm-include-order.cpp
@@ -35,3 +35,8 @@
 
 // CHECK-FIXES: #include "a.h"
 // CHECK-FIXES-NEXT: #include "b.h"
+
+// CHECK-MESSAGES-NOT: [[@LINE+1]]:1: warning: #includes are not sorted properly
+#include "cross-file-a.h"
+
+#include "cross-file-c.h"
Index: test/clang-tidy/Inputs/Headers/cross-file-c.h
===
--- /dev/null
+++ test/clang-tidy/Inputs/Headers/cross-file-c.h
@@ -0,0 +1,2 @@
+
+#include "cross-file-b.h"
Index: clang-tidy/llvm/IncludeOrderCheck.cpp
===
--- clang-tidy/llvm/IncludeOrderCheck.cpp
+++ clang-tidy/llvm/IncludeOrderCheck.cpp
@@ -73,6 +73,9 @@
 SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
 bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File,
 StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+  if (SM.getFileID(HashLoc) != SM.getMainFileID())
+return;
+
   // We recognize the first include as a special main module header and want
   // to leave it in the top position.
   IncludeDirective ID = {HashLoc, FilenameRange, FileName, IsAngled, false};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23420: libcxx: Fix libcxx test on aarch64 with libunwind

[Adhemerval Zanella via cfe-commits]

zatrazz added a comment.

I tried to find why exactly libgcc has to appear before and after but I 
couldn't get any information from commit history. On my system (aarch64/linux) 
I am also getting "-lc++" "-lm" "-lgcc_s" "-lgcc" "-lpthread" "-lc" "-lgcc_s" 
"-lgcc".

I think we can got by the safe side and just add the final libgcc at the end 
for the soft-fp symbols.


https://reviews.llvm.org/D23420



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

Re: [PATCH] D23385: Implement __has_constant_initializer(obj) expression traits.

[Eric Fiselier via cfe-commits]

EricWF marked an inline comment as done.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6807
@@ +6806,3 @@
+
+def err_has_constant_init_expression_trait_invalid_arg : Error<
+  "expression does not reference a named variable">;

Help improving this wording would be appreciated.


Comment at: lib/Sema/SemaExprCXX.cpp:4775
@@ +4774,3 @@
+  // a 'constant initializer'.
+  else if ((VD->hasGlobalStorage() ||
+  VD->getTLSKind() != VarDecl::TLS_None) && VD->hasInit()) {

jroelofs wrote:
> no else after return.
> 
> Also, what do you want this to do for `ParmVarDecl`s that happen to have an 
> initializer? i.e:
> 
> ```
> void foo(int i = 45) {}
> ```
I believe that case should return false. `i` isn't static or thread local so it 
doesn't have a "constant initializer" according to [basic.start.static].



https://reviews.llvm.org/D23385



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

Re: [PATCH] D23434: Don't allow llvm-include-order to intermingle includes from different files.

[Zachary Turner via cfe-commits]

zturner updated this revision to Diff 67772.
zturner added a comment.

Fixed the test for this.  Confirmed that it fails before the patch and succeeds 
after the patch.


https://reviews.llvm.org/D23434

Files:
  clang-tidy/llvm/IncludeOrderCheck.cpp
  test/clang-tidy/Inputs/Headers/cross-file-a.h
  test/clang-tidy/Inputs/Headers/cross-file-b.h
  test/clang-tidy/Inputs/Headers/cross-file-c.h
  test/clang-tidy/llvm-include-order.cpp

Index: test/clang-tidy/llvm-include-order.cpp
===
--- test/clang-tidy/llvm-include-order.cpp
+++ test/clang-tidy/llvm-include-order.cpp
@@ -35,3 +35,8 @@
 
 // CHECK-FIXES: #include "a.h"
 // CHECK-FIXES-NEXT: #include "b.h"
+
+// CHECK-MESSAGES-NOT: [[@LINE+1]]:1: warning: #includes are not sorted 
properly
+#include "cross-file-c.h"
+
+#include "cross-file-a.h"
Index: test/clang-tidy/Inputs/Headers/cross-file-c.h
===
--- /dev/null
+++ test/clang-tidy/Inputs/Headers/cross-file-c.h
@@ -0,0 +1,41 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+#include "cross-file-b.h"
Index: clang-tidy/llvm/IncludeOrderCheck.cpp
===
--- clang-tidy/llvm/IncludeOrderCheck.cpp
+++ clang-tidy/llvm/IncludeOrderCheck.cpp
@@ -73,6 +73,9 @@
 SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
 bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File,
 StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+  if (SM.getFileID(HashLoc) != SM.getMainFileID())
+return;
+
   // We recognize the first include as a special main module header and want
   // to leave it in the top position.
   IncludeDirective ID = {HashLoc, FilenameRange, FileName, IsAngled, false};


Index: test/clang-tidy/llvm-include-order.cpp
===
--- test/clang-tidy/llvm-include-order.cpp
+++ test/clang-tidy/llvm-include-order.cpp
@@ -35,3 +35,8 @@
 
 // CHECK-FIXES: #include "a.h"
 // CHECK-FIXES-NEXT: #include "b.h"
+
+// CHECK-MESSAGES-NOT: [[@LINE+1]]:1: warning: #includes are not sorted properly
+#include "cross-file-c.h"
+
+#include "cross-file-a.h"
Index: test/clang-tidy/Inputs/Headers/cross-file-c.h
===
--- /dev/null
+++ test/clang-tidy/Inputs/Headers/cross-file-c.h
@@ -0,0 +1,41 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+#include "cross-file-b.h"
Index: clang-tidy/llvm/IncludeOrderCheck.cpp
===
--- clang-tidy/llvm/IncludeOrderCheck.cpp
+++ clang-tidy/llvm/IncludeOrderCheck.cpp
@@ -73,6 +73,9 @@
 SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
 bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File,
 StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+  if (SM.getFileID(HashLoc) != SM.getMainFileID())
+return;
+
   // We recognize the first include as a special main module header and want
   // to leave it in the top position.
   IncludeDirective ID = {HashLoc, FilenameRange, FileName, IsAngled, false};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23387: [Analyzer] Report found fields order in PaddingChecker.

[Alexander Shaposhnikov via cfe-commits]

alexshap updated this revision to Diff 67773.
alexshap added a comment.

Use more stable sort (preserving the original order if possible).
Change the format of the report.
Update the tests.
I have not turned it to "note:" or "remark:" because in this case 
it's not included into the description and it's not being displayed by 
scan-build
(not very convenient).


https://reviews.llvm.org/D23387

Files:
  lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
  test/Analysis/padding_message.cpp

Index: test/Analysis/padding_message.cpp
===
--- test/Analysis/padding_message.cpp
+++ test/Analysis/padding_message.cpp
@@ -1,13 +1,25 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux -std=c++14 -analyze -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=2 -verify %s
 
-// expected-warning@+1{{Excessive padding in 'struct IntSandwich' (6 padding bytes, where 2 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'struct IntSandwich' (6 padding bytes, where 2 is optimal).\
+Optimal fields order:\
+int i \
+char c1 \
+char c2 \
+}}
 struct IntSandwich {
   char c1;
   int i;
   char c2;
 };
 
-// expected-warning@+1{{Excessive padding in 'struct TurDuckHen' (6 padding bytes, where 2 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'struct TurDuckHen' (6 padding bytes, where 2 is optimal).\
+Optimal fields order:\
+struct IntSandwich i \
+char c1 \
+char c2 \
+}}
 struct TurDuckHen {
   char c1;
   struct IntSandwich i;
@@ -16,7 +28,17 @@
 
 #pragma pack(push)
 #pragma pack(2)
-// expected-warning@+1{{Excessive padding in 'struct SmallIntSandwich' (4 padding bytes, where 0 is optimal)}}
+// expected-warning@+11{{\
+Excessive padding in 'struct SmallIntSandwich' (4 padding bytes, where 0 is optimal).\
+Optimal fields order:\
+int i1 \
+int i2 \
+int i3 \
+char c1 \
+char c2 \
+char c3 \
+char c4 \
+}}
 struct SmallIntSandwich {
   char c1;
   int i1;
@@ -34,7 +56,13 @@
   int i;
 };
 
-// expected-warning@+1{{Excessive padding in 'struct HoldsAUnion' (6 padding bytes, where 2 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'struct HoldsAUnion' (6 padding bytes, where 2 is optimal).\
+Optimal fields order:\
+union SomeUnion u \
+char c1 \
+char c2 \
+}}
 struct HoldsAUnion {
   char c1;
   union SomeUnion u;
@@ -49,28 +77,53 @@
   int i[5];
 };
 
-// expected-warning@+1{{Excessive padding in 'struct StructSandwich' (6 padding bytes, where 2 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'struct StructSandwich' (6 padding bytes, where 2 is optimal).\
+Optimal fields order:\
+struct MediumIntArray m \
+struct SmallCharArray s \
+struct SmallCharArray s2 \
+}}
 struct StructSandwich {
   struct SmallCharArray s;
   struct MediumIntArray m;
   struct SmallCharArray s2;
 };
 
-// expected-warning@+1{{Excessive padding in 'TypedefSandwich' (6 padding bytes, where 2 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'TypedefSandwich' (6 padding bytes, where 2 is optimal).\
+Optimal fields order:\
+int i \
+char c1 \
+char c2 \
+}}
 typedef struct {
   char c1;
   int i;
   char c2;
 } TypedefSandwich;
 
-// expected-warning@+1{{Excessive padding in 'struct StructAttrAlign' (10 padding bytes, where 2 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'struct StructAttrAlign' (10 padding bytes, where 2 is optimal).\
+Optimal fields order:\
+int i \
+char c1 \
+char c2 \
+}}
 struct StructAttrAlign {
   char c1;
   int i;
   char c2;
 } __attribute__((aligned(8)));
 
-// expected-warning@+1{{Excessive padding in 'struct OverlyAlignedChar' (8185 padding bytes, where 4089 is optimal)}}
+// expected-warning@+8{{\
+Excessive padding in 'struct OverlyAlignedChar' (8185 padding bytes, where 4089 is optimal).\
+Optimal fields order:\
+char c \
+char c1 \
+char c2 \
+int x \
+}}
 struct OverlyAlignedChar {
   char c1;
   int x;
@@ -78,7 +131,13 @@
   char c __attribute__((aligned(4096)));
 };
 
-// expected-warning@+1{{Excessive padding in 'struct HoldsOverlyAlignedChar' (8190 padding bytes, where 4094 is optimal)}}
+// expected-warning@+7{{\
+Excessive padding in 'struct HoldsOverlyAlignedChar' (8190 padding bytes, where 4094 is optimal).\
+Optimal fields order:\
+struct OverlyAlignedChar o \
+char c1 \
+char c2 \
+}}
 struct HoldsOverlyAlignedChar {
   char c1;
   struct OverlyAlignedChar o;
@@ -86,7 +145,13 @@
 };
 
 void internalStructFunc() {
-  // expected-warning@+1{{Excessive padding in 'struct X' (6 padding bytes, where 2 is optimal)}}
+  // expected-warning@+7{{\
+Excessive padding in 'struct X' (6 padding bytes, where 2 is optimal).\
+Optimal fields order:\
+int t \
+char c1 \
+char c2 \
+}}
   struct X {
 char c1;
 int t;
@@ -96,7 +161,13 @@
 }
 
 void typedefStructFunc() {
-  // expected-warning@+1{{Excessive padding in 'S' (6 padding bytes, where 2 is optimal)}}
+  // expected-warning@+7{{\
+Excessive padding in 'S' (6 padding bytes, where 2 i

Re: [PATCH] D23385: Implement __has_constant_initializer(obj) expression traits.

[Eric Fiselier via cfe-commits]

EricWF added inline comments.


Comment at: lib/Sema/SemaExprCXX.cpp:4775
@@ +4774,3 @@
+  // a 'constant initializer'.
+  else if ((VD->hasGlobalStorage() ||
+  VD->getTLSKind() != VarDecl::TLS_None) && VD->hasInit()) {

EricWF wrote:
> jroelofs wrote:
> > no else after return.
> > 
> > Also, what do you want this to do for `ParmVarDecl`s that happen to have an 
> > initializer? i.e:
> > 
> > ```
> > void foo(int i = 45) {}
> > ```
> I believe that case should return false. `i` isn't static or thread local so 
> it doesn't have a "constant initializer" according to [basic.start.static].
> 
Perhaps using `__has_constant_initializer` should emit a diagnostic when used 
on non-static, non-thread-local objects?


https://reviews.llvm.org/D23385



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

Re: [PATCH] D23433: [Basic] Add missing `const` qualifier (NFC)

[Visoiu Mistrih Francis via cfe-commits]

thegameg added a comment.

Thanks @vsk. As I don't have commit access, is it possible to commit this for 
me, please? Thanks.


https://reviews.llvm.org/D23433



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