r346733 - [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag

[Takuto Ikuta via cfe-commits]

Author: tikuta
Date: Mon Nov 12 20:14:09 2018
New Revision: 346733

URL: http://llvm.org/viewvc/llvm-project?rev=346733=rev
Log:
[clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag

Summary: /Zc:dllexportInlines with /fallback may cause unexpected linker error. 
It is better to disallow compile rather than warn for this combination.

Reviewers: hans, thakis

Reviewed By: hans

Subscribers: cfe-commits, llvm-commits

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
cfe/trunk/test/Driver/cl-options.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=346733=346732=346733=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Mon Nov 12 20:14:09 
2018
@@ -161,9 +161,8 @@ def warn_drv_yc_multiple_inputs_clang_cl
   "support for '/Yc' with more than one source file not implemented yet; flag 
ignored">,
   InGroup;
 
-def warn_drv_non_fallback_argument_clang_cl : Warning<
-  "option '%0' is ignored when /fallback happens">,
-  InGroup;
+def err_drv_dllexport_inlines_and_fallback : Error<
+  "option '/Zc:dllexportInlines-' is ABI-changing and not compatible with 
'/fallback'">;
 
 def err_drv_invalid_value : Error<"invalid value '%1' in '%0'">;
 def err_drv_invalid_int_value : Error<"invalid integral value '%1' in '%0'">;

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=346733=346732=346733=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Mon Nov 12 20:14:09 2018
@@ -5502,8 +5502,13 @@ void Clang::AddClangCLArgs(const ArgList
 
  if (Args.hasFlag(options::OPT__SLASH_Zc_dllexportInlines_,
   options::OPT__SLASH_Zc_dllexportInlines,
-  false))
+  false)) {
+   if (Args.hasArg(options::OPT__SLASH_fallback)) {
+ D.Diag(clang::diag::err_drv_dllexport_inlines_and_fallback);
+   } else {
 CmdArgs.push_back("-fno-dllexport-inlines");
+   }
+ }
 
   Arg *MostGeneralArg = Args.getLastArg(options::OPT__SLASH_vmg);
   Arg *BestCaseArg = Args.getLastArg(options::OPT__SLASH_vmb);

Modified: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/MSVC.cpp?rev=346733=346732=346733=diff
==
--- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp Mon Nov 12 20:14:09 2018
@@ -669,12 +669,6 @@ std::unique_ptr visualstudio::C
   // them too.
   Args.AddAllArgs(CmdArgs, options::OPT_UNKNOWN);
 
-  // Warning for ignored flag.
-  if (const Arg *dllexportInlines =
-  Args.getLastArg(options::OPT__SLASH_Zc_dllexportInlines_))
-C.getDriver().Diag(clang::diag::warn_drv_non_fallback_argument_clang_cl)
-  << dllexportInlines->getAsString(Args);
-
   // Input filename.
   assert(Inputs.size() == 1);
   const InputInfo  = Inputs[0];

Modified: cfe/trunk/test/Driver/cl-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=346733=346732=346733=diff
==
--- cfe/trunk/test/Driver/cl-options.c (original)
+++ cfe/trunk/test/Driver/cl-options.c Mon Nov 12 20:14:09 2018
@@ -495,7 +495,7 @@
 // RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck 
-check-prefix=DllExportInlines %s
 // DllExportInlines-NOT: "-fno-dllexport-inlines"
 // RUN: %clang_cl /fallback /Zc:dllexportInlines- /c -### -- %s 2>&1 | 
FileCheck -check-prefix=DllExportInlinesFallback %s
-// DllExportInlinesFallback: warning: option '/Zc:dllexportInlines-' is 
ignored when /fallback happens [-Woption-ignored]
+// DllExportInlinesFallback: error: option '/Zc:dllexportInlines-' is 
ABI-changing and not compatible with '/fallback'
 
 // RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
 // Zi: "-gcodeview"


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

[PATCH] D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC346733: [clang-cl] Do not allow using both 
/Zc:dllexportInlines- and /fallback flag (authored by tikuta, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54426?vs=173810=173811#toc

Repository:
  rC Clang

https://reviews.llvm.org/D54426

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/MSVC.cpp
  test/Driver/cl-options.c


Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -495,7 +495,7 @@
 // RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck 
-check-prefix=DllExportInlines %s
 // DllExportInlines-NOT: "-fno-dllexport-inlines"
 // RUN: %clang_cl /fallback /Zc:dllexportInlines- /c -### -- %s 2>&1 | 
FileCheck -check-prefix=DllExportInlinesFallback %s
-// DllExportInlinesFallback: warning: option '/Zc:dllexportInlines-' is 
ignored when /fallback happens [-Woption-ignored]
+// DllExportInlinesFallback: error: option '/Zc:dllexportInlines-' is 
ABI-changing and not compatible with '/fallback'
 
 // RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
 // Zi: "-gcodeview"
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -669,12 +669,6 @@
   // them too.
   Args.AddAllArgs(CmdArgs, options::OPT_UNKNOWN);
 
-  // Warning for ignored flag.
-  if (const Arg *dllexportInlines =
-  Args.getLastArg(options::OPT__SLASH_Zc_dllexportInlines_))
-C.getDriver().Diag(clang::diag::warn_drv_non_fallback_argument_clang_cl)
-  << dllexportInlines->getAsString(Args);
-
   // Input filename.
   assert(Inputs.size() == 1);
   const InputInfo  = Inputs[0];
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5502,8 +5502,13 @@
 
  if (Args.hasFlag(options::OPT__SLASH_Zc_dllexportInlines_,
   options::OPT__SLASH_Zc_dllexportInlines,
-  false))
+  false)) {
+   if (Args.hasArg(options::OPT__SLASH_fallback)) {
+ D.Diag(clang::diag::err_drv_dllexport_inlines_and_fallback);
+   } else {
 CmdArgs.push_back("-fno-dllexport-inlines");
+   }
+ }
 
   Arg *MostGeneralArg = Args.getLastArg(options::OPT__SLASH_vmg);
   Arg *BestCaseArg = Args.getLastArg(options::OPT__SLASH_vmb);
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -161,9 +161,8 @@
   "support for '/Yc' with more than one source file not implemented yet; flag 
ignored">,
   InGroup;
 
-def warn_drv_non_fallback_argument_clang_cl : Warning<
-  "option '%0' is ignored when /fallback happens">,
-  InGroup;
+def err_drv_dllexport_inlines_and_fallback : Error<
+  "option '/Zc:dllexportInlines-' is ABI-changing and not compatible with 
'/fallback'">;
 
 def err_drv_invalid_value : Error<"invalid value '%1' in '%0'">;
 def err_drv_invalid_int_value : Error<"invalid integral value '%1' in '%0'">;


Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -495,7 +495,7 @@
 // RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s
 // DllExportInlines-NOT: "-fno-dllexport-inlines"
 // RUN: %clang_cl /fallback /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlinesFallback %s
-// DllExportInlinesFallback: warning: option '/Zc:dllexportInlines-' is ignored when /fallback happens [-Woption-ignored]
+// DllExportInlinesFallback: error: option '/Zc:dllexportInlines-' is ABI-changing and not compatible with '/fallback'
 
 // RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
 // Zi: "-gcodeview"
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -669,12 +669,6 @@
   // them too.
   Args.AddAllArgs(CmdArgs, options::OPT_UNKNOWN);
 
-  // Warning for ignored flag.
-  if (const Arg *dllexportInlines =
-  Args.getLastArg(options::OPT__SLASH_Zc_dllexportInlines_))
-C.getDriver().Diag(clang::diag::warn_drv_non_fallback_argument_clang_cl)
-  << dllexportInlines->getAsString(Args);
-
   // Input filename.
   assert(Inputs.size() == 1);
   const InputInfo  = Inputs[0];
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5502,8 +5502,13 @@
 

[PATCH] D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag

[Takuto Ikuta via Phabricator via cfe-commits]

takuto.ikuta updated this revision to Diff 173810.
takuto.ikuta marked an inline comment as done.
takuto.ikuta added a comment.

rebase


Repository:
  rL LLVM

https://reviews.llvm.org/D54426

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/Driver/cl-options.c


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -495,7 +495,7 @@
 // RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck 
-check-prefix=DllExportInlines %s
 // DllExportInlines-NOT: "-fno-dllexport-inlines"
 // RUN: %clang_cl /fallback /Zc:dllexportInlines- /c -### -- %s 2>&1 | 
FileCheck -check-prefix=DllExportInlinesFallback %s
-// DllExportInlinesFallback: warning: option '/Zc:dllexportInlines-' is 
ignored when /fallback happens [-Woption-ignored]
+// DllExportInlinesFallback: error: option '/Zc:dllexportInlines-' is 
ABI-changing and not compatible with '/fallback'
 
 // RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
 // Zi: "-gcodeview"
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -669,12 +669,6 @@
   // them too.
   Args.AddAllArgs(CmdArgs, options::OPT_UNKNOWN);
 
-  // Warning for ignored flag.
-  if (const Arg *dllexportInlines =
-  Args.getLastArg(options::OPT__SLASH_Zc_dllexportInlines_))
-C.getDriver().Diag(clang::diag::warn_drv_non_fallback_argument_clang_cl)
-  << dllexportInlines->getAsString(Args);
-
   // Input filename.
   assert(Inputs.size() == 1);
   const InputInfo  = Inputs[0];
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5502,8 +5502,13 @@
 
  if (Args.hasFlag(options::OPT__SLASH_Zc_dllexportInlines_,
   options::OPT__SLASH_Zc_dllexportInlines,
-  false))
+  false)) {
+   if (Args.hasArg(options::OPT__SLASH_fallback)) {
+ D.Diag(clang::diag::err_drv_dllexport_inlines_and_fallback);
+   } else {
 CmdArgs.push_back("-fno-dllexport-inlines");
+   }
+ }
 
   Arg *MostGeneralArg = Args.getLastArg(options::OPT__SLASH_vmg);
   Arg *BestCaseArg = Args.getLastArg(options::OPT__SLASH_vmb);
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -161,9 +161,8 @@
   "support for '/Yc' with more than one source file not implemented yet; flag 
ignored">,
   InGroup;
 
-def warn_drv_non_fallback_argument_clang_cl : Warning<
-  "option '%0' is ignored when /fallback happens">,
-  InGroup;
+def err_drv_dllexport_inlines_and_fallback : Error<
+  "option '/Zc:dllexportInlines-' is ABI-changing and not compatible with 
'/fallback'">;
 
 def err_drv_invalid_value : Error<"invalid value '%1' in '%0'">;
 def err_drv_invalid_int_value : Error<"invalid integral value '%1' in '%0'">;


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -495,7 +495,7 @@
 // RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s
 // DllExportInlines-NOT: "-fno-dllexport-inlines"
 // RUN: %clang_cl /fallback /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlinesFallback %s
-// DllExportInlinesFallback: warning: option '/Zc:dllexportInlines-' is ignored when /fallback happens [-Woption-ignored]
+// DllExportInlinesFallback: error: option '/Zc:dllexportInlines-' is ABI-changing and not compatible with '/fallback'
 
 // RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
 // Zi: "-gcodeview"
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -669,12 +669,6 @@
   // them too.
   Args.AddAllArgs(CmdArgs, options::OPT_UNKNOWN);
 
-  // Warning for ignored flag.
-  if (const Arg *dllexportInlines =
-  Args.getLastArg(options::OPT__SLASH_Zc_dllexportInlines_))
-C.getDriver().Diag(clang::diag::warn_drv_non_fallback_argument_clang_cl)
-  << dllexportInlines->getAsString(Args);
-
   // Input filename.
   assert(Inputs.size() == 1);
   const InputInfo  = Inputs[0];
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5502,8 +5502,13 @@
 
  if 

[PATCH] D54463: [CMake] Support cross-compiling with Fuchsia toolchain build

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: beanz, mcgrathr.
Herald added subscribers: cfe-commits, mgorny.

When second stage is being cross-compiled for a different platform
we need to build enough of first stage runtimes to get a working
compiler.


Repository:
  rC Clang

https://reviews.llvm.org/D54463

Files:
  clang/cmake/caches/Fuchsia.cmake


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -1,6 +1,6 @@
 # This file sets up a CMakeCache for a Fuchsia toolchain build.
 
-set(LLVM_TARGETS_TO_BUILD Native CACHE STRING "")
+set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64 CACHE STRING "")
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
@@ -57,6 +57,38 @@
   install-distribution-stripped
   clang CACHE STRING "")
 
+if(BOOTSTRAP_CMAKE_SYSTEM_NAME)
+  string(FIND ${BOOTSTRAP_CMAKE_CXX_COMPILER_TARGET} "-" dash_index)
+  string(SUBSTRING ${BOOTSTRAP_CMAKE_CXX_COMPILER_TARGET} 0 ${dash_index} 
target)
+  if(STAGE2_LINUX_${target}_SYSROOT)
+set(BUILTINS_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
+set(BUILTINS_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(BUILTINS_${target}-linux-gnu_CMAKE_SYSROOT 
${STAGE2_LINUX_${target}_SYSROOT} CACHE STRING "")
+set(LLVM_BUILTIN_TARGETS "${target}-linux-gnu" CACHE STRING "")
+
+set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
+set(RUNTIMES_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSROOT 
${STAGE2_LINUX_${target}_SYSROOT} CACHE STRING "")
+set(RUNTIMES_${target}-linux-gnu_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL 
"")
+set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL 
"")
+set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL 
"")
+set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL 
"")
+set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE 
BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL 
"")
+set(RUNTIMES_${target}-linux-gnu_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE 
BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBCXX_ABI_VERSION 2 CACHE STRING "")
+set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI "libc++" CACHE STRING 
"")
+set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE 
BOOL "")
+set(LLVM_RUNTIME_TARGETS "${target}-linux-gnu" CACHE STRING "")
+  endif()
+endif()
+
 get_cmake_property(variableNames VARIABLES)
 foreach(variableName ${variableNames})
   if(variableName MATCHES "^STAGE2_")


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -1,6 +1,6 @@
 # This file sets up a CMakeCache for a Fuchsia toolchain build.
 
-set(LLVM_TARGETS_TO_BUILD Native CACHE STRING "")
+set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64 CACHE STRING "")
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
@@ -57,6 +57,38 @@
   install-distribution-stripped
   clang CACHE STRING "")
 
+if(BOOTSTRAP_CMAKE_SYSTEM_NAME)
+  string(FIND ${BOOTSTRAP_CMAKE_CXX_COMPILER_TARGET} "-" dash_index)
+  string(SUBSTRING ${BOOTSTRAP_CMAKE_CXX_COMPILER_TARGET} 0 ${dash_index} target)
+  if(STAGE2_LINUX_${target}_SYSROOT)
+set(BUILTINS_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
+set(BUILTINS_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(BUILTINS_${target}-linux-gnu_CMAKE_SYSROOT ${STAGE2_LINUX_${target}_SYSROOT} CACHE STRING "")
+set(LLVM_BUILTIN_TARGETS "${target}-linux-gnu" CACHE STRING "")
+
+set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
+set(RUNTIMES_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSROOT ${STAGE2_LINUX_${target}_SYSROOT} CACHE STRING "")
+set(RUNTIMES_${target}-linux-gnu_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
+

[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

[Artem Dergachev via Phabricator via cfe-commits]

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

Ok, so the point is to fix the current checker name problem, right? I guess 
let's land it then :)

Code looks great. I'm thinking aloud in inline comments a little bit, but don't 
mind me.

@rnkovacs, do you have any immediate opinions on how the header structure 
changes here?




Comment at: lib/StaticAnalyzer/Checkers/InnerPointer/InnerPointerChecker.h:23-25
+  CallDescription AppendFn, AssignFn, ClearFn, CStrFn, DataFn, EraseFn,
+  InsertFn, PopBackFn, PushBackFn, ReplaceFn, ReserveFn, ResizeFn,
+  ShrinkToFitFn, SwapFn;

We should eventually initialize these in place.



Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:160
+ firstRelatedChecker != lastRelatedChecker; ++firstRelatedChecker) {
+  if (opt.second) {
+// Enable the checker.

Because we already forget by now what the second part of this pair is, i 
believe that decomposing a `std::pair` into two variables as early as possible 
is a good idea.


Repository:
  rC Clang

https://reviews.llvm.org/D54438



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

[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/altivec-generic-overload.c:74
+  convert1(gv1);
+  // CHECK: call void @_Z8convert1Dv16_a(<16 x i8> %{{[0-9]+}})
+  convert1(gv2);

Checking that the call is to the expected target in terms of overload 
resolution can be achieved by having a distinct return types for each member of 
the overload set.


https://reviews.llvm.org/D53417



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

[PATCH] D51714: CMake: Deprecate using llvm-config to detect llvm installation

[Tom Stellard via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC346732: CMake: Deprecate using llvm-config to detect llvm 
installation (authored by tstellar, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51714?vs=167413=173808#toc

Repository:
  rC Clang

https://reviews.llvm.org/D51714

Files:
  CMakeLists.txt


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -11,9 +11,14 @@
 
   # Rely on llvm-config.
   set(CONFIG_OUTPUT)
-  find_program(LLVM_CONFIG "llvm-config")
   if(LLVM_CONFIG)
+set (LLVM_CONFIG_FOUND 1)
 message(STATUS "Found LLVM_CONFIG as ${LLVM_CONFIG}")
+message(DEPRECATION "Using llvm-config to detect the LLVM installation is \
+  deprecated.  The installed cmake files should be used \
+  instead.  CMake should be able to detect your LLVM install \
+  automatically, but you can also use LLVM_DIR to specify \
+  the path containing LLVMConfig.cmake.")
 set(CONFIG_COMMAND ${LLVM_CONFIG}
   "--assertion-mode"
   "--bindir"
@@ -36,41 +41,51 @@
   message(STATUS "${CONFIG_COMMAND_STR}")
   message(FATAL_ERROR "llvm-config failed with status ${HAD_ERROR}")
 endif()
-  else()
-message(FATAL_ERROR "llvm-config not found -- ${LLVM_CONFIG}")
+
+list(GET CONFIG_OUTPUT 0 ENABLE_ASSERTIONS)
+list(GET CONFIG_OUTPUT 1 TOOLS_BINARY_DIR)
+list(GET CONFIG_OUTPUT 2 LIBRARY_DIR)
+list(GET CONFIG_OUTPUT 3 INCLUDE_DIR)
+list(GET CONFIG_OUTPUT 4 LLVM_OBJ_ROOT)
+list(GET CONFIG_OUTPUT 5 MAIN_SRC_DIR)
+list(GET CONFIG_OUTPUT 6 LLVM_CONFIG_CMAKE_PATH)
+
+# Normalize LLVM_CMAKE_PATH. --cmakedir might contain backslashes.
+# CMake assumes slashes as PATH.
+file(TO_CMAKE_PATH ${LLVM_CONFIG_CMAKE_PATH} LLVM_CMAKE_PATH)
   endif()
 
-  list(GET CONFIG_OUTPUT 0 ENABLE_ASSERTIONS)
-  list(GET CONFIG_OUTPUT 1 TOOLS_BINARY_DIR)
-  list(GET CONFIG_OUTPUT 2 LIBRARY_DIR)
-  list(GET CONFIG_OUTPUT 3 INCLUDE_DIR)
-  list(GET CONFIG_OUTPUT 4 LLVM_OBJ_ROOT)
-  list(GET CONFIG_OUTPUT 5 MAIN_SRC_DIR)
-  list(GET CONFIG_OUTPUT 6 LLVM_CONFIG_CMAKE_PATH)
 
   if(NOT MSVC_IDE)
 set(LLVM_ENABLE_ASSERTIONS ${ENABLE_ASSERTIONS}
   CACHE BOOL "Enable assertions")
 # Assertions should follow llvm-config's.
 mark_as_advanced(LLVM_ENABLE_ASSERTIONS)
   endif()
 
+  find_package(LLVM REQUIRED HINTS "${LLVM_CMAKE_PATH}")
+  list(APPEND CMAKE_MODULE_PATH ${LLVM_DIR})
+
+  # We can't check LLVM_CONFIG here, because find_package(LLVM ...) also sets
+  # LLVM_CONFIG.
+  if (NOT LLVM_CONFIG_FOUND)
+# Pull values from LLVMConfig.cmake.  We can drop this once the llvm-config
+# path is removed.
+set(TOOLS_BINARY_DIR ${LLVM_TOOLS_BINARY_DIR})
+set(LIBRARY_DIR ${LLVM_LIBRARY_DIR})
+set(INCLUDE_DIR ${LLVM_INCLUDE_DIR})
+set(LLVM_OBJ_DIR ${LLVM_BINARY_DIR})
+  endif()
+
   set(LLVM_TOOLS_BINARY_DIR ${TOOLS_BINARY_DIR} CACHE PATH "Path to llvm/bin")
   set(LLVM_LIBRARY_DIR ${LIBRARY_DIR} CACHE PATH "Path to llvm/lib")
   set(LLVM_MAIN_INCLUDE_DIR ${INCLUDE_DIR} CACHE PATH "Path to llvm/include")
   set(LLVM_BINARY_DIR ${LLVM_OBJ_ROOT} CACHE PATH "Path to LLVM build tree")
   set(LLVM_MAIN_SRC_DIR ${MAIN_SRC_DIR} CACHE PATH "Path to LLVM source tree")
 
-  # Normalize LLVM_CMAKE_PATH. --cmakedir might contain backslashes.
-  # CMake assumes slashes as PATH.
-  file(TO_CMAKE_PATH ${LLVM_CONFIG_CMAKE_PATH} LLVM_CMAKE_PATH)
-
   find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR}
 NO_DEFAULT_PATH)
 
-  find_package(LLVM REQUIRED HINTS "${LLVM_CMAKE_PATH}")
-  list(APPEND CMAKE_MODULE_PATH ${LLVM_DIR})
-
   # They are used as destination of target generators.
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
   set(LLVM_LIBRARY_OUTPUT_INTDIR 
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX})


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -11,9 +11,14 @@
 
   # Rely on llvm-config.
   set(CONFIG_OUTPUT)
-  find_program(LLVM_CONFIG "llvm-config")
   if(LLVM_CONFIG)
+set (LLVM_CONFIG_FOUND 1)
 message(STATUS "Found LLVM_CONFIG as ${LLVM_CONFIG}")
+message(DEPRECATION "Using llvm-config to detect the LLVM installation is \
+  deprecated.  The installed cmake files should be used \
+  instead.  CMake should be able to detect your LLVM install \
+  automatically, but you can also use LLVM_DIR to specify \
+  the path containing LLVMConfig.cmake.")
 set(CONFIG_COMMAND ${LLVM_CONFIG}
   "--assertion-mode"
   "--bindir"
@@ -36,41 +41,51 @@
   message(STATUS "${CONFIG_COMMAND_STR}")
   message(FATAL_ERROR "llvm-config failed with status ${HAD_ERROR}")
 endif()
-  else()
-message(FATAL_ERROR "llvm-config not found -- ${LLVM_CONFIG}")
+
+list(GET CONFIG_OUTPUT 0 

r346732 - CMake: Deprecate using llvm-config to detect llvm installation

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Mon Nov 12 19:42:46 2018
New Revision: 346732

URL: http://llvm.org/viewvc/llvm-project?rev=346732=rev
Log:
CMake: Deprecate using llvm-config to detect llvm installation

Summary:
clang currently uses llvm-config to determine the installation paths
for llvm's headers and binaries.  clang is also using LLVM's cmake
files to determine other information about the LLVM build, like
LLVM_LIBDIR_SUFFIX, LLVM_VERSION_*, etc.  Since the installation
paths are also available via the cmake files, we can simplify the code
by only relying on information from cmake about the LLVM install and
dropping the use of llvm-config altogether.

In addition to simplifying the code, the cmake files have more
accurate information about the llvm installation paths.  llvm-config
assumes that the lib, bin, and cmake directories are always located
in the same place relative to the path of the llvm-config executable.
This can be wrong if a user decides to install headers, binaries
or libraries to a non-standard location: e.g. static libraries
installed to /usr/lib/llvm6.0/

This patch takes the first step towards dropping llvm-config by
removing the automatic detection of llvm-config (users can still
manually supply a path to llvm-config by passing
-DLLVM_CONFIG=/usr/bin/llvm-config to cmake) and adding a
deprecation warning when users try to use this option.

Reviewers: chandlerc, beanz, mgorny, chapuni

Subscribers: mehdi_amini, dexonsmith, cfe-commits

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

Modified:
cfe/trunk/CMakeLists.txt

Modified: cfe/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=346732=346731=346732=diff
==
--- cfe/trunk/CMakeLists.txt (original)
+++ cfe/trunk/CMakeLists.txt Mon Nov 12 19:42:46 2018
@@ -11,9 +11,14 @@ if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
 
   # Rely on llvm-config.
   set(CONFIG_OUTPUT)
-  find_program(LLVM_CONFIG "llvm-config")
   if(LLVM_CONFIG)
+set (LLVM_CONFIG_FOUND 1)
 message(STATUS "Found LLVM_CONFIG as ${LLVM_CONFIG}")
+message(DEPRECATION "Using llvm-config to detect the LLVM installation is \
+  deprecated.  The installed cmake files should be used \
+  instead.  CMake should be able to detect your LLVM install \
+  automatically, but you can also use LLVM_DIR to specify \
+  the path containing LLVMConfig.cmake.")
 set(CONFIG_COMMAND ${LLVM_CONFIG}
   "--assertion-mode"
   "--bindir"
@@ -36,17 +41,20 @@ if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
   message(STATUS "${CONFIG_COMMAND_STR}")
   message(FATAL_ERROR "llvm-config failed with status ${HAD_ERROR}")
 endif()
-  else()
-message(FATAL_ERROR "llvm-config not found -- ${LLVM_CONFIG}")
+
+list(GET CONFIG_OUTPUT 0 ENABLE_ASSERTIONS)
+list(GET CONFIG_OUTPUT 1 TOOLS_BINARY_DIR)
+list(GET CONFIG_OUTPUT 2 LIBRARY_DIR)
+list(GET CONFIG_OUTPUT 3 INCLUDE_DIR)
+list(GET CONFIG_OUTPUT 4 LLVM_OBJ_ROOT)
+list(GET CONFIG_OUTPUT 5 MAIN_SRC_DIR)
+list(GET CONFIG_OUTPUT 6 LLVM_CONFIG_CMAKE_PATH)
+
+# Normalize LLVM_CMAKE_PATH. --cmakedir might contain backslashes.
+# CMake assumes slashes as PATH.
+file(TO_CMAKE_PATH ${LLVM_CONFIG_CMAKE_PATH} LLVM_CMAKE_PATH)
   endif()
 
-  list(GET CONFIG_OUTPUT 0 ENABLE_ASSERTIONS)
-  list(GET CONFIG_OUTPUT 1 TOOLS_BINARY_DIR)
-  list(GET CONFIG_OUTPUT 2 LIBRARY_DIR)
-  list(GET CONFIG_OUTPUT 3 INCLUDE_DIR)
-  list(GET CONFIG_OUTPUT 4 LLVM_OBJ_ROOT)
-  list(GET CONFIG_OUTPUT 5 MAIN_SRC_DIR)
-  list(GET CONFIG_OUTPUT 6 LLVM_CONFIG_CMAKE_PATH)
 
   if(NOT MSVC_IDE)
 set(LLVM_ENABLE_ASSERTIONS ${ENABLE_ASSERTIONS}
@@ -55,22 +63,29 @@ if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
 mark_as_advanced(LLVM_ENABLE_ASSERTIONS)
   endif()
 
+  find_package(LLVM REQUIRED HINTS "${LLVM_CMAKE_PATH}")
+  list(APPEND CMAKE_MODULE_PATH ${LLVM_DIR})
+
+  # We can't check LLVM_CONFIG here, because find_package(LLVM ...) also sets
+  # LLVM_CONFIG.
+  if (NOT LLVM_CONFIG_FOUND)
+# Pull values from LLVMConfig.cmake.  We can drop this once the llvm-config
+# path is removed.
+set(TOOLS_BINARY_DIR ${LLVM_TOOLS_BINARY_DIR})
+set(LIBRARY_DIR ${LLVM_LIBRARY_DIR})
+set(INCLUDE_DIR ${LLVM_INCLUDE_DIR})
+set(LLVM_OBJ_DIR ${LLVM_BINARY_DIR})
+  endif()
+
   set(LLVM_TOOLS_BINARY_DIR ${TOOLS_BINARY_DIR} CACHE PATH "Path to llvm/bin")
   set(LLVM_LIBRARY_DIR ${LIBRARY_DIR} CACHE PATH "Path to llvm/lib")
   set(LLVM_MAIN_INCLUDE_DIR ${INCLUDE_DIR} CACHE PATH "Path to llvm/include")
   set(LLVM_BINARY_DIR ${LLVM_OBJ_ROOT} CACHE PATH "Path to LLVM build tree")
   set(LLVM_MAIN_SRC_DIR ${MAIN_SRC_DIR} CACHE PATH "Path to LLVM source tree")
 
-  # Normalize LLVM_CMAKE_PATH. --cmakedir might contain backslashes.
-  # CMake assumes slashes as PATH.
-  file(TO_CMAKE_PATH ${LLVM_CONFIG_CMAKE_PATH} LLVM_CMAKE_PATH)
-
   find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" 

[PATCH] D32577: CMake: Replace open-coded find_package

[Tom Stellard via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346731: CMake: Replace open-coded find_package (authored by 
tstellar, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D32577

Files:
  cfe/trunk/CMakeLists.txt


Index: cfe/trunk/CMakeLists.txt
===
--- cfe/trunk/CMakeLists.txt
+++ cfe/trunk/CMakeLists.txt
@@ -68,13 +68,8 @@
   find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR}
 NO_DEFAULT_PATH)
 
-  set(LLVMCONFIG_FILE "${LLVM_CMAKE_PATH}/LLVMConfig.cmake")
-  if(EXISTS ${LLVMCONFIG_FILE})
-list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_PATH}")
-include(${LLVMCONFIG_FILE})
-  else()
-message(FATAL_ERROR "Not found: ${LLVMCONFIG_FILE}")
-  endif()
+  find_package(LLVM REQUIRED HINTS "${LLVM_CMAKE_PATH}")
+  list(APPEND CMAKE_MODULE_PATH ${LLVM_DIR})
 
   # They are used as destination of target generators.
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)


Index: cfe/trunk/CMakeLists.txt
===
--- cfe/trunk/CMakeLists.txt
+++ cfe/trunk/CMakeLists.txt
@@ -68,13 +68,8 @@
   find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR}
 NO_DEFAULT_PATH)
 
-  set(LLVMCONFIG_FILE "${LLVM_CMAKE_PATH}/LLVMConfig.cmake")
-  if(EXISTS ${LLVMCONFIG_FILE})
-list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_PATH}")
-include(${LLVMCONFIG_FILE})
-  else()
-message(FATAL_ERROR "Not found: ${LLVMCONFIG_FILE}")
-  endif()
+  find_package(LLVM REQUIRED HINTS "${LLVM_CMAKE_PATH}")
+  list(APPEND CMAKE_MODULE_PATH ${LLVM_DIR})
 
   # They are used as destination of target generators.
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r346731 - CMake: Replace open-coded find_package

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Mon Nov 12 19:16:45 2018
New Revision: 346731

URL: http://llvm.org/viewvc/llvm-project?rev=346731=rev
Log:
CMake: Replace open-coded find_package

Reviewers: beanz, mgorny

Reviewed By: mgorny

Subscribers: cfe-commits, chapuni, llvm-commits

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

Modified:
cfe/trunk/CMakeLists.txt

Modified: cfe/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=346731=346730=346731=diff
==
--- cfe/trunk/CMakeLists.txt (original)
+++ cfe/trunk/CMakeLists.txt Mon Nov 12 19:16:45 2018
@@ -68,13 +68,8 @@ if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
   find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR}
 NO_DEFAULT_PATH)
 
-  set(LLVMCONFIG_FILE "${LLVM_CMAKE_PATH}/LLVMConfig.cmake")
-  if(EXISTS ${LLVMCONFIG_FILE})
-list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_PATH}")
-include(${LLVMCONFIG_FILE})
-  else()
-message(FATAL_ERROR "Not found: ${LLVMCONFIG_FILE}")
-  endif()
+  find_package(LLVM REQUIRED HINTS "${LLVM_CMAKE_PATH}")
+  list(APPEND CMAKE_MODULE_PATH ${LLVM_DIR})
 
   # They are used as destination of target generators.
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)


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

[PATCH] D54459: [analyzer] Dump reproducible identifiers for objects under construction.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, 
rnkovacs.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, baloghadamsoftware.

This continues the work that was  started in https://reviews.llvm.org/D51826, 
which now gets applied to object-under-construction tracking for C++.


Repository:
  rC Clang

https://reviews.llvm.org/D54459

Files:
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/dump_egraph.cpp


Index: test/Analysis/dump_egraph.cpp
===
--- test/Analysis/dump_egraph.cpp
+++ test/Analysis/dump_egraph.cpp
@@ -2,14 +2,21 @@
 // RUN: cat %t.dot | FileCheck %s
 // REQUIRES: asserts
 
-
 struct S {
   ~S();
 };
 
+struct T {
+  S s;
+  T() : s() {}
+};
+
 void foo() {
   // Test that dumping symbols conjured on null statements doesn't crash.
-  S s;
+  T t;
 }
 
-// CHECK: conj_$0\{int, LC1, no stmt, #1\}
+// CHECK: (LC1,S{{[0-9]*}},construct into local variable) T t;\n : 
+// CHECK: (LC2,I{{[0-9]*}},construct into member variable) s : \>s
+// CHECK: conj_$5\{int, LC3, no stmt, #1\}
+
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -138,9 +138,17 @@
   const ConstructionContextItem () const { return Impl.first; }
   const LocationContext *getLocationContext() const { return Impl.second; }
 
+  ASTContext () const {
+return getLocationContext()->getDecl()->getASTContext();
+  }
+
   void print(llvm::raw_ostream , PrinterHelper *Helper, PrintingPolicy ) 
{
-OS << '(' << getLocationContext() << ',' << getAnyASTNodePtr() << ','
-   << getItem().getKindAsString();
+OS << "(LC" << getLocationContext()->getID() << ',';
+if (const Stmt *S = getItem().getStmtOrNull())
+  OS << 'S' << S->getID(getASTContext());
+else
+  OS << 'I' << getItem().getCXXCtorInitializer()->getID(getASTContext());
+OS << ',' << getItem().getKindAsString();
 if (getItem().getKind() == ConstructionContextItem::ArgumentKind)
   OS << " #" << getItem().getIndex();
 OS << ") ";


Index: test/Analysis/dump_egraph.cpp
===
--- test/Analysis/dump_egraph.cpp
+++ test/Analysis/dump_egraph.cpp
@@ -2,14 +2,21 @@
 // RUN: cat %t.dot | FileCheck %s
 // REQUIRES: asserts
 
-
 struct S {
   ~S();
 };
 
+struct T {
+  S s;
+  T() : s() {}
+};
+
 void foo() {
   // Test that dumping symbols conjured on null statements doesn't crash.
-  S s;
+  T t;
 }
 
-// CHECK: conj_$0\{int, LC1, no stmt, #1\}
+// CHECK: (LC1,S{{[0-9]*}},construct into local variable) T t;\n : 
+// CHECK: (LC2,I{{[0-9]*}},construct into member variable) s : \>s
+// CHECK: conj_$5\{int, LC3, no stmt, #1\}
+
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -138,9 +138,17 @@
   const ConstructionContextItem () const { return Impl.first; }
   const LocationContext *getLocationContext() const { return Impl.second; }
 
+  ASTContext () const {
+return getLocationContext()->getDecl()->getASTContext();
+  }
+
   void print(llvm::raw_ostream , PrinterHelper *Helper, PrintingPolicy ) {
-OS << '(' << getLocationContext() << ',' << getAnyASTNodePtr() << ','
-   << getItem().getKindAsString();
+OS << "(LC" << getLocationContext()->getID() << ',';
+if (const Stmt *S = getItem().getStmtOrNull())
+  OS << 'S' << S->getID(getASTContext());
+else
+  OS << 'I' << getItem().getCXXCtorInitializer()->getID(getASTContext());
+OS << ',' << getItem().getKindAsString();
 if (getItem().getKind() == ConstructionContextItem::ArgumentKind)
   OS << " #" << getItem().getIndex();
 OS << ") ";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

rsmith added a comment.

Thank you, this is looking really good.




Comment at: include/clang/Parse/Parser.h:2054
+case DeclSpecContext::DSC_template_param:
+case DeclSpecContext::DSC_template_type_arg:
+case DeclSpecContext::DSC_normal:

Rakete wrote:
> rsmith wrote:
> > This doesn't seem right to me. Doesn't this mean that `X` will change 
> > from being a non-type template argument to being a type template argument?
> > 
> > Maybe that's a bug in the wording paper, though, since that context *is* a 
> > /type-id/.
> AFAIK no it won't, but I can't really tell if that's the right behavior or 
> not. This patch doesn't touch the `X` case.
I don't think I agree. `ParseTemplateArgument`, after disambiguation, parses a 
type name in `TemplateArgContext`, which results in a `DeclSpecContext` of 
`DSC_template_type_arg`, which means that we treat the context as an implicit 
typename context.

It looks to me like that will cause us to accept implicit `typename` in cases 
that we can disambiguate as being type template arguments, which would be 
wrong. For example, it looks like `X` would be incorrectly 
accepted: `typename` is required here because this type-id is not an implicit 
typename context.

Perhaps the right way to fix this would be to change 
`Parser::getDeclSpecContextFromDeclaratorContext` to map `TemplateArgContext` 
to a different kind of `DeclSpecContext` than `TemplateTypeArgContext` so that 
we can tell the difference here.



Comment at: lib/Parse/ParseDecl.cpp:6433
 
-ParseDeclarationSpecifiers(DS);
+bool AllowImplicitTypename;
+if (D.getContext() == DeclaratorContext::MemberContext ||

We should determine this up-front rather than computing it once per parameter 
that we parse.



Comment at: lib/Parse/ParseDecl.cpp:6439
+  AllowImplicitTypename =
+  D.getCXXScopeSpec().isSet() && Actions.isDeclaratorFunctionLike(D);
 

I don't think you need to do name lookup here. I think you can use 
`D.isFunctionDeclaratorAFunctionDeclaration()` instead of 
`isDeclaratorFunctionLike(D)` -- because we've already committed to parsing 
this as a function declaration in that case, a qualified function name will 
either redeclare something from its context (in which case implicit typename is 
permitted) or be ill-formed.



Comment at: lib/Parse/Parser.cpp:1518
+if (TryAnnotateTypeOrScopeTokenAfterScopeSpec(
+SS, !WasScopeAnnotation, /*AllowImplicitTypename=*/true))
   return ANK_Error;

What justifies allowing implicit typename here? Don't we need the caller to 
tell us if that's OK?

... actually, perhaps this tentatively-declared identifiers logic should only 
be performed if `SS.isEmpty()` (because a tentatively-declared identifier can 
never be found within a scope named by a nested-name-specifier), at which point 
the value of this flag doesn't matter.



Comment at: lib/Parse/Parser.cpp:1783
+AllowImplicitTypename &&
+getCurScope()->isFunctionPrototypeScope())) {
   SourceLocation BeginLoc = Tok.getLocation();

Do we need this scope check? (It would seem preferable to trust the caller to 
have passed in the right flag value, if we can.)



Comment at: lib/Sema/Sema.cpp:2006-2019
+bool Sema::isDeclaratorFunctionLike(const Declarator ) {
+  assert(D.getCXXScopeSpec().isSet() &&
+ "can only be called for qualified names");
+  LookupResult LR(*this, D.getIdentifier(), D.getBeginLoc(), 
LookupOrdinaryName,
+  ForVisibleRedeclaration);
+  DeclContext *DC = computeDeclContext(D.getCXXScopeSpec());
+  if (!DC)

Some thoughts on this:

 * Can this be unified with the lookup code in `HandleDeclarator`? This is 
really the same lookup, repeated in two places.
 * It'd be nice to cache this lookup, rather than performing it three times 
(once when disambiguating a parenthesized initializer from a function 
declaration, once when we're about to parse a parameter-declaration-clause, and 
once in `HandleDeclarator` after parsing completes -- though at least that's 
reduced to two lookups if you make the change I suggested in 
`ParseParameterDeclarationClause`)
 * If we don't do the caching, what happens if lookup fails due to ambiguity? 
Do we get the same error multiple times (once for each time we perform the 
lookup)?



Comment at: lib/Sema/Sema.cpp:2010
+  LookupResult LR(*this, D.getIdentifier(), D.getBeginLoc(), 
LookupOrdinaryName,
+  ForVisibleRedeclaration);
+  DeclContext *DC = computeDeclContext(D.getCXXScopeSpec());

Should this be `forRedeclarationInCurContext()`?



Comment at: lib/Sema/Sema.cpp:2011
+  ForVisibleRedeclaration);
+  DeclContext *DC = computeDeclContext(D.getCXXScopeSpec());
+  if (!DC)

[PATCH] D54457: [AST] Generate unique identifiers for CXXCtorInitializer objects.

[George Karpenkov via Phabricator via cfe-commits]

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



Comment at: lib/AST/DeclCXX.cpp:2249
 
+int64_t CXXCtorInitializer::getID(const ASTContext ) const {
+  Optional Out = Context.getAllocator().identifyObject(this);

Should we factor out this code? It's a fourth duplication now, probably a 
generic form should be in the allocator.


Repository:
  rC Clang

https://reviews.llvm.org/D54457



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

[PATCH] D54457: [AST] Generate unique identifiers for CXXCtorInitializer objects.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: george.karpenkov, rsmith.
Herald added a subscriber: cfe-commits.

This follows https://reviews.llvm.org/D51822 and 
https://reviews.llvm.org/D52113 to add a cheap way of obtaining a unique and 
relatively stable* numeric identifier for `CXXCtorInitializer`s.

__
*I.e., reproducible across runs but not necessarily across different host 
machines.


Repository:
  rC Clang

https://reviews.llvm.org/D54457

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


Index: lib/AST/DeclCXX.cpp
===
--- lib/AST/DeclCXX.cpp
+++ lib/AST/DeclCXX.cpp
@@ -2246,6 +2246,14 @@
 : Initializee(TInfo), Init(Init), LParenLoc(L), RParenLoc(R),
   IsDelegating(true), IsVirtual(false), IsWritten(false), SourceOrder(0) {}
 
+int64_t CXXCtorInitializer::getID(const ASTContext ) const {
+  Optional Out = Context.getAllocator().identifyObject(this);
+  assert(Out && "Wrong allocator used");
+  assert(*Out % alignof(CXXCtorInitializer) == 0 &&
+ "Wrong alignment information");
+  return *Out / alignof(CXXCtorInitializer);
+}
+
 TypeLoc CXXCtorInitializer::getBaseClassLoc() const {
   if (isBaseInitializer())
 return Initializee.get()->getTypeLoc();
Index: include/clang/AST/DeclCXX.h
===
--- include/clang/AST/DeclCXX.h
+++ include/clang/AST/DeclCXX.h
@@ -2315,6 +2315,9 @@
   CXXCtorInitializer(ASTContext , TypeSourceInfo *TInfo,
  SourceLocation L, Expr *Init, SourceLocation R);
 
+  /// \return Unique reproducible object identifier.
+  int64_t getID(const ASTContext ) const;
+
   /// Determine whether this initializer is initializing a base class.
   bool isBaseInitializer() const {
 return Initializee.is() && !IsDelegating;


Index: lib/AST/DeclCXX.cpp
===
--- lib/AST/DeclCXX.cpp
+++ lib/AST/DeclCXX.cpp
@@ -2246,6 +2246,14 @@
 : Initializee(TInfo), Init(Init), LParenLoc(L), RParenLoc(R),
   IsDelegating(true), IsVirtual(false), IsWritten(false), SourceOrder(0) {}
 
+int64_t CXXCtorInitializer::getID(const ASTContext ) const {
+  Optional Out = Context.getAllocator().identifyObject(this);
+  assert(Out && "Wrong allocator used");
+  assert(*Out % alignof(CXXCtorInitializer) == 0 &&
+ "Wrong alignment information");
+  return *Out / alignof(CXXCtorInitializer);
+}
+
 TypeLoc CXXCtorInitializer::getBaseClassLoc() const {
   if (isBaseInitializer())
 return Initializee.get()->getTypeLoc();
Index: include/clang/AST/DeclCXX.h
===
--- include/clang/AST/DeclCXX.h
+++ include/clang/AST/DeclCXX.h
@@ -2315,6 +2315,9 @@
   CXXCtorInitializer(ASTContext , TypeSourceInfo *TInfo,
  SourceLocation L, Expr *Init, SourceLocation R);
 
+  /// \return Unique reproducible object identifier.
+  int64_t getID(const ASTContext ) const;
+
   /// Determine whether this initializer is initializing a base class.
   bool isBaseInitializer() const {
 return Initializee.is() && !IsDelegating;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54425: [AArch64] Add aarch64_vector_pcs function attribute to Clang

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:1792
+  let Spellings = [GNU<"aarch64_vector_pcs">,
+   CXX11<"clang", "aarch64_vector_pcs">,
+   Keyword<"__aarch64_vector_pcs">,

Rather than using GNU and CXX11 spellings, you should use the Clang spelling. 
If the attribute is not useful in C, then set `allowInC` to 0 instead of its 
default of 1.



Comment at: include/clang/Basic/Attr.td:1794
+   Keyword<"__aarch64_vector_pcs">,
+   Keyword<"_aarch64_vector_pcs">];
+  let Documentation = [AArch64VectorPcsDocs];

This steps on the user's namespace -- is that intended and necessary?



Comment at: include/clang/Basic/AttrDocs.td:1748
+  let Content = [{
+On 64 bit ARM targets, this argument causes the function to obey the vector
+procedural call standard (VPCS) rules as described in the Vector ABI for

64 bit -> 64-bit


https://reviews.llvm.org/D54425



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

[PATCH] D53982: Output "rule" information in SARIF

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:237-242
+#define GET_CHECKERS
+#define CHECKER(FULLNAME, CLASS, CXXFILE, HELPTEXT, GROUPINDEX, HIDDEN)
\
+  .Case(FULLNAME, HELPTEXT)
+#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
+#undef CHECKER
+#undef GET_CHECKERS

Szelethus wrote:
> aaron.ballman wrote:
> > Szelethus wrote:
> > > aaron.ballman wrote:
> > > > Szelethus wrote:
> > > > > Szelethus wrote:
> > > > > > Hmmm, this won't handle checkers loaded from plugins.
> > > > > I don't immediately know the solution is to this, but when you invoke 
> > > > > clang with `-cc1 -analyzer-checker-help`, plugin checkers are also 
> > > > > displayed, and [[ 
> > > > > https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp#L150
> > > > >  | this is line that magically does it ]].
> > > > > Maybe store the plugins in `AnalyzerOptions`, and move 
> > > > > `ClangCheckerRegistry` to `include/clang/StaticAnalyzer/Frontend`?
> > > > Oof, this is a good point. Thank you for bringing it up! I'll look into 
> > > > this as soon as I have the chance.
> > > > 
> > > > Do we have any tests that use plugins (preferably on Windows) so that I 
> > > > have a way to test this functionality out?
> > > I'm working on the exact same issue, which is why I spotted this! There 
> > > are some in `unittests/`, but I plan on enhancing them //a lot// in the 
> > > coming days. I'll make sure to add you as subscriber on related paches, 
> > > until then, unless users of the Saris output are likely to use the 
> > > StaticAnalyzer with plugins, I think feel free to wait on my progress. I 
> > > expect to come out with a patch during the week. But you never know, I 
> > > guess.
> > > 
> > > https://github.com/llvm-mirror/clang/blob/master/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
> > I'm fine with waiting. I don't expect to see heavy use of the Sarif 
> > exporter until the spec is finalized, as it's hard to write a Sarif viewer 
> > against a moving target. So we've got a few months to find these sort of 
> > things out.
> > 
> > Thank you for CCing me on the related patches!
> Well, yea, skeletons are falling out of the closet left and right, so I'll 
> need to clean a lot of other things up before finding a neat solution to 
> this. Please let me know when you need to make progress on this.
It's not a high priority for me -- I'm not certain about the intersection of 
users of the static analyzer with plugins and folks who want to export SARIF. 
I'm sure such people will exist someday, but it seems like a small audience to 
fret over for the moment.


https://reviews.llvm.org/D53982



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

[PATCH] D54258: [Clang] Fix pretty printing of CUDA address spaces

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D54258#1295158, @richardmembarth wrote:

> There are external tools (e.g. hipacc ) that 
> generate Clang AST. This AST uses `LangAS` annotations and emits incorrect 
> memory space specifiers for CUDA when pretty-printed.


That's good to know!

> We would need a different frontend that annotates LangAS::cuda_shared.

Do you happen to know why this behaves the way it does? e.g., is the bug that 
the frontend is annotating incorrectly and forces codegen to work around it, 
and fixing the frontend to annotate properly lets us remove some workarounds 
and fixes your issue?


Repository:
  rC Clang

https://reviews.llvm.org/D54258



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

[PATCH] D54405: Record whether a AST Matcher constructs a Node

[Zachary Turner via Phabricator via cfe-commits]

zturner added inline comments.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:77
+  internal::MatcherDescriptor *matchDescriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind<
+typename 
ast_matchers::internal::VariadicAllOfMatcher::Type>();

steveire wrote:
> aaron.ballman wrote:
> > zturner wrote:
> > > steveire wrote:
> > > > aaron.ballman wrote:
> > > > > steveire wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Mildly not keen on the use of `auto` here. It's a factory 
> > > > > > > function, so it kind of names the resulting type, but it also 
> > > > > > > looks like the type will be 
> > > > > > > `ast_matchers::internal::VariadicAllOfMatcher::Type` 
> > > > > > > from the template argument, which is incorrect.
> > > > > > There is no reason to assume that taking a template argument means 
> > > > > > that type is result.
> > > > > > 
> > > > > > The method is `getFrom` which decreases the ambiguity still further.
> > > > > > 
> > > > > > Spelling out the type doesn't add anything useful. This should be 
> > > > > > ok.
> > > > > > There is no reason to assume that taking a template argument means 
> > > > > > that type is result.
> > > > > 
> > > > > Aside from all the other places that do exactly that (getAs<>, 
> > > > > cast<>, dyn_cast<>, castAs<>, and so on). Generally, when we have a 
> > > > > function named get that takes a template type argument, the result 
> > > > > when seen in proximity to auto is the template type.
> > > > > 
> > > > > > Spelling out the type doesn't add anything useful. This should be 
> > > > > > ok.
> > > > > 
> > > > > I slept on this one and fall on the other side of it; using auto 
> > > > > hides information that tripped up at least one person when reading 
> > > > > the code, so don't use auto. It's not clear enough what the resulting 
> > > > > type will be.
> > > > I put this in the category of requiring 
> > > > 
> > > > SomeType ST = SomeType::create();
> > > > 
> > > > instead of 
> > > > 
> > > > auto ST = SomeType::create();
> > > > 
> > > > `ast_type_traits::ASTNodeKind` is already on that line and you want to 
> > > > see it again.
> > > > 
> > > FWIW I'm with Aaron here.  Im' not familiar at all with this codebase, 
> > > and looking at the code, my first instinct is "the result type is 
> > > probably `ast_matchers::internal::VariadicAllOfMatcher::Type`".  
> > > According to Aaron's earlier comment, that is incorrect, so there's at 
> > > least 1 data point that it hinders readability.
> > > 
> > > So, honest question.  What *is* the return type here?
> > > So, honest question. What *is* the return type here?
> > 
> > Much to my surprise, it's `ASTNodeKind`. It was entirely unobvious to me 
> > that this was a factory function.
> @zturner Quoting myself:
> 
> > `ast_type_traits::ASTNodeKind` is already on that line and you want to see 
> > it again.
> 
> The expression is `getFromNodeKind`. There is a pattern of 
> `SomeType::fromFooKind()` returning a `SomeType`. This is not so 
> unusual.
> 
> 
Note that at the top of this file there's already a `using namespace 
clang::ast_type_traits;`  So if you're worried about verbosity, then you can 
remove the `ast_type_traits::`, remove the `auto`, and the net effect is that 
the overall line will end up being shorter.


Repository:
  rC Clang

https://reviews.llvm.org/D54405



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

[PATCH] D54246: [clang-tidy] Add the abseil-duration-factory-scale check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:57-58
+// One and only one of `IntLit` and `FloatLit` should be provided.
+static double GetValue(const IntegerLiteral *IntLit,
+   const FloatingLiteral *FloatLit) {
+  if (IntLit) {

hwright wrote:
> aaron.ballman wrote:
> > I really don't like this interface where you pass two arguments, only one 
> > of which is ever valid. That is pretty confusing. Given that the result of 
> > this function is only ever passed to `GetNewMultScale()`, and that function 
> > only does integral checks, I'd prefer logic more like:
> > 
> > * If the literal is integral, get its value and call `GetNewMultScale()`.
> > * If the literal is float, convert it to an integral and call 
> > `GetNewMultScale()` only if the conversion is exact (this can be done via 
> > `APFloat::convertToInteger()`).
> > * `GetNewMultScale()` can now accept an integer value and removes the 
> > questions about inexact equality tests from the function.
> > 
> > With that logic, I don't see a need for `GetValue()` at all, but if a 
> > helper function is useful, I'd probably guess this is a better signature: 
> > `int64_t getIntegralValue(const Expr *Literal, bool );`
> >  Given that the result of this function is only ever passed to 
> > `GetNewMultScale()`, and that function only does integral checks, I'd 
> > prefer logic more like:
> 
> That's actually not true: `GetNewMultScale()` does checks against values like 
> `1e-3` which aren't integers.  Does this change your suggestion?
Hmm, yeah, I suppose it has to! :-D



Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:63
+  assert(FloatLit != nullptr && "Neither IntLit nor FloatLit set");
+  return FloatLit->getValueAsApproximateDouble();
+}

hwright wrote:
> aaron.ballman wrote:
> > I believe the approximate results here can lead to bugs where the 
> > floating-point literal is subnormal -- it may return 0.0 for literals that 
> > are not zero.
> Do you have an example which I could put in a test?
`0x0.01p-126f` should get you a new, exciting way to spell `0`.



Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:81-84
+if (Multiplier == 60.0)
+  return DurationScale::Minutes;
+if (Multiplier == 1e-3)
+  return DurationScale::Milliseconds;

hwright wrote:
> aaron.ballman wrote:
> > What about scaling with a multiplier of 3600 to go from seconds to hours, 
> > and other plausible conversions?
> That's a good point, and part of a broader design discussion: should we 
> support all multipliers?  (e.g., what about multiplying microseconds by 
> `1.0/864.0`?)
> 
> If we do think it's worth handling all of these cases, we probably want a 
> different construct than the equivalent of a lookup table to do this 
> computation.
> That's a good point, and part of a broader design discussion: should we 
> support all multipliers?

That's kind of what I'm leaning towards. It's certainly more explainable to 
users that all the various scaling operations just work, rather than some 
particular set.

However, maybe you know more about the user base than I do and there's a sound 
reason to not handle all cases?

> If we do think it's worth handling all of these cases, we probably want a 
> different construct than the equivalent of a lookup table to do this 
> computation.

There's a part of me that wonders if we can use `std::ratio` to describe the 
scaling operations, but I freely admit I've not thought about implementation 
strategies all that hard.


https://reviews.llvm.org/D54246



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

[PATCH] D54372: [analyzer] MisusedMovedObject: NFC: Remove dead code after D18860

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 173779.
NoQ added a comment.

Bring back the early return for destructors. Cleaning up the state might be 
unnecessary, but calling a destructor on a moved object is still fine and 
should not cause a warning.

Add a test because this wasn't caught by tests.


https://reviews.llvm.org/D54372

Files:
  lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
  test/Analysis/MisusedMovedObject.cpp

Index: test/Analysis/MisusedMovedObject.cpp
===
--- test/Analysis/MisusedMovedObject.cpp
+++ test/Analysis/MisusedMovedObject.cpp
@@ -710,3 +710,15 @@
 Empty f = std::move(e); // no-warning
   }
 }
+
+struct MoveOnlyWithDestructor {
+  MoveOnlyWithDestructor();
+  ~MoveOnlyWithDestructor();
+  MoveOnlyWithDestructor(const MoveOnlyWithDestructor ) = delete;
+  MoveOnlyWithDestructor(MoveOnlyWithDestructor &);
+};
+
+MoveOnlyWithDestructor foo() {
+  MoveOnlyWithDestructor m;
+  return m;
+}
Index: lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
@@ -43,10 +43,9 @@
 };
 
 class MisusedMovedObjectChecker
-: public Checker {
 public:
-  void checkEndFunction(const ReturnStmt *RS, CheckerContext ) const;
   void checkPreCall(const CallEvent , CheckerContext ) const;
   void checkPostCall(const CallEvent , CheckerContext ) const;
   void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
@@ -218,42 +217,6 @@
   return nullptr;
 }
 
-// Removing the function parameters' MemRegion from the state. This is needed
-// for PODs where the trivial destructor does not even created nor executed.
-void MisusedMovedObjectChecker::checkEndFunction(const ReturnStmt *RS,
- CheckerContext ) const {
-  auto State = C.getState();
-  TrackedRegionMapTy Objects = State->get();
-  if (Objects.isEmpty())
-return;
-
-  auto LC = C.getLocationContext();
-
-  const auto LD = dyn_cast_or_null(LC->getDecl());
-  if (!LD)
-return;
-  llvm::SmallSet InvalidRegions;
-
-  for (auto Param : LD->parameters()) {
-auto Type = Param->getType().getTypePtrOrNull();
-if (!Type)
-  continue;
-if (!Type->isPointerType() && !Type->isReferenceType()) {
-  InvalidRegions.insert(State->getLValue(Param, LC).getAsRegion());
-}
-  }
-
-  if (InvalidRegions.empty())
-return;
-
-  for (const auto  : State->get()) {
-if (InvalidRegions.count(E.first->getBaseRegion()))
-  State = State->remove(E.first);
-  }
-
-  C.addTransition(State);
-}
-
 void MisusedMovedObjectChecker::checkPostCall(const CallEvent ,
   CheckerContext ) const {
   const auto *AFC = dyn_cast();
@@ -384,20 +347,18 @@
 return;
   }
 
+  // Calling a destructor on a moved object is fine.
+  if (isa())
+return;
+
   const auto IC = dyn_cast();
   if (!IC)
 return;
   // In case of destructor call we do not track the object anymore.
   const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
   if (!ThisRegion)
 return;
 
-  if (dyn_cast_or_null(Call.getDecl())) {
-State = removeFromState(State, ThisRegion);
-C.addTransition(State);
-return;
-  }
-
   const auto MethodDecl = dyn_cast_or_null(IC->getDecl());
   if (!MethodDecl)
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

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

Szelethus added a comment.

Actually, no. The main problem here is that `InnerPointerChecker` doesn't only 
want to register `MallocBase`, it needs to modify the checker object. In any 
case, either `MallocChecker.cpp` needs the definition of `InnerPointerChecker`, 
or vice versa. Sure, I could separate out the part that adds a new way to 
register dependencies, but it doesn't stop anyone from making the same mistake 
again, so what would be the point of that? I think it would be just a lot 
cleaner to split those checkers up properly, and completely replace the current 
system of registering checkers to stop anyone accidentally doing the same 
mistake again.


Repository:
  rC Clang

https://reviews.llvm.org/D54438



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

[PATCH] D54405: Record whether a AST Matcher constructs a Node

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:77
+  internal::MatcherDescriptor *matchDescriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind<
+typename 
ast_matchers::internal::VariadicAllOfMatcher::Type>();

aaron.ballman wrote:
> zturner wrote:
> > steveire wrote:
> > > aaron.ballman wrote:
> > > > steveire wrote:
> > > > > aaron.ballman wrote:
> > > > > > Mildly not keen on the use of `auto` here. It's a factory function, 
> > > > > > so it kind of names the resulting type, but it also looks like the 
> > > > > > type will be 
> > > > > > `ast_matchers::internal::VariadicAllOfMatcher::Type` from 
> > > > > > the template argument, which is incorrect.
> > > > > There is no reason to assume that taking a template argument means 
> > > > > that type is result.
> > > > > 
> > > > > The method is `getFrom` which decreases the ambiguity still further.
> > > > > 
> > > > > Spelling out the type doesn't add anything useful. This should be ok.
> > > > > There is no reason to assume that taking a template argument means 
> > > > > that type is result.
> > > > 
> > > > Aside from all the other places that do exactly that (getAs<>, cast<>, 
> > > > dyn_cast<>, castAs<>, and so on). Generally, when we have a function 
> > > > named get that takes a template type argument, the result when seen in 
> > > > proximity to auto is the template type.
> > > > 
> > > > > Spelling out the type doesn't add anything useful. This should be ok.
> > > > 
> > > > I slept on this one and fall on the other side of it; using auto hides 
> > > > information that tripped up at least one person when reading the code, 
> > > > so don't use auto. It's not clear enough what the resulting type will 
> > > > be.
> > > I put this in the category of requiring 
> > > 
> > > SomeType ST = SomeType::create();
> > > 
> > > instead of 
> > > 
> > > auto ST = SomeType::create();
> > > 
> > > `ast_type_traits::ASTNodeKind` is already on that line and you want to 
> > > see it again.
> > > 
> > FWIW I'm with Aaron here.  Im' not familiar at all with this codebase, and 
> > looking at the code, my first instinct is "the result type is probably 
> > `ast_matchers::internal::VariadicAllOfMatcher::Type`".  According 
> > to Aaron's earlier comment, that is incorrect, so there's at least 1 data 
> > point that it hinders readability.
> > 
> > So, honest question.  What *is* the return type here?
> > So, honest question. What *is* the return type here?
> 
> Much to my surprise, it's `ASTNodeKind`. It was entirely unobvious to me that 
> this was a factory function.
@zturner Quoting myself:

> `ast_type_traits::ASTNodeKind` is already on that line and you want to see it 
> again.

The expression is `getFromNodeKind`. There is a pattern of 
`SomeType::fromFooKind()` returning a `SomeType`. This is not so 
unusual.




Repository:
  rC Clang

https://reviews.llvm.org/D54405



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

[PATCH] D54405: Record whether a AST Matcher constructs a Node

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:77
+  internal::MatcherDescriptor *matchDescriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind<
+typename 
ast_matchers::internal::VariadicAllOfMatcher::Type>();

zturner wrote:
> steveire wrote:
> > aaron.ballman wrote:
> > > steveire wrote:
> > > > aaron.ballman wrote:
> > > > > Mildly not keen on the use of `auto` here. It's a factory function, 
> > > > > so it kind of names the resulting type, but it also looks like the 
> > > > > type will be 
> > > > > `ast_matchers::internal::VariadicAllOfMatcher::Type` from 
> > > > > the template argument, which is incorrect.
> > > > There is no reason to assume that taking a template argument means that 
> > > > type is result.
> > > > 
> > > > The method is `getFrom` which decreases the ambiguity still further.
> > > > 
> > > > Spelling out the type doesn't add anything useful. This should be ok.
> > > > There is no reason to assume that taking a template argument means that 
> > > > type is result.
> > > 
> > > Aside from all the other places that do exactly that (getAs<>, cast<>, 
> > > dyn_cast<>, castAs<>, and so on). Generally, when we have a function 
> > > named get that takes a template type argument, the result when seen in 
> > > proximity to auto is the template type.
> > > 
> > > > Spelling out the type doesn't add anything useful. This should be ok.
> > > 
> > > I slept on this one and fall on the other side of it; using auto hides 
> > > information that tripped up at least one person when reading the code, so 
> > > don't use auto. It's not clear enough what the resulting type will be.
> > I put this in the category of requiring 
> > 
> > SomeType ST = SomeType::create();
> > 
> > instead of 
> > 
> > auto ST = SomeType::create();
> > 
> > `ast_type_traits::ASTNodeKind` is already on that line and you want to see 
> > it again.
> > 
> FWIW I'm with Aaron here.  Im' not familiar at all with this codebase, and 
> looking at the code, my first instinct is "the result type is probably 
> `ast_matchers::internal::VariadicAllOfMatcher::Type`".  According to 
> Aaron's earlier comment, that is incorrect, so there's at least 1 data point 
> that it hinders readability.
> 
> So, honest question.  What *is* the return type here?
> So, honest question. What *is* the return type here?

Much to my surprise, it's `ASTNodeKind`. It was entirely unobvious to me that 
this was a factory function.


Repository:
  rC Clang

https://reviews.llvm.org/D54405



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

[PATCH] D54402: Extract method to allow re-use

[Aaron Ballman via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D54402#1296273, @steveire wrote:

> This is just a NFC change, which is normal to appear without tests. The 
> consensus on IRC is that this is fine.


This NFC would likely be rejected were it not for your other patches because it 
would serve no purpose while adding complexity and overhead. I'm not certain 
why you feel so strongly about not merging this with other patches. We ask 
developers to do that for new features because it makes it trivial for us to 
know what to revert when a random bot goes red. That said, if you continue to 
feel strongly about this, I'll hold my nose.

LGTM with some formatting nits.




Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:565
+template 
+void processAcceptableMatchers(ArrayRef AcceptedTypes, Callable 
&) {
 

80 col.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:594-595
 
 if (!RetKinds.empty() && MaxSpecificity > 0) {
-  std::string Decl;
-  llvm::raw_string_ostream OS(Decl);
-
-  if (IsPolymorphic) {
-OS << "Matcher " << Name << "(Matcher";
-  } else {
-OS << "Matcher<" << RetKinds << "> " << Name << "(";
-for (const std::vector  : ArgsKinds) {
-  if ( != [0])
-OS << ", ";
-
-  bool FirstArgKind = true;
-  std::set MatcherKinds;
-  // Two steps. First all non-matchers, then matchers only.
-  for (const ArgKind  : Arg) {
-if (AK.getArgKind() == ArgKind::AK_Matcher) {
-  MatcherKinds.insert(AK.getMatcherKind());
-} else {
-  if (!FirstArgKind) OS << "|";
-  FirstArgKind = false;
-  OS << AK.asString();
+  std::forward(Func)(Name, Matcher, RetKinds, ArgsKinds, 
MaxSpecificity);
+}

80 col and elide braces.


Repository:
  rC Clang

https://reviews.llvm.org/D54402



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

[PATCH] D54405: Record whether a AST Matcher constructs a Node

[Zachary Turner via Phabricator via cfe-commits]

zturner added inline comments.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:77
+  internal::MatcherDescriptor *matchDescriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind<
+typename 
ast_matchers::internal::VariadicAllOfMatcher::Type>();

steveire wrote:
> aaron.ballman wrote:
> > steveire wrote:
> > > aaron.ballman wrote:
> > > > Mildly not keen on the use of `auto` here. It's a factory function, so 
> > > > it kind of names the resulting type, but it also looks like the type 
> > > > will be `ast_matchers::internal::VariadicAllOfMatcher::Type` 
> > > > from the template argument, which is incorrect.
> > > There is no reason to assume that taking a template argument means that 
> > > type is result.
> > > 
> > > The method is `getFrom` which decreases the ambiguity still further.
> > > 
> > > Spelling out the type doesn't add anything useful. This should be ok.
> > > There is no reason to assume that taking a template argument means that 
> > > type is result.
> > 
> > Aside from all the other places that do exactly that (getAs<>, cast<>, 
> > dyn_cast<>, castAs<>, and so on). Generally, when we have a function named 
> > get that takes a template type argument, the result when seen in proximity 
> > to auto is the template type.
> > 
> > > Spelling out the type doesn't add anything useful. This should be ok.
> > 
> > I slept on this one and fall on the other side of it; using auto hides 
> > information that tripped up at least one person when reading the code, so 
> > don't use auto. It's not clear enough what the resulting type will be.
> I put this in the category of requiring 
> 
> SomeType ST = SomeType::create();
> 
> instead of 
> 
> auto ST = SomeType::create();
> 
> `ast_type_traits::ASTNodeKind` is already on that line and you want to see it 
> again.
> 
FWIW I'm with Aaron here.  Im' not familiar at all with this codebase, and 
looking at the code, my first instinct is "the result type is probably 
`ast_matchers::internal::VariadicAllOfMatcher::Type`".  According to 
Aaron's earlier comment, that is incorrect, so there's at least 1 data point 
that it hinders readability.

So, honest question.  What *is* the return type here?


Repository:
  rC Clang

https://reviews.llvm.org/D54405



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

[PATCH] D54453: Remove myself as owner of clang-query.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for all of your hard work on clang-query!

I'm happy to pick up the torch as I seem to do a fair amount of the reviews for 
the tool, but I'm also happy to let Manuel or Alex pick it up if they would 
prefer.


Repository:
  rL LLVM

https://reviews.llvm.org/D54453



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

[PATCH] D54405: Record whether a AST Matcher constructs a Node

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:77
+  internal::MatcherDescriptor *matchDescriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind<
+typename 
ast_matchers::internal::VariadicAllOfMatcher::Type>();

aaron.ballman wrote:
> steveire wrote:
> > aaron.ballman wrote:
> > > Mildly not keen on the use of `auto` here. It's a factory function, so it 
> > > kind of names the resulting type, but it also looks like the type will be 
> > > `ast_matchers::internal::VariadicAllOfMatcher::Type` from the 
> > > template argument, which is incorrect.
> > There is no reason to assume that taking a template argument means that 
> > type is result.
> > 
> > The method is `getFrom` which decreases the ambiguity still further.
> > 
> > Spelling out the type doesn't add anything useful. This should be ok.
> > There is no reason to assume that taking a template argument means that 
> > type is result.
> 
> Aside from all the other places that do exactly that (getAs<>, cast<>, 
> dyn_cast<>, castAs<>, and so on). Generally, when we have a function named 
> get that takes a template type argument, the result when seen in proximity to 
> auto is the template type.
> 
> > Spelling out the type doesn't add anything useful. This should be ok.
> 
> I slept on this one and fall on the other side of it; using auto hides 
> information that tripped up at least one person when reading the code, so 
> don't use auto. It's not clear enough what the resulting type will be.
I put this in the category of requiring 

SomeType ST = SomeType::create();

instead of 

auto ST = SomeType::create();

`ast_type_traits::ASTNodeKind` is already on that line and you want to see it 
again.



Repository:
  rC Clang

https://reviews.llvm.org/D54405



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

[PATCH] D54399: Move ExprMutationAnalyzer to Tooling/Analysis (1/3)

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Could you fix the modulemap (might amount to reverting the change Eric made in 
r342827? or maybe it's more involved than that) & validate that the modules 
build is working with this change (probably undo Eric's change, validate that 
you see the breakage that Eric was trying to fix, then apply your change & see 
if it clears up as well)?


Repository:
  rC Clang

https://reviews.llvm.org/D54399



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

[PATCH] D54166: [AST] Store the string data in StringLiteral in a trailing array of chars

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In https://reviews.llvm.org/D54166#1295730, @riccibruno wrote:

> @dblaikie Thanks for looking at this patch !
>
> I have a set of patches shrinking the other statements/expressions.
>  Can I add you to review some of these too ? Most of them consists of just 
> moving
>  some data. I added rsmith but I think that he is busy with the standard.


Sure thing! do pull out as many bits of cleanup, renaming, refactoring, etc, 
into separate patches (no worries if you have to also send those for review 
(not sure if you have commit access/what level of stuff you want to 
review-after-commit, etc) - still much easier to review in little pieces than 
all together)


Repository:
  rC Clang

https://reviews.llvm.org/D54166



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

[PATCH] D54407: Record the matcher type when storing a binding

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:154
+bool operator<(const NodeEntry ) const {
+  return DynNode < other.DynNode && NodeKind < other.NodeKind;
+}

aaron.ballman wrote:
> This doesn't provide the right ordering guarantees. Use `std::tie()` instead: 
> `return std::tie(DynNode, NodeKind) < std::tie(Other.DynNode, 
> Other.NodeKind);`
Thanks, I forgot about `tie` :).


Repository:
  rC Clang

https://reviews.llvm.org/D54407



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

[PATCH] D54453: Remove myself as owner of clang-query.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added reviewers: klimek, alexfh.
Herald added a subscriber: llvm-commits.

I haven't been involved with the project for years, so it's probably
best for someone else to be the code owner.


Repository:
  rL LLVM

https://reviews.llvm.org/D54453

Files:
  clang-tools-extra/CODE_OWNERS.TXT


Index: clang-tools-extra/CODE_OWNERS.TXT
===
--- clang-tools-extra/CODE_OWNERS.TXT
+++ clang-tools-extra/CODE_OWNERS.TXT
@@ -8,10 +8,6 @@
 (W), PGP key ID and fingerprint (P), description (D), and snail-mail address
 (S).
 
-N: Peter Collingbourne
-E: pe...@pcc.me.uk
-D: clang-query
-
 N: Manuel Klimek
 E: kli...@google.com
 D: clang-rename, all parts of clang-tools-extra not covered by someone else


Index: clang-tools-extra/CODE_OWNERS.TXT
===
--- clang-tools-extra/CODE_OWNERS.TXT
+++ clang-tools-extra/CODE_OWNERS.TXT
@@ -8,10 +8,6 @@
 (W), PGP key ID and fingerprint (P), description (D), and snail-mail address
 (S).
 
-N: Peter Collingbourne
-E: pe...@pcc.me.uk
-D: clang-query
-
 N: Manuel Klimek
 E: kli...@google.com
 D: clang-rename, all parts of clang-tools-extra not covered by someone else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54407: Record the matcher type when storing a binding

[Aaron Ballman via Phabricator via cfe-commits]

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

Aside from a minor formatting nit, LGTM.




Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:154
+bool operator<(const NodeEntry ) const {
+  return std::tie(DynNode, NodeKind) < std::tie(Other.DynNode, 
Other.NodeKind);
+}

80 col.


Repository:
  rC Clang

https://reviews.llvm.org/D54407



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

[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

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

Szelethus added a comment.

In https://reviews.llvm.org/D54438#1296239, @NoQ wrote:

> Mm, i don't understand. I mean, what prevents you from cutting it off even 
> earlier and completely omitting that part of the patch? Somebody will get to 
> this later in order to see how exactly does the separation needs to be 
> performed.


I somewhat misunderstood what you meant in your earlier comment.

Originally, I created `CStringBase`, but not `MallocBase`, hence the insane 
amount of workaround, witch eventually lead to the createion of `MallocBase`, 
but I never removed the now unnecessary stuff, but now that you've shed some 
light to it, it makes little sense for it to stay around. I guess you could say 
that I couldn't see the forest for the trees.

Thanks! There's still a lot of work to be done, as this patch supplies a new 
way of expressing dependencies, but doesn't actually stop anyone from doing 
this mistake again, but let that be an issue for another time.


Repository:
  rC Clang

https://reviews.llvm.org/D54438



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

[PATCH] D54402: Extract method to allow re-use

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

This is just a NFC change, which is normal to appear without tests. The 
consensus on IRC is that this is fine.


Repository:
  rC Clang

https://reviews.llvm.org/D54402



___
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

[Artem Dergachev via Phabricator via cfe-commits]

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

Mm, ok, i admit that i don't know what specific de-duplication do we want to 
have and what are the usability implications of it.

If we want de-duplication for the same region reported in different spots of 
the same path, we need a state trait.

If we want de-duplication for the same region globally across the whole 
analysis, we need a checker-wide set of regions, like what you're doing.

If we want de-duplication for the same region globally across the whole 
translation unit, we need a checker-wide set of AST nodes (eg., field chains) 
that describe regions in terms that outlive `ExprEngine`.

If we want de-duplication for different regions within the same constructor 
call site, we need a uniqueing location at the call site, like what you're 
doing.

If we want de-duplication for different regions within the same constructor 
itself, we don't need a uniqueing location.

I guess you should just do whatever you want here?

In https://reviews.llvm.org/D51531#1286110, @Szelethus wrote:

> Oh, and the reason why I think it would add a lot of complication: since only 
> `ExprEngine` is avaible in the `checkEndAnalysis` callback, which, from what 
> I can see, doesn't have a handly `isDead` method, so I'm not even sure how 
> I'd implement it.


Mm, what do you need `isDead` for? Everything is dead at the end of the 
analysis, you need to clean up everything, otherwise you get use-after-free(?)




Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:203
   const Stmt *CallSite = Context.getStackFrame()->getCallSite();
   if (CallSite)
 LocUsedForUniqueing = PathDiagnosticLocation::createBegin(

Hmm, does `LocUsedForUniqueing` remain uninitialized sometimes? I guess it's on 
the top frame. What does it do in this case? Why do we need a uniqueing 
location in other cases? I guess it only *increases* the amount of reports, 
right? I.e., constructors called from different call sites produce different 
reports(?)


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] D54407: Record the matcher type when storing a binding

[Stephen Kelly via Phabricator via cfe-commits]

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

Update


Repository:
  rC Clang

https://reviews.llvm.org/D54407

Files:
  include/clang/ASTMatchers/ASTMatchersInternal.h
  lib/ASTMatchers/ASTMatchersInternal.cpp
  lib/Tooling/RefactoringCallbacks.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  unittests/ASTMatchers/ASTMatchersTest.h

Index: unittests/ASTMatchers/ASTMatchersTest.h
===
--- unittests/ASTMatchers/ASTMatchersTest.h
+++ unittests/ASTMatchers/ASTMatchersTest.h
@@ -349,12 +349,12 @@
   BoundNodes::IDToNodeMap::const_iterator I = M.find(Id);
   EXPECT_NE(M.end(), I);
   if (I != M.end()) {
-EXPECT_EQ(Nodes->getNodeAs(Id), I->second.get());
+EXPECT_EQ(Nodes->getNodeAs(Id), I->second.DynNode.get());
   }
   return true;
 }
 EXPECT_TRUE(M.count(Id) == 0 ||
-  M.find(Id)->second.template get() == nullptr);
+M.find(Id)->second.DynNode.get() == nullptr);
 return false;
   }
 
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1724,20 +1724,35 @@
   std::string SourceCode = "struct A { void f() {} };";
   auto Matcher = functionDecl(isDefinition()).bind("method");
 
+  using namespace ast_type_traits;
+
   auto astUnit = tooling::buildASTFromCode(SourceCode);
 
   auto GlobalBoundNodes = matchDynamic(Matcher, astUnit->getASTContext());
 
   EXPECT_EQ(GlobalBoundNodes.size(), 1u);
   EXPECT_EQ(GlobalBoundNodes[0].getMap().size(), 1u);
+  auto GlobalMapPair = *GlobalBoundNodes[0].getMap().begin();
+  EXPECT_TRUE(GlobalMapPair.second.DynNode.getNodeKind().isSame(
+  ASTNodeKind::getFromNodeKind()));
+  EXPECT_TRUE(GlobalMapPair.second.NodeKind.isSame(
+  ASTNodeKind::getFromNodeKind()));
 
   auto GlobalMethodNode = GlobalBoundNodes[0].getNodeAs("method");
   EXPECT_TRUE(GlobalMethodNode != nullptr);
 
   auto MethodBoundNodes =
   matchDynamic(Matcher, *GlobalMethodNode, astUnit->getASTContext());
   EXPECT_EQ(MethodBoundNodes.size(), 1u);
   EXPECT_EQ(MethodBoundNodes[0].getMap().size(), 1u);
+  auto MethodMapPair = *MethodBoundNodes[0].getMap().begin();
+  EXPECT_TRUE(MethodMapPair.second.DynNode.getNodeKind().isSame(
+  ASTNodeKind::getFromNodeKind()));
+  EXPECT_TRUE(MethodMapPair.second.NodeKind.isSame(
+  ASTNodeKind::getFromNodeKind()));
+  EXPECT_EQ(MethodMapPair.second.DynNode, GlobalMapPair.second.DynNode);
+  EXPECT_TRUE(
+  MethodMapPair.second.NodeKind.isSame(GlobalMapPair.second.NodeKind));
 
   auto MethodNode = MethodBoundNodes[0].getNodeAs("method");
   EXPECT_EQ(MethodNode, GlobalMethodNode);
Index: lib/Tooling/RefactoringCallbacks.cpp
===
--- lib/Tooling/RefactoringCallbacks.cpp
+++ lib/Tooling/RefactoringCallbacks.cpp
@@ -213,8 +213,8 @@
  << " used in replacement template not bound in Matcher \n";
 llvm::report_fatal_error("Unbound node in replacement template.");
   }
-  CharSourceRange Source =
-  CharSourceRange::getTokenRange(NodeIter->second.getSourceRange());
+  CharSourceRange Source = CharSourceRange::getTokenRange(
+  NodeIter->second.DynNode.getSourceRange());
   ToText += Lexer::getSourceText(Source, *Result.SourceManager,
  Result.Context->getLangOpts());
   break;
@@ -227,8 +227,8 @@
 llvm::report_fatal_error("FromId node not bound in MatchResult");
   }
   auto Replacement =
-  tooling::Replacement(*Result.SourceManager, (FromId), ToText,
-   Result.Context->getLangOpts());
+  tooling::Replacement(*Result.SourceManager, (FromId).DynNode,
+   ToText, Result.Context->getLangOpts());
   llvm::Error Err = Replace.add(Replacement);
   if (Err) {
 llvm::errs() << "Query and replace failed in " << Replacement.getFilePath()
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -52,21 +52,25 @@
 
 bool NotUnaryOperator(const ast_type_traits::DynTypedNode ,
   ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder,
+  ast_type_traits::ASTNodeKind NodeKind,
   ArrayRef InnerMatchers);
 
 bool AllOfVariadicOperator(const ast_type_traits::DynTypedNode ,
ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder,
+   ast_type_traits::ASTNodeKind NodeKind,
ArrayRef InnerMatchers);
 
 bool EachOfVariadicOperator(const ast_type_traits::DynTypedNode ,
 ASTMatchFinder 

[PATCH] D54408: Add matchers available through casting to derived

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:645
+getNodeConstructorType(MatcherCtor targetCtor) {
+  auto const  = RegistryData->nodeConstructors();
+

Don't use `auto` here (and if you did. the `const` would go on the other side).



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:647-651
+  for (auto ctor : ctors) {
+if (ctor.second.second == targetCtor)
+  return std::make_pair(ctor.first, ctor.second.first);
+  }
+  return llvm::None;

I think this can be replaced with `llvm::find_if()`.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:659
+getMatchingMatchersImpl(ast_type_traits::ASTNodeKind StaticType,
+bool ExactOnly = false) {
+

I'd prefer this to not be a default argument (the callers can pass it in). It 
might also be nice to use an enum here rather than a bool, but alternative to 
that, you should stick in comments in the callers that describe the parameter 
name. e.g. `getMatchingMatchersImpl(yada, /*ExactOnly*/true);`



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:711
+  if (ExactOnly) {
+auto IsConvertible = Matcher.isConvertibleTo(
+StaticType, , );

Don't use `auto`.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:716-718
+}
+
+{

Why do these need their own inner block scopes?



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:719
+{
+  auto TypeForMatcherOpt = getNodeConstructorType();
+  if (TypeForMatcherOpt) {

`auto`.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:720-721
+  auto TypeForMatcherOpt = getNodeConstructorType();
+  if (TypeForMatcherOpt) {
+if (StaticType.isBaseOf(TypeForMatcherOpt->first)) {
+  auto DerivedResults =

`if (TypeForMatcherOpt && StaticType.isBaseOf(...)) {`



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:722-723
+if (StaticType.isBaseOf(TypeForMatcherOpt->first)) {
+  auto DerivedResults =
+  getDerivedResults(TypeForMatcherOpt->first, Name);
+  Result.insert(Result.end(), DerivedResults.begin(),

`auto`, here and everywhere in this file.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:749
+  for (auto item : NestedResult) {
+
+auto nestedMatcherName = item.MatcherString;

Can remove whitespace around here.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:761-766
+Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
+
+  std::vector Result = getMatchingMatchersImpl(StaticType);
+
+  return Result;
+}

It seems like `getMatchingMatchersImpl()` should just be renamed to 
`getMatchingMatchers()`.


Repository:
  rC Clang

https://reviews.llvm.org/D54408



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

[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Mm, i don't understand. I mean, what prevents you from cutting it off even 
earlier and completely omitting that part of the patch? Somebody will get to 
this later in order to see how exactly does the separation needs to be 
performed.


Repository:
  rC Clang

https://reviews.llvm.org/D54438



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

[PATCH] D54407: Record the matcher type when storing a binding

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D54407#129, @sbenza wrote:

> In https://reviews.llvm.org/D54407#1294934, @aaron.ballman wrote:
>
> > Adding @sbenza in case he has opinions on this approach. I think it's 
> > reasonable, but I also know that changes to the the AST matcher internals 
> > sometimes have unintended side effects with the various instantiations.
>
>
> The problem used to come with the number of template instantiations, but 
> afaik this was with an MSVC configuration that is no longer supported.
>  In any case, I don't see this change making new template instantiations.


Ah, I think I was remembering the issues from binary sizes due to template 
instantiations. I didn't think this would introduce new instantiations, but I 
thought it might make them all larger in a harmful way. I don't think that's a 
concern here, but your second set of eyes doesn't hurt.




Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:153
+
+bool operator<(const NodeEntry ) const {
+  return DynNode < other.DynNode && NodeKind < other.NodeKind;

other -> Other



Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:154
+bool operator<(const NodeEntry ) const {
+  return DynNode < other.DynNode && NodeKind < other.NodeKind;
+}

This doesn't provide the right ordering guarantees. Use `std::tie()` instead: 
`return std::tie(DynNode, NodeKind) < std::tie(Other.DynNode, Other.NodeKind);`


Repository:
  rC Clang

https://reviews.llvm.org/D54407



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

[PATCH] D54406: Add matchDynamic convenience functions

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D54406



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

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Seems reasonable to me.


Repository:
  rC Clang

https://reviews.llvm.org/D53949



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

[PATCH] D54405: Record whether a AST Matcher constructs a Node

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Aside from the uses of auto and the lack of tests, this LGTM.




Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:77
+  internal::MatcherDescriptor *matchDescriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind<
+typename 
ast_matchers::internal::VariadicAllOfMatcher::Type>();

steveire wrote:
> aaron.ballman wrote:
> > Mildly not keen on the use of `auto` here. It's a factory function, so it 
> > kind of names the resulting type, but it also looks like the type will be 
> > `ast_matchers::internal::VariadicAllOfMatcher::Type` from the 
> > template argument, which is incorrect.
> There is no reason to assume that taking a template argument means that type 
> is result.
> 
> The method is `getFrom` which decreases the ambiguity still further.
> 
> Spelling out the type doesn't add anything useful. This should be ok.
> There is no reason to assume that taking a template argument means that type 
> is result.

Aside from all the other places that do exactly that (getAs<>, cast<>, 
dyn_cast<>, castAs<>, and so on). Generally, when we have a function named get 
that takes a template type argument, the result when seen in proximity to auto 
is the template type.

> Spelling out the type doesn't add anything useful. This should be ok.

I slept on this one and fall on the other side of it; using auto hides 
information that tripped up at least one person when reading the code, so don't 
use auto. It's not clear enough what the resulting type will be.


Repository:
  rC Clang

https://reviews.llvm.org/D54405



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

[PATCH] D54404: Exclude matchers which can have multiple results

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: sbenza.
aaron.ballman added a subscriber: sbenza.
aaron.ballman added a comment.

In https://reviews.llvm.org/D54404#1295426, @steveire wrote:

> I acknowledge and share the future-proofing concern.
>
> We could possibly use something trait-based instead and put the trait beside 
> the matcher definition in ASTMatchers.h, but that doesn't really solve the 
> problem. It only moves the problem.


If we could somehow incorporate it into the matcher definition itself, though, 
it means we don't have two lists of things to keep updated. You're right that 
it doesn't eliminate the problem -- we still have to know to ask the question 
when reviewing new matchers (so perhaps something that requires you to 
explicitly opt-in/out would be better).

Adding @sbenza in case he has ideas.




Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:604
 
+  static std::vector excludedMatchers{
+  "allOf",

aaron.ballman wrote:
> aaron.ballman wrote:
> > `excludedMatchers` -> `ExcludedMatchers`
> Please add comments that explain why these are excluded and under what 
> circumstances this list should be updated to add or remove items.
> 
> Also: how do we ensure this list stays properly updated? We sometimes miss 
> updating `RegistryMaps()`, so I'm a bit concerned that adding a second list 
> in a different location will be inviting trouble.
The comment is a bit better but still not enough to tell me what's common to 
all of these matchers. It sort of reads to me like "all the matchers with 
overloads" since overloaded matchers can be ambiguous with particular 
arguments, but that's not what I think you're going for here.

For instance, I don't see how the `anything()` matcher can be ambiguous when 
used with particular arguments -- it doesn't accept arguments and it matches 
literally everything.


Repository:
  rC Clang

https://reviews.llvm.org/D54404



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

[PATCH] D10833: Retrieve BinaryOperator::getOpcode and BinaryOperator::getOpcodeStr via libclang and its python interface

[Nemanja Boric via Phabricator via cfe-commits]

Burgos added a comment.

Another ping :-). This looks like a very useful addition (and I need it myself 
so I'll end up applying this patchset manually) and I would like to find it 
merged. It doesn't look it brings any kind of negative value to the C API.


Repository:
  rC Clang

https://reviews.llvm.org/D10833



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

[PATCH] D54429: [analyzer] Creating standard Sphinx documentation

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a reviewer: dcoughlin.
NoQ added a subscriber: dcoughlin.
NoQ added a comment.

I think we should escalate this to cfe-dev first; this sounds pretty important.

@dcoughlin should definitely say a word here, as the code owner.

I agree that pictures and screenshots are very important to keep somehow. A 
terrifying amount of users think that the right way to use Static Analyzer is 
to read raw warnings without path notes in the command line.


Repository:
  rC Clang

https://reviews.llvm.org/D54429



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

[PATCH] D54403: Add new API for returning matching matchers

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a minor commenting nit.




Comment at: include/clang/ASTMatchers/Dynamic/Registry.h:106
+  /// Compute matchers which can be used within a matcher of
+  /// type @p StaticType.
+  static std::vector

I think we typically go with \p instead of @p in our doc comments.


Repository:
  rC Clang

https://reviews.llvm.org/D54403



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

[PATCH] D54402: Extract method to allow re-use

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D54402#1295420, @steveire wrote:

> I think this commit is fine without tests.


That's not something we do except under specific extenuating circumstances (NFC 
fixes or changes for which existing coverage suffices), per our developer 
policy.

> The new method remains internal to the file but follow-up commits add public 
> interface and tests.
> 
> I'll push the commits separately. I'll not be squashing them.

I'd prefer to squash them at least into commits that have tests for the 
functionality. More specifically: if the functionality is testable, it needs 
tests when it's committed. If it's not testable, squash it into the most 
closely-related commit that is testable. In terms of review cycles, I'm fine 
with "this functionality will be commit with that functionality, whose tests 
cover this.", though I'd personally prefer future reviews to come pre-squashed 
to cut down on confusion.


Repository:
  rC Clang

https://reviews.llvm.org/D54402



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

[PATCH] D54408: Add matchers available through casting to derived

[Stephen Kelly via Phabricator via cfe-commits]

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

Update


Repository:
  rC Clang

https://reviews.llvm.org/D54408

Files:
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/Dynamic/RegistryTest.cpp

Index: unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -461,6 +461,17 @@
   "Matcher "
   "hasDescendant(Matcher)"));
 
+  CompVector FuncDeclComps = getCompletions("functionDecl", 0);
+
+  EXPECT_TRUE(!FuncDeclComps.empty());
+
+  // Exclude bases and self
+  EXPECT_TRUE(!hasCompletion(FuncDeclComps, "decl("));
+  EXPECT_TRUE(!hasCompletion(FuncDeclComps, "namedDecl("));
+  EXPECT_TRUE(!hasCompletion(FuncDeclComps, "valueDecl("));
+  EXPECT_TRUE(!hasCompletion(FuncDeclComps, "declaratorDecl("));
+  EXPECT_TRUE(!hasCompletion(FuncDeclComps, "functionDecl("));
+
   CompVector WhileComps = getCompletions("whileStmt", 0);
 
   EXPECT_TRUE(hasCompletion(WhileComps, "hasBody(",
@@ -571,14 +582,14 @@
   EXPECT_TRUE(Contains(Matchers, "hasType()"));
   EXPECT_TRUE(Contains(Matchers, "hasTypeLoc()"));
   EXPECT_TRUE(Contains(Matchers, "parameterCountIs()"));
+  EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl(isOverride())"));
 
   EXPECT_TRUE(!Contains(Matchers, "decl()"));
   EXPECT_TRUE(!Contains(Matchers, "namedDecl()"));
   EXPECT_TRUE(!Contains(Matchers, "valueDecl()"));
   EXPECT_TRUE(!Contains(Matchers, "declaratorDecl()"));
   EXPECT_TRUE(!Contains(Matchers, "functionDecl()"));
-
-  EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl()"));
+  EXPECT_TRUE(!Contains(Matchers, "cxxMethodDecl()"));
 
   EXPECT_TRUE(!Contains(Matchers, "has()"));
 }
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -638,8 +638,26 @@
   }
 }
 
+using SR = ast_matchers::dynamic::SourceRange;
+
+llvm::Optional>
+getNodeConstructorType(MatcherCtor targetCtor) {
+  auto const  = RegistryData->nodeConstructors();
+
+  for (auto ctor : ctors) {
+if (ctor.second.second == targetCtor)
+  return std::make_pair(ctor.first, ctor.second.first);
+  }
+  return llvm::None;
+}
+
 std::vector
-Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
+getDerivedResults(ast_type_traits::ASTNodeKind StaticType, StringRef Name);
+
+std::vector
+getMatchingMatchersImpl(ast_type_traits::ASTNodeKind StaticType,
+bool ExactOnly = false) {
+
   std::vector Result;
 
   // Exclude matchers which can't unambiguously
@@ -683,9 +701,33 @@
   std::vector AcceptedTypes{StaticType};
 
   processAcceptableMatchers(
-  AcceptedTypes, [](StringRef Name, const MatcherDescriptor &,
-   std::set &,
-   std::vector>, unsigned) {
+  AcceptedTypes, [&](StringRef Name, const MatcherDescriptor ,
+ std::set &,
+ std::vector>, unsigned) {
+{
+  unsigned Specificity;
+  ASTNodeKind LeastDerivedKind;
+  if (ExactOnly) {
+auto IsConvertible = Matcher.isConvertibleTo(
+StaticType, , );
+if (IsConvertible && !LeastDerivedKind.isSame(StaticType))
+  return;
+  }
+}
+
+{
+  auto TypeForMatcherOpt = getNodeConstructorType();
+  if (TypeForMatcherOpt) {
+if (StaticType.isBaseOf(TypeForMatcherOpt->first)) {
+  auto DerivedResults =
+  getDerivedResults(TypeForMatcherOpt->first, Name);
+  Result.insert(Result.end(), DerivedResults.begin(),
+DerivedResults.end());
+  return;
+}
+  }
+}
+
 if (!std::binary_search(ExcludedMatchers.begin(),
 ExcludedMatchers.end(), Name)) {
   Result.emplace_back((Name + "()").str());
@@ -695,6 +737,34 @@
   return Result;
 }
 
+std::vector
+getDerivedResults(ast_type_traits::ASTNodeKind StaticType, StringRef Name) {
+  auto NestedResult = getMatchingMatchersImpl(StaticType, true);
+
+  std::vector Result;
+
+  Diagnostics DiagnosticIgnorer;
+
+  for (auto item : NestedResult) {
+
+auto nestedMatcherName = item.MatcherString;
+
+nestedMatcherName = Name.str() + "(" + nestedMatcherName + ")";
+
+Result.emplace_back(nestedMatcherName);
+  }
+
+  return Result;
+}
+
+std::vector
+Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
+
+  std::vector Result = getMatchingMatchersImpl(StaticType);
+
+  return Result;
+}
+
 std::vector
 Registry::getMatcherCompletions(ArrayRef AcceptedTypes) {
   std::vector Completions;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D54407: Record the matcher type when storing a binding

[Stephen Kelly via Phabricator via cfe-commits]

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

Update


Repository:
  rC Clang

https://reviews.llvm.org/D54407

Files:
  include/clang/ASTMatchers/ASTMatchersInternal.h
  lib/ASTMatchers/ASTMatchersInternal.cpp
  lib/Tooling/RefactoringCallbacks.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  unittests/ASTMatchers/ASTMatchersTest.h

Index: unittests/ASTMatchers/ASTMatchersTest.h
===
--- unittests/ASTMatchers/ASTMatchersTest.h
+++ unittests/ASTMatchers/ASTMatchersTest.h
@@ -349,12 +349,12 @@
   BoundNodes::IDToNodeMap::const_iterator I = M.find(Id);
   EXPECT_NE(M.end(), I);
   if (I != M.end()) {
-EXPECT_EQ(Nodes->getNodeAs(Id), I->second.get());
+EXPECT_EQ(Nodes->getNodeAs(Id), I->second.DynNode.get());
   }
   return true;
 }
 EXPECT_TRUE(M.count(Id) == 0 ||
-  M.find(Id)->second.template get() == nullptr);
+M.find(Id)->second.DynNode.get() == nullptr);
 return false;
   }
 
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1724,20 +1724,35 @@
   std::string SourceCode = "struct A { void f() {} };";
   auto Matcher = functionDecl(isDefinition()).bind("method");
 
+  using namespace ast_type_traits;
+
   auto astUnit = tooling::buildASTFromCode(SourceCode);
 
   auto GlobalBoundNodes = matchDynamic(Matcher, astUnit->getASTContext());
 
   EXPECT_EQ(GlobalBoundNodes.size(), 1u);
   EXPECT_EQ(GlobalBoundNodes[0].getMap().size(), 1u);
+  auto GlobalMapPair = *GlobalBoundNodes[0].getMap().begin();
+  EXPECT_TRUE(GlobalMapPair.second.DynNode.getNodeKind().isSame(
+  ASTNodeKind::getFromNodeKind()));
+  EXPECT_TRUE(GlobalMapPair.second.NodeKind.isSame(
+  ASTNodeKind::getFromNodeKind()));
 
   auto GlobalMethodNode = GlobalBoundNodes[0].getNodeAs("method");
   EXPECT_TRUE(GlobalMethodNode != nullptr);
 
   auto MethodBoundNodes =
   matchDynamic(Matcher, *GlobalMethodNode, astUnit->getASTContext());
   EXPECT_EQ(MethodBoundNodes.size(), 1u);
   EXPECT_EQ(MethodBoundNodes[0].getMap().size(), 1u);
+  auto MethodMapPair = *MethodBoundNodes[0].getMap().begin();
+  EXPECT_TRUE(MethodMapPair.second.DynNode.getNodeKind().isSame(
+  ASTNodeKind::getFromNodeKind()));
+  EXPECT_TRUE(MethodMapPair.second.NodeKind.isSame(
+  ASTNodeKind::getFromNodeKind()));
+  EXPECT_EQ(MethodMapPair.second.DynNode, GlobalMapPair.second.DynNode);
+  EXPECT_TRUE(
+  MethodMapPair.second.NodeKind.isSame(GlobalMapPair.second.NodeKind));
 
   auto MethodNode = MethodBoundNodes[0].getNodeAs("method");
   EXPECT_EQ(MethodNode, GlobalMethodNode);
Index: lib/Tooling/RefactoringCallbacks.cpp
===
--- lib/Tooling/RefactoringCallbacks.cpp
+++ lib/Tooling/RefactoringCallbacks.cpp
@@ -213,8 +213,8 @@
  << " used in replacement template not bound in Matcher \n";
 llvm::report_fatal_error("Unbound node in replacement template.");
   }
-  CharSourceRange Source =
-  CharSourceRange::getTokenRange(NodeIter->second.getSourceRange());
+  CharSourceRange Source = CharSourceRange::getTokenRange(
+  NodeIter->second.DynNode.getSourceRange());
   ToText += Lexer::getSourceText(Source, *Result.SourceManager,
  Result.Context->getLangOpts());
   break;
@@ -227,8 +227,8 @@
 llvm::report_fatal_error("FromId node not bound in MatchResult");
   }
   auto Replacement =
-  tooling::Replacement(*Result.SourceManager, (FromId), ToText,
-   Result.Context->getLangOpts());
+  tooling::Replacement(*Result.SourceManager, (FromId).DynNode,
+   ToText, Result.Context->getLangOpts());
   llvm::Error Err = Replace.add(Replacement);
   if (Err) {
 llvm::errs() << "Query and replace failed in " << Replacement.getFilePath()
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -52,21 +52,25 @@
 
 bool NotUnaryOperator(const ast_type_traits::DynTypedNode ,
   ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder,
+  ast_type_traits::ASTNodeKind NodeKind,
   ArrayRef InnerMatchers);
 
 bool AllOfVariadicOperator(const ast_type_traits::DynTypedNode ,
ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder,
+   ast_type_traits::ASTNodeKind NodeKind,
ArrayRef InnerMatchers);
 
 bool EachOfVariadicOperator(const ast_type_traits::DynTypedNode ,
 ASTMatchFinder 

[PATCH] D54406: Add matchDynamic convenience functions

[Stephen Kelly via Phabricator via cfe-commits]

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

Update


Repository:
  rC Clang

https://reviews.llvm.org/D54406

Files:
  include/clang/ASTMatchers/ASTMatchFinder.h
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp


Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1719,5 +1719,29 @@
   EXPECT_FALSE(matchesObjC(ObjCStringNoPool, autoreleasePoolStmt()));
 }
 
+TEST(MatchFinderAPI, matchesDynamic) {
+
+  std::string SourceCode = "struct A { void f() {} };";
+  auto Matcher = functionDecl(isDefinition()).bind("method");
+
+  auto astUnit = tooling::buildASTFromCode(SourceCode);
+
+  auto GlobalBoundNodes = matchDynamic(Matcher, astUnit->getASTContext());
+
+  EXPECT_EQ(GlobalBoundNodes.size(), 1u);
+  EXPECT_EQ(GlobalBoundNodes[0].getMap().size(), 1u);
+
+  auto GlobalMethodNode = 
GlobalBoundNodes[0].getNodeAs("method");
+  EXPECT_TRUE(GlobalMethodNode != nullptr);
+
+  auto MethodBoundNodes =
+  matchDynamic(Matcher, *GlobalMethodNode, astUnit->getASTContext());
+  EXPECT_EQ(MethodBoundNodes.size(), 1u);
+  EXPECT_EQ(MethodBoundNodes[0].getMap().size(), 1u);
+
+  auto MethodNode = MethodBoundNodes[0].getNodeAs("method");
+  EXPECT_EQ(MethodNode, GlobalMethodNode);
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: include/clang/ASTMatchers/ASTMatchFinder.h
===
--- include/clang/ASTMatchers/ASTMatchFinder.h
+++ include/clang/ASTMatchers/ASTMatchFinder.h
@@ -310,6 +310,33 @@
   return std::move(Callback.Nodes);
 }
 
+inline SmallVector
+matchDynamic(internal::DynTypedMatcher Matcher,
+ const ast_type_traits::DynTypedNode , ASTContext ) {
+  internal::CollectMatchesCallback Callback;
+  MatchFinder Finder;
+  Finder.addDynamicMatcher(Matcher, );
+  Finder.match(Node, Context);
+  return std::move(Callback.Nodes);
+}
+
+template 
+SmallVector matchDynamic(internal::DynTypedMatcher Matcher,
+const NodeT ,
+ASTContext ) {
+  return matchDynamic(Matcher, ast_type_traits::DynTypedNode::create(Node),
+  Context);
+}
+
+inline SmallVector
+matchDynamic(internal::DynTypedMatcher Matcher, ASTContext ) {
+  internal::CollectMatchesCallback Callback;
+  MatchFinder Finder;
+  Finder.addDynamicMatcher(Matcher, );
+  Finder.matchAST(Context);
+  return std::move(Callback.Nodes);
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
 


Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1719,5 +1719,29 @@
   EXPECT_FALSE(matchesObjC(ObjCStringNoPool, autoreleasePoolStmt()));
 }
 
+TEST(MatchFinderAPI, matchesDynamic) {
+
+  std::string SourceCode = "struct A { void f() {} };";
+  auto Matcher = functionDecl(isDefinition()).bind("method");
+
+  auto astUnit = tooling::buildASTFromCode(SourceCode);
+
+  auto GlobalBoundNodes = matchDynamic(Matcher, astUnit->getASTContext());
+
+  EXPECT_EQ(GlobalBoundNodes.size(), 1u);
+  EXPECT_EQ(GlobalBoundNodes[0].getMap().size(), 1u);
+
+  auto GlobalMethodNode = GlobalBoundNodes[0].getNodeAs("method");
+  EXPECT_TRUE(GlobalMethodNode != nullptr);
+
+  auto MethodBoundNodes =
+  matchDynamic(Matcher, *GlobalMethodNode, astUnit->getASTContext());
+  EXPECT_EQ(MethodBoundNodes.size(), 1u);
+  EXPECT_EQ(MethodBoundNodes[0].getMap().size(), 1u);
+
+  auto MethodNode = MethodBoundNodes[0].getNodeAs("method");
+  EXPECT_EQ(MethodNode, GlobalMethodNode);
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: include/clang/ASTMatchers/ASTMatchFinder.h
===
--- include/clang/ASTMatchers/ASTMatchFinder.h
+++ include/clang/ASTMatchers/ASTMatchFinder.h
@@ -310,6 +310,33 @@
   return std::move(Callback.Nodes);
 }
 
+inline SmallVector
+matchDynamic(internal::DynTypedMatcher Matcher,
+ const ast_type_traits::DynTypedNode , ASTContext ) {
+  internal::CollectMatchesCallback Callback;
+  MatchFinder Finder;
+  Finder.addDynamicMatcher(Matcher, );
+  Finder.match(Node, Context);
+  return std::move(Callback.Nodes);
+}
+
+template 
+SmallVector matchDynamic(internal::DynTypedMatcher Matcher,
+const NodeT ,
+ASTContext ) {
+  return matchDynamic(Matcher, ast_type_traits::DynTypedNode::create(Node),
+  Context);
+}
+
+inline SmallVector
+matchDynamic(internal::DynTypedMatcher Matcher, ASTContext ) {
+  internal::CollectMatchesCallback Callback;
+  MatchFinder Finder;
+  Finder.addDynamicMatcher(Matcher, );
+  Finder.matchAST(Context);
+  return 

[PATCH] D54405: Record whether a AST Matcher constructs a Node

[Stephen Kelly via Phabricator via cfe-commits]

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

Update


Repository:
  rC Clang

https://reviews.llvm.org/D54405

Files:
  include/clang/ASTMatchers/ASTMatchersInternal.h
  lib/ASTMatchers/Dynamic/Registry.cpp


Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -44,18 +44,54 @@
 
 using ConstructorMap = llvm::StringMap>;
 
+using NodeConstructorMap =
+std::map>;
+
 class RegistryMaps {
 public:
   RegistryMaps();
   ~RegistryMaps();
 
   const ConstructorMap () const { return Constructors; }
+  const NodeConstructorMap () const {
+return NodeConstructors;
+  }
+
+  void registerNodeMatcher(ast_type_traits::ASTNodeKind kind,
+   internal::MatcherDescriptor *Callback);
 
 private:
   void registerMatcher(StringRef MatcherName,
std::unique_ptr Callback);
 
+  template 
+  void registerIfNodeMatcher(MatcherType,
+ internal::MatcherDescriptor *Descriptor,
+ StringRef) {}
+
+  template 
+  void registerIfNodeMatcher(
+  ast_matchers::internal::VariadicAllOfMatcher VarFunc,
+  internal::MatcherDescriptor *Descriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind<
+typename 
ast_matchers::internal::VariadicAllOfMatcher::Type>();
+if (!K.isNone())
+  NodeConstructors[K] = std::make_pair(MatcherName, Descriptor);
+  }
+
+  template 
+  void registerIfNodeMatcher(
+  ast_matchers::internal::VariadicDynCastAllOfMatcher
+  VarFunc,
+  internal::MatcherDescriptor *Descriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind();
+if (!K.isNone())
+  NodeConstructors[K] = std::make_pair(MatcherName, Descriptor);
+  }
+
   ConstructorMap Constructors;
+  NodeConstructorMap NodeConstructors;
 };
 
 } // namespace
@@ -67,8 +103,13 @@
 }
 
 #define REGISTER_MATCHER(name) 
\
-  registerMatcher(#name, internal::makeMatcherAutoMarshall(
\
- ::clang::ast_matchers::name, #name));
+  {
\
+auto Descriptor =  
\
+internal::makeMatcherAutoMarshall(::clang::ast_matchers::name, #name); 
\
+registerIfNodeMatcher(::clang::ast_matchers::name, Descriptor.get(),   
\
+  #name);  
\
+registerMatcher(#name, std::move(Descriptor)); 
\
+  }
 
 #define REGISTER_MATCHER_OVERLOAD(name)
\
   registerMatcher(#name,   
\
Index: include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- include/clang/ASTMatchers/ASTMatchersInternal.h
+++ include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -1577,6 +1577,7 @@
 : public VariadicFunction, Matcher,
   makeAllOfComposite> {
 public:
+  using Type = T;
   VariadicAllOfMatcher() {}
 };
 


Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -44,18 +44,54 @@
 
 using ConstructorMap = llvm::StringMap>;
 
+using NodeConstructorMap =
+std::map>;
+
 class RegistryMaps {
 public:
   RegistryMaps();
   ~RegistryMaps();
 
   const ConstructorMap () const { return Constructors; }
+  const NodeConstructorMap () const {
+return NodeConstructors;
+  }
+
+  void registerNodeMatcher(ast_type_traits::ASTNodeKind kind,
+   internal::MatcherDescriptor *Callback);
 
 private:
   void registerMatcher(StringRef MatcherName,
std::unique_ptr Callback);
 
+  template 
+  void registerIfNodeMatcher(MatcherType,
+ internal::MatcherDescriptor *Descriptor,
+ StringRef) {}
+
+  template 
+  void registerIfNodeMatcher(
+  ast_matchers::internal::VariadicAllOfMatcher VarFunc,
+  internal::MatcherDescriptor *Descriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind<
+typename ast_matchers::internal::VariadicAllOfMatcher::Type>();
+if (!K.isNone())
+  NodeConstructors[K] = std::make_pair(MatcherName, Descriptor);
+  }
+
+  template 
+  void registerIfNodeMatcher(
+  ast_matchers::internal::VariadicDynCastAllOfMatcher
+  VarFunc,
+  internal::MatcherDescriptor *Descriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind();
+if (!K.isNone())
+  

[PATCH] D54404: Exclude matchers which can have multiple results

[Stephen Kelly via Phabricator via cfe-commits]

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

Update


Repository:
  rC Clang

https://reviews.llvm.org/D54404

Files:
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/Dynamic/RegistryTest.cpp


Index: unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -579,6 +579,8 @@
   EXPECT_TRUE(!Contains(Matchers, "functionDecl()"));
 
   EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl()"));
+
+  EXPECT_TRUE(!Contains(Matchers, "has()"));
 }
 
 } // end anonymous namespace
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -601,13 +601,55 @@
 Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
   std::vector Result;
 
+  // Exclude matchers which can't unambiguously
+  // be used with particular arguments.
+  static std::vector ExcludedMatchers{
+  "allOf",
+  "anyOf",
+  "anything",
+  "containsDeclaration",
+  "eachOf",
+  "equalsNode",
+  "findAll",
+  "forEach",
+  "forEachConstructorInitializer",
+  "forEachDescendant",
+  "forEachOverridden",
+  "forEachSwitchCase",
+  "has",
+  "hasAncestor",
+  "hasAnyArgument",
+  "hasAnyConstructorInitializer",
+  "hasAnyDeclaration",
+  "hasAnyName",
+  "hasAnyParameter",
+  "hasAnySelector",
+  "hasAnySubstatement",
+  "hasAnyTemplateArgument",
+  "hasAnyUsingShadowDecl",
+  "hasArgumentOfType",
+  "hasDescendant",
+  "hasEitherOperand",
+  "hasMethod",
+  "hasParent",
+  "isExpansionInFileMatching",
+  "isSameOrDerivedFrom",
+  "matchesName",
+  "matchesSelector",
+  "unless"};
+  assert(std::is_sorted(ExcludedMatchers.begin(), ExcludedMatchers.end()));
+
   std::vector AcceptedTypes{StaticType};
 
   processAcceptableMatchers(
-  AcceptedTypes,
-  [](StringRef Name, const MatcherDescriptor &,
-std::set &, std::vector>,
-unsigned) { Result.emplace_back((Name + "()").str()); });
+  AcceptedTypes, [](StringRef Name, const MatcherDescriptor &,
+   std::set &,
+   std::vector>, unsigned) {
+if (!std::binary_search(ExcludedMatchers.begin(),
+ExcludedMatchers.end(), Name)) {
+  Result.emplace_back((Name + "()").str());
+}
+  });
 
   return Result;
 }


Index: unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -579,6 +579,8 @@
   EXPECT_TRUE(!Contains(Matchers, "functionDecl()"));
 
   EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl()"));
+
+  EXPECT_TRUE(!Contains(Matchers, "has()"));
 }
 
 } // end anonymous namespace
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -601,13 +601,55 @@
 Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
   std::vector Result;
 
+  // Exclude matchers which can't unambiguously
+  // be used with particular arguments.
+  static std::vector ExcludedMatchers{
+  "allOf",
+  "anyOf",
+  "anything",
+  "containsDeclaration",
+  "eachOf",
+  "equalsNode",
+  "findAll",
+  "forEach",
+  "forEachConstructorInitializer",
+  "forEachDescendant",
+  "forEachOverridden",
+  "forEachSwitchCase",
+  "has",
+  "hasAncestor",
+  "hasAnyArgument",
+  "hasAnyConstructorInitializer",
+  "hasAnyDeclaration",
+  "hasAnyName",
+  "hasAnyParameter",
+  "hasAnySelector",
+  "hasAnySubstatement",
+  "hasAnyTemplateArgument",
+  "hasAnyUsingShadowDecl",
+  "hasArgumentOfType",
+  "hasDescendant",
+  "hasEitherOperand",
+  "hasMethod",
+  "hasParent",
+  "isExpansionInFileMatching",
+  "isSameOrDerivedFrom",
+  "matchesName",
+  "matchesSelector",
+  "unless"};
+  assert(std::is_sorted(ExcludedMatchers.begin(), ExcludedMatchers.end()));
+
   std::vector AcceptedTypes{StaticType};
 
   processAcceptableMatchers(
-  AcceptedTypes,
-  [](StringRef Name, const MatcherDescriptor &,
-std::set &, std::vector>,
-unsigned) { Result.emplace_back((Name + "()").str()); });
+  AcceptedTypes, [](StringRef Name, const MatcherDescriptor &,
+   std::set &,
+   std::vector>, unsigned) {
+if (!std::binary_search(ExcludedMatchers.begin(),
+ 

[PATCH] D54403: Add new API for returning matching matchers

[Stephen Kelly via Phabricator via cfe-commits]

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

Update


Repository:
  rC Clang

https://reviews.llvm.org/D54403

Files:
  include/clang/ASTMatchers/Dynamic/Registry.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/Dynamic/RegistryTest.cpp


Index: unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -555,6 +555,32 @@
   EXPECT_FALSE(matches("int x = 120;", CharStmt));
 }
 
+TEST_F(RegistryTest, MatchingMatchers) {
+
+  auto Matchers = Registry::getMatchingMatchers(
+  ast_type_traits::ASTNodeKind::getFromNodeKind());
+
+  auto Contains = [](const std::vector , StringRef Name) {
+return llvm::find_if(C, [Name](const MatchingMatcher ) {
+ return M.MatcherString == Name;
+   }) != C.end();
+  };
+
+  EXPECT_TRUE(Contains(Matchers, "isPublic()"));
+  EXPECT_TRUE(Contains(Matchers, "hasName()"));
+  EXPECT_TRUE(Contains(Matchers, "hasType()"));
+  EXPECT_TRUE(Contains(Matchers, "hasTypeLoc()"));
+  EXPECT_TRUE(Contains(Matchers, "parameterCountIs()"));
+
+  EXPECT_TRUE(!Contains(Matchers, "decl()"));
+  EXPECT_TRUE(!Contains(Matchers, "namedDecl()"));
+  EXPECT_TRUE(!Contains(Matchers, "valueDecl()"));
+  EXPECT_TRUE(!Contains(Matchers, "declaratorDecl()"));
+  EXPECT_TRUE(!Contains(Matchers, "functionDecl()"));
+
+  EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl()"));
+}
+
 } // end anonymous namespace
 } // end namespace dynamic
 } // end namespace ast_matchers
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -597,6 +597,21 @@
   }
 }
 
+std::vector
+Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
+  std::vector Result;
+
+  std::vector AcceptedTypes{StaticType};
+
+  processAcceptableMatchers(
+  AcceptedTypes,
+  [](StringRef Name, const MatcherDescriptor &,
+std::set &, std::vector>,
+unsigned) { Result.emplace_back((Name + "()").str()); });
+
+  return Result;
+}
+
 std::vector
 Registry::getMatcherCompletions(ArrayRef AcceptedTypes) {
   std::vector Completions;
Index: include/clang/ASTMatchers/Dynamic/Registry.h
===
--- include/clang/ASTMatchers/Dynamic/Registry.h
+++ include/clang/ASTMatchers/Dynamic/Registry.h
@@ -63,6 +63,12 @@
   unsigned Specificity;
 };
 
+struct MatchingMatcher {
+  MatchingMatcher(std::string MatcherString)
+  : MatcherString(std::move(MatcherString)) {}
+  std::string MatcherString;
+};
+
 class Registry {
 public:
   Registry() = delete;
@@ -96,6 +102,11 @@
   static std::vector
   getMatcherCompletions(ArrayRef AcceptedTypes);
 
+  /// Compute matchers which can be used within a matcher of
+  /// type @p StaticType.
+  static std::vector
+  getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType);
+
   /// Construct a matcher from the registry.
   ///
   /// \param Ctor The matcher constructor to instantiate.


Index: unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -555,6 +555,32 @@
   EXPECT_FALSE(matches("int x = 120;", CharStmt));
 }
 
+TEST_F(RegistryTest, MatchingMatchers) {
+
+  auto Matchers = Registry::getMatchingMatchers(
+  ast_type_traits::ASTNodeKind::getFromNodeKind());
+
+  auto Contains = [](const std::vector , StringRef Name) {
+return llvm::find_if(C, [Name](const MatchingMatcher ) {
+ return M.MatcherString == Name;
+   }) != C.end();
+  };
+
+  EXPECT_TRUE(Contains(Matchers, "isPublic()"));
+  EXPECT_TRUE(Contains(Matchers, "hasName()"));
+  EXPECT_TRUE(Contains(Matchers, "hasType()"));
+  EXPECT_TRUE(Contains(Matchers, "hasTypeLoc()"));
+  EXPECT_TRUE(Contains(Matchers, "parameterCountIs()"));
+
+  EXPECT_TRUE(!Contains(Matchers, "decl()"));
+  EXPECT_TRUE(!Contains(Matchers, "namedDecl()"));
+  EXPECT_TRUE(!Contains(Matchers, "valueDecl()"));
+  EXPECT_TRUE(!Contains(Matchers, "declaratorDecl()"));
+  EXPECT_TRUE(!Contains(Matchers, "functionDecl()"));
+
+  EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl()"));
+}
+
 } // end anonymous namespace
 } // end namespace dynamic
 } // end namespace ast_matchers
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -597,6 +597,21 @@
   }
 }
 
+std::vector
+Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
+  std::vector Result;
+
+  std::vector AcceptedTypes{StaticType};
+
+  processAcceptableMatchers(
+  AcceptedTypes,
+  [](StringRef 

[PATCH] D53982: Output "rule" information in SARIF

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

Szelethus added inline comments.



Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:237-242
+#define GET_CHECKERS
+#define CHECKER(FULLNAME, CLASS, CXXFILE, HELPTEXT, GROUPINDEX, HIDDEN)
\
+  .Case(FULLNAME, HELPTEXT)
+#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
+#undef CHECKER
+#undef GET_CHECKERS

aaron.ballman wrote:
> Szelethus wrote:
> > aaron.ballman wrote:
> > > Szelethus wrote:
> > > > Szelethus wrote:
> > > > > Hmmm, this won't handle checkers loaded from plugins.
> > > > I don't immediately know the solution is to this, but when you invoke 
> > > > clang with `-cc1 -analyzer-checker-help`, plugin checkers are also 
> > > > displayed, and [[ 
> > > > https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp#L150
> > > >  | this is line that magically does it ]].
> > > > Maybe store the plugins in `AnalyzerOptions`, and move 
> > > > `ClangCheckerRegistry` to `include/clang/StaticAnalyzer/Frontend`?
> > > Oof, this is a good point. Thank you for bringing it up! I'll look into 
> > > this as soon as I have the chance.
> > > 
> > > Do we have any tests that use plugins (preferably on Windows) so that I 
> > > have a way to test this functionality out?
> > I'm working on the exact same issue, which is why I spotted this! There are 
> > some in `unittests/`, but I plan on enhancing them //a lot// in the coming 
> > days. I'll make sure to add you as subscriber on related paches, until 
> > then, unless users of the Saris output are likely to use the StaticAnalyzer 
> > with plugins, I think feel free to wait on my progress. I expect to come 
> > out with a patch during the week. But you never know, I guess.
> > 
> > https://github.com/llvm-mirror/clang/blob/master/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
> I'm fine with waiting. I don't expect to see heavy use of the Sarif exporter 
> until the spec is finalized, as it's hard to write a Sarif viewer against a 
> moving target. So we've got a few months to find these sort of things out.
> 
> Thank you for CCing me on the related patches!
Well, yea, skeletons are falling out of the closet left and right, so I'll need 
to clean a lot of other things up before finding a neat solution to this. 
Please let me know when you need to make progress on this.


https://reviews.llvm.org/D53982



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

[PATCH] D54402: Extract method to allow re-use

[Stephen Kelly via Phabricator via cfe-commits]

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

Update


Repository:
  rC Clang

https://reviews.llvm.org/D54402

Files:
  lib/ASTMatchers/Dynamic/Registry.cpp

Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -561,9 +561,8 @@
   return std::vector(TypeSet.begin(), TypeSet.end());
 }
 
-std::vector
-Registry::getMatcherCompletions(ArrayRef AcceptedTypes) {
-  std::vector Completions;
+template 
+void processAcceptableMatchers(ArrayRef AcceptedTypes, Callable &) {
 
   // Search the registry for acceptable matchers.
   for (const auto  : RegistryData->constructors()) {
@@ -593,49 +592,65 @@
 }
 
 if (!RetKinds.empty() && MaxSpecificity > 0) {
-  std::string Decl;
-  llvm::raw_string_ostream OS(Decl);
-
-  if (IsPolymorphic) {
-OS << "Matcher " << Name << "(Matcher";
-  } else {
-OS << "Matcher<" << RetKinds << "> " << Name << "(";
-for (const std::vector  : ArgsKinds) {
-  if ( != [0])
-OS << ", ";
-
-  bool FirstArgKind = true;
-  std::set MatcherKinds;
-  // Two steps. First all non-matchers, then matchers only.
-  for (const ArgKind  : Arg) {
-if (AK.getArgKind() == ArgKind::AK_Matcher) {
-  MatcherKinds.insert(AK.getMatcherKind());
-} else {
-  if (!FirstArgKind) OS << "|";
-  FirstArgKind = false;
-  OS << AK.asString();
+  std::forward(Func)(Name, Matcher, RetKinds, ArgsKinds, MaxSpecificity);
+}
+  }
+}
+
+std::vector
+Registry::getMatcherCompletions(ArrayRef AcceptedTypes) {
+  std::vector Completions;
+
+  processAcceptableMatchers(
+  AcceptedTypes,
+  [](StringRef Name, const MatcherDescriptor ,
+ std::set ,
+ std::vector> ArgsKinds,
+ unsigned MaxSpecificity) {
+std::string Decl;
+llvm::raw_string_ostream OS(Decl);
+
+if (Matcher.isPolymorphic()) {
+  OS << "Matcher " << Name << "(Matcher";
+} else {
+  OS << "Matcher<" << RetKinds << "> " << Name << "(";
+  for (const std::vector  : ArgsKinds) {
+if ( != [0])
+  OS << ", ";
+
+bool FirstArgKind = true;
+std::set MatcherKinds;
+// Two steps. First all non-matchers, then matchers only.
+for (const ArgKind  : Arg) {
+  if (AK.getArgKind() == ArgKind::AK_Matcher) {
+MatcherKinds.insert(AK.getMatcherKind());
+  } else {
+if (!FirstArgKind)
+  OS << "|";
+FirstArgKind = false;
+OS << AK.asString();
+  }
+}
+if (!MatcherKinds.empty()) {
+  if (!FirstArgKind)
+OS << "|";
+  OS << "Matcher<" << MatcherKinds << ">";
 }
-  }
-  if (!MatcherKinds.empty()) {
-if (!FirstArgKind) OS << "|";
-OS << "Matcher<" << MatcherKinds << ">";
   }
 }
-  }
-  if (Matcher.isVariadic())
-OS << "...";
-  OS << ")";
-
-  std::string TypedText = Name;
-  TypedText += "(";
-  if (ArgsKinds.empty())
-TypedText += ")";
-  else if (ArgsKinds[0][0].getArgKind() == ArgKind::AK_String)
-TypedText += "\"";
-
-  Completions.emplace_back(TypedText, OS.str(), MaxSpecificity);
-}
-  }
+if (Matcher.isVariadic())
+  OS << "...";
+OS << ")";
+
+std::string TypedText = Name;
+TypedText += "(";
+if (ArgsKinds.empty())
+  TypedText += ")";
+else if (ArgsKinds[0][0].getArgKind() == ArgKind::AK_String)
+  TypedText += "\"";
+
+Completions.emplace_back(TypedText, OS.str(), MaxSpecificity);
+  });
 
   return Completions;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

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

Szelethus added a comment.

In https://reviews.llvm.org/D54438#1296155, @NoQ wrote:

> Can we reduce this patch to simply introducing the dependency item in 
> `Checkers.td` and using it in, like, one place (eg., 
> `MallocChecker`/`CStringChecker` inter-op) and discuss the rest of the stuff 
> later before we jump into it?


Short answer, no. The original intent was to somehow make sense out of checker 
options, and look where that got me :D There are no more shortcuts to make, 
sadly. I strongly believe it'd be better to figure out how those checkers 
should operate before moving forward with this patch, because I know I wouldn't 
like to touch any of those checkers once this patch goes through, so let's 
tough it out first.

Also, doing some checker related work might be refreshing after the never 
ending refactoring.

I'll brush the dust off some notebooks and see what I can come up with.


Repository:
  rC Clang

https://reviews.llvm.org/D54438



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

Re: r346266 - Don't use std::next() on an input iterator; NFC.

[Aaron Ballman via cfe-commits]

On Mon, Nov 12, 2018 at 5:24 PM David Blaikie  wrote:
>
> Thanks!

Thank you for the review feedback, that was a good catch. I've changed
to an assert in r346714.

~Aaron

>
> On Mon, Nov 12, 2018 at 2:14 PM Aaron Ballman  wrote:
>>
>> On Mon, Nov 12, 2018 at 1:30 PM David Blaikie  wrote:
>> >
>> > The previous code didn't have a conditional for Iter != End - was that a 
>> > bug? Should there be a test case for that bug?
>> >
>> > If that's not an actual change in behavior, could it be an assert instead 
>> > of a condition?
>>
>> I think an assertion would make the most sense. I can't devise a way
>> where there wouldn't be at least a root path and a file name, so
>> skipping over the root component should always be okay. I'll fix that
>> up.
>>
>> ~Aaron
>>
>> >
>> > On Tue, Nov 6, 2018 at 1:14 PM Aaron Ballman via cfe-commits 
>> >  wrote:
>> >>
>> >> Author: aaronballman
>> >> Date: Tue Nov  6 13:12:44 2018
>> >> New Revision: 346266
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=346266=rev
>> >> Log:
>> >> Don't use std::next() on an input iterator; NFC.
>> >>
>> >> Instead, advance the old-fashioned way, as std::next() cannot be used on 
>> >> an input iterator until C++17.
>> >>
>> >> Modified:
>> >> cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
>> >>
>> >> Modified: cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
>> >> URL: 
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp?rev=346266=346265=346266=diff
>> >> ==
>> >> --- cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp (original)
>> >> +++ cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp Tue Nov  6 
>> >> 13:12:44 2018
>> >> @@ -82,25 +82,27 @@ static std::string fileNameToURI(StringR
>> >>  Ret += Twine("/" + Root).str();
>> >>}
>> >>
>> >> -  // Add the rest of the path components, encoding any reserved 
>> >> characters.
>> >> -  std::for_each(std::next(sys::path::begin(Filename)), 
>> >> sys::path::end(Filename),
>> >> -[](StringRef Component) {
>> >> -  // For reasons unknown to me, we may get a backslash 
>> >> with
>> >> -  // Windows native paths for the initial backslash 
>> >> following
>> >> -  // the drive component, which we need to ignore as a 
>> >> URI path
>> >> -  // part.
>> >> -  if (Component == "\\")
>> >> -return;
>> >> +  auto Iter = sys::path::begin(Filename), End = sys::path::end(Filename);
>> >> +  if (Iter != End) {
>> >> +// Add the rest of the path components, encoding any reserved 
>> >> characters;
>> >> +// we skip past the first path component, as it was handled it above.
>> >> +std::for_each(++Iter, End, [](StringRef Component) {
>> >> +  // For reasons unknown to me, we may get a backslash with Windows 
>> >> native
>> >> +  // paths for the initial backslash following the drive component, 
>> >> which
>> >> +  // we need to ignore as a URI path part.
>> >> +  if (Component == "\\")
>> >> +return;
>> >>
>> >> -  // Add the separator between the previous path part 
>> >> and the
>> >> -  // one being currently processed.
>> >> -  Ret += "/";
>> >> +  // Add the separator between the previous path part and the one 
>> >> being
>> >> +  // currently processed.
>> >> +  Ret += "/";
>> >>
>> >> -  // URI encode the part.
>> >> -  for (char C : Component) {
>> >> -Ret += percentEncodeURICharacter(C);
>> >> -  }
>> >> -});
>> >> +  // URI encode the part.
>> >> +  for (char C : Component) {
>> >> +Ret += percentEncodeURICharacter(C);
>> >> +  }
>> >> +});
>> >> +  }
>> >>
>> >>return Ret.str().str();
>> >>  }
>> >>
>> >>
>> >> ___
>> >> cfe-commits mailing list
>> >> cfe-commits@lists.llvm.org
>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r346714 - Convert a condition into an assertion per post-review feedback; NFC intended.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Mon Nov 12 14:32:38 2018
New Revision: 346714

URL: http://llvm.org/viewvc/llvm-project?rev=346714=rev
Log:
Convert a condition into an assertion per post-review feedback; NFC intended.

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp?rev=346714=346713=346714=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp Mon Nov 12 14:32:38 
2018
@@ -83,26 +83,25 @@ static std::string fileNameToURI(StringR
   }
 
   auto Iter = sys::path::begin(Filename), End = sys::path::end(Filename);
-  if (Iter != End) {
-// Add the rest of the path components, encoding any reserved characters;
-// we skip past the first path component, as it was handled it above.
-std::for_each(++Iter, End, [](StringRef Component) {
-  // For reasons unknown to me, we may get a backslash with Windows native
-  // paths for the initial backslash following the drive component, which
-  // we need to ignore as a URI path part.
-  if (Component == "\\")
-return;
+  assert(Iter != End && "Expected there to be a non-root path component.");
+  // Add the rest of the path components, encoding any reserved characters;
+  // we skip past the first path component, as it was handled it above.
+  std::for_each(++Iter, End, [](StringRef Component) {
+// For reasons unknown to me, we may get a backslash with Windows native
+// paths for the initial backslash following the drive component, which
+// we need to ignore as a URI path part.
+if (Component == "\\")
+  return;
 
-  // Add the separator between the previous path part and the one being
-  // currently processed.
-  Ret += "/";
+// Add the separator between the previous path part and the one being
+// currently processed.
+Ret += "/";
 
-  // URI encode the part.
-  for (char C : Component) {
-Ret += percentEncodeURICharacter(C);
-  }
-});
-  }
+// URI encode the part.
+for (char C : Component) {
+  Ret += percentEncodeURICharacter(C);
+}
+  });
 
   return Ret.str().str();
 }


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

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, dblaikie, echristo.
Herald added subscribers: jsji, kbarton, nemanjai.

While working on https://reviews.llvm.org/D54349, it was noted that the 
`SourceRange` returned from the preprocessor callbacks was bogus. It was 
expected to pass the source range for the condition to a `#if` or `#elif` 
directive, but what was actually passed was a much larger range that could even 
include all of the conditionally skipped tokens.

This patch adjusts the source range passed in to the callbacks to only include 
the condition range itself by tracking that information a bit better in the 
preprocessor.


https://reviews.llvm.org/D54450

Files:
  include/clang/Lex/Preprocessor.h
  lib/Lex/PPDirectives.cpp
  lib/Lex/PPExpressions.cpp
  unittests/Lex/PPCallbacksTest.cpp

Index: unittests/Lex/PPCallbacksTest.cpp
===
--- unittests/Lex/PPCallbacksTest.cpp
+++ unittests/Lex/PPCallbacksTest.cpp
@@ -65,6 +65,29 @@
   SrcMgr::CharacteristicKind FileType;
 };
 
+class CondDirectiveCallbacks : public PPCallbacks {
+public:
+  struct Result {
+SourceRange ConditionRange;
+ConditionValueKind ConditionValue;
+
+Result(SourceRange R, ConditionValueKind K)
+: ConditionRange(R), ConditionValue(K) {}
+  };
+
+  std::vector Results;
+
+  void If(SourceLocation Loc, SourceRange ConditionRange,
+  ConditionValueKind ConditionValue) override {
+Results.emplace_back(ConditionRange, ConditionValue);
+  }
+
+  void Elif(SourceLocation Loc, SourceRange ConditionRange,
+ConditionValueKind ConditionValue, SourceLocation IfLoc) override {
+Results.emplace_back(ConditionRange, ConditionValue);
+  }
+};
+
 // Stub to collect data from PragmaOpenCLExtension callbacks.
 class PragmaOpenCLExtensionCallbacks : public PPCallbacks {
 public:
@@ -137,6 +160,15 @@
 return StringRef(B, E - B);
   }
 
+  StringRef GetSourceStringToEnd(CharSourceRange Range) {
+const char *B = SourceMgr.getCharacterData(Range.getBegin());
+const char *E = SourceMgr.getCharacterData(Range.getEnd());
+
+return StringRef(
+B,
+E - B + Lexer::MeasureTokenLength(Range.getEnd(), SourceMgr, LangOpts));
+  }
+
   // Run lexer over SourceText and collect FilenameRange from
   // the InclusionDirective callback.
   CharSourceRange InclusionDirectiveFilenameRange(const char *SourceText,
@@ -199,6 +231,36 @@
 return Callbacks;
   }
 
+  std::vector
+  DirectiveExprRange(StringRef SourceText) {
+TrivialModuleLoader ModLoader;
+MemoryBufferCache PCMCache;
+std::unique_ptr Buf =
+llvm::MemoryBuffer::getMemBuffer(SourceText);
+SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
+HeaderSearch HeaderInfo(std::make_shared(), SourceMgr,
+Diags, LangOpts, Target.get());
+Preprocessor PP(std::make_shared(), Diags, LangOpts,
+SourceMgr, PCMCache, HeaderInfo, ModLoader,
+/*IILookup =*/nullptr,
+/*OwnsHeaderSearch =*/false);
+PP.Initialize(*Target);
+auto *Callbacks = new CondDirectiveCallbacks;
+PP.addPPCallbacks(std::unique_ptr(Callbacks));
+
+// Lex source text.
+PP.EnterMainSourceFile();
+
+while (true) {
+  Token Tok;
+  PP.Lex(Tok);
+  if (Tok.is(tok::eof))
+break;
+}
+
+return Callbacks->Results;
+  }
+
   PragmaOpenCLExtensionCallbacks::CallbackParameters
   PragmaOpenCLExtensionCall(const char *SourceText) {
 LangOptions OpenCLLangOpts;
@@ -368,4 +430,42 @@
   ASSERT_EQ(ExpectedState, Parameters.State);
 }
 
-} // anonoymous namespace
+TEST_F(PPCallbacksTest, DirectiveExprRanges) {
+  const auto  = DirectiveExprRange("#if FLUZZY_FLOOF\n#endif\n");
+  EXPECT_EQ(Results1.size(), 1);
+  EXPECT_EQ(
+  GetSourceStringToEnd(CharSourceRange(Results1[0].ConditionRange, false)),
+  "FLUZZY_FLOOF");
+
+  const auto  = DirectiveExprRange("#if 1 + 4 < 7\n#endif\n");
+  EXPECT_EQ(Results2.size(), 1);
+  EXPECT_EQ(
+  GetSourceStringToEnd(CharSourceRange(Results2[0].ConditionRange, false)),
+  "1 + 4 < 7");
+
+  const auto  = DirectiveExprRange("#if 1 + \\\n  2\n#endif\n");
+  EXPECT_EQ(Results3.size(), 1);
+  EXPECT_EQ(
+  GetSourceStringToEnd(CharSourceRange(Results3[0].ConditionRange, false)),
+  "1 + \\\n  2");
+
+  const auto  = DirectiveExprRange("#if 0\n#elif FLOOFY\n#endif\n");
+  EXPECT_EQ(Results4.size(), 2);
+  EXPECT_EQ(
+  GetSourceStringToEnd(CharSourceRange(Results4[0].ConditionRange, false)),
+  "0");
+  EXPECT_EQ(
+  GetSourceStringToEnd(CharSourceRange(Results4[1].ConditionRange, false)),
+  "FLOOFY");
+
+  const auto  = DirectiveExprRange("#if 1\n#elif FLOOFY\n#endif\n");
+  EXPECT_EQ(Results5.size(), 2);
+  EXPECT_EQ(
+  GetSourceStringToEnd(CharSourceRange(Results5[0].ConditionRange, false)),
+  "1");
+  EXPECT_EQ(
+  

[PATCH] D52296: [Clang] - Add '-gsplit-dwarf[=split, =single]' version for '-gsplit-dwarf' option.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:813-830
+  if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
+if (StringRef(A->getValue()) == "single")
+  return Args.MakeArgString(Output.getFilename());
+
   Arg *FinalOutput = Args.getLastArg(options::OPT_o);
   if (FinalOutput && Args.hasArg(options::OPT_c)) {
 SmallString<128> T(FinalOutput->getValue());

If this function now takes the output file name - could it be simplified to 
just always use that, rather than the conditional code that follows and tests 
whether -o is specified and builds up something like the output file name & 
uses the dwo suffix?



Comment at: test/CodeGen/split-debug-single-file.c:12
+//  RUN:   -enable-split-dwarf=split -split-dwarf-file %t.o -emit-obj -o %t.o 
%s
+//  RUN: llvm-objdump -section-headers %t.o | FileCheck 
--check-prefix=MODE-SPLIT %s
+//  MODE-SPLIT-NOT: .dwo

This looks like an end-to-end test, which we don't usually do in Clang (or in 
the LLVM project in general).

For example, the previous testing for split-dwarf had a driver test (that 
tested only that the driver produced the right cc1 invocation) and a CodeGen 
test (that tested that the right IR was produced), but I don't see any test 
like this (that tested the resulting object file)?

I know there's a narrow gap in IR testing - things that don't go in the IR, but 
instead go through MCOptions  & that the SplitDwarfFile is one of those.

So, yeah, in this case it's a bit of a test hole - if you're going to keep this 
test, perhaps it could test assembly, rather than the object file? Since it 
doesn't need complex parsing, etc, perhaps?


https://reviews.llvm.org/D52296



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

Re: r346266 - Don't use std::next() on an input iterator; NFC.

[David Blaikie via cfe-commits]

Thanks!

On Mon, Nov 12, 2018 at 2:14 PM Aaron Ballman 
wrote:

> On Mon, Nov 12, 2018 at 1:30 PM David Blaikie  wrote:
> >
> > The previous code didn't have a conditional for Iter != End - was that a
> bug? Should there be a test case for that bug?
> >
> > If that's not an actual change in behavior, could it be an assert
> instead of a condition?
>
> I think an assertion would make the most sense. I can't devise a way
> where there wouldn't be at least a root path and a file name, so
> skipping over the root component should always be okay. I'll fix that
> up.
>
> ~Aaron
>
> >
> > On Tue, Nov 6, 2018 at 1:14 PM Aaron Ballman via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >>
> >> Author: aaronballman
> >> Date: Tue Nov  6 13:12:44 2018
> >> New Revision: 346266
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=346266=rev
> >> Log:
> >> Don't use std::next() on an input iterator; NFC.
> >>
> >> Instead, advance the old-fashioned way, as std::next() cannot be used
> on an input iterator until C++17.
> >>
> >> Modified:
> >> cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
> >>
> >> Modified: cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
> >> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp?rev=346266=346265=346266=diff
> >>
> ==
> >> --- cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp (original)
> >> +++ cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp Tue Nov  6
> 13:12:44 2018
> >> @@ -82,25 +82,27 @@ static std::string fileNameToURI(StringR
> >>  Ret += Twine("/" + Root).str();
> >>}
> >>
> >> -  // Add the rest of the path components, encoding any reserved
> characters.
> >> -  std::for_each(std::next(sys::path::begin(Filename)),
> sys::path::end(Filename),
> >> -[](StringRef Component) {
> >> -  // For reasons unknown to me, we may get a backslash
> with
> >> -  // Windows native paths for the initial backslash
> following
> >> -  // the drive component, which we need to ignore as a
> URI path
> >> -  // part.
> >> -  if (Component == "\\")
> >> -return;
> >> +  auto Iter = sys::path::begin(Filename), End =
> sys::path::end(Filename);
> >> +  if (Iter != End) {
> >> +// Add the rest of the path components, encoding any reserved
> characters;
> >> +// we skip past the first path component, as it was handled it
> above.
> >> +std::for_each(++Iter, End, [](StringRef Component) {
> >> +  // For reasons unknown to me, we may get a backslash with
> Windows native
> >> +  // paths for the initial backslash following the drive
> component, which
> >> +  // we need to ignore as a URI path part.
> >> +  if (Component == "\\")
> >> +return;
> >>
> >> -  // Add the separator between the previous path part
> and the
> >> -  // one being currently processed.
> >> -  Ret += "/";
> >> +  // Add the separator between the previous path part and the one
> being
> >> +  // currently processed.
> >> +  Ret += "/";
> >>
> >> -  // URI encode the part.
> >> -  for (char C : Component) {
> >> -Ret += percentEncodeURICharacter(C);
> >> -  }
> >> -});
> >> +  // URI encode the part.
> >> +  for (char C : Component) {
> >> +Ret += percentEncodeURICharacter(C);
> >> +  }
> >> +});
> >> +  }
> >>
> >>return Ret.str().str();
> >>  }
> >>
> >>
> >> ___
> >> cfe-commits mailing list
> >> cfe-commits@lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Hmm.

All intentions in this patch are great and i love them!

I think that the refactoring work in `MallocChecker` is a bit pre-mature; i 
don't have a clear picture of how the ideal `MallocChecker` should look like, 
and i'm not sure this patch moves us into the right direction. Like, it tries 
to deal with multiple API packs (malloc-free, new-delete, new[]-delete[], 
g_malloc()-g_free(), alloca()-???, obtaininnerpointer-invalidateinnerpointer, 
`CStringChecker` interop) and multiple warning kinds (use-after-free, double 
free, leak, free at offset, mismatched deallocators) and i've no idea what's 
the proper way to split this into smaller checkers, how many `Checker` 
sub-classes do we need, how should they be split up into multiple files and 
inter-operate via headers (maybe it's better for sub-checkers include 
`MallocChecker.h` than for `MallocChecker` to include sub-checkers' `.h`s?), 
what exactly should the "base" checker do, how many `Checkers.td` entries do we 
need, how should the `Checkers.td` dependency graph look like, AA. 
I want to see a picture of all this stuff drawn first, before jumping into 
solving this.

Can we reduce this patch to simply introducing the dependency item in 
`Checkers.td` and using it in, like, one place (eg., 
`MallocChecker`/`CStringChecker` inter-op) and discuss the rest of the stuff 
later before we jump into it?


Repository:
  rC Clang

https://reviews.llvm.org/D54438



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

Re: r346266 - Don't use std::next() on an input iterator; NFC.

[Aaron Ballman via cfe-commits]

On Mon, Nov 12, 2018 at 1:30 PM David Blaikie  wrote:
>
> The previous code didn't have a conditional for Iter != End - was that a bug? 
> Should there be a test case for that bug?
>
> If that's not an actual change in behavior, could it be an assert instead of 
> a condition?

I think an assertion would make the most sense. I can't devise a way
where there wouldn't be at least a root path and a file name, so
skipping over the root component should always be okay. I'll fix that
up.

~Aaron

>
> On Tue, Nov 6, 2018 at 1:14 PM Aaron Ballman via cfe-commits 
>  wrote:
>>
>> Author: aaronballman
>> Date: Tue Nov  6 13:12:44 2018
>> New Revision: 346266
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=346266=rev
>> Log:
>> Don't use std::next() on an input iterator; NFC.
>>
>> Instead, advance the old-fashioned way, as std::next() cannot be used on an 
>> input iterator until C++17.
>>
>> Modified:
>> cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
>>
>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp?rev=346266=346265=346266=diff
>> ==
>> --- cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp Tue Nov  6 
>> 13:12:44 2018
>> @@ -82,25 +82,27 @@ static std::string fileNameToURI(StringR
>>  Ret += Twine("/" + Root).str();
>>}
>>
>> -  // Add the rest of the path components, encoding any reserved characters.
>> -  std::for_each(std::next(sys::path::begin(Filename)), 
>> sys::path::end(Filename),
>> -[](StringRef Component) {
>> -  // For reasons unknown to me, we may get a backslash with
>> -  // Windows native paths for the initial backslash 
>> following
>> -  // the drive component, which we need to ignore as a URI 
>> path
>> -  // part.
>> -  if (Component == "\\")
>> -return;
>> +  auto Iter = sys::path::begin(Filename), End = sys::path::end(Filename);
>> +  if (Iter != End) {
>> +// Add the rest of the path components, encoding any reserved 
>> characters;
>> +// we skip past the first path component, as it was handled it above.
>> +std::for_each(++Iter, End, [](StringRef Component) {
>> +  // For reasons unknown to me, we may get a backslash with Windows 
>> native
>> +  // paths for the initial backslash following the drive component, 
>> which
>> +  // we need to ignore as a URI path part.
>> +  if (Component == "\\")
>> +return;
>>
>> -  // Add the separator between the previous path part and 
>> the
>> -  // one being currently processed.
>> -  Ret += "/";
>> +  // Add the separator between the previous path part and the one being
>> +  // currently processed.
>> +  Ret += "/";
>>
>> -  // URI encode the part.
>> -  for (char C : Component) {
>> -Ret += percentEncodeURICharacter(C);
>> -  }
>> -});
>> +  // URI encode the part.
>> +  for (char C : Component) {
>> +Ret += percentEncodeURICharacter(C);
>> +  }
>> +});
>> +  }
>>
>>return Ret.str().str();
>>  }
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D54310#1296080, @arphaman wrote:

> Apologies for not seeing this earlier.


No worries, thanks for the input!

>> The logic to do this was based on resource dir as an approximation of
>>  where the compiler is installed. This broke the tools that read
>>  'compile_commands.json' and don't ship with the compiler, as they
>>  typically change resource dir.
> 
> In that case I think the tool should adjust the `-resource-dir` to point to 
> the appropriate `-resource-dir` for the compiler in the toolchain. However, 
> that comes with its own problems, as the builtin headers might be 
> incompatible between the tool and the compiler in the toolchain.

Exactly the problem we're facing. Since the -resource-dir is primarily used to 
find the builtin headers, we **have** to override it in the tools to avoid 
breaking them.

> In that case I think it would be preferable for the tool to ship with its own 
> libc++ headers in addition to the builtin headers rather than implementing 
> this behavior.

There's value in providing the users with warnings/errors from exactly the same 
version of the C++ standard library that's used by the compiler, so I would 
avoid going down this path if possible.

> If that's unacceptable I think we should adjust libTooling to add a search 
> path for the libc++ headers when the tool detects that it's running outside 
> of the toolchain on Darwin (i.e. 'clang' doesn't exist in the same directory 
> as the tool, and '../include/c++' doesn't exist as well)  instead of changing 
> this behavior.

This was the original intention of the patch, but I don't think we should limit 
this to libTooling.
I'll have a look at what exactly lldb broke and will come up with a fix to this 
behavior.


Repository:
  rL LLVM

https://reviews.llvm.org/D54310



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

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Ping.


Repository:
  rC Clang

https://reviews.llvm.org/D53949



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

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Apologies for not seeing this earlier.
I agree with George, this behavior doesn't seem right to me.

> The logic to do this was based on resource dir as an approximation of
>  where the compiler is installed. This broke the tools that read
>  'compile_commands.json' and don't ship with the compiler, as they
>  typically change resource dir.

In that case I think the tool should adjust the `-resource-dir` to point to the 
appropriate `-resource-dir` for the compiler in the toolchain. However, that 
comes with its own problems, as the builtin headers might be incompatible 
between the tool and the compiler in the toolchain. In that case I think it 
would be preferable for the tool to ship with its own libc++ headers in 
addition to the builtin headers rather than implementing this behavior. If 
that's unacceptable I think we should adjust libTooling to add a search path 
for the libc++ headers when the tool detects that it's running outside of the 
toolchain on Darwin (i.e. 'clang' doesn't exist in the same directory as the 
tool, and '../include/c++' doesn't exist as well)  instead of changing this 
behavior.


Repository:
  rL LLVM

https://reviews.llvm.org/D54310



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

r346709 - [Sema] Make sure we substitute an instantiation-dependent default template argument

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Mon Nov 12 13:31:06 2018
New Revision: 346709

URL: http://llvm.org/viewvc/llvm-project?rev=346709=rev
Log:
[Sema] Make sure we substitute an instantiation-dependent default template 
argument

Fixes llvm.org/PR39623

Differential revision: https://reviews.llvm.org/D54414

Modified:
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/SemaCXX/alias-template.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=346709=346708=346709=diff
==
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Mon Nov 12 13:31:06 2018
@@ -4434,7 +4434,7 @@ SubstDefaultTemplateArgument(Sema 
 
   // If the argument type is dependent, instantiate it now based
   // on the previously-computed template arguments.
-  if (ArgType->getType()->isDependentType()) {
+  if (ArgType->getType()->isInstantiationDependentType()) {
 Sema::InstantiatingTemplate Inst(SemaRef, TemplateLoc,
  Param, Template, Converted,
  SourceRange(TemplateLoc, RAngleLoc));

Modified: cfe/trunk/test/SemaCXX/alias-template.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/alias-template.cpp?rev=346709=346708=346709=diff
==
--- cfe/trunk/test/SemaCXX/alias-template.cpp (original)
+++ cfe/trunk/test/SemaCXX/alias-template.cpp Mon Nov 12 13:31:06 2018
@@ -179,3 +179,13 @@ struct S {
 };
 static_assert(__is_same(S<3>::U, X[2]), ""); // expected-error {{static_assert 
failed}}
 }
+
+namespace PR39623 {
+template 
+using void_t = void;
+
+template >
+int sfinae_me() { return 0; } // expected-note{{candidate template ignored: 
substitution failure}}
+
+int g = sfinae_me(); // expected-error{{no matching function for call to 
'sfinae_me'}}
+}


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

[PATCH] D54414: [Sema] Make sure we substitute an instantiation-dependent default template parameter

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346709: [Sema] Make sure we substitute an 
instantiation-dependent default template… (authored by epilk, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54414?vs=173615=173752#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D54414

Files:
  cfe/trunk/lib/Sema/SemaTemplate.cpp
  cfe/trunk/test/SemaCXX/alias-template.cpp


Index: cfe/trunk/test/SemaCXX/alias-template.cpp
===
--- cfe/trunk/test/SemaCXX/alias-template.cpp
+++ cfe/trunk/test/SemaCXX/alias-template.cpp
@@ -179,3 +179,13 @@
 };
 static_assert(__is_same(S<3>::U, X[2]), ""); // expected-error {{static_assert 
failed}}
 }
+
+namespace PR39623 {
+template 
+using void_t = void;
+
+template >
+int sfinae_me() { return 0; } // expected-note{{candidate template ignored: 
substitution failure}}
+
+int g = sfinae_me(); // expected-error{{no matching function for call to 
'sfinae_me'}}
+}
Index: cfe/trunk/lib/Sema/SemaTemplate.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplate.cpp
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp
@@ -4434,7 +4434,7 @@
 
   // If the argument type is dependent, instantiate it now based
   // on the previously-computed template arguments.
-  if (ArgType->getType()->isDependentType()) {
+  if (ArgType->getType()->isInstantiationDependentType()) {
 Sema::InstantiatingTemplate Inst(SemaRef, TemplateLoc,
  Param, Template, Converted,
  SourceRange(TemplateLoc, RAngleLoc));


Index: cfe/trunk/test/SemaCXX/alias-template.cpp
===
--- cfe/trunk/test/SemaCXX/alias-template.cpp
+++ cfe/trunk/test/SemaCXX/alias-template.cpp
@@ -179,3 +179,13 @@
 };
 static_assert(__is_same(S<3>::U, X[2]), ""); // expected-error {{static_assert failed}}
 }
+
+namespace PR39623 {
+template 
+using void_t = void;
+
+template >
+int sfinae_me() { return 0; } // expected-note{{candidate template ignored: substitution failure}}
+
+int g = sfinae_me(); // expected-error{{no matching function for call to 'sfinae_me'}}
+}
Index: cfe/trunk/lib/Sema/SemaTemplate.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplate.cpp
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp
@@ -4434,7 +4434,7 @@
 
   // If the argument type is dependent, instantiate it now based
   // on the previously-computed template arguments.
-  if (ArgType->getType()->isDependentType()) {
+  if (ArgType->getType()->isInstantiationDependentType()) {
 Sema::InstantiatingTemplate Inst(SemaRef, TemplateLoc,
  Param, Template, Converted,
  SourceRange(TemplateLoc, RAngleLoc));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

Szelethus updated this revision to Diff 173745.
Szelethus edited the summary of this revision.
Szelethus added a comment.

Restored the discussed note.


https://reviews.llvm.org/D54401

Files:
  include/clang/StaticAnalyzer/Core/CheckerRegistry.h
  include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  lib/FrontendTool/ExecuteCompilerInvocation.cpp
  lib/StaticAnalyzer/Core/CheckerRegistry.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
  test/Analysis/disable-all-checks.c

Index: test/Analysis/disable-all-checks.c
===
--- test/Analysis/disable-all-checks.c
+++ test/Analysis/disable-all-checks.c
@@ -1,9 +1,18 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -analyzer-disable-all-checks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-disable-all-checks -analyzer-checker=core -analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region \
+// RUN:   -analyzer-disable-all-checks -verify %s
+//
+// RUN: %clang_analyze_cc1 -analyzer-disable-all-checks -analyzer-checker=core \
+// RUN:   -analyzer-store=region -verify %s
+//
 // RUN: %clang_analyze_cc1 -analyzer-disable-all-checks -verify %s
-// RUN: not %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -analyzer-disable-checker -verify %s 2>&1 | FileCheck %s
+//
+// RUN: not %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region \
+// RUN:   -analyzer-disable-checker non.existant.Checker -verify %s 2>&1 \
+// RUN:   | FileCheck %s
+//
 // expected-no-diagnostics
 
+// CHECK: no analyzer checkers are associated with 'non.existant.Checker'
 // CHECK: use -analyzer-disable-all-checks to disable all static analyzer checkers
 int buggy() {
   int x = 0;
Index: lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
@@ -130,10 +130,11 @@
 
 void ento::printEnabledCheckerList(raw_ostream ,
ArrayRef plugins,
-   const AnalyzerOptions ) {
+   const AnalyzerOptions ,
+   DiagnosticsEngine ) {
   out << "OVERVIEW: Clang Static Analyzer Enabled Checkers List\n\n";
 
-  ClangCheckerRegistry(plugins).printList(out, opts);
+  ClangCheckerRegistry(plugins).printList(out, opts, diags);
 }
 
 void ento::printAnalyzerConfigList(raw_ostream ) {
Index: lib/StaticAnalyzer/Core/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Core/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Core/CheckerRegistry.cpp
@@ -9,7 +9,6 @@
 
 #include "clang/StaticAnalyzer/Core/CheckerRegistry.h"
 #include "clang/Basic/Diagnostic.h"
-#include "clang/Basic/LLVM.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
@@ -19,50 +18,11 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
-#include 
-#include 
 
 using namespace clang;
 using namespace ento;
 
-static const char PackageSeparator = '.';
-
-using CheckerInfoSet = llvm::SetVector;
-
-namespace {
-/// Represents a request to include or exclude a checker or package from a
-/// specific analysis run.
-///
-/// \sa CheckerRegistry::initializeManager
-class CheckerOptInfo {
-  StringRef Name;
-  bool Enable;
-  bool Claimed;
-
-public:
-  CheckerOptInfo(StringRef name, bool enable)
-: Name(name), Enable(enable), Claimed(false) { }
-
-  StringRef getName() const { return Name; }
-  bool isEnabled() const { return Enable; }
-  bool isDisabled() const { return !isEnabled(); }
-
-  bool isClaimed() const { return Claimed; }
-  bool isUnclaimed() const { return !isClaimed(); }
-  void claim() { Claimed = true; }
-};
-
-} // end of anonymous namespace
-
-static SmallVector
-getCheckerOptList(const AnalyzerOptions ) {
-  SmallVector checkerOpts;
-  for (unsigned i = 0, e = opts.CheckersControlList.size(); i != e; ++i) {
-const std::pair  = opts.CheckersControlList[i];
-checkerOpts.push_back(CheckerOptInfo(opt.first, opt.second));
-  }
-  return checkerOpts;
-}
+static constexpr char PackageSeparator = '.';
 
 static bool checkerNameLT(const CheckerRegistry::CheckerInfo ,
   const CheckerRegistry::CheckerInfo ) {
@@ -86,40 +46,49 @@
   return false;
 }
 
-/// Collects the checkers for the supplied \p opt option into \p collected.
-static void collectCheckers(const CheckerRegistry::CheckerInfoList ,
-const llvm::StringMap ,
-CheckerOptInfo , CheckerInfoSet ) {
-  // Use a binary search to find the possible start of the package.
-  CheckerRegistry::CheckerInfo packageInfo(nullptr, opt.getName(), "");

r346705 - Fix the 'fixit' for inline namespace replacement.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Mon Nov 12 13:08:41 2018
New Revision: 346705

URL: http://llvm.org/viewvc/llvm-project?rev=346705=rev
Log:
Fix the 'fixit' for inline namespace replacement.

I'd neglected to add to the fixit for r346677.  Richard Smith mentioned
this in a review-after-commit, so fixing it here.

Change-Id: I77e612be978d4eedda8d5bbd60b812b88f875cda

Modified:
cfe/trunk/lib/Parse/ParseDeclCXX.cpp

Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=346705=346704=346705=diff
==
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Mon Nov 12 13:08:41 2018
@@ -178,7 +178,10 @@ Parser::DeclGroupPtrTy Parser::ParseName
 } else {
   std::string NamespaceFix;
   for (const auto  : ExtraNSs) {
-NamespaceFix += " { namespace ";
+NamespaceFix += " { ";
+if (ExtraNS.InlineLoc.isValid())
+  NamespaceFix += "inline ";
+NamespaceFix += "namespace ";
 NamespaceFix += ExtraNS.Ident->getName();
   }
 


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

[PATCH] D54324: [AST] Store the value of CharacterLiteral in the bit-fields of Stmt if possible

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: include/clang/AST/Expr.h:1407
 public:
-  enum CharacterKind {
-Ascii,
-Wide,
-UTF8,
-UTF16,
-UTF32
-  };
+  enum CharacterKind { Ascii, Wide, UTF8, UTF16, UTF32 };
 

shafik wrote:
> Minor comment, does it make sense to covert this to a scoped enum since it 
> looks like it is being strictly used as a set of values.
Does it really add anything ?
It means that instead of writing `CharacterLiteral::UTF32`
you have to write `CharacterLiteral::CharacterKind::UTF32`

Seems a bit verbose. But I don't have any strong opinion on this.


Repository:
  rC Clang

https://reviews.llvm.org/D54324



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

[PATCH] D54441: [OPENMP] Support relational-op !- (not-equal) as one of the canonical forms of random access iterator

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rC Clang

https://reviews.llvm.org/D54441



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

[PATCH] D53934: [clangd] Improve code completion for ObjC methods

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 173737.
dgoldman added a comment.

CodeCompleteTests fixes


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53934

Files:
  clangd/CodeCompletionStrings.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/CodeCompletionStringsTests.cpp

Index: unittests/clangd/CodeCompletionStringsTests.cpp
===
--- unittests/clangd/CodeCompletionStringsTests.cpp
+++ unittests/clangd/CodeCompletionStringsTests.cpp
@@ -109,6 +109,53 @@
   EXPECT_EQ(Snippet, "");
 }
 
+TEST_F(CompletionStringTest, ObjectiveCMethodNoArguments) {
+  Builder.AddResultTypeChunk("void");
+  Builder.AddTypedTextChunk("methodName");
+
+  auto *CCS = Builder.TakeString();
+  computeSignature(*CCS);
+  EXPECT_EQ(Signature, "");
+  EXPECT_EQ(Snippet, "");
+}
+
+TEST_F(CompletionStringTest, ObjectiveCMethodOneArgument) {
+  Builder.AddResultTypeChunk("void");
+  Builder.AddTypedTextChunk("methodWithArg:");
+  Builder.AddPlaceholderChunk("(type)");
+
+  auto *CCS = Builder.TakeString();
+  computeSignature(*CCS);
+  EXPECT_EQ(Signature, "(type)");
+  EXPECT_EQ(Snippet, "${1:(type)}");
+}
+
+TEST_F(CompletionStringTest, ObjectiveCMethodTwoArgumentsFromBeginning) {
+  Builder.AddResultTypeChunk("int");
+  Builder.AddTypedTextChunk("withFoo:");
+  Builder.AddPlaceholderChunk("(type)");
+  Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
+  Builder.AddTypedTextChunk("bar:");
+  Builder.AddPlaceholderChunk("(type2)");
+
+  auto *CCS = Builder.TakeString();
+  computeSignature(*CCS);
+  EXPECT_EQ(Signature, "(type) bar:(type2)");
+  EXPECT_EQ(Snippet, "${1:(type)} bar:${2:(type2)}");
+}
+
+TEST_F(CompletionStringTest, ObjectiveCMethodTwoArgumentsFromMiddle) {
+  Builder.AddResultTypeChunk("int");
+  Builder.AddInformativeChunk("withFoo:");
+  Builder.AddTypedTextChunk("bar:");
+  Builder.AddPlaceholderChunk("(type2)");
+
+  auto *CCS = Builder.TakeString();
+  computeSignature(*CCS);
+  EXPECT_EQ(Signature, "(type2)");
+  EXPECT_EQ(Snippet, "${1:(type2)}");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -67,6 +67,7 @@
 }
 MATCHER_P(SnippetSuffix, Text, "") { return arg.SnippetSuffix == Text; }
 MATCHER_P(Origin, OriginSet, "") { return arg.Origin == OriginSet; }
+MATCHER_P(Signature, S, "") { return arg.Signature == S; }
 
 // Shorthand for Contains(Named(Name)).
 Matcher &> Has(std::string Name) {
@@ -105,15 +106,16 @@
 
 CodeCompleteResult completions(ClangdServer , StringRef Text,
std::vector IndexSymbols = {},
-   clangd::CodeCompleteOptions Opts = {}) {
+   clangd::CodeCompleteOptions Opts = {},
+   PathRef FilePath = "foo.cpp") {
   std::unique_ptr OverrideIndex;
   if (!IndexSymbols.empty()) {
 assert(!Opts.Index && "both Index and IndexSymbols given!");
 OverrideIndex = memIndex(std::move(IndexSymbols));
 Opts.Index = OverrideIndex.get();
   }
 
-  auto File = testPath("foo.cpp");
+  auto File = testPath(FilePath);
   Annotations Test(Text);
   runAddDocument(Server, File, Test.code());
   auto CompletionList =
@@ -125,12 +127,14 @@
 // If IndexSymbols is non-empty, an index will be built and passed to opts.
 CodeCompleteResult completions(StringRef Text,
std::vector IndexSymbols = {},
-   clangd::CodeCompleteOptions Opts = {}) {
+   clangd::CodeCompleteOptions Opts = {},
+   PathRef FilePath = "foo.cpp") {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
-  return completions(Server, Text, std::move(IndexSymbols), std::move(Opts));
+  return completions(Server, Text, std::move(IndexSymbols), std::move(Opts),
+ FilePath);
 }
 
 std::string replace(StringRef Haystack, StringRef Needle, StringRef Repl) {
@@ -2187,6 +2191,84 @@
  {cls("naber"), cls("nx::naber")}, Opts);
   EXPECT_THAT(Results.Completions, UnorderedElementsAre());
 }
+
+TEST(CompletionTest, ObjectiveCMethodNoArguments) {
+  auto Results = completions(R"objc(
+  @interface Foo
+  @property(nonatomic, setter=setXToIgnoreComplete:) int value;
+  @end
+  Foo *foo = [Foo new]; int y = [foo v^]
+)objc",
+/*IndexSymbols=*/{},
+/*Opts=*/{},
+"Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(Named("value")));
+  EXPECT_THAT(C, ElementsAre(Kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(ReturnType("int")));
+  EXPECT_THAT(C, ElementsAre(Signature("")));
+  

[PATCH] D54441: [OPENMP] Support relational-op !- (not-equal) as one of the canonical forms of random access iterator

[Anh Tuyen Tran via Phabricator via cfe-commits]

anhtuyen created this revision.
anhtuyen added reviewers: ABataev, Hahnfeld, RaviNarayanaswamy, mikerice, 
kkwli0, hfinkel, gtbercea.
anhtuyen added projects: OpenMP, clang.

In OpenMP 4.5, only 4 relational operators are supported: <, <=, >, and >=.
This work is to enable support for relational operator !- (not-equal) as one of 
the canonical forms.


Repository:
  rC Clang

https://reviews.llvm.org/D54441

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_loop_messages.cpp
  clang/test/OpenMP/distribute_simd_loop_messages.cpp
  clang/test/OpenMP/for_loop_messages.cpp
  clang/test/OpenMP/for_simd_loop_messages.cpp
  clang/test/OpenMP/parallel_for_ast_print.cpp
  clang/test/OpenMP/parallel_for_codegen.cpp
  clang/test/OpenMP/parallel_for_loop_messages.cpp
  clang/test/OpenMP/parallel_for_simd_loop_messages.cpp
  clang/test/OpenMP/simd_loop_messages.cpp
  clang/test/OpenMP/target_parallel_for_loop_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_loop_messages.cpp
  clang/test/OpenMP/target_simd_loop_messages.cpp
  clang/test/OpenMP/target_teams_distribute_loop_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_loop_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_simd_loop_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_loop_messages.cpp
  clang/test/OpenMP/taskloop_loop_messages.cpp
  clang/test/OpenMP/taskloop_simd_loop_messages.cpp
  clang/test/OpenMP/teams_distribute_loop_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_loop_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_loop_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_loop_messages.cpp

Index: clang/test/OpenMP/teams_distribute_simd_loop_messages.cpp
===
--- clang/test/OpenMP/teams_distribute_simd_loop_messages.cpp
+++ clang/test/OpenMP/teams_distribute_simd_loop_messages.cpp
@@ -132,9 +132,9 @@
   for (int i = 0; i != 1; i++)
 c[i] = a[i];
 
+// Ok
 #pragma omp target
 #pragma omp teams distribute simd
-// expected-error@+1 {{condition of OpenMP for loop must be a relational comparison ('<', '<=', '>', or '>=') of loop variable 'i'}}
   for (int i = 0;; i++)
 c[i] = a[i];
 
Index: clang/test/OpenMP/teams_distribute_parallel_for_simd_loop_messages.cpp
===
--- clang/test/OpenMP/teams_distribute_parallel_for_simd_loop_messages.cpp
+++ clang/test/OpenMP/teams_distribute_parallel_for_simd_loop_messages.cpp
@@ -126,9 +126,9 @@
   for (int i = 0; !!i; i++)
 c[i] = a[i];
 
+// Ok
 #pragma omp target
 #pragma omp teams distribute parallel for simd
-// expected-error@+1 {{condition of OpenMP for loop must be a relational comparison ('<', '<=', '>', or '>=') of loop variable 'i'}}
   for (int i = 0; i != 1; i++)
 c[i] = a[i];
 
Index: clang/test/OpenMP/teams_distribute_parallel_for_loop_messages.cpp
===
--- clang/test/OpenMP/teams_distribute_parallel_for_loop_messages.cpp
+++ clang/test/OpenMP/teams_distribute_parallel_for_loop_messages.cpp
@@ -126,9 +126,9 @@
   for (int i = 0; !!i; i++)
 c[i] = a[i];
 
+// Ok
 #pragma omp target
 #pragma omp teams distribute parallel for
-// expected-error@+1 {{condition of OpenMP for loop must be a relational comparison ('<', '<=', '>', or '>=') of loop variable 'i'}}
   for (int i = 0; i != 1; i++)
 c[i] = a[i];
 
Index: clang/test/OpenMP/teams_distribute_loop_messages.cpp
===
--- clang/test/OpenMP/teams_distribute_loop_messages.cpp
+++ clang/test/OpenMP/teams_distribute_loop_messages.cpp
@@ -126,9 +126,9 @@
   for (int i = 0; !!i; i++)
 c[i] = a[i];
 
+// Ok
 #pragma omp target
 #pragma omp teams distribute
-// expected-error@+1 {{condition of OpenMP for loop must be a relational comparison ('<', '<=', '>', or '>=') of loop variable 'i'}}
   for (int i = 0; i != 1; i++)
 c[i] = a[i];
 
Index: clang/test/OpenMP/taskloop_simd_loop_messages.cpp
===
--- clang/test/OpenMP/taskloop_simd_loop_messages.cpp
+++ clang/test/OpenMP/taskloop_simd_loop_messages.cpp
@@ -131,8 +131,8 @@
   for (int i = 0; !!i; i++)
 c[i] = a[i];
 
+// Ok
 #pragma omp parallel
-// expected-error@+2 {{condition of OpenMP for loop must be a relational comparison ('<', '<=', '>', or '>=') of loop variable 'i'}}
 #pragma omp taskloop simd
   for (int i = 0; i != 1; i++)
 c[i] = a[i];
Index: clang/test/OpenMP/taskloop_loop_messages.cpp
===
--- clang/test/OpenMP/taskloop_loop_messages.cpp
+++ clang/test/OpenMP/taskloop_loop_messages.cpp
@@ -131,8 +131,8 @@
   for (int i = 0; !!i; i++)
 c[i] = a[i];
 
+// Ok
 #pragma omp parallel
-// expected-error@+2 {{condition of OpenMP for loop must be a 

[PATCH] D54324: [AST] Store the value of CharacterLiteral in the bit-fields of Stmt if possible

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: include/clang/AST/Expr.h:1407
 public:
-  enum CharacterKind {
-Ascii,
-Wide,
-UTF8,
-UTF16,
-UTF32
-  };
+  enum CharacterKind { Ascii, Wide, UTF8, UTF16, UTF32 };
 

Minor comment, does it make sense to covert this to a scoped enum since it 
looks like it is being strictly used as a set of values.


Repository:
  rC Clang

https://reviews.llvm.org/D54324



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

RE: r346677 - Implement P1094R2 (nested inline namespaces)

[Keane, Erich via cfe-commits]

Coolbeans, will do!  Thanks for the review.

-Erich

From: Richard Smith [mailto:rich...@metafoo.co.uk]
Sent: Monday, November 12, 2018 11:59 AM
To: Keane, Erich 
Cc: cfe-commits 
Subject: Re: r346677 - Implement P1094R2 (nested inline namespaces)

On Mon, 12 Nov 2018 at 09:22, Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Mon Nov 12 09:19:48 2018
New Revision: 346677

URL: http://llvm.org/viewvc/llvm-project?rev=346677=rev
Log:
Implement P1094R2 (nested inline namespaces)

As approved for the Working Paper in San Diego, support annotating
inline namespaces with 'inline'.

Change-Id: I51a654e11ffb475bf27cccb2458768151619e384

Added:
cfe/trunk/test/Parser/cxx2a-inline-nested-namespace-definition.cpp   (with 
props)
Modified:
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=346677=346676=346677=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Mon Nov 12 09:19:48 
2018
@@ -222,6 +222,11 @@ def ext_nested_namespace_definition : Ex
 def warn_cxx14_compat_nested_namespace_definition : Warning<
   "nested namespace definition is incompatible with C++ standards before 
C++17">,
   InGroup, DefaultIgnore;
+def ext_inline_nested_namespace_definition : ExtWarn<
+  "inline nested namespace definition is a C++2a extension">, InGroup;
+def warn_cxx17_compat_inline_nested_namespace_definition : Warning<
+  "inline nested namespace definition is incompatible with C++ standards 
before"
+  " C++2a">, InGroup, DefaultIgnore;
 def err_inline_nested_namespace_definition : Error<
   "nested namespace definition cannot be 'inline'">;
 def err_expected_semi_after_attribute_list : Error<

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=346677=346676=346677=diff
==
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Mon Nov 12 09:19:48 2018
@@ -2659,9 +2659,16 @@ private:
   DeclGroupPtrTy ParseNamespace(DeclaratorContext Context,
 SourceLocation ,
 SourceLocation InlineLoc = SourceLocation());
-  void ParseInnerNamespace(std::vector ,
-   std::vector ,
-   std::vector ,
+
+  struct InnerNamespaceInfo {
+SourceLocation NamespaceLoc;
+SourceLocation InlineLoc;
+SourceLocation IdentLoc;
+IdentifierInfo *Ident;
+  };
+  using InnerNamespaceInfoList = llvm::SmallVector;
+
+  void ParseInnerNamespace(const InnerNamespaceInfoList ,
unsigned int index, SourceLocation ,
ParsedAttributes ,
BalancedDelimiterTracker );

Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=346677=346676=346677=diff
==
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Mon Nov 12 09:19:48 2018
@@ -33,24 +33,23 @@ using namespace clang;
 /// may either be a top level namespace or a block-level namespace alias. If
 /// there was an inline keyword, it has already been parsed.
 ///
-///   namespace-definition: [C++ 7.3: basic.namespace]
+///   namespace-definition: [C++: namespace.def]
 /// named-namespace-definition
 /// unnamed-namespace-definition
+/// nested-namespace-definition
+///
+///   named-namespace-definition:
+/// 'inline'[opt] 'namespace' attributes[opt] identifier '{' 
namespace-body '}'
 ///
 ///   unnamed-namespace-definition:
 /// 'inline'[opt] 'namespace' attributes[opt] '{' namespace-body '}'
 ///
-///   named-namespace-definition:
-/// original-namespace-definition
-/// extension-namespace-definition
+///   nested-namespace-definition:
+/// 'namespace' enclosing-namespace-specifier '::' 'inline'[opt] 
identifier '{' namespace-body '}'
 ///
-///   original-namespace-definition:
-/// 'inline'[opt] 'namespace' identifier attributes[opt]
-/// '{' namespace-body '}'
-///
-///   extension-namespace-definition:
-/// 'inline'[opt] 'namespace' original-namespace-name
-/// '{' namespace-body '}'
+///   enclosing-namespace-specifier:
+/// identifier
+/// enclosing-namespace-specifier '::' 

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

[Mailing List "llvm-commits" via Phabricator via cfe-commits]

llvm-commits added a comment.



- F7538987: msg-17782-346.txt 


Repository:
  rL LLVM

https://reviews.llvm.org/D54310



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

Re: [PATCH] D54310: Make clang-based tools find libc++ on MacOS

[Sam McCall via cfe-commits]

On Mon, Nov 12, 2018, 20:53 George Karpenkov via Phabricator <
revi...@reviews.llvm.org wrote:

> george.karpenkov added a comment.
>
> I don't quite understand the need for this patch.
> If we are talking about a binary built from source, wouldn't it make more
> sense to build libcxx from source as well?
>
I'm not sure what you mean by "built from source" - all programs are?
The intent is to distribute as binary (I think), and not distribute libcxx.
It's important the system stdlib is used, as that's the one consistent with
the CDB and the actual build.

If libcxx is built from source in the same repo, clang-based tools do find
> it.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D54310
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r346699 - PR39628 Treat all non-zero values as 'true' in bool compound-assignment

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Nov 12 12:11:57 2018
New Revision: 346699

URL: http://llvm.org/viewvc/llvm-project?rev=346699=rev
Log:
PR39628 Treat all non-zero values as 'true' in bool compound-assignment
in constant evaluation, not just odd values.

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=346699=346698=346699=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Nov 12 12:11:57 2018
@@ -2074,11 +2074,12 @@ static APSInt HandleIntToIntCast(EvalInf
  QualType DestType, QualType SrcType,
  const APSInt ) {
   unsigned DestWidth = Info.Ctx.getIntWidth(DestType);
-  APSInt Result = Value;
   // Figure out if this is a truncate, extend or noop cast.
   // If the input is signed, do a sign extend, noop, or truncate.
-  Result = Result.extOrTrunc(DestWidth);
+  APSInt Result = Value.extOrTrunc(DestWidth);
   Result.setIsUnsigned(DestType->isUnsignedIntegerOrEnumerationType());
+  if (DestType->isBooleanType())
+Result = Value.getBoolValue();
   return Result;
 }
 

Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp?rev=346699=346698=346699=diff
==
--- cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp Mon Nov 12 12:11:57 
2018
@@ -374,6 +374,30 @@ namespace compound_assign {
   }
   static_assert(test_float(), "");
 
+  constexpr bool test_bool() {
+bool b = false;
+b |= 2;
+if (b != true) return false;
+b <<= 1;
+if (b != true) return false;
+b *= 2;
+if (b != true) return false;
+b -= 1;
+if (b != false) return false;
+b -= 1;
+if (b != true) return false;
+b += -1;
+if (b != false) return false;
+b += 1;
+if (b != true) return false;
+b += 1;
+if (b != true) return false;
+b ^= b;
+if (b != false) return false;
+return true;
+  }
+  static_assert(test_bool(), "");
+
   constexpr bool test_ptr() {
 int arr[123] = {};
 int *p = arr;
@@ -879,7 +903,7 @@ namespace Bitfields {
 --a.n;
 --a.u;
 a.n = -a.n * 3;
-return a.b == false && a.n == 3 && a.u == 31;
+return a.b == true && a.n == 3 && a.u == 31;
   }
   static_assert(test(), "");
 }


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

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

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

Szelethus added inline comments.



Comment at: include/clang/Basic/DiagnosticFrontendKinds.td:177
 "no analyzer checkers are associated with '%0'">;
-def note_suggest_disabling_all_checkers : Note<
-"use -analyzer-disable-all-checks to disable all static analyzer 
checkers">;

NoQ wrote:
> george.karpenkov wrote:
> > @NoQ what does this option even do? Can you think of some legitimate uses?
> rC216763 is the commit in which it was added and the commit message sounds 
> useful. I never understood how exactly are people using it, but it sounds as 
> if they are trying to skip analysis of certain files by appending a flag that 
> disables checkers to the build(?!) command of those files.
Okay, let's move this to another revision then.


Repository:
  rC Clang

https://reviews.llvm.org/D54401



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

[PATCH] D54405: Record whether a AST Matcher constructs a Node

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:77
+  internal::MatcherDescriptor *matchDescriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind<
+typename 
ast_matchers::internal::VariadicAllOfMatcher::Type>();

aaron.ballman wrote:
> Mildly not keen on the use of `auto` here. It's a factory function, so it 
> kind of names the resulting type, but it also looks like the type will be 
> `ast_matchers::internal::VariadicAllOfMatcher::Type` from the 
> template argument, which is incorrect.
There is no reason to assume that taking a template argument means that type is 
result.

The method is `getFrom` which decreases the ambiguity still further.

Spelling out the type doesn't add anything useful. This should be ok.


Repository:
  rC Clang

https://reviews.llvm.org/D54405



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

[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

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

Szelethus created this revision.
Szelethus added reviewers: george.karpenkov, NoQ, xazax.hun, rnkovacs.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, jfb, 
mikhail.ramalho, a.sidorin, mgrang, szepet, whisperity, mgorny.

This patch solves the problem I explained in an earlier revision[1]. It's huge, 
I'm still looking for ways to split it up, hence the WIP tag. Also, it fixes a 
major bug, but has no tests.

The solution is implemented as follows:

- Add a new field to the `Checker` class in `CheckerBase.td` called 
`Dependencies`, which is a list of `Checker`s. I know some of us have mixed 
feelings on the use of tblgen, but I personally found it very helpful, because 
few things are more miserable then working with the preprocessor, and this 
extra complication is actually another point where things can be verified, 
properly organized, and so on. The best way for me to prove that it shouldn't 
be dropped is actually use it and document it properly, so I did just that in 
this patch.
- Move `unix` checkers before `cplusplus`, as there is no forward declaration 
in tblgen :/
- Introduce two new checkers: `CStringBase` and `MallocBase` are essentially 
checkers without any of their modules/subcheckers enabled. These are important, 
as for example `InnerPointerChecker` would like to see `MallocChecker` enabled, 
but does not add `CK_MallocChecker` to `MallocChecker::ChecksEnabled` on it's 
own. This was weird, so from now on, "barebone" versions of `CStringChecker` 
and `MallocChecker` may be enabled via the new checkers. Note that this doesn't 
change anything on the user facing interface.
- Move `InnerPointerChecker` to it's own directory, separate the checker class 
into a header file, move its registry function to `MallocChecker`.
- Clear up and registry functions in `MallocChecker`, happily remove old FIXMEs.
- Remove `lib/StaticAnalyzer/Checkers/InterCheckerAPI.h`.
- Add a new `addDependency` function to `CheckerRegistry`.
- Neatly format `RUN` lines in files I looked at while debugging.

[1]**(Copied from https://reviews.llvm.org/D54397's summary!)** While on the 
quest of fixing checker options the same way I cleaned up non-checker options, 
although I'm making good progress, I ran into a wall: In order to emit warnings 
on incorrect checker options, we need to ensure that checkers actually acquire 
their options properly -- but, I unearthed a huge bug in checker registration: 
dependencies are currently implemented in a way that breaks the already very 
fragile registration infrastructure.

Here is where the problem really originates from: this is a snippet from 
`CheckerRegistry::initializeManager`.

  // Initialize the CheckerManager with all enabled checkers.
  for (const auto *i :enabledCheckers) {
checkerMgr.setCurrentCheckName(CheckName(i->FullName));
i->Initialize(checkerMgr);
  }

Note that `Initialize` is a function pointer to `register##CHECKERNAME`, like 
`registerInnerPointerChecker`. Since for each register function call the 
current checker name is only set once, when `InnerPointerChecker` attempts to 
also register it's dependent `MallocChecker`, both it and `MallocChecker` will 
receive the `cplusplus.InnerPointer` name. This results in `MallocChecker` 
trying to acquire it's `Optimistic` option as 
`cplusplus.InnerPointerChecker:Optimistic`.

Clearly, this problem has to be solved at a higher level -- it makes no sense 
to be able to affect other checkers in a registry function. Since I'll already 
make changes to how checker registration works, I'll probably tune some things 
for the current C++ standard, add much needed documentation, and try to make 
the code //a lot less confusing//.


Repository:
  rC Clang

https://reviews.llvm.org/D54438

Files:
  include/clang/StaticAnalyzer/Checkers/CheckerBase.td
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Checkers/InnerPointer/InnerPointerChecker.cpp
  lib/StaticAnalyzer/Checkers/InnerPointer/InnerPointerChecker.h
  lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
  lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/NewDelete+MismatchedDeallocator_intersections.cpp
  test/Analysis/NewDelete-checker-test.cpp
  test/Analysis/inner-pointer.cpp
  utils/TableGen/ClangSACheckersEmitter.cpp

Index: utils/TableGen/ClangSACheckersEmitter.cpp
===
--- utils/TableGen/ClangSACheckersEmitter.cpp
+++ utils/TableGen/ClangSACheckersEmitter.cpp
@@ -57,14 +57,26 @@
   return std::string();
 }
 
+static void printChecker(llvm::raw_ostream , const Record ) {
+  OS << '\"';
+  OS.write_escaped(getCheckerFullName()) << "\", ";
+  OS << R.getName() << ", \"";
+  

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: include/clang/Basic/DiagnosticFrontendKinds.td:177
 "no analyzer checkers are associated with '%0'">;
-def note_suggest_disabling_all_checkers : Note<
-"use -analyzer-disable-all-checks to disable all static analyzer 
checkers">;

george.karpenkov wrote:
> @NoQ what does this option even do? Can you think of some legitimate uses?
rC216763 is the commit in which it was added and the commit message sounds 
useful. I never understood how exactly are people using it, but it sounds as if 
they are trying to skip analysis of certain files by appending a flag that 
disables checkers to the build(?!) command of those files.


Repository:
  rC Clang

https://reviews.llvm.org/D54401



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

Re: r346677 - Implement P1094R2 (nested inline namespaces)

[Richard Smith via cfe-commits]

On Mon, 12 Nov 2018 at 09:22, Erich Keane via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: erichkeane
> Date: Mon Nov 12 09:19:48 2018
> New Revision: 346677
>
> URL: http://llvm.org/viewvc/llvm-project?rev=346677=rev
> Log:
> Implement P1094R2 (nested inline namespaces)
>
> As approved for the Working Paper in San Diego, support annotating
> inline namespaces with 'inline'.
>
> Change-Id: I51a654e11ffb475bf27cccb2458768151619e384
>
> Added:
> cfe/trunk/test/Parser/cxx2a-inline-nested-namespace-definition.cpp
>  (with props)
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
> cfe/trunk/include/clang/Parse/Parser.h
> cfe/trunk/lib/Parse/ParseDeclCXX.cpp
> cfe/trunk/www/cxx_status.html
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=346677=346676=346677=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Mon Nov 12
> 09:19:48 2018
> @@ -222,6 +222,11 @@ def ext_nested_namespace_definition : Ex
>  def warn_cxx14_compat_nested_namespace_definition : Warning<
>"nested namespace definition is incompatible with C++ standards before
> C++17">,
>InGroup, DefaultIgnore;
> +def ext_inline_nested_namespace_definition : ExtWarn<
> +  "inline nested namespace definition is a C++2a extension">,
> InGroup;
> +def warn_cxx17_compat_inline_nested_namespace_definition : Warning<
> +  "inline nested namespace definition is incompatible with C++ standards
> before"
> +  " C++2a">, InGroup, DefaultIgnore;
>  def err_inline_nested_namespace_definition : Error<
>"nested namespace definition cannot be 'inline'">;
>  def err_expected_semi_after_attribute_list : Error<
>
> Modified: cfe/trunk/include/clang/Parse/Parser.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=346677=346676=346677=diff
>
> ==
> --- cfe/trunk/include/clang/Parse/Parser.h (original)
> +++ cfe/trunk/include/clang/Parse/Parser.h Mon Nov 12 09:19:48 2018
> @@ -2659,9 +2659,16 @@ private:
>DeclGroupPtrTy ParseNamespace(DeclaratorContext Context,
>  SourceLocation ,
>  SourceLocation InlineLoc =
> SourceLocation());
> -  void ParseInnerNamespace(std::vector ,
> -   std::vector ,
> -   std::vector ,
> +
> +  struct InnerNamespaceInfo {
> +SourceLocation NamespaceLoc;
> +SourceLocation InlineLoc;
> +SourceLocation IdentLoc;
> +IdentifierInfo *Ident;
> +  };
> +  using InnerNamespaceInfoList = llvm::SmallVector;
> +
> +  void ParseInnerNamespace(const InnerNamespaceInfoList ,
> unsigned int index, SourceLocation ,
> ParsedAttributes ,
> BalancedDelimiterTracker );
>
> Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=346677=346676=346677=diff
>
> ==
> --- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Mon Nov 12 09:19:48 2018
> @@ -33,24 +33,23 @@ using namespace clang;
>  /// may either be a top level namespace or a block-level namespace alias.
> If
>  /// there was an inline keyword, it has already been parsed.
>  ///
> -///   namespace-definition: [C++ 7.3: basic.namespace]
> +///   namespace-definition: [C++: namespace.def]
>  /// named-namespace-definition
>  /// unnamed-namespace-definition
> +/// nested-namespace-definition
> +///
> +///   named-namespace-definition:
> +/// 'inline'[opt] 'namespace' attributes[opt] identifier '{'
> namespace-body '}'
>  ///
>  ///   unnamed-namespace-definition:
>  /// 'inline'[opt] 'namespace' attributes[opt] '{' namespace-body
> '}'
>  ///
> -///   named-namespace-definition:
> -/// original-namespace-definition
> -/// extension-namespace-definition
> +///   nested-namespace-definition:
> +/// 'namespace' enclosing-namespace-specifier '::' 'inline'[opt]
> identifier '{' namespace-body '}'
>  ///
> -///   original-namespace-definition:
> -/// 'inline'[opt] 'namespace' identifier attributes[opt]
> -/// '{' namespace-body '}'
> -///
> -///   extension-namespace-definition:
> -/// 'inline'[opt] 'namespace' original-namespace-name
> -/// '{' namespace-body '}'
> +///   enclosing-namespace-specifier:
> +/// identifier
> +/// enclosing-namespace-specifier '::' 'inline'[opt] identifier
>  ///
>  ///   

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

In the future, for macOS-specific changes I think it would be better to wait 
for a sign-off from at least one maintainer who is an expert on Apple tools.


Repository:
  rL LLVM

https://reviews.llvm.org/D54310



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

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

I suspect it broke 
http://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/14090/console as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D54310



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

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

I don't quite understand the need for this patch.
If we are talking about a binary built from source, wouldn't it make more sense 
to build libcxx from source as well?
If libcxx is built from source in the same repo, clang-based tools do find it.


Repository:
  rL LLVM

https://reviews.llvm.org/D54310



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

[PATCH] D54437: [analyzer][NFC] Merge ClangCheckerRegistry to CheckerRegistry

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

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

Now that `CheckerRegistry` lies in `Frontend`, we can finally eliminate 
`ClangCheckerRegistry`. Fortunately, this also provides us with a 
`DiagnosticsEngine`, so I went ahead and removed some parameters from it's 
methods.


Repository:
  rC Clang

https://reviews.llvm.org/D54437

Files:
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  lib/FrontendTool/ExecuteCompilerInvocation.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -10,17 +10,73 @@
 #include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/DynamicLibrary.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 
 using namespace clang;
 using namespace ento;
+using llvm::sys::DynamicLibrary;
+
+using RegisterCheckersFn = void (*)(CheckerRegistry &);
+
+static bool isCompatibleAPIVersion(const char *versionString) {
+  // If the version string is null, it's not an analyzer plugin.
+  if (!versionString)
+return false;
+
+  // For now, none of the static analyzer API is considered stable.
+  // Versions must match exactly.
+  return strcmp(versionString, CLANG_ANALYZER_API_VERSION_STRING) == 0;
+}
+
+CheckerRegistry::CheckerRegistry(ArrayRef plugins,
+ DiagnosticsEngine ) : Diags(diags) {
+#define GET_CHECKERS
+#define CHECKER(FULLNAME, CLASS, HELPTEXT) \
+  addChecker(register##CLASS, FULLNAME, HELPTEXT);
+#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
+#undef CHECKER
+#undef GET_CHECKERS
+
+  for (ArrayRef::iterator i = plugins.begin(), e = plugins.end();
+   i != e; ++i) {
+// Get access to the plugin.
+std::string err;
+DynamicLibrary lib = DynamicLibrary::getPermanentLibrary(i->c_str(), );
+if (!lib.isValid()) {
+  diags.Report(diag::err_fe_unable_to_load_plugin) << *i << err;
+  continue;
+}
+
+// See if it's compatible with this build of clang.
+const char *pluginAPIVersion =
+  (const char *) lib.getAddressOfSymbol("clang_analyzerAPIVersionString");
+if (!isCompatibleAPIVersion(pluginAPIVersion)) {
+  Diags.Report(diag::warn_incompatible_analyzer_plugin_api)
+  << llvm::sys::path::filename(*i);
+  Diags.Report(diag::note_incompatible_analyzer_plugin_api)
+  << CLANG_ANALYZER_API_VERSION_STRING
+  << pluginAPIVersion;
+  continue;
+}
+
+// Register its checkers.
+RegisterCheckersFn registerPluginCheckers =
+  (RegisterCheckersFn) (intptr_t) lib.getAddressOfSymbol(
+  "clang_registerCheckers");
+if (registerPluginCheckers)
+  registerPluginCheckers(*this);
+  }
+}
 
 static constexpr char PackageSeparator = '.';
 
@@ -47,8 +103,7 @@
 }
 
 CheckerRegistry::CheckerInfoSet CheckerRegistry::getEnabledCheckers(
-  const AnalyzerOptions ,
-  DiagnosticsEngine ) const {
+const AnalyzerOptions ) const {
 
   assert(std::is_sorted(Checkers.begin(), Checkers.end(), checkerNameLT) &&
  "In order to efficiently gather checkers, this function expects them "
@@ -65,7 +120,7 @@
 
 if (firstRelatedChecker == end ||
 !isInPackage(*firstRelatedChecker, opt.first)) {
-  diags.Report(diag::err_unknown_analyzer_checker) << opt.first;
+  Diags.Report(diag::err_unknown_analyzer_checker) << opt.first;
   return {};
 }
 
@@ -104,23 +159,22 @@
 }
 
 void CheckerRegistry::initializeManager(CheckerManager ,
-const AnalyzerOptions ,
-DiagnosticsEngine ) const {
+const AnalyzerOptions ) const {
   // Sort checkers for efficient collection.
   llvm::sort(Checkers, checkerNameLT);
 
   // Collect checkers enabled by the options.
-  CheckerInfoSet enabledCheckers = getEnabledCheckers(Opts, diags);
+  CheckerInfoSet enabledCheckers = 

r346696 - [NFC] Fix formatting in inline nested namespace definition.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Mon Nov 12 11:29:26 2018
New Revision: 346696

URL: http://llvm.org/viewvc/llvm-project?rev=346696=rev
Log:
[NFC] Fix formatting in inline nested namespace definition.

Apparently my invocation of clang-format in VIM didn't get this right,
but the patch-version DID. This patch just runs CF on this file.

Change-Id: Ied462a2d921cbb813fa427740d3ef6e97959b56d

Modified:
cfe/trunk/test/Parser/cxx2a-inline-nested-namespace-definition.cpp

Modified: cfe/trunk/test/Parser/cxx2a-inline-nested-namespace-definition.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx2a-inline-nested-namespace-definition.cpp?rev=346696=346695=346696=diff
==
--- cfe/trunk/test/Parser/cxx2a-inline-nested-namespace-definition.cpp 
(original)
+++ cfe/trunk/test/Parser/cxx2a-inline-nested-namespace-definition.cpp Mon Nov 
12 11:29:26 2018
@@ -16,14 +16,14 @@ inline namespace foo4::foo5::foo6 { // e
 #else
 // expected-warning@+2 {{inline nested namespace definition is incompatible 
with C++ standards before C++2a}}
 #endif
-namespace valid1::valid2::inline valid3::inline valid4::valid5{}
+namespace valid1::valid2::inline valid3::inline valid4::valid5 {}
 // expected-note@-1 2 {{previous definition is here}}
 
 #if __cplusplus <= 201402L
 // expected-warning@+3 {{nested namespace definition is a C++17 extension; 
define each namespace separately}}
 #endif
 //expected-warning@+1 2 {{inline namespace reopened as a non-inline namespace}}
-namespace valid1::valid2::valid3::valid4::valid5{}
+namespace valid1::valid2::valid3::valid4::valid5 {}
 
 #if __cplusplus <= 201402L
 // expected-warning@+7 {{nested namespace definition is a C++17 extension; 
define each namespace separately}}
@@ -33,19 +33,19 @@ namespace valid1::valid2::valid3::valid4
 #else
 // expected-warning@+2 {{inline nested namespace definition is incompatible 
with C++ standards before C++2a}}
 #endif
-namespace valid1::valid2::inline valid3::inline valid4::valid5{}
+namespace valid1::valid2::inline valid3::inline valid4::valid5 {}
 // expected-note@-1 2 {{previous definition is here}}
 
 namespace valid1 {
-  namespace valid2 {
+namespace valid2 {
 //expected-warning@+1 {{inline namespace reopened as a non-inline namespace}}
-namespace valid3 {
+namespace valid3 {
 //expected-warning@+1 {{inline namespace reopened as a non-inline namespace}}
-  namespace valid4 {
-namespace valid5 {
-}
-  }
-}
-  }
+namespace valid4 {
+namespace valid5 {
 }
+} // namespace valid4
+} // namespace valid3
+} // namespace valid2
+} // namespace valid1
 


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

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

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

Szelethus added a comment.

In https://reviews.llvm.org/D54401#1295832, @george.karpenkov wrote:

> > Also, remove diags::note_suggest_disabling_all_checkers.
>
> Isn't that a separate revision? Given that removing existing options is often 
> questionable, I'd much rather see this patch separated.


This isn't an option, just a note :)


Repository:
  rC Clang

https://reviews.llvm.org/D54401



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

[PATCH] D54429: [analyzer] Creating standard Sphinx documentation

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

Szelethus added a comment.

In https://reviews.llvm.org/D54429#1295838, @george.karpenkov wrote:

> What do you propose we should do with other pages on the existing website? 
> (OpenProjects / etc.)


I think we should just move everything here, and forget about the old site, as 
there clearly isn't a desire to maintain it any longer.


Repository:
  rC Clang

https://reviews.llvm.org/D54429



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

[PATCH] D54436: [analyzer][NFC] Move CheckerRegistry from the Core directory to Frontend

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

Szelethus created this revision.
Szelethus added reviewers: george.karpenkov, NoQ, xazax.hun.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, whisperity, mgorny.
Herald added a reviewer: teemperor.

`ClangCheckerRegistry` is a very non-obvious, poorly documented, weird concept. 
It derives from `CheckerRegistry`, and is placed in 
`lib/StaticAnalyzer/Frontend`, whereas it's base is located in 
`lib/StaticAnalyzer/Core`. It was, from what I can imagine, used to circumvent 
the problem that the registry functions of the checkers are located in the 
`clangStaticAnalyzerCheckers` library, but that library depends on 
`clangStaticAnalyzerCore`. However, `clangStaticAnalyzerFrontend` depends on 
both of those libraries.

One can make the observation however, that `CheckerRegistry` has no place in 
`Core`, it isn't used there at all! The only place where it is used is 
`Frontend`, which where it ultimately belongs.

This move implies that since 
`include/clang/StaticAnalyzer/Checkers/ClangCheckers.h` only a single function:

  class CheckerRegistry;
  
  void registerBuiltinCheckers(CheckerRegistry );

it had to re purposed, as `CheckerRegistry` is no longer avaible to 
`clangStaticAnalyzerCheckers`. It was renamed to 
`BuiltinCheckerRegistration.h`, which actually describes it -- it **does not** 
contain the registration functions for checkers, but only those generated by 
the tblgen files.


Repository:
  rC Clang

https://reviews.llvm.org/D54436

Files:
  include/clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h
  include/clang/StaticAnalyzer/Checkers/ClangCheckers.h
  include/clang/StaticAnalyzer/Core/CheckerRegistry.h
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
  lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp
  lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
  lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
  lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
  lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
  lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
  lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
  lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
  lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
  lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp
  lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp
  lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  lib/StaticAnalyzer/Checkers/CheckObjCInstMethSignature.cpp
  lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp
  lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
  lib/StaticAnalyzer/Checkers/ChrootChecker.cpp
  lib/StaticAnalyzer/Checkers/ClangCheckers.cpp
  lib/StaticAnalyzer/Checkers/ClangSACheckers.h
  lib/StaticAnalyzer/Checkers/CloneChecker.cpp
  lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
  lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
  lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
  lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
  lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
  lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
  lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp
  lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
  lib/StaticAnalyzer/Checkers/GTestChecker.cpp
  lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp
  lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
  lib/StaticAnalyzer/Checkers/LLVMConventionsChecker.cpp
  lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
  lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
  lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
  lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
  lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
  lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
  lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp
  lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
  lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
  lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
  lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
  lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
  

[PATCH] D54334: [AVR] Automatically link CRT and libgcc from the system avr-gcc

[Dylan McKay via Phabricator via cfe-commits]

dylanmckay updated this revision to Diff 173722.
dylanmckay added a comment.

Add the search path that Ubuntu installs libc to.


Repository:
  rC Clang

https://reviews.llvm.org/D54334

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/DiagnosticGroups.td
  lib/Driver/ToolChains/AVR.cpp
  lib/Driver/ToolChains/AVR.h
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/avr-link-mcu-family-unimplemented.c
  test/Driver/avr-link-no-mcu-specified.c
  test/Driver/avr-link-nostdlib-nodefaultlibs.c

Index: test/Driver/avr-link-nostdlib-nodefaultlibs.c
===
--- /dev/null
+++ test/Driver/avr-link-nostdlib-nodefaultlibs.c
@@ -0,0 +1,8 @@
+// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps -mmcu=atmega328 -nostdlib %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps -mmcu=atmega328 -nodefaultlibs %s 2>&1 | FileCheck %s
+
+// nostdlib and nodefaultlibs programs should compile fine.
+
+// CHECK: main
+int main() { return 0; }
+
Index: test/Driver/avr-link-no-mcu-specified.c
===
--- /dev/null
+++ test/Driver/avr-link-no-mcu-specified.c
@@ -0,0 +1,10 @@
+// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps %s 2>&1 | FileCheck --check-prefix=WARN %s
+// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps -mmcu=atmega328 %s 2>&1 | FileCheck --check-prefix=NOWARN %s
+
+// WARN: warning: no target microcontroller specified on command line, cannot link standard libraries, please pass -mmcu=
+// WARN: warning: standard library not linked and so no interrupt vector table or compiler runtime routines will be linked
+
+// NOWARN: main
+
+int main() { return 0; }
+
Index: test/Driver/avr-link-mcu-family-unimplemented.c
===
--- /dev/null
+++ test/Driver/avr-link-mcu-family-unimplemented.c
@@ -0,0 +1,7 @@
+// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps -mmcu=attiny13a %s 2>&1 | FileCheck --check-prefix=WARN %s
+
+// WARN: warning: support for linking stdlibs for microcontroller 'attiny13a' is not implemented, please file an AVR backend bug at http://bugs.llvm.org/ with your mcu name
+// WARN: warning: standard library not linked and so no interrupt vector table or compiler runtime routines will be linked
+
+int main() { return 0; }
+
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -1865,6 +1865,9 @@
   static const char *const ARMebHFTriples[] = {
   "armeb-linux-gnueabihf", "armebv7hl-redhat-linux-gnueabi"};
 
+  static const char *const AVRLibDirs[] = {"/lib"};
+  static const char *const AVRTriples[] = {"avr"};
+
   static const char *const X86_64LibDirs[] = {"/lib64", "/lib"};
   static const char *const X86_64Triples[] = {
   "x86_64-linux-gnu",   "x86_64-unknown-linux-gnu",
@@ -2077,6 +2080,10 @@
   TripleAliases.append(begin(ARMebTriples), end(ARMebTriples));
 }
 break;
+  case llvm::Triple::avr:
+LibDirs.append(begin(AVRLibDirs), end(AVRLibDirs));
+TripleAliases.append(begin(AVRTriples), end(AVRTriples));
+break;
   case llvm::Triple::x86_64:
 LibDirs.append(begin(X86_64LibDirs), end(X86_64LibDirs));
 TripleAliases.append(begin(X86_64Triples), end(X86_64Triples));
@@ -2205,6 +2212,8 @@
   return false;
   } else if (isRISCV(TargetArch)) {
 findRISCVMultilibs(D, TargetTriple, Path, Args, Detected);
+  } else if (TargetArch == llvm::Triple::avr) {
+// AVR has no multilibs.
   } else if (!findBiarchMultilibs(D, TargetTriple, Path, Args,
   NeedsBiarchSuffix, Detected)) {
 return false;
Index: lib/Driver/ToolChains/AVR.h
===
--- lib/Driver/ToolChains/AVR.h
+++ lib/Driver/ToolChains/AVR.h
@@ -20,26 +20,45 @@
 namespace toolchains {
 
 class LLVM_LIBRARY_VISIBILITY AVRToolChain : public Generic_ELF {
-protected:
-  Tool *buildLinker() const override;
 public:
   AVRToolChain(const Driver , const llvm::Triple ,
const llvm::opt::ArgList );
+protected:
+  Tool *buildLinker() const override;
+
+private:
+  /// Whether libgcc, libct, and friends should be linked.
+  ///
+  /// This is not done if the user does not specify a
+  /// microcontroller on the command line.
+  bool LinkStdlib;
+
+  llvm::Optional findAVRLibcInstallation() const;
 };
 
 } // end namespace toolchains
 
 namespace tools {
 namespace AVR {
 class LLVM_LIBRARY_VISIBILITY Linker : public GnuTool {
 public:
-  Linker(const ToolChain ) : GnuTool("AVR::Linker", "avr-ld", TC) {}
+  Linker(const llvm::Triple ,
+ const ToolChain ,
+ bool LinkStdlib)
+: GnuTool("AVR::Linker", "avr-ld", TC),
+  Triple(Triple),
+  LinkStdlib(LinkStdlib) {}
+
   

[PATCH] D54429: [analyzer] Creating standard Sphinx documentation

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

From a first glance, it's missing screenshots and an introduction section.

What do you propose we should do with other pages on the existing website? 
(OpenProjects / etc.)


Repository:
  rC Clang

https://reviews.llvm.org/D54429



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

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

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

> Also, remove diags::note_suggest_disabling_all_checkers.

Isn't that a separate revision? Given that removing existing options is often 
questionable, I'd much rather see this patch separated.


Repository:
  rC Clang

https://reviews.llvm.org/D54401



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

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

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added inline comments.



Comment at: include/clang/Basic/DiagnosticFrontendKinds.td:177
 "no analyzer checkers are associated with '%0'">;
-def note_suggest_disabling_all_checkers : Note<
-"use -analyzer-disable-all-checks to disable all static analyzer 
checkers">;

@NoQ what does this option even do? Can you think of some legitimate uses?


Repository:
  rC Clang

https://reviews.llvm.org/D54401



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

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

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

Szelethus updated this revision to Diff 173717.
Szelethus set the repository for this revision to rC Clang.
Szelethus added a comment.

Might as well make it a method.


Repository:
  rC Clang

https://reviews.llvm.org/D54401

Files:
  include/clang/Basic/DiagnosticFrontendKinds.td
  include/clang/StaticAnalyzer/Core/CheckerRegistry.h
  include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  lib/FrontendTool/ExecuteCompilerInvocation.cpp
  lib/StaticAnalyzer/Core/CheckerRegistry.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
  test/Analysis/disable-all-checks.c

Index: test/Analysis/disable-all-checks.c
===
--- test/Analysis/disable-all-checks.c
+++ test/Analysis/disable-all-checks.c
@@ -1,10 +1,18 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -analyzer-disable-all-checks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-disable-all-checks -analyzer-checker=core -analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region \
+// RUN:   -analyzer-disable-all-checks -verify %s
+//
+// RUN: %clang_analyze_cc1 -analyzer-disable-all-checks -analyzer-checker=core \
+// RUN:   -analyzer-store=region -verify %s
+//
 // RUN: %clang_analyze_cc1 -analyzer-disable-all-checks -verify %s
-// RUN: not %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -analyzer-disable-checker -verify %s 2>&1 | FileCheck %s
+//
+// RUN: not %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region \
+// RUN:   -analyzer-disable-checker non.existant.Checker -verify %s 2>&1 \
+// RUN:   | FileCheck %s
+//
 // expected-no-diagnostics
 
-// CHECK: use -analyzer-disable-all-checks to disable all static analyzer checkers
+// CHECK: no analyzer checkers are associated with 'non.existant.Checker'
 int buggy() {
   int x = 0;
   return 5/x; // no warning
Index: lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
@@ -130,10 +130,11 @@
 
 void ento::printEnabledCheckerList(raw_ostream ,
ArrayRef plugins,
-   const AnalyzerOptions ) {
+   const AnalyzerOptions ,
+   DiagnosticsEngine ) {
   out << "OVERVIEW: Clang Static Analyzer Enabled Checkers List\n\n";
 
-  ClangCheckerRegistry(plugins).printList(out, opts);
+  ClangCheckerRegistry(plugins).printList(out, opts, diags);
 }
 
 void ento::printAnalyzerConfigList(raw_ostream ) {
Index: lib/StaticAnalyzer/Core/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Core/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Core/CheckerRegistry.cpp
@@ -9,7 +9,6 @@
 
 #include "clang/StaticAnalyzer/Core/CheckerRegistry.h"
 #include "clang/Basic/Diagnostic.h"
-#include "clang/Basic/LLVM.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
@@ -19,50 +18,11 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
-#include 
-#include 
 
 using namespace clang;
 using namespace ento;
 
-static const char PackageSeparator = '.';
-
-using CheckerInfoSet = llvm::SetVector;
-
-namespace {
-/// Represents a request to include or exclude a checker or package from a
-/// specific analysis run.
-///
-/// \sa CheckerRegistry::initializeManager
-class CheckerOptInfo {
-  StringRef Name;
-  bool Enable;
-  bool Claimed;
-
-public:
-  CheckerOptInfo(StringRef name, bool enable)
-: Name(name), Enable(enable), Claimed(false) { }
-
-  StringRef getName() const { return Name; }
-  bool isEnabled() const { return Enable; }
-  bool isDisabled() const { return !isEnabled(); }
-
-  bool isClaimed() const { return Claimed; }
-  bool isUnclaimed() const { return !isClaimed(); }
-  void claim() { Claimed = true; }
-};
-
-} // end of anonymous namespace
-
-static SmallVector
-getCheckerOptList(const AnalyzerOptions ) {
-  SmallVector checkerOpts;
-  for (unsigned i = 0, e = opts.CheckersControlList.size(); i != e; ++i) {
-const std::pair  = opts.CheckersControlList[i];
-checkerOpts.push_back(CheckerOptInfo(opt.first, opt.second));
-  }
-  return checkerOpts;
-}
+static constexpr char PackageSeparator = '.';
 
 static bool checkerNameLT(const CheckerRegistry::CheckerInfo ,
   const CheckerRegistry::CheckerInfo ) {
@@ -86,40 +46,48 @@
   return false;
 }
 
-/// Collects the checkers for the supplied \p opt option into \p collected.
-static void collectCheckers(const CheckerRegistry::CheckerInfoList ,
-const llvm::StringMap ,
-CheckerOptInfo , CheckerInfoSet ) {
-  // Use a binary 

[PATCH] D53764: [OpenCL] Enable address spaces for references in C++

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaExprCXX.cpp:4289
+ /*BasePath=*/nullptr, CCK)
+   .get();
 

Okay.  But if `ToType` *isn't* a reference type, this will never be an 
address-space conversion.  I feel like this code could be written more clearly 
to express what it's trying to do.


https://reviews.llvm.org/D53764



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

[PATCH] D54246: [clang-tidy] Add the abseil-duration-factory-scale check

[Hyrum Wright via Phabricator via cfe-commits]

hwright added inline comments.



Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:57-58
+// One and only one of `IntLit` and `FloatLit` should be provided.
+static double GetValue(const IntegerLiteral *IntLit,
+   const FloatingLiteral *FloatLit) {
+  if (IntLit) {

aaron.ballman wrote:
> I really don't like this interface where you pass two arguments, only one of 
> which is ever valid. That is pretty confusing. Given that the result of this 
> function is only ever passed to `GetNewMultScale()`, and that function only 
> does integral checks, I'd prefer logic more like:
> 
> * If the literal is integral, get its value and call `GetNewMultScale()`.
> * If the literal is float, convert it to an integral and call 
> `GetNewMultScale()` only if the conversion is exact (this can be done via 
> `APFloat::convertToInteger()`).
> * `GetNewMultScale()` can now accept an integer value and removes the 
> questions about inexact equality tests from the function.
> 
> With that logic, I don't see a need for `GetValue()` at all, but if a helper 
> function is useful, I'd probably guess this is a better signature: `int64_t 
> getIntegralValue(const Expr *Literal, bool );`
>  Given that the result of this function is only ever passed to 
> `GetNewMultScale()`, and that function only does integral checks, I'd prefer 
> logic more like:

That's actually not true: `GetNewMultScale()` does checks against values like 
`1e-3` which aren't integers.  Does this change your suggestion?



Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:63
+  assert(FloatLit != nullptr && "Neither IntLit nor FloatLit set");
+  return FloatLit->getValueAsApproximateDouble();
+}

aaron.ballman wrote:
> I believe the approximate results here can lead to bugs where the 
> floating-point literal is subnormal -- it may return 0.0 for literals that 
> are not zero.
Do you have an example which I could put in a test?



Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:81-84
+if (Multiplier == 60.0)
+  return DurationScale::Minutes;
+if (Multiplier == 1e-3)
+  return DurationScale::Milliseconds;

aaron.ballman wrote:
> What about scaling with a multiplier of 3600 to go from seconds to hours, and 
> other plausible conversions?
That's a good point, and part of a broader design discussion: should we support 
all multipliers?  (e.g., what about multiplying microseconds by 
`1.0/864.0`?)

If we do think it's worth handling all of these cases, we probably want a 
different construct than the equivalent of a lookup table to do this 
computation.


https://reviews.llvm.org/D54246



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

[PATCH] D54246: [clang-tidy] Add the abseil-duration-factory-scale check

[Hyrum Wright via Phabricator via cfe-commits]

hwright updated this revision to Diff 173714.
hwright marked 11 inline comments as done.
hwright added a comment.

Addressed small concerns.

Larger issues pending feedback.


https://reviews.llvm.org/D54246

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationFactoryScaleCheck.cpp
  clang-tidy/abseil/DurationFactoryScaleCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-duration-factory-scale.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-duration-factory-scale.cpp

Index: test/clang-tidy/abseil-duration-factory-scale.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-duration-factory-scale.cpp
@@ -0,0 +1,110 @@
+// RUN: %check_clang_tidy %s abseil-duration-factory-scale %t
+
+// Mimic the implementation of absl::Duration
+namespace absl {
+
+class Duration {};
+
+Duration Nanoseconds(long long);
+Duration Microseconds(long long);
+Duration Milliseconds(long long);
+Duration Seconds(long long);
+Duration Minutes(long long);
+Duration Hours(long long);
+
+#define GENERATE_DURATION_FACTORY_OVERLOADS(NAME) \
+  Duration NAME(float n); \
+  Duration NAME(double n);\
+  template\
+  Duration NAME(T n);
+
+GENERATE_DURATION_FACTORY_OVERLOADS(Nanoseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Microseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Milliseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Seconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Minutes);
+GENERATE_DURATION_FACTORY_OVERLOADS(Hours);
+#undef GENERATE_DURATION_FACTORY_OVERLOADS
+
+}  // namespace absl
+
+void ScaleTest() {
+  absl::Duration d;
+
+  // Zeroes
+  d = absl::Hours(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Minutes(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Seconds(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Milliseconds(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Microseconds(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Nanoseconds(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Seconds(0.0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+
+  // Fold seconds into minutes
+  d = absl::Seconds(30 * 60);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Minutes(30);
+  d = absl::Seconds(60 * 30);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Minutes(30);
+
+  // Try a few more exotic multiplications
+  d = absl::Seconds(60 * 30 * 60);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Minutes(60 * 30);
+  d = absl::Seconds(1e-3 * 30);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Milliseconds(30);
+  d = absl::Milliseconds(30 * 1000);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Seconds(30);
+  d = absl::Milliseconds(30 * 0.001);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Microseconds(30);
+
+  // Division
+  d = absl::Hours(30 / 60.);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Minutes(30);
+  d = absl::Seconds(30 / 1000.);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Milliseconds(30);
+  d = absl::Milliseconds(30 / 1e3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Microseconds(30);
+