[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:26
   }
   throw non_derived_exception(); // Bad
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 
'non_derived_exception' is not derived from 'std::exception'

lebedev.ri wrote:
> Could you please update the comments?
> I find them misleading. What does this `bad` mean?
> That this line is not handled correctly? Then please use `FIXME: wrong 
> handling`
> That this line is invalid? The `// CHECK-MESSAGES:` already states that...
yes, they are left over when i started writing the test and did not know the 
diagnostics. These comments help, when something is reported from clang-tidy, 
to instantly see if this is correctly found, since the source line is shown. I 
can remove them.



Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:54-62
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: throwing an exception whose type 
'bad_generic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-3]]:31: warning: throwing an exception whose type 
'bad_generic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-5]]:31: warning: throwing an exception whose type 
'exotic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 74:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-7]]:31: warning: throwing an exception whose type 
'int' is not derived from 'std::exception'

lebedev.ri wrote:
> JonasToth wrote:
> > these diagnostic come from the many instantiations of this function. 
> > do you think, they should exist or rather not?
> They definitively should exist.
> But they also should ideally point to the origin of the `T`.
they kinda do. when templates are used, the template class is pointed to, but 
not the instantiation. At least they shouldn't point to the `` 
anymore.


https://reviews.llvm.org/D37060



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

[PATCH] D37287: [X86] Implement broadcastf32x2 and broadcasti32x2 intrinsics using __builtin_shufflevector instead builtins

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.

This patch implements the broadcastf32x2/broadcasti32x2 intrinsics using 
__builtin_shufflevector.


https://reviews.llvm.org/D37287

Files:
  include/clang/Basic/BuiltinsX86.def
  lib/Headers/avx512dqintrin.h
  lib/Headers/avx512vldqintrin.h
  test/CodeGen/avx512dq-builtins.c
  test/CodeGen/avx512vldq-builtins.c

Index: test/CodeGen/avx512vldq-builtins.c
===
--- test/CodeGen/avx512vldq-builtins.c
+++ test/CodeGen/avx512vldq-builtins.c
@@ -909,19 +909,21 @@
 
 __m256 test_mm256_broadcast_f32x2(__m128 __A) {
   // CHECK-LABEL: @test_mm256_broadcast_f32x2
-  // CHECK: @llvm.x86.avx512.mask.broadcastf32x2
+  // CHECK: shufflevector <4 x float> %{{.*}}, <4 x float> zeroinitializer, <8 x i32> 
   return _mm256_broadcast_f32x2(__A); 
 }
 
 __m256 test_mm256_mask_broadcast_f32x2(__m256 __O, __mmask8 __M, __m128 __A) {
   // CHECK-LABEL: @test_mm256_mask_broadcast_f32x2
-  // CHECK: @llvm.x86.avx512.mask.broadcastf32x2
+  // CHECK: shufflevector <4 x float> %{{.*}}, <4 x float> zeroinitializer, <8 x i32> 
+  // CHECK: select <8 x i1> %{{.*}}, <8 x float> %{{.*}}, <8 x float> %{{.*}}
   return _mm256_mask_broadcast_f32x2(__O, __M, __A); 
 }
 
 __m256 test_mm256_maskz_broadcast_f32x2(__mmask8 __M, __m128 __A) {
   // CHECK-LABEL: @test_mm256_maskz_broadcast_f32x2
-  // CHECK: @llvm.x86.avx512.mask.broadcastf32x2
+  // CHECK: shufflevector <4 x float> %{{.*}}, <4 x float> zeroinitializer, <8 x i32> 
+  // CHECK: select <8 x i1> %{{.*}}, <8 x float> %{{.*}}, <8 x float> %{{.*}}
   return _mm256_maskz_broadcast_f32x2(__M, __A); 
 }
 
@@ -947,37 +949,41 @@
 
 __m128i test_mm_broadcast_i32x2(__m128i __A) {
   // CHECK-LABEL: @test_mm_broadcast_i32x2
-  // CHECK: @llvm.x86.avx512.mask.broadcasti32x2
+  // CHECK: shufflevector <4 x i32> %{{.*}}, <4 x i32> zeroinitializer, <4 x i32> 
   return _mm_broadcast_i32x2(__A); 
 }
 
 __m128i test_mm_mask_broadcast_i32x2(__m128i __O, __mmask8 __M, __m128i __A) {
   // CHECK-LABEL: @test_mm_mask_broadcast_i32x2
-  // CHECK: @llvm.x86.avx512.mask.broadcasti32x2
+  // CHECK: shufflevector <4 x i32> %{{.*}}, <4 x i32> zeroinitializer, <4 x i32> 
+  // CHECK: select <4 x i1> %{{.*}}, <4 x i32> %{{.*}}, <4 x i32> %{{.*}}
   return _mm_mask_broadcast_i32x2(__O, __M, __A); 
 }
 
 __m128i test_mm_maskz_broadcast_i32x2(__mmask8 __M, __m128i __A) {
   // CHECK-LABEL: @test_mm_maskz_broadcast_i32x2
-  // CHECK: @llvm.x86.avx512.mask.broadcasti32x2
+  // CHECK: shufflevector <4 x i32> %{{.*}}, <4 x i32> zeroinitializer, <4 x i32> 
+  // CHECK: select <4 x i1> %{{.*}}, <4 x i32> %{{.*}}, <4 x i32> %{{.*}}
   return _mm_maskz_broadcast_i32x2(__M, __A); 
 }
 
 __m256i test_mm256_broadcast_i32x2(__m128i __A) {
   // CHECK-LABEL: @test_mm256_broadcast_i32x2
-  // CHECK: @llvm.x86.avx512.mask.broadcasti32x2
+  // CHECK: shufflevector <4 x i32> %{{.*}}, <4 x i32> zeroinitializer, <8 x i32> 
   return _mm256_broadcast_i32x2(__A); 
 }
 
 __m256i test_mm256_mask_broadcast_i32x2(__m256i __O, __mmask8 __M, __m128i __A) {
   // CHECK-LABEL: @test_mm256_mask_broadcast_i32x2
-  // CHECK: @llvm.x86.avx512.mask.broadcasti32x2
+  // CHECK: shufflevector <4 x i32> %{{.*}}, <4 x i32> zeroinitializer, <8 x i32> 
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> %{{.*}}
   return _mm256_mask_broadcast_i32x2(__O, __M, __A); 
 }
 
 __m256i test_mm256_maskz_broadcast_i32x2(__mmask8 __M, __m128i __A) {
   // CHECK-LABEL: @test_mm256_maskz_broadcast_i32x2
-  // CHECK: @llvm.x86.avx512.mask.broadcasti32x2
+  // CHECK: shufflevector <4 x i32> %{{.*}}, <4 x i32> zeroinitializer, <8 x i32> 
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> %{{.*}}
   return _mm256_maskz_broadcast_i32x2(__M, __A); 
 }
 
Index: test/CodeGen/avx512dq-builtins.c
===
--- test/CodeGen/avx512dq-builtins.c
+++ test/CodeGen/avx512dq-builtins.c
@@ -949,19 +949,21 @@
 
 __m512 test_mm512_broadcast_f32x2(__m128 __A) {
   // CHECK-LABEL: @test_mm512_broadcast_f32x2
-  // CHECK: @llvm.x86.avx512.mask.broadcastf32x2
+  // CHECK: shufflevector <4 x float> %{{.*}}, <4 x float> zeroinitializer, <16 x i32> 
   return _mm512_broadcast_f32x2(__A); 
 }
 
 __m512 test_mm512_mask_broadcast_f32x2(__m512 __O, __mmask16 __M, __m128 __A) {
   // CHECK-LABEL: @test_mm512_mask_broadcast_f32x2
-  // CHECK: @llvm.x86.avx512.mask.broadcastf32x2
+  // CHECK: shufflevector <4 x float> %{{.*}}, <4 x float> zeroinitializer, <16 x i32> 
+  // CHECK: select <16 x i1> %{{.*}}, <16 x float> %{{.*}}, <16 x float> %{{.*}}
   return _mm512_mask_broadcast_f32x2(__O, __M, __A); 
 }
 
 __m512 test_mm512_maskz_broadcast_f32x2(__mmask16 __M, __m128 __A) {
   // CHECK-LABEL: @test_mm512_maskz_broadcast_f32x2
-  // CHECK: @llvm.x86.avx512.mask.broadcastf32x2
+  // CHECK: shufflevector <4 x float> %{{.*}}, <4 x float> zeroinitializer, <16 x i32> 
+  // CHECK: select <16 x i1> %{{.*}}, <16 x float> 

[PATCH] D37287: [X86] Implement broadcastf32x2 and broadcasti32x2 intrinsics using __builtin_shufflevector instead builtins

[Ayman Musa via Phabricator via cfe-commits]

aymanmus added a comment.

Did you make sure the resulted IR was lowered to the expected X86 instructions?


https://reviews.llvm.org/D37287



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

[PATCH] D37287: [X86] Implement broadcastf32x2 and broadcasti32x2 intrinsics using __builtin_shufflevector instead builtins

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Yes I did. With the other bug fixed they all produced the correct instruction.


https://reviews.llvm.org/D37287



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

[clang-tools-extra] r312102 - [clang-tidy] test commit for granted access

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Wed Aug 30 00:50:28 2017
New Revision: 312102

URL: http://llvm.org/viewvc/llvm-project?rev=312102&view=rev
Log:
[clang-tidy] test commit for granted access

Modified:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp?rev=312102&r1=312101&r2=312102&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp Wed 
Aug 30 00:50:28 2017
@@ -29,7 +29,7 @@ Matcher hasAnyListedName(c
   utils::options::parseStringList(FunctionNames);
   return hasAnyName(std::vector(NameList.begin(), NameList.end()));
 }
-}
+} // namespace
 
 void NoMallocCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "Allocations", AllocList);


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

[PATCH] D35020: [Modules] Add ability to specify module name to module file mapping

[Boris Kolpackov via Phabricator via cfe-commits]

boris marked 2 inline comments as done.
boris added inline comments.



Comment at: lib/Frontend/CompilerInvocation.cpp:986
+if (Val.find('=') == StringRef::npos)
+  Opts.ExtraDeps.push_back(Val);
+  }

rsmith wrote:
> Does a module file specified via `-fmodule-file=foo=foo.pcm` get included in 
> the dep file if it is used? (If not, I'm happy for that to be fixed in a 
> separate change, but it does need to be fixed for depfile-based build 
> systems.)
No, currently it does not and neither a module file that is discovered via 
-fprebuilt-module-path. I think if we are doing this, then we should do for 
both cases. What do you think?


https://reviews.llvm.org/D35020



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

[PATCH] D37287: [X86] Implement broadcastf32x2 and broadcasti32x2 intrinsics using __builtin_shufflevector instead builtins

[Ayman Musa via Phabricator via cfe-commits]

aymanmus accepted this revision.
aymanmus added a comment.
This revision is now accepted and ready to land.

LGTM
Thanks for the 2 patches.


https://reviews.llvm.org/D37287



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

[PATCH] D34512: Add preliminary Cross Translation Unit support library

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In https://reviews.llvm.org/D34512#856184, @dcoughlin wrote:

> In either case, the important scenario I think we should support is choosing 
> at a call site to a C function the most likely definition of the called 
> function, based on number and type of parameters, from multiple possible 
> definitions in other translation units. If the API is rich enough to support 
> this then I think that is a good indication it will be useful for other 
> scenarios as well.


Note that the lookup is already based on USR which is similar to mangled names 
in a sense that it contains information about the function parameters. So the 
only way to get multiple candidates from the lookup is having multiple function 
definitions with the same signature. It is only possible to disambiguate with 
knowledge about the linking graph. I think that information is better to be 
stored in the index in the future and do the disambiguation within the library 
instead of making the client do that.

> 
> 
>> The reason we introduced the diagnostic is that we assumed that the client 
>> can not recover from a configuration error and will end up reporting the 
>> problem to the user.
> 
> For the static analyzer client, at least, one possible recovery is performing 
> the existing invalidation that we do when calling a function defined in 
> another translation unit. (I'm not really sure this a good idea; I just 
> wanted to point out that the decision probably belongs with the client). I 
> think it is reasonable to report an error as a diagnostic -- but I think this 
> should be up to the client and I don't think it should show up in the index 
> file itself. In an ideal world the user wouldn't even know that file existed. 
> Further, for large projects/incremental rebuilds a text file is unlikely to 
> be a suitable representation for the index. In the long term I doubt the file 
> will be end-user editable/readable.

I agree that we do not consider this format to be final for the index. In the 
future, it would be great to include linking information as well and also have 
utilities to merge partial indexes. This is a first version to make the 
machinery work for the most common cases which are already pretty usable for 
the open source projects we tested.


https://reviews.llvm.org/D34512



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

[PATCH] D15465: [git-clang-format]: New option to perform formatting against staged changes only

[Alexander Shukaev via Phabricator via cfe-commits]

Alexander-Shukaev added a comment.

Did anybody have a chance to review it and/or try it out?


Repository:
  rL LLVM

https://reviews.llvm.org/D15465



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

r312105 - [modules] Add ability to specify module name to module file mapping

[Boris Kolpackov via cfe-commits]

Author: borisk
Date: Wed Aug 30 01:45:59 2017
New Revision: 312105

URL: http://llvm.org/viewvc/llvm-project?rev=312105&view=rev
Log:
[modules] Add ability to specify module name to module file mapping

Extend the -fmodule-file option to support the [=] value format.
If the name is omitted, then the old semantics is preserved (the module file
is loaded whether needed or not). If the name is specified, then the mapping
is treated as just another prebuilt module search mechanism, similar to
-fprebuilt-module-path, and the module file is only loaded if actually used
(e.g., via import). With one exception: this mapping also overrides module
file references embedded in other modules (which can be useful if module files
are moved/renamed as often happens during remote compilation).

This override semantics requires some extra work: we now store the module name
in addition to the file name in the serialized AST representation.

Reviewed By: rsmith

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


Added:
cfe/trunk/test/CXX/modules-ts/basic/basic.search/
cfe/trunk/test/CXX/modules-ts/basic/basic.search/module-import.cpp   (with 
props)
Modified:
cfe/trunk/docs/Modules.rst
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Lex/HeaderSearch.h
cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/include/clang/Serialization/ModuleManager.h
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/FrontendActions.cpp
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
cfe/trunk/lib/Serialization/ModuleManager.cpp

Modified: cfe/trunk/docs/Modules.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/Modules.rst?rev=312105&r1=312104&r2=312105&view=diff
==
--- cfe/trunk/docs/Modules.rst (original)
+++ cfe/trunk/docs/Modules.rst Wed Aug 30 01:45:59 2017
@@ -213,8 +213,14 @@ Command-line parameters
 ``-fno-implicit-modules``
   All modules used by the build must be specified with ``-fmodule-file``.
 
-``-fmodule-file=``
-  Load the given precompiled module file.
+``-fmodule-file=[=]``
+  Specify the mapping of module names to precompiled module files. If the
+  name is omitted, then the module file is loaded whether actually required
+  or not. If the name is specified, then the mapping is treated as another
+  prebuilt module search mechanism (in addition to ``-fprebuilt-module-path``)
+  and the module is only loaded if required. Note that in this case the
+  specified file also overrides this module's paths that might be embedded
+  in other precompiled module files.
 
 ``-fprebuilt-module-path=``
   Specify the path to the prebuilt modules. If specified, we will look for 
modules in this directory for a given top-level module name. We don't need a 
module map for loading prebuilt modules in this directory and the compiler will 
not try to rebuild these modules. This can be specified multiple times.
@@ -945,4 +951,3 @@ PCHInternals_
 .. [#] The preprocessing context in which the modules are parsed is actually 
dependent on the command-line options provided to the compiler, including the 
language dialect and any ``-D`` options. However, the compiled modules for 
different command-line options are kept distinct, and any preprocessor 
directives that occur within the translation unit are ignored. See the section 
on the `Configuration macros declaration`_ for more information.
 
 .. _PCHInternals: PCHInternals.html
- 

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=312105&r1=312104&r2=312105&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Wed Aug 30 01:45:59 2017
@@ -1125,8 +1125,8 @@ def fmodule_map_file : Joined<["-"], "fm
   Group, Flags<[DriverOption,CC1Option]>, MetaVarName<"">,
   HelpText<"Load this module map file">;
 def fmodule_file : Joined<["-"], "fmodule-file=">,
-  Group, Flags<[DriverOption,CC1Option]>,
-  HelpText<"Load this precompiled module file">, MetaVarName<"">;
+  Group, Flags<[DriverOption,CC1Option]>, 
MetaVarName<"[=]">,
+  HelpText<"Specify the mapping of module name to precompiled module file, or 
load a module file if name is omitted.">;
 def fmodules_ignore_macro : Joined<["-"], "fmodules-ignore-macro=">, 
Group, Flags<[CC1Option]>,
   HelpText<"Ignore the definition of the given macro when building and loading 
modules">;
 def fmodules_decluse : Flag <["-"], "fmodules-decluse">, Group,

Modified: cfe/trunk/include/clang/Lex

[PATCH] D35020: [Modules] Add ability to specify module name to module file mapping

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL312105: [modules] Add ability to specify module name to 
module file mapping (authored by borisk).

Changed prior to commit:
  https://reviews.llvm.org/D35020?vs=111826&id=113207#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35020

Files:
  cfe/trunk/docs/Modules.rst
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Lex/HeaderSearch.h
  cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
  cfe/trunk/include/clang/Serialization/ASTReader.h
  cfe/trunk/include/clang/Serialization/ModuleManager.h
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInstance.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/Frontend/FrontendActions.cpp
  cfe/trunk/lib/Lex/HeaderSearch.cpp
  cfe/trunk/lib/Serialization/ASTReader.cpp
  cfe/trunk/lib/Serialization/ASTWriter.cpp
  cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
  cfe/trunk/lib/Serialization/ModuleManager.cpp
  cfe/trunk/test/CXX/modules-ts/basic/basic.search/module-import.cpp

Index: cfe/trunk/lib/Serialization/ASTWriter.cpp
===
--- cfe/trunk/lib/Serialization/ASTWriter.cpp
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp
@@ -1505,6 +1505,7 @@
   for (auto I : M.Signature)
 Record.push_back(I);
 
+  AddString(M.ModuleName, Record);
   AddPath(M.FileName, Record);
 }
 Stream.EmitRecord(IMPORTS, Record);
@@ -4779,7 +4780,8 @@
 // each of those modules were mapped into our own offset/ID space, so that
 // the reader can build the appropriate mapping to its own offset/ID space.
 // The map consists solely of a blob with the following format:
-// *(module-name-len:i16 module-name:len*i8
+// *(module-kind:i8
+//   module-name-len:i16 module-name:len*i8
 //   source-location-offset:i32
 //   identifier-id:i32
 //   preprocessed-entity-id:i32
@@ -4790,6 +4792,10 @@
 //   c++-base-specifiers-id:i32
 //   type-id:i32)
 // 
+// module-kind is the ModuleKind enum value. If it is MK_PrebuiltModule or
+// MK_ExplicitModule, then the module-name is the module name. Otherwise,
+// it is the module file name.
+//
 auto Abbrev = std::make_shared();
 Abbrev->Add(BitCodeAbbrevOp(MODULE_OFFSET_MAP));
 Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
@@ -4800,9 +4806,13 @@
   for (ModuleFile &M : Chain->ModuleMgr) {
 using namespace llvm::support;
 endian::Writer LE(Out);
-StringRef FileName = M.FileName;
-LE.write(FileName.size());
-Out.write(FileName.data(), FileName.size());
+LE.write(static_cast(M.Kind));
+StringRef Name =
+  M.Kind == MK_PrebuiltModule || M.Kind == MK_ExplicitModule
+  ? M.ModuleName
+  : M.FileName;
+LE.write(Name.size());
+Out.write(Name.data(), Name.size());
 
 // Note: if a base ID was uint max, it would not be possible to load
 // another module after it or have more than one entity inside it.
Index: cfe/trunk/lib/Serialization/ModuleManager.cpp
===
--- cfe/trunk/lib/Serialization/ModuleManager.cpp
+++ cfe/trunk/lib/Serialization/ModuleManager.cpp
@@ -28,15 +28,23 @@
 using namespace clang;
 using namespace serialization;
 
-ModuleFile *ModuleManager::lookup(StringRef Name) const {
+ModuleFile *ModuleManager::lookupByFileName(StringRef Name) const {
   const FileEntry *Entry = FileMgr.getFile(Name, /*openFile=*/false,
/*cacheFailure=*/false);
   if (Entry)
 return lookup(Entry);
 
   return nullptr;
 }
 
+ModuleFile *ModuleManager::lookupByModuleName(StringRef Name) const {
+  if (const Module *Mod = HeaderSearchInfo.getModuleMap().findModule(Name))
+if (const FileEntry *File = Mod->getASTFile())
+  return lookup(File);
+
+  return nullptr;
+}
+
 ModuleFile *ModuleManager::lookup(const FileEntry *File) const {
   auto Known = Modules.find(File);
   if (Known == Modules.end())
@@ -306,9 +314,11 @@
 }
 
 ModuleManager::ModuleManager(FileManager &FileMgr, MemoryBufferCache &PCMCache,
- const PCHContainerReader &PCHContainerRdr)
+ const PCHContainerReader &PCHContainerRdr,
+ const HeaderSearch& HeaderSearchInfo)
 : FileMgr(FileMgr), PCMCache(&PCMCache), PCHContainerRdr(PCHContainerRdr),
-  GlobalIndex(), FirstVisitState(nullptr) {}
+  HeaderSearchInfo (HeaderSearchInfo), GlobalIndex(),
+  FirstVisitState(nullptr) {}
 
 ModuleManager::~ModuleManager() { delete FirstVisitState; }
 
Index: cfe/trunk/lib/Serialization/ASTReader.cpp
===
--- cfe/trunk/lib/Serialization/ASTReader.cpp
+++ cfe/trunk/lib/Serialization/ASTReader.cpp
@@ -2485,7 +2485,23 @@

[PATCH] D34512: Add preliminary Cross Translation Unit support library

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 113206.
xazax.hun added a comment.

- Added unit test to ensure the produced index format can be parsed.
- Added further diagnostics.


https://reviews.llvm.org/D34512

Files:
  include/clang/Basic/AllDiagnostics.h
  include/clang/Basic/CMakeLists.txt
  include/clang/Basic/Diagnostic.td
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/Basic/DiagnosticIDs.h
  include/clang/CrossTU/CrossTUDiagnostic.h
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/AST/ASTImporter.cpp
  lib/Basic/DiagnosticIDs.cpp
  lib/CMakeLists.txt
  lib/CrossTU/CMakeLists.txt
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/func-mapping-test.cpp
  test/CMakeLists.txt
  test/lit.cfg
  tools/CMakeLists.txt
  tools/clang-func-mapping/CMakeLists.txt
  tools/clang-func-mapping/ClangFnMapGen.cpp
  tools/diagtool/DiagnosticNames.cpp
  unittests/CMakeLists.txt
  unittests/CrossTU/CMakeLists.txt
  unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- /dev/null
+++ unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -0,0 +1,122 @@
+//===- unittest/Tooling/CrossTranslationUnitTest.cpp - Tooling unit tests -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Config/llvm-config.h"
+#include "llvm/Support/Path.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace clang {
+namespace cross_tu {
+
+namespace {
+StringRef IndexFileName = "index.txt";
+StringRef ASTFileName = "f.ast";
+StringRef DefinitionFileName = "input.cc";
+
+class CTUASTConsumer : public clang::ASTConsumer {
+public:
+  explicit CTUASTConsumer(clang::CompilerInstance &CI, bool *Success)
+  : CTU(CI), Success(Success) {}
+
+  void HandleTranslationUnit(ASTContext &Ctx) {
+const TranslationUnitDecl *TU = Ctx.getTranslationUnitDecl();
+const FunctionDecl *FD = nullptr;
+for (const Decl *D : TU->decls()) {
+  FD = dyn_cast(D);
+  if (FD && FD->getName() == "f")
+break;
+}
+assert(FD);
+bool OrigFDHasBody = FD->hasBody();
+
+// Prepare the index file and the AST file.
+std::error_code EC;
+llvm::raw_fd_ostream OS(IndexFileName, EC, llvm::sys::fs::F_Text);
+OS << "c:@F@f#I# " << ASTFileName << "\n";
+OS.flush();
+StringRef SourceText = "int f(int) { return 0; }\n";
+// This file must exist since the saved ASTFile will reference it.
+llvm::raw_fd_ostream OS2(DefinitionFileName, EC, llvm::sys::fs::F_Text);
+OS2 << SourceText;
+OS2.flush();
+std::unique_ptr ASTWithDefinition =
+tooling::buildASTFromCode(SourceText);
+ASTWithDefinition->Save(ASTFileName);
+
+// Load the definition from the AST file.
+const FunctionDecl *NewFD =
+CTU.getCrossTUDefinition(FD, ".", IndexFileName);
+
+*Success = NewFD && NewFD->hasBody() && !OrigFDHasBody;
+  }
+
+private:
+  CrossTranslationUnitContext CTU;
+  bool *Success;
+};
+
+class CTUAction : public clang::ASTFrontendAction {
+public:
+  CTUAction(bool *Success) : Success(Success) {}
+
+protected:
+  std::unique_ptr
+  CreateASTConsumer(clang::CompilerInstance &CI, StringRef) override {
+return llvm::make_unique(CI, Success);
+  }
+
+private:
+  bool *Success;
+};
+
+} // end namespace
+
+TEST(CrossTranslationUnit, CanLoadFunctionDefinition) {
+  bool Success = false;
+  EXPECT_TRUE(tooling::runToolOnCode(new CTUAction(&Success), "int f(int);"));
+  EXPECT_TRUE(Success);
+  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
+  EXPECT_FALSE((bool)llvm::sys::fs::remove(IndexFileName));
+  EXPECT_TRUE(llvm::sys::fs::exists(ASTFileName));
+  EXPECT_FALSE((bool)llvm::sys::fs::remove(ASTFileName));
+  EXPECT_TRUE(llvm::sys::fs::exists(DefinitionFileName));
+  EXPECT_FALSE((bool)llvm::sys::fs::remove(DefinitionFileName));
+}
+
+TEST(CrossTranslationUnit, IndexFormatCanBeParsed) {
+  llvm::StringMap Index;
+  Index["a"] = "b";
+  Index["c"] = "d";
+  Index["e"] = "f";
+  std::string IndexText = createCrossTUIndexString(Index);
+  std::error_code EC;
+  llvm::raw_fd_ostream OS(IndexFileName, EC, llvm::sys::fs::F_Text);
+  OS << IndexText;
+  OS.flush();
+  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
+  llvm::Expected> IndexOrErr =
+  parseCrossTUIndex(IndexFileName, "");
+  EXPECT_TRUE((bool)IndexOrErr);
+  llvm::StringMap ParsedIndex = IndexOrErr.get();
+  for (const auto &E : Index) {
+EXPECT_TRUE(ParsedIndex.count(E.getKey()));
+EXPECT_EQ(ParsedIndex[E.getKey()], E.getValue());
+  }
+  for (const auto &E : ParsedIndex)
+EXPECT_TRUE(Index.coun

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 113212.
JonasToth marked 2 inline comments as done.
JonasToth added a comment.

- removing trailing comments


https://reviews.llvm.org/D37060

Files:
  test/clang-tidy/hicpp-exception-baseclass.cpp

Index: test/clang-tidy/hicpp-exception-baseclass.cpp
===
--- test/clang-tidy/hicpp-exception-baseclass.cpp
+++ test/clang-tidy/hicpp-exception-baseclass.cpp
@@ -10,39 +10,39 @@
 
 void problematic() {
   try {
-throw int(42); // Built in is not allowed
+throw int(42);
 // CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
   } catch (int e) {
   }
-  throw int(42); // Bad
+  throw int(42);
   // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
 
   try {
-throw non_derived_exception(); // Some class is not allowed
+throw non_derived_exception();
 // CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
 // CHECK-MESSAGES: 9:1: note: type defined here
   } catch (non_derived_exception &e) {
   }
-  throw non_derived_exception(); // Bad
+  throw non_derived_exception();
   // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
   // CHECK-MESSAGES: 9:1: note: type defined here
 }
 
 void allowed_throws() {
   try {
-throw std::exception(); // Ok
+throw std::exception(); // Ok
   } catch (std::exception &e) { // Ok
   }
   throw std::exception();
 
   try {
-throw derived_exception(); // Ok
+throw derived_exception(); // Ok
   } catch (derived_exception &e) { // Ok
   }
   throw derived_exception(); // Ok
 
   try {
-throw deep_hierarchy(); // Ok, multiple levels of inheritance
+throw deep_hierarchy(); // Ok, multiple levels of inheritance
   } catch (deep_hierarchy &e) { // Ok
   }
   throw deep_hierarchy(); // Ok
@@ -75,39 +75,39 @@
 class exotic_exception : public T {};
 
 void generic_exceptions() {
-  THROW_EXCEPTION(int); // Bad
+  THROW_EXCEPTION(int);
   // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
-  THROW_EXCEPTION(non_derived_exception); // Bad
+  THROW_EXCEPTION(non_derived_exception);
   // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
   // CHECK MESSAGES: 9:1: note: type defined here
-  THROW_EXCEPTION(std::exception); // Ok
+  THROW_EXCEPTION(std::exception);// Ok
   THROW_EXCEPTION(derived_exception); // Ok
-  THROW_EXCEPTION(deep_hierarchy); // Ok
+  THROW_EXCEPTION(deep_hierarchy);// Ok
 
   THROW_BAD_EXCEPTION;
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
   // CHECK-MESSAGES: [[@LINE-25]]:35: note: expanded from macro 'THROW_BAD_EXCEPTION'
   THROW_GOOD_EXCEPTION;
   THROW_DERIVED_EXCEPTION;
 
-  throw generic_exception(); // Ok,
+  throw generic_exception();// Ok,
   THROW_EXCEPTION(generic_exception); // Ok
 
-  throw bad_generic_exception(); // Bad, not derived
+  throw bad_generic_exception();
   // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception'
-  throw bad_generic_exception(); // Bad as well, since still not derived
+  throw bad_generic_exception();
   // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception'
-  THROW_EXCEPTION(bad_generic_exception); // Bad
+  THROW_EXCEPTION(bad_generic_exception);
   // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception'
-  THROW_EXCEPTION(bad_generic_exception); // Bad
+  THROW_EXCEPTION(bad_generic_exception);
   // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception'
 
-  throw exotic_exception(); // Bad
+  throw exotic_exception();
   // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'exotic_exception' is not derived from 'std::exception'
-  THROW_EXCEPTION(exotic_exception); // Bad
+  THROW_EXCEPTION(exotic_exception);
   // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'exotic_exception' is not derived from 'std::exception'
 
-  throw exotic_exception(); // Ok
+  throw exotic_exception();  // Ok
   THROW_EXCEPTION(exotic_exception); // Ok
 }
 
@@ -118,11 +118,11 @@
 using UsingGood = deep_hierarchy;
 
 void typedefed() {
-  throw TypedefedBad(); // Bad
+  throw TypedefedBad();
   // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose typ

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 113213.
JonasToth added a comment.

fix patch, to diff against master again


https://reviews.llvm.org/D37060

Files:
  clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
  test/clang-tidy/hicpp-exception-baseclass.cpp

Index: test/clang-tidy/hicpp-exception-baseclass.cpp
===
--- test/clang-tidy/hicpp-exception-baseclass.cpp
+++ test/clang-tidy/hicpp-exception-baseclass.cpp
@@ -5,38 +5,124 @@
 } // namespace std
 
 class derived_exception : public std::exception {};
+class deep_hierarchy : public derived_exception {};
 class non_derived_exception {};
 
 void problematic() {
   try {
-throw int(42); // Built in is not allowed
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: throwing an exception whose type is not derived from 'std::exception'
+throw int(42);
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
   } catch (int e) {
   }
-  throw int(42); // Bad
-// CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type is not derived from 'std::exception'
+  throw int(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
 
   try {
-throw non_derived_exception(); // Some class is not allowed
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: throwing an exception whose type is not derived from 'std::exception'
-// CHECK-MESSAGES: 8:1: note: type defined here
+throw non_derived_exception();
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 9:1: note: type defined here
   } catch (non_derived_exception &e) {
   }
-  throw non_derived_exception(); // Bad
-// CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type is not derived from 'std::exception'
-// CHECK-MESSAGES: 8:1: note: type defined here
+  throw non_derived_exception();
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+  // CHECK-MESSAGES: 9:1: note: type defined here
 }
 
 void allowed_throws() {
   try {
-throw std::exception(); // Ok
+throw std::exception(); // Ok
   } catch (std::exception &e) { // Ok
   }
   throw std::exception();
 
   try {
-throw derived_exception(); // Ok
+throw derived_exception(); // Ok
   } catch (derived_exception &e) { // Ok
   }
   throw derived_exception(); // Ok
+
+  try {
+throw deep_hierarchy(); // Ok, multiple levels of inheritance
+  } catch (deep_hierarchy &e) { // Ok
+  }
+  throw deep_hierarchy(); // Ok
+}
+
+// Templated function that throws exception based on template type
+template 
+void ThrowException() { throw T(); }
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-3]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-5]]:31: warning: throwing an exception whose type 'exotic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 74:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-7]]:31: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+// CHECK-MESSAGES: [[@LINE-8]]:31: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 9:1: note: type defined here
+#define THROW_EXCEPTION(CLASS) ThrowException()
+#define THROW_BAD_EXCEPTION throw int(42);
+#define THROW_GOOD_EXCEPTION throw std::exception();
+#define THROW_DERIVED_EXCEPTION throw deep_hierarchy();
+
+template 
+class generic_exception : std::exception {};
+
+template 
+class bad_generic_exception {};
+
+template 
+class exotic_exception : public T {};
+
+void generic_exceptions() {
+  THROW_EXCEPTION(int);
+  // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+  THROW_EXCEPTION(non_derived_exception);
+  // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+  // CHECK MESSAGES: 9:1: note: type defined here
+  THROW_EXCEPTION(std::exception);// Ok
+  THROW_EXCEPTION(derived_exception); // Ok
+  THROW_EXCEPTION(deep_hierarchy);// Ok
+
+  THROW_BAD_EXCEPTION;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+  // CHECK-MESSAGES: [[@LINE-25]]:35: note: expanded from macro 'THROW_BAD_EXCEPTION'
+  THROW_GOOD_EXCEPTION;
+  THROW_DERIVED_EXCEPTION;
+
+  throw generic_exception();// Ok,

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 113214.
JonasToth added a comment.

struggling with arc...


https://reviews.llvm.org/D37060

Files:
  clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
  test/clang-tidy/hicpp-exception-baseclass.cpp

Index: test/clang-tidy/hicpp-exception-baseclass.cpp
===
--- test/clang-tidy/hicpp-exception-baseclass.cpp
+++ test/clang-tidy/hicpp-exception-baseclass.cpp
@@ -5,38 +5,124 @@
 } // namespace std
 
 class derived_exception : public std::exception {};
+class deep_hierarchy : public derived_exception {};
 class non_derived_exception {};
 
 void problematic() {
   try {
-throw int(42); // Built in is not allowed
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: throwing an exception whose type is not derived from 'std::exception'
+throw int(42);
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
   } catch (int e) {
   }
-  throw int(42); // Bad
-// CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type is not derived from 'std::exception'
+  throw int(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
 
   try {
-throw non_derived_exception(); // Some class is not allowed
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: throwing an exception whose type is not derived from 'std::exception'
-// CHECK-MESSAGES: 8:1: note: type defined here
+throw non_derived_exception();
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 9:1: note: type defined here
   } catch (non_derived_exception &e) {
   }
-  throw non_derived_exception(); // Bad
-// CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type is not derived from 'std::exception'
-// CHECK-MESSAGES: 8:1: note: type defined here
+  throw non_derived_exception();
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+  // CHECK-MESSAGES: 9:1: note: type defined here
 }
 
 void allowed_throws() {
   try {
-throw std::exception(); // Ok
+throw std::exception(); // Ok
   } catch (std::exception &e) { // Ok
   }
   throw std::exception();
 
   try {
-throw derived_exception(); // Ok
+throw derived_exception(); // Ok
   } catch (derived_exception &e) { // Ok
   }
   throw derived_exception(); // Ok
+
+  try {
+throw deep_hierarchy(); // Ok, multiple levels of inheritance
+  } catch (deep_hierarchy &e) { // Ok
+  }
+  throw deep_hierarchy(); // Ok
+}
+
+// Templated function that throws exception based on template type
+template 
+void ThrowException() { throw T(); }
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-3]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-5]]:31: warning: throwing an exception whose type 'exotic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 74:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-7]]:31: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+// CHECK-MESSAGES: [[@LINE-8]]:31: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 9:1: note: type defined here
+#define THROW_EXCEPTION(CLASS) ThrowException()
+#define THROW_BAD_EXCEPTION throw int(42);
+#define THROW_GOOD_EXCEPTION throw std::exception();
+#define THROW_DERIVED_EXCEPTION throw deep_hierarchy();
+
+template 
+class generic_exception : std::exception {};
+
+template 
+class bad_generic_exception {};
+
+template 
+class exotic_exception : public T {};
+
+void generic_exceptions() {
+  THROW_EXCEPTION(int);
+  // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+  THROW_EXCEPTION(non_derived_exception);
+  // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+  // CHECK MESSAGES: 9:1: note: type defined here
+  THROW_EXCEPTION(std::exception);// Ok
+  THROW_EXCEPTION(derived_exception); // Ok
+  THROW_EXCEPTION(deep_hierarchy);// Ok
+
+  THROW_BAD_EXCEPTION;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+  // CHECK-MESSAGES: [[@LINE-25]]:35: note: expanded from macro 'THROW_BAD_EXCEPTION'
+  THROW_GOOD_EXCEPTION;
+  THROW_DERIVED_EXCEPTION;
+
+  throw generic_exception();// Ok,
+  THROW_EXCEPTI

[PATCH] D37210: [refactor] add a refactoring action rule that returns symbol occurrences

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: include/clang/Tooling/Refactoring/RefactoringActionRule.h:29
+SourceChangeRefactoringRuleKind,
+FindSymbolOccurrencesRefactoringRuleKind
+  };

I might miss some context here. As per your comment in 
https://reviews.llvm.org/D36075#inline-323769, you'll try to remove this tag, 
so I think we will hold off this patch until that is done?



Comment at: unittests/Tooling/RefactoringActionRulesTest.cpp:167
+Expected>
+findOccurrences(const std::unique_ptr &Rule,
+RefactoringRuleContext &Context) {

I'm not a fan of passing a const smart_pointer& as function parameter. The 
function being called doesn't care about the lifetime management, so I'd use a 
raw pointer here.


Repository:
  rL LLVM

https://reviews.llvm.org/D37210



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

[PATCH] D15465: [git-clang-format]: New option to perform formatting against staged changes only

[Jason Newton via Phabricator via cfe-commits]

nevion added a comment.

I'm paying attention at least.  I updated your patch prior to your posting and 
temporarily made due with it.  I'm pretty nervous that I will lose work with my 
commit style and the lingering issue.  Based on what I've seen so far I can't 
use the git hooks and so I want to do this more manually which entices more 
accidents as that's when staged data is more frequent.


Repository:
  rL LLVM

https://reviews.llvm.org/D15465



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

r312106 - [docs] Regenerate command line options reference

[Boris Kolpackov via cfe-commits]

Author: borisk
Date: Wed Aug 30 02:15:53 2017
New Revision: 312106

URL: http://llvm.org/viewvc/llvm-project?rev=312106&view=rev
Log:
[docs] Regenerate command line options reference

Modified:
cfe/trunk/docs/ClangCommandLineReference.rst

Modified: cfe/trunk/docs/ClangCommandLineReference.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangCommandLineReference.rst?rev=312106&r1=312105&r2=312106&view=diff
==
--- cfe/trunk/docs/ClangCommandLineReference.rst (original)
+++ cfe/trunk/docs/ClangCommandLineReference.rst Wed Aug 30 02:15:53 2017
@@ -56,6 +56,16 @@ Pass  to fatbinary invocation
 
 Pass  to the ptxas assembler
 
+.. option:: -Xopenmp-target 
+
+Pass  to the target offloading toolchain.
+
+.. program:: clang1
+.. option:: -Xopenmp-target= 
+.. program:: clang
+
+Pass  to the specified target offloading toolchain. The triple that 
identifies the toolchain must be provided after the equals sign.
+
 .. option:: -Z
 
 .. option:: -a, --profile-blocks
@@ -312,6 +322,10 @@ Disable standard #include directories fo
 
 .. option:: -nostdlib, --no-standard-libraries
 
+.. program:: clang1
+.. option:: -nostdlib++
+.. program:: clang
+
 .. option:: -nostdlibinc
 
 .. option:: -o, --output , --output=
@@ -656,6 +670,10 @@ Pass  to the assembler
 
 Pass  to the clang compiler
 
+.. option:: -fclang-abi-compat=
+
+Attempt to match the ABI of Clang 
+
 .. option:: -fcomment-block-commands=,...
 
 Treat each comma separated argument in  as a documentation comment block 
command
@@ -742,6 +760,8 @@ Enable origins tracking in MemorySanitiz
 
 Enable use-after-destroy detection in MemorySanitizer
 
+.. option:: -fsanitize-minimal-runtime, -fno-sanitize-minimal-runtime
+
 .. option:: -fsanitize-recover, -fno-sanitize-recover
 
 .. program:: clang1
@@ -852,6 +872,10 @@ Use the last modification time of 
 
 Time when the current build session started
 
+.. option:: -fmodule-file=\[=\]
+
+Specify the mapping of module name to precompiled module file, or load a 
module file if name is omitted.
+
 .. option:: -fmodules-cache-path=
 
 Specify the module cache path
@@ -1361,10 +1385,6 @@ Specify the maximum alignment to enforce
 
 .. option:: -fmodule-file-deps, -fno-module-file-deps
 
-.. option:: -fmodule-file=
-
-Load this precompiled module file
-
 .. option:: -fmodule-map-file=
 
 Load this module map file
@@ -1447,6 +1467,10 @@ Do not treat C++ operator name keywords
 
 .. option:: -fno-working-directory
 
+.. option:: -fnoopenmp-relocatable-target
+
+Do not compile OpenMP target code as relocatable.
+
 .. option:: -fnoopenmp-use-tls
 
 .. option:: -fobjc-abi-version=
@@ -1489,6 +1513,10 @@ Enable ARC-style weak references in Obje
 
 .. option:: -fopenmp-dump-offload-linker-script
 
+.. option:: -fopenmp-relocatable-target
+
+OpenMP target code is compiled as relocatable using the -c flag. For OpenMP 
targets the code is relocatable by default.
+
 .. option:: -fopenmp-use-tls
 
 .. option:: -fopenmp-version=
@@ -1567,6 +1595,13 @@ Generate instrumented code to collect ex
 
 Use instrumentation data for profile-guided optimization
 
+.. option:: -fprofile-sample-accurate, -fauto-profile-accurate, 
-fno-profile-sample-accurate
+
+Specifies that the sample profile is accurate. If the sample
+   profile is accurate, callsites without profile samples are 
marked
+   as cold. Otherwise, treat callsites without profile samples as 
if
+   we have no profile
+
 .. option:: -fprofile-sample-use, -fauto-profile, -fno-profile-sample-use
 
 .. program:: clang1
@@ -1901,6 +1936,8 @@ Put objects of at most  bytes into
 
 Enable SVR4-style position-independent code (Mips only)
 
+.. option:: -mabs=
+
 .. option:: -malign-double
 
 Align doubles to two words in structs (x86 only)
@@ -1939,6 +1976,14 @@ Link stack frames through backchain on S
 
 Set EABI type, e.g. 4, 5 or gnu (default depends on triple)
 
+.. option:: -membedded-data, -mno-embedded-data
+
+Place constants in the .rodata section instead of the .sdata section even if 
they meet the -G  threshold (MIPS)
+
+.. option:: -mextern-sdata, -mno-extern-sdata
+
+Assume that externally defined data is in the small data if it meets the -G 
 threshold (MIPS)
+
 .. option:: -mfentry
 
 Insert calls to fentry at function entry (x86 only)
@@ -1989,6 +2034,10 @@ Use Intel MCU ABI
 
 .. option:: -mldc1-sdc1, -mno-ldc1-sdc1
 
+.. option:: -mlocal-sdata, -mno-local-sdata
+
+Extend the -G behaviour to object local data (MIPS)
+
 .. option:: -mlong-calls, -mno-long-calls
 
 Generate branches with extended addressability, usually via indirect jumps.


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

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 113217.
JonasToth marked 5 inline comments as done.
JonasToth added a comment.

- fix spelling and grammar errors
- adjust unit test to new diagnostics


https://reviews.llvm.org/D36354

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp
  clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
  clang-tidy/utils/Matchers.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-owning-memory.cpp

Index: test/clang-tidy/cppcoreguidelines-owning-memory.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-owning-memory.cpp
@@ -0,0 +1,349 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-owning-memory %t
+
+namespace gsl {
+template 
+using owner = T;
+} // namespace gsl
+
+template 
+class unique_ptr {
+public:
+  unique_ptr(gsl::owner resource) : memory(resource) {}
+  unique_ptr(const unique_ptr &) = default;
+
+  ~unique_ptr() { delete memory; }
+
+private:
+  gsl::owner memory;
+};
+
+void takes_owner(gsl::owner owned_int) {
+}
+
+void takes_pointer(int *unowned_int) {
+}
+
+void takes_owner_and_more(int some_int, gsl::owner owned_int, float f) {
+}
+
+template 
+void takes_templated_owner(gsl::owner owned_T) {
+}
+
+gsl::owner returns_owner1() { return gsl::owner(new int(42)); } // Ok
+gsl::owner returns_owner2() { return new int(42); } // Ok
+
+int *returns_no_owner1() { return nullptr; }
+int *returns_no_owner2() {
+  return new int(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: returning a 'gsl::owner<>' from a function but not declaring it; return type is 'int *'
+}
+int *returns_no_owner3() {
+  int *should_be_owner = new int(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+  return should_be_owner;
+}
+int *returns_no_owner4() {
+  gsl::owner owner = new int(42);
+  return owner;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: returning a 'gsl::owner<>' from a function but not declaring it; return type is 'int *'
+}
+
+unique_ptr returns_no_owner5() {
+  return unique_ptr(new int(42)); // Ok
+}
+
+/// FIXME: CSA finds it, but the report is misleading.
+void csa_not_finding_leak() {
+  gsl::owner o1 = new int(42); // Ok
+
+  gsl::owner o2 = o1; // Ok
+  o2 = new int(45); // conceptual leak, the memory from o1 is now leaked, since its considered moved in the guidelines
+
+  delete o2;
+  // actual leak occurs here, its found, but mixed
+  delete o1;
+}
+
+void test_assignment_and_initialization() {
+  int stack_int1 = 15;
+  int stack_int2;
+
+  gsl::owner owned_int1 = &stack_int1; // BAD
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected initialization with value of type 'gsl::owner<>'; got 'int *'
+
+  gsl::owner owned_int2;
+  owned_int2 = &stack_int2; // BAD since no owner, bad since uninitialized
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected assignment source to be of type 'gsl::owner<>'; got 'int *'
+
+  gsl::owner owned_int3 = new int(42); // Good
+  owned_int3 = nullptr; // Good
+
+  gsl::owner owned_int4(nullptr); // Ok
+  owned_int4 = new int(42); // Good
+
+  gsl::owner owned_int5 = owned_int3; // Good
+
+  gsl::owner owned_int6{nullptr}; // Ok
+  owned_int6 = owned_int4; // Good
+
+  // FIXME:, flow analysis for the case of reassignment. Value must be released before
+  owned_int6 = owned_int3; // BAD, because reassignment without resource release
+
+  auto owned_int7 = returns_owner1(); // Bad, since typededuction eliminates the owner wrapper
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: type deduction did not result in an owner
+
+  const auto owned_int8 = returns_owner2(); // Bad, since typededuction eliminates the owner wrapper
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *const' with a newly created 'gsl::owner<>'
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: type deduction did not result in an owner
+
+  gsl::owner owned_int9 = returns_owner1(); // Ok
+  int *unowned_int3 = returns_owner1(); // Bad
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+
+  gsl::owner owned_int10;
+  owned_int10 = returns_owner1(); // Ok
+
+  int *unowned_int4;
+  unowned_int4 = returns_owner1(); // Bad
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: assigning newly created 'gsl::owner<>' to non-owner 'int *'
+
+  gsl::owner owned_int11 = returns_no_owner1(); // Bad since no owner
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected initialization with value of type 'gsl::owner<>'; got 'int *'
+
+  gsl::owner owned_int12;
+  owned_int12 = returns_no_owner1(); // Bad since no owner
+  // CHECK-MESSAGES: [[@LIN

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

addressed some issues, not all yet


https://reviews.llvm.org/D36354



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

[PATCH] D37210: [refactor] add a refactoring action rule that returns symbol occurrences

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: include/clang/Tooling/Refactoring/RefactoringActionRule.h:29
+SourceChangeRefactoringRuleKind,
+FindSymbolOccurrencesRefactoringRuleKind
+  };

hokein wrote:
> I might miss some context here. As per your comment in 
> https://reviews.llvm.org/D36075#inline-323769, you'll try to remove this tag, 
> so I think we will hold off this patch until that is done?
Yeah, that would be better. I will remove the tag first.


Repository:
  rL LLVM

https://reviews.llvm.org/D37210



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

Any news on this?
cc @djasper


Repository:
  rL LLVM

https://reviews.llvm.org/D28462



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

[PATCH] D36586: [clang-tidy] hicpp bitwise operations on signed integers

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/hicpp/SignedBitwiseCheck.cpp:23
+  const auto SignedIntegerOperand =
+  
expr(ignoringImpCasts(hasType(isSignedInteger(.bind("signed_operand");
+

JonasToth wrote:
> aaron.ballman wrote:
> > JonasToth wrote:
> > > aaron.ballman wrote:
> > > > JonasToth wrote:
> > > > > aaron.ballman wrote:
> > > > > > Is ignoring implicit casts the correct behavior here as far as the 
> > > > > > coding standard is concerned? Consider:
> > > > > > ```
> > > > > > unsigned char c = 128;
> > > > > > f(~c); // c promotes to int before ~ is applied
> > > > > > ```
> > > > > > Looking at the coding standard, I get the impression this is not 
> > > > > > allowed, but I'm not *super* familiar with HIC++.
> > > > > I first implemented it without ignoring the implicit integer casts, 
> > > > > the result was, that most cases (in test cases) where not found. 
> > > > > therefore i implemented it that way. I add an testcase for this and 
> > > > > see how i need to adjust the matcher.
> > > > > 
> > > > > Could you help me there with the semantic, since i am not so fluent 
> > > > > in C/C++ standardese, but i think the findings are reasonable.
> > > > It kind of boils down to the intention from the HIC++. Consider a test 
> > > > case like:
> > > > ```
> > > > void f(int i);
> > > > 
> > > > void g() {
> > > >   unsigned char c = 127;
> > > >   f(~c);
> > > > }
> > > > 
> > > > ```
> > > > Does `f()` expect to receive `-128` or `128`? I think this code will 
> > > > pass your check (ignoring the promotion means the type is `unsigned 
> > > > char`), but the actual bitwise operation is on a signed integer value 
> > > > because there is an integer promotion. So 127 is promoted to int, then 
> > > > ~ is applied, resulting in the value `-128` being passed to the 
> > > > function.
> > > Yeah i see, i have such cases added in the tests.
> > > TBH. i don't know if the standard wants this covered, but the 
> > > demonstrated case is definitly bad.
> > > 
> > > Would it be a good idea, to warn on assigning/initializing `signed` 
> > > integers with `unsigned` integers?
> > > 
> > > The CppCoreGuidelines have some sections on that as well: [[ 
> > > https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#arithmetic
> > >  | Section ES.100-103 ]]
> > > 
> > > Not sure if this check should care. On the other hand, would it  be nice 
> > > to have a check that covers all "integer problems".
> > > Yeah i see, i have such cases added in the tests.
> > > TBH. i don't know if the standard wants this covered, but the 
> > > demonstrated case is definitly bad.
> > 
> > I think you should ask the HIC++ people what they think; the rule text does 
> > not make it clear what the behavior should be here.
> > 
> > > Would it be a good idea, to warn on assigning/initializing signed 
> > > integers with unsigned integers?
> > 
> > We already have such a warning in clang (-Wsign-conversion).
> > 
> > >The CppCoreGuidelines have some sections on that as well: Section 
> > >ES.100-103
> > >
> > >Not sure if this check should care. On the other hand, would it be nice to 
> > >have a check that covers all "integer problems".
> > 
> > Any such check will require so many options that it is likely to be almost 
> > unusable. However, I would not be opposed to seeing a clang-tidy module 
> > that has a bunch of checks in it that relate to integer problems.
> i think, those could land in `bugprone-`, and be aliases to hicpp and the 
> cppcoreguidelines if appropriate.
> 
> depending on my time (i should have a lot of free time for 2 months) 
> i will try to implement some.
> is there a resource with all common problems found with integers?
The response from Christof:



> Hi Jonas,
> 
> using bitwise operators with unsigned operands is compliant with this
> rule, even if the operand is promoted to signed int.
> 
> expr.unary.op in the C++ standard says "The operand of ~ shall have
> integral or unscoped enumeration type; [...] Integral promotions are
> performed. The type of the result is the type of the promoted operand."
> 
> The HICPP rule refers to the type of the operand (and not the type of
> the promoted operand), it therefore refers to the type before promotion
> (which is "unsigned char" in the example).
> 
> 
> Regards,
> Christof

Your requested case is therefore conforming and i think the current 
implementation handles these things correct.


https://reviews.llvm.org/D36586



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

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

[Raoul Wols via Phabricator via cfe-commits]

rwols updated this revision to Diff 113219.
rwols added a comment.

Tidy up snippet handling

- Put CK_Informative chunks in the label
- Assert that there's at most one CK_ResultType chunk
- CK_CurrentParameter never occurs while collecting completions, only while 
handling overloads.
- For CK_VerticalSpace, use a space for the label (but a newline for the 
insertText).
- Move the SnippetEscape function outside the class and into an unnamed 
namespace.


https://reviews.llvm.org/D37101

Files:
  clangd/ClangdUnit.cpp
  clangd/Protocol.cpp

Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -724,8 +724,8 @@
   if (!CI.insertText.empty())
 Os << R"("insertText":")" << llvm::yaml::escape(CI.insertText) << R"(",)";
   if (CI.insertTextFormat != InsertTextFormat::Missing) {
-Os << R"("insertTextFormat":")" << static_cast(CI.insertTextFormat)
-   << R"(",)";
+Os << R"("insertTextFormat":)" << static_cast(CI.insertTextFormat)
+   << R"(,)";
   }
   if (CI.textEdit)
 Os << R"("textEdit":)" << TextEdit::unparse(*CI.textEdit) << ',';
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -272,6 +272,17 @@
   }
 }
 
+std::string SnippetEscape(const llvm::StringRef Text) {
+  std::string Result;
+  Result.reserve(Text.size()); // Assume '$', '}' and '\\' are rare.
+  for (const auto Character : Text) {
+if (Character == '$' || Character == '}' || Character == '\\')
+  Result.push_back('\\');
+Result.push_back(Character);
+  }
+  return Result;
+}
+
 class CompletionItemsCollector : public CodeCompleteConsumer {
   std::vector *Items;
   std::shared_ptr Allocator;
@@ -288,45 +299,161 @@
   void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,
   CodeCompletionResult *Results,
   unsigned NumResults) override {
-for (unsigned I = 0; I != NumResults; ++I) {
-  CodeCompletionResult &Result = Results[I];
-  CodeCompletionString *CCS = Result.CreateCodeCompletionString(
+assert(Items && "We need a non-null items container here");
+Items->reserve(NumResults);
+for (unsigned I = 0; I < NumResults; ++I) {
+  auto &Result = Results[I];
+  const auto *CCS = Result.CreateCodeCompletionString(
   S, Context, *Allocator, CCTUInfo,
   CodeCompleteOpts.IncludeBriefComments);
-  if (CCS) {
-CompletionItem Item;
-for (CodeCompletionString::Chunk C : *CCS) {
-  switch (C.Kind) {
-  case CodeCompletionString::CK_ResultType:
-Item.detail = C.Text;
-break;
-  case CodeCompletionString::CK_Optional:
-break;
-  default:
-Item.label += C.Text;
-break;
-  }
-}
-assert(CCS->getTypedText());
-Item.kind = getKind(Result.CursorKind);
-// Priority is a 16-bit integer, hence at most 5 digits.
-assert(CCS->getPriority() < 9 && "Expecting code completion result "
- "priority to have at most "
- "5-digits");
-llvm::raw_string_ostream(Item.sortText)
-<< llvm::format("%05d%s", CCS->getPriority(), CCS->getTypedText());
-Item.insertText = Item.filterText = CCS->getTypedText();
-if (CCS->getBriefComment())
-  Item.documentation = CCS->getBriefComment();
-Items->push_back(std::move(Item));
-  }
+  assert(CCS && "Expected the CodeCompletionString to be non-null");
+  Items->push_back(ProcessCodeCompleteResult(Result, *CCS));
 }
   }
 
   GlobalCodeCompletionAllocator &getAllocator() override { return *Allocator; }
 
   CodeCompletionTUInfo &getCodeCompletionTUInfo() override { return CCTUInfo; }
-};
+
+private:
+  CompletionItem
+  ProcessCodeCompleteResult(const CodeCompletionResult &Result,
+const CodeCompletionString &CCS) const {
+// The object that we'll return.
+CompletionItem Item;
+
+// Always a snippet. This is because CK_Informative chunks should not be
+// inserted into the text buffer, but they are part of the label. For
+// example, "foo::bar() const" is the label, but "bar()" is the insertText.
+Item.insertTextFormat = InsertTextFormat::Snippet;
+
+// Things like __attribute__((nonnull(1,3))) and [[noreturn]]. Present this
+// information in the documentation field.
+if (CCS.getAnnotationCount() > 0) {
+  Item.documentation += "Annotation";
+  if (CCS.getAnnotationCount() == 1) {
+Item.documentation += ": ";
+  } else /* CCS.getAnnotationCount() > 1 */ {
+Item.documentation += "s: ";
+  }
+  for (unsigned j = 0; j < CCS.getAnnotationCount(); ++j) {
+Item.documentation += CCS

[PATCH] D33719: Add _Float16 as a C/C++ source language type

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 113218.
SjoerdMeijer added a comment.

Comments addressed. Thanks for reviewing.


https://reviews.llvm.org/D33719

Files:
  include/clang-c/Index.h
  include/clang/AST/ASTContext.h
  include/clang/AST/BuiltinTypes.def
  include/clang/Basic/Specifiers.h
  include/clang/Basic/TokenKinds.def
  include/clang/Lex/LiteralSupport.h
  include/clang/Sema/DeclSpec.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/AST/NSAPI.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/Type.cpp
  lib/AST/TypeLoc.cpp
  lib/Analysis/PrintfFormatString.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Format/FormatToken.cpp
  lib/Index/USRGeneration.cpp
  lib/Lex/LiteralSupport.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseTentative.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaTemplateVariadic.cpp
  lib/Sema/SemaType.cpp
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReader.cpp
  test/CodeGenCXX/float16-declarations.cpp
  test/Frontend/float16.cpp
  test/Lexer/half-literal.cpp
  tools/libclang/CXType.cpp

Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,7 @@
 BTCASE(Float);
 BTCASE(Double);
 BTCASE(LongDouble);
+BTCASE(Float16);
 BTCASE(Float128);
 BTCASE(NullPtr);
 BTCASE(Overload);
@@ -520,7 +521,7 @@
 TKIND(Char_U);
 TKIND(UChar);
 TKIND(Char16);
-TKIND(Char32);  
+TKIND(Char32);
 TKIND(UShort);
 TKIND(UInt);
 TKIND(ULong);
@@ -538,6 +539,7 @@
 TKIND(Float);
 TKIND(Double);
 TKIND(LongDouble);
+TKIND(Float16);
 TKIND(Float128);
 TKIND(NullPtr);
 TKIND(Overload);
Index: test/Lexer/half-literal.cpp
===
--- test/Lexer/half-literal.cpp
+++ test/Lexer/half-literal.cpp
@@ -1,3 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -pedantic %s
 float a = 1.0h; // expected-error{{invalid suffix 'h' on floating constant}}
 float b = 1.0H; // expected-error{{invalid suffix 'H' on floating constant}}
+
+_Float16 c = 1.f166; // expected-error{{invalid suffix 'f166' on floating constant}}
+_Float16 d = 1.f1;   // expected-error{{invalid suffix 'f1' on floating constant}}
Index: test/Frontend/float16.cpp
===
--- /dev/null
+++ test/Frontend/float16.cpp
@@ -0,0 +1,326 @@
+// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
+// RUN: %clang_cc1 -std=c++11 -ast-dump -fnative-half-type %s | FileCheck %s --check-prefix=CHECK-NATIVE --strict-whitespace
+
+/*  Various contexts where type _Float16 can appear. */
+
+/*  Namespace */
+namespace {
+  _Float16 f1n;
+  _Float16 f2n = 33.f16;
+  _Float16 arr1n[10];
+  _Float16 arr2n[] = { 1.2, 3.0, 3.e4 };
+  const volatile _Float16 func1n(const _Float16 &arg) {
+return arg + f2n + arr1n[4] - arr2n[1];
+  }
+}
+
+//CHECK:  |-NamespaceDecl
+//CHECK-NEXT: | |-VarDecl {{.*}} f1n '_Float16'
+//CHECK-NEXT: | |-VarDecl {{.*}} f2n '_Float16' cinit
+//CHECK-NEXT: | | `-FloatingLiteral {{.*}} '_Float16' 3.30e+01
+//CHECK-NEXT: | |-VarDecl {{.*}} arr1n '_Float16 [10]'
+//CHECK-NEXT: | |-VarDecl {{.*}} arr2n '_Float16 [3]' cinit
+//CHECK-NEXT: | | `-InitListExpr {{.*}} '_Float16 [3]'
+//CHECK-NEXT: | |   |-ImplicitCastExpr {{.*}} '_Float16' 
+//CHECK-NEXT: | |   | `-FloatingLiteral {{.*}} 'double' 1.20e+00
+//CHECK-NEXT: | |   |-ImplicitCastExpr {{.*}} '_Float16' 
+//CHECK-NEXT: | |   | `-FloatingLiteral {{.*}} 'double' 3.00e+00
+//CHECK-NEXT: | |   `-ImplicitCastExpr {{.*}} '_Float16' 
+//CHECK-NEXT: | | `-FloatingLiteral {{.*}} 'double' 3.00e+04
+//CHECK-NEXT: | `-FunctionDecl {{.*}} func1n 'const volatile _Float16 (const _Float16 &)'
+
+/* File */
+_Float16 f1f;
+_Float16 f2f = 32.4;
+_Float16 arr1f[10];
+_Float16 arr2f[] = { -1.2, -3.0, -3.e4 };
+_Float16 func1f(_Float16 arg);
+
+//CHECK:  |-VarDecl {{.*}} f1f '_Float16'
+//CHECK-NEXT: |-VarDecl {{.*}} f2f '_Float16' cinit
+//CHECK-NEXT: | `-ImplicitCastExpr {{.*}} '_Float16' 
+//CHECK-NEXT: |   `-FloatingLiteral {{.*}} 'double' 3.24e+01
+//CHECK-NEXT: |-VarDecl {{.*}} arr1f '_Float16 [10]'
+//CHECK-NEXT: |-VarDecl {{.*}} arr2f '_Float16 [3]' cinit
+//CHECK-NEXT: | `-InitListExpr {{.*}} '_Float16 [3]'
+//CHECK-NEXT: |   |-ImplicitCastExpr {{.*}} '_Float16' 
+//CHECK-NEXT: |   | `-UnaryOperator {{.*}} 'double' prefix '-'
+//CHECK-NEXT: |   |   `-FloatingLiteral {{.*}} 'double' 1.20e+00
+//CHECK-NEXT: |   |-ImplicitCastExpr {{.*}} '_Float16' 
+//CHECK-NEXT: |   | `-UnaryOperator {{.*}} 'double' prefix '-'
+//CHECK-NEXT: |   |   `-FloatingLiteral {{.*}} 'double' 3.

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

[Raoul Wols via Phabricator via cfe-commits]

rwols marked 10 inline comments as done.
rwols added a comment.

I followed your advice and kept the snippet functionality. We'll do the 
SignatureHelp stuff in another review.

A "major" change is that, because CK_Informative chunks are put into the label 
now, we have to use the insertText, always.


https://reviews.llvm.org/D37101



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

[PATCH] D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
Herald added a reviewer: JonasToth.

This patch changes the way that the refactoring results are produced. Instead 
of using different `RefactoringActionRule` subclasses for each result type, we 
now use a single `RefactoringResultConsumer`. This was suggested by Manuel in 
https://reviews.llvm.org/D36075.


Repository:
  rL LLVM

https://reviews.llvm.org/D37291

Files:
  include/clang/Tooling/Refactoring/RefactoringActionRule.h
  include/clang/Tooling/Refactoring/RefactoringActionRules.h
  include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
  include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
  unittests/Tooling/RefactoringActionRulesTest.cpp

Index: unittests/Tooling/RefactoringActionRulesTest.cpp
===
--- unittests/Tooling/RefactoringActionRulesTest.cpp
+++ unittests/Tooling/RefactoringActionRulesTest.cpp
@@ -35,8 +35,27 @@
 Expected>
 createReplacements(const std::unique_ptr &Rule,
RefactoringRuleContext &Context) {
-  return cast(*Rule).createSourceReplacements(
-  Context);
+  class Consumer final : public RefactoringResultConsumer {
+void handleInitiationFailure() {
+  Result = Expected>(None);
+}
+void handleInitiationError(llvm::Error Err) { Result = std::move(Err); }
+
+void handleInvocationError(llvm::Error Err) {
+  handleInitiationError(std::move(Err));
+}
+
+void handle(AtomicChanges SourceReplacements) {
+  Result = std::move(SourceReplacements);
+}
+
+  public:
+Optional>> Result;
+  };
+
+  Consumer C;
+  Rule->invoke(C, Context);
+  return std::move(*C.Result);
 }
 
 TEST_F(RefactoringActionRulesTest, MyFirstRefactoringRule) {
Index: include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
===
--- /dev/null
+++ include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
@@ -0,0 +1,72 @@
+//===--- RefactoringResultConsumer.h - Clang refactoring library --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H
+#define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H
+
+#include "clang/Basic/LLVM.h"
+#include "clang/Tooling/Refactoring/AtomicChange.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace tooling {
+
+/// An abstract interface that consumes the various refactoring results that can
+/// be produced by refactoring actions.
+///
+/// A valid refactoring result must be handled by a \c handle method.
+class RefactoringResultConsumer {
+public:
+  virtual ~RefactoringResultConsumer() {}
+
+  /// Handles an initiation failure. An action typically fails to initiate when
+  /// the source selection has no overlap at all with any relevant AST nodes.
+  virtual void handleInitiationFailure() = 0;
+
+  /// Handles an initation error. The error typically has a \c DiagnosticError
+  /// payload that describes why initation failed.
+  virtual void handleInitiationError(llvm::Error Err) = 0;
+
+  virtual void handleInvocationError(llvm::Error Err) = 0;
+
+  /// The specific rule must return an llvm::Error with a DiagnosticError
+  /// payload or None when the refactoring action couldn't be initiated/
+  /// performed, or \c AtomicChanges when the action was performed successfully.
+  ///
+  /// Handles the source replacements that are produced by a refactoring action.
+  virtual void handle(AtomicChanges SourceReplacements) = 0;
+};
+
+namespace traits {
+namespace internal {
+
+template  struct HasHandle {
+private:
+  template 
+  static auto check(ClassT *) -> typename std::is_same<
+  decltype(std::declval().handle(std::declval())), void>::type;
+
+  template  static std::false_type check(...);
+
+public:
+  using Type = decltype(check(nullptr));
+};
+
+} // end namespace internal
+
+/// A type trait that returns true iff the given type is a valid refactoring
+/// result.
+template 
+struct IsValidRefactoringResult : internal::HasHandle::Type {};
+
+} // end namespace traits
+} // end namespace tooling
+} // end namespace clang
+
+#endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H
Index: include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
===
--- include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
+++ include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
@@ -13,6 +13,7 @@
 #include "clang/Basic/LLVM.h"
 #include "clang/Tooling/Refactoring/RefactoringActionRule.h"
 #include "clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h"
+#include "clang/Tooling/Refactoring/RefactoringResultConsumer.

[PATCH] D35216: [analyzer] Escape symbols when creating std::initializer_list.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D35216#856093, @rsmith wrote:

> The `CXXStdInitializerListExpr` node has pretty simple evaluation semantics: 
> it takes a glvalue array expression, and constructs a 
> `std::initializer_list` from it as if by filling in the two members with a 
> pointer to the array and either the size of the array or a pointer to its 
> end. Can we not model those semantics directly here?


Yeah, it's definitely an easy case to perform complete modeling. However, in 
the analyzer we still lack some infrastructure for C++ library modeling, which 
was discussed quite a lot above, and i feel we've run out of debating time and 
still need to fix the false positive. It's important to make our checker APIs 
more powerful before we jump into modeling myriads of STL classes. The main 
problem is how to represent the private fields in the 
`std::initializer_list` object in our symbolic memory model. Here's a quick 
summary of the above.

We already have the object's fields in the AST, with its AST record layout, 
however field names and layouts are implementation-dependent, and it is unsafe 
to try to understand how the specific standard library implementation's layout 
works and what specific private fields we see in the AST mean. Instead it seems 
safer to attach metadata to the object, which would contain values of these 
private fields - this is a common operation for the analyzer (eg. attach string 
length metadata to a null-terminated C-string).

However, because such metadata should be handled by an analyzer's checker (we 
cannot possibly model all APIs in the core, moving it into checkers is much 
more scalable and natural), and therefore such metadata would be private to the 
checker in one way or another, it makes checkers fully responsible for modeling 
such metadata. In particular, the very feature lack of which that was causing 
the false positive (expressing the fact that if the list is passed into an 
external function (eg. with no visible body available for analysis), objects 
stored in it are also "escaping" into a function and can be, for example, 
deallocated here, so other checkers need to be notified to stop tracking them) 
cannot currently be implemented on the checker side. If we let checkers express 
this, it'd require an additional checker callback boilerplate that would need 
to be repeated in every C++ checker, and we already have a lot of boilerplate. 
Additionally, when constructing the `initializer_list`'s elements, we already 
need to have the storage for them, which is hard to handle by the core when 
such storage is checker-specific.

Another solution would be to let the analyzer core help checkers with the 
simple boilerplate, which is a long-standing project that is to be took up. The 
only downside of this approach is that it might make checker APIs less 
omnipotent, because they can no longer maintain their metadata in arbitrary 
manners, but only in manners that are understandable by the core.

An alternative solution described here was to let the checkers produce 
"imaginary" fields within our symbolic memory model, with unknown offsets 
within the object (similar to objective-c instance variables) that would have 
benefits of no longer being implementation-defined. The difference with 
maintaining metadata is that most of the boilerplate would be handled by the 
memory model (RegionStore) rather than the checker, and the core would 
generally know that the pointers to the array are located within the object. 
However, this also raises multiple concerns, because such imaginary "ghost" 
fields may conflict with the real fields in weird manners, they may 
accidentally leak to user's diagnostics, and what not. And we still have the 
problem with checker-specific storage.

So these are the excuses :)


https://reviews.llvm.org/D35216



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

[PATCH] D37150: [clangd] Command line arg to specify compile_commands.json path

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: clangd/GlobalCompilationDatabase.cpp:89
+  // if --compileCommands arg was invoked, check value and override default 
path
+  std::size_t found = CompileCommands.find_first_of("/");
+  std::string TempString = CompileCommands;

ilya-biryukov wrote:
> Please check command arguments for validity in `main()`. 
> What is the purpose of `find_first_of("/")`? Checking for absolute paths? If 
> yes, you could use `llvm::sys::path::is_absolute`.
Please make `CompileCommands` a member variable and pass it through the 
constructor of `DirectoryBasedGlobalCompilationDatabase`. This is more flexible 
and, among other things, allows for easier unit testing.



Comment at: clangd/GlobalCompilationDatabase.cpp:94
+ File = TempString;
+  }
+

LLVM style uses no braces for single statement `if`'s.


https://reviews.llvm.org/D37150



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

[PATCH] D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 113222.
arphaman added a comment.

Fixed a comment


Repository:
  rL LLVM

https://reviews.llvm.org/D37291

Files:
  include/clang/Tooling/Refactoring/RefactoringActionRule.h
  include/clang/Tooling/Refactoring/RefactoringActionRules.h
  include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
  include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
  unittests/Tooling/RefactoringActionRulesTest.cpp

Index: unittests/Tooling/RefactoringActionRulesTest.cpp
===
--- unittests/Tooling/RefactoringActionRulesTest.cpp
+++ unittests/Tooling/RefactoringActionRulesTest.cpp
@@ -35,8 +35,27 @@
 Expected>
 createReplacements(const std::unique_ptr &Rule,
RefactoringRuleContext &Context) {
-  return cast(*Rule).createSourceReplacements(
-  Context);
+  class Consumer final : public RefactoringResultConsumer {
+void handleInitiationFailure() {
+  Result = Expected>(None);
+}
+void handleInitiationError(llvm::Error Err) { Result = std::move(Err); }
+
+void handleInvocationError(llvm::Error Err) {
+  handleInitiationError(std::move(Err));
+}
+
+void handle(AtomicChanges SourceReplacements) {
+  Result = std::move(SourceReplacements);
+}
+
+  public:
+Optional>> Result;
+  };
+
+  Consumer C;
+  Rule->invoke(C, Context);
+  return std::move(*C.Result);
 }
 
 TEST_F(RefactoringActionRulesTest, MyFirstRefactoringRule) {
Index: include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
===
--- /dev/null
+++ include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
@@ -0,0 +1,68 @@
+//===--- RefactoringResultConsumer.h - Clang refactoring library --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H
+#define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H
+
+#include "clang/Basic/LLVM.h"
+#include "clang/Tooling/Refactoring/AtomicChange.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace tooling {
+
+/// An abstract interface that consumes the various refactoring results that can
+/// be produced by refactoring actions.
+///
+/// A valid refactoring result must be handled by a \c handle method.
+class RefactoringResultConsumer {
+public:
+  virtual ~RefactoringResultConsumer() {}
+
+  /// Handles an initiation failure. An action typically fails to initiate when
+  /// the source selection has no overlap at all with any relevant AST nodes.
+  virtual void handleInitiationFailure() = 0;
+
+  /// Handles an initation error. The error typically has a \c DiagnosticError
+  /// payload that describes why initation failed.
+  virtual void handleInitiationError(llvm::Error Err) = 0;
+
+  virtual void handleInvocationError(llvm::Error Err) = 0;
+
+  /// Handles the source replacements that are produced by a refactoring action.
+  virtual void handle(AtomicChanges SourceReplacements) = 0;
+};
+
+namespace traits {
+namespace internal {
+
+template  struct HasHandle {
+private:
+  template 
+  static auto check(ClassT *) -> typename std::is_same<
+  decltype(std::declval().handle(std::declval())), void>::type;
+
+  template  static std::false_type check(...);
+
+public:
+  using Type = decltype(check(nullptr));
+};
+
+} // end namespace internal
+
+/// A type trait that returns true iff the given type is a valid refactoring
+/// result.
+template 
+struct IsValidRefactoringResult : internal::HasHandle::Type {};
+
+} // end namespace traits
+} // end namespace tooling
+} // end namespace clang
+
+#endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H
Index: include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
===
--- include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
+++ include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
@@ -13,6 +13,7 @@
 #include "clang/Basic/LLVM.h"
 #include "clang/Tooling/Refactoring/RefactoringActionRule.h"
 #include "clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h"
+#include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
 #include "clang/Tooling/Refactoring/RefactoringRuleContext.h"
 #include "llvm/Support/Error.h"
 #include 
@@ -22,39 +23,20 @@
 namespace refactoring_action_rules {
 namespace internal {
 
-/// A wrapper around a specific refactoring action rule that calls a generic
-/// 'perform' method from the specific refactoring method.
-template  struct SpecificRefactoringRuleAdapter {};
-
-template <>
-class SpecificRefactoringRuleAdapter
-: public SourceChangeRefactoringRule {
-public:

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

[Raoul Wols via Phabricator via cfe-commits]

rwols added a comment.

So at this point the C++ changes are basically done, save for some minor things 
I guess. The problem is still that the VSCode extension doesn't do anything 
clever with the snippets. I have zero experience with TypeScript let alone 
extension development for VSCode, so it can take a while for me to investigate. 
I'd appreciate if someone else could take a look.


https://reviews.llvm.org/D37101



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

[PATCH] D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 113223.
arphaman added a comment.

Use `std::move`


Repository:
  rL LLVM

https://reviews.llvm.org/D37291

Files:
  include/clang/Tooling/Refactoring/RefactoringActionRule.h
  include/clang/Tooling/Refactoring/RefactoringActionRules.h
  include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
  include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
  unittests/Tooling/RefactoringActionRulesTest.cpp

Index: unittests/Tooling/RefactoringActionRulesTest.cpp
===
--- unittests/Tooling/RefactoringActionRulesTest.cpp
+++ unittests/Tooling/RefactoringActionRulesTest.cpp
@@ -35,8 +35,27 @@
 Expected>
 createReplacements(const std::unique_ptr &Rule,
RefactoringRuleContext &Context) {
-  return cast(*Rule).createSourceReplacements(
-  Context);
+  class Consumer final : public RefactoringResultConsumer {
+void handleInitiationFailure() {
+  Result = Expected>(None);
+}
+void handleInitiationError(llvm::Error Err) { Result = std::move(Err); }
+
+void handleInvocationError(llvm::Error Err) {
+  handleInitiationError(std::move(Err));
+}
+
+void handle(AtomicChanges SourceReplacements) {
+  Result = std::move(SourceReplacements);
+}
+
+  public:
+Optional>> Result;
+  };
+
+  Consumer C;
+  Rule->invoke(C, Context);
+  return std::move(*C.Result);
 }
 
 TEST_F(RefactoringActionRulesTest, MyFirstRefactoringRule) {
Index: include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
===
--- /dev/null
+++ include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
@@ -0,0 +1,68 @@
+//===--- RefactoringResultConsumer.h - Clang refactoring library --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H
+#define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H
+
+#include "clang/Basic/LLVM.h"
+#include "clang/Tooling/Refactoring/AtomicChange.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace tooling {
+
+/// An abstract interface that consumes the various refactoring results that can
+/// be produced by refactoring actions.
+///
+/// A valid refactoring result must be handled by a \c handle method.
+class RefactoringResultConsumer {
+public:
+  virtual ~RefactoringResultConsumer() {}
+
+  /// Handles an initiation failure. An action typically fails to initiate when
+  /// the source selection has no overlap at all with any relevant AST nodes.
+  virtual void handleInitiationFailure() = 0;
+
+  /// Handles an initation error. The error typically has a \c DiagnosticError
+  /// payload that describes why initation failed.
+  virtual void handleInitiationError(llvm::Error Err) = 0;
+
+  virtual void handleInvocationError(llvm::Error Err) = 0;
+
+  /// Handles the source replacements that are produced by a refactoring action.
+  virtual void handle(AtomicChanges SourceReplacements) = 0;
+};
+
+namespace traits {
+namespace internal {
+
+template  struct HasHandle {
+private:
+  template 
+  static auto check(ClassT *) -> typename std::is_same<
+  decltype(std::declval().handle(std::declval())), void>::type;
+
+  template  static std::false_type check(...);
+
+public:
+  using Type = decltype(check(nullptr));
+};
+
+} // end namespace internal
+
+/// A type trait that returns true iff the given type is a valid refactoring
+/// result.
+template 
+struct IsValidRefactoringResult : internal::HasHandle::Type {};
+
+} // end namespace traits
+} // end namespace tooling
+} // end namespace clang
+
+#endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H
Index: include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
===
--- include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
+++ include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
@@ -13,6 +13,7 @@
 #include "clang/Basic/LLVM.h"
 #include "clang/Tooling/Refactoring/RefactoringActionRule.h"
 #include "clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h"
+#include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
 #include "clang/Tooling/Refactoring/RefactoringRuleContext.h"
 #include "llvm/Support/Error.h"
 #include 
@@ -22,39 +23,20 @@
 namespace refactoring_action_rules {
 namespace internal {
 
-/// A wrapper around a specific refactoring action rule that calls a generic
-/// 'perform' method from the specific refactoring method.
-template  struct SpecificRefactoringRuleAdapter {};
-
-template <>
-class SpecificRefactoringRuleAdapter
-: public SourceChangeRefactoringRule {
-public:

[PATCH] D37210: [refactor] add a refactoring action rule that returns symbol occurrences

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 113224.
arphaman marked an inline comment as done.
arphaman added a comment.
Herald added a reviewer: JonasToth.
This revision now requires review to proceed.

Rebase on top of https://reviews.llvm.org/D37291


Repository:
  rL LLVM

https://reviews.llvm.org/D37210

Files:
  include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
  unittests/Tooling/RefactoringActionRulesTest.cpp

Index: unittests/Tooling/RefactoringActionRulesTest.cpp
===
--- unittests/Tooling/RefactoringActionRulesTest.cpp
+++ unittests/Tooling/RefactoringActionRulesTest.cpp
@@ -11,6 +11,7 @@
 #include "RewriterTestContext.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Refactoring/RefactoringActionRules.h"
+#include "clang/Tooling/Refactoring/Rename/SymbolName.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/Errc.h"
 #include "gtest/gtest.h"
@@ -32,10 +33,19 @@
   std::string DefaultCode = std::string(100, 'a');
 };
 
+class DefaultRefactoringResultConsumer : public RefactoringResultConsumer {
+public:
+  void handleInitiationFailure() override {}
+  void handleInitiationError(llvm::Error) override {}
+  void handleInvocationError(llvm::Error) override {}
+  void handle(AtomicChanges) override {}
+  void handle(SymbolOccurrences) override {}
+};
+
 Expected>
 createReplacements(const std::unique_ptr &Rule,
RefactoringRuleContext &Context) {
-  class Consumer final : public RefactoringResultConsumer {
+  class Consumer final : public DefaultRefactoringResultConsumer {
 void handleInitiationFailure() {
   Result = Expected>(None);
 }
@@ -181,4 +191,48 @@
   EXPECT_EQ(Message, "bad selection");
 }
 
+Optional findOccurrences(RefactoringActionRule &Rule,
+RefactoringRuleContext &Context) {
+  class Consumer final : public DefaultRefactoringResultConsumer {
+void handle(SymbolOccurrences Occurrences) override {
+  Result = std::move(Occurrences);
+}
+
+  public:
+Optional Result;
+  };
+
+  Consumer C;
+  Rule.invoke(C, Context);
+  return std::move(C.Result);
+}
+
+TEST_F(RefactoringActionRulesTest, ReturnSymbolOccurrences) {
+  auto Rule = createRefactoringRule(
+  [](selection::SourceSelectionRange Selection)
+  -> Expected {
+SymbolOccurrences Occurrences;
+Occurrences.push_back(SymbolOccurrence(
+SymbolName("test"), SymbolOccurrence::MatchingSymbol,
+Selection.getRange().getBegin()));
+return Occurrences;
+  },
+  requiredSelection(
+  selection::identity()));
+
+  RefactoringRuleContext RefContext(Context.Sources);
+  SourceLocation Cursor =
+  Context.Sources.getLocForStartOfFile(Context.Sources.getMainFileID());
+  RefContext.setSelectionRange({Cursor, Cursor});
+  Optional Result = findOccurrences(*Rule, RefContext);
+
+  ASSERT_FALSE(!Result);
+  SymbolOccurrences Occurrences = std::move(*Result);
+  EXPECT_EQ(Occurrences.size(), 1u);
+  EXPECT_EQ(Occurrences[0].getKind(), SymbolOccurrence::MatchingSymbol);
+  EXPECT_EQ(Occurrences[0].getNameRanges().size(), 1u);
+  EXPECT_EQ(Occurrences[0].getNameRanges()[0],
+SourceRange(Cursor, Cursor.getLocWithOffset(strlen("test";
+}
+
 } // end anonymous namespace
Index: include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
===
--- include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
+++ include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
@@ -12,6 +12,7 @@
 
 #include "clang/Basic/LLVM.h"
 #include "clang/Tooling/Refactoring/AtomicChange.h"
+#include "clang/Tooling/Refactoring/Rename/SymbolOccurrences.h"
 #include "llvm/Support/Error.h"
 
 namespace clang {
@@ -37,6 +38,10 @@
 
   /// Handles the source replacements that are produced by a refactoring action.
   virtual void handle(AtomicChanges SourceReplacements) = 0;
+
+  /// Handles the symbol occurrences that are found by an interactive
+  /// refactoring action.
+  virtual void handle(SymbolOccurrences Occurrences) = 0;
 };
 
 namespace traits {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Thank you for working on this!
I just tried, and the original false-positive i was hitting is now gone.

So as far i'm concerned, this is good to go.




Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:26
   }
   throw non_derived_exception(); // Bad
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 
'non_derived_exception' is not derived from 'std::exception'

JonasToth wrote:
> lebedev.ri wrote:
> > Could you please update the comments?
> > I find them misleading. What does this `bad` mean?
> > That this line is not handled correctly? Then please use `FIXME: wrong 
> > handling`
> > That this line is invalid? The `// CHECK-MESSAGES:` already states that...
> yes, they are left over when i started writing the test and did not know the 
> diagnostics. These comments help, when something is reported from clang-tidy, 
> to instantly see if this is correctly found, since the source line is shown. 
> I can remove them.
> These comments help, when something is reported from clang-tidy, to instantly 
> see if this is correctly found, since the source line is shown. I can remove 
> them.

Aha. I'd use some less confusing comments then, maybe



Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:54-62
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: throwing an exception whose type 
'bad_generic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-3]]:31: warning: throwing an exception whose type 
'bad_generic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-5]]:31: warning: throwing an exception whose type 
'exotic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 74:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-7]]:31: warning: throwing an exception whose type 
'int' is not derived from 'std::exception'

JonasToth wrote:
> lebedev.ri wrote:
> > JonasToth wrote:
> > > these diagnostic come from the many instantiations of this function. 
> > > do you think, they should exist or rather not?
> > They definitively should exist.
> > But they also should ideally point to the origin of the `T`.
> they kinda do. when templates are used, the template class is pointed to, but 
> not the instantiation. At least they shouldn't point to the `` 
> anymore.
> but not the instantiation 
It would be best if they would, but looking at the AST, i'm not sure how to do 
that...
https://godbolt.org/g/ejScyZ

Maybe someone else knows.


https://reviews.llvm.org/D37060



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

r312108 - Revert "Improve constant expression evaluation of arrays of unknown bound."

[Martin Bohme via cfe-commits]

Author: mboehme
Date: Wed Aug 30 03:44:46 2017
New Revision: 312108

URL: http://llvm.org/viewvc/llvm-project?rev=312108&view=rev
Log:
Revert "Improve constant expression evaluation of arrays of unknown bound."

This reverts commit r311970.

Breaks internal tests.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/include/clang/Basic/DiagnosticIDs.h
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=312108&r1=312107&r2=312108&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Wed Aug 30 03:44:46 2017
@@ -127,10 +127,6 @@ def note_constexpr_access_null : Note<
 def note_constexpr_access_past_end : Note<
   "%select{read of|assignment to|increment of|decrement of}0 "
   "dereferenced one-past-the-end pointer is not allowed in a constant 
expression">;
-def note_constexpr_access_unsized_array : Note<
-  "%select{read of|assignment to|increment of|decrement of}0 "
-  "pointer to element of array without known bound "
-  "is not allowed in a constant expression">;
 def note_constexpr_access_inactive_union_member : Note<
   "%select{read of|assignment to|increment of|decrement of}0 "
   "member %1 of union with %select{active member %3|no active member}2 "
@@ -158,11 +154,6 @@ def note_constexpr_baa_insufficient_alig
 def note_constexpr_baa_value_insufficient_alignment : Note<
   "value of the aligned pointer (%0) is not a multiple of the asserted %1 "
   "%plural{1:byte|:bytes}1">;
-def note_constexpr_unsupported_unsized_array : Note<
-  "array-to-pointer decay of array member without known bound is not 
supported">;
-def note_constexpr_unsized_array_indexed : Note<
-  "indexing of array without known bound is not allowed "
-  "in a constant expression">;
 
 def warn_integer_constant_overflow : Warning<
   "overflow in expression; result is %0 with type %1">,

Modified: cfe/trunk/include/clang/Basic/DiagnosticIDs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticIDs.h?rev=312108&r1=312107&r2=312108&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticIDs.h (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticIDs.h Wed Aug 30 03:44:46 2017
@@ -34,7 +34,7 @@ namespace clang {
   DIAG_SIZE_SERIALIZATION =  120,
   DIAG_SIZE_LEX   =  400,
   DIAG_SIZE_PARSE =  500,
-  DIAG_SIZE_AST   =  150,
+  DIAG_SIZE_AST   =  110,
   DIAG_SIZE_COMMENT   =  100,
   DIAG_SIZE_SEMA  = 3500,
   DIAG_SIZE_ANALYSIS  =  100

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=312108&r1=312107&r2=312108&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Aug 30 03:44:46 2017
@@ -141,12 +141,6 @@ namespace {
 return E && E->getType()->isPointerType() && tryUnwrapAllocSizeCall(E);
   }
 
-  /// The bound to claim that an array of unknown bound has.
-  /// The value in MostDerivedArraySize is undefined in this case. So, set it
-  /// to an arbitrary value that's likely to loudly break things if it's used.
-  static const uint64_t AssumedSizeForUnsizedArray =
-  std::numeric_limits::max() / 2;
-
   /// Determines if an LValue with the given LValueBase will have an unsized
   /// array in its designator.
   /// Find the path length and type of the most-derived subobject in the given
@@ -154,8 +148,7 @@ namespace {
   static unsigned
   findMostDerivedSubobject(ASTContext &Ctx, APValue::LValueBase Base,
ArrayRef Path,
-   uint64_t &ArraySize, QualType &Type, bool &IsArray,
-   bool &FirstEntryIsUnsizedArray) {
+   uint64_t &ArraySize, QualType &Type, bool &IsArray) 
{
 // This only accepts LValueBases from APValues, and APValues don't support
 // arrays that lack size info.
 assert(!isBaseAnAllocSizeCall(Base) &&
@@ -165,18 +158,12 @@ namespace {
 
 for (unsigned I = 0, N = Path.size(); I != N; ++I) {
   if (Type->isArrayType()) {
-const ArrayType *AT = Ctx.getAsArrayType(Type);
-Type = AT->getElementType();
+const ConstantArrayType *CAT =
+cast(Ctx.getAsArrayType(Type));
+Type = CAT->getElementType();
+ArraySize = CAT->getSize().getZExtValue();
 MostDerivedLength = I + 1;
 IsArray = true;
-
-if (auto *CAT = dyn_cast(AT)) {
-  ArraySize = CAT->getSize(

r312109 - Add test case that was broken by r311970.

[Martin Bohme via cfe-commits]

Author: mboehme
Date: Wed Aug 30 03:44:51 2017
New Revision: 312109

URL: http://llvm.org/viewvc/llvm-project?rev=312109&view=rev
Log:
Add test case that was broken by r311970.

See also discussion here:
https://reviews.llvm.org/rL301963

As far as I can tell, this discussion was never resolved.

Modified:
cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp

Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp?rev=312109&r1=312108&r2=312109&view=diff
==
--- cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp Wed Aug 30 03:44:51 
2017
@@ -604,6 +604,22 @@ static_assert(NATDCArray{}[1][1].n == 0,
 
 }
 
+// Tests for indexes into arrays of unknown bounds.
+namespace ArrayOfUnknownBound {
+  // This is a corner case of the language where it's not clear whether this
+  // should be an error: When we see the initializer for Z::a, the bounds of
+  // Z::b aren't known yet, but they will be known by the end of the 
translation
+  // unit, so the compiler can in theory check the indexing into Z::b.
+  // For the time being, as long as this is unclear, we want to make sure that
+  // this compiles.
+  struct Z {
+static const void *const a[];
+static const void *const b[];
+  };
+  constexpr const void *Z::a[] = {&b[0], &b[1]};
+  constexpr const void *Z::b[] = {&a[0], &a[1]};
+}
+
 namespace DependentValues {
 
 struct I { int n; typedef I V[10]; };


___
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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 113226.
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
@@ -73,21 +73,23 @@

   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('Neither CHECK-FIXES nor CHECK-MESSAGES '
+ 'nor CHECK-NOTES 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
   # avoiding empty lines which could potentially trigger formatting-related
   # checks.
   cleaned_test = re.sub('// *CHECK-[A-Z-]*:[^\r\n]*', '//', input_text)

   write_file(temp_file_name, cleaned_test)

   original_file_name = temp_file_name + ".orig"
   write_file(original_file_name, cleaned_test)

   args = ['clang-tidy', temp_file_name, '-fix', '--checks=-*,' + check_name] + 
\
 clang_tidy_extra_args
   print('Running ' + repr(args) + '...')
@@ -136,5 +138,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
@@ -73,21 +73,23 @@

   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('Neither CHECK-FIXES nor CHECK-MESSAGES '
+ 'nor CHECK-NOTES 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
   # avoiding empty lines which could potentially trigger formatting-related
   # checks.
   cleaned_test = re.sub('// *CHECK-[A-Z-]*:[^\r\n]*', '//', input_text)

   write_file(temp_file_name, cleaned_test)

   original_file_name = temp_file_name + ".orig"
   write_file(original_file_name, cleaned_test)

   args = ['clang-tidy', temp_file_name, '-fix', '--checks=-*,' + check_name] + \
 clang_tidy_extra_args
   print('Running ' + repr(args) + '...')
@@ -136,5 +138,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] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 113228.
lebedev.ri added a comment.

Rebased.


Repository:
  rL LLVM

https://reviews.llvm.org/D36836

Files:
  clang-tidy/readability/CMakeLists.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,549 @@
+// 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;
+end:
+  return;
+}
+
+////
+//-- 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 5 (threshold 0) [readability-function-cognitive-complexity]
+  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]]:7: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+  } else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:10: note: +1, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:14: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+  } else {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +1, nesting level increased to 1{{$}}
+  }
+}
+
+void unittest_b1_01() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_01' has cognitive complexity of 2 (threshold 0) [readability-function-cognitive-complexity]
+  int i = (1 ? 1 : 0) ? 1 : 0;
+// CHECK-NOTES: :[[@LINE-1]]:11: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:12: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// FIXME: would be nice to point at the '?' symbol. does not seem to be possible
+}
+
+void unittest_b1_02(int x) {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_02' has cognitive complexity of 5 (threshold 0) [readability-function-cognitive-complexity]
+  switch (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:11: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+  case -1:
+return;
+  case 1 ? 1 : 0:
+// CHECK-NOTES: :[[@LINE-1]]:8: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+return;
+  case (1 ? 2 : 0) ... (1 ? 3 : 0):
+// CHECK-NOTES: :[[@LINE-1]]:9: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:25: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+return;
+  default:
+break;
+  }
+}
+
+void unittest_b1_03(int x) {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_03' has cognitive complexity of 4 (threshold 0) [readability-function-cognitive-complexity]
+  for (x = 1 ? 1 : 0; x < (1 ? 1 : 0); x += 1 ? 1 : 0

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 113230.
lebedev.ri added a comment.

Rebased the correct branch this time...
When splitting off https://reviews.llvm.org/D36892, and addressing the other 
review note in this Revision, i slightly messed up the local branches, so the 
wrong code got rebased previously.
Sorry about that.


Repository:
  rL LLVM

https://reviews.llvm.org/D36836

Files:
  clang-tidy/readability/CMakeLists.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,549 @@
+// 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;
+end:
+  return;
+}
+
+////
+//-- 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 5 (threshold 0) [readability-function-cognitive-complexity]
+  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]]:7: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+  } else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:10: note: +1, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:14: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+  } else {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +1, nesting level increased to 1{{$}}
+  }
+}
+
+void unittest_b1_01() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_01' has cognitive complexity of 2 (threshold 0) [readability-function-cognitive-complexity]
+  int i = (1 ? 1 : 0) ? 1 : 0;
+// CHECK-NOTES: :[[@LINE-1]]:11: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:12: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// FIXME: would be nice to point at the '?' symbol. does not seem to be possible
+}
+
+void unittest_b1_02(int x) {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_02' has cognitive complexity of 5 (threshold 0) [readability-function-cognitive-complexity]
+  switch (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:11: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+  case -1:
+return;
+  case 1 ? 1 : 0:
+// CHECK-NOTES: :[[@LINE-1]]:8: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+return;
+  case (1 ? 2 : 0) ... (1 ? 3 : 0):
+// CHECK-NOTES: :[[@LINE-1]]:9: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:25: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+return;
+  default:
+break;
+  }
+}
+

[PATCH] D34365: [FrontEnd] Allow overriding the default C/C++ -std via CMake vars

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

mgorny added a comment.

Ping again.


https://reviews.llvm.org/D34365



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

r312112 - Revert r312105 [modules] Add ability to specify module name to module file mapping

[Victor Leschuk via cfe-commits]

Author: vleschuk
Date: Wed Aug 30 04:31:56 2017
New Revision: 312112

URL: http://llvm.org/viewvc/llvm-project?rev=312112&view=rev
Log:
Revert r312105 [modules] Add ability to specify module name to module file 
mapping

Looks like it breaks win10 builder.

Removed:
cfe/trunk/test/CXX/modules-ts/basic/basic.search/
Modified:
cfe/trunk/docs/Modules.rst
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Lex/HeaderSearch.h
cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/include/clang/Serialization/ModuleManager.h
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/FrontendActions.cpp
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
cfe/trunk/lib/Serialization/ModuleManager.cpp

Modified: cfe/trunk/docs/Modules.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/Modules.rst?rev=312112&r1=312111&r2=312112&view=diff
==
--- cfe/trunk/docs/Modules.rst (original)
+++ cfe/trunk/docs/Modules.rst Wed Aug 30 04:31:56 2017
@@ -213,14 +213,8 @@ Command-line parameters
 ``-fno-implicit-modules``
   All modules used by the build must be specified with ``-fmodule-file``.
 
-``-fmodule-file=[=]``
-  Specify the mapping of module names to precompiled module files. If the
-  name is omitted, then the module file is loaded whether actually required
-  or not. If the name is specified, then the mapping is treated as another
-  prebuilt module search mechanism (in addition to ``-fprebuilt-module-path``)
-  and the module is only loaded if required. Note that in this case the
-  specified file also overrides this module's paths that might be embedded
-  in other precompiled module files.
+``-fmodule-file=``
+  Load the given precompiled module file.
 
 ``-fprebuilt-module-path=``
   Specify the path to the prebuilt modules. If specified, we will look for 
modules in this directory for a given top-level module name. We don't need a 
module map for loading prebuilt modules in this directory and the compiler will 
not try to rebuild these modules. This can be specified multiple times.
@@ -951,3 +945,4 @@ PCHInternals_
 .. [#] The preprocessing context in which the modules are parsed is actually 
dependent on the command-line options provided to the compiler, including the 
language dialect and any ``-D`` options. However, the compiled modules for 
different command-line options are kept distinct, and any preprocessor 
directives that occur within the translation unit are ignored. See the section 
on the `Configuration macros declaration`_ for more information.
 
 .. _PCHInternals: PCHInternals.html
+ 

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=312112&r1=312111&r2=312112&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Wed Aug 30 04:31:56 2017
@@ -1125,8 +1125,8 @@ def fmodule_map_file : Joined<["-"], "fm
   Group, Flags<[DriverOption,CC1Option]>, MetaVarName<"">,
   HelpText<"Load this module map file">;
 def fmodule_file : Joined<["-"], "fmodule-file=">,
-  Group, Flags<[DriverOption,CC1Option]>, 
MetaVarName<"[=]">,
-  HelpText<"Specify the mapping of module name to precompiled module file, or 
load a module file if name is omitted.">;
+  Group, Flags<[DriverOption,CC1Option]>,
+  HelpText<"Load this precompiled module file">, MetaVarName<"">;
 def fmodules_ignore_macro : Joined<["-"], "fmodules-ignore-macro=">, 
Group, Flags<[CC1Option]>,
   HelpText<"Ignore the definition of the given macro when building and loading 
modules">;
 def fmodules_decluse : Flag <["-"], "fmodules-decluse">, Group,

Modified: cfe/trunk/include/clang/Lex/HeaderSearch.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearch.h?rev=312112&r1=312111&r2=312112&view=diff
==
--- cfe/trunk/include/clang/Lex/HeaderSearch.h (original)
+++ cfe/trunk/include/clang/Lex/HeaderSearch.h Wed Aug 30 04:31:56 2017
@@ -471,41 +471,29 @@ public:
   /// \brief Get filenames for all registered header maps.
   void getHeaderMapFileNames(SmallVectorImpl &Names) const;
 
-  /// \brief Retrieve the name of the cached module file that should be used
-  /// to load the given module.
+  /// \brief Retrieve the name of the module file that should be used to 
+  /// load the given module.
   ///
   /// \param Module The module whose module file name will be returned.
   ///
   /// \returns The name of the m

Re: [PATCH] D35020: [Modules] Add ability to specify module name to module file mapping

[Victor Leschuk via cfe-commits]

Hello Boris, looks like this revision broke tests on our win10 builder:
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/11760

Clang :: CXX/modules-ts/basic/basic.link/module-declaration.cpp

I had to revert this revision. Could you please take a look?


On 08/30/2017 11:47 AM, Phabricator via Phabricator via cfe-commits wrote:
> This revision was automatically updated to reflect the committed changes.
> Closed by commit rL312105: [modules] Add ability to specify module name to 
> module file mapping (authored by borisk).
>
> Changed prior to commit:
>   https://reviews.llvm.org/D35020?vs=111826&id=113207#toc
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D35020
>
> Files:
>   cfe/trunk/docs/Modules.rst
>   cfe/trunk/include/clang/Driver/Options.td
>   cfe/trunk/include/clang/Lex/HeaderSearch.h
>   cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
>   cfe/trunk/include/clang/Serialization/ASTReader.h
>   cfe/trunk/include/clang/Serialization/ModuleManager.h
>   cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>   cfe/trunk/lib/Frontend/CompilerInstance.cpp
>   cfe/trunk/lib/Frontend/CompilerInvocation.cpp
>   cfe/trunk/lib/Frontend/FrontendActions.cpp
>   cfe/trunk/lib/Lex/HeaderSearch.cpp
>   cfe/trunk/lib/Serialization/ASTReader.cpp
>   cfe/trunk/lib/Serialization/ASTWriter.cpp
>   cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
>   cfe/trunk/lib/Serialization/ModuleManager.cpp
>   cfe/trunk/test/CXX/modules-ts/basic/basic.search/module-import.cpp
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

-- 
Best Regards,

Victor Leschuk | Software Engineer |Access Softek

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

[PATCH] D33852: Enable __declspec(selectany) on linux

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:2477
   let Spellings = [Declspec<"selectany">, GCC<"selectany">];
   let Documentation = [Undocumented];
 }

aaron.ballman wrote:
> Since we're drastically modifying what platforms this is supported on, can 
> you please add documentation for this attribute?
This documentation isn't correct. Please see the other attributes (and 
AttrDocs.td) for an example of how to add the documentation.


https://reviews.llvm.org/D33852



___
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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Instead of CHECK-NOTES, do we want to extend CHECK-MESSAGES to handle `note` in 
addition to `warning` and `error`? I'd prefer to keep the number of "magic" 
names as low as possible so I have to remember less stuff when writing or 
reviewing tests.


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] D36586: [clang-tidy] hicpp bitwise operations on signed integers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/hicpp/SignedBitwiseCheck.cpp:23
+  const auto SignedIntegerOperand =
+  
expr(ignoringImpCasts(hasType(isSignedInteger(.bind("signed_operand");
+

JonasToth wrote:
> JonasToth wrote:
> > aaron.ballman wrote:
> > > JonasToth wrote:
> > > > aaron.ballman wrote:
> > > > > JonasToth wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Is ignoring implicit casts the correct behavior here as far as 
> > > > > > > the coding standard is concerned? Consider:
> > > > > > > ```
> > > > > > > unsigned char c = 128;
> > > > > > > f(~c); // c promotes to int before ~ is applied
> > > > > > > ```
> > > > > > > Looking at the coding standard, I get the impression this is not 
> > > > > > > allowed, but I'm not *super* familiar with HIC++.
> > > > > > I first implemented it without ignoring the implicit integer casts, 
> > > > > > the result was, that most cases (in test cases) where not found. 
> > > > > > therefore i implemented it that way. I add an testcase for this and 
> > > > > > see how i need to adjust the matcher.
> > > > > > 
> > > > > > Could you help me there with the semantic, since i am not so fluent 
> > > > > > in C/C++ standardese, but i think the findings are reasonable.
> > > > > It kind of boils down to the intention from the HIC++. Consider a 
> > > > > test case like:
> > > > > ```
> > > > > void f(int i);
> > > > > 
> > > > > void g() {
> > > > >   unsigned char c = 127;
> > > > >   f(~c);
> > > > > }
> > > > > 
> > > > > ```
> > > > > Does `f()` expect to receive `-128` or `128`? I think this code will 
> > > > > pass your check (ignoring the promotion means the type is `unsigned 
> > > > > char`), but the actual bitwise operation is on a signed integer value 
> > > > > because there is an integer promotion. So 127 is promoted to int, 
> > > > > then ~ is applied, resulting in the value `-128` being passed to the 
> > > > > function.
> > > > Yeah i see, i have such cases added in the tests.
> > > > TBH. i don't know if the standard wants this covered, but the 
> > > > demonstrated case is definitly bad.
> > > > 
> > > > Would it be a good idea, to warn on assigning/initializing `signed` 
> > > > integers with `unsigned` integers?
> > > > 
> > > > The CppCoreGuidelines have some sections on that as well: [[ 
> > > > https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#arithmetic
> > > >  | Section ES.100-103 ]]
> > > > 
> > > > Not sure if this check should care. On the other hand, would it  be 
> > > > nice to have a check that covers all "integer problems".
> > > > Yeah i see, i have such cases added in the tests.
> > > > TBH. i don't know if the standard wants this covered, but the 
> > > > demonstrated case is definitly bad.
> > > 
> > > I think you should ask the HIC++ people what they think; the rule text 
> > > does not make it clear what the behavior should be here.
> > > 
> > > > Would it be a good idea, to warn on assigning/initializing signed 
> > > > integers with unsigned integers?
> > > 
> > > We already have such a warning in clang (-Wsign-conversion).
> > > 
> > > >The CppCoreGuidelines have some sections on that as well: Section 
> > > >ES.100-103
> > > >
> > > >Not sure if this check should care. On the other hand, would it be nice 
> > > >to have a check that covers all "integer problems".
> > > 
> > > Any such check will require so many options that it is likely to be 
> > > almost unusable. However, I would not be opposed to seeing a clang-tidy 
> > > module that has a bunch of checks in it that relate to integer problems.
> > i think, those could land in `bugprone-`, and be aliases to hicpp and the 
> > cppcoreguidelines if appropriate.
> > 
> > depending on my time (i should have a lot of free time for 2 months) 
> > i will try to implement some.
> > is there a resource with all common problems found with integers?
> The response from Christof:
> 
> 
> 
> > Hi Jonas,
> > 
> > using bitwise operators with unsigned operands is compliant with this
> > rule, even if the operand is promoted to signed int.
> > 
> > expr.unary.op in the C++ standard says "The operand of ~ shall have
> > integral or unscoped enumeration type; [...] Integral promotions are
> > performed. The type of the result is the type of the promoted operand."
> > 
> > The HICPP rule refers to the type of the operand (and not the type of
> > the promoted operand), it therefore refers to the type before promotion
> > (which is "unsigned char" in the example).
> > 
> > 
> > Regards,
> > Christof
> 
> Your requested case is therefore conforming and i think the current 
> implementation handles these things correct.
> Your requested case is therefore conforming and i think the current 
> implementation handles these things correct.

Agreed; thank you for following up on this!



Comment at: test/clang-tidy/hicpp-signed-bitwise.cpp:205
+
+#if 0
+// Scoped Enums must define thei

[PATCH] D37210: [refactor] add a refactoring action rule that returns symbol occurrences

[Jonas Toth via Phabricator via cfe-commits]

JonasToth resigned from this revision.
JonasToth added a comment.
This revision is now accepted and ready to land.

sry. misconfigured herald :(


Repository:
  rL LLVM

https://reviews.llvm.org/D37210



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

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:54-62
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: throwing an exception whose type 
'bad_generic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-3]]:31: warning: throwing an exception whose type 
'bad_generic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-5]]:31: warning: throwing an exception whose type 
'exotic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 74:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-7]]:31: warning: throwing an exception whose type 
'int' is not derived from 'std::exception'

lebedev.ri wrote:
> JonasToth wrote:
> > lebedev.ri wrote:
> > > JonasToth wrote:
> > > > these diagnostic come from the many instantiations of this function. 
> > > > do you think, they should exist or rather not?
> > > They definitively should exist.
> > > But they also should ideally point to the origin of the `T`.
> > they kinda do. when templates are used, the template class is pointed to, 
> > but not the instantiation. At least they shouldn't point to the ` > T>` anymore.
> > but not the instantiation 
> It would be best if they would, but looking at the AST, i'm not sure how to 
> do that...
> https://godbolt.org/g/ejScyZ
> 
> Maybe someone else knows.
it is probably too much magic anyway. I thinnk you would need to trace the 
callgraph for that, which is probably not worth it (any i am not able to 
program it either :D )


https://reviews.llvm.org/D37060



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

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: clangd/ClangdUnit.cpp:302
   unsigned NumResults) override {
-for (unsigned I = 0; I != NumResults; ++I) {
-  CodeCompletionResult &Result = Results[I];
-  CodeCompletionString *CCS = Result.CreateCodeCompletionString(
+assert(Items && "We need a non-null items container here");
+Items->reserve(NumResults);

Move this assert in the `CompletionItemsCollector` constructor.



Comment at: clangd/ClangdUnit.cpp:322
+const CodeCompletionString &CCS) const {
+// The object that we'll return.
+CompletionItem Item;

This comment is unnecessary.



Comment at: clangd/ClangdUnit.cpp:339
+  }
+  for (unsigned j = 0; j < CCS.getAnnotationCount(); ++j) {
+Item.documentation += CCS.getAnnotation(j);

In LLVM, prefer using an explicit variable for the limit of iteration. In this 
case, since you're using `getAnnotationCount` before, you could directly put it 
into a variable above.


https://reviews.llvm.org/D37101



___
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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Note can be handled right now as well. E.g.

  // CHECK-MESSAGES: [[@LINE-2]]:3: note: type deduction did not result in an 
owner

would the patch handle the codelocation correctly?


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] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

@aaron.ballman is it ok for you as well? otherwise i would commit it.


https://reviews.llvm.org/D37060



___
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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D36892#856469, @aaron.ballman wrote:

> Instead of CHECK-NOTES, do we want to extend CHECK-MESSAGES to handle `note` 
> in addition to `warning` and `error`?


If i change `CHECK-MESSAGES` to also require all the notes to be checked, a 
*lot* of tests fail:

  
  Testing Time: 4.04s
  
  Failing Tests (165):
  Clang Tools :: clang-tidy/android-cloexec-accept.cpp
  Clang Tools :: clang-tidy/android-cloexec-accept4.cpp
  Clang Tools :: clang-tidy/android-cloexec-creat.cpp
  Clang Tools :: clang-tidy/android-cloexec-dup.cpp
  Clang Tools :: clang-tidy/android-cloexec-epoll-create.cpp
  Clang Tools :: clang-tidy/android-cloexec-epoll-create1.cpp
  Clang Tools :: clang-tidy/android-cloexec-fopen.cpp
  Clang Tools :: clang-tidy/android-cloexec-inotify-init.cpp
  Clang Tools :: clang-tidy/android-cloexec-inotify-init1.cpp
  Clang Tools :: clang-tidy/android-cloexec-memfd-create.cpp
  Clang Tools :: clang-tidy/android-cloexec-open.cpp
  Clang Tools :: clang-tidy/android-cloexec-socket.cpp
  Clang Tools :: clang-tidy/boost-use-to-string.cpp
  Clang Tools :: clang-tidy/bugprone-integer-division.cpp
  Clang Tools :: clang-tidy/bugprone-suspicious-memset-usage.cpp
  Clang Tools :: clang-tidy/bugprone-undefined-memory-manipulation.cpp
  Clang Tools :: clang-tidy/cert-dcl21-cpp.cpp
  Clang Tools :: clang-tidy/cert-oop11-cpp.cpp
  Clang Tools :: clang-tidy/clean-up-code.cpp
  Clang Tools :: 
clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
  Clang Tools :: clang-tidy/cppcoreguidelines-pro-type-cstyle-cast.cpp
  Clang Tools :: clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp
  Clang Tools :: 
clang-tidy/cppcoreguidelines-pro-type-member-init-delayed.cpp
  Clang Tools :: clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
  Clang Tools :: 
clang-tidy/cppcoreguidelines-pro-type-static-cast-downcast.cpp
  Clang Tools :: clang-tidy/cppcoreguidelines-pro-type-vararg.cpp
  Clang Tools :: clang-tidy/deduplication.cpp
  Clang Tools :: clang-tidy/google-build-explicit-make-pair.cpp
  Clang Tools :: clang-tidy/google-explicit-constructor.cpp
  Clang Tools :: clang-tidy/google-readability-casting.c
  Clang Tools :: clang-tidy/google-readability-casting.cpp
  Clang Tools :: clang-tidy/google-readability-namespace-comments.cpp
  Clang Tools :: clang-tidy/google-readability-todo.cpp
  Clang Tools :: clang-tidy/llvm-include-order.cpp
  Clang Tools :: clang-tidy/llvm-twine-local.cpp
  Clang Tools :: clang-tidy/misc-argument-comment-gmock.cpp
  Clang Tools :: clang-tidy/misc-argument-comment-strict.cpp
  Clang Tools :: clang-tidy/misc-argument-comment.cpp
  Clang Tools :: clang-tidy/misc-assert-side-effect.cpp
  Clang Tools :: clang-tidy/misc-bool-pointer-implicit-conversion.cpp
  Clang Tools :: clang-tidy/misc-definitions-in-headers.hpp
  Clang Tools :: clang-tidy/misc-forwarding-reference-overload.cpp
  Clang Tools :: clang-tidy/misc-inaccurate-erase.cpp
  Clang Tools :: clang-tidy/misc-inefficient-algorithm.cpp
  Clang Tools :: clang-tidy/misc-lambda-function-name.cpp
  Clang Tools :: clang-tidy/misc-macro-parentheses.cpp
  Clang Tools :: clang-tidy/misc-macro-repeated-side-effects.c
  Clang Tools :: clang-tidy/misc-misplaced-const.c
  Clang Tools :: clang-tidy/misc-misplaced-const.cpp
  Clang Tools :: clang-tidy/misc-move-const-arg.cpp
  Clang Tools :: clang-tidy/misc-move-constructor-init.cpp
  Clang Tools :: clang-tidy/misc-move-forwarding-reference.cpp
  Clang Tools :: clang-tidy/misc-multiple-statement-macro.cpp
  Clang Tools :: clang-tidy/misc-static-assert.c
  Clang Tools :: clang-tidy/misc-static-assert.cpp
  Clang Tools :: clang-tidy/misc-string-compare.cpp
  Clang Tools :: clang-tidy/misc-string-constructor.cpp
  Clang Tools :: clang-tidy/misc-string-integer-assignment.cpp
  Clang Tools :: clang-tidy/misc-suspicious-semicolon.cpp
  Clang Tools :: clang-tidy/misc-suspicious-string-compare.c
  Clang Tools :: clang-tidy/misc-suspicious-string-compare.cpp
  Clang Tools :: clang-tidy/misc-swapped-arguments.cpp
  Clang Tools :: clang-tidy/misc-uniqueptr-reset-release.cpp
  Clang Tools :: clang-tidy/misc-unused-alias-decls.cpp
  Clang Tools :: clang-tidy/misc-unused-parameters.c
  Clang Tools :: clang-tidy/misc-unused-parameters.cpp
  Clang Tools :: clang-tidy/misc-unused-raii.cpp
  Clang Tools :: clang-tidy/misc-unused-using-decls.cpp
  Clang Tools :: clang-tidy/misc-virtual-near-miss.cpp
  Clang Tools :: clang-tidy/modernize-avoid-bind.cpp
  Clang Tools :: clang-tidy/modernize-deprecated-headers-cxx03.cpp
  Clang Tools :: clang-tidy/modernize-deprecated-headers-cxx11.cpp
  Clang Tools :: clang-tidy/modernize-lo

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D36892#856501, @JonasToth wrote:

> Note can be handled right now as well.


Yes. Adding this new prefix is about adding `-implicit-check-not=notes`.
I.e. if you use `CHECK-MESSAGES`, then it will only enforce you to have 
check-lines for all the `error: ` and `warning: ` the check outputs.
It does *not* enforce that all the `note: ` are to be handled. `CHECK-NOTES` 
however would enforce that.

> E.g.
> 
>   // CHECK-MESSAGES: [[@LINE-2]]:3: note: type deduction did not result in an 
> owner
> 
> would the patch handle the codelocation correctly?

I'm not sure what is the question to be honest. This does not change anything 
about codelocation handling


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] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

alright. i thought it would do something different, but the enforcement to 
handle all notes is a good thing. forget what i wrote :)


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] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp:41
+  "'std::exception'")
+  << BadThrow->getSubExpr()->getType() << BadThrow->getSourceRange();
 

Can you add a test that uses a rethrow? e.g.,
```
try {
  throw 12; // Diagnose this
} catch (...) {
  throw; // Does not diagnose this
}
```



Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:9
+class deep_hierarchy : public derived_exception {};
 class non_derived_exception {};
 

Can you add a test that uses multiple inheritance? e.g.,
```
class terrible_idea : public non_derived_exception, public derived_exception {};
```
Also, is private inheritance also acceptable, or does it need to be public 
inheritance? I kind of get the impression it needs to be public, because the 
goal appears to be that you should always be able to catch a `std::exception` 
instance, and you can't do that if it's privately inherited. That should have a 
test as well.


https://reviews.llvm.org/D37060



___
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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

> I do agree that it makes sense to keep it as low as possible, but also i see 
> a clear logic between all thee current checks:

Thank you for the explanation. I think that makes sense to me, but I'd like to 
hear from @alexfh before accepting.




Comment at: test/clang-tidy/check_clang_tidy.py:79-80
   except subprocess.CalledProcessError as e:
 print('clang-tidy failed:\n' + e.output.decode())
 raise
   print(' clang-tidy output ---\n' 
+

I think the text should be rephrased to "CHECK-FIXES, CHECK-MESSAGES, or 
CHECK-NOTES not found in the input"


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] D36586: [clang-tidy] hicpp bitwise operations on signed integers

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 113246.
JonasToth marked 8 inline comments as done.
JonasToth added a comment.

- added additional testcases, like @aaron.ballman requested
- fixed diagnostics


https://reviews.llvm.org/D36586

Files:
  clang-tidy/hicpp/CMakeLists.txt
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/hicpp/SignedBitwiseCheck.cpp
  clang-tidy/hicpp/SignedBitwiseCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/hicpp-signed-bitwise.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/hicpp-signed-bitwise.cpp

Index: test/clang-tidy/hicpp-signed-bitwise.cpp
===
--- /dev/null
+++ test/clang-tidy/hicpp-signed-bitwise.cpp
@@ -0,0 +1,219 @@
+// RUN: %check_clang_tidy %s hicpp-signed-bitwise %t
+
+// These could cause false positives and should not be considered.
+struct StreamClass {
+};
+StreamClass &operator<<(StreamClass &os, unsigned int i) {
+  return os;
+}
+StreamClass &operator<<(StreamClass &os, int i) {
+  return os;
+}
+StreamClass &operator>>(StreamClass &os, unsigned int i) {
+  return os;
+}
+StreamClass &operator>>(StreamClass &os, int i) {
+  return os;
+}
+struct AnotherStream {
+  AnotherStream &operator<<(unsigned char c) { return *this; }
+  AnotherStream &operator<<(char c) { return *this; }
+
+  AnotherStream &operator>>(unsigned char c) { return *this; }
+  AnotherStream &operator>>(char c) { return *this; }
+};
+
+void binary_bitwise() {
+  int SValue = 42;
+  int SResult;
+
+  unsigned int UValue = 42;
+  unsigned int UResult;
+
+  SResult = SValue & 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  SResult = SValue & -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  SResult = SValue & SValue;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = SValue & 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = SValue & -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = UValue & 1u; // Ok
+  UResult = UValue & UValue; // Ok
+
+  unsigned char UByte1 = 0u;
+  unsigned char UByte2 = 16u;
+  char SByte1 = 0;
+  char SByte2 = 16;
+
+  UByte1 = UByte1 & UByte2; // Ok
+  UByte1 = SByte1 & UByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  UByte1 = SByte1 & SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  SByte1 = SByte1 & SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  // More complex expressions.
+  UResult = UValue & (SByte1 + (SByte1 | SByte2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-2]]:33: warning: use of a signed integer operand with a binary bitwise operator
+
+  // The rest is to demonstrate functionality but all operators are matched equally.
+  // Therefore functionality is the same for all binary operations.
+  UByte1 = UByte1 | UByte2; // Ok
+  UByte1 = UByte1 | SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  UByte1 = UByte1 ^ UByte2; // Ok
+  UByte1 = UByte1 ^ SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  UByte1 = UByte1 >> UByte2; // Ok
+  UByte1 = UByte1 >> SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  UByte1 = UByte1 << UByte2; // Ok
+  UByte1 = UByte1 << SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  int SignedInt1 = 1 << 12;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand with a binary bitwise operator
+  int SignedInt2 = 1u << 12;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand with a binary bitwise operator
+}
+
+void f1(unsigned char c) {}
+void f2(char c) {}
+void f3(int c) {}
+
+void unary_bitwise() {
+  unsigned char UByte1 = 0u;
+  char SByte1 = 0;
+
+  UByte1 = ~UByte1; // Ok
+  SByte1 = ~UByte1;
+  SByte1 = ~SByte1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a unary bitwise operator
+  UByte1 = ~SByte1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a unary bitwise operator
+
+  unsigned int UInt = 0u;
+  int SInt = 0;
+
+  f1(~UByte1); // Ok
+  f1(~SByte1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use of a signed integer operand with a unary bitwise

[PATCH] D36586: [clang-tidy] hicpp bitwise operations on signed integers

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked an inline comment as done.
JonasToth added inline comments.



Comment at: test/clang-tidy/hicpp-signed-bitwise.cpp:205
+
+#if 0
+// Scoped Enums must define their operations, so the overloaded operators must 
take care

aaron.ballman wrote:
> I don't think it helps to have this code remain in the test case.
> 
> Can you add tests for the following?
> ```
> int i1 = 1 << 12;
> int i2 = 1u << 12;
> 
> enum E {
>   one = 1,
>   two = 2,
>   test1 = 1 << 12,
>   test2 = one << two,
>   test3 = 1u << 12
> };
> ```
the first two cases `int i1 ...` are in the first function of this file( 
`binary_bitwise`).


https://reviews.llvm.org/D36586



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

[PATCH] D37140: [clang-format] Fixed one-line if statement

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: lib/Format/UnwrappedLineFormatter.cpp:286
 }
+if (TheLine->Last->is(tok::l_brace) &&
+TheLine->First != TheLine->Last &&

No tests fail if this `if` statement gets removed. Please either remove it or 
add a test for it.



Comment at: lib/Format/UnwrappedLineFormatter.cpp:291
+}
+if (I[1]->First->is(tok::l_brace) && 
+TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) { 

Please add a short comment about which case is this addressing.



Comment at: lib/Format/UnwrappedLineFormatter.cpp:295
+}
+if (TheLine->First->is(tok::l_brace) &&
+   (TheLine->First == TheLine->Last) &&

Which case does this `if` cover?



Comment at: lib/Format/UnwrappedLineFormatter.cpp:296
+if (TheLine->First->is(tok::l_brace) &&
+   (TheLine->First == TheLine->Last) &&
+   (I != AnnotatedLines.begin()) &&

Please remove unnecessary parenthesis.



Comment at: lib/Format/UnwrappedLineFormatter.cpp:300
+  return Style.AllowShortBlocksOnASingleLine ? tryMergeSimpleBlock(I-1, E, 
Limit) : 0;
+}
 if (TheLine->Last->is(tok::l_brace)) {

Please reformat the newly added code with `clang-format`.



Comment at: lib/Format/UnwrappedLineFormatter.cpp:300
+  return Style.AllowShortBlocksOnASingleLine ? tryMergeSimpleBlock(I-1, E, 
Limit) : 0;
+}
 if (TheLine->Last->is(tok::l_brace)) {

krasimir wrote:
> Please reformat the newly added code with `clang-format`.
Why in this case we use `Style.AllowShortBlocksOnASingleLine` and in the case 
above we use `AfterControlStatement`?
Also, why `tryMergeSimpleBlock` instead of `tryMergeControlStatement`?



Comment at: lib/Format/UnwrappedLineFormatter.cpp:538
+  (I[1]->First == I[1]->Last && I + 2 != E &&
+  I[2]->First->is(tok::r_brace {
+  MergedLines = tryMergeSimpleBlock(I + 1, E, Limit);

Please remove unnecessary parenthesis.



Comment at: lib/Format/UnwrappedLineFormatter.cpp:539
+  I[2]->First->is(tok::r_brace {
+  MergedLines = tryMergeSimpleBlock(I + 1, E, Limit);
+  // If we managed to merge the block, count the statement header, which is

Please reformat this code with clang-format. This block must be indented.



Comment at: unittests/Format/FormatTest.cpp:519
+  verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
+  verifyFormat("if (true)\n" 
+   "{ //\n"

Could you also please add a test case where the wrapping is forced by exceeding 
the column limit instead of by the empty comment, like:
```
if (true)
{
  ff();
}
```



Comment at: unittests/Format/FormatTest.cpp:537
+   "}",
+   AllowSimpleBracedStatements);
+

Why is this example not on a single line? Is it because it has an `else`?



Comment at: unittests/Format/FormatTest.cpp:539
+
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
+  verifyFormat("if (true)\n"

Could you please also add a test with empty braces, like `if (true) {}` after 
this?


https://reviews.llvm.org/D37140



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

[PATCH] D36586: [clang-tidy] hicpp bitwise operations on signed integers

[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.

Thanks! LGTM!


https://reviews.llvm.org/D36586



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

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked 3 inline comments as done.
JonasToth added inline comments.



Comment at: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp:41
+  "'std::exception'")
+  << BadThrow->getSubExpr()->getType() << BadThrow->getSourceRange();
 

aaron.ballman wrote:
> Can you add a test that uses a rethrow? e.g.,
> ```
> try {
>   throw 12; // Diagnose this
> } catch (...) {
>   throw; // Does not diagnose this
> }
> ```
I added this case, but i have questions on this one.

The type of the caught exception is not know in general right? 
In this case, a deeper analysis would find that the second `throw` is 
problematic, too.
But since the rethrow depends on the original thrown type, conforming code 
could never rethrow a bad exception.



Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:9
+class deep_hierarchy : public derived_exception {};
 class non_derived_exception {};
 

aaron.ballman wrote:
> Can you add a test that uses multiple inheritance? e.g.,
> ```
> class terrible_idea : public non_derived_exception, public derived_exception 
> {};
> ```
> Also, is private inheritance also acceptable, or does it need to be public 
> inheritance? I kind of get the impression it needs to be public, because the 
> goal appears to be that you should always be able to catch a `std::exception` 
> instance, and you can't do that if it's privately inherited. That should have 
> a test as well.
The rules do not state directly, that it must be inherited public, but i dont 
see a good reason to allow non-public inheritance.
Another thing is, that you can always call `e.what()` on public derived 
exceptions.

Multiple inheritance is harder, since the type is still a `std::exception`. One 
could catch it and use its interface, so these reasons are gone to disallow it.


https://reviews.llvm.org/D37060



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

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 113252.
JonasToth added a comment.

- added inheritance cases, adjusted matcher is required


https://reviews.llvm.org/D37060

Files:
  clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
  test/clang-tidy/hicpp-exception-baseclass.cpp

Index: test/clang-tidy/hicpp-exception-baseclass.cpp
===
--- test/clang-tidy/hicpp-exception-baseclass.cpp
+++ test/clang-tidy/hicpp-exception-baseclass.cpp
@@ -5,38 +5,152 @@
 } // namespace std
 
 class derived_exception : public std::exception {};
+class deep_hierarchy : public derived_exception {};
 class non_derived_exception {};
+class bad_inheritance : private std::exception {};
+class no_good_inheritance : protected std::exception {};
+class terrible_idead : public non_derived_exception, public derived_exception {};
+class really_creative : public non_derived_exception, private std::exception {};
 
 void problematic() {
   try {
-throw int(42); // Built in is not allowed
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: throwing an exception whose type is not derived from 'std::exception'
+throw int(42);
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
   } catch (int e) {
   }
-  throw int(42); // Bad
-// CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type is not derived from 'std::exception'
+  throw int(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
 
   try {
-throw non_derived_exception(); // Some class is not allowed
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: throwing an exception whose type is not derived from 'std::exception'
-// CHECK-MESSAGES: 8:1: note: type defined here
+throw 12;
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+  } catch (...) {
+throw; // FIXME: This throw is not catched, but known to be of type 'int'
+  }
+
+  try {
+throw non_derived_exception();
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 9:1: note: type defined here
   } catch (non_derived_exception &e) {
   }
-  throw non_derived_exception(); // Bad
-// CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type is not derived from 'std::exception'
-// CHECK-MESSAGES: 8:1: note: type defined here
+  throw non_derived_exception();
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+  // CHECK-MESSAGES: 9:1: note: type defined here
+
+  // Handle private inheritance cases correctly.
+  try {
+throw bad_inheritance();
+throw no_good_inheritance();
+throw really_creative();
+  } catch (...) {
+  }
+  throw bad_inheritance();
+  throw no_good_inheritance();
+  throw really_creative();
 }
 
 void allowed_throws() {
   try {
-throw std::exception(); // Ok
+throw std::exception(); // Ok
   } catch (std::exception &e) { // Ok
   }
   throw std::exception();
 
   try {
-throw derived_exception(); // Ok
+throw derived_exception(); // Ok
   } catch (derived_exception &e) { // Ok
   }
   throw derived_exception(); // Ok
+
+  try {
+throw deep_hierarchy(); // Ok, multiple levels of inheritance
+  } catch (deep_hierarchy &e) { // Ok
+  }
+  throw deep_hierarchy(); // Ok
+
+  try {
+throw terrible_idead(); // Ok, but multiple inheritance isn't clean
+  } catch (std::exception &e) { // Can be caught as std::exception, even with multiple inheritance
+  }
+  throw terrible_idead(); // Ok, but multiple inheritance
+}
+
+// Templated function that throws exception based on template type
+template 
+void ThrowException() { throw T(); }
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-3]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-5]]:31: warning: throwing an exception whose type 'exotic_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 74:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-7]]:31: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+// CHECK-MESSAGES: [[@LINE-8]]:31: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 9:1: note: type defined here
+#define THROW_EXCEPTION(CLASS) ThrowException()
+#define THROW_BAD_EXCEPTION throw int(42);
+#define THROW_GOOD_EXCEPTION throw std::exception();
+#define THROW_DERIVED_EXCEPTION t

r312121 - [refactor] Examine the whole range for ObjC @implementation decls

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Aug 30 06:24:37 2017
New Revision: 312121

URL: http://llvm.org/viewvc/llvm-project?rev=312121&view=rev
Log:
[refactor] Examine the whole range for ObjC @implementation decls
when computing the AST selection

Modified:
cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp
cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp

Modified: cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp?rev=312121&r1=312120&r2=312121&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp Wed Aug 30 06:24:37 2017
@@ -17,6 +17,21 @@ using ast_type_traits::DynTypedNode;
 
 namespace {
 
+CharSourceRange getLexicalDeclRange(Decl *D, const SourceManager &SM,
+const LangOptions &LangOpts) {
+  if (!isa(D))
+return CharSourceRange::getTokenRange(D->getSourceRange());
+  // Objective-C implementation declarations end at the '@' instead of the 
'end'
+  // keyword. Use the lexer to find the location right after 'end'.
+  SourceRange R = D->getSourceRange();
+  SourceLocation LocAfterEnd = Lexer::findLocationAfterToken(
+  R.getEnd(), tok::raw_identifier, SM, LangOpts,
+  /*SkipTrailingWhitespaceAndNewLine=*/false);
+  return LocAfterEnd.isValid()
+ ? CharSourceRange::getCharRange(R.getBegin(), LocAfterEnd)
+ : CharSourceRange::getTokenRange(R);
+}
+
 /// Constructs the tree of selected AST nodes that either contain the location
 /// of the cursor or overlap with the selection range.
 class ASTSelectionFinder
@@ -62,9 +77,8 @@ public:
 if (SM.getFileID(FileLoc) != TargetFile)
   return true;
 
-// FIXME (Alex Lorenz): Add location adjustment for ObjCImplDecls.
 SourceSelectionKind SelectionKind =
-selectionKindFor(CharSourceRange::getTokenRange(D->getSourceRange()));
+selectionKindFor(getLexicalDeclRange(D, SM, Context.getLangOpts()));
 SelectionStack.push_back(
 SelectedASTNode(DynTypedNode::create(*D), SelectionKind));
 LexicallyOrderedRecursiveASTVisitor::TraverseDecl(D);

Modified: cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp?rev=312121&r1=312120&r2=312121&view=diff
==
--- cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp Wed Aug 30 06:24:37 2017
@@ -494,4 +494,23 @@ void foo() {
   });
 }
 
+TEST(ASTSelectionFinder, CorrectEndForObjectiveCImplementation) {
+  StringRef Source = R"(
+@interface I
+@end
+@implementation I
+@ end
+)";
+  // Just after '@ end'
+  findSelectedASTNodes(Source, {5, 6}, None,
+   [](Optional Node) {
+ EXPECT_TRUE(Node);
+ EXPECT_EQ(Node->Children.size(), 1u);
+ checkNode(
+ Node->Children[0],
+ SourceSelectionKind::ContainsSelection);
+   },
+   SelectionFinderVisitor::Lang_OBJC);
+}
+
 } // end anonymous namespace


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

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp:41
+  "'std::exception'")
+  << BadThrow->getSubExpr()->getType() << BadThrow->getSourceRange();
 

JonasToth wrote:
> aaron.ballman wrote:
> > Can you add a test that uses a rethrow? e.g.,
> > ```
> > try {
> >   throw 12; // Diagnose this
> > } catch (...) {
> >   throw; // Does not diagnose this
> > }
> > ```
> I added this case, but i have questions on this one.
> 
> The type of the caught exception is not know in general right? 
> In this case, a deeper analysis would find that the second `throw` is 
> problematic, too.
> But since the rethrow depends on the original thrown type, conforming code 
> could never rethrow a bad exception.
I think it's fine to not diagnose the rethrow -- as you mentioned, the only way 
for it to be a problem is when the original throw is a problem and that will 
get diagnosed elsewhere.



Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:9
+class deep_hierarchy : public derived_exception {};
 class non_derived_exception {};
 

JonasToth wrote:
> aaron.ballman wrote:
> > Can you add a test that uses multiple inheritance? e.g.,
> > ```
> > class terrible_idea : public non_derived_exception, public 
> > derived_exception {};
> > ```
> > Also, is private inheritance also acceptable, or does it need to be public 
> > inheritance? I kind of get the impression it needs to be public, because 
> > the goal appears to be that you should always be able to catch a 
> > `std::exception` instance, and you can't do that if it's privately 
> > inherited. That should have a test as well.
> The rules do not state directly, that it must be inherited public, but i dont 
> see a good reason to allow non-public inheritance.
> Another thing is, that you can always call `e.what()` on public derived 
> exceptions.
> 
> Multiple inheritance is harder, since the type is still a `std::exception`. 
> One could catch it and use its interface, so these reasons are gone to 
> disallow it.
> The rules do not state directly, that it must be inherited public, but i dont 
> see a good reason to allow non-public inheritance.

Agreed.

> Another thing is, that you can always call e.what() on public derived 
> exceptions.

Agreed.

> Multiple inheritance is harder, since the type is still a std::exception. One 
> could catch it and use its interface, so these reasons are gone to disallow 
> it.

I think the multiple inheritance case should not diagnose because it meets the 
HIC++ requirement of being derived from `std::exception`.


https://reviews.llvm.org/D37060



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

[clang-tools-extra] r312122 - [clang-tidy] hicpp bitwise operations on signed integers

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Wed Aug 30 06:32:05 2017
New Revision: 312122

URL: http://llvm.org/viewvc/llvm-project?rev=312122&view=rev
Log:
[clang-tidy] hicpp bitwise operations on signed integers

Summary:
This check implements the rule [[ 
http://www.codingstandard.com/section/5-6-shift-operators/ | 5.6. HIC++ ]]
that forbidds bitwise operations on signed integer types.

Reviewers: aaron.ballman, hokein, alexfh, Eugene.Zelenko

Reviewed By: aaron.ballman

Subscribers: cfe-commits, mgorny, JDevlieghere, xazax.hun

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

Added:
clang-tools-extra/trunk/clang-tidy/hicpp/SignedBitwiseCheck.cpp
clang-tools-extra/trunk/clang-tidy/hicpp/SignedBitwiseCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
clang-tools-extra/trunk/test/clang-tidy/hicpp-signed-bitwise.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt?rev=312122&r1=312121&r2=312122&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt Wed Aug 30 06:32:05 
2017
@@ -4,6 +4,7 @@ add_clang_library(clangTidyHICPPModule
   ExceptionBaseclassCheck.cpp
   NoAssemblerCheck.cpp
   HICPPTidyModule.cpp
+  SignedBitwiseCheck.cpp
 
   LINK_LIBS
   clangAST

Modified: clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp?rev=312122&r1=312121&r2=312122&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp Wed Aug 30 
06:32:05 2017
@@ -26,6 +26,7 @@
 #include "../readability/IdentifierNamingCheck.h"
 #include "ExceptionBaseclassCheck.h"
 #include "NoAssemblerCheck.h"
+#include "SignedBitwiseCheck.h"
 
 namespace clang {
 namespace tidy {
@@ -38,6 +39,8 @@ public:
 "hicpp-braces-around-statements");
 CheckFactories.registerCheck(
 "hicpp-exception-baseclass");
+CheckFactories.registerCheck(
+"hicpp-signed-bitwise");
 CheckFactories.registerCheck(
 "hicpp-explicit-conversions");
 CheckFactories.registerCheck(

Added: clang-tools-extra/trunk/clang-tidy/hicpp/SignedBitwiseCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/SignedBitwiseCheck.cpp?rev=312122&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/hicpp/SignedBitwiseCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/hicpp/SignedBitwiseCheck.cpp Wed Aug 30 
06:32:05 2017
@@ -0,0 +1,56 @@
+//===--- SignedBitwiseCheck.cpp - 
clang-tidy---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "SignedBitwiseCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+using namespace clang::ast_matchers::internal;
+
+namespace clang {
+namespace tidy {
+namespace hicpp {
+
+void SignedBitwiseCheck::registerMatchers(MatchFinder *Finder) {
+  const auto SignedIntegerOperand =
+  
expr(ignoringImpCasts(hasType(isSignedInteger(.bind("signed_operand");
+
+  // Match binary bitwise operations on signed integer arguments.
+  Finder->addMatcher(
+  binaryOperator(allOf(anyOf(hasOperatorName("|"), hasOperatorName("&"),
+ hasOperatorName("^"), hasOperatorName("<<"),
+ hasOperatorName(">>")),
+   hasEitherOperand(SignedIntegerOperand)))
+  .bind("binary_signed"),
+  this);
+
+  // Match unary operations on signed integer types.
+  Finder->addMatcher(unaryOperator(allOf(hasOperatorName("~"),
+ 
hasUnaryOperand(SignedIntegerOperand)))
+ .bind("unary_signed"),
+ this);
+}
+
+void SignedBitwiseCheck::check(const MatchFinder::MatchResult &Result) {
+  const ast_matchers::BoundNodes &N = Result.Nodes;
+  const auto *SignedBinary = N.getNodeAs("binary_signed");
+  const auto *SignedUnary = N.getNodeAs("unary_signed");
+  const auto *SignedOperand = N.getNodeAs("signe

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 113254.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.

Reword the 'no magic found' error message.


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
@@ -73,21 +73,23 @@

   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
   # avoiding empty lines which could potentially trigger formatting-related
   # checks.
   cleaned_test = re.sub('// *CHECK-[A-Z-]*:[^\r\n]*', '//', input_text)

   write_file(temp_file_name, cleaned_test)

   original_file_name = temp_file_name + ".orig"
   write_file(original_file_name, cleaned_test)

   args = ['clang-tidy', temp_file_name, '-fix', '--checks=-*,' + check_name] + 
\
 clang_tidy_extra_args
   print('Running ' + repr(args) + '...')
@@ -136,5 +138,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
@@ -73,21 +73,23 @@

   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
   # avoiding empty lines which could potentially trigger formatting-related
   # checks.
   cleaned_test = re.sub('// *CHECK-[A-Z-]*:[^\r\n]*', '//', input_text)

   write_file(temp_file_name, cleaned_test)

   original_file_name = temp_file_name + ".orig"
   write_file(original_file_name, cleaned_test)

   args = ['clang-tidy', temp_file_name, '-fix', '--checks=-*,' + check_name] + \
 clang_tidy_extra_args
   print('Running ' + repr(args) + '...')
@@ -136,5 +138,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] D33826: [clang-tidy] avoid pointer cast to more strict alignment check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Any status update here? :)
I generally do see a benefit in this check, because `-Wcast-align` (at least 
currently?) does not warn on `reinterpret_cast<>()`.


Repository:
  rL LLVM

https://reviews.llvm.org/D33826



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

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

[Raoul Wols via Phabricator via cfe-commits]

rwols updated this revision to Diff 113258.
rwols added a comment.

Some more tweaks

- Move assert to constructor of CompletionItemsCollector
- Use a local variable for the annotations count
- Move the documentation handling to its own private const member function


https://reviews.llvm.org/D37101

Files:
  clangd/ClangdUnit.cpp
  clangd/Protocol.cpp

Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -724,8 +724,8 @@
   if (!CI.insertText.empty())
 Os << R"("insertText":")" << llvm::yaml::escape(CI.insertText) << R"(",)";
   if (CI.insertTextFormat != InsertTextFormat::Missing) {
-Os << R"("insertTextFormat":")" << static_cast(CI.insertTextFormat)
-   << R"(",)";
+Os << R"("insertTextFormat":)" << static_cast(CI.insertTextFormat)
+   << R"(,)";
   }
   if (CI.textEdit)
 Os << R"("textEdit":)" << TextEdit::unparse(*CI.textEdit) << ',';
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -272,6 +272,17 @@
   }
 }
 
+std::string SnippetEscape(const llvm::StringRef Text) {
+  std::string Result;
+  Result.reserve(Text.size()); // Assume '$', '}' and '\\' are rare.
+  for (const auto Character : Text) {
+if (Character == '$' || Character == '}' || Character == '\\')
+  Result.push_back('\\');
+Result.push_back(Character);
+  }
+  return Result;
+}
+
 class CompletionItemsCollector : public CodeCompleteConsumer {
   std::vector *Items;
   std::shared_ptr Allocator;
@@ -283,50 +294,178 @@
   : CodeCompleteConsumer(CodeCompleteOpts, /*OutputIsBinary=*/false),
 Items(Items),
 Allocator(std::make_shared()),
-CCTUInfo(Allocator) {}
+CCTUInfo(Allocator) {
+assert(Items && "We need a non-null items container here");
+  }
 
   void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,
   CodeCompletionResult *Results,
   unsigned NumResults) override {
-for (unsigned I = 0; I != NumResults; ++I) {
-  CodeCompletionResult &Result = Results[I];
-  CodeCompletionString *CCS = Result.CreateCodeCompletionString(
+Items->reserve(NumResults);
+for (unsigned I = 0; I < NumResults; ++I) {
+  auto &Result = Results[I];
+  const auto *CCS = Result.CreateCodeCompletionString(
   S, Context, *Allocator, CCTUInfo,
   CodeCompleteOpts.IncludeBriefComments);
-  if (CCS) {
-CompletionItem Item;
-for (CodeCompletionString::Chunk C : *CCS) {
-  switch (C.Kind) {
-  case CodeCompletionString::CK_ResultType:
-Item.detail = C.Text;
-break;
-  case CodeCompletionString::CK_Optional:
-break;
-  default:
-Item.label += C.Text;
-break;
-  }
-}
-assert(CCS->getTypedText());
-Item.kind = getKind(Result.CursorKind);
-// Priority is a 16-bit integer, hence at most 5 digits.
-assert(CCS->getPriority() < 9 && "Expecting code completion result "
- "priority to have at most "
- "5-digits");
-llvm::raw_string_ostream(Item.sortText)
-<< llvm::format("%05d%s", CCS->getPriority(), CCS->getTypedText());
-Item.insertText = Item.filterText = CCS->getTypedText();
-if (CCS->getBriefComment())
-  Item.documentation = CCS->getBriefComment();
-Items->push_back(std::move(Item));
-  }
+  assert(CCS && "Expected the CodeCompletionString to be non-null");
+  Items->push_back(ProcessCodeCompleteResult(Result, *CCS));
 }
   }
 
   GlobalCodeCompletionAllocator &getAllocator() override { return *Allocator; }
 
   CodeCompletionTUInfo &getCodeCompletionTUInfo() override { return CCTUInfo; }
-};
+
+private:
+  CompletionItem
+  ProcessCodeCompleteResult(const CodeCompletionResult &Result,
+const CodeCompletionString &CCS) const {
+CompletionItem Item;
+
+// Always a snippet. This is because CK_Informative chunks should not be
+// inserted into the text buffer, but they are part of the label. For
+// example, "foo::bar() const" is the label, but "bar()" is the insertText.
+Item.insertTextFormat = InsertTextFormat::Snippet;
+
+// Fill in the documentation field of the CompletionItem.
+ProcessDocumentation(CCS, Item);
+
+// Fill in the label, detail and insertText fields of the CompletionItem.
+ProcessChunks(CCS, Item);
+
+// Fill in the kind field of the CompletionItem.
+Item.kind = getKind(Result.CursorKind);
+
+// Fill in the sortText of the CompletionItem.
+assert(CCS.getPriority() < 9 && "Expecting code completion result "
+"priority to have at most

[PATCH] D15465: [git-clang-format]: New option to perform formatting against staged changes only

[Mark Lodato via Phabricator via cfe-commits]

lodato added a comment.

Sorry, I have been very busy with other work so I haven't had a chance to 
follow along. (I don't work on LLVM team - I just contributed this script.)

I'll try to carve out some time to review within the next week.


Repository:
  rL LLVM

https://reviews.llvm.org/D15465



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

[PATCH] D33826: [clang-tidy] avoid pointer cast to more strict alignment check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

In https://reviews.llvm.org/D33826#856610, @lebedev.ri wrote:

> Any status update here? :)
>  I generally do see a benefit in this check, because `-Wcast-align` (at least 
> currently?) does not warn on `reinterpret_cast<>()`.


I think will be good idea to extend -Wcast-align.


Repository:
  rL LLVM

https://reviews.llvm.org/D33826



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

[PATCH] D33826: [clang-tidy] avoid pointer cast to more strict alignment check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D33826#856619, @Eugene.Zelenko wrote:

> In https://reviews.llvm.org/D33826#856610, @lebedev.ri wrote:
>
> > Any status update here? :)
> >  I generally do see a benefit in this check, because `-Wcast-align` (at 
> > least currently?) does not warn on `reinterpret_cast<>()`.
>
>
> I think will be good idea to extend -Wcast-align.


Hmm, i though it was actually intentional, but filled 
https://bugs.llvm.org/show_bug.cgi?id=33397 anyway back then.
I'll look into it then, should be easy.


Repository:
  rL LLVM

https://reviews.llvm.org/D33826



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

[PATCH] D15465: [git-clang-format]: New option to perform formatting against staged changes only

[Alexander Shukaev via Phabricator via cfe-commits]

Alexander-Shukaev added a comment.

Hi @lodato, thanks mate.


Repository:
  rL LLVM

https://reviews.llvm.org/D15465



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

[PATCH] D35955: clang-format: Add preprocessor directive indentation

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

I'll land this for you, as discussed offline. The best thing is to apply for 
commit rights 
 after you 
have a few patches landed.


https://reviews.llvm.org/D35955



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

r312124 - Driver: out-of-line static analyzer flag handling (NFC)

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Aug 30 07:18:08 2017
New Revision: 312124

URL: http://llvm.org/viewvc/llvm-project?rev=312124&view=rev
Log:
Driver: out-of-line static analyzer flag handling (NFC)

Extract the analyzer flag handling into its own function to reduce the
overall complexity of the construction of the clang compiler arguments.
NFC.

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=312124&r1=312123&r2=312124&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Wed Aug 30 07:18:08 2017
@@ -1968,6 +1968,78 @@ static void CollectArgsForIntegratedAsse
   }
 }
 
+static void RenderAnalyzerOptions(const ArgList &Args, ArgStringList &CmdArgs,
+  const llvm::Triple &Triple,
+  const InputInfo &Input) {
+  // Enable region store model by default.
+  CmdArgs.push_back("-analyzer-store=region");
+
+  // Treat blocks as analysis entry points.
+  CmdArgs.push_back("-analyzer-opt-analyze-nested-blocks");
+
+  CmdArgs.push_back("-analyzer-eagerly-assume");
+
+  // Add default argument set.
+  if (!Args.hasArg(options::OPT__analyzer_no_default_checks)) {
+CmdArgs.push_back("-analyzer-checker=core");
+CmdArgs.push_back("-analyzer-checker=apiModeling");
+
+if (!Triple.isWindowsMSVCEnvironment()) {
+  CmdArgs.push_back("-analyzer-checker=unix");
+} else {
+  // Enable "unix" checkers that also work on Windows.
+  CmdArgs.push_back("-analyzer-checker=unix.API");
+  CmdArgs.push_back("-analyzer-checker=unix.Malloc");
+  CmdArgs.push_back("-analyzer-checker=unix.MallocSizeof");
+  CmdArgs.push_back("-analyzer-checker=unix.MismatchedDeallocator");
+  CmdArgs.push_back("-analyzer-checker=unix.cstring.BadSizeArg");
+  CmdArgs.push_back("-analyzer-checker=unix.cstring.NullArg");
+}
+
+// Disable some unix checkers for PS4.
+if (Triple.isPS4CPU()) {
+  CmdArgs.push_back("-analyzer-disable-checker=unix.API");
+  CmdArgs.push_back("-analyzer-disable-checker=unix.Vfork");
+}
+
+if (Triple.isOSDarwin())
+  CmdArgs.push_back("-analyzer-checker=osx");
+
+CmdArgs.push_back("-analyzer-checker=deadcode");
+
+if (types::isCXX(Input.getType()))
+  CmdArgs.push_back("-analyzer-checker=cplusplus");
+
+if (!Triple.isPS4CPU()) {
+  
CmdArgs.push_back("-analyzer-checker=security.insecureAPI.UncheckedReturn");
+  CmdArgs.push_back("-analyzer-checker=security.insecureAPI.getpw");
+  CmdArgs.push_back("-analyzer-checker=security.insecureAPI.gets");
+  CmdArgs.push_back("-analyzer-checker=security.insecureAPI.mktemp");
+  CmdArgs.push_back("-analyzer-checker=security.insecureAPI.mkstemp");
+  CmdArgs.push_back("-analyzer-checker=security.insecureAPI.vfork");
+}
+
+// Default nullability checks.
+CmdArgs.push_back("-analyzer-checker=nullability.NullPassedToNonnull");
+CmdArgs.push_back("-analyzer-checker=nullability.NullReturnedFromNonnull");
+  }
+
+  // Set the output format. The default is plist, for (lame) historical 
reasons.
+  CmdArgs.push_back("-analyzer-output");
+  if (Arg *A = Args.getLastArg(options::OPT__analyzer_output))
+CmdArgs.push_back(A->getValue());
+  else
+CmdArgs.push_back("plist");
+
+  // Disable the presentation of standard compiler warnings when using
+  // --analyze.  We only want to show static analyzer diagnostics or frontend
+  // errors.
+  CmdArgs.push_back("-w");
+
+  // Add -Xanalyzer arguments when running as analyzer.
+  Args.AddAllArgValues(CmdArgs, options::OPT_Xanalyzer);
+}
+
 static void RenderSSPOptions(const ToolChain &TC, const ArgList &Args,
  ArgStringList &CmdArgs, bool KernelOrKext,
  bool IsHosted) {
@@ -2273,78 +2345,8 @@ void Clang::ConstructJob(Compilation &C,
   if (Args.hasArg(options::OPT_static))
 CmdArgs.push_back("-static-define");
 
-  if (isa(JA)) {
-// Enable region store model by default.
-CmdArgs.push_back("-analyzer-store=region");
-
-// Treat blocks as analysis entry points.
-CmdArgs.push_back("-analyzer-opt-analyze-nested-blocks");
-
-CmdArgs.push_back("-analyzer-eagerly-assume");
-
-// Add default argument set.
-if (!Args.hasArg(options::OPT__analyzer_no_default_checks)) {
-  CmdArgs.push_back("-analyzer-checker=core");
-  CmdArgs.push_back("-analyzer-checker=apiModeling");
-
-if (!IsWindowsMSVC) {
-  CmdArgs.push_back("-analyzer-checker=unix");
-} else {
-  // Enable "unix" checkers that also work on Windows.
-  CmdArgs.push_back("-analyzer-checker=unix.API");
-  CmdArgs.push_back("-analyzer-checker=unix.Malloc");
-  CmdArgs.push_back("-analyzer-checker=unix.MallocSizeof")

r312125 - clang-format: Add preprocessor directive indentation

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Wed Aug 30 07:34:57 2017
New Revision: 312125

URL: http://llvm.org/viewvc/llvm-project?rev=312125&view=rev
Log:
clang-format: Add preprocessor directive indentation

Summary:
This is an implementation for [bug 
17362](https://bugs.llvm.org/attachment.cgi?bugid=17362) which adds support for 
indenting preprocessor statements inside if/ifdef/endif. This takes previous 
work from fmauch (https://github.com/fmauch/clang/tree/preprocessor_indent) and 
makes it into a full feature.
The context of this patch is that I'm a VMware intern, and I implemented this 
because VMware needs the feature. As such, some decisions were made based on 
what VMware wants, and I would appreciate suggestions on expanding this if 
necessary to use-cases other people may want.

This adds a new enum config option, `IndentPPDirectives`. Values are:

* `PPDIS_None` (in config: `None`):
```
#if FOO
#if BAR
#include 
#endif
#endif
```
* `PPDIS_AfterHash` (in config: `AfterHash`):
```
#if FOO
#  if BAR
#include 
#  endif
#endif
```
This is meant to work whether spaces or tabs are used for indentation. 
Preprocessor indentation is independent of indentation for non-preprocessor 
lines.

Preprocessor indentation also attempts to ignore include guards with the checks:
1. Include guards cover the entire file
2. Include guards don't have `#else`
3. Include guards begin with
```
#ifndef 
#define 
```

This patch allows `UnwrappedLineParser::PPBranchLevel` to be decremented to -1 
(the initial value is -1) so the variable can be used for indent tracking.

Defects:
* This patch does not handle the case where there's code between the `#ifndef` 
and `#define` but all other conditions hold. This is because when the #define 
line is parsed, `UnwrappedLineParser::Lines` doesn't hold the previous code 
line yet, so we can't detect it. This is out of the scope of this patch.

* This patch does not handle cases where legitimate lines may be outside an 
include guard. Examples are `#pragma once` and `#pragma GCC diagnostic`, or 
anything else that does not change the meaning of the file if it's included 
multiple times.

* This does not detect when there is a single non-preprocessor line in front of 
an include-guard-like structure where other conditions hold because 
`ScopedLineState` hides the line.

* Preprocessor indentation throws off `TokenAnnotator::setCommentLineLevels` so 
the indentation of comments immediately before indented preprocessor lines is 
toggled on each run. Fixing this issue appears to be a major change and too 
much complexity for this patch.

Contributed by @euhlmann!

Reviewers: djasper, klimek, krasimir

Reviewed By: djasper, krasimir

Subscribers: krasimir, mzeren-vmw, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.h
cfe/trunk/unittests/Format/FormatTest.cpp
cfe/trunk/unittests/Format/FormatTestUtils.h

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=312125&r1=312124&r2=312125&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Wed Aug 30 07:34:57 2017
@@ -1179,6 +1179,33 @@ the configuration (without a prefix: ``A
plop();  plop();
  }  }
 
+**IndentPPDirectives** (``PPDirectiveIndentStyle``)
+  Indent preprocessor directives on conditionals.
+
+  Possible values:
+
+  * ``PPDIS_None`` (in configuration: ``None``)
+Does not indent any directives.
+
+.. code-block:: c++
+
+  #if FOO
+  #if BAR
+  #include 
+  #endif
+  #endif
+
+  * ``PPDIS_AfterHash`` (in configuration: ``AfterHash``)
+Indents directives after the hash.
+
+.. code-block:: c++
+
+  #if FOO
+  #  if BAR
+  #include 
+  #  endif
+  #endif
+
 **IndentWidth** (``unsigned``)
   The number of columns to use for indentation.
 

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=312125&r1=312124&r2=312125&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Wed Aug 30 07:34:57 2017
@@ -1024,6 +1024,31 @@ struct FormatStyle {
   /// \endcode
   bool IndentCaseLabels;
 
+  /// \brief Options for indenting preprocessor directives.
+  enum PPDirectiveInd

[PATCH] D35955: clang-format: Add preprocessor directive indentation

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL312125: clang-format: Add preprocessor directive indentation 
(authored by krasimir).

Changed prior to commit:
  https://reviews.llvm.org/D35955?vs=112432&id=113261#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35955

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/ContinuationIndenter.cpp
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp
  cfe/trunk/lib/Format/UnwrappedLineParser.h
  cfe/trunk/unittests/Format/FormatTest.cpp
  cfe/trunk/unittests/Format/FormatTestUtils.h

Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -231,10 +231,15 @@
 : Line(new UnwrappedLine), MustBreakBeforeNextToken(false),
   CurrentLines(&Lines), Style(Style), Keywords(Keywords),
   CommentPragmasRegex(Style.CommentPragmas), Tokens(nullptr),
-  Callback(Callback), AllTokens(Tokens), PPBranchLevel(-1) {}
+  Callback(Callback), AllTokens(Tokens), PPBranchLevel(-1),
+  IfNdefCondition(nullptr), FoundIncludeGuardStart(false),
+  IncludeGuardRejected(false) {}
 
 void UnwrappedLineParser::reset() {
   PPBranchLevel = -1;
+  IfNdefCondition = nullptr;
+  FoundIncludeGuardStart = false;
+  IncludeGuardRejected = false;
   Line.reset(new UnwrappedLine);
   CommentsBeforeNextToken.clear();
   FormatTok = nullptr;
@@ -679,7 +684,7 @@
 }
   }
   // Guard against #endif's without #if.
-  if (PPBranchLevel > 0)
+  if (PPBranchLevel > -1)
 --PPBranchLevel;
   if (!PPChainBranchIndex.empty())
 PPChainBranchIndex.pop();
@@ -696,19 +701,53 @@
   if (IfDef && !IfNDef && FormatTok->TokenText == "SWIG")
 Unreachable = true;
   conditionalCompilationStart(Unreachable);
+  FormatToken *IfCondition = FormatTok;
+  // If there's a #ifndef on the first line, and the only lines before it are
+  // comments, it could be an include guard.
+  bool MaybeIncludeGuard = IfNDef;
+  if (!IncludeGuardRejected && !FoundIncludeGuardStart && MaybeIncludeGuard) {
+for (auto &Line : Lines) {
+  if (!Line.Tokens.front().Tok->is(tok::comment)) {
+MaybeIncludeGuard = false;
+IncludeGuardRejected = true;
+break;
+  }
+}
+  }
+  --PPBranchLevel;
   parsePPUnknown();
+  ++PPBranchLevel;
+  if (!IncludeGuardRejected && !FoundIncludeGuardStart && MaybeIncludeGuard)
+IfNdefCondition = IfCondition;
 }
 
 void UnwrappedLineParser::parsePPElse() {
+  // If a potential include guard has an #else, it's not an include guard.
+  if (FoundIncludeGuardStart && PPBranchLevel == 0)
+FoundIncludeGuardStart = false;
   conditionalCompilationAlternative();
+  if (PPBranchLevel > -1)
+--PPBranchLevel;
   parsePPUnknown();
+  ++PPBranchLevel;
 }
 
 void UnwrappedLineParser::parsePPElIf() { parsePPElse(); }
 
 void UnwrappedLineParser::parsePPEndIf() {
   conditionalCompilationEnd();
   parsePPUnknown();
+  // If the #endif of a potential include guard is the last thing in the file,
+  // then we count it as a real include guard and subtract one from every
+  // preprocessor indent.
+  unsigned TokenPosition = Tokens->getPosition();
+  FormatToken *PeekNext = AllTokens[TokenPosition];
+  if (FoundIncludeGuardStart && PPBranchLevel == -1 && PeekNext->is(tok::eof)) {
+for (auto &Line : Lines) {
+  if (Line.InPPDirective && Line.Level > 0)
+--Line.Level;
+}
+  }
 }
 
 void UnwrappedLineParser::parsePPDefine() {
@@ -718,14 +757,26 @@
 parsePPUnknown();
 return;
   }
+  if (IfNdefCondition && IfNdefCondition->TokenText == FormatTok->TokenText) {
+FoundIncludeGuardStart = true;
+for (auto &Line : Lines) {
+  if (!Line.Tokens.front().Tok->isOneOf(tok::comment, tok::hash)) {
+FoundIncludeGuardStart = false;
+break;
+  }
+}
+  }
+  IfNdefCondition = nullptr;
   nextToken();
   if (FormatTok->Tok.getKind() == tok::l_paren &&
   FormatTok->WhitespaceRange.getBegin() ==
   FormatTok->WhitespaceRange.getEnd()) {
 parseParens();
   }
+  if (Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash)
+Line->Level += PPBranchLevel + 1;
   addUnwrappedLine();
-  Line->Level = 1;
+  ++Line->Level;
 
   // Errors during a preprocessor directive can only affect the layout of the
   // preprocessor directive, and thus we ignore them. An alternative approach
@@ -739,7 +790,10 @@
   do {
 nextToken();
   } while (!eof());
+  if (Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash)
+Line->Level += PPBranchLevel + 1;
   addUnwrappedLine();
+  IfNdefCondition = nullptr;
 }
 
 // Here we blacklist certain tokens that are not usually the first token in an
Index: cfe/trunk/lib/Format/Format.cpp

[PATCH] D33826: [clang-tidy] avoid pointer cast to more strict alignment check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a subscriber: rjmccall.
lebedev.ri added a comment.

In https://reviews.llvm.org/D33826#856619, @Eugene.Zelenko wrote:

> In https://reviews.llvm.org/D33826#856610, @lebedev.ri wrote:
>
> > Any status update here? :)
> >  I generally do see a benefit in this check, because `-Wcast-align` (at 
> > least currently?) does not warn on `reinterpret_cast<>()`.
>
>
> I think will be good idea to extend -Wcast-align.


Hm, are you *sure* `reinterpret_cast<>()` is not allowed to increase alignment?
There was a commit that intentionally removed that warning: (by @rjmccall, i 
believe)
https://github.com/llvm-mirror/clang/commit/35a38d95da89d48778019c37b5f8c9a20f7e309c


Repository:
  rL LLVM

https://reviews.llvm.org/D33826



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

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked 3 inline comments as done.
JonasToth added inline comments.



Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:9
+class deep_hierarchy : public derived_exception {};
 class non_derived_exception {};
 

aaron.ballman wrote:
> JonasToth wrote:
> > aaron.ballman wrote:
> > > Can you add a test that uses multiple inheritance? e.g.,
> > > ```
> > > class terrible_idea : public non_derived_exception, public 
> > > derived_exception {};
> > > ```
> > > Also, is private inheritance also acceptable, or does it need to be 
> > > public inheritance? I kind of get the impression it needs to be public, 
> > > because the goal appears to be that you should always be able to catch a 
> > > `std::exception` instance, and you can't do that if it's privately 
> > > inherited. That should have a test as well.
> > The rules do not state directly, that it must be inherited public, but i 
> > dont see a good reason to allow non-public inheritance.
> > Another thing is, that you can always call `e.what()` on public derived 
> > exceptions.
> > 
> > Multiple inheritance is harder, since the type is still a `std::exception`. 
> > One could catch it and use its interface, so these reasons are gone to 
> > disallow it.
> > The rules do not state directly, that it must be inherited public, but i 
> > dont see a good reason to allow non-public inheritance.
> 
> Agreed.
> 
> > Another thing is, that you can always call e.what() on public derived 
> > exceptions.
> 
> Agreed.
> 
> > Multiple inheritance is harder, since the type is still a std::exception. 
> > One could catch it and use its interface, so these reasons are gone to 
> > disallow it.
> 
> I think the multiple inheritance case should not diagnose because it meets 
> the HIC++ requirement of being derived from `std::exception`.
I have a problem with implementing the inheritance rules.

From the Matchers, there seems to be no way to test, if the inheritance is 
public. Should i work a new matcher for that, or rather move the tests, if the 
type holds all conditions into the callback function. This would mean, that 
every `throw` gets matched.


https://reviews.llvm.org/D37060



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

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:9
+class deep_hierarchy : public derived_exception {};
 class non_derived_exception {};
 

JonasToth wrote:
> aaron.ballman wrote:
> > JonasToth wrote:
> > > aaron.ballman wrote:
> > > > Can you add a test that uses multiple inheritance? e.g.,
> > > > ```
> > > > class terrible_idea : public non_derived_exception, public 
> > > > derived_exception {};
> > > > ```
> > > > Also, is private inheritance also acceptable, or does it need to be 
> > > > public inheritance? I kind of get the impression it needs to be public, 
> > > > because the goal appears to be that you should always be able to catch 
> > > > a `std::exception` instance, and you can't do that if it's privately 
> > > > inherited. That should have a test as well.
> > > The rules do not state directly, that it must be inherited public, but i 
> > > dont see a good reason to allow non-public inheritance.
> > > Another thing is, that you can always call `e.what()` on public derived 
> > > exceptions.
> > > 
> > > Multiple inheritance is harder, since the type is still a 
> > > `std::exception`. One could catch it and use its interface, so these 
> > > reasons are gone to disallow it.
> > > The rules do not state directly, that it must be inherited public, but i 
> > > dont see a good reason to allow non-public inheritance.
> > 
> > Agreed.
> > 
> > > Another thing is, that you can always call e.what() on public derived 
> > > exceptions.
> > 
> > Agreed.
> > 
> > > Multiple inheritance is harder, since the type is still a std::exception. 
> > > One could catch it and use its interface, so these reasons are gone to 
> > > disallow it.
> > 
> > I think the multiple inheritance case should not diagnose because it meets 
> > the HIC++ requirement of being derived from `std::exception`.
> I have a problem with implementing the inheritance rules.
> 
> From the Matchers, there seems to be no way to test, if the inheritance is 
> public. Should i work a new matcher for that, or rather move the tests, if 
> the type holds all conditions into the callback function. This would mean, 
> that every `throw` gets matched.
I would say you can handle private inheritance in a follow-up patch. I would 
look into changing the `isPublic()` (and related) matchers to handle 
inheritance (might as well handle `isVirtual()` at the same time, too), though 
I've not given this interface a ton of thought.


https://reviews.llvm.org/D37060



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

[PATCH] D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: include/clang/Tooling/Refactoring/RefactoringResultConsumer.h:39
+  /// Handles the source replacements that are produced by a refactoring 
action.
+  virtual void handle(AtomicChanges SourceReplacements) = 0;
+};

I think this interface is specific to some refactoring rules and should be 
pushed down to derived classes.



Comment at: unittests/Tooling/RefactoringActionRulesTest.cpp:39
+  class Consumer final : public RefactoringResultConsumer {
+void handleInitiationFailure() {
+  Result = Expected>(None);

Can we probably have default error handling in the base class so that we don't 
need to re-implement these for every derived consumer. I would expect the error 
handling for initiation and invocation to be similar in different consumers.


Repository:
  rL LLVM

https://reviews.llvm.org/D37291



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

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 113265.
JonasToth added a comment.

- adjusted the diagnostic for member variables


https://reviews.llvm.org/D36354

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp
  clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
  clang-tidy/hicpp/CMakeLists.txt
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/hicpp/SignedBitwiseCheck.cpp
  clang-tidy/hicpp/SignedBitwiseCheck.h
  clang-tidy/utils/Matchers.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst
  docs/clang-tidy/checks/hicpp-signed-bitwise.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-owning-memory.cpp
  test/clang-tidy/hicpp-signed-bitwise.cpp

Index: test/clang-tidy/hicpp-signed-bitwise.cpp
===
--- /dev/null
+++ test/clang-tidy/hicpp-signed-bitwise.cpp
@@ -0,0 +1,219 @@
+// RUN: %check_clang_tidy %s hicpp-signed-bitwise %t
+
+// These could cause false positives and should not be considered.
+struct StreamClass {
+};
+StreamClass &operator<<(StreamClass &os, unsigned int i) {
+  return os;
+}
+StreamClass &operator<<(StreamClass &os, int i) {
+  return os;
+}
+StreamClass &operator>>(StreamClass &os, unsigned int i) {
+  return os;
+}
+StreamClass &operator>>(StreamClass &os, int i) {
+  return os;
+}
+struct AnotherStream {
+  AnotherStream &operator<<(unsigned char c) { return *this; }
+  AnotherStream &operator<<(char c) { return *this; }
+
+  AnotherStream &operator>>(unsigned char c) { return *this; }
+  AnotherStream &operator>>(char c) { return *this; }
+};
+
+void binary_bitwise() {
+  int SValue = 42;
+  int SResult;
+
+  unsigned int UValue = 42;
+  unsigned int UResult;
+
+  SResult = SValue & 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  SResult = SValue & -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  SResult = SValue & SValue;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = SValue & 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = SValue & -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = UValue & 1u; // Ok
+  UResult = UValue & UValue; // Ok
+
+  unsigned char UByte1 = 0u;
+  unsigned char UByte2 = 16u;
+  char SByte1 = 0;
+  char SByte2 = 16;
+
+  UByte1 = UByte1 & UByte2; // Ok
+  UByte1 = SByte1 & UByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  UByte1 = SByte1 & SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  SByte1 = SByte1 & SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  // More complex expressions.
+  UResult = UValue & (SByte1 + (SByte1 | SByte2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-2]]:33: warning: use of a signed integer operand with a binary bitwise operator
+
+  // The rest is to demonstrate functionality but all operators are matched equally.
+  // Therefore functionality is the same for all binary operations.
+  UByte1 = UByte1 | UByte2; // Ok
+  UByte1 = UByte1 | SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  UByte1 = UByte1 ^ UByte2; // Ok
+  UByte1 = UByte1 ^ SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  UByte1 = UByte1 >> UByte2; // Ok
+  UByte1 = UByte1 >> SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  UByte1 = UByte1 << UByte2; // Ok
+  UByte1 = UByte1 << SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  int SignedInt1 = 1 << 12;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand with a binary bitwise operator
+  int SignedInt2 = 1u << 12;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand with a binary bitwise operator
+}
+
+void f1(unsigned char c) {}
+void f2(char c) {}
+void f3(int c) {}
+
+void unary_bitwise() {
+  unsigned char UByte1 = 0u;
+  char SByte1 = 0;
+
+  UByte1 = ~UByte1; // Ok
+  SByte1 = ~UByte1;
+  SByte1 = ~SByte1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a unary bitwise operator
+  UByte1 = ~SByte1;
+  // CHECK-ME

r312127 - [refactor] AST selection tree should contain syntactic form

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Aug 30 08:00:27 2017
New Revision: 312127

URL: http://llvm.org/viewvc/llvm-project?rev=312127&view=rev
Log:
[refactor] AST selection tree should contain syntactic form
of PseudoObjectExpr

The AST selection finder now constructs a selection tree that contains only the
syntactic form of PseudoObjectExpr. This form of selection tree is more
meaningful when doing downstream analysis as we're interested in the syntactic
features of the AST and the correct lexical parent relation.

Modified:
cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp
cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp

Modified: cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp?rev=312127&r1=312126&r2=312127&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp Wed Aug 30 08:00:27 2017
@@ -10,6 +10,7 @@
 #include "clang/Tooling/Refactoring/ASTSelection.h"
 #include "clang/AST/LexicallyOrderedRecursiveASTVisitor.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/Support/SaveAndRestore.h"
 
 using namespace clang;
 using namespace tooling;
@@ -60,6 +61,21 @@ public:
 return std::move(Result);
   }
 
+  bool TraversePseudoObjectExpr(PseudoObjectExpr *E) {
+// Avoid traversing the semantic expressions. They should be handled by
+// looking through the appropriate opaque expressions in order to build
+// a meaningful selection tree.
+llvm::SaveAndRestore LookThrough(LookThroughOpaqueValueExprs, true);
+return TraverseStmt(E->getSyntacticForm());
+  }
+
+  bool TraverseOpaqueValueExpr(OpaqueValueExpr *E) {
+if (!LookThroughOpaqueValueExprs)
+  return true;
+llvm::SaveAndRestore LookThrough(LookThroughOpaqueValueExprs, false);
+return TraverseStmt(E->getSourceExpr());
+  }
+
   bool TraverseDecl(Decl *D) {
 if (isa(D))
   return LexicallyOrderedRecursiveASTVisitor::TraverseDecl(D);
@@ -97,6 +113,8 @@ public:
   bool TraverseStmt(Stmt *S) {
 if (!S)
   return true;
+if (auto *Opaque = dyn_cast(S))
+  return TraverseOpaqueValueExpr(Opaque);
 // FIXME (Alex Lorenz): Improve handling for macro locations.
 SourceSelectionKind SelectionKind =
 selectionKindFor(CharSourceRange::getTokenRange(S->getSourceRange()));
@@ -149,6 +167,10 @@ private:
   FileID TargetFile;
   const ASTContext &Context;
   std::vector SelectionStack;
+  /// Controls whether we can traverse through the OpaqueValueExpr. This is
+  /// typically enabled during the traversal of syntactic form for
+  /// PseudoObjectExprs.
+  bool LookThroughOpaqueValueExprs = false;
 };
 
 } // end anonymous namespace

Modified: cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp?rev=312127&r1=312126&r2=312127&view=diff
==
--- cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp Wed Aug 30 08:00:27 2017
@@ -513,4 +513,140 @@ TEST(ASTSelectionFinder, CorrectEndForOb
SelectionFinderVisitor::Lang_OBJC);
 }
 
+const SelectedASTNode &checkFnBody(const Optional &Node,
+   StringRef Name) {
+  EXPECT_TRUE(Node);
+  EXPECT_EQ(Node->Children.size(), 1u);
+  const auto &Fn = checkNode(
+  Node->Children[0], SourceSelectionKind::ContainsSelection,
+  /*NumChildren=*/1, Name);
+  return checkNode(Fn.Children[0],
+ SourceSelectionKind::ContainsSelection,
+ /*NumChildren=*/1);
+}
+
+TEST(ASTSelectionFinder, SelectObjectiveCPseudoObjectExprs) {
+  StringRef Source = R"(
+@interface I
+@property(readwrite) int prop;
+@end
+void selectProp(I *i) {
+(void)i.prop;
+i.prop = 21;
+}
+
+typedef unsigned int size_t;
+@interface NSMutableArray
+- (id)objectAtIndexedSubscript:(size_t)index;
+- (void)setObject:(id)object atIndexedSubscript:(size_t)index;
+@end
+
+void selectSubscript(NSMutableArray *array, I *i) {
+  (void)array[10];
+  array[i.prop] = i;
+}
+)";
+  // Just 'i.prop'.
+  findSelectedASTNodes(
+  Source, {6, 7}, FileRange{{6, 7}, {6, 13}},
+  [](Optional Node) {
+const auto &CS = checkFnBody(Node, /*Name=*/"selectProp");
+const auto &CCast = checkNode(
+CS.Children[0], SourceSelectionKind::ContainsSelection,
+/*NumChildren=*/1);
+const auto &POE = checkNode(
+CCast.Children[0], SourceSelectionKind::ContainsSelection,
+/*NumChildren=*/1);
+const auto &PRE = checkNode(
+POE.Children[0], SourceSelectionKind::ContainsSelection,
+/*NumChildren=*/1);
+const auto &Cast = checkNode(
+PR

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked 5 inline comments as done.
JonasToth added inline comments.



Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:9
+class deep_hierarchy : public derived_exception {};
 class non_derived_exception {};
 

aaron.ballman wrote:
> JonasToth wrote:
> > aaron.ballman wrote:
> > > JonasToth wrote:
> > > > aaron.ballman wrote:
> > > > > Can you add a test that uses multiple inheritance? e.g.,
> > > > > ```
> > > > > class terrible_idea : public non_derived_exception, public 
> > > > > derived_exception {};
> > > > > ```
> > > > > Also, is private inheritance also acceptable, or does it need to be 
> > > > > public inheritance? I kind of get the impression it needs to be 
> > > > > public, because the goal appears to be that you should always be able 
> > > > > to catch a `std::exception` instance, and you can't do that if it's 
> > > > > privately inherited. That should have a test as well.
> > > > The rules do not state directly, that it must be inherited public, but 
> > > > i dont see a good reason to allow non-public inheritance.
> > > > Another thing is, that you can always call `e.what()` on public derived 
> > > > exceptions.
> > > > 
> > > > Multiple inheritance is harder, since the type is still a 
> > > > `std::exception`. One could catch it and use its interface, so these 
> > > > reasons are gone to disallow it.
> > > > The rules do not state directly, that it must be inherited public, but 
> > > > i dont see a good reason to allow non-public inheritance.
> > > 
> > > Agreed.
> > > 
> > > > Another thing is, that you can always call e.what() on public derived 
> > > > exceptions.
> > > 
> > > Agreed.
> > > 
> > > > Multiple inheritance is harder, since the type is still a 
> > > > std::exception. One could catch it and use its interface, so these 
> > > > reasons are gone to disallow it.
> > > 
> > > I think the multiple inheritance case should not diagnose because it 
> > > meets the HIC++ requirement of being derived from `std::exception`.
> > I have a problem with implementing the inheritance rules.
> > 
> > From the Matchers, there seems to be no way to test, if the inheritance is 
> > public. Should i work a new matcher for that, or rather move the tests, if 
> > the type holds all conditions into the callback function. This would mean, 
> > that every `throw` gets matched.
> I would say you can handle private inheritance in a follow-up patch. I would 
> look into changing the `isPublic()` (and related) matchers to handle 
> inheritance (might as well handle `isVirtual()` at the same time, too), 
> though I've not given this interface a ton of thought.
Ok. I will commit this one with according FIXME: sections and will `#if 0` the 
currently offending check-messages.

from the usability, something like:
`isSameOrDerivedFrom("std::exception", isPublic())` would be nice, but i dont 
know if this is even possible.
In that sense, the other modifiers could be used as well (`isVirtual()`, 
`isProtected()`...).


https://reviews.llvm.org/D37060



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

r312131 - Revert r312127 as the ObjC unittest code fails to compile on Linux

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Aug 30 08:11:45 2017
New Revision: 312131

URL: http://llvm.org/viewvc/llvm-project?rev=312131&view=rev
Log:
Revert r312127 as the ObjC unittest code fails to compile on Linux

Modified:
cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp
cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp

Modified: cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp?rev=312131&r1=312130&r2=312131&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp Wed Aug 30 08:11:45 2017
@@ -10,7 +10,6 @@
 #include "clang/Tooling/Refactoring/ASTSelection.h"
 #include "clang/AST/LexicallyOrderedRecursiveASTVisitor.h"
 #include "clang/Lex/Lexer.h"
-#include "llvm/Support/SaveAndRestore.h"
 
 using namespace clang;
 using namespace tooling;
@@ -61,21 +60,6 @@ public:
 return std::move(Result);
   }
 
-  bool TraversePseudoObjectExpr(PseudoObjectExpr *E) {
-// Avoid traversing the semantic expressions. They should be handled by
-// looking through the appropriate opaque expressions in order to build
-// a meaningful selection tree.
-llvm::SaveAndRestore LookThrough(LookThroughOpaqueValueExprs, true);
-return TraverseStmt(E->getSyntacticForm());
-  }
-
-  bool TraverseOpaqueValueExpr(OpaqueValueExpr *E) {
-if (!LookThroughOpaqueValueExprs)
-  return true;
-llvm::SaveAndRestore LookThrough(LookThroughOpaqueValueExprs, false);
-return TraverseStmt(E->getSourceExpr());
-  }
-
   bool TraverseDecl(Decl *D) {
 if (isa(D))
   return LexicallyOrderedRecursiveASTVisitor::TraverseDecl(D);
@@ -113,8 +97,6 @@ public:
   bool TraverseStmt(Stmt *S) {
 if (!S)
   return true;
-if (auto *Opaque = dyn_cast(S))
-  return TraverseOpaqueValueExpr(Opaque);
 // FIXME (Alex Lorenz): Improve handling for macro locations.
 SourceSelectionKind SelectionKind =
 selectionKindFor(CharSourceRange::getTokenRange(S->getSourceRange()));
@@ -167,10 +149,6 @@ private:
   FileID TargetFile;
   const ASTContext &Context;
   std::vector SelectionStack;
-  /// Controls whether we can traverse through the OpaqueValueExpr. This is
-  /// typically enabled during the traversal of syntactic form for
-  /// PseudoObjectExprs.
-  bool LookThroughOpaqueValueExprs = false;
 };
 
 } // end anonymous namespace

Modified: cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp?rev=312131&r1=312130&r2=312131&view=diff
==
--- cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp Wed Aug 30 08:11:45 2017
@@ -513,140 +513,4 @@ TEST(ASTSelectionFinder, CorrectEndForOb
SelectionFinderVisitor::Lang_OBJC);
 }
 
-const SelectedASTNode &checkFnBody(const Optional &Node,
-   StringRef Name) {
-  EXPECT_TRUE(Node);
-  EXPECT_EQ(Node->Children.size(), 1u);
-  const auto &Fn = checkNode(
-  Node->Children[0], SourceSelectionKind::ContainsSelection,
-  /*NumChildren=*/1, Name);
-  return checkNode(Fn.Children[0],
- SourceSelectionKind::ContainsSelection,
- /*NumChildren=*/1);
-}
-
-TEST(ASTSelectionFinder, SelectObjectiveCPseudoObjectExprs) {
-  StringRef Source = R"(
-@interface I
-@property(readwrite) int prop;
-@end
-void selectProp(I *i) {
-(void)i.prop;
-i.prop = 21;
-}
-
-typedef unsigned int size_t;
-@interface NSMutableArray
-- (id)objectAtIndexedSubscript:(size_t)index;
-- (void)setObject:(id)object atIndexedSubscript:(size_t)index;
-@end
-
-void selectSubscript(NSMutableArray *array, I *i) {
-  (void)array[10];
-  array[i.prop] = i;
-}
-)";
-  // Just 'i.prop'.
-  findSelectedASTNodes(
-  Source, {6, 7}, FileRange{{6, 7}, {6, 13}},
-  [](Optional Node) {
-const auto &CS = checkFnBody(Node, /*Name=*/"selectProp");
-const auto &CCast = checkNode(
-CS.Children[0], SourceSelectionKind::ContainsSelection,
-/*NumChildren=*/1);
-const auto &POE = checkNode(
-CCast.Children[0], SourceSelectionKind::ContainsSelection,
-/*NumChildren=*/1);
-const auto &PRE = checkNode(
-POE.Children[0], SourceSelectionKind::ContainsSelection,
-/*NumChildren=*/1);
-const auto &Cast = checkNode(
-PRE.Children[0], SourceSelectionKind::InsideSelection,
-/*NumChildren=*/1);
-checkNode(Cast.Children[0],
-   SourceSelectionKind::InsideSelection);
-  },
-  SelectionFinderVisitor::Lang_OBJC);
-  // Just 'i.prop = 21'
-  findSelectedASTNodes(
-  So

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added subscribers: sbenza, klimek.
aaron.ballman added inline comments.



Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:9
+class deep_hierarchy : public derived_exception {};
 class non_derived_exception {};
 

JonasToth wrote:
> aaron.ballman wrote:
> > JonasToth wrote:
> > > aaron.ballman wrote:
> > > > JonasToth wrote:
> > > > > aaron.ballman wrote:
> > > > > > Can you add a test that uses multiple inheritance? e.g.,
> > > > > > ```
> > > > > > class terrible_idea : public non_derived_exception, public 
> > > > > > derived_exception {};
> > > > > > ```
> > > > > > Also, is private inheritance also acceptable, or does it need to be 
> > > > > > public inheritance? I kind of get the impression it needs to be 
> > > > > > public, because the goal appears to be that you should always be 
> > > > > > able to catch a `std::exception` instance, and you can't do that if 
> > > > > > it's privately inherited. That should have a test as well.
> > > > > The rules do not state directly, that it must be inherited public, 
> > > > > but i dont see a good reason to allow non-public inheritance.
> > > > > Another thing is, that you can always call `e.what()` on public 
> > > > > derived exceptions.
> > > > > 
> > > > > Multiple inheritance is harder, since the type is still a 
> > > > > `std::exception`. One could catch it and use its interface, so these 
> > > > > reasons are gone to disallow it.
> > > > > The rules do not state directly, that it must be inherited public, 
> > > > > but i dont see a good reason to allow non-public inheritance.
> > > > 
> > > > Agreed.
> > > > 
> > > > > Another thing is, that you can always call e.what() on public derived 
> > > > > exceptions.
> > > > 
> > > > Agreed.
> > > > 
> > > > > Multiple inheritance is harder, since the type is still a 
> > > > > std::exception. One could catch it and use its interface, so these 
> > > > > reasons are gone to disallow it.
> > > > 
> > > > I think the multiple inheritance case should not diagnose because it 
> > > > meets the HIC++ requirement of being derived from `std::exception`.
> > > I have a problem with implementing the inheritance rules.
> > > 
> > > From the Matchers, there seems to be no way to test, if the inheritance 
> > > is public. Should i work a new matcher for that, or rather move the 
> > > tests, if the type holds all conditions into the callback function. This 
> > > would mean, that every `throw` gets matched.
> > I would say you can handle private inheritance in a follow-up patch. I 
> > would look into changing the `isPublic()` (and related) matchers to handle 
> > inheritance (might as well handle `isVirtual()` at the same time, too), 
> > though I've not given this interface a ton of thought.
> Ok. I will commit this one with according FIXME: sections and will `#if 0` 
> the currently offending check-messages.
> 
> from the usability, something like:
> `isSameOrDerivedFrom("std::exception", isPublic())` would be nice, but i dont 
> know if this is even possible.
> In that sense, the other modifiers could be used as well (`isVirtual()`, 
> `isProtected()`...).
I'm not too certain (maybe @klimek or @sbenza knows more), but I think you can 
modify `isDerivedFrom()` to accept an additional matcher so that could can 
write `isDerivedFrom(hasName("X"), allOf(isPublic(), isVirtual()))` and then 
thread that change through to the other derived matchers. I would guess this 
means `isPublic()` (et al) would need to accept a `CXXBaseSpecifier` as well as 
the declarations they currently accept.


https://reviews.llvm.org/D37060



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

[PATCH] D36410: [OpenCL] Handle taking address of block captures

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In https://reviews.llvm.org/D36410#855358, @bader wrote:

> In https://reviews.llvm.org/D36410#855282, @Anastasia wrote:
>
> > Ok, I will update it to be implicitly generic then. Just to be sure, @bader 
> > do you agree on this too?
>
>
>
>
> > An alternative approached could be (in case we want to allow this code) to 
> > **assume captures are located in the generic AS implicitly**. However, in 
> > this case programmers should be advised that erroneous AS casts can occur 
> > further that can't be diagnosed by the frontend (i.e. if capture address is 
> > used further in the operations of a different address space to where the 
> > captures physical location is).
>
> I don't have a strong opinion on this topic, but I'm worried about relying on 
> implicit assumptions, which might make code less portable.
>
> How the alternative approach is this suppose to work for enqueue_kernel? Do 
> we assume that the captured variables passed by generic pointer and compiler 
> will assign the correct address space based on the explicit casts in the 
> block?
>  There are some restrictions on kernel parameters: 6.9.a. Arguments to kernel 
> functions declared in a program that are pointers must be declared with the 
> global, constant or local qualifier.


The captured variable is still passed by value. The address taking is on the 
duplicate of the captured variable, not on the original variable. My 
understanding is that OpenCL does not require the real address space of a 
generic pointer to be deducible at compile/link time, therefore a compliant 
implementation should be able to dereference a generic pointer at runtime.

>> I feel forbidding user taking address of captured variable is too 
>> restrictive and make blocks less useful.
> 
> I have no information how widely blocks are used by the OpenCL community and 
> how important this feature is.

The OpenCL spec does not forbid taking address of captured variable. Forbidding 
taking address of captured variable would violate the spec.


https://reviews.llvm.org/D36410



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

[PATCH] D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: include/clang/Tooling/Refactoring/RefactoringResultConsumer.h:39
+  /// Handles the source replacements that are produced by a refactoring 
action.
+  virtual void handle(AtomicChanges SourceReplacements) = 0;
+};

ioeric wrote:
> I think this interface is specific to some refactoring rules and should be 
> pushed down to derived classes.
Are you talking about derived classes of `RefactoringResultConsumer`? So 
something like

```
class RefactoringResultConsumer {
  virtual void handleInvocationError(llvm::Error Err) = 0;
};

class RefactoringResultSourceReplacementConsumer: RefactoringResultConsumer {
 virtual void handle(AtomicChanges SourceReplacements) = 0;
};
```

If yes, can you please clarify how the rule can call `handle` if it's in a 
subclass of `RefactoringResultConsumer`?



Repository:
  rL LLVM

https://reviews.llvm.org/D37291



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

[PATCH] D36272: [CodeGen][x86_64] Enable 'force_align_arg_pointer' attribute at x86_64

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D36272#856040, @anatol.pomozov wrote:

> Hi Eric, thank you for your reply. Both these triples are currently broken, 
> with my change and without.
>
> The attribute functionality in WinX86_64TargetCodeGenInfo mirrors one in 
> X86_64TargetCodeGenInfo and it should work the same way. It is done 
> intentionally as both these systems have the stack alignment restrictions.
>
> In addition to option you proposed there are other ways to make the forward 
> progress:
>
> - fix function-attributes.c for win32 systems (both 32 and 64bit). Having 
> working tests is important not only for force_align_arg_pointer functionality 
> but for any other win32 change. It needs attention from Clang developers who 
> is familiar with this area.
> - bypass the broken win32 test as it is done up until now.


As I said, as long as BOTH windows versions have the same issue, I think _I_ am 
OK with it.  Since x86-windows already didn't work, you've simply added 
x64-windows support, which seems like a win to me.  I think I'm ok with this 
for now, so I'll approve.
Hope that is OK Aaron :)


https://reviews.llvm.org/D36272



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

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 113269.
JonasToth marked an inline comment as done.
JonasToth added a comment.

- adjusted expected diagnostics
- adjust diagnostics and remove private inheritance cases


https://reviews.llvm.org/D37060

Files:
  clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
  test/clang-tidy/hicpp-exception-baseclass.cpp

Index: test/clang-tidy/hicpp-exception-baseclass.cpp
===
--- test/clang-tidy/hicpp-exception-baseclass.cpp
+++ test/clang-tidy/hicpp-exception-baseclass.cpp
@@ -5,38 +5,175 @@
 } // namespace std
 
 class derived_exception : public std::exception {};
+class deep_hierarchy : public derived_exception {};
 class non_derived_exception {};
+class terrible_idea : public non_derived_exception, public derived_exception {};
+
+// FIXME: More complicated kinds of inheritance should be checked later, but there is
+// currently no way use ASTMatchers for this kind of task.
+#if 0
+class bad_inheritance : private std::exception {};
+class no_good_inheritance : protected std::exception {};
+class really_creative : public non_derived_exception, private std::exception {};
+#endif
 
 void problematic() {
   try {
-throw int(42); // Built in is not allowed
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: throwing an exception whose type is not derived from 'std::exception'
+throw int(42);
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
   } catch (int e) {
   }
-  throw int(42); // Bad
-// CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type is not derived from 'std::exception'
+  throw int(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+
+  try {
+throw 12;
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+  } catch (...) {
+throw; // Ok, even if the type is not known, conforming code can never rethrow a non-std::exception object.
+  }
 
   try {
-throw non_derived_exception(); // Some class is not allowed
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: throwing an exception whose type is not derived from 'std::exception'
-// CHECK-MESSAGES: 8:1: note: type defined here
+throw non_derived_exception();
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 9:1: note: type defined here
   } catch (non_derived_exception &e) {
   }
-  throw non_derived_exception(); // Bad
-// CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type is not derived from 'std::exception'
-// CHECK-MESSAGES: 8:1: note: type defined here
+  throw non_derived_exception();
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+  // CHECK-MESSAGES: 9:1: note: type defined here
+
+// FIXME: More complicated kinds of inheritance should be checked later, but there is
+// currently no way use ASTMatchers for this kind of task.
+#if 0
+  // Handle private inheritance cases correctly.
+  try {
+throw bad_inheritance();
+// CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
+// CHECK MESSAGES: 10:1: note: type defined here
+throw no_good_inheritance();
+// CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
+// CHECK MESSAGES: 11:1: note: type defined here
+throw really_creative();
+// CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
+// CHECK MESSAGES: 12:1: note: type defined here
+  } catch (...) {
+  }
+  throw bad_inheritance();
+  // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
+  // CHECK MESSAGES: 10:1: note: type defined here
+  throw no_good_inheritance();
+  // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
+  // CHECK MESSAGES: 11:1: note: type defined here
+  throw really_creative();
+  // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
+  // CHECK MESSAGES: 12:1: note: type defined here
+#endif
 }
 
 void allowed_throws() {
   try {
-throw std::exception(); // Ok
+throw std::exception(); // Ok
   } catch (std::exception &e) { // Ok
   }
   throw std::exception();
 
   try {
-throw derived_exception(); // Ok
+throw derived_exception(); // Ok
   } catch (derived_exception &e) { // Ok
   }
   throw derived_exception(); // Ok
+
+  try 

r312132 - Recommit r312127: [refactor] AST selection tree should contain syntactic

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Aug 30 08:28:01 2017
New Revision: 312132

URL: http://llvm.org/viewvc/llvm-project?rev=312132&view=rev
Log:
Recommit r312127: [refactor] AST selection tree should contain syntactic
form of PseudoObjectExpr

The new commit adjusts unittest test code compilation options so that the
Objective-C code in the unittest can be parsed on non-macOS platforms.

Original message:

The AST selection finder now constructs a selection tree that contains only the
syntactic form of PseudoObjectExpr. This form of selection tree is more
meaningful when doing downstream analysis as we're interested in the syntactic
features of the AST and the correct lexical parent relation.

Modified:
cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp
cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp
cfe/trunk/unittests/Tooling/TestVisitor.h

Modified: cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp?rev=312132&r1=312131&r2=312132&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp Wed Aug 30 08:28:01 2017
@@ -10,6 +10,7 @@
 #include "clang/Tooling/Refactoring/ASTSelection.h"
 #include "clang/AST/LexicallyOrderedRecursiveASTVisitor.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/Support/SaveAndRestore.h"
 
 using namespace clang;
 using namespace tooling;
@@ -60,6 +61,21 @@ public:
 return std::move(Result);
   }
 
+  bool TraversePseudoObjectExpr(PseudoObjectExpr *E) {
+// Avoid traversing the semantic expressions. They should be handled by
+// looking through the appropriate opaque expressions in order to build
+// a meaningful selection tree.
+llvm::SaveAndRestore LookThrough(LookThroughOpaqueValueExprs, true);
+return TraverseStmt(E->getSyntacticForm());
+  }
+
+  bool TraverseOpaqueValueExpr(OpaqueValueExpr *E) {
+if (!LookThroughOpaqueValueExprs)
+  return true;
+llvm::SaveAndRestore LookThrough(LookThroughOpaqueValueExprs, false);
+return TraverseStmt(E->getSourceExpr());
+  }
+
   bool TraverseDecl(Decl *D) {
 if (isa(D))
   return LexicallyOrderedRecursiveASTVisitor::TraverseDecl(D);
@@ -97,6 +113,8 @@ public:
   bool TraverseStmt(Stmt *S) {
 if (!S)
   return true;
+if (auto *Opaque = dyn_cast(S))
+  return TraverseOpaqueValueExpr(Opaque);
 // FIXME (Alex Lorenz): Improve handling for macro locations.
 SourceSelectionKind SelectionKind =
 selectionKindFor(CharSourceRange::getTokenRange(S->getSourceRange()));
@@ -149,6 +167,10 @@ private:
   FileID TargetFile;
   const ASTContext &Context;
   std::vector SelectionStack;
+  /// Controls whether we can traverse through the OpaqueValueExpr. This is
+  /// typically enabled during the traversal of syntactic form for
+  /// PseudoObjectExprs.
+  bool LookThroughOpaqueValueExprs = false;
 };
 
 } // end anonymous namespace

Modified: cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp?rev=312132&r1=312131&r2=312132&view=diff
==
--- cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp Wed Aug 30 08:28:01 2017
@@ -513,4 +513,140 @@ TEST(ASTSelectionFinder, CorrectEndForOb
SelectionFinderVisitor::Lang_OBJC);
 }
 
+const SelectedASTNode &checkFnBody(const Optional &Node,
+   StringRef Name) {
+  EXPECT_TRUE(Node);
+  EXPECT_EQ(Node->Children.size(), 1u);
+  const auto &Fn = checkNode(
+  Node->Children[0], SourceSelectionKind::ContainsSelection,
+  /*NumChildren=*/1, Name);
+  return checkNode(Fn.Children[0],
+ SourceSelectionKind::ContainsSelection,
+ /*NumChildren=*/1);
+}
+
+TEST(ASTSelectionFinder, SelectObjectiveCPseudoObjectExprs) {
+  StringRef Source = R"(
+@interface I
+@property(readwrite) int prop;
+@end
+void selectProp(I *i) {
+(void)i.prop;
+i.prop = 21;
+}
+
+typedef unsigned int size_t;
+@interface NSMutableArray
+- (id)objectAtIndexedSubscript:(size_t)index;
+- (void)setObject:(id)object atIndexedSubscript:(size_t)index;
+@end
+
+void selectSubscript(NSMutableArray *array, I *i) {
+  (void)array[10];
+  array[i.prop] = i;
+}
+)";
+  // Just 'i.prop'.
+  findSelectedASTNodes(
+  Source, {6, 7}, FileRange{{6, 7}, {6, 13}},
+  [](Optional Node) {
+const auto &CS = checkFnBody(Node, /*Name=*/"selectProp");
+const auto &CCast = checkNode(
+CS.Children[0], SourceSelectionKind::ContainsSelection,
+/*NumChildren=*/1);
+const auto &POE = checkNode(
+CCast.Children[0], SourceSelectionKind::ContainsSelection,

[PATCH] D37182: [libcxx] Special visibility macros for the experimental library

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

The reason for building the filesystem library as a statically linked lib 
(instead of dynamic) is that for quite a while it was changing significantly.  
Having people link statically means that we can make changes w/o worrying (as 
much) about people using the library - they can pick up new features /changes 
when they re-link their programs. Once we change to a dylib (and we will, but 
not yet), then we have to worry about ABI changes affecting programs in the 
field.


https://reviews.llvm.org/D37182



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

r312133 - Avoid 'size_t' typedef in the unittest ObjC code

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Aug 30 08:37:30 2017
New Revision: 312133

URL: http://llvm.org/viewvc/llvm-project?rev=312133&view=rev
Log:
Avoid 'size_t' typedef in the unittest ObjC code

This should fix
http://bb.pgr.jp/builders/test-clang-msc-x64-on-i686-linux-RA

Modified:
cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp

Modified: cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp?rev=312133&r1=312132&r2=312133&view=diff
==
--- cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp Wed Aug 30 08:37:30 2017
@@ -535,10 +535,10 @@ void selectProp(I *i) {
 i.prop = 21;
 }
 
-typedef unsigned int size_t;
+
 @interface NSMutableArray
-- (id)objectAtIndexedSubscript:(size_t)index;
-- (void)setObject:(id)object atIndexedSubscript:(size_t)index;
+- (id)objectAtIndexedSubscript:(unsigned int)index;
+- (void)setObject:(id)object atIndexedSubscript:(unsigned int)index;
 @end
 
 void selectSubscript(NSMutableArray *array, I *i) {


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

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst:11
+The relevant sections in the `C++ Core Guidelines 
`_
 are I.11, C.33, R.3 and GSL.Views
+The definition of an ``gsl::owner`` is straight forward
+

s/an/a/



Comment at: docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst:17
+
+it is therefore simple to introduce the owner even without using an 
implementation of
+the `Guideline Support Library 
`_.

s/it/It/



Comment at: docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst:27
+
+  // Creating an owner with factory functions is caught.
+  gsl::owner function_that_returns_owner() { return gsl::owner(new 
int(42)); }

s/caught/checked/



Comment at: docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst:97
+  void bad_template_function(T some_object) {
+// This line will trigger the warning, that an non-owner is assigned to an 
owner
+gsl::owner new_owner = some_object;

s/an/a/



Comment at: docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst:131
+
+The semantic of an ``gsl::owner`` is mostly like a ``std::unique_ptr``, 
therefore
+assignment of two ``gsl::owner`` is considered a move, but therefore the 
resource

therefore ... therefore



Comment at: docs/clang-tidy/checks/hicpp-signed-bitwise.rst:1
+.. title:: clang-tidy - hicpp-signed-bitwise
+

remove from diff



Comment at: docs/clang-tidy/checks/list.rst:83
hicpp-noexcept-move (redirects to misc-noexcept-moveconstructor) 

+   hicpp-signed-bitwise
hicpp-special-member-functions (redirects to 
cppcoreguidelines-special-member-functions) 

remove from diff



Comment at: test/clang-tidy/hicpp-signed-bitwise.cpp:1
+// RUN: %check_clang_tidy %s hicpp-signed-bitwise %t
+

remove from diff


https://reviews.llvm.org/D36354



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

[PATCH] D37299: [Modules] Add ability to specify module name to module file mapping in a file

[Boris Kolpackov via Phabricator via cfe-commits]

boris created this revision.

Add the -fmodule-file-map=[=] option which can be used to specify 
a file that contains module name to precompiled modules files mapping, similar 
to -fmodule-file==. The  can be used to only consider 
certain lines which can be useful if we want to store the mapping in an already 
existing file (for example, as comments in the .d makefile fragment generated 
with the -M option or some such).

-

Additional notes:

1. Based on this mailing list discussion: 
http://lists.llvm.org/pipermail/cfe-dev/2017-June/054431.html

2. Based on the functionality implemented in: https://reviews.llvm.org/D35020


https://reviews.llvm.org/D37299

Files:
  docs/Modules.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CXX/modules-ts/basic/basic.search/module-import.cpp

Index: test/CXX/modules-ts/basic/basic.search/module-import.cpp
===
--- test/CXX/modules-ts/basic/basic.search/module-import.cpp
+++ test/CXX/modules-ts/basic/basic.search/module-import.cpp
@@ -5,6 +5,8 @@
 // RUN: echo 'export module x; int a, b;' > %t/x.cppm
 // RUN: echo 'export module y; import x; int c;' > %t/y.cppm
 // RUN: echo 'export module z; import y; int d;' > %t/z.cppm
+// RUN: echo 'x=%t/x.pcm'  > %t/modmap
+// RUN: echo 'y=%t/y.pcm' >> %t/modmap
 //
 // RUN: %clang_cc1 -std=c++1z -fmodules-ts -emit-module-interface %t/x.cppm -o %t/x.pcm
 // RUN: %clang_cc1 -std=c++1z -fmodules-ts -emit-module-interface -fmodule-file=%t/x.pcm %t/y.cppm -o %t/y.pcm
@@ -19,7 +21,17 @@
 // RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=y=%t/y.pcm -verify %s \
 // RUN:-DMODULE_NAME=y
 //
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file-map=%t/modmap -verify %s \
+// RUN:-DMODULE_NAME=x
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file-map=%t/modmap -verify %s \
+// RUN:-DMODULE_NAME=y
+//
 // RUN: mv %t/x.pcm %t/a.pcm
+// RUN: echo 'foo.o: foo.cxx'   > %t/modmap
+// RUN: echo '# Mmodule name to file mapping:' >> %t/modmap
+// RUN: echo '#@z=%t/z.pcm'>> %t/modmap
+// RUN: echo '#@ y=%t/y.pcm'   >> %t/modmap
+// RUN: echo '#@x=%t/a.pcm '   >> %t/modmap
 //
 // RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=x=%t/a.pcm -verify %s \
 // RUN:-DMODULE_NAME=x
@@ -33,6 +45,14 @@
 // RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=z=%t/z.pcm -fmodule-file=y=%t/y.pcm -fmodule-file=x=%t/a.pcm -verify %s \
 // RUN:-DMODULE_NAME=z
 //
+//
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file-map=#@=%t/modmap -verify %s \
+// RUN:-DMODULE_NAME=x
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file-map=#@=%t/modmap -verify %s \
+// RUN:-DMODULE_NAME=y
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file-map=#@=%t/modmap -verify %s \
+// RUN:-DMODULE_NAME=z
+//
 
 import MODULE_NAME;
 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1528,8 +1528,89 @@
   return P.str();
 }
 
+// Read the mapping of module names to precompiled module files from a file.
+// The argument can include an optional line prefix ([]=), in
+// which case only lines that start with the prefix are considered (with the
+// prefix and the following whitespaces, if any, ignored).
+//
+// Each mapping entry should be in the same form as the -fmodule-file option
+// value (=) with leading/trailing whitespaces ignored.
+//
+static void LoadModuleFileMap(HeaderSearchOptions &Opts,
+  DiagnosticsEngine &Diags,
+  FileManager &FileMgr,
+  StringRef Val,
+  const std::string &Arg) {
+  // See if we have the prefix.
+  StringRef File;
+  StringRef Prefix;
+  if (Val.find('=') != StringRef::npos)
+  {
+auto Pair = Val.split('=');
+Prefix = Pair.first;
+File = Pair.second;
+if (Prefix.empty())
+{
+  Diags.Report(diag::err_drv_invalid_value) << Arg << Val;
+  return;
+}
+  }
+  else
+File = Val;
+
+  if (File.empty())
+  {
+Diags.Report(diag::err_drv_invalid_value) << Arg << Val;
+return;
+  }
+
+  auto Buf = FileMgr.getBufferForFile(File);
+  if (!Buf)
+  {
+Diags.Report(diag::err_cannot_open_file) << File << Buf.getError().message();
+return;
+  }
+
+  // Read the file line by line.
+  StringRef Str = Buf.get()->getBuffer();
+  for (size_t B(0), E(B); B < Str.size(); B = E + 1)
+  {
+E = Str.find_first_of(StringRef("\n\0", 2), B);
+
+if (E == StringRef::npos)
+  E = Str.size();
+else if (Str[E] == '\0')
+  break; // The file (or the rest of it

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 113272.
JonasToth marked 5 inline comments as done.
JonasToth added a comment.

- fix typos and bad language


https://reviews.llvm.org/D36354

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp
  clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
  clang-tidy/utils/Matchers.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-owning-memory.cpp

Index: test/clang-tidy/cppcoreguidelines-owning-memory.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-owning-memory.cpp
@@ -0,0 +1,347 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-owning-memory %t
+
+namespace gsl {
+template 
+using owner = T;
+} // namespace gsl
+
+template 
+class unique_ptr {
+public:
+  unique_ptr(gsl::owner resource) : memory(resource) {}
+  unique_ptr(const unique_ptr &) = default;
+
+  ~unique_ptr() { delete memory; }
+
+private:
+  gsl::owner memory;
+};
+
+void takes_owner(gsl::owner owned_int) {
+}
+
+void takes_pointer(int *unowned_int) {
+}
+
+void takes_owner_and_more(int some_int, gsl::owner owned_int, float f) {
+}
+
+template 
+void takes_templated_owner(gsl::owner owned_T) {
+}
+
+gsl::owner returns_owner1() { return gsl::owner(new int(42)); } // Ok
+gsl::owner returns_owner2() { return new int(42); } // Ok
+
+int *returns_no_owner1() { return nullptr; }
+int *returns_no_owner2() {
+  return new int(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: returning a 'gsl::owner<>' from a function but not declaring it; return type is 'int *'
+}
+int *returns_no_owner3() {
+  int *should_be_owner = new int(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+  return should_be_owner;
+}
+int *returns_no_owner4() {
+  gsl::owner owner = new int(42);
+  return owner;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: returning a 'gsl::owner<>' from a function but not declaring it; return type is 'int *'
+}
+
+unique_ptr returns_no_owner5() {
+  return unique_ptr(new int(42)); // Ok
+}
+
+/// FIXME: CSA finds it, but the report is misleading.
+void csa_not_finding_leak() {
+  gsl::owner o1 = new int(42); // Ok
+
+  gsl::owner o2 = o1; // Ok
+  o2 = new int(45); // conceptual leak, the memory from o1 is now leaked, since its considered moved in the guidelines
+
+  delete o2;
+  // actual leak occurs here, its found, but mixed
+  delete o1;
+}
+
+void test_assignment_and_initialization() {
+  int stack_int1 = 15;
+  int stack_int2;
+
+  gsl::owner owned_int1 = &stack_int1; // BAD
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected initialization with value of type 'gsl::owner<>'; got 'int *'
+
+  gsl::owner owned_int2;
+  owned_int2 = &stack_int2; // BAD since no owner, bad since uninitialized
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected assignment source to be of type 'gsl::owner<>'; got 'int *'
+
+  gsl::owner owned_int3 = new int(42); // Good
+  owned_int3 = nullptr; // Good
+
+  gsl::owner owned_int4(nullptr); // Ok
+  owned_int4 = new int(42); // Good
+
+  gsl::owner owned_int5 = owned_int3; // Good
+
+  gsl::owner owned_int6{nullptr}; // Ok
+  owned_int6 = owned_int4; // Good
+
+  // FIXME:, flow analysis for the case of reassignment. Value must be released before
+  owned_int6 = owned_int3; // BAD, because reassignment without resource release
+
+  auto owned_int7 = returns_owner1(); // Bad, since typededuction eliminates the owner wrapper
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: type deduction did not result in an owner
+
+  const auto owned_int8 = returns_owner2(); // Bad, since typededuction eliminates the owner wrapper
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *const' with a newly created 'gsl::owner<>'
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: type deduction did not result in an owner
+
+  gsl::owner owned_int9 = returns_owner1(); // Ok
+  int *unowned_int3 = returns_owner1(); // Bad
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+
+  gsl::owner owned_int10;
+  owned_int10 = returns_owner1(); // Ok
+
+  int *unowned_int4;
+  unowned_int4 = returns_owner1(); // Bad
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: assigning newly created 'gsl::owner<>' to non-owner 'int *'
+
+  gsl::owner owned_int11 = returns_no_owner1(); // Bad since no owner
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected initialization with value of type 'gsl::owner<>'; got 'int *'
+
+  gsl::owner owned_int12;
+  owned_int12 = returns_no_owner1(); // Bad since no owner
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected assignment sourc

[PATCH] D36272: [CodeGen][x86_64] Enable 'force_align_arg_pointer' attribute at x86_64

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D36272#856722, @erichkeane wrote:

> In https://reviews.llvm.org/D36272#856040, @anatol.pomozov wrote:
>
> > Hi Eric, thank you for your reply. Both these triples are currently broken, 
> > with my change and without.
> >
> > The attribute functionality in WinX86_64TargetCodeGenInfo mirrors one in 
> > X86_64TargetCodeGenInfo and it should work the same way. It is done 
> > intentionally as both these systems have the stack alignment restrictions.
> >
> > In addition to option you proposed there are other ways to make the forward 
> > progress:
> >
> > - fix function-attributes.c for win32 systems (both 32 and 64bit). Having 
> > working tests is important not only for force_align_arg_pointer 
> > functionality but for any other win32 change. It needs attention from Clang 
> > developers who is familiar with this area.
> > - bypass the broken win32 test as it is done up until now.
>
>
> As I said, as long as BOTH windows versions have the same issue, I think _I_ 
> am OK with it.  Since x86-windows already didn't work, you've simply added 
> x64-windows support, which seems like a win to me.  I think I'm ok with this 
> for now, so I'll approve.
>  Hope that is OK Aaron :)


Yes, I'm okay with that.  I'd also be more okay if we corrected the attribute 
spelling to be GCC instead of GNU and added some documentation to the 
attribute, since we're updating it.


https://reviews.llvm.org/D36272



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

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

language in the docuementation improved


https://reviews.llvm.org/D36354



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

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!


https://reviews.llvm.org/D37060



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 113271.
hamzasood added a comment.
Herald added a subscriber: klimek.

Implemented pretty printing and recursive AST visiting (both with appropriate 
unit tests).

The pretty printing implementation includes fixing a few printing bugs (which I 
needed fixed to write decent tests);

- Unnamed template parameters now print without a trailing space (e.g.  
instead of ).
- C++14 generic lambda parameters are now printed as `auto` as opposed to 
`typeparameter-depth-index`.

RecursiveASTVisitor now visits each explicit template parameter, and also the 
implicit ones if shouldVisitImplicitCode is set.
However there is a problem with this implementation that I'm not sure about.
Consider a TU that is simply: `auto l = []() { };`
If shouldVisitImplicitCode is set, the template parameter will be visited 
twice, Once when visiting the lambda (which is what I added) and once when 
visiting the implicit lambda class type (existing behaviour). Is that a problem?

With regards to serialisation, I don't think any changes are needed. However 
I've added a PCH test to be sure (which passes).


https://reviews.llvm.org/D36527

Files:
  include/clang/AST/DeclTemplate.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Sema/ScopeInfo.h
  include/clang/Sema/Sema.h
  lib/AST/DeclPrinter.cpp
  lib/AST/ExprCXX.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/TypePrinter.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaType.cpp
  test/CXX/temp/temp.decls/temp.variadic/p4.cpp
  test/PCH/cxx11-lambdas.mm
  test/PCH/cxx1y-lambdas.mm
  test/PCH/cxx2a-template-lambdas.cpp
  test/Parser/cxx2a-template-lambdas.cpp
  test/SemaCXX/cxx2a-template-lambdas.cpp
  unittests/AST/StmtPrinterTest.cpp
  unittests/Tooling/RecursiveASTVisitorTest.cpp
  unittests/Tooling/TestVisitor.h
  www/cxx_status.html

Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -822,7 +822,7 @@
 
   template-parameter-list for generic lambdas
   http://wg21.link/p0428r2";>P0428R2
-  No
+  SVN
 
 
   Initializer list constructors in class template argument deduction
Index: unittests/Tooling/TestVisitor.h
===
--- unittests/Tooling/TestVisitor.h
+++ unittests/Tooling/TestVisitor.h
@@ -44,6 +44,8 @@
 Lang_CXX98,
 Lang_CXX11,
 Lang_CXX14,
+Lang_CXX17,
+Lang_CXX2a,
 Lang_OBJC,
 Lang_OBJCXX11,
 Lang_CXX = Lang_CXX98
@@ -60,6 +62,8 @@
   case Lang_CXX98: Args.push_back("-std=c++98"); break;
   case Lang_CXX11: Args.push_back("-std=c++11"); break;
   case Lang_CXX14: Args.push_back("-std=c++14"); break;
+  case Lang_CXX17: Args.push_back("-std=c++17"); break;
+  case Lang_CXX2a: Args.push_back("-std=c++2a"); break;
   case Lang_OBJC:
 Args.push_back("-ObjC");
 Args.push_back("-fobjc-runtime=macosx-10.12.0");
Index: unittests/Tooling/RecursiveASTVisitorTest.cpp
===
--- unittests/Tooling/RecursiveASTVisitorTest.cpp
+++ unittests/Tooling/RecursiveASTVisitorTest.cpp
@@ -79,6 +79,41 @@
   LambdaDefaultCaptureVisitor::Lang_CXX11));
 }
 
+// Matches (optional) explicit template parameters.
+class LambdaTemplateParametersVisitor
+  : public ExpectedLocationVisitor {
+public:
+  bool shouldVisitImplicitCode() const { return false; }
+
+  bool VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
+EXPECT_FALSE(D->isImplicit());
+Match(D->getName(), D->getLocStart());
+return true;
+  }
+  bool VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) {
+EXPECT_FALSE(D->isImplicit());
+Match(D->getName(), D->getLocStart());
+return true;
+  }
+  bool VisitTemplateTemplateParmDecl(TemplateTemplateParmDecl *D) {
+EXPECT_FALSE(D->isImplicit());
+Match(D->getName(), D->getLocStart());
+return true;
+  }
+};
+
+TEST(RecursiveASTVisitor, VisitsLambdaExplicitTemplateParameters) {
+  LambdaTemplateParametersVisitor Visitor;
+  Visitor.ExpectMatch("T",  2, 15);
+  Visitor.ExpectMatch("I",  2, 24);
+  Visitor.ExpectMatch("TT", 2, 31);
+  EXPECT_TRUE(Visitor.runOver(
+  "void f() { \n"
+  "  auto l = [] class TT>(auto p) { }; \n"
+  "}",
+  LambdaTemplateParametersVisitor::Lang_CXX2a));
+}
+
 // Checks for lambda classes that are not marked as implicitly-generated.
 // (There should be none.)
 class ClassVisitor : public ExpectedLocationVisitor {
Index: unittests/AST/StmtPrinterTest.cpp
===
--- unittests/AST/StmtPrinterTest.cpp
+++ unittests/AST/StmtPrinterTest.cpp
@@ -67,9 +67,8 @@
 
 template 
 ::testing::AssertionResult
-PrintedStmtMatches(StringRef Code, const std::vector &Args,
-  

[clang-tools-extra] r312134 - [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Wed Aug 30 08:59:01 2017
New Revision: 312134

URL: http://llvm.org/viewvc/llvm-project?rev=312134&view=rev
Log:
[clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

Summary:
This patch is a followup to the first revision D36583, that had problems with 
generic code and its diagnostic messages, which were found by @lebedev.ri

Reviewers: alexfh, aaron.ballman, lebedev.ri, hokein

Reviewed By: aaron.ballman, lebedev.ri

Subscribers: klimek, sbenza, cfe-commits, JDevlieghere, lebedev.ri, xazax.hun

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

Modified:
clang-tools-extra/trunk/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/hicpp-exception-baseclass.cpp

Modified: clang-tools-extra/trunk/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp?rev=312134&r1=312133&r2=312134&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp Wed 
Aug 30 08:59:01 2017
@@ -11,8 +11,6 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
-#include 
-
 using namespace clang::ast_matchers;
 
 namespace clang {
@@ -24,20 +22,23 @@ void ExceptionBaseclassCheck::registerMa
 return;
 
   Finder->addMatcher(
-  cxxThrowExpr(
-  allOf(
-  has(expr(unless(hasType(cxxRecordDecl(
-  isSameOrDerivedFrom(hasName("std::exception"))),
-  eachOf(has(expr(hasType(namedDecl().bind("decl", 
anything(
+  cxxThrowExpr(allOf(has(expr(unless(hasType(qualType(hasCanonicalType(
+ hasDeclaration(cxxRecordDecl(isSameOrDerivedFrom(
+ hasName("std::exception")),
+ has(expr(unless(cxxUnresolvedConstructExpr(,
+ eachOf(has(expr(hasType(namedDecl().bind("decl",
+anything(
   .bind("bad_throw"),
   this);
 }
 
 void ExceptionBaseclassCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *BadThrow = Result.Nodes.getNodeAs("bad_throw");
-  diag(BadThrow->getLocStart(),
-   "throwing an exception whose type is not derived from 'std::exception'")
-  << BadThrow->getSourceRange();
+
+  diag(BadThrow->getSubExpr()->getLocStart(), "throwing an exception whose "
+  "type %0 is not derived from "
+  "'std::exception'")
+  << BadThrow->getSubExpr()->getType() << BadThrow->getSourceRange();
 
   const auto *TypeDecl = Result.Nodes.getNodeAs("decl");
   if (TypeDecl != nullptr)

Modified: clang-tools-extra/trunk/test/clang-tidy/hicpp-exception-baseclass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/hicpp-exception-baseclass.cpp?rev=312134&r1=312133&r2=312134&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/hicpp-exception-baseclass.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/hicpp-exception-baseclass.cpp Wed 
Aug 30 08:59:01 2017
@@ -5,38 +5,175 @@ class exception {};
 } // namespace std
 
 class derived_exception : public std::exception {};
+class deep_hierarchy : public derived_exception {};
 class non_derived_exception {};
+class terrible_idea : public non_derived_exception, public derived_exception 
{};
+
+// FIXME: More complicated kinds of inheritance should be checked later, but 
there is
+// currently no way use ASTMatchers for this kind of task.
+#if 0
+class bad_inheritance : private std::exception {};
+class no_good_inheritance : protected std::exception {};
+class really_creative : public non_derived_exception, private std::exception 
{};
+#endif
 
 void problematic() {
   try {
-throw int(42); // Built in is not allowed
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: throwing an exception whose type is 
not derived from 'std::exception'
+throw int(42);
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose 
type 'int' is not derived from 'std::exception'
   } catch (int e) {
   }
-  throw int(42); // Bad
-// CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type is 
not derived from 'std::exception'
+  throw int(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 
'int' is not derived from 'std::exception'
+
+  try {
+throw 12;
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose 
type 'int' is not derived from 'std::exception'
+  } catch (...) {
+throw; // Ok, even if the type is not known, conforming code can never 
rethrow a non-std::exce

Re: [clang-tools-extra] r312134 - [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Diana Picus via cfe-commits]

Hi Jonas,

I haven't seen any commit from you (*) fixing the buildbot failures
caused by your previous commit (r312122). This has been keeping
several bots red for quite a while:
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/10590
http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/6845
etc etc

It's customary to keep an eye on the buildbots and either revert or
fix such problems as soon as possible. It's particularly inappropriate
to commit something else on top before dealing with the bot failures.

For the record, Alex has been trying to help you on IRC in #llvm:
"alexlorenz: jonas_toth: btw, char is not signed by default (it's
platform-specific), so you should use 'signed char' in your clang-tidy
test to fix the bot failures"

Please be more careful with the buildbots in the future.

Thanks,
Diana

(*) My apologies if I missed it, but I really don't see anything in
cfe-commits nor in the situation of the bots to reflect that.

On 30 August 2017 at 17:59, Jonas Toth via cfe-commits
 wrote:
> Author: jonastoth
> Date: Wed Aug 30 08:59:01 2017
> New Revision: 312134
>
> URL: http://llvm.org/viewvc/llvm-project?rev=312134&view=rev
> Log:
> [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better
>
> Summary:
> This patch is a followup to the first revision D36583, that had problems with
> generic code and its diagnostic messages, which were found by @lebedev.ri
>
> Reviewers: alexfh, aaron.ballman, lebedev.ri, hokein
>
> Reviewed By: aaron.ballman, lebedev.ri
>
> Subscribers: klimek, sbenza, cfe-commits, JDevlieghere, lebedev.ri, xazax.hun
>
> Differential Revision: https://reviews.llvm.org/D37060
>
> Modified:
> clang-tools-extra/trunk/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
> clang-tools-extra/trunk/test/clang-tidy/hicpp-exception-baseclass.cpp
>
> Modified: clang-tools-extra/trunk/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp?rev=312134&r1=312133&r2=312134&view=diff
> ==
> --- clang-tools-extra/trunk/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp 
> (original)
> +++ clang-tools-extra/trunk/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp Wed 
> Aug 30 08:59:01 2017
> @@ -11,8 +11,6 @@
>  #include "clang/AST/ASTContext.h"
>  #include "clang/ASTMatchers/ASTMatchFinder.h"
>
> -#include 
> -
>  using namespace clang::ast_matchers;
>
>  namespace clang {
> @@ -24,20 +22,23 @@ void ExceptionBaseclassCheck::registerMa
>  return;
>
>Finder->addMatcher(
> -  cxxThrowExpr(
> -  allOf(
> -  has(expr(unless(hasType(cxxRecordDecl(
> -  isSameOrDerivedFrom(hasName("std::exception"))),
> -  eachOf(has(expr(hasType(namedDecl().bind("decl", 
> anything(
> +  cxxThrowExpr(allOf(has(expr(unless(hasType(qualType(hasCanonicalType(
> + 
> hasDeclaration(cxxRecordDecl(isSameOrDerivedFrom(
> + hasName("std::exception")),
> + has(expr(unless(cxxUnresolvedConstructExpr(,
> + eachOf(has(expr(hasType(namedDecl().bind("decl",
> +anything(
>.bind("bad_throw"),
>this);
>  }
>
>  void ExceptionBaseclassCheck::check(const MatchFinder::MatchResult &Result) {
>const auto *BadThrow = Result.Nodes.getNodeAs("bad_throw");
> -  diag(BadThrow->getLocStart(),
> -   "throwing an exception whose type is not derived from 
> 'std::exception'")
> -  << BadThrow->getSourceRange();
> +
> +  diag(BadThrow->getSubExpr()->getLocStart(), "throwing an exception whose "
> +  "type %0 is not derived from "
> +  "'std::exception'")
> +  << BadThrow->getSubExpr()->getType() << BadThrow->getSourceRange();
>
>const auto *TypeDecl = Result.Nodes.getNodeAs("decl");
>if (TypeDecl != nullptr)
>
> Modified: 
> clang-tools-extra/trunk/test/clang-tidy/hicpp-exception-baseclass.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/hicpp-exception-baseclass.cpp?rev=312134&r1=312133&r2=312134&view=diff
> ==
> --- clang-tools-extra/trunk/test/clang-tidy/hicpp-exception-baseclass.cpp 
> (original)
> +++ clang-tools-extra/trunk/test/clang-tidy/hicpp-exception-baseclass.cpp Wed 
> Aug 30 08:59:01 2017
> @@ -5,38 +5,175 @@ class exception {};
>  } // namespace std
>
>  class derived_exception : public std::exception {};
> +class deep_hierarchy : public derived_exception {};
>  class non_derived_exception {};
> +class terrible_idea : public non_derived_exception, public derived_exception 
> {};
> +
> +// FIXME: More complicated kinds of inheritance should be ch