[PATCH] D26768: [analyzer] Improve VirtualCallChecker diagnostics and move out of alpha

2016-11-17 Thread Artem Dergachev via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

LGTM!




Comment at: test/Analysis/virtualcall.h:23
+#if INTERPROCEDURAL
+  // expected-warning-re@-2 ^}}Call Path : fooCall to virtual 
function during construction or destruction will not dispatch to derived class}}
+#else

Hmm, line break handling is quite strange.


https://reviews.llvm.org/D26768



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


[PATCH] D26588: Add LocationContext to members of check::RegionChanges

2016-11-17 Thread Artem Dergachev via cfe-commits
NoQ added inline comments.



Comment at: include/clang/StaticAnalyzer/Core/Checker.h:325
+  const CallEvent *Call,
+  const LocationContext *LCtx) {
+return ((const CHECKER *) checker)->checkRegionChanges(state, invalidated,

zaks.anna wrote:
> k-wisniewski wrote:
> > NoQ wrote:
> > > zaks.anna wrote:
> > > > LocationContext can be obtained by calling 
> > > > CallEvent::getLocationContext(). I do not think that adding another 
> > > > parameter here buys us much. Am I missing something?
> > > CallEvent* is optional here - it's there only for invalidations through 
> > > calls.
> > How about the situation when this callback is triggered by something other 
> > than a call, like variable assignment? I've tried using that location 
> > context and had lots of segfaults as in such cases it appeared to be null. 
> > Do you have some info that it should be non-null in such a case?
> Ok, makes sense. Have you looked into providing a checker context there? How 
> much more difficult would that be? If that is too difficult, adding 
> LocationContext is good as well.
> 
> If Call is optional and LocationContext is not, should the Call parameter be 
> last.
If we add a CheckerContext, then we may end up calling callbacks that split 
states within callbacks that split states, and that'd be quite a mess to 
support.

Right now a checker may trigger `checkRegionChanges()` within its callback by 
manipulating the Store, which already leads to callbacks within callbacks, but 
that's bearable as long as you can't add transitions within the inner callbacks.


https://reviews.llvm.org/D26588



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


[clang-tools-extra] r287215 - [clang-tidy] Changes to modernize-use-default check

2016-11-17 Thread Malcolm Parsons via cfe-commits
Author: malcolm.parsons
Date: Thu Nov 17 03:14:04 2016
New Revision: 287215

URL: http://llvm.org/viewvc/llvm-project?rev=287215&view=rev
Log:
[clang-tidy] Changes to modernize-use-default check

Summary:
Warn about special member functions that only contain a comment.
Report the location of the special member function, unless it is
defined in a macro.  Reporting the location of the body in a macro is
more helpful as it causes the macro expansion location to be reported too.

Fixes PR30920.

Reviewers: alexfh, aaron.ballman

Subscribers: cfe-commits

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

Modified:
clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-copy.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default.cpp

Modified: clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultCheck.cpp?rev=287215&r1=287214&r2=287215&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultCheck.cpp Thu Nov 17 
03:14:04 2016
@@ -249,11 +249,14 @@ void UseDefaultCheck::check(const MatchF
   if (!Body)
 return;
 
-  // If there are comments inside the body, don't do the change.
-  if (!SpecialFunctionDecl->isCopyAssignmentOperator() &&
-  !bodyEmpty(Result.Context, Body))
+  // If there is code inside the body, don't warn.
+  if (!SpecialFunctionDecl->isCopyAssignmentOperator() && !Body->body_empty())
 return;
 
+  // If there are comments inside the body, don't do the change.
+  bool ApplyFix = SpecialFunctionDecl->isCopyAssignmentOperator() ||
+  bodyEmpty(Result.Context, Body);
+
   std::vector RemoveInitializers;
 
   if (const auto *Ctor = dyn_cast(SpecialFunctionDecl)) {
@@ -277,10 +280,18 @@ void UseDefaultCheck::check(const MatchF
 SpecialFunctionName = "copy-assignment operator";
   }
 
-  diag(SpecialFunctionDecl->getLocStart(),
-   "use '= default' to define a trivial " + SpecialFunctionName)
-  << FixItHint::CreateReplacement(Body->getSourceRange(), "= default;")
-  << RemoveInitializers;
+  // The location of the body is more useful inside a macro as spelling and
+  // expansion locations are reported.
+  SourceLocation Location = SpecialFunctionDecl->getLocation();
+  if (Location.isMacroID())
+Location = Body->getLocStart();
+
+  auto Diag = diag(Location, "use '= default' to define a trivial " +
+ SpecialFunctionName);
+
+  if (ApplyFix)
+Diag << FixItHint::CreateReplacement(Body->getSourceRange(), "= default;")
+ << RemoveInitializers;
 }
 
 } // namespace modernize

Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-copy.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-copy.cpp?rev=287215&r1=287214&r2=287215&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-copy.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-copy.cpp Thu 
Nov 17 03:14:04 2016
@@ -7,13 +7,13 @@ struct OL {
   int Field;
 };
 OL::OL(const OL &Other) : Field(Other.Field) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a 
trivial copy constructor [modernize-use-default]
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default' to define a 
trivial copy constructor [modernize-use-default]
 // CHECK-FIXES: OL::OL(const OL &Other)  = default;
 OL &OL::operator=(const OL &Other) {
   Field = Other.Field;
   return *this;
 }
-// CHECK-MESSAGES: :[[@LINE-4]]:1: warning: use '= default' to define a 
trivial copy-assignment operator [modernize-use-default]
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: use '= default' to define a 
trivial copy-assignment operator [modernize-use-default]
 // CHECK-FIXES: OL &OL::operator=(const OL &Other) = default;
 
 // Inline.
@@ -25,7 +25,7 @@ struct IL {
 Field = Other.Field;
 return *this;
   }
-  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use '= default'
+  // CHECK-MESSAGES: :[[@LINE-4]]:7: warning: use '= default'
   // CHECK-FIXES: IL &operator=(const IL &Other) = default;
   int Field;
 };
@@ -110,7 +110,7 @@ struct Empty {
   Empty &operator=(const Empty &);
 };
 Empty &Empty::operator=(const Empty &Other) { return *this; }
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default'
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use '= default'
 // CHECK-FIXES: Empty &Empty::operator=(const Empty &Other) = default;
 
 // Bit fields.
@@ -137,7 +137,7 @@ BF &BF::operator=(const BF &Other) {
   Field4 = Other.Field4;
   return *this;
 }
-// CHECK-MESSAGES: :[[@LINE-7]]:1: warning: use

[PATCH] D26741: [clang-tidy] Changes to modernize-use-default check

2016-11-17 Thread Malcolm Parsons via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL287215: [clang-tidy] Changes to modernize-use-default check 
(authored by malcolm.parsons).

Changed prior to commit:
  https://reviews.llvm.org/D26741?vs=78164&id=78328#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26741

Files:
  clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-copy.cpp
  clang-tools-extra/trunk/test/clang-tidy/modernize-use-default.cpp

Index: clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultCheck.cpp
@@ -249,11 +249,14 @@
   if (!Body)
 return;
 
-  // If there are comments inside the body, don't do the change.
-  if (!SpecialFunctionDecl->isCopyAssignmentOperator() &&
-  !bodyEmpty(Result.Context, Body))
+  // If there is code inside the body, don't warn.
+  if (!SpecialFunctionDecl->isCopyAssignmentOperator() && !Body->body_empty())
 return;
 
+  // If there are comments inside the body, don't do the change.
+  bool ApplyFix = SpecialFunctionDecl->isCopyAssignmentOperator() ||
+  bodyEmpty(Result.Context, Body);
+
   std::vector RemoveInitializers;
 
   if (const auto *Ctor = dyn_cast(SpecialFunctionDecl)) {
@@ -277,10 +280,18 @@
 SpecialFunctionName = "copy-assignment operator";
   }
 
-  diag(SpecialFunctionDecl->getLocStart(),
-   "use '= default' to define a trivial " + SpecialFunctionName)
-  << FixItHint::CreateReplacement(Body->getSourceRange(), "= default;")
-  << RemoveInitializers;
+  // The location of the body is more useful inside a macro as spelling and
+  // expansion locations are reported.
+  SourceLocation Location = SpecialFunctionDecl->getLocation();
+  if (Location.isMacroID())
+Location = Body->getLocStart();
+
+  auto Diag = diag(Location, "use '= default' to define a trivial " +
+ SpecialFunctionName);
+
+  if (ApplyFix)
+Diag << FixItHint::CreateReplacement(Body->getSourceRange(), "= default;")
+ << RemoveInitializers;
 }
 
 } // namespace modernize
Index: clang-tools-extra/trunk/test/clang-tidy/modernize-use-default.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/modernize-use-default.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-default.cpp
@@ -8,10 +8,10 @@
 };
 
 OL::OL() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a trivial default constructor [modernize-use-default]
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default' to define a trivial default constructor [modernize-use-default]
 // CHECK-FIXES: OL::OL() = default;
 OL::~OL() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a trivial destructor [modernize-use-default]
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default' to define a trivial destructor [modernize-use-default]
 // CHECK-FIXES: OL::~OL() = default;
 
 // Inline definitions.
@@ -92,10 +92,10 @@
 class KW {
 public:
   explicit KW() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use '= default'
   // CHECK-FIXES: explicit KW() = default;
   virtual ~KW() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use '= default'
   // CHECK-FIXES: virtual ~KW() = default;
 };
 
@@ -134,11 +134,11 @@
 
 template 
 TempODef::TempODef() {}
-// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: use '= default'
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use '= default'
 // CHECK-FIXES: TempODef::TempODef() = default;
 template 
 TempODef::~TempODef() {}
-// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: use '= default'
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use '= default'
 // CHECK-FIXES: TempODef::~TempODef() = default;
 
 template class TempODef;
@@ -178,9 +178,11 @@
   Comments() {
 // Don't erase comments inside the body.
   }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use '= default'
   ~Comments() {
 // Don't erase comments inside the body.
   }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use '= default'
 };
 
 // Try-catch.
@@ -195,3 +197,13 @@
 };
 OTC::OTC() try {} catch(...) {}
 OTC::~OTC() try {} catch(...) {}
+
+#define STRUCT_WITH_DEFAULT(_base, _type) \
+  struct _type {  \
+_type() {}\
+_base value;  \
+  };
+
+STRUCT_WITH_DEFAULT(unsigned char, Hex8Default)
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a trivial default constructor
+// CHECK-MESSAGES: :[[@LINE-6]]:13: note:
Index: clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-copy.cpp

[PATCH] D26752: [include-fixer] Refactor include fixer to be usable as a plugin

2016-11-17 Thread Haojian Wu via cfe-commits
hokein added inline comments.



Comment at: include-fixer/IncludeFixer.cpp:64
   getIncludeFixerContext(const clang::SourceManager &SourceManager,
  clang::HeaderSearch &HeaderSearch) {
+return SemaSource.getIncludeFixerContext(SourceManager, HeaderSearch);

also make it const?



Comment at: include-fixer/IncludeFixer.cpp:124
+  auto Reps = createIncludeFixerReplacements(Code, Context,
+ format::getLLVMStyle(), false);
+  if (!Reps)

nit: `/*AddQualifiers=*/false`.



Comment at: include-fixer/IncludeFixer.cpp:136
+
+auto Begin = StartOfFile.getLocWithOffset(Placed.getOffset());
+auto End = Begin.getLocWithOffset(Placed.getLength());

I have a concern that `Placed` here might be not the replacement for inserting 
the new header, becuase the `Reps` returned from 
`createIncludeFixerReplacements` may have some replacements for cleanup.

To make it more correct, maybe we can check whether 
`Placed.getReplacementText()` is equal to `"#include" + 
Context.getHeaderInfos().front().Header`?



Comment at: include-fixer/IncludeFixer.cpp:283
+  if (GenerateDiagnostics) {
+FileID FID = CI->getSourceManager().getFileID(Typo.getLoc());
+StringRef Code = CI->getSourceManager().getBufferData(FID);

You can use `SM` directly here since you have created a `SM` variable for 
`SourceManager` above? The same below.



Comment at: include-fixer/IncludeFixer.h:119
+  IncludeFixerContext
+  getIncludeFixerContext(const clang::SourceManager &SourceManager,
+ clang::HeaderSearch &HeaderSearch);

Make it a `const` member function?



Comment at: include-fixer/IncludeFixer.h:124
+  /// Query the database for a given identifier.
+  bool query(StringRef Query, StringRef ScopedQualifiers, tooling::Range 
Range);
+  CompilerInstance *CI;

A blank line below.



Comment at: include-fixer/plugin/IncludeFixerPlugin.cpp:1
+//===- ClangTidyPlugin.cpp - clang-tidy as a clang plugin 
-===//
+//

nit: IncludeFixerPlugin



Comment at: include-fixer/plugin/IncludeFixerPlugin.cpp:91
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the clang-tidy plugin.
+volatile int ClangIncludeFixerPluginAnchorSource = 0;

nit: clang-tidy=>include-fixer


https://reviews.llvm.org/D26752



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


[PATCH] D26751: [clang-tidy] Ignore template instantiations in modernize-use-equals-delete check

2016-11-17 Thread Haojian Wu via cfe-commits
hokein accepted this revision.
hokein added a reviewer: hokein.
hokein added a comment.
This revision is now accepted and ready to land.

LGTM, thanks.




Comment at: clang-tidy/modernize/UseEqualsDeleteCheck.cpp:37
   unless(anyOf(hasBody(stmt()), isDefaulted(), isDeleted(),
+   ast_matchers::isTemplateInstantiation(),
// Ensure that all methods except private special member

Can we remove `ast_matchers::`?


https://reviews.llvm.org/D26751



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


[PATCH] D21298: [Clang-tidy] delete null check

2016-11-17 Thread Haojian Wu via cfe-commits
hokein added inline comments.



Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:3
+
+#include 
+

We don't rely on implementations of standard headers in lit test. You should 
fake the function/class that you need in this test. 



Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:7
+  int *p = 0;
+  if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; 
deleting null pointer has no effect [readability-delete-null-pointer]

Does it work the case like:

```
int *p = nullptr;
if (p == nullptr) {
   p = new int[3];
   delete[] p;
}
```

?


https://reviews.llvm.org/D21298



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


[PATCH] D26751: [clang-tidy] Ignore template instantiations in modernize-use-equals-delete check

2016-11-17 Thread Malcolm Parsons via cfe-commits
malcolm.parsons added inline comments.



Comment at: clang-tidy/modernize/UseEqualsDeleteCheck.cpp:37
   unless(anyOf(hasBody(stmt()), isDefaulted(), isDeleted(),
+   ast_matchers::isTemplateInstantiation(),
// Ensure that all methods except private special member

hokein wrote:
> Can we remove `ast_matchers::`?
No, there's a collision with 
`isTemplateInstantiation(TemplateSpecializationKind Kind)`.


https://reviews.llvm.org/D26751



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


[PATCH] D26774: [CUDA] Driver changes to support CUDA compilation on MacOS.

2016-11-17 Thread Samuel Antao via cfe-commits
sfantao added a comment.

Hi Justin,

Thanks for the patch.




Comment at: clang/lib/Driver/Driver.cpp:479
+// the device toolchain we create depends on both.
+ToolChain *&CudaTC = ToolChains[CudaTriple.str() + "/" + HostTriple.str()];
+if (!CudaTC) {

I am not sure I understand why to pair host and device toolchain in the map. 
The driver can be used to several compilations, but how do these compilation 
use different host toolchains? Can you give an example of an invocation? Maybe 
add it to the regression tests bellow. 


https://reviews.llvm.org/D26774



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


[PATCH] D26746: [OpenCL] Split PipeType into ReadPipe/WritePipe

2016-11-17 Thread Alexey Bader via cfe-commits
bader accepted this revision.
bader added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks!
A few minor comments regarding outdated comments and style.




Comment at: include/clang/AST/ASTContext.h:1124
 
   /// \brief Return pipe type for the specified type.
+  QualType getReadPipeType(QualType T) const;

Please, update the comment to specify that this function return pipe type with 
'__read_only' access qualifier.



Comment at: lib/AST/ASTContext.cpp:3341
 
 /// Return pipe type for the specified type.
+QualType ASTContext::getReadPipeType(QualType T) const {

Please, remove this comment. It's a copy of the comment from the header file.



Comment at: lib/Serialization/ASTReader.cpp:5806-5807
+return Context.getReadPipeType(ElementType);
+  }
+  case TYPE_WRITE_PIPE: {
+if (Record.size() != 1) {

Please, separate case with an empty line.


Repository:
  rL LLVM

https://reviews.llvm.org/D26746



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


[clang-tools-extra] r287221 - [clang-tidy] Ignore template instantiations in modernize-use-equals-delete check

2016-11-17 Thread Malcolm Parsons via cfe-commits
Author: malcolm.parsons
Date: Thu Nov 17 05:40:02 2016
New Revision: 287221

URL: http://llvm.org/viewvc/llvm-project?rev=287221&view=rev
Log:
[clang-tidy] Ignore template instantiations in modernize-use-equals-delete check

Summary: Template instantiations were causing misplaced fixits.

Reviewers: aaron.ballman, alexfh, hokein

Subscribers: hokein, cfe-commits

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

Modified:
clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp

Modified: clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.cpp?rev=287221&r1=287220&r2=287221&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.cpp Thu 
Nov 17 05:40:02 2016
@@ -34,6 +34,7 @@ void UseEqualsDeleteCheck::registerMatch
   cxxMethodDecl(
   PrivateSpecialFn,
   unless(anyOf(hasBody(stmt()), isDefaulted(), isDeleted(),
+   ast_matchers::isTemplateInstantiation(),
// Ensure that all methods except private special member
// functions are defined.
hasParent(cxxRecordDecl(hasMethod(unless(

Modified: 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp?rev=287221&r1=287220&r2=287221&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp Thu 
Nov 17 05:40:02 2016
@@ -22,6 +22,32 @@ private:
   // CHECK-FIXES: ~PositivePrivate() = delete;
 };
 
+template
+struct PositivePrivateTemplate {
+private:
+  PositivePrivateTemplate();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit 
calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivateTemplate() = delete;
+  PositivePrivateTemplate(const PositivePrivateTemplate &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit 
calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivateTemplate(const PositivePrivateTemplate &) = 
delete;
+  PositivePrivateTemplate &operator=(const PositivePrivateTemplate &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use '= delete' to prohibit 
calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivateTemplate &operator=(const 
PositivePrivateTemplate &) = delete;
+  PositivePrivateTemplate(PositivePrivateTemplate &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit 
calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivateTemplate(PositivePrivateTemplate &&) = delete;
+  PositivePrivateTemplate &operator=(PositivePrivateTemplate &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use '= delete' to prohibit 
calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivateTemplate &operator=(PositivePrivateTemplate 
&&) = delete;
+  ~PositivePrivateTemplate();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit 
calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: ~PositivePrivateTemplate() = delete;
+};
+
+template struct PositivePrivateTemplate;
+template struct PositivePrivateTemplate;
+
 struct NegativePublic {
   NegativePublic(const NegativePublic &);
 };


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


[PATCH] D26751: [clang-tidy] Ignore template instantiations in modernize-use-equals-delete check

2016-11-17 Thread Malcolm Parsons via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL287221: [clang-tidy] Ignore template instantiations in 
modernize-use-equals-delete check (authored by malcolm.parsons).

Changed prior to commit:
  https://reviews.llvm.org/D26751?vs=78202&id=78344#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26751

Files:
  clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp


Index: clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp
@@ -22,6 +22,32 @@
   // CHECK-FIXES: ~PositivePrivate() = delete;
 };
 
+template
+struct PositivePrivateTemplate {
+private:
+  PositivePrivateTemplate();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit 
calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivateTemplate() = delete;
+  PositivePrivateTemplate(const PositivePrivateTemplate &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit 
calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivateTemplate(const PositivePrivateTemplate &) = 
delete;
+  PositivePrivateTemplate &operator=(const PositivePrivateTemplate &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use '= delete' to prohibit 
calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivateTemplate &operator=(const 
PositivePrivateTemplate &) = delete;
+  PositivePrivateTemplate(PositivePrivateTemplate &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit 
calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivateTemplate(PositivePrivateTemplate &&) = delete;
+  PositivePrivateTemplate &operator=(PositivePrivateTemplate &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use '= delete' to prohibit 
calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivateTemplate &operator=(PositivePrivateTemplate 
&&) = delete;
+  ~PositivePrivateTemplate();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit 
calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: ~PositivePrivateTemplate() = delete;
+};
+
+template struct PositivePrivateTemplate;
+template struct PositivePrivateTemplate;
+
 struct NegativePublic {
   NegativePublic(const NegativePublic &);
 };
Index: clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.cpp
@@ -34,6 +34,7 @@
   cxxMethodDecl(
   PrivateSpecialFn,
   unless(anyOf(hasBody(stmt()), isDefaulted(), isDeleted(),
+   ast_matchers::isTemplateInstantiation(),
// Ensure that all methods except private special member
// functions are defined.
hasParent(cxxRecordDecl(hasMethod(unless(


Index: clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp
@@ -22,6 +22,32 @@
   // CHECK-FIXES: ~PositivePrivate() = delete;
 };
 
+template
+struct PositivePrivateTemplate {
+private:
+  PositivePrivateTemplate();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivateTemplate() = delete;
+  PositivePrivateTemplate(const PositivePrivateTemplate &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivateTemplate(const PositivePrivateTemplate &) = delete;
+  PositivePrivateTemplate &operator=(const PositivePrivateTemplate &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivateTemplate &operator=(const PositivePrivateTemplate &) = delete;
+  PositivePrivateTemplate(PositivePrivateTemplate &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivateTemplate(PositivePrivateTemplate &&) = delete;
+  PositivePrivateTemplate &operator

[PATCH] D26752: [include-fixer] Refactor include fixer to be usable as a plugin

2016-11-17 Thread Benjamin Kramer via cfe-commits
bkramer added inline comments.



Comment at: include-fixer/IncludeFixer.cpp:136
+
+auto Begin = StartOfFile.getLocWithOffset(Placed.getOffset());
+auto End = Begin.getLocWithOffset(Placed.getLength());

hokein wrote:
> I have a concern that `Placed` here might be not the replacement for 
> inserting the new header, becuase the `Reps` returned from 
> `createIncludeFixerReplacements` may have some replacements for cleanup.
> 
> To make it more correct, maybe we can check whether 
> `Placed.getReplacementText()` is equal to `"#include" + 
> Context.getHeaderInfos().front().Header`?
I don't think that will work. We do want to put the replacement into the right 
position so we have to apply the full cleanup, right? Just comparing with the 
header path doesn't work because the cleanup is larger than that.


https://reviews.llvm.org/D26752



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


[PATCH] D26752: [include-fixer] Refactor include fixer to be usable as a plugin

2016-11-17 Thread Benjamin Kramer via cfe-commits
bkramer updated this revision to Diff 78345.
bkramer marked 7 inline comments as done.
bkramer added a comment.

- Address review comments.


https://reviews.llvm.org/D26752

Files:
  include-fixer/CMakeLists.txt
  include-fixer/IncludeFixer.cpp
  include-fixer/IncludeFixer.h
  include-fixer/plugin/CMakeLists.txt
  include-fixer/plugin/IncludeFixerPlugin.cpp

Index: include-fixer/plugin/IncludeFixerPlugin.cpp
===
--- /dev/null
+++ include-fixer/plugin/IncludeFixerPlugin.cpp
@@ -0,0 +1,97 @@
+//===- IncludeFixerPlugin.cpp - clang-include-fixer as a clang plugin -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "../IncludeFixer.h"
+#include "../YamlSymbolIndex.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/Parse/ParseAST.h"
+#include "clang/Sema/Sema.h"
+#include "llvm/Support/Path.h"
+
+namespace clang {
+namespace include_fixer {
+
+/// The core include fixer plugin action. This just provides the AST consumer
+/// and command line flag parsing for using include fixer as a clang plugin.
+class ClangIncludeFixerPluginAction : public PluginASTAction {
+  /// ASTConsumer to keep the symbol index alive. We don't really need an
+  /// ASTConsumer for this plugin (everything is funneled on the side through
+  /// Sema) but we have to keep the symbol index alive until sema is done.
+  struct ASTConsumerManagerWrapper : public ASTConsumer {
+ASTConsumerManagerWrapper(std::shared_ptr SIM)
+: SymbolIndexMgr(std::move(SIM)) {}
+std::shared_ptr SymbolIndexMgr;
+  };
+
+public:
+  explicit ClangIncludeFixerPluginAction()
+  : SymbolIndexMgr(std::make_shared()),
+SemaSource(new IncludeFixerSemaSource(*SymbolIndexMgr,
+  /*MinimizeIncludePaths=*/true,
+  /*GenerateDiagnostics=*/true)) {}
+
+  std::unique_ptr
+  CreateASTConsumer(clang::CompilerInstance &CI, StringRef InFile) override {
+CI.setExternalSemaSource(SemaSource);
+SemaSource->setFilePath(InFile);
+SemaSource->setCompilerInstance(&CI);
+return llvm::make_unique(SymbolIndexMgr);
+  }
+
+  void ExecuteAction() override {} // Do nothing.
+
+  bool ParseArgs(const CompilerInstance &CI,
+ const std::vector &Args) override {
+StringRef DB = "yaml";
+StringRef Input;
+
+// Parse the extra command line args.
+// FIXME: This is very limited at the moment.
+for (StringRef Arg : Args) {
+  if (Arg.startswith("-db="))
+DB = Arg.substr(strlen("-db="));
+  else if (Arg.startswith("-input="))
+Input = Arg.substr(strlen("-input="));
+}
+
+llvm::ErrorOr> SymbolIdx(
+nullptr);
+if (DB == "yaml") {
+  if (!Input.empty()) {
+SymbolIdx = include_fixer::YamlSymbolIndex::createFromFile(Input);
+  } else {
+// If we don't have any input file, look in the directory of the first
+// file and its parents.
+const FrontendOptions &FO = CI.getFrontendOpts();
+SmallString<128> AbsolutePath(
+tooling::getAbsolutePath(FO.Inputs[0].getFile()));
+StringRef Directory = llvm::sys::path::parent_path(AbsolutePath);
+SymbolIdx = include_fixer::YamlSymbolIndex::createFromDirectory(
+Directory, "find_all_symbols_db.yaml");
+  }
+}
+SymbolIndexMgr->addSymbolIndex(std::move(*SymbolIdx));
+return true;
+  }
+
+private:
+  std::shared_ptr SymbolIndexMgr;
+  IntrusiveRefCntPtr SemaSource;
+};
+} // namespace include_fixer
+} // namespace clang
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the include fixer plugin.
+volatile int ClangIncludeFixerPluginAnchorSource = 0;
+
+static clang::FrontendPluginRegistry::Add<
+clang::include_fixer::ClangIncludeFixerPluginAction>
+X("clang-include-fixer", "clang-include-fixer");
Index: include-fixer/plugin/CMakeLists.txt
===
--- /dev/null
+++ include-fixer/plugin/CMakeLists.txt
@@ -0,0 +1,12 @@
+add_clang_library(clangIncludeFixerPlugin
+  IncludeFixerPlugin.cpp
+
+  LINK_LIBS
+  clangAST
+  clangBasic
+  clangFrontend
+  clangIncludeFixer
+  clangParse
+  clangSema
+  clangTooling
+  )
Index: include-fixer/IncludeFixer.h
===
--- include-fixer/IncludeFixer.h
+++ include-fixer/IncludeFixer.h
@@ -13,6 +13,7 @@
 #include "IncludeFixerContext.h"
 #include "SymbolIndexManager.h"
 #include "clang/Format/Format.h"
+#include "clang/Sema/ExternalSemaSource.h"
 #include "clang/Tooling/Core/Replacement.h"
 #includ

[PATCH] D21099: [Sema] Teach -Wcast-align to look at the aligned attribute of the declared variables

2016-11-17 Thread Alex Lorenz via cfe-commits
arphaman added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:10304
   CharUnits SrcAlign = Context.getTypeAlignInChars(SrcPointee);
+  Expr *SE = nullptr;
+

NIT, but I think you don't need the extra variable and the `if (SE)` below if 
you extract the code inside `if (SE) {` into a static function that returns 
SrcAlign. Then the if/else below can be rewritten like:

```
  if (auto *CE = dyn_cast(Op)) {
if (CE->getCastKind() == CK_ArrayToPointerDecay)
  SrcAlign = newFunction(CE->getSubExpr(), Context);
  } else if (auto *UO = dyn_cast(Op)) {
if (UO->getOpcode() == UO_AddrOf)
  SrcAlign = newFunction(UO->getSubExpr(), Context);
  }
```



Comment at: lib/Sema/SemaChecking.cpp:10315
+  if (SE) {
+if (const DeclRefExpr *DRE = dyn_cast(SE))
+  SrcAlign = Context.getDeclAlign(DRE->getDecl());

You can use `const auto` here.



Comment at: lib/Sema/SemaChecking.cpp:10317
+  SrcAlign = Context.getDeclAlign(DRE->getDecl());
+else if (const MemberExpr *ME = dyn_cast(SE))
+  SrcAlign = Context.getDeclAlign(ME->getMemberDecl());

Same here.


https://reviews.llvm.org/D21099



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


[PATCH] D26752: [include-fixer] Refactor include fixer to be usable as a plugin

2016-11-17 Thread Eric Liu via cfe-commits
ioeric added inline comments.



Comment at: include-fixer/IncludeFixer.cpp:136
+
+auto Begin = StartOfFile.getLocWithOffset(Placed.getOffset());
+auto End = Begin.getLocWithOffset(Placed.getLength());

bkramer wrote:
> hokein wrote:
> > I have a concern that `Placed` here might be not the replacement for 
> > inserting the new header, becuase the `Reps` returned from 
> > `createIncludeFixerReplacements` may have some replacements for cleanup.
> > 
> > To make it more correct, maybe we can check whether 
> > `Placed.getReplacementText()` is equal to `"#include" + 
> > Context.getHeaderInfos().front().Header`?
> I don't think that will work. We do want to put the replacement into the 
> right position so we have to apply the full cleanup, right? Just comparing 
> with the header path doesn't work because the cleanup is larger than that.
Yep, `createIncludeFixerReplacements` also sorts `#include`s with 
`formatReplacements`, so the returned replacements can be larger than header 
insertion.


https://reviews.llvm.org/D26752



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


[PATCH] D26752: [include-fixer] Refactor include fixer to be usable as a plugin

2016-11-17 Thread Haojian Wu via cfe-commits
hokein added inline comments.



Comment at: include-fixer/IncludeFixer.cpp:136
+
+auto Begin = StartOfFile.getLocWithOffset(Placed.getOffset());
+auto End = Begin.getLocWithOffset(Placed.getLength());

ioeric wrote:
> bkramer wrote:
> > hokein wrote:
> > > I have a concern that `Placed` here might be not the replacement for 
> > > inserting the new header, becuase the `Reps` returned from 
> > > `createIncludeFixerReplacements` may have some replacements for cleanup.
> > > 
> > > To make it more correct, maybe we can check whether 
> > > `Placed.getReplacementText()` is equal to `"#include" + 
> > > Context.getHeaderInfos().front().Header`?
> > I don't think that will work. We do want to put the replacement into the 
> > right position so we have to apply the full cleanup, right? Just comparing 
> > with the header path doesn't work because the cleanup is larger than that.
> Yep, `createIncludeFixerReplacements` also sorts `#include`s with 
> `formatReplacements`, so the returned replacements can be larger than header 
> insertion.
I see. What I concern is that the behavior won't be correct if there are 
multiple replacements in `*Reps`. But looks like if we pass only one 
Replacement for inserting the new header to `createIncludeFixerReplacements`, 
the `Reps` only has a `Replacement` inside, which is correct.

Maybe add an `assert` here?


https://reviews.llvm.org/D26752



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


[PATCH] D26752: [include-fixer] Refactor include fixer to be usable as a plugin

2016-11-17 Thread Eric Liu via cfe-commits
ioeric added inline comments.



Comment at: include-fixer/IncludeFixer.cpp:136
+
+auto Begin = StartOfFile.getLocWithOffset(Placed.getOffset());
+auto End = Begin.getLocWithOffset(Placed.getLength());

hokein wrote:
> ioeric wrote:
> > bkramer wrote:
> > > hokein wrote:
> > > > I have a concern that `Placed` here might be not the replacement for 
> > > > inserting the new header, becuase the `Reps` returned from 
> > > > `createIncludeFixerReplacements` may have some replacements for cleanup.
> > > > 
> > > > To make it more correct, maybe we can check whether 
> > > > `Placed.getReplacementText()` is equal to `"#include" + 
> > > > Context.getHeaderInfos().front().Header`?
> > > I don't think that will work. We do want to put the replacement into the 
> > > right position so we have to apply the full cleanup, right? Just 
> > > comparing with the header path doesn't work because the cleanup is larger 
> > > than that.
> > Yep, `createIncludeFixerReplacements` also sorts `#include`s with 
> > `formatReplacements`, so the returned replacements can be larger than 
> > header insertion.
> I see. What I concern is that the behavior won't be correct if there are 
> multiple replacements in `*Reps`. But looks like if we pass only one 
> Replacement for inserting the new header to `createIncludeFixerReplacements`, 
> the `Reps` only has a `Replacement` inside, which is correct.
> 
> Maybe add an `assert` here?
This is implementation detail; the interface does not guarantee that. And I 
guess there might still be cases where more than one replacement is generated.


https://reviews.llvm.org/D26752



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


[PATCH] D25660: [Analyzer] Checker for iterators dereferenced beyond their range.

2016-11-17 Thread Balogh , Ádám via cfe-commits
baloghadamsoftware updated this revision to Diff 78352.
baloghadamsoftware added a comment.

Updated according to comments.


https://reviews.llvm.org/D25660

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/Inputs/system-header-simulator-for-iterators.h
  test/Analysis/iterator-past-end.cpp

Index: test/Analysis/iterator-past-end.cpp
===
--- /dev/null
+++ test/Analysis/iterator-past-end.cpp
@@ -0,0 +1,193 @@
+// RUN: %clang_cc1 -std=c++11 -analyze -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorPastEnd -analyzer-eagerly-assume %s -verify
+
+#include "Inputs/system-header-simulator-for-iterators.h"
+
+void simple_good(const std::vector &v) {
+  auto i = v.end();
+  if (i != v.end())
+*i; // no-warning
+}
+
+void simple_good_negated(const std::vector &v) {
+  auto i = v.end();
+  if (!(i == v.end()))
+*i; // no-warning
+}
+
+void simple_bad(const std::vector &v) {
+  auto i = v.end();
+  *i; // expected-warning{{Iterator accessed past its end}}
+}
+
+void copy(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  *i2; // expected-warning{{Iterator accessed past its end}}
+}
+
+void decrease(const std::vector &v) {
+  auto i = v.end();
+  --i;
+  *i; // no-warning
+}
+
+void copy_and_decrease1(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  --i1;
+  *i1; // no-warning
+}
+
+void copy_and_decrease2(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  --i1;
+  *i2; // expected-warning{{Iterator accessed past its end}}
+}
+
+void copy_and_increase1(const std::vector &v) {
+  auto i1 = v.begin();
+  auto i2 = i1;
+  ++i1;
+  if (i1 == v.end())
+*i2; // no-warning
+}
+
+void copy_and_increase2(const std::vector &v) {
+  auto i1 = v.begin();
+  auto i2 = i1;
+  ++i1;
+  if (i2 == v.end())
+*i2; // expected-warning{{Iterator accessed past its end}}
+}
+
+void good_find(std::vector &vec, int e) {
+  auto first = std::find(vec.begin(), vec.end(), e);
+  if (vec.end() != first)
+*first; // no-warning
+}
+
+void bad_find(std::vector &vec, int e) {
+  auto first = std::find(vec.begin(), vec.end(), e);
+  *first; // expected-warning{{Iterator accessed past its end}}
+}
+
+void good_find_end(std::vector &vec, std::vector &seq) {
+  auto last = std::find_end(vec.begin(), vec.end(), seq.begin(), seq.end());
+  if (vec.end() != last)
+*last; // no-warning
+}
+
+void bad_find_end(std::vector &vec, std::vector &seq) {
+  auto last = std::find_end(vec.begin(), vec.end(), seq.begin(), seq.end());
+  *last; // expected-warning{{Iterator accessed past its end}}
+}
+
+void good_find_first_of(std::vector &vec, std::vector &seq) {
+  auto first =
+  std::find_first_of(vec.begin(), vec.end(), seq.begin(), seq.end());
+  if (vec.end() != first)
+*first; // no-warning
+}
+
+void bad_find_first_of(std::vector &vec, std::vector &seq) {
+  auto first = std::find_end(vec.begin(), vec.end(), seq.begin(), seq.end());
+  *first; // expected-warning{{Iterator accessed past its end}}
+}
+
+bool odd(int i) { return i % 2; }
+
+void good_find_if(std::vector &vec) {
+  auto first = std::find_if(vec.begin(), vec.end(), odd);
+  if (vec.end() != first)
+*first; // no-warning
+}
+
+void bad_find_if(std::vector &vec, int e) {
+  auto first = std::find_if(vec.begin(), vec.end(), odd);
+  *first; // expected-warning{{Iterator accessed past its end}}
+}
+
+void good_find_if_not(std::vector &vec) {
+  auto first = std::find_if_not(vec.begin(), vec.end(), odd);
+  if (vec.end() != first)
+*first; // no-warning
+}
+
+void bad_find_if_not(std::vector &vec, int e) {
+  auto first = std::find_if_not(vec.begin(), vec.end(), odd);
+  *first; // expected-warning{{Iterator accessed past its end}}
+}
+
+void good_lower_bound(std::vector &vec, int e) {
+  auto first = std::lower_bound(vec.begin(), vec.end(), e);
+  if (vec.end() != first)
+*first; // no-warning
+}
+
+void bad_lower_bound(std::vector &vec, int e) {
+  auto first = std::lower_bound(vec.begin(), vec.end(), e);
+  *first; // expected-warning{{Iterator accessed past its end}}
+}
+
+void good_upper_bound(std::vector &vec, int e) {
+  auto last = std::lower_bound(vec.begin(), vec.end(), e);
+  if (vec.end() != last)
+*last; // no-warning
+}
+
+void bad_upper_bound(std::vector &vec, int e) {
+  auto last = std::lower_bound(vec.begin(), vec.end(), e);
+  *last; // expected-warning{{Iterator accessed past its end}}
+}
+
+void good_search(std::vector &vec, std::vector &seq) {
+  auto first = std::search(vec.begin(), vec.end(), seq.begin(), seq.end());
+  if (vec.end() != first)
+*first; // no-warning
+}
+
+void bad_search(std::vector &vec, std::vector &seq) {
+  auto first = std::search(vec.begin(), vec.end(), seq.begin(), seq.end());
+  *first; // expected-warning{{Iterator acces

[PATCH] D26794: [OpenCL] Blocks are allowed to capture arrays in OpenCL 2.0 and higher.

2016-11-17 Thread Egor Churaev via cfe-commits
echuraev created this revision.
echuraev added a reviewer: Anastasia.
echuraev added subscribers: cfe-commits, yaxunl, bader.

https://reviews.llvm.org/D26794

Files:
  lib/Sema/SemaExpr.cpp
  test/SemaOpenCL/blocks_with_arrays.cl


Index: test/SemaOpenCL/blocks_with_arrays.cl
===
--- /dev/null
+++ test/SemaOpenCL/blocks_with_arrays.cl
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -triple spir64-unkown-unkown -emit-llvm 
%s -o -| FileCheck %s
+
+typedef int (^block_t)();
+
+int block_typedef_kernel(global int* res) {
+  int a[3] = {1, 2, 3};
+  block_t b = ^() {
+// CHECK:   %{{.*}} = getelementptr inbounds [3 x i32], [3 x i32]* %{{.*}}, 
i64 0, i64 0
+return a[0];
+  };
+  return b();
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13477,13 +13477,16 @@
   bool ByRef = false;
   
   // Blocks are not allowed to capture arrays.
-  if (CaptureType->isArrayType()) {
-if (BuildAndDiagnose) {
-  S.Diag(Loc, diag::err_ref_array_type);
-  S.Diag(Var->getLocation(), diag::note_previous_decl) 
-  << Var->getDeclName();
+  // Only if it's not OpenCL 2.0.
+  if (!(S.getLangOpts().OpenCL && S.getLangOpts().OpenCLVersion >= 200)) {
+if (CaptureType->isArrayType()) {
+  if (BuildAndDiagnose) {
+S.Diag(Loc, diag::err_ref_array_type);
+S.Diag(Var->getLocation(), diag::note_previous_decl) 
+<< Var->getDeclName();
+  }
+  return false;
 }
-return false;
   }
 
   // Forbid the block-capture of autoreleasing variables.


Index: test/SemaOpenCL/blocks_with_arrays.cl
===
--- /dev/null
+++ test/SemaOpenCL/blocks_with_arrays.cl
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -triple spir64-unkown-unkown -emit-llvm %s -o -| FileCheck %s
+
+typedef int (^block_t)();
+
+int block_typedef_kernel(global int* res) {
+  int a[3] = {1, 2, 3};
+  block_t b = ^() {
+// CHECK:   %{{.*}} = getelementptr inbounds [3 x i32], [3 x i32]* %{{.*}}, i64 0, i64 0
+return a[0];
+  };
+  return b();
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13477,13 +13477,16 @@
   bool ByRef = false;
   
   // Blocks are not allowed to capture arrays.
-  if (CaptureType->isArrayType()) {
-if (BuildAndDiagnose) {
-  S.Diag(Loc, diag::err_ref_array_type);
-  S.Diag(Var->getLocation(), diag::note_previous_decl) 
-  << Var->getDeclName();
+  // Only if it's not OpenCL 2.0.
+  if (!(S.getLangOpts().OpenCL && S.getLangOpts().OpenCLVersion >= 200)) {
+if (CaptureType->isArrayType()) {
+  if (BuildAndDiagnose) {
+S.Diag(Loc, diag::err_ref_array_type);
+S.Diag(Var->getLocation(), diag::note_previous_decl) 
+<< Var->getDeclName();
+  }
+  return false;
 }
-return false;
   }
 
   // Forbid the block-capture of autoreleasing variables.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D21298: [Clang-tidy] delete null check

2016-11-17 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.



Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:7
+  int *p = 0;
+  if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; 
deleting null pointer has no effect [readability-delete-null-pointer]

hokein wrote:
> Does it work the case like:
> 
> ```
> int *p = nullptr;
> if (p == nullptr) {
>p = new int[3];
>delete[] p;
> }
> ```
> 
> ?
Similarly, it should not mishandle a case like:

void f(int *p) {
  if (p) {
delete p;
  } else {
// Do something else
  }
}



Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:59
+}
\ No newline at end of file

Please add a newline.


https://reviews.llvm.org/D21298



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


[PATCH] D26750: [clang-tidy] Add modernize-use-default-member-init check

2016-11-17 Thread Malcolm Parsons via cfe-commits
malcolm.parsons added inline comments.



Comment at: docs/clang-tidy/checks/modernize-use-default-member-init.rst:7
+This check converts a default constructor's member initializers into default
+member initializers.  Other member initializers that match the default
+member initializer are removed.  This can reduce repeated code or allow use of

Eugene.Zelenko wrote:
> Please fix double spaces. Same below.
If you like, but what harm are they causing?


Repository:
  rL LLVM

https://reviews.llvm.org/D26750



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


[PATCH] D26752: [include-fixer] Refactor include fixer to be usable as a plugin

2016-11-17 Thread Benjamin Kramer via cfe-commits
bkramer updated this revision to Diff 78354.
bkramer added a comment.

- Turn loop into an assertions, there should never be more than one replacement 
coming back.


https://reviews.llvm.org/D26752

Files:
  include-fixer/CMakeLists.txt
  include-fixer/IncludeFixer.cpp
  include-fixer/IncludeFixer.h
  include-fixer/plugin/CMakeLists.txt
  include-fixer/plugin/IncludeFixerPlugin.cpp

Index: include-fixer/plugin/IncludeFixerPlugin.cpp
===
--- /dev/null
+++ include-fixer/plugin/IncludeFixerPlugin.cpp
@@ -0,0 +1,97 @@
+//===- IncludeFixerPlugin.cpp - clang-include-fixer as a clang plugin -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "../IncludeFixer.h"
+#include "../YamlSymbolIndex.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/Parse/ParseAST.h"
+#include "clang/Sema/Sema.h"
+#include "llvm/Support/Path.h"
+
+namespace clang {
+namespace include_fixer {
+
+/// The core include fixer plugin action. This just provides the AST consumer
+/// and command line flag parsing for using include fixer as a clang plugin.
+class ClangIncludeFixerPluginAction : public PluginASTAction {
+  /// ASTConsumer to keep the symbol index alive. We don't really need an
+  /// ASTConsumer for this plugin (everything is funneled on the side through
+  /// Sema) but we have to keep the symbol index alive until sema is done.
+  struct ASTConsumerManagerWrapper : public ASTConsumer {
+ASTConsumerManagerWrapper(std::shared_ptr SIM)
+: SymbolIndexMgr(std::move(SIM)) {}
+std::shared_ptr SymbolIndexMgr;
+  };
+
+public:
+  explicit ClangIncludeFixerPluginAction()
+  : SymbolIndexMgr(std::make_shared()),
+SemaSource(new IncludeFixerSemaSource(*SymbolIndexMgr,
+  /*MinimizeIncludePaths=*/true,
+  /*GenerateDiagnostics=*/true)) {}
+
+  std::unique_ptr
+  CreateASTConsumer(clang::CompilerInstance &CI, StringRef InFile) override {
+CI.setExternalSemaSource(SemaSource);
+SemaSource->setFilePath(InFile);
+SemaSource->setCompilerInstance(&CI);
+return llvm::make_unique(SymbolIndexMgr);
+  }
+
+  void ExecuteAction() override {} // Do nothing.
+
+  bool ParseArgs(const CompilerInstance &CI,
+ const std::vector &Args) override {
+StringRef DB = "yaml";
+StringRef Input;
+
+// Parse the extra command line args.
+// FIXME: This is very limited at the moment.
+for (StringRef Arg : Args) {
+  if (Arg.startswith("-db="))
+DB = Arg.substr(strlen("-db="));
+  else if (Arg.startswith("-input="))
+Input = Arg.substr(strlen("-input="));
+}
+
+llvm::ErrorOr> SymbolIdx(
+nullptr);
+if (DB == "yaml") {
+  if (!Input.empty()) {
+SymbolIdx = include_fixer::YamlSymbolIndex::createFromFile(Input);
+  } else {
+// If we don't have any input file, look in the directory of the first
+// file and its parents.
+const FrontendOptions &FO = CI.getFrontendOpts();
+SmallString<128> AbsolutePath(
+tooling::getAbsolutePath(FO.Inputs[0].getFile()));
+StringRef Directory = llvm::sys::path::parent_path(AbsolutePath);
+SymbolIdx = include_fixer::YamlSymbolIndex::createFromDirectory(
+Directory, "find_all_symbols_db.yaml");
+  }
+}
+SymbolIndexMgr->addSymbolIndex(std::move(*SymbolIdx));
+return true;
+  }
+
+private:
+  std::shared_ptr SymbolIndexMgr;
+  IntrusiveRefCntPtr SemaSource;
+};
+} // namespace include_fixer
+} // namespace clang
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the include fixer plugin.
+volatile int ClangIncludeFixerPluginAnchorSource = 0;
+
+static clang::FrontendPluginRegistry::Add<
+clang::include_fixer::ClangIncludeFixerPluginAction>
+X("clang-include-fixer", "clang-include-fixer");
Index: include-fixer/plugin/CMakeLists.txt
===
--- /dev/null
+++ include-fixer/plugin/CMakeLists.txt
@@ -0,0 +1,12 @@
+add_clang_library(clangIncludeFixerPlugin
+  IncludeFixerPlugin.cpp
+
+  LINK_LIBS
+  clangAST
+  clangBasic
+  clangFrontend
+  clangIncludeFixer
+  clangParse
+  clangSema
+  clangTooling
+  )
Index: include-fixer/IncludeFixer.h
===
--- include-fixer/IncludeFixer.h
+++ include-fixer/IncludeFixer.h
@@ -13,6 +13,7 @@
 #include "IncludeFixerContext.h"
 #include "SymbolIndexManager.h"
 #include "clang/Format/Format.h"
+#include "clang/Sema/ExternalSemaSource.h"
 #include "clang/Tooling/Cor

[PATCH] D26752: [include-fixer] Refactor include fixer to be usable as a plugin

2016-11-17 Thread Benjamin Kramer via cfe-commits
bkramer added inline comments.



Comment at: include-fixer/IncludeFixer.cpp:136
+
+auto Begin = StartOfFile.getLocWithOffset(Placed.getOffset());
+auto End = Begin.getLocWithOffset(Placed.getLength());

ioeric wrote:
> hokein wrote:
> > ioeric wrote:
> > > bkramer wrote:
> > > > hokein wrote:
> > > > > I have a concern that `Placed` here might be not the replacement for 
> > > > > inserting the new header, becuase the `Reps` returned from 
> > > > > `createIncludeFixerReplacements` may have some replacements for 
> > > > > cleanup.
> > > > > 
> > > > > To make it more correct, maybe we can check whether 
> > > > > `Placed.getReplacementText()` is equal to `"#include" + 
> > > > > Context.getHeaderInfos().front().Header`?
> > > > I don't think that will work. We do want to put the replacement into 
> > > > the right position so we have to apply the full cleanup, right? Just 
> > > > comparing with the header path doesn't work because the cleanup is 
> > > > larger than that.
> > > Yep, `createIncludeFixerReplacements` also sorts `#include`s with 
> > > `formatReplacements`, so the returned replacements can be larger than 
> > > header insertion.
> > I see. What I concern is that the behavior won't be correct if there are 
> > multiple replacements in `*Reps`. But looks like if we pass only one 
> > Replacement for inserting the new header to 
> > `createIncludeFixerReplacements`, the `Reps` only has a `Replacement` 
> > inside, which is correct.
> > 
> > Maybe add an `assert` here?
> This is implementation detail; the interface does not guarantee that. And I 
> guess there might still be cases where more than one replacement is generated.
We'll see if this comes up. The current implementation wouldn't deal well with 
multiple replacements as it would format them as alternatives. I'll do the 
assert for now.


https://reviews.llvm.org/D26752



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


[PATCH] D26750: [clang-tidy] Add modernize-use-default-member-init check

2016-11-17 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.



Comment at: docs/clang-tidy/checks/modernize-use-default-member-init.rst:7
+This check converts a default constructor's member initializers into default
+member initializers.  Other member initializers that match the default
+member initializer are removed.  This can reduce repeated code or allow use of

malcolm.parsons wrote:
> Eugene.Zelenko wrote:
> > Please fix double spaces. Same below.
> If you like, but what harm are they causing?
Consistency; double-spacing is atypical in our documentation, probably because 
it's generally viewed online rather than in print.


Repository:
  rL LLVM

https://reviews.llvm.org/D26750



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


[PATCH] D25660: [Analyzer] Checker for iterators dereferenced beyond their range.

2016-11-17 Thread Balogh , Ádám via cfe-commits
baloghadamsoftware marked 10 inline comments as done.
baloghadamsoftware added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp:209
+   CheckerContext &C) const {
+  const auto *Func = Call.getDecl()->getAsFunction();
+  if (Func->isOverloadedOperator()) {

a.sidorin wrote:
> As I remember, `PostCall` is also called for ObjC calls like `ObjCMethodCall` 
> which may not have `FunctionDecl` as their callee. So, `Func` may be a 
> nullptr and needs a check.
You are right, and the same is true for PreCall.



Comment at: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp:219
+
+  assert(LCtx->getKind() == LocationContext::StackFrame &&
+ "Function does not begin with stack frame context");

zaks.anna wrote:
> a.sidorin wrote:
> > `isa(LCtx)`?
> > And `cast<>` below already does the same check with an assertion.
> At least one advantage of the assert is that it provides an error message. 
> I'd not try to minimize the number of asserts.
I agree, but I think Alexei is rigt here: cast<> already has the assert we need 
here.



Comment at: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp:322
+   bool Assumption) const {
+  const auto *SE = Cond.getAsSymExpr();
+  if (!SE)

a.sidorin wrote:
> What will happen if we compare two iterators related to different containers? 
> I guess the result will be undefined but I'm not sure if  we can track it in 
> this checker without referencing the owning container. Let's leave this code 
> as-is but I think this choice deserves a comment.
That will be part of another checker, but where exactly to put the comment you 
suggest?



Comment at: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp:423
+
+void IteratorPastEndChecker::handleComparison(CheckerContext &C,
+  const SVal &LVal,

zaks.anna wrote:
> baloghadamsoftware wrote:
> > baloghadamsoftware wrote:
> > > NoQ wrote:
> > > > baloghadamsoftware wrote:
> > > > > NoQ wrote:
> > > > > > a.sidorin wrote:
> > > > > > > What will happen if we write something like this:
> > > > > > > ```
> > > > > > > bool Eq1 = it1 == it2;
> > > > > > > bool Eq2 = it3 == it4;
> > > > > > > if (Eq1) {...}?
> > > > > > > ```
> > > > > > > 
> > > > > > > As I understand, we'll assume the second condition instead of 
> > > > > > > first.
> > > > > > Had a look. So the problem is, we obtain the result of the 
> > > > > > comparison as a symbol, from which it is too hard to recover the 
> > > > > > operands in order to move iterator position data from one value to 
> > > > > > another.
> > > > > > 
> > > > > > Normally we obtain a simple SymbolConjured for the return value of 
> > > > > > the `operator==()` call (or, similarly, `operator!=()`). For 
> > > > > > plain-value iterators (eg. `typedef T *iterator`) we might be 
> > > > > > obtaining an actual binary symbolic expression, but even then it's 
> > > > > > not quite clear how to obtain operands (the structure of the 
> > > > > > expression might have changed due to algebraic simplifications). 
> > > > > > Additionally, LHS and RHS aren't necessarily symbols (they might be 
> > > > > > semi-concrete), so composing symbolic expressions from them in 
> > > > > > general is problematic with our symbol hierarchy, which is rarely a 
> > > > > > problem for numbers but for structural symbols it'd be a mess.
> > > > > > 
> > > > > > For now i suggest, instead of storing only the last LHS and RHS, to 
> > > > > > save a map from symbols (which are results of comparisons) to (LHS 
> > > > > > value, RHS value) pairs. This map should, apart from the obvious, 
> > > > > > be cleaned up whenever one of the iterators in the pair gets 
> > > > > > mutated (incremented or decremented). This should take care of the 
> > > > > > problem Alexey points out, and should work with semi-concrete stuff.
> > > > > > 
> > > > > > For the future i suggest to let users construct their own symbols 
> > > > > > and symbolic expressions more easily. In fact, if only we had all 
> > > > > > iterators as regions, we should have probably used SymbolMetadata 
> > > > > > for this purpose: it's easy to both recover the parent region from 
> > > > > > it and use it in symbolic expressions. We could also deprecate the 
> > > > > > confusing structural symbols (provide default-bound lazy compound 
> > > > > > values for conjured structures instead), and then it'd be possible 
> > > > > > to transition to SymbolMetadata entirely.
> > > > > Thank you for the suggestion. I am not sure if I fully understand 
> > > > > you. If I create a map where the key is the resulting symbol of the 
> > > > > comparison, it will not work because evalAssume is called for the 
> > > > > innermost comparison. So if the body of operator== 

[PATCH] D26752: [include-fixer] Refactor include fixer to be usable as a plugin

2016-11-17 Thread Haojian Wu via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

LGTM.


https://reviews.llvm.org/D26752



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


[PATCH] D26796: [Driver] Use arch type to find compiler-rt libraries (on Linux)

2016-11-17 Thread Michał Górny via cfe-commits
mgorny created this revision.
mgorny added reviewers: rsmith, compnerd, pcc, rengolin.
mgorny added a subscriber: cfe-commits.
Herald added a subscriber: dberris.

Use llvm::Triple::getArchTypeName() when looking for compiler-rt
libraries, rather than the exact arch string from the triple. This is
more correct as it matches the values used when building compiler-rt
(builtin-config-ix.cmake) which are the subset of the values allowed
in triples.

For example, this fixes an issue when the compiler set for
i686-pc-linux-gnu triple would not find an i386 compiler-rt library,
while this is the exact arch that is detected by compiler-rt. The same
applies to any other i?86 variant allowed by LLVM.

This also makes the special case for MSVC unnecessary, since now i386
will be used reliably for all 32-bit x86 variants.


https://reviews.llvm.org/D26796

Files:
  lib/Driver/ToolChain.cpp


Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -283,15 +283,12 @@
   const llvm::Triple &Triple = TC.getTriple();
   bool IsWindows = Triple.isOSWindows();
 
-  if (Triple.isWindowsMSVCEnvironment() && TC.getArch() == llvm::Triple::x86)
-return "i386";
-
   if (TC.getArch() == llvm::Triple::arm || TC.getArch() == llvm::Triple::armeb)
 return (arm::getARMFloatABI(TC, Args) == arm::FloatABI::Hard && !IsWindows)
? "armhf"
: "arm";
 
-  return TC.getArchName();
+  return llvm::Triple::getArchTypeName(TC.getArch());
 }
 
 std::string ToolChain::getCompilerRT(const ArgList &Args, StringRef Component,


Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -283,15 +283,12 @@
   const llvm::Triple &Triple = TC.getTriple();
   bool IsWindows = Triple.isOSWindows();
 
-  if (Triple.isWindowsMSVCEnvironment() && TC.getArch() == llvm::Triple::x86)
-return "i386";
-
   if (TC.getArch() == llvm::Triple::arm || TC.getArch() == llvm::Triple::armeb)
 return (arm::getARMFloatABI(TC, Args) == arm::FloatABI::Hard && !IsWindows)
? "armhf"
: "arm";
 
-  return TC.getArchName();
+  return llvm::Triple::getArchTypeName(TC.getArch());
 }
 
 std::string ToolChain::getCompilerRT(const ArgList &Args, StringRef Component,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D26798: [docs] Remove doubled spaces

2016-11-17 Thread Malcolm Parsons via cfe-commits
malcolm.parsons created this revision.
malcolm.parsons added a reviewer: aaron.ballman.
malcolm.parsons added a subscriber: cfe-commits.
Herald added a subscriber: nemanjai.

https://reviews.llvm.org/D26798

Files:
  docs/ModularizeUsage.rst
  docs/clang-tidy/checks/cppcoreguidelines-pro-type-cstyle-cast.rst
  docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
  docs/clang-tidy/checks/misc-sizeof-expression.rst
  docs/clang-tidy/checks/modernize-raw-string-literal.rst
  docs/clang-tidy/checks/modernize-use-auto.rst
  docs/clang-tidy/checks/modernize-use-default.rst
  docs/clang-tidy/checks/modernize-use-emplace.rst
  docs/clang-tidy/checks/performance-faster-string-find.rst
  docs/clang-tidy/checks/readability-braces-around-statements.rst
  docs/clang-tidy/index.rst
  docs/include-fixer.rst
  docs/index.rst
  docs/modularize.rst
  docs/pp-trace.rst

Index: docs/pp-trace.rst
===
--- docs/pp-trace.rst
+++ docs/pp-trace.rst
@@ -8,11 +8,11 @@
:hidden:
 
 :program:`pp-trace` is a standalone tool that traces preprocessor
-activity.  It's also used as a test of Clang's PPCallbacks interface.
+activity. It's also used as a test of Clang's PPCallbacks interface.
 It runs a given source file through the Clang preprocessor, displaying
 selected information from callback functions overridden in a
 `PPCallbacks `_
-derivation.  The output is in a high-level YAML format, described in
+derivation. The output is in a high-level YAML format, described in
 :ref:`OutputFormat`.
 
 .. _Usage:
@@ -43,8 +43,8 @@
 .. option:: -ignore 
 
   This option specifies a comma-separated list of names of callbacks
-  that shouldn't be traced.  It can be used to eliminate unwanted
-  trace output.  The callback names are the name of the actual
+  that shouldn't be traced. It can be used to eliminate unwanted
+  trace output. The callback names are the name of the actual
   callback function names in the PPCallbacks class:
 
   * FileChanged
@@ -80,16 +80,16 @@
 
 .. option:: -output 
 
-  By default, pp-trace outputs the trace information to stdout.  Use this
+  By default, pp-trace outputs the trace information to stdout. Use this
   option to output the trace information to a file.
 
 .. _OutputFormat:
 
 pp-trace Output Format
 ==
 
-The pp-trace output is formatted as YAML.  See http://yaml.org/ for general
-YAML information.  It's arranged as a sequence of information about the
+The pp-trace output is formatted as YAML. See http://yaml.org/ for general
+YAML information. It's arranged as a sequence of information about the
 callback call, including the callback name and argument information, for
 example:::
 
@@ -135,9 +135,9 @@
 callback function parameter.
 
 The Argument Value Syntax field describes the values that will be displayed
-for the argument value.  It uses an ad hoc representation that mixes literal
-and symbolic representations.  Enumeration member symbols are shown as the
-actual enum member in a (member1|member2|...) form.  A name in parentheses
+for the argument value. It uses an ad hoc representation that mixes literal
+and symbolic representations. Enumeration member symbols are shown as the
+actual enum member in a (member1|member2|...) form. A name in parentheses
 can either represent a place holder for the described value, or confusingly,
 it might be a literal, such as (null), for a null pointer.
 Locations are shown as quoted only to avoid confusing the documentation generator.
@@ -154,18 +154,18 @@
 
 
 FileChanged is called when the preprocessor enters or exits a file, both the
-top level file being compiled, as well as any #include directives.  It will
+top level file being compiled, as well as any #include directives. It will
 also be called as a result of a system header pragma or in internal renaming
 of a file.
 
 Argument descriptions:
 
 ==   ==   == ==
 Argument NameArgument Value SyntaxClang C++ Type Description   
 ==   ==   == ==
-Loc  "(file):(line):(col)"SourceLocation The location of the directive.
-Reason   (EnterFile|ExitFile|SystemHeaderPragma|RenameFile)   PPCallbacks::FileChangeReason  Reason for change.
-FileType (C_User|C_System|C_ExternCSystem)SrcMgr::CharacteristicKind Include type. 
+Loc  "(file):(line):(col)"SourceLocation The location of the di

[PATCH] D25660: [Analyzer] Checker for iterators dereferenced beyond their range.

2016-11-17 Thread Balogh , Ádám via cfe-commits
baloghadamsoftware marked 2 inline comments as done.
baloghadamsoftware added a comment.

In https://reviews.llvm.org/D25660#590778, @NoQ wrote:

> - Agree on the `evalAssume()` implementation (i'm still not quite 
> understanding what the problem is here, see the new inline comments);


I think it will be settled soon.

> - We should probably not warn by default on unchecked std::find (see comments 
> following the `push_back(2016)` example), unless some strong arguments 
> against such code patterns are provided;

It is automatic. The role of evalCall is only to reduce the exploded graph. If 
I remove it, we get the same result (that is why we have a nonStdFind there, to 
check this case). but with far more states. Especially in case of vector, where 
the GNU implementation is quite complicated because of optimizations.

> - A BugReporterVisitor should be added to report iterator state changes to 
> the user across the diagnostic path;

I also thought of this. The question is where to start the chain.

> - Our code owners often have strong opinions regarding warning message 
> wording.

I need suggestions here.

> Then there are a few ideas on finding more bugs, which you shouldn't 
> necessarily implement, that came up during the review, eg.:
> 
> - Alexey suspects that iterators implemented as plain pointers (commonly used 
> in LLVM itself) might be unsupported;

I think it is supported now.

> - Alexey points out that ++/-- may be handled in a less conservative manner;

That is a future plan, but then it also results in a new name for the checker, 
e.g. IteratorRange.

> - More checks could be implemented in this checker, eg. passing `end()` as 
> first argument of `std::find()` is an instant bug (somebody accidentally 
> swapped begin and end?).

Good idea, but what if it is intentional? I do not mean that we pass end() 
directly, but if we do a loop of find() functions where the beginning of the 
next range is always the successor of the last found element, we may result in 
a range of [end(), end()[, which I think is a valid empty range:

const auto start = v.begin();
while(true) {

  const auto item = find(start, v.end());
  if(item==v.end())
 break;
  doSomething(*item);
  start = ++item;

}

> A list of ideas on improving core experience, mostly for myself because i 
> seem to be the only one with strong opinions on this:
> 
> - Provide a checker callback for structure copies, which would unify the 
> multitude of similar callbacks in this checker;

A callback? Or just move the copy into a simple (or template?) function?

> - Consider removing the conjured structure symbol hack.

Which hack do you mean here? In evalCall() of the various std functions? As I 
mentioned, they can be removed, but then we will get more states in the 
exploded graph.

> Did i forget anything?




https://reviews.llvm.org/D25660



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


[PATCH] D26798: [docs] Remove doubled spaces

2016-11-17 Thread Aaron Ballman via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


https://reviews.llvm.org/D26798



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


[clang-tools-extra] r287226 - [docs] Remove doubled spaces

2016-11-17 Thread Malcolm Parsons via cfe-commits
Author: malcolm.parsons
Date: Thu Nov 17 08:26:45 2016
New Revision: 287226

URL: http://llvm.org/viewvc/llvm-project?rev=287226&view=rev
Log:
[docs] Remove doubled spaces

Reviewers: aaron.ballman

Subscribers: nemanjai, cfe-commits

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

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

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-cstyle-cast.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-expression.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-raw-string-literal.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-auto.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-default.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-emplace.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/performance-faster-string-find.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-braces-around-statements.rst
clang-tools-extra/trunk/docs/clang-tidy/index.rst
clang-tools-extra/trunk/docs/include-fixer.rst
clang-tools-extra/trunk/docs/index.rst
clang-tools-extra/trunk/docs/modularize.rst
clang-tools-extra/trunk/docs/pp-trace.rst

Modified: clang-tools-extra/trunk/docs/ModularizeUsage.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ModularizeUsage.rst?rev=287226&r1=287225&r2=287226&view=diff
==
--- clang-tools-extra/trunk/docs/ModularizeUsage.rst (original)
+++ clang-tools-extra/trunk/docs/ModularizeUsage.rst Thu Nov 17 08:26:45 2016
@@ -10,9 +10,9 @@ specific to modularize, which are descri
 `Modularize Command Line Options`.
 
  specifies the path of a file name for an
-existing module map.  The module map must be well-formed in
-terms of syntax.  Modularize will extract the header file names
-from the map.  Only normal headers are checked, assuming headers
+existing module map. The module map must be well-formed in
+terms of syntax. Modularize will extract the header file names
+from the map. Only normal headers are checked, assuming headers
 marked "private", "textual", or "exclude" are not to be checked
 as a top-level include, assuming they either are included by
 other headers which are checked, or they are not suitable for
@@ -32,7 +32,7 @@ but must be on the same line. For exampl
 
 Note that unless a ``-prefix (header path)`` option is specified,
 non-absolute file paths in the header list file will be relative
-to the header list file directory.  Use -prefix to specify a different
+to the header list file directory. Use -prefix to specify a different
 directory.
 
  is a place-holder for regular Clang
@@ -53,24 +53,24 @@ Modularize Command Line Options
 
   Prepend the given path to non-absolute file paths in the header list file.
   By default, headers are assumed to be relative to the header list file
-  directory.  Use ``-prefix`` to specify a different directory.
+  directory. Use ``-prefix`` to specify a different directory.
 
 .. option:: -module-map-path=
 
-  Generate a module map and output it to the given file.  See the description
+  Generate a module map and output it to the given file. See the description
   in :ref:`module-map-generation`.
   
 .. option:: -problem-files-list=
 
-  For use only with module map assistant.  Input list of files that
-  have problems with respect to modules.  These will still be
+  For use only with module map assistant. Input list of files that
+  have problems with respect to modules. These will still be
   included in the generated module map, but will be marked as
   "excluded" headers.
 
 .. option:: -root-module=
 
   Put modules generated by the -module-map-path option in an enclosing
-  module with the given name.  See the description in 
:ref:`module-map-generation`.
+  module with the given name. See the description in 
:ref:`module-map-generation`.
 
 .. option:: -block-check-header-list-only
 
@@ -93,6 +93,6 @@ Modularize Command Line Options
   and a combined list with problem files preceded by a '#'.
   This can be used to quickly determine which files have problems.
   The latter combined list might be useful in starting to modularize
-  a set of headers.  You can start with a full list of headers,
+  a set of headers. You can start with a full list of headers,
   use -display-file-lists option, and then use the combined list as
   your intermediate list, uncommenting-out headers as you fix them.

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-cstyle-cast.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-cstyle-cast.rst?rev=287226&r1=287225&r2=287226&view=diff
==

[PATCH] D26798: [docs] Remove doubled spaces

2016-11-17 Thread Malcolm Parsons via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL287226: [docs] Remove doubled spaces (authored by 
malcolm.parsons).

Changed prior to commit:
  https://reviews.llvm.org/D26798?vs=78361&id=78362#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26798

Files:
  clang-tools-extra/trunk/docs/ModularizeUsage.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-cstyle-cast.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-expression.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-raw-string-literal.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-auto.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-default.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-emplace.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/performance-faster-string-find.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-braces-around-statements.rst
  clang-tools-extra/trunk/docs/clang-tidy/index.rst
  clang-tools-extra/trunk/docs/include-fixer.rst
  clang-tools-extra/trunk/docs/index.rst
  clang-tools-extra/trunk/docs/modularize.rst
  clang-tools-extra/trunk/docs/pp-trace.rst

Index: clang-tools-extra/trunk/docs/modularize.rst
===
--- clang-tools-extra/trunk/docs/modularize.rst
+++ clang-tools-extra/trunk/docs/modularize.rst
@@ -18,7 +18,7 @@
 map.
 
 :program:`modularize` also has an assistant mode option for generating
-a module map file based on the provided header list.  The generated file
+a module map file based on the provided header list. The generated file
 is a functional module map that can be used as a starting point for a
 module.map file.
 
@@ -115,10 +115,10 @@
 =
 
 The coverage check uses the Clang library to read and parse the
-module map file.  Starting at the module map file directory, or just the
+module map file. Starting at the module map file directory, or just the
 include paths, if specified, it will collect the names of all the files it
 considers headers (no extension, .h, or .inc--if you need more, modify the
-isHeader function).  It then compares the headers against those referenced
+isHeader function). It then compares the headers against those referenced
 in the module map, either explicitly named, or implicitly named via an
 umbrella directory or umbrella file, as parsed by the ModuleMap object.
 If headers are found which are not referenced or covered by an umbrella
@@ -128,7 +128,7 @@
 
 Note that in the case of umbrella headers, this tool invokes the compiler
 to preprocess the file, and uses a callback to collect the header files
-included by the umbrella header or any of its nested includes.  If any
+included by the umbrella header or any of its nested includes. If any
 front end options are needed for these compiler invocations, these
 can be included on the command line after the module map file argument.
 
@@ -154,10 +154,10 @@
 
 If you specify the ``-module-map-path=``,
 :program:`modularize` will output a module map based on the input header list.
-A module will be created for each header.  Also, if the header in the header
+A module will be created for each header. Also, if the header in the header
 list is a partial path, a nested module hierarchy will be created in which a
 module will be created for each subdirectory component in the header path,
-with the header itself represented by the innermost module.  If other headers
+with the header itself represented by the innermost module. If other headers
 use the same subdirectories, they will be enclosed in these same modules also.
 
 For example, for the header list::
@@ -258,8 +258,8 @@
 to be included first.
 
 The module map format defines some keywords which can't be used in module
-names.  If a header has one of these names, an underscore ('_') will be
-prepended to the name.  For example, if the header name is ``header.h``,
+names. If a header has one of these names, an underscore ('_') will be
+prepended to the name. For example, if the header name is ``header.h``,
 because ``header`` is a keyword, the module name will be ``_header``.
 For a list of the module map keywords, please see:
 `Lexical structure `_
Index: clang-tools-extra/trunk/docs/pp-trace.rst
===
--- clang-tools-extra/trunk/docs/pp-trace.rst
+++ clang-tools-extra/trunk/docs/pp-trace.rst
@@ -8,11 +8,11 @@
:hidden:
 
 :program:`pp-trace` is a standalone tool that traces preprocessor
-activity.  It's also used as a test of Clang's PPCallbacks interface.
+activity. It's also used as a test of Clang's PPCallbacks interface.
 It runs a given source file through the Clang preprocessor, displaying
 se

[PATCH] D22374: [analyzer] Copy and move constructors - ExprEngine extended for "almost trivial" copy and move constructors

2016-11-17 Thread Balogh , Ádám via cfe-commits
baloghadamsoftware added a comment.

DO I have to apply your path over patch and update the diff?


https://reviews.llvm.org/D22374



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


[PATCH] D26750: [clang-tidy] Add modernize-use-default-member-init check

2016-11-17 Thread Malcolm Parsons via cfe-commits
malcolm.parsons updated this revision to Diff 78363.
malcolm.parsons added a comment.

Remove doubled spaces


https://reviews.llvm.org/D26750

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  clang-tidy/modernize/UseDefaultMemberInitCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-default-member-init.rst
  test/clang-tidy/modernize-use-default-member-init-assignment.cpp
  test/clang-tidy/modernize-use-default-member-init.cpp

Index: test/clang-tidy/modernize-use-default-member-init.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-default-member-init.cpp
@@ -0,0 +1,202 @@
+// RUN: %check_clang_tidy %s modernize-use-default-member-init %t -- -- -std=c++11
+
+struct S {
+};
+
+struct PositiveValueInt {
+  PositiveValueInt() : i() {}
+  // CHECK-FIXES: PositiveValueInt()  {}
+  const int i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use default member initializer for 'i' [modernize-use-default-member-init]
+  // CHECK-FIXES: const int i{};
+};
+
+struct PositiveInt {
+  PositiveInt() : j(1) {}
+  // CHECK-FIXES: PositiveInt()  {}
+  int j;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'j' [modernize-use-default-member-init]
+  // CHECK-FIXES: int j{1};
+};
+
+struct PositiveValueDouble {
+  PositiveValueDouble() : d() {}
+  // CHECK-FIXES: PositiveValueDouble()  {}
+  double d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use default member initializer for 'd' [modernize-use-default-member-init]
+  // CHECK-FIXES: double d{};
+};
+
+struct PositiveDouble {
+  PositiveDouble() : f(2.5463e43) {}
+  // CHECK-FIXES: PositiveDouble()  {}
+  double f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use default member initializer for 'f' [modernize-use-default-member-init]
+  // CHECK-FIXES: double f{2.5463e43};
+};
+
+struct PositiveValueBool {
+  PositiveValueBool() : b() {}
+  // CHECK-FIXES: PositiveValueBool()  {}
+  bool b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use default member initializer for 'b' [modernize-use-default-member-init]
+  // CHECK-FIXES: bool b{};
+};
+
+struct PositiveBool {
+  PositiveBool() : a(true) {}
+  // CHECK-FIXES: PositiveBool()  {}
+  bool a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use default member initializer for 'a' [modernize-use-default-member-init]
+  // CHECK-FIXES: bool a{true};
+};
+
+struct PositiveValuePointer {
+  PositiveValuePointer() : p() {}
+  // CHECK-FIXES: PositiveValuePointer()  {}
+  int *p;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use default member initializer for 'p' [modernize-use-default-member-init]
+  // CHECK-FIXES: int *p{};
+};
+
+struct PositiveNullPointer {
+  PositiveNullPointer() : q(nullptr) {}
+  // CHECK-FIXES: PositiveNullPointer()  {}
+  int *q;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use default member initializer for 'q' [modernize-use-default-member-init]
+  // CHECK-FIXES: int *q{nullptr};
+};
+
+enum Enum { Foo, Bar };
+struct PositiveEnum {
+  PositiveEnum() : e(Foo) {}
+  // CHECK-FIXES: PositiveEnum()  {}
+  Enum e;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use default member initializer for 'e' [modernize-use-default-member-init]
+  // CHECK-FIXES: Enum e{Foo};
+};
+
+struct NegativeMemberInit {
+  NegativeMemberInit() {}
+  int i = 2;
+};
+
+struct NegativeClass : S {
+  NegativeClass() : s() {}
+  S s;
+};
+
+struct NegativeBase : S {
+  NegativeBase() : S() {}
+};
+
+struct NegativeDefaultOtherMember{
+  NegativeDefaultOtherMember() : i(3) {}
+  int i = 4;
+};
+
+struct NegativeUnion {
+  NegativeUnion() : d(5.0) {}
+  union {
+int i;
+double d;
+  };
+};
+
+struct NegativeBitField
+{
+  NegativeBitField() : i(6) {}
+  int i : 5;
+};
+
+struct NegativeNotDefaultInt
+{
+  NegativeNotDefaultInt(int) : i(7) {}
+  int i;
+};
+
+struct ExistingInt {
+  ExistingInt(short) : e1(), e2(), e3(), e4() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: member initializer for 'e1' is redundant [modernize-use-default-member-init]
+  // CHECK-MESSAGES: :[[@LINE-2]]:30: warning: member initializer for 'e2' is redundant [modernize-use-default-member-init]
+  // CHECK-FIXES: ExistingInt(short) :  e3(), e4() {}
+  ExistingInt(int) : e1(0), e2(0), e3(0), e4(0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: member initializer for 'e1' is redundant [modernize-use-default-member-init]
+  // CHECK-MESSAGES: :[[@LINE-2]]:29: warning: member initializer for 'e2' is redundant [modernize-use-default-member-init]
+  // CHECK-FIXES: ExistingInt(int) :  e3(0), e4(0) {}
+  ExistingInt(long) : e1(5), e2(5), e3(5), e4(5) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: member initializer for 'e3' is redundant [modernize-use-default-member-init]
+  // CHECK-MESSAGES: :[[@LINE-2]]:44: warning: member initializer for 'e4' is redundant [modernize-use-defau

[PATCH] D26796: [Driver] Use arch type to find compiler-rt libraries (on Linux)

2016-11-17 Thread Jonathan Roelofs via cfe-commits
jroelofs added a comment.

Testcase?


https://reviews.llvm.org/D26796



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


[PATCH] D26800: [Sema] Make __attribute__((notnull)) inheritable through function parameters

2016-11-17 Thread Matt Bettinson via cfe-commits
bettinson created this revision.
bettinson added reviewers: rsmith, aaron.ballman.
bettinson added a subscriber: cfe-commits.

Fix PR 30828. ` __attribute__((notnull))` was not inheritable in the 
redefinition of a function. This is because attribute NonNull wasn't 
`InheritableParamAttr`, it was `InheritableAttr`. Clang will now emit warning 
after calling the redeclared function with a null argument.

This is my first patch into Clang, so I'll need someone to commit this for me.

Cheers


https://reviews.llvm.org/D26800

Files:
  include/clang/Basic/Attr.td
  test/Sema/nonnull.c


Index: test/Sema/nonnull.c
===
--- test/Sema/nonnull.c
+++ test/Sema/nonnull.c
@@ -111,7 +111,7 @@
  return 0;
} else {
  return *pointer;
-   } 
+   }
 
set_param_to_null(&pointer);
if (!pointer)
@@ -167,3 +167,10 @@
   int and_again = !returns_nonnull_whee(); // expected-warning {{nonnull 
function call 'returns_nonnull_whee()' will evaluate to 'true' on first 
encounter}}
   and_again = !returns_nonnull_whee(); // expected-warning {{nonnull function 
call 'returns_nonnull_whee()' will evaluate to 'true' on first encounter}}
 }
+
+void pr31040(char *p __attribute__((nonnull)));
+void pr31040(char *p) {}
+
+void call_pr31040() {
+  pr31040(0); // expected-warning {{null passed to a callee that requires a 
non-null argument}}
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1100,7 +1100,7 @@
   let Documentation = [NoSplitStackDocs];
 }
 
-def NonNull : InheritableAttr {
+def NonNull : InheritableParamAttr {
   let Spellings = [GCC<"nonnull">];
   let Subjects = SubjectList<[ObjCMethod, HasFunctionProto, ParmVar], WarnDiag,
  "ExpectedFunctionMethodOrParameter">;


Index: test/Sema/nonnull.c
===
--- test/Sema/nonnull.c
+++ test/Sema/nonnull.c
@@ -111,7 +111,7 @@
  return 0;
} else {
  return *pointer;
-   } 
+   }
 
set_param_to_null(&pointer);
if (!pointer)
@@ -167,3 +167,10 @@
   int and_again = !returns_nonnull_whee(); // expected-warning {{nonnull function call 'returns_nonnull_whee()' will evaluate to 'true' on first encounter}}
   and_again = !returns_nonnull_whee(); // expected-warning {{nonnull function call 'returns_nonnull_whee()' will evaluate to 'true' on first encounter}}
 }
+
+void pr31040(char *p __attribute__((nonnull)));
+void pr31040(char *p) {}
+
+void call_pr31040() {
+  pr31040(0); // expected-warning {{null passed to a callee that requires a non-null argument}}
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1100,7 +1100,7 @@
   let Documentation = [NoSplitStackDocs];
 }
 
-def NonNull : InheritableAttr {
+def NonNull : InheritableParamAttr {
   let Spellings = [GCC<"nonnull">];
   let Subjects = SubjectList<[ObjCMethod, HasFunctionProto, ParmVar], WarnDiag,
  "ExpectedFunctionMethodOrParameter">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r287227 - [OPENMP] Fixed codegen for 'omp cancel' construct.

2016-11-17 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Thu Nov 17 09:12:05 2016
New Revision: 287227

URL: http://llvm.org/viewvc/llvm-project?rev=287227&view=rev
Log:
[OPENMP] Fixed codegen for 'omp cancel' construct.

If 'omp cancel' construct is used in a worksharing construct it may
cause hanging of the software in case if reduction clause is used. Patch fixes 
this problem by avoiding extra reduction processing for branches that were 
canceled.

Modified:
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/test/OpenMP/cancel_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp?rev=287227&r1=287226&r2=287227&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp Thu Nov 17 09:12:05 2016
@@ -1781,9 +1781,11 @@ void CodeGenFunction::EmitOMPOuterLoop(b
   EmitBlock(LoopExit.getBlock());
 
   // Tell the runtime we are done.
-  if (!DynamicOrOrdered)
-RT.emitForStaticFinish(*this, S.getLocEnd());
-
+  auto &&CodeGen = [DynamicOrOrdered, &S](CodeGenFunction &CGF) {
+if (!DynamicOrOrdered)
+  CGF.CGM.getOpenMPRuntime().emitForStaticFinish(CGF, S.getLocEnd());
+  };
+  OMPCancelStack.emitExit(*this, S.getDirectiveKind(), CodeGen);
 }
 
 void CodeGenFunction::EmitOMPForOuterLoop(
@@ -1899,6 +1901,8 @@ void CodeGenFunction::EmitOMPDistributeP
   *this, OMPD_distribute_parallel_for,
   [&S](CodeGenFunction &CGF, PrePostActionTy &) {
 OMPLoopScope PreInitScope(CGF, S);
+OMPCancelStackRAII CancelRegion(CGF, OMPD_distribute_parallel_for,
+/*HasCancel=*/false);
 CGF.EmitStmt(
 cast(S.getAssociatedStmt())->getCapturedStmt());
   });
@@ -2123,7 +2127,10 @@ bool CodeGenFunction::EmitOMPWorksharing
  [](CodeGenFunction &) {});
 EmitBlock(LoopExit.getBlock());
 // Tell the runtime we are done.
-RT.emitForStaticFinish(*this, S.getLocStart());
+auto &&CodeGen = [&S](CodeGenFunction &CGF) {
+  CGF.CGM.getOpenMPRuntime().emitForStaticFinish(CGF, S.getLocEnd());
+};
+OMPCancelStack.emitExit(*this, S.getDirectiveKind(), CodeGen);
   } else {
 const bool IsMonotonic =
 Ordered || ScheduleKind.Schedule == OMPC_SCHEDULE_static ||
@@ -2173,6 +2180,7 @@ void CodeGenFunction::EmitOMPForDirectiv
   bool HasLastprivates = false;
   auto &&CodeGen = [&S, &HasLastprivates](CodeGenFunction &CGF,
   PrePostActionTy &) {
+OMPCancelStackRAII CancelRegion(CGF, OMPD_for, S.hasCancel());
 HasLastprivates = CGF.EmitOMPWorksharingLoop(S);
   };
   {
@@ -2313,7 +2321,10 @@ void CodeGenFunction::EmitSections(const
 CGF.EmitOMPInnerLoop(S, /*RequiresCleanup=*/false, &Cond, &Inc, BodyGen,
  [](CodeGenFunction &) {});
 // Tell the runtime we are done.
-CGF.CGM.getOpenMPRuntime().emitForStaticFinish(CGF, S.getLocStart());
+auto &&CodeGen = [&S](CodeGenFunction &CGF) {
+  CGF.CGM.getOpenMPRuntime().emitForStaticFinish(CGF, S.getLocEnd());
+};
+CGF.OMPCancelStack.emitExit(CGF, S.getDirectiveKind(), CodeGen);
 CGF.EmitOMPReductionClauseFinal(S);
 // Emit post-update of the reduction variables if IsLastIter != 0.
 emitPostUpdateForReductionClause(
@@ -2335,6 +2346,7 @@ void CodeGenFunction::EmitSections(const
 HasCancel = OSD->hasCancel();
   else if (auto *OPSD = dyn_cast(&S))
 HasCancel = OPSD->hasCancel();
+  OMPCancelStackRAII CancelRegion(*this, S.getDirectiveKind(), HasCancel);
   CGM.getOpenMPRuntime().emitInlinedDirective(*this, OMPD_sections, CodeGen,
   HasCancel);
   // Emit barrier for lastprivates only if 'sections' directive has 'nowait'
@@ -2438,6 +2450,7 @@ void CodeGenFunction::EmitOMPParallelFor
   // Emit directive as a combined directive that consists of two implicit
   // directives: 'parallel' with 'for' directive.
   auto &&CodeGen = [&S](CodeGenFunction &CGF, PrePostActionTy &) {
+OMPCancelStackRAII CancelRegion(CGF, OMPD_parallel_for, S.hasCancel());
 CGF.EmitOMPWorksharingLoop(S);
   };
   emitCommonOMPParallelDirective(*this, S, OMPD_for, CodeGen);
@@ -3435,11 +3448,14 @@ void CodeGenFunction::EmitOMPCancelDirec
 
 CodeGenFunction::JumpDest
 CodeGenFunction::getOMPCancelDestination(OpenMPDirectiveKind Kind) {
-  if (Kind == OMPD_parallel || Kind == OMPD_task)
+  if (Kind == OMPD_parallel || Kind == OMPD_task ||
+  Kind == OMPD_target_parallel)
 return ReturnBlock;
   assert(Kind == OMPD_for || Kind == OMPD_section || Kind == OMPD_sections ||
- Kind == OMPD_parallel_sections || Kind == OMPD_parallel_for);
-  return BreakContinueStack.back().BreakBlock;
+ Kind == OMPD_parallel_sections || Kind == OMPD_para

[PATCH] D26752: [include-fixer] Refactor include fixer to be usable as a plugin

2016-11-17 Thread Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL287228: [include-fixer] Refactor include fixer to be usable 
as a plugin (authored by d0k).

Changed prior to commit:
  https://reviews.llvm.org/D26752?vs=78354&id=78366#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26752

Files:
  clang-tools-extra/trunk/include-fixer/CMakeLists.txt
  clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
  clang-tools-extra/trunk/include-fixer/IncludeFixer.h
  clang-tools-extra/trunk/include-fixer/plugin/CMakeLists.txt
  clang-tools-extra/trunk/include-fixer/plugin/IncludeFixerPlugin.cpp

Index: clang-tools-extra/trunk/include-fixer/IncludeFixer.h
===
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.h
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.h
@@ -13,6 +13,7 @@
 #include "IncludeFixerContext.h"
 #include "SymbolIndexManager.h"
 #include "clang/Format/Format.h"
+#include "clang/Sema/ExternalSemaSource.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Tooling.h"
 #include 
@@ -80,6 +81,70 @@
 const format::FormatStyle &Style = format::getLLVMStyle(),
 bool AddQualifiers = true);
 
+/// Handles callbacks from sema, does the include lookup and turns it into an
+/// IncludeFixerContext.
+class IncludeFixerSemaSource : public clang::ExternalSemaSource {
+public:
+  explicit IncludeFixerSemaSource(SymbolIndexManager &SymbolIndexMgr,
+  bool MinimizeIncludePaths,
+  bool GenerateDiagnostics)
+  : SymbolIndexMgr(SymbolIndexMgr),
+MinimizeIncludePaths(MinimizeIncludePaths),
+GenerateDiagnostics(GenerateDiagnostics) {}
+
+  void setCompilerInstance(CompilerInstance *CI) { this->CI = CI; }
+  void setFilePath(StringRef FilePath) { this->FilePath = FilePath; }
+
+  /// Callback for incomplete types. If we encounter a forward declaration we
+  /// have the fully qualified name ready. Just query that.
+  bool MaybeDiagnoseMissingCompleteType(clang::SourceLocation Loc,
+clang::QualType T) override;
+
+  /// Callback for unknown identifiers. Try to piece together as much
+  /// qualification as we can get and do a query.
+  clang::TypoCorrection CorrectTypo(const DeclarationNameInfo &Typo,
+int LookupKind, Scope *S, CXXScopeSpec *SS,
+CorrectionCandidateCallback &CCC,
+DeclContext *MemberContext,
+bool EnteringContext,
+const ObjCObjectPointerType *OPT) override;
+
+  /// Get the minimal include for a given path.
+  std::string minimizeInclude(StringRef Include,
+  const clang::SourceManager &SourceManager,
+  clang::HeaderSearch &HeaderSearch) const;
+
+  /// Get the include fixer context for the queried symbol.
+  IncludeFixerContext
+  getIncludeFixerContext(const clang::SourceManager &SourceManager,
+ clang::HeaderSearch &HeaderSearch) const;
+
+private:
+  /// Query the database for a given identifier.
+  bool query(StringRef Query, StringRef ScopedQualifiers, tooling::Range Range);
+
+  CompilerInstance *CI;
+
+  /// The client to use to find cross-references.
+  SymbolIndexManager &SymbolIndexMgr;
+
+  /// The information of the symbols being queried.
+  std::vector QuerySymbolInfos;
+
+  /// All symbol candidates which match QuerySymbol. We only include the first
+  /// discovered identifier to avoid getting caught in results from error
+  /// recovery.
+  std::vector MatchedSymbols;
+
+  /// The file path to the file being processed.
+  std::string FilePath;
+
+  /// Whether we should use the smallest possible include path.
+  bool MinimizeIncludePaths = true;
+
+  /// Whether we should generate diagnostics with fixits for missing symbols.
+  bool GenerateDiagnostics = false;
+};
 } // namespace include_fixer
 } // namespace clang
 
Index: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
===
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
@@ -13,7 +13,6 @@
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Parse/ParseAST.h"
-#include "clang/Sema/ExternalSemaSource.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -25,19 +24,17 @@
 namespace clang {
 namespace include_fixer {
 namespace {
-
 /// Manages the parse, gathers include suggestions.
-class Action : public clang::ASTFrontendAction,
-   public clang::ExternalSemaSource {
+class Action : public clang::ASTFrontendAction {
 public:
   explicit Action(SymbolIndexManager &SymbolIndexMgr, bool 

[clang-tools-extra] r287228 - [include-fixer] Refactor include fixer to be usable as a plugin

2016-11-17 Thread Benjamin Kramer via cfe-commits
Author: d0k
Date: Thu Nov 17 09:16:05 2016
New Revision: 287228

URL: http://llvm.org/viewvc/llvm-project?rev=287228&view=rev
Log:
[include-fixer] Refactor include fixer to be usable as a plugin

- Refactor the external sema source into a visible class
- Add support for emitting FixIts
- Wrap up include fixer as a plugin as I did with clang-tidy

Test case will follow as soon as I wire this up in libclang.

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

Added:
clang-tools-extra/trunk/include-fixer/plugin/
clang-tools-extra/trunk/include-fixer/plugin/CMakeLists.txt
clang-tools-extra/trunk/include-fixer/plugin/IncludeFixerPlugin.cpp
Modified:
clang-tools-extra/trunk/include-fixer/CMakeLists.txt
clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/IncludeFixer.h

Modified: clang-tools-extra/trunk/include-fixer/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/CMakeLists.txt?rev=287228&r1=287227&r2=287228&view=diff
==
--- clang-tools-extra/trunk/include-fixer/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/include-fixer/CMakeLists.txt Thu Nov 17 09:16:05 
2016
@@ -22,5 +22,6 @@ add_clang_library(clangIncludeFixer
   findAllSymbols
   )
 
+add_subdirectory(plugin)
 add_subdirectory(tool)
 add_subdirectory(find-all-symbols)

Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp?rev=287228&r1=287227&r2=287228&view=diff
==
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp Thu Nov 17 09:16:05 
2016
@@ -13,7 +13,6 @@
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Parse/ParseAST.h"
-#include "clang/Sema/ExternalSemaSource.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -25,19 +24,17 @@ using namespace clang;
 namespace clang {
 namespace include_fixer {
 namespace {
-
 /// Manages the parse, gathers include suggestions.
-class Action : public clang::ASTFrontendAction,
-   public clang::ExternalSemaSource {
+class Action : public clang::ASTFrontendAction {
 public:
   explicit Action(SymbolIndexManager &SymbolIndexMgr, bool 
MinimizeIncludePaths)
-  : SymbolIndexMgr(SymbolIndexMgr),
-MinimizeIncludePaths(MinimizeIncludePaths) {}
+  : SemaSource(SymbolIndexMgr, MinimizeIncludePaths,
+   /*GenerateDiagnostics=*/false) {}
 
   std::unique_ptr
   CreateASTConsumer(clang::CompilerInstance &Compiler,
 StringRef InFile) override {
-FilePath = InFile;
+SemaSource.setFilePath(InFile);
 return llvm::make_unique();
   }
 
@@ -55,254 +52,21 @@ public:
   CompletionConsumer = &Compiler->getCodeCompletionConsumer();
 
 Compiler->createSema(getTranslationUnitKind(), CompletionConsumer);
-Compiler->getSema().addExternalSource(this);
+SemaSource.setCompilerInstance(Compiler);
+Compiler->getSema().addExternalSource(&SemaSource);
 
 clang::ParseAST(Compiler->getSema(), Compiler->getFrontendOpts().ShowStats,
 Compiler->getFrontendOpts().SkipFunctionBodies);
   }
 
-  /// Callback for incomplete types. If we encounter a forward declaration we
-  /// have the fully qualified name ready. Just query that.
-  bool MaybeDiagnoseMissingCompleteType(clang::SourceLocation Loc,
-clang::QualType T) override {
-// Ignore spurious callbacks from SFINAE contexts.
-if (getCompilerInstance().getSema().isSFINAEContext())
-  return false;
-
-clang::ASTContext &context = getCompilerInstance().getASTContext();
-std::string QueryString =
-T.getUnqualifiedType().getAsString(context.getPrintingPolicy());
-DEBUG(llvm::dbgs() << "Query missing complete type '" << QueryString
-   << "'");
-// Pass an empty range here since we don't add qualifier in this case.
-query(QueryString, "", tooling::Range());
-return false;
-  }
-
-  /// Callback for unknown identifiers. Try to piece together as much
-  /// qualification as we can get and do a query.
-  clang::TypoCorrection CorrectTypo(const DeclarationNameInfo &Typo,
-int LookupKind, Scope *S, CXXScopeSpec *SS,
-CorrectionCandidateCallback &CCC,
-DeclContext *MemberContext,
-bool EnteringContext,
-const ObjCObjectPointerType *OPT) override 
{
-// Ignore spurious callbacks from SFINAE contexts.
-if (getCompilerInstance().getSema().isSFINAEContext())
-  ret

r287229 - Link include-fixer into libclang if clang-tools-extra is checked out.

2016-11-17 Thread Benjamin Kramer via cfe-commits
Author: d0k
Date: Thu Nov 17 09:22:36 2016
New Revision: 287229

URL: http://llvm.org/viewvc/llvm-project?rev=287229&view=rev
Log:
Link include-fixer into libclang if clang-tools-extra is checked out.

include-fixer only slightly bloats the size of libclang, but since
libclang has no explicit plugin mechanism it's the only way of getting
this to work. Clang-tidy is already there and so far there weren't many
complaints ;)

This is designed to be easy to remove again if libclang ever grows
proper plugin support.

Modified:
cfe/trunk/tools/libclang/CIndex.cpp
cfe/trunk/tools/libclang/CMakeLists.txt

Modified: cfe/trunk/tools/libclang/CIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=287229&r1=287228&r2=287229&view=diff
==
--- cfe/trunk/tools/libclang/CIndex.cpp (original)
+++ cfe/trunk/tools/libclang/CIndex.cpp Thu Nov 17 09:22:36 2016
@@ -8120,4 +8120,10 @@ cxindex::Logger::~Logger() {
 extern volatile int ClangTidyPluginAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED ClangTidyPluginAnchorDestination =
 ClangTidyPluginAnchorSource;
+
+// This anchor is used to force the linker to link the clang-include-fixer
+// plugin.
+extern volatile int ClangIncludeFixerPluginAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ClangIncludeFixerPluginAnchorDestination =
+ClangIncludeFixerPluginAnchorSource;
 #endif

Modified: cfe/trunk/tools/libclang/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CMakeLists.txt?rev=287229&r1=287228&r2=287229&view=diff
==
--- cfe/trunk/tools/libclang/CMakeLists.txt (original)
+++ cfe/trunk/tools/libclang/CMakeLists.txt Thu Nov 17 09:22:36 2016
@@ -50,6 +50,7 @@ endif ()
 if (TARGET clangTidyPlugin)
   add_definitions(-DCLANG_TOOL_EXTRA_BUILD)
   list(APPEND LIBS clangTidyPlugin)
+  list(APPEND LIBS clangIncludeFixerPlugin)
 endif ()
 
 find_library(DL_LIBRARY_PATH dl)


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


[clang-tools-extra] r287230 - [include-fixer] Add a test for the full round trip through libclang and the plugin.

2016-11-17 Thread Benjamin Kramer via cfe-commits
Author: d0k
Date: Thu Nov 17 09:23:06 2016
New Revision: 287230

URL: http://llvm.org/viewvc/llvm-project?rev=287230&view=rev
Log:
[include-fixer] Add a test for the full round trip through libclang and the 
plugin.

Added:
clang-tools-extra/trunk/test/include-fixer/yamldb_plugin.cpp

Added: clang-tools-extra/trunk/test/include-fixer/yamldb_plugin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/include-fixer/yamldb_plugin.cpp?rev=287230&view=auto
==
--- clang-tools-extra/trunk/test/include-fixer/yamldb_plugin.cpp (added)
+++ clang-tools-extra/trunk/test/include-fixer/yamldb_plugin.cpp Thu Nov 17 
09:23:06 2016
@@ -0,0 +1,10 @@
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin 
-Xclang clang-include-fixer -fspell-checking -Xclang 
-plugin-arg-clang-include-fixer -Xclang -input=%p/Inputs/fake_yaml_db.yaml 2>&1 
| FileCheck %s
+
+foo f;
+
+// CHECK: yamldb_plugin.cpp:3:1: error: unknown type name 'foo'; did you mean 
'foo'?
+// CHECK: Number FIX-ITs = 1
+// CHECK: FIX-IT: Replace [3:1 - 3:4] with "foo"
+// CHECK: yamldb_plugin.cpp:3:1: note: Add '#include "foo.h"' to provide the 
missing declaration [clang-include-fixer]
+// CHECK: Number FIX-ITs = 1
+// CHECK: FIX-IT: Replace [3:1 - 3:4] with "#include "foo.h"


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


[PATCH] D23765: Fix for clang PR 29087

2016-11-17 Thread Gonzalo BG via cfe-commits
gnzlbg added a comment.

ping


https://reviews.llvm.org/D23765



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


[PATCH] D26796: [Driver] Use arch type to find compiler-rt libraries (on Linux)

2016-11-17 Thread Michał Górny via cfe-commits
mgorny updated this revision to Diff 78370.
mgorny added a comment.

Updated expected output for existing tests and added an additional test to 
linux-ld.c.


https://reviews.llvm.org/D26796

Files:
  lib/Driver/ToolChain.cpp
  test/Driver/Inputs/basic_linux_tree/usr/i686-unknown-linux/lib/.keep
  
test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/i686-unknown-linux/4.6.0/crtbegin.o
  test/Driver/linux-ld.c
  test/Driver/nostdlib.c
  test/Driver/print-libgcc-file-name-clangrt.c
  test/Driver/windows-cross.c

Index: test/Driver/windows-cross.c
===
--- test/Driver/windows-cross.c
+++ test/Driver/windows-cross.c
@@ -59,7 +59,7 @@
 // RUN:| FileCheck %s --check-prefix CHECK-SANITIZE-ADDRESS-EXE-X86
 
 // CHECK-SANITIZE-ADDRESS-EXE-X86: "-fsanitize=address"
-// CHECK-SANITIZE-ADDRESS-EXE-X86: "{{.*}}clang_rt.asan_dynamic-i686.lib" "{{.*}}clang_rt.asan_dynamic_runtime_thunk-i686.lib" "--undefined" "___asan_seh_interceptor"
+// CHECK-SANITIZE-ADDRESS-EXE-X86: "{{.*}}clang_rt.asan_dynamic-i386.lib" "{{.*}}clang_rt.asan_dynamic_runtime_thunk-i386.lib" "--undefined" "___asan_seh_interceptor"
 
 // RUN: %clang -### -target armv7-windows-itanium --sysroot %S/Inputs/Windows/ARM/8.1 -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=lld-link2 -shared -o shared.dll -fsanitize=tsan -x c++ %s 2>&1 \
 // RUN:| FileCheck %s --check-prefix CHECK-SANITIZE-TSAN
Index: test/Driver/print-libgcc-file-name-clangrt.c
===
--- test/Driver/print-libgcc-file-name-clangrt.c
+++ test/Driver/print-libgcc-file-name-clangrt.c
@@ -6,6 +6,6 @@
 // CHECK-CLANGRT-X8664: libclang_rt.builtins-x86_64.a
 
 // RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \
-// RUN: --target=i686-pc-linux \
+// RUN: --target=i386-pc-linux \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-I686 %s
-// CHECK-CLANGRT-I686: libclang_rt.builtins-i686.a
+// CHECK-CLANGRT-I686: libclang_rt.builtins-i386.a
Index: test/Driver/nostdlib.c
===
--- test/Driver/nostdlib.c
+++ test/Driver/nostdlib.c
@@ -14,18 +14,18 @@
 // passed down to link line
 // RUN: %clang -no-canonical-prefixes %s -### -Wno-liblto -o %t.o 2>&1 \
 // RUN: -target i686-pc-linux-gnu -nostdlib --rtlib=compiler-rt \
-// RUN: -resource-dir=%S/Inputs/resource_dir -lclang_rt.builtins-i686 \
+// RUN: -resource-dir=%S/Inputs/resource_dir -lclang_rt.builtins-i386 \
 // RUN:   | FileCheck --check-prefix=CHECK-LINUX-NOSTDLIB %s
 //
 // RUN: %clang -no-canonical-prefixes %s -### -Wno-liblto -o %t.o 2>&1 \
 // RUN: -target i686-pc-linux-gnu --rtlib=compiler-rt -nostdlib \
-// RUN: -resource-dir=%S/Inputs/resource_dir -lclang_rt.builtins-i686 \
+// RUN: -resource-dir=%S/Inputs/resource_dir -lclang_rt.builtins-i386 \
 // RUN:   | FileCheck --check-prefix=CHECK-LINUX-NOSTDLIB %s
 //
 // RUN: %clang -target x86_64-pc-windows-msvc -nostdlib --rtlib=compiler-rt -### -Wno-liblto %s 2>&1 | FileCheck %s -check-prefix CHECK-MSVC-NOSTDLIB
 // RUN: %clang -target x86_64-pc-windows-msvc --rtlib=compiler-rt -nostdlib -### -Wno-liblto %s 2>&1 | FileCheck %s -check-prefix CHECK-MSVC-NOSTDLIB
 //
 // CHECK-LINUX-NOSTDLIB: warning: argument unused during compilation: '--rtlib=compiler-rt'
 // CHECK-LINUX-NOSTDLIB: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
-// CHECK-LINUX-NOSTDLIB-NOT: "{{.*}}/Inputs/resource_dir{{/|}}lib{{/|}}linux{{/|}}libclang_rt.builtins-i686.a"
+// CHECK-LINUX-NOSTDLIB-NOT: "{{.*}}/Inputs/resource_dir{{/|}}lib{{/|}}linux{{/|}}libclang_rt.builtins-i386.a"
 // CHECK-MSVC-NOSTDLIB: warning: argument unused during compilation: '--rtlib=compiler-rt'
Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -71,6 +71,27 @@
 // CHECK-LD-RT: libclang_rt.builtins-x86_64.a"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=i686-unknown-linux \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: --rtlib=compiler-rt \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-RT-I686 %s
+// CHECK-LD-RT-I686-NOT: warning:
+// CHECK-LD-RT-I686: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-LD-RT-I686: "--eh-frame-hdr"
+// CHECK-LD-RT-I686: "-m" "elf_i386"
+// CHECK-LD-RT-I686: "-dynamic-linker"
+// CHECK-LD-RT-I686: "{{.*}}/usr/lib/gcc/i686-unknown-linux/4.6.0{{/|}}crtbegin.o"
+// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib/gcc/i686-unknown-linux/4.6.0"
+// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib/gcc/i686-unknown-linux/4.6.0/../../../../i686-unknown-linux/lib"
+// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib/gcc/i686-unknown-linux/4.6.0/../../.."
+// CHECK-LD-RT-I686: "-L[[SYSROOT]]/lib"
+// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-RT-I686: libclang_rt.builtins-i386.a"
+// CHECK-L

[PATCH] D26796: [Driver] Use arch type to find compiler-rt libraries (on Linux)

2016-11-17 Thread Jonathan Roelofs via cfe-commits
jroelofs added inline comments.



Comment at: test/Driver/print-libgcc-file-name-clangrt.c:11
 // RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-I686 %s
-// CHECK-CLANGRT-I686: libclang_rt.builtins-i686.a
+// CHECK-CLANGRT-I686: libclang_rt.builtins-i386.a

This CHECK's name is now a lie.


https://reviews.llvm.org/D26796



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


[PATCH] D24289: Add warning when assigning enums to bitfields without an explicit unsigned underlying type

2016-11-17 Thread Reid Kleckner via cfe-commits
rnk added a comment.

In https://reviews.llvm.org/D24289#598169, @rsmith wrote:

> This is causing warnings to fire for headers shared between C and C++, where 
> the "give the enum an unsigned underlying type" advice doesn't work, and 
> where the code in question will never be built for the MS ABI. It seems 
> really hard to justify this being on by default.
>
> I'm going to turn it off by default for now, but we should probably consider 
> turning it back on by default when targeting the MS ABI (as a "your code is 
> wrong" warning rather than a "your code is not portable" warning).


Seems reasonable. I asked to make it off by default, but got argued down to 
putting it under -Wall.

> Yeah, suggesting adding an underlying type to the enum to solve this problem 
> seems like a bad idea, since that fundamentally changes the nature of the 
> enum -- typically allowing it to store a lot more values, and making putting 
> it in a bitfield a bad idea.

Any time you use a bitfield it stores fewer values than the original integer 
type. I don't see how enums are special here. Even if I can store values in the 
enum not representable as a bitwise combination of enumerators, people usually 
don't, and adding an underlying type doesn't change the conventions of normal 
C++ code.

Do you have any better suggestions for people that want this code to do the 
right thing when built with MSVC?

  enum E /* : unsigned */ { E0, E1, E2, E3 };
  struct A { E b : 2; };
  int main() {
A a;
a.b = E3;
return a.b; // comes out as -1 without the underlying type
  }

Widening the bitfield wastes a bit. Making the bitfield a plain integer and 
cast in and out of the enum type, but that obscures the true type of the 
bitfield. So, I still support this suggestion.


Repository:
  rL LLVM

https://reviews.llvm.org/D24289



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


[PATCH] D26763: [compiler-rt] [asan] Use __SSE2__ (rather than __i686__...) for SSE2 test

2016-11-17 Thread Kostya Serebryany via cfe-commits
kcc accepted this revision.
kcc added a comment.
This revision is now accepted and ready to land.

LGTM, 
assuming you have verified that the test is still executed.


https://reviews.llvm.org/D26763



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


[PATCH] D25660: [Analyzer] Checker for iterators dereferenced beyond their range.

2016-11-17 Thread Anna Zaks via cfe-commits
zaks.anna added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp:423
+
+void IteratorPastEndChecker::handleComparison(CheckerContext &C,
+  const SVal &LVal,

baloghadamsoftware wrote:
> zaks.anna wrote:
> > baloghadamsoftware wrote:
> > > baloghadamsoftware wrote:
> > > > NoQ wrote:
> > > > > baloghadamsoftware wrote:
> > > > > > NoQ wrote:
> > > > > > > a.sidorin wrote:
> > > > > > > > What will happen if we write something like this:
> > > > > > > > ```
> > > > > > > > bool Eq1 = it1 == it2;
> > > > > > > > bool Eq2 = it3 == it4;
> > > > > > > > if (Eq1) {...}?
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > As I understand, we'll assume the second condition instead of 
> > > > > > > > first.
> > > > > > > Had a look. So the problem is, we obtain the result of the 
> > > > > > > comparison as a symbol, from which it is too hard to recover the 
> > > > > > > operands in order to move iterator position data from one value 
> > > > > > > to another.
> > > > > > > 
> > > > > > > Normally we obtain a simple SymbolConjured for the return value 
> > > > > > > of the `operator==()` call (or, similarly, `operator!=()`). For 
> > > > > > > plain-value iterators (eg. `typedef T *iterator`) we might be 
> > > > > > > obtaining an actual binary symbolic expression, but even then 
> > > > > > > it's not quite clear how to obtain operands (the structure of the 
> > > > > > > expression might have changed due to algebraic simplifications). 
> > > > > > > Additionally, LHS and RHS aren't necessarily symbols (they might 
> > > > > > > be semi-concrete), so composing symbolic expressions from them in 
> > > > > > > general is problematic with our symbol hierarchy, which is rarely 
> > > > > > > a problem for numbers but for structural symbols it'd be a mess.
> > > > > > > 
> > > > > > > For now i suggest, instead of storing only the last LHS and RHS, 
> > > > > > > to save a map from symbols (which are results of comparisons) to 
> > > > > > > (LHS value, RHS value) pairs. This map should, apart from the 
> > > > > > > obvious, be cleaned up whenever one of the iterators in the pair 
> > > > > > > gets mutated (incremented or decremented). This should take care 
> > > > > > > of the problem Alexey points out, and should work with 
> > > > > > > semi-concrete stuff.
> > > > > > > 
> > > > > > > For the future i suggest to let users construct their own symbols 
> > > > > > > and symbolic expressions more easily. In fact, if only we had all 
> > > > > > > iterators as regions, we should have probably used SymbolMetadata 
> > > > > > > for this purpose: it's easy to both recover the parent region 
> > > > > > > from it and use it in symbolic expressions. We could also 
> > > > > > > deprecate the confusing structural symbols (provide default-bound 
> > > > > > > lazy compound values for conjured structures instead), and then 
> > > > > > > it'd be possible to transition to SymbolMetadata entirely.
> > > > > > Thank you for the suggestion. I am not sure if I fully understand 
> > > > > > you. If I create a map where the key is the resulting symbol of the 
> > > > > > comparison, it will not work because evalAssume is called for the 
> > > > > > innermost comparison. So if the body of operator== (or operator!=) 
> > > > > > is inlined, then I get a binary symbolic expression in evalAssume, 
> > > > > > not the SymbolConjured. This binary Symbolic expression is a 
> > > > > > comparison of the internals of the iterators, e.g. the internal 
> > > > > > pointer. So the key will not match any LHS and RHS value pair in 
> > > > > > the map. I also thought on such solution earlier but I dismissed it 
> > > > > > because of this.
> > > > > Well, even if the body of the comparison operator is inlined, 
> > > > > PreStmt/PostStmt callbacks should still work, and it doesn't really 
> > > > > matter if there's a `SymbolConjured` or not, we can still add the 
> > > > > symbolic expression to our map as a key.
> > > > > 
> > > > > Essentially, you ignore whatever happens in the iterator's 
> > > > > operator==() when it's inlined (including any evalAssume events), 
> > > > > then in PostStmt of operator==() you map the return-value symbol of 
> > > > > the operator to the operator's arguments (operands), then whenever an 
> > > > > assumption is being made against the return-value symbol, you carry 
> > > > > over this assumption to the operands. I think it shouldn't really 
> > > > > matter if the operator call was inlined.
> > > > > 
> > > > > The only unexpected thing that may happen due to inlining is if the 
> > > > > inlined operator returns a concrete value (plain true or plain false) 
> > > > > instead of the symbol, but in this case what we need to do is to just 
> > > > > carry over the assumption to the operands instantly.
> > > > Maybe if I evaluate the operator==() call for iterators using 
> > > > evalCall()?

r287238 - Sema: correct typo correction for ivars in @implementation

2016-11-17 Thread Saleem Abdulrasool via cfe-commits
Author: compnerd
Date: Thu Nov 17 11:10:54 2016
New Revision: 287238

URL: http://llvm.org/viewvc/llvm-project?rev=287238&view=rev
Log:
Sema: correct typo correction for ivars in @implementation

The previous typo correction handling assumed that ivars are only declared in
the interface declaration rather than as a private ivar in the implementation.
Adjust the handling to permit both interfaces.  Assert earlier that the
interface has been acquired to ensure that we can identify when both possible
casts have failed.

Addresses PR31040!

Modified:
cfe/trunk/lib/Sema/SemaExprMember.cpp
cfe/trunk/test/SemaObjC/typo-correction.m

Modified: cfe/trunk/lib/Sema/SemaExprMember.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprMember.cpp?rev=287238&r1=287237&r2=287238&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprMember.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprMember.cpp Thu Nov 17 11:10:54 2016
@@ -1394,10 +1394,17 @@ static ExprResult LookupMemberExpr(Sema
 
 // Figure out the class that declares the ivar.
 assert(!ClassDeclared);
+
 Decl *D = cast(IV->getDeclContext());
-if (ObjCCategoryDecl *CAT = dyn_cast(D))
-  D = CAT->getClassInterface();
-ClassDeclared = cast(D);
+if (auto *Category = dyn_cast(D))
+  D = Category->getClassInterface();
+
+if (auto *Implementation = dyn_cast(D))
+  ClassDeclared = Implementation->getClassInterface();
+else if (auto *Interface = dyn_cast(D))
+  ClassDeclared = Interface;
+
+assert(ClassDeclared && "cannot query interface");
   } else {
 if (IsArrow &&
 IDecl->FindPropertyDeclaration(

Modified: cfe/trunk/test/SemaObjC/typo-correction.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/typo-correction.m?rev=287238&r1=287237&r2=287238&view=diff
==
--- cfe/trunk/test/SemaObjC/typo-correction.m (original)
+++ cfe/trunk/test/SemaObjC/typo-correction.m Thu Nov 17 11:10:54 2016
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -fsyntax-only
+// RUN: %clang_cc1 %s -verify -fsyntax-only -fobjc-runtime=ios
 
 @protocol P
 -(id)description;
@@ -28,3 +28,26 @@ typedef int super1;
   [self foo:[super description] other:someivar]; // expected-error {{use of 
undeclared identifier 'someivar'; did you mean '_someivar'?}}
 }
 @end
+
+__attribute__ (( __objc_root_class__ ))
+@interface I {
+  id _interface; // expected-note {{'_interface' declared here}}
+}
+-(void)method;
+@end
+
+@interface I () {
+  id _extension; // expected-note {{'_extension' declared here}}
+}
+@end
+
+@implementation I {
+  id _implementation; // expected-note {{'_implementation' declared here}}
+}
+-(void)method {
+  (void)self->implementation; // expected-error {{'I' does not have a member 
named 'implementation'; did you mean '_implementation'?}}
+  (void)self->interface; // expected-error {{'I' does not have a member named 
'interface'; did you mean '_interface'?}}
+  (void)self->extension; // expected-error {{'I' does not have a member named 
'extension'; did you mean '_extension'?}}
+}
+@end
+


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


[PATCH] D26435: Use unique_ptr for cached tokens for default arguments in C++.

2016-11-17 Thread Malcolm Parsons via cfe-commits
malcolm.parsons accepted this revision.
malcolm.parsons added a comment.
This revision is now accepted and ready to land.

LGTM.


https://reviews.llvm.org/D26435



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


[PATCH] D26435: Use unique_ptr for cached tokens for default arguments in C++.

2016-11-17 Thread David Tarditi via cfe-commits
dtarditi updated this revision to Diff 78375.
dtarditi added a comment.

The parameter array needed to be initialized so that assignments involving 
unique pointers work properly.  The memory could be uninitialized according to 
C++ semantics..  Visual C++ was zeroing the memory and GCC was not.  This is 
why the tests passed on Windows and failed on Linux.  I updated 
Sema/DeclSpec.cpp to zero the parameter array before it is used.

Here are the test results from an x64 Linux Ubuntu box:

Testing Time: 465.12s

  Expected Passes: 10017
  Expected Failures  : 18
  Unsupported Tests  : 27


https://reviews.llvm.org/D26435

Files:
  include/clang/Parse/Parser.h
  include/clang/Sema/DeclSpec.h
  lib/Parse/ParseCXXInlineMethods.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaDeclCXX.cpp

Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -395,17 +395,15 @@
++argIdx) {
 ParmVarDecl *Param = cast(chunk.Fun.Params[argIdx].Param);
 if (Param->hasUnparsedDefaultArg()) {
-  CachedTokens *Toks = chunk.Fun.Params[argIdx].DefaultArgTokens;
+  std::unique_ptr Toks = std::move(chunk.Fun.Params[argIdx].DefaultArgTokens);
   SourceRange SR;
   if (Toks->size() > 1)
 SR = SourceRange((*Toks)[1].getLocation(),
  Toks->back().getLocation());
   else
 SR = UnparsedDefaultArgLocs[Param];
   Diag(Param->getLocation(), diag::err_param_default_argument_nonfunc)
 << SR;
-  delete Toks;
-  chunk.Fun.Params[argIdx].DefaultArgTokens = nullptr;
 } else if (Param->getDefaultArg()) {
   Diag(Param->getLocation(), diag::err_param_default_argument_nonfunc)
 << Param->getDefaultArg()->getSourceRange();
Index: lib/Sema/DeclSpec.cpp
===
--- lib/Sema/DeclSpec.cpp
+++ lib/Sema/DeclSpec.cpp
@@ -223,13 +223,19 @@
 if (!TheDeclarator.InlineStorageUsed &&
 NumParams <= llvm::array_lengthof(TheDeclarator.InlineParams)) {
   I.Fun.Params = TheDeclarator.InlineParams;
+  // Zero the memory block so that unique pointers are initialized
+  // properly.
+  memset(I.Fun.Params, 0, sizeof(Params[0]) * NumParams);
   I.Fun.DeleteParams = false;
   TheDeclarator.InlineStorageUsed = true;
 } else {
-  I.Fun.Params = new DeclaratorChunk::ParamInfo[NumParams];
+  // Call the version of new that zeroes memory so that unique pointers
+  // are initialized properly.
+  I.Fun.Params = new DeclaratorChunk::ParamInfo[NumParams]();
   I.Fun.DeleteParams = true;
 }
-memcpy(I.Fun.Params, Params, sizeof(Params[0]) * NumParams);
+for (unsigned i = 0; i < NumParams; i++)
+  I.Fun.Params[i] = std::move(Params[i]);
   }
 
   // Check what exception specification information we should actually store.
Index: lib/Parse/ParseDeclCXX.cpp
===
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -2039,7 +2039,7 @@
 LateMethod->DefaultArgs.reserve(FTI.NumParams);
 for (unsigned ParamIdx = 0; ParamIdx < FTI.NumParams; ++ParamIdx)
   LateMethod->DefaultArgs.push_back(LateParsedDefaultArgument(
-FTI.Params[ParamIdx].Param, FTI.Params[ParamIdx].DefaultArgTokens));
+FTI.Params[ParamIdx].Param, std::move(FTI.Params[ParamIdx].DefaultArgTokens)));
   }
 }
 
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -6022,7 +6022,7 @@
 
 // DefArgToks is used when the parsing of default arguments needs
 // to be delayed.
-CachedTokens *DefArgToks = nullptr;
+std::unique_ptr DefArgToks;
 
 // If no parameter was specified, verify that *something* was specified,
 // otherwise we have a missing type and identifier.
@@ -6058,13 +6058,11 @@
   // If we're inside a class definition, cache the tokens
   // corresponding to the default argument. We'll actually parse
   // them when we see the end of the class definition.
-  // FIXME: Can we use a smart pointer for Toks?
-  DefArgToks = new CachedTokens;
+  DefArgToks.reset(new CachedTokens);
 
   SourceLocation ArgStartLoc = NextToken().getLocation();
   if (!ConsumeAndStoreInitializer(*DefArgToks, CIK_DefaultArgument)) {
-delete DefArgToks;
-DefArgToks = nullptr;
+DefArgToks.reset();
 Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
   } else {
 Actions.ActOnParamUnparsedDefaultArgument(Param, EqualLoc,
@@ -6100,7 +6098,7 @@
 
   ParamInfo.push_back(DeclaratorChunk::ParamInfo(ParmII,
  

[PATCH] D26796: [Driver] Use arch type to find compiler-rt libraries (on Linux)

2016-11-17 Thread Michał Górny via cfe-commits
mgorny added inline comments.



Comment at: test/Driver/print-libgcc-file-name-clangrt.c:11
 // RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-I686 %s
-// CHECK-CLANGRT-I686: libclang_rt.builtins-i686.a
+// CHECK-CLANGRT-I686: libclang_rt.builtins-i386.a

jroelofs wrote:
> This CHECK's name is now a lie.
Right. I'll change it.


https://reviews.llvm.org/D26796



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


Re: [PATCH] D24289: Add warning when assigning enums to bitfields without an explicit unsigned underlying type

2016-11-17 Thread Richard Smith via cfe-commits
On 17 Nov 2016 8:56 am, "Reid Kleckner"  wrote:

rnk added a comment.

In https://reviews.llvm.org/D24289#598169, @rsmith wrote:

> This is causing warnings to fire for headers shared between C and C++,
where the "give the enum an unsigned underlying type" advice doesn't work,
and where the code in question will never be built for the MS ABI. It seems
really hard to justify this being on by default.
>
> I'm going to turn it off by default for now, but we should probably
consider turning it back on by default when targeting the MS ABI (as a
"your code is wrong" warning rather than a "your code is not portable"
warning).


Seems reasonable. I asked to make it off by default, but got argued down to
putting it under -Wall.

> Yeah, suggesting adding an underlying type to the enum to solve this
problem seems like a bad idea, since that fundamentally changes the nature
of the enum -- typically allowing it to store a lot more values, and making
putting it in a bitfield a bad idea.

Any time you use a bitfield it stores fewer values than the original
integer type. I don't see how enums are special here. Even if I can store
values in the enum not representable as a bitwise combination of
enumerators, people usually don't, and adding an underlying type doesn't
change the conventions of normal C++ code.


The range of representable values for a bitfield with no fixed underlying
type is actually smaller than that of its underlying type. See
http://eel.is/c++draft/dcl.enum#8

So a bitfield of width equal to the number of bits needed to store any
enumerator does not have fewer values than the original type.

Do you have any better suggestions for people that want this code to do the
right thing when built with MSVC?

  enum E /* : unsigned */ { E0, E1, E2, E3 };
  struct A { E b : 2; };
  int main() {
A a;
a.b = E3;
return a.b; // comes out as -1 without the underlying type
  }

Widening the bitfield wastes a bit. Making the bitfield a plain integer and
cast in and out of the enum type, but that obscures the true type of the
bitfield. So, I still support this suggestion.


How about we consider msvc's behaviour to just be a bug, and zero-extend
loads from enum bitfields with no negative enumerators? Since loading such
a negative value results in UB, this wouldn't change the meaning of any
program with defined behaviour, and still respects the MS ABI.

Repository:
  rL LLVM

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


[PATCH] D26544: [PPC] support for arithmetic builtins in the FE

2016-11-17 Thread Ehsan Amiri via cfe-commits
amehsan updated the summary for this revision.
amehsan updated this revision to Diff 78376.

https://reviews.llvm.org/D26544

Files:
  lib/Headers/altivec.h
  test/CodeGen/builtins-ppc-altivec.c
  test/CodeGen/builtins-ppc-p8vector.c
  test/CodeGen/builtins-ppc-quadword.c
  test/CodeGen/builtins-ppc-vsx.c

Index: test/CodeGen/builtins-ppc-vsx.c
===
--- test/CodeGen/builtins-ppc-vsx.c
+++ test/CodeGen/builtins-ppc-vsx.c
@@ -69,6 +69,18 @@
 // CHECK: call <4 x float> @llvm.fabs.v4f32(<4 x float> %{{[0-9]*}})
 // CHECK-LE: call <4 x float> @llvm.fabs.v4f32(<4 x float> %{{[0-9]*}})
 
+  res_vd = vec_abs(vd);
+// CHECK: call <2 x double> @llvm.fabs.v2f64(<2 x double> %{{[0-9]*}})
+// CHECK-LE: call <2 x double> @llvm.fabs.v2f64(<2 x double> %{{[0-9]*}})
+
+  res_vf = vec_nabs(vf);
+// CHECK: [[VEC:%[0-9]+]] = call <4 x float> @llvm.fabs.v4f32(<4 x float> %{{[0-9]*}})
+// CHECK-NEXT: fsub <4 x float> , [[VEC]]
+
+  res_vd = vec_nabs(vd);
+// CHECK: [[VECD:%[0-9]+]] = call <2 x double> @llvm.fabs.v2f64(<2 x double> %{{[0-9]*}})
+// CHECK: fsub <2 x double> , [[VECD]]
+
   dummy();
 // CHECK: call void @dummy()
 // CHECK-LE: call void @dummy()
@@ -1080,4 +1092,12 @@
 // CHECK: fmul <2 x double>
 // CHECK-LE: uitofp <2 x i64> %{{.*}} to <2 x double>
 // CHECK-LE: fmul <2 x double>
+
+  res_vf = vec_neg(vf);
+// CHECK: fsub <4 x float> , {{%[0-9]+}}
+// CHECK-LE: fsub <4 x float> , {{%[0-9]+}}
+
+  res_vd = vec_neg(vd);
+// CHECK: fsub <2 x double> , {{%[0-9]+}}
+// CHECK-LE: fsub <2 x double> , {{%[0-9]+}}
 }
Index: test/CodeGen/builtins-ppc-quadword.c
===
--- test/CodeGen/builtins-ppc-quadword.c
+++ test/CodeGen/builtins-ppc-quadword.c
@@ -119,11 +119,32 @@
 // CHECK-LE: @llvm.ppc.altivec.vsubeuqm
 // CHECK-PPC: error: assigning to '__vector __int128' (vector of 1 '__int128' value) from incompatible type 'int'
   
+  /* vec_sube */
+  res_vlll = vec_sube(vlll, vlll, vlll);
+// CHECK: @llvm.ppc.altivec.vsubeuqm
+// CHECK-LE: @llvm.ppc.altivec.vsubeuqm
+// CHECK-PPC: error: call to 'vec_sube' is ambiguous
+  
+  res_vulll = vec_sube(vulll, vulll, vulll);
+// CHECK: @llvm.ppc.altivec.vsubeuqm
+// CHECK-LE: @llvm.ppc.altivec.vsubeuqm
+// CHECK-PPC: error: call to 'vec_sube' is ambiguous
+  
+  res_vlll = vec_sube(vlll, vlll, vlll);
+// CHECK: @llvm.ppc.altivec.vsubeuqm
+// CHECK-LE: @llvm.ppc.altivec.vsubeuqm
+// CHECK-PPC: error: call to 'vec_sube' is ambiguous
+  
   res_vulll = vec_vsubeuqm(vulll, vulll, vulll);
 // CHECK: @llvm.ppc.altivec.vsubeuqm
 // CHECK-LE: @llvm.ppc.altivec.vsubeuqm
 // CHECK-PPC: error: assigning to '__vector unsigned __int128' (vector of 1 'unsigned __int128' value) from incompatible type 'int'
-  
+
+  res_vulll = vec_sube(vulll, vulll, vulll);
+// CHECK: @llvm.ppc.altivec.vsubeuqm
+// CHECK-LE: @llvm.ppc.altivec.vsubeuqm
+// CHECK-PPC: error: call to 'vec_sube' is ambiguous
+
   /* vec_subc */
   res_vlll = vec_subc(vlll, vlll);
 // CHECK: @llvm.ppc.altivec.vsubcuq
@@ -150,11 +171,21 @@
   res_vlll = vec_vsubecuq(vlll, vlll, vlll);
 // CHECK: @llvm.ppc.altivec.vsubecuq
 // CHECK-LE: @llvm.ppc.altivec.vsubecuq
-// CHECK-PPC: error: assigning to '__vector __int128' (vector of 1 '__int128' value) from incompatible type 'int'  
+// CHECK-PPC: error: assigning to '__vector __int128' (vector of 1 '__int128' value) from incompatible type 'int'
 
   res_vulll = vec_vsubecuq(vulll, vulll, vulll);
 // CHECK: @llvm.ppc.altivec.vsubecuq
 // CHECK-LE: @llvm.ppc.altivec.vsubecuq
+// CHECK-PPC: error: assigning to '__vector unsigned __int128' (vector of 1 'unsigned __int128' value) from incompatible type 'int'
+
+  res_vlll = vec_subec(vlll, vlll, vlll);
+// CHECK: @llvm.ppc.altivec.vsubecuq
+// CHECK-LE: @llvm.ppc.altivec.vsubecuq
+// CHECK-PPC: error: assigning to '__vector __int128' (vector of 1 '__int128' value) from incompatible type 'int'  
+
+  res_vulll = vec_subec(vulll, vulll, vulll);
+// CHECK: @llvm.ppc.altivec.vsubecuq
+// CHECK-LE: @llvm.ppc.altivec.vsubecuq
 // CHECK-PPC: error: assigning to '__vector unsigned __int128' (vector of 1 'unsigned __int128' value) from incompatible type 'int'  
 
 }
Index: test/CodeGen/builtins-ppc-p8vector.c
===
--- test/CodeGen/builtins-ppc-p8vector.c
+++ test/CodeGen/builtins-ppc-p8vector.c
@@ -73,13 +73,6 @@
 // CHECK-LE: call <2 x i64> @llvm.ppc.altivec.vmaxsd(<2 x i64> %{{[0-9]*}}, <2 x i64>
 // CHECK-PPC: error: call to 'vec_abs' is ambiguous
 
-  res_vd = vec_abs(vda);
-// CHECK: call <2 x double> @llvm.fabs.v2f64(<2 x double> %{{.*}})
-// CHECK: store <2 x double> %{{.*}}, <2 x double>* @res_vd
-// CHECK-LE: call <2 x double> @llvm.fabs.v2f64(<2 x double> %{{.*}})
-// CHECK-LE: store <2 x double> %{{.*}}, <2 x double>* @res_vd
-// CHECK-PPC: error: call to 'vec_abs' is ambiguous
-
   /* vec_add */
   res_vsll = vec_add(vsll, vsll);
 // CHECK: add <2 x i64>
@@ -1

[PATCH] D26564: Use PIC relocation mode by default for PowerPC64 ELF

2016-11-17 Thread Ehsan Amiri via cfe-commits
amehsan added inline comments.



Comment at: lib/Driver/ToolChains.cpp:2799-2807
+  switch (getArch()) {
+  case llvm::Triple::x86_64:
+return getTriple().isOSWindows();
+  case llvm::Triple::ppc64:
+  case llvm::Triple::ppc64le:
+return !getTriple().isOSBinFormatMachO() && !getTriple().isMacOSX();
+  default:

This is a minor nit. Almost any switch statement that I have come across in 
LLVM code, has default in the very beginning instead of the end. At least for 
the sake of consistency this is better to change here. (The reason that I have 
heard for it, is readability for large switch statements. But in the codebase, 
even stmts of this size, have default first).


https://reviews.llvm.org/D26564



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


r287241 - Use unique_ptr for cached tokens for default arguments in C++.

2016-11-17 Thread Malcolm Parsons via cfe-commits
Author: malcolm.parsons
Date: Thu Nov 17 11:52:58 2016
New Revision: 287241

URL: http://llvm.org/viewvc/llvm-project?rev=287241&view=rev
Log:
Use unique_ptr for cached tokens for default arguments in C++.

Summary:
This changes pointers to cached tokens for default arguments in C++ from raw 
pointers to unique_ptrs.  There was a fixme in the code where the cached tokens 
are created  about using a smart pointer.

The change is straightforward, though I did have to track down and fix a memory 
corruption caused by the change.  memcpy was being used to copy parameter 
information.  This duplicated the unique_ptr, which led to the cached token 
buffer being deleted prematurely.

Patch by David Tarditi!

Reviewers: malcolm.parsons

Subscribers: arphaman, malcolm.parsons, cfe-commits

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

Modified:
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/DeclSpec.h
cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Sema/DeclSpec.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=287241&r1=287240&r2=287241&view=diff
==
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Thu Nov 17 11:52:58 2016
@@ -1017,8 +1017,8 @@ private:
   /// (C++ [class.mem]p2).
   struct LateParsedDefaultArgument {
 explicit LateParsedDefaultArgument(Decl *P,
-   CachedTokens *Toks = nullptr)
-  : Param(P), Toks(Toks) { }
+   std::unique_ptr Toks = 
nullptr)
+  : Param(P), Toks(std::move(Toks)) { }
 
 /// Param - The parameter declaration for this parameter.
 Decl *Param;
@@ -1027,7 +1027,7 @@ private:
 /// argument expression, not including the '=' or the terminating
 /// ')' or ','. This will be NULL for parameters that have no
 /// default argument.
-CachedTokens *Toks;
+std::unique_ptr Toks;
   };
 
   /// LateParsedMethodDeclaration - A method declaration inside a class that

Modified: cfe/trunk/include/clang/Sema/DeclSpec.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/DeclSpec.h?rev=287241&r1=287240&r2=287241&view=diff
==
--- cfe/trunk/include/clang/Sema/DeclSpec.h (original)
+++ cfe/trunk/include/clang/Sema/DeclSpec.h Thu Nov 17 11:52:58 2016
@@ -1188,14 +1188,14 @@ struct DeclaratorChunk {
 /// declaration of a member function), it will be stored here as a
 /// sequence of tokens to be parsed once the class definition is
 /// complete. Non-NULL indicates that there is a default argument.
-CachedTokens *DefaultArgTokens;
+std::unique_ptr DefaultArgTokens;
 
 ParamInfo() = default;
 ParamInfo(IdentifierInfo *ident, SourceLocation iloc,
   Decl *param,
-  CachedTokens *DefArgTokens = nullptr)
+  std::unique_ptr DefArgTokens = nullptr)
   : Ident(ident), IdentLoc(iloc), Param(param),
-DefaultArgTokens(DefArgTokens) {}
+DefaultArgTokens(std::move(DefArgTokens)) {}
   };
 
   struct TypeAndRange {
@@ -1310,10 +1310,8 @@ struct DeclaratorChunk {
 ///
 /// This is used in various places for error recovery.
 void freeParams() {
-  for (unsigned I = 0; I < NumParams; ++I) {
-delete Params[I].DefaultArgTokens;
-Params[I].DefaultArgTokens = nullptr;
-  }
+  for (unsigned I = 0; I < NumParams; ++I)
+Params[I].DefaultArgTokens.reset();
   if (DeleteParams) {
 delete[] Params;
 DeleteParams = false;

Modified: cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp?rev=287241&r1=287240&r2=287241&view=diff
==
--- cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp (original)
+++ cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp Thu Nov 17 11:52:58 2016
@@ -319,7 +319,8 @@ void Parser::ParseLexedMethodDeclaration
 // Introduce the parameter into scope.
 bool HasUnparsed = Param->hasUnparsedDefaultArg();
 Actions.ActOnDelayedCXXMethodParameter(getCurScope(), Param);
-if (CachedTokens *Toks = LM.DefaultArgs[I].Toks) {
+std::unique_ptr Toks = std::move(LM.DefaultArgs[I].Toks);
+if (Toks) {
   // Mark the end of the default argument so that we know when to stop when
   // we parse it later on.
   Token LastDefaultArgToken = Toks->back();
@@ -377,9 +378,6 @@ void Parser::ParseLexedMethodDeclaration
 
   if (Tok.is(tok::eof) && Tok.getEofData() == Param)
 ConsumeAnyToken();
-
-  delete Toks;
-

[PATCH] D26782: [libcxx] [test] Test changes for P0504R0 "Revisiting in-place tag types for any/optional/variant"

2016-11-17 Thread Casey Carter via cfe-commits
CaseyCarter marked an inline comment as done.
CaseyCarter added a comment.

> Do these tests pass with the current `` implementation, or will they 
> have to wait?

These tests **do not pass** without making the changes required in P0504R0 to 
`` and ``.  (Interestingly  is unaffected; its use of 
`in_place_t` and `in_place` is source-compatible despite the changed 
definitions of those names.) I would have made those changes as well, but my 
request for permission to contribute changes to non-test code hasn't yet 
returned from the void into which I cast it. If neither of you get around to 
it, I may put an hour into it over the weekend.




Comment at: test/std/utilities/utility/utility.inplace/inplace.pass.cpp:40
 
-template 
-struct CheckRet : std::false_type {};
-template 
-struct CheckRet : std::true_type {};
+#define STATIC_ASSERT(...) static_assert((__VA_ARGS__), #__VA_ARGS__)
 

mclow.lists wrote:
> Please just use `static_assert(x)` instead.  Since this is C++1z only, you 
> don't need to supply a message. If the test fails, the compiler will give you 
> file and line #.
> 
> If I see an all caps `STATIC_ASSERT`, then my first bit of research has to be 
> "how is this different from `static_assert`?"
> 
`static_assert(false);` in Visual C++ diagnoses with the incredibly informative:

> inplace.pass.cpp(59): error C2607: static assertion failed

so I've developed this habit which inadvertently leaked into the diff. Fixing.


https://reviews.llvm.org/D26782



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


[PATCH] D26435: Use unique_ptr for cached tokens for default arguments in C++.

2016-11-17 Thread Malcolm Parsons via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL287241: Use unique_ptr for cached tokens for default 
arguments in C++. (authored by malcolm.parsons).

Changed prior to commit:
  https://reviews.llvm.org/D26435?vs=78375&id=78380#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26435

Files:
  cfe/trunk/include/clang/Parse/Parser.h
  cfe/trunk/include/clang/Sema/DeclSpec.h
  cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
  cfe/trunk/lib/Parse/ParseDecl.cpp
  cfe/trunk/lib/Parse/ParseDeclCXX.cpp
  cfe/trunk/lib/Sema/DeclSpec.cpp
  cfe/trunk/lib/Sema/SemaDeclCXX.cpp

Index: cfe/trunk/include/clang/Sema/DeclSpec.h
===
--- cfe/trunk/include/clang/Sema/DeclSpec.h
+++ cfe/trunk/include/clang/Sema/DeclSpec.h
@@ -1188,14 +1188,14 @@
 /// declaration of a member function), it will be stored here as a
 /// sequence of tokens to be parsed once the class definition is
 /// complete. Non-NULL indicates that there is a default argument.
-CachedTokens *DefaultArgTokens;
+std::unique_ptr DefaultArgTokens;
 
 ParamInfo() = default;
 ParamInfo(IdentifierInfo *ident, SourceLocation iloc,
   Decl *param,
-  CachedTokens *DefArgTokens = nullptr)
+  std::unique_ptr DefArgTokens = nullptr)
   : Ident(ident), IdentLoc(iloc), Param(param),
-DefaultArgTokens(DefArgTokens) {}
+DefaultArgTokens(std::move(DefArgTokens)) {}
   };
 
   struct TypeAndRange {
@@ -1310,10 +1310,8 @@
 ///
 /// This is used in various places for error recovery.
 void freeParams() {
-  for (unsigned I = 0; I < NumParams; ++I) {
-delete Params[I].DefaultArgTokens;
-Params[I].DefaultArgTokens = nullptr;
-  }
+  for (unsigned I = 0; I < NumParams; ++I)
+Params[I].DefaultArgTokens.reset();
   if (DeleteParams) {
 delete[] Params;
 DeleteParams = false;
Index: cfe/trunk/include/clang/Parse/Parser.h
===
--- cfe/trunk/include/clang/Parse/Parser.h
+++ cfe/trunk/include/clang/Parse/Parser.h
@@ -1017,17 +1017,17 @@
   /// (C++ [class.mem]p2).
   struct LateParsedDefaultArgument {
 explicit LateParsedDefaultArgument(Decl *P,
-   CachedTokens *Toks = nullptr)
-  : Param(P), Toks(Toks) { }
+   std::unique_ptr Toks = nullptr)
+  : Param(P), Toks(std::move(Toks)) { }
 
 /// Param - The parameter declaration for this parameter.
 Decl *Param;
 
 /// Toks - The sequence of tokens that comprises the default
 /// argument expression, not including the '=' or the terminating
 /// ')' or ','. This will be NULL for parameters that have no
 /// default argument.
-CachedTokens *Toks;
+std::unique_ptr Toks;
   };
 
   /// LateParsedMethodDeclaration - A method declaration inside a class that
Index: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
===
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp
@@ -2039,7 +2039,7 @@
 LateMethod->DefaultArgs.reserve(FTI.NumParams);
 for (unsigned ParamIdx = 0; ParamIdx < FTI.NumParams; ++ParamIdx)
   LateMethod->DefaultArgs.push_back(LateParsedDefaultArgument(
-FTI.Params[ParamIdx].Param, FTI.Params[ParamIdx].DefaultArgTokens));
+FTI.Params[ParamIdx].Param, std::move(FTI.Params[ParamIdx].DefaultArgTokens)));
   }
 }
 
Index: cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
===
--- cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
+++ cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
@@ -319,7 +319,8 @@
 // Introduce the parameter into scope.
 bool HasUnparsed = Param->hasUnparsedDefaultArg();
 Actions.ActOnDelayedCXXMethodParameter(getCurScope(), Param);
-if (CachedTokens *Toks = LM.DefaultArgs[I].Toks) {
+std::unique_ptr Toks = std::move(LM.DefaultArgs[I].Toks);
+if (Toks) {
   // Mark the end of the default argument so that we know when to stop when
   // we parse it later on.
   Token LastDefaultArgToken = Toks->back();
@@ -377,9 +378,6 @@
 
   if (Tok.is(tok::eof) && Tok.getEofData() == Param)
 ConsumeAnyToken();
-
-  delete Toks;
-  LM.DefaultArgs[I].Toks = nullptr;
 } else if (HasUnparsed) {
   assert(Param->hasInheritedDefaultArg());
   FunctionDecl *Old = cast(LM.Method)->getPreviousDecl();
Index: cfe/trunk/lib/Parse/ParseDecl.cpp
===
--- cfe/trunk/lib/Parse/ParseDecl.cpp
+++ cfe/trunk/lib/Parse/ParseDecl.cpp
@@ -6022,7 +6022,7 @@
 
 // DefArgToks is used when the parsing of default arguments needs
 // to be delayed.
-CachedTokens *DefArgToks = nullptr;
+std::unique_ptr DefArgToks;
 
 // If no

[PATCH] D26796: [Driver] Use arch type to find compiler-rt libraries (on Linux)

2016-11-17 Thread Michał Górny via cfe-commits
mgorny updated this revision to Diff 78378.
mgorny marked 2 inline comments as done.
mgorny added a comment.

Updated the -print-libgcc-file-name test name. Additionally, I've added another 
subvariant of that test using i686-* target to ensure that the mapping works 
for that function too, in case we ever decide to split it.


https://reviews.llvm.org/D26796

Files:
  lib/Driver/ToolChain.cpp
  test/Driver/Inputs/basic_linux_tree/usr/i686-unknown-linux/lib/.keep
  
test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/i686-unknown-linux/4.6.0/crtbegin.o
  test/Driver/linux-ld.c
  test/Driver/nostdlib.c
  test/Driver/print-libgcc-file-name-clangrt.c
  test/Driver/windows-cross.c

Index: test/Driver/windows-cross.c
===
--- test/Driver/windows-cross.c
+++ test/Driver/windows-cross.c
@@ -59,7 +59,7 @@
 // RUN:| FileCheck %s --check-prefix CHECK-SANITIZE-ADDRESS-EXE-X86
 
 // CHECK-SANITIZE-ADDRESS-EXE-X86: "-fsanitize=address"
-// CHECK-SANITIZE-ADDRESS-EXE-X86: "{{.*}}clang_rt.asan_dynamic-i686.lib" "{{.*}}clang_rt.asan_dynamic_runtime_thunk-i686.lib" "--undefined" "___asan_seh_interceptor"
+// CHECK-SANITIZE-ADDRESS-EXE-X86: "{{.*}}clang_rt.asan_dynamic-i386.lib" "{{.*}}clang_rt.asan_dynamic_runtime_thunk-i386.lib" "--undefined" "___asan_seh_interceptor"
 
 // RUN: %clang -### -target armv7-windows-itanium --sysroot %S/Inputs/Windows/ARM/8.1 -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=lld-link2 -shared -o shared.dll -fsanitize=tsan -x c++ %s 2>&1 \
 // RUN:| FileCheck %s --check-prefix CHECK-SANITIZE-TSAN
Index: test/Driver/print-libgcc-file-name-clangrt.c
===
--- test/Driver/print-libgcc-file-name-clangrt.c
+++ test/Driver/print-libgcc-file-name-clangrt.c
@@ -6,6 +6,12 @@
 // CHECK-CLANGRT-X8664: libclang_rt.builtins-x86_64.a
 
 // RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \
+// RUN: --target=i386-pc-linux \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-I386 %s
+// CHECK-CLANGRT-I386: libclang_rt.builtins-i386.a
+
+// Check whether alternate arch values map to the correct library.
+//
+// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \
 // RUN: --target=i686-pc-linux \
-// RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-I686 %s
-// CHECK-CLANGRT-I686: libclang_rt.builtins-i686.a
+// RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-I386 %s
Index: test/Driver/nostdlib.c
===
--- test/Driver/nostdlib.c
+++ test/Driver/nostdlib.c
@@ -14,18 +14,18 @@
 // passed down to link line
 // RUN: %clang -no-canonical-prefixes %s -### -Wno-liblto -o %t.o 2>&1 \
 // RUN: -target i686-pc-linux-gnu -nostdlib --rtlib=compiler-rt \
-// RUN: -resource-dir=%S/Inputs/resource_dir -lclang_rt.builtins-i686 \
+// RUN: -resource-dir=%S/Inputs/resource_dir -lclang_rt.builtins-i386 \
 // RUN:   | FileCheck --check-prefix=CHECK-LINUX-NOSTDLIB %s
 //
 // RUN: %clang -no-canonical-prefixes %s -### -Wno-liblto -o %t.o 2>&1 \
 // RUN: -target i686-pc-linux-gnu --rtlib=compiler-rt -nostdlib \
-// RUN: -resource-dir=%S/Inputs/resource_dir -lclang_rt.builtins-i686 \
+// RUN: -resource-dir=%S/Inputs/resource_dir -lclang_rt.builtins-i386 \
 // RUN:   | FileCheck --check-prefix=CHECK-LINUX-NOSTDLIB %s
 //
 // RUN: %clang -target x86_64-pc-windows-msvc -nostdlib --rtlib=compiler-rt -### -Wno-liblto %s 2>&1 | FileCheck %s -check-prefix CHECK-MSVC-NOSTDLIB
 // RUN: %clang -target x86_64-pc-windows-msvc --rtlib=compiler-rt -nostdlib -### -Wno-liblto %s 2>&1 | FileCheck %s -check-prefix CHECK-MSVC-NOSTDLIB
 //
 // CHECK-LINUX-NOSTDLIB: warning: argument unused during compilation: '--rtlib=compiler-rt'
 // CHECK-LINUX-NOSTDLIB: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
-// CHECK-LINUX-NOSTDLIB-NOT: "{{.*}}/Inputs/resource_dir{{/|}}lib{{/|}}linux{{/|}}libclang_rt.builtins-i686.a"
+// CHECK-LINUX-NOSTDLIB-NOT: "{{.*}}/Inputs/resource_dir{{/|}}lib{{/|}}linux{{/|}}libclang_rt.builtins-i386.a"
 // CHECK-MSVC-NOSTDLIB: warning: argument unused during compilation: '--rtlib=compiler-rt'
Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -71,6 +71,27 @@
 // CHECK-LD-RT: libclang_rt.builtins-x86_64.a"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=i686-unknown-linux \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: --rtlib=compiler-rt \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-RT-I686 %s
+// CHECK-LD-RT-I686-NOT: warning:
+// CHECK-LD-RT-I686: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-LD-RT-I686: "--eh-frame-hdr"
+// CHECK-LD-RT-I686: "-m" "elf_i386"
+// CHECK-LD-RT-I686: "-dynamic-linker"
+// CHECK-LD-RT-I686: "{{.*}}/usr/lib/gcc/i686-unknown-linux/4.6.0{{/|}

[PATCH] D26782: [libcxx] [test] Test changes for P0504R0 "Revisiting in-place tag types for any/optional/variant"

2016-11-17 Thread Casey Carter via cfe-commits
CaseyCarter updated this revision to Diff 78379.
CaseyCarter marked an inline comment as done.
CaseyCarter added a comment.

Don't `STATIC_ASSERT`; `static_assert`.


https://reviews.llvm.org/D26782

Files:
  test/std/utilities/any/any.class/any.cons/in_place_type.pass.cpp
  test/std/utilities/any/any.class/any.cons/value.pass.cpp
  test/std/utilities/any/any.nonmembers/any.cast/any_cast_reference.pass.cpp
  test/std/utilities/utility/utility.inplace/inplace.pass.cpp

Index: test/std/utilities/utility/utility.inplace/inplace.pass.cpp
===
--- test/std/utilities/utility/utility.inplace/inplace.pass.cpp
+++ test/std/utilities/utility/utility.inplace/inplace.pass.cpp
@@ -11,21 +11,24 @@
 
 // 
 
-// struct in_place_tag { in_place_tag() = delete; };
-//
-// using in_place_t = in_place_tag(&)(unspecified);
+// struct in_place_t {
+//   explicit in_place_t() = default;
+// };
+// inline constexpr in_place_t in_place{};
+
 // template 
-// using in_place_type_t = in_place_tag(&)(unspecified);
-// template 
-// using in_place_index_t = in_place_tag(&)(unspecified);
-//
-// in_place_tag in_place(unspecified);
-//
-// template ;
-// in_place_tag in_place(unspecified);
-//
-// template 
-// in_place_tag in_place(unspecified);
+//   struct in_place_type_t {
+// explicit in_place_type_t() = default;
+//   };
+// template 
+//   inline constexpr in_place_type_t in_place_type{};
+
+// template 
+//   struct in_place_index_t {
+// explicit in_place_index_t() = default;
+//   };
+// template 
+//   inline constexpr in_place_index_t in_place_index{};
 
 #include 
 #include 
@@ -34,66 +37,38 @@
 #include "test_macros.h"
 #include "type_id.h"
 
-template 
-struct CheckRet : std::false_type {};
-template 
-struct CheckRet : std::true_type {};
-
-TypeID const* test_fn(std::in_place_t) { return &makeTypeID(); }
-template 
-TypeID const* test_fn(std::in_place_type_t)
-{ return &makeTypeID>(); }
-
-template 
-TypeID const* test_fn(std::in_place_index_t)
-{ return &makeTypeID>(); }
-
-// Concrete test overloads that don't have to be deduced.
-template 
-TypeID const* concrete_test_fn(Tag) {  return &makeTypeID(); }
-
-template 
-bool check_tag_basic() {
-  using RawTp = typename std::remove_reference::type;
-  static_assert(std::is_lvalue_reference::value, "");
-  static_assert(std::is_function::value, "");
-  static_assert(CheckRet::value, "");
-  auto concrete_fn = concrete_test_fn;
-  return test_fn((Tp)std::in_place) == &makeTypeID()
-  && concrete_fn(std::in_place) == &makeTypeID();
+template 
+constexpr bool check_tag(Up) {
+return std::is_same>::value
+&& std::is_same::value;
 }
 
 int main() {
-// test in_place_tag
-{
-static_assert(!std::is_default_constructible::value, "");
-}
 // test in_place_t
 {
 using T = std::in_place_t;
-assert(check_tag_basic());
-assert(test_fn((T)std::in_place) == &makeTypeID());
+static_assert(check_tag(std::in_place));
 }
 // test in_place_type_t
 {
 using T1 = std::in_place_type_t;
 using T2 = std::in_place_type_t;
 using T3 = std::in_place_type_t;
-assert(check_tag_basic());
-assert(check_tag_basic());
-assert(check_tag_basic());
-static_assert(!std::is_same::value && !std::is_same::value, "");
-static_assert(!std::is_same::value, "");
+static_assert(!std::is_same::value && !std::is_same::value);
+static_assert(!std::is_same::value);
+static_assert(check_tag(std::in_place_type));
+static_assert(check_tag(std::in_place_type));
+static_assert(check_tag(std::in_place_type));
 }
 // test in_place_index_t
 {
 using T1 = std::in_place_index_t<0>;
 using T2 = std::in_place_index_t<1>;
 using T3 = std::in_place_index_t(-1)>;
-assert(check_tag_basic());
-assert(check_tag_basic());
-assert(check_tag_basic());
-static_assert(!std::is_same::value && !std::is_same::value, "");
-static_assert(!std::is_same::value, "");
+static_assert(!std::is_same::value && !std::is_same::value);
+static_assert(!std::is_same::value);
+static_assert(check_tag(std::in_place_index<0>));
+static_assert(check_tag(std::in_place_index<1>));
+static_assert(check_tag(std::in_place_index(-1)>));
 }
-}
\ No newline at end of file
+}
Index: test/std/utilities/any/any.nonmembers/any.cast/any_cast_reference.pass.cpp
===
--- test/std/utilities/any/any.nonmembers/any.cast/any_cast_reference.pass.cpp
+++ test/std/utilities/any/any.nonmembers/any.cast/any_cast_reference.pass.cpp
@@ -192,7 +192,6 @@
 Type::reset();
 {
 any a((Type(42)));
-any const& ca = a;
 assert(Type::count == 1);
 assert(Type::copied == 0);
 assert(Type::moved == 1);
Index: te

[PATCH] D26794: [OpenCL] Blocks are allowed to capture arrays in OpenCL 2.0 and higher.

2016-11-17 Thread Anastasia Stulova via cfe-commits
Anastasia added a comment.

I have created a bug to Khronos regarding this, but unfortunately I don't see 
it being progressed yet.
https://cvs.khronos.org/bugzilla/show_bug.cgi?id=15659

The problem here is that I am not sure we should deviate from the ObjC 
implementation because OpenCL blocks are largely taken from Clang ObjC 
implementation. My issue is in particular that it's not clear what the capture 
of array would mean and spec should either state it precisely or disallow using 
this feature at all to avoid costly operations. In ObjC community itself there 
were multiple interpretation of this in the past: 
http://lists.llvm.org/pipermail/cfe-dev/2016-March/047849.html

I am not sure we should go ahead with any implementation without further 
clarifications. I will ping the Khronos bug to see if the documentation can be 
improved.

I think this issue has been seen in the OpenCL conformance tests, but was fixed 
later on?


https://reviews.llvm.org/D26794



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


[PATCH] D26773: [analyzer] Refactor recursive symbol reachability check to use symbol_iterator

2016-11-17 Thread Artem Dergachev via cfe-commits
NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:542
+if (!visitor.VisitSymbol(*SI))
+  Result = false;
   }

I guess we should break the loop here.


https://reviews.llvm.org/D26773



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


[PATCH] D26807: Fix a comment for -fsave-optimization-record

2016-11-17 Thread Adam Nemet via cfe-commits
anemet created this revision.
anemet added a reviewer: hfinkel.
anemet added a subscriber: cfe-commits.

https://reviews.llvm.org/D26807

Files:
  lib/Driver/Tools.cpp


Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -6225,7 +6225,7 @@
   Args.hasArg(options::OPT_S))) {
 F = Output.getFilename();
   } else {
-// Use the compilation directory.
+// Use the input filename.
 F = llvm::sys::path::stem(Input.getBaseInput());
 
 // If we're compiling for an offload architecture (i.e. a CUDA device),


Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -6225,7 +6225,7 @@
   Args.hasArg(options::OPT_S))) {
 F = Output.getFilename();
   } else {
-// Use the compilation directory.
+// Use the input filename.
 F = llvm::sys::path::stem(Input.getBaseInput());
 
 // If we're compiling for an offload architecture (i.e. a CUDA device),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D26773: [analyzer] Refactor recursive symbol reachability check to use symbol_iterator

2016-11-17 Thread Artem Dergachev via cfe-commits
NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:535
+if (!isa(*SI))
+  continue;
 

Hmm, the original code does actually visit non-SymbolData.


https://reviews.llvm.org/D26773



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


[PATCH] D26763: [compiler-rt] [asan] Use __SSE2__ (rather than __i686__...) for SSE2 test

2016-11-17 Thread Michał Górny via cfe-commits
mgorny added a comment.

I did, even twice ;-). Thanks.


https://reviews.llvm.org/D26763



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


r287244 - ObjC Module: try to make objc module deterministic.

2016-11-17 Thread Manman Ren via cfe-commits
Author: mren
Date: Thu Nov 17 12:41:18 2016
New Revision: 287244

URL: http://llvm.org/viewvc/llvm-project?rev=287244&view=rev
Log:
ObjC Module: try to make objc module deterministic.

Make sure that comparing selectors in DeclarationName does its job.
rdar://problem/28988750

Added:
cfe/trunk/test/Modules/stress-objc.m
Modified:
cfe/trunk/lib/AST/DeclarationName.cpp

Modified: cfe/trunk/lib/AST/DeclarationName.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclarationName.cpp?rev=287244&r1=287243&r2=287244&view=diff
==
--- cfe/trunk/lib/AST/DeclarationName.cpp (original)
+++ cfe/trunk/lib/AST/DeclarationName.cpp Thu Nov 17 12:41:18 2016
@@ -95,12 +95,18 @@ int DeclarationName::compare(Declaration
   case DeclarationName::ObjCMultiArgSelector: {
 Selector LHSSelector = LHS.getObjCSelector();
 Selector RHSSelector = RHS.getObjCSelector();
+// getNumArgs for ZeroArgSelector returns 0, but we still need to compare.
+if (LHS.getNameKind() == DeclarationName::ObjCZeroArgSelector &&
+RHS.getNameKind() == DeclarationName::ObjCZeroArgSelector) {
+  return LHSSelector.getAsIdentifierInfo()->getName().compare(
+ RHSSelector.getAsIdentifierInfo()->getName());
+}
 unsigned LN = LHSSelector.getNumArgs(), RN = RHSSelector.getNumArgs();
 for (unsigned I = 0, N = std::min(LN, RN); I != N; ++I) {
   switch (LHSSelector.getNameForSlot(I).compare(
RHSSelector.getNameForSlot(I))) 
{
-  case -1: return true;
-  case 1: return false;
+  case -1: return -1;
+  case 1: return 1;
   default: break;
   }
 }

Added: cfe/trunk/test/Modules/stress-objc.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/stress-objc.m?rev=287244&view=auto
==
--- cfe/trunk/test/Modules/stress-objc.m (added)
+++ cfe/trunk/test/Modules/stress-objc.m Thu Nov 17 12:41:18 2016
@@ -0,0 +1,22 @@
+// RUN: cd %S
+
+// RUN: %clang_cc1 -emit-pch -x objective-c-header %s -o %t_c00.pch 
-fno-pch-timestamp
+// RUN: %clang_cc1 -emit-pch -x objective-c-header %s -o %t_c00_1.pch 
-fno-pch-timestamp
+// RUN: diff %t_c00.pch %t_c00_1.pch
+
+// RUN: %clang_cc1 -emit-pch -x objective-c-header %s -o %t_c00_2.pch 
-fno-pch-timestamp
+// RUN: diff %t_c00.pch %t_c00_2.pch
+
+// RUN: %clang_cc1 -emit-pch -x objective-c-header %s -o %t_c00_3.pch 
-fno-pch-timestamp
+// RUN: diff %t_c00.pch %t_c00_3.pch
+
+// RUN: %clang_cc1 -emit-pch -x objective-c-header %s -o %t_c00_4.pch 
-fno-pch-timestamp
+// RUN: diff %t_c00.pch %t_c00_4.pch
+
+// RUN: %clang_cc1 -emit-pch -x objective-c-header %s -o %t_c00_5.pch 
-fno-pch-timestamp
+// RUN: diff %t_c00.pch %t_c00_5.pch
+
+@protocol NSObject
+- (void)doesNotRecognizeSelector:(SEL)aSelector;
+- (id)forwardingTargetForSelector:(SEL)aSelector;
+@end


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


[PATCH] D26763: [compiler-rt] [asan] Use __SSE2__ (rather than __i686__...) for SSE2 test

2016-11-17 Thread Michał Górny via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL287245: [tests] Use __SSE2__ (rather than __i686__...) for 
SSE2 ASAN test (authored by mgorny).

Changed prior to commit:
  https://reviews.llvm.org/D26763?vs=78245&id=78391#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26763

Files:
  compiler-rt/trunk/lib/asan/tests/asan_test.cc


Index: compiler-rt/trunk/lib/asan/tests/asan_test.cc
===
--- compiler-rt/trunk/lib/asan/tests/asan_test.cc
+++ compiler-rt/trunk/lib/asan/tests/asan_test.cc
@@ -692,7 +692,7 @@
   PTHREAD_JOIN(t, 0);
 }
 
-#if defined(__i686__) || defined(__x86_64__)
+#if defined(__SSE2__)
 #include 
 TEST(AddressSanitizer, Store128Test) {
   char *a = Ident((char*)malloc(Ident(12)));


Index: compiler-rt/trunk/lib/asan/tests/asan_test.cc
===
--- compiler-rt/trunk/lib/asan/tests/asan_test.cc
+++ compiler-rt/trunk/lib/asan/tests/asan_test.cc
@@ -692,7 +692,7 @@
   PTHREAD_JOIN(t, 0);
 }
 
-#if defined(__i686__) || defined(__x86_64__)
+#if defined(__SSE2__)
 #include 
 TEST(AddressSanitizer, Store128Test) {
   char *a = Ident((char*)malloc(Ident(12)));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D26774: [CUDA] Driver changes to support CUDA compilation on MacOS.

2016-11-17 Thread Justin Lebar via cfe-commits
jlebar added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:479
+// the device toolchain we create depends on both.
+ToolChain *&CudaTC = ToolChains[CudaTriple.str() + "/" + HostTriple.str()];
+if (!CudaTC) {

sfantao wrote:
> I am not sure I understand why to pair host and device toolchain in the map. 
> The driver can be used to several compilations, but how do these compilation 
> use different host toolchains? Can you give an example of an invocation? 
> Maybe add it to the regression tests bellow. 
> The driver can be used to several compilations, but how do these compilation 
> use different host toolchains? 

I don't know if it's possible to do so when compiling through the command line. 
 But if using clang as a library, you can create a Driver and use it for 
multiple compilations with arbitrary targets.

I am not certain we do this inside of the tree, although there are a few places 
where we create Driver objects, such as lib/Tooling/CompilationDatabase.cpp and 
lib/Tooling/Tooling.cpp.  But also anyone downstream can presumably use clang 
this way.


https://reviews.llvm.org/D26774



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


[PATCH] D26773: [analyzer] Refactor recursive symbol reachability check to use symbol_iterator

2016-11-17 Thread Dominic Chen via cfe-commits
ddcc updated this revision to Diff 78392.
ddcc added a comment.

Fix visitation, add early termination, add comments


https://reviews.llvm.org/D26773

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


Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -527,32 +527,17 @@
 }
 
 bool ScanReachableSymbols::scan(const SymExpr *sym) {
-  bool wasVisited = !visited.insert(sym).second;
-  if (wasVisited)
-return true;
-
-  if (!visitor.VisitSymbol(sym))
-return false;
+  for (SymExpr::symbol_iterator SI = sym->symbol_begin(),
+SE = sym->symbol_end();
+   SI != SE; ++SI) {
+bool wasVisited = !visited.insert(*SI).second;
+if (wasVisited)
+  continue;
 
-  // TODO: should be rewritten using SymExpr::symbol_iterator.
-  switch (sym->getKind()) {
-case SymExpr::SymbolRegionValueKind:
-case SymExpr::SymbolConjuredKind:
-case SymExpr::SymbolDerivedKind:
-case SymExpr::SymbolExtentKind:
-case SymExpr::SymbolMetadataKind:
-  break;
-case SymExpr::SymbolCastKind:
-  return scan(cast(sym)->getOperand());
-case SymExpr::SymIntExprKind:
-  return scan(cast(sym)->getLHS());
-case SymExpr::IntSymExprKind:
-  return scan(cast(sym)->getRHS());
-case SymExpr::SymSymExprKind: {
-  const SymSymExpr *x = cast(sym);
-  return scan(x->getLHS()) && scan(x->getRHS());
-}
+if (!visitor.VisitSymbol(*SI))
+  return false;
   }
+
   return true;
 }
 
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -824,8 +824,9 @@
 }
 
 /// \class ScanReachableSymbols
-/// A Utility class that allows to visit the reachable symbols using a custom
-/// SymbolVisitor.
+/// A utility class that visits the reachable symbols using a custom
+/// SymbolVisitor. Terminates recursive traversal when the visitor function
+/// returns false.
 class ScanReachableSymbols {
   typedef llvm::DenseSet VisitedItems;
 


Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -527,32 +527,17 @@
 }
 
 bool ScanReachableSymbols::scan(const SymExpr *sym) {
-  bool wasVisited = !visited.insert(sym).second;
-  if (wasVisited)
-return true;
-
-  if (!visitor.VisitSymbol(sym))
-return false;
+  for (SymExpr::symbol_iterator SI = sym->symbol_begin(),
+SE = sym->symbol_end();
+   SI != SE; ++SI) {
+bool wasVisited = !visited.insert(*SI).second;
+if (wasVisited)
+  continue;
 
-  // TODO: should be rewritten using SymExpr::symbol_iterator.
-  switch (sym->getKind()) {
-case SymExpr::SymbolRegionValueKind:
-case SymExpr::SymbolConjuredKind:
-case SymExpr::SymbolDerivedKind:
-case SymExpr::SymbolExtentKind:
-case SymExpr::SymbolMetadataKind:
-  break;
-case SymExpr::SymbolCastKind:
-  return scan(cast(sym)->getOperand());
-case SymExpr::SymIntExprKind:
-  return scan(cast(sym)->getLHS());
-case SymExpr::IntSymExprKind:
-  return scan(cast(sym)->getRHS());
-case SymExpr::SymSymExprKind: {
-  const SymSymExpr *x = cast(sym);
-  return scan(x->getLHS()) && scan(x->getRHS());
-}
+if (!visitor.VisitSymbol(*SI))
+  return false;
   }
+
   return true;
 }
 
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -824,8 +824,9 @@
 }
 
 /// \class ScanReachableSymbols
-/// A Utility class that allows to visit the reachable symbols using a custom
-/// SymbolVisitor.
+/// A utility class that visits the reachable symbols using a custom
+/// SymbolVisitor. Terminates recursive traversal when the visitor function
+/// returns false.
 class ScanReachableSymbols {
   typedef llvm::DenseSet VisitedItems;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D26773: [analyzer] Refactor recursive symbol reachability check to use symbol_iterator

2016-11-17 Thread Dominic Chen via cfe-commits
ddcc added a comment.

I believe you're correct, the original code terminates early because of the 
short circuit evaluation on line 553, and visits all reachable nodes but 
doesn't recurse on non-SymbolData.


https://reviews.llvm.org/D26773



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


[PATCH] D25654: [Sema] Don't perform aggregate initialization for types with explicit constructors

2016-11-17 Thread Eric Fiselier via cfe-commits
EricWF added a comment.

@rsmith ping. This is kind-of blocking making libc++'s tag types not 
constructible from `{}`.


https://reviews.llvm.org/D25654



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


Re: r287241 - Use unique_ptr for cached tokens for default arguments in C++.

2016-11-17 Thread Richard Smith via cfe-commits
On 17 November 2016 at 09:52, Malcolm Parsons via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: malcolm.parsons
> Date: Thu Nov 17 11:52:58 2016
> New Revision: 287241
>
> URL: http://llvm.org/viewvc/llvm-project?rev=287241&view=rev
> Log:
> Use unique_ptr for cached tokens for default arguments in C++.
>
> Summary:
> This changes pointers to cached tokens for default arguments in C++ from
> raw pointers to unique_ptrs.  There was a fixme in the code where the
> cached tokens are created  about using a smart pointer.
>
> The change is straightforward, though I did have to track down and fix a
> memory corruption caused by the change.  memcpy was being used to copy
> parameter information.  This duplicated the unique_ptr, which led to the
> cached token buffer being deleted prematurely.
>
> Patch by David Tarditi!
>
> Reviewers: malcolm.parsons
>
> Subscribers: arphaman, malcolm.parsons, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D26435
>
> Modified:
> cfe/trunk/include/clang/Parse/Parser.h
> cfe/trunk/include/clang/Sema/DeclSpec.h
> cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
> cfe/trunk/lib/Parse/ParseDecl.cpp
> cfe/trunk/lib/Parse/ParseDeclCXX.cpp
> cfe/trunk/lib/Sema/DeclSpec.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>
> Modified: cfe/trunk/include/clang/Parse/Parser.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Parse/Parser.h?rev=287241&r1=287240&r2=287241&view=diff
> 
> ==
> --- cfe/trunk/include/clang/Parse/Parser.h (original)
> +++ cfe/trunk/include/clang/Parse/Parser.h Thu Nov 17 11:52:58 2016
> @@ -1017,8 +1017,8 @@ private:
>/// (C++ [class.mem]p2).
>struct LateParsedDefaultArgument {
>  explicit LateParsedDefaultArgument(Decl *P,
> -   CachedTokens *Toks = nullptr)
> -  : Param(P), Toks(Toks) { }
> +   std::unique_ptr Toks
> = nullptr)
> +  : Param(P), Toks(std::move(Toks)) { }
>
>  /// Param - The parameter declaration for this parameter.
>  Decl *Param;
> @@ -1027,7 +1027,7 @@ private:
>  /// argument expression, not including the '=' or the terminating
>  /// ')' or ','. This will be NULL for parameters that have no
>  /// default argument.
> -CachedTokens *Toks;
> +std::unique_ptr Toks;
>};
>
>/// LateParsedMethodDeclaration - A method declaration inside a class
> that
>
> Modified: cfe/trunk/include/clang/Sema/DeclSpec.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Sema/DeclSpec.h?rev=287241&r1=287240&r2=287241&view=diff
> 
> ==
> --- cfe/trunk/include/clang/Sema/DeclSpec.h (original)
> +++ cfe/trunk/include/clang/Sema/DeclSpec.h Thu Nov 17 11:52:58 2016
> @@ -1188,14 +1188,14 @@ struct DeclaratorChunk {
>  /// declaration of a member function), it will be stored here as a
>  /// sequence of tokens to be parsed once the class definition is
>  /// complete. Non-NULL indicates that there is a default argument.
> -CachedTokens *DefaultArgTokens;
> +std::unique_ptr DefaultArgTokens;
>
>  ParamInfo() = default;
>  ParamInfo(IdentifierInfo *ident, SourceLocation iloc,
>Decl *param,
> -  CachedTokens *DefArgTokens = nullptr)
> +  std::unique_ptr DefArgTokens = nullptr)
>: Ident(ident), IdentLoc(iloc), Param(param),
> -DefaultArgTokens(DefArgTokens) {}
> +DefaultArgTokens(std::move(DefArgTokens)) {}
>};
>
>struct TypeAndRange {
> @@ -1310,10 +1310,8 @@ struct DeclaratorChunk {
>  ///
>  /// This is used in various places for error recovery.
>  void freeParams() {
> -  for (unsigned I = 0; I < NumParams; ++I) {
> -delete Params[I].DefaultArgTokens;
> -Params[I].DefaultArgTokens = nullptr;
> -  }
> +  for (unsigned I = 0; I < NumParams; ++I)
> +Params[I].DefaultArgTokens.reset();
>if (DeleteParams) {
>  delete[] Params;
>  DeleteParams = false;
>
> Modified: cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/
> ParseCXXInlineMethods.cpp?rev=287241&r1=287240&r2=287241&view=diff
> 
> ==
> --- cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp Thu Nov 17 11:52:58 2016
> @@ -319,7 +319,8 @@ void Parser::ParseLexedMethodDeclaration
>  // Introduce the parameter into scope.
>  bool HasUnparsed = Param->hasUnparsedDefaultArg();
>  Actions.ActOnDelayedCXXMethodParameter(getCurScope(), Param);
> -if (CachedTokens *Toks = LM.DefaultArgs[I].Toks) {
> +std::unique_ptr Toks = std::move(LM.DefaultArgs[I].
> Toks);
> +if (Toks) {
>// Mark 

[PATCH] D26782: [libcxx] [test] Test changes for P0504R0 "Revisiting in-place tag types for any/optional/variant"

2016-11-17 Thread Eric Fiselier via cfe-commits
EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.

LGTM. This patch misses `any.modifiers/emplace.pass.cpp` but I'll fix that up 
and commit with the libc++ changes!


https://reviews.llvm.org/D26782



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


[libcxx] r287249 - Test changes for P0504R0 "Revisiting in-place tag types for any/optional/variant". Patch from Casey Carter

2016-11-17 Thread Eric Fiselier via cfe-commits
Author: ericwf
Date: Thu Nov 17 13:23:35 2016
New Revision: 287249

URL: http://llvm.org/viewvc/llvm-project?rev=287249&view=rev
Log:
Test changes for P0504R0 "Revisiting in-place tag types for 
any/optional/variant". Patch from Casey Carter

Modified:

libcxx/trunk/test/std/utilities/any/any.class/any.cons/in_place_type.pass.cpp
libcxx/trunk/test/std/utilities/any/any.class/any.cons/value.pass.cpp
libcxx/trunk/test/std/utilities/any/any.class/any.modifiers/emplace.pass.cpp

libcxx/trunk/test/std/utilities/any/any.nonmembers/any.cast/any_cast_reference.pass.cpp
libcxx/trunk/test/std/utilities/utility/utility.inplace/inplace.pass.cpp

Modified: 
libcxx/trunk/test/std/utilities/any/any.class/any.cons/in_place_type.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/any/any.class/any.cons/in_place_type.pass.cpp?rev=287249&r1=287248&r2=287249&view=diff
==
--- 
libcxx/trunk/test/std/utilities/any/any.class/any.cons/in_place_type.pass.cpp 
(original)
+++ 
libcxx/trunk/test/std/utilities/any/any.class/any.cons/in_place_type.pass.cpp 
Thu Nov 17 13:23:35 2016
@@ -40,7 +40,7 @@ void test_in_place_type() {
 assert(Type::count == 0);
 Type::reset();
 {
-any a(std::in_place);
+any a(std::in_place_type);
 
 assert(Type::count == 1);
 assert(Type::copied == 0);
@@ -50,7 +50,7 @@ void test_in_place_type() {
 assert(Type::count == 0);
 Type::reset();
 { // Test that the in_place argument is properly decayed
-any a(std::in_place);
+any a(std::in_place_type);
 
 assert(Type::count == 1);
 assert(Type::copied == 0);
@@ -60,7 +60,7 @@ void test_in_place_type() {
 assert(Type::count == 0);
 Type::reset();
 {
-any a(std::in_place, 101);
+any a(std::in_place_type, 101);
 
 assert(Type::count == 1);
 assert(Type::copied == 0);
@@ -70,7 +70,7 @@ void test_in_place_type() {
 assert(Type::count == 0);
 Type::reset();
 {
-any a(std::in_place, -1, 42, -1);
+any a(std::in_place_type, -1, 42, -1);
 
 assert(Type::count == 1);
 assert(Type::copied == 0);
@@ -86,21 +86,21 @@ void test_in_place_type_tracked() {
 // constructing from a small type should perform no allocations.
 DisableAllocationGuard g(isSmallType()); ((void)g);
 {
-any a(std::in_place);
+any a(std::in_place_type);
 assertArgsMatch(a);
 }
 {
-any a(std::in_place, -1, 42, -1);
+any a(std::in_place_type, -1, 42, -1);
 assertArgsMatch(a);
 }
 // initializer_list constructor tests
 {
-any a(std::in_place, {-1, 42, -1});
+any a(std::in_place_type, {-1, 42, -1});
 assertArgsMatch>(a);
 }
 {
 int x = 42;
-any a(std::in_place, {-1, 42, -1}, x);
+any a(std::in_place_type, {-1, 42, -1}, x);
 assertArgsMatch, int&>(a);
 }
 }
@@ -111,7 +111,7 @@ void test_in_place_type_decayed() {
 {
 using Type = decltype(test_func);
 using DecayT = void(*)();
-any a(std::in_place, test_func);
+any a(std::in_place_type, test_func);
 assert(containsType(a));
 assert(any_cast(a) == test_func);
 }
@@ -119,14 +119,14 @@ void test_in_place_type_decayed() {
 int my_arr[5];
 using Type = int(&)[5];
 using DecayT = int*;
-any a(std::in_place, my_arr);
+any a(std::in_place_type, my_arr);
 assert(containsType(a));
 assert(any_cast(a) == my_arr);
 }
 {
 using Type = int[5];
 using DecayT = int*;
-any a(std::in_place);
+any a(std::in_place_type);
 assert(containsType(a));
 assert(any_cast(a) == nullptr);
 }

Modified: libcxx/trunk/test/std/utilities/any/any.class/any.cons/value.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/any/any.class/any.cons/value.pass.cpp?rev=287249&r1=287248&r2=287249&view=diff
==
--- libcxx/trunk/test/std/utilities/any/any.class/any.cons/value.pass.cpp 
(original)
+++ libcxx/trunk/test/std/utilities/any/any.class/any.cons/value.pass.cpp Thu 
Nov 17 13:23:35 2016
@@ -106,13 +106,12 @@ void test_copy_move_value() {
 }
 }
 
-// Test that any(ValueType&&) is *never* selected for a std::in_place type.
+// Test that any(ValueType&&) is *never* selected for a std::in_place_type_t 
specialization.
 void test_sfinae_constraints() {
 using BadTag = std::in_place_type_t;
 using OKTag = std::in_place_t;
-using OKDecay = std::decay_t;
 // Test that the tag type is properly handled in SFINAE
-BadTag t = std::in_place;
+BadTag t = std::in_place_type;
 OKTag ot = std::in_place;
 {
 std::any a(t);
@@ -124,12 +123,7 @@ void test_sfinae_constr

[libcxx] r287250 - Implement P0504R0: Revisiting in-place tag types for any/optional/variant

2016-11-17 Thread Eric Fiselier via cfe-commits
Author: ericwf
Date: Thu Nov 17 13:24:04 2016
New Revision: 287250

URL: http://llvm.org/viewvc/llvm-project?rev=287250&view=rev
Log:
Implement P0504R0: Revisiting in-place tag types for any/optional/variant

Modified:
libcxx/trunk/include/any
libcxx/trunk/include/utility

Modified: libcxx/trunk/include/any
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/any?rev=287250&r1=287249&r2=287250&view=diff
==
--- libcxx/trunk/include/any (original)
+++ libcxx/trunk/include/any Thu Nov 17 13:24:04 2016
@@ -200,7 +200,7 @@ public:
 , class _Tp = decay_t<_ValueType>
 , class = enable_if_t<
 !is_same<_Tp, any>::value &&
-!__is_inplace_type_tag<_ValueType>::value &&
+!__is_inplace_type<_ValueType>::value &&
 is_copy_constructible<_Tp>::value>
 >
   _LIBCPP_INLINE_VISIBILITY
@@ -561,13 +561,13 @@ void swap(any & __lhs, any & __rhs) _NOE
 template 
 inline _LIBCPP_INLINE_VISIBILITY
 any make_any(_Args&&... __args) {
-return any(in_place<_Tp>, _VSTD::forward<_Args>(__args)...);
+return any(in_place_type<_Tp>, _VSTD::forward<_Args>(__args)...);
 }
 
 template 
 inline _LIBCPP_INLINE_VISIBILITY
 any make_any(initializer_list<_Up> __il, _Args&&... __args) {
-return any(in_place<_Tp>, __il, _VSTD::forward<_Args>(__args)...);
+return any(in_place_type<_Tp>, __il, _VSTD::forward<_Args>(__args)...);
 }
 
 template 

Modified: libcxx/trunk/include/utility
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/utility?rev=287250&r1=287249&r2=287250&view=diff
==
--- libcxx/trunk/include/utility (original)
+++ libcxx/trunk/include/utility Thu Nov 17 13:24:04 2016
@@ -173,17 +173,22 @@ template
 T exchange(T& obj, U&& new_value);
 
 // 20.2.7, in-place construction // C++17
-struct in_place_tag { in_place_tag() = delete; }; // C++17
-using in_place_t = in_place_tag(&)(unspecified );
+struct in_place_t {
+  explicit in_place_t() = default;
+};
+inline constexpr in_place_t in_place{};
 template 
-using in_place_type_t = in_place_tag(&)(unspecified );
-template 
-using in_place_index_t = in_place_tag(&)(unspecified );
-in_place_tag in_place(unspecified );
+  struct in_place_type_t {
+explicit in_place_type_t() = default;
+  };
 template 
-in_place_tag in_place(unspecified );
+  inline constexpr in_place_type_t in_place_type{};
 template 
-in_place_tag in_place(unspecified );
+  struct in_place_index_t {
+explicit in_place_index_t() = default;
+  };
+template 
+  inline constexpr in_place_index_t in_place_index{};
 
 }  // std
 
@@ -889,59 +894,30 @@ _T1 exchange(_T1& __obj, _T2 && __new_va
 
 #if _LIBCPP_STD_VER > 14
 
-struct _LIBCPP_TYPE_VIS_ONLY __in_place_tag {};
-template  struct _LIBCPP_TYPE_VIS_ONLY __in_place_type_tag {};
-template  struct _LIBCPP_TYPE_VIS_ONLY __in_place_index_tag {};
-
-struct _LIBCPP_TYPE_VIS_ONLY in_place_tag;
+struct _LIBCPP_TYPE_VIS in_place_t {
+explicit in_place_t() = default;
+};
+inline constexpr in_place_t in_place{};
 
-using in_place_t = in_place_tag(&)(__in_place_tag);
 template 
-using in_place_type_t = in_place_tag(&)(__in_place_type_tag<_Tp>);
-template 
-using in_place_index_t = in_place_tag(&)(__in_place_index_tag<_Nx>);
-
-struct in_place_tag {
-  in_place_tag() = delete;
-private:
-  explicit in_place_tag(__in_place_tag) {}
-
-  friend inline in_place_tag in_place(__in_place_tag __t);
-  template 
-  friend inline in_place_tag in_place(__in_place_type_tag<_Tp>);
-  template 
-  friend inline in_place_tag in_place(__in_place_index_tag<_Nx>);
+struct _LIBCPP_TYPE_VIS in_place_type_t {
+explicit in_place_type_t() = default;
 };
-
-inline in_place_tag in_place(__in_place_tag __t) {
-_LIBCPP_ASSERT(false, "The in_place function cannot be invoked");
-return in_place_tag(__t);
-}
 template 
-inline in_place_tag in_place(__in_place_type_tag<_Tp>) {
-_LIBCPP_ASSERT(false, "The in_place function cannot be invoked");
-return in_place_tag(__in_place_tag{});
-}
-template 
-inline in_place_tag in_place(__in_place_index_tag<_Nx>) {
-_LIBCPP_ASSERT(false, "The in_place function cannot be invoked");
-return in_place_tag(__in_place_tag{});
-}
-
-templatestruct __is_inplace_tag_imp : false_type {};
-template <>struct 
__is_inplace_tag_imp : true_type {};
-templatestruct 
__is_inplace_tag_imp)> : true_type {};
-template  struct 
__is_inplace_tag_imp)> : true_type {};
+inline constexpr in_place_type_t<_Tp> in_place_type{};
 
-template 
-using __is_inplace_tag = __is_inplace_tag_imp>>;
+template 
+struct _LIBCPP_TYPE_VIS in_place_index_t {
+explicit in_place_index_t() = default;
+};
+template 
+inline constexpr in_place_index_t<_Idx> in_place_index{};
 
-template  struct __is_inplace_type_tag_imp : false_type {};
-template  struct 
__is_inplace_type_tag_imp)> : true_type 
{};
+template  struc

[libcxx] r287251 - Remove files missed in r287250

2016-11-17 Thread Eric Fiselier via cfe-commits
Author: ericwf
Date: Thu Nov 17 13:24:34 2016
New Revision: 287251

URL: http://llvm.org/viewvc/llvm-project?rev=287251&view=rev
Log:
Remove files missed in r287250

Removed:
libcxx/trunk/test/libcxx/utilities/utility/utility.inplace/

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


[PATCH] D26782: [libcxx] [test] Test changes for P0504R0 "Revisiting in-place tag types for any/optional/variant"

2016-11-17 Thread Eric Fiselier via cfe-commits
EricWF closed this revision.
EricWF added a comment.

r287249. Thanks again!


https://reviews.llvm.org/D26782



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


[PATCH] D25654: [Sema] Don't perform aggregate initialization for types with explicit constructors

2016-11-17 Thread Richard Smith via cfe-commits
rsmith added inline comments.



Comment at: lib/AST/DeclCXX.cpp:564
+// C++1z [dcl.init.aggr]p1:
+//  - no user-provided, explicit, or inherited constructors,
+if (getASTContext().getLangOpts().CPlusPlus1z && Constructor->isExplicit())

rsmith wrote:
> Do we correctly handle the "or inherited" part? I'd also like to find out 
> whether core intended for this issue to be treated as a DR or not (if so, 
> this should apply all the way back to C++11).
According to the current issues list, this issue is in DRWP status, so this 
change should be applied retroactively to C++11 and C++14 as well.


https://reviews.llvm.org/D25654



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


[PATCH] D26808: [Sema] Don't allow applying address-of operator to a call to a function with __unknown_anytype return type

2016-11-17 Thread Akira Hatanaka via cfe-commits
ahatanak created this revision.
ahatanak added reviewers: doug.gregor, spyffe.
ahatanak added a subscriber: cfe-commits.

This patch fixes an assert in CodeGenFunction::EmitCallExprLValue that is 
triggered when the CallExpr's return type is not a reference type:

assert(E->getCallReturnType(getContext())->isReferenceType() &&

  "Can't have a scalar return unless the return type is a "
  "reference type!");

Alternatively, since it's legal to apply the address-of operator to a reference 
return, we can change the function return type to be a reference (in the test 
case, that would be double&) in RebuildUnknownAnyExpr::VisitUnaryAddrOf when 
compiling for C++ (but not when compiling for C).


https://reviews.llvm.org/D26808

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/unknown-anytype.cpp


Index: test/SemaCXX/unknown-anytype.cpp
===
--- test/SemaCXX/unknown-anytype.cpp
+++ test/SemaCXX/unknown-anytype.cpp
@@ -56,3 +56,15 @@
 (X)test0(); // expected-error{{implicit instantiation of undefined 
template 'test5::X'}}
   }
 }
+
+namespace test6 {
+  extern __unknown_anytype func();
+  extern __unknown_anytype var;
+  double *d;
+
+  void test() {
+d = (double*)&func(); // expected-error{{address-of operator cannot be 
applied to a call to a function with unknown return type}}
+d = (double*)&var;
+  }
+
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -14773,6 +14773,13 @@
   << E->getSourceRange();
 return ExprError();
   }
+
+  if (isa(E->getSubExpr())) {
+S.Diag(E->getOperatorLoc(), diag::err_unknown_any_addrof_call)
+  << E->getSourceRange();
+return ExprError();
+  }
+
   assert(E->getValueKind() == VK_RValue);
   assert(E->getObjectKind() == OK_Ordinary);
   E->setType(DestType);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8017,6 +8017,9 @@
 def err_unknown_any_addrof : Error<
   "the address of a declaration with unknown type "
   "can only be cast to a pointer type">;
+def err_unknown_any_addrof_call : Error<
+  "address-of operator cannot be applied to a call to a function with "
+  "unknown return type">;
 def err_unknown_any_var_function_type : Error<
   "variable %0 with unknown type cannot be given a function type">;
 def err_unknown_any_function : Error<


Index: test/SemaCXX/unknown-anytype.cpp
===
--- test/SemaCXX/unknown-anytype.cpp
+++ test/SemaCXX/unknown-anytype.cpp
@@ -56,3 +56,15 @@
 (X)test0(); // expected-error{{implicit instantiation of undefined template 'test5::X'}}
   }
 }
+
+namespace test6 {
+  extern __unknown_anytype func();
+  extern __unknown_anytype var;
+  double *d;
+
+  void test() {
+d = (double*)&func(); // expected-error{{address-of operator cannot be applied to a call to a function with unknown return type}}
+d = (double*)&var;
+  }
+
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -14773,6 +14773,13 @@
   << E->getSourceRange();
 return ExprError();
   }
+
+  if (isa(E->getSubExpr())) {
+S.Diag(E->getOperatorLoc(), diag::err_unknown_any_addrof_call)
+  << E->getSourceRange();
+return ExprError();
+  }
+
   assert(E->getValueKind() == VK_RValue);
   assert(E->getObjectKind() == OK_Ordinary);
   E->setType(DestType);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8017,6 +8017,9 @@
 def err_unknown_any_addrof : Error<
   "the address of a declaration with unknown type "
   "can only be cast to a pointer type">;
+def err_unknown_any_addrof_call : Error<
+  "address-of operator cannot be applied to a call to a function with "
+  "unknown return type">;
 def err_unknown_any_var_function_type : Error<
   "variable %0 with unknown type cannot be given a function type">;
 def err_unknown_any_function : Error<
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D26753: ASTImporter: improve support for C++ templates

2016-11-17 Thread Aleksei Sidorin via cfe-commits
a.sidorin updated this revision to Diff 78382.
a.sidorin added a comment.

Address review comments; fix tests.


https://reviews.llvm.org/D26753

Files:
  include/clang/AST/TemplateBase.h
  lib/AST/ASTImporter.cpp
  
test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp
  
test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp
  test/ASTMerge/class-template-partial-spec/test.cpp

Index: test/ASTMerge/class-template-partial-spec/test.cpp
===
--- /dev/null
+++ test/ASTMerge/class-template-partial-spec/test.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -emit-pch -std=c++1z -o %t.1.ast %S/Inputs/class-template-partial-spec1.cpp
+// RUN: %clang_cc1 -emit-pch -std=c++1z -o %t.2.ast %S/Inputs/class-template-partial-spec2.cpp
+// RUN: not %clang_cc1 -std=c++1z -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only %s 2>&1 | FileCheck %s
+
+static_assert(sizeof(**SingleSource.member) == sizeof(**SingleDest.member));
+static_assert(sizeof(SecondDoubleSource.member) == sizeof(SecondDoubleDest.member));
+static_assert(NumberSource.val == 42);
+static_assert(sizeof(Z0Source.member) == sizeof(char));
+static_assert(sizeof(Dst::Z0Dst.member) == sizeof(double));
+static_assert(sizeof(One::Child1>::member) == sizeof(double));
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:21:32: error: external variable 'X1' declared with incompatible types in different translation units ('TwoOptionTemplate' vs. 'TwoOptionTemplate')
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:21:31: note: declared here with type 'TwoOptionTemplate'
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:24:29: error: external variable 'X4' declared with incompatible types in different translation units ('TwoOptionTemplate' vs. 'TwoOptionTemplate')
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:24:33: note: declared here with type 'TwoOptionTemplate'
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:38:8: warning: type 'IntTemplateSpec<5, void *>' has incompatible definitions in different translation units
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:39:7: note: field 'member' has type 'int' here
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:39:10: note: field 'member' has type 'double' here
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:52:25: error: external variable 'Y3' declared with incompatible types in different translation units ('IntTemplateSpec<2, int>' vs. 'IntTemplateSpec<3, int>')
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:52:25: note: declared here with type 'IntTemplateSpec<3, int>'
+
+// CHECK-NOT: static_assert
Index: test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp
===
--- /dev/null
+++ test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp
@@ -0,0 +1,79 @@
+template
+struct TwoOptionTemplate {};
+
+template
+struct TwoOptionTemplate {
+  int member;
+};
+
+
+template
+struct TwoOptionTemplate {
+  float member;
+};
+
+template
+struct TwoOptionTemplate {
+  T** member;
+};
+
+TwoOptionTemplate X0;
+TwoOptionTemplate X1;
+TwoOptionTemplate X2;
+TwoOptionTemplate X3;
+TwoOptionTemplate X4;
+TwoOptionTemplate SingleDest;
+TwoOptionTemplate SecondDoubleDest;
+
+
+template
+struct IntTemplateSpec {};
+
+template
+struct IntTemplateSpec<4, C> {
+  C member;
+};
+
+template
+struct IntTemplateSpec {
+  double member;
+  static constexpr int val = I;
+};
+
+template
+struct IntTemplateSpec {
+  char member;
+  static constexpr int val = I;
+};
+
+IntTemplateSpec<4, wchar_t>Y0;
+IntTemplateSpec<5, void *> Y1;
+IntTemplateSpec<1, int> Y2;
+IntTemplateSpec<2, int> Y3;
+IntTemplateSpec<43, double> NumberDest;
+
+namespace One {
+namespace Two {
+namespace Three {
+
+template
+class Parent {};
+
+} // namespace Three
+
+} // namespace Two
+
+template
+struct Child1: public Two::Three::Parent {
+  char member;
+};
+
+template
+struct Child1> {
+  T member;
+};
+
+} // namespace One
+
+namespace Dst { One::Child1> Z0Dst; }
+One::Child1 Z1;
Index: test/ASTMerge/class-template-partial-spec/Inputs

[PATCH] D26753: ASTImporter: improve support for C++ templates

2016-11-17 Thread Aleksei Sidorin via cfe-commits
a.sidorin added a comment.

Thank you! I'll update this review again when I have a test for 
NestedNameSpecifierLocs.




Comment at: lib/AST/ASTImporter.cpp:458
+  }
+  return true;
+}

spyffe wrote:
> Is this really an appropriate default result?  I would argue for `false` here 
> so that an error would propagate, as is typical in ASTImporter.
> Note that this does disagree with the original source's `true` but I think 
> that was because we knew we didn't handle anything, whereas now the 
> assumption is we handle everything.
Good point. Default `false` will also fail import if a new non-handled property 
or option will be added in AST and clearly indicate an error so I will change 
it.


https://reviews.llvm.org/D26753



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


[PATCH] D22334: Fix for Bug 28172 : clang crashes on invalid code (with too few arguments to __builtin_signbit) without any proper diagnostics.

2016-11-17 Thread Mayur Pandey via cfe-commits
mayurpandey updated this revision to Diff 78390.
mayurpandey added a comment.

Hi,

Updated the patch to incorporate the review comments. I had missed adding 
ValArg->getType() when emitting the diagnostic which was cauing the crash.

Testing done, no regressions.

Thanks,
Mayur


https://reviews.llvm.org/D22334

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Sema/builtins.c
  test/SemaCXX/builtins.cpp


Index: test/SemaCXX/builtins.cpp
===
--- test/SemaCXX/builtins.cpp
+++ test/SemaCXX/builtins.cpp
@@ -44,3 +44,10 @@
   __noop(1); // expected-error {{use of undeclared}}
   __debugbreak(); // expected-error {{use of undeclared}}
 }
+
+template 
+int test_signbit(T t) { return __builtin_signbit(t); } // expected-error 
{{floating point type is required}}
+
+int test_signbit_call () {
+return test_signbit("1"); // expected-note {{instantiation of function 
template specialization}} 
+}
Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -248,3 +248,11 @@
 
 return buf;
 }
+
+int test21(double a) {
+  return __builtin_signbit();  // expected-error {{too few arguments}}
+}
+
+int test22(void) {
+  return __builtin_signbit("1");  // expected-error {{floating point type is 
required}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -98,6 +98,22 @@
   return false;
 }
 
+static bool SemaBuiltinSignbit(Sema &S, CallExpr *TheCall) {
+  if (checkArgCount(S, TheCall, 1))
+return true;
+
+  // Argument should be an float, double or long double.
+  Expr *ValArg = TheCall->getArg(0);
+  QualType Ty = ValArg->getType();
+  if (!Ty->isRealFloatingType()) {
+S.Diag(ValArg->getLocStart(), diag::err_typecheck_cond_expect_float)
+  << ValArg->getType() << ValArg->getSourceRange();
+return true;
+  }
+
+  return false;
+}
+
 /// Check that the argument to __builtin_addressof is a glvalue, and set the
 /// result type to the corresponding pointer type.
 static bool SemaBuiltinAddressof(Sema &S, CallExpr *TheCall) {
@@ -762,6 +778,10 @@
 }
 break;
   }
+  case Builtin::BI__builtin_signbit:
+if (SemaBuiltinSignbit(*this, TheCall))
+  return ExprError();
+break;
   case Builtin::BI__builtin_isgreater:
   case Builtin::BI__builtin_isgreaterequal:
   case Builtin::BI__builtin_isless:
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6845,6 +6845,8 @@
   "(passed in %0)">;
 def err_typecheck_cond_expect_int_float : Error<
   "used type %0 where integer or floating point type is required">;
+def err_typecheck_cond_expect_float : Error<
+  "used type %0 where floating point type is required">;
 def err_typecheck_cond_expect_scalar : Error<
   "used type %0 where arithmetic or pointer type is required">;
 def err_typecheck_cond_expect_nonfloat : Error<


Index: test/SemaCXX/builtins.cpp
===
--- test/SemaCXX/builtins.cpp
+++ test/SemaCXX/builtins.cpp
@@ -44,3 +44,10 @@
   __noop(1); // expected-error {{use of undeclared}}
   __debugbreak(); // expected-error {{use of undeclared}}
 }
+
+template 
+int test_signbit(T t) { return __builtin_signbit(t); } // expected-error {{floating point type is required}}
+
+int test_signbit_call () {
+return test_signbit("1"); // expected-note {{instantiation of function template specialization}} 
+}
Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -248,3 +248,11 @@
 
 return buf;
 }
+
+int test21(double a) {
+  return __builtin_signbit();  // expected-error {{too few arguments}}
+}
+
+int test22(void) {
+  return __builtin_signbit("1");  // expected-error {{floating point type is required}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -98,6 +98,22 @@
   return false;
 }
 
+static bool SemaBuiltinSignbit(Sema &S, CallExpr *TheCall) {
+  if (checkArgCount(S, TheCall, 1))
+return true;
+
+  // Argument should be an float, double or long double.
+  Expr *ValArg = TheCall->getArg(0);
+  QualType Ty = ValArg->getType();
+  if (!Ty->isRealFloatingType()) {
+S.Diag(ValArg->getLocStart(), diag::err_typecheck_cond_expect_float)
+  << ValArg->getType() << ValArg->getSourceRange();
+return true;
+  }
+
+  return false;
+}
+
 /// Check that the argument to __builtin_addressof is a glvalue, and set the
 /// result type to the corresponding pointer type.
 static bool SemaBuiltinAddressof(Sema &S, CallEx

[libcxx] r287255 - Workaround compilers w/o C++1z inline variables

2016-11-17 Thread Eric Fiselier via cfe-commits
Author: ericwf
Date: Thu Nov 17 14:08:43 2016
New Revision: 287255

URL: http://llvm.org/viewvc/llvm-project?rev=287255&view=rev
Log:
Workaround compilers w/o C++1z inline variables

Modified:
libcxx/trunk/include/__config
libcxx/trunk/include/utility

Modified: libcxx/trunk/include/__config
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=287255&r1=287254&r2=287255&view=diff
==
--- libcxx/trunk/include/__config (original)
+++ libcxx/trunk/include/__config Thu Nov 17 14:08:43 2016
@@ -796,6 +796,11 @@ template  struct __static_asse
 #define _LIBCPP_CONSTEXPR_AFTER_CXX14
 #endif
 
+// FIXME: Remove all usages of this macro once compilers catch up.
+#if !defined(__cpp_inline_variables) || (__cpp_inline_variables < 201606L)
+# define _LIBCPP_HAS_NO_INLINE_VARIABLES
+#endif
+
 #ifdef _LIBCPP_HAS_NO_RVALUE_REFERENCES
 #  define _LIBCPP_EXPLICIT_MOVE(x) _VSTD::move(x)
 #else

Modified: libcxx/trunk/include/utility
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/utility?rev=287255&r1=287254&r2=287255&view=diff
==
--- libcxx/trunk/include/utility (original)
+++ libcxx/trunk/include/utility Thu Nov 17 14:08:43 2016
@@ -897,21 +897,30 @@ _T1 exchange(_T1& __obj, _T2 && __new_va
 struct _LIBCPP_TYPE_VIS in_place_t {
 explicit in_place_t() = default;
 };
-inline constexpr in_place_t in_place{};
+#ifndef _LIBCPP_HAS_NO_INLINE_VARIABLES
+inline
+#endif
+constexpr in_place_t in_place{};
 
 template 
 struct _LIBCPP_TYPE_VIS in_place_type_t {
 explicit in_place_type_t() = default;
 };
 template 
-inline constexpr in_place_type_t<_Tp> in_place_type{};
+#ifndef _LIBCPP_HAS_NO_INLINE_VARIABLES
+inline
+#endif
+constexpr in_place_type_t<_Tp> in_place_type{};
 
 template 
 struct _LIBCPP_TYPE_VIS in_place_index_t {
 explicit in_place_index_t() = default;
 };
 template 
-inline constexpr in_place_index_t<_Idx> in_place_index{};
+#ifndef _LIBCPP_HAS_NO_INLINE_VARIABLES
+inline
+#endif
+constexpr in_place_index_t<_Idx> in_place_index{};
 
 template  struct __is_inplace_type_imp : false_type {};
 template  struct __is_inplace_type_imp> : 
true_type {};


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


[PATCH] D22334: Fix for Bug 28172 : clang crashes on invalid code (with too few arguments to __builtin_signbit) without any proper diagnostics.

2016-11-17 Thread David Majnemer via cfe-commits
majnemer accepted this revision.
majnemer added a comment.
This revision is now accepted and ready to land.

LGTM with nits




Comment at: lib/Sema/SemaChecking.cpp:110
+S.Diag(ValArg->getLocStart(), diag::err_typecheck_cond_expect_float)
+  << ValArg->getType() << ValArg->getSourceRange();
+return true;

Can you change this `ValArg->getType()` to `Ty`?


https://reviews.llvm.org/D22334



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


[PATCH] D21099: [Sema] Teach -Wcast-align to look at the aligned attribute of the declared variables

2016-11-17 Thread Akira Hatanaka via cfe-commits
ahatanak updated this revision to Diff 78399.
ahatanak added a comment.

Address Alex's review comments.

Define a static function for setting SrcAlign.


https://reviews.llvm.org/D21099

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/warn-cast-align.c


Index: test/Sema/warn-cast-align.c
===
--- test/Sema/warn-cast-align.c
+++ test/Sema/warn-cast-align.c
@@ -39,3 +39,23 @@
 void test3(char *P) {
   struct B *b = (struct B*) P;
 }
+
+// Do not issue a warning. The aligned attribute changes the alignment of the
+// variables and fields.
+char __attribute__((aligned(4))) a[16];
+
+struct S0 {
+  char a[16];
+};
+
+struct S {
+  char __attribute__((aligned(4))) a[16];
+  struct S0 __attribute__((aligned(4))) s0;
+};
+
+void test4() {
+  struct S s;
+  int *i = (int *)s.a;
+  i = (int *)&s.s0;
+  i = (int *)a;
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -10267,6 +10267,13 @@
   return HasInvalidParm;
 }
 
+static void setSrcAlign(Expr *SE, CharUnits &SrcAlign, ASTContext &Context) {
+  if (const auto *DRE = dyn_cast(SE))
+SrcAlign = Context.getDeclAlign(DRE->getDecl());
+  else if (const auto *ME = dyn_cast(SE))
+SrcAlign = Context.getDeclAlign(ME->getMemberDecl());
+}
+
 /// CheckCastAlign - Implements -Wcast-align, which warns when a
 /// pointer cast increases the alignment requirements.
 void Sema::CheckCastAlign(Expr *Op, QualType T, SourceRange TRange) {
@@ -10301,6 +10308,15 @@
   if (SrcPointee->isIncompleteType()) return;
 
   CharUnits SrcAlign = Context.getTypeAlignInChars(SrcPointee);
+
+  if (auto *CE = dyn_cast(Op)) {
+if (CE->getCastKind() == CK_ArrayToPointerDecay)
+  setSrcAlign(CE->getSubExpr(), SrcAlign, Context);
+  } else if (auto *UO = dyn_cast(Op)) {
+if (UO->getOpcode() == UO_AddrOf)
+  setSrcAlign(UO->getSubExpr(), SrcAlign, Context);
+  }
+
   if (SrcAlign >= DestAlign) return;
 
   Diag(TRange.getBegin(), diag::warn_cast_align)


Index: test/Sema/warn-cast-align.c
===
--- test/Sema/warn-cast-align.c
+++ test/Sema/warn-cast-align.c
@@ -39,3 +39,23 @@
 void test3(char *P) {
   struct B *b = (struct B*) P;
 }
+
+// Do not issue a warning. The aligned attribute changes the alignment of the
+// variables and fields.
+char __attribute__((aligned(4))) a[16];
+
+struct S0 {
+  char a[16];
+};
+
+struct S {
+  char __attribute__((aligned(4))) a[16];
+  struct S0 __attribute__((aligned(4))) s0;
+};
+
+void test4() {
+  struct S s;
+  int *i = (int *)s.a;
+  i = (int *)&s.s0;
+  i = (int *)a;
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -10267,6 +10267,13 @@
   return HasInvalidParm;
 }
 
+static void setSrcAlign(Expr *SE, CharUnits &SrcAlign, ASTContext &Context) {
+  if (const auto *DRE = dyn_cast(SE))
+SrcAlign = Context.getDeclAlign(DRE->getDecl());
+  else if (const auto *ME = dyn_cast(SE))
+SrcAlign = Context.getDeclAlign(ME->getMemberDecl());
+}
+
 /// CheckCastAlign - Implements -Wcast-align, which warns when a
 /// pointer cast increases the alignment requirements.
 void Sema::CheckCastAlign(Expr *Op, QualType T, SourceRange TRange) {
@@ -10301,6 +10308,15 @@
   if (SrcPointee->isIncompleteType()) return;
 
   CharUnits SrcAlign = Context.getTypeAlignInChars(SrcPointee);
+
+  if (auto *CE = dyn_cast(Op)) {
+if (CE->getCastKind() == CK_ArrayToPointerDecay)
+  setSrcAlign(CE->getSubExpr(), SrcAlign, Context);
+  } else if (auto *UO = dyn_cast(Op)) {
+if (UO->getOpcode() == UO_AddrOf)
+  setSrcAlign(UO->getSubExpr(), SrcAlign, Context);
+  }
+
   if (SrcAlign >= DestAlign) return;
 
   Diag(TRange.getBegin(), diag::warn_cast_align)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24289: Add warning when assigning enums to bitfields without an explicit unsigned underlying type

2016-11-17 Thread Arthur O'Dwyer via cfe-commits
On Thu, Nov 17, 2016 at 9:52 AM, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> On 17 Nov 2016 8:56 am, "Reid Kleckner"  wrote:
>> In https://reviews.llvm.org/D24289#598169, @rsmith wrote:
>>> This is causing warnings to fire for headers shared between C and C++,
>>> where the "give the enum an unsigned underlying type" advice doesn't
work,
>>> and where the code in question will never be built for the MS ABI. It
seems
>>> really hard to justify this being on by default.
>>>
>>> I'm going to turn it off by default for now, but we should probably
>>> consider turning it back on by default when targeting the MS ABI (as a
"your
>>> code is wrong" warning rather than a "your code is not portable"
warning).
[...]
>>> Yeah, suggesting adding an underlying type to the enum to solve this
>>> problem seems like a bad idea, since that fundamentally changes the
nature
>>> of the enum -- typically allowing it to store a lot more values, and
making
>>> putting it in a bitfield a bad idea.
>
>> Any time you use a bitfield it stores fewer values than the original
integer
>> type. I don't see how enums are special here. [...]
>
> The range of representable values for a bitfield with no fixed underlying
> type is actually smaller than that of its underlying type. See
> http://eel.is/c++draft/dcl.enum#8
>
> So a bitfield of width equal to the number of bits needed to store any
> enumerator does not have fewer values than the original type.

My understanding (from osmosis and practice more than from reading the
standard) is that programmers are more likely to specify an "unnaturally
narrow" underlying type (e.g. "int8_t") than to specify an "unnaturally
wide" underlying type (e.g. "int32_t". When I specify an underlying type,
I'm saying "The compiler is going to do the wrong thing with this type's
*storage* by default"; I'm not saying anything about the type's *value
range*.
The same goes for bit-fields: I specify a number of bits after the colon
because the compiler would otherwise do the wrong thing with *storage*, not
because I'm trying to change the semantics of the *values* involved.


>> Do you have any better suggestions for people that want this code to do
the
>> right thing when built with MSVC?
>>
>>   enum E /* : unsigned */ { E0, E1, E2, E3 };
>>   struct A { E b : 2; };
>>   int main() {
>> A a;
>> a.b = E3;
>> return a.b; // comes out as -1 without the underlying type
>>   }
>>
>> Widening the bitfield wastes a bit. Making the bitfield a plain integer
and
>> cast in and out of the enum type, but that obscures the true type of the
>> bitfield. So, I still support this suggestion.

The safest fix is to just change ": 2" to ": 3", even though that "wastes a
bit" (really it wastes 0 bits in most cases and 32 to 64 bits in some
cases).

If I had my druthers, the compiler would be completely silent unless it
detected exactly the cases that would result in changes to the semantics of
enum *values*. That is,

when declaring a bit-field of enum type E with width B bits:
  if E has an enumerator e whose value requires >B bits:
warn that e cannot be stored in the bit-field
if a fixit is really required, suggest increasing the bit-field's
width
  if E has an enumerator e whose positive value requires exactly B
bits, and E's underlying type is signed:
warn that e cannot be stored in the bit-field when MSVC semantics
are in use
in C++, append the note that this happens because E's underlying
type is signed
if a fixit is really required, suggest increasing the bit-field's
width

Changing the width of the bit-field can affect only the layout of the
struct in question; you could even static_assert that the size *doesn't*
change, if there are padding bits available.
Changing a whole type from signed to unsigned seems like it would have more
dangerous action-at-a-distance than just increasing the width of the
bit-field: that would *unavoidably* affect comparisons (and overload
resolutions?) across the entire codebase using that type.
http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#172
Again I'm making a philosophical distinction between the details of storage
(e.g. struct layout) and the actual semantics of values and types (e.g.
signedness).

The problem is with the bit-field inside the struct, so IMHO the solution
should involve changing the bit-field and/or the struct. Not altering the
enum type... which by the way, the programmer might not even control,
right? What if the relevant enum type comes from a third-party library?

> How about we consider msvc's behaviour to just be a bug, and zero-extend
> loads from enum bitfields with no negative enumerators? Since loading
such a
> negative value results in UB, this wouldn't change the meaning of any
> program with defined behaviour, and still respects the MS ABI.

SGTM, at least to put this behavior under a separately toggleable
command-line switch. (-fms-enum-bitfields?)

my $.02,

[PATCH] D26560: Add a test for vcall on a null ptr.

2016-11-17 Thread Ivan Krasin via cfe-commits
krasin updated this revision to Diff 78405.
krasin added a comment.

sync


https://reviews.llvm.org/D26560

Files:
  test/ubsan/TestCases/TypeCheck/null.cpp


Index: test/ubsan/TestCases/TypeCheck/null.cpp
===
--- test/ubsan/TestCases/TypeCheck/null.cpp
+++ test/ubsan/TestCases/TypeCheck/null.cpp
@@ -1,20 +1,50 @@
-// RUN: %clangxx -fsanitize=null %s -O3 -o %t
-// RUN: %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD
-// RUN: %expect_crash %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE
-// RUN: %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE
-// RUN: %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER
-// RUN: %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN
+// RUN: %clangxx -fsanitize=null -fno-sanitize-recover=null -g %s -O3 -o %t
+// RUN: not %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD
+// RUN: not %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE
+// RUN: not %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE
+// RUN: not %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER
+// RUN: not %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN
+// RUN: not %run %t t 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL
+// RUN: not %run %t u 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL2
+
+#include 
 
 struct S {
   int f() { return 0; }
   int k;
 };
 
+struct T {
+  virtual int v() { return 1; }
+};
+
+struct U : T {
+  virtual int v() { return 2; }
+};
+
+static inline void break_optimization(void *arg) {
+  __asm__ __volatile__("" : : "r" (arg) : "memory");
+}
+
 int main(int, char **argv) {
   int *p = 0;
   S *s = 0;
+  T *t = 0;
+  U *u = 0;
+
+  if (argv[1][0] == 'T') {
+t = new T;
+  }
+  if (argv[1][0] == 'U') {
+u = new U;
+  }
+
+  break_optimization(s);
+  break_optimization(t);
+  break_optimization(u);
 
   (void)*p; // ok!
+  (void)*t; // ok!
 
   switch (argv[1][0]) {
   case 'l':
@@ -34,5 +64,13 @@
   case 'f':
 // CHECK-MEMFUN: null.cpp:[[@LINE+1]]:15: runtime error: member call on 
null pointer of type 'S'
 return s->f();
+  case 't':
+  case 'T':
+// CHECK-VCALL: null.cpp:[[@LINE+1]]:15: runtime error: member call on 
null pointer of type 'T'
+return t->v();
+  case 'u':
+  case 'U':
+// CHECK-VCALL2: null.cpp:[[@LINE+1]]:15: runtime error: member call on 
null pointer of type 'U'
+return u->v();
   }
 }


Index: test/ubsan/TestCases/TypeCheck/null.cpp
===
--- test/ubsan/TestCases/TypeCheck/null.cpp
+++ test/ubsan/TestCases/TypeCheck/null.cpp
@@ -1,20 +1,50 @@
-// RUN: %clangxx -fsanitize=null %s -O3 -o %t
-// RUN: %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD
-// RUN: %expect_crash %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE
-// RUN: %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE
-// RUN: %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER
-// RUN: %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN
+// RUN: %clangxx -fsanitize=null -fno-sanitize-recover=null -g %s -O3 -o %t
+// RUN: not %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD
+// RUN: not %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE
+// RUN: not %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE
+// RUN: not %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER
+// RUN: not %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN
+// RUN: not %run %t t 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL
+// RUN: not %run %t u 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL2
+
+#include 
 
 struct S {
   int f() { return 0; }
   int k;
 };
 
+struct T {
+  virtual int v() { return 1; }
+};
+
+struct U : T {
+  virtual int v() { return 2; }
+};
+
+static inline void break_optimization(void *arg) {
+  __asm__ __volatile__("" : : "r" (arg) : "memory");
+}
+
 int main(int, char **argv) {
   int *p = 0;
   S *s = 0;
+  T *t = 0;
+  U *u = 0;
+
+  if (argv[1][0] == 'T') {
+t = new T;
+  }
+  if (argv[1][0] == 'U') {
+u = new U;
+  }
+
+  break_optimization(s);
+  break_optimization(t);
+  break_optimization(u);
 
   (void)*p; // ok!
+  (void)*t; // ok!
 
   switch (argv[1][0]) {
   case 'l':
@@ -34,5 +64,13 @@
   case 'f':
 // CHECK-MEMFUN: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'S'
 return s->f();
+  case 't':
+  case 'T':
+// CHECK-VCALL: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'T'
+return t->v();
+  case 'u':
+  case 'U':
+// CHECK-VCALL2: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'U'
+return u->v();
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r287258 - Fixes for r287241. Use placement new. Apply clang-format.

2016-11-17 Thread Malcolm Parsons via cfe-commits
Author: malcolm.parsons
Date: Thu Nov 17 15:00:09 2016
New Revision: 287258

URL: http://llvm.org/viewvc/llvm-project?rev=287258&view=rev
Log:
Fixes for r287241. Use placement new. Apply clang-format.

Modified:
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Sema/DeclSpec.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp

Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=287258&r1=287257&r2=287258&view=diff
==
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Thu Nov 17 15:00:09 2016
@@ -2039,7 +2039,8 @@ void Parser::HandleMemberFunctionDeclDel
 LateMethod->DefaultArgs.reserve(FTI.NumParams);
 for (unsigned ParamIdx = 0; ParamIdx < FTI.NumParams; ++ParamIdx)
   LateMethod->DefaultArgs.push_back(LateParsedDefaultArgument(
-FTI.Params[ParamIdx].Param, 
std::move(FTI.Params[ParamIdx].DefaultArgTokens)));
+  FTI.Params[ParamIdx].Param,
+  std::move(FTI.Params[ParamIdx].DefaultArgTokens)));
   }
 }
 

Modified: cfe/trunk/lib/Sema/DeclSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/DeclSpec.cpp?rev=287258&r1=287257&r2=287258&view=diff
==
--- cfe/trunk/lib/Sema/DeclSpec.cpp (original)
+++ cfe/trunk/lib/Sema/DeclSpec.cpp Thu Nov 17 15:00:09 2016
@@ -223,15 +223,11 @@ DeclaratorChunk DeclaratorChunk::getFunc
 if (!TheDeclarator.InlineStorageUsed &&
 NumParams <= llvm::array_lengthof(TheDeclarator.InlineParams)) {
   I.Fun.Params = TheDeclarator.InlineParams;
-  // Zero the memory block so that unique pointers are initialized
-  // properly.
-  memset(I.Fun.Params, 0, sizeof(Params[0]) * NumParams);
+  new (I.Fun.Params) ParamInfo[NumParams];
   I.Fun.DeleteParams = false;
   TheDeclarator.InlineStorageUsed = true;
 } else {
-  // Call the version of new that zeroes memory so that unique pointers
-  // are initialized properly.
-  I.Fun.Params = new DeclaratorChunk::ParamInfo[NumParams]();
+  I.Fun.Params = new DeclaratorChunk::ParamInfo[NumParams];
   I.Fun.DeleteParams = true;
 }
 for (unsigned i = 0; i < NumParams; i++)

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=287258&r1=287257&r2=287258&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Nov 17 15:00:09 2016
@@ -395,7 +395,8 @@ void Sema::CheckExtraCXXDefaultArguments
++argIdx) {
 ParmVarDecl *Param = cast(chunk.Fun.Params[argIdx].Param);
 if (Param->hasUnparsedDefaultArg()) {
-  std::unique_ptr Toks = 
std::move(chunk.Fun.Params[argIdx].DefaultArgTokens);
+  std::unique_ptr Toks =
+  std::move(chunk.Fun.Params[argIdx].DefaultArgTokens);
   SourceRange SR;
   if (Toks->size() > 1)
 SR = SourceRange((*Toks)[1].getLocation(),


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


[PATCH] D26807: Fix a comment for -fsave-optimization-record

2016-11-17 Thread Hal Finkel via cfe-commits
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D26807



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


[PATCH] D25654: [Sema] Don't perform aggregate initialization for types with explicit constructors

2016-11-17 Thread Eric Fiselier via cfe-commits
EricWF updated this revision to Diff 78410.
EricWF added a comment.

OK, I've applied the fix to C++11 and C++14. Although the 
inheriting-constructor part still only matters in C++1z since it requires 
having base classes.

I also fixed aggregate initialization for types with non-aggregate base 
classes. For example:

  struct A { A(int); };
  struct B : A {};
  B b = {42}; // OK in C++1z


https://reviews.llvm.org/D25654

Files:
  lib/AST/DeclCXX.cpp
  test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p1.cpp
  test/CXX/drs/dr15xx.cpp

Index: test/CXX/drs/dr15xx.cpp
===
--- test/CXX/drs/dr15xx.cpp
+++ test/CXX/drs/dr15xx.cpp
@@ -135,6 +135,53 @@
   }
 }
 
+namespace dr1518 { // dr1518: 4.0
+#if __cplusplus >= 201103L
+struct Z0 { // expected-note 0+ {{candidate}}
+  explicit Z0() = default; // expected-note 0+ {{here}}
+};
+struct Z { // expected-note 0+ {{candidate}}
+  explicit Z(); // expected-note 0+ {{here}}
+  explicit Z(int);
+  explicit Z(int, int); // expected-note 0+ {{here}}
+};
+template  int Eat(T); // expected-note 0+ {{candidate}}
+Z0 a;
+Z0 b{};
+Z0 c = {}; // expected-error {{explicit in copy-initialization}}
+int i = Eat({}); // expected-error {{no matching function for call to 'Eat'}}
+
+Z c2 = {}; // expected-error {{explicit in copy-initialization}}
+int i2 = Eat({}); // expected-error {{no matching function for call to 'Eat'}}
+Z a1 = 1; // expected-error {{no viable conversion}}
+Z a3 = Z(1);
+Z a2(1);
+Z *p = new Z(1);
+Z a4 = (Z)1;
+Z a5 = static_cast(1);
+Z a6 = {4, 3}; // expected-error {{explicit in copy-initialization}}
+
+struct UserProvidedBaseCtor { // expected-note 0+ {{candidate}}
+  UserProvidedBaseCtor() {}
+};
+struct DoesntInheritCtor : UserProvidedBaseCtor { // expected-note 0+ {{candidate}}
+  int x;
+};
+DoesntInheritCtor I{{}, 42};
+#if __cplusplus <= 201402L
+// expected-error@-2 {{no matching constructor}}
+#endif
+
+struct BaseCtor { BaseCtor() = default; }; // expected-note 0+ {{candidate}}
+struct InheritsCtor : BaseCtor { // expected-note 1+ {{candidate}}
+  using BaseCtor::BaseCtor;  // expected-note 2 {{inherited here}}
+  int x;
+};
+InheritsCtor II = {{}, 42}; // expected-error {{no matching constructor}}
+
+#endif  // __cplusplus >= 201103L
+}
+
 namespace dr1550 { // dr1550: yes
   int f(bool b, int n) {
 return (b ? (throw 0) : n) + (b ? n : (throw 0));
Index: test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p1.cpp
===
--- test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p1.cpp
+++ test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p1.cpp
@@ -122,3 +122,38 @@
   ~DefaultedAggr() = default;
 };
 DefaultedAggr da = { 42 } ;
+
+struct ExplicitDefaultedAggr {
+  int n;
+  explicit ExplicitDefaultedAggr() = default; // expected-note {{candidate}}
+  ExplicitDefaultedAggr(const ExplicitDefaultedAggr &) = default; // expected-note {{candidate}}
+  ExplicitDefaultedAggr(ExplicitDefaultedAggr &&) = default; // expected-note {{candidate}}
+};
+ExplicitDefaultedAggr eda = { 42 }; // expected-error {{no matching constructor}}
+
+struct DefaultedBase {
+  int n;
+  DefaultedBase() = default; // expected-note 0+ {{candidate}}
+  DefaultedBase(DefaultedBase const&) = default; // expected-note 0+ {{candidate}}
+  DefaultedBase(DefaultedBase &&) = default; // expected-note 0+ {{candidate}}
+};
+
+struct InheritingConstructors : DefaultedBase { // expected-note 3 {{candidate}}
+  using DefaultedBase::DefaultedBase; // expected-note 2 {{inherited here}}
+};
+InheritingConstructors ic = { 42 }; // expected-error {{no matching constructor}}
+
+struct NonInheritingConstructors : DefaultedBase {}; // expected-note 0+ {{candidate}}
+NonInheritingConstructors nic = { 42 };
+#if __cplusplus <= 201402L
+// expected-error@-2 {{no matching constructor}}
+#endif
+
+struct NonAggrBase {
+  NonAggrBase(int) {}
+};
+struct HasNonAggrBase : NonAggrBase {}; // expected-note 0+ {{candidate}}
+HasNonAggrBase hnab = {42};
+#if __cplusplus <= 201402L
+// expected-error@-2 {{no matching constructor}}
+#endif
Index: lib/AST/DeclCXX.cpp
===
--- lib/AST/DeclCXX.cpp
+++ lib/AST/DeclCXX.cpp
@@ -533,6 +533,17 @@
   } else if (Constructor->isMoveConstructor())
 SMKind |= SMF_MoveConstructor;
 }
+
+// C++ [dcl.init.aggr]p1:
+//   An aggregate is an array or a class with no user-declared
+//   constructors [...].
+// C++11 [dcl.init.aggr]p1: DR1518
+//  An aggregate is an array or a class with no user-provided, explicit, or
+//  inherited constructors
+if (getASTContext().getLangOpts().CPlusPlus11
+? (Constructor->isUserProvided() || Constructor->isExplicit())
+: !Constructor->isImplicit())
+  data().Aggregate = false;
   }
 
   // Handle constructors, including those inherited from base classes.
@@ -546,20 +557,6 @@
 //   constructor [...]
  

[PATCH] D26774: [CUDA] Driver changes to support CUDA compilation on MacOS.

2016-11-17 Thread Artem Belevich via cfe-commits
tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM, with couple of minor nits.




Comment at: clang/lib/Driver/Driver.cpp:3650-3654
+
+  // Intentionally omitted from the switch above: llvm::Triple::CUDA.  CUDA
+  // compiles always need two toolchains, the CUDA toolchain and the host
+  // toolchain.  So the only valid way to create a CUDA toolchain is via
+  // CreateOffloadingDeviceToolChains.

should there be an assert() or llvm_unreachable() to ensure that? Right now 
we'll happily return default toolchain.



Comment at: clang/test/Driver/cuda-detect.cu:67
 // RUN: -check-prefix LIBDEVICE -check-prefix LIBDEVICE35
+// RUN: %clang -### -v --target=i386-unknown-linux --cuda-gpu-arch=sm_35 \
+// RUN:   -nocudainc --cuda-path=%S/Inputs/CUDA/usr/local/cuda %s 2>&1 \

Should that be --target=i386-apple-macosx ?


https://reviews.llvm.org/D26774



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


[PATCH] D26812: [libcxx] [test] In random tests, use real static_asserts and silence a warning.

2016-11-17 Thread Stephan T. Lavavej via cfe-commits
STL_MSFT created this revision.
STL_MSFT added reviewers: EricWF, mclow.lists.
STL_MSFT added a subscriber: cfe-commits.

[libcxx] [test] In random tests, use real static_asserts and silence a warning.

One test triggers MSVC's warning C4310 "cast truncates constant value".
The code is valid, and yet the warning is valid, so I'm silencing it
through push-disable-pop.


https://reviews.llvm.org/D26812

Files:
  test/std/numerics/rand/rand.adapt/rand.adapt.disc/values.pass.cpp
  test/std/numerics/rand/rand.adapt/rand.adapt.ibits/values.pass.cpp
  test/std/numerics/rand/rand.adapt/rand.adapt.shuf/values.pass.cpp
  test/std/numerics/rand/rand.eng/rand.eng.lcong/values.pass.cpp
  test/std/numerics/rand/rand.eng/rand.eng.mers/values.pass.cpp
  test/std/numerics/rand/rand.eng/rand.eng.sub/values.pass.cpp

Index: test/std/numerics/rand/rand.eng/rand.eng.sub/values.pass.cpp
===
--- test/std/numerics/rand/rand.eng/rand.eng.sub/values.pass.cpp
+++ test/std/numerics/rand/rand.eng/rand.eng.sub/values.pass.cpp
@@ -38,8 +38,8 @@
 static_assert((E::word_size == 24), "");
 static_assert((E::short_lag == 10), "");
 static_assert((E::long_lag == 24), "");
-/*static_*/assert((E::min() == 0)/*, ""*/);
-/*static_*/assert((E::max() == 0xFF)/*, ""*/);
+static_assert((E::min() == 0), "");
+static_assert((E::max() == 0xFF), "");
 static_assert((E::default_seed == 19780503u), "");
 where(E::word_size);
 where(E::short_lag);
@@ -54,8 +54,8 @@
 static_assert((E::word_size == 48), "");
 static_assert((E::short_lag == 5), "");
 static_assert((E::long_lag == 12), "");
-/*static_*/assert((E::min() == 0)/*, ""*/);
-/*static_*/assert((E::max() == 0xull)/*, ""*/);
+static_assert((E::min() == 0), "");
+static_assert((E::max() == 0xull), "");
 static_assert((E::default_seed == 19780503u), "");
 where(E::word_size);
 where(E::short_lag);
Index: test/std/numerics/rand/rand.eng/rand.eng.mers/values.pass.cpp
===
--- test/std/numerics/rand/rand.eng/rand.eng.mers/values.pass.cpp
+++ test/std/numerics/rand/rand.eng/rand.eng.mers/values.pass.cpp
@@ -60,8 +60,8 @@
 static_assert((E::tempering_c == 0xefc6), "");
 static_assert((E::tempering_l == 18), "");
 static_assert((E::initialization_multiplier == 1812433253), "");
-/*static_*/assert((E::min() == 0)/*, ""*/);
-/*static_*/assert((E::max() == 0x)/*, ""*/);
+static_assert((E::min() == 0), "");
+static_assert((E::max() == 0x), "");
 static_assert((E::default_seed == 5489u), "");
 where(E::word_size);
 where(E::state_size);
@@ -96,8 +96,8 @@
 static_assert((E::tempering_c == 0xfff7eee0ull), "");
 static_assert((E::tempering_l == 43), "");
 static_assert((E::initialization_multiplier == 6364136223846793005ull), "");
-/*static_*/assert((E::min() == 0)/*, ""*/);
-/*static_*/assert((E::max() == 0xull)/*, ""*/);
+static_assert((E::min() == 0), "");
+static_assert((E::max() == 0xull), "");
 static_assert((E::default_seed == 5489u), "");
 where(E::word_size);
 where(E::state_size);
Index: test/std/numerics/rand/rand.eng/rand.eng.lcong/values.pass.cpp
===
--- test/std/numerics/rand/rand.eng/rand.eng.lcong/values.pass.cpp
+++ test/std/numerics/rand/rand.eng/rand.eng.lcong/values.pass.cpp
@@ -37,8 +37,19 @@
 static_assert((LCE::multiplier == a), "");
 static_assert((LCE::increment == c), "");
 static_assert((LCE::modulus == m), "");
-/*static_*/assert((LCE::min() == (c == 0u ? 1u: 0u))/*, ""*/);
-/*static_*/assert((LCE::max() == result_type(m - 1u))/*, ""*/);
+static_assert((LCE::min() == (c == 0u ? 1u: 0u)), "");
+
+#ifdef _MSC_VER
+#pragma warning(push)
+#pragma warning(disable: 4310) // cast truncates constant value
+#endif // _MSC_VER
+
+static_assert((LCE::max() == result_type(m - 1u)), "");
+
+#ifdef _MSC_VER
+#pragma warning(pop)
+#endif // _MSC_VER
+
 static_assert((LCE::default_seed == 1), "");
 where(LCE::multiplier);
 where(LCE::increment);
Index: test/std/numerics/rand/rand.adapt/rand.adapt.shuf/values.pass.cpp
===
--- test/std/numerics/rand/rand.adapt/rand.adapt.shuf/values.pass.cpp
+++ test/std/numerics/rand/rand.adapt/rand.adapt.shuf/values.pass.cpp
@@ -33,8 +33,8 @@
 {
 typedef std::knuth_b E;
 static_assert(E::table_size == 256, "");
-/*static_*/assert((E::min() == 1)/*, ""*/);
-/*static_*/assert((E::max() == 2147483646)/*, ""*/);
+static_assert((E::min() == 1), "");
+static_assert((E::max() == 2147483646), "");
 where(E::table_size);
 }
 
Index: test/std/numerics/rand/rand.adapt/rand.adapt.ibits/values.pass.cpp
===

[PATCH] D26813: [libcxx] [test] allocator is non-Standard.

2016-11-17 Thread Stephan T. Lavavej via cfe-commits
STL_MSFT created this revision.
STL_MSFT added reviewers: EricWF, mclow.lists.
STL_MSFT added a subscriber: cfe-commits.

[libcxx] [test] allocator is non-Standard.

N4582 17.6.3.5 [allocator.requirements] says that allocators are given
cv-unqualified object types, and N4582 20.9.9 [default.allocator]
implies that allocator is ill-formed (due to colliding
address() overloads). Therefore, tests for allocator
should be marked as libcxx-specific (if not removed outright).


https://reviews.llvm.org/D26813

Files:
  
test/std/utilities/memory/default.allocator/allocator.members/allocate.size.pass.cpp
  test/std/utilities/memory/default.allocator/allocator_types.pass.cpp


Index: test/std/utilities/memory/default.allocator/allocator_types.pass.cpp
===
--- test/std/utilities/memory/default.allocator/allocator_types.pass.cpp
+++ test/std/utilities/memory/default.allocator/allocator_types.pass.cpp
@@ -32,6 +32,8 @@
 #include 
 #include 
 
+#include "test_macros.h"
+
 int main()
 {
 static_assert((std::is_same::size_type, 
std::size_t>::value), "");
@@ -45,7 +47,7 @@
 std::allocator >::value), "");
 
 static_assert((std::is_same::is_always_equal, 
std::true_type>::value), "");
-static_assert((std::is_same::is_always_equal, 
std::true_type>::value), "");
+LIBCPP_STATIC_ASSERT((std::is_same::is_always_equal, std::true_type>::value), "");
 
 std::allocator a;
 std::allocator a2 = a;
Index: 
test/std/utilities/memory/default.allocator/allocator.members/allocate.size.pass.cpp
===
--- 
test/std/utilities/memory/default.allocator/allocator.members/allocate.size.pass.cpp
+++ 
test/std/utilities/memory/default.allocator/allocator.members/allocate.size.pass.cpp
@@ -16,6 +16,8 @@
 #include 
 #include 
 
+#include "test_macros.h"
+
 template 
 void test_max(size_t count)
 {
@@ -27,23 +29,19 @@
 }
 }
 
-int main()
+template 
+void test()
 {
-{  // Bug 26812 -- allocating too large
-typedef double T;
-std::allocator a;
-test_max (a.max_size() + 1);// just barely too large
-test_max (a.max_size() * 2);// significantly too 
large
-test_max (((size_t) -1) / sizeof(T) + 1);   // multiply will 
overflow
-test_max ((size_t) -1); // way too large
-}
+// Bug 26812 -- allocating too large
+std::allocator a;
+test_max (a.max_size() + 1);// just barely too large
+test_max (a.max_size() * 2);// significantly too large
+test_max (((size_t) -1) / sizeof(T) + 1);   // multiply will overflow
+test_max ((size_t) -1); // way too large
+}
 
-{
-typedef const double T;
-std::allocator a;
-test_max (a.max_size() + 1);// just barely too large
-test_max (a.max_size() * 2);// significantly too 
large
-test_max (((size_t) -1) / sizeof(T) + 1);   // multiply will 
overflow
-test_max ((size_t) -1); // way too large
-}
+int main()
+{
+test();
+LIBCPP_ONLY(test());
 }


Index: test/std/utilities/memory/default.allocator/allocator_types.pass.cpp
===
--- test/std/utilities/memory/default.allocator/allocator_types.pass.cpp
+++ test/std/utilities/memory/default.allocator/allocator_types.pass.cpp
@@ -32,6 +32,8 @@
 #include 
 #include 
 
+#include "test_macros.h"
+
 int main()
 {
 static_assert((std::is_same::size_type, std::size_t>::value), "");
@@ -45,7 +47,7 @@
 std::allocator >::value), "");
 
 static_assert((std::is_same::is_always_equal, std::true_type>::value), "");
-static_assert((std::is_same::is_always_equal, std::true_type>::value), "");
+LIBCPP_STATIC_ASSERT((std::is_same::is_always_equal, std::true_type>::value), "");
 
 std::allocator a;
 std::allocator a2 = a;
Index: test/std/utilities/memory/default.allocator/allocator.members/allocate.size.pass.cpp
===
--- test/std/utilities/memory/default.allocator/allocator.members/allocate.size.pass.cpp
+++ test/std/utilities/memory/default.allocator/allocator.members/allocate.size.pass.cpp
@@ -16,6 +16,8 @@
 #include 
 #include 
 
+#include "test_macros.h"
+
 template 
 void test_max(size_t count)
 {
@@ -27,23 +29,19 @@
 }
 }
 
-int main()
+template 
+void test()
 {
-{  // Bug 26812 -- allocating too large
-typedef double T;
-std::allocator a;
-test_max (a.max_size() + 1);// just barely too large
-test_max (a.max_size() * 2);// significantly too large
-test_max (((size_t) -1) / sizeof(T) + 1);   // multiply will overflow
-test_max ((size_t) -1);  

[PATCH] D26814: [libcxx] [test] Change ifstream constructor tests to handle read-only files.

2016-11-17 Thread Stephan T. Lavavej via cfe-commits
STL_MSFT created this revision.
STL_MSFT added reviewers: EricWF, mclow.lists.
STL_MSFT added a subscriber: cfe-commits.
Herald added a subscriber: aemerson.

[libcxx] [test] Change ifstream constructor tests to handle read-only files.

Certain source control systems like to set the read-only bit on their files,
which interferes with opening "test.dat" for both input and output.
Fortunately, we can work around this without losing test coverage.
Now, the ifstream.cons tests harmlessly ignore failures to open files for
input and output simultaneously. The ofstream.cons tests are creating writable
files (not checked into source control), where the ifstream tests will succeed.


https://reviews.llvm.org/D26814

Files:
  test/std/input.output/file.streams/fstreams/ifstream.cons/pointer.pass.cpp
  test/std/input.output/file.streams/fstreams/ifstream.cons/string.pass.cpp
  test/std/input.output/file.streams/fstreams/ofstream.cons/pointer.pass.cpp
  test/std/input.output/file.streams/fstreams/ofstream.cons/string.pass.cpp

Index: test/std/input.output/file.streams/fstreams/ofstream.cons/string.pass.cpp
===
--- test/std/input.output/file.streams/fstreams/ofstream.cons/string.pass.cpp
+++ test/std/input.output/file.streams/fstreams/ofstream.cons/string.pass.cpp
@@ -31,6 +31,12 @@
 fs >> x;
 assert(x == 3.25);
 }
+{
+std::ifstream fs(temp, std::ios_base::out);
+double x = 0;
+fs >> x;
+assert(x == 3.25);
+}
 std::remove(temp.c_str());
 {
 std::wofstream fs(temp);
@@ -42,5 +48,11 @@
 fs >> x;
 assert(x == 3.25);
 }
+{
+std::wifstream fs(temp, std::ios_base::out);
+double x = 0;
+fs >> x;
+assert(x == 3.25);
+}
 std::remove(temp.c_str());
 }
Index: test/std/input.output/file.streams/fstreams/ofstream.cons/pointer.pass.cpp
===
--- test/std/input.output/file.streams/fstreams/ofstream.cons/pointer.pass.cpp
+++ test/std/input.output/file.streams/fstreams/ofstream.cons/pointer.pass.cpp
@@ -31,6 +31,12 @@
 fs >> x;
 assert(x == 3.25);
 }
+{
+std::ifstream fs(temp.c_str(), std::ios_base::out);
+double x = 0;
+fs >> x;
+assert(x == 3.25);
+}
 std::remove(temp.c_str());
 {
 std::wofstream fs(temp.c_str());
@@ -42,5 +48,11 @@
 fs >> x;
 assert(x == 3.25);
 }
+{
+std::wifstream fs(temp.c_str(), std::ios_base::out);
+double x = 0;
+fs >> x;
+assert(x == 3.25);
+}
 std::remove(temp.c_str());
 }
Index: test/std/input.output/file.streams/fstreams/ifstream.cons/string.pass.cpp
===
--- test/std/input.output/file.streams/fstreams/ifstream.cons/string.pass.cpp
+++ test/std/input.output/file.streams/fstreams/ifstream.cons/string.pass.cpp
@@ -27,9 +27,13 @@
 }
 {
 std::ifstream fs(std::string("test.dat"), std::ios_base::out);
-double x = 0;
-fs >> x;
-assert(x == 3.25);
+
+if (fs) // "test.dat" might be read-only.
+{ // See also: test/std/input.output/file.streams/fstreams/ofstream.cons/string.pass.cpp
+double x = 0;
+fs >> x;
+assert(x == 3.25);
+}
 }
 {
 std::wifstream fs(std::string("test.dat"));
@@ -39,8 +43,12 @@
 }
 {
 std::wifstream fs(std::string("test.dat"), std::ios_base::out);
-double x = 0;
-fs >> x;
-assert(x == 3.25);
+
+if (fs) // "test.dat" might be read-only.
+{ // See also: test/std/input.output/file.streams/fstreams/ofstream.cons/string.pass.cpp
+double x = 0;
+fs >> x;
+assert(x == 3.25);
+}
 }
 }
Index: test/std/input.output/file.streams/fstreams/ifstream.cons/pointer.pass.cpp
===
--- test/std/input.output/file.streams/fstreams/ifstream.cons/pointer.pass.cpp
+++ test/std/input.output/file.streams/fstreams/ifstream.cons/pointer.pass.cpp
@@ -27,9 +27,13 @@
 }
 {
 std::ifstream fs("test.dat", std::ios_base::out);
-double x = 0;
-fs >> x;
-assert(x == 3.25);
+
+if (fs) // "test.dat" might be read-only.
+{ // See also: test/std/input.output/file.streams/fstreams/ofstream.cons/pointer.pass.cpp
+double x = 0;
+fs >> x;
+assert(x == 3.25);
+}
 }
 {
 std::wifstream fs("test.dat");
@@ -39,8 +43,12 @@
 }
 {
 std::wifstream fs("test.dat", std::ios_base::out);
-double x = 0;
-fs >> x;
-assert(x == 3.25);
+
+if (fs) // "test.dat" might be read-only.
+{ // See also: test/std/input.output/file.streams/fstreams/ofstream.cons/

[PATCH] D26815: [libcxx] [test] Fix an assumption about the state of moved-from std::functions.

2016-11-17 Thread Stephan T. Lavavej via cfe-commits
STL_MSFT created this revision.
STL_MSFT added reviewers: EricWF, mclow.lists.
STL_MSFT added a subscriber: cfe-commits.

[libcxx] [test] Fix an assumption about the state of moved-from std::functions.

The Standard doesn't provide any guarantees beyond "valid but unspecified" for
moved-from std::functions. libcxx moves from small targets and leaves them
there, while MSVC's STL empties out the source. Mark these assertions as
libcxx-specific.


https://reviews.llvm.org/D26815

Files:
  
test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_move.pass.cpp


Index: 
test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_move.pass.cpp
===
--- 
test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_move.pass.cpp
+++ 
test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_move.pass.cpp
@@ -132,7 +132,7 @@
 assert(A::count == 1);
 assert(f2.target() == nullptr);
 assert(f2.target());
-assert(f.target()); // f is unchanged because the target is small
+LIBCPP_ASSERT(f.target()); // f is unchanged because the target 
is small
 }
 {
 // Test that moving a function constructed from a function pointer
@@ -146,7 +146,7 @@
 std::function f2(std::move(f));
 assert(f2.target() == nullptr);
 assert(f2.target());
-assert(f.target()); // f is unchanged because the target is small
+LIBCPP_ASSERT(f.target()); // f is unchanged because the target 
is small
 }
 #endif  // TEST_STD_VER >= 11
 }


Index: test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_move.pass.cpp
===
--- test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_move.pass.cpp
+++ test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_move.pass.cpp
@@ -132,7 +132,7 @@
 assert(A::count == 1);
 assert(f2.target() == nullptr);
 assert(f2.target());
-assert(f.target()); // f is unchanged because the target is small
+LIBCPP_ASSERT(f.target()); // f is unchanged because the target is small
 }
 {
 // Test that moving a function constructed from a function pointer
@@ -146,7 +146,7 @@
 std::function f2(std::move(f));
 assert(f2.target() == nullptr);
 assert(f2.target());
-assert(f.target()); // f is unchanged because the target is small
+LIBCPP_ASSERT(f.target()); // f is unchanged because the target is small
 }
 #endif  // TEST_STD_VER >= 11
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D26816: [libcxx] [test] Fix non-Standard assumptions when testing sample().

2016-11-17 Thread Stephan T. Lavavej via cfe-commits
STL_MSFT created this revision.
STL_MSFT added reviewers: EricWF, mclow.lists.
STL_MSFT added a subscriber: cfe-commits.

[libcxx] [test] Fix non-Standard assumptions when testing sample().

sample() isn't specified with a reproducible algorithm, so expecting
exact output is non-Standard. Mark those tests with LIBCPP_ASSERT.

In test_small_population(), we're guaranteed to get all of the elements,
but not necessarily in their original order. When PopulationCategory is
forward, we're guaranteed stability (and can therefore test equal()).
Otherwise, we can only test is_permutation(). (As it happens, both libcxx
and MSVC's STL provide stability in this scenario for input-only iterators.)


https://reviews.llvm.org/D26816

Files:
  test/std/algorithms/alg.modifying.operations/alg.random.sample/sample.pass.cpp


Index: 
test/std/algorithms/alg.modifying.operations/alg.random.sample/sample.pass.cpp
===
--- 
test/std/algorithms/alg.modifying.operations/alg.random.sample/sample.pass.cpp
+++ 
test/std/algorithms/alg.modifying.operations/alg.random.sample/sample.pass.cpp
@@ -19,9 +19,11 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include "test_iterators.h"
+#include "test_macros.h"
 
 struct ReservoirSampleExpectations {
   enum { os = 4 };
@@ -60,19 +62,21 @@
   const unsigned os = Expectations::os;
   SampleItem oa[os];
   const int *oa1 = Expectations::oa1;
+  ((void)oa1); // Prevent unused warning
   const int *oa2 = Expectations::oa2;
+  ((void)oa2); // Prevent unused warning
   std::minstd_rand g;
   SampleIterator end;
   end = std::sample(PopulationIterator(ia),
   PopulationIterator(ia + is),
   SampleIterator(oa), os, g);
   assert(end.base() - oa == std::min(os, is));
-  assert(std::equal(oa, oa + os, oa1));
+  LIBCPP_ASSERT(std::equal(oa, oa + os, oa1));
   end = std::sample(PopulationIterator(ia),
   PopulationIterator(ia + is),
   SampleIterator(oa), os, std::move(g));
   assert(end.base() - oa == std::min(os, is));
-  assert(std::equal(oa, oa + os, oa2));
+  LIBCPP_ASSERT(std::equal(oa, oa + os, oa2));
 }
 
 template  class PopulationIteratorType, class 
PopulationItem,
@@ -122,7 +126,12 @@
   PopulationIterator(ia + is),
   SampleIterator(oa), os, g);
   assert(end.base() - oa == std::min(os, is));
-  assert(std::equal(oa, end.base(), oa1));
+  typedef typename std::iterator_traits::iterator_category 
PopulationCategory;
+  if (std::is_base_of::value) {
+assert(std::equal(oa, end.base(), oa1));
+  } else {
+assert(std::is_permutation(oa, end.base(), oa1));
+  }
 }
 
 int main() {


Index: test/std/algorithms/alg.modifying.operations/alg.random.sample/sample.pass.cpp
===
--- test/std/algorithms/alg.modifying.operations/alg.random.sample/sample.pass.cpp
+++ test/std/algorithms/alg.modifying.operations/alg.random.sample/sample.pass.cpp
@@ -19,9 +19,11 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include "test_iterators.h"
+#include "test_macros.h"
 
 struct ReservoirSampleExpectations {
   enum { os = 4 };
@@ -60,19 +62,21 @@
   const unsigned os = Expectations::os;
   SampleItem oa[os];
   const int *oa1 = Expectations::oa1;
+  ((void)oa1); // Prevent unused warning
   const int *oa2 = Expectations::oa2;
+  ((void)oa2); // Prevent unused warning
   std::minstd_rand g;
   SampleIterator end;
   end = std::sample(PopulationIterator(ia),
   PopulationIterator(ia + is),
   SampleIterator(oa), os, g);
   assert(end.base() - oa == std::min(os, is));
-  assert(std::equal(oa, oa + os, oa1));
+  LIBCPP_ASSERT(std::equal(oa, oa + os, oa1));
   end = std::sample(PopulationIterator(ia),
   PopulationIterator(ia + is),
   SampleIterator(oa), os, std::move(g));
   assert(end.base() - oa == std::min(os, is));
-  assert(std::equal(oa, oa + os, oa2));
+  LIBCPP_ASSERT(std::equal(oa, oa + os, oa2));
 }
 
 template  class PopulationIteratorType, class PopulationItem,
@@ -122,7 +126,12 @@
   PopulationIterator(ia + is),
   SampleIterator(oa), os, g);
   assert(end.base() - oa == std::min(os, is));
-  assert(std::equal(oa, end.base(), oa1));
+  typedef typename std::iterator_traits::iterator_category PopulationCategory;
+  if (std::is_base_of::value) {
+assert(std::equal(oa, end.base(), oa1));
+  } else {
+assert(std::is_permutation(oa, end.base(), oa1));
+  }
 }
 
 int main() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r287262 - [CrashReproducer][Darwin] Suggest attaching .crash diagnostic file

2016-11-17 Thread Bruno Cardoso Lopes via cfe-commits
Author: bruno
Date: Thu Nov 17 15:41:22 2016
New Revision: 287262

URL: http://llvm.org/viewvc/llvm-project?rev=287262&view=rev
Log:
[CrashReproducer][Darwin] Suggest attaching .crash diagnostic file

In addition to the preprocessed sources file and reproducer script, also
point to the .crash diagnostic files on Darwin. Example:

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang-4.0: note: diagnostic msg: 
/var/folders/bk/1hj20g8j4xvdj5gd25ywhd3mgq/T/RegAllocGreedy-238f28.cpp
clang-4.0: note: diagnostic msg: 
/var/folders/bk/1hj20g8j4xvdj5gd25ywhd3mgq/T/RegAllocGreedy-238f28.cache
clang-4.0: note: diagnostic msg: 
/var/folders/bk/1hj20g8j4xvdj5gd25ywhd3mgq/T/RegAllocGreedy-238f28.sh
clang-4.0: note: diagnostic msg: 
/var/folders/bk/1hj20g8j4xvdj5gd25ywhd3mgq/T/RegAllocGreedy-238f28.crash

When no match is found for the .crash, point the user to a directory
where those can be found. Example:

clang-4.0: note: diagnostic msg: Crash backtrace is located in
clang-4.0: note: diagnostic msg: 
/Users/bruno/Library/Logs/DiagnosticReports/clang-4.0__.crash
clang-4.0: note: diagnostic msg: (choose the .crash file that corresponds to 
your crash)

rdar://problem/27286266

Added:
cfe/trunk/test/Driver/crash-report-crashfile.m
Modified:
cfe/trunk/include/clang/Driver/Driver.h
cfe/trunk/lib/Driver/Driver.cpp

Modified: cfe/trunk/include/clang/Driver/Driver.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=287262&r1=287261&r2=287262&view=diff
==
--- cfe/trunk/include/clang/Driver/Driver.h (original)
+++ cfe/trunk/include/clang/Driver/Driver.h Thu Nov 17 15:41:22 2016
@@ -246,6 +246,20 @@ private:
   void generatePrefixedToolNames(StringRef Tool, const ToolChain &TC,
  SmallVectorImpl &Names) const;
 
+  /// \brief Find the appropriate .crash diagonostic file for the child crash
+  /// under this driver and copy it out to a temporary destination with the
+  /// other reproducer related files (.sh, .cache, etc). If not found, suggest 
a
+  /// directory for the user to look at.
+  ///
+  /// \param The file path to copy the .crash to.
+  /// \param The suggested directory for the user to look at in case the search
+  /// or copy fails.
+  ///
+  /// \returns If the .crash is found and successfully copied return true,
+  /// otherwise false and return the suggested directory in \p CrashDiagDir.
+  bool getCrashDiagnosticFile(StringRef ReproCrashFilename,
+  SmallString<128> &CrashDiagDir);
+
 public:
   Driver(StringRef ClangExecutable, StringRef DefaultTargetTriple,
  DiagnosticsEngine &Diags,

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=287262&r1=287261&r2=287262&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Thu Nov 17 15:41:22 2016
@@ -42,6 +42,9 @@
 #include 
 #include 
 #include 
+#if LLVM_ON_UNIX
+#include  // getpid
+#endif
 
 using namespace clang::driver;
 using namespace clang;
@@ -695,6 +698,95 @@ static void printArgList(raw_ostream &OS
   OS << '\n';
 }
 
+bool Driver::getCrashDiagnosticFile(StringRef ReproCrashFilename,
+SmallString<128> &CrashDiagDir) {
+  using namespace llvm::sys;
+  assert(llvm::Triple(llvm::sys::getProcessTriple()).isOSDarwin() &&
+ "Only knows about .crash files on Darwin");
+
+  // The .crash file can be found on at ~/Library/Logs/DiagnosticReports/
+  // (or /Library/Logs/DiagnosticReports for root) and has the filename pattern
+  // clang-__.crash.
+  path::home_directory(CrashDiagDir);
+  if (CrashDiagDir.startswith("/var/root"))
+CrashDiagDir = "/";
+  path::append(CrashDiagDir, "Library/Logs/DiagnosticReports");
+  int PID =
+#if LLVM_ON_UNIX
+  getpid();
+#else
+  0;
+#endif
+  std::error_code EC;
+  fs::file_status FileStatus;
+  TimePoint<> LastAccessTime;
+  SmallString<128> CrashFilePath;
+  // Lookup the .crash files and get the one generated by a subprocess spawned
+  // by this driver invocation.
+  for (fs::directory_iterator File(CrashDiagDir, EC), FileEnd;
+   File != FileEnd && !EC; File.increment(EC)) {
+StringRef FileName = path::filename(File->path());
+if (!FileName.startswith(Name))
+  continue;
+if (fs::status(File->path(), FileStatus))
+  continue;
+llvm::ErrorOr> CrashFile =
+llvm::MemoryBuffer::getFile(File->path());
+if (!CrashFile)
+  continue;
+// The first line should start with "Process:", otherwise this isn't a real
+// .crash file.
+StringRef Data = CrashFile.get()->getBuffer();
+if (!Data.startswith("Process:"))
+  continue;
+// Parse parent process pi

[PATCH] D25949: [Driver] Refactor distro detection & classification as a separate API

2016-11-17 Thread Bruno Cardoso Lopes via cfe-commits
bruno accepted this revision.
bruno added a comment.
This revision is now accepted and ready to land.

Very nice! LGTM


https://reviews.llvm.org/D25949



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


r287275 - [Preprocessor] Support for '-dI' flag

2016-11-17 Thread Bruno Cardoso Lopes via cfe-commits
Author: bruno
Date: Thu Nov 17 16:45:31 2016
New Revision: 287275

URL: http://llvm.org/viewvc/llvm-project?rev=287275&view=rev
Log:
[Preprocessor] Support for '-dI' flag

Re-introduce r285411.

Implement the -dI as supported by GCC: Output ‘#include’ directives in addition
to the result of preprocessing.

This change aims to add this option, pass it through to the preprocessor via
the options class, and when inclusions occur we output some information (+ test
cases).

Patch by Steve O'Brien!

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

Added:
cfe/trunk/test/Preprocessor/dump_import.h
cfe/trunk/test/Preprocessor/dump_import.m
cfe/trunk/test/Preprocessor/dump_include.c
cfe/trunk/test/Preprocessor/dump_include.h
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/PreprocessorOutputOptions.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=287275&r1=287274&r2=287275&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Nov 17 16:45:31 2016
@@ -429,6 +429,8 @@ def fno_cuda_approx_transcendentals : Fl
 def dA : Flag<["-"], "dA">, Group;
 def dD : Flag<["-"], "dD">, Group, Flags<[CC1Option]>,
   HelpText<"Print macro definitions in -E mode in addition to normal output">;
+def dI : Flag<["-"], "dI">, Group, Flags<[CC1Option]>,
+  HelpText<"Print include directives in -E mode in addition to normal output">;
 def dM : Flag<["-"], "dM">, Group, Flags<[CC1Option]>,
   HelpText<"Print macro definitions in -E mode instead of normal output">;
 def dead__strip : Flag<["-"], "dead_strip">;

Modified: cfe/trunk/include/clang/Frontend/PreprocessorOutputOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/PreprocessorOutputOptions.h?rev=287275&r1=287274&r2=287275&view=diff
==
--- cfe/trunk/include/clang/Frontend/PreprocessorOutputOptions.h (original)
+++ cfe/trunk/include/clang/Frontend/PreprocessorOutputOptions.h Thu Nov 17 
16:45:31 2016
@@ -22,6 +22,7 @@ public:
   unsigned UseLineDirectives : 1;   ///< Use \#line instead of GCC-style \# N.
   unsigned ShowMacroComments : 1;  ///< Show comments, even in macros.
   unsigned ShowMacros : 1; ///< Print macro definitions.
+  unsigned ShowIncludeDirectives : 1;  ///< Print includes, imports etc. 
within preprocessed output.
   unsigned RewriteIncludes : 1;///< Preprocess include directives only.
 
 public:
@@ -32,6 +33,7 @@ public:
 UseLineDirectives = 0;
 ShowMacroComments = 0;
 ShowMacros = 0;
+ShowIncludeDirectives = 0;
 RewriteIncludes = 0;
   }
 };

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=287275&r1=287274&r2=287275&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Thu Nov 17 16:45:31 2016
@@ -2357,6 +2357,7 @@ static void ParsePreprocessorOutputArgs(
   Opts.ShowLineMarkers = !Args.hasArg(OPT_P);
   Opts.ShowMacroComments = Args.hasArg(OPT_CC);
   Opts.ShowMacros = Args.hasArg(OPT_dM) || Args.hasArg(OPT_dD);
+  Opts.ShowIncludeDirectives = Args.hasArg(OPT_dI);
   Opts.RewriteIncludes = Args.hasArg(OPT_frewrite_includes);
   Opts.UseLineDirectives = Args.hasArg(OPT_fuse_line_directives);
 }

Modified: cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp?rev=287275&r1=287274&r2=287275&view=diff
==
--- cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp (original)
+++ cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp Thu Nov 17 16:45:31 2016
@@ -93,13 +93,16 @@ private:
   bool Initialized;
   bool DisableLineMarkers;
   bool DumpDefines;
+  bool DumpIncludeDirectives;
   bool UseLineDirectives;
   bool IsFirstFileEntered;
 public:
   PrintPPOutputPPCallbacks(Preprocessor &pp, raw_ostream &os, bool lineMarkers,
-   bool defines, bool UseLineDirectives)
+   bool defines, bool DumpIncludeDirectives,
+   bool UseLineDirectives)
   : PP(pp), SM(PP.getSourceManager()), ConcatInfo(PP), OS(os),
 DisableLineMarkers(lineMarkers), DumpDefines(defines),
+DumpIncludeDirectives(DumpIncludeDirectives),
 UseLineDirectives(UseLineDirectives) {
 CurLine = 0;
 CurFilename += "";
@@ -320,10 +323,10 @@ void PrintPPOutputPPCallbacks::I

[PATCH] D21298: [Clang-tidy] delete null check

2016-11-17 Thread Gergely Angeli via cfe-commits
SilverGeri added inline comments.



Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:7
+  int *p = 0;
+  if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; 
deleting null pointer has no effect [readability-delete-null-pointer]

aaron.ballman wrote:
> hokein wrote:
> > Does it work the case like:
> > 
> > ```
> > int *p = nullptr;
> > if (p == nullptr) {
> >p = new int[3];
> >delete[] p;
> > }
> > ```
> > 
> > ?
> Similarly, it should not mishandle a case like:
> 
> void f(int *p) {
>   if (p) {
> delete p;
>   } else {
> // Do something else
>   }
> }
it warns only if the compund statement contains only one statement (which is 
the delete). We want to warn because it is unnecessary to check the pointer 
validity if you want to just call `delete`. In other cases, we can't be sure 
about the actual behaviour.


https://reviews.llvm.org/D21298



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


  1   2   >