date:20181001

[PATCH] D52705: First-pass at ARM hard/soft-float multilib driver (NOT WORKING YET)

2018-10-01 Thread Kristina Brooks via Phabricator via cfe-commits

kristina added subscribers: rsmith, kristina.
kristina added reviewers: t.p.northover, rsmith.
kristina added a comment.

Accepted blocking differential https://reviews.llvm.org/D52149, updated 
reviewers to include ARM code owner and @rsmith.


Repository:
  rC Clang

https://reviews.llvm.org/D52705



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

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

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

So we don't have to run "check-clang-tools" (which builds and tests all
clang tools) to verify our clangd-related change. It'd save waiting time for
clangd developers.

check-clangd (build ~1000 files, run ~340 tests) vs check-clang-tools (build
~3000 files, run ~1000 tests).

In the future, we probably want to add similar target for other
clang-tools (e.g. clang-tidy).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52710

Files:
  test/CMakeLists.txt


Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -73,20 +73,38 @@
 )
 endif()
 
-set(llvm_utils
-  FileCheck count not
-  )
-
-foreach(t ${llvm_utils})
-  if(TARGET ${t})
-list(APPEND CLANG_TOOLS_TEST_DEPS ${t})
-  endif()
-endforeach()
-
+macro(add_llvm_utils_deps deps)
+  set(llvm_utils
+  FileCheck count not
+  )
+  foreach(t ${llvm_utils})
+if(TARGET ${t})
+  list(APPEND "${deps}" ${t})
+endif()
+  endforeach()
+endmacro()
+
+add_llvm_utils_deps(CLANG_TOOLS_TEST_DEPS)
 add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression 
tests"
   ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   ARGS ${CLANG_TOOLS_TEST_EXTRA_ARGS}
   )
 
 set_target_properties(check-clang-tools PROPERTIES FOLDER "Clang extra tools' 
tests")
+
+# Setup an individual test for building and testing clangd-only stuff.
+set(CLANGD_TEST_DEPS
+  clangd
+  ClangdTests
+  # clangd-related tools which don't have tests, add them to the test to make
+  # sure we don't introduce new changes that break their compilations.
+  clangd-indexer
+  dexp
+)
+add_llvm_utils_deps(CLANGD_TEST_DEPS)
+add_lit_testsuite(check-clangd "Running the Clangd regression tests"
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/clangd;${CMAKE_CURRENT_BINARY_DIR}/clangd
+  DEPENDS ${CLANGD_TEST_DEPS}
+)
+set_target_properties(check-clangd PROPERTIES FOLDER "Clangd tests")


Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -73,20 +73,38 @@
 )
 endif()
 
-set(llvm_utils
-  FileCheck count not
-  )
-
-foreach(t ${llvm_utils})
-  if(TARGET ${t})
-list(APPEND CLANG_TOOLS_TEST_DEPS ${t})
-  endif()
-endforeach()
-
+macro(add_llvm_utils_deps deps)
+  set(llvm_utils
+  FileCheck count not
+  )
+  foreach(t ${llvm_utils})
+if(TARGET ${t})
+  list(APPEND "${deps}" ${t})
+endif()
+  endforeach()
+endmacro()
+
+add_llvm_utils_deps(CLANG_TOOLS_TEST_DEPS)
 add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression tests"
   ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   ARGS ${CLANG_TOOLS_TEST_EXTRA_ARGS}
   )
 
 set_target_properties(check-clang-tools PROPERTIES FOLDER "Clang extra tools' tests")
+
+# Setup an individual test for building and testing clangd-only stuff.
+set(CLANGD_TEST_DEPS
+  clangd
+  ClangdTests
+  # clangd-related tools which don't have tests, add them to the test to make
+  # sure we don't introduce new changes that break their compilations.
+  clangd-indexer
+  dexp
+)
+add_llvm_utils_deps(CLANGD_TEST_DEPS)
+add_lit_testsuite(check-clangd "Running the Clangd regression tests"
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/clangd;${CMAKE_CURRENT_BINARY_DIR}/clangd
+  DEPENDS ${CLANGD_TEST_DEPS}
+)
+set_target_properties(check-clangd PROPERTIES FOLDER "Clangd tests")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

Thanks for doing this!
Sorry for the dumb questions, I don't get cmake.




Comment at: test/CMakeLists.txt:36
 
 set(CLANG_TOOLS_TEST_DEPS
   # For the clang-apply-replacements test that uses clang-rename.

can we add all of clangd to this list with a loop, instead of duplicating?



Comment at: test/CMakeLists.txt:76
 
-set(llvm_utils
-  FileCheck count not
-  )
-
-foreach(t ${llvm_utils})
-  if(TARGET ${t})
-list(APPEND CLANG_TOOLS_TEST_DEPS ${t})
-  endif()
-endforeach()
-
+macro(add_llvm_utils_deps deps)
+  set(llvm_utils

Conversely, I'd consider just expanding these out into each of the dep lists 
rather than keeping two levels of indirection here.





Comment at: test/CMakeLists.txt:107
+add_lit_testsuite(check-clangd "Running the Clangd regression tests"
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/clangd;${CMAKE_CURRENT_BINARY_DIR}/clangd
+  DEPENDS ${CLANGD_TEST_DEPS}

Is /Unit/ really right?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52710



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

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

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



Comment at: test/CMakeLists.txt:36
 
 set(CLANG_TOOLS_TEST_DEPS
   # For the clang-apply-replacements test that uses clang-rename.

sammccall wrote:
> can we add all of clangd to this list with a loop, instead of duplicating?
We could do this, or alternatively we can remove all clangd-related binaries, 
and add `check-clangd` target. The only difference is that the output will 
change a bit, like below, I think this is not a big deal, WDYT?

```
[0/2] Running the Clangd regression tests
Testing Time: 2.78s
  Expected Passes: 343
  Unsupported Tests  : 1
[1/2] Running the Clang extra tools' regression tests
Testing Time: 4.08s
  Expected Passes: 1001
  Unsupported Tests  : 2
```




Comment at: test/CMakeLists.txt:107
+add_lit_testsuite(check-clangd "Running the Clangd regression tests"
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/clangd;${CMAKE_CURRENT_BINARY_DIR}/clangd
+  DEPENDS ${CLANGD_TEST_DEPS}

sammccall wrote:
> Is /Unit/ really right?
Yes, indeed. It is used for running unittest (we have lit configuration file in 
`test/Unit/` directory).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52710



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

[clang-tools-extra] r343448 - [clangd] Fix header mapping for std::string. NFC

2018-10-01 Thread Eric Liu via cfe-commits

Author: ioeric
Date: Mon Oct  1 01:50:49 2018
New Revision: 343448

URL: http://llvm.org/viewvc/llvm-project?rev=343448&view=rev
Log:
[clangd] Fix header mapping for std::string. NFC

Some implementation has std::string declared in .

Modified:
clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp

Modified: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp?rev=343448&r1=343447&r2=343448&view=diff
==
--- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp Mon Oct  1 
01:50:49 2018
@@ -143,6 +143,7 @@ void addSystemHeadersMapping(CanonicalIn
   {"std::basic_stringstream", ""},
   {"std::istringstream", ""},
   {"std::ostringstream", ""},
+  {"std::string", ""},
   {"std::stringbuf", ""},
   {"std::stringstream", ""},
   {"std::wistringstream", ""},


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

[PATCH] D52225: [clang] Implement Override Suggestions in Sema.

2018-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added a comment.

ping


Repository:
  rC Clang

https://reviews.llvm.org/D52225



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

[PATCH] D52334: [clang-tidy] Build it even without static analyzer

2018-10-01 Thread Stephen Kelly via Phabricator via cfe-commits

steveire added a comment.

I'll take care of it in a few days and commit it myself. Thanks!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52334



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

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Sam McCall via Phabricator via cfe-commits

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

Awesome! Please do remove that duplication if it's easy.




Comment at: test/CMakeLists.txt:36
 
 set(CLANG_TOOLS_TEST_DEPS
   # For the clang-apply-replacements test that uses clang-rename.

hokein wrote:
> sammccall wrote:
> > can we add all of clangd to this list with a loop, instead of duplicating?
> We could do this, or alternatively we can remove all clangd-related binaries, 
> and add `check-clangd` target. The only difference is that the output will 
> change a bit, like below, I think this is not a big deal, WDYT?
> 
> ```
> [0/2] Running the Clangd regression tests
> Testing Time: 2.78s
>   Expected Passes: 343
>   Unsupported Tests  : 1
> [1/2] Running the Clang extra tools' regression tests
> Testing Time: 4.08s
>   Expected Passes: 1001
>   Unsupported Tests  : 2
> ```
> 
Yes, that looks great!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52710



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

[PATCH] D52225: [clang] Implement Override Suggestions in Sema.

2018-10-01 Thread Eric Liu via Phabricator via cfe-commits

ioeric added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:1603
+   const CodeCompletionContext &CCContext,
+   CodeCompletionBuilder &Builder, Sema &S) {
+  const auto *CR = llvm::dyn_cast(S.CurContext);

Sema is available via `Results.getSema()`



Comment at: lib/Sema/SemaCodeComplete.cpp:1639
+llvm::raw_string_ostream OS(OverrideSignature);
+CodeCompletionResult CCR(Method, 0);
+PrintingPolicy Policy =

Could you add comments explaining what the following code does? The original 
code in clangd only has `Results.emplace_back(Method, 0);`, and it's 
non-trivial what the new code here is for.



Comment at: lib/Sema/SemaCodeComplete.cpp:1644
+S.getPreprocessor(), S.getASTContext(), Builder,
+false, CCContext, Policy);
+for(const auto& C : *Builder.TakeString()) {

nit: `/*Parameter-name=*/false`



Comment at: lib/Sema/SemaCodeComplete.cpp:2905
+
+void CodeCompletionResult::createCodeCompletionStringForDecl(
+Preprocessor &PP, ASTContext &Ctx,

nit: I think the contract would be clearer if this returns 
`CodeCompletionString *`. The empty `return`s below are a bit hard to follow.



Comment at: test/CodeCompletion/overrides.cpp:20
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:8 %s -o - | 
FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC1: COMPLETION: Pattern : int ttt(bool param) const override
+// CHECK-CC2-NOT: COMPLETION: Pattern : int ttt(bool param) const override

nit: add `{{$}}` to the end of patterns.



Comment at: test/CodeCompletion/overrides.cpp:20
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:8 %s -o - | 
FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC1: COMPLETION: Pattern : int ttt(bool param) const override
+// CHECK-CC2-NOT: COMPLETION: Pattern : int ttt(bool param) const override

ioeric wrote:
> nit: add `{{$}}` to the end of patterns.
nit: I think it's more common practice to put CHECKs for one completion in one 
group and after the completion command. I think it would be easier to read.

Please also add comment on what each case is testing.


Repository:
  rC Clang

https://reviews.llvm.org/D52225



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

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-01 Thread Takuto Ikuta via Phabricator via cfe-commits

takuto.ikuta updated this revision to Diff 167682.
takuto.ikuta added a comment.

Extract inline function export check to function


https://reviews.llvm.org/D51340

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/CLCompatOptions.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp

Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,159 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc   \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc   \
+// RUN: -emit-llvm -O0 -o - |   \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+  static int static_variable = 0;
+  ++static_variable;
+  return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+  static int static_variable = 0;
+  ++static_variable;
+  return static_variable;
+}
+
+int ImportedFunctionUser() {
+  return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+  // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+  NoTemplateExportedClass() = default;
+
+  // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+  // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+  void InclassDefFunc() {}
+
+  int f();
+
+  // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+  int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+  }
+
+  // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+  int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+  }
+
+  // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+  __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+  // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+  __forceinline void InlineOutclassDefFunc();
+
+  // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+  __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+  // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+  void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+  static int static_variable = 0;
+  return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+  NoTemplateExportedClass a;
+  a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+  void InclassDefFunc() {}
+  void OutclassDefFunc();
+
+  T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+class A11{};
+class B22{};
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11@@
+// DEFAULT-DAG: define weak_odr dso_local dllexpor

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-01 Thread Takuto Ikuta via Phabricator via cfe-commits

takuto.ikuta updated this revision to Diff 167683.
takuto.ikuta added a comment.

Update comment for Sema::ActOnFinishInlineFunctionDef


https://reviews.llvm.org/D51340

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/CLCompatOptions.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp

Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,159 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc   \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc   \
+// RUN: -emit-llvm -O0 -o - |   \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+  static int static_variable = 0;
+  ++static_variable;
+  return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+  static int static_variable = 0;
+  ++static_variable;
+  return static_variable;
+}
+
+int ImportedFunctionUser() {
+  return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+  // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+  NoTemplateExportedClass() = default;
+
+  // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+  // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+  void InclassDefFunc() {}
+
+  int f();
+
+  // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+  int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+  }
+
+  // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+  int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+  }
+
+  // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+  __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+  // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+  __forceinline void InlineOutclassDefFunc();
+
+  // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+  __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+  // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+  void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+  static int static_variable = 0;
+  return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+  NoTemplateExportedClass a;
+  a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+  void InclassDefFunc() {}
+  void OutclassDefFunc();
+
+  T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+class A11{};
+class B22{};
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11@@
+// DEFAULT-DAG: define weak_odr dso_local dll

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-01 Thread Takuto Ikuta via Phabricator via cfe-commits

takuto.ikuta added a comment.

Thank you for review!

In https://reviews.llvm.org/D51340#1246508, @hans wrote:

> The static local stuff still makes me nervous. How much does Chromium break 
> if we just ignore the problem? And how does -fvisibility-inlines-hidden 
> handle the issue?


I'm not sure how much chromium will break, if we ignore static local variables. 
But ignoring static local var may cause some unhappy behavior and experience to 
developer.
I'd like to have check for local static variables as 
-fvisibility-inlines-hidden does.

-fvisibility-inlines-hidden checks static local around here.
https://github.com/llvm-project/llvm-project-20170507/blob/77b7ca42b0ea496373c5e5a9710b1fe11e1fba68/clang/lib/AST/Decl.cpp#L1265

> Also, Sema already has a check for static locals in C inline functions:
> 
>   $ echo "inline void f() { static int x; }" | bin/clang -c -x c -
>   :1:19: warning: non-constant static local variable in inline 
> function may be different in different files [-Wstatic-local-in-inline]
>   inline void f() { static int x; }
> ^
>   
> 
> 
> could we reuse that check somehow for this case with static locals in 
> dllexport inline methods?

I think we can do the same thing when we are given -fno-dllexport-inlines 
around here.
https://github.com/llvm-project/llvm-project-20170507/blob/77b7ca42b0ea496373c5e5a9710b1fe11e1fba68/clang/lib/Sema/SemaDecl.cpp#L6661

But I bit prefer to do export functions with static local variables. Because 
that is consistent with -fvisibility-inlines-hidden and we won't need more 
changes to chromium than the CLs in description.




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5702
+!getLangOpts().DllExportInlines &&
+Class->getTemplateSpecializationKind() != 
TSK_ExplicitInstantiationDeclaration &&
+Class->getTemplateSpecializationKind() != 
TSK_ExplicitInstantiationDefinition) {

hans wrote:
> What worries me is that now we have logic in two different places about 
> inheriting the dll attribute.
> 
> The new place in ActOnFinishInlineFunctionDef doesn't have the same checks 
> (checking for MS ABI and the template specialization kind) which means the 
> logic for when to inherit is already a little out of sync...
Agree. Do you allow me to extract check to function and re-use that?


https://reviews.llvm.org/D51340



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

[PATCH] D52711: [clangd] Dex: add Corpus factory for iterators, rename, fold constant. NFC

2018-10-01 Thread 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.

- Corpus avoids having to pass size to the true iterator, and (soon) any 
iterator that might optimize down to true.
- Shorten names of factory functions now they're scoped to the Corpus. 
intersect() and unionOf() rather than createAnd() or createOr() as this seems 
to read better to me, and fits with other short names. Opinion wanted!
- DEFAULT_BOOST_SCORE --> 1. This is a multiplier, don't obfuscate identity.
- Simplify variadic templates in Iterator.h


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52711

Files:
  clangd/index/dex/Dex.cpp
  clangd/index/dex/Dex.h
  clangd/index/dex/Iterator.cpp
  clangd/index/dex/Iterator.h
  clangd/index/dex/PostingList.cpp
  unittests/clangd/DexTests.cpp

Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -70,15 +70,16 @@
 }
 
 TEST(DexIterators, AndTwoLists) {
+  Corpus C{1};
   const PostingList L0({0, 5, 7, 10, 42, 320, 9000});
   const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000});
 
-  auto And = createAnd(L1.iterator(), L0.iterator());
+  auto And = C.intersect(L1.iterator(), L0.iterator());
 
   EXPECT_FALSE(And->reachedEnd());
   EXPECT_THAT(consumeIDs(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U));
 
-  And = createAnd(L0.iterator(), L1.iterator());
+  And = C.intersect(L0.iterator(), L1.iterator());
 
   And->advanceTo(0);
   EXPECT_EQ(And->peek(), 0U);
@@ -94,11 +95,12 @@
 }
 
 TEST(DexIterators, AndThreeLists) {
+  Corpus C{1};
   const PostingList L0({0, 5, 7, 10, 42, 320, 9000});
   const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000});
   const PostingList L2({1, 4, 7, 11, 30, 60, 320, 9000});
 
-  auto And = createAnd(L0.iterator(), L1.iterator(), L2.iterator());
+  auto And = C.intersect(L0.iterator(), L1.iterator(), L2.iterator());
   EXPECT_EQ(And->peek(), 7U);
   And->advanceTo(300);
   EXPECT_EQ(And->peek(), 320U);
@@ -108,10 +110,11 @@
 }
 
 TEST(DexIterators, OrTwoLists) {
+  Corpus C{1};
   const PostingList L0({0, 5, 7, 10, 42, 320, 9000});
   const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000});
 
-  auto Or = createOr(L0.iterator(), L1.iterator());
+  auto Or = C.unionOf(L0.iterator(), L1.iterator());
 
   EXPECT_FALSE(Or->reachedEnd());
   EXPECT_EQ(Or->peek(), 0U);
@@ -134,18 +137,19 @@
   Or->advanceTo(9001);
   EXPECT_TRUE(Or->reachedEnd());
 
-  Or = createOr(L0.iterator(), L1.iterator());
+  Or = C.unionOf(L0.iterator(), L1.iterator());
 
   EXPECT_THAT(consumeIDs(*Or),
   ElementsAre(0U, 4U, 5U, 7U, 10U, 30U, 42U, 60U, 320U, 9000U));
 }
 
 TEST(DexIterators, OrThreeLists) {
+  Corpus C{1};
   const PostingList L0({0, 5, 7, 10, 42, 320, 9000});
   const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000});
   const PostingList L2({1, 4, 7, 11, 30, 60, 320, 9000});
 
-  auto Or = createOr(L0.iterator(), L1.iterator(), L2.iterator());
+  auto Or = C.unionOf(L0.iterator(), L1.iterator(), L2.iterator());
 
   EXPECT_FALSE(Or->reachedEnd());
   EXPECT_EQ(Or->peek(), 0U);
@@ -194,17 +198,18 @@
   //  |1, 5, 7, 9||1, 5||0, 3, 5|
   //  +--++++---+
   //
+  Corpus C{10};
   const PostingList L0({1, 3, 5, 8, 9});
   const PostingList L1({1, 5, 7, 9});
   const PostingList L2({1, 5});
   const PostingList L3({0, 3, 5});
 
   // Root of the query tree: [1, 5]
-  auto Root = createAnd(
+  auto Root = C.intersect(
   // Lower And Iterator: [1, 5, 9]
-  createAnd(L0.iterator(), createBoost(L1.iterator(), 2U)),
+  C.intersect(L0.iterator(), C.boost(L1.iterator(), 2U)),
   // Lower Or Iterator: [0, 1, 5]
-  createOr(createBoost(L2.iterator(), 3U), createBoost(L3.iterator(), 4U)));
+  C.unionOf(C.boost(L2.iterator(), 3U), C.boost(L3.iterator(), 4U)));
 
   EXPECT_FALSE(Root->reachedEnd());
   EXPECT_EQ(Root->peek(), 1U);
@@ -241,51 +246,55 @@
 }
 
 TEST(DexIterators, Limit) {
+  Corpus C{1};
   const PostingList L0({3, 6, 7, 20, 42, 100});
   const PostingList L1({1, 3, 5, 6, 7, 30, 100});
   const PostingList L2({0, 3, 5, 7, 8, 100});
 
-  auto DocIterator = createLimit(L0.iterator(), 42);
+  auto DocIterator = C.limit(L0.iterator(), 42);
   EXPECT_THAT(consumeIDs(*DocIterator), ElementsAre(3, 6, 7, 20, 42, 100));
 
-  DocIterator = createLimit(L0.iterator(), 3);
+  DocIterator = C.limit(L0.iterator(), 3);
   EXPECT_THAT(consumeIDs(*DocIterator), ElementsAre(3, 6, 7));
 
-  DocIterator = createLimit(L0.iterator(), 0);
+  DocIterator = C.limit(L0.iterator(), 0);
   EXPECT_THAT(consumeIDs(*DocIterator), ElementsAre());
 
-  auto AndIterator = createAnd(
-  createLimit(createTrue(9000), 343), createLimit(L0.iterator(), 2),
-  createLimit(L1.iterator(), 3), createLimit(L2.iterator(), 42));
+  auto AndIte

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-01 Thread Takuto Ikuta via Phabricator via cfe-commits

takuto.ikuta added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:12624
+  isa(D)) {
+CXXMethodDecl *MD = dyn_cast(D);
+CXXRecordDecl *Class = MD->getParent();

hans wrote:
> Hmm, now we're adding an AST walk over all inline methods which probably 
> slows us down a bit. Not sure I have any better ideas though.
> 
> In any case, ActOnFinishInlineFunctionDef needs a comment explaining why it's 
> doing this.
Added comment. I think typical code does not have static variables in inline 
function and this check is worth to be done for the performance improvement.


https://reviews.llvm.org/D51340



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

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 167689.
hokein marked 2 inline comments as done.
hokein added a comment.

Remove clangd-binary dep in check-clang-tools, and use check-clangd.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52710

Files:
  test/CMakeLists.txt


Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -43,7 +43,6 @@
   # Individual tools we test.
   clang-apply-replacements
   clang-change-namespace
-  clangd
   clang-doc
   clang-include-fixer
   clang-move
@@ -53,10 +52,7 @@
   modularize
   pp-trace
 
-  # These individual tools have no tests, add them here to make them compile
-  # together with check-clang-tools, so that we won't break them in the future.
-  clangd-indexer
-  dexp
+  check-clangd
 
   # Unit tests
   ExtraToolsUnitTests
@@ -73,20 +69,38 @@
 )
 endif()
 
-set(llvm_utils
-  FileCheck count not
-  )
-
-foreach(t ${llvm_utils})
-  if(TARGET ${t})
-list(APPEND CLANG_TOOLS_TEST_DEPS ${t})
-  endif()
-endforeach()
-
+macro(add_llvm_utils_deps deps)
+  set(llvm_utils
+  FileCheck count not
+  )
+  foreach(t ${llvm_utils})
+if(TARGET ${t})
+  list(APPEND "${deps}" ${t})
+endif()
+  endforeach()
+endmacro()
+
+add_llvm_utils_deps(CLANG_TOOLS_TEST_DEPS)
 add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression 
tests"
   ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   ARGS ${CLANG_TOOLS_TEST_EXTRA_ARGS}
   )
 
 set_target_properties(check-clang-tools PROPERTIES FOLDER "Clang extra tools' 
tests")
+
+# Setup an individual test for building and testing clangd-only stuff.
+set(CLANGD_TEST_DEPS
+  clangd
+  ClangdTests
+  # clangd-related tools which don't have tests, add them to the test to make
+  # sure we don't introduce new changes that break their compilations.
+  clangd-indexer
+  dexp
+)
+add_llvm_utils_deps(CLANGD_TEST_DEPS)
+add_lit_testsuite(check-clangd "Running the Clangd regression tests"
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/clangd;${CMAKE_CURRENT_BINARY_DIR}/clangd
+  DEPENDS ${CLANGD_TEST_DEPS}
+)
+set_target_properties(check-clangd PROPERTIES FOLDER "Clangd tests")


Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -43,7 +43,6 @@
   # Individual tools we test.
   clang-apply-replacements
   clang-change-namespace
-  clangd
   clang-doc
   clang-include-fixer
   clang-move
@@ -53,10 +52,7 @@
   modularize
   pp-trace
 
-  # These individual tools have no tests, add them here to make them compile
-  # together with check-clang-tools, so that we won't break them in the future.
-  clangd-indexer
-  dexp
+  check-clangd
 
   # Unit tests
   ExtraToolsUnitTests
@@ -73,20 +69,38 @@
 )
 endif()
 
-set(llvm_utils
-  FileCheck count not
-  )
-
-foreach(t ${llvm_utils})
-  if(TARGET ${t})
-list(APPEND CLANG_TOOLS_TEST_DEPS ${t})
-  endif()
-endforeach()
-
+macro(add_llvm_utils_deps deps)
+  set(llvm_utils
+  FileCheck count not
+  )
+  foreach(t ${llvm_utils})
+if(TARGET ${t})
+  list(APPEND "${deps}" ${t})
+endif()
+  endforeach()
+endmacro()
+
+add_llvm_utils_deps(CLANG_TOOLS_TEST_DEPS)
 add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression tests"
   ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   ARGS ${CLANG_TOOLS_TEST_EXTRA_ARGS}
   )
 
 set_target_properties(check-clang-tools PROPERTIES FOLDER "Clang extra tools' tests")
+
+# Setup an individual test for building and testing clangd-only stuff.
+set(CLANGD_TEST_DEPS
+  clangd
+  ClangdTests
+  # clangd-related tools which don't have tests, add them to the test to make
+  # sure we don't introduce new changes that break their compilations.
+  clangd-indexer
+  dexp
+)
+add_llvm_utils_deps(CLANGD_TEST_DEPS)
+add_lit_testsuite(check-clangd "Running the Clangd regression tests"
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/clangd;${CMAKE_CURRENT_BINARY_DIR}/clangd
+  DEPENDS ${CLANGD_TEST_DEPS}
+)
+set_target_properties(check-clangd PROPERTIES FOLDER "Clangd tests")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: test/CMakeLists.txt:76
 
-set(llvm_utils
-  FileCheck count not
-  )
-
-foreach(t ${llvm_utils})
-  if(TARGET ${t})
-list(APPEND CLANG_TOOLS_TEST_DEPS ${t})
-  endif()
-endforeach()
-
+macro(add_llvm_utils_deps deps)
+  set(llvm_utils

sammccall wrote:
> Conversely, I'd consider just expanding these out into each of the dep lists 
> rather than keeping two levels of indirection here.
> 
> 
Ah, I missed this comment. It seems that there is no elegant way to expand 
these to `CLANG_TOOLS_TEST_DEPS` and `CLANGD_TOOLS_TEST_DEPS`, we have to 
repeat the code twice. So I'd prefer using macro to eliminate code duplication.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52710



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

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: test/CMakeLists.txt:76
 
-set(llvm_utils
-  FileCheck count not
-  )
-
-foreach(t ${llvm_utils})
-  if(TARGET ${t})
-list(APPEND CLANG_TOOLS_TEST_DEPS ${t})
-  endif()
-endforeach()
-
+macro(add_llvm_utils_deps deps)
+  set(llvm_utils

hokein wrote:
> sammccall wrote:
> > Conversely, I'd consider just expanding these out into each of the dep 
> > lists rather than keeping two levels of indirection here.
> > 
> > 
> Ah, I missed this comment. It seems that there is no elegant way to expand 
> these to `CLANG_TOOLS_TEST_DEPS` and `CLANGD_TOOLS_TEST_DEPS`, we have to 
> repeat the code twice. So I'd prefer using macro to eliminate code 
> duplication.
Right, that was my suggestion: instead of 13 lines of macros, to add "FileCheck 
count not" twice, we just write it twice. It won't change often, and if it does 
there's no reason to assume you'd want them in sync.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52710



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

[PATCH] D52713: Move llvm util dependencies from clang-tools-extra to add_lit_target.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: llvm-commits, mgorny.

Address fixme in r301762. And would simplify the cmake file in
clang-tools-extra.


Repository:
  rL LLVM

https://reviews.llvm.org/D52713

Files:
  cmake/modules/AddLLVM.cmake


Index: cmake/modules/AddLLVM.cmake
===
--- cmake/modules/AddLLVM.cmake
+++ cmake/modules/AddLLVM.cmake
@@ -1367,6 +1367,17 @@
   COMMAND ${CMAKE_COMMAND} -E echo "${target} does nothing, no tools 
built.")
 message(STATUS "${target} does nothing.")
   endif()
+
+  # Add lit test dependencies.
+  set(llvm_utils_deps
+FileCheck count not
+  )
+  foreach(dep ${llvm_utils_deps})
+if (TARGET ${dep})
+  add_dependencies(${target} ${dep})
+endif()
+  endforeach()
+
   if (ARG_DEPENDS)
 add_dependencies(${target} ${ARG_DEPENDS})
   endif()


Index: cmake/modules/AddLLVM.cmake
===
--- cmake/modules/AddLLVM.cmake
+++ cmake/modules/AddLLVM.cmake
@@ -1367,6 +1367,17 @@
   COMMAND ${CMAKE_COMMAND} -E echo "${target} does nothing, no tools built.")
 message(STATUS "${target} does nothing.")
   endif()
+
+  # Add lit test dependencies.
+  set(llvm_utils_deps
+FileCheck count not
+  )
+  foreach(dep ${llvm_utils_deps})
+if (TARGET ${dep})
+  add_dependencies(${target} ${dep})
+endif()
+  endforeach()
+
   if (ARG_DEPENDS)
 add_dependencies(${target} ${ARG_DEPENDS})
   endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52714: Build clang-headers when building clang tools.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, ioeric, ilya-biryukov, mgorny.

clang tools require clang headers to work on real project, e.g. when we
build clangd via `ninja clangd`, we expect the binary can run on
real-world project (without running another command `ninja clang-headers`).


Repository:
  rC Clang

https://reviews.llvm.org/D52714

Files:
  cmake/modules/AddClang.cmake


Index: cmake/modules/AddClang.cmake
===
--- cmake/modules/AddClang.cmake
+++ cmake/modules/AddClang.cmake
@@ -131,6 +131,7 @@
   endif()
 
   add_clang_executable(${name} ${ARGN})
+  add_dependencies(${name} clang-headers)
 
   if (CLANG_BUILD_TOOLS)
 if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR


Index: cmake/modules/AddClang.cmake
===
--- cmake/modules/AddClang.cmake
+++ cmake/modules/AddClang.cmake
@@ -131,6 +131,7 @@
   endif()
 
   add_clang_executable(${name} ${ARGN})
+  add_dependencies(${name} clang-headers)
 
   if (CLANG_BUILD_TOOLS)
 if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

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

Remove llvm util dependencies (we move it to add_lit_target().


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52710

Files:
  test/CMakeLists.txt


Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -43,7 +43,6 @@
   # Individual tools we test.
   clang-apply-replacements
   clang-change-namespace
-  clangd
   clang-doc
   clang-include-fixer
   clang-move
@@ -53,10 +52,7 @@
   modularize
   pp-trace
 
-  # These individual tools have no tests, add them here to make them compile
-  # together with check-clang-tools, so that we won't break them in the future.
-  clangd-indexer
-  dexp
+  check-clangd
 
   # Unit tests
   ExtraToolsUnitTests
@@ -73,20 +69,25 @@
 )
 endif()
 
-set(llvm_utils
-  FileCheck count not
-  )
-
-foreach(t ${llvm_utils})
-  if(TARGET ${t})
-list(APPEND CLANG_TOOLS_TEST_DEPS ${t})
-  endif()
-endforeach()
-
 add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression 
tests"
   ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   ARGS ${CLANG_TOOLS_TEST_EXTRA_ARGS}
   )
 
 set_target_properties(check-clang-tools PROPERTIES FOLDER "Clang extra tools' 
tests")
+
+# Setup an individual test for building and testing clangd-only stuff.
+set(CLANGD_TEST_DEPS
+  clangd
+  ClangdTests
+  # clangd-related tools which don't have tests, add them to the test to make
+  # sure we don't introduce new changes that break their compilations.
+  clangd-indexer
+  dexp
+)
+add_lit_testsuite(check-clangd "Running the Clangd regression tests"
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/clangd;${CMAKE_CURRENT_BINARY_DIR}/clangd
+  DEPENDS ${CLANGD_TEST_DEPS}
+)
+set_target_properties(check-clangd PROPERTIES FOLDER "Clangd tests")


Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -43,7 +43,6 @@
   # Individual tools we test.
   clang-apply-replacements
   clang-change-namespace
-  clangd
   clang-doc
   clang-include-fixer
   clang-move
@@ -53,10 +52,7 @@
   modularize
   pp-trace
 
-  # These individual tools have no tests, add them here to make them compile
-  # together with check-clang-tools, so that we won't break them in the future.
-  clangd-indexer
-  dexp
+  check-clangd
 
   # Unit tests
   ExtraToolsUnitTests
@@ -73,20 +69,25 @@
 )
 endif()
 
-set(llvm_utils
-  FileCheck count not
-  )
-
-foreach(t ${llvm_utils})
-  if(TARGET ${t})
-list(APPEND CLANG_TOOLS_TEST_DEPS ${t})
-  endif()
-endforeach()
-
 add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression tests"
   ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   ARGS ${CLANG_TOOLS_TEST_EXTRA_ARGS}
   )
 
 set_target_properties(check-clang-tools PROPERTIES FOLDER "Clang extra tools' tests")
+
+# Setup an individual test for building and testing clangd-only stuff.
+set(CLANGD_TEST_DEPS
+  clangd
+  ClangdTests
+  # clangd-related tools which don't have tests, add them to the test to make
+  # sure we don't introduce new changes that break their compilations.
+  clangd-indexer
+  dexp
+)
+add_lit_testsuite(check-clangd "Running the Clangd regression tests"
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/clangd;${CMAKE_CURRENT_BINARY_DIR}/clangd
+  DEPENDS ${CLANGD_TEST_DEPS}
+)
+set_target_properties(check-clangd PROPERTIES FOLDER "Clangd tests")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Sam McCall via Phabricator via cfe-commits

sammccall accepted this revision.
sammccall added a comment.

Awesome, thanks!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52710



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

[PATCH] D52715: [clangd] Dex iterator printer shows query structure, not iterator state.

2018-10-01 Thread 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.

This makes it suitable for logging (which immediately found a bug, to
be fixed in the next patch...)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52715

Files:
  clangd/index/dex/Dex.cpp
  clangd/index/dex/Iterator.cpp
  clangd/index/dex/PostingList.cpp
  clangd/index/dex/PostingList.h
  clangd/index/dex/Token.h
  unittests/clangd/DexTests.cpp

Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -226,18 +226,17 @@
 }
 
 TEST(DexIterators, StringRepresentation) {
-  const PostingList L0({4, 7, 8, 20, 42, 100});
-  const PostingList L1({1, 3, 5, 8, 9});
-  const PostingList L2({1, 5, 7, 9});
-  const PostingList L3({0, 5});
-  const PostingList L4({0, 1, 5});
-
-  EXPECT_EQ(llvm::to_string(*(L0.iterator())), "[4 ...]");
-  auto It = L0.iterator();
-  It->advanceTo(19);
-  EXPECT_EQ(llvm::to_string(*It), "[... 20 ...]");
-  It->advanceTo(9000);
-  EXPECT_EQ(llvm::to_string(*It), "[... END]");
+  const PostingList L1({1, 3, 5});
+  const PostingList L2({1, 7, 9});
+  auto I1 = L1.iterator();
+  Token Tok(Token::Kind::Trigram, "L2");
+  auto I2 = L1.iterator(&Tok);
+
+  EXPECT_EQ(llvm::to_string(*I1), "[1 3 5]");
+  EXPECT_EQ(llvm::to_string(*I2), "T=L2");
+
+  auto Tree = createLimit(createAnd(move(I1), move(I2)), 10);
+  EXPECT_EQ(llvm::to_string(*Tree), "(LIMIT 10 (& [1 3 5] T=L2))");
 }
 
 TEST(DexIterators, Limit) {
Index: clangd/index/dex/Token.h
===
--- clangd/index/dex/Token.h
+++ clangd/index/dex/Token.h
@@ -82,6 +82,20 @@
   Kind TokenKind;
 
   friend llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Token &T) {
+switch (T.TokenKind) {
+case Kind::Trigram:
+  OS << "T=";
+  break;
+case Kind::Scope:
+  OS << "S=";
+  break;
+case Kind::ProximityURI:
+  OS << "U=";
+  break;
+case Kind::Sentinel:
+  OS << "?=";
+  break;
+}
 return OS << T.Data;
   }
 
Index: clangd/index/dex/PostingList.h
===
--- clangd/index/dex/PostingList.h
+++ clangd/index/dex/PostingList.h
@@ -33,8 +33,7 @@
 namespace clang {
 namespace clangd {
 namespace dex {
-
-class Iterator;
+struct Token;
 
 /// NOTE: This is an implementation detail.
 ///
@@ -64,7 +63,8 @@
 
   /// Constructs DocumentIterator over given posting list. DocumentIterator will
   /// go through the chunks and decompress them on-the-fly when necessary.
-  std::unique_ptr iterator() const;
+  /// If given, Tok is only used for the string representation.
+  std::unique_ptr iterator(const Token *Tok = nullptr) const;
 
   /// Returns in-memory size of external storage.
   size_t bytes() const { return Chunks.capacity() * sizeof(Chunk); }
Index: clangd/index/dex/PostingList.cpp
===
--- clangd/index/dex/PostingList.cpp
+++ clangd/index/dex/PostingList.cpp
@@ -9,6 +9,7 @@
 
 #include "PostingList.h"
 #include "Iterator.h"
+#include "Token.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MathExtras.h"
 
@@ -23,8 +24,8 @@
 /// them on-the-fly when the contents of chunk are to be seen.
 class ChunkIterator : public Iterator {
 public:
-  explicit ChunkIterator(llvm::ArrayRef Chunks)
-  : Chunks(Chunks), CurrentChunk(Chunks.begin()) {
+  explicit ChunkIterator(const Token *Tok, llvm::ArrayRef Chunks)
+  : Tok(Tok), Chunks(Chunks), CurrentChunk(Chunks.begin()) {
 if (!Chunks.empty()) {
   DecompressedChunk = CurrentChunk->decompress();
   CurrentID = DecompressedChunk.begin();
@@ -71,15 +72,16 @@
 
 private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
+if (Tok != nullptr)
+  return OS << *Tok;
 OS << '[';
-if (CurrentChunk != Chunks.begin() ||
-(CurrentID != DecompressedChunk.begin() && !DecompressedChunk.empty()))
-  OS << "... ";
-OS << (reachedEnd() ? "END" : std::to_string(*CurrentID));
-if (!reachedEnd() && CurrentID < DecompressedChunk.end() - 1)
-  OS << " ...";
-OS << ']';
-return OS;
+const char *Sep = "";
+for (const Chunk &C : Chunks)
+  for (const DocID Doc : C.decompress()) {
+OS << Sep << Doc;
+Sep = " ";
+  }
+return OS << ']';
   }
 
   /// If the cursor is at the end of a chunk, place it at the start of the next
@@ -110,6 +112,7 @@
 }
   }
 
+  const Token *Tok;
   llvm::ArrayRef Chunks;
   /// Iterator over chunks.
   /// If CurrentChunk is valid, then DecompressedChunk is
@@ -217,8 +220,8 @@
 PostingList::PostingList(llvm::ArrayRef Documents)
 : Chunks(encodeStream(Documents)) {}
 
-std::unique_ptr PostingList::iterator() const {
-  return llvm::make_

[clang-tools-extra] r343453 - [clangd] Query dex index using query-style trigrams, not identifier-style trigrams

2018-10-01 Thread Sam McCall via cfe-commits

Author: sammccall
Date: Mon Oct  1 03:42:51 2018
New Revision: 343453

URL: http://llvm.org/viewvc/llvm-project?rev=343453&view=rev
Log:
[clangd] Query dex index using query-style trigrams, not identifier-style 
trigrams

Modified:
clang-tools-extra/trunk/clangd/index/dex/Dex.cpp

Modified: clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Dex.cpp?rev=343453&r1=343452&r2=343453&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Dex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Dex.cpp Mon Oct  1 03:42:51 2018
@@ -148,7 +148,7 @@ bool Dex::fuzzyFind(const FuzzyFindReque
   bool More = false;
 
   std::vector> TopLevelChildren;
-  const auto TrigramTokens = generateIdentifierTrigrams(Req.Query);
+  const auto TrigramTokens = generateQueryTrigrams(Req.Query);
 
   // Generate query trigrams and construct AND iterator over all query
   // trigrams.


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

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

2018-10-01 Thread Jonas Toth via Phabricator via cfe-commits

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

- remove spurious change in fixithintutils


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclarationCheck.cpp
  clang-tidy/readability/IsolateDeclarationCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/utils/LexerUtils.cpp
  clang-tidy/utils/LexerUtils.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-declaration.rst
  test/clang-tidy/readability-isolate-declaration-cxx17.cpp
  test/clang-tidy/readability-isolate-declaration-fixing.cpp
  test/clang-tidy/readability-isolate-declaration.c
  test/clang-tidy/readability-isolate-declaration.cpp

Index: test/clang-tidy/readability-isolate-declaration.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-declaration.cpp
@@ -0,0 +1,412 @@
+// RUN: %check_clang_tidy %s readability-isolate-declaration %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+
+  auto int1 = 42, int2 = 0, int3 = 43;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: auto int1 = 42;
+  // CHECK-FIXES: {{^  }}auto int2 = 0;
+  // CHECK-FIXES: {{^  }}auto int3 = 43;
+
+  decltype(auto) ptr1 = &int1, ptr2 = &int1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: decltype(auto) ptr1 = &int1;
+  // CHECK-FIXES: {{^  }}decltype(auto) ptr2 = &int1;
+
+  decltype(k) ptr3 = &int1, ptr4 = &int1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: decltype(k) ptr3 = &int1;
+  // CHECK-FIXES: {{^  }}decltype(k) ptr4 = &int1;
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = &i;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = &i;
+
+  int *(i_ptr) = nullptr, *((i_ptr2));
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int *(i_ptr) = nullptr;
+  // CHECK-FIXES: {{^  }}int *((i_ptr2));
+
+  float(*f_ptr)[42], (((f_value))) = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: float (*f_ptr)[42];
+  // CHECK-FIXES: {{^  }}float (((f_value))) = 42;
+
+  float(((*f_ptr2)))[42], ((*f_ptr3)), f_value2 = 42.f;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: float (((*f_ptr2)))[42];
+  // CHECK-FIXES: {{^  }}float ((*f_ptr3));
+  // CHECK-FIXES: {{^  }}float f_value2 = 42.f;
+
+  float(((*f_ptr4)))[42], *f_ptr5, ((f_value3));
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: float (((*f_ptr4)))[42];
+  // CHECK-FIXES: {{^  }}float *f_ptr5;
+  // CHECK-FIXES: {{^  }}float ((f_value3));
+
+  void(((*f2))(int)), (*g2)(int, float);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: void (((*f2))(int));
+  // CHECK-FIXES: {{^  }}void (*g2)(int, float);
+
+  float(*(*(*f_ptr6)))[42], (*f_ptr7);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: float (*(*(*f_ptr6)))[42];
+  // CHECK-FIXES: {{^  }}float (*f_ptr7);
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESS

[PATCH] D52705: First-pass at ARM hard/soft-float multilib driver (NOT WORKING YET)

2018-10-01 Thread Peter Smith via Phabricator via cfe-commits

peter.smith added a comment.

I think that you may be better off posting a RFC to llvm-dev to get discussion 
on the best approach here. It would also be good to step back a bit and 
consider the requirements, as it stands it looks like this might be a solution 
for just one particular multilib layout and we might be able to get some input 
on some other use cases that others may have. I personally would like to enable 
clang to be able to query a gcc installation to find out (and potentially 
cache) its multlib directories as I've been told for the Arm embedded toolchain 
that the directory structure is not fixed and may change at any point in the 
future.

Another unrelated thing that may be worth doing is to check for other places 
where the target might affect the code-generation independently of -mfloat-abi, 
I'm thinking in particular of builtin functions as this has changed a bit over 
the past few years.




Comment at: lib/Driver/ToolChains/Gnu.cpp:1441
 
+static bool findArmEABIMultilibs(const Driver &D,
+ const llvm::Triple &TargetTriple,

When I saw EABIMultilibs I first thought of multilib support for arm-none-eabi 
, I think that this may be worth renaming to something like ArmLinuxGNUEABI as 
this could be specific to Linux Distributions. 



Comment at: lib/Driver/ToolChains/Gnu.cpp:1451
+  bool DefaultHardFloat = TargetTriple.isHardFloatEABI();
+  llvm::Triple AltTriple(DefaultHardFloat ?
+ TargetTriple.getSoftFloatArchVariant() :

AltTriple doesn't seem to be used anywhere in the rest of the function?



Comment at: lib/Driver/ToolChains/Gnu.cpp:1454
+ TargetTriple.getHardFloatArchVariant());
+  // We assume the Debian libdir/includedir arrangement for armel/armhf,
+  // since Debian and its descendents are apparently the only common Linux

Another place where this is used, and clang can't yet handle, is the GCC 
Embedded toolchain arm-none-eabi which includes both hard and soft float abi 
libraries. These have a different structure to the one presented here though.   



Comment at: lib/Driver/ToolChains/Gnu.cpp:1458
+  // SuSE and RedHat seem to stick with hard-float only and no libdir suffix.
+  // TODO: this (and a lot of other code here) could be generalized via the
+  // output of "gcc  --print-multi-os-dir".

I definitely agree here. It seems to me to be quite fragile to hard-code in 
directory structures of gcc toolchains or linux distributions that can, and do, 
vary over time.


Repository:
  rC Clang

https://reviews.llvm.org/D52705



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

[PATCH] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager

2018-10-01 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware added a comment.

Since modifications of the infrastructure are always error-prone I tried to 
generate tests to have full coverage for smaller numbers. However for full 
coverage I need type `signed char` and `unsigned char`, but these tests failed 
because of this bug: Bug 39138 . 
If that bug is fixed we can have full coverage for both `signed` and `unsigned 
char` types which is more than enough. Of course the generated test should not 
be part of the standard test suite because of its execution time.


https://reviews.llvm.org/D49074



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

[PATCH] D52225: [clang] Implement Override Suggestions in Sema.

2018-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 167700.
kadircet marked 6 inline comments as done.
kadircet added a comment.

- Address comments.


Repository:
  rC Clang

https://reviews.llvm.org/D52225

Files:
  include/clang/Sema/CodeCompleteConsumer.h
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/overrides.cpp

Index: test/CodeCompletion/overrides.cpp
===
--- /dev/null
+++ test/CodeCompletion/overrides.cpp
@@ -0,0 +1,30 @@
+class A {
+ public:
+  virtual void vfunc(bool param);
+  virtual void vfunc(bool param, int p);
+  void func(bool param);
+};
+class B : public A {
+virtual int ttt(bool param) const;
+void vfunc(bool param, int p) override;
+};
+class C : public B {
+ public:
+  void vfunc(bool param) override;
+  void
+};
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:3 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1: COMPLETION: Pattern : int ttt(bool param) const override{{$}}
+// CHECK-CC1: COMPLETION: Pattern : void vfunc(bool param, int p) override{{$}}
+// CHECK-CC1-NOT: COMPLETION: Pattern : void vfunc(bool param) override{{$}}
+//
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:5 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// CHECK-CC2: COMPLETION: Pattern : void vfunc(bool param, int p) override{{$}}
+// CHECK-CC2-NOT: COMPLETION: Pattern : int ttt(bool param) const override{{$}}
+// CHECK-CC2-NOT: COMPLETION: Pattern : void vfunc(bool param) override{{$}}
+//
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:8 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC3-NOT: COMPLETION: Pattern : int ttt(bool param) const override{{$}}
+// CHECK-CC3-NOT: COMPLETION: Pattern : void vfunc(bool param, int p) override{{$}}
+// CHECK-CC3-NOT: COMPLETION: Pattern : void vfunc(bool param) override{{$}}
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -1598,6 +1598,69 @@
   Results.AddResult(CodeCompletionResult(Builder.TakeString()));
 }
 
+static void AddOverrideResults(ResultBuilder &Results,
+   const CodeCompletionContext &CCContext,
+   CodeCompletionBuilder &Builder) {
+  Sema &S = Results.getSema();
+  const auto *CR = llvm::dyn_cast(S.CurContext);
+  // If not inside a class/struct/union return empty.
+  if (!CR)
+return;
+  // First store overrides within current class.
+  // These are stored by name to make querying fast in the later step.
+  llvm::StringMap> Overrides;
+  for (auto *Method : CR->methods()) {
+if (!Method->isVirtual() || !Method->getIdentifier())
+  continue;
+Overrides[Method->getName()].push_back(Method);
+  }
+
+  for (const auto &Base : CR->bases()) {
+const auto *BR = Base.getType().getTypePtr()->getAsCXXRecordDecl();
+if (!BR)
+  continue;
+for (auto *Method : BR->methods()) {
+  if (!Method->isVirtual() || !Method->getIdentifier())
+continue;
+  const auto it = Overrides.find(Method->getName());
+  bool IsOverriden = false;
+  if (it != Overrides.end()) {
+for (auto *MD : it->second) {
+  // If the method in current body is not an overload of this virtual
+  // function, then it overrides this one.
+  if (!S.IsOverload(MD, Method, false)) {
+IsOverriden = true;
+break;
+  }
+}
+  }
+  if (!IsOverriden) {
+// Generates a new CodeCompletionResult by taking this function and
+// converting it into an override declaration.
+std::string OverrideSignature;
+llvm::raw_string_ostream OS(OverrideSignature);
+CodeCompletionResult CCR(Method, 0);
+PrintingPolicy Policy =
+getCompletionPrintingPolicy(S.getASTContext(), S.getPreprocessor());
+auto *CCS = CCR.createCodeCompletionStringForDecl(
+S.getPreprocessor(), S.getASTContext(), Builder,
+/*IncludeBriefComments=*/false, CCContext, Policy);
+for (const auto &C : *CCS) {
+  if (C.Kind == CodeCompletionString::CK_Optional)
+continue;
+  OS << C.Text;
+  if (C.Kind == CodeCompletionString::CK_ResultType)
+OS << ' ';
+}
+OS << " override";
+Builder.AddTypedTextChunk(Builder.getAllocator().CopyString(OS.str()));
+Results.AddResult(CodeCompletionResult(Builder.TakeString(), Method,
+   CCP_CodePattern));
+  }
+}
+  }
+}
+
 /// Add language constructs that show up for "ordinary" names.
 static void AddOrdinaryNameResults(Sema::ParserCompletionContext CCC,
Scope *S,
@@ -1706,6 +1769,9 @@
 if (IsNotInheritanceScope && Results.includeCodePatterns())
   Builder.AddChunk(CodeCompletionString::CK_Colon);
 Results.AddRe

[PATCH] D52225: [clang] Implement Override Suggestions in Sema.

2018-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:1639
+llvm::raw_string_ostream OS(OverrideSignature);
+CodeCompletionResult CCR(Method, 0);
+PrintingPolicy Policy =

ioeric wrote:
> Could you add comments explaining what the following code does? The original 
> code in clangd only has `Results.emplace_back(Method, 0);`, and it's 
> non-trivial what the new code here is for.
Yeah you are right added some comments, previously this was handled on 
CodeCompletionBuilder in clangd which converted the completion item into a 
override declaration to be inserted.


Repository:
  rC Clang

https://reviews.llvm.org/D52225



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

[PATCH] D52225: [clang] Implement Override Suggestions in Sema.

2018-10-01 Thread Eric Liu via Phabricator via cfe-commits

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

lg




Comment at: lib/Sema/SemaCodeComplete.cpp:1639
+llvm::raw_string_ostream OS(OverrideSignature);
+CodeCompletionResult CCR(Method, 0);
+PrintingPolicy Policy =

kadircet wrote:
> ioeric wrote:
> > Could you add comments explaining what the following code does? The 
> > original code in clangd only has `Results.emplace_back(Method, 0);`, and 
> > it's non-trivial what the new code here is for.
> Yeah you are right added some comments, previously this was handled on 
> CodeCompletionBuilder in clangd which converted the completion item into a 
> override declaration to be inserted.
Thanks! Could you elaborate on why you need to explicitly handle `CCR` and 
`CCS`, i.e. why was `Results.emplace_back(Method, 0);` not enough? I guess it's 
to avoid inserting return type when it's already typed, but it's not obvious 
from the code.



Comment at: test/CodeCompletion/overrides.cpp:17
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:3 %s -o - | 
FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1: COMPLETION: Pattern : int ttt(bool param) const override{{$}}

nit: add `// completion ^void`



Comment at: test/CodeCompletion/overrides.cpp:22
+//
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:5 %s -o - | 
FileCheck -check-prefix=CHECK-CC2 %s
+// CHECK-CC2: COMPLETION: Pattern : void vfunc(bool param, int p) override{{$}}

add `// completion at vo^id`



Comment at: test/CodeCompletion/overrides.cpp:27
+//
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:8 %s -o - | 
FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC3-NOT: COMPLETION: Pattern : int ttt(bool param) const override{{$}}

add `// completion at void ^`


Repository:
  rC Clang

https://reviews.llvm.org/D52225



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

r343457 - [CodeComplete] #include completion treats -I as non-system (require header-like extension).

2018-10-01 Thread Sam McCall via cfe-commits

Author: sammccall
Date: Mon Oct  1 04:56:42 2018
New Revision: 343457

URL: http://llvm.org/viewvc/llvm-project?rev=343457&view=rev
Log:
[CodeComplete] #include completion treats -I as non-system (require header-like 
extension).

Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/CodeCompletion/included-files.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=343457&r1=343456&r2=343457&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Mon Oct  1 04:56:42 2018
@@ -8098,7 +8098,7 @@ void Sema::CodeCompleteIncludedFile(llvm
   AddFilesFromDirLookup(D, false);
   }
   for (const auto &D : make_range(S.angled_dir_begin(), S.angled_dir_end()))
-AddFilesFromDirLookup(D, true);
+AddFilesFromDirLookup(D, false);
   for (const auto &D : make_range(S.system_dir_begin(), S.system_dir_end()))
 AddFilesFromDirLookup(D, true);
 

Modified: cfe/trunk/test/CodeCompletion/included-files.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/included-files.cpp?rev=343457&r1=343456&r2=343457&view=diff
==
--- cfe/trunk/test/CodeCompletion/included-files.cpp (original)
+++ cfe/trunk/test/CodeCompletion/included-files.cpp Mon Oct  1 04:56:42 2018
@@ -15,15 +15,21 @@
 // CHECK-2: foosys.h"
 // CHECK-2-NOT: foosys"
 
-// Angled string showes all files, but only in system dirs.
+// Angled shows headers from system dirs.
 #include 
 // RUN: %clang -fsyntax-only -isystem %t/a -Xclang 
-code-completion-at=%t/main.cc:19:13 %t/main.cc | FileCheck 
-check-prefix=CHECK-3 %s
 // CHECK-3-NOT: foo.cc>
 // CHECK-3-NOT: foo.h>
 // CHECK-3: foosys>
 
+// With -I rather than -isystem, the header extension is required.
+#include 
+// RUN: %clang -fsyntax-only -I %t/a -Xclang 
-code-completion-at=%t/main.cc:26:13 %t/main.cc | FileCheck 
-check-prefix=CHECK-4 %s
+// CHECK-4-NOT: foo.cc>
+// CHECK-4-NOT: foo.h>
+// CHECK-4-NOT: foosys>
+
 // Backslash handling.
 #include "a\foosys"
-// RUN: %clang -fsyntax-only -isystem %t/a -Xclang 
-code-completion-at=%t/main.cc:26:13 %t/main.cc -fms-compatibility | FileCheck 
-check-prefix=CHECK-4 %s
-// CHECK-4: foosys.h"
-
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang 
-code-completion-at=%t/main.cc:33:13 %t/main.cc -fms-compatibility | FileCheck 
-check-prefix=CHECK-5 %s
+// CHECK-5: foosys.h"


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

[PATCH] D52691: [clang-tidy] NFC use CHECK-NOTES in tests for performance-move-constructor-init

2018-10-01 Thread Alexander Kornienko via Phabricator via cfe-commits

alexfh added inline comments.



Comment at: test/clang-tidy/performance-move-constructor-init.cpp:115-117
+  // CHECK-NOTES: 7:1: note: FIX-IT applied suggested code changes
+  // CHECK-NOTES: 113:28: note: FIX-IT applied suggested code changes
+  // CHECK-NOTES: 113:29: note: FIX-IT applied suggested code changes

This is an example of not very useful CHECK-NOTES patterns.  The corresponding 
notes are issued in Clang and there's not much value in testing them here. They 
also duplicate the CHECK-FIXES pattern below. Should the script filter these 
notes out, maybe?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52691



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

[PATCH] D52690: [clang-tidy] NFC use CHECK-NOTES in tests for misc-misplaced-const

2018-10-01 Thread Alexander Kornienko via Phabricator via cfe-commits

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



Comment at: test/clang-tidy/misc-misplaced-const.c:18
+  // CHECK-NOTES: :[[@LINE-1]]:12: warning: 'i3' declared with a 
const-qualified typedef type; results in the type being 'int *const' instead of 
'const int *'
+  // CHECK-NOTES: :[[@LINE-14]]:14: note: typedef declared here
 

These notes are also just marginally useful and make it harder to change the 
test. I wonder whether an absolute line number would make more sense here. Or 
maybe just add a test for one of the notes and leave out the rest (and keep 
CHECK-MESSAGES)?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52690



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

[PATCH] D52686: [clang-tidy] NFC use CHECK-NOTES in test for cppgoreguidelines-avoid-goto

2018-10-01 Thread Alexander Kornienko via Phabricator via cfe-commits

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

LG


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52686



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

[PATCH] D52714: Build clang-headers when building clang tools.

2018-10-01 Thread Sam McCall via Phabricator via cfe-commits

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

> when we build clangd via ninja clangd, we expect the binary can run on 
> real-world project

FWIW, I don't particularly have this expectation - I expect to be able to run 
the tests, and install the built binary.

But in practice this doesn't seem to slow things down much.


Repository:
  rC Clang

https://reviews.llvm.org/D52714



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

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a subscriber: jsji.

This also fixes a crash of code completion on `#include "./^"`.


Repository:
  rC Clang

https://reviews.llvm.org/D52721

Files:
  lib/Lex/PPDirectives.cpp
  test/Preprocessor/include-nonalpha-no-crash.c


Index: test/Preprocessor/include-nonalpha-no-crash.c
===
--- /dev/null
+++ test/Preprocessor/include-nonalpha-no-crash.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "./" // expected-error {{'./' file not found}}
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1889,27 +1889,29 @@
   // characters
   StringRef OriginalFilename = Filename;
   if (!File) {
-while (!isAlphanumeric(Filename.front())) {
+while (!Filename.empty() && !isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();
 }
-while (!isAlphanumeric(Filename.back())) {
+while (!Filename.empty() && !isAlphanumeric(Filename.back())) {
   Filename = Filename.drop_back();
 }
 
-File = LookupFile(
-FilenameLoc,
-LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
-LookupFrom, LookupFromFile, CurDir,
-Callbacks ? &SearchPath : nullptr,
-Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
-if (File) {
-  SourceRange Range(FilenameTok.getLocation(), CharEnd);
-  auto Hint = isAngled ? FixItHint::CreateReplacement(
- Range, "<" + Filename.str() + ">")
-   : FixItHint::CreateReplacement(
- Range, "\"" + Filename.str() + "\"");
-  Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal)
-  << OriginalFilename << Filename << Hint;
+if (!Filename.empty()) {
+  File = LookupFile(
+  FilenameLoc,
+  LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, 
isAngled,
+  LookupFrom, LookupFromFile, CurDir,
+  Callbacks ? &SearchPath : nullptr,
+  Callbacks ? &RelativePath : nullptr, &SuggestedModule, 
&IsMapped);
+  if (File) {
+SourceRange Range(FilenameTok.getLocation(), CharEnd);
+auto Hint = isAngled ? FixItHint::CreateReplacement(
+  Range, "<" + Filename.str() + ">")
+: FixItHint::CreateReplacement(
+  Range, "\"" + Filename.str() + "\"");
+Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal)
+<< OriginalFilename << Filename << Hint;
+  }
 }
   }
 
@@ -1920,6 +1922,9 @@
 }
   }
 
+  if (Filename.empty())
+return;
+
   if (usingPCHWithThroughHeader() && SkippingUntilPCHThroughHeader) {
 if (isPCHThroughHeader(File))
   SkippingUntilPCHThroughHeader = false;


Index: test/Preprocessor/include-nonalpha-no-crash.c
===
--- /dev/null
+++ test/Preprocessor/include-nonalpha-no-crash.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "./" // expected-error {{'./' file not found}}
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1889,27 +1889,29 @@
   // characters
   StringRef OriginalFilename = Filename;
   if (!File) {
-while (!isAlphanumeric(Filename.front())) {
+while (!Filename.empty() && !isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();
 }
-while (!isAlphanumeric(Filename.back())) {
+while (!Filename.empty() && !isAlphanumeric(Filename.back())) {
   Filename = Filename.drop_back();
 }
 
-File = LookupFile(
-FilenameLoc,
-LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
-LookupFrom, LookupFromFile, CurDir,
-Callbacks ? &SearchPath : nullptr,
-Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
-if (File) {
-  SourceRange Range(FilenameTok.getLocation(), CharEnd);
-  auto Hint = isAngled ? FixItHint::CreateReplacement(
- Range, "<" + Filename.str() + ">")
-   : FixItHint::CreateReplacement(
- Range, "\"" + Filename.str() + "\"");
-  Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal)
-  << OriginalFilename << Filename << Hint;
+if (!Filename.empty()) {
+  File = LookupFile(
+  FilenameLoc,
+

[PATCH] D52684: [clang-tidy] NFC refactor lexer-utils slightly to be easier to use

2018-10-01 Thread Alexander Kornienko via Phabricator via cfe-commits

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

So far the change doesn't look really helpful. Are you going to use the 
function from code where no `ASTContext` is available? Can you give an example?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52684



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

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

2018-10-01 Thread Alexander Kornienko via Phabricator via cfe-commits

alexfh added inline comments.



Comment at: clang-tidy/tool/clang-tidy-diff.py:123-130
   if args.fix:
 command.append('-fix')
   if args.checks != '':
 command.append('-checks=' + quote + args.checks + quote)
   if args.quiet:
 command.append('-quiet')
   if args.build_path is not None:

janosimas wrote:
> janosimas wrote:
> > alexfh wrote:
> > > If we make the script leave out the `--` flag, we should stop forwarding 
> > > these flags and the `extra_arg(_before)?` below. Otherwise it's too 
> > > confusing (should one place -fix before `--` or after? what about 
> > > `-warnings-as-errors`?).
> > > 
> > > Please also update the usage example at the top.
> > What about keep the current `--` behavior and add a new flag 
> > `-extra-tidy-flags` ? 
> `-extra-tidy-arg` to maintain consistency.
What's the benefit of `-extra-tidy-arg` compared to pass-by arguments?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49864



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

r343459 - Build clang-headers when building clang tools.

2018-10-01 Thread Haojian Wu via cfe-commits

Author: hokein
Date: Mon Oct  1 05:16:38 2018
New Revision: 343459

URL: http://llvm.org/viewvc/llvm-project?rev=343459&view=rev
Log:
Build clang-headers when building clang tools.

Summary:
clang tools require clang headers to work on real project, e.g. when we
build clangd via `ninja clangd`, we expect the binary can run on
real-world project (without running another command `ninja clang-headers`).

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: mgorny, ilya-biryukov, ioeric, kadircet, cfe-commits

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

Modified:
cfe/trunk/cmake/modules/AddClang.cmake

Modified: cfe/trunk/cmake/modules/AddClang.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/modules/AddClang.cmake?rev=343459&r1=343458&r2=343459&view=diff
==
--- cfe/trunk/cmake/modules/AddClang.cmake (original)
+++ cfe/trunk/cmake/modules/AddClang.cmake Mon Oct  1 05:16:38 2018
@@ -131,6 +131,7 @@ macro(add_clang_tool name)
   endif()
 
   add_clang_executable(${name} ${ARGN})
+  add_dependencies(${name} clang-headers)
 
   if (CLANG_BUILD_TOOLS)
 if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR


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

[PATCH] D52714: Build clang-headers when building clang tools.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343459: Build clang-headers when building clang tools. 
(authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D52714

Files:
  cfe/trunk/cmake/modules/AddClang.cmake


Index: cfe/trunk/cmake/modules/AddClang.cmake
===
--- cfe/trunk/cmake/modules/AddClang.cmake
+++ cfe/trunk/cmake/modules/AddClang.cmake
@@ -131,6 +131,7 @@
   endif()
 
   add_clang_executable(${name} ${ARGN})
+  add_dependencies(${name} clang-headers)
 
   if (CLANG_BUILD_TOOLS)
 if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR


Index: cfe/trunk/cmake/modules/AddClang.cmake
===
--- cfe/trunk/cmake/modules/AddClang.cmake
+++ cfe/trunk/cmake/modules/AddClang.cmake
@@ -131,6 +131,7 @@
   endif()
 
   add_clang_executable(${name} ${ARGN})
+  add_dependencies(${name} clang-headers)
 
   if (CLANG_BUILD_TOOLS)
 if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: lib/Lex/PPDirectives.cpp:1891
   StringRef OriginalFilename = Filename;
   if (!File) {
+while (!Filename.empty() && !isAlphanumeric(Filename.front())) {

everything in this block is guarded by !Filename.empty().
Just add it to the if condition?



Comment at: lib/Lex/PPDirectives.cpp:1925
 
+  if (Filename.empty())
+return;

this is mysterious - what does it solve? is it the right place to handle this 
problem?

(You allude to a code complete crash - can you explain?)


Repository:
  rC Clang

https://reviews.llvm.org/D52721



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

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Kristina Brooks via Phabricator via cfe-commits

kristina added a reviewer: kristina.
kristina added a comment.

Able to reproduce easily:

  clang-8: /SourceCache/llvm-trunk-8.0/include/llvm/ADT/StringRef.h:143: char 
llvm::StringRef::front() const: Assertion `!empty()' failed.

Will try it with patches applied in a moment.


Repository:
  rC Clang

https://reviews.llvm.org/D52721



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

[PATCH] D52620: Added Support for StatOnly Files in VFS.

2018-10-01 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

> I would call it a requirement instead of an assumption. The replay must be 
> exactly the same (even the file stats and reads). If Clang reads the file in 
> replay which was only stat()ed during compilation, it indicates 
> non-determinism or something wrong (in clang or FS).
>  We currently deal with such files by adding empty buffers for them based on 
> this assumption/requirement only.

Does adding a flag to clang to skip the module inputs checks seems 
feasible/reasonable? +1 to Sam's comments, the mode for FS that has the file 
sizes, but not the file contents seems too specific for the use-case at hand, 
does not fit well into the design.


Repository:
  rC Clang

https://reviews.llvm.org/D52620



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

[PATCH] D52225: [clang] Implement Override Suggestions in Sema.

2018-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 167708.
kadircet marked 4 inline comments as done.
kadircet added a comment.

- Fix a bug and address comments.


Repository:
  rC Clang

https://reviews.llvm.org/D52225

Files:
  include/clang/Sema/CodeCompleteConsumer.h
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/overrides.cpp

Index: test/CodeCompletion/overrides.cpp
===
--- /dev/null
+++ test/CodeCompletion/overrides.cpp
@@ -0,0 +1,33 @@
+class A {
+ public:
+  virtual void vfunc(bool param);
+  virtual void vfunc(bool param, int p);
+  void func(bool param);
+};
+class B : public A {
+virtual int ttt(bool param, int x = 3) const;
+void vfunc(bool param, int p) override;
+};
+class C : public B {
+ public:
+  void vfunc(bool param) override;
+  void
+};
+
+// Runs completion at ^void.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:3 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1: COMPLETION: Pattern : int ttt(bool param, int x = 3) const override{{$}}
+// CHECK-CC1: COMPLETION: Pattern : void vfunc(bool param, int p) override{{$}}
+// CHECK-CC1-NOT: COMPLETION: Pattern : void vfunc(bool param) override{{$}}
+//
+// Runs completion at vo^id.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:5 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// CHECK-CC2: COMPLETION: Pattern : void vfunc(bool param, int p) override{{$}}
+// CHECK-CC2-NOT: COMPLETION: Pattern : int ttt(bool param, int x = 3) const override{{$}}
+// CHECK-CC2-NOT: COMPLETION: Pattern : void vfunc(bool param) override{{$}}
+//
+// Runs completion at void ^.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:8 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC3-NOT: COMPLETION: Pattern : int ttt(bool param, int x = 3) const override{{$}}
+// CHECK-CC3-NOT: COMPLETION: Pattern : void vfunc(bool param, int p) override{{$}}
+// CHECK-CC3-NOT: COMPLETION: Pattern : void vfunc(bool param) override{{$}}
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -1598,6 +1598,78 @@
   Results.AddResult(CodeCompletionResult(Builder.TakeString()));
 }
 
+namespace {
+void printOverrideString(llvm::raw_ostream &OS, CodeCompletionString *CCS) {
+  for (const auto &C : *CCS) {
+if (C.Kind == CodeCompletionString::CK_Optional)
+  printOverrideString(OS, C.Optional);
+else
+  OS << C.Text;
+// Add a space after return type.
+if (C.Kind == CodeCompletionString::CK_ResultType)
+  OS << ' ';
+  }
+}
+} // namespace
+
+static void AddOverrideResults(ResultBuilder &Results,
+   const CodeCompletionContext &CCContext,
+   CodeCompletionBuilder &Builder) {
+  Sema &S = Results.getSema();
+  const auto *CR = llvm::dyn_cast(S.CurContext);
+  // If not inside a class/struct/union return empty.
+  if (!CR)
+return;
+  // First store overrides within current class.
+  // These are stored by name to make querying fast in the later step.
+  llvm::StringMap> Overrides;
+  for (auto *Method : CR->methods()) {
+if (!Method->isVirtual() || !Method->getIdentifier())
+  continue;
+Overrides[Method->getName()].push_back(Method);
+  }
+
+  for (const auto &Base : CR->bases()) {
+const auto *BR = Base.getType().getTypePtr()->getAsCXXRecordDecl();
+if (!BR)
+  continue;
+for (auto *Method : BR->methods()) {
+  if (!Method->isVirtual() || !Method->getIdentifier())
+continue;
+  const auto it = Overrides.find(Method->getName());
+  bool IsOverriden = false;
+  if (it != Overrides.end()) {
+for (auto *MD : it->second) {
+  // If the method in current body is not an overload of this virtual
+  // function, then it overrides this one.
+  if (!S.IsOverload(MD, Method, false)) {
+IsOverriden = true;
+break;
+  }
+}
+  }
+  if (!IsOverriden) {
+// Generates a new CodeCompletionResult by taking this function and
+// converting it into an override declaration with only one chunk in the
+// final CodeCompletionString as a TypedTextChunk.
+std::string OverrideSignature;
+llvm::raw_string_ostream OS(OverrideSignature);
+CodeCompletionResult CCR(Method, 0);
+PrintingPolicy Policy =
+getCompletionPrintingPolicy(S.getASTContext(), S.getPreprocessor());
+auto *CCS = CCR.createCodeCompletionStringForDecl(
+S.getPreprocessor(), S.getASTContext(), Builder,
+/*IncludeBriefComments=*/false, CCContext, Policy);
+printOverrideString(OS, CCS);
+OS << " override";
+Builder.AddTypedTextChunk(Builder.getAllocator().CopyString(OS.str()));
+Results.AddResult(CodeCompletionResult(Builder.TakeString(), Method,
+

[PATCH] D52225: [clang] Implement Override Suggestions in Sema.

2018-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:1639
+llvm::raw_string_ostream OS(OverrideSignature);
+CodeCompletionResult CCR(Method, 0);
+PrintingPolicy Policy =

ioeric wrote:
> kadircet wrote:
> > ioeric wrote:
> > > Could you add comments explaining what the following code does? The 
> > > original code in clangd only has `Results.emplace_back(Method, 0);`, and 
> > > it's non-trivial what the new code here is for.
> > Yeah you are right added some comments, previously this was handled on 
> > CodeCompletionBuilder in clangd which converted the completion item into a 
> > override declaration to be inserted.
> Thanks! Could you elaborate on why you need to explicitly handle `CCR` and 
> `CCS`, i.e. why was `Results.emplace_back(Method, 0);` not enough? I guess 
> it's to avoid inserting return type when it's already typed, but it's not 
> obvious from the code.
Actually it is to generate a new CodeCompletionString with a single TypedText 
chunk that will include all of the declaration. Since you need to type whole 
declaration, which is also the part that has been used to filter completion 
candidates based on completion token.

It also helped me notice a bug relating optional params, I wasn't putting them 
into the typedtext chunk whereas I should've, fixed that one also.


Repository:
  rC Clang

https://reviews.llvm.org/D52225



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

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Kristina Brooks via Phabricator via cfe-commits

kristina added inline comments.



Comment at: lib/Lex/PPDirectives.cpp:1892
   if (!File) {
-while (!isAlphanumeric(Filename.front())) {
+while (!Filename.empty() && !isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();

This line is tripping the assert, it seems best course of action would be a 
single check here and then just diagnosing an error unless you have managed to 
find other cases, in which case all the checks below are also warranted.


Repository:
  rC Clang

https://reviews.llvm.org/D52721



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

[PATCH] D52687: [clang-tidy] NFC use CHECK-NOTES in tests for cppcoreguidelines-owning-memory

2018-10-01 Thread Alexander Kornienko via Phabricator via cfe-commits

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

LG


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52687



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

[PATCH] D52691: [clang-tidy] NFC use CHECK-NOTES in tests for performance-move-constructor-init

2018-10-01 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added inline comments.



Comment at: test/clang-tidy/performance-move-constructor-init.cpp:115-117
+  // CHECK-NOTES: 7:1: note: FIX-IT applied suggested code changes
+  // CHECK-NOTES: 113:28: note: FIX-IT applied suggested code changes
+  // CHECK-NOTES: 113:29: note: FIX-IT applied suggested code changes

alexfh wrote:
> This is an example of not very useful CHECK-NOTES patterns.  The 
> corresponding notes are issued in Clang and there's not much value in testing 
> them here. They also duplicate the CHECK-FIXES pattern below. Should the 
> script filter these notes out, maybe?
I agree that they dont add any value.
I try to change the filtering regex to give a pass to `note: FIX-IT`


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52691



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

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Kristina Brooks via Phabricator via cfe-commits

kristina added inline comments.



Comment at: lib/Lex/PPDirectives.cpp:1892
   if (!File) {
-while (!isAlphanumeric(Filename.front())) {
+while (!Filename.empty() && !isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();

kristina wrote:
> This line is tripping the assert, it seems best course of action would be a 
> single check here and then just diagnosing an error unless you have managed 
> to find other cases, in which case all the checks below are also warranted.
In either case, the diagnostic emitted doesn't really make sense, at least to 
me, I think it would be better to explicitly diagnose this case as an error and 
then bail, before an assertion fires.

I also crashed clang with `#include "./"`, so the test case does seem to be 
fairly minimal which is good. Though I think a diagnostic about a bogus file 
path would be better (I don't know how to word it well), rather than saying 
file not found.


Repository:
  rC Clang

https://reviews.llvm.org/D52721



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

[PATCH] D52690: [clang-tidy] NFC use CHECK-NOTES in tests for misc-misplaced-const

2018-10-01 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added inline comments.



Comment at: test/clang-tidy/misc-misplaced-const.c:18
+  // CHECK-NOTES: :[[@LINE-1]]:12: warning: 'i3' declared with a 
const-qualified typedef type; results in the type being 'int *const' instead of 
'const int *'
+  // CHECK-NOTES: :[[@LINE-14]]:14: note: typedef declared here
 

alexfh wrote:
> These notes are also just marginally useful and make it harder to change the 
> test. I wonder whether an absolute line number would make more sense here. Or 
> maybe just add a test for one of the notes and leave out the rest (and keep 
> CHECK-MESSAGES)?
Absolute line number makes sense. IMHO the tests should cover all generated 
diagnostics including the notes. Would you accept sticking with `CHECK-NOTES` 
but with absolute line numbers?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52690



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

[PATCH] D52684: [clang-tidy] NFC refactor lexer-utils slightly to be easier to use

2018-10-01 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added a comment.

This patch is related to https://reviews.llvm.org/D51949

To isolate variable declarations (split `int * p, v;` up) it is necessary to do 
a lot of work with source location and requires some forward and backwards 
lexing. The functions there just use the LangOpts and the SourceManager and 
don't have a `ASTContext` available, that why I changed this interface.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52684



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

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

This is awesome! Can't wait for it to land!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52710



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

[PATCH] D52722: [analyzer] Move StdCLibraryFunctions to apiModeling

2018-10-01 Thread Donát Nagy via Phabricator via cfe-commits

donat.nagy created this revision.
donat.nagy added a reviewer: NoQ.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho, a.sidorin, 
szepet, eraman, xazax.hun.
Herald added a reviewer: george.karpenkov.

StdCLibraryFunctionsChecker models the evaluation of several well-known
functions of the C standard library. This commit moves it to the
apiModeling category, which is loaded by default (it was in the unix
category, despite the fact that it models platform-independent standard
functions). As we load this checker, ConversionChecker will no longer
emit false positives when it encunters certain functions (e.g. getc()
and the character classification functions); this commit updates its
regression test to reflect this fact.


Repository:
  rC Clang

https://reviews.llvm.org/D52722

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  test/Analysis/conversion.c
  test/Analysis/std-c-library-functions-inlined.c
  test/Analysis/std-c-library-functions.c
  test/Analysis/std-c-library-functions.cpp

Index: test/Analysis/std-c-library-functions.cpp
===
--- test/Analysis/std-c-library-functions.cpp
+++ test/Analysis/std-c-library-functions.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify %s
 
 // Test that we don't model functions with broken prototypes.
 // Because they probably work differently as well.
Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -1,8 +1,8 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -triple armv7-a15-linux -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -triple thumbv7-a15-linux -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple armv7-a15-linux -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple thumbv7-a15-linux -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
 
 void clang_analyzer_eval(int);
 
Index: test/Analysis/std-c-library-functions-inlined.c
===
--- test/Analysis/std-c-library-functions-inlined.c
+++ test/Analysis/std-c-library-functions-inlined.c
@@ -1,8 +1,8 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=unix.StdCLibraryFunctions -verify %s
-// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=unix.StdCLibraryFunctions -verify %s
-// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux -analyzer-checker=unix.StdCLibraryFunctions -verify %s
-// RUN: %clang_analyze_cc1 -triple armv7-a15-linux -analyzer-checker=unix.StdCLibraryFunctions -verify %s
-// RUN: %clang_analyze_cc1 -triple thumbv7-a15-linux -analyzer-checker=unix.StdCLibraryFunctions -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=apiModeling.StdCLibraryFunctions -verify %s
+// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=apiModeling.StdCLibraryFunctions -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux -analyzer-checker=apiModeling.StdCLibraryFunctions -verify %s
+// RUN: %clang_analyze_cc1 -triple armv7-a15-linux -analyzer-checker=apiModeling.StdCLibraryFunctions -verify %s
+// RUN: %clang_analyze_cc1 -triple thumbv7-a15-linux -analyzer-checker=apiModeling.StdCLibraryFunctions -verify %s
 
 // This test tests crashes that occur when standard functions are available
 // for inlining.
Index: test/Analys

[PATCH] D52423: [analyzer] Make ConversionChecker load StdCLibraryFunctionsChecker

2018-10-01 Thread Donát Nagy via Phabricator via cfe-commits

donat.nagy abandoned this revision.
donat.nagy added a comment.

I submitted a new patch, which moves stdCLibraryFunctions to apiModeling 
(https://reviews.llvm.org/D52722).


Repository:
  rC Clang

https://reviews.llvm.org/D52423



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

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

Need someone stamps on https://reviews.llvm.org/D52713 before landing this 
patch :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52710



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

[PATCH] D52226: [clangd] Remove override result handling logic from clangd

2018-10-01 Thread Eric Liu via Phabricator via cfe-commits

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

lg


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52226



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

[PATCH] D52713: Move llvm util dependencies from clang-tools-extra to add_lit_target.

2018-10-01 Thread Sam McCall via Phabricator via cfe-commits

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

(again I'd just inline this into three `add_dependencies` calls, but up to you)


Repository:
  rL LLVM

https://reviews.llvm.org/D52713



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

[PATCH] D52225: [clang] Implement Override Suggestions in Sema.

2018-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 167717.
kadircet added a comment.

- Add a fixme on limitation.


Repository:
  rC Clang

https://reviews.llvm.org/D52225

Files:
  include/clang/Sema/CodeCompleteConsumer.h
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/overrides.cpp

Index: test/CodeCompletion/overrides.cpp
===
--- /dev/null
+++ test/CodeCompletion/overrides.cpp
@@ -0,0 +1,33 @@
+class A {
+ public:
+  virtual void vfunc(bool param);
+  virtual void vfunc(bool param, int p);
+  void func(bool param);
+};
+class B : public A {
+virtual int ttt(bool param, int x = 3) const;
+void vfunc(bool param, int p) override;
+};
+class C : public B {
+ public:
+  void vfunc(bool param) override;
+  void
+};
+
+// Runs completion at ^void.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:3 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1: COMPLETION: Pattern : int ttt(bool param, int x = 3) const override{{$}}
+// CHECK-CC1: COMPLETION: Pattern : void vfunc(bool param, int p) override{{$}}
+// CHECK-CC1-NOT: COMPLETION: Pattern : void vfunc(bool param) override{{$}}
+//
+// Runs completion at vo^id.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:5 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// CHECK-CC2: COMPLETION: Pattern : void vfunc(bool param, int p) override{{$}}
+// CHECK-CC2-NOT: COMPLETION: Pattern : int ttt(bool param, int x = 3) const override{{$}}
+// CHECK-CC2-NOT: COMPLETION: Pattern : void vfunc(bool param) override{{$}}
+//
+// Runs completion at void ^.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:8 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC3-NOT: COMPLETION: Pattern : int ttt(bool param, int x = 3) const override{{$}}
+// CHECK-CC3-NOT: COMPLETION: Pattern : void vfunc(bool param, int p) override{{$}}
+// CHECK-CC3-NOT: COMPLETION: Pattern : void vfunc(bool param) override{{$}}
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -1598,6 +1598,74 @@
   Results.AddResult(CodeCompletionResult(Builder.TakeString()));
 }
 
+namespace {
+void printOverrideString(llvm::raw_ostream &OS, CodeCompletionString *CCS) {
+  for (const auto &C : *CCS) {
+if (C.Kind == CodeCompletionString::CK_Optional)
+  printOverrideString(OS, C.Optional);
+else
+  OS << C.Text;
+// Add a space after return type.
+if (C.Kind == CodeCompletionString::CK_ResultType)
+  OS << ' ';
+  }
+}
+} // namespace
+
+static void AddOverrideResults(ResultBuilder &Results,
+   const CodeCompletionContext &CCContext,
+   CodeCompletionBuilder &Builder) {
+  Sema &S = Results.getSema();
+  const auto *CR = llvm::dyn_cast(S.CurContext);
+  // If not inside a class/struct/union return empty.
+  if (!CR)
+return;
+  // First store overrides within current class.
+  // These are stored by name to make querying fast in the later step.
+  llvm::StringMap> Overrides;
+  for (auto *Method : CR->methods()) {
+if (!Method->isVirtual() || !Method->getIdentifier())
+  continue;
+Overrides[Method->getName()].push_back(Method);
+  }
+
+  for (const auto &Base : CR->bases()) {
+const auto *BR = Base.getType().getTypePtr()->getAsCXXRecordDecl();
+if (!BR)
+  continue;
+for (auto *Method : BR->methods()) {
+  if (!Method->isVirtual() || !Method->getIdentifier())
+continue;
+  const auto it = Overrides.find(Method->getName());
+  bool IsOverriden = false;
+  if (it != Overrides.end()) {
+for (auto *MD : it->second) {
+  // If the method in current body is not an overload of this virtual
+  // function, then it overrides this one.
+  if (!S.IsOverload(MD, Method, false)) {
+IsOverriden = true;
+break;
+  }
+}
+  }
+  if (!IsOverriden) {
+// Generates a new CodeCompletionResult by taking this function and
+// converting it into an override declaration with only one chunk in the
+// final CodeCompletionString as a TypedTextChunk.
+std::string OverrideSignature;
+llvm::raw_string_ostream OS(OverrideSignature);
+CodeCompletionResult CCR(Method, 0);
+PrintingPolicy Policy =
+getCompletionPrintingPolicy(S.getASTContext(), S.getPreprocessor());
+auto *CCS = CCR.createCodeCompletionStringForOverride(
+S.getPreprocessor(), S.getASTContext(), Builder,
+/*IncludeBriefComments=*/false, CCContext, Policy);
+Results.AddResult(CodeCompletionResult(CCS, Method, CCP_CodePattern));
+  }
+}
+  }
+}
+
 /// Add language constructs that show up for "ordinary" names.
 static void AddOrdinaryNameResults(Sema::ParserCompletionContext CCC,
Scope *S,
@@ -1706,

[PATCH] D52301: [clang] Set TypeSourceInfo for vardecl's in addition to type when we can deduce.

2018-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 167718.
kadircet added a comment.

rebase & ping


Repository:
  rC Clang

https://reviews.llvm.org/D52301

Files:
  include/clang/AST/PrettyPrinter.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/TypePrinter.cpp
  lib/Frontend/ASTConsumers.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
  test/Analysis/scopes-cfg-output.cpp
  test/SemaOpenCL/to_addr_builtin.cl

Index: test/SemaOpenCL/to_addr_builtin.cl
===
--- test/SemaOpenCL/to_addr_builtin.cl
+++ test/SemaOpenCL/to_addr_builtin.cl
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -verify -fsyntax-only %s
-// RUN: %clang_cc1 -Wconversion -verify -fsyntax-only -cl-std=CL2.0 %s
+// RUN: %clang_cc1 -verify -fsyntax-only -cl-std=CL2.0 %s
 
 void test(void) {
   global int *glob;
@@ -45,7 +45,6 @@
   // expected-warning@-2{{incompatible integer to pointer conversion assigning to '__local int *' from 'int'}}
 #else
   // expected-error@-4{{assigning '__global int *' to '__local int *' changes address space of pointer}}
-  // expected-warning@-5{{passing non-generic address space pointer to to_global may cause dynamic conversion affecting performance}}
 #endif
 
   loc = to_private(glob);
@@ -119,7 +118,6 @@
   // expected-warning@-2{{incompatible integer to pointer conversion initializing '__global char *' with an expression of type 'int'}}
 #else
   // expected-warning@-4{{incompatible pointer types initializing '__global char *' with an expression of type '__global int *'}}
-  // expected-warning@-5{{passing non-generic address space pointer to to_global may cause dynamic conversion affecting performance}}
 #endif
 
   glob_wrong_ty = to_global(glob);
Index: test/Analysis/scopes-cfg-output.cpp
===
--- test/Analysis/scopes-cfg-output.cpp
+++ test/Analysis/scopes-cfg-output.cpp
@@ -834,7 +834,7 @@
 // CHECK-NEXT:   2: __begin1
 // CHECK-NEXT:   3: [B4.2] (ImplicitCastExpr, LValueToRValue, class A *)
 // CHECK-NEXT:   4: *[B4.3]
-// CHECK-NEXT:   5: auto &i = *__begin1;
+// CHECK-NEXT:   5: A &i = *__begin1;
 // CHECK-NEXT:   6: operator=
 // CHECK-NEXT:   7: [B4.6] (ImplicitCastExpr, FunctionToPointerDecay, class A &(*)(const class A &)
 // CHECK-NEXT:   8: i
@@ -849,16 +849,16 @@
 // CHECK-NEXT:   2:  (CXXConstructExpr, [B5.3], class A [10])
 // CHECK-NEXT:   3: A a[10];
 // CHECK-NEXT:   4: a
-// CHECK-NEXT:   5: auto &&__range1 = a;
+// CHECK-NEXT:   5: A (&__range1)[10] = a;
 // CHECK-NEXT:   6: CFGScopeBegin(__end1)
 // CHECK-NEXT:   7: __range1
 // CHECK-NEXT:   8: [B5.7] (ImplicitCastExpr, ArrayToPointerDecay, class A *)
 // CHECK-NEXT:   9: 10
 // CHECK-NEXT:  10: [B5.8] + [B5.9]
-// CHECK-NEXT:  11: auto __end1 = __range1 + 10
+// CHECK-NEXT:  11: A *__end1 = __range1 + 10
 // CHECK-NEXT:  12: __range1
 // CHECK-NEXT:  13: [B5.12] (ImplicitCastExpr, ArrayToPointerDecay, class A *)
-// CHECK-NEXT:  14: auto __begin1 = __range1;
+// CHECK-NEXT:  14: A *__begin1 = __range1;
 // CHECK-NEXT:   Preds (1): B6
 // CHECK-NEXT:   Succs (1): B2
 // CHECK:  [B0 (EXIT)]
Index: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
===
--- lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
+++ lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
@@ -351,6 +351,8 @@
   }
 
   bool VisitTypeLoc(TypeLoc Loc) {
+if(Loc.getTypeLocClass() == TypeLoc::Auto)
+  return true;
 auto Parents = Context.getParents(Loc);
 TypeLoc ParentTypeLoc;
 if (!Parents.empty()) {
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -4340,19 +4340,20 @@
   return E;
 }
 
-QualType Apply(TypeLoc TL) {
+TypeSourceInfo* Apply(TypeLoc TL) {
   // Create some scratch storage for the transformed type locations.
   // FIXME: We're just going to throw this information away. Don't build it.
   TypeLocBuilder TLB;
   TLB.reserve(TL.getFullDataSize());
-  return TransformType(TLB, TL);
+  QualType Result = TransformType(TLB, TL);
+  return TLB.getTypeSourceInfo(getSema().Context, Result);
 }
   };
 
 } // namespace
 
 Sema::DeduceAutoResult
-Sema::DeduceAutoType(TypeSourceInfo *Type, Expr *&Init, QualType &Result,
+Sema::DeduceAutoType(TypeSourceInfo *Type, Expr *&Init, TypeSourceInfo *&Result,
  Optional DependentDeductionDepth) {
   return DeduceAutoType(Type->getTypeLoc(), Init, Result,
 DependentDeductionDepth);
@@ -4398,7 +4399,7 @@
 ///'auto' template parameters. The value specified is the template
 ///

[PATCH] D52690: [clang-tidy] NFC use CHECK-NOTES in tests for misc-misplaced-const

2018-10-01 Thread Alexander Kornienko via Phabricator via cfe-commits

alexfh added inline comments.



Comment at: test/clang-tidy/misc-misplaced-const.c:18
+  // CHECK-NOTES: :[[@LINE-1]]:12: warning: 'i3' declared with a 
const-qualified typedef type; results in the type being 'int *const' instead of 
'const int *'
+  // CHECK-NOTES: :[[@LINE-14]]:14: note: typedef declared here
 

JonasToth wrote:
> alexfh wrote:
> > These notes are also just marginally useful and make it harder to change 
> > the test. I wonder whether an absolute line number would make more sense 
> > here. Or maybe just add a test for one of the notes and leave out the rest 
> > (and keep CHECK-MESSAGES)?
> Absolute line number makes sense. IMHO the tests should cover all generated 
> diagnostics including the notes. Would you accept sticking with `CHECK-NOTES` 
> but with absolute line numbers?
> IMHO the tests should cover all generated diagnostics including the notes.

I wouldn't call this the most important goal. I'd say that tests should cover 
important aspects of the output, not every single character of it. Another 
useful feature is that tests should be easy to create, read, and change.  In 
cases like this - where the benefit of the change is not obvious - I would 
leave the decision to the author of the check.

Aaron, WDYT?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52690



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

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

2018-10-01 Thread Alexander Kornienko via Phabricator via cfe-commits

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

In https://reviews.llvm.org/D36836#1249971, @JonasToth wrote:

> Anything new here? Is there a LLVM foundation lawyer or something like that 
> we can ask?

I didn't notice any substantial changes w.r.t. license since Chandler's comment 
on Feb. 28th.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D36836

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

[PATCH] D52713: Move llvm util dependencies from clang-tools-extra to add_lit_target.

2018-10-01 Thread Stephen Kelly via Phabricator via cfe-commits

steveire added a comment.

inlining this into three `add_dependencies` calls would remove the `if 
(TARGET)` conditions. Are those dependencies conditionally built?


Repository:
  rL LLVM

https://reviews.llvm.org/D52713



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

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

2018-10-01 Thread Jano Simas via Phabricator via cfe-commits

janosimas added a comment.

I was thinking about the usage of `--` and `-extra-arg`, don't they do the same 
thing?
To be honest, for me, the current behavior of `--` doesn't make much sense. If 
there is a use case for `-extra-arg-before` and `-extra-arg`, they are much 
more clearer in intent.
For me, `--` usual behavior would be pass by options for the first next 
program, `clang-tidy` in this case.

---

Is there a reason to limit how the flags a passed to `clang-tidy`?
Why not pass all options as if using `clang-tidy` and only threat special cases 
and default values?

Comment at: clang-tidy/tool/clang-tidy-diff.py:123-130
   if args.fix:
 command.append('-fix')
   if args.checks != '':
 command.append('-checks=' + quote + args.checks + quote)
   if args.quiet:
 command.append('-quiet')
   if args.build_path is not None:

alexfh wrote:
> janosimas wrote:
> > janosimas wrote:
> > > alexfh wrote:
> > > > If we make the script leave out the `--` flag, we should stop 
> > > > forwarding these flags and the `extra_arg(_before)?` below. Otherwise 
> > > > it's too confusing (should one place -fix before `--` or after? what 
> > > > about `-warnings-as-errors`?).
> > > > 
> > > > Please also update the usage example at the top.
> > > What about keep the current `--` behavior and add a new flag 
> > > `-extra-tidy-flags` ? 
> > `-extra-tidy-arg` to maintain consistency.
> What's the benefit of `-extra-tidy-arg` compared to pass-by arguments?
`-extra-tidy-arg` would keep the current `--` behavior and cover other cases of 
extra flags not supported by this script.
I was worried about how changing the behavior would affect people that already 
using it.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49864

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

[PATCH] D52713: Move llvm util dependencies from clang-tools-extra to add_lit_target.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

In https://reviews.llvm.org/D52713#1250947, @steveire wrote:

> inlining this into three `add_dependencies` calls would remove the `if 
> (TARGET)` conditions. Are those dependencies conditionally built?

Yeah, I believe these targets are built conditionally, see 
https://reviews.llvm.org/D29851.

Repository:
  rL LLVM

https://reviews.llvm.org/D52713

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

r343472 - Add support for unified_shared_memory clause on requires directive

2018-10-01 Thread Patrick Lyster via cfe-commits

Author: plyster
Date: Mon Oct  1 06:47:43 2018
New Revision: 343472

URL: http://llvm.org/viewvc/llvm-project?rev=343472&view=rev
Log:
Add support for unified_shared_memory clause on requires directive

Modified:
cfe/trunk/include/clang/AST/OpenMPClause.h
cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
cfe/trunk/include/clang/Basic/OpenMPKinds.def
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/OpenMPClause.cpp
cfe/trunk/lib/AST/StmtPrinter.cpp
cfe/trunk/lib/AST/StmtProfile.cpp
cfe/trunk/lib/Basic/OpenMPKinds.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/test/OpenMP/requires_unified_address_ast_print.cpp
cfe/trunk/test/OpenMP/requires_unified_address_messages.cpp
cfe/trunk/tools/libclang/CIndex.cpp

Modified: cfe/trunk/include/clang/AST/OpenMPClause.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OpenMPClause.h?rev=343472&r1=343471&r2=343472&view=diff
==
--- cfe/trunk/include/clang/AST/OpenMPClause.h (original)
+++ cfe/trunk/include/clang/AST/OpenMPClause.h Mon Oct  1 06:47:43 2018
@@ -765,6 +765,37 @@ public:
   }
 };
 
+/// This represents 'unified_shared_memory' clause in the '#pragma omp 
requires'
+/// directive.
+///
+/// \code
+/// #pragma omp requires unified_shared_memory
+/// \endcode
+/// In this example directive '#pragma omp requires' has 
'unified_shared_memory'
+/// clause.
+class OMPUnifiedSharedMemoryClause final : public OMPClause {
+public:
+  friend class OMPClauseReader;
+  /// Build 'unified_shared_memory' clause.
+  ///
+  /// \param StartLoc Starting location of the clause.
+  /// \param EndLoc Ending location of the clause.
+  OMPUnifiedSharedMemoryClause(SourceLocation StartLoc, SourceLocation EndLoc)
+  : OMPClause(OMPC_unified_shared_memory, StartLoc, EndLoc) {}
+
+  /// Build an empty clause.
+  OMPUnifiedSharedMemoryClause()
+  : OMPClause(OMPC_unified_shared_memory, SourceLocation(), 
SourceLocation()) {}
+
+  child_range children() {
+return child_range(child_iterator(), child_iterator());
+  }
+
+  static bool classof(const OMPClause *T) {
+return T->getClauseKind() == OMPC_unified_shared_memory;
+  }
+};
+
 /// This represents 'schedule' clause in the '#pragma omp ...' directive.
 ///
 /// \code

Modified: cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecursiveASTVisitor.h?rev=343472&r1=343471&r2=343472&view=diff
==
--- cfe/trunk/include/clang/AST/RecursiveASTVisitor.h (original)
+++ cfe/trunk/include/clang/AST/RecursiveASTVisitor.h Mon Oct  1 06:47:43 2018
@@ -2868,6 +2868,12 @@ bool RecursiveASTVisitor::Visit
 }
 
 template 
+bool RecursiveASTVisitor::VisitOMPUnifiedSharedMemoryClause(
+OMPUnifiedSharedMemoryClause *) {
+  return true;
+}
+
+template 
 bool
 RecursiveASTVisitor::VisitOMPScheduleClause(OMPScheduleClause *C) {
   TRY_TO(VisitOMPClauseWithPreInit(C));

Modified: cfe/trunk/include/clang/Basic/OpenMPKinds.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/OpenMPKinds.def?rev=343472&r1=343471&r2=343472&view=diff
==
--- cfe/trunk/include/clang/Basic/OpenMPKinds.def (original)
+++ cfe/trunk/include/clang/Basic/OpenMPKinds.def Mon Oct  1 06:47:43 2018
@@ -280,6 +280,7 @@ OPENMP_CLAUSE(is_device_ptr, OMPIsDevice
 OPENMP_CLAUSE(task_reduction,  OMPTaskReductionClause)
 OPENMP_CLAUSE(in_reduction,  OMPInReductionClause)
 OPENMP_CLAUSE(unified_address, OMPUnifiedAddressClause)
+OPENMP_CLAUSE(unified_shared_memory, OMPUnifiedSharedMemoryClause)
 
 // Clauses allowed for OpenMP directive 'parallel'.
 OPENMP_PARALLEL_CLAUSE(if)
@@ -463,6 +464,7 @@ OPENMP_TARGET_CLAUSE(reduction)
 
 // Clauses allowed for OpenMP directive 'requires'.
 OPENMP_REQUIRES_CLAUSE(unified_address)
+OPENMP_REQUIRES_CLAUSE(unified_shared_memory)
 
 // Clauses allowed for OpenMP directive 'target data'.
 OPENMP_TARGET_DATA_CLAUSE(if)

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=343472&r1=343471&r2=343472&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon Oct  1 06:47:43 2018
@@ -9168,6 +9168,10 @@ public:
   OMPClause *ActOnOpenMPUnifiedAddressClause(SourceLocation StartLoc,
  SourceLocation EndLoc);
 
+  /// Called on well-formed 'unified_address' clause.
+  OMPClause *ActOnOpenMPUnifiedSharedMemoryClause(Sour

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 167724.
hokein marked an inline comment as done.
hokein added a comment.

Update the code.


Repository:
  rC Clang

https://reviews.llvm.org/D52721

Files:
  lib/Lex/PPDirectives.cpp
  test/Preprocessor/include-nonalpha-no-crash.c


Index: test/Preprocessor/include-nonalpha-no-crash.c
===
--- /dev/null
+++ test/Preprocessor/include-nonalpha-no-crash.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "./" // expected-error {{'./' file not found}}
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1889,19 +1889,31 @@
   // characters
   StringRef OriginalFilename = Filename;
   if (!File) {
-while (!isAlphanumeric(Filename.front())) {
-  Filename = Filename.drop_front();
-}
-while (!isAlphanumeric(Filename.back())) {
-  Filename = Filename.drop_back();
-}
-
-File = LookupFile(
-FilenameLoc,
-LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
-LookupFrom, LookupFromFile, CurDir,
-Callbacks ? &SearchPath : nullptr,
-Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
+// A heuristic to correct a typo file name by removing leading and
+// trailing non-isAlphanumeric characters.
+auto CorrectTypoFilename = [](llvm::StringRef Filename) {
+  while (!Filename.empty() && !isAlphanumeric(Filename.front())) {
+Filename = Filename.drop_front();
+  }
+  if (Filename.empty())
+return Filename;
+  while (!isAlphanumeric(Filename.back())) {
+Filename = Filename.drop_back();
+assert(!Filename.empty());
+  }
+  return Filename;
+};
+Filename = CorrectTypoFilename(Filename);
+
+File = Filename.empty()
+   ? nullptr
+   : LookupFile(FilenameLoc,
+LangOpts.MSVCCompat ? NormalizedPath.c_str()
+: Filename,
+isAngled, LookupFrom, LookupFromFile, CurDir,
+Callbacks ? &SearchPath : nullptr,
+Callbacks ? &RelativePath : nullptr,
+&SuggestedModule, &IsMapped);
 if (File) {
   SourceRange Range(FilenameTok.getLocation(), CharEnd);
   auto Hint = isAngled ? FixItHint::CreateReplacement(


Index: test/Preprocessor/include-nonalpha-no-crash.c
===
--- /dev/null
+++ test/Preprocessor/include-nonalpha-no-crash.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "./" // expected-error {{'./' file not found}}
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1889,19 +1889,31 @@
   // characters
   StringRef OriginalFilename = Filename;
   if (!File) {
-while (!isAlphanumeric(Filename.front())) {
-  Filename = Filename.drop_front();
-}
-while (!isAlphanumeric(Filename.back())) {
-  Filename = Filename.drop_back();
-}
-
-File = LookupFile(
-FilenameLoc,
-LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
-LookupFrom, LookupFromFile, CurDir,
-Callbacks ? &SearchPath : nullptr,
-Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
+// A heuristic to correct a typo file name by removing leading and
+// trailing non-isAlphanumeric characters.
+auto CorrectTypoFilename = [](llvm::StringRef Filename) {
+  while (!Filename.empty() && !isAlphanumeric(Filename.front())) {
+Filename = Filename.drop_front();
+  }
+  if (Filename.empty())
+return Filename;
+  while (!isAlphanumeric(Filename.back())) {
+Filename = Filename.drop_back();
+assert(!Filename.empty());
+  }
+  return Filename;
+};
+Filename = CorrectTypoFilename(Filename);
+
+File = Filename.empty()
+   ? nullptr
+   : LookupFile(FilenameLoc,
+LangOpts.MSVCCompat ? NormalizedPath.c_str()
+: Filename,
+isAngled, LookupFrom, LookupFromFile, CurDir,
+Callbacks ? &SearchPath : nullptr,
+Callbacks ? &RelativePath : nullptr,
+&SuggestedModule, &IsMapped);
 if (File) {
   SourceRange Range(Fil

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: lib/Lex/PPDirectives.cpp:1891
   StringRef OriginalFilename = Filename;
   if (!File) {
+while (!Filename.empty() && !isAlphanumeric(Filename.front())) {

sammccall wrote:
> everything in this block is guarded by !Filename.empty().
> Just add it to the if condition?
The logic is tricky... inside the if body, we modify `Filename`, so we can't 
just add the judgement here.



Comment at: lib/Lex/PPDirectives.cpp:1892
   if (!File) {
-while (!isAlphanumeric(Filename.front())) {
+while (!Filename.empty() && !isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();

kristina wrote:
> kristina wrote:
> > This line is tripping the assert, it seems best course of action would be a 
> > single check here and then just diagnosing an error unless you have managed 
> > to find other cases, in which case all the checks below are also warranted.
> In either case, the diagnostic emitted doesn't really make sense, at least to 
> me, I think it would be better to explicitly diagnose this case as an error 
> and then bail, before an assertion fires.
> 
> I also crashed clang with `#include "./"`, so the test case does seem to be 
> fairly minimal which is good. Though I think a diagnostic about a bogus file 
> path would be better (I don't know how to word it well), rather than saying 
> file not found.
Thanks for the comment.

IIUC, do you mean we create a new diagnostic message for this specific case? 
I'm double that it is worth doing it, since IMO this is a heuristic that 
getting a typo file path, and user input is arbitrary, make the diagnostic fit 
all users input would be very tricky.

I think we can just provide best-effort for this case (e.g. no crash), 
"err_pp_file_not_found" seems the best one from existing messages. 

I checked with `g++,` it provides a slightly better message ("fatal error: ./: 
No such file or directory" vs clang's "'./' file not found").




Comment at: lib/Lex/PPDirectives.cpp:1925
 
+  if (Filename.empty())
+return;

sammccall wrote:
> this is mysterious - what does it solve? is it the right place to handle this 
> problem?
> 
> (You allude to a code complete crash - can you explain?)
ah, I created this patch when fixing a clangd crash, it seems still crash 
clangd without it, but we can pass the test without it. I think that is another 
issue, so I remove this code now, and address it in a follow-up patch.


Repository:
  rC Clang

https://reviews.llvm.org/D52721



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

[PATCH] D51429: [AArch64] Return Address Signing B Key Support

2018-10-01 Thread Luke Cheeseman via Phabricator via cfe-commits

LukeCheeseman updated this revision to Diff 167723.
LukeCheeseman added a comment.

- Introduce the -mbranch-protection option. This is to be used for both pointer 
authentication and branch protection security features.
  - The branch protection compiler support will follow later
- The options available are -mbranch-protection=(+)* where  
::= [bti,pac-ret[+leaf,b-key]*]
- This should be the primary way of using branch protection features and the 
-msign-return-address option should be deprecated
- Remove the b-key selection from the earlier patch for -msign-return-address


https://reviews.llvm.org/D51429

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/CC1Options.td
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/TargetInfo.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/aarch64-sign-return-address.c
  test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp

Index: test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp
===
--- test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp
+++ test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp
@@ -1,9 +1,9 @@
 // RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=none  %s | \
 // RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NONE
 // RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=non-leaf %s | \
-// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-PARTIAL
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-PARTIAL  --check-prefix=CHECK-A-KEY
 // RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=all %s | \
-// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ALL
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ALL  --check-prefix=CHECK-A-KEY
 
 struct Foo {
   Foo() {}
@@ -16,6 +16,7 @@
 
 // CHECK: @[[CTOR_FN]]() #[[ATTR:[0-9]*]]
 
-// CHECK-NONE-NOT: attributes #[[ATTR]] = { {{.*}} "sign-return-address"={{.*}} }}
-// CHECK-PARTIAL: attributes #[[ATTR]] = { {{.*}} "sign-return-address"="non-leaf" {{.*}}}
-// CHECK-ALL: attributes #[[ATTR]] = { {{.*}} "sign-return-address"="all" {{.*}} }
+// CHECK-NONE-NOT: "sign-return-address"={{.*}}
+// CHECK-PARTIAL: "sign-return-address"="non-leaf"
+// CHECK-ALL: "sign-return-address"="all"
+// CHECK-A-KEY: "sign-return-address-key"="a_key"
Index: test/CodeGen/aarch64-sign-return-address.c
===
--- test/CodeGen/aarch64-sign-return-address.c
+++ test/CodeGen/aarch64-sign-return-address.c
@@ -1,14 +1,15 @@
-// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=none  %s | FileCheck %s --check-prefix=CHECK-NONE
-// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=non-leaf %s | FileCheck %s --check-prefix=CHECK-PARTIAL
-// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=all %s | FileCheck %s --check-prefix=CHECK-ALL
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=none  %s | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NONE
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=non-leaf %s | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-PARTIAL -check-prefix=CHECK-A-KEY
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=all %s | FileCheck %s  --check-prefix=CHECK --check-prefix=CHECK-ALL -check-prefix=CHECK-A-KEY
 
-// CHECK-NONE: @foo() #[[ATTR:[0-9]*]]
-// CHECK-NONE-NOT: attributes #[[ATTR]] = { {{.*}} "sign-return-address"={{.*}} }}
+// CHECK: @foo() #[[ATTR:[0-9]*]]
+//
+// CHECK-NONE-NOT: "sign-return-address"={{.*}}
 
-// CHECK-PARTIAL: @foo() #[[ATTR:[0-9]*]]
-// CHECK-PARTIAL: attributes #[[ATTR]] = { {{.*}} "sign-return-address"="non-leaf" {{.*}}}
+// CHECK-PARTIAL: "sign-return-address"="non-leaf"
 
-// CHECK-ALL: @foo() #[[ATTR:[0-9]*]]
-// CHECK-ALL: attributes #[[ATTR]] = { {{.*}} "sign-return-address"="all" {{.*}} }
+// CHECK-ALL: "sign-return-address"="all"
+
+// CHECK-A-KEY: "sign-return-address-key"="a_key"
 
 void foo() {}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1129,8 +1129,11 @@
 
   Opts.Addrsig = Args.hasArg(OPT_faddrsig);
 
-  if (Arg *A = Args.getLastArg(OPT_msign_return_address)) {
-StringRef SignScope = A->getValue();
+  if (Arg *A = Args.getLastArg(OPT_msign_return_address_EQ)) {
+const auto SignScopeKey = StringRef(A->getValue()).split('+');
+StringRef SignScope = SignScopeKey.first;
+StringRef SignKey = SignScopeKey.seco

[PATCH] D52713: Move llvm util dependencies from clang-tools-extra to add_lit_target.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343473: Move llvm util dependencies from clang-tools-extra 
to add_lit_target. (authored by hokein, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D52713

Files:
  llvm/trunk/cmake/modules/AddLLVM.cmake


Index: llvm/trunk/cmake/modules/AddLLVM.cmake
===
--- llvm/trunk/cmake/modules/AddLLVM.cmake
+++ llvm/trunk/cmake/modules/AddLLVM.cmake
@@ -1367,6 +1367,17 @@
   COMMAND ${CMAKE_COMMAND} -E echo "${target} does nothing, no tools 
built.")
 message(STATUS "${target} does nothing.")
   endif()
+
+  # Add lit test dependencies.
+  set(llvm_utils_deps
+FileCheck count not
+  )
+  foreach(dep ${llvm_utils_deps})
+if (TARGET ${dep})
+  add_dependencies(${target} ${dep})
+endif()
+  endforeach()
+
   if (ARG_DEPENDS)
 add_dependencies(${target} ${ARG_DEPENDS})
   endif()


Index: llvm/trunk/cmake/modules/AddLLVM.cmake
===
--- llvm/trunk/cmake/modules/AddLLVM.cmake
+++ llvm/trunk/cmake/modules/AddLLVM.cmake
@@ -1367,6 +1367,17 @@
   COMMAND ${CMAKE_COMMAND} -E echo "${target} does nothing, no tools built.")
 message(STATUS "${target} does nothing.")
   endif()
+
+  # Add lit test dependencies.
+  set(llvm_utils_deps
+FileCheck count not
+  )
+  foreach(dep ${llvm_utils_deps})
+if (TARGET ${dep})
+  add_dependencies(${target} ${dep})
+endif()
+  endforeach()
+
   if (ARG_DEPENDS)
 add_dependencies(${target} ${ARG_DEPENDS})
   endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r343474 - [clangd] Add "check-clangd" target

2018-10-01 Thread Haojian Wu via cfe-commits

Author: hokein
Date: Mon Oct  1 07:02:02 2018
New Revision: 343474

URL: http://llvm.org/viewvc/llvm-project?rev=343474&view=rev
Log:
[clangd] Add "check-clangd" target

Summary:
So we don't have to run "check-clang-tools" (which builds and tests all
clang tools) to verify our clangd-related change. It'd save waiting time for
clangd developers.

check-clangd (build ~1000 files, run ~340 tests) vs check-clang-tools (build
~3000 files, run ~1000 tests).

In the future, we probably want to add similar target for other
clang-tools (e.g. clang-tidy).

Reviewers: sammccall

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

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

Modified:
clang-tools-extra/trunk/test/CMakeLists.txt

Modified: clang-tools-extra/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/CMakeLists.txt?rev=343474&r1=343473&r2=343474&view=diff
==
--- clang-tools-extra/trunk/test/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/test/CMakeLists.txt Mon Oct  1 07:02:02 2018
@@ -43,7 +43,6 @@ set(CLANG_TOOLS_TEST_DEPS
   # Individual tools we test.
   clang-apply-replacements
   clang-change-namespace
-  clangd
   clang-doc
   clang-include-fixer
   clang-move
@@ -53,10 +52,7 @@ set(CLANG_TOOLS_TEST_DEPS
   modularize
   pp-trace
 
-  # These individual tools have no tests, add them here to make them compile
-  # together with check-clang-tools, so that we won't break them in the future.
-  clangd-indexer
-  dexp
+  check-clangd
 
   # Unit tests
   ExtraToolsUnitTests
@@ -73,16 +69,6 @@ if(CLANG_ENABLE_STATIC_ANALYZER)
 )
 endif()
 
-set(llvm_utils
-  FileCheck count not
-  )
-
-foreach(t ${llvm_utils})
-  if(TARGET ${t})
-list(APPEND CLANG_TOOLS_TEST_DEPS ${t})
-  endif()
-endforeach()
-
 add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression 
tests"
   ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
@@ -90,3 +76,18 @@ add_lit_testsuite(check-clang-tools "Run
   )
 
 set_target_properties(check-clang-tools PROPERTIES FOLDER "Clang extra tools' 
tests")
+
+# Setup an individual test for building and testing clangd-only stuff.
+set(CLANGD_TEST_DEPS
+  clangd
+  ClangdTests
+  # clangd-related tools which don't have tests, add them to the test to make
+  # sure we don't introduce new changes that break their compilations.
+  clangd-indexer
+  dexp
+)
+add_lit_testsuite(check-clangd "Running the Clangd regression tests"
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/clangd;${CMAKE_CURRENT_BINARY_DIR}/clangd
+  DEPENDS ${CLANGD_TEST_DEPS}
+)
+set_target_properties(check-clangd PROPERTIES FOLDER "Clangd tests")


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

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343474: [clangd] Add "check-clangd" target 
(authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D52710

Files:
  clang-tools-extra/trunk/test/CMakeLists.txt


Index: clang-tools-extra/trunk/test/CMakeLists.txt
===
--- clang-tools-extra/trunk/test/CMakeLists.txt
+++ clang-tools-extra/trunk/test/CMakeLists.txt
@@ -43,7 +43,6 @@
   # Individual tools we test.
   clang-apply-replacements
   clang-change-namespace
-  clangd
   clang-doc
   clang-include-fixer
   clang-move
@@ -53,10 +52,7 @@
   modularize
   pp-trace
 
-  # These individual tools have no tests, add them here to make them compile
-  # together with check-clang-tools, so that we won't break them in the future.
-  clangd-indexer
-  dexp
+  check-clangd
 
   # Unit tests
   ExtraToolsUnitTests
@@ -73,20 +69,25 @@
 )
 endif()
 
-set(llvm_utils
-  FileCheck count not
-  )
-
-foreach(t ${llvm_utils})
-  if(TARGET ${t})
-list(APPEND CLANG_TOOLS_TEST_DEPS ${t})
-  endif()
-endforeach()
-
 add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression 
tests"
   ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   ARGS ${CLANG_TOOLS_TEST_EXTRA_ARGS}
   )
 
 set_target_properties(check-clang-tools PROPERTIES FOLDER "Clang extra tools' 
tests")
+
+# Setup an individual test for building and testing clangd-only stuff.
+set(CLANGD_TEST_DEPS
+  clangd
+  ClangdTests
+  # clangd-related tools which don't have tests, add them to the test to make
+  # sure we don't introduce new changes that break their compilations.
+  clangd-indexer
+  dexp
+)
+add_lit_testsuite(check-clangd "Running the Clangd regression tests"
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/clangd;${CMAKE_CURRENT_BINARY_DIR}/clangd
+  DEPENDS ${CLANGD_TEST_DEPS}
+)
+set_target_properties(check-clangd PROPERTIES FOLDER "Clangd tests")


Index: clang-tools-extra/trunk/test/CMakeLists.txt
===
--- clang-tools-extra/trunk/test/CMakeLists.txt
+++ clang-tools-extra/trunk/test/CMakeLists.txt
@@ -43,7 +43,6 @@
   # Individual tools we test.
   clang-apply-replacements
   clang-change-namespace
-  clangd
   clang-doc
   clang-include-fixer
   clang-move
@@ -53,10 +52,7 @@
   modularize
   pp-trace
 
-  # These individual tools have no tests, add them here to make them compile
-  # together with check-clang-tools, so that we won't break them in the future.
-  clangd-indexer
-  dexp
+  check-clangd
 
   # Unit tests
   ExtraToolsUnitTests
@@ -73,20 +69,25 @@
 )
 endif()
 
-set(llvm_utils
-  FileCheck count not
-  )
-
-foreach(t ${llvm_utils})
-  if(TARGET ${t})
-list(APPEND CLANG_TOOLS_TEST_DEPS ${t})
-  endif()
-endforeach()
-
 add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression tests"
   ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   ARGS ${CLANG_TOOLS_TEST_EXTRA_ARGS}
   )
 
 set_target_properties(check-clang-tools PROPERTIES FOLDER "Clang extra tools' tests")
+
+# Setup an individual test for building and testing clangd-only stuff.
+set(CLANGD_TEST_DEPS
+  clangd
+  ClangdTests
+  # clangd-related tools which don't have tests, add them to the test to make
+  # sure we don't introduce new changes that break their compilations.
+  clangd-indexer
+  dexp
+)
+add_lit_testsuite(check-clangd "Running the Clangd regression tests"
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/clangd;${CMAKE_CURRENT_BINARY_DIR}/clangd
+  DEPENDS ${CLANGD_TEST_DEPS}
+)
+set_target_properties(check-clangd PROPERTIES FOLDER "Clangd tests")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Sam McCall via Phabricator via cfe-commits

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/Lex/PPDirectives.cpp:1895
+auto CorrectTypoFilename = [](llvm::StringRef Filename) {
+  while (!Filename.empty() && !isAlphanumeric(Filename.front())) {
+Filename = Filename.drop_front();

`Filename = Filename.drop_until(isAlphanumeric)`

(unfortunately there's no equivalent "back" version)





Comment at: lib/Lex/PPDirectives.cpp:1898
+  }
+  if (Filename.empty())
+return Filename;

simplify the logic by merging with the while loop? (and drop the assert)



Comment at: lib/Lex/PPDirectives.cpp:1909
+File = Filename.empty()
+   ? nullptr
+   : LookupFile(FilenameLoc,

if this early bail-out isn't necessary, please drop it - it's optimizing an 
extremely uncommon case


Repository:
  rC Clang

https://reviews.llvm.org/D52721



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

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Kristina Brooks via Phabricator via cfe-commits

kristina added a comment.

Have you tested that build is successful and obviously the FileCheck test 
should pass now? What sort of behavior does this produce when you attempt to 
use an invalid path like that now when compiling with clang? Just a predecessor 
diagnostic?

I'm updating software on my buildbot so I can't really do a quick run to test 
it right now, though if you definitely know it compiles and the test passes, 
I'm happy to approve this.




Comment at: lib/Lex/PPDirectives.cpp:1892
   if (!File) {
-while (!isAlphanumeric(Filename.front())) {
+while (!Filename.empty() && !isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();

hokein wrote:
> kristina wrote:
> > kristina wrote:
> > > This line is tripping the assert, it seems best course of action would be 
> > > a single check here and then just diagnosing an error unless you have 
> > > managed to find other cases, in which case all the checks below are also 
> > > warranted.
> > In either case, the diagnostic emitted doesn't really make sense, at least 
> > to me, I think it would be better to explicitly diagnose this case as an 
> > error and then bail, before an assertion fires.
> > 
> > I also crashed clang with `#include "./"`, so the test case does seem to be 
> > fairly minimal which is good. Though I think a diagnostic about a bogus 
> > file path would be better (I don't know how to word it well), rather than 
> > saying file not found.
> Thanks for the comment.
> 
> IIUC, do you mean we create a new diagnostic message for this specific case? 
> I'm double that it is worth doing it, since IMO this is a heuristic that 
> getting a typo file path, and user input is arbitrary, make the diagnostic 
> fit all users input would be very tricky.
> 
> I think we can just provide best-effort for this case (e.g. no crash), 
> "err_pp_file_not_found" seems the best one from existing messages. 
> 
> I checked with `g++,` it provides a slightly better message ("fatal error: 
> ./: No such file or directory" vs clang's "'./' file not found").
> 
Hm, I think it's up to you if you want to make a new diagnostic or reuse one 
that's similar enough, as long as you emit it and then bail out before you hit 
the assertion, that should fix the bug. I like the new asserts as well.


Repository:
  rC Clang

https://reviews.llvm.org/D52721



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

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Kristina Brooks via Phabricator via cfe-commits

kristina added inline comments.



Comment at: lib/Lex/PPDirectives.cpp:1898
+  }
+  if (Filename.empty())
+return Filename;

sammccall wrote:
> simplify the logic by merging with the while loop? (and drop the assert)
I thought the assert was a good idea in case a similar issue popped up again 
(somehow triggering this but in reverse, not sure if that makes sense), it's a 
no-op in upstream release builds anyway.


Repository:
  rC Clang

https://reviews.llvm.org/D52721



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

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: lib/Lex/PPDirectives.cpp:1898
+  }
+  if (Filename.empty())
+return Filename;

kristina wrote:
> sammccall wrote:
> > simplify the logic by merging with the while loop? (and drop the assert)
> I thought the assert was a good idea in case a similar issue popped up again 
> (somehow triggering this but in reverse, not sure if that makes sense), it's 
> a no-op in upstream release builds anyway.
`while (!empty && !isAlpha(back)) { drop_back }`
is just as safe, and easier to understand than
```
// ... subtly establish that there is an alphanum in the string ...
while (!isAlpha(back)) { drop_back; assert(!empty) }
```


Repository:
  rC Clang

https://reviews.llvm.org/D52721



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

[PATCH] D51531: [analyzer][UninitializedObjectChecker] Uninit regions are only reported once

2018-10-01 Thread Umann Kristóf via Phabricator via cfe-commits

Szelethus added a comment.

Ping ^-^


https://reviews.llvm.org/D51531



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

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Kristina Brooks via Phabricator via cfe-commits

kristina accepted this revision.
kristina added a comment.

Yes I think the clangd crash will have to go in another diff, but this fixes 
the crash in clang, which is pretty good in itself. I don't build clangd at the 
moment and have no buildbot available so I can try to look into it later if you 
can open a Bugzilla thing with steps to reproduce maybe unless Sam wants to dig 
into it?


Repository:
  rC Clang

https://reviews.llvm.org/D52721



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

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Kristina Brooks via Phabricator via cfe-commits

kristina added a comment.

Please make sure it builds after you update the revision, I usually do it 
myself but it'll take too long on my desktop and I accidentally broke the 
hypervisor on my buildbot so can't do it this time.




Comment at: lib/Lex/PPDirectives.cpp:1898
+  }
+  if (Filename.empty())
+return Filename;

sammccall wrote:
> kristina wrote:
> > sammccall wrote:
> > > simplify the logic by merging with the while loop? (and drop the assert)
> > I thought the assert was a good idea in case a similar issue popped up 
> > again (somehow triggering this but in reverse, not sure if that makes 
> > sense), it's a no-op in upstream release builds anyway.
> `while (!empty && !isAlpha(back)) { drop_back }`
> is just as safe, and easier to understand than
> ```
> // ... subtly establish that there is an alphanum in the string ...
> while (!isAlpha(back)) { drop_back; assert(!empty) }
> ```
Fair enough, that would be better. Don't think I have anything else to add, I 
think this revision just requires addressing that and then it's good to go, but 
see my comment on `clangd`. 


Repository:
  rC Clang

https://reviews.llvm.org/D52721



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

[PATCH] D29660: [OpenMP] Add flag for overwriting default PTX version for OpenMP targets

2018-10-01 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits

gtbercea abandoned this revision.
gtbercea added a comment.

In https://reviews.llvm.org/D29660#1250333, @Hahnfeld wrote:

> Going through my list of reviews, this patch was reverted because of memory 
> leaks in other changes. However, I don't think we need this anymore because 
> Clang is raising the PTX level as needed for that CUDA version. Can we 
> abandon this flag?


You are correct. I'll close this.


Repository:
  rL LLVM

https://reviews.llvm.org/D29660



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

r343479 - [OPENMP] Fix enum identifier, NFC.

2018-10-01 Thread Alexey Bataev via cfe-commits

Author: abataev
Date: Mon Oct  1 07:26:31 2018
New Revision: 343479

URL: http://llvm.org/viewvc/llvm-project?rev=343479&view=rev
Log:
[OPENMP] Fix enum identifier, NFC.

Modified:
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp

Modified: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp?rev=343479&r1=343478&r2=343479&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp Mon Oct  1 07:26:31 2018
@@ -3912,7 +3912,7 @@ static void emitOMPAtomicExpr(CodeGenFun
   case OMPC_use_device_ptr:
   case OMPC_is_device_ptr:
   case OMPC_unified_address:
-  case OMP_unified_shared_memory:
+  case OMPC_unified_shared_memory:
 llvm_unreachable("Clause is not allowed in 'omp atomic'.");
   }
 }


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

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 167737.
hokein marked 3 inline comments as done.
hokein added a comment.

Address review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D52721

Files:
  lib/Lex/PPDirectives.cpp
  test/Preprocessor/include-nonalpha-no-crash.c


Index: test/Preprocessor/include-nonalpha-no-crash.c
===
--- /dev/null
+++ test/Preprocessor/include-nonalpha-no-crash.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "./" // expected-error {{'./' file not found}}
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1889,13 +1889,16 @@
   // characters
   StringRef OriginalFilename = Filename;
   if (!File) {
-while (!isAlphanumeric(Filename.front())) {
-  Filename = Filename.drop_front();
-}
-while (!isAlphanumeric(Filename.back())) {
-  Filename = Filename.drop_back();
-}
-
+// A heuristic to correct a typo file name by removing leading and
+// trailing non-isAlphanumeric characters.
+auto CorrectTypoFilename = [](llvm::StringRef Filename) {
+  Filename = Filename.drop_until(isAlphanumeric);
+  while (!Filename.empty() && !isAlphanumeric(Filename.back())) {
+Filename = Filename.drop_back();
+  }
+  return Filename;
+};
+Filename = CorrectTypoFilename(Filename);
 File = LookupFile(
 FilenameLoc,
 LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,


Index: test/Preprocessor/include-nonalpha-no-crash.c
===
--- /dev/null
+++ test/Preprocessor/include-nonalpha-no-crash.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "./" // expected-error {{'./' file not found}}
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1889,13 +1889,16 @@
   // characters
   StringRef OriginalFilename = Filename;
   if (!File) {
-while (!isAlphanumeric(Filename.front())) {
-  Filename = Filename.drop_front();
-}
-while (!isAlphanumeric(Filename.back())) {
-  Filename = Filename.drop_back();
-}
-
+// A heuristic to correct a typo file name by removing leading and
+// trailing non-isAlphanumeric characters.
+auto CorrectTypoFilename = [](llvm::StringRef Filename) {
+  Filename = Filename.drop_until(isAlphanumeric);
+  while (!Filename.empty() && !isAlphanumeric(Filename.back())) {
+Filename = Filename.drop_back();
+  }
+  return Filename;
+};
+Filename = CorrectTypoFilename(Filename);
 File = LookupFile(
 FilenameLoc,
 LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

I verified this patch is passed all clang tests (`ninja check-clang`).


Repository:
  rC Clang

https://reviews.llvm.org/D52721



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

[PATCH] D52673: [HIP] Remove disabled irif library

2018-10-01 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added a comment.

this seems to be duplicate of https://reviews.llvm.org/D51857

Is HIP github ready for this change?


Repository:
  rC Clang

https://reviews.llvm.org/D52673



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

r343481 - [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Haojian Wu via cfe-commits

Author: hokein
Date: Mon Oct  1 07:38:43 2018
New Revision: 343481

URL: http://llvm.org/viewvc/llvm-project?rev=343481&view=rev
Log:
[Preprocessor] Fix a crash when handling non-alpha include header.

Summary: the crash is casued by an assertion in StringRef.
(llvm::StringRef::front() const: Assertion `!empty()' failed.)

Reviewers: sammccall

Subscribers: jsji, cfe-commits

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

Added:
cfe/trunk/test/Preprocessor/include-nonalpha-no-crash.c
Modified:
cfe/trunk/lib/Lex/PPDirectives.cpp

Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=343481&r1=343480&r2=343481&view=diff
==
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Mon Oct  1 07:38:43 2018
@@ -1889,13 +1889,16 @@ void Preprocessor::HandleIncludeDirectiv
   // characters
   StringRef OriginalFilename = Filename;
   if (!File) {
-while (!isAlphanumeric(Filename.front())) {
-  Filename = Filename.drop_front();
-}
-while (!isAlphanumeric(Filename.back())) {
-  Filename = Filename.drop_back();
-}
-
+// A heuristic to correct a typo file name by removing leading and
+// trailing non-isAlphanumeric characters.
+auto CorrectTypoFilename = [](llvm::StringRef Filename) {
+  Filename = Filename.drop_until(isAlphanumeric);
+  while (!Filename.empty() && !isAlphanumeric(Filename.back())) {
+Filename = Filename.drop_back();
+  }
+  return Filename;
+};
+Filename = CorrectTypoFilename(Filename);
 File = LookupFile(
 FilenameLoc,
 LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,

Added: cfe/trunk/test/Preprocessor/include-nonalpha-no-crash.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/include-nonalpha-no-crash.c?rev=343481&view=auto
==
--- cfe/trunk/test/Preprocessor/include-nonalpha-no-crash.c (added)
+++ cfe/trunk/test/Preprocessor/include-nonalpha-no-crash.c Mon Oct  1 07:38:43 
2018
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "./" // expected-error {{'./' file not found}}


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

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC343481: [Preprocessor] Fix a crash when handling non-alpha 
include header. (authored by hokein, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52721?vs=167737&id=167738#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52721

Files:
  lib/Lex/PPDirectives.cpp
  test/Preprocessor/include-nonalpha-no-crash.c


Index: test/Preprocessor/include-nonalpha-no-crash.c
===
--- test/Preprocessor/include-nonalpha-no-crash.c
+++ test/Preprocessor/include-nonalpha-no-crash.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "./" // expected-error {{'./' file not found}}
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1889,13 +1889,16 @@
   // characters
   StringRef OriginalFilename = Filename;
   if (!File) {
-while (!isAlphanumeric(Filename.front())) {
-  Filename = Filename.drop_front();
-}
-while (!isAlphanumeric(Filename.back())) {
-  Filename = Filename.drop_back();
-}
-
+// A heuristic to correct a typo file name by removing leading and
+// trailing non-isAlphanumeric characters.
+auto CorrectTypoFilename = [](llvm::StringRef Filename) {
+  Filename = Filename.drop_until(isAlphanumeric);
+  while (!Filename.empty() && !isAlphanumeric(Filename.back())) {
+Filename = Filename.drop_back();
+  }
+  return Filename;
+};
+Filename = CorrectTypoFilename(Filename);
 File = LookupFile(
 FilenameLoc,
 LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,


Index: test/Preprocessor/include-nonalpha-no-crash.c
===
--- test/Preprocessor/include-nonalpha-no-crash.c
+++ test/Preprocessor/include-nonalpha-no-crash.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "./" // expected-error {{'./' file not found}}
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1889,13 +1889,16 @@
   // characters
   StringRef OriginalFilename = Filename;
   if (!File) {
-while (!isAlphanumeric(Filename.front())) {
-  Filename = Filename.drop_front();
-}
-while (!isAlphanumeric(Filename.back())) {
-  Filename = Filename.drop_back();
-}
-
+// A heuristic to correct a typo file name by removing leading and
+// trailing non-isAlphanumeric characters.
+auto CorrectTypoFilename = [](llvm::StringRef Filename) {
+  Filename = Filename.drop_until(isAlphanumeric);
+  while (!Filename.empty() && !isAlphanumeric(Filename.back())) {
+Filename = Filename.drop_back();
+  }
+  return Filename;
+};
+Filename = CorrectTypoFilename(Filename);
 File = LookupFile(
 FilenameLoc,
 LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r343483 - [OPENMP] Simplify code, NFC.

2018-10-01 Thread Alexey Bataev via cfe-commits

Author: abataev
Date: Mon Oct  1 07:40:06 2018
New Revision: 343483

URL: http://llvm.org/viewvc/llvm-project?rev=343483&view=rev
Log:
[OPENMP] Simplify code, NFC.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=343483&r1=343482&r2=343483&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Mon Oct  1 07:40:06 2018
@@ -781,10 +781,8 @@ static bool supportsSPMDExecutionMode(AS
   case OMPD_target_parallel:
   case OMPD_target_parallel_for:
   case OMPD_target_parallel_for_simd:
-return !hasParallelIfNumThreadsClause(Ctx, D);
   case OMPD_target_teams_distribute_parallel_for:
   case OMPD_target_teams_distribute_parallel_for_simd:
-// Distribute with lastprivates requires non-SPMD execution mode.
 return !hasParallelIfNumThreadsClause(Ctx, D);
   case OMPD_target_simd:
   case OMPD_target_teams_distribute:


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

[PATCH] D52673: [HIP] Remove disabled irif library

2018-10-01 Thread Aaron Enye Shi via Phabricator via cfe-commits

ashi1 updated this revision to Diff 167741.
ashi1 added a comment.

Added diff with full context. Also, I need to find replacements for few 
functions in HIP github before this can be submitted.


https://reviews.llvm.org/D52673

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


Index: test/Driver/hip-device-libs.hip
===
--- test/Driver/hip-device-libs.hip
+++ test/Driver/hip-device-libs.hip
@@ -21,8 +21,8 @@


 // COM: [[LLVM_LINK:"*.llvm-link"]]
-// COM-SAME: {{.*}} "{{.*}}ocml.amdgcn.bc" "{{.*}}ockl.amdgcn.bc" 
"{{.*}}irif.amdgcn.bc"
+// COM-SAME: {{.*}} "{{.*}}ocml.amdgcn.bc" "{{.*}}ockl.amdgcn.bc"
 // FLUSHD-SAME: {{.*}} "{{.*}}oclc_daz_opt_on.amdgcn.bc"
 // NOFLUSHD-SAME: {{.*}} "{{.*}}oclc_daz_opt_off.amdgcn.bc"
 // COM-SAME: {{.*}} "-o" "{{.*}}-gfx900-linked-{{.*bc}}"

Index: lib/Driver/ToolChains/HIP.cpp
===
--- lib/Driver/ToolChains/HIP.cpp
+++ lib/Driver/ToolChains/HIP.cpp
@@ -82,15 +82,15 @@
   FlushDenormalControlBC = "oclc_daz_opt_off.amdgcn.bc";

 BCLibs.append({"opencl.amdgcn.bc",
-   "ocml.amdgcn.bc", "ockl.amdgcn.bc", "irif.amdgcn.bc",
+   "ocml.amdgcn.bc", "ockl.amdgcn.bc",
"oclc_finite_only_off.amdgcn.bc",
FlushDenormalControlBC,
"oclc_correctly_rounded_sqrt_on.amdgcn.bc",
"oclc_unsafe_math_off.amdgcn.bc", ISAVerBC});
   }
   for (auto Lib : BCLibs)
 addBCLib(C, Args, CmdArgs, LibraryPaths, Lib);

   // Add an intermediate output file.
   CmdArgs.push_back("-o");
   std::string TmpName =


Index: test/Driver/hip-device-libs.hip
===
--- test/Driver/hip-device-libs.hip
+++ test/Driver/hip-device-libs.hip
@@ -21,8 +21,8 @@


 // COM: [[LLVM_LINK:"*.llvm-link"]]
-// COM-SAME: {{.*}} "{{.*}}ocml.amdgcn.bc" "{{.*}}ockl.amdgcn.bc" "{{.*}}irif.amdgcn.bc"
+// COM-SAME: {{.*}} "{{.*}}ocml.amdgcn.bc" "{{.*}}ockl.amdgcn.bc"
 // FLUSHD-SAME: {{.*}} "{{.*}}oclc_daz_opt_on.amdgcn.bc"
 // NOFLUSHD-SAME: {{.*}} "{{.*}}oclc_daz_opt_off.amdgcn.bc"
 // COM-SAME: {{.*}} "-o" "{{.*}}-gfx900-linked-{{.*bc}}"

Index: lib/Driver/ToolChains/HIP.cpp
===
--- lib/Driver/ToolChains/HIP.cpp
+++ lib/Driver/ToolChains/HIP.cpp
@@ -82,15 +82,15 @@
   FlushDenormalControlBC = "oclc_daz_opt_off.amdgcn.bc";

 BCLibs.append({"opencl.amdgcn.bc",
-   "ocml.amdgcn.bc", "ockl.amdgcn.bc", "irif.amdgcn.bc",
+   "ocml.amdgcn.bc", "ockl.amdgcn.bc",
"oclc_finite_only_off.amdgcn.bc",
FlushDenormalControlBC,
"oclc_correctly_rounded_sqrt_on.amdgcn.bc",
"oclc_unsafe_math_off.amdgcn.bc", ISAVerBC});
   }
   for (auto Lib : BCLibs)
 addBCLib(C, Args, CmdArgs, LibraryPaths, Lib);

   // Add an intermediate output file.
   CmdArgs.push_back("-o");
   std::string TmpName =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

2018-10-01 Thread Krasimir Georgiev via Phabricator via cfe-commits

krasimir added a comment.

In https://reviews.llvm.org/D52676#1250124, @oleg.smolsky wrote:

> In https://reviews.llvm.org/D52676#1250071, @krasimir wrote:
>
> > In https://reviews.llvm.org/D52676#1249828, @oleg.smolsky wrote:
> >
> > > In https://reviews.llvm.org/D52676#1249784, @krasimir wrote:
> > >
> > > > IMO `BinPackArguments==false` does not imply that there should be a 
> > > > line break before the first arguments, only that there should not be 
> > > > two arguments from the same argument list that appear on the same line.
> > >
> > >
> > > That's right. However consider the following points:
> > >
> > > 1. a lambda function is already placed onto its own line when it is the 
> > > first arg (as you can see in the first test)
> >
> >
> > I believe that a newline before a first arg lambda is controlled by a 
> > different mechanism independent of bin packing, probably by a rule that is 
> > more general than just "newline before first arg lambda". I think djasper@ 
> > could know more about this case.
>
>
> Hmm... perhaps. I have not been able to find an explicit rule for that.
>
> >> 1. the other args are placed onto a distinct line
> >> 2. this behavior looks very close to "bin packing"
> >> 3. adding a new option for the minor cases seems to be an overkill
> >> 
> >>   Having said that, I can add a new explicit style option. Do you think 
> >> that will improve consensus? Would you expect test cases for positive and 
> >> negative values of the option?


Not really. It's generally hard to add a new style option, as that is mandated 
by an existing commonly used public style guide that requires it.

>> is inconsistent, as not adding a newline before the first argument is 
>> typical, as in:
>> 
>>   $ clang-format -style='{BasedOnStyle: google, BinPackArguments: false}' 
>> test.cc ~
>>   void f() {
>> something->Method2s("11",
>> 
>> "",
>> 3);
>>   }
> 
> Right, it's consistent with your example but inconsistent with with the 
> following two:
> 
> lambda at arg0:
> 
>   void f() {
> something->Method2s(
> [this] {
>   Do1();
>   Do2();
> },
> 1);
>   }
> 
> 
> and two lambdas:
> 
>   // Multiple lambdas in the same parentheses change indentation rules.
>   verifyFormat("SomeFunction(\n"
>"[]() {\n"
>"  int i = 42;\n"
>"  return i;\n"
>"},\n"
>"[]() {\n"
>"  int j = 43;\n"
>"  return j;\n"
>"});");
>

The behavior you're observing here is a consequence of a more general rule: if 
the first argument is put on a new line, it's indented +4 spaces (I believe 
it's ContinuationIndentWidth) from the "current line"'s indent, for example 
also in:

  int f() {
(

""
"bbb",
2);
  }


Repository:
  rC Clang

https://reviews.llvm.org/D52676



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

[PATCH] D52664: Update CMakeLists.txt snippet so that example compiles

2018-10-01 Thread Dan Zimmerman via Phabricator via cfe-commits

danzimm added a comment.

@steveire can you land this for me? I don't have commit access


Repository:
  rC Clang

https://reviews.llvm.org/D52664



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

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

2018-10-01 Thread Alexander Kornienko via Phabricator via cfe-commits

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

Comment at: clang-tidy/tool/clang-tidy-diff.py:123-130
   if args.fix:
 command.append('-fix')
   if args.checks != '':
 command.append('-checks=' + quote + args.checks + quote)
   if args.quiet:
 command.append('-quiet')
   if args.build_path is not None:

janosimas wrote:
> alexfh wrote:
> > janosimas wrote:
> > > janosimas wrote:
> > > > alexfh wrote:
> > > > > If we make the script leave out the `--` flag, we should stop 
> > > > > forwarding these flags and the `extra_arg(_before)?` below. Otherwise 
> > > > > it's too confusing (should one place -fix before `--` or after? what 
> > > > > about `-warnings-as-errors`?).
> > > > > 
> > > > > Please also update the usage example at the top.
> > > > What about keep the current `--` behavior and add a new flag 
> > > > `-extra-tidy-flags` ? 
> > > `-extra-tidy-arg` to maintain consistency.
> > What's the benefit of `-extra-tidy-arg` compared to pass-by arguments?
> `-extra-tidy-arg` would keep the current `--` behavior and cover other cases 
> of extra flags not supported by this script.
> I was worried about how changing the behavior would affect people that 
> already using it.
> 
> -extra-tidy-arg would keep the current -- behavior and cover other cases of 
> extra flags not supported by this script.
> I was worried about how changing the behavior would affect people that 
> already using it.

Good point about backward compatibility, however it may be a good time to reset 
the complexity at the expense of breaking the interface. Luckily, call-sites 
will be easy to upgrade.

As you may have noticed, the system is already quite confusing. At the very 
beginning the script tried to mimic clang-tidy command-line interface with an 
addition of diff file parsing (which is translated to -line-filter=). 
Gradually, the script got more and more options - some of them were just 
forwarded to clang-tidy, some were needed for the script. Some options started 
conflicting (e.g. the script's -p and clang-tidy's -p) and had to be given 
different names (-path -> -p).

At this point adding another way to pass options to clang-tidy will just make 
this all even more confusing. So I'm suggesting to clearly split the script's 
options and clang-tidy's options:

  clang-tidy-diff.py [-clang-tidy-binary ...] [-p ...] [-regex ...] [-iregex 
...] -- [-fix] [-checks=...] [other clang-tidy options]

If there's a need to run clang-tidy with the fixed compilation database, the 
second `--` will be needed, e.g.:

  git diff -U0 HEAD^ | clang-tidy-diff.py -p1 -- -fix -- -std=c++11

The good thing is that script won't have to do anything extra to enable this 
and that it's very easy to explain: everything after the first `--` is just 
passed to clang-tidy verbatim.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49864

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

[PATCH] D52377: [HIP] Support early finalization of device code

2018-10-01 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added a comment.

In https://reviews.llvm.org/D52377#1242547, @tra wrote:

> Overall the patch look OK. I'll take a closer look on Monday.
>
> Which mode do you expect will be most commonly used for HIP by default? With 
> this patch we'll have two different ways to do similar things in HIP vs. CUDA.
>  E.g. by default CUDA compiles GPU code in each TU in a complete executable 
> and requires -fcuda-rdc to compile to GPU object file.
>  HIP defaults to object-file compilation and requires --hip-early-finalize to 
> match CUDA's default behavior.
>
> I wonder if it would make sense to provide a single way to control this 
> behavior. E.g. `--fgpu-rdc` (an alias for -cuda-rdc, perhaps?) would default 
> to true in HIP, but disabled in CUDA. `-fno-gpu-rdc` would force 'whole GPU 
> executable per TU' mode.


Agree that --fgpu-rdc and -fno-gpu-rdc are better names of the options. I will 
make changes to use these options.

For the default option, we will use -fno-gpu-rdc to be consistent with 
cuda-clang.


https://reviews.llvm.org/D52377



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

[PATCH] D51429: [AArch64] Return Address Signing B Key Support

2018-10-01 Thread Javed Absar via Phabricator via cfe-commits

javed.absar added inline comments.



Comment at: include/clang/Frontend/CodeGenOptions.h:117
 
+  enum SignReturnAddressKeyValue { AKey, BKey };
+

Perhaps a line of comment on each enum-value would be useful (much like the 
code around).


https://reviews.llvm.org/D51429



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

[PATCH] D52726: [clangd] Support refs() in dex. Largely cloned from MemIndex.

2018-10-01 Thread Sam McCall via Phabricator via cfe-commits

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

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52726

Files:
  clangd/index/Serialization.cpp
  clangd/index/dex/Dex.cpp
  clangd/index/dex/Dex.h
  clangd/indexer/IndexerMain.cpp
  unittests/clangd/DexTests.cpp

Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -411,7 +411,8 @@
 //===--===//
 
 TEST(Dex, Lookup) {
-  auto I = Dex::build(generateSymbols({"ns::abc", "ns::xyz"}), URISchemes);
+  auto I = Dex::build(generateSymbols({"ns::abc", "ns::xyz"}), RefSlab(),
+  URISchemes);
   EXPECT_THAT(lookup(*I, SymbolID("ns::abc")), UnorderedElementsAre("ns::abc"));
   EXPECT_THAT(lookup(*I, {SymbolID("ns::abc"), SymbolID("ns::xyz")}),
   UnorderedElementsAre("ns::abc", "ns::xyz"));
@@ -424,7 +425,7 @@
   auto Index =
   Dex::build(generateSymbols({"ns::ABC", "ns::BCD", "::ABC",
   "ns::nested::ABC", "other::ABC", "other::A"}),
- URISchemes);
+ RefSlab(), URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "ABC";
   Req.Scopes = {"ns::"};
@@ -454,13 +455,13 @@
  symbol("2") /* duplicate */};
   FuzzyFindRequest Req;
   Req.Query = "2";
-  Dex I(Symbols, URISchemes);
+  Dex I(Symbols, RefSlab(), URISchemes);
   EXPECT_FALSE(Req.Limit);
   EXPECT_THAT(match(I, Req), ElementsAre("2", "2"));
 }
 
 TEST(DexTest, DexLimitedNumMatches) {
-  auto I = Dex::build(generateNumSymbols(0, 100), URISchemes);
+  auto I = Dex::build(generateNumSymbols(0, 100), RefSlab(), URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "5";
   Req.Limit = 3;
@@ -474,58 +475,61 @@
 TEST(DexTest, FuzzyMatch) {
   auto I = Dex::build(
   generateSymbols({"LaughingOutLoud", "LionPopulation", "LittleOldLady"}),
-  URISchemes);
+  RefSlab(), URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "lol";
   Req.Limit = 2;
   EXPECT_THAT(match(*I, Req),
   UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
 }
 
 TEST(DexTest, MatchQualifiedNamesWithoutSpecificScope) {
-  auto I = Dex::build(generateSymbols({"a::y1", "b::y2", "y3"}), URISchemes);
+  auto I = Dex::build(generateSymbols({"a::y1", "b::y2", "y3"}), RefSlab(),
+  URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "y";
   EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1", "b::y2", "y3"));
 }
 
 TEST(DexTest, MatchQualifiedNamesWithGlobalScope) {
-  auto I = Dex::build(generateSymbols({"a::y1", "b::y2", "y3"}), URISchemes);
+  auto I = Dex::build(generateSymbols({"a::y1", "b::y2", "y3"}), RefSlab(),
+  URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {""};
   EXPECT_THAT(match(*I, Req), UnorderedElementsAre("y3"));
 }
 
 TEST(DexTest, MatchQualifiedNamesWithOneScope) {
-  auto I = Dex::build(
-  generateSymbols({"a::y1", "a::y2", "a::x", "b::y2", "y3"}), URISchemes);
+  auto I =
+  Dex::build(generateSymbols({"a::y1", "a::y2", "a::x", "b::y2", "y3"}),
+ RefSlab(), URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {"a::"};
   EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1", "a::y2"));
 }
 
 TEST(DexTest, MatchQualifiedNamesWithMultipleScopes) {
   auto I = Dex::build(
-  generateSymbols({"a::y1", "a::y2", "a::x", "b::y3", "y3"}), URISchemes);
+  generateSymbols({"a::y1", "a::y2", "a::x", "b::y3", "y3"}), RefSlab(), URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {"a::", "b::"};
   EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1", "a::y2", "b::y3"));
 }
 
 TEST(DexTest, NoMatchNestedScopes) {
-  auto I = Dex::build(generateSymbols({"a::y1", "a::b::y2"}), URISchemes);
+  auto I = Dex::build(generateSymbols({"a::y1", "a::b::y2"}), RefSlab(), URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {"a::"};
   EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1"));
 }
 
 TEST(DexTest, WildcardScope) {
   auto I =
-  Dex::build(generateSymbols({"a::y1", "a::b::y2", "c::y3"}), URISchemes);
+  Dex::build(generateSymbols({"a::y1", "a::b::y2", "c::y3"}), RefSlab(), URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {"a::"};
@@ -535,15 +539,15 @@
 }
 
 TEST(DexTest, IgnoreCases) {
-  auto I = Dex::build(generateSymbols({"ns::ABC", "ns::abc"}), URISchemes);
+  auto I = Dex::build(generateSymbols({"ns::ABC", "ns::abc"}), RefSlab(), URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "AB";
   Req.Scopes = {"ns::"};
   EXPECT_THAT(match(*I, Req), UnorderedElementsAre("ns::ABC", "ns::abc"));
 }
 
 TEST(DexTest, Lookup) {
-  auto I = Dex::build(generate

[PATCH] D51429: [AArch64] Return Address Signing B Key Support

2018-10-01 Thread Luke Cheeseman via Phabricator via cfe-commits

LukeCheeseman added inline comments.



Comment at: include/clang/Frontend/CodeGenOptions.h:117
 
+  enum SignReturnAddressKeyValue { AKey, BKey };
+

javed.absar wrote:
> Perhaps a line of comment on each enum-value would be useful (much like the 
> code around).
I'm not sure what value there would be in adding extra comments here, I think 
the values are fairly self explanatory.


https://reviews.llvm.org/D51429



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

[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

2018-10-01 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: flx, alexfh, aaron.ballman, lebedev.ri.
baloghadamsoftware added a project: clang-tools-extra.

New option added to these three checks to be able to silence false positives on 
types that are intentionally passed by value or copied. Such types are e.g. 
intrusive reference counting pointer types like `llvm::IntrusiveRefCntPtr`. The 
new option is named `WhiteListTypes` and can contain a semicolon-separated list 
of names of these types. Regular expressions are allowed. Default is empty.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52727

Files:
  clang-tidy/performance/ForRangeCopyCheck.cpp
  clang-tidy/performance/ForRangeCopyCheck.h
  clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tidy/performance/UnnecessaryCopyInitialization.h
  clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tidy/performance/UnnecessaryValueParamCheck.h
  docs/clang-tidy/checks/performance-for-range-copy.rst
  docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
  docs/clang-tidy/checks/performance-unnecessary-value-param.rst
  test/clang-tidy/performance-for-range-copy.cpp
  test/clang-tidy/performance-unnecessary-copy-initialization.cpp
  test/clang-tidy/performance-unnecessary-value-param.cpp

Index: test/clang-tidy/performance-unnecessary-value-param.cpp
===
--- test/clang-tidy/performance-unnecessary-value-param.cpp
+++ test/clang-tidy/performance-unnecessary-value-param.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s performance-unnecessary-value-param %t
+// RUN: %check_clang_tidy %s performance-unnecessary-value-param %t -- -config="{CheckOptions: [{key: performance-unnecessary-value-param.WhiteListTypes, value: '[Pp]ointer$|[Pp]tr$|[Rr]ef(erence)?$'}]}" --
 
 // CHECK-FIXES: #include 
 
@@ -123,6 +123,8 @@
 void negativeArray(const ExpensiveToCopyType[]) {
 }
 
+// Test white list
+
 void negativePointer(ExpensiveToCopyType* Obj) {
 }
 
@@ -381,3 +383,59 @@
   // CHECK-FIXES: void templateFunction(ExpensiveToCopyType E) {
   E.constReference();
 }
+
+struct SmartPointer {
+  ~SmartPointer();
+};
+
+struct smart_pointer {
+  ~smart_pointer();
+};
+
+struct SmartPtr {
+  ~SmartPtr();
+};
+
+struct smart_ptr {
+  ~smart_ptr();
+};
+
+struct SmartReference {
+  ~SmartReference();
+};
+
+struct smart_reference {
+  ~smart_reference();
+};
+
+struct SmartRef {
+  ~SmartRef();
+};
+
+struct smart_ref {
+  ~smart_ref();
+};
+
+void negativeSmartPointer(SmartPointer P) {
+}
+
+void negative_smart_pointer(smart_pointer p) {
+}
+
+void negativeSmartPtr(SmartPtr P) {
+}
+
+void negative_smart_ptr(smart_ptr p) {
+}
+
+void negativeSmartReference(SmartReference R) {
+}
+
+void negative_smart_reference(smart_reference r) {
+}
+
+void negativeSmartRef(SmartRef R) {
+}
+
+void negative_smart_ref(smart_ref r) {
+}
Index: test/clang-tidy/performance-unnecessary-copy-initialization.cpp
===
--- test/clang-tidy/performance-unnecessary-copy-initialization.cpp
+++ test/clang-tidy/performance-unnecessary-copy-initialization.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t
+// RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t -- -config="{CheckOptions: [{key: performance-unnecessary-copy-initialization.WhiteListTypes, value: '[Pp]ointer$|[Pp]tr$|[Rr]ef(erence)?$'}]}" --
 
 struct ExpensiveToCopyType {
   ExpensiveToCopyType();
@@ -387,3 +387,78 @@
   for (const Element &E : Container()) {
   }
 }
+
+// Test white list
+
+struct SmartPointer {
+  ~SmartPointer();
+};
+
+struct smart_pointer {
+  ~smart_pointer();
+};
+
+struct SmartPtr {
+  ~SmartPtr();
+};
+
+struct smart_ptr {
+  ~smart_ptr();
+};
+
+struct SmartReference {
+  ~SmartReference();
+};
+
+struct smart_reference {
+  ~smart_reference();
+};
+
+struct SmartRef {
+  ~SmartRef();
+};
+
+struct smart_ref {
+  ~smart_ref();
+};
+
+const SmartPointer &getSmartPointer();
+const smart_pointer &get_smart_pointer();
+const SmartPtr &getSmartPtr();
+const smart_ptr &get_smart_ptr();
+const SmartReference &getSmartReference();
+const smart_reference &get_smart_reference();
+const SmartRef &getSmartRef();
+const smart_ref &get_smart_ref();
+
+void negativeSmartPointer() {
+  const auto P = getSmartPointer();
+}
+
+void negative_smart_pointer() {
+  const auto p = get_smart_pointer();
+}
+
+void negativeSmartPtr() {
+  const auto P = getSmartPtr();
+}
+
+void negative_smart_ptr() {
+  const auto p = get_smart_ptr();
+}
+
+void negativeSmartReference() {
+  const auto R = getSmartReference();
+}
+
+void negative_smart_reference() {
+  const auto r = get_smart_reference();
+}
+
+void negativeSmartRef() {
+  const auto R = getSmartRef();
+}
+
+void negative_smart_ref() {
+  const auto r = get_smart_ref();
+}
Index: test/clang-tidy/performance-for-range-copy.cpp

r343488 - [Basic] Update clang tests (really testing sys::fs) that broke with r343460

2018-10-01 Thread Sam McCall via cfe-commits

Author: sammccall
Date: Mon Oct  1 09:07:03 2018
New Revision: 343488

URL: http://llvm.org/viewvc/llvm-project?rev=343488&view=rev
Log:
[Basic] Update clang tests (really testing sys::fs) that broke with r343460

Modified:
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp

Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=343488&r1=343487&r2=343488&view=diff
==
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Mon Oct  1 09:07:03 2018
@@ -23,6 +23,9 @@
 using namespace clang;
 using namespace llvm;
 using llvm::sys::fs::UniqueID;
+using testing::ElementsAre;
+using testing::Pair;
+using testing::UnorderedElementsAre;
 
 namespace {
 struct DummyFile : public vfs::File {
@@ -418,30 +421,22 @@ TEST(VirtualFileSystemTest, BrokenSymlin
   ScopedDir _b(TestDirectory + "/b");
   ScopedLink _c("no_such_file", TestDirectory + "/c");
 
+  // Should get no iteration error, but a stat error for the broken symlinks.
+  std::map StatResults;
   std::error_code EC;
   for (vfs::directory_iterator I = FS->dir_begin(Twine(TestDirectory), EC), E;
I != E; I.increment(EC)) {
-// Skip broken symlinks.
-auto EC2 = std::make_error_code(std::errc::no_such_file_or_directory);
-if (EC == EC2) {
-  EC.clear();
-  continue;
-}
-// For bot debugging.
-if (EC) {
-  outs() << "Error code found:\n"
- << "EC value: " << EC.value() << "\n"
- << "EC category: " << EC.category().name()
- << "EC message: " << EC.message() << "\n";
-
-  outs() << "Error code tested for:\n"
- << "EC value: " << EC2.value() << "\n"
- << "EC category: " << EC2.category().name()
- << "EC message: " << EC2.message() << "\n";
-}
-ASSERT_FALSE(EC);
-EXPECT_TRUE(I->path() == _b);
+EXPECT_FALSE(EC);
+StatResults[sys::path::filename(I->path())] =
+FS->status(I->path()).getError();
   }
+  EXPECT_THAT(
+  StatResults,
+  ElementsAre(
+  Pair("a", 
std::make_error_code(std::errc::no_such_file_or_directory)),
+  Pair("b", std::error_code()),
+  Pair("c",
+   std::make_error_code(std::errc::no_such_file_or_directory;
 }
 #endif
 
@@ -500,45 +495,24 @@ TEST(VirtualFileSystemTest, BrokenSymlin
   ScopedDir _ddd(TestDirectory + "/d/d/d");
   ScopedLink _e("no_such_file", TestDirectory + "/e");
 
-  std::vector ExpectedBrokenSymlinks = {_a, _ba, _bc, _c, _e};
-  std::vector ExpectedNonBrokenSymlinks = {_b, _bb, _d, _dd, _ddd};
   std::vector VisitedBrokenSymlinks;
   std::vector VisitedNonBrokenSymlinks;
   std::error_code EC;
   for (vfs::recursive_directory_iterator I(*FS, Twine(TestDirectory), EC), E;
I != E; I.increment(EC)) {
-auto EC2 = std::make_error_code(std::errc::no_such_file_or_directory);
-if (EC == EC2) {
-  VisitedBrokenSymlinks.push_back(I->path());
-  continue;
-}
-// For bot debugging.
-if (EC) {
-  outs() << "Error code found:\n"
- << "EC value: " << EC.value() << "\n"
- << "EC category: " << EC.category().name()
- << "EC message: " << EC.message() << "\n";
-
-  outs() << "Error code tested for:\n"
- << "EC value: " << EC2.value() << "\n"
- << "EC category: " << EC2.category().name()
- << "EC message: " << EC2.message() << "\n";
-}
-ASSERT_FALSE(EC);
-VisitedNonBrokenSymlinks.push_back(I->path());
+EXPECT_FALSE(EC);
+(FS->status(I->path()) ? VisitedNonBrokenSymlinks : VisitedBrokenSymlinks)
+.push_back(I->path());
   }
 
   // Check visited file names.
-  std::sort(VisitedBrokenSymlinks.begin(), VisitedBrokenSymlinks.end());
-  std::sort(VisitedNonBrokenSymlinks.begin(), VisitedNonBrokenSymlinks.end());
-  EXPECT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size());
-  EXPECT_TRUE(std::equal(VisitedBrokenSymlinks.begin(),
- VisitedBrokenSymlinks.end(),
- ExpectedBrokenSymlinks.begin()));
-  EXPECT_EQ(ExpectedNonBrokenSymlinks.size(), VisitedNonBrokenSymlinks.size());
-  EXPECT_TRUE(std::equal(VisitedNonBrokenSymlinks.begin(),
- VisitedNonBrokenSymlinks.end(),
- ExpectedNonBrokenSymlinks.begin()));
+  EXPECT_THAT(VisitedBrokenSymlinks,
+  UnorderedElementsAre(StringRef(_a), StringRef(_ba),
+   StringRef(_bc), StringRef(_c),
+   StringRef(_e)));
+  EXPECT_THAT(VisitedNonBrokenSymlinks,
+  UnorderedElementsAre(StringRef(_b), StringRef(_bb), 
StringRef(_d),
+   StringRef(_dd), StringRef(_ddd)));
 }
 #endif
 


___

[PATCH] D52448: [clang-format] Break before next parameter after a formatted multiline raw string parameter

2018-10-01 Thread Krasimir Georgiev via Phabricator via cfe-commits

krasimir updated this revision to Diff 167749.
krasimir added a comment.

- Add more tests and tidy-up


Repository:
  rC Clang

https://reviews.llvm.org/D52448

Files:
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTestRawStrings.cpp

Index: unittests/Format/FormatTestRawStrings.cpp
===
--- unittests/Format/FormatTestRawStrings.cpp
+++ unittests/Format/FormatTestRawStrings.cpp
@@ -576,10 +576,13 @@
 auto S = R"pb(item_1:1 item_2:2)pb"+rest;)test",
getRawStringPbStyleWithColumns(40)));
 
+  // `rest` fits on the line after )pb", but forced on newline since the raw
+  // string literal is multiline.
   expect_eq(R"test(
 auto S = R"pb(item_1: 1,
   item_2: 2,
-  item_3: 3)pb" + rest;)test",
+  item_3: 3)pb" +
+ rest;)test",
 format(R"test(
 auto S = R"pb(item_1:1,item_2:2,item_3:3)pb"+rest;)test",
getRawStringPbStyleWithColumns(40)));
@@ -615,7 +618,8 @@
   expect_eq(R"test(
 auto S = R"pb(item_1: 1,
   item_2: 2,
-  item_3: 3)pb" + rest;)test",
+  item_3: 3)pb" +
+ rest;)test",
 format(R"test(
 auto S = R"pb(item_1:1,item_2:2,item_3:3)pb"+rest;)test",
getRawStringPbStyleWithColumns(40)));
@@ -889,6 +893,95 @@
Style));
 }
 
+TEST_F(FormatTestRawStrings,
+   BreaksBeforeNextParamAfterMultilineRawStringParam) {
+  FormatStyle Style = getRawStringPbStyleWithColumns(60);
+  expect_eq(R"test(
+int f() {
+  int a = g(x, R"pb(
+  key: 1  #
+  key: 2
+)pb",
+3, 4);
+}
+)test",
+format(R"test(
+int f() {
+  int a = g(x, R"pb(
+  key: 1 #
+  key: 2
+)pb", 3, 4);
+}
+)test",
+   Style));
+
+  // Breaks after a parent of a multiline param.
+  expect_eq(R"test(
+int f() {
+  int a = g(x, h(R"pb(
+  key: 1  #
+  key: 2
+)pb"),
+3, 4);
+}
+)test",
+format(R"test(
+int f() {
+  int a = g(x, h(R"pb(
+  key: 1 #
+  key: 2
+)pb"), 3, 4);
+}
+)test",
+   Style));
+  
+  expect_eq(R"test(
+int f() {
+  int a = g(x,
+h(R"pb(
+key: 1  #
+key: 2
+  )pb",
+  2),
+3, 4);
+}
+)test",
+format(R"test(
+int f() {
+  int a = g(x, h(R"pb(
+  key: 1 #
+  key: 2
+)pb", 2), 3, 4);
+}
+)test",
+   Style));
+  // Breaks if formatting introduces a multiline raw string.
+  expect_eq(R"test(
+int f() {
+  int a = g(x, R"pb(key1: value1
+key2: value22)pb",
+3, 4);
+}
+)test",
+format(R"test(
+int f() {
+  int a = g(x, R"pb(key1: value1 key2: value22)pb", 3, 4);
+}
+)test",
+   Style));
+  // Does not force a break after an original multiline param that is
+  // reformatterd as on single line.
+  expect_eq(R"test(
+int f() {
+  int a = g(R"pb(key: 1)pb", 2);
+})test",
+format(R"test(
+int f() {
+  int a = g(R"pb(key:
+ 1)pb", 2);
+})test", Style));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1502,10 +1502,25 @@
   // violate the rectangle rule and visually flows within the surrounding
   // source.
   bool ContentStartsOnNewline = Current.TokenText[OldPrefixSize] == '\n';
-  unsigned NextStartColumn =
-  ContentStartsOnNewline
-  ? State.Stack.back().NestedBlockIndent + Style.IndentWidth
-  : FirstStartColumn;
+  // If this token is the last parameter (checked by looking if it's followed by
+  // `)`, the base the indent off the line's nested block indent. Otherwise,
+  // base the indent off the arguments indent, so we can achieve:
+  // fff(1, 2, 3, R"pb(
+  // key1: 1  #
+  // key2: 2)pb");
+  //
+  // fff(1, 2, 3,
+  // R"pb(
+  //   key1: 1  #
+  //   key2: 2
+  // )pb",
+  // 5);
+  unsigned CurrentIndent = (Current.Next && Current.Next->is(tok::r_paren))
+   ? State.Stack.back().NestedBlockIndent
+   : State.Stack.back().Indent;
+  unsigned NextStartColumn = ContentStartsOnNewline
+ ? CurrentIndent + Style.IndentWidth
+ : FirstStartColumn;
 
   // The last start column is the column the raw string suffix starts if it is
   // put on a newline.
@@ -1517,7 +1532,7 @@
   // indent.
   unsigned LastStartColumn = Current.NewlinesBefore

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-10-01 Thread Marshall Clow via Phabricator via cfe-commits

mclow.lists accepted this revision.
mclow.lists added a comment.

Ok. I'm fine with this. Thanks for your patience.


Repository:
  rL LLVM

https://reviews.llvm.org/D52401



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

[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

2018-10-01 Thread Oleg Smolsky via Phabricator via cfe-commits

oleg.smolsky added a comment.

In https://reviews.llvm.org/D52676#1250124, @oleg.smolsky wrote:

> In https://reviews.llvm.org/D52676#1250071, @krasimir wrote:
>
> > In https://reviews.llvm.org/D52676#1249828, @oleg.smolsky wrote:
> >
> > > In https://reviews.llvm.org/D52676#1249784, @krasimir wrote:
> > >
> > > > IMO `BinPackArguments==false` does not imply that there should be a 
> > > > line break before the first arguments, only that there should not be 
> > > > two arguments from the same argument list that appear on the same line.
> > >
> > >
> > > That's right. However consider the following points:
> > >
> > > 1. a lambda function is already placed onto its own line when it is the 
> > > first arg (as you can see in the first test)
> >
> >
> > I believe that a newline before a first arg lambda is controlled by a 
> > different mechanism independent of bin packing, probably by a rule that is 
> > more general than just "newline before first arg lambda". I think djasper@ 
> > could know more about this case.
>
>
> Hmm... perhaps. I have not been able to find an explicit rule for that.
>
> >> 1. the other args are placed onto a distinct line
> >> 2. this behavior looks very close to "bin packing"
> >> 3. adding a new option for the minor cases seems to be an overkill
> >> 
> >>   Having said that, I can add a new explicit style option. Do you think 
> >> that will improve consensus? Would you expect test cases for positive and 
> >> negative values of the option?
> > 
> > I don't see how the example before:
> > 
> >   void f() {
> > something->Method2s(1,
> > [this] {
> >   Do1();
> >   Do2();
> > },
> > 1);
> >   }
> > 
> > 
> > is inconsistent, as not adding a newline before the first argument is 
> > typical, as in:
> > 
> >   $ clang-format -style='{BasedOnStyle: google, BinPackArguments: false}' 
> > test.cc ~
> >   void f() {
> > something->Method2s("11",
> > 
> > "",
> > 3);
> >   }
>
> Right, it's consistent with your example but inconsistent with with the 
> following two:
>
> lambda at arg0:
>
>   void f() {
> something->Method2s(
> [this] {
>   Do1();
>   Do2();
> },
> 1);
>   }
>
>
> and two lambdas:
>
>   // Multiple lambdas in the same parentheses change indentation rules.
>   verifyFormat("SomeFunction(\n"
>"[]() {\n"
>"  int i = 42;\n"
>"  return i;\n"
>"},\n"
>"[]() {\n"
>"  int j = 43;\n"
>"  return j;\n"
>"});");
>   




In https://reviews.llvm.org/D52676#1251064, @krasimir wrote:

> In https://reviews.llvm.org/D52676#1250124, @oleg.smolsky wrote:
>
> > In https://reviews.llvm.org/D52676#1250071, @krasimir wrote:
> >
> > > In https://reviews.llvm.org/D52676#1249828, @oleg.smolsky wrote:
> > >
> > > > In https://reviews.llvm.org/D52676#1249784, @krasimir wrote:
> > > >
> > > > > IMO `BinPackArguments==false` does not imply that there should be a 
> > > > > line break before the first arguments, only that there should not be 
> > > > > two arguments from the same argument list that appear on the same 
> > > > > line.
> > > >
> > > >
> > > > That's right. However consider the following points:
> > > >
> > > > 1. a lambda function is already placed onto its own line when it is the 
> > > > first arg (as you can see in the first test)
> > >
> > >
> > > I believe that a newline before a first arg lambda is controlled by a 
> > > different mechanism independent of bin packing, probably by a rule that 
> > > is more general than just "newline before first arg lambda". I think 
> > > djasper@ could know more about this case.
> >
> >
> > Hmm... perhaps. I have not been able to find an explicit rule for that.
> >
> > >> 1. the other args are placed onto a distinct line
> > >> 2. this behavior looks very close to "bin packing"
> > >> 3. adding a new option for the minor cases seems to be an overkill
> > >> 
> > >>   Having said that, I can add a new explicit style option. Do you think 
> > >> that will improve consensus? Would you expect test cases for positive 
> > >> and negative values of the option?
>
>
> Not really. It's generally hard to add a new style option, as that is 
> mandated by an existing commonly used public style guide that requires it.
>
> >> is inconsistent, as not adding a newline before the first argument is 
> >> typical, as in:
> >> 
> >>   $ clang-format -style='{BasedOnStyle: google, BinPackArguments: false}' 
> >> test.cc ~
> >>   void f() {
> >> something->Method2s("11",
> >>

r343492 - [OPENMP][NVPTX] Handle `requires datasharing` flag correctly with

2018-10-01 Thread Alexey Bataev via cfe-commits

Author: abataev
Date: Mon Oct  1 09:20:57 2018
New Revision: 343492

URL: http://llvm.org/viewvc/llvm-project?rev=343492&view=rev
Log:
[OPENMP][NVPTX] Handle `requires datasharing` flag correctly with
lightweight runtime.

The datasharing flag must be set to `1` when executing SPMD-mode compatible 
directive with reduction|lastprivate clauses.

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

cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=343492&r1=343491&r2=343492&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Mon Oct  1 09:20:57 2018
@@ -1207,6 +1207,10 @@ void CGOpenMPRuntimeNVPTX::emitSPMDKerne
IsOffloadEntry, CodeGen);
 }
 
+static void
+getDistributeLastprivateVars(const OMPExecutableDirective &D,
+ llvm::SmallVectorImpl &Vars);
+
 void CGOpenMPRuntimeNVPTX::emitSPMDEntryHeader(
 CodeGenFunction &CGF, EntryFunctionState &EST,
 const OMPExecutableDirective &D) {
@@ -1219,11 +1223,33 @@ void CGOpenMPRuntimeNVPTX::emitSPMDEntry
   // Initialize the OMP state in the runtime; called by all active threads.
   bool RequiresFullRuntime = CGM.getLangOpts().OpenMPCUDAForceFullRuntime ||
  !supportsLightweightRuntime(CGF.getContext(), D);
+  // Check if we have inner distribute + lastprivate|reduction clauses.
+  bool RequiresDatasharing = RequiresFullRuntime;
+  if (!RequiresDatasharing) {
+const OMPExecutableDirective *TD = &D;
+if (!isOpenMPTeamsDirective(TD->getDirectiveKind()) &&
+!isOpenMPParallelDirective(TD->getDirectiveKind())) {
+  const Stmt *S = getSingleCompoundChild(
+  TD->getInnermostCapturedStmt()->getCapturedStmt()->IgnoreContainers(
+  /*IgnoreCaptured=*/true));
+  TD = cast(S);
+}
+if (!isOpenMPDistributeDirective(TD->getDirectiveKind()) &&
+!isOpenMPParallelDirective(TD->getDirectiveKind())) {
+  const Stmt *S = getSingleCompoundChild(
+  TD->getInnermostCapturedStmt()->getCapturedStmt()->IgnoreContainers(
+  /*IgnoreCaptured=*/true));
+  TD = cast(S);
+}
+if (isOpenMPDistributeDirective(TD->getDirectiveKind()))
+  RequiresDatasharing = TD->hasClausesOfKind() ||
+TD->hasClausesOfKind();
+  }
   llvm::Value *Args[] = {
   getThreadLimit(CGF, /*IsInSPMDExecutionMode=*/true),
   /*RequiresOMPRuntime=*/
   Bld.getInt16(RequiresFullRuntime ? 1 : 0),
-  /*RequiresDataSharing=*/Bld.getInt16(RequiresFullRuntime ? 1 : 0)};
+  /*RequiresDataSharing=*/Bld.getInt16(RequiresDatasharing ? 1 : 0)};
   CGF.EmitRuntimeCall(
   createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_spmd_kernel_init), Args);
 

Modified: cfe/trunk/test/OpenMP/nvptx_SPMD_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_SPMD_codegen.cpp?rev=343492&r1=343491&r2=343492&view=diff
==
--- cfe/trunk/test/OpenMP/nvptx_SPMD_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_SPMD_codegen.cpp Mon Oct  1 09:20:57 2018
@@ -40,7 +40,7 @@ void foo() {
   for (int i = 0; i < 10; ++i)
 ;
 int a;
-// CHECK: call void @__kmpc_spmd_kernel_init(i32 {{.+}}, i16 0, i16 0)
+// CHECK: call void @__kmpc_spmd_kernel_init(i32 {{.+}}, i16 0, i16 1)
 // CHECK: call void @__kmpc_spmd_kernel_init(i32 {{.+}}, i16 0, i16 0)
 // CHECK: call void @__kmpc_spmd_kernel_init(i32 {{.+}}, i16 0, i16 0)
 // CHECK: call void @__kmpc_spmd_kernel_init(i32 {{.+}}, i16 1, i16 {{.+}})

Modified: 
cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp?rev=343492&r1=343491&r2=343492&view=diff
==
--- 
cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp 
(original)
+++ 
cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp 
Mon Oct  1 09:20:57 2018
@@ -69,7 +69,7 @@ int bar(int n){
 
 // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+}}_l32(
 // CHECK-DAG: [[THREAD_LIMIT:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
-// CHECK: call void @__kmpc_spmd_kernel_init(i32 [[THREAD_LIMIT]], i16 0, i16 
0)
+// CHECK: call void @__kmpc_spmd_kernel_init(i32 [[THREAD_LIMIT]], i16 0, i16 
1)
 // CHECK: [[TEAM_ALLOC:%.+]] = call i8* 
@__kmpc_data_sharing_push_stack(i{{[0-9]+}} 4, i16 0)
 //

[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2018-10-01 Thread Peter Wu via Phabricator via cfe-commits

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

The functionality looks correct to me, but could you include some tests in 
test/Driver/ and test/Preprocessor/ just to be sure?
test/Driver/debug-prefix-map.c and test/CodeGen/debug-prefix-map.c could serve 
as inspiration.

The documentation should probable be updated too: 
docs/ClangCommandLineReference.rst

(It would be nice to have this feature for Reproducible Builds)




Comment at: lib/Lex/PPMacroExpansion.cpp:1460
+  for (const auto &Entry : MacroPrefixMap)
+if (Path.startswith(Entry.first))
+  return (Twine(Entry.second) + Path.substr(Entry.first.size())).str();

dankm wrote:
> dankm wrote:
> > joerg wrote:
> > > This doesn't handle directory vs string prefix prefix correctly, does it? 
> > > At the very least, it should have a test case :)
> > Good catch. I expect not. But on the other hand, it's exactly what 
> > debug-prefix-map does :)
> > 
> > I'll add test cases in a future review. My first goal was getting something 
> > sort-of working.
> There should be a test, but apparently the debug prefix map code also does 
> this.
> 
> What do you think the correct behaviour should be? a string prefix, or a 
> directory prefix?
It should be a string prefix (like GCC)


Repository:
  rC Clang

https://reviews.llvm.org/D49466



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

1 2 3 >

1 - 100 of 232 matches

Mail list logo