[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL285909: Support for Python 3 in libclang python bindings 
(authored by jbcoe).

Changed prior to commit:
  https://reviews.llvm.org/D26082?vs=76774=76854#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26082

Files:
  cfe/trunk/bindings/python/clang/cindex.py
  cfe/trunk/bindings/python/tests/cindex/test_translation_unit.py

Index: cfe/trunk/bindings/python/clang/cindex.py
===
--- cfe/trunk/bindings/python/clang/cindex.py
+++ cfe/trunk/bindings/python/clang/cindex.py
@@ -64,6 +64,7 @@
 
 from ctypes import *
 import collections
+import sys
 
 import clang.enumerations
 
@@ -73,6 +74,33 @@
 # this by marshalling object arguments as void**.
 c_object_p = POINTER(c_void_p)
 
+if sys.version_info[0] > 2:
+# Python 3 strings are unicode, translate them to/from utf8 for C-interop
+# Python 3 replaces xrange with range, we want xrange behaviour
+xrange = range
+
+class c_string_p(c_char_p):
+def __init__(self, p=None):
+if type(p) == str:
+p = p.encode("utf8")
+super(c_char_p, self).__init__(p)
+
+def __str__(self):
+return str(self.value)
+
+@property
+def value(self):
+if super(c_char_p, self).value is None:
+return None
+return super(c_char_p, self).value.decode("utf8")
+
+@classmethod
+def from_param(cls, param):
+return cls(param)
+else:
+c_string_p = c_char_p
+
+
 callbacks = {}
 
 ### Exception Classes ###
@@ -147,7 +175,7 @@
 class _CXString(Structure):
 """Helper for transforming CXString results."""
 
-_fields_ = [("spelling", c_char_p), ("free", c_int)]
+_fields_ = [("spelling", c_string_p), ("free", c_int)]
 
 def __del__(self):
 conf.lib.clang_disposeString(self)
@@ -329,7 +357,7 @@
 
 @property
 def spelling(self):
-return conf.lib.clang_getDiagnosticSpelling(self)
+return str(conf.lib.clang_getDiagnosticSpelling(self))
 
 @property
 def ranges(self):
@@ -358,8 +386,8 @@
 
 def __getitem__(self, key):
 range = SourceRange()
-value = conf.lib.clang_getDiagnosticFixIt(self.diag, key,
-byref(range))
+value = str(conf.lib.clang_getDiagnosticFixIt(self.diag, key,
+byref(range)))
 if len(value) == 0:
 raise IndexError
 
@@ -392,20 +420,20 @@
 @property
 def category_name(self):
 """The string name of the category for this diagnostic."""
-return conf.lib.clang_getDiagnosticCategoryText(self)
+return str(conf.lib.clang_getDiagnosticCategoryText(self))
 
 @property
 def option(self):
 """The command-line option that enables this diagnostic."""
-return conf.lib.clang_getDiagnosticOption(self, None)
+return str(conf.lib.clang_getDiagnosticOption(self, None))
 
 @property
 def disable_option(self):
 """The command-line option that disables this diagnostic."""
 disable = _CXString()
 conf.lib.clang_getDiagnosticOption(self, byref(disable))
 
-return conf.lib.clang_getCString(disable)
+return str(conf.lib.clang_getCString(disable))
 
 def format(self, options=None):
 """
@@ -554,8 +582,8 @@
 if value >= len(self.__class__._kinds):
 self.__class__._kinds += [None] * (value - len(self.__class__._kinds) + 1)
 if self.__class__._kinds[value] is not None:
-raise ValueError,'{0} value {1} already loaded'.format(
-str(self.__class__), value)
+raise ValueError('{0} value {1} already loaded'.format(
+str(self.__class__), value))
 self.value = value
 self.__class__._kinds[value] = self
 self.__class__._name_map = None
@@ -572,12 +600,12 @@
 for key, value in self.__class__.__dict__.items():
 if isinstance(value, self.__class__):
 self._name_map[value] = key
-return self._name_map[self]
+return str(self._name_map[self])
 
 @classmethod
 def from_id(cls, id):
 if id >= len(cls._kinds) or cls._kinds[id] is None:
-raise ValueError,'Unknown template argument kind %d' % id
+raise ValueError('Unknown template argument kind %d' % id)
 return cls._kinds[id]
 
 def __repr__(self):
@@ -596,7 +624,7 @@
 @staticmethod
 def get_all_kinds():
 """Return all CursorKind enumeration instances."""
-return filter(None, CursorKind._kinds)
+return [x for x in CursorKind._kinds if x]
 
 def is_declaration(self):
 """Test if this is a declaration kind."""
@@ -1427,9 +1455,9 @@
 def spelling(self):
 """Return the spelling of the 

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via cfe-commits]

jbcoe added inline comments.



Comment at: bindings/python/clang/cindex.py:77
+# Python 3 strings are unicode, translate them to/from utf8 for C-interop
+if type(u"") == str:
+class c_string_p(c_char_p):

mgorny wrote:
> What is the Python3 version you're aiming to support? If I recall correctly, 
> `u""` is allowed (again) since 3.4. And even then, the condition looks weird 
> and makes me think a while before I figure out what it's supposed to mean.
Replaced with a simple version check



Comment at: bindings/python/clang/cindex.py:518
 
-for i in xrange(0, count):
+for i in range(0, count):
 token = Token()

mgorny wrote:
> compnerd wrote:
> > IIRC, `range` and `xrange` did have some performance difference.  This 
> > would slow down the bindings on python 2.  The difference is obviously not 
> > immediately visible unless count is very high.  I wonder if we can do 
> > anything here to detect the python version and dispatch to `xrange` in 
> > python 2.
> The difference is that `range()` used to construct a complete list in Python 
> 2. In Python 3, `xrange()` (that uses iterator) got renamed to `range()`.
> 
> If you want to avoid performance impact (not sure if it's really measurable 
> here), you can do something alike C for loop:
> 
> i = 0
> while i < count:
> #...
> i += 1
> 
> It's not really idiomatic Python though. OTOH, it won't take more lines than 
> the conditional.
I've defined `xrange` to be `range` for python3. A bit confusing but then we 
can use `xrange` uniformly.



Comment at: bindings/python/clang/cindex.py:623
 """Return all CursorKind enumeration instances."""
-return filter(None, CursorKind._kinds)
+return [x for x in CursorKind._kinds if x]
 

mgorny wrote:
> Why are you changing this? The old version seems to be correct for Python3.
`filter` has changed in Python 3 and the replaced code does not behave the same 
as this simple list comprehension. I've not delved deeper into why but see test 
failures without this change.



Comment at: bindings/python/clang/cindex.py:2573
 if len(args) > 0:
-args_array = (c_char_p * len(args))(* args)
+args_array = (c_string_p * len(args))()
+for i,a in enumerate(args):

mgorny wrote:
> I may be wrong but I think you could use a list comprehension here.
> 
> args_array = (c_string_p * len(args))([c_string_p(x) for x in args])
> 
> You can also try without `[]` to avoid the overhead of constructing list, if 
> the function can take an iterator.
I can't get this to work with a comprension or generator, I agree either would 
be an improvement.



Comment at: bindings/python/tests/cindex/test_translation_unit.py:62
 def test_unsaved_files_2():
-import StringIO
+try:
+from StringIO import StringIO

mgorny wrote:
> Could you try inverting this? Python 2.7 already has `io.StringIO`, so that 
> branch is much more likely to work.
Python 2.7's `io.StringIO` needs a unicode string.


https://reviews.llvm.org/D26082



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

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via cfe-commits]

jbcoe updated this revision to Diff 76774.
jbcoe marked an inline comment as done.
jbcoe added a comment.

Respond to review comments.


https://reviews.llvm.org/D26082

Files:
  bindings/python/clang/cindex.py
  bindings/python/tests/cindex/test_translation_unit.py

Index: bindings/python/tests/cindex/test_translation_unit.py
===
--- bindings/python/tests/cindex/test_translation_unit.py
+++ bindings/python/tests/cindex/test_translation_unit.py
@@ -59,9 +59,13 @@
 assert spellings[-1] == 'y'
 
 def test_unsaved_files_2():
-import StringIO
+try:
+from StringIO import StringIO
+except:
+from io import StringIO
+
 tu = TranslationUnit.from_source('fake.c', unsaved_files = [
-('fake.c', StringIO.StringIO('int x;'))])
+('fake.c', StringIO('int x;'))])
 spellings = [c.spelling for c in tu.cursor.get_children()]
 assert spellings[-1] == 'x'
 
Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -64,6 +64,7 @@
 
 from ctypes import *
 import collections
+import sys
 
 import clang.enumerations
 
@@ -73,6 +74,33 @@
 # this by marshalling object arguments as void**.
 c_object_p = POINTER(c_void_p)
 
+if sys.version_info[0] > 2:
+# Python 3 strings are unicode, translate them to/from utf8 for C-interop
+# Python 3 replaces xrange with range, we want xrange behaviour
+xrange = range
+
+class c_string_p(c_char_p):
+def __init__(self, p=None):
+if type(p) == str:
+p = p.encode("utf8")
+super(c_char_p, self).__init__(p)
+
+def __str__(self):
+return str(self.value)
+
+@property
+def value(self):
+if super(c_char_p, self).value is None:
+return None
+return super(c_char_p, self).value.decode("utf8")
+
+@classmethod
+def from_param(cls, param):
+return cls(param)
+else:
+c_string_p = c_char_p
+
+
 callbacks = {}
 
 ### Exception Classes ###
@@ -147,7 +175,7 @@
 class _CXString(Structure):
 """Helper for transforming CXString results."""
 
-_fields_ = [("spelling", c_char_p), ("free", c_int)]
+_fields_ = [("spelling", c_string_p), ("free", c_int)]
 
 def __del__(self):
 conf.lib.clang_disposeString(self)
@@ -329,7 +357,7 @@
 
 @property
 def spelling(self):
-return conf.lib.clang_getDiagnosticSpelling(self)
+return str(conf.lib.clang_getDiagnosticSpelling(self))
 
 @property
 def ranges(self):
@@ -358,8 +386,8 @@
 
 def __getitem__(self, key):
 range = SourceRange()
-value = conf.lib.clang_getDiagnosticFixIt(self.diag, key,
-byref(range))
+value = str(conf.lib.clang_getDiagnosticFixIt(self.diag, key,
+byref(range)))
 if len(value) == 0:
 raise IndexError
 
@@ -392,20 +420,20 @@
 @property
 def category_name(self):
 """The string name of the category for this diagnostic."""
-return conf.lib.clang_getDiagnosticCategoryText(self)
+return str(conf.lib.clang_getDiagnosticCategoryText(self))
 
 @property
 def option(self):
 """The command-line option that enables this diagnostic."""
-return conf.lib.clang_getDiagnosticOption(self, None)
+return str(conf.lib.clang_getDiagnosticOption(self, None))
 
 @property
 def disable_option(self):
 """The command-line option that disables this diagnostic."""
 disable = _CXString()
 conf.lib.clang_getDiagnosticOption(self, byref(disable))
 
-return conf.lib.clang_getCString(disable)
+return str(conf.lib.clang_getCString(disable))
 
 def format(self, options=None):
 """
@@ -554,8 +582,8 @@
 if value >= len(self.__class__._kinds):
 self.__class__._kinds += [None] * (value - len(self.__class__._kinds) + 1)
 if self.__class__._kinds[value] is not None:
-raise ValueError,'{0} value {1} already loaded'.format(
-str(self.__class__), value)
+raise ValueError('{0} value {1} already loaded'.format(
+str(self.__class__), value))
 self.value = value
 self.__class__._kinds[value] = self
 self.__class__._name_map = None
@@ -572,12 +600,12 @@
 for key, value in self.__class__.__dict__.items():
 if isinstance(value, self.__class__):
 self._name_map[value] = key
-return self._name_map[self]
+return str(self._name_map[self])
 
 @classmethod
 def from_id(cls, id):
 if id >= len(cls._kinds) or cls._kinds[id] is None:
-raise ValueError,'Unknown template argument kind %d' % id
+  

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via cfe-commits]

jbcoe removed rL LLVM as the repository for this revision.
jbcoe updated this revision to Diff 76344.
jbcoe added a comment.

Remove mistakenly committed debugging output.


https://reviews.llvm.org/D26082

Files:
  bindings/python/clang/cindex.py
  bindings/python/tests/cindex/test_translation_unit.py

Index: bindings/python/tests/cindex/test_translation_unit.py
===
--- bindings/python/tests/cindex/test_translation_unit.py
+++ bindings/python/tests/cindex/test_translation_unit.py
@@ -59,9 +59,13 @@
 assert spellings[-1] == 'y'
 
 def test_unsaved_files_2():
-import StringIO
+try:
+from StringIO import StringIO
+except:
+from io import StringIO
+
 tu = TranslationUnit.from_source('fake.c', unsaved_files = [
-('fake.c', StringIO.StringIO('int x;'))])
+('fake.c', StringIO('int x;'))])
 spellings = [c.spelling for c in tu.cursor.get_children()]
 assert spellings[-1] == 'x'
 
Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -73,6 +73,30 @@
 # this by marshalling object arguments as void**.
 c_object_p = POINTER(c_void_p)
 
+# Python 3 strings are unicode, translate them to/from utf8 for C-interop
+if type(u"") == str:
+class c_string_p(c_char_p):
+def __init__(self, p=None):
+if type(p) == str:
+p = p.encode("utf8")
+super(c_char_p, self).__init__(p)
+
+def __str__(self):
+return str(self.value)
+
+@property
+def value(self):
+if super(c_char_p, self).value is None:
+return None
+return super(c_char_p, self).value.decode("utf8")
+
+@classmethod
+def from_param(cls, param):
+return cls(param)
+else:
+c_string_p = c_char_p
+
+
 callbacks = {}
 
 ### Exception Classes ###
@@ -147,7 +171,7 @@
 class _CXString(Structure):
 """Helper for transforming CXString results."""
 
-_fields_ = [("spelling", c_char_p), ("free", c_int)]
+_fields_ = [("spelling", c_string_p), ("free", c_int)]
 
 def __del__(self):
 conf.lib.clang_disposeString(self)
@@ -329,7 +353,7 @@
 
 @property
 def spelling(self):
-return conf.lib.clang_getDiagnosticSpelling(self)
+return str(conf.lib.clang_getDiagnosticSpelling(self))
 
 @property
 def ranges(self):
@@ -358,8 +382,8 @@
 
 def __getitem__(self, key):
 range = SourceRange()
-value = conf.lib.clang_getDiagnosticFixIt(self.diag, key,
-byref(range))
+value = str(conf.lib.clang_getDiagnosticFixIt(self.diag, key,
+byref(range)))
 if len(value) == 0:
 raise IndexError
 
@@ -392,20 +416,20 @@
 @property
 def category_name(self):
 """The string name of the category for this diagnostic."""
-return conf.lib.clang_getDiagnosticCategoryText(self)
+return str(conf.lib.clang_getDiagnosticCategoryText(self))
 
 @property
 def option(self):
 """The command-line option that enables this diagnostic."""
-return conf.lib.clang_getDiagnosticOption(self, None)
+return str(conf.lib.clang_getDiagnosticOption(self, None))
 
 @property
 def disable_option(self):
 """The command-line option that disables this diagnostic."""
 disable = _CXString()
 conf.lib.clang_getDiagnosticOption(self, byref(disable))
 
-return conf.lib.clang_getCString(disable)
+return str(conf.lib.clang_getCString(disable))
 
 def format(self, options=None):
 """
@@ -491,7 +515,7 @@
 
 token_group = TokenGroup(tu, tokens_memory, tokens_count)
 
-for i in xrange(0, count):
+for i in range(0, count):
 token = Token()
 token.int_data = tokens_array[i].int_data
 token.ptr_data = tokens_array[i].ptr_data
@@ -554,8 +578,8 @@
 if value >= len(self.__class__._kinds):
 self.__class__._kinds += [None] * (value - len(self.__class__._kinds) + 1)
 if self.__class__._kinds[value] is not None:
-raise ValueError,'{0} value {1} already loaded'.format(
-str(self.__class__), value)
+raise ValueError('{0} value {1} already loaded'.format(
+str(self.__class__), value))
 self.value = value
 self.__class__._kinds[value] = self
 self.__class__._name_map = None
@@ -572,12 +596,12 @@
 for key, value in self.__class__.__dict__.items():
 if isinstance(value, self.__class__):
 self._name_map[value] = key
-return self._name_map[self]
+return str(self._name_map[self])
 
 @classmethod
 def from_id(cls, 

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via cfe-commits]

jbcoe retitled this revision from "Incomplete support for Python 3 in libclang 
python bindings" to "Support for Python 3 in libclang python bindings".
jbcoe updated the summary for this revision.
jbcoe updated this revision to Diff 76338.
jbcoe added a comment.

Python bindings tests now pass in Python 3.


Repository:
  rL LLVM

https://reviews.llvm.org/D26082

Files:
  bindings/python/clang/cindex.py
  bindings/python/tests/cindex/test_comment.py
  bindings/python/tests/cindex/test_translation_unit.py

Index: bindings/python/tests/cindex/test_translation_unit.py
===
--- bindings/python/tests/cindex/test_translation_unit.py
+++ bindings/python/tests/cindex/test_translation_unit.py
@@ -59,9 +59,13 @@
 assert spellings[-1] == 'y'
 
 def test_unsaved_files_2():
-import StringIO
+try:
+from StringIO import StringIO
+except:
+from io import StringIO
+
 tu = TranslationUnit.from_source('fake.c', unsaved_files = [
-('fake.c', StringIO.StringIO('int x;'))])
+('fake.c', StringIO('int x;'))])
 spellings = [c.spelling for c in tu.cursor.get_children()]
 assert spellings[-1] == 'x'
 
Index: bindings/python/tests/cindex/test_comment.py
===
--- bindings/python/tests/cindex/test_comment.py
+++ bindings/python/tests/cindex/test_comment.py
@@ -34,6 +34,12 @@
 f = get_cursor(tu, 'f')
 raw = f.raw_comment
 brief = f.brief_comment
+
+print(raw)
+print(type(raw))
+print(brief)
+print(type(brief))
+
 assert raw is None
 assert brief is None
 
Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -73,6 +73,30 @@
 # this by marshalling object arguments as void**.
 c_object_p = POINTER(c_void_p)
 
+# Python 3 strings are unicode, translate them to/from utf8 for C-interop
+if type(u"") == str:
+class c_string_p(c_char_p):
+def __init__(self, p=None):
+if type(p) == str:
+p = p.encode("utf8")
+super(c_char_p, self).__init__(p)
+
+def __str__(self):
+return str(self.value)
+
+@property
+def value(self):
+if super(c_char_p, self).value is None:
+return None
+return super(c_char_p, self).value.decode("utf8")
+
+@classmethod
+def from_param(cls, param):
+return cls(param)
+else:
+c_string_p = c_char_p
+
+
 callbacks = {}
 
 ### Exception Classes ###
@@ -147,7 +171,7 @@
 class _CXString(Structure):
 """Helper for transforming CXString results."""
 
-_fields_ = [("spelling", c_char_p), ("free", c_int)]
+_fields_ = [("spelling", c_string_p), ("free", c_int)]
 
 def __del__(self):
 conf.lib.clang_disposeString(self)
@@ -329,7 +353,7 @@
 
 @property
 def spelling(self):
-return conf.lib.clang_getDiagnosticSpelling(self)
+return str(conf.lib.clang_getDiagnosticSpelling(self))
 
 @property
 def ranges(self):
@@ -358,8 +382,8 @@
 
 def __getitem__(self, key):
 range = SourceRange()
-value = conf.lib.clang_getDiagnosticFixIt(self.diag, key,
-byref(range))
+value = str(conf.lib.clang_getDiagnosticFixIt(self.diag, key,
+byref(range)))
 if len(value) == 0:
 raise IndexError
 
@@ -392,20 +416,20 @@
 @property
 def category_name(self):
 """The string name of the category for this diagnostic."""
-return conf.lib.clang_getDiagnosticCategoryText(self)
+return str(conf.lib.clang_getDiagnosticCategoryText(self))
 
 @property
 def option(self):
 """The command-line option that enables this diagnostic."""
-return conf.lib.clang_getDiagnosticOption(self, None)
+return str(conf.lib.clang_getDiagnosticOption(self, None))
 
 @property
 def disable_option(self):
 """The command-line option that disables this diagnostic."""
 disable = _CXString()
 conf.lib.clang_getDiagnosticOption(self, byref(disable))
 
-return conf.lib.clang_getCString(disable)
+return str(conf.lib.clang_getCString(disable))
 
 def format(self, options=None):
 """
@@ -491,7 +515,7 @@
 
 token_group = TokenGroup(tu, tokens_memory, tokens_count)
 
-for i in xrange(0, count):
+for i in range(0, count):
 token = Token()
 token.int_data = tokens_array[i].int_data
 token.ptr_data = tokens_array[i].ptr_data
@@ -554,8 +578,8 @@
 if value >= len(self.__class__._kinds):
 self.__class__._kinds += [None] * (value - len(self.__class__._kinds) + 1)
 if self.__class__._kinds[value] is 

[PATCH] D26082: Incomplete support for Python 3 in libclang python bindings

[Jonathan B Coe via cfe-commits]

jbcoe created this revision.
jbcoe added reviewers: eliben, compnerd, nemanjai, skalinichev.
jbcoe added a subscriber: cfe-commits.
jbcoe set the repository for this revision to rL LLVM.

This is incomplete and I'm in need of some input.

Some test pass in Python 3 now. Python 2 tests pass as before.

Work so far:

`map` in Python 3 is lazily evaluated so the method by which functions are 
registered needed updating.

Strings are unicode in Python 3 not UTF-8, I've tried to create an new 
c_types-like class (c_string_p) to automate the conversion. It mostly works but 
I may have overlooked things.

Once we can get all Python 3 tests to pass then I'd like to get this merged.


Repository:
  rL LLVM

https://reviews.llvm.org/D26082

Files:
  bindings/python/clang/cindex.py

Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -73,6 +73,30 @@
 # this by marshalling object arguments as void**.
 c_object_p = POINTER(c_void_p)
 
+# Python 3 strings are unicode, translate them to/from utf8 for C-interop
+if type(u"") == str:
+class c_string_p(c_char_p):
+def __init__(self, p=None):
+if p is None:
+p = ""
+if type(p) == str:
+p = p.encode("utf8")
+super(c_char_p, self).__init__(p)
+
+def __str__(self):
+return self.value
+
+@property
+def value(self):
+return super(c_char_p, self).value.decode("utf8")
+
+@classmethod
+def from_param(cls, param):
+return cls(param)
+else:
+c_string_p = c_char_p
+
+
 callbacks = {}
 
 ### Exception Classes ###
@@ -147,7 +171,7 @@
 class _CXString(Structure):
 """Helper for transforming CXString results."""
 
-_fields_ = [("spelling", c_char_p), ("free", c_int)]
+_fields_ = [("spelling", c_string_p), ("free", c_int)]
 
 def __del__(self):
 conf.lib.clang_disposeString(self)
@@ -554,8 +578,8 @@
 if value >= len(self.__class__._kinds):
 self.__class__._kinds += [None] * (value - len(self.__class__._kinds) + 1)
 if self.__class__._kinds[value] is not None:
-raise ValueError,'{0} value {1} already loaded'.format(
-str(self.__class__), value)
+raise ValueError('{0} value {1} already loaded'.format(
+str(self.__class__), value))
 self.value = value
 self.__class__._kinds[value] = self
 self.__class__._name_map = None
@@ -577,7 +601,7 @@
 @classmethod
 def from_id(cls, id):
 if id >= len(cls._kinds) or cls._kinds[id] is None:
-raise ValueError,'Unknown template argument kind %d' % id
+raise ValueError('Unknown template argument kind %d' % id)
 return cls._kinds[id]
 
 def __repr__(self):
@@ -1775,7 +1799,7 @@
 if value >= len(StorageClass._kinds):
 StorageClass._kinds += [None] * (value - len(StorageClass._kinds) + 1)
 if StorageClass._kinds[value] is not None:
-raise ValueError,'StorageClass already loaded'
+raise ValueError('StorageClass already loaded')
 self.value = value
 StorageClass._kinds[value] = self
 StorageClass._name_map = None
@@ -1796,7 +1820,7 @@
 @staticmethod
 def from_id(id):
 if id >= len(StorageClass._kinds) or not StorageClass._kinds[id]:
-raise ValueError,'Unknown storage class %d' % id
+raise ValueError('Unknown storage class %d' % id)
 return StorageClass._kinds[id]
 
 def __repr__(self):
@@ -2125,7 +2149,7 @@
 """
 Retrieve the offset of a field in the record.
 """
-return conf.lib.clang_Type_getOffsetOf(self, c_char_p(fieldname))
+return conf.lib.clang_Type_getOffsetOf(self, c_string_p(fieldname))
 
 def get_ref_qualifier(self):
 """
@@ -2184,7 +2208,7 @@
 
 class _CXUnsavedFile(Structure):
 """Helper for passing unsaved file arguments."""
-_fields_ = [("name", c_char_p), ("contents", c_char_p), ('length', c_ulong)]
+_fields_ = [("name", c_string_p), ("contents", c_string_p), ('length', c_ulong)]
 
 # Functions calls through the python interface are rather slow. Fortunately,
 # for most symboles, we do not need to perform a function call. Their spelling
@@ -2539,17 +2563,19 @@
 
 args_array = None
 if len(args) > 0:
-args_array = (c_char_p * len(args))(* args)
+args_array = (c_string_p * len(args))()
+for i,a in enumerate(args):
+args_array[i] = c_string_p(a)
 
 unsaved_array = None
 if len(unsaved_files) > 0:
 unsaved_array = (_CXUnsavedFile * len(unsaved_files))()
 for i, (name, contents) in enumerate(unsaved_files):
 if hasattr(contents, "read"):
 contents = 

Re: [PATCH] D23008: [clang-tidy] fix segfault in cppcore-guidelines-special-member-functions check

[Jonathan B Coe via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL277523: [clang-tidy] Fix segfault in 
cppcore-guidelines-special-member-functions check (authored by jbcoe).

Changed prior to commit:
  https://reviews.llvm.org/D23008?vs=66541=66558#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23008

Files:
  
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
  
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
  
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions.cpp

Index: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -1,4 +1,4 @@
-//===--- SpecialMemberFunctionsCheck.h - clang-tidy---*- C++ -*-===//
+//===--- SpecialMemberFunctionsCheck.h - clang-tidy--*- C++ -*-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -41,15 +41,11 @@
 
   using ClassDefId = std::pair;
 
-  using ClassDefiningSpecialMembersMap = llvm::DenseMap>;
+  using ClassDefiningSpecialMembersMap =
+  llvm::DenseMap>;
 
 private:
-
-  static llvm::StringRef toString(SpecialMemberFunctionKind K);
-
-  static std::string join(llvm::ArrayRef SMFS,
-  llvm::StringRef AndOr);
-
   ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
 };
 
@@ -65,7 +61,7 @@
 struct DenseMapInfo<
 clang::tidy::cppcoreguidelines::SpecialMemberFunctionsCheck::ClassDefId> {
   using ClassDefId =
-clang::tidy::cppcoreguidelines::SpecialMemberFunctionsCheck::ClassDefId;
+  clang::tidy::cppcoreguidelines::SpecialMemberFunctionsCheck::ClassDefId;
 
   static inline ClassDefId getEmptyKey() {
 return ClassDefId(
Index: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -43,25 +43,26 @@
   this);
 }
 
-llvm::StringRef SpecialMemberFunctionsCheck::toString(
-SpecialMemberFunctionsCheck::SpecialMemberFunctionKind K) {
+static llvm::StringRef
+toString(SpecialMemberFunctionsCheck::SpecialMemberFunctionKind K) {
   switch (K) {
-  case SpecialMemberFunctionKind::Destructor:
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::Destructor:
 return "a destructor";
-  case SpecialMemberFunctionKind::CopyConstructor:
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::CopyConstructor:
 return "a copy constructor";
-  case SpecialMemberFunctionKind::CopyAssignment:
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::CopyAssignment:
 return "a copy assignment operator";
-  case SpecialMemberFunctionKind::MoveConstructor:
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::MoveConstructor:
 return "a move constructor";
-  case SpecialMemberFunctionKind::MoveAssignment:
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::MoveAssignment:
 return "a move assignment operator";
   }
   llvm_unreachable("Unhandled SpecialMemberFunctionKind");
 }
 
-std::string SpecialMemberFunctionsCheck::join(
-llvm::ArrayRef SMFS, llvm::StringRef AndOr) {
+static std::string
+join(ArrayRef SMFS,
+ llvm::StringRef AndOr) {
 
   assert(!SMFS.empty() &&
  "List of defined or undefined members should never be empty.");
@@ -97,7 +98,7 @@
 
   for (const auto  : Matchers)
 if (Result.Nodes.getNodeAs(KV.first))
-  ClassWithSpecialMembers[ID].push_back(KV.second);
+  ClassWithSpecialMembers[ID].insert(KV.second);
 }
 
 void SpecialMemberFunctionsCheck::onEndOfTranslationUnit() {
@@ -112,7 +113,7 @@
   }
 
   for (const auto  : ClassWithSpecialMembers) {
-ArrayRef DefinedSpecialMembers = C.second;
+const auto  = C.second;
 
 if (DefinedSpecialMembers.size() == AllSpecialMembers.size())
   continue;
@@ -124,7 +125,7 @@
 std::back_inserter(UndefinedSpecialMembers));
 
 diag(C.first.first, "class '%0' defines %1 but does not define %2")
-<< C.first.second << join(DefinedSpecialMembers, " and ")
+<< C.first.second << join(DefinedSpecialMembers.getArrayRef(), " and ")
 << join(UndefinedSpecialMembers, " or ");
   }
 }
Index: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
===
--- 

Re: [PATCH] D23008: [clang-tidy] fix segfault in cppcore-guidelines-special-member-functions check

[Jonathan B Coe via cfe-commits]

jbcoe marked an inline comment as done.


Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:63-64
@@ -62,3 +62,4 @@
 
-std::string SpecialMemberFunctionsCheck::join(
-llvm::ArrayRef SMFS, llvm::StringRef AndOr) {
+static std::string
+join(ArrayRef SMFS,
+ llvm::StringRef AndOr) {

iterator_range loses me size, empty and index access. There's a function that 
gets and ArrayRef from a SmallSetVector (I wonder why there's no implicit 
conversion defined) so I can change the signature to take ArrayRef.


https://reviews.llvm.org/D23008



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

Re: [PATCH] D23008: [clang-tidy] fix segfault in cppcore-guidelines-special-member-functions check

[Jonathan B Coe via cfe-commits]

jbcoe updated this revision to Diff 66541.
jbcoe added a comment.

static `join` function is no longer a function template.


https://reviews.llvm.org/D23008

Files:
  clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
  clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
  test/clang-tidy/cppcoreguidelines-special-member-functions.cpp

Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
@@ -50,3 +50,18 @@
   DeletesCopyDefaultsMove =(DeletesCopyDefaultsMove &&) = default;
   ~DeletesCopyDefaultsMove() = default;
 };
+
+template 
+struct TemplateClass {
+  TemplateClass() = default;
+  TemplateClass(const TemplateClass &);
+  TemplateClass =(const TemplateClass &);
+  TemplateClass(TemplateClass &&);
+  TemplateClass =(TemplateClass &&);
+  ~TemplateClass();
+};
+
+// Multiple instantiations of a class template will trigger multiple matches for defined special members.
+// This should not cause problems.
+TemplateClass InstantiationWithInt;
+TemplateClass InstantiationWithDouble;
Index: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
===
--- clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++ clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -1,4 +1,4 @@
-//===--- SpecialMemberFunctionsCheck.h - clang-tidy---*- C++ -*-===//
+//===--- SpecialMemberFunctionsCheck.h - clang-tidy--*- C++ -*-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -41,15 +41,11 @@
 
   using ClassDefId = std::pair;
 
-  using ClassDefiningSpecialMembersMap = llvm::DenseMap>;
+  using ClassDefiningSpecialMembersMap =
+  llvm::DenseMap>;
 
 private:
-
-  static llvm::StringRef toString(SpecialMemberFunctionKind K);
-
-  static std::string join(llvm::ArrayRef SMFS,
-  llvm::StringRef AndOr);
-
   ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
 };
 
@@ -65,7 +61,7 @@
 struct DenseMapInfo<
 clang::tidy::cppcoreguidelines::SpecialMemberFunctionsCheck::ClassDefId> {
   using ClassDefId =
-clang::tidy::cppcoreguidelines::SpecialMemberFunctionsCheck::ClassDefId;
+  clang::tidy::cppcoreguidelines::SpecialMemberFunctionsCheck::ClassDefId;
 
   static inline ClassDefId getEmptyKey() {
 return ClassDefId(
Index: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
===
--- clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++ clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -43,25 +43,26 @@
   this);
 }
 
-llvm::StringRef SpecialMemberFunctionsCheck::toString(
-SpecialMemberFunctionsCheck::SpecialMemberFunctionKind K) {
+static llvm::StringRef
+toString(SpecialMemberFunctionsCheck::SpecialMemberFunctionKind K) {
   switch (K) {
-  case SpecialMemberFunctionKind::Destructor:
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::Destructor:
 return "a destructor";
-  case SpecialMemberFunctionKind::CopyConstructor:
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::CopyConstructor:
 return "a copy constructor";
-  case SpecialMemberFunctionKind::CopyAssignment:
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::CopyAssignment:
 return "a copy assignment operator";
-  case SpecialMemberFunctionKind::MoveConstructor:
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::MoveConstructor:
 return "a move constructor";
-  case SpecialMemberFunctionKind::MoveAssignment:
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::MoveAssignment:
 return "a move assignment operator";
   }
   llvm_unreachable("Unhandled SpecialMemberFunctionKind");
 }
 
-std::string SpecialMemberFunctionsCheck::join(
-llvm::ArrayRef SMFS, llvm::StringRef AndOr) {
+static std::string
+join(ArrayRef SMFS,
+ llvm::StringRef AndOr) {
 
   assert(!SMFS.empty() &&
  "List of defined or undefined members should never be empty.");
@@ -97,7 +98,7 @@
 
   for (const auto  : Matchers)
 if (Result.Nodes.getNodeAs(KV.first))
-  ClassWithSpecialMembers[ID].push_back(KV.second);
+  ClassWithSpecialMembers[ID].insert(KV.second);
 }
 
 void SpecialMemberFunctionsCheck::onEndOfTranslationUnit() {
@@ -112,7 +113,7 @@
   }
 
   for (const auto  : ClassWithSpecialMembers) {
-ArrayRef DefinedSpecialMembers = C.second;
+const auto  = C.second;
 
 if (DefinedSpecialMembers.size() == AllSpecialMembers.size())
   continue;
@@ -124,7 +125,7 @@
 

Re: [PATCH] D23008: [clang-tidy] fix segfault in cppcore-guidelines-special-member-functions check

[Jonathan B Coe via cfe-commits]

jbcoe added inline comments.


Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:63-64
@@ -62,4 +62,4 @@
 
-std::string SpecialMemberFunctionsCheck::join(
-llvm::ArrayRef SMFS, llvm::StringRef AndOr) {
+template 
+static std::string join(const R , llvm::StringRef AndOr) {
 

aaron.ballman wrote:
> I hadn't noticed this before -- why has this been converted to an 
> unrestricted template? If generality is what you were going for, it should 
> have accepted a range (or a pair of iterators) instead?
It needs to handle SmallSetVector which I can't convert to an ArrayRef.

How do I specify a range?


https://reviews.llvm.org/D23008



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

Re: [PATCH] D23008: [clang-tidy] fix segfault in cppcore-guidelines-special-member-functions check

[Jonathan B Coe via cfe-commits]

jbcoe removed rL LLVM as the repository for this revision.
jbcoe updated this revision to Diff 66444.
jbcoe added a comment.

Add test that triggers segfault without fix in place.


https://reviews.llvm.org/D23008

Files:
  clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
  clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
  test/clang-tidy/cppcoreguidelines-special-member-functions.cpp

Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
@@ -50,3 +50,18 @@
   DeletesCopyDefaultsMove =(DeletesCopyDefaultsMove &&) = default;
   ~DeletesCopyDefaultsMove() = default;
 };
+
+template 
+struct TemplateClass {
+  TemplateClass() = default;
+  TemplateClass(const TemplateClass &);
+  TemplateClass =(const TemplateClass &);
+  TemplateClass(TemplateClass &&);
+  TemplateClass =(TemplateClass &&);
+  ~TemplateClass();
+};
+
+// Multiple instantiations of a class template will trigger multiple matches for defined special members.
+// This should not cause problems.
+TemplateClass InstantiationWithInt;
+TemplateClass InstantiationWithDouble;
Index: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
===
--- clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++ clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -1,4 +1,4 @@
-//===--- SpecialMemberFunctionsCheck.h - clang-tidy---*- C++ -*-===//
+//===--- SpecialMemberFunctionsCheck.h - clang-tidy--*- C++ -*-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -41,15 +41,11 @@
 
   using ClassDefId = std::pair;
 
-  using ClassDefiningSpecialMembersMap = llvm::DenseMap>;
+  using ClassDefiningSpecialMembersMap =
+  llvm::DenseMap>;
 
 private:
-
-  static llvm::StringRef toString(SpecialMemberFunctionKind K);
-
-  static std::string join(llvm::ArrayRef SMFS,
-  llvm::StringRef AndOr);
-
   ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
 };
 
@@ -65,7 +61,7 @@
 struct DenseMapInfo<
 clang::tidy::cppcoreguidelines::SpecialMemberFunctionsCheck::ClassDefId> {
   using ClassDefId =
-clang::tidy::cppcoreguidelines::SpecialMemberFunctionsCheck::ClassDefId;
+  clang::tidy::cppcoreguidelines::SpecialMemberFunctionsCheck::ClassDefId;
 
   static inline ClassDefId getEmptyKey() {
 return ClassDefId(
Index: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
===
--- clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++ clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -43,25 +43,25 @@
   this);
 }
 
-llvm::StringRef SpecialMemberFunctionsCheck::toString(
-SpecialMemberFunctionsCheck::SpecialMemberFunctionKind K) {
+static llvm::StringRef
+toString(SpecialMemberFunctionsCheck::SpecialMemberFunctionKind K) {
   switch (K) {
-  case SpecialMemberFunctionKind::Destructor:
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::Destructor:
 return "a destructor";
-  case SpecialMemberFunctionKind::CopyConstructor:
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::CopyConstructor:
 return "a copy constructor";
-  case SpecialMemberFunctionKind::CopyAssignment:
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::CopyAssignment:
 return "a copy assignment operator";
-  case SpecialMemberFunctionKind::MoveConstructor:
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::MoveConstructor:
 return "a move constructor";
-  case SpecialMemberFunctionKind::MoveAssignment:
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::MoveAssignment:
 return "a move assignment operator";
   }
   llvm_unreachable("Unhandled SpecialMemberFunctionKind");
 }
 
-std::string SpecialMemberFunctionsCheck::join(
-llvm::ArrayRef SMFS, llvm::StringRef AndOr) {
+template 
+static std::string join(const R , llvm::StringRef AndOr) {
 
   assert(!SMFS.empty() &&
  "List of defined or undefined members should never be empty.");
@@ -97,7 +97,7 @@
 
   for (const auto  : Matchers)
 if (Result.Nodes.getNodeAs(KV.first))
-  ClassWithSpecialMembers[ID].push_back(KV.second);
+  ClassWithSpecialMembers[ID].insert(KV.second);
 }
 
 void SpecialMemberFunctionsCheck::onEndOfTranslationUnit() {
@@ -112,7 +112,7 @@
   }
 
   for (const auto  : ClassWithSpecialMembers) {
-ArrayRef DefinedSpecialMembers = C.second;
+const auto  = C.second;
 
 if (DefinedSpecialMembers.size() == AllSpecialMembers.size())
   continue;

Re: [PATCH] D22513: [clang-tidy] add check cppcoreguidelines-special-member-functions

[Jonathan B Coe via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL277262: [clang-tidy] add check 
cppcoreguidelines-special-member-functions (authored by jbcoe).

Changed prior to commit:
  https://reviews.llvm.org/D22513?vs=66161=66219#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D22513

Files:
  clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt
  
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
  
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
  
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions.cpp

Index: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -22,6 +22,7 @@
 #include "ProTypeStaticCastDowncastCheck.h"
 #include "ProTypeUnionAccessCheck.h"
 #include "ProTypeVarargCheck.h"
+#include "SpecialMemberFunctionsCheck.h"
 #include "SlicingCheck.h"
 
 namespace clang {
@@ -54,6 +55,8 @@
 "cppcoreguidelines-pro-type-union-access");
 CheckFactories.registerCheck(
 "cppcoreguidelines-pro-type-vararg");
+CheckFactories.registerCheck(
+"cppcoreguidelines-special-member-functions");
 CheckFactories.registerCheck(
 "cppcoreguidelines-slicing");
 CheckFactories.registerCheck(
Index: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -0,0 +1,101 @@
+//===--- SpecialMemberFunctionsCheck.h - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_SPECIAL_MEMBER_FUNCTIONS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_SPECIAL_MEMBER_FUNCTIONS_H
+
+#include "../ClangTidy.h"
+
+#include "llvm/ADT/DenseMapInfo.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Checks for classes where some, but not all, of the special member functions
+/// are defined.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-special-member-functions.html
+class SpecialMemberFunctionsCheck : public ClangTidyCheck {
+public:
+  SpecialMemberFunctionsCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void onEndOfTranslationUnit() override;
+  
+  enum class SpecialMemberFunctionKind {
+Destructor,
+CopyConstructor,
+CopyAssignment,
+MoveConstructor,
+MoveAssignment
+  };
+
+  using ClassDefId = std::pair;
+
+  using ClassDefiningSpecialMembersMap = llvm::DenseMap>;
+
+private:
+  
+  static llvm::StringRef toString(SpecialMemberFunctionKind K);
+
+  static std::string join(llvm::ArrayRef SMFS,
+  llvm::StringRef AndOr);
+  
+  ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+namespace llvm {
+/// Specialisation of DenseMapInfo to allow ClassDefId objects in DenseMaps
+/// FIXME: Move this to the corresponding cpp file as is done for
+/// clang-tidy/readability/IdentifierNamingCheck.cpp. 
+template <>
+struct DenseMapInfo<
+clang::tidy::cppcoreguidelines::SpecialMemberFunctionsCheck::ClassDefId> {
+  using ClassDefId =
+clang::tidy::cppcoreguidelines::SpecialMemberFunctionsCheck::ClassDefId;
+
+  static inline ClassDefId getEmptyKey() {
+return ClassDefId(
+clang::SourceLocation::getFromRawEncoding(static_cast(-1)),
+"EMPTY");
+  }
+
+  static inline ClassDefId getTombstoneKey() {
+return ClassDefId(
+

Re: [PATCH] D22513: [clang-tidy] add check cppcoreguidelines-special-member-functions

[Jonathan B Coe via cfe-commits]

jbcoe marked 9 inline comments as done.
jbcoe added a comment.

Repository:
  rL LLVM

https://reviews.llvm.org/D22513



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

Re: [PATCH] D22513: [clang-tidy] add check cppcoreguidelines-special-member-functions

[Jonathan B Coe via cfe-commits]

jbcoe set the repository for this revision to rL LLVM.
jbcoe updated this revision to Diff 66161.
jbcoe added a comment.

Fix MSVC warning.


Repository:
  rL LLVM

https://reviews.llvm.org/D22513

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
  clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
  test/clang-tidy/cppcoreguidelines-special-member-functions.cpp

Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t
+
+class DefinesDestructor {
+  ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyConstructor {
+  DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyAssignment {
+  DefinesCopyAssignment =(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor, a copy constructor, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveConstructor {
+  DefinesMoveConstructor(DefinesMoveConstructor &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveConstructor' defines a move constructor but does not define a destructor, a copy constructor, a copy assignment operator or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveAssignment {
+  DefinesMoveAssignment =(DefinesMoveAssignment &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveAssignment' defines a move assignment operator but does not define a destructor, a copy constructor, a copy assignment operator or a move constructor [cppcoreguidelines-special-member-functions]
+class DefinesNothing {
+};
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  DefinesEverything =(const DefinesEverything &);
+  DefinesEverything(DefinesEverything &&);
+  DefinesEverything =(DefinesEverything &&);
+  ~DefinesEverything();
+};
+
+class DeletesEverything {
+  DeletesEverything(const DeletesEverything &) = delete;
+  DeletesEverything =(const DeletesEverything &) = delete;
+  DeletesEverything(DeletesEverything &&) = delete;
+  DeletesEverything =(DeletesEverything &&) = delete;
+  ~DeletesEverything() = delete;
+};
+
+class DeletesCopyDefaultsMove {
+  DeletesCopyDefaultsMove(const DeletesCopyDefaultsMove &) = delete;
+  DeletesCopyDefaultsMove =(const DeletesCopyDefaultsMove &) = delete;
+  DeletesCopyDefaultsMove(DeletesCopyDefaultsMove &&) = default;
+  DeletesCopyDefaultsMove =(DeletesCopyDefaultsMove &&) = default;
+  ~DeletesCopyDefaultsMove() = default;
+};
Index: test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
@@ -0,0 +1,26 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -- -std=c++03
+
+class DefinesDestructor {
+  ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyConstructor {
+  DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyAssignment {
+  DefinesCopyAssignment =(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor or a copy constructor [cppcoreguidelines-special-member-functions]
+
+class DefinesNothing {
+};
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  

Re: [PATCH] D22513: [clang-tidy] add check cppcoreguidelines-special-member-functions

[Jonathan B Coe via cfe-commits]

jbcoe added inline comments.


Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:62
@@ +61,3 @@
+std::string
+SpecialMemberFunctionsCheck::join(llvm::ArrayRef 
SMFS,
+ llvm::StringRef AndOr) {

aaron.ballman wrote:
> I think this join function can maybe be replaced by a call to llvm::join() 
> from StringExtras.h?
I want to join the last member function with "and" or "for", `llvm::join` won't 
let me do that.
Thanks for the suggestion though. I need to explore more of llvm/ADT.


Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h:42-44
@@ +41,5 @@
+
+  using ClassDefId = std::pair;
+
+  using ClassDefiningSpecialMembersMap = llvm::DenseMap>;
+

aaron.ballman wrote:
> Do these need to be public?
I think so, I can't get the specialisation of DenseMapInfo to work otherwise.


Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h:62-63
@@ +61,4 @@
+/// Specialisation of DenseMapInfo to allow ClassDefId objects in DenseMaps
+/// FIXME: Move this to the corresponding cpp file as is done for
+/// clang-tidy/readability/IdentifierNamingCheck.cpp. 
+template <>

aaron.ballman wrote:
> Any reason why not to do this as part of this patch?
I've spent days trying. I can't get code to compile if it's moved and can't 
work out why. As the FIXME says, it works in IdentifierNaming.


https://reviews.llvm.org/D22513



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

Re: [PATCH] D22513: [clang-tidy] add check cppcoreguidelines-special-member-functions

[Jonathan B Coe via cfe-commits]

jbcoe updated this revision to Diff 66011.
jbcoe added a comment.

Minor changes from review.


https://reviews.llvm.org/D22513

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
  clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
  test/clang-tidy/cppcoreguidelines-special-member-functions.cpp

Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t
+
+class DefinesDestructor {
+  ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyConstructor {
+  DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyAssignment {
+  DefinesCopyAssignment =(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor, a copy constructor, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveConstructor {
+  DefinesMoveConstructor(DefinesMoveConstructor &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveConstructor' defines a move constructor but does not define a destructor, a copy constructor, a copy assignment operator or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveAssignment {
+  DefinesMoveAssignment =(DefinesMoveAssignment &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveAssignment' defines a move assignment operator but does not define a destructor, a copy constructor, a copy assignment operator or a move constructor [cppcoreguidelines-special-member-functions]
+class DefinesNothing {
+};
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  DefinesEverything =(const DefinesEverything &);
+  DefinesEverything(DefinesEverything &&);
+  DefinesEverything =(DefinesEverything &&);
+  ~DefinesEverything();
+};
+
+class DeletesEverything {
+  DeletesEverything(const DeletesEverything &) = delete;
+  DeletesEverything =(const DeletesEverything &) = delete;
+  DeletesEverything(DeletesEverything &&) = delete;
+  DeletesEverything =(DeletesEverything &&) = delete;
+  ~DeletesEverything() = delete;
+};
+
+class DeletesCopyDefaultsMove {
+  DeletesCopyDefaultsMove(const DeletesCopyDefaultsMove &) = delete;
+  DeletesCopyDefaultsMove =(const DeletesCopyDefaultsMove &) = delete;
+  DeletesCopyDefaultsMove(DeletesCopyDefaultsMove &&) = default;
+  DeletesCopyDefaultsMove =(DeletesCopyDefaultsMove &&) = default;
+  ~DeletesCopyDefaultsMove() = default;
+};
Index: test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
@@ -0,0 +1,26 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -- -std=c++03
+
+class DefinesDestructor {
+  ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyConstructor {
+  DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyAssignment {
+  DefinesCopyAssignment =(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor or a copy constructor [cppcoreguidelines-special-member-functions]
+
+class DefinesNothing {
+};
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  DefinesEverything =(const DefinesEverything &);
+  ~DefinesEverything();
+};
+

Re: [PATCH] D22513: [clang-tidy] add check cppcoreguidelines-rule-of-five-and-zero

[Jonathan B Coe via cfe-commits]

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

https://reviews.llvm.org/D22513



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

Re: [PATCH] D22513: [clang-tidy] add check cppcoreguidelines-rule-of-five-and-zero

[Jonathan B Coe via cfe-commits]

jbcoe removed rL LLVM as the repository for this revision.
jbcoe updated this revision to Diff 65423.
jbcoe added a comment.

Fix underline length in docs.


https://reviews.llvm.org/D22513

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
  clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
  test/clang-tidy/cppcoreguidelines-special-member-functions.cpp

Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t
+
+class DefinesDestructor {
+  ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyConstructor {
+  DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyAssignment {
+  DefinesCopyAssignment =(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor, a copy constructor, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveConstructor {
+  DefinesMoveConstructor(DefinesMoveConstructor &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveConstructor' defines a move constructor but does not define a destructor, a copy constructor, a copy assignment operator or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveAssignment {
+  DefinesMoveAssignment =(DefinesMoveAssignment &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveAssignment' defines a move assignment operator but does not define a destructor, a copy constructor, a copy assignment operator or a move constructor [cppcoreguidelines-special-member-functions]
+class DefinesNothing {
+};
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  DefinesEverything =(const DefinesEverything &);
+  DefinesEverything(DefinesEverything &&);
+  DefinesEverything =(DefinesEverything &&);
+  ~DefinesEverything();
+};
+
+class DeletesEverything {
+  DeletesEverything(const DeletesEverything &) = delete;
+  DeletesEverything =(const DeletesEverything &) = delete;
+  DeletesEverything(DeletesEverything &&) = delete;
+  DeletesEverything =(DeletesEverything &&) = delete;
+  ~DeletesEverything() = delete;
+};
+
+class DeletesCopyDefaultsMove {
+  DeletesCopyDefaultsMove(const DeletesCopyDefaultsMove &) = delete;
+  DeletesCopyDefaultsMove =(const DeletesCopyDefaultsMove &) = delete;
+  DeletesCopyDefaultsMove(DeletesCopyDefaultsMove &&) = default;
+  DeletesCopyDefaultsMove =(DeletesCopyDefaultsMove &&) = default;
+  ~DeletesCopyDefaultsMove() = default;
+};
Index: test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
@@ -0,0 +1,26 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -- -std=c++03
+
+class DefinesDestructor {
+  ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyConstructor {
+  DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyAssignment {
+  DefinesCopyAssignment =(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor or a copy constructor [cppcoreguidelines-special-member-functions]
+
+class DefinesNothing {
+};
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  

Re: [PATCH] D22513: [clang-tidy] add check cppcoreguidelines-rule-of-five-and-zero

[Jonathan B Coe via cfe-commits]

jbcoe set the repository for this revision to rL LLVM.
jbcoe updated this revision to Diff 65414.
jbcoe marked an inline comment as done.
jbcoe added a comment.

Rename to cppcoreguidelines-special-member-functions to avoid the duplication 
required with naming it rule-of-3 and rule-of-5.

Reduce code duplication by iterating over a list of matchers.

Use dense map. [FIXME: I can't get DenseMap to work with the DenseMapInfo 
defined in the header. IdentifierNamingCheck.cpp does this without any issues 
but I hit compiler errors around DenseMap and SourceLocation. Moving the 
specialisation from the header to the source of SpecialMemberFunctionsCheck 
suffices to repeat the problem. Any input here would be appreciated.]


Repository:
  rL LLVM

https://reviews.llvm.org/D22513

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
  clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
  test/clang-tidy/cppcoreguidelines-special-member-functions.cpp

Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t
+
+class DefinesDestructor {
+  ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyConstructor {
+  DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyAssignment {
+  DefinesCopyAssignment =(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor, a copy constructor, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveConstructor {
+  DefinesMoveConstructor(DefinesMoveConstructor &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveConstructor' defines a move constructor but does not define a destructor, a copy constructor, a copy assignment operator or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveAssignment {
+  DefinesMoveAssignment =(DefinesMoveAssignment &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveAssignment' defines a move assignment operator but does not define a destructor, a copy constructor, a copy assignment operator or a move constructor [cppcoreguidelines-special-member-functions]
+class DefinesNothing {
+};
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  DefinesEverything =(const DefinesEverything &);
+  DefinesEverything(DefinesEverything &&);
+  DefinesEverything =(DefinesEverything &&);
+  ~DefinesEverything();
+};
+
+class DeletesEverything {
+  DeletesEverything(const DeletesEverything &) = delete;
+  DeletesEverything =(const DeletesEverything &) = delete;
+  DeletesEverything(DeletesEverything &&) = delete;
+  DeletesEverything =(DeletesEverything &&) = delete;
+  ~DeletesEverything() = delete;
+};
+
+class DeletesCopyDefaultsMove {
+  DeletesCopyDefaultsMove(const DeletesCopyDefaultsMove &) = delete;
+  DeletesCopyDefaultsMove =(const DeletesCopyDefaultsMove &) = delete;
+  DeletesCopyDefaultsMove(DeletesCopyDefaultsMove &&) = default;
+  DeletesCopyDefaultsMove =(DeletesCopyDefaultsMove &&) = default;
+  ~DeletesCopyDefaultsMove() = default;
+};
Index: test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
@@ -0,0 +1,26 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -- -std=c++03
+
+class DefinesDestructor {
+  ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyConstructor {
+  DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: 

Re: [PATCH] D22513: [clang-tidy] add check cppcoreguidelines-rule-of-five-and-zero

[Jonathan B Coe via cfe-commits]

jbcoe updated this revision to Diff 65096.
jbcoe marked 6 inline comments as done.
jbcoe added a comment.

Address comments from review, thanks for those.


https://reviews.llvm.org/D22513

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp
  clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.h
  docs/clang-tidy/checks/cppcoreguidelines-rule-of-five-and-zero.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero-cxx-03.cpp
  test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero.cpp

Index: test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero.cpp
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-rule-of-five-and-zero %t
+
+class DefinesDestructor {
+  ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-rule-of-five-and-zero] 
+
+class DefinesCopyConstructor {
+  DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesCopyAssignment {
+  DefinesCopyAssignment =(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor, a copy constructor, a move constructor or a move assignment operator [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesMoveConstructor {
+  DefinesMoveConstructor(DefinesMoveConstructor &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveConstructor' defines a move constructor but does not define a destructor, a copy constructor, a copy assignment operator or a move assignment operator [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesMoveAssignment {
+  DefinesMoveAssignment =(DefinesMoveAssignment &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveAssignment' defines a move assignment operator but does not define a destructor, a copy constructor, a copy assignment operator or a move constructor [cppcoreguidelines-rule-of-five-and-zero]
+class DefinesNothing {
+};
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  DefinesEverything =(const DefinesEverything &);
+  DefinesEverything(DefinesEverything &&);
+  DefinesEverything =(DefinesEverything &&);
+  ~DefinesEverything();
+};
+
+class DeletesEverything {
+  DeletesEverything(const DeletesEverything &) = delete;
+  DeletesEverything =(const DeletesEverything &) = delete;
+  DeletesEverything(DeletesEverything &&) = delete;
+  DeletesEverything =(DeletesEverything &&) = delete;
+  ~DeletesEverything() = delete;
+};
+
+class DeletesCopyDefaultsMove {
+  DeletesCopyDefaultsMove(const DeletesCopyDefaultsMove &) = delete;
+  DeletesCopyDefaultsMove =(const DeletesCopyDefaultsMove &) = delete;
+  DeletesCopyDefaultsMove(DeletesCopyDefaultsMove &&) = default;
+  DeletesCopyDefaultsMove =(DeletesCopyDefaultsMove &&) = default;
+  ~DeletesCopyDefaultsMove() = default;
+};
Index: test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero-cxx-03.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero-cxx-03.cpp
@@ -0,0 +1,26 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-rule-of-five-and-zero %t -- -- -std=c++03
+
+class DefinesDestructor {
+  ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-rule-of-five-and-zero] 
+
+class DefinesCopyConstructor {
+  DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor or a copy assignment operator [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesCopyAssignment {
+  DefinesCopyAssignment =(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor or a copy constructor [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesNothing {
+};
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  DefinesEverything =(const DefinesEverything &);
+  ~DefinesEverything();
+};
+
Index: 

Re: [PATCH] D22513: [clang-tidy] add check cppcoreguidelines-rule-of-five-and-zero

[Jonathan B Coe via cfe-commits]

jbcoe removed rL LLVM as the repository for this revision.
jbcoe updated this revision to Diff 65092.
jbcoe marked 2 inline comments as done.
jbcoe added a comment.

Addressed review comments, thanks for those, check is nicer now.


https://reviews.llvm.org/D22513

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp
  clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.h
  docs/clang-tidy/checks/cppcoreguidelines-rule-of-five-and-zero.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero-cxx-03.cpp
  test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero.cpp

Index: test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero.cpp
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-rule-of-five-and-zero %t
+
+class DefinesDestructor {
+  ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-rule-of-five-and-zero] 
+
+class DefinesCopyConstructor {
+  DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesCopyAssignment {
+  DefinesCopyAssignment =(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor, a copy constructor, a move constructor or a move assignment operator [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesMoveConstructor {
+  DefinesMoveConstructor(DefinesMoveConstructor &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveConstructor' defines a move constructor but does not define a destructor, a copy constructor, a copy assignment operator or a move assignment operator [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesMoveAssignment {
+  DefinesMoveAssignment =(DefinesMoveAssignment &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveAssignment' defines a move assignment operator but does not define a destructor, a copy constructor, a copy assignment operator or a move constructor [cppcoreguidelines-rule-of-five-and-zero]
+class DefinesNothing {
+};
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  DefinesEverything =(const DefinesEverything &);
+  DefinesEverything(DefinesEverything &&);
+  DefinesEverything =(DefinesEverything &&);
+  ~DefinesEverything();
+};
+
+class DeletesEverything {
+  DeletesEverything(const DeletesEverything &);
+  DeletesEverything =(const DeletesEverything &);
+  DeletesEverything(DeletesEverything &&);
+  DeletesEverything =(DeletesEverything &&);
+  ~DeletesEverything();
+};
+
+class DeletesCopyDefaultsMove {
+  DeletesCopyDefaultsMove(const DeletesCopyDefaultsMove &) = delete;
+  DeletesCopyDefaultsMove =(const DeletesCopyDefaultsMove &) = delete;
+  DeletesCopyDefaultsMove(DeletesCopyDefaultsMove &&) = default;
+  DeletesCopyDefaultsMove =(DeletesCopyDefaultsMove &&) = default;
+  ~DeletesCopyDefaultsMove() = default;
+};
Index: test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero-cxx-03.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero-cxx-03.cpp
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-rule-of-five-and-zero %t -- -- -std=c++03
+
+class DefinesDestructor {
+  ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-rule-of-five-and-zero] 
+
+class DefinesCopyConstructor {
+  DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor or a copy assignment operator [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesCopyAssignment {
+  DefinesCopyAssignment =(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor or a copy constructor [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesNothing {
+};
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  DefinesEverything =(const DefinesEverything &);
+  

Re: [PATCH] D16376: clang-tidy check: cppcoreguidelines-rule-of-five-and-zero

[Jonathan B Coe via cfe-commits]

jbcoe retitled this revision from "clang-tidy check: 
misc-deprecated-special-member-functions" to "clang-tidy check: 
cppcoreguidelines-rule-of-five-and-zero".
jbcoe updated the summary for this revision.
jbcoe updated this revision to Diff 64490.
jbcoe added a comment.
Herald added a subscriber: nemanjai.

I've rewritten this patch to implement a rule-of-five-and-zero check.

No fixes are offered as we've not found them to be useful.


https://reviews.llvm.org/D16376

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp
  clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.h
  docs/clang-tidy/checks/cppcoreguidelines-rule-of-five-and-zero.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero.cpp

Index: test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-rule-of-five-and-zero %t
+
+class DefinesDestructor {
+  ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesCopyConstructor {
+  DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+class DefinesCopyAssignment {
+  DefinesCopyAssignment =(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesMoveConstructor {
+  DefinesMoveConstructor(DefinesMoveConstructor &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveConstructor' defines a move constructor but does not define or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesMoveAssignment {
+  DefinesMoveAssignment =(DefinesMoveAssignment &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveAssignment' defines a move assignment operator but does not define or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+
+class DefinesNothing {
+};
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  DefinesEverything =(const DefinesEverything &);
+  DefinesEverything(DefinesEverything &&);
+  DefinesEverything =(DefinesEverything &&);
+  ~DefinesEverything();
+};
+
+class DeletesEverything {
+  DeletesEverything(const DeletesEverything &);
+  DeletesEverything =(const DeletesEverything &);
+  DeletesEverything(DeletesEverything &&);
+  DeletesEverything =(DeletesEverything &&);
+  ~DeletesEverything();
+};
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -29,6 +29,7 @@
cppcoreguidelines-pro-type-static-cast-downcast
cppcoreguidelines-pro-type-union-access
cppcoreguidelines-pro-type-vararg
+   cppcoreguidelines-rule-of-five-and-zero
google-build-explicit-make-pair
google-build-namespaces
google-build-using-namespace
Index: docs/clang-tidy/checks/cppcoreguidelines-rule-of-five-and-zero.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-rule-of-five-and-zero.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - cppcoreguidelines-rule-of-five-and-zero
+
+cppcoreguidelines-rule-of-five-and-zero
+===
+
+The check finds classes where some but not all of the special member functions
+are defined.
+
+By default the compiler defines a copy constructor, copy assignment operator,
+move constructor, move assignment operator and destructor. The default can be
+supressed by explciti user-definitions. The relationship between which
+functions will be supressed by definitions of other functions is complicated
+and it is advised that all five are defaulted or explicitly defined.
+
+Note that defining a function with ``=delete`` is considered to be a
+definition. 
+
+This rule is part of the "Constructors, assignments, and destructors" profile of the C++ Core
+Guidelines, corresponding to rule C.21. See
+
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all.
Index: clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.h

Re: [PATCH] D16962: clang-tidy: avoid std::bind

[Jonathan B Coe via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL269341: [clang-tidy] Adds modernize-avoid-bind check 
(authored by jbcoe).

Changed prior to commit:
  http://reviews.llvm.org/D16962?vs=57067=57095#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D16962

Files:
  clang-tools-extra/trunk/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/trunk/clang-tidy/modernize/AvoidBindCheck.h
  clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-avoid-bind.rst
  clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-bind.cpp

Index: clang-tools-extra/trunk/clang-tidy/modernize/AvoidBindCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/modernize/AvoidBindCheck.h
+++ clang-tools-extra/trunk/clang-tidy/modernize/AvoidBindCheck.h
@@ -0,0 +1,36 @@
+//===--- AvoidBindCheck.h - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_AVOID_BIND_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_AVOID_BIND_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace simple uses of std::bind with a lambda.
+///
+/// FIXME: Add support for function references and member function references.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-avoid-std-bind.html
+class AvoidBindCheck : public ClangTidyCheck {
+public:
+  AvoidBindCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_AVOID_BIND_H
Index: clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyModernizeModule
+  AvoidBindCheck.cpp
   DeprecatedHeadersCheck.cpp
   LoopConvertCheck.cpp
   LoopConvertUtils.cpp
Index: clang-tools-extra/trunk/clang-tidy/modernize/AvoidBindCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/AvoidBindCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/AvoidBindCheck.cpp
@@ -0,0 +1,163 @@
+//===--- AvoidBindCheck.cpp - clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "AvoidBindCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include 
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+namespace {
+enum BindArgumentKind { BK_Temporary, BK_Placeholder, BK_CallExpr, BK_Other };
+
+struct BindArgument {
+  StringRef Tokens;
+  BindArgumentKind Kind = BK_Other;
+  size_t PlaceHolderIndex = 0;
+};
+
+} // end namespace
+
+static SmallVector
+buildBindArguments(const MatchFinder::MatchResult , const CallExpr *C) {
+  SmallVector BindArguments;
+  llvm::Regex MatchPlaceholder("^_([0-9]+)$");
+
+  // Start at index 1 as first argument to bind is the function name.
+  for (size_t I = 1, ArgCount = C->getNumArgs(); I < ArgCount; ++I) {
+const Expr *E = C->getArg(I);
+BindArgument B;
+if (const auto *M = dyn_cast(E)) {
+  const auto *TE = M->GetTemporaryExpr();
+  B.Kind = isa(TE) ? BK_CallExpr : BK_Temporary;
+}
+
+B.Tokens = Lexer::getSourceText(
+CharSourceRange::getTokenRange(E->getLocStart(), E->getLocEnd()),
+*Result.SourceManager, Result.Context->getLangOpts());
+
+SmallVector Matches;
+if (B.Kind == BK_Other && MatchPlaceholder.match(B.Tokens, )) {
+  B.Kind = BK_Placeholder;
+  B.PlaceHolderIndex = std::stoi(Matches[1]);
+}
+BindArguments.push_back(B);
+  }
+  return BindArguments;
+}
+
+static void addPlaceholderArgs(const ArrayRef 

Re: [PATCH] D16962: clang-tidy: avoid std::bind

[Jonathan B Coe via cfe-commits]

jbcoe updated this revision to Diff 57067.
jbcoe added a comment.

update diff to allow arc to apply patch.


http://reviews.llvm.org/D16962

Files:
  clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tidy/modernize/AvoidBindCheck.h
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-avoid-bind.rst
  test/clang-tidy/modernize-avoid-bind.cpp

Index: test/clang-tidy/modernize-avoid-bind.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-avoid-bind.cpp
@@ -0,0 +1,70 @@
+// RUN: %check_clang_tidy %s modernize-avoid-bind %t -- -- -std=c++14
+
+namespace std {
+inline namespace impl {
+template 
+class bind_rt {};
+
+template 
+bind_rt bind(Fp &&, Arguments &&...);
+}
+}
+
+int add(int x, int y) { return x + y; }
+
+void f() {
+  auto clj = std::bind(add, 2, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+  // CHECK-FIXES: auto clj = [] { return add(2, 2); };
+}
+
+void g() {
+  int x = 2;
+  int y = 2;
+  auto clj = std::bind(add, x, y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind 
+  // CHECK-FIXES: auto clj = [=] { return add(x, y); };
+}
+
+struct placeholder {};
+placeholder _1;
+placeholder _2;
+
+void h() {
+  int x = 2;
+  auto clj = std::bind(add, x, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind 
+  // CHECK-FIXES: auto clj = [=](auto && arg1) { return add(x, arg1); };
+}
+
+struct A;
+struct B;
+bool ABTest(const A &, const B &);
+
+void i() {
+  auto BATest = std::bind(ABTest, _2, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: prefer a lambda to std::bind 
+  // CHECK-FIXES: auto BATest = [](auto && arg1, auto && arg2) { return ABTest(arg2, arg1); };
+}
+
+void j() {
+  auto clj = std::bind(add, 2, 2, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind 
+  // No fix is applied for argument mismatches.
+  // CHECK-FIXES: auto clj = std::bind(add, 2, 2, 2);
+}
+
+void k() {
+  auto clj = std::bind(add, _1, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind 
+  // No fix is applied for reused placeholders.
+  // CHECK-FIXES: auto clj = std::bind(add, _1, _1);
+}
+
+void m() {
+  auto clj = std::bind(add, 1, add(2, 5));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind 
+  // No fix is applied for nested calls.
+  // CHECK-FIXES: auto clj = std::bind(add, 1, add(2, 5));
+}
+
Index: docs/clang-tidy/checks/modernize-avoid-bind.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-avoid-bind.rst
@@ -0,0 +1,38 @@
+.. title:: clang-tidy - modernize-avoid-std-bind
+
+modernize-avoid-bind
+==
+
+The check finds uses of ``std::bind`` and replaces simple uses with lambdas.
+Lambdas will use value-capture where required.
+
+Right now it only handles free functions, not member functions.
+
+Given:
+
+.. code:: C++
+
+  int add(int x, int y) { return x + y; }
+
+Then:
+
+.. code:: C++
+  
+  void f() {
+int x = 2;
+auto clj = std::bind(add, x, _1);
+  }
+
+is replaced by:
+  
+.. code:: C++
+  
+  void f() {
+int x = 2;
+auto clj = [=](auto && arg1) { return add(x, arg1); };
+  }
+
+``std::bind`` can be hard to read and can result in larger object files and
+binaries due to type information that will not be produced by equivalent
+lambdas.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -91,6 +91,7 @@
misc-unused-raii
misc-unused-using-decls
misc-virtual-near-miss
+   modernize-avoid-bind
modernize-deprecated-headers
modernize-loop-convert
modernize-make-shared
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===
--- clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "AvoidBindCheck.h"
 #include "DeprecatedHeadersCheck.h"
 #include "LoopConvertCheck.h"
 #include "MakeSharedCheck.h"
@@ -34,6 +35,8 @@
 class ModernizeModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck(
+"modernize-avoid-bind");
 CheckFactories.registerCheck(
 "modernize-deprecated-headers");
 CheckFactories.registerCheck("modernize-loop-convert");
Index: clang-tidy/modernize/CMakeLists.txt
===
--- clang-tidy/modernize/CMakeLists.txt
+++ clang-tidy/modernize/CMakeLists.txt
@@ -1,6 +1,7 

Re: [PATCH] D16962: clang-tidy: avoid std::bind

[Jonathan B Coe via cfe-commits]

jbcoe added a comment.

I can submit the patch myself, thanks.

Thanks again for taking the time to give such a thorough review.


http://reviews.llvm.org/D16962



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

Re: [PATCH] D16962: clang-tidy: avoid std::bind

[Jonathan B Coe via cfe-commits]

jbcoe updated this revision to Diff 57022.
jbcoe marked 6 inline comments as done.
jbcoe added a comment.

Apply fixes from review.


http://reviews.llvm.org/D16962

Files:
  clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tidy/modernize/AvoidBindCheck.h
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-avoid-bind.rst
  test/clang-tidy/modernize-avoid-bind.cpp

Index: test/clang-tidy/modernize-avoid-bind.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-avoid-bind.cpp
@@ -0,0 +1,70 @@
+// RUN: %check_clang_tidy %s modernize-avoid-bind %t -- -- -std=c++14
+
+namespace std {
+inline namespace impl {
+template 
+class bind_rt {};
+
+template 
+bind_rt bind(Fp &&, Arguments &&...);
+}
+}
+
+int add(int x, int y) { return x + y; }
+
+void f() {
+  auto clj = std::bind(add, 2, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+  // CHECK-FIXES: auto clj = [] { return add(2, 2); };
+}
+
+void g() {
+  int x = 2;
+  int y = 2;
+  auto clj = std::bind(add, x, y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind 
+  // CHECK-FIXES: auto clj = [=] { return add(x, y); };
+}
+
+struct placeholder {};
+placeholder _1;
+placeholder _2;
+
+void h() {
+  int x = 2;
+  auto clj = std::bind(add, x, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind 
+  // CHECK-FIXES: auto clj = [=](auto && arg1) { return add(x, arg1); };
+}
+
+struct A;
+struct B;
+bool ABTest(const A &, const B &);
+
+void i() {
+  auto BATest = std::bind(ABTest, _2, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: prefer a lambda to std::bind 
+  // CHECK-FIXES: auto BATest = [](auto && arg1, auto && arg2) { return ABTest(arg2, arg1); };
+}
+
+void j() {
+  auto clj = std::bind(add, 2, 2, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind 
+  // No fix is applied for argument mismatches.
+  // CHECK-FIXES: auto clj = std::bind(add, 2, 2, 2);
+}
+
+void k() {
+  auto clj = std::bind(add, _1, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind 
+  // No fix is applied for reused placeholders.
+  // CHECK-FIXES: auto clj = std::bind(add, _1, _1);
+}
+
+void m() {
+  auto clj = std::bind(add, 1, add(2, 5));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind 
+  // No fix is applied for nested calls.
+  // CHECK-FIXES: auto clj = std::bind(add, 1, add(2, 5));
+}
+
Index: docs/clang-tidy/checks/modernize-avoid-bind.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-avoid-bind.rst
@@ -0,0 +1,38 @@
+.. title:: clang-tidy - modernize-avoid-std-bind
+
+modernize-avoid-bind
+==
+
+The check finds uses of ``std::bind`` and replaces simple uses with lambdas.
+Lambdas will use value-capture where required.
+
+Right now it only handles free functions, not member functions.
+
+Given:
+
+.. code:: C++
+
+  int add(int x, int y) { return x + y; }
+
+Then:
+
+.. code:: C++
+  
+  void f() {
+int x = 2;
+auto clj = std::bind(add, x, _1);
+  }
+
+is replaced by:
+  
+.. code:: C++
+  
+  void f() {
+int x = 2;
+auto clj = [=](auto && arg1) { return add(x, arg1); };
+  }
+
+``std::bind`` can be hard to read and can result in larger object files and
+binaries due to type information that will not be produced by equivalent
+lambdas.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -87,6 +87,7 @@
misc-unused-raii
misc-unused-using-decls
misc-virtual-near-miss
+   modernize-avoid-bind
modernize-deprecated-headers
modernize-loop-convert
modernize-make-unique
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===
--- clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "AvoidBindCheck.h"
 #include "DeprecatedHeadersCheck.h"
 #include "LoopConvertCheck.h"
 #include "MakeUniqueCheck.h"
@@ -32,6 +33,8 @@
 class ModernizeModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck(
+"modernize-avoid-bind");
 CheckFactories.registerCheck(
 "modernize-deprecated-headers");
 CheckFactories.registerCheck("modernize-loop-convert");
Index: clang-tidy/modernize/CMakeLists.txt
===
--- clang-tidy/modernize/CMakeLists.txt
+++ 

Re: [PATCH] D16962: clang-tidy: avoid std::bind

[Jonathan B Coe via cfe-commits]

jbcoe removed a reviewer: djasper.
jbcoe updated this revision to Diff 56273.
jbcoe added a comment.

Move to modernize. Rename to `avoid-bind` as a later patch will add support for 
`boost::bind`, `std::bind1st` etc.


http://reviews.llvm.org/D16962

Files:
  clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tidy/modernize/AvoidBindCheck.h
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-avoid-bind.rst
  test/clang-tidy/modernize-avoid-bind.cpp

Index: test/clang-tidy/modernize-avoid-bind.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-avoid-bind.cpp
@@ -0,0 +1,70 @@
+// RUN: %check_clang_tidy %s modernize-avoid-bind %t -- -- -std=c++14
+
+namespace std {
+inline namespace impl {
+template 
+class bind_rt {};
+
+template 
+bind_rt bind(Fp &&, Arguments &&...);
+}
+}
+
+int add(int x, int y) { return x + y; }
+
+void f() {
+  auto clj = std::bind(add, 2, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+  // CHECK-FIXES: auto clj = [] { return add(2, 2); };
+}
+
+void g() {
+  int x = 2;
+  int y = 2;
+  auto clj = std::bind(add, x, y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+  // CHECK-FIXES: auto clj = [=] { return add(x, y); };
+}
+
+struct placeholder {};
+placeholder _1;
+placeholder _2;
+
+void h() {
+  int x = 2;
+  auto clj = std::bind(add, x, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+  // CHECK-FIXES: auto clj = [=](auto && arg1) { return add(x, arg1); };
+}
+
+struct A;
+struct B;
+bool ABTest(const A &, const B &);
+
+void i() {
+  auto BATest = std::bind(ABTest, _2, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+  // CHECK-FIXES: auto BATest = [](auto && arg1, auto && arg2) { return ABTest(arg2, arg1); };
+}
+
+void j() {
+  auto clj = std::bind(add, 2, 2, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+  // No fix is applied for argument mismatches.
+  // CHECK-FIXES: auto clj = std::bind(add, 2, 2, 2);
+}
+
+void k() {
+  auto clj = std::bind(add, _1, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+  // No fix is applied for reused placeholders.
+  // CHECK-FIXES: auto clj = std::bind(add, _1, _1);
+}
+
+void m() {
+  auto clj = std::bind(add, 1, add(2, 5));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+  // No fix is applied for nested calls.
+  // CHECK-FIXES: auto clj = std::bind(add, 1, add(2, 5));
+}
+
Index: docs/clang-tidy/checks/modernize-avoid-bind.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-avoid-bind.rst
@@ -0,0 +1,35 @@
+.. title:: clang-tidy - modernize-avoid-std-bind
+
+modernize-avoid-bind
+==
+
+The check finds uses of ``std::bind`` and replaces simple uses with lambdas.
+Lambdas will use value-capture where required.
+
+Right now it only handles free functions, not member functions.
+
+Given:
+
+.. code:: C++
+  int add(int x, int y) { return x + y; }
+
+Then:
+
+.. code:: C++
+  void f() {
+int x = 2;
+auto clj = std::bind(add, x, _1);
+  }
+
+is replaced by:
+  
+.. code:: C++
+  void f() {
+int x = 2;
+auto clj = [=](auto && arg1) { return add(x, arg1); };
+  }
+
+We created this check because ``std::bind`` can be hard to read and can result
+in larger object files and binaries due to type information that will not be
+produced by equivalent lambdas.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -87,6 +87,7 @@
misc-unused-raii
misc-unused-using-decls
misc-virtual-near-miss
+   modernize-avoid-bind
modernize-deprecated-headers
modernize-loop-convert
modernize-make-unique
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===
--- clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "AvoidBindCheck.h"
 #include "DeprecatedHeadersCheck.h"
 #include "LoopConvertCheck.h"
 #include "MakeUniqueCheck.h"
@@ -32,6 +33,8 @@
 class ModernizeModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck(
+"modernize-avoid-bind");
 CheckFactories.registerCheck(
 "modernize-deprecated-headers");
 

Re: [PATCH] D16962: clang-tidy: avoid std::bind

[Jonathan B Coe via cfe-commits]

jbcoe added inline comments.


Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:17
@@ -15,2 +16,2 @@
 #include "ContainerSizeEmptyCheck.h"
 #include "DeletedDefaultCheck.h"

aaron.ballman wrote:
> I believe we use "modernize" to really mean "migrate from the old way to the 
> new way", which this definitely fits into since I think the point to this 
> check is to replace bind with better alternatives.
Would you prefer it to be in `modernize`? I can be easily convinced either way 
and am happy to move it. If I do move it I might add a script to facilitate 
doing so.


http://reviews.llvm.org/D16962



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

Re: [PATCH] D16376: clang-tidy check: misc-deprecated-special-member-functions

[Jonathan B Coe via cfe-commits]

jbcoe added a comment.

After some pondering I think I **will**extend  move this check to 
cppcoreguidelines and call it rule-of-five.

https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all


http://reviews.llvm.org/D16376



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

Re: [PATCH] D16962: clang-tidy: avoid std::bind

[Jonathan B Coe via cfe-commits]

jbcoe updated this revision to Diff 55970.
jbcoe marked 16 inline comments as done.
jbcoe added a comment.

Minor fixes from review.


http://reviews.llvm.org/D16962

Files:
  clang-tidy/readability/AvoidStdBindCheck.cpp
  clang-tidy/readability/AvoidStdBindCheck.h
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-avoid-std-bind.rst
  test/clang-tidy/readability-avoid-std-bind.cpp

Index: test/clang-tidy/readability-avoid-std-bind.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-avoid-std-bind.cpp
@@ -0,0 +1,70 @@
+// RUN: %check_clang_tidy %s readability-avoid-std-bind %t -- -- -std=c++14
+
+namespace std {
+inline namespace impl {
+template 
+class bind_rt {};
+
+template 
+bind_rt bind(Fp &&, Arguments &&...);
+}
+}
+
+int add(int x, int y) { return x + y; }
+
+void f() {
+  auto clj = std::bind(add, 2, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [readability-avoid-std-bind]
+  // CHECK-FIXES: auto clj = [] { return add(2, 2); };
+}
+
+void g() {
+  int x = 2;
+  int y = 2;
+  auto clj = std::bind(add, x, y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [readability-avoid-std-bind]
+  // CHECK-FIXES: auto clj = [=] { return add(x, y); };
+}
+
+struct placeholder {};
+placeholder _1;
+placeholder _2;
+
+void h() {
+  int x = 2;
+  auto clj = std::bind(add, x, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [readability-avoid-std-bind]
+  // CHECK-FIXES: auto clj = [=](auto && arg1) { return add(x, arg1); };
+}
+
+struct A;
+struct B;
+bool ABTest(const A &, const B &);
+
+void i() {
+  auto BATest = std::bind(ABTest, _2, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: prefer a lambda to std::bind [readability-avoid-std-bind]
+  // CHECK-FIXES: auto BATest = [](auto && arg1, auto && arg2) { return ABTest(arg2, arg1); };
+}
+
+void j() {
+  auto clj = std::bind(add, 2, 2, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [readability-avoid-std-bind]
+  // No fix is applied for argument mismatches.
+  // CHECK-FIXES: auto clj = std::bind(add, 2, 2, 2);
+}
+
+void k() {
+  auto clj = std::bind(add, _1, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [readability-avoid-std-bind]
+  // No fix is applied for reused placeholders.
+  // CHECK-FIXES: auto clj = std::bind(add, _1, _1);
+}
+
+void m() {
+  auto clj = std::bind(add, 1, add(2, 5));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [readability-avoid-std-bind]
+  // No fix is applied for nested calls.
+  // CHECK-FIXES: auto clj = std::bind(add, 1, add(2, 5));
+}
+
Index: docs/clang-tidy/checks/readability-avoid-std-bind.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-avoid-std-bind.rst
@@ -0,0 +1,35 @@
+.. title:: clang-tidy - readability-avoid-std-bind
+
+readability-avoid-std-bind
+==
+
+The check finds uses of ``std::bind`` and replaces simple uses with lambdas.
+Lambdas will use value-capture where required.
+
+Right now it only handles free functions, not member functions.
+
+Given:
+
+.. code:: C++
+  int add(int x, int y) { return x + y; }
+
+Then:
+
+.. code:: C++
+  void f() {
+int x = 2;
+auto clj = std::bind(add, x, _1);
+  }
+
+is replaced by:
+  
+.. code:: C++
+  void f() {
+int x = 2;
+auto clj = [=](auto && arg1) { return add(x, arg1); };
+  }
+
+We created this check because ``std::bind`` can be hard to read and can result
+in larger object files and binaries due to type information that will not be
+produced by equivalent lambdas.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -105,6 +105,7 @@
performance-unnecessary-copy-initialization
performance-unnecessary-value-param
readability-avoid-const-params-in-decls
+   readability-avoid-std-bind
readability-braces-around-statements
readability-container-size-empty
readability-deleted-default
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -11,6 +11,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "AvoidConstParamsInDecls.h"
+#include "AvoidStdBindCheck.h"
 #include "BracesAroundStatementsCheck.h"
 #include "ContainerSizeEmptyCheck.h"
 #include "DeletedDefaultCheck.h"
@@ -37,6 +38,8 @@
   void addCheckFactories(ClangTidyCheckFactories ) override {
 CheckFactories.registerCheck(
 

Re: [PATCH] D15654: A Python code model for C++ used to drive code generators

[Jonathan B Coe via cfe-commits]

jbcoe abandoned this revision.
jbcoe added a comment.

I'm abandoning this revision for lack of interest.


http://reviews.llvm.org/D15654



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

Re: [PATCH] D15469: Expose cxx constructor and method properties through libclang and python bindings.

[Jonathan B Coe via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL267706: Expose cxx constructor and method properties through 
libclang and python… (authored by jbcoe).

Changed prior to commit:
  http://reviews.llvm.org/D15469?vs=53519=55196#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D15469

Files:
  cfe/trunk/bindings/python/clang/cindex.py
  cfe/trunk/bindings/python/tests/cindex/test_cursor.py
  cfe/trunk/include/clang-c/Index.h
  cfe/trunk/test/Index/availability.cpp
  cfe/trunk/test/Index/file-refs.cpp
  cfe/trunk/test/Index/get-cursor.cpp
  cfe/trunk/test/Index/index-file.cpp
  cfe/trunk/test/Index/load-classes.cpp
  cfe/trunk/test/Index/print-type.cpp
  cfe/trunk/test/Index/recursive-cxx-member-calls.cpp
  cfe/trunk/test/Parser/skip-function-bodies.mm
  cfe/trunk/tools/c-index-test/c-index-test.c
  cfe/trunk/tools/libclang/CIndex.cpp
  cfe/trunk/tools/libclang/libclang.exports

Index: cfe/trunk/tools/libclang/CIndex.cpp
===
--- cfe/trunk/tools/libclang/CIndex.cpp
+++ cfe/trunk/tools/libclang/CIndex.cpp
@@ -7360,6 +7360,48 @@
 //===--===//
 
 extern "C" {
+
+unsigned clang_CXXConstructor_isDefaultConstructor(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXConstructorDecl *Constructor =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  return (Constructor && Constructor->isDefaultConstructor()) ? 1 : 0;
+}
+
+unsigned clang_CXXConstructor_isCopyConstructor(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXConstructorDecl *Constructor =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  return (Constructor && Constructor->isCopyConstructor()) ? 1 : 0;
+}
+
+unsigned clang_CXXConstructor_isMoveConstructor(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXConstructorDecl *Constructor =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  return (Constructor && Constructor->isMoveConstructor()) ? 1 : 0;
+}
+
+unsigned clang_CXXConstructor_isConvertingConstructor(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXConstructorDecl *Constructor =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  // Passing 'false' excludes constructors marked 'explicit'.
+  return (Constructor && Constructor->isConvertingConstructor(false)) ? 1 : 0;
+}
+
 unsigned clang_CXXField_isMutable(CXCursor C) {
   if (!clang_isDeclaration(C.kind))
 return 0;
@@ -7390,6 +7432,16 @@
   return (Method && (Method->getTypeQualifiers() & Qualifiers::Const)) ? 1 : 0;
 }
 
+unsigned clang_CXXMethod_isDefaulted(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXMethodDecl *Method =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  return (Method && Method->isDefaulted()) ? 1 : 0;
+}
+
 unsigned clang_CXXMethod_isStatic(CXCursor C) {
   if (!clang_isDeclaration(C.kind))
 return 0;
Index: cfe/trunk/tools/libclang/libclang.exports
===
--- cfe/trunk/tools/libclang/libclang.exports
+++ cfe/trunk/tools/libclang/libclang.exports
@@ -2,7 +2,12 @@
 clang_CXCursorSet_insert
 clang_CXIndex_getGlobalOptions
 clang_CXIndex_setGlobalOptions
+clang_CXXConstructor_isConvertingConstructor
+clang_CXXConstructor_isCopyConstructor
+clang_CXXConstructor_isDefaultConstructor
+clang_CXXConstructor_isMoveConstructor
 clang_CXXField_isMutable
+clang_CXXMethod_isDefaulted
 clang_CXXMethod_isConst
 clang_CXXMethod_isPureVirtual
 clang_CXXMethod_isStatic
Index: cfe/trunk/tools/c-index-test/c-index-test.c
===
--- cfe/trunk/tools/c-index-test/c-index-test.c
+++ cfe/trunk/tools/c-index-test/c-index-test.c
@@ -772,9 +772,20 @@
 
 clang_disposeString(DeprecatedMessage);
 clang_disposeString(UnavailableMessage);
-
+
+if (clang_CXXConstructor_isDefaultConstructor(Cursor))
+  printf(" (default constructor)");
+
+if (clang_CXXConstructor_isMoveConstructor(Cursor))
+  printf(" (move constructor)");
+if (clang_CXXConstructor_isCopyConstructor(Cursor))
+  printf(" (copy constructor)");
+if (clang_CXXConstructor_isConvertingConstructor(Cursor))
+  printf(" (converting constructor)");
 if (clang_CXXField_isMutable(Cursor))
   printf(" (mutable)");
+if (clang_CXXMethod_isDefaulted(Cursor))
+  printf(" (defaulted)");
 if (clang_CXXMethod_isStatic(Cursor))
   printf(" (static)");
 if (clang_CXXMethod_isVirtual(Cursor))
Index: cfe/trunk/bindings/python/clang/cindex.py

Re: [PATCH] D15469: Expose cxx constructor and method properties through libclang and python bindings.

[Jonathan B Coe via cfe-commits]

jbcoe updated this revision to Diff 53519.
jbcoe added a comment.

Remove python test for deleted method.

I missed this because the python tests are not run as part of check-clang - is 
this something I could submit a patch to change?


http://reviews.llvm.org/D15469

Files:
  bindings/python/clang/cindex.py
  bindings/python/tests/cindex/test_cursor.py
  include/clang-c/Index.h
  test/Index/availability.cpp
  test/Index/file-refs.cpp
  test/Index/get-cursor.cpp
  test/Index/index-file.cpp
  test/Index/load-classes.cpp
  test/Index/print-type.cpp
  test/Index/recursive-cxx-member-calls.cpp
  test/Parser/skip-function-bodies.mm
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp
  tools/libclang/libclang.exports

Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -2,7 +2,12 @@
 clang_CXCursorSet_insert
 clang_CXIndex_getGlobalOptions
 clang_CXIndex_setGlobalOptions
+clang_CXXConstructor_isConvertingConstructor
+clang_CXXConstructor_isCopyConstructor
+clang_CXXConstructor_isDefaultConstructor
+clang_CXXConstructor_isMoveConstructor
 clang_CXXField_isMutable
+clang_CXXMethod_isDefaulted
 clang_CXXMethod_isConst
 clang_CXXMethod_isPureVirtual
 clang_CXXMethod_isStatic
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7379,6 +7379,48 @@
 //===--===//
 
 extern "C" {
+
+unsigned clang_CXXConstructor_isDefaultConstructor(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXConstructorDecl *Constructor =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  return (Constructor && Constructor->isDefaultConstructor()) ? 1 : 0;
+}
+
+unsigned clang_CXXConstructor_isCopyConstructor(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXConstructorDecl *Constructor =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  return (Constructor && Constructor->isCopyConstructor()) ? 1 : 0;
+}
+
+unsigned clang_CXXConstructor_isMoveConstructor(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXConstructorDecl *Constructor =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  return (Constructor && Constructor->isMoveConstructor()) ? 1 : 0;
+}
+
+unsigned clang_CXXConstructor_isConvertingConstructor(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXConstructorDecl *Constructor =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  // Passing 'false' excludes constructors marked 'explicit'.
+  return (Constructor && Constructor->isConvertingConstructor(false)) ? 1 : 0;
+}
+
 unsigned clang_CXXField_isMutable(CXCursor C) {
   if (!clang_isDeclaration(C.kind))
 return 0;
@@ -7409,6 +7451,16 @@
   return (Method && (Method->getTypeQualifiers() & Qualifiers::Const)) ? 1 : 0;
 }
 
+unsigned clang_CXXMethod_isDefaulted(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXMethodDecl *Method =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  return (Method && Method->isDefaulted()) ? 1 : 0;
+}
+
 unsigned clang_CXXMethod_isStatic(CXCursor C) {
   if (!clang_isDeclaration(C.kind))
 return 0;
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -772,9 +772,20 @@
 
 clang_disposeString(DeprecatedMessage);
 clang_disposeString(UnavailableMessage);
-
+
+if (clang_CXXConstructor_isDefaultConstructor(Cursor))
+  printf(" (default constructor)");
+
+if (clang_CXXConstructor_isMoveConstructor(Cursor))
+  printf(" (move constructor)");
+if (clang_CXXConstructor_isCopyConstructor(Cursor))
+  printf(" (copy constructor)");
+if (clang_CXXConstructor_isConvertingConstructor(Cursor))
+  printf(" (converting constructor)");
 if (clang_CXXField_isMutable(Cursor))
   printf(" (mutable)");
+if (clang_CXXMethod_isDefaulted(Cursor))
+  printf(" (defaulted)");
 if (clang_CXXMethod_isStatic(Cursor))
   printf(" (static)");
 if (clang_CXXMethod_isVirtual(Cursor))
Index: test/Parser/skip-function-bodies.mm
===
--- test/Parser/skip-function-bodies.mm
+++ test/Parser/skip-function-bodies.mm
@@ -30,7 +30,7 @@
 // CHECK: skip-function-bodies.mm:3:7: ClassDecl=A:3:7 (Definition) Extent=[3:1 - 14:2]
 // CHECK: skip-function-bodies.mm:4:9: 

Re: [PATCH] D15469: Expose cxx constructor and method properties through libclang and python bindings.

[Jonathan B Coe via cfe-commits]

jbcoe updated this revision to Diff 53316.
jbcoe added a comment.
This revision is now accepted and ready to land.

Remove `isDeleted` as it needs to be added for non-member functions.

I will add `isDeleted` as an isolated follow up patch.


http://reviews.llvm.org/D15469

Files:
  bindings/python/clang/cindex.py
  bindings/python/tests/cindex/test_cursor.py
  include/clang-c/Index.h
  test/Index/availability.cpp
  test/Index/file-refs.cpp
  test/Index/get-cursor.cpp
  test/Index/index-file.cpp
  test/Index/load-classes.cpp
  test/Index/print-type.cpp
  test/Index/recursive-cxx-member-calls.cpp
  test/Parser/skip-function-bodies.mm
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp
  tools/libclang/libclang.exports

Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -2,7 +2,12 @@
 clang_CXCursorSet_insert
 clang_CXIndex_getGlobalOptions
 clang_CXIndex_setGlobalOptions
+clang_CXXConstructor_isConvertingConstructor
+clang_CXXConstructor_isCopyConstructor
+clang_CXXConstructor_isDefaultConstructor
+clang_CXXConstructor_isMoveConstructor
 clang_CXXField_isMutable
+clang_CXXMethod_isDefaulted
 clang_CXXMethod_isConst
 clang_CXXMethod_isPureVirtual
 clang_CXXMethod_isStatic
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7387,6 +7387,48 @@
 //===--===//
 
 extern "C" {
+
+unsigned clang_CXXConstructor_isDefaultConstructor(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXConstructorDecl *Constructor =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  return (Constructor && Constructor->isDefaultConstructor()) ? 1 : 0;
+}
+
+unsigned clang_CXXConstructor_isCopyConstructor(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXConstructorDecl *Constructor =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  return (Constructor && Constructor->isCopyConstructor()) ? 1 : 0;
+}
+
+unsigned clang_CXXConstructor_isMoveConstructor(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXConstructorDecl *Constructor =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  return (Constructor && Constructor->isMoveConstructor()) ? 1 : 0;
+}
+
+unsigned clang_CXXConstructor_isConvertingConstructor(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXConstructorDecl *Constructor =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  // Passing 'false' excludes constructors marked 'explicit'.
+  return (Constructor && Constructor->isConvertingConstructor(false)) ? 1 : 0;
+}
+
 unsigned clang_CXXField_isMutable(CXCursor C) {
   if (!clang_isDeclaration(C.kind))
 return 0;
@@ -7417,6 +7459,16 @@
   return (Method && (Method->getTypeQualifiers() & Qualifiers::Const)) ? 1 : 0;
 }
 
+unsigned clang_CXXMethod_isDefaulted(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXMethodDecl *Method =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  return (Method && Method->isDefaulted()) ? 1 : 0;
+}
+
 unsigned clang_CXXMethod_isStatic(CXCursor C) {
   if (!clang_isDeclaration(C.kind))
 return 0;
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -772,9 +772,20 @@
 
 clang_disposeString(DeprecatedMessage);
 clang_disposeString(UnavailableMessage);
-
+
+if (clang_CXXConstructor_isDefaultConstructor(Cursor))
+  printf(" (default constructor)");
+
+if (clang_CXXConstructor_isMoveConstructor(Cursor))
+  printf(" (move constructor)");
+if (clang_CXXConstructor_isCopyConstructor(Cursor))
+  printf(" (copy constructor)");
+if (clang_CXXConstructor_isConvertingConstructor(Cursor))
+  printf(" (converting constructor)");
 if (clang_CXXField_isMutable(Cursor))
   printf(" (mutable)");
+if (clang_CXXMethod_isDefaulted(Cursor))
+  printf(" (defaulted)");
 if (clang_CXXMethod_isStatic(Cursor))
   printf(" (static)");
 if (clang_CXXMethod_isVirtual(Cursor))
Index: test/Parser/skip-function-bodies.mm
===
--- test/Parser/skip-function-bodies.mm
+++ test/Parser/skip-function-bodies.mm
@@ -30,7 +30,7 @@
 // CHECK: skip-function-bodies.mm:3:7: ClassDecl=A:3:7 (Definition) Extent=[3:1 - 14:2]
 // CHECK: skip-function-bodies.mm:4:9: 

Re: [PATCH] D15469: Expose cxx constructor and method properties through libclang and python bindings.

[Jonathan B Coe via cfe-commits]

jbcoe added a comment.

I plan to strip out the `isDeleted` methods as the associated tests now fail 
(which I need to raise a bug for) and need reworking to apply to non-member 
functions.

I'll submit a new patch for `isDeleted` on its own. The rest of this patch 
seems uncontentious.

I hope to find time this week to make the changes.


http://reviews.llvm.org/D15469



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

Re: [PATCH] D17180: Fix failing python bindings test

[Jonathan B Coe via cfe-commits]

jbcoe added a comment.

The other python test fix has been applied in r263170 (Differential Revision: 
http://reviews.llvm.org/D17226).

Has anyone had a chance to look at _this_ patch yet?


Repository:
  rL LLVM

http://reviews.llvm.org/D17180



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

Re: [PATCH] D17811: [clang-tidy] Add check to detect dangling references in value handlers.

[Jonathan B Coe via cfe-commits]

jbcoe added a comment.

Do you have commit access? I can apply this patch for you if not.


http://reviews.llvm.org/D17811



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

Re: [PATCH] D15469: Expose cxx constructor and method properties through libclang and python bindings.

[Jonathan B Coe via cfe-commits]

jbcoe added a comment.

If I remove `= delete` from the declaration of the member function `foo` in 
test/Index/index-file.cpp line 36 then `foo` is reported in the output as

  [indexDeclaration]: kind: c++-instance-method | name: foo | USR: 
c:@S@B@F@foo# | lang: C++ | cursor: CXXMethod=foo:36:8 | loc: 36:8 | 
semantic-container: [B:27:7] | lexical-container: [B:27:7] | isRedecl: 0 | 
isDef: 0 | isContainer: 0 | isImplicit: 0

with `=delete` in place, `foo` is not reported at all.

This is a behaviour change since I wrote this patch. I'm not sure what the 
correct behaviour should be. I can update the test and not check for the 
deleted function `foo` when we are confident that behaviour is correct.

The changed behaviour (not reporting deleted functions) is not part of my patch.


http://reviews.llvm.org/D15469



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

Re: [PATCH] D16962: clang-tidy: avoid std::bind

[Jonathan B Coe via cfe-commits]

jbcoe added inline comments.


Comment at: clang-tidy/readability/AvoidStdBindCheck.cpp:42
@@ +41,3 @@
+
+  for (size_t I = 1, ArgCount = C->getNumArgs(); I < ArgCount; ++I) {
+const Expr *E = C->getArg(I);

alexfh wrote:
> Please use a range-based for loop over `C->arguments()`.
I'm starting at 1 not zero, hence the explicit loop.

Elements are non-contiguous so I can't use a restricted ArrayView.

I've added a comment saying why it starts at 1, not 0.


Comment at: clang-tidy/readability/AvoidStdBindCheck.cpp:127
@@ +126,3 @@
+  auto DiagnosticBuilder =
+  diag(MatchedDecl->getLocStart(), "avoid using std::bind");
+

alexfh wrote:
> alexfh wrote:
> > Should the message recommend something instead?
> In pre-C++11 code the check will just warn without suggesting any 
> alternative. That will lead to a lot of user confusion. We either need to 
> restrict the warning to C++14 code or suggest a better alternative even in 
> pre-C++14 code.
The message now recommends using a lambda so I think this is addressed.


http://reviews.llvm.org/D16962



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

Re: [PATCH] D16962: clang-tidy: avoid std::bind

[Jonathan B Coe via cfe-commits]

jbcoe updated this revision to Diff 50623.
jbcoe marked 15 inline comments as done.
jbcoe added a comment.

Apply requested fixes from review.

Thanks for taking time to review this, the patch is much improved for the 
attention.


http://reviews.llvm.org/D16962

Files:
  clang-tidy/readability/AvoidStdBindCheck.cpp
  clang-tidy/readability/AvoidStdBindCheck.h
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-avoid-std-bind.rst
  test/clang-tidy/readability-avoid-std-bind.cpp

Index: test/clang-tidy/readability-avoid-std-bind.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-avoid-std-bind.cpp
@@ -0,0 +1,81 @@
+// RUN: %check_clang_tidy %s readability-avoid-std-bind %t -- -- -std=c++14
+
+namespace std {
+inline namespace impl {
+template 
+class bind_rt {};
+
+template 
+bind_rt bind(Fp&&, Arguments&& ...);
+}
+}
+
+int add(int x, int y) { return x + y; }
+
+void f()
+{
+  auto clj = std::bind(add,2,2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [readability-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto clj = [] { return add(2, 2); };
+
+void g()
+{
+  int x = 2;
+  int y = 2;
+  auto clj = std::bind(add,x,y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [readability-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto clj = [=] { return add(x, y); };
+
+struct placeholder {};
+placeholder _1;
+placeholder _2;
+
+void h()
+{
+  int x = 2;
+  auto clj = std::bind(add,x,_1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [readability-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto clj = [=](auto && arg1) { return add(x, arg1); };
+
+struct A;
+struct B;
+bool ABTest(const A&, const B&);
+
+void i()
+{
+  auto BATest = std::bind(ABTest, _2, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: prefer a lambda to std::bind [readability-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto BATest = [](auto && arg1, auto && arg2) { return ABTest(arg2, arg1); };
+
+void j()
+{
+  auto clj = std::bind(add, 2, 2, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [readability-avoid-std-bind]
+}
+// No fix is applied for argument mismatches.
+// CHECK-FIXES: auto clj = std::bind(add, 2, 2, 2);
+
+void k()
+{
+  auto clj = std::bind(add, _1, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [readability-avoid-std-bind]
+}
+// No fix is applied for reused placeholders.
+// CHECK-FIXES: auto clj = std::bind(add, _1, _1);
+
+void m()
+{
+  auto clj = std::bind(add, 1, add(2, 5));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [readability-avoid-std-bind]
+}
+// No fix is applied for nested calls.
+// CHECK-FIXES: auto clj = std::bind(add, 1, add(2, 5));
+
Index: docs/clang-tidy/checks/readability-avoid-std-bind.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-avoid-std-bind.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - readability-avoid-std-bind
+
+readability-avoid-std-bind
+==
+
+Find uses of ``std::bind``. Replace simple uses of ``std::bind`` with lambdas.
+Lambdas will use value-capture where required.
+
+Right now it only handles free functions not member functions.
+
+Fixits are only generated for simple uses of ``std::bind``.
+
+``std::bind`` can be hard to read and can result in larger object files and
+binaries due to type information that will not be produced by equivalent
+lambdas.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -88,6 +88,7 @@
performance-faster-string-find
performance-for-range-copy
performance-implicit-cast-in-loop
+   readability-avoid-std-bind
readability-braces-around-statements
readability-container-size-empty
readability-else-after-return
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "AvoidStdBindCheck.h"
 #include "BracesAroundStatementsCheck.h"
 #include "ContainerSizeEmptyCheck.h"
 #include "ElseAfterReturnCheck.h"
@@ -32,6 +33,8 @@
 class ReadabilityModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck(
+"readability-avoid-std-bind");
 CheckFactories.registerCheck(
 "readability-braces-around-statements");
 CheckFactories.registerCheck(
Index: 

Re: [PATCH] D16376: clang-tidy check: misc-deprecated-special-member-functions

[Jonathan B Coe via cfe-commits]

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

I'll move this to `modernize` and update docs when I get over my cold. Thanks 
for the feedback.


http://reviews.llvm.org/D16376



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

Re: [PATCH] D15469: Expose cxx constructor and method properties through libclang and python bindings.

[Jonathan B Coe via cfe-commits]

jbcoe added a comment.

I don't know if it's at all helpful but I get the following output from 
llvm-lit on the failing test file. There's no mention of a function `foo` at 
all:

  [startedTranslationUnit]
  [enteredMainFile]: 
/Users/jon/DEV/llvm-git-svn/tools/clang/test/Index/index-file.cpp
  [indexDeclaration]: kind: type-alias | name: MyTypeAlias | USR: 
c:@MyTypeAlias | lang: C++ | cursor: TypeAliasDecl=MyTypeAlias:1:7 (Definition) 
| loc: 1:7 | semantic-container: [TU]
  [indexDeclaration]: kind: function-template | name: Allocate | USR: 
c:@FT@>1#TAllocate | lang: C++ | cursor: FunctionDecl=Allocate:4:28 
(Definition) | loc: 4:28 | semantic-container
  [indexDeclaration]: kind: namespace | name: rdar14063074 | USR: 
c:@N@rdar14063074 | lang: C++ | cursor: Namespace=rdar14063074:8:11 
(Definition) | loc: 8:11 | semantic-container: [T
  [indexDeclaration]: kind: c++-class-template | name: TS | USR: 
c:@N@rdar14063074@ST>1#T@TS | lang: C++ | cursor: StructDecl=TS:10:8 
(Definition) | loc: 10:8 | semantic-container: [r
  [indexDeclaration]: kind: struct-template-spec | name: TS | USR: 
c:@N@rdar14063074@S@TS>#I | lang: C++ | cursor: StructDecl=TS:11:8 (Definition) 
[Specialization of TS:10:8] | loc: 1
  [indexDeclaration]: kind: function-template | name: tfoo | USR: 
c:@N@rdar14063074@FT@>1#Ttfoo#v# | lang: C++ | cursor: FunctionDecl=tfoo:14:6 
(Definition) | loc: 14:6 | semantic-con
  [indexDeclaration]: kind: function-template-spec | name: tfoo | USR: 
c:@N@rdar14063074@F@tfoo<#I># | lang: C++ | cursor: FunctionDecl=tfoo:15:6 
(Definition) [Specialization of tfoo:
  [indexDeclaration]: kind: namespace | name: crash1 | USR: c:@N@crash1 | lang: 
C++ | cursor: Namespace=crash1:18:11 (Definition) | loc: 18:11 | 
semantic-container: [TU] | lexical-con
  [indexDeclaration]: kind: c++-class-template | name: A | USR: 
c:@N@crash1@ST>1#T@A | lang: C++ | cursor: ClassDecl=A:19:28 (Definition) | 
loc: 19:28 | semantic-container: [crash1:18
  [indexDeclaration]: kind: c++-instance-method | name: meth | USR: 
c:@N@crash1@ST>1#T@A@F@meth# | lang: C++ | cursor: CXXMethod=meth:21:8 | loc: 
21:8 | semantic-container: [A:19:28] 
  [indexDeclaration]: kind: c++-instance-method | name: meth | USR: 
c:@N@crash1@S@A>#I@F@meth# | lang: C++ | cursor: CXXMethod=meth:23:26 
[Specialization of meth:21:8] | loc: 23:26 | 
  [indexEntityReference]: kind: c++-class-template | name: A | USR: 
c:@N@crash1@ST>1#T@A | lang: C++ | cursor: TemplateRef=A:19:28 | loc: 23:18 | 
:: kind: c++-instance-method 
  [indexDeclaration]: kind: c++-class | name: B | USR: c:@S@B | lang: C++ | 
cursor: ClassDecl=B:27:7 (Definition) | loc: 27:7 | semantic-container: [TU] | 
lexical-container: [TU] | is
  [indexDeclaration]: kind: field | name: x_ | USR: c:@S@B@FI@x_ | lang: C++ | 
cursor: FieldDecl=x_:28:15 (Definition) (mutable) | loc: 28:15 | 
semantic-container: [B:27:7] | lexical-
  [indexDeclaration]: kind: field | name: y_ | USR: c:@S@B@FI@y_ | lang: C++ | 
cursor: FieldDecl=y_:29:7 (Definition) | loc: 29:7 | semantic-container: 
[B:27:7] | lexical-container: [
  [indexDeclaration]: kind: constructor | name: B | USR: c:@S@B@F@B# | lang: 
C++ | cursor: CXXConstructor=B:31:3 (default constructor) (defaulted) | loc: 
31:3 | semantic-container: [B
  [indexDeclaration]: kind: constructor | name: B | USR: c:@S@B@F@B#I# | lang: 
C++ | cursor: CXXConstructor=B:32:3 (converting constructor) | loc: 32:3 | 
semantic-container: [B:27:7] 
  [indexDeclaration]: kind: constructor | name: B | USR: c:@S@B@F@B#d# | lang: 
C++ | cursor: CXXConstructor=B:33:12 | loc: 33:12 | semantic-container: 
[B:27:7] | lexical-container: [B
  [indexDeclaration]: kind: constructor | name: B | USR: c:@S@B@F@B#&1$@S@B# | 
lang: C++ | cursor: CXXConstructor=B:34:3 (copy constructor) (converting 
constructor) | loc: 34:3 | sema
  [indexEntityReference]: kind: c++-class | name: B | USR: c:@S@B | lang: C++ | 
cursor: TypeRef=class B:27:7 | loc: 34:11 | :: kind: constructor | 
name: B | USR: c:@S@B@F@B#&1
  [indexDeclaration]: kind: constructor | name: B | USR: c:@S@B@F@B#&&$@S@B# | 
lang: C++ | cursor: CXXConstructor=B:35:3 (move constructor) (converting 
constructor) | loc: 35:3 | sema
  [indexEntityReference]: kind: c++-class | name: B | USR: c:@S@B | lang: C++ | 
cursor: TypeRef=class B:27:7 | loc: 35:5 | :: kind: constructor | name: 
B | USR: c:@S@B@F@B#&&$
  [indexDeclaration]: kind: c++-class | name: C | USR: c:@S@C | lang: C++ | 
cursor: ClassDecl=C:39:7 (Definition) | loc: 39:7 | semantic-container: [TU] | 
lexical-container: [TU] | is
  [indexDeclaration]: kind: constructor | name: C | USR: c:@S@C@F@C#&1$@S@C# | 
lang: C++ | cursor: CXXConstructor=C:40:12 (copy constructor) | loc: 40:12 | 
semantic-container: [C:39:7
  [indexEntityReference]: kind: c++-class | name: C | USR: c:@S@C | lang: C++ | 
cursor: TypeRef=class C:39:7 | loc: 40:20 | :: kind: constructor | 
name: C | USR: c:@S@C@F@C#&1
  [diagnostic]: 

Re: [PATCH] D15469: Expose cxx constructor and method properties through libclang and python bindings.

[Jonathan B Coe via cfe-commits]

jbcoe added a comment.

@skalinichev I get the same failure as you. This patch is old now, all tests 
used to pass. Can you see what is wrong with the test logic?


http://reviews.llvm.org/D15469



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

Re: [PATCH] D17811: [clang-tidy] Add check to detect dangling references in value handlers.

[Jonathan B Coe via cfe-commits]

jbcoe added inline comments.


Comment at: test/clang-tidy/misc-dangling-handle.cpp:86
@@ +85,3 @@
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: std::basic_string_view outlives
+
+  view1 = std::string();

Thanks.


http://reviews.llvm.org/D17811



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

Re: [PATCH] D17226: libclang python bindings: Fix for bug 26394

[Jonathan B Coe via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL263170: libclang python bindings: Fix for bug 26394 
(authored by jbcoe).

Changed prior to commit:
  http://reviews.llvm.org/D17226?vs=47874=50369#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D17226

Files:
  cfe/trunk/bindings/python/clang/cindex.py

Index: cfe/trunk/bindings/python/clang/cindex.py
===
--- cfe/trunk/bindings/python/clang/cindex.py
+++ cfe/trunk/bindings/python/clang/cindex.py
@@ -2383,7 +2383,7 @@
 functions above. __init__ is only called internally.
 """
 assert isinstance(index, Index)
-
+self.index = index
 ClangObject.__init__(self, ptr)
 
 def __del__(self):


Index: cfe/trunk/bindings/python/clang/cindex.py
===
--- cfe/trunk/bindings/python/clang/cindex.py
+++ cfe/trunk/bindings/python/clang/cindex.py
@@ -2383,7 +2383,7 @@
 functions above. __init__ is only called internally.
 """
 assert isinstance(index, Index)
-
+self.index = index
 ClangObject.__init__(self, ptr)
 
 def __del__(self):
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17811: [clang-tidy] Add check to detect dangling references in value handlers.

[Jonathan B Coe via cfe-commits]

jbcoe added a comment.

I think there are some minor issues with the tests, easily fixed. I wonder if 
the matchers can catch things inside implementation-defined inline namespaces 
like `std::_v1_::experimental::library_fundamentals_v1::string_view`.



Comment at: clang-tidy/misc/DanglingHandleCheck.cpp:57
@@ +56,3 @@
+ast_matchers::internal::Matcher isASequence() {
+  return hasAnyName("::std::deque", "::std::forward_list", "::std::list",
+"::std::vector");

Will this (and similar checkers) handle inline namespaces?


Comment at: test/clang-tidy/misc-dangling-handle.cpp:73
@@ +72,3 @@
+  StringRef(const char*);  // NOLINT
+  StringRef(const std::string&);  // NOLINT
+};

What does `NOLINT` do here?


Comment at: test/clang-tidy/misc-dangling-handle.cpp:85
@@ +84,3 @@
+  std::string_view view_2 = ReturnsAString();
+  // CHECK-MESSAGES: [[@LINE-1]]:29: warning: std::basic_string_view outlives
+

This (and other) check-messages line looks truncated.


http://reviews.llvm.org/D17811



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

Re: [PATCH] D17772: [clang-tidy]: string_view of temporary string

[Jonathan B Coe via cfe-commits]

jbcoe abandoned this revision.
jbcoe added a comment.

rendered obsolete by http://reviews.llvm.org/D17811


Repository:
  rL LLVM

http://reviews.llvm.org/D17772



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

Re: [PATCH] D15654: A Python code model for C++ used to drive code generators

[Jonathan B Coe via cfe-commits]

jbcoe added a comment.

Has anyone had a chance to look over this?


http://reviews.llvm.org/D15654



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

[PATCH] D17772: [clang-tidy]: string_view of temporary string

[Jonathan B Coe via cfe-commits]

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

Find instances where a string_view is constructed from a temporary string and 
replace the string_view with a string.

Repository:
  rL LLVM

http://reviews.llvm.org/D17772

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/StringViewOfTemporaryCheck.cpp
  clang-tidy/misc/StringViewOfTemporaryCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-string_view-of-temporary.rst
  test/clang-tidy/misc-string_view-of-temporary.cpp

Index: test/clang-tidy/misc-string_view-of-temporary.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-string_view-of-temporary.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s misc-string_view-of-temporary %t
+
+namespace std {
+inline namespace implementation {
+class string {
+public:
+  string(const char *);
+};
+
+class string_view {
+public:
+  string_view(const string &);
+  string_view(const char*);
+};
+}
+}
+
+using std::string;
+using std::string_view;
+
+string f();
+void g(string_view);
+void h()
+{
+  string_view sv(f());
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: string_view 'sv' is constructed from a temporary string [misc-string_view-of-temporary]
+// CHECK-FIXES: std::string sv(f());
+  g(f());
+  auto s = f();
+  string_view sv2(s);
+  string_view sv3("char[] construction");
+}
+
Index: docs/clang-tidy/checks/misc-string_view-of-temporary.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-string_view-of-temporary.rst
@@ -0,0 +1,25 @@
+.. title:: clang-tidy - misc-string_view-of-temporary
+
+misc-string_view-of-temporary
+=
+
+Find non-temporary ``string_view``s constructed from temporary strings and
+replace them with ``std::string``s.
+
+The following construction of a ``string_view`` constructs a view of a
+temporary object: 
+
+.. code:: c++
+
+  std::string f();
+
+  std::string_view s(f());
+
+The fix replaces the ``std::string_view`` with an ``std::string``:
+
+.. code:: c++
+
+  std::string f();
+
+  std::string s(f());
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -65,6 +65,7 @@
misc-sizeof-container
misc-static-assert
misc-string-integer-assignment
+   misc-string_view-of-temporary
misc-suspicious-semicolon
misc-swapped-arguments
misc-throw-by-value-catch-by-reference
Index: clang-tidy/misc/StringViewOfTemporaryCheck.h
===
--- /dev/null
+++ clang-tidy/misc/StringViewOfTemporaryCheck.h
@@ -0,0 +1,35 @@
+//===--- String_viewOfTemporaryCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_VIEW_OF_TEMPORARY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_VIEW_OF_TEMPORARY_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Find non-temporary string_views constructed from temporary strings.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-string_view-of-temporary.html
+class String_viewOfTemporaryCheck : public ClangTidyCheck {
+public:
+  String_viewOfTemporaryCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_VIEW_OF_TEMPORARY_H
Index: clang-tidy/misc/StringViewOfTemporaryCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/StringViewOfTemporaryCheck.cpp
@@ -0,0 +1,55 @@
+//===--- StringViewOfTemporaryCheck.cpp - clang-tidy--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "StringViewOfTemporaryCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+namespace {
+

Re: [PATCH] D16376: clang-tidy check: misc-deprecated-special-member-functions

[Jonathan B Coe via cfe-commits]

jbcoe added a comment.

The Sema diagnostic warning is only produced if a deprecated special member 
function is used whereas I want to find places where it would be 
compiler-generated and explicitly delete them. This is useful for library code 
where I don't have control over the warnings my users will run with.

The AST Matcher I use are simple enough and I'm not convinced that it's worth 
refactoring Sema to expose what I need. If someone (Richard?) with a deeper 
understanding can point me in the right direction I'm happy to be corrected.


http://reviews.llvm.org/D16376



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

Re: [PATCH] D16376: clang-tidy check: misc-deprecated-special-member-functions

[Jonathan B Coe via cfe-commits]

jbcoe added a comment.

It's more than the warning because it offers fixits. Other than that it should 
be the same. Using the same code as used to warn on deprecated special members 
would be a great idea. I'm not too sure where to start looking and how much of 
Sema is exposed to clang-tidy though.


http://reviews.llvm.org/D16376



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

Re: [PATCH] D16376: clang-tidy check: misc-deprecated-special-member-functions

[Jonathan B Coe via cfe-commits]

jbcoe marked 2 inline comments as done.
jbcoe added a comment.

http://reviews.llvm.org/D16376



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

Re: [PATCH] D15654: A Python code model for C++ used to drive code generators

[Jonathan B Coe via cfe-commits]

jbcoe updated this revision to Diff 47745.
jbcoe added a comment.

I've added more python classes to handle types and free functions.


http://reviews.llvm.org/D15654

Files:
  bindings/python/clang/cppmodel.py
  bindings/python/tests/cppmodel/__init__.py
  bindings/python/tests/cppmodel/test_classes.py
  bindings/python/tests/cppmodel/test_free_functions.py

Index: bindings/python/tests/cppmodel/test_free_functions.py
===
--- /dev/null
+++ bindings/python/tests/cppmodel/test_free_functions.py
@@ -0,0 +1,55 @@
+from ..cindex.util import get_tu
+from clang import cppmodel
+from clang.cindex import TypeKind
+
+def test_function_name():
+source = """
+void foo();
+void bar();
+"""
+tu = get_tu(source, 'cpp')
+
+model = cppmodel.Model(tu)
+functions = model.functions
+
+assert len(functions) == 2
+assert functions[0].name == 'foo'
+assert functions[1].name == 'bar'
+
+
+def test_function_return_type():
+source = """
+int foo();
+double* bar();
+"""
+
+tu = get_tu(source, 'cpp')
+
+model = cppmodel.Model(tu)
+functions = model.functions
+
+assert functions[0].return_type.kind == TypeKind.INT
+assert functions[1].return_type.kind == TypeKind.POINTER
+assert functions[1].return_type.is_pointer
+assert functions[1].return_type.pointee.kind == TypeKind.DOUBLE
+assert not functions[1].return_type.pointee.is_const
+
+
+def test_function_arguments():
+source = """
+int foo();
+double bar(int x, char y);
+"""
+
+tu = get_tu(source, 'cpp')
+
+model = cppmodel.Model(tu)
+functions = model.functions
+
+assert len(functions[0].arguments) == 0
+assert len(functions[1].arguments) == 2
+assert functions[1].arguments[0].type.kind == TypeKind.INT
+assert functions[1].arguments[0].name == 'x'
+assert functions[1].arguments[1].type.kind == TypeKind.CHAR_S
+assert functions[1].arguments[1].name == 'y'
+
Index: bindings/python/tests/cppmodel/test_classes.py
===
--- /dev/null
+++ bindings/python/tests/cppmodel/test_classes.py
@@ -0,0 +1,129 @@
+from ..cindex.util import get_tu
+from clang import cppmodel
+from clang.cindex import TypeKind
+
+def test_class_name():
+source = 'class A{};'
+tu = get_tu(source, 'cpp')
+
+model = cppmodel.Model(tu)
+classes = model.classes
+
+assert len(classes)==1
+assert classes[0].name == 'A'
+
+def test_class_methods():
+source = """
+class A{};
+class B{
+void foo();
+int bar();
+};"""
+tu = get_tu(source, 'cpp')
+
+model = cppmodel.Model(tu)
+classes = model.classes
+
+assert len(classes[0].methods) == 0
+assert len(classes[1].methods) == 2
+
+def test_class_method_return_types():
+source = """
+class B{
+void foo();
+int bar();
+};"""
+tu = get_tu(source, 'cpp')
+
+model = cppmodel.Model(tu)
+classes = model.classes
+
+assert classes[0].methods[0].return_type.kind == TypeKind.VOID
+assert classes[0].methods[1].return_type.kind == TypeKind.INT
+
+def test_class_method_argument_types():
+source = """
+class A {
+int foo(int i, const char* p);
+};"""
+tu = get_tu(source, 'cpp')
+
+model = cppmodel.Model(tu)
+classes = model.classes
+args = classes[0].methods[0].arguments
+
+assert args[0].type.kind == TypeKind.INT
+assert args[0].name == "i"
+assert args[1].type.kind == TypeKind.POINTER
+assert args[1].name == "p"
+
+def test_class_method_const_qualifiers():
+source = """
+class A {
+int foo() const;
+int bar();
+};"""
+tu = get_tu(source, 'cpp')
+
+model = cppmodel.Model(tu)
+classes = model.classes
+methods = classes[0].methods
+
+assert methods[0].is_const
+assert not methods[1].is_const
+
+def test_class_methods_are_virtual():
+source = """
+class A {
+virtual int foo();
+int bar();
+virtual int foobar() = 0;
+};"""
+tu = get_tu(source, 'cpp')
+
+model = cppmodel.Model(tu)
+classes = model.classes
+methods = classes[0].methods
+
+assert methods[0].is_virtual
+assert not methods[0].is_pure_virtual
+assert not methods[1].is_virtual
+assert methods[2].is_pure_virtual
+
+def test_namespaces():
+source = """
+class A{};
+namespace outer {
+class B{};
+namespace inner {
+class C{};
+} // end inner
+class D{};
+} // end outer
+class E{};"""
+tu = get_tu(source, 'cpp')
+
+model = cppmodel.Model(tu)
+classes = model.classes
+
+assert classes[0].namespace == ""
+assert classes[1].namespace == "outer"
+assert classes[2].namespace == "outer::inner"
+assert classes[3].namespace == "outer"
+assert classes[4].namespace == ""
+
+def test_access_specifiers():
+

Re: [PATCH] D15654: A Python code model for C++ used to drive code generators

[Jonathan B Coe via cfe-commits]

jbcoe updated this revision to Diff 47747.
jbcoe added a comment.

Add missing test for types - apologies for noise.


http://reviews.llvm.org/D15654

Files:
  bindings/python/clang/cppmodel.py
  bindings/python/tests/cppmodel/__init__.py
  bindings/python/tests/cppmodel/test_classes.py
  bindings/python/tests/cppmodel/test_free_functions.py
  bindings/python/tests/cppmodel/test_types.py

Index: bindings/python/tests/cppmodel/test_types.py
===
--- /dev/null
+++ bindings/python/tests/cppmodel/test_types.py
@@ -0,0 +1,81 @@
+from ..cindex.util import get_tu
+from clang import cppmodel
+from clang.cindex import TypeKind
+
+def test_pointer_type():
+source = "double* pd();"
+
+tu = get_tu(source, 'cpp')
+model = cppmodel.Model(tu)
+f = model.functions[0]
+
+assert f.return_type.kind == TypeKind.POINTER
+assert not f.return_type.is_const
+assert f.return_type.pointee.kind == TypeKind.DOUBLE
+assert not f.return_type.pointee.is_const
+
+
+def test_const_pointer_to_double_type():
+source = "double* const cpd();"
+
+tu = get_tu(source, 'cpp')
+model = cppmodel.Model(tu)
+f = model.functions[0]
+
+assert f.return_type.kind == TypeKind.POINTER
+assert f.return_type.is_const
+assert f.return_type.pointee.kind == TypeKind.DOUBLE
+assert not f.return_type.pointee.is_const
+
+
+def test_const_pointer_to_const_double_type():
+source = "const double* const cpcd();"
+
+tu = get_tu(source, 'cpp')
+model = cppmodel.Model(tu)
+functions = model.functions
+f = model.functions[0]
+
+assert f.return_type.kind == TypeKind.POINTER
+assert f.return_type.is_const
+assert f.return_type.pointee.kind == TypeKind.DOUBLE
+assert f.return_type.pointee.is_const
+
+
+def test_pointer_to_pointer_type():
+source = "double** ppd();"
+
+tu = get_tu(source, 'cpp')
+model = cppmodel.Model(tu)
+f = model.functions[0]
+
+assert f.return_type.kind == TypeKind.POINTER
+assert f.return_type.is_pointer
+assert f.return_type.pointee.kind == TypeKind.POINTER
+assert f.return_type.pointee.is_pointer
+assert f.return_type.pointee.pointee.kind == TypeKind.DOUBLE
+
+
+def test_pointer_to_record_type():
+source = "class A{}; A* pA();"
+
+tu = get_tu(source, 'cpp')
+model = cppmodel.Model(tu)
+f = model.functions[0]
+
+assert f.return_type.kind == TypeKind.POINTER
+assert f.return_type.is_pointer
+assert f.return_type.pointee.kind == TypeKind.RECORD
+
+def test_reference_to_record_type():
+source = "class A{}; A& pA();"
+
+tu = get_tu(source, 'cpp')
+model = cppmodel.Model(tu)
+f = model.functions[0]
+
+assert f.return_type.kind == TypeKind.LVALUEREFERENCE
+assert not f.return_type.is_pointer
+assert f.return_type.is_reference
+assert f.return_type.pointee.kind == TypeKind.RECORD
+
Index: bindings/python/tests/cppmodel/test_free_functions.py
===
--- /dev/null
+++ bindings/python/tests/cppmodel/test_free_functions.py
@@ -0,0 +1,55 @@
+from ..cindex.util import get_tu
+from clang import cppmodel
+from clang.cindex import TypeKind
+
+def test_function_name():
+source = """
+void foo();
+void bar();
+"""
+tu = get_tu(source, 'cpp')
+
+model = cppmodel.Model(tu)
+functions = model.functions
+
+assert len(functions) == 2
+assert functions[0].name == 'foo'
+assert functions[1].name == 'bar'
+
+
+def test_function_return_type():
+source = """
+int foo();
+double* bar();
+"""
+
+tu = get_tu(source, 'cpp')
+
+model = cppmodel.Model(tu)
+functions = model.functions
+
+assert functions[0].return_type.kind == TypeKind.INT
+assert functions[1].return_type.kind == TypeKind.POINTER
+assert functions[1].return_type.is_pointer
+assert functions[1].return_type.pointee.kind == TypeKind.DOUBLE
+assert not functions[1].return_type.pointee.is_const
+
+
+def test_function_arguments():
+source = """
+int foo();
+double bar(int x, char y);
+"""
+
+tu = get_tu(source, 'cpp')
+
+model = cppmodel.Model(tu)
+functions = model.functions
+
+assert len(functions[0].arguments) == 0
+assert len(functions[1].arguments) == 2
+assert functions[1].arguments[0].type.kind == TypeKind.INT
+assert functions[1].arguments[0].name == 'x'
+assert functions[1].arguments[1].type.kind == TypeKind.CHAR_S
+assert functions[1].arguments[1].name == 'y'
+
Index: bindings/python/tests/cppmodel/test_classes.py
===
--- /dev/null
+++ bindings/python/tests/cppmodel/test_classes.py
@@ -0,0 +1,129 @@
+from ..cindex.util import get_tu
+from clang import cppmodel
+from clang.cindex import TypeKind
+
+def test_class_name():
+source = 'class A{};'
+tu = get_tu(source, 'cpp')
+
+model = 

Re: [PATCH] D16376: clang-tidy check: misc-deprecated-special-member-functions

[Jonathan B Coe via cfe-commits]

jbcoe added a comment.

Tests are now more thorough and more readable.

Insertions are always pre-insertions as getting the correct post-insertion 
position is ambiguous if end-of-line comments exist.


Repository:
  rL LLVM

http://reviews.llvm.org/D16376



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

Re: [PATCH] D16376: clang-tidy check: misc-deprecated-special-member-functions

[Jonathan B Coe via cfe-commits]

jbcoe retitled this revision from "clang-tidy check: rule-of-five" to 
"clang-tidy check: misc-deprecated-special-member-functions".
jbcoe updated the summary for this revision.
jbcoe set the repository for this revision to rL LLVM.
jbcoe updated this revision to Diff 47735.
jbcoe added a comment.

This check now deals only with deprecated special member function generation, 
not move constructors or move assignment operators. I have renamed it and 
modified documentation accordingly.

Code has been updated in line with review comments.


Repository:
  rL LLVM

http://reviews.llvm.org/D16376

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/DeprecatedSpecialMemberGenerationCheck.cpp
  clang-tidy/misc/DeprecatedSpecialMemberGenerationCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-deprecated-special-member-generation.rst
  test/clang-tidy/misc-deprecated-special-member-generation.cpp

Index: test/clang-tidy/misc-deprecated-special-member-generation.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-deprecated-special-member-generation.cpp
@@ -0,0 +1,122 @@
+// RUN: %check_clang_tidy %s misc-deprecated-special-member-generation %t
+
+//
+// User defined copy constructors
+//
+class DeclaresCopyConstructor {
+  DeclaresCopyConstructor(const DeclaresCopyConstructor &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'DeclaresCopyConstructor' defines a copy constructor but not a copy assignment operator [misc-deprecated-special-member-generation]
+};
+
+// CHECK-FIXES: DeclaresCopyConstructor & operator=(const DeclaresCopyConstructor &) = delete;
+
+class DefinesDefaultedCopyConstructor {
+  DefinesDefaultedCopyConstructor(const DefinesDefaultedCopyConstructor &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'DefinesDefaultedCopyConstructor' defines a copy constructor but not a copy assignment operator [misc-deprecated-special-member-generation]
+};
+
+// CHECK-FIXES: DefinesDefaultedCopyConstructor & operator=(const DefinesDefaultedCopyConstructor &) = default;
+
+class DefinesDeletedCopyConstructor {
+  DefinesDeletedCopyConstructor(const DefinesDeletedCopyConstructor &) = delete;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'DefinesDeletedCopyConstructor' defines a copy constructor but not a copy assignment operator [misc-deprecated-special-member-generation]
+};
+
+// CHECK-FIXES: DefinesDeletedCopyConstructor & operator=(const DefinesDeletedCopyConstructor &) = delete;
+
+
+//
+// User defined copy assignment
+//
+class DeclaresCopyAssignment {
+  DeclaresCopyAssignment & operator=(const DeclaresCopyAssignment &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: class 'DeclaresCopyAssignment' defines a copy assignment operator but not a copy constructor [misc-deprecated-special-member-generation]
+};
+
+// CHECK-FIXES: DeclaresCopyAssignment(const DeclaresCopyAssignment &) = delete;
+
+class DefinesDefaultedCopyAssignment {
+  DefinesDefaultedCopyAssignment & operator=(const DefinesDefaultedCopyAssignment &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: class 'DefinesDefaultedCopyAssignment' defines a copy assignment operator but not a copy constructor [misc-deprecated-special-member-generation]
+};
+
+// CHECK-FIXES: DefinesDefaultedCopyAssignment(const DefinesDefaultedCopyAssignment &) = default;
+
+class DefinesDeletedCopyAssignment {
+  DefinesDeletedCopyAssignment & operator=(const DefinesDeletedCopyAssignment &) = delete;
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: class 'DefinesDeletedCopyAssignment' defines a copy assignment operator but not a copy constructor [misc-deprecated-special-member-generation]
+};
+
+// CHECK-FIXES: DefinesDeletedCopyAssignment(const DefinesDeletedCopyAssignment &) = delete;
+
+class DeclaresDestructor {
+  ~DeclaresDestructor();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'DeclaresDestructor' defines a destructor but not a copy constructor or copy assignment operator [misc-deprecated-special-member-generation]
+};
+// CHECK-FIXES: DeclaresDestructor(const DeclaresDestructor &) = delete;
+// CHECK-FIXES: DeclaresDestructor & operator=(const DeclaresDestructor &) = delete;
+
+class DefinesDestructor {
+  ~DefinesDestructor() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'DefinesDestructor' defines a destructor but not a copy constructor or copy assignment operator [misc-deprecated-special-member-generation]
+};
+// CHECK-FIXES: DefinesDestructor(const DefinesDestructor &) = delete;
+// CHECK-FIXES: DefinesDestructor & operator=(const DefinesDestructor &) = delete;
+
+
+class DefinesDefaultedDestructor {
+  ~DefinesDefaultedDestructor() = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'DefinesDefaultedDestructor' defines a destructor but not a copy constructor or copy assignment operator [misc-deprecated-special-member-generation]
+};
+// CHECK-FIXES: 

[PATCH] D17180: Fix failing python bindings test

[Jonathan B Coe via cfe-commits]

jbcoe created this revision.
jbcoe added a reviewer: akyrtzi.
jbcoe added a subscriber: cfe-commits.
jbcoe set the repository for this revision to rL LLVM.

Order of compilation database commands has changed causing test failure. 

Reordering fixes the test `cindex.test_cdb.test_all_compilecommand`.

Other Python bindings tests currently fail. I will address these in a later 
patch.


Repository:
  rL LLVM

http://reviews.llvm.org/D17180

Files:
  bindings/python/tests/cindex/test_cdb.py

Index: bindings/python/tests/cindex/test_cdb.py
===
--- bindings/python/tests/cindex/test_cdb.py
+++ bindings/python/tests/cindex/test_cdb.py
@@ -38,16 +38,17 @@
 cmds = cdb.getAllCompileCommands()
 assert len(cmds) == 3
 expected = [
+{ 'wd': '/home/john.doe/MyProject',
+  'line': ['clang++', '-o', 'project.o', '-c',
+   '/home/john.doe/MyProject/project.cpp']},
 { 'wd': '/home/john.doe/MyProjectA',
   'line': ['clang++', '-o', 'project2.o', '-c',
'/home/john.doe/MyProject/project2.cpp']},
 { 'wd': '/home/john.doe/MyProjectB',
   'line': ['clang++', '-DFEATURE=1', '-o', 'project2-feature.o', '-c',
-   '/home/john.doe/MyProject/project2.cpp']},
-{ 'wd': '/home/john.doe/MyProject',
-  'line': ['clang++', '-o', 'project.o', '-c',
-   '/home/john.doe/MyProject/project.cpp']}
+   '/home/john.doe/MyProject/project2.cpp']}
 ]
+
 for i in range(len(cmds)):
 assert cmds[i].directory == expected[i]['wd']
 for arg, exp in zip(cmds[i].arguments, expected[i]['line']):


Index: bindings/python/tests/cindex/test_cdb.py
===
--- bindings/python/tests/cindex/test_cdb.py
+++ bindings/python/tests/cindex/test_cdb.py
@@ -38,16 +38,17 @@
 cmds = cdb.getAllCompileCommands()
 assert len(cmds) == 3
 expected = [
+{ 'wd': '/home/john.doe/MyProject',
+  'line': ['clang++', '-o', 'project.o', '-c',
+   '/home/john.doe/MyProject/project.cpp']},
 { 'wd': '/home/john.doe/MyProjectA',
   'line': ['clang++', '-o', 'project2.o', '-c',
'/home/john.doe/MyProject/project2.cpp']},
 { 'wd': '/home/john.doe/MyProjectB',
   'line': ['clang++', '-DFEATURE=1', '-o', 'project2-feature.o', '-c',
-   '/home/john.doe/MyProject/project2.cpp']},
-{ 'wd': '/home/john.doe/MyProject',
-  'line': ['clang++', '-o', 'project.o', '-c',
-   '/home/john.doe/MyProject/project.cpp']}
+   '/home/john.doe/MyProject/project2.cpp']}
 ]
+
 for i in range(len(cmds)):
 assert cmds[i].directory == expected[i]['wd']
 for arg, exp in zip(cmds[i].arguments, expected[i]['line']):
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16962: clang-tidy: avoid std::bind

[Jonathan B Coe via cfe-commits]

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

Placeholder handling needs correcting.


Repository:
  rL LLVM

http://reviews.llvm.org/D16962



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

Re: [PATCH] D16962: clang-tidy: avoid std::bind

[Jonathan B Coe via cfe-commits]

jbcoe removed rL LLVM as the repository for this revision.
jbcoe updated this revision to Diff 47247.
jbcoe added a comment.

Require C++14 for improved fixits.

Do not attempt to generate fixits for more complicated uses of bind.


http://reviews.llvm.org/D16962

Files:
  clang-tidy/misc/AvoidStdBindCheck.cpp
  clang-tidy/misc/AvoidStdBindCheck.h
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-avoid-std-bind.rst
  test/clang-tidy/misc-avoid-std-bind.cpp

Index: test/clang-tidy/misc-avoid-std-bind.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-avoid-std-bind.cpp
@@ -0,0 +1,81 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-bind %t -- -- -std=c++14
+
+namespace std {
+inline namespace impl {
+template 
+class bind_rt {};
+
+template 
+bind_rt bind(Fp&&, Arguments&& ...);
+}
+}
+
+int add(int x, int y) { return x + y; }
+
+void f()
+{
+  auto clj = std::bind(add,2,2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto clj = [] { return add(2, 2); };
+
+void g()
+{
+  int x = 2;
+  int y = 2;
+  auto clj = std::bind(add,x,y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto clj = [=] { return add(x, y); };
+
+struct placeholder {};
+placeholder _1;
+placeholder _2;
+
+void h()
+{
+  int x = 2;
+  auto clj = std::bind(add,x,_1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto clj = [=](auto && arg1) { return add(x, arg1); };
+
+struct A;
+struct B;
+bool ABTest(const A&, const B&);
+
+void i()
+{
+  auto BATest = std::bind(ABTest, _2, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto BATest = [](auto && arg1, auto && arg2) { return ABTest(arg2, arg1); };
+
+void j()
+{
+  auto clj = std::bind(add, 2, 2, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+// No fix is applied for argument mismatches.
+// CHECK-FIXES: auto clj = std::bind(add, 2, 2, 2);
+
+void k()
+{
+  auto clj = std::bind(add, _1, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+// No fix is applied for reused placeholders.
+// CHECK-FIXES: auto clj = std::bind(add, _1, _1);
+
+void m()
+{
+  auto clj = std::bind(add, 1, add(2, 5));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+// No fix is applied for nested calls.
+// CHECK-FIXES: auto clj = std::bind(add, 1, add(2, 5));
+
Index: docs/clang-tidy/checks/misc-avoid-std-bind.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-avoid-std-bind.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - misc-avoid-std-bind
+
+misc-avoid-std-bind
+===
+
+Find uses of std::bind. Replace simple uses of std::bind with lambdas. Lambdas
+will use value-capture where required.
+
+Right now it only handles free functions not member functions.
+
+Fixits are only generated for simple uses of std::bind.
+
+std::bind can result in larger object files and binaries due to type
+information that will not be produced by equivalent lambdas.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -46,6 +46,7 @@
misc-argument-comment
misc-assert-side-effect
misc-assign-operator-signature
+   misc-avoid-std-bind
misc-bool-pointer-implicit-conversion
misc-definitions-in-headers
misc-inaccurate-erase
Index: clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -13,6 +13,7 @@
 #include "ArgumentCommentCheck.h"
 #include "AssertSideEffectCheck.h"
 #include "AssignOperatorSignatureCheck.h"
+#include "AvoidStdBindCheck.h"
 #include "BoolPointerImplicitConversionCheck.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "InaccurateEraseCheck.h"
@@ -48,6 +49,8 @@
 "misc-assert-side-effect");
 CheckFactories.registerCheck(
 "misc-assign-operator-signature");
+CheckFactories.registerCheck(
+"misc-avoid-std-bind");
 CheckFactories.registerCheck(
 "misc-bool-pointer-implicit-conversion");
 CheckFactories.registerCheck(
Index: clang-tidy/misc/CMakeLists.txt
===
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -4,6 +4,7 @@
   ArgumentCommentCheck.cpp
   AssertSideEffectCheck.cpp
   AssignOperatorSignatureCheck.cpp
+  AvoidStdBindCheck.cpp
   

Re: [PATCH] D16962: clang-tidy: avoid std::bind

[Jonathan B Coe via cfe-commits]

jbcoe updated this revision to Diff 47259.
jbcoe added a comment.

Moved check to readability module.

Aside: would it be worthwhile creating a python script to move checks from one 
module to another?


http://reviews.llvm.org/D16962

Files:
  clang-tidy/readability/AvoidStdBindCheck.cpp
  clang-tidy/readability/AvoidStdBindCheck.h
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-avoid-std-bind.rst
  test/clang-tidy/readability-avoid-std-bind.cpp

Index: test/clang-tidy/readability-avoid-std-bind.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-avoid-std-bind.cpp
@@ -0,0 +1,81 @@
+// RUN: %check_clang_tidy %s readability-avoid-std-bind %t -- -- -std=c++14
+
+namespace std {
+inline namespace impl {
+template 
+class bind_rt {};
+
+template 
+bind_rt bind(Fp&&, Arguments&& ...);
+}
+}
+
+int add(int x, int y) { return x + y; }
+
+void f()
+{
+  auto clj = std::bind(add,2,2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [readability-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto clj = [] { return add(2, 2); };
+
+void g()
+{
+  int x = 2;
+  int y = 2;
+  auto clj = std::bind(add,x,y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [readability-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto clj = [=] { return add(x, y); };
+
+struct placeholder {};
+placeholder _1;
+placeholder _2;
+
+void h()
+{
+  int x = 2;
+  auto clj = std::bind(add,x,_1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [readability-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto clj = [=](auto && arg1) { return add(x, arg1); };
+
+struct A;
+struct B;
+bool ABTest(const A&, const B&);
+
+void i()
+{
+  auto BATest = std::bind(ABTest, _2, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: avoid using std::bind [readability-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto BATest = [](auto && arg1, auto && arg2) { return ABTest(arg2, arg1); };
+
+void j()
+{
+  auto clj = std::bind(add, 2, 2, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [readability-avoid-std-bind]
+}
+// No fix is applied for argument mismatches.
+// CHECK-FIXES: auto clj = std::bind(add, 2, 2, 2);
+
+void k()
+{
+  auto clj = std::bind(add, _1, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [readability-avoid-std-bind]
+}
+// No fix is applied for reused placeholders.
+// CHECK-FIXES: auto clj = std::bind(add, _1, _1);
+
+void m()
+{
+  auto clj = std::bind(add, 1, add(2, 5));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [readability-avoid-std-bind]
+}
+// No fix is applied for nested calls.
+// CHECK-FIXES: auto clj = std::bind(add, 1, add(2, 5));
+
Index: docs/clang-tidy/checks/readability-avoid-std-bind.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-avoid-std-bind.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - readability-avoid-std-bind
+
+readability-avoid-std-bind
+===
+
+Find uses of std::bind. Replace simple uses of std::bind with lambdas. Lambdas
+will use value-capture where required.
+
+Right now it only handles free functions not member functions.
+
+Fixits are only generated for simple uses of std::bind.
+
+std::bind can be hard to read and can result in larger object files and
+binaries due to type information that will not be produced by equivalent
+lambdas.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -79,6 +79,7 @@
modernize-use-override
performance-for-range-copy
performance-implicit-cast-in-loop
+   readability-avoid-std-bind
readability-braces-around-statements
readability-container-size-empty
readability-else-after-return
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "AvoidStdBindCheck.h"
 #include "BracesAroundStatementsCheck.h"
 #include "ContainerSizeEmptyCheck.h"
 #include "ElseAfterReturnCheck.h"
@@ -31,6 +32,8 @@
 class ReadabilityModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck(
+"readability-avoid-std-bind");
 CheckFactories.registerCheck(
 "readability-braces-around-statements");
 CheckFactories.registerCheck(
Index: clang-tidy/readability/CMakeLists.txt
===
--- 

[PATCH] D16962: clang-tidy: avoid std::bind

[Jonathan B Coe via cfe-commits]

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

Replace std::bind with a lambda.

Not yet working for member functions.

Repository:
  rL LLVM

http://reviews.llvm.org/D16962

Files:
  clang-tidy/misc/AvoidStdBindCheck.cpp
  clang-tidy/misc/AvoidStdBindCheck.h
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-avoid-std-bind.rst
  test/clang-tidy/misc-avoid-std-bind.cpp

Index: test/clang-tidy/misc-avoid-std-bind.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-avoid-std-bind.cpp
@@ -0,0 +1,64 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-bind %t
+
+namespace std {
+inline namespace impl {
+template 
+class bind_rt {};
+
+template 
+bind_rt bind(Fp&&, Arguments&& ...);
+}
+}
+
+int add(int x, int y) { return x + y; }
+
+void f()
+{
+  auto clj = std::bind(add,2,2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto clj = [] { return add(2, 2); };
+
+void g()
+{
+  int x = 2;
+  int y = 2;
+  auto clj = std::bind(add,x,y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto clj = [=] { return add(x, y); };
+
+struct placeholder {};
+placeholder _1;
+placeholder _2;
+
+void h()
+{
+  int x = 2;
+  auto clj = std::bind(add,x,_1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto clj = [=](int arg1) { return add(x, arg1); };
+
+struct A;
+struct B;
+bool ABTest(const A&, const B&);
+
+void i()
+{
+  auto BATest = std::bind(ABTest, _2, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto BATest = [](const struct B & arg1, const struct A & arg2) { return ABTest(arg2, arg1); };
+
+void j()
+{
+  auto clj = std::bind(add, 2, 2, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+// No fix is applied for argument mismatches.
+// CHECK-FIXES: auto clj = std::bind(add, 2, 2, 2);
Index: docs/clang-tidy/checks/misc-avoid-std-bind.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-avoid-std-bind.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - misc-avoid-std-bind
+
+misc-avoid-std-bind
+===
+
+Find uses of std::bind. Replace simple uses of std::bind with lambdas. Lambdas
+will use value-capture where required.
+
+Right now it only handles free functions not member functions.
+
+std::bind can result in larger object files and binaries due to type
+information that will not be produced by equivalent lambdas.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -46,6 +46,7 @@
misc-argument-comment
misc-assert-side-effect
misc-assign-operator-signature
+   misc-avoid-std-bind
misc-bool-pointer-implicit-conversion
misc-definitions-in-headers
misc-inaccurate-erase
Index: clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -13,6 +13,7 @@
 #include "ArgumentCommentCheck.h"
 #include "AssertSideEffectCheck.h"
 #include "AssignOperatorSignatureCheck.h"
+#include "AvoidStdBindCheck.h"
 #include "BoolPointerImplicitConversionCheck.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "InaccurateEraseCheck.h"
@@ -48,6 +49,8 @@
 "misc-assert-side-effect");
 CheckFactories.registerCheck(
 "misc-assign-operator-signature");
+CheckFactories.registerCheck(
+"misc-avoid-std-bind");
 CheckFactories.registerCheck(
 "misc-bool-pointer-implicit-conversion");
 CheckFactories.registerCheck(
Index: clang-tidy/misc/CMakeLists.txt
===
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -4,6 +4,7 @@
   ArgumentCommentCheck.cpp
   AssertSideEffectCheck.cpp
   AssignOperatorSignatureCheck.cpp
+  AvoidStdBindCheck.cpp
   BoolPointerImplicitConversionCheck.cpp
   DefinitionsInHeadersCheck.cpp
   InaccurateEraseCheck.cpp
Index: clang-tidy/misc/AvoidStdBindCheck.h
===
--- /dev/null
+++ clang-tidy/misc/AvoidStdBindCheck.h
@@ -0,0 +1,59 @@
+//===--- AvoidStdBindCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for 

Re: [PATCH] D16376: clang-tidy check: rule-of-five

[Jonathan B Coe via cfe-commits]

jbcoe added a comment.

The method I'm using to insert after a function declaration is flawed. 
Advancing by one character is not always the right thing to do and won't handle 
cases where there is a space before a semi-colon. I'll add extra tests and see 
if I can come up with a neater way of handling the post-declaration insertion.


http://reviews.llvm.org/D16376



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

Re: [PATCH] D16376: clang-tidy check: rule-of-five

[Jonathan B Coe via cfe-commits]

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

I think I'll move this check to `cppcoreguidelines` and call it `rule-of-five`.

https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all

I'll add destructor handling as a later patch.


http://reviews.llvm.org/D16376



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

Re: [PATCH] D16376: clang-tidy check: rule-of-five

[Jonathan B Coe via cfe-commits]

jbcoe retitled this revision from "clang-tidy check: Assignment and 
Construction" to "clang-tidy check: rule-of-five".
jbcoe removed rL LLVM as the repository for this revision.
jbcoe updated this revision to Diff 46775.
jbcoe added a comment.

I've responded to review comments (thanks for those) and renamed the check to 
'rule-of-five'.

I think it needs moving to cppcoreguidelines and needs destructor handling 
adding to it. As suggested, I'll address that in a later patch if everything 
else looks ok.


http://reviews.llvm.org/D16376

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/RuleOfFiveCheck.cpp
  clang-tidy/misc/RuleOfFiveCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-rule-of-five.rst
  test/clang-tidy/misc-rule-of-five.cpp

Index: test/clang-tidy/misc-rule-of-five.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-rule-of-five.cpp
@@ -0,0 +1,209 @@
+// RUN: %check_clang_tidy %s misc-rule-of-five %t
+
+//
+// User defined copy constructors
+//
+class A {
+  A(const A &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'A' defines a copy constructor but not a copy assignment operator [misc-rule-of-five]
+};
+
+// CHECK-FIXES: class A {
+// CHECK-FIXES-NEXT: A(const A &);
+// CHECK-FIXES-NEXT: A =(const A &) = delete;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B {
+  B(const B &);
+  B =(const B &);
+};
+
+class C {
+  C(const C &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'C' defines a copy constructor but not a copy assignment operator [misc-rule-of-five]
+};
+
+// CHECK-FIXES: class C {
+// CHECK-FIXES-NEXT: C(const C &) = default;
+// CHECK-FIXES-NEXT: C =(const C &) = default;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D {
+  D(const D &);
+  D =(const D &) = default;
+};
+
+class E {
+  E(const E &);
+  E =(const E &) = delete;
+};
+
+class F {
+  F(const F &) = delete;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'F' defines a copy constructor but not a copy assignment operator [misc-rule-of-five]
+};
+
+// CHECK-FIXES: class F {
+// CHECK-FIXES-NEXT: F(const F &) = delete;
+// CHECK-FIXES-NEXT: F =(const F &) = delete;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+
+//
+// User defined copy assignment
+//
+class A2 {
+  A2 =(const A2 &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'A2' defines a copy assignment operator but not a copy constructor [misc-rule-of-five]
+};
+
+// CHECK-FIXES: class A2 {
+// CHECK-FIXES-NEXT: A2(const A2 &) = delete;
+// CHECK-FIXES-NEXT: A2 =(const A2 &);
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B2 {
+  B2(const B2 &);
+  B2 =(const B2 &);
+};
+
+class C2 {
+  C2 =(const C2 &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'C2' defines a copy assignment operator but not a copy constructor [misc-rule-of-five]
+};
+
+// CHECK-FIXES: class C2 {
+// CHECK-FIXES-NEXT: C2(const C2 &) = default;
+// CHECK-FIXES-NEXT: C2 =(const C2 &) = default;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D2 {
+  D2(const D2 &) = default;
+  D2 =(const D2 &);
+};
+
+class E2 {
+  E2(const E2 &) = delete;
+  E2 =(const E2 &);
+};
+
+class F2 {
+  F2 =(const F2 &) = delete;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'F2' defines a copy assignment operator but not a copy constructor [misc-rule-of-five]
+};
+
+// CHECK-FIXES: class F2 {
+// CHECK-FIXES-NEXT: F2(const F2 &) = delete;
+// CHECK-FIXES-NEXT: F2 =(const F2 &) = delete;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+
+//
+// User defined move constructors
+//
+class A3 {
+  A3(A3 &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'A3' defines a move constructor but not a move assignment operator [misc-rule-of-five]
+};
+
+// CHECK-FIXES: class A3 {
+// CHECK-FIXES-NEXT: A3(A3 &&);
+// CHECK-FIXES-NEXT: A3 =(A3 &&) = delete;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B3 {
+  B3(B3 &&);
+  B3 =(B3 &&);
+};
+
+class C3 {
+  C3(C3 &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'C3' defines a move constructor but not a move assignment operator [misc-rule-of-five]
+};
+
+// CHECK-FIXES: class C3 {
+// CHECK-FIXES-NEXT: C3(C3 &&) = default;
+// CHECK-FIXES-NEXT: C3 =(C3 &&) = default;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D3 {
+  D3(D3 &&);
+  D3 =(D3 &&) = default;
+};
+
+class E3 {
+  E3(E3 &&);
+  E3 =(E3 &&) = delete;
+};
+
+class F3 {
+  F3(F3 &&) = delete;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'F3' defines a move constructor but not a move assignment operator [misc-rule-of-five]
+};
+
+// CHECK-FIXES: class F3 {
+// CHECK-FIXES-NEXT: F3(F3 &&) = delete;
+// CHECK-FIXES-NEXT: F3 =(F3 &&) = delete;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+
+//
+// User defined move assignment
+//
+class A4 {
+  A4 =(A4 &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'A4' 

Re: [PATCH] D16376: clang-tidy check: rule-of-five

[Jonathan B Coe via cfe-commits]

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

If the destructor is user-declared then I need to `=delete` the 
compiler-generated copy constructor and copy assignment  operator (if they are 
not defined, if either is defined then they are already handled by this check).

The move constructor and move assignment operator do not suffer from deprecated 
compiler behaviour so do not need to be explicitly deleted if the destructor is 
user-declared. Perhaps they should be though as this is called 'rule-of-five' 
(for now).

I think that deleting special member functions in the presence of a 
user-defined destructor is the right thing to do even if the user has defined 
the destructor as `=default` otherwise the check will go against the intention 
of the standard.


http://reviews.llvm.org/D16376



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

Re: [PATCH] D16376: clang-tidy check: Assignment and Construction

[Jonathan B Coe via cfe-commits]

jbcoe retitled this revision from "clang-tidy check: User-defined copy without 
assignment" to "clang-tidy check: Assignment and Construction".
jbcoe updated the summary for this revision.
jbcoe set the repository for this revision to rL LLVM.
jbcoe updated this revision to Diff 45987.
jbcoe added a comment.

Rename check and restructure code to avoid repetition.


Repository:
  rL LLVM

http://reviews.llvm.org/D16376

Files:
  clang-tidy/misc/AssignmentAndConstructionCheck.cpp
  clang-tidy/misc/AssignmentAndConstructionCheck.h
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-assignment-and-construction.rst
  test/clang-tidy/misc-assignment-and-construction.cpp

Index: test/clang-tidy/misc-assignment-and-construction.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-assignment-and-construction.cpp
@@ -0,0 +1,161 @@
+// RUN: %check_clang_tidy %s misc-assignment-and-construction %t
+
+//
+// User defined copy-constructors
+//
+class A {
+  A(const A &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'A' defines a copy-constructor but not a copy-assignment operator [misc-assignment-and-construction]
+};
+
+// CHECK-FIXES: class A {
+// CHECK-FIXES-NEXT: A(const A &);
+// CHECK-FIXES-NEXT: A =(const A &) = delete;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B {
+  B(const B &);
+  B =(const B &);
+};
+
+class C {
+  C(const C &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'C' defines a copy-constructor but not a copy-assignment operator [misc-assignment-and-construction]
+};
+
+// CHECK-FIXES: class C {
+// CHECK-FIXES-NEXT: C(const C &) = default;
+// CHECK-FIXES-NEXT: C =(const C &) = default;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D {
+  D(const D &);
+  D =(const D &) = default;
+};
+
+class E {
+  E(const E &);
+  E =(const E &) = delete;
+};
+
+//
+// User defined copy-assignment
+//
+class A2 {
+  A2 =(const A2 &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'A2' defines a copy-assignment operator but not a copy-constructor [misc-assignment-and-construction]
+};
+
+// CHECK-FIXES: class A2 {
+// CHECK-FIXES-NEXT: A2(const A2 &) = delete;
+// CHECK-FIXES-NEXT: A2 =(const A2 &);
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B2 {
+  B2(const B2 &);
+  B2 =(const B2 &);
+};
+
+class C2 {
+  C2 =(const C2 &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'C2' defines a copy-assignment operator but not a copy-constructor [misc-assignment-and-construction]
+};
+
+// CHECK-FIXES: class C2 {
+// CHECK-FIXES-NEXT: C2(const C2 &) = default;
+// CHECK-FIXES-NEXT: C2 =(const C2 &) = default;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D2 {
+  D2(const D2 &) = default;
+  D2 =(const D2 &);
+};
+
+class E2 {
+  E2(const E2 &) = delete;
+  E2 =(const E2 &);
+};
+
+//
+// User defined move-constructors
+//
+class A3 {
+  A3(A3 &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'A3' defines a move-constructor but not a move-assignment operator [misc-assignment-and-construction]
+};
+
+// CHECK-FIXES: class A3 {
+// CHECK-FIXES-NEXT: A3(A3 &&);
+// CHECK-FIXES-NEXT: A3 =(A3 &&) = delete;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B3 {
+  B3(B3 &&);
+  B3 =(B3 &&);
+};
+
+class C3 {
+  C3(C3 &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'C3' defines a move-constructor but not a move-assignment operator [misc-assignment-and-construction]
+};
+
+// CHECK-FIXES: class C3 {
+// CHECK-FIXES-NEXT: C3(C3 &&) = default;
+// CHECK-FIXES-NEXT: C3 =(C3 &&) = default;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D3 {
+  D3(D3 &&);
+  D3 =(D3 &&) = default;
+};
+
+class E3 {
+  E3(E3 &&);
+  E3 =(E3 &&) = delete;
+};
+
+//
+// User defined move-assignment
+//
+class A4 {
+  A4 =(A4 &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'A4' defines a move-assignment operator but not a move-constructor [misc-assignment-and-construction]
+};
+
+// CHECK-FIXES: class A4 {
+// CHECK-FIXES-NEXT: A4(A4 &&) = delete;
+// CHECK-FIXES-NEXT: A4 =(A4 &&);
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B4 {
+  B4(B4 &&);
+  B4 =(B4 &&);
+};
+
+class C4 {
+  C4 =(C4 &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'C4' defines a move-assignment operator but not a move-constructor [misc-assignment-and-construction]
+};
+
+// CHECK-FIXES: class C4 {
+// CHECK-FIXES-NEXT: C4(C4 &&) = default;
+// CHECK-FIXES-NEXT: C4 =(C4 &&) = default;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D4 {
+  D4(D4 &&) = default;
+  D4 =(D4 &&);
+};
+
+class E4 {
+  E4(E4 &&) = delete;
+  E4 =(E4 &&);
+};
Index: docs/clang-tidy/checks/misc-assignment-and-construction.rst
===
--- /dev/null
+++ 

Re: [PATCH] D16376: clang-tidy check: User-defined copy without assignment

[Jonathan B Coe via cfe-commits]

jbcoe added inline comments.


Comment at: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp:52
@@ +51,3 @@
+  hasDescendant(
+  cxxMethodDecl(isMoveAssignmentOperator(), unless(isImplicit()))
+  .bind("move-assignment")),

I've moved the C++11 check to just before creating the fixit.


http://reviews.llvm.org/D16376



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

Re: [PATCH] D16376: clang-tidy check: User-defined copy without assignment

[Jonathan B Coe via cfe-commits]

jbcoe updated this revision to Diff 45729.
jbcoe added a comment.

Added handling for move-constructor/move-assignment pairs.


http://reviews.llvm.org/D16376

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp
  clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst
  test/clang-tidy/misc-user-defined-copy-without-assignment.cpp

Index: test/clang-tidy/misc-user-defined-copy-without-assignment.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-user-defined-copy-without-assignment.cpp
@@ -0,0 +1,161 @@
+// RUN: %check_clang_tidy %s misc-user-defined-copy-without-assignment %t
+
+//
+// User defined copy-constructors
+//
+class A {
+  A(const A &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'A' defines a copy-constructor but not a copy-assignment operator [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class A {
+// CHECK-FIXES-NEXT: A(const A &);
+// CHECK-FIXES-NEXT: A =(const A &) = delete;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B {
+  B(const B &);
+  B =(const B &);
+};
+
+class C {
+  C(const C &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'C' defines a copy-constructor but not a copy-assignment operator [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class C {
+// CHECK-FIXES-NEXT: C(const C &) = default;
+// CHECK-FIXES-NEXT: C =(const C &) = default;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D {
+  D(const D &);
+  D =(const D &) = default;
+};
+
+class E {
+  E(const E &);
+  E =(const E &) = delete;
+};
+
+//
+// User defined copy-assignment
+//
+class A2 {
+  A2 =(const A2 &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'A2' defines a copy-assignment operator but not a copy-constructor [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class A2 {
+// CHECK-FIXES-NEXT: A2(const A2 &) = delete;
+// CHECK-FIXES-NEXT: A2 =(const A2 &);
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B2 {
+  B2(const B2 &);
+  B2 =(const B2 &);
+};
+
+class C2 {
+  C2 =(const C2 &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'C2' defines a copy-assignment operator but not a copy-constructor [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class C2 {
+// CHECK-FIXES-NEXT: C2(const C2 &) = default;
+// CHECK-FIXES-NEXT: C2 =(const C2 &) = default;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D2 {
+  D2(const D2 &) = default;
+  D2 =(const D2 &);
+};
+
+class E2 {
+  E2(const E2 &) = delete;
+  E2 =(const E2 &);
+};
+
+//
+// User defined move-constructors
+//
+class A3 {
+  A3(A3 &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'A3' defines a move-constructor but not a move-assignment operator [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class A3 {
+// CHECK-FIXES-NEXT: A3(A3 &&);
+// CHECK-FIXES-NEXT: A3 =(A3 &&) = delete;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B3 {
+  B3(B3 &&);
+  B3 =(B3 &&);
+};
+
+class C3 {
+  C3(C3 &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'C3' defines a move-constructor but not a move-assignment operator [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class C3 {
+// CHECK-FIXES-NEXT: C3(C3 &&) = default;
+// CHECK-FIXES-NEXT: C3 =(C3 &&) = default;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D3 {
+  D3(D3 &&);
+  D3 =(D3 &&) = default;
+};
+
+class E3 {
+  E3(E3 &&);
+  E3 =(E3 &&) = delete;
+};
+
+//
+// User defined move-assignment
+//
+class A4 {
+  A4 =(A4 &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'A4' defines a move-assignment operator but not a move-constructor [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class A4 {
+// CHECK-FIXES-NEXT: A4(A4 &&) = delete;
+// CHECK-FIXES-NEXT: A4 =(A4 &&);
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B4 {
+  B4(B4 &&);
+  B4 =(B4 &&);
+};
+
+class C4 {
+  C4 =(C4 &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'C4' defines a move-assignment operator but not a move-constructor [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class C4 {
+// CHECK-FIXES-NEXT: C4(C4 &&) = default;
+// CHECK-FIXES-NEXT: C4 =(C4 &&) = default;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D4 {
+  D4(D4 &&) = default;
+  D4 =(D4 &&);
+};
+
+class E4 {
+  E4(E4 &&) = delete;
+  E4 =(E4 &&);
+};
Index: docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - misc-user-defined-copy-without-assignment
+

Re: [PATCH] D16376: clang-tidy check: User-defined copy without assignment

[Jonathan B Coe via cfe-commits]

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

i need the matcher from http://reviews.llvm.org/D16470 to add a corresponding 
pair of checks and fixes for move-constructors and move-assignment.


http://reviews.llvm.org/D16376



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

Re: [PATCH] D16376: clang-tidy check: User-defined copy without assignment

[Jonathan B Coe via cfe-commits]

jbcoe added inline comments.


Comment at: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h:25
@@ +24,3 @@
+/// assignment operator to be `= delete`.
+///
+/// For the user-facing documentation see:

jbcoe wrote:
> The standard says that compiler generation of the assignment operator is 
> deprecated if there is a user-defined copy constructor 12.8.18 . 
> 
> '= default' still counts as user-defined to the best of my understanding.
> 
> MSVC, clang and gcc all generate assignment operators in the face of deleted 
> copy constructors. None of them seem to follow the deprecation rule.
> 
> I agree entirely with your point in principle, but it seems to be at odds 
> with the standard. I think requiring the user to explicitly default the 
> assignment operator by generating a deleted assignment operator is the right 
> thing to do.
On reflection this is pedantic. As you say if a user defaults one, they will 
want to default the other. 


http://reviews.llvm.org/D16376



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

Re: [PATCH] D16376: clang-tidy check: User-defined copy without assignment

[Jonathan B Coe via cfe-commits]

jbcoe marked 5 inline comments as done.


Comment at: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h:25
@@ +24,3 @@
+/// assignment operator to be `= delete`.
+///
+/// For the user-facing documentation see:

The standard says that compiler generation of the assignment operator is 
deprecated if there is a user-defined copy constructor 12.8.18 . 

'= default' still counts as user-defined to the best of my understanding.

MSVC, clang and gcc all generate assignment operators in the face of deleted 
copy constructors. None of them seem to follow the deprecation rule.

I agree entirely with your point in principle, but it seems to be at odds with 
the standard. I think requiring the user to explicitly default the assignment 
operator by generating a deleted assignment operator is the right thing to do.


Comment at: 
docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst:24
@@ +23,3 @@
+};
+
+Will be matched and fixed to delete the assignment operator:

How do I go about disabling a check by default?


http://reviews.llvm.org/D16376



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

Re: [PATCH] D16376: clang-tidy check: User-defined copy without assignment

[Jonathan B Coe via cfe-commits]

jbcoe removed rL LLVM as the repository for this revision.
jbcoe updated this revision to Diff 45554.
jbcoe added a comment.

Made requested changes.


http://reviews.llvm.org/D16376

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp
  clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h
  clang-tidy/modernize/UseDefaultCheck.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst
  test/clang-tidy/misc-user-defined-copy-without-assignment.cpp
  test/clang-tidy/modernize-use-default-delayed.cpp

Index: test/clang-tidy/modernize-use-default-delayed.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-default-delayed.cpp
@@ -0,0 +1,8 @@
+// RUN: clang-tidy %s -checks=-*,modernize-use-default -- -std=c++11 -fdelayed-template-parsing -fexceptions | count 0
+// Note: this test expects no diagnostics, but FileCheck cannot handle that,
+// hence the use of | count 0.
+
+template 
+struct S {
+  S& operator=(const S&) { return *this; }
+};
Index: test/clang-tidy/misc-user-defined-copy-without-assignment.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-user-defined-copy-without-assignment.cpp
@@ -0,0 +1,38 @@
+// RUN: %check_clang_tidy %s misc-user-defined-copy-without-assignment %t
+class A {
+  A(const A &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'A' defines a copy-constructor but not an assignment operator [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class A {
+// CHECK-FIXES-NEXT: A(const A &);
+// CHECK-FIXES-NEXT: A =(const A &) = delete;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B {
+  B(const B &);
+  B =(const B &);
+};
+
+class C {
+  C(const C &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'C' defines a copy-constructor but not an assignment operator [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class C {
+// CHECK-FIXES-NEXT: C(const C &) = default;
+// CHECK-FIXES-NEXT: C =(const C &) = delete;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D {
+  D(const D &);
+  D =(const D &) = default;
+};
+
+class E {
+  E(const E &);
+  E =(const E &) = delete;
+};
+
Index: docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - misc-user-defined-copy-without-assignment
+
+misc-user-defined-copy-without-assignment
+=
+
+Compilers will generate an assignment operator even if the user defines a copy
+constructor.  This behaviour is deprecated by the standard (C++ 14 draft
+standard 12.8.18)
+
+"If the class definition does not explicitly declare a copy assignment
+operator, one is declared implicitly. If the class definition declares a move
+constructor or move assignment operator, the implicitly declared copy
+assignment operator is defined as deleted; otherwise, it is defined as
+defaulted (8.4). The latter case is deprecated if the class has a user-declared
+copy constructor or a user-declared destructor."
+
+This check finds classes with a user-defined (including deleted)
+copy-constructor but no assignment operator.
+
+  .. code:: c++
+class A {
+  A(const A&);
+};
+
+Will be matched and fixed to delete the assignment operator:
+
+  .. code:: c++
+class A {
+  A(const A&);
+  A& operator = (const A&) = delete;
+};
+
+The fix is defensive. Incorrect compiler-generated assignement can cause
+unexpected behaviour. An explicitly deleted assignment operator will cause a
+compiler error if it is used.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -65,6 +65,7 @@
misc-unused-alias-decls
misc-unused-parameters
misc-unused-raii
+   misc-user-defined-copy-without-assignment
misc-virtual-near-miss
modernize-loop-convert
modernize-make-unique
Index: clang-tidy/modernize/UseDefaultCheck.cpp
===
--- clang-tidy/modernize/UseDefaultCheck.cpp
+++ clang-tidy/modernize/UseDefaultCheck.cpp
@@ -272,6 +272,7 @@
   // that are not user-provided (automatically generated).
   if (SpecialFunctionDecl->isDeleted() ||
   SpecialFunctionDecl->isExplicitlyDefaulted() ||
+  SpecialFunctionDecl->isLateTemplateParsed() ||
   !SpecialFunctionDecl->isUserProvided() || !SpecialFunctionDecl->hasBody())
 return;
 
Index: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h
===
--- /dev/null
+++ 

Re: [PATCH] D7714: missing-namespace-std check for clang-tidy

[Jonathan B Coe via cfe-commits]

jbcoe abandoned this revision.
jbcoe added a comment.

This work needs such extensive revision that I'll submit a new version when I 
start work on it again.


Repository:
  rL LLVM

http://reviews.llvm.org/D7714



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