r355666 - [Clang] Include the test directory ommited in r355665

[Petr Hosek via cfe-commits]

Author: phosek
Date: Thu Mar  7 22:16:32 2019
New Revision: 355666

URL: http://llvm.org/viewvc/llvm-project?rev=355666=rev
Log:
[Clang] Include the test directory ommited in r355665

This was omitted in r355655 causing the test to fail.

Added:

cfe/trunk/test/Driver/Inputs/basic_linux_libcxx_tree/usr/lib/x86_64-linux-gnu/

cfe/trunk/test/Driver/Inputs/basic_linux_libcxx_tree/usr/lib/x86_64-linux-gnu/.keep

Added: 
cfe/trunk/test/Driver/Inputs/basic_linux_libcxx_tree/usr/lib/x86_64-linux-gnu/.keep
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/basic_linux_libcxx_tree/usr/lib/x86_64-linux-gnu/.keep?rev=355666=auto
==
(empty)


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

[PATCH] D56990: Bugfix for Replacement of tied operand of inline asm

[Xiang Zhang via Phabricator via cfe-commits]

xiangzhangllvm added a comment.

!!! Hi, dear efriedma, very sorry! I just saw your reply.
line 2093: getTargetHooks().adjustInlineAsmType(... InputConstraint,...)  will 
just deal with the constrain string,  and it can't check the TiedOperand in the 
function.
So, this will make inconsistent adjust for the operand and its tied operand.
The error will not be found in the IR files, it will cause back end error. like:
"error in backend: Unsupported asm: input constraint with a matching output 
constraint of incompatible type!"

Please refer the adjustInlineAsmType() function, it will call the following 
function

  static llvm::Type* X86AdjustInlineAsmType(CodeGen::CodeGenFunction ,
StringRef Constraint,
llvm::Type* Ty) {
bool IsMMXCons = llvm::StringSwitch(Constraint)
   .Cases("y", "", "^Ym", true)
   .Default(false);
if (IsMMXCons && Ty->isVectorTy()) {
  if (cast(Ty)->getBitWidth() != 64) {
// Invalid MMX constraint
return nullptr;
  }
  return llvm::Type::getX86_MMXTy(CGF.getLLVMContext());
}
// No operation needed
return Ty;
  }




Repository:
  rC Clang

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

https://reviews.llvm.org/D56990



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

r355665 - [runtimes] Move libunwind, libc++abi and libc++ to lib/ and include/

[Petr Hosek via cfe-commits]

Author: phosek
Date: Thu Mar  7 21:35:22 2019
New Revision: 355665

URL: http://llvm.org/viewvc/llvm-project?rev=355665=rev
Log:
[runtimes] Move libunwind, libc++abi and libc++ to lib/ and include/

This change is a consequence of the discussion in "RFC: Place libs in
Clang-dedicated directories", specifically the suggestion that
libunwind, libc++abi and libc++ shouldn't be using Clang resource
directory.  Tools like clangd make this assumption, but this is
currently not true for the LLVM_ENABLE_PER_TARGET_RUNTIME_DIR build.
This change addresses that by moving the output of these libraries to
lib/ and include/ directories, leaving resource directory only
for compiler-rt runtimes and Clang builtin headers.

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

Modified:
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
cfe/trunk/lib/Driver/ToolChains/Linux.cpp
cfe/trunk/test/Driver/linux-per-target-runtime-dir.c

Modified: cfe/trunk/lib/Driver/ToolChain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=355665=355664=355665=diff
==
--- cfe/trunk/lib/Driver/ToolChain.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChain.cpp Thu Mar  7 21:35:22 2019
@@ -75,6 +75,11 @@ ToolChain::ToolChain(const Driver , co
   CachedRTTIMode(CalculateRTTIMode(Args, Triple, CachedRTTIArg)) {
   SmallString<128> P;
 
+  P.assign(D.Dir);
+  llvm::sys::path::append(P, "..", "lib", D.getTargetTriple());
+  if (getVFS().exists(P))
+getLibraryPaths().push_back(P.str());
+
   P.assign(D.ResourceDir);
   llvm::sys::path::append(P, D.getTargetTriple(), "lib");
   if (getVFS().exists(P))

Modified: cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp?rev=355665=355664=355665=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp Thu Mar  7 21:35:22 2019
@@ -257,8 +257,8 @@ void Fuchsia::AddClangCXXStdlibIncludeAr
 
   switch (GetCXXStdlibType(DriverArgs)) {
   case ToolChain::CST_Libcxx: {
-SmallString<128> P(getDriver().ResourceDir);
-llvm::sys::path::append(P, "include", "c++", "v1");
+SmallString<128> P(getDriver().Dir);
+llvm::sys::path::append(P, "..", "include", "c++", "v1");
 addSystemInclude(DriverArgs, CC1Args, P.str());
 break;
   }

Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=355665=355664=355665=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Thu Mar  7 21:35:22 2019
@@ -880,7 +880,6 @@ void Linux::addLibCxxIncludePaths(const
   llvm::opt::ArgStringList ) const {
   const std::string& SysRoot = computeSysRoot();
   const std::string LibCXXIncludePathCandidates[] = {
-  DetectLibcxxIncludePath(getDriver().ResourceDir + "/include/c++"),
   DetectLibcxxIncludePath(getDriver().Dir + "/../include/c++"),
   // If this is a development, non-installed, clang, libcxx will
   // not be found at ../include/c++ but it likely to be found at

Modified: cfe/trunk/test/Driver/linux-per-target-runtime-dir.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/linux-per-target-runtime-dir.c?rev=355665=355664=355665=diff
==
--- cfe/trunk/test/Driver/linux-per-target-runtime-dir.c (original)
+++ cfe/trunk/test/Driver/linux-per-target-runtime-dir.c Thu Mar  7 21:35:22 
2019
@@ -6,12 +6,14 @@
 // RUN: --gcc-toolchain="" \
 // RUN: --sysroot=%S/Inputs/basic_linux_libcxx_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-PER-TARGET-RUNTIME %s
+// CHECK-PER-TARGET-RUNTIME: InstalledDir: [[INSTDIR:.*]]
 // CHECK-PER-TARGET-RUNTIME: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
 // CHECK-PER-TARGET-RUNTIME: "-resource-dir" "[[RESDIR:[^"]*]]"
 // CHECK-PER-TARGET-RUNTIME: "-isysroot" "[[SYSROOT:[^"]+]]"
-// CHECK-PER-TARGET-RUNTIME: "-internal-isystem" "[[RESDIR]]/include/c++/v1"
+// CHECK-PER-TARGET-RUNTIME: "-internal-isystem" 
"[[INSTDIR]]/../include/c++/v1"
 // CHECK-PER-TARGET-RUNTIME: "-internal-isystem" 
"[[SYSROOT]]/usr/local/include"
 // CHECK-PER-TARGET-RUNTIME: "--sysroot=[[SYSROOT]]"
+// CHECK-PER-TARGET-RUNTIME: 
"-L[[INSTDIR]]{{/|}}..{{/|}}lib{{/|}}x86_64-linux-gnu"
 // CHECK-PER-TARGET-RUNTIME: 
"-L[[RESDIR]]{{/|}}x86_64-linux-gnu{{/|}}lib"
 
 // RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \


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

[libunwind] r355665 - [runtimes] Move libunwind, libc++abi and libc++ to lib/ and include/

[Petr Hosek via cfe-commits]

Author: phosek
Date: Thu Mar  7 21:35:22 2019
New Revision: 355665

URL: http://llvm.org/viewvc/llvm-project?rev=355665=rev
Log:
[runtimes] Move libunwind, libc++abi and libc++ to lib/ and include/

This change is a consequence of the discussion in "RFC: Place libs in
Clang-dedicated directories", specifically the suggestion that
libunwind, libc++abi and libc++ shouldn't be using Clang resource
directory.  Tools like clangd make this assumption, but this is
currently not true for the LLVM_ENABLE_PER_TARGET_RUNTIME_DIR build.
This change addresses that by moving the output of these libraries to
lib/ and include/ directories, leaving resource directory only
for compiler-rt runtimes and Clang builtin headers.

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

Modified:
libunwind/trunk/CMakeLists.txt
libunwind/trunk/src/CMakeLists.txt

Modified: libunwind/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/CMakeLists.txt?rev=355665=355664=355665=diff
==
--- libunwind/trunk/CMakeLists.txt (original)
+++ libunwind/trunk/CMakeLists.txt Thu Mar  7 21:35:22 2019
@@ -187,20 +187,25 @@ string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[
${PACKAGE_VERSION})
 
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
-  set(DEFAULT_INSTALL_PREFIX 
lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
-  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}${LIBUNWIND_LIBDIR_SUBDIR})
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR 
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE})
+  if(LIBCXX_LIBDIR_SUBDIR)
+string(APPEND LIBUNWIND_LIBRARY_DIR /${LIBUNWIND_LIBDIR_SUBDIR})
+string(APPEND LIBUNWIND_INSTALL_LIBRARY_DIR /${LIBUNWIND_LIBDIR_SUBDIR})
+  endif()
 elseif(LLVM_LIBRARY_OUTPUT_INTDIR)
   set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LIBUNWIND_LIBDIR_SUFFIX})
 else()
   set(LIBUNWIND_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LIBUNWIND_LIBDIR_SUFFIX})
 endif()
 
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIBUNWIND_LIBRARY_DIR})
 set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LIBUNWIND_LIBRARY_DIR})
 set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LIBUNWIND_LIBRARY_DIR})
 
-set(LIBUNWIND_INSTALL_PREFIX ${DEFAULT_INSTALL_PREFIX} CACHE STRING
-"Define libunwind destination prefix.")
+set(LIBUNWIND_INSTALL_PREFIX "" CACHE STRING "Define libunwind destination 
prefix.")
 
 set(LIBUNWIND_C_FLAGS "")
 set(LIBUNWIND_CXX_FLAGS "")

Modified: libunwind/trunk/src/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/CMakeLists.txt?rev=355665=355664=355665=diff
==
--- libunwind/trunk/src/CMakeLists.txt (original)
+++ libunwind/trunk/src/CMakeLists.txt Thu Mar  7 21:35:22 2019
@@ -178,8 +178,8 @@ add_custom_target(unwind DEPENDS ${LIBUN
 
 if (LIBUNWIND_INSTALL_LIBRARY)
   install(TARGETS ${LIBUNWIND_INSTALL_TARGETS}
-LIBRARY DESTINATION 
${LIBUNWIND_INSTALL_PREFIX}lib${LIBUNWIND_LIBDIR_SUFFIX} COMPONENT unwind
-ARCHIVE DESTINATION 
${LIBUNWIND_INSTALL_PREFIX}lib${LIBUNWIND_LIBDIR_SUFFIX} COMPONENT unwind)
+LIBRARY DESTINATION 
${LIBUNWIND_INSTALL_PREFIX}${LIBUNWIND_INSTALL_LIBRARY_DIR} COMPONENT unwind
+ARCHIVE DESTINATION 
${LIBUNWIND_INSTALL_PREFIX}${LIBUNWIND_INSTALL_LIBRARY_DIR} COMPONENT unwind)
 endif()
 
 if (NOT CMAKE_CONFIGURATION_TYPES AND LIBUNWIND_INSTALL_LIBRARY)


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

[PATCH] D59013: [CMake][runtimes] Move libunwind, libc++abi and libc++ to lib/ and include/

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355665: [runtimes] Move libunwind, libc++abi and libc++ to 
lib/ and include/ (authored by phosek, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59013?vs=189795=189813#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59013

Files:
  cfe/trunk/lib/Driver/ToolChain.cpp
  cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
  cfe/trunk/lib/Driver/ToolChains/Linux.cpp
  cfe/trunk/test/Driver/linux-per-target-runtime-dir.c
  libcxx/trunk/CMakeLists.txt
  libcxx/trunk/lib/CMakeLists.txt
  libcxxabi/trunk/CMakeLists.txt
  libcxxabi/trunk/src/CMakeLists.txt
  libunwind/trunk/CMakeLists.txt
  libunwind/trunk/src/CMakeLists.txt
  llvm/trunk/runtimes/CMakeLists.txt

Index: libcxxabi/trunk/CMakeLists.txt
===
--- libcxxabi/trunk/CMakeLists.txt
+++ libcxxabi/trunk/CMakeLists.txt
@@ -177,16 +177,21 @@
${PACKAGE_VERSION})
 
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
-  set(DEFAULT_INSTALL_PREFIX lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
-  set(LIBCXXABI_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LIBCXXABI_LIBDIR_SUFFIX})
+  set(LIBCXXABI_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE})
+  set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE})
+  if(LIBCXX_LIBDIR_SUBDIR)
+string(APPEND LIBCXXABI_LIBRARY_DIR /${LIBCXXABI_LIBDIR_SUBDIR})
+string(APPEND LIBCXXABI_INSTALL_LIBRARY_DIR /${LIBCXXABI_LIBDIR_SUBDIR})
+  endif()
 elseif(LLVM_LIBRARY_OUTPUT_INTDIR)
   set(LIBCXXABI_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
+  set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LIBCXXABI_LIBDIR_SUFFIX})
 else()
   set(LIBCXXABI_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBCXXABI_LIBDIR_SUFFIX})
+  set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LIBCXXABI_LIBDIR_SUFFIX})
 endif()
 
-set(LIBCXXABI_INSTALL_PREFIX ${DEFAULT_INSTALL_PREFIX} CACHE STRING
-"Define libc++abi destination prefix.")
+set(LIBCXXABI_INSTALL_PREFIX "" CACHE STRING "Define libc++abi destination prefix.")
 
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIBCXXABI_LIBRARY_DIR})
 set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LIBCXXABI_LIBRARY_DIR})
Index: libcxxabi/trunk/src/CMakeLists.txt
===
--- libcxxabi/trunk/src/CMakeLists.txt
+++ libcxxabi/trunk/src/CMakeLists.txt
@@ -252,8 +252,8 @@
 
 if (LIBCXXABI_INSTALL_LIBRARY)
   install(TARGETS ${LIBCXXABI_INSTALL_TARGETS}
-LIBRARY DESTINATION ${LIBCXXABI_INSTALL_PREFIX}lib${LIBCXXABI_LIBDIR_SUFFIX} COMPONENT cxxabi
-ARCHIVE DESTINATION ${LIBCXXABI_INSTALL_PREFIX}lib${LIBCXXABI_LIBDIR_SUFFIX} COMPONENT cxxabi
+LIBRARY DESTINATION ${LIBCXXABI_INSTALL_PREFIX}${LIBCXXABI_INSTALL_LIBRARY_DIR} COMPONENT cxxabi
+ARCHIVE DESTINATION ${LIBCXXABI_INSTALL_PREFIX}${LIBCXXABI_INSTALL_LIBRARY_DIR} COMPONENT cxxabi
 )
 endif()
 
Index: cfe/trunk/test/Driver/linux-per-target-runtime-dir.c
===
--- cfe/trunk/test/Driver/linux-per-target-runtime-dir.c
+++ cfe/trunk/test/Driver/linux-per-target-runtime-dir.c
@@ -6,12 +6,14 @@
 // RUN: --gcc-toolchain="" \
 // RUN: --sysroot=%S/Inputs/basic_linux_libcxx_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-PER-TARGET-RUNTIME %s
+// CHECK-PER-TARGET-RUNTIME: InstalledDir: [[INSTDIR:.*]]
 // CHECK-PER-TARGET-RUNTIME: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
 // CHECK-PER-TARGET-RUNTIME: "-resource-dir" "[[RESDIR:[^"]*]]"
 // CHECK-PER-TARGET-RUNTIME: "-isysroot" "[[SYSROOT:[^"]+]]"
-// CHECK-PER-TARGET-RUNTIME: "-internal-isystem" "[[RESDIR]]/include/c++/v1"
+// CHECK-PER-TARGET-RUNTIME: "-internal-isystem" "[[INSTDIR]]/../include/c++/v1"
 // CHECK-PER-TARGET-RUNTIME: "-internal-isystem" "[[SYSROOT]]/usr/local/include"
 // CHECK-PER-TARGET-RUNTIME: "--sysroot=[[SYSROOT]]"
+// CHECK-PER-TARGET-RUNTIME: "-L[[INSTDIR]]{{/|}}..{{/|}}lib{{/|}}x86_64-linux-gnu"
 // CHECK-PER-TARGET-RUNTIME: "-L[[RESDIR]]{{/|}}x86_64-linux-gnu{{/|}}lib"
 
 // RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \
Index: cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
@@ -257,8 +257,8 @@
 
   switch (GetCXXStdlibType(DriverArgs)) {
   case ToolChain::CST_Libcxx: {
-SmallString<128> P(getDriver().ResourceDir);
-llvm::sys::path::append(P, "include", "c++", "v1");
+SmallString<128> P(getDriver().Dir);
+llvm::sys::path::append(P, "..", "include", "c++", "v1");
 addSystemInclude(DriverArgs, CC1Args, P.str());
 break;
   }
Index: cfe/trunk/lib/Driver/ToolChains/Linux.cpp

r355664 - Fix test case committed in r355662.

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Thu Mar  7 21:30:54 2019
New Revision: 355664

URL: http://llvm.org/viewvc/llvm-project?rev=355664=rev
Log:
Fix test case committed in r355662.

Build bots were failing because wide string literals don't have type
'int *' on some targets.

Modified:
cfe/trunk/test/SemaObjC/boxing-illegal.m

Modified: cfe/trunk/test/SemaObjC/boxing-illegal.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/boxing-illegal.m?rev=355664=355663=355664=diff
==
--- cfe/trunk/test/SemaObjC/boxing-illegal.m (original)
+++ cfe/trunk/test/SemaObjC/boxing-illegal.m Thu Mar  7 21:30:54 2019
@@ -66,7 +66,7 @@ void testStringLiteral() {
   s = @(u8"abc");
   s = @(u"abc"); // expected-error {{illegal type 'unsigned short *' used in a 
boxed expression}}
   s = @(U"abc"); // expected-error {{illegal type 'unsigned int *' used in a 
boxed expression}}
-  s = @(L"abc"); // expected-error {{illegal type 'int *' used in a boxed 
expression}}
+  s = @(L"abc"); // expected-error-re {{illegal type {{'int \*'|'unsigned 
short \*'}} used in a boxed expression}}
   s = @("\pabc"); // expected-error {{illegal type 'unsigned char *' used in a 
boxed expression}}
 }
 


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

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[Akira Hatanaka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355662: [ObjC] Emit a boxed expression as a compile-time 
constant if the (authored by ahatanak, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58729?vs=189809=189811#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58729

Files:
  cfe/trunk/include/clang/AST/ExprObjC.h
  cfe/trunk/include/clang/Basic/DiagnosticGroups.td
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/AST/ExprConstant.cpp
  cfe/trunk/lib/CodeGen/CGExprConstant.cpp
  cfe/trunk/lib/CodeGen/CGObjC.cpp
  cfe/trunk/lib/Sema/SemaExprObjC.cpp
  cfe/trunk/test/CodeGenObjC/boxing.m
  cfe/trunk/test/SemaObjC/boxing-illegal.m
  cfe/trunk/test/SemaObjC/objc-literal-sig.m
  cfe/trunk/test/SemaObjC/transfer-boxed-string-nullability.m
  cfe/trunk/test/SemaObjCXX/literals.mm

Index: cfe/trunk/include/clang/AST/ExprObjC.h
===
--- cfe/trunk/include/clang/AST/ExprObjC.h
+++ cfe/trunk/include/clang/AST/ExprObjC.h
@@ -138,6 +138,12 @@
 return BoxingMethod;
   }
 
+  // Indicates whether this boxed expression can be emitted as a compile-time
+  // constant.
+  bool isExpressibleAsConstantInitializer() const {
+return !BoxingMethod && SubExpr;
+  }
+
   SourceLocation getAtLoc() const { return Range.getBegin(); }
 
   SourceLocation getBeginLoc() const LLVM_READONLY { return Range.getBegin(); }
Index: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td
@@ -404,6 +404,7 @@
 def ObjCPointerIntrospect : DiagGroup<"deprecated-objc-pointer-introspection", [ObjCPointerIntrospectPerformSelector]>;
 def ObjCMultipleMethodNames : DiagGroup<"objc-multiple-method-names">;
 def ObjCFlexibleArray : DiagGroup<"objc-flexible-array">;
+def ObjCBoxing : DiagGroup<"objc-boxing">;
 def OpenCLUnsupportedRGBA: DiagGroup<"opencl-unsupported-rgba">;
 def DeprecatedObjCIsaUsage : DiagGroup<"deprecated-objc-isa-usage">;
 def ExplicitInitializeCall : DiagGroup<"explicit-initialize-call">;
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -918,6 +918,9 @@
   "inconsistent number of instance variables specified">;
 def warn_undef_method_impl : Warning<"method definition for %0 not found">,
   InGroup>;
+def warn_objc_boxing_invalid_utf8_string : Warning<
+  "string is ill-formed as UTF-8 and will become a null %0 when boxed">,
+  InGroup;
 
 def warn_conflicting_overriding_ret_types : Warning<
   "conflicting return type in "
Index: cfe/trunk/test/SemaObjC/boxing-illegal.m
===
--- cfe/trunk/test/SemaObjC/boxing-illegal.m
+++ cfe/trunk/test/SemaObjC/boxing-illegal.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wattributes %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wattributes -fpascal-strings %s
 
 typedef long NSInteger;
 typedef unsigned long NSUInteger;
@@ -57,6 +57,19 @@
   box = @(*(enum ForwE*)p); // expected-error {{incomplete type 'enum ForwE' used in a boxed expression}}
 }
 
+@interface NSString
+@end
+
+void testStringLiteral() {
+  NSString *s;
+  s = @("abc");
+  s = @(u8"abc");
+  s = @(u"abc"); // expected-error {{illegal type 'unsigned short *' used in a boxed expression}}
+  s = @(U"abc"); // expected-error {{illegal type 'unsigned int *' used in a boxed expression}}
+  s = @(L"abc"); // expected-error {{illegal type 'int *' used in a boxed expression}}
+  s = @("\pabc"); // expected-error {{illegal type 'unsigned char *' used in a boxed expression}}
+}
+
 // rdar://1205
 @class NSMutableDictionary;
 
Index: cfe/trunk/test/SemaObjC/objc-literal-sig.m
===
--- cfe/trunk/test/SemaObjC/objc-literal-sig.m
+++ cfe/trunk/test/SemaObjC/objc-literal-sig.m
@@ -39,6 +39,8 @@
 
 // All tests are doubled to make sure that a bad method is not saved
 // and then used un-checked.
+const char *getStr(void);
+
 void test_sig() {
   (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
   (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
@@ -46,6 +48,6 @@
   id array2 = @[ @17 ]; // expected-error{{literal construction method 'arrayWithObjects:count:' has incompatible signature}}
   id dict = @{ @"hello" : @17 }; // expected-error{{literal construction method 'dictionaryWithObjects:forKeys:count:' has incompatible 

r355662 - [ObjC] Emit a boxed expression as a compile-time constant if the

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Thu Mar  7 20:45:37 2019
New Revision: 355662

URL: http://llvm.org/viewvc/llvm-project?rev=355662=rev
Log:
[ObjC] Emit a boxed expression as a compile-time constant if the
expression inside the parentheses is a valid UTF-8 string literal.

Previously clang emitted an expression like @("abc") as a message send
to stringWithUTF8String. This commit makes clang emit the boxed
expression as a compile-time constant instead.

This commit also has the effect of silencing the nullable-to-nonnull
conversion warning clang started emitting after r317727, which
originally motivated this commit (see https://oleb.net/2018/@keypath).

rdar://problem/42684601

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

Modified:
cfe/trunk/include/clang/AST/ExprObjC.h
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/CodeGen/CGExprConstant.cpp
cfe/trunk/lib/CodeGen/CGObjC.cpp
cfe/trunk/lib/Sema/SemaExprObjC.cpp
cfe/trunk/test/CodeGenObjC/boxing.m
cfe/trunk/test/SemaObjC/boxing-illegal.m
cfe/trunk/test/SemaObjC/objc-literal-sig.m
cfe/trunk/test/SemaObjC/transfer-boxed-string-nullability.m
cfe/trunk/test/SemaObjCXX/literals.mm

Modified: cfe/trunk/include/clang/AST/ExprObjC.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprObjC.h?rev=355662=355661=355662=diff
==
--- cfe/trunk/include/clang/AST/ExprObjC.h (original)
+++ cfe/trunk/include/clang/AST/ExprObjC.h Thu Mar  7 20:45:37 2019
@@ -138,6 +138,12 @@ public:
 return BoxingMethod;
   }
 
+  // Indicates whether this boxed expression can be emitted as a compile-time
+  // constant.
+  bool isExpressibleAsConstantInitializer() const {
+return !BoxingMethod && SubExpr;
+  }
+
   SourceLocation getAtLoc() const { return Range.getBegin(); }
 
   SourceLocation getBeginLoc() const LLVM_READONLY { return Range.getBegin(); }

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=355662=355661=355662=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Thu Mar  7 20:45:37 2019
@@ -404,6 +404,7 @@ def ObjCPointerIntrospectPerformSelector
 def ObjCPointerIntrospect : DiagGroup<"deprecated-objc-pointer-introspection", 
[ObjCPointerIntrospectPerformSelector]>;
 def ObjCMultipleMethodNames : DiagGroup<"objc-multiple-method-names">;
 def ObjCFlexibleArray : DiagGroup<"objc-flexible-array">;
+def ObjCBoxing : DiagGroup<"objc-boxing">;
 def OpenCLUnsupportedRGBA: DiagGroup<"opencl-unsupported-rgba">;
 def DeprecatedObjCIsaUsage : DiagGroup<"deprecated-objc-isa-usage">;
 def ExplicitInitializeCall : DiagGroup<"explicit-initialize-call">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=355662=355661=355662=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Mar  7 20:45:37 
2019
@@ -918,6 +918,9 @@ def err_inconsistent_ivar_count : Error<
   "inconsistent number of instance variables specified">;
 def warn_undef_method_impl : Warning<"method definition for %0 not found">,
   InGroup>;
+def warn_objc_boxing_invalid_utf8_string : Warning<
+  "string is ill-formed as UTF-8 and will become a null %0 when boxed">,
+  InGroup;
 
 def warn_conflicting_overriding_ret_types : Warning<
   "conflicting return type in "

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=355662=355661=355662=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Thu Mar  7 20:45:37 2019
@@ -1743,6 +1743,8 @@ static bool IsGlobalLValue(APValue::LVal
   case Expr::CXXTypeidExprClass:
   case Expr::CXXUuidofExprClass:
 return true;
+  case Expr::ObjCBoxedExprClass:
+return cast(E)->isExpressibleAsConstantInitializer();
   case Expr::CallExprClass:
 return IsStringLiteralCall(cast(E));
   // For GCC compatibility, & has static storage duration.
@@ -5794,6 +5796,8 @@ public:
   bool VisitObjCStringLiteral(const ObjCStringLiteral *E)
   { return Success(E); }
   bool VisitObjCBoxedExpr(const ObjCBoxedExpr *E) {
+if (E->isExpressibleAsConstantInitializer())
+  return Success(E);
 if (Info.noteFailure())
   EvaluateIgnoredValue(Info, E->getSubExpr());
 return Error(E);

Modified: 

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[Akira Hatanaka via Phabricator via cfe-commits]

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



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:922
+def warn_objc_boxing_invalid_utf8_string : Warning<
+  "string is ill-formed as UTF-8 and will become a null NSString* when boxed">,
+  InGroup;

rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > Might as well use the `NSStringPointer` type we stash on Sema so that 
> > > this will be the right type even in more obscure environments.
> > I wasn't exactly sure when `NSStringPointer` can be anything other than 
> > `NSString *`.
> Hmm.  It's possible that it can't be for boxing, but I'm pretty sure it can 
> be overridden for string literals, and it's not ridiculous that we'd allow 
> that to be changed for arbitrary boxing as well, so this is good idea 
> regardless.
I agree, it's probably a good idea.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729



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

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[John McCall via Phabricator via cfe-commits]

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



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:922
+def warn_objc_boxing_invalid_utf8_string : Warning<
+  "string is ill-formed as UTF-8 and will become a null NSString* when boxed">,
+  InGroup;

ahatanak wrote:
> rjmccall wrote:
> > Might as well use the `NSStringPointer` type we stash on Sema so that this 
> > will be the right type even in more obscure environments.
> I wasn't exactly sure when `NSStringPointer` can be anything other than 
> `NSString *`.
Hmm.  It's possible that it can't be for boxing, but I'm pretty sure it can be 
overridden for string literals, and it's not ridiculous that we'd allow that to 
be changed for arbitrary boxing as well, so this is good idea regardless.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729



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

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:922
+def warn_objc_boxing_invalid_utf8_string : Warning<
+  "string is ill-formed as UTF-8 and will become a null NSString* when boxed">,
+  InGroup;

rjmccall wrote:
> Might as well use the `NSStringPointer` type we stash on Sema so that this 
> will be the right type even in more obscure environments.
I wasn't exactly sure when `NSStringPointer` can be anything other than 
`NSString *`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729



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

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 189809.
ahatanak marked an inline comment as done.
ahatanak added a comment.

Use the type of `NSStringPointer` in the diagnostic string.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729

Files:
  include/clang/AST/ExprObjC.h
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGObjC.cpp
  lib/Sema/SemaExprObjC.cpp
  test/CodeGenObjC/boxing.m
  test/SemaObjC/boxing-illegal.m
  test/SemaObjC/objc-literal-sig.m
  test/SemaObjC/transfer-boxed-string-nullability.m
  test/SemaObjCXX/literals.mm

Index: test/SemaObjCXX/literals.mm
===
--- test/SemaObjCXX/literals.mm
+++ test/SemaObjCXX/literals.mm
@@ -50,6 +50,9 @@
 + (id)dictionaryWithObjects:(const id [])objects forKeys:(const id [])keys count:(unsigned long)cnt;
 @end
 
+@interface NSString
+@end
+
 template
 struct ConvertibleTo {
   operator T();
@@ -185,3 +188,8 @@
 void test_dictionary_colon() {
   id dict = @{ key : value };
 }
+
+void testConstExpr() {
+  constexpr NSString *t0 = @"abc";
+  constexpr NSString *t1 = @("abc");
+}
Index: test/SemaObjC/transfer-boxed-string-nullability.m
===
--- test/SemaObjC/transfer-boxed-string-nullability.m
+++ test/SemaObjC/transfer-boxed-string-nullability.m
@@ -16,13 +16,23 @@
 void takesNonNull(NSString * _Nonnull ptr);
 
 void testBoxedString() {
+  // No diagnostic emitted as this doesn't need a stringWithUTF8String message
+  // send.
+  takesNonNull(@("hey"));
+  takesNonNull(@(u8"hey"));
+
+  // If the string isn't a valid UTF-8 string, a diagnostic is emitted since the
+  // boxed expression turns into a message send.
+  takesNonNull(@(u8"\xFF")); // expected-warning {{string is ill-formed as UTF-8}}
+  takesNonNull(@(u8"\xC0\x80")); // expected-warning {{string is ill-formed as UTF-8}}
+
   const char *str = "hey";
   takesNonNull([NSString stringWithUTF8String:str]);
   takesNonNull(@(str));
 #ifndef NOWARN
-  // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
-  // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
-#else
-  // expected-no-diagnostics
+  // expected-warning@-7 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-7 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-5 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-5 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
 #endif
 }
Index: test/SemaObjC/objc-literal-sig.m
===
--- test/SemaObjC/objc-literal-sig.m
+++ test/SemaObjC/objc-literal-sig.m
@@ -39,6 +39,8 @@
 
 // All tests are doubled to make sure that a bad method is not saved
 // and then used un-checked.
+const char *getStr(void);
+
 void test_sig() {
   (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
   (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
@@ -46,6 +48,6 @@
   id array2 = @[ @17 ]; // expected-error{{literal construction method 'arrayWithObjects:count:' has incompatible signature}}
   id dict = @{ @"hello" : @17 }; // expected-error{{literal construction method 'dictionaryWithObjects:forKeys:count:' has incompatible signature}}
   id dict2 = @{ @"hello" : @17 }; // expected-error{{literal construction method 'dictionaryWithObjects:forKeys:count:' has incompatible signature}}
-  id str = @("hello"); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
-  id str2 = @("hello"); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
+  id str = @(getStr()); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
+  id str2 = @(getStr()); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
 }
Index: test/SemaObjC/boxing-illegal.m
===
--- test/SemaObjC/boxing-illegal.m
+++ test/SemaObjC/boxing-illegal.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wattributes %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wattributes -fpascal-strings %s
 
 typedef long NSInteger;
 

[PATCH] D57850: [analyzer] Emit an error rather than assert on invalid checker option input

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> Would you mind me committing as is?

Np, please do, the patch is great regardless!




Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:352-355
+  if (Checker->AllowedPad < 0)
+
Mgr.getDiagnostics().Report(diag::err_analyzer_checker_option_invalid_input)
+<< (llvm::Twine() + Checker->getTagDescription() + ":AllowedPad").str()
+<< "a non-negative";

Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > NoQ wrote:
> > > > 
> > > > I passively wish for a certain amount of de-duplication that wouldn't 
> > > > require every checker to obtain a diagnostics engine every time it 
> > > > tries to read an option. Eg.,
> > > > ```lang=c++
> > > >   auto *Checker = Mgr.registerChecker();
> > > >   Checker->AllowedPad = Mgr.getAnalyzerOptions()
> > > >   .getCheckerIntegerOption(Checker, "AllowedPad", 24);
> > > >   if (Checker->AllowedPad < 0)
> > > > Mgr.reportInvalidOptionValue(Checker, "AllowedPad", "a 
> > > > non-negative");
> > > > ```
> > > > 
> > > > Or maybe even something like that:
> > > > 
> > > > ```lang=c++
> > > >   auto *Checker = Mgr.registerChecker();
> > > >   Checker->AllowedPad = Mgr.getAnalyzerOptions()
> > > >   .getCheckerIntegerOption(Checker, "AllowedPad", 24,
> > > >   [](int x) -> Option {
> > > >   if (x < 0) {
> > > > // Makes getCheckerIntegerOption() emit a 
> > > > diagnostic
> > > > // and return the default value.
> > > > return "a non-negative";
> > > >   }
> > > >   // Makes getCheckerIntegerOption() successfully 
> > > > return
> > > >   // the user-specified value.
> > > >   return None;
> > > >   });
> > > > ```
> > > > I.e., a validator lambda.
> > > First one, sure. I'm a little unsure about the second: No other 
> > > "options"-like classes have access to a `DiagnosticsEngine` in clang, as 
> > > far as I'm aware, and I guess keeping `AnalyzerOptions` as simple is 
> > > possible is preferred. Not only that, but a validator lambda seems an to 
> > > be an overkill (though really-really cool) solution. Your first bit of 
> > > code is far more readable IMO.
> > Hmm, in the first example we'll also have to manually reset the option to 
> > the default value if it is invalid, which is also annoying - even if easy 
> > to understand, it's also easy to forget.
> > 
> > And with that and a bit of polish, the lambda approach isn't really much 
> > more verbose, and definitely involves less duplication:
> > 
> > ```lang=c++
> > auto *Checker = Mgr.registerChecker();
> > Checker->AllowedPad = Mgr.getAnalyzerOptions()
> > .getCheckerIntegerOption(Checker, "AllowedPad", 24);
> > if (Checker->AllowedPad < 0) {
> >   Mgr.reportInvalidOptionValue(Checker, "AllowedPad", "a non-negative 
> > value");
> >   Checker->AllowedPad = 24;
> > }
> > ```
> > vs.
> > ```lang=c++
> > auto *Checker = Mgr.registerChecker();
> > Checker->AllowedPad = Mgr.getAnalyzerOptions()
> > .getCheckerIntegerOption(Checker, "AllowedPad", /*Default=*/ 24,
> >  /*Validate=*/ [](int x) { return x >= 0; },
> >  /*ValidMsg=*/ "a non-negative value");
> > ```
> Alright, so I've given this a lot of thought, here's where I'm standing on 
> the issue:
> 
> * I would prefer not to add `DiagnosticsEngine` to `AnalyzerOptions`. In 
> fact, I'd prefer not to add it even as a parameter to one of it's methods -- 
> designwise, it should be a simple mapping of the command line parameters, not 
> doing any complicated hackery.
> * You got me convinced the validator lambda thing ;). However, a nice 
> implementation of this (emphasis on //nice//) is most definitely a bigger 
> undertaking.
> * Once we're at the topic of "easy to forget", we could also verify 
> compile-time whether checker options are actually used -- what I'm thinking 
> here, is something like this:
> 
> ```
> auto *Checker = Mgr.registerChecker();
> Mgr.initFieldWithOption(Checker, "AllowedPad",
> // Note that we should be able
> // to know the default value.
> Checker->AllowedPad,
> // We could make this optional by defining a
> // default validator...
> /*Validate=*/ [](int x) { return x >= 0; },
> // ...nd a default error message.
> /*ValidMsg=*/ "a non-negative value");
> ```
> `CheckerManager` later (once all checker registry functions finished) could 
> validate, with the help of `CheckerRegistry`, whether
> * All options for a given checker were queried for,
> * The supplied checker options is valid, if not, restore them in 
> compatibility mode, emit an error otherwise,
> * No 

[PATCH] D58880: [WIP] [Looking for API feedback] [clangd] Type hierarchy subtypes

[Nathan Ridge via Phabricator via cfe-commits]

nridge planned changes to this revision.
nridge added a comment.

Thanks for the detailed analysis!

For completeness, I think another advantage of option 3 is that it allows us to 
make the key for relation lookups be a (SymbolID, RelationKind) pair rather 
than just a SymbolID.

It sounds like I should continue implementing the current approach. Marking the 
revision as "changes planned" accordingly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58880



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

[PATCH] D59123: [analyzer] RetainCount: Fix a crash when a function follows retain/autorelease naming convention but takes no arguments.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

The actual crash happens in `RetainCountChecker::evalCall()`:

  912 const Expr *BindReturnTo =
  913 (BSmr == BehaviorSummary::IdentityThis)
  914 ? cast(CE)->getImplicitObjectArgument()
  915 : CE->getArg(0); // <== here


Repository:
  rC Clang

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

https://reviews.llvm.org/D59123



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

[PATCH] D59123: [analyzer] RetainCount: Fix a crash when a function follows retain/autorelease naming convention but takes no arguments.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added a reviewer: dcoughlin.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

We desperately need a checker that finds unchecked `getArg()` calls :)


Repository:
  rC Clang

https://reviews.llvm.org/D59123

Files:
  clang/lib/Analysis/RetainSummaryManager.cpp
  clang/test/Analysis/retain-release.mm


Index: clang/test/Analysis/retain-release.mm
===
--- clang/test/Analysis/retain-release.mm
+++ clang/test/Analysis/retain-release.mm
@@ -503,3 +503,15 @@
 }
 
 }
+
+namespace yet_another_unexpected_signature_crash {
+
+CFTypeRef CFSomethingSomethingRetain();
+CFTypeRef CFSomethingSomethingAutorelease();
+
+void foo() {
+  CFSomethingSomethingRetain(); // no-crash
+  CFSomethingSomethingAutorelease(); // no-crash
+}
+
+}
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -722,12 +722,13 @@
   // These are not retain. They just return something and retain it.
   return None;
 }
-if (cocoa::isRefType(ResultTy, "CF", FName) ||
-cocoa::isRefType(ResultTy, "CG", FName) ||
-cocoa::isRefType(ResultTy, "CV", FName))
-  if (isRetain(FD, FName) || isAutorelease(FD, FName) ||
-  isMakeCollectable(FName))
-return BehaviorSummary::Identity;
+if (CE->getNumArgs() == 1 &&
+(cocoa::isRefType(ResultTy, "CF", FName) ||
+ cocoa::isRefType(ResultTy, "CG", FName) ||
+ cocoa::isRefType(ResultTy, "CV", FName)) &&
+(isRetain(FD, FName) || isAutorelease(FD, FName) ||
+ isMakeCollectable(FName)))
+  return BehaviorSummary::Identity;
 
 // safeMetaCast is called by OSDynamicCast.
 // We assume that OSDynamicCast is either an identity (cast is OK,


Index: clang/test/Analysis/retain-release.mm
===
--- clang/test/Analysis/retain-release.mm
+++ clang/test/Analysis/retain-release.mm
@@ -503,3 +503,15 @@
 }
 
 }
+
+namespace yet_another_unexpected_signature_crash {
+
+CFTypeRef CFSomethingSomethingRetain();
+CFTypeRef CFSomethingSomethingAutorelease();
+
+void foo() {
+  CFSomethingSomethingRetain(); // no-crash
+  CFSomethingSomethingAutorelease(); // no-crash
+}
+
+}
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -722,12 +722,13 @@
   // These are not retain. They just return something and retain it.
   return None;
 }
-if (cocoa::isRefType(ResultTy, "CF", FName) ||
-cocoa::isRefType(ResultTy, "CG", FName) ||
-cocoa::isRefType(ResultTy, "CV", FName))
-  if (isRetain(FD, FName) || isAutorelease(FD, FName) ||
-  isMakeCollectable(FName))
-return BehaviorSummary::Identity;
+if (CE->getNumArgs() == 1 &&
+(cocoa::isRefType(ResultTy, "CF", FName) ||
+ cocoa::isRefType(ResultTy, "CG", FName) ||
+ cocoa::isRefType(ResultTy, "CV", FName)) &&
+(isRetain(FD, FName) || isAutorelease(FD, FName) ||
+ isMakeCollectable(FName)))
+  return BehaviorSummary::Identity;
 
 // safeMetaCast is called by OSDynamicCast.
 // We assume that OSDynamicCast is either an identity (cast is OK,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58878: [Diagnostics] Warn for assignments in bool contexts

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/Basic/DiagnosticGroups.td:699
 def ParenthesesOnEquality : DiagGroup<"parentheses-equality">;
+def AssigmentInBoolContext : DiagGroup<"assigment-in-bool-context">;
 def Parentheses : DiagGroup<"parentheses",

"assignment", in both places



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6738
+  "assignment to bool">,
+  InGroup;
 // Completely identical except off by default.

Sorry, I didn't mean to suggest that you should change the existing diagnostic. 
 You should add a second diagnostic in a new warning group (which should be 
implied by `-Wparentheses`) that you use just in this case.

We try to add new warnings this way — even when they're just generalizations of 
existing warnings — so that they can be independently disabled by e.g. the 
compiler teams at Apple and Google that need to periodically roll out new 
compilers across a large codebase.


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

https://reviews.llvm.org/D58878



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

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/Basic/DiagnosticGroups.td:407
 def ObjCFlexibleArray : DiagGroup<"objc-flexible-array">;
+def ObjCBoxing : DiagGroup<"objc-boxing">;
 def OpenCLUnsupportedRGBA: DiagGroup<"opencl-unsupported-rgba">;

Sure.  If we decide to add more things to this warning group, we can pull this 
one out into a sub-group with a more targeted name.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:922
+def warn_objc_boxing_invalid_utf8_string : Warning<
+  "string is ill-formed as UTF-8 and will become a null NSString* when boxed">,
+  InGroup;

Might as well use the `NSStringPointer` type we stash on Sema so that this will 
be the right type even in more obscure environments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729



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

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

[Artem Dergachev via Phabricator via cfe-commits]

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

On the newly included test case the previous behavior was

  Line 53: note: Assuming 'i' is equal to nested_null_split

which is meh.

I tried to make this piece of logic slightly more correct, but it's most likely 
still completely incorrect.


Repository:
  rC Clang

https://reviews.llvm.org/D59121

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/macros.cpp


Index: clang/test/Analysis/diagnostics/macros.cpp
===
--- clang/test/Analysis/diagnostics/macros.cpp
+++ clang/test/Analysis/diagnostics/macros.cpp
@@ -46,3 +46,14 @@
 // expected-note@-1 {{Dereference of null pointer (loaded 
from variable 'p')}}
   }
 }
+
+#define nested_null_split(x) if ((x) != UINT32_MAX) {}
+
+void testNestedNullSplitMacro(int i, int *p) {
+  nested_null_split(i); // expected-note {{Assuming 'i' is equal to 
UINT32_MAX}}
+// expected-note@-1 {{Taking false branch}}
+  if (!p) // expected-note {{Assuming 'p' is null}}
+  // expected-note@-1 {{Taking true branch}}
+*p = 1; // expected-warning {{Dereference of null pointer (loaded from 
variable 'p')}}
+// expected-note@-1 {{Dereference of null pointer (loaded from 
variable 'p')}}
+}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1982,8 +1982,9 @@
 bool beginAndEndAreTheSameMacro = StartName.equals(EndName);
 
 bool partOfParentMacro = false;
+StringRef PName = "";
 if (ParentEx->getBeginLoc().isMacroID()) {
-  StringRef PName = Lexer::getImmediateMacroNameForDiagnostics(
+  PName = Lexer::getImmediateMacroNameForDiagnostics(
   ParentEx->getBeginLoc(), BRC.getSourceManager(),
   BRC.getASTContext().getLangOpts());
   partOfParentMacro = PName.equals(StartName);
@@ -1991,13 +1992,20 @@
 
 if (beginAndEndAreTheSameMacro && !partOfParentMacro ) {
   // Get the location of the macro name as written by the caller.
-  SourceLocation Loc = LocStart;
-  while (LocStart.isMacroID()) {
-Loc = LocStart;
+  StringRef MacroName = StartName;
+  while (true) {
 LocStart = BRC.getSourceManager().getImmediateMacroCallerLoc(LocStart);
+if (!LocStart.isMacroID())
+  break;
+
+StringRef NextMacroName = Lexer::getImmediateMacroNameForDiagnostics(
+LocStart, BRC.getSourceManager(),
+BRC.getASTContext().getLangOpts());
+if (NextMacroName == PName)
+  break;
+
+MacroName = NextMacroName;
   }
-  StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
-Loc, BRC.getSourceManager(), BRC.getASTContext().getLangOpts());
 
   // Return the macro name.
   Out << MacroName;


Index: clang/test/Analysis/diagnostics/macros.cpp
===
--- clang/test/Analysis/diagnostics/macros.cpp
+++ clang/test/Analysis/diagnostics/macros.cpp
@@ -46,3 +46,14 @@
 // expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
   }
 }
+
+#define nested_null_split(x) if ((x) != UINT32_MAX) {}
+
+void testNestedNullSplitMacro(int i, int *p) {
+  nested_null_split(i); // expected-note {{Assuming 'i' is equal to UINT32_MAX}}
+// expected-note@-1 {{Taking false branch}}
+  if (!p) // expected-note {{Assuming 'p' is null}}
+  // expected-note@-1 {{Taking true branch}}
+*p = 1; // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+// expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
+}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1982,8 +1982,9 @@
 bool beginAndEndAreTheSameMacro = StartName.equals(EndName);
 
 bool partOfParentMacro = false;
+StringRef PName = "";
 if (ParentEx->getBeginLoc().isMacroID()) {
-  StringRef PName = Lexer::getImmediateMacroNameForDiagnostics(
+  PName = Lexer::getImmediateMacroNameForDiagnostics(
   ParentEx->getBeginLoc(), BRC.getSourceManager(),
   BRC.getASTContext().getLangOpts());
   partOfParentMacro = PName.equals(StartName);
@@ -1991,13 +1992,20 @@
 
 if (beginAndEndAreTheSameMacro && !partOfParentMacro ) {
   // Get the 

[PATCH] D58885: Variable auto-init: split out small arrays

[JF Bastien via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355660: Variable auto-init: split out small arrays (authored 
by jfb, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58885

Files:
  cfe/trunk/lib/CodeGen/CGDecl.cpp
  cfe/trunk/test/CodeGenCXX/auto-var-init.cpp

Index: cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
===
--- cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
+++ cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
@@ -129,7 +129,6 @@
 // PATTERN-O1-NOT: @__const.test_bool4_custom.custom
 // ZERO-O1-NOT: @__const.test_bool4_custom.custom
 
-// PATTERN: @__const.test_intptr4_uninit.uninit = private unnamed_addr constant [4 x i32*] [i32* inttoptr (i64 -6148914691236517206 to i32*), i32* inttoptr (i64 -6148914691236517206 to i32*), i32* inttoptr (i64 -6148914691236517206 to i32*), i32* inttoptr (i64 -6148914691236517206 to i32*)], align 16
 // PATTERN: @__const.test_intptr4_custom.custom = private unnamed_addr constant [4 x i32*] [i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*)], align 16
 // ZERO: @__const.test_intptr4_custom.custom = private unnamed_addr constant [4 x i32*] [i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*)], align 16
 // PATTERN-O0: @__const.test_tailpad4_uninit.uninit = private unnamed_addr constant [4 x { i16, i8, [1 x i8] }] [{ i16, i8, [1 x i8] } { i16 -21846, i8 -86, [1 x i8] c"\AA" }, { i16, i8, [1 x i8] } { i16 -21846, i8 -86, [1 x i8] c"\AA" }, { i16, i8, [1 x i8] } { i16 -21846, i8 -86, [1 x i8] c"\AA" }, { i16, i8, [1 x i8] } { i16 -21846, i8 -86, [1 x i8] c"\AA" }], align 16
@@ -1019,13 +1018,20 @@
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%custom)
 
 TEST_UNINIT(intptr4, int*[4]);
-// CHECK-LABEL: @test_intptr4_uninit()
-// CHECK:   %uninit = alloca [4 x i32*], align
-// CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
-// PATTERN-LABEL: @test_intptr4_uninit()
-// PATTERN: call void @llvm.memcpy{{.*}} @__const.test_intptr4_uninit.uninit
-// ZERO-LABEL: @test_intptr4_uninit()
-// ZERO: call void @llvm.memset{{.*}}, i8 0,
+// CHECK-LABEL:  @test_intptr4_uninit()
+// CHECK:%uninit = alloca [4 x i32*], align
+// CHECK-NEXT:   call void @{{.*}}used{{.*}}%uninit)
+// PATTERN-O1-LABEL: @test_intptr4_uninit()
+// PATTERN-O1:   %1 = getelementptr inbounds [4 x i32*], [4 x i32*]* %uninit, i64 0, i64 0
+// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), i32** %1, align 16
+// PATTERN-O1-NEXT:  %2 = getelementptr inbounds [4 x i32*], [4 x i32*]* %uninit, i64 0, i64 1
+// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), i32** %2, align 8
+// PATTERN-O1-NEXT:  %3 = getelementptr inbounds [4 x i32*], [4 x i32*]* %uninit, i64 0, i64 2
+// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), i32** %3, align 16
+// PATTERN-O1-NEXT:  %4 = getelementptr inbounds [4 x i32*], [4 x i32*]* %uninit, i64 0, i64 3
+// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), i32** %4, align 8
+// ZERO-LABEL:   @test_intptr4_uninit()
+// ZERO: call void @llvm.memset{{.*}}, i8 0,
 
 TEST_BRACES(intptr4, int*[4]);
 // CHECK-LABEL: @test_intptr4_braces()
@@ -1124,7 +1130,7 @@
 // PATTERN-LABEL: @test_atomicnotlockfree_uninit()
 // PATTERN-O0: call void @llvm.memcpy{{.*}} @__const.test_atomicnotlockfree_uninit.uninit
 // PATTERN-O1: bitcast
-// PATTERN-O1: call void @llvm.memset{{.*}}({{.*}}0, i8 -86, i64 32
+// PATTERN-O1: call void @llvm.memset{{.*}}({{.*}}, i8 -86, i64 32
 // ZERO-LABEL: @test_atomicnotlockfree_uninit()
 // ZERO: call void @llvm.memset{{.*}}, i8 0,
 
Index: cfe/trunk/lib/CodeGen/CGDecl.cpp
===
--- cfe/trunk/lib/CodeGen/CGDecl.cpp
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp
@@ -970,12 +970,12 @@
   return llvm::isBytewiseValue(Init);
 }
 
-/// Decide whether we want to split a constant structure store into a sequence
-/// of its fields' stores. This may cost us code size and compilation speed,
-/// but plays better with store optimizations.
-static bool shouldSplitStructStore(CodeGenModule ,
-   uint64_t GlobalByteSize) {
-  // Don't break structures that occupy more than one cacheline.
+/// Decide whether we want to split a constant structure or array store into a
+/// sequence of its fields' stores. This may cost us code size and compilation
+/// speed, but plays better with store optimizations.
+static bool shouldSplitConstantStore(CodeGenModule ,
+ uint64_t 

r355660 - Variable auto-init: split out small arrays

[JF Bastien via cfe-commits]

Author: jfb
Date: Thu Mar  7 17:26:49 2019
New Revision: 355660

URL: http://llvm.org/viewvc/llvm-project?rev=355660=rev
Log:
Variable auto-init: split out small arrays

Summary: Following up with r355181, initialize small arrays as well.

LLVM stage2 shows a tiny size gain.



Reviewers: glider, pcc, kcc, rjmccall

Subscribers: jkorous, dexonsmith, jdoerfert, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/test/CodeGenCXX/auto-var-init.cpp

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=355660=355659=355660=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Thu Mar  7 17:26:49 2019
@@ -970,12 +970,12 @@ static llvm::Value *shouldUseMemSetToIni
   return llvm::isBytewiseValue(Init);
 }
 
-/// Decide whether we want to split a constant structure store into a sequence
-/// of its fields' stores. This may cost us code size and compilation speed,
-/// but plays better with store optimizations.
-static bool shouldSplitStructStore(CodeGenModule ,
-   uint64_t GlobalByteSize) {
-  // Don't break structures that occupy more than one cacheline.
+/// Decide whether we want to split a constant structure or array store into a
+/// sequence of its fields' stores. This may cost us code size and compilation
+/// speed, but plays better with store optimizations.
+static bool shouldSplitConstantStore(CodeGenModule ,
+ uint64_t GlobalByteSize) {
+  // Don't break things that occupy more than one cacheline.
   uint64_t ByteSizeLimit = 64;
   if (CGM.getCodeGenOpts().OptimizationLevel == 0)
 return false;
@@ -1203,9 +1203,9 @@ static void emitStoresForConstant(CodeGe
   CGBuilderTy ,
   llvm::Constant *constant) {
   auto *Ty = constant->getType();
-  bool isScalar = Ty->isIntOrIntVectorTy() || Ty->isPtrOrPtrVectorTy() ||
-  Ty->isFPOrFPVectorTy();
-  if (isScalar) {
+  bool canDoSingleStore = Ty->isIntOrIntVectorTy() ||
+  Ty->isPtrOrPtrVectorTy() || Ty->isFPOrFPVectorTy();
+  if (canDoSingleStore) {
 Builder.CreateStore(constant, Loc, isVolatile);
 return;
   }
@@ -1213,12 +1213,13 @@ static void emitStoresForConstant(CodeGe
   auto *Int8Ty = llvm::IntegerType::getInt8Ty(CGM.getLLVMContext());
   auto *IntPtrTy = CGM.getDataLayout().getIntPtrType(CGM.getLLVMContext());
 
-  // If the initializer is all or mostly the same, codegen with bzero / memset
-  // then do a few stores afterward.
   uint64_t ConstantSize = CGM.getDataLayout().getTypeAllocSize(Ty);
   if (!ConstantSize)
 return;
   auto *SizeVal = llvm::ConstantInt::get(IntPtrTy, ConstantSize);
+
+  // If the initializer is all or mostly the same, codegen with bzero / memset
+  // then do a few stores afterward.
   if (shouldUseBZeroPlusStoresToInitialize(constant, ConstantSize)) {
 Builder.CreateMemSet(Loc, llvm::ConstantInt::get(Int8Ty, 0), SizeVal,
  isVolatile);
@@ -1232,6 +1233,7 @@ static void emitStoresForConstant(CodeGe
 return;
   }
 
+  // If the initializer is a repeated byte pattern, use memset.
   llvm::Value *Pattern = shouldUseMemSetToInitialize(constant, ConstantSize);
   if (Pattern) {
 uint64_t Value = 0x00;
@@ -1245,20 +1247,34 @@ static void emitStoresForConstant(CodeGe
 return;
   }
 
-  llvm::StructType *STy = dyn_cast(Ty);
-  // FIXME: handle the case when STy != Loc.getElementType().
-  // FIXME: handle non-struct aggregate types.
-  if (STy && (STy == Loc.getElementType()) &&
-  shouldSplitStructStore(CGM, ConstantSize)) {
-for (unsigned i = 0; i != constant->getNumOperands(); i++) {
-  Address EltPtr = Builder.CreateStructGEP(Loc, i);
-  emitStoresForConstant(
-  CGM, D, EltPtr, isVolatile, Builder,
-  cast(Builder.CreateExtractValue(constant, i)));
+  // If the initializer is small, use a handful of stores.
+  if (shouldSplitConstantStore(CGM, ConstantSize)) {
+if (auto *STy = dyn_cast(Ty)) {
+  // FIXME: handle the case when STy != Loc.getElementType().
+  if (STy == Loc.getElementType()) {
+for (unsigned i = 0; i != constant->getNumOperands(); i++) {
+  Address EltPtr = Builder.CreateStructGEP(Loc, i);
+  emitStoresForConstant(
+  CGM, D, EltPtr, isVolatile, Builder,
+  cast(Builder.CreateExtractValue(constant, i)));
+}
+return;
+  }
+} else if (auto *ATy = dyn_cast(Ty)) {
+  // FIXME: handle the case when ATy != Loc.getElementType().
+  if (ATy == Loc.getElementType()) {
+for (unsigned i = 0; i != ATy->getNumElements(); i++) {
+  Address EltPtr = Builder.CreateConstArrayGEP(Loc, i);
+  

[PATCH] D59118: creduce script for clang crashes

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

Thanks for this! Functionally, this looks good. My comments are mostly just 
simplicity/readability nitpicks.




Comment at: clang/utils/creduce-clang-crash.py:36
+  # Get clang call from build script
+  cmd = None
+  with open(build_script, 'r') as f:

nit: this isn't necessary; `with` doesn't introduce its own scope



Comment at: clang/utils/creduce-clang-crash.py:37
+  cmd = None
+  with open(build_script, 'r') as f:
+cmd = f.read()

nit `, 'r'` is implied; please remove



Comment at: clang/utils/creduce-clang-crash.py:38
+  with open(build_script, 'r') as f:
+cmd = f.read()
+cmd = re.sub("#!.*\n", "", cmd)

Hrm. Looks like there are cases where these crash reproducers are multiple 
lines, though the actually meaningful one (to us) is always the last one? If 
so, it may be cleaner to say `cmd = f.readlines()[-1]` here



Comment at: clang/utils/creduce-clang-crash.py:43
+  # Get crash output
+  p = subprocess.Popen(build_script,
+   stdout=subprocess.PIPE,

nit: can replace with `subprocess.check_output` unless we explicitly want to 
ignore the return value (in which case, we should probably still call `wait()` 
anyway?)



Comment at: clang/utils/creduce-clang-crash.py:49
+  output = ['#!/bin/bash']
+  output.append('%s --crash %s >& t.log || exit 1' % (llvm_not, cmd))
+

please `pipes.quote(llvm_not)` and `pipes.quote(cmd)`



Comment at: clang/utils/creduce-clang-crash.py:54
+  # last five stack trace functions
+  assertion_re = "Assertion \`([^\']+)\' failed"
+  assertion_match = re.search(assertion_re, crash_output)

Why are we escaping the graves and single-quotes?

Also, nit: when possible with regex strings, please use [raw 
strings](https://stackoverflow.com/questions/2081640/what-exactly-do-u-and-r-string-flags-do-and-what-are-raw-string-literals).
 Doing so makes it way easier to reason about what Python's going to transform 
in the string literal before the regex engine gets to see it.



Comment at: clang/utils/creduce-clang-crash.py:58
+msg = assertion_match.group(1).replace('"', '\\"')
+output.append('grep "%s" t.log || exit 1' % msg)
+  else:

nit: please `pipes.quote` instead of adding manual quotes



Comment at: clang/utils/creduce-clang-crash.py:62
+matches = re.findall(stacktrace_re, crash_output)
+del matches[:len(matches)-5]
+output += ['grep "%s" t.log || exit 1' % msg for msg in matches]

nit: please simplify to `del matches[:-5]`



Comment at: clang/utils/creduce-clang-crash.py:63
+del matches[:len(matches)-5]
+output += ['grep "%s" t.log || exit 1' % msg for msg in matches]
+

nit: `pipes.quote` please



Comment at: clang/utils/creduce-clang-crash.py:76
+  parser.add_argument('--llvm-not', dest='llvm_not', type=str,
+  help="""The path to the llvm-not executable.
+  Required if 'not' is not in PATH environment.""");

nit: probably better to concatenate these; otherwise, I think we'll end up with 
a lot of spaces between these sentences? e.g.

```
help="The path to the llvm-not executable. "
 "Required if [...]")
```



Comment at: clang/utils/creduce-clang-crash.py:108
+  testfile = testname + '.test.sh'
+  open(testfile, 'w').write('\n'.join(test_contents))
+  os.chmod(testfile, os.stat(testfile).st_mode | stat.S_IEXEC)

I hate being *that* person, this technically isn't portable Python. I don't 
honestly know if we care about not-CPython being able to run our code, but the 
fix is to simply use `with open(...) as f:` instead of this one-liner.

(Specifically, CPython uses refcounts, so `testfile` will always be closed 
promptly, unless CPython decides someday to make files cyclic with themselves. 
GC'ed python implementations might take a while to flush this file and clean it 
up, which would be bad...)



Comment at: clang/utils/creduce-clang-crash.py:113
+  try:
+p = subprocess.Popen([creduce, testfile, file_to_reduce])
+p.communicate()

Does creduce try and jump out into its own pgid? If not, I think this 
try/except block can be replaced with `sys.exit(subprocess.call([creduce, 
testfile, file_to_reduce]))`



Comment at: clang/utils/creduce-clang-crash.py:120
+
+  sys.exit(0)
+

nit: this is implied; please remove


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59118



___
cfe-commits mailing list
cfe-commits@lists.llvm.org

Re: r355322 - Enable _rotl, _lrotl, _rotr, _lrotr on all platforms.

[Reid Kleckner via cfe-commits]

I would recommend doing what was done for _xgetbv as in r351391:
- Provide __builtin_ia32_rot* everywhere (or something like that in the
implementor's namespace)
- Conditionally #define _rot* to __builtin_ia32_rot* in ia32intrin.h, maybe
ifdef _GNUC
- Also provide _rot* builtins when -fms-extensions is enabled

That way, we aren't non-conforming out of the box before Intel intrinsic
headers start getting included.

On Thu, Mar 7, 2019 at 3:52 PM James Y Knight via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> This patch breaks some code which is (conditionally) defining functions of
> these names on certain platforms. Now, it's true that it shouldn't be doing
> that, and if the claim in the commit message about GCC was true, I'd just
> say "don't do that".
>
> But, the commit message is wrong. GCC does _not_ define these as builtins,
> it defines them in ia32intrin.h header (publicly via x86intrin.h) .
>
> IMO, this commit should be reverted, and  functions defined in the header,
> instead. Perhaps similar to what 2c8f9c2c23e0cafd7b85a7aec969c949349f747c
> 
> did, although I note that was reverted in
> b62c5bc64dee3642884c31b8208c07a6f74b81fd
> ,
> reportedly due to breaking mingw.
>
> (I'd further note that even MSVC doesn't enable these builtins by default!
> It implements a `#pragma intrinsic(NAME)` mechanism which you need to use
> in order to enable them (and #include  will do so). But clang
> doesn't really implement that pragma, and instead enables all the
> MSVC-builtins unconditionally.)
>
> On Mon, Mar 4, 2019 at 1:46 PM Erich Keane via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: erichkeane
>> Date: Mon Mar  4 10:47:21 2019
>> New Revision: 355322
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=355322=rev
>> Log:
>> Enable _rotl, _lrotl, _rotr, _lrotr on all platforms.
>>
>> The above builtins are currently implemented for MSVC mode, however GCC
>> also implements these.  This patch enables them for all platforms.
>>
>> Additionally, this corrects the type for these builtins to always be
>> 'long int' to match the specification in the Intel Intrinsics Guide.
>>
>> Change-Id: Ida34be98078709584ef5136c8761783435ec02b1
>>
>> Added:
>> cfe/trunk/test/CodeGen/rot-intrinsics.c   (with props)
>> Modified:
>> cfe/trunk/include/clang/Basic/Builtins.def
>> cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c
>>
>> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r355659 - [X86] Make x86-intrinsics-headers-clean.cpp stricter.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Thu Mar  7 17:15:18 2019
New Revision: 355659

URL: http://llvm.org/viewvc/llvm-project?rev=355659=rev
Log:
[X86] Make x86-intrinsics-headers-clean.cpp stricter.

Remove the -Wno-ignored-attributes.

Add -fno-lax-vector-conversions

Also use -ffreestanding instead of defining _MM_MALLOC_H.

Modified:
cfe/trunk/test/Headers/x86-intrinsics-headers-clean.cpp

Modified: cfe/trunk/test/Headers/x86-intrinsics-headers-clean.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/x86-intrinsics-headers-clean.cpp?rev=355659=355658=355659=diff
==
--- cfe/trunk/test/Headers/x86-intrinsics-headers-clean.cpp (original)
+++ cfe/trunk/test/Headers/x86-intrinsics-headers-clean.cpp Thu Mar  7 17:15:18 
2019
@@ -1,14 +1,8 @@
 // Make sure the intrinsic headers compile cleanly with no warnings or errors.
 
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wsystem-headers \
-// RUN:   -fsyntax-only -x c++ -Wno-ignored-attributes -verify %s
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wsystem-headers \
-// RUN:   -fsyntax-only -x c++ -Wno-ignored-attributes -target-feature +f16c \
-// RUN:   -verify %s
+// RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown 
-Wsystem-headers \
+// RUN:   -fsyntax-only -fno-lax-vector-conversions -x c++ -verify %s
 
 // expected-no-diagnostics
 
-// Dont' include mm_malloc.h. It's system specific.
-#define __MM_MALLOC_H
-
 #include 


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

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

Looks solid to me overall; just a few nits.

I don't think I have actual ownership over any of this code, so I'll defer to 
either Aaron or Richard for the final LGTM

Thanks again!




Comment at: clang/lib/Sema/SemaChecking.cpp:317
+  // buffer size would be.
+  auto computeObjectSize = [&](unsigned ObjIdx) {
+// If the parameter has a pass_object_size attribute, then we should use

nit: I think the prevailing preference is to name lambdas as you'd name 
variables, rather than as you'd name methods/functions



Comment at: clang/lib/Sema/SemaChecking.cpp:321
+// assume type 0.
+int BOSType = 0;
+if (auto *POS = FD->getParamDecl(ObjIdx)->getAttr())

Should this also try to consider `fortify_stdlib`?



Comment at: clang/lib/Sema/SemaChecking.cpp:367
+DiagID = diag::warn_memcpy_chk_overflow;
+if (!evaluateObjectSize(TheCall->getNumArgs()-1) ||
+!evaluateSizeArg(TheCall->getNumArgs()-2))

nit: All of these cases (and the two lambdas above) look super similar. Might 
it be clearer to just set `SizeIndex` and `ObjectIndex` variables from here, 
and actually `evaluate` them before `UsedSize.ule(ComputedSize)`?

If not, I'm OK with this as-is.



Comment at: clang/lib/Sema/SemaExpr.cpp:5929
 
+checkFortifiedBuiltinMemoryFunction(FDecl, TheCall);
+

Why isn't this in CheckBuiltinFunctionCall?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D59118: creduce script for clang crashes

[Amy Huang via Phabricator via cfe-commits]

akhuang created this revision.
akhuang added reviewers: rnk, george.burgess.iv.
Herald added a reviewer: serge-sans-paille.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add a script that calls C-Reduce on an input file and given the clang crash 
script, which is used to generate an interestingness test for C-Reduce.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59118

Files:
  .arcconfig
  clang/utils/creduce-clang-crash.py

Index: clang/utils/creduce-clang-crash.py
===
--- /dev/null
+++ clang/utils/creduce-clang-crash.py
@@ -0,0 +1,123 @@
+#!/usr/bin/env python
+# A tool that calls C-Reduce to create a minimal reproducer for clang crashes
+# Requires C-Reduce and not (part of LLVM utils) to be installed
+
+from argparse import ArgumentParser
+import os
+import re
+import stat
+import sys
+import subprocess
+
+def is_exe(fpath):
+  return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
+
+def which(program):
+  """
+  Return the full path to a program or return None.
+  """
+  fpath, fname = os.path.split(program)
+  if fpath:
+if is_exe(program):
+  return program
+  else:
+for path in os.environ["PATH"].split(os.pathsep):
+  exe_file = os.path.join(path, program)
+  if is_exe(exe_file):
+return exe_file
+  return None
+
+def create_test(build_script, llvm_not):
+  """
+  Create an interestingness test from the crash output.
+  Return as a string.
+  """
+  # Get clang call from build script
+  cmd = None
+  with open(build_script, 'r') as f:
+cmd = f.read()
+cmd = re.sub("#!.*\n", "", cmd)
+cmd = cmd.rstrip('\n\r')
+
+  # Get crash output
+  p = subprocess.Popen(build_script,
+   stdout=subprocess.PIPE,
+   stderr=subprocess.STDOUT)
+  crash_output, _ = p.communicate()
+
+  output = ['#!/bin/bash']
+  output.append('%s --crash %s >& t.log || exit 1' % (llvm_not, cmd))
+
+  # Add messages from crash output to the test
+  # If there is an Assertion failure, use that; otherwise use the
+  # last five stack trace functions
+  assertion_re = "Assertion \`([^\']+)\' failed"
+  assertion_match = re.search(assertion_re, crash_output)
+  if assertion_match:
+msg = assertion_match.group(1).replace('"', '\\"')
+output.append('grep "%s" t.log || exit 1' % msg)
+  else:
+stacktrace_re = "#[0-9]+\s+0[xX][0-9a-fA-F]+\s*([^(]+)\("
+matches = re.findall(stacktrace_re, crash_output)
+del matches[:len(matches)-5]
+output += ['grep "%s" t.log || exit 1' % msg for msg in matches]
+
+  return output
+
+def main():
+  parser = ArgumentParser(description='Runs C-Reduce on the input file')
+  parser.add_argument('build_script', type=str, nargs=1,
+  help='Name of the script that generates the crash.')
+  parser.add_argument('file_to_reduce', type=str, nargs=1,
+  help='Name of the file to be reduced.')
+  parser.add_argument('-o', '--output', dest='output', type=str,
+  help='Name of the output file for the reduction. Optional.')
+  parser.add_argument('--llvm-not', dest='llvm_not', type=str,
+  help="""The path to the llvm-not executable.
+  Required if 'not' is not in PATH environment.""");
+  parser.add_argument('--creduce', dest='creduce', type=str,
+  help="""The path to the C-Reduce executable.
+  Required if 'creduce' is not in PATH environment.""");
+  args = parser.parse_args()
+
+  build_script = os.path.abspath(args.build_script[0])
+  file_to_reduce = os.path.abspath(args.file_to_reduce[0])
+  llvm_not = which(args.llvm_not) if args.llvm_not else which('not')
+  creduce = which(args.creduce) if args.creduce else which('creduce')
+
+  if not os.path.isfile(build_script):
+print(("ERROR: input file '%s' does not exist") % build_script)
+sys.exit(1)
+
+  if not os.path.isfile(file_to_reduce):
+print(("ERROR: input file '%s' does not exist") % file_to_reduce)
+sys.exit(1)
+
+  if not llvm_not:
+parser.print_help()
+sys.exit(1)
+
+  if not creduce:
+parser.print_help()
+sys.exit(1)
+
+  # Write interestingness test to file
+  test_contents = create_test(build_script, llvm_not)
+  testname, _ = os.path.splitext(file_to_reduce)
+  testfile = testname + '.test.sh'
+  open(testfile, 'w').write('\n'.join(test_contents))
+  os.chmod(testfile, os.stat(testfile).st_mode | stat.S_IEXEC)
+
+  # Call C-Reduce
+  try:
+p = subprocess.Popen([creduce, testfile, file_to_reduce])
+p.communicate()
+
+  except KeyboardInterrupt:
+print('\n\nctrl-c detected, killed creduce')
+p.kill()
+
+  sys.exit(0)
+
+if __name__ == '__main__':
+  main()
Index: .arcconfig
===
--- .arcconfig
+++ .arcconfig
@@ -1,4 +1,4 @@
 {
-  "repository.callsign" : "G",
-  "conduit_uri" : 

[PATCH] D59055: [analyzer] Prepare generic taint checker for new sources

[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 "source" functions are now merely "propagate from nothing" functions? 
Fair enough!


Repository:
  rC Clang

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

https://reviews.llvm.org/D59055



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

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

Ok, thanks, sounds good! Let's land and see where it goes.




Comment at: test/Analysis/ptr-sort.cpp:4-25
+namespace std {
+  template
+  bool is_sorted(ForwardIt first, ForwardIt last);
+
+  template 
+  void nth_element(RandomIt first, RandomIt nth, RandomIt last);
+

Please feel free to put these directly into `system-header-simulator-cxx.h`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D50488



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

[PATCH] D58777: [analyzer] Fix an assertation failurure for invalid sourcelocation, add a new debug checker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a subscriber: george.karpenkov.
NoQ added a comment.
This revision is now accepted and ready to land.

Very nice! Crashing diagnostic locations are a fairly common issue. That said, 
this still makes me wonder what sort of checker were you writing that triggered 
this originally :)




Comment at: lib/StaticAnalyzer/Checkers/DebugCheckers.cpp:271
+//===--===//
+// Emits a report for each and every Stmt.
+//===--===//

Technically, we don't invoke `checkPreStmt` for every `Stmt` (eg., `IfStmt` is 
omitted in favor of `checkBranchCondition`, `IntegerLiteral` is outright 
skipped because `ExprEngine` doesn't even evaluate anything at this point). 
Moreover, for some `Stmt`s we more-or-less-intentionally only invoke 
`checkPreStmt` but not `checkPostStmt`.

Also would you eventually want to expand this checker with non-`Stmt` callbacks?



Comment at: lib/StaticAnalyzer/Checkers/DebugCheckers.cpp:277
+class ReportStmts : public Checker> {
+  std::unique_ptr BT_stmtLoc;
+

My current favorite approach is:
```lang=c++
BuiltinBug BT_stmtLoc{this, "Statement"};
```

...and remove the constructor. And remove the `*` in your 
`make_unique`.



Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:574-575
 
+  // FIXME: Ironically, this assert actually fails in some cases.
+  //assert(L.isValid());
   return L;

A normal day in Clang :)



Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:679
+
+  if (ME->getMemberLoc().isValid())
+return PathDiagnosticLocation(ME->getMemberLoc(), SM, SingleLocK);

I think it's worth commenting why exactly this may fail. It's fairly hard to 
guess that we're talking about member operators.



Comment at: test/Analysis/invalid-pos-fix.cpp:6
+
+struct h {
+  operator int();

Random ideas on organizing tests so that to avoid adding thousands of one-test 
files:
* Give these objects fancier names and/or add a `no-crash` so that it was clear 
that this test tests a crash for reporting a bug over a `MemberExpr` that 
corresponds to an `operator()`
* Or maybe wrap this into a namespace, so that it was easier to expand this 
file.
* We traditionally put such tests into `test/Analysis/diagnostics`, dunno why 
though.
etc.

I also recall @george.karpenkov arguing for making test directory structure 
mirror source directory structure, but that's definitely not a blocker for this 
patch :]



Comment at: test/Analysis/invalid-pos-fix.cpp:11-13
+  return h(); // expected-warning{{Statement}}
+  // expected-warning@-1{{Statement}}
+  // expected-warning@-2{{Statement}}

`// expected-warning 3 {{Statement}}`


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

https://reviews.llvm.org/D58777



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

Re: r355322 - Enable _rotl, _lrotl, _rotr, _lrotr on all platforms.

[James Y Knight via cfe-commits]

This patch breaks some code which is (conditionally) defining functions of
these names on certain platforms. Now, it's true that it shouldn't be doing
that, and if the claim in the commit message about GCC was true, I'd just
say "don't do that".

But, the commit message is wrong. GCC does _not_ define these as builtins,
it defines them in ia32intrin.h header (publicly via x86intrin.h) .

IMO, this commit should be reverted, and  functions defined in the header,
instead. Perhaps similar to what 2c8f9c2c23e0cafd7b85a7aec969c949349f747c

did, although I note that was reverted in
b62c5bc64dee3642884c31b8208c07a6f74b81fd
,
reportedly due to breaking mingw.

(I'd further note that even MSVC doesn't enable these builtins by default!
It implements a `#pragma intrinsic(NAME)` mechanism which you need to use
in order to enable them (and #include  will do so). But clang
doesn't really implement that pragma, and instead enables all the
MSVC-builtins unconditionally.)

On Mon, Mar 4, 2019 at 1:46 PM Erich Keane via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: erichkeane
> Date: Mon Mar  4 10:47:21 2019
> New Revision: 355322
>
> URL: http://llvm.org/viewvc/llvm-project?rev=355322=rev
> Log:
> Enable _rotl, _lrotl, _rotr, _lrotr on all platforms.
>
> The above builtins are currently implemented for MSVC mode, however GCC
> also implements these.  This patch enables them for all platforms.
>
> Additionally, this corrects the type for these builtins to always be
> 'long int' to match the specification in the Intel Intrinsics Guide.
>
> Change-Id: Ida34be98078709584ef5136c8761783435ec02b1
>
> Added:
> cfe/trunk/test/CodeGen/rot-intrinsics.c   (with props)
> Modified:
> cfe/trunk/include/clang/Basic/Builtins.def
> cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59013: [CMake][runtimes] Move libunwind, libc++abi and libc++ to lib/ and include/

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 189795.
phosek marked 2 inline comments as done.

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

https://reviews.llvm.org/D59013

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/linux-per-target-runtime-dir.c
  libcxx/CMakeLists.txt
  libcxx/lib/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libcxxabi/src/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  llvm/runtimes/CMakeLists.txt

Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -124,8 +124,8 @@
 string(REPLACE "-" "_" canon_name ${projName})
 string(TOUPPER ${canon_name} canon_name)
 
-if(LLVM_RUNTIMES_LIBDIR_SUFFIX)
-  set(${canon_name}_LIBDIR_SUFFIX "${LLVM_RUNTIMES_LIBDIR_SUFFIX}" CACHE STRING "" FORCE)
+if(LLVM_RUNTIMES_LIBDIR_SUBDIR)
+  set(${canon_name}_LIBDIR_SUBDIR "${LLVM_RUNTIMES_LIBDIR_SUBDIR}" CACHE STRING "" FORCE)
 endif()
 
 # Setting a variable to let sub-projects detect which other projects
@@ -349,6 +349,7 @@
  # Builtins were built separately above
  CMAKE_ARGS -DCOMPILER_RT_BUILD_BUILTINS=Off
 -DLLVM_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
+-DLLVM_BINARY_DIR=${LLVM_BINARY_DIR}
 -DLLVM_LIBRARY_DIR=${LLVM_LIBRARY_DIR}
 -DLLVM_DEFAULT_TARGET_TRIPLE=${TARGET_TRIPLE}
 -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON
@@ -432,6 +433,7 @@
  # Builtins were built separately above
  CMAKE_ARGS -DCOMPILER_RT_BUILD_BUILTINS=Off
 -DLLVM_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
+-DLLVM_BINARY_DIR=${LLVM_BINARY_DIR}
 -DLLVM_LIBRARY_DIR=${LLVM_LIBRARY_DIR}
 -DLLVM_DEFAULT_TARGET_TRIPLE=${target}
 -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON
@@ -514,8 +516,7 @@
   runtime_register_target(${name}-${sanitizer_name} ${name}
 DEPENDS runtimes-${name}
 CMAKE_ARGS -DLLVM_USE_SANITIZER=${sanitizer}
-   -DLLVM_RUNTIMES_PREFIX=${name}/
-   -DLLVM_RUNTIMES_LIBDIR_SUFFIX=/${sanitizer_name})
+   -DLLVM_RUNTIMES_LIBDIR_SUBDIR=${sanitizer_name})
   add_dependencies(runtimes runtimes-${name}-${sanitizer_name})
   add_dependencies(runtimes-configure runtimes-${name}-${sanitizer_name}-configure)
   add_dependencies(install-runtimes install-runtimes-${name}-${sanitizer_name})
Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -178,8 +178,8 @@
 
 if (LIBUNWIND_INSTALL_LIBRARY)
   install(TARGETS ${LIBUNWIND_INSTALL_TARGETS}
-LIBRARY DESTINATION ${LIBUNWIND_INSTALL_PREFIX}lib${LIBUNWIND_LIBDIR_SUFFIX} COMPONENT unwind
-ARCHIVE DESTINATION ${LIBUNWIND_INSTALL_PREFIX}lib${LIBUNWIND_LIBDIR_SUFFIX} COMPONENT unwind)
+LIBRARY DESTINATION ${LIBUNWIND_INSTALL_PREFIX}${LIBUNWIND_INSTALL_LIBRARY_DIR} COMPONENT unwind
+ARCHIVE DESTINATION ${LIBUNWIND_INSTALL_PREFIX}${LIBUNWIND_INSTALL_LIBRARY_DIR} COMPONENT unwind)
 endif()
 
 if (NOT CMAKE_CONFIGURATION_TYPES AND LIBUNWIND_INSTALL_LIBRARY)
Index: libunwind/CMakeLists.txt
===
--- libunwind/CMakeLists.txt
+++ libunwind/CMakeLists.txt
@@ -187,20 +187,25 @@
${PACKAGE_VERSION})
 
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
-  set(DEFAULT_INSTALL_PREFIX lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
-  set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}${LIBUNWIND_LIBDIR_SUBDIR})
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE})
+  if(LIBCXX_LIBDIR_SUBDIR)
+string(APPEND LIBUNWIND_LIBRARY_DIR /${LIBUNWIND_LIBDIR_SUBDIR})
+string(APPEND LIBUNWIND_INSTALL_LIBRARY_DIR /${LIBUNWIND_LIBDIR_SUBDIR})
+  endif()
 elseif(LLVM_LIBRARY_OUTPUT_INTDIR)
   set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LIBUNWIND_LIBDIR_SUFFIX})
 else()
   set(LIBUNWIND_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LIBUNWIND_LIBDIR_SUFFIX})
 endif()
 
 

[PATCH] D59054: [analyzer] C++17: PR40022: Support aggregate initialization with compound values in presence of base classes.

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG


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

https://reviews.llvm.org/D59054



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

[PATCH] D59054: [analyzer] C++17: PR40022: Support aggregate initialization with compound values in presence of base classes.

[Artem Dergachev via Phabricator via cfe-commits]

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

Added this as a comment.

If the patch is not obvious at a glance, feel free to demand comments! Every 
second you spend trying to figure out how the code works is multiplied by the 
number of people who will have to do the same in the future.


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

https://reviews.llvm.org/D59054

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/array-struct-region.cpp

Index: clang/test/Analysis/array-struct-region.cpp
===
--- clang/test/Analysis/array-struct-region.cpp
+++ clang/test/Analysis/array-struct-region.cpp
@@ -1,7 +1,21 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ -analyzer-config c++-inlining=constructors %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c++ -analyzer-config c++-inlining=constructors %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection -verify\
+// RUN:-x c %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection -verify\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection -verify\
+// RUN:-x c++ -std=c++17 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection -verify\
+// RUN:-DINLINE -x c %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection -verify\
+// RUN:-DINLINE -x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection -verify\
+// RUN:-DINLINE -x c++ -std=c++17 %s
 
 void clang_analyzer_eval(int);
 
@@ -196,4 +210,49 @@
   }
 }
 
+#if __cplusplus >= 201703L
+namespace aggregate_inheritance_cxx17 {
+struct A {
+  int x;
+};
+
+struct B {
+  int y;
+};
+
+struct C: B {
+  int z;
+};
+
+struct D: A, C {
+  int w;
+};
+
+void foo() {
+  D d{1, 2, 3, 4};
+  clang_analyzer_eval(d.x == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(d.y == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(d.z == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(d.w == 4); // expected-warning{{TRUE}}
+}
+} // namespace aggregate_inheritance_cxx17
+#endif
+
+namespace flex_array_inheritance_cxx17 {
+struct A {
+  int flexible_array[];
+};
+
+struct B {
+  long cookie;
+};
+
+struct C : B {
+  A a;
+};
+
+void foo() {
+  C c{}; // no-crash
+}
+} // namespace flex_array_inheritance_cxx17
 #endif
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2334,12 +2334,57 @@
   if (V.isUnknown() || !V.getAs())
 return bindAggregate(B, R, UnknownVal());
 
+  // The raw CompoundVal is essentially a symbolic InitListExpr: an (immutable)
+  // list of other values. It appears pretty much only when there's an actual
+  // initializer list expression in the program, and the analyzer tries to
+  // unwrap it as soon as possible.
+  // This code is where such unwrap happens: when the compound value is put into
+  // the object that it was supposed to initialize (it's an *initializer* list,
+  // after all), instead of binding the whole value to the whole object, we bind
+  // sub-values to sub-objects. Sub-values may themselves be compound values,
+  // and in this case the procedure becomes recursive.
+  // FIXME: The annoying part about compound values is that they don't carry
+  // any sort of information about which value corresponds to which sub-object.
+  // It's simply a list of values in the middle of nowhere; we expect to match
+  // them to sub-objects, essentially, "by index": first value binds to
+  // the first field, second value binds to the second field, etc.
+  // It would have been much safer to organize non-lazy compound values as
+  // a mapping from fields/bases to values.
   const nonloc::CompoundVal& CV = V.castAs();
   nonloc::CompoundVal::iterator VI = CV.begin(), VE = CV.end();
 
-  RecordDecl::field_iterator FI, FE;
   RegionBindingsRef NewB(B);
 
+  // In C++17 aggregates may have base classes, handle those as well.
+  // They appear before fields in the 

[PATCH] D58573: [analyzer] Move UninitializedObject out of alpha

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

At a glance, it seems as if these are great findings for a hardening effort, 
but most of these don't look like immediate bugs (had a look at 10 or so), so i 
guess i'd suggest `optin.cplusplus`, at least for now.

Also, hmm, how about the following heuristics (looked at the positive in 
`rtags` and thought about this):

- if the field is public, don't warn (the structure seems to be used simply as 
a convenient group of variables?)
- if all uninitialized fields are in all cases written into immediately after 
the constructor ends, suppress the warning (i vaguely remember that something 
similar was already proposed, but i might be wrong)

Also huge thanks for keeping things documented!!


Repository:
  rC Clang

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

https://reviews.llvm.org/D58573



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

[PATCH] D59048: Add AIX Target Info

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/Preprocessor/init.c:6420
+// PPC64-AIX:#define _LONG_LONG 1
+// PPC64-AIX:#define _POWER 1
+// PPC64-AIX:#define __64BIT__ 1

hubert.reinterpretcast wrote:
> apaprocki wrote:
> > XL on AIX emits `#define _LP64 1` in 64-bit mode and `#define _ILP32 1` in 
> > 32-bit mode in the pre-defined macros. Is that important to capture?
> I think so. The v16.1 XL compiler's `xlclang` also produces these.
> 
> ```
> #define __LP64__ 1
> #define _LP64 1
> ```
> 
> ```
> #define __ILP32__ 1
> #define _ILP32 1
> ```
It seems GCC on AIX only defines the macros for 64-bit, and not the 32-bit 
versions. The system headers do not appear to depend on the 32-bit versions. It 
makes sense to start with the common intersection between the GCC and XL 
predefined macros first. We can add `__LP64__` and `_LP64` with this patch. I 
think we can leave more macros for later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59048



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

[PATCH] D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable

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

Szelethus planned changes to this revision.
Szelethus added a comment.

Uhh, we still need to restore the default value -- I might end up splitting 
this patch a little further.


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

https://reviews.llvm.org/D57922



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

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

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

Szelethus added a reviewer: JonasToth.
Szelethus added a comment.

Is this patch good to go? :)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D48866



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

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

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

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

@steakhal I think it is basically done. If you have any recommendations what 
needs to be changed, then I welcome your contribution. I can also fix new 
comments, but I have limited time for this.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D48866



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

[PATCH] D59054: [analyzer] C++17: PR40022: Support aggregate initialization with compound values in presence of base classes.

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

Szelethus added a comment.

Ooooh you explained that very well, I'm clear on what's happening then :D 
Still, I'll poke the code here and there to learn about this part of the 
analyzer, I might even come up with some meaningful feedback (or a meaningful 
patch acceptance).


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

https://reviews.llvm.org/D59054



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

[PATCH] D59054: [analyzer] C++17: PR40022: Support aggregate initialization with compound values in presence of base classes.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2348
+// Virtual bases still aren't allowed. Multiple bases are fine though.
+for (auto B : CRD->bases()) {
+  assert(B.isVirtual() == false);

alexfh wrote:
> Should this be `const auto&`?
Whoops. Also i still somehow hate it that these aren't pointers :)



Comment at: clang/test/Analysis/array-struct-region.cpp:3
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ 
-analyzer-config c++-inlining=constructors %s
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ 
-std=c++17 -analyzer-config c++-inlining=constructors %s
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c %s

Szelethus wrote:
> I didn't see `-analyzer-config c++-inlining=constructors` in the bugreport -- 
> why is it needed?
Hmm, right, these flags are outdated long time ago. Wiped them out from this 
test.


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

https://reviews.llvm.org/D59054



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

[PATCH] D59054: [analyzer] C++17: PR40022: Support aggregate initialization with compound values in presence of base classes.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 189781.
NoQ marked 5 inline comments as done.
NoQ added a comment.

Thx! Also added the more direct test.

> I'll take the time to understand the infrastructure behind it

Well, roughly, the `CompoundVal` kind of value is used very rarely, unlike 
`LazyCompoundVal`. The raw `CompoundVal` is essentially a symbolic 
`InitListExpr`: an (immutable) list of other values. It appears pretty much 
only when there's an actual initializer list expression in the program, and the 
analyzer tries to unwrap it as soon as possible.

This code is where such unwrap happens: when the compound value is put into the 
object that it was supposed to initialize (it's an *initializer* list, after 
all), instead of binding the whole value to the whole object, we bind 
sub-values to sub-objects. Sub-values may themselves be compound values, and in 
this case the procedure becomes recursive.

The annoying part about compound values is that they don't carry any sort of 
information about which value corresponds to which sub-object. It's simply a 
list of values in the middle of nowhere; the Store expects to match them to 
sub-objects, essentially, by *index*: first value binds to the first field, 
second value binds to the second field, etc. The crash was occuring when it was 
accidentally trying to bind a base-class value to a field object - because it 
simply didn't know that base objects are a thing.


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

https://reviews.llvm.org/D59054

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/array-struct-region.cpp

Index: clang/test/Analysis/array-struct-region.cpp
===
--- clang/test/Analysis/array-struct-region.cpp
+++ clang/test/Analysis/array-struct-region.cpp
@@ -1,7 +1,21 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ -analyzer-config c++-inlining=constructors %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c++ -analyzer-config c++-inlining=constructors %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection -verify\
+// RUN:-x c %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection -verify\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection -verify\
+// RUN:-x c++ -std=c++17 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection -verify\
+// RUN:-DINLINE -x c %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection -verify\
+// RUN:-DINLINE -x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection -verify\
+// RUN:-DINLINE -x c++ -std=c++17 %s
 
 void clang_analyzer_eval(int);
 
@@ -196,4 +210,49 @@
   }
 }
 
+#if __cplusplus >= 201703L
+namespace aggregate_inheritance_cxx17 {
+struct A {
+  int x;
+};
+
+struct B {
+  int y;
+};
+
+struct C: B {
+  int z;
+};
+
+struct D: A, C {
+  int w;
+};
+
+void foo() {
+  D d{1, 2, 3, 4};
+  clang_analyzer_eval(d.x == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(d.y == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(d.z == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(d.w == 4); // expected-warning{{TRUE}}
+}
+} // namespace aggregate_inheritance_cxx17
+#endif
+
+namespace flex_array_inheritance_cxx17 {
+struct A {
+  int flexible_array[];
+};
+
+struct B {
+  long cookie;
+};
+
+struct C : B {
+  A a;
+};
+
+void foo() {
+  C c{}; // no-crash
+}
+} // namespace flex_array_inheritance_cxx17
 #endif
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2337,9 +2337,38 @@
   const nonloc::CompoundVal& CV = V.castAs();
   nonloc::CompoundVal::iterator VI = CV.begin(), VE = CV.end();
 
-  RecordDecl::field_iterator FI, FE;
   RegionBindingsRef NewB(B);
 
+  // In C++17 aggregates may have base classes, handle those as well.
+  // They appear before fields in the initializer list / compound value.
+  if (const auto *CRD = dyn_cast(RD)) {
+assert(CRD->isAggregate() &&
+   

[PATCH] D59105: [RFC] Create an Arbitrary Precision Integer Type.

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

rsmith added a comment.

In principle, I think an extension in this space seems reasonable and useful. 
Considering the points in http://clang.llvm.org/get_involved.html, I'm happy to 
assume you meet points 2, 5, and 6, but I'd like to see a written-up rationale 
for the other points. (In particular, you do not yet meet the bar for points 3 
and 7, and if you're not proposing this to WG14 or WG21 or similar, some amount 
of rationale justifying that is necessary.)

I don't like the name `ArbPrecIntType`; this is not an arbitrary-precision 
integer type, it's a fixed-width integer type. Something like `ExtIntegerType` 
might fit better.

Defining new types via attributed `typedef`s is a horrible hack, and I would 
not like to extend its scope any further. Have you considered alternative 
interfaces for specifying the type? For instance, we could support an 
`__int(N)` //type-specifier//. (I would expect such a specifier to behave like 
a normal integer width //type-specifier// (eg, like `short` or `long` or 
`__int128`), so we would allow things like `unsigned __int(54)`.)

Have you considered whether `__int(128)` (however you spell it) should be the 
same type as the existing `__int128`? (I think it makes sense for the 
`__int(N)` types to be distinct from all the standard types, even if `N` 
matches, unlike the behavior of the GNU `mode` attribute. But the relationship 
to `__int128` is less clear.)

How do integral promotions behave on these types? Are these types intended to 
follow the rules for extended integer types? If not, in what ways, and why not?

I'd like to see some tests for use of these types in these contexts:

- bit-fields
- enum-bases
- non-type template arguments (incl. mangling thereof)
- UBSan, and in particular `-fsanitize=signed-integer-overflow` (this will 
break; UBSan's static data representation assumes that integer types have 
power-of-two sizes)
- overload resolution with these types as parameters / arguments, particularly 
involving overload sets also including standard integer types
- TBAA metadata (I assume there is no aliasing relationship between these and 
standard integer types)




Comment at: include/clang/Basic/AttrDocs.td:4123-4126
+Since FPGAs have the ability to produce non-power-of-2 math units, there is a
+requirement to have an integer type that supports an arbitrary compile time bit
+representation. These need to be expressed in IR as the correct precision int
+types (such as i3, i46, etc).

Please frame the documentation around the feature itself rather than your 
motivating use case. Mentioning FPGAs as an aside might make some sense, but it 
shouldn't be the primary introduction to the extension's documentation.



Comment at: include/clang/Basic/AttrDocs.td:4139-4141
+this type has no affect on the ArbPrecInt. All operations on an ArbPrecInt
+should take place as an ArbPrecInt type, promoting to the larger of the two
+types.

Please mention that you convert to unsigned if the sizes match, like standard 
integer types do.

Please also describe the promotion rules for these types.



Comment at: lib/AST/ASTContext.cpp:1687-1693
+  // toCharUnitsFromBits always rounds down, which isn't a problem when the 
size
+  // Width is a multiple of chars, however ArbPrecInt makes this not a valid
+  // assumption.
+  return std::make_pair(toCharUnitsFromBits(Info.Width) +
+(Info.Width % getCharWidth() == 0
+ ? CharUnits::Zero()
+ : CharUnits::One()),

This should not be necessary. `getTypeInfo` should not be returning sizes that 
are not a multiple of the bit-width of `char`.



Comment at: lib/AST/ASTContext.cpp:1788
"Overflow in array type bit size evaluation");
-Width = EltInfo.Width * Size;
+Width = llvm::alignTo(EltInfo.Width, EltInfo.Align) * Size;
 Align = EltInfo.Align;

Isn't this wrong for arrays of `__attribute__((aligned(N)))` types? Eg: 
https://godbolt.org/z/jOW_B5

(GCC changed this and broke ABI in version 5 onwards, but if/when we follow 
suit it should be an intentional decision, likely with a `-fclang-abi-compat=` 
flag, not a side-effect of an unrelated change such as this one.)



Comment at: lib/AST/ASTContext.cpp:2132
+const ArbPrecIntType *AT = cast(T);
+Width = AT->getNumBits();
+Align = std::min(std::max(getCharWidth(), llvm::PowerOf2Ceil(Width)),

Please do not add another case where a type's size is not a multiple of its 
alignment. This behavior of `__attribute__((aligned(N)))` is a mistake that we 
should not repeat. (Remember that `sizeof(T)` is supposed to be the size of `T` 
as an array element.) `Width` should include the padding necessary to reach a 
multiple of the alignment.



Comment at: lib/AST/ASTContext.cpp:2133-2134
+Width 

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I'm definitely worried about the false positive rate for this check -- from a 
quick look at the diagnostics from LLVM, every one of them is a false positive 
(though perhaps I've missed something; it was a very quick look). Requiring 
users to explicitly use a declaration within the operator seems like it could 
get onerous for complex classes that support the notion of equality from a few 
members as opposed to all members.

Also, can you add some tests for classes that inherit members, perhaps even 
private ones? e.g.,

  struct Base {
int Member;
  };
  
  struct Derived : Base {
int OtherMember;
  
bool operator==(const Derived ) const { return OtherMember == 
RHS.OtherMember; }
  };
  
  struct Derived2 : private Base {
int OtherMember;
  
bool operator==(const Derived2 ) const { return OtherMember == 
RHS.OtherMember; }
  };

Another good test is for scenarios where you cannot compare all of the members, 
such as:

  struct Interesting {
bool operator==(const Interesting ) const = delete;
  };
  
  struct S {
int Member;
Interesting OtherMember;
  
bool operator==(const S ) const { return Member == Other.Member; }
  };


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59103



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

Re: r355489 - clang-cl: Parse /Qspectre and a few other missing options (PR40964)

[Hans Wennborg via cfe-commits]

Yes, we warn about them being unused.

On Thu, Mar 7, 2019 at 21:59 Nico Weber via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Do we warn "ignoring flag" or something like that for these, or are these
> now silently ignored?
>
> On Wed, Mar 6, 2019 at 4:37 AM Hans Wennborg via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: hans
>> Date: Wed Mar  6 01:38:04 2019
>> New Revision: 355489
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=355489=rev
>> Log:
>> clang-cl: Parse /Qspectre and a few other missing options (PR40964)
>>
>> Modified:
>> cfe/trunk/include/clang/Driver/CLCompatOptions.td
>> cfe/trunk/test/Driver/cl-options.c
>>
>> Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=355489=355488=355489=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Driver/CLCompatOptions.td (original)
>> +++ cfe/trunk/include/clang/Driver/CLCompatOptions.td Wed Mar  6 01:38:04
>> 2019
>> @@ -394,6 +394,8 @@ def _SLASH_Zo_ : CLIgnoredFlag<"Zo-">;
>>
>>  // Unsupported:
>>
>> +def _SLASH_await : CLFlag<"await">;
>> +def _SLASH_constexpr : CLJoined<"constexpr:">;
>>  def _SLASH_AI : CLJoinedOrSeparate<"AI">;
>>  def _SLASH_Bt : CLFlag<"Bt">;
>>  def _SLASH_Bt_plus : CLFlag<"Bt+">;
>> @@ -429,6 +431,9 @@ def _SLASH_Qfast_transcendentals : CLFla
>>  def _SLASH_QIfist : CLFlag<"QIfist">;
>>  def _SLASH_Qimprecise_fwaits : CLFlag<"Qimprecise_fwaits">;
>>  def _SLASH_Qpar : CLFlag<"Qpar">;
>> +def _SLASH_Qpar_report : CLJoined<"Qpar-report">;
>> +def _SLASH_Qsafe_fp_loads : CLFlag<"Qsafe_fp_loads">;
>> +def _SLASH_Qspectre : CLFlag<"Qspectre">;
>>  def _SLASH_Qvec_report : CLJoined<"Qvec-report">;
>>  def _SLASH_u : CLFlag<"u">;
>>  def _SLASH_V : CLFlag<"V">;
>>
>> Modified: cfe/trunk/test/Driver/cl-options.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=355489=355488=355489=diff
>>
>> ==
>> --- cfe/trunk/test/Driver/cl-options.c (original)
>> +++ cfe/trunk/test/Driver/cl-options.c Wed Mar  6 01:38:04 2019
>> @@ -390,6 +390,8 @@
>>  // Unsupported but parsed options. Check that we don't error on them.
>>  // (/Zs is for syntax-only)
>>  // RUN: %clang_cl /Zs \
>> +// RUN: /await \
>> +// RUN: /constexpr:depth1000 /constexpr:backtrace1000
>> /constexpr:steps1000 \
>>  // RUN: /AIfoo \
>>  // RUN: /AI foo_does_not_exist \
>>  // RUN: /Bt \
>> @@ -443,6 +445,9 @@
>>  // RUN: /QIfist \
>>  // RUN: /Qimprecise_fwaits \
>>  // RUN: /Qpar \
>> +// RUN: /Qpar-report:1 \
>> +// RUN: /Qsafe_fp_loads \
>> +// RUN: /Qspectre \
>>  // RUN: /Qvec-report:2 \
>>  // RUN: /u \
>>  // RUN: /V \
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Gor Nishanov via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaCoroutine.cpp:675
+  // Second emphasis of [expr.await]p2: must be outside of an exception 
handler.
+  if (S.getCurScope()->getFlags() & Scope::CatchScope) {
+S.Diag(Loc, diag::err_coroutine_within_handler) << Keyword;

lewissbaker wrote:
> modocache wrote:
> > modocache wrote:
> > > lewissbaker wrote:
> > > > modocache wrote:
> > > > > EricWF wrote:
> > > > > > We can still build a valid AST after encountering this error, no?
> > > > > > 
> > > > > > 
> > > > > I believe so. Just to be clear: you'd like me to continue building 
> > > > > the AST even after emitting this error diagnostic? My understanding 
> > > > > is that most of this file bails soon after any error is encountered 
> > > > > (correct me if that's wrong). I'm happy to change that, but I wonder 
> > > > > if it'd be better to do that in a separate diff...?
> > > > Do the scope flags reset when a lambda occurs within a catch-block?
> > > > 
> > > > eg. The following should still be valid.
> > > > ```
> > > > try { ... }
> > > > catch (...) {
> > > >   []() -> task { co_await foo(); }();
> > > > }
> > > > ```
> > > I believe they're reset, the example you posted compiles fine with this 
> > > patch. I'll add a test case specifically for this and confirm exactly 
> > > where the scope flags are reset or ignored in the lambda case.
> > When the parser encounters a lambda, it takes the path 
> > `Parser::ParseLambdaExpression -> 
> > Parser::ParseLambdaExpressionAfterIntroducer -> 
> > Parser::ParseCompoundStatementBody`, which creates an inner scope for the 
> > body of the lambda. This inner scope does not have the `Scope::CatchScope` 
> > flag, so it doesn't result in the error.
> > 
> > Although, your question did make me realize that this compiles just fine, 
> > even with this patch:
> > 
> > ```
> > void example() {
> >   try {
> > throw;
> >   } catch (...) {
> > try {
> >   co_await a;
> > }
> >   }
> > }
> > ```
> > 
> > But I believe this above case should also be treated as an error, right?
> Yes, I believe that co_await within a try-block within a catch-block should 
> not be allowed.
Yes. That will result in suspension of the coroutine and we don't yet know how 
to suspend in catch blocks.

Also, I agree with @EricWF, this error should not prevent us from continuing 
semantic analysis with the rest of the function body.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59076



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

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Lewis Baker via Phabricator via cfe-commits]

lewissbaker added inline comments.



Comment at: lib/Sema/SemaCoroutine.cpp:675
+  // Second emphasis of [expr.await]p2: must be outside of an exception 
handler.
+  if (S.getCurScope()->getFlags() & Scope::CatchScope) {
+S.Diag(Loc, diag::err_coroutine_within_handler) << Keyword;

modocache wrote:
> modocache wrote:
> > lewissbaker wrote:
> > > modocache wrote:
> > > > EricWF wrote:
> > > > > We can still build a valid AST after encountering this error, no?
> > > > > 
> > > > > 
> > > > I believe so. Just to be clear: you'd like me to continue building the 
> > > > AST even after emitting this error diagnostic? My understanding is that 
> > > > most of this file bails soon after any error is encountered (correct me 
> > > > if that's wrong). I'm happy to change that, but I wonder if it'd be 
> > > > better to do that in a separate diff...?
> > > Do the scope flags reset when a lambda occurs within a catch-block?
> > > 
> > > eg. The following should still be valid.
> > > ```
> > > try { ... }
> > > catch (...) {
> > >   []() -> task { co_await foo(); }();
> > > }
> > > ```
> > I believe they're reset, the example you posted compiles fine with this 
> > patch. I'll add a test case specifically for this and confirm exactly where 
> > the scope flags are reset or ignored in the lambda case.
> When the parser encounters a lambda, it takes the path 
> `Parser::ParseLambdaExpression -> 
> Parser::ParseLambdaExpressionAfterIntroducer -> 
> Parser::ParseCompoundStatementBody`, which creates an inner scope for the 
> body of the lambda. This inner scope does not have the `Scope::CatchScope` 
> flag, so it doesn't result in the error.
> 
> Although, your question did make me realize that this compiles just fine, 
> even with this patch:
> 
> ```
> void example() {
>   try {
> throw;
>   } catch (...) {
> try {
>   co_await a;
> }
>   }
> }
> ```
> 
> But I believe this above case should also be treated as an error, right?
Yes, I believe that co_await within a try-block within a catch-block should not 
be allowed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59076



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

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Lewis Baker via Phabricator via cfe-commits]

lewissbaker added inline comments.



Comment at: test/SemaCXX/coroutines.cpp:299-311
   // FIXME: The spec says this is ill-formed.
   void operator=(CtorDtor&) {
 co_yield 0; // expected-error {{'co_yield' cannot be used in a copy 
assignment operator}}
   }
   void operator=(CtorDtor const &) {
 co_yield 0; // expected-error {{'co_yield' cannot be used in a copy 
assignment operator}}
   }

GorNishanov wrote:
> modocache wrote:
> > lewissbaker wrote:
> > > Not related to this diff, but...
> > > 
> > > I don't think that these should be ill-formed.
> > > 
> > > According to N4775 there are only exclusions added for [class.ctor] and 
> > > [class.dtor].
> > > I can't see anything in the spec that says that assignment special member 
> > > functions cannot be coroutines.
> > That's a great point. Could you create a Bugzilla for this work? And maybe 
> > we can get @GorNishanov's opinion?
> In 2015, such as https://wg21.link/N4499 there was a blank prohibition: 
> "A special member function shall not be a coroutine."
> 
> Later, at @rsmith suggestion, we relaxed it a little and banned only 
> constructors and destructors.
> 
> I am not yet aware of any use cases where such coroutines would be useful.
See https://bugs.llvm.org/show_bug.cgi?id=40997


Repository:
  rC Clang

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

https://reviews.llvm.org/D59076



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

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added inline comments.



Comment at: test/SemaCXX/coroutines.cpp:299-311
   // FIXME: The spec says this is ill-formed.
   void operator=(CtorDtor&) {
 co_yield 0; // expected-error {{'co_yield' cannot be used in a copy 
assignment operator}}
   }
   void operator=(CtorDtor const &) {
 co_yield 0; // expected-error {{'co_yield' cannot be used in a copy 
assignment operator}}
   }

modocache wrote:
> lewissbaker wrote:
> > Not related to this diff, but...
> > 
> > I don't think that these should be ill-formed.
> > 
> > According to N4775 there are only exclusions added for [class.ctor] and 
> > [class.dtor].
> > I can't see anything in the spec that says that assignment special member 
> > functions cannot be coroutines.
> That's a great point. Could you create a Bugzilla for this work? And maybe we 
> can get @GorNishanov's opinion?
In 2015, such as https://wg21.link/N4499 there was a blank prohibition: 
"A special member function shall not be a coroutine."

Later, at @rsmith suggestion, we relaxed it a little and banned only 
constructors and destructors.

I am not yet aware of any use cases where such coroutines would be useful.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59076



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

[PATCH] D58530: Add PragmaHandler for MSVC pragma execution_character_set

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Does our serialization machinery serialize these correctly automatically? What 
happens if you compiler a header saying `execution_character_set_push, 
"utf-8")` in a header, compile that to a pch, then include that through the pch 
in a .c file that does pop?

My guess is that this either needs (de)serialization code or a warning when the 
stack is not empty when a pch is written.

(Examples: https://reviews.llvm.org/rL262493 https://reviews.llvm.org/rL262539 
-- the latter one is probably a better example)


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

https://reviews.llvm.org/D58530



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

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/bugprone/IncompleteComparisonOperatorCheck.cpp:197
+
+  const auto Ops = utils::options::parseStringList(CheckedFunctions);
+  Finder->addMatcher(

Please don't use auto where return type is not obvious.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59103



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

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Brian Gesiak via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaCoroutine.cpp:675
+  // Second emphasis of [expr.await]p2: must be outside of an exception 
handler.
+  if (S.getCurScope()->getFlags() & Scope::CatchScope) {
+S.Diag(Loc, diag::err_coroutine_within_handler) << Keyword;

modocache wrote:
> lewissbaker wrote:
> > modocache wrote:
> > > EricWF wrote:
> > > > We can still build a valid AST after encountering this error, no?
> > > > 
> > > > 
> > > I believe so. Just to be clear: you'd like me to continue building the 
> > > AST even after emitting this error diagnostic? My understanding is that 
> > > most of this file bails soon after any error is encountered (correct me 
> > > if that's wrong). I'm happy to change that, but I wonder if it'd be 
> > > better to do that in a separate diff...?
> > Do the scope flags reset when a lambda occurs within a catch-block?
> > 
> > eg. The following should still be valid.
> > ```
> > try { ... }
> > catch (...) {
> >   []() -> task { co_await foo(); }();
> > }
> > ```
> I believe they're reset, the example you posted compiles fine with this 
> patch. I'll add a test case specifically for this and confirm exactly where 
> the scope flags are reset or ignored in the lambda case.
When the parser encounters a lambda, it takes the path 
`Parser::ParseLambdaExpression -> Parser::ParseLambdaExpressionAfterIntroducer 
-> Parser::ParseCompoundStatementBody`, which creates an inner scope for the 
body of the lambda. This inner scope does not have the `Scope::CatchScope` 
flag, so it doesn't result in the error.

Although, your question did make me realize that this compiles just fine, even 
with this patch:

```
void example() {
  try {
throw;
  } catch (...) {
try {
  co_await a;
}
  }
}
```

But I believe this above case should also be treated as an error, right?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59076



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

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Brian Gesiak via Phabricator via cfe-commits]

modocache updated this revision to Diff 189763.
modocache added a comment.

Add test case for executing a lambda using co_yield within a catch block.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59076

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Scope.h
  lib/Parse/ParseStmt.cpp
  lib/Sema/SemaCoroutine.cpp
  test/SemaCXX/coroutines.cpp

Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -328,6 +328,34 @@
 // not allowed. A user may not understand that this is "outside a function."
 void default_argument(int arg = co_await 0) {} // expected-error {{'co_await' cannot be used outside a function}}
 
+void await_in_catch_coroutine() {
+  try {
+  } catch (...) {
+co_await a; // expected-error {{'co_await' cannot be used in the handler of a try block}}
+  }
+}
+
+void await_in_lambda_in_catch_coroutine() {
+  try {
+  } catch (...) {
+[]() -> void { co_await a; }(); // OK
+  }
+}
+
+void yield_in_catch_coroutine() {
+  try {
+  } catch (...) {
+co_yield a; // expected-error {{'co_yield' cannot be used in the handler of a try block}}
+  }
+}
+
+void return_in_catch_coroutine() {
+  try {
+  } catch (...) {
+co_return 123; // OK
+  }
+}
+
 constexpr auto constexpr_deduced_return_coroutine() {
   co_yield 0; // expected-error {{'co_yield' cannot be used in a constexpr function}}
   // expected-error@-1 {{'co_yield' cannot be used in a function with a deduced return type}}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -185,21 +185,8 @@
 
 static bool isValidCoroutineContext(Sema , SourceLocation Loc,
 StringRef Keyword) {
-  // 'co_await' and 'co_yield' are not permitted in unevaluated operands,
-  // such as subexpressions of \c sizeof.
-  //
-  // [expr.await]p2, emphasis added: "An await-expression shall appear only in
-  // a *potentially evaluated* expression within the compound-statement of a
-  // function-body outside of a handler [...] A context within a function where
-  // an await-expression can appear is called a suspension context of the
-  // function." And per [expr.yield]p1: "A yield-expression shall appear only
-  // within a suspension context of a function."
-  if (S.isUnevaluatedContext()) {
-S.Diag(Loc, diag::err_coroutine_unevaluated_context) << Keyword;
-return false;
-  }
-
-  // Per [expr.await]p2, any other usage must be within a function.
+  // [expr.await]p2 dictates that 'co_await' and 'co_yield' must be used within
+  // a function body.
   // FIXME: This also covers [expr.await]p2: "An await-expression shall not
   // appear in a default argument." But the diagnostic QoI here could be
   // improved to inform the user that default arguments specifically are not
@@ -668,8 +655,34 @@
   return true;
 }
 
+// [expr.await]p2, emphasis added: "An await-expression shall appear only in
+// a *potentially evaluated* expression within the compound-statement of a
+// function-body *outside of a handler* [...] A context within a function
+// where an await-expression can appear is called a suspension context of the
+// function."
+static bool isValidSuspensionContext(Sema , SourceLocation Loc,
+ StringRef Keyword,
+ bool IsImplicit = false) {
+  // First emphasis of [expr.await]p2: must be a potentially evaluated context.
+  // That is, 'co_await' and 'co_yield' cannot appear in subexpressions of
+  // \c sizeof.
+  if (S.isUnevaluatedContext()) {
+S.Diag(Loc, diag::err_coroutine_unevaluated_context) << Keyword;
+return false;
+  }
+
+  // Second emphasis of [expr.await]p2: must be outside of an exception handler.
+  if (S.getCurScope()->getFlags() & Scope::CatchScope) {
+S.Diag(Loc, diag::err_coroutine_within_handler) << Keyword;
+return false;
+  }
+
+  return true;
+}
+
 ExprResult Sema::ActOnCoawaitExpr(Scope *S, SourceLocation Loc, Expr *E) {
-  if (!ActOnCoroutineBodyStart(S, Loc, "co_await")) {
+  if (!ActOnCoroutineBodyStart(S, Loc, "co_await") ||
+  !isValidSuspensionContext(*this, Loc, "co_await")) {
 CorrectDelayedTyposInExpr(E);
 return ExprError();
   }
@@ -689,7 +702,7 @@
 ExprResult Sema::BuildUnresolvedCoawaitExpr(SourceLocation Loc, Expr *E,
 UnresolvedLookupExpr *Lookup) {
   auto *FSI = checkCoroutineContext(*this, Loc, "co_await");
-  if (!FSI)
+  if (!FSI || !isValidSuspensionContext(*this, Loc, "co_await"))
 return ExprError();
 
   if (E->getType()->isPlaceholderType()) {
@@ -766,7 +779,8 @@
 }
 
 ExprResult Sema::ActOnCoyieldExpr(Scope *S, SourceLocation Loc, Expr *E) {
-  if (!ActOnCoroutineBodyStart(S, Loc, "co_yield")) {
+  if 

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

@lebedev.ri @kallehuttunen what do you think of putting this into context of 
the proposal (i believe its being standardized with c++20) of `operator==() = 
default` and the spaceship-operator.
This check could serve as a basis to modernize the `operator==` to the default 
if appropriate and warn/diagnose in the other cases.
WDYT?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59103



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

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Brian Gesiak via Phabricator via cfe-commits]

modocache added inline comments.



Comment at: lib/Sema/SemaCoroutine.cpp:197-200
-  if (S.isUnevaluatedContext()) {
-S.Diag(Loc, diag::err_coroutine_unevaluated_context) << Keyword;
-return false;
-  }

lewissbaker wrote:
> Does removing this check now mean that we're not checking that `co_return` 
> statements don't appear in unevaluated contexts?
> 
> Or is that already handled elsewhere by the fact that `co_return` statements 
> are not expressions and are therefore detected earlier as a grammar violation 
> when parsing `sizeof()` expression?
That's right! If one were to attempt to use a `co_return` within a 
subexpression of `sizeof`, they'd see `error: expected expression` before 
they'd ever have seen this error message. So I believe there's no need to 
perform this check for `co_return`, and I believe that's why the revisions to 
the standard included in the Coroutines TS don't make any special mention of 
disallowing `co_return` in those contexts.



Comment at: lib/Sema/SemaCoroutine.cpp:675
+  // Second emphasis of [expr.await]p2: must be outside of an exception 
handler.
+  if (S.getCurScope()->getFlags() & Scope::CatchScope) {
+S.Diag(Loc, diag::err_coroutine_within_handler) << Keyword;

lewissbaker wrote:
> modocache wrote:
> > EricWF wrote:
> > > We can still build a valid AST after encountering this error, no?
> > > 
> > > 
> > I believe so. Just to be clear: you'd like me to continue building the AST 
> > even after emitting this error diagnostic? My understanding is that most of 
> > this file bails soon after any error is encountered (correct me if that's 
> > wrong). I'm happy to change that, but I wonder if it'd be better to do that 
> > in a separate diff...?
> Do the scope flags reset when a lambda occurs within a catch-block?
> 
> eg. The following should still be valid.
> ```
> try { ... }
> catch (...) {
>   []() -> task { co_await foo(); }();
> }
> ```
I believe they're reset, the example you posted compiles fine with this patch. 
I'll add a test case specifically for this and confirm exactly where the scope 
flags are reset or ignored in the lambda case.



Comment at: test/SemaCXX/coroutines.cpp:299-311
   // FIXME: The spec says this is ill-formed.
   void operator=(CtorDtor&) {
 co_yield 0; // expected-error {{'co_yield' cannot be used in a copy 
assignment operator}}
   }
   void operator=(CtorDtor const &) {
 co_yield 0; // expected-error {{'co_yield' cannot be used in a copy 
assignment operator}}
   }

lewissbaker wrote:
> Not related to this diff, but...
> 
> I don't think that these should be ill-formed.
> 
> According to N4775 there are only exclusions added for [class.ctor] and 
> [class.dtor].
> I can't see anything in the spec that says that assignment special member 
> functions cannot be coroutines.
That's a great point. Could you create a Bugzilla for this work? And maybe we 
can get @GorNishanov's opinion?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59076



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

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I definately been burnt by not handling all the cases, but imagine a string 
class

class mystring
{

  const char *ptr;
  int size;
  int numallocated;

}

to compare the strings I wouldn't want to compare all the fields?

I think this could lead to a lot of false positives


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59103



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

[PATCH] D59094: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

It looks like every call to getABIKind() is doing the wrong thing; I'm worried 
that's going to lead to some sort of subtle miscompile without some sort of 
centralized fix to compute the correct calling convention for every place that 
checks it.  Not sure how to write a testcase off the top of my head... have you 
tried homogeneous aggregates with more than two elements?




Comment at: clang/test/CodeGenCXX/arm-pcs.cpp:4
+
+// RUN: %clang -mfloat-abi=hard --target=armv7-unknown-linux-gnueabi -O3 -S -o 
- %s | FileCheck %s -check-prefixes=HARD_INSTRs,CHECK
+// RUN: %clang -mfloat-abi=softfp --target=armv7-unknown-linux-gnueabi -O3 -S 
-o - %s | FileCheck %s -check-prefixes=HARD_INSTRs,CHECK

This test needs "// REQUIRES: arm-registered-target" so it's automatically 
disabled if the ARM target isn't available.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59094



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

r355627 - Recommit "Support attribute used in member funcs of class templates"

[Rafael Auler via cfe-commits]

Author: rafauler
Date: Thu Mar  7 11:14:30 2019
New Revision: 355627

URL: http://llvm.org/viewvc/llvm-project?rev=355627=rev
Log:
Recommit "Support attribute used in member funcs of class templates"

The patch originally broke code that was incompatible with GCC, but
we want to follow GCC behavior here according to the discussion in
https://reviews.llvm.org/D58216

Original commit message:
As PR17480 describes, clang does not support the used attribute
for member functions of class templates. This means that if the member
function is not used, its definition is never instantiated. This patch
changes clang to emit the definition if it has the used attribute.

Test Plan: Added a testcase

Reviewed By: aaron.ballman

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

Added:

cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
Modified:
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=355627=355626=355627=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Thu Mar  7 11:14:30 2019
@@ -2232,6 +2232,20 @@ TemplateDeclInstantiator::VisitCXXMethod
 Owner->addDecl(Method);
   }
 
+  // PR17480: Honor the used attribute to instantiate member function
+  // definitions
+  if (Method->hasAttr()) {
+if (const auto *A = dyn_cast(Owner)) {
+  SourceLocation Loc;
+  if (const MemberSpecializationInfo *MSInfo =
+  A->getMemberSpecializationInfo())
+Loc = MSInfo->getPointOfInstantiation();
+  else if (const auto *Spec = dyn_cast(A))
+Loc = Spec->getPointOfInstantiation();
+  SemaRef.MarkFunctionReferenced(Loc, Method);
+}
+  }
+
   return Method;
 }
 

Added: 
cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp?rev=355627=auto
==
--- 
cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp 
(added)
+++ 
cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp 
Thu Mar  7 11:14:30 2019
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | 
FileCheck %s
+
+// Check that PR17480 is fixed: __attribute__((used)) ignored in templated
+// classes
+namespace InstantiateUsedMemberDefinition {
+template 
+struct S {
+  int __attribute__((used)) f() {
+return 0;
+  }
+};
+
+void test() {
+  // Check that InstantiateUsedMemberDefinition::S::f() is defined
+  // as a result of the S class template implicit instantiation
+  // CHECK: define linkonce_odr i32 
@_ZN31InstantiateUsedMemberDefinition1SIiE1fEv
+  S inst;
+}
+} // namespace InstantiateUsedMemberDefinition


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

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/bugprone/IncompleteComparisonOperatorCheck.cpp:242-243
+diag(Operator->getBeginLoc(), "incomplete comparison operator");
+Lhs->diagFields(*this, Operator->getBeginLoc());
+Rhs->diagFields(*this, Operator->getBeginLoc());
+  }

These should point to the actual field decl location in the class


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59103



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

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

[Kalle Huttunen via Phabricator via cfe-commits]

kallehuttunen added a comment.

In D59103#1421875 , @lebedev.ri wrote:

> Thank you for working on this!
>
> This looks questionable to me.
>  Is this based on some coding standard?
>  Are there any numbers on the false-positive rate of theck?


It's not based on any coding standard, but is rather an attempt to eliminate 
one class of bugs (forgetting to update comparison operator when adding struct 
members) that I've encountered several times.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59103



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

r355625 - Fix some clang analysis tests passing arguments incorrectly

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Thu Mar  7 10:57:04 2019
New Revision: 355625

URL: http://llvm.org/viewvc/llvm-project?rev=355625=rev
Log:
Fix some clang analysis tests passing arguments incorrectly

Modified:
cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Modified: cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp?rev=355625=355624=355625=diff
==
--- cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp (original)
+++ cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp Thu Mar  7 
10:57:04 2019
@@ -933,14 +933,14 @@ TEST(ExprMutationAnalyzerTest, CommaExpr
   auto AST = buildASTFromCodeWithArgs(
   "template  struct S;"
   "template  void f() { S s; int x, y; s.mf((y, x)); }",
-  {"-fno-delayed-template-parsing -Wno-unused-value"});
+  {"-fno-delayed-template-parsing", "-Wno-unused-value"});
   auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_TRUE(isMutated(Results, AST.get()));
 
   AST = buildASTFromCodeWithArgs(
   "template  void f(T t) { int x, y; g(t, (y, x)); }",
-  {"-fno-delayed-template-parsing -Wno-unused-value"});
+  {"-fno-delayed-template-parsing", "-Wno-unused-value"});
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_TRUE(isMutated(Results, AST.get()));
 }
@@ -1006,7 +1006,7 @@ TEST(ExprMutationAnalyzerTest, CommaExpr
   UniquePtrDef + "template  void f() "
  "{ UniquePtr x; UniquePtr y;"
  " (y, x)->mf(); }",
-  {"-fno-delayed-template-parsing -Wno-unused-value"});
+  {"-fno-delayed-template-parsing", "-Wno-unused-value"});
   const auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_TRUE(isMutated(Results, AST.get()));


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

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

[Kalle Huttunen via Phabricator via cfe-commits]

kallehuttunen added a comment.

The checker gives quite many warnings on LLVM code base. For example, running 
it for lib/Transforms/Scalar:

  /home/kalle/llvm/lib/Transforms/Scalar/GVN.cpp:119:3: warning: incomplete 
comparison operator [bugprone-incomplete-comparison-operator]
bool operator==(const Expression ) const {
^
  /home/kalle/llvm/lib/Transforms/Scalar/GVN.cpp:119:3: note: commutative not 
accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVN.cpp:119:3: note: other.commutative 
not accessed
  
  /home/kalle/llvm/lib/Transforms/Scalar/GVNHoist.cpp:151:3: warning: 
incomplete comparison operator [bugprone-incomplete-comparison-operator]
bool operator==(const CHIArg ) { return VN == A.VN; }
^
  /home/kalle/llvm/lib/Transforms/Scalar/GVNHoist.cpp:151:3: note: Dest not 
accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVNHoist.cpp:151:3: note: I not 
accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVNHoist.cpp:151:3: note: A.Dest not 
accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVNHoist.cpp:151:3: note: A.I not 
accessed
  
  /home/kalle/llvm/lib/Transforms/Scalar/GVNHoist.cpp:152:3: warning: 
incomplete comparison operator [bugprone-incomplete-comparison-operator]
bool operator!=(const CHIArg ) { return !(*this == A); }
^
  /home/kalle/llvm/lib/Transforms/Scalar/GVNHoist.cpp:152:3: note: Dest not 
accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVNHoist.cpp:152:3: note: I not 
accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVNHoist.cpp:152:3: note: A.Dest not 
accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVNHoist.cpp:152:3: note: A.I not 
accessed
  
  /home/kalle/llvm/lib/Transforms/Scalar/GVNSink.cpp:211:3: warning: incomplete 
comparison operator [bugprone-incomplete-comparison-operator]
bool operator>(const SinkingInstructionCandidate ) const {
^
  /home/kalle/llvm/lib/Transforms/Scalar/GVNSink.cpp:211:3: note: NumBlocks not 
accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVNSink.cpp:211:3: note: 
NumInstructions not accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVNSink.cpp:211:3: note: NumPHIs not 
accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVNSink.cpp:211:3: note: 
NumMemoryInsts not accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVNSink.cpp:211:3: note: Blocks not 
accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVNSink.cpp:211:3: note: 
Other.NumBlocks not accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVNSink.cpp:211:3: note: 
Other.NumInstructions not accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVNSink.cpp:211:3: note: Other.NumPHIs 
not accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVNSink.cpp:211:3: note: 
Other.NumMemoryInsts not accessed
  /home/kalle/llvm/lib/Transforms/Scalar/GVNSink.cpp:211:3: note: Other.Blocks 
not accessed
  
  /home/kalle/llvm/lib/Transforms/Scalar/MergeICmps.cpp:90:3: warning: 
incomplete comparison operator [bugprone-incomplete-comparison-operator]
bool operator<(const BCEAtom ) const {
^
  /home/kalle/llvm/lib/Transforms/Scalar/MergeICmps.cpp:90:3: note: GEP not 
accessed
  /home/kalle/llvm/lib/Transforms/Scalar/MergeICmps.cpp:90:3: note: LoadI not 
accessed
  /home/kalle/llvm/lib/Transforms/Scalar/MergeICmps.cpp:90:3: note: O.GEP not 
accessed
  /home/kalle/llvm/lib/Transforms/Scalar/MergeICmps.cpp:90:3: note: O.LoadI not 
accessed
  
  /home/kalle/llvm/lib/Transforms/Scalar/NewGVN.cpp:436:3: warning: incomplete 
comparison operator [bugprone-incomplete-comparison-operator]
bool operator==(const Expression ) const {
^
  /home/kalle/llvm/lib/Transforms/Scalar/NewGVN.cpp:436:3: note: Other.Opcode 
not accessed
  /home/kalle/llvm/lib/Transforms/Scalar/NewGVN.cpp:436:3: note: Other.HashVal 
not accessed
  
  /home/kalle/llvm/lib/Transforms/Scalar/SROA.cpp:206:3: warning: incomplete 
comparison operator [bugprone-incomplete-comparison-operator]
friend LLVM_ATTRIBUTE_UNUSED bool operator<(const Slice ,
^
  /home/kalle/llvm/lib/Transforms/Scalar/SROA.cpp:206:3: note: LHS.EndOffset 
not accessed
  /home/kalle/llvm/lib/Transforms/Scalar/SROA.cpp:206:3: note: 
LHS.UseAndIsSplittable not accessed
  
  /home/kalle/llvm/lib/Transforms/Scalar/SROA.cpp:210:3: warning: incomplete 
comparison operator [bugprone-incomplete-comparison-operator]
friend LLVM_ATTRIBUTE_UNUSED bool operator<(uint64_t LHSOffset,
^
  /home/kalle/llvm/lib/Transforms/Scalar/SROA.cpp:210:3: note: RHS.EndOffset 
not accessed
  /home/kalle/llvm/lib/Transforms/Scalar/SROA.cpp:210:3: note: 
RHS.UseAndIsSplittable not accessed
  
  /home/kalle/llvm/lib/Transforms/Scalar/SROA.cpp:582:3: warning: incomplete 
comparison operator [bugprone-incomplete-comparison-operator]
bool operator==(const partition_iterator ) const {
^
  /home/kalle/llvm/lib/Transforms/Scalar/SROA.cpp:582:3: note: 
MaxSplitSliceEndOffset not accessed
  /home/kalle/llvm/lib/Transforms/Scalar/SROA.cpp:582:3: note: 
RHS.MaxSplitSliceEndOffset not accessed

Running the checker on 100k SLOC proprietary code base I'm 

[PATCH] D59034: Delete x86_64 ShadowCallStack support

[Vlad Tsyrklevich via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCRT355624: Delete x86_64 ShadowCallStack support (authored by 
vlad.tsyrklevich, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59034?vs=189750=189756#toc

Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D59034

Files:
  cmake/config-ix.cmake
  test/shadowcallstack/libc_support.h
  test/shadowcallstack/lit.cfg
  test/shadowcallstack/minimal_runtime.h
  test/shadowcallstack/overflow-aarch64.c
  test/shadowcallstack/overflow-x86_64.c
  test/shadowcallstack/overflow.c

Index: cmake/config-ix.cmake
===
--- cmake/config-ix.cmake
+++ cmake/config-ix.cmake
@@ -254,7 +254,7 @@
 else()
 set(ALL_XRAY_SUPPORTED_ARCH ${X86_64} ${ARM32} ${ARM64} ${MIPS32} ${MIPS64} powerpc64le)
 endif()
-set(ALL_SHADOWCALLSTACK_SUPPORTED_ARCH ${X86_64} ${ARM64})
+set(ALL_SHADOWCALLSTACK_SUPPORTED_ARCH ${ARM64})
 
 if(APPLE)
   include(CompilerRTDarwinUtils)
Index: test/shadowcallstack/minimal_runtime.h
===
--- test/shadowcallstack/minimal_runtime.h
+++ test/shadowcallstack/minimal_runtime.h
@@ -4,10 +4,6 @@
 
 #pragma once
 
-#ifdef __x86_64__
-#include 
-int arch_prctl(int code, void *addr);
-#endif
 #include 
 #include 
 #include 
@@ -21,10 +17,7 @@
   if (stack == MAP_FAILED)
 abort();
 
-#if defined(__x86_64__)
-  if (arch_prctl(ARCH_SET_GS, stack))
-abort();
-#elif defined(__aarch64__)
+#if defined(__aarch64__)
   __asm__ __volatile__("mov x18, %0" ::"r"(stack));
 #else
 #error Unsupported platform
Index: test/shadowcallstack/libc_support.h
===
--- test/shadowcallstack/libc_support.h
+++ test/shadowcallstack/libc_support.h
@@ -33,9 +33,5 @@
 }
 
 #else
-
-__attribute__((noinline)) void scs_fputs_stdout(const char *p) {
-  fputs(p, stdout);
-}
-
+#error Unsupported platform
 #endif
Index: test/shadowcallstack/lit.cfg
===
--- test/shadowcallstack/lit.cfg
+++ test/shadowcallstack/lit.cfg
@@ -19,5 +19,5 @@
   scs_arch_cflags += ' -ffixed-x18 '
 config.substitutions.append( ("%clang_scs ", config.clang + ' -O0 -fsanitize=shadow-call-stack ' + scs_arch_cflags + ' ') )
 
-if config.host_os not in ['Linux'] or config.target_arch not in ['x86_64', 'aarch64']:
+if config.host_os not in ['Linux'] or config.target_arch not in ['aarch64']:
config.unsupported = True
Index: test/shadowcallstack/overflow.c
===
--- test/shadowcallstack/overflow.c
+++ test/shadowcallstack/overflow.c
@@ -8,12 +8,10 @@
 // RUN: %clang_scs %s -o %t -DITERATIONS=3
 // RUN: %run %t | FileCheck %s
 
-// The behavioral check for SCS + overflow lives in the tests overflow-x86_64.c
-// and overflow-aarch64.c. This is because the expected behavior is different
-// between the two platforms. On x86_64 we crash because the comparison between
-// the shadow call stack and the regular stack fails. On aarch64 there is no
-// comparison, we just load the return address from the shadow call stack. So we
-// just expect not to see the output from print_and_exit.
+// On aarch64 we just load the return address from the shadow call stack so we
+// do not expect to see the output from print_and_exit.
+// RUN: %clang_scs %s -o %t -DITERATIONS=12
+// RUN: %run %t | FileCheck %S/overflow.c
 
 #include 
 #include 
Index: test/shadowcallstack/overflow-aarch64.c
===
--- test/shadowcallstack/overflow-aarch64.c
+++ test/shadowcallstack/overflow-aarch64.c
@@ -1,5 +0,0 @@
-// See overflow.c for a description.
-
-// REQUIRES: aarch64-target-arch
-// RUN: %clang_scs %S/overflow.c -o %t -DITERATIONS=12
-// RUN: %run %t | FileCheck %S/overflow.c
Index: test/shadowcallstack/overflow-x86_64.c
===
--- test/shadowcallstack/overflow-x86_64.c
+++ test/shadowcallstack/overflow-x86_64.c
@@ -1,5 +0,0 @@
-// See overflow.c for a description.
-
-// REQUIRES: x86_64-target-arch
-// RUN: %clang_scs %S/overflow.c -o %t -DITERATIONS=12
-// RUN: not --crash %run %t
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r355624 - Delete x86_64 ShadowCallStack support

[Vlad Tsyrklevich via cfe-commits]

Author: vlad.tsyrklevich
Date: Thu Mar  7 10:56:36 2019
New Revision: 355624

URL: http://llvm.org/viewvc/llvm-project?rev=355624=rev
Log:
Delete x86_64 ShadowCallStack support

Summary:
ShadowCallStack on x86_64 suffered from the same racy security issues as
Return Flow Guard and had performance overhead as high as 13% depending
on the benchmark. x86_64 ShadowCallStack was always an experimental
feature and never shipped a runtime required to support it, as such
there are no expected downstream users.

Reviewers: pcc

Reviewed By: pcc

Subscribers: mgorny, javed.absar, hiraditya, jdoerfert, cfe-commits, 
#sanitizers, llvm-commits

Tags: #clang, #sanitizers, #llvm

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

Modified:
cfe/trunk/docs/ShadowCallStack.rst

Modified: cfe/trunk/docs/ShadowCallStack.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ShadowCallStack.rst?rev=355624=355623=355624=diff
==
--- cfe/trunk/docs/ShadowCallStack.rst (original)
+++ cfe/trunk/docs/ShadowCallStack.rst Thu Mar  7 10:56:36 2019
@@ -9,7 +9,7 @@ Introduction
 
 
 ShadowCallStack is an instrumentation pass, currently only implemented for
-aarch64 and x86_64, that protects programs against return address overwrites
+aarch64, that protects programs against return address overwrites
 (e.g. stack buffer overflows.) It works by saving a function's return address
 to a separately allocated 'shadow call stack' in the function prolog in
 non-leaf functions and loading the return address from the shadow call stack
@@ -18,11 +18,10 @@ for compatibility with unwinders, but is
 
 The aarch64 implementation is considered production ready, and
 an `implementation of the runtime`_ has been added to Android's libc
-(bionic). The x86_64 implementation was evaluated using Chromium and was
-found to have critical performance and security deficiencies, and may be
-removed in a future release of the compiler. This document only describes
-the aarch64 implementation; details on the x86_64 implementation are found
-in the `Clang 7.0.1 documentation`_.
+(bionic). An x86_64 implementation was evaluated using Chromium and was found
+to have critical performance and security deficiencies--it was removed in
+LLVM 9.0. Details on the x86_64 implementation can be found in the
+`Clang 7.0.1 documentation`_.
 
 .. _`implementation of the runtime`: 
https://android.googlesource.com/platform/bionic/+/808d176e7e0dd727c7f929622ec017f6e065c582/libc/bionic/pthread_create.cpp#128
 .. _`Clang 7.0.1 documentation`: 
https://releases.llvm.org/7.0.1/tools/clang/docs/ShadowCallStack.html
@@ -37,10 +36,9 @@ consuming more memory for shorter functi
 memory accesses.
 
 `Return Flow Guard`_ is a pure software implementation of shadow call stacks
-on x86_64. It is similar to the ShadowCallStack x86_64 implementation but
-trades off higher memory usage for a shorter prologue and epilogue. Like
-x86_64 ShadowCallStack, it is inherently racy due to the architecture's use
-of the stack for calls and returns.
+on x86_64. Like the previous implementation of ShadowCallStack on x86_64, it is
+inherently racy due to the architecture's use of the stack for calls and
+returns.
 
 Intel `Control-flow Enforcement Technology`_ (CET) is a proposed hardware
 extension that would add native support to use a shadow stack to store/check


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

[PATCH] D58530: Add PragmaHandler for MSVC pragma execution_character_set

[Matt Gardner via Phabricator via cfe-commits]

sigatrev added a comment.

Yeah, that would be great. Thanks!


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

https://reviews.llvm.org/D58530



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

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Thank you for working on this!

This looks questionable to me.
Is this based on some coding standard?
Are there any numbers on the false-positive rate of theck?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59103



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

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

[Kalle Huttunen via Phabricator via cfe-commits]

kallehuttunen created this revision.
kallehuttunen added reviewers: alexfh, hokein, JonasToth, aaron.ballman.
kallehuttunen added a project: clang-tools-extra.
Herald added subscribers: jdoerfert, xazax.hun, mgorny.
Herald added a project: clang.

The checker detects comparison operators that don't access all fields of the 
compared types.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D59103

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/IncompleteComparisonOperatorCheck.cpp
  clang-tidy/bugprone/IncompleteComparisonOperatorCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-incomplete-comparison-operator.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-incomplete-comparison-operator-custom.cpp
  test/clang-tidy/bugprone-incomplete-comparison-operator.cpp

Index: test/clang-tidy/bugprone-incomplete-comparison-operator.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-incomplete-comparison-operator.cpp
@@ -0,0 +1,280 @@
+// RUN: %check_clang_tidy %s bugprone-incomplete-comparison-operator %t
+
+struct Good {
+  int Member1;
+  int Member2;
+};
+
+bool operator==(const Good , const Good ) {
+  return Lhs.Member1 == Rhs.Member1 && Lhs.Member2 == Rhs.Member2;
+}
+
+bool operator!=(const Good , const Good ) {
+  return !(Lhs == Rhs);
+}
+
+bool operator<(const Good , const Good ) {
+  if (Lhs.Member1 != Rhs.Member1)
+return Lhs.Member1 < Rhs.Member1;
+  return Lhs.Member2 < Rhs.Member2;
+}
+
+bool operator>(const Good , const Good ) {
+  return Rhs < Lhs;
+}
+
+bool operator<=(const Good , const Good ) {
+  return !(Rhs < Lhs);
+}
+
+bool operator>=(const Good , const Good ) {
+  return !(Lhs < Rhs);
+}
+
+struct Bad {
+  int Member1;
+  int Member2;
+};
+
+bool operator==(const Bad , const Bad ) {
+  // CHECK-NOTES: [[@LINE-1]]:1: warning: incomplete comparison operator
+  // CHECK-NOTES: [[@LINE-2]]:1: note: Lhs.Member2 not accessed
+  // CHECK-NOTES: [[@LINE-3]]:1: note: Rhs.Member2 not accessed
+  return Lhs.Member1 == Rhs.Member1;
+}
+
+bool operator!=(const Bad , const Bad ) {
+  // CHECK-NOTES: [[@LINE-1]]:1: warning: incomplete comparison operator
+  // CHECK-NOTES: [[@LINE-2]]:1: note: Lhs.Member2 not accessed
+  // CHECK-NOTES: [[@LINE-3]]:1: note: Rhs.Member2 not accessed
+  return !(Lhs == Rhs);
+}
+
+bool operator<(const Bad , const Bad ) {
+  // CHECK-NOTES: [[@LINE-1]]:1: warning: incomplete comparison operator
+  // CHECK-NOTES: [[@LINE-2]]:1: note: Lhs.Member1 not accessed
+  // CHECK-NOTES: [[@LINE-3]]:1: note: Rhs.Member1 not accessed
+  return Lhs.Member2 < Rhs.Member2;
+}
+
+bool operator>(const Bad , const Bad ) {
+  // CHECK-NOTES: [[@LINE-1]]:1: warning: incomplete comparison operator
+  // CHECK-NOTES: [[@LINE-2]]:1: note: Lhs.Member1 not accessed
+  // CHECK-NOTES: [[@LINE-3]]:1: note: Rhs.Member1 not accessed
+  return Rhs < Lhs;
+}
+
+bool operator<=(const Bad , const Bad ) {
+  // CHECK-NOTES: [[@LINE-1]]:1: warning: incomplete comparison operator
+  // CHECK-NOTES: [[@LINE-2]]:1: note: Lhs.Member1 not accessed
+  // CHECK-NOTES: [[@LINE-3]]:1: note: Rhs.Member1 not accessed
+  return !(Rhs < Lhs);
+}
+
+bool operator>=(const Bad , const Bad ) {
+  // CHECK-NOTES: [[@LINE-1]]:1: warning: incomplete comparison operator
+  // CHECK-NOTES: [[@LINE-2]]:1: note: Lhs.Member1 not accessed
+  // CHECK-NOTES: [[@LINE-3]]:1: note: Rhs.Member1 not accessed
+  return !(Lhs < Rhs);
+}
+
+struct Ugly {
+  Good GoodParts;
+  Bad BadParts;
+};
+
+// Missing field access for only one of the params -> warning
+bool operator==(const Ugly , const Ugly ) {
+  // CHECK-NOTES: [[@LINE-1]]:1: warning: incomplete comparison operator
+  // CHECK-NOTES: [[@LINE-2]]:1: note: Rhs.BadParts not accessed
+  return Lhs.GoodParts == Rhs.GoodParts && Lhs.BadParts == Lhs.BadParts;
+}
+
+// Args are passed to a function in another TU -> no warning
+bool testForInequality(const Ugly &, const Ugly &);
+bool operator!=(const Ugly , const Ugly ) {
+  return testForInequality(Lhs, Rhs);
+}
+
+// Fields are passed to a function in another TU, but some fields are not accessed -> warning
+bool compareGoodness(const Good &, const Good &);
+bool operator<(const Ugly , const Ugly ) {
+  // CHECK-NOTES: [[@LINE-1]]:1: warning: incomplete comparison operator
+  // CHECK-NOTES: [[@LINE-2]]:1: note: Lhs.BadParts not accessed
+  // CHECK-NOTES: [[@LINE-3]]:1: note: Rhs.BadParts not accessed
+  return compareGoodness(Lhs.GoodParts, Rhs.GoodParts);
+}
+
+// Args are passed to recursive function -> no warning
+bool strangeLoop(const Ugly ) { return strangeLoop(U); }
+bool operator>(const Ugly , const Ugly ) {
+  return strangeLoop(Lhs);
+}
+
+class Encapsulated {
+public:
+  int get1() const { return Member1; }
+  int get2() const { return Member2; }
+
+  // getters that are defined later in the TU
+  int query1() const;
+  int query2() const;
+
+private:
+ 

[PATCH] D58530: Add PragmaHandler for MSVC pragma execution_character_set

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm, thanks! Would you like someone to land this for you?


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

https://reviews.llvm.org/D58530



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

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Lewis Baker via Phabricator via cfe-commits]

lewissbaker added inline comments.



Comment at: lib/Sema/SemaCoroutine.cpp:197-200
-  if (S.isUnevaluatedContext()) {
-S.Diag(Loc, diag::err_coroutine_unevaluated_context) << Keyword;
-return false;
-  }

Does removing this check now mean that we're not checking that `co_return` 
statements don't appear in unevaluated contexts?

Or is that already handled elsewhere by the fact that `co_return` statements 
are not expressions and are therefore detected earlier as a grammar violation 
when parsing `sizeof()` expression?



Comment at: lib/Sema/SemaCoroutine.cpp:675
+  // Second emphasis of [expr.await]p2: must be outside of an exception 
handler.
+  if (S.getCurScope()->getFlags() & Scope::CatchScope) {
+S.Diag(Loc, diag::err_coroutine_within_handler) << Keyword;

modocache wrote:
> EricWF wrote:
> > We can still build a valid AST after encountering this error, no?
> > 
> > 
> I believe so. Just to be clear: you'd like me to continue building the AST 
> even after emitting this error diagnostic? My understanding is that most of 
> this file bails soon after any error is encountered (correct me if that's 
> wrong). I'm happy to change that, but I wonder if it'd be better to do that 
> in a separate diff...?
Do the scope flags reset when a lambda occurs within a catch-block?

eg. The following should still be valid.
```
try { ... }
catch (...) {
  []() -> task { co_await foo(); }();
}
```



Comment at: test/SemaCXX/coroutines.cpp:299-311
   // FIXME: The spec says this is ill-formed.
   void operator=(CtorDtor&) {
 co_yield 0; // expected-error {{'co_yield' cannot be used in a copy 
assignment operator}}
   }
   void operator=(CtorDtor const &) {
 co_yield 0; // expected-error {{'co_yield' cannot be used in a copy 
assignment operator}}
   }

Not related to this diff, but...

I don't think that these should be ill-formed.

According to N4775 there are only exclusions added for [class.ctor] and 
[class.dtor].
I can't see anything in the spec that says that assignment special member 
functions cannot be coroutines.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59076



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

[PATCH] D59034: Delete x86_64 ShadowCallStack support

[Vlad Tsyrklevich via Phabricator via cfe-commits]

vlad.tsyrklevich updated this revision to Diff 189750.
vlad.tsyrklevich added a comment.

- Keep x86_64 doc link


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59034

Files:
  clang/docs/ShadowCallStack.rst
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/test/shadowcallstack/libc_support.h
  compiler-rt/test/shadowcallstack/lit.cfg
  compiler-rt/test/shadowcallstack/minimal_runtime.h
  compiler-rt/test/shadowcallstack/overflow-aarch64.c
  compiler-rt/test/shadowcallstack/overflow-x86_64.c
  compiler-rt/test/shadowcallstack/overflow.c
  llvm/lib/Target/X86/CMakeLists.txt
  llvm/lib/Target/X86/ShadowCallStack.cpp
  llvm/lib/Target/X86/X86.h
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/O3-pipeline.ll
  llvm/test/CodeGen/X86/shadow-call-stack.mir
  llvm/utils/gn/secondary/llvm/lib/Target/X86/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/lib/Target/X86/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/lib/Target/X86/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/lib/Target/X86/BUILD.gn
@@ -76,7 +76,6 @@
 deps += [ ":X86GenFoldTables" ]
   }
   sources = [
-"ShadowCallStack.cpp",
 "X86AsmPrinter.cpp",
 "X86AvoidStoreForwardingBlocks.cpp",
 "X86CallFrameOptimization.cpp",
Index: llvm/test/CodeGen/X86/shadow-call-stack.mir
===
--- llvm/test/CodeGen/X86/shadow-call-stack.mir
+++ /dev/null
@@ -1,212 +0,0 @@
-# RUN: llc -mtriple=x86_64-unknown-linux-gnu -run-pass shadow-call-stack -verify-machineinstrs -o - %s | FileCheck %s
 |
-
-  define void @no_return() #0 { ret void }
-  define void @normal_return() #0 { ret void }
-  define void @normal_return_leaf_func() #0 { ret void }
-  define void @short_leaf_func() #0 { ret void }
-  define void @normal_tail_call() #0 { ret void }
-  define void @r11_tail_call() #0 { ret void }
-  define void @conditional_tail_call() #0 { ret void }
-  define void @r10_live_in() #0 { ret void }
-
-  attributes #0 = { shadowcallstack }
-
-...

-# CHECK-LABEL: name: no_return
-name: no_return
-tracksRegLiveness: true
-frameInfo:
-  adjustsStack: true # not a leaf function
-body: |
-  ; CHECK: bb.0:
-  bb.0:
-; CHECK-NEXT: $eax = MOV32ri 13
-$eax = MOV32ri 13
-...

-# CHECK-LABEL: name: normal_return
-name: normal_return
-tracksRegLiveness: true
-frameInfo:
-  adjustsStack: true # not a leaf function
-body: |
-  ; CHECK: bb.0:
-  bb.0:
-; CHECK: $r10 = MOV64rm $rsp, 1, $noreg, 0, $noreg
-; CHECK-NEXT: $r11 = XOR64rr undef $r11, undef $r11, implicit-def $eflags
-; CHECK-NEXT: ADD64mi8 $r11, 1, $noreg, 0, $gs, 8, implicit-def $eflags
-; CHECK-NEXT: $r11 = MOV64rm $r11, 1, $noreg, 0, $gs
-; CHECK-NEXT: MOV64mr $r11, 1, $noreg, 0, $gs, $r10
-; CHECK-NEXT: $eax = MOV32ri 13
-$eax = MOV32ri 13
-
-; CHECK-NEXT: $r11 = XOR64rr undef $r11, undef $r11, implicit-def $eflags
-; CHECK-NEXT: $r10 = MOV64rm $r11, 1, $noreg, 0, $gs
-; CHECK-NEXT: $r10 = MOV64rm $r10, 1, $noreg, 0, $gs
-; CHECK-NEXT: SUB64mi8 $r11, 1, $noreg, 0, $gs, 8, implicit-def $eflags
-; CHECK-NEXT: CMP64mr $rsp, 1, $noreg, 0, $noreg, $r10, implicit-def $eflags
-; CHECK-NEXT: JNE_1 %bb.1, implicit $eflags
-; CHECK-NEXT: RETQ $eax
-RETQ $eax
-
-; CHECK: bb.1:
-; CHECK-NEXT; TRAP
-...

-# CHECK-LABEL: name: normal_return_leaf_func
-name: normal_return_leaf_func
-tracksRegLiveness: true
-frameInfo:
-  adjustsStack: false # leaf function
-body: |
-  ; CHECK: bb.0:
-  ; CHECK: liveins: $rcx
-  bb.0:
-liveins: $rcx
-
-; CHECK: $rdx = MOV64rm $rsp, 1, $noreg, 0, $noreg
-; CHECK-NEXT: $eax = MOV32ri 0
-$eax = MOV32ri 0
-; CHECK-NEXT: CMP64ri8 $rcx, 5, implicit-def $eflags
-CMP64ri8 $rcx, 5, implicit-def $eflags
-; CHECK-NEXT: JA_1 %bb.1, implicit $eflags
-JA_1 %bb.1, implicit $eflags
-; CHECK-NEXT: JMP_1 %bb.2
-JMP_1 %bb.2
-
-  ; CHECK: bb.1
-  ; CHECK: liveins: $eax, $rdx
-  bb.1:
-liveins: $eax
-
-; CHECKT: $eax = MOV32ri 1
-$eax = MOV32ri 1
-
-  ; CHECK: bb.2
-  ; CHECK: liveins: $eax, $rdx
-  bb.2:
-liveins: $eax
-
-; CHECK: CMP64mr $rsp, 1, $noreg, 0, $noreg, $rdx, implicit-def $eflags
-; CHECK-NEXT: JNE_1 %bb.3, implicit $eflags
-; CHECK-NEXT: RETQ $eax
-RETQ $eax
-
-; CHECK: bb.3:
-; CHECK-NEXT; TRAP
-...

-# CHECK-LABEL: name: short_leaf_func
-name: short_leaf_func
-tracksRegLiveness: true
-frameInfo:
-  adjustsStack: false # leaf function
-body: |
-  ; CHECK: bb.0:
-  bb.0:
-; Ensure these are not counted as machine instructions
-CFI_INSTRUCTION 0
-CFI_INSTRUCTION 0
-CFI_INSTRUCTION 0
-DBG_VALUE 0
-DBG_VALUE 0
-DBG_VALUE 0
-
-; CHECK: $eax = MOV32ri 13
-$eax = MOV32ri 13
-
-; CHECK-NEXT: RETQ $eax
-RETQ $eax
-...

-# CHECK-LABEL: 

[PATCH] D58814: [clang][Index] Constructors and Destructors do not reference class

[Nathan Hawes via Phabricator via cfe-commits]

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

Thank you!


Repository:
  rC Clang

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

https://reviews.llvm.org/D58814



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

r355605 - [analyzer] handle modification of vars inside an expr with comma operator

[Petar Jovanovic via cfe-commits]

Author: petarj
Date: Thu Mar  7 07:50:52 2019
New Revision: 355605

URL: http://llvm.org/viewvc/llvm-project?rev=355605=rev
Log:
[analyzer] handle modification of vars inside an expr with comma operator

We should track mutation of a variable within a comma operator expression.
Current code in ExprMutationAnalyzer does not handle it.

This will handle cases like:

(a, b) ++ < == b is modified
(a, b) = c < == b is modifed


Patch by Djordje Todorovic.

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

Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=355605=355604=355605=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Mar  7 07:50:52 2019
@@ -3403,6 +3403,9 @@ public:
   static bool isComparisonOp(Opcode Opc) { return Opc >= BO_Cmp && Opc<=BO_NE; 
}
   bool isComparisonOp() const { return isComparisonOp(getOpcode()); }
 
+  static bool isCommaOp(Opcode Opc) { return Opc == BO_Comma; }
+  bool isCommaOp() const { return isCommaOp(getOpcode()); }
+
   static Opcode negateComparisonOp(Opcode Opc) {
 switch (Opc) {
 default:

Modified: cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp?rev=355605=355604=355605=diff
==
--- cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp (original)
+++ cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp Thu Mar  7 07:50:52 2019
@@ -24,6 +24,18 @@ AST_MATCHER_P(CXXForRangeStmt, hasRangeS
   return InnerMatcher.matches(*Range, Finder, Builder);
 }
 
+AST_MATCHER_P(Expr, maybeEvalCommaExpr,
+ ast_matchers::internal::Matcher, InnerMatcher) {
+  const Expr* Result = 
+  while (const auto *BOComma =
+   dyn_cast_or_null(Result->IgnoreParens())) {
+if (!BOComma->isCommaOp())
+  break;
+Result = BOComma->getRHS();
+  }
+  return InnerMatcher.matches(*Result, Finder, Builder);
+}
+
 const ast_matchers::internal::VariadicDynCastAllOfMatcher
 cxxTypeidExpr;
 
@@ -193,24 +205,28 @@ const Stmt *ExprMutationAnalyzer::findDe
 const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
   // LHS of any assignment operators.
   const auto AsAssignmentLhs =
-  binaryOperator(isAssignmentOperator(), hasLHS(equalsNode(Exp)));
+  binaryOperator(isAssignmentOperator(),
+ hasLHS(maybeEvalCommaExpr(equalsNode(Exp;
 
   // Operand of increment/decrement operators.
   const auto AsIncDecOperand =
   unaryOperator(anyOf(hasOperatorName("++"), hasOperatorName("--")),
-hasUnaryOperand(equalsNode(Exp)));
+hasUnaryOperand(maybeEvalCommaExpr(equalsNode(Exp;
 
   // Invoking non-const member function.
   // A member function is assumed to be non-const when it is unresolved.
   const auto NonConstMethod = cxxMethodDecl(unless(isConst()));
   const auto AsNonConstThis =
-  expr(anyOf(cxxMemberCallExpr(callee(NonConstMethod), 
on(equalsNode(Exp))),
+  expr(anyOf(cxxMemberCallExpr(callee(NonConstMethod),
+   on(maybeEvalCommaExpr(equalsNode(Exp,
  cxxOperatorCallExpr(callee(NonConstMethod),
- hasArgument(0, equalsNode(Exp))),
+ hasArgument(0,
+ 
maybeEvalCommaExpr(equalsNode(Exp,
  callExpr(callee(expr(anyOf(
- 
unresolvedMemberExpr(hasObjectExpression(equalsNode(Exp))),
+ unresolvedMemberExpr(
+   
hasObjectExpression(maybeEvalCommaExpr(equalsNode(Exp,
  cxxDependentScopeMemberExpr(
- hasObjectExpression(equalsNode(Exp);
+ 
hasObjectExpression(maybeEvalCommaExpr(equalsNode(Exp));
 
   // Taking address of 'Exp'.
   // We're assuming 'Exp' is mutated as soon as its address is taken, though in
@@ -220,10 +236,11 @@ const Stmt *ExprMutationAnalyzer::findDi
   unaryOperator(hasOperatorName("&"),
 // A NoOp implicit cast is adding const.
 unless(hasParent(implicitCastExpr(hasCastKind(CK_NoOp,
-hasUnaryOperand(equalsNode(Exp)));
+hasUnaryOperand(maybeEvalCommaExpr(equalsNode(Exp;
   const auto AsPointerFromArrayDecay =
   castExpr(hasCastKind(CK_ArrayToPointerDecay),
-   unless(hasParent(arraySubscriptExpr())), has(equalsNode(Exp)));
+   unless(hasParent(arraySubscriptExpr())),
+   

r355616 - Rollback of rL355585.

[Mitch Phillips via cfe-commits]

Author: hctim
Date: Thu Mar  7 10:13:39 2019
New Revision: 355616

URL: http://llvm.org/viewvc/llvm-project?rev=355616=rev
Log:
Rollback of rL355585.

Introduces memory leak in FunctionTest.GetPointerAlignment that breaks 
sanitizer buildbots:

```
=
==2453==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 128 byte(s) in 1 object(s) allocated from:
#0 0x610428 in operator new(unsigned long) 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:105
#1 0x16936bc in llvm::User::operator new(unsigned long) 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/lib/IR/User.cpp:151:19
#2 0x7c3fe9 in Create 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/include/llvm/IR/Function.h:144:12
#3 0x7c3fe9 in (anonymous 
namespace)::FunctionTest_GetPointerAlignment_Test::TestBody() 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/unittests/IR/FunctionTest.cpp:136
#4 0x1a836a0 in HandleExceptionsInMethodIfSupported 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc
#5 0x1a836a0 in testing::Test::Run() 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:2474
#6 0x1a85c55 in testing::TestInfo::Run() 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:2656:11
#7 0x1a870d0 in testing::TestCase::Run() 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:2774:28
#8 0x1aa5b84 in testing::internal::UnitTestImpl::RunAllTests() 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:4649:43
#9 0x1aa4d30 in 
HandleExceptionsInMethodIfSupported 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc
#10 0x1aa4d30 in testing::UnitTest::Run() 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:4257
#11 0x1a6b656 in RUN_ALL_TESTS 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46
#12 0x1a6b656 in main 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/UnitTestMain/TestMain.cpp:50
#13 0x7f5af37a22e0 in __libc_start_main 
(/lib/x86_64-linux-gnu/libc.so.6+0x202e0)

Indirect leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x610428 in operator new(unsigned long) 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:105
#1 0x151be6b in make_unique 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/include/llvm/ADT/STLExtras.h:1349:29
#2 0x151be6b in llvm::Function::Function(llvm::FunctionType*, 
llvm::GlobalValue::LinkageTypes, unsigned int, llvm::Twine const&, 
llvm::Module*) 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/lib/IR/Function.cpp:241
#3 0x7c4006 in Create 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/include/llvm/IR/Function.h:144:16
#4 0x7c4006 in (anonymous 
namespace)::FunctionTest_GetPointerAlignment_Test::TestBody() 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/unittests/IR/FunctionTest.cpp:136
#5 0x1a836a0 in HandleExceptionsInMethodIfSupported 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc
#6 0x1a836a0 in testing::Test::Run() 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:2474
#7 0x1a85c55 in testing::TestInfo::Run() 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:2656:11
#8 0x1a870d0 in testing::TestCase::Run() 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:2774:28
#9 0x1aa5b84 in testing::internal::UnitTestImpl::RunAllTests() 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:4649:43
#10 0x1aa4d30 in 
HandleExceptionsInMethodIfSupported 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc
#11 0x1aa4d30 in testing::UnitTest::Run() 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:4257
#12 0x1a6b656 in RUN_ALL_TESTS 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46
#13 0x1a6b656 in main 
/b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/UnitTestMain/TestMain.cpp:50
#14 0x7f5af37a22e0 in __libc_start_main 
(/lib/x86_64-linux-gnu/libc.so.6+0x202e0)

SUMMARY: AddressSanitizer: 168 byte(s) leaked in 2 allocation(s).
```

See 
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/11358/steps/check-llvm%20asan/logs/stdio
 for more information.

Also introduces use-of-uninitialized-value in 
ConstantsTest.FoldGlobalVariablePtr:
```
==7070==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x14e703c in User 
/b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/IR/User.h:79:5
#1 0x14e703c in Constant 

r355614 - [OPENMP 5.0]Add initial support for 'allocate' directive.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Thu Mar  7 09:54:44 2019
New Revision: 355614

URL: http://llvm.org/viewvc/llvm-project?rev=355614=rev
Log:
[OPENMP 5.0]Add initial support for 'allocate' directive.

Added parsing/sema analysis/serialization/deserialization support for
'allocate' directive.

Added:
cfe/trunk/test/OpenMP/allocate_ast_print.cpp
cfe/trunk/test/OpenMP/allocate_messages.cpp
cfe/trunk/test/PCH/chain-openmp-allocate.cpp
Modified:
cfe/trunk/include/clang/AST/ASTMutationListener.h
cfe/trunk/include/clang/AST/ASTNodeTraverser.h
cfe/trunk/include/clang/AST/DeclOpenMP.h
cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/DeclNodes.td
cfe/trunk/include/clang/Basic/OpenMPKinds.def
cfe/trunk/include/clang/Basic/OpenMPKinds.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/include/clang/Serialization/ASTBitCodes.h
cfe/trunk/include/clang/Serialization/ASTWriter.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/AST/DeclOpenMP.cpp
cfe/trunk/lib/AST/DeclPrinter.cpp
cfe/trunk/lib/AST/OpenMPClause.cpp
cfe/trunk/lib/Basic/OpenMPKinds.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/Frontend/MultiplexConsumer.cpp
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/lib/Parse/Parser.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/lib/Serialization/ASTCommon.cpp
cfe/trunk/lib/Serialization/ASTCommon.h
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
cfe/trunk/tools/libclang/CIndex.cpp

Modified: cfe/trunk/include/clang/AST/ASTMutationListener.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTMutationListener.h?rev=355614=355613=355614=diff
==
--- cfe/trunk/include/clang/AST/ASTMutationListener.h (original)
+++ cfe/trunk/include/clang/AST/ASTMutationListener.h Thu Mar  7 09:54:44 2019
@@ -127,6 +127,11 @@ public:
   virtual void DeclarationMarkedOpenMPDeclareTarget(const Decl *D,
 const Attr *Attr) {}
 
+  /// A declaration is marked as a variable with OpenMP allocator.
+  ///
+  /// \param D the declaration marked as a variable with OpenMP allocator.
+  virtual void DeclarationMarkedOpenMPAllocate(const Decl *D, const Attr *A) {}
+
   /// A definition has been made visible by being redefined locally.
   ///
   /// \param D The definition that was previously not visible.

Modified: cfe/trunk/include/clang/AST/ASTNodeTraverser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTNodeTraverser.h?rev=355614=355613=355614=diff
==
--- cfe/trunk/include/clang/AST/ASTNodeTraverser.h (original)
+++ cfe/trunk/include/clang/AST/ASTNodeTraverser.h Thu Mar  7 09:54:44 2019
@@ -393,6 +393,11 @@ public:
 Visit(D->getInit());
   }
 
+  void VisitOMPAllocateDecl(const OMPAllocateDecl *D) {
+for (const auto *E : D->varlists())
+  Visit(E);
+  }
+
   template 
   void dumpTemplateDeclSpecialization(const SpecializationDecl *D) {
 for (const auto *RedeclWithBadType : D->redecls()) {

Modified: cfe/trunk/include/clang/AST/DeclOpenMP.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclOpenMP.h?rev=355614=355613=355614=diff
==
--- cfe/trunk/include/clang/AST/DeclOpenMP.h (original)
+++ cfe/trunk/include/clang/AST/DeclOpenMP.h Thu Mar  7 09:54:44 2019
@@ -405,6 +405,73 @@ public:
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K == OMPRequires; }
 };
+
+/// This represents '#pragma omp allocate ...' directive.
+/// For example, in the following, the default allocator is used for both 'a'
+/// and 'A::b':
+///
+/// \code
+/// int a;
+/// #pragma omp allocate(a)
+/// struct A {
+///   static int b;
+/// #pragma omp allocate(b)
+/// };
+/// \endcode
+///
+class OMPAllocateDecl final
+: public Decl,
+  private llvm::TrailingObjects {
+  friend class ASTDeclReader;
+  friend TrailingObjects;
+
+  /// Number of variable within the allocate directive.
+  unsigned NumVars = 0;
+
+  virtual void anchor();
+
+  OMPAllocateDecl(Kind DK, DeclContext *DC, SourceLocation L)
+  : Decl(DK, DC, L) {}
+
+  ArrayRef getVars() const {
+return llvm::makeArrayRef(getTrailingObjects(), NumVars);
+  }
+
+  MutableArrayRef getVars() {
+return 

[PATCH] D59086: [clangd] Adjust compile commands to be applicable for tooling

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clangd/GlobalCompilationDatabase.cpp:24
  llvm::StringRef ResourceDir) {
+  // Clangd does not generate dependency file.
+  Cmd.CommandLine = tooling::getClangStripDependencyFileAdjuster()(

kadircet wrote:
> gribozavr wrote:
> > Please don't duplicate code in comments. Explain why it shouldn't generate 
> > the dependency files, or drop the comment.
> > 
> > 
> I am saying we are stripping "dependecy file related" commands, because 
> clangd is not producing them. 
> I feel like this provides the cause of the following statement, not 
> replicates it. Becuase the following statement just says strip "dependency 
> file related" commands.
> 
> But happy to drop the comment if you insist.
What about "clangd should not write files to disk, including dependency files 
requested on the command line."


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59086



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

[PATCH] D59086: [clangd] Adjust compile commands to be applicable for tooling

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clangd/GlobalCompilationDatabase.cpp:24
  llvm::StringRef ResourceDir) {
+  // Clangd does not generate dependency file.
+  Cmd.CommandLine = tooling::getClangStripDependencyFileAdjuster()(

gribozavr wrote:
> Please don't duplicate code in comments. Explain why it shouldn't generate 
> the dependency files, or drop the comment.
> 
> 
I am saying we are stripping "dependecy file related" commands, because clangd 
is not producing them. 
I feel like this provides the cause of the following statement, not replicates 
it. Becuase the following statement just says strip "dependency file related" 
commands.

But happy to drop the comment if you insist.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59086



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

[PATCH] D58216: Support attribute used in member funcs of class templates II

[Bob Wilson via Phabricator via cfe-commits]

bob.wilson added a comment.

OK, that doesn't sound like it will be a problem


Repository:
  rC Clang

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

https://reviews.llvm.org/D58216



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

[PATCH] D58708: [PR40778] Preserve addr space in Derived to Base cast

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia added a comment.

Committed in r355606.


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

https://reviews.llvm.org/D58708



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

r355609 - [Sema] Change addr space diagnostics in casts to follow C++ style.

[Anastasia Stulova via cfe-commits]

Author: stulova
Date: Thu Mar  7 09:06:30 2019
New Revision: 355609

URL: http://llvm.org/viewvc/llvm-project?rev=355609=rev
Log:
[Sema] Change addr space diagnostics in casts to follow C++ style.

This change adds a new diagnostic for mismatching address spaces
to be used for C++ casts (only enabled in C style cast for now,
the rest will follow!).

The change extends C-style cast rules to account for address spaces.
It also adds a separate function for address space cast checking that
can be used to map from a separate address space cast operator
addrspace_cast (to be added as a follow up patch).

Note, that after this change clang will no longer allows arbitrary
address space conversions in reinterpret_casts because they can lead
to accidental errors. The implicit safe conversions would still be
allowed.

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


Added:
cfe/trunk/test/CodeGenOpenCLCXX/address-space-castoperators.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaCast.cpp
cfe/trunk/test/SemaCXX/address-space-conversion.cpp
cfe/trunk/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
cfe/trunk/test/SemaOpenCL/address-spaces.cl

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=355609=355608=355609=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Mar  7 09:06:30 
2019
@@ -6271,6 +6271,10 @@ def err_bad_cxx_cast_bitfield : Error<
 def err_bad_cxx_cast_qualifiers_away : Error<
   "%select{const_cast|static_cast|reinterpret_cast|dynamic_cast|C-style cast|"
   "functional-style cast}0 from %1 to %2 casts away qualifiers">;
+def err_bad_cxx_cast_addr_space_mismatch : Error<
+  "%select{const_cast|static_cast|reinterpret_cast|dynamic_cast|C-style cast|"
+  "functional-style cast}0 from %1 to %2 converts between mismatching address"
+  " spaces">;
 def ext_bad_cxx_cast_qualifiers_away_incoherent : ExtWarn<
   "ISO C++ does not allow "
   "%select{const_cast|static_cast|reinterpret_cast|dynamic_cast|C-style cast|"

Modified: cfe/trunk/lib/Sema/SemaCast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=355609=355608=355609=diff
==
--- cfe/trunk/lib/Sema/SemaCast.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCast.cpp Thu Mar  7 09:06:30 2019
@@ -2212,7 +2212,15 @@ static TryCastResult TryReinterpretCast(
  /*CheckObjCLifetime=*/CStyle))
 SuccessResult = getCastAwayConstnessCastKind(CACK, msg);
 
-  if (IsLValueCast) {
+  if (IsAddressSpaceConversion(SrcType, DestType)) {
+Kind = CK_AddressSpaceConversion;
+assert(SrcType->isPointerType() && DestType->isPointerType());
+if (!CStyle &&
+!DestType->getPointeeType().getQualifiers().isAddressSpaceSupersetOf(
+SrcType->getPointeeType().getQualifiers())) {
+  SuccessResult = TC_Failed;
+}
+  } else if (IsLValueCast) {
 Kind = CK_LValueBitCast;
   } else if (DestType->isObjCObjectPointerType()) {
 Kind = Self.PrepareCastToObjCObjectPointer(SrcExpr);
@@ -,8 +2230,6 @@ static TryCastResult TryReinterpretCast(
 } else {
   Kind = CK_BitCast;
 }
-  } else if (IsAddressSpaceConversion(SrcType, DestType)) {
-Kind = CK_AddressSpaceConversion;
   } else {
 Kind = CK_BitCast;
   }
@@ -2278,6 +2284,41 @@ static TryCastResult TryReinterpretCast(
   return SuccessResult;
 }
 
+static TryCastResult TryAddressSpaceCast(Sema , ExprResult ,
+ QualType DestType, bool CStyle,
+ unsigned ) {
+  if (!Self.getLangOpts().OpenCL)
+// FIXME: As compiler doesn't have any information about overlapping addr
+// spaces at the moment we have to be permissive here.
+return TC_NotApplicable;
+  // Even though the logic below is general enough and can be applied to
+  // non-OpenCL mode too, we fast-path above because no other languages
+  // define overlapping address spaces currently.
+  auto SrcType = SrcExpr.get()->getType();
+  auto SrcPtrType = SrcType->getAs();
+  if (!SrcPtrType)
+return TC_NotApplicable;
+  auto DestPtrType = DestType->getAs();
+  if (!DestPtrType)
+return TC_NotApplicable;
+  auto SrcPointeeType = SrcPtrType->getPointeeType();
+  auto DestPointeeType = DestPtrType->getPointeeType();
+  if (SrcPointeeType.getAddressSpace() == DestPointeeType.getAddressSpace())
+return TC_NotApplicable;
+  if (!DestPtrType->isAddressSpaceOverlapping(*SrcPtrType)) {
+msg = diag::err_bad_cxx_cast_addr_space_mismatch;
+return TC_Failed;
+  }
+  auto SrcPointeeTypeWithoutAS =
+  

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355609: [Sema] Change addr space diagnostics in casts to 
follow C++ style. (authored by stulova, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58346?vs=189474=189731#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58346

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/SemaCast.cpp
  cfe/trunk/test/CodeGenOpenCLCXX/address-space-castoperators.cpp
  cfe/trunk/test/SemaCXX/address-space-conversion.cpp
  cfe/trunk/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
  cfe/trunk/test/SemaOpenCL/address-spaces.cl

Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6271,6 +6271,10 @@
 def err_bad_cxx_cast_qualifiers_away : Error<
   "%select{const_cast|static_cast|reinterpret_cast|dynamic_cast|C-style cast|"
   "functional-style cast}0 from %1 to %2 casts away qualifiers">;
+def err_bad_cxx_cast_addr_space_mismatch : Error<
+  "%select{const_cast|static_cast|reinterpret_cast|dynamic_cast|C-style cast|"
+  "functional-style cast}0 from %1 to %2 converts between mismatching address"
+  " spaces">;
 def ext_bad_cxx_cast_qualifiers_away_incoherent : ExtWarn<
   "ISO C++ does not allow "
   "%select{const_cast|static_cast|reinterpret_cast|dynamic_cast|C-style cast|"
Index: cfe/trunk/test/SemaCXX/address-space-conversion.cpp
===
--- cfe/trunk/test/SemaCXX/address-space-conversion.cpp
+++ cfe/trunk/test/SemaCXX/address-space-conversion.cpp
@@ -131,24 +131,24 @@
 void test_reinterpret_cast(void_ptr vp, void_ptr_1 vp1, void_ptr_2 vp2,
A_ptr ap, A_ptr_1 ap1, A_ptr_2 ap2,
B_ptr bp, B_ptr_1 bp1, B_ptr_2 bp2,
-   const void __attribute__((address_space(1))) *cvp1) {
-  // reinterpret_cast can be used to cast to a different address space.
-  (void)reinterpret_cast(ap1);
-  (void)reinterpret_cast(ap2);
+   const void __attribute__((address_space(1))) * cvp1) {
+  // reinterpret_cast can't be used to cast to a different address space unless they are matching (i.e. overlapping).
+  (void)reinterpret_cast(ap1); // expected-error{{reinterpret_cast from 'A_ptr_1' (aka '__attribute__((address_space(1))) A *') to 'A_ptr' (aka 'A *') is not allowed}}
+  (void)reinterpret_cast(ap2); // expected-error{{reinterpret_cast from 'A_ptr_2' (aka '__attribute__((address_space(2))) A *') to 'A_ptr' (aka 'A *') is not allowed}}
   (void)reinterpret_cast(bp);
-  (void)reinterpret_cast(bp1);
-  (void)reinterpret_cast(bp2);
+  (void)reinterpret_cast(bp1); // expected-error{{reinterpret_cast from 'B_ptr_1' (aka '__attribute__((address_space(1))) B *') to 'A_ptr' (aka 'A *') is not allowed}}
+  (void)reinterpret_cast(bp2); // expected-error{{reinterpret_cast from 'B_ptr_2' (aka '__attribute__((address_space(2))) B *') to 'A_ptr' (aka 'A *') is not allowed}}
   (void)reinterpret_cast(vp);
-  (void)reinterpret_cast(vp1);
-  (void)reinterpret_cast(vp2);
-  (void)reinterpret_cast(ap);
-  (void)reinterpret_cast(ap2);
-  (void)reinterpret_cast(bp);
+  (void)reinterpret_cast(vp1);   // expected-error{{reinterpret_cast from 'void_ptr_1' (aka '__attribute__((address_space(1))) void *') to 'A_ptr' (aka 'A *') is not allowed}}
+  (void)reinterpret_cast(vp2);   // expected-error{{reinterpret_cast from 'void_ptr_2' (aka '__attribute__((address_space(2))) void *') to 'A_ptr' (aka 'A *') is not allowed}}
+  (void)reinterpret_cast(ap);  // expected-error{{reinterpret_cast from 'A_ptr' (aka 'A *') to 'A_ptr_1' (aka '__attribute__((address_space(1))) A *') is not allowed}}
+  (void)reinterpret_cast(ap2); // expected-error{{reinterpret_cast from 'A_ptr_2' (aka '__attribute__((address_space(2))) A *') to 'A_ptr_1' (aka '__attribute__((address_space(1))) A *') is not allowed}}
+  (void)reinterpret_cast(bp);  // expected-error{{reinterpret_cast from 'B_ptr' (aka 'B *') to 'A_ptr_1' (aka '__attribute__((address_space(1))) A *') is not allowed}}
   (void)reinterpret_cast(bp1);
-  (void)reinterpret_cast(bp2);
-  (void)reinterpret_cast(vp);
+  (void)reinterpret_cast(bp2); // expected-error{{reinterpret_cast from 'B_ptr_2' (aka '__attribute__((address_space(2))) B *') to 'A_ptr_1' (aka '__attribute__((address_space(1))) A *') is not allowed}}
+  (void)reinterpret_cast(vp);  // expected-error{{reinterpret_cast from 'void_ptr' (aka 'void *') to 'A_ptr_1' (aka '__attribute__((address_space(1))) A *') is not allowed}}
   (void)reinterpret_cast(vp1);
-  (void)reinterpret_cast(vp2);
+  (void)reinterpret_cast(vp2); // 

[PATCH] D58719: [PR40778][Sema] Adjust address space of operands in builtin operators

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC355608: [PR40778][Sema] Adjust addr space of operands in 
builtin operators. (authored by stulova, committed by ).
Herald added a subscriber: kristina.
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D58719?vs=189527=189725#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58719

Files:
  lib/Sema/SemaOverload.cpp
  test/CodeGenOpenCLCXX/addrspace-operators.cl

Index: test/CodeGenOpenCLCXX/addrspace-operators.cl
===
--- test/CodeGenOpenCLCXX/addrspace-operators.cl
+++ test/CodeGenOpenCLCXX/addrspace-operators.cl
@@ -0,0 +1,46 @@
+//RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+enum E {
+  a,
+  b,
+};
+
+class C {
+public:
+  void Assign(E e) { me = e; }
+  void OrAssign(E e) { mi |= e; }
+  E me;
+  int mi;
+};
+
+__global E globE;
+//CHECK-LABEL: define spir_func void @_Z3barv()
+void bar() {
+  C c;
+  //CHECK: addrspacecast %class.C* %c to %class.C addrspace(4)*
+  //CHECK: call void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* %{{[0-9]+}}, i32 0)
+  c.Assign(a);
+  //CHECK: addrspacecast %class.C* %c to %class.C addrspace(4)*
+  //CHECK: call void @_ZNU3AS41C8OrAssignE1E(%class.C addrspace(4)* %{{[0-9]+}}, i32 0)
+  c.OrAssign(a);
+
+  E e;
+  // CHECK: store i32 1, i32* %e
+  e = b;
+  // CHECK: store i32 0, i32 addrspace(1)* @globE
+  globE = a;
+  // FIXME: Sema fails here because it thinks the types are incompatible.
+  //e = b;
+  //globE = a;
+}
+
+//CHECK: define linkonce_odr void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* %this, i32 %e)
+//CHECK: [[E:%[0-9]+]] = load i32, i32* %e.addr
+//CHECK: %me = getelementptr inbounds %class.C, %class.C addrspace(4)* %this1, i32 0, i32 0
+//CHECK: store i32 [[E]], i32 addrspace(4)* %me
+
+//CHECK define linkonce_odr void @_ZNU3AS41C8OrAssignE1E(%class.C addrspace(4)* %this, i32 %e)
+//CHECK: [[E:%[0-9]+]] = load i32, i32* %e.addr
+//CHECK: %mi = getelementptr inbounds %class.C, %class.C addrspace(4)* %this1, i32 0, i32 1
+//CHECK: [[MI:%[0-9]+]] = load i32, i32 addrspace(4)* %mi
+//CHECK: %or = or i32 [[MI]], [[E]]
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -7659,6 +7659,12 @@
 }
   }
 }
+/// Helper function for adjusting address spaces for the pointer or reference
+/// operands of builtin operators depending on the argument.
+static QualType AdjustAddressSpaceForBuiltinOperandType(Sema , QualType T,
+Expr *Arg) {
+  return S.Context.getAddrSpaceQualType(T, Arg->getType().getAddressSpace());
+}
 
 /// Helper function for AddBuiltinOperatorCandidates() that adds
 /// the volatile- and non-volatile-qualified assignment operators for the
@@ -7670,15 +7676,17 @@
   QualType ParamTypes[2];
 
   // T& operator=(T&, T)
-  ParamTypes[0] = S.Context.getLValueReferenceType(T);
+  ParamTypes[0] = S.Context.getLValueReferenceType(
+  AdjustAddressSpaceForBuiltinOperandType(S, T, Args[0]));
   ParamTypes[1] = T;
   S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
 /*IsAssignmentOperator=*/true);
 
   if (!S.Context.getCanonicalType(T).isVolatileQualified()) {
 // volatile T& operator=(volatile T&, T)
-ParamTypes[0]
-  = S.Context.getLValueReferenceType(S.Context.getVolatileType(T));
+ParamTypes[0] = S.Context.getLValueReferenceType(
+AdjustAddressSpaceForBuiltinOperandType(S, S.Context.getVolatileType(T),
+Args[0]));
 ParamTypes[1] = T;
 S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
   /*IsAssignmentOperator=*/true);
@@ -8573,8 +8581,9 @@
 ParamTypes[1] = ArithmeticTypes[Right];
 
 // Add this built-in operator as a candidate (VQ is empty).
-ParamTypes[0] =
-  S.Context.getLValueReferenceType(ArithmeticTypes[Left]);
+ParamTypes[0] = S.Context.getLValueReferenceType(
+AdjustAddressSpaceForBuiltinOperandType(S, ArithmeticTypes[Left],
+Args[0]));
 S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
 if (VisibleTypeConversionsQuals.hasVolatile()) {
   // Add this built-in operator as a candidate (VQ is 'volatile').
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r355608 - [PR40778][Sema] Adjust addr space of operands in builtin operators.

[Anastasia Stulova via cfe-commits]

Author: stulova
Date: Thu Mar  7 08:43:41 2019
New Revision: 355608

URL: http://llvm.org/viewvc/llvm-project?rev=355608=rev
Log:
[PR40778][Sema] Adjust addr space of operands in builtin operators.

Adjust address space for references and pointer operands of builtin operators.

Currently this change only fixes addr space in assignment (= and |=) operator,
that is needed for the test case reported in the bug. Wider support for all
other operations will follow.

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


Added:
cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl
Modified:
cfe/trunk/lib/Sema/SemaOverload.cpp

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=355608=355607=355608=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Thu Mar  7 08:43:41 2019
@@ -7659,6 +7659,12 @@ BuiltinCandidateTypeSet::AddTypesConvert
 }
   }
 }
+/// Helper function for adjusting address spaces for the pointer or reference
+/// operands of builtin operators depending on the argument.
+static QualType AdjustAddressSpaceForBuiltinOperandType(Sema , QualType T,
+Expr *Arg) {
+  return S.Context.getAddrSpaceQualType(T, Arg->getType().getAddressSpace());
+}
 
 /// Helper function for AddBuiltinOperatorCandidates() that adds
 /// the volatile- and non-volatile-qualified assignment operators for the
@@ -7670,15 +7676,17 @@ static void AddBuiltinAssignmentOperator
   QualType ParamTypes[2];
 
   // T& operator=(T&, T)
-  ParamTypes[0] = S.Context.getLValueReferenceType(T);
+  ParamTypes[0] = S.Context.getLValueReferenceType(
+  AdjustAddressSpaceForBuiltinOperandType(S, T, Args[0]));
   ParamTypes[1] = T;
   S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
 /*IsAssignmentOperator=*/true);
 
   if (!S.Context.getCanonicalType(T).isVolatileQualified()) {
 // volatile T& operator=(volatile T&, T)
-ParamTypes[0]
-  = S.Context.getLValueReferenceType(S.Context.getVolatileType(T));
+ParamTypes[0] = S.Context.getLValueReferenceType(
+AdjustAddressSpaceForBuiltinOperandType(S, 
S.Context.getVolatileType(T),
+Args[0]));
 ParamTypes[1] = T;
 S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
   /*IsAssignmentOperator=*/true);
@@ -8573,8 +8581,9 @@ public:
 ParamTypes[1] = ArithmeticTypes[Right];
 
 // Add this built-in operator as a candidate (VQ is empty).
-ParamTypes[0] =
-  S.Context.getLValueReferenceType(ArithmeticTypes[Left]);
+ParamTypes[0] = S.Context.getLValueReferenceType(
+AdjustAddressSpaceForBuiltinOperandType(S, ArithmeticTypes[Left],
+Args[0]));
 S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
 if (VisibleTypeConversionsQuals.hasVolatile()) {
   // Add this built-in operator as a candidate (VQ is 'volatile').

Added: cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl?rev=355608=auto
==
--- cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl (added)
+++ cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl Thu Mar  7 08:43:41 
2019
@@ -0,0 +1,46 @@
+//RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck 
%s
+
+enum E {
+  a,
+  b,
+};
+
+class C {
+public:
+  void Assign(E e) { me = e; }
+  void OrAssign(E e) { mi |= e; }
+  E me;
+  int mi;
+};
+
+__global E globE;
+//CHECK-LABEL: define spir_func void @_Z3barv()
+void bar() {
+  C c;
+  //CHECK: addrspacecast %class.C* %c to %class.C addrspace(4)*
+  //CHECK: call void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* %{{[0-9]+}}, 
i32 0)
+  c.Assign(a);
+  //CHECK: addrspacecast %class.C* %c to %class.C addrspace(4)*
+  //CHECK: call void @_ZNU3AS41C8OrAssignE1E(%class.C addrspace(4)* 
%{{[0-9]+}}, i32 0)
+  c.OrAssign(a);
+
+  E e;
+  // CHECK: store i32 1, i32* %e
+  e = b;
+  // CHECK: store i32 0, i32 addrspace(1)* @globE
+  globE = a;
+  // FIXME: Sema fails here because it thinks the types are incompatible.
+  //e = b;
+  //globE = a;
+}
+
+//CHECK: define linkonce_odr void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* 
%this, i32 %e)
+//CHECK: [[E:%[0-9]+]] = load i32, i32* %e.addr
+//CHECK: %me = getelementptr inbounds %class.C, %class.C addrspace(4)* %this1, 
i32 0, i32 0
+//CHECK: store i32 [[E]], i32 addrspace(4)* %me
+
+//CHECK define linkonce_odr void @_ZNU3AS41C8OrAssignE1E(%class.C 
addrspace(4)* %this, i32 %e)
+//CHECK: [[E:%[0-9]+]] = load i32, i32* %e.addr
+//CHECK: %mi = getelementptr inbounds %class.C, %class.C 

r355606 - [PR40778] Preserve addr space in Derived to Base cast.

[Anastasia Stulova via cfe-commits]

Author: stulova
Date: Thu Mar  7 08:23:15 2019
New Revision: 355606

URL: http://llvm.org/viewvc/llvm-project?rev=355606=rev
Log:
[PR40778] Preserve addr space in Derived to Base cast.

The address space for the Base class pointer when up-casting
from Derived should be taken from the Derived class pointer.

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


Added:
cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
Modified:
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=355606=355605=355606=diff
==
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Thu Mar  7 08:23:15 2019
@@ -302,7 +302,8 @@ Address CodeGenFunction::GetAddressOfBas
 
   // Get the base pointer type.
   llvm::Type *BasePtrTy =
-ConvertType((PathEnd[-1])->getType())->getPointerTo();
+  ConvertType((PathEnd[-1])->getType())
+  ->getPointerTo(Value.getType()->getPointerAddressSpace());
 
   QualType DerivedTy = getContext().getRecordType(Derived);
   CharUnits DerivedAlign = CGM.getClassPointerAlignment(Derived);

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=355606=355605=355606=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Mar  7 08:23:15 2019
@@ -2660,10 +2660,15 @@ Sema::PerformObjectMemberConversion(Expr
   bool PointerConversions = false;
   if (isa(Member)) {
 DestRecordType = Context.getCanonicalType(Context.getTypeDeclType(RD));
+auto FromPtrType = FromType->getAs();
+DestRecordType = Context.getAddrSpaceQualType(
+DestRecordType, FromPtrType
+? FromType->getPointeeType().getAddressSpace()
+: FromType.getAddressSpace());
 
-if (FromType->getAs()) {
+if (FromPtrType) {
   DestType = Context.getPointerType(DestRecordType);
-  FromRecordType = FromType->getPointeeType();
+  FromRecordType = FromPtrType->getPointeeType();
   PointerConversions = true;
 } else {
   DestType = DestRecordType;

Added: cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl?rev=355606=auto
==
--- cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl (added)
+++ cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl Thu Mar  7 
08:23:15 2019
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck 
%s
+
+struct B {
+  int mb;
+};
+
+class D : public B {
+public:
+  int getmb() { return mb; }
+};
+
+void foo() {
+  D d;
+  //CHECK: addrspacecast %class.D* %d to %class.D addrspace(4)*
+  //CHECK: call i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)*
+  d.getmb();
+}
+
+//Derived and Base are in the same address space.
+
+//CHECK: define linkonce_odr i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)* 
%this)
+//CHECK: bitcast %class.D addrspace(4)* %this1 to %struct.B addrspace(4)*


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

[PATCH] D59092: [clangd] Deduplicate Refs on the fly.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clangd/index/Ref.cpp:44
 
 RefSlab RefSlab::Builder::build() && {
   // We can reuse the arena, as it only has unique strings and we need them 
all.

Can you add a comment saying that references will be ordered?



Comment at: clangd/indexer/IndexerMain.cpp:59
  for (const auto  : S) {
-   // No need to merge as currently all Refs are from main 
file.
for (const auto  : Sym.second)

instead of just deleting let's mention that it de-duplicates during insertion.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59092



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

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

[Petar Jovanovic via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355605: [analyzer] handle modification of vars inside an 
expr with comma operator (authored by petarj, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58894?vs=189661=189719#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58894

Files:
  cfe/trunk/include/clang/AST/Expr.h
  cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
  cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Index: cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
+++ cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -24,6 +24,18 @@
   return InnerMatcher.matches(*Range, Finder, Builder);
 }
 
+AST_MATCHER_P(Expr, maybeEvalCommaExpr,
+ ast_matchers::internal::Matcher, InnerMatcher) {
+  const Expr* Result = 
+  while (const auto *BOComma =
+   dyn_cast_or_null(Result->IgnoreParens())) {
+if (!BOComma->isCommaOp())
+  break;
+Result = BOComma->getRHS();
+  }
+  return InnerMatcher.matches(*Result, Finder, Builder);
+}
+
 const ast_matchers::internal::VariadicDynCastAllOfMatcher
 cxxTypeidExpr;
 
@@ -193,24 +205,28 @@
 const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
   // LHS of any assignment operators.
   const auto AsAssignmentLhs =
-  binaryOperator(isAssignmentOperator(), hasLHS(equalsNode(Exp)));
+  binaryOperator(isAssignmentOperator(),
+ hasLHS(maybeEvalCommaExpr(equalsNode(Exp;
 
   // Operand of increment/decrement operators.
   const auto AsIncDecOperand =
   unaryOperator(anyOf(hasOperatorName("++"), hasOperatorName("--")),
-hasUnaryOperand(equalsNode(Exp)));
+hasUnaryOperand(maybeEvalCommaExpr(equalsNode(Exp;
 
   // Invoking non-const member function.
   // A member function is assumed to be non-const when it is unresolved.
   const auto NonConstMethod = cxxMethodDecl(unless(isConst()));
   const auto AsNonConstThis =
-  expr(anyOf(cxxMemberCallExpr(callee(NonConstMethod), on(equalsNode(Exp))),
+  expr(anyOf(cxxMemberCallExpr(callee(NonConstMethod),
+   on(maybeEvalCommaExpr(equalsNode(Exp,
  cxxOperatorCallExpr(callee(NonConstMethod),
- hasArgument(0, equalsNode(Exp))),
+ hasArgument(0,
+ maybeEvalCommaExpr(equalsNode(Exp,
  callExpr(callee(expr(anyOf(
- unresolvedMemberExpr(hasObjectExpression(equalsNode(Exp))),
+ unresolvedMemberExpr(
+   hasObjectExpression(maybeEvalCommaExpr(equalsNode(Exp,
  cxxDependentScopeMemberExpr(
- hasObjectExpression(equalsNode(Exp);
+ hasObjectExpression(maybeEvalCommaExpr(equalsNode(Exp));
 
   // Taking address of 'Exp'.
   // We're assuming 'Exp' is mutated as soon as its address is taken, though in
@@ -220,10 +236,11 @@
   unaryOperator(hasOperatorName("&"),
 // A NoOp implicit cast is adding const.
 unless(hasParent(implicitCastExpr(hasCastKind(CK_NoOp,
-hasUnaryOperand(equalsNode(Exp)));
+hasUnaryOperand(maybeEvalCommaExpr(equalsNode(Exp;
   const auto AsPointerFromArrayDecay =
   castExpr(hasCastKind(CK_ArrayToPointerDecay),
-   unless(hasParent(arraySubscriptExpr())), has(equalsNode(Exp)));
+   unless(hasParent(arraySubscriptExpr())),
+   has(maybeEvalCommaExpr(equalsNode(Exp;
   // Treat calling `operator->()` of move-only classes as taking address.
   // These are typically smart pointers with unique ownership so we treat
   // mutation of pointee as mutation of the smart pointer itself.
@@ -231,7 +248,8 @@
   cxxOperatorCallExpr(hasOverloadedOperatorName("->"),
   callee(cxxMethodDecl(ofClass(isMoveOnly()),
returns(nonConstPointerType(,
-  argumentCountIs(1), hasArgument(0, equalsNode(Exp)));
+  argumentCountIs(1),
+  hasArgument(0, maybeEvalCommaExpr(equalsNode(Exp;
 
   // Used as non-const-ref argument when calling a function.
   // An argument is assumed to be non-const-ref when the function is unresolved.
@@ -239,7 +257,8 @@
   // findFunctionArgMutation which has additional smarts for handling forwarding
   // references.
   const auto NonConstRefParam = forEachArgumentWithParam(
-  equalsNode(Exp), 

[PATCH] D59086: [clangd] Adjust compile commands to be applicable for tooling

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/GlobalCompilationDatabase.cpp:25
+  // Clangd does not generate dependency file.
+  Cmd.CommandLine = tooling::getClangStripDependencyFileAdjuster()(
+  Cmd.CommandLine, Cmd.Filename);

Please use `clang::tooling::combineAdjusters`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59086



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

[PATCH] D59086: [clangd] Adjust compile commands to be applicable for tooling

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clangd/GlobalCompilationDatabase.cpp:24
  llvm::StringRef ResourceDir) {
+  // Clangd does not generate dependency file.
+  Cmd.CommandLine = tooling::getClangStripDependencyFileAdjuster()(

Please don't duplicate code in comments. Explain why it shouldn't generate the 
dependency files, or drop the comment.





Comment at: clangd/GlobalCompilationDatabase.h:116
+  /// Adjusts given compile command for clangd.
+  tooling::CompileCommand adjustArguments(tooling::CompileCommand Cmd) const;
+

kadircet wrote:
> hokein wrote:
> > ioeric wrote:
> > > This doesn't seem to be used in this patch (except for tests). Could you 
> > > include intended uses in the patch so we can understand the problem 
> > > better?
> > Looks like this patch introduces two changes:
> > 
> > - move the internal adjustArguments to public, I have the same question, 
> > any reason doing it? because we can test it? 
> > - add additional Adjusters to adjustArguments
> It was only for testing
This patch also changes `getCompileCommand` to call `adjustArguments`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59086



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

[PATCH] D59054: [analyzer] C++17: PR40022: Support aggregate initialization with compound values in presence of base classes.

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

Szelethus added a comment.

At first glance this looks OK, but I'll take the time to understand the 
infrastructure behind it and give a proper review.




Comment at: clang/test/Analysis/array-struct-region.cpp:3
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ 
-analyzer-config c++-inlining=constructors %s
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ 
-std=c++17 -analyzer-config c++-inlining=constructors %s
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c %s

I didn't see `-analyzer-config c++-inlining=constructors` in the bugreport -- 
why is it needed?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59054



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

[PATCH] D53757: [ASTImporter] Changed use of Import to Import_New in ASTNodeImporter.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.
Herald added a subscriber: rnkovacs.

> This patch is the next in the series of patches to finish the proper error 
> handling (i.e. using Error and Expected). Please see the "Stack" column 
> above.

@balazske Or is it superseded by this one? https://reviews.llvm.org/D53818


Repository:
  rC Clang

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

https://reviews.llvm.org/D53757



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

[PATCH] D59094: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

[Carey Williams via Phabricator via cfe-commits]

carwil created this revision.
Herald added subscribers: cfe-commits, jdoerfert, kristof.beyls, javed.absar.
Herald added a project: clang.

See: https://bugs.llvm.org/show_bug.cgi?id=39982

When considering how to classify homogeneous aggregates as return/argument 
types, the ABI of the function (specified by attribute pcs) wasn't being taken 
into account. This resulted in some weird and unexpected hybrid assembly when 
compiling with softfp.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59094

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCXX/arm-pcs.cpp

Index: clang/test/CodeGenCXX/arm-pcs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/arm-pcs.cpp
@@ -0,0 +1,46 @@
+// Covers a bug fix for ABI selection with homogenous aggregates:
+//  See: https://bugs.llvm.org/show_bug.cgi?id=39982
+
+// RUN: %clang -mfloat-abi=hard --target=armv7-unknown-linux-gnueabi -O3 -S -o - %s | FileCheck %s -check-prefixes=HARD_INSTRs,CHECK
+// RUN: %clang -mfloat-abi=softfp --target=armv7-unknown-linux-gnueabi -O3 -S -o - %s | FileCheck %s -check-prefixes=HARD_INSTRs,CHECK
+// RUN: %clang -mfloat-abi=soft --target=armv7-unknown-linux-gnueabi -O3 -S -o - %s | FileCheck %s -check-prefixes=SOFT_INSTRs,CHECK
+
+struct S {
+  float f;
+  float d;
+};
+
+// Variadic functions should always marshal for the base standard.
+// See section 5.5 (Parameter Passing) of the AAPCS.
+float __attribute__((pcs("aapcs-vfp"))) variadic(S s, ...) {
+  // CHECK-NOT: vmov s{{[0-9]+}}, s{{[0-9]+}}
+  // CHECK: mov r{{[0-9]+}}, r{{[0-9]+}}
+  return s.d;
+}
+
+float __attribute__((pcs("aapcs-vfp"))) not_variadic(S s) {
+  // SOFT_INSTRs: mov r{{[0-9]+}}, r{{[0-9]+}}
+  // HARD_INSTRs-NOT: vmov s{{[0-9]+}}, r{{[0-9]+}}
+  // HARD_INSTRs: vmov.f32 s{{[0-9]+}}, s{{[0-9]+}}
+  return s.d;
+}
+
+float __attribute__((pcs("aapcs-vfp"))) baz(float x, float y) {
+  // SOFT_INSTRs: mov r{{[0-9]+}}, r{{[0-9]+}}
+  // HARD_INSTRs-NOT: mov s{{[0-9]+}}, r{{[0-9]+}}
+  // HARD_INSTRs: vmov.f32 s{{[0-9]+}}, s{{[0-9]+}}
+  return y;
+}
+
+float __attribute__((pcs("aapcs-vfp"))) foo(S s) {
+  // SOFT_INSTRs: mov r{{[0-9]+}}, r{{[0-9]+}}
+  // HARD_INSTRs-NOT: mov r{{[0-9]+}}, r{{[0-9]+}}
+  // HARD_INSTRs: vmov.f32 s{{[0-9]+}}, s{{[0-9]+}}
+  return s.d;
+}
+
+float __attribute__((pcs("aapcs"))) bar(S s) {
+  // CHECK-NOT: vmov s{{[0-9]+}}, s{{[0-9]+}}
+  // CHECK: mov r{{[0-9]+}}, r{{[0-9]+}}
+  return s.d;
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -5591,8 +5591,10 @@
   ABIKind getABIKind() const { return Kind; }
 
 private:
-  ABIArgInfo classifyReturnType(QualType RetTy, bool isVariadic) const;
-  ABIArgInfo classifyArgumentType(QualType RetTy, bool isVariadic) const;
+  ABIArgInfo classifyReturnType(QualType RetTy, bool isVariadic,
+unsigned functionCallConv) const;
+  ABIArgInfo classifyArgumentType(QualType RetTy, bool isVariadic,
+  unsigned functionCallConv) const;
   ABIArgInfo classifyHomogeneousAggregate(QualType Ty, const Type *Base,
   uint64_t Members) const;
   ABIArgInfo coerceIllegalVector(QualType Ty) const;
@@ -5722,11 +5724,12 @@
 
 void ARMABIInfo::computeInfo(CGFunctionInfo ) const {
   if (!::classifyReturnType(getCXXABI(), FI, *this))
-FI.getReturnInfo() =
-classifyReturnType(FI.getReturnType(), FI.isVariadic());
+FI.getReturnInfo() = classifyReturnType(FI.getReturnType(), FI.isVariadic(),
+FI.getCallingConvention());
 
   for (auto  : FI.arguments())
-I.info = classifyArgumentType(I.type, FI.isVariadic());
+I.info = classifyArgumentType(I.type, FI.isVariadic(),
+  FI.getCallingConvention());
 
   // Always honor user-specified calling convention.
   if (FI.getCallingConvention() != llvm::CallingConv::C)
@@ -5805,8 +5808,8 @@
   return ABIArgInfo::getDirect(nullptr, 0, nullptr, false);
 }
 
-ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty,
-bool isVariadic) const {
+ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty, bool isVariadic,
+unsigned functionCallConv) const {
   // 6.1.2.1 The following argument types are VFP CPRCs:
   //   A single-precision floating-point type (including promoted
   //   half-precision types); A double-precision floating-point type;
@@ -5814,8 +5817,11 @@
   //   with a Base Type of a single- or double-precision floating-point type,
   //   64-bit containerized vectors or 128-bit containerized vectors with one
   //   to four Elements.
-  bool IsEffectivelyAAPCS_VFP = getABIKind() == AAPCS_VFP && !isVariadic;
 
+  bool IsEffectivelyAAPCS_VFP =
+  (getABIKind() == 

[PATCH] D53757: [ASTImporter] Changed use of Import to Import_New in ASTNodeImporter.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.
Herald added a reviewer: martong.
Herald added a project: clang.

Ping. This patch is the next in the series of patches to finish the proper 
error handling (i.e. using Error and Expected). Please see the "Stack" 
column above.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53757



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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Jason Liu via Phabricator via cfe-commits]

jasonliu updated this revision to Diff 189713.
jasonliu added a comment.

Share the same "unsupported error" message with WASM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/unittests/ADT/TripleTest.cpp

Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -1258,6 +1258,11 @@
   EXPECT_EQ(Triple::Wasm,
 Triple("wasm64-unknown-wasi-musl-wasm").getObjectFormat());
 
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc-ibm-aix").getObjectFormat());
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc64-ibm-aix").getObjectFormat());
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc---xcoff").getObjectFormat());
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc64---xcoff").getObjectFormat());
+
   Triple MSVCNormalized(Triple::normalize("i686-pc-windows-msvc-elf"));
   EXPECT_EQ(Triple::ELF, MSVCNormalized.getObjectFormat());
 
@@ -1276,6 +1281,9 @@
 
   T.setObjectFormat(Triple::MachO);
   EXPECT_EQ(Triple::MachO, T.getObjectFormat());
+  
+  T.setObjectFormat(Triple::XCOFF);
+  EXPECT_EQ(Triple::XCOFF, T.getObjectFormat());
 }
 
 TEST(TripleTest, NormalizeWindows) {
Index: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
===
--- llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -5594,6 +5594,9 @@
   case MCObjectFileInfo::IsWasm:
 CurrentFormat = WASM;
 break;
+  case MCObjectFileInfo::IsXCOFF:
+llvm_unreachable("unexpected object format");
+break;
   }
 
   if (~Prefix->SupportedFormats & CurrentFormat) {
Index: llvm/lib/Support/Triple.cpp
===
--- llvm/lib/Support/Triple.cpp
+++ llvm/lib/Support/Triple.cpp
@@ -534,6 +534,9 @@
 
 static Triple::ObjectFormatType parseFormat(StringRef EnvironmentName) {
   return StringSwitch(EnvironmentName)
+// "xcoff" must come before "coff" because of the order-dependendent
+// pattern matching.
+.EndsWith("xcoff", Triple::XCOFF)
 .EndsWith("coff", Triple::COFF)
 .EndsWith("elf", Triple::ELF)
 .EndsWith("macho", Triple::MachO)
@@ -622,6 +625,7 @@
   case Triple::ELF: return "elf";
   case Triple::MachO: return "macho";
   case Triple::Wasm: return "wasm";
+  case Triple::XCOFF: return "xcoff";
   }
   llvm_unreachable("unknown object format type");
 }
@@ -686,6 +690,8 @@
   case Triple::ppc64:
 if (T.isOSDarwin())
   return Triple::MachO;
+else if (T.isOSAIX())
+  return Triple::XCOFF;
 return Triple::ELF;
 
   case Triple::wasm32:
Index: llvm/lib/MC/MCParser/AsmParser.cpp
===
--- llvm/lib/MC/MCParser/AsmParser.cpp
+++ llvm/lib/MC/MCParser/AsmParser.cpp
@@ -710,6 +710,9 @@
   case MCObjectFileInfo::IsWasm:
 PlatformParser.reset(createWasmAsmParser());
 break;
+  case MCObjectFileInfo::IsXCOFF:
+// TODO: Need to implement createXCOFFAsmParser for XCOFF format.
+break;
   }
 
   PlatformParser->Initialize(*this);
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -801,6 +801,10 @@
 Env = IsWasm;
 initWasmMCObjectFileInfo(TT);
 break;
+  case Triple::XCOFF:
+Env = IsXCOFF;
+// TODO: Initialize MCObjectFileInfo for XCOFF format when MCSectionXCOFF is ready.
+break;
   case Triple::UnknownObjectFormat:
 report_fatal_error("Cannot initialize MC for unknown object file format.");
 break;
@@ -816,6 +820,7 @@
   case Triple::MachO:
   case Triple::COFF:
   case Triple::Wasm:
+  case Triple::XCOFF:
   case Triple::UnknownObjectFormat:
 report_fatal_error("Cannot get DWARF comdat section for this object file "
"format: not implemented.");
Index: llvm/lib/MC/MCContext.cpp
===
--- llvm/lib/MC/MCContext.cpp
+++ llvm/lib/MC/MCContext.cpp
@@ -161,6 +161,9 @@
   return new (Name, *this) MCSymbolMachO(Name, IsTemporary);
 case MCObjectFileInfo::IsWasm:
   return new (Name, *this) MCSymbolWasm(Name, IsTemporary);
+case MCObjectFileInfo::IsXCOFF:
+  // TODO: Need to implement class MCSymbolXCOFF.
+  break;
 }
   }
   return new (Name, *this) MCSymbol(MCSymbol::SymbolKindUnset, Name,

[PATCH] D58878: [Diagnostics] Warn for assignments in bool contexts

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Addressed notes.

Not sure why we miss
_Bool warn3(int x, _Bool a) {

  return x = 0 || a;

}

in C mode :/


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

https://reviews.llvm.org/D58878



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

[PATCH] D58878: [Diagnostics] Warn for assignments in bool contexts

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 189711.

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

https://reviews.llvm.org/D58878

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExprCXX.cpp
  test/CodeCompletion/crash-skipped-bodies-template-inst.cpp
  test/CodeCompletion/skip-auto-funcs.cpp
  test/Modules/diag-pragma.c
  test/Modules/diag-pragma.cpp
  test/Sema/assigment-in-bool-context.c
  test/Sema/parentheses.c
  test/SemaCXX/assigment-in-bool-context.cpp
  test/SemaCXX/warn-assignment-condition.cpp
  test/SemaObjC/idiomatic-parentheses.m
  test/SemaObjC/self-assign.m

Index: test/SemaObjC/self-assign.m
===
--- test/SemaObjC/self-assign.m
+++ test/SemaObjC/self-assign.m
@@ -6,7 +6,7 @@
 - (id):(int)x :(int)y {
 int z;
 // 
-if (self = [self :x :y]) {} // expected-warning{{using the result of an assignment as a condition without parentheses}} \
+if (self = [self :x :y]) {} // expected-warning{{converting the result of an assignment to bool}} \
 // expected-note{{use '==' to turn this assignment into an equality comparison}} \
 // expected-note{{place parentheses around the assignment to silence this warning}}
 return self;
Index: test/SemaObjC/idiomatic-parentheses.m
===
--- test/SemaObjC/idiomatic-parentheses.m
+++ test/SemaObjC/idiomatic-parentheses.m
@@ -27,7 +27,7 @@
   if (self = [self initWithInt: i]) {
   }
   // rdar://11066598
-  if (self.uid = 100) { // expected-warning {{using the result of an assignment as a condition without parentheses}} \
+  if (self.uid = 100) { // expected-warning {{converting the result of an assignment to bool}} \
 // expected-note {{place parentheses around the assignment to silence this warning}} \
 // expected-note {{use '==' to turn this assignment into an equality comparison}}
 // ...
Index: test/SemaCXX/warn-assignment-condition.cpp
===
--- test/SemaCXX/warn-assignment-condition.cpp
+++ test/SemaCXX/warn-assignment-condition.cpp
@@ -12,41 +12,41 @@
   A a, b;
 
   // With scalars.
-  if (x = 7) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \
+  if (x = 7) {} // expected-warning {{converting the result of an assignment to bool}} \
 // expected-note{{use '==' to turn this assignment into an equality comparison}} \
   // expected-note{{place parentheses around the assignment to silence this warning}}
   if ((x = 7)) {}
   do {
-  } while (x = 7); // expected-warning {{using the result of an assignment as a condition without parentheses}} \
+  } while (x = 7); // expected-warning {{converting the result of an assignment to bool}} \
 // expected-note{{use '==' to turn this assignment into an equality comparison}} \
   // expected-note{{place parentheses around the assignment to silence this warning}}
   do {
   } while ((x = 7));
-  while (x = 7) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \
+  while (x = 7) {} // expected-warning {{converting the result of an assignment to bool}} \
 // expected-note{{use '==' to turn this assignment into an equality comparison}} \
   // expected-note{{place parentheses around the assignment to silence this warning}}
 
   while ((x = 7)) {}
-  for (; x = 7; ) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \
+  for (; x = 7; ) {} // expected-warning {{converting the result of an assignment to bool}} \
 // expected-note{{use '==' to turn this assignment into an equality comparison}} \
   // expected-note{{place parentheses around the assignment to silence this warning}}
   for (; (x = 7); ) {}
 
-  if (p = p) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \
+  if (p = p) {} // expected-warning {{converting the result of an assignment to bool}} \
 // expected-note{{use '==' to turn this assignment into an equality comparison}} \
   // expected-note{{place parentheses around the assignment to silence this warning}}
   if ((p = p)) {}
   do {
-  } while (p = p); // expected-warning {{using the result of an assignment as a condition without parentheses}} \
+  } while (p = p); // expected-warning {{converting the result of an assignment to bool}} \
 // expected-note{{use '==' to turn this assignment into an equality comparison}} \
   // expected-note{{place parentheses around the assignment to silence this warning}}
   do {
   } while ((p = p));
-  while (p = p) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \
+  while (p = p) {} // expected-warning 

r355601 - expected-no-diagnostics@ does not make sense, switching to a more idiomatic form; NFC.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Thu Mar  7 07:03:06 2019
New Revision: 355601

URL: http://llvm.org/viewvc/llvm-project?rev=355601=rev
Log:
expected-no-diagnostics@ does not make sense, switching to a more idiomatic 
form; NFC.

Modified:
cfe/trunk/test/SemaObjCXX/vararg-non-pod.mm

Modified: cfe/trunk/test/SemaObjCXX/vararg-non-pod.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/vararg-non-pod.mm?rev=355601=355600=355601=diff
==
--- cfe/trunk/test/SemaObjCXX/vararg-non-pod.mm (original)
+++ cfe/trunk/test/SemaObjCXX/vararg-non-pod.mm Thu Mar  7 07:03:06 2019
@@ -2,6 +2,10 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s -Wno-error=non-pod-varargs 
-std=c++98
 // RUN: %clang_cc1 -fsyntax-only -verify %s -Wno-error=non-pod-varargs 
-std=c++11
 
+#if __cplusplus > 199711L
+// expected-no-diagnostics
+#endif
+
 extern char version[];
 
 @protocol P;
@@ -22,8 +26,6 @@ void t1(D *d)
   [d g:10, c]; 
 #if __cplusplus <= 199711L // C++03 or earlier modes
   // expected-warning@-2{{cannot pass object of non-POD type 'C' through 
variadic method; call will abort at runtime}}
-#else
-  // expected-no-diagnostics@-4
 #endif
   [d g:10, version];
 }


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

[PATCH] D45978: dllexport const variables must have external linkage.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:5970
 auto *VD = dyn_cast();
-if (!ND.isExternallyVisible() || (VD && VD->isStaticLocal())) {
+const NamespaceDecl *NS = nullptr;
+bool isAnonymousNS = false;

This can be lowered into the below `if` statement.



Comment at: lib/Sema/SemaDecl.cpp:5971-5972
+const NamespaceDecl *NS = nullptr;
+bool isAnonymousNS = false;
+bool isMicrosoft = S.Context.getTargetInfo().getCXXABI().isMicrosoft();
+if (VD) {

is -> Is per our naming convention rules.



Comment at: lib/Sema/SemaDecl.cpp:5977
+  while (DC && NS) {
+isAnonymousNS = isAnonymousNS || (NS && NS->getDeclName().isEmpty());
+DC = DC->getParent();

No need to test that `NS` is nonnull here; already done as part of the while 
loop predicate. Also, you can call `NamespaceDecl::isAnonymousNamespace()` 
rather than manually try to test the declared identifier.

I think a more clear way to write this is:
```
if (VD) {
  const NamespaceDecl *NS = dyn_cast(VD->getDeclContext());
  while (NS && !IsAnonymousNS) {
IsAnonymousNS = NS->isAnonymousNamespace();
NS = dyn_cast(NS->getParent());
  }
  ...
}
```



Comment at: lib/Sema/SemaDecl.cpp:5982-5984
+if ((ND.isExternallyVisible() && isAnonymousNS && isMicrosoft) ||
+(!(isAnonymousNS && isMicrosoft) &&
+ (!ND.isExternallyVisible() || (VD && VD->isStaticLocal() {

This is a pretty complicated predicate; I'd simplify it a bit and then add some 
comments to explain it further. The comments from line 5967 could move down 
here (with modification), in fact. Untested:
```
bool AnonNSInMicrosoftMode = IsAnonymous && IsMicrosoft;
if ((ND.isExternallyVisible() && AnonNSInMicrosoftMode) ||
(!AnonNSInMicrosoftMode &&
 (!ND.isExternallyVisible() || VD->isStaticLocal( {
  ...
}
```



Comment at: test/Sema/dllexport-1.cpp:7
+#if MSVC
+// expected-no-diagnostics@+4
+#else

I am almost certain that `expected-no-diagnostics` applies to the entire file, 
so the @ doesn't make sense. I think you just need one of these for the entire 
file:
```
#ifdef MSVC
// expected-no-diagnostics
#endif
```
(Note, I switched it to use `#ifdef` as well.)



Comment at: test/Sema/dllexport-2.cpp:6
+
+#if MSVC
+// expected-error@+6 {{default initialization of an object of const type 
'const int'}}

Please switch to `#ifdef` rather than `#if`.



Comment at: test/Sema/dllexport-2.cpp:28
+#if MSVC
+// expected-nodiagnostics
+#else

This isn't correct -- had the typo not been there, then the test would have 
failed because the file has diagnostics in MSVC mode. This should read:
```
#ifndef MSVC
// expected-warning@+2 {{__declspec attribute 'dllexport' is not supported}}
#endif
```



Comment at: test/SemaCXX/dllexport.cpp:73
+#ifdef MS
+// expected-nodiagnostics
+#else

Typo, but this is the wrong construct. Should be:
```
#ifndef MS
namespace {
  __declspec(dllexport) int InternalGlobal; // expected-error{{anonymous 
namespace)::InternalGlobal' must have external linkage when declared 
'dllexport'}}
}
#endif
```



Comment at: test/SemaCXX/dllexport.cpp:133
+#ifdef MS
+// expected-nodiagnostics
+#else

Similar here as above.



Comment at: test/SemaCXX/dllimport.cpp:125
+#ifdef MS
+// expected-nodiagnostics
+#else

Here too.



Comment at: test/SemaCXX/dllimport.cpp:222
+#ifdef MS
+// expected-nodiagnostics
+#else

And here.


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

https://reviews.llvm.org/D45978



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

[PATCH] D59092: [clangd] Deduplicate Refs on the fly.

[Haojian Wu via Phabricator via cfe-commits]

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

Currently, we only do deduplication when we flush final results. We may
have huge duplications (refs from headers) during the indexing period (running
clangd-indexer on Chromium).

With this change, clangd-indexer can index the whole chromium projects
(48 threads, 40 GB peak memory usage).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D59092

Files:
  clangd/index/Ref.cpp
  clangd/index/Ref.h
  clangd/indexer/IndexerMain.cpp


Index: clangd/indexer/IndexerMain.cpp
===
--- clangd/indexer/IndexerMain.cpp
+++ clangd/indexer/IndexerMain.cpp
@@ -56,7 +56,6 @@
[&](RefSlab S) {
  std::lock_guard Lock(SymbolsMu);
  for (const auto  : S) {
-   // No need to merge as currently all Refs are from main 
file.
for (const auto  : Sym.second)
  Refs.insert(Sym.first, Ref);
  }
Index: clangd/index/Ref.h
===
--- clangd/index/Ref.h
+++ clangd/index/Ref.h
@@ -16,6 +16,7 @@
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -99,7 +100,7 @@
   private:
 llvm::BumpPtrAllocator Arena;
 llvm::UniqueStringSaver UniqueStrings; // Contents on the arena.
-llvm::DenseMap> Refs;
+llvm::DenseMap> Refs;
   };
 
 private:
Index: clangd/index/Ref.cpp
===
--- clangd/index/Ref.cpp
+++ clangd/index/Ref.cpp
@@ -33,9 +33,12 @@
 
 void RefSlab::Builder::insert(const SymbolID , const Ref ) {
   auto  = Refs[ID];
-  M.push_back(S);
-  M.back().Location.FileURI =
-  UniqueStrings.save(M.back().Location.FileURI).data();
+  if (M.count(S))
+return;
+  Ref R = S;
+  R.Location.FileURI =
+  UniqueStrings.save(R.Location.FileURI).data();
+  M.insert(std::move(R));
 }
 
 RefSlab RefSlab::Builder::build() && {
@@ -45,11 +48,7 @@
   Result.reserve(Refs.size());
   size_t NumRefs = 0;
   for (auto  : Refs) {
-auto  = Sym.second;
-llvm::sort(SymRefs);
-// FIXME: do we really need to dedup?
-SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
-
+std::vector SymRefs(Sym.second.begin(), Sym.second.end());
 NumRefs += SymRefs.size();
 Result.emplace_back(Sym.first, llvm::ArrayRef(SymRefs).copy(Arena));
   }


Index: clangd/indexer/IndexerMain.cpp
===
--- clangd/indexer/IndexerMain.cpp
+++ clangd/indexer/IndexerMain.cpp
@@ -56,7 +56,6 @@
[&](RefSlab S) {
  std::lock_guard Lock(SymbolsMu);
  for (const auto  : S) {
-   // No need to merge as currently all Refs are from main file.
for (const auto  : Sym.second)
  Refs.insert(Sym.first, Ref);
  }
Index: clangd/index/Ref.h
===
--- clangd/index/Ref.h
+++ clangd/index/Ref.h
@@ -16,6 +16,7 @@
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -99,7 +100,7 @@
   private:
 llvm::BumpPtrAllocator Arena;
 llvm::UniqueStringSaver UniqueStrings; // Contents on the arena.
-llvm::DenseMap> Refs;
+llvm::DenseMap> Refs;
   };
 
 private:
Index: clangd/index/Ref.cpp
===
--- clangd/index/Ref.cpp
+++ clangd/index/Ref.cpp
@@ -33,9 +33,12 @@
 
 void RefSlab::Builder::insert(const SymbolID , const Ref ) {
   auto  = Refs[ID];
-  M.push_back(S);
-  M.back().Location.FileURI =
-  UniqueStrings.save(M.back().Location.FileURI).data();
+  if (M.count(S))
+return;
+  Ref R = S;
+  R.Location.FileURI =
+  UniqueStrings.save(R.Location.FileURI).data();
+  M.insert(std::move(R));
 }
 
 RefSlab RefSlab::Builder::build() && {
@@ -45,11 +48,7 @@
   Result.reserve(Refs.size());
   size_t NumRefs = 0;
   for (auto  : Refs) {
-auto  = Sym.second;
-llvm::sort(SymRefs);
-// FIXME: do we really need to dedup?
-SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
-
+std::vector SymRefs(Sym.second.begin(), Sym.second.end());
 NumRefs += SymRefs.size();
 Result.emplace_back(Sym.first, llvm::ArrayRef(SymRefs).copy(Arena));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59022: [clangd] Strip plugin arguments in clangd-indexer.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355599: [clangd] Strip plugin arguments in clangd-indexer. 
(authored by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59022

Files:
  clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp


Index: clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
===
--- clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
@@ -16,6 +16,7 @@
 #include "index/Serialization.h"
 #include "index/Symbol.h"
 #include "index/SymbolCollector.h"
+#include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
 #include "clang/Tooling/Tooling.h"
@@ -110,7 +111,8 @@
   // Collect symbols found in each translation unit, merging as we go.
   clang::clangd::IndexFileIn Data;
   auto Err = Executor->get()->execute(
-  llvm::make_unique(Data));
+  llvm::make_unique(Data),
+  clang::tooling::getStripPluginsAdjuster());
   if (Err) {
 llvm::errs() << llvm::toString(std::move(Err)) << "\n";
   }


Index: clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
===
--- clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
@@ -16,6 +16,7 @@
 #include "index/Serialization.h"
 #include "index/Symbol.h"
 #include "index/SymbolCollector.h"
+#include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
 #include "clang/Tooling/Tooling.h"
@@ -110,7 +111,8 @@
   // Collect symbols found in each translation unit, merging as we go.
   clang::clangd::IndexFileIn Data;
   auto Err = Executor->get()->execute(
-  llvm::make_unique(Data));
+  llvm::make_unique(Data),
+  clang::tooling::getStripPluginsAdjuster());
   if (Err) {
 llvm::errs() << llvm::toString(std::move(Err)) << "\n";
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r355599 - [clangd] Strip plugin arguments in clangd-indexer.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Thu Mar  7 06:47:17 2019
New Revision: 355599

URL: http://llvm.org/viewvc/llvm-project?rev=355599=rev
Log:
[clangd] Strip plugin arguments in clangd-indexer.

Summary: This would allow clangd-indexer runs on chromium repo.

Reviewers: kadircet

Reviewed By: kadircet

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp

Modified: clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp?rev=355599=355598=355599=diff
==
--- clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp Thu Mar  7 06:47:17 
2019
@@ -16,6 +16,7 @@
 #include "index/Serialization.h"
 #include "index/Symbol.h"
 #include "index/SymbolCollector.h"
+#include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
 #include "clang/Tooling/Tooling.h"
@@ -110,7 +111,8 @@ int main(int argc, const char **argv) {
   // Collect symbols found in each translation unit, merging as we go.
   clang::clangd::IndexFileIn Data;
   auto Err = Executor->get()->execute(
-  llvm::make_unique(Data));
+  llvm::make_unique(Data),
+  clang::tooling::getStripPluginsAdjuster());
   if (Err) {
 llvm::errs() << llvm::toString(std::move(Err)) << "\n";
   }


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

[PATCH] D59086: [clangd] Adjust compile commands to be applicable for tooling

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Change testing strategy


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59086

Files:
  clangd/GlobalCompilationDatabase.cpp
  unittests/clangd/GlobalCompilationDatabaseTests.cpp

Index: unittests/clangd/GlobalCompilationDatabaseTests.cpp
===
--- unittests/clangd/GlobalCompilationDatabaseTests.cpp
+++ unittests/clangd/GlobalCompilationDatabaseTests.cpp
@@ -16,15 +16,18 @@
 namespace clang {
 namespace clangd {
 namespace {
+using ::testing::AllOf;
+using ::testing::Contains;
 using ::testing::ElementsAre;
 using ::testing::EndsWith;
+using ::testing::Not;
 
 TEST(GlobalCompilationDatabaseTest, FallbackCommand) {
   DirectoryBasedGlobalCompilationDatabase DB(None);
   auto Cmd = DB.getFallbackCommand(testPath("foo/bar.cc"));
   EXPECT_EQ(Cmd.Directory, testPath("foo"));
-  EXPECT_THAT(Cmd.CommandLine, ElementsAre(
-EndsWith("clang"), testPath("foo/bar.cc")));
+  EXPECT_THAT(Cmd.CommandLine,
+  ElementsAre(EndsWith("clang"), testPath("foo/bar.cc")));
   EXPECT_EQ(Cmd.Output, "");
 
   // .h files have unknown language, so they are parsed liberally as obj-c++.
@@ -65,16 +68,18 @@
 
 TEST_F(OverlayCDBTest, GetCompileCommand) {
   OverlayCDB CDB(Base.get(), {}, std::string(""));
-  EXPECT_EQ(CDB.getCompileCommand(testPath("foo.cc")),
-Base->getCompileCommand(testPath("foo.cc")));
+  EXPECT_THAT(CDB.getCompileCommand(testPath("foo.cc"))->CommandLine,
+  AllOf(Contains(testPath("foo.cc")), Contains("-DA=1")));
   EXPECT_EQ(CDB.getCompileCommand(testPath("missing.cc")), llvm::None);
 
   auto Override = cmd(testPath("foo.cc"), "-DA=3");
   CDB.setCompileCommand(testPath("foo.cc"), Override);
-  EXPECT_EQ(CDB.getCompileCommand(testPath("foo.cc")), Override);
+  EXPECT_THAT(CDB.getCompileCommand(testPath("foo.cc"))->CommandLine,
+  Contains("-DA=3"));
   EXPECT_EQ(CDB.getCompileCommand(testPath("missing.cc")), llvm::None);
   CDB.setCompileCommand(testPath("missing.cc"), Override);
-  EXPECT_EQ(CDB.getCompileCommand(testPath("missing.cc")), Override);
+  EXPECT_THAT(CDB.getCompileCommand(testPath("missing.cc"))->CommandLine,
+  Contains("-DA=3"));
 }
 
 TEST_F(OverlayCDBTest, GetFallbackCommand) {
@@ -88,7 +93,8 @@
   EXPECT_EQ(CDB.getCompileCommand(testPath("bar.cc")), None);
   auto Override = cmd(testPath("bar.cc"), "-DA=5");
   CDB.setCompileCommand(testPath("bar.cc"), Override);
-  EXPECT_EQ(CDB.getCompileCommand(testPath("bar.cc")), Override);
+  EXPECT_THAT(CDB.getCompileCommand(testPath("bar.cc"))->CommandLine,
+  Contains("-DA=5"));
 
   EXPECT_THAT(CDB.getFallbackCommand(testPath("foo.cc")).CommandLine,
   ElementsAre(EndsWith("clang"), testPath("foo.cc"), "-DA=6"));
@@ -111,6 +117,35 @@
ElementsAre("A.cpp"), ElementsAre("C.cpp")));
 }
 
+TEST_F(OverlayCDBTest, Adjustments) {
+  OverlayCDB CDB(Base.get(), {}, std::string(""));
+  auto Cmd = CDB.getCompileCommand(testPath("foo.cc")).getValue();
+  // Delete the file name.
+  Cmd.CommandLine.pop_back();
+
+  // Check dependency file commands are dropped.
+  Cmd.CommandLine.push_back("-MF");
+  Cmd.CommandLine.push_back("random-dependency");
+
+  // Check plugin-related commands are dropped.
+  Cmd.CommandLine.push_back("-Xclang");
+  Cmd.CommandLine.push_back("-load");
+  Cmd.CommandLine.push_back("-Xclang");
+  Cmd.CommandLine.push_back("random-plugin");
+
+  Cmd.CommandLine.push_back("-DA=5");
+  Cmd.CommandLine.push_back(Cmd.Filename);
+
+  CDB.setCompileCommand(testPath("foo.cc"), Cmd);
+
+  EXPECT_THAT(CDB.getCompileCommand(testPath("foo.cc"))->CommandLine,
+  AllOf(Contains("-fsyntax-only"), Contains("-DA=5"),
+Contains(testPath("foo.cc")), Not(Contains("-MF")),
+Not(Contains("random-dependency")),
+Not(Contains("-Xclang")), Not(Contains("-load")),
+Not(Contains("random-plugin";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/GlobalCompilationDatabase.cpp
===
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -21,11 +21,17 @@
 
 void adjustArguments(tooling::CompileCommand ,
  llvm::StringRef ResourceDir) {
+  // Clangd does not generate dependency file.
+  Cmd.CommandLine = tooling::getClangStripDependencyFileAdjuster()(
+  Cmd.CommandLine, Cmd.Filename);
   // Strip plugin related command line arguments. Clangd does
   // not support plugins currently. Therefore it breaks if
   // compiler tries to load plugins.
   Cmd.CommandLine =
   tooling::getStripPluginsAdjuster()(Cmd.CommandLine, Cmd.Filename);
+
+  

[PATCH] D57464: Generalize method overloading on addr spaces to C++

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

Any more input on this?


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

https://reviews.llvm.org/D57464



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