[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

ping, and should we add new warning messages as suggested by @Rakete ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang added a comment.

If no more changes need to be made, is this okay to be merged in?


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

https://reviews.llvm.org/D55044



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

[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace

[Yucheng Wu via Phabricator via cfe-commits]

Ywicheng updated this revision to Diff 194614.
Ywicheng marked an inline comment as done.
Ywicheng edited the summary of this revision.

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

https://reviews.llvm.org/D55411

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/SafelyScopedCheck.cpp
  clang-tidy/abseil/SafelyScopedCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-safely-scoped.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-safely-scoped.cpp

Index: test/clang-tidy/abseil-safely-scoped.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-safely-scoped.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s abseil-safely-scoped %t
+namespace bar {
+
+class A {};
+class B {};
+} // namespace bar
+
+namespace foo1 {
+
+using bar::A;
+void f(A a);
+
+namespace {} // anonymous namespace
+} // namespace foo1
+// CHECK-MESSAGES: :[[@LINE-4]]:12: warning: using declaration 'A' not
+// declared in the innermost namespace. [abseil-safely-scoped]
+
+namespace foo2 {
+
+namespace {
+
+using ::bar::B;
+
+} // anonymous namespace
+void g(B b);
+} // namespace foo2
+
+
+// Check should not be triggered below when we are at 
+// function (instead of namespace) scope.
+namespace outer {
+
+  int fun_scope() {
+using ::bar::A;
+return 0;
+  } // function scope
+  
+  namespace inner {
+
+  } // namespace inner
+
+} // namespace outer
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
+   abseil-safely-scoped
abseil-str-cat-append
abseil-string-find-startswith
abseil-time-subtraction
Index: docs/clang-tidy/checks/abseil-safely-scoped.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-safely-scoped.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - abseil-safely-scoped
+
+abseil-safely-scoped
+
+
+Flags using declarations that are not contained in an innermost
+namespace, and suggests these declarations be moved elsewhere.
+
+Example:
+
+.. code-block:: c++
+
+  using something; // should be inside the innermost namespace bar below
+
+  namespace foo {
+  namespace bar {
+	
+  } // bar
+
+  using something_else; // shoulw be inside the innermost namespace bar above
+
+  } // foo
+
+Placing convenience aliases in upper level namespaces can lead to ambiguity in 
+which name the compiler should use. 
+
+See https://abseil.io/tips/119 for more explanation. 
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,8 +67,15 @@
 Improvements to clang-tidy
 --
 
+- New :doc:`abseil-safely-scoped
+  ` check.
+
+  Finds instances of using declarations not in the innermost layer
+  of a series of namespaces.
+
 - New :doc:`abseil-duration-addition
   ` check.
+>>> master
 
   Checks for cases where addition should be performed in the ``absl::Time``
   domain.
Index: clang-tidy/abseil/SafelyScopedCheck.h
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.h
@@ -0,0 +1,36 @@
+//===--- SafelyScopedCheck.h - clang-tidy ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Detecting using declarations not in a namespace declaration or not in
+/// the innermost layer of namespace declarations.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-safely-scoped.html
+class SafelyScopedCheck : public ClangTidyCheck {
+public:
+  SafelyScopedCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
Index: clang-tidy/abseil/SafelyScopedCheck.cpp
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.cpp
@@ -0,0 +1,43 @@
+//===--- 

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/abseil/WrapUniqueCheck.cpp:85
+
+std::string Left = (ConsDecl) ? "auto " + NameRef.str() + " = " : "";
+std::string NewText =

I don't think that you need round brackets around ConsDecl. Same below.



Comment at: docs/clang-tidy/checks/abseil-wrap-unique.rst:5
+==
+Looks for instances of factory functions which uses a non-public constructor
+that returns a ``std::unqiue_ptr`` then recommends using  
``absl::wrap_unique(new T(...))``.

Please separate with empty line.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435



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

[PATCH] D17407: [Sema] PR25755 Handle out of order designated initializers

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

ping...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17407



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

Re: Notice: The buildbot bb.pgr.jp will be suspended within a few days

[Mike Edwards via cfe-commits]

Takumi,
I am very sorry it took me almost 18 months to respond to this note.  I'm
sorry to see you had to shutter bb.pgr.jp.  It was a very helpful resource
to the community.  Thank you very much for your contribution of effort and
resources to make bb.pgr.jp available to the LLVM community for as long as
you were able.  Your efforts are always very much appreciated.

Respectfully,
Mike

On Mon, Nov 20, 2017 at 1:44 AM NAKAMURA Takumi via llvm-commits <
llvm-comm...@lists.llvm.org> wrote:

> Due to resource issue, I have to terminate it. As you know, it has been
> working for several years and I am certain it has been useful and helpful
> to guys.
> I am not sure whether I could restart it or not.
> I loved it.
>
> Thank you,
> Takumi Nakamura
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Ryan Piantedosi via Phabricator via cfe-commits]

Dosi-Dough updated this revision to Diff 194621.
Dosi-Dough marked 2 inline comments as done.
Dosi-Dough added a comment.

added whitespace to check doc


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/WrapUniqueCheck.cpp
  clang-tidy/abseil/WrapUniqueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-wrap-unique.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-wrap-unique.cpp

Index: test/clang-tidy/abseil-wrap-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-wrap-unique.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s abseil-wrap-unique %t
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr ) = delete;
+  unique_ptr(unique_ptr &) {}
+  ~unique_ptr() {}
+  type *() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr =(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr =(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+}  // namespace std
+
+class A {
+ public:
+  static A* NewA() {
+return new A();
+  }
+
+ private:
+  A() {}
+};
+
+class B {
+ public:
+  static B* NewB(int bIn) {
+return new B();
+  }
+
+ private:
+  B() {}
+};
+
+struct C {
+  int x;
+  int y;
+};
+/*
+std::unique_ptr returnPointer() {
+  return std::unique_ptr(A::NewA());
+}
+*/
+void positives() {
+  std::unique_ptr a;
+  a.reset(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: a = absl::WrapUnique(A::NewA())
+  
+  std::unique_ptr b(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto b = absl::WrapUnique(A::NewA())
+
+  int cIn;
+  std::unique_ptr c(B::NewB(cIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto c = absl::WrapUnique(B::NewB(cIn))
+
+  int dIn;
+  std::unique_ptr d;
+  d.reset(B::NewB(dIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: d = absl::WrapUnique(B::NewB(dIn))
+  
+  auto e = std::unique_ptr(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: e = absl::WrapUnique(A::NewA())
+
+  //std::unique_ptr e(new int[2] {1,2});
+}
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -20,6 +20,7 @@
abseil-string-find-startswith
abseil-time-subtraction
abseil-upgrade-duration-conversions
+   abseil-wrap-unique
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-wrap-unique.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-wrap-unique.rst
@@ -0,0 +1,35 @@
+.. title:: clang-tidy - abseil-wrap-unique
+
+abseil-wrap-unique
+==
+
+Looks for instances of factory functions which uses a non-public constructor
+that returns a ``std::unqiue_ptr`` then recommends using 
+``absl::wrap_unique(new T(...))``.
+
+Examples:
+
+.. code-block:: c++
+ 
+  class A {
+  public:
+static A* NewA() { return new A(); }
+
+  private:
+A() = default; 
+  };
+
+  std::unique_ptr a;
+
+  // Original - reset called with a static function returning a std::unqiue_ptr
+  a.reset(A::NewA());
+
+  // Suggested - reset ptr with absl::WrapUnique
+  a = absl::WrapUnique(A::NewA());
+
+  // Original - std::unique_ptr initialized with static function
+  std::unique_ptr b(A::NewA());
+
+  // Suggested - initialize with absl::WrapUnique instead
+  auto b = absl::WrapUnique(A::NewA())
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -91,6 +91,13 @@
   Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
   in the Time domain instead of the numeric domain.
 
+- New :doc:`abseil-wrap-unique
+  ` check.
+
+  Looks for instances of factory functions which uses a non-public constructor
+  that returns a ``std::unqiue_ptr`` then recommends using 
+  ``absl::wrap_unique(new T(...))``.
+
 - New :doc:`google-readability-avoid-underscore-in-googletest-name
   `
   check.
Index: clang-tidy/abseil/WrapUniqueCheck.h

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Ryan Piantedosi via Phabricator via cfe-commits]

Dosi-Dough updated this revision to Diff 194622.
Dosi-Dough added a comment.

added white space to check doc.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/WrapUniqueCheck.cpp
  clang-tidy/abseil/WrapUniqueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-wrap-unique.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-wrap-unique.cpp

Index: test/clang-tidy/abseil-wrap-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-wrap-unique.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s abseil-wrap-unique %t
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr ) = delete;
+  unique_ptr(unique_ptr &) {}
+  ~unique_ptr() {}
+  type *() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr =(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr =(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+}  // namespace std
+
+class A {
+ public:
+  static A* NewA() {
+return new A();
+  }
+
+ private:
+  A() {}
+};
+
+class B {
+ public:
+  static B* NewB(int bIn) {
+return new B();
+  }
+
+ private:
+  B() {}
+};
+
+struct C {
+  int x;
+  int y;
+};
+/*
+std::unique_ptr returnPointer() {
+  return std::unique_ptr(A::NewA());
+}
+*/
+void positives() {
+  std::unique_ptr a;
+  a.reset(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: a = absl::WrapUnique(A::NewA())
+  
+  std::unique_ptr b(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto b = absl::WrapUnique(A::NewA())
+
+  int cIn;
+  std::unique_ptr c(B::NewB(cIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto c = absl::WrapUnique(B::NewB(cIn))
+
+  int dIn;
+  std::unique_ptr d;
+  d.reset(B::NewB(dIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: d = absl::WrapUnique(B::NewB(dIn))
+  
+  auto e = std::unique_ptr(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: e = absl::WrapUnique(A::NewA())
+
+  //std::unique_ptr e(new int[2] {1,2});
+}
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -20,6 +20,7 @@
abseil-string-find-startswith
abseil-time-subtraction
abseil-upgrade-duration-conversions
+   abseil-wrap-unique
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-wrap-unique.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-wrap-unique.rst
@@ -0,0 +1,35 @@
+.. title:: clang-tidy - abseil-wrap-unique
+
+abseil-wrap-unique
+==
+
+Looks for instances of factory functions which uses a non-public constructor
+that returns a ``std::unqiue_ptr`` then recommends using 
+``absl::wrap_unique(new T(...))``.
+
+Examples:
+
+.. code-block:: c++
+ 
+  class A {
+  public:
+static A* NewA() { return new A(); }
+
+  private:
+A() = default; 
+  };
+
+  std::unique_ptr a;
+
+  // Original - reset called with a static function returning a std::unqiue_ptr
+  a.reset(A::NewA());
+
+  // Suggested - reset ptr with absl::WrapUnique
+  a = absl::WrapUnique(A::NewA());
+
+  // Original - std::unique_ptr initialized with static function
+  std::unique_ptr b(A::NewA());
+
+  // Suggested - initialize with absl::WrapUnique instead
+  auto b = absl::WrapUnique(A::NewA())
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -91,6 +91,13 @@
   Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
   in the Time domain instead of the numeric domain.
 
+- New :doc:`abseil-wrap-unique
+  ` check.
+
+  Looks for instances of factory functions which uses a non-public constructor
+  that returns a ``std::unqiue_ptr`` then recommends using 
+  ``absl::wrap_unique(new T(...))``.
+
 - New :doc:`google-readability-avoid-underscore-in-googletest-name
   `
   check.
Index: clang-tidy/abseil/WrapUniqueCheck.h

[PATCH] D60409: [clangd] Add -header-insertion=never flag to disable include insertion in code completion

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D60409#1461579 , @phosek wrote:

> Our Mac builders have started failing after this change with the following:
>
>   [3145/3502] Building CXX object 
> tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o
>   FAILED: 
> tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o 
>   /b/s/w/ir/cache/goma/client/gomacc /b/s/w/ir/k/cipd/bin/clang++  
> -DCLANG_VENDOR="\"Fuchsia \"" -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE 
> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
> -Itools/clang/tools/extra/clangd/tool 
> -I/b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool 
> -I/b/s/w/ir/k/llvm-project/clang/include -Itools/clang/include 
> -I/usr/include/libxml2 -Iinclude -I/b/s/w/ir/k/llvm-project/llvm/include 
> -I/b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool/.. 
> -Itools/clang/tools/extra/clangd/tool/.. -fPIC -fvisibility-inlines-hidden 
> -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra 
> -Wno-unused-parameter -Wwrite-strings -Wcast-qual 
> -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough 
> -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
> -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -fno-common 
> -Woverloaded-virtual -Wno-nested-anon-types -O3 -gline-tables-only   -UNDEBUG 
>  -fno-exceptions -fno-rtti -MD -MT 
> tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o 
> -MF 
> tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o.d 
> -o tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o 
> -c /b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool/ClangdMain.cpp
>   
> /b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool/ClangdMain.cpp:474:22: 
> error: use of undeclared identifier 'newXPCTransport'
>   TransportLayer = newXPCTransport();
>^
>   1 error generated.
>
>
> I don't understand why since this change hasn't touched the failing line.


It has been fixed by rCTE358103 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60409



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

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

[Jan Korous via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/ASTContext.cpp:372
   D = adjustDeclToTemplate(D);
+  const Decl* CanonicalDecl = D->getCanonicalDecl();
 

gribozavr wrote:
> jkorous wrote:
> > arphaman wrote:
> > > Why are we now checking for the canonical declaration instead of `D` as 
> > > before?
> > Before this we were caching comment for every redeclaration separately but 
> > for every redeclaration the comment was the same.
> > 
> > As I understand it - for a given declaration we found the first comment in 
> > the redeclaration chain and then saved it to the cache for every 
> > redeclaration (the same comment).
> > Later, during lookup, we iterated the redeclaration chain again and did a 
> > lookup for every redeclaration (see L392 in the original implementation).
> > 
> > Unless I missed something, it's suboptimal in both memory consumption and 
> > runtime overhead.
> > 
> > That's the reason why I want to cache the comment for the redeclaration 
> > group as a whole. The only thing I am not entirely sure about is what key 
> > to use to represent the whole redeclaration chain - maybe the first 
> > declaration in the redeclaration chain would be better then the canonical 
> > declaration...
> > 
> > WDYT?
> > As I understand it - for a given declaration we found the first comment in 
> > the redeclaration chain and then saved it to the cache for every 
> > redeclaration (the same comment).
> 
> Only if there was only one comment in the redeclaration chain.  If a 
> redeclaration had a different comment, this function would return that 
> comment.
I see. I made a wrong assumption - that for any two redeclarations the 
redeclaration chain always starts from the same declaration.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60432



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

[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace

[Yucheng Wu via Phabricator via cfe-commits]

Ywicheng updated this revision to Diff 194623.
Ywicheng marked 2 inline comments as done.

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

https://reviews.llvm.org/D55411

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/SafelyScopedCheck.cpp
  clang-tidy/abseil/SafelyScopedCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-safely-scoped.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-safely-scoped.cpp

Index: test/clang-tidy/abseil-safely-scoped.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-safely-scoped.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s abseil-safely-scoped %t
+namespace bar {
+
+class A {};
+class B {};
+} // namespace bar
+
+namespace foo1 {
+
+using bar::A;
+void f(A a);
+
+namespace {} // anonymous namespace
+} // namespace foo1
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: using declaration 'A' not
+// declared in the innermost namespace. [abseil-safely-scoped]
+
+namespace foo2 {
+
+namespace {
+
+using ::bar::B;
+
+} // anonymous namespace
+void g(B b);
+} // namespace foo2
+
+
+// Check should not be triggered below when we are at 
+// function (instead of namespace) scope.
+namespace outer {
+
+  int fun_scope() {
+using ::bar::A;
+return 0;
+  } // function scope
+  
+  namespace inner {
+
+  } // namespace inner
+
+} // namespace outer
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
+   abseil-safely-scoped
abseil-str-cat-append
abseil-string-find-startswith
abseil-time-subtraction
Index: docs/clang-tidy/checks/abseil-safely-scoped.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-safely-scoped.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - abseil-safely-scoped
+
+abseil-safely-scoped
+
+
+Flags using declarations that are not contained in an innermost
+namespace, and suggests these declarations be moved elsewhere.
+
+Example:
+
+.. code-block:: c++
+
+  using something; // should be inside the innermost namespace bar below
+
+  namespace foo {
+  namespace bar {
+	
+  } // bar
+
+  using something_else; // shoulw be inside the innermost namespace bar above
+
+  } // foo
+
+Placing convenience aliases in upper level namespaces can lead to ambiguity in 
+which name the compiler should use. 
+
+See https://abseil.io/tips/119 for more explanation. 
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,8 +67,15 @@
 Improvements to clang-tidy
 --
 
+- New :doc:`abseil-safely-scoped
+  ` check.
+
+  Finds instances of using declarations not in the innermost layer
+  of a series of namespaces.
+
 - New :doc:`abseil-duration-addition
   ` check.
+>>> master
 
   Checks for cases where addition should be performed in the ``absl::Time``
   domain.
Index: clang-tidy/abseil/SafelyScopedCheck.h
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.h
@@ -0,0 +1,36 @@
+//===--- SafelyScopedCheck.h - clang-tidy ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Detecting using declarations not in a namespace declaration or not in
+/// the innermost layer of namespace declarations.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-safely-scoped.html
+class SafelyScopedCheck : public ClangTidyCheck {
+public:
+  SafelyScopedCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
Index: clang-tidy/abseil/SafelyScopedCheck.cpp
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.cpp
@@ -0,0 +1,43 @@
+//===--- SafelyScopedCheck.cpp - clang-tidy 

[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace

[Yucheng Wu via Phabricator via cfe-commits]

Ywicheng updated this revision to Diff 194631.

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

https://reviews.llvm.org/D55411

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/SafelyScopedCheck.cpp
  clang-tidy/abseil/SafelyScopedCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-safely-scoped.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-safely-scoped.cpp

Index: test/clang-tidy/abseil-safely-scoped.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-safely-scoped.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s abseil-safely-scoped %t
+namespace bar {
+
+class A {};
+class B {};
+} // namespace bar
+
+namespace foo1 {
+
+using bar::A;
+void f(A a);
+
+namespace {} // anonymous namespace
+} // namespace foo1
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: using declaration 'A' not
+// declared in the innermost namespace. [abseil-safely-scoped]
+
+namespace foo2 {
+
+namespace {
+
+using ::bar::B;
+
+} // anonymous namespace
+void g(B b);
+} // namespace foo2
+
+
+// Check should not be triggered below when we are at 
+// function (instead of namespace) scope.
+namespace outer {
+
+  int fun_scope() {
+using ::bar::A;
+return 0;
+  } // function scope
+  
+  namespace inner {
+
+  } // namespace inner
+
+} // namespace outer
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
+   abseil-safely-scoped
abseil-str-cat-append
abseil-string-find-startswith
abseil-time-subtraction
Index: docs/clang-tidy/checks/abseil-safely-scoped.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-safely-scoped.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - abseil-safely-scoped
+
+abseil-safely-scoped
+
+
+Flags using declarations that are not contained in an innermost
+namespace, and suggests these declarations be moved elsewhere.
+
+Example:
+
+.. code-block:: c++
+
+  using something; // should be inside the innermost namespace bar below
+
+  namespace foo {
+  namespace bar {
+	
+  } // bar
+
+  using something_else; // shoulw be inside the innermost namespace bar above
+
+  } // foo
+
+Placing convenience aliases in upper level namespaces can lead to ambiguity in 
+which name the compiler should use. 
+
+See https://abseil.io/tips/119 for more explanation. 
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,12 @@
 Improvements to clang-tidy
 --
 
+- New :doc:`abseil-safely-scoped
+  ` check.
+
+  Finds instances of using declarations not in the innermost layer
+  of a series of namespaces.
+
 - New :doc:`abseil-duration-addition
   ` check.
 
Index: clang-tidy/abseil/SafelyScopedCheck.h
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.h
@@ -0,0 +1,36 @@
+//===--- SafelyScopedCheck.h - clang-tidy ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Detecting using declarations not in a namespace declaration or not in
+/// the innermost layer of namespace declarations.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-safely-scoped.html
+class SafelyScopedCheck : public ClangTidyCheck {
+public:
+  SafelyScopedCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
Index: clang-tidy/abseil/SafelyScopedCheck.cpp
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.cpp
@@ -0,0 +1,43 @@
+//===--- SafelyScopedCheck.cpp - clang-tidy ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open 

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194633.
hintonda edited the summary of this revision.
hintonda added a comment.

- Fix bug in updateArgStr() where it didn't handle isInAllSubCommands() 
correctly, and refactored code based on how SubCommands work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp

Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -98,6 +98,8 @@
   // This collects additional help to be printed.
   std::vector MoreHelp;
 
+  SmallVector, 4> DefaultOptions;
+
   // This collects the different option categories that have been registered.
   SmallPtrSet RegisteredOptionCategories;
 
@@ -111,6 +113,10 @@
 
   void ResetAllOptionOccurrences();
 
+  bool tryRemoveDefaultOption(Option *O, SubCommand *SC, StringRef NewArg);
+
+  void handleDefaultOptions();
+
   bool ParseCommandLineOptions(int argc, const char *const *argv,
StringRef Overview, raw_ostream *Errs = nullptr);
 
@@ -186,6 +192,13 @@
   }
 
   void addOption(Option *O) {
+// Handle DefaultOptions
+if (O->getMiscFlags() & cl::DefaultOption) {
+  std::string DefaultArg(O->ArgStr + ":default_option");
+  DefaultOptions.push_back(std::make_pair(DefaultArg, O));
+  O->setArgStr(DefaultOptions.back().first);
+}
+
 if (O->Subs.empty()) {
   addOption(O, &*TopLevelSubCommand);
 } else {
@@ -266,8 +279,13 @@
 if (O->Subs.empty())
   updateArgStr(O, NewName, &*TopLevelSubCommand);
 else {
-  for (auto SC : O->Subs)
-updateArgStr(O, NewName, SC);
+  if (O->isInAllSubCommands()) {
+for (auto SC : RegisteredSubCommands)
+  updateArgStr(O, NewName, SC);
+  } else {
+for (auto SC : O->Subs)
+  updateArgStr(O, NewName, SC);
+  }
 }
   }
 
@@ -1118,6 +1136,41 @@
   }
 }
 
+bool CommandLineParser::tryRemoveDefaultOption(Option *O, SubCommand *SC,
+   StringRef NewArg) {
+  StringRef Val;
+  if (LookupOption(*SC, NewArg, Val)) {
+O->removeArgument();
+return true;
+  }
+  return false;
+}
+
+void CommandLineParser::handleDefaultOptions() {
+  for (auto Pair : DefaultOptions) {
+StringRef Arg(Pair.first);
+Option *O = Pair.second;
+StringRef NewArg = Arg.substr(0, Arg.find(":"));
+
+bool Removed = false;
+if (O->isInAllSubCommands()) {
+  for (auto SC : RegisteredSubCommands) {
+if (SC == &*AllSubCommands)
+  continue;
+if ((Removed = tryRemoveDefaultOption(O, SC, NewArg)))
+  break;
+  }
+} else {
+  for (auto SC : O->Subs) {
+if ((Removed = tryRemoveDefaultOption(O, SC, NewArg)))
+  break;
+  }
+}
+if (!Removed)
+  O->setArgStr(NewArg);
+  }
+}
+
 bool CommandLineParser::ParseCommandLineOptions(int argc,
 const char *const *argv,
 StringRef Overview,
@@ -1167,6 +1220,8 @@
   auto  = ChosenSubCommand->SinkOpts;
   auto  = ChosenSubCommand->OptionsMap;
 
+  handleDefaultOptions();
+
   if (ConsumeAfterOpt) {
 assert(PositionalOpts.size() > 0 &&
"Cannot specify cl::ConsumeAfter without a positional argument!");
@@ -2146,6 +2201,9 @@
 cl::location(WrappedNormalPrinter), cl::ValueDisallowed,
 cl::cat(GenericCategory), cl::sub(*AllSubCommands));
 
+static cl::alias HOpA("h", cl::desc("Alias for -help"), cl::aliasopt(HOp),
+  cl::DefaultOption);
+
 static cl::opt>
 HHOp("help-hidden", cl::desc("Display all available options"),
  cl::location(WrappedHiddenPrinter), cl::Hidden, cl::ValueDisallowed,
Index: llvm/include/llvm/Support/CommandLine.h
===
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -175,7 +175,10 @@
   // If this is enabled, multiple letter options are allowed to bunch together
   // with only a single hyphen for the whole group.  This allows emulation
   // of the behavior that ls uses for example: ls -la === ls -l -a
-  Grouping = 0x08
+  Grouping = 0x08,
+
+  // Default option
+  DefaultOption = 0x10
 };
 
 //===--===//
@@ -270,12 +273,12 @@
   unsigned Value : 2;
   unsigned HiddenFlag : 2; // enum OptionHidden
   unsigned Formatting : 2; // enum FormattingFlags
-  unsigned Misc : 4;
+  unsigned Misc : 5;
   unsigned Position = 0;   // Position of last occurrence of the option
   unsigned AdditionalVals = 0; // Greater than 0 for multi-valued 

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Ryan Piantedosi via Phabricator via cfe-commits]

Dosi-Dough updated this revision to Diff 194608.
Dosi-Dough added a comment.

fixed docs and white spacing


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/WrapUniqueCheck.cpp
  clang-tidy/abseil/WrapUniqueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-wrap-unique.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-wrap-unique.cpp

Index: test/clang-tidy/abseil-wrap-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-wrap-unique.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s abseil-wrap-unique %t
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr ) = delete;
+  unique_ptr(unique_ptr &) {}
+  ~unique_ptr() {}
+  type *() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr =(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr =(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+}  // namespace std
+
+class A {
+ public:
+  static A* NewA() {
+return new A();
+  }
+
+ private:
+  A() {}
+};
+
+class B {
+ public:
+  static B* NewB(int bIn) {
+return new B();
+  }
+
+ private:
+  B() {}
+};
+
+struct C {
+  int x;
+  int y;
+};
+/*
+std::unique_ptr returnPointer() {
+  return std::unique_ptr(A::NewA());
+}
+*/
+void positives() {
+  std::unique_ptr a;
+  a.reset(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: a = absl::WrapUnique(A::NewA())
+  
+  std::unique_ptr b(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto b = absl::WrapUnique(A::NewA())
+
+  int cIn;
+  std::unique_ptr c(B::NewB(cIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto c = absl::WrapUnique(B::NewB(cIn))
+
+  int dIn;
+  std::unique_ptr d;
+  d.reset(B::NewB(dIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: d = absl::WrapUnique(B::NewB(dIn))
+  
+  auto e = std::unique_ptr(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: e = absl::WrapUnique(A::NewA())
+
+  //std::unique_ptr e(new int[2] {1,2});
+}
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -20,6 +20,7 @@
abseil-string-find-startswith
abseil-time-subtraction
abseil-upgrade-duration-conversions
+   abseil-wrap-unique
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-wrap-unique.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-wrap-unique.rst
@@ -0,0 +1,33 @@
+.. title:: clang-tidy - abseil-wrap-unique
+
+abseil-wrap-unique
+==
+Looks for instances of factory functions which uses a non-public constructor
+that returns a ``std::unqiue_ptr`` then recommends using  ``absl::wrap_unique(new T(...))``.
+
+Examples:
+
+.. code-block:: c++
+ 
+  class A {
+  public:
+static A* NewA() { return new A(); }
+
+  private:
+A() = default; 
+  };
+
+  std::unique_ptr a;
+
+  // Original - reset called with a static function returning a std::unqiue_ptr
+  a.reset(A::NewA());
+
+  // Suggested - reset ptr with absl::WrapUnique
+  a = absl::WrapUnique(A::NewA());
+
+  // Original - std::unique_ptr initialized with static function
+  std::unique_ptr b(A::NewA());
+
+  // Suggested - initialize with absl::WrapUnique instead
+  auto b = absl::WrapUnique(A::NewA())
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -91,6 +91,13 @@
   Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
   in the Time domain instead of the numeric domain.
 
+- New :doc:`abseil-wrap-unique
+  ` check.
+
+  Looks for instances of factory functions which uses a non-public constructor
+  that returns a ``std::unqiue_ptr`` then recommends using 
+  ``absl::wrap_unique(new T(...))``.
+
 - New :doc:`google-readability-avoid-underscore-in-googletest-name
   `
   check.
Index: clang-tidy/abseil/WrapUniqueCheck.h

LLVM buildmaster will be updated and restarted tonight

[Galina Kistanova via cfe-commits]

 Hello everyone,

LLVM buildmaster will be updated and restarted after 7PM Pacific time today.

Thanks

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

Re: r358104 - Don't emit an unreachable return block.

[John McCall via cfe-commits]

On 10 Apr 2019, at 21:50, wolfgang.p...@sony.com wrote:


Hi,
This commit seems to be causing a test failure on several buildbots 
(e.g. 
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/26305). 
instrprof-shared-gcov-flush.test is failing because of differences in 
profile info. The test passes when I revert your change, so perhaps 
it's just a case of updating the test.

Could you please take a look?


Adjusted the test in r358149 as a hopeful fix.  CC'ing Marco since I 
believe this was originally his test.


John.



Thanks,
Wolfgang Pieb

-Original Message-
From: cfe-commits  On Behalf Of 
John McCall via cfe-commits

Sent: Wednesday, April 10, 2019 10:03 AM
To: cfe-commits@lists.llvm.org
Subject: r358104 - Don't emit an unreachable return block.

Author: rjmccall
Date: Wed Apr 10 10:03:09 2019
New Revision: 358104

URL: http://llvm.org/viewvc/llvm-project?rev=358104=rev
Log:
Don't emit an unreachable return block.

Patch by Brad Moody.

Added:
cfe/trunk/test/CodeGen/unreachable-ret.c
Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=358104=358103=358104=diff

==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Apr 10 10:03:09 2019
@@ -2873,15 +2873,6 @@ void CodeGenFunction::EmitFunctionEpilog
 RV = SI->getValueOperand();
 SI->eraseFromParent();

-// If that was the only use of the return value, nuke it as 
well now.

-auto returnValueInst = ReturnValue.getPointer();
-if (returnValueInst->use_empty()) {
-  if (auto alloca = 
dyn_cast(returnValueInst)) {

-alloca->eraseFromParent();
-ReturnValue = Address::invalid();
-  }
-}
-
   // Otherwise, we have to do a simple load.
   } else {
 RV = Builder.CreateLoad(ReturnValue);

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=358104=358103=358104=diff

==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Apr 10 10:03:09 2019
@@ -255,6 +255,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
 if (CurBB->empty() || ReturnBlock.getBlock()->use_empty()) {
   ReturnBlock.getBlock()->replaceAllUsesWith(CurBB);
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
 } else
   EmitBlock(ReturnBlock.getBlock());
 return llvm::DebugLoc();
@@ -274,6 +275,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
   Builder.SetInsertPoint(BI->getParent());
   BI->eraseFromParent();
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
   return Loc;
 }
   }
@@ -448,6 +450,19 @@ void CodeGenFunction::FinishFunction(Sou
   // 5. Width of vector aguments and return types for functions 
called by this

   //function.
   CurFn->addFnAttr("min-legal-vector-width", 
llvm::utostr(LargestVectorWidth));

+
+  // If we generated an unreachable return block, delete it now.
+  if (ReturnBlock.isValid() && ReturnBlock.getBlock()->use_empty()) {
+Builder.ClearInsertionPoint();
+ReturnBlock.getBlock()->eraseFromParent();
+  }
+  if (ReturnValue.isValid()) {
+auto *RetAlloca = 
dyn_cast(ReturnValue.getPointer());

+if (RetAlloca && RetAlloca->use_empty()) {
+  RetAlloca->eraseFromParent();
+  ReturnValue = Address::invalid();
+}
+  }
 }

 /// ShouldInstrumentFunction - Return true if the current function 
should be


Added: cfe/trunk/test/CodeGen/unreachable-ret.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/unreachable-ret.c?rev=358104=auto

==
--- cfe/trunk/test/CodeGen/unreachable-ret.c (added)
+++ cfe/trunk/test/CodeGen/unreachable-ret.c Wed Apr 10 10:03:09 2019
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+extern void abort() __attribute__((noreturn));
+
+void f1() {
+  abort();
+}
+// CHECK-LABEL: define void @f1()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+
+void *f2() {
+  abort();
+  return 0;
+}
+// CHECK-LABEL: define i8* @f2()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+

Modified: cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp?rev=358104=358103=358104=diff

==
--- 

[PATCH] D60543: [clang] Add matcher for subclasses of Objective-C interfaces 

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
stephanemoore added a reviewer: aaron.ballman.

This change adds a new AST matcher to detect Objective-C classes that
are subclasses of matching Objective-C interfaces.

Test Notes:
Ran clang unit tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60543

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

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1389,6 +1389,23 @@
   EXPECT_TRUE(matchesObjC("void f() { ^{}(); }", blockExpr()));
 }
 
+TEST(IsSubclassOfInterfaceMatcher, SubclassMatching) {
+  std::string ObjCString = "@interface A @end"
+   "@interface B : A @end"
+   "@interface C : B @end";
+  EXPECT_TRUE(matchesObjC(
+  ObjCString, objcInterfaceDecl(isSubclassOfInterface(hasName("A");
+  EXPECT_TRUE(matchesObjC(ObjCString,
+  objcInterfaceDecl(isSubclassOfInterface(hasName("A")),
+unless(hasName("B");
+  EXPECT_TRUE(matchesObjC(
+  ObjCString, objcInterfaceDecl(isSubclassOfInterface(hasName("B");
+  EXPECT_FALSE(matchesObjC(
+  ObjCString, objcInterfaceDecl(isSubclassOfInterface(hasName("C");
+  EXPECT_FALSE(matchesObjC(
+  ObjCString, objcInterfaceDecl(isSubclassOfInterface(hasName("X");
+}
+
 TEST(StatementCountIs, FindsNoStatementsInAnEmptyCompoundStatement) {
   EXPECT_TRUE(matches("void f() { }",
   compoundStmt(statementCountIs(0;
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -393,6 +393,7 @@
   REGISTER_MATCHER(isStaticLocal);
   REGISTER_MATCHER(isStaticStorageClass);
   REGISTER_MATCHER(isStruct);
+  REGISTER_MATCHER(isSubclassOfInterface);
   REGISTER_MATCHER(isTemplateInstantiation);
   REGISTER_MATCHER(isTypeDependent);
   REGISTER_MATCHER(isUnion);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -1461,6 +1461,35 @@
 extern const internal::VariadicDynCastAllOfMatcher
 objcFinallyStmt;
 
+/// Matches Objective-C classes that directly or indirectly subclass
+/// a matching superclass.
+///
+/// Note that a class is not considered to be a subclass of itself.
+///
+/// Example matches implementation declarations for Y and Z.
+///   (matcher = objcInterfaceDecl(isSubclassOfInterface(hasName("X"
+/// \code
+///   @interface X
+///   @end
+///   @interface Y : X  // directly derived
+///   @end
+///   @interface Z : Y  // indirectly derived
+///   @end
+/// \endcode
+AST_MATCHER_P(ObjCInterfaceDecl, isSubclassOfInterface,
+  internal::Matcher,
+  InnerMatcher) {
+  // Check if any of the superclasses of the class match.
+  for (const ObjCInterfaceDecl *SuperClass = Node.getSuperClass();
+   SuperClass != nullptr; SuperClass = SuperClass->getSuperClass()) {
+if (InnerMatcher.matches(*SuperClass, Finder, Builder))
+  return true;
+  }
+
+  // No matches found.
+  return false;
+}
+
 /// Matches expressions that introduce cleanups to be run at the end
 /// of the sub-expression's evaluation.
 ///
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -6285,6 +6285,23 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCInterfaceDecl.html;>ObjCInterfaceDeclisSubclassOfInterfaceMatcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCInterfaceDecl.html;>ObjCInterfaceDecl InnerMatcher
+Matches Objective-C classes that directly or indirectly subclass
+a matching superclass.
+
+Note that a class is not considered to be a subclass of itself.
+
+Example matches implementation declarations for Y and Z.
+  (matcher = objcInterfaceDecl(isSubclassOfInterface(hasName("X"
+  @interface X
+  @end
+  @interface Y : X  // directly derived
+  @end
+  @interface Z : Y  // indirectly derived
+  @end
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html;>ObjCMessageExprhasAnyArgumentMatcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>Expr InnerMatcher
 Matches any argument of a call expression or a constructor call
 expression, or an ObjC-message-send expression.

[PATCH] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, aaron.ballman.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.

Fixes rdar://49523079


Repository:
  rC Clang

https://reviews.llvm.org/D60544

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/non-lazy-classes.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/attr-objc-non-lazy.m

Index: clang/test/SemaObjC/attr-objc-non-lazy.m
===
--- clang/test/SemaObjC/attr-objc-non-lazy.m
+++ clang/test/SemaObjC/attr-objc-non-lazy.m
@@ -29,7 +29,11 @@
 
 @interface E
 @end
-// expected-error@+1 {{'objc_nonlazy_class' attribute only applies to Objective-C interfaces}}
+
 __attribute__((objc_nonlazy_class))
 @implementation E
 @end
+
+__attribute__((objc_nonlazy_class))
+@implementation E (MyCat)
+@end
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -102,7 +102,7 @@
 // CHECK-NEXT: ObjCExplicitProtocolImpl (SubjectMatchRule_objc_protocol)
 // CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
-// CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface)
+// CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface, SubjectMatchRule_objc_implementation)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCRequiresPropertyDefs (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCRequiresSuper (SubjectMatchRule_objc_method)
Index: clang/test/CodeGenObjC/non-lazy-classes.m
===
--- clang/test/CodeGenObjC/non-lazy-classes.m
+++ clang/test/CodeGenObjC/non-lazy-classes.m
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -Wno-objc-root-class -emit-llvm -o - %s | \
-// RUN: FileCheck %s
-// CHECK: @"OBJC_LABEL_NONLAZY_CLASS_$" = private global [2 x {{.*}}]{{.*}}@"OBJC_CLASS_$_A"{{.*}},{{.*}}@"OBJC_CLASS_$_D"{{.*}} section "__DATA,__objc_nlclslist,regular,no_dead_strip", align 8
-// CHECK: @"OBJC_LABEL_NONLAZY_CATEGORY_$" = private global [1 x {{.*}}] {{.*}}@"\01l_OBJC_$_CATEGORY_A_$_Cat"{{.*}}, section "__DATA,__objc_nlcatlist,regular,no_dead_strip", align 8
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -Wno-objc-root-class -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: @"OBJC_LABEL_NONLAZY_CLASS_$" = private global [3 x {{.*}}]{{.*}}@"OBJC_CLASS_$_A"{{.*}},{{.*}}@"OBJC_CLASS_$_D"{{.*}},{{.*}}"OBJC_CLASS_$_E"{{.*}} section "__DATA,__objc_nlclslist,regular,no_dead_strip", align 8
+// CHECK: @"OBJC_LABEL_NONLAZY_CATEGORY_$" = private global [2 x {{.*}}] {{.*}}@"\01l_OBJC_$_CATEGORY_A_$_Cat"{{.*}},{{.*}}@"\01l_OBJC_$_CATEGORY_E_$_MyCat"{{.*}}, section "__DATA,__objc_nlcatlist,regular,no_dead_strip", align 8
 
 @interface A @end
 @implementation A
@@ -35,3 +35,11 @@
 @interface D @end
 
 @implementation D @end
+
+@interface E @end
+
+__attribute__((objc_nonlazy_class))
+@implementation E @end
+
+__attribute__((objc_nonlazy_class))
+@implementation E (MyCat) @end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -6262,7 +6262,8 @@
 bool CGObjCNonFragileABIMac::ImplementationIsNonLazy(
 const ObjCImplDecl *OD) const {
   return OD->getClassMethod(GetNullarySelector("load")) != nullptr ||
- OD->getClassInterface()->hasAttr();
+ OD->getClassInterface()->hasAttr() ||
+ OD->hasAttr();
 }
 
 void CGObjCNonFragileABIMac::GetClassSizeInfo(const ObjCImplementationDecl *OID,
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -3711,14 +3711,14 @@
 def ObjCNonLazyClassDocs : Documentation {
   let Category = DocCatDecl;
   let Content = [{
-This attribute can be added to an Objective-C ``@interface`` declaration to
-add the class to the list of non-lazily initialized classes. A non-lazy class
-will be initialized eagerly when the Objective-C runtime is loaded.  This is
-required for certain system classes which have instances allocated in
-non-standard ways, such as the classes for blocks and constant strings.  Adding
-this attribute is essentially equivalent to providing a trivial `+load` method 
-but avoids the (fairly small) load-time overheads associated with defining and
-calling such a method.
+This attribute can 

[PATCH] D59121: [analyzer] Fix macro names in diagnostics within bigger macros.

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

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

LGTM! Thanks!


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

https://reviews.llvm.org/D59121



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

[PATCH] D60543: [clang] Add matcher for subclasses of Objective-C interfaces 

[Stephane Moore via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:1479
+/// \endcode
+AST_MATCHER_P(ObjCInterfaceDecl, isSubclassOfInterface,
+  internal::Matcher,

I am still uncertain about the naming.

`isSubclassOf` seemed too generic as that could apply to C++ classes.
`objcIsSubclassOf` seemed unconventional as a matcher name.
`isSubclassOfObjCInterface` and `isSubclassOfObjCClass` seemed awkwardly 
lengthy.
Creating a new namespace `clang::ast_matchers::objc` seemed unprecedented.

I am happy to change the name if you think another name would be more 
appropriate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60543



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

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

[Jan Korous via Phabricator via cfe-commits]

jkorous updated this revision to Diff 194609.
jkorous added a comment.

rename RawCommentAndCacheFlags -> CommentAndOrigin


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

https://reviews.llvm.org/D60432

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp

Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -370,70 +370,69 @@
 const Decl **OriginalDecl) const {
   D = adjustDeclToTemplate(D);
 
-  // Check whether we have cached a comment for this declaration already.
-  {
-llvm::DenseMap::iterator Pos =
-RedeclComments.find(D);
+  auto CacheLookup = [&](const Decl *SpecificD) -> const RawComment * {
+llvm::DenseMap::iterator Pos =
+RedeclComments.find(SpecificD);
 if (Pos != RedeclComments.end()) {
-  const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-if (OriginalDecl)
-  *OriginalDecl = Raw.getOriginalDecl();
-return Raw.getRaw();
-  }
+  const CommentAndOrigin  = Pos->second;
+  if (OriginalDecl)
+*OriginalDecl = CachedComment.getOriginalDecl();
+  return CachedComment.getRaw();
 }
-  }
+return nullptr;
+  };
 
-  // Search for comments attached to declarations in the redeclaration chain.
-  const RawComment *RC = nullptr;
-  const Decl *OriginalDeclForRC = nullptr;
-  for (auto I : D->redecls()) {
-llvm::DenseMap::iterator Pos =
-RedeclComments.find(I);
-if (Pos != RedeclComments.end()) {
-  const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-RC = Raw.getRaw();
-OriginalDeclForRC = Raw.getOriginalDecl();
-break;
-  }
-} else {
-  RC = getRawCommentForDeclNoCache(I);
-  OriginalDeclForRC = I;
-  RawCommentAndCacheFlags Raw;
-  if (RC) {
-// Call order swapped to work around ICE in VS2015 RTM (Release Win32)
-// https://connect.microsoft.com/VisualStudio/feedback/details/1741530
-Raw.setKind(RawCommentAndCacheFlags::FromDecl);
-Raw.setRaw(RC);
-  } else
-Raw.setKind(RawCommentAndCacheFlags::NoCommentInDecl);
-  Raw.setOriginalDecl(I);
-  RedeclComments[I] = Raw;
-  if (RC)
-break;
+  auto GetCommentNoCache = [&](const Decl *SpecificD) -> const llvm::Optional {
+if (const RawComment *RC = getRawCommentForDeclNoCache(SpecificD)) {
+  // If we find a comment, it should be a documentation comment.
+  assert(RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+
+  CommentAndOrigin NewCachedComment;
+  // Call order swapped to work around ICE in VS2015 RTM (Release Win32)
+  // https://connect.microsoft.com/VisualStudio/feedback/details/1741530
+  NewCachedComment.setRaw(RC);
+  NewCachedComment.setOriginalDecl(SpecificD);
+
+  if (OriginalDecl)
+*OriginalDecl = SpecificD;
+
+  return NewCachedComment;
 }
-  }
+return llvm::None;
+  };
 
-  // If we found a comment, it should be a documentation comment.
-  assert(!RC || RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+  // Check whether we have cached a comment for this specific declaration.
+  if (auto * CachedComment = CacheLookup(D))
+return CachedComment;
 
-  if (OriginalDecl)
-*OriginalDecl = OriginalDeclForRC;
+  // Try to find comment for this specific declaration.
+  if (llvm::Optional OptCommentAndFlags = GetCommentNoCache(D)) {
+const CommentAndOrigin& CommentAndFlags = OptCommentAndFlags.getValue();
+RedeclComments[D] = CommentAndFlags;
+return CommentAndFlags.getRaw();
+  }
+
+  // In case this is the canonical Decl we're done.
+  auto CanonicalD = D->getCanonicalDecl();
+  if (!CanonicalD)
+return nullptr;
 
-  // Update cache for every declaration in the redeclaration chain.
-  RawCommentAndCacheFlags Raw;
-  Raw.setRaw(RC);
-  Raw.setKind(RawCommentAndCacheFlags::FromRedecl);
-  Raw.setOriginalDecl(OriginalDeclForRC);
+  // Check whether we have cached a comment for canonical declaration of this declaration.
+  if (auto * CachedComment = CacheLookup(CanonicalD))
+return CachedComment;
 
+  // We don't have comment for neither D, nor it's canonical declaration in cache and D doesn't have any comment attached to itself.
+  // Search for any comment attached to redeclarations of D and cache it for canonical declaration of D.
   for (auto I : D->redecls()) {
-RawCommentAndCacheFlags  = RedeclComments[I];
-if (R.getKind() == RawCommentAndCacheFlags::NoCommentInDecl)
-  R = Raw;
+if (llvm::Optional OptCommentAndFlags = GetCommentNoCache(I)) {
+  const CommentAndOrigin& CommentAndFlags = OptCommentAndFlags.getValue();
+  RedeclComments[CanonicalD] = 

[PATCH] D55409: [clang-tidy] check for using declarations not in an anonymous namespace when there exists one

[Shyan Akmal via Phabricator via cfe-commits]

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



Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.cpp:41
+// to the vector containing all candidate using declarations.
+if (AnonymousNamespaceDecl) {
+   diag(MatchedUsingDecl->getLocation(),

aaron.ballman wrote:
> I don't think this logic works in practice because there's no way to 
> determine that the anonymous namespace is even a candidate for putting the 
> using declaration into it. Consider a common scenario where there's an 
> anonymous namespace declared in a header file (like an STL header outside of 
> the user's control), and a using declaration inside of an implementation 
> file. Just because the STL declared an anonymous namespace doesn't mean that 
> the user could have put their using declaration in it.
If we altered the check to only apply to anonymous namespaces and using 
declarations at namespace scope (that is, we only suggest aliases be moved to 
anonymous namespaces when the unnamed namespace and alias are themselves inside 
some other namespace), would this issue be resolved? 


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

https://reviews.llvm.org/D55409



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

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Ryan Piantedosi via Phabricator via cfe-commits]

Dosi-Dough updated this revision to Diff 194618.
Dosi-Dough marked 3 inline comments as done.
Dosi-Dough added a comment.

removed parens from turnary, added whitespace.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/WrapUniqueCheck.cpp
  clang-tidy/abseil/WrapUniqueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-wrap-unique.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-wrap-unique.cpp

Index: test/clang-tidy/abseil-wrap-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-wrap-unique.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s abseil-wrap-unique %t
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr ) = delete;
+  unique_ptr(unique_ptr &) {}
+  ~unique_ptr() {}
+  type *() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr =(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr =(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+}  // namespace std
+
+class A {
+ public:
+  static A* NewA() {
+return new A();
+  }
+
+ private:
+  A() {}
+};
+
+class B {
+ public:
+  static B* NewB(int bIn) {
+return new B();
+  }
+
+ private:
+  B() {}
+};
+
+struct C {
+  int x;
+  int y;
+};
+/*
+std::unique_ptr returnPointer() {
+  return std::unique_ptr(A::NewA());
+}
+*/
+void positives() {
+  std::unique_ptr a;
+  a.reset(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: a = absl::WrapUnique(A::NewA())
+  
+  std::unique_ptr b(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto b = absl::WrapUnique(A::NewA())
+
+  int cIn;
+  std::unique_ptr c(B::NewB(cIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto c = absl::WrapUnique(B::NewB(cIn))
+
+  int dIn;
+  std::unique_ptr d;
+  d.reset(B::NewB(dIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: d = absl::WrapUnique(B::NewB(dIn))
+  
+  auto e = std::unique_ptr(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: e = absl::WrapUnique(A::NewA())
+
+  //std::unique_ptr e(new int[2] {1,2});
+}
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -20,6 +20,7 @@
abseil-string-find-startswith
abseil-time-subtraction
abseil-upgrade-duration-conversions
+   abseil-wrap-unique
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-wrap-unique.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-wrap-unique.rst
@@ -0,0 +1,34 @@
+.. title:: clang-tidy - abseil-wrap-unique
+
+abseil-wrap-unique
+==
+Looks for instances of factory functions which uses a non-public constructor
+
+that returns a ``std::unqiue_ptr`` then recommends using  ``absl::wrap_unique(new T(...))``.
+
+Examples:
+
+.. code-block:: c++
+ 
+  class A {
+  public:
+static A* NewA() { return new A(); }
+
+  private:
+A() = default; 
+  };
+
+  std::unique_ptr a;
+
+  // Original - reset called with a static function returning a std::unqiue_ptr
+  a.reset(A::NewA());
+
+  // Suggested - reset ptr with absl::WrapUnique
+  a = absl::WrapUnique(A::NewA());
+
+  // Original - std::unique_ptr initialized with static function
+  std::unique_ptr b(A::NewA());
+
+  // Suggested - initialize with absl::WrapUnique instead
+  auto b = absl::WrapUnique(A::NewA())
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -91,6 +91,13 @@
   Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
   in the Time domain instead of the numeric domain.
 
+- New :doc:`abseil-wrap-unique
+  ` check.
+
+  Looks for instances of factory functions which uses a non-public constructor
+  that returns a ``std::unqiue_ptr`` then recommends using 
+  ``absl::wrap_unique(new T(...))``.
+
 - New :doc:`google-readability-avoid-underscore-in-googletest-name
   `
   check.
Index: clang-tidy/abseil/WrapUniqueCheck.h

RE: r358104 - Don't emit an unreachable return block.

[via cfe-commits]

Hi, 
This commit seems to be causing a test failure on several buildbots (e.g. 
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/26305). 
instrprof-shared-gcov-flush.test is failing because of differences in profile 
info. The test passes when I revert your change, so perhaps it's just a case of 
updating the test.
Could you please take a look? 

Thanks,
Wolfgang Pieb

-Original Message-
From: cfe-commits  On Behalf Of John McCall 
via cfe-commits
Sent: Wednesday, April 10, 2019 10:03 AM
To: cfe-commits@lists.llvm.org
Subject: r358104 - Don't emit an unreachable return block.

Author: rjmccall
Date: Wed Apr 10 10:03:09 2019
New Revision: 358104

URL: http://llvm.org/viewvc/llvm-project?rev=358104=rev
Log:
Don't emit an unreachable return block.

Patch by Brad Moody.

Added:
cfe/trunk/test/CodeGen/unreachable-ret.c
Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=358104=358103=358104=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Apr 10 10:03:09 2019
@@ -2873,15 +2873,6 @@ void CodeGenFunction::EmitFunctionEpilog
 RV = SI->getValueOperand();
 SI->eraseFromParent();
 
-// If that was the only use of the return value, nuke it as well now.
-auto returnValueInst = ReturnValue.getPointer();
-if (returnValueInst->use_empty()) {
-  if (auto alloca = dyn_cast(returnValueInst)) {
-alloca->eraseFromParent();
-ReturnValue = Address::invalid();
-  }
-}
-
   // Otherwise, we have to do a simple load.
   } else {
 RV = Builder.CreateLoad(ReturnValue);

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=358104=358103=358104=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Apr 10 10:03:09 2019
@@ -255,6 +255,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
 if (CurBB->empty() || ReturnBlock.getBlock()->use_empty()) {
   ReturnBlock.getBlock()->replaceAllUsesWith(CurBB);
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
 } else
   EmitBlock(ReturnBlock.getBlock());
 return llvm::DebugLoc();
@@ -274,6 +275,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
   Builder.SetInsertPoint(BI->getParent());
   BI->eraseFromParent();
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
   return Loc;
 }
   }
@@ -448,6 +450,19 @@ void CodeGenFunction::FinishFunction(Sou
   // 5. Width of vector aguments and return types for functions called by this
   //function.
   CurFn->addFnAttr("min-legal-vector-width", llvm::utostr(LargestVectorWidth));
+
+  // If we generated an unreachable return block, delete it now.
+  if (ReturnBlock.isValid() && ReturnBlock.getBlock()->use_empty()) {
+Builder.ClearInsertionPoint();
+ReturnBlock.getBlock()->eraseFromParent();
+  }
+  if (ReturnValue.isValid()) {
+auto *RetAlloca = dyn_cast(ReturnValue.getPointer());
+if (RetAlloca && RetAlloca->use_empty()) {
+  RetAlloca->eraseFromParent();
+  ReturnValue = Address::invalid();
+}
+  }
 }
 
 /// ShouldInstrumentFunction - Return true if the current function should be

Added: cfe/trunk/test/CodeGen/unreachable-ret.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/unreachable-ret.c?rev=358104=auto
==
--- cfe/trunk/test/CodeGen/unreachable-ret.c (added)
+++ cfe/trunk/test/CodeGen/unreachable-ret.c Wed Apr 10 10:03:09 2019
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+extern void abort() __attribute__((noreturn));
+
+void f1() {
+  abort();
+}
+// CHECK-LABEL: define void @f1()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+
+void *f2() {
+  abort();
+  return 0;
+}
+// CHECK-LABEL: define i8* @f2()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+

Modified: cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp?rev=358104=358103=358104=diff
==
--- cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp Wed Apr 10 
+++ 10:03:09 2019
@@ -622,7 +622,7 @@ int main() {
 
 // CHECK-NOT: call i32 

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

[Jan Korous via Phabricator via cfe-commits]

jkorous updated this revision to Diff 194604.
jkorous added a comment.

Second attempt on reducing the cache size and number of operations.

I try in this order

1. cache lookup for the specific provided declaration
2. try to find comment attached to the specific provided declaration without 
using cache (and cache it using the specific declaration as a key)
3. cache lookup using the canonical declaration (which would return comment 
from any redeclaration - see the next step)
4. try to find comment attached to any redeclaration (and cache it using the 
canonical declaration as a key)
5. give up, return nullptr


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

https://reviews.llvm.org/D60432

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp

Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -370,70 +370,69 @@
 const Decl **OriginalDecl) const {
   D = adjustDeclToTemplate(D);
 
-  // Check whether we have cached a comment for this declaration already.
-  {
+  auto CacheLookup = [&](const Decl *SpecificD) -> const RawComment * {
 llvm::DenseMap::iterator Pos =
-RedeclComments.find(D);
+RedeclComments.find(SpecificD);
 if (Pos != RedeclComments.end()) {
-  const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-if (OriginalDecl)
-  *OriginalDecl = Raw.getOriginalDecl();
-return Raw.getRaw();
-  }
+  const RawCommentAndCacheFlags  = Pos->second;
+  if (OriginalDecl)
+*OriginalDecl = CachedComment.getOriginalDecl();
+  return CachedComment.getRaw();
 }
-  }
+return nullptr;
+  };
 
-  // Search for comments attached to declarations in the redeclaration chain.
-  const RawComment *RC = nullptr;
-  const Decl *OriginalDeclForRC = nullptr;
-  for (auto I : D->redecls()) {
-llvm::DenseMap::iterator Pos =
-RedeclComments.find(I);
-if (Pos != RedeclComments.end()) {
-  const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-RC = Raw.getRaw();
-OriginalDeclForRC = Raw.getOriginalDecl();
-break;
-  }
-} else {
-  RC = getRawCommentForDeclNoCache(I);
-  OriginalDeclForRC = I;
-  RawCommentAndCacheFlags Raw;
-  if (RC) {
-// Call order swapped to work around ICE in VS2015 RTM (Release Win32)
-// https://connect.microsoft.com/VisualStudio/feedback/details/1741530
-Raw.setKind(RawCommentAndCacheFlags::FromDecl);
-Raw.setRaw(RC);
-  } else
-Raw.setKind(RawCommentAndCacheFlags::NoCommentInDecl);
-  Raw.setOriginalDecl(I);
-  RedeclComments[I] = Raw;
-  if (RC)
-break;
+  auto GetCommentNoCache = [&](const Decl *SpecificD) -> const llvm::Optional {
+if (const RawComment *RC = getRawCommentForDeclNoCache(SpecificD)) {
+  // If we find a comment, it should be a documentation comment.
+  assert(RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+
+  RawCommentAndCacheFlags NewCachedComment;
+  // Call order swapped to work around ICE in VS2015 RTM (Release Win32)
+  // https://connect.microsoft.com/VisualStudio/feedback/details/1741530
+  NewCachedComment.setRaw(RC);
+  NewCachedComment.setOriginalDecl(SpecificD);
+
+  if (OriginalDecl)
+*OriginalDecl = SpecificD;
+
+  return NewCachedComment;
 }
-  }
+return llvm::None;
+  };
 
-  // If we found a comment, it should be a documentation comment.
-  assert(!RC || RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+  // Check whether we have cached a comment for this specific declaration.
+  if (auto * CachedComment = CacheLookup(D))
+return CachedComment;
 
-  if (OriginalDecl)
-*OriginalDecl = OriginalDeclForRC;
+  // Try to find comment for this specific declaration.
+  if (llvm::Optional OptCommentAndFlags = GetCommentNoCache(D)) {
+const RawCommentAndCacheFlags& CommentAndFlags = OptCommentAndFlags.getValue();
+RedeclComments[D] = CommentAndFlags;
+return CommentAndFlags.getRaw();
+  }
+
+  // In case this is the canonical Decl we're done.
+  auto CanonicalD = D->getCanonicalDecl();
+  if (!CanonicalD)
+return nullptr;
 
-  // Update cache for every declaration in the redeclaration chain.
-  RawCommentAndCacheFlags Raw;
-  Raw.setRaw(RC);
-  Raw.setKind(RawCommentAndCacheFlags::FromRedecl);
-  Raw.setOriginalDecl(OriginalDeclForRC);
+  // Check whether we have cached a comment for canonical declaration of this declaration.
+  if (auto * CachedComment = CacheLookup(CanonicalD))
+return CachedComment;
 
+  // We don't have comment for neither D, nor it's canonical declaration in cache and D doesn't have any comment 

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Ryan Piantedosi via Phabricator via cfe-commits]

Dosi-Dough updated this revision to Diff 194607.
Dosi-Dough marked an inline comment as done.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/WrapUniqueCheck.cpp
  clang-tidy/abseil/WrapUniqueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-wrap-unique.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-wrap-unique.cpp

Index: test/clang-tidy/abseil-wrap-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-wrap-unique.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s abseil-wrap-unique %t
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr ) = delete;
+  unique_ptr(unique_ptr &) {}
+  ~unique_ptr() {}
+  type *() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr =(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr =(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+}  // namespace std
+
+class A {
+ public:
+  static A* NewA() {
+return new A();
+  }
+
+ private:
+  A() {}
+};
+
+class B {
+ public:
+  static B* NewB(int bIn) {
+return new B();
+  }
+
+ private:
+  B() {}
+};
+
+struct C {
+  int x;
+  int y;
+};
+/*
+std::unique_ptr returnPointer() {
+  return std::unique_ptr(A::NewA());
+}
+*/
+void positives() {
+  std::unique_ptr a;
+  a.reset(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: a = absl::WrapUnique(A::NewA())
+  
+  std::unique_ptr b(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto b = absl::WrapUnique(A::NewA())
+
+  int cIn;
+  std::unique_ptr c(B::NewB(cIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto c = absl::WrapUnique(B::NewB(cIn))
+
+  int dIn;
+  std::unique_ptr d;
+  d.reset(B::NewB(dIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: d = absl::WrapUnique(B::NewB(dIn))
+  
+  auto e = std::unique_ptr(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: e = absl::WrapUnique(A::NewA())
+
+  //std::unique_ptr e(new int[2] {1,2});
+}
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -20,6 +20,7 @@
abseil-string-find-startswith
abseil-time-subtraction
abseil-upgrade-duration-conversions
+   abseil-wrap-unique
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-wrap-unique.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-wrap-unique.rst
@@ -0,0 +1,32 @@
+.. title:: clang-tidy - abseil-wrap-unique
+
+abseil-wrap-unique
+==
+Looks for instances of factory functions which uses a non-public constructor
+that returns a ``std::unqiue_ptr`` then recommends using 
+``absl::wrap_unique(new T(...))``.
+
+.. code-block:: c++
+ 
+  class A {
+  public:
+static A* NewA() { return new A(); }
+
+  private:
+A() = default; 
+  };
+
+  std::unique_ptr a;
+
+  // Original - reset called with a static function returning a std::unqiue_ptr
+  a.reset(A::NewA());
+
+  // Suggested - reset ptr with absl::WrapUnique
+  a = absl::WrapUnique(A::NewA());
+
+  // Original - std::unique_ptr initialized with static function
+  std::unique_ptr b(A::NewA());
+
+  // Suggested - initialize with absl::WrapUnique instead
+  auto b = absl::WrapUnique(A::NewA())
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -91,6 +91,13 @@
   Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
   in the Time domain instead of the numeric domain.
 
+- New :doc:`abseil-wrap-unique
+  ` check.
+
+  Looks for instances of factory functions which uses a non-public constructor
+  that returns a ``std::unqiue_ptr`` then recommends using 
+  ``absl::wrap_unique(new T(...))``.
+
 - New :doc:`google-readability-avoid-underscore-in-googletest-name
   `
   check.
Index: clang-tidy/abseil/WrapUniqueCheck.h
===
--- /dev/null
+++ 

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/abseil-wrap-unique.rst:5
+==
+Looks for instances of factory functions which uses a non-public constructor
+that returns a ``std::unqiue_ptr`` then recommends using  
``absl::wrap_unique(new T(...))``.

Eugene.Zelenko wrote:
> Please separate with empty line.
Sorry, if I was not clear. I meant empty line between ==  and 
statement.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435



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

[PATCH] D60552: Enable intrinsics of AVX512_BF16, which are supported for BFLOAT16 in Cooper Lake

[Tianle Liu via Phabricator via cfe-commits]

liutianle created this revision.
liutianle added reviewers: craig.topper, smaslov, LuoYuanke, wxiao3, 
annita.zhang.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.

1. Enable infrastructure of AVX512_BF16, which is supported for BFLOAT16 in 
Cooper Lake;
2. Enable intrinsics for VCVTNE2PS2BF16, VCVTNEPS2BF16 and DPBF16PS 
instructions, which are Vector Neural Network Instructions supporting BFLOAT16 
inputs and conversion instructions from IEEE single precision.

For more details about BF16 intrinsic, please refer to the latest ISE document: 
https://software.intel.com/en-us/download/intel-architecture-instruction-set-extensions-programming-reference


Repository:
  rC Clang

https://reviews.llvm.org/D60552

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Basic/BuiltinsX86.def
  include/clang/Driver/Options.td
  lib/Basic/Targets/X86.cpp
  lib/Basic/Targets/X86.h
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/CMakeLists.txt
  lib/Headers/avx512bf16intrin.h
  lib/Headers/avx512vlbf16intrin.h
  lib/Headers/cpuid.h
  lib/Headers/immintrin.h
  test/CodeGen/attr-target-x86.c
  test/CodeGen/avx512bf16-builtins.c
  test/CodeGen/avx512vlbf16-builtins.c

Index: test/CodeGen/avx512vlbf16-builtins.c
===
--- /dev/null
+++ test/CodeGen/avx512vlbf16-builtins.c
@@ -0,0 +1,142 @@
+//  RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin \
+//  RUN:-target-feature +avx512bf16 -target-feature \
+//  RUN:+avx512vl -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+#include 
+
+__m128bh test_mm_cvtne2ps2bf16(__m128 A, __m128 B) {
+  // CHECK-LABEL: @test_mm_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.128
+  return _mm_cvtne2ps_pbh(A, B);
+}
+
+__m128bh test_mm_maskz_cvtne2ps2bf16(__m128 A, __m128 B, __mmask8 U) {
+  // CHECK-LABEL: @test_mm_maskz_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.128
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i16> %{{.*}}, <8 x i16> %{{.*}}
+  return _mm_maskz_cvtne2ps_pbh(U, A, B);
+}
+
+__m128bh test_mm_mask_cvtne2ps2bf16(__m128bh C, __mmask8 U, __m128 A, __m128 B) {
+  // CHECK-LABEL: @test_mm_mask_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.128
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i16> %{{.*}}, <8 x i16> %{{.*}}
+  return _mm_mask_cvtne2ps_pbh(C, U, A, B);
+}
+
+__m256bh test_mm256_cvtne2ps2bf16(__m256 A, __m256 B) {
+  // CHECK-LABEL: @test_mm256_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.256
+  return _mm256_cvtne2ps_pbh(A, B);
+}
+
+__m256bh test_mm256_maskz_cvtne2ps2bf16(__m256 A, __m256 B, __mmask16 U) {
+  // CHECK-LABEL: @test_mm256_maskz_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.256
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> %{{.*}}
+  return _mm256_maskz_cvtne2ps_pbh(U, A, B);
+}
+
+__m256bh test_mm256_mask_cvtne2ps2bf16(__m256bh C, __mmask16 U, __m256 A, __m256 B) {
+  // CHECK-LABEL: @test_mm256_mask_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.256
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> %{{.*}}
+  return _mm256_mask_cvtne2ps_pbh(C, U, A, B);
+}
+
+__m512bh test_mm512_cvtne2ps2bf16(__m512 A, __m512 B) {
+  // CHECK-LABEL: @test_mm512_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.512
+  return _mm512_cvtne2ps_pbh(A, B);
+}
+
+__m512bh test_mm512_maskz_cvtne2ps2bf16(__m512 A, __m512 B, __mmask32 U) {
+  // CHECK-LABEL: @test_mm512_maskz_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.512
+  // CHECK: select <32 x i1> %{{.*}}, <32 x i16> %{{.*}}, <32 x i16> %{{.*}}
+  return _mm512_maskz_cvtne2ps_pbh(U, A, B);
+}
+
+__m512bh test_mm512_mask_cvtne2ps2bf16(__m512bh C, __mmask32 U, __m512 A, __m512 B) {
+  // CHECK-LABEL: @test_mm512_mask_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.512
+  // CHECK: select <32 x i1> %{{.*}}, <32 x i16> %{{.*}}, <32 x i16> %{{.*}}
+  return _mm512_mask_cvtne2ps_pbh(C, U, A, B);
+}
+
+__m128bh test_mm_cvtneps2bf16(__m128 A) {
+  // CHECK-LABEL: @test_mm_cvtneps2bf16
+  // CHECK: @llvm.x86.avx512bf16.mask.cvtneps2bf16.128
+  return _mm_cvtneps_pbh(A);
+}
+
+__m128bh test_mm_mask_cvtneps2bf16(__m128bh C, __mmask8 U, __m128 A) {
+  // CHECK-LABEL: @test_mm_mask_cvtneps2bf16
+  // CHECK: @llvm.x86.avx512bf16.mask.cvtneps2bf16.
+  return _mm_mask_cvtneps_pbh(C, U, A);
+}
+
+__m128bh test_mm_maskz_cvtneps2bf16(__m128 A, __mmask8 U) {
+  // CHECK-LABEL: @test_mm_maskz_cvtneps2bf16
+  // CHECK: @llvm.x86.avx512bf16.mask.cvtneps2bf16.128
+  return _mm_maskz_cvtneps_pbh(U, A);
+}
+
+__m128bh test_mm256_cvtneps2bf16(__m256 A) {
+  // CHECK-LABEL: @test_mm256_cvtneps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtneps2bf16.256
+  return _mm256_cvtneps_pbh(A);
+}
+
+__m128bh test_mm256_mask_cvtneps2bf16(__m128bh C, __mmask8 U, __m256 A) {
+  // CHECK-LABEL: @test_mm256_mask_cvtneps2bf16
+  // CHECK: 

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

One downside to alloca is that we can's use `__attribute__((uninitialized))` 
because it's a builtin. Maybe we need a separate uninitialized alloca? What do 
you all think?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb created this revision.
Herald added subscribers: cfe-commits, dexonsmith, jkorous.
Herald added a project: clang.

alloca isn’t auto-init’d right now because it’s a different path in clang that 
all the other stuff we support (it’s a builtin, not an expression). 
Interestingly, alloca doesn’t have a type (as opposed to even VLA) so we can 
really only initialize it with memset.

rdar://problem/49794007


Repository:
  rC Clang

https://reviews.llvm.org/D60548

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGenCXX/trivial-auto-var-init.cpp


Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -173,6 +173,34 @@
   used(ptr);
 }
 
+// UNINIT-LABEL:  test_alloca(
+// ZERO-LABEL:test_alloca(
+// ZERO:  %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// ZERO-NEXT: %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 
[[ALIGN:[0-9]+]]
+// ZERO-NEXT: call void @llvm.memset{{.*}}(i8* align [[ALIGN]] 
%[[ALLOCA]], i8 0, i64 %[[SIZE]], i1 false)
+// PATTERN-LABEL: test_alloca(
+// PATTERN:   %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// PATTERN-NEXT:  %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 
[[ALIGN:[0-9]+]]
+// PATTERN-NEXT:  call void @llvm.memset{{.*}}(i8* align [[ALIGN]] 
%[[ALLOCA]], i8 -86, i64 %[[SIZE]], i1 false)
+void test_alloca(int size) {
+  void *ptr = __builtin_alloca(size);
+  used(ptr);
+}
+
+// UNINIT-LABEL:  test_alloca_with_align(
+// ZERO-LABEL:test_alloca_with_align(
+// ZERO:  %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// ZERO-NEXT: %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 128
+// ZERO-NEXT: call void @llvm.memset{{.*}}(i8* align 128 %[[ALLOCA]], i8 
0, i64 %[[SIZE]], i1 false)
+// PATTERN-LABEL: test_alloca_with_align(
+// PATTERN:   %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// PATTERN-NEXT:  %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 128
+// PATTERN-NEXT:  call void @llvm.memset{{.*}}(i8* align 128 %[[ALLOCA]], i8 
-86, i64 %[[SIZE]], i1 false)
+void test_alloca_with_align(int size) {
+  void *ptr = __builtin_alloca_with_align(size, 1024);
+  used(ptr);
+}
+
 // UNINIT-LABEL:  test_struct_vla(
 // ZERO-LABEL:test_struct_vla(
 // ZERO:  %[[SIZE:[0-9]+]] = mul nuw i64 %{{.*}}, 16
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -44,6 +44,22 @@
   return std::min(High, std::max(Low, Value));
 }
 
+static void initializeAlloca(CodeGenFunction , AllocaInst *AI, Value 
*Size, unsigned AlignmentInBytes) {
+  ConstantInt *Byte;
+  switch (CGF.getLangOpts().getTrivialAutoVarInit()) {
+  case LangOptions::TrivialAutoVarInitKind::Uninitialized:
+// Nothing to initialize.
+return;
+  case LangOptions::TrivialAutoVarInitKind::Zero:
+Byte = CGF.Builder.getInt8(0);
+break;
+  case LangOptions::TrivialAutoVarInitKind::Pattern:
+Byte = CGF.Builder.getInt8(0xAA);
+break;
+  }
+  CGF.Builder.CreateMemSet(AI, Byte, Size, AlignmentInBytes);
+}
+
 /// getBuiltinLibFunction - Given a builtin id for a function like
 /// "__builtin_fabsf", return a Function* for "fabsf".
 llvm::Constant *CodeGenModule::getBuiltinLibFunction(const FunctionDecl *FD,
@@ -2259,6 +2275,7 @@
 .getQuantity();
 AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
 AI->setAlignment(SuitableAlignmentInBytes);
+initializeAlloca(*this, AI, Size, SuitableAlignmentInBytes);
 return RValue::get(AI);
   }
 
@@ -2271,6 +2288,7 @@
 CGM.getContext().toCharUnitsFromBits(AlignmentInBits).getQuantity();
 AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
 AI->setAlignment(AlignmentInBytes);
+initializeAlloca(*this, AI, Size, AlignmentInBytes);
 return RValue::get(AI);
   }
 


Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -173,6 +173,34 @@
   used(ptr);
 }
 
+// UNINIT-LABEL:  test_alloca(
+// ZERO-LABEL:test_alloca(
+// ZERO:  %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// ZERO-NEXT: %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align [[ALIGN:[0-9]+]]
+// ZERO-NEXT: call void @llvm.memset{{.*}}(i8* align [[ALIGN]] %[[ALLOCA]], i8 0, i64 %[[SIZE]], i1 false)
+// PATTERN-LABEL: test_alloca(
+// PATTERN:   %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// PATTERN-NEXT:  %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align [[ALIGN:[0-9]+]]
+// PATTERN-NEXT:  call void @llvm.memset{{.*}}(i8* align [[ALIGN]] %[[ALLOCA]], i8 -86, i64 %[[SIZE]], i1 false)
+void test_alloca(int size) {
+  void *ptr = __builtin_alloca(size);
+  used(ptr);
+}
+
+// UNINIT-LABEL:  

[PATCH] D59924: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

[Chaofan Qiu via Phabricator via cfe-commits]

qiucf marked an inline comment as done.
qiucf added a comment.

Part of a function signature can also match where it is called, so the test 
failed. I did the changes to pass the test but diff file was generated locally 
so it didn't contain this change. Sorry for the mistake.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59924



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

[PATCH] D60302: [CodeGen][ObjC] Emit the retainRV marker as a module flag instead of named metadata.

[Akira Hatanaka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358048: [CodeGen][ObjC] Emit the retainRV marker as a module 
flag instead of (authored by ahatanak, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60302?vs=193823=194450#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60302

Files:
  cfe/trunk/lib/CodeGen/CGObjC.cpp
  cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m


Index: cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m
===
--- cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m
+++ cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m
@@ -231,4 +231,5 @@
 
 // This is always at the end of the module.
 
-// CHECK-OPTIMIZED: !clang.arc.retainAutoreleasedReturnValueMarker = !{!0}
+// CHECK-OPTIMIZED: !llvm.module.flags = !{!0,
+// CHECK-OPTIMIZED: !0 = !{i32 1, 
!"clang.arc.retainAutoreleasedReturnValueMarker", !"mov{{.*}}marker for 
objc_retainAutoreleaseReturnValue"}
Index: cfe/trunk/lib/CodeGen/CGObjC.cpp
===
--- cfe/trunk/lib/CodeGen/CGObjC.cpp
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp
@@ -2161,14 +2161,10 @@
 // with this marker yet, so leave a breadcrumb for the ARC
 // optimizer to pick up.
 } else {
-  llvm::NamedMDNode *metadata =
-CGF.CGM.getModule().getOrInsertNamedMetadata(
-"clang.arc.retainAutoreleasedReturnValueMarker");
-  assert(metadata->getNumOperands() <= 1);
-  if (metadata->getNumOperands() == 0) {
-auto  = CGF.getLLVMContext();
-metadata->addOperand(llvm::MDNode::get(ctx,
- llvm::MDString::get(ctx, assembly)));
+  const char *markerKey = "clang.arc.retainAutoreleasedReturnValueMarker";
+  if (!CGF.CGM.getModule().getModuleFlag(markerKey)) {
+auto *str = llvm::MDString::get(CGF.getLLVMContext(), assembly);
+CGF.CGM.getModule().addModuleFlag(llvm::Module::Error, markerKey, str);
   }
 }
   }


Index: cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m
===
--- cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m
+++ cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m
@@ -231,4 +231,5 @@
 
 // This is always at the end of the module.
 
-// CHECK-OPTIMIZED: !clang.arc.retainAutoreleasedReturnValueMarker = !{!0}
+// CHECK-OPTIMIZED: !llvm.module.flags = !{!0,
+// CHECK-OPTIMIZED: !0 = !{i32 1, !"clang.arc.retainAutoreleasedReturnValueMarker", !"mov{{.*}}marker for objc_retainAutoreleaseReturnValue"}
Index: cfe/trunk/lib/CodeGen/CGObjC.cpp
===
--- cfe/trunk/lib/CodeGen/CGObjC.cpp
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp
@@ -2161,14 +2161,10 @@
 // with this marker yet, so leave a breadcrumb for the ARC
 // optimizer to pick up.
 } else {
-  llvm::NamedMDNode *metadata =
-CGF.CGM.getModule().getOrInsertNamedMetadata(
-"clang.arc.retainAutoreleasedReturnValueMarker");
-  assert(metadata->getNumOperands() <= 1);
-  if (metadata->getNumOperands() == 0) {
-auto  = CGF.getLLVMContext();
-metadata->addOperand(llvm::MDNode::get(ctx,
- llvm::MDString::get(ctx, assembly)));
+  const char *markerKey = "clang.arc.retainAutoreleasedReturnValueMarker";
+  if (!CGF.CGM.getModule().getModuleFlag(markerKey)) {
+auto *str = llvm::MDString::get(CGF.getLLVMContext(), assembly);
+CGF.CGM.getModule().addModuleFlag(llvm::Module::Error, markerKey, str);
   }
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r358048 - [CodeGen][ObjC] Emit the retainRV marker as a module flag instead of

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Tue Apr  9 23:20:23 2019
New Revision: 358048

URL: http://llvm.org/viewvc/llvm-project?rev=358048=rev
Log:
[CodeGen][ObjC] Emit the retainRV marker as a module flag instead of
named metadata.

This fixes a bug where ARC contract wasn't inserting the retainRV
marker when LTO was enabled, which caused objects returned from a
function to be auto-released.

rdar://problem/49464214

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

Modified:
cfe/trunk/lib/CodeGen/CGObjC.cpp
cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m

Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=358048=358047=358048=diff
==
--- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp Tue Apr  9 23:20:23 2019
@@ -2161,14 +2161,10 @@ static void emitAutoreleasedReturnValueM
 // with this marker yet, so leave a breadcrumb for the ARC
 // optimizer to pick up.
 } else {
-  llvm::NamedMDNode *metadata =
-CGF.CGM.getModule().getOrInsertNamedMetadata(
-"clang.arc.retainAutoreleasedReturnValueMarker");
-  assert(metadata->getNumOperands() <= 1);
-  if (metadata->getNumOperands() == 0) {
-auto  = CGF.getLLVMContext();
-metadata->addOperand(llvm::MDNode::get(ctx,
- llvm::MDString::get(ctx, assembly)));
+  const char *markerKey = "clang.arc.retainAutoreleasedReturnValueMarker";
+  if (!CGF.CGM.getModule().getModuleFlag(markerKey)) {
+auto *str = llvm::MDString::get(CGF.getLLVMContext(), assembly);
+CGF.CGM.getModule().addModuleFlag(llvm::Module::Error, markerKey, str);
   }
 }
   }

Modified: cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m?rev=358048=358047=358048=diff
==
--- cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m (original)
+++ cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m Tue Apr  9 23:20:23 2019
@@ -231,4 +231,5 @@ void test_cast_to_void() {
 
 // This is always at the end of the module.
 
-// CHECK-OPTIMIZED: !clang.arc.retainAutoreleasedReturnValueMarker = !{!0}
+// CHECK-OPTIMIZED: !llvm.module.flags = !{!0,
+// CHECK-OPTIMIZED: !0 = !{i32 1, 
!"clang.arc.retainAutoreleasedReturnValueMarker", !"mov{{.*}}marker for 
objc_retainAutoreleaseReturnValue"}


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

[PATCH] D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clangd/CodeComplete.cpp:545
const CodeCompleteOptions ) {
-  auto GetAllAccessibleScopes = [](CodeCompletionContext ) {
-SpecifiedScope Info;
-for (auto *Context : CCContext.getVisitedContexts()) {
-  if (isa(Context))
-Info.AccessibleScopes.push_back(""); // global namespace
-  else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
-}
-return Info;
-  };
-
-  auto SS = CCContext.getCXXScopeSpecifier();
+  SpecifiedScope Scopes;
+  for (auto *Context : CCContext.getVisitedContexts()) {

(AFAICT this lambda was executed the same way on every code path, so I just 
inlined it)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60503



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

[PATCH] D60463: [ASTImporter] Add check for correct import of source locations.

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

balazske added a comment.

This test will work theoretically only if the order of every imported Decl (and 
not only FieldDecl) is correct, this is not the case now. So probably a better 
solution for the problem should be found: Enumerate and match (the From and To) 
SourceLocations with AST visitor. There should be some existing code that is 
doing somewhat similar in clang-query but I did not find it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60463



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

[PATCH] D60029: Add const children() accessors to all AST nodes.

[Nicolas Manichon via Phabricator via cfe-commits]

nicolas added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60029



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

[PATCH] D60055: Check i < FD->getNumParams() before querying

[Dmitri Gribenko via Phabricator via cfe-commits]

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

This fix makes sense to me, however, Richard Smith @rsmith is the best person 
to review this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60055



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

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/include/clang/AST/ASTContext.h:756
   /// lazily.
   mutable llvm::DenseMap RedeclComments;
 

The name of this variable (and `RawCommentAndCacheFlags`) don't make sense 
after the change.



Comment at: clang/lib/AST/ASTContext.cpp:372
   D = adjustDeclToTemplate(D);
+  const Decl* CanonicalDecl = D->getCanonicalDecl();
 

jkorous wrote:
> arphaman wrote:
> > Why are we now checking for the canonical declaration instead of `D` as 
> > before?
> Before this we were caching comment for every redeclaration separately but 
> for every redeclaration the comment was the same.
> 
> As I understand it - for a given declaration we found the first comment in 
> the redeclaration chain and then saved it to the cache for every 
> redeclaration (the same comment).
> Later, during lookup, we iterated the redeclaration chain again and did a 
> lookup for every redeclaration (see L392 in the original implementation).
> 
> Unless I missed something, it's suboptimal in both memory consumption and 
> runtime overhead.
> 
> That's the reason why I want to cache the comment for the redeclaration group 
> as a whole. The only thing I am not entirely sure about is what key to use to 
> represent the whole redeclaration chain - maybe the first declaration in the 
> redeclaration chain would be better then the canonical declaration...
> 
> WDYT?
> As I understand it - for a given declaration we found the first comment in 
> the redeclaration chain and then saved it to the cache for every 
> redeclaration (the same comment).

Only if there was only one comment in the redeclaration chain.  If a 
redeclaration had a different comment, this function would return that comment.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60432



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

[PATCH] D60267: [clangd] Support relatedInformation in diagnostics.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Diagnostics.cpp:271
+  if (!Note.AbsFile) {
+log("Dropping note from unknown file: {0}", Note);
+continue;

Maybe `vlog`? This is what we use for dropped diagnostics, should probably 
stick to the same level with dropped notes (even though the dropped notes 
probably come up less often in practice).


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60267



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

[PATCH] D60267: [clangd] Support relatedInformation in diagnostics.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Protocol.cpp:280
 R.DiagnosticFixes = *CodeActions;
+  if (auto CodeActions = Diagnostics->getBoolean("relatedInformation"))
+R.DiagnosticRelatedInformation = *CodeActions;

kadircet wrote:
> `s/CodeActions/RelatedInfoSupport/`
copy-paste detected? :-)))


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60267



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

[clang-tools-extra] r358074 - [clangd] Refactor speculateCompletionFilter and also extract scope.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Apr 10 04:50:40 2019
New Revision: 358074

URL: http://llvm.org/viewvc/llvm-project?rev=358074=rev
Log:
[clangd] Refactor speculateCompletionFilter and also extract scope.

Summary:
Intent is to use the heuristically-parsed scope in cases where we get bogus
results from sema, such as in complex macro expansions.
Added a motivating testcase we currently get wrong.

Name changed because we (already) use this for things other than speculation.

Reviewers: ioeric

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=358074=358073=358074=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Apr 10 04:50:40 2019
@@ -37,6 +37,7 @@
 #include "index/Symbol.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/Basic/CharInfo.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Format/Format.h"
@@ -1110,14 +,12 @@ llvm::Optional
 speculativeFuzzyFindRequestForCompletion(FuzzyFindRequest CachedReq,
  PathRef File, llvm::StringRef Content,
  Position Pos) {
-  auto Filter = speculateCompletionFilter(Content, Pos);
-  if (!Filter) {
-elog("Failed to speculate filter text for code completion at Pos "
- "{0}:{1}: {2}",
- Pos.line, Pos.character, Filter.takeError());
-return None;
+  auto Offset = positionToOffset(Content, Pos);
+  if (!Offset) {
+elog("No speculative filter: bad offset {0} in {1}", Pos, File);
+return llvm::None;
   }
-  CachedReq.Query = *Filter;
+  CachedReq.Query = guessCompletionPrefix(Content, *Offset).Name;
   return CachedReq;
 }
 
@@ -1537,29 +1536,26 @@ clang::CodeCompleteOptions CodeCompleteO
   return Result;
 }
 
-llvm::Expected
-speculateCompletionFilter(llvm::StringRef Content, Position Pos) {
-  auto Offset = positionToOffset(Content, Pos);
-  if (!Offset)
-return llvm::make_error(
-"Failed to convert position to offset in content.",
-llvm::inconvertibleErrorCode());
-  if (*Offset == 0)
-return "";
+CompletionPrefix
+guessCompletionPrefix(llvm::StringRef Content, unsigned Offset) {
+  assert(Offset <= Content.size());
+  StringRef Rest = Content.take_front(Offset);
+  CompletionPrefix Result;
+
+  // Consume the unqualified name. We only handle ASCII characters.
+  // isIdentifierBody will let us match "0invalid", but we don't mind.
+  while (!Rest.empty() && isIdentifierBody(Rest.back()))
+Rest = Rest.drop_back();
+  Result.Name = Content.slice(Rest.size(), Offset);
+
+  // Consume qualifiers.
+  while (Rest.consume_back("::") && !Rest.endswith(":")) // reject 
+while (!Rest.empty() && isIdentifierBody(Rest.back()))
+  Rest = Rest.drop_back();
+  Result.Qualifier =
+  Content.slice(Rest.size(), Result.Name.begin() - Content.begin());
 
-  // Start from the character before the cursor.
-  int St = *Offset - 1;
-  // FIXME(ioeric): consider UTF characters?
-  auto IsValidIdentifierChar = [](char C) {
-return ((C >= 'a' && C <= 'z') || (C >= 'A' && C <= 'Z') ||
-(C >= '0' && C <= '9') || (C == '_'));
-  };
-  size_t Len = 0;
-  for (; (St >= 0) && IsValidIdentifierChar(Content[St]); --St, ++Len) {
-  }
-  if (Len > 0)
-St++; // Shift to the first valid character.
-  return Content.substr(St, Len);
+  return Result;
 }
 
 CodeCompleteResult

Modified: clang-tools-extra/trunk/clangd/CodeComplete.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.h?rev=358074=358073=358074=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.h (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.h Wed Apr 10 04:50:40 2019
@@ -254,10 +254,21 @@ SignatureHelp signatureHelp(PathRef File
 // completion.
 bool isIndexedForCodeCompletion(const NamedDecl , ASTContext );
 
-/// Retrives a speculative code completion filter text before the cursor.
-/// Exposed for testing only.
-llvm::Expected
-speculateCompletionFilter(llvm::StringRef Content, Position Pos);
+// Text immediately before the completion point that should be completed.
+// This is heuristically derived from the source code, and is used when:
+//   - semantic analysis fails
+//   - semantic analysis may be slow, and we speculatively query the index
+struct CompletionPrefix {
+  // The 

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clangd/CodeComplete.cpp:1542
+  assert(Offset <= Content.size());
+  StringRef Suffix = Content.take_front(Offset);
+  CompletionPrefix Result;

ioeric wrote:
> `Suffix` is actually a prefix of Content? This seems a bit confusing...
Haha, just checking we're all paying attention...


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60500



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

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rL358074: [clangd] Refactor speculateCompletionFilter and also 
extract scope. (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60500

Files:
  clang-tools-extra/trunk/clangd/CodeComplete.cpp
  clang-tools-extra/trunk/clangd/CodeComplete.h
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -32,7 +32,6 @@
 using ::llvm::Failed;
 using ::testing::AllOf;
 using ::testing::Contains;
-using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::HasSubstr;
@@ -1915,28 +1914,37 @@
   )cpp");
 }
 
-TEST(SpeculateCompletionFilter, Filters) {
-  Annotations F(R"cpp($bof^
-  $bol^
-  ab$ab^
-  x.ab$dot^
-  x.$dotempty^
-  x::ab$scoped^
-  x::$scopedempty^
+TEST(GuessCompletionPrefix, Filters) {
+  for (llvm::StringRef Case : {
+"[[scope::]][[ident]]^",
+"[[]][[]]^",
+"\n[[]][[]]^",
+"[[]][[ab]]^",
+"x.[[]][[ab]]^",
+"x.[[]][[]]^",
+"[[x::]][[ab]]^",
+"[[x::]][[]]^",
+"[[::x::]][[ab]]^",
+"some text [[scope::more::]][[identif]]^ier",
+"some text [[scope::]][[mor]]^e::identifier",
+"weird case foo::[[::bar::]][[baz]]^",
+  }) {
+Annotations F(Case);
+auto Offset = cantFail(positionToOffset(F.code(), F.point()));
+auto ToStringRef = [&](Range R) {
+  return F.code().slice(cantFail(positionToOffset(F.code(), R.start)),
+cantFail(positionToOffset(F.code(), R.end)));
+};
+auto WantQualifier = ToStringRef(F.ranges()[0]),
+ WantName = ToStringRef(F.ranges()[1]);
 
-  )cpp");
-  auto speculate = [&](StringRef PointName) {
-auto Filter = speculateCompletionFilter(F.code(), F.point(PointName));
-assert(Filter);
-return *Filter;
-  };
-  EXPECT_EQ(speculate("bof"), "");
-  EXPECT_EQ(speculate("bol"), "");
-  EXPECT_EQ(speculate("ab"), "ab");
-  EXPECT_EQ(speculate("dot"), "ab");
-  EXPECT_EQ(speculate("dotempty"), "");
-  EXPECT_EQ(speculate("scoped"), "ab");
-  EXPECT_EQ(speculate("scopedempty"), "");
+auto Prefix = guessCompletionPrefix(F.code(), Offset);
+// Even when components are empty, check their offsets are correct.
+EXPECT_EQ(WantQualifier, Prefix.Qualifier) << Case;
+EXPECT_EQ(WantQualifier.begin(), Prefix.Qualifier.begin()) << Case;
+EXPECT_EQ(WantName, Prefix.Name) << Case;
+EXPECT_EQ(WantName.begin(), Prefix.Name.begin()) << Case;
+  }
 }
 
 TEST(CompletionTest, EnableSpeculativeIndexRequest) {
@@ -2366,6 +2374,23 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ";
 }
 
+// Regression test: clang parser gets confused here and doesn't report the ns::
+// prefix - naive behavior is to insert it again.
+// However we can recognize this from the source code.
+// Test disabled until we can make it pass.
+TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+  clangd::CodeCompleteOptions Opts = {};
+
+  auto Results = completions(R"cpp(
+namespace ns {}
+#define M(X) < X
+M(ns::ABC^
+  )cpp",
+ {cls("ns::ABCDE")}, Opts);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -37,6 +37,7 @@
 #include "index/Symbol.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/Basic/CharInfo.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Format/Format.h"
@@ -1110,14 +,12 @@
 speculativeFuzzyFindRequestForCompletion(FuzzyFindRequest CachedReq,
  PathRef File, llvm::StringRef Content,
  Position Pos) {
-  auto Filter = speculateCompletionFilter(Content, Pos);
-  if (!Filter) {
-elog("Failed to speculate filter text for code completion at Pos "
- "{0}:{1}: {2}",
- Pos.line, Pos.character, Filter.takeError());
-return None;
+  auto Offset = positionToOffset(Content, Pos);
+  if (!Offset) {
+elog("No speculative filter: bad offset {0} in {1}", Pos, File);
+return llvm::None;
   }
-  CachedReq.Query = *Filter;
+  CachedReq.Query = 

[PATCH] D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 194486.
sammccall added a comment.

Update comment


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60503

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2374,19 +2374,19 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ";
 }
 
-// Regression test: clang parser gets confused here and doesn't report the ns::
-// prefix - naive behavior is to insert it again.
-// However we can recognize this from the source code.
-// Test disabled until we can make it pass.
-TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+// Clang parser gets confused here and doesn't report the ns:: prefix.
+// Naive behavior is to insert it again. We examine the source and recover.
+TEST(CompletionTest, NamespaceDoubleInsertion) {
   clangd::CodeCompleteOptions Opts = {};
 
   auto Results = completions(R"cpp(
+namespace foo {
 namespace ns {}
 #define M(X) < X
 M(ns::ABC^
+}
   )cpp",
- {cls("ns::ABCDE")}, Opts);
+ {cls("foo::ns::ABCDE")}, Opts);
   EXPECT_THAT(Results.Completions,
   UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
 }
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -45,6 +45,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
@@ -539,54 +540,59 @@
 // (e.g. enclosing namespace).
 std::pair, bool>
 getQueryScopes(CodeCompletionContext , const Sema ,
+   const CompletionPrefix ,
const CodeCompleteOptions ) {
-  auto GetAllAccessibleScopes = [](CodeCompletionContext ) {
-SpecifiedScope Info;
-for (auto *Context : CCContext.getVisitedContexts()) {
-  if (isa(Context))
-Info.AccessibleScopes.push_back(""); // global namespace
-  else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
-}
-return Info;
-  };
-
-  auto SS = CCContext.getCXXScopeSpecifier();
+  SpecifiedScope Scopes;
+  for (auto *Context : CCContext.getVisitedContexts()) {
+if (isa(Context))
+  Scopes.AccessibleScopes.push_back(""); // global namespace
+else if (isa(Context))
+  Scopes.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  }
 
-  // Unqualified completion (e.g. "vec^").
-  if (!SS) {
-std::vector Scopes;
+  const CXXScopeSpec *SemaSpecifier =
+  CCContext.getCXXScopeSpecifier().getValueOr(nullptr);
+  // Case 1: unqualified completion.
+  if (!SemaSpecifier) {
+// Case 2 (exception): sema saw no qualifier, but there appears to be one!
+// This can happen e.g. in incomplete macro expansions. Use heuristics.
+if (!HeuristicPrefix.Qualifier.empty()) {
+  vlog("Sema said no scope specifier, but we saw {0} in the source code",
+   HeuristicPrefix.Qualifier);
+  StringRef SpelledSpecifier = HeuristicPrefix.Qualifier;
+  if (SpelledSpecifier.consume_front("::"))
+Scopes.AccessibleScopes = {""};
+  Scopes.UnresolvedQualifier = SpelledSpecifier;
+  return {Scopes.scopesForIndexQuery(), false};
+}
+// The enclosing namespace must be first, it gets a quality boost.
+std::vector EnclosingAtFront;
 std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext);
-Scopes.push_back(EnclosingScope);
-for (auto  : GetAllAccessibleScopes(CCContext).scopesForIndexQuery()) {
+EnclosingAtFront.push_back(EnclosingScope);
+for (auto  : Scopes.scopesForIndexQuery()) {
   if (EnclosingScope != S)
-Scopes.push_back(std::move(S));
+EnclosingAtFront.push_back(std::move(S));
 }
-// Allow AllScopes completion only for there is no explicit scope qualifier.
-return {Scopes, Opts.AllScopes};
+// Allow AllScopes completion as there is no explicit scope qualifier.
+return {EnclosingAtFront, Opts.AllScopes};
   }
-
-  // Qualified completion ("std::vec^"), we have two cases depending on whether
-  // the qualifier can be resolved by Sema.
-  if ((*SS)->isValid()) { // Resolved qualifier.
-return {GetAllAccessibleScopes(CCContext).scopesForIndexQuery(), false};
-  }
-
-  // Unresolved qualifier.
-  SpecifiedScope Info = GetAllAccessibleScopes(CCContext);
-  Info.AccessibleScopes.push_back(""); // Make sure global scope is included.
-
-  llvm::StringRef SpelledSpecifier =
-  

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

[Sam McCall via Phabricator via cfe-commits]

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

Intent is to use the heuristically-parsed scope in cases where we get bogus
results from sema, such as in complex macro expansions.
Added a motivating testcase we currently get wrong.

Name changed because we (already) use this for things other than speculation.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D60500

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1915,28 +1915,35 @@
   )cpp");
 }
 
-TEST(SpeculateCompletionFilter, Filters) {
-  Annotations F(R"cpp($bof^
-  $bol^
-  ab$ab^
-  x.ab$dot^
-  x.$dotempty^
-  x::ab$scoped^
-  x::$scopedempty^
-
-  )cpp");
-  auto speculate = [&](StringRef PointName) {
-auto Filter = speculateCompletionFilter(F.code(), F.point(PointName));
-assert(Filter);
-return *Filter;
-  };
-  EXPECT_EQ(speculate("bof"), "");
-  EXPECT_EQ(speculate("bol"), "");
-  EXPECT_EQ(speculate("ab"), "ab");
-  EXPECT_EQ(speculate("dot"), "ab");
-  EXPECT_EQ(speculate("dotempty"), "");
-  EXPECT_EQ(speculate("scoped"), "ab");
-  EXPECT_EQ(speculate("scopedempty"), "");
+TEST(GuessCompletionPrefix, Filters) {
+  for (llvm::StringRef Case : {
+"[[scope::]][[ident]]^",
+"[[]][[]]^",
+"\n[[]][[]]^",
+"[[]][[ab]]^",
+"x.[[]][[ab]]^",
+"x.[[]][[]]^",
+"[[x::]][[ab]]^",
+"[[x::]][[]]^",
+"some text [[scope::more::]][[identif]]^ier",
+"some text [[scope::]][[mor]]^e::identifier",
+  }) {
+Annotations F(Case);
+auto Offset = cantFail(positionToOffset(F.code(), F.point()));
+auto ToStringRef = [&](Range R) {
+  return F.code().slice(cantFail(positionToOffset(F.code(), R.start)),
+cantFail(positionToOffset(F.code(), R.end)));
+};
+auto WantQualifier = ToStringRef(F.ranges()[0]),
+ WantName = ToStringRef(F.ranges()[1]);
+
+auto Prefix = guessCompletionPrefix(F.code(), Offset);
+// Even when components are empty, check their offsets are correct.
+EXPECT_EQ(WantQualifier, Prefix.Qualifier) << Case;
+EXPECT_EQ(WantQualifier.begin(), Prefix.Qualifier.begin()) << Case;
+EXPECT_EQ(WantName, Prefix.Name) << Case;
+EXPECT_EQ(WantName.begin(), Prefix.Name.begin()) << Case;
+  }
 }
 
 TEST(CompletionTest, EnableSpeculativeIndexRequest) {
@@ -2366,6 +2373,23 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ";
 }
 
+// Regression test: clang parser gets confused here and doesn't report the ns::
+// prefix - naive behavior is to insert it again.
+// However we can recognize this from the source code.
+// Test disabled until we can make it pass.
+TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+  clangd::CodeCompleteOptions Opts = {};
+
+  auto Results = completions(R"cpp(
+namespace ns {}
+#define M(X) < X
+M(ns::ABC^
+  )cpp",
+ {cls("ns::ABCDE")}, Opts);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.h
===
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -248,10 +248,21 @@
 // completion.
 bool isIndexedForCodeCompletion(const NamedDecl , ASTContext );
 
-/// Retrives a speculative code completion filter text before the cursor.
-/// Exposed for testing only.
-llvm::Expected
-speculateCompletionFilter(llvm::StringRef Content, Position Pos);
+// Text immediately before the completion point that should be completed.
+// This is heuristically derived from the source code, and is used when:
+//   - semantic analysis fails
+//   - semantic analysis may be slow, and we speculatively query the index
+struct CompletionPrefix {
+  // The unqualified partial name.
+  // If there is none, begin() == end() == completion position.
+  llvm::StringRef Name;
+  // The spelled scope qualifier, such as Foo::.
+  // If there is none, begin() == end() == Name.begin().
+  llvm::StringRef Qualifier;
+};
+// Heuristically parses before Offset to determine what should be completed.
+CompletionPrefix guessCompletionPrefix(llvm::StringRef Content,
+   unsigned Offset);
 
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -37,6 +37,7 @@
 #include "index/Symbol.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include 

[PATCH] D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

[Sam McCall via Phabricator via cfe-commits]

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

There are cases where Sema can't tell that "foo" in foo::Bar is a
namespace qualifier, like in incomplete macro expansions.

After this patch, if sema reports no specifier but it looks like there's one in
the source code, then we take it into account.

Reworked structure and comments in getQueryScopes to try to fight
creeping complexity - unsure if I succeeded.

I made the test harder (the original version also passes).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D60503

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2378,15 +2378,17 @@
 // prefix - naive behavior is to insert it again.
 // However we can recognize this from the source code.
 // Test disabled until we can make it pass.
-TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+TEST(CompletionTest, NamespaceDoubleInsertion) {
   clangd::CodeCompleteOptions Opts = {};
 
   auto Results = completions(R"cpp(
+namespace foo {
 namespace ns {}
 #define M(X) < X
 M(ns::ABC^
+}
   )cpp",
- {cls("ns::ABCDE")}, Opts);
+ {cls("foo::ns::ABCDE")}, Opts);
   EXPECT_THAT(Results.Completions,
   UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
 }
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -45,6 +45,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
@@ -539,54 +540,59 @@
 // (e.g. enclosing namespace).
 std::pair, bool>
 getQueryScopes(CodeCompletionContext , const Sema ,
+   const CompletionPrefix ,
const CodeCompleteOptions ) {
-  auto GetAllAccessibleScopes = [](CodeCompletionContext ) {
-SpecifiedScope Info;
-for (auto *Context : CCContext.getVisitedContexts()) {
-  if (isa(Context))
-Info.AccessibleScopes.push_back(""); // global namespace
-  else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
-}
-return Info;
-  };
-
-  auto SS = CCContext.getCXXScopeSpecifier();
+  SpecifiedScope Scopes;
+  for (auto *Context : CCContext.getVisitedContexts()) {
+if (isa(Context))
+  Scopes.AccessibleScopes.push_back(""); // global namespace
+else if (isa(Context))
+  Scopes.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  }
 
-  // Unqualified completion (e.g. "vec^").
-  if (!SS) {
-std::vector Scopes;
+  const CXXScopeSpec *SemaSpecifier =
+  CCContext.getCXXScopeSpecifier().getValueOr(nullptr);
+  // Case 1: unqualified completion.
+  if (!SemaSpecifier) {
+// Case 2 (exception): sema saw no qualifier, but there appears to be one!
+// This can happen e.g. in incomplete macro expansions. Use heuristics.
+if (!HeuristicPrefix.Qualifier.empty()) {
+  vlog("Sema said no scope specifier, but we saw {0} in the source code",
+   HeuristicPrefix.Qualifier);
+  StringRef SpelledSpecifier = HeuristicPrefix.Qualifier;
+  if (SpelledSpecifier.consume_front("::"))
+Scopes.AccessibleScopes = {""};
+  Scopes.UnresolvedQualifier = SpelledSpecifier;
+  return {Scopes.scopesForIndexQuery(), false};
+}
+// The enclosing namespace must be first, it gets a quality boost.
+std::vector EnclosingAtFront;
 std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext);
-Scopes.push_back(EnclosingScope);
-for (auto  : GetAllAccessibleScopes(CCContext).scopesForIndexQuery()) {
+EnclosingAtFront.push_back(EnclosingScope);
+for (auto  : Scopes.scopesForIndexQuery()) {
   if (EnclosingScope != S)
-Scopes.push_back(std::move(S));
+EnclosingAtFront.push_back(std::move(S));
 }
-// Allow AllScopes completion only for there is no explicit scope qualifier.
-return {Scopes, Opts.AllScopes};
+// Allow AllScopes completion as there is no explicit scope qualifier.
+return {EnclosingAtFront, Opts.AllScopes};
   }
-
-  // Qualified completion ("std::vec^"), we have two cases depending on whether
-  // the qualifier can be resolved by Sema.
-  if ((*SS)->isValid()) { // Resolved qualifier.
-return {GetAllAccessibleScopes(CCContext).scopesForIndexQuery(), false};
-  }
-
-  // Unresolved qualifier.
-  SpecifiedScope Info = GetAllAccessibleScopes(CCContext);
-  

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 194462.
sammccall added a comment.

Handle  edge-case correctly.
Add test for leading :: case.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60500

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -32,7 +32,6 @@
 using ::llvm::Failed;
 using ::testing::AllOf;
 using ::testing::Contains;
-using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::HasSubstr;
@@ -1915,28 +1914,37 @@
   )cpp");
 }
 
-TEST(SpeculateCompletionFilter, Filters) {
-  Annotations F(R"cpp($bof^
-  $bol^
-  ab$ab^
-  x.ab$dot^
-  x.$dotempty^
-  x::ab$scoped^
-  x::$scopedempty^
-
-  )cpp");
-  auto speculate = [&](StringRef PointName) {
-auto Filter = speculateCompletionFilter(F.code(), F.point(PointName));
-assert(Filter);
-return *Filter;
-  };
-  EXPECT_EQ(speculate("bof"), "");
-  EXPECT_EQ(speculate("bol"), "");
-  EXPECT_EQ(speculate("ab"), "ab");
-  EXPECT_EQ(speculate("dot"), "ab");
-  EXPECT_EQ(speculate("dotempty"), "");
-  EXPECT_EQ(speculate("scoped"), "ab");
-  EXPECT_EQ(speculate("scopedempty"), "");
+TEST(GuessCompletionPrefix, Filters) {
+  for (llvm::StringRef Case : {
+"[[scope::]][[ident]]^",
+"[[]][[]]^",
+"\n[[]][[]]^",
+"[[]][[ab]]^",
+"x.[[]][[ab]]^",
+"x.[[]][[]]^",
+"[[x::]][[ab]]^",
+"[[x::]][[]]^",
+"[[::x::]][[ab]]^",
+"some text [[scope::more::]][[identif]]^ier",
+"some text [[scope::]][[mor]]^e::identifier",
+"weird case foo::[[::bar::]][[baz]]^",
+  }) {
+Annotations F(Case);
+auto Offset = cantFail(positionToOffset(F.code(), F.point()));
+auto ToStringRef = [&](Range R) {
+  return F.code().slice(cantFail(positionToOffset(F.code(), R.start)),
+cantFail(positionToOffset(F.code(), R.end)));
+};
+auto WantQualifier = ToStringRef(F.ranges()[0]),
+ WantName = ToStringRef(F.ranges()[1]);
+
+auto Prefix = guessCompletionPrefix(F.code(), Offset);
+// Even when components are empty, check their offsets are correct.
+EXPECT_EQ(WantQualifier, Prefix.Qualifier) << Case;
+EXPECT_EQ(WantQualifier.begin(), Prefix.Qualifier.begin()) << Case;
+EXPECT_EQ(WantName, Prefix.Name) << Case;
+EXPECT_EQ(WantName.begin(), Prefix.Name.begin()) << Case;
+  }
 }
 
 TEST(CompletionTest, EnableSpeculativeIndexRequest) {
@@ -2366,6 +2374,23 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ";
 }
 
+// Regression test: clang parser gets confused here and doesn't report the ns::
+// prefix - naive behavior is to insert it again.
+// However we can recognize this from the source code.
+// Test disabled until we can make it pass.
+TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+  clangd::CodeCompleteOptions Opts = {};
+
+  auto Results = completions(R"cpp(
+namespace ns {}
+#define M(X) < X
+M(ns::ABC^
+  )cpp",
+ {cls("ns::ABCDE")}, Opts);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.h
===
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -248,10 +248,21 @@
 // completion.
 bool isIndexedForCodeCompletion(const NamedDecl , ASTContext );
 
-/// Retrives a speculative code completion filter text before the cursor.
-/// Exposed for testing only.
-llvm::Expected
-speculateCompletionFilter(llvm::StringRef Content, Position Pos);
+// Text immediately before the completion point that should be completed.
+// This is heuristically derived from the source code, and is used when:
+//   - semantic analysis fails
+//   - semantic analysis may be slow, and we speculatively query the index
+struct CompletionPrefix {
+  // The unqualified partial name.
+  // If there is none, begin() == end() == completion position.
+  llvm::StringRef Name;
+  // The spelled scope qualifier, such as Foo::.
+  // If there is none, begin() == end() == Name.begin().
+  llvm::StringRef Qualifier;
+};
+// Heuristically parses before Offset to determine what should be completed.
+CompletionPrefix guessCompletionPrefix(llvm::StringRef Content,
+   unsigned Offset);
 
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -37,6 +37,7 @@
 #include "index/Symbol.h"
 #include "clang/AST/Decl.h"
 #include 

[PATCH] D60267: [clangd] Support relatedInformation in diagnostics.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Diagnostics.cpp:185
   OS << Note.Message;
-  OS << "\n\n";
-  printDiag(OS, Main);
+  // If there's no structured link between the note and the original diagnostic
+  // then emit the main diagnostic to give context.

NIT: the comment looks open to misinterpretation, "no structured link" refers 
to the fact the clients don't support it, but could be read that we don't know 
which notes correspond to a main diagnostic.

Maybe rephrase to something link "If the client does not support structured 
links …"?



Comment at: clangd/Diagnostics.cpp:280
+  Main.relatedInformation->push_back(std::move(RelInfo));
+}
   }

NIT: maybe call `OutFn` and return here to avoid checking for 
`EmitRelatedLocations` again?
Would arguably make the code simpler, although would require another call to 
`OutFn(Main)` outside the if branch.



Comment at: clangd/SourceCode.cpp:310
+  if (!F) {
 return None;
+  }

NIT: seems unrelated. Maybe revert?



Comment at: unittests/clangd/DiagnosticsTests.cpp:62
+return false;
+  if (toJSON(arg) != toJSON(LSPDiag)) {
+*result_listener << llvm::formatv("expected:\n{0:2}\ngot\n{1:2}",

Maybe omit the `std::tie()`-based comparison altogether?
This would not change the semantics of the matcher, right?



Comment at: unittests/clangd/DiagnosticsTests.cpp:259
+#ifdef _WIN32
+  "c:\\path\\to\\foo\\bar\\main.cpp",
+#else

maybe use `testPath()` here to avoid PP directives?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60267



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

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

[Eric Liu via Phabricator via cfe-commits]

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

lgtm




Comment at: clangd/CodeComplete.cpp:1542
+  assert(Offset <= Content.size());
+  StringRef Suffix = Content.take_front(Offset);
+  CompletionPrefix Result;

`Suffix` is actually a prefix of Content? This seems a bit confusing...


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60500



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

[PATCH] D60267: [clangd] Support relatedInformation in diagnostics.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/Protocol.cpp:280
 R.DiagnosticFixes = *CodeActions;
+  if (auto CodeActions = Diagnostics->getBoolean("relatedInformation"))
+R.DiagnosticRelatedInformation = *CodeActions;

`s/CodeActions/RelatedInfoSupport/`


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60267



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

[PATCH] D59599: [clangd] Fix a crash while printing Template Arguments

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 194470.
kadircet marked 3 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59599

Files:
  clangd/AST.cpp
  unittests/clangd/SymbolCollectorTests.cpp


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -1222,6 +1222,22 @@
   EXPECT_THAT(Symbols, Contains(QName("std::foo")));
 }
 
+TEST_F(SymbolCollectorTest, TemplateSpecForwardDecl) {
+  // FIXME: getTypeAsWritten returns null for friend decls, this should be 
fixed
+  // in AST. Testing just to make sure we don't crash.
+  Annotations Header(R"(
+  template  struct [[Foo]];
+  struct Bar {
+  friend class Foo;
+  };
+  template <> struct Foo {};
+  )");
+  runSymbolCollector(Header.code(), /*Main=*/"");
+  EXPECT_THAT(Symbols,
+  Contains(AllOf(QName("Foo"), DeclRange(Header.range()),
+ ForCodeCompletion(true;
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/AST.cpp
===
--- clangd/AST.cpp
+++ clangd/AST.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/TemplateBase.h"
+#include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
@@ -82,14 +83,17 @@
   if (auto Args = getTemplateSpecializationArgLocs(ND))
 printTemplateArgumentList(OS, *Args, Policy);
   else if (auto *Cls = llvm::dyn_cast()) {
-if (auto STL = Cls->getTypeAsWritten()
-   ->getTypeLoc()
-   .getAs()) {
-  llvm::SmallVector ArgLocs;
-  ArgLocs.reserve(STL.getNumArgs());
-  for (unsigned I = 0; I < STL.getNumArgs(); ++I)
-ArgLocs.push_back(STL.getArgLoc(I));
-  printTemplateArgumentList(OS, ArgLocs, Policy);
+if (const TypeSourceInfo *TSI = Cls->getTypeAsWritten()) {
+  if (auto STL = TSI->getTypeLoc().getAs()) 
{
+llvm::SmallVector ArgLocs;
+ArgLocs.reserve(STL.getNumArgs());
+for (unsigned I = 0; I < STL.getNumArgs(); ++I)
+  ArgLocs.push_back(STL.getArgLoc(I));
+printTemplateArgumentList(OS, ArgLocs, Policy);
+  }
+} else {
+  // FIXME: Fix cases when getTypeAsWritten returns null, e.g. friend 
decls.
+  printTemplateArgumentList(OS, Cls->getTemplateArgs().asArray(), Policy);
 }
   }
   OS.flush();


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -1222,6 +1222,22 @@
   EXPECT_THAT(Symbols, Contains(QName("std::foo")));
 }
 
+TEST_F(SymbolCollectorTest, TemplateSpecForwardDecl) {
+  // FIXME: getTypeAsWritten returns null for friend decls, this should be fixed
+  // in AST. Testing just to make sure we don't crash.
+  Annotations Header(R"(
+  template  struct [[Foo]];
+  struct Bar {
+  friend class Foo;
+  };
+  template <> struct Foo {};
+  )");
+  runSymbolCollector(Header.code(), /*Main=*/"");
+  EXPECT_THAT(Symbols,
+  Contains(AllOf(QName("Foo"), DeclRange(Header.range()),
+ ForCodeCompletion(true;
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/AST.cpp
===
--- clangd/AST.cpp
+++ clangd/AST.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/TemplateBase.h"
+#include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
@@ -82,14 +83,17 @@
   if (auto Args = getTemplateSpecializationArgLocs(ND))
 printTemplateArgumentList(OS, *Args, Policy);
   else if (auto *Cls = llvm::dyn_cast()) {
-if (auto STL = Cls->getTypeAsWritten()
-   ->getTypeLoc()
-   .getAs()) {
-  llvm::SmallVector ArgLocs;
-  ArgLocs.reserve(STL.getNumArgs());
-  for (unsigned I = 0; I < STL.getNumArgs(); ++I)
-ArgLocs.push_back(STL.getArgLoc(I));
-  printTemplateArgumentList(OS, ArgLocs, Policy);
+if (const TypeSourceInfo *TSI = Cls->getTypeAsWritten()) {
+  if (auto STL = TSI->getTypeLoc().getAs()) {
+llvm::SmallVector ArgLocs;
+ArgLocs.reserve(STL.getNumArgs());
+for (unsigned I = 0; I < STL.getNumArgs(); ++I)
+  ArgLocs.push_back(STL.getArgLoc(I));
+printTemplateArgumentList(OS, ArgLocs, Policy);
+  }
+} else {
+  // FIXME: Fix cases when getTypeAsWritten returns null, e.g. friend decls.
+  

[PATCH] D60485: [AArch64] Add support for MTE intrinsics

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

Do we need a separate file for that one "__arm_mte_ptrdiff" test? Wouldn't it 
be easier to wrap the whole function in "__cplusplus" and put it in the same 
file.


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

https://reviews.llvm.org/D60485



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

[PATCH] D60409: [clangd] Add -header-insertion=never flag to disable include insertion in code completion

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall marked 2 inline comments as done.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clangd/CodeComplete.cpp:1176
   // This is available after Sema has run.
-  llvm::Optional Inserter;  // Available during runWithSema.
+  llvm::Optional Inserter;  // Optional during runWithSema.
   llvm::Optional FileProximity; // Initialized once Sema runs.

ioeric wrote:
> Why optional? In the current implementation, it's always initialized.
Oops, this was left-over from a previous iteration.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60409



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

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

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

balazske created this revision.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.

ASTImporter contained wrong or missing imports of SourceLocation
and SourceRange for some objects. At least a part of such errors
is fixed now.
Tests for SourceLocation import do not exist yet. A separate
patch is needed to add a general SourceLocation import test
that runs on every import as part of the already existing tests.


Repository:
  rC Clang

https://reviews.llvm.org/D60499

Files:
  lib/AST/ASTImporter.cpp

Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2146,6 +2146,9 @@
   ExpectedSLoc BeginLocOrErr = import(D->getBeginLoc());
   if (!BeginLocOrErr)
 return BeginLocOrErr.takeError();
+  ExpectedSLoc RBraceLocOrErr = import(D->getRBraceLoc());
+  if (!RBraceLocOrErr)
+return RBraceLocOrErr.takeError();
 
   // Create the "to" namespace, if needed.
   NamespaceDecl *ToNamespace = MergeWithNamespace;
@@ -2155,6 +2158,7 @@
 *BeginLocOrErr, Loc, Name.getAsIdentifierInfo(),
 /*PrevDecl=*/nullptr))
   return ToNamespace;
+ToNamespace->setRBraceLoc(*RBraceLocOrErr);
 ToNamespace->setLexicalDeclContext(LexicalDC);
 LexicalDC->addDeclInternal(ToNamespace);
 
@@ -2697,6 +2701,10 @@
 LexicalDC->addDeclInternal(D2);
   }
 
+  if (auto BraceRangeOrErr = import(D->getBraceRange()))
+D2->setBraceRange(*BraceRangeOrErr);
+  else
+return BraceRangeOrErr.takeError();
   if (auto QualifierLocOrErr = import(D->getQualifierLoc()))
 D2->setQualifierInfo(*QualifierLocOrErr);
   else
@@ -5118,6 +5126,11 @@
 LexicalDC->addDeclInternal(D2);
   }
 
+  if (auto BraceRangeOrErr = import(D->getBraceRange()))
+D2->setBraceRange(*BraceRangeOrErr);
+  else
+return BraceRangeOrErr.takeError();
+
   // Import the qualifier, if any.
   if (auto LocOrErr = import(D->getQualifierLoc()))
 D2->setQualifierInfo(*LocOrErr);
@@ -6111,7 +6124,8 @@
   TemplateArgumentListInfo *ToResInfo = nullptr;
   if (E->hasExplicitTemplateArgs()) {
 if (Error Err =
-ImportTemplateArgumentListInfo(E->template_arguments(), ToTAInfo))
+ImportTemplateArgumentListInfo(E->getLAngleLoc(), E->getRAngleLoc(),
+   E->template_arguments(), ToTAInfo))
   return std::move(Err);
 ToResInfo = 
   }
@@ -7130,20 +7144,18 @@
 
 ExpectedStmt
 ASTNodeImporter::VisitDependentScopeDeclRefExpr(DependentScopeDeclRefExpr *E) {
-  auto Imp = importSeq(
-  E->getQualifierLoc(), E->getTemplateKeywordLoc(), E->getDeclName(),
-  E->getExprLoc(), E->getLAngleLoc(), E->getRAngleLoc());
+  auto Imp = importSeq(E->getQualifierLoc(), E->getTemplateKeywordLoc(),
+   E->getDeclName(), E->getNameInfo().getLoc(),
+   E->getLAngleLoc(), E->getRAngleLoc());
   if (!Imp)
 return Imp.takeError();
 
   NestedNameSpecifierLoc ToQualifierLoc;
-  SourceLocation ToTemplateKeywordLoc, ToExprLoc, ToLAngleLoc, ToRAngleLoc;
+  SourceLocation ToTemplateKeywordLoc, ToNameLoc, ToLAngleLoc, ToRAngleLoc;
   DeclarationName ToDeclName;
-  std::tie(
-  ToQualifierLoc, ToTemplateKeywordLoc, ToDeclName, ToExprLoc,
-  ToLAngleLoc, ToRAngleLoc) = *Imp;
-
-  DeclarationNameInfo ToNameInfo(ToDeclName, ToExprLoc);
+  std::tie(ToQualifierLoc, ToTemplateKeywordLoc, ToDeclName, ToNameLoc,
+   ToLAngleLoc, ToRAngleLoc) = *Imp;
+  DeclarationNameInfo ToNameInfo(ToDeclName, ToNameLoc);
   if (Error Err = ImportDeclarationNameLoc(E->getNameInfo(), ToNameInfo))
 return std::move(Err);
 
@@ -7208,7 +7220,7 @@
 else
   return ToDOrErr.takeError();
 
-  if (E->hasExplicitTemplateArgs() && E->getTemplateKeywordLoc().isValid()) {
+  if (E->hasExplicitTemplateArgs()) {
 TemplateArgumentListInfo ToTAInfo;
 if (Error Err = ImportTemplateArgumentListInfo(
 E->getLAngleLoc(), E->getRAngleLoc(), E->template_arguments(),
@@ -7262,8 +7274,9 @@
   TemplateArgumentListInfo ToTAInfo;
   TemplateArgumentListInfo *ResInfo = nullptr;
   if (E->hasExplicitTemplateArgs()) {
-if (Error Err =
-ImportTemplateArgumentListInfo(E->template_arguments(), ToTAInfo))
+TemplateArgumentListInfo FromTAInfo;
+E->copyTemplateArgumentsInto(FromTAInfo);
+if (Error Err = ImportTemplateArgumentListInfo(FromTAInfo, ToTAInfo))
   return std::move(Err);
 ResInfo = 
   }
@@ -8023,8 +8036,14 @@
 return std::move(Err);
   TypeSourceInfo *TSI = getToContext().getTrivialTypeSourceInfo(
 QualType(Spec->getAsType(), 0), ToTLoc);
-  Builder.Extend(getToContext(), ToLocalBeginLoc, TSI->getTypeLoc(),
- ToLocalEndLoc);
+  if (Kind == NestedNameSpecifier::TypeSpecWithTemplate)
+// ToLocalBeginLoc is 

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 194463.
sammccall added a comment.

Fix terrible variable name.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60500

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -32,7 +32,6 @@
 using ::llvm::Failed;
 using ::testing::AllOf;
 using ::testing::Contains;
-using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::HasSubstr;
@@ -1915,28 +1914,37 @@
   )cpp");
 }
 
-TEST(SpeculateCompletionFilter, Filters) {
-  Annotations F(R"cpp($bof^
-  $bol^
-  ab$ab^
-  x.ab$dot^
-  x.$dotempty^
-  x::ab$scoped^
-  x::$scopedempty^
-
-  )cpp");
-  auto speculate = [&](StringRef PointName) {
-auto Filter = speculateCompletionFilter(F.code(), F.point(PointName));
-assert(Filter);
-return *Filter;
-  };
-  EXPECT_EQ(speculate("bof"), "");
-  EXPECT_EQ(speculate("bol"), "");
-  EXPECT_EQ(speculate("ab"), "ab");
-  EXPECT_EQ(speculate("dot"), "ab");
-  EXPECT_EQ(speculate("dotempty"), "");
-  EXPECT_EQ(speculate("scoped"), "ab");
-  EXPECT_EQ(speculate("scopedempty"), "");
+TEST(GuessCompletionPrefix, Filters) {
+  for (llvm::StringRef Case : {
+"[[scope::]][[ident]]^",
+"[[]][[]]^",
+"\n[[]][[]]^",
+"[[]][[ab]]^",
+"x.[[]][[ab]]^",
+"x.[[]][[]]^",
+"[[x::]][[ab]]^",
+"[[x::]][[]]^",
+"[[::x::]][[ab]]^",
+"some text [[scope::more::]][[identif]]^ier",
+"some text [[scope::]][[mor]]^e::identifier",
+"weird case foo::[[::bar::]][[baz]]^",
+  }) {
+Annotations F(Case);
+auto Offset = cantFail(positionToOffset(F.code(), F.point()));
+auto ToStringRef = [&](Range R) {
+  return F.code().slice(cantFail(positionToOffset(F.code(), R.start)),
+cantFail(positionToOffset(F.code(), R.end)));
+};
+auto WantQualifier = ToStringRef(F.ranges()[0]),
+ WantName = ToStringRef(F.ranges()[1]);
+
+auto Prefix = guessCompletionPrefix(F.code(), Offset);
+// Even when components are empty, check their offsets are correct.
+EXPECT_EQ(WantQualifier, Prefix.Qualifier) << Case;
+EXPECT_EQ(WantQualifier.begin(), Prefix.Qualifier.begin()) << Case;
+EXPECT_EQ(WantName, Prefix.Name) << Case;
+EXPECT_EQ(WantName.begin(), Prefix.Name.begin()) << Case;
+  }
 }
 
 TEST(CompletionTest, EnableSpeculativeIndexRequest) {
@@ -2366,6 +2374,23 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ";
 }
 
+// Regression test: clang parser gets confused here and doesn't report the ns::
+// prefix - naive behavior is to insert it again.
+// However we can recognize this from the source code.
+// Test disabled until we can make it pass.
+TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+  clangd::CodeCompleteOptions Opts = {};
+
+  auto Results = completions(R"cpp(
+namespace ns {}
+#define M(X) < X
+M(ns::ABC^
+  )cpp",
+ {cls("ns::ABCDE")}, Opts);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.h
===
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -248,10 +248,21 @@
 // completion.
 bool isIndexedForCodeCompletion(const NamedDecl , ASTContext );
 
-/// Retrives a speculative code completion filter text before the cursor.
-/// Exposed for testing only.
-llvm::Expected
-speculateCompletionFilter(llvm::StringRef Content, Position Pos);
+// Text immediately before the completion point that should be completed.
+// This is heuristically derived from the source code, and is used when:
+//   - semantic analysis fails
+//   - semantic analysis may be slow, and we speculatively query the index
+struct CompletionPrefix {
+  // The unqualified partial name.
+  // If there is none, begin() == end() == completion position.
+  llvm::StringRef Name;
+  // The spelled scope qualifier, such as Foo::.
+  // If there is none, begin() == end() == Name.begin().
+  llvm::StringRef Qualifier;
+};
+// Heuristically parses before Offset to determine what should be completed.
+CompletionPrefix guessCompletionPrefix(llvm::StringRef Content,
+   unsigned Offset);
 
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -37,6 +37,7 @@
 #include "index/Symbol.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include 

r358136 - Fix a test, NFC

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Wed Apr 10 14:18:21 2019
New Revision: 358136

URL: http://llvm.org/viewvc/llvm-project?rev=358136=rev
Log:
Fix a test, NFC

This test was duplicated, and the last declaration had some syntax errors since
the invalid attribute caused the @implementation to be skipped by the parser.

Removed:
cfe/trunk/test/CodeGenObjC/objc-asm-attribute-neg-test.m
Modified:
cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m

Removed: cfe/trunk/test/CodeGenObjC/objc-asm-attribute-neg-test.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/objc-asm-attribute-neg-test.m?rev=358135=auto
==
--- cfe/trunk/test/CodeGenObjC/objc-asm-attribute-neg-test.m (original)
+++ cfe/trunk/test/CodeGenObjC/objc-asm-attribute-neg-test.m (removed)
@@ -1,34 +0,0 @@
-// RUN: %clang_cc1  -fsyntax-only -verify -Wno-objc-root-class %s
-// rdar://16462586
-
-__attribute__((objc_runtime_name("MySecretNamespace.Protocol")))
-@protocol Protocol
-@end
-
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-@interface Message  { 
-__attribute__((objc_runtime_name("MySecretNamespace.Message"))) // 
expected-error {{'objc_runtime_name' attribute only applies to Objective-C 
interfaces and Objective-C protocols}}
-  id MyIVAR;
-}
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-@property int MyProperty; // expected-error {{prefix attribute must be 
followed by an interface or protocol
-
-- (int) getMyProperty 
__attribute__((objc_runtime_name("MySecretNamespace.Message"))); // 
expected-error {{'objc_runtime_name' attribute only applies to}}
-
-- (void) setMyProperty : (int) arg 
__attribute__((objc_runtime_name("MySecretNamespace.Message"))); // 
expected-error {{'objc_runtime_name' attribute only applies to}}
-
-@end
-
-__attribute__((objc_runtime_name("MySecretNamespace.ForwardClass")))
-@class ForwardClass; // expected-error {{prefix attribute must be followed by 
an interface or protocol}}
-
-__attribute__((objc_runtime_name("MySecretNamespace.ForwardProtocol")))
-@protocol ForwardProtocol;
-
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-@implementation Message // expected-error {{prefix attribute must be followed 
by an interface or protocol}}
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-- (id) MyMethod {
-  return MyIVAR;
-}
-@end

Modified: cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m?rev=358136=358135=358136=diff
==
--- cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m (original)
+++ cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m Wed Apr 10 14:18:21 
2019
@@ -33,10 +33,18 @@ __attribute__((objc_runtime_name("MySecr
 __attribute__((objc_runtime_name("MySecretNamespace.ForwardProtocol")))
 @protocol ForwardProtocol;
 
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-@implementation Message // expected-error {{prefix attribute must be followed 
by an interface or protocol}}
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-- (id) MyMethod {
+@implementation Message
+// expected-error@+1 {{'objc_runtime_name' attribute only applies to 
Objective-C interfaces and Objective-C protocols}}
+- (id) MyMethod 
__attribute__((objc_runtime_name("MySecretNamespace.Message"))) {
   return MyIVAR;
 }
+
+-(int)getMyProperty { return 0; }
+-(void)setMyProperty:(int)arg {}
 @end
+
+@interface NoImpl @end
+
+__attribute__((objc_runtime_name("MySecretNamespace.Message")))
+// expected-error@+1 {{prefix attribute must be followed by an interface or 
protocol}}
+@implementation NoImpl @end


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

[PATCH] D60539: Add -std=c++14 language standard option to tests that require C++14 default

[Amy Kwan via Phabricator via cfe-commits]

amyk created this revision.
amyk added reviewers: ilya-biryukov, sammccall, ioeric, hokein, akyrtzi, yvvan.
amyk added projects: clang, LLVM.
Herald added subscribers: kadircet, arphaman, dexonsmith, jkorous.

On one of the platforms that we build on, we build with the CMake macro, 
`CLANG_DEFAULT_STD_CXX` to set the default language level when building Clang 
and LLVM.

In our case, we set the default to be `gnucxx11`. However, doing so will cause 
the test cases in this patch to fail as they rely on the C++14 default.

This patch explicitly adds the `-std=c++14` to the affected test cases so they 
will work when the default language level is set.

I have added individuals who have worked with these test cases in the past as 
reviewers. I would greatly appreciate it if any of you can inform me on whether 
or not this change is acceptable.


https://reviews.llvm.org/D60539

Files:
  clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
  clang/test/Index/print-type-size.cpp


Index: clang/test/Index/print-type-size.cpp
===
--- clang/test/Index/print-type-size.cpp
+++ clang/test/Index/print-type-size.cpp
@@ -1,6 +1,6 @@
 // from SemaCXX/class-layout.cpp
-// RUN: c-index-test -test-print-type-size %s -target x86_64-pc-linux-gnu | 
FileCheck -check-prefix=CHECK64 %s
-// RUN: c-index-test -test-print-type-size %s -target i386-apple-darwin9 | 
FileCheck -check-prefix=CHECK32 %s
+// RUN: c-index-test -test-print-type-size %s -target x86_64-pc-linux-gnu 
-std=c++14 | FileCheck -check-prefix=CHECK64 %s
+// RUN: c-index-test -test-print-type-size %s -target i386-apple-darwin9 
-std=c++14 | FileCheck -check-prefix=CHECK32 %s
 
 namespace basic {
 
Index: clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
@@ -109,6 +109,7 @@
 File.Filename = FileName;
 File.HeaderCode = HeaderCode;
 File.Code = Code;
+File.ExtraArgs.push_back("-std=c++14");
 AST = File.build();
   }
 


Index: clang/test/Index/print-type-size.cpp
===
--- clang/test/Index/print-type-size.cpp
+++ clang/test/Index/print-type-size.cpp
@@ -1,6 +1,6 @@
 // from SemaCXX/class-layout.cpp
-// RUN: c-index-test -test-print-type-size %s -target x86_64-pc-linux-gnu | FileCheck -check-prefix=CHECK64 %s
-// RUN: c-index-test -test-print-type-size %s -target i386-apple-darwin9 | FileCheck -check-prefix=CHECK32 %s
+// RUN: c-index-test -test-print-type-size %s -target x86_64-pc-linux-gnu -std=c++14 | FileCheck -check-prefix=CHECK64 %s
+// RUN: c-index-test -test-print-type-size %s -target i386-apple-darwin9 -std=c++14 | FileCheck -check-prefix=CHECK32 %s
 
 namespace basic {
 
Index: clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
@@ -109,6 +109,7 @@
 File.Filename = FileName;
 File.HeaderCode = HeaderCode;
 File.Code = Code;
+File.ExtraArgs.push_back("-std=c++14");
 AST = File.build();
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60539: Add -std=c++14 language standard option to tests that require C++14 default

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

LGTM

Generally it's a good thing for our test suite to be robust against changes to 
Clang's default language mode.


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

https://reviews.llvm.org/D60539



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

[PATCH] D59673: [Driver] Allow setting the DWO name DWARF attribute separately

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D59673#1460605 , @aaronpuchert 
wrote:

> Ok, here is an idea. We introduce `-split-dwarf-output` in Clang instead of 
> `-fsplit-dwarf-dwo-name-attr`. If given, it overrides the output file name 
> for the Split DWARF file, which we otherwise take from `-split-dwarf-file`. 
> The option is obviously incompatible with `-enable-split-dwarf=single`, so we 
> will disallow that. This should be backwards-compatible, and bring the 
> behavior of `llc` and `clang -cc1` closer together. What do you think?


Sure, I think the naming's a bit weird (but hard to come up with good names for 
any of this) - but consistency seems like a good first pass at least, given 
we're halfway there anyway.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59673



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

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

[David Blaikie via Phabricator via cfe-commits]

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

Approved pending the LLVM side changes/discussion.


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

https://reviews.llvm.org/D59347



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

[PATCH] D60112: [analyzer] Treat write into a top-level parameter variable with destructor as escape.

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

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

Okay, I played around with this patch, I see now where this is going! LGTM!

> Do you think i should document it somehow?

Aye, the description you gave was enlightening, thanks! If you can squeeze it 
somewhere in the code where it isn't out of place, it's all the better! :)




Comment at: clang/test/Analysis/malloc.cpp:151
+  char *getName() {
+if (!name) {
+  name = static_cast(malloc(10));

NoQ wrote:
> Szelethus wrote:
> > Is this relevant? `name` will never be null.
> Not really, just makes the code look a bit more sensible and idiomatic and 
> less warning-worthy-anyway, to make it as clear as possible that the positive 
> here is indeed false. We don't really have a constructor in this class, but 
> we can imagine that it zero-initializes name. Without this check calling 
> `getName()` multiple times would immediately result in a leak.
Convinced ;)


Repository:
  rC Clang

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

https://reviews.llvm.org/D60112



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

[PATCH] D60281: [analyzer] Add docs for cplusplus.InnerPointer

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

Szelethus added inline comments.



Comment at: docs/analyzer/checkers.rst:225-226
+``std::string``s, by recognizing member functions that may re/deallocate the 
buffer
+before use. In the future, it would be great to add support for other STL and
+non-STL containers, and most notably, ``std::string_view``s.
+

dkrupp wrote:
> Szelethus wrote:
> > Hmm. While this page is a documentation, I would still expect regular users 
> > to browse through it -- are we sure that we want to add future plans for a 
> > non-alpha checker? I'm not against it, just a question.
> I think it is a good idea. A non-alpha checker can also be further developed, 
> by anyone else. It is good that we don't forget about further features. This 
> note also highlights the limitations of the checker.
How about this: "Future plans include to add support for blahblah". The current 
statement should rather be a TODO in the code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60281



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

[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

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

Szelethus added a comment.

Sorry, I rushed my earlier comment -- I definitely think that we should get rid 
of the `UINT_MAX` thing before landing this.


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

https://reviews.llvm.org/D59555



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

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

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

Szelethus added a comment.

Hmmm, interesting. Could there be an issue with `NoteTag` not being trivially 
destructible?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58367



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

[PATCH] D60541: [clang-format] Use SpacesBeforeTrailingComments for "option" directive

[Donald Chai via Phabricator via cfe-commits]

dchai created this revision.
dchai added a reviewer: krasimir.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

AnnotatingParser::next() is needed to implicitly set TT_BlockComment
versus TT_LineComment.  On most other paths through
AnnotatingParser::parseLine(), all tokens are consumed to achieve that.
This change updates one place where this wasn't done.


Repository:
  rC Clang

https://reviews.llvm.org/D60541

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


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -193,6 +193,10 @@
  "\"some.really.long.package.that.exceeds.the.column.limit\";"));
 }
 
+TEST_F(FormatTestProto, TrailingCommentAfterFileOption) {
+  verifyFormat("option java_package = \"foo.pkg\";  // comment\n");
+}
+
 TEST_F(FormatTestProto, FormatsOptions) {
   verifyFormat("option (MyProto.options) = {\n"
"  field_a: OK\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1124,8 +1124,11 @@
 if (Style.Language == FormatStyle::LK_Proto && Line.Level == 0 &&
 CurrentToken->is(Keywords.kw_option)) {
   next();
-  if (CurrentToken && CurrentToken->is(tok::identifier))
+  if (CurrentToken && CurrentToken->is(tok::identifier)) {
+while (CurrentToken)
+  next();
 return LT_ImportStatement;
+  }
 }
 
 bool KeywordVirtualFound = false;


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -193,6 +193,10 @@
  "\"some.really.long.package.that.exceeds.the.column.limit\";"));
 }
 
+TEST_F(FormatTestProto, TrailingCommentAfterFileOption) {
+  verifyFormat("option java_package = \"foo.pkg\";  // comment\n");
+}
+
 TEST_F(FormatTestProto, FormatsOptions) {
   verifyFormat("option (MyProto.options) = {\n"
"  field_a: OK\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1124,8 +1124,11 @@
 if (Style.Language == FormatStyle::LK_Proto && Line.Level == 0 &&
 CurrentToken->is(Keywords.kw_option)) {
   next();
-  if (CurrentToken && CurrentToken->is(tok::identifier))
+  if (CurrentToken && CurrentToken->is(tok::identifier)) {
+while (CurrentToken)
+  next();
 return LT_ImportStatement;
+  }
 }
 
 bool KeywordVirtualFound = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: aaron.ballman, rjmccall.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.

We want to make `objc_nonlazy_class` apply to implementations, but ran into 
this. There doesn't seem to be any reason that this isn't supported.

Thanks for taking a look!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D60542

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/FixIt/fixit-pragma-attribute.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Parser/attributes.mm
  clang/test/Parser/objc-implementation-attrs.m
  clang/test/Parser/placeholder-recovery.m
  clang/test/Sema/pragma-attribute-strict-subjects.c
  clang/test/SemaObjC/attr-objc-non-lazy.m
  clang/test/SemaObjC/objc-asm-attribute-neg-test.m

Index: clang/test/SemaObjC/objc-asm-attribute-neg-test.m
===
--- clang/test/SemaObjC/objc-asm-attribute-neg-test.m
+++ clang/test/SemaObjC/objc-asm-attribute-neg-test.m
@@ -19,7 +19,7 @@
   id MyIVAR;
 }
 __attribute__((objc_runtime_name("MySecretNamespace.Message")))
-@property int MyProperty; // expected-error {{prefix attribute must be followed by an interface or protocol
+@property int MyProperty; // expected-error {{prefix attribute must be followed by an interface, protocol, or implementation
 
 - (int) getMyProperty __attribute__((objc_runtime_name("MySecretNamespace.Message"))); // expected-error {{'objc_runtime_name' attribute only applies to}}
 
@@ -28,7 +28,7 @@
 @end
 
 __attribute__((objc_runtime_name("MySecretNamespace.ForwardClass")))
-@class ForwardClass; // expected-error {{prefix attribute must be followed by an interface or protocol}}
+@class ForwardClass; // expected-error {{prefix attribute must be followed by an interface, protocol, or implementation}}
 
 __attribute__((objc_runtime_name("MySecretNamespace.ForwardProtocol")))
 @protocol ForwardProtocol;
@@ -45,6 +45,6 @@
 
 @interface NoImpl @end
 
+// expected-error@+1 {{'objc_runtime_name' attribute only applies to Objective-C interfaces and Objective-C protocols}}
 __attribute__((objc_runtime_name("MySecretNamespace.Message")))
-// expected-error@+1 {{prefix attribute must be followed by an interface or protocol}}
 @implementation NoImpl @end
Index: clang/test/SemaObjC/attr-objc-non-lazy.m
===
--- clang/test/SemaObjC/attr-objc-non-lazy.m
+++ clang/test/SemaObjC/attr-objc-non-lazy.m
@@ -29,6 +29,7 @@
 
 @interface E
 @end
+// expected-error@+1 {{'objc_nonlazy_class' attribute only applies to Objective-C interfaces}}
 __attribute__((objc_nonlazy_class))
-@implementation E // expected-error {{prefix attribute must be followed by an interface or protocol}}
+@implementation E
 @end
Index: clang/test/Sema/pragma-attribute-strict-subjects.c
===
--- clang/test/Sema/pragma-attribute-strict-subjects.c
+++ clang/test/Sema/pragma-attribute-strict-subjects.c
@@ -56,7 +56,8 @@
 #pragma clang attribute pop
 
 #pragma clang attribute push (__attribute__((abi_tag("a"))), apply_to = any(enum_constant, function, record(unless(is_union)), variable, variable(is_parameter)))
-// expected-error@-1 {{attribute 'abi_tag' can't be applied to 'variable(is_parameter)', and 'enum_constant'}}
+// FIXME: comma in this diagnostic is wrong.
+// expected-error@-2 {{attribute 'abi_tag' can't be applied to 'enum_constant', and 'variable(is_parameter)'}}
 #pragma clang attribute pop
 
 #pragma clang attribute push (__attribute__((abi_tag("a"))), apply_to = any(function, record(unless(is_union)), enum))
Index: clang/test/Parser/placeholder-recovery.m
===
--- clang/test/Parser/placeholder-recovery.m
+++ clang/test/Parser/placeholder-recovery.m
@@ -11,4 +11,4 @@
 // bogus 'archaic' warnings with bad location info.
 <#methods#> // expected-error {{editor placeholder in source file}}
 
-@end // expected-error {{prefix attribute must be followed by an interface or protocol}} expected-error {{missing '@end'}}
+@end // expected-error {{prefix attribute must be followed by an interface, protocol, or implementation}} expected-error {{missing '@end'}}
Index: clang/test/Parser/objc-implementation-attrs.m
===
--- /dev/null
+++ clang/test/Parser/objc-implementation-attrs.m
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsyntax-only -Wno-objc-root-class -verify %s
+
+@interface I1 @end
+
+// expected-warning@+1 {{'always_inline' attribute only applies to functions}}

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL marked 12 inline comments as done.
DennisL added a comment.

In D60139#1460233 , @JonasToth wrote:

> Hey Dennis,
>
> my 2cents on the check. I think it is very good to have! Did you check coding 
> guidelines if they say something to this issue? (e.g. cppcoreguidelines, 
> hicpp, cert) As we have modules for them it would be great to make aliases to 
> this check if they demand this to be checked.


Thanks for the great suggestions. Updated the diff according to the feedback. 
Also checked with cppcoreguidelines, hicpp as well as cert. Only cert has a 
related, yet different rule 

 stating that calls to placement new shall be provided with properly aligned 
pointers. I'd say this should be a distinct check. Happy to work on it after 
this one.




Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:42
+
+  assert((Cast->getSubExpr()->getType()->isPointerType() ||
+ Cast->getSubExpr()->getType()->isArrayType()) &&

JonasToth wrote:
> Is this universally true? What about the nothrow-overload, would that 
> interfere?
Thanks, rewrote that check.


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

https://reviews.llvm.org/D60139



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

What's the motivation for this change, are you working towards common flags for 
both platforms? The current way to select crypto on AArch64 is 
'-march=armv8.x-a+crypto/nocrypto'. I can see that would be an issue if Power 
PC doesn't support that syntax, or doesn't have a specific crypto extension.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

In D60472#1461351 , @peter.smith wrote:

>




> Is that not a limitation of the build system? I'd expect a package to be able 
> to locally override a global default rather than vice-versa. Although crypto 
> might be the area of concern here and there may be a conveniently named 
> option for PPC, where does this stop? Crypto is not the only architectural 
> extension, for example see 
> https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html . To maintain a 
> consistent interface we'd need command line options for all the extensions. 
> May I encourage you to reply to the RFC on command line options that I 
> mentioned earlier if it doesn't work for you? I think the extensions need to 
> be considered as a whole and not just individually.

I'll also read the RFC and respond.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D60513: [HIP] Use -mlink-builtin-bitcode to link device library

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, ashi1.
Herald added a subscriber: jdoerfert.

Use -mlink-builtin-bitcode instead of llvm-link to link
device library so that device library bitcode and user
device code can be compiled in a consistent way.

This is the same approach used by CUDA and OpenMP.


https://reviews.llvm.org/D60513

Files:
  lib/Driver/ToolChains/HIP.cpp
  test/Driver/hip-device-libs.hip
  test/Driver/hip-toolchain-no-rdc.hip
  test/Driver/hip-toolchain-rdc.hip

Index: test/Driver/hip-toolchain-rdc.hip
===
--- test/Driver/hip-toolchain-rdc.hip
+++ test/Driver/hip-toolchain-rdc.hip
@@ -18,6 +18,7 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fgpu-rdc" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
 
@@ -27,11 +28,11 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fgpu-rdc" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[B_SRC:".*b.hip"]]
 
 // CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC]] [[B_BC]]
-// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: "-o" [[LINKED_BC_DEV1:".*-gfx803-linked-.*bc"]]
 
 // CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV1]] "-mtriple=amdgcn-amd-amdhsa"
@@ -49,18 +50,21 @@
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
 // CHECK-SAME: "-emit-llvm-bc"
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx900"
-// CHECK-SAME: "-fcuda-is-device" {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: "-fcuda-is-device" "-fgpu-rdc"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
+// CHECK-SAME: {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC]]
 
 // CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
 // CHECK-SAME: "-emit-llvm-bc"
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx900"
-// CHECK-SAME: "-fcuda-is-device" {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: "-fcuda-is-device" "-fgpu-rdc"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
+// CHECK-SAME: {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[B_SRC]]
 
 // CHECK: [[LLVM_LINK]] [[A_BC]] [[B_BC]]
-// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: "-o" [[LINKED_BC_DEV2:".*-gfx900-linked-.*bc"]]
 
 // CHECK: [[OPT]] [[LINKED_BC_DEV2]] "-mtriple=amdgcn-amd-amdhsa"
Index: test/Driver/hip-toolchain-no-rdc.hip
===
--- test/Driver/hip-toolchain-no-rdc.hip
+++ test/Driver/hip-toolchain-no-rdc.hip
@@ -22,11 +22,11 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: {{.*}} "-o" [[A_BC_803:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
 
 // CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_803]]
-// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: "-o" [[LINKED_BC_DEV_A_803:".*-gfx803-linked-.*bc"]]
 
 // CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_803]] "-mtriple=amdgcn-amd-amdhsa"
@@ -50,11 +50,11 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx900"
 // CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: {{.*}} "-o" [[A_BC_900:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC]]
 
 // CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_900]]
-// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: "-o" [[LINKED_BC_DEV_A_900:".*-gfx900-linked-.*bc"]]
 
 // CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_900]] "-mtriple=amdgcn-amd-amdhsa"
@@ -94,11 +94,11 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: {{.*}} "-o" [[B_BC_803:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[B_SRC:".*b.hip"]]
 
 // CHECK: [[LLVM_LINK:"*.llvm-link"]] [[B_BC_803]]
-// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: "-o" [[LINKED_BC_DEV_B_803:".*-gfx803-linked-.*bc"]]
 
 // CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_B_803]] "-mtriple=amdgcn-amd-amdhsa"
@@ -122,11 +122,11 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx900"
 // CHECK-SAME: "-fcuda-is-device" 

[PATCH] D60516: [LTO] Add plumbing to save stats during LTO on MacOS.

[Florian Hahn via Phabricator via cfe-commits]

fhahn created this revision.
fhahn added reviewers: anemet, tejohnson, thegameg.
Herald added subscribers: cfe-commits, dang, dexonsmith, steven_wu, hiraditya, 
inglorion, mehdi_amini.
Herald added projects: clang, LLVM.

Gold and ld on Linux already support saving stats, but the
infrastructure is missing on MacOS. Unfortunately it seems like the
configuration from lib/LTO/LTO.cpp is not used.

This patch adds a new LTOStatsFile option and adds plumbing in Clang to
use it on MacOS, similar to the way remarks are handled.

Currnetly the handling of LTO flags seems quite spread out, with a bunch
of duplication. But I am not sure if there is an easy way to improve
that?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60516

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  llvm/include/llvm/LTO/LTO.h
  llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp

Index: llvm/lib/LTO/LTOCodeGenerator.cpp
===
--- llvm/lib/LTO/LTOCodeGenerator.cpp
+++ llvm/lib/LTO/LTOCodeGenerator.cpp
@@ -95,6 +95,11 @@
 "lto-pass-remarks-with-hotness",
 cl::desc("With PGO, include profile count in optimization remarks"),
 cl::Hidden);
+
+cl::opt LTOStatsFile(
+"lto-stats-file",
+cl::desc("With PGO, include profile count in optimization remarks"),
+cl::Hidden);
 }
 
 LTOCodeGenerator::LTOCodeGenerator(LLVMContext )
@@ -518,6 +523,14 @@
   }
   DiagnosticOutputFile = std::move(*DiagFileOrErr);
 
+  // Setup output file to emit statistics.
+  auto StatsFileOrErr = lto::setupStatsFile(LTOStatsFile);
+  if (!StatsFileOrErr) {
+errs() << "Error: " << toString(StatsFileOrErr.takeError()) << "\n";
+report_fatal_error("Can't get an output file for the statistics");
+  }
+  StatsFile = std::move(StatsFileOrErr.get());
+
   // We always run the verifier once on the merged module, the `DisableVerify`
   // parameter only applies to subsequent verify.
   verifyMergedModuleOnce();
@@ -585,8 +598,12 @@
   ShouldRestoreGlobalsLinkage);
 
   // If statistics were requested, print them out after codegen.
-  if (llvm::AreStatisticsEnabled())
+  if (llvm::AreStatisticsEnabled() && !StatsFile)
 llvm::PrintStatistics();
+
+  if (StatsFile)
+PrintStatisticsJSON(StatsFile->os());
+
   reportAndResetTimings();
 
   finishOptimizationRemarks();
Index: llvm/lib/LTO/LTO.cpp
===
--- llvm/lib/LTO/LTO.cpp
+++ llvm/lib/LTO/LTO.cpp
@@ -880,16 +880,10 @@
   isPrevailing, Conf.OptLevel > 0);
 
   // Setup output file to emit statistics.
-  std::unique_ptr StatsFile = nullptr;
-  if (!Conf.StatsFile.empty()) {
-EnableStatistics(false);
-std::error_code EC;
-StatsFile =
-llvm::make_unique(Conf.StatsFile, EC, sys::fs::F_None);
-if (EC)
-  return errorCodeToError(EC);
-StatsFile->keep();
-  }
+  auto StatsFileOrErr = setupStatsFile(Conf.StatsFile);
+  if (!StatsFileOrErr)
+return StatsFileOrErr.takeError();
+  std::unique_ptr StatsFile = std::move(StatsFileOrErr.get());
 
   // Finalize linking of regular LTO modules containing summaries now that
   // we have computed liveness information.
@@ -1338,3 +1332,20 @@
   DiagnosticFile->keep();
   return std::move(DiagnosticFile);
 }
+
+Expected>
+lto::setupStatsFile(StringRef StatsFilename) {
+  // Setup output file to emit statistics.
+  if (StatsFilename.empty())
+return nullptr;
+
+  llvm::EnableStatistics(false);
+  std::error_code EC;
+  auto StatsFile =
+  llvm::make_unique(StatsFilename, EC, sys::fs::F_None);
+  if (EC)
+return errorCodeToError(EC);
+
+  StatsFile->keep();
+  return std::move(StatsFile);
+}
Index: llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
===
--- llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
+++ llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
@@ -241,6 +241,7 @@
   TargetMachine::CodeGenFileType FileType = TargetMachine::CGFT_ObjectFile;
   std::unique_ptr DiagnosticOutputFile;
   bool Freestanding = false;
+  std::unique_ptr StatsFile = nullptr;
 };
 }
 #endif
Index: llvm/include/llvm/LTO/LTO.h
===
--- llvm/include/llvm/LTO/LTO.h
+++ llvm/include/llvm/LTO/LTO.h
@@ -87,6 +87,10 @@
  StringRef LTORemarksPasses,
  bool LTOPassRemarksWithHotness, int Count = -1);
 
+/// Setups the output file for saving statistics.
+Expected>
+setupStatsFile(StringRef StatsFilename);
+
 class LTO;
 struct SymbolResolution;
 class ThinBackendProc;
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -514,6 +514,14 @@
 }
   }
 
+  // Setup statistics 

[PATCH] D59692: [ASTImporter] Fix name conflict handling

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Ping @a_sidorin @shafik


Repository:
  rC Clang

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

https://reviews.llvm.org/D59692



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

[PATCH] D59692: [ASTImporter] Fix name conflict handling

[Gabor Marton via Phabricator via cfe-commits]

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

- Put back the call to GetOriginalDecl


Repository:
  rC Clang

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

https://reviews.llvm.org/D59692

Files:
  include/clang/AST/ASTImporter.h
  lib/AST/ASTImporter.cpp
  test/Analysis/Inputs/ctu-other.c
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1982,7 +1982,7 @@
   auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
   // We expect one (ODR) warning during the import.
   EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
-  EXPECT_EQ(2u,
+  EXPECT_EQ(1u,
 DeclCounter().match(ToTU, recordDecl(hasName("X";
 }
 
@@ -2692,6 +2692,64 @@
 functionDecl(hasName("f"), hasDescendant(declRefExpr()));
 }
 
+struct ImportFunctionTemplates : ASTImporterOptionSpecificTestBase {};
+
+TEST_P(ImportFunctionTemplates,
+   ImportFunctionWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void foo(T) {}
+  void foo();
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl("void foo();", Lang_CXX);
+  auto *FromD = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("foo")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportConstructorWhenThereIsAFunTemplateWithSameName) {
+  auto Code =
+  R"(
+  struct Foo {
+template 
+Foo(T) {}
+Foo();
+  };
+  )";
+  getToTuDecl(Code, Lang_CXX);
+  Decl *FromTU = getTuDecl(Code, Lang_CXX);
+  auto *FromD =
+  LastDeclMatcher().match(FromTU, cxxConstructorDecl());
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportOperatorWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void operator<(T,T) {}
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  auto *FromD = LastDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("<")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
@@ -5552,6 +5610,9 @@
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctions,
 DefaultTestValuesForRunOptions, );
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionTemplates,
+DefaultTestValuesForRunOptions, );
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFriendFunctionTemplates,
 DefaultTestValuesForRunOptions, );
 
Index: test/Analysis/Inputs/ctu-other.c
===
--- test/Analysis/Inputs/ctu-other.c
+++ test/Analysis/Inputs/ctu-other.c
@@ -12,11 +12,11 @@
 }
 
 // Test enums.
-enum B { x = 42,
- l,
- s };
+enum B { x2 = 42,
+ y2,
+ z2 };
 int enumCheck(void) {
-  return x;
+  return x2;
 }
 
 // Test reporting an error in macro definition
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -79,6 +79,7 @@
   using ExpectedExpr = llvm::Expected;
   using ExpectedDecl = llvm::Expected;
   using ExpectedSLoc = llvm::Expected;
+  using ExpectedName = llvm::Expected;
 
   std::string ImportError::toString() const {
 // FIXME: Improve error texts.
@@ -2135,11 +2136,11 @@
 }
 
 if (!ConflictingDecls.empty()) {
-  Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Namespace,
- ConflictingDecls.data(),
- ConflictingDecls.size());
-  if (!Name)
-return make_error(ImportError::NameConflict);
+  ExpectedName NameOrErr = Importer.HandleNameConflict(
+  Name, DC, Decl::IDNS_Namespace, ConflictingDecls.data(),
+  ConflictingDecls.size());
+  if (!NameOrErr)
+return NameOrErr.takeError();
 }
   }
 
@@ -2243,20 +2244,17 @@
   // already have a complete underlying type then return with that.
   if (!FromUT->isIncompleteType() && !FoundUT->isIncompleteType())
 return Importer.MapImported(D, FoundTypedef);
+} else {
+  ConflictingDecls.push_back(FoundDecl);
 }
-// FIXME Handle redecl chain.
-break;
   }
-
-  ConflictingDecls.push_back(FoundDecl);
 }
 
 if (!ConflictingDecls.empty()) {
-  Name = 

[clang-tools-extra] r358094 - [clangd] Use #if CLANGD_BUILD_XPC because it may be defined as 0

[Fangrui Song via cfe-commits]

Author: maskray
Date: Wed Apr 10 08:45:54 2019
New Revision: 358094

URL: http://llvm.org/viewvc/llvm-project?rev=358094=rev
Log:
[clangd] Use #if CLANGD_BUILD_XPC because it may be defined as 0

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

Modified: clang-tools-extra/trunk/clangd/Transport.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Transport.h?rev=358094=358093=358094=diff
==
--- clang-tools-extra/trunk/clangd/Transport.h (original)
+++ clang-tools-extra/trunk/clangd/Transport.h Wed Apr 10 08:45:54 2019
@@ -85,7 +85,7 @@ newJSONTransport(std::FILE *In, llvm::ra
  llvm::raw_ostream *InMirror, bool Pretty,
  JSONStreamStyle = JSONStreamStyle::Standard);
 
-#ifdef CLANGD_BUILD_XPC
+#if CLANGD_BUILD_XPC
 // Returns a Transport for macOS based on XPC.
 // Clangd with this transport is meant to be run as bundled XPC service.
 std::unique_ptr newXPCTransport();


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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 194528.
kadircet marked 10 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302

Files:
  clangd/ClangdUnit.cpp
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -9,6 +9,7 @@
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
@@ -45,6 +46,17 @@
   return arg.Range == Range && arg.Message == Message;
 }
 
+MATCHER_P3(Diag, Range, Message, IncludeStack,
+   "Diag at " + llvm::to_string(Range) + " = [" + Message + "]") {
+  if (arg.Range != Range || arg.Message != Message ||
+  arg.IncludeStack.size() != IncludeStack.size())
+return false;
+  for (size_t I = 0, E = IncludeStack.size(); I < E; ++I)
+if (IncludeStack[I] != arg.IncludeStack[I])
+  return false;
+  return true;
+}
+
 MATCHER_P3(Fix, Range, Replacement, Message,
"Fix " + llvm::to_string(Range) + " => " +
testing::PrintToString(Replacement) + " = [" + Message + "]") {
@@ -73,7 +85,6 @@
   return true;
 }
 
-
 // Helper function to make tests shorter.
 Position pos(int line, int character) {
   Position Res;
@@ -251,6 +262,8 @@
   D.InsideMainFile = true;
   D.Severity = DiagnosticsEngine::Error;
   D.File = "foo/bar/main.cpp";
+  D.IncludeStack.push_back("a/b.h:1:2");
+  D.IncludeStack.push_back("a/c.h:2:2");
 
   clangd::Note NoteInMain;
   NoteInMain.Message = "declared somewhere in the main file";
@@ -282,7 +295,8 @@
 
   // Diagnostics should turn into these:
   clangd::Diagnostic MainLSP =
-  MatchingLSP(D, R"(Something terrible happened (fix available)
+  MatchingLSP(D, R"(In file included from: a/b.h:1:2: 
+a/c.h:2:2: something terrible happened (fix available)
 
 main.cpp:6:7: remark: declared somewhere in the main file
 
@@ -603,7 +617,185 @@
   "Add include \"x.h\" for symbol a::X");
 }
 
+ParsedAST build(const std::string ,
+const llvm::StringMap ) {
+  std::vector Cmd = {"clang", MainFile.c_str()};
+  ParseInputs Inputs;
+  Inputs.CompileCommand.Filename = MainFile;
+  Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+  Inputs.CompileCommand.Directory = testRoot();
+  Inputs.Contents = Files.lookup(MainFile);
+  Inputs.FS = buildTestFS(Files);
+  Inputs.Opts = ParseOptions();
+  auto PCHs = std::make_shared();
+  auto CI = buildCompilerInvocation(Inputs);
+  assert(CI && "Failed to build compilation invocation.");
+  auto Preamble =
+  buildPreamble(MainFile, *CI,
+/*OldPreamble=*/nullptr,
+/*OldCompileCommand=*/Inputs.CompileCommand, Inputs,
+/*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
+  auto AST = buildAST(MainFile, createInvocationFromCommandLine(Cmd), Inputs,
+  Preamble);
+  if (!AST.hasValue()) {
+ADD_FAILURE() << "Failed to build code:\n" << Files.lookup(MainFile);
+llvm_unreachable("Failed to build TestTU!");
+  }
+  return std::move(*AST);
+}
+
+TEST(DiagsInHeaders, DiagInsideHeader) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {{MainFile, Main.code()},
+{testPath("a.h"), "no_type_spec;"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(),
+   "C++ requires a type specifier for all declarations",
+   std::vector{"/clangd-test/a.h:1:1"})));
+}
+
+TEST(DiagsInHeaders, DiagInTransitiveInclude) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {{MainFile, Main.code()},
+{testPath("a.h"), "#include \"b.h\""},
+{testPath("b.h"), "no_type_spec;"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(),
+   "C++ requires a type specifier for all declarations",
+   std::vector{"/clangd-test/a.h:1:10",
+"/clangd-test/b.h:1:1"})));
+}
+
+TEST(DiagsInHeaders, DiagInMultipleHeaders) {
+  Annotations Main(R"cpp(
+#include $a[["a.h"]]
+#include $b[["b.h"]]
+void foo() {})cpp");
+  std::string 

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[Tony Allevato via Phabricator via cfe-commits]

allevato updated this revision to Diff 194530.
allevato added a comment.

- Rename generate* to get* and cleanup param names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161

Files:
  clang/include/clang/CodeGen/CodeGenABITypes.h
  clang/lib/CodeGen/CGNonTrivialStruct.cpp

Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp
===
--- clang/lib/CodeGen/CGNonTrivialStruct.cpp
+++ clang/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -14,6 +14,7 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "clang/AST/NonTrivialTypeVisitor.h"
+#include "clang/CodeGen/CodeGenABITypes.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include 
 
@@ -822,6 +823,29 @@
   Gen.callFunc(FuncName, QT, Addrs, CGF);
 }
 
+template  std::array createNullAddressArray();
+
+template <> std::array createNullAddressArray() {
+  return std::array({Address(nullptr, CharUnits::Zero())});
+}
+
+template <> std::array createNullAddressArray() {
+  return std::array({Address(nullptr, CharUnits::Zero()),
+ Address(nullptr, CharUnits::Zero())});
+}
+
+template 
+static llvm::Function *
+getSpecialFunction(G &, StringRef FuncName, QualType QT, bool IsVolatile,
+   std::array Alignments, CodeGenModule ) {
+  QT = IsVolatile ? QT.withVolatile() : QT;
+  // The following call requires an array of addresses as arguments, but doesn't
+  // actually use them (it overwrites them with the addresses of the arguments
+  // of the created function).
+  return Gen.getFunction(FuncName, QT, createNullAddressArray(), Alignments,
+ CGM);
+}
+
 // Functions to emit calls to the special functions of a non-trivial C struct.
 void CodeGenFunction::callCStructDefaultConstructor(LValue Dst) {
   bool IsVolatile = Dst.isVolatile();
@@ -907,3 +931,69 @@
   callSpecialFunction(GenMoveAssignment(getContext()), FuncName, QT, IsVolatile,
   *this, std::array({{DstPtr, SrcPtr}}));
 }
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructDefaultConstructor(
+CodeGenModule , CharUnits DstAlignment, bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenDefaultInitializeFuncName GenName(DstAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return getSpecialFunction(GenDefaultInitialize(Ctx), FuncName, QT, IsVolatile,
+std::array({{DstAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructCopyConstructor(
+CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenBinaryFuncName GenName("__copy_constructor_", DstAlignment,
+   SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return getSpecialFunction(
+  GenCopyConstructor(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructMoveConstructor(
+CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenBinaryFuncName GenName("__move_constructor_", DstAlignment,
+  SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return getSpecialFunction(
+  GenMoveConstructor(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructCopyAssignmentOperator(
+CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenBinaryFuncName GenName("__copy_assignment_", DstAlignment,
+   SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return getSpecialFunction(
+  GenCopyAssignment(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructMoveAssignmentOperator(
+CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenBinaryFuncName GenName("__move_assignment_", DstAlignment,
+  SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return getSpecialFunction(
+  GenMoveAssignment(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructDestructor(
+CodeGenModule , CharUnits DstAlignment, bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenDestructorFuncName GenName("__destructor_", DstAlignment, Ctx);
+  std::string 

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

[Samuel Benzaquen via Phabricator via cfe-commits]

sbenza added inline comments.



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:45
+  /// Evaluates this part to a string and appends it to `result`.
+  virtual llvm::Error eval(const ast_matchers::MatchFinder::MatchResult ,
+   std::string *Result) const = 0;

Why not use `llvm::Expected` as the return type?



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:75
+  // Copy constructor/assignment produce a deep copy.
+  StencilPart(const StencilPart ) : Impl(P.Impl->clone()) {}
+  StencilPart(StencilPart &&) = default;

The interface API is all const. Why not keep a `shared_ptr` instead?
That way we don't have to have users implement a clone function.



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:94
+  return false;
+return Impl->isEqual(*(Other.Impl));
+  }

Parens around Other.Impl are not necessary.



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:110
+  template  static Stencil cat(Ts &&... Parts) {
+Stencil Stencil;
+Stencil.Parts.reserve(sizeof...(Parts));

I don't like naming the variable the same as the type.
You could just use `S` as the variable name. That is ok with llvm style for 
small functions.



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:127
+  // Allow Stencils to operate as std::function, for compatibility with
+  // Transformer's TextGenerator. Calls `eval()` and asserts on failure.
+  std::string operator()(const ast_matchers::MatchFinder::MatchResult &) const;

"asserts" as it only fails in debug mode?



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:130
+
+  bool operator==(const Stencil ) const {
+return Parts == Other.Parts;

I usually prefer free functions for binary operators (friends is fine).
It makes them symmetric.



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:155
+/// statement.
+StencilPart snode(llvm::StringRef Id);
+

`sNode` ?
ie camelCase



Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:45
+
+namespace {
+// An arbitrary fragment of code within a stencil.

maybe this anon namespace should cover the functions above?



Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:76
+
+static bool operator==(const RawTextData , const RawTextData ) {
+  return A.Text == B.Text;

If you define ==, imo you should define != too.
Otherwise just call it `isEqual`



Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:94
+Error evalData(const T , const MatchFinder::MatchResult ,
+   std::string *Result);
+

alignment.
(below too)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59371



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

[clang-tools-extra] r358098 - [clangd] Fix non-indexing of builtin functions like printf when the TU is C

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Apr 10 09:26:58 2019
New Revision: 358098

URL: http://llvm.org/viewvc/llvm-project?rev=358098=rev
Log:
[clangd] Fix non-indexing of builtin functions like printf when the TU is C

Modified:
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=358098=358097=358098=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Wed Apr 10 
09:26:58 2019
@@ -236,8 +236,6 @@ bool SymbolCollector::shouldCollectSymbo
   const ASTContext ,
   const Options ,
   bool IsMainFileOnly) {
-  if (ND.isImplicit())
-return false;
   // Skip anonymous declarations, e.g (anonymous enum/class/struct).
   if (ND.getDeclName().isEmpty())
 return false;
@@ -328,7 +326,9 @@ bool SymbolCollector::handleDeclOccurenc
   // then no public declaration was visible, so assume it's main-file only.
   bool IsMainFileOnly =
   SM.isWrittenInMainFile(SM.getExpansionLoc(ND->getBeginLoc()));
-  if (!shouldCollectSymbol(*ND, *ASTCtx, Opts, IsMainFileOnly))
+  // In C, printf is a redecl of an implicit builtin! So check OrigD instead.
+  if (ASTNode.OrigD->isImplicit() ||
+  !shouldCollectSymbol(*ND, *ASTCtx, Opts, IsMainFileOnly))
 return true;
   // Do not store references to main-file symbols.
   if (CollectRef && !IsMainFileOnly && !isa(ND) &&

Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=358098=358097=358098=diff
==
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Wed Apr 
10 09:26:58 2019
@@ -254,9 +254,8 @@ public:
 auto Factory = llvm::make_unique(
 CollectorOpts, PragmaHandler.get());
 
-std::vector Args = {
-"symbol_collector", "-fsyntax-only", "-xc++",
-"-std=c++11",   "-include",  TestHeaderName};
+std::vector Args = {"symbol_collector", "-fsyntax-only",
+ "-xc++", "-include", TestHeaderName};
 Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
 // This allows to override the "-xc++" with something else, i.e.
 // -xobjective-c++.
@@ -1165,6 +1164,15 @@ TEST_F(SymbolCollectorTest, UsingDecl) {
   EXPECT_THAT(Symbols, Contains(QName("std::foo")));
 }
 
+TEST_F(SymbolCollectorTest, CBuiltins) {
+  // In C, printf in stdio.h is a redecl of an implicit builtin.
+  const char *Header = R"(
+extern int printf(const char*, ...);
+  )";
+  runSymbolCollector(Header, /**/ "", {"-xc"});
+  EXPECT_THAT(Symbols, Contains(QName("printf")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


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

[clang-tools-extra] r358091 - [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Apr 10 08:16:54 2019
New Revision: 358091

URL: http://llvm.org/viewvc/llvm-project?rev=358091=rev
Log:
[clangd] Don't insert extra namespace qualifiers when Sema gets lost.

Summary:
There are cases where Sema can't tell that "foo" in foo::Bar is a
namespace qualifier, like in incomplete macro expansions.

After this patch, if sema reports no specifier but it looks like there's one in
the source code, then we take it into account.

Reworked structure and comments in getQueryScopes to try to fight
creeping complexity - unsure if I succeeded.

I made the test harder (the original version also passes).

Reviewers: ioeric

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=358091=358090=358091=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Apr 10 08:16:54 2019
@@ -45,6 +45,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
@@ -543,54 +544,59 @@ struct SpecifiedScope {
 // (e.g. enclosing namespace).
 std::pair, bool>
 getQueryScopes(CodeCompletionContext , const Sema ,
+   const CompletionPrefix ,
const CodeCompleteOptions ) {
-  auto GetAllAccessibleScopes = [](CodeCompletionContext ) {
-SpecifiedScope Info;
-for (auto *Context : CCContext.getVisitedContexts()) {
-  if (isa(Context))
-Info.AccessibleScopes.push_back(""); // global namespace
-  else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  SpecifiedScope Scopes;
+  for (auto *Context : CCContext.getVisitedContexts()) {
+if (isa(Context))
+  Scopes.AccessibleScopes.push_back(""); // global namespace
+else if (isa(Context))
+  Scopes.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  }
+
+  const CXXScopeSpec *SemaSpecifier =
+  CCContext.getCXXScopeSpecifier().getValueOr(nullptr);
+  // Case 1: unqualified completion.
+  if (!SemaSpecifier) {
+// Case 2 (exception): sema saw no qualifier, but there appears to be one!
+// This can happen e.g. in incomplete macro expansions. Use heuristics.
+if (!HeuristicPrefix.Qualifier.empty()) {
+  vlog("Sema said no scope specifier, but we saw {0} in the source code",
+   HeuristicPrefix.Qualifier);
+  StringRef SpelledSpecifier = HeuristicPrefix.Qualifier;
+  if (SpelledSpecifier.consume_front("::"))
+Scopes.AccessibleScopes = {""};
+  Scopes.UnresolvedQualifier = SpelledSpecifier;
+  return {Scopes.scopesForIndexQuery(), false};
 }
-return Info;
-  };
-
-  auto SS = CCContext.getCXXScopeSpecifier();
-
-  // Unqualified completion (e.g. "vec^").
-  if (!SS) {
-std::vector Scopes;
+// The enclosing namespace must be first, it gets a quality boost.
+std::vector EnclosingAtFront;
 std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext);
-Scopes.push_back(EnclosingScope);
-for (auto  : GetAllAccessibleScopes(CCContext).scopesForIndexQuery()) {
+EnclosingAtFront.push_back(EnclosingScope);
+for (auto  : Scopes.scopesForIndexQuery()) {
   if (EnclosingScope != S)
-Scopes.push_back(std::move(S));
+EnclosingAtFront.push_back(std::move(S));
 }
-// Allow AllScopes completion only for there is no explicit scope 
qualifier.
-return {Scopes, Opts.AllScopes};
-  }
-
-  // Qualified completion ("std::vec^"), we have two cases depending on whether
-  // the qualifier can be resolved by Sema.
-  if ((*SS)->isValid()) { // Resolved qualifier.
-return {GetAllAccessibleScopes(CCContext).scopesForIndexQuery(), false};
+// Allow AllScopes completion as there is no explicit scope qualifier.
+return {EnclosingAtFront, Opts.AllScopes};
   }
-
-  // Unresolved qualifier.
-  SpecifiedScope Info = GetAllAccessibleScopes(CCContext);
-  Info.AccessibleScopes.push_back(""); // Make sure global scope is included.
-
-  llvm::StringRef SpelledSpecifier =
-  Lexer::getSourceText(CharSourceRange::getCharRange((*SS)->getRange()),
-   CCSema.SourceMgr, clang::LangOptions());
+  // Case 3: sema saw and resolved a scope qualifier.
+  if (Specifier && SemaSpecifier->isValid())
+return {Scopes.scopesForIndexQuery(), false};
+
+  // Case 4: There was a qualifier, and Sema didn't resolve it.
+  

[PATCH] D59746: [LibTooling] Fix '-h' option

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

Just chiming in from a LLVM binutils developer perspective. Some of the 
binutils are missing -h as an alias, when they really should have it for 
compatibility (specifically I checked llvm-nm and llvm-symbolizer). As a 
result, a solution that auto-adds -h as an alias would be good, as long as we 
have the ability to override it somehow. I wouldn't mind having logic in 
llvm-objdump/llvm-readobj to explicitly override the short alias if it is 
required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[Tony Allevato via Phabricator via cfe-commits]

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

Thanks for the review! This is ready to go on my end if there aren't any other 
comments.




Comment at: clang/lib/CodeGen/CGNonTrivialStruct.cpp:845
+  // actually use them (it overwrites them with the addresses of the arguments
+  // of the created function).
+  return Gen.getFunction(FuncName, QT, createNullAddressArray(), Alignments,

ahatanak wrote:
> I think `getFunction` shouldn't require passing addresses, but that's not 
> this patch's fault. I'll see if I can remove the parameter. 
Yeah, this hack bummed me out and I tried cleaning up the other related 
functions to have them not reuse the array in this way, but the way std::array 
and the template arg size_t N are currently being used throughout made those 
attempts ripple through in ways that would have required a deeper refactor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161



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

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[John McCall via Phabricator via cfe-commits]

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

LGTM, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161



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

[PATCH] D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358091: [clangd] Dont insert extra namespace 
qualifiers when Sema gets lost. (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60503?vs=194486=194520#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60503

Files:
  clang-tools-extra/trunk/clangd/CodeComplete.cpp
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -2384,19 +2384,19 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ";
 }
 
-// Regression test: clang parser gets confused here and doesn't report the ns::
-// prefix - naive behavior is to insert it again.
-// However we can recognize this from the source code.
-// Test disabled until we can make it pass.
-TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+// Clang parser gets confused here and doesn't report the ns:: prefix.
+// Naive behavior is to insert it again. We examine the source and recover.
+TEST(CompletionTest, NamespaceDoubleInsertion) {
   clangd::CodeCompleteOptions Opts = {};
 
   auto Results = completions(R"cpp(
+namespace foo {
 namespace ns {}
 #define M(X) < X
 M(ns::ABC^
+}
   )cpp",
- {cls("ns::ABCDE")}, Opts);
+ {cls("foo::ns::ABCDE")}, Opts);
   EXPECT_THAT(Results.Completions,
   UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
 }
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -45,6 +45,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
@@ -543,54 +544,59 @@
 // (e.g. enclosing namespace).
 std::pair, bool>
 getQueryScopes(CodeCompletionContext , const Sema ,
+   const CompletionPrefix ,
const CodeCompleteOptions ) {
-  auto GetAllAccessibleScopes = [](CodeCompletionContext ) {
-SpecifiedScope Info;
-for (auto *Context : CCContext.getVisitedContexts()) {
-  if (isa(Context))
-Info.AccessibleScopes.push_back(""); // global namespace
-  else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  SpecifiedScope Scopes;
+  for (auto *Context : CCContext.getVisitedContexts()) {
+if (isa(Context))
+  Scopes.AccessibleScopes.push_back(""); // global namespace
+else if (isa(Context))
+  Scopes.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  }
+
+  const CXXScopeSpec *SemaSpecifier =
+  CCContext.getCXXScopeSpecifier().getValueOr(nullptr);
+  // Case 1: unqualified completion.
+  if (!SemaSpecifier) {
+// Case 2 (exception): sema saw no qualifier, but there appears to be one!
+// This can happen e.g. in incomplete macro expansions. Use heuristics.
+if (!HeuristicPrefix.Qualifier.empty()) {
+  vlog("Sema said no scope specifier, but we saw {0} in the source code",
+   HeuristicPrefix.Qualifier);
+  StringRef SpelledSpecifier = HeuristicPrefix.Qualifier;
+  if (SpelledSpecifier.consume_front("::"))
+Scopes.AccessibleScopes = {""};
+  Scopes.UnresolvedQualifier = SpelledSpecifier;
+  return {Scopes.scopesForIndexQuery(), false};
 }
-return Info;
-  };
-
-  auto SS = CCContext.getCXXScopeSpecifier();
-
-  // Unqualified completion (e.g. "vec^").
-  if (!SS) {
-std::vector Scopes;
+// The enclosing namespace must be first, it gets a quality boost.
+std::vector EnclosingAtFront;
 std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext);
-Scopes.push_back(EnclosingScope);
-for (auto  : GetAllAccessibleScopes(CCContext).scopesForIndexQuery()) {
+EnclosingAtFront.push_back(EnclosingScope);
+for (auto  : Scopes.scopesForIndexQuery()) {
   if (EnclosingScope != S)
-Scopes.push_back(std::move(S));
+EnclosingAtFront.push_back(std::move(S));
 }
-// Allow AllScopes completion only for there is no explicit scope qualifier.
-return {Scopes, Opts.AllScopes};
-  }
-
-  // Qualified completion ("std::vec^"), we have two cases depending on whether
-  // the qualifier can be resolved by Sema.
-  if ((*SS)->isValid()) { // Resolved qualifier.
-return 

[clang-tools-extra] r358093 - clangd: repair the build after SVN r358091

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Apr 10 08:45:05 2019
New Revision: 358093

URL: http://llvm.org/viewvc/llvm-project?rev=358093=rev
Log:
clangd: repair the build after SVN r358091

Fix the name of the variable being checked.  NFCI.

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

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=358093=358092=358093=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Apr 10 08:45:05 2019
@@ -581,7 +581,7 @@ getQueryScopes(CodeCompletionContext 
 return {EnclosingAtFront, Opts.AllScopes};
   }
   // Case 3: sema saw and resolved a scope qualifier.
-  if (Specifier && SemaSpecifier->isValid())
+  if (SemaSpecifier && SemaSpecifier->isValid())
 return {Scopes.scopesForIndexQuery(), false};
 
   // Case 4: There was a qualifier, and Sema didn't resolve it.


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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/Diagnostics.cpp:396
+  for (llvm::StringRef Inc : IncludeStack)
+OS << "In file included from: " << Inc << ":\n";
+}

ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > NIT: could we reuse the function from clang that does the same thing?
> > > 
> > > The code here is pretty simple, though, so writing it here is fine. Just 
> > > wanted to make sure we considered this option and found it's easier to 
> > > redo this work ourselves.
> > there is `TextDiagnostic` which prints the desired output expect the fact 
> > that it also prints the first line saying the header was included in main 
> > file. Therefore, I didn't make use of it since we decided that was not very 
> > useful for our use case. And it requires inheriting from 
> > `DiagnosticRenderer` which was requiring too much boiler plate code just to 
> > get this functionality so instead I've chosen doing it like this.
> > 
> > Can fallback to `TextDiagnostic` if you believe showing `Included in 
> > mainfile.cc:x:y:` at the beginning of the diagnostic won't be annoying.
> LG, it's not too hard to reconstruct the same output.
> Note that D60267 starts outputting notes in a structured way, using the 
> `RelatedInformation` struct from the LSP.
> 
> We should eventually add the include stack into related information too. With 
> that in mind, we should probably add the include stack as a new field to the 
> struct we use for diagnostics.
> 
SG, delaying serialization to LSP layer then.



Comment at: clangd/Diagnostics.cpp:372
+auto IncludeLocation = Info.getSourceManager()
+   .getPresumedLoc(Info.getLocation(),
+   /*UseLineDirectives=*/false)

ilya-biryukov wrote:
> Why use `getPresumedLoc`? Making clangd sensitive to pp directives that 
> rename file or change line numbers does not make any sense.
> 
> Could you also add tests for that?
It was the way `DiagnosticRenderer` emitted include stacks, I had just copied 
it over.
Changing with `SourceManger::getIncludeLoc`


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302



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

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[Tony Allevato via Phabricator via cfe-commits]

allevato added a comment.

Oh, and in case I need to mention this (I don't contribute to llvm/clang 
frequently), I don't have commit access so I'll need someone else to merge it 
in. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

In D60472#1461336 , @manojgupta wrote:

> The motivation for this change is to make "crypto" setting an additive option 
> e.g. like "-mavx" used in many media packages.  Some packages in Chrome want 
> to enable crypto conditionally for a few files to allow crypto feature to be 
> used based on runtime cpu detection. They set "-march=armv8+crypto" flag but 
> it gets overridden by the global "-march=armv8a" flag set by the build system 
> in Chrome OS because the target cpu does not support crypto causing 
> compile-time errors. 
>  Ability to specify "-mcrypto"  standalone makes it an additive option and 
> ensures that it it is not lost. i.e. this will help in decoupling  "-mcrypto" 
> from "-march" so that they could be set independently. The current additive 
> alternate is  '-Xclang -target-feature -Xclang "+crypto" ' which is ugly.


Is that not a limitation of the build system? I'd expect a package to be able 
to locally override a global default rather than vice-versa. Although crypto 
might be the area of concern here and there may be a conveniently named option 
for PPC, where does this stop? Crypto is not the only architectural extension, 
for example see https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html . To 
maintain a consistent interface we'd need command line options for all the 
extensions. May I encourage you to reply to the RFC on command line options 
that I mentioned earlier if it doesn't work for you? I think the extensions 
need to be considered as a whole and not just individually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D59221: [asan] Add gcc 8's driver option -fsanitize=pointer-compare and -fsanitize=pointer-substract.

[pierre gousseau via Phabricator via cfe-commits]

pgousseau added a comment.

Ping!


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

https://reviews.llvm.org/D59221



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

[PATCH] D59415: Do not resolve directory junctions for `-fdiagnostics-absolute-paths` on Windows.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

I don't feel like I have enough background on what you're trying to do to 
review this. It sounds related to clangd issues maybe?

"If the source file path contains directory junctions, and we resolve them when 
printing diagnostic messages"

I didn't think Clang tried to resolve symlinks or otherwise canonicalize paths 
when refering to files in diagnostics in the first place?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59415



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

[PATCH] D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

[Eric Liu via Phabricator via cfe-commits]

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

Feel free to land this. I'll rebase D60126  on 
this when it's in.




Comment at: clangd/CodeComplete.cpp:580
+  // Case 3: sema saw and resolved a scope qualifier.
+  if (SemaSpecifier && SemaSpecifier->isValid())
+return {Scopes.scopesForIndexQuery(), false};

nit: Sema Specifier can't be nullptr at this point.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60503



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

[PATCH] D60055: Check i < FD->getNumParams() before querying

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

I'd be happy to land it, but I do want @rsmith to take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60055



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas created this revision.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
Herald added a project: clang.

This check searches for copy assignment operators which might not handle 
self-assignment properly. There are three patterns of
handling a self assignment situation: self check, copy-and-swap or the less 
common copy-and-move. The new check warns if none of
these patterns is found in a user defined implementation.

See also:
OOP54-CPP. Gracefully handle self-copy assignment
https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP54-CPP.+Gracefully+handle+self-copy+assignment


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D60507

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp

Index: clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp
@@ -0,0 +1,351 @@
+// RUN: %check_clang_tidy %s bugprone-unhandled-self-assignment %t
+
+namespace std {
+
+template 
+void swap(T x, T y) {
+}
+
+template 
+T &(T x) {
+}
+
+template 
+class unique_ptr {
+};
+
+template 
+class shared_ptr {
+};
+
+template 
+class weak_ptr {
+};
+
+} // namespace std
+
+void assert(int expression){};
+
+///
+/// Test cases correctly caught by the check.
+
+class PtrField {
+public:
+  PtrField =(const PtrField );
+
+private:
+  int *p;
+};
+
+PtrField ::operator=(const PtrField ) {
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+  // ...
+  return *this;
+}
+
+// Class with an inline operator definition.
+class InlineDefinition {
+public:
+  InlineDefinition =(const InlineDefinition ) {
+// CHECK-MESSAGES: [[@LINE-1]]:21: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+};
+
+class UniquePtrField {
+public:
+  UniquePtrField =(const UniquePtrField ) {
+// CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::unique_ptr p;
+};
+
+class SharedPtrField {
+public:
+  SharedPtrField =(const SharedPtrField ) {
+// CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::shared_ptr p;
+};
+
+class WeakPtrField {
+public:
+  WeakPtrField =(const WeakPtrField ) {
+// CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::weak_ptr p;
+};
+
+// Class with C array field.
+class CArrayField {
+public:
+  CArrayField =(const CArrayField ) {
+// CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  int array[256];
+};
+
+// Make sure to not ignore cases when the operator definition calls
+// a copy constructor of another class.
+class CopyConstruct {
+public:
+  CopyConstruct =(const CopyConstruct ) {
+// CHECK-MESSAGES: [[@LINE-1]]:18: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+WeakPtrField a;
+WeakPtrField b(a);
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+};
+
+// Make sure to not ignore cases when the operator definition calls
+// a copy assignment operator of another class.
+class AssignOperator {
+public:
+  AssignOperator =(const AssignOperator ) {
+// CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+a.operator=(object.a);
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+  WeakPtrField a;
+};
+
+///
+/// Test cases correctly ignored by the check.
+
+// Self-assignment is checked using the equality operator.
+class SelfCheck1 {
+public:
+  SelfCheck1 =(const SelfCheck1 ) {
+if (this == )
+  return *this;
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+};
+
+class SelfCheck2 {
+public:
+  SelfCheck2 =(const 

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:102
 "bugprone-too-small-loop-variable");
+CheckFactories.registerCheck(
+"bugprone-unhandled-self-assignment");

please order this list alphabetically.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:51
+  // Matcher for standard smart pointers.
+  const auto SmartPointerType = qualType(hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(classTemplateSpecializationDecl(

what about `auto_ptr`? I am actually not sure if we should care, as its 
deprecated and removed already, on the other hand legacy code probably still 
has it.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:10
+
+This check corresponds to the CERT C++ Coding Standard rule
+`OOP54-CPP. Gracefully handle self-copy assignment

It is worth an alias into the cert module. Please add this with this revision 
already.



Comment at: 
clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp:351
+  int *p;
+};

Please add tests with templated classes and self-assignment.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60507



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

[PATCH] D60510: [ADT] Fix template parameter names of llvm::{upper|lower}_bound

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: llvm-commits, dexonsmith.
Herald added a project: LLVM.

Rename template parameter for a search value from 'ForwardIt' to 'T'.
While here, also use perfect forwarding to pass the value to STL algos.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60510

Files:
  llvm/include/llvm/ADT/STLExtras.h


Index: llvm/include/llvm/ADT/STLExtras.h
===
--- llvm/include/llvm/ADT/STLExtras.h
+++ llvm/include/llvm/ADT/STLExtras.h
@@ -1277,28 +1277,32 @@
 
 /// Provide wrappers to std::lower_bound which take ranges instead of having to
 /// pass begin/end explicitly.
-template 
-auto lower_bound(R &, ForwardIt I) -> decltype(adl_begin(Range)) {
-  return std::lower_bound(adl_begin(Range), adl_end(Range), I);
+template 
+auto lower_bound(R &, T &) -> decltype(adl_begin(Range)) {
+  return std::lower_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value));
 }
 
-template 
-auto lower_bound(R &, ForwardIt I, Compare C)
+template 
+auto lower_bound(R &, T &, Compare C)
 -> decltype(adl_begin(Range)) {
-  return std::lower_bound(adl_begin(Range), adl_end(Range), I, C);
+  return std::lower_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value), C);
 }
 
 /// Provide wrappers to std::upper_bound which take ranges instead of having to
 /// pass begin/end explicitly.
-template 
-auto upper_bound(R &, ForwardIt I) -> decltype(adl_begin(Range)) {
-  return std::upper_bound(adl_begin(Range), adl_end(Range), I);
+template 
+auto upper_bound(R &, T && Value) -> decltype(adl_begin(Range)) {
+  return std::upper_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value));
 }
 
-template 
-auto upper_bound(R &, ForwardIt I, Compare C)
+template 
+auto upper_bound(R &, T && Value, Compare C)
 -> decltype(adl_begin(Range)) {
-  return std::upper_bound(adl_begin(Range), adl_end(Range), I, C);
+  return std::upper_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value), C);
 }
 /// Wrapper function around std::equal to detect if all elements
 /// in a container are same.


Index: llvm/include/llvm/ADT/STLExtras.h
===
--- llvm/include/llvm/ADT/STLExtras.h
+++ llvm/include/llvm/ADT/STLExtras.h
@@ -1277,28 +1277,32 @@
 
 /// Provide wrappers to std::lower_bound which take ranges instead of having to
 /// pass begin/end explicitly.
-template 
-auto lower_bound(R &, ForwardIt I) -> decltype(adl_begin(Range)) {
-  return std::lower_bound(adl_begin(Range), adl_end(Range), I);
+template 
+auto lower_bound(R &, T &) -> decltype(adl_begin(Range)) {
+  return std::lower_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value));
 }
 
-template 
-auto lower_bound(R &, ForwardIt I, Compare C)
+template 
+auto lower_bound(R &, T &, Compare C)
 -> decltype(adl_begin(Range)) {
-  return std::lower_bound(adl_begin(Range), adl_end(Range), I, C);
+  return std::lower_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value), C);
 }
 
 /// Provide wrappers to std::upper_bound which take ranges instead of having to
 /// pass begin/end explicitly.
-template 
-auto upper_bound(R &, ForwardIt I) -> decltype(adl_begin(Range)) {
-  return std::upper_bound(adl_begin(Range), adl_end(Range), I);
+template 
+auto upper_bound(R &, T && Value) -> decltype(adl_begin(Range)) {
+  return std::upper_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value));
 }
 
-template 
-auto upper_bound(R &, ForwardIt I, Compare C)
+template 
+auto upper_bound(R &, T && Value, Compare C)
 -> decltype(adl_begin(Range)) {
-  return std::upper_bound(adl_begin(Range), adl_end(Range), I, C);
+  return std::upper_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value), C);
 }
 /// Wrapper function around std::equal to detect if all elements
 /// in a container are same.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r358087 - clang-cl: Fix parsing of the /F option (PR41405)

[Hans Wennborg via cfe-commits]

Author: hans
Date: Wed Apr 10 07:27:47 2019
New Revision: 358087

URL: http://llvm.org/viewvc/llvm-project?rev=358087=rev
Log:
clang-cl: Fix parsing of the /F option (PR41405)

Modified:
cfe/trunk/include/clang/Driver/CLCompatOptions.td
cfe/trunk/test/Driver/cl-options.c

Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=358087=358086=358087=diff
==
--- cfe/trunk/include/clang/Driver/CLCompatOptions.td (original)
+++ cfe/trunk/include/clang/Driver/CLCompatOptions.td Wed Apr 10 07:27:47 2019
@@ -404,7 +404,7 @@ def _SLASH_d2 : CLJoined<"d2">;
 def _SLASH_doc : CLJoined<"doc">;
 def _SLASH_FA_joined : CLJoined<"FA">;
 def _SLASH_favor : CLJoined<"favor">;
-def _SLASH_F : CLFlag<"F">;
+def _SLASH_F : CLJoinedOrSeparate<"F">;
 def _SLASH_Fm : CLJoined<"Fm">;
 def _SLASH_Fr : CLJoined<"Fr">;
 def _SLASH_FR : CLJoined<"FR">;

Modified: cfe/trunk/test/Driver/cl-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=358087=358086=358087=diff
==
--- cfe/trunk/test/Driver/cl-options.c (original)
+++ cfe/trunk/test/Driver/cl-options.c Wed Apr 10 07:27:47 2019
@@ -400,7 +400,7 @@
 // RUN: /d2FH4 \
 // RUN: /docname \
 // RUN: /EHsc \
-// RUN: /F \
+// RUN: /F 42 \
 // RUN: /FA \
 // RUN: /FAc \
 // RUN: /Fafilename \


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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

The motivation for this change is to make "crypto" setting an additive option 
e.g. like "-mavx" used in many media packages.  Some packages in Chrome want to 
enable crypto conditionally for a few files to allow crypto feature to be used 
based on runtime cpu detection. They set "-march=armv8+crypto" flag but it gets 
overridden by the global "-march=armv8a" flag set by the build system in Chrome 
OS because the target cpu does not support crypto causing compile-time errors. 
Ability to specify "-mcrypto"  standalone makes it an additive option and 
ensures that it it is not lost. i.e. this will help in decoupling  "-mcrypto" 
from "-march" so that they could be set independently. The current additive 
alternate is  '-Xclang -target-feature -Xclang "+crypto" ' which is ugly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 194505.
DennisL marked an inline comment as done.
DennisL added a comment.

Changed the following

- addressed reviewer feedback
- fetch the placement parameter as written
- add further test cases as requested
- stylistic changes
- add nothrow parameter support
- ignore matches with unresolved template parameters
- add examples of bad and good code


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp

Index: test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
@@ -0,0 +1,118 @@
+// RUN: %check_clang_tidy %s bugprone-placement-new-target-type-mismatch %t
+
+// definitions
+	
+namespace std {
+struct nothrow_t { explicit nothrow_t() = default; } nothrow;
+template T* addressof(T& arg) noexcept;
+template< class T > struct remove_reference  {typedef T type;};
+template< class T > struct remove_reference  {typedef T type;};
+template< class T > struct remove_reference {typedef T type;};
+template< class T >
+T&& forward( typename std::remove_reference::type& t ) noexcept;
+} // namespace std
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+void *operator new[](size_type, void *);
+void* operator new(size_type size, const std::nothrow_t&) noexcept;
+void* operator new(size_type size, const std::nothrow_t&) noexcept;
+void* operator new[](size_type size, const std::nothrow_t&) noexcept;
+
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+};
+
+template
+T& getT() {
+  static T f;
+  return f;
+}
+
+// instances emitting warnings
+
+void f1() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f2() {
+  int * ptr = new int;
+  new (ptr) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f3() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float[13];
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f4() {
+  new (std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f5() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f6() {
+  char array[17*sizeof(char)];
+  new (array) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+// instances not emitting a warning
+
+void g1() {
+  Foo * ptr = new Foo;
+  new (ptr) Foo;
+}
+
+void g2() {
+  char *ptr = new char[17*sizeof(char)];
+  new ((float *)ptr) float{13.f};
+}
+
+void g3() {
+  char array[17*sizeof(char)];
+  new (array) char('A');
+}
+
+void g4() {
+  new ((void *)std::addressof(getT())) Foo;
+}
+
+union
+{
+  char * buffer;
+} Union;
+
+template 
+void g5(U &&... V) {
+  new (static_cast(Union.buffer)) T(std::forward(V)...);
+}
+
+template 
+void g6(U &&... V) {
+  new (std::nothrow) T(std::forward(V)...);
+}
+
+template  void g7(U &&... Us) {
+  new (static_cast(Union.buffer)) T(std::forward(Us)...);
+}
+
+template  void g8(U &&... Us) {
+  new (Union.buffer) T(std::forward(Us)...);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -55,6 +55,7 @@
bugprone-move-forwarding-reference
bugprone-multiple-statement-macro
bugprone-parent-virtual-call
+   bugprone-placement-new-target-type-mismatch
bugprone-sizeof-container
bugprone-sizeof-expression
bugprone-string-constructor
Index: docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
@@ -0,0 +1,23 @@
+.. title:: clang-tidy - misc-placement-new-target-type-mismatch
+

[PATCH] D60055: Check i < FD->getNumParams() before querying

[Violet via Phabricator via cfe-commits]

Violet added a comment.

Thanks. Can you land it for me? I'm a newcommer without landing privilege.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60055



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

I'm not in favour of adding AArch64 support to -mcrypto -mnocrypto for a few 
reasons:

- Arm are trying to keep the options for controlling target features as 
consistent as possible with GCC and this option isn't supported in GCC 
(https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html)
- There is http://lists.llvm.org/pipermail/llvm-dev/2018-September/126346.html 
which is Arm's proposal for how target options can be better supported in 
Clang. I think that supporting -mcrypto as well would add more complexity as 
there are interactions between the options.
- Arm 32-bit also supports crypto so this would need to be added to Arm as well 
for consistency.

Can you expand on why you need -m[no]crypto?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

In D60472#1461351 , @peter.smith wrote:

>




> Is that not a limitation of the build system? I'd expect a package to be able 
> to locally override a global default rather than vice-versa. Although crypto 
> might be the area of concern here and there may be a conveniently named 
> option for PPC, where does this stop? Crypto is not the only architectural 
> extension, for example see 
> https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html . To maintain a 
> consistent interface we'd need command line options for all the extensions. 
> May I encourage you to reply to the RFC on command line options that I 
> mentioned earlier if it doesn't work for you? I think the extensions need to 
> be considered as a whole and not just individually.

While it partly is a build system issue, another problem is enabling crypto via 
"-march" requires picking an architecture as well. So even if it could override 
the global default, it would also override the global "-march" as well. If the 
global "-march" was a better/higher option, it does not make sense to override 
it e.g. "-march=armv8-a+crypto" overriding "-march=armv8.3-a" is not helpful 
since it will disable 8.3 features.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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