[PATCH] D56358: [AST] Move back BasePathSize to the bit-fields of CastExpr

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/AST/Expr.h:3029
+assert((CastExprBits.BasePathSize == BasePathSize) &&
+   "BasePathSize overflow!");
 assert(CastConsistency());

riccibruno wrote:
> lebedev.ri wrote:
> > riccibruno wrote:
> > > It cannot overflow now, but someone in the future is going
> > > to shrink `CastExprBitfields::BasePathSize`.
> > Can you move that static assert here instead of deleting it?
> I am not sure it is really useful since when someone will want to
> shrink it, it will be stored as `unsigned BasePathSite : x;`. What we
> don't want is having `x` too small. However `numeric_limits` will
> be based on the underlying type of the bit-field and so will never fail.
> 
> Storing it as something other than an unsigned, say a short, would work,
> but it will pack poorly with MSVC.
> 
> Hopefully the comment in `CastExprBitfields` will tell people to not shrink it
> mindlessly.
I agree that the type-based numeric limit is not useful if we're anticipating
it becoming a bit-field.  The comment seems fine since it's in the code that
would need to be edited.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56358/new/

https://reviews.llvm.org/D56358



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

[PATCH] D56367: [AST] Pack CXXDependentScopeMemberExpr

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Does the regression disappear if you make the scope specifier the first 
trailing object?  Later trailing objects are more expensive to access, and I 
would imagine that the scope specifier and template arguments are more 
important than the other fields.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56367/new/

https://reviews.llvm.org/D56367



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

[PATCH] D56368: [AST] Store the results in OverloadExpr in a trailing array

[John McCall via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56368/new/

https://reviews.llvm.org/D56368



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

[PATCH] D56365: [X86] Use funnel shift intrinsics for the VBMI2 vshld/vshrd builtins.

[Craig Topper via Phabricator via cfe-commits]

craig.topper updated this revision to Diff 180441.
craig.topper added a comment.

Rebase after fixing immediates to be in range without a modulo


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56365/new/

https://reviews.llvm.org/D56365

Files:
  include/clang/Basic/BuiltinsX86.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/avx512vbmi2intrin.h
  lib/Headers/avx512vlvbmi2intrin.h
  test/CodeGen/avx512vbmi2-builtins.c
  test/CodeGen/avx512vlvbmi2-builtins.c

Index: test/CodeGen/avx512vlvbmi2-builtins.c
===
--- test/CodeGen/avx512vlvbmi2-builtins.c
+++ test/CodeGen/avx512vlvbmi2-builtins.c
@@ -172,457 +172,481 @@
 
 __m256i test_mm256_mask_shldi_epi64(__m256i __S, __mmask8 __U, __m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_mask_shldi_epi64
-  // CHECK: @llvm.x86.avx512.vpshld.q.256
+  // CHECK: @llvm.fshl.v4i64(<4 x i64> %{{.*}}, <4 x i64> %{{.*}}, <4 x i64> )
   // CHECK: select <4 x i1> %{{.*}}, <4 x i64> %{{.*}}, <4 x i64> %{{.*}}
   return _mm256_mask_shldi_epi64(__S, __U, __A, __B, 47);
 }
 
 __m256i test_mm256_maskz_shldi_epi64(__mmask8 __U, __m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_maskz_shldi_epi64
-  // CHECK: @llvm.x86.avx512.vpshld.q.256
+  // CHECK: @llvm.fshl.v4i64(<4 x i64> %{{.*}}, <4 x i64> %{{.*}}, <4 x i64> )
   // CHECK: select <4 x i1> %{{.*}}, <4 x i64> %{{.*}}, <4 x i64> %{{.*}}
   return _mm256_maskz_shldi_epi64(__U, __A, __B, 63);
 }
 
 __m256i test_mm256_shldi_epi64(__m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_shldi_epi64
-  // CHECK: @llvm.x86.avx512.vpshld.q.256
+  // CHECK: @llvm.fshl.v4i64(<4 x i64> %{{.*}}, <4 x i64> %{{.*}}, <4 x i64> )
   return _mm256_shldi_epi64(__A, __B, 31);
 }
 
 __m128i test_mm_mask_shldi_epi64(__m128i __S, __mmask8 __U, __m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_mask_shldi_epi64
-  // CHECK: @llvm.x86.avx512.vpshld.q.128
+  // CHECK: @llvm.fshl.v2i64(<2 x i64> %{{.*}}, <2 x i64> %{{.*}}, <2 x i64> )
   // CHECK: select <2 x i1> %{{.*}}, <2 x i64> %{{.*}}, <2 x i64> %{{.*}}
   return _mm_mask_shldi_epi64(__S, __U, __A, __B, 47);
 }
 
 __m128i test_mm_maskz_shldi_epi64(__mmask8 __U, __m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_maskz_shldi_epi64
-  // CHECK: @llvm.x86.avx512.vpshld.q.128
+  // CHECK: @llvm.fshl.v2i64(<2 x i64> %{{.*}}, <2 x i64> %{{.*}}, <2 x i64> )
   // CHECK: select <2 x i1> %{{.*}}, <2 x i64> %{{.*}}, <2 x i64> %{{.*}}
   return _mm_maskz_shldi_epi64(__U, __A, __B, 63);
 }
 
 __m128i test_mm_shldi_epi64(__m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_shldi_epi64
-  // CHECK: @llvm.x86.avx512.vpshld.q.128
+  // CHECK: @llvm.fshl.v2i64(<2 x i64> %{{.*}}, <2 x i64> %{{.*}}, <2 x i64> )
   return _mm_shldi_epi64(__A, __B, 31);
 }
 
 __m256i test_mm256_mask_shldi_epi32(__m256i __S, __mmask8 __U, __m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_mask_shldi_epi32
-  // CHECK: @llvm.x86.avx512.vpshld.d.256
+  // CHECK: @llvm.fshl.v8i32(<8 x i32> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> )
   // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> %{{.*}}
   return _mm256_mask_shldi_epi32(__S, __U, __A, __B, 7);
 }
 
 __m256i test_mm256_maskz_shldi_epi32(__mmask8 __U, __m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_maskz_shldi_epi32
-  // CHECK: @llvm.x86.avx512.vpshld.d.256
+  // CHECK: @llvm.fshl.v8i32(<8 x i32> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> )
   // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> %{{.*}}
   return _mm256_maskz_shldi_epi32(__U, __A, __B, 15);
 }
 
 __m256i test_mm256_shldi_epi32(__m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_shldi_epi32
-  // CHECK: @llvm.x86.avx512.vpshld.d.256
+  // CHECK: @llvm.fshl.v8i32(<8 x i32> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> )
   return _mm256_shldi_epi32(__A, __B, 31);
 }
 
 __m128i test_mm_mask_shldi_epi32(__m128i __S, __mmask8 __U, __m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_mask_shldi_epi32
-  // CHECK: @llvm.x86.avx512.vpshld.d.128
+  // CHECK: @llvm.fshl.v4i32(<4 x i32> %{{.*}}, <4 x i32> %{{.*}}, <4 x i32> )
   // CHECK: select <4 x i1> %{{.*}}, <4 x i32> %{{.*}}, <4 x i32> %{{.*}}
   return _mm_mask_shldi_epi32(__S, __U, __A, __B, 7);
 }
 
 __m128i test_mm_maskz_shldi_epi32(__mmask8 __U, __m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_maskz_shldi_epi32
-  // CHECK: @llvm.x86.avx512.vpshld.d.128
+  // CHECK: @llvm.fshl.v4i32(<4 x i32> %{{.*}}, <4 x i32> %{{.*}}, <4 x i32> )
   // CHECK: select <4 x i1> %{{.*}}, <4 x i32> %{{.*}}, <4 x i32> %{{.*}}
   return _mm_maskz_shldi_epi32(__U, __A, __B, 15);
 }
 
 __m128i test_mm_shldi_epi32(__m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_shldi_epi32
-  // CHECK: @llvm.x86.avx512.vpshld.d.128
+  // CHECK: @llvm.fshl.v4i32(<4 x i32> %{{.*}}, <4 x i32> %{{.*}}, <4 x i32> )
   return _mm_shldi_epi32(__A, __B, 31);
 }
 
 __m256i test_mm256_mask_shldi_epi16(__m256i __S, __mmask16 __U, __m256i __A, __m256

[PATCH] D56358: [AST] Move back BasePathSize to the bit-fields of CastExpr

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

LG in principle.




Comment at: include/clang/AST/Expr.h:3029
+assert((CastExprBits.BasePathSize == BasePathSize) &&
+   "BasePathSize overflow!");
 assert(CastConsistency());

rjmccall wrote:
> riccibruno wrote:
> > lebedev.ri wrote:
> > > riccibruno wrote:
> > > > It cannot overflow now, but someone in the future is going
> > > > to shrink `CastExprBitfields::BasePathSize`.
> > > Can you move that static assert here instead of deleting it?
> > I am not sure it is really useful since when someone will want to
> > shrink it, it will be stored as `unsigned BasePathSite : x;`. What we
> > don't want is having `x` too small. However `numeric_limits` will
> > be based on the underlying type of the bit-field and so will never fail.
> > 
> > Storing it as something other than an unsigned, say a short, would work,
> > but it will pack poorly with MSVC.
> > 
> > Hopefully the comment in `CastExprBitfields` will tell people to not shrink 
> > it
> > mindlessly.
> I agree that the type-based numeric limit is not useful if we're anticipating
> it becoming a bit-field.  The comment seems fine since it's in the code that
> would need to be edited.
I'm just worried that now there will be less of a test coverage,
The actual run-time test already does not test template instantiation with 
depth of `16384`, but much less.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56358/new/

https://reviews.llvm.org/D56358



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

[PATCH] D56380: [clangd] Fix a regression issue caused by r348365.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric.

With r348365, we now detect libc++ dir using the actual compiler path
(from the compilation command), rather than the resource-dir.

This new behavior will cause clangd couldn't find libc++ dir (even the libc++ is
built from the source) when using a fallback compilation command (`clang xxx`)

The fix is to use `/clang` as the actual compiler path.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56380

Files:
  clangd/GlobalCompilationDatabase.cpp
  unittests/clangd/GlobalCompilationDatabaseTests.cpp


Index: unittests/clangd/GlobalCompilationDatabaseTests.cpp
===
--- unittests/clangd/GlobalCompilationDatabaseTests.cpp
+++ unittests/clangd/GlobalCompilationDatabaseTests.cpp
@@ -19,18 +19,21 @@
 namespace clangd {
 namespace {
 using ::testing::ElementsAre;
+using ::testing::EndsWith;
 
 TEST(GlobalCompilationDatabaseTest, FallbackCommand) {
   DirectoryBasedGlobalCompilationDatabase DB(None);
   auto Cmd = DB.getFallbackCommand(testPath("foo/bar.cc"));
   EXPECT_EQ(Cmd.Directory, testPath("foo"));
-  EXPECT_THAT(Cmd.CommandLine, ElementsAre("clang", testPath("foo/bar.cc")));
+  EXPECT_THAT(Cmd.CommandLine, ElementsAre(
+EndsWith("clang"), testPath("foo/bar.cc")));
   EXPECT_EQ(Cmd.Output, "");
 
   // .h files have unknown language, so they are parsed liberally as obj-c++.
   Cmd = DB.getFallbackCommand(testPath("foo/bar.h"));
-  EXPECT_THAT(Cmd.CommandLine, ElementsAre("clang", "-xobjective-c++-header",
-   testPath("foo/bar.h")));
+  EXPECT_THAT(Cmd.CommandLine,
+  ElementsAre(EndsWith("clang"), "-xobjective-c++-header",
+  testPath("foo/bar.h")));
 }
 
 static tooling::CompileCommand cmd(StringRef File, StringRef Arg) {
@@ -88,7 +91,7 @@
   EXPECT_EQ(CDB.getCompileCommand(testPath("bar.cc")), Override);
 
   EXPECT_THAT(CDB.getFallbackCommand(testPath("foo.cc")).CommandLine,
-  ElementsAre("clang", testPath("foo.cc"), "-DA=6"));
+  ElementsAre(EndsWith("clang"), testPath("foo.cc"), "-DA=6"));
 }
 
 TEST_F(OverlayCDBTest, Watch) {
Index: clangd/GlobalCompilationDatabase.cpp
===
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -17,9 +17,19 @@
 namespace clang {
 namespace clangd {
 
+static std::string getFallbackClangPath() {
+  static int Dummy;
+  std::string ClangdExecutable =
+  llvm::sys::fs::getMainExecutable("clangd", (void *)&Dummy);
+  SmallString<128> ClangPath;
+  ClangPath = llvm::sys::path::parent_path(ClangdExecutable);
+  llvm::sys::path::append(ClangPath, "clang");
+  return ClangPath.str();
+}
+
 tooling::CompileCommand
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
-  std::vector Argv = {"clang"};
+  std::vector Argv = {getFallbackClangPath()};
   // Clang treats .h files as C by default, resulting in unhelpful diagnostics.
   // Parsing as Objective C++ is friendly to more cases.
   if (sys::path::extension(File) == ".h")


Index: unittests/clangd/GlobalCompilationDatabaseTests.cpp
===
--- unittests/clangd/GlobalCompilationDatabaseTests.cpp
+++ unittests/clangd/GlobalCompilationDatabaseTests.cpp
@@ -19,18 +19,21 @@
 namespace clangd {
 namespace {
 using ::testing::ElementsAre;
+using ::testing::EndsWith;
 
 TEST(GlobalCompilationDatabaseTest, FallbackCommand) {
   DirectoryBasedGlobalCompilationDatabase DB(None);
   auto Cmd = DB.getFallbackCommand(testPath("foo/bar.cc"));
   EXPECT_EQ(Cmd.Directory, testPath("foo"));
-  EXPECT_THAT(Cmd.CommandLine, ElementsAre("clang", testPath("foo/bar.cc")));
+  EXPECT_THAT(Cmd.CommandLine, ElementsAre(
+EndsWith("clang"), testPath("foo/bar.cc")));
   EXPECT_EQ(Cmd.Output, "");
 
   // .h files have unknown language, so they are parsed liberally as obj-c++.
   Cmd = DB.getFallbackCommand(testPath("foo/bar.h"));
-  EXPECT_THAT(Cmd.CommandLine, ElementsAre("clang", "-xobjective-c++-header",
-   testPath("foo/bar.h")));
+  EXPECT_THAT(Cmd.CommandLine,
+  ElementsAre(EndsWith("clang"), "-xobjective-c++-header",
+  testPath("foo/bar.h")));
 }
 
 static tooling::CompileCommand cmd(StringRef File, StringRef Arg) {
@@ -88,7 +91,7 @@
   EXPECT_EQ(CDB.getCompileCommand(testPath("bar.cc")), Override);
 
   EXPECT_THAT(CDB.getFallbackCommand(testPath("foo.cc")).CommandLine,
-  ElementsAre("clang", testPath("foo.cc"), "-DA=6"));
+  ElementsAre(EndsWith("clang"), testPath("foo.cc"), "-DA=6"));
 }
 
 TEST_F(OverlayCDBTest, Watch) {
Index: clangd/GlobalCompilationDatabase.cpp
===
--- clangd/

[PATCH] D56066: [OpenCL] Address space for default class members

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: lib/Sema/SemaInit.cpp:4539
+  if (InitCategory.isPRValue() || InitCategory.isXValue())
+T1Quals.removeAddressSpace();
+

rjmccall wrote:
> rjmccall wrote:
> > I can understand why a pr-value wouldn't have an address space, but an 
> > x-value's address space is surely important.
> No, wait, I think this is more deeply wrong.  We're initializing a reference 
> to type `cv1 T1`, and if we're initializing it with a temporary, we're 
> dropping the address space from `cv1`?  That's definitely wrong.
> 
> If we're starting with a pr-value, reference-initialization normally has to 
> start by initializing a temporary.  (The exception is it's a class type with 
> a conversion operator to a reference type; C++ is abysmally complicated.  
> Let's ignore this for a moment.)  Temporaries are always in the default 
> address space (the address space of local variables) because the language 
> (neither OpenCL nor Embedded C) does not give us any facility for allocating 
> temporary memory anywhere else.  So reference-initialization from a pr-value 
> creates a temporary in the default address space and then attempts to 
> initialize the destination reference type with that temporary.  That 
> initialization should fail if there's no conversion from the default address 
> space to the destination address space.  For example, consider initializing a 
> `const int __global &` from a pr-value: this should clearly not succeed, 
> because we have no way of putting a temporary in the global address space.  
> That reference can only be initialized with a gl-value that's already in the 
> `__global` address space.
> 
> On the other hand, we can successfully initialize a `const int &` with a 
> gl-value of type `const int __global` not by direct reference initialization 
> but by loading from the operand and then materializing the result into a new 
> temporary.
> 
> I think what this means is:
> 
> - We need to be doing this checking as if pr-values were in `__private`.  
> This includes both (1) expressions that were originally pr-values and (2) 
> expressions that have been converted to pr-values due to a failure to perform 
> direct reference initialization.
> 
> - We need to make sure that reference compatibility correctly prevents 
> references from being bound to gl-values in incompatible address spaces.
> On the other hand, we can successfully initialize a `const int &` with a 
> gl-value of type `const int __global` not by direct reference initialization 
> but by loading from the operand and then materializing the result into a new 
> temporary.

Is this the right thing to do? It means that initializing such a reference 
would incur a cost under the hood that might be unnoticed/unwanted by the user. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56066/new/

https://reviews.llvm.org/D56066



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

[PATCH] D56343: [clang-tidy] Refactor: Compose Method on check_clang_tidy.py

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: test/clang-tidy/check_clang_tidy.py:103
 
+  return (has_check_fixes, has_check_messages, has_check_notes, \
+check_fixes_prefixes, check_messages_prefixes, check_notes_prefixes)

Such a long list of returned values make me wonder if it would be worth 
creating a NamedTuple / class to hold them.
This may make the call site more readable too.



Comment at: test/clang-tidy/check_clang_tidy.py:141
+   '-check-prefixes=' + ','.join(check_notes_prefixes),
+   '-implicit-check-not={{note|warning|error}}:'])
+  return

These three `check_*` function all use the same `'FileCheck', '-']` 
pattern. Maybe it's worth syndicating that to a try_run_filecheck(input_file0, 
input_file1, *extra_args)` function.



Comment at: test/clang-tidy/check_clang_tidy.py:178
+check_fixes_prefixes, check_messages_prefixes, check_notes_prefixes = \
+get_prefixes(args.check_suffix, input_text)
+

This is the verbose call site I was pointing to above.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56343/new/

https://reviews.llvm.org/D56343



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

Re: [clang-tools-extra] r349496 - [clangd] BackgroundIndex rebuilds symbol index periodically.

[Diana Picus via cfe-commits]

Hi Eric,

This is still failing intermittently on AArch64 in spite of your and
Ilya's timeout increases. Could you please revert and rework this
test?

Thanks,
Diana

http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld/builds/5872
http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld/builds/5878
http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld/builds/5879

On Tue, 18 Dec 2018 at 16:42, Eric Liu via cfe-commits
 wrote:
>
> Author: ioeric
> Date: Tue Dec 18 07:39:33 2018
> New Revision: 349496
>
> URL: http://llvm.org/viewvc/llvm-project?rev=349496&view=rev
> Log:
> [clangd] BackgroundIndex rebuilds symbol index periodically.
>
> Summary:
> Currently, background index rebuilds symbol index on every indexed file,
> which can be inefficient. This patch makes it only rebuild symbol index 
> periodically.
> As the rebuild no longer happens too often, we could also build more efficient
> dex index.
>
> Reviewers: ilya-biryukov, kadircet
>
> Reviewed By: kadircet
>
> Subscribers: dblaikie, MaskRay, jkorous, arphaman, jfb, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D55770
>
> Modified:
> clang-tools-extra/trunk/clangd/ClangdServer.cpp
> clang-tools-extra/trunk/clangd/ClangdServer.h
> clang-tools-extra/trunk/clangd/index/Background.cpp
> clang-tools-extra/trunk/clangd/index/Background.h
> clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
> clang-tools-extra/trunk/test/clangd/background-index.test
> clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
>
> Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=349496&r1=349495&r2=349496&view=diff
> ==
> --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
> +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Dec 18 07:39:33 2018
> @@ -139,7 +139,8 @@ ClangdServer::ClangdServer(const GlobalC
>if (Opts.BackgroundIndex) {
>  BackgroundIdx = llvm::make_unique(
>  Context::current().clone(), ResourceDir, FSProvider, CDB,
> -BackgroundIndexStorage::createDiskBackedStorageFactory());
> +BackgroundIndexStorage::createDiskBackedStorageFactory(),
> +Opts.BackgroundIndexRebuildPeriodMs);
>  AddIndex(BackgroundIdx.get());
>}
>if (DynamicIdx)
>
> Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=349496&r1=349495&r2=349496&view=diff
> ==
> --- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
> +++ clang-tools-extra/trunk/clangd/ClangdServer.h Tue Dec 18 07:39:33 2018
> @@ -85,6 +85,10 @@ public:
>  /// If true, ClangdServer automatically indexes files in the current 
> project
>  /// on background threads. The index is stored in the project root.
>  bool BackgroundIndex = false;
> +/// If set to non-zero, the background index rebuilds the symbol index
> +/// periodically every BuildIndexPeriodMs milliseconds; otherwise, the
> +/// symbol index will be updated for each indexed file.
> +size_t BackgroundIndexRebuildPeriodMs = 0;
>
>  /// If set, use this index to augment code completion results.
>  SymbolIndex *StaticIndex = nullptr;
>
> Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=349496&r1=349495&r2=349496&view=diff
> ==
> --- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
> +++ clang-tools-extra/trunk/clangd/index/Background.cpp Tue Dec 18 07:39:33 
> 2018
> @@ -27,11 +27,13 @@
>  #include "llvm/ADT/StringRef.h"
>  #include "llvm/Support/SHA1.h"
>
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>
>  using namespace llvm;
>  namespace clang {
> @@ -125,10 +127,13 @@ createFileFilter(const llvm::StringMap  BackgroundIndex::BackgroundIndex(
>  Context BackgroundContext, StringRef ResourceDir,
>  const FileSystemProvider &FSProvider, const GlobalCompilationDatabase 
> &CDB,
> -BackgroundIndexStorage::Factory IndexStorageFactory, size_t 
> ThreadPoolSize)
> +BackgroundIndexStorage::Factory IndexStorageFactory,
> +size_t BuildIndexPeriodMs, size_t ThreadPoolSize)
>  : SwapIndex(make_unique()), ResourceDir(ResourceDir),
>FSProvider(FSProvider), CDB(CDB),
>BackgroundContext(std::move(BackgroundContext)),
> +  BuildIndexPeriodMs(BuildIndexPeriodMs),
> +  SymbolsUpdatedSinceLastIndex(false),
>IndexStorageFactory(std::move(IndexStorageFactory)),
>CommandsChanged(
>CDB.watch([&](const std::vector &ChangedFiles) {
> @@ -138,6 +143,11

[PATCH] D56365: [X86] Use funnel shift intrinsics for the VBMI2 vshld/vshrd builtins.

[Simon Pilgrim via Phabricator via cfe-commits]

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

LGTM in conjunction with the fast-isel changes in D56377 



Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56365/new/

https://reviews.llvm.org/D56365



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

[clang-tools-extra] r350512 - [clangd] Disable BackgroundIndexTest.PeriodicalIndex

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Mon Jan  7 03:18:11 2019
New Revision: 350512

URL: http://llvm.org/viewvc/llvm-project?rev=350512&view=rev
Log:
[clangd] Disable BackgroundIndexTest.PeriodicalIndex

It sometimes fails on AArch64.

Modified:
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp?rev=350512&r1=350511&r2=350512&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp Mon Jan  
7 03:18:11 2019
@@ -243,7 +243,9 @@ TEST_F(BackgroundIndexTest, DirectInclud
   EmptyIncludeNode());
 }
 
-TEST_F(BackgroundIndexTest, PeriodicalIndex) {
+// FIXME: figure out the right timeouts or rewrite to not use the timeouts and
+// re-enable.
+TEST_F(BackgroundIndexTest, DISABLED_PeriodicalIndex) {
   MockFSProvider FS;
   llvm::StringMap Storage;
   size_t CacheHits = 0;


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

Re: [clang-tools-extra] r349496 - [clangd] BackgroundIndex rebuilds symbol index periodically.

[Ilya Biryukov via cfe-commits]

Temporarily disabled the test in r350512.

On Mon, Jan 7, 2019 at 10:54 AM Diana Picus  wrote:

> Hi Eric,
>
> This is still failing intermittently on AArch64 in spite of your and
> Ilya's timeout increases. Could you please revert and rework this
> test?
>
> Thanks,
> Diana
>
> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld/builds/5872
> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld/builds/5878
> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld/builds/5879
>
> On Tue, 18 Dec 2018 at 16:42, Eric Liu via cfe-commits
>  wrote:
> >
> > Author: ioeric
> > Date: Tue Dec 18 07:39:33 2018
> > New Revision: 349496
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=349496&view=rev
> > Log:
> > [clangd] BackgroundIndex rebuilds symbol index periodically.
> >
> > Summary:
> > Currently, background index rebuilds symbol index on every indexed file,
> > which can be inefficient. This patch makes it only rebuild symbol index
> periodically.
> > As the rebuild no longer happens too often, we could also build more
> efficient
> > dex index.
> >
> > Reviewers: ilya-biryukov, kadircet
> >
> > Reviewed By: kadircet
> >
> > Subscribers: dblaikie, MaskRay, jkorous, arphaman, jfb, cfe-commits
> >
> > Differential Revision: https://reviews.llvm.org/D55770
> >
> > Modified:
> > clang-tools-extra/trunk/clangd/ClangdServer.cpp
> > clang-tools-extra/trunk/clangd/ClangdServer.h
> > clang-tools-extra/trunk/clangd/index/Background.cpp
> > clang-tools-extra/trunk/clangd/index/Background.h
> > clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
> > clang-tools-extra/trunk/test/clangd/background-index.test
> > clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
> >
> > Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=349496&r1=349495&r2=349496&view=diff
> >
> ==
> > --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
> > +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Dec 18 07:39:33
> 2018
> > @@ -139,7 +139,8 @@ ClangdServer::ClangdServer(const GlobalC
> >if (Opts.BackgroundIndex) {
> >  BackgroundIdx = llvm::make_unique(
> >  Context::current().clone(), ResourceDir, FSProvider, CDB,
> > -BackgroundIndexStorage::createDiskBackedStorageFactory());
> > +BackgroundIndexStorage::createDiskBackedStorageFactory(),
> > +Opts.BackgroundIndexRebuildPeriodMs);
> >  AddIndex(BackgroundIdx.get());
> >}
> >if (DynamicIdx)
> >
> > Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
> > URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=349496&r1=349495&r2=349496&view=diff
> >
> ==
> > --- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
> > +++ clang-tools-extra/trunk/clangd/ClangdServer.h Tue Dec 18 07:39:33
> 2018
> > @@ -85,6 +85,10 @@ public:
> >  /// If true, ClangdServer automatically indexes files in the
> current project
> >  /// on background threads. The index is stored in the project root.
> >  bool BackgroundIndex = false;
> > +/// If set to non-zero, the background index rebuilds the symbol
> index
> > +/// periodically every BuildIndexPeriodMs milliseconds; otherwise,
> the
> > +/// symbol index will be updated for each indexed file.
> > +size_t BackgroundIndexRebuildPeriodMs = 0;
> >
> >  /// If set, use this index to augment code completion results.
> >  SymbolIndex *StaticIndex = nullptr;
> >
> > Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=349496&r1=349495&r2=349496&view=diff
> >
> ==
> > --- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
> > +++ clang-tools-extra/trunk/clangd/index/Background.cpp Tue Dec 18
> 07:39:33 2018
> > @@ -27,11 +27,13 @@
> >  #include "llvm/ADT/StringRef.h"
> >  #include "llvm/Support/SHA1.h"
> >
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  using namespace llvm;
> >  namespace clang {
> > @@ -125,10 +127,13 @@ createFileFilter(const llvm::StringMap >  BackgroundIndex::BackgroundIndex(
> >  Context BackgroundContext, StringRef ResourceDir,
> >  const FileSystemProvider &FSProvider, const
> GlobalCompilationDatabase &CDB,
> > -BackgroundIndexStorage::Factory IndexStorageFactory, size_t
> ThreadPoolSize)
> > +BackgroundIndexStorage::Factory IndexStorageFactory,
> > +size_t BuildIndexPeriodMs, size_t ThreadPoolSize)
> >  : SwapIndex(make_unique()), ResourceDir(ResourceDir),
> >FSProvider(FSProvider), CDB(CDB),
> >   

[PATCH] D56380: [clangd] Fix a regression issue caused by r348365.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56380/new/

https://reviews.llvm.org/D56380



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

[PATCH] D56314: [clangd] Don't store completion info if the symbol is not used for code completion.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

@hokein, do you need reviewers for this? I'm happy to volunteer.

In D56314#1347511 , @nridge wrote:

> Might we want to keep some of this information for `workspace/symbol`? I 
> mean, surely not "documentation", but perhaps "signature" and "return type"?


There's nothing stopping us from reintroducing this information if we start 
doing the same. I don't foresee difficulties with this. It would be easier to 
figure out the bits we actually need when we implement this functionality.
But I generally prefer to move fast and fix things as you go, even if that 
means going back and forth, others might disagree.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56314/new/

https://reviews.llvm.org/D56314



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

[PATCH] D53072: [clang-format] Introduce the flag which allows not to shrink lines

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

In D53072#1342363 , @djasper wrote:

> I don't quite understand. What you are describing should already be the 
> behavior with ColumnLimit=0 and I think your test should pass without the new 
> option. Doesn't it?


As far as I see how it works column limit does not prevent the shrinking 
behavior.

In D53072#1342353 , @krasimir wrote:

> In D53072#1268883 , @yvvan wrote:
>
> > Do you know the better way to accomplish my aim than adding an option to 
> > libFormat? For example making a dependent library which serves a little 
> > different purpose than libFormat itself or something simpler?
>
>
> I don't know about the specific use-case, but in general a fail-safe way to 
> force a line break in C++ code is to add an empty line comment on the 
> preceding line.


This is close but will not work for the case when more than two lines are 
packed together. So it requires to put the empty comment on the each line 
ending which is not too nice (if i'm not missing something)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53072/new/

https://reviews.llvm.org/D53072



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

[PATCH] D55212: Handle alloc_size attribute on function pointers

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

I don't see an easy way of fixing the pragma clang attribute support for this. 
Would it be okay to commit this change anyway?
Since `alloc_size` is only used for very few functions I'd be very surprised if 
there's any existing code that relies on using `#pragma clang atrribute` with 
`alloc_size`.

By the way GCC now also supports the attribute on function pointers: 
https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=267158


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55212/new/

https://reviews.llvm.org/D55212



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

[PATCH] D56385: clang-format: [JS] support goog.requireType.

[Martin Probst via Phabricator via cfe-commits]

mprobst created this revision.
mprobst added a reviewer: krasimir.

It's a new primitive for importing symbols, and should be treated like
the (previously handled) `goog.require` and `goog.forwardDeclare`.


Repository:
  rC Clang

https://reviews.llvm.org/D56385

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestJS.cpp


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -600,6 +600,8 @@
getGoogleJSStyleWithColumns(40));
   verifyFormat("var long = goog.require('this.is.really.absurdly.long');",
getGoogleJSStyleWithColumns(40));
+  verifyFormat("const X = goog.requireType('this.is.really.absurdly.long');",
+   getGoogleJSStyleWithColumns(40));
   verifyFormat("goog.forwardDeclare('this.is.really.absurdly.long');",
getGoogleJSStyleWithColumns(40));
 
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1123,6 +1123,7 @@
(Tok.Next->Next->TokenText == "module" ||
 Tok.Next->Next->TokenText == "provide" ||
 Tok.Next->Next->TokenText == "require" ||
+Tok.Next->Next->TokenText == "requireType" ||
 Tok.Next->Next->TokenText == "forwardDeclare") &&
Tok.Next->Next->Next && Tok.Next->Next->Next->is(tok::l_paren);
   }


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -600,6 +600,8 @@
getGoogleJSStyleWithColumns(40));
   verifyFormat("var long = goog.require('this.is.really.absurdly.long');",
getGoogleJSStyleWithColumns(40));
+  verifyFormat("const X = goog.requireType('this.is.really.absurdly.long');",
+   getGoogleJSStyleWithColumns(40));
   verifyFormat("goog.forwardDeclare('this.is.really.absurdly.long');",
getGoogleJSStyleWithColumns(40));
 
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1123,6 +1123,7 @@
(Tok.Next->Next->TokenText == "module" ||
 Tok.Next->Next->TokenText == "provide" ||
 Tok.Next->Next->TokenText == "require" ||
+Tok.Next->Next->TokenText == "requireType" ||
 Tok.Next->Next->TokenText == "forwardDeclare") &&
Tok.Next->Next->Next && Tok.Next->Next->Next->is(tok::l_paren);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54309: [AST] Allow limiting the scope of common AST traversals (getParents, RAV).

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D54309#1320628 , @JonasToth wrote:

> this patch "broke" `ExprMutAnalyzer`, at least it creates an assertion 
> failure that seems harmless. Your thought on it would be great!


As mentioned on the bug, this assertion already existed and this patch tries to 
preserve it in as many cases as possible.
You may be uncovering some unrelated bug (the kind the assertion was meant to 
catch). Does the assertion go away by reverting this patch (and keeping the old 
version of the assert)?

I've tried to check myself, but I can't use either of your reproducers. I'll 
try the one from the bug next (https://bugs.llvm.org/show_bug.cgi?id=39949)

> The assertion in: `ASTMatchFinder.cpp +680` is triggered in the Analysis to 
> figure out if something is mutated, because `Parents` is empty and the 
> traversal scope is the TU itself, but the node is a `CXXConstructoDecl`. This 
> does not happen for alle cases though, please take a look at this reproducer: 
> https://godbolt.org/z/0yOq2I
> 
> Only for the Lambda-Case problems occur. If I `#if 0` the assertion out the 
> issue goes away, and the real world code that triggered this behaviour is not 
> bothered, too.

Oops, took me a while to realize this is a test for a check that hasn't landed 
:-)
The fact that this only triggers for a lambda case does suggest to me it's 
unrelated to this patch - those are exactly the sort of cases where apparently 
AST traversal has been incomplete in the past, yielding a partial parent map. 
My understanding is this tends to be harmless where it actually fires, but 
points to an AST inconsistency that is likely to cause problems in other cases. 
Thus there's pushback against removing the assertion (which I initially 
attempted to do).

> I am really puzzled why this issue occurs, but as you implemented the 
> restrictions it would like to hear your opinion on the issue. If you want to 
> reproduce yourself I am of course happy to help, but 
>  
> https://github.com/JonasToth/clang/blob/fix_crash/unittests/Analysis/ExprMutationAnalyzerTest.cpp#L1134
>is probably the easiest way to do so (just add this to trunk and run 
> `check-clang-unit`).

Is this still current? The code fails to parse (on 1137 `a&&` should be `T&&`). 
When I fix that the `Results11 = match(withEnclosingCompound(...))` yields an 
empty vector. (The test does subsequently crash, but not in an interesting way).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54309/new/

https://reviews.llvm.org/D54309



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

Re: [clang-tools-extra] r349496 - [clangd] BackgroundIndex rebuilds symbol index periodically.

[Diana Picus via cfe-commits]

Thanks, Ilya!

On Mon, 7 Jan 2019 at 12:22, Ilya Biryukov  wrote:
>
> Temporarily disabled the test in r350512.
>
> On Mon, Jan 7, 2019 at 10:54 AM Diana Picus  wrote:
>>
>> Hi Eric,
>>
>> This is still failing intermittently on AArch64 in spite of your and
>> Ilya's timeout increases. Could you please revert and rework this
>> test?
>>
>> Thanks,
>> Diana
>>
>> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld/builds/5872
>> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld/builds/5878
>> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld/builds/5879
>>
>> On Tue, 18 Dec 2018 at 16:42, Eric Liu via cfe-commits
>>  wrote:
>> >
>> > Author: ioeric
>> > Date: Tue Dec 18 07:39:33 2018
>> > New Revision: 349496
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=349496&view=rev
>> > Log:
>> > [clangd] BackgroundIndex rebuilds symbol index periodically.
>> >
>> > Summary:
>> > Currently, background index rebuilds symbol index on every indexed file,
>> > which can be inefficient. This patch makes it only rebuild symbol index 
>> > periodically.
>> > As the rebuild no longer happens too often, we could also build more 
>> > efficient
>> > dex index.
>> >
>> > Reviewers: ilya-biryukov, kadircet
>> >
>> > Reviewed By: kadircet
>> >
>> > Subscribers: dblaikie, MaskRay, jkorous, arphaman, jfb, cfe-commits
>> >
>> > Differential Revision: https://reviews.llvm.org/D55770
>> >
>> > Modified:
>> > clang-tools-extra/trunk/clangd/ClangdServer.cpp
>> > clang-tools-extra/trunk/clangd/ClangdServer.h
>> > clang-tools-extra/trunk/clangd/index/Background.cpp
>> > clang-tools-extra/trunk/clangd/index/Background.h
>> > clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
>> > clang-tools-extra/trunk/test/clangd/background-index.test
>> > clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
>> >
>> > Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
>> > URL: 
>> > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=349496&r1=349495&r2=349496&view=diff
>> > ==
>> > --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
>> > +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Dec 18 07:39:33 
>> > 2018
>> > @@ -139,7 +139,8 @@ ClangdServer::ClangdServer(const GlobalC
>> >if (Opts.BackgroundIndex) {
>> >  BackgroundIdx = llvm::make_unique(
>> >  Context::current().clone(), ResourceDir, FSProvider, CDB,
>> > -BackgroundIndexStorage::createDiskBackedStorageFactory());
>> > +BackgroundIndexStorage::createDiskBackedStorageFactory(),
>> > +Opts.BackgroundIndexRebuildPeriodMs);
>> >  AddIndex(BackgroundIdx.get());
>> >}
>> >if (DynamicIdx)
>> >
>> > Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
>> > URL: 
>> > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=349496&r1=349495&r2=349496&view=diff
>> > ==
>> > --- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
>> > +++ clang-tools-extra/trunk/clangd/ClangdServer.h Tue Dec 18 07:39:33 2018
>> > @@ -85,6 +85,10 @@ public:
>> >  /// If true, ClangdServer automatically indexes files in the current 
>> > project
>> >  /// on background threads. The index is stored in the project root.
>> >  bool BackgroundIndex = false;
>> > +/// If set to non-zero, the background index rebuilds the symbol index
>> > +/// periodically every BuildIndexPeriodMs milliseconds; otherwise, the
>> > +/// symbol index will be updated for each indexed file.
>> > +size_t BackgroundIndexRebuildPeriodMs = 0;
>> >
>> >  /// If set, use this index to augment code completion results.
>> >  SymbolIndex *StaticIndex = nullptr;
>> >
>> > Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
>> > URL: 
>> > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=349496&r1=349495&r2=349496&view=diff
>> > ==
>> > --- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
>> > +++ clang-tools-extra/trunk/clangd/index/Background.cpp Tue Dec 18 
>> > 07:39:33 2018
>> > @@ -27,11 +27,13 @@
>> >  #include "llvm/ADT/StringRef.h"
>> >  #include "llvm/Support/SHA1.h"
>> >
>> > +#include 
>> >  #include 
>> >  #include 
>> >  #include 
>> >  #include 
>> >  #include 
>> > +#include 
>> >
>> >  using namespace llvm;
>> >  namespace clang {
>> > @@ -125,10 +127,13 @@ createFileFilter(const llvm::StringMap> >  BackgroundIndex::BackgroundIndex(
>> >  Context BackgroundContext, StringRef ResourceDir,
>> >  const FileSystemProvider &FSProvider, const GlobalCompilationDatabase 
>> > &CDB,
>> > -BackgroundIndexStorage::Factory IndexStorageFactory, size_t 
>> > ThreadPoolSize)
>> > +BackgroundIndexSto

[PATCH] D56380: [clangd] Fix a regression issue caused by r348365.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE350515: [clangd] Fix a regression issue caused by r348365. 
(authored by hokein, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D56380?vs=180447&id=180460#toc

Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56380/new/

https://reviews.llvm.org/D56380

Files:
  clangd/GlobalCompilationDatabase.cpp
  unittests/clangd/GlobalCompilationDatabaseTests.cpp


Index: unittests/clangd/GlobalCompilationDatabaseTests.cpp
===
--- unittests/clangd/GlobalCompilationDatabaseTests.cpp
+++ unittests/clangd/GlobalCompilationDatabaseTests.cpp
@@ -19,18 +19,21 @@
 namespace clangd {
 namespace {
 using ::testing::ElementsAre;
+using ::testing::EndsWith;
 
 TEST(GlobalCompilationDatabaseTest, FallbackCommand) {
   DirectoryBasedGlobalCompilationDatabase DB(None);
   auto Cmd = DB.getFallbackCommand(testPath("foo/bar.cc"));
   EXPECT_EQ(Cmd.Directory, testPath("foo"));
-  EXPECT_THAT(Cmd.CommandLine, ElementsAre("clang", testPath("foo/bar.cc")));
+  EXPECT_THAT(Cmd.CommandLine, ElementsAre(
+EndsWith("clang"), testPath("foo/bar.cc")));
   EXPECT_EQ(Cmd.Output, "");
 
   // .h files have unknown language, so they are parsed liberally as obj-c++.
   Cmd = DB.getFallbackCommand(testPath("foo/bar.h"));
-  EXPECT_THAT(Cmd.CommandLine, ElementsAre("clang", "-xobjective-c++-header",
-   testPath("foo/bar.h")));
+  EXPECT_THAT(Cmd.CommandLine,
+  ElementsAre(EndsWith("clang"), "-xobjective-c++-header",
+  testPath("foo/bar.h")));
 }
 
 static tooling::CompileCommand cmd(StringRef File, StringRef Arg) {
@@ -88,7 +91,7 @@
   EXPECT_EQ(CDB.getCompileCommand(testPath("bar.cc")), Override);
 
   EXPECT_THAT(CDB.getFallbackCommand(testPath("foo.cc")).CommandLine,
-  ElementsAre("clang", testPath("foo.cc"), "-DA=6"));
+  ElementsAre(EndsWith("clang"), testPath("foo.cc"), "-DA=6"));
 }
 
 TEST_F(OverlayCDBTest, Watch) {
Index: clangd/GlobalCompilationDatabase.cpp
===
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -17,9 +17,19 @@
 namespace clang {
 namespace clangd {
 
+static std::string getFallbackClangPath() {
+  static int Dummy;
+  std::string ClangdExecutable =
+  llvm::sys::fs::getMainExecutable("clangd", (void *)&Dummy);
+  SmallString<128> ClangPath;
+  ClangPath = llvm::sys::path::parent_path(ClangdExecutable);
+  llvm::sys::path::append(ClangPath, "clang");
+  return ClangPath.str();
+}
+
 tooling::CompileCommand
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
-  std::vector Argv = {"clang"};
+  std::vector Argv = {getFallbackClangPath()};
   // Clang treats .h files as C by default, resulting in unhelpful diagnostics.
   // Parsing as Objective C++ is friendly to more cases.
   if (sys::path::extension(File) == ".h")


Index: unittests/clangd/GlobalCompilationDatabaseTests.cpp
===
--- unittests/clangd/GlobalCompilationDatabaseTests.cpp
+++ unittests/clangd/GlobalCompilationDatabaseTests.cpp
@@ -19,18 +19,21 @@
 namespace clangd {
 namespace {
 using ::testing::ElementsAre;
+using ::testing::EndsWith;
 
 TEST(GlobalCompilationDatabaseTest, FallbackCommand) {
   DirectoryBasedGlobalCompilationDatabase DB(None);
   auto Cmd = DB.getFallbackCommand(testPath("foo/bar.cc"));
   EXPECT_EQ(Cmd.Directory, testPath("foo"));
-  EXPECT_THAT(Cmd.CommandLine, ElementsAre("clang", testPath("foo/bar.cc")));
+  EXPECT_THAT(Cmd.CommandLine, ElementsAre(
+EndsWith("clang"), testPath("foo/bar.cc")));
   EXPECT_EQ(Cmd.Output, "");
 
   // .h files have unknown language, so they are parsed liberally as obj-c++.
   Cmd = DB.getFallbackCommand(testPath("foo/bar.h"));
-  EXPECT_THAT(Cmd.CommandLine, ElementsAre("clang", "-xobjective-c++-header",
-   testPath("foo/bar.h")));
+  EXPECT_THAT(Cmd.CommandLine,
+  ElementsAre(EndsWith("clang"), "-xobjective-c++-header",
+  testPath("foo/bar.h")));
 }
 
 static tooling::CompileCommand cmd(StringRef File, StringRef Arg) {
@@ -88,7 +91,7 @@
   EXPECT_EQ(CDB.getCompileCommand(testPath("bar.cc")), Override);
 
   EXPECT_THAT(CDB.getFallbackCommand(testPath("foo.cc")).CommandLine,
-  ElementsAre("clang", testPath("foo.cc"), "-DA=6"));
+  ElementsAre(EndsWith("clang"), testPath("foo.cc"), "-DA=6"));
 }
 
 TEST_F(OverlayCDBTest, Watch) {
Index: clangd/GlobalCompilationDatabase.cpp
===
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -17,9 +17,19 @@
 namespace clang {
 namespace clangd {
 
+static std::string g

[clang-tools-extra] r350515 - [clangd] Fix a regression issue caused by r348365.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Mon Jan  7 04:35:02 2019
New Revision: 350515

URL: http://llvm.org/viewvc/llvm-project?rev=350515&view=rev
Log:
[clangd] Fix a regression issue caused by r348365.

Summary:
With r348365, we now detect libc++ dir using the actual compiler path
(from the compilation command), rather than the resource-dir.

This new behavior will cause clangd couldn't find libc++ dir (even the libc++ is
built from the source) when using a fallback compilation command (`clang xxx`)

The fix is to use `/clang` as the actual compiler path.

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp

Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp?rev=350515&r1=350514&r2=350515&view=diff
==
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp (original)
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp Mon Jan  7 
04:35:02 2019
@@ -17,9 +17,19 @@ using namespace llvm;
 namespace clang {
 namespace clangd {
 
+static std::string getFallbackClangPath() {
+  static int Dummy;
+  std::string ClangdExecutable =
+  llvm::sys::fs::getMainExecutable("clangd", (void *)&Dummy);
+  SmallString<128> ClangPath;
+  ClangPath = llvm::sys::path::parent_path(ClangdExecutable);
+  llvm::sys::path::append(ClangPath, "clang");
+  return ClangPath.str();
+}
+
 tooling::CompileCommand
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
-  std::vector Argv = {"clang"};
+  std::vector Argv = {getFallbackClangPath()};
   // Clang treats .h files as C by default, resulting in unhelpful diagnostics.
   // Parsing as Objective C++ is friendly to more cases.
   if (sys::path::extension(File) == ".h")

Modified: 
clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp?rev=350515&r1=350514&r2=350515&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp 
(original)
+++ clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp 
Mon Jan  7 04:35:02 2019
@@ -19,18 +19,21 @@ namespace clang {
 namespace clangd {
 namespace {
 using ::testing::ElementsAre;
+using ::testing::EndsWith;
 
 TEST(GlobalCompilationDatabaseTest, FallbackCommand) {
   DirectoryBasedGlobalCompilationDatabase DB(None);
   auto Cmd = DB.getFallbackCommand(testPath("foo/bar.cc"));
   EXPECT_EQ(Cmd.Directory, testPath("foo"));
-  EXPECT_THAT(Cmd.CommandLine, ElementsAre("clang", testPath("foo/bar.cc")));
+  EXPECT_THAT(Cmd.CommandLine, ElementsAre(
+EndsWith("clang"), testPath("foo/bar.cc")));
   EXPECT_EQ(Cmd.Output, "");
 
   // .h files have unknown language, so they are parsed liberally as obj-c++.
   Cmd = DB.getFallbackCommand(testPath("foo/bar.h"));
-  EXPECT_THAT(Cmd.CommandLine, ElementsAre("clang", "-xobjective-c++-header",
-   testPath("foo/bar.h")));
+  EXPECT_THAT(Cmd.CommandLine,
+  ElementsAre(EndsWith("clang"), "-xobjective-c++-header",
+  testPath("foo/bar.h")));
 }
 
 static tooling::CompileCommand cmd(StringRef File, StringRef Arg) {
@@ -88,7 +91,7 @@ TEST_F(OverlayCDBTest, NoBase) {
   EXPECT_EQ(CDB.getCompileCommand(testPath("bar.cc")), Override);
 
   EXPECT_THAT(CDB.getFallbackCommand(testPath("foo.cc")).CommandLine,
-  ElementsAre("clang", testPath("foo.cc"), "-DA=6"));
+  ElementsAre(EndsWith("clang"), testPath("foo.cc"), "-DA=6"));
 }
 
 TEST_F(OverlayCDBTest, Watch) {


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

[PATCH] D53072: [clang-format] Introduce the flag which allows not to shrink lines

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

  $ cat /tmp/test.cc
  int foo(int a,
 int b) {
f();
}
  
  $ clang-format -style="{ColumnLimit: 0}" /tmp/test.cc
  int foo(int a,
  int b) {
f();
  }

Is this not what you want? If so, in what way?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53072/new/

https://reviews.llvm.org/D53072



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

[PATCH] D53072: [clang-format] Introduce the flag which allows not to shrink lines

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

In D53072#1348033 , @djasper wrote:

>   $ cat /tmp/test.cc
>   int foo(int a,
>  int b) {
> f();
> }
>   
>   $ clang-format -style="{ColumnLimit: 0}" /tmp/test.cc
>   int foo(int a,
>   int b) {
> f();
>   }
>   
>
> Is this not what you want? If so, in what way?


Ok, this might mean that my test is bad and does not test what I want (there 
are more cases which are not formatted the way I want with ColumnLimit: 0). But 
since the patch is never to be submitted I don't see a reason to improve it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53072/new/

https://reviews.llvm.org/D53072



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

[PATCH] D53072: [clang-format] Introduce the flag which allows not to shrink lines

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

Without understanding this in more detail I do not know how to move forward 
with this. What this patch is describing is what should already be the case 
with ColumnLimit set to zero. If it isn't this might be a bug or there might be 
a different way to move forward. However, the flag as is does not make sense to 
me without more information.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53072/new/

https://reviews.llvm.org/D53072



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

[PATCH] D53072: [clang-format] Introduce the flag which allows not to shrink lines

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

@djasper
Ok, if there's a possible way to go forward I will find time to provide a 
better test which behaves differently depending on this check. Also my current 
patch is a bit bigger than this version.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53072/new/

https://reviews.llvm.org/D53072



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

[PATCH] D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 180464.
Anastasia added a comment.

Added `hasMethodTypeQualifiers` method.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55948/new/

https://reviews.llvm.org/D55948

Files:
  include/clang/Sema/DeclSpec.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaType.cpp

Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -724,12 +724,8 @@
   /*NumArgs=*/0,
   /*EllipsisLoc=*/NoLoc,
   /*RParenLoc=*/NoLoc,
-  /*TypeQuals=*/0,
   /*RefQualifierIsLvalueRef=*/true,
   /*RefQualifierLoc=*/NoLoc,
-  /*ConstQualifierLoc=*/NoLoc,
-  /*VolatileQualifierLoc=*/NoLoc,
-  /*RestrictQualifierLoc=*/NoLoc,
   /*MutableLoc=*/NoLoc, EST_None,
   /*ESpecRange=*/SourceRange(),
   /*Exceptions=*/nullptr,
@@ -737,8 +733,7 @@
   /*NumExceptions=*/0,
   /*NoexceptExpr=*/nullptr,
   /*ExceptionSpecTokens=*/nullptr,
-  /*DeclsInPrototype=*/None,
-  loc, loc, declarator));
+  /*DeclsInPrototype=*/None, loc, loc, declarator));
 
   // For consistency, make sure the state still has us as processing
   // the decl spec.
@@ -4460,7 +4455,8 @@
   // does not have a K&R-style identifier list), then the arguments are part
   // of the type, otherwise the argument list is ().
   const DeclaratorChunk::FunctionTypeInfo &FTI = DeclType.Fun;
-  IsQualifiedFunction = FTI.TypeQuals || FTI.hasRefQualifier();
+  IsQualifiedFunction =
+  FTI.hasMethodTypeQualifiers() || FTI.hasRefQualifier();
 
   // Check for auto functions and trailing return type and adjust the
   // return type accordingly.
@@ -4698,7 +4694,9 @@
 EPI.ExtInfo = EI;
 EPI.Variadic = FTI.isVariadic;
 EPI.HasTrailingReturn = FTI.hasTrailingReturnType();
-EPI.TypeQuals.addCVRUQualifiers(FTI.TypeQuals);
+EPI.TypeQuals.addCVRUQualifiers(
+FTI.MethodQualifiers ? FTI.MethodQualifiers->getTypeQualifiers()
+ : 0);
 EPI.RefQualifier = !FTI.hasRefQualifier()? RQ_None
 : FTI.RefQualifierIsLValueRef? RQ_LValue
 : RQ_RValue;
@@ -5024,14 +5022,15 @@
 SmallVector RemovalLocs;
 const DeclaratorChunk &Chunk = D.getTypeObject(I);
 assert(Chunk.Kind == DeclaratorChunk::Function);
+
 if (Chunk.Fun.hasRefQualifier())
   RemovalLocs.push_back(Chunk.Fun.getRefQualifierLoc());
-if (Chunk.Fun.TypeQuals & Qualifiers::Const)
-  RemovalLocs.push_back(Chunk.Fun.getConstQualifierLoc());
-if (Chunk.Fun.TypeQuals & Qualifiers::Volatile)
-  RemovalLocs.push_back(Chunk.Fun.getVolatileQualifierLoc());
-if (Chunk.Fun.TypeQuals & Qualifiers::Restrict)
-  RemovalLocs.push_back(Chunk.Fun.getRestrictQualifierLoc());
+
+if (Chunk.Fun.hasMethodTypeQualifiers())
+  Chunk.Fun.MethodQualifiers->forEachQualifier(
+  [&](DeclSpec::TQ TypeQual, StringRef QualName,
+  SourceLocation SL) { RemovalLocs.push_back(SL); });
+
 if (!RemovalLocs.empty()) {
   llvm::sort(RemovalLocs,
  BeforeThanCompare(S.getSourceManager()));
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -884,8 +884,10 @@
 //   This function call operator is declared const (9.3.1) if and only if
 //   the lambda-expression's parameter-declaration-clause is not followed
 //   by mutable. It is neither virtual nor declared volatile. [...]
-if (!FTI.hasMutableQualifier())
-  FTI.TypeQuals |= DeclSpec::TQ_const;
+if (!FTI.hasMutableQualifier()) {
+  FTI.getOrCreateMethodQualifiers().SetTypeQual(DeclSpec::TQ_const,
+SourceLocation());
+}
 
 MethodTyInfo = GetTypeForDeclarator(ParamInfo, CurScope);
 assert(MethodTyInfo && "no type from lambda-declarator");
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -8172,16 +8172,12 @@
   }
 
   DeclaratorChunk::FunctionTypeInfo &FTI = D.getFunctionTypeInfo();
-  if (FTI.TypeQuals != 0) {
-if (FTI.TypeQuals & Qualifiers::Const)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "const" << SourceRange(D.getIdentifierLoc());
-if (FTI.TypeQuals & Qualifiers::Volatile)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "volatile" << SourceRange(D.getIdentifierLoc());
-if (FTI.TypeQuals & Qualifiers::Restrict)
-  Diag(D.getIdentif

[PATCH] D56358: [AST] Move back BasePathSize to the bit-fields of CastExpr

[Bruno Ricci via Phabricator via cfe-commits]

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



Comment at: include/clang/AST/Expr.h:3029
+assert((CastExprBits.BasePathSize == BasePathSize) &&
+   "BasePathSize overflow!");
 assert(CastConsistency());

lebedev.ri wrote:
> rjmccall wrote:
> > riccibruno wrote:
> > > lebedev.ri wrote:
> > > > riccibruno wrote:
> > > > > It cannot overflow now, but someone in the future is going
> > > > > to shrink `CastExprBitfields::BasePathSize`.
> > > > Can you move that static assert here instead of deleting it?
> > > I am not sure it is really useful since when someone will want to
> > > shrink it, it will be stored as `unsigned BasePathSite : x;`. What we
> > > don't want is having `x` too small. However `numeric_limits` will
> > > be based on the underlying type of the bit-field and so will never fail.
> > > 
> > > Storing it as something other than an unsigned, say a short, would work,
> > > but it will pack poorly with MSVC.
> > > 
> > > Hopefully the comment in `CastExprBitfields` will tell people to not 
> > > shrink it
> > > mindlessly.
> > I agree that the type-based numeric limit is not useful if we're 
> > anticipating
> > it becoming a bit-field.  The comment seems fine since it's in the code that
> > would need to be edited.
> I'm just worried that now there will be less of a test coverage,
> The actual run-time test already does not test template instantiation with 
> depth of `16384`, but much less.
Would it make sense to go systematically through the limits recommended by
[implimits], and for each limit devise a test which will exercise it ?

Or would this take too long to be a test ?

It would perhaps also be nice to document these limits somewhere,
even if it is just "You can rely on [implimits]".


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56358/new/

https://reviews.llvm.org/D56358



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

[PATCH] D56358: [AST] Move back BasePathSize to the bit-fields of CastExpr

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: include/clang/AST/Expr.h:3029
+assert((CastExprBits.BasePathSize == BasePathSize) &&
+   "BasePathSize overflow!");
 assert(CastConsistency());

riccibruno wrote:
> lebedev.ri wrote:
> > rjmccall wrote:
> > > riccibruno wrote:
> > > > lebedev.ri wrote:
> > > > > riccibruno wrote:
> > > > > > It cannot overflow now, but someone in the future is going
> > > > > > to shrink `CastExprBitfields::BasePathSize`.
> > > > > Can you move that static assert here instead of deleting it?
> > > > I am not sure it is really useful since when someone will want to
> > > > shrink it, it will be stored as `unsigned BasePathSite : x;`. What we
> > > > don't want is having `x` too small. However `numeric_limits` will
> > > > be based on the underlying type of the bit-field and so will never fail.
> > > > 
> > > > Storing it as something other than an unsigned, say a short, would work,
> > > > but it will pack poorly with MSVC.
> > > > 
> > > > Hopefully the comment in `CastExprBitfields` will tell people to not 
> > > > shrink it
> > > > mindlessly.
> > > I agree that the type-based numeric limit is not useful if we're 
> > > anticipating
> > > it becoming a bit-field.  The comment seems fine since it's in the code 
> > > that
> > > would need to be edited.
> > I'm just worried that now there will be less of a test coverage,
> > The actual run-time test already does not test template instantiation with 
> > depth of `16384`, but much less.
> Would it make sense to go systematically through the limits recommended by
> [implimits], and for each limit devise a test which will exercise it ?
> 
> Or would this take too long to be a test ?
> 
> It would perhaps also be nice to document these limits somewhere,
> even if it is just "You can rely on [implimits]".
> Would it make sense to go systematically through the limits recommended by
> [implimits], and for each limit devise a test which will exercise it ?

Better test coverage is always good.

> Or would this take too long to be a test ?

The problem is, even this you can't really test with a run-time test.
Template instantiation is recursive, so in most environments you overflow
your stack long before you reach the recommended depth of `16384`.



Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56358/new/

https://reviews.llvm.org/D56358



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

r350516 - clang-format: [JS] support goog.requireType.

[Martin Probst via cfe-commits]

Author: mprobst
Date: Mon Jan  7 05:12:50 2019
New Revision: 350516

URL: http://llvm.org/viewvc/llvm-project?rev=350516&view=rev
Log:
clang-format: [JS] support goog.requireType.

Summary:
It's a new primitive for importing symbols, and should be treated like
the (previously handled) `goog.require` and `goog.forwardDeclare`.

Reviewers: krasimir

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=350516&r1=350515&r2=350516&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Jan  7 05:12:50 2019
@@ -1123,6 +1123,7 @@ private:
(Tok.Next->Next->TokenText == "module" ||
 Tok.Next->Next->TokenText == "provide" ||
 Tok.Next->Next->TokenText == "require" ||
+Tok.Next->Next->TokenText == "requireType" ||
 Tok.Next->Next->TokenText == "forwardDeclare") &&
Tok.Next->Next->Next && Tok.Next->Next->Next->is(tok::l_paren);
   }

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=350516&r1=350515&r2=350516&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Mon Jan  7 05:12:50 2019
@@ -600,6 +600,8 @@ TEST_F(FormatTestJS, GoogModules) {
getGoogleJSStyleWithColumns(40));
   verifyFormat("var long = goog.require('this.is.really.absurdly.long');",
getGoogleJSStyleWithColumns(40));
+  verifyFormat("const X = goog.requireType('this.is.really.absurdly.long');",
+   getGoogleJSStyleWithColumns(40));
   verifyFormat("goog.forwardDeclare('this.is.really.absurdly.long');",
getGoogleJSStyleWithColumns(40));
 


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

[PATCH] D56367: [AST] Pack CXXDependentScopeMemberExpr

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D56367#1347845 , @rjmccall wrote:

> Does the regression disappear if you make the scope specifier the first 
> trailing object?  Later trailing objects are more expensive to access, and I 
> would imagine that the scope specifier and template arguments are more 
> important than the other fields.


That was with the scope specifier first. I would imagine that putting it last 
would be indeed be more expensive.
I can leave a TODO here saying that it could be in principle be stored as a 
trailing object, but the that the run-time effect
of doing so should be investigated.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56367/new/

https://reviews.llvm.org/D56367



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

[PATCH] D56385: clang-format: [JS] support goog.requireType.

[Martin Probst via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350516: clang-format: [JS] support goog.requireType. 
(authored by mprobst, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56385/new/

https://reviews.llvm.org/D56385

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTestJS.cpp


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -1123,6 +1123,7 @@
(Tok.Next->Next->TokenText == "module" ||
 Tok.Next->Next->TokenText == "provide" ||
 Tok.Next->Next->TokenText == "require" ||
+Tok.Next->Next->TokenText == "requireType" ||
 Tok.Next->Next->TokenText == "forwardDeclare") &&
Tok.Next->Next->Next && Tok.Next->Next->Next->is(tok::l_paren);
   }
Index: cfe/trunk/unittests/Format/FormatTestJS.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJS.cpp
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp
@@ -600,6 +600,8 @@
getGoogleJSStyleWithColumns(40));
   verifyFormat("var long = goog.require('this.is.really.absurdly.long');",
getGoogleJSStyleWithColumns(40));
+  verifyFormat("const X = goog.requireType('this.is.really.absurdly.long');",
+   getGoogleJSStyleWithColumns(40));
   verifyFormat("goog.forwardDeclare('this.is.really.absurdly.long');",
getGoogleJSStyleWithColumns(40));
 


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -1123,6 +1123,7 @@
(Tok.Next->Next->TokenText == "module" ||
 Tok.Next->Next->TokenText == "provide" ||
 Tok.Next->Next->TokenText == "require" ||
+Tok.Next->Next->TokenText == "requireType" ||
 Tok.Next->Next->TokenText == "forwardDeclare") &&
Tok.Next->Next->Next && Tok.Next->Next->Next->is(tok::l_paren);
   }
Index: cfe/trunk/unittests/Format/FormatTestJS.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJS.cpp
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp
@@ -600,6 +600,8 @@
getGoogleJSStyleWithColumns(40));
   verifyFormat("var long = goog.require('this.is.really.absurdly.long');",
getGoogleJSStyleWithColumns(40));
+  verifyFormat("const X = goog.requireType('this.is.really.absurdly.long');",
+   getGoogleJSStyleWithColumns(40));
   verifyFormat("goog.forwardDeclare('this.is.really.absurdly.long');",
getGoogleJSStyleWithColumns(40));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56358: [AST] Move back BasePathSize to the bit-fields of CastExpr

[Bruno Ricci via Phabricator via cfe-commits]

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



Comment at: include/clang/AST/Expr.h:3029
+assert((CastExprBits.BasePathSize == BasePathSize) &&
+   "BasePathSize overflow!");
 assert(CastConsistency());

lebedev.ri wrote:
> riccibruno wrote:
> > lebedev.ri wrote:
> > > rjmccall wrote:
> > > > riccibruno wrote:
> > > > > lebedev.ri wrote:
> > > > > > riccibruno wrote:
> > > > > > > It cannot overflow now, but someone in the future is going
> > > > > > > to shrink `CastExprBitfields::BasePathSize`.
> > > > > > Can you move that static assert here instead of deleting it?
> > > > > I am not sure it is really useful since when someone will want to
> > > > > shrink it, it will be stored as `unsigned BasePathSite : x;`. What we
> > > > > don't want is having `x` too small. However `numeric_limits` will
> > > > > be based on the underlying type of the bit-field and so will never 
> > > > > fail.
> > > > > 
> > > > > Storing it as something other than an unsigned, say a short, would 
> > > > > work,
> > > > > but it will pack poorly with MSVC.
> > > > > 
> > > > > Hopefully the comment in `CastExprBitfields` will tell people to not 
> > > > > shrink it
> > > > > mindlessly.
> > > > I agree that the type-based numeric limit is not useful if we're 
> > > > anticipating
> > > > it becoming a bit-field.  The comment seems fine since it's in the code 
> > > > that
> > > > would need to be edited.
> > > I'm just worried that now there will be less of a test coverage,
> > > The actual run-time test already does not test template instantiation 
> > > with depth of `16384`, but much less.
> > Would it make sense to go systematically through the limits recommended by
> > [implimits], and for each limit devise a test which will exercise it ?
> > 
> > Or would this take too long to be a test ?
> > 
> > It would perhaps also be nice to document these limits somewhere,
> > even if it is just "You can rely on [implimits]".
> > Would it make sense to go systematically through the limits recommended by
> > [implimits], and for each limit devise a test which will exercise it ?
> 
> Better test coverage is always good.
> 
> > Or would this take too long to be a test ?
> 
> The problem is, even this you can't really test with a run-time test.
> Template instantiation is recursive, so in most environments you overflow
> your stack long before you reach the recommended depth of `16384`.
> 
But you can perhaps test this without doing any template instantiation.
The limit is "Direct and indirect base classes [16384]". Therefore you can
just construct 2^16 classes with macros which should not be recursive,
and then somehow construct a class with all of these classes as a base
(although this step is perhaps recursive).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56358/new/

https://reviews.llvm.org/D56358



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

r350519 - [AST][NFC] Pack OpaqueValueExpr

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Mon Jan  7 05:39:26 2019
New Revision: 350519

URL: http://llvm.org/viewvc/llvm-project?rev=350519&view=rev
Log:
[AST][NFC] Pack OpaqueValueExpr

Use the newly available space in the bit-fields of Stmt.
This saves 1 pointer per OpaqueValueExpr. NFC.


Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=350519&r1=350518&r2=350519&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Mon Jan  7 05:39:26 2019
@@ -945,7 +945,6 @@ public:
 class OpaqueValueExpr : public Expr {
   friend class ASTStmtReader;
   Expr *SourceExpr;
-  SourceLocation Loc;
 
 public:
   OpaqueValueExpr(SourceLocation Loc, QualType T, ExprValueKind VK,
@@ -959,8 +958,9 @@ public:
T->isInstantiationDependentType() ||
(SourceExpr && SourceExpr->isInstantiationDependent()),
false),
-  SourceExpr(SourceExpr), Loc(Loc) {
+  SourceExpr(SourceExpr) {
 setIsUnique(false);
+OpaqueValueExprBits.Loc = Loc;
   }
 
   /// Given an expression which invokes a copy constructor --- i.e.  a
@@ -969,20 +969,19 @@ public:
   static const OpaqueValueExpr *findInCopyConstruct(const Expr *expr);
 
   explicit OpaqueValueExpr(EmptyShell Empty)
-: Expr(OpaqueValueExprClass, Empty) { }
+: Expr(OpaqueValueExprClass, Empty) {}
 
   /// Retrieve the location of this expression.
-  SourceLocation getLocation() const { return Loc; }
+  SourceLocation getLocation() const { return OpaqueValueExprBits.Loc; }
 
   SourceLocation getBeginLoc() const LLVM_READONLY {
-return SourceExpr ? SourceExpr->getBeginLoc() : Loc;
+return SourceExpr ? SourceExpr->getBeginLoc() : getLocation();
   }
   SourceLocation getEndLoc() const LLVM_READONLY {
-return SourceExpr ? SourceExpr->getEndLoc() : Loc;
+return SourceExpr ? SourceExpr->getEndLoc() : getLocation();
   }
   SourceLocation getExprLoc() const LLVM_READONLY {
-if (SourceExpr) return SourceExpr->getExprLoc();
-return Loc;
+return SourceExpr ? SourceExpr->getExprLoc() : getLocation();
   }
 
   child_range children() {

Modified: cfe/trunk/include/clang/AST/Stmt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=350519&r1=350518&r2=350519&view=diff
==
--- cfe/trunk/include/clang/AST/Stmt.h (original)
+++ cfe/trunk/include/clang/AST/Stmt.h Mon Jan  7 05:39:26 2019
@@ -706,13 +706,16 @@ protected:
   //===--- Clang Extensions bitfields classes ---===//
 
   class OpaqueValueExprBitfields {
+friend class ASTStmtReader;
 friend class OpaqueValueExpr;
 
 unsigned : NumExprBits;
 
-/// The OVE is a unique semantic reference to its source expressio if this
+/// The OVE is a unique semantic reference to its source expression if this
 /// bit is set to true.
 unsigned IsUnique : 1;
+
+SourceLocation Loc;
   };
 
   union {

Modified: cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderStmt.cpp?rev=350519&r1=350518&r2=350519&view=diff
==
--- cfe/trunk/lib/Serialization/ASTReaderStmt.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderStmt.cpp Mon Jan  7 05:39:26 2019
@@ -1773,7 +1773,7 @@ void ASTStmtReader::VisitCXXFoldExpr(CXX
 void ASTStmtReader::VisitOpaqueValueExpr(OpaqueValueExpr *E) {
   VisitExpr(E);
   E->SourceExpr = Record.readSubExpr();
-  E->Loc = ReadSourceLocation();
+  E->OpaqueValueExprBits.Loc = ReadSourceLocation();
   E->setIsUnique(Record.readInt());
 }
 


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

[PATCH] D56314: [clangd] Don't store completion info if the symbol is not used for code completion.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D56314#1347964 , @ilya-biryukov 
wrote:

> @hokein, do you need reviewers for this? I'm happy to volunteer.


Thanks.

> In D56314#1347511 , @nridge wrote:
> 
>> Might we want to keep some of this information for `workspace/symbol`? I 
>> mean, surely not "documentation", but perhaps "signature" and "return type"?
> 
> 
> There's nothing stopping us from reintroducing this information if we start 
> doing the same. I don't foresee difficulties with this. It would be easier to 
> figure out the bits we actually need when we implement this functionality.
>  But I generally prefer to move fast and fix things as you go, even if that 
> means going back and forth, others might disagree.

+1, we don't use the `signature` and `return type` in the `workspace/symbol` at 
the moment. We could revisit it when we actually need them.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56314/new/

https://reviews.llvm.org/D56314



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

[PATCH] D56354: [AST] Replace asserts with if() in SourceLocation accessors

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The downside to this change is that callers now have to check the return values 
for validity where they didn't previously because the assertion covered it. 
However, I think that's probably reasonable in these cases since they're mostly 
returning source locations or ranges.




Comment at: include/clang/AST/DeclarationName.h:733
+if (Name.getNameKind() != DeclarationName::CXXConstructorName &&
+   Name.getNameKind() != DeclarationName::CXXDestructorName &&
+   Name.getNameKind() != DeclarationName::CXXConversionFunctionName)

Formatting is incorrect here and elsewhere -- you should run the patch through 
clang-format.



Comment at: include/clang/AST/DeclarationName.h:735
+   Name.getNameKind() != DeclarationName::CXXConversionFunctionName)
+  return nullptr;
 return LocInfo.NamedType.TInfo;

Did you investigate the callers of this function to see which ones need a null 
pointer check inserted into them? Otherwise, this change turns an assertion 
into harder to track down UB. (I'm less worried about the other changes because 
those will fail more gracefully.)



Comment at: lib/AST/NestedNameSpecifier.cpp:465
 TypeLoc NestedNameSpecifierLoc::getTypeLoc() const {
-  assert((Qualifier->getKind() == NestedNameSpecifier::TypeSpec ||
-  Qualifier->getKind() == NestedNameSpecifier::TypeSpecWithTemplate) &&
- "Nested-name-specifier location is not a type");
+  if ((Qualifier->getKind() != NestedNameSpecifier::TypeSpec &&
+  Qualifier->getKind() != NestedNameSpecifier::TypeSpecWithTemplate))

Remove spurious parens.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56354/new/

https://reviews.llvm.org/D56354



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

[PATCH] D54309: [AST] Allow limiting the scope of common AST traversals (getParents, RAV).

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

@JonasToth As noted on https://bugs.llvm.org/show_bug.cgi?id=39949, this 
assertion is a pre-existing problem with the parent map + lambdas, unrelated to 
this patch.

I've analyzed where it comes from (details in the bug) but don't know what the 
right fix should be. @klimek, can you take a look at the bug?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54309/new/

https://reviews.llvm.org/D54309



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

[PATCH] D55280: [CTU] Make loadExternalAST return with non nullptr on success

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC350521: [CTU] Make loadExternalAST return with non nullptr 
on success (authored by martong, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55280?vs=176779&id=180479#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55280/new/

https://reviews.llvm.org/D55280

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp


Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -208,9 +208,6 @@
   if (!ASTUnitOrError)
 return ASTUnitOrError.takeError();
   ASTUnit *Unit = *ASTUnitOrError;
-  if (!Unit)
-return llvm::make_error(
-index_error_code::failed_to_get_external_ast);
   assert(&Unit->getFileManager() ==
  &Unit->getASTContext().getSourceManager().getFileManager());
 
@@ -324,6 +321,9 @@
   } else {
 Unit = FnUnitCacheEntry->second;
   }
+  if (!Unit)
+return llvm::make_error(
+index_error_code::failed_to_get_external_ast);
   return Unit;
 }
 
Index: include/clang/CrossTU/CrossTranslationUnit.h
===
--- include/clang/CrossTU/CrossTranslationUnit.h
+++ include/clang/CrossTU/CrossTranslationUnit.h
@@ -130,8 +130,9 @@
   /// \p IndexName. In case the declaration is found in the index the
   /// corresponding AST file will be loaded.
   ///
-  /// \return Returns an ASTUnit that contains the definition of the looked up
-  /// function.
+  /// \return Returns a pointer to the ASTUnit that contains the definition of
+  /// the looked up function or an Error.
+  /// The returned pointer is never a nullptr.
   ///
   /// Note that the AST files should also be in the \p CrossTUDir.
   llvm::Expected loadExternalAST(StringRef LookupName,


Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -208,9 +208,6 @@
   if (!ASTUnitOrError)
 return ASTUnitOrError.takeError();
   ASTUnit *Unit = *ASTUnitOrError;
-  if (!Unit)
-return llvm::make_error(
-index_error_code::failed_to_get_external_ast);
   assert(&Unit->getFileManager() ==
  &Unit->getASTContext().getSourceManager().getFileManager());
 
@@ -324,6 +321,9 @@
   } else {
 Unit = FnUnitCacheEntry->second;
   }
+  if (!Unit)
+return llvm::make_error(
+index_error_code::failed_to_get_external_ast);
   return Unit;
 }
 
Index: include/clang/CrossTU/CrossTranslationUnit.h
===
--- include/clang/CrossTU/CrossTranslationUnit.h
+++ include/clang/CrossTU/CrossTranslationUnit.h
@@ -130,8 +130,9 @@
   /// \p IndexName. In case the declaration is found in the index the
   /// corresponding AST file will be loaded.
   ///
-  /// \return Returns an ASTUnit that contains the definition of the looked up
-  /// function.
+  /// \return Returns a pointer to the ASTUnit that contains the definition of
+  /// the looked up function or an Error.
+  /// The returned pointer is never a nullptr.
   ///
   /// Note that the AST files should also be in the \p CrossTUDir.
   llvm::Expected loadExternalAST(StringRef LookupName,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r350521 - [CTU] Make loadExternalAST return with non nullptr on success

[Gabor Marton via cfe-commits]

Author: martong
Date: Mon Jan  7 06:05:19 2019
New Revision: 350521

URL: http://llvm.org/viewvc/llvm-project?rev=350521&view=rev
Log:
[CTU] Make loadExternalAST return with non nullptr on success

Summary:
In loadExternalAST we return with either an error or with a valid
ASTUnit pointer which should not be a nullptr.
This prevents in the call site any superfluous check for being a nullptr.

Reviewers: xazax.hun, a_sidorin, Szelethus, balazske

Subscribers: rnkovacs, dkrupp, gamesh411, cfe-commits

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

Modified:
cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp

Modified: cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h?rev=350521&r1=350520&r2=350521&view=diff
==
--- cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h (original)
+++ cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h Mon Jan  7 06:05:19 
2019
@@ -130,8 +130,9 @@ public:
   /// \p IndexName. In case the declaration is found in the index the
   /// corresponding AST file will be loaded.
   ///
-  /// \return Returns an ASTUnit that contains the definition of the looked up
-  /// function.
+  /// \return Returns a pointer to the ASTUnit that contains the definition of
+  /// the looked up function or an Error.
+  /// The returned pointer is never a nullptr.
   ///
   /// Note that the AST files should also be in the \p CrossTUDir.
   llvm::Expected loadExternalAST(StringRef LookupName,

Modified: cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp?rev=350521&r1=350520&r2=350521&view=diff
==
--- cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp (original)
+++ cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp Mon Jan  7 06:05:19 2019
@@ -208,9 +208,6 @@ CrossTranslationUnitContext::getCrossTUD
   if (!ASTUnitOrError)
 return ASTUnitOrError.takeError();
   ASTUnit *Unit = *ASTUnitOrError;
-  if (!Unit)
-return llvm::make_error(
-index_error_code::failed_to_get_external_ast);
   assert(&Unit->getFileManager() ==
  &Unit->getASTContext().getSourceManager().getFileManager());
 
@@ -324,6 +321,9 @@ llvm::Expected CrossTranslati
   } else {
 Unit = FnUnitCacheEntry->second;
   }
+  if (!Unit)
+return llvm::make_error(
+index_error_code::failed_to_get_external_ast);
   return Unit;
 }
 


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

[PATCH] D56288: [ELF] Do not enable 'new dtags' on NetBSD by default

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

mgorny added a comment.

Ok, maybe I'm being silly but since clang driver has to pass 
`--enable-new-dtags` for GNU ld compatibility anyway, wouldn't it make sense to 
keep the default as disabled in order to match GNU ld behavior?


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56288/new/

https://reviews.llvm.org/D56288



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

r350523 - [clang] Add AST matcher for initializer list members

[Hyrum Wright via cfe-commits]

Author: hwright
Date: Mon Jan  7 06:14:36 2019
New Revision: 350523

URL: http://llvm.org/viewvc/llvm-project?rev=350523&view=rev
Log:
[clang] Add AST matcher for initializer list members

Summary:
Much like hasArg for various call expressions, this allows LibTooling users to
match against a member of an initializer list.

This is currently being used as part of the abseil-duration-scale clang-tidy
check.

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

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=350523&r1=350522&r2=350523&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Mon Jan  7 06:14:36 2019
@@ -718,7 +718,7 @@ Example matches a || b
 
 
 MatcherStmt>blockExprMatcherBlockExpr>...
-MAtches a reference to a 
block.
+Matches a reference to a 
block.
 
 Example: matches "^{}":
   void f() { ^{}(); }
@@ -5866,6 +5866,15 @@ FIXME: Unit test this matcher
 
 
 
+MatcherInitListExpr>hasInitunsigned N, 
ast_matchers::MatcherExpr> 
InnerMatcher
+Matches the n'th item of an 
initializer list expression.
+
+Example matches y.
+(matcher = initListExpr(hasInit(0, expr(
+  int x{y}.
+
+
+
 MatcherInitListExpr>hasSyntacticFormMatcherExpr> 
InnerMatcher
 Matches the 
syntactic form of init list expressions
 (if expression have it).

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=350523&r1=350522&r2=350523&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Mon Jan  7 06:14:36 2019
@@ -3514,6 +3514,19 @@ AST_POLYMORPHIC_MATCHER_P2(hasArgument,
   *Node.getArg(N)->IgnoreParenImpCasts(), Finder, Builder));
 }
 
+/// Matches the n'th item of an initializer list expression.
+///
+/// Example matches y.
+/// (matcher = initListExpr(hasInit(0, expr(
+/// \code
+///   int x{y}.
+/// \endcode
+AST_MATCHER_P2(InitListExpr, hasInit, unsigned, N,
+   ast_matchers::internal::Matcher, InnerMatcher) {
+  return N < Node.getNumInits() &&
+  InnerMatcher.matches(*Node.getInit(N), Finder, Builder);
+}
+
 /// Matches declaration statements that contain a specific number of
 /// declarations.
 ///

Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=350523&r1=350522&r2=350523&view=diff
==
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Mon Jan  7 06:14:36 2019
@@ -273,6 +273,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(hasInClassInitializer);
   REGISTER_MATCHER(hasIncrement);
   REGISTER_MATCHER(hasIndex);
+  REGISTER_MATCHER(hasInit);
   REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp?rev=350523&r1=350522&r2=350523&view=diff
==
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp Mon Jan  7 
06:14:36 2019
@@ -2255,6 +2255,18 @@ TEST(IsAssignmentOperator, Basic) {
   notMatches("void x() { int a; if(a == 0) return; }", BinAsgmtOperator));
 }
 
+TEST(HasInit, Basic) {
+  EXPECT_TRUE(
+matches("int x{0};",
+initListExpr(hasInit(0, expr();
+  EXPECT_FALSE(
+matches("int x{0};",
+initListExpr(hasInit(1, expr();
+  EXPECT_FALSE(
+matches("int x;",
+initListExpr(hasInit(0, expr();
+}
+
 TEST(Matcher, isMain) {
   EXPECT_TRUE(
 matches("int main() {}", functionDecl(isMain(;


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

[PATCH] D56090: Add a matcher for members of an initializer list expression

[Hyrum Wright via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350523: [clang] Add AST matcher for initializer list members 
(authored by hwright, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56090?vs=180248&id=180481#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56090/new/

https://reviews.llvm.org/D56090

Files:
  cfe/trunk/docs/LibASTMatchersReference.html
  cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
  cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
  cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp


Index: cfe/trunk/docs/LibASTMatchersReference.html
===
--- cfe/trunk/docs/LibASTMatchersReference.html
+++ cfe/trunk/docs/LibASTMatchersReference.html
@@ -718,7 +718,7 @@
 
 
 MatcherStmt>blockExprMatcherBlockExpr>...
-MAtches a reference to a 
block.
+Matches a reference to a 
block.
 
 Example: matches "^{}":
   void f() { ^{}(); }
@@ -5866,6 +5866,15 @@
 
 
 
+MatcherInitListExpr>hasInitunsigned N, 
ast_matchers::MatcherExpr> 
InnerMatcher
+Matches the n'th item of an 
initializer list expression.
+
+Example matches y.
+(matcher = initListExpr(hasInit(0, expr(
+  int x{y}.
+
+
+
 MatcherInitListExpr>hasSyntacticFormMatcherExpr> 
InnerMatcher
 Matches the 
syntactic form of init list expressions
 (if expression have it).
Index: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
===
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
@@ -3514,6 +3514,19 @@
   *Node.getArg(N)->IgnoreParenImpCasts(), Finder, Builder));
 }
 
+/// Matches the n'th item of an initializer list expression.
+///
+/// Example matches y.
+/// (matcher = initListExpr(hasInit(0, expr(
+/// \code
+///   int x{y}.
+/// \endcode
+AST_MATCHER_P2(InitListExpr, hasInit, unsigned, N,
+   ast_matchers::internal::Matcher, InnerMatcher) {
+  return N < Node.getNumInits() &&
+  InnerMatcher.matches(*Node.getInit(N), Finder, Builder);
+}
+
 /// Matches declaration statements that contain a specific number of
 /// declarations.
 ///
Index: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -273,6 +273,7 @@
   REGISTER_MATCHER(hasInClassInitializer);
   REGISTER_MATCHER(hasIncrement);
   REGISTER_MATCHER(hasIndex);
+  REGISTER_MATCHER(hasInit);
   REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
Index: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2255,6 +2255,18 @@
   notMatches("void x() { int a; if(a == 0) return; }", BinAsgmtOperator));
 }
 
+TEST(HasInit, Basic) {
+  EXPECT_TRUE(
+matches("int x{0};",
+initListExpr(hasInit(0, expr();
+  EXPECT_FALSE(
+matches("int x{0};",
+initListExpr(hasInit(1, expr();
+  EXPECT_FALSE(
+matches("int x;",
+initListExpr(hasInit(0, expr();
+}
+
 TEST(Matcher, isMain) {
   EXPECT_TRUE(
 matches("int main() {}", functionDecl(isMain(;


Index: cfe/trunk/docs/LibASTMatchersReference.html
===
--- cfe/trunk/docs/LibASTMatchersReference.html
+++ cfe/trunk/docs/LibASTMatchersReference.html
@@ -718,7 +718,7 @@
 
 
 MatcherStmt>blockExprMatcherBlockExpr>...
-MAtches a reference to a block.
+Matches a reference to a block.
 
 Example: matches "^{}":
   void f() { ^{}(); }
@@ -5866,6 +5866,15 @@
 
 
 
+MatcherInitListExpr>hasInitunsigned N, ast_matchers::MatcherExpr> InnerMatcher
+Matches the n'th item of an initializer list expression.
+
+Example matches y.
+(matcher = initListExpr(hasInit(0, expr(
+  int x{y}.
+
+
+
 MatcherInitListExpr>hasSyntacticFormMatcher

[PATCH] D56288: [ELF] Do not enable 'new dtags' on NetBSD by default

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D56288#1348148 , @mgorny wrote:

> Ok, maybe I'm being silly but since clang driver has to pass 
> `--enable-new-dtags` for GNU ld compatibility anyway, wouldn't it make sense 
> to keep the default as disabled in order to match GNU ld behavior?


This would be the best especially in the context of philosophical issues of 
builtin knowledge in LLD and difficulty to resolve it.

Most Linux software doesn't need RPATH/RUNPATH as there is ld-config.. but it's 
completely the opposite on NetBSD where almost everything needs proper RPATH 
setting due to lack of Linux style ld-config.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56288/new/

https://reviews.llvm.org/D56288



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

[PATCH] D56358: [AST] Move back BasePathSize to the bit-fields of CastExpr

[Bruno Ricci via Phabricator via cfe-commits]

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



Comment at: include/clang/AST/Expr.h:3029
+assert((CastExprBits.BasePathSize == BasePathSize) &&
+   "BasePathSize overflow!");
 assert(CastConsistency());

riccibruno wrote:
> lebedev.ri wrote:
> > riccibruno wrote:
> > > lebedev.ri wrote:
> > > > rjmccall wrote:
> > > > > riccibruno wrote:
> > > > > > lebedev.ri wrote:
> > > > > > > riccibruno wrote:
> > > > > > > > It cannot overflow now, but someone in the future is going
> > > > > > > > to shrink `CastExprBitfields::BasePathSize`.
> > > > > > > Can you move that static assert here instead of deleting it?
> > > > > > I am not sure it is really useful since when someone will want to
> > > > > > shrink it, it will be stored as `unsigned BasePathSite : x;`. What 
> > > > > > we
> > > > > > don't want is having `x` too small. However `numeric_limits` will
> > > > > > be based on the underlying type of the bit-field and so will never 
> > > > > > fail.
> > > > > > 
> > > > > > Storing it as something other than an unsigned, say a short, would 
> > > > > > work,
> > > > > > but it will pack poorly with MSVC.
> > > > > > 
> > > > > > Hopefully the comment in `CastExprBitfields` will tell people to 
> > > > > > not shrink it
> > > > > > mindlessly.
> > > > > I agree that the type-based numeric limit is not useful if we're 
> > > > > anticipating
> > > > > it becoming a bit-field.  The comment seems fine since it's in the 
> > > > > code that
> > > > > would need to be edited.
> > > > I'm just worried that now there will be less of a test coverage,
> > > > The actual run-time test already does not test template instantiation 
> > > > with depth of `16384`, but much less.
> > > Would it make sense to go systematically through the limits recommended by
> > > [implimits], and for each limit devise a test which will exercise it ?
> > > 
> > > Or would this take too long to be a test ?
> > > 
> > > It would perhaps also be nice to document these limits somewhere,
> > > even if it is just "You can rely on [implimits]".
> > > Would it make sense to go systematically through the limits recommended by
> > > [implimits], and for each limit devise a test which will exercise it ?
> > 
> > Better test coverage is always good.
> > 
> > > Or would this take too long to be a test ?
> > 
> > The problem is, even this you can't really test with a run-time test.
> > Template instantiation is recursive, so in most environments you overflow
> > your stack long before you reach the recommended depth of `16384`.
> > 
> But you can perhaps test this without doing any template instantiation.
> The limit is "Direct and indirect base classes [16384]". Therefore you can
> just construct 2^16 classes with macros which should not be recursive,
> and then somehow construct a class with all of these classes as a base
> (although this step is perhaps recursive).
I meant obviously 2^14


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56358/new/

https://reviews.llvm.org/D56358



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

r350525 - [AST][NFC] Pack DependentScopeDeclRefExpr and CXXUnresolvedConstructExpr

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Mon Jan  7 06:27:04 2019
New Revision: 350525

URL: http://llvm.org/viewvc/llvm-project?rev=350525&view=rev
Log:
[AST][NFC] Pack DependentScopeDeclRefExpr and CXXUnresolvedConstructExpr

Use the newly available space in the bit-fields of Stmt.
This saves 1 pointer per DependentScopeDeclRefExpr/CXXUnresolvedConstructExpr.

Additionally rename "TypeSourceInfo *Type;" to "TypeSourceInfo *TSI;"
as was done in D56022 (r350003) (but this is an internal detail anyway),
and clang-format both classes. NFC.


Modified:
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/lib/AST/ExprCXX.cpp
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=350525&r1=350524&r2=350525&view=diff
==
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Mon Jan  7 06:27:04 2019
@@ -2936,6 +2936,10 @@ class DependentScopeDeclRefExpr final
   private llvm::TrailingObjects {
+  friend class ASTStmtReader;
+  friend class ASTStmtWriter;
+  friend TrailingObjects;
+
   /// The nested-name-specifier that qualifies this unresolved
   /// declaration name.
   NestedNameSpecifierLoc QualifierLoc;
@@ -2943,32 +2947,26 @@ class DependentScopeDeclRefExpr final
   /// The name of the entity we will be referencing.
   DeclarationNameInfo NameInfo;
 
-  /// Whether the name includes info for explicit template
-  /// keyword and arguments.
-  bool HasTemplateKWAndArgsInfo;
-
-  DependentScopeDeclRefExpr(QualType T,
-NestedNameSpecifierLoc QualifierLoc,
+  DependentScopeDeclRefExpr(QualType Ty, NestedNameSpecifierLoc QualifierLoc,
 SourceLocation TemplateKWLoc,
 const DeclarationNameInfo &NameInfo,
 const TemplateArgumentListInfo *Args);
 
   size_t numTrailingObjects(OverloadToken) const {
-return HasTemplateKWAndArgsInfo ? 1 : 0;
+return hasTemplateKWAndArgsInfo();
   }
 
-public:
-  friend class ASTStmtReader;
-  friend class ASTStmtWriter;
-  friend TrailingObjects;
+  bool hasTemplateKWAndArgsInfo() const {
+return DependentScopeDeclRefExprBits.HasTemplateKWAndArgsInfo;
+  }
 
-  static DependentScopeDeclRefExpr *Create(const ASTContext &C,
-   NestedNameSpecifierLoc QualifierLoc,
-   SourceLocation TemplateKWLoc,
-   const DeclarationNameInfo &NameInfo,
-  const TemplateArgumentListInfo *TemplateArgs);
+public:
+  static DependentScopeDeclRefExpr *
+  Create(const ASTContext &Context, NestedNameSpecifierLoc QualifierLoc,
+ SourceLocation TemplateKWLoc, const DeclarationNameInfo &NameInfo,
+ const TemplateArgumentListInfo *TemplateArgs);
 
-  static DependentScopeDeclRefExpr *CreateEmpty(const ASTContext &C,
+  static DependentScopeDeclRefExpr *CreateEmpty(const ASTContext &Context,
 bool HasTemplateKWAndArgsInfo,
 unsigned NumTemplateArgs);
 
@@ -2996,21 +2994,24 @@ public:
   /// Retrieve the location of the template keyword preceding
   /// this name, if any.
   SourceLocation getTemplateKeywordLoc() const {
-if (!HasTemplateKWAndArgsInfo) return SourceLocation();
+if (!hasTemplateKWAndArgsInfo())
+  return SourceLocation();
 return getTrailingObjects()->TemplateKWLoc;
   }
 
   /// Retrieve the location of the left angle bracket starting the
   /// explicit template argument list following the name, if any.
   SourceLocation getLAngleLoc() const {
-if (!HasTemplateKWAndArgsInfo) return SourceLocation();
+if (!hasTemplateKWAndArgsInfo())
+  return SourceLocation();
 return getTrailingObjects()->LAngleLoc;
   }
 
   /// Retrieve the location of the right angle bracket ending the
   /// explicit template argument list following the name, if any.
   SourceLocation getRAngleLoc() const {
-if (!HasTemplateKWAndArgsInfo) return SourceLocation();
+if (!hasTemplateKWAndArgsInfo())
+  return SourceLocation();
 return getTrailingObjects()->RAngleLoc;
   }
 
@@ -3164,7 +3165,7 @@ class CXXUnresolvedConstructExpr final
   friend TrailingObjects;
 
   /// The type being constructed.
-  TypeSourceInfo *Type = nullptr;
+  TypeSourceInfo *TSI;
 
   /// The location of the left parentheses ('(').
   SourceLocation LParenLoc;
@@ -3172,34 +3173,31 @@ class CXXUnresolvedConstructExpr final
   /// The location of the right parentheses (')').
   SourceLocation RParenLoc;
 
-  /// The number of arguments used to construct the type.
-  unsigned NumArgs;
-
-  CXXUnresolvedConstructEx

[clang-tools-extra] r350526 - [clang-tidy] Use the public hasInit matcher, rather than defining our own, NFC

[Hyrum Wright via cfe-commits]

Author: hwright
Date: Mon Jan  7 06:36:47 2019
New Revision: 350526

URL: http://llvm.org/viewvc/llvm-project?rev=350526&view=rev
Log:
[clang-tidy] Use the public hasInit matcher, rather than defining our own, NFC

Modified:
clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp

Modified: clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp?rev=350526&r1=350525&r2=350526&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp Mon Jan  7 
06:36:47 2019
@@ -104,20 +104,6 @@ llvm::StringRef getFactoryForScale(Durat
   llvm_unreachable("unknown scaling factor");
 }
 
-/// Matches the n'th item of an initializer list expression.
-///
-/// Example matches y.
-/// (matcher = initListExpr(hasInit(0, expr(
-/// \code
-///   int x{y}.
-/// \endcode
-AST_MATCHER_P2(InitListExpr, hasInit, unsigned, N,
-   ast_matchers::internal::Matcher, InnerMatcher) {
-  return N < Node.getNumInits() &&
-  InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder,
-   Builder);
-}
-
 /// Returns `true` if `Node` is a value which evaluates to a literal `0`.
 bool IsLiteralZero(const MatchFinder::MatchResult &Result, const Expr &Node) {
   auto ZeroMatcher =
@@ -132,13 +118,13 @@ bool IsLiteralZero(const MatchFinder::Ma
   // Now check to see if we're using a functional cast with a scalar
   // initializer expression, e.g. `int{0}`.
   if (selectFirst(
-  "val",
-  match(cxxFunctionalCastExpr(
-hasDestinationType(
-anyOf(isInteger(), realFloatingPointType())),
-hasSourceExpression(initListExpr(hasInit(0, ZeroMatcher
-.bind("val"),
-Node, *Result.Context)) != nullptr)
+  "val", match(cxxFunctionalCastExpr(
+   hasDestinationType(
+   anyOf(isInteger(), realFloatingPointType())),
+   hasSourceExpression(initListExpr(
+   hasInit(0, 
ignoringParenImpCasts(ZeroMatcher)
+   .bind("val"),
+   Node, *Result.Context)) != nullptr)
 return true;
 
   return false;


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

r350527 - [AST] Store some data of CXXNewExpr as trailing objects

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Mon Jan  7 07:04:45 2019
New Revision: 350527

URL: http://llvm.org/viewvc/llvm-project?rev=350527&view=rev
Log:
[AST] Store some data of CXXNewExpr as trailing objects

Store the optional array size expression, optional initialization expression
and optional placement new arguments in a trailing array. Additionally store
the range for the parenthesized type-id in a trailing object if needed since
in the vast majority of cases the type is not parenthesized (not a single new
expression in the translation unit of SemaDecl.cpp has a parenthesized type-id).

This saves 2 pointers per CXXNewExpr in all cases, and 2 pointers + 8 bytes
per CXXNewExpr in the common case where the type is not parenthesized.

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

Reviewed By: rjmccall


Modified:
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/lib/AST/ExprCXX.cpp
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=350527&r1=350526&r2=350527&view=diff
==
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Mon Jan  7 07:04:45 2019
@@ -1933,54 +1933,56 @@ public:
 
 /// Represents a new-expression for memory allocation and constructor
 /// calls, e.g: "new CXXNewExpr(foo)".
-class CXXNewExpr : public Expr {
+class CXXNewExpr final
+: public Expr,
+  private llvm::TrailingObjects {
   friend class ASTStmtReader;
   friend class ASTStmtWriter;
-
-  /// Contains an optional array size expression, an optional initialization
-  /// expression, and any number of optional placement arguments, in that 
order.
-  Stmt **SubExprs = nullptr;
+  friend TrailingObjects;
 
   /// Points to the allocation function used.
   FunctionDecl *OperatorNew;
 
-  /// Points to the deallocation function used in case of error. May be
-  /// null.
+  /// Points to the deallocation function used in case of error. May be null.
   FunctionDecl *OperatorDelete;
 
   /// The allocated type-source information, as written in the source.
   TypeSourceInfo *AllocatedTypeInfo;
 
-  /// If the allocated type was expressed as a parenthesized type-id,
-  /// the source range covering the parenthesized type-id.
-  SourceRange TypeIdParens;
-
   /// Range of the entire new expression.
   SourceRange Range;
 
   /// Source-range of a paren-delimited initializer.
   SourceRange DirectInitRange;
 
-  /// Was the usage ::new, i.e. is the global new to be used?
-  unsigned GlobalNew : 1;
-
-  /// Do we allocate an array? If so, the first SubExpr is the size expression.
-  unsigned Array : 1;
+  // CXXNewExpr is followed by several optional trailing objects.
+  // They are in order:
+  //
+  // * An optional "Stmt *" for the array size expression.
+  //Present if and ony if isArray().
+  //
+  // * An optional "Stmt *" for the init expression.
+  //Present if and only if hasInitializer().
+  //
+  // * An array of getNumPlacementArgs() "Stmt *" for the placement new
+  //   arguments, if any.
+  //
+  // * An optional SourceRange for the range covering the parenthesized type-id
+  //if the allocated type was expressed as a parenthesized type-id.
+  //Present if and only if isParenTypeId().
+  unsigned arraySizeOffset() const { return 0; }
+  unsigned initExprOffset() const { return arraySizeOffset() + isArray(); }
+  unsigned placementNewArgsOffset() const {
+return initExprOffset() + hasInitializer();
+  }
 
-  /// Should the alignment be passed to the allocation function?
-  unsigned PassAlignment : 1;
+  unsigned numTrailingObjects(OverloadToken) const {
+return isArray() + hasInitializer() + getNumPlacementArgs();
+  }
 
-  /// If this is an array allocation, does the usual deallocation
-  /// function for the allocated type want to know the allocated size?
-  unsigned UsualArrayDeleteWantsSize : 1;
-
-  /// The number of placement new arguments.
-  unsigned NumPlacementArgs : 26;
-
-  /// What kind of initializer do we have? Could be none, parens, or braces.
-  /// In storage, we distinguish between "none, and no initializer expr", and
-  /// "none, but an implicit initializer expr".
-  unsigned StoredInitializationStyle : 2;
+  unsigned numTrailingObjects(OverloadToken) const {
+return isParenTypeId();
+  }
 
 public:
   enum InitializationStyle {
@@ -1994,18 +1996,35 @@ public:
 ListInit
   };
 
-  CXXNewExpr(const ASTContext &C, bool globalNew, FunctionDecl *operatorNew,
- FunctionDecl *operatorDelete, bool PassAlignment,
- bool usualArrayDeleteWantsSize, ArrayRef placementArgs,
- SourceRange typeIdPare

[PATCH] D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 180487.
r.stahl added a comment.

rebased


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55701/new/

https://reviews.llvm.org/D55701

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Index: unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
===
--- unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -21,16 +21,7 @@
 namespace ento {
 namespace {
 
-class CustomChecker : public Checker {
-public:
-  void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
-BugReporter &BR) const {
-BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
-   "Custom diagnostic description",
-   PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
-  }
-};
-
+template 
 class TestAction : public ASTFrontendAction {
   class DiagConsumer : public PathDiagnosticConsumer {
 llvm::raw_ostream &Output;
@@ -59,23 +50,55 @@
 Compiler.getAnalyzerOpts()->CheckersControlList = {
 {"custom.CustomChecker", true}};
 AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
-  Registry.addChecker("custom.CustomChecker", "Description",
- "");
+  Registry.addChecker("custom.CustomChecker", "Description", "");
 });
 return std::move(AnalysisConsumer);
   }
 };
 
+template 
+bool runCheckerOnCode(const std::string &Code, std::string &Diags) {
+  llvm::raw_string_ostream OS(Diags);
+  return tooling::runToolOnCode(new TestAction(OS), Code);
+}
+template 
+bool runCheckerOnCode(const std::string &Code) {
+  std::string Diags;
+  return runCheckerOnCode(Code, Diags);
+}
+
+
+class CustomChecker : public Checker {
+public:
+  void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
+BugReporter &BR) const {
+BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
+   "Custom diagnostic description",
+   PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
+  }
+};
 
 TEST(RegisterCustomCheckers, RegisterChecker) {
   std::string Diags;
-  {
-llvm::raw_string_ostream OS(Diags);
-EXPECT_TRUE(tooling::runToolOnCode(new TestAction(OS), "void f() {;}"));
-  }
+  EXPECT_TRUE(runCheckerOnCode("void f() {;}", Diags));
   EXPECT_EQ(Diags, "custom.CustomChecker:Custom diagnostic description");
 }
 
+class LocIncDecChecker : public Checker {
+public:
+  void checkLocation(SVal Loc, bool IsLoad, const Stmt *S,
+ CheckerContext &C) const {
+auto UnaryOp = dyn_cast(S);
+if (UnaryOp && !IsLoad)
+  EXPECT_FALSE(UnaryOp->isIncrementOp());
+  }
+};
+
+TEST(RegisterCustomCheckers, CheckLocationIncDec) {
+  EXPECT_TRUE(
+  runCheckerOnCode("void f() { int *p; (*p)++; }"));
+}
+
 }
 }
 }
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -1052,7 +1052,7 @@
   // Perform the store, so that the uninitialized value detection happens.
   Bldr.takeNodes(*I);
   ExplodedNodeSet Dst3;
-  evalStore(Dst3, U, U, *I, state, loc, V2_untested);
+  evalStore(Dst3, U, Ex, *I, state, loc, V2_untested);
   Bldr.addNodes(Dst3);
 
   continue;
@@ -1120,7 +1120,7 @@
 // Perform the store.
 Bldr.takeNodes(*I);
 ExplodedNodeSet Dst3;
-evalStore(Dst3, U, U, *I, state, loc, Result);
+evalStore(Dst3, U, Ex, *I, state, loc, Result);
 Bldr.addNodes(Dst3);
   }
   Dst.insert(Dst2);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56134: [AST] Store some data of CXXNewExpr as trailing objects

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350527: [AST] Store some data of CXXNewExpr as trailing 
objects (authored by brunoricci, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56134?vs=179639&id=180488#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56134/new/

https://reviews.llvm.org/D56134

Files:
  cfe/trunk/include/clang/AST/ExprCXX.h
  cfe/trunk/include/clang/AST/Stmt.h
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/lib/AST/ExprCXX.cpp
  cfe/trunk/lib/CodeGen/CGExprCXX.cpp
  cfe/trunk/lib/Sema/SemaExprCXX.cpp
  cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
  cfe/trunk/lib/Serialization/ASTWriterStmt.cpp

Index: cfe/trunk/include/clang/AST/ExprCXX.h
===
--- cfe/trunk/include/clang/AST/ExprCXX.h
+++ cfe/trunk/include/clang/AST/ExprCXX.h
@@ -1933,54 +1933,56 @@
 
 /// Represents a new-expression for memory allocation and constructor
 /// calls, e.g: "new CXXNewExpr(foo)".
-class CXXNewExpr : public Expr {
+class CXXNewExpr final
+: public Expr,
+  private llvm::TrailingObjects {
   friend class ASTStmtReader;
   friend class ASTStmtWriter;
-
-  /// Contains an optional array size expression, an optional initialization
-  /// expression, and any number of optional placement arguments, in that order.
-  Stmt **SubExprs = nullptr;
+  friend TrailingObjects;
 
   /// Points to the allocation function used.
   FunctionDecl *OperatorNew;
 
-  /// Points to the deallocation function used in case of error. May be
-  /// null.
+  /// Points to the deallocation function used in case of error. May be null.
   FunctionDecl *OperatorDelete;
 
   /// The allocated type-source information, as written in the source.
   TypeSourceInfo *AllocatedTypeInfo;
 
-  /// If the allocated type was expressed as a parenthesized type-id,
-  /// the source range covering the parenthesized type-id.
-  SourceRange TypeIdParens;
-
   /// Range of the entire new expression.
   SourceRange Range;
 
   /// Source-range of a paren-delimited initializer.
   SourceRange DirectInitRange;
 
-  /// Was the usage ::new, i.e. is the global new to be used?
-  unsigned GlobalNew : 1;
-
-  /// Do we allocate an array? If so, the first SubExpr is the size expression.
-  unsigned Array : 1;
+  // CXXNewExpr is followed by several optional trailing objects.
+  // They are in order:
+  //
+  // * An optional "Stmt *" for the array size expression.
+  //Present if and ony if isArray().
+  //
+  // * An optional "Stmt *" for the init expression.
+  //Present if and only if hasInitializer().
+  //
+  // * An array of getNumPlacementArgs() "Stmt *" for the placement new
+  //   arguments, if any.
+  //
+  // * An optional SourceRange for the range covering the parenthesized type-id
+  //if the allocated type was expressed as a parenthesized type-id.
+  //Present if and only if isParenTypeId().
+  unsigned arraySizeOffset() const { return 0; }
+  unsigned initExprOffset() const { return arraySizeOffset() + isArray(); }
+  unsigned placementNewArgsOffset() const {
+return initExprOffset() + hasInitializer();
+  }
 
-  /// Should the alignment be passed to the allocation function?
-  unsigned PassAlignment : 1;
+  unsigned numTrailingObjects(OverloadToken) const {
+return isArray() + hasInitializer() + getNumPlacementArgs();
+  }
 
-  /// If this is an array allocation, does the usual deallocation
-  /// function for the allocated type want to know the allocated size?
-  unsigned UsualArrayDeleteWantsSize : 1;
-
-  /// The number of placement new arguments.
-  unsigned NumPlacementArgs : 26;
-
-  /// What kind of initializer do we have? Could be none, parens, or braces.
-  /// In storage, we distinguish between "none, and no initializer expr", and
-  /// "none, but an implicit initializer expr".
-  unsigned StoredInitializationStyle : 2;
+  unsigned numTrailingObjects(OverloadToken) const {
+return isParenTypeId();
+  }
 
 public:
   enum InitializationStyle {
@@ -1994,18 +1996,35 @@
 ListInit
   };
 
-  CXXNewExpr(const ASTContext &C, bool globalNew, FunctionDecl *operatorNew,
- FunctionDecl *operatorDelete, bool PassAlignment,
- bool usualArrayDeleteWantsSize, ArrayRef placementArgs,
- SourceRange typeIdParens, Expr *arraySize,
- InitializationStyle initializationStyle, Expr *initializer,
- QualType ty, TypeSourceInfo *AllocatedTypeInfo,
- SourceRange Range, SourceRange directInitRange);
-  explicit CXXNewExpr(EmptyShell Shell)
-  : Expr(CXXNewExprClass, Shell) {}
-
-  void AllocateArgsArray(const ASTContext &C, bool isArray,
- unsigned numPlaceArgs, bool hasInitializer);
+private:
+  /// Build a c++ new expression.
+  CXXNewExpr(bool IsGlobalNew, FunctionDecl *OperatorNew,
+ FunctionDecl *Operat

r350528 - [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore

[Rafael Stahl via cfe-commits]

Author: r.stahl
Date: Mon Jan  7 07:07:01 2019
New Revision: 350528

URL: http://llvm.org/viewvc/llvm-project?rev=350528&view=rev
Log:
[analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore

Summary: The LocationE parameter of evalStore is documented as "The location 
expression that is stored to". When storing from an increment / decrement 
operator this was not satisfied. In user code this causes an inconsistency 
between the SVal and Stmt parameters of checkLocation.

Reviewers: NoQ, dcoughlin, george.karpenkov

Reviewed By: NoQ

Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, 
Szelethus, donat.nagy, dkrupp, cfe-commits

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp?rev=350528&r1=350527&r2=350528&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp Mon Jan  7 07:07:01 2019
@@ -1052,7 +1052,7 @@ void ExprEngine::VisitIncrementDecrement
   // Perform the store, so that the uninitialized value detection happens.
   Bldr.takeNodes(*I);
   ExplodedNodeSet Dst3;
-  evalStore(Dst3, U, U, *I, state, loc, V2_untested);
+  evalStore(Dst3, U, Ex, *I, state, loc, V2_untested);
   Bldr.addNodes(Dst3);
 
   continue;
@@ -1120,7 +1120,7 @@ void ExprEngine::VisitIncrementDecrement
 // Perform the store.
 Bldr.takeNodes(*I);
 ExplodedNodeSet Dst3;
-evalStore(Dst3, U, U, *I, state, loc, Result);
+evalStore(Dst3, U, Ex, *I, state, loc, Result);
 Bldr.addNodes(Dst3);
   }
   Dst.insert(Dst2);

Modified: cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp?rev=350528&r1=350527&r2=350528&view=diff
==
--- cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp (original)
+++ cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp Mon Jan  
7 07:07:01 2019
@@ -21,16 +21,7 @@ namespace clang {
 namespace ento {
 namespace {
 
-class CustomChecker : public Checker {
-public:
-  void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
-BugReporter &BR) const {
-BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
-   "Custom diagnostic description",
-   PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
-  }
-};
-
+template 
 class TestAction : public ASTFrontendAction {
   class DiagConsumer : public PathDiagnosticConsumer {
 llvm::raw_ostream &Output;
@@ -59,23 +50,55 @@ public:
 Compiler.getAnalyzerOpts()->CheckersControlList = {
 {"custom.CustomChecker", true}};
 AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
-  Registry.addChecker("custom.CustomChecker", "Description",
- "");
+  Registry.addChecker("custom.CustomChecker", "Description", "");
 });
 return std::move(AnalysisConsumer);
   }
 };
 
+template 
+bool runCheckerOnCode(const std::string &Code, std::string &Diags) {
+  llvm::raw_string_ostream OS(Diags);
+  return tooling::runToolOnCode(new TestAction(OS), Code);
+}
+template 
+bool runCheckerOnCode(const std::string &Code) {
+  std::string Diags;
+  return runCheckerOnCode(Code, Diags);
+}
+
+
+class CustomChecker : public Checker {
+public:
+  void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
+BugReporter &BR) const {
+BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
+   "Custom diagnostic description",
+   PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
+  }
+};
 
 TEST(RegisterCustomCheckers, RegisterChecker) {
   std::string Diags;
-  {
-llvm::raw_string_ostream OS(Diags);
-EXPECT_TRUE(tooling::runToolOnCode(new TestAction(OS), "void f() {;}"));
-  }
+  EXPECT_TRUE(runCheckerOnCode("void f() {;}", Diags));
   EXPECT_EQ(Diags, "custom.CustomChecker:Custom diagnostic description");
 }
 
+class LocIncDecChecker : public Checker {
+public:
+  void checkLocation(SVal Loc, bool IsLoad, const Stmt *S,
+ CheckerContext &C) const {
+auto UnaryOp = dyn_cast(S);
+if (UnaryOp && !IsLoad)
+  EXPECT_FALSE(UnaryOp->isIncrementOp());
+  }
+};
+
+TEST(RegisterCustomCheckers, CheckLocationIncDec) {
+  EXPECT_TRUE(
+  runCheckerOnCode("void f() { int *p; (*p)++; }"));
+}
+
 }
 }
 }


___
cfe

[PATCH] D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore

[Rafael Stahl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC350528: [analyzer] Pass the correct loc Expr from 
VisitIncDecOp to evalStore (authored by r.stahl, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55701?vs=180487&id=180489#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55701/new/

https://reviews.llvm.org/D55701

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -1052,7 +1052,7 @@
   // Perform the store, so that the uninitialized value detection happens.
   Bldr.takeNodes(*I);
   ExplodedNodeSet Dst3;
-  evalStore(Dst3, U, U, *I, state, loc, V2_untested);
+  evalStore(Dst3, U, Ex, *I, state, loc, V2_untested);
   Bldr.addNodes(Dst3);
 
   continue;
@@ -1120,7 +1120,7 @@
 // Perform the store.
 Bldr.takeNodes(*I);
 ExplodedNodeSet Dst3;
-evalStore(Dst3, U, U, *I, state, loc, Result);
+evalStore(Dst3, U, Ex, *I, state, loc, Result);
 Bldr.addNodes(Dst3);
   }
   Dst.insert(Dst2);
Index: unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
===
--- unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -21,16 +21,7 @@
 namespace ento {
 namespace {
 
-class CustomChecker : public Checker {
-public:
-  void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
-BugReporter &BR) const {
-BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
-   "Custom diagnostic description",
-   PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
-  }
-};
-
+template 
 class TestAction : public ASTFrontendAction {
   class DiagConsumer : public PathDiagnosticConsumer {
 llvm::raw_ostream &Output;
@@ -59,23 +50,55 @@
 Compiler.getAnalyzerOpts()->CheckersControlList = {
 {"custom.CustomChecker", true}};
 AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
-  Registry.addChecker("custom.CustomChecker", "Description",
- "");
+  Registry.addChecker("custom.CustomChecker", "Description", "");
 });
 return std::move(AnalysisConsumer);
   }
 };
 
+template 
+bool runCheckerOnCode(const std::string &Code, std::string &Diags) {
+  llvm::raw_string_ostream OS(Diags);
+  return tooling::runToolOnCode(new TestAction(OS), Code);
+}
+template 
+bool runCheckerOnCode(const std::string &Code) {
+  std::string Diags;
+  return runCheckerOnCode(Code, Diags);
+}
+
+
+class CustomChecker : public Checker {
+public:
+  void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
+BugReporter &BR) const {
+BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
+   "Custom diagnostic description",
+   PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
+  }
+};
 
 TEST(RegisterCustomCheckers, RegisterChecker) {
   std::string Diags;
-  {
-llvm::raw_string_ostream OS(Diags);
-EXPECT_TRUE(tooling::runToolOnCode(new TestAction(OS), "void f() {;}"));
-  }
+  EXPECT_TRUE(runCheckerOnCode("void f() {;}", Diags));
   EXPECT_EQ(Diags, "custom.CustomChecker:Custom diagnostic description");
 }
 
+class LocIncDecChecker : public Checker {
+public:
+  void checkLocation(SVal Loc, bool IsLoad, const Stmt *S,
+ CheckerContext &C) const {
+auto UnaryOp = dyn_cast(S);
+if (UnaryOp && !IsLoad)
+  EXPECT_FALSE(UnaryOp->isIncrementOp());
+  }
+};
+
+TEST(RegisterCustomCheckers, CheckLocationIncDec) {
+  EXPECT_TRUE(
+  runCheckerOnCode("void f() { int *p; (*p)++; }"));
+}
+
 }
 }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

I tried adding isGLValue to evalStore and the test suite didn't complain. For 
evalLoad (on BoundEx) it failed in pretty much every test. Should the evalStore 
assert also go into trunk?

Since the analyzer behavior itself remains unchanged, I don't believe any other 
checker should be affected.

For myself it was pretty obvious that there is something weird going on when I 
didn't get the expected Stmt in my checker callback. So I added the following 
workaround. With this patch, the workaround is safely skipped.

The only change that this patch might cause "in the wild" is when someone used 
the Stmt as location, but didn't test with IncDecOps. However, as far as I can 
tell this should only have positive outcome.

Do I have any other means to check if other checkers were affected than to run 
the test suite?

  // Workaround for an inconsistency with IncDecOps: The statement is not the 
location expr.
  if (auto unaryE = dyn_cast(S))
  {
if (unaryE->isIncrementDecrementOp())
{
  S = unaryE->getSubExpr();
}
  }


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55701/new/

https://reviews.llvm.org/D55701



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

[PATCH] D55775: [Driver] Don't override '-march' when using '-arch x86_64h'

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

In D55775#1338512 , @qcolombet wrote:

> Should we emit an error if we request x86_64h with an arch older than haswell?


Makes sense. I'll put up a patch soon.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55775/new/

https://reviews.llvm.org/D55775



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

[PATCH] D56390: Generate Checkers.inc under clang/Driver and use from CC1Options.td

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi created this revision.
yamaguchi added a reviewer: thakis.
Herald added a subscriber: mgorny.

Generate Checkers.inc also under include/clang/Driver so that we can
include this file from Driver/CC1Options.td without making Driver's
tablegen output depending on StaticAnalyzer/Checker's tablegen output.


https://reviews.llvm.org/D56390

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/CMakeLists.txt


Index: clang/include/clang/Driver/CMakeLists.txt
===
--- clang/include/clang/Driver/CMakeLists.txt
+++ clang/include/clang/Driver/CMakeLists.txt
@@ -1,3 +1,7 @@
 set(LLVM_TARGET_DEFINITIONS Options.td)
 tablegen(LLVM Options.inc -gen-opt-parser-defs)
+clang_tablegen(Checkers.inc -gen-clang-sa-checkers
+   -I ${CMAKE_CURRENT_SOURCE_DIR}/../StaticAnalyzer/Checkers
+   SOURCE ${CMAKE_CURRENT_SOURCE_DIR}/../StaticAnalyzer/Checkers/Checkers.td
+   TARGET SACheckerForClangDriver)
 add_public_tablegen_target(ClangDriverOptions)
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -109,11 +109,11 @@
 const char *Values =
 #define GET_CHECKERS
 #define CHECKER(FULLNAME, CLASS, HT, DOC_URI)  FULLNAME ","
-#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
+#include "clang/Driver/Checkers.inc"
 #undef GET_CHECKERS
 #define GET_PACKAGES
 #define PACKAGE(FULLNAME)  FULLNAME ","
-#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
+#include "clang/Driver/Checkers.inc"
 #undef GET_PACKAGES
 ;
   }]>;


Index: clang/include/clang/Driver/CMakeLists.txt
===
--- clang/include/clang/Driver/CMakeLists.txt
+++ clang/include/clang/Driver/CMakeLists.txt
@@ -1,3 +1,7 @@
 set(LLVM_TARGET_DEFINITIONS Options.td)
 tablegen(LLVM Options.inc -gen-opt-parser-defs)
+clang_tablegen(Checkers.inc -gen-clang-sa-checkers
+   -I ${CMAKE_CURRENT_SOURCE_DIR}/../StaticAnalyzer/Checkers
+   SOURCE ${CMAKE_CURRENT_SOURCE_DIR}/../StaticAnalyzer/Checkers/Checkers.td
+   TARGET SACheckerForClangDriver)
 add_public_tablegen_target(ClangDriverOptions)
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -109,11 +109,11 @@
 const char *Values =
 #define GET_CHECKERS
 #define CHECKER(FULLNAME, CLASS, HT, DOC_URI)  FULLNAME ","
-#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
+#include "clang/Driver/Checkers.inc"
 #undef GET_CHECKERS
 #define GET_PACKAGES
 #define PACKAGE(FULLNAME)  FULLNAME ","
-#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
+#include "clang/Driver/Checkers.inc"
 #undef GET_PACKAGES
 ;
   }]>;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r350529 - [Sema] Fix unused variable warning in Release builds

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Mon Jan  7 07:22:08 2019
New Revision: 350529

URL: http://llvm.org/viewvc/llvm-project?rev=350529&view=rev
Log:
[Sema] Fix unused variable warning in Release builds

Modified:
cfe/trunk/lib/Sema/SemaDeclCXX.cpp

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=350529&r1=350528&r2=350529&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Jan  7 07:22:08 2019
@@ -14294,8 +14294,7 @@ NamedDecl *Sema::ActOnFriendFunctionDecl
 
   CXXScopeSpec &SS = D.getCXXScopeSpec();
   DeclarationNameInfo NameInfo = GetNameForDeclarator(D);
-  DeclarationName Name = NameInfo.getName();
-  assert(Name);
+  assert(NameInfo.getName());
 
   // Check for unexpanded parameter packs.
   if (DiagnoseUnexpandedParameterPack(Loc, TInfo, UPPC_FriendDeclaration) ||


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

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

[Louis Dionne via Phabricator via cfe-commits]

ldionne accepted this revision.
ldionne added a comment.

LGTM, but I suggest you address @Quuxplusone 's concerns regarding the tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D48342/new/

https://reviews.llvm.org/D48342



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

[PATCH] D56391: Limit COFF 'common' emission to <=32 alignment types.

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: majnemer, rnk, pengfei.

As reported in PR33035, LLVM crashes if given a common object with an
alignment of greater than 32 bits.  This is because the COFF file format
does not support these alignments, so emitting them is broken anyway.

This patch changes any global definitions greater than 32 bit alignment
to no longer be in 'common'.

https://bugs.llvm.org/show_bug.cgi?id=33035


Repository:
  rC Clang

https://reviews.llvm.org/D56391

Files:
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGen/microsoft-no-common-align.c


Index: test/CodeGen/microsoft-no-common-align.c
===
--- /dev/null
+++ test/CodeGen/microsoft-no-common-align.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -emit-llvm -o - %s | FileCheck 
%s
+typedef float TooLargeAlignment __attribute__((__vector_size__(64)));
+typedef float NormalAlignment __attribute__((__vector_size__(4)));
+
+TooLargeAlignment TooBig;
+// CHECK: @TooBig = dso_local global <16 x float>  zeroinitializer, align 64
+NormalAlignment JustRight;
+// CHECK: @JustRight = common dso_local global <1 x float>  zeroinitializer, 
align 4
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -3761,6 +3761,11 @@
   }
 }
   }
+  // COFF doesn't support alignments greater than 32, so these cannot be
+  // in common.
+  if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF() &&
+  Context.getTypeAlignIfKnown(D->getType()) > 32)
+return true;
 
   return false;
 }


Index: test/CodeGen/microsoft-no-common-align.c
===
--- /dev/null
+++ test/CodeGen/microsoft-no-common-align.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -emit-llvm -o - %s | FileCheck %s
+typedef float TooLargeAlignment __attribute__((__vector_size__(64)));
+typedef float NormalAlignment __attribute__((__vector_size__(4)));
+
+TooLargeAlignment TooBig;
+// CHECK: @TooBig = dso_local global <16 x float>  zeroinitializer, align 64
+NormalAlignment JustRight;
+// CHECK: @JustRight = common dso_local global <1 x float>  zeroinitializer, align 4
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -3761,6 +3761,11 @@
   }
 }
   }
+  // COFF doesn't support alignments greater than 32, so these cannot be
+  // in common.
+  if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF() &&
+  Context.getTypeAlignIfKnown(D->getType()) > 32)
+return true;
 
   return false;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r350530 - [OPENMP][NVPTX]Reduce number of barriers in reductions.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Jan  7 07:45:09 2019
New Revision: 350530

URL: http://llvm.org/viewvc/llvm-project?rev=350530&view=rev
Log:
[OPENMP][NVPTX]Reduce number of barriers in reductions.

After the fix for the syncthreads we don't need to generate extra
barriers for the parallel reductions.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_teams_reduction_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=350530&r1=350529&r2=350530&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Mon Jan  7 07:45:09 2019
@@ -3124,7 +3124,6 @@ static void emitReductionListCopy(
 /// sync
 /// if (I am the first warp)
 ///   Copy smem[thread_id] to my local D
-/// sync
 static llvm::Value *emitInterWarpCopyFunction(CodeGenModule &CGM,
   ArrayRef Privates,
   QualType ReductionArrayTy,
@@ -3337,12 +3336,6 @@ static llvm::Value *emitInterWarpCopyFun
 
   CGF.EmitBlock(W0MergeBB);
 
-  // While warp 0 copies values from transfer medium, all other warps must
-  // wait.
-  // kmpc_barrier.
-  CGM.getOpenMPRuntime().emitBarrierCall(CGF, Loc, OMPD_unknown,
- /*EmitChecks=*/false,
- /*ForceSimpleCall=*/true);
   if (NumIters > 1) {
 Cnt = Bld.CreateNSWAdd(Cnt, llvm::ConstantInt::get(CGM.IntTy, 
/*V=*/1));
 CGF.EmitStoreOfScalar(Cnt, CntAddr, /*Volatile=*/false, C.IntTy);

Modified: cfe/trunk/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp?rev=350530&r1=350529&r2=350530&view=diff
==
--- cfe/trunk/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp Mon Jan  
7 07:45:09 2019
@@ -231,7 +231,6 @@ int bar(int n){
   // CHECK: br label {{%?}}[[READ_CONT]]
   //
   // CHECK: [[READ_CONT]]
-  // CHECK: call void @__kmpc_barrier(%struct.ident_t* @
   // CHECK: [[NEXT:%.+]] = add nsw i32 [[CNT]], 1
   // CHECK: store i32 [[NEXT]], i32* [[CNT_ADDR]],
   // CHECK: br label
@@ -468,7 +467,6 @@ int bar(int n){
   //
   // CHECK: [[READ_CONT]]
   // CHECK: call void @__kmpc_barrier(%struct.ident_t* @
-  // CHECK: call void @__kmpc_barrier(%struct.ident_t* @
   // CHECK: [[IS_WARP_MASTER:%.+]] = icmp eq i32 [[LANEID]], 0
   // CHECK: br i1 [[IS_WARP_MASTER]], label {{%?}}[[DO_COPY:.+]], label 
{{%?}}[[COPY_ELSE:.+]]
   //
@@ -507,7 +505,6 @@ int bar(int n){
   // CHECK: br label {{%?}}[[READ_CONT]]
   //
   // CHECK: [[READ_CONT]]
-  // CHECK: call void @__kmpc_barrier(%struct.ident_t* @
   // CHECK: ret
 
 
@@ -823,7 +820,6 @@ int bar(int n){
   // CHECK: br label {{%?}}[[READ_CONT]]
   //
   // CHECK: [[READ_CONT]]
-  // CHECK: call void @__kmpc_barrier(%struct.ident_t* @
   // CHECK: ret
 
 #endif

Modified: cfe/trunk/test/OpenMP/nvptx_teams_reduction_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_teams_reduction_codegen.cpp?rev=350530&r1=350529&r2=350530&view=diff
==
--- cfe/trunk/test/OpenMP/nvptx_teams_reduction_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_teams_reduction_codegen.cpp Mon Jan  7 07:45:09 
2019
@@ -473,7 +473,6 @@ int bar(int n){
   // CHECK: br label {{%?}}[[READ_CONT]]
   //
   // CHECK: [[READ_CONT]]
-  // CHECK: call void @__kmpc_barrier(%struct.ident_t* @
   // CHECK: ret
 
 #endif


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

[PATCH] D55212: Handle alloc_size attribute on function pointers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: erik.pilkington, dexonsmith.
aaron.ballman added a comment.

In D55212#1347994 , @arichardson wrote:

> I don't see an easy way of fixing the pragma clang attribute support for 
> this. 
>  Would it be okay to commit this change anyway?


It causes a regression in behavior, so I'm not certain we should move forward 
with it just yet (unless there's a pressing reason?).

> Since `alloc_size` is only used for very few functions I'd be very surprised 
> if there's any existing code that relies on using `#pragma clang atrribute` 
> with `alloc_size`.

This feature has already been available in a public release and there's no way 
to know how much code it will break (if any). It does seem rather unlikely, but 
I've been surprised by how people use this feature in the past.

> By the way GCC now also supports the attribute on function pointers: 
> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=267158

Yeah, my concern isn't with the new functionality so much as it breaking old 
functionality.

I'm also adding Erik and Duncan to the review because they may have some more 
insights into whether `alloc_size` is being used with `#pragma clang attribute` 
in the wild. My feeling is: if we can't spot any uses of that feature being 
used to apply `alloc_size` with a reasonable amount of looking for it, then we 
can go ahead with this patch even if it removes support for `alloc_size` from 
the pragma. If we get push back from the community, we can fix or revert at 
that time. However, given that this is plausibly a breaking change, I'd rather 
not commit to trunk until after we branch to give folks more time to react. 
WDYT?




Comment at: test/Misc/pragma-attribute-supported-attributes-list.test:15
+// FIXME: After changing the subject from Function to HasFunctionProto, 
AllocSize is no longer listed (similar to Format, etc)
+// FIXME-NEXT: AllocSize (SubjectMatchRule_function)
 // CHECK-NEXT: AlwaysDestroy (SubjectMatchRule_variable)

arichardson wrote:
> aaron.ballman wrote:
> > arichardson wrote:
> > > This seems to also affect __attribute((format)) and others so I'm not 
> > > sure whether removing AllocSize from this list is a blocker for this 
> > > patch.
> > I believe you may need to add an `AttrSubjectMatcherSubRule` in Attr.td to 
> > expose the attribute for `#pragma clang attribute` support. Can you see if 
> > you can support it instead of shrinking this list? Otherwise, I worry that 
> > this change will break code that was using the pragma to apply the 
> > attribute to declarations.
> The problem here already has a fixme in attr.td:
> 
> ```
>   // FIXME: There's a matcher ambiguity with objc methods and blocks since
>   // functionType excludes them but functionProtoType includes them.
>   // AttrSubjectMatcherSubRule<"functionProtoType", [HasFunctionProto]>
> ```
> 
> It seems like this was already added as part of the initial pragma clang 
> attribute commit in https://reviews.llvm.org/D30009
> 
> I had a look at the AttrEmitter.cpp in tablegen but couldn't find a 
> straightforward solution.
> I'll add @arphaman to the review.
Ah, good to know that this was already a known issue. Hopefully @arphaman has 
some ideas on how to support this so we don't remove a supported attribute from 
the list.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55212/new/

https://reviews.llvm.org/D55212



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

[PATCH] D56314: [clangd] Don't store completion info if the symbol is not used for code completion.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/index/Index.h:232
 /// See also isIndexedForCodeCompletion().
+/// Note that we don't store completion information (signature, snippet,
+/// documentation, type, inclues, etc) if the symbol is not indexed for 
code

This comment would be most useful beside the mentioned fields themselves. Maybe 
add it there too? Possibly with a shorter form, since there's no need to 
mention the field names there.



Comment at: clangd/index/SymbolCollector.cpp:543
+
+  if (!(S.Flags & Symbol::IndexedForCodeCompletion))
+return Insert(S);

Most of the fields updated at the bottom aren't useful. However, I feel the 
documentation is actually important, since Sema only has doc comments for the 
**current** file and the rest are currently expected to be provided by the 
index.

I'm not sure if we already have the code to query the doc comments via index 
for member completions. If not, it's an oversight.
In any case, I suggest we **always** store the comments in **dynamic** index. 
Not storing the comments in the static index is fine, since any data for member 
completions should be provided by the dynamic index (we see a member in 
completion ⇒ sema has processed the headers ⇒ the dynamic index should know 
about those members)


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56314/new/

https://reviews.llvm.org/D56314



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

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D54945#1340528 , @MarinaKalashina 
wrote:

> @alexfh Just for me to be sure, should there be the following structure in 
> http://clang.llvm.org/extra/index.html: ...


Roughly like that. More specifically, I'd suggest to modify the See also 
section of clang-tidy/index.rst as follows:

  See also:
  
  .. toctree::
 :maxdepth: 1
  
 The list of clang-tidy checks 
 Clang-tidy IDE/Editor Integrations 
 Getting Involved 

The order of TOC items in http://clang.llvm.org/extra/index.html will be 
slightly weird (the three external docs will be placed before the headings of 
the main document), but that doesn't seem like a huge problem (and may be 
fixable).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54945/new/

https://reviews.llvm.org/D54945



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

[PATCH] D56393: [DebugInfo] Don't emit DW_AT_enum_class unless it's actually an 'enum class'.

[Paul Robinson via Phabricator via cfe-commits]

probinson created this revision.
probinson added reviewers: dblaikie, rnk, zturner.
probinson added a project: debug-info.
Herald added subscribers: cfe-commits, JDevlieghere, aprantl.

The original fix for PR36168 would emit DW_AT_enum_class for both of these 
cases:

enum Fixed : short { F1  = 1 };
enum class Class { C1 = 1 };

However, it really should be applied only to the latter case.

I'd just do this, except I'm not sure whether PDB cares about this difference.  
If it does, let me know and I guess we'll have to invent a new flag so the 
DWARF and PDB generators can each DTRT.

If not, I'll follow up with NFC name changes so the flag becomes 
DIFlagEnumClass.


Repository:
  rC Clang

https://reviews.llvm.org/D56393

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-enum-class.cpp


Index: clang/test/CodeGenCXX/debug-info-enum-class.cpp
===
--- clang/test/CodeGenCXX/debug-info-enum-class.cpp
+++ clang/test/CodeGenCXX/debug-info-enum-class.cpp
@@ -52,6 +52,7 @@
 // FIXME: this should just be a declaration under -fno-standalone-debug
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E"
 // CHECK-SAME: scope: [[TEST2:![0-9]+]]
+// CHECK-NOT:  DIFlagFixedEnum
 // CHECK-SAME: elements: [[TEST_ENUMS:![0-9]+]]
 // CHECK-SAME: identifier: "_ZTSN5test21EE"
 // CHECK: [[TEST2]] = !DINamespace(name: "test2"
@@ -67,6 +68,7 @@
 // FIXME: this should just be a declaration under -fno-standalone-debug
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E"
 // CHECK-SAME: scope: [[TEST3:![0-9]+]]
+// CHECK-NOT:  DIFlagFixedEnum
 // CHECK-SAME: elements: [[TEST_ENUMS]]
 // CHECK-SAME: identifier: "_ZTSN5test31EE"
 // CHECK: [[TEST3]] = !DINamespace(name: "test3"
@@ -78,6 +80,7 @@
 namespace test4 {
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E"
 // CHECK-SAME: scope: [[TEST4:![0-9]+]]
+// CHECK-NOT:  DIFlagFixedEnum
 // CHECK-SAME: elements: [[TEST_ENUMS]]
 // CHECK-SAME: identifier: "_ZTSN5test41EE"
 // CHECK: [[TEST4]] = !DINamespace(name: "test4"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2711,7 +2711,7 @@
   llvm::DIType *ClassTy = getOrCreateType(ED->getIntegerType(), DefUnit);
   return DBuilder.createEnumerationType(EnumContext, ED->getName(), DefUnit,
 Line, Size, Align, EltArray, ClassTy,
-Identifier, ED->isFixed());
+Identifier, ED->isScoped());
 }
 
 llvm::DIMacro *CGDebugInfo::CreateMacro(llvm::DIMacroFile *Parent,


Index: clang/test/CodeGenCXX/debug-info-enum-class.cpp
===
--- clang/test/CodeGenCXX/debug-info-enum-class.cpp
+++ clang/test/CodeGenCXX/debug-info-enum-class.cpp
@@ -52,6 +52,7 @@
 // FIXME: this should just be a declaration under -fno-standalone-debug
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E"
 // CHECK-SAME: scope: [[TEST2:![0-9]+]]
+// CHECK-NOT:  DIFlagFixedEnum
 // CHECK-SAME: elements: [[TEST_ENUMS:![0-9]+]]
 // CHECK-SAME: identifier: "_ZTSN5test21EE"
 // CHECK: [[TEST2]] = !DINamespace(name: "test2"
@@ -67,6 +68,7 @@
 // FIXME: this should just be a declaration under -fno-standalone-debug
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E"
 // CHECK-SAME: scope: [[TEST3:![0-9]+]]
+// CHECK-NOT:  DIFlagFixedEnum
 // CHECK-SAME: elements: [[TEST_ENUMS]]
 // CHECK-SAME: identifier: "_ZTSN5test31EE"
 // CHECK: [[TEST3]] = !DINamespace(name: "test3"
@@ -78,6 +80,7 @@
 namespace test4 {
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E"
 // CHECK-SAME: scope: [[TEST4:![0-9]+]]
+// CHECK-NOT:  DIFlagFixedEnum
 // CHECK-SAME: elements: [[TEST_ENUMS]]
 // CHECK-SAME: identifier: "_ZTSN5test41EE"
 // CHECK: [[TEST4]] = !DINamespace(name: "test4"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2711,7 +2711,7 @@
   llvm::DIType *ClassTy = getOrCreateType(ED->getIntegerType(), DefUnit);
   return DBuilder.createEnumerationType(EnumContext, ED->getName(), DefUnit,
 Line, Size, Align, EltArray, ClassTy,
-Identifier, ED->isFixed());
+Identifier, ED->isScoped());
 }
 
 llvm::DIMacro *CGDebugInfo::CreateMacro(llvm:

[PATCH] D56012: [clang-tidy] Be more liberal about literal zeroes in abseil checks

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

One random late comment.




Comment at: 
clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp:38
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for 
zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Seconds(int64_t{0});

I'd add more context to the CHECK-FIXES patterns, otherwise they are not very 
reliable. There's no implicit connection to the line numbers, so any long 
enough subsequence of lines containing 'absl::ZeroDuration()' would satisfy the 
test. One way to do that is to declare variables with unique names. Another is 
to add unique comments.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56012/new/

https://reviews.llvm.org/D56012



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

Re: [PATCH] D56393: [DebugInfo] Don't emit DW_AT_enum_class unless it's actually an 'enum class'.

[Zachary Turner via cfe-commits]

PDB/CodeView doesn’t doesn’t distinguish between various flavors of enums
On Mon, Jan 7, 2019 at 8:14 AM Paul Robinson via Phabricator <
revi...@reviews.llvm.org> wrote:

> probinson created this revision.
> probinson added reviewers: dblaikie, rnk, zturner.
> probinson added a project: debug-info.
> Herald added subscribers: cfe-commits, JDevlieghere, aprantl.
>
> The original fix for PR36168 would emit DW_AT_enum_class for both of these
> cases:
>
> enum Fixed : short { F1  = 1 };
> enum class Class { C1 = 1 };
>
> However, it really should be applied only to the latter case.
>
> I'd just do this, except I'm not sure whether PDB cares about this
> difference.  If it does, let me know and I guess we'll have to invent a new
> flag so the DWARF and PDB generators can each DTRT.
>
> If not, I'll follow up with NFC name changes so the flag becomes
> DIFlagEnumClass.
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D56393
>
> Files:
>   clang/lib/CodeGen/CGDebugInfo.cpp
>   clang/test/CodeGenCXX/debug-info-enum-class.cpp
>
>
> Index: clang/test/CodeGenCXX/debug-info-enum-class.cpp
> ===
> --- clang/test/CodeGenCXX/debug-info-enum-class.cpp
> +++ clang/test/CodeGenCXX/debug-info-enum-class.cpp
> @@ -52,6 +52,7 @@
>  // FIXME: this should just be a declaration under -fno-standalone-debug
>  // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E"
>  // CHECK-SAME: scope: [[TEST2:![0-9]+]]
> +// CHECK-NOT:  DIFlagFixedEnum
>  // CHECK-SAME: elements: [[TEST_ENUMS:![0-9]+]]
>  // CHECK-SAME: identifier: "_ZTSN5test21EE"
>  // CHECK: [[TEST2]] = !DINamespace(name: "test2"
> @@ -67,6 +68,7 @@
>  // FIXME: this should just be a declaration under -fno-standalone-debug
>  // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E"
>  // CHECK-SAME: scope: [[TEST3:![0-9]+]]
> +// CHECK-NOT:  DIFlagFixedEnum
>  // CHECK-SAME: elements: [[TEST_ENUMS]]
>  // CHECK-SAME: identifier: "_ZTSN5test31EE"
>  // CHECK: [[TEST3]] = !DINamespace(name: "test3"
> @@ -78,6 +80,7 @@
>  namespace test4 {
>  // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E"
>  // CHECK-SAME: scope: [[TEST4:![0-9]+]]
> +// CHECK-NOT:  DIFlagFixedEnum
>  // CHECK-SAME: elements: [[TEST_ENUMS]]
>  // CHECK-SAME: identifier: "_ZTSN5test41EE"
>  // CHECK: [[TEST4]] = !DINamespace(name: "test4"
> Index: clang/lib/CodeGen/CGDebugInfo.cpp
> ===
> --- clang/lib/CodeGen/CGDebugInfo.cpp
> +++ clang/lib/CodeGen/CGDebugInfo.cpp
> @@ -2711,7 +2711,7 @@
>llvm::DIType *ClassTy = getOrCreateType(ED->getIntegerType(), DefUnit);
>return DBuilder.createEnumerationType(EnumContext, ED->getName(),
> DefUnit,
>  Line, Size, Align, EltArray,
> ClassTy,
> -Identifier, ED->isFixed());
> +Identifier, ED->isScoped());
>  }
>
>  llvm::DIMacro *CGDebugInfo::CreateMacro(llvm::DIMacroFile *Parent,
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56323: [clang-tidy] Handle member variables in readability-simplify-boolean-expr

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D56323#1347489 , @LegalizeAdulthood 
wrote:

> I managed to do some limited testing on the original source file from the bug 
> report in lldb and verified that the correct fix was applied there.  I also 
> tried a few other files and verified no false positives.


Thanks. That is no issue, I will run this check locally over multiple projects 
once committed. If I find issues I will report back!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56323/new/

https://reviews.llvm.org/D56323



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

[PATCH] D56303: [clang-tidy] Handle case/default statements when simplifying boolean expressions

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:386
 
-  bool BoolValue = Bool->getValue();
+  const bool BoolValue = Bool->getValue();
 

aaron.ballman wrote:
> LegalizeAdulthood wrote:
> > JonasToth wrote:
> > > `const` on values is uncommon in clang-tidy code. Please keep that 
> > > consistent.
> > I can change the code, but I don't understand the push back.
> > 
> > "That's the way it's done elsewhere" just doesn't seem like good reasoning.
> > 
> > I write const on values to signify that they are computed once and only 
> > once.  What is gained by removing that statement of once-and-only-once?
> > "That's the way it's done elsewhere" just doesn't seem like good reasoning.
> 
> Keeping the code consistent with the vast majority of the remainder of the 
> project is valid reasoning. I am echoing the request to drop the top-level 
> const. We don't use this pattern for non-pointer/reference types and there's 
> not a whole lot gained by using it inconsistently.
Plus we do have a clang-tidy check (in the pipeline) that could do that 
automatically if the LLVM projects decides to go that route. So we won't suffer 
in the future, if we add the const.



Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:745
+  // matchers, but that is currently impossible.
+  //
+  const auto *If = dyn_cast(SwitchCase->getSubStmt());

LegalizeAdulthood wrote:
> JonasToth wrote:
> > redundant empty comment line
> Meh, it's not redundant.  It's there to aid readability of a big block of 
> text by visually separating it from the associated code.
> 
> Why is this a problem?
Its subjective, I wouldn't do it so I thought you might have overlooked it, if 
we prefer this its fine from my side.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56303/new/

https://reviews.llvm.org/D56303



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

[PATCH] D56395: [ASTMatchers] Improve assert message for broken parent map.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added a subscriber: cfe-commits.

This assert catches places where the AST (as seen by RecursiveASTVisitor)
becomes disconnected due to incomplete traversal.
Making it print the actual parent-less node is a lot more helpful - it's
possible to work out which part of the tree wasn't traversed.


Repository:
  rC Clang

https://reviews.llvm.org/D56395

Files:
  lib/ASTMatchers/ASTMatchFinder.cpp


Index: lib/ASTMatchers/ASTMatchFinder.cpp
===
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -676,13 +676,17 @@
   //  c) there is a bug in the AST, and the node is not reachable
   // Usually the traversal scope is the whole AST, which precludes b.
   // Bugs are common enough that it's worthwhile asserting when we can.
-  assert((Node.get() ||
-  /* Traversal scope is limited if none of the bounds are the TU */
-  llvm::none_of(ActiveASTContext->getTraversalScope(),
-[](Decl *D) {
-  return D->getKind() == Decl::TranslationUnit;
-})) &&
- "Found node that is not in the complete parent map!");
+#ifndef NDEBUG
+  if (!Node.get() &&
+  /* Traversal scope is full AST if any of the bounds are the TU */
+  llvm::any_of(ActiveASTContext->getTraversalScope(), [](Decl *D) {
+return D->getKind() == Decl::TranslationUnit;
+  })) {
+llvm::errs() << "Tried to match orphan node:\n";
+Node.dump(llvm::errs(), ActiveASTContext->getSourceManager());
+llvm_unreachable("Parent map should be complete!");
+  }
+#endif
   return false;
 }
 if (Parents.size() == 1) {


Index: lib/ASTMatchers/ASTMatchFinder.cpp
===
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -676,13 +676,17 @@
   //  c) there is a bug in the AST, and the node is not reachable
   // Usually the traversal scope is the whole AST, which precludes b.
   // Bugs are common enough that it's worthwhile asserting when we can.
-  assert((Node.get() ||
-  /* Traversal scope is limited if none of the bounds are the TU */
-  llvm::none_of(ActiveASTContext->getTraversalScope(),
-[](Decl *D) {
-  return D->getKind() == Decl::TranslationUnit;
-})) &&
- "Found node that is not in the complete parent map!");
+#ifndef NDEBUG
+  if (!Node.get() &&
+  /* Traversal scope is full AST if any of the bounds are the TU */
+  llvm::any_of(ActiveASTContext->getTraversalScope(), [](Decl *D) {
+return D->getKind() == Decl::TranslationUnit;
+  })) {
+llvm::errs() << "Tried to match orphan node:\n";
+Node.dump(llvm::errs(), ActiveASTContext->getSourceManager());
+llvm_unreachable("Parent map should be complete!");
+  }
+#endif
   return false;
 }
 if (Parents.size() == 1) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54309: [AST] Allow limiting the scope of common AST traversals (getParents, RAV).

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

@sammccall Thank you for analyzing and investigation! Keeping the assertion is 
of course good and I am not sure on how to proceed.

Given the close branch for 8.0 we might opt for an hotfix that resolves the 
issue for now.
I think the bug-report is the better place to continue working on it.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54309/new/

https://reviews.llvm.org/D54309



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

[clang-tools-extra] r350540 - [clangd] Include instead of . NFC

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Mon Jan  7 08:55:59 2019
New Revision: 350540

URL: http://llvm.org/viewvc/llvm-project?rev=350540&view=rev
Log:
[clangd] Include  instead of . NFC

This fixes the only clang-tidy check currently enabled by clangd.

Modified:
clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp
clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp

Modified: clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp?rev=350540&r1=350539&r2=350540&view=diff
==
--- clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp (original)
+++ clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp Mon Jan  7 08:55:59 
2019
@@ -16,8 +16,8 @@
 #include "ClangdLSPServer.h"
 #include "ClangdServer.h"
 #include "CodeComplete.h"
+#include 
 #include 
-#include 
 
 using namespace clang::clangd;
 

Modified: clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp?rev=350540&r1=350539&r2=350540&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp Mon Jan  7 
08:55:59 2019
@@ -10,7 +10,7 @@
 #include "Transport.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
-#include 
+#include 
 
 namespace clang {
 namespace clangd {


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

[clang-tools-extra] r350542 - [clangd] Fix Windows build after r350531

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Mon Jan  7 09:03:15 2019
New Revision: 350542

URL: http://llvm.org/viewvc/llvm-project?rev=350542&view=rev
Log:
[clangd] Fix Windows build after r350531

Modified:
clang-tools-extra/trunk/clangd/FSProvider.cpp

Modified: clang-tools-extra/trunk/clangd/FSProvider.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FSProvider.cpp?rev=350542&r1=350541&r2=350542&view=diff
==
--- clang-tools-extra/trunk/clangd/FSProvider.cpp (original)
+++ clang-tools-extra/trunk/clangd/FSProvider.cpp Mon Jan  7 09:03:15 2019
@@ -75,7 +75,7 @@ clang::clangd::RealFileSystemProvider::g
 // FIXME: Try to use a similar approach in Sema instead of relying on
 //propagation of the 'isVolatile' flag through all layers.
 #ifdef _WIN32
-  return new VolatileFileSystem(vfs::getRealFileSystem());
+  return new VolatileFileSystem(llvm::vfs::getRealFileSystem());
 #else
   return llvm::vfs::getRealFileSystem();
 #endif


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

[PATCH] D56226: [clang-format] square parens that are followed by '=' are not Objective-C message sends

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:501
+CurrentToken->Next->is(tok::equal))) ||
+  (CurrentToken->Previous->Previous == Left)) &&
  Left->is(TT_ObjCMethodExpr)) {

I think you need to check if `CurrentToken->Previous` is null before 
dereferencing it.




Comment at: lib/Format/TokenAnnotator.cpp:504
+  // An ObjC method call is rarely followed by an open parenthesis or
+  // an assignment. It also usually contains more than one token.
   // FIXME: Do we incorrectly label ":" with this?

I think it's actually required to have more than one token. Can we reduce the 
change to that?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56226/new/

https://reviews.llvm.org/D56226



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

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> - if the closing parenthesis of the function is inside a macro, no FixIt will 
> be created (I cannot relyably lex for subsequent CV and ref qualifiers and 
> maybe we do not want to make changes in macros)

Usually macros are untouched because its impossible to transform them 
correctly. Someone, somewhere does evil stuff with them and is of course mad if 
the transformation interfers ;)

> - if the return type is not CV qualified, i allow macros to be part of it 
> because no lexing is required
> - if the return type is CV qualified and contains macros, I provide no FixIt
> 
>   I improved findTrailingReturnTypeSourceLocation() by discovering 
> FunctionTypeLoc::getRParenLoc(). I still require some lexing for CV and ref 
> qualifiers though.
> 
>   I tried to improve findReturnTypeAndCVSourceRange() by finding a way to get 
> the source range directly from the AST, but it seems the AST does not store 
> source locations for CV qualifiers of types. I read that in the docs for 
> QualifiedTypeLoc 
> . 
> So it seems I cannot circumvent finding the locations of const and volatile 
> by my own.

Yup.




Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:161-163
+  const auto &ctx = *Result.Context;
+  const auto &sm = *Result.SourceManager;
+  const auto &opts = getLangOpts();

bernhardmgruber wrote:
> lebedev.ri wrote:
> > 1. All the variables should be WithThisCase
> > 2. Can't tell if the `auto` is ok here? 
> > 
> 1. done
> 2. Is it important to know what types Result.Context and the others are? I am 
> just passing them on to further functions because I have to, they are not 
> relevant for the logic I am coding.
`auto` shuold only be used if its obvious what type the variable has (e.g. 
`auto foo = make_unique()`, but not for `auto Foo = GetSomeResult();`).
This makes review and reading easier and is the guideline for LLVM. It has room 
for subjective reasoning, but adding the type is usually better.



Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:19
+namespace modernize {
+constexpr auto Message = "use a trailing return type for this function";
+

`auto -> const char*` here, thats not nice :)

How about a `llvm::StringRef` or 
https://llvm.org/doxygen/classllvm_1_1StringLiteral.html  (in this case better)



Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:33
+  const TypeSourceInfo *TSI = F.getTypeSourceInfo();
+  assert(TSI);
+  const FunctionTypeLoc FTL =

Please add an error-msg to the assertion like `assert(TSI && "Reason why this 
must hold");`. Humanreadable for debugging.



Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:38
+
+  // if the function argument list ends inside of a macro, it is dangerous to
+  // start lexing from here, bail out

Please make that comment (and all comments in general) full grammatical 
sentence with correct punctuation.



Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:42
+  if (ClosingParen.isMacroID()) {
+diag(
+F.getLocation(),

weird formatting, did clang-format do that?



Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:45
+std::string(Message) +
+" (no FixIt provided, function argument list end is inside 
macro)");
+return {};

I think you can ellide that extra message. Not emitting the fixit is clear 
already.



Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:46
+" (no FixIt provided, function argument list end is inside 
macro)");
+return {};
+  }

Indiciator to use `llvm::Optional` as return type instead. What do you think?



Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:53
+  // skip subsequent CV and ref qualifiers
+  const std::pair Loc = SM.getDecomposedLoc(Result);
+  const StringRef File = SM.getBufferData(Loc.first);

Values are usually not `const`'ed. Please change that for consistency.



Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:67
+
+if (T.is(tok::amp) || T.is(tok::ampamp) || T.is(tok::kw_const) ||
+T.is(tok::kw_volatile)) {

why are pointers not relevant here?
There should be `Token.oneOf()` or `Token.isOneOf()` or similar for this usecase



Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:88
+diag(F.getLocation(),
+ std::string(Message) + " (failed to determine return type source "
+"range, could clang resolve all #includes?)",

Same with the other extra-bit of information. This will rather confuse the user 
of clang-tidy then give additional information. Please ellide it.


===

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D55433#1347553 , @MyDeveloperDay 
wrote:

> libprotobuf still builds cleanly with just the one expected warning..despite 
> adding over 500 [[nodiscards]]
>
> F7800873: image.png 
>
> I'll continue to look at other projects, but I'm happy at present that this 
> gives us a good starting point.


That looks good. If llvm (significant parts of it) are fine too, ready to go.

P.S. did you request commit rights already?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55433/new/

https://reviews.llvm.org/D55433



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

[PATCH] D56395: [ASTMatchers] Improve assert message for broken parent map.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56395/new/

https://reviews.llvm.org/D56395



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

r350555 - [X86] Use funnel shift intrinsics for the VBMI2 vshld/vshrd builtins.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Mon Jan  7 11:10:22 2019
New Revision: 350555

URL: http://llvm.org/viewvc/llvm-project?rev=350555&view=rev
Log:
[X86] Use funnel shift intrinsics for the VBMI2 vshld/vshrd builtins.

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

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/avx512vbmi2intrin.h
cfe/trunk/lib/Headers/avx512vlvbmi2intrin.h
cfe/trunk/test/CodeGen/avx512vbmi2-builtins.c
cfe/trunk/test/CodeGen/avx512vlvbmi2-builtins.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=350555&r1=350554&r2=350555&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Mon Jan  7 11:10:22 2019
@@ -1256,43 +1256,25 @@ TARGET_BUILTIN(__builtin_ia32_vpshldw128
 TARGET_BUILTIN(__builtin_ia32_vpshldw256, "V16sV16sV16sIi", "ncV:256:", 
"avx512vl,avx512vbmi2")
 TARGET_BUILTIN(__builtin_ia32_vpshldw512, "V32sV32sV32sIi", "ncV:512:", 
"avx512vbmi2")
 
-TARGET_BUILTIN(__builtin_ia32_vpshldvd128_mask, "V4iV4iV4iV4iUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvd256_mask, "V8iV8iV8iV8iUc", "ncV:256:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvd512_mask, "V16iV16iV16iV16iUs", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvq128_mask, "V2LLiV2LLiV2LLiV2LLiUc", 
"ncV:128:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvq256_mask, "V4LLiV4LLiV4LLiV4LLiUc", 
"ncV:256:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvq512_mask, "V8LLiV8LLiV8LLiV8LLiUc", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvw128_mask, "V8sV8sV8sV8sUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvw256_mask, "V16sV16sV16sV16sUs", 
"ncV:256:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvw512_mask, "V32sV32sV32sV32sUi", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvd128_maskz, "V4iV4iV4iV4iUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvd256_maskz, "V8iV8iV8iV8iUc", "ncV:256:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvd512_maskz, "V16iV16iV16iV16iUs", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvq128_maskz, "V2LLiV2LLiV2LLiV2LLiUc", 
"ncV:128:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvq256_maskz, "V4LLiV4LLiV4LLiV4LLiUc", 
"ncV:256:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvq512_maskz, "V8LLiV8LLiV8LLiV8LLiUc", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvw128_maskz, "V8sV8sV8sV8sUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvw256_maskz, "V16sV16sV16sV16sUs", 
"ncV:256:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvw512_maskz, "V32sV32sV32sV32sUi", 
"ncV:512:", "avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvd128, "V4iV4iV4iV4i", "ncV:128:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvd256, "V8iV8iV8iV8i", "ncV:256:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvd512, "V16iV16iV16iV16i", "ncV:512:", 
"avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvq128, "V2LLiV2LLiV2LLiV2LLi", "ncV:128:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvq256, "V4LLiV4LLiV4LLiV4LLi", "ncV:256:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvq512, "V8LLiV8LLiV8LLiV8LLi", "ncV:512:", 
"avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvw128, "V8sV8sV8sV8s", "ncV:128:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvw256, "V16sV16sV16sV16s", "ncV:256:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvw512, "V32sV32sV32sV32s", "ncV:512:", 
"avx512vbmi2")
 
-TARGET_BUILTIN(__builtin_ia32_vpshrdvd128_mask, "V4iV4iV4iV4iUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvd256_mask, "V8iV8iV8iV8iUc", "ncV:256:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvd512_mask, "V16iV16iV16iV16iUs", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvq128_mask, "V2LLiV2LLiV2LLiV2LLiUc", 
"ncV:128:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvq256_mask, "V4LLiV4LLiV4LLiV4LLiUc", 
"ncV:256:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvq512_mask, "V8LLiV8LLiV8LLiV8LLiUc", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvw128_mask, "V8sV8sV8sV8sUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvw256_mask, "V16sV16sV16sV16sUs", 
"ncV:256:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvw512_mask, "V32sV32sV32sV32sUi", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvd128_maskz, "V4iV4iV

[PATCH] D56365: [X86] Use funnel shift intrinsics for the VBMI2 vshld/vshrd builtins.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350555: [X86] Use funnel shift intrinsics for the VBMI2 
vshld/vshrd builtins. (authored by ctopper, committed by ).

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56365/new/

https://reviews.llvm.org/D56365

Files:
  cfe/trunk/include/clang/Basic/BuiltinsX86.def
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/Headers/avx512vbmi2intrin.h
  cfe/trunk/lib/Headers/avx512vlvbmi2intrin.h
  cfe/trunk/test/CodeGen/avx512vbmi2-builtins.c
  cfe/trunk/test/CodeGen/avx512vlvbmi2-builtins.c

Index: cfe/trunk/lib/Headers/avx512vlvbmi2intrin.h
===
--- cfe/trunk/lib/Headers/avx512vlvbmi2intrin.h
+++ cfe/trunk/lib/Headers/avx512vlvbmi2intrin.h
@@ -421,327 +421,279 @@
   (__v8hi)_mm_setzero_si128())
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
-_mm256_mask_shldv_epi64(__m256i __S, __mmask8 __U, __m256i __A, __m256i __B)
+_mm256_shldv_epi64(__m256i __A, __m256i __B, __m256i __C)
 {
-  return (__m256i) __builtin_ia32_vpshldvq256_mask ((__v4di) __S,
-  (__v4di) __A,
-  (__v4di) __B,
-  __U);
+  return (__m256i)__builtin_ia32_vpshldvq256((__v4di)__A, (__v4di)__B,
+ (__v4di)__C);
 }
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
-_mm256_maskz_shldv_epi64(__mmask8 __U, __m256i __S, __m256i __A, __m256i __B)
+_mm256_mask_shldv_epi64(__m256i __A, __mmask8 __U, __m256i __B, __m256i __C)
 {
-  return (__m256i) __builtin_ia32_vpshldvq256_maskz ((__v4di) __S,
-  (__v4di) __A,
-  (__v4di) __B,
-  __U);
+  return (__m256i)__builtin_ia32_selectq_256(__U,
+  (__v4di)_mm256_shldv_epi64(__A, __B, __C),
+  (__v4di)__A);
 }
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
-_mm256_shldv_epi64(__m256i __S, __m256i __A, __m256i __B)
+_mm256_maskz_shldv_epi64(__mmask8 __U, __m256i __A, __m256i __B, __m256i __C)
 {
-  return (__m256i) __builtin_ia32_vpshldvq256_mask ((__v4di) __S,
-  (__v4di) __A,
-  (__v4di) __B,
-  (__mmask8) -1);
+  return (__m256i)__builtin_ia32_selectq_256(__U,
+  (__v4di)_mm256_shldv_epi64(__A, __B, __C),
+  (__v4di)_mm256_setzero_si256());
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS128
-_mm_mask_shldv_epi64(__m128i __S, __mmask8 __U, __m128i __A, __m128i __B)
+_mm_shldv_epi64(__m128i __A, __m128i __B, __m128i __C)
 {
-  return (__m128i) __builtin_ia32_vpshldvq128_mask ((__v2di) __S,
-  (__v2di) __A,
-  (__v2di) __B,
-  __U);
+  return (__m128i)__builtin_ia32_vpshldvq128((__v2di)__A, (__v2di)__B,
+ (__v2di)__C);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS128
-_mm_maskz_shldv_epi64(__mmask8 __U, __m128i __S, __m128i __A, __m128i __B)
+_mm_mask_shldv_epi64(__m128i __A, __mmask8 __U, __m128i __B, __m128i __C)
 {
-  return (__m128i) __builtin_ia32_vpshldvq128_maskz ((__v2di) __S,
-  (__v2di) __A,
-  (__v2di) __B,
-  __U);
+  return (__m128i)__builtin_ia32_selectq_128(__U,
+ (__v2di)_mm_shldv_epi64(__A, __B, __C),
+ (__v2di)__A);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS128
-_mm_shldv_epi64(__m128i __S, __m128i __A, __m128i __B)
+_mm_maskz_shldv_epi64(__mmask8 __U, __m128i __A, __m128i __B, __m128i __C)
 {
-  return (__m128i) __builtin_ia32_vpshldvq128_mask ((__v2di) __S,
-  (__v2di) __A,
-  (__v2di) __B,
-  (__mmask8) -1);
+  return (__m128i)__builtin_ia32_selectq_128(__U,
+ (__v2di)_mm_shldv_epi64(__A, __B, __C),
+ (__v2di)_mm_setzero_si128());
 }
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
-_mm256_mask_shldv_epi32(__m256i __S, __mmask8 __U, __m256i __A, __m256i __B)
+_mm256_shldv_epi32(__m256i __A, __m256i __B, __m256i __C)
 {
-  return (__m256i) __builtin_ia32_vpshldvd256_mask ((__v8si) __S,
-  (__v8si) __A,
-  (__v8si) __B,
-  __U);
+  return (__m256i)__builtin_ia32_vpshldvd256((__v8si)__A, (__v8si)__B,
+ (__v8si)__C);
 }
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
-_mm256_maskz_shldv_epi32(__mmask8 __U, __m256i __S, __m256i __A, __m256i __B)
+_mm256_mask_shldv_epi32(__m256i __A, __mmask8 __U, __m256i __B, __m256i __C)
 {
-  return (__m256i) __builtin_ia32_vpshldvd256_maskz ((__v8si) __S,
-  (__v8si) __A,
-  (__v8si) __B,
-  __U);
+  return (__m256i)__builtin_ia32_selectd_256(__U,
+  (__v8si)_mm256_shldv_e

[PATCH] D56288: [ELF] Do not enable 'new dtags' on NetBSD by default

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

I think when we implemented the feature, there was an understanding that the 
"new dtags" might be "new" 20 years ago but they are not new at all now. This 
is not the only example of changing the default in lld; one example being that 
text segments are not writable by default (`-z text` is default as opposed to 
`-z notext`).

We could argue that making the text segment writable is a total nonsense today, 
but new/old dtags are different story. Bug even if that's the case, changing 
the default at this point has perhaps too much impact to existing lld users.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56288/new/

https://reviews.llvm.org/D56288



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

[PATCH] D56367: [AST] Pack CXXDependentScopeMemberExpr

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay.  That comment seems reasonable.  Glad to hear you're on top of it. :)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56367/new/

https://reviews.llvm.org/D56367



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

r350563 - Revert r350555 "[X86] Use funnel shift intrinsics for the VBMI2 vshld/vshrd builtins."

[Craig Topper via cfe-commits]

Author: ctopper
Date: Mon Jan  7 11:39:25 2019
New Revision: 350563

URL: http://llvm.org/viewvc/llvm-project?rev=350563&view=rev
Log:
Revert r350555 "[X86] Use funnel shift intrinsics for the VBMI2 vshld/vshrd 
builtins."

Had to revert the LLVM patch this depends on to fix a MSVC compiler limit in 
AutoUpgrade.cpp

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/avx512vbmi2intrin.h
cfe/trunk/lib/Headers/avx512vlvbmi2intrin.h
cfe/trunk/test/CodeGen/avx512vbmi2-builtins.c
cfe/trunk/test/CodeGen/avx512vlvbmi2-builtins.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=350563&r1=350562&r2=350563&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Mon Jan  7 11:39:25 2019
@@ -1256,25 +1256,43 @@ TARGET_BUILTIN(__builtin_ia32_vpshldw128
 TARGET_BUILTIN(__builtin_ia32_vpshldw256, "V16sV16sV16sIi", "ncV:256:", 
"avx512vl,avx512vbmi2")
 TARGET_BUILTIN(__builtin_ia32_vpshldw512, "V32sV32sV32sIi", "ncV:512:", 
"avx512vbmi2")
 
-TARGET_BUILTIN(__builtin_ia32_vpshldvd128, "V4iV4iV4iV4i", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvd256, "V8iV8iV8iV8i", "ncV:256:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvd512, "V16iV16iV16iV16i", "ncV:512:", 
"avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvq128, "V2LLiV2LLiV2LLiV2LLi", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvq256, "V4LLiV4LLiV4LLiV4LLi", "ncV:256:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvq512, "V8LLiV8LLiV8LLiV8LLi", "ncV:512:", 
"avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvw128, "V8sV8sV8sV8s", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvw256, "V16sV16sV16sV16s", "ncV:256:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvw512, "V32sV32sV32sV32s", "ncV:512:", 
"avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvd128_mask, "V4iV4iV4iV4iUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvd256_mask, "V8iV8iV8iV8iUc", "ncV:256:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvd512_mask, "V16iV16iV16iV16iUs", 
"ncV:512:", "avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvq128_mask, "V2LLiV2LLiV2LLiV2LLiUc", 
"ncV:128:", "avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvq256_mask, "V4LLiV4LLiV4LLiV4LLiUc", 
"ncV:256:", "avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvq512_mask, "V8LLiV8LLiV8LLiV8LLiUc", 
"ncV:512:", "avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvw128_mask, "V8sV8sV8sV8sUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvw256_mask, "V16sV16sV16sV16sUs", 
"ncV:256:", "avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvw512_mask, "V32sV32sV32sV32sUi", 
"ncV:512:", "avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvd128_maskz, "V4iV4iV4iV4iUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvd256_maskz, "V8iV8iV8iV8iUc", "ncV:256:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvd512_maskz, "V16iV16iV16iV16iUs", 
"ncV:512:", "avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvq128_maskz, "V2LLiV2LLiV2LLiV2LLiUc", 
"ncV:128:", "avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvq256_maskz, "V4LLiV4LLiV4LLiV4LLiUc", 
"ncV:256:", "avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvq512_maskz, "V8LLiV8LLiV8LLiV8LLiUc", 
"ncV:512:", "avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvw128_maskz, "V8sV8sV8sV8sUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvw256_maskz, "V16sV16sV16sV16sUs", 
"ncV:256:", "avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvw512_maskz, "V32sV32sV32sV32sUi", 
"ncV:512:", "avx512vbmi2")
 
-TARGET_BUILTIN(__builtin_ia32_vpshrdvd128, "V4iV4iV4iV4i", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvd256, "V8iV8iV8iV8i", "ncV:256:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvd512, "V16iV16iV16iV16i", "ncV:512:", 
"avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvq128, "V2LLiV2LLiV2LLiV2LLi", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvq256, "V4LLiV4LLiV4LLiV4LLi", "ncV:256:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvq512, "V8LLiV8LLiV8LLiV8LLi", "ncV:512:", 
"avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvw128, "V8sV8sV8sV8s", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvw256, "V16sV16sV16sV16s", "ncV:256:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvw512, "V32sV32sV32sV32s", "ncV:512:", 
"avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshrdvd128_mask, "V4iV4iV4iV4iUc

[PATCH] D55662: [Sema][ObjC] Do not warn about repeated uses of weak variables when the variables are accessed in an unevaluated context.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

We can make `DeduceVariableDeclarationType` return the rewritten expression and 
replace `Init` in `Sema::AddInitializerToDecl` with it.

Alternatively, we can keep a set of `ObjCPropertyRefExpr`s passed to 
`recordUseOfWeak` (the original `ObjCPropertyRefExpr`, if it was rebuilt in 
SemaPseudoObject.cpp) and avoid recording the use of a weak variable if the 
expression is passed the second time.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55662/new/

https://reviews.llvm.org/D55662



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

[PATCH] D56066: [OpenCL] Address space for default class members

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaInit.cpp:4539
+  if (InitCategory.isPRValue() || InitCategory.isXValue())
+T1Quals.removeAddressSpace();
+

ebevhan wrote:
> rjmccall wrote:
> > rjmccall wrote:
> > > I can understand why a pr-value wouldn't have an address space, but an 
> > > x-value's address space is surely important.
> > No, wait, I think this is more deeply wrong.  We're initializing a 
> > reference to type `cv1 T1`, and if we're initializing it with a temporary, 
> > we're dropping the address space from `cv1`?  That's definitely wrong.
> > 
> > If we're starting with a pr-value, reference-initialization normally has to 
> > start by initializing a temporary.  (The exception is it's a class type 
> > with a conversion operator to a reference type; C++ is abysmally 
> > complicated.  Let's ignore this for a moment.)  Temporaries are always in 
> > the default address space (the address space of local variables) because 
> > the language (neither OpenCL nor Embedded C) does not give us any facility 
> > for allocating temporary memory anywhere else.  So reference-initialization 
> > from a pr-value creates a temporary in the default address space and then 
> > attempts to initialize the destination reference type with that temporary.  
> > That initialization should fail if there's no conversion from the default 
> > address space to the destination address space.  For example, consider 
> > initializing a `const int __global &` from a pr-value: this should clearly 
> > not succeed, because we have no way of putting a temporary in the global 
> > address space.  That reference can only be initialized with a gl-value 
> > that's already in the `__global` address space.
> > 
> > On the other hand, we can successfully initialize a `const int &` with a 
> > gl-value of type `const int __global` not by direct reference 
> > initialization but by loading from the operand and then materializing the 
> > result into a new temporary.
> > 
> > I think what this means is:
> > 
> > - We need to be doing this checking as if pr-values were in `__private`.  
> > This includes both (1) expressions that were originally pr-values and (2) 
> > expressions that have been converted to pr-values due to a failure to 
> > perform direct reference initialization.
> > 
> > - We need to make sure that reference compatibility correctly prevents 
> > references from being bound to gl-values in incompatible address spaces.
> > On the other hand, we can successfully initialize a `const int &` with a 
> > gl-value of type `const int __global` not by direct reference 
> > initialization but by loading from the operand and then materializing the 
> > result into a new temporary.
> 
> Is this the right thing to do? It means that initializing such a reference 
> would incur a cost under the hood that might be unnoticed/unwanted by the 
> user. 
Hmm.  I was going to say that it's consistent with the normal behavior of C++, 
but looking at the actual rule, it's more complicated than that: C++ will only 
do this if the types are not reference-related or if the gl-value is a 
bit-field.  So this would only be allowed if the reference type was e.g. `const 
long &`.  It's a strange rule, but it's straightforward to stay consistent with 
it.

I think we will eventually find ourselves needing a special-case rule for 
copy-initializing and copy-assigning trivially-copyable types, but we're not 
there yet.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56066/new/

https://reviews.llvm.org/D56066



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

[PATCH] D55662: [Sema][ObjC] Do not warn about repeated uses of weak variables when the variables are accessed in an unevaluated context.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

If it's at all reasonable to just avoid doing the work multiple times, let's do 
that.  It should also avoid duplicate diagnostics if e.g. an overload is 
resolved to an unavailable function.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55662/new/

https://reviews.llvm.org/D55662



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

[PATCH] D56405: Split -Wdelete-non-virtual-dtor into two groups

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: rsmith, aaron.ballman.
Herald added subscribers: dexonsmith, jkorous.

`-Wdelete-non-virtual-dtor` controlled two diagnostics: 1) calling a 
non-virtual dtor from an abstract class, and 2) calling a non-virtual dtor from 
a polymorphic class. 1) is a lot more severe than 2), since 1) is a guaranteed 
crash, but 2) is just "code smell". Previously, projects compiled with `-Wall 
-Wno-delete-non-virtual-dtor`, which is somewhat reasonable, silently crashed 
on 1).

rdar://40380564

Thanks for taking a look!


Repository:
  rC Clang

https://reviews.llvm.org/D56405

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/SemaCXX/non-virtual-dtors.cpp


Index: clang/test/SemaCXX/non-virtual-dtors.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/non-virtual-dtors.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 %s -verify -DDIAG1
+// RUN: %clang_cc1 %s -verify -DDIAG1 -DDIAG2 -Wdelete-non-virtual-dtor
+// RUN: %clang_cc1 %s -verify -DDIAG1 -Wmost 
-Wno-delete-non-virtual-dtor
+// RUN: %clang_cc1 %s -verify -Wmost 
-Wno-delete-abstract-non-virtual-dtor
+
+#ifndef DIAG1
+#ifndef DIAG2
+// expected-no-diagnostics
+#endif
+#endif
+
+struct S1 {
+  ~S1() {}
+  virtual void abs() = 0;
+};
+
+void f1(S1 *s1) { delete s1; }
+#ifdef DIAG1
+// expected-warning@-2 {{delete called on 'S1' that is abstract but has 
non-virtual destructor}}
+#endif
+
+struct Base {
+  virtual void abs() = 0;
+};
+struct S2 : Base {
+  ~S2() {}
+  void abs() {}
+};
+void f2(S2 *s2) { delete s2; }
+#ifdef DIAG2
+// expected-warning@-2 {{delete called on non-final 'S2' that has virtual 
functions but non-virtual destructor}}
+#endif
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6460,7 +6460,7 @@
   "qualify call to silence this warning">;
 def warn_delete_abstract_non_virtual_dtor : Warning<
   "%select{delete|destructor}0 called on %1 that is abstract but has "
-  "non-virtual destructor">, InGroup, ShowInSystemHeader;
+  "non-virtual destructor">, InGroup, 
ShowInSystemHeader;
 def warn_overloaded_virtual : Warning<
   "%q0 hides overloaded virtual %select{function|functions}1">,
   InGroup, DefaultIgnore;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -105,6 +105,8 @@
 
 def DeleteIncomplete : DiagGroup<"delete-incomplete">;
 def DeleteNonVirtualDtor : DiagGroup<"delete-non-virtual-dtor">;
+def DeleteAbstractNonVirtualDtor : 
DiagGroup<"delete-abstract-non-virtual-dtor",
+ [DeleteNonVirtualDtor]>;
 def AbstractFinalClass : DiagGroup<"abstract-final-class">;
 
 def CXX11CompatDeprecatedWritableStr :


Index: clang/test/SemaCXX/non-virtual-dtors.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/non-virtual-dtors.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 %s -verify -DDIAG1
+// RUN: %clang_cc1 %s -verify -DDIAG1 -DDIAG2 -Wdelete-non-virtual-dtor
+// RUN: %clang_cc1 %s -verify -DDIAG1 -Wmost -Wno-delete-non-virtual-dtor
+// RUN: %clang_cc1 %s -verify -Wmost -Wno-delete-abstract-non-virtual-dtor
+
+#ifndef DIAG1
+#ifndef DIAG2
+// expected-no-diagnostics
+#endif
+#endif
+
+struct S1 {
+  ~S1() {}
+  virtual void abs() = 0;
+};
+
+void f1(S1 *s1) { delete s1; }
+#ifdef DIAG1
+// expected-warning@-2 {{delete called on 'S1' that is abstract but has non-virtual destructor}}
+#endif
+
+struct Base {
+  virtual void abs() = 0;
+};
+struct S2 : Base {
+  ~S2() {}
+  void abs() {}
+};
+void f2(S2 *s2) { delete s2; }
+#ifdef DIAG2
+// expected-warning@-2 {{delete called on non-final 'S2' that has virtual functions but non-virtual destructor}}
+#endif
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6460,7 +6460,7 @@
   "qualify call to silence this warning">;
 def warn_delete_abstract_non_virtual_dtor : Warning<
   "%select{delete|destructor}0 called on %1 that is abstract but has "
-  "non-virtual destructor">, InGroup, ShowInSystemHeader;
+  "non-virtual destructor">, InGroup, ShowInSystemHeader;
 def warn_overloaded_virtual : Warning<
   "%q0 hides overloaded virtual %select{function|functions}1">,
   InGroup, DefaultIgnore;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/Diagnostic

[libclc] r350565 - cmake: Install libraries to DATADIR from GNUInstallDirs

[Jan Vesely via cfe-commits]

Author: jvesely
Date: Mon Jan  7 12:20:37 2019
New Revision: 350565

URL: http://llvm.org/viewvc/llvm-project?rev=350565&view=rev
Log:
cmake: Install libraries to DATADIR from GNUInstallDirs

This moves default installation location to /usr/share to match libclc.pc.
Signed-off-by: Jan Vesely 
Reviewer: Tom Stellard

Modified:
libclc/trunk/CMakeLists.txt
libclc/trunk/libclc.pc.in

Modified: libclc/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/CMakeLists.txt?rev=350565&r1=350564&r2=350565&view=diff
==
--- libclc/trunk/CMakeLists.txt (original)
+++ libclc/trunk/CMakeLists.txt Mon Jan  7 12:20:37 2019
@@ -1,6 +1,7 @@
 cmake_minimum_required( VERSION 3.9.2 )
 
 project( libclc VERSION 0.2.0 LANGUAGES CXX )
+include( GNUInstallDirs )
 
 # List of all targets
 set( LIBCLC_TARGETS_ALL
@@ -145,8 +146,8 @@ endif()
 
 # pkg-config file
 configure_file( libclc.pc.in libclc.pc @ONLY )
-install( FILES ${CMAKE_CURRENT_BINARY_DIR}/libclc.pc DESTINATION 
share/pkgconfig )
-install( DIRECTORY generic/include/clc DESTINATION include )
+install( FILES ${CMAKE_CURRENT_BINARY_DIR}/libclc.pc DESTINATION 
${CMAKE_INSTALL_DATADIR}/pkgconfig )
+install( DIRECTORY generic/include/clc DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} 
)
 
 if( ENABLE_RUNTIME_SUBNORMAL )
add_library( subnormal_use_default STATIC
@@ -154,7 +155,7 @@ if( ENABLE_RUNTIME_SUBNORMAL )
add_library( subnormal_disable STATIC
generic/lib/subnormal_disable.ll )
install( TARGETS subnormal_use_default subnormal_disable ARCHIVE
-   DESTINATION lib/clc )
+   DESTINATION ${CMAKE_INSTALL_DATADIR}/clc )
 endif()
 
 find_program( PYTHON python )
@@ -274,7 +275,7 @@ foreach( t ${LIBCLC_TARGETS_TO_BUILD} )
prepare_builtins )
add_custom_target( "prepare-${obj_suffix}" ALL
   DEPENDS "${obj_suffix}" )
-   install( FILES ${CMAKE_CURRENT_BINARY_DIR}/${obj_suffix} 
DESTINATION lib/clc )
+   install( FILES ${CMAKE_CURRENT_BINARY_DIR}/${obj_suffix} 
DESTINATION ${CMAKE_INSTALL_DATADIR}/clc )
# nvptx-- targets don't include workitem builtins
if( NOT ${t} MATCHES ".*ptx.*--$" )
add_test( NAME external-calls-${obj_suffix}
@@ -292,7 +293,7 @@ foreach( t ${LIBCLC_TARGETS_TO_BUILD} )
   create_symlink ${obj_suffix}
   ${alias_suffix}
   DEPENDS "prepare-${obj_suffix}" )
-   install( FILES 
${CMAKE_CURRENT_BINARY_DIR}/${alias_suffix} DESTINATION lib/clc )
+   install( FILES 
${CMAKE_CURRENT_BINARY_DIR}/${alias_suffix} DESTINATION 
${CMAKE_INSTALL_DATADIR}/clc )
endforeach( a )
endforeach( d )
 endforeach( t )

Modified: libclc/trunk/libclc.pc.in
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/libclc.pc.in?rev=350565&r1=350564&r2=350565&view=diff
==
--- libclc/trunk/libclc.pc.in (original)
+++ libclc/trunk/libclc.pc.in Mon Jan  7 12:20:37 2019
@@ -1,5 +1,5 @@
-includedir=@CMAKE_INSTALL_PREFIX@/include
-libexecdir=@CMAKE_INSTALL_PREFIX@/lib/clc
+includedir=@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_INCLUDEDIR@
+libexecdir=@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_DATADIR@/clc
 
 Name: libclc
 Description: Library requirements of the OpenCL C programming language


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

[PATCH] D56288: [ELF] Do not enable 'new dtags' on NetBSD by default

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

On NetBSD the 'new' semantics was already implemented with 'old' RPATH. I don't 
know the timing whether there already existed RUNPATH, but the result is that 
we never implemented it and never needed it.  The core of NetBSD was convinced 
to add an alias as it was very difficult in some examples (probably due to 
Rust) to rollback to 'old' RPATH.

I don't have any complains against lld developers, the only ones are against 
the NetBSD community that we are forced to be out of the lld development. It 
would be addressed already long time and probably RUNPATH wouldn't be leaked 
into executables shipped to NetBSD in the first place... it's counter-effective 
attitude to be in opposition to use lld on philosophical principle.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56288/new/

https://reviews.llvm.org/D56288



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

[PATCH] D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers

[John McCall via Phabricator via cfe-commits]

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

Thanks!  LGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55948/new/

https://reviews.llvm.org/D55948



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Ed Maste via Phabricator via cfe-commits]

emaste added a comment.

> There is a list of 140-150 unsafe (LLD_UNSAFE) packages with LLD.

Some of these are tagged with a PR or comment explaining why they are 
LLD_UNSAFE, but we haven't done it consistently. Some of these 140-150 were 
probably added in the early days of bringing lld into FreeBSD, and added 
without much thought - some could be as easy as differences in -ztext/-znotext 
default.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D56391: Limit COFF 'common' emission to <=32 alignment types.

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/CodeGen/CodeGenModule.cpp:3766
+  // in common.
+  if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF() &&
+  Context.getTypeAlignIfKnown(D->getType()) > 32)

I think this should be isKnownWindowsMSVCEnvironment


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56391/new/

https://reviews.llvm.org/D56391



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

[PATCH] D56354: [AST] Replace asserts with if() in SourceLocation accessors

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 180548.
steveire added a comment.

Fix formatting


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56354/new/

https://reviews.llvm.org/D56354

Files:
  include/clang/AST/DeclarationName.h
  include/clang/AST/TemplateBase.h
  lib/AST/NestedNameSpecifier.cpp


Index: lib/AST/NestedNameSpecifier.cpp
===
--- lib/AST/NestedNameSpecifier.cpp
+++ lib/AST/NestedNameSpecifier.cpp
@@ -462,9 +462,9 @@
 }
 
 TypeLoc NestedNameSpecifierLoc::getTypeLoc() const {
-  assert((Qualifier->getKind() == NestedNameSpecifier::TypeSpec ||
-  Qualifier->getKind() == NestedNameSpecifier::TypeSpecWithTemplate) &&
- "Nested-name-specifier location is not a type");
+  if (Qualifier->getKind() != NestedNameSpecifier::TypeSpec &&
+   Qualifier->getKind() != NestedNameSpecifier::TypeSpecWithTemplate)
+return TypeLoc();
 
   // The "void*" that points at the TypeLoc data.
   unsigned Offset = getDataLength(Qualifier->getPrefix());
Index: include/clang/AST/TemplateBase.h
===
--- include/clang/AST/TemplateBase.h
+++ include/clang/AST/TemplateBase.h
@@ -530,19 +530,22 @@
   }
 
   NestedNameSpecifierLoc getTemplateQualifierLoc() const {
-assert(Argument.getKind() == TemplateArgument::Template ||
-   Argument.getKind() == TemplateArgument::TemplateExpansion);
+if (Argument.getKind() != TemplateArgument::Template &&
+Argument.getKind() != TemplateArgument::TemplateExpansion)
+  return NestedNameSpecifierLoc();
 return LocInfo.getTemplateQualifierLoc();
   }
 
   SourceLocation getTemplateNameLoc() const {
-assert(Argument.getKind() == TemplateArgument::Template ||
-   Argument.getKind() == TemplateArgument::TemplateExpansion);
+if (Argument.getKind() != TemplateArgument::Template &&
+Argument.getKind() != TemplateArgument::TemplateExpansion)
+  return SourceLocation();
 return LocInfo.getTemplateNameLoc();
   }
 
   SourceLocation getTemplateEllipsisLoc() const {
-assert(Argument.getKind() == TemplateArgument::TemplateExpansion);
+if (Argument.getKind() != TemplateArgument::TemplateExpansion)
+  return SourceLocation();
 return LocInfo.getTemplateEllipsisLoc();
   }
 };
Index: include/clang/AST/DeclarationName.h
===
--- include/clang/AST/DeclarationName.h
+++ include/clang/AST/DeclarationName.h
@@ -729,9 +729,10 @@
   /// getNamedTypeInfo - Returns the source type info associated to
   /// the name. Assumes it is a constructor, destructor or conversion.
   TypeSourceInfo *getNamedTypeInfo() const {
-assert(Name.getNameKind() == DeclarationName::CXXConstructorName ||
-   Name.getNameKind() == DeclarationName::CXXDestructorName ||
-   Name.getNameKind() == DeclarationName::CXXConversionFunctionName);
+if (Name.getNameKind() != DeclarationName::CXXConstructorName &&
+Name.getNameKind() != DeclarationName::CXXDestructorName &&
+Name.getNameKind() != DeclarationName::CXXConversionFunctionName)
+  return nullptr;
 return LocInfo.NamedType.TInfo;
   }
 
@@ -747,7 +748,8 @@
   /// getCXXOperatorNameRange - Gets the range of the operator name
   /// (without the operator keyword). Assumes it is a (non-literal) operator.
   SourceRange getCXXOperatorNameRange() const {
-assert(Name.getNameKind() == DeclarationName::CXXOperatorName);
+if (Name.getNameKind() != DeclarationName::CXXOperatorName)
+  return SourceRange();
 return SourceRange(
  
SourceLocation::getFromRawEncoding(LocInfo.CXXOperatorName.BeginOpNameLoc),
  SourceLocation::getFromRawEncoding(LocInfo.CXXOperatorName.EndOpNameLoc)
@@ -766,7 +768,8 @@
   /// operator name (not the operator keyword).
   /// Assumes it is a literal operator.
   SourceLocation getCXXLiteralOperatorNameLoc() const {
-assert(Name.getNameKind() == DeclarationName::CXXLiteralOperatorName);
+if (Name.getNameKind() != DeclarationName::CXXLiteralOperatorName)
+  return SourceLocation();
 return SourceLocation::
   getFromRawEncoding(LocInfo.CXXLiteralOperatorName.OpNameLoc);
   }


Index: lib/AST/NestedNameSpecifier.cpp
===
--- lib/AST/NestedNameSpecifier.cpp
+++ lib/AST/NestedNameSpecifier.cpp
@@ -462,9 +462,9 @@
 }
 
 TypeLoc NestedNameSpecifierLoc::getTypeLoc() const {
-  assert((Qualifier->getKind() == NestedNameSpecifier::TypeSpec ||
-  Qualifier->getKind() == NestedNameSpecifier::TypeSpecWithTemplate) &&
- "Nested-name-specifier location is not a type");
+  if (Qualifier->getKind() != NestedNameSpecifier::TypeSpec &&
+   Qualifier->getKind() != NestedNameSpecifier::TypeSpecWithTemplate)
+return TypeLoc();
 
   // The "void*" that points at the TypeLoc data.
   unsigned Offset = ge

[PATCH] D56354: [AST] Replace asserts with if() in SourceLocation accessors

[Stephen Kelly via Phabricator via cfe-commits]

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



Comment at: include/clang/AST/DeclarationName.h:735
+   Name.getNameKind() != DeclarationName::CXXConversionFunctionName)
+  return nullptr;
 return LocInfo.NamedType.TInfo;

aaron.ballman wrote:
> Did you investigate the callers of this function to see which ones need a 
> null pointer check inserted into them? Otherwise, this change turns an 
> assertion into harder to track down UB. (I'm less worried about the other 
> changes because those will fail more gracefully.)
All callers in clang are fine. Here's the output of `git grep -h 
TypeSourceInfo`:

  /// getNamedTypeInfo - Returns the source type info associated to
  TypeSourceInfo *getNamedTypeInfo() const {
if (TypeSourceInfo *TSInfo = NameInfo.getNamedTypeInfo())
if (auto ToTInfoOrErr = import(From.getNamedTypeInfo()))
  if (auto TypeNameInfo = Dtor->getNameInfo().getNamedTypeInfo()) {
if (TypeSourceInfo *TSInfo = NameInfo.getNamedTypeInfo())
if (TypeSourceInfo *OldTInfo = NameInfo.getNamedTypeInfo()) {
if (TypeSourceInfo *TSInfo = Name.getNamedTypeInfo())

clang-tools-extra has no uses of it. Is there anywhere else to check?



Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56354/new/

https://reviews.llvm.org/D56354



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

r350568 - Recommit r350555 "[X86] Use funnel shift intrinsics for the VBMI2 vshld/vshrd builtins."

[Craig Topper via cfe-commits]

Author: ctopper
Date: Mon Jan  7 13:00:41 2019
New Revision: 350568

URL: http://llvm.org/viewvc/llvm-project?rev=350568&view=rev
Log:
Recommit r350555 "[X86] Use funnel shift intrinsics for the VBMI2 vshld/vshrd 
builtins."

The MSVC limit hit in AutoUpgrade.cpp has been worked around for now.

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/avx512vbmi2intrin.h
cfe/trunk/lib/Headers/avx512vlvbmi2intrin.h
cfe/trunk/test/CodeGen/avx512vbmi2-builtins.c
cfe/trunk/test/CodeGen/avx512vlvbmi2-builtins.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=350568&r1=350567&r2=350568&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Mon Jan  7 13:00:41 2019
@@ -1256,43 +1256,25 @@ TARGET_BUILTIN(__builtin_ia32_vpshldw128
 TARGET_BUILTIN(__builtin_ia32_vpshldw256, "V16sV16sV16sIi", "ncV:256:", 
"avx512vl,avx512vbmi2")
 TARGET_BUILTIN(__builtin_ia32_vpshldw512, "V32sV32sV32sIi", "ncV:512:", 
"avx512vbmi2")
 
-TARGET_BUILTIN(__builtin_ia32_vpshldvd128_mask, "V4iV4iV4iV4iUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvd256_mask, "V8iV8iV8iV8iUc", "ncV:256:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvd512_mask, "V16iV16iV16iV16iUs", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvq128_mask, "V2LLiV2LLiV2LLiV2LLiUc", 
"ncV:128:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvq256_mask, "V4LLiV4LLiV4LLiV4LLiUc", 
"ncV:256:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvq512_mask, "V8LLiV8LLiV8LLiV8LLiUc", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvw128_mask, "V8sV8sV8sV8sUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvw256_mask, "V16sV16sV16sV16sUs", 
"ncV:256:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvw512_mask, "V32sV32sV32sV32sUi", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvd128_maskz, "V4iV4iV4iV4iUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvd256_maskz, "V8iV8iV8iV8iUc", "ncV:256:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvd512_maskz, "V16iV16iV16iV16iUs", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvq128_maskz, "V2LLiV2LLiV2LLiV2LLiUc", 
"ncV:128:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvq256_maskz, "V4LLiV4LLiV4LLiV4LLiUc", 
"ncV:256:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvq512_maskz, "V8LLiV8LLiV8LLiV8LLiUc", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvw128_maskz, "V8sV8sV8sV8sUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvw256_maskz, "V16sV16sV16sV16sUs", 
"ncV:256:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshldvw512_maskz, "V32sV32sV32sV32sUi", 
"ncV:512:", "avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvd128, "V4iV4iV4iV4i", "ncV:128:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvd256, "V8iV8iV8iV8i", "ncV:256:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvd512, "V16iV16iV16iV16i", "ncV:512:", 
"avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvq128, "V2LLiV2LLiV2LLiV2LLi", "ncV:128:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvq256, "V4LLiV4LLiV4LLiV4LLi", "ncV:256:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvq512, "V8LLiV8LLiV8LLiV8LLi", "ncV:512:", 
"avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvw128, "V8sV8sV8sV8s", "ncV:128:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvw256, "V16sV16sV16sV16s", "ncV:256:", 
"avx512vl,avx512vbmi2")
+TARGET_BUILTIN(__builtin_ia32_vpshldvw512, "V32sV32sV32sV32s", "ncV:512:", 
"avx512vbmi2")
 
-TARGET_BUILTIN(__builtin_ia32_vpshrdvd128_mask, "V4iV4iV4iV4iUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvd256_mask, "V8iV8iV8iV8iUc", "ncV:256:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvd512_mask, "V16iV16iV16iV16iUs", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvq128_mask, "V2LLiV2LLiV2LLiV2LLiUc", 
"ncV:128:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvq256_mask, "V4LLiV4LLiV4LLiV4LLiUc", 
"ncV:256:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvq512_mask, "V8LLiV8LLiV8LLiV8LLiUc", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvw128_mask, "V8sV8sV8sV8sUc", "ncV:128:", 
"avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvw256_mask, "V16sV16sV16sV16sUs", 
"ncV:256:", "avx512vl,avx512vbmi2")
-TARGET_BUILTIN(__builtin_ia32_vpshrdvw512_mask, "V32sV32sV32sV32sUi", 
"ncV:512:", "avx512vbmi2")
-TARGET_BUILTIN(__built

[PATCH] D56354: [AST] Replace asserts with if() in SourceLocation accessors

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a formatting nit.




Comment at: include/clang/AST/DeclarationName.h:735
+   Name.getNameKind() != DeclarationName::CXXConversionFunctionName)
+  return nullptr;
 return LocInfo.NamedType.TInfo;

steveire wrote:
> aaron.ballman wrote:
> > Did you investigate the callers of this function to see which ones need a 
> > null pointer check inserted into them? Otherwise, this change turns an 
> > assertion into harder to track down UB. (I'm less worried about the other 
> > changes because those will fail more gracefully.)
> All callers in clang are fine. Here's the output of `git grep -h 
> TypeSourceInfo`:
> 
>   /// getNamedTypeInfo - Returns the source type info associated to
>   TypeSourceInfo *getNamedTypeInfo() const {
> if (TypeSourceInfo *TSInfo = NameInfo.getNamedTypeInfo())
> if (auto ToTInfoOrErr = import(From.getNamedTypeInfo()))
>   if (auto TypeNameInfo = Dtor->getNameInfo().getNamedTypeInfo()) {
> if (TypeSourceInfo *TSInfo = NameInfo.getNamedTypeInfo())
> if (TypeSourceInfo *OldTInfo = NameInfo.getNamedTypeInfo()) {
> if (TypeSourceInfo *TSInfo = Name.getNamedTypeInfo())
> 
> clang-tools-extra has no uses of it. Is there anywhere else to check?
> 
I double-checked the call to `import()` and it gracefully handles null pointers 
as well, so I think this change is safe (I cannot find any further instances 
that you didn't find). Thank you for looking into it!



Comment at: lib/AST/NestedNameSpecifier.cpp:465
 TypeLoc NestedNameSpecifierLoc::getTypeLoc() const {
-  assert((Qualifier->getKind() == NestedNameSpecifier::TypeSpec ||
-  Qualifier->getKind() == NestedNameSpecifier::TypeSpecWithTemplate) &&
- "Nested-name-specifier location is not a type");
+  if ((Qualifier->getKind() != NestedNameSpecifier::TypeSpec &&
+  Qualifier->getKind() != NestedNameSpecifier::TypeSpecWithTemplate))

aaron.ballman wrote:
> Remove spurious parens.
Formatting is still off here for the second line of the if statement (one too 
many spaces on the indentation).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56354/new/

https://reviews.llvm.org/D56354



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

[PATCH] D56407: Implement the TreeStructure interface through the TextNodeDumper

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added reviewers: aaron.ballman, erichkeane.
Herald added a subscriber: cfe-commits.

This way, when the generic ASTTraverser is extracted from ASTDumper,
there can't be any problem related to ordering of class members, a
concern that was raised in https://reviews.llvm.org/D55337.

This will also preserve the property that the generic traverser does not
enforce any coupling between the NodeDumper and the TreeStructure.

https://godbolt.org/z/PEtT1_


Repository:
  rC Clang

https://reviews.llvm.org/D56407

Files:
  include/clang/AST/ASTDumperUtils.h
  include/clang/AST/TextNodeDumper.h
  lib/AST/ASTDumper.cpp
  lib/AST/TextNodeDumper.cpp

Index: lib/AST/TextNodeDumper.cpp
===
--- lib/AST/TextNodeDumper.cpp
+++ lib/AST/TextNodeDumper.cpp
@@ -19,8 +19,8 @@
const SourceManager *SM,
const PrintingPolicy &PrintPolicy,
const comments::CommandTraits *Traits)
-: OS(OS), ShowColors(ShowColors), SM(SM), PrintPolicy(PrintPolicy),
-  Traits(Traits) {}
+: TextTreeStructure(OS, ShowColors), OS(OS), ShowColors(ShowColors), SM(SM),
+  PrintPolicy(PrintPolicy), Traits(Traits) {}
 
 void TextNodeDumper::Visit(const comments::Comment *C,
const comments::FullComment *FC) {
Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -44,7 +44,6 @@
 public ConstCommentVisitor,
 public TypeVisitor {
 
-TextTreeStructure TreeStructure;
 TextNodeDumper NodeDumper;
 
 raw_ostream &OS;
@@ -60,7 +59,7 @@
 
 /// Dump a child of the current node.
 template void dumpChild(Fn doDumpChild) {
-  TreeStructure.addChild(doDumpChild);
+  NodeDumper.addChild(doDumpChild);
 }
 
   public:
@@ -75,8 +74,7 @@
 ASTDumper(raw_ostream &OS, const CommandTraits *Traits,
   const SourceManager *SM, bool ShowColors,
   const PrintingPolicy &PrintPolicy)
-: TreeStructure(OS, ShowColors),
-  NodeDumper(OS, ShowColors, SM, PrintPolicy, Traits), OS(OS),
+: NodeDumper(OS, ShowColors, SM, PrintPolicy, Traits), OS(OS),
   PrintPolicy(PrintPolicy), ShowColors(ShowColors) {}
 
 void setDeserialize(bool D) { Deserialize = D; }
Index: include/clang/AST/TextNodeDumper.h
===
--- include/clang/AST/TextNodeDumper.h
+++ include/clang/AST/TextNodeDumper.h
@@ -22,9 +22,94 @@
 
 namespace clang {
 
+class TextTreeStructure {
+  raw_ostream &OS;
+  const bool ShowColors;
+
+  /// Pending[i] is an action to dump an entity at level i.
+  llvm::SmallVector, 32> Pending;
+
+  /// Indicates whether we're at the top level.
+  bool TopLevel = true;
+
+  /// Indicates if we're handling the first child after entering a new depth.
+  bool FirstChild = true;
+
+  /// Prefix for currently-being-dumped entity.
+  std::string Prefix;
+
+public:
+  /// Add a child of the current node.  Calls doAddChild without arguments
+  template  void addChild(Fn doAddChild) {
+// If we're at the top level, there's nothing interesting to do; just
+// run the dumper.
+if (TopLevel) {
+  TopLevel = false;
+  doAddChild();
+  while (!Pending.empty()) {
+Pending.back()(true);
+Pending.pop_back();
+  }
+  Prefix.clear();
+  OS << "\n";
+  TopLevel = true;
+  return;
+}
+
+auto dumpWithIndent = [this, doAddChild](bool isLastChild) {
+  // Print out the appropriate tree structure and work out the prefix for
+  // children of this node. For instance:
+  //
+  //   APrefix = ""
+  //   |-B  Prefix = "| "
+  //   | `-CPrefix = "|   "
+  //   `-D  Prefix = "  "
+  // |-EPrefix = "  | "
+  // `-FPrefix = ""
+  //   GPrefix = ""
+  //
+  // Note that the first level gets no prefix.
+  {
+OS << '\n';
+ColorScope Color(OS, ShowColors, IndentColor);
+OS << Prefix << (isLastChild ? '`' : '|') << '-';
+this->Prefix.push_back(isLastChild ? ' ' : '|');
+this->Prefix.push_back(' ');
+  }
+
+  FirstChild = true;
+  unsigned Depth = Pending.size();
+
+  doAddChild();
+
+  // If any children are left, they're the last at their nesting level.
+  // Dump those ones out now.
+  while (Depth < Pending.size()) {
+Pending.back()(true);
+this->Pending.pop_back();
+  }
+
+  // Restore the old prefix.
+  this->Prefix.resize(Prefix.size() - 2);
+};
+
+if (FirstChild) {
+  Pending.push_back(std::move(dumpWithIndent));
+} else {
+  Pending.back()(false);
+  Pending.back() = std::move(dumpWithIndent);
+}
+FirstChild = false;
+  }
+
+  TextTreeStr

[PATCH] D55337: NFC: Move dumpDeclRef to NodeDumper

[Stephen Kelly via Phabricator via cfe-commits]

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



Comment at: include/clang/AST/TextNodeDumper.h:28
const comments::FullComment *> {
+  TextTreeStructure &TreeStructure;
   raw_ostream &OS;

steveire wrote:
> steveire wrote:
> > aaron.ballman wrote:
> > > steveire wrote:
> > > > aaron.ballman wrote:
> > > > > This makes me a bit wary because you create a node dumper in the same 
> > > > > situations you make a tree structure object, but now there's a strict 
> > > > > ordering between the two object creations. If you're doing this 
> > > > > construction local to a function, you wind up with a dangling 
> > > > > reference unless you're careful (which is unfortunate, but not the 
> > > > > end of the world). If you're doing this construction as part of a 
> > > > > constructor's initializer list, you now have to properly order the 
> > > > > member declarations within the class and that is also unfortunate. 
> > > > > Given that those are the two common scenarios for how I envision 
> > > > > constructing an ast dump of some kind, I worry about the fragility. 
> > > > > e.g.,
> > > > > ```
> > > > > unique_ptr createASTDumper(...) {
> > > > >   TextTreeStructure TreeStructure;
> > > > >   TextNodeDumper NodeDumper(TreeStructure); // Oops, dangling 
> > > > > reference
> > > > >   return make_unique(TreeStructure, 
> > > > > NodeDumper, ...);
> > > > > }
> > > > > 
> > > > > // vs
> > > > > 
> > > > > struct MySuperAwesomeASTDumper : ... {
> > > > >   MySuperAwesomeASTDumper() : TreeStructure(...), 
> > > > > NodeDumper(TreeStructure, ...) {}
> > > > > private:
> > > > >   TextTreeStructure TreeStructure; // This order is now SUPER 
> > > > > important
> > > > >   TextNodeDumper NodeDumper;
> > > > > };
> > > > > ```
> > > > > There's a part of me that wonders if a better approach is to have 
> > > > > this object passed to the `dumpFoo()` calls as a reference parameter. 
> > > > > This way, the caller is still responsible for creating an object, but 
> > > > > the creation order between the tree and the node dumper isn't as 
> > > > > fragile.
> > > > In your first snippet there is a dangling reference because the author 
> > > > of `MySuperAwesomeASTDumper` decided to make the members references. If 
> > > > the members are references, code like your first snippet will cause 
> > > > dangling references and nothing can prevent that. Adding 
> > > > `TreeStructure&` to Visit methods as you suggested does not prevent it.
> > > > 
> > > > The only solution is make the `MySuperAwesomeASTDumper` not use member 
> > > > references (ie your second snippet). The order is then in fact not 
> > > > problematic because "taking a reference to an uninitialized object is 
> > > > legal".
> > > >  The order is then in fact not problematic because "taking a reference 
> > > > to an uninitialized object is legal".
> > > 
> > > This presumes that the constructors aren't using those references to the 
> > > uninitialized object, which would be illegal. That's what I mean about 
> > > this being very fragile -- if the stars line up correctly, everything 
> > > works fine, but if the stars aren't aligned just right, you get really 
> > > hard problems to track down.
> > Actually 'the stars would have to line up in a particular way' in order to 
> > reach the scenario you are concerned about. What would have to happen is:
> > 
> > * This patch gets in as-is
> > * Someone in the future reorders the members 
> > * But they don't reorder the init-list
> > * They build on a platform without -Wreorder (only MSVC?) enabled in the 
> > build.
> > * That passes review
> > * Other users update their checkout and everyone else also ignores the 
> > -Wreorder warning.
> > 
> > That is a vanishingly likely scenario. It's just unreasonable to consider 
> > that as a reason to create a broken interface.
> > 
> > And it would be a broken interface.
> > 
> > After the refactoring is complete, we have something like 
> > 
> > ```
> > class ASTDumper
> > : public ASTDumpTraverser 
> > {
> >   TextTreeStructure TreeStructure;
> >   TextNodeDumper NodeDumper;
> > public:
> >   TextTreeStructure &getTreeStructure() { return TreeStructure; }
> >   TextNodeDumper &getNodeDumper() { return NodeDumper; }
> > 
> >   ASTDumper(raw_ostream &OS, const SourceManager *SM)
> >   : TreeStructure(OS),
> > NodeDumper(TreeStructure, OS, SM) {}
> > };
> > 
> > ```
> > 
> > In the case, of the `ASTDumper`, the `TextNodeDumper` uses the 
> > `TextTreeStructure`.
> > 
> > However, in the case of any other subclass of `ASTDumpTraverser`, the 
> > `NodeDumper` type does not depend on the `TreeStructure` type. The 
> > `ASTDumpTraverser` should not pass the `TreeStructure` to the 
> > `NodeDumper`because the `ASTDumpTraverser` should not assume the 
> > `NodeDumper` needs the `ASTDumpTraverser`. 
> > 
> > That is true in one concrete case (the `TextNo

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF marked 13 inline comments as done.
EricWF added a comment.

Address review comments. Updated patch coming shortly.




Comment at: include/clang/AST/Expr.h:4147
+  SourceLocation getBeginLoc() const LLVM_READONLY { return BuiltinLoc; }
+  SourceLocation getEndLoc() const LLVM_READONLY { return RParenLoc; }
+

riccibruno wrote:
> I don't think that `LLVM_READONLY` is useful here since it is just a simple 
> getter.
> Same remark for `getIdentType`, `isStringType` and `isIntType`.
> And even for `getBuiltinStr` does it actually make a difference ?
It probably doesn't make any difference. Removing.



Comment at: lib/AST/ASTContext.cpp:10145
+   unsigned Length) const {
+  // A C++ string literal has a const-qualified element type (C++ 2.13.4p1).
+  EltTy = EltTy.withConst();

riccibruno wrote:
> And what about C ? It seems to me that `getStringLiteralArrayType`
> should be used systematically when the type of a string literal is needed.
> 
> (eg in `ActOnStringLiteral` but this is not the only place...)
It should be. I'll fix it to act like `ActOnStringLiteral` and then deduplicate 
the code.



Comment at: lib/AST/Expr.cpp:1961
+// A C++ string literal has a const-qualified element type (C++ 2.13.4p1).
+if (Ctx.getLangOpts().CPlusPlus || Ctx.getLangOpts().ConstStrings)
+  Ty = Ty.withConst();

riccibruno wrote:
> Is it taking into account `adjustStringLiteralBaseType` as in 
> `getStringLiteralArrayType` ?
Nope. Fixed.



Comment at: lib/AST/Expr.cpp:2025
+  // If we have a simple identifier there is no need to cache the
+  // human readable name.
+  if (IdentifierInfo *II = FD->getDeclName().getAsIdentifierInfo())

riccibruno wrote:
> This comment is strange since MkStr will call 
> `getPredefinedStringLiteralFromCache`
> when `FD->getDeclName()` is a simple identifier.
I think this should say `compute` instead of `cache`.



Comment at: lib/AST/Expr.cpp:2037
+llvm::APSInt IntVal(
+llvm::APInt(Ctx.getTargetInfo().getIntWidth(), LineOrCol));
+return APValue(IntVal);

riccibruno wrote:
> Both `getLine`, `getColumn` and `APInt` take an unsigned.
What's your objection here? That I used `int64_t` or `getIntWidth()`? I've 
reworked this bit of code. Let me know if it's still problematic.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D37035/new/

https://reviews.llvm.org/D37035



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

r350571 - [OPENMP]Add call to __kmpc_push_target_tripcount() function.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Jan  7 13:30:43 2019
New Revision: 350571

URL: http://llvm.org/viewvc/llvm-project?rev=350571&view=rev
Log:
[OPENMP]Add call to __kmpc_push_target_tripcount() function.

Each we create the target regions with the teams distribute inner
region, we can better estimate number of the teams required to execute
the target region. Function __kmpc_push_target_tripcount() is used for
purpose, which accepts device_id and the number of the iterations,
performed by the associated loop.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_if_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_if_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=350571&r1=350570&r2=350571&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Mon Jan  7 13:30:43 2019
@@ -673,6 +673,9 @@ enum OpenMPRTLFunction {
   //
   // Offloading related calls
   //
+  // Call to void __kmpc_push_target_tripcount(int64_t device_id, kmp_uint64
+  // size);
+  OMPRTL__kmpc_push_target_tripcount,
   // Call to int32_t __tgt_target(int64_t device_id, void *host_ptr, int32_t
   // arg_num, void** args_base, void **args, size_t *arg_sizes, int64_t
   // *arg_types);
@@ -2163,6 +2166,15 @@ CGOpenMPRuntime::createRuntimeFunction(u
 FnTy, /*Name=*/"__kmpc_task_reduction_get_th_data");
 break;
   }
+  case OMPRTL__kmpc_push_target_tripcount: {
+// Build void __kmpc_push_target_tripcount(int64_t device_id, kmp_uint64
+// size);
+llvm::Type *TypeParams[] = {CGM.Int64Ty, CGM.Int64Ty};
+llvm::FunctionType *FnTy =
+llvm::FunctionType::get(CGM.VoidTy, TypeParams, /*isVarArg=*/false);
+RTLFn = CGM.CreateRuntimeFunction(FnTy, "__kmpc_push_target_tripcount");
+break;
+  }
   case OMPRTL__tgt_target: {
 // Build int32_t __tgt_target(int64_t device_id, void *host_ptr, int32_t
 // arg_num, void** args_base, void **args, size_t *arg_sizes, int64_t
@@ -8053,6 +8065,183 @@ static void emitOffloadingArraysArgument
   }
 }
 
+/// Checks if the expression is constant or does not have non-trivial function
+/// calls.
+static bool isTrivial(ASTContext &Ctx, const Expr * E) {
+  // We can skip constant expressions.
+  // We can skip expressions with trivial calls or simple expressions.
+  return (E->isEvaluatable(Ctx, Expr::SE_AllowUndefinedBehavior) ||
+  !E->hasNonTrivialCall(Ctx)) &&
+ !E->HasSideEffects(Ctx, /*IncludePossibleEffects=*/true);
+}
+
+/// Checks if the \p Body is the \a CompoundStmt and returns its child 
statement
+/// iff there is only one that is not evaluatable at the compile time.
+static const Stmt *getSingleCompoundChild(ASTContext &Ctx, const Stmt *Body) {
+  if (const auto *C = dyn_cast(Body)) {
+const Stmt *Child = nullptr;
+for (const Stmt *S : C->body()) {
+  if (const auto *E = dyn_cast(S)) {
+if (isTrivial(Ctx, E))
+  continue;
+  }
+  // Some of the statements can be ignored.
+  if (isa(S) || isa(S) || isa(S) ||
+  isa(S) || isa(S))
+continue;
+  // Analyze declarations.
+  if (const auto *DS = dyn_cast(S)) {
+if (llvm::all_of(DS->decls(), [&Ctx](const Decl *D) {
+  if (isa(D) || isa(D) ||
+  isa(D) || isa(D) ||
+  isa(D) || isa(D) ||
+  isa(D) ||
+  isa(D) ||
+  isa(D))
+return true;
+  const auto *VD = dyn_cast(D);
+  if (!VD)
+return false;
+  return VD->isConstexpr() ||
+ ((VD->getType().isTrivialType(Ctx) ||
+   VD->getType()->isReferenceType()) &&
+  (!VD->hasInit() || isTrivial(Ctx, VD->getInit(;
+}))
+  continue;
+  }
+  // Found multiple children - cannot get the one child only.
+  if (Child)
+return Body;
+  Child = S;
+}
+if (Child)
+  return Child;
+  }
+  return Body;
+}
+
+/// Check for inner distribute directive.
+static const OMPExecutableDirective *
+getNestedDistributeDirective(ASTContext &Ctx, const OMPExecutableDirective &

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai updated this revision to Diff 180556.
vsapsai added a comment.

- Test initializing vector of `unsigned int` with array of `int`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D48342/new/

https://reviews.llvm.org/D48342

Files:
  libcxx/include/memory
  
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp

Index: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp
===
--- libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp
+++ libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp
@@ -146,9 +146,29 @@
 #endif
 }
 
+// Initialize a vector with a different value type.
+void test_ctor_with_different_value_type() {
+  {
+// Make sure initialization is performed with each element value, not with
+// a memory blob.
+float array[3] = {0.0f, 1.0f, 2.0f};
+std::vector v(array, array + 3);
+assert(v[0] == 0);
+assert(v[1] == 1);
+assert(v[2] == 2);
+  }
+  {
+// Though types are different, initialization can be done with `memcpy`.
+int32_t array[1] = {-1};
+std::vector v(array, array + 1);
+assert(v[0] == 4294967295);
+  }
+}
+
 
 int main() {
   basic_test_cases();
   emplaceable_concept_tests(); // See PR34898
   test_ctor_under_alloc();
+  test_ctor_with_different_value_type();
 }
Index: libcxx/include/memory
===
--- libcxx/include/memory
+++ libcxx/include/memory
@@ -1502,6 +1502,12 @@
 typedef typename _Alloc::difference_type type;
 };
 
+template 
+struct __is_default_allocator : false_type {};
+
+template 
+struct __is_default_allocator<_VSTD::allocator<_Tp> > : true_type {};
+
 template 
 struct _LIBCPP_TEMPLATE_VIS allocator_traits
 {
@@ -1615,7 +1621,7 @@
 static
 typename enable_if
 <
-(is_same >::value
+(__is_default_allocator::value
 || !__has_construct::value) &&
  is_trivially_move_constructible<_Tp>::value,
 void
@@ -1640,23 +1646,25 @@
 construct(__a, _VSTD::__to_raw_pointer(__begin2), *__begin1);
 }
 
-template 
+template ::type,
+  class _RawDestTp = typename remove_const<_DestTp>::type>
 _LIBCPP_INLINE_VISIBILITY
 static
 typename enable_if
 <
-(is_same >::value
-|| !__has_construct::value) &&
- is_trivially_move_constructible<_Tp>::value,
+is_trivially_move_constructible<_DestTp>::value &&
+is_same<_RawSourceTp, _RawDestTp>::value &&
+(__is_default_allocator::value ||
+ !__has_construct::value),
 void
 >::type
-__construct_range_forward(allocator_type&, _Tp* __begin1, _Tp* __end1, _Tp*& __begin2)
+__construct_range_forward(allocator_type&, _SourceTp* __begin1, _SourceTp* __end1, _DestTp*& __begin2)
 {
-typedef typename remove_const<_Tp>::type _Vp;
 ptrdiff_t _Np = __end1 - __begin1;
 if (_Np > 0)
 {
-_VSTD::memcpy(const_cast<_Vp*>(__begin2), __begin1, _Np * sizeof(_Tp));
+_VSTD::memcpy(const_cast<_RawDestTp*>(__begin2), __begin1, _Np * sizeof(_DestTp));
 __begin2 += _Np;
 }
 }
@@ -1679,7 +1687,7 @@
 static
 typename enable_if
 <
-(is_same >::value
+(__is_default_allocator::value
 || !__has_construct::value) &&
  is_trivially_move_constructible<_Tp>::value,
 void
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: 
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:161
+  {
+// Though types are different, initialization can be done with `memcpy`.
+int32_t array[1] = {-1};

I might add "can be, but currently isn't, done with memcpy."

Here's my other suggested test:

```
struct X { int x; };
struct Y { int y; };
struct Z : X, Y { int z; };
{
Z z;
Z *array[1] = { &z };
// Though the types Z* and Y* are very similar, initialization still cannot 
be done with memcpy.
std::vector v(array, array + 1);
assert(v[0] == &z);
}
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D48342/new/

https://reviews.llvm.org/D48342



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

[PATCH] D56410: Forbid combination of --status-bugs and non-HTML output

[Rob Day via Phabricator via cfe-commits]

rkday created this revision.
rkday added a reviewer: dcoughlin.
Herald added a subscriber: cfe-commits.

I recently spent some time resolving an issue where the --status-bugs parameter 
conflicted with -plist, and scan-build always returned 0 even when there were 
errors.

This change means that scan-build rejects these as incompatible arguments.

I've tested that this works with this test program:

  $ cat test.c
  int abcd(char* x, char* y)
  {
strcpy(x, y);
  }
  
  int main()
  {
return 0;
  }

and these two commands:

  ./bin/scan-build --status-bugs -enable-checker security --use-analyzer 
/usr/bin/clang clang test.c; echo $?



  ./bin/scan-build --status-bugs -plist -enable-checker security --use-analyzer 
/usr/bin/clang clang test.c; echo $?


Repository:
  rC Clang

https://reviews.llvm.org/D56410

Files:
  bin/scan-build


Index: bin/scan-build
===
--- bin/scan-build
+++ bin/scan-build
@@ -1849,6 +1849,10 @@
   DieDiag("'c++-analyzer' does not exist at '$CmdCXX'\n") if(! -e $CmdCXX);
 }
 
+if ($Options{ExitStatusFoundBugs} == 1 && !($Options{OutputFormat} =~ /html/)) 
{
+  DieDiag("--status-bugs is only supported with the HTML output format\n");
+}
+
 Diag("Using '$Clang' for static analysis\n");
 
 SetHtmlEnv(\@ARGV, $Options{OutputDir});


Index: bin/scan-build
===
--- bin/scan-build
+++ bin/scan-build
@@ -1849,6 +1849,10 @@
   DieDiag("'c++-analyzer' does not exist at '$CmdCXX'\n") if(! -e $CmdCXX);
 }
 
+if ($Options{ExitStatusFoundBugs} == 1 && !($Options{OutputFormat} =~ /html/)) {
+  DieDiag("--status-bugs is only supported with the HTML output format\n");
+}
+
 Diag("Using '$Clang' for static analysis\n");
 
 SetHtmlEnv(\@ARGV, $Options{OutputDir});
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r350572 - Add a __has_feature check for namespaces on #pragma clang attribute.

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Mon Jan  7 13:54:00 2019
New Revision: 350572

URL: http://llvm.org/viewvc/llvm-project?rev=350572&view=rev
Log:
Add a __has_feature check for namespaces on #pragma clang attribute.

Support for this was added in r349845.

Modified:
cfe/trunk/docs/LanguageExtensions.rst
cfe/trunk/include/clang/Basic/Features.def
cfe/trunk/test/Sema/pragma-attribute-namespace.c

Modified: cfe/trunk/docs/LanguageExtensions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=350572&r1=350571&r2=350572&view=diff
==
--- cfe/trunk/docs/LanguageExtensions.rst (original)
+++ cfe/trunk/docs/LanguageExtensions.rst Mon Jan  7 13:54:00 2019
@@ -2725,7 +2725,9 @@ same namespace. For instance:
 Without the namespaces on the macros, ``other_function`` will be annotated with
 ``[[noreturn]]`` instead of ``__attribute__((unavailable))``. This may seem 
like
 a contrived example, but its very possible for this kind of situation to appear
-in real code if the pragmas are spread out across a large file.
+in real code if the pragmas are spread out across a large file. You can test if
+your version of clang supports namespaces on ``#pragma clang attribute`` with
+``__has_feature(pragma_clang_attribute_namespaces)``.
 
 Subject Match Rules
 ---

Modified: cfe/trunk/include/clang/Basic/Features.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Features.def?rev=350572&r1=350571&r2=350572&view=diff
==
--- cfe/trunk/include/clang/Basic/Features.def (original)
+++ cfe/trunk/include/clang/Basic/Features.def Mon Jan  7 13:54:00 2019
@@ -69,6 +69,7 @@ FEATURE(attribute_overloadable, true)
 FEATURE(attribute_unavailable_with_message, true)
 FEATURE(attribute_unused_on_fields, true)
 FEATURE(attribute_diagnose_if_objc, true)
+FEATURE(pragma_clang_attribute_namespaces, true)
 FEATURE(blocks, LangOpts.Blocks)
 FEATURE(c_thread_safety_attributes, true)
 FEATURE(cxx_exceptions, LangOpts.CXXExceptions)

Modified: cfe/trunk/test/Sema/pragma-attribute-namespace.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/pragma-attribute-namespace.c?rev=350572&r1=350571&r2=350572&view=diff
==
--- cfe/trunk/test/Sema/pragma-attribute-namespace.c (original)
+++ cfe/trunk/test/Sema/pragma-attribute-namespace.c Mon Jan  7 13:54:00 2019
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
+#if !__has_feature(pragma_clang_attribute_namespaces)
+#error
+#endif
+
 #pragma clang attribute MyNamespace.push (__attribute__((annotate)), 
apply_to=function) // expected-error 2 {{'annotate' attribute}}
 
 int some_func(); // expected-note{{when applied to this declaration}}


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

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: 
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:196
   test_ctor_under_alloc();
+  test_ctor_with_different_value_type();
 }

Quuxplusone wrote:
> vsapsai wrote:
> > Quuxplusone wrote:
> > > I suggest that interesting test cases include "array of `int` to vector 
> > > of `unsigned int`" (trivial, but unimplemented in this patch) and "array 
> > > of `iostream*` to vector of `ostream*`" (non-trivial because each pointer 
> > > must be adjusted).
> > What is that supposed to test? My `float/int` test is to make sure we have 
> > `is_same<_RawSourceTp, _RawDestTp>::value` and don't try to `memcpy` 
> > unrelated types. I've chosen `float` and `int` because it is easier for a 
> > human to reason about them.
> > 
> > `int` and `unsigned int` are interested for testing for values that are 
> > outside of common range. But in this case you pay more attention to 
> > conversion between ints, not to the behaviour of the constructor. That's my 
> > interpretation but maybe I've missed some of your intentions.
> > My `float/int` test is to make sure we [...] don't try to `memcpy` 
> > unrelated types [when it's unsafe to do so].
> 
> Right. I suggest two additional tests. The first additional test, 
> `int/unsigned int`, is to verify whether we `memcpy` unrelated types when it 
> //is// safe to do so. The second test, `iostream*/ostream*`, is to verify 
> whether we `memcpy` unrelated types when it is unsafe to `memcpy` them even 
> though they are both of the same "kind" (both pointer types).
> 
> These will act as regression tests, to make sure that future changes to the 
> memcpy criteria don't break these cases' behavior (although the first 
> additional test //is// expected to get more efficient).
Added test for `int32_t/uint32_t`. Size is mentioned explicitly to avoid 
surprises with testing on different architectures. Tested that such 
initialization isn't using `memcpy` and still slow.

Do you know ways to verify pointer adjustment for `iostream*/ostream*` using 
their public API? Because I found a way to do that with accessing 
`vector::data()` and mucking with pointers. But I don't like this approach 
because it seems brittle and specific to vector implementation. I'd rather use 
stable public API. Currently I don't plan to invest much effort into 
`iostream*/ostream*` test because I agree it is nice to have but `is_same` part 
is already covered.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D48342/new/

https://reviews.llvm.org/D48342



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