[PATCH] D39162: [test] Fix clang-test for FreeBSD and NetBSD

2017-10-21 Thread Zhihao Yuan via Phabricator via cfe-commits
lichray updated this revision to Diff 119772.
lichray retitled this revision from "[test] Fix clang-test for FreeBSD" to 
"[test] Fix clang-test for FreeBSD and NetBSD".
lichray edited the summary of this revision.

https://reviews.llvm.org/D39162

Files:
  test/Unit/lit.cfg.py


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -36,12 +36,14 @@
 config.environment[symbolizer] = os.environ[symbolizer]
 
 shlibpath_var = ''
-if platform.system() == 'Linux':
+if platform.system() in ['Linux', 'FreeBSD', 'NetBSD']:
 shlibpath_var = 'LD_LIBRARY_PATH'
 elif platform.system() == 'Darwin':
 shlibpath_var = 'DYLD_LIBRARY_PATH'
 elif platform.system() == 'Windows':
 shlibpath_var = 'PATH'
+else:
+raise EnvironmentError('unknown platform.system()')
 
 # in stand-alone builds, shlibdir is clang's build tree
 # while llvm_libs_dir is installed LLVM (and possibly older clang)


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -36,12 +36,14 @@
 config.environment[symbolizer] = os.environ[symbolizer]
 
 shlibpath_var = ''
-if platform.system() == 'Linux':
+if platform.system() in ['Linux', 'FreeBSD', 'NetBSD']:
 shlibpath_var = 'LD_LIBRARY_PATH'
 elif platform.system() == 'Darwin':
 shlibpath_var = 'DYLD_LIBRARY_PATH'
 elif platform.system() == 'Windows':
 shlibpath_var = 'PATH'
+else:
+raise EnvironmentError('unknown platform.system()')
 
 # in stand-alone builds, shlibdir is clang's build tree
 # while llvm_libs_dir is installed LLVM (and possibly older clang)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39162: [test] Fix clang-test for FreeBSD

2017-10-21 Thread Kamil Rytarowski via Phabricator via cfe-commits
krytarowski added inline comments.



Comment at: test/Unit/lit.cfg.py:39
 shlibpath_var = ''
-if platform.system() == 'Linux':
+if platform.system() in ['Linux', 'FreeBSD']:
 shlibpath_var = 'LD_LIBRARY_PATH'

Please include 'NetBSD' next to 'FreeBSD'.


https://reviews.llvm.org/D39162



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


[PATCH] D39162: [test] Fix clang-test for FreeBSD

2017-10-21 Thread Zhihao Yuan via Phabricator via cfe-commits
lichray created this revision.
Herald added subscribers: krytarowski, emaste.

Lit tries to inject the shared library paths, but no action is taken when 
`platform.system()` is not recognized, results in an environment variable with 
an empty name, which is illegal.

The patch fixes this mechanism for FreeBSD, and throws an exception on other 
platforms, so that the latecomers don't have to spend time on debugging lit.


https://reviews.llvm.org/D39162

Files:
  test/Unit/lit.cfg.py


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -36,12 +36,14 @@
 config.environment[symbolizer] = os.environ[symbolizer]
 
 shlibpath_var = ''
-if platform.system() == 'Linux':
+if platform.system() in ['Linux', 'FreeBSD']:
 shlibpath_var = 'LD_LIBRARY_PATH'
 elif platform.system() == 'Darwin':
 shlibpath_var = 'DYLD_LIBRARY_PATH'
 elif platform.system() == 'Windows':
 shlibpath_var = 'PATH'
+else:
+raise EnvironmentError('unknown platform.system()')
 
 # in stand-alone builds, shlibdir is clang's build tree
 # while llvm_libs_dir is installed LLVM (and possibly older clang)


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -36,12 +36,14 @@
 config.environment[symbolizer] = os.environ[symbolizer]
 
 shlibpath_var = ''
-if platform.system() == 'Linux':
+if platform.system() in ['Linux', 'FreeBSD']:
 shlibpath_var = 'LD_LIBRARY_PATH'
 elif platform.system() == 'Darwin':
 shlibpath_var = 'DYLD_LIBRARY_PATH'
 elif platform.system() == 'Windows':
 shlibpath_var = 'PATH'
+else:
+raise EnvironmentError('unknown platform.system()')
 
 # in stand-alone builds, shlibdir is clang's build tree
 # while llvm_libs_dir is installed LLVM (and possibly older clang)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39156: [libunwind] Make HIDDEN_DIRECTIVE a function-like macro. NFCI.

2017-10-21 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd accepted this revision.
compnerd added a comment.
This revision is now accepted and ready to land.

Thanks for cleaning this up.  IIRC, I have similar behavior in compiler-rt for 
`HIDDEN_SYMBOL`.


https://reviews.llvm.org/D39156



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


[PATCH] D37954: Try to shorten system header paths when using -MD depfiles

2017-10-21 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment.

I think the diagnosis on the original issue was incorrect.

It seems to me that it was caused by the prefix being set as "/bin" instead of 
"/usr/bin", because clang _doesn't_ actually canonicalize its prefix, even when 
-no-canonical-prefixes isn't specified! All it does, now, is to make the prefix 
absolute -- without fully canonicalizing. I think that's simply a bug.

If the prefix had been, properly, /usr/bin, then the include path would've been:

  
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/7.2.0/../../../../include/c++/7.2.0/iostream

And in that case, it would've worked properly with ninja, without adding the 
feature in this patch.

Reference clang/tools/driver/driver.cpp:297:

  // FIXME: We don't actually canonicalize this, we just make it absolute.
  if (CanonicalPrefixes)
llvm::sys::fs::make_absolute(InstalledPath);


Repository:
  rL LLVM

https://reviews.llvm.org/D37954



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


Re: r316278 - [libclang, bindings]: add spelling location

2017-10-21 Thread Masud Rahman via cfe-commits
Thanks, I will take a look.

On Sat, Oct 21, 2017 at 5:53 PM, Aaron Ballman 
wrote:

> I've reverted back to green in r316279 due to more bots failing.
>
> ~Aaron
>
> On Sat, Oct 21, 2017 at 5:35 PM, Aaron Ballman 
> wrote:
> > This commit appears to have broken several bots. Can you revert or
> > quickly fix the issue?
> >
> > http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/11896
> > http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/12380
> >
> > Thanks!
> >
> > ~Aaron
> >
> > On Sat, Oct 21, 2017 at 4:53 PM, Masud Rahman via cfe-commits
> >  wrote:
> >> Author: frutiger
> >> Date: Sat Oct 21 13:53:49 2017
> >> New Revision: 316278
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=316278=rev
> >> Log:
> >> [libclang, bindings]: add spelling location
> >>
> >>  o) Add a 'Location' class that represents the four properties of a
> >> physical location
> >>
> >>  o) Enhance 'SourceLocation' to provide 'expansion' and 'spelling'
> >> locations, maintaining backwards compatibility with existing code by
> >> forwarding the four properties to 'expansion'.
> >>
> >>  o) Update the implementation to use 'clang_getExpansionLocation'
> >> instead of the deprecated 'clang_getInstantiationLocation', which
> >> has been present since 2011.
> >>
> >>  o) Update the implementation of 'clang_getSpellingLocation' to actually
> >> obtain spelling location instead of file location.
> >>
> >> Modified:
> >> cfe/trunk/bindings/python/clang/cindex.py
> >> cfe/trunk/bindings/python/tests/cindex/test_location.py
> >> cfe/trunk/tools/libclang/CXSourceLocation.cpp
> >>
> >> Modified: cfe/trunk/bindings/python/clang/cindex.py
> >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/
> python/clang/cindex.py?rev=316278=316277=316278=diff
> >> 
> ==
> >> --- cfe/trunk/bindings/python/clang/cindex.py (original)
> >> +++ cfe/trunk/bindings/python/clang/cindex.py Sat Oct 21 13:53:49 2017
> >> @@ -214,25 +214,45 @@ class _CXString(Structure):
> >>  assert isinstance(res, _CXString)
> >>  return conf.lib.clang_getCString(res)
> >>
> >> +class Location(object):
> >> +"""A Location is a specific kind of source location.  A
> SourceLocation
> >> +refers to several kinds of locations (e.g.  spelling location vs.
> expansion
> >> +location)."""
> >> +
> >> +def __init__(self, file, line, column, offset):
> >> +self._file   = File(file) if file else None
> >> +self._line   = int(line.value)
> >> +self._column = int(column.value)
> >> +self._offset = int(offset.value)
> >> +
> >> +
> >> +@property
> >> +def file(self):
> >> +"""Get the file represented by this source location."""
> >> +return self._file
> >> +
> >> +@property
> >> +def line(self):
> >> +"""Get the line represented by this source location."""
> >> +return self._line
> >> +
> >> +@property
> >> +def column(self):
> >> +"""Get the column represented by this source location."""
> >> +return self._column
> >> +
> >> +@property
> >> +def offset(self):
> >> +"""Get the file offset represented by this source location."""
> >> +return self._offset
> >>
> >>  class SourceLocation(Structure):
> >>  """
> >>  A SourceLocation represents a particular location within a source
> file.
> >>  """
> >>  _fields_ = [("ptr_data", c_void_p * 2), ("int_data", c_uint)]
> >> -_data = None
> >> -
> >> -def _get_instantiation(self):
> >> -if self._data is None:
> >> -f, l, c, o = c_object_p(), c_uint(), c_uint(), c_uint()
> >> -conf.lib.clang_getInstantiationLocation(self, byref(f),
> byref(l),
> >> -byref(c), byref(o))
> >> -if f:
> >> -f = File(f)
> >> -else:
> >> -f = None
> >> -self._data = (f, int(l.value), int(c.value), int(o.value))
> >> -return self._data
> >> +_expansion = None
> >> +_spelling = None
> >>
> >>  @staticmethod
> >>  def from_position(tu, file, line, column):
> >> @@ -253,24 +273,72 @@ class SourceLocation(Structure):
> >>  return conf.lib.clang_getLocationForOffset(tu, file, offset)
> >>
> >>  @property
> >> +def expansion(self):
> >> +"""
> >> +The source location where then entity this object is referring
> to is
> >> +expanded.
> >> +"""
> >> +if not self._expansion:
> >> +file   = c_object_p()
> >> +line   = c_uint()
> >> +column = c_uint()
> >> +offset = c_uint()
> >> +conf.lib.clang_getExpansionLocation(self,
> >> +byref(file),
> >> +  

Re: r316278 - [libclang, bindings]: add spelling location

2017-10-21 Thread Aaron Ballman via cfe-commits
I've reverted back to green in r316279 due to more bots failing.

~Aaron

On Sat, Oct 21, 2017 at 5:35 PM, Aaron Ballman  wrote:
> This commit appears to have broken several bots. Can you revert or
> quickly fix the issue?
>
> http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/11896
> http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/12380
>
> Thanks!
>
> ~Aaron
>
> On Sat, Oct 21, 2017 at 4:53 PM, Masud Rahman via cfe-commits
>  wrote:
>> Author: frutiger
>> Date: Sat Oct 21 13:53:49 2017
>> New Revision: 316278
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=316278=rev
>> Log:
>> [libclang, bindings]: add spelling location
>>
>>  o) Add a 'Location' class that represents the four properties of a
>> physical location
>>
>>  o) Enhance 'SourceLocation' to provide 'expansion' and 'spelling'
>> locations, maintaining backwards compatibility with existing code by
>> forwarding the four properties to 'expansion'.
>>
>>  o) Update the implementation to use 'clang_getExpansionLocation'
>> instead of the deprecated 'clang_getInstantiationLocation', which
>> has been present since 2011.
>>
>>  o) Update the implementation of 'clang_getSpellingLocation' to actually
>> obtain spelling location instead of file location.
>>
>> Modified:
>> cfe/trunk/bindings/python/clang/cindex.py
>> cfe/trunk/bindings/python/tests/cindex/test_location.py
>> cfe/trunk/tools/libclang/CXSourceLocation.cpp
>>
>> Modified: cfe/trunk/bindings/python/clang/cindex.py
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/clang/cindex.py?rev=316278=316277=316278=diff
>> ==
>> --- cfe/trunk/bindings/python/clang/cindex.py (original)
>> +++ cfe/trunk/bindings/python/clang/cindex.py Sat Oct 21 13:53:49 2017
>> @@ -214,25 +214,45 @@ class _CXString(Structure):
>>  assert isinstance(res, _CXString)
>>  return conf.lib.clang_getCString(res)
>>
>> +class Location(object):
>> +"""A Location is a specific kind of source location.  A SourceLocation
>> +refers to several kinds of locations (e.g.  spelling location vs. 
>> expansion
>> +location)."""
>> +
>> +def __init__(self, file, line, column, offset):
>> +self._file   = File(file) if file else None
>> +self._line   = int(line.value)
>> +self._column = int(column.value)
>> +self._offset = int(offset.value)
>> +
>> +
>> +@property
>> +def file(self):
>> +"""Get the file represented by this source location."""
>> +return self._file
>> +
>> +@property
>> +def line(self):
>> +"""Get the line represented by this source location."""
>> +return self._line
>> +
>> +@property
>> +def column(self):
>> +"""Get the column represented by this source location."""
>> +return self._column
>> +
>> +@property
>> +def offset(self):
>> +"""Get the file offset represented by this source location."""
>> +return self._offset
>>
>>  class SourceLocation(Structure):
>>  """
>>  A SourceLocation represents a particular location within a source file.
>>  """
>>  _fields_ = [("ptr_data", c_void_p * 2), ("int_data", c_uint)]
>> -_data = None
>> -
>> -def _get_instantiation(self):
>> -if self._data is None:
>> -f, l, c, o = c_object_p(), c_uint(), c_uint(), c_uint()
>> -conf.lib.clang_getInstantiationLocation(self, byref(f), 
>> byref(l),
>> -byref(c), byref(o))
>> -if f:
>> -f = File(f)
>> -else:
>> -f = None
>> -self._data = (f, int(l.value), int(c.value), int(o.value))
>> -return self._data
>> +_expansion = None
>> +_spelling = None
>>
>>  @staticmethod
>>  def from_position(tu, file, line, column):
>> @@ -253,24 +273,72 @@ class SourceLocation(Structure):
>>  return conf.lib.clang_getLocationForOffset(tu, file, offset)
>>
>>  @property
>> +def expansion(self):
>> +"""
>> +The source location where then entity this object is referring to is
>> +expanded.
>> +"""
>> +if not self._expansion:
>> +file   = c_object_p()
>> +line   = c_uint()
>> +column = c_uint()
>> +offset = c_uint()
>> +conf.lib.clang_getExpansionLocation(self,
>> +byref(file),
>> +byref(line),
>> +byref(column),
>> +byref(offset))
>> +
>> +self._expansion = Location(file, line, column, offset)
>> +return self._expansion
>> +
>> +@property
>> +def spelling(self):
>> +"""
>> +The source location 

r316279 - Reverting r316278 due to failing build bots.

2017-10-21 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Sat Oct 21 14:52:48 2017
New Revision: 316279

URL: http://llvm.org/viewvc/llvm-project?rev=316279=rev
Log:
Reverting r316278 due to failing build bots.

http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/11896
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/12380

Modified:
cfe/trunk/bindings/python/clang/cindex.py
cfe/trunk/bindings/python/tests/cindex/test_location.py
cfe/trunk/tools/libclang/CXSourceLocation.cpp

Modified: cfe/trunk/bindings/python/clang/cindex.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/clang/cindex.py?rev=316279=316278=316279=diff
==
--- cfe/trunk/bindings/python/clang/cindex.py (original)
+++ cfe/trunk/bindings/python/clang/cindex.py Sat Oct 21 14:52:48 2017
@@ -214,45 +214,25 @@ class _CXString(Structure):
 assert isinstance(res, _CXString)
 return conf.lib.clang_getCString(res)
 
-class Location(object):
-"""A Location is a specific kind of source location.  A SourceLocation
-refers to several kinds of locations (e.g.  spelling location vs. expansion
-location)."""
-
-def __init__(self, file, line, column, offset):
-self._file   = File(file) if file else None
-self._line   = int(line.value)
-self._column = int(column.value)
-self._offset = int(offset.value)
-
-
-@property
-def file(self):
-"""Get the file represented by this source location."""
-return self._file
-
-@property
-def line(self):
-"""Get the line represented by this source location."""
-return self._line
-
-@property
-def column(self):
-"""Get the column represented by this source location."""
-return self._column
-
-@property
-def offset(self):
-"""Get the file offset represented by this source location."""
-return self._offset
 
 class SourceLocation(Structure):
 """
 A SourceLocation represents a particular location within a source file.
 """
 _fields_ = [("ptr_data", c_void_p * 2), ("int_data", c_uint)]
-_expansion = None
-_spelling = None
+_data = None
+
+def _get_instantiation(self):
+if self._data is None:
+f, l, c, o = c_object_p(), c_uint(), c_uint(), c_uint()
+conf.lib.clang_getInstantiationLocation(self, byref(f), byref(l),
+byref(c), byref(o))
+if f:
+f = File(f)
+else:
+f = None
+self._data = (f, int(l.value), int(c.value), int(o.value))
+return self._data
 
 @staticmethod
 def from_position(tu, file, line, column):
@@ -273,72 +253,24 @@ class SourceLocation(Structure):
 return conf.lib.clang_getLocationForOffset(tu, file, offset)
 
 @property
-def expansion(self):
-"""
-The source location where then entity this object is referring to is
-expanded.
-"""
-if not self._expansion:
-file   = c_object_p()
-line   = c_uint()
-column = c_uint()
-offset = c_uint()
-conf.lib.clang_getExpansionLocation(self,
-byref(file),
-byref(line),
-byref(column),
-byref(offset))
-
-self._expansion = Location(file, line, column, offset)
-return self._expansion
-
-@property
-def spelling(self):
-"""
-The source location where then entity this object is referring to is
-written.
-"""
-if not self._spelling:
-file   = c_object_p()
-line   = c_uint()
-column = c_uint()
-offset = c_uint()
-conf.lib.clang_getSpellingLocation(self,
-   byref(file),
-   byref(line),
-   byref(column),
-   byref(offset))
-
-self._spelling = Location(file, line, column, offset)
-return self._spelling
-
-@property
 def file(self):
-"""Get the file represented by this source location.
-
-DEPRECATED: use expansion.file."""
-return self.expansion.file
+"""Get the file represented by this source location."""
+return self._get_instantiation()[0]
 
 @property
 def line(self):
-"""Get the line represented by this source location.
-
-DEPRECATED: use expansion.line."""
-return self.expansion.line
+"""Get the line represented by this source location."""
+return self._get_instantiation()[1]
 
 @property
 def column(self):
-"""Get the column represented 

Re: r316278 - [libclang, bindings]: add spelling location

2017-10-21 Thread Aaron Ballman via cfe-commits
This commit appears to have broken several bots. Can you revert or
quickly fix the issue?

http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/11896
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/12380

Thanks!

~Aaron

On Sat, Oct 21, 2017 at 4:53 PM, Masud Rahman via cfe-commits
 wrote:
> Author: frutiger
> Date: Sat Oct 21 13:53:49 2017
> New Revision: 316278
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316278=rev
> Log:
> [libclang, bindings]: add spelling location
>
>  o) Add a 'Location' class that represents the four properties of a
> physical location
>
>  o) Enhance 'SourceLocation' to provide 'expansion' and 'spelling'
> locations, maintaining backwards compatibility with existing code by
> forwarding the four properties to 'expansion'.
>
>  o) Update the implementation to use 'clang_getExpansionLocation'
> instead of the deprecated 'clang_getInstantiationLocation', which
> has been present since 2011.
>
>  o) Update the implementation of 'clang_getSpellingLocation' to actually
> obtain spelling location instead of file location.
>
> Modified:
> cfe/trunk/bindings/python/clang/cindex.py
> cfe/trunk/bindings/python/tests/cindex/test_location.py
> cfe/trunk/tools/libclang/CXSourceLocation.cpp
>
> Modified: cfe/trunk/bindings/python/clang/cindex.py
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/clang/cindex.py?rev=316278=316277=316278=diff
> ==
> --- cfe/trunk/bindings/python/clang/cindex.py (original)
> +++ cfe/trunk/bindings/python/clang/cindex.py Sat Oct 21 13:53:49 2017
> @@ -214,25 +214,45 @@ class _CXString(Structure):
>  assert isinstance(res, _CXString)
>  return conf.lib.clang_getCString(res)
>
> +class Location(object):
> +"""A Location is a specific kind of source location.  A SourceLocation
> +refers to several kinds of locations (e.g.  spelling location vs. 
> expansion
> +location)."""
> +
> +def __init__(self, file, line, column, offset):
> +self._file   = File(file) if file else None
> +self._line   = int(line.value)
> +self._column = int(column.value)
> +self._offset = int(offset.value)
> +
> +
> +@property
> +def file(self):
> +"""Get the file represented by this source location."""
> +return self._file
> +
> +@property
> +def line(self):
> +"""Get the line represented by this source location."""
> +return self._line
> +
> +@property
> +def column(self):
> +"""Get the column represented by this source location."""
> +return self._column
> +
> +@property
> +def offset(self):
> +"""Get the file offset represented by this source location."""
> +return self._offset
>
>  class SourceLocation(Structure):
>  """
>  A SourceLocation represents a particular location within a source file.
>  """
>  _fields_ = [("ptr_data", c_void_p * 2), ("int_data", c_uint)]
> -_data = None
> -
> -def _get_instantiation(self):
> -if self._data is None:
> -f, l, c, o = c_object_p(), c_uint(), c_uint(), c_uint()
> -conf.lib.clang_getInstantiationLocation(self, byref(f), byref(l),
> -byref(c), byref(o))
> -if f:
> -f = File(f)
> -else:
> -f = None
> -self._data = (f, int(l.value), int(c.value), int(o.value))
> -return self._data
> +_expansion = None
> +_spelling = None
>
>  @staticmethod
>  def from_position(tu, file, line, column):
> @@ -253,24 +273,72 @@ class SourceLocation(Structure):
>  return conf.lib.clang_getLocationForOffset(tu, file, offset)
>
>  @property
> +def expansion(self):
> +"""
> +The source location where then entity this object is referring to is
> +expanded.
> +"""
> +if not self._expansion:
> +file   = c_object_p()
> +line   = c_uint()
> +column = c_uint()
> +offset = c_uint()
> +conf.lib.clang_getExpansionLocation(self,
> +byref(file),
> +byref(line),
> +byref(column),
> +byref(offset))
> +
> +self._expansion = Location(file, line, column, offset)
> +return self._expansion
> +
> +@property
> +def spelling(self):
> +"""
> +The source location where then entity this object is referring to is
> +written.
> +"""
> +if not self._spelling:
> +file   = c_object_p()
> +line   = c_uint()
> +column = c_uint()
> +offset = c_uint()
> +

[PATCH] D39161: [bindings] remove unique_external test failure

2017-10-21 Thread Masud Rahman via Phabricator via cfe-commits
frutiger added a comment.

I was unable to produce a simple test case that would still result in a cursor 
with 'unique external' linkage.  @rsmith I would appreciate if you can think of 
a symbol that may still have this kind of linkage.  Thanks!


https://reviews.llvm.org/D39161



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


[PATCH] D39161: [bindings] remove unique_external test failure

2017-10-21 Thread Masud Rahman via Phabricator via cfe-commits
frutiger created this revision.

In SVN r314037, linkage determination of a symbol was significantly
refactored.  This resulted in extern anonymous namespace declarations to
no longer be considered 'unique_external'.  This ultimately broke a test
in the Python bindings.

This commit removes the 'unique_external' test case.


https://reviews.llvm.org/D39161

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


Index: bindings/python/tests/cindex/test_linkage.py
===
--- bindings/python/tests/cindex/test_linkage.py
+++ bindings/python/tests/cindex/test_linkage.py
@@ -12,7 +12,6 @@
 tu = get_tu("""
 void foo() { int no_linkage; }
 static int internal;
-namespace { extern int unique_external; }
 extern int external;
 """, lang = 'cpp')
 
@@ -22,9 +21,6 @@
 internal = get_cursor(tu.cursor, 'internal')
 assert internal.linkage == LinkageKind.INTERNAL
 
-unique_external = get_cursor(tu.cursor, 'unique_external')
-assert unique_external.linkage == LinkageKind.UNIQUE_EXTERNAL
-
 external = get_cursor(tu.cursor, 'external')
 assert external.linkage == LinkageKind.EXTERNAL
 


Index: bindings/python/tests/cindex/test_linkage.py
===
--- bindings/python/tests/cindex/test_linkage.py
+++ bindings/python/tests/cindex/test_linkage.py
@@ -12,7 +12,6 @@
 tu = get_tu("""
 void foo() { int no_linkage; }
 static int internal;
-namespace { extern int unique_external; }
 extern int external;
 """, lang = 'cpp')
 
@@ -22,9 +21,6 @@
 internal = get_cursor(tu.cursor, 'internal')
 assert internal.linkage == LinkageKind.INTERNAL
 
-unique_external = get_cursor(tu.cursor, 'unique_external')
-assert unique_external.linkage == LinkageKind.UNIQUE_EXTERNAL
-
 external = get_cursor(tu.cursor, 'external')
 assert external.linkage == LinkageKind.EXTERNAL
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37905: [libclang, bindings]: add spelling location

2017-10-21 Thread Masud Rahman via Phabricator via cfe-commits
frutiger closed this revision.
frutiger added a comment.

@compnerd I do now.  Landed in https://reviews.llvm.org/rL316278.


https://reviews.llvm.org/D37905



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


r316278 - [libclang, bindings]: add spelling location

2017-10-21 Thread Masud Rahman via cfe-commits
Author: frutiger
Date: Sat Oct 21 13:53:49 2017
New Revision: 316278

URL: http://llvm.org/viewvc/llvm-project?rev=316278=rev
Log:
[libclang, bindings]: add spelling location

 o) Add a 'Location' class that represents the four properties of a
physical location

 o) Enhance 'SourceLocation' to provide 'expansion' and 'spelling'
locations, maintaining backwards compatibility with existing code by
forwarding the four properties to 'expansion'.

 o) Update the implementation to use 'clang_getExpansionLocation'
instead of the deprecated 'clang_getInstantiationLocation', which
has been present since 2011.

 o) Update the implementation of 'clang_getSpellingLocation' to actually
obtain spelling location instead of file location.

Modified:
cfe/trunk/bindings/python/clang/cindex.py
cfe/trunk/bindings/python/tests/cindex/test_location.py
cfe/trunk/tools/libclang/CXSourceLocation.cpp

Modified: cfe/trunk/bindings/python/clang/cindex.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/clang/cindex.py?rev=316278=316277=316278=diff
==
--- cfe/trunk/bindings/python/clang/cindex.py (original)
+++ cfe/trunk/bindings/python/clang/cindex.py Sat Oct 21 13:53:49 2017
@@ -214,25 +214,45 @@ class _CXString(Structure):
 assert isinstance(res, _CXString)
 return conf.lib.clang_getCString(res)
 
+class Location(object):
+"""A Location is a specific kind of source location.  A SourceLocation
+refers to several kinds of locations (e.g.  spelling location vs. expansion
+location)."""
+
+def __init__(self, file, line, column, offset):
+self._file   = File(file) if file else None
+self._line   = int(line.value)
+self._column = int(column.value)
+self._offset = int(offset.value)
+
+
+@property
+def file(self):
+"""Get the file represented by this source location."""
+return self._file
+
+@property
+def line(self):
+"""Get the line represented by this source location."""
+return self._line
+
+@property
+def column(self):
+"""Get the column represented by this source location."""
+return self._column
+
+@property
+def offset(self):
+"""Get the file offset represented by this source location."""
+return self._offset
 
 class SourceLocation(Structure):
 """
 A SourceLocation represents a particular location within a source file.
 """
 _fields_ = [("ptr_data", c_void_p * 2), ("int_data", c_uint)]
-_data = None
-
-def _get_instantiation(self):
-if self._data is None:
-f, l, c, o = c_object_p(), c_uint(), c_uint(), c_uint()
-conf.lib.clang_getInstantiationLocation(self, byref(f), byref(l),
-byref(c), byref(o))
-if f:
-f = File(f)
-else:
-f = None
-self._data = (f, int(l.value), int(c.value), int(o.value))
-return self._data
+_expansion = None
+_spelling = None
 
 @staticmethod
 def from_position(tu, file, line, column):
@@ -253,24 +273,72 @@ class SourceLocation(Structure):
 return conf.lib.clang_getLocationForOffset(tu, file, offset)
 
 @property
+def expansion(self):
+"""
+The source location where then entity this object is referring to is
+expanded.
+"""
+if not self._expansion:
+file   = c_object_p()
+line   = c_uint()
+column = c_uint()
+offset = c_uint()
+conf.lib.clang_getExpansionLocation(self,
+byref(file),
+byref(line),
+byref(column),
+byref(offset))
+
+self._expansion = Location(file, line, column, offset)
+return self._expansion
+
+@property
+def spelling(self):
+"""
+The source location where then entity this object is referring to is
+written.
+"""
+if not self._spelling:
+file   = c_object_p()
+line   = c_uint()
+column = c_uint()
+offset = c_uint()
+conf.lib.clang_getSpellingLocation(self,
+   byref(file),
+   byref(line),
+   byref(column),
+   byref(offset))
+
+self._spelling = Location(file, line, column, offset)
+return self._spelling
+
+@property
 def file(self):
-"""Get the file represented by this source location."""
-return self._get_instantiation()[0]
+"""Get the file represented by this source location.
+
+   

[PATCH] [bindings] fix TLS test failure

2017-10-21 Thread Masud Rahman via cfe-commits

Since cfe commit r237337, '__declspec(thread)' and 'thread_local' have
been the same since MSVC 2015.  i.e. they are both considered to supply
a dynamic TLS kind, not a static TLS kind.

This test originally did not specify which version of MS compatibility
to assume.  As a result, the test was brittle, since changing the
default compatibility version could break the test.

This commit adds a specific version when building up the flags used to
parse the translation unit, and tests both versions.
---
 bindings/python/tests/cindex/test_tls_kind.py | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/bindings/python/tests/cindex/test_tls_kind.py b/bindings/python/tests/cindex/test_tls_kind.py
index 6a03c0d5ee..d0ee4587bc 100644
--- a/bindings/python/tests/cindex/test_tls_kind.py
+++ b/bindings/python/tests/cindex/test_tls_kind.py
@@ -27,11 +27,21 @@ _Thread_local int tls_static;
 # The following case tests '__declspec(thread)'.  Since it is a Microsoft
 # specific extension, specific flags are required for the parser to pick
 # these up.
-flags = ['-fms-extensions', '-target', 'x86_64-unknown-windows-win32']
+flags = ['-fms-extensions', '-target', 'x86_64-unknown-windows-win32',
+ '-fms-compatibility-version=18']
 tu = get_tu("""
-__declspec(thread) int tls_declspec;
+__declspec(thread) int tls_declspec_msvc18;
 """, lang = 'cpp', flags=flags)
 
-tls_declspec = get_cursor(tu.cursor, 'tls_declspec')
-assert tls_declspec.tls_kind == TLSKind.STATIC
+tls_declspec_msvc18 = get_cursor(tu.cursor, 'tls_declspec_msvc18')
+assert tls_declspec_msvc18.tls_kind == TLSKind.STATIC
+
+flags = ['-fms-extensions', '-target', 'x86_64-unknown-windows-win32',
+ '-fms-compatibility-version=19']
+tu = get_tu("""
+__declspec(thread) int tls_declspec_msvc19;
+""", lang = 'cpp', flags=flags)
+
+tls_declspec_msvc19 = get_cursor(tu.cursor, 'tls_declspec_msvc19')
+assert tls_declspec_msvc19.tls_kind == TLSKind.DYNAMIC
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r316275 - Fix a typo with -fno-double-square-bracket-attributes and add a test to demonstrate that it works as expected in C++11 mode. Additionally corrected the handling of -fdouble-square-bracket-at

2017-10-21 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Sat Oct 21 13:28:58 2017
New Revision: 316275

URL: http://llvm.org/viewvc/llvm-project?rev=316275=rev
Log:
Fix a typo with -fno-double-square-bracket-attributes and add a test to 
demonstrate that it works as expected in C++11 mode. Additionally corrected the 
handling of -fdouble-square-bracket-attributes to be properly passed down to 
the cc1 option.

Added:
cfe/trunk/test/SemaCXX/attr-cxx-disabled.cpp
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=316275=316274=316275=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Sat Oct 21 13:28:58 2017
@@ -613,8 +613,8 @@ def fasynchronous_unwind_tables : Flag<[
 def fdouble_square_bracket_attributes : Flag<[ "-" ], 
"fdouble-square-bracket-attributes">,
   Group, Flags<[DriverOption, CC1Option]>,
   HelpText<"Enable '[[]]' attributes in all C and C++ language modes">;
-def fno_double_square_bracket_attributes : Flag<[ "-" ], 
"fno-fdouble-square-bracket-attributes">,
-  Group, Flags<[DriverOption]>,
+def fno_double_square_bracket_attributes : Flag<[ "-" ], 
"fno-double-square-bracket-attributes">,
+  Group, Flags<[DriverOption, CC1Option]>,
   HelpText<"Disable '[[]]' attributes in all C and C++ language modes">;
 
 def fautolink : Flag <["-"], "fautolink">, Group;

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=316275=316274=316275=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Sat Oct 21 13:28:58 2017
@@ -4010,6 +4010,9 @@ void Clang::ConstructJob(Compilation ,
 CmdArgs.push_back("-fcoroutines-ts");
   }
 
+  Args.AddLastArg(CmdArgs, options::OPT_fdouble_square_bracket_attributes,
+  options::OPT_fno_double_square_bracket_attributes);
+
   bool HaveModules = false;
   RenderModulesOptions(C, D, Args, Input, Output, CmdArgs, HaveModules);
 

Added: cfe/trunk/test/SemaCXX/attr-cxx-disabled.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-cxx-disabled.cpp?rev=316275=auto
==
--- cfe/trunk/test/SemaCXX/attr-cxx-disabled.cpp (added)
+++ cfe/trunk/test/SemaCXX/attr-cxx-disabled.cpp Sat Oct 21 13:28:58 2017
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -fno-double-square-bracket-attributes -verify 
-pedantic -std=c++11 -DERRORS %s
+// RUN: %clang_cc1 -fsyntax-only -fdouble-square-bracket-attributes -verify 
-pedantic -std=c++11 %s
+
+struct [[]] S {};
+
+#ifdef ERRORS
+// expected-error@-3 {{declaration of anonymous struct must be a definition}}
+// expected-warning@-4 {{declaration does not declare anything}}
+#else
+// expected-no-diagnostics
+#endif
+


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


[PATCH] D37855: [bindings] allow null strings in Python 3

2017-10-21 Thread Masud Rahman via Phabricator via cfe-commits
frutiger closed this revision.
frutiger added a comment.

Landed in https://reviews.llvm.org/rL316264.


https://reviews.llvm.org/D37855



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


[PATCH] D36101: Fix usage of right shift operator in fold expressions

2017-10-21 Thread Nicolas Lesser via Phabricator via cfe-commits
Rakete updated this revision to Diff 119767.
Rakete added a comment.

Used the naming convention of LLVM + friendly ping. :)


https://reviews.llvm.org/D36101

Files:
  lib/Parse/ParseExpr.cpp
  test/Parser/cxx1z-fold-expressions.cpp


Index: test/Parser/cxx1z-fold-expressions.cpp
===
--- test/Parser/cxx1z-fold-expressions.cpp
+++ test/Parser/cxx1z-fold-expressions.cpp
@@ -43,3 +43,8 @@
 (int)(undeclared_junk + ...) + // expected-error {{undeclared}}
 (int)(a + ...); // expected-error {{does not contain any unexpanded}}
 }
+
+// fold-operator can be '>' or '>>' too.
+template bool greaterThan() { return (N > ...); }
+template int rightShift() { return (N >> ...); }
+
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -270,7 +270,7 @@
   return Level > prec::Unknown && Level != prec::Conditional;
 }
 static bool isFoldOperator(tok::TokenKind Kind) {
-  return isFoldOperator(getBinOpPrecedence(Kind, false, true));
+  return isFoldOperator(getBinOpPrecedence(Kind, true, true));
 }
 
 /// \brief Parse a binary expression that starts with \p LHS and has a


Index: test/Parser/cxx1z-fold-expressions.cpp
===
--- test/Parser/cxx1z-fold-expressions.cpp
+++ test/Parser/cxx1z-fold-expressions.cpp
@@ -43,3 +43,8 @@
 (int)(undeclared_junk + ...) + // expected-error {{undeclared}}
 (int)(a + ...); // expected-error {{does not contain any unexpanded}}
 }
+
+// fold-operator can be '>' or '>>' too.
+template bool greaterThan() { return (N > ...); }
+template int rightShift() { return (N >> ...); }
+
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -270,7 +270,7 @@
   return Level > prec::Unknown && Level != prec::Conditional;
 }
 static bool isFoldOperator(tok::TokenKind Kind) {
-  return isFoldOperator(getBinOpPrecedence(Kind, false, true));
+  return isFoldOperator(getBinOpPrecedence(Kind, true, true));
 }
 
 /// \brief Parse a binary expression that starts with \p LHS and has a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-10-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 119766.
lebedev.ri marked 28 inline comments as done.
lebedev.ri added a comment.

Rebased.
Addressed @aaron.ballman review notes (mainly stylistic)


Repository:
  rL LLVM

https://reviews.llvm.org/D36836

Files:
  LICENSE.TXT
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/FunctionCognitiveComplexityCheck.LICENSE.txt
  clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
  clang-tidy/readability/FunctionCognitiveComplexityCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
  test/clang-tidy/readability-function-cognitive-complexity.cpp

Index: test/clang-tidy/readability-function-cognitive-complexity.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-function-cognitive-complexity.cpp
@@ -0,0 +1,952 @@
+// RUN: %check_clang_tidy %s readability-function-cognitive-complexity %t -- -config='{CheckOptions: [{key: readability-function-cognitive-complexity.Threshold, value: 0}]}' -- -std=c++11
+
+// any function should be checked.
+
+extern int ext_func(int x = 0);
+
+int some_func(int x = 0);
+
+static int some_other_func(int x = 0) {}
+
+template void some_templ_func(T x = 0) {}
+
+class SomeClass {
+public:
+  int *begin(int x = 0);
+  int *end(int x = 0);
+  static int func(int x = 0);
+  template void some_templ_func(T x = 0) {}
+};
+
+// nothing ever decreases cognitive complexity, so we can check all the things
+// in one go. none of the following should increase cognitive complexity:
+void unittest_false() {
+  {};
+  ext_func();
+  some_func();
+  some_other_func();
+  some_templ_func();
+  some_templ_func();
+  SomeClass::func();
+  SomeClass C;
+  C.some_templ_func();
+  C.some_templ_func();
+  C.func();
+  C.end();
+  int i = some_func();
+  i = i;
+  i++;
+  --i;
+  i < 0;
+  int j = 0 ?: 1;
+  auto k = new int;
+  delete k;
+  throw i;
+  {
+throw i;
+  }
+end:
+  return;
+}
+
+#if 1
+#define CC100
+#else
+// this macro has cognitive complexity of 100.
+// it is needed to be able to compare the testcases with the
+// reference Sonar implementation. please place it right after the first
+// CHECK-NOTES in each function
+#define CC100 if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){}if(1){}
+#endif
+
+////
+//-- B1. Increments --//
+////
+// Check that every thing listed in B1 of the specification does indeed   //
+// recieve the base increment, and that not-body does not increase nesting//
+////
+
+// break does not increase cognitive complexity.
+// only  break LABEL  does, but it is unavaliable in C or C++
+
+// continue does not increase cognitive complexity.
+// only  continue LABEL  does, but it is unavaliable in C or C++
+
+void unittest_b1_00() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_00' has cognitive complexity of 33 (threshold 0) [readability-function-cognitive-complexity]
+  CC100;
+
+  if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:9: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+
+if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:11: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+} else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:12: note: +1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:18: note: +3, including nesting penalty of 2, nesting level increased to 3{{$}}
+} else {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +1, nesting level increased to 2{{$}}
+}
+  } else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:10: note: +1, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:16: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+
+if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:11: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+} else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:12: note: +1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:18: note: +3, including nesting penalty of 2, nesting level increased to 3{{$}}
+} else {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +1, nesting level increased to 2{{$}}
+}
+  } else {
+// CHECK-NOTES: :[[@LINE-1]]:5: 

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-10-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:102
+const unsigned short Nesting; /// How deeply nested is Loc located?
+const Criteria C : 3; /// The criteria of the increment
+

aaron.ballman wrote:
> lebedev.ri wrote:
> > aaron.ballman wrote:
> > > Why is this turned into a bit-field? If that is important, it should be 
> > > of type `unsigned` to prevent differences between compilers (MSVC treats 
> > > bit-fields differently than GCC and Clang).
> > The comment right after this member variable should have explained it:
> > ```
> > /// Criteria C is a bitfield. Even though Criteria is an unsigned char; and
> > /// only using 3 bits will still result in padding, the fact that it is a
> > /// bitfield is a reminder that it is important to min(sizeof(Detail))
> > ```
> > It is already `unsigned`:
> > ```
> > enum Criteria : unsigned char {
> > ```
> No, it's underlying type is `unsigned char`, but the type is `Criteria`. I 
> meant the field itself needs to be `unsigned`. However, since that means you 
> have to do more type casting, I think making the type `uint8_t` instead of a 
> bit-field would be an improvement.
Better?



Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:118
+
+  unsigned MsgId;   /// The id of the message to output
+  unsigned short Increment; /// How much of an increment?

aaron.ballman wrote:
> aaron.ballman wrote:
> > We use `//` instead of `///` unless we're specifically documenting 
> > something for doxygen.
> This change is marked as done but wasn't applied consistently. For instance, 
> see lines 130-131 where it switches between uses. I would use `//` 
> consistently everywhere unless documenting a public interface in a header.
OK.



Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:495
+  Finder->addMatcher(
+  functionDecl(unless(anyOf(isInstantiated(), isImplicit(.bind("func"),
+  this);

aaron.ballman wrote:
> You only need to match function *definitions* (not declarations), correct? It 
> might be useful to specify that in the matcher so that it matches less code.
Good idea!


Repository:
  rL LLVM

https://reviews.llvm.org/D36836



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


[PATCH] D38831: [libcxx] P0604, invoke_result and is_invocable

2017-10-21 Thread Agustín Bergé via Phabricator via cfe-commits
K-ballo updated this revision to Diff 119764.
K-ballo added a comment.

Fix synopsis


https://reviews.llvm.org/D38831

Files:
  include/type_traits
  include/variant
  test/std/utilities/function.objects/func.not_fn/not_fn.pass.cpp
  test/std/utilities/function.objects/unord.hash/non_enum.pass.cpp
  test/std/utilities/meta/meta.rel/is_callable.pass.cpp
  test/std/utilities/meta/meta.rel/is_invocable.pass.cpp
  test/std/utilities/meta/meta.rel/is_nothrow_callable.pass.cpp
  test/std/utilities/meta/meta.rel/is_nothrow_invocable.pass.cpp
  test/std/utilities/meta/meta.trans/meta.trans.other/result_of.pass.cpp
  test/std/utilities/meta/meta.trans/meta.trans.other/result_of11.pass.cpp

Index: test/std/utilities/meta/meta.trans/meta.trans.other/result_of11.pass.cpp
===
--- test/std/utilities/meta/meta.trans/meta.trans.other/result_of11.pass.cpp
+++ test/std/utilities/meta/meta.trans/meta.trans.other/result_of11.pass.cpp
@@ -27,6 +27,23 @@
 struct F {};
 struct FD : public F {};
 
+#if TEST_STD_VER > 14
+template 
+struct test_invoke_result;
+
+template 
+struct test_invoke_result
+{
+static void call()
+{
+static_assert(std::is_invocable::value, "");
+static_assert(std::is_invocable_r::value, "");
+static_assert((std::is_same::type, Ret>::value), "");
+static_assert((std::is_same, Ret>::value), "");
+}
+};
+#endif
+
 template 
 void test_result_of_imp()
 {
@@ -35,8 +52,7 @@
 static_assert((std::is_same::value), "");
 #endif
 #if TEST_STD_VER > 14
-static_assert(std::is_callable::value, "");
-static_assert(std::is_callable::value, "");
+test_invoke_result::call();
 #endif
 }
 
Index: test/std/utilities/meta/meta.trans/meta.trans.other/result_of.pass.cpp
===
--- test/std/utilities/meta/meta.trans/meta.trans.other/result_of.pass.cpp
+++ test/std/utilities/meta/meta.trans/meta.trans.other/result_of.pass.cpp
@@ -42,16 +42,46 @@
 template 
 struct HasType : std::true_type {};
 
+#if TEST_STD_VER > 14
+template 
+struct test_invoke_result;
+
+template 
+struct test_invoke_result
+{
+static void call()
+{
+static_assert(std::is_invocable::value, "");
+static_assert(std::is_invocable_r::value, "");
+static_assert((std::is_same::type, Ret>::value), "");
+}
+};
+#endif
+
 template 
 void test_result_of()
 {
+static_assert((std::is_same::type, U>::value), "");
 #if TEST_STD_VER > 14
-static_assert(std::is_callable::value, "");
-static_assert(std::is_callable::value, "");
+test_invoke_result::call();
 #endif
-static_assert((std::is_same::type, U>::value), "");
 }
 
+#if TEST_STD_VER > 14
+template 
+struct test_invoke_no_result;
+
+template 
+struct test_invoke_no_result
+{
+static void call()
+{
+static_assert(std::is_invocable::value == false, "");
+static_assert((!HasType >::value), "");
+}
+};
+#endif
+
 template 
 void test_no_result()
 {
@@ -59,7 +89,7 @@
 static_assert((!HasType::value), "");
 #endif
 #if TEST_STD_VER > 14
-static_assert(std::is_callable::value == false, "");
+test_invoke_no_result::call();
 #endif
 }
 
Index: test/std/utilities/meta/meta.rel/is_nothrow_invocable.pass.cpp
===
--- /dev/null
+++ test/std/utilities/meta/meta.rel/is_nothrow_invocable.pass.cpp
@@ -0,0 +1,121 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+// type_traits
+
+// is_nothrow_invocable
+
+#include 
+#include 
+
+#include "test_macros.h"
+
+struct Tag {};
+
+struct Implicit {
+  Implicit(int) noexcept {}
+};
+
+struct ThrowsImplicit {
+  ThrowsImplicit(int) {}
+};
+
+struct Explicit {
+  explicit Explicit(int) noexcept {}
+};
+
+template 
+struct CallObject {
+  Ret operator()(Args&&...) const noexcept(IsNoexcept);
+};
+
+template 
+constexpr bool throws_invocable() {
+return std::is_invocable::value &&
+!std::is_nothrow_invocable::value;
+}
+
+template 
+constexpr bool throws_invocable_r() {
+return std::is_invocable_r::value &&
+!std::is_nothrow_invocable_r::value;
+}
+
+// FIXME(EricWF) Don't test the where noexcept is *not* part of the type system
+// once 

[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316268: [Sema] Fixes for enum handling for tautological 
comparison diagnostics (authored by lebedevri).

Changed prior to commit:
  https://reviews.llvm.org/D39122?vs=119759=119760#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39122

Files:
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/Sema/outof-range-enum-constant-compare.c
  cfe/trunk/test/Sema/tautological-constant-enum-compare.c
  cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c
  cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.cpp

Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -8181,8 +8181,12 @@
 if (const AtomicType *AT = dyn_cast(T))
   T = AT->getValueType().getTypePtr();
 
-// For enum types, use the known bit width of the enumerators.
-if (const EnumType *ET = dyn_cast(T)) {
+if (!C.getLangOpts().CPlusPlus) {
+  // For enum types in C code, use the underlying datatype.
+  if (const EnumType *ET = dyn_cast(T))
+T = ET->getDecl()->getIntegerType().getDesugaredType(C).getTypePtr();
+} else if (const EnumType *ET = dyn_cast(T)) {
+  // For enum types in C++, use the known bit width of the enumerators.
   EnumDecl *Enum = ET->getDecl();
   // In C++11, enums without definitions can have an explicitly specified
   // underlying type.  Use this type to compute the range.
@@ -8584,8 +8588,10 @@
 }
 
 enum class LimitType {
-  Max, // e.g. 32767 for short
-  Min  // e.g. -32768 for short
+  Max = 1U << 0U,  // e.g. 32767 for short
+  Min = 1U << 1U,  // e.g. -32768 for short
+  Both = Max | Min // When the value is both the Min and the Max limit at the
+   // same time; e.g. in C++, A::a in enum A { a = 0 };
 };
 
 /// Checks whether Expr 'Constant' may be the
@@ -8608,6 +8614,10 @@
 
   IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
 
+  // Special-case for C++ for enum with one enumerator with value of 0.
+  if (OtherRange.Width == 0)
+return Value == 0 ? LimitType::Both : llvm::Optional();
+
   if (llvm::APSInt::isSameValue(
   llvm::APSInt::getMaxValue(OtherRange.Width,
 OtherT->isUnsignedIntegerType()),
@@ -8620,7 +8630,7 @@
   Value))
 return LimitType::Min;
 
-  return llvm::Optional();
+  return llvm::None;
 }
 
 bool HasEnumType(Expr *E) {
@@ -8655,9 +8665,12 @@
 
   bool ConstIsLowerBound = (Op == BO_LT || Op == BO_LE) ^ RhsConstant;
   bool ResultWhenConstEqualsOther = (Op == BO_LE || Op == BO_GE);
-  bool ResultWhenConstNeOther =
-  ConstIsLowerBound ^ (ValueType == LimitType::Max);
-  if (ResultWhenConstEqualsOther != ResultWhenConstNeOther)
+  if (ValueType != LimitType::Both) {
+bool ResultWhenConstNeOther =
+ConstIsLowerBound ^ (ValueType == LimitType::Max);
+if (ResultWhenConstEqualsOther != ResultWhenConstNeOther)
+  return false; // The comparison is not tautological.
+  } else if (ResultWhenConstEqualsOther == ConstIsLowerBound)
 return false; // The comparison is not tautological.
 
   const bool Result = ResultWhenConstEqualsOther;
Index: cfe/trunk/test/Sema/outof-range-enum-constant-compare.c
===
--- cfe/trunk/test/Sema/outof-range-enum-constant-compare.c
+++ cfe/trunk/test/Sema/outof-range-enum-constant-compare.c
@@ -0,0 +1,379 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -verify %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify %s
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -DSILENCE -Wno-tautological-constant-out-of-range-compare -verify %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -DSILENCE -Wno-tautological-constant-out-of-range-compare -verify %s
+
+int main() {
+  enum A { A_a = 2 };
+  enum A a;
+
+#ifdef SILENCE
+  // expected-no-diagnostics
+#endif
+
+#ifdef UNSIGNED
+#ifndef SILENCE
+  if (a < 4294967296) // expected-warning {{comparison of constant 4294967296 with expression of type 'enum A' is always true}}
+return 0;
+  if (4294967296 >= a) // expected-warning {{comparison of constant 4294967296 with expression of type 'enum A' is always true}}
+return 0;
+  if (a > 4294967296) // expected-warning {{comparison of constant 4294967296 with expression of type 'enum A' is always false}}
+return 0;
+  if (4294967296 <= a) // expected-warning {{comparison of constant 4294967296 with expression of type 'enum A' is always false}}
+return 0;
+  if (a <= 4294967296) // expected-warning {{comparison of constant 4294967296 with expression of type 'enum A' is always true}}
+return 0;
+  if (4294967296 > a) // expected-warning {{comparison of constant 4294967296 with expression of 

r316269 - Add release notes for the recent -fdouble-square-bracket-attributes and -fno-double-square-bracket-attributes compiler flags.

2017-10-21 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Sat Oct 21 09:45:08 2017
New Revision: 316269

URL: http://llvm.org/viewvc/llvm-project?rev=316269=rev
Log:
Add release notes for the recent -fdouble-square-bracket-attributes and 
-fno-double-square-bracket-attributes compiler flags.

Modified:
cfe/trunk/docs/ReleaseNotes.rst

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=316269=316268=316269=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Sat Oct 21 09:45:08 2017
@@ -103,6 +103,13 @@ New Compiler Flags
 
 - --autocomplete was implemented to obtain a list of flags and its arguments. 
This is used for shell autocompletion.
 
+- The ``-fdouble-square-bracket-attributes`` and corresponding
+  ``-fno-double-square-bracket-attributes`` flags were added to enable or
+  disable [[]] attributes in any language mode. Currently, only a limited
+  number of attributes are supported outside of C++ mode. See the Clang
+  attribute documentation for more information about which attributes are
+  supported for each syntax.
+
 Deprecated Compiler Flags
 -
 


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


r316268 - [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Roman Lebedev via cfe-commits
Author: lebedevri
Date: Sat Oct 21 09:44:03 2017
New Revision: 316268

URL: http://llvm.org/viewvc/llvm-project?rev=316268=rev
Log:
[Sema] Fixes for enum handling for tautological comparison diagnostics

Summary:
As Mattias Eriksson has reported in PR35009, in C, for enums, the underlying 
type should
be used when checking for the tautological comparison, unlike C++, where the 
enumerator
values define the value range. So if not in CPlusPlus mode, use the enum 
underlying type.

Also, i have discovered a problem (a crash) when evaluating tautological-ness 
of the following comparison:
```
enum A { A_a = 0 };
if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is 
always false}}
return 0;
```
This affects both the C and C++, but after the first fix, only C++ code was 
affected.
That was also fixed, while preserving (i think?) the proper diagnostic output.

And while there, attempt to enhance the test coverage.
Yes, some tests got moved around, sorry about that :)

Fixes PR35009

Reviewers: aaron.ballman, rsmith, rjmccall

Reviewed By: aaron.ballman

Subscribers: Rakete, efriedma, materi, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/Sema/outof-range-enum-constant-compare.c
cfe/trunk/test/Sema/tautological-constant-enum-compare.c
Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c
cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.cpp

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=316268=316267=316268=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Sat Oct 21 09:44:03 2017
@@ -8181,8 +8181,12 @@ struct IntRange {
 if (const AtomicType *AT = dyn_cast(T))
   T = AT->getValueType().getTypePtr();
 
-// For enum types, use the known bit width of the enumerators.
-if (const EnumType *ET = dyn_cast(T)) {
+if (!C.getLangOpts().CPlusPlus) {
+  // For enum types in C code, use the underlying datatype.
+  if (const EnumType *ET = dyn_cast(T))
+T = ET->getDecl()->getIntegerType().getDesugaredType(C).getTypePtr();
+} else if (const EnumType *ET = dyn_cast(T)) {
+  // For enum types in C++, use the known bit width of the enumerators.
   EnumDecl *Enum = ET->getDecl();
   // In C++11, enums without definitions can have an explicitly specified
   // underlying type.  Use this type to compute the range.
@@ -8584,8 +8588,10 @@ bool isNonBooleanUnsignedValue(Expr *E)
 }
 
 enum class LimitType {
-  Max, // e.g. 32767 for short
-  Min  // e.g. -32768 for short
+  Max = 1U << 0U,  // e.g. 32767 for short
+  Min = 1U << 1U,  // e.g. -32768 for short
+  Both = Max | Min // When the value is both the Min and the Max limit at the
+   // same time; e.g. in C++, A::a in enum A { a = 0 };
 };
 
 /// Checks whether Expr 'Constant' may be the
@@ -8608,6 +8614,10 @@ llvm::Optional IsTypeLimit(Se
 
   IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
 
+  // Special-case for C++ for enum with one enumerator with value of 0.
+  if (OtherRange.Width == 0)
+return Value == 0 ? LimitType::Both : llvm::Optional();
+
   if (llvm::APSInt::isSameValue(
   llvm::APSInt::getMaxValue(OtherRange.Width,
 OtherT->isUnsignedIntegerType()),
@@ -8620,7 +8630,7 @@ llvm::Optional IsTypeLimit(Se
   Value))
 return LimitType::Min;
 
-  return llvm::Optional();
+  return llvm::None;
 }
 
 bool HasEnumType(Expr *E) {
@@ -8655,9 +8665,12 @@ bool CheckTautologicalComparison(Sema 
 
   bool ConstIsLowerBound = (Op == BO_LT || Op == BO_LE) ^ RhsConstant;
   bool ResultWhenConstEqualsOther = (Op == BO_LE || Op == BO_GE);
-  bool ResultWhenConstNeOther =
-  ConstIsLowerBound ^ (ValueType == LimitType::Max);
-  if (ResultWhenConstEqualsOther != ResultWhenConstNeOther)
+  if (ValueType != LimitType::Both) {
+bool ResultWhenConstNeOther =
+ConstIsLowerBound ^ (ValueType == LimitType::Max);
+if (ResultWhenConstEqualsOther != ResultWhenConstNeOther)
+  return false; // The comparison is not tautological.
+  } else if (ResultWhenConstEqualsOther == ConstIsLowerBound)
 return false; // The comparison is not tautological.
 
   const bool Result = ResultWhenConstEqualsOther;

Added: cfe/trunk/test/Sema/outof-range-enum-constant-compare.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/outof-range-enum-constant-compare.c?rev=316268=auto
==
--- cfe/trunk/test/Sema/outof-range-enum-constant-compare.c (added)
+++ cfe/trunk/test/Sema/outof-range-enum-constant-compare.c Sat Oct 21 09:44:03 
2017
@@ -0,0 +1,379 @@
+// RUN: %clang_cc1 

r316267 - Fixing broken attribute documentation for __attribute__((noescape)); a code block was missing and the existing code block was missing a mandatory newline.

2017-10-21 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Sat Oct 21 09:43:01 2017
New Revision: 316267

URL: http://llvm.org/viewvc/llvm-project?rev=316267=rev
Log:
Fixing broken attribute documentation for __attribute__((noescape)); a code 
block was missing and the existing code block was missing a mandatory newline.

Modified:
cfe/trunk/include/clang/Basic/AttrDocs.td

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=316267=316266=316267=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Sat Oct 21 09:43:01 2017
@@ -142,6 +142,7 @@ annotated with ``noescape`` do not actua
 For example:
 
 .. code-block:: c
+
   int *gp;
 
   void nonescapingFunc(__attribute__((noescape)) int *p) {
@@ -156,6 +157,8 @@ Additionally, when the parameter is a `b
 `, the same restriction
 applies to copies of the block. For example:
 
+.. code-block:: c
+
   typedef void (^BlockTy)();
   BlockTy g0, g1;
 


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


[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!




Comment at: lib/Sema/SemaChecking.cpp:8619
+  if (OtherRange.Width == 0)
+return Value == 0 ? LimitType::Both : llvm::Optional();
+

lebedev.ri wrote:
> aaron.ballman wrote:
> > Instead of default constructing the Optional, you should use `llvm::None` 
> > instead.
> Thank you for the suggestion, i did not really know about `llvm::None`.
> However only the last `return` can be enhanced:
> ```
> clang/lib/Sema/SemaChecking.cpp:8619:23: error: incompatible operand types 
> ('(anonymous namespace)::LimitType' and 'llvm::NoneType')
> return Value == 0 ? LimitType::Both : llvm::None;
>   ^ ~~~   ~~
> ```
> 
Blech, that's because there's no common type for the `:?` operator. I think 
it'd be worse to split this into multiple statements just to use `llvm::None`, 
so it's fine as-is.


Repository:
  rL LLVM

https://reviews.llvm.org/D39122



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


r316263 - Test commit

2017-10-21 Thread Masud Rahman via cfe-commits
Author: frutiger
Date: Sat Oct 21 09:03:17 2017
New Revision: 316263

URL: http://llvm.org/viewvc/llvm-project?rev=316263=rev
Log:
Test commit

Modified:
cfe/trunk/README.txt

Modified: cfe/trunk/README.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/README.txt?rev=316263=316262=316263=diff
==
--- cfe/trunk/README.txt (original)
+++ cfe/trunk/README.txt Sat Oct 21 09:03:17 2017
@@ -13,10 +13,10 @@ different source-level tools.  One examp
 If you're interested in more (including how to build Clang) it is best to read
 the relevant web sites.  Here are some pointers:
 
-Information on Clang:  http://clang.llvm.org/
-Building and using Clang:  http://clang.llvm.org/get_started.html
-Clang Static Analyzer: http://clang-analyzer.llvm.org/
-Information on the LLVM project:   http://llvm.org/
+Information on Clang: http://clang.llvm.org/
+Building and using Clang: http://clang.llvm.org/get_started.html
+Clang Static Analyzer:http://clang-analyzer.llvm.org/
+Information on the LLVM project:  http://llvm.org/
 
 If you have questions or comments about Clang, a great place to discuss them is
 on the Clang development mailing list:


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


r316264 - [bindings] allow null strings in Python 3

2017-10-21 Thread Masud Rahman via cfe-commits
Author: frutiger
Date: Sat Oct 21 09:13:41 2017
New Revision: 316264

URL: http://llvm.org/viewvc/llvm-project?rev=316264=rev
Log:
[bindings] allow null strings in Python 3

Some API calls accept 'NULL' instead of a char array (e.g. the second
argument to 'clang_ParseTranslationUnit').  For Python 3 compatibility,
all strings are passed through 'c_interop_string' which expects to
receive only 'bytes' or 'str' objects.  This change extends this
behavior to additionally allow 'None' to be supplied.

A test case was added which breaks in Python 3, and is fixed by this
change.  All the test cases pass in both, Python 2 and Python 3.

Modified:
cfe/trunk/bindings/python/clang/cindex.py
cfe/trunk/bindings/python/tests/cindex/test_index.py

Modified: cfe/trunk/bindings/python/clang/cindex.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/clang/cindex.py?rev=316264=316263=316264=diff
==
--- cfe/trunk/bindings/python/clang/cindex.py (original)
+++ cfe/trunk/bindings/python/clang/cindex.py Sat Oct 21 09:13:41 2017
@@ -94,6 +94,9 @@ if sys.version_info[0] == 3:
 return cls(param)
 if isinstance(param, bytes):
 return cls(param)
+if param is None:
+# Support passing null to C functions expecting char arrays
+return None
 raise TypeError("Cannot convert '{}' to 
'{}'".format(type(param).__name__, cls.__name__))
 
 @staticmethod

Modified: cfe/trunk/bindings/python/tests/cindex/test_index.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/tests/cindex/test_index.py?rev=316264=316263=316264=diff
==
--- cfe/trunk/bindings/python/tests/cindex/test_index.py (original)
+++ cfe/trunk/bindings/python/tests/cindex/test_index.py Sat Oct 21 09:13:41 
2017
@@ -13,3 +13,5 @@ def test_parse():
 assert isinstance(index, Index)
 tu = index.parse(os.path.join(kInputsDir, 'hello.cpp'))
 assert isinstance(tu, TranslationUnit)
+tu = index.parse(None, ['-c', os.path.join(kInputsDir, 'hello.cpp')])
+assert isinstance(tu, TranslationUnit)


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


[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 119759.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.

Address review notes.


Repository:
  rL LLVM

https://reviews.llvm.org/D39122

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/outof-range-enum-constant-compare.c
  test/Sema/tautological-constant-enum-compare.c
  test/Sema/tautological-unsigned-enum-zero-compare.c
  test/Sema/tautological-unsigned-enum-zero-compare.cpp

Index: test/Sema/tautological-unsigned-enum-zero-compare.cpp
===
--- test/Sema/tautological-unsigned-enum-zero-compare.cpp
+++ test/Sema/tautological-unsigned-enum-zero-compare.cpp
@@ -1,176 +1,347 @@
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only -DALL_WARN -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSIGN_WARN -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -Wno-tautological-unsigned-enum-zero-compare -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSILENCE -Wno-tautological-unsigned-enum-zero-compare -verify %s
 
 // Okay, this is where it gets complicated.
 // Then default enum sigdness is target-specific.
 // On windows, it is signed by default. We do not want to warn in that case.
 
 int main() {
-  enum A { A_foo, A_bar };
+  enum A { A_foo = 0, A_bar, };
   enum A a;
 
-  enum B : unsigned { B_foo, B_bar };
+  enum B : unsigned { B_foo = 0, B_bar, };
   enum B b;
 
-  enum C : signed { c_foo, c_bar };
+  enum C : signed { C_foo = 0, C_bar, };
   enum C c;
 
-#ifdef ALL_WARN
+#ifdef UNSIGNED
   if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
 return 0;
-  if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  if (0 >= a)
+return 0;
+  if (a > 0)
 return 0;
   if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
 return 0;
+  if (a <= 0)
+return 0;
   if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
 return 0;
+  if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+return 0;
+  if (0 < a)
+return 0;
+
   if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
 return 0;
-  if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  if (0U >= a)
+return 0;
+  if (a > 0U)
 return 0;
   if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
 return 0;
+  if (a <= 0U)
+return 0;
   if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
 return 0;
+  if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+return 0;
+  if (0U < a)
+return 0;
 
   if (b < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
 return 0;
-  if (b >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  if (0 >= b)
+return 0;
+  if (b > 0)
 return 0;
   if (0 <= b) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
 return 0;
+  if (b <= 0)
+return 0;
   if (0 > b) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
 return 0;
-  if (b < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
-return 0;
-  if (b >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
-return 0;
-  if (0U <= b) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
-return 0;
-  if (0U > b) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
-return 0;
-
-  if (c < 0) // ok
-return 0;
-  if (c >= 0) // ok
-return 0;
-  if (0 <= c) // ok
-return 0;
-  if (0 > c) // ok
-return 0;
-  if (c < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
-return 0;
-  if (c >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
-return 0;
-  if (0U <= c) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
-return 0;
-  if (0U > c) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
-return 0;
-#elif defined(SIGN_WARN)
-  if (a < 0) // ok
-return 0;
-  if (a >= 0) // ok
-return 0;
-  if (0 <= a) // ok
-return 0;
-  if (0 > a) // ok
-return 0;
-  if (a < 0U) // expected-warning {{comparison of unsigned enum 

[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:8619
+  if (OtherRange.Width == 0)
+return Value == 0 ? LimitType::Both : llvm::Optional();
+

aaron.ballman wrote:
> Instead of default constructing the Optional, you should use `llvm::None` 
> instead.
Thank you for the suggestion, i did not really know about `llvm::None`.
However only the last `return` can be enhanced:
```
clang/lib/Sema/SemaChecking.cpp:8619:23: error: incompatible operand types 
('(anonymous namespace)::LimitType' and 'llvm::NoneType')
return Value == 0 ? LimitType::Both : llvm::None;
  ^ ~~~   ~~
```



Repository:
  rL LLVM

https://reviews.llvm.org/D39122



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


[PATCH] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

2017-10-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

I am okay with this direction but would still like @alexfh to accept before you 
commit.


Repository:
  rL LLVM

https://reviews.llvm.org/D36892



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


[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:8186
+  // For enum types, for C code, use underlying data type.
+  if (const EnumType *ET = dyn_cast(T))
+T = ET->getDecl()->getIntegerType().getDesugaredType(C).getTypePtr();

lebedev.ri wrote:
> aaron.ballman wrote:
> > Can use `const auto *` here.
> I do agree that as per [[ 
> https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
>  | coding style, that is preferred ]]  but the rest of the class would still 
> use the
> explicitly-spelled types, and IMO not deviating from local coding style in 
> small changes might be best?
> 
> (clang-tidy does not complain about this, but maybe that is because it is not 
> aware of this LLVM internal type casting functions)
> 
> But if you insist i will change it.
Consistency is best, so you might as well leave it. A follow-up NFC commit 
would be fine if you wanted to switch everything to `auto`.



Comment at: lib/Sema/SemaChecking.cpp:8593
+  Min, // e.g. -32768 for short
+  Both // e.g. in C++, A::a in enum A { a = 0 };
 };

lebedev.ri wrote:
> aaron.ballman wrote:
> > `Both` is not very descriptive -- I think `Zero` might be an improvement.
> But then it will be confusing what is the difference between `Zero` and `Min` 
> for `unsigned` types.
> I think i should just clarify the comment here.
Okay, that comment adds sufficient clarity for me.



Comment at: lib/Sema/SemaChecking.cpp:8593
+  Min = 1U << 1U,  // e.g. -32768 for short
+  Both = Max | Min // when the value is both the Min and the Max limit at the
+   // same time; e.g. in C++, A::a in enum A { a = 0 };

when -> When



Comment at: lib/Sema/SemaChecking.cpp:8619
+  if (OtherRange.Width == 0)
+return Value == 0 ? LimitType::Both : llvm::Optional();
+

Instead of default constructing the Optional, you should use `llvm::None` 
instead.


Repository:
  rL LLVM

https://reviews.llvm.org/D39122



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


[PATCH] D39160: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-21 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a subscriber: cfe-commits.
spatel added a comment.

Oops - I wrongly made llvm-commits a subscriber rather than cfe-commits. Let me 
know if I should reopen under a new thread to get the patch to hit the right 
mailing list.


https://reviews.llvm.org/D39160



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


Attribute spelling policy

2017-10-21 Thread Aaron Ballman via cfe-commits
Attributes come with multiple spelling flavors, but when it comes to
adding new attributes that are not present in other compiler tools
such as GCC or MSVC, we have done a poor job of being consistent with
which spelling flavors we adopt the attributes under. Some of our
attributes are specified with only an __attribute__ spelling (about
100), while others are specified with both __attribute__ and
[[clang::XXX]] (about 30), and still others are specified as only
[[clang::XXX]] attributes (only 1). This puts additional burden on
developers to remember which attributes are spelled with what syntax
and the various rules surrounding how to write attributes with that
spelling.

I am proposing that we take a more principled approach when adding new
attributes so that we provide a better user experience. Specifically,
when adding an attribute that other vendors do not support, the
attribute should be given an __attribute__ and [[clang::]] spelling
unless there's good reason not to. This is not a novel proposal -- GCC
supports all of their documented __attribute__ spellings under a
[[gnu::XXX]] spelling, and I am proposing we do the same with our
vendor namespace.

Assuming this approach is reasonable to the community, I will add a
CLANG spelling that behaves similar to the GCC spelling in that it
automatically provides both the GNU and CXX11 spellings as
appropriate. There are some attributes for which a [[clang::XXX]]
spelling is not appropriate:
  * attributes that appertain to function declarations but require
accessing the function parameters, such as disable_if or
requires_capability
  * attributes with GNU spellings whose use is discouraged or
deprecated, such as no_sanitize_memory
  * attributes that are part of other vendor specifications, like CUDA or OpenCL
These deviations are reasonable, but should be documented in Attr.td
near the Spelling definition for the attribute so that it's explicitly
understood why the spelling differs.

Additionally, I intend for the proposed CLANG spelling to be extended
in the future to more easily expose [[clang::XXX]] spellings for
attributes intended to be used in C (with
-fdouble-square-bracket-attributes) as well as C++.

As always, feedback is welcome!

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


[PATCH] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

2017-10-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 119758.
lebedev.ri added a comment.

Rebased.


Repository:
  rL LLVM

https://reviews.llvm.org/D36892

Files:
  test/clang-tidy/check_clang_tidy.py


Index: test/clang-tidy/check_clang_tidy.py
===
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -80,9 +80,11 @@
 
   has_check_fixes = input_text.find('CHECK-FIXES') >= 0
   has_check_messages = input_text.find('CHECK-MESSAGES') >= 0
+  has_check_notes = input_text.find('CHECK-NOTES') >= 0
 
-  if not has_check_fixes and not has_check_messages:
-sys.exit('Neither CHECK-FIXES nor CHECK-MESSAGES found in the input')
+  if not has_check_fixes and not has_check_messages and not has_check_notes:
+sys.exit('CHECK-FIXES, CHECK-MESSAGES, or CHECK-NOTES not found in the '
+ 'input')
 
   # Remove the contents of the CHECK lines to avoid CHECKs matching on
   # themselves.  We need to keep the comments to preserve line numbers while
@@ -143,5 +145,18 @@
   print('FileCheck failed:\n' + e.output.decode())
   raise
 
+  if has_check_notes:
+messages_file = temp_file_name + '.msg'
+write_file(messages_file, clang_tidy_output)
+try:
+  subprocess.check_output(
+  ['FileCheck', '-input-file=' + messages_file, input_file_name,
+   '-check-prefix=CHECK-NOTES',
+   '-implicit-check-not={{note|warning|error}}:'],
+  stderr=subprocess.STDOUT)
+except subprocess.CalledProcessError as e:
+  print('FileCheck failed:\n' + e.output.decode())
+  raise
+
 if __name__ == '__main__':
   main()


Index: test/clang-tidy/check_clang_tidy.py
===
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -80,9 +80,11 @@
 
   has_check_fixes = input_text.find('CHECK-FIXES') >= 0
   has_check_messages = input_text.find('CHECK-MESSAGES') >= 0
+  has_check_notes = input_text.find('CHECK-NOTES') >= 0
 
-  if not has_check_fixes and not has_check_messages:
-sys.exit('Neither CHECK-FIXES nor CHECK-MESSAGES found in the input')
+  if not has_check_fixes and not has_check_messages and not has_check_notes:
+sys.exit('CHECK-FIXES, CHECK-MESSAGES, or CHECK-NOTES not found in the '
+ 'input')
 
   # Remove the contents of the CHECK lines to avoid CHECKs matching on
   # themselves.  We need to keep the comments to preserve line numbers while
@@ -143,5 +145,18 @@
   print('FileCheck failed:\n' + e.output.decode())
   raise
 
+  if has_check_notes:
+messages_file = temp_file_name + '.msg'
+write_file(messages_file, clang_tidy_output)
+try:
+  subprocess.check_output(
+  ['FileCheck', '-input-file=' + messages_file, input_file_name,
+   '-check-prefix=CHECK-NOTES',
+   '-implicit-check-not={{note|warning|error}}:'],
+  stderr=subprocess.STDOUT)
+except subprocess.CalledProcessError as e:
+  print('FileCheck failed:\n' + e.output.decode())
+  raise
+
 if __name__ == '__main__':
   main()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:8186
+  // For enum types, for C code, use underlying data type.
+  if (const EnumType *ET = dyn_cast(T))
+T = ET->getDecl()->getIntegerType().getDesugaredType(C).getTypePtr();

aaron.ballman wrote:
> Can use `const auto *` here.
I do agree that as per [[ 
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
 | coding style, that is preferred ]]  but the rest of the class would still 
use the
explicitly-spelled types, and IMO not deviating from local coding style in 
small changes might be best?

(clang-tidy does not complain about this, but maybe that is because it is not 
aware of this LLVM internal type casting functions)

But if you insist i will change it.



Comment at: lib/Sema/SemaChecking.cpp:8593
+  Min, // e.g. -32768 for short
+  Both // e.g. in C++, A::a in enum A { a = 0 };
 };

aaron.ballman wrote:
> `Both` is not very descriptive -- I think `Zero` might be an improvement.
But then it will be confusing what is the difference between `Zero` and `Min` 
for `unsigned` types.
I think i should just clarify the comment here.



Comment at: lib/Sema/SemaChecking.cpp:8612
+OtherT = OtherT->getAs()->getDecl()->getIntegerType();
+  }
+

efriedma wrote:
> Why are you changing this here, as opposed to changing 
> IntRange::forValueOfCanonicalType?
No particular reason. Initially i had trouble with moving it into 
`IntRange::forValueOfCanonicalType()`,
so i left that in the state that you saw. Now done.



Comment at: test/Sema/outof-range-enum-constant-compare.c:1
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED 
-verify %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify %s

Rakete wrote:
> I don't know what the convention is, but I would prefer to use platform 
> independent tests wherever possible. I couldn't find a flag to change the 
> underlying type of an enum, so I'm not sure if my suggestion is even feasible.
https://reviews.llvm.org/D38101?id=118143#inline-338796
That is exactly the flag to change the underlying type of an enum.
I believe the `RUN` lines are correct here.



Repository:
  rL LLVM

https://reviews.llvm.org/D39122



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


[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 119757.
lebedev.ri marked 2 inline comments as done.
lebedev.ri added a comment.

Address review notes.


Repository:
  rL LLVM

https://reviews.llvm.org/D39122

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/outof-range-enum-constant-compare.c
  test/Sema/tautological-constant-enum-compare.c
  test/Sema/tautological-unsigned-enum-zero-compare.c
  test/Sema/tautological-unsigned-enum-zero-compare.cpp

Index: test/Sema/tautological-unsigned-enum-zero-compare.cpp
===
--- test/Sema/tautological-unsigned-enum-zero-compare.cpp
+++ test/Sema/tautological-unsigned-enum-zero-compare.cpp
@@ -1,176 +1,347 @@
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only -DALL_WARN -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSIGN_WARN -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -Wno-tautological-unsigned-enum-zero-compare -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSILENCE -Wno-tautological-unsigned-enum-zero-compare -verify %s
 
 // Okay, this is where it gets complicated.
 // Then default enum sigdness is target-specific.
 // On windows, it is signed by default. We do not want to warn in that case.
 
 int main() {
-  enum A { A_foo, A_bar };
+  enum A { A_foo = 0, A_bar, };
   enum A a;
 
-  enum B : unsigned { B_foo, B_bar };
+  enum B : unsigned { B_foo = 0, B_bar, };
   enum B b;
 
-  enum C : signed { c_foo, c_bar };
+  enum C : signed { C_foo = 0, C_bar, };
   enum C c;
 
-#ifdef ALL_WARN
+#ifdef UNSIGNED
   if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
 return 0;
-  if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  if (0 >= a)
+return 0;
+  if (a > 0)
 return 0;
   if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
 return 0;
+  if (a <= 0)
+return 0;
   if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
 return 0;
+  if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+return 0;
+  if (0 < a)
+return 0;
+
   if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
 return 0;
-  if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  if (0U >= a)
+return 0;
+  if (a > 0U)
 return 0;
   if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
 return 0;
+  if (a <= 0U)
+return 0;
   if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
 return 0;
+  if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+return 0;
+  if (0U < a)
+return 0;
 
   if (b < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
 return 0;
-  if (b >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  if (0 >= b)
+return 0;
+  if (b > 0)
 return 0;
   if (0 <= b) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
 return 0;
+  if (b <= 0)
+return 0;
   if (0 > b) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
 return 0;
-  if (b < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
-return 0;
-  if (b >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
-return 0;
-  if (0U <= b) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
-return 0;
-  if (0U > b) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
-return 0;
-
-  if (c < 0) // ok
-return 0;
-  if (c >= 0) // ok
-return 0;
-  if (0 <= c) // ok
-return 0;
-  if (0 > c) // ok
-return 0;
-  if (c < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
-return 0;
-  if (c >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
-return 0;
-  if (0U <= c) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
-return 0;
-  if (0U > c) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
-return 0;
-#elif defined(SIGN_WARN)
-  if (a < 0) // ok
-return 0;
-  if (a >= 0) // ok
-return 0;
-  if (0 <= a) // ok
-return 0;
-  if (0 > a) // ok
-return 0;
-  if (a < 0U) // expected-warning {{comparison of unsigned enum 

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-10-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:77
+  const std::array Msgs = {{
+  // FIXME: these messages somehow trigger an assertion:
+  // Fix conflicts with existing fix! The new replacement overlaps with an

lebedev.ri wrote:
> aaron.ballman wrote:
> > Are you intending to fix this -- that sounds rather serious?
> I do intend to fix it, as soon as i'm aware of how to fix this.
> Specifically, https://reviews.llvm.org/D36836#889350
> ```
> Question:
> Is it expected that clang-tidy somehow parses the DiagnosticIDs::Note's as 
> FixIt's, and thus fails with the following assert:
> ...
> ```
> I.e. what is the proper way to fix this, should i change the message, or 
> change the code not to parse the message as FixIt?
This is one for @alexfh to answer, I think. My gut says that the code should 
not be parsing notes as fixits (as a separate commit).



Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:99
+  /// details nessesary
+  struct Detail final {
+const SourceLocation Loc; /// What caused the increment?

lebedev.ri wrote:
> aaron.ballman wrote:
> > I don't think the `final` adds value here.
> I don't think it hurts, either?
> I *personally* prefer to specify it always, and remove if subclassing is 
> needed.
> But if you insist i can certainly drop it
That's not the typical coding style for the project, so I would prefer to leave 
it off. It's reasonable to use when there are vtables involved, however.



Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:102
+const unsigned short Nesting; /// How deeply nested is Loc located?
+const Criteria C : 3; /// The criteria of the increment
+

lebedev.ri wrote:
> aaron.ballman wrote:
> > Why is this turned into a bit-field? If that is important, it should be of 
> > type `unsigned` to prevent differences between compilers (MSVC treats 
> > bit-fields differently than GCC and Clang).
> The comment right after this member variable should have explained it:
> ```
> /// Criteria C is a bitfield. Even though Criteria is an unsigned char; and
> /// only using 3 bits will still result in padding, the fact that it is a
> /// bitfield is a reminder that it is important to min(sizeof(Detail))
> ```
> It is already `unsigned`:
> ```
> enum Criteria : unsigned char {
> ```
No, it's underlying type is `unsigned char`, but the type is `Criteria`. I 
meant the field itself needs to be `unsigned`. However, since that means you 
have to do more type casting, I think making the type `uint8_t` instead of a 
bit-field would be an improvement.



Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:118
+
+  unsigned MsgId;   /// The id of the message to output
+  unsigned short Increment; /// How much of an increment?

aaron.ballman wrote:
> We use `//` instead of `///` unless we're specifically documenting something 
> for doxygen.
This change is marked as done but wasn't applied consistently. For instance, 
see lines 130-131 where it switches between uses. I would use `//` consistently 
everywhere unless documenting a public interface in a header.



Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:111
+  };
+  static_assert(sizeof(Detail) <= 8, "it's best to keep the size minimal");
+

This `static_assert` should come with some further comments explaining that 
size being small is good, but not critical for correctness.



Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:212
+  bool TraverseStmtWithIncreasedNestingLevel(Stmt *Node) {
+bool ShouldContinue;
+++CurrentNestingLevel;

Declare with its initialization below.



Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:220
+  bool TraverseDeclWithIncreasedNestingLevel(Decl *Node) {
+bool ShouldContinue;
+++CurrentNestingLevel;

Declare with its initialization below.



Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:250
+
+/// If this IfStmt is *NOT* "else if", then only the body (i.e. "Then" and
+/// "Else" are traversed with increased Nesting level.

Missing a closing paren in the comment.



Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:275-276
+
+if (isa(Node->getElse()))
+  return TraverseIfStmt(dyn_cast(Node->getElse()), true);
+

Would be better to just get the AST node directly instead of `isa<>()` and 
`dyn_cast<>()`.

`if (const auto *E = dyn_cast(Node->getElse()))`



Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:317
+  

[PATCH] D39127: Fix template parameter default args missed if redecled

2017-10-21 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added inline comments.



Comment at: lib/Sema/SemaTemplate.cpp:4808
+  // Make sure we get the template parameter list from the most
+  // recentdeclaration, since that is the only one that has is guaranteed to
+  // have all the default template argument information.

Typo in "recentdeclaration"



Comment at: lib/Sema/SemaTemplate.cpp:4811
+  TemplateParameterList *Params =
+  cast(Template->getMostRecentDecl())
+  ->getTemplateParameters();

How does this work if there's another forward declaration missing the parameter 
later? Is the logic with "most recent" ok, or should it be "most 
qualified/complete" instead?


https://reviews.llvm.org/D39127



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


[PATCH] D39129: [OpenCL] Fix generation of constant address space sampler in function scope

2017-10-21 Thread Alexey Bader via Phabricator via cfe-commits
bader added a comment.

@Anastasia, during the discussion of similar fix 
(https://reviews.llvm.org/D34342).

I found another bug in the CodeGen library. Do you have time to fix it too?
Here is the reproducer from the old review request:

  int get_sampler_initializer(void);
  kernel void foo() {
const sampler_t const_smp_func_init = get_sampler_initializer();
  }


https://reviews.llvm.org/D39129



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


[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:8185
+if (!C.getLangOpts().CPlusPlus) {
+  // For enum types, for C code, use underlying data type.
+  if (const EnumType *ET = dyn_cast(T))

For enum types in C code, use the underlying datatype.



Comment at: lib/Sema/SemaChecking.cpp:8186
+  // For enum types, for C code, use underlying data type.
+  if (const EnumType *ET = dyn_cast(T))
+T = ET->getDecl()->getIntegerType().getDesugaredType(C).getTypePtr();

Can use `const auto *` here.



Comment at: lib/Sema/SemaChecking.cpp:8188
+T = ET->getDecl()->getIntegerType().getDesugaredType(C).getTypePtr();
+} else if (const EnumType *ET = dyn_cast(T)) {
+  // For enum types, in C++, use the known bit width of the enumerators.

Here as well.



Comment at: lib/Sema/SemaChecking.cpp:8189
+} else if (const EnumType *ET = dyn_cast(T)) {
+  // For enum types, in C++, use the known bit width of the enumerators.
   EnumDecl *Enum = ET->getDecl();

For enum types in C++,



Comment at: lib/Sema/SemaChecking.cpp:8593
+  Min, // e.g. -32768 for short
+  Both // e.g. in C++, A::a in enum A { a = 0 };
 };

`Both` is not very descriptive -- I think `Zero` might be an improvement.


Repository:
  rL LLVM

https://reviews.llvm.org/D39122



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


[clang-tools-extra] r316260 - clang-tidy: Fix deps.

2017-10-21 Thread NAKAMURA Takumi via cfe-commits
Author: chapuni
Date: Sat Oct 21 04:02:30 2017
New Revision: 316260

URL: http://llvm.org/viewvc/llvm-project?rev=316260=rev
Log:
clang-tidy: Fix deps.

Modified:
clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt

Modified: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/CMakeLists.txt?rev=316260=316259=316260=diff
==
--- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt Sat Oct 21 04:02:30 2017
@@ -1,7 +1,4 @@
 set(LLVM_LINK_COMPONENTS
-  AllTargetsAsmParsers
-  AllTargetsDescs
-  AllTargetsInfos
   Support
   )
 

Modified: clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt?rev=316260=316259=316260=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt Sat Oct 21 04:02:30 
2017
@@ -1,4 +1,7 @@
 set(LLVM_LINK_COMPONENTS
+  AllTargetsAsmParsers
+  AllTargetsDescs
+  AllTargetsInfos
   support
   )
 


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


[PATCH] D39156: [libunwind] Make HIDDEN_DIRECTIVE a function-like macro. NFCI.

2017-10-21 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo created this revision.

This avoids a hack for making it a no-op for windows.

Also explicitly check for `_WIN32` instead of asduming it.


https://reviews.llvm.org/D39156

Files:
  src/assembly.h


Index: src/assembly.h
===
--- src/assembly.h
+++ src/assembly.h
@@ -24,27 +24,14 @@
 #define SEPARATOR ;
 #endif
 
-#if defined(__APPLE__)
-#define HIDDEN_DIRECTIVE .private_extern
-#elif defined(_WIN32)
-// In the COFF object file format, there's no attributes for a global,
-// non-static symbol to make it somehow hidden. So on windows, we don't
-// want to set this at all. To avoid conditionals in
-// DEFINE_LIBUNWIND_PRIVATE_FUNCTION below, make it .globl (which it already
-// is, defined in the same DEFINE_LIBUNWIND_PRIVATE_FUNCTION macro; the
-// duplicate .globl directives are harmless).
-#define HIDDEN_DIRECTIVE .globl
-#else
-#define HIDDEN_DIRECTIVE .hidden
-#endif
-
 #define GLUE2(a, b) a ## b
 #define GLUE(a, b) GLUE2(a, b)
 #define SYMBOL_NAME(name) GLUE(__USER_LABEL_PREFIX__, name)
 
 #if defined(__APPLE__)
 
 #define SYMBOL_IS_FUNC(name)
+#define HIDDEN_SYMBOL(name) .private_extern name
 #define NO_EXEC_STACK_DIRECTIVE
 
 #elif defined(__ELF__)
@@ -54,24 +41,30 @@
 #else
 #define SYMBOL_IS_FUNC(name) .type name,@function
 #endif
+#define HIDDEN_SYMBOL(name) .hidden name
 
 #if defined(__GNU__) || defined(__FreeBSD__) || defined(__Fuchsia__) || \
 defined(__linux__)
 #define NO_EXEC_STACK_DIRECTIVE .section .note.GNU-stack,"",%progbits
 #else
 #define NO_EXEC_STACK_DIRECTIVE
 #endif
 
-#else
+#elif defined(_WIN32)
 
 #define SYMBOL_IS_FUNC(name)   
\
   .def name SEPARATOR  
\
 .scl 2 SEPARATOR   
\
 .type 32 SEPARATOR 
\
   .endef
+#define HIDDEN_SYMBOL(name)
 
 #define NO_EXEC_STACK_DIRECTIVE
 
+#else
+
+#error Unsupported target
+
 #endif
 
 #define DEFINE_LIBUNWIND_FUNCTION(name)   \
@@ -81,7 +74,7 @@
 
 #define DEFINE_LIBUNWIND_PRIVATE_FUNCTION(name)   \
   .globl SYMBOL_NAME(name) SEPARATOR  \
-  HIDDEN_DIRECTIVE SYMBOL_NAME(name) SEPARATOR\
+  HIDDEN_SYMBOL(SYMBOL_NAME(name)) SEPARATOR  \
   SYMBOL_IS_FUNC(SYMBOL_NAME(name)) SEPARATOR \
   SYMBOL_NAME(name):
 


Index: src/assembly.h
===
--- src/assembly.h
+++ src/assembly.h
@@ -24,27 +24,14 @@
 #define SEPARATOR ;
 #endif
 
-#if defined(__APPLE__)
-#define HIDDEN_DIRECTIVE .private_extern
-#elif defined(_WIN32)
-// In the COFF object file format, there's no attributes for a global,
-// non-static symbol to make it somehow hidden. So on windows, we don't
-// want to set this at all. To avoid conditionals in
-// DEFINE_LIBUNWIND_PRIVATE_FUNCTION below, make it .globl (which it already
-// is, defined in the same DEFINE_LIBUNWIND_PRIVATE_FUNCTION macro; the
-// duplicate .globl directives are harmless).
-#define HIDDEN_DIRECTIVE .globl
-#else
-#define HIDDEN_DIRECTIVE .hidden
-#endif
-
 #define GLUE2(a, b) a ## b
 #define GLUE(a, b) GLUE2(a, b)
 #define SYMBOL_NAME(name) GLUE(__USER_LABEL_PREFIX__, name)
 
 #if defined(__APPLE__)
 
 #define SYMBOL_IS_FUNC(name)
+#define HIDDEN_SYMBOL(name) .private_extern name
 #define NO_EXEC_STACK_DIRECTIVE
 
 #elif defined(__ELF__)
@@ -54,24 +41,30 @@
 #else
 #define SYMBOL_IS_FUNC(name) .type name,@function
 #endif
+#define HIDDEN_SYMBOL(name) .hidden name
 
 #if defined(__GNU__) || defined(__FreeBSD__) || defined(__Fuchsia__) || \
 defined(__linux__)
 #define NO_EXEC_STACK_DIRECTIVE .section .note.GNU-stack,"",%progbits
 #else
 #define NO_EXEC_STACK_DIRECTIVE
 #endif
 
-#else
+#elif defined(_WIN32)
 
 #define SYMBOL_IS_FUNC(name)   \
   .def name SEPARATOR  \
 .scl 2 SEPARATOR   \
 .type 32 SEPARATOR \
   .endef
+#define HIDDEN_SYMBOL(name)
 
 #define NO_EXEC_STACK_DIRECTIVE
 
+#else
+
+#error Unsupported target
+
 #endif
 
 #define DEFINE_LIBUNWIND_FUNCTION(name)   \
@@ -81,7 +74,7 @@
 
 #define DEFINE_LIBUNWIND_PRIVATE_FUNCTION(name)   \
   .globl SYMBOL_NAME(name) SEPARATOR  \
-  HIDDEN_DIRECTIVE SYMBOL_NAME(name) SEPARATOR\
+  HIDDEN_SYMBOL(SYMBOL_NAME(name)) SEPARATOR  \
   SYMBOL_IS_FUNC(SYMBOL_NAME(name)) SEPARATOR \
   SYMBOL_NAME(name):
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits