Re: Add warning for c++ member variable shadowing

[James Sun via cfe-commits]

Thanks Richard! Hopefully this is the last patch :D
Could you please help me to commit it maybe?

Thanks

James

From:  on behalf of Richard Smith 
Date: Saturday, February 4, 2017 at 10:43 PM
To: James Sun 
Cc: Saleem Abdulrasool , "cfe-commits@lists.llvm.org" 
, Aaron Ballman 
Subject: Re: Add warning for c++ member variable shadowing

Thanks, just one more thing I noticed (sorry!) and this looks good to go.

+def warn_shadow_field : Warning<
+  "non-static data member '%0' of '%1' shadows member inherited from type 
'%2'">,
+   InGroup;

-Wshadow-ivar doesn't really make sense for this; that's for an Objective-C 
feature. A new -Wshadow-field group might make sense (especially since we 
already have -Wshadow-field-in-constructor). Also, the other -Wshadow warnings 
(other than -Wshadow-ivar) are DefaultIgnore; this one should probably be too.

Do you need someone to commit for you?

On 4 February 2017 at 22:21, James Sun 
> wrote:
oops

From: James Sun >
Date: Saturday, February 4, 2017 at 9:19 PM

To: Richard Smith >
Cc: Saleem Abdulrasool >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>
Subject: Re: Add warning for c++ member variable shadowing

updated

From: James Sun >
Date: Saturday, February 4, 2017 at 6:52 PM
To: Richard Smith >
Cc: Saleem Abdulrasool >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>
Subject: Re: Add warning for c++ member variable shadowing

Ok I get your point. Suppose there are two paths from class B to base class A. 
One is with access as_none; the other is as_public. Then there is a chance that 
the as_none path is recorded and the as_public one is skipped. In this case we 
should give the warning as well. Should be easy to fix with the existing map. 
Will do.

Sent from my iPhone

On Feb 4, 2017, at 6:22 PM, Richard Smith 
> wrote:
Hmm, now we're emitting one warning per path, it looks like we might diagnose 
shadowing the same field more than once (for a repeated non-virtual base 
class). Is that intentional? Maybe we should just skip producing the warning if 
the lookup would be ambiguous, since any use of the shadowed field would 
otherwise be ill-formed.

On 4 February 2017 at 17:48, James Sun 
> wrote:
Thanks Richard! Good catch! The updated version is attached. --James

From: > on behalf of Richard Smith 
>
Date: Thursday, February 2, 2017 at 11:59 AM
To: James Sun >
Cc: Saleem Abdulrasool >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>
Subject: Re: Add warning for c++ member variable shadowing

Thanks, James! I think I have only one more substantive comment:

+  (Field->getAccess() == AS_public || Field->getAccess() == 
AS_protected)) {

Have you considered also taking into account the access of the inheritance 
path? Eg, a public member of a private base class of a public base class is 
typically inaccessible, even though it was declared public:

  struct A { int n; };
  struct B : private A {};
  struct C : B { int n; }; // A::n is not accessible here, should we suppress 
the warning?

You can use CXXRecordDecl::MergeAccess to combine the access of the path with 
the access of the field and compute the effective access of the field in the 
derived class (and you should test to see if the resulting access is AS_None to 
tell if the field is inaccessible; fields with effective access of AS_Private 
-- such as public members of a private direct base class -- are accessible from 
the derived class). You'll need to set RecordPaths to true in the CXXBasePaths 
object in order for lookupInBases to compute the path access.

Oh, and you may as well use a range-based for loop here:

+auto Result = Base->lookup(FieldName);
+for (auto I = Result.begin(); I != Result.end(); ++I) {


On 2 February 2017 at 00:19, James Sun 
> wrote:
Hi 

Re: Add warning for c++ member variable shadowing

[Richard Smith via cfe-commits]

Thanks, just one more thing I noticed (sorry!) and this looks good to go.

+def warn_shadow_field : Warning<
+  "non-static data member '%0' of '%1' shadows member inherited from type
'%2'">,
+   InGroup;

-Wshadow-ivar doesn't really make sense for this; that's for an Objective-C
feature. A new -Wshadow-field group might make sense (especially since we
already have -Wshadow-field-in-constructor). Also, the other -Wshadow
warnings (other than -Wshadow-ivar) are DefaultIgnore; this one should
probably be too.

Do you need someone to commit for you?

On 4 February 2017 at 22:21, James Sun  wrote:

> oops
>
>
>
> *From: *James Sun 
> *Date: *Saturday, February 4, 2017 at 9:19 PM
>
> *To: *Richard Smith 
> *Cc: *Saleem Abdulrasool , "
> cfe-commits@lists.llvm.org" , Aaron Ballman <
> aa...@aaronballman.com>
> *Subject: *Re: Add warning for c++ member variable shadowing
>
>
>
> updated
>
>
>
> *From: *James Sun 
> *Date: *Saturday, February 4, 2017 at 6:52 PM
> *To: *Richard Smith 
> *Cc: *Saleem Abdulrasool , "
> cfe-commits@lists.llvm.org" , Aaron Ballman <
> aa...@aaronballman.com>
> *Subject: *Re: Add warning for c++ member variable shadowing
>
>
>
> Ok I get your point. Suppose there are two paths from class B to base
> class A. One is with access as_none; the other is as_public. Then there is
> a chance that the as_none path is recorded and the as_public one is
> skipped. In this case we should give the warning as well. Should be easy to
> fix with the existing map. Will do.
>
> Sent from my iPhone
>
>
> On Feb 4, 2017, at 6:22 PM, Richard Smith  wrote:
>
> Hmm, now we're emitting one warning per path, it looks like we might
> diagnose shadowing the same field more than once (for a repeated
> non-virtual base class). Is that intentional? Maybe we should just skip
> producing the warning if the lookup would be ambiguous, since any use of
> the shadowed field would otherwise be ill-formed.
>
>
>
> On 4 February 2017 at 17:48, James Sun  wrote:
>
> Thanks Richard! Good catch! The updated version is attached. --James
>
>
>
> *From: * on behalf of Richard Smith <
> rich...@metafoo.co.uk>
> *Date: *Thursday, February 2, 2017 at 11:59 AM
>
> *To: *James Sun 
> *Cc: *Saleem Abdulrasool , "
> cfe-commits@lists.llvm.org" , Aaron Ballman <
> aa...@aaronballman.com>
> *Subject: *Re: Add warning for c++ member variable shadowing
>
>
>
> Thanks, James! I think I have only one more substantive comment:
>
>
>
> +  (Field->getAccess() == AS_public || Field->getAccess() ==
> AS_protected)) {
>
>
>
> Have you considered also taking into account the access of the inheritance
> path? Eg, a public member of a private base class of a public base class is
> typically inaccessible, even though it was declared public:
>
>
>
>   struct A { int n; };
>
>   struct B : private A {};
>
>   struct C : B { int n; }; // A::n is not accessible here, should we
> suppress the warning?
>
>
>
> You can use CXXRecordDecl::MergeAccess to combine the access of the path
> with the access of the field and compute the effective access of the field
> in the derived class (and you should test to see if the resulting access is
> AS_None to tell if the field is inaccessible; fields with effective access
> of AS_Private -- such as public members of a private direct base class --
> are accessible from the derived class). You'll need to set RecordPaths to
> true in the CXXBasePaths object in order for lookupInBases to compute the
> path access.
>
>
>
> Oh, and you may as well use a range-based for loop here:
>
>
>
> +auto Result = Base->lookup(FieldName);
>
> +for (auto I = Result.begin(); I != Result.end(); ++I) {
>
>
>
>
>
> On 2 February 2017 at 00:19, James Sun  wrote:
>
> Hi Richard
>
>
>
> Thanks for the feedback! Hopefully addressed!
>
>
>
> Thanks
>
>
>
> James
>
>
>
>
>
>
>
> *From: * on behalf of Richard Smith <
> rich...@metafoo.co.uk>
> *Date: *Wednesday, February 1, 2017 at 3:50 PM
> *To: *James Sun 
>
>
> *Cc: *Saleem Abdulrasool , "
> cfe-commits@lists.llvm.org" , Aaron Ballman <
> aa...@aaronballman.com>
> *Subject: *Re: Add warning for c++ member variable shadowing
>
>
>
> +  std::set bases;
>
> +const auto baseName = Specifier->getType()->
> getAsCXXRecordDecl()->getName();
>
>
>
> Please capitalize local variable names. Also, please don't use the record
> name as a key in your set; that's not guaranteed to be unique. Instead, you
> could either use a set of canonical types or of canonical CXXRecordDecl*s.
>
>
>
> +for (const auto *Field : Specifier->getType()->
> 

Re: Add warning for c++ member variable shadowing

[James Sun via cfe-commits]

oops

From: James Sun 
Date: Saturday, February 4, 2017 at 9:19 PM
To: Richard Smith 
Cc: Saleem Abdulrasool , "cfe-commits@lists.llvm.org" 
, Aaron Ballman 
Subject: Re: Add warning for c++ member variable shadowing

updated

From: James Sun 
Date: Saturday, February 4, 2017 at 6:52 PM
To: Richard Smith 
Cc: Saleem Abdulrasool , "cfe-commits@lists.llvm.org" 
, Aaron Ballman 
Subject: Re: Add warning for c++ member variable shadowing

Ok I get your point. Suppose there are two paths from class B to base class A. 
One is with access as_none; the other is as_public. Then there is a chance that 
the as_none path is recorded and the as_public one is skipped. In this case we 
should give the warning as well. Should be easy to fix with the existing map. 
Will do.

Sent from my iPhone

On Feb 4, 2017, at 6:22 PM, Richard Smith 
> wrote:
Hmm, now we're emitting one warning per path, it looks like we might diagnose 
shadowing the same field more than once (for a repeated non-virtual base 
class). Is that intentional? Maybe we should just skip producing the warning if 
the lookup would be ambiguous, since any use of the shadowed field would 
otherwise be ill-formed.

On 4 February 2017 at 17:48, James Sun 
> wrote:
Thanks Richard! Good catch! The updated version is attached. --James

From: > on behalf of Richard Smith 
>
Date: Thursday, February 2, 2017 at 11:59 AM
To: James Sun >
Cc: Saleem Abdulrasool >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>
Subject: Re: Add warning for c++ member variable shadowing

Thanks, James! I think I have only one more substantive comment:

+  (Field->getAccess() == AS_public || Field->getAccess() == 
AS_protected)) {

Have you considered also taking into account the access of the inheritance 
path? Eg, a public member of a private base class of a public base class is 
typically inaccessible, even though it was declared public:

  struct A { int n; };
  struct B : private A {};
  struct C : B { int n; }; // A::n is not accessible here, should we suppress 
the warning?

You can use CXXRecordDecl::MergeAccess to combine the access of the path with 
the access of the field and compute the effective access of the field in the 
derived class (and you should test to see if the resulting access is AS_None to 
tell if the field is inaccessible; fields with effective access of AS_Private 
-- such as public members of a private direct base class -- are accessible from 
the derived class). You'll need to set RecordPaths to true in the CXXBasePaths 
object in order for lookupInBases to compute the path access.

Oh, and you may as well use a range-based for loop here:

+auto Result = Base->lookup(FieldName);
+for (auto I = Result.begin(); I != Result.end(); ++I) {


On 2 February 2017 at 00:19, James Sun 
> wrote:
Hi Richard

Thanks for the feedback! Hopefully addressed!

Thanks

James



From: > on behalf of Richard Smith 
>
Date: Wednesday, February 1, 2017 at 3:50 PM
To: James Sun >

Cc: Saleem Abdulrasool >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>
Subject: Re: Add warning for c++ member variable shadowing

+  std::set bases;
+const auto baseName = 
Specifier->getType()->getAsCXXRecordDecl()->getName();

Please capitalize local variable names. Also, please don't use the record name 
as a key in your set; that's not guaranteed to be unique. Instead, you could 
either use a set of canonical types or of canonical CXXRecordDecl*s.

+for (const auto *Field : 
Specifier->getType()->getAsCXXRecordDecl()->fields()) {
+  if ((Field->getAccess() == AS_public || Field->getAccess() == 
AS_protected) &&
+  Field->getName() == FieldName) {

Use Specifier->getType()->getAsCXXRecordDecl()->lookup(Field->getName()) here 
to look up the field by name, rather than walking all the fields of all base 
classes and checking if each of them has the right name. You should also check 
for IndirectFieldDecls, for this case:

  struct A {

Re: r293065 - Clarify how to forward-declare __llvm_profile symbols.

[Sean Silva via cfe-commits]

Thanks!

On Wed, Jan 25, 2017 at 8:01 AM, Nico Weber via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: nico
> Date: Wed Jan 25 10:01:32 2017
> New Revision: 293065
>
> URL: http://llvm.org/viewvc/llvm-project?rev=293065=rev
> Log:
> Clarify how to forward-declare __llvm_profile symbols.
>
> Modified:
> cfe/trunk/docs/SourceBasedCodeCoverage.rst
>
> Modified: cfe/trunk/docs/SourceBasedCodeCoverage.rst
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/
> SourceBasedCodeCoverage.rst?rev=293065=293064=293065=diff
> 
> ==
> --- cfe/trunk/docs/SourceBasedCodeCoverage.rst (original)
> +++ cfe/trunk/docs/SourceBasedCodeCoverage.rst Wed Jan 25 10:01:32 2017
> @@ -256,6 +256,8 @@ without using static initializers, do th
>otherwise. Calling this function multiple times appends profile data to
> an
>existing on-disk raw profile.
>
> +In C++ files, declare these as ``extern "C"``.
> +
>  Collecting coverage reports for the llvm project
>  
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: Add warning for c++ member variable shadowing

[James Sun via cfe-commits]

updated

From: James Sun 
Date: Saturday, February 4, 2017 at 6:52 PM
To: Richard Smith 
Cc: Saleem Abdulrasool , "cfe-commits@lists.llvm.org" 
, Aaron Ballman 
Subject: Re: Add warning for c++ member variable shadowing

Ok I get your point. Suppose there are two paths from class B to base class A. 
One is with access as_none; the other is as_public. Then there is a chance that 
the as_none path is recorded and the as_public one is skipped. In this case we 
should give the warning as well. Should be easy to fix with the existing map. 
Will do.

Sent from my iPhone

On Feb 4, 2017, at 6:22 PM, Richard Smith 
> wrote:
Hmm, now we're emitting one warning per path, it looks like we might diagnose 
shadowing the same field more than once (for a repeated non-virtual base 
class). Is that intentional? Maybe we should just skip producing the warning if 
the lookup would be ambiguous, since any use of the shadowed field would 
otherwise be ill-formed.

On 4 February 2017 at 17:48, James Sun 
> wrote:
Thanks Richard! Good catch! The updated version is attached. --James

From: > on behalf of Richard Smith 
>
Date: Thursday, February 2, 2017 at 11:59 AM
To: James Sun >
Cc: Saleem Abdulrasool >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>
Subject: Re: Add warning for c++ member variable shadowing

Thanks, James! I think I have only one more substantive comment:

+  (Field->getAccess() == AS_public || Field->getAccess() == 
AS_protected)) {

Have you considered also taking into account the access of the inheritance 
path? Eg, a public member of a private base class of a public base class is 
typically inaccessible, even though it was declared public:

  struct A { int n; };
  struct B : private A {};
  struct C : B { int n; }; // A::n is not accessible here, should we suppress 
the warning?

You can use CXXRecordDecl::MergeAccess to combine the access of the path with 
the access of the field and compute the effective access of the field in the 
derived class (and you should test to see if the resulting access is AS_None to 
tell if the field is inaccessible; fields with effective access of AS_Private 
-- such as public members of a private direct base class -- are accessible from 
the derived class). You'll need to set RecordPaths to true in the CXXBasePaths 
object in order for lookupInBases to compute the path access.

Oh, and you may as well use a range-based for loop here:

+auto Result = Base->lookup(FieldName);
+for (auto I = Result.begin(); I != Result.end(); ++I) {


On 2 February 2017 at 00:19, James Sun 
> wrote:
Hi Richard

Thanks for the feedback! Hopefully addressed!

Thanks

James



From: > on behalf of Richard Smith 
>
Date: Wednesday, February 1, 2017 at 3:50 PM
To: James Sun >

Cc: Saleem Abdulrasool >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>
Subject: Re: Add warning for c++ member variable shadowing

+  std::set bases;
+const auto baseName = 
Specifier->getType()->getAsCXXRecordDecl()->getName();

Please capitalize local variable names. Also, please don't use the record name 
as a key in your set; that's not guaranteed to be unique. Instead, you could 
either use a set of canonical types or of canonical CXXRecordDecl*s.

+for (const auto *Field : 
Specifier->getType()->getAsCXXRecordDecl()->fields()) {
+  if ((Field->getAccess() == AS_public || Field->getAccess() == 
AS_protected) &&
+  Field->getName() == FieldName) {

Use Specifier->getType()->getAsCXXRecordDecl()->lookup(Field->getName()) here 
to look up the field by name, rather than walking all the fields of all base 
classes and checking if each of them has the right name. You should also check 
for IndirectFieldDecls, for this case:

  struct A {
union { int x; float f; };
  };
  struct B : A {
int x;
  };

+bases.emplace(baseName);

It's more efficient to use insert rather than emplace when inserting an element 
into a set.

+Diag(Loc, diag::warn_shadow_field)
+  << FieldName << RD->getName() << baseName;

It'd be nice to add a note here pointing at 

[PATCH] D29557: [clang-tools-extra] Fix pthread link

[Eric Fiselier via Phabricator via cfe-commits]

EricWF created this revision.
Herald added a subscriber: mgorny.

This patch contains the clang-tools-extra changes required by 
https://reviews.llvm.org/D29555


https://reviews.llvm.org/D29557

Files:
  include-fixer/plugin/CMakeLists.txt


Index: include-fixer/plugin/CMakeLists.txt
===
--- include-fixer/plugin/CMakeLists.txt
+++ include-fixer/plugin/CMakeLists.txt
@@ -9,5 +9,5 @@
   clangParse
   clangSema
   clangTooling
-  ${PTHREAD_LIB}
+  ${LLVM_PTHREAD_LIB}
   )


Index: include-fixer/plugin/CMakeLists.txt
===
--- include-fixer/plugin/CMakeLists.txt
+++ include-fixer/plugin/CMakeLists.txt
@@ -9,5 +9,5 @@
   clangParse
   clangSema
   clangTooling
-  ${PTHREAD_LIB}
+  ${LLVM_PTHREAD_LIB}
   )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: Add warning for c++ member variable shadowing

[James Sun via cfe-commits]

Ok I get your point. Suppose there are two paths from class B to base class A. 
One is with access as_none; the other is as_public. Then there is a chance that 
the as_none path is recorded and the as_public one is skipped. In this case we 
should give the warning as well. Should be easy to fix with the existing map. 
Will do.

Sent from my iPhone

On Feb 4, 2017, at 6:22 PM, Richard Smith 
> wrote:

Hmm, now we're emitting one warning per path, it looks like we might diagnose 
shadowing the same field more than once (for a repeated non-virtual base 
class). Is that intentional? Maybe we should just skip producing the warning if 
the lookup would be ambiguous, since any use of the shadowed field would 
otherwise be ill-formed.

On 4 February 2017 at 17:48, James Sun 
> wrote:
Thanks Richard! Good catch! The updated version is attached. --James

From: > on behalf of Richard Smith 
>
Date: Thursday, February 2, 2017 at 11:59 AM
To: James Sun >
Cc: Saleem Abdulrasool >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>
Subject: Re: Add warning for c++ member variable shadowing

Thanks, James! I think I have only one more substantive comment:

+  (Field->getAccess() == AS_public || Field->getAccess() == 
AS_protected)) {

Have you considered also taking into account the access of the inheritance 
path? Eg, a public member of a private base class of a public base class is 
typically inaccessible, even though it was declared public:

  struct A { int n; };
  struct B : private A {};
  struct C : B { int n; }; // A::n is not accessible here, should we suppress 
the warning?

You can use CXXRecordDecl::MergeAccess to combine the access of the path with 
the access of the field and compute the effective access of the field in the 
derived class (and you should test to see if the resulting access is AS_None to 
tell if the field is inaccessible; fields with effective access of AS_Private 
-- such as public members of a private direct base class -- are accessible from 
the derived class). You'll need to set RecordPaths to true in the CXXBasePaths 
object in order for lookupInBases to compute the path access.

Oh, and you may as well use a range-based for loop here:

+auto Result = Base->lookup(FieldName);
+for (auto I = Result.begin(); I != Result.end(); ++I) {


On 2 February 2017 at 00:19, James Sun 
> wrote:
Hi Richard

Thanks for the feedback! Hopefully addressed!

Thanks

James



From: > on behalf of Richard Smith 
>
Date: Wednesday, February 1, 2017 at 3:50 PM
To: James Sun >

Cc: Saleem Abdulrasool >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>
Subject: Re: Add warning for c++ member variable shadowing

+  std::set bases;
+const auto baseName = 
Specifier->getType()->getAsCXXRecordDecl()->getName();

Please capitalize local variable names. Also, please don't use the record name 
as a key in your set; that's not guaranteed to be unique. Instead, you could 
either use a set of canonical types or of canonical CXXRecordDecl*s.

+for (const auto *Field : 
Specifier->getType()->getAsCXXRecordDecl()->fields()) {
+  if ((Field->getAccess() == AS_public || Field->getAccess() == 
AS_protected) &&
+  Field->getName() == FieldName) {

Use Specifier->getType()->getAsCXXRecordDecl()->lookup(Field->getName()) here 
to look up the field by name, rather than walking all the fields of all base 
classes and checking if each of them has the right name. You should also check 
for IndirectFieldDecls, for this case:

  struct A {
union { int x; float f; };
  };
  struct B : A {
int x;
  };

+bases.emplace(baseName);

It's more efficient to use insert rather than emplace when inserting an element 
into a set.

+Diag(Loc, diag::warn_shadow_field)
+  << FieldName << RD->getName() << baseName;

It'd be nice to add a note here pointing at the base class member that was 
shadowed.



On 31 January 2017 at 19:20, James Sun 
> wrote:
Fixed!

From: Saleem Abdulrasool >
Date: Tuesday, January 31, 2017 at 6:53 PM

To: James Sun >
Cc: Richard 

Re: [clang-tools-extra] r291892 - Fix the build of the include-fixer plugin for some shared cmake builds and MSVC.

[Eric Fiselier via cfe-commits]

Hi Ben,

This change doesn't work when doing out-of-tree builds since `PTHREAD_LIB`
is defined by LLVM's `config-ix.cmake` and
therefore isn't defined during out-of-tree builds. It looks like the
pthread configuration needs to be lifted out of LLVM's
`config-ix.cmake` and placed somewhere the installed `AddLLVM.cmake` can
see it.

I don't think it's critical to fix this for the 4.0 release, but it should
get fixed.

@Ben do you see a better way to fix this?

On Mon, Jan 23, 2017 at 3:17 PM, Hans Wennborg via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> I've merged this to the release branch in r292834.
>
> Thanks,
> Hans
>
> On Fri, Jan 13, 2017 at 2:14 AM, Benjamin Kramer via cfe-commits
>  wrote:
> > Author: d0k
> > Date: Fri Jan 13 04:14:07 2017
> > New Revision: 291892
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=291892=rev
> > Log:
> > Fix the build of the include-fixer plugin for some shared cmake builds
> and MSVC.
> >
> > - The include fixer plugin does not directly depend on pthread, but can
> > pick up pthread references transitively through inlining. Just add
> > pthreads to the linked libs unconditionally.
> > - MSVC emits bogus warnings when including  and building without
> > exceptions. Blacklist the warnings explicitly.
> >
> > Modified:
> > clang-tools-extra/trunk/include-fixer/SymbolIndexManager.h
> > clang-tools-extra/trunk/include-fixer/plugin/CMakeLists.txt
> >
> > Modified: clang-tools-extra/trunk/include-fixer/SymbolIndexManager.h
> > URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/include-fixer/SymbolIndexManager.h?rev=291892=291891=291892&
> view=diff
> > 
> ==
> > --- clang-tools-extra/trunk/include-fixer/SymbolIndexManager.h
> (original)
> > +++ clang-tools-extra/trunk/include-fixer/SymbolIndexManager.h Fri Jan
> 13 04:14:07 2017
> > @@ -13,8 +13,19 @@
> >  #include "SymbolIndex.h"
> >  #include "find-all-symbols/SymbolInfo.h"
> >  #include "llvm/ADT/StringRef.h"
> > +
> > +#ifdef _MSC_VER
> > +// Disable warnings from ppltasks.h transitively included by .
> > +#pragma warning(push)
> > +#pragma warning(disable:4530)
> > +#endif
> > +
> >  #include 
> >
> > +#ifdef _MSC_VER
> > +#pragma warning(pop)
> > +#endif
> > +
> >  namespace clang {
> >  namespace include_fixer {
> >
> >
> > Modified: clang-tools-extra/trunk/include-fixer/plugin/CMakeLists.txt
> > URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/include-fixer/plugin/CMakeLists.txt?rev=291892=
> 291891=291892=diff
> > 
> ==
> > --- clang-tools-extra/trunk/include-fixer/plugin/CMakeLists.txt
> (original)
> > +++ clang-tools-extra/trunk/include-fixer/plugin/CMakeLists.txt Fri Jan
> 13 04:14:07 2017
> > @@ -9,4 +9,5 @@ add_clang_library(clangIncludeFixerPlugi
> >clangParse
> >clangSema
> >clangTooling
> > +  ${PTHREAD_LIB}
> >)
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: Add warning for c++ member variable shadowing

[James Sun via cfe-commits]

Don't we have a map to prevent this? I think the unit test does have cases to 
cover ambiguous paths to the same base. --J

Sent from my iPhone

On Feb 4, 2017, at 6:22 PM, Richard Smith 
> wrote:

Hmm, now we're emitting one warning per path, it looks like we might diagnose 
shadowing the same field more than once (for a repeated non-virtual base 
class). Is that intentional? Maybe we should just skip producing the warning if 
the lookup would be ambiguous, since any use of the shadowed field would 
otherwise be ill-formed.

On 4 February 2017 at 17:48, James Sun 
> wrote:
Thanks Richard! Good catch! The updated version is attached. --James

From: > on behalf of Richard Smith 
>
Date: Thursday, February 2, 2017 at 11:59 AM
To: James Sun >
Cc: Saleem Abdulrasool >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>
Subject: Re: Add warning for c++ member variable shadowing

Thanks, James! I think I have only one more substantive comment:

+  (Field->getAccess() == AS_public || Field->getAccess() == 
AS_protected)) {

Have you considered also taking into account the access of the inheritance 
path? Eg, a public member of a private base class of a public base class is 
typically inaccessible, even though it was declared public:

  struct A { int n; };
  struct B : private A {};
  struct C : B { int n; }; // A::n is not accessible here, should we suppress 
the warning?

You can use CXXRecordDecl::MergeAccess to combine the access of the path with 
the access of the field and compute the effective access of the field in the 
derived class (and you should test to see if the resulting access is AS_None to 
tell if the field is inaccessible; fields with effective access of AS_Private 
-- such as public members of a private direct base class -- are accessible from 
the derived class). You'll need to set RecordPaths to true in the CXXBasePaths 
object in order for lookupInBases to compute the path access.

Oh, and you may as well use a range-based for loop here:

+auto Result = Base->lookup(FieldName);
+for (auto I = Result.begin(); I != Result.end(); ++I) {


On 2 February 2017 at 00:19, James Sun 
> wrote:
Hi Richard

Thanks for the feedback! Hopefully addressed!

Thanks

James



From: > on behalf of Richard Smith 
>
Date: Wednesday, February 1, 2017 at 3:50 PM
To: James Sun >

Cc: Saleem Abdulrasool >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>
Subject: Re: Add warning for c++ member variable shadowing

+  std::set bases;
+const auto baseName = 
Specifier->getType()->getAsCXXRecordDecl()->getName();

Please capitalize local variable names. Also, please don't use the record name 
as a key in your set; that's not guaranteed to be unique. Instead, you could 
either use a set of canonical types or of canonical CXXRecordDecl*s.

+for (const auto *Field : 
Specifier->getType()->getAsCXXRecordDecl()->fields()) {
+  if ((Field->getAccess() == AS_public || Field->getAccess() == 
AS_protected) &&
+  Field->getName() == FieldName) {

Use Specifier->getType()->getAsCXXRecordDecl()->lookup(Field->getName()) here 
to look up the field by name, rather than walking all the fields of all base 
classes and checking if each of them has the right name. You should also check 
for IndirectFieldDecls, for this case:

  struct A {
union { int x; float f; };
  };
  struct B : A {
int x;
  };

+bases.emplace(baseName);

It's more efficient to use insert rather than emplace when inserting an element 
into a set.

+Diag(Loc, diag::warn_shadow_field)
+  << FieldName << RD->getName() << baseName;

It'd be nice to add a note here pointing at the base class member that was 
shadowed.



On 31 January 2017 at 19:20, James Sun 
> wrote:
Fixed!

From: Saleem Abdulrasool >
Date: Tuesday, January 31, 2017 at 6:53 PM

To: James Sun >
Cc: Richard Smith >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 

Re: Add warning for c++ member variable shadowing

[Richard Smith via cfe-commits]

Hmm, now we're emitting one warning per path, it looks like we might
diagnose shadowing the same field more than once (for a repeated
non-virtual base class). Is that intentional? Maybe we should just skip
producing the warning if the lookup would be ambiguous, since any use of
the shadowed field would otherwise be ill-formed.

On 4 February 2017 at 17:48, James Sun  wrote:

> Thanks Richard! Good catch! The updated version is attached. --James
>
>
>
> *From: * on behalf of Richard Smith <
> rich...@metafoo.co.uk>
> *Date: *Thursday, February 2, 2017 at 11:59 AM
> *To: *James Sun 
> *Cc: *Saleem Abdulrasool , "
> cfe-commits@lists.llvm.org" , Aaron Ballman <
> aa...@aaronballman.com>
> *Subject: *Re: Add warning for c++ member variable shadowing
>
>
>
> Thanks, James! I think I have only one more substantive comment:
>
>
>
> +  (Field->getAccess() == AS_public || Field->getAccess() ==
> AS_protected)) {
>
>
>
> Have you considered also taking into account the access of the inheritance
> path? Eg, a public member of a private base class of a public base class is
> typically inaccessible, even though it was declared public:
>
>
>
>   struct A { int n; };
>
>   struct B : private A {};
>
>   struct C : B { int n; }; // A::n is not accessible here, should we
> suppress the warning?
>
>
>
> You can use CXXRecordDecl::MergeAccess to combine the access of the path
> with the access of the field and compute the effective access of the field
> in the derived class (and you should test to see if the resulting access is
> AS_None to tell if the field is inaccessible; fields with effective access
> of AS_Private -- such as public members of a private direct base class --
> are accessible from the derived class). You'll need to set RecordPaths to
> true in the CXXBasePaths object in order for lookupInBases to compute the
> path access.
>
>
>
> Oh, and you may as well use a range-based for loop here:
>
>
>
> +auto Result = Base->lookup(FieldName);
>
> +for (auto I = Result.begin(); I != Result.end(); ++I) {
>
>
>
>
>
> On 2 February 2017 at 00:19, James Sun  wrote:
>
> Hi Richard
>
>
>
> Thanks for the feedback! Hopefully addressed!
>
>
>
> Thanks
>
>
>
> James
>
>
>
>
>
>
>
> *From: * on behalf of Richard Smith <
> rich...@metafoo.co.uk>
> *Date: *Wednesday, February 1, 2017 at 3:50 PM
> *To: *James Sun 
>
>
> *Cc: *Saleem Abdulrasool , "
> cfe-commits@lists.llvm.org" , Aaron Ballman <
> aa...@aaronballman.com>
> *Subject: *Re: Add warning for c++ member variable shadowing
>
>
>
> +  std::set bases;
>
> +const auto baseName = Specifier->getType()->
> getAsCXXRecordDecl()->getName();
>
>
>
> Please capitalize local variable names. Also, please don't use the record
> name as a key in your set; that's not guaranteed to be unique. Instead, you
> could either use a set of canonical types or of canonical CXXRecordDecl*s.
>
>
>
> +for (const auto *Field : Specifier->getType()->
> getAsCXXRecordDecl()->fields()) {
>
> +  if ((Field->getAccess() == AS_public || Field->getAccess() ==
> AS_protected) &&
>
> +  Field->getName() == FieldName) {
>
>
>
> Use Specifier->getType()->getAsCXXRecordDecl()->lookup(Field->getName())
> here to look up the field by name, rather than walking all the fields of
> all base classes and checking if each of them has the right name. You
> should also check for IndirectFieldDecls, for this case:
>
>
>
>   struct A {
>
> union { int x; float f; };
>
>   };
>
>   struct B : A {
>
> int x;
>
>   };
>
>
>
> +bases.emplace(baseName);
>
>
>
> It's more efficient to use insert rather than emplace when inserting an
> element into a set.
>
>
>
> +Diag(Loc, diag::warn_shadow_field)
>
> +  << FieldName << RD->getName() << baseName;
>
>
>
> It'd be nice to add a note here pointing at the base class member that was
> shadowed.
>
>
>
>
>
>
>
> On 31 January 2017 at 19:20, James Sun  wrote:
>
> Fixed!
>
>
>
> *From: *Saleem Abdulrasool 
> *Date: *Tuesday, January 31, 2017 at 6:53 PM
>
>
> *To: *James Sun 
> *Cc: *Richard Smith , "cfe-commits@lists.llvm.org"
> , Aaron Ballman 
> *Subject: *Re: Add warning for c++ member variable shadowing
>
>
>
> Hmm, the braces in the if (bases.find(...)...) are not needed.
>
>
>
> Could you also add a test case for virtual inheritance?
>
>
>
> On Mon, Jan 30, 2017 at 8:34 PM, James Sun  wrote:
>
> Hi Saleem
>
>
>
> Thanks for the quick response. A test case is added. It covers some
> ordinary cases as well as corner cases like multiple paths to the same base.
>
>
>
> Thanks
>
>
>
> James
>
>
>
> *From: *Saleem Abdulrasool 
> *Date: *Monday, 

[PATCH] D24703: [clang-format] BreakBeforeBinaryOperations and AlignAfterOpenBracket conflict, bug 30304

[Daphne Pfister via Phabricator via cfe-commits]

daphnediane added a comment.

ping. I've been using this for a long time and still seems to apply to cleanly 
and works, though the line numbers have shifted a little since I generated the 
previous diff.


https://reviews.llvm.org/D24703



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

Re: Add warning for c++ member variable shadowing

[James Sun via cfe-commits]

Thanks Richard! Good catch! The updated version is attached. --James

From:  on behalf of Richard Smith 
Date: Thursday, February 2, 2017 at 11:59 AM
To: James Sun 
Cc: Saleem Abdulrasool , "cfe-commits@lists.llvm.org" 
, Aaron Ballman 
Subject: Re: Add warning for c++ member variable shadowing

Thanks, James! I think I have only one more substantive comment:

+  (Field->getAccess() == AS_public || Field->getAccess() == 
AS_protected)) {

Have you considered also taking into account the access of the inheritance 
path? Eg, a public member of a private base class of a public base class is 
typically inaccessible, even though it was declared public:

  struct A { int n; };
  struct B : private A {};
  struct C : B { int n; }; // A::n is not accessible here, should we suppress 
the warning?

You can use CXXRecordDecl::MergeAccess to combine the access of the path with 
the access of the field and compute the effective access of the field in the 
derived class (and you should test to see if the resulting access is AS_None to 
tell if the field is inaccessible; fields with effective access of AS_Private 
-- such as public members of a private direct base class -- are accessible from 
the derived class). You'll need to set RecordPaths to true in the CXXBasePaths 
object in order for lookupInBases to compute the path access.

Oh, and you may as well use a range-based for loop here:

+auto Result = Base->lookup(FieldName);
+for (auto I = Result.begin(); I != Result.end(); ++I) {


On 2 February 2017 at 00:19, James Sun 
> wrote:
Hi Richard

Thanks for the feedback! Hopefully addressed!

Thanks

James



From: > on behalf of Richard Smith 
>
Date: Wednesday, February 1, 2017 at 3:50 PM
To: James Sun >

Cc: Saleem Abdulrasool >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>
Subject: Re: Add warning for c++ member variable shadowing

+  std::set bases;
+const auto baseName = 
Specifier->getType()->getAsCXXRecordDecl()->getName();

Please capitalize local variable names. Also, please don't use the record name 
as a key in your set; that's not guaranteed to be unique. Instead, you could 
either use a set of canonical types or of canonical CXXRecordDecl*s.

+for (const auto *Field : 
Specifier->getType()->getAsCXXRecordDecl()->fields()) {
+  if ((Field->getAccess() == AS_public || Field->getAccess() == 
AS_protected) &&
+  Field->getName() == FieldName) {

Use Specifier->getType()->getAsCXXRecordDecl()->lookup(Field->getName()) here 
to look up the field by name, rather than walking all the fields of all base 
classes and checking if each of them has the right name. You should also check 
for IndirectFieldDecls, for this case:

  struct A {
union { int x; float f; };
  };
  struct B : A {
int x;
  };

+bases.emplace(baseName);

It's more efficient to use insert rather than emplace when inserting an element 
into a set.

+Diag(Loc, diag::warn_shadow_field)
+  << FieldName << RD->getName() << baseName;

It'd be nice to add a note here pointing at the base class member that was 
shadowed.



On 31 January 2017 at 19:20, James Sun 
> wrote:
Fixed!

From: Saleem Abdulrasool >
Date: Tuesday, January 31, 2017 at 6:53 PM

To: James Sun >
Cc: Richard Smith >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>
Subject: Re: Add warning for c++ member variable shadowing

Hmm, the braces in the if (bases.find(...)...) are not needed.

Could you also add a test case for virtual inheritance?

On Mon, Jan 30, 2017 at 8:34 PM, James Sun 
> wrote:
Hi Saleem

Thanks for the quick response. A test case is added. It covers some ordinary 
cases as well as corner cases like multiple paths to the same base.

Thanks

James

From: Saleem Abdulrasool >
Date: Monday, January 30, 2017 at 6:50 PM
To: James Sun >
Cc: Richard Smith >, 
"cfe-commits@lists.llvm.org" 
>, 

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

[Daphne Pfister via Phabricator via cfe-commits]

daphnediane added a comment.

In https://reviews.llvm.org/D21279#663670, @bmharper wrote:

> Thanks - the merge conflicts don't look too bad. I should have it cleaned up 
> by tomorrow.


Have you had a chance to do the merge yet? If not I have a merged version which 
passes the tests that I could post here if you want.


https://reviews.llvm.org/D21279



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

[libcxx] r294116 - Remove CMake hack

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Sat Feb  4 19:19:02 2017
New Revision: 294116

URL: http://llvm.org/viewvc/llvm-project?rev=294116=rev
Log:
Remove CMake hack

Modified:
libcxx/trunk/CMakeLists.txt

Modified: libcxx/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/CMakeLists.txt?rev=294116=294115=294116=diff
==
--- libcxx/trunk/CMakeLists.txt (original)
+++ libcxx/trunk/CMakeLists.txt Sat Feb  4 19:19:02 2017
@@ -107,15 +107,6 @@ set(LIBCXX_CXX_ABI "default" CACHE STRIN
 set(CXXABIS none default libcxxabi libcxxrt libstdc++ libsupc++ vcruntime)
 set_property(CACHE LIBCXX_CXX_ABI PROPERTY STRINGS ;${CXXABIS})
 
-# FIXME: This is a temporary hack to force LLVM buildbots to store
-# the fixed cache entry instead of the previous cache entry. This is needed
-# because some LLVM buildbots don't clear their cache. It will be removed
-# once all LLVM bots have been run.
-if (LIBCXX_CXX_ABI STREQUAL "")
-  set(LIBCXX_CXX_ABI "default" CACHE STRING
-  "Specify the C++ ABI library to use." FORCE)
-endif()
-
 # Setup the default options if LIBCXX_CXX_ABI is not specified.
 if (LIBCXX_CXX_ABI STREQUAL "default")
   find_path(


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

[libcxx] r294115 - Fix typo in docs

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Sat Feb  4 19:16:25 2017
New Revision: 294115

URL: http://llvm.org/viewvc/llvm-project?rev=294115=rev
Log:
Fix typo in docs

Modified:
libcxx/trunk/docs/DesignDocs/DebugMode.rst

Modified: libcxx/trunk/docs/DesignDocs/DebugMode.rst
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/DesignDocs/DebugMode.rst?rev=294115=294114=294115=diff
==
--- libcxx/trunk/docs/DesignDocs/DebugMode.rst (original)
+++ libcxx/trunk/docs/DesignDocs/DebugMode.rst Sat Feb  4 19:16:25 2017
@@ -3,7 +3,7 @@ Debug Mode
 ==
 
 .. contents::
-   :local
+   :local:
 
 .. _using-debug-mode:
 


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

[libcxx] r294107 - Recommit [libcxx] Never use within libc++

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Sat Feb  4 17:22:28 2017
New Revision: 294107

URL: http://llvm.org/viewvc/llvm-project?rev=294107=rev
Log:
Recommit [libcxx] Never use  within libc++

It is my opinion that libc++ should never use ``, including in the 
`dylib`.
This patch remove all uses of `assert` from within libc++ and replaces most of 
them with `_LIBCPP_ASSERT` instead.

Additionally this patch turn `LIBCXX_ENABLE_ASSERTIONS`  off by default,
because the standard library should not be aborting user programs unless 
explicitly asked to.

Modified:
libcxx/trunk/CMakeLists.txt
libcxx/trunk/include/__config
libcxx/trunk/include/__threading_support
libcxx/trunk/src/condition_variable.cpp
libcxx/trunk/src/experimental/filesystem/path.cpp
libcxx/trunk/src/mutex.cpp
libcxx/trunk/src/system_error.cpp

Modified: libcxx/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/CMakeLists.txt?rev=294107=294106=294107=diff
==
--- libcxx/trunk/CMakeLists.txt (original)
+++ libcxx/trunk/CMakeLists.txt Sat Feb  4 17:22:28 2017
@@ -60,7 +60,7 @@ endif()
 include(CMakeDependentOption)
 
 # Basic options ---
-option(LIBCXX_ENABLE_ASSERTIONS "Enable assertions independent of build mode." 
ON)
+option(LIBCXX_ENABLE_ASSERTIONS "Enable assertions independent of build mode." 
OFF)
 option(LIBCXX_ENABLE_SHARED "Build libc++ as a shared library." ON)
 option(LIBCXX_ENABLE_STATIC "Build libc++ as a static library." ON)
 option(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY "Build libc++experimental.a" ON)
@@ -501,6 +501,7 @@ endif()
 # Assertion flags =
 define_if(LIBCXX_ENABLE_ASSERTIONS -UNDEBUG)
 define_if_not(LIBCXX_ENABLE_ASSERTIONS -DNDEBUG)
+define_if(LIBCXX_ENABLE_ASSERTIONS -D_LIBCPP_DEBUG=0)
 define_if(LIBCXX_DEBUG_BUILD -D_DEBUG)
 if (LIBCXX_ENABLE_ASSERTIONS AND NOT LIBCXX_DEBUG_BUILD)
   # MSVC doesn't like _DEBUG on release builds. See PR 4379.

Modified: libcxx/trunk/include/__config
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=294107=294106=294107=diff
==
--- libcxx/trunk/include/__config (original)
+++ libcxx/trunk/include/__config Sat Feb  4 17:22:28 2017
@@ -817,7 +817,9 @@ template  struct __static_asse
 #   else
 #   error Supported values for _LIBCPP_DEBUG are 0 and 1
 #   endif
+# if !defined(_LIBCPP_BUILDING_LIBRARY)
 #   define _LIBCPP_EXTERN_TEMPLATE(...)
+# endif
 #endif
 
 #ifndef _LIBCPP_EXTERN_TEMPLATE

Modified: libcxx/trunk/include/__threading_support
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__threading_support?rev=294107=294106=294107=diff
==
--- libcxx/trunk/include/__threading_support (original)
+++ libcxx/trunk/include/__threading_support Sat Feb  4 17:22:28 2017
@@ -25,7 +25,6 @@
 # include 
 # include 
 #elif defined(_LIBCPP_HAS_THREAD_API_WIN32)
-#include 
 #include 
 #include 
 #include 

Modified: libcxx/trunk/src/condition_variable.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/condition_variable.cpp?rev=294107=294106=294107=diff
==
--- libcxx/trunk/src/condition_variable.cpp (original)
+++ libcxx/trunk/src/condition_variable.cpp Sat Feb  4 17:22:28 2017
@@ -14,7 +14,6 @@
 #include "condition_variable"
 #include "thread"
 #include "system_error"
-#include "cassert"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 

Modified: libcxx/trunk/src/experimental/filesystem/path.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/experimental/filesystem/path.cpp?rev=294107=294106=294107=diff
==
--- libcxx/trunk/src/experimental/filesystem/path.cpp (original)
+++ libcxx/trunk/src/experimental/filesystem/path.cpp Sat Feb  4 17:22:28 2017
@@ -6,11 +6,9 @@
 // Source Licenses. See LICENSE.TXT for details.
 //
 
//===--===//
-#undef NDEBUG
 #include "experimental/filesystem"
 #include "string_view"
 #include "utility"
-#include "cassert"
 
 namespace { namespace parser
 {
@@ -113,7 +111,6 @@ public:
   void decrement() noexcept {
 const PosPtr REnd = () - 1;
 const PosPtr RStart = getCurrentTokenStartPos() - 1;
-assert(RStart != REnd);
 
 switch (State) {
 case PS_AtEnd: {
@@ -322,7 +319,6 @@ string_view_t path::__root_path_raw() co
   auto NextCh = PP.peek();
   if (NextCh && *NextCh == '/') {
 ++PP;
-assert(PP.State == PathParser::PS_InRootDir);
 return createView(__pn_.data(), ());
   }
   return PP.RawEntry;

Modified: libcxx/trunk/src/mutex.cpp
URL: 

[libcxx] r294106 - Fix inconsistency in tuple's SFINAE. Patch from Andrey Khalyavin"

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Sat Feb  4 16:57:01 2017
New Revision: 294106

URL: http://llvm.org/viewvc/llvm-project?rev=294106=rev
Log:
Fix inconsistency in tuple's SFINAE. Patch from Andrey Khalyavin"

Modified:
libcxx/trunk/include/tuple

Modified: libcxx/trunk/include/tuple
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/tuple?rev=294106=294105=294106=diff
==
--- libcxx/trunk/include/tuple (original)
+++ libcxx/trunk/include/tuple Sat Feb  4 16:57:01 2017
@@ -751,7 +751,7 @@ public:
  _CheckArgsConstructor<
 !_EnableImplicitReducedArityExtension
 && sizeof...(_Up) < sizeof...(_Tp)
-&& !_PackExpandsToThisTuple<_Up...>()
+&& !_PackExpandsToThisTuple<_Up...>::value
  >::template __enable_implicit<_Up...>(),
  bool
   >::type = false


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

[PATCH] D29135: [libcxx] [test] Fix Clang -Wunused-local-typedef, part 1/3.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

Since all of these changes are for libc++ specific tests I would much rather 
see changes of this form:

  #if defined(_LIBCPP_VERSION)
  {
typedef foo bar;
static_assert(baz, "");
  }
  #endif

I think that's a lot nicer than wrapping every statement in the block in a 
`LIBCPP_ONLY`.
If you agree feel free to commit those changes without review.


https://reviews.llvm.org/D29135



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

Re: [libcxx] r294099 - Undefine min/max in __tree

[Eric Fiselier via cfe-commits]

Merged in r294103.

On Sat, Feb 4, 2017 at 1:39 PM, Eric Fiselier  wrote:

> I'm going to merge this into v4.0 in order to fix
> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=216772
>
> /Eric
>
> On Sat, Feb 4, 2017 at 1:27 PM, Eric Fiselier via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: ericwf
>> Date: Sat Feb  4 14:27:46 2017
>> New Revision: 294099
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=294099=rev
>> Log:
>> Undefine min/max in __tree
>>
>> Added:
>> libcxx/trunk/test/libcxx/containers/associative/undef_min_
>> max.pass.cpp
>> Modified:
>> libcxx/trunk/include/__tree
>>
>> Modified: libcxx/trunk/include/__tree
>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__
>> tree?rev=294099=294098=294099=diff
>> 
>> ==
>> --- libcxx/trunk/include/__tree (original)
>> +++ libcxx/trunk/include/__tree Sat Feb  4 14:27:46 2017
>> @@ -17,6 +17,8 @@
>>  #include 
>>  #include 
>>
>> +#include <__undef_min_max>
>> +
>>  #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
>>  #pragma GCC system_header
>>  #endif
>>
>> Added: libcxx/trunk/test/libcxx/containers/associative/undef_min_
>> max.pass.cpp
>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx
>> /containers/associative/undef_min_max.pass.cpp?rev=294099=auto
>> 
>> ==
>> --- libcxx/trunk/test/libcxx/containers/associative/undef_min_max.pass.cpp
>> (added)
>> +++ libcxx/trunk/test/libcxx/containers/associative/undef_min_max.pass.cpp
>> Sat Feb  4 14:27:46 2017
>> @@ -0,0 +1,22 @@
>> +//===--
>> ===//
>> +//
>> +// The LLVM Compiler Infrastructure
>> +//
>> +// This file is dual licensed under the MIT and the University of
>> Illinois Open
>> +// Source Licenses. See LICENSE.TXT for details.
>> +//
>> +//===--
>> ===//
>> +
>> +#if defined(__GNUC__)
>> +#pragma GCC diagnostic ignored "-W#warnings"
>> +#endif
>> +
>> +#define min THIS IS A NASTY MACRO!
>> +#define max THIS IS A NASTY MACRO!
>> +
>> +#include 
>> +
>> +int main() {
>> +  std::map m;
>> +  ((void)m);
>> +}
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r294103 - Merge r294099 - Undefine min/max in __tree

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Sat Feb  4 16:36:27 2017
New Revision: 294103

URL: http://llvm.org/viewvc/llvm-project?rev=294103=rev
Log:
Merge r294099 - Undefine min/max in __tree

Added:

libcxx/branches/release_40/test/libcxx/containers/associative/undef_min_max.pass.cpp
Modified:
libcxx/branches/release_40/include/__tree

Modified: libcxx/branches/release_40/include/__tree
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/branches/release_40/include/__tree?rev=294103=294102=294103=diff
==
--- libcxx/branches/release_40/include/__tree (original)
+++ libcxx/branches/release_40/include/__tree Sat Feb  4 16:36:27 2017
@@ -17,6 +17,8 @@
 #include 
 #include 
 
+#include <__undef_min_max>
+
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif

Added: 
libcxx/branches/release_40/test/libcxx/containers/associative/undef_min_max.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/branches/release_40/test/libcxx/containers/associative/undef_min_max.pass.cpp?rev=294103=auto
==
--- 
libcxx/branches/release_40/test/libcxx/containers/associative/undef_min_max.pass.cpp
 (added)
+++ 
libcxx/branches/release_40/test/libcxx/containers/associative/undef_min_max.pass.cpp
 Sat Feb  4 16:36:27 2017
@@ -0,0 +1,22 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#if defined(__GNUC__)
+#pragma GCC diagnostic ignored "-W#warnings"
+#endif
+
+#define min THIS IS A NASTY MACRO!
+#define max THIS IS A NASTY MACRO!
+
+#include 
+
+int main() {
+  std::map m;
+  ((void)m);
+}


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

[PATCH] D29550: Diagnose when "future_error(error_code)" constructor is called by user code

[Eric Fiselier via Phabricator via cfe-commits]

EricWF created this revision.

The constructor `future_error(error_code)` isn't part of the C++ standard, but 
libc++ provides it in order to construct `future_error`'s before C++17.
However since it's non-standard we probably shouldn't be providing it all. We 
could make the constructor private but I suspect users already depend
on this extension. Instead this patch does the following:

1. Make it explicit so it doesn't cause weird implicit conversion bugs.
2. Generate a warning when the constructor is called from user code.
3. Cleanup tests that depend on the constructor.

@mclow.lists thoughts on handling this?


https://reviews.llvm.org/D29550

Files:
  docs/UsingLibcxx.rst
  include/__config
  include/future
  
test/libcxx/thread/futures/futures.future_error/diagnose_extended_constructor.fail.cpp
  test/libcxx/thread/futures/futures.future_error/extended_constructor.pass.cpp
  test/std/thread/futures/futures.future_error/code.pass.cpp
  test/std/thread/futures/futures.future_error/what.pass.cpp

Index: test/std/thread/futures/futures.future_error/what.pass.cpp
===
--- test/std/thread/futures/futures.future_error/what.pass.cpp
+++ test/std/thread/futures/futures.future_error/what.pass.cpp
@@ -28,24 +28,38 @@
 
 #include "test_macros.h"
 
+
+#if defined(__clang__)
+#pragma clang diagnostic ignored "-Wuser-defined-warnings"
+#endif
+
+template  // make dependent to prevent compile errors
+std::future_error make_error(ErrC ec) {
+#if TEST_STD_VER > 14
+  return std::future_error(ec);
+#else
+  return std::future_error(std::make_error_code(ec)); // libc++ extension
+#endif
+}
+
 int main()
 {
 {
-std::future_error f(std::make_error_code(std::future_errc::broken_promise));
+std::future_error f = make_error(std::future_errc::broken_promise);
 LIBCPP_ASSERT(std::strcmp(f.what(), "The associated promise has been destructed prior "
   "to the associated state becoming ready.") == 0);
 }
 {
-std::future_error f(std::make_error_code(std::future_errc::future_already_retrieved));
+std::future_error f = make_error(std::future_errc::future_already_retrieved);
 LIBCPP_ASSERT(std::strcmp(f.what(), "The future has already been retrieved from "
   "the promise or packaged_task.") == 0);
 }
 {
-std::future_error f(std::make_error_code(std::future_errc::promise_already_satisfied));
+std::future_error f = make_error(std::future_errc::promise_already_satisfied);
 LIBCPP_ASSERT(std::strcmp(f.what(), "The state of the promise has already been set.") == 0);
 }
 {
-std::future_error f(std::make_error_code(std::future_errc::no_state));
+std::future_error f = make_error(std::future_errc::no_state);
 LIBCPP_ASSERT(std::strcmp(f.what(), "Operation not permitted on an object without "
   "an associated state.") == 0);
 }
Index: test/std/thread/futures/futures.future_error/code.pass.cpp
===
--- test/std/thread/futures/futures.future_error/code.pass.cpp
+++ test/std/thread/futures/futures.future_error/code.pass.cpp
@@ -22,8 +22,13 @@
 
 #include "test_macros.h"
 
+#if defined(__clang__)
+#pragma clang diagnostic ignored "-Wuser-defined-warnings"
+#endif
+
 int main()
 {
+#if defined(_LIBCPP_VERSION)
 {
 std::error_code ec = std::make_error_code(std::future_errc::broken_promise);
 std::future_error f(ec);
@@ -44,6 +49,7 @@
 std::future_error f(ec);
 assert(f.code() == ec);
 }
+#endif // _LIBCPP_VERSION
 #if TEST_STD_VER > 14
 {
 std::future_error f(std::future_errc::broken_promise);
Index: test/libcxx/thread/futures/futures.future_error/extended_constructor.pass.cpp
===
--- /dev/null
+++ test/libcxx/thread/futures/futures.future_error/extended_constructor.pass.cpp
@@ -0,0 +1,24 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// UNSUPPORTED: libcpp-has-no-threads
+
+// 
+
+// class future_error
+
+// future_error(error_code) // libc++ extension
+
+#include 
+#include 
+
+int main() {
+  static_assert(std::is_constructible::value, "");
+  static_assert(!std::is_convertible::value, "");
+}
Index: test/libcxx/thread/futures/futures.future_error/diagnose_extended_constructor.fail.cpp
===
--- /dev/null
+++ 

[PATCH] D29545: Driver: Do not link safestack with --whole-archive.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.

This allows it to be used with the other sanitizers.


https://reviews.llvm.org/D29545

Files:
  clang/lib/Driver/Tools.cpp
  clang/test/Driver/sanitizer-ld.c


Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -416,7 +416,9 @@
 //
 // CHECK-SAFESTACK-LINUX: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-SAFESTACK-LINUX-NOT: "-lc"
+// CHECK-SAFESTACK-LINUX-NOT: whole-archive
 // CHECK-SAFESTACK-LINUX: libclang_rt.safestack-x86_64.a"
+// CHECK-SAFESTACK-LINUX: "-u" "__safestack_init"
 // CHECK-SAFESTACK-LINUX: "-lpthread"
 // CHECK-SAFESTACK-LINUX: "-ldl"
 
Index: clang/lib/Driver/Tools.cpp
===
--- clang/lib/Driver/Tools.cpp
+++ clang/lib/Driver/Tools.cpp
@@ -3365,8 +3365,10 @@
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("ubsan_standalone_cxx");
   }
-  if (SanArgs.needsSafeStackRt())
-StaticRuntimes.push_back("safestack");
+  if (SanArgs.needsSafeStackRt()) {
+NonWholeStaticRuntimes.push_back("safestack");
+RequiredSymbols.push_back("__safestack_init");
+  }
   if (SanArgs.needsCfiRt())
 StaticRuntimes.push_back("cfi");
   if (SanArgs.needsCfiDiagRt()) {
@@ -3417,7 +3419,7 @@
   if (SanArgs.hasCrossDsoCfi() && !AddExportDynamic)
 CmdArgs.push_back("-export-dynamic-symbol=__cfi_check");
 
-  return !StaticRuntimes.empty();
+  return !StaticRuntimes.empty() || !NonWholeStaticRuntimes.empty();
 }
 
 static bool addXRayRuntime(const ToolChain , const ArgList ,


Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -416,7 +416,9 @@
 //
 // CHECK-SAFESTACK-LINUX: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-SAFESTACK-LINUX-NOT: "-lc"
+// CHECK-SAFESTACK-LINUX-NOT: whole-archive
 // CHECK-SAFESTACK-LINUX: libclang_rt.safestack-x86_64.a"
+// CHECK-SAFESTACK-LINUX: "-u" "__safestack_init"
 // CHECK-SAFESTACK-LINUX: "-lpthread"
 // CHECK-SAFESTACK-LINUX: "-ldl"
 
Index: clang/lib/Driver/Tools.cpp
===
--- clang/lib/Driver/Tools.cpp
+++ clang/lib/Driver/Tools.cpp
@@ -3365,8 +3365,10 @@
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("ubsan_standalone_cxx");
   }
-  if (SanArgs.needsSafeStackRt())
-StaticRuntimes.push_back("safestack");
+  if (SanArgs.needsSafeStackRt()) {
+NonWholeStaticRuntimes.push_back("safestack");
+RequiredSymbols.push_back("__safestack_init");
+  }
   if (SanArgs.needsCfiRt())
 StaticRuntimes.push_back("cfi");
   if (SanArgs.needsCfiDiagRt()) {
@@ -3417,7 +3419,7 @@
   if (SanArgs.hasCrossDsoCfi() && !AddExportDynamic)
 CmdArgs.push_back("-export-dynamic-symbol=__cfi_check");
 
-  return !StaticRuntimes.empty();
+  return !StaticRuntimes.empty() || !NonWholeStaticRuntimes.empty();
 }
 
 static bool addXRayRuntime(const ToolChain , const ArgList ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28344: [AVR] Add support for the full set of inline asm constraints

[Anton Korobeynikov via Phabricator via cfe-commits]

asl added inline comments.



Comment at: test/CodeGen/avr-inline-asm-constraints.c:2
+// REQUIRES: avr-registered-target
+// RUN: %clang_cc1 -triple avr-unknown-unknown -emit-llvm -o - %s | FileCheck 
%s
+

You need checks for multi-character stuff and unsupported constraints.


https://reviews.llvm.org/D28344



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

[PATCH] D28346: [AVR] Allow specifying the CPU on the command line

[Anton Korobeynikov via Phabricator via cfe-commits]

asl added inline comments.



Comment at: test/CodeGen/avr/target-cpu-defines/atmega328p.c:1
+// RUN: %clang_cc1 -triple avr-unknown-unknown -target-cpu atmega328p 
-emit-llvm %s -o - | FileCheck %s
+

This looks wrong. How you're using FileCheck? Consider dumping pre-built 
defines and checking the output.



Comment at: test/CodeGen/avr/target-cpu-defines/attiny104.c:1
+// RUN: %clang_cc1 -triple avr-unknown-unknown -target-cpu attiny104 
-emit-llvm %s -o - | FileCheck %s
+

Same issue here and in all the tests.


https://reviews.llvm.org/D28346



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

[libcxx] r294100 - Mark basic_string::assign templates as inline to improve ABI stability.

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Sat Feb  4 14:38:35 2017
New Revision: 294100

URL: http://llvm.org/viewvc/llvm-project?rev=294100=rev
Log:
Mark basic_string::assign templates as inline to improve ABI stability.

Visible definitions for basic_string::assign are sometimes emitted in
the dylib depending on the version of LLVM used to compile libc++.
This can cause the check-cxx-abilist target to fail.

This patch attempts marks the basic_string::assign templates as inline
to prevent this. That way the export list is consistent across LLVM
versions.

Modified:
libcxx/trunk/include/string

Modified: libcxx/trunk/include/string
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=294100=294099=294100=diff
==
--- libcxx/trunk/include/string (original)
+++ libcxx/trunk/include/string Sat Feb  4 14:38:35 2017
@@ -998,7 +998,7 @@ public:
 basic_string& assign(const value_type* __s);
 basic_string& assign(size_type __n, value_type __c);
 template
-typename enable_if
+inline typename enable_if
 <
__is_exactly_input_iterator<_InputIterator>::value
 || 
!__libcpp_string_gets_noexcept_iterator<_InputIterator>::value,
@@ -1006,7 +1006,7 @@ public:
 >::type
 assign(_InputIterator __first, _InputIterator __last);
 template
-typename enable_if
+  inline typename enable_if
 <
 __is_forward_iterator<_ForwardIterator>::value
  && 
__libcpp_string_gets_noexcept_iterator<_ForwardIterator>::value,


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

[PATCH] D29542: [TargetInfo] Adjust x86-32 atomic support to the CPU used

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

In https://reviews.llvm.org/D29542#666831, @jlebar wrote:

> > Could someone help me figure out what is the cause and correct solution to 
> > that failure? @jlebar?
>
> You can see in NVPTXTargetInfo that we read properties from the host 
> targetinfo so that we export the same macros.  The problem here seems to be 
> that we're mutating the x86 targetinfo after the nvptx targetinfo reads its 
> properties.
>
> Does that give you enough context to fix the problem?


Thanks. I'll try to find a reasonably sane solution ;-).




Comment at: lib/Basic/Targets.cpp:4244
+} else // allow locked atomics up to 4 bytes
+  MaxAtomicPromoteWidth = 32;
+  }

dim wrote:
> Are you purposefully not setting `MaxAtomicInlineWidth` here?  (It seems from 
> `TargetInfo` that the default value is zero.)
> 
Yes. I've based this on what's done for ARM. Unless I misunderstood something, 
this means that on 'plain' i386 there is no inline atomics support but we still 
want to do atomics-via-locking up to 32-bit types. I'm not sure about 32/64 
here to match i486.



Comment at: lib/Basic/Targets.cpp:4265
 
-// x86-32 has atomics up to 8 bytes
-// FIXME: Check that we actually have cmpxchg8b before setting
-// MaxAtomicInlineWidth. (cmpxchg8b is an i586 instruction.)
-MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+setAtomic();
   }

dim wrote:
> As far as I can see, in the constructor this call is _always_ made with `CPU` 
> set to `CK_Generic`, i.e. zero.  Therefore, the "allow locked atomics up to 4 
> bytes" path in `setAtomic` is always chosen.  Maybe it is clearer to just 
> initialize `MaxAtomicPromoteWidth` to 32 directly here, instead?
> 
Well, I just copied the idea from ARM. I thought of it more like 'make sure it 
is initialized to some value, possibly update it later when setting CPU'. I'm 
fine either way.


Repository:
  rL LLVM

https://reviews.llvm.org/D29542



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

Re: [libcxx] r294099 - Undefine min/max in __tree

[Eric Fiselier via cfe-commits]

I'm going to merge this into v4.0 in order to fix
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=216772

/Eric

On Sat, Feb 4, 2017 at 1:27 PM, Eric Fiselier via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: ericwf
> Date: Sat Feb  4 14:27:46 2017
> New Revision: 294099
>
> URL: http://llvm.org/viewvc/llvm-project?rev=294099=rev
> Log:
> Undefine min/max in __tree
>
> Added:
> libcxx/trunk/test/libcxx/containers/associative/undef_min_max.pass.cpp
> Modified:
> libcxx/trunk/include/__tree
>
> Modified: libcxx/trunk/include/__tree
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/_
> _tree?rev=294099=294098=294099=diff
> 
> ==
> --- libcxx/trunk/include/__tree (original)
> +++ libcxx/trunk/include/__tree Sat Feb  4 14:27:46 2017
> @@ -17,6 +17,8 @@
>  #include 
>  #include 
>
> +#include <__undef_min_max>
> +
>  #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
>  #pragma GCC system_header
>  #endif
>
> Added: libcxx/trunk/test/libcxx/containers/associative/undef_
> min_max.pass.cpp
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/
> libcxx/containers/associative/undef_min_max.pass.cpp?rev=294099=auto
> 
> ==
> --- libcxx/trunk/test/libcxx/containers/associative/undef_min_max.pass.cpp
> (added)
> +++ libcxx/trunk/test/libcxx/containers/associative/undef_min_max.pass.cpp
> Sat Feb  4 14:27:46 2017
> @@ -0,0 +1,22 @@
> +//===--
> ===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is dual licensed under the MIT and the University of
> Illinois Open
> +// Source Licenses. See LICENSE.TXT for details.
> +//
> +//===--
> ===//
> +
> +#if defined(__GNUC__)
> +#pragma GCC diagnostic ignored "-W#warnings"
> +#endif
> +
> +#define min THIS IS A NASTY MACRO!
> +#define max THIS IS A NASTY MACRO!
> +
> +#include 
> +
> +int main() {
> +  std::map m;
> +  ((void)m);
> +}
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r294099 - Undefine min/max in __tree

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Sat Feb  4 14:27:46 2017
New Revision: 294099

URL: http://llvm.org/viewvc/llvm-project?rev=294099=rev
Log:
Undefine min/max in __tree

Added:
libcxx/trunk/test/libcxx/containers/associative/undef_min_max.pass.cpp
Modified:
libcxx/trunk/include/__tree

Modified: libcxx/trunk/include/__tree
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__tree?rev=294099=294098=294099=diff
==
--- libcxx/trunk/include/__tree (original)
+++ libcxx/trunk/include/__tree Sat Feb  4 14:27:46 2017
@@ -17,6 +17,8 @@
 #include 
 #include 
 
+#include <__undef_min_max>
+
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif

Added: libcxx/trunk/test/libcxx/containers/associative/undef_min_max.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/containers/associative/undef_min_max.pass.cpp?rev=294099=auto
==
--- libcxx/trunk/test/libcxx/containers/associative/undef_min_max.pass.cpp 
(added)
+++ libcxx/trunk/test/libcxx/containers/associative/undef_min_max.pass.cpp Sat 
Feb  4 14:27:46 2017
@@ -0,0 +1,22 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#if defined(__GNUC__)
+#pragma GCC diagnostic ignored "-W#warnings"
+#endif
+
+#define min THIS IS A NASTY MACRO!
+#define max THIS IS A NASTY MACRO!
+
+#include 
+
+int main() {
+  std::map m;
+  ((void)m);
+}


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

[PATCH] D29542: [TargetInfo] Adjust x86-32 atomic support to the CPU used

[Dimitry Andric via Phabricator via cfe-commits]

dim added inline comments.



Comment at: lib/Basic/Targets.cpp:4244
+} else // allow locked atomics up to 4 bytes
+  MaxAtomicPromoteWidth = 32;
+  }

Are you purposefully not setting `MaxAtomicInlineWidth` here?  (It seems from 
`TargetInfo` that the default value is zero.)




Comment at: lib/Basic/Targets.cpp:4265
 
-// x86-32 has atomics up to 8 bytes
-// FIXME: Check that we actually have cmpxchg8b before setting
-// MaxAtomicInlineWidth. (cmpxchg8b is an i586 instruction.)
-MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+setAtomic();
   }

As far as I can see, in the constructor this call is _always_ made with `CPU` 
set to `CK_Generic`, i.e. zero.  Therefore, the "allow locked atomics up to 4 
bytes" path in `setAtomic` is always chosen.  Maybe it is clearer to just 
initialize `MaxAtomicPromoteWidth` to 32 directly here, instead?



Repository:
  rL LLVM

https://reviews.llvm.org/D29542



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

[PATCH] D29542: [TargetInfo] Adjust x86-32 atomic support to the CPU used

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

> Could someone help me figure out what is the cause and correct solution to 
> that failure? @jlebar?

The test is checking that the macros have the same value when compiling for 
CUDA host and device.  That is, if we're compiling for an x86 CPU and an NVPTX 
GPU, we invoke cc1 twice, and the macros should have the same values both 
times.  Which, I know, is a lie.  But because when we're compiling for NVPTX we 
still parse all of the CPU code, macros generally need to have the same values 
otherwise we get into Big Trouble.  NVPTX atomics are controlled separately.

You can see in NVPTXTargetInfo that we read properties from the host targetinfo 
so that we export the same macros.  The problem here seems to be that we're 
mutating the x86 targetinfo after the nvptx targetinfo reads its properties.

Does that give you enough context to fix the problem?


Repository:
  rL LLVM

https://reviews.llvm.org/D29542



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

[PATCH] D29542: [TargetInfo] Adjust x86-32 atomic support to the CPU used

[Michał Górny via Phabricator via cfe-commits]

mgorny created this revision.
Herald added a subscriber: emaste.

Set the maximum width of atomic operations on x86-32 based on the target
CPU. The 64-bit inline atomics require cmpxchg8b which is an i586
instruction. Other inline atomics require cmpxchg which is an i486
instruction.

This fixes the incorrect value of __GCC_ATOMIC_LLONG_LOCK_FREE
and __atomic_always_lock_free() on FreeBSD where clang defaults to i486
CPU (PR#31864).

TODO: one CUDA test is broken:

  /home/mgorny/llvm/_build/./bin/clang  --cuda-host-only -nocudainc -target 
x86_64-windows-msvc -x cuda -E -dM -o - /dev/null| grep 'define __[^ 
]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define __GCC_ATOMIC'| grep -v 
'__LDBL\|_LONG_DOUBLE' > 
/home/mgorny/llvm/_build/tools/clang/test/Preprocessor/Output/x86_64-msvc-host-defines-filtered
  /home/mgorny/llvm/_build/./bin/clang  --cuda-device-only -nocudainc 
-nocudalib -target x86_64-windows-msvc -x cuda -E -dM -o - /dev/null| grep 
'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define __GCC_ATOMIC'| grep -v 
'__LDBL\|_LONG_DOUBLE' > 
/home/mgorny/llvm/_build/tools/clang/test/Preprocessor/Output/x86_64-msvc-device-defines-filtered
  diff 
/home/mgorny/llvm/_build/tools/clang/test/Preprocessor/Output/x86_64-msvc-host-defines-filtered
 
/home/mgorny/llvm/_build/tools/clang/test/Preprocessor/Output/x86_64-msvc-device-defines-filtered
  --
  Exit Code: 1
  
  Command Output (stdout):
  --
  9,17c9,17
  < #define __GCC_ATOMIC_BOOL_LOCK_FREE 2
  < #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 2
  < #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 2
  < #define __GCC_ATOMIC_CHAR_LOCK_FREE 2
  < #define __GCC_ATOMIC_INT_LOCK_FREE 2
  < #define __GCC_ATOMIC_LLONG_LOCK_FREE 2
  < #define __GCC_ATOMIC_LONG_LOCK_FREE 2
  < #define __GCC_ATOMIC_POINTER_LOCK_FREE 2
  < #define __GCC_ATOMIC_SHORT_LOCK_FREE 2
  ---
  > #define __GCC_ATOMIC_BOOL_LOCK_FREE 1
  > #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 1
  > #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 1
  > #define __GCC_ATOMIC_CHAR_LOCK_FREE 1
  > #define __GCC_ATOMIC_INT_LOCK_FREE 1
  > #define __GCC_ATOMIC_LLONG_LOCK_FREE 1
  > #define __GCC_ATOMIC_LONG_LOCK_FREE 1
  > #define __GCC_ATOMIC_POINTER_LOCK_FREE 1
  > #define __GCC_ATOMIC_SHORT_LOCK_FREE 1
  19c19
  < #define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 2
  ---
  > #define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 1
  
  --

Could someone help me figure out what is the cause and correct solution to that 
failure? @jlebar?


Repository:
  rL LLVM

https://reviews.llvm.org/D29542

Files:
  lib/Basic/Targets.cpp
  test/CodeGen/atomic-ops.c
  test/CodeGen/ms-volatile.c
  test/CodeGenCXX/atomicinit.cpp
  test/Sema/atomic-ops.c

Index: test/Sema/atomic-ops.c
===
--- test/Sema/atomic-ops.c
+++ test/Sema/atomic-ops.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -verify -ffreestanding -fsyntax-only -triple=i686-linux-gnu -std=c11
+// RUN: %clang_cc1 %s -verify -ffreestanding -fsyntax-only -triple=i686-linux-gnu -target-cpu i686 -std=c11
+// RUN: %clang_cc1 %s -verify -ffreestanding -fsyntax-only -triple=i486-linux-gnu -target-cpu i486 -std=c11
 
 // Basic parsing/Sema tests for __c11_atomic_*
 
@@ -14,22 +15,34 @@
 _Static_assert(__GCC_ATOMIC_SHORT_LOCK_FREE == 2, "");
 _Static_assert(__GCC_ATOMIC_INT_LOCK_FREE == 2, "");
 _Static_assert(__GCC_ATOMIC_LONG_LOCK_FREE == 2, "");
+#if defined(__i486__)
+_Static_assert(__GCC_ATOMIC_LLONG_LOCK_FREE == 1, "");
+#else
 _Static_assert(__GCC_ATOMIC_LLONG_LOCK_FREE == 2, "");
+#endif
 _Static_assert(__GCC_ATOMIC_POINTER_LOCK_FREE == 2, "");
 
 _Static_assert(__c11_atomic_is_lock_free(1), "");
 _Static_assert(__c11_atomic_is_lock_free(2), "");
 _Static_assert(__c11_atomic_is_lock_free(3), ""); // expected-error {{not an integral constant expression}}
 _Static_assert(__c11_atomic_is_lock_free(4), "");
+#if defined(__i486__)
+_Static_assert(__c11_atomic_is_lock_free(8), ""); // expected-error {{not an integral constant expression}}
+#else
 _Static_assert(__c11_atomic_is_lock_free(8), "");
+#endif
 _Static_assert(__c11_atomic_is_lock_free(16), ""); // expected-error {{not an integral constant expression}}
 _Static_assert(__c11_atomic_is_lock_free(17), ""); // expected-error {{not an integral constant expression}}
 
 _Static_assert(__atomic_is_lock_free(1, 0), "");
 _Static_assert(__atomic_is_lock_free(2, 0), "");
 _Static_assert(__atomic_is_lock_free(3, 0), ""); // expected-error {{not an integral constant expression}}
 _Static_assert(__atomic_is_lock_free(4, 0), "");
+#if defined(__i486__)
+_Static_assert(__atomic_is_lock_free(8, 0), ""); // expected-error {{not an integral constant expression}}
+#else
 _Static_assert(__atomic_is_lock_free(8, 0), "");
+#endif
 _Static_assert(__atomic_is_lock_free(16, 0), ""); // expected-error {{not an integral constant expression}}
 _Static_assert(__atomic_is_lock_free(17, 0), ""); // expected-error {{not an integral constant expression}}
 
@@ -56,13 +69,21 @@
 _Static_assert(__atomic_is_lock_free(4, ), "");
 

[PATCH] D28235: [clang-format cleanup] merge continuous deleted lines into one deletion.

[Paweł Żukowski via Phabricator via cfe-commits]

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

Few other tests (from `ChangeNamespaceTest`) require update too.


https://reviews.llvm.org/D28235



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

[PATCH] D28235: [clang-format cleanup] merge continuous deleted lines into one deletion.

[Paweł Żukowski via Phabricator via cfe-commits]

idlecode added inline comments.



Comment at: lib/Format/Format.cpp:1182
+ I != E; ++I)
+  LastToNextLineFirst[(*I)->Last] = (*std::next(I))->First;
 tooling::Replacements Fixes;

Could you add brackets around loop body?



Comment at: lib/Format/Format.cpp:1184
 tooling::Replacements Fixes;
 std::vector Tokens;
 std::copy(DeletedTokens.begin(), DeletedTokens.end(),

While you are here, could you get rid of `Tokens` vector too?
I believe iterating over `DeletedTokens`directly will do the job.


https://reviews.llvm.org/D28235



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

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added a comment.

In https://reviews.llvm.org/D29151#665948, @alexfh wrote:

> In https://reviews.llvm.org/D29151#662504, @zaks.anna wrote:
>
> > Before clang-tidy came into existence the guidelines were very clear. One 
> > should write a clang warning if the diagnostic:
> >
> > - can be implemented without path-insensitive analysis (including 
> > flow-sensitive)
> > - is checking for language rules (not specific API misuse)
> >
> >   In my view, this should still be the rule going forward because compiler 
> > warnings are most effective in reaching users.
> >
> >   The Clang Static Analyzer used to be the place for all other diagnostics. 
> > Most of the checkers it contains rely on path-sensitive analysis. Note that 
> > one might catch the same bug with flow-sensitive as well as path-sensitive 
> > analysis. So in some of those cases, we have both warnings as well as 
> > analyzer checkers finding the same type of user error. However, the 
> > checkers can catch more bugs since they are path-sensitive. The analyzer 
> > also has several flow-sensitive/ AST matching checkers. Those checks could 
> > not have been written as warnings mainly because they check for APIs, which 
> > are not part of the language.
> >
> >   My understanding is that clang-tidy supports fixits, which the clang 
> > static analyzer currently does not support. However, there could be other 
> > benefits to placing not path-sensitive checks there as well. I am not sure. 
> > I've also heard opinions that it's more of a linter tool, so the user 
> > expectations could be different.
> >
> > > Even right now there are clang-tidy checks that finds subset of alpha 
> > > checks, but probably having lower false positive rate.
> >
> > Again, alpha checks are unfinished work, so we should not use them as 
> > examples of checkers that have false positives. Some of them are research 
> > projects and should probably be deleted.
>
>
> I tried to come up with some advice on where checks should go in 
> http://clang.llvm.org/extra/clang-tidy/#choosing-the-right-place-for-your-check:
>
> | Clang diagnostic: if the check is generic enough, targets code patterns 
> that most probably are bugs (rather than style or readability issues), can be 
> implemented effectively and with extremely low false positive rate, it may 
> make a good Clang diagnostic. |
> | Clang static analyzer check: if the check requires some sort of control 
> flow analysis, it should probably be implemented as a static analyzer check.  
>   
>|
> | clang-tidy check is a good choice for linter-style checks, checks that are 
> related to a certain coding style, checks that address code readability, etc. 
>   
> |
>
> There's no doubt path-sensitive checks should go to Static Analyzer, since it 
> provides all the infrastructure for path-sensitive analyses. Whatever meets 
> the criteria for a Clang diagnostic should be a diagnostic. Whatever needs 
> automated fixes (and can be implemented on AST or preprocessor level) should 
> go to clang-tidy.
>
> But there's still a large set of analyses that don't clearly fall into one of 
> the categories above and can be implemented both in Clang Static Analyzer and 
> in clang-tidy. Currently there are no firm rules about those, only 
> recommendations on the clang-tidy page (quoted above). We might need to agree 
> upon some reasonable guidelines, though.
>
> For example, IMO, AST-based analyses make more sense in clang-tidy for a few 
> reasons:
>
> 1. They usually are easier expressible in terms of AST matchers, and 
> clang-tidy allows to use AST matchers with less boilerplate.
> 2. Clang Static Analyzer is linked into clang, where AST matchers are 
> undesired, since they tend to blow the size of binary significantly.
> 3. It's more consistent to keep all similar analyses together, it simplifies 
> search for already implemented similar functionality and code reviews.
>
>   Flow-sensitive analyses (that don't need any path-sensitivity) seem to be 
> equally suitable for SA and clang-tidy (unless I'm missing something), so I 
> don't feel strongly as to where they should go.
>
>   WDYT?


This sounds resonable. I think there are also other factors like:

- heuristics: clang-tidy tends to look for bugs based on some heuristics. E.g 
use-after-move consider methods as reinitializing based on method names (like 
clear).

This is approach works great if the heuristic is good enough to not have many 
false positives, but generally it will have some false positives and false 
negatives.

- flexibility: The other problem is that check can be flexible, for example to 
work on every vector-like container. I think static analyzer tends to be more 
conservative about bugs it wants to find. E.g we know how std::vector works, so 
the 

[PATCH] Improved plugin/tool support by expanding an existing attribute

[Marcwell Helpdesk via cfe-commits]

Many plugins/tools could benefit from having a generic way for communicating 
control directives directly from the source code to itself (via the AST) when 
acting, for example, as source code transformers, generators, collectors and 
the like. Attributes are a suitable way of doing this but most available 
attributes have predefined functionality and modifying the compiler for every 
plugin/tool is obviously not an option. There is however one undocumented but 
existing attribute that could be used for a generic solution if it was slightly 
modified and expanded - the annotate attribute.

The current functionality of the annotate attribute encompass annotating 
declarations with arbitrary strings using the GNU spelling. The attached patch 
expands on this functionality by adding annotation of statements, C++11 
spelling and documentation. With the patch applied most of the code could be 
annotated for the use by any Clang plugin or tool. For a more detailed 
description of the updated attribute, see patch for "AttrDocs.td".

An example demonstratiing the use and syntax of the updated attribute:

  class [[annotate("plain")]] Object
  { . . . };

  int main(int argc, char* argv[]) __attribute((annotate("entry-point")))
  {
[[annotate("local_var")]]   // Decl annotation
int n = 1 ;

// Multiple Stmt annotations
[[annotate("group-A"), annotate("opt-1:2")]]
switch(n)
{ . . . }
return 0;
  }

Cheers,
Chris

Modified:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/Sema/SemaStmtAttr.cpp
Added:
  test/SemaCXX/attr-annotate.cpp


attrib-annotate.patch
Description: Binary data
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin

[Dimitry Andric via Phabricator via cfe-commits]

dim added a comment.

Created https://llvm.org/bugs/show_bug.cgi?id=31864 to continue this on the bug 
tracker.


Repository:
  rL LLVM

https://reviews.llvm.org/D28213



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

[PATCH] D29267: [clang-tidy] safety-no-assembler

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe planned changes to this revision.
jbcoe added inline comments.



Comment at: clang-tools-extra/test/clang-tidy/safety-no-assembler.cpp:2
+// RUN: %check_clang_tidy %s safety-no-assembler %t
+
+void f() {

idlecode wrote:
> Maybe this check should handle `FileScopeAsmDecl` and `AsmLabelAttr` too?
> 
> ```
> __asm__(".symver foo, bar@v");
> static int s asm("spam");
> ```
Agreed.


https://reviews.llvm.org/D29267



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

[PATCH] D29267: [clang-tidy] safety-no-assembler

[Paweł Żukowski via Phabricator via cfe-commits]

idlecode added inline comments.



Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:156
+   safety-no-assembler
+   safety-no-vector-bool

`safety-no-vector-bool` seems to belong to the other patch.



Comment at: clang-tools-extra/test/clang-tidy/safety-no-assembler.cpp:2
+// RUN: %check_clang_tidy %s safety-no-assembler %t
+
+void f() {

Maybe this check should handle `FileScopeAsmDecl` and `AsmLabelAttr` too?

```
__asm__(".symver foo, bar@v");
static int s asm("spam");
```


https://reviews.llvm.org/D29267



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

[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32

[Anton Korobeynikov via Phabricator via cfe-commits]

asl requested changes to this revision.
asl added inline comments.
This revision now requires changes to proceed.



Comment at: lib/Driver/MinGWToolChain.cpp:211
 
+#ifndef LLVM_ON_WIN32
   if (GetRuntimeLibType(DriverArgs) == ToolChain::RLT_Libgcc) {

This check is certainly wrong because it checks for host, not for target.


https://reviews.llvm.org/D29464



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

[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

In https://reviews.llvm.org/D28213#666269, @dim wrote:

> > What's the value of `__atomic_always_lock_free(sizeof(long long), 0)` for 
> > gcc and clang?
>
> For gcc, it is always 0, for clang (I tested 3.4.1 through 4.0.0) it is 
> always 1.  Maybe that was always incorrect on 32-bit FreeBSD, then?


That's what I suspected. Yeah, looks like clang always made the wrong 
assumption and possibly someone just swept it under the carpet.

In https://reviews.llvm.org/D28213#666352, @dim wrote:

> Hmm, I just noticed the following rather disappointing comment in 
> `tools/clang/lib/Basic/Targets.cpp`:
>
>   X86_32TargetInfo(const llvm::Triple , const TargetOptions )
>   : X86TargetInfo(Triple, Opts) {
> [...]
> // x86-32 has atomics up to 8 bytes
> // FIXME: Check that we actually have cmpxchg8b before setting
> // MaxAtomicInlineWidth. (cmpxchg8b is an i586 instruction.)
> MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
>   
>
> So this can never have worked properly for e.g. i486 and i586...


Yep, looks like it. I see that the triple is passed there, so I guess making 
that conditional to `>=i586-*` should be relatively easy. However, I don't know 
how to make it properly respect `-march`.


Repository:
  rL LLVM

https://reviews.llvm.org/D28213



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

[PATCH] D29393: [clang-tidy] Don't warn about call to unresolved operator*

[Paweł Żukowski via Phabricator via cfe-commits]

idlecode added inline comments.



Comment at: clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp:63
+cxxOperatorCallExpr(argumentCountIs(1),
+callee(unresolvedLookupExpr()),
+hasArgument(0, cxxThisExpr(;

malcolm.parsons wrote:
> idlecode wrote:
> > Seems that it will catch all unary operators with ##this## as first 
> > argument.
> > e.g. in case `operator-` is defined somewhere, check will not warn about 
> > returning `-this`.
> > Do you think adding `hasOverloadedOperatorName("*")` for such abstract case 
> > is worth it?
> I'm just trying to suppress them warning from the template. The operator can 
> be checked properly in an instantiation.
LGTM then :)


https://reviews.llvm.org/D29393



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