Re: [PATCH] D24848: [clang-tidy] fix false-positive for cppcoreguidelines-pro-type-member-init with in-class initializers

2016-09-28 Thread Matthias Gehre via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL282625: [clang-tidy] fix false-positive for 
cppcoreguidelines-pro-type-member-init… (authored by mgehre).

Changed prior to commit:
  https://reviews.llvm.org/D24848?vs=72677&id=72885#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24848

Files:
  clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp
  
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp

Index: clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp
@@ -62,8 +62,10 @@
   if (ClassDecl->hasTrivialDefaultConstructor())
 return true;
 
-  // If all its fields are trivially constructible.
+  // If all its fields are trivially constructible and have no default 
initializers.
   for (const FieldDecl *Field : ClassDecl->fields()) {
+if (Field->hasInClassInitializer())
+  return false;
 if (!isTriviallyDefaultConstructible(Field->getType(), Context))
   return false;
   }
Index: 
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
===
--- 
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
+++ 
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
@@ -73,6 +73,11 @@
   NegativeInClassInitialized() {}
 };
 
+struct NegativeInClassInitializedDefaulted {
+  int F = 0;
+  NegativeInClassInitializedDefaulted() = default;
+};
+
 struct NegativeConstructorDelegated {
   int F;
 
@@ -367,3 +372,8 @@
   PositiveIndirectMember() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize 
these fields: A
 };
+
+void Bug30487()
+{
+  NegativeInClassInitializedDefaulted s;
+}


Index: clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp
@@ -62,8 +62,10 @@
   if (ClassDecl->hasTrivialDefaultConstructor())
 return true;
 
-  // If all its fields are trivially constructible.
+  // If all its fields are trivially constructible and have no default initializers.
   for (const FieldDecl *Field : ClassDecl->fields()) {
+if (Field->hasInClassInitializer())
+  return false;
 if (!isTriviallyDefaultConstructible(Field->getType(), Context))
   return false;
   }
Index: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
@@ -73,6 +73,11 @@
   NegativeInClassInitialized() {}
 };
 
+struct NegativeInClassInitializedDefaulted {
+  int F = 0;
+  NegativeInClassInitializedDefaulted() = default;
+};
+
 struct NegativeConstructorDelegated {
   int F;
 
@@ -367,3 +372,8 @@
   PositiveIndirectMember() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A
 };
+
+void Bug30487()
+{
+  NegativeInClassInitializedDefaulted s;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24848: [clang-tidy] fix false-positive for cppcoreguidelines-pro-type-member-init with in-class initializers

2016-09-28 Thread Malcolm Parsons via cfe-commits 
malcolm.parsons added a comment.

In https://reviews.llvm.org/D24848#555636, @mgehre wrote:

> Are you okay with me committing this as it currently is?


Yes.


https://reviews.llvm.org/D24848



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

Re: [PATCH] D24848: [clang-tidy] fix false-positive for cppcoreguidelines-pro-type-member-init with in-class initializers

2016-09-28 Thread Matthias Gehre via cfe-commits 
mgehre added a comment.

I would like to close that particular bug report, and thus I would like to have 
the reproducer of that bug as part of the test case.
The PositivePartiallyInClassInitialized is also a good test, but I fail to see 
how it is proves that that particular bug is solved.

Are you okay with me committing this as it currently is?


https://reviews.llvm.org/D24848



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

Re: [PATCH] D24848: [clang-tidy] fix false-positive for cppcoreguidelines-pro-type-member-init with in-class initializers

2016-09-27 Thread Malcolm Parsons via cfe-commits 
malcolm.parsons added a comment.

In https://reviews.llvm.org/D24848#554145, @mgehre wrote:

> Rename the struct that was introduced in the test. Note that I need to keep 
> the function Bug30487,
>  because that is where the false-positive warning was emitted.


https://reviews.llvm.org/D24965 will allow you to write a positive test instead:

  struct PositivePartiallyInClassInitialized {
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: constructor does not initialize 
these fields: G
int F = 0;
int G;
// CHECK-FIXES: int G{};
  };


https://reviews.llvm.org/D24848



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

Re: [PATCH] D24848: [clang-tidy] fix false-positive for cppcoreguidelines-pro-type-member-init with in-class initializers

2016-09-27 Thread Aaron Ballman via cfe-commits 
On Tue, Sep 27, 2016 at 2:05 PM, Matthias Gehre  wrote:
> mgehre updated this revision to Diff 72677.
> mgehre added a comment.
>
> Rename the struct that was introduced in the test. Note that I need to keep 
> the function Bug30487,
> because that is where the false-positive warning was emitted.

We usually use namespaces for this when working with C++ code, where
the namespace identifier is pr30487 (e.g.).

~Aaron

>
>
> https://reviews.llvm.org/D24848
>
> Files:
>   clang-tidy/utils/TypeTraits.cpp
>   test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
>
> Index: test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
> ===
> --- test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
> +++ test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
> @@ -73,6 +73,11 @@
>NegativeInClassInitialized() {}
>  };
>
> +struct NegativeInClassInitializedDefaulted {
> +  int F = 0;
> +  NegativeInClassInitializedDefaulted() = default;
> +};
> +
>  struct NegativeConstructorDelegated {
>int F;
>
> @@ -367,3 +372,8 @@
>PositiveIndirectMember() {}
>// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not 
> initialize these fields: A
>  };
> +
> +void Bug30487()
> +{
> +  NegativeInClassInitializedDefaulted s;
> +}
> Index: clang-tidy/utils/TypeTraits.cpp
> ===
> --- clang-tidy/utils/TypeTraits.cpp
> +++ clang-tidy/utils/TypeTraits.cpp
> @@ -62,8 +62,10 @@
>if (ClassDecl->hasTrivialDefaultConstructor())
>  return true;
>
> -  // If all its fields are trivially constructible.
> +  // If all its fields are trivially constructible and have no default 
> initializers.
>for (const FieldDecl *Field : ClassDecl->fields()) {
> +if (Field->hasInClassInitializer())
> +  return false;
>  if (!isTriviallyDefaultConstructible(Field->getType(), Context))
>return false;
>}
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24848: [clang-tidy] fix false-positive for cppcoreguidelines-pro-type-member-init with in-class initializers

2016-09-27 Thread Matthias Gehre via cfe-commits 
mgehre updated this revision to Diff 72677.
mgehre added a comment.

Rename the struct that was introduced in the test. Note that I need to keep the 
function Bug30487,
because that is where the false-positive warning was emitted.


https://reviews.llvm.org/D24848

Files:
  clang-tidy/utils/TypeTraits.cpp
  test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp

Index: test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
@@ -73,6 +73,11 @@
   NegativeInClassInitialized() {}
 };
 
+struct NegativeInClassInitializedDefaulted {
+  int F = 0;
+  NegativeInClassInitializedDefaulted() = default;
+};
+
 struct NegativeConstructorDelegated {
   int F;
 
@@ -367,3 +372,8 @@
   PositiveIndirectMember() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize 
these fields: A
 };
+
+void Bug30487()
+{
+  NegativeInClassInitializedDefaulted s;
+}
Index: clang-tidy/utils/TypeTraits.cpp
===
--- clang-tidy/utils/TypeTraits.cpp
+++ clang-tidy/utils/TypeTraits.cpp
@@ -62,8 +62,10 @@
   if (ClassDecl->hasTrivialDefaultConstructor())
 return true;
 
-  // If all its fields are trivially constructible.
+  // If all its fields are trivially constructible and have no default 
initializers.
   for (const FieldDecl *Field : ClassDecl->fields()) {
+if (Field->hasInClassInitializer())
+  return false;
 if (!isTriviallyDefaultConstructible(Field->getType(), Context))
   return false;
   }


Index: test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
@@ -73,6 +73,11 @@
   NegativeInClassInitialized() {}
 };
 
+struct NegativeInClassInitializedDefaulted {
+  int F = 0;
+  NegativeInClassInitializedDefaulted() = default;
+};
+
 struct NegativeConstructorDelegated {
   int F;
 
@@ -367,3 +372,8 @@
   PositiveIndirectMember() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A
 };
+
+void Bug30487()
+{
+  NegativeInClassInitializedDefaulted s;
+}
Index: clang-tidy/utils/TypeTraits.cpp
===
--- clang-tidy/utils/TypeTraits.cpp
+++ clang-tidy/utils/TypeTraits.cpp
@@ -62,8 +62,10 @@
   if (ClassDecl->hasTrivialDefaultConstructor())
 return true;
 
-  // If all its fields are trivially constructible.
+  // If all its fields are trivially constructible and have no default initializers.
   for (const FieldDecl *Field : ClassDecl->fields()) {
+if (Field->hasInClassInitializer())
+  return false;
 if (!isTriviallyDefaultConstructible(Field->getType(), Context))
   return false;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24848: [clang-tidy] fix false-positive for cppcoreguidelines-pro-type-member-init with in-class initializers

2016-09-26 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 with Malcom's test suggestions addressed.


https://reviews.llvm.org/D24848



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

Re: [PATCH] D24848: [clang-tidy] fix false-positive for cppcoreguidelines-pro-type-member-init with in-class initializers

2016-09-26 Thread Malcolm Parsons via cfe-commits 
malcolm.parsons added inline comments.


Comment at: test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp:372
@@ +371,3 @@
+
+struct Bug30487
+{

There's already this test:

```
struct NegativeInClassInitialized {
int F = 0;  
NegativeInClassInitialized() {}
};
```

I'd call your test NegativeInClassInitializedImplicit, and also add this:

```
struct NegativeInClassInitializedDefaulted {
int F = 0;
NegativeInClassInitializedDefaulted() = default;
};
```


https://reviews.llvm.org/D24848



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

[PATCH] D24848: [clang-tidy] fix false-positive for cppcoreguidelines-pro-type-member-init with in-class initializers

2016-09-22 Thread Matthias Gehre via cfe-commits 
mgehre created this revision.
mgehre added reviewers: alexfh, aaron.ballman.
mgehre added a subscriber: cfe-commits.
Herald added a subscriber: nemanjai.

This fixes https://llvm.org/bugs/show_bug.cgi?id=30487 where
```
warning: uninitialized record type: 's' [cppcoreguidelines-pro-type-member-init]
```
is emitted on
```
struct MyStruct
{
int a = 5;
int b = 7;
};

int main()
{
MyStruct s;
}
```

https://reviews.llvm.org/D24848

Files:
  clang-tidy/utils/TypeTraits.cpp
  test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp

Index: test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
@@ -367,3 +367,14 @@
   PositiveIndirectMember() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize 
these fields: A
 };
+
+
+struct Bug30487
+{
+  int a = 0;
+};
+
+void Bug30487_f()
+{
+  Bug30487 s;
+}
Index: clang-tidy/utils/TypeTraits.cpp
===
--- clang-tidy/utils/TypeTraits.cpp
+++ clang-tidy/utils/TypeTraits.cpp
@@ -62,8 +62,10 @@
   if (ClassDecl->hasTrivialDefaultConstructor())
 return true;
 
-  // If all its fields are trivially constructible.
+  // If all its fields are trivially constructible and have no default 
initializers.
   for (const FieldDecl *Field : ClassDecl->fields()) {
+if (Field->hasInClassInitializer())
+  return false;
 if (!isTriviallyDefaultConstructible(Field->getType(), Context))
   return false;
   }


Index: test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
@@ -367,3 +367,14 @@
   PositiveIndirectMember() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A
 };
+
+
+struct Bug30487
+{
+  int a = 0;
+};
+
+void Bug30487_f()
+{
+  Bug30487 s;
+}
Index: clang-tidy/utils/TypeTraits.cpp
===
--- clang-tidy/utils/TypeTraits.cpp
+++ clang-tidy/utils/TypeTraits.cpp
@@ -62,8 +62,10 @@
   if (ClassDecl->hasTrivialDefaultConstructor())
 return true;
 
-  // If all its fields are trivially constructible.
+  // If all its fields are trivially constructible and have no default initializers.
   for (const FieldDecl *Field : ClassDecl->fields()) {
+if (Field->hasInClassInitializer())
+  return false;
 if (!isTriviallyDefaultConstructible(Field->getType(), Context))
   return false;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits