[PATCH] D48628: [AST] Structural equivalence of methods

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin accepted this revision.
a_sidorin added a comment.

LG with a nit. Thank you!




Comment at: unittests/AST/StructuralEquivalenceTest.cpp:489
+
+TEST_F(StructuralEquivalenceRecordTest, DISABLED_Methods) {
+  auto t = makeNamedDecls(

Could you add a comment why this test is disabled?


Repository:
  rC Clang

https://reviews.llvm.org/D48628



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

[PATCH] D48044: [Power9] Update fp128 as a valid homogenous aggregate base type

[Lei Huang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL336308: [Power9] Update fp128 as a valid homogenous 
aggregate base type (authored by lei, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D48044?vs=150821=154166#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D48044

Files:
  cfe/trunk/include/clang/AST/Type.h
  cfe/trunk/lib/CodeGen/TargetInfo.cpp
  cfe/trunk/test/CodeGen/ppc64le-f128Aggregates.c

Index: cfe/trunk/include/clang/AST/Type.h
===
--- cfe/trunk/include/clang/AST/Type.h
+++ cfe/trunk/include/clang/AST/Type.h
@@ -1807,6 +1807,7 @@
   bool isFloatingType() const; // C99 6.2.5p11 (real floating + complex)
   bool isHalfType() const; // OpenCL 6.1.1.1, NEON (IEEE 754-2008 half)
   bool isFloat16Type() const;  // C11 extension ISO/IEC TS 18661
+  bool isFloat128Type() const;
   bool isRealType() const; // C99 6.2.5p17 (real floating + integer)
   bool isArithmeticType() const;   // C99 6.2.5p18 (integer + floating)
   bool isVoidType() const; // C99 6.2.5p19
@@ -6299,6 +6300,12 @@
   return false;
 }
 
+inline bool Type::isFloat128Type() const {
+  if (const auto *BT = dyn_cast(CanonicalType))
+return BT->getKind() == BuiltinType::Float128;
+  return false;
+}
+
 inline bool Type::isNullPtrType() const {
   if (const auto *BT = getAs())
 return BT->getKind() == BuiltinType::NullPtr;
Index: cfe/trunk/test/CodeGen/ppc64le-f128Aggregates.c
===
--- cfe/trunk/test/CodeGen/ppc64le-f128Aggregates.c
+++ cfe/trunk/test/CodeGen/ppc64le-f128Aggregates.c
@@ -0,0 +1,124 @@
+// RUN: %clang_cc1 -triple powerpc64le-unknown-linux-gnu -emit-llvm \
+// RUN:   -target-cpu pwr9 -target-feature +float128 -o - %s | FileCheck %s
+
+// Test homogeneous fp128 aggregate passing and returning.
+
+struct fp1 { __float128 f[1]; };
+struct fp2 { __float128 f[2]; };
+struct fp3 { __float128 f[3]; };
+struct fp4 { __float128 f[4]; };
+struct fp5 { __float128 f[5]; };
+struct fp6 { __float128 f[6]; };
+struct fp7 { __float128 f[7]; };
+struct fp8 { __float128 f[8]; };
+struct fp9 { __float128 f[9]; };
+
+struct fpab { __float128 a; __float128 b; };
+struct fpabc { __float128 a; __float128 b; __float128 c; };
+
+struct fp2a2b { __float128 a[2]; __float128 b[2]; };
+
+// CHECK: define [1 x fp128] @func_f1(fp128 inreg %x.coerce)
+struct fp1 func_f1(struct fp1 x) { return x; }
+
+// CHECK: define [2 x fp128] @func_f2([2 x fp128] %x.coerce)
+struct fp2 func_f2(struct fp2 x) { return x; }
+
+// CHECK: define [3 x fp128] @func_f3([3 x fp128] %x.coerce)
+struct fp3 func_f3(struct fp3 x) { return x; }
+
+// CHECK: define [4 x fp128] @func_f4([4 x fp128] %x.coerce)
+struct fp4 func_f4(struct fp4 x) { return x; }
+
+// CHECK: define [5 x fp128] @func_f5([5 x fp128] %x.coerce)
+struct fp5 func_f5(struct fp5 x) { return x; }
+
+// CHECK: define [6 x fp128] @func_f6([6 x fp128] %x.coerce)
+struct fp6 func_f6(struct fp6 x) { return x; }
+
+// CHECK: define [7 x fp128] @func_f7([7 x fp128] %x.coerce)
+struct fp7 func_f7(struct fp7 x) { return x; }
+
+// CHECK: define [8 x fp128] @func_f8([8 x fp128] %x.coerce)
+struct fp8 func_f8(struct fp8 x) { return x; }
+
+// CHECK: define void @func_f9(%struct.fp9* noalias sret %agg.result, %struct.fp9* byval align 16 %x)
+struct fp9 func_f9(struct fp9 x) { return x; }
+
+// CHECK: define [2 x fp128] @func_fab([2 x fp128] %x.coerce)
+struct fpab func_fab(struct fpab x) { return x; }
+
+// CHECK: define [3 x fp128] @func_fabc([3 x fp128] %x.coerce)
+struct fpabc func_fabc(struct fpabc x) { return x; }
+
+// CHECK: define [4 x fp128] @func_f2a2b([4 x fp128] %x.coerce)
+struct fp2a2b func_f2a2b(struct fp2a2b x) { return x; }
+
+// CHECK-LABEL: @call_fp1
+// CHECK: %[[TMP:[^ ]+]] = load fp128, fp128* getelementptr inbounds (%struct.fp1, %struct.fp1* @global_f1, i32 0, i32 0, i32 0), align 16
+// CHECK: call [1 x fp128] @func_f1(fp128 inreg %[[TMP]])
+struct fp1 global_f1;
+void call_fp1(void) { global_f1 = func_f1(global_f1); }
+
+// CHECK-LABEL: @call_fp2
+// CHECK: %[[TMP:[^ ]+]] = load [2 x fp128], [2 x fp128]* getelementptr inbounds (%struct.fp2, %struct.fp2* @global_f2, i32 0, i32 0), align 16
+// CHECK: call [2 x fp128] @func_f2([2 x fp128] %[[TMP]])
+struct fp2 global_f2;
+void call_fp2(void) { global_f2 = func_f2(global_f2); }
+
+// CHECK-LABEL: @call_fp3
+// CHECK: %[[TMP:[^ ]+]] = load [3 x fp128], [3 x fp128]* getelementptr inbounds (%struct.fp3, %struct.fp3* @global_f3, i32 0, i32 0), align 16
+// CHECK: call [3 x fp128] @func_f3([3 x fp128] %[[TMP]])
+struct fp3 global_f3;
+void call_fp3(void) { global_f3 = func_f3(global_f3); }
+
+// CHECK-LABEL: @call_fp4
+// CHECK: %[[TMP:[^ ]+]] = load [4 x fp128], [4 x fp128]* getelementptr inbounds (%struct.fp4, %struct.fp4* @global_f4, i32 0, i32 0), align 16
+// CHECK: call [4 x fp128] 

[clang-tools-extra] r336302 - Adding some documentation changes that were missed in r336301.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Jul  4 18:35:49 2018
New Revision: 336302

URL: http://llvm.org/viewvc/llvm-project?rev=336302=rev
Log:
Adding some documentation changes that were missed in r336301.

Added:
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-msc51-cpp.rst
Modified:
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Added: clang-tools-extra/trunk/docs/clang-tidy/checks/cert-msc51-cpp.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cert-msc51-cpp.rst?rev=336302=auto
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/cert-msc51-cpp.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/cert-msc51-cpp.rst Wed Jul  
4 18:35:49 2018
@@ -0,0 +1,40 @@
+.. title:: clang-tidy - cert-msc51-cpp
+
+cert-msc51-cpp
+==
+
+This check flags all pseudo-random number engines, engine adaptor
+instantiations and ``srand()`` when initialized or seeded with default 
argument,
+constant expression or any user-configurable type. Pseudo-random number
+engines seeded with a predictable value may cause vulnerabilities e.g. in
+security protocols.
+This is a CERT security rule, see
+`MSC51-CPP. Ensure your random number generator is properly seeded
+`_
 and
+`MSC32-C. Properly seed pseudorandom number generators
+`_.
+
+Examples:
+
+.. code-block:: c++
+
+  void foo() {
+std::mt19937 engine1; // Diagnose, always generate the same sequence
+std::mt19937 engine2(1); // Diagnose
+engine1.seed(); // Diagnose
+engine2.seed(1); // Diagnose
+
+std::time_t t;
+engine1.seed(std::time()); // Diagnose, system time might be controlled 
by user
+
+int x = atoi(argv[1]);
+std::mt19937 engine3(x);  // Will not warn
+  }
+
+Options
+---
+
+.. option:: DisallowedSeedTypes
+
+   A comma-separated list of the type names which are disallowed.
+   Default values are ``time_t``, ``std::time_t``.

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=336302=336301=336302=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Wed Jul  4 18:35:49 
2018
@@ -73,7 +73,9 @@ Clang-Tidy Checks
cert-fio38-c (redirects to misc-non-copyable-objects) 
cert-flp30-c
cert-msc30-c (redirects to cert-msc50-cpp) 
+   cert-msc32-c (redirects to cert-msc51-cpp) 
cert-msc50-cpp
+   cert-msc51-cpp
cert-oop11-cpp (redirects to performance-move-constructor-init) 

cppcoreguidelines-avoid-goto
cppcoreguidelines-c-copy-assignment-signature (redirects to 
misc-unconventional-assign-operator) 



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

[PATCH] D44143: [clang-tidy] Create properly seeded random generator check

[Aaron Ballman via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D44143#1152550, @boga95 wrote:

> How can I commit it?


You need to have obtained commit access first. I went ahead and committed on 
your behalf in r336301. Thank you for the patch!




Comment at: docs/clang-tidy/checks/cert-msc51-cpp.rst:7
+This check flags all pseudo-random number engines, engine adaptor
+instantiations and `srand()` when initialized or seeded with default argument,
+constant expression or any user-configurable type. Pseudo-random number

boga95 wrote:
> aaron.ballman wrote:
> > Please add double backticks around `srand()` instead of single backticks.
> Should I use double backticks in release notes too?
Good catch; I'll add those when I commit.


https://reviews.llvm.org/D44143



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

[clang-tools-extra] r336301 - Add the cert-msc51-cpp and cert-msc32-c checks.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Jul  4 18:16:31 2018
New Revision: 336301

URL: http://llvm.org/viewvc/llvm-project?rev=336301=rev
Log:
Add the cert-msc51-cpp and cert-msc32-c checks.

These checks flag use of random number generators with poor seeds that would 
possibly lead to degraded random number generation.

Patch by Borsik Gábor

Added:

clang-tools-extra/trunk/clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp
clang-tools-extra/trunk/clang-tidy/cert/ProperlySeededRandomGeneratorCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-msc32-c.rst
clang-tools-extra/trunk/test/clang-tidy/cert-msc32-c.c
clang-tools-extra/trunk/test/clang-tidy/cert-msc51-cpp.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
clang-tools-extra/trunk/docs/ReleaseNotes.rst

Modified: clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp?rev=336301=336300=336301=diff
==
--- clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp Wed Jul  4 
18:16:31 2018
@@ -21,6 +21,7 @@
 #include "FloatLoopCounter.h"
 #include "LimitedRandomnessCheck.h"
 #include "PostfixOperatorCheck.h"
+#include "ProperlySeededRandomGeneratorCheck.h"
 #include "SetLongJmpCheck.h"
 #include "StaticObjectExceptionCheck.h"
 #include "StrToNumCheck.h"
@@ -58,6 +59,8 @@ public:
 "cert-err61-cpp");
 // MSC
 CheckFactories.registerCheck("cert-msc50-cpp");
+CheckFactories.registerCheck(
+"cert-msc51-cpp");
 
 // C checkers
 // DCL
@@ -72,6 +75,8 @@ public:
 CheckFactories.registerCheck("cert-err34-c");
 // MSC
 CheckFactories.registerCheck("cert-msc30-c");
+CheckFactories.registerCheck(
+"cert-msc32-c");
   }
 };
 

Modified: clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt?rev=336301=336300=336301=diff
==
--- clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt Wed Jul  4 18:16:31 
2018
@@ -7,6 +7,7 @@ add_clang_library(clangTidyCERTModule
   FloatLoopCounter.cpp
   LimitedRandomnessCheck.cpp
   PostfixOperatorCheck.cpp
+  ProperlySeededRandomGeneratorCheck.cpp
   SetLongJmpCheck.cpp
   StaticObjectExceptionCheck.cpp
   StrToNumCheck.cpp

Added: 
clang-tools-extra/trunk/clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp?rev=336301=auto
==
--- 
clang-tools-extra/trunk/clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp 
(added)
+++ 
clang-tools-extra/trunk/clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp 
Wed Jul  4 18:16:31 2018
@@ -0,0 +1,124 @@
+//===--- ProperlySeededRandomGeneratorCheck.cpp - 
clang-tidy---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ProperlySeededRandomGeneratorCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/STLExtras.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cert {
+
+ProperlySeededRandomGeneratorCheck::ProperlySeededRandomGeneratorCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  RawDisallowedSeedTypes(
+  Options.get("DisallowedSeedTypes", "time_t,std::time_t")) {
+  StringRef(RawDisallowedSeedTypes).split(DisallowedSeedTypes, ',');
+}
+
+void ProperlySeededRandomGeneratorCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "DisallowedSeedTypes", RawDisallowedSeedTypes);
+}
+
+void ProperlySeededRandomGeneratorCheck::registerMatchers(MatchFinder *Finder) 
{
+  auto RandomGeneratorEngineDecl = cxxRecordDecl(hasAnyName(
+  "::std::linear_congruential_engine", "::std::mersenne_twister_engine",
+  "::std::subtract_with_carry_engine", "::std::discard_block_engine",
+  "::std::independent_bits_engine", "::std::shuffle_order_engine"));
+  auto RandomGeneratorEngineTypeMatcher = hasType(hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(RandomGeneratorEngineDecl;
+
+  // std::mt19937 engine;
+  // engine.seed();
+  //^
+  // engine.seed(1);
+  //^
+  // const int x = 1;
+  // 

r336300 - [Index] Remove unused index::IndexDataConsumer::_anchor()

[Fangrui Song via cfe-commits]

Author: maskray
Date: Wed Jul  4 17:33:03 2018
New Revision: 336300

URL: http://llvm.org/viewvc/llvm-project?rev=336300=rev
Log:
[Index] Remove unused index::IndexDataConsumer::_anchor()

It was supposed to serve as a key function, but it was invalid as it was not 
the first out-of-line non-pure virtual function.

Modified:
cfe/trunk/include/clang/Index/IndexDataConsumer.h
cfe/trunk/lib/Index/IndexingAction.cpp

Modified: cfe/trunk/include/clang/Index/IndexDataConsumer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Index/IndexDataConsumer.h?rev=336300=336299=336300=diff
==
--- cfe/trunk/include/clang/Index/IndexDataConsumer.h (original)
+++ cfe/trunk/include/clang/Index/IndexDataConsumer.h Wed Jul  4 17:33:03 2018
@@ -54,9 +54,6 @@ public:
  SymbolRoleSet Roles, SourceLocation Loc);
 
   virtual void finish() {}
-
-private:
-  virtual void _anchor();
 };
 
 } // namespace index

Modified: cfe/trunk/lib/Index/IndexingAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexingAction.cpp?rev=336300=336299=336300=diff
==
--- cfe/trunk/lib/Index/IndexingAction.cpp (original)
+++ cfe/trunk/lib/Index/IndexingAction.cpp Wed Jul  4 17:33:03 2018
@@ -19,8 +19,6 @@
 using namespace clang;
 using namespace clang::index;
 
-void IndexDataConsumer::_anchor() {}
-
 bool IndexDataConsumer::handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
 ArrayRef Relations,
 SourceLocation Loc,


___
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/include/memory:1479
+struct __has_construct_missing
+: false_type
+{

Quuxplusone wrote:
> vsapsai wrote:
> > Quuxplusone wrote:
> > > vsapsai wrote:
> > > > vsapsai wrote:
> > > > > vsapsai wrote:
> > > > > > erik.pilkington wrote:
> > > > > > > vsapsai wrote:
> > > > > > > > erik.pilkington wrote:
> > > > > > > > > Shouldn't this be true_type?
> > > > > > > > I see this is confusing and I'm still struggling how to express 
> > > > > > > > it. The issue is that in C++03 `__has_construct` should be 
> > > > > > > > something like unknown, so that neither `__has_construct` nor 
> > > > > > > > `! __has_construct` evaluate to true because we don't really 
> > > > > > > > know if allocator has construct. This case is covered by the 
> > > > > > > > added test, in C++03 the memcpy specialization was enabled when
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > is_same >
> > > > > > > >   || !false_type
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > So `is_same` check had no effect and we were using memcpy to 
> > > > > > > > convert between int and float.
> > > > > > > > 
> > > > > > > > I was considering using something like
> > > > > > > > 
> > > > > > > > ```lang=c++
> > > > > > > > typename enable_if
> > > > > > > > <
> > > > > > > > (is_same
> > > > > > > >  <
> > > > > > > > typename _VSTD::remove_const > > > > > > > allocator_type::value_type>::type,
> > > > > > > > typename _VSTD::remove_const<_SourceTp>::type
> > > > > > > >  >::value
> > > > > > > > #ifndef _LIBCPP_CXX03_LANG
> > > > > > > > || !__has_construct > > > > > > > _SourceTp>::value
> > > > > > > > #endif
> > > > > > > > ) &&
> > > > > > > >  is_trivially_move_constructible<_DestTp>::value,
> > > > > > > > void
> > > > > > > > >::type
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > But that doesn't look readable to me, so I've introduced ad-hoc 
> > > > > > > > ternary logic with `__has_construct_missing`.
> > > > > > > Oh I see, yikes! That's a pretty bad bug. I agree that this fix 
> > > > > > > is best then, but can you add a comment explaining this to 
> > > > > > > `__has_construct_missing` for future casual readers? Also, I 
> > > > > > > think we should move the __has_construct_missing bugfix into a 
> > > > > > > different (prerequisite) patch. Seems unrelated to the `const` 
> > > > > > > related optimization below.
> > > > > > The bug as I described isn't really present now because function 
> > > > > > signature
> > > > > > 
> > > > > > __construct_range_forward(allocator_type&, _Tp* __begin1, _Tp* 
> > > > > > __end1, _Tp*& __begin2)
> > > > > > 
> > > > > > works as implicit `is_same` for `__begin1` and `__begin2` types. I 
> > > > > > think it is worth fixing separately and there is a bug with C++03 
> > > > > > and custom allocators.
> > > > > Instead of `__has_construct_missing` I've implemented real 
> > > > > `__has_construct` in D48753. But it is stricter in C++03 than in 
> > > > > C++11 and later. So it made me think that absence of `construct` with 
> > > > > exact signature isn't a good reason to use memcpy.
> > > > I was wrong. Now I think the logic for using memcpy should be
> > > > 
> > > > if types are the same modulo constness
> > > > and
> > > > (
> > > > using default allocator
> > > > or
> > > > using custom allocator without `construct`
> > > > )
> > > > and
> > > > is_trivially_move_constructible
> > > > 
> > > > The purpose of the allocator check is to cover cases when `static 
> > > > construct` would end up calling not user's code but libc++ code that we 
> > > > know can be replaced with memcpy.
> > > I'd like to request the spelling `__has_trivial_construct_and_destroy > > T, T&&>` as described here: 
> > > https://www.youtube.com/watch?v=MWBfmmg8-Yo=21m45s
> > > I have an implementation in my fork that might be useful for comparison, 
> > > although even it doesn't add that last `T&&` parameter.
> > > https://github.com/Quuxplusone/libcxx/commit/34eb0b5c8f03880b94d53b877cbca384783ad65a
> > > 
> > > What we're *really* interested in, in most cases, is 
> > > `__has_trivial_construct && __has_trivial_destroy`. We 
> > > don't care if there happens to exist an obscure overload such as 
> > > `A::construct(T*, Widget, Gadget)` as long as it is not selected. This is 
> > > particularly relevant to `scoped_allocator_adaptor`, whose `construct` is 
> > > trivial for primitive types but non-trivial for allocator-aware types.
> > > 
> > > Also, when we write out the template type parameters we care about, we 
> > > can do the decltype stuff really easily, without having to "fudge" the 
> > > metaprogramming and possibly get it wrong. For example, if `A::construct` 
> > > is an overload set, in which case the 

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

In https://reviews.llvm.org/D48342#1152063, @EricWF wrote:

> Are there any tests which actually exercise the new behavior?


Added tests only verify we don't use memcpy erroneously. And existing tests 
make sure there are no functionality regressions. But there is nothing to test 
the performance improvement. Are there any recommendations for that?




Comment at: libcxx/include/memory:1665
+(is_same::type> >::value
+|| is_same >::value
+|| !__has_construct::value) &&

EricWF wrote:
> I'm not sure we should care about allocators for `T const`. The're all but an 
> abomination. 
My main goal was to avoid performance regression for

std::vector v(const_raw, const_raw + SIZE);

I'm not protecting allocators for `T const`, it just seems cheap to support 
them anyway.



Comment at: 
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_different_value_type.pass.cpp:13
+// template  vector(InputIter first, InputIter last);
+
+// Initialize a vector with a different value type. Make sure initialization

EricWF wrote:
> Can this be folded into an existing test file for the constructor it's 
> targeting?
Will move to construct_iter_iter.pass.cpp. Which reminded me that I need to 
make construct_iter_iter_alloc.pass.cpp and construct_iter_iter.pass.cpp more 
in sync.


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] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: 
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp:60
+
+  void construct(pointer p, const value_type& val)
+  {

Quuxplusone wrote:
> Per my comments on D48342, I think it would be fun to add a test allocator 
> like
> ```
> struct cpp03_fun_allocator : bare_allocator {
> ...
> void construct(pointer p, const value_type& val) {
> construct(p, val, std::is_class{});
> }
> void construct(pointer p, const value_type& val, std::true_type) {
> ::new(p) value_type(val);
> }
> void construct(pointer p, const value_type& val, std::false_type) {
> ::new(p) value_type(val);
> }
> ```
> and just see whether it passes the test suite. If it does, might as well add 
> that test. But if it doesn't, *I'm* not going to force anyone to fix it. 
> Allocators in C++03 seems like masochism to me. :)
I've tweaked the test a little bit: added `construct_called = true;` in 2 
places, used `std::is_class::value`. That shouldn't change your intention. 
Tried and it works in C++17 but not in C++03.

Given that it didn't work earlier, not planning to fix it now.


https://reviews.llvm.org/D48753



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

@rsmith Ping. This needs to land before the next branch for release.


https://reviews.llvm.org/D45015



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

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hello Rafael.
This change is good, just some cleanup is needed.




Comment at: lib/AST/ASTImporter.cpp:2559
D->isImplicit());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   } else if (auto *FromConversion = dyn_cast(D)) {

martong wrote:
> Why don't we need to call the `setRangeEnd()` when e.g. we create a 
> `CXXMethodDecl` or a `CXXConversionDecl`? Couldn't we just call this after 
> all these branches (at line 2587)?
I guess the reason is that they take LocEnd as ctor arguments. However, I think 
that having a single point is better.


Repository:
  rC Clang

https://reviews.llvm.org/D48941



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

[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Nice!


Repository:
  rC Clang

https://reviews.llvm.org/D48773



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

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

I opened https://reviews.llvm.org/D48951 to fix the failures in clang-move.


Repository:
  rC Clang

https://reviews.llvm.org/D48903



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

[PATCH] D48951: [clang-move] ClangMoveTests: Remove dots in output paths

[Simon Marchi via Phabricator via cfe-commits]

simark created this revision.
Herald added subscribers: cfe-commits, ioeric.

Following https://reviews.llvm.org/D48903 ([VirtualFileSystem] 
InMemoryFileSystem::status: Return
a Status with the requested name), the paths output by clang-move in the
FileToReplacements map may contain leading "./".  For example, where we
would get "foo.h", we'll now get "./foo.h".  This breaks the tests,
because we are doing exact string lookups in the FileToFileID and
Results maps (they contain "foo.h", but we search for "./foo.h").

To mitigate this, try to normalize a little bit the paths output by
clang-move to remove that leading "./".

This patch should be safe to merge before https://reviews.llvm.org/D48903, 
remove_dots will just
be a no-op.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48951

Files:
  unittests/clang-move/ClangMoveTests.cpp


Index: unittests/clang-move/ClangMoveTests.cpp
===
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -239,8 +239,10 @@
   // The Key is file name, value is the new code after moving the class.
   std::map Results;
   for (const auto  : FileToReplacements) {
-StringRef FilePath = It.first;
-Results[FilePath] = Context.getRewrittenText(FileToFileID[FilePath]);
+// The path may come out as "./foo.h", normalize to "foo.h".
+SmallString<32> FilePath (It.first);
+llvm::sys::path::remove_dots(FilePath);
+Results[FilePath.str().str()] = 
Context.getRewrittenText(FileToFileID[FilePath]);
   }
   return Results;
 }


Index: unittests/clang-move/ClangMoveTests.cpp
===
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -239,8 +239,10 @@
   // The Key is file name, value is the new code after moving the class.
   std::map Results;
   for (const auto  : FileToReplacements) {
-StringRef FilePath = It.first;
-Results[FilePath] = Context.getRewrittenText(FileToFileID[FilePath]);
+// The path may come out as "./foo.h", normalize to "foo.h".
+SmallString<32> FilePath (It.first);
+llvm::sys::path::remove_dots(FilePath);
+Results[FilePath.str().str()] = Context.getRewrittenText(FileToFileID[FilePath]);
   }
   return Results;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r336297 - Remove old workaround that is no longer needed

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Wed Jul  4 13:16:05 2018
New Revision: 336297

URL: http://llvm.org/viewvc/llvm-project?rev=336297=rev
Log:
Remove old workaround that is no longer needed

Modified:
libcxx/trunk/utils/libcxx/test/config.py

Modified: libcxx/trunk/utils/libcxx/test/config.py
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/utils/libcxx/test/config.py?rev=336297=336296=336297=diff
==
--- libcxx/trunk/utils/libcxx/test/config.py (original)
+++ libcxx/trunk/utils/libcxx/test/config.py Wed Jul  4 13:16:05 2018
@@ -933,9 +933,6 @@ class Configuration(object):
 # FIXME: Enable the two warnings below.
 self.cxx.addWarningFlagIfSupported('-Wno-conversion')
 self.cxx.addWarningFlagIfSupported('-Wno-unused-local-typedef')
-# FIXME: Remove this warning once the min/max handling patch lands
-# See https://reviews.llvm.org/D33080
-self.cxx.addWarningFlagIfSupported('-Wno-#warnings')
 std = self.get_lit_conf('std', None)
 if std in ['c++98', 'c++03']:
 # The '#define static_assert' provided by libc++ in C++03 mode


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

[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Ping?


Repository:
  rL LLVM

https://reviews.llvm.org/D45179



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

[PATCH] D38680: [libunwind] Fix handling of DW_CFA_GNU_args_size

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Ping @joerg


https://reviews.llvm.org/D38680



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

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

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

xazax.hun added a comment.

In https://reviews.llvm.org/D48027#1142324, @MTC wrote:

> - There is possible match the wrong AST node, e.g. for the NamedDecl `foo()`, 
> if the function body has the `a::b::x`, when we match `a::b::X()`, we will 
> match `foo()` . Because I'm not familiar with ASTMatcher, my bad, I can't 
> think of a perfect way to match only the specified nodes.


Hmm, instead of using the match function which will traverse the ast, we could 
use the `HasNameMatcher` class which can be invoked on only one node. That 
class is in an internal namespace though, so I think the best would be to ask 
the maintainers whether it is ok to use that class externally, or were whe 
should put the functionality we need.


Repository:
  rC Clang

https://reviews.llvm.org/D48027



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

[PATCH] D44143: [clang-tidy] Create properly seeded random generator check

[Borsik Gábor via Phabricator via cfe-commits]

boga95 added a comment.

How can I commit it?




Comment at: docs/clang-tidy/checks/cert-msc51-cpp.rst:7
+This check flags all pseudo-random number engines, engine adaptor
+instantiations and `srand()` when initialized or seeded with default argument,
+constant expression or any user-configurable type. Pseudo-random number

aaron.ballman wrote:
> Please add double backticks around `srand()` instead of single backticks.
Should I use double backticks in release notes too?


https://reviews.llvm.org/D44143



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

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

In https://reviews.llvm.org/D48903#1152485, @ilya-biryukov wrote:

> I usually run `ninja check-clang check-clang-tools` for clang changes. Have 
> never used `clang-test`, not sure what it does.


I think `clang-test` is an alias for `check-clang`.

> I ran it with this change, found a few failures from clang-move:
> 
>   Failing Tests (5):
>   Extra Tools Unit Tests :: 
> clang-move/./ClangMoveTests/ClangMove.AddDependentNewHeader
>   Extra Tools Unit Tests :: 
> clang-move/./ClangMoveTests/ClangMove.AddDependentOldHeader
>   Extra Tools Unit Tests :: 
> clang-move/./ClangMoveTests/ClangMove.DontMoveAll
>   Extra Tools Unit Tests :: 
> clang-move/./ClangMoveTests/ClangMove.MoveHeaderAndCC
>   Extra Tools Unit Tests :: 
> clang-move/./ClangMoveTests/ClangMove.MoveHeaderOnly

Doh, I'll run `check-clang-tools` too.


Repository:
  rC Clang

https://reviews.llvm.org/D48903



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

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I usually run `ninja check-clang check-clang-tools` for clang changes. Have 
never used `clang-test`, not sure what it does.
I ran it with this change, found a few failures from clang-move:

  Failing Tests (5):
  Extra Tools Unit Tests :: 
clang-move/./ClangMoveTests/ClangMove.AddDependentNewHeader
  Extra Tools Unit Tests :: 
clang-move/./ClangMoveTests/ClangMove.AddDependentOldHeader
  Extra Tools Unit Tests :: 
clang-move/./ClangMoveTests/ClangMove.DontMoveAll
  Extra Tools Unit Tests :: 
clang-move/./ClangMoveTests/ClangMove.MoveHeaderAndCC
  Extra Tools Unit Tests :: 
clang-move/./ClangMoveTests/ClangMove.MoveHeaderOnly


Repository:
  rC Clang

https://reviews.llvm.org/D48903



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

[PATCH] D48721: Patch to fix pragma metadata for do-while loops

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D48721#1152023, @deepak2427 wrote:

> I encountered the issue while working with the unroller and found that it was 
> not following the pragma info, and traced it back to the issue with metadata.
>  As far as I understood, for for-loops and while-loops, we add the metadata 
> only to the loop back-edge. So it would make sense to keep them consistent.
>  I'm not an expert in clang, and do not know how we can detect such problems.


The code change is likely okay. We need to have the tests updated. With rare 
exception, we don't have end-to-end tests in Clang. We test Clang's CodeGen 
independently, and so Clang's CodeGen tests shouldn't run the optimizer. Please 
write tests that directly check the expected output of Clang (without running 
the optimizer). If you look at other tests in the CodeGen directory, you should 
see what I mean. If you have any questions, please feel free to ask. Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D48721



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

[PATCH] D48946: [Preamble] Check system dependencies in preamble too

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: sammccall, ioeric.

PrecompiledPreamble hasn't checked the system dependencies changed
before. This result in invalid preamble not being rebuilt if headers
that changed were found in -isystem include paths.

This pattern is sometimes used to avoid showing warnings in third
party code, so we want to correctly handle those cases.

Tested in clangd, see the follow-up patch.


Repository:
  rC Clang

https://reviews.llvm.org/D48946

Files:
  lib/Frontend/PrecompiledPreamble.cpp


Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -63,6 +63,15 @@
   return Overlay;
 }
 
+class PreambleDependencyCollector : public DependencyCollector {
+public:
+ // We want to collect all dependencies for correctness. Avoiding the real
+ // system dependencies (e.g. stl from /usr/lib) would probably be a good idea,
+ // but there is no way to distinguish between those and the ones that can be
+ // spuriously added by '-isystem' (e.g. to avoid errors from those headers).
+ bool needSystemDependencies() override { return true; }
+};
+
 /// Keeps a track of files to be deleted in destructor.
 class TemporaryFiles {
 public:
@@ -311,7 +320,7 @@
   Clang->setSourceManager(
   new SourceManager(Diagnostics, Clang->getFileManager()));
 
-  auto PreambleDepCollector = std::make_shared();
+  auto PreambleDepCollector = std::make_shared();
   Clang->addDependencyCollector(PreambleDepCollector);
 
   // Remap the main source file to the preamble buffer.


Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -63,6 +63,15 @@
   return Overlay;
 }
 
+class PreambleDependencyCollector : public DependencyCollector {
+public:
+ // We want to collect all dependencies for correctness. Avoiding the real
+ // system dependencies (e.g. stl from /usr/lib) would probably be a good idea,
+ // but there is no way to distinguish between those and the ones that can be
+ // spuriously added by '-isystem' (e.g. to avoid errors from those headers).
+ bool needSystemDependencies() override { return true; }
+};
+
 /// Keeps a track of files to be deleted in destructor.
 class TemporaryFiles {
 public:
@@ -311,7 +320,7 @@
   Clang->setSourceManager(
   new SourceManager(Diagnostics, Clang->getFileManager()));
 
-  auto PreambleDepCollector = std::make_shared();
+  auto PreambleDepCollector = std::make_shared();
   Clang->addDependencyCollector(PreambleDepCollector);
 
   // Remap the main source file to the preamble buffer.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48947: [clangd] Added a test for preambles and -isystem

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: sammccall, ioeric.
Herald added subscribers: jkorous, MaskRay.

Checks that preambles are properly invalidated when headers from
-isystem paths change.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48947

Files:
  unittests/clangd/ClangdTests.cpp


Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -34,13 +34,16 @@
 namespace clangd {
 
 using ::testing::ElementsAre;
+using ::testing::Contains;
 using ::testing::Eq;
 using ::testing::Gt;
 using ::testing::IsEmpty;
 using ::testing::Pair;
 using ::testing::UnorderedElementsAre;
 
 namespace {
+// FIXME: This is copied from CodeCompleteTests.cpp. Share the code instead.
+MATCHER_P(Named, Name, "") { return arg.Name == Name; }
 
 bool diagsContainErrors(const std::vector ) {
   for (auto D : Diagnostics) {
@@ -927,6 +930,40 @@
   EXPECT_EQ(Expected, *Changed);
 }
 
+TEST_F(ClangdVFSTest, ChangedHeaderFromISystem) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto SourcePath = testPath("source/foo.cpp");
+  auto HeaderPath = testPath("headers/foo.h");
+  FS.Files[HeaderPath] = "struct X { int bar; };";
+  Annotations Code(R"cpp(
+#include "foo.h"
+
+int main() {
+  X().ba^
+})cpp");
+  CDB.ExtraClangFlags.push_back("-xc++");
+  CDB.ExtraClangFlags.push_back("-isystem" + testPath("headers"));
+
+  runAddDocument(Server, SourcePath, Code.code());
+  auto Completions = cantFail(runCodeComplete(Server, SourcePath, Code.point(),
+clangd::CodeCompleteOptions()))
+   .Completions;
+  EXPECT_THAT(Completions, ElementsAre(Named("bar")));
+  // Update the header and rerun addDocument to make sure we get the updated
+  // files.
+  FS.Files[HeaderPath] = "struct X { int bar; int baz; };";
+  runAddDocument(Server, SourcePath, Code.code());
+  Completions = cantFail(runCodeComplete(Server, SourcePath, Code.point(),
+clangd::CodeCompleteOptions()))
+   .Completions;
+  // We want to make sure we see the updated version.
+  EXPECT_THAT(Completions, ElementsAre(Named("bar"), Named("baz")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -34,13 +34,16 @@
 namespace clangd {
 
 using ::testing::ElementsAre;
+using ::testing::Contains;
 using ::testing::Eq;
 using ::testing::Gt;
 using ::testing::IsEmpty;
 using ::testing::Pair;
 using ::testing::UnorderedElementsAre;
 
 namespace {
+// FIXME: This is copied from CodeCompleteTests.cpp. Share the code instead.
+MATCHER_P(Named, Name, "") { return arg.Name == Name; }
 
 bool diagsContainErrors(const std::vector ) {
   for (auto D : Diagnostics) {
@@ -927,6 +930,40 @@
   EXPECT_EQ(Expected, *Changed);
 }
 
+TEST_F(ClangdVFSTest, ChangedHeaderFromISystem) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto SourcePath = testPath("source/foo.cpp");
+  auto HeaderPath = testPath("headers/foo.h");
+  FS.Files[HeaderPath] = "struct X { int bar; };";
+  Annotations Code(R"cpp(
+#include "foo.h"
+
+int main() {
+  X().ba^
+})cpp");
+  CDB.ExtraClangFlags.push_back("-xc++");
+  CDB.ExtraClangFlags.push_back("-isystem" + testPath("headers"));
+
+  runAddDocument(Server, SourcePath, Code.code());
+  auto Completions = cantFail(runCodeComplete(Server, SourcePath, Code.point(),
+clangd::CodeCompleteOptions()))
+   .Completions;
+  EXPECT_THAT(Completions, ElementsAre(Named("bar")));
+  // Update the header and rerun addDocument to make sure we get the updated
+  // files.
+  FS.Files[HeaderPath] = "struct X { int bar; int baz; };";
+  runAddDocument(Server, SourcePath, Code.code());
+  Completions = cantFail(runCodeComplete(Server, SourcePath, Code.point(),
+clangd::CodeCompleteOptions()))
+   .Completions;
+  // We want to make sure we see the updated version.
+  EXPECT_THAT(Completions, ElementsAre(Named("bar"), Named("baz")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48721: Patch to fix pragma metadata for do-while loops

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D48721#1150677, @bjope wrote:

> In https://reviews.llvm.org/D48721#1150361, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D48721#1150333, @bjope wrote:
> >
> > > Is the fault that the metadata only should be put on the back edge, not 
> > > the branch in the preheader?
> >
> >
> > Yea. Our past thinking has been that any backedge in the loop is valid. The 
> > metadata shouldn't end up other places, although it's benign unless those 
> > other places are (or may later become) a backedge for some different loop.
>
>
> I'm no expert on clang. The patch seems to fix the problem if the goal is to 
> only add the loop-metadata on the backedge , but I'll leave it to someone 
> else to approve it.
>
> I'm a little bit concerned about opt not detecting this kind of problems 
> though.
>  Would it be possible for some verifier to detect if we have loop metadata on 
> some branch that aren't in the latch block?
>  And/or should the optimization that "merges" two branches with different 
> loop metadata), be smarter about which loop metadata to keep? Or maybe we 
> should be defensive and discard loop metadata on the merged branch 
> instruction?


We could add this to the verifier. We have transformation passes that aren't 
explicitly loop aware, and so the question is would those passes now need to do 
something special to remain valid in the presence of this metadata. As a 
general rule, metadata in invalid locations is simply ignored. It clearly is a 
problem, if metadata is moving from one valid location to an unrelated valid 
location.


Repository:
  rC Clang

https://reviews.llvm.org/D48721



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

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Some patterns are covered by compiler diagnostics: 
https://godbolt.org/g/HvsjnP. Is there any benefit in re-implementing them?




Comment at: clang-tidy/misc/IncorrectPointerCastCheck.cpp:60
+diag(CastExpr->getLocStart(),
+ "Do not use C-style cast from %0 to %1! "
+ "Different type layout. Struct members are incompatible")

The style of the message differs from that of the other checks (and Clang 
warnings). Messages are not complete sentences and as such should not use 
capitalization, trailing periods etc. Also I would avoid imperatives and 
exclamation  marks. The message should use neutral tone and state what exactly 
is wrong with the code and why.



Comment at: test/clang-tidy/misc-incorrect-pointer-cast.cpp:109
+}
\ No newline at end of file


"No newline at end of file". Please fix.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48866



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

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: lib/AST/ASTImporter.cpp:2559
D->isImplicit());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   } else if (auto *FromConversion = dyn_cast(D)) {

Why don't we need to call the `setRangeEnd()` when e.g. we create a 
`CXXMethodDecl` or a `CXXConversionDecl`? Couldn't we just call this after all 
these branches (at line 2587)?


Repository:
  rC Clang

https://reviews.llvm.org/D48941



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

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

I ran `ninja clang-test`:

  Testing Time: 1720.20s
Expected Passes: 12472
Expected Failures  : 18
Unsupported Tests  : 263


Repository:
  rC Clang

https://reviews.llvm.org/D48903



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

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

[Simon Marchi via Phabricator via cfe-commits]

simark updated this revision to Diff 154118.
simark added a comment.

- Fixed formatting (ran git-clang-format)
- Fixed expectation in TEST_F(InMemoryFileSystemTest, WorkingDirectory)
- Added test TEST_F(InMemoryFileSystemTest, StatusName)


Repository:
  rC Clang

https://reviews.llvm.org/D48903

Files:
  lib/Basic/VirtualFileSystem.cpp
  unittests/Basic/VirtualFileSystemTest.cpp


Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -794,7 +794,7 @@
 
   auto Stat = FS.status("/b/c");
   ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString();
-  ASSERT_EQ("c", Stat->getName());
+  ASSERT_EQ("/b/c", Stat->getName());
   ASSERT_EQ("/b", *FS.getCurrentWorkingDirectory());
 
   Stat = FS.status("c");
@@ -919,6 +919,30 @@
   ASSERT_TRUE(Stat->isRegularFile());
 }
 
+// Test that the name returned by status() is in the same form as the path that
+// was requested.
+TEST_F(InMemoryFileSystemTest, StatusName) {
+  NormalizedFS.addFile("/a/b/c", 0, MemoryBuffer::getMemBuffer("abc"),
+   /*User=*/None,
+   /*Group=*/None, sys::fs::file_type::regular_file);
+  NormalizedFS.setCurrentWorkingDirectory("/a/b");
+
+  auto Stat = NormalizedFS.status("../b/c");
+  ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n"
+<< NormalizedFS.toString();
+  ASSERT_TRUE(Stat->isRegularFile());
+  ASSERT_EQ("../b/c", Stat->getName());
+
+  auto File = NormalizedFS.openFileForRead("../b/c");
+  ASSERT_FALSE(File.getError()) << File.getError() << "\n"
+<< NormalizedFS.toString();
+  Stat = (*File)->status();
+  ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n"
+<< NormalizedFS.toString();
+  ASSERT_TRUE(Stat->isRegularFile());
+  ASSERT_EQ("../b/c", Stat->getName());
+}
+
 // NOTE: in the tests below, we use '//root/' as our root directory, since it 
is
 // a legal *absolute* path on Windows as well as *nix.
 class VFSFromYAMLTest : public ::testing::Test {
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -508,10 +508,17 @@
 class InMemoryFileAdaptor : public File {
   InMemoryFile 
 
+  // The name to use when returning a Status for this file.
+  std::string RequestedName;
+
 public:
-  explicit InMemoryFileAdaptor(InMemoryFile ) : Node(Node) {}
+  explicit InMemoryFileAdaptor(InMemoryFile , std::string RequestedName)
+  : Node(Node), RequestedName(std::move(RequestedName)) {}
 
-  llvm::ErrorOr status() override { return Node.getStatus(); }
+  llvm::ErrorOr status() override {
+Status St = Node.getStatus();
+return Status::copyWithNewName(St, RequestedName);
+  }
 
   llvm::ErrorOr>
   getBuffer(const Twine , int64_t FileSize, bool RequiresNullTerminator,
@@ -710,8 +717,10 @@
 
 llvm::ErrorOr InMemoryFileSystem::status(const Twine ) {
   auto Node = lookupInMemoryNode(*this, Root.get(), Path);
-  if (Node)
-return (*Node)->getStatus();
+  if (Node) {
+Status St = (*Node)->getStatus();
+return Status::copyWithNewName(St, Path.str());
+  }
   return Node.getError();
 }
 
@@ -724,7 +733,8 @@
   // When we have a file provide a heap-allocated wrapper for the memory buffer
   // to match the ownership semantics for File.
   if (auto *F = dyn_cast(*Node))
-return std::unique_ptr(new detail::InMemoryFileAdaptor(*F));
+return std::unique_ptr(
+new detail::InMemoryFileAdaptor(*F, Path.str()));
 
   // FIXME: errc::not_a_file?
   return make_error_code(llvm::errc::invalid_argument);


Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -794,7 +794,7 @@
 
   auto Stat = FS.status("/b/c");
   ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString();
-  ASSERT_EQ("c", Stat->getName());
+  ASSERT_EQ("/b/c", Stat->getName());
   ASSERT_EQ("/b", *FS.getCurrentWorkingDirectory());
 
   Stat = FS.status("c");
@@ -919,6 +919,30 @@
   ASSERT_TRUE(Stat->isRegularFile());
 }
 
+// Test that the name returned by status() is in the same form as the path that
+// was requested.
+TEST_F(InMemoryFileSystemTest, StatusName) {
+  NormalizedFS.addFile("/a/b/c", 0, MemoryBuffer::getMemBuffer("abc"),
+   /*User=*/None,
+   /*Group=*/None, sys::fs::file_type::regular_file);
+  NormalizedFS.setCurrentWorkingDirectory("/a/b");
+
+  auto Stat = NormalizedFS.status("../b/c");
+  ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n"
+<< NormalizedFS.toString();
+  ASSERT_TRUE(Stat->isRegularFile());
+  ASSERT_EQ("../b/c", 

[PATCH] D48942: [PCH] Add an option to not write comments into PCH

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Not sure about two things:

- Using PreprocessorOptions for plumbing this setting. I'd rather put it into 
FrontendOptions, but there seems to be no way to get those in the ASTWriter... 
Happy to search for the alternatives
- Lack of tests. I'm not sure how to properly test it in clang. I've added 
asserts in clangd, see the follow-up https://reviews.llvm.org/D48943. But happy 
to look into writing actual tests if that looks important.

Also not sure if someone else should look at this change, suggestions for 
reviewers are very welcome!


Repository:
  rC Clang

https://reviews.llvm.org/D48942



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

[PATCH] D48942: [PCH] Add an option to not write comments into PCH

[Ilya Biryukov via Phabricator via cfe-commits]

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

Will be used in clangd, see the follow-up change.
Clangd does not use comments read from PCH to avoid crashes due to
changed contents of the file. However, reading them considerably slows
down code completion on files with large preambles.


Repository:
  rC Clang

https://reviews.llvm.org/D48942

Files:
  include/clang/Lex/PreprocessorOptions.h
  lib/Serialization/ASTWriter.cpp


Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -78,6 +78,7 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
@@ -3252,6 +3253,9 @@
 
 void ASTWriter::WriteComments() {
   Stream.EnterSubblock(COMMENTS_BLOCK_ID, 3);
+  auto _ = llvm::make_scope_exit([this] { Stream.ExitBlock(); });
+  if (!PP->getPreprocessorOpts().WriteCommentListToPCH)
+return;
   ArrayRef RawComments = Context->Comments.getComments();
   RecordData Record;
   for (const auto *I : RawComments) {
@@ -3262,7 +3266,6 @@
 Record.push_back(I->isAlmostTrailingComment());
 Stream.EmitRecord(COMMENTS_RAW_COMMENT, Record);
   }
-  Stream.ExitBlock();
 }
 
 
//===--===//
Index: include/clang/Lex/PreprocessorOptions.h
===
--- include/clang/Lex/PreprocessorOptions.h
+++ include/clang/Lex/PreprocessorOptions.h
@@ -88,6 +88,11 @@
   /// processing the rest of the file.
   bool GeneratePreamble = false;
 
+  /// Whether to write comment locations into the PCH when building it.
+  /// Reading the comments from the PCH can be a performance hit even if the
+  /// clients don't use them.
+  bool WriteCommentListToPCH = true;
+
   /// The implicit PTH input included at the start of the translation unit, or
   /// empty.
   std::string ImplicitPTHInclude;


Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -78,6 +78,7 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
@@ -3252,6 +3253,9 @@
 
 void ASTWriter::WriteComments() {
   Stream.EnterSubblock(COMMENTS_BLOCK_ID, 3);
+  auto _ = llvm::make_scope_exit([this] { Stream.ExitBlock(); });
+  if (!PP->getPreprocessorOpts().WriteCommentListToPCH)
+return;
   ArrayRef RawComments = Context->Comments.getComments();
   RecordData Record;
   for (const auto *I : RawComments) {
@@ -3262,7 +3266,6 @@
 Record.push_back(I->isAlmostTrailingComment());
 Stream.EmitRecord(COMMENTS_RAW_COMMENT, Record);
   }
-  Stream.ExitBlock();
 }
 
 //===--===//
Index: include/clang/Lex/PreprocessorOptions.h
===
--- include/clang/Lex/PreprocessorOptions.h
+++ include/clang/Lex/PreprocessorOptions.h
@@ -88,6 +88,11 @@
   /// processing the rest of the file.
   bool GeneratePreamble = false;
 
+  /// Whether to write comment locations into the PCH when building it.
+  /// Reading the comments from the PCH can be a performance hit even if the
+  /// clients don't use them.
+  bool WriteCommentListToPCH = true;
+
   /// The implicit PTH input included at the start of the translation unit, or
   /// empty.
   std::string ImplicitPTHInclude;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48943: [clangd] Do not write comments into Preamble PCH

[Ilya Biryukov via Phabricator via cfe-commits]

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

To avoid wasting time deserializing them on code completion and
further reparses.

We do not use the comments anyway, because we cannot rely on the file
contents staying the same for reparses that reuse the prebuilt
preamble PCH.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48943

Files:
  clangd/ClangdUnit.cpp
  clangd/CodeCompletionStrings.cpp


Index: clangd/CodeCompletionStrings.cpp
===
--- clangd/CodeCompletionStrings.cpp
+++ clangd/CodeCompletionStrings.cpp
@@ -32,34 +32,6 @@
   }
 }
 
-bool canRequestComment(const ASTContext , const NamedDecl ,
-   bool CommentsFromHeaders) {
-  if (CommentsFromHeaders)
-return true;
-  auto  = Ctx.getSourceManager();
-  // Accessing comments for decls from  invalid preamble can lead to crashes.
-  // So we only return comments from the main file when doing code completion.
-  // For indexing, we still read all the comments.
-  // FIXME: find a better fix, e.g. store file contents in the preamble or get
-  // doc comments from the index.
-  auto canRequestForDecl = [&](const NamedDecl ) -> bool {
-for (auto *Redecl : D.redecls()) {
-  auto Loc = SourceMgr.getSpellingLoc(Redecl->getLocation());
-  if (!SourceMgr.isWrittenInMainFile(Loc))
-return false;
-}
-return true;
-  };
-  // First, check the decl itself.
-  if (!canRequestForDecl(D))
-return false;
-  // Completion also returns comments for properties, corresponding to ObjC
-  // methods.
-  const ObjCMethodDecl *M = dyn_cast();
-  const ObjCPropertyDecl *PDecl = M ? M->findPropertyDecl() : nullptr;
-  return !PDecl || canRequestForDecl(*PDecl);
-}
-
 bool LooksLikeDocComment(llvm::StringRef CommentText) {
   // We don't report comments that only contain "special" chars.
   // This avoids reporting various delimiters, like:
@@ -87,12 +59,13 @@
 // the comments for namespaces.
 return "";
   }
-  if (!canRequestComment(Ctx, *Decl, CommentsFromHeaders))
-return "";
-
   const RawComment *RC = getCompletionComment(Ctx, Decl);
   if (!RC)
 return "";
+
+  // Sanity check that the comment does not come from the PCH. We choose to not
+  // write them into PCH, because they are racy and slow to load.
+  assert(!Ctx.getSourceManager().isLoadedSourceLocation(RC->getLocStart()));
   std::string Doc = RC->getFormattedText(Ctx.getSourceManager(), 
Ctx.getDiagnostics());
   if (!LooksLikeDocComment(Doc))
 return "";
@@ -104,11 +77,14 @@
const CodeCompleteConsumer::OverloadCandidate ,
unsigned ArgIndex, bool CommentsFromHeaders) {
   auto *Func = Result.getFunction();
-  if (!Func || !canRequestComment(Ctx, *Func, CommentsFromHeaders))
+  if (!Func)
 return "";
   const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
   if (!RC)
 return "";
+  // Sanity check that the comment does not come from the PCH. We choose to not
+  // write them into PCH, because they are racy and slow to load.
+  assert(!Ctx.getSourceManager().isLoadedSourceLocation(RC->getLocStart()));
   std::string Doc = RC->getFormattedText(Ctx.getSourceManager(), 
Ctx.getDiagnostics());
   if (!LooksLikeDocComment(Doc))
 return "";
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -24,6 +24,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Serialization/ASTWriter.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -323,6 +324,9 @@
   // the preamble and make it smaller.
   assert(!CI.getFrontendOpts().SkipFunctionBodies);
   CI.getFrontendOpts().SkipFunctionBodies = true;
+  // We don't want to write comments into PCH. They are racy and slow to read
+  // back. We rely on dynamic index for the comments instead.
+  CI.getPreprocessorOpts().WriteCommentListToPCH = false;
 
   CppFilePreambleCallbacks SerializedDeclsCollector(FileName, 
PreambleCallback);
   if (Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) {


Index: clangd/CodeCompletionStrings.cpp
===
--- clangd/CodeCompletionStrings.cpp
+++ clangd/CodeCompletionStrings.cpp
@@ -32,34 +32,6 @@
   }
 }
 
-bool canRequestComment(const ASTContext , const NamedDecl ,
-   bool CommentsFromHeaders) {
-  if (CommentsFromHeaders)
-return true;
-  auto  = Ctx.getSourceManager();
-  // Accessing comments for decls from  invalid preamble can lead to crashes.
-  // So we only return comments from the main file when doing code completion.
-  // For indexing, we still read all the comments.
-  // FIXME: 

[clang-tools-extra] r336283 - [clang-tidy] Fix http://llvm.org/PR38055

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Wed Jul  4 08:19:49 2018
New Revision: 336283

URL: http://llvm.org/viewvc/llvm-project?rev=336283=rev
Log:
[clang-tidy] Fix http://llvm.org/PR38055

Modified:
clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp?rev=336283=336282=336283=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp Wed Jul  
4 08:19:49 2018
@@ -159,8 +159,9 @@ void UnusedParametersCheck::warnOnUnused
   MyDiag << removeParameter(Result, FD, ParamIndex);
 
   // Fix all call sites.
-  for (const auto *Call : Indexer->getFnCalls(Function))
-MyDiag << removeArgument(Result, Call, ParamIndex);
+  for (const CallExpr *Call : Indexer->getFnCalls(Function))
+if (ParamIndex < Call->getNumArgs()) // See PR38055 for example.
+  MyDiag << removeArgument(Result, Call, ParamIndex);
 }
 
 void UnusedParametersCheck::check(const MatchFinder::MatchResult ) {

Modified: clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp?rev=336283=336282=336283=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp Wed Jul  
4 08:19:49 2018
@@ -222,6 +222,21 @@ static Function dontGe
   return Function();
 }
 
+namespace PR38055 {
+namespace {
+struct a {
+  void b(int c) {;}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: parameter 'c' is unused
+// CHECK-FIXES: {{^}}  void b() {;}{{$}}
+};
+template 
+class d {
+  a e;
+  void f() { e.b(); }
+};
+}  // namespace
+}  // namespace PR38055
+
 namespace strict_mode_off {
 // Do not warn on empty function bodies.
 void f1(int foo1) {}


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

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

In https://reviews.llvm.org/D47698#1141871, @r.stahl wrote:

> improved code quality; added nested macro test. it "works", but is disabled 
> because it revealed another bug: the function end location is not imported. 
> will send a patch


Related to this.


Repository:
  rC Clang

https://reviews.llvm.org/D48941



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

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: martong, a.sidorin, balazske, xazax.hun.
Herald added subscribers: cfe-commits, rnkovacs.

On constructors that do not take the end source location, it was not imported. 
Fixes test from https://reviews.llvm.org/D47698 / 
https://reviews.llvm.org/rC336269.


Repository:
  rC Clang

https://reviews.llvm.org/D48941

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
   FromSM);
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
   Decl *FromTU = getTuDecl(
   R"(
   #define FUNC_INT void declToImport
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2533,6 +2533,7 @@
 D->isInlineSpecified(), 
 D->isImplicit(),
 D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
 if (unsigned NumInitializers = FromConstructor->getNumCtorInitializers()) {
   SmallVector CtorInitializers;
   for (auto *I : FromConstructor->inits()) {
@@ -2555,6 +2556,7 @@
NameInfo, T, TInfo,
D->isInlineSpecified(),
D->isImplicit());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   } else if (auto *FromConversion = dyn_cast(D)) {
 ToFunction = CXXConversionDecl::Create(Importer.getToContext(), 
cast(DC),
@@ -2580,6 +2582,7 @@
   D->isInlineSpecified(),
   D->hasWrittenPrototype(),
   D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   }
 
   // Import the qualifier, if any.


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
   FromSM);
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
   Decl *FromTU = getTuDecl(
   R"(
   #define FUNC_INT void declToImport
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2533,6 +2533,7 @@
 D->isInlineSpecified(), 
 D->isImplicit(),
 D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
 if (unsigned NumInitializers = FromConstructor->getNumCtorInitializers()) {
   SmallVector CtorInitializers;
   for (auto *I : FromConstructor->inits()) {
@@ -2555,6 +2556,7 @@
NameInfo, T, TInfo,
D->isInlineSpecified(),
D->isImplicit());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   } else if (auto *FromConversion = dyn_cast(D)) {
 ToFunction = CXXConversionDecl::Create(Importer.getToContext(), 
cast(DC),
@@ -2580,6 +2582,7 @@
   D->isInlineSpecified(),
   D->hasWrittenPrototype(),
   D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   }
 
   // Import the qualifier, if any.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48044: [Power9] Update fp128 as a valid homogenous aggregate base type

[Nemanja Ivanovic via Phabricator via cfe-commits]

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

Other than a few style nits that can be fixed on the commit, this LGTM.




Comment at: include/clang/AST/Type.h:1802
   bool isFloat16Type() const;  // C11 extension ISO/IEC TS 18661
+  bool isFloat128Type() const;
   bool isRealType() const; // C99 6.2.5p17 (real floating + integer)

// IEEE 754 binary128



Comment at: lib/CodeGen/TargetInfo.cpp:4609
   // Homogeneous aggregates for ELFv2 must have base types of float,
   // double, long double, or 128-bit vectors.
   if (const BuiltinType *BT = Ty->getAs()) {

This comment should probably be updated.



Comment at: lib/CodeGen/TargetInfo.cpp:4633
   uint32_t NumRegs =
-  Base->isVectorType() ? 1 : (getContext().getTypeSize(Base) + 63) / 64;
+  ((getContext().getTargetInfo().hasFloat128Type() &&
+  Base->isFloat128Type()) ||

This expression looks very messy, I think it's probably better to rewrite it as 
multiple expressions or an `if` statement.



Comment at: test/CodeGen/ppc64le-f128Aggregates.c:19
+
+struct fp2a2b { __float128 a[2]; __float128 b[2]; };
+

Is it still a homogeneous aggregate if it's nested?
i.e. `struct fp10 { struct fp2 a, struct fp3 b };`

And if so, should we add that to the test?


https://reviews.llvm.org/D48044



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

Re: r335795 - [CFG] [analyzer] Add construction contexts that explain pre-C++17 copy elision.

[Alexander Kornienko via cfe-commits]

We've started seeing an assertion failure after this commit:
assert.h assertion failed at llvm/tools/clang/lib/Analysis/CFG.cpp:1266 in
void (anonymous namespace)::CFGBuilder::findConstructionContexts(const
clang::ConstructionContextLayer *, clang::Stmt *): CE->getNumArgs() == 1

The stack trace is:
::CFGBuilder::findConstructionContexts
::CFGBuilder::VisitReturnStmt
::CFGBuilder::Visit
::CFGBuilder::addStmt
::CFGBuilder::VisitCompoundStmt
::CFGBuilder::Visit
::CFGBuilder::addStmt
::CFGBuilder::buildCFG
clang::CFG::buildCFG
clang::AnalysisDeclContext::getCFG
clang::ento::AnalysisManager::getCFG
::AnalysisConsumer::HandleCode
clang::RecursiveASTVisitor::TraverseDecl
clang::RecursiveASTVisitor::TraverseDeclContextHelper
clang::RecursiveASTVisitor::TraverseDecl
clang::RecursiveASTVisitor::TraverseDeclContextHelper
clang::RecursiveASTVisitor::TraverseDecl
::AnalysisConsumer::runAnalysisOnTranslationUnit
::AnalysisConsumer::HandleTranslationUnit

No test case yet.

On Thu, Jun 28, 2018 at 2:09 AM Artem Dergachev via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: dergachev
> Date: Wed Jun 27 17:04:54 2018
> New Revision: 335795
>
> URL: http://llvm.org/viewvc/llvm-project?rev=335795=rev
> Log:
> [CFG] [analyzer] Add construction contexts that explain pre-C++17 copy
> elision.
>
> Before C++17 copy elision was optional, even if the elidable copy/move
> constructor had arbitrary side effects. The elidable constructor is present
> in the AST, but marked as elidable.
>
> In these cases CFG now contains additional information that allows its
> clients
> to figure out if a temporary object is only being constructed so that to
> pass
> it to an elidable constructor. If so, it includes a reference to the
> elidable
> constructor's construction context, so that the client could elide the
> elidable constructor and construct the object directly at its final
> destination.
>
> Differential Revision: https://reviews.llvm.org/D47616
>
> Modified:
> cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
> cfe/trunk/include/clang/Analysis/CFG.h
> cfe/trunk/include/clang/Analysis/ConstructionContext.h
> cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
> cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
> cfe/trunk/lib/Analysis/CFG.cpp
> cfe/trunk/lib/Analysis/ConstructionContext.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
> cfe/trunk/test/Analysis/analyzer-config.c
> cfe/trunk/test/Analysis/analyzer-config.cpp
> cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
> cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp
>
> Modified: cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h?rev=335795=335794=335795=diff
>
> ==
> --- cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h (original)
> +++ cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h Wed Jun 27
> 17:04:54 2018
> @@ -451,6 +451,7 @@ public:
>   bool addStaticInitBranches = false,
>   bool addCXXNewAllocator = true,
>   bool addRichCXXConstructors = true,
> + bool markElidedCXXConstructors = true,
>   CodeInjector *injector = nullptr);
>
>AnalysisDeclContext *getContext(const Decl *D);
>
> Modified: cfe/trunk/include/clang/Analysis/CFG.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/CFG.h?rev=335795=335794=335795=diff
>
> ==
> --- cfe/trunk/include/clang/Analysis/CFG.h (original)
> +++ cfe/trunk/include/clang/Analysis/CFG.h Wed Jun 27 17:04:54 2018
> @@ -1025,6 +1025,7 @@ public:
>  bool AddCXXNewAllocator = false;
>  bool AddCXXDefaultInitExprInCtors = false;
>  bool AddRichCXXConstructors = false;
> +bool MarkElidedCXXConstructors = false;
>
>  BuildOptions() = default;
>
>
> Modified: cfe/trunk/include/clang/Analysis/ConstructionContext.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ConstructionContext.h?rev=335795=335794=335795=diff
>
> ==
> --- cfe/trunk/include/clang/Analysis/ConstructionContext.h (original)
> +++ cfe/trunk/include/clang/Analysis/ConstructionContext.h Wed Jun 27
> 17:04:54 2018
> @@ -107,7 +107,10 @@ public:
>  INITIALIZER_BEGIN = SimpleConstructorInitializerKind,
>  INITIALIZER_END = CXX17ElidedCopyConstructorInitializerKind,
>  NewAllocatedObjectKind,
> -TemporaryObjectKind,
> +SimpleTemporaryObjectKind,
> +ElidedTemporaryObjectKind,
> +TEMPORARY_BEGIN = 

Re: r335800 - [analyzer] Add support for pre-C++17 copy elision.

[Alexander Kornienko via cfe-commits]

We've started seeing assertion failures after this commit.
assert.h assertion failed at
llvm/tools/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:485 in static
clang::ento::ProgramStateRef
clang::ento::ExprEngine::elideDestructor(clang::ento::ProgramStateRef,
const clang::CXXBindTemporaryExpr *, const clang::LocationContext *):
!State->contains(I)


The stack trace is
clang::ento::ExprEngine::elideDestructor
clang::ento::ExprEngine::prepareForObjectConstruction
clang::ento::ExprEngine::prepareForObjectConstruction
clang::ento::ExprEngine::VisitCXXConstructExpr
clang::ento::ExprEngine::Visit
clang::ento::ExprEngine::ProcessStmt
clang::ento::ExprEngine::processCFGElement
clang::ento::CoreEngine::HandlePostStmt
clang::ento::CoreEngine::dispatchWorkItem
clang::ento::CoreEngine::ExecuteWorkList
clang::ento::ExprEngine::ExecuteWorkList
::AnalysisConsumer::ActionExprEngine
::AnalysisConsumer::HandleCode
::AnalysisConsumer::HandleDeclsCallGraph
::AnalysisConsumer::runAnalysisOnTranslationUnit
::AnalysisConsumer::HandleTranslationUnit

I haven't come up with a test case yet.

On Thu, Jun 28, 2018 at 2:34 AM Artem Dergachev via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: dergachev
> Date: Wed Jun 27 17:30:18 2018
> New Revision: 335800
>
> URL: http://llvm.org/viewvc/llvm-project?rev=335800=rev
> Log:
> [analyzer] Add support for pre-C++17 copy elision.
>
> r335795 adds copy elision information to CFG. This commit allows static
> analyzer
> to elide elidable copy constructors by constructing the objects that were
> previously subject to elidable copy directly in the target region of the
> copy.
>
> The chain of elided constructors may potentially be indefinitely long. This
> only happens when the object is being returned from a function which in
> turn is
> returned from another function, etc.
>
> NRVO is not supported yet.
>
> Differential Revision: https://reviews.llvm.org/D47671
>
> Modified:
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
> cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp
> cfe/trunk/test/Analysis/gtest.cpp
> cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp
> cfe/trunk/test/Analysis/lifetime-extension.cpp
> cfe/trunk/test/Analysis/temporaries.cpp
>
> Modified:
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=335800=335799=335800=diff
>
> ==
> --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
> (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
> Wed Jun 27 17:30:18 2018
> @@ -780,9 +780,30 @@ private:
>llvm::PointerUnion P,
>const LocationContext *LC);
>
> +  /// If the given expression corresponds to a temporary that was used for
> +  /// passing into an elidable copy/move constructor and that constructor
> +  /// was actually elided, track that we also need to elide the
> destructor.
> +  static ProgramStateRef elideDestructor(ProgramStateRef State,
> + const CXXBindTemporaryExpr *BTE,
> + const LocationContext *LC);
> +
> +  /// Stop tracking the destructor that corresponds to an elided
> constructor.
> +  static ProgramStateRef
> +  cleanupElidedDestructor(ProgramStateRef State,
> +  const CXXBindTemporaryExpr *BTE,
> +  const LocationContext *LC);
> +
> +  /// Returns true if the given expression corresponds to a temporary that
> +  /// was constructed for passing into an elidable copy/move constructor
> +  /// and that constructor was actually elided.
> +  static bool isDestructorElided(ProgramStateRef State,
> + const CXXBindTemporaryExpr *BTE,
> + const LocationContext *LC);
> +
>/// Check if all objects under construction have been fully constructed
>/// for the given context range (including FromLC, not including ToLC).
> -  /// This is useful for assertions.
> +  /// This is useful for assertions. Also checks if elided destructors
> +  /// were cleaned up.
>static bool areAllObjectsFullyConstructed(ProgramStateRef State,
>  const LocationContext *FromLC,
>  const LocationContext *ToLC);
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=335800=335799=335800=diff
>
> ==
> --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
> 

r336275 - [analyzer][ctu] fix unsortable diagnostics

[Rafael Stahl via cfe-commits]

Author: r.stahl
Date: Wed Jul  4 07:12:58 2018
New Revision: 336275

URL: http://llvm.org/viewvc/llvm-project?rev=336275=rev
Log:
[analyzer][ctu] fix unsortable diagnostics

Summary: In the provided test case the PathDiagnostic compare function was not 
able to find a difference.

Reviewers: xazax.hun, NoQ, dcoughlin, george.karpenkov

Reviewed By: george.karpenkov

Subscribers: a_sidorin, szepet, rnkovacs, a.sidorin, mikhail.ramalho, 
cfe-commits

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

Added:
cfe/trunk/test/Analysis/ctu-hdr.h
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
cfe/trunk/test/Analysis/Inputs/ctu-other.cpp
cfe/trunk/test/Analysis/Inputs/externalFnMap.txt
cfe/trunk/test/Analysis/ctu-main.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp?rev=336275=336274=336275=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp Wed Jul  4 07:12:58 
2018
@@ -406,11 +406,15 @@ static bool compareCrossTUSourceLocs(Ful
   std::pair InSameTU = SM.isInTheSameTranslationUnit(XOffs, YOffs);
   if (InSameTU.first)
 return XL.isBeforeInTranslationUnitThan(YL);
-  const FileEntry *XFE = SM.getFileEntryForID(XL.getFileID());
-  const FileEntry *YFE = SM.getFileEntryForID(YL.getFileID());
+  const FileEntry *XFE = SM.getFileEntryForID(XL.getSpellingLoc().getFileID());
+  const FileEntry *YFE = SM.getFileEntryForID(YL.getSpellingLoc().getFileID());
   if (!XFE || !YFE)
 return XFE && !YFE;
-  return XFE->getName() < YFE->getName();
+  int NameCmp = XFE->getName().compare(YFE->getName());
+  if (NameCmp != 0)
+return NameCmp == -1;
+  // Last resort: Compare raw file IDs that are possibly expansions.
+  return XL.getFileID() < YL.getFileID();
 }
 
 static bool compare(const PathDiagnostic , const PathDiagnostic ) {

Modified: cfe/trunk/test/Analysis/Inputs/ctu-other.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/ctu-other.cpp?rev=336275=336274=336275=diff
==
--- cfe/trunk/test/Analysis/Inputs/ctu-other.cpp (original)
+++ cfe/trunk/test/Analysis/Inputs/ctu-other.cpp Wed Jul  4 07:12:58 2018
@@ -1,3 +1,5 @@
+#include "../ctu-hdr.h"
+
 int callback_to_main(int x);
 int f(int x) {
   return x - 1;
@@ -68,3 +70,8 @@ int chf1(int x) {
 
 typedef struct { int n; } Anonymous;
 int fun_using_anon_struct(int n) { Anonymous anon; anon.n = n; return anon.n; }
+
+int other_macro_diag(int x) {
+  MACRODIAG();
+  return x;
+}

Modified: cfe/trunk/test/Analysis/Inputs/externalFnMap.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/externalFnMap.txt?rev=336275=336274=336275=diff
==
--- cfe/trunk/test/Analysis/Inputs/externalFnMap.txt (original)
+++ cfe/trunk/test/Analysis/Inputs/externalFnMap.txt Wed Jul  4 07:12:58 2018
@@ -12,3 +12,4 @@ c:@F@h_chain#I# ctu-chain.cpp.ast
 c:@N@chns@S@chcls@F@chf4#I# ctu-chain.cpp.ast
 c:@N@chns@F@chf2#I# ctu-chain.cpp.ast
 c:@F@fun_using_anon_struct#I# ctu-other.cpp.ast
+c:@F@other_macro_diag#I# ctu-other.cpp.ast

Added: cfe/trunk/test/Analysis/ctu-hdr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ctu-hdr.h?rev=336275=auto
==
--- cfe/trunk/test/Analysis/ctu-hdr.h (added)
+++ cfe/trunk/test/Analysis/ctu-hdr.h Wed Jul  4 07:12:58 2018
@@ -0,0 +1,3 @@
+#define MACRODIAG() clang_analyzer_warnIfReached()
+
+void clang_analyzer_warnIfReached();

Modified: cfe/trunk/test/Analysis/ctu-main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ctu-main.cpp?rev=336275=336274=336275=diff
==
--- cfe/trunk/test/Analysis/ctu-main.cpp (original)
+++ cfe/trunk/test/Analysis/ctu-main.cpp Wed Jul  4 07:12:58 2018
@@ -4,6 +4,8 @@
 // RUN: cp %S/Inputs/externalFnMap.txt %T/ctudir/
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze 
-analyzer-checker=core,debug.ExprInspection -analyzer-config 
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir 
-verify %s
 
+#include "ctu-hdr.h"
+
 void clang_analyzer_eval(int);
 
 int f(int);
@@ -41,6 +43,7 @@ int chf1(int x);
 }
 
 int fun_using_anon_struct(int);
+int other_macro_diag(int);
 
 int main() {
   clang_analyzer_eval(f(3) == 2); // expected-warning{{TRUE}}
@@ -58,4 +61,8 @@ int main() {
 
   clang_analyzer_eval(chns::chf1(4) == 12); // expected-warning{{TRUE}}
   clang_analyzer_eval(fun_using_anon_struct(8) == 8); // 
expected-warning{{TRUE}}
+
+  clang_analyzer_eval(other_macro_diag(1) == 1); // 

[PATCH] D48474: [analyzer][ctu] fix unsortable diagnostics

[Rafael Stahl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC336275: [analyzer][ctu] fix unsortable diagnostics (authored 
by r.stahl, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48474?vs=152433=154107#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48474

Files:
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/Inputs/ctu-other.cpp
  test/Analysis/Inputs/externalFnMap.txt
  test/Analysis/ctu-hdr.h
  test/Analysis/ctu-main.cpp


Index: test/Analysis/ctu-hdr.h
===
--- test/Analysis/ctu-hdr.h
+++ test/Analysis/ctu-hdr.h
@@ -0,0 +1,3 @@
+#define MACRODIAG() clang_analyzer_warnIfReached()
+
+void clang_analyzer_warnIfReached();
Index: test/Analysis/ctu-main.cpp
===
--- test/Analysis/ctu-main.cpp
+++ test/Analysis/ctu-main.cpp
@@ -4,6 +4,8 @@
 // RUN: cp %S/Inputs/externalFnMap.txt %T/ctudir/
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze 
-analyzer-checker=core,debug.ExprInspection -analyzer-config 
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir 
-verify %s
 
+#include "ctu-hdr.h"
+
 void clang_analyzer_eval(int);
 
 int f(int);
@@ -41,6 +43,7 @@
 }
 
 int fun_using_anon_struct(int);
+int other_macro_diag(int);
 
 int main() {
   clang_analyzer_eval(f(3) == 2); // expected-warning{{TRUE}}
@@ -58,4 +61,8 @@
 
   clang_analyzer_eval(chns::chf1(4) == 12); // expected-warning{{TRUE}}
   clang_analyzer_eval(fun_using_anon_struct(8) == 8); // 
expected-warning{{TRUE}}
+
+  clang_analyzer_eval(other_macro_diag(1) == 1); // expected-warning{{TRUE}}
+  // expected-warning@Inputs/ctu-other.cpp:75{{REACHABLE}}
+  MACRODIAG(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/Inputs/externalFnMap.txt
===
--- test/Analysis/Inputs/externalFnMap.txt
+++ test/Analysis/Inputs/externalFnMap.txt
@@ -12,3 +12,4 @@
 c:@N@chns@S@chcls@F@chf4#I# ctu-chain.cpp.ast
 c:@N@chns@F@chf2#I# ctu-chain.cpp.ast
 c:@F@fun_using_anon_struct#I# ctu-other.cpp.ast
+c:@F@other_macro_diag#I# ctu-other.cpp.ast
Index: test/Analysis/Inputs/ctu-other.cpp
===
--- test/Analysis/Inputs/ctu-other.cpp
+++ test/Analysis/Inputs/ctu-other.cpp
@@ -1,3 +1,5 @@
+#include "../ctu-hdr.h"
+
 int callback_to_main(int x);
 int f(int x) {
   return x - 1;
@@ -68,3 +70,8 @@
 
 typedef struct { int n; } Anonymous;
 int fun_using_anon_struct(int n) { Anonymous anon; anon.n = n; return anon.n; }
+
+int other_macro_diag(int x) {
+  MACRODIAG();
+  return x;
+}
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -406,11 +406,15 @@
   std::pair InSameTU = SM.isInTheSameTranslationUnit(XOffs, YOffs);
   if (InSameTU.first)
 return XL.isBeforeInTranslationUnitThan(YL);
-  const FileEntry *XFE = SM.getFileEntryForID(XL.getFileID());
-  const FileEntry *YFE = SM.getFileEntryForID(YL.getFileID());
+  const FileEntry *XFE = SM.getFileEntryForID(XL.getSpellingLoc().getFileID());
+  const FileEntry *YFE = SM.getFileEntryForID(YL.getSpellingLoc().getFileID());
   if (!XFE || !YFE)
 return XFE && !YFE;
-  return XFE->getName() < YFE->getName();
+  int NameCmp = XFE->getName().compare(YFE->getName());
+  if (NameCmp != 0)
+return NameCmp == -1;
+  // Last resort: Compare raw file IDs that are possibly expansions.
+  return XL.getFileID() < YL.getFileID();
 }
 
 static bool compare(const PathDiagnostic , const PathDiagnostic ) {


Index: test/Analysis/ctu-hdr.h
===
--- test/Analysis/ctu-hdr.h
+++ test/Analysis/ctu-hdr.h
@@ -0,0 +1,3 @@
+#define MACRODIAG() clang_analyzer_warnIfReached()
+
+void clang_analyzer_warnIfReached();
Index: test/Analysis/ctu-main.cpp
===
--- test/Analysis/ctu-main.cpp
+++ test/Analysis/ctu-main.cpp
@@ -4,6 +4,8 @@
 // RUN: cp %S/Inputs/externalFnMap.txt %T/ctudir/
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir -verify %s
 
+#include "ctu-hdr.h"
+
 void clang_analyzer_eval(int);
 
 int f(int);
@@ -41,6 +43,7 @@
 }
 
 int fun_using_anon_struct(int);
+int other_macro_diag(int);
 
 int main() {
   clang_analyzer_eval(f(3) == 2); // expected-warning{{TRUE}}
@@ -58,4 +61,8 @@
 
   clang_analyzer_eval(chns::chf1(4) == 12); // expected-warning{{TRUE}}
   clang_analyzer_eval(fun_using_anon_struct(8) == 8); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(other_macro_diag(1) == 1); // expected-warning{{TRUE}}
+  // 

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

[Henry Wong via Phabricator via cfe-commits]

MTC added a comment.

Thanks for your review, NoQ!




Comment at: lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp:68
 : IILockGuard(nullptr), IIUniqueLock(nullptr),
-  LockFn("lock"), UnlockFn("unlock"), SleepFn("sleep"), GetcFn("getc"),
-  FgetsFn("fgets"), ReadFn("read"), RecvFn("recv"),
-  PthreadLockFn("pthread_mutex_lock"),
-  PthreadTryLockFn("pthread_mutex_trylock"),
-  PthreadUnlockFn("pthread_mutex_unlock"),
-  MtxLock("mtx_lock"),
-  MtxTimedLock("mtx_timedlock"),
-  MtxTryLock("mtx_trylock"),
-  MtxUnlock("mtx_unlock"),
+  LockFn({"lock"}), UnlockFn({"unlock"}), SleepFn({"sleep"}), 
GetcFn({"getc"}),
+  FgetsFn({"fgets"}), ReadFn({"read"}), RecvFn({"recv"}),

NoQ wrote:
> I wish the old syntax for simple C functions was preserved.
> 
> This could be accidentally achieved by using `ArrayRef` instead of 
> `std::vector` for your constructor's argument.
`ArrayRef` can't achieve this goal. The only way I can figure out is 
changing the declaration to the following form.

`CallDescription(StringRef FuncName, unsigned RequiredArgs = NoArgRequirement, 
ArrayRef QualifiedName = None) {}`



Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:273-280
+std::string Regex = "^::(.*)?";
+Regex += llvm::join(CD.QualifiedName, "(.*)?::(.*)?") + "(.*)?$";
+
+auto Matches = match(namedDecl(matchesName(Regex)).bind("Regex"), *ND,
+ LCtx->getAnalysisDeclContext()->getASTContext());
+
+if (Matches.empty())

NoQ wrote:
> Uhm, i think we don't need to flatten the class name to a string and then 
> regex to do this.
> 
> We can just unwrap the declaration and see if the newly unwrapped layer 
> matches the expected name on every step, until all expected names have been 
> found.
Is the way you mentioned above is similar `NamedDecl::printQualifiedName()`? 
Get the full name through the `DeclContext` chain. See 
https://github.com/llvm-mirror/clang/blob/master/lib/AST/Decl.cpp#L1511.

Or ignore namespace ,like `std`, just only consider the `CXXRecordDecl`? If so, 
`dyn_cast<>` might be enough.


Repository:
  rC Clang

https://reviews.llvm.org/D48027



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

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

In https://reviews.llvm.org/D48903#1151866, @ilya-biryukov wrote:

> Mimicing RealFS seems like the right thing to do here, so I would vouch for 
> checking this change in.
>  I'm a little worried that there are tests/clients relying on the old 
> behavior, have you run all the tests?


I didn't have time yesterday, I'm doing this now.  Is `ninja/make clang-test` 
sufficient?


Repository:
  rC Clang

https://reviews.llvm.org/D48903



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

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

In https://reviews.llvm.org/D48903#1151847, @hokein wrote:

> I'm not familiar with this part of code, but the change looks fine to me. I 
> think @bkramer is the right person to review it.
>
> Please make sure the style align with LLVM code style.


Woops indeed I forgot to run `git clang-format`.


Repository:
  rC Clang

https://reviews.llvm.org/D48903



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

[PATCH] D48938: [clangd] Track origins of symbols (various indexes, Sema).

[Eric Liu via Phabricator via cfe-commits]

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

lgtm. neat!




Comment at: clangd/index/Index.cpp:50
+return OS << "unknown";
+  static char Sigils[] = "ADSM4567";
+  for (unsigned I = 0; I < sizeof(Sigils); ++I)

nit: `constexpr`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48938



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

[PATCH] D48940: [clangd] Wait for first preamble before code completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: jkorous, MaskRay, ioeric, javed.absar.

To avoid doing extra work of processing headers in the preamble
mutilple times in parallel.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48940

Files:
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h


Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -101,6 +101,10 @@
   /// - validate that the preamble is still valid, and only use it in this case
   /// - accept that preamble contents may be outdated, and try to avoid reading
   ///   source code from headers.
+  /// However, Action will be scheduled to run after the first rebuild of the
+  /// preamble for the corresponding file finishes. Note that the rebuild can
+  /// still fail, in which case Action will get a null pointer instead of the
+  /// preamble.
   /// If an error occurs during processing, it is forwarded to the \p Action
   /// callback.
   void runWithPreamble(llvm::StringRef Name, PathRef File,
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -176,6 +176,12 @@
   bool blockUntilIdle(Deadline Timeout) const;
 
   std::shared_ptr getPossiblyStalePreamble() const;
+  /// Wait for the first build of preamble to finish. Preamble itself can be
+  /// accessed via getPossibleStalePreamble(). Note that this function will
+  /// return after an unsuccessful build of the preamble too, i.e. result of
+  /// getPossiblyStalePreamble() can be null even after this function returns.
+  void waitForFirstPreamble() const;
+
   std::size_t getUsedBytes() const;
   bool isASTCached() const;
 
@@ -226,6 +232,8 @@
   /// Guards members used by both TUScheduler and the worker thread.
   mutable std::mutex Mutex;
   std::shared_ptr LastBuiltPreamble; /* GUARDED_BY(Mutex) 
*/
+  /// Becomes ready when the first preamble build finishes.
+  Notification PreambleWasBuilt;
   /// Set to true to signal run() to finish processing.
   bool Done;/* GUARDED_BY(Mutex) */
   std::deque Requests; /* GUARDED_BY(Mutex) */
@@ -329,6 +337,9 @@
 buildCompilerInvocation(Inputs);
 if (!Invocation) {
   log("Could not build CompilerInvocation for file " + FileName);
+  // Make sure anyone waiting for the preamble gets notified it could not
+  // be built.
+  PreambleWasBuilt.notify();
   return;
 }
 
@@ -340,6 +351,8 @@
   if (NewPreamble)
 LastBuiltPreamble = NewPreamble;
 }
+PreambleWasBuilt.notify();
+
 // Build the AST for diagnostics.
 llvm::Optional AST =
 buildAST(FileName, std::move(Invocation), Inputs, NewPreamble, PCHs);
@@ -392,6 +405,10 @@
   return LastBuiltPreamble;
 }
 
+void ASTWorker::waitForFirstPreamble() const {
+  PreambleWasBuilt.wait();
+}
+
 std::size_t ASTWorker::getUsedBytes() const {
   // Note that we don't report the size of ASTs currently used for processing
   // the in-flight requests. We used this information for debugging purposes
@@ -655,6 +672,11 @@
  std::string Contents,
  tooling::CompileCommand Command, Context Ctx,
  decltype(Action) Action) mutable {
+// We don't want to be running preamble actions before the preamble was
+// built for the first time. This avoids extra work of processing the
+// preamble headers in parallel multiple times.
+Worker->waitForFirstPreamble();
+
 std::lock_guard BarrierLock(Barrier);
 WithContext Guard(std::move(Ctx));
 trace::Span Tracer(Name);


Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -101,6 +101,10 @@
   /// - validate that the preamble is still valid, and only use it in this case
   /// - accept that preamble contents may be outdated, and try to avoid reading
   ///   source code from headers.
+  /// However, Action will be scheduled to run after the first rebuild of the
+  /// preamble for the corresponding file finishes. Note that the rebuild can
+  /// still fail, in which case Action will get a null pointer instead of the
+  /// preamble.
   /// If an error occurs during processing, it is forwarded to the \p Action
   /// callback.
   void runWithPreamble(llvm::StringRef Name, PathRef File,
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -176,6 +176,12 @@
   bool blockUntilIdle(Deadline Timeout) const;
 
   std::shared_ptr getPossiblyStalePreamble() const;
+  /// Wait for the first build of preamble to finish. Preamble itself can be
+  /// accessed via getPossibleStalePreamble(). Note that this function will
+  /// return 

[PATCH] D48938: [clangd] Track origins of symbols (various indexes, Sema).

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov.

Surface it in the completion items C++ API, and when a flag is set.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48938

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
  clangd/index/FileIndex.cpp
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/Merge.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/IndexTests.cpp

Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -282,6 +282,8 @@
   DetR.Documentation = "--doc--";
   L.Detail = 
   R.Detail = 
+  L.Origin = SymbolOrigin::Dynamic;
+  R.Origin = SymbolOrigin::Static;
 
   Symbol::Details Scratch;
   Symbol M = mergeSymbol(L, R, );
@@ -293,6 +295,8 @@
   ASSERT_TRUE(M.Detail);
   EXPECT_EQ(M.Detail->ReturnType, "DetL");
   EXPECT_EQ(M.Detail->Documentation, "--doc--");
+  EXPECT_EQ(M.Origin,
+SymbolOrigin::Dynamic | SymbolOrigin::Static | SymbolOrigin::Merge);
 }
 
 TEST(MergeTest, PreferSymbolWithDefn) {
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -59,6 +59,7 @@
 }
 MATCHER(InsertInclude, "") { return bool(arg.HeaderInsertion); }
 MATCHER_P(SnippetSuffix, Text, "") { return arg.SnippetSuffix == Text; }
+MATCHER_P(Origin, OriginSet, "") { return arg.Origin == OriginSet; }
 
 // Shorthand for Contains(Named(Name)).
 Matcher &> Has(std::string Name) {
@@ -137,6 +138,7 @@
   Sym.ID = SymbolID(USR);
   Sym.SymInfo.Kind = Kind;
   Sym.IsIndexedForCodeCompletion = true;
+  Sym.Origin = SymbolOrigin::Static;
   return Sym;
 }
 Symbol func(StringRef Name) { // Assumes the function has no args.
@@ -511,9 +513,12 @@
   )cpp",
   {func("ns::both"), cls("ns::Index")});
   // We get results from both index and sema, with no duplicates.
-  EXPECT_THAT(
-  Results.Completions,
-  UnorderedElementsAre(Named("local"), Named("Index"), Named("both")));
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(
+  AllOf(Named("local"), Origin(SymbolOrigin::AST)),
+  AllOf(Named("Index"), Origin(SymbolOrigin::Static)),
+  AllOf(Named("both"),
+Origin(SymbolOrigin::AST | SymbolOrigin::Static;
 }
 
 TEST(CompletionTest, SemaIndexMergeWithLimit) {
@@ -1252,6 +1257,8 @@
   C.Header = "\"foo.h\"";
   C.Kind = CompletionItemKind::Method;
   C.Score.Total = 1.0;
+  C.Origin =
+  static_cast(SymbolOrigin::AST | SymbolOrigin::Static);
 
   CodeCompleteOptions Opts;
   Opts.IncludeIndicator.Insert = "^";
@@ -1278,6 +1285,10 @@
   EXPECT_EQ(R.label, "^Foo::x(bool) const");
   EXPECT_THAT(R.additionalTextEdits, Not(IsEmpty()));
 
+  Opts.ShowOrigins = true;
+  R = C.render(Opts);
+  EXPECT_EQ(R.label, "^[AS]Foo::x(bool) const");
+
   C.BundleSize = 2;
   R = C.render(Opts);
   EXPECT_EQ(R.detail, "[2 overloads]\n\"foo.h\"");
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -143,6 +143,11 @@
"Clang uses an index built from symbols in opened files"),
 llvm::cl::init(true));
 
+static llvm::cl::opt ShowOrigins(
+"debug-origin", llvm::cl::desc("Show origins of completion items"),
+llvm::cl::init(clangd::CodeCompleteOptions().ShowOrigins),
+llvm::cl::Hidden);
+
 static llvm::cl::opt YamlSymbolFile(
 "yaml-symbol-file",
 llvm::cl::desc(
@@ -261,6 +266,7 @@
   CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
   CCOpts.Limit = LimitResults;
   CCOpts.BundleOverloads = CompletionStyle != Detailed;
+  CCOpts.ShowOrigins = ShowOrigins;
 
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(Out, CCOpts, CompileCommandsDirPath, Opts);
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -52,6 +52,8 @@
 const CanonicalIncludes *Includes = nullptr;
 // Populate the Symbol.References field.
 bool CountReferences = false;
+// Every symbol collected will be stamped with this origin.
+SymbolOrigin Origin = SymbolOrigin::Unknown;
   };
 
   SymbolCollector(Options Opts);
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -411,6 +411,7 @@
   Detail.IncludeHeader = Include;
   S.Detail = 
 
+  

r336269 - [ASTImporter] import macro source locations

[Rafael Stahl via cfe-commits]

Author: r.stahl
Date: Wed Jul  4 06:34:05 2018
New Revision: 336269

URL: http://llvm.org/viewvc/llvm-project?rev=336269=rev
Log:
[ASTImporter] import macro source locations

Summary: Implement full import of macro expansion info with spelling and 
expansion locations.

Reviewers: a.sidorin, klimek, martong, balazske, xazax.hun

Reviewed By: martong

Subscribers: thakis, xazax.hun, balazske, rnkovacs, cfe-commits

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

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=336269=336268=336269=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Wed Jul  4 06:34:05 2018
@@ -7164,19 +7164,13 @@ SourceLocation ASTImporter::Import(Sourc
 return {};
 
   SourceManager  = FromContext.getSourceManager();
-  
-  // For now, map everything down to its file location, so that we
-  // don't have to import macro expansions.
-  // FIXME: Import macro expansions!
-  FromLoc = FromSM.getFileLoc(FromLoc);
+
   std::pair Decomposed = FromSM.getDecomposedLoc(FromLoc);
-  SourceManager  = ToContext.getSourceManager();
   FileID ToFileID = Import(Decomposed.first);
   if (ToFileID.isInvalid())
 return {};
-  SourceLocation ret = ToSM.getLocForStartOfFile(ToFileID)
-   .getLocWithOffset(Decomposed.second);
-  return ret;
+  SourceManager  = ToContext.getSourceManager();
+  return ToSM.getComposedLoc(ToFileID, Decomposed.second);
 }
 
 SourceRange ASTImporter::Import(SourceRange FromRange) {
@@ -7184,41 +7178,56 @@ SourceRange ASTImporter::Import(SourceRa
 }
 
 FileID ASTImporter::Import(FileID FromID) {
-  llvm::DenseMap::iterator Pos
-= ImportedFileIDs.find(FromID);
+  llvm::DenseMap::iterator Pos = ImportedFileIDs.find(FromID);
   if (Pos != ImportedFileIDs.end())
 return Pos->second;
-  
+
   SourceManager  = FromContext.getSourceManager();
   SourceManager  = ToContext.getSourceManager();
   const SrcMgr::SLocEntry  = FromSM.getSLocEntry(FromID);
-  assert(FromSLoc.isFile() && "Cannot handle macro expansions yet");
-  
-  // Include location of this file.
-  SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
-  
-  // Map the FileID for to the "to" source manager.
+
+  // Map the FromID to the "to" source manager.
   FileID ToID;
-  const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
-  if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
-// FIXME: We probably want to use getVirtualFile(), so we don't hit the
-// disk again
-// FIXME: We definitely want to re-use the existing MemoryBuffer, rather
-// than mmap the files several times.
-const FileEntry *Entry = 
ToFileManager.getFile(Cache->OrigEntry->getName());
-if (!Entry)
-  return {};
-ToID = ToSM.createFileID(Entry, ToIncludeLoc, 
- FromSLoc.getFile().getFileCharacteristic());
+  if (FromSLoc.isExpansion()) {
+const SrcMgr::ExpansionInfo  = FromSLoc.getExpansion();
+SourceLocation ToSpLoc = Import(FromEx.getSpellingLoc());
+SourceLocation ToExLocS = Import(FromEx.getExpansionLocStart());
+unsigned TokenLen = FromSM.getFileIDSize(FromID);
+SourceLocation MLoc;
+if (FromEx.isMacroArgExpansion()) {
+  MLoc = ToSM.createMacroArgExpansionLoc(ToSpLoc, ToExLocS, TokenLen);
+} else {
+  SourceLocation ToExLocE = Import(FromEx.getExpansionLocEnd());
+  MLoc = ToSM.createExpansionLoc(ToSpLoc, ToExLocS, ToExLocE, TokenLen,
+ FromEx.isExpansionTokenRange());
+}
+ToID = ToSM.getFileID(MLoc);
   } else {
-// FIXME: We want to re-use the existing MemoryBuffer!
-const llvm::MemoryBuffer *
-FromBuf = Cache->getBuffer(FromContext.getDiagnostics(), FromSM);
-std::unique_ptr ToBuf
-  = llvm::MemoryBuffer::getMemBufferCopy(FromBuf->getBuffer(),
- FromBuf->getBufferIdentifier());
-ToID = ToSM.createFileID(std::move(ToBuf),
- FromSLoc.getFile().getFileCharacteristic());
+// Include location of this file.
+SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
+
+const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
+if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
+  // FIXME: We probably want to use getVirtualFile(), so we don't hit the
+  // disk again
+  // FIXME: We definitely want to re-use the existing MemoryBuffer, rather
+  // than mmap the files several times.
+  const FileEntry *Entry =
+  ToFileManager.getFile(Cache->OrigEntry->getName());
+  if (!Entry)
+return {};
+  ToID = ToSM.createFileID(Entry, ToIncludeLoc,
+

[PATCH] D47698: [ASTImporter] import macro source locations

[Rafael Stahl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
r.stahl marked an inline comment as done.
Closed by commit rC336269: [ASTImporter] import macro source locations 
(authored by r.stahl, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47698?vs=152633=154103#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47698

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -7164,61 +7164,70 @@
 return {};
 
   SourceManager  = FromContext.getSourceManager();
-  
-  // For now, map everything down to its file location, so that we
-  // don't have to import macro expansions.
-  // FIXME: Import macro expansions!
-  FromLoc = FromSM.getFileLoc(FromLoc);
+
   std::pair Decomposed = FromSM.getDecomposedLoc(FromLoc);
-  SourceManager  = ToContext.getSourceManager();
   FileID ToFileID = Import(Decomposed.first);
   if (ToFileID.isInvalid())
 return {};
-  SourceLocation ret = ToSM.getLocForStartOfFile(ToFileID)
-   .getLocWithOffset(Decomposed.second);
-  return ret;
+  SourceManager  = ToContext.getSourceManager();
+  return ToSM.getComposedLoc(ToFileID, Decomposed.second);
 }
 
 SourceRange ASTImporter::Import(SourceRange FromRange) {
   return SourceRange(Import(FromRange.getBegin()), Import(FromRange.getEnd()));
 }
 
 FileID ASTImporter::Import(FileID FromID) {
-  llvm::DenseMap::iterator Pos
-= ImportedFileIDs.find(FromID);
+  llvm::DenseMap::iterator Pos = ImportedFileIDs.find(FromID);
   if (Pos != ImportedFileIDs.end())
 return Pos->second;
-  
+
   SourceManager  = FromContext.getSourceManager();
   SourceManager  = ToContext.getSourceManager();
   const SrcMgr::SLocEntry  = FromSM.getSLocEntry(FromID);
-  assert(FromSLoc.isFile() && "Cannot handle macro expansions yet");
-  
-  // Include location of this file.
-  SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
-  
-  // Map the FileID for to the "to" source manager.
+
+  // Map the FromID to the "to" source manager.
   FileID ToID;
-  const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
-  if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
-// FIXME: We probably want to use getVirtualFile(), so we don't hit the
-// disk again
-// FIXME: We definitely want to re-use the existing MemoryBuffer, rather
-// than mmap the files several times.
-const FileEntry *Entry = ToFileManager.getFile(Cache->OrigEntry->getName());
-if (!Entry)
-  return {};
-ToID = ToSM.createFileID(Entry, ToIncludeLoc, 
- FromSLoc.getFile().getFileCharacteristic());
+  if (FromSLoc.isExpansion()) {
+const SrcMgr::ExpansionInfo  = FromSLoc.getExpansion();
+SourceLocation ToSpLoc = Import(FromEx.getSpellingLoc());
+SourceLocation ToExLocS = Import(FromEx.getExpansionLocStart());
+unsigned TokenLen = FromSM.getFileIDSize(FromID);
+SourceLocation MLoc;
+if (FromEx.isMacroArgExpansion()) {
+  MLoc = ToSM.createMacroArgExpansionLoc(ToSpLoc, ToExLocS, TokenLen);
+} else {
+  SourceLocation ToExLocE = Import(FromEx.getExpansionLocEnd());
+  MLoc = ToSM.createExpansionLoc(ToSpLoc, ToExLocS, ToExLocE, TokenLen,
+ FromEx.isExpansionTokenRange());
+}
+ToID = ToSM.getFileID(MLoc);
   } else {
-// FIXME: We want to re-use the existing MemoryBuffer!
-const llvm::MemoryBuffer *
-FromBuf = Cache->getBuffer(FromContext.getDiagnostics(), FromSM);
-std::unique_ptr ToBuf
-  = llvm::MemoryBuffer::getMemBufferCopy(FromBuf->getBuffer(),
- FromBuf->getBufferIdentifier());
-ToID = ToSM.createFileID(std::move(ToBuf),
- FromSLoc.getFile().getFileCharacteristic());
+// Include location of this file.
+SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
+
+const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
+if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
+  // FIXME: We probably want to use getVirtualFile(), so we don't hit the
+  // disk again
+  // FIXME: We definitely want to re-use the existing MemoryBuffer, rather
+  // than mmap the files several times.
+  const FileEntry *Entry =
+  ToFileManager.getFile(Cache->OrigEntry->getName());
+  if (!Entry)
+return {};
+  ToID = ToSM.createFileID(Entry, ToIncludeLoc,
+   FromSLoc.getFile().getFileCharacteristic());
+} else {
+  // FIXME: We want to re-use the existing MemoryBuffer!
+  const llvm::MemoryBuffer *FromBuf =
+  Cache->getBuffer(FromContext.getDiagnostics(), FromSM);
+  std::unique_ptr ToBuf =
+  

[PATCH] D47946: [ASTmporter] Fix infinite recursion on function import with struct definition in parameters

[Zoltán Gera via Phabricator via cfe-commits]

gerazo added a comment.

@a.sidorin what do you think?


https://reviews.llvm.org/D47946



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

r336239 - [Sema] Consider all format_arg attributes.

[Michael Kruse via cfe-commits]

Author: meinersbur
Date: Tue Jul  3 18:37:11 2018
New Revision: 336239

URL: http://llvm.org/viewvc/llvm-project?rev=336239=rev
Log:
[Sema] Consider all format_arg attributes.

If a function has multiple format_arg attributes, clang only considers
the first it finds (because AttributeLists are in reverse order, not
necessarily the textually first) and ignores all others.

Loop over all FormatArgAttr to print warnings for all declared
format_arg attributes.

For instance, libintl's ngettext (select plural or singular version of
format string) has two __format_arg__ attributes.

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

Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/attr-format_arg.c

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=336239=336238=336239=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Jul  3 18:37:11 2018
@@ -5518,13 +5518,22 @@ checkFormatStringExpr(Sema , const Exp
   case Stmt::CXXMemberCallExprClass: {
 const CallExpr *CE = cast(E);
 if (const NamedDecl *ND = 
dyn_cast_or_null(CE->getCalleeDecl())) {
-  if (const FormatArgAttr *FA = ND->getAttr()) {
+  bool IsFirst = true;
+  StringLiteralCheckType CommonResult;
+  for (const auto *FA : ND->specific_attrs()) {
 const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
-return checkFormatStringExpr(S, Arg, Args,
- HasVAListArg, format_idx, firstDataArg,
- Type, CallType, InFunctionCall,
- CheckedVarArgs, UncoveredArg, Offset);
-  } else if (const FunctionDecl *FD = dyn_cast(ND)) {
+StringLiteralCheckType Result = checkFormatStringExpr(
+S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
+CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset);
+if (IsFirst) {
+  CommonResult = Result;
+  IsFirst = false;
+}
+  }
+  if (!IsFirst)
+return CommonResult;
+
+  if (const auto *FD = dyn_cast(ND)) {
 unsigned BuiltinID = FD->getBuiltinID();
 if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
 BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) {

Modified: cfe/trunk/test/Sema/attr-format_arg.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-format_arg.c?rev=336239=336238=336239=diff
==
--- cfe/trunk/test/Sema/attr-format_arg.c (original)
+++ cfe/trunk/test/Sema/attr-format_arg.c Tue Jul  3 18:37:11 2018
@@ -4,10 +4,27 @@ int printf(const char *, ...);
 
 const char* f(const char *s) __attribute__((format_arg(1)));
 
+const char *h(const char *msg1, const char *msg2)
+__attribute__((__format_arg__(1))) __attribute__((__format_arg__(2)));
+
 void g(const char *s) {
   printf("%d", 123);
   printf("%d %d", 123); // expected-warning{{more '%' conversions than data 
arguments}}
 
   printf(f("%d"), 123);
   printf(f("%d %d"), 123); // expected-warning{{more '%' conversions than data 
arguments}}
+
+  printf(h(
+"", // expected-warning {{format string is empty}}
+""  // expected-warning {{format string is empty}}
+  ), 123);
+  printf(h(
+"%d",
+""  // expected-warning {{format string is empty}}
+  ), 123);
+  printf(h(
+"", // expected-warning {{format string is empty}}
+"%d"
+  ), 123);
+  printf(h("%d", "%d"), 123);
 }


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

[PATCH] D48933: [clangd] Treat class constructor as in the same scope as the class in ranking.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48933

Files:
  clangd/Quality.cpp
  unittests/clangd/QualityTests.cpp


Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -21,6 +21,8 @@
 #include "Quality.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "clang/AST/DeclCXX.h"
+#include "llvm/Support/Casting.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -199,6 +201,39 @@
   EXPECT_LT(sortText(0, "a"), sortText(0, "z"));
 }
 
+TEST(QualityTests, NoBoostForClassConstructor) {
+  auto Header = TestTU::withHeaderCode(R"cpp(
+class Foo {
+public:
+  Foo(int);
+};
+  )cpp");
+  auto Symbols = Header.headerSymbols();
+  auto AST = Header.build();
+
+  const NamedDecl *Foo = (AST, "Foo");
+  SymbolRelevanceSignals Cls;
+  Cls.merge(CodeCompletionResult(Foo, /*Priority=*/0));
+
+  // `findDecl` would return the implicit injected class for "Foo::Foo"; simply
+  // look for a constructor decl under the class.
+  const DeclContext *FooCtx = llvm::dyn_cast(Foo);
+  assert(FooCtx);
+  const NamedDecl *CtorDecl = nullptr;
+  for (const auto *D : FooCtx->decls())
+if (const auto *ND = llvm::dyn_cast(D))
+  if (llvm::isa(D)) {
+CtorDecl = ND;
+break;
+  }
+  assert(CtorDecl);
+  SymbolRelevanceSignals Ctor;
+  Ctor.merge(CodeCompletionResult(CtorDecl, /*Priority=*/0));
+
+  EXPECT_EQ(Cls.Scope, SymbolRelevanceSignals::GlobalScope);
+  EXPECT_EQ(Ctor.Scope, SymbolRelevanceSignals::GlobalScope);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -11,10 +11,12 @@
 #include "URI.h"
 #include "index/Index.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclVisitor.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
@@ -195,6 +197,9 @@
   if (auto *R = dyn_cast_or_null(D))
 if (R->isInjectedClassName())
   DC = DC->getParent();
+  // Class constructor should have the same scope as the class.
+  if (const auto *Ctor = llvm::dyn_cast(D))
+DC = DC->getParent();
   bool InClass = false;
   for (; !DC->isFileContext(); DC = DC->getParent()) {
 if (DC->isFunctionOrMethod())


Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -21,6 +21,8 @@
 #include "Quality.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "clang/AST/DeclCXX.h"
+#include "llvm/Support/Casting.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -199,6 +201,39 @@
   EXPECT_LT(sortText(0, "a"), sortText(0, "z"));
 }
 
+TEST(QualityTests, NoBoostForClassConstructor) {
+  auto Header = TestTU::withHeaderCode(R"cpp(
+class Foo {
+public:
+  Foo(int);
+};
+  )cpp");
+  auto Symbols = Header.headerSymbols();
+  auto AST = Header.build();
+
+  const NamedDecl *Foo = (AST, "Foo");
+  SymbolRelevanceSignals Cls;
+  Cls.merge(CodeCompletionResult(Foo, /*Priority=*/0));
+
+  // `findDecl` would return the implicit injected class for "Foo::Foo"; simply
+  // look for a constructor decl under the class.
+  const DeclContext *FooCtx = llvm::dyn_cast(Foo);
+  assert(FooCtx);
+  const NamedDecl *CtorDecl = nullptr;
+  for (const auto *D : FooCtx->decls())
+if (const auto *ND = llvm::dyn_cast(D))
+  if (llvm::isa(D)) {
+CtorDecl = ND;
+break;
+  }
+  assert(CtorDecl);
+  SymbolRelevanceSignals Ctor;
+  Ctor.merge(CodeCompletionResult(CtorDecl, /*Priority=*/0));
+
+  EXPECT_EQ(Cls.Scope, SymbolRelevanceSignals::GlobalScope);
+  EXPECT_EQ(Ctor.Scope, SymbolRelevanceSignals::GlobalScope);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -11,10 +11,12 @@
 #include "URI.h"
 #include "index/Index.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclVisitor.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
@@ -195,6 +197,9 @@
   if (auto *R = dyn_cast_or_null(D))
 if 

[PATCH] D48314: [Frontend] Cache preamble-related data

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I agree that the optimization is compelling, we do also share preamble in 
clangd and run code completion in parallel with other actions that need an AST.
However, I would argue that a proper way to introduce the optimization would be 
to change interface of `ASTUnit` that would somehow allow reusing the PCHs 
between different `ASTUnit`s.

Having the cache is fine, but I suggest we:

1. Make it explicit in the interface, e.g. `ASTUnit` ctor will accept an extra 
`PreambleCache` parameter.
2. Have it off by default.

Whenever ASTUnit needs to rebuild a preamble, it can look into the cache and 
check there's a cached preamble and whether it matches the one that we're 
building.
After the rebuild is finished, it will try to push the built preamble into the 
cache.
ASTUnit can store the shared reference to a const preamble, i.e. 
`std::shared_ptr`. The cache itself will store the 
`weak_ptr`, thus allowing the preambles to be 
removed when they're not used anymore.

It would allow to:

1. avoid global state in ASTUnit, the cache will have to be created somewhere 
up the stack and passed to ASTUnit explicitly.
2. avoid complicating ASTUnit with synchronization code, it can be moved to 
preamble cache.
3. keep changes to the ASTUnit minimal. The only function that needs to be 
changed is the one building the preamble. Given how complicated ASTUnit is 
already, I think that's a big win.


https://reviews.llvm.org/D48314



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

[PATCH] D46190: For a used declaration, mark any associated usings as referenced.

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso added inline comments.



Comment at: lib/Sema/Sema.cpp:1884-1885
+  ND = NA->getAliasedNamespace();
+  if (auto *NNS = NA->getQualifier())
+MarkNamespaceAliasReferenced(NNS->getAsNamespaceAlias());
+}

rsmith wrote:
> The loop and recursion here -- and indeed this whole function -- seem 
> unnecessary.
> 
> ```
> namespace A {} // #1 
> namespace X {
>   namespace B = A; // #2
> }
> namespace Y = X; // #3
> namespace C = Y::B; // #4
> ```
> 
> Declaration `#4` should mark `#2` and `#3` referenced. So the only 
> 'unreferenced namespace alias' warning we should get here is for `#4` -- and 
> there is no need for marking `#4` as referenced to affect `#2` or `#3`.
The function and recursion is to be able to handle cases like:

```
namespace N {
  int var;
}

void Foo() {
  namespace NA = N;
  namespace NB = NA;
  NB::var = 4;
}
```
'var' is referenced and N, NA and NB should be marked as 'referenced' as well.




Comment at: lib/Sema/Sema.cpp:1890-1891
+
+/// \brief Mark as referenced any 'using declaration' that have introduced
+/// the given declaration in the current context.
+void Sema::MarkUsingReferenced(Decl *D, CXXScopeSpec *SS, Expr *E) {

rsmith wrote:
> Why is this ever appropriate? I would think we should just mark the named 
> declaration from the relevant lookup as referenced in all cases.
My intention is to detect cases where the using statements do not affect other 
declarations.

```
namespace N {
  int var;
}

void Foo() {
  using N::var;<- No Referenced
  N::var = 1;  <- Used
}
```



https://reviews.llvm.org/D46190



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

[PATCH] D46190: For a used declaration, mark any associated usings as referenced.

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso updated this revision to Diff 154091.
CarlosAlbertoEnciso added a comment.

Update the patch to address the comments from the reviewers.


https://reviews.llvm.org/D46190

Files:
  include/clang/Sema/Lookup.h
  include/clang/Sema/Sema.h
  include/clang/Sema/SemaInternal.h
  lib/Sema/SemaCXXScopeSpec.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaTemplate.cpp
  test/PCH/cxx-templates.cpp
  test/SemaCXX/referenced_alias_declaration_1.cpp
  test/SemaCXX/referenced_alias_declaration_2.cpp
  test/SemaCXX/referenced_using_all.cpp
  test/SemaCXX/referenced_using_declaration_1.cpp
  test/SemaCXX/referenced_using_declaration_2.cpp
  test/SemaCXX/referenced_using_directive.cpp

Index: test/SemaCXX/referenced_using_directive.cpp
===
--- test/SemaCXX/referenced_using_directive.cpp
+++ test/SemaCXX/referenced_using_directive.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
+
+namespace N {
+  typedef int Integer;
+  int var;
+}
+
+void Fa() {
+  using namespace N;  // Referenced
+  var = 1;
+}
+
+void Fb() {
+  using namespace N;
+  N::var = 1;
+}
+
+void Fc() {
+  using namespace N;  // Referenced
+  Integer var = 1;
+}
+
+void Fd() {
+  using namespace N;
+  N::Integer var = 1;
+}
+
+//CHECK:  |-FunctionDecl {{.*}} Fa 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: |   |-DeclStmt {{.*}}
+//CHECK-NEXT: |   | `-UsingDirectiveDecl {{.*}} referenced Namespace {{.*}} 'N'
+//CHECK-NEXT: |   `-BinaryOperator {{.*}} 'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'var' 'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: |-FunctionDecl {{.*}} Fb 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: |   |-DeclStmt {{.*}}
+//CHECK-NEXT: |   | `-UsingDirectiveDecl {{.*}} Namespace {{.*}} 'N'
+//CHECK-NEXT: |   `-BinaryOperator {{.*}} 'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'var' 'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: |-FunctionDecl {{.*}} Fc 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: |   |-DeclStmt {{.*}}
+//CHECK-NEXT: |   | `-UsingDirectiveDecl {{.*}} referenced Namespace {{.*}} 'N'
+//CHECK-NEXT: |   `-DeclStmt {{.*}}
+//CHECK-NEXT: | `-VarDecl {{.*}} var 'N::Integer':'int' cinit
+//CHECK-NEXT: |   `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: `-FunctionDecl {{.*}} Fd 'void ()'
+//CHECK-NEXT:   `-CompoundStmt {{.*}}
+//CHECK-NEXT: |-DeclStmt {{.*}}
+//CHECK-NEXT: | `-UsingDirectiveDecl {{.*}} Namespace {{.*}} 'N'
+//CHECK-NEXT: `-DeclStmt {{.*}}
+//CHECK-NEXT:   `-VarDecl {{.*}} var 'N::Integer':'int' cinit
+//CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
Index: test/SemaCXX/referenced_using_declaration_2.cpp
===
--- test/SemaCXX/referenced_using_declaration_2.cpp
+++ test/SemaCXX/referenced_using_declaration_2.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
+
+namespace N {
+  typedef int Integer;
+  typedef char Char;
+}
+
+using N::Integer;
+using N::Char;  // Referenced
+
+void Foo(int p1, N::Integer p2, Char p3) {
+  N::Integer var;
+  var = 0;
+}
+
+using N::Integer;   // Referenced
+Integer Bar() {
+  using N::Char;
+  return 0;
+}
+
+//CHECK:  |-UsingDecl {{.*}} N::Integer
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Integer' sugar
+//CHECK-NEXT: |   |-Typedef {{.*}} 'Integer'
+//CHECK-NEXT: |   `-BuiltinType {{.*}} 'int'
+//CHECK-NEXT: |-UsingDecl {{.*}} referenced N::Char
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit referenced Typedef {{.*}} 'Char'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Char' sugar
+//CHECK-NEXT: |   |-Typedef {{.*}} 'Char'
+//CHECK-NEXT: |   `-BuiltinType {{.*}} 'char'
+//CHECK-NEXT: |-FunctionDecl {{.*}} Foo 'void (int, N::Integer, N::Char)'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p1 'int'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p2 'N::Integer':'int'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p3 'N::Char':'char'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: |   |-DeclStmt {{.*}}
+//CHECK-NEXT: |   | `-VarDecl {{.*}} used var 'N::Integer':'int'
+//CHECK-NEXT: |   `-BinaryOperator {{.*}} 'N::Integer':'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'N::Integer':'int' lvalue Var {{.*}} 'var' 'N::Integer':'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 0
+//CHECK-NEXT: |-UsingDecl {{.*}} referenced N::Integer
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit referenced Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Integer' sugar
+//CHECK-NEXT: |   |-Typedef {{.*}} 'Integer'
+//CHECK-NEXT: |   `-BuiltinType {{.*}} 'int'

r336264 - NFC - Fix type in builtins-ppc-p9vector.c test

[Gabor Buella via cfe-commits]

Author: gbuella
Date: Wed Jul  4 04:29:21 2018
New Revision: 336264

URL: http://llvm.org/viewvc/llvm-project?rev=336264=rev
Log:
NFC - Fix type in builtins-ppc-p9vector.c test

Modified:
cfe/trunk/test/CodeGen/builtins-ppc-p9vector.c

Modified: cfe/trunk/test/CodeGen/builtins-ppc-p9vector.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins-ppc-p9vector.c?rev=336264=336263=336264=diff
==
--- cfe/trunk/test/CodeGen/builtins-ppc-p9vector.c (original)
+++ cfe/trunk/test/CodeGen/builtins-ppc-p9vector.c Wed Jul  4 04:29:21 2018
@@ -983,7 +983,7 @@ vector bool int test86(void) {
 }
 vector bool long long test87(void) {
 // CHECK-BE: @llvm.ppc.vsx.xvtstdcdp(<2 x double> {{.+}}, i32 127)
-// CHECK-BE_NEXT: ret <2 x i64
+// CHECK-BE-NEXT: ret <2 x i64>
 // CHECK: @llvm.ppc.vsx.xvtstdcdp(<2 x double> {{.+}}, i32 127)
 // CHECK-NEXT: ret <2 x i64>
   return vec_test_data_class(vda, __VEC_CLASS_FP_NOT_NORMAL);


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

r336263 - NFC - Fix typo in test/CodeGenObjC/gnustep2-class.m

[Gabor Buella via cfe-commits]

Author: gbuella
Date: Wed Jul  4 04:26:09 2018
New Revision: 336263

URL: http://llvm.org/viewvc/llvm-project?rev=336263=rev
Log:
NFC - Fix typo in test/CodeGenObjC/gnustep2-class.m

Modified:
cfe/trunk/test/CodeGenObjC/gnustep2-class.m

Modified: cfe/trunk/test/CodeGenObjC/gnustep2-class.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/gnustep2-class.m?rev=336263=336262=336263=diff
==
--- cfe/trunk/test/CodeGenObjC/gnustep2-class.m (original)
+++ cfe/trunk/test/CodeGenObjC/gnustep2-class.m Wed Jul  4 04:26:09 2018
@@ -51,5 +51,5 @@
 // And check that we get a pointer to it in the right place
 // CHECK: @._OBJC_REF_CLASS_X = global 
 // CHECK-SAME: @._OBJC_CLASS_X
-// CHECK-SAMEsection "__objc_class_refs"
+// CHECK-SAME: section "__objc_class_refs"
 


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

r336262 - NFC - Fix typo in test/Layout/itanium-pack-and-align.cpp

[Gabor Buella via cfe-commits]

Author: gbuella
Date: Wed Jul  4 04:21:44 2018
New Revision: 336262

URL: http://llvm.org/viewvc/llvm-project?rev=336262=rev
Log:
NFC - Fix typo in test/Layout/itanium-pack-and-align.cpp

Modified:
cfe/trunk/test/Layout/itanium-pack-and-align.cpp

Modified: cfe/trunk/test/Layout/itanium-pack-and-align.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/itanium-pack-and-align.cpp?rev=336262=336261=336262=diff
==
--- cfe/trunk/test/Layout/itanium-pack-and-align.cpp (original)
+++ cfe/trunk/test/Layout/itanium-pack-and-align.cpp Wed Jul  4 04:21:44 2018
@@ -23,4 +23,4 @@ T t;
 // CHECK-NEXT:  0 |   char x
 // CHECK-NEXT:  1 |   int y
 // CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
-// CHECK-NETX:|  nvsize=8, nvalign=8]
+// CHECK-NEXT:|  nvsize=8, nvalign=8]


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

[PATCH] D48314: [Frontend] Cache preamble-related data

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

@ilya-biryukov Sorry. I didn't have time to post comments here.
The usecase that we have is a supportive translation unit for code completion. 
Probably you use something similar in clangd not to wait for the TU to be 
reparsed after a change?
The gain from this change is both performance and memory but I don't have 
measurements under my hand right now. And of course they are only valid for 
this usecase.


https://reviews.llvm.org/D48314



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

r336261 - Add missing include for size_t

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Wed Jul  4 04:14:18 2018
New Revision: 336261

URL: http://llvm.org/viewvc/llvm-project?rev=336261=rev
Log:
Add missing include for size_t

Modified:
cfe/trunk/include/clang/Basic/Stack.h

Modified: cfe/trunk/include/clang/Basic/Stack.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Stack.h?rev=336261=336260=336261=diff
==
--- cfe/trunk/include/clang/Basic/Stack.h (original)
+++ cfe/trunk/include/clang/Basic/Stack.h Wed Jul  4 04:14:18 2018
@@ -15,6 +15,8 @@
 #ifndef LLVM_CLANG_BASIC_STACK_H
 #define LLVM_CLANG_BASIC_STACK_H
 
+#include 
+
 namespace clang {
   /// The amount of stack space that Clang would like to be provided with.
   /// If less than this much is available, we may be unable to reach our


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

[PATCH] D48441: [clangd] Incorporate transitive #includes into code complete proximity scoring.

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D48441#1151889, @ioeric wrote:

> Hi Carlos, thanks for letting us know! I sent r336249 to fix the windows
>  test.


Hi @ioeric,

Thanks very much. Happy to help.


Repository:
  rL LLVM

https://reviews.llvm.org/D48441



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

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

Are there any tests which actually exercise the new behavior?




Comment at: libcxx/include/memory:1665
+(is_same::type> >::value
+|| is_same >::value
+|| !__has_construct::value) &&

I'm not sure we should care about allocators for `T const`. The're all but an 
abomination. 



Comment at: 
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_different_value_type.pass.cpp:13
+// template  vector(InputIter first, InputIter last);
+
+// Initialize a vector with a different value type. Make sure initialization

Can this be folded into an existing test file for the constructor it's 
targeting?


https://reviews.llvm.org/D48342



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

[clang-tools-extra] r336260 - [clangd] only ignore collected symbols if TU has uncompilable errors.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Jul  4 03:39:48 2018
New Revision: 336260

URL: http://llvm.org/viewvc/llvm-project?rev=336260=rev
Log:
[clangd] only ignore collected symbols if TU has uncompilable errors.

Modified:

clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp

Modified: 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=336260=336259=336260=diff
==
--- 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
 (original)
+++ 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
 Wed Jul  4 03:39:48 2018
@@ -84,9 +84,10 @@ public:
 
 const auto  = getCompilerInstance();
 if (CI.hasDiagnostics() &&
-(CI.getDiagnosticClient().getNumErrors() > 0)) {
-  llvm::errs() << "Found errors in the translation unit. Igoring "
-  "collected symbols...\n";
+CI.getDiagnostics().hasUncompilableErrorOccurred()) {
+  llvm::errs()
+  << "Found uncompilable errors in the translation unit. Igoring "
+ "collected symbols...\n";
   return;
 }
 


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

[PATCH] D48928: [ms] Fix mangling of string literals used to initialize arrays larger or smaller than the literal

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

I couldn't get MSVC to create a symbol with padding at the end; they seem to 
always insert the padding with a separate loop during initialization, but this 
does show the truncated case: https://godbolt.org/g/B8ktA3


https://reviews.llvm.org/D48928



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

[PATCH] D48628: [AST] Structural equivalence of methods

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: lib/AST/ASTImporter.cpp:2454
+  if (IsStructuralMatch(D, FoundFunction)) {
+const FunctionDecl *Definition = nullptr;
+if (D->doesThisDeclarationHaveABody() &&

This change with `Definition` is needed to make the test 
ImportOfEquivalentMethod work. But this is a general problem with importing, I 
have a new test that fails and is not related to CXXMethodDecl. Add this new 
(not directly related) test or remove this change and disable the failing test?


Repository:
  rC Clang

https://reviews.llvm.org/D48628



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

[PATCH] D48928: [ms] Fix mangling of string literals used to initialize arrays larger or smaller than the literal

[Hans Wennborg via Phabricator via cfe-commits]

hans created this revision.
hans added reviewers: thakis, majnemer.

A Chromium developer reported a bug which turned out to be a mangling collision 
between these two literals:

  char s[] = "foo";
  char t[32] = "foo";

They may look the same, but for the initialization of t we will (under some 
circumstances) use a literal that's extended with zeros, and both the length 
and those zeros should be accounted for by the mangling.

This actually makes the mangling code simpler: where it previously had special 
logic for null terminators, which are not part of the StringLiteral, that is 
now covered by the general algorithm.


https://reviews.llvm.org/D48928

Files:
  lib/AST/MicrosoftMangle.cpp
  test/CodeGen/mangle-ms-string-literals.c

Index: test/CodeGen/mangle-ms-string-literals.c
===
--- /dev/null
+++ test/CodeGen/mangle-ms-string-literals.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c -emit-llvm %s -o - -triple=i386-pc-win32 | FileCheck %s
+// RUN: %clang_cc1 -x c -emit-llvm %s -o - -triple=x86_64-pc-win32 | FileCheck %s
+
+void crbug857442(int x) {
+  // Make sure to handle truncated or padded literals. The truncation is only valid in C.
+  struct {int x; char s[2]; } truncatedAscii = {x, "hello"};
+  // CHECK: "??_C@_01CONKJJHI@he@"
+  struct {int x; char s[16]; } paddedAscii = {x, "hello"};
+  // CHECK: "??_C@_0BA@EAAINDNC@hello?$AA?$AA?$AA?$AA?$AA?$AA?$AA?$AA?$AA?$AA?$AA@"
+}
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -3171,7 +3171,7 @@
   //  ::=   # the length of the literal
   //
   // ::= + @  # crc of the literal including
-  //  # null-terminator
+  //  # trailing null bytes
   //
   //  ::=# uninteresting character
   //  ::= '?$'   # these two nibbles
@@ -3186,44 +3186,50 @@
   MicrosoftCXXNameMangler Mangler(*this, Out);
   Mangler.getStream() << "??_C@_";
 
+  // The actual string length might be different from that of the string literal
+  // in cases like:
+  // char foo[3] = "foobar";
+  // char bar[42] = "foobar";
+  // Where it is truncated or zero-padded to fit the array. This is the length
+  // used for mangling, and any trailing null-bytes also need to be mangled.
+  unsigned StringLength = getASTContext()
+  .getAsConstantArrayType(SL->getType())
+  ->getSize()
+  .getZExtValue();
+
   // : The "kind" of string literal is encoded into the mangled name.
   if (SL->isWide())
 Mangler.getStream() << '1';
   else
 Mangler.getStream() << '0';
 
   // : The next part of the mangled name consists of the length
-  // of the string.
-  // The StringLiteral does not consider the NUL terminator byte(s) but the
-  // mangling does.
-  // N.B. The length is in terms of bytes, not characters.
-  Mangler.mangleNumber(SL->getByteLength() + SL->getCharByteWidth());
+  // of the string in bytes.
+  Mangler.mangleNumber(StringLength * SL->getCharByteWidth());
 
-  auto GetLittleEndianByte = [](unsigned Index) {
+  auto GetLittleEndianByte = [, StringLength](unsigned Index) {
 unsigned CharByteWidth = SL->getCharByteWidth();
+if (Index / CharByteWidth >= SL->getLength())
+  return static_cast(0);
 uint32_t CodeUnit = SL->getCodeUnit(Index / CharByteWidth);
 unsigned OffsetInCodeUnit = Index % CharByteWidth;
 return static_cast((CodeUnit >> (8 * OffsetInCodeUnit)) & 0xff);
   };
 
-  auto GetBigEndianByte = [](unsigned Index) {
+  auto GetBigEndianByte = [, StringLength](unsigned Index) {
 unsigned CharByteWidth = SL->getCharByteWidth();
+if (Index / CharByteWidth >= SL->getLength())
+  return static_cast(0);
 uint32_t CodeUnit = SL->getCodeUnit(Index / CharByteWidth);
 unsigned OffsetInCodeUnit = (CharByteWidth - 1) - (Index % CharByteWidth);
 return static_cast((CodeUnit >> (8 * OffsetInCodeUnit)) & 0xff);
   };
 
   // CRC all the bytes of the StringLiteral.
   llvm::JamCRC JC;
-  for (unsigned I = 0, E = SL->getByteLength(); I != E; ++I)
+  for (unsigned I = 0, E = StringLength * SL->getCharByteWidth(); I != E; ++I)
 JC.update(GetLittleEndianByte(I));
 
-  // The NUL terminator byte(s) were not present earlier,
-  // we need to manually process those bytes into the CRC.
-  for (unsigned NullTerminator = 0; NullTerminator < SL->getCharByteWidth();
-   ++NullTerminator)
-JC.update('\x00');
-
   // : The CRC is encoded utilizing the standard number mangling
   // scheme.
   Mangler.mangleNumber(JC.getCRC());
@@ -3260,18 +3266,14 @@
 
   // Enforce our 32 bytes max, except wchar_t which gets 32 chars instead.
   unsigned MaxBytesToMangle = SL->isWide() ? 64U : 32U;
-  unsigned NumBytesToMangle = std::min(MaxBytesToMangle, 

[PATCH] D47537: [clang-tools-extra] Cleanup documentation routine

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL336257: [clang-tools-extra] Cleanup documentation routine 
(authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47537?vs=150083=154086#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47537

Files:
  clang-tools-extra/trunk/docs/Doxyfile
  clang-tools-extra/trunk/docs/doxygen.cfg.in

Index: clang-tools-extra/trunk/docs/doxygen.cfg.in
===
--- clang-tools-extra/trunk/docs/doxygen.cfg.in
+++ clang-tools-extra/trunk/docs/doxygen.cfg.in
@@ -743,15 +743,20 @@
 # spaces.
 # Note: If this tag is empty the current directory is searched.
 
-INPUT  = \
- @abs_srcdir@/../clang-tidy \
- @abs_srcdir@/../clang-apply-replacements \
- @abs_srcdir@/../clang-query \
- @abs_srcdir@/../clang-rename \
- @abs_srcdir@/../modularize \
- @abs_srcdir@/../pp-trace \
- @abs_srcdir@/../tool-template \
- @abs_srcdir@/doxygen-mainpage.dox
+INPUT  =  \
+  @abs_srcdir@/../change-namespace \
+  @abs_srcdir@/../clang-apply-replacements \
+  @abs_srcdir@/../clang-doc \
+  @abs_srcdir@/../clang-move \
+  @abs_srcdir@/../clang-query \
+  @abs_srcdir@/../clang-reorder-fields \
+  @abs_srcdir@/../clang-tidy \
+  @abs_srcdir@/../clangd \
+  @abs_srcdir@/../include-fixer \
+  @abs_srcdir@/../modularize \
+  @abs_srcdir@/../pp-trace \
+  @abs_srcdir@/../tool-template \
+  @abs_srcdir@/doxygen-mainpage.dox
 
 # This tag can be used to specify the character encoding of the source files
 # that doxygen parses. Internally doxygen uses the UTF-8 encoding. Doxygen uses
Index: clang-tools-extra/trunk/docs/Doxyfile
===
--- clang-tools-extra/trunk/docs/Doxyfile
+++ clang-tools-extra/trunk/docs/Doxyfile
@@ -1,1808 +0,0 @@
-# Doxyfile 1.7.6.1
-
-# This file describes the settings to be used by the documentation system
-# doxygen (www.doxygen.org) for a project
-#
-# All text after a hash (#) is considered a comment and will be ignored
-# The format is:
-#   TAG = value [value, ...]
-# For lists items can also be appended using:
-#   TAG += value [value, ...]
-# Values that contain spaces should be placed between quotes (" ")
-
-#---
-# Project related configuration options
-#---
-
-# This tag specifies the encoding used for all characters in the config file
-# that follow. The default is UTF-8 which is also the encoding used for all
-# text before the first occurrence of this tag. Doxygen uses libiconv (or the
-# iconv built into libc) for the transcoding. See
-# http://www.gnu.org/software/libiconv for the list of possible encodings.
-
-DOXYFILE_ENCODING  = UTF-8
-
-# The PROJECT_NAME tag is a single word (or sequence of words) that should
-# identify the project. Note that if you do not use Doxywizard you need
-# to put quotes around the project name if it contains spaces.
-
-PROJECT_NAME   = clang-tools-extra
-
-# The PROJECT_NUMBER tag can be used to enter a project or revision number.
-# This could be handy for archiving the generated documentation or
-# if some version control system is used.
-
-PROJECT_NUMBER =
-
-# Using the PROJECT_BRIEF tag one can provide an optional one line description
-# for a project that appears at the top of each page and should give viewer
-# a quick idea about the purpose of the project. Keep the description short.
-
-PROJECT_BRIEF  =
-
-# With the PROJECT_LOGO tag one can specify an logo or icon that is
-# included in the documentation. The maximum height of the logo should not
-# exceed 55 pixels and the maximum width should not exceed 200 pixels.
-# Doxygen will copy the logo to the output directory.
-
-PROJECT_LOGO   =
-
-# The OUTPUT_DIRECTORY tag is used to specify the (relative or absolute)
-# base path where the generated documentation will be put.
-# If a relative path is entered, it will be relative to the location
-# where doxygen was started. If left blank the current directory will be used.
-
-# Same directory that Sphinx uses.
-OUTPUT_DIRECTORY   = ./_build/
-
-# If the CREATE_SUBDIRS tag is set to YES, then doxygen will create
-# 4096 sub-directories (in 2 levels) under the output directory of 

[PATCH] D48314: [Frontend] Cache preamble-related data

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I would argue this should be handled by the clients instead. Adding global 
state and locking is complicated. (And ASTUnit is complicated enough).

What are the use-cases of creating multiple `ASTUnit` inside the same process 
for the same file? Which clients do that and why they can't have a single 
ASTUnit per file?


https://reviews.llvm.org/D48314



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

[PATCH] D48721: Patch to fix pragma metadata for do-while loops

[Deepak Panickal via Phabricator via cfe-commits]

deepak2427 added a comment.

I encountered the issue while working with the unroller and found that it was 
not following the pragma info, and traced it back to the issue with metadata.
As far as I understood, for for-loops and while-loops, we add the metadata only 
to the loop back-edge. So it would make sense to keep them consistent.
I'm not an expert in clang, and do not know how we can detect such problems.


Repository:
  rC Clang

https://reviews.llvm.org/D48721



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

[clang-tools-extra] r336257 - [clang-tools-extra] Cleanup documentation routine

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Wed Jul  4 03:18:03 2018
New Revision: 336257

URL: http://llvm.org/viewvc/llvm-project?rev=336257=rev
Log:
[clang-tools-extra] Cleanup documentation routine

The following issues are resolved:

* Doxygen didn't generate documentation for a bunch of existing tools
due to the absence of their directories in the doxygen configuration
file. This patch adds all relevant directories to the appropriate list.

* clang-tools-extra/docs/Doxyfile seems to be unused and irrelevant,
doxygen.cfg.in is passed to the CMake's Doxygen invocation, hence
Doxyfile is removed.

The validity of proposed changes was manually checked by building
doxygen-clang-tools and making sure that clangd and other tools are
present in Doxygen-generated docs of clang-tools-extra.

Reviewers: ioeric

Subscribers: cfe-commits

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

Removed:
clang-tools-extra/trunk/docs/Doxyfile
Modified:
clang-tools-extra/trunk/docs/doxygen.cfg.in

Removed: clang-tools-extra/trunk/docs/Doxyfile
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/Doxyfile?rev=336256=auto
==
--- clang-tools-extra/trunk/docs/Doxyfile (original)
+++ clang-tools-extra/trunk/docs/Doxyfile (removed)
@@ -1,1808 +0,0 @@
-# Doxyfile 1.7.6.1
-
-# This file describes the settings to be used by the documentation system
-# doxygen (www.doxygen.org) for a project
-#
-# All text after a hash (#) is considered a comment and will be ignored
-# The format is:
-#   TAG = value [value, ...]
-# For lists items can also be appended using:
-#   TAG += value [value, ...]
-# Values that contain spaces should be placed between quotes (" ")
-
-#---
-# Project related configuration options
-#---
-
-# This tag specifies the encoding used for all characters in the config file
-# that follow. The default is UTF-8 which is also the encoding used for all
-# text before the first occurrence of this tag. Doxygen uses libiconv (or the
-# iconv built into libc) for the transcoding. See
-# http://www.gnu.org/software/libiconv for the list of possible encodings.
-
-DOXYFILE_ENCODING  = UTF-8
-
-# The PROJECT_NAME tag is a single word (or sequence of words) that should
-# identify the project. Note that if you do not use Doxywizard you need
-# to put quotes around the project name if it contains spaces.
-
-PROJECT_NAME   = clang-tools-extra
-
-# The PROJECT_NUMBER tag can be used to enter a project or revision number.
-# This could be handy for archiving the generated documentation or
-# if some version control system is used.
-
-PROJECT_NUMBER =
-
-# Using the PROJECT_BRIEF tag one can provide an optional one line description
-# for a project that appears at the top of each page and should give viewer
-# a quick idea about the purpose of the project. Keep the description short.
-
-PROJECT_BRIEF  =
-
-# With the PROJECT_LOGO tag one can specify an logo or icon that is
-# included in the documentation. The maximum height of the logo should not
-# exceed 55 pixels and the maximum width should not exceed 200 pixels.
-# Doxygen will copy the logo to the output directory.
-
-PROJECT_LOGO   =
-
-# The OUTPUT_DIRECTORY tag is used to specify the (relative or absolute)
-# base path where the generated documentation will be put.
-# If a relative path is entered, it will be relative to the location
-# where doxygen was started. If left blank the current directory will be used.
-
-# Same directory that Sphinx uses.
-OUTPUT_DIRECTORY   = ./_build/
-
-# If the CREATE_SUBDIRS tag is set to YES, then doxygen will create
-# 4096 sub-directories (in 2 levels) under the output directory of each output
-# format and will distribute the generated files over these directories.
-# Enabling this option can be useful when feeding doxygen a huge amount of
-# source files, where putting all generated files in the same directory would
-# otherwise cause performance problems for the file system.
-
-CREATE_SUBDIRS = NO
-
-# The OUTPUT_LANGUAGE tag is used to specify the language in which all
-# documentation generated by doxygen is written. Doxygen will use this
-# information to generate all constant output in the proper language.
-# The default language is English, other supported languages are:
-# Afrikaans, Arabic, Brazilian, Catalan, Chinese, Chinese-Traditional,
-# Croatian, Czech, Danish, Dutch, Esperanto, Farsi, Finnish, French, German,
-# Greek, Hungarian, Italian, Japanese, Japanese-en (Japanese with English
-# messages), Korean, Korean-en, Lithuanian, Norwegian, Macedonian, Persian,
-# Polish, Portuguese, Romanian, Russian, Serbian, Serbian-Cyrillic, Slovak,
-# Slovene, Spanish, Swedish, Ukrainian, and Vietnamese.
-
-OUTPUT_LANGUAGE= English
-
-# If the 

[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC336255: [SemaCodeComplete] Make sure visited contexts are 
passed to completion results… (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48917?vs=154077=154080#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48917

Files:
  lib/Sema/SemaCodeComplete.cpp
  unittests/Sema/CodeCompleteTest.cpp

Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3700,9 +3700,11 @@
 /// type we're looking for.
 void Sema::CodeCompleteExpression(Scope *S, 
   const CodeCompleteExpressionData ) {
-  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
-CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_Expression);
+  ResultBuilder Results(
+  *this, CodeCompleter->getAllocator(),
+  CodeCompleter->getCodeCompletionTUInfo(),
+  CodeCompletionContext(CodeCompletionContext::CCC_Expression,
+Data.PreferredType));
   if (Data.ObjCCollection)
 Results.setFilter(::IsObjCCollection);
   else if (Data.IntegralConstantExpression)
@@ -3741,10 +3743,8 @@
 
   if (CodeCompleter->includeMacros())
 AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext(CodeCompletionContext::CCC_Expression, 
-  Data.PreferredType),
-Results.data(),Results.size());
+  HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 void Sema::CodeCompletePostfixExpression(Scope *S, ExprResult E) {
@@ -4360,17 +4360,11 @@
   }
   Results.ExitScope();
 
-  //We need to make sure we're setting the right context, 
-  //so only say we include macros if the code completer says we do
-  enum CodeCompletionContext::Kind kind = CodeCompletionContext::CCC_Other;
   if (CodeCompleter->includeMacros()) {
 AddMacroResults(PP, Results, false);
-kind = CodeCompletionContext::CCC_OtherWithMacros;
   }
-  
-  HandleCodeCompleteResults(this, CodeCompleter, 
-kind,
-Results.data(),Results.size());
+  HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 static bool anyNullArguments(ArrayRef Args) {
@@ -4773,10 +4767,9 @@
  CodeCompleter->includeGlobals(),
  CodeCompleter->loadExternal());
   Results.ExitScope();
-  
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext::CCC_PotentiallyQualifiedName,
-Results.data(),Results.size());
+
+  HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 void Sema::CodeCompleteUsingDirective(Scope *S) {
@@ -4795,9 +4788,8 @@
  CodeCompleter->includeGlobals(),
  CodeCompleter->loadExternal());
   Results.ExitScope();
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext::CCC_Namespace,
-Results.data(),Results.size());
+  HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 void Sema::CodeCompleteNamespaceDecl(Scope *S)  {
@@ -4893,10 +4885,9 @@
   // Add any type specifiers
   AddTypeSpecifierResults(getLangOpts(), Results);
   Results.ExitScope();
-  
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext::CCC_Type,
-Results.data(),Results.size());
+
+  HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 void Sema::CodeCompleteConstructorInitializer(
@@ -5177,9 +5168,8 @@
   else
 AddObjCTopLevelResults(Results, false);
   Results.ExitScope();
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext::CCC_Other,
-Results.data(),Results.size());
+  HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 static void AddObjCExpressionResults(ResultBuilder , bool NeedAt) {
@@ -5311,9 +5301,8 @@
   Results.EnterNewScope();
   AddObjCVisibilityResults(getLangOpts(), Results, false);
   Results.ExitScope();
-  HandleCodeCompleteResults(this, CodeCompleter, 
-

r336255 - [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Jul  4 03:01:18 2018
New Revision: 336255

URL: http://llvm.org/viewvc/llvm-project?rev=336255=rev
Log:
[SemaCodeComplete] Make sure visited contexts are passed to completion results 
handler.

Reviewers: ilya-biryukov

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/unittests/Sema/CodeCompleteTest.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=336255=336254=336255=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Wed Jul  4 03:01:18 2018
@@ -3700,9 +3700,11 @@ struct Sema::CodeCompleteExpressionData
 /// type we're looking for.
 void Sema::CodeCompleteExpression(Scope *S, 
   const CodeCompleteExpressionData ) {
-  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
-CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_Expression);
+  ResultBuilder Results(
+  *this, CodeCompleter->getAllocator(),
+  CodeCompleter->getCodeCompletionTUInfo(),
+  CodeCompletionContext(CodeCompletionContext::CCC_Expression,
+Data.PreferredType));
   if (Data.ObjCCollection)
 Results.setFilter(::IsObjCCollection);
   else if (Data.IntegralConstantExpression)
@@ -3741,10 +3743,8 @@ void Sema::CodeCompleteExpression(Scope
 
   if (CodeCompleter->includeMacros())
 AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext(CodeCompletionContext::CCC_Expression, 
-  Data.PreferredType),
-Results.data(),Results.size());
+  HandleCodeCompleteResults(this, CodeCompleter, 
Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 void Sema::CodeCompletePostfixExpression(Scope *S, ExprResult E) {
@@ -4360,17 +4360,11 @@ void Sema::CodeCompleteCase(Scope *S) {
   }
   Results.ExitScope();
 
-  //We need to make sure we're setting the right context, 
-  //so only say we include macros if the code completer says we do
-  enum CodeCompletionContext::Kind kind = CodeCompletionContext::CCC_Other;
   if (CodeCompleter->includeMacros()) {
 AddMacroResults(PP, Results, false);
-kind = CodeCompletionContext::CCC_OtherWithMacros;
   }
-  
-  HandleCodeCompleteResults(this, CodeCompleter, 
-kind,
-Results.data(),Results.size());
+  HandleCodeCompleteResults(this, CodeCompleter, 
Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 static bool anyNullArguments(ArrayRef Args) {
@@ -4773,10 +4767,9 @@ void Sema::CodeCompleteUsing(Scope *S) {
  CodeCompleter->includeGlobals(),
  CodeCompleter->loadExternal());
   Results.ExitScope();
-  
-  HandleCodeCompleteResults(this, CodeCompleter, 
-
CodeCompletionContext::CCC_PotentiallyQualifiedName,
-Results.data(),Results.size());
+
+  HandleCodeCompleteResults(this, CodeCompleter, 
Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 void Sema::CodeCompleteUsingDirective(Scope *S) {
@@ -4795,9 +4788,8 @@ void Sema::CodeCompleteUsingDirective(Sc
  CodeCompleter->includeGlobals(),
  CodeCompleter->loadExternal());
   Results.ExitScope();
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext::CCC_Namespace,
-Results.data(),Results.size());
+  HandleCodeCompleteResults(this, CodeCompleter, 
Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 void Sema::CodeCompleteNamespaceDecl(Scope *S)  {
@@ -4893,10 +4885,9 @@ void Sema::CodeCompleteOperatorName(Scop
   // Add any type specifiers
   AddTypeSpecifierResults(getLangOpts(), Results);
   Results.ExitScope();
-  
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext::CCC_Type,
-Results.data(),Results.size());
+
+  HandleCodeCompleteResults(this, CodeCompleter, 
Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 void Sema::CodeCompleteConstructorInitializer(
@@ -5177,9 +5168,8 @@ void Sema::CodeCompleteObjCAtDirective(S
   else
 AddObjCTopLevelResults(Results, false);
   Results.ExitScope();
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext::CCC_Other,
-

[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.

[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


Repository:
  rC Clang

https://reviews.llvm.org/D48917



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

[clang-tools-extra] r336253 - [clangd] Cleanup unittest: URIs. NFC.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Jul  4 02:54:23 2018
New Revision: 336253

URL: http://llvm.org/viewvc/llvm-project?rev=336253=rev
Log:
[clangd] Cleanup unittest: URIs. NFC.

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

Modified: clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp?rev=336253=336252=336253=diff
==
--- clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp Wed Jul  4 
02:54:23 2018
@@ -76,8 +76,8 @@ TEST(FileDistanceTests, URI) {
 #else
   EXPECT_EQ(D.distance("file:///not/a/testpath/either"), 3u);
 #endif
-  EXPECT_EQ(D.distance("unittest:foo"), 1000u);
-  EXPECT_EQ(D.distance("unittest:bar"), 1008u);
+  EXPECT_EQ(D.distance("unittest:///foo"), 1000u);
+  EXPECT_EQ(D.distance("unittest:///bar"), 1008u);
 }
 
 TEST(FileDistance, LimitUpTraversals) {


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

[PATCH] D48881: [clangd] Avoid collecting symbols from broken TUs in global-symbol-builder.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:88
+(CI.getDiagnosticClient().getNumErrors() > 0)) {
+  llvm::errs() << "Found errors in the translation unit. Igoring "
+  "collected symbols...\n";

ioeric wrote:
> ilya-biryukov wrote:
> > NIT: Maybe use clangd's `log` here? To get timestamps in the output, etc.
> The default logger doesn't seem to add timestamp? I'll keep `llvm::errs()` 
> here for consistency in this file.
> The default logger doesn't seem to add timestamp?
Ah, yes, it's the `JSONOutput` that adds timestamps Sorry.

> I'll keep llvm::errs() here for consistency in this file.
We write to `stderr` at the bottom of the file to show errors to the user. 
This instance looks much more like a logging routine.

Since our dependencies (e.g. `SymbolCollector`) use logs, I'd argue it's 
actually **more** consistent across the program to use `log` here as well. E.g. 
if someone adds a `LoggingSession` that writes timestamps, not only 
SymbolCollector logs should start showing those, but this one as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D48881



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

[PATCH] D48881: [clangd] Avoid collecting symbols from broken TUs in global-symbol-builder.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL336252: [clangd] Avoid collecting symbols from broken TUs in 
global-symbol-builder. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D48881

Files:
  
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp


Index: 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
===
--- 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
+++ 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
@@ -82,6 +82,14 @@
   void EndSourceFileAction() override {
 WrapperFrontendAction::EndSourceFileAction();
 
+const auto  = getCompilerInstance();
+if (CI.hasDiagnostics() &&
+(CI.getDiagnosticClient().getNumErrors() > 0)) {
+  llvm::errs() << "Found errors in the translation unit. Igoring "
+  "collected symbols...\n";
+  return;
+}
+
 auto Symbols = Collector->takeSymbols();
 for (const auto  : Symbols) {
   Ctx->reportResult(Sym.ID.str(), SymbolToYAML(Sym));


Index: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
===
--- clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
+++ clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
@@ -82,6 +82,14 @@
   void EndSourceFileAction() override {
 WrapperFrontendAction::EndSourceFileAction();
 
+const auto  = getCompilerInstance();
+if (CI.hasDiagnostics() &&
+(CI.getDiagnosticClient().getNumErrors() > 0)) {
+  llvm::errs() << "Found errors in the translation unit. Igoring "
+  "collected symbols...\n";
+  return;
+}
+
 auto Symbols = Collector->takeSymbols();
 for (const auto  : Symbols) {
   Ctx->reportResult(Sym.ID.str(), SymbolToYAML(Sym));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r336252 - [clangd] Avoid collecting symbols from broken TUs in global-symbol-builder.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Jul  4 02:43:35 2018
New Revision: 336252

URL: http://llvm.org/viewvc/llvm-project?rev=336252=rev
Log:
[clangd] Avoid collecting symbols from broken TUs in global-symbol-builder.

Summary:
For example, template parameter might not be resolved in a broken TU,
which can result in wrong USR/SymbolID.

Reviewers: ilya-biryukov

Subscribers: MaskRay, jkorous, cfe-commits

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

Modified:

clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp

Modified: 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=336252=336251=336252=diff
==
--- 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
 (original)
+++ 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
 Wed Jul  4 02:43:35 2018
@@ -82,6 +82,14 @@ public:
   void EndSourceFileAction() override {
 WrapperFrontendAction::EndSourceFileAction();
 
+const auto  = getCompilerInstance();
+if (CI.hasDiagnostics() &&
+(CI.getDiagnosticClient().getNumErrors() > 0)) {
+  llvm::errs() << "Found errors in the translation unit. Igoring "
+  "collected symbols...\n";
+  return;
+}
+
 auto Symbols = Collector->takeSymbols();
 for (const auto  : Symbols) {
   Ctx->reportResult(Sym.ID.str(), SymbolToYAML(Sym));


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

[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:3744
 AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext(CodeCompletionContext::CCC_Expression, 

ilya-biryukov wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > ilya-biryukov wrote:
> > > > `ResultsBuilder`'s constructor accepts a `CodeCompletionContext`. Can 
> > > > we pass in the context with `PreferedType` there instead of 
> > > > reconstructing it later?
> > > > To make sure we don't miss other things (incl. any future additions) 
> > > > that `ResultsBuilder` puts into the context.
> > > The `PreferedType` should actually already be set in the `ResultsBuilder` 
> > > (line 3715). In thoery, `Results.getCompletionContext()` should work fine 
> > > here as well, but it would break some Index tests - it introduced some 
> > > inconsistency in sema scoring when running with and without result 
> > > caching 
> > > (https://github.com/llvm-mirror/clang/blob/master/test/Index/complete-exprs.c#L35).
> > >  This is probably a bug, but I'm not sure if I'm the right person to 
> > > chase it :( 
> > What kind of inconsistencies? Maybe we should just update the CHECKS in the 
> > test?
> After chatting offline: it seems passing in the context that has the 
> preferred type set into `ResultsBuilder` saves us from check failures.
> The fact that `ResultsBuilder` also has `setPreferrredType`, which can be 
> different from the one in the `CodeCompletionContext` is still confusing, but 
> that's unrelated to the problem at hand.
Sounds good. Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D48917



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

[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 154077.
ioeric added a comment.

- Addressed review comment.


Repository:
  rC Clang

https://reviews.llvm.org/D48917

Files:
  lib/Sema/SemaCodeComplete.cpp
  unittests/Sema/CodeCompleteTest.cpp

Index: unittests/Sema/CodeCompleteTest.cpp
===
--- unittests/Sema/CodeCompleteTest.cpp
+++ unittests/Sema/CodeCompleteTest.cpp
@@ -131,4 +131,15 @@
   EXPECT_TRUE(VisitedNS.empty());
 }
 
+TEST(SemaCodeCompleteTest, VisitedNSWithoutQualifier) {
+  auto VisitedNS = runCodeCompleteOnCode(R"cpp(
+namespace n1 {
+namespace n2 {
+  void f(^) {}
+}
+}
+  )cpp");
+  EXPECT_THAT(VisitedNS, UnorderedElementsAre("n1", "n1::n2"));
+}
+
 } // namespace
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3700,9 +3700,11 @@
 /// type we're looking for.
 void Sema::CodeCompleteExpression(Scope *S, 
   const CodeCompleteExpressionData ) {
-  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
-CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_Expression);
+  ResultBuilder Results(
+  *this, CodeCompleter->getAllocator(),
+  CodeCompleter->getCodeCompletionTUInfo(),
+  CodeCompletionContext(CodeCompletionContext::CCC_Expression,
+Data.PreferredType));
   if (Data.ObjCCollection)
 Results.setFilter(::IsObjCCollection);
   else if (Data.IntegralConstantExpression)
@@ -3741,10 +3743,8 @@
 
   if (CodeCompleter->includeMacros())
 AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext(CodeCompletionContext::CCC_Expression, 
-  Data.PreferredType),
-Results.data(),Results.size());
+  HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 void Sema::CodeCompletePostfixExpression(Scope *S, ExprResult E) {
@@ -4360,17 +4360,11 @@
   }
   Results.ExitScope();
 
-  //We need to make sure we're setting the right context, 
-  //so only say we include macros if the code completer says we do
-  enum CodeCompletionContext::Kind kind = CodeCompletionContext::CCC_Other;
   if (CodeCompleter->includeMacros()) {
 AddMacroResults(PP, Results, false);
-kind = CodeCompletionContext::CCC_OtherWithMacros;
   }
-  
-  HandleCodeCompleteResults(this, CodeCompleter, 
-kind,
-Results.data(),Results.size());
+  HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 static bool anyNullArguments(ArrayRef Args) {
@@ -4773,10 +4767,9 @@
  CodeCompleter->includeGlobals(),
  CodeCompleter->loadExternal());
   Results.ExitScope();
-  
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext::CCC_PotentiallyQualifiedName,
-Results.data(),Results.size());
+
+  HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 void Sema::CodeCompleteUsingDirective(Scope *S) {
@@ -4795,9 +4788,8 @@
  CodeCompleter->includeGlobals(),
  CodeCompleter->loadExternal());
   Results.ExitScope();
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext::CCC_Namespace,
-Results.data(),Results.size());
+  HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 void Sema::CodeCompleteNamespaceDecl(Scope *S)  {
@@ -4893,10 +4885,9 @@
   // Add any type specifiers
   AddTypeSpecifierResults(getLangOpts(), Results);
   Results.ExitScope();
-  
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext::CCC_Type,
-Results.data(),Results.size());
+
+  HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 void Sema::CodeCompleteConstructorInitializer(
@@ -5177,9 +5168,8 @@
   else
 AddObjCTopLevelResults(Results, false);
   Results.ExitScope();
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext::CCC_Other,
-Results.data(),Results.size());
+  HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+

[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:3744
 AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext(CodeCompletionContext::CCC_Expression, 

ilya-biryukov wrote:
> ioeric wrote:
> > ilya-biryukov wrote:
> > > `ResultsBuilder`'s constructor accepts a `CodeCompletionContext`. Can we 
> > > pass in the context with `PreferedType` there instead of reconstructing 
> > > it later?
> > > To make sure we don't miss other things (incl. any future additions) that 
> > > `ResultsBuilder` puts into the context.
> > The `PreferedType` should actually already be set in the `ResultsBuilder` 
> > (line 3715). In thoery, `Results.getCompletionContext()` should work fine 
> > here as well, but it would break some Index tests - it introduced some 
> > inconsistency in sema scoring when running with and without result caching 
> > (https://github.com/llvm-mirror/clang/blob/master/test/Index/complete-exprs.c#L35).
> >  This is probably a bug, but I'm not sure if I'm the right person to chase 
> > it :( 
> What kind of inconsistencies? Maybe we should just update the CHECKS in the 
> test?
After chatting offline: it seems passing in the context that has the preferred 
type set into `ResultsBuilder` saves us from check failures.
The fact that `ResultsBuilder` also has `setPreferrredType`, which can be 
different from the one in the `CodeCompletionContext` is still confusing, but 
that's unrelated to the problem at hand.


Repository:
  rC Clang

https://reviews.llvm.org/D48917



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

[PATCH] D46190: For a referenced declaration, mark any associated usings as referenced.

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D46190#1135295, @probinson wrote:

> Style comments.
>  The two new Sema methods (for namespaces and using references) are C++ 
> specific, so SemaDeclCXX.cpp would seem like a more appropriate home for them.


Both functions have been moved to SemaDeclCXX.cpp.


https://reviews.llvm.org/D46190



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

[PATCH] D46190: For a referenced declaration, mark any associated usings as referenced.

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D46190#1135688, @rsmith wrote:

> The right way to handle this is to pass both the ultimately-selected 
> declaration and the declaration found by name lookup into the calls to 
> `MarkAnyDeclReferenced` and friends. We should mark the selected declaration 
> as `Used` (when appropriate), and mark the found declaration as `Referenced`.
>
> We should not be trying to reconstruct which using declarations are in scope 
> after the fact. Only declarations actually found by name lookup and then 
> selected by the context performing the lookup should be marked referenced. 
> (There may be cases where name lookup discards equivalent lookup results that 
> are not redeclarations of one another, though, and to handle such cases 
> properly, you may need to teach name lookup to track a list of found 
> declarations rather than only one.)


Following your comments, I have replaced the scope traversal with `LookupName` 
in order to find the declarations.

But there are 2 specific cases:

- using-directives

The `LookupName` function does not record the using-directives. With this 
patch, there is an extra option to store those directives in the lookup 
results, to be processed during the setting of the 'Referenced' bit.

- namespace-alias

I am aware of your comment on the function that recursively traverse the 
namespace alias, but I can't see another way to do it. If you have a more 
specific idea, I am happy to explore it.

For both cases, may be `LookupName` function can have that functionality and 
store in the results any namespace-directives and namespace-alias associated 
with the given declaration.




Comment at: lib/Sema/Sema.cpp:1879
+void Sema::MarkNamespaceAliasReferenced(NamedDecl *ND) {
+  if (ND && !ND->isReferenced()) {
+NamespaceAliasDecl *NA = nullptr;

probinson wrote:
> You could do this as an early return and reduce indentation in the rest of 
> the method.
> ```
> if (!ND || ND->isReferenced())
>   return;
> ```
> 
Corrected to

```
if (!ND || ND->isReferenced())
  return;
```




Comment at: lib/Sema/Sema.cpp:1880
+  if (ND && !ND->isReferenced()) {
+NamespaceAliasDecl *NA = nullptr;
+while ((NA = dyn_cast(ND)) && !NA->isReferenced()) {

probinson wrote:
> Initializing this to nullptr is redundant, as you set NA in the while-loop 
> expression.
Removed the `nullptr` initialization.



Comment at: lib/Sema/Sema.cpp:1891
+/// \brief Mark as referenced any 'using declaration' that have introduced
+/// the given declaration in the current context.
+void Sema::MarkUsingReferenced(Decl *D, CXXScopeSpec *SS, Expr *E) {

probinson wrote:
> `\brief` is unnecessary, as we have auto-brief turned on.
Removed the `\brief` format.




Comment at: lib/Sema/Sema.cpp:1903
+  if (auto *NNS = SS ? SS->getScopeRep()
+ : E ? dyn_cast(E)->getQualifier()
+ : nullptr) {

probinson wrote:
> This dyn_cast<> can be simply a cast<>.
Changed the dyn_cast<> to cast<>



Comment at: lib/Sema/Sema.cpp:1916
+  if ((USD = dyn_cast(DR)) && !USD->isReferenced()) {
+if (USD->getTargetDecl() == D) {
+  USD->setReferenced();

probinson wrote:
> You could sink the declaration of USD like so:
> ```
> for (auto *DR : S->decls())
>   if (auto *USD = dyn_cast(DR))
> if (!USD->isReferenced() && USD->getTargetDecl() == D) {
> ```
> Also I would put braces around the 'for' loop body, even though it is 
> technically one statement.
Not available in the new patch.



Comment at: lib/Sema/Sema.cpp:1927
+// Check if the declaration was introduced by a 'using-directive'.
+auto *Target = dyn_cast(DC);
+for (auto *UD : S->using_directives())

probinson wrote:
> You verified that his is a namespace earlier, so use cast<> not dyn_cast<>.
Not available in the new patch.


https://reviews.llvm.org/D46190



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

[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:3744
 AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext(CodeCompletionContext::CCC_Expression, 

ioeric wrote:
> ilya-biryukov wrote:
> > `ResultsBuilder`'s constructor accepts a `CodeCompletionContext`. Can we 
> > pass in the context with `PreferedType` there instead of reconstructing it 
> > later?
> > To make sure we don't miss other things (incl. any future additions) that 
> > `ResultsBuilder` puts into the context.
> The `PreferedType` should actually already be set in the `ResultsBuilder` 
> (line 3715). In thoery, `Results.getCompletionContext()` should work fine 
> here as well, but it would break some Index tests - it introduced some 
> inconsistency in sema scoring when running with and without result caching 
> (https://github.com/llvm-mirror/clang/blob/master/test/Index/complete-exprs.c#L35).
>  This is probably a bug, but I'm not sure if I'm the right person to chase it 
> :( 
What kind of inconsistencies? Maybe we should just update the CHECKS in the 
test?


Repository:
  rC Clang

https://reviews.llvm.org/D48917



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

[PATCH] D48881: [clangd] Avoid collecting symbols from broken TUs in global-symbol-builder.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:88
+(CI.getDiagnosticClient().getNumErrors() > 0)) {
+  llvm::errs() << "Found errors in the translation unit. Igoring "
+  "collected symbols...\n";

ilya-biryukov wrote:
> NIT: Maybe use clangd's `log` here? To get timestamps in the output, etc.
The default logger doesn't seem to add timestamp? I'll keep `llvm::errs()` here 
for consistency in this file.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48881



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

[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types

[Gabor Marton via Phabricator via cfe-commits]

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



Comment at: lib/AST/ASTImporter.cpp:2085
 }
+  } else {
+if (!IsStructuralMatch(D, FoundRecord, false))

balazske wrote:
> martong wrote:
> > a_sidorin wrote:
> > > Is it possible to use the added code for the entire condition `if (auto 
> > > *FoundRecord = dyn_cast(Found))`, replacing its body? Our 
> > > structural matching already knows how to handle unnamed structures, and 
> > > the upper code partially duplicates 
> > > `IsStructurallyEquivalent(StructuralEquivalenceContext 
> > > ,RecordDecl *D1, RecordDecl *D2)`. Can we change structural 
> > > matching to handle this stuff instead of doing it in ASTNodeImporter?
> > Yes, this is a good point, updated the patch accordingly.
> My idea was to remove this whole `if (!SearchName)` branch, but some 
> restructuring of the next conditions may be needed. Setting of `PrevDecl` in 
> the `if` branch does not look correct (if `!SearchName` is false it is never 
> set, unlike in the old code).
I tried what you mentioned Balazs.
It turned out, we can't just skip the whole `if (!SearchName)` branch. Because 
in the below case we would match the first unnamed union with the second, 
therefore we will break functionality (and some lit tests). The `continue` at 
line 2078 is very important in this edge case. (I agree that this code is very 
messy and would be good if we could refactor that to be cleaner, particularly 
if we could remove the `SearchName` variable would make it way better.)
```
// Matches
struct Unnamed {
  union {
struct {
  int i;
} S;
struct {
  float i;
} R;
  } U;
} x14;

```

The failing lit test and it's output:
```
/home/egbomrt/WORK/llvm3/git/llvm/tools/clang/test/ASTMerge/struct/Inputs/struct1.c:84:5:
 warning: type 'struct Unnamed::(anonymous at 
/home/egbomrt/WORK/llvm3/git/llvm/tools/clang/test/ASTMerge/struct/Inputs/struct1.c:84:5)'
 has incompatible definitions in different translation units
struct {
^
/home/egbomrt/WORK/llvm3/git/llvm/tools/clang/test/ASTMerge/struct/Inputs/struct1.c:85:11:
 note: field 'i' has type 'int' here
  int i;
  ^
/home/egbomrt/WORK/llvm3/git/llvm/tools/clang/test/ASTMerge/struct/Inputs/struct1.c:88:13:
 note: field 'i' has type 'float' here
  float i;
^

```


Repository:
  rC Clang

https://reviews.llvm.org/D48773



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

[PATCH] D48881: [clangd] Avoid collecting symbols from broken TUs in global-symbol-builder.

[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 with a minor NIT




Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:88
+(CI.getDiagnosticClient().getNumErrors() > 0)) {
+  llvm::errs() << "Found errors in the translation unit. Igoring "
+  "collected symbols...\n";

NIT: Maybe use clangd's `log` here? To get timestamps in the output, etc.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48881



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

[PATCH] D48628: [AST] Structural equivalence of methods

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 154073.
balazske added a comment.

- Updates according to review comments


Repository:
  rC Clang

https://reviews.llvm.org/D48628

Files:
  lib/AST/ASTImporter.cpp
  lib/AST/ASTStructuralEquivalence.cpp
  unittests/AST/ASTImporterTest.cpp
  unittests/AST/StructuralEquivalenceTest.cpp

Index: unittests/AST/StructuralEquivalenceTest.cpp
===
--- unittests/AST/StructuralEquivalenceTest.cpp
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -18,13 +18,13 @@
   std::unique_ptr AST0, AST1;
   std::string Code0, Code1; // Buffers for SourceManager
 
-  // Get a pair of Decl pointers to the synthetised declarations from the given
-  // code snipets. By default we search for the unique Decl with name 'foo' in
-  // both snippets.
-  std::tuple
-  makeNamedDecls(const std::string , const std::string ,
- Language Lang, const char *const Identifier = "foo") {
-
+  // Get a pair of node pointers into the synthesized AST from the given code
+  // snippets. To determine the returned node, a separate matcher is specified
+  // for both snippets. The first matching node is returned.
+  template 
+  std::tuple makeDecls(
+  const std::string , const std::string , Language Lang,
+  const MatcherType , const MatcherType ) {
 this->Code0 = SrcCode0;
 this->Code1 = SrcCode1;
 ArgVector Args = getBasicRunOptionsForLanguage(Lang);
@@ -34,28 +34,32 @@
 AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args, InputFileName);
 AST1 = tooling::buildASTFromCodeWithArgs(Code1, Args, InputFileName);
 
-ASTContext  = AST0->getASTContext(),  = AST1->getASTContext();
-
-auto getDecl = [](ASTContext , const std::string ) -> NamedDecl * {
-  IdentifierInfo *SearchedII = (Name);
-  assert(SearchedII && "Declaration with the identifier "
-   "should be specified in test!");
-  DeclarationName SearchDeclName(SearchedII);
-  SmallVector FoundDecls;
-  Ctx.getTranslationUnitDecl()->localUncachedLookup(SearchDeclName,
-FoundDecls);
+NodeType *D0 = FirstDeclMatcher().match(
+AST0->getASTContext().getTranslationUnitDecl(), Matcher0);
+NodeType *D1 = FirstDeclMatcher().match(
+AST1->getASTContext().getTranslationUnitDecl(), Matcher1);
 
-  // We should find one Decl but one only.
-  assert(FoundDecls.size() == 1);
+return std::make_tuple(D0, D1);
+  }
 
-  return FoundDecls[0];
-};
+  // Get a pair of node pointers into the synthesized AST from the given code
+  // snippets. The same matcher is used for both snippets.
+  template 
+  std::tuple makeDecls(
+  const std::string , const std::string , Language Lang,
+  const MatcherType ) {
+return makeDecls(
+  SrcCode0, SrcCode1, Lang, AMatcher, AMatcher);
+  }
 
-NamedDecl *D0 = getDecl(Ctx0, Identifier);
-NamedDecl *D1 = getDecl(Ctx1, Identifier);
-assert(D0);
-assert(D1);
-return std::make_tuple(D0, D1);
+  // Get a pair of Decl pointers to the synthesized declarations from the given
+  // code snippets. We search for the first NamedDecl with given name in both
+  // snippets.
+  std::tuple makeNamedDecls(
+  const std::string , const std::string ,
+  Language Lang, const char *const Identifier = "foo") {
+auto Matcher = namedDecl(hasName(Identifier));
+return makeDecls(SrcCode0, SrcCode1, Lang, Matcher);
   }
 
   bool testStructuralMatch(NamedDecl *D0, NamedDecl *D1) {
@@ -110,35 +114,29 @@
 }
 
 TEST_F(StructuralEquivalenceTest, IntVsSignedIntTemplateSpec) {
-  auto Decls = makeNamedDecls(
-  "template  struct foo; template<> struct foo{};",
-  "template  struct foo; template<> struct foo{};",
-  Lang_CXX);
-  ClassTemplateSpecializationDecl *Spec0 =
-  *cast(get<0>(Decls))->spec_begin();
-  ClassTemplateSpecializationDecl *Spec1 =
-  *cast(get<1>(Decls))->spec_begin();
-  ASSERT_TRUE(Spec0 != nullptr);
-  ASSERT_TRUE(Spec1 != nullptr);
+  auto Decls = makeDecls(
+  R"(template  struct foo; template<> struct foo{};)",
+  R"(template  struct foo; template<> struct foo{};)",
+  Lang_CXX,
+  classTemplateSpecializationDecl());
+  auto Spec0 = get<0>(Decls);
+  auto Spec1 = get<1>(Decls);
   EXPECT_TRUE(testStructuralMatch(Spec0, Spec1));
 }
 
 TEST_F(StructuralEquivalenceTest, CharVsSignedCharTemplateSpec) {
-  auto Decls = makeNamedDecls(
-  "template  struct foo; template<> struct foo{};",
-  "template  struct foo; template<> struct foo{};",
-  Lang_CXX);
-  ClassTemplateSpecializationDecl *Spec0 =
-  *cast(get<0>(Decls))->spec_begin();
-  ClassTemplateSpecializationDecl *Spec1 =
-  *cast(get<1>(Decls))->spec_begin();
-  ASSERT_TRUE(Spec0 != nullptr);
-  ASSERT_TRUE(Spec1 != nullptr);
+  auto Decls = makeDecls(
+  R"(template  struct foo; template<> struct foo{};)",
+  R"(template  

[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:3744
 AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
-  HandleCodeCompleteResults(this, CodeCompleter, 
-CodeCompletionContext(CodeCompletionContext::CCC_Expression, 

ilya-biryukov wrote:
> `ResultsBuilder`'s constructor accepts a `CodeCompletionContext`. Can we pass 
> in the context with `PreferedType` there instead of reconstructing it later?
> To make sure we don't miss other things (incl. any future additions) that 
> `ResultsBuilder` puts into the context.
The `PreferedType` should actually already be set in the `ResultsBuilder` (line 
3715). In thoery, `Results.getCompletionContext()` should work fine here as 
well, but it would break some Index tests - it introduced some inconsistency in 
sema scoring when running with and without result caching 
(https://github.com/llvm-mirror/clang/blob/master/test/Index/complete-exprs.c#L35).
 This is probably a bug, but I'm not sure if I'm the right person to chase it 
:( 


Repository:
  rC Clang

https://reviews.llvm.org/D48917



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

[PATCH] D48916: Fix setting of empty implicit-section-name attribute for functions affected by '#pragma clang section'

[Petr Pavlu via Phabricator via cfe-commits]

petpav01 added inline comments.



Comment at: test/CodeGen/clang-sections-attribute.c:1
+// RUN: %clang_cc1 -emit-llvm -triple arm-none-eabi -o - %s | FileCheck %s
+

chill wrote:
> Isn't it possible for the test to fail if the Arm target is not configured?
I think it should not be necessary to add a REQUIRES line because the test 
emits and checks only LLVM IR but no target codegen is done. An experiment 
without having the ARM target configured shows that the test still works in 
such a setting. Note also that the two already present tests for `#pragma clang 
section` (`clang-sections.cpp` and `clang-sections-tentative.c`) have a similar 
header as this new test as well.


Repository:
  rC Clang

https://reviews.llvm.org/D48916



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

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Sorry, have missed the @hokein 's comments, so one of mine seems like a 
duplicate.


Repository:
  rC Clang

https://reviews.llvm.org/D48903



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

[PATCH] D48441: [clangd] Incorporate transitive #includes into code complete proximity scoring.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a subscriber: sammccall.
ioeric added a comment.

Hi Carlos, thanks for letting us know! I sent r336249 to fix the windows
test.


Repository:
  rL LLVM

https://reviews.llvm.org/D48441



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

Re: [PATCH] D48441: [clangd] Incorporate transitive #includes into code complete proximity scoring.

[Eric Liu via cfe-commits]

Hi Carlos, thanks for letting us know! I sent r336249 to fix the windows
test.

On Wed, Jul 4, 2018 at 9:51 AM Carlos Alberto Enciso via Phabricator <
revi...@reviews.llvm.org> wrote:

> CarlosAlbertoEnciso added a comment.
>
> Hi,
>
> It seems that your patch is causing an error in the tests for
>
> http://lab.llvm.org:8011/builders/clang-x64-ninja-win7
>
>
> http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/11510/steps/ninja%20check%201/logs/FAIL%3A%20Extra%20Tools%20Unit%20Tests%3A%3AFileDistanceTests.URI
>
> Thanks very much
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D48441
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r336249 - Try to fix FileDistance test on windows.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Jul  4 02:08:40 2018
New Revision: 336249

URL: http://llvm.org/viewvc/llvm-project?rev=336249=rev
Log:
Try to fix FileDistance test on windows.

http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/11510/steps/ninja%20check%201/logs/FAIL%3A%20Extra%20Tools%20Unit%20Tests%3A%3AFileDistanceTests.URI

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

Modified: clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp?rev=336249=336248=336249=diff
==
--- clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp Wed Jul  4 
02:08:40 2018
@@ -67,9 +67,15 @@ TEST(FileDistanceTests, URI) {
   SourceParams CostLots;
   CostLots.Cost = 1000;
 
-  URIDistance D(
-  {{testPath("foo"), CostLots}, {"/not/a/testpath", SourceParams()}}, 
Opts);
+  URIDistance D({{testPath("foo"), CostLots},
+ {"/not/a/testpath", SourceParams()},
+ {"C:\\not\\a\\testpath", SourceParams()}},
+Opts);
+#ifdef _WIN32
+  EXPECT_EQ(D.distance("file:///C%3a/not/a/testpath/either"), 3u);
+#else
   EXPECT_EQ(D.distance("file:///not/a/testpath/either"), 3u);
+#endif
   EXPECT_EQ(D.distance("unittest:foo"), 1000u);
   EXPECT_EQ(D.distance("unittest:bar"), 1008u);
 }


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

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

> I think the fixing way is to normalize the file path from AST (making it 
> absolute).

Totally agree. Could we run the code used to get the URI to store in the 
dynamic index? Should we expose and reuse code in `getSymbolLocation()` from 
`SymbolCollector.cpp`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48687



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

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a reviewer: ilya-biryukov.
ilya-biryukov added a comment.

Mimicing RealFS seems like the right thing to do here, so I would vouch for 
checking this change in.
I'm a little worried that there are tests/clients relying on the old behavior, 
have you run all the tests?

Also, could you please run `git-clang-format` to fix the formatting issues?




Comment at: lib/Basic/VirtualFileSystem.cpp:516
+  explicit InMemoryFileAdaptor(InMemoryFile , std::string RequestedName)
+  : Node(Node), RequestedName (std::move (RequestedName))
+  {}

NIT: The formatting is broken here.



Comment at: lib/Basic/VirtualFileSystem.cpp:520
+  llvm::ErrorOr status() override {
+Status St = Node.getStatus();
+return Status::copyWithNewName(St, RequestedName);

Maybe add a `RequestedName` parameter to the `InMemoryNode` instead to make 
sure it's not misused?
It looks like all the clients calling it have to change the name and some are 
not doing it now, e.g. the directory iterator will use statuses with full paths.



Comment at: lib/Basic/VirtualFileSystem.cpp:521
+Status St = Node.getStatus();
+return Status::copyWithNewName(St, RequestedName);
+  }

Maybe add a comment that this matches the real file system behavior?



Comment at: lib/Basic/VirtualFileSystem.cpp:722
   if (Node)
-return (*Node)->getStatus();
+{
+  Status St = (*Node)->getStatus();

NIT: The indent is incorrect here.



Comment at: lib/Basic/VirtualFileSystem.cpp:724
+  Status St = (*Node)->getStatus();
+  return Status::copyWithNewName(St, Path.str());
+}

NIT: we don't need `str()` here


Repository:
  rC Clang

https://reviews.llvm.org/D48903



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

[clang-tools-extra] r336248 - [clangd] FileDistance: temporarily disable in CodeComplete, it's behaving badly

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Jul  4 02:01:04 2018
New Revision: 336248

URL: http://llvm.org/viewvc/llvm-project?rev=336248=rev
Log:
[clangd] FileDistance: temporarily disable in CodeComplete, it's behaving badly

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

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=336248=336247=336248=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Jul  4 02:01:04 2018
@@ -1137,7 +1137,8 @@ private:
 SymbolQualitySignals Quality;
 SymbolRelevanceSignals Relevance;
 Relevance.Query = SymbolRelevanceSignals::CodeComplete;
-Relevance.FileProximityMatch = FileProximity.getPointer();
+// FIXME: re-enable this after working out why it eats memory.
+// Relevance.FileProximityMatch = FileProximity.getPointer();
 auto  = Bundle.front();
 if (auto FuzzyScore = fuzzyScore(First))
   Relevance.NameMatch = *FuzzyScore;


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

[PATCH] D48921: NFC - type fix in test/CodeGenCXX/runtime-dllstorage.cpp

[Gabor Buella via Phabricator via cfe-commits]

GBuella created this revision.
GBuella added reviewers: compnerd, espindola.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D48921

Files:
  test/CodeGenCXX/runtime-dllstorage.cpp


Index: test/CodeGenCXX/runtime-dllstorage.cpp
===
--- test/CodeGenCXX/runtime-dllstorage.cpp
+++ test/CodeGenCXX/runtime-dllstorage.cpp
@@ -1,13 +1,13 @@
 // RUN: %clang_cc1 -triple i686-windows-msvc -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -emit-llvm -o - %s | FileCheck 
%s -check-prefix CHECK-MS -check-prefix CHECK-MS-DYNAMIC
 // RUN: %clang_cc1 -triple i686-windows-msvc -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std 
-emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-MS -check-prefix 
CHECK-MS-STATIC
 
-// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -emit-llvm -o - %s | FileCheck 
%s -check-prefix CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix 
CHECK-DYNAMIC-NODECL-IA -check-prefix CHECK-DYANMIC-IA-CXA-ATEXIT
+// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -emit-llvm -o - %s | FileCheck 
%s -check-prefix CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix 
CHECK-DYNAMIC-NODECL-IA -check-prefix CHECK-DYNAMIC-IA-CXA-ATEXIT
 // RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std 
-emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix 
CHECK-STATIC-IA -check-prefix CHECK-STATIC-NODECL-IA -check-prefix 
CHECK-IA-STATIC-CXA-ATEXIT
-// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DIMPORT_DECLARATIONS 
-emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix 
CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-IMPORT-IA -check-prefix 
CHECK-DYANMIC-IA-CXA-ATEXIT
+// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DIMPORT_DECLARATIONS 
-emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix 
CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-IMPORT-IA -check-prefix 
CHECK-DYNAMIC-IA-CXA-ATEXIT
 // RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std 
-DIMPORT_DECLARATIONS -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA 
-check-prefix CHECK-STATIC-IA -check-prefix CHECK-STATIC-IMPORT-IA 
-check-prefix CHECK-IA-STATIC-CXA-ATEXIT
-// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DEXPORT_DECLARATIONS 
-emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix 
CHECK-DYNAMIC-IA -check-prefix CHECK-DYANMIC-IA-CXA-ATEXIT
+// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DEXPORT_DECLARATIONS 
-emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix 
CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-IA-CXA-ATEXIT
 // RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std 
-DEXPORT_DECLARATIONS -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA 
-check-prefix CHECK-STATIC-IA -check-prefix CHECK-IA-STATIC-CXA-ATEXIT
-// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DDECL -emit-llvm -o - %s | 
FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-DYNAMIC-IA 
-check-prefix CHECK-DYNAMIC-DECL-IA -check-prefix CHECK-DYANMIC-IA-CXA-ATEXIT
+// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DDECL -emit-llvm -o - %s | 
FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-DYNAMIC-IA 
-check-prefix CHECK-DYNAMIC-DECL-IA -check-prefix CHECK-DYNAMIC-IA-CXA-ATEXIT
 // RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std 
-DDECL -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix 
CHECK-STATIC-IA -check-prefix CHECK-STATIC-DECL-IA -check-prefix 
CHECK-IA-STATIC-CXA-ATEXIT
 // %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -fno-use-cxa-atexit -emit-llvm 
-o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-DYNAMIC-IA 
-check-prefix CHECK-DYNAMIC-IA-ATEXIT
 // %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -fno-use-cxa-atexit -emit-llvm 
-o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix 

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

[Haojian Wu via Phabricator via cfe-commits]

hokein added subscribers: bkramer, hokein.
hokein added a comment.

I'm not familiar with this part of code, but the change looks fine to me. I 
think @bkramer is the right person to review it.

Please make sure the style align with LLVM code style.




Comment at: lib/Basic/VirtualFileSystem.cpp:507
 
 /// Adapt a InMemoryFile for VFS' File interface.
 class InMemoryFileAdaptor : public File {

I think we should have a comment saying the InMemoryFile has the same behavior 
as the real file system.


Repository:
  rC Clang

https://reviews.llvm.org/D48903



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

[PATCH] D48427: [Analyzer] Fix for D47417 to make the tests pass

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 154066.
baloghadamsoftware added a comment.

Adding of transition removed since it is part of 
https://reviews.llvm.org/D47417.


https://reviews.llvm.org/D48427

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp


Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -291,6 +291,7 @@
   const MemRegion *Cont);
 ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
  const ContainerData );
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
 bool isOutOfRange(ProgramStateRef State, const IteratorPosition );
 bool isZero(ProgramStateRef State, const NonLoc );
 } // namespace
@@ -532,7 +533,9 @@
   auto ContMap = State->get();
   for (const auto Cont : ContMap) {
 if (!SR.isLiveRegion(Cont.first)) {
-  State = State->remove(Cont.first);
+  if (!hasLiveIterators(State, Cont.first)) {
+State = State->remove(Cont.first);
+  }
 }
   }
 
@@ -1174,6 +1177,22 @@
   return State;
 }
 
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont) {
+  auto RegionMap = State->get();
+  for (const auto Reg : RegionMap) {
+if (Reg.second.getContainer() == Cont)
+  return true;
+  }
+
+  auto SymbolMap = State->get();
+  for (const auto Sym : SymbolMap) {
+if (Sym.second.getContainer() == Cont)
+  return true;
+  }
+
+  return false;
+}
+
 bool isZero(ProgramStateRef State, const NonLoc ) {
   auto  = State->getBasicVals();
   return compare(State, Val,


Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -291,6 +291,7 @@
   const MemRegion *Cont);
 ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
  const ContainerData );
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
 bool isOutOfRange(ProgramStateRef State, const IteratorPosition );
 bool isZero(ProgramStateRef State, const NonLoc );
 } // namespace
@@ -532,7 +533,9 @@
   auto ContMap = State->get();
   for (const auto Cont : ContMap) {
 if (!SR.isLiveRegion(Cont.first)) {
-  State = State->remove(Cont.first);
+  if (!hasLiveIterators(State, Cont.first)) {
+State = State->remove(Cont.first);
+  }
 }
   }
 
@@ -1174,6 +1177,22 @@
   return State;
 }
 
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont) {
+  auto RegionMap = State->get();
+  for (const auto Reg : RegionMap) {
+if (Reg.second.getContainer() == Cont)
+  return true;
+  }
+
+  auto SymbolMap = State->get();
+  for (const auto Sym : SymbolMap) {
+if (Sym.second.getContainer() == Cont)
+  return true;
+  }
+
+  return false;
+}
+
 bool isZero(ProgramStateRef State, const NonLoc ) {
   auto  = State->getBasicVals();
   return compare(State, Val,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r336246 - [clangd] FileDistance: missing constexpr

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Jul  4 01:52:13 2018
New Revision: 336246

URL: http://llvm.org/viewvc/llvm-project?rev=336246=rev
Log:
[clangd] FileDistance: missing constexpr

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

Modified: clang-tools-extra/trunk/clangd/FileDistance.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FileDistance.cpp?rev=336246=336245=336246=diff
==
--- clang-tools-extra/trunk/clangd/FileDistance.cpp (original)
+++ clang-tools-extra/trunk/clangd/FileDistance.cpp Wed Jul  4 01:52:13 2018
@@ -54,7 +54,7 @@ static SmallString<128> canonicalize(Str
   return Result;
 }
 
-const unsigned FileDistance::kUnreachable;
+constexpr const unsigned FileDistance::kUnreachable;
 
 FileDistance::FileDistance(StringMap Sources,
const FileDistanceOptions )


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

[PATCH] D48880: [Sema] Fix crash in getConstructorName.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC336244: [Sema] Fix crash in getConstructorName. (authored by 
ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48880?vs=154063=154064#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48880

Files:
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/injected-class-name-crash.cpp


Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -113,6 +113,8 @@
   break;
 }
   }
+  if (!InjectedClassName && CurClass->isInvalidDecl())
+return ParsedType();
   assert(InjectedClassName && "couldn't find injected class name");
 
   QualType T = Context.getTypeDeclType(InjectedClassName);
Index: test/SemaCXX/injected-class-name-crash.cpp
===
--- test/SemaCXX/injected-class-name-crash.cpp
+++ test/SemaCXX/injected-class-name-crash.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template 
+struct X : public Foo
+X::X() {
+}


Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -113,6 +113,8 @@
   break;
 }
   }
+  if (!InjectedClassName && CurClass->isInvalidDecl())
+return ParsedType();
   assert(InjectedClassName && "couldn't find injected class name");
 
   QualType T = Context.getTypeDeclType(InjectedClassName);
Index: test/SemaCXX/injected-class-name-crash.cpp
===
--- test/SemaCXX/injected-class-name-crash.cpp
+++ test/SemaCXX/injected-class-name-crash.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template 
+struct X : public Foo
+X::X() {
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits