r348239 - [WIP][Sema] Improve static_assert diagnostics for type traits.

[Clement Courbet via cfe-commits]

Author: courbet
Date: Mon Dec  3 23:59:57 2018
New Revision: 348239

URL: http://llvm.org/viewvc/llvm-project?rev=348239&view=rev
Log:
[WIP][Sema] Improve static_assert diagnostics for type traits.

Summary:
In our codebase, `static_assert(std::some_type_trait::value, "msg")`
(where `some_type_trait` is an std type_trait and `Ts...` is the
appropriate template parameters) account for 11.2% of the `static_assert`s.

In these cases, the `Ts` are typically not spelled out explicitly, e.g.
`static_assert(std::is_same::value, "message");`

The diagnostic when the assert fails is typically not very useful, e.g.
`static_assert failed due to requirement 'std::is_same::value' "message"`

This change makes the diagnostic spell out the types explicitly , e.g.
`static_assert failed due to requirement 'std::is_same::value' 
"message"`

See tests for more examples.

After this is submitted, I intend to handle
`static_assert(!std::some_type_trait::value, "msg")`,
which is another 6.6% of static_asserts.

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/SemaCXX/static-assert-cxx17.cpp
Modified:
cfe/trunk/include/clang/AST/NestedNameSpecifier.h
cfe/trunk/lib/AST/NestedNameSpecifier.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/SemaCXX/static-assert.cpp

Modified: cfe/trunk/include/clang/AST/NestedNameSpecifier.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/NestedNameSpecifier.h?rev=348239&r1=348238&r2=348239&view=diff
==
--- cfe/trunk/include/clang/AST/NestedNameSpecifier.h (original)
+++ cfe/trunk/include/clang/AST/NestedNameSpecifier.h Mon Dec  3 23:59:57 2018
@@ -212,9 +212,12 @@ public:
   /// parameter pack (for C++11 variadic templates).
   bool containsUnexpandedParameterPack() const;
 
-  /// Print this nested name specifier to the given output
-  /// stream.
-  void print(raw_ostream &OS, const PrintingPolicy &Policy) const;
+  /// Print this nested name specifier to the given output stream. If
+  /// `ResolveTemplateArguments` is true, we'll print actual types, e.g.
+  /// `ns::SomeTemplate` instead of
+  /// `ns::SomeTemplate`.
+  void print(raw_ostream &OS, const PrintingPolicy &Policy,
+ bool ResolveTemplateArguments = false) const;
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
 ID.AddPointer(Prefix.getOpaqueValue());

Modified: cfe/trunk/lib/AST/NestedNameSpecifier.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/NestedNameSpecifier.cpp?rev=348239&r1=348238&r2=348239&view=diff
==
--- cfe/trunk/lib/AST/NestedNameSpecifier.cpp (original)
+++ cfe/trunk/lib/AST/NestedNameSpecifier.cpp Mon Dec  3 23:59:57 2018
@@ -16,6 +16,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
@@ -270,9 +271,8 @@ bool NestedNameSpecifier::containsUnexpa
 
 /// Print this nested name specifier to the given output
 /// stream.
-void
-NestedNameSpecifier::print(raw_ostream &OS,
-   const PrintingPolicy &Policy) const {
+void NestedNameSpecifier::print(raw_ostream &OS, const PrintingPolicy &Policy,
+bool ResolveTemplateArguments) const {
   if (getPrefix())
 getPrefix()->print(OS, Policy);
 
@@ -305,6 +305,15 @@ NestedNameSpecifier::print(raw_ostream &
 LLVM_FALLTHROUGH;
 
   case TypeSpec: {
+const auto *Record =
+
dyn_cast_or_null(getAsRecordDecl());
+if (ResolveTemplateArguments && Record) {
+// Print the type trait with resolved template parameters.
+Record->printName(OS);
+printTemplateArgumentList(OS, Record->getTemplateArgs().asArray(),
+  Policy);
+break;
+}
 const Type *T = getAsType();
 
 PrintingPolicy InnerPolicy(Policy);

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=348239&r1=348238&r2=348239&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Mon Dec  3 23:59:57 2018
@@ -3052,6 +3052,28 @@ static Expr *lookThroughRangesV3Conditio
   return Cond;
 }
 
+// Print a diagnostic for the failing static_assert expression. Defaults to
+// pretty-printing the expression.
+static void prettyPrintFailedBooleanCondition(llvm::raw_string_ostream &OS,
+  const Expr *FailedCond,
+  const PrintingPolicy &Policy) {
+  const auto *DR = dyn_cast(FailedCond);
+  if (DR && DR->getQualifier()) {
+// If this is a

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

[Clement Courbet via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348239: [WIP][Sema] Improve static_assert diagnostics for 
type traits. (authored by courbet, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D54903

Files:
  cfe/trunk/include/clang/AST/NestedNameSpecifier.h
  cfe/trunk/lib/AST/NestedNameSpecifier.cpp
  cfe/trunk/lib/Sema/SemaTemplate.cpp
  cfe/trunk/test/SemaCXX/static-assert-cxx17.cpp
  cfe/trunk/test/SemaCXX/static-assert.cpp

Index: cfe/trunk/include/clang/AST/NestedNameSpecifier.h
===
--- cfe/trunk/include/clang/AST/NestedNameSpecifier.h
+++ cfe/trunk/include/clang/AST/NestedNameSpecifier.h
@@ -212,9 +212,12 @@
   /// parameter pack (for C++11 variadic templates).
   bool containsUnexpandedParameterPack() const;
 
-  /// Print this nested name specifier to the given output
-  /// stream.
-  void print(raw_ostream &OS, const PrintingPolicy &Policy) const;
+  /// Print this nested name specifier to the given output stream. If
+  /// `ResolveTemplateArguments` is true, we'll print actual types, e.g.
+  /// `ns::SomeTemplate` instead of
+  /// `ns::SomeTemplate`.
+  void print(raw_ostream &OS, const PrintingPolicy &Policy,
+ bool ResolveTemplateArguments = false) const;
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
 ID.AddPointer(Prefix.getOpaqueValue());
Index: cfe/trunk/test/SemaCXX/static-assert.cpp
===
--- cfe/trunk/test/SemaCXX/static-assert.cpp
+++ cfe/trunk/test/SemaCXX/static-assert.cpp
@@ -68,3 +68,100 @@
 };
 
 static_assert(first_trait::value && second_trait::value, "message"); // expected-error{{static_assert failed due to requirement 'second_trait::value' "message"}}
+
+namespace std {
+
+template 
+struct integral_constant {
+  static const Tp value = v;
+  typedef Tp value_type;
+  typedef integral_constant type;
+};
+
+template 
+const Tp integral_constant::value;
+
+typedef integral_constant true_type;
+typedef integral_constant false_type;
+
+template 
+struct is_const : public false_type {};
+template 
+struct is_const : public true_type {};
+
+// We do not define is_same in terms of integral_constant to check that both implementations are supported.
+template 
+struct is_same {
+  static const bool value = false;
+};
+
+template 
+struct is_same {
+  static const bool value = true;
+};
+
+} // namespace std
+
+struct ExampleTypes {
+  using T = int;
+  using U = float;
+};
+
+static_assert(std::is_same::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_same::value' "message"}}
+static_assert(std::is_const::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_const::value' "message"}}
+
+struct BI_tag {};
+struct RAI_tag : BI_tag {};
+struct MyIterator {
+  using tag = BI_tag;
+};
+struct MyContainer {
+  using iterator = MyIterator;
+};
+template 
+void foo() {
+  static_assert(std::is_same::value, "message");
+  // expected-error@-1{{static_assert failed due to requirement 'std::is_same::value' "message"}}
+}
+template void foo();
+// expected-note@-1{{in instantiation of function template specialization 'foo' requested here}}
+
+namespace ns {
+template 
+struct NestedTemplates1 {
+  struct NestedTemplates2 {
+template 
+struct NestedTemplates3 : public std::is_same {};
+  };
+};
+} // namespace ns
+
+template 
+void foo2() {
+  static_assert(::ns::NestedTemplates1::NestedTemplates2::template NestedTemplates3::value, "message");
+  // expected-error@-1{{static_assert failed due to requirement '::ns::NestedTemplates1::NestedTemplates2::NestedTemplates3::value' "message"}}
+}
+template void foo2();
+// expected-note@-1{{in instantiation of function template specialization 'foo2' requested here}}
+
+template 
+void foo3(T t) {
+  static_assert(std::is_const::value, "message");
+  // expected-error-re@-1{{static_assert failed due to requirement 'std::is_const<(lambda at {{.*}}static-assert.cpp:{{[0-9]*}}:{{[0-9]*}})>::value' "message"}}
+  static_assert(std::is_const::value, "message");
+  // expected-error-re@-1{{static_assert failed due to requirement 'std::is_const<(lambda at {{.*}}static-assert.cpp:{{[0-9]*}}:{{[0-9]*}})>::value' "message"}}
+}
+void callFoo3() {
+  foo3([]() {});
+  // expected-note@-1{{in instantiation of function template specialization 'foo3<(lambda at }}
+}
+
+template 
+void foo4(T t) {
+  static_assert(std::is_const::value, "message");
+  // expected-error@-1{{type 'int' cannot be used prior to '::' because it has no members}}
+}
+void callFoo4() { foo4(42); }
+// expected-note@-1{{in instantiation of function template specialization 'foo4' requested here}}
Index: cfe/trunk/test/SemaCXX/static-assert-cxx17.cpp

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete updated this revision to Diff 176554.
Rakete added a comment.

Rebase + friendly ping :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D53847

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Parse/Parser.h
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseTemplate.cpp
  lib/Parse/ParseTentative.cpp
  lib/Parse/Parser.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaTemplate.cpp
  test/CXX/drs/dr1xx.cpp
  test/CXX/drs/dr2xx.cpp
  test/CXX/drs/dr4xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/CXX/temp/temp.res/temp.dep/temp.dep.type/p1.cpp
  test/FixIt/fixit.cpp
  test/Parser/cxx-member-initializers.cpp
  test/Parser/editor-placeholder-recovery.cpp
  test/SemaCXX/MicrosoftCompatibility.cpp
  test/SemaCXX/MicrosoftExtensions.cpp
  test/SemaCXX/MicrosoftSuper.cpp
  test/SemaCXX/unknown-type-name.cpp

Index: test/SemaCXX/unknown-type-name.cpp
===
--- test/SemaCXX/unknown-type-name.cpp
+++ test/SemaCXX/unknown-type-name.cpp
@@ -36,15 +36,15 @@
 
   static int n;
   static type m;
-  static int h(T::type, int); // expected-error{{missing 'typename'}}
-  static int h(T::type x, char); // expected-error{{missing 'typename'}}
+  static int h(T::type, int); // expected-warning{{implicit 'typename' is a C++2a extension}}
+  static int h(T::type x, char); // expected-warning{{implicit 'typename' is a C++2a extension}}
 };
 
 template
-A::type g(T t) { return t; } // expected-error{{missing 'typename'}}
+A::type g(T t) { return t; } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template
-A::type A::f() { return type(); } // expected-error{{missing 'typename'}}
+A::type A::f() { return type(); } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template
 void f(T::type) { } // expected-error{{missing 'typename'}}
@@ -72,9 +72,7 @@
 
 int *p;
 
-// FIXME: We should assume that 'undeclared' is a type, not a parameter name
-//here, and produce an 'unknown type name' diagnostic instead.
-int f1(undeclared, int); // expected-error{{requires a type specifier}}
+int f1(undeclared, int); // expected-error{{unknown type name 'undeclared'}}
 
 int f2(undeclared, 0); // expected-error{{undeclared identifier}}
 
@@ -86,11 +84,11 @@
 
 template int A::n(T::value); // ok
 template
-A::type // expected-error{{missing 'typename'}}
+A::type // expected-warning {{implicit 'typename' is a C++2a extension}}
 A::m(T::value, 0); // ok
 
-template int A::h(T::type, int) {} // expected-error{{missing 'typename'}}
-template int A::h(T::type x, char) {} // expected-error{{missing 'typename'}}
+template int A::h(T::type, int) {} // expected-warning{{implicit 'typename' is a C++2a extension}}
+template int A::h(T::type x, char) {} // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template int h(T::type, int); // expected-error{{missing 'typename'}}
 template int h(T::type x, char); // expected-error{{missing 'typename'}}
@@ -118,4 +116,5 @@
 // FIXME: We know which type specifier should have been specified here. Provide
 //a fix-it to add 'typename A::type'
 template
-A::g() { } // expected-error{{requires a type specifier}}
+A::g() { } // expected-error{{expected unqualified-id}}
+// expected-warning@-1{{implicit 'typename' is a C++2a extension}}
Index: test/SemaCXX/MicrosoftSuper.cpp
===
--- test/SemaCXX/MicrosoftSuper.cpp
+++ test/SemaCXX/MicrosoftSuper.cpp
@@ -108,8 +108,8 @@
   typename __super::XXX a;
   typedef typename __super::XXX b;
 
-  __super::XXX c; // expected-error {{missing 'typename'}}
-  typedef __super::XXX d; // expected-error {{missing 'typename'}}
+  __super::XXX c; // expected-warning {{implicit 'typename' is a C++2a extension}}
+  typedef __super::XXX d; // expected-warning {{implicit 'typename' is a C++2a extension}}
 
   void foo() {
 typename __super::XXX e;
@@ -127,8 +127,8 @@
   typename __super::XXX a;
   typedef typename __super::XXX b;
 
-  __super::XXX c; // expected-error {{missing 'typename'}}
-  typedef __super::XXX d; // expected-error {{missing 'typename'}}
+  __super::XXX c; // expected-warning {{implicit 'typename' is a C++2a extension}}
+  typedef __super::XXX d; // expected-warning {{implicit 'typename' is a C++2a extension}}
 
   void foo() {
 typename __super::XXX e;
Index: test/SemaCXX/MicrosoftExtensions.cpp
===
--- test/SemaCXX/MicrosoftExtensions.cpp
+++ test/SemaCXX/MicrosoftExtensions.cpp
@@ -526,7 +526,7 @@
 
 namespace PR32750 {
 template struct A {};
-template struct B : A> { A::C::D d; }; // expected-error {{missing 'typename' prior to dependent type name

[PATCH] D55006: [clang] - Simplify tools::SplitDebugName

[George Rimar via Phabricator via cfe-commits]

grimar added a comment.

In D55006#1316985 , @JonasToth wrote:

> from my side no objections, mailing list did not react AFAIK (just in case 
> your waiting for me until you recommit).


David requested the test case, I added it and now waiting for his approval :]


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

https://reviews.llvm.org/D55006



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

[PATCH] D55255: Fix a false positive in misplaced-widening-cast

[Daniel Krupp via Phabricator via cfe-commits]

dkrupp created this revision.
dkrupp added reviewers: JonasToth, alexfh.
dkrupp added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, rnkovacs.

bugprone-misplaced-widening-cast check
used to give a false warning to the
following example.

  

enum DaysEnum{

  MON = 0,
  TUE = 1
  };


day = (DaysEnum)(day + 1);
 //warning: either cast from 'int' to 'DaysEnum' is ineffective...

But i think int to enum cast is not widening neither ineffective.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55255

Files:
  clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
  test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp


Index: test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp
===
--- test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp
+++ test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp
@@ -62,3 +62,29 @@
   enum Type {};
   static char *m_fn1() { char p = (Type)(&p - m_fn1()); }
 };
+
+enum DaysEnum{
+MON = 0,
+TUE = 1,
+WED = 2,
+THR = 3,
+FRI = 4,
+SAT = 5,
+SUN = 6
+};
+
+//do not warn for int to enum casts
+DaysEnum nextDay(DaysEnum day){
+  if (day < SUN)
+  day = (DaysEnum)(day + 1);
+  return day;
+}
+
+//do not warn for int to enum casts
+DaysEnum nextDay_cpp(DaysEnum day){
+  if (day < SUN)
+  day = static_cast(day + 1);
+  return day;
+}
+
+
Index: clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
===
--- clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
+++ clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
@@ -213,8 +213,9 @@
 dyn_cast(CastType->getUnqualifiedDesugaredType());
 const auto *CalcBuiltinType =
 dyn_cast(CalcType->getUnqualifiedDesugaredType());
-if (CastBuiltinType && CalcBuiltinType &&
-!isFirstWider(CastBuiltinType->getKind(), CalcBuiltinType->getKind()))
+if (!CastBuiltinType || !CalcBuiltinType)
+  return;
+if (!isFirstWider(CastBuiltinType->getKind(), CalcBuiltinType->getKind()))
   return;
   }
 


Index: test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp
===
--- test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp
+++ test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp
@@ -62,3 +62,29 @@
   enum Type {};
   static char *m_fn1() { char p = (Type)(&p - m_fn1()); }
 };
+
+enum DaysEnum{
+MON = 0,
+TUE = 1,
+WED = 2,
+THR = 3,
+FRI = 4,
+SAT = 5,
+SUN = 6
+};
+
+//do not warn for int to enum casts
+DaysEnum nextDay(DaysEnum day){
+  if (day < SUN)
+  day = (DaysEnum)(day + 1);
+  return day;
+}
+
+//do not warn for int to enum casts
+DaysEnum nextDay_cpp(DaysEnum day){
+  if (day < SUN)
+  day = static_cast(day + 1);
+  return day;
+}
+
+
Index: clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
===
--- clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
+++ clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
@@ -213,8 +213,9 @@
 dyn_cast(CastType->getUnqualifiedDesugaredType());
 const auto *CalcBuiltinType =
 dyn_cast(CalcType->getUnqualifiedDesugaredType());
-if (CastBuiltinType && CalcBuiltinType &&
-!isFirstWider(CastBuiltinType->getKind(), CalcBuiltinType->getKind()))
+if (!CastBuiltinType || !CalcBuiltinType)
+  return;
+if (!isFirstWider(CastBuiltinType->getKind(), CalcBuiltinType->getKind()))
   return;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno updated this revision to Diff 176559.
bruno added a comment.

Address @rsmith comments


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

https://reviews.llvm.org/D55097

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
  test/CXX/drs/dr6xx.cpp
  www/cxx_status.html

Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -953,13 +953,15 @@
 
   Relaxations of constexpr restrictions
   http://wg21.link/p1064r0";>P1064R0
-  No
+  No
 

 http://wg21.link/p1002r1";>P1002R1
+SVN
   
   
 http://wg21.link/p1327r1";>P1327R1
+No
   
   
 http://wg21.link/p1330r0";>P1330R0
Index: test/CXX/drs/dr6xx.cpp
===
--- test/CXX/drs/dr6xx.cpp
+++ test/CXX/drs/dr6xx.cpp
@@ -492,7 +492,13 @@
   struct C {
 constexpr C(NonLiteral);
 constexpr C(NonLiteral, int) {} // expected-error {{not a literal type}}
-constexpr C() try {} catch (...) {} // expected-error {{function try block}}
+constexpr C() try {} catch (...) {}
+#if __cplusplus <= 201703L
+// expected-error@-2 {{function try block in constexpr constructor is a C++2a extension}}
+#endif
+#if __cplusplus < 201402L
+// expected-error@-5 {{use of this statement in a constexpr constructor is a C++14 extension}}
+#endif
   };
 
   struct D {
Index: test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
===
--- test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
+++ test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -verify -std=c++11 -fcxx-exceptions -Werror=c++1y-extensions %s
-// RUN: %clang_cc1 -verify -std=c++1y -fcxx-exceptions -DCXX1Y %s
+// RUN: %clang_cc1 -verify -std=c++11 -fcxx-exceptions -Werror=c++1y-extensions -Werror=c++2a-extensions %s
+// RUN: %clang_cc1 -verify -std=c++1y -fcxx-exceptions -DCXX1Y -Werror=c++2a-extensions %s
+// RUN: %clang_cc1 -verify -std=c++2a -fcxx-exceptions -DCXX1Y -DCXX2A %s
 
 namespace N {
   typedef char C;
@@ -49,8 +50,14 @@
 // - its function-body shall not be a function-try-block;
 struct U {
   constexpr U()
-try // expected-error {{function try block not allowed in constexpr constructor}}
+try
+#ifndef CXX2A
+  // expected-error@-2 {{function try block in constexpr constructor is a C++2a extension}}
+#endif
 : u() {
+#ifndef CXX1Y
+  // expected-error@-2 {{use of this statement in a constexpr constructor is a C++14 extension}}
+#endif
   } catch (...) {
 throw;
   }
Index: test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
===
--- test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++11 -Werror=c++1y-extensions %s
-// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++1y -DCXX1Y %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++11 -Werror=c++1y-extensions -Werror=c++2a-extensions %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++1y -DCXX1Y -Werror=c++2a-extensions %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++2a -DCXX1Y -DCXX2A %s
 
 namespace N {
   typedef char C;
@@ -78,7 +79,12 @@
 };
 struct T3 {
   constexpr T3 &operator=(const T3&) const = default;
-  // expected-error@-1 {{an explicitly-defaulted copy assignment operator may not have 'const' or 'volatile' qualifiers}}
+#ifndef CXX2A
+  // expected-error@-2 {{an explicitly-defaulted copy assignment operator may not have 'const' or 'volatile' qualifiers}}
+#else
+  // expected-warning@-4 {{explicitly defaulted copy assignment operator is implicitly deleted}}
+  // expected-note@-5 {{function is implicitly deleted because its declared type does not match the type of an implicit copy assignment operator}}
+#endif
 };
 #endif
 struct U {
@@ -131,7 +137,13 @@
 }
 constexpr int DisallowedStmtsCXX1Y_3() {
   //  - a try-block,
-  try {} catch (...) {} // expected-error {{statement not allowed in constexpr function}}
+  try {} catch (...) {}
+#ifndef CXX2A
+  // expected-error@-2 {{use of this statement in a constexpr function is a C++2a extension}}
+#ifndef CXX1Y
+  // expected-error@-4 {{use of this statement in a constexpr function is a C++14 extension}}
+#endif
+#endif
   return 0;
 }
 constexpr int DisallowedStmtsCXX1Y_4() {
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -1803,7 +1803,7 @@
 static bool
 CheckConstexprFu

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: include/clang/Lex/Preprocessor.h:391
   } PreambleConditionalStack;
+  bool PreambleGenerationFailed = false;
 

There's a mechanism to handle preamble with errors, see 
`PreprocessorOpts::AllowPCHWithCompilerErrors`.
Maybe rely on it and avoid adding a different one?



Comment at: lib/Lex/PPDirectives.cpp:1945
+// Generate a fatal error to skip further processing.
+Diag(FilenameTok.getLocation(), diag::err_pp_error_opening_file) << ""
+ << "";

Maybe add a new error or find a more appropriate existing one to reuse?
"Error opening file", especially without any arguments does not provide enough 
context to figure out what went wrong.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53866



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

[PATCH] D55062: [clangd] Partition include graph on auto-index.

[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




Comment at: unittests/clangd/BackgroundIndexTests.cpp:204
+  EXPECT_EQ(ShardSource->Sources->size(), 2U); // A.cc, A.h
+  EXPECT_THAT(
+  ShardSource->Sources->lookup("unittest:///root/A.cc").DirectIncludes,

NIT: maybe also check the hash of `A.cc` is initialized (the value doesn't 
matter, probably, just to make sure it's not 0)?



Comment at: unittests/clangd/BackgroundIndexTests.cpp:212
+  EXPECT_TRUE(ShardHeader->Sources);
+  EXPECT_EQ(ShardHeader->Sources->size(), 2U); // A.h B.h
+  EXPECT_THAT(

NIT: maybe add a comma? `A.h, B.h`


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55062



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

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan abandoned this revision.
yvvan added a comment.

@zturner

The fact that so many call-sites decide what to do is pretty error-prone.
There was already at least one issue when this flag was not properly set by 
some of the call-sites.


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

https://reviews.llvm.org/D54995



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

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc requested changes to this revision.
chandlerc added a comment.
This revision now requires changes to proceed.

I think this should be `internal-driver-option` and the text updated? I don't 
think these are necessarily experimental, they're internals of the compiler 
implementation, and not a supported interface for users. Unsure how to best 
explain this.


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

https://reviews.llvm.org/D55150



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

[PATCH] D55256: [clangd] Support clang-tidy configuration in clangd.

[Haojian Wu via Phabricator via cfe-commits]

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

Clangd will support a minimal set of clang-tidy configurations

- respect .clang-tidy for each file
- add a `clang-tidy-checks` CLI option that can override options from 
.clang-tidy file

TODO: add a lit test.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55256

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/TUSchedulerTests.cpp
  unittests/clangd/TestTU.cpp

Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -37,6 +37,10 @@
   Inputs.CompileCommand.Directory = testRoot();
   Inputs.Contents = Code;
   Inputs.FS = buildTestFS({{FullFilename, Code}, {FullHeaderName, HeaderCode}});
+  Inputs.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults();
+  Inputs.ClangTidyOpts.Checks =
+  "-*, bugprone-sizeof-expression, bugprone-macro-repeated-side-effects, "
+  "modernize-deprecated-headers";
   auto PCHs = std::make_shared();
   auto Preamble =
   buildPreamble(FullFilename, *createInvocationFromCommandLine(Cmd),
Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -34,7 +34,8 @@
 protected:
   ParseInputs getInputs(PathRef File, std::string Contents) {
 return ParseInputs{*CDB.getCompileCommand(File),
-   buildTestFS(Files, Timestamps), std::move(Contents)};
+   buildTestFS(Files, Timestamps), std::move(Contents),
+   tidy::ClangTidyOptions::getDefaults()};
   }
 
   void updateWithCallback(TUScheduler &S, PathRef File, StringRef Contents,
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -364,7 +364,8 @@
   auto AST =
   ParsedAST::build(createInvocationFromCommandLine(Cmd), PreambleData,
MemoryBuffer::getMemBufferCopy(Main.code()),
-   std::make_shared(), PI.FS);
+   std::make_shared(), PI.FS,
+   tidy::ClangTidyOptions::getDefaults());
   ASSERT_TRUE(AST);
   FileIndex Index;
   Index.updateMain(MainFile, *AST);
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -188,6 +188,11 @@
  "placeholders for method parameters."),
 cl::init(clangd::CodeCompleteOptions().EnableFunctionArgSnippets));
 
+static cl::opt
+ClangTidyChecks("clang-tidy-checks",
+cl::desc("A list of clang-tidy checks running in clangd"),
+cl::init(""), cl::Hidden);
+
 namespace {
 
 /// \brief Supports a test URI scheme with relaxed constraints for lit tests.
@@ -234,7 +239,6 @@
 #else
 const char TestScheme::TestDir[] = "/clangd-test";
 #endif
-
 }
 
 int main(int argc, char *argv[]) {
@@ -388,8 +392,19 @@
   stdin, outs(),
   InputMirrorStream ? InputMirrorStream.getPointer() : nullptr, PrettyPrint,
   InputStyle);
+  RealFileSystemProvider FSProvider;
+  std::unique_ptr ClangTidyOptProvider;
+  if (!ClangTidyChecks.empty()) {
+// Create an empty option.
+auto ClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
+ClangTidyOptions.Checks = ClangTidyChecks;
+ClangTidyOptProvider = llvm::make_unique(
+tidy::ClangTidyGlobalOptions(), tidy::ClangTidyOptions::getDefaults(),
+ClangTidyOptions, FSProvider.getFileSystem());
+  }
+  Opts.ClangTidyOptProvider = ClangTidyOptProvider.get();
   ClangdLSPServer LSPServer(
-  *Transport, CCOpts, CompileCommandsDirPath,
+  *Transport, FSProvider, CCOpts, CompileCommandsDirPath,
   /*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs, Opts);
   constexpr int NoShutdownRequestErrorCode = 1;
   set_thread_name("clangd.main");
Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -10,6 +10,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDUNIT_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDUNIT_H
 
+#include "../clang-tidy/ClangTidyOptions.h"
 #include "Diagnostics.h"
 #include "FS.h"
 #include "Function.h"
@@ -65,6 +66,7 @@
   tooling::CompileCommand CompileCommand;
   IntrusiveRefCntPtr FS;
   std::string Contents;
+  tidy::ClangTidyOptions ClangTidyOpts;
 };
 
 /// Stores and provides access to parsed AST.
@@ -77,7 +79,8 @@
 std::shared_ptr 

r348241 - Extend test for DependentSizedArrayType

[Stephen Kelly via cfe-commits]

Author: steveire
Date: Tue Dec  4 01:53:36 2018
New Revision: 348241

URL: http://llvm.org/viewvc/llvm-project?rev=348241&view=rev
Log:
Extend test for DependentSizedArrayType

Use a using declaration to force the type to appear in the -ast-dump
output.

Modified:
cfe/trunk/test/AST/ast-dump-array.cpp

Modified: cfe/trunk/test/AST/ast-dump-array.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-array.cpp?rev=348241&r1=348240&r2=348241&view=diff
==
--- cfe/trunk/test/AST/ast-dump-array.cpp (original)
+++ cfe/trunk/test/AST/ast-dump-array.cpp Tue Dec  4 01:53:36 2018
@@ -8,3 +8,12 @@ void testArrayInitExpr()
 // CHECK: |-ArrayInitLoopExpr 0x{{[^ ]*}}  'int [10]'
 // CHECK: | `-ArrayInitIndexExpr 0x{{[^ ]*}} <> 
'unsigned long'
 }
+
+template
+class array {
+  T data[Size];
+
+  using array_T_size = T[Size];
+  // CHECK: `-DependentSizedArrayType 0x{{[^ ]*}} 'T [Size]' dependent   

+};
+


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

[PATCH] D55257: Inline handling of DependentSizedArrayType

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
Herald added a subscriber: cfe-commits.

Re-order handling of getElementType and getBracketsRange. It is
necessary to perform all printing before any traversal to child nodes.

This causes no change in the output of ast-dump-array.cpp due to the way
child nodes are printed with a delay.  This new order of the code is
also the order that produces the expected output anyway.


Repository:
  rC Clang

https://reviews.llvm.org/D55257

Files:
  lib/AST/ASTDumper.cpp


Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -299,9 +299,15 @@
   dumpStmt(T->getSizeExpr());
 }
 void VisitDependentSizedArrayType(const DependentSizedArrayType *T) {
-  VisitArrayType(T);
+  switch (T->getSizeModifier()) {
+case ArrayType::Normal: break;
+case ArrayType::Static: OS << " static"; break;
+case ArrayType::Star: OS << " *"; break;
+  }
+  OS << " " << T->getIndexTypeQualifiers().getAsString();
   OS << " ";
   dumpSourceRange(T->getBracketsRange());
+  dumpTypeAsChild(T->getElementType());
   dumpStmt(T->getSizeExpr());
 }
 void VisitDependentSizedExtVectorType(


Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -299,9 +299,15 @@
   dumpStmt(T->getSizeExpr());
 }
 void VisitDependentSizedArrayType(const DependentSizedArrayType *T) {
-  VisitArrayType(T);
+  switch (T->getSizeModifier()) {
+case ArrayType::Normal: break;
+case ArrayType::Static: OS << " static"; break;
+case ArrayType::Star: OS << " *"; break;
+  }
+  OS << " " << T->getIndexTypeQualifiers().getAsString();
   OS << " ";
   dumpSourceRange(T->getBracketsRange());
+  dumpTypeAsChild(T->getElementType());
   dumpStmt(T->getSizeExpr());
 }
 void VisitDependentSizedExtVectorType(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: test/Sema/format-strings-bitfield-promotion.c:1
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
+

aaron.ballman wrote:
> ebevhan wrote:
> > ebevhan wrote:
> > > aaron.ballman wrote:
> > > > aaron.ballman wrote:
> > > > > Running your test through GCC looks like the behavior matches here 
> > > > > for C; can you also add a C++ test that demonstrates the behavior 
> > > > > does not change?
> > > > > 
> > > > > https://godbolt.org/z/zRYDMG
> > > > Strangely, the above godbolt link dropped the output windows, here's a 
> > > > different link that shows the behavioral differences between C and C++ 
> > > > mode in GCC: https://godbolt.org/z/R3zRHe
> > > Hmm, I'll have a look at this.
> > That gcc godbolt is a bit odd. The type of the bitfield expression in the 
> > C++ example is `long` and not `int`, but in Clang, it's clearly being 
> > converted. If I change the example a bit, we get this warning:
> > ```
> > :11:12: warning: format '%d' expects argument of type 'int', but 
> > argument 2 has type 'long int' [-Wformat=]
> >11 |   printf("%d", bf.a); // expected-warning {{format specifies type 
> > 'long' but the argument has type 'int'}}
> >   |   ~^   
> >   ||  |
> >   |intlong int
> > ```
> > But in Clang, we get a cast to `int`:
> > ```
> > | `-ImplicitCastExpr 0xd190748  'int' 
> > |   `-ImplicitCastExpr 0xd190730  'long' 
> > 
> > | `-MemberExpr 0xd190618  'long' lvalue bitfield .a 
> > 0xd18f790
> > |   `-DeclRefExpr 0xd1905f8  'struct bitfields':'bitfields' 
> > lvalue Var 0xd18fa18 'bf' 'struct bitfields':'bitfields'
> > ```
> > 
> > So gcc and Clang are doing things differently here.
> > 
> > The code in `isPromotableBitField` says:
> > ```
> >   // FIXME: C does not permit promotion of a 'long : 3' bitfield to int.
> >   //We perform that promotion here to match GCC and C++.
> > ```
> > but clearly gcc isn't doing this in the C++ case. The comments also mention 
> > some things about gcc bugs that Clang does not follow, but that's in 
> > reference to a C DR.
> C++ disallows the rank conversion from int to long as well. [conv.prom]p1 
> does not apply because `long int` has a higher rank than `int`, but 
> [conv.prom]p5 allows the promotion if the range of values is identical 
> between the two types.
> 
> C makes this UB in several ways -- you can't have a bit-field whose type is 
> something other than int, unsigned int, or _Bool (6.7.2.1p5) or promoting 
> from types other than those (6.3.1.1p2), but otherwise matches the C++ 
> behavior in terms of promotion (including the rank conversion).
> 
> You may have to dig further into what Clang is doing, but I would guess that 
> the diagnostics should be triggered in both C and C++ similarly.
> 
> Ultimately, I'd like to see tests for cases where `sizeof(int) == 
> sizeof(long)`, `sizeof(int) != sizeof(long)`, and variants for C and C++ of 
> each.
I'm not sure the warning should trigger in C++; the behavior is correct there. 
The expression in those cases should be of type `long`, not `int`. The bitfield 
promotions in C++ say that values _can_ be promoted if the value fits in `int`, 
but the rules in C say that the value _is_ promoted.

The strange promotion behavior only occurs in C because of the issue with 
bitfields larger than int. It's not really permitted according to the standard, 
but it's supported anyway to match C++. Though, it ends up not matching C++ due 
to these promotion differences.

I'll add tests for the different int and long sizes, though the only case where 
it would make a difference would be if int was larger than 32 bits, which it 
isn't on any target.


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

https://reviews.llvm.org/D51211



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

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan updated this revision to Diff 176571.
ebevhan added a comment.

Added testing for C++ and different sizes of `int` and `long`.


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

https://reviews.llvm.org/D51211

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/format-strings-bitfield-promotion.c
  test/Sema/format-strings-bitfield-promotion.cxx


Index: test/Sema/format-strings-bitfield-promotion.cxx
===
--- /dev/null
+++ test/Sema/format-strings-bitfield-promotion.cxx
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -fsyntax-only -verify %s
+
+// In C++, the bitfield promotion from long to int does not occur, unlike C.
+// expected-no-diagnostics
+
+int printf(const char *restrict, ...);
+
+struct bitfields {
+  long a : 2;
+  unsigned long b : 2;
+  long c : 32;  // assumes that int is 32 bits
+  unsigned long d : 32; // assumes that int is 32 bits
+} bf;
+
+void bitfield_promotion() {
+  printf("%ld", bf.a);
+  printf("%lu", bf.b);
+  printf("%ld", bf.c);
+  printf("%lu", bf.d);
+}
Index: test/Sema/format-strings-bitfield-promotion.c
===
--- /dev/null
+++ test/Sema/format-strings-bitfield-promotion.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -fsyntax-only -verify %s
+
+int printf(const char *restrict, ...);
+
+struct bitfields {
+  long a : 2;
+  unsigned long b : 2;
+  long c : 32;  // assumes that int is 32 bits
+  unsigned long d : 32; // assumes that int is 32 bits
+} bf;
+
+void bitfield_promotion() {
+  printf("%ld", bf.a); // expected-warning {{format specifies type 'long' but 
the argument has type 'int'}}
+  printf("%lu", bf.b); // expected-warning {{format specifies type 'unsigned 
long' but the argument has type 'int'}}
+  printf("%ld", bf.c); // expected-warning {{format specifies type 'long' but 
the argument has type 'int'}}
+  printf("%lu", bf.d); // expected-warning {{format specifies type 'unsigned 
long' but the argument has type 'unsigned int'}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -7697,6 +7697,24 @@
   return std::make_pair(QualType(), StringRef());
 }
 
+/// Return true if \p ICE is an implicit argument promotion of an arithmetic
+/// type. Bit-field 'promotions' from a higher ranked type to a lower ranked
+/// type do not count.
+static bool
+isArithmeticArgumentPromotion(Sema &S, const ImplicitCastExpr *ICE) {
+  QualType From = ICE->getSubExpr()->getType();
+  QualType To = ICE->getType();
+  // It's a floating promotion if the source type is a lower rank.
+  if (ICE->getCastKind() == CK_FloatingCast &&
+  S.Context.getFloatingTypeOrder(From, To) < 0)
+return true;
+  // It's an integer promotion if the destination type is the promoted
+  // source type.
+  return ICE->getCastKind() == CK_IntegralCast &&
+ From->isPromotableIntegerType() &&
+ S.Context.getPromotedIntegerType(From) == To;
+}
+
 bool
 CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
 const char *StartSpecifier,
@@ -7724,11 +7742,11 @@
 
   // Look through argument promotions for our error message's reported type.
   // This includes the integral and floating promotions, but excludes array
-  // and function pointer decay; seeing that an argument intended to be a
-  // string has type 'char [6]' is probably more confusing than 'char *'.
+  // and function pointer decay (seeing that an argument intended to be a
+  // string has type 'char [6]' is probably more confusing than 'char *') and
+  // certain bitfield promotions (bitfields can be 'demoted' to a lesser type).
   if (const ImplicitCastExpr *ICE = dyn_cast(E)) {
-if (ICE->getCastKind() == CK_IntegralCast ||
-ICE->getCastKind() == CK_FloatingCast) {
+if (isArithmeticArgumentPromotion(S, ICE)) {
   E = ICE->getSubExpr();
   ExprTy = E->getType();
 


Index: test/Sema/format-strings-bitfield-promotion.cxx
===
--- /dev/null
+++ test/Sema/format-strings-bitfield-promotion.cxx
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -fsyntax-only -verify %s
+
+// In C++, the bitfield promotion from long to int does not occur, unlike C.
+// expected-no-diagnostics
+
+int printf(const char *restrict, ...);
+
+struct bitfields {
+  long a : 2;
+  unsigned long b : 2;
+  long c : 32;  // assumes that int is 32 bits
+  unsigned long d : 32; // assumes that int is 32 bits
+} bf;
+
+void bitfield_promotion

r348244 - [Analyzer] Iterator Checkers - Use the region of the topmost base class for iterators stored in a region

[Adam Balogh via cfe-commits]

Author: baloghadamsoftware
Date: Tue Dec  4 02:22:28 2018
New Revision: 348244

URL: http://llvm.org/viewvc/llvm-project?rev=348244&view=rev
Log:
[Analyzer] Iterator Checkers - Use the region of the topmost base class for 
iterators stored in a region

If an iterator is represented by a derived C++ class but its comparison operator
is for its base the iterator checkers cannot recognize the iterators compared.
This results in false positives in very straightforward cases (range error when
dereferencing an iterator after disclosing that it is equal to the past-the-end
iterator).

To overcome this problem we always use the region of the topmost base class for
iterators stored in a region. A new method called getMostDerivedObjectRegion()
was added to the MemRegion class to get this region.

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


Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
cfe/trunk/test/Analysis/iterator-range.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h?rev=348244&r1=348243&r2=348244&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h Tue 
Dec  4 02:22:28 2018
@@ -118,6 +118,10 @@ public:
 
   const MemRegion *getBaseRegion() const;
 
+  /// Recursively retrieve the region of the most derived class instance of
+  /// regions of C++ base class instances.
+  const MemRegion *getMostDerivedObjectRegion() const;
+
   /// Check if the region is a subregion of the given region.
   /// Each region is a subregion of itself.
   virtual bool isSubRegionOf(const MemRegion *R) const;

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp?rev=348244&r1=348243&r2=348244&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp Tue Dec  4 
02:22:28 2018
@@ -1089,9 +1089,7 @@ void IteratorChecker::verifyRandomIncrOr
 void IteratorChecker::verifyMatch(CheckerContext &C, const SVal &Iter,
   const MemRegion *Cont) const {
   // Verify match between a container and the container of an iterator
-  while (const auto *CBOR = Cont->getAs()) {
-Cont = CBOR->getSuperRegion();
-  }
+  Cont = Cont->getMostDerivedObjectRegion();
 
   auto State = C.getState();
   const auto *Pos = getIteratorPosition(State, Iter);
@@ -1125,9 +1123,7 @@ void IteratorChecker::handleBegin(Checke
   if (!ContReg)
 return;
 
-  while (const auto *CBOR = ContReg->getAs()) {
-ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   // If the container already has a begin symbol then use it. Otherwise first
   // create a new one.
@@ -1151,9 +1147,7 @@ void IteratorChecker::handleEnd(CheckerC
   if (!ContReg)
 return;
 
-  while (const auto *CBOR = ContReg->getAs()) {
-ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   // If the container already has an end symbol then use it. Otherwise first
   // create a new one.
@@ -1174,9 +1168,7 @@ void IteratorChecker::handleEnd(CheckerC
 void IteratorChecker::assignToContainer(CheckerContext &C, const Expr *CE,
 const SVal &RetVal,
 const MemRegion *Cont) const {
-  while (const auto *CBOR = Cont->getAs()) {
-Cont = CBOR->getSuperRegion();
-  }
+  Cont = Cont->getMostDerivedObjectRegion();
 
   auto State = C.getState();
   auto &SymMgr = C.getSymbolManager();
@@ -1194,9 +1186,7 @@ void IteratorChecker::handleAssign(Check
   if (!ContReg)
 return;
 
-  while (const auto *CBOR = ContReg->getAs()) {
-ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   // Assignment of a new value to a container always invalidates all its
   // iterators
@@ -1211,9 +1201,7 @@ void IteratorChecker::handleAssign(Check
   if (!OldCont.isUndef()) {
 const auto *OldContReg = OldCont.getAsRegion();
 if (OldContReg) {
-  while (const auto *CBOR = OldContReg->getAs()) {
-OldContReg = CBOR->getSuperRegion();
-  }
+  OldContReg = OldContReg->getMostDerivedObjectRegion();
   const auto OldCData = getContainerData(State, OldContReg);
   if (OldCData) {
 if (const auto OldEndSym = OldCData->getEnd()) {
@@ -1273,9 +1261,7 @@ void IteratorChecker::handleC

[PATCH] D54466: [Analyzer] Iterator Checkers - Use the region of the topmost base class for iterators stored in a region

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348244: [Analyzer] Iterator Checkers - Use the region of the 
topmost base class for… (authored by baloghadamsoftware, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54466?vs=176374&id=176575#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D54466

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
  cfe/trunk/test/Analysis/iterator-range.cpp

Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -118,6 +118,10 @@
 
   const MemRegion *getBaseRegion() const;
 
+  /// Recursively retrieve the region of the most derived class instance of
+  /// regions of C++ base class instances.
+  const MemRegion *getMostDerivedObjectRegion() const;
+
   /// Check if the region is a subregion of the given region.
   /// Each region is a subregion of itself.
   virtual bool isSubRegionOf(const MemRegion *R) const;
Index: cfe/trunk/test/Analysis/iterator-range.cpp
===
--- cfe/trunk/test/Analysis/iterator-range.cpp
+++ cfe/trunk/test/Analysis/iterator-range.cpp
@@ -200,3 +200,40 @@
   ++i0;
   *++i0; // expected-warning{{Iterator accessed outside of its range}}
 }
+
+struct simple_iterator_base {
+  simple_iterator_base();
+  simple_iterator_base(const simple_iterator_base& rhs);
+  simple_iterator_base &operator=(const simple_iterator_base& rhs);
+  virtual ~simple_iterator_base();
+  bool friend operator==(const simple_iterator_base &lhs,
+ const simple_iterator_base &rhs);
+  bool friend operator!=(const simple_iterator_base &lhs,
+ const simple_iterator_base &rhs);
+private:
+  int *ptr;
+};
+
+struct simple_derived_iterator: public simple_iterator_base {
+  int& operator*();
+  int* operator->();
+  simple_iterator_base &operator++();
+  simple_iterator_base operator++(int);
+  simple_iterator_base &operator--();
+  simple_iterator_base operator--(int);
+};
+
+struct simple_container {
+  typedef simple_derived_iterator iterator;
+
+  iterator begin();
+  iterator end();
+};
+
+void good_derived(simple_container c) {
+  auto i0 = c.end();
+  if (i0 != c.end()) {
+clang_analyzer_warnIfReached();
+*i0; // no-warning
+  }
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1175,6 +1175,15 @@
   return R;
 }
 
+// getgetMostDerivedObjectRegion gets the region of the root class of a C++
+// class hierarchy.
+const MemRegion *MemRegion::getMostDerivedObjectRegion() const {
+  const MemRegion *R = this;
+  while (const auto *BR = dyn_cast(R))
+R = BR->getSuperRegion();
+  return R;
+}
+
 bool MemRegion::isSubRegionOf(const MemRegion *) const {
   return false;
 }
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -1089,9 +1089,7 @@
 void IteratorChecker::verifyMatch(CheckerContext &C, const SVal &Iter,
   const MemRegion *Cont) const {
   // Verify match between a container and the container of an iterator
-  while (const auto *CBOR = Cont->getAs()) {
-Cont = CBOR->getSuperRegion();
-  }
+  Cont = Cont->getMostDerivedObjectRegion();
 
   auto State = C.getState();
   const auto *Pos = getIteratorPosition(State, Iter);
@@ -1125,9 +1123,7 @@
   if (!ContReg)
 return;
 
-  while (const auto *CBOR = ContReg->getAs()) {
-ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   // If the container already has a begin symbol then use it. Otherwise first
   // create a new one.
@@ -1151,9 +1147,7 @@
   if (!ContReg)
 return;
 
-  while (const auto *CBOR = ContReg->getAs()) {
-ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   // If the container already has an end symbol then use it. Otherwise first
   // create a new one.
@@ -1174,9 +1168,7 @@
 void IteratorChecker::assignToContainer(CheckerContext &C, const Expr *CE,
 const SVal &RetVal,
 const MemRegion *Cont) const {
-  while (const auto *CBOR = Cont->getAs()) {
-C

r348245 - [Analyzer] Iterator Checker - Forbid decrements past the begin() and increments past the end() of containers

[Adam Balogh via cfe-commits]

Author: baloghadamsoftware
Date: Tue Dec  4 02:27:27 2018
New Revision: 348245

URL: http://llvm.org/viewvc/llvm-project?rev=348245&view=rev
Log:
[Analyzer] Iterator Checker - Forbid decrements past the begin() and increments 
past the end() of containers

Previously, the iterator range checker only warned upon dereferencing of
iterators outside their valid range as well as increments and decrements of
out-of-range iterators where the result remains out-of-range. However, the C++
standard is more strict than this: decrementing begin() or incrementing end()
results in undefined behaviour even if the iterator is not dereferenced
afterwards. Coming back to the range once out-of-range is also undefined.

This patch corrects the behaviour of the iterator range checker: warnings are
given for any operation whose result is ahead of begin() or past the end()
(which is the past-end iterator itself, thus now we are speaking of past
past-the-end).

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


Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
cfe/trunk/test/Analysis/iterator-range.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp?rev=348245&r1=348244&r2=348245&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp Tue Dec  4 
02:27:27 2018
@@ -238,14 +238,17 @@ class IteratorChecker
   void handleEraseAfter(CheckerContext &C, const SVal &Iter) const;
   void handleEraseAfter(CheckerContext &C, const SVal &Iter1,
 const SVal &Iter2) const;
+  void verifyIncrement(CheckerContext &C, const SVal &Iter) const;
+  void verifyDecrement(CheckerContext &C, const SVal &Iter) const;
   void verifyRandomIncrOrDecr(CheckerContext &C, OverloadedOperatorKind Op,
-  const SVal &RetVal, const SVal &LHS,
-  const SVal &RHS) const;
+  const SVal &LHS, const SVal &RHS) const;
   void verifyMatch(CheckerContext &C, const SVal &Iter,
const MemRegion *Cont) const;
   void verifyMatch(CheckerContext &C, const SVal &Iter1,
const SVal &Iter2) const;
-
+  IteratorPosition advancePosition(CheckerContext &C, OverloadedOperatorKind 
Op,
+   const IteratorPosition &Pos,
+   const SVal &Distance) const;
   void reportOutOfRangeBug(const StringRef &Message, const SVal &Val,
CheckerContext &C, ExplodedNode *ErrNode) const;
   void reportMismatchedBug(const StringRef &Message, const SVal &Val1,
@@ -388,7 +391,9 @@ ProgramStateRef setContainerData(Program
 bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
 bool isBoundThroughLazyCompoundVal(const Environment &Env,
const MemRegion *Reg);
-bool isOutOfRange(ProgramStateRef State, const IteratorPosition &Pos);
+bool isPastTheEnd(ProgramStateRef State, const IteratorPosition &Pos);
+bool isAheadOfRange(ProgramStateRef State, const IteratorPosition &Pos);
+bool isBehindPastTheEnd(ProgramStateRef State, const IteratorPosition &Pos);
 bool isZero(ProgramStateRef State, const NonLoc &Val);
 } // namespace
 
@@ -422,29 +427,46 @@ void IteratorChecker::checkPreCall(const
 verifyAccess(C, Call.getArgSVal(0));
   }
 }
-if (ChecksEnabled[CK_IteratorRangeChecker] &&
-isRandomIncrOrDecrOperator(Func->getOverloadedOperator())) {
-  if (const auto *InstCall = dyn_cast(&Call)) {
-// Check for out-of-range incrementions and decrementions
-if (Call.getNumArgs() >= 1) {
-  verifyRandomIncrOrDecr(C, Func->getOverloadedOperator(),
- Call.getReturnValue(),
- InstCall->getCXXThisVal(), 
Call.getArgSVal(0));
+if (ChecksEnabled[CK_IteratorRangeChecker]) {
+  if (isIncrementOperator(Func->getOverloadedOperator())) {
+// Check for out-of-range incrementions
+if (const auto *InstCall = dyn_cast(&Call)) {
+  verifyIncrement(C, InstCall->getCXXThisVal());
+} else {
+  if (Call.getNumArgs() >= 1) {
+verifyIncrement(C, Call.getArgSVal(0));
+  }
 }
-  } else {
-if (Call.getNumArgs() >= 2) {
-  verifyRandomIncrOrDecr(C, Func->getOverloadedOperator(),
- Call.getReturnValue(), Call.getArgSVal(0),
- Call.getArgSVal(1));
+  } else if (isDecrementOperator(Func->getOverloadedOperator())) {
+// Check for out-of-range decrementions
+if (const auto *InstCall = dyn_cast(&Call)) {
+  verifyDecrement(C, InstCall->getCXXThisVal());
+   

[PATCH] D53812: [Analyzer] Iterator Checker - Forbid decrements past the begin() and increments past the end() of containers

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rC348245: [Analyzer] Iterator Checker - Forbid decrements past 
the begin() and increments… (authored by baloghadamsoftware, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53812?vs=176389&id=176576#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D53812

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/iterator-range.cpp

Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -238,14 +238,17 @@
   void handleEraseAfter(CheckerContext &C, const SVal &Iter) const;
   void handleEraseAfter(CheckerContext &C, const SVal &Iter1,
 const SVal &Iter2) const;
+  void verifyIncrement(CheckerContext &C, const SVal &Iter) const;
+  void verifyDecrement(CheckerContext &C, const SVal &Iter) const;
   void verifyRandomIncrOrDecr(CheckerContext &C, OverloadedOperatorKind Op,
-  const SVal &RetVal, const SVal &LHS,
-  const SVal &RHS) const;
+  const SVal &LHS, const SVal &RHS) const;
   void verifyMatch(CheckerContext &C, const SVal &Iter,
const MemRegion *Cont) const;
   void verifyMatch(CheckerContext &C, const SVal &Iter1,
const SVal &Iter2) const;
-
+  IteratorPosition advancePosition(CheckerContext &C, OverloadedOperatorKind Op,
+   const IteratorPosition &Pos,
+   const SVal &Distance) const;
   void reportOutOfRangeBug(const StringRef &Message, const SVal &Val,
CheckerContext &C, ExplodedNode *ErrNode) const;
   void reportMismatchedBug(const StringRef &Message, const SVal &Val1,
@@ -388,7 +391,9 @@
 bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
 bool isBoundThroughLazyCompoundVal(const Environment &Env,
const MemRegion *Reg);
-bool isOutOfRange(ProgramStateRef State, const IteratorPosition &Pos);
+bool isPastTheEnd(ProgramStateRef State, const IteratorPosition &Pos);
+bool isAheadOfRange(ProgramStateRef State, const IteratorPosition &Pos);
+bool isBehindPastTheEnd(ProgramStateRef State, const IteratorPosition &Pos);
 bool isZero(ProgramStateRef State, const NonLoc &Val);
 } // namespace
 
@@ -422,29 +427,46 @@
 verifyAccess(C, Call.getArgSVal(0));
   }
 }
-if (ChecksEnabled[CK_IteratorRangeChecker] &&
-isRandomIncrOrDecrOperator(Func->getOverloadedOperator())) {
-  if (const auto *InstCall = dyn_cast(&Call)) {
-// Check for out-of-range incrementions and decrementions
-if (Call.getNumArgs() >= 1) {
-  verifyRandomIncrOrDecr(C, Func->getOverloadedOperator(),
- Call.getReturnValue(),
- InstCall->getCXXThisVal(), Call.getArgSVal(0));
+if (ChecksEnabled[CK_IteratorRangeChecker]) {
+  if (isIncrementOperator(Func->getOverloadedOperator())) {
+// Check for out-of-range incrementions
+if (const auto *InstCall = dyn_cast(&Call)) {
+  verifyIncrement(C, InstCall->getCXXThisVal());
+} else {
+  if (Call.getNumArgs() >= 1) {
+verifyIncrement(C, Call.getArgSVal(0));
+  }
 }
-  } else {
-if (Call.getNumArgs() >= 2) {
-  verifyRandomIncrOrDecr(C, Func->getOverloadedOperator(),
- Call.getReturnValue(), Call.getArgSVal(0),
- Call.getArgSVal(1));
+  } else if (isDecrementOperator(Func->getOverloadedOperator())) {
+// Check for out-of-range decrementions
+if (const auto *InstCall = dyn_cast(&Call)) {
+  verifyDecrement(C, InstCall->getCXXThisVal());
+} else {
+  if (Call.getNumArgs() >= 1) {
+verifyDecrement(C, Call.getArgSVal(0));
+  }
+}
+  } else if (isRandomIncrOrDecrOperator(Func->getOverloadedOperator())) {
+if (const auto *InstCall = dyn_cast(&Call)) {
+  // Check for out-of-range incrementions and decrementions
+  if (Call.getNumArgs() >= 1) {
+verifyRandomIncrOrDecr(C, Func->getOverloadedOperator(),
+   InstCall->getCXXThisVal(),
+   Call.getArgSVal(0));
+  }
+} else {
+  if (Call.getNumArgs() >= 2) {
+verifyRandomIncrOrDecr(C, Func->getOverloadedOperator(),
+   Call.getArgSVal(0), Call.getArgSVal(1));
+  }
+}
+  } else if (isDereferenceOperator(Func->getOverloadedOperator())) {
+// Check for dereference of out-

[PATCH] D55260: [CodeComplete] Fix a crash in access checks of inner classes

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: kadircet.

The crash was introduced in r348135.


Repository:
  rC Clang

https://reviews.llvm.org/D55260

Files:
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/accessibility.cpp


Index: test/CodeCompletion/accessibility.cpp
===
--- test/CodeCompletion/accessibility.cpp
+++ test/CodeCompletion/accessibility.cpp
@@ -71,3 +71,27 @@
 // RUN: | FileCheck -check-prefix=UNRELATED %s
   }
 };
+
+class Outer {
+ public:
+  static int pub;
+ protected:
+  static int prot;
+ private:
+  static int priv;
+
+  class Inner {
+int test() {
+  Outer::pub = 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:85:14 %s -o - \
+// RUN: | FileCheck -check-prefix=OUTER %s
+// OUTER: priv : [#int#]priv
+// OUTER: prot : [#int#]prot
+// OUTER: pub : [#int#]pub
+
+// Also check the unqualified case.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:85:1 %s -o - \
+// RUN: | FileCheck -check-prefix=OUTER %s
+}
+  };
+};
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -11,6 +11,7 @@
 //
 
//===--===//
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/ExprCXX.h"
@@ -1313,23 +1314,39 @@
 
   void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, DeclContext *Ctx,
  bool InBaseClass) override {
-// Naming class to use for access check. In most cases it was provided
-// explicitly (e.g. member access (lhs.foo) or qualified lookup (X::)), for
-// unqualified lookup we fallback to the \p Ctx in which we found the
-// member.
-auto *NamingClass = this->NamingClass;
-if (!NamingClass)
-  NamingClass = llvm::dyn_cast_or_null(Ctx);
-bool Accessible =
-Results.getSema().IsSimplyAccessible(ND, NamingClass, BaseType);
 ResultBuilder::Result Result(ND, Results.getBasePriority(ND), nullptr,
- false, Accessible, FixIts);
+ false, IsAccessible(ND, Ctx), FixIts);
 Results.AddResult(Result, InitialLookupCtx, Hiding, InBaseClass);
   }
 
   void EnteredContext(DeclContext *Ctx) override {
 Results.addVisitedContext(Ctx);
   }
+
+  private:
+  bool IsAccessible(NamedDecl *ND, DeclContext *Ctx) {
+auto *Cls = llvm::dyn_cast_or_null(Ctx);
+if (!Cls)
+  return true; // Only class members require access checking.
+
+// Naming class to use for access check. In most cases it was provided
+// explicitly (e.g. member access (lhs.foo) or qualified lookup (X::)),
+// for unqualified lookup we fallback to the \p Ctx in which we found the
+// member.
+auto *NamingClass = this->NamingClass;
+QualType BaseType = this->BaseType;
+if (!NamingClass)
+  NamingClass = Cls;
+// When we emulate implicit 'this->' in an unqualified lookup, we might end
+// up with an invalid naming class. In that case, we avoid emulating
+// 'this->' qualifier to satisfy preconditions of the access checking.
+if (NamingClass->getCanonicalDecl() != Cls->getCanonicalDecl() &&
+!NamingClass->isDerivedFrom(Cls)) {
+  NamingClass = Cls;
+  BaseType = QualType();
+}
+return Results.getSema().IsSimplyAccessible(ND, Cls, BaseType);
+  }
 };
 } // namespace
 


Index: test/CodeCompletion/accessibility.cpp
===
--- test/CodeCompletion/accessibility.cpp
+++ test/CodeCompletion/accessibility.cpp
@@ -71,3 +71,27 @@
 // RUN: | FileCheck -check-prefix=UNRELATED %s
   }
 };
+
+class Outer {
+ public:
+  static int pub;
+ protected:
+  static int prot;
+ private:
+  static int priv;
+
+  class Inner {
+int test() {
+  Outer::pub = 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:85:14 %s -o - \
+// RUN: | FileCheck -check-prefix=OUTER %s
+// OUTER: priv : [#int#]priv
+// OUTER: prot : [#int#]prot
+// OUTER: pub : [#int#]pub
+
+// Also check the unqualified case.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:85:1 %s -o - \
+// RUN: | FileCheck -check-prefix=OUTER %s
+}
+  };
+};
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -11,6 +11,7 @@
 //
 //===--===//
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/ExprCXX.h"
@@ -1313,23 +1314,39 @@
 
   void FoundDecl(NamedDecl *ND, NamedDecl *Hiding,

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176581.
martong marked 4 inline comments as done.
martong added a comment.

- Rename AnalyzerDisplayCtuProgress to Opts.AnalyzerDisplayCTUProgress
- Change the CTU progress message


Repository:
  rC Clang

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

https://reviews.llvm.org/D55135

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  include/clang/Driver/CC1Options.td
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/ctu-main.cpp

Index: test/Analysis/ctu-main.cpp
===
--- test/Analysis/ctu-main.cpp
+++ test/Analysis/ctu-main.cpp
@@ -3,6 +3,10 @@
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o %T/ctudir/ctu-chain.cpp.ast %S/Inputs/ctu-chain.cpp
 // 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
+// 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 -analyzer-display-ctu-progress 2>&1 %s | FileCheck %s
+
+// CHECK: ANALYZE (CTU loaded AST for source file): {{.*}}/ctu-other.cpp
+// CHECK: ANALYZE (CTU loaded AST for source file): {{.*}}/ctu-chain.cpp
 
 #include "ctu-hdr.h"
 
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -560,7 +560,8 @@
   cross_tu::CrossTranslationUnitContext &CTUCtx =
   *Engine->getCrossTranslationUnitContext();
   llvm::Expected CTUDeclOrError =
-  CTUCtx.getCrossTUDefinition(FD, Opts.getCTUDir(), Opts.getCTUIndexName());
+  CTUCtx.getCrossTUDefinition(FD, Opts.getCTUDir(), Opts.getCTUIndexName(),
+  Opts.AnalyzerDisplayCTUProgress);
 
   if (!CTUDeclOrError) {
 handleAllErrors(CTUDeclOrError.takeError(),
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -289,6 +289,8 @@
   Opts.NoRetryExhausted = Args.hasArg(OPT_analyzer_disable_retry_exhausted);
   Opts.AnalyzeAll = Args.hasArg(OPT_analyzer_opt_analyze_headers);
   Opts.AnalyzerDisplayProgress = Args.hasArg(OPT_analyzer_display_progress);
+  Opts.AnalyzerDisplayCTUProgress =
+  Args.hasArg(OPT_analyzer_display_ctu_progress);
   Opts.AnalyzeNestedBlocks =
 Args.hasArg(OPT_analyzer_opt_analyze_nested_blocks);
   Opts.AnalyzeSpecificFunction = Args.getLastArgValue(OPT_analyze_function);
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -149,14 +149,15 @@
 llvm::Expected
 CrossTranslationUnitContext::getCrossTUDefinition(const FunctionDecl *FD,
   StringRef CrossTUDir,
-  StringRef IndexName) {
+  StringRef IndexName,
+  bool DisplayCTUProgress) {
   assert(!FD->hasBody() && "FD has a definition in current translation unit!");
   const std::string LookupFnName = getLookupName(FD);
   if (LookupFnName.empty())
 return llvm::make_error(
 index_error_code::failed_to_generate_usr);
   llvm::Expected ASTUnitOrError =
-  loadExternalAST(LookupFnName, CrossTUDir, IndexName);
+  loadExternalAST(LookupFnName, CrossTUDir, IndexName, DisplayCTUProgress);
   if (!ASTUnitOrError)
 return ASTUnitOrError.takeError();
   ASTUnit *Unit = *ASTUnitOrError;
@@ -193,7 +194,8 @@
 }
 
 llvm::Expected CrossTranslationUnitContext::loadExternalAST(
-StringRef LookupName, StringRef CrossTUDir, StringRef IndexName) {
+StringRef LookupName, StringRef CrossTUDir, StringRef IndexName,
+bool DisplayCTUProgress) {
   // FIXME: The current implementation only supports loading functions with
   //a lookup name from a single translation unit. If multiple
   //translation units contains functions with the same lookup name an
@@ -233,6 +235,10 @@
   ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts()));
   Unit = LoadedUnit.get();
   FileASTUnitMap[ASTFileName] = std::move(LoadedUnit);
+  if (DisplayCTUProgress) {
+llvm::errs() << "ANALYZE CTU loaded AST file: "
+ << ASTFileName << "\n";
+  }
 } else {
   Unit = ASTCacheEntry->second.get();

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> Also, almost everywhere CTU is capitalized, so I guess it should be in the 
> field name too.

Ok, I renamed it to have CTU all capitalized.




Comment at: lib/CrossTU/CrossTranslationUnit.cpp:239
+  if (DisplayCTUProgress) {
+llvm::errs() << "ANALYZE (CTU loaded AST for source file): "
+ << ASTFileName << "\n";

a_sidorin wrote:
> I think we can remove parens from the message.
Ok, I removed the parens. And also changed the text from `CTU loaded AST for 
source file` to `CTU loaded AST file` since we dump the path to the AST, not 
the path to the source file from which we generated the AST file.



Comment at: test/Analysis/ctu-main.cpp:6
 // 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
+// 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 
-analyzer-display-ctu-progress 2>&1 %s | FileCheck %s
+

Szelethus wrote:
> I think these RUN lines would really benefit from introducing line breaks.
Yes, I agree. Unfortunately, I could not figure out how to break them. Using a 
simple "\" at around the 80th column gives `Test has unterminated run lines 
(with '\')`. If I use block comments with "\" the same happens. If I use block 
comments and don't use the "\" then the second line is not interpreted. Is it 
really possible to break RUN lines? I could not find anything about that in the 
online docs.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55135



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

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

[Ulrich Weigand via Phabricator via cfe-commits]

uweigand added inline comments.



Comment at: lib/Headers/__stddef_max_align_t.h:40
   __attribute__((__aligned__(__alignof__(long double;
+#ifdef __i386__
+  __float128 __clang_max_align_nonce3

jyknight wrote:
> EricWF wrote:
> > uweigand wrote:
> > > jyknight wrote:
> > > > uweigand wrote:
> > > > > jyknight wrote:
> > > > > > Can you fix clang to consistently define `__SIZEOF_FLOAT128__` in 
> > > > > > InitPreprocessor alongside the rest of the SIZEOF macros?
> > > > > > 
> > > > > > And then use that to determine whether to add float128 to the 
> > > > > > union? This change, as is, will break on any target which is i386 
> > > > > > but doesn't define __float128, e.g. i386-unknown-unknown.
> > > > > > 
> > > > > > The only additional target which will be modified with that (that 
> > > > > > is: the only other target which has a float128 type, but for which 
> > > > > > max_align isn't already 16) is systemz-*-linux.
> > > > > > 
> > > > > > But I think that's actually indicating a pre-existing bug in the 
> > > > > > SystemZ config -- it's configured for a 16-byte long double, with 
> > > > > > 8-byte alignment, but the ABI doc seems to call for 16-byte 
> > > > > > alignment. +Ulrich for comment on that.
> > > > > That's a bug in the ABI doc which we'll fix once we get around to 
> > > > > releasing an updated version.
> > > > > 
> > > > > long double on SystemZ must be 8-byte aligned, which is the maximum 
> > > > > alignment of all standard types on Z, and this was chosen because 
> > > > > historically the ABI only guarantees an 8-byte stack alignment, so 
> > > > > 16-byte aligned standard types would be awkward.
> > > > Then perhaps it's a bug that `__alignof__(__float128)` returns 16 with 
> > > > `-target systemz-linux`?
> > > Huh, really __float128 should not be supported at all on SystemZ.  It's 
> > > not supported with GCC either (GCC never provides __float128 on targets 
> > > where long double is already IEEE-128).  (GCC does support the new 
> > > _Float128 on SystemZ, but that's not yet supported by clang anywhere.)
> > > 
> > > If it were supported, I agree it should be an alias for long double, and 
> > > also have an alignof of 8.
> > @jyknight Ack. I'll add `__SIZEOF_FLOAT128__`.
> @uweigand : One of those fixes needs to land before this, so that systemz's 
> max_align_t doesn't change to 16 in the meantime. I think your preference 
> would be for it to be simply removed, right? Looks like the type was 
> originally added in https://reviews.llvm.org/D19125 -- possibly in error, 
> since the focus was x86_64.
@jyknight : Yes, this seems to have been simply an error.  I'll check in a 
patch to remove `__float128` on SystemZ.



Repository:
  rC Clang

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

https://reviews.llvm.org/D55057



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

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176582.
martong added a comment.

- Change the CTU progress message in the test too


Repository:
  rC Clang

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

https://reviews.llvm.org/D55135

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  include/clang/Driver/CC1Options.td
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/ctu-main.cpp

Index: test/Analysis/ctu-main.cpp
===
--- test/Analysis/ctu-main.cpp
+++ test/Analysis/ctu-main.cpp
@@ -3,6 +3,10 @@
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o %T/ctudir/ctu-chain.cpp.ast %S/Inputs/ctu-chain.cpp
 // 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
+// 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 -analyzer-display-ctu-progress 2>&1 %s | FileCheck %s
+
+// CHECK: ANALYZE CTU loaded AST file: {{.*}}/ctu-other.cpp
+// CHECK: ANALYZE CTU loaded AST file: {{.*}}/ctu-chain.cpp
 
 #include "ctu-hdr.h"
 
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -560,7 +560,8 @@
   cross_tu::CrossTranslationUnitContext &CTUCtx =
   *Engine->getCrossTranslationUnitContext();
   llvm::Expected CTUDeclOrError =
-  CTUCtx.getCrossTUDefinition(FD, Opts.getCTUDir(), Opts.getCTUIndexName());
+  CTUCtx.getCrossTUDefinition(FD, Opts.getCTUDir(), Opts.getCTUIndexName(),
+  Opts.AnalyzerDisplayCTUProgress);
 
   if (!CTUDeclOrError) {
 handleAllErrors(CTUDeclOrError.takeError(),
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -289,6 +289,8 @@
   Opts.NoRetryExhausted = Args.hasArg(OPT_analyzer_disable_retry_exhausted);
   Opts.AnalyzeAll = Args.hasArg(OPT_analyzer_opt_analyze_headers);
   Opts.AnalyzerDisplayProgress = Args.hasArg(OPT_analyzer_display_progress);
+  Opts.AnalyzerDisplayCTUProgress =
+  Args.hasArg(OPT_analyzer_display_ctu_progress);
   Opts.AnalyzeNestedBlocks =
 Args.hasArg(OPT_analyzer_opt_analyze_nested_blocks);
   Opts.AnalyzeSpecificFunction = Args.getLastArgValue(OPT_analyze_function);
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -149,14 +149,15 @@
 llvm::Expected
 CrossTranslationUnitContext::getCrossTUDefinition(const FunctionDecl *FD,
   StringRef CrossTUDir,
-  StringRef IndexName) {
+  StringRef IndexName,
+  bool DisplayCTUProgress) {
   assert(!FD->hasBody() && "FD has a definition in current translation unit!");
   const std::string LookupFnName = getLookupName(FD);
   if (LookupFnName.empty())
 return llvm::make_error(
 index_error_code::failed_to_generate_usr);
   llvm::Expected ASTUnitOrError =
-  loadExternalAST(LookupFnName, CrossTUDir, IndexName);
+  loadExternalAST(LookupFnName, CrossTUDir, IndexName, DisplayCTUProgress);
   if (!ASTUnitOrError)
 return ASTUnitOrError.takeError();
   ASTUnit *Unit = *ASTUnitOrError;
@@ -193,7 +194,8 @@
 }
 
 llvm::Expected CrossTranslationUnitContext::loadExternalAST(
-StringRef LookupName, StringRef CrossTUDir, StringRef IndexName) {
+StringRef LookupName, StringRef CrossTUDir, StringRef IndexName,
+bool DisplayCTUProgress) {
   // FIXME: The current implementation only supports loading functions with
   //a lookup name from a single translation unit. If multiple
   //translation units contains functions with the same lookup name an
@@ -233,6 +235,10 @@
   ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts()));
   Unit = LoadedUnit.get();
   FileASTUnitMap[ASTFileName] = std::move(LoadedUnit);
+  if (DisplayCTUProgress) {
+llvm::errs() << "ANALYZE CTU loaded AST file: "
+ << ASTFileName << "\n";
+  }
 } else {
   Unit = ASTCacheEntry->second.get();
 }
Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
=

r348247 - [SystemZ] Do not support __float128

[Ulrich Weigand via cfe-commits]

Author: uweigand
Date: Tue Dec  4 02:51:36 2018
New Revision: 348247

URL: http://llvm.org/viewvc/llvm-project?rev=348247&view=rev
Log:
[SystemZ] Do not support __float128

As of rev. 268898, clang supports __float128 on SystemZ.  This seems to
have been in error.  GCC has never supported __float128 on SystemZ,
since the "long double" type on the platform is already IEEE-128. (GCC
only supports __float128 on platforms where "long double" is some other
data type.)

For compatibility reasons this patch removes __float128 on SystemZ
again.  The test case is updated accordingly.


Modified:
cfe/trunk/lib/Basic/Targets/OSTargets.h
cfe/trunk/test/CodeGenCXX/float128-declarations.cpp

Modified: cfe/trunk/lib/Basic/Targets/OSTargets.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/OSTargets.h?rev=348247&r1=348246&r2=348247&view=diff
==
--- cfe/trunk/lib/Basic/Targets/OSTargets.h (original)
+++ cfe/trunk/lib/Basic/Targets/OSTargets.h Tue Dec  4 02:51:36 2018
@@ -364,7 +364,6 @@ public:
   break;
 case llvm::Triple::x86:
 case llvm::Triple::x86_64:
-case llvm::Triple::systemz:
   this->HasFloat128 = true;
   break;
 }

Modified: cfe/trunk/test/CodeGenCXX/float128-declarations.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/float128-declarations.cpp?rev=348247&r1=348246&r2=348247&view=diff
==
--- cfe/trunk/test/CodeGenCXX/float128-declarations.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/float128-declarations.cpp Tue Dec  4 02:51:36 2018
@@ -6,8 +6,6 @@
 // RUN:   %s -o - | FileCheck %s -check-prefix=CHECK-X86
 // RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-linux-gnu -std=c++11 \
 // RUN:   %s -o - | FileCheck %s -check-prefix=CHECK-X86
-// RUN: %clang_cc1 -emit-llvm -triple systemz-unknown-linux-gnu -std=c++11 \
-// RUN:   %s -o - | FileCheck %s -check-prefix=CHECK-SYSZ
 // RUN: %clang_cc1 -emit-llvm -triple i686-pc-openbsd -std=c++11 \
 // RUN:   %s -o - | FileCheck %s -check-prefix=CHECK-X86
 // RUN: %clang_cc1 -emit-llvm -triple amd64-pc-openbsd -std=c++11 \
@@ -18,8 +16,7 @@
 // RUN:   %s -o - | FileCheck %s -check-prefix=CHECK-X86
 //
 /*  Various contexts where type __float128 can appear. The different check
-prefixes are due to different mangling on X86 and different calling
-convention on SystemZ. */
+prefixes are due to different mangling on X86.  */
 
 /*  Namespace */
 namespace {
@@ -122,25 +119,3 @@ int main(void) {
 // CHECK-X86-DAG: [[F4L:%[a-z0-9]+]] = load fp128, fp128* %f4l
 // CHECK-X86-DAG: [[INC:%[a-z0-9]+]] = fadd fp128 [[F4L]], 
0xL3FFF
 // CHECK-X86-DAG: store fp128 [[INC]], fp128* %f4l
-
-// CHECK-SYSZ-DAG: @_ZN12_GLOBAL__N_13f1nE = internal global fp128 
0xL
-// CHECK-SYSZ-DAG: @_ZN12_GLOBAL__N_13f2nE = internal global fp128 
0xL40040800
-// CHECK-SYSZ-DAG: @_ZN12_GLOBAL__N_15arr1nE = internal global [10 x fp128]
-// CHECK-SYSZ-DAG: @_ZN12_GLOBAL__N_15arr2nE = internal global [3 x fp128] 
[fp128 0xL3FFF, fp128 
0xL40008000, fp128 0xL4025176592E0]
-// CHECK-SYSZ-DAG: define internal void 
@_ZN12_GLOBAL__N_16func1nERKU10__float128(fp128*
-// CHECK-SYSZ-DAG: @f1f = global fp128 0xL
-// CHECK-SYSZ-DAG: @f2f = global fp128 0xL40040333
-// CHECK-SYSZ-DAG: @arr1f = global [10 x fp128]
-// CHECK-SYSZ-DAG: @arr2f = global [3 x fp128] [fp128 
0xLBFFF, fp128 0xLC0008000, 
fp128 0xLC025176592E0]
-// CHECK-SYSZ-DAG: declare void @_Z6func1fU10__float128(fp128*
-// CHECK-SYSZ-DAG: define linkonce_odr void @_ZN2C1C2EU10__float128(%class.C1* 
%this, fp128*
-// CHECK-SYSZ-DAG: define linkonce_odr void @_ZN2C16func2cEU10__float128(fp128*
-// CHECK-SYSZ-DAG: define linkonce_odr void 
@_Z6func1tIU10__float128ET_S0_(fp128*
-// CHECK-SYSZ-DAG: @__const.main.s1 = private unnamed_addr constant %struct.S1 
{ fp128 0xL40060800 }
-// CHECK-SYSZ-DAG: store fp128 0xLF0AFD0EBFF292DCE42E0B38CDD83F26F, fp128* 
%f1l, align 16
-// CHECK-SYSZ-DAG: store fp128 0xL8000, fp128* 
%f2l, align 16
-// CHECK-SYSZ-DAG: store fp128 0xL7FFE, fp128* 
%f3l, align 16
-// CHECK-SYSZ-DAG: store fp128 0xLBFFF, fp128* 
%f5l, align 16
-// CHECK-SYSZ-DAG: [[F4L:%[a-z0-9]+]] = load fp128, fp128* %f4l
-// CHECK-SYSZ-DAG: [[INC:%[a-z0-9]+]] = fadd fp128 [[F4L]], 
0xL3FFF
-// CHECK-SYSZ-DAG: store fp128 [[INC]], fp128* %f4l


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

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

[Ulrich Weigand via Phabricator via cfe-commits]

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



Comment at: lib/Headers/__stddef_max_align_t.h:40
   __attribute__((__aligned__(__alignof__(long double;
+#ifdef __i386__
+  __float128 __clang_max_align_nonce3

uweigand wrote:
> jyknight wrote:
> > EricWF wrote:
> > > uweigand wrote:
> > > > jyknight wrote:
> > > > > uweigand wrote:
> > > > > > jyknight wrote:
> > > > > > > Can you fix clang to consistently define `__SIZEOF_FLOAT128__` in 
> > > > > > > InitPreprocessor alongside the rest of the SIZEOF macros?
> > > > > > > 
> > > > > > > And then use that to determine whether to add float128 to the 
> > > > > > > union? This change, as is, will break on any target which is i386 
> > > > > > > but doesn't define __float128, e.g. i386-unknown-unknown.
> > > > > > > 
> > > > > > > The only additional target which will be modified with that (that 
> > > > > > > is: the only other target which has a float128 type, but for 
> > > > > > > which max_align isn't already 16) is systemz-*-linux.
> > > > > > > 
> > > > > > > But I think that's actually indicating a pre-existing bug in the 
> > > > > > > SystemZ config -- it's configured for a 16-byte long double, with 
> > > > > > > 8-byte alignment, but the ABI doc seems to call for 16-byte 
> > > > > > > alignment. +Ulrich for comment on that.
> > > > > > That's a bug in the ABI doc which we'll fix once we get around to 
> > > > > > releasing an updated version.
> > > > > > 
> > > > > > long double on SystemZ must be 8-byte aligned, which is the maximum 
> > > > > > alignment of all standard types on Z, and this was chosen because 
> > > > > > historically the ABI only guarantees an 8-byte stack alignment, so 
> > > > > > 16-byte aligned standard types would be awkward.
> > > > > Then perhaps it's a bug that `__alignof__(__float128)` returns 16 
> > > > > with `-target systemz-linux`?
> > > > Huh, really __float128 should not be supported at all on SystemZ.  It's 
> > > > not supported with GCC either (GCC never provides __float128 on targets 
> > > > where long double is already IEEE-128).  (GCC does support the new 
> > > > _Float128 on SystemZ, but that's not yet supported by clang anywhere.)
> > > > 
> > > > If it were supported, I agree it should be an alias for long double, 
> > > > and also have an alignof of 8.
> > > @jyknight Ack. I'll add `__SIZEOF_FLOAT128__`.
> > @uweigand : One of those fixes needs to land before this, so that systemz's 
> > max_align_t doesn't change to 16 in the meantime. I think your preference 
> > would be for it to be simply removed, right? Looks like the type was 
> > originally added in https://reviews.llvm.org/D19125 -- possibly in error, 
> > since the focus was x86_64.
> @jyknight : Yes, this seems to have been simply an error.  I'll check in a 
> patch to remove `__float128` on SystemZ.
> 
Checked in as rev. 348247.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55057



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

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

[Ulrich Weigand via Phabricator via cfe-commits]

uweigand added a comment.

As an aside, it would be nice if we had a test case that verifies the explicit 
values of alignof(max_align_t) on all supported platforms.  This is an ABI 
property that should never change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55057



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176584.
martong added a comment.

- Break long RUN lines


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/ctu-different-triples.cpp
  test/Analysis/ctu-unknown-parts-in-triples.cpp

Index: test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -0,0 +1,21 @@
+// We do not expect any error when one part of the triple is unknown, but other
+// known parts are equal.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -verify %s
+
+// expected-no-diagnostics
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: test/Analysis/ctu-different-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-different-triples.cpp
@@ -0,0 +1,19 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple powerpc64-montavista-linux-gnu -fsyntax-only \
+// RUN:   -analyze -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -verify %s
+
+// We expect an error in this file, but without a location.
+// expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -32,6 +32,36 @@
 namespace cross_tu {
 
 namespace {
+
+// Same as Triple's equality operator, but we check a field only if that is
+// known in both instances.
+bool hasEqualKnownFields(const llvm::Triple &Lhs, const llvm::Triple &Rhs) {
+  using llvm::Triple;
+  if (Lhs.getArch() != Triple::UnknownArch &&
+  Rhs.getArch() != Triple::UnknownArch && Lhs.getArch() != Rhs.getArch())
+return false;
+  if (Lhs.getSubArch() != Triple::NoSubArch &&
+  Rhs.getSubArch() != Triple::NoSubArch &&
+  Lhs.getSubArch() != Rhs.getSubArch())
+return false;
+  if (Lhs.getVendor() != Triple::UnknownVendor &&
+  Rhs.getVendor() != Triple::UnknownVendor &&
+  Lhs.getVendor() != Rhs.getVendor())
+return false;
+  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() &&
+  Lhs.getOS() != Rhs.getOS())
+return false;
+  if (Lhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Rhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Lhs.getEnvironment() != Rhs.getEnvironment())
+return false;
+  if (Lhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Rhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Lhs.getObjectFormat() != Rhs.getObjectFormat())
+return false;
+  return true;
+}
+
 // FIXME: This class is will be removed after the transition to llvm::Error.
 class IndexErrorCategory : public std::error_category {
 public:
@@ -55,6 +85,10 @@
   return "Failed to load external AST source.";
 case index_error_code::failed_to_generate_usr:
   return "Failed to generate USR.";
+case index_error_code::triple_mismatch:
+  return "Triple mismatch";
+case index_error_code::lang_mismatch:
+  return "Language mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -166,6 +200,30 @@
   assert(&Unit->getFileManager() ==
  &Unit->getASTContext().getSourceManager().getFileManager());
 
+  const auto &TripleTo = Context.getTargetInfo().getTriple();
+  const auto &TripleFrom = Unit->getASTContext().getTargetInfo().getTriple();
+  // The imported AST had been generated for a different target.
+  // Some parts of the triple in the loaded ASTContext can be unknown while the
+  // very same parts in the target ASTContext are known. Thus we check for the
+  // known parts only.
+  if (!hasEqualKnownFields(TripleTo, TripleFrom)) {
+// TODO: Pass the Sou

[PATCH] D54796: [clangd] C++ API for emitting file status

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 176588.
hokein marked 6 inline comments as done.
hokein added a comment.

Address review comments

- remove Unknown enum type
- make TUState only accessed by the worker thread.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54796

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  unittests/clangd/TUSchedulerTests.cpp

Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -7,12 +7,13 @@
 //
 //===--===//
 
+#include "Annotations.h"
 #include "Context.h"
 #include "Matchers.h"
 #include "TUScheduler.h"
 #include "TestFS.h"
-#include "llvm/ADT/ScopeExit.h"
 #include "gmock/gmock.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "gtest/gtest.h"
 #include 
 #include 
@@ -23,6 +24,7 @@
 namespace {
 
 using ::testing::_;
+using ::testing::AllOf;
 using ::testing::AnyOf;
 using ::testing::Each;
 using ::testing::ElementsAre;
@@ -30,6 +32,10 @@
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
+MATCHER_P2(TUState, State, ActionName, "") {
+  return arg.Action.S == State && arg.Action.Name == ActionName;
+}
+
 class TUSchedulerTests : public ::testing::Test {
 protected:
   ParseInputs getInputs(PathRef File, std::string Contents) {
@@ -658,6 +664,78 @@
   EXPECT_EQ(Counter.load(), 3);
 }
 
+TEST_F(TUSchedulerTests, TUStatus) {
+  class CaptureTUStatus : public DiagnosticsConsumer {
+  public:
+void onDiagnosticsReady(PathRef File,
+std::vector Diagnostics) override {}
+
+void onFileUpdated(PathRef File, const TUStatus &Status) override {
+  std::lock_guard Lock(Mutex);
+  AllStatus.push_back(Status);
+}
+
+std::vector AllStatus;
+
+  private:
+std::mutex Mutex;
+  } CaptureTUStatus;
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  ClangdServer Server(CDB, FS, CaptureTUStatus, ClangdServer::optsForTest());
+  Annotations Code("int m^ain () {}");
+
+  // We schedule the following tasks in the queue:
+  //   [Update] [GoToDefinition]
+  Server.addDocument(testPath("foo.cpp"), Code.code(), WantDiagnostics::Yes);
+  Server.findDefinitions(testPath("foo.cpp"), Code.point(),
+ [](Expected> Result) {
+   ASSERT_TRUE((bool)Result);
+ });
+
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
+
+  EXPECT_THAT(CaptureTUStatus.AllStatus,
+  ElementsAre(
+  // Statuses of "Update" action.
+  TUState(TUAction::Queued, "Update"),
+  TUState(TUAction::RunningAction, "Update"),
+  TUState(TUAction::BuildingPreamble, "Update"),
+  TUState(TUAction::BuildingFile, "Update"),
+
+  // Statuses of "Definitions" action
+  TUState(TUAction::Queued, "Definitions"),
+  TUState(TUAction::RunningAction, "Definitions"),
+  TUState(TUAction::Idle, /*No action*/ "")));
+}
+
+TEST_F(TUSchedulerTests, NoTUStatusEmittedForRemovedFile) {
+  class CaptureTUStatus : public DiagnosticsConsumer {
+  public:
+void onDiagnosticsReady(PathRef File,
+std::vector Diagnostics) override {}
+
+void onFileUpdated(PathRef File, const TUStatus &Status) override {
+  // Queued is emitted by the main thread, we don't block it.
+  if (Status.Action.S == TUAction::Queued)
+return;
+  // Block the worker thread until the document is removed.
+  ASSERT_TRUE(Status.Action.S == TUAction::RunningAction);
+  Removed.wait();
+}
+Notification Removed;
+  } CaptureTUStatus;
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  ClangdServer Server(CDB, FS, CaptureTUStatus, ClangdServer::optsForTest());
+
+  Server.addDocument(testPath("foo.cpp"), "int main() {}",
+ WantDiagnostics::Yes);
+  Server.removeDocument(testPath("foo.cpp"));
+  CaptureTUStatus.Removed.notify();
+  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for finishing";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -52,6 +52,36 @@
   unsigned MaxRetainedASTs = 3;
 };
 
+struct TUAction {
+  enum State {
+Queued,   // The TU is pending in the thread task queue to be built.
+RunningAction,// Starting running actions on the TU.
+BuildingPreamble, // The preamble of the TU is being built.
+BuildingFile, // The TU is being built. It is only emitted when building
+  // the AST for diagnostics in write action (update).

[PATCH] D54796: [clangd] C++ API for emitting file status

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clangd/TUScheduler.cpp:268
+  /// Status of the TU.
+  TUStatus Status; /* GUARDED_BY(DiagMu) */
 };

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > Is `Status` actually ever read from multiple threads?
> > > I assume it's only accessed on the worker thread, right? I think we can 
> > > leave out the locks around it and put beside other fields only accessed 
> > > by the worker thread, i.e. `DiagsWereReported`, etc.
> > > 
> > > PS Sorry go going back and forth, I didn't think about it in the previous 
> > > review iteration.
> > Unfortunately not, most statuses are emitted via worker thread, but we emit 
> > `Queued` status in the main thread...
> Hm, that sounds surprising.
> What if the rebuild is in progress on the worker thread and we emit the 
> queued status on the main thread at the same time? We might get weird 
> sequences of statuses, right? 
> E.g. `Queued → BuildingPreamble → [new update on the main thread] Queued → 
> [processing request on a worker thread] RunningAction('XRefs')`
> 
> The `Queued` request status between `BuildingPreamble` and `RunningAction` 
> looks **really** weird.
> Maybe we could avoid that by setting emitting the `Queued` status on the 
> worker thread whenever we're about to sleep on the debounce timeout?
This sounds fair enough, and can simplify the code.

> Maybe we could avoid that by setting emitting the Queued status on the worker 
> thread whenever we're about to sleep on the debounce timeout?

The `Queued` state is originally designed for the state that the worker thread 
is blocked by the max-concurrent-threads semaphore.
Emitting it when we're about to sleep on the debounce timeout doesn't seem a 
right place to me. I think a reasonable place is before requiring the `Barrier`.




Comment at: clangd/TUScheduler.cpp:636
+  if (Requests.empty())
+emitTUStatus({TUAction::Idle, /*Name*/ ""});
 }

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > Maybe do it outside the lock? The less dependencies between the locks we 
> > > have, the better and this one seems unnecessary.
> > Is it safe to read the `Requests` out of the lock here?
> No, but we could store the value of `.empty()` in a local var under the lock. 
> The code looks a bit weirder, but that's a small price to pay for less locks 
> being held at the same time.
SG.



Comment at: clangd/TUScheduler.h:60
+BuildingPreamble, // The preamble of the TU is being built.
+BuildingFile, // The TU is being built.
+Idle, // Indicates the worker thread is idle, and ready to run any upcoming

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > hokein wrote:
> > > > ilya-biryukov wrote:
> > > > > What's the fundamental difference between `BuildingFile` and 
> > > > > `RunningAction`?
> > > > > We will often rebuild ASTs while running various actions that read 
> > > > > the preamble.
> > > > > 
> > > > > Maybe we could squash those two together? One can view diagnostics as 
> > > > > an action on the AST, similar to a direct LSP request like 
> > > > > findReferences.
> > > > They are two different states, you can think `RunningAction` means the 
> > > > AST worker starts processing a task (for example `Update`, 
> > > > `GoToDefinition`) and `BuildingFile` means building the AST (which is 
> > > > one possible step when processing the task).  
> > > > 
> > > > In the current implementation, `BuildingPreamble` and `BuildingFile` 
> > > > are only emitted when AST worker processes the `Update` task (as we are 
> > > > mostly interested in TU states of `ASTWorker::update`); for other AST 
> > > > tasks, we just emit `RunningAction` which should be enough.
> > > > 
> > > > Given the following requests in the worker queue:
> > > > `[Update]`
> > > > `[GoToDef]`
> > > > `[Update]`
> > > > `[CodeComplete]`
> > > > 
> > > > statuses we emitted are
> > > > 
> > > > `RunningAction(Update) BuildingPreamble BuildingFile`
> > > > `RunningAction(GoToDef)`
> > > > `RunningAction(Update)  BuildingPreamble BuildingFile`
> > > > `RunningAction(GetCurrentPreamble)`
> > > > `Idle`
> > > > 
> > > That's the confusing part: clangd might be building the ASTs inside 
> > > `RunningAction` too, but we only choose to report `BuildingFile` during 
> > > updates.
> > > I would get rid of `BuildingFile` and change it to 
> > > `RunningAction(Diagnostics)` instead, it feels like that would allow 
> > > avoiding some confusion in the future.
> > > 
> > `BuildingFile` might be more natural (understandable) than `Diagnostics`. 
> > Two possible ways here:
> > 
> > - we can also report `BuildingFile` for other AST tasks, does it reduce the 
> > confusing part? 
> > - or if you think `Diagnostic` is an important state of `Update`, how about 
> > adding a new state `RunningDiagnostic`? `RunningAction` is a higher level 
> > infor

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: test/Analysis/ctu-main.cpp:6
 // 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
+// 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 
-analyzer-display-ctu-progress 2>&1 %s | FileCheck %s
+

martong wrote:
> Szelethus wrote:
> > I think these RUN lines would really benefit from introducing line breaks.
> Yes, I agree. Unfortunately, I could not figure out how to break them. Using 
> a simple "\" at around the 80th column gives `Test has unterminated run lines 
> (with '\')`. If I use block comments with "\" the same happens. If I use 
> block comments and don't use the "\" then the second line is not interpreted. 
> Is it really possible to break RUN lines? I could not find anything about 
> that in the online docs.
Oh, I just have found your other comment to the other patch. So yes, it is 
indeed possible to break this line. I updated the patch accordingly. The other 
long lines which are already there I am going to change in an independent 
patch: (https://reviews.llvm.org/D55129).


Repository:
  rC Clang

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

https://reviews.llvm.org/D55135



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

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> A quick, tangentially related idea: Would it be useful to implement multiline 
> nolint sections (e.g. `//BEGINNOLINT` – `//ENDNOLINT` or something similar)? 
> This would be helpful for using clang-tidy on projects that contain some 
> automatically generated fragments.

Yes, I think so. The filtering mechanism we have right now are not sufficient i 
think.
Features similar to pylint would be great, were you can disable certain checks 
for scopes (functions, modules, classes). With this kind of functionality we 
could bring clang-tidy to system-level libraries too, as the "unsafe" code is 
usually chatty in many regards, but with fine-grained and powerful filtering it 
should be easier to manage the output.

TL;DR I am in favor of such approaches, a short proposal (with what you plan) 
on the mailing-list wouldn't hurt for more opinions :)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54757



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

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176590.
martong marked an inline comment as done.
martong added a comment.

- Break long RUN line


Repository:
  rC Clang

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

https://reviews.llvm.org/D55135

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  include/clang/Driver/CC1Options.td
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/ctu-main.cpp

Index: test/Analysis/ctu-main.cpp
===
--- test/Analysis/ctu-main.cpp
+++ test/Analysis/ctu-main.cpp
@@ -3,6 +3,14 @@
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o %T/ctudir/ctu-chain.cpp.ast %S/Inputs/ctu-chain.cpp
 // 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
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%T/ctudir \
+// RUN:   -analyzer-display-ctu-progress 2>&1 %s | FileCheck %s
+
+// CHECK: ANALYZE CTU loaded AST file: {{.*}}/ctu-other.cpp
+// CHECK: ANALYZE CTU loaded AST file: {{.*}}/ctu-chain.cpp
 
 #include "ctu-hdr.h"
 
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -560,7 +560,8 @@
   cross_tu::CrossTranslationUnitContext &CTUCtx =
   *Engine->getCrossTranslationUnitContext();
   llvm::Expected CTUDeclOrError =
-  CTUCtx.getCrossTUDefinition(FD, Opts.getCTUDir(), Opts.getCTUIndexName());
+  CTUCtx.getCrossTUDefinition(FD, Opts.getCTUDir(), Opts.getCTUIndexName(),
+  Opts.AnalyzerDisplayCTUProgress);
 
   if (!CTUDeclOrError) {
 handleAllErrors(CTUDeclOrError.takeError(),
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -289,6 +289,8 @@
   Opts.NoRetryExhausted = Args.hasArg(OPT_analyzer_disable_retry_exhausted);
   Opts.AnalyzeAll = Args.hasArg(OPT_analyzer_opt_analyze_headers);
   Opts.AnalyzerDisplayProgress = Args.hasArg(OPT_analyzer_display_progress);
+  Opts.AnalyzerDisplayCTUProgress =
+  Args.hasArg(OPT_analyzer_display_ctu_progress);
   Opts.AnalyzeNestedBlocks =
 Args.hasArg(OPT_analyzer_opt_analyze_nested_blocks);
   Opts.AnalyzeSpecificFunction = Args.getLastArgValue(OPT_analyze_function);
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -149,14 +149,15 @@
 llvm::Expected
 CrossTranslationUnitContext::getCrossTUDefinition(const FunctionDecl *FD,
   StringRef CrossTUDir,
-  StringRef IndexName) {
+  StringRef IndexName,
+  bool DisplayCTUProgress) {
   assert(!FD->hasBody() && "FD has a definition in current translation unit!");
   const std::string LookupFnName = getLookupName(FD);
   if (LookupFnName.empty())
 return llvm::make_error(
 index_error_code::failed_to_generate_usr);
   llvm::Expected ASTUnitOrError =
-  loadExternalAST(LookupFnName, CrossTUDir, IndexName);
+  loadExternalAST(LookupFnName, CrossTUDir, IndexName, DisplayCTUProgress);
   if (!ASTUnitOrError)
 return ASTUnitOrError.takeError();
   ASTUnit *Unit = *ASTUnitOrError;
@@ -193,7 +194,8 @@
 }
 
 llvm::Expected CrossTranslationUnitContext::loadExternalAST(
-StringRef LookupName, StringRef CrossTUDir, StringRef IndexName) {
+StringRef LookupName, StringRef CrossTUDir, StringRef IndexName,
+bool DisplayCTUProgress) {
   // FIXME: The current implementation only supports loading functions with
   //a lookup name from a single translation unit. If multiple
   //translation units contains functions with the same lookup name an
@@ -233,6 +235,10 @@
   ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts()));
   Unit = LoadedUnit.get();
   FileASTUnitMap[ASTFileName] = std::move(LoadedUnit);
+  if (DisplayCTUProgress) {
+llvm::errs() << "ANALYZE CTU loaded AST file: "
+ << ASTFileName << "\n";
+  }
 } else {
   Unit = ASTCacheEntry->second.get();
 }
Index: include/clang/StaticAnalyzer/Core/AnalyzerO

[PATCH] D55262: [OpenCL] Fix for TBAA information of pointer after addresspacecast

[Romanov Vlad via Phabricator via cfe-commits]

romanovvlad created this revision.
romanovvlad added reviewers: rjmccall, yaxunl, Anastasia.
Herald added a subscriber: kosarev.

When addresspacecast is generated resulting pointer should preserve TBAA 
information from original value.


Repository:
  rC Clang

https://reviews.llvm.org/D55262

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGenOpenCLCXX/address-space-deduction2.cl


Index: test/CodeGenOpenCLCXX/address-space-deduction2.cl
===
--- test/CodeGenOpenCLCXX/address-space-deduction2.cl
+++ test/CodeGenOpenCLCXX/address-space-deduction2.cl
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -O0 -emit-llvm 
-o - | FileCheck %s
+
+// CHECK: call void @llvm.memcpy{{.*}}, {{.*}}, i32 16
+
+class P {
+public:
+  P(const P &Rhs) = default;
+
+  long a;
+  long b;
+};
+
+__kernel void foo(__global P* GPtr) {
+  P Val = GPtr[0];
+}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -4270,7 +4270,9 @@
 llvm::Value *V = getTargetHooks().performAddrSpaceCast(
 *this, LV.getPointer(), E->getSubExpr()->getType().getAddressSpace(),
 DestTy.getAddressSpace(), ConvertType(DestTy));
-return MakeNaturalAlignPointeeAddrLValue(V, DestTy);
+return MakeAddrLValue(Address(V, LV.getAddress().getAlignment()),
+  E->getType(), LV.getBaseInfo(),
+  LV.getTBAAInfo());
   }
   case CK_ObjCObjectLValueCast: {
 LValue LV = EmitLValue(E->getSubExpr());


Index: test/CodeGenOpenCLCXX/address-space-deduction2.cl
===
--- test/CodeGenOpenCLCXX/address-space-deduction2.cl
+++ test/CodeGenOpenCLCXX/address-space-deduction2.cl
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -O0 -emit-llvm -o - | FileCheck %s
+
+// CHECK: call void @llvm.memcpy{{.*}}, {{.*}}, i32 16
+
+class P {
+public:
+  P(const P &Rhs) = default;
+
+  long a;
+  long b;
+};
+
+__kernel void foo(__global P* GPtr) {
+  P Val = GPtr[0];
+}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -4270,7 +4270,9 @@
 llvm::Value *V = getTargetHooks().performAddrSpaceCast(
 *this, LV.getPointer(), E->getSubExpr()->getType().getAddressSpace(),
 DestTy.getAddressSpace(), ConvertType(DestTy));
-return MakeNaturalAlignPointeeAddrLValue(V, DestTy);
+return MakeAddrLValue(Address(V, LV.getAddress().getAlignment()),
+  E->getType(), LV.getBaseInfo(),
+  LV.getTBAAInfo());
   }
   case CK_ObjCObjectLValueCast: {
 LValue LV = EmitLValue(E->getSubExpr());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55256: [clangd] Support clang-tidy configuration in clangd.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 176591.
hokein added a comment.

Minor cleanup


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55256

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/TUSchedulerTests.cpp
  unittests/clangd/TestTU.cpp

Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -37,6 +37,10 @@
   Inputs.CompileCommand.Directory = testRoot();
   Inputs.Contents = Code;
   Inputs.FS = buildTestFS({{FullFilename, Code}, {FullHeaderName, HeaderCode}});
+  Inputs.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults();
+  Inputs.ClangTidyOpts.Checks =
+  "-*, bugprone-sizeof-expression, bugprone-macro-repeated-side-effects, "
+  "modernize-deprecated-headers";
   auto PCHs = std::make_shared();
   auto Preamble =
   buildPreamble(FullFilename, *createInvocationFromCommandLine(Cmd),
Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -34,7 +34,8 @@
 protected:
   ParseInputs getInputs(PathRef File, std::string Contents) {
 return ParseInputs{*CDB.getCompileCommand(File),
-   buildTestFS(Files, Timestamps), std::move(Contents)};
+   buildTestFS(Files, Timestamps), std::move(Contents),
+   tidy::ClangTidyOptions::getDefaults()};
   }
 
   void updateWithCallback(TUScheduler &S, PathRef File, StringRef Contents,
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -364,7 +364,8 @@
   auto AST =
   ParsedAST::build(createInvocationFromCommandLine(Cmd), PreambleData,
MemoryBuffer::getMemBufferCopy(Main.code()),
-   std::make_shared(), PI.FS);
+   std::make_shared(), PI.FS,
+   tidy::ClangTidyOptions::getDefaults());
   ASSERT_TRUE(AST);
   FileIndex Index;
   Index.updateMain(MainFile, *AST);
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -188,6 +188,11 @@
  "placeholders for method parameters."),
 cl::init(clangd::CodeCompleteOptions().EnableFunctionArgSnippets));
 
+static cl::opt
+ClangTidyChecks("clang-tidy-checks",
+cl::desc("A list of clang-tidy checks running in clangd"),
+cl::init(""), cl::Hidden);
+
 namespace {
 
 /// \brief Supports a test URI scheme with relaxed constraints for lit tests.
@@ -234,7 +239,6 @@
 #else
 const char TestScheme::TestDir[] = "/clangd-test";
 #endif
-
 }
 
 int main(int argc, char *argv[]) {
@@ -388,8 +392,17 @@
   stdin, outs(),
   InputMirrorStream ? InputMirrorStream.getPointer() : nullptr, PrettyPrint,
   InputStyle);
+  RealFileSystemProvider FSProvider;
+  // Create an empty option.
+  auto ClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
+  if (!ClangTidyChecks.empty())
+ClangTidyOptions.Checks = ClangTidyChecks;
+  tidy::FileOptionsProvider ClangTidyOptProvider(
+  tidy::ClangTidyGlobalOptions(), tidy::ClangTidyOptions::getDefaults(),
+  ClangTidyOptions, FSProvider.getFileSystem());
+  Opts.ClangTidyOptProvider = &ClangTidyOptProvider;
   ClangdLSPServer LSPServer(
-  *Transport, CCOpts, CompileCommandsDirPath,
+  *Transport, FSProvider, CCOpts, CompileCommandsDirPath,
   /*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs, Opts);
   constexpr int NoShutdownRequestErrorCode = 1;
   set_thread_name("clangd.main");
Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -10,6 +10,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDUNIT_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDUNIT_H
 
+#include "../clang-tidy/ClangTidyOptions.h"
 #include "Diagnostics.h"
 #include "FS.h"
 #include "Function.h"
@@ -65,6 +66,7 @@
   tooling::CompileCommand CompileCommand;
   IntrusiveRefCntPtr FS;
   std::string Contents;
+  tidy::ClangTidyOptions ClangTidyOpts;
 };
 
 /// Stores and provides access to parsed AST.
@@ -77,7 +79,8 @@
 std::shared_ptr Preamble,
 std::unique_ptr Buffer,
 std::shared_ptr PCHs,
-IntrusiveRefCntPtr VFS);
+IntrusiveRefCntPtr VFS,
+tidy::ClangTidyOptions ClangTidyOpts);
 
   ParsedAST(ParsedAST &&Other);
   ParsedAST &operator=(ParsedAST &&Other);
Index: 

[PATCH] D55255: Fix a false positive in misplaced-widening-cast

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG in general, but a few comments re: test.




Comment at: 
test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp:67-73
+MON = 0,
+TUE = 1,
+WED = 2,
+THR = 3,
+FRI = 4,
+SAT = 5,
+SUN = 6

No need for initializers here. Please clang-format the new code.



Comment at: 
test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp:76
+
+//do not warn for int to enum casts
+DaysEnum nextDay(DaysEnum day){

Please use proper capitalization and punctuation in comments. And put a space 
between `//` and the text.



Comment at: 
test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp:77-88
+DaysEnum nextDay(DaysEnum day){
+  if (day < SUN)
+  day = (DaysEnum)(day + 1);
+  return day;
+}
+
+//do not warn for int to enum casts

Let's make the code a bit shorter and remove unnecessary details. For example, 
like this:

```
void f(DaysEnum day){
  if (day < SUN)
  day = (DaysEnum)(day + 1);
  if (day < SUN)
  day = static_cast(day + 1);
}
```


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55255



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

[PATCH] D55260: [CodeComplete] Fix a crash in access checks of inner classes

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:1348
+}
+return Results.getSema().IsSimplyAccessible(ND, Cls, BaseType);
+  }

I suppose `Cls` should be `NamingClass`


Repository:
  rC Clang

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

https://reviews.llvm.org/D55260



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

[PATCH] D55260: [CodeComplete] Fix a crash in access checks of inner classes

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 176593.
ilya-biryukov added a comment.

- Actually use the computed NamingClass


Repository:
  rC Clang

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

https://reviews.llvm.org/D55260

Files:
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/accessibility.cpp


Index: test/CodeCompletion/accessibility.cpp
===
--- test/CodeCompletion/accessibility.cpp
+++ test/CodeCompletion/accessibility.cpp
@@ -71,3 +71,27 @@
 // RUN: | FileCheck -check-prefix=UNRELATED %s
   }
 };
+
+class Outer {
+ public:
+  static int pub;
+ protected:
+  static int prot;
+ private:
+  static int priv;
+
+  class Inner {
+int test() {
+  Outer::pub = 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:85:14 %s -o - \
+// RUN: | FileCheck -check-prefix=OUTER %s
+// OUTER: priv : [#int#]priv
+// OUTER: prot : [#int#]prot
+// OUTER: pub : [#int#]pub
+
+// Also check the unqualified case.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:85:1 %s -o - \
+// RUN: | FileCheck -check-prefix=OUTER %s
+}
+  };
+};
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -11,6 +11,7 @@
 //
 
//===--===//
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/ExprCXX.h"
@@ -1313,23 +1314,39 @@
 
   void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, DeclContext *Ctx,
  bool InBaseClass) override {
-// Naming class to use for access check. In most cases it was provided
-// explicitly (e.g. member access (lhs.foo) or qualified lookup (X::)), for
-// unqualified lookup we fallback to the \p Ctx in which we found the
-// member.
-auto *NamingClass = this->NamingClass;
-if (!NamingClass)
-  NamingClass = llvm::dyn_cast_or_null(Ctx);
-bool Accessible =
-Results.getSema().IsSimplyAccessible(ND, NamingClass, BaseType);
 ResultBuilder::Result Result(ND, Results.getBasePriority(ND), nullptr,
- false, Accessible, FixIts);
+ false, IsAccessible(ND, Ctx), FixIts);
 Results.AddResult(Result, InitialLookupCtx, Hiding, InBaseClass);
   }
 
   void EnteredContext(DeclContext *Ctx) override {
 Results.addVisitedContext(Ctx);
   }
+
+  private:
+  bool IsAccessible(NamedDecl *ND, DeclContext *Ctx) {
+auto *Cls = llvm::dyn_cast_or_null(Ctx);
+if (!Cls)
+  return true; // Only class members require access checking.
+
+// Naming class to use for access check. In most cases it was provided
+// explicitly (e.g. member access (lhs.foo) or qualified lookup (X::)),
+// for unqualified lookup we fallback to the \p Ctx in which we found the
+// member.
+auto *NamingClass = this->NamingClass;
+QualType BaseType = this->BaseType;
+if (!NamingClass)
+  NamingClass = Cls;
+// When we emulate implicit 'this->' in an unqualified lookup, we might end
+// up with an invalid naming class. In that case, we avoid emulating
+// 'this->' qualifier to satisfy preconditions of the access checking.
+if (NamingClass->getCanonicalDecl() != Cls->getCanonicalDecl() &&
+!NamingClass->isDerivedFrom(Cls)) {
+  NamingClass = Cls;
+  BaseType = QualType();
+}
+return Results.getSema().IsSimplyAccessible(ND, NamingClass, BaseType);
+  }
 };
 } // namespace
 


Index: test/CodeCompletion/accessibility.cpp
===
--- test/CodeCompletion/accessibility.cpp
+++ test/CodeCompletion/accessibility.cpp
@@ -71,3 +71,27 @@
 // RUN: | FileCheck -check-prefix=UNRELATED %s
   }
 };
+
+class Outer {
+ public:
+  static int pub;
+ protected:
+  static int prot;
+ private:
+  static int priv;
+
+  class Inner {
+int test() {
+  Outer::pub = 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:85:14 %s -o - \
+// RUN: | FileCheck -check-prefix=OUTER %s
+// OUTER: priv : [#int#]priv
+// OUTER: prot : [#int#]prot
+// OUTER: pub : [#int#]pub
+
+// Also check the unqualified case.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:85:1 %s -o - \
+// RUN: | FileCheck -check-prefix=OUTER %s
+}
+  };
+};
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -11,6 +11,7 @@
 //
 //===--===//
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/ExprCXX

[PATCH] D55062: [clangd] Partition include graph on auto-index.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 176594.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55062

Files:
  clangd/Headers.h
  clangd/index/Background.cpp
  unittests/clangd/BackgroundIndexTests.cpp

Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -24,6 +24,11 @@
 RefsAre(std::vector> Matchers) {
   return ElementsAre(testing::Pair(_, UnorderedElementsAreArray(Matchers)));
 }
+// URI cannot be empty since it references keys in the IncludeGraph.
+MATCHER(EmptyIncludeNode, "") {
+  return !arg.IsTU && !arg.URI.empty() && arg.Digest == FileDigest{0} &&
+ arg.DirectIncludes.empty();
+}
 
 class MemoryShardStorage : public BackgroundIndexStorage {
   mutable std::mutex StorageMu;
@@ -93,7 +98,7 @@
   Cmd.Filename = testPath("root/A.cc");
   Cmd.Directory = testPath("root");
   Cmd.CommandLine = {"clang++", "-DA=1", testPath("root/A.cc")};
-  CDB.setCompileCommand(testPath("root"), Cmd);
+  CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
 
   ASSERT_TRUE(Idx.blockUntilIdleForTest());
   EXPECT_THAT(
@@ -103,7 +108,7 @@
 
   Cmd.Filename = testPath("root/B.cc");
   Cmd.CommandLine = {"clang++", Cmd.Filename};
-  CDB.setCompileCommand(testPath("root"), Cmd);
+  CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
 
   ASSERT_TRUE(Idx.blockUntilIdleForTest());
   // B_CC is dropped as we don't collect symbols from A.h in this compilation.
@@ -143,7 +148,7 @@
 OverlayCDB CDB(/*Base=*/nullptr);
 BackgroundIndex Idx(Context::empty(), "", FS, CDB,
 [&](llvm::StringRef) { return &MSS; });
-CDB.setCompileCommand(testPath("root"), Cmd);
+CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
 ASSERT_TRUE(Idx.blockUntilIdleForTest());
   }
   EXPECT_EQ(CacheHits, 0U);
@@ -165,5 +170,56 @@
   EXPECT_THAT(*ShardSource->Refs, RefsAre({FileURI("unittest:///root/A.cc")}));
 }
 
+TEST_F(BackgroundIndexTest, DirectIncludesTest) {
+  MockFSProvider FS;
+  FS.Files[testPath("root/B.h")] = "";
+  FS.Files[testPath("root/A.h")] = R"cpp(
+  #include "B.h"
+  void common();
+  void f_b();
+  class A_CC {};
+  )cpp";
+  std::string A_CC = "#include \"A.h\"\nvoid g() { (void)common; }";
+  FS.Files[testPath("root/A.cc")] = A_CC;
+
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
+  {
+OverlayCDB CDB(/*Base=*/nullptr);
+BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+[&](llvm::StringRef) { return &MSS; });
+CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
+ASSERT_TRUE(Idx.blockUntilIdleForTest());
+  }
+
+  auto ShardSource = MSS.loadShard(testPath("root/A.cc"));
+  EXPECT_TRUE(ShardSource->Sources);
+  EXPECT_EQ(ShardSource->Sources->size(), 2U); // A.cc, A.h
+  EXPECT_THAT(
+  ShardSource->Sources->lookup("unittest:///root/A.cc").DirectIncludes,
+  UnorderedElementsAre("unittest:///root/A.h"));
+  EXPECT_NE(ShardSource->Sources->lookup("unittest:///root/A.cc").Digest,
+FileDigest{0});
+  EXPECT_THAT(ShardSource->Sources->lookup("unittest:///root/A.h"),
+  EmptyIncludeNode());
+
+  auto ShardHeader = MSS.loadShard(testPath("root/A.h"));
+  EXPECT_TRUE(ShardHeader->Sources);
+  EXPECT_EQ(ShardHeader->Sources->size(), 2U); // A.h, B.h
+  EXPECT_THAT(
+  ShardHeader->Sources->lookup("unittest:///root/A.h").DirectIncludes,
+  UnorderedElementsAre("unittest:///root/B.h"));
+  EXPECT_NE(ShardHeader->Sources->lookup("unittest:///root/A.h").Digest,
+FileDigest{0});
+  EXPECT_THAT(ShardHeader->Sources->lookup("unittest:///root/B.h"),
+  EmptyIncludeNode());
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/index/Background.cpp
===
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -35,6 +35,58 @@
 using namespace llvm;
 namespace clang {
 namespace clangd {
+namespace {
+// Resolves URI to file paths with cache.
+class URIToFileCache {
+public:
+  URIToFileCache(llvm::StringRef HintPath) : HintPath(HintPath) {}
+
+  llvm::StringRef resolve(llvm::StringRef FileURI) {
+auto I = URIToPathCache.try_emplace(FileURI);
+if (I.second) {
+  auto U = URI::parse(FileURI);
+  if (!U) {
+elog("Failed to parse URI {0}: {1}", FileURI, U.takeError());
+assert(false && "Failed to parse URI");
+return "";
+  }
+  auto Path = URI::resolve(*U, HintPath);
+  if (!Path) {
+elog("Failed to res

[clang-tools-extra] r348252 - [clangd] Partition include graph on auto-index.

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Tue Dec  4 03:31:57 2018
New Revision: 348252

URL: http://llvm.org/viewvc/llvm-project?rev=348252&view=rev
Log:
[clangd] Partition include graph on auto-index.

Summary:
Partitions include graphs in auto-index so that each shards contains
only part of the include graph related to itself.

Reviewers: ilya-biryukov

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

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

Modified:
clang-tools-extra/trunk/clangd/Headers.h
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/Headers.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Headers.h?rev=348252&r1=348251&r2=348252&view=diff
==
--- clang-tools-extra/trunk/clangd/Headers.h (original)
+++ clang-tools-extra/trunk/clangd/Headers.h Tue Dec  4 03:31:57 2018
@@ -53,9 +53,9 @@ llvm::raw_ostream &operator<<(llvm::raw_
 // self-contained).
 struct IncludeGraphNode {
   // True if current file is a main file rather than a header.
-  bool IsTU;
+  bool IsTU = false;
   llvm::StringRef URI;
-  FileDigest Digest;
+  FileDigest Digest{0};
   std::vector DirectIncludes;
 };
 // FileURI and FileInclusions are references to keys of the map containing

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=348252&r1=348251&r2=348252&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Tue Dec  4 03:31:57 2018
@@ -35,6 +35,58 @@
 using namespace llvm;
 namespace clang {
 namespace clangd {
+namespace {
+// Resolves URI to file paths with cache.
+class URIToFileCache {
+public:
+  URIToFileCache(llvm::StringRef HintPath) : HintPath(HintPath) {}
+
+  llvm::StringRef resolve(llvm::StringRef FileURI) {
+auto I = URIToPathCache.try_emplace(FileURI);
+if (I.second) {
+  auto U = URI::parse(FileURI);
+  if (!U) {
+elog("Failed to parse URI {0}: {1}", FileURI, U.takeError());
+assert(false && "Failed to parse URI");
+return "";
+  }
+  auto Path = URI::resolve(*U, HintPath);
+  if (!Path) {
+elog("Failed to resolve URI {0}: {1}", FileURI, Path.takeError());
+assert(false && "Failed to resolve URI");
+return "";
+  }
+  I.first->second = *Path;
+}
+return I.first->second;
+  }
+
+private:
+  std::string HintPath;
+  llvm::StringMap URIToPathCache;
+};
+
+// We keep only the node "U" and its edges. Any node other than "U" will be
+// empty in the resultant graph.
+IncludeGraph getSubGraph(const URI &U, const IncludeGraph &FullGraph) {
+  IncludeGraph IG;
+
+  std::string FileURI = U.toString();
+  auto Entry = IG.try_emplace(FileURI).first;
+  auto &Node = Entry->getValue();
+  Node = FullGraph.lookup(Entry->getKey());
+  Node.URI = Entry->getKey();
+
+  // URIs inside nodes must point into the keys of the same IncludeGraph.
+  for (auto &Include : Node.DirectIncludes) {
+auto I = IG.try_emplace(Include).first;
+I->getValue().URI = I->getKey();
+Include = I->getKey();
+  }
+
+  return IG;
+}
+} // namespace
 
 BackgroundIndex::BackgroundIndex(
 Context BackgroundContext, StringRef ResourceDir,
@@ -150,36 +202,6 @@ void BackgroundIndex::enqueueTask(Task T
   QueueCV.notify_all();
 }
 
-// Resolves URI to file paths with cache.
-class URIToFileCache {
-public:
-  URIToFileCache(llvm::StringRef HintPath) : HintPath(HintPath) {}
-
-  llvm::StringRef resolve(llvm::StringRef FileURI) {
-auto I = URIToPathCache.try_emplace(FileURI);
-if (I.second) {
-  auto U = URI::parse(FileURI);
-  if (!U) {
-elog("Failed to parse URI {0}: {1}", FileURI, U.takeError());
-assert(false && "Failed to parse URI");
-return "";
-  }
-  auto Path = URI::resolve(*U, HintPath);
-  if (!Path) {
-elog("Failed to resolve URI {0}: {1}", FileURI, Path.takeError());
-assert(false && "Failed to resolve URI");
-return "";
-  }
-  I.first->second = *Path;
-}
-return I.first->second;
-  }
-
-private:
-  std::string HintPath;
-  llvm::StringMap URIToPathCache;
-};
-
 /// Given index results from a TU, only update files in \p FilesToUpdate.
 void BackgroundIndex::update(StringRef MainFile, IndexFileIn Index,
  const StringMap &FilesToUpdate,
@@ -233,6 +255,8 @@ void BackgroundIndex::update(StringRef M
 
 auto SS = llvm::make_unique(std::move(Syms).build());
 auto RS = llvm::make_unique(std::move(Refs).build());
+auto IG = llvm::make_unique(
+getSubGraph(URI::create(Path), Index.Sources.getValue()));
 
 auto Hash = Fil

[PATCH] D55062: [clangd] Partition include graph on auto-index.

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348252: [clangd] Partition include graph on auto-index. 
(authored by kadircet, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55062

Files:
  clang-tools-extra/trunk/clangd/Headers.h
  clang-tools-extra/trunk/clangd/index/Background.cpp
  clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp

Index: clang-tools-extra/trunk/clangd/index/Background.cpp
===
--- clang-tools-extra/trunk/clangd/index/Background.cpp
+++ clang-tools-extra/trunk/clangd/index/Background.cpp
@@ -35,6 +35,58 @@
 using namespace llvm;
 namespace clang {
 namespace clangd {
+namespace {
+// Resolves URI to file paths with cache.
+class URIToFileCache {
+public:
+  URIToFileCache(llvm::StringRef HintPath) : HintPath(HintPath) {}
+
+  llvm::StringRef resolve(llvm::StringRef FileURI) {
+auto I = URIToPathCache.try_emplace(FileURI);
+if (I.second) {
+  auto U = URI::parse(FileURI);
+  if (!U) {
+elog("Failed to parse URI {0}: {1}", FileURI, U.takeError());
+assert(false && "Failed to parse URI");
+return "";
+  }
+  auto Path = URI::resolve(*U, HintPath);
+  if (!Path) {
+elog("Failed to resolve URI {0}: {1}", FileURI, Path.takeError());
+assert(false && "Failed to resolve URI");
+return "";
+  }
+  I.first->second = *Path;
+}
+return I.first->second;
+  }
+
+private:
+  std::string HintPath;
+  llvm::StringMap URIToPathCache;
+};
+
+// We keep only the node "U" and its edges. Any node other than "U" will be
+// empty in the resultant graph.
+IncludeGraph getSubGraph(const URI &U, const IncludeGraph &FullGraph) {
+  IncludeGraph IG;
+
+  std::string FileURI = U.toString();
+  auto Entry = IG.try_emplace(FileURI).first;
+  auto &Node = Entry->getValue();
+  Node = FullGraph.lookup(Entry->getKey());
+  Node.URI = Entry->getKey();
+
+  // URIs inside nodes must point into the keys of the same IncludeGraph.
+  for (auto &Include : Node.DirectIncludes) {
+auto I = IG.try_emplace(Include).first;
+I->getValue().URI = I->getKey();
+Include = I->getKey();
+  }
+
+  return IG;
+}
+} // namespace
 
 BackgroundIndex::BackgroundIndex(
 Context BackgroundContext, StringRef ResourceDir,
@@ -150,36 +202,6 @@
   QueueCV.notify_all();
 }
 
-// Resolves URI to file paths with cache.
-class URIToFileCache {
-public:
-  URIToFileCache(llvm::StringRef HintPath) : HintPath(HintPath) {}
-
-  llvm::StringRef resolve(llvm::StringRef FileURI) {
-auto I = URIToPathCache.try_emplace(FileURI);
-if (I.second) {
-  auto U = URI::parse(FileURI);
-  if (!U) {
-elog("Failed to parse URI {0}: {1}", FileURI, U.takeError());
-assert(false && "Failed to parse URI");
-return "";
-  }
-  auto Path = URI::resolve(*U, HintPath);
-  if (!Path) {
-elog("Failed to resolve URI {0}: {1}", FileURI, Path.takeError());
-assert(false && "Failed to resolve URI");
-return "";
-  }
-  I.first->second = *Path;
-}
-return I.first->second;
-  }
-
-private:
-  std::string HintPath;
-  llvm::StringMap URIToPathCache;
-};
-
 /// Given index results from a TU, only update files in \p FilesToUpdate.
 void BackgroundIndex::update(StringRef MainFile, IndexFileIn Index,
  const StringMap &FilesToUpdate,
@@ -233,6 +255,8 @@
 
 auto SS = llvm::make_unique(std::move(Syms).build());
 auto RS = llvm::make_unique(std::move(Refs).build());
+auto IG = llvm::make_unique(
+getSubGraph(URI::create(Path), Index.Sources.getValue()));
 
 auto Hash = FilesToUpdate.lookup(Path);
 // We need to store shards before updating the index, since the latter
@@ -241,6 +265,7 @@
   IndexFileOut Shard;
   Shard.Symbols = SS.get();
   Shard.Refs = RS.get();
+  Shard.Sources = IG.get();
 
   if (auto Error = IndexStorage->storeShard(Path, Shard))
 elog("Failed to write background-index shard for file {0}: {1}", Path,
@@ -260,9 +285,9 @@
 // digests.
 // \p FileDigests contains file digests for the current indexed files, and all
 // changed files will be added to \p FilesToUpdate.
-decltype(SymbolCollector::Options::FileFilter) createFileFilter(
-const llvm::StringMap &FileDigests,
-llvm::StringMap &FilesToUpdate) {
+decltype(SymbolCollector::Options::FileFilter)
+createFileFilter(const llvm::StringMap &FileDigests,
+ llvm::StringMap &FilesToUpdate) {
   return [&FileDigests, &FilesToUpdate](const SourceManager &SM, FileID FID) {
 StringRef Path;
 if (const auto *F = SM.getFileEntryForID(FID))
@@ -338,12 +363,11 @@
   SymbolCollector::Options IndexOpts;
   StringMap FilesToUpdate;
   IndexOpts.FileFilter = createFileFilter(DigestsSnap

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

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D54945#1316251 , @MarinaKalashina 
wrote:

> Fixes:
>
> - empty line before 'Standalone tool'
> - table columns with '+/-' aligned
> - line width limited to 80 (except for the table)
>
>   Additions:
> - clang-tidy-vs plugin
> - Clangd in the intro, the table, and CLion's paragraph


Did you forget to add a new file to the patch?

Please also include full context into the diff. See 
https://llvm.org/docs/Phabricator.html


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

https://reviews.llvm.org/D54945



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

[PATCH] D55260: [CodeComplete] Fix a crash in access checks of inner classes

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

I believe also we need another test case where `Cls` and `NamingClass` are 
different.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55260



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

[PATCH] D54905: [AddressSanitizer] Add flag to disable linking with CXX runtime

[Eugene Leviant via Phabricator via cfe-commits]

evgeny777 added a comment.

> ASan uses -Wl,-whole-archive to pull all its members

Yep, I forgot about this. Looks like weak new/delete will do the job then.


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

https://reviews.llvm.org/D54905



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

[PATCH] D54401: [analyzer] Prefer returns values to out-params in CheckerRegistry.cpp

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D54401#1315354 , @NoQ wrote:

> The code looks good, but i vaguely remember that we might accidentally break 
> clang-tidy integration that uses this API to enable/disable specific checkers 
> via `-checks=-analyzer-...`.
>
> *summons @alexfh*


It's hard to tell without trying. Could you build and test clang-tools-extra 
with this patch? There should be a test for the relevant functionality in 
clang-tidy.


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

https://reviews.llvm.org/D54401



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

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

[Marina Kalashina via Phabricator via cfe-commits]

MarinaKalashina marked 4 inline comments as done.
MarinaKalashina added a comment.

In D54945#1318278 , @alexfh wrote:

> In D54945#1316251 , @MarinaKalashina 
> wrote:
>
> > Fixes:
> >
> > - empty line before 'Standalone tool'
> > - table columns with '+/-' aligned
> > - line width limited to 80 (except for the table)
> >
> >   Additions:
> > - clang-tidy-vs plugin
> > - Clangd in the intro, the table, and CLion's paragraph
>
>
> Did you forget to add a new file to the patch?
>
> Please also include full context into the diff. See 
> https://llvm.org/docs/Phabricator.html


Sorry but the new diff was included to the patch.. I can see the updates as 
Diff 176328. Could you please check and let me know if it does not work? Thank 
you.


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

https://reviews.llvm.org/D54945



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

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:601
 
+static bool compareToks(Token &T1, Token &T2, const SourceManager &SM) {
+  if (T1.getLength() != T2.getLength())

Should this function compare token kinds first?



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:602
+static bool compareToks(Token &T1, Token &T2, const SourceManager &SM) {
+  if (T1.getLength() != T2.getLength())
+return false;

`Token::getLength()` will assert-fail for annotation tokens.



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:605
+  return strncmp(SM.getCharacterData(T1.getLocation()),
+ SM.getCharacterData(T2.getLocation()), T1.getLength()) == 0;
+}

JonasToth wrote:
> This operation could overflow if `len(T1) > len(T2)` and `T2` is the last 
> token of the file. I think `std::min(T1.getLength(), T2.getLength())` would 
> be better.
This code is only executed when they have the same length.


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

https://reviews.llvm.org/D55125



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

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

[Mikael Nilsson via Phabricator via cfe-commits]

mikael updated this revision to Diff 176596.

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

https://reviews.llvm.org/D54862

Files:
  include/clang/AST/CanonicalType.h
  include/clang/AST/DeclCXX.h
  include/clang/AST/Type.h
  lib/AST/ASTContext.cpp
  lib/AST/ASTDumper.cpp
  lib/AST/DeclCXX.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/AST/Type.cpp
  lib/AST/TypePrinter.cpp
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGValue.h
  lib/Index/USRGeneration.cpp
  lib/Parse/ParseCXXInlineMethods.cpp
  lib/Sema/SemaCodeComplete.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/SemaType.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/CodeGenOpenCLCXX/addrspace-of-this.cl
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -8370,7 +8370,7 @@
   const Decl *D = cxcursor::getCursorDecl(C);
   const CXXMethodDecl *Method =
   D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
-  return (Method && (Method->getTypeQualifiers() & Qualifiers::Const)) ? 1 : 0;
+  return (Method && Method->getTypeQualifiers().hasConst()) ? 1 : 0;
 }
 
 unsigned clang_CXXMethod_isDefaulted(CXCursor C) {
Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -0,0 +1,129 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -pedantic -verify -O0 -o - | FileCheck %s
+// expected-no-diagnostics
+
+// Test that the 'this' pointer is in the __generic address space.
+
+// FIXME: Add support for __constant address space.
+
+class C {
+public:
+  int v;
+  C() { v = 2; }
+  C(const C &c) { v = c.v; }
+  C &operator=(const C &c) {
+v = c.v;
+return *this;
+  }
+  int get() { return v; }
+};
+
+__global C c;
+
+__kernel void test__global() {
+  int i = c.get();
+  C c1(c);
+  C c2;
+  c2 = c1;
+}
+
+// CHECK-LABEL: @__cxx_global_var_init()
+// CHECK: call void @_ZN1CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*)) #4
+
+// Test that the address space is __generic for the constructor
+// CHECK-LABEL: @_ZN1CC1Ev(%class.C addrspace(4)* %this)
+// CHECK: entry:
+// CHECK:   %this.addr = alloca %class.C addrspace(4)*, align 4
+// CHECK:   store %class.C addrspace(4)* %this, %class.C addrspace(4)** %this.addr, align 4
+// CHECK:   %this1 = load %class.C addrspace(4)*, %class.C addrspace(4)** %this.addr, align 4
+// CHECK:   call void @_ZN1CC2Ev(%class.C addrspace(4)* %this1) #4
+// CHECK:   ret void
+
+// CHECK-LABEL: @_Z12test__globalv()
+
+// Test the address space of 'this' when invoking a method.
+// CHECK: %call = call i32 @_ZN1C3getEv(%class.C addrspace(4)* addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*))
+
+// Test the address space of 'this' when invoking copy-constructor.
+// CHECK: %0 = addrspacecast %class.C* %c1 to %class.C addrspace(4)*
+// CHECK: call void @_ZN1CC1ERU3AS4KS_(%class.C addrspace(4)* %0, %class.C addrspace(4)* dereferenceable(4) addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*))
+
+// Test the address space of 'this' when invoking a constructor.
+// CHECK:   %1 = addrspacecast %class.C* %c2 to %class.C addrspace(4)*
+// CHECK:   call void @_ZN1CC1Ev(%class.C addrspace(4)* %1) #4
+
+// Test the address space of 'this' when invoking assignment operator.
+// CHECK:   %2 = addrspacecast %class.C* %c1 to %class.C addrspace(4)*
+// CHECK:   %3 = addrspacecast %class.C* %c2 to %class.C addrspace(4)*
+// CHECK:   %call1 = call dereferenceable(4) %class.C addrspace(4)* @_ZN1CaSERU3AS4KS_(%class.C addrspace(4)* %3, %class.C addrspace(4)* dereferenceable(4) %2)
+
+#define TEST(AS) \
+  __kernel void test##AS() { \
+AS C c;  \
+int i = c.get(); \
+C c1(c); \
+C c2;\
+c2 = c1; \
+  }
+
+TEST(__local)
+
+// CHECK-LABEL: _Z11test__localv
+// CHECK: @__cxa_guard_acquire
+
+// Test the address space of 'this' when invoking a method.
+// CHECK: call void @_ZN1CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+
+// Test the address space of 'this' when invoking copy-constructor.
+// CHECK: %call = call i32 @_ZN1C3getEv(%class.C addrspace(4)* addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+
+// Test the address space of 'this' when invoking a constructor.
+// CHECK: %3 = addrspacecas

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

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D54945#1318283 , @MarinaKalashina 
wrote:

> In D54945#1318278 , @alexfh wrote:
>
> > In D54945#1316251 , 
> > @MarinaKalashina wrote:
> >
> > > Fixes:
> > >
> > > - empty line before 'Standalone tool'
> > > - table columns with '+/-' aligned
> > > - line width limited to 80 (except for the table)
> > >
> > >   Additions:
> > > - clang-tidy-vs plugin
> > > - Clangd in the intro, the table, and CLion's paragraph
> >
> >
> > Did you forget to add a new file to the patch?
> >
> > Please also include full context into the diff. See 
> > https://llvm.org/docs/Phabricator.html
>
>
> Sorry but the new diff was included to the patch.. I can see the updates as 
> Diff 176328. Could you please check and let me know if it does not work? 
> Thank you.


Please see the screenshot. The diff might be reversed (new vs. old) or just 
wrong. And there's no context (not overly important in this particular patch, 
but still makes it more convenient to review).
F7647494: Screenshot from 2018-12-04 13-04-11.jpg 



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

https://reviews.llvm.org/D54945



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

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

[Henry Wong via Phabricator via cfe-commits]

MTC added a comment.

Please add more context using the `-U` option, like `git diff -U9 ...`.

In D55226#1317119 , @Pierre-vh wrote:

> Sadly I'm not experienced enough to think of every case that should pass this 
> check, so I limited myself to just fixing the bug.
>  Can't we totally remove the outer if so we allow every `Target` expression 
> that has a `ConstantArrayType` to pass this check?


IMHO, it's fine to remove the outer check!


Repository:
  rC Clang

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

https://reviews.llvm.org/D55226



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

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

[Kristüf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: test/Analysis/ctu-main.cpp:6
 // 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
+// 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 
-analyzer-display-ctu-progress 2>&1 %s | FileCheck %s
+

martong wrote:
> martong wrote:
> > Szelethus wrote:
> > > I think these RUN lines would really benefit from introducing line breaks.
> > Yes, I agree. Unfortunately, I could not figure out how to break them. 
> > Using a simple "\" at around the 80th column gives `Test has unterminated 
> > run lines (with '\')`. If I use block comments with "\" the same happens. 
> > If I use block comments and don't use the "\" then the second line is not 
> > interpreted. Is it really possible to break RUN lines? I could not find 
> > anything about that in the online docs.
> Oh, I just have found your other comment to the other patch. So yes, it is 
> indeed possible to break this line. I updated the patch accordingly. The 
> other long lines which are already there I am going to change in an 
> independent patch: (https://reviews.llvm.org/D55129).
Is there any particular reason why we use `%clang_cc1 -analyze` as opposed to 
`%clang_analyze_cc1`? If not, lets change it.

By the way, thanks! This looks neat.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55135



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

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

[Marina Kalashina via Phabricator via cfe-commits]

MarinaKalashina updated this revision to Diff 176601.
MarinaKalashina added a comment.

Full-context patch for the changes introduced in the previous commit.


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

https://reviews.llvm.org/D54945

Files:
  docs/clang-tidy/index.rst


Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -403,7 +403,7 @@
 
 // Silent only the specified diagnostics for the next line
 // NOLINTNEXTLINE(google-explicit-constructor, google-runtime-int)
-Foo(bool param);
+Foo(bool param); 
   };
 
 The formal syntax of ``NOLINT``/``NOLINTNEXTLINE`` is the following:


Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -403,7 +403,7 @@
 
 // Silent only the specified diagnostics for the next line
 // NOLINTNEXTLINE(google-explicit-constructor, google-runtime-int)
-Foo(bool param);
+Foo(bool param); 
   };
 
 The formal syntax of ``NOLINT``/``NOLINTNEXTLINE`` is the following:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaType.cpp:4816
+// OpenCLCPlusPlus: A class member function has an address space.
+if (state.getSema().getLangOpts().OpenCLCPlusPlus &&
+state.getDeclarator().getContext() ==

Can this be moved into `deduceOpenCLImplicitAddrSpace`?


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

https://reviews.llvm.org/D54862



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

[PATCH] D55255: Fix a false positive in misplaced-widening-cast

[Daniel Krupp via Phabricator via cfe-commits]

dkrupp updated this revision to Diff 176604.
dkrupp added a comment.

Comments addressed. Please commit if looks good, I don't have commit rights.
Thanks.


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

https://reviews.llvm.org/D55255

Files:
  clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
  test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp


Index: test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp
===
--- test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp
+++ test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp
@@ -62,3 +62,21 @@
   enum Type {};
   static char *m_fn1() { char p = (Type)(&p - m_fn1()); }
 };
+
+enum DaysEnum {
+  MON,
+  TUE,
+  WED,
+  THR,
+  FRI,
+  SAT,
+  SUN
+};
+
+// Do not warn for int to enum casts.
+void nextDay(DaysEnum day) {
+  if (day < SUN)
+day = (DaysEnum)(day + 1);
+  if (day < SUN)
+day = static_cast(day + 1);
+}
Index: clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
===
--- clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
+++ clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
@@ -213,8 +213,9 @@
 dyn_cast(CastType->getUnqualifiedDesugaredType());
 const auto *CalcBuiltinType =
 dyn_cast(CalcType->getUnqualifiedDesugaredType());
-if (CastBuiltinType && CalcBuiltinType &&
-!isFirstWider(CastBuiltinType->getKind(), CalcBuiltinType->getKind()))
+if (!CastBuiltinType || !CalcBuiltinType)
+  return;
+if (!isFirstWider(CastBuiltinType->getKind(), CalcBuiltinType->getKind()))
   return;
   }
 


Index: test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp
===
--- test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp
+++ test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp
@@ -62,3 +62,21 @@
   enum Type {};
   static char *m_fn1() { char p = (Type)(&p - m_fn1()); }
 };
+
+enum DaysEnum {
+  MON,
+  TUE,
+  WED,
+  THR,
+  FRI,
+  SAT,
+  SUN
+};
+
+// Do not warn for int to enum casts.
+void nextDay(DaysEnum day) {
+  if (day < SUN)
+day = (DaysEnum)(day + 1);
+  if (day < SUN)
+day = static_cast(day + 1);
+}
Index: clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
===
--- clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
+++ clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
@@ -213,8 +213,9 @@
 dyn_cast(CastType->getUnqualifiedDesugaredType());
 const auto *CalcBuiltinType =
 dyn_cast(CalcType->getUnqualifiedDesugaredType());
-if (CastBuiltinType && CalcBuiltinType &&
-!isFirstWider(CastBuiltinType->getKind(), CalcBuiltinType->getKind()))
+if (!CastBuiltinType || !CalcBuiltinType)
+  return;
+if (!isFirstWider(CastBuiltinType->getKind(), CalcBuiltinType->getKind()))
   return;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55257: Inline handling of DependentSizedArrayType

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

> It is necessary to perform all printing before any traversal to child nodes.

This piqued my interest -- is `VisitFunctionDecl()` then incorrect because it 
streams output, then dumps parameter children, then dumps more output, then 
dumps override children? Or do you mean "don't interleave `VisitFoo()` calls 
with streaming output"?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55257



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

[PATCH] D55127: [OpenCL] Diagnose conflicting address spaces between template definition and its instantiation

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

> Can you explain why the changes to `TreeTransform` are required?

When address spaces of template parameter and argument are given explicitly 
they might mismatch. For example, `foo1` (in the test) is instantiated with  
`__local int` but the template parameter of `foo1` is  `__global T`. Currently 
clang fails to compile this with ICE while rebuilding the type that has 
multiple inconsistent addr space qualifiers. My patch fixes it, by giving an 
error instead of attempting to rebuild the type, that the addr space qualifiers 
mismatch.

There is another example of the same issue - `foo3`, but with non-pointer type.


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

https://reviews.llvm.org/D55127



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

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

[Mikael Nilsson via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaType.cpp:4816
+// OpenCLCPlusPlus: A class member function has an address space.
+if (state.getSema().getLangOpts().OpenCLCPlusPlus &&
+state.getDeclarator().getContext() ==

Anastasia wrote:
> Can this be moved into `deduceOpenCLImplicitAddrSpace`?
deduceOpenCLImplicitAddrSpace is only wrapping a type with an address space. I 
also need to make sure the FunctionProtoType gets generated with the address 
space.


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

https://reviews.llvm.org/D54862



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

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

[Marina Kalashina via Phabricator via cfe-commits]

MarinaKalashina added a comment.

@alexfh Thanks a lot for your patience and help. I've made another revision, 
now with the diff made by 'git show HEAD -U99' to have the full context 
availlable.


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

https://reviews.llvm.org/D54945



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

[PATCH] D37726: clang: alias -static-{libstdc++, libgcc} for LLVM variants

[Martell Malone via Phabricator via cfe-commits]

martell abandoned this revision.
martell added a comment.

Dropping in favour of D53238 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D37726



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

[PATCH] D53238: [Driver] Add -static-{rtlib, stdlib} and make -static-{libgcc, libstdc++} their aliases

[Martell Malone via Phabricator via cfe-commits]

martell added a comment.

Seems like a better solution than what I had in D37726 
.
I'm not sully sure on the naming so I think someone else should also sign off 
on this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53238



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

[PATCH] D53238: [Driver] Add -static-{rtlib, stdlib} and make -static-{libgcc, libstdc++} their aliases

[Martell Malone via Phabricator via cfe-commits]

martell added a comment.

Also,  Sorry for the long time to reply, I don't LLVM often enough :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D53238



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

[PATCH] D55007: [Analyzer] Constraint Manager - Calculate Effective Range for Differences

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

baloghadamsoftware added a comment.

My original idea was that once we ony store either A - B or B - A. Thus if we 
already have A - B stored then do not store range for B - A but negate both the 
difference and the range. I can think on two ways to implement this:

1. Create a separate function e.g. `setRange()` to store the range. This 
function checks whether the symbol is a difference and whether we already have 
a range for its negated. If so, then negate the difference and the range as 
well. ( We do not need to intersect them because the caller already did it.) 
However, in this case we negate twice: once in `getRange()` then once in 
`setRange()`.

2. Move the negation out of `getRange()` and call check for a stored negated 
difference before calling it. If it exist then call the appropriate assume 
function for the negated difference (for `==` and `!=` it is the same function, 
but reverse the operator for the rest).

Your idea (store either `A - B` or `B - A` based on their symbol ID is also 
feasible but then we also face the same question. So 1) or 2)?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55007



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

[PATCH] D55257: Inline handling of DependentSizedArrayType

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D55257#1318328 , @aaron.ballman 
wrote:

> > It is necessary to perform all printing before any traversal to child nodes.
>
> This piqued my interest -- is `VisitFunctionDecl()` then incorrect because it 
> streams output, then dumps parameter children, then dumps more output, then 
> dumps override children? Or do you mean "don't interleave `VisitFoo()` calls 
> with streaming output"?


Can you relate your question to https://reviews.llvm.org/D55083 ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55257



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

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus requested changes to this revision.
Szelethus added a comment.
This revision now requires changes to proceed.

The code not directly related to adding the actual flag looks great, but this 
still should be an `-analyzer-config` option.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55135



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

[PATCH] D55129: [CTU] Eliminate race condition in CTU lit tests

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

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

After the review comment is resolved, the rest LGTM!


Repository:
  rC Clang

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

https://reviews.llvm.org/D55129



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

[PATCH] D55132: [CTU] Add asserts to protect invariants

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

xazax.hun accepted this revision.
xazax.hun added inline comments.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:249
 CrossTranslationUnitContext::importDefinition(const FunctionDecl *FD) {
+  assert(FD->hasBody() && "Functions to be imported should have body.");
+

I wonder if this actually is true. While it is not supported now, we 
could/might use this to import/merge attributes even without definition. 
But of course, I have no hard feelings about having this assert now and remove 
it later on once other use cases are actually supported.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55132



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

[PATCH] D50256: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager (for == and != only)

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

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

Thank you for reviewing this patch!




Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:576-597
+  RangeSet New = getRange(St, Sym);
+  for (llvm::APSInt I = AdjustmentType.getZeroValue();
+  I < (Scale.Reciprocal ? AdjustmentType.getValue(1) : Scale.Val); ++I) {
+
+llvm::APSInt ScInt = AdjInt;
+if (!rescale(ScInt, Scale, I))
+  continue;

NoQ wrote:
> I believe that this code should be moved directly into `getRange()`. If it's 
> about looking at a single symbol and figuring out what range information 
> about it do we already have, it should go into `getRange()`. This way we 
> don't need to duplicate it in all the `assume...()` functions, and also it's 
> exactly what `getRange()` is supposed to accomplish.
`getRange()` retrieves the existing range for the symbol. However, similarly to 
the `Adjustment` we use the `Scale` to change the right side of the relation, 
not the left one.

I also dislike code multiplication. Maybe we should use the Strategy pattern 
here and create a function that does the loop. However, if you take a look at 
D49074 you will see that the body of the loop may look quite different.



Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:616-617
+  RangeSet New = F.getEmptySet();
+  for (llvm::APSInt I = AdjustmentType.getZeroValue();
+  I < (Scale.Reciprocal ? AdjustmentType.getValue(1) : Scale.Val); ++I) {
+  

NoQ wrote:
> Mmm, what if `Scale.Val` is vry large?
That is a real problem. We either have to limit this functionality for small 
numbers (for the short term, maybe) or find a better algorithm (for the long 
term).


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

https://reviews.llvm.org/D50256



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

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaType.cpp:4816
+// OpenCLCPlusPlus: A class member function has an address space.
+if (state.getSema().getLangOpts().OpenCLCPlusPlus &&
+state.getDeclarator().getContext() ==

mikael wrote:
> Anastasia wrote:
> > Can this be moved into `deduceOpenCLImplicitAddrSpace`?
> deduceOpenCLImplicitAddrSpace is only wrapping a type with an address space. 
> I also need to make sure the FunctionProtoType gets generated with the 
> address space.
FunctionProtoType is a type too. Does it go into deduceOpenCLImplicitAddrSpace 
when creating this?


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

https://reviews.llvm.org/D54862



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

[clang-tools-extra] r348262 - Fix "array must be initialized with a brace-enclosed initializer" build error.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Tue Dec  4 06:07:29 2018
New Revision: 348262

URL: http://llvm.org/viewvc/llvm-project?rev=348262&view=rev
Log:
Fix "array must be initialized with a brace-enclosed initializer" build error.

Try to fix clang-bpf-build buildbot.

Modified:
clang-tools-extra/trunk/clangd/Headers.h

Modified: clang-tools-extra/trunk/clangd/Headers.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Headers.h?rev=348262&r1=348261&r2=348262&view=diff
==
--- clang-tools-extra/trunk/clangd/Headers.h (original)
+++ clang-tools-extra/trunk/clangd/Headers.h Tue Dec  4 06:07:29 2018
@@ -55,7 +55,7 @@ struct IncludeGraphNode {
   // True if current file is a main file rather than a header.
   bool IsTU = false;
   llvm::StringRef URI;
-  FileDigest Digest{0};
+  FileDigest Digest{{0}};
   std::vector DirectIncludes;
 };
 // FileURI and FileInclusions are references to keys of the map containing


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

[PATCH] D55131: [CTU] Add more lit tests and better error handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176619.
martong marked 8 inline comments as done.
martong added a comment.

- Break long RUN lines
- Clang format the test files
- Use consistent naming style
- Remove braces


Repository:
  rC Clang

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

https://reviews.llvm.org/D55131

Files:
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/Inputs/ctu-other.c
  test/Analysis/Inputs/externalFnMap2.txt
  test/Analysis/ctu-main.c

Index: test/Analysis/ctu-main.c
===
--- /dev/null
+++ test/Analysis/ctu-main.c
@@ -0,0 +1,67 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir2
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir2/ctu-other.c.ast %S/Inputs/ctu-other.c
+// RUN: cp %S/Inputs/externalFnMap2.txt %t/ctudir2/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -std=c89 -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir2 \
+// RUN:   -verify %s
+
+void clang_analyzer_eval(int);
+
+// Test typedef and global variable in function.
+typedef struct {
+  int a;
+  int b;
+} FooBar;
+extern FooBar fb;
+int f(int);
+void testGlobalVariable() {
+  clang_analyzer_eval(f(5) == 1); // expected-warning{{TRUE}}
+}
+
+// Test enums.
+int enumCheck(void);
+enum A { x,
+ y,
+ z };
+void testEnum() {
+  clang_analyzer_eval(x == 0);// expected-warning{{TRUE}}
+  clang_analyzer_eval(enumCheck() == 42); // expected-warning{{TRUE}}
+}
+
+// Test that asm import does not fail.
+int inlineAsm();
+int testInlineAsm() {
+  return inlineAsm();
+}
+
+// Test reporting error in a macro.
+struct S;
+int g(struct S *);
+void testMacro(void) {
+  g(0); // expected-warning@Inputs/ctu-other.c:29 {{Access to field 'a' results in a dereference of a null pointer (loaded from variable 'ctx')}}
+}
+
+// The external function prototype is incomplete.
+// warning:implicit functions are prohibited by c99
+void testImplicit() {
+  int res = identImplicit(6);   // external implicit functions are not inlined
+  clang_analyzer_eval(res == 6); // expected-warning{{TRUE}}
+}
+
+// Tests the import of functions that have a struct parameter
+// defined in its prototype.
+struct DataType {
+  int a;
+  int b;
+};
+int structInProto(struct DataType *d);
+void testStructDefInArgument() {
+  struct DataType d;
+  d.a = 1;
+  d.b = 0;
+  clang_analyzer_eval(structInProto(&d) == 0); // expected-warning{{TRUE}} expected-warning{{FALSE}}
+}
Index: test/Analysis/Inputs/externalFnMap2.txt
===
--- /dev/null
+++ test/Analysis/Inputs/externalFnMap2.txt
@@ -0,0 +1,6 @@
+c:@F@inlineAsm ctu-other.c.ast
+c:@F@g ctu-other.c.ast
+c:@F@f ctu-other.c.ast
+c:@F@enumCheck ctu-other.c.ast
+c:@F@identImplicit ctu-other.c.ast
+c:@F@structInProto ctu-other.c.ast
Index: test/Analysis/Inputs/ctu-other.c
===
--- /dev/null
+++ test/Analysis/Inputs/ctu-other.c
@@ -0,0 +1,49 @@
+// Test typedef and global variable in function.
+typedef struct {
+  int a;
+  int b;
+} FooBar;
+FooBar fb;
+int f(int i) {
+  if (fb.a) {
+fb.b = i;
+  }
+  return 1;
+}
+
+// Test enums.
+enum B { x = 42,
+ l,
+ s };
+int enumCheck(void) {
+  return x;
+}
+
+// Test reporting an error in macro definition
+#define MYMACRO(ctx) \
+  ctx->a;
+struct S {
+  int a;
+};
+int g(struct S *ctx) {
+  MYMACRO(ctx);
+  return 0;
+}
+
+// Test that asm import does not fail.
+int inlineAsm() {
+  int res;
+  asm("mov $42, %0"
+  : "=r"(res));
+  return res;
+}
+
+// Implicit function.
+int identImplicit(int in) {
+  return in;
+}
+
+// ASTImporter doesn't support this construct.
+int structInProto(struct DataType {int a;int b; } * d) {
+  return 0;
+}
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -247,7 +247,9 @@
 CrossTranslationUnitContext::importDefinition(const FunctionDecl *FD) {
   ASTImporter &Importer = getOrCreateASTImporter(FD->getASTContext());
   auto *ToDecl =
-  cast(Importer.Import(const_cast(FD)));
+  cast_or_null(Importer.Import(const_cast(FD)));
+  if (!ToDecl)
+return llvm::make_error(index_error_code::failed_import);
   assert(ToDecl->hasBody());
   assert(FD->hasBody() && "Functions already imported should have body.");
   return ToDecl;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55131: [CTU] Add more lit tests and better error handling

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: test/Analysis/Inputs/ctu-other.c:6
+  int b;
+} foobar;
+

a_sidorin wrote:
> Please use a consistent naming style across the file. There are names 
> starting with capital, having underscores and written like this.
Ok, I renamed the things, I was trying to be consistent with the LLVM style, 
one exception is the name of variables still start with small case.
I also restructured the file a bit and added more comments, so it is more 
cleaner which things belong together to the same test.



Comment at: test/Analysis/ctu-main.c:3-5
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o 
%t/ctudir2/ctu-other.c.ast %S/Inputs/ctu-other.c
+// RUN: cp %S/Inputs/externalFnMap2.txt %t/ctudir2/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -std=c89 -analyze 
-analyzer-checker=core,debug.ExprInspection  -analyzer-config 
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%t/ctudir2 
-verify %s

Szelethus wrote:
> Could you please break these lines?
> ```
> // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu %S/Inputs/ctu-other.c \
> // RUN:-emit-pch -o %t/ctudir2/ctu-other.c.ast
> //
> // RUN: cp %S/Inputs/externalFnMap2.txt %t/ctudir2/externalFnMap.txt
> //
> // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -std=c89 
> -analyze -verify %s
> // RUN:   -analyzer-checker=core \
> // RUN:   -analyzer-checker=debug.ExprInspection \
> // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
> // RUN:   -analyzer-config ctu-dir=%t/ctudir2
> ```
Ok, good point.



Comment at: test/Analysis/ctu-main.c:4
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o 
%t/ctudir2/ctu-other.c.ast %S/Inputs/ctu-other.c
+// RUN: cp %S/Inputs/externalFnMap2.txt %t/ctudir2/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -std=c89 -analyze 
-analyzer-checker=core,debug.ExprInspection  -analyzer-config 
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%t/ctudir2 
-verify %s

Szelethus wrote:
> This is a question rather than anything else, why do we have both 
> externalFnMap2.txt and externalFnMap.txt?
`externalFnMap.txt` goes for `ctu-other.cpp`.
`externalFnMap2.txt` goes for `ctu-other.c`.
Perhaps we should rename them in the `Inputs` directory to include the indexed 
file name. E.g. `ctu-other.cpp.externalFnMap.txt`. What do you think?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55131



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

[PATCH] D55131: [CTU] Add more lit tests and better error handling

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> Also, maybe it'd be worth making a CTU directory under test/Analysis for CTU 
> related test files.

It is a good point, but I'd do that in the future once we have even more ctu 
related test files. Perhaps together with a new check-clang-analysis-ctu build 
target.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55131



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

r348268 - Remove reference to recently removed PTH Documentation.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Tue Dec  4 06:46:25 2018
New Revision: 348268

URL: http://llvm.org/viewvc/llvm-project?rev=348268&view=rev
Log:
Remove reference to recently removed PTH Documentation.

Removed in r348266

Change-Id: Icff0212f57c42ca84ec174ddd4366ae63a7923fa

Modified:
cfe/trunk/docs/index.rst

Modified: cfe/trunk/docs/index.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/index.rst?rev=348268&r1=348267&r2=348268&view=diff
==
--- cfe/trunk/docs/index.rst (original)
+++ cfe/trunk/docs/index.rst Tue Dec  4 06:46:25 2018
@@ -83,7 +83,6 @@ Design Documents
 
InternalsManual
DriverInternals
-   PTHInternals
PCHInternals
ItaniumMangleAbiTags
HardwareAssistedAddressSanitizerDesign.rst


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

[PATCH] D55133: [CTU] Add statistics

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

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

The code LGTM! I am not good at wordsmithing, but if the descriptions of the 
statistics are not clear enough, I agree that they should be rephrased.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55133



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

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

xazax.hun requested changes to this revision.
xazax.hun added inline comments.
This revision now requires changes to proceed.



Comment at: include/clang/Basic/DiagnosticCrossTUKinds.td:19
+
+def err_ctu_incompat_triple : Error<
+  "imported AST from '%0' had been generated for a different target, current: 
%1, imported: %2">;

I am not sure if we want this to be an error. The analysis could continue 
without any problems if we do not actually merge the two ASTs. So maybe having 
a warning only is better. My concern is that once we have this error, there 
would be no way to analyze mixed language (C/C++) projects cleanly using CTU.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:212
+// diagnostics.
+Context.getDiagnostics().Report(diag::err_ctu_incompat_triple)
+<< Unit->getMainFileName() << TripleTo.str() << TripleFrom.str();

I think we should not emit an error here. It should be up to the caller (the 
user of the library) to decide if she wants to handle this as an error, 
warnings, or just suppress these kinds of problems. I would rather extend 
`emitCrossTUDiagnostics` as a shorthand for the user if emitting an error is 
the desired behavior.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

r348271 - [OPENMP][NVPTX]Mark __kmpc_barrier functions as convergent.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Dec  4 07:03:25 2018
New Revision: 348271

URL: http://llvm.org/viewvc/llvm-project?rev=348271&view=rev
Log:
[OPENMP][NVPTX]Mark __kmpc_barrier functions as convergent.

__kmpc_barrier runtime functions must be marked as convergent to prevent
some dangerous optimizations. Also, for NVPTX target all barriers must
be emitted as simple barriers.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
cfe/trunk/test/OpenMP/nvptx_parallel_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=348271&r1=348270&r2=348271&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Tue Dec  4 07:03:25 2018
@@ -3214,13 +3214,7 @@ void CGOpenMPRuntime::emitOrderedRegion(
   emitInlinedDirective(CGF, OMPD_ordered, OrderedOpGen);
 }
 
-void CGOpenMPRuntime::emitBarrierCall(CodeGenFunction &CGF, SourceLocation Loc,
-  OpenMPDirectiveKind Kind, bool 
EmitChecks,
-  bool ForceSimpleCall) {
-  if (!CGF.HaveInsertPoint())
-return;
-  // Build call __kmpc_cancel_barrier(loc, thread_id);
-  // Build call __kmpc_barrier(loc, thread_id);
+unsigned CGOpenMPRuntime::getDefaultFlagsForBarriers(OpenMPDirectiveKind Kind) 
{
   unsigned Flags;
   if (Kind == OMPD_for)
 Flags = OMP_IDENT_BARRIER_IMPL_FOR;
@@ -3232,6 +3226,17 @@ void CGOpenMPRuntime::emitBarrierCall(Co
 Flags = OMP_IDENT_BARRIER_EXPL;
   else
 Flags = OMP_IDENT_BARRIER_IMPL;
+  return Flags;
+}
+
+void CGOpenMPRuntime::emitBarrierCall(CodeGenFunction &CGF, SourceLocation Loc,
+  OpenMPDirectiveKind Kind, bool 
EmitChecks,
+  bool ForceSimpleCall) {
+  if (!CGF.HaveInsertPoint())
+return;
+  // Build call __kmpc_cancel_barrier(loc, thread_id);
+  // Build call __kmpc_barrier(loc, thread_id);
+  unsigned Flags = getDefaultFlagsForBarriers(Kind);
   // Build call __kmpc_cancel_barrier(loc, thread_id) or __kmpc_barrier(loc,
   // thread_id);
   llvm::Value *Args[] = {emitUpdateLocation(CGF, Loc, Flags),

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h?rev=348271&r1=348270&r2=348271&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h Tue Dec  4 07:03:25 2018
@@ -290,6 +290,10 @@ protected:
   /// default location.
   virtual unsigned getDefaultLocationReserved2Flags() const { return 0; }
 
+  /// Returns default flags for the barriers depending on the directive, for
+  /// which this barier is going to be emitted.
+  static unsigned getDefaultFlagsForBarriers(OpenMPDirectiveKind Kind);
+
   /// Get the LLVM type for the critical name.
   llvm::ArrayType *getKmpCriticalNameTy() const {return KmpCriticalNameTy;}
 

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=348271&r1=348270&r2=348271&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Tue Dec  4 07:03:25 2018
@@ -96,6 +96,8 @@ enum OpenMPRTLFunctionNVPTX {
   OMPRTL_NVPTX__kmpc_get_team_static_memory,
   /// Call to void __kmpc_restore_team_static_memory(int16_t is_shared);
   OMPRTL_NVPTX__kmpc_restore_team_static_memory,
+  // Call to void __kmpc_barrier(ident_t *loc, kmp_int32 global_tid);
+  OMPRTL__kmpc_barrier,
 };
 
 /// Pre(post)-action for different OpenMP constructs specialized for NVPTX.
@@ -1824,6 +1826,15 @@ CGOpenMPRuntimeNVPTX::createNVPTXRuntime
 CGM.CreateRuntimeFunction(FnTy, "__kmpc_restore_team_static_memory");
 break;
   }
+  case OMPRTL__kmpc_barrier: {
+// Build void __kmpc_barrier(ident_t *loc, kmp_int32 global_tid);
+llvm::Type *TypeParams[] = {getIdentTyPointerTy(), CGM.Int32Ty};
+auto *FnTy =
+llvm::FunctionType::get(CGM.VoidTy, TypeParams, /*isVarArg*/ false);
+RTLFn = CGM.CreateRuntimeFunction(FnTy, /*Name*/ "__kmpc_barrier");
+cast(RTLFn)->addFnAttr(llvm::Attribute::Convergent);
+break;
+  }
   }
   return RTLFn;
 }
@@ -2676,6 +2687,20 @@ void CGOpenMPRuntimeNVPTX::emitSPMDParal
   }
 }
 
+void CGOpenMPRuntimeNVPTX::emitBarrierCall(CodeGenFunction &CGF,
+   SourceLocation Loc,
+   OpenMPDirectiveKind Kind, bool,
+

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

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

xazax.hun added a comment.

Having an analyzer config option makes sense.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55135



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

[PATCH] D55270: [Sema] Further improvements to to static_assert diagnostics.

[Clement Courbet via Phabricator via cfe-commits]

courbet created this revision.
courbet added reviewers: aaron.ballman, Quuxplusone.

We're now handling cases like `static_assert(!expr)` and
static_assert(!(expr))`.


Repository:
  rC Clang

https://reviews.llvm.org/D55270

Files:
  lib/Sema/SemaTemplate.cpp
  test/SemaCXX/static-assert.cpp


Index: test/SemaCXX/static-assert.cpp
===
--- test/SemaCXX/static-assert.cpp
+++ test/SemaCXX/static-assert.cpp
@@ -111,6 +111,10 @@
 // expected-error@-1{{static_assert failed due to requirement 
'std::is_same::value' "message"}}
 static_assert(std::is_const::value, "message");
 // expected-error@-1{{static_assert failed due to requirement 
'std::is_const::value' "message"}}
+static_assert(!std::is_const::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 
'!std::is_const::value' "message"}}
+static_assert(!(std::is_const::value), "message");
+// expected-error@-1{{static_assert failed due to requirement 
'!(std::is_const::value)' "message"}}
 
 struct BI_tag {};
 struct RAI_tag : BI_tag {};
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -3052,27 +3052,41 @@
   return Cond;
 }
 
-// Print a diagnostic for the failing static_assert expression. Defaults to
-// pretty-printing the expression.
-static void prettyPrintFailedBooleanCondition(llvm::raw_string_ostream &OS,
-  const Expr *FailedCond,
-  const PrintingPolicy &Policy) {
-  const auto *DR = dyn_cast(FailedCond);
-  if (DR && DR->getQualifier()) {
-// If this is a qualified name, expand the template arguments in nested
-// qualifiers.
-DR->getQualifier()->print(OS, Policy, true);
-// Then print the decl itself.
-const ValueDecl *VD = DR->getDecl();
-OS << VD->getName();
-if (const auto *IV = dyn_cast(VD)) {
-  // This is a template variable, print the expanded template arguments.
-  printTemplateArgumentList(OS, IV->getTemplateArgs().asArray(), Policy);
+namespace {
+
+// A PrinterHelper that prints more helpful diagnostics for some 
sub-expressions
+// within failing boolean expression, such as substituting template parameters
+// for actual types.
+class FailedBooleanConditionPrinterHelper : public PrinterHelper {
+public:
+  FailedBooleanConditionPrinterHelper(const PrintingPolicy &Policy)
+  : Policy(Policy) {}
+
+  ~FailedBooleanConditionPrinterHelper() override {}
+
+  bool handledStmt(Stmt *E, raw_ostream &OS) override {
+const auto *DR = dyn_cast(E);
+if (DR && DR->getQualifier()) {
+  // If this is a qualified name, expand the template arguments in nested
+  // qualifiers.
+  DR->getQualifier()->print(OS, Policy, true);
+  // Then print the decl itself.
+  const ValueDecl *VD = DR->getDecl();
+  OS << VD->getName();
+  if (const auto *IV = dyn_cast(VD)) {
+// This is a template variable, print the expanded template arguments.
+printTemplateArgumentList(OS, IV->getTemplateArgs().asArray(), Policy);
+  }
+  return true;
 }
-return;
+return false;
   }
-  FailedCond->printPretty(OS, nullptr, Policy);
-}
+
+private:
+  const PrintingPolicy &Policy;
+};
+
+} // namespace
 
 std::pair
 Sema::findFailedBooleanCondition(Expr *Cond, bool AllowTopLevelCond) {
@@ -3115,7 +3129,8 @@
   std::string Description;
   {
 llvm::raw_string_ostream Out(Description);
-prettyPrintFailedBooleanCondition(Out, FailedCond, getPrintingPolicy());
+FailedBooleanConditionPrinterHelper Helper(getPrintingPolicy());
+FailedCond->printPretty(Out, &Helper, getPrintingPolicy());
   }
   return { FailedCond, Description };
 }


Index: test/SemaCXX/static-assert.cpp
===
--- test/SemaCXX/static-assert.cpp
+++ test/SemaCXX/static-assert.cpp
@@ -111,6 +111,10 @@
 // expected-error@-1{{static_assert failed due to requirement 'std::is_same::value' "message"}}
 static_assert(std::is_const::value, "message");
 // expected-error@-1{{static_assert failed due to requirement 'std::is_const::value' "message"}}
+static_assert(!std::is_const::value, "message");
+// expected-error@-1{{static_assert failed due to requirement '!std::is_const::value' "message"}}
+static_assert(!(std::is_const::value), "message");
+// expected-error@-1{{static_assert failed due to requirement '!(std::is_const::value)' "message"}}
 
 struct BI_tag {};
 struct RAI_tag : BI_tag {};
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -3052,27 +3052,41 @@
   return Cond;
 }
 
-// Print a diagnostic for the failing static_assert expression. Defaults to
-// pretty-printing the expression.
-static void prettyPrintFailedBooleanCondition(llvm

[PATCH] D55269: [CUDA][OpenMP] Fix nvidia-cuda-toolkit detection on Debian/Ubuntu

[Joel E. Denny via Phabricator via cfe-commits]

jdenny created this revision.
jdenny added reviewers: ABataev, Hahnfeld, tra, sylvestre.ledru.
Herald added a subscriber: guansong.

D40453  (r319317) was meant to handle 
nvidia-cuda-toolkit's split CUDA
installation in Debian.  This patch addresses two issues left
unresolved by D40453 , at least on my Ubuntu 
18.04.1 installation:

1. IsDebian() doesn't include IsUbuntu(), so D40453 
 doesn't help me.

2. When `--cuda-path=/usr` is meant to find the nvidia-cuda-toolkit 
installation, the split installation isn't handled.

Issue 2 occurs when I follow step 4 in the following guide to rebuild
the OpenMP runtime with clang to build bitcode libraries:

https://www.hahnjo.de/blog/2018/10/08/clang-7.0-openmp-offloading-nvidia.html

The reason is that
`libomptarget/cmake/Modules/LibomptargetNVPTXBitcodeLibrary.cmake`
then specifies `--cuda-path=/usr` when testing clang, but it would
need to leave `--cuda-path` unspecified to trigger D40453 
's fix.
Perhaps that cmake file could be adjusted instead, but the fix in this
patch is more general.

Obviously, this isn't the first time this issue has been discussed,
but googling didn't reveal to me a definitive answer on the path
forward.  Proposing this patch seems like the easiest way to ask.


Repository:
  rC Clang

https://reviews.llvm.org/D55269

Files:
  clang/lib/Driver/ToolChains/Cuda.cpp


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -80,9 +80,19 @@
   // In decreasing order so we prefer newer versions to older versions.
   std::initializer_list Versions = {"8.0", "7.5", "7.0"};
 
+  // Special case for Debian to have nvidia-cuda-toolkit work
+  // out of the box. More info on http://bugs.debian.org/882505
+  const char *NvidiaCudaToolkit =
+  (Distro(D.getVFS()).IsDebian() || Distro(D.getVFS()).IsUbuntu())
+  ? "/usr/lib/cuda"
+  : nullptr;
+
   if (Args.hasArg(clang::driver::options::OPT_cuda_path_EQ)) {
-Candidates.emplace_back(
-Args.getLastArgValue(clang::driver::options::OPT_cuda_path_EQ).str());
+std::string CudaPath =
+Args.getLastArgValue(clang::driver::options::OPT_cuda_path_EQ).str();
+Candidates.emplace_back(CudaPath);
+if (NvidiaCudaToolkit && CudaPath == "/usr")
+  Candidates.emplace_back(D.SysRoot + NvidiaCudaToolkit);
   } else if (HostTriple.isOSWindows()) {
 for (const char *Ver : Versions)
   Candidates.emplace_back(
@@ -114,10 +124,8 @@
 for (const char *Ver : Versions)
   Candidates.emplace_back(D.SysRoot + "/usr/local/cuda-" + Ver);
 
-if (Distro(D.getVFS()).IsDebian())
-  // Special case for Debian to have nvidia-cuda-toolkit work
-  // out of the box. More info on http://bugs.debian.org/882505
-  Candidates.emplace_back(D.SysRoot + "/usr/lib/cuda");
+if (NvidiaCudaToolkit)
+  Candidates.emplace_back(D.SysRoot + NvidiaCudaToolkit);
   }
 
   bool NoCudaLib = Args.hasArg(options::OPT_nocudalib);


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -80,9 +80,19 @@
   // In decreasing order so we prefer newer versions to older versions.
   std::initializer_list Versions = {"8.0", "7.5", "7.0"};
 
+  // Special case for Debian to have nvidia-cuda-toolkit work
+  // out of the box. More info on http://bugs.debian.org/882505
+  const char *NvidiaCudaToolkit =
+  (Distro(D.getVFS()).IsDebian() || Distro(D.getVFS()).IsUbuntu())
+  ? "/usr/lib/cuda"
+  : nullptr;
+
   if (Args.hasArg(clang::driver::options::OPT_cuda_path_EQ)) {
-Candidates.emplace_back(
-Args.getLastArgValue(clang::driver::options::OPT_cuda_path_EQ).str());
+std::string CudaPath =
+Args.getLastArgValue(clang::driver::options::OPT_cuda_path_EQ).str();
+Candidates.emplace_back(CudaPath);
+if (NvidiaCudaToolkit && CudaPath == "/usr")
+  Candidates.emplace_back(D.SysRoot + NvidiaCudaToolkit);
   } else if (HostTriple.isOSWindows()) {
 for (const char *Ver : Versions)
   Candidates.emplace_back(
@@ -114,10 +124,8 @@
 for (const char *Ver : Versions)
   Candidates.emplace_back(D.SysRoot + "/usr/local/cuda-" + Ver);
 
-if (Distro(D.getVFS()).IsDebian())
-  // Special case for Debian to have nvidia-cuda-toolkit work
-  // out of the box. More info on http://bugs.debian.org/882505
-  Candidates.emplace_back(D.SysRoot + "/usr/lib/cuda");
+if (NvidiaCudaToolkit)
+  Candidates.emplace_back(D.SysRoot + NvidiaCudaToolkit);
   }
 
   bool NoCudaLib = Args.hasArg(options::OPT_nocudalib);
___
cfe-commits mailing list
cfe-commits@lists.llvm.or

r348272 - [OPENMP][NVPTX]Fixed emission of the critical region.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Dec  4 07:25:01 2018
New Revision: 348272

URL: http://llvm.org/viewvc/llvm-project?rev=348272&view=rev
Log:
[OPENMP][NVPTX]Fixed emission of the critical region.

Critical regions in NVPTX are the constructs, which, generally speaking,
are not supported by the NVPTX target. Instead we're using special
technique to handle the critical regions. Currently they are supported
only within the loop and all the threads in the loop must execute the
same critical region.
Inside of this special regions the regions still must be emitted as
critical, to avoid possible data races between the teams +
synchronization must use __kmpc_barrier functions.

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

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=348272&r1=348271&r2=348272&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Tue Dec  4 07:25:01 2018
@@ -2743,14 +2743,16 @@ void CGOpenMPRuntimeNVPTX::emitCriticalR
   CGF.EmitBlock(BodyBB);
 
   // Output the critical statement.
-  CriticalOpGen(CGF);
+  CGOpenMPRuntime::emitCriticalRegion(CGF, CriticalName, CriticalOpGen, Loc,
+  Hint);
 
   // After the body surrounded by the critical region, the single executing
   // thread will jump to the synchronisation point.
   // Block waits for all threads in current team to finish then increments the
   // counter variable and returns to the loop.
   CGF.EmitBlock(SyncBB);
-  getNVPTXCTABarrier(CGF);
+  emitBarrierCall(CGF, Loc, OMPD_unknown, /*EmitChecks=*/false,
+  /*ForceSimpleCall=*/true);
 
   llvm::Value *IncCounterVal =
   CGF.Builder.CreateNSWAdd(CounterVal, CGF.Builder.getInt32(1));

Modified: cfe/trunk/test/OpenMP/nvptx_parallel_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_parallel_codegen.cpp?rev=348272&r1=348271&r2=348272&view=diff
==
--- cfe/trunk/test/OpenMP/nvptx_parallel_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_parallel_codegen.cpp Tue Dec  4 07:25:01 2018
@@ -356,7 +356,13 @@ int bar(int n){
 // CHECK:  [[RES:%.+]] = icmp eq i32 [[TID]], [[CC_VAL]]
 // CHECK:  br i1 [[RES]], label
 
-// CHECK:  call void @llvm.nvvm.barrier0()
+// CHECK:  call void @__kmpc_critical(
+// CHECK:  load i32, i32*
+// CHECK:  add nsw i32
+// CHECK:  store i32
+// CHECK:  call void @__kmpc_end_critical(
+
+// CHECK:  call void @__kmpc_barrier(%struct.ident_t* @{{.+}}, i32 %{{.+}})
 // CHECK:  [[NEW_CC_VAL:%.+]] = add nsw i32 [[CC_VAL]], 1
 // CHECK:  store i32 [[NEW_CC_VAL]], i32* [[CC]],
 // CHECK:  br label


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

[PATCH] D53701: [Analyzer] Record and process comparison of symbols instead of iterator positions in interator checkers

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

baloghadamsoftware added a comment.

In D53701#1315242 , @NoQ wrote:

> I think it's worth a try to do a total evalCall at first, and then disable 
> evalCall (together with the attempt to model the iterator position) in an 
> incrementally growing blacklist of cases (1. iterator adaptors, 2. ) as 
> we encounter problems. This essentially becomes part of the logic that 
> decides whether an object is an iterator. Eg., if it's more like an adaptor 
> than an actual iterator, let's treat it as if it's not an iterator, but 
> inline instead, and hope that we model the underlying iterators correctly via 
> evalCall.


I think that only tracking the inner iterator and treating the outer iterator 
as a non-iterator is a nightmare from the user's perspective: all detections 
seem to be "internal" errors of the underlying API and thus regarded as 
"probably false positives". When using iterator adapters of the STL the bugs 
may also be filtered out by the analyzer if this option is enabled. The user 
must see the errors on the topmost level whenever possible to understand them.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53701



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

Re: r348131 - [AST] Fix an uninitialized bug in the bits of FunctionDecl

[Bruno Ricci via cfe-commits]

There is two reasons for this change:

1.) The first one is that the bit FunctionDeclBits.IsCopyDeductionCandidate
is only used by CXXDeductionGuideDecl and so there is no getter/setter
for it in FunctionDecl, and it would not make sense to add one.
This is a legacy of when these bits where stored in FunctionDecl itself.

2.) The second one is that some of these setter do a little bit more than
initializing the appropriate bit. For example setInlineSpecified sets both
FunctionDeclBits.IsInlineSpecified and FunctionDeclBits.IsInline, but a 
quick
reading of the body of the constructor of FunctionDecl would lead someone to
believe that FunctionDeclBits.IsInline is not initialized.

However these are not strong reasons, and I can revert it to use the setters
instead if preferred. My apologies if it would have been preferable to review
it first. It seemed obvious to me and I was familiar with this piece of code
since I did the change a few month ago, but I am still trying to fine tune
this decision process.

Cheers,

Bruno

On 03/12/2018 22:01, David Blaikie wrote:
> Why the change from using setter functions to direct assignment?
> 
> On Mon, Dec 3, 2018 at 5:06 AM Bruno Ricci via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> 
> Author: brunoricci
> Date: Mon Dec  3 05:04:10 2018
> New Revision: 348131
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=348131&view=rev
> Log:
> [AST] Fix an uninitialized bug in the bits of FunctionDecl
> 
> FunctionDeclBits.IsCopyDeductionCandidate was not initialized.
> This caused a warning with valgrind.
> 
> 
> Modified:
>     cfe/trunk/lib/AST/Decl.cpp
> 
> Modified: cfe/trunk/lib/AST/Decl.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=348131&r1=348130&r2=348131&view=diff
> 
> ==
> --- cfe/trunk/lib/AST/Decl.cpp (original)
> +++ cfe/trunk/lib/AST/Decl.cpp Mon Dec  3 05:04:10 2018
> @@ -2653,27 +2653,29 @@ FunctionDecl::FunctionDecl(Kind DK, ASTC
>        DeclContext(DK), redeclarable_base(C), ODRHash(0),
>        EndRangeLoc(NameInfo.getEndLoc()), DNLoc(NameInfo.getInfo()) {
>    assert(T.isNull() || T->isFunctionType());
> -  setStorageClass(S);
> -  setInlineSpecified(isInlineSpecified);
> -  setExplicitSpecified(false);
> -  setVirtualAsWritten(false);
> -  setPure(false);
> -  setHasInheritedPrototype(false);
> -  setHasWrittenPrototype(true);
> -  setDeletedAsWritten(false);
> -  setTrivial(false);
> -  setTrivialForCall(false);
> -  setDefaulted(false);
> -  setExplicitlyDefaulted(false);
> -  setHasImplicitReturnZero(false);
> -  setLateTemplateParsed(false);
> -  setConstexpr(isConstexprSpecified);
> -  setInstantiationIsPending(false);
> -  setUsesSEHTry(false);
> -  setHasSkippedBody(false);
> -  setWillHaveBody(false);
> -  setIsMultiVersion(false);
> -  setHasODRHash(false);
> +  FunctionDeclBits.SClass = S;
> +  FunctionDeclBits.IsInline = isInlineSpecified;
> +  FunctionDeclBits.IsInlineSpecified = isInlineSpecified;
> +  FunctionDeclBits.IsExplicitSpecified = false;
> +  FunctionDeclBits.IsVirtualAsWritten = false;
> +  FunctionDeclBits.IsPure = false;
> +  FunctionDeclBits.HasInheritedPrototype = false;
> +  FunctionDeclBits.HasWrittenPrototype = true;
> +  FunctionDeclBits.IsDeleted = false;
> +  FunctionDeclBits.IsTrivial = false;
> +  FunctionDeclBits.IsTrivialForCall = false;
> +  FunctionDeclBits.IsDefaulted = false;
> +  FunctionDeclBits.IsExplicitlyDefaulted = false;
> +  FunctionDeclBits.HasImplicitReturnZero = false;
> +  FunctionDeclBits.IsLateTemplateParsed = false;
> +  FunctionDeclBits.IsConstexpr = isConstexprSpecified;
> +  FunctionDeclBits.InstantiationIsPending = false;
> +  FunctionDeclBits.UsesSEHTry = false;
> +  FunctionDeclBits.HasSkippedBody = false;
> +  FunctionDeclBits.WillHaveBody = false;
> +  FunctionDeclBits.IsMultiVersion = false;
> +  FunctionDeclBits.IsCopyDeductionCandidate = false;
> +  FunctionDeclBits.HasODRHash = false;
>  }
> 
>  void FunctionDecl::getNameForDiagnostic(
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org 
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348276 - [AST][NFC] Make ArrayTypeTraitExpr non polymorphic

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Tue Dec  4 08:01:24 2018
New Revision: 348276

URL: http://llvm.org/viewvc/llvm-project?rev=348276&view=rev
Log:
[AST][NFC] Make ArrayTypeTraitExpr non polymorphic

ArrayTypeTraitExpr is the only expression class which is polymorphic.
As far as I can tell this is completely pointless.

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

Reviewed By: aaron.ballman


Modified:
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/lib/AST/ExprCXX.cpp

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=348276&r1=348275&r2=348276&view=diff
==
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Tue Dec  4 08:01:24 2018
@@ -2455,8 +2455,6 @@ class ArrayTypeTraitExpr : public Expr {
   /// The type being queried.
   TypeSourceInfo *QueriedType = nullptr;
 
-  virtual void anchor();
-
 public:
   friend class ASTStmtReader;
 
@@ -2474,8 +2472,6 @@ public:
   explicit ArrayTypeTraitExpr(EmptyShell Empty)
   : Expr(ArrayTypeTraitExprClass, Empty), ATT(0) {}
 
-  virtual ~ArrayTypeTraitExpr() = default;
-
   SourceLocation getBeginLoc() const LLVM_READONLY { return Loc; }
   SourceLocation getEndLoc() const LLVM_READONLY { return RParen; }
 

Modified: cfe/trunk/lib/AST/ExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprCXX.cpp?rev=348276&r1=348275&r2=348276&view=diff
==
--- cfe/trunk/lib/AST/ExprCXX.cpp (original)
+++ cfe/trunk/lib/AST/ExprCXX.cpp Tue Dec  4 08:01:24 2018
@@ -1443,5 +1443,3 @@ TypeTraitExpr *TypeTraitExpr::CreateDese
   void *Mem = C.Allocate(totalSizeToAlloc(NumArgs));
   return new (Mem) TypeTraitExpr(EmptyShell());
 }
-
-void ArrayTypeTraitExpr::anchor() {}


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

[PATCH] D55221: [AST] Make ArrayTypeTraitExpr non-polymorphic.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348276: [AST][NFC] Make ArrayTypeTraitExpr non polymorphic 
(authored by brunoricci, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55221?vs=176405&id=176640#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55221

Files:
  cfe/trunk/include/clang/AST/ExprCXX.h
  cfe/trunk/lib/AST/ExprCXX.cpp


Index: cfe/trunk/lib/AST/ExprCXX.cpp
===
--- cfe/trunk/lib/AST/ExprCXX.cpp
+++ cfe/trunk/lib/AST/ExprCXX.cpp
@@ -1443,5 +1443,3 @@
   void *Mem = C.Allocate(totalSizeToAlloc(NumArgs));
   return new (Mem) TypeTraitExpr(EmptyShell());
 }
-
-void ArrayTypeTraitExpr::anchor() {}
Index: cfe/trunk/include/clang/AST/ExprCXX.h
===
--- cfe/trunk/include/clang/AST/ExprCXX.h
+++ cfe/trunk/include/clang/AST/ExprCXX.h
@@ -2455,8 +2455,6 @@
   /// The type being queried.
   TypeSourceInfo *QueriedType = nullptr;
 
-  virtual void anchor();
-
 public:
   friend class ASTStmtReader;
 
@@ -2474,8 +2472,6 @@
   explicit ArrayTypeTraitExpr(EmptyShell Empty)
   : Expr(ArrayTypeTraitExprClass, Empty), ATT(0) {}
 
-  virtual ~ArrayTypeTraitExpr() = default;
-
   SourceLocation getBeginLoc() const LLVM_READONLY { return Loc; }
   SourceLocation getEndLoc() const LLVM_READONLY { return RParen; }
 


Index: cfe/trunk/lib/AST/ExprCXX.cpp
===
--- cfe/trunk/lib/AST/ExprCXX.cpp
+++ cfe/trunk/lib/AST/ExprCXX.cpp
@@ -1443,5 +1443,3 @@
   void *Mem = C.Allocate(totalSizeToAlloc(NumArgs));
   return new (Mem) TypeTraitExpr(EmptyShell());
 }
-
-void ArrayTypeTraitExpr::anchor() {}
Index: cfe/trunk/include/clang/AST/ExprCXX.h
===
--- cfe/trunk/include/clang/AST/ExprCXX.h
+++ cfe/trunk/include/clang/AST/ExprCXX.h
@@ -2455,8 +2455,6 @@
   /// The type being queried.
   TypeSourceInfo *QueriedType = nullptr;
 
-  virtual void anchor();
-
 public:
   friend class ASTStmtReader;
 
@@ -2474,8 +2472,6 @@
   explicit ArrayTypeTraitExpr(EmptyShell Empty)
   : Expr(ArrayTypeTraitExprClass, Empty), ATT(0) {}
 
-  virtual ~ArrayTypeTraitExpr() = default;
-
   SourceLocation getBeginLoc() const LLVM_READONLY { return Loc; }
   SourceLocation getEndLoc() const LLVM_READONLY { return RParen; }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348278 - [AST] Assert that no statement/expression class is polymorphic

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Tue Dec  4 08:04:19 2018
New Revision: 348278

URL: http://llvm.org/viewvc/llvm-project?rev=348278&view=rev
Log:
[AST] Assert that no statement/expression class is polymorphic

Add a static_assert checking that no statement/expression class
is polymorphic. People should use LLVM style RTTI instead.

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

Reviewed By: aaron.ballman


Modified:
cfe/trunk/lib/AST/Stmt.cpp

Modified: cfe/trunk/lib/AST/Stmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Stmt.cpp?rev=348278&r1=348277&r2=348278&view=diff
==
--- cfe/trunk/lib/AST/Stmt.cpp (original)
+++ cfe/trunk/lib/AST/Stmt.cpp Tue Dec  4 08:04:19 2018
@@ -76,6 +76,14 @@ const char *Stmt::getStmtClassName() con
   return getStmtInfoTableEntry((StmtClass) StmtBits.sClass).Name;
 }
 
+// Check that no statement / expression class is polymorphic. LLVM style RTTI
+// should be used instead. If absolutely needed an exception can still be added
+// here by defining the appropriate macro (but please don't do this).
+#define STMT(CLASS, PARENT) \
+  static_assert(!std::is_polymorphic::value, \
+#CLASS " should not be polymorphic!");
+#include "clang/AST/StmtNodes.inc"
+
 void Stmt::PrintStats() {
   // Ensure the table is primed.
   getStmtInfoTableEntry(Stmt::NullStmtClass);


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

[PATCH] D55222: [AST] Assert that no statement/expression class is polymorphic

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC348278: [AST] Assert that no statement/expression class is 
polymorphic (authored by brunoricci, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D55222

Files:
  lib/AST/Stmt.cpp


Index: lib/AST/Stmt.cpp
===
--- lib/AST/Stmt.cpp
+++ lib/AST/Stmt.cpp
@@ -76,6 +76,14 @@
   return getStmtInfoTableEntry((StmtClass) StmtBits.sClass).Name;
 }
 
+// Check that no statement / expression class is polymorphic. LLVM style RTTI
+// should be used instead. If absolutely needed an exception can still be added
+// here by defining the appropriate macro (but please don't do this).
+#define STMT(CLASS, PARENT) \
+  static_assert(!std::is_polymorphic::value, \
+#CLASS " should not be polymorphic!");
+#include "clang/AST/StmtNodes.inc"
+
 void Stmt::PrintStats() {
   // Ensure the table is primed.
   getStmtInfoTableEntry(Stmt::NullStmtClass);


Index: lib/AST/Stmt.cpp
===
--- lib/AST/Stmt.cpp
+++ lib/AST/Stmt.cpp
@@ -76,6 +76,14 @@
   return getStmtInfoTableEntry((StmtClass) StmtBits.sClass).Name;
 }
 
+// Check that no statement / expression class is polymorphic. LLVM style RTTI
+// should be used instead. If absolutely needed an exception can still be added
+// here by defining the appropriate macro (but please don't do this).
+#define STMT(CLASS, PARENT) \
+  static_assert(!std::is_polymorphic::value, \
+#CLASS " should not be polymorphic!");
+#include "clang/AST/StmtNodes.inc"
+
 void Stmt::PrintStats() {
   // Ensure the table is primed.
   getStmtInfoTableEntry(Stmt::NullStmtClass);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55269: [CUDA][OpenMP] Fix nvidia-cuda-toolkit detection on Debian/Ubuntu

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

Ah, this packaging style is a never ending source of fun... Adding `isUbuntu()` 
seems fine to solve the first issue.

For the second point I'd instead propose to fix `FindCuda.cmake`: It's wrong to 
report `/usr` if the shim tree is in `/usr/lib/cuda`. @sylvestre.ledru what's 
your take on this, looking from a Debian perspective?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55269



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

r348280 - Revert "Avoid emitting redundant or unusable directories in DIFile metadata entries."

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Dec  4 08:30:45 2018
New Revision: 348280

URL: http://llvm.org/viewvc/llvm-project?rev=348280&view=rev
Log:
Revert "Avoid emitting redundant or unusable directories in DIFile metadata 
entries."

This reverts commit r348154 and follow-up commits r348211 and r3248213.
Reason: the original commit broke compiler-rt tests and a follow-up fix
(r348203) broke our integrate and was reverted.

Removed:
cfe/trunk/test/CodeGen/debug-info-abspath.c
Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CodeGenAction.cpp
cfe/trunk/test/CodeGen/debug-prefix-map.c
cfe/trunk/test/Modules/module-debuginfo-prefix.m

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=348280&r1=348279&r2=348280&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Dec  4 08:30:45 2018
@@ -181,7 +181,8 @@ void CGDebugInfo::setLocation(SourceLoca
   SourceManager &SM = CGM.getContext().getSourceManager();
   auto *Scope = cast(LexicalBlockStack.back());
   PresumedLoc PCLoc = SM.getPresumedLoc(CurLoc);
-  if (PCLoc.isInvalid() || Scope->getFile() == getOrCreateFile(CurLoc))
+
+  if (PCLoc.isInvalid() || Scope->getFilename() == PCLoc.getFilename())
 return;
 
   if (auto *LBF = dyn_cast(Scope)) {
@@ -409,13 +410,13 @@ llvm::DIFile *CGDebugInfo::getOrCreateFi
   SourceManager &SM = CGM.getContext().getSourceManager();
   PresumedLoc PLoc = SM.getPresumedLoc(Loc);
 
-  StringRef FileName = PLoc.getFilename();
-  if (PLoc.isInvalid() || FileName.empty())
+  if (PLoc.isInvalid() || StringRef(PLoc.getFilename()).empty())
 // If the location is not valid then use main input file.
 return getOrCreateMainFile();
 
   // Cache the results.
-  auto It = DIFileCache.find(FileName.data());
+  const char *fname = PLoc.getFilename();
+  auto It = DIFileCache.find(fname);
 
   if (It != DIFileCache.end()) {
 // Verify that the information still exists.
@@ -430,41 +431,11 @@ llvm::DIFile *CGDebugInfo::getOrCreateFi
   if (CSKind)
 CSInfo.emplace(*CSKind, Checksum);
 
-  StringRef Dir;
-  StringRef File;
-  std::string RemappedFile = remapDIPath(FileName);
-  std::string CurDir = remapDIPath(getCurrentDirname());
-  SmallString<128> DirBuf;
-  SmallString<128> FileBuf;
-  if (llvm::sys::path::is_absolute(RemappedFile)) {
-// Strip the common prefix (if it is more than just "/") from current
-// directory and FileName for a more space-efficient encoding.
-auto FileIt = llvm::sys::path::begin(RemappedFile);
-auto FileE = llvm::sys::path::end(RemappedFile);
-auto CurDirIt = llvm::sys::path::begin(CurDir);
-auto CurDirE = llvm::sys::path::end(CurDir);
-for (; CurDirIt != CurDirE && *CurDirIt == *FileIt; ++CurDirIt, ++FileIt)
-  llvm::sys::path::append(DirBuf, *CurDirIt);
-if (std::distance(llvm::sys::path::begin(CurDir), CurDirIt) == 1) {
-  // The common prefix only the root; stripping it would cause
-  // LLVM diagnostic locations to be more confusing.
-  Dir = {};
-  File = RemappedFile;
-} else {
-  for (; FileIt != FileE; ++FileIt)
-llvm::sys::path::append(FileBuf, *FileIt);
-  Dir = DirBuf;
-  File = FileBuf;
-}
-  } else {
-Dir = CurDir;
-File = RemappedFile;
-  }
-  llvm::DIFile *F =
-  DBuilder.createFile(File, Dir, CSInfo,
-  getSource(SM, SM.getFileID(Loc)));
+  llvm::DIFile *F = DBuilder.createFile(
+  remapDIPath(PLoc.getFilename()), remapDIPath(getCurrentDirname()), 
CSInfo,
+  getSource(SM, SM.getFileID(Loc)));
 
-  DIFileCache[FileName.data()].reset(F);
+  DIFileCache[fname].reset(F);
   return F;
 }
 

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=348280&r1=348279&r2=348280&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Tue Dec  4 08:30:45 2018
@@ -549,16 +549,12 @@ const FullSourceLoc BackendConsumer::get
   SourceLocation DILoc;
 
   if (D.isLocationAvailable()) {
-D.getLocation(Filename, Line, Column);
-if (Line > 0) {
-  const FileEntry *FE = FileMgr.getFile(Filename);
-  if (!FE)
-FE = FileMgr.getFile(D.getAbsolutePath());
-  if (FE) {
-// If -gcolumn-info was not used, Column will be 0. This upsets the
-// source manager, so pass 1 if Column is not set.
-DILoc = SourceMgr.translateFileLineCol(FE, Line, Column ? Column : 1);
-  }
+D.getLocation(&Filename, &Line, &Column);
+const FileEntry *FE = FileMgr.getFile(Filename);
+if (FE && Line > 0) {
+  // If -gcolumn-info was not used, Column will be 0. This upsets the
+  // source m

[PATCH] D55275: [clangd] Dont provide locations for non-existent files.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

We were getting assertion errors when we had bad file names, instead we
should skip those.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55275

Files:
  clangd/index/SymbolCollector.cpp
  unittests/clangd/SymbolCollectorTests.cpp


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -1028,6 +1028,15 @@
   AllOf(QName("Public"), Not(ImplementationDetail();
 }
 
+TEST_F(SymbolCollectorTest, RenameSymbolWithMacro) {
+  const std::string Header = R"(
+  void foo() {}
+  )";
+  runSymbolCollector(Header, /*Main=*/"", {"-Dfoo=bar"});
+  EXPECT_THAT(Symbols, UnorderedElementsAre(QName("bar")));
+}
+
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -211,7 +211,10 @@
   const SymbolCollector::Options &Opts,
   const clang::LangOptions &LangOpts,
   std::string &FileURIStorage) {
-  auto U = toURI(SM, SM.getFilename(TokLoc), Opts);
+  llvm::StringRef FileName = SM.getFilename(TokLoc);
+  if (FileName.empty())
+return None;
+  auto U = toURI(SM, FileName, Opts);
   if (!U)
 return None;
   FileURIStorage = std::move(*U);


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -1028,6 +1028,15 @@
   AllOf(QName("Public"), Not(ImplementationDetail();
 }
 
+TEST_F(SymbolCollectorTest, RenameSymbolWithMacro) {
+  const std::string Header = R"(
+  void foo() {}
+  )";
+  runSymbolCollector(Header, /*Main=*/"", {"-Dfoo=bar"});
+  EXPECT_THAT(Symbols, UnorderedElementsAre(QName("bar")));
+}
+
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -211,7 +211,10 @@
   const SymbolCollector::Options &Opts,
   const clang::LangOptions &LangOpts,
   std::string &FileURIStorage) {
-  auto U = toURI(SM, SM.getFilename(TokLoc), Opts);
+  llvm::StringRef FileName = SM.getFilename(TokLoc);
+  if (FileName.empty())
+return None;
+  auto U = toURI(SM, FileName, Opts);
   if (!U)
 return None;
   FileURIStorage = std::move(*U);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r348154 - Avoid emitting redundant or unusable directories in DIFile metadata entries.

[Ilya Biryukov via cfe-commits]

Hi!
I had to revert the fix (r348203) in r348280 as this broke our integrate.
Also reverted the original change to avoid having compiler-rt in a broken
state.
Sorry for the inconvenience, see the r348203 thread for more details.


On Tue, Dec 4, 2018 at 12:14 AM Adrian Prantl via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Should be fixed in r348211.
>
> -- adrian
>
> > On Dec 3, 2018, at 3:07 PM, Adrian Prantl via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >
> > No, your failures are Windows-specific (/ vs \), and I haven't fixed
> them yet. Thanks for letting me know!
> >
> > -- adrian
> >
> >> On Dec 3, 2018, at 3:06 PM, Galina Kistanova 
> wrote:
> >>
> >> Adrian, did not see your response, please ignore my email.
> >>
> >> Thanks
> >>
> >> Galina
> >>
> >> On Mon, Dec 3, 2018 at 3:04 PM Galina Kistanova 
> wrote:
> >> Hello Adrian,
> >>
> >> This commit broke tests on couple of our builders:
> >>
> >>
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/14371
> >>
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast
> >>
> >> . . .
> >> Failing Tests (2):
> >> Clang :: CodeGen/debug-prefix-map.c
> >> Clang :: Modules/module-debuginfo-prefix.m
> >>
> >> The builders were already red and no notifications were sent on this.
> >> Please have a look?
> >>
> >> Thanks
> >>
> >> Galina
> >>
> >> On Mon, Dec 3, 2018 at 9:58 AM Adrian Prantl via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >> Author: adrian
> >> Date: Mon Dec  3 09:55:27 2018
> >> New Revision: 348154
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=348154&view=rev
> >> Log:
> >> Avoid emitting redundant or unusable directories in DIFile metadata
> entries.
> >>
> >> As discussed on llvm-dev recently, Clang currently emits redundant
> >> directories in DIFile entries, such as
> >>
> >>   .file  1 "/Volumes/Data/llvm"
> "/Volumes/Data/llvm/tools/clang/test/CodeGen/debug-info-abspath.c"
> >>
> >> This patch looks at any common prefix between the compilation
> >> directory and the (absolute) file path and strips the redundant
> >> part. More importantly it leaves the compilation directory empty if
> >> the two paths have no common prefix.
> >>
> >> After this patch the above entry is (assuming a compilation dir of
> "/Volumes/Data/llvm/_build"):
> >>
> >>   .file 1 "/Volumes/Data/llvm"
> "tools/clang/test/CodeGen/debug-info-abspath.c"
> >>
> >> When building the FileCheck binary with debug info, this patch makes
> >> the build artifacts ~1kb smaller.
> >>
> >> Differential Revision: https://reviews.llvm.org/D55085
> >>
> >> Added:
> >> cfe/trunk/test/CodeGen/debug-info-abspath.c
> >> Modified:
> >> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> >> cfe/trunk/lib/CodeGen/CodeGenAction.cpp
> >> cfe/trunk/test/CodeGen/debug-prefix-map.c
> >> cfe/trunk/test/Modules/module-debuginfo-prefix.m
> >>
> >> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> >> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=348154&r1=348153&r2=348154&view=diff
> >>
> ==
> >> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> >> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Dec  3 09:55:27 2018
> >> @@ -181,8 +181,7 @@ void CGDebugInfo::setLocation(SourceLoca
> >>SourceManager &SM = CGM.getContext().getSourceManager();
> >>auto *Scope = cast(LexicalBlockStack.back());
> >>PresumedLoc PCLoc = SM.getPresumedLoc(CurLoc);
> >> -
> >> -  if (PCLoc.isInvalid() || Scope->getFilename() == PCLoc.getFilename())
> >> +  if (PCLoc.isInvalid() || Scope->getFile() == getOrCreateFile(CurLoc))
> >>  return;
> >>
> >>if (auto *LBF = dyn_cast(Scope)) {
> >> @@ -410,13 +409,13 @@ llvm::DIFile *CGDebugInfo::getOrCreateFi
> >>SourceManager &SM = CGM.getContext().getSourceManager();
> >>PresumedLoc PLoc = SM.getPresumedLoc(Loc);
> >>
> >> -  if (PLoc.isInvalid() || StringRef(PLoc.getFilename()).empty())
> >> +  StringRef FileName = PLoc.getFilename();
> >> +  if (PLoc.isInvalid() || FileName.empty())
> >>  // If the location is not valid then use main input file.
> >>  return getOrCreateMainFile();
> >>
> >>// Cache the results.
> >> -  const char *fname = PLoc.getFilename();
> >> -  auto It = DIFileCache.find(fname);
> >> +  auto It = DIFileCache.find(FileName.data());
> >>
> >>if (It != DIFileCache.end()) {
> >>  // Verify that the information still exists.
> >> @@ -431,11 +430,41 @@ llvm::DIFile *CGDebugInfo::getOrCreateFi
> >>if (CSKind)
> >>  CSInfo.emplace(*CSKind, Checksum);
> >>
> >> -  llvm::DIFile *F = DBuilder.createFile(
> >> -  remapDIPath(PLoc.getFilename()),
> remapDIPath(getCurrentDirname()), CSInfo,
> >> -  getSource(SM, SM.getFileID(Loc)));
> >> +  StringRef Dir;
> >> +  StringRef File;
> >> +  std::string RemappedFile = remapDIPath(FileName);
> >> +  std::string CurDir = remapDIPath(get

r348281 - [AST] Assert that no type class is polymorphic

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Tue Dec  4 08:36:28 2018
New Revision: 348281

URL: http://llvm.org/viewvc/llvm-project?rev=348281&view=rev
Log:
[AST] Assert that no type class is polymorphic

Add a static_assert checking that no type class is polymorphic.
People should use LLVM style RTTI instead.

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

Reviewed By: aaron.ballman


Modified:
cfe/trunk/lib/AST/Type.cpp

Modified: cfe/trunk/lib/AST/Type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=348281&r1=348280&r2=348281&view=diff
==
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Tue Dec  4 08:36:28 2018
@@ -291,6 +291,14 @@ QualType QualType::getSingleStepDesugare
   return Context.getQualifiedType(desugar, split.Quals);
 }
 
+// Check that no type class is polymorphic. LLVM style RTTI should be used
+// instead. If absolutely needed an exception can still be added here by
+// defining the appropriate macro (but please don't do this).
+#define TYPE(CLASS, BASE) \
+  static_assert(!std::is_polymorphic::value, \
+#CLASS "Type should not be polymorphic!");
+#include "clang/AST/TypeNodes.def"
+
 QualType Type::getLocallyUnqualifiedSingleStepDesugaredType() const {
   switch (getTypeClass()) {
 #define ABSTRACT_TYPE(Class, Parent)


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

[PATCH] D55225: [AST] Assert that no type class is polymorphic

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348281: [AST] Assert that no type class is polymorphic 
(authored by brunoricci, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55225?vs=176414&id=176649#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55225

Files:
  cfe/trunk/lib/AST/Type.cpp


Index: cfe/trunk/lib/AST/Type.cpp
===
--- cfe/trunk/lib/AST/Type.cpp
+++ cfe/trunk/lib/AST/Type.cpp
@@ -291,6 +291,14 @@
   return Context.getQualifiedType(desugar, split.Quals);
 }
 
+// Check that no type class is polymorphic. LLVM style RTTI should be used
+// instead. If absolutely needed an exception can still be added here by
+// defining the appropriate macro (but please don't do this).
+#define TYPE(CLASS, BASE) \
+  static_assert(!std::is_polymorphic::value, \
+#CLASS "Type should not be polymorphic!");
+#include "clang/AST/TypeNodes.def"
+
 QualType Type::getLocallyUnqualifiedSingleStepDesugaredType() const {
   switch (getTypeClass()) {
 #define ABSTRACT_TYPE(Class, Parent)


Index: cfe/trunk/lib/AST/Type.cpp
===
--- cfe/trunk/lib/AST/Type.cpp
+++ cfe/trunk/lib/AST/Type.cpp
@@ -291,6 +291,14 @@
   return Context.getQualifiedType(desugar, split.Quals);
 }
 
+// Check that no type class is polymorphic. LLVM style RTTI should be used
+// instead. If absolutely needed an exception can still be added here by
+// defining the appropriate macro (but please don't do this).
+#define TYPE(CLASS, BASE) \
+  static_assert(!std::is_polymorphic::value, \
+#CLASS "Type should not be polymorphic!");
+#include "clang/AST/TypeNodes.def"
+
 QualType Type::getLocallyUnqualifiedSingleStepDesugaredType() const {
   switch (getTypeClass()) {
 #define ABSTRACT_TYPE(Class, Parent)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55006: [clang] - Simplify tools::SplitDebugName

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added subscribers: labath, klimek.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good to me - tagging @labath @klimek here in case they have some further 
context on whether this is the right place for the test. But I'm fine with them 
providing post-commit review - please go ahead with this change as-is.


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

https://reviews.llvm.org/D55006



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

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176653.
martong added a comment.

- Use clang_analyze_cc1
- Change to be an analyzer config option


Repository:
  rC Clang

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

https://reviews.llvm.org/D55135

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/ctu-main.cpp

Index: test/Analysis/ctu-main.cpp
===
--- test/Analysis/ctu-main.cpp
+++ test/Analysis/ctu-main.cpp
@@ -3,6 +3,14 @@
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o %T/ctudir/ctu-chain.cpp.ast %S/Inputs/ctu-chain.cpp
 // 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
+// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%T/ctudir \
+// RUN:   -analyzer-config display-ctu-progress=true 2>&1 %s | FileCheck %s
+
+// CHECK: ANALYZE CTU loaded AST file: {{.*}}/ctu-other.cpp
+// CHECK: ANALYZE CTU loaded AST file: {{.*}}/ctu-chain.cpp
 
 #include "ctu-hdr.h"
 
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -21,6 +21,7 @@
 // CHECK-NEXT: crosscheck-with-z3 = false
 // CHECK-NEXT: ctu-dir = ""
 // CHECK-NEXT: ctu-index-name = externalFnMap.txt
+// CHECK-NEXT: display-ctu-progress = false
 // CHECK-NEXT: eagerly-assume = true
 // CHECK-NEXT: elide-constructors = true
 // CHECK-NEXT: expand-macros = false
@@ -51,4 +52,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 48
+// CHECK-NEXT: num-entries = 49
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -560,8 +560,8 @@
   cross_tu::CrossTranslationUnitContext &CTUCtx =
   *Engine->getCrossTranslationUnitContext();
   llvm::Expected CTUDeclOrError =
-  CTUCtx.getCrossTUDefinition(FD, Opts.CTUDir,
-  Opts.CTUIndexName);
+  CTUCtx.getCrossTUDefinition(FD, Opts.CTUDir, Opts.CTUIndexName,
+  Opts.DisplayCTUProgress);
 
   if (!CTUDeclOrError) {
 handleAllErrors(CTUDeclOrError.takeError(),
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -149,14 +149,15 @@
 llvm::Expected
 CrossTranslationUnitContext::getCrossTUDefinition(const FunctionDecl *FD,
   StringRef CrossTUDir,
-  StringRef IndexName) {
+  StringRef IndexName,
+  bool DisplayCTUProgress) {
   assert(!FD->hasBody() && "FD has a definition in current translation unit!");
   const std::string LookupFnName = getLookupName(FD);
   if (LookupFnName.empty())
 return llvm::make_error(
 index_error_code::failed_to_generate_usr);
   llvm::Expected ASTUnitOrError =
-  loadExternalAST(LookupFnName, CrossTUDir, IndexName);
+  loadExternalAST(LookupFnName, CrossTUDir, IndexName, DisplayCTUProgress);
   if (!ASTUnitOrError)
 return ASTUnitOrError.takeError();
   ASTUnit *Unit = *ASTUnitOrError;
@@ -193,7 +194,8 @@
 }
 
 llvm::Expected CrossTranslationUnitContext::loadExternalAST(
-StringRef LookupName, StringRef CrossTUDir, StringRef IndexName) {
+StringRef LookupName, StringRef CrossTUDir, StringRef IndexName,
+bool DisplayCTUProgress) {
   // FIXME: The current implementation only supports loading functions with
   //a lookup name from a single translation unit. If multiple
   //translation units contains functions with the same lookup name an
@@ -233,6 +235,10 @@
   ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts()));
   Unit = LoadedUnit.get();
   FileASTUnitMap[ASTFileName] = std::move(LoadedUnit);
+  if (DisplayCTUProgress) {
+llvm::errs() << "ANALYZE CTU loaded AST file: "
+ << ASTFileName << "\n";
+  }
 } else {
   Unit = ASTCacheEntry->second.get();
 }
Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> should be an -analyzer-config option.

Ok, just changed it to be.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55135



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

Re: r348131 - [AST] Fix an uninitialized bug in the bits of FunctionDecl

[David Blaikie via cfe-commits]

Ah, thanks for the explanation! No worries about pre-commit review or
anything - this is what post-commit review is :) Only note for the future
is that it might be worth mentioning in the body of the commit message
(title/first line was fine) so it's clear why this "extra" work is being
done.

Thanks!
- Dave

On Tue, Dec 4, 2018 at 7:54 AM Bruno Ricci  wrote:

> There is two reasons for this change:
>
> 1.) The first one is that the bit FunctionDeclBits.IsCopyDeductionCandidate
> is only used by CXXDeductionGuideDecl and so there is no getter/setter
> for it in FunctionDecl, and it would not make sense to add one.
> This is a legacy of when these bits where stored in FunctionDecl
> itself.
>
> 2.) The second one is that some of these setter do a little bit more than
> initializing the appropriate bit. For example setInlineSpecified sets
> both
> FunctionDeclBits.IsInlineSpecified and FunctionDeclBits.IsInline, but
> a quick
> reading of the body of the constructor of FunctionDecl would lead
> someone to
> believe that FunctionDeclBits.IsInline is not initialized.
>
> However these are not strong reasons, and I can revert it to use the
> setters
> instead if preferred. My apologies if it would have been preferable to
> review
> it first. It seemed obvious to me and I was familiar with this piece of
> code
> since I did the change a few month ago, but I am still trying to fine tune
> this decision process.
>
> Cheers,
>
> Bruno
>
> On 03/12/2018 22:01, David Blaikie wrote:
> > Why the change from using setter functions to direct assignment?
> >
> > On Mon, Dec 3, 2018 at 5:06 AM Bruno Ricci via cfe-commits <
> cfe-commits@lists.llvm.org > wrote:
> >
> > Author: brunoricci
> > Date: Mon Dec  3 05:04:10 2018
> > New Revision: 348131
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=348131&view=rev
> > Log:
> > [AST] Fix an uninitialized bug in the bits of FunctionDecl
> >
> > FunctionDeclBits.IsCopyDeductionCandidate was not initialized.
> > This caused a warning with valgrind.
> >
> >
> > Modified:
> > cfe/trunk/lib/AST/Decl.cpp
> >
> > Modified: cfe/trunk/lib/AST/Decl.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=348131&r1=348130&r2=348131&view=diff
> >
>  
> ==
> > --- cfe/trunk/lib/AST/Decl.cpp (original)
> > +++ cfe/trunk/lib/AST/Decl.cpp Mon Dec  3 05:04:10 2018
> > @@ -2653,27 +2653,29 @@ FunctionDecl::FunctionDecl(Kind DK, ASTC
> >DeclContext(DK), redeclarable_base(C), ODRHash(0),
> >EndRangeLoc(NameInfo.getEndLoc()), DNLoc(NameInfo.getInfo()) {
> >assert(T.isNull() || T->isFunctionType());
> > -  setStorageClass(S);
> > -  setInlineSpecified(isInlineSpecified);
> > -  setExplicitSpecified(false);
> > -  setVirtualAsWritten(false);
> > -  setPure(false);
> > -  setHasInheritedPrototype(false);
> > -  setHasWrittenPrototype(true);
> > -  setDeletedAsWritten(false);
> > -  setTrivial(false);
> > -  setTrivialForCall(false);
> > -  setDefaulted(false);
> > -  setExplicitlyDefaulted(false);
> > -  setHasImplicitReturnZero(false);
> > -  setLateTemplateParsed(false);
> > -  setConstexpr(isConstexprSpecified);
> > -  setInstantiationIsPending(false);
> > -  setUsesSEHTry(false);
> > -  setHasSkippedBody(false);
> > -  setWillHaveBody(false);
> > -  setIsMultiVersion(false);
> > -  setHasODRHash(false);
> > +  FunctionDeclBits.SClass = S;
> > +  FunctionDeclBits.IsInline = isInlineSpecified;
> > +  FunctionDeclBits.IsInlineSpecified = isInlineSpecified;
> > +  FunctionDeclBits.IsExplicitSpecified = false;
> > +  FunctionDeclBits.IsVirtualAsWritten = false;
> > +  FunctionDeclBits.IsPure = false;
> > +  FunctionDeclBits.HasInheritedPrototype = false;
> > +  FunctionDeclBits.HasWrittenPrototype = true;
> > +  FunctionDeclBits.IsDeleted = false;
> > +  FunctionDeclBits.IsTrivial = false;
> > +  FunctionDeclBits.IsTrivialForCall = false;
> > +  FunctionDeclBits.IsDefaulted = false;
> > +  FunctionDeclBits.IsExplicitlyDefaulted = false;
> > +  FunctionDeclBits.HasImplicitReturnZero = false;
> > +  FunctionDeclBits.IsLateTemplateParsed = false;
> > +  FunctionDeclBits.IsConstexpr = isConstexprSpecified;
> > +  FunctionDeclBits.InstantiationIsPending = false;
> > +  FunctionDeclBits.UsesSEHTry = false;
> > +  FunctionDeclBits.HasSkippedBody = false;
> > +  FunctionDeclBits.WillHaveBody = false;
> > +  FunctionDeclBits.IsMultiVersion = false;
> > +  FunctionDeclBits.IsCopyDeductionCandidate = false;
> > +  FunctionDeclBits.HasODRHash = false;
> >  }
> >
> >  void FunctionDecl::getNameForDiagnostic(
> >
> >
> > ___
> >   

[PATCH] D54592: [CStringChecker] evaluate explicit_bzero

[David CARLIER via Phabricator via cfe-commits]

devnexen added a comment.

ping @MaskRay my only hope seemingly :-)


Repository:
  rC Clang

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

https://reviews.llvm.org/D54592



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

Re: r348131 - [AST] Fix an uninitialized bug in the bits of FunctionDecl

[Bruno Ricci via cfe-commits]

Got it. Thanks for the review!

Bruno

On 04/12/2018 16:59, David Blaikie wrote:
> Ah, thanks for the explanation! No worries about pre-commit review or 
> anything - this is what post-commit review is :) Only note for the future is 
> that it might be worth mentioning in the body of the commit message 
> (title/first line was fine) so it's clear why this "extra" work is being done.
> 
> Thanks!
> - Dave
> 
> On Tue, Dec 4, 2018 at 7:54 AM Bruno Ricci  > wrote:
> 
> There is two reasons for this change:
> 
> 1.) The first one is that the bit 
> FunctionDeclBits.IsCopyDeductionCandidate
>     is only used by CXXDeductionGuideDecl and so there is no getter/setter
>     for it in FunctionDecl, and it would not make sense to add one.
>     This is a legacy of when these bits where stored in FunctionDecl 
> itself.
> 
> 2.) The second one is that some of these setter do a little bit more than
>     initializing the appropriate bit. For example setInlineSpecified sets 
> both
>     FunctionDeclBits.IsInlineSpecified and FunctionDeclBits.IsInline, but 
> a quick
>     reading of the body of the constructor of FunctionDecl would lead 
> someone to
>     believe that FunctionDeclBits.IsInline is not initialized.
> 
> However these are not strong reasons, and I can revert it to use the 
> setters
> instead if preferred. My apologies if it would have been preferable to 
> review
> it first. It seemed obvious to me and I was familiar with this piece of 
> code
> since I did the change a few month ago, but I am still trying to fine tune
> this decision process.
> 
> Cheers,
> 
> Bruno
> 
> On 03/12/2018 22:01, David Blaikie wrote:
> > Why the change from using setter functions to direct assignment?
> >
> > On Mon, Dec 3, 2018 at 5:06 AM Bruno Ricci via cfe-commits 
> mailto:cfe-commits@lists.llvm.org> 
> >> 
> wrote:
> >
> >     Author: brunoricci
> >     Date: Mon Dec  3 05:04:10 2018
> >     New Revision: 348131
> >
> >     URL: http://llvm.org/viewvc/llvm-project?rev=348131&view=rev
> >     Log:
> >     [AST] Fix an uninitialized bug in the bits of FunctionDecl
> >
> >     FunctionDeclBits.IsCopyDeductionCandidate was not initialized.
> >     This caused a warning with valgrind.
> >
> >
> >     Modified:
> >         cfe/trunk/lib/AST/Decl.cpp
> >
> >     Modified: cfe/trunk/lib/AST/Decl.cpp
> >     URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=348131&r1=348130&r2=348131&view=diff
> >     
> ==
> >     --- cfe/trunk/lib/AST/Decl.cpp (original)
> >     +++ cfe/trunk/lib/AST/Decl.cpp Mon Dec  3 05:04:10 2018
> >     @@ -2653,27 +2653,29 @@ FunctionDecl::FunctionDecl(Kind DK, ASTC
> >            DeclContext(DK), redeclarable_base(C), ODRHash(0),
> >            EndRangeLoc(NameInfo.getEndLoc()), DNLoc(NameInfo.getInfo()) 
> {
> >        assert(T.isNull() || T->isFunctionType());
> >     -  setStorageClass(S);
> >     -  setInlineSpecified(isInlineSpecified);
> >     -  setExplicitSpecified(false);
> >     -  setVirtualAsWritten(false);
> >     -  setPure(false);
> >     -  setHasInheritedPrototype(false);
> >     -  setHasWrittenPrototype(true);
> >     -  setDeletedAsWritten(false);
> >     -  setTrivial(false);
> >     -  setTrivialForCall(false);
> >     -  setDefaulted(false);
> >     -  setExplicitlyDefaulted(false);
> >     -  setHasImplicitReturnZero(false);
> >     -  setLateTemplateParsed(false);
> >     -  setConstexpr(isConstexprSpecified);
> >     -  setInstantiationIsPending(false);
> >     -  setUsesSEHTry(false);
> >     -  setHasSkippedBody(false);
> >     -  setWillHaveBody(false);
> >     -  setIsMultiVersion(false);
> >     -  setHasODRHash(false);
> >     +  FunctionDeclBits.SClass = S;
> >     +  FunctionDeclBits.IsInline = isInlineSpecified;
> >     +  FunctionDeclBits.IsInlineSpecified = isInlineSpecified;
> >     +  FunctionDeclBits.IsExplicitSpecified = false;
> >     +  FunctionDeclBits.IsVirtualAsWritten = false;
> >     +  FunctionDeclBits.IsPure = false;
> >     +  FunctionDeclBits.HasInheritedPrototype = false;
> >     +  FunctionDeclBits.HasWrittenPrototype = true;
> >     +  FunctionDeclBits.IsDeleted = false;
> >     +  FunctionDeclBits.IsTrivial = false;
> >     +  FunctionDeclBits.IsTrivialForCall = false;
> >     +  FunctionDeclBits.IsDefaulted = false;
> >     +  FunctionDeclBits.IsExplicitlyDefaulted = false;
> >     +  FunctionDeclBits.HasImplicitReturnZero = false;
> >     +  FunctionDeclBits.IsLateTemplateParsed = false;
> >     +  FunctionDeclBits.IsConst

[PATCH] D55133: [CTU] Add statistics

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176654.
martong added a comment.

- Remove NumNoUnit


Repository:
  rC Clang

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

https://reviews.llvm.org/D55133

Files:
  lib/CrossTU/CrossTranslationUnit.cpp


Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -21,6 +21,7 @@
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Index/USRGeneration.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/ADT/Statistic.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
@@ -32,6 +33,15 @@
 namespace cross_tu {
 
 namespace {
+#define DEBUG_TYPE "CrossTranslationUnit"
+STATISTIC(NumGetCTUCalled, "The # of getCTUDefinition function called");
+STATISTIC(
+NumNotInOtherTU,
+"The # of getCTUDefinition called but the function is not in any other 
TU");
+STATISTIC(NumGetCTUSuccess,
+  "The # of getCTUDefinition successfully returned the "
+  "requested function's body");
+
 // FIXME: This class is will be removed after the transition to llvm::Error.
 class IndexErrorCategory : public std::error_category {
 public:
@@ -151,18 +161,21 @@
   StringRef CrossTUDir,
   StringRef IndexName) {
   assert(!FD->hasBody() && "FD has a definition in current translation unit!");
+  ++NumGetCTUCalled;
   const std::string LookupFnName = getLookupName(FD);
   if (LookupFnName.empty())
 return llvm::make_error(
 index_error_code::failed_to_generate_usr);
   llvm::Expected ASTUnitOrError =
   loadExternalAST(LookupFnName, CrossTUDir, IndexName);
-  if (!ASTUnitOrError)
+  if (!ASTUnitOrError) {
 return ASTUnitOrError.takeError();
+  }
   ASTUnit *Unit = *ASTUnitOrError;
-  if (!Unit)
+  if (!Unit) {
 return llvm::make_error(
 index_error_code::failed_to_get_external_ast);
+  }
   assert(&Unit->getFileManager() ==
  &Unit->getASTContext().getSourceManager().getFileManager());
 
@@ -216,8 +229,10 @@
 }
 
 auto It = FunctionFileMap.find(LookupName);
-if (It == FunctionFileMap.end())
+if (It == FunctionFileMap.end()) {
+  ++NumNotInOtherTU;
   return 
llvm::make_error(index_error_code::missing_definition);
+}
 StringRef ASTFileName = It->second;
 auto ASTCacheEntry = FileASTUnitMap.find(ASTFileName);
 if (ASTCacheEntry == FileASTUnitMap.end()) {
@@ -250,6 +265,7 @@
   cast(Importer.Import(const_cast(FD)));
   assert(ToDecl->hasBody());
   assert(FD->hasBody() && "Functions already imported should have body.");
+  ++NumGetCTUSuccess;
   return ToDecl;
 }
 


Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -21,6 +21,7 @@
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Index/USRGeneration.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/ADT/Statistic.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
@@ -32,6 +33,15 @@
 namespace cross_tu {
 
 namespace {
+#define DEBUG_TYPE "CrossTranslationUnit"
+STATISTIC(NumGetCTUCalled, "The # of getCTUDefinition function called");
+STATISTIC(
+NumNotInOtherTU,
+"The # of getCTUDefinition called but the function is not in any other TU");
+STATISTIC(NumGetCTUSuccess,
+  "The # of getCTUDefinition successfully returned the "
+  "requested function's body");
+
 // FIXME: This class is will be removed after the transition to llvm::Error.
 class IndexErrorCategory : public std::error_category {
 public:
@@ -151,18 +161,21 @@
   StringRef CrossTUDir,
   StringRef IndexName) {
   assert(!FD->hasBody() && "FD has a definition in current translation unit!");
+  ++NumGetCTUCalled;
   const std::string LookupFnName = getLookupName(FD);
   if (LookupFnName.empty())
 return llvm::make_error(
 index_error_code::failed_to_generate_usr);
   llvm::Expected ASTUnitOrError =
   loadExternalAST(LookupFnName, CrossTUDir, IndexName);
-  if (!ASTUnitOrError)
+  if (!ASTUnitOrError) {
 return ASTUnitOrError.takeError();
+  }
   ASTUnit *Unit = *ASTUnitOrError;
-  if (!Unit)
+  if (!Unit) {
 return llvm::make_error(
 index_error_code::failed_to_get_external_ast);
+  }
   assert(&Unit->getFileManager() ==
  &Unit->getASTContext().getSourceManager().getFileManager());
 
@@ -216,8 +229,10 @@
 }
 
 auto It = FunctionFileMap.find(LookupName);
-if (It == FunctionFileMap.end())
+if (It == FunctionFileMap.end()) {
+  ++NumNotInOtherTU;
   return llvm::make_

[PATCH] D55133: [CTU] Add statistics

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> Sorry, but I don't understand the meaning of some options. Could you please 
> explain what are NumNoUnit and NumNotInOtherTU and what is the difference 
> between them?

Your point is absolutely true. They are the same, I think at some point some 
time ago they were different (in our fork), now it is just redundancy. So I 
removed `NumNoUnit`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55133



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