[PATCH] D85874: [PowerPC] Add readflm/setflm intrinsics to Clang

2020-08-12 Thread Qiu Chaofan via Phabricator via cfe-commits
qiucf created this revision.
qiucf added reviewers: jsji, nemanjai, steven.zhang, ZhangKang, PowerPC.
Herald added subscribers: llvm-commits, cfe-commits, shchenz, kbarton.
Herald added projects: clang, LLVM.
qiucf requested review of this revision.
Herald added a subscriber: wuzish.

dbcfbffc  
adds `ppc.readflm` and `ppc.setflm` intrinsics to read or write FPSCR register. 
This patch adds them to Clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85874

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/test/CodeGen/builtins-ppc.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td


Index: llvm/include/llvm/IR/IntrinsicsPowerPC.td
===
--- llvm/include/llvm/IR/IntrinsicsPowerPC.td
+++ llvm/include/llvm/IR/IntrinsicsPowerPC.td
@@ -48,9 +48,11 @@
   def int_ppc_eieio : Intrinsic<[],[],[]>;
 
   // Get content from current FPSCR register
-  def int_ppc_readflm : Intrinsic<[llvm_double_ty], [], [IntrNoMem]>;
+  def int_ppc_readflm : GCCBuiltin<"__builtin_readflm">,
+Intrinsic<[llvm_double_ty], [], [IntrNoMem]>;
   // Set FPSCR register, and return previous content
-  def int_ppc_setflm : Intrinsic<[llvm_double_ty], [llvm_double_ty], []>;
+  def int_ppc_setflm : GCCBuiltin<"__builtin_setflm">,
+   Intrinsic<[llvm_double_ty], [llvm_double_ty], []>;
 
   // Intrinsics for [double]word extended forms of divide instructions
   def int_ppc_divwe : GCCBuiltin<"__builtin_divwe">,
Index: clang/test/CodeGen/builtins-ppc.c
===
--- clang/test/CodeGen/builtins-ppc.c
+++ clang/test/CodeGen/builtins-ppc.c
@@ -27,3 +27,12 @@
   // CHECK: call double @llvm.ppc.setrnd(i32 %2)
   res = __builtin_setrnd(x);
 }
+
+void test_builtin_ppc_flm() {
+  volatile double res;
+  // CHECK: call double @llvm.ppc.readflm()
+  res = __builtin_readflm();
+
+  // CHECK: call double @llvm.ppc.setflm(double %1)
+  res = __builtin_setflm(res);
+}
Index: clang/include/clang/Basic/BuiltinsPPC.def
===
--- clang/include/clang/Basic/BuiltinsPPC.def
+++ clang/include/clang/Basic/BuiltinsPPC.def
@@ -549,6 +549,12 @@
 // Set the floating point rounding mode
 BUILTIN(__builtin_setrnd, "di", "")
 
+// Get content from current FPSCR
+BUILTIN(__builtin_readflm, "d", "")
+
+// Set content of FPSCR, and return its content before update
+BUILTIN(__builtin_setflm, "dd", "")
+
 // Cache built-ins
 BUILTIN(__builtin_dcbf, "vvC*", "")
 


Index: llvm/include/llvm/IR/IntrinsicsPowerPC.td
===
--- llvm/include/llvm/IR/IntrinsicsPowerPC.td
+++ llvm/include/llvm/IR/IntrinsicsPowerPC.td
@@ -48,9 +48,11 @@
   def int_ppc_eieio : Intrinsic<[],[],[]>;
 
   // Get content from current FPSCR register
-  def int_ppc_readflm : Intrinsic<[llvm_double_ty], [], [IntrNoMem]>;
+  def int_ppc_readflm : GCCBuiltin<"__builtin_readflm">,
+Intrinsic<[llvm_double_ty], [], [IntrNoMem]>;
   // Set FPSCR register, and return previous content
-  def int_ppc_setflm : Intrinsic<[llvm_double_ty], [llvm_double_ty], []>;
+  def int_ppc_setflm : GCCBuiltin<"__builtin_setflm">,
+   Intrinsic<[llvm_double_ty], [llvm_double_ty], []>;
 
   // Intrinsics for [double]word extended forms of divide instructions
   def int_ppc_divwe : GCCBuiltin<"__builtin_divwe">,
Index: clang/test/CodeGen/builtins-ppc.c
===
--- clang/test/CodeGen/builtins-ppc.c
+++ clang/test/CodeGen/builtins-ppc.c
@@ -27,3 +27,12 @@
   // CHECK: call double @llvm.ppc.setrnd(i32 %2)
   res = __builtin_setrnd(x);
 }
+
+void test_builtin_ppc_flm() {
+  volatile double res;
+  // CHECK: call double @llvm.ppc.readflm()
+  res = __builtin_readflm();
+
+  // CHECK: call double @llvm.ppc.setflm(double %1)
+  res = __builtin_setflm(res);
+}
Index: clang/include/clang/Basic/BuiltinsPPC.def
===
--- clang/include/clang/Basic/BuiltinsPPC.def
+++ clang/include/clang/Basic/BuiltinsPPC.def
@@ -549,6 +549,12 @@
 // Set the floating point rounding mode
 BUILTIN(__builtin_setrnd, "di", "")
 
+// Get content from current FPSCR
+BUILTIN(__builtin_readflm, "d", "")
+
+// Set content of FPSCR, and return its content before update
+BUILTIN(__builtin_setflm, "dd", "")
+
 // Cache built-ins
 BUILTIN(__builtin_dcbf, "vvC*", "")
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85545: [Diagnostics] Diagnose missing comma in string array initialization

2020-08-12 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

It still seems to trigger on structs at head:

$ cat /tmp/a.cc
struct A {

  const char *a;
  const char *b;
  const char *c;

};

static A a = {"",

  ""
  "",
  ""};

$ ./build/bin/clang++ -Wstring-concatenation -o /dev/null -c /tmp/a.cc
C:/src/tmp/a.cc:10:15: warning: suspicious concatenation of string literals in 
an array initialization; did you mean to separate the elements with a comma? 
[-Wstring-concatenation]

  "",
  ^

C:/src/tmp/a.cc:9:15: note: place parentheses around the string literal to 
silence warning

  ""
  ^

1 warning generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85545

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


[PATCH] D85796: [Analysis] Bug fix for exploded graph branching in evalCall for constructor

2020-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:39
 void derefAfterRelease() {
-  std::unique_ptr P(new A());
+  std::unique_ptr P(new A()); // expected-note {{Smart pointer 'P' is 
constructed}}
   P.release(); // expected-note {{Smart pointer 'P' is released and set to 
null}}

vrnithinkumar wrote:
> NoQ wrote:
> > Ok, these notes shouldn't be there; a note on `.release()` is sufficient to 
> > understand the warning and it looks like that's one more place where we 
> > should mark the region as uninteresting.
> > 
> > Can you try to debug why did they suddenly show up?
> I checked the exploded graph for this test case. 
> Before the bug fix, there exists a path where the no Note Tag is added to the 
> corresponding `CXXConstructExpr`. After the fix removed this branching theres 
> always a Note Tag on Ctr. {F12591752}
> 
> Since the note on .release() is sufficient to understand the warning and I 
> agree we should mark this region as uninteresting.
Ok, fair enough! Let's add a FIXME.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85796

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


[PATCH] D85796: [Analysis] Bug fix for exploded graph branching in evalCall for constructor

2020-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:619
   getCheckerManager().runCheckersForEvalCall(DstEvaluated, *I, *Call, 
*this,
- CallOpts);
+ CallOpts, Bldr);
   }

vrnithinkumar wrote:
> NoQ wrote:
> > We should probably delete the copy-constructor for node builders. I've no 
> > idea what it's supposed to do anyway and the whole problem that we're 
> > having here is due to there being //too many// of them already.
> So we should disable the copying of `NodeBuilder` and create a heap allocated 
> `NodeBuilder` and use pointer to pass around functions?
No-no, keep it on the stack and don't pass it around *at all*.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85796

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


[PATCH] D85799: [DebugInfo] Add -fuse-ctor-homing cc1 flag so we can turn on constructor homing only if limited debug info is already on.

2020-08-12 Thread David Blaikie via Phabricator via cfe-commits
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks great, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85799

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


[PATCH] D85798: Split Preprocessor/init.c test. NFC.

2020-08-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/Preprocessor/init-arm.c:19
+// ARM:#define __CHAR16_TYPE__ unsigned short
+// ARM:#define __CHAR32_TYPE__ unsigned int
+// ARM:#define __CHAR_BIT__ 8

Consider spending a bit more time and adding `-NEXT:` whenever applicable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85798

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


[PATCH] D85778: More accurately compute the ranges of possible values for +, -, *, &, %.

2020-08-12 Thread Richard Trieu via Phabricator via cfe-commits
rtrieu accepted this revision.
rtrieu added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang/lib/Sema/SemaChecking.cpp:10166
+  /// The number of bits active in the int. Note that this includes exactly one
+  /// sign bit if !NoNegative.
   unsigned Width;

NonNegative


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85778

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


[PATCH] D80263: [HeaderSearch] Fix processing #import-ed headers multiple times with modules enabled.

2020-08-12 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80263

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


[PATCH] D84458: [Modules] Improve error message when cannot find parent module for submodule definition.

2020-08-12 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84458

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


[PATCH] D85576: [clang][Fuchsia] Add relative-vtables multilib

2020-08-12 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan updated this revision to Diff 285219.
leonardchan edited the summary of this revision.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85576

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  
clang/test/Driver/Inputs/basic_fuchsia_tree/lib/aarch64-fuchsia/c++/relative-vtables+noexcept/libc++.so
  
clang/test/Driver/Inputs/basic_fuchsia_tree/lib/aarch64-fuchsia/c++/relative-vtables/libc++.so
  
clang/test/Driver/Inputs/basic_fuchsia_tree/lib/x86_64-fuchsia/c++/relative-vtables+noexcept/libc++.so
  
clang/test/Driver/Inputs/basic_fuchsia_tree/lib/x86_64-fuchsia/c++/relative-vtables/libc++.so
  clang/test/Driver/fuchsia.cpp


Index: clang/test/Driver/fuchsia.cpp
===
--- clang/test/Driver/fuchsia.cpp
+++ clang/test/Driver/fuchsia.cpp
@@ -86,8 +86,20 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: -fuse-ld=lld 2>&1\
 // RUN: | FileCheck %s 
-check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-ASAN-NOEXCEPT-X86
+// RUN: %clangxx %s -### --target=x86_64-fuchsia -fc++-abi=fuchsia \
+// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1\
+// RUN: | FileCheck %s 
-check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-RELATIVE-VTABLES-X86
+// RUN: %clangxx %s -### --target=x86_64-fuchsia -fc++-abi=fuchsia 
-fno-exceptions \
+// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1\
+// RUN: | FileCheck %s 
-check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-RELATIVE-VTABLES-NOEXCEPT-X86
 // CHECK-MULTILIB-X86: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-MULTILIB-ASAN-X86: 
"-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}c++{{/|}}asan"
 // CHECK-MULTILIB-NOEXCEPT-X86: 
"-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}c++{{/|}}noexcept"
 // CHECK-MULTILIB-ASAN-NOEXCEPT-X86: 
"-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}c++{{/|}}asan+noexcept"
+// CHECK-MULTILIB-RELATIVE-VTABLES-X86: 
"-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}c++{{/|}}relative-vtables"
+// CHECK-MULTILIB-RELATIVE-VTABLES-NOEXCEPT-X86: 
"-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}c++{{/|}}relative-vtables+noexcept"
 // CHECK-MULTILIB-X86: 
"-L{{.*}}{{/|}}..{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}c++"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -8,6 +8,7 @@
 
 #include "Fuchsia.h"
 #include "CommonArgs.h"
+#include "clang/Basic/TargetCXXABI.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
@@ -208,6 +209,13 @@
   .flag("+fsanitize=address")
   .flag("-fexceptions")
   .flag("+fno-exceptions"));
+  // Use the relative vtables ABI.
+  Multilibs.push_back(Multilib("relative-vtables", {}, {}, 4)
+  .flag("+fc++-abi=fuchsia"));
+  Multilibs.push_back(Multilib("relative-vtables+noexcept", {}, {}, 5)
+  .flag("+fc++-abi=fuchsia")
+  .flag("-fexceptions")
+  .flag("+fno-exceptions"));
   Multilibs.FilterOut([&](const Multilib ) {
 std::vector RD = FilePaths(M);
 return std::all_of(RD.begin(), RD.end(), [&](std::string P) {
@@ -220,6 +228,11 @@
   Args.hasFlag(options::OPT_fexceptions, options::OPT_fno_exceptions, 
true),
   "fexceptions", Flags);
   addMultilibFlag(getSanitizerArgs().needsAsanRt(), "fsanitize=address", 
Flags);
+
+  StringRef ABI = Args.getLastArgValue(options::OPT_fcxx_abi_EQ);
+  bool UsingRelativeVTables = TargetCXXABI::isABI(ABI) && 
TargetCXXABI::getKind(ABI) == TargetCXXABI::Fuchsia;
+  addMultilibFlag(UsingRelativeVTables, "fc++-abi=fuchsia", Flags);
+
   Multilibs.setFilePathsCallback(FilePaths);
 
   if (Multilibs.select(Flags, SelectedMultilib))
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -212,14 +212,24 @@
 
set(RUNTIMES_${target}-unknown-fuchsia+asan+noexcept_LIBCXXABI_ENABLE_EXCEPTIONS
 OFF CACHE BOOL "")
 
set(RUNTIMES_${target}-unknown-fuchsia+asan+noexcept_LIBCXX_ENABLE_EXCEPTIONS 
OFF CACHE BOOL "")
 
+
set(RUNTIMES_${target}-unknown-fuchsia+relative-vtables_LLVM_BUILD_COMPILER_RT 
OFF CACHE BOOL "")
+

[PATCH] D85799: [DebugInfo] Add -fuse-ctor-homing cc1 flag so we can turn on constructor homing only if limited debug info is already on.

2020-08-12 Thread Amy Huang via Phabricator via cfe-commits
akhuang updated this revision to Diff 285214.
akhuang added a comment.

Add more extensive check that -fuse-ctor-homing only does something when 
-debug-info-kind=limited


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85799

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/debug-info-ctor-homing-flag.cpp


Index: clang/test/CodeGenCXX/debug-info-ctor-homing-flag.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-ctor-homing-flag.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang -cc1 -debug-info-kind=constructor -emit-llvm %s -o - \
+// RUN:| FileCheck %s -check-prefix=CTOR_HOMING
+// RUN: %clang -cc1 -debug-info-kind=limited -fuse-ctor-homing -emit-llvm %s 
-o - \
+// RUN:| FileCheck %s -check-prefix=CTOR_HOMING
+// RUN: %clang -cc1 -debug-info-kind=standalone -fuse-ctor-homing -emit-llvm 
%s -o - \
+// RUN:| FileCheck %s -check-prefix=FULL_DEBUG
+// RUN: %clang -cc1 -debug-info-kind=line-tables-only -fuse-ctor-homing 
-emit-llvm %s -o - \
+// RUN:| FileCheck %s -check-prefix=NO_DEBUG
+// RUN: %clang -cc1 -fuse-ctor-homing -emit-llvm %s -o - \
+// RUN:| FileCheck %s -check-prefix=NO_DEBUG
+
+
+// This tests that the -fuse-ctor-homing is only used if limited debug info 
would have
+// been used otherwise.
+
+// CTOR_HOMING: !DICompositeType(tag: DW_TAG_structure_type, name: 
"A"{{.*}}flags: DIFlagFwdDecl
+// FULL_DEBUG: !DICompositeType(tag: DW_TAG_structure_type, name: 
"A"{{.*}}DIFlagTypePassByValue
+// NO_DEBUG-NOT: !DICompositeType(tag: DW_TAG_structure_type, name: "A"
+struct A {
+  A();
+} TestA;
+
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -775,6 +775,12 @@
 else
   Opts.setDebugInfo(static_cast(Val));
   }
+  // If -fuse-ctor-homing is set and limited debug info is already on, then use
+  // constructor homing.
+  if (Arg *A = Args.getLastArg(OPT_fuse_ctor_homing))
+if (Opts.getDebugInfo() == codegenoptions::LimitedDebugInfo)
+  Opts.setDebugInfo(codegenoptions::DebugInfoConstructor);
+
   if (Arg *A = Args.getLastArg(OPT_debugger_tuning_EQ)) {
 unsigned Val = llvm::StringSwitch(A->getValue())
.Case("gdb", unsigned(llvm::DebuggerKind::GDB))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3691,6 +3691,8 @@
   AutoNormalizeEnum;
 def fno_math_builtin : Flag<["-"], "fno-math-builtin">,
   HelpText<"Disable implicit builtin knowledge of math functions">;
+def fuse_ctor_homing: Flag<["-"], "fuse-ctor-homing">,
+HelpText<"Use constructor homing if we are using limited debug info 
already">;
 }
 
 def disable_llvm_verifier : Flag<["-"], "disable-llvm-verifier">,


Index: clang/test/CodeGenCXX/debug-info-ctor-homing-flag.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-ctor-homing-flag.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang -cc1 -debug-info-kind=constructor -emit-llvm %s -o - \
+// RUN:| FileCheck %s -check-prefix=CTOR_HOMING
+// RUN: %clang -cc1 -debug-info-kind=limited -fuse-ctor-homing -emit-llvm %s -o - \
+// RUN:| FileCheck %s -check-prefix=CTOR_HOMING
+// RUN: %clang -cc1 -debug-info-kind=standalone -fuse-ctor-homing -emit-llvm %s -o - \
+// RUN:| FileCheck %s -check-prefix=FULL_DEBUG
+// RUN: %clang -cc1 -debug-info-kind=line-tables-only -fuse-ctor-homing -emit-llvm %s -o - \
+// RUN:| FileCheck %s -check-prefix=NO_DEBUG
+// RUN: %clang -cc1 -fuse-ctor-homing -emit-llvm %s -o - \
+// RUN:| FileCheck %s -check-prefix=NO_DEBUG
+
+
+// This tests that the -fuse-ctor-homing is only used if limited debug info would have
+// been used otherwise.
+
+// CTOR_HOMING: !DICompositeType(tag: DW_TAG_structure_type, name: "A"{{.*}}flags: DIFlagFwdDecl
+// FULL_DEBUG: !DICompositeType(tag: DW_TAG_structure_type, name: "A"{{.*}}DIFlagTypePassByValue
+// NO_DEBUG-NOT: !DICompositeType(tag: DW_TAG_structure_type, name: "A"
+struct A {
+  A();
+} TestA;
+
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -775,6 +775,12 @@
 else
   Opts.setDebugInfo(static_cast(Val));
   }
+  // If -fuse-ctor-homing is set and limited debug info is already on, then use
+  // constructor homing.
+  if (Arg *A = Args.getLastArg(OPT_fuse_ctor_homing))
+if (Opts.getDebugInfo() == codegenoptions::LimitedDebugInfo)
+  

[clang] bd08e0c - PR47143: Don't crash while constant-evaluating value-initialization of

2020-08-12 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2020-08-12T16:53:45-07:00
New Revision: bd08e0cf1cb1f1f294e4253ba5907ec4c81b05fe

URL: 
https://github.com/llvm/llvm-project/commit/bd08e0cf1cb1f1f294e4253ba5907ec4c81b05fe
DIFF: 
https://github.com/llvm/llvm-project/commit/bd08e0cf1cb1f1f294e4253ba5907ec4c81b05fe.diff

LOG: PR47143: Don't crash while constant-evaluating value-initialization of
an array of unknown bound as the initializer of an array new expression.

Added: 


Modified: 
clang/lib/AST/ExprConstant.cpp
clang/test/SemaCXX/constant-expression-cxx2a.cpp

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 4238adb5a947..448a683c9088 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -8987,6 +8987,7 @@ bool PointerExprEvaluator::VisitCXXNewExpr(const 
CXXNewExpr *E) {
   const Expr *Init = E->getInitializer();
   const InitListExpr *ResizedArrayILE = nullptr;
   const CXXConstructExpr *ResizedArrayCCE = nullptr;
+  bool ValueInit = false;
 
   QualType AllocType = E->getAllocatedType();
   if (Optional ArraySize = E->getArraySize()) {
@@ -9030,7 +9031,14 @@ bool PointerExprEvaluator::VisitCXXNewExpr(const 
CXXNewExpr *E) {
 //   -- the new-initializer is a braced-init-list and the number of
 //  array elements for which initializers are provided [...]
 //  exceeds the number of elements to initialize
-if (Init && !isa(Init)) {
+if (!Init) {
+  // No initialization is performed.
+} else if (isa(Init) ||
+   isa(Init)) {
+  ValueInit = true;
+} else if (auto *CCE = dyn_cast(Init)) {
+  ResizedArrayCCE = CCE;
+} else {
   auto *CAT = Info.Ctx.getAsConstantArrayType(Init->getType());
   assert(CAT && "unexpected type for array initializer");
 
@@ -9053,8 +9061,6 @@ bool PointerExprEvaluator::VisitCXXNewExpr(const 
CXXNewExpr *E) {
   // special handling for this case when we initialize.
   if (InitBound != AllocBound)
 ResizedArrayILE = cast(Init);
-} else if (Init) {
-  ResizedArrayCCE = cast(Init);
 }
 
 AllocType = Info.Ctx.getConstantArrayType(AllocType, ArrayBound, nullptr,
@@ -9115,7 +9121,11 @@ bool PointerExprEvaluator::VisitCXXNewExpr(const 
CXXNewExpr *E) {
   return false;
   }
 
-  if (ResizedArrayILE) {
+  if (ValueInit) {
+ImplicitValueInitExpr VIE(AllocType);
+if (!EvaluateInPlace(*Val, Info, Result, ))
+  return false;
+  } else if (ResizedArrayILE) {
 if (!EvaluateArrayNewInitList(Info, Result, *Val, ResizedArrayILE,
   AllocType))
   return false;

diff  --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp 
b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
index f66f380b635f..344797bafb11 100644
--- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -950,6 +950,20 @@ namespace dynamic_alloc {
 p = new ((std::align_val_t)n) char[n];
 p = new char(n);
   }
+
+  namespace PR47143 {
+constexpr char *f(int n) {
+  return new char[n]();
+}
+const char *p = f(3);
+constexpr bool test() {
+  char *p = f(3);
+  bool result = !p[0] && !p[1] && !p[2];
+  delete [] p;
+  return result;
+}
+static_assert(test());
+  }
 }
 
 struct placement_new_arg {};



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


[PATCH] D85799: [DebugInfo] Add -fuse-ctor-homing cc1 flag so we can turn on constructor homing only if limited debug info is already on.

2020-08-12 Thread Amy Huang via Phabricator via cfe-commits
akhuang added a comment.

In D85799#2214330 , @dblaikie wrote:

> If possible, could you test the negative cases too? That -fuse-ctor-homing 
> doesn't override -debug-info-kind=line-tables-only or no -debug-info-kind at 
> all?

Oh, good point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85799

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


[PATCH] D85796: [Analysis] Bug fix for exploded graph branching in evalCall for constructor

2020-08-12 Thread Nithin VR via Phabricator via cfe-commits
vrnithinkumar added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:619
   getCheckerManager().runCheckersForEvalCall(DstEvaluated, *I, *Call, 
*this,
- CallOpts);
+ CallOpts, Bldr);
   }

NoQ wrote:
> We should probably delete the copy-constructor for node builders. I've no 
> idea what it's supposed to do anyway and the whole problem that we're having 
> here is due to there being //too many// of them already.
So we should disable the copying of `NodeBuilder` and create a heap allocated 
`NodeBuilder` and use pointer to pass around functions?



Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:39
 void derefAfterRelease() {
-  std::unique_ptr P(new A());
+  std::unique_ptr P(new A()); // expected-note {{Smart pointer 'P' is 
constructed}}
   P.release(); // expected-note {{Smart pointer 'P' is released and set to 
null}}

NoQ wrote:
> Ok, these notes shouldn't be there; a note on `.release()` is sufficient to 
> understand the warning and it looks like that's one more place where we 
> should mark the region as uninteresting.
> 
> Can you try to debug why did they suddenly show up?
I checked the exploded graph for this test case. 
Before the bug fix, there exists a path where the no Note Tag is added to the 
corresponding `CXXConstructExpr`. After the fix removed this branching theres 
always a Note Tag on Ctr. {F12591752}

Since the note on .release() is sufficient to understand the warning and I 
agree we should mark this region as uninteresting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85796

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


[PATCH] D83338: [PowerPC][Power10] Implemented Vector Shift Builtins

2020-08-12 Thread Albion Fung via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3136cbe29e74: [PowerPC]  Implement Vector Shift Builtins 
(authored by Conanap).

Changed prior to commit:
  https://reviews.llvm.org/D83338?vs=283980=285208#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83338

Files:
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/p10-vector-shift.ll

Index: llvm/test/CodeGen/PowerPC/p10-vector-shift.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/p10-vector-shift.ll
@@ -0,0 +1,74 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
+; RUN:   FileCheck %s
+
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
+; RUN:   FileCheck %s
+
+; These test cases demonstrate that the vector shift quadword instructions
+; introduced within Power10 are correctly exploited.
+
+define dso_local <1 x i128> @test_vec_vslq(<1 x i128> %a, <1 x i128> %b) {
+; CHECK-LABEL: test_vec_vslq:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vslq v2, v2, v3
+; CHECK-NEXT:blr
+entry:
+  %rem = urem <1 x i128> %b, 
+  %shl = shl <1 x i128> %a, %rem
+  ret <1 x i128> %shl
+}
+
+define dso_local <1 x i128> @test_vec_vsrq(<1 x i128> %a, <1 x i128> %b) {
+; CHECK-LABEL: test_vec_vsrq:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vsrq v2, v2, v3
+; CHECK-NEXT:blr
+entry:
+  %rem = urem <1 x i128> %b, 
+  %shr = lshr <1 x i128> %a, %rem
+  ret <1 x i128> %shr
+}
+
+define dso_local <1 x i128> @test_vec_vsraq(<1 x i128> %a, <1 x i128> %b) {
+; CHECK-LABEL: test_vec_vsraq:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vsraq v2, v2, v3
+; CHECK-NEXT:blr
+entry:
+  %rem = urem <1 x i128> %b, 
+  %shr = ashr <1 x i128> %a, %rem
+  ret <1 x i128> %shr
+}
+
+define dso_local <1 x i128> @test_vec_vslq2(<1 x i128> %a, <1 x i128> %b) {
+; CHECK-LABEL: test_vec_vslq2:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vslq v2, v2, v3
+; CHECK-NEXT:blr
+entry:
+  %shl = shl <1 x i128> %a, %b
+  ret <1 x i128> %shl
+}
+
+define dso_local <1 x i128> @test_vec_vsrq2(<1 x i128> %a, <1 x i128> %b) {
+; CHECK-LABEL: test_vec_vsrq2:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vsrq v2, v2, v3
+; CHECK-NEXT:blr
+entry:
+  %shr = lshr <1 x i128> %a, %b
+  ret <1 x i128> %shr
+}
+
+define dso_local <1 x i128> @test_vec_vsraq2(<1 x i128> %a, <1 x i128> %b) {
+; CHECK-LABEL: test_vec_vsraq2:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vsraq v2, v2, v3
+; CHECK-NEXT:blr
+entry:
+  %shr = ashr <1 x i128> %a, %b
+  ret <1 x i128> %shr
+}
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -1288,6 +1288,18 @@
 (EXTRACT_SUBREG (XVTLSBB (COPY_TO_REGCLASS $XB, VSRC)), sub_lt)>;
   def : Pat<(i32 (int_ppc_vsx_xvtlsbb v16i8:$XB, 0)),
 (EXTRACT_SUBREG (XVTLSBB (COPY_TO_REGCLASS $XB, VSRC)), sub_eq)>;
+  def : Pat<(v1i128 (shl v1i128:$VRA, v1i128:$VRB)),
+(v1i128 (VSLQ v1i128:$VRA, v1i128:$VRB))>;
+  def : Pat<(v1i128 (PPCshl v1i128:$VRA, v1i128:$VRB)),
+(v1i128 (VSLQ v1i128:$VRA, v1i128:$VRB))>;
+  def : Pat<(v1i128 (srl v1i128:$VRA, v1i128:$VRB)),
+(v1i128 (VSRQ v1i128:$VRA, v1i128:$VRB))>;
+  def : Pat<(v1i128 (PPCsrl v1i128:$VRA, v1i128:$VRB)),
+(v1i128 (VSRQ v1i128:$VRA, v1i128:$VRB))>;
+  def : Pat<(v1i128 (sra v1i128:$VRA, v1i128:$VRB)),
+(v1i128 (VSRAQ v1i128:$VRA, v1i128:$VRB))>;
+  def : Pat<(v1i128 (PPCsra v1i128:$VRA, v1i128:$VRB)),
+(v1i128 (VSRAQ v1i128:$VRA, v1i128:$VRB))>;
 }
 
 let AddedComplexity = 400, Predicates = [IsISA3_1] in {
Index: llvm/lib/Target/PowerPC/PPCISelLowering.cpp
===
--- llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -1128,6 +1128,9 @@
   if (Subtarget.has64BitSupport())
 setOperationAction(ISD::PREFETCH, MVT::Other, Legal);
 
+  if (Subtarget.isISA3_1())
+setOperationAction(ISD::SRA, MVT::v1i128, Legal);
+
   setOperationAction(ISD::READCYCLECOUNTER, MVT::i64, isPPC64 ? Legal : Custom);
 
   if (!isPPC64) {
Index: clang/test/CodeGen/builtins-ppc-p10vector.c
===
--- 

[clang] 3136cbe - [PowerPC] Implement Vector Shift Builtins

2020-08-12 Thread Albion Fung via cfe-commits

Author: Albion Fung
Date: 2020-08-12T18:26:58-05:00
New Revision: 3136cbe29e74e19e6cb71c5ce71e4b92a63d03d8

URL: 
https://github.com/llvm/llvm-project/commit/3136cbe29e74e19e6cb71c5ce71e4b92a63d03d8
DIFF: 
https://github.com/llvm/llvm-project/commit/3136cbe29e74e19e6cb71c5ce71e4b92a63d03d8.diff

LOG: [PowerPC]  Implement Vector Shift Builtins

This patch implements the builtins for the vector shifts (shl, srl, sra), and
adds the appropriate test cases for these builtins. The builtins utilize the
vector shift instructions introduced within ISA 3.1.

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

Added: 
llvm/test/CodeGen/PowerPC/p10-vector-shift.ll

Modified: 
clang/lib/Headers/altivec.h
clang/test/CodeGen/builtins-ppc-p10vector.c
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
llvm/lib/Target/PowerPC/PPCInstrPrefix.td

Removed: 




diff  --git a/clang/lib/Headers/altivec.h b/clang/lib/Headers/altivec.h
index d0a3b198351c..ac4182613cdd 100644
--- a/clang/lib/Headers/altivec.h
+++ b/clang/lib/Headers/altivec.h
@@ -17321,6 +17321,53 @@ vec_test_lsbb_all_zeros(vector unsigned char __a) {
   return __builtin_vsx_xvtlsbb(__a, 0);
 }
 #endif /* __VSX__ */
+
+/* vs[l | r | ra] */
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_sl(vector unsigned __int128 __a, vector unsigned __int128 __b) {
+  return __a << (__b % (vector unsigned __int128)(sizeof(unsigned __int128) *
+  __CHAR_BIT__));
+}
+
+static __inline__ vector signed __int128 __ATTRS_o_ai
+vec_sl(vector signed __int128 __a, vector unsigned __int128 __b) {
+  return __a << (__b % (vector unsigned __int128)(sizeof(unsigned __int128) *
+  __CHAR_BIT__));
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_sr(vector unsigned __int128 __a, vector unsigned __int128 __b) {
+  return __a >> (__b % (vector unsigned __int128)(sizeof(unsigned __int128) *
+  __CHAR_BIT__));
+}
+
+static __inline__ vector signed __int128 __ATTRS_o_ai
+vec_sr(vector signed __int128 __a, vector unsigned __int128 __b) {
+  return (
+  vector signed __int128)(((vector unsigned __int128)__a) >>
+  (__b %
+   (vector unsigned __int128)(sizeof(
+  unsigned 
__int128) *
+  __CHAR_BIT__)));
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_sra(vector unsigned __int128 __a, vector unsigned __int128 __b) {
+  return (
+  vector unsigned __int128)(((vector signed __int128)__a) >>
+(__b %
+ (vector unsigned __int128)(sizeof(
+unsigned 
__int128) *
+__CHAR_BIT__)));
+}
+
+static __inline__ vector signed __int128 __ATTRS_o_ai
+vec_sra(vector signed __int128 __a, vector unsigned __int128 __b) {
+  return __a >> (__b % (vector unsigned __int128)(sizeof(unsigned __int128) *
+  __CHAR_BIT__));
+}
+
 #endif /* __POWER10_VECTOR__ */
 
 #undef __ATTRS_o_ai

diff  --git a/clang/test/CodeGen/builtins-ppc-p10vector.c 
b/clang/test/CodeGen/builtins-ppc-p10vector.c
index 2279d9c57c86..a575f5a924c5 100644
--- a/clang/test/CodeGen/builtins-ppc-p10vector.c
+++ b/clang/test/CodeGen/builtins-ppc-p10vector.c
@@ -8,6 +8,7 @@
 
 #include 
 
+vector signed __int128 vi128a;
 vector signed char vsca, vscb;
 vector unsigned char vuca, vucb, vucc;
 vector signed short vssa, vssb;
@@ -778,6 +779,49 @@ void test_vec_xst_trunc_ull(vector unsigned __int128 __a, 
signed long long __b,
   vec_xst_trunc(__a, __b, __c);
 }
 
+vector unsigned __int128 test_vec_slq_unsigned (void) {
+  // CHECK-LABEL: test_vec_slq_unsigned
+  // CHECK: shl <1 x i128> %{{.+}}, %{{.+}}
+  // CHECK: ret <1 x i128> %{{.+}}
+  return vec_sl(vui128a, vui128b);
+}
+
+vector signed __int128 test_vec_slq_signed (void) {
+  // CHECK-LABEL: test_vec_slq_signed
+  // CHECK: shl <1 x i128> %{{.+}}, %{{.+}}
+  // CHECK: ret <1 x i128>
+  return vec_sl(vi128a, vui128a);
+}
+
+vector unsigned __int128 test_vec_srq_unsigned (void) {
+  // CHECK-LABEL: test_vec_srq_unsigned
+  // CHECK: lshr <1 x i128> %{{.+}}, %{{.+}}
+  // CHECK: ret <1 x i128>
+  return vec_sr(vui128a, vui128b);
+}
+
+vector signed __int128 test_vec_srq_signed (void) {
+  // CHECK-LABEL: test_vec_srq_signed
+  // CHECK: lshr <1 x i128> %{{.+}}, %{{.+}}
+  // CHECK: ret <1 x i128>
+  return vec_sr(vi128a, vui128a);
+}
+
+vector unsigned __int128 test_vec_sraq_unsigned (void) {
+  // CHECK-LABEL: test_vec_sraq_unsigned
+  // CHECK: ashr <1 x i128> %{{.+}}, %{{.+}}
+  // CHECK: ret <1 x i128>
+  

[PATCH] D85799: [DebugInfo] Add -fuse-ctor-homing cc1 flag so we can turn on constructor homing only if limited debug info is already on.

2020-08-12 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

If possible, could you test the negative cases too? That -fuse-ctor-homing 
doesn't override -debug-info-kind=line-tables-only or no -debug-info-kind at 
all?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85799

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


[PATCH] D85176: [Coverage] Enable emitting gap area between macros

2020-08-12 Thread Zequan Wu via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa31c89c1b7a0: [Coverage] Enable emitting gap area between 
macros (authored by zequanwu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85176

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/if.cpp
  clang/test/CoverageMapping/macro-expressions.cpp
  clang/test/CoverageMapping/macroparams2.c
  clang/test/CoverageMapping/macros.c
  clang/test/CoverageMapping/macroscopes.cpp
  clang/test/CoverageMapping/moremacros.c

Index: clang/test/CoverageMapping/moremacros.c
===
--- clang/test/CoverageMapping/moremacros.c
+++ clang/test/CoverageMapping/moremacros.c
@@ -9,16 +9,18 @@
   // CHECK-NEXT: File 0, [[@LINE+1]]:7 -> [[@LINE+1]]:12 = #0
   if (!argc) {} // CHECK: File 0, [[@LINE]]:14 -> [[@LINE]]:16 = #1
 
-  // CHECK-NEXT: File 0, [[@LINE+3]]:7 -> [[@LINE+3]]:12 = #0
+  // CHECK-NEXT: File 0, [[@LINE+4]]:7 -> [[@LINE+4]]:12 = #0
+  // CHECK-NEXT: Gap,File 0, [[@LINE+3]]:13 -> [[@LINE+3]]:14 = #2
   // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:14 -> [[@LINE+2]]:19 = #2
   // CHECK-NEXT: File 0, [[@LINE+1]]:19 -> [[@LINE+4]]:8 = #2
   if (!argc) LBRAC
 return 0;
   // CHECK-NEXT: Expansion,File 0, [[@LINE+1]]:3 -> [[@LINE+1]]:8 = #2
-  RBRAC // CHECK-NEXT: [[@LINE]]:8 -> [[@LINE+6]]:3 = (#0 - #2)
+  RBRAC // CHECK-NEXT: [[@LINE]]:8 -> [[@LINE+7]]:3 = (#0 - #2)
 
-  // CHECK-NEXT: File 0, [[@LINE+4]]:3 -> [[@LINE+15]]:2 = (#0 - #2)
-  // CHECK-NEXT: File 0, [[@LINE+3]]:7 -> [[@LINE+3]]:12 = (#0 - #2)
+  // CHECK-NEXT: File 0, [[@LINE+5]]:3 -> [[@LINE+16]]:2 = (#0 - #2)
+  // CHECK-NEXT: File 0, [[@LINE+4]]:7 -> [[@LINE+4]]:12 = (#0 - #2)
+  // CHECK-NEXT: Gap,File 0, [[@LINE+3]]:13 -> [[@LINE+3]]:14 = #3
   // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:14 -> [[@LINE+2]]:19 = #3
   // CHECK-NEXT: File 0, [[@LINE+1]]:19 -> [[@LINE+3]]:4 = #3
   if (!argc) LBRAC
Index: clang/test/CoverageMapping/macroscopes.cpp
===
--- clang/test/CoverageMapping/macroscopes.cpp
+++ clang/test/CoverageMapping/macroscopes.cpp
@@ -61,13 +61,15 @@
   starts_a_scope
   ends_a_scope
 
-  // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:3 -> [[@LINE+2]]:17 = #0
+  // CHECK-NEXT: Expansion,File 0, [[@LINE+3]]:3 -> [[@LINE+3]]:17 = #0
+  // CHECK-NEXT: Gap,File 0, [[@LINE+2]]:3 -> [[@LINE+3]]:5 = #8
   // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:5 -> [[@LINE+2]]:16 = #8
   starts_a_while
 simple_stmt;
 
   x = 0;
-  // CHECK-NEXT: Expansion,File 0, [[@LINE+4]]:3 -> [[@LINE+4]]:17 = #0
+  // CHECK-NEXT: Expansion,File 0, [[@LINE+5]]:3 -> [[@LINE+5]]:17 = #0
+  // CHECK-NEXT: Gap,File 0, [[@LINE+4]]:3 -> [[@LINE+4]]:18 = #9
   // CHECK-NEXT: File 0, [[@LINE+3]]:18 -> [[@LINE+5]]:15 = #9
   // CHECK-NEXT: Expansion,File 0, [[@LINE+3]]:5 -> [[@LINE+3]]:16 = #9
   // CHECK-NEXT: Expansion,File 0, [[@LINE+3]]:3 -> [[@LINE+3]]:15 = #9
Index: clang/test/CoverageMapping/macros.c
===
--- clang/test/CoverageMapping/macros.c
+++ clang/test/CoverageMapping/macros.c
@@ -38,29 +38,32 @@
 // CHECK-NEXT: File 2, 4:17 -> 4:22 = #0
 
 // CHECK-NEXT: func4
-void func4() { // CHECK-NEXT: File 0, [[@LINE]]:14 -> [[@LINE+6]]:2 = #0
+void func4() { // CHECK-NEXT: File 0, [[@LINE]]:14 -> [[@LINE+7]]:2 = #0
   int i = 0;
   while (i++ < 10) // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE]]:18 = (#0 + #1)
-if (i < 5) // CHECK: File 0, [[@LINE]]:5 -> [[@LINE+2]]:14 = #1
+if (i < 5) // CHECK: File 0, [[@LINE]]:5 -> [[@LINE+3]]:14 = #1
// CHECK-NEXT: File 0, [[@LINE-1]]:9 -> [[@LINE-1]]:14 = #1
+   // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:15 -> [[@LINE+1]]:7 = #2
   MACRO_2; // CHECK-NEXT: Expansion,File 0, [[@LINE]]:7 -> [[@LINE]]:14 = #2
 }
 // CHECK-NEXT: File 1, 4:17 -> 4:22 = #2
 // CHECK-NOT: File 1
 
 // CHECK-NEXT: func5
-void func5() { // CHECK-NEXT: File 0, [[@LINE]]:14 -> [[@LINE+4]]:2 = #0
+void func5() { // CHECK-NEXT: File 0, [[@LINE]]:14 -> [[@LINE+5]]:2 = #0
   int i = 0;
   if (i > 5) // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:12 = #0
+ // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:13 -> [[@LINE+1]]:5 = #1
 MACRO_3; // CHECK-NEXT: Expansion,File 0, [[@LINE]]:5 -> [[@LINE]]:12 = #1
 }
 // CHECK-NEXT: Expansion,File 1, 6:17 -> 6:24 = #1
 // CHECK-NEXT: File 2, 4:17 -> 4:22 = #1
 
 // CHECK-NEXT: func6
-void func6(unsigned count) { // CHECK-NEXT: File 0, [[@LINE]]:28 -> [[@LINE+4]]:2 = #0
-begin:   // CHECK-NEXT: File 0, [[@LINE]]:1 -> [[@LINE+3]]:2 = #1
+void func6(unsigned count) { // CHECK-NEXT: File 0, [[@LINE]]:28 -> [[@LINE+5]]:2 = #0
+begin:   // CHECK-NEXT: File 0, [[@LINE]]:1 -> [[@LINE+4]]:2 = #1
 if 

[clang] a31c89c - [Coverage] Enable emitting gap area between macros

2020-08-12 Thread Zequan Wu via cfe-commits

Author: Zequan Wu
Date: 2020-08-12T16:25:27-07:00
New Revision: a31c89c1b7a0a2fd3e2c0b8a587a60921abf4abd

URL: 
https://github.com/llvm/llvm-project/commit/a31c89c1b7a0a2fd3e2c0b8a587a60921abf4abd
DIFF: 
https://github.com/llvm/llvm-project/commit/a31c89c1b7a0a2fd3e2c0b8a587a60921abf4abd.diff

LOG: [Coverage] Enable emitting gap area between macros

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

Added: 


Modified: 
clang/lib/CodeGen/CoverageMappingGen.cpp
clang/test/CoverageMapping/if.cpp
clang/test/CoverageMapping/macro-expressions.cpp
clang/test/CoverageMapping/macroparams2.c
clang/test/CoverageMapping/macros.c
clang/test/CoverageMapping/macroscopes.cpp
clang/test/CoverageMapping/moremacros.c

Removed: 




diff  --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 9a7096b8d1d0..e6e1b2111935 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -864,22 +864,13 @@ struct CounterCoverageMappingBuilder
   /// Find a valid gap range between \p AfterLoc and \p BeforeLoc.
   Optional findGapAreaBetween(SourceLocation AfterLoc,
SourceLocation BeforeLoc) {
-// If the start and end locations of the gap are both within the same macro
-// file, the range may not be in source order.
-if (AfterLoc.isMacroID() || BeforeLoc.isMacroID())
-  return None;
+AfterLoc = SM.getExpansionLoc(AfterLoc);
+BeforeLoc = SM.getExpansionLoc(BeforeLoc);
 if (!SM.isWrittenInSameFile(AfterLoc, BeforeLoc))
   return None;
 return {{AfterLoc, BeforeLoc}};
   }
 
-  /// Find the source range after \p AfterStmt and before \p BeforeStmt.
-  Optional findGapAreaBetween(const Stmt *AfterStmt,
-   const Stmt *BeforeStmt) {
-return findGapAreaBetween(getPreciseTokenLocEnd(getEnd(AfterStmt)),
-  getStart(BeforeStmt));
-  }
-
   /// Emit a gap region between \p StartLoc and \p EndLoc with the given count.
   void fillGapAreaWithCount(SourceLocation StartLoc, SourceLocation EndLoc,
 Counter Count) {
@@ -1044,7 +1035,8 @@ struct CounterCoverageMappingBuilder
 adjustForOutOfOrderTraversal(getEnd(S));
 
 // The body count applies to the area immediately after the increment.
-auto Gap = findGapAreaBetween(S->getCond(), S->getBody());
+auto Gap = findGapAreaBetween(getPreciseTokenLocEnd(S->getRParenLoc()),
+  getStart(S->getBody()));
 if (Gap)
   fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), BodyCount);
 
@@ -1261,7 +1253,8 @@ struct CounterCoverageMappingBuilder
 propagateCounts(ParentCount, S->getCond());
 
 // The 'then' count applies to the area immediately after the condition.
-auto Gap = findGapAreaBetween(S->getCond(), S->getThen());
+auto Gap = findGapAreaBetween(getPreciseTokenLocEnd(S->getRParenLoc()),
+  getStart(S->getThen()));
 if (Gap)
   fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
 
@@ -1271,7 +1264,8 @@ struct CounterCoverageMappingBuilder
 Counter ElseCount = subtractCounters(ParentCount, ThenCount);
 if (const Stmt *Else = S->getElse()) {
   // The 'else' count applies to the area immediately after the 'then'.
-  Gap = findGapAreaBetween(S->getThen(), Else);
+  Gap = findGapAreaBetween(getPreciseTokenLocEnd(getEnd(S->getThen())),
+   getStart(Else));
   if (Gap)
 fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ElseCount);
   extendRegion(Else);

diff  --git a/clang/test/CoverageMapping/if.cpp 
b/clang/test/CoverageMapping/if.cpp
index 8ffc09d29a3c..5cbc974aaf04 100644
--- a/clang/test/CoverageMapping/if.cpp
+++ b/clang/test/CoverageMapping/if.cpp
@@ -44,10 +44,3 @@ int main() {// CHECK: File 0, 
[[@LINE]]:12 -> {{[0-9]+}}:2 =
 
   return 0;
 }
-
-#define FOO true
-
-// CHECK-LABEL: _Z7ternaryv:
-void ternary() {
-  true ? FOO : FOO; // CHECK-NOT: Gap,{{.*}}, [[@LINE]]:8 ->
-}

diff  --git a/clang/test/CoverageMapping/macro-expressions.cpp 
b/clang/test/CoverageMapping/macro-expressions.cpp
index 60afc5238b9e..6ac82523fef8 100644
--- a/clang/test/CoverageMapping/macro-expressions.cpp
+++ b/clang/test/CoverageMapping/macro-expressions.cpp
@@ -57,7 +57,8 @@ void foo(int i) {
   // CHECK: File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:12 = #1
   if (0) {}
 
-  // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:7 -> [[@LINE+2]]:11 = #0
+  // CHECK-NEXT: Expansion,File 0, [[@LINE+3]]:7 -> [[@LINE+3]]:11 = #0
+  // CHECK-NEXT: Gap,File 0, [[@LINE+2]]:15 -> [[@LINE+2]]:16 = #2
   // CHECK-NEXT: File 0, [[@LINE+1]]:16 -> [[@LINE+1]]:18 = #2
   if (EXPR(i)) {}
   // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:9 -> [[@LINE+2]]:14 = (#0 + #3)

[PATCH] D85810: [clang] Pass-through remarks options to lld

2020-08-12 Thread Wei Wang via Phabricator via cfe-commits
weiwang updated this revision to Diff 285205.
weiwang added a comment.

update test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/remarks-pass-through.c

Index: clang/test/Driver/remarks-pass-through.c
===
--- /dev/null
+++ clang/test/Driver/remarks-pass-through.c
@@ -0,0 +1,28 @@
+// This test verifies remarks options pass-through into linker(lld)
+
+// no pass-through if lto is disabled
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | not FileCheck %s
+
+// no pass-through if linker is not lld
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=gold -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | not FileCheck %s
+
+// pass-through cases
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -fsave-optimization-record=some-format -foptimization-record-file=FOO.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-CUSTOM
+//
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -Rpass=inline -Rpass-missed=inline -Rpass-analysis=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-RPASS
+//
+// CHECK: "--opt-remarks-filename" "FOO.opt.ld.yaml"
+// CHECK: "--opt-remarks-passes" "inline"
+// CHECK: "--opt-remarks-format" "yaml"
+// CHECK: "--opt-remarks-hotness-threshold=100"
+//
+// CHECK-CUSTOM: "--opt-remarks-filename" "FOO.txt.opt.ld.some-format"
+// CHECK-CUSTOM: "--opt-remarks-format" "some-format"
+// CHECK-CUSTOM: "--opt-remarks-hotness-threshold=100"
+//
+// CHECK-RPASS: "-mllvm" "-pass-remarks=inline"
+// CHECK-RPASS: "-mllvm" "-pass-remarks-missed=inline"
+// CHECK-RPASS: "-mllvm" "-pass-remarks-analysis=inline"
+// CHECK-RPASS: "--opt-remarks-hotness-threshold=100"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -60,6 +60,95 @@
 using namespace clang;
 using namespace llvm::opt;
 
+// Remarks option pass-through only happens when
+// 1). single arch target
+// 2). linker is lld
+static bool checkRemarksOptions(StringRef LinkerPath, const ArgList ,
+const llvm::Triple ) {
+  bool hasMultipleArchs =
+  Triple.isOSDarwin() && Args.getAllArgValues(options::OPT_arch).size() > 1;
+
+  bool isLLD = llvm::sys::path::filename(LinkerPath) == "ld.lld" ||
+   llvm::sys::path::stem(LinkerPath) != "ld.lld";
+  if (hasMultipleArchs || !isLLD)
+return false;
+  return true;
+}
+
+static void renderRpassOptions(const ArgList , ArgStringList ) {
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_EQ)) {
+CmdArgs.push_back("-mllvm");
+std::string Passes = std::string("-pass-remarks=") + A->getValue();
+CmdArgs.push_back(Args.MakeArgString(Passes));
+  }
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_missed_EQ)) {
+CmdArgs.push_back("-mllvm");
+std::string Passes = std::string("-pass-remarks-missed=") + A->getValue();
+CmdArgs.push_back(Args.MakeArgString(Passes));
+  }
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_analysis_EQ)) {
+CmdArgs.push_back("-mllvm");
+std::string Passes = std::string("-pass-remarks-analysis=") + A->getValue();
+CmdArgs.push_back(Args.MakeArgString(Passes));
+  }
+}
+
+static void renderRemarksOptions(const ArgList , ArgStringList ,
+ const llvm::Triple ,
+ const InputInfo ,
+ const InputInfo ) {
+  StringRef Format = "yaml";
+  if (const Arg *A = Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
+Format = A->getValue();
+
+  CmdArgs.push_back("--opt-remarks-filename");
+
+  SmallString<128> F;
+  const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ);
+  if (A) {
+F = A->getValue();
+  } else {
+if (Output.isFilename())
+  F = Output.getFilename();
+
+if (F.empty()) {
+  // Use the input filename.
+  F = llvm::sys::path::stem(Input.getBaseInput());
+}
+  }
+  // Append "opt.ld." to the end of the file name.
+  SmallString<32> Extension;
+  Extension += ".opt.ld.";
+  Extension += Format;
+
+  CmdArgs.push_back(Args.MakeArgString(F + Extension));
+
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_foptimization_record_passes_EQ)) {
+

[PATCH] D85808: [remarks] Optimization remarks hotness filtering from profile summary

2020-08-12 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added a comment.

In D85808#2212297 , @weiwang wrote:

> This is the 3rd of 3 dependent patches:
>
> 1. [lld] Enable remarks hotness filtering in lld: 
> https://reviews.llvm.org/D85809
> 2. [clang] Pass-through remarks options to lld: 
> https://reviews.llvm.org/D85810
> 3. [remarks] Optimization remarks hotness filtering from profile summary: 
> https://reviews.llvm.org/D85808

You can relate these as Parent/Child patches in Phabricator. See "Edit Related 
Revisions" on the top right. It helps reviewers to see the relationship and 
keep track of what patches are still open.

Note these relationships get set up automatically when you upload a patch with 
"Depends on Dx." in the description.




Comment at: lld/ELF/Config.h:280
+  // If threshold option is not specified, it is disabled by default.
+  llvm::Optional optRemarksHotnessThreshold = 0;
 

Since this field is being added in patch D85809 as an unsigned, why not add it 
as llvm::Optional<> there to start with instead of adding and then changing?

Ditto for a number of other places in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

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


[PATCH] D85799: [DebugInfo] Add -fuse-ctor-homing cc1 flag so we can turn on constructor homing only if limited debug info is already on.

2020-08-12 Thread Amy Huang via Phabricator via cfe-commits
akhuang added a comment.

Yep, just added a line to the existing ctor homing test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85799

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


[PATCH] D85799: [DebugInfo] Add -fuse-ctor-homing cc1 flag so we can turn on constructor homing only if limited debug info is already on.

2020-08-12 Thread Amy Huang via Phabricator via cfe-commits
akhuang updated this revision to Diff 285203.
akhuang added a comment.

Add test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85799

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/debug-info-limited-ctor.cpp


Index: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
===
--- clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
+++ clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
@@ -1,5 +1,8 @@
 // RUN: %clang -cc1 -debug-info-kind=constructor -emit-llvm %s -o - | 
FileCheck %s
 
+// This tests the -fuse-ctor-homing flag.
+// RUN: %clang -cc1 -fuse-ctor-homing -debug-info-kind=limited -emit-llvm %s 
-o - | FileCheck %s
+
 // CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: 
"A"{{.*}}DIFlagTypePassByValue
 struct A {
 } TestA;
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -775,6 +775,12 @@
 else
   Opts.setDebugInfo(static_cast(Val));
   }
+  // If -fuse-ctor-homing is set and limited debug info is already on, then use
+  // constructor homing.
+  if (Arg *A = Args.getLastArg(OPT_fuse_ctor_homing))
+if (Opts.getDebugInfo() == codegenoptions::LimitedDebugInfo)
+  Opts.setDebugInfo(codegenoptions::DebugInfoConstructor);
+
   if (Arg *A = Args.getLastArg(OPT_debugger_tuning_EQ)) {
 unsigned Val = llvm::StringSwitch(A->getValue())
.Case("gdb", unsigned(llvm::DebuggerKind::GDB))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3691,6 +3691,8 @@
   AutoNormalizeEnum;
 def fno_math_builtin : Flag<["-"], "fno-math-builtin">,
   HelpText<"Disable implicit builtin knowledge of math functions">;
+def fuse_ctor_homing: Flag<["-"], "fuse-ctor-homing">,
+HelpText<"Use constructor homing if we are using limited debug info 
already">;
 }
 
 def disable_llvm_verifier : Flag<["-"], "disable-llvm-verifier">,


Index: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
===
--- clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
+++ clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
@@ -1,5 +1,8 @@
 // RUN: %clang -cc1 -debug-info-kind=constructor -emit-llvm %s -o - | FileCheck %s
 
+// This tests the -fuse-ctor-homing flag.
+// RUN: %clang -cc1 -fuse-ctor-homing -debug-info-kind=limited -emit-llvm %s -o - | FileCheck %s
+
 // CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "A"{{.*}}DIFlagTypePassByValue
 struct A {
 } TestA;
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -775,6 +775,12 @@
 else
   Opts.setDebugInfo(static_cast(Val));
   }
+  // If -fuse-ctor-homing is set and limited debug info is already on, then use
+  // constructor homing.
+  if (Arg *A = Args.getLastArg(OPT_fuse_ctor_homing))
+if (Opts.getDebugInfo() == codegenoptions::LimitedDebugInfo)
+  Opts.setDebugInfo(codegenoptions::DebugInfoConstructor);
+
   if (Arg *A = Args.getLastArg(OPT_debugger_tuning_EQ)) {
 unsigned Val = llvm::StringSwitch(A->getValue())
.Case("gdb", unsigned(llvm::DebuggerKind::GDB))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3691,6 +3691,8 @@
   AutoNormalizeEnum;
 def fno_math_builtin : Flag<["-"], "fno-math-builtin">,
   HelpText<"Disable implicit builtin knowledge of math functions">;
+def fuse_ctor_homing: Flag<["-"], "fuse-ctor-homing">,
+HelpText<"Use constructor homing if we are using limited debug info already">;
 }
 
 def disable_llvm_verifier : Flag<["-"], "disable-llvm-verifier">,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85099: [UpdateTestChecks] Match unnamed values like "@[0-9]+" and "![0-9]+"

2020-08-12 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added inline comments.



Comment at: llvm/utils/UpdateTestChecks/common.py:390
+for nameless_value in nameless_values:
+if re.fullmatch(nameless_value.ir_prefix + r'[0-9]+?', var, 
re.IGNORECASE):
+warn("Change IR value name '%s' to prevent possible conflict with 
scripted FileCheck name." % (var,))

thakis wrote:
> This is Py3.4+. Per https://llvm.org/docs/GettingStarted.html#id7 we still 
> support 2.7, which is why all my bots run that, which is why this breaks my 
> bots :) (...and everyone still using Python 2.7 locally). Can you do this 
> some other way? Maybe just surround the re with "^..$'?
Addressed and restored actual functionality in 
07448c550457d2afb1e7d69254a58ad44dece3d2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85099

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


[PATCH] D85819: [SyntaxTree] Split tests

2020-08-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision.
gribozavr2 added a comment.
This revision is now accepted and ready to land.

Please also consider splitting the file into multiple.




Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1238
+namespace n {
+template
+struct ST {

Indent -2.



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1412
 
-TEST_P(SyntaxTreeTest, QualifiedIdDecltype) {
+TEST_P(SyntaxTreeTest, QualifiedId_Decltype) {
   if (!GetParam().isCXX11OrLater()) {

For consistency with other test names: `QualifiedId_DecltypeSpecifier`



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:2646
 
-TEST_P(SyntaxTreeTest, NestedBinaryOperator) {
+TEST_P(SyntaxTreeTest, NestedBinaryOperator_Parenthesis) {
   EXPECT_TRUE(treeDumpEqual(

BinaryOperator_NestedWithParenthesis



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:2690
+
+TEST_P(SyntaxTreeTest, NestedBinaryOperator_Associativity) {
+  EXPECT_TRUE(treeDumpEqual(

BinaryOperator_Associativity



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:2750
+
+TEST_P(SyntaxTreeTest, NestedBinaryOperator_Precedence) {
+  EXPECT_TRUE(treeDumpEqual(

BinaryOperator_Precedence



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:2805
 
-TEST_P(SyntaxTreeTest, UserDefinedBinaryOperator) {
+TEST_P(SyntaxTreeTest, UserDefinedOperator_Assignment) {
   if (!GetParam().isCXX()) {

UserDefinedOperator => OverloadedOperator?

"user-defined" seems to suggest that the operator was previously not a thing in 
C++.



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:3021
+
+TEST_P(SyntaxTreeTest, UserDefinedOperator_Shift) {
+  if (!GetParam().isCXX()) {

UserDefinedOperator_LeftShift



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:3161
+
+TEST_P(SyntaxTreeTest, UserDefinedOperator_ArrowPointer) {
+  if (!GetParam().isCXX()) {

UserDefinedOperator_PointerToMember



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:3230
 
-TEST_P(SyntaxTreeTest, UserDefinedUnaryPrefixOperator) {
+TEST_P(SyntaxTreeTest, UserDefinedOperator_PrefixIncr) {
   if (!GetParam().isCXX()) {

PrefixIncrement



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:3284
+
+TEST_P(SyntaxTreeTest, UserDefinedOperator_Exclam) {
+  if (!GetParam().isCXX()) {

UserDefinedOperator_Negation



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:3393
 
-TEST_P(SyntaxTreeTest, UserDefinedUnaryPostfixOperator) {
+TEST_P(SyntaxTreeTest, UserDefinedOperator_PostfixIncr) {
   if (!GetParam().isCXX()) {

PostfixIncrement

Also, group it right after prefix increment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85819

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


[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

2020-08-12 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan updated this revision to Diff 285192.
leonardchan marked 2 inline comments as done.
leonardchan edited the summary of this revision.
leonardchan added a comment.

- Check for lowercase flag values
- Remove fuchsia-specific changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/TargetCXXABI.def
  clang/include/clang/Basic/TargetCXXABI.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Frontend/invalid-cxx-abi.cpp

Index: clang/test/Frontend/invalid-cxx-abi.cpp
===
--- /dev/null
+++ clang/test/Frontend/invalid-cxx-abi.cpp
@@ -0,0 +1,16 @@
+// These should succeed.
+// RUN: %clang_cc1 -fc++-abi=itanium %s
+// RUN: %clang_cc1 -fc++-abi=arm %s
+// RUN: %clang_cc1 -fc++-abi=ios %s
+// RUN: %clang_cc1 -fc++-abi=ios64 %s
+// RUN: %clang_cc1 -fc++-abi=aarch64 %s
+// RUN: %clang_cc1 -fc++-abi=mips %s
+// RUN: %clang_cc1 -fc++-abi=webassembly %s
+// RUN: %clang_cc1 -fc++-abi=fuchsia %s
+// RUN: %clang_cc1 -fc++-abi=xl %s
+// RUN: %clang_cc1 -fc++-abi=microsoft %s
+
+// RUN: not %clang_cc1 -fc++-abi=InvalidABI %s 2>&1 | FileCheck %s -check-prefix=INVALID
+// RUN: not %clang_cc1 -fc++-abi=Fuchsia %s 2>&1 | FileCheck %s -check-prefix=CASE-SENSITIVE
+// INVALID: error: Invalid C++ ABI name 'InvalidABI'
+// CASE-SENSITIVE: error: Invalid C++ ABI name 'Fuchsia'
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3465,6 +3465,13 @@
   Args.hasFlag(OPT_fexperimental_relative_cxx_abi_vtables,
OPT_fno_experimental_relative_cxx_abi_vtables,
/*default=*/false);
+
+  // The value can be empty, which indicates the system default should be used.
+  Opts.CXXABI = Args.getLastArgValue(OPT_fcxx_abi_EQ).str();
+  if (!Opts.CXXABI.empty() && !TargetCXXABI::isABI(Opts.CXXABI)) {
+Diags.Report(diag::err_invalid_cxx_abi) << Opts.CXXABI;
+Opts.CXXABI = "";
+  }
 }
 
 static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4965,6 +4965,9 @@
/*Default=*/false))
 Args.AddLastArg(CmdArgs, options::OPT_ffixed_point);
 
+  if (Arg *A = Args.getLastArg(options::OPT_fcxx_abi_EQ))
+CmdArgs.push_back(Args.MakeArgString("-fc++-abi=" + Twine(A->getValue(;
+
   // Handle -{std, ansi, trigraphs} -- take the last of -{std, ansi}
   // (-ansi is equivalent to -std=c89 or -std=c++98).
   //
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -542,7 +542,7 @@
 }
 
 CodeGen::CGCXXABI *CodeGen::CreateItaniumCXXABI(CodeGenModule ) {
-  switch (CGM.getTarget().getCXXABI().getKind()) {
+  switch (CGM.getContext().getCXXABIKind()) {
   // For IR-generation purposes, there's no significant difference
   // between the ARM and iOS ABIs.
   case TargetCXXABI::GenericARM:
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -75,19 +75,14 @@
 static const char AnnotationSection[] = "llvm.metadata";
 
 static CGCXXABI *createCXXABI(CodeGenModule ) {
-  switch (CGM.getTarget().getCXXABI().getKind()) {
-  case TargetCXXABI::Fuchsia:
-  case TargetCXXABI::GenericAArch64:
-  case TargetCXXABI::GenericARM:
-  case TargetCXXABI::iOS:
-  case TargetCXXABI::iOS64:
-  case TargetCXXABI::WatchOS:
-  case TargetCXXABI::GenericMIPS:
-  case TargetCXXABI::GenericItanium:
-  case TargetCXXABI::WebAssembly:
-  case TargetCXXABI::XL:
+  switch (CGM.getContext().getCXXABIKind()) {
+#define ITANIUM_CXXABI(Name, Str) case TargetCXXABI::Name:
+#define CXXABI(Name, Str)
+#include "clang/Basic/TargetCXXABI.def"
 return CreateItaniumCXXABI(CGM);
-  case TargetCXXABI::Microsoft:
+#define MICROSOFT_CXXABI(Name, Str) case TargetCXXABI::Name:
+#define CXXABI(Name, Str)
+#include "clang/Basic/TargetCXXABI.def"
 return CreateMicrosoftCXXABI(CGM);
   }
 
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -879,10 

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

2020-08-12 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan added inline comments.



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:275
+  if (DriverArgs.getLastArgValue(options::OPT_fcxx_abi_EQ) == "Fuchsia")
+CC1Args.push_back("-fexperimental-relative-c++-abi-vtables");
 }

phosek wrote:
> mcgrathr wrote:
> > It's surprising to me that this is the way to do this.  Isn't there code in 
> > the actual front end that tests the TargetCXXABI value?  That seems like 
> > the place where it makes sense to have Fuchsia imply specific settings, 
> > rather than in the driver.
> > 
> Couldn't we make this the default in `FuchsiaCXXABI` (akin to 
> `HasThisReturn`)?
> It's surprising to me that this is the way to do this. Isn't there code in 
> the actual front end that tests the TargetCXXABI value? That seems like the 
> place where it makes sense to have Fuchsia imply specific settings, rather 
> than in the driver.

Yeah, this was just some confusion on my part but we addressed this offline. 
Removing this specific part for now since we'll just want to land the flag in 
this patch. In a future patch when we want to change the meaning behind 
`-fc++-abi=fuchsia`, I'll probably add something like 
`TargetCXXABI::canUseRelVTables()` which checks the ABI kind instead of the 
flag directly.

> Couldn't we make this the default in FuchsiaCXXABI (akin to HasThisReturn)?

Right now, relative vtables are enabled through 
https://github.com/llvm/llvm-project/blob/62ef1cb2079123b86878e4bfed3c14db448f1373/clang/lib/AST/ASTContext.cpp#L10652,
 so we can't explicitly turn this on though `FuchsiaCXXABI` in codegen, but 
down the line we can update the condition to also make it the default in for 
the `Fuchsia` enum kind. The reason why it's not exclusively controlled in 
codegen is because there are parts outside of codegen where we need to tweak 
Itanium, specifically `VCallAndVBaseOffsetBuilder` which exists in the AST 
level and we need to modify to properly calculate vtable offsets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802

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


[PATCH] D85176: [Coverage] Enable emitting gap area between macros

2020-08-12 Thread Vedant Kumar via Phabricator via cfe-commits
vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

Thanks, looks good to me! Please check for any issues in a stage2 coverage 
build before landing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85176

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


[PATCH] D85728: [Analyzer] Support for the new variadic isa<> and isa_and_not_null<> in CastValueChecker

2020-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:289
+  for (QualType CastToTy: CastToTyVec) {
+if (CastFromTy->isPointerType())
+  CastToTy = C.getASTContext().getPointerType(CastToTy);

Hmm, is this phabricator's way of displaying tabs?



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:270-272
+default:
+  llvm_unreachable("Invalid template argument for isa<> or "
+   "isa_and_nonnull<>");

martong wrote:
> baloghadamsoftware wrote:
> > NoQ wrote:
> > > We shouldn't crash when code under analysis doesn't match our expectation.
> > The question is whether we can get any other template argument kind for 
> > `llvm::isa<>()` and `llvm::isa_and_nonnull<>()` than `Type` or `Pack`?
> I think it is worth to ponder about the below case. What if `isa` is used in 
> a dependent context (i.e. it is used inside another template)?
> ```
> /// The template argument is an expression, and we've not resolved it to 
> one
> /// of the other forms yet, either because it's dependent or because we're
> /// representing a non-canonical template argument (for instance, in a
> /// TemplateSpecializationType).
> Expression,
> ```
Aha, yup, that's better, thanks.

> The question is whether we can get any other template argument kind for 
> `llvm::isa<>()` and `llvm::isa_and_nonnull<>()` than Type or Pack?

It's a more general problem: what about a completely unrelated template 
function that is accidentally called `llvm::isa<>()` or 
`llvm::isa_and_nonnull<>()`? Or what if `llvm::isa<>()`'s/ 
`llvm::isa_and_nonnull<>()`'s implementation changes again? If it's at all 
possible in any template function, we should take it into account and at least 
provide an early return, we shouldn't rule out that possibility by looking at 
the name only.

> I think it is worth to ponder about the below case. What if `isa` is used in 
> a dependent context (i.e. it is used inside another template)?

Ok, this one probably doesn't happen, because we don't ever try to symbolically 
execute templated ASTs (that aren't fully instantiated).


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

https://reviews.llvm.org/D85728

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


[PATCH] D85817: [analyzer] Fix crash with pointer to members values

2020-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Aha, i see, fair enough!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85817

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


[PATCH] D83088: Introduce CfgTraits abstraction

2020-08-12 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

In D83088#2213864 , @nhaehnle wrote:

> In D83088#2213802 , @dblaikie wrote:
>
>> In D83088#2213797 , @nhaehnle wrote:
>>
>>> In D83088#2208611 , @dblaikie 
>>> wrote:
>>>
 This seems like a strange hybrid between a static-polymorphism (with 
 traits) and dynamic polymorphism (with base classes/virtual functions). 
 Could this more readily be just one or the other? (sounds like you're 
 leaning towards dynamic polymorphism)
>>>
>>> No, it's very much this way on purpose. The idea is to support the same set 
>>> of functionality as much as possible in both static **and** dynamic 
>>> polymorphism.
>>
>> Could it be implemented statically as a primary interface, with a dynamic 
>> wrapper? (eg: a base class, then a derived class template that takes the 
>> static CFG type to wrap into the dynamic type) keeping the two concepts more 
>> clearly separated?
>
> That is how it is implemented. CfgTraits is the primary static interface, and 
> then CfgInterface / CfgInterfaceImpl is the dynamic wrapper.

Ah, fair enough. The inheritance details in the traits class confused me a 
bit/I had a hard time following, with all the features being in the one patch. 
Might be easier separately, but not sure.

Would it be possible for this not to use traits - I know @asbirlea and I had 
trouble with some things using GraphTraits owing to the traits API. An 
alternative would be to describe a CFGGraph concept (same as a standard 
container concept, for instance) - where there is a concrete graph object and 
that object is queried for things like nodes, edges, etc. (actually one of the 
significant things we tripped over was the API choice to navigate edges from a 
node itself without any extra state - which meant nodes/edge iteration had to 
carry state (potentially pointers back to the graph, etc) to be able to 
manifest their edges - trait or concept could both address this by, for traits, 
passing the graph as well as the node when querying the trait for edges, or for 
a concept passing the node back to the graph to query for edges).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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


[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

2020-08-12 Thread Petr Hosek via Phabricator via cfe-commits
phosek added inline comments.



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:275
+  if (DriverArgs.getLastArgValue(options::OPT_fcxx_abi_EQ) == "Fuchsia")
+CC1Args.push_back("-fexperimental-relative-c++-abi-vtables");
 }

mcgrathr wrote:
> It's surprising to me that this is the way to do this.  Isn't there code in 
> the actual front end that tests the TargetCXXABI value?  That seems like the 
> place where it makes sense to have Fuchsia imply specific settings, rather 
> than in the driver.
> 
Couldn't we make this the default in `FuchsiaCXXABI` (akin to `HasThisReturn`)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802

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


[PATCH] D83088: Introduce CfgTraits abstraction

2020-08-12 Thread Nicolai Hähnle via Phabricator via cfe-commits
nhaehnle added a comment.

In D83088#2213802 , @dblaikie wrote:

> In D83088#2213797 , @nhaehnle wrote:
>
>> In D83088#2208611 , @dblaikie wrote:
>>
>>> This seems like a strange hybrid between a static-polymorphism (with 
>>> traits) and dynamic polymorphism (with base classes/virtual functions). 
>>> Could this more readily be just one or the other? (sounds like you're 
>>> leaning towards dynamic polymorphism)
>>
>> No, it's very much this way on purpose. The idea is to support the same set 
>> of functionality as much as possible in both static **and** dynamic 
>> polymorphism.
>
> Could it be implemented statically as a primary interface, with a dynamic 
> wrapper? (eg: a base class, then a derived class template that takes the 
> static CFG type to wrap into the dynamic type) keeping the two concepts more 
> clearly separated?

That is how it is implemented. CfgTraits is the primary static interface, and 
then CfgInterface / CfgInterfaceImpl is the dynamic wrapper.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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


[PATCH] D85807: [Target] Cache the command line derived feature map in TargetOptions.

2020-08-12 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment.

Thanks @craig.topper ! `++beers_owed;`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85807

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


[clang] 5c1fe4e - [Target] Cache the command line derived feature map in TargetOptions.

2020-08-12 Thread Craig Topper via cfe-commits

Author: Craig Topper
Date: 2020-08-12T12:37:23-07:00
New Revision: 5c1fe4e20f887286baac6989943a0875e12834fe

URL: 
https://github.com/llvm/llvm-project/commit/5c1fe4e20f887286baac6989943a0875e12834fe
DIFF: 
https://github.com/llvm/llvm-project/commit/5c1fe4e20f887286baac6989943a0875e12834fe.diff

LOG: [Target] Cache the command line derived feature map in TargetOptions.

We can use this to remove some calls to initFeatureMap from Sema
and CodeGen when a function doesn't have a target attribute.

This reduces compile time of the linux kernel where this map
is needed to diagnose some inline assembly constraints based
on whether sse, avx, or avx512 is enabled.

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

Added: 


Modified: 
clang/include/clang/Basic/TargetOptions.h
clang/lib/AST/ASTContext.cpp
clang/lib/Basic/Targets.cpp
clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/TargetOptions.h 
b/clang/include/clang/Basic/TargetOptions.h
index bbe86aebb074..4a5d469b8e54 100644
--- a/clang/include/clang/Basic/TargetOptions.h
+++ b/clang/include/clang/Basic/TargetOptions.h
@@ -54,6 +54,10 @@ class TargetOptions {
   /// be a list of strings starting with by '+' or '-'.
   std::vector Features;
 
+  /// The map of which features have been enabled disabled based on the command
+  /// line.
+  llvm::StringMap FeatureMap;
+
   /// Supported OpenCL extensions and optional core features.
   OpenCLOptions SupportedOpenCLOptions;
 

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 544bbb5b0740..fd3e94fbf3c2 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -11186,8 +11186,7 @@ void 
ASTContext::getFunctionFeatureMap(llvm::StringMap ,
 std::vector Features(FeaturesTmp.begin(), FeaturesTmp.end());
 Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU, Features);
   } else {
-Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU,
-   Target->getTargetOpts().Features);
+FeatureMap = Target->getTargetOpts().FeatureMap;
   }
 }
 

diff  --git a/clang/lib/Basic/Targets.cpp b/clang/lib/Basic/Targets.cpp
index bd3c2d66f958..e4456ea7fa0f 100644
--- a/clang/lib/Basic/Targets.cpp
+++ b/clang/lib/Basic/Targets.cpp
@@ -666,14 +666,13 @@ TargetInfo::CreateTargetInfo(DiagnosticsEngine ,
 
   // Compute the default target features, we need the target to handle this
   // because features may have dependencies on one another.
-  llvm::StringMap Features;
-  if (!Target->initFeatureMap(Features, Diags, Opts->CPU,
+  if (!Target->initFeatureMap(Opts->FeatureMap, Diags, Opts->CPU,
   Opts->FeaturesAsWritten))
 return nullptr;
 
   // Add the features to the compile options.
   Opts->Features.clear();
-  for (const auto  : Features)
+  for (const auto  : Opts->FeatureMap)
 Opts->Features.push_back((F.getValue() ? "+" : "-") + F.getKey().str());
   // Sort here, so we handle the features in a predictable order. (This matters
   // when we're dealing with features that overlap.)

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
index 9440758a85b6..1b2608e9854a 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -4948,11 +4948,7 @@ bool 
CGOpenMPRuntimeGPU::hasAllocateAttributeForGlobalVar(const VarDecl *VD,
 static CudaArch getCudaArch(CodeGenModule ) {
   if (!CGM.getTarget().hasFeature("ptx"))
 return CudaArch::UNKNOWN;
-  llvm::StringMap Features;
-  CGM.getTarget().initFeatureMap(Features, CGM.getDiags(),
- CGM.getTarget().getTargetOpts().CPU,
- CGM.getTarget().getTargetOpts().Features);
-  for (const auto  : Features) {
+  for (const auto  : CGM.getTarget().getTargetOpts().FeatureMap) {
 if (Feature.getValue()) {
   CudaArch Arch = StringToCudaArch(Feature.getKey());
   if (Arch != CudaArch::UNKNOWN)



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


[PATCH] D85807: [Target] Cache the command line derived feature map in TargetOptions.

2020-08-12 Thread Craig Topper via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5c1fe4e20f88: [Target] Cache the command line derived 
feature map in TargetOptions. (authored by craig.topper).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85807

Files:
  clang/include/clang/Basic/TargetOptions.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp


Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -4948,11 +4948,7 @@
 static CudaArch getCudaArch(CodeGenModule ) {
   if (!CGM.getTarget().hasFeature("ptx"))
 return CudaArch::UNKNOWN;
-  llvm::StringMap Features;
-  CGM.getTarget().initFeatureMap(Features, CGM.getDiags(),
- CGM.getTarget().getTargetOpts().CPU,
- CGM.getTarget().getTargetOpts().Features);
-  for (const auto  : Features) {
+  for (const auto  : CGM.getTarget().getTargetOpts().FeatureMap) {
 if (Feature.getValue()) {
   CudaArch Arch = StringToCudaArch(Feature.getKey());
   if (Arch != CudaArch::UNKNOWN)
Index: clang/lib/Basic/Targets.cpp
===
--- clang/lib/Basic/Targets.cpp
+++ clang/lib/Basic/Targets.cpp
@@ -666,14 +666,13 @@
 
   // Compute the default target features, we need the target to handle this
   // because features may have dependencies on one another.
-  llvm::StringMap Features;
-  if (!Target->initFeatureMap(Features, Diags, Opts->CPU,
+  if (!Target->initFeatureMap(Opts->FeatureMap, Diags, Opts->CPU,
   Opts->FeaturesAsWritten))
 return nullptr;
 
   // Add the features to the compile options.
   Opts->Features.clear();
-  for (const auto  : Features)
+  for (const auto  : Opts->FeatureMap)
 Opts->Features.push_back((F.getValue() ? "+" : "-") + F.getKey().str());
   // Sort here, so we handle the features in a predictable order. (This matters
   // when we're dealing with features that overlap.)
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -11186,8 +11186,7 @@
 std::vector Features(FeaturesTmp.begin(), FeaturesTmp.end());
 Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU, Features);
   } else {
-Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU,
-   Target->getTargetOpts().Features);
+FeatureMap = Target->getTargetOpts().FeatureMap;
   }
 }
 
Index: clang/include/clang/Basic/TargetOptions.h
===
--- clang/include/clang/Basic/TargetOptions.h
+++ clang/include/clang/Basic/TargetOptions.h
@@ -54,6 +54,10 @@
   /// be a list of strings starting with by '+' or '-'.
   std::vector Features;
 
+  /// The map of which features have been enabled disabled based on the command
+  /// line.
+  llvm::StringMap FeatureMap;
+
   /// Supported OpenCL extensions and optional core features.
   OpenCLOptions SupportedOpenCLOptions;
 


Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -4948,11 +4948,7 @@
 static CudaArch getCudaArch(CodeGenModule ) {
   if (!CGM.getTarget().hasFeature("ptx"))
 return CudaArch::UNKNOWN;
-  llvm::StringMap Features;
-  CGM.getTarget().initFeatureMap(Features, CGM.getDiags(),
- CGM.getTarget().getTargetOpts().CPU,
- CGM.getTarget().getTargetOpts().Features);
-  for (const auto  : Features) {
+  for (const auto  : CGM.getTarget().getTargetOpts().FeatureMap) {
 if (Feature.getValue()) {
   CudaArch Arch = StringToCudaArch(Feature.getKey());
   if (Arch != CudaArch::UNKNOWN)
Index: clang/lib/Basic/Targets.cpp
===
--- clang/lib/Basic/Targets.cpp
+++ clang/lib/Basic/Targets.cpp
@@ -666,14 +666,13 @@
 
   // Compute the default target features, we need the target to handle this
   // because features may have dependencies on one another.
-  llvm::StringMap Features;
-  if (!Target->initFeatureMap(Features, Diags, Opts->CPU,
+  if (!Target->initFeatureMap(Opts->FeatureMap, Diags, Opts->CPU,
   Opts->FeaturesAsWritten))
 return nullptr;
 
   // Add the features to the compile options.
   Opts->Features.clear();
-  for (const auto  : Features)
+  for (const auto  : Opts->FeatureMap)
 Opts->Features.push_back((F.getValue() ? "+" : "-") + F.getKey().str());

[clang] 269bc3f - PR47138: Don't crash if the preferred alignment of an invalid record

2020-08-12 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2020-08-12T12:31:20-07:00
New Revision: 269bc3f5df6c3b75de515a48063c6941ef8fbbe6

URL: 
https://github.com/llvm/llvm-project/commit/269bc3f5df6c3b75de515a48063c6941ef8fbbe6
DIFF: 
https://github.com/llvm/llvm-project/commit/269bc3f5df6c3b75de515a48063c6941ef8fbbe6.diff

LOG: PR47138: Don't crash if the preferred alignment of an invalid record
type is requested.

Added: 


Modified: 
clang/lib/AST/ASTContext.cpp
clang/test/SemaCXX/alignof.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 4d708d57cabf..544bbb5b0740 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -2457,7 +2457,7 @@ unsigned ASTContext::getPreferredTypeAlign(const Type *T) 
const {
 return ABIAlign;
 
   if (const auto *RT = T->getAs()) {
-if (TI.AlignIsRequired)
+if (TI.AlignIsRequired || RT->getDecl()->isInvalidDecl())
   return ABIAlign;
 
 unsigned PreferredAlign = static_cast(

diff  --git a/clang/test/SemaCXX/alignof.cpp b/clang/test/SemaCXX/alignof.cpp
index f2854024da1a..64986d3103e1 100644
--- a/clang/test/SemaCXX/alignof.cpp
+++ b/clang/test/SemaCXX/alignof.cpp
@@ -102,3 +102,8 @@ typedef int __attribute__((aligned(16))) aligned_int;
 template 
 using template_alias = aligned_int;
 static_assert(alignof(template_alias) == 16, "Expected alignment of 16" 
);
+
+struct PR47138 {
+  invalid_type a; // expected-error {{unknown type}}
+};
+static_assert(__alignof__(PR47138) == 1, ""); // Don't crash.



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


[PATCH] D83088: Introduce CfgTraits abstraction

2020-08-12 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

In D83088#2213797 , @nhaehnle wrote:

> In D83088#2208611 , @dblaikie wrote:
>
>> This seems like a strange hybrid between a static-polymorphism (with traits) 
>> and dynamic polymorphism (with base classes/virtual functions). Could this 
>> more readily be just one or the other? (sounds like you're leaning towards 
>> dynamic polymorphism)
>
> No, it's very much this way on purpose. The idea is to support the same set 
> of functionality as much as possible in both static **and** dynamic 
> polymorphism.

Could it be implemented statically as a primary interface, with a dynamic 
wrapper? (eg: a base class, then a derived class template that takes the static 
CFG type to wrap into the dynamic type) keeping the two concepts more clearly 
separated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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


[PATCH] D83088: Introduce CfgTraits abstraction

2020-08-12 Thread Nicolai Hähnle via Phabricator via cfe-commits
nhaehnle added a comment.

In D83088#2208611 , @dblaikie wrote:

> This seems like a strange hybrid between a static-polymorphism (with traits) 
> and dynamic polymorphism (with base classes/virtual functions). Could this 
> more readily be just one or the other? (sounds like you're leaning towards 
> dynamic polymorphism)

No, it's very much this way on purpose. The idea is to support the same set of 
functionality as much as possible in both static **and** dynamic polymorphism.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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


[PATCH] D85193: [clang] Check `expr` inside `InitListChecker::UpdateStructuredListElement()`

2020-08-12 Thread Aleksandr Platonov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3fa0a039ab6f: [clang] Check `expr` inside 
`InitListChecker::UpdateStructuredListElement()` (authored by ArcsinX).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85193

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/Sema/init-invalid-struct-array.c


Index: clang/test/Sema/init-invalid-struct-array.c
===
--- /dev/null
+++ clang/test/Sema/init-invalid-struct-array.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+struct S {
+  Unknown u; // expected-error {{unknown type name 'Unknown'}}
+  int i;
+};
+// Should not crash
+struct S s[] = {[0].i = 0, [1].i = 1, {}};
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1585,10 +1585,7 @@
   IList->setInit(Index, ResultExpr);
 }
   }
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
   ++Index;
 }
 
@@ -1643,10 +1640,7 @@
   if (!VerifyOnly && expr)
 IList->setInit(Index, expr);
 
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex, expr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, expr);
   ++Index;
 }
 
@@ -1697,11 +1691,7 @@
   IList->setInit(Index, ResultExpr);
 }
   }
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex,
-ResultExpr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
   ++Index;
   return;
 }
@@ -3100,8 +3090,12 @@
 
   if (Expr *PrevInit = StructuredList->updateInit(SemaRef.Context,
   StructuredIndex, expr)) {
-// This initializer overwrites a previous initializer. Warn.
-diagnoseInitOverride(PrevInit, expr->getSourceRange());
+// This initializer overwrites a previous initializer.
+// No need to diagnose when `expr` is nullptr because a more relevant
+// diagnostic has already been issued and this diagnostic is potentially
+// noise.
+if (expr)
+  diagnoseInitOverride(PrevInit, expr->getSourceRange());
   }
 
   ++StructuredIndex;


Index: clang/test/Sema/init-invalid-struct-array.c
===
--- /dev/null
+++ clang/test/Sema/init-invalid-struct-array.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+struct S {
+  Unknown u; // expected-error {{unknown type name 'Unknown'}}
+  int i;
+};
+// Should not crash
+struct S s[] = {[0].i = 0, [1].i = 1, {}};
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1585,10 +1585,7 @@
   IList->setInit(Index, ResultExpr);
 }
   }
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
   ++Index;
 }
 
@@ -1643,10 +1640,7 @@
   if (!VerifyOnly && expr)
 IList->setInit(Index, expr);
 
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex, expr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, expr);
   ++Index;
 }
 
@@ -1697,11 +1691,7 @@
   IList->setInit(Index, ResultExpr);
 }
   }
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex,
-ResultExpr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
   ++Index;
   return;
 }
@@ -3100,8 +3090,12 @@
 
   if (Expr *PrevInit = StructuredList->updateInit(SemaRef.Context,
   StructuredIndex, expr)) {
-// This initializer overwrites a previous initializer. Warn.
-diagnoseInitOverride(PrevInit, expr->getSourceRange());
+// This initializer overwrites a previous initializer.
+// No need to diagnose when `expr` is nullptr because a more relevant
+// diagnostic has already been issued and this diagnostic is potentially
+// noise.
+if (expr)
+  diagnoseInitOverride(PrevInit, expr->getSourceRange());
   }
 
   ++StructuredIndex;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 3fa0a03 - [clang] Check `expr` inside `InitListChecker::UpdateStructuredListElement()`

2020-08-12 Thread Aleksandr Platonov via cfe-commits

Author: Aleksandr Platonov
Date: 2020-08-12T22:12:43+03:00
New Revision: 3fa0a039ab6f856e39dab973df56831b63ed51c5

URL: 
https://github.com/llvm/llvm-project/commit/3fa0a039ab6f856e39dab973df56831b63ed51c5
DIFF: 
https://github.com/llvm/llvm-project/commit/3fa0a039ab6f856e39dab973df56831b63ed51c5.diff

LOG: [clang] Check `expr` inside 
`InitListChecker::UpdateStructuredListElement()`

- Prevent nullptr-deference at try to emit warning for invalid `expr`
- Simplify `InitListChecker::UpdateStructuredListElement()` usages. We do not 
need to check `expr` and increment `StructuredIndex` (for invalid `expr`) 
before the call anymore.

Reviewed By: aaron.ballman

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

Added: 
clang/test/Sema/init-invalid-struct-array.c

Modified: 
clang/lib/Sema/SemaInit.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index f4cfae0f7d85..f63d600032ce 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -1585,10 +1585,7 @@ void InitListChecker::CheckScalarType(const 
InitializedEntity ,
   IList->setInit(Index, ResultExpr);
 }
   }
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
   ++Index;
 }
 
@@ -1643,10 +1640,7 @@ void InitListChecker::CheckReferenceType(const 
InitializedEntity ,
   if (!VerifyOnly && expr)
 IList->setInit(Index, expr);
 
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex, expr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, expr);
   ++Index;
 }
 
@@ -1697,11 +1691,7 @@ void InitListChecker::CheckVectorType(const 
InitializedEntity ,
   IList->setInit(Index, ResultExpr);
 }
   }
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex,
-ResultExpr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
   ++Index;
   return;
 }
@@ -3100,8 +3090,12 @@ void 
InitListChecker::UpdateStructuredListElement(InitListExpr *StructuredList,
 
   if (Expr *PrevInit = StructuredList->updateInit(SemaRef.Context,
   StructuredIndex, expr)) {
-// This initializer overwrites a previous initializer. Warn.
-diagnoseInitOverride(PrevInit, expr->getSourceRange());
+// This initializer overwrites a previous initializer.
+// No need to diagnose when `expr` is nullptr because a more relevant
+// diagnostic has already been issued and this diagnostic is potentially
+// noise.
+if (expr)
+  diagnoseInitOverride(PrevInit, expr->getSourceRange());
   }
 
   ++StructuredIndex;

diff  --git a/clang/test/Sema/init-invalid-struct-array.c 
b/clang/test/Sema/init-invalid-struct-array.c
new file mode 100644
index ..e234d68039c0
--- /dev/null
+++ b/clang/test/Sema/init-invalid-struct-array.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+struct S {
+  Unknown u; // expected-error {{unknown type name 'Unknown'}}
+  int i;
+};
+// Should not crash
+struct S s[] = {[0].i = 0, [1].i = 1, {}};



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


[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

2020-08-12 Thread Gui Andrade via Phabricator via cfe-commits
guiand added a comment.

In D82317#2212145 , @jdoerfert wrote:

> TBH, I don't see how this solves any problem. It just makes it a problem for 
> someone in the future... (FWIW, I say this being in full support of `noundef`)

That's true. At the same time, it might allow us to have a more baby-steps 
approach: be able to use the attribute now, update tests in their own time, and 
hopefully amortize the effect on downstream forks as @jrtc27 mentioned.

But the work for updating the tests is done now, in this patch, and as time 
goes on more and more of this work will need to be duplicated as the tests 
change every day.

There's one other option here, which has been mentioned a few times but I'll 
flesh it out for the purposes of discussion.

- Instead of updating the default test configuration to use 
`-disable-noundef-analysis`, we update every failing test (due to noundef) to 
use that flag in the RUN lines. This unblocks the clang noundef change.
- Then I can split up this patch into many smaller patches, probably organized 
by subfolder of `clang/test`, and these smaller patches would remove the 
`-disable-noundef-analysis` flags and add in the relevant `noundef` attributes.
- Test authors which don't want theirs changed could voice that they want to 
just keep the masking flag for their files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82317

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


[PATCH] D85799: [DebugInfo] Add -fuse-ctor-homing cc1 flag so we can turn on constructor homing only if limited debug info is already on.

2020-08-12 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

Looks good! Could you add a test case for this too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85799

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


[PATCH] D85806: [WebAssembly] Don't depend on the flags set by handleTargetFeatures in initFeatureMap.

2020-08-12 Thread Craig Topper via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2b8ad6b60408: [WebAssembly] Dont depend on the flags 
set by handleTargetFeatures in… (authored by craig.topper).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85806

Files:
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/lib/Basic/Targets/WebAssembly.h

Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -69,7 +69,8 @@
 MacroBuilder ) const override;
 
 private:
-  static void setSIMDLevel(llvm::StringMap , SIMDEnum Level);
+  static void setSIMDLevel(llvm::StringMap , SIMDEnum Level,
+   bool Enabled);
 
   bool
   initFeatureMap(llvm::StringMap , DiagnosticsEngine ,
@@ -77,6 +78,9 @@
  const std::vector ) const override;
   bool hasFeature(StringRef Feature) const final;
 
+  void setFeatureEnabled(llvm::StringMap , StringRef Name,
+ bool Enabled) const final;
+
   bool handleTargetFeatures(std::vector ,
 DiagnosticsEngine ) final;
 
Index: clang/lib/Basic/Targets/WebAssembly.cpp
===
--- clang/lib/Basic/Targets/WebAssembly.cpp
+++ clang/lib/Basic/Targets/WebAssembly.cpp
@@ -96,19 +96,43 @@
 }
 
 void WebAssemblyTargetInfo::setSIMDLevel(llvm::StringMap ,
- SIMDEnum Level) {
+ SIMDEnum Level, bool Enabled) {
+  if (Enabled) {
+switch (Level) {
+case UnimplementedSIMD128:
+  Features["unimplemented-simd128"] = true;
+  LLVM_FALLTHROUGH;
+case SIMD128:
+  Features["simd128"] = true;
+  LLVM_FALLTHROUGH;
+case NoSIMD:
+  break;
+}
+return;
+  }
+
   switch (Level) {
-  case UnimplementedSIMD128:
-Features["unimplemented-simd128"] = true;
-LLVM_FALLTHROUGH;
+  case NoSIMD:
   case SIMD128:
-Features["simd128"] = true;
+Features["simd128"] = false;
 LLVM_FALLTHROUGH;
-  case NoSIMD:
+  case UnimplementedSIMD128:
+Features["unimplemented-simd128"] = false;
 break;
   }
 }
 
+void WebAssemblyTargetInfo::setFeatureEnabled(llvm::StringMap ,
+  StringRef Name,
+  bool Enabled) const {
+  if (Name == "simd128")
+setSIMDLevel(Features, SIMD128, Enabled);
+  else if (Name == "unimplemented-simd128")
+setSIMDLevel(Features, UnimplementedSIMD128, Enabled);
+  else
+Features[Name] = Enabled;
+}
+
 bool WebAssemblyTargetInfo::initFeatureMap(
 llvm::StringMap , DiagnosticsEngine , StringRef CPU,
 const std::vector ) const {
@@ -119,30 +143,8 @@
 Features["atomics"] = true;
 Features["mutable-globals"] = true;
 Features["tail-call"] = true;
-setSIMDLevel(Features, SIMD128);
+setSIMDLevel(Features, SIMD128, true);
   }
-  // Other targets do not consider user-configured features here, but while we
-  // are actively developing new features it is useful to let user-configured
-  // features control availability of builtins
-  setSIMDLevel(Features, SIMDLevel);
-  if (HasNontrappingFPToInt)
-Features["nontrapping-fptoint"] = true;
-  if (HasSignExt)
-Features["sign-ext"] = true;
-  if (HasExceptionHandling)
-Features["exception-handling"] = true;
-  if (HasBulkMemory)
-Features["bulk-memory"] = true;
-  if (HasAtomics)
-Features["atomics"] = true;
-  if (HasMutableGlobals)
-Features["mutable-globals"] = true;
-  if (HasMultivalue)
-Features["multivalue"] = true;
-  if (HasTailCall)
-Features["tail-call"] = true;
-  if (HasReferenceTypes)
-Features["reference-types"] = true;
 
   return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 2b8ad6b - [WebAssembly] Don't depend on the flags set by handleTargetFeatures in initFeatureMap.

2020-08-12 Thread Craig Topper via cfe-commits

Author: Craig Topper
Date: 2020-08-12T11:43:46-07:00
New Revision: 2b8ad6b6040833f4f8702721ebaa7749e5c23e60

URL: 
https://github.com/llvm/llvm-project/commit/2b8ad6b6040833f4f8702721ebaa7749e5c23e60
DIFF: 
https://github.com/llvm/llvm-project/commit/2b8ad6b6040833f4f8702721ebaa7749e5c23e60.diff

LOG: [WebAssembly] Don't depend on the flags set by handleTargetFeatures in 
initFeatureMap.

Properly set "simd128" in the feature map when "unimplemented-simd128"
is requested.

initFeatureMap is used to create the feature vector used by
handleTargetFeatures. There are later calls to initFeatureMap in
CodeGen that were using these flags to recreate the map. But the
original feature vector should be passed to those calls. So that
should be enough to rebuild the map.

The only issue seemed to be that simd128 was not enabled in the
map by the first call to initFeatureMap. Using the SIMDLevel set
by handleTargetFeatures in the later calls allowed simd128 to be
set in the later versions of the map.

To fix this I've added an override of setFeatureEnabled that
will update the map the first time with the correct simd dependency.

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

Added: 


Modified: 
clang/lib/Basic/Targets/WebAssembly.cpp
clang/lib/Basic/Targets/WebAssembly.h

Removed: 




diff  --git a/clang/lib/Basic/Targets/WebAssembly.cpp 
b/clang/lib/Basic/Targets/WebAssembly.cpp
index 6746768090f5..dcb3d8fd7790 100644
--- a/clang/lib/Basic/Targets/WebAssembly.cpp
+++ b/clang/lib/Basic/Targets/WebAssembly.cpp
@@ -96,19 +96,43 @@ void WebAssemblyTargetInfo::getTargetDefines(const 
LangOptions ,
 }
 
 void WebAssemblyTargetInfo::setSIMDLevel(llvm::StringMap ,
- SIMDEnum Level) {
+ SIMDEnum Level, bool Enabled) {
+  if (Enabled) {
+switch (Level) {
+case UnimplementedSIMD128:
+  Features["unimplemented-simd128"] = true;
+  LLVM_FALLTHROUGH;
+case SIMD128:
+  Features["simd128"] = true;
+  LLVM_FALLTHROUGH;
+case NoSIMD:
+  break;
+}
+return;
+  }
+
   switch (Level) {
-  case UnimplementedSIMD128:
-Features["unimplemented-simd128"] = true;
-LLVM_FALLTHROUGH;
+  case NoSIMD:
   case SIMD128:
-Features["simd128"] = true;
+Features["simd128"] = false;
 LLVM_FALLTHROUGH;
-  case NoSIMD:
+  case UnimplementedSIMD128:
+Features["unimplemented-simd128"] = false;
 break;
   }
 }
 
+void WebAssemblyTargetInfo::setFeatureEnabled(llvm::StringMap ,
+  StringRef Name,
+  bool Enabled) const {
+  if (Name == "simd128")
+setSIMDLevel(Features, SIMD128, Enabled);
+  else if (Name == "unimplemented-simd128")
+setSIMDLevel(Features, UnimplementedSIMD128, Enabled);
+  else
+Features[Name] = Enabled;
+}
+
 bool WebAssemblyTargetInfo::initFeatureMap(
 llvm::StringMap , DiagnosticsEngine , StringRef CPU,
 const std::vector ) const {
@@ -119,30 +143,8 @@ bool WebAssemblyTargetInfo::initFeatureMap(
 Features["atomics"] = true;
 Features["mutable-globals"] = true;
 Features["tail-call"] = true;
-setSIMDLevel(Features, SIMD128);
+setSIMDLevel(Features, SIMD128, true);
   }
-  // Other targets do not consider user-configured features here, but while we
-  // are actively developing new features it is useful to let user-configured
-  // features control availability of builtins
-  setSIMDLevel(Features, SIMDLevel);
-  if (HasNontrappingFPToInt)
-Features["nontrapping-fptoint"] = true;
-  if (HasSignExt)
-Features["sign-ext"] = true;
-  if (HasExceptionHandling)
-Features["exception-handling"] = true;
-  if (HasBulkMemory)
-Features["bulk-memory"] = true;
-  if (HasAtomics)
-Features["atomics"] = true;
-  if (HasMutableGlobals)
-Features["mutable-globals"] = true;
-  if (HasMultivalue)
-Features["multivalue"] = true;
-  if (HasTailCall)
-Features["tail-call"] = true;
-  if (HasReferenceTypes)
-Features["reference-types"] = true;
 
   return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
 }

diff  --git a/clang/lib/Basic/Targets/WebAssembly.h 
b/clang/lib/Basic/Targets/WebAssembly.h
index 77a2fe9ae117..0068ccb5d71f 100644
--- a/clang/lib/Basic/Targets/WebAssembly.h
+++ b/clang/lib/Basic/Targets/WebAssembly.h
@@ -69,7 +69,8 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyTargetInfo : public 
TargetInfo {
 MacroBuilder ) const override;
 
 private:
-  static void setSIMDLevel(llvm::StringMap , SIMDEnum Level);
+  static void setSIMDLevel(llvm::StringMap , SIMDEnum Level,
+   bool Enabled);
 
   bool
   initFeatureMap(llvm::StringMap , DiagnosticsEngine ,
@@ -77,6 +78,9 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyTargetInfo : public 
TargetInfo {
  const 

[PATCH] D85710: Code generation support for lvalue bit-field conditionals.

2020-08-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D85710#2212174 , @rjmccall wrote:

> I always imagined that we'd implement this by just inverting the emission: 
> emit the RHS of the assignment and then pass a callback down to a function 
> that descended into conditional operators and called the callback when it hit 
> a leaf.  Having an LValue case that's an arbitrarily-nested conditional seems 
> unfortunate.

I thought I'd convinced myself that that approach couldn't work, but I'm no 
longer so convinced. (I had thought there were cases where we would need to run 
arbitrary code between creating the lvalue and disposing of it, and that 
arbitrary code could be something we were only able to emit once for whatever 
reason.) The fact that assignment and compound-assignment operators all 
evaluate their RHS before their LHS (in the language modes where they're 
sequenced) helps a lot; expressions such as `((cond ? a.x : a.y) += f()) *= 
g()` still seem like they could compositionally work with this model, and even 
things like `(cond1 ? b : (cond2 ? a.x : a.y) += f()) += g()`. Hm, but what 
about `int n = (cond ? a.x : a.y) += 1;`? I guess we'd need to carry the intent 
to perform a load of the conditional lvalue into the callback too.

OK, I think that all works, but I'm a lot less confident of the absence of 
surprising corners that would defeat that strategy than with the approach of 
this patch -- the correctness of doing this with a single branch seems to hinge 
crucially on subtle program structure restrictions, in particular the 
presumptive fact that we never need control flow to converge between creating 
the lvalue and consuming it. On the other hand, it would also let us emit 
better IR for things like `(a += 1) *= 2` (removing the intermediate store to 
`a`) if we wanted, and maybe it's a good strategy to pursue for that reason?

I can give that a go next time I find some cycles to work on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85710

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


[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

2020-08-12 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:3330
+// appendices to the Procedure Call Standard for the Arm Architecture, see:
+// 
https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#appendix-c-mangling
+void CXXNameMangler::mangleAArch64FixedSveVectorType(const VectorType *T) {

Mangling them the same way is going to cause practical issues; they're 
different types from a C++ perspective, so they need distinct manglings.  For 
example, you'll crash the compiler if you refer to both foo and 
foo.



Comment at: clang/lib/CodeGen/CGCall.cpp:1238
+Src = EnterStructPointerForCoercedAccess(Src, SrcSTy,
+ DstSize.getKnownMinSize(), CGF);
 SrcTy = Src.getElementType();

getFixedSize()?



Comment at: clang/lib/CodeGen/CGCall.cpp:1254
+  if ((!SrcSize.isScalable() && !DstSize.isScalable()) &&
+  SrcSize.getKnownMinSize() >= DstSize.getKnownMinSize()) {
 // Generally SrcSize is never greater than DstSize, since this means we are

getFixedSize():?  (etc.; please go through the whole patch.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85743

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


[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

2020-08-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:115
+// either due to crashes or false positives.
+const char *getClangTidyBlacklist() {
+  static const std::string FalsePositives =

kadircet wrote:
> aaron.ballman wrote:
> > njames93 wrote:
> > > Return by StringRef?
> > How about `getDisabledClangTidyChecks()` (or literally any other name than 
> > blacklist)?
> thanks for bringing this to my attention, i will try to be more conscious 
> next time.
> 
> I would prefer allow/deny as `disabled` might also be offensive in some 
> contexts. Do you know if we already have some settlements around this one in 
> the wider community?
> thanks for bringing this to my attention, i will try to be more conscious 
> next time.

No worries!

> I would prefer allow/deny as disabled might also be offensive in some 
> contexts. Do you know if we already have some settlements around this one in 
> the wider community?

I don't believe there's any consensus around avoiding use of "disabled" (we use 
the term in a lot of places, especially when paired with "enabled"), but I'd 
also be fine with allow/deny terminology instead.

As a minor drive-by comment, the function should also be marked `static` and 
placed outside of the anonymous namespace (per our usual coding style).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83224

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


[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

2020-08-12 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added inline comments.



Comment at: clang/include/clang/Basic/TargetCXXABI.h:39
 
+  static const auto () {
+static llvm::StringMap ABIMap = {

The option UI should use lowercase values by default, or else just do 
case-insensitive matching.




Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:274
+
+  if (DriverArgs.getLastArgValue(options::OPT_fcxx_abi_EQ) == "Fuchsia")
+CC1Args.push_back("-fexperimental-relative-c++-abi-vtables");

This should match "fuchsia", either only lowercase or case-insensitive.  IMHO 
it seems wise to handle this in some way such that individual tests like this 
are not free-form string comparisons that could have typos, but test the 
TargetCXXABI enum value after decoding from string.

This should be a separate change.  We'll need to do staged roll-out, first of a 
compiler where `-fc++abi=compat` (or itanium whatever it's called) is available 
(complete with multilibs et al), and then later of a compiler where 
`-fc++abi=fuchsia` (and the default for *-fuchsia targets) has changed meaning.




Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:275
+  if (DriverArgs.getLastArgValue(options::OPT_fcxx_abi_EQ) == "Fuchsia")
+CC1Args.push_back("-fexperimental-relative-c++-abi-vtables");
 }

It's surprising to me that this is the way to do this.  Isn't there code in the 
actual front end that tests the TargetCXXABI value?  That seems like the place 
where it makes sense to have Fuchsia imply specific settings, rather than in 
the driver.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802

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


[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

2020-08-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:115
+// either due to crashes or false positives.
+const char *getClangTidyBlacklist() {
+  static const std::string FalsePositives =

aaron.ballman wrote:
> njames93 wrote:
> > Return by StringRef?
> How about `getDisabledClangTidyChecks()` (or literally any other name than 
> blacklist)?
thanks for bringing this to my attention, i will try to be more conscious next 
time.

I would prefer allow/deny as `disabled` might also be offensive in some 
contexts. Do you know if we already have some settlements around this one in 
the wider community?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83224

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


[PATCH] D85826: [clang] Make signature help work with dependent args

2020-08-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 285141.
kadircet added a comment.

- Change logic to find all signatures without any dependent args and post 
filter non-viable overloads using the argument counts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85826

Files:
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/call.cpp

Index: clang/test/CodeCompletion/call.cpp
===
--- clang/test/CodeCompletion/call.cpp
+++ clang/test/CodeCompletion/call.cpp
@@ -32,3 +32,23 @@
   // CHECK-CC3-NEXT: OVERLOAD: [#void#]f(<#int i#>, int j, int k)
   // CHECK-CC3-NEXT: OVERLOAD: [#void#]f(<#float x#>, float y)
 }
+
+void f(int, int, int, int);
+template 
+void foo(T t) {
+  f(t, t, t);
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:39:5 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+  // CHECK-CC4: f()
+  // CHECK-CC4-NEXT: f(<#X#>)
+  // CHECK-CC4-NEXT: f(<#int i#>, int j, int k)
+  // CHECK-CC4-NEXT: f(<#float x#>, float y)
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:39:8 %s -o - | FileCheck -check-prefix=CHECK-CC5 %s
+  // CHECK-CC5-NOT: f()
+  // CHECK-CC5: f(int i, <#int j#>, int k)
+  // CHECK-CC5-NEXT: f(float x, <#float y#>)
+  f(5, t, t);
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:49:11 %s -o - | FileCheck -check-prefix=CHECK-CC6 %s
+  // CHECK-CC6-NOT: f(float x, float y)
+  // CHECK-CC6: f(int, int, <#int#>, int)
+  // CHECK-CC6-NEXT: f(int i, int j, <#int k#>)
+}
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -5500,7 +5500,7 @@
 
 static void mergeCandidatesWithResults(
 Sema , SmallVectorImpl ,
-OverloadCandidateSet , SourceLocation Loc) {
+OverloadCandidateSet , SourceLocation Loc, size_t ArgSize) {
   // Sort the overload candidate set by placing the best overloads first.
   llvm::stable_sort(CandidateSet, [&](const OverloadCandidate ,
   const OverloadCandidate ) {
@@ -5510,8 +5510,19 @@
 
   // Add the remaining viable overload candidates as code-completion results.
   for (OverloadCandidate  : CandidateSet) {
-if (Candidate.Function && Candidate.Function->isDeleted())
-  continue;
+if (Candidate.Function) {
+  if (Candidate.Function->isDeleted())
+continue;
+  if (!Candidate.Function->isVariadic() &&
+  Candidate.Function->getNumParams() <= ArgSize &&
+  // Having zero args is annoying, normally we don't surface a function
+  // with 2 params, if you already have 2 params, because you are
+  // inserting the 3rd now. But with zero, it helps the user to figure
+  // out there are no overloads that take any arguments. Hence we are
+  // keeping the overload.
+  ArgSize > 0)
+continue;
+}
 if (Candidate.Viable)
   Results.push_back(ResultCandidate(Candidate.Function));
   }
@@ -5562,22 +5573,25 @@
 
   // FIXME: Provide support for variadic template functions.
   // Ignore type-dependent call expressions entirely.
-  if (!Fn || Fn->isTypeDependent() || anyNullArguments(Args) ||
-  Expr::hasAnyTypeDependentArguments(Args)) {
+  if (!Fn || Fn->isTypeDependent() || anyNullArguments(Args))
 return QualType();
-  }
+  // In presence of dependent args we surface all possible signatures using the
+  // non-dependent args in the prefix. Afterwards we do a post filtering to make
+  // sure provided candidates satisfy parameter count restrictions.
+  auto ArgsWithoutDependentTypes =
+  Args.take_while([](Expr *Arg) { return !Arg->isTypeDependent(); });
 
+  SmallVector Results;
+
+  Expr *NakedFn = Fn->IgnoreParenCasts();
   // Build an overload candidate set based on the functions we find.
   SourceLocation Loc = Fn->getExprLoc();
   OverloadCandidateSet CandidateSet(Loc, OverloadCandidateSet::CSK_Normal);
 
-  SmallVector Results;
-
-  Expr *NakedFn = Fn->IgnoreParenCasts();
-  if (auto ULE = dyn_cast(NakedFn))
-AddOverloadedCallCandidates(ULE, Args, CandidateSet,
+  if (auto ULE = dyn_cast(NakedFn)) {
+AddOverloadedCallCandidates(ULE, ArgsWithoutDependentTypes, CandidateSet,
 /*PartialOverloading=*/true);
-  else if (auto UME = dyn_cast(NakedFn)) {
+  } else if (auto UME = dyn_cast(NakedFn)) {
 TemplateArgumentListInfo TemplateArgsBuffer, *TemplateArgs = nullptr;
 if (UME->hasExplicitTemplateArgs()) {
   UME->copyTemplateArgumentsInto(TemplateArgsBuffer);
@@ -5587,7 +5601,8 @@
 // Add the base as first argument (use a nullptr if the base is implicit).
 SmallVector ArgExprs(
 1, UME->isImplicitAccess() ? nullptr : UME->getBase());
-ArgExprs.append(Args.begin(), Args.end());
+   

[PATCH] D85806: [WebAssembly] Don't depend on the flags set by handleTargetFeatures in initFeatureMap.

2020-08-12 Thread Thomas Lively via Phabricator via cfe-commits
tlively accepted this revision.
tlively added a comment.
This revision is now accepted and ready to land.

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85806

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


[clang] a7a06de - Recommit "[InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X transforms" and its follow up patches

2020-08-12 Thread Craig Topper via cfe-commits

Author: Craig Topper
Date: 2020-08-12T10:45:27-07:00
New Revision: a7a06ded8b0635268b5db218b3aca0b5b2bfb04a

URL: 
https://github.com/llvm/llvm-project/commit/a7a06ded8b0635268b5db218b3aca0b5b2bfb04a
DIFF: 
https://github.com/llvm/llvm-project/commit/a7a06ded8b0635268b5db218b3aca0b5b2bfb04a.diff

LOG: Recommit "[InstSimplify] Remove select ?, undef, X -> X and select ?, X, 
undef -> X transforms" and its follow up patches

This recommits the following patches now that D85684 has landed

1cf6f210a2e [IR] Disable select ? C : undef -> C fold in 
ConstantFoldSelectInstruction unless we know C isn't poison.
469da663f2d [InstSimplify] Re-enable select ?, undef, X -> X transform when X 
is provably not poison
122b0640fc9 [InstSimplify] Don't fold vectors of partial undef in 
SimplifySelectInst if the non-undef element value might produce poison
ac0af12ed2f [InstSimplify] Add test cases for opportunities to fold select ?, 
X, undef -> X when we can prove X isn't poison
9b1e95329af [InstSimplify] Remove select ?, undef, X -> X and select ?, X, 
undef -> X transforms

Added: 


Modified: 
clang/test/CodeGen/arm-mve-intrinsics/dup.c
llvm/lib/Analysis/InstructionSimplify.cpp
llvm/lib/IR/ConstantFold.cpp
llvm/test/Transforms/InferAddressSpaces/AMDGPU/select.ll
llvm/test/Transforms/InstCombine/select.ll
llvm/test/Transforms/InstSimplify/select.ll

Removed: 




diff  --git a/clang/test/CodeGen/arm-mve-intrinsics/dup.c 
b/clang/test/CodeGen/arm-mve-intrinsics/dup.c
index 283c08257005..b443917cb258 100644
--- a/clang/test/CodeGen/arm-mve-intrinsics/dup.c
+++ b/clang/test/CodeGen/arm-mve-intrinsics/dup.c
@@ -242,7 +242,8 @@ uint32x4_t test_vdupq_m_n_u32(uint32x4_t inactive, uint32_t 
a, mve_pred16_t p)
 // CHECK-NEXT:[[TMP1:%.*]] = call <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32 
[[TMP0]])
 // CHECK-NEXT:[[DOTSPLATINSERT:%.*]] = insertelement <8 x half> undef, 
half [[A:%.*]], i32 0
 // CHECK-NEXT:[[DOTSPLAT:%.*]] = shufflevector <8 x half> 
[[DOTSPLATINSERT]], <8 x half> undef, <8 x i32> zeroinitializer
-// CHECK-NEXT:ret <8 x half> [[DOTSPLAT]]
+// CHECK-NEXT:[[TMP2:%.*]] = select <8 x i1> [[TMP1]], <8 x half> 
[[DOTSPLAT]], <8 x half> undef
+// CHECK-NEXT:ret <8 x half> [[TMP2]]
 //
 float16x8_t test_vdupq_x_n_f16(float16_t a, mve_pred16_t p)
 {
@@ -255,7 +256,8 @@ float16x8_t test_vdupq_x_n_f16(float16_t a, mve_pred16_t p)
 // CHECK-NEXT:[[TMP1:%.*]] = call <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32 
[[TMP0]])
 // CHECK-NEXT:[[DOTSPLATINSERT:%.*]] = insertelement <4 x float> undef, 
float [[A:%.*]], i32 0
 // CHECK-NEXT:[[DOTSPLAT:%.*]] = shufflevector <4 x float> 
[[DOTSPLATINSERT]], <4 x float> undef, <4 x i32> zeroinitializer
-// CHECK-NEXT:ret <4 x float> [[DOTSPLAT]]
+// CHECK-NEXT:[[TMP2:%.*]] = select <4 x i1> [[TMP1]], <4 x float> 
[[DOTSPLAT]], <4 x float> undef
+// CHECK-NEXT:ret <4 x float> [[TMP2]]
 //
 float32x4_t test_vdupq_x_n_f32(float32_t a, mve_pred16_t p)
 {
@@ -268,7 +270,8 @@ float32x4_t test_vdupq_x_n_f32(float32_t a, mve_pred16_t p)
 // CHECK-NEXT:[[TMP1:%.*]] = call <16 x i1> 
@llvm.arm.mve.pred.i2v.v16i1(i32 [[TMP0]])
 // CHECK-NEXT:[[DOTSPLATINSERT:%.*]] = insertelement <16 x i8> undef, i8 
[[A:%.*]], i32 0
 // CHECK-NEXT:[[DOTSPLAT:%.*]] = shufflevector <16 x i8> 
[[DOTSPLATINSERT]], <16 x i8> undef, <16 x i32> zeroinitializer
-// CHECK-NEXT:ret <16 x i8> [[DOTSPLAT]]
+// CHECK-NEXT:[[TMP2:%.*]] = select <16 x i1> [[TMP1]], <16 x i8> 
[[DOTSPLAT]], <16 x i8> undef
+// CHECK-NEXT:ret <16 x i8> [[TMP2]]
 //
 int8x16_t test_vdupq_x_n_s8(int8_t a, mve_pred16_t p)
 {
@@ -281,7 +284,8 @@ int8x16_t test_vdupq_x_n_s8(int8_t a, mve_pred16_t p)
 // CHECK-NEXT:[[TMP1:%.*]] = call <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32 
[[TMP0]])
 // CHECK-NEXT:[[DOTSPLATINSERT:%.*]] = insertelement <8 x i16> undef, i16 
[[A:%.*]], i32 0
 // CHECK-NEXT:[[DOTSPLAT:%.*]] = shufflevector <8 x i16> 
[[DOTSPLATINSERT]], <8 x i16> undef, <8 x i32> zeroinitializer
-// CHECK-NEXT:ret <8 x i16> [[DOTSPLAT]]
+// CHECK-NEXT:[[TMP2:%.*]] = select <8 x i1> [[TMP1]], <8 x i16> 
[[DOTSPLAT]], <8 x i16> undef
+// CHECK-NEXT:ret <8 x i16> [[TMP2]]
 //
 int16x8_t test_vdupq_x_n_s16(int16_t a, mve_pred16_t p)
 {
@@ -294,7 +298,8 @@ int16x8_t test_vdupq_x_n_s16(int16_t a, mve_pred16_t p)
 // CHECK-NEXT:[[TMP1:%.*]] = call <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32 
[[TMP0]])
 // CHECK-NEXT:[[DOTSPLATINSERT:%.*]] = insertelement <4 x i32> undef, i32 
[[A:%.*]], i32 0
 // CHECK-NEXT:[[DOTSPLAT:%.*]] = shufflevector <4 x i32> 
[[DOTSPLATINSERT]], <4 x i32> undef, <4 x i32> zeroinitializer
-// CHECK-NEXT:ret <4 x i32> [[DOTSPLAT]]
+// CHECK-NEXT:[[TMP2:%.*]] = select <4 x i1> [[TMP1]], <4 x i32> 
[[DOTSPLAT]], <4 x i32> undef
+// CHECK-NEXT:ret <4 x i32> [[TMP2]]
 //
 int32x4_t test_vdupq_x_n_s32(int32_t a, 

[PATCH] D85736: [Sema][AArch64] Support arm_sve_vector_bits attribute

2020-08-12 Thread Richard Sandiford via Phabricator via cfe-commits
rsandifo-arm added inline comments.



Comment at: clang/include/clang/AST/Type.h:1897
+  /// 'arm_sve_vector_bits' type attribute as VectorType.
+  QualType getFixedLengthSveEltType(const ASTContext ) const;
+

It feels to me like the information is more general than that, since the type 
of the element is meaningful independently of the new attribute.  How about 
just getSveEltType instead?



Comment at: clang/lib/AST/ASTContext.cpp:1941
+// Adjust the alignment for fixed-length SVE predicates.
+if (VT->getVectorKind() == VectorType::SveFixedLengthPredicateVector)
+  Align = 16;

The alignment of the SVE data vectors should be 128 too (see the alignof stuff 
in the ACLE doc).  FWIW, there were two reasons for defining it like that:
  - the fixed-length types map to the same ABI machine type as the 
variable-length types
  - the length isn't required to be a power of 2: an implementation that 
supports fixed-length 384-bit vectors could define `__ARM_FEATURE_SVE_BITS` to 
384.






Comment at: clang/lib/AST/TextNodeDumper.cpp:1413
+  case VectorType::SveFixedLengthPredicateVector:
+OS << " fixed-length sve";
   }

Is it worth distinguishing these, similarly to altivec?



Comment at: clang/lib/AST/Type.cpp:2338
+  case BuiltinType::SveUint32:
+return Ctx.UnsignedIntTy;
+  case BuiltinType::SveInt64:

This is just a note, not sure what can be done about it, but:

The element types are defined in terms of the stdint.h types.  One snag is that 
ILP32 newlib defines `int32_t` to be UnsignedLongTy instead of UnsignedIntTy, 
and defines `uint64_t` to UnsignedLongLongTy.

In GCC we got away with this because GCC already has hard-coded knowledge about 
the target C library's choices.  I don't think that information is available 
here though.

Like I say, there's nothing that necessarily needs to be “fixed”/dealt-with 
now, just thought it was worth mentioning.



Comment at: clang/lib/Sema/SemaExpr.cpp:8934
+  ASTContext ) {
+  auto IsValidCast = [](QualType LHSType, QualType RHSType,
+ASTContext ) {

I guess this is personal preference, but it seems more natural to use `[&]` and 
not pass the context.  Maybe different names from LHSType and RHSType would be 
better for the nested function, since it's called with both orders.



Comment at: clang/lib/Sema/SemaExpr.cpp:8942
+VT->getVectorKind() == VectorType::SveFixedLengthPredicateVector &&
+isVector(RHSType, LHSType->getFixedLengthSveEltType(Context)))
+  return true;

Is the isVector just being defensive?  I'd have expected it to be redundant, 
since we shouldn't create SveFixedLengthPredicateVectors with invalid element 
types.



Comment at: clang/lib/Sema/SemaExpr.cpp:8945
+
+if (VT->getVectorKind() == VectorType::SveFixedLengthDataVector &&
+isVector(RHSType, LHSType->getFixedLengthSveEltType(Context)))

It looks like this could still trigger for SveBools.  Maybe it would be better 
to have:

  if (BT->getKind() == BuiltinType::SveBool) {
…
  } else {
…
  }

instead.  Perhaps it'd also be worth having an assert to show that we've 
already checked that any builtin type is an SVE type.



Comment at: clang/lib/Sema/SemaExpr.cpp:9968
+  if ((LHSVecType &&
+   ((LHSVecType->getVectorKind() == VectorType::SveFixedLengthDataVector) 
||
+(LHSVecType->getVectorKind() ==

Seems like it might be useful to have a helper function for testing this pair 
of vector kinds.


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

https://reviews.llvm.org/D85736

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


[PATCH] D85807: [Target] Cache the command line derived feature map in TargetOptions.

2020-08-12 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers accepted this revision.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.

LGTM; comparing the report at the bottom of 
https://reviews.llvm.org/D85257#2197114, repasted (before this patch):

  +3.59%  clang-12   [.] llvm::X86::getImpliedFeatures
  -3.36%  clang-12   [.] 
llvm::StringMapImpl::LookupBucketFor
 - 3.32% llvm::StringMapImpl::LookupBucketFor
- 1.24% llvm::StringMap::try_emplace<>
   + 1.24% clang::targets::X86TargetInfo::setFeatureEnabled

  Elapsed (wall clock) time (h:mm:ss or m:ss): 0:53.39

(After this patch):
A profile of a full x86_64 build with this applied, none of the above appear in 
the profile.  `clang::targets::X86TargetInfo::setFeatureEnabled`, does but near 
at the bottom with < 0.00%.  So this shaves another ~5% off build times!

  Elapsed (wall clock) time (h:mm:ss or m:ss): 0:50.98


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85807

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


[PATCH] D85817: [analyzer] Fix crash with pointer to members values

2020-08-12 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment.

In D85817#2213435 , @NoQ wrote:

> That's a fix for https://bugs.llvm.org/show_bug.cgi?id=46264.
>
> Your code looks great but i don't understand at a glance what the crash was 
> caused by and how does your code fix it, can you explain? Like, the original 
> test doesn't have any `void *`s and it doesn't have any indirect fields. Also 
> the crash happens during visitor phase but the fixes are entirely during 
> modeling phase.

Sure! The problem was in the **deleted code**, we modeled `FieldDecl` and 
`IndirectFieldDecl` in such matter that `::d` ended up as `void *` and it was 
totally fine for path exploration, but the moment we try to //explain// such 
value is where we hit the assertion (I tried to explain that in the summary). I 
could've fixed the symptom and patch that, but it didn't seem right.  It is not 
a pointer to void and it never was.  Then I found out about the fact that we 
actually have a mechanism to deal with pointer to members.  So I decided to 
remove the code with 2 FIXMEs (by doing one of the FIXMEs).  Removing only 
`FieldDecl` from the condition would've fixed the original problem, but 
wouldn't have solved a very similar example with indirect fields (maybe I 
should expand the test to include that one as well).  That's how I came to 
fixing the behavior for indirect fields as well.




Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:2533-2534
   if (isa(D) || isa(D)) {
-// FIXME: Compute lvalue of field pointers-to-member.
-// Right now we just use a non-null void pointer, so that it gives proper
-// results in boolean contexts.
-// FIXME: Maybe delegate this to the surrounding operator&.
-// Note how this expression is lvalue, however pointer-to-member is NonLoc.
-SVal V = svalBuilder.conjureSymbolVal(Ex, LCtx, getContext().VoidPtrTy,
-  currBldrCtx->blockCount());
-state = state->assume(V.castAs(), true);
-Bldr.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V), nullptr,
-  ProgramPoint::PostLValueKind);
+// Delegate all work related to pointer to members to the surrounding
+// operator&.
 return;

NoQ wrote:
> Ok so you're saying that there's *always* going to be a surrounding operator 
> `&`? That kind of makes sense but if you add more explanation/proof of how 
> you figured this out that'd be great.
I don't know actually that there's *always* a surrounding `&`, I mean it makes 
sense and I guess it is the only case we care about.



Comment at: clang/test/Analysis/PR46264.cpp:5
+
+namespace a {
+class b {

NoQ wrote:
> I'm pretty sure the namespace is not actually important for the test. 
> `creduce` is great but sometimes it misses stuff. Generally, i'm a big fan of 
> manually cleaning up `creduce`d test to make them look more like real code. 
> At least turn things like `b h;` and `e *i;` into something like `A a;` and 
> `B *b;`. Also replace `class` with `struct` because we never care about this 
> entire public/private business and `struct` provides a nice default.
I didn't change the code because it is the actual snippet from the bug report, 
but OK



Comment at: clang/test/Analysis/PR46264.cpp:9
+  typedef int b::*c;
+  operator c() { return ::d; }
+  int d;

NoQ wrote:
> It's worth it to comment where exactly is this conversion operator used in 
> the text (i.e., within the if-condition). People do in fact sometimes do this 
> kind of thing in real life: provide a conversion to pointer-to-member 
> *instead of* conversion to `bool` because it causes fewer potential further 
> accidental implicit conversions to be possible.
OK, will do


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85817

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


[PATCH] D85193: [clang] Check `expr` inside `InitListChecker::UpdateStructuredListElement()`

2020-08-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.

In D85193#2213384 , @ArcsinX wrote:

> In D85193#2212976 , @aaron.ballman 
> wrote:
>
>> I have a suggestion for the added comment,
>
> Fixed comment according to your suggestion.
>
>> but I also forgot to ask, do you need someone to commit on your behalf?
>
> I already have commit access.
>
> Thanks for review!

Excellent, this LGTM, thank you for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85193

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


[PATCH] D85842: Fix `NestedNameSpecifierLoc::getLocalSourceRange()`

2020-08-12 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas added a comment.

Unit tests will be provided once we decide that this is the way to go ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85842

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


[PATCH] D85736: [Sema][AArch64] Support arm_sve_vector_bits attribute

2020-08-12 Thread Cullen Rhodes via Phabricator via cfe-commits
c-rhodes updated this revision to Diff 285124.
c-rhodes added a comment.

Added missing implicit conversions for C++. I considered handling this with the
existing implicit vector conversion although one side of the conversion will be
an SVE builtin, so instead I've added a new conversion specifically for SVE. I
suspect the existing one could support this but I wasn't sure if that was a good
idea (?). In C++ implicit conversions between VLA/VLS have a rank just below
derived-to-base conversion.


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

https://reviews.llvm.org/D85736

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Overload.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/attr-arm-sve-vector-bits.c
  clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp

Index: clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
+// expected-no-diagnostics
+
+#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+
+typedef __SVInt8_t svint8_t;
+typedef svint8_t fixed_int8_t __attribute__((arm_sve_vector_bits(N)));
+
+template struct S { T var; };
+
+S s;
+
+svint8_t to_svint8_t(fixed_int8_t x) { return x; }
+fixed_int8_t from_svint8_t(svint8_t x) { return x; }
Index: clang/test/Sema/attr-arm-sve-vector-bits.c
===
--- clang/test/Sema/attr-arm-sve-vector-bits.c
+++ clang/test/Sema/attr-arm-sve-vector-bits.c
@@ -102,8 +102,11 @@
   svint8_t ss8;
 
   void *sel __attribute__((unused));
-  sel = c ? ss8 : fs8; // expected-error {{incompatible operand types ('svint8_t' (aka '__SVInt8_t') and 'fixed_int8_t' (aka '__SVInt8_t'))}}
-  sel = c ? fs8 : ss8; // expected-error {{incompatible operand types ('fixed_int8_t' (aka '__SVInt8_t') and 'svint8_t' (aka '__SVInt8_t'))}}
+  sel = c ? ss8 : fs8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
+  sel = c ? fs8 : ss8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
+
+  sel = fs8 + ss8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
+  sel = ss8 + fs8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
 }
 
 // --//
@@ -192,14 +195,18 @@
 TEST_CAST(bool)
 
 // Test the implicit conversion only applies to valid types
-fixed_int8_t to_fixed_int8_t__from_svuint8_t(svuint8_t x) { return x; } // expected-error {{returning 'svuint8_t' (aka '__SVUint8_t') from a function with incompatible result type 'fixed_int8_t' (aka '__SVInt8_t')}}
-fixed_bool_t to_fixed_bool_t__from_svint32_t(svint32_t x) { return x; } // expected-error {{returning 'svint32_t' (aka '__SVInt32_t') from a function with incompatible result type 'fixed_bool_t' (aka '__SVBool_t')}}
+fixed_int8_t to_fixed_int8_t__from_svuint8_t(svuint8_t x) { return x; } // expected-error-re {{returning 'svuint8_t' (aka '__SVUint8_t') from a function with incompatible result type 'fixed_int8_t' (vector of {{[0-9]+}} 'signed char' values)}}
+fixed_bool_t to_fixed_bool_t__from_svint32_t(svint32_t x) { return x; } // expected-error-re {{returning 'svint32_t' (aka '__SVInt32_t') from a function with incompatible result type 'fixed_bool_t' (vector of {{[0-9]+}} 'unsigned char' values)}}
+
+// Test conversion between predicate and uint8 is invalid, both have the same
+// memory representation.
+fixed_bool_t to_fixed_bool_t__from_svuint8_t(svuint8_t x) { return x; } // expected-error-re {{returning 'svuint8_t' (aka '__SVUint8_t') from a function with incompatible result type 'fixed_bool_t' (vector of {{[0-9]+}} 'unsigned char' values)}}
 
 // Test the implicit conversion only applies to fixed-length types
 typedef signed int vSInt32 __attribute__((__vector_size__(16)));
-svint32_t to_svint32_t_from_gnut(vSInt32 x) { return x; } // expected-error {{returning 'vSInt32' (vector of 4 'int' values) from a function with incompatible result type 'svint32_t' (aka '__SVInt32_t')}}
+svint32_t to_svint32_t_from_gnut(vSInt32 x) { return x; } // expected-error-re {{returning 'vSInt32' (vector of {{[0-9]+}} 'int' values) from a function with incompatible result type 'svint32_t' (aka '__SVInt32_t')}}
 
-vSInt32 

[PATCH] D85118: [clang][AArch64] Correct return type of Neon vqmovun intrinsics

2020-08-12 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki added a comment.

For intrinsics that are plain functions (not macros) you could just check the 
signatures using assignments to function pointers, e.g.:

  #include 
  uint16_t (*fp1)(int32_t) = _s32;



  $ clang -target=aarch64-arm-none-eabi -march=armv8-a+simd -c neon.c 
-fsyntax-only -Werror=incompatible-function-pointer-types
  neon.c:2:12: error: incompatible function pointer types initializing 
'uint16_t (*)(int32_t)' (aka 'unsigned short (*)(int)') with an expression of 
type 'int16_t (*)(int32_t)' (aka 'short (*)(int)') 
[-Werror,-Wincompatible-function-pointer-types]
  uint16_t (*fp1)(int32_t) = _s32;
 ^   ~
  1 error generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85118

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


[PATCH] D85826: [clang] Make signature help work with dependent args

2020-08-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5573
+  // performing any semantic checks on availability. That's to improve user
+  // experience, it is better to see all overloads rather than none.
+  if (Expr::hasAnyTypeDependentArguments(Args)) {

hokein wrote:
> I'm a bit nervous to show all overloads regardlessly, I think we could do 
> some smarter things like
> 
> 1. if there are some prefix non-type-dependent arguments, we could use these 
> type information to filter out some unrelated candidates;
> 2. different overloads may have different number of parameters, this is an 
> important signal, e.g. `foo(t, ^t)` should not give `void foo(int)` result;
> 
> 2 seems quite critical and trivial to implement, maybe we can do it in this 
> patch.
> if there are some prefix non-type-dependent arguments, we could use these 
> type information to filter out some unrelated candidates;

well actually now that I think about it, this is actually not that hard. 
possibly even easier than current version. changing the logic surface every 
candidate using the prefix, and do a post-filtering instead of trying to mock 
all the results.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85826

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


[PATCH] D85842: Fix `NestedNameSpecifierLoc::getLocalSourceRange()`

2020-08-12 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 285129.
eduucaldas added a comment.

Update comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85842

Files:
  clang/lib/AST/NestedNameSpecifier.cpp


Index: clang/lib/AST/NestedNameSpecifier.cpp
===
--- clang/lib/AST/NestedNameSpecifier.cpp
+++ clang/lib/AST/NestedNameSpecifier.cpp
@@ -439,6 +439,20 @@
 // Note: the 'template' keyword is part of the TypeLoc.
 void *TypeData = LoadPointer(Data, Offset);
 TypeLoc TL(Qualifier->getAsType(), TypeData);
+// For `T::template ST::x`
+// `NestedNameSpecifierLoc::getLocalSourceRange()` should return `template
+// ST::`.
+// For any TypeLoc we can just use `TypeLoc::getBeginLoc()` to
+// get the beginning of this LocalSourceRange but for dependent template
+// specializations `getBeginLoc` returns the location of `T` yielding
+// `T::template ST::`.
+if (auto DependentTL = TL.getAs()) 
{
+  if (getTemplateKeywordLoc().isValid())
+return SourceRange(getTemplateKeywordLoc(),
+   LoadSourceLocation(Data, Offset + sizeof(void *)));
+  return SourceRange(getTemplateNameLoc(),
+ LoadSourceLocation(Data, Offset + sizeof(void *)));
+}
 return SourceRange(TL.getBeginLoc(),
LoadSourceLocation(Data, Offset + sizeof(void*)));
   }


Index: clang/lib/AST/NestedNameSpecifier.cpp
===
--- clang/lib/AST/NestedNameSpecifier.cpp
+++ clang/lib/AST/NestedNameSpecifier.cpp
@@ -439,6 +439,20 @@
 // Note: the 'template' keyword is part of the TypeLoc.
 void *TypeData = LoadPointer(Data, Offset);
 TypeLoc TL(Qualifier->getAsType(), TypeData);
+// For `T::template ST::x`
+// `NestedNameSpecifierLoc::getLocalSourceRange()` should return `template
+// ST::`.
+// For any TypeLoc we can just use `TypeLoc::getBeginLoc()` to
+// get the beginning of this LocalSourceRange but for dependent template
+// specializations `getBeginLoc` returns the location of `T` yielding
+// `T::template ST::`.
+if (auto DependentTL = TL.getAs()) {
+  if (getTemplateKeywordLoc().isValid())
+return SourceRange(getTemplateKeywordLoc(),
+   LoadSourceLocation(Data, Offset + sizeof(void *)));
+  return SourceRange(getTemplateNameLoc(),
+ LoadSourceLocation(Data, Offset + sizeof(void *)));
+}
 return SourceRange(TL.getBeginLoc(),
LoadSourceLocation(Data, Offset + sizeof(void*)));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

2020-08-12 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added inline comments.



Comment at: cfe/trunk/lib/Frontend/CompilerInvocation.cpp:3457
+  if (Diags.isIgnored(diag::warn_profile_data_misexpect, SourceLocation()))
+Res.FrontendOpts.LLVMArgs.push_back("-pgo-warn-misexpect");
+

rnk wrote:
> Clang generally tries to avoid relying on LLVM option parsing if at all 
> possible. It is not thread-safe, among other things. This causes essentially 
> every compile to call `llvm::cl::ParseCommandLineOptions`, when previously it 
> would only happen if the user passed unstable -mllvm flags.
And it is buggy, if you add "!" as a fix, there are many test failures...
http://45.33.8.238/linux/18098/step_7.txt


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66324

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


[PATCH] D85099: [UpdateTestChecks] Match unnamed values like "@[0-9]+" and "![0-9]+"

2020-08-12 Thread Nico Weber via Phabricator via cfe-commits
thakis added inline comments.



Comment at: llvm/utils/UpdateTestChecks/common.py:390
+for nameless_value in nameless_values:
+if re.fullmatch(nameless_value.ir_prefix + r'[0-9]+?', var, 
re.IGNORECASE):
+warn("Change IR value name '%s' to prevent possible conflict with 
scripted FileCheck name." % (var,))

This is Py3.4+. Per https://llvm.org/docs/GettingStarted.html#id7 we still 
support 2.7, which is why all my bots run that, which is why this breaks my 
bots :) (...and everyone still using Python 2.7 locally). Can you do this some 
other way? Maybe just surround the re with "^..$'?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85099

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


[PATCH] D85842: Fix `NestedNameSpecifierLoc::getLocalSourceRange()`

2020-08-12 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 285122.
eduucaldas added a comment.

Don't assume the code is completely correct


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85842

Files:
  clang/lib/AST/NestedNameSpecifier.cpp


Index: clang/lib/AST/NestedNameSpecifier.cpp
===
--- clang/lib/AST/NestedNameSpecifier.cpp
+++ clang/lib/AST/NestedNameSpecifier.cpp
@@ -439,6 +439,17 @@
 // Note: the 'template' keyword is part of the TypeLoc.
 void *TypeData = LoadPointer(Data, Offset);
 TypeLoc TL(Qualifier->getAsType(), TypeData);
+// For `T::template ST::x`
+// `NestedNameSpecifierLoc::getLocalSourceRange()` should return `template
+// ST::` but `DependentTemplateSpecializationTypeLoc::getBeginLoc()`
+// returns `^T::temp...`
+if (auto DependentTL = TL.getAs()) 
{
+  if (getTemplateKeywordLoc().isValid())
+return SourceRange(getTemplateKeywordLoc(),
+   LoadSourceLocation(Data, Offset + sizeof(void *)));
+  return SourceRange(getTemplateNameLoc(),
+ LoadSourceLocation(Data, Offset + sizeof(void *)));
+}
 return SourceRange(TL.getBeginLoc(),
LoadSourceLocation(Data, Offset + sizeof(void*)));
   }


Index: clang/lib/AST/NestedNameSpecifier.cpp
===
--- clang/lib/AST/NestedNameSpecifier.cpp
+++ clang/lib/AST/NestedNameSpecifier.cpp
@@ -439,6 +439,17 @@
 // Note: the 'template' keyword is part of the TypeLoc.
 void *TypeData = LoadPointer(Data, Offset);
 TypeLoc TL(Qualifier->getAsType(), TypeData);
+// For `T::template ST::x`
+// `NestedNameSpecifierLoc::getLocalSourceRange()` should return `template
+// ST::` but `DependentTemplateSpecializationTypeLoc::getBeginLoc()`
+// returns `^T::temp...`
+if (auto DependentTL = TL.getAs()) {
+  if (getTemplateKeywordLoc().isValid())
+return SourceRange(getTemplateKeywordLoc(),
+   LoadSourceLocation(Data, Offset + sizeof(void *)));
+  return SourceRange(getTemplateNameLoc(),
+ LoadSourceLocation(Data, Offset + sizeof(void *)));
+}
 return SourceRange(TL.getBeginLoc(),
LoadSourceLocation(Data, Offset + sizeof(void*)));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] df3bfaa - [Driver] Change -fnostack-clash-protection to -fno-stack-clash-protection

2020-08-12 Thread Dávid Bolvanský via cfe-commits

Author: Dávid Bolvanský
Date: 2020-08-12T18:36:26+02:00
New Revision: df3bfaa39071a1382a59a94658ee1a2da30d92fd

URL: 
https://github.com/llvm/llvm-project/commit/df3bfaa39071a1382a59a94658ee1a2da30d92fd
DIFF: 
https://github.com/llvm/llvm-project/commit/df3bfaa39071a1382a59a94658ee1a2da30d92fd.diff

LOG: [Driver] Change -fnostack-clash-protection to  -fno-stack-clash-protection

Clang command line docs mention `-fno-stack-clash-protection`, and GCC also 
uses  -fno-stack-clash-protection.

Fixes PR47139

Reviewed By: tstellar

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/stack-clash-protection.c

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index e09b6468eea7..99a2ec7ad2d4 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1794,7 +1794,7 @@ def fstack_protector_all : Flag<["-"], 
"fstack-protector-all">, Group,
   HelpText<"Enable stack protectors for all functions">;
 def fstack_clash_protection : Flag<["-"], "fstack-clash-protection">, 
Group, Flags<[CC1Option]>,
   HelpText<"Enable stack clash protection">;
-def fnostack_clash_protection : Flag<["-"], "fnostack-clash-protection">, 
Group,
+def fno_stack_clash_protection : Flag<["-"], "fno-stack-clash-protection">, 
Group,
   HelpText<"Disable stack clash protection">;
 def fstack_protector_strong : Flag<["-"], "fstack-protector-strong">, 
Group,
   HelpText<"Enable stack protectors for some functions vulnerable to stack 
smashing. "

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 50fe1fcd1615..82cf2538338f 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2978,7 +2978,7 @@ static void RenderSCPOptions(const ToolChain , const 
ArgList ,
 return;
 
   if (Args.hasFlag(options::OPT_fstack_clash_protection,
-   options::OPT_fnostack_clash_protection, false))
+   options::OPT_fno_stack_clash_protection, false))
 CmdArgs.push_back("-fstack-clash-protection");
 }
 

diff  --git a/clang/test/Driver/stack-clash-protection.c 
b/clang/test/Driver/stack-clash-protection.c
index a2cf3f82a8fd..5217ed26a5b1 100644
--- a/clang/test/Driver/stack-clash-protection.c
+++ b/clang/test/Driver/stack-clash-protection.c
@@ -1,6 +1,6 @@
 // RUN: %clang -target i386-unknown-linux -fstack-clash-protection -### %s 
2>&1 | FileCheck %s -check-prefix=SCP-i386
-// RUN: %clang -target i386-unknown-linux -fnostack-clash-protection 
-fstack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386
-// RUN: %clang -target i386-unknown-linux -fstack-clash-protection 
-fnostack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386-NO
+// RUN: %clang -target i386-unknown-linux -fno-stack-clash-protection 
-fstack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386
+// RUN: %clang -target i386-unknown-linux -fstack-clash-protection 
-fno-stack-clash-protection -### %s 2>&1 | FileCheck %s 
-check-prefix=SCP-i386-NO
 // SCP-i386: "-fstack-clash-protection"
 // SCP-i386-NO-NOT: "-fstack-clash-protection"
 



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


[PATCH] D85844: [Driver] Change -fnostack-clash-protection to -fno-stack-clash-protection

2020-08-12 Thread Dávid Bolvanský via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdf3bfaa39071: [Driver] Change -fnostack-clash-protection to  
-fno-stack-clash-protection (authored by xbolva00).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85844

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-clash-protection.c


Index: clang/test/Driver/stack-clash-protection.c
===
--- clang/test/Driver/stack-clash-protection.c
+++ clang/test/Driver/stack-clash-protection.c
@@ -1,6 +1,6 @@
 // RUN: %clang -target i386-unknown-linux -fstack-clash-protection -### %s 
2>&1 | FileCheck %s -check-prefix=SCP-i386
-// RUN: %clang -target i386-unknown-linux -fnostack-clash-protection 
-fstack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386
-// RUN: %clang -target i386-unknown-linux -fstack-clash-protection 
-fnostack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386-NO
+// RUN: %clang -target i386-unknown-linux -fno-stack-clash-protection 
-fstack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386
+// RUN: %clang -target i386-unknown-linux -fstack-clash-protection 
-fno-stack-clash-protection -### %s 2>&1 | FileCheck %s 
-check-prefix=SCP-i386-NO
 // SCP-i386: "-fstack-clash-protection"
 // SCP-i386-NO-NOT: "-fstack-clash-protection"
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2978,7 +2978,7 @@
 return;
 
   if (Args.hasFlag(options::OPT_fstack_clash_protection,
-   options::OPT_fnostack_clash_protection, false))
+   options::OPT_fno_stack_clash_protection, false))
 CmdArgs.push_back("-fstack-clash-protection");
 }
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1794,7 +1794,7 @@
   HelpText<"Enable stack protectors for all functions">;
 def fstack_clash_protection : Flag<["-"], "fstack-clash-protection">, 
Group, Flags<[CC1Option]>,
   HelpText<"Enable stack clash protection">;
-def fnostack_clash_protection : Flag<["-"], "fnostack-clash-protection">, 
Group,
+def fno_stack_clash_protection : Flag<["-"], "fno-stack-clash-protection">, 
Group,
   HelpText<"Disable stack clash protection">;
 def fstack_protector_strong : Flag<["-"], "fstack-protector-strong">, 
Group,
   HelpText<"Enable stack protectors for some functions vulnerable to stack 
smashing. "


Index: clang/test/Driver/stack-clash-protection.c
===
--- clang/test/Driver/stack-clash-protection.c
+++ clang/test/Driver/stack-clash-protection.c
@@ -1,6 +1,6 @@
 // RUN: %clang -target i386-unknown-linux -fstack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386
-// RUN: %clang -target i386-unknown-linux -fnostack-clash-protection -fstack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386
-// RUN: %clang -target i386-unknown-linux -fstack-clash-protection -fnostack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386-NO
+// RUN: %clang -target i386-unknown-linux -fno-stack-clash-protection -fstack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386
+// RUN: %clang -target i386-unknown-linux -fstack-clash-protection -fno-stack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386-NO
 // SCP-i386: "-fstack-clash-protection"
 // SCP-i386-NO-NOT: "-fstack-clash-protection"
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2978,7 +2978,7 @@
 return;
 
   if (Args.hasFlag(options::OPT_fstack_clash_protection,
-   options::OPT_fnostack_clash_protection, false))
+   options::OPT_fno_stack_clash_protection, false))
 CmdArgs.push_back("-fstack-clash-protection");
 }
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1794,7 +1794,7 @@
   HelpText<"Enable stack protectors for all functions">;
 def fstack_clash_protection : Flag<["-"], "fstack-clash-protection">, Group, Flags<[CC1Option]>,
   HelpText<"Enable stack clash protection">;
-def fnostack_clash_protection : Flag<["-"], "fnostack-clash-protection">, Group,
+def fno_stack_clash_protection : Flag<["-"], "fno-stack-clash-protection">, Group,
   HelpText<"Disable stack 

[PATCH] D85826: [clang] Make signature help work with dependent args

2020-08-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 285117.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Fix typo in comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85826

Files:
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/call.cpp

Index: clang/test/CodeCompletion/call.cpp
===
--- clang/test/CodeCompletion/call.cpp
+++ clang/test/CodeCompletion/call.cpp
@@ -32,3 +32,17 @@
   // CHECK-CC3-NEXT: OVERLOAD: [#void#]f(<#int i#>, int j, int k)
   // CHECK-CC3-NEXT: OVERLOAD: [#void#]f(<#float x#>, float y)
 }
+
+template 
+void foo(T t) {
+  f(t, t, t);
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:38:5 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+  // CHECK-CC4: f()
+  // CHECK-CC4-NEXT: f(<#X#>)
+  // CHECK-CC4-NEXT: f(<#int i#>, int j, int k)
+  // CHECK-CC4-NEXT: f(<#float x#>, float y)
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:38:8 %s -o - | FileCheck -check-prefix=CHECK-CC5 %s
+  // CHECK-CC5-NOT: f()
+  // CHECK-CC5: f(int i, <#int j#>, int k)
+  // CHECK-CC5-NEXT: f(float x, <#float y#>)
+}
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -5562,22 +5562,43 @@
 
   // FIXME: Provide support for variadic template functions.
   // Ignore type-dependent call expressions entirely.
-  if (!Fn || Fn->isTypeDependent() || anyNullArguments(Args) ||
-  Expr::hasAnyTypeDependentArguments(Args)) {
+  if (!Fn || Fn->isTypeDependent() || anyNullArguments(Args))
 return QualType();
+
+  SmallVector Results;
+
+  Expr *NakedFn = Fn->IgnoreParenCasts();
+  // In presence of dependent args we surface all possible signatures, without
+  // performing any semantic checks on availability. That's to improve user
+  // experience, it is better to see all overloads rather than none.
+  if (Expr::hasAnyTypeDependentArguments(Args)) {
+auto AddIfArgCountChecks = [, ](FunctionDecl *FD) {
+  if (FD->getNumParams() < Args.size())
+return;
+  Results.push_back(ResultCandidate(FD));
+};
+if (auto *ULE = dyn_cast(NakedFn)) {
+  for (auto *D : ULE->decls())
+AddIfArgCountChecks(D->getAsFunction());
+} else if (auto *UME = dyn_cast(NakedFn)) {
+  for (auto *D : UME->decls())
+AddIfArgCountChecks(D->getAsFunction());
+} else if (auto *ME = dyn_cast(NakedFn)) {
+  if (auto *FD = ME->getMemberDecl()->getAsFunction())
+AddIfArgCountChecks(FD);
+}
+// FIXME: handle function pointers and lambdas.
+return ProduceSignatureHelp(*this, S, Results, Args.size(), OpenParLoc);
   }
 
   // Build an overload candidate set based on the functions we find.
   SourceLocation Loc = Fn->getExprLoc();
   OverloadCandidateSet CandidateSet(Loc, OverloadCandidateSet::CSK_Normal);
 
-  SmallVector Results;
-
-  Expr *NakedFn = Fn->IgnoreParenCasts();
-  if (auto ULE = dyn_cast(NakedFn))
+  if (auto ULE = dyn_cast(NakedFn)) {
 AddOverloadedCallCandidates(ULE, Args, CandidateSet,
 /*PartialOverloading=*/true);
-  else if (auto UME = dyn_cast(NakedFn)) {
+  } else if (auto UME = dyn_cast(NakedFn)) {
 TemplateArgumentListInfo TemplateArgsBuffer, *TemplateArgs = nullptr;
 if (UME->hasExplicitTemplateArgs()) {
   UME->copyTemplateArgumentsInto(TemplateArgsBuffer);
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1166,46 +1166,75 @@
 }
 
 TEST(SignatureHelpTest, OpeningParen) {
-  llvm::StringLiteral Tests[] = {// Recursive function call.
- R"cpp(
-int foo(int a, int b, int c);
-int main() {
-  foo(foo $p^( foo(10, 10, 10), ^ )));
-})cpp",
- // Functional type cast.
- R"cpp(
-struct Foo {
-  Foo(int a, int b, int c);
-};
-int main() {
-  Foo $p^( 10, ^ );
-})cpp",
- // New expression.
- R"cpp(
-struct Foo {
-  Foo(int a, int b, int c);
-};
-int main() {
-  new Foo $p^( 10, ^ );
-})cpp",
- // Macro expansion.
- R"cpp(
-int foo(int a, int b, int c);
-#define FOO foo(
-
-int main() {
-  // Macro expansions.
-  $p^FOO 10, ^ );
-})cpp",
- // Macro arguments.
- R"cpp(
-int foo(int a, int b, int c);
-int main() {
-#define ID(X) X

[PATCH] D85826: [clang] Make signature help work with dependent args

2020-08-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 285116.
kadircet added a comment.

- Add tests into clang-lit
- Make sure current number of args is less than overloads param count.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85826

Files:
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/call.cpp

Index: clang/test/CodeCompletion/call.cpp
===
--- clang/test/CodeCompletion/call.cpp
+++ clang/test/CodeCompletion/call.cpp
@@ -32,3 +32,17 @@
   // CHECK-CC3-NEXT: OVERLOAD: [#void#]f(<#int i#>, int j, int k)
   // CHECK-CC3-NEXT: OVERLOAD: [#void#]f(<#float x#>, float y)
 }
+
+template 
+void foo(T t) {
+  f(t, t, t);
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:38:5 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+  // CHECK-CC4: f()
+  // CHECK-CC4-NEXT: f(<#X#>)
+  // CHECK-CC4-NEXT: f(<#int i#>, int j, int k)
+  // CHECK-CC4-NEXT: f(<#float x#>, float y)
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:38:8 %s -o - | FileCheck -check-prefix=CHECK-CC5 %s
+  // CHECK-CC5-NOT: f()
+  // CHECK-CC5: f(int i, <#int j#>, int k)
+  // CHECK-CC5-NEXT: f(float x, <#float y#>)
+}
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -5562,22 +5562,43 @@
 
   // FIXME: Provide support for variadic template functions.
   // Ignore type-dependent call expressions entirely.
-  if (!Fn || Fn->isTypeDependent() || anyNullArguments(Args) ||
-  Expr::hasAnyTypeDependentArguments(Args)) {
+  if (!Fn || Fn->isTypeDependent() || anyNullArguments(Args))
 return QualType();
+
+  SmallVector Results;
+
+  Expr *NakedFn = Fn->IgnoreParenCasts();
+  // In presence of dependent args we surface all posible signatures, without
+  // performing any semantic checks on availability. That's to improve user
+  // experience, it is better to see all overloads rather than none.
+  if (Expr::hasAnyTypeDependentArguments(Args)) {
+auto AddIfArgCountChecks = [, ](FunctionDecl *FD) {
+  if (FD->getNumParams() < Args.size())
+return;
+  Results.push_back(ResultCandidate(FD));
+};
+if (auto *ULE = dyn_cast(NakedFn)) {
+  for (auto *D : ULE->decls())
+AddIfArgCountChecks(D->getAsFunction());
+} else if (auto *UME = dyn_cast(NakedFn)) {
+  for (auto *D : UME->decls())
+AddIfArgCountChecks(D->getAsFunction());
+} else if (auto *ME = dyn_cast(NakedFn)) {
+  if (auto *FD = ME->getMemberDecl()->getAsFunction())
+AddIfArgCountChecks(FD);
+}
+// FIXME: handle function pointers and lambdas.
+return ProduceSignatureHelp(*this, S, Results, Args.size(), OpenParLoc);
   }
 
   // Build an overload candidate set based on the functions we find.
   SourceLocation Loc = Fn->getExprLoc();
   OverloadCandidateSet CandidateSet(Loc, OverloadCandidateSet::CSK_Normal);
 
-  SmallVector Results;
-
-  Expr *NakedFn = Fn->IgnoreParenCasts();
-  if (auto ULE = dyn_cast(NakedFn))
+  if (auto ULE = dyn_cast(NakedFn)) {
 AddOverloadedCallCandidates(ULE, Args, CandidateSet,
 /*PartialOverloading=*/true);
-  else if (auto UME = dyn_cast(NakedFn)) {
+  } else if (auto UME = dyn_cast(NakedFn)) {
 TemplateArgumentListInfo TemplateArgsBuffer, *TemplateArgs = nullptr;
 if (UME->hasExplicitTemplateArgs()) {
   UME->copyTemplateArgumentsInto(TemplateArgsBuffer);
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1166,46 +1166,75 @@
 }
 
 TEST(SignatureHelpTest, OpeningParen) {
-  llvm::StringLiteral Tests[] = {// Recursive function call.
- R"cpp(
-int foo(int a, int b, int c);
-int main() {
-  foo(foo $p^( foo(10, 10, 10), ^ )));
-})cpp",
- // Functional type cast.
- R"cpp(
-struct Foo {
-  Foo(int a, int b, int c);
-};
-int main() {
-  Foo $p^( 10, ^ );
-})cpp",
- // New expression.
- R"cpp(
-struct Foo {
-  Foo(int a, int b, int c);
-};
-int main() {
-  new Foo $p^( 10, ^ );
-})cpp",
- // Macro expansion.
- R"cpp(
-int foo(int a, int b, int c);
-#define FOO foo(
-
-int main() {
-  // Macro expansions.
-  $p^FOO 10, ^ );
-})cpp",
- // Macro arguments.
- R"cpp(
-int foo(int a, int b, int c);
-

[PATCH] D85844: [Driver] Change -fnostack-clash-protection to -fno-stack-clash-protection

2020-08-12 Thread Tom Stellard via Phabricator via cfe-commits
tstellar accepted this revision.
tstellar added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85844

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


[PATCH] D85817: [analyzer] Fix crash with pointer to members values

2020-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

That's a fix for https://bugs.llvm.org/show_bug.cgi?id=46264.

Your code looks great but i don't understand at a glance what the crash was 
caused by and how does your code fix it, can you explain? Like, the original 
test doesn't have any `void *`s and it doesn't have any indirect fields. Also 
the crash happens during visitor phase but the fixes are entirely during 
modeling phase.




Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:2533-2534
   if (isa(D) || isa(D)) {
-// FIXME: Compute lvalue of field pointers-to-member.
-// Right now we just use a non-null void pointer, so that it gives proper
-// results in boolean contexts.
-// FIXME: Maybe delegate this to the surrounding operator&.
-// Note how this expression is lvalue, however pointer-to-member is NonLoc.
-SVal V = svalBuilder.conjureSymbolVal(Ex, LCtx, getContext().VoidPtrTy,
-  currBldrCtx->blockCount());
-state = state->assume(V.castAs(), true);
-Bldr.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V), nullptr,
-  ProgramPoint::PostLValueKind);
+// Delegate all work related to pointer to members to the surrounding
+// operator&.
 return;

Ok so you're saying that there's *always* going to be a surrounding operator 
`&`? That kind of makes sense but if you add more explanation/proof of how you 
figured this out that'd be great.



Comment at: clang/test/Analysis/PR46264.cpp:5
+
+namespace a {
+class b {

I'm pretty sure the namespace is not actually important for the test. `creduce` 
is great but sometimes it misses stuff. Generally, i'm a big fan of manually 
cleaning up `creduce`d test to make them look more like real code. At least 
turn things like `b h;` and `e *i;` into something like `A a;` and `B *b;`. 
Also replace `class` with `struct` because we never care about this entire 
public/private business and `struct` provides a nice default.



Comment at: clang/test/Analysis/PR46264.cpp:9
+  typedef int b::*c;
+  operator c() { return ::d; }
+  int d;

It's worth it to comment where exactly is this conversion operator used in the 
text (i.e., within the if-condition). People do in fact sometimes do this kind 
of thing in real life: provide a conversion to pointer-to-member *instead of* 
conversion to `bool` because it causes fewer potential further accidental 
implicit conversions to be possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85817

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


[PATCH] D85844: [Driver] Change -fnostack-clash-protection to -fno-stack-clash-protection

2020-08-12 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 created this revision.
xbolva00 added a reviewer: sberg.
Herald added subscribers: cfe-commits, dang.
Herald added a project: clang.
xbolva00 requested review of this revision.

Clang command line docs mention `-fno-stack-clash-protection`, and GCC also 
uses  -fno-stack-clash-protection.

Fixes PR47139


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85844

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-clash-protection.c


Index: clang/test/Driver/stack-clash-protection.c
===
--- clang/test/Driver/stack-clash-protection.c
+++ clang/test/Driver/stack-clash-protection.c
@@ -1,6 +1,6 @@
 // RUN: %clang -target i386-unknown-linux -fstack-clash-protection -### %s 
2>&1 | FileCheck %s -check-prefix=SCP-i386
-// RUN: %clang -target i386-unknown-linux -fnostack-clash-protection 
-fstack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386
-// RUN: %clang -target i386-unknown-linux -fstack-clash-protection 
-fnostack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386-NO
+// RUN: %clang -target i386-unknown-linux -fno-stack-clash-protection 
-fstack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386
+// RUN: %clang -target i386-unknown-linux -fstack-clash-protection 
-fno-stack-clash-protection -### %s 2>&1 | FileCheck %s 
-check-prefix=SCP-i386-NO
 // SCP-i386: "-fstack-clash-protection"
 // SCP-i386-NO-NOT: "-fstack-clash-protection"
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2978,7 +2978,7 @@
 return;
 
   if (Args.hasFlag(options::OPT_fstack_clash_protection,
-   options::OPT_fnostack_clash_protection, false))
+   options::OPT_fno_stack_clash_protection, false))
 CmdArgs.push_back("-fstack-clash-protection");
 }
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1794,7 +1794,7 @@
   HelpText<"Enable stack protectors for all functions">;
 def fstack_clash_protection : Flag<["-"], "fstack-clash-protection">, 
Group, Flags<[CC1Option]>,
   HelpText<"Enable stack clash protection">;
-def fnostack_clash_protection : Flag<["-"], "fnostack-clash-protection">, 
Group,
+def fno_stack_clash_protection : Flag<["-"], "fno-stack-clash-protection">, 
Group,
   HelpText<"Disable stack clash protection">;
 def fstack_protector_strong : Flag<["-"], "fstack-protector-strong">, 
Group,
   HelpText<"Enable stack protectors for some functions vulnerable to stack 
smashing. "


Index: clang/test/Driver/stack-clash-protection.c
===
--- clang/test/Driver/stack-clash-protection.c
+++ clang/test/Driver/stack-clash-protection.c
@@ -1,6 +1,6 @@
 // RUN: %clang -target i386-unknown-linux -fstack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386
-// RUN: %clang -target i386-unknown-linux -fnostack-clash-protection -fstack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386
-// RUN: %clang -target i386-unknown-linux -fstack-clash-protection -fnostack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386-NO
+// RUN: %clang -target i386-unknown-linux -fno-stack-clash-protection -fstack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386
+// RUN: %clang -target i386-unknown-linux -fstack-clash-protection -fno-stack-clash-protection -### %s 2>&1 | FileCheck %s -check-prefix=SCP-i386-NO
 // SCP-i386: "-fstack-clash-protection"
 // SCP-i386-NO-NOT: "-fstack-clash-protection"
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2978,7 +2978,7 @@
 return;
 
   if (Args.hasFlag(options::OPT_fstack_clash_protection,
-   options::OPT_fnostack_clash_protection, false))
+   options::OPT_fno_stack_clash_protection, false))
 CmdArgs.push_back("-fstack-clash-protection");
 }
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1794,7 +1794,7 @@
   HelpText<"Enable stack protectors for all functions">;
 def fstack_clash_protection : Flag<["-"], "fstack-clash-protection">, Group, Flags<[CC1Option]>,
   HelpText<"Enable stack clash protection">;
-def fnostack_clash_protection : Flag<["-"], "fnostack-clash-protection">, Group,
+def fno_stack_clash_protection : Flag<["-"], "fno-stack-clash-protection">, Group,
   HelpText<"Disable stack clash 

[PATCH] D85843: Add "status" to the list of absl libraries.

2020-08-12 Thread Vy Nguyen via Phabricator via cfe-commits
oontvoo created this revision.
oontvoo added a reviewer: sbenza.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
oontvoo requested review of this revision.

The Abseil-NoInternalDependenciesCheck currently mistakenly triggers on any 
usage of internal helpers even if it is within absl/status.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85843

Files:
  clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h


Index: clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
===
--- clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
+++ clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
@@ -50,8 +50,8 @@
   static const char *AbseilLibraries[] = {
   "algorithm", "base", "container",   "debugging", "flags",
   "hash",  "iterator", "memory",  "meta",  "numeric",
-  "random","strings",  "synchronization", "time",  "types",
-  "utility"};
+  "random","strings",  "synchronization", "status","time",
+  "types", "utility"};
   return std::any_of(
   std::begin(AbseilLibraries), std::end(AbseilLibraries),
   [&](const char *Library) { return Path.startswith(Library); });


Index: clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
===
--- clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
+++ clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
@@ -50,8 +50,8 @@
   static const char *AbseilLibraries[] = {
   "algorithm", "base", "container",   "debugging", "flags",
   "hash",  "iterator", "memory",  "meta",  "numeric",
-  "random","strings",  "synchronization", "time",  "types",
-  "utility"};
+  "random","strings",  "synchronization", "status","time",
+  "types", "utility"};
   return std::any_of(
   std::begin(AbseilLibraries), std::end(AbseilLibraries),
   [&](const char *Library) { return Path.startswith(Library); });
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85842: Fix `NestedNameSpecifierLoc::getLocalSourceRange()`

2020-08-12 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
eduucaldas requested review of this revision.

>From the documentation comment above 
>`NestedNameSpecifierLoc::getLocalSourceRange()`

  /// Retrieve the source range covering just the last part of
  /// this nested-name-specifier, not including the prefix.
  ///
  /// For example, if this instance refers to a nested-name-specifier
  /// \c \::std::vector::, the returned source range would cover
  /// from "vector" to the last '::'.

We would expect that for a nested-name-specifier with a dependent
template specialization type: `T::template ST::`
`NestedNameSpecifierLoc::getLocalSourceRange()` would return `template
ST::` but instead it returns `T::template ST::`.

The issue might as well be on
`DependentTemplateSpecializationTypeLoc::getLocalSourceRange()` which is
indirectly called from `NestedNameSpecifierLoc::getLocalSourceRange()`.
I think not.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85842

Files:
  clang/lib/AST/NestedNameSpecifier.cpp


Index: clang/lib/AST/NestedNameSpecifier.cpp
===
--- clang/lib/AST/NestedNameSpecifier.cpp
+++ clang/lib/AST/NestedNameSpecifier.cpp
@@ -439,6 +439,14 @@
 // Note: the 'template' keyword is part of the TypeLoc.
 void *TypeData = LoadPointer(Data, Offset);
 TypeLoc TL(Qualifier->getAsType(), TypeData);
+// For `T::template ST::x`
+// `NestedNameSpecifierLoc::getLocalSourceRange()` should return `template
+// ST::` but `DependentTemplateSpecializationTypeLoc::getBeginLoc()`
+// returns `^T::temp...`
+if (auto DependentTL = TL.getAs()) 
{
+  return SourceRange(DependentTL.getTemplateKeywordLoc(),
+ LoadSourceLocation(Data, Offset + sizeof(void *)));
+}
 return SourceRange(TL.getBeginLoc(),
LoadSourceLocation(Data, Offset + sizeof(void*)));
   }


Index: clang/lib/AST/NestedNameSpecifier.cpp
===
--- clang/lib/AST/NestedNameSpecifier.cpp
+++ clang/lib/AST/NestedNameSpecifier.cpp
@@ -439,6 +439,14 @@
 // Note: the 'template' keyword is part of the TypeLoc.
 void *TypeData = LoadPointer(Data, Offset);
 TypeLoc TL(Qualifier->getAsType(), TypeData);
+// For `T::template ST::x`
+// `NestedNameSpecifierLoc::getLocalSourceRange()` should return `template
+// ST::` but `DependentTemplateSpecializationTypeLoc::getBeginLoc()`
+// returns `^T::temp...`
+if (auto DependentTL = TL.getAs()) {
+  return SourceRange(DependentTL.getTemplateKeywordLoc(),
+ LoadSourceLocation(Data, Offset + sizeof(void *)));
+}
 return SourceRange(TL.getBeginLoc(),
LoadSourceLocation(Data, Offset + sizeof(void*)));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

2020-08-12 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment.

In D72705#2213300 , @whisperity wrote:

> In D72705#2210255 , @balazske wrote:
>
>> More results in CodeChecker: emacs_errorreturn 
>> 
>
> Something's not quite right, e.g. at 
> https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=emacs_errorreturn=Unreviewed=Confirmed%20bug=New=Reopened=Unresolved=7258
>  there's a bug path element `Function '???' called [...]`. This seems to be 
> the case for almost every report... but I found no indication where this 
> "???" could come from, in the patch...

Because that is an improved version of the checker. The current form is not 
testable because it detects too less functions. To have more results functions 
were added that may return NULL on error and a missing comparison to NULL is 
detected (similar as missing comparison to `EOF` in this patch). (To have the 
check for NULL some other improvements were made.) The '???' is part of 
development of note tags, the name of the called function is to be placed there 
(probably there is better way of doing these reports).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72705

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


[PATCH] D85193: [clang] Check `expr` inside `InitListChecker::UpdateStructuredListElement()`

2020-08-12 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX added a comment.

In D85193#2212976 , @aaron.ballman 
wrote:

> I have a suggestion for the added comment,

Fixed comment according to your suggestion.

> but I also forgot to ask, do you need someone to commit on your behalf?

I already have commit access.

Thanks for review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85193

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


[PATCH] D84248: [analyzer][StdLibraryFunctionsChecker] Add POSIX time handling functions

2020-08-12 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:2040
+
+if (ConstStructTimevalPtrTy && StructTimespecPtrTy)
+  // int nanosleep(const struct timespec *rqtp, struct timespec *rmtp);

Should be `ConstStructTimespecPtrTy`.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:2092
+  .ArgConstraint(NotNull(ArgNo(0)))
+  .ArgConstraint(NotNull(ArgNo(1;
+

Is it possible to check for the size of the passed buffer? The man page says 
that `buf` should have room for 26 bytes (for `ctime_r` too).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84248

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


[PATCH] D85193: [clang] Check `expr` inside `InitListChecker::UpdateStructuredListElement()`

2020-08-12 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX updated this revision to Diff 285101.
ArcsinX added a comment.

Fix comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85193

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/Sema/init-invalid-struct-array.c


Index: clang/test/Sema/init-invalid-struct-array.c
===
--- /dev/null
+++ clang/test/Sema/init-invalid-struct-array.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+struct S {
+  Unknown u; // expected-error {{unknown type name 'Unknown'}}
+  int i;
+};
+// Should not crash
+struct S s[] = {[0].i = 0, [1].i = 1, {}};
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1585,10 +1585,7 @@
   IList->setInit(Index, ResultExpr);
 }
   }
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
   ++Index;
 }
 
@@ -1643,10 +1640,7 @@
   if (!VerifyOnly && expr)
 IList->setInit(Index, expr);
 
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex, expr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, expr);
   ++Index;
 }
 
@@ -1697,11 +1691,7 @@
   IList->setInit(Index, ResultExpr);
 }
   }
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex,
-ResultExpr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
   ++Index;
   return;
 }
@@ -3100,8 +3090,12 @@
 
   if (Expr *PrevInit = StructuredList->updateInit(SemaRef.Context,
   StructuredIndex, expr)) {
-// This initializer overwrites a previous initializer. Warn.
-diagnoseInitOverride(PrevInit, expr->getSourceRange());
+// This initializer overwrites a previous initializer.
+// No need to diagnose when `expr` is nullptr because a more relevant
+// diagnostic has already been issued and this diagnostic is potentially
+// noise.
+if (expr)
+  diagnoseInitOverride(PrevInit, expr->getSourceRange());
   }
 
   ++StructuredIndex;


Index: clang/test/Sema/init-invalid-struct-array.c
===
--- /dev/null
+++ clang/test/Sema/init-invalid-struct-array.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+struct S {
+  Unknown u; // expected-error {{unknown type name 'Unknown'}}
+  int i;
+};
+// Should not crash
+struct S s[] = {[0].i = 0, [1].i = 1, {}};
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1585,10 +1585,7 @@
   IList->setInit(Index, ResultExpr);
 }
   }
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
   ++Index;
 }
 
@@ -1643,10 +1640,7 @@
   if (!VerifyOnly && expr)
 IList->setInit(Index, expr);
 
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex, expr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, expr);
   ++Index;
 }
 
@@ -1697,11 +1691,7 @@
   IList->setInit(Index, ResultExpr);
 }
   }
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex,
-ResultExpr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
   ++Index;
   return;
 }
@@ -3100,8 +3090,12 @@
 
   if (Expr *PrevInit = StructuredList->updateInit(SemaRef.Context,
   StructuredIndex, expr)) {
-// This initializer overwrites a previous initializer. Warn.
-diagnoseInitOverride(PrevInit, expr->getSourceRange());
+// This initializer overwrites a previous initializer.
+// No need to diagnose when `expr` is nullptr because a more relevant
+// diagnostic has already been issued and this diagnostic is potentially
+// noise.
+if (expr)
+  diagnoseInitOverride(PrevInit, expr->getSourceRange());
   }
 
   ++StructuredIndex;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] fc915d1 - [clang-tidy] use stable_sort instead of sort to fix EXPENSIVE_CHECKS tests

2020-08-12 Thread Erik Pilkington via cfe-commits

Author: Erik Pilkington
Date: 2020-08-12T12:12:10-04:00
New Revision: fc915d13b8671ceddea06e3f2f2d0e18869c41fe

URL: 
https://github.com/llvm/llvm-project/commit/fc915d13b8671ceddea06e3f2f2d0e18869c41fe
DIFF: 
https://github.com/llvm/llvm-project/commit/fc915d13b8671ceddea06e3f2f2d0e18869c41fe.diff

LOG: [clang-tidy] use stable_sort instead of sort to fix EXPENSIVE_CHECKS tests

http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-expensive/17317/console

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 1471301a3431..079d57477216 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -744,7 +744,7 @@ struct EqualClangTidyError {
 std::vector ClangTidyDiagnosticConsumer::take() {
   finalizeLastError();
 
-  llvm::sort(Errors, LessClangTidyError());
+  llvm::stable_sort(Errors, LessClangTidyError());
   Errors.erase(std::unique(Errors.begin(), Errors.end(), 
EqualClangTidyError()),
Errors.end());
   if (RemoveIncompatibleErrors)



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


[PATCH] D84908: [darwin][compiler-rt] build libclang_rt.sim.a Apple Silicon slice, if SDK supports it

2020-08-12 Thread Steven Wu via Phabricator via cfe-commits
steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D84908

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


[PATCH] D68720: Support -fstack-clash-protection for x86

2020-08-12 Thread Stephan Bergmann via Phabricator via cfe-commits
sberg added a comment.

I filed https://bugs.llvm.org/show_bug.cgi?id=47139 "Misspelled 
-fnostack-clash-protection" now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

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


[clang] fbd6d2c - [OPENMP] Fix PR47063: crash when trying to get captured statetment.

2020-08-12 Thread Alexey Bataev via cfe-commits

Author: Alexey Bataev
Date: 2020-08-12T12:05:58-04:00
New Revision: fbd6d2c54e57a4968d29bb22742dd49759b3ecd0

URL: 
https://github.com/llvm/llvm-project/commit/fbd6d2c54e57a4968d29bb22742dd49759b3ecd0
DIFF: 
https://github.com/llvm/llvm-project/commit/fbd6d2c54e57a4968d29bb22742dd49759b3ecd0.diff

LOG: [OPENMP] Fix PR47063: crash when trying to get captured statetment.

Need to call getRawStmt() function instead, when trying to get inner
associated statement for the executable directive. Not all directives
use captured statements.

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/test/OpenMP/target_codegen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 66d1b36dbb6a..c55403920d8f 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -9801,8 +9801,7 @@ void CGOpenMPRuntime::scanForTargetRegionsFunctions(const 
Stmt *S,
 if (!E->hasAssociatedStmt() || !E->getAssociatedStmt())
   return;
 
-scanForTargetRegionsFunctions(
-E->getInnermostCapturedStmt()->getCapturedStmt(), ParentName);
+scanForTargetRegionsFunctions(E->getRawStmt(), ParentName);
 return;
   }
 

diff  --git a/clang/test/OpenMP/target_codegen.cpp 
b/clang/test/OpenMP/target_codegen.cpp
index 9cec6bfa5a48..55b03aae00a5 100644
--- a/clang/test/OpenMP/target_codegen.cpp
+++ b/clang/test/OpenMP/target_codegen.cpp
@@ -712,6 +712,8 @@ int bar(int n){
 // OMP45:   [[BP:%.+]] = alloca [1 x i8*]
 // OMP45:   [[P:%.+]] = alloca [1 x i8*]
 // OMP45:   [[LOCAL_THIS1:%.+]] = load [[S2]]*, [[S2]]** [[LOCAL_THIS]]
+
+// OMP45:   call void @__kmpc_critical(
 // OMP45:   [[ARR_IDX:%.+]] = getelementptr inbounds [[S2]], [[S2]]* 
[[LOCAL_THIS1]], i[[SZ]] 0
 // OMP45:   [[ARR_IDX2:%.+]] = getelementptr inbounds [[S2]], [[S2]]* 
[[LOCAL_THIS1]], i[[SZ]] 0
 
@@ -731,6 +733,7 @@ int bar(int n){
 // OMP45:   call void [[HVT0:@.+]]([[S2]]* [[LOCAL_THIS1]])
 // OMP45-NEXT:  br label %[[END]]
 // OMP45:   [[END]]
+// OMP45:   call void @__kmpc_end_critical(
 
 // Check that the offloading functions are emitted and that the arguments are
 // correct and loaded correctly for the target regions of the callees of bar().
@@ -826,7 +829,7 @@ int bar(int n){
 // OMP50:   call void [[HVT0:@.+]]([[S2]]* [[LOCAL_THIS1]])
 // OMP50-NEXT:  br label %[[END]]
 // OMP50:   [[END]]
- 
+
 void bar () {
 #define pragma_target _Pragma("omp target")
 pragma_target
@@ -838,6 +841,7 @@ class S2 {
 
 public:
   void zee() {
+#pragma omp critical
 #pragma omp target map(this[0])
   a++;
   }



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


[clang] f4f3f67 - [OPENMP]Fix PR37671: Privatize local(private) variables in untied tasks.

2020-08-12 Thread Alexey Bataev via cfe-commits

Author: Alexey Bataev
Date: 2020-08-12T11:28:19-04:00
New Revision: f4f3f678f1994d47f745cbfd6a1026f2408425c6

URL: 
https://github.com/llvm/llvm-project/commit/f4f3f678f1994d47f745cbfd6a1026f2408425c6
DIFF: 
https://github.com/llvm/llvm-project/commit/f4f3f678f1994d47f745cbfd6a1026f2408425c6.diff

LOG: [OPENMP]Fix PR37671: Privatize local(private) variables in untied tasks.

In untied tasks, need to allocate the space for local variales, declared
in task region, when the memory for task data is allocated. THe function
can be interrupted and we can exit from the function in untied task
switch. Need to keep the state of the local variables in this case.
Also, the compiler should not call cleanup when exiting in untied task
switch until the real exit out of the declaration scope is met during
 execution.

Reviewed By: jdoerfert

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

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGOpenMPRuntime.h
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/test/OpenMP/task_codegen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 19c703623d27..66d1b36dbb6a 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -180,7 +180,7 @@ class CGOpenMPTaskOutlinedRegionInfo final : public 
CGOpenMPRegionInfo {
 UntiedCodeGen(CGF);
 CodeGenFunction::JumpDest CurPoint =
 CGF.getJumpDestInCurrentScope(".untied.next.");
-CGF.EmitBranchThroughCleanup(CGF.ReturnBlock);
+CGF.EmitBranch(CGF.ReturnBlock.getBlock());
 CGF.EmitBlock(CGF.createBasicBlock(".untied.jmp."));
 
UntiedSwitch->addCase(CGF.Builder.getInt32(UntiedSwitch->getNumCases()),
   CGF.Builder.GetInsertBlock());
@@ -3370,10 +3370,14 @@ struct PrivateHelpersTy {
const VarDecl *PrivateCopy, const VarDecl *PrivateElemInit)
   : OriginalRef(OriginalRef), Original(Original), PrivateCopy(PrivateCopy),
 PrivateElemInit(PrivateElemInit) {}
+  PrivateHelpersTy(const VarDecl *Original) : Original(Original) {}
   const Expr *OriginalRef = nullptr;
   const VarDecl *Original = nullptr;
   const VarDecl *PrivateCopy = nullptr;
   const VarDecl *PrivateElemInit = nullptr;
+  bool isLocalPrivate() const {
+return !OriginalRef && !PrivateCopy && !PrivateElemInit;
+  }
 };
 typedef std::pair PrivateDataTy;
 } // anonymous namespace
@@ -3390,6 +3394,11 @@ createPrivatesRecordDecl(CodeGenModule , 
ArrayRef Privates) {
 for (const auto  : Privates) {
   const VarDecl *VD = Pair.second.Original;
   QualType Type = VD->getType().getNonReferenceType();
+  // If the private variable is a local variable with lvalue ref type,
+  // allocate the pointer instead of the pointee type.
+  if (Pair.second.isLocalPrivate() &&
+  VD->getType()->isLValueReferenceType())
+Type = C.getPointerType(Type);
   FieldDecl *FD = addFieldToRecordDecl(C, RD, Type);
   if (VD->hasAttrs()) {
 for (specific_attr_iterator I(VD->getAttrs().begin()),
@@ -3643,10 +3652,7 @@ static llvm::Value 
*emitDestructorsFunction(CodeGenModule ,
 /// \endcode
 static llvm::Value *
 emitTaskPrivateMappingFunction(CodeGenModule , SourceLocation Loc,
-   ArrayRef PrivateVars,
-   ArrayRef FirstprivateVars,
-   ArrayRef LastprivateVars,
-   QualType PrivatesQTy,
+   const OMPTaskDataTy , QualType PrivatesQTy,
ArrayRef Privates) {
   ASTContext  = CGM.getContext();
   FunctionArgList Args;
@@ -3655,9 +3661,9 @@ emitTaskPrivateMappingFunction(CodeGenModule , 
SourceLocation Loc,
   C.getPointerType(PrivatesQTy).withConst().withRestrict(),
   ImplicitParamDecl::Other);
   Args.push_back();
-  llvm::DenseMap PrivateVarsPos;
+  llvm::DenseMap, unsigned> PrivateVarsPos;
   unsigned Counter = 1;
-  for (const Expr *E : PrivateVars) {
+  for (const Expr *E : Data.PrivateVars) {
 Args.push_back(ImplicitParamDecl::Create(
 C, /*DC=*/nullptr, Loc, /*Id=*/nullptr,
 C.getPointerType(C.getPointerType(E->getType()))
@@ -3668,7 +3674,7 @@ emitTaskPrivateMappingFunction(CodeGenModule , 
SourceLocation Loc,
 PrivateVarsPos[VD] = Counter;
 ++Counter;
   }
-  for (const Expr *E : FirstprivateVars) {
+  for (const Expr *E : Data.FirstprivateVars) {
 Args.push_back(ImplicitParamDecl::Create(
 C, /*DC=*/nullptr, Loc, /*Id=*/nullptr,
 C.getPointerType(C.getPointerType(E->getType()))
@@ -3679,7 +3685,7 @@ emitTaskPrivateMappingFunction(CodeGenModule , 
SourceLocation Loc,
 PrivateVarsPos[VD] = Counter;
 ++Counter;
   }
-  for (const Expr *E : LastprivateVars) {
+  for (const 

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

2020-08-12 Thread Whisperity via Phabricator via cfe-commits
whisperity added a comment.

In D72705#2210255 , @balazske wrote:

> More results in CodeChecker: emacs_errorreturn 
> 

Something's not quite right, e.g. at 
https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=emacs_errorreturn=Unreviewed=Confirmed%20bug=New=Reopened=Unresolved=7258
 there's a bug path element `Function '???' called [...]`. This seems to be the 
case for almost every report... but I found no indication where this "???" 
could come from, in the patch...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72705

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


[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

2020-08-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp:73
+  // If the variable has an alias then it can be changed by that alias as well.
+  // FIXME: Track pointers and references.
+  if (hasPtrOrReferenceInFunc(Func, CondVar))

baloghadamsoftware wrote:
> aaron.ballman wrote:
> > This FIXME makes me worried that what we really want is a clang static 
> > analyzer check, since the check is already flow sensitive and needs to be 
> > data sensitive as well. Have you considered implementing this as a static 
> > analyzer check?
> Actually, it is only a proposal for improvement. Maybe FIXME is not the best 
> wording for it. This is not a bug to fix, just a possibility to find more 
> true positives. This check is very similar to the infinite loop check which 
> is also in Tidy. In the Static Analyzer this check would be too heavyweight 
> and would probably produce false positives.
Ah, thank you for clarifying (though I think it's interesting that we come down 
on opposite sides of the FP question -- I would argue that the static analyzer 
check would have considerably fewer false positives than the tidy check when 
handling this case).

If the idea is that pointers and references need to be supported in a future 
patch, I think FIXME is a fine comment. If the idea is that pointers and 
references are a potential improvement, I'd probably clarify the comment to 
something more like `FIXME: could potentially support tracking pointers and 
references in the future to improve catching true positives through aliases.`



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc-redundant-condition.cpp:1097
+  }
+}

baloghadamsoftware wrote:
> aaron.ballman wrote:
> > Can you add some tests that exercise GNU expression statements, just to 
> > make sure we get the behavior correct?
> > ```
> > if (({foo;})) {
> > } else if (({foo;})) {
> > }
> > 
> > if (foo) ({bar;});
> > else if (foo) ({bar;});
> > ```
> > Another thing that's interesting to test is whether the redundant 
> > expression is in a subexpression which doesn't contribute to the value of 
> > the control expression:
> > ```
> > if (foo(), val) {
> > } else if (foo(), other_val) {
> > }
> > ```
> > (Similar can be tested with GNU statement expressions.)
> Do you mean that I should handle these cases as well? (Detect a bug, provide 
> a fix.)
If the check misses these cases, I think that's reasonable (just comment that 
the tests aren't currently handled). If the check diagnoses these cases but the 
fixit causes an issue, then I think that should be corrected in this patch. If 
it's trivial to support these cases (diagnosing or fixing), it's your call on 
whether you want to do so. I mostly just want the test coverage so I can know 
what to expect.


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

https://reviews.llvm.org/D81272

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


[PATCH] D85429: [OpenCL] Allow for variadic macros in C++ for OpenCL

2020-08-12 Thread Anastasia Stulova via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c8a4ee0764c: [OpenCL] Remove warning for variadic macros in 
C++ for OpenCL. (authored by Anastasia).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85429

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/macro_variadic.cl


Index: clang/test/Preprocessor/macro_variadic.cl
===
--- clang/test/Preprocessor/macro_variadic.cl
+++ clang/test/Preprocessor/macro_variadic.cl
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -verify %s -cl-std=CL1.2
 // RUN: %clang_cc1 -verify %s -pedantic -DPEDANTIC -cl-std=CL1.2
+// RUN: %clang_cc1 -verify %s -cl-std=CLC++
+// RUN: %clang_cc1 -verify %s -pedantic -cl-std=CLC++
 
 
 #define NO_VAR_FUNC(...)  5
@@ -15,6 +17,11 @@
 
 void foo() {
   NO_VAR_FUNC(1, 2, 3);
-  VAR_FUNC(1, 2, 3); //expected-error{{implicit declaration of function 'func' 
is invalid in OpenCL}}
+  VAR_FUNC(1, 2, 3);
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2{{implicit declaration of function 'func' is invalid in 
OpenCL}}
+#else
+// expected-error@-4{{use of undeclared identifier 'func'}}
+#endif
   VAR_PRINTF("%i", 1);
 }
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2397,7 +2397,7 @@
  diag::ext_variadic_macro);
 
   // OpenCL v1.2 s6.9.e: variadic macros are not supported.
-  if (LangOpts.OpenCL) {
+  if (LangOpts.OpenCL && !LangOpts.OpenCLCPlusPlus) {
 Diag(Tok, diag::ext_pp_opencl_variadic_macros);
   }
 


Index: clang/test/Preprocessor/macro_variadic.cl
===
--- clang/test/Preprocessor/macro_variadic.cl
+++ clang/test/Preprocessor/macro_variadic.cl
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -verify %s -cl-std=CL1.2
 // RUN: %clang_cc1 -verify %s -pedantic -DPEDANTIC -cl-std=CL1.2
+// RUN: %clang_cc1 -verify %s -cl-std=CLC++
+// RUN: %clang_cc1 -verify %s -pedantic -cl-std=CLC++
 
 
 #define NO_VAR_FUNC(...)  5
@@ -15,6 +17,11 @@
 
 void foo() {
   NO_VAR_FUNC(1, 2, 3);
-  VAR_FUNC(1, 2, 3); //expected-error{{implicit declaration of function 'func' is invalid in OpenCL}}
+  VAR_FUNC(1, 2, 3);
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2{{implicit declaration of function 'func' is invalid in OpenCL}}
+#else
+// expected-error@-4{{use of undeclared identifier 'func'}}
+#endif
   VAR_PRINTF("%i", 1);
 }
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2397,7 +2397,7 @@
  diag::ext_variadic_macro);
 
   // OpenCL v1.2 s6.9.e: variadic macros are not supported.
-  if (LangOpts.OpenCL) {
+  if (LangOpts.OpenCL && !LangOpts.OpenCLCPlusPlus) {
 Diag(Tok, diag::ext_pp_opencl_variadic_macros);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 3c8a4ee - [OpenCL] Remove warning for variadic macros in C++ for OpenCL.

2020-08-12 Thread Anastasia Stulova via cfe-commits

Author: Anastasia Stulova
Date: 2020-08-12T16:17:54+01:00
New Revision: 3c8a4ee0764cafb2ba204c7cb7d8b37e6adf72a8

URL: 
https://github.com/llvm/llvm-project/commit/3c8a4ee0764cafb2ba204c7cb7d8b37e6adf72a8
DIFF: 
https://github.com/llvm/llvm-project/commit/3c8a4ee0764cafb2ba204c7cb7d8b37e6adf72a8.diff

LOG: [OpenCL] Remove warning for variadic macros in C++ for OpenCL.

Patch by Ole Strohm (olestrohm)!

Tags: #clang

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

Added: 


Modified: 
clang/lib/Lex/PPDirectives.cpp
clang/test/Preprocessor/macro_variadic.cl

Removed: 




diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 053ef1d2dd18..e4b901a950ae 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -2397,7 +2397,7 @@ bool Preprocessor::ReadMacroParameterList(MacroInfo *MI, 
Token ) {
  diag::ext_variadic_macro);
 
   // OpenCL v1.2 s6.9.e: variadic macros are not supported.
-  if (LangOpts.OpenCL) {
+  if (LangOpts.OpenCL && !LangOpts.OpenCLCPlusPlus) {
 Diag(Tok, diag::ext_pp_opencl_variadic_macros);
   }
 

diff  --git a/clang/test/Preprocessor/macro_variadic.cl 
b/clang/test/Preprocessor/macro_variadic.cl
index cc9458da558e..e58896487121 100644
--- a/clang/test/Preprocessor/macro_variadic.cl
+++ b/clang/test/Preprocessor/macro_variadic.cl
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -verify %s -cl-std=CL1.2
 // RUN: %clang_cc1 -verify %s -pedantic -DPEDANTIC -cl-std=CL1.2
+// RUN: %clang_cc1 -verify %s -cl-std=CLC++
+// RUN: %clang_cc1 -verify %s -pedantic -cl-std=CLC++
 
 
 #define NO_VAR_FUNC(...)  5
@@ -15,6 +17,11 @@ int printf(__constant const char *st, ...);
 
 void foo() {
   NO_VAR_FUNC(1, 2, 3);
-  VAR_FUNC(1, 2, 3); //expected-error{{implicit declaration of function 'func' 
is invalid in OpenCL}}
+  VAR_FUNC(1, 2, 3);
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2{{implicit declaration of function 'func' is invalid in 
OpenCL}}
+#else
+// expected-error@-4{{use of undeclared identifier 'func'}}
+#endif
   VAR_PRINTF("%i", 1);
 }



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


[PATCH] D84637: [AST] Enhance the const expression evaluator to support error-dependent exprs.

2020-08-12 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

friendly ping @rsmith, the patch is ready for  another round of review. Would 
be nice to have it in clang11.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84637

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


[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

2020-08-12 Thread Balogh , Ádám via Phabricator via cfe-commits
baloghadamsoftware added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp:73
+  // If the variable has an alias then it can be changed by that alias as well.
+  // FIXME: Track pointers and references.
+  if (hasPtrOrReferenceInFunc(Func, CondVar))

aaron.ballman wrote:
> This FIXME makes me worried that what we really want is a clang static 
> analyzer check, since the check is already flow sensitive and needs to be 
> data sensitive as well. Have you considered implementing this as a static 
> analyzer check?
Actually, it is only a proposal for improvement. Maybe FIXME is not the best 
wording for it. This is not a bug to fix, just a possibility to find more true 
positives. This check is very similar to the infinite loop check which is also 
in Tidy. In the Static Analyzer this check would be too heavyweight and would 
probably produce false positives.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc-redundant-condition.cpp:1097
+  }
+}

aaron.ballman wrote:
> Can you add some tests that exercise GNU expression statements, just to make 
> sure we get the behavior correct?
> ```
> if (({foo;})) {
> } else if (({foo;})) {
> }
> 
> if (foo) ({bar;});
> else if (foo) ({bar;});
> ```
> Another thing that's interesting to test is whether the redundant expression 
> is in a subexpression which doesn't contribute to the value of the control 
> expression:
> ```
> if (foo(), val) {
> } else if (foo(), other_val) {
> }
> ```
> (Similar can be tested with GNU statement expressions.)
Do you mean that I should handle these cases as well? (Detect a bug, provide a 
fix.)


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

https://reviews.llvm.org/D81272

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


[PATCH] D81865: [clang] Use string tables for static diagnostic descriptions

2020-08-12 Thread Nathan Froyd via Phabricator via cfe-commits
froydnj added a comment.

Ping for an opinion on the recent discussion of how to move forward with this 
patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81865

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


[PATCH] D85834: [AST] Fix a crash on calling getASTRecordLayout on an invalid RecordDecl.

2020-08-12 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/AST/ASTContext.cpp:2463
 
-unsigned PreferredAlign = static_cast(
-toBits(getASTRecordLayout(RT->getDecl()).PreferredAlignment));
-assert(PreferredAlign >= ABIAlign &&
-   "PreferredAlign should be at least as large as ABIAlign.");
-return PreferredAlign;
+if (!RT->getDecl()->isInvalidDecl()) {
+  unsigned PreferredAlign = static_cast(

hokein wrote:
> another proper fix might be change the `getPreferredTypeAlign` signature to 
> support error handling (e.g. return a bool), but that would require a large 
> invasive change for its callers, not sure it is worth.
> 
> I think the current implementation is OK, as the result doesn't really matter 
> for an invalid RecordDecl, and we do similar thing int `getTypeInfoImpl` 
> (returning a fake `Align`/`Width` for invalid decl).  
I think this is fine though it could be clearer/more explicit about how the 
special case is handled.

```
if (RT->getDecl()->isInvalidDecl())
  return ABIAlign;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85834

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


[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

2020-08-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:115
+// either due to crashes or false positives.
+const char *getClangTidyBlacklist() {
+  static const std::string FalsePositives =

njames93 wrote:
> Return by StringRef?
How about `getDisabledClangTidyChecks()` (or literally any other name than 
blacklist)?



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:120
+   // clangd doesn't replay those when using a preamble.
+   "-llvm-header-guard");
+  static const std::string CrashingChecks =

I suspect there are more checks that should be added here. For instance, much 
of `modernize-` is purely stylistic so it's easy to view as being false 
positives (like `modernize-use-trailing-return-types` or whatever it's called).



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:127
+   "-bugprone-use-after-move");
+  static const std::string BlackList =
+  llvm::join_items(", ", FalsePositives, CrashingChecks);

Similarly, rename this. I'd suggest `DisabledChecks`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83224

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


[clang] 25bbe23 - [analyzer] StdLibraryFunctionsChecker: Add support for new functions

2020-08-12 Thread Zurab Tsinadze via cfe-commits

Author: Zurab Tsinadze
Date: 2020-08-12T16:20:00+02:00
New Revision: 25bbe234e4e73e6345f4f0b61e680abf5a90d59f

URL: 
https://github.com/llvm/llvm-project/commit/25bbe234e4e73e6345f4f0b61e680abf5a90d59f
DIFF: 
https://github.com/llvm/llvm-project/commit/25bbe234e4e73e6345f4f0b61e680abf5a90d59f.diff

LOG: [analyzer] StdLibraryFunctionsChecker: Add support for new functions

`toupper`, `tolower`, `toascii` functions were added to
StdLibraryFunctionsChecker to fully cover CERT STR37-C rule:
https://wiki.sei.cmu.edu/confluence/x/BNcxBQ

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/std-c-library-functions-arg-constraints.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index e437c33bd81a..030f995739e3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -40,12 +40,12 @@
 //
 // The following standard C functions are currently supported:
 //
-//   fgetc  getline   isdigit   isupper
+//   fgetc  getline   isdigit   isupper toascii
 //   fread  isalnum   isgraph   isxdigit
 //   fwrite isalpha   islower   read
 //   getc   isascii   isprint   write
-//   getcharisblank   ispunct
-//   getdelim   iscntrl   isspace
+//   getcharisblank   ispunct   toupper
+//   getdelim   iscntrl   isspace   tolower
 //
 
//===--===//
 
@@ -147,9 +147,7 @@ class StdLibraryFunctionsChecker
 RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector )
 : ValueConstraint(ArgN), Kind(Kind), Args(Args) {}
 
-const IntRangeVector () const {
-  return Args;
-}
+const IntRangeVector () const { return Args; }
 
   private:
 ProgramStateRef applyAsOutOfRange(ProgramStateRef State,
@@ -158,6 +156,7 @@ class StdLibraryFunctionsChecker
 ProgramStateRef applyAsWithinRange(ProgramStateRef State,
const CallEvent ,
const Summary ) const;
+
   public:
 ProgramStateRef apply(ProgramStateRef State, const CallEvent ,
   const Summary ,
@@ -408,7 +407,7 @@ class StdLibraryFunctionsChecker
   return *this;
 }
 
-Summary (ConstraintSet&& CS) {
+Summary (ConstraintSet &) {
   CaseConstraints.push_back(std::move(CS));
   return *this;
 }
@@ -796,13 +795,13 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
   const QualType LongLongTy = ACtx.LongLongTy;
   const QualType SizeTy = ACtx.getSizeType();
 
-  const QualType VoidPtrTy = ACtx.VoidPtrTy; // void *
+  const QualType VoidPtrTy = ACtx.VoidPtrTy;// void *
   const QualType IntPtrTy = ACtx.getPointerType(IntTy); // int *
   const QualType UnsignedIntPtrTy =
   ACtx.getPointerType(UnsignedIntTy); // unsigned int *
   const QualType VoidPtrRestrictTy = getRestrictTy(VoidPtrTy);
   const QualType ConstVoidPtrTy =
-  ACtx.getPointerType(ACtx.VoidTy.withConst()); // const void *
+  ACtx.getPointerType(ACtx.VoidTy.withConst());// const void *
   const QualType CharPtrTy = ACtx.getPointerType(ACtx.CharTy); // char *
   const QualType CharPtrRestrictTy = getRestrictTy(CharPtrTy);
   const QualType ConstCharPtrTy =
@@ -1117,6 +1116,18 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
   .Case({ArgumentCondition(0U, OutOfRange,
{{'0', '9'}, {'A', 'F'}, {'a', 'f'}}),
  ReturnValueCondition(WithinRange, SingleValue(0))}));
+  addToFunctionSummaryMap(
+  "toupper", Summary(ArgTypes{IntTy}, RetType{IntTy}, EvalCallAsPure)
+ .ArgConstraint(ArgumentCondition(
+ 0U, WithinRange, {{EOFv, EOFv}, {0, 
UCharRangeMax}})));
+  addToFunctionSummaryMap(
+  "tolower", Summary(ArgTypes{IntTy}, RetType{IntTy}, EvalCallAsPure)
+ .ArgConstraint(ArgumentCondition(
+ 0U, WithinRange, {{EOFv, EOFv}, {0, 
UCharRangeMax}})));
+  addToFunctionSummaryMap(
+  "toascii", Summary(ArgTypes{IntTy}, RetType{IntTy}, EvalCallAsPure)
+ .ArgConstraint(ArgumentCondition(
+ 0U, WithinRange, {{EOFv, EOFv}, {0, 
UCharRangeMax}})));
 
   // The getc() family of functions that returns either a char or an EOF.
   if (FilePtrTy) {
@@ -2033,7 +2044,8 @@ void 
ento::registerStdCLibraryFunctionsChecker(CheckerManager ) {
   mgr.getAnalyzerOptions().getCheckerBooleanOption(Checker, "ModelPOSIX");
 }
 
-bool ento::shouldRegisterStdCLibraryFunctionsChecker(const CheckerManager 
) {
+bool 

[PATCH] D85093: [analyzer] StdLibraryFunctionsChecker: Add support for new functions

2020-08-12 Thread Zurab Tsinadze via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG25bbe234e4e7: [analyzer] StdLibraryFunctionsChecker: Add 
support for new functions (authored by zukatsinadze).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85093

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c

Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -45,7 +45,6 @@
   // bugpath-note{{TRUE}} \
   // bugpath-note{{Left side of '&&' is true}} \
   // bugpath-note{{'x' is <= 255}}
-
 }
 
 void test_alnum_symbolic2(int x) {
@@ -62,6 +61,114 @@
   }
 }
 
+int toupper(int);
+
+void test_toupper_concrete(int v) {
+  int ret = toupper(256); // \
+  // report-warning{{Function argument constraint is not satisfied}} \
+  // bugpath-warning{{Function argument constraint is not satisfied}} \
+  // bugpath-note{{Function argument constraint is not satisfied}}
+  (void)ret;
+}
+
+void test_toupper_symbolic(int x) {
+  int ret = toupper(x);
+  (void)ret;
+
+  clang_analyzer_eval(EOF <= x && x <= 255); // \
+  // report-warning{{TRUE}} \
+  // bugpath-warning{{TRUE}} \
+  // bugpath-note{{TRUE}} \
+  // bugpath-note{{Left side of '&&' is true}} \
+  // bugpath-note{{'x' is <= 255}}
+}
+
+void test_toupper_symbolic2(int x) {
+  if (x > 255) { // \
+// bugpath-note{{Assuming 'x' is > 255}} \
+// bugpath-note{{Taking true branch}}
+
+int ret = toupper(x); // \
+// report-warning{{Function argument constraint is not satisfied}} \
+// bugpath-warning{{Function argument constraint is not satisfied}} \
+// bugpath-note{{Function argument constraint is not satisfied}}
+
+(void)ret;
+  }
+}
+
+int tolower(int);
+
+void test_tolower_concrete(int v) {
+  int ret = tolower(256); // \
+  // report-warning{{Function argument constraint is not satisfied}} \
+  // bugpath-warning{{Function argument constraint is not satisfied}} \
+  // bugpath-note{{Function argument constraint is not satisfied}}
+  (void)ret;
+}
+
+void test_tolower_symbolic(int x) {
+  int ret = tolower(x);
+  (void)ret;
+
+  clang_analyzer_eval(EOF <= x && x <= 255); // \
+  // report-warning{{TRUE}} \
+  // bugpath-warning{{TRUE}} \
+  // bugpath-note{{TRUE}} \
+  // bugpath-note{{Left side of '&&' is true}} \
+  // bugpath-note{{'x' is <= 255}}
+}
+
+void test_tolower_symbolic2(int x) {
+  if (x > 255) { // \
+// bugpath-note{{Assuming 'x' is > 255}} \
+// bugpath-note{{Taking true branch}}
+
+int ret = tolower(x); // \
+// report-warning{{Function argument constraint is not satisfied}} \
+// bugpath-warning{{Function argument constraint is not satisfied}} \
+// bugpath-note{{Function argument constraint is not satisfied}}
+
+(void)ret;
+  }
+}
+
+int toascii(int);
+
+void test_toascii_concrete(int v) {
+  int ret = toascii(256); // \
+  // report-warning{{Function argument constraint is not satisfied}} \
+  // bugpath-warning{{Function argument constraint is not satisfied}} \
+  // bugpath-note{{Function argument constraint is not satisfied}}
+  (void)ret;
+}
+
+void test_toascii_symbolic(int x) {
+  int ret = toascii(x);
+  (void)ret;
+
+  clang_analyzer_eval(EOF <= x && x <= 255); // \
+  // report-warning{{TRUE}} \
+  // bugpath-warning{{TRUE}} \
+  // bugpath-note{{TRUE}} \
+  // bugpath-note{{Left side of '&&' is true}} \
+  // bugpath-note{{'x' is <= 255}}
+}
+
+void test_toascii_symbolic2(int x) {
+  if (x > 255) { // \
+// bugpath-note{{Assuming 'x' is > 255}} \
+// bugpath-note{{Taking true branch}}
+
+int ret = toascii(x); // \
+// report-warning{{Function argument constraint is not satisfied}} \
+// bugpath-warning{{Function argument constraint is not satisfied}} \
+// bugpath-note{{Function argument constraint is not satisfied}}
+
+(void)ret;
+  }
+}
+
 typedef struct FILE FILE;
 typedef typeof(sizeof(int)) size_t;
 size_t fread(void *restrict, size_t, size_t, FILE *restrict);
@@ -80,7 +187,7 @@
   // bugpath-note{{'buf' is not equal to null}}
 }
 void test_notnull_symbolic2(FILE *fp, int *buf) {
-  if (!buf) // bugpath-note{{Assuming 'buf' is null}} \
+  if (!buf)  // bugpath-note{{Assuming 'buf' is null}} \
 // bugpath-note{{Taking true branch}}
 fread(buf, sizeof(int), 10, fp); // \
 // report-warning{{Function argument constraint is not satisfied}} \
@@ -151,7 +258,7 @@
 }
 int __buf_size_arg_constraint_mul(const void *, size_t, size_t);
 void test_buf_size_concrete_with_multiplication() {
-  short buf[3]; // bugpath-note{{'buf' initialized here}}
+  short buf[3]; // 

[clang] ddbd21d - [OPENMP]Do not add TGT_OMP_TARGET_PARAM flag to non-captured mapped arguments.

2020-08-12 Thread Alexey Bataev via cfe-commits

Author: Alexey Bataev
Date: 2020-08-12T10:06:52-04:00
New Revision: ddbd21d288f6ff7d175f18ddee0ee6407626445a

URL: 
https://github.com/llvm/llvm-project/commit/ddbd21d288f6ff7d175f18ddee0ee6407626445a
DIFF: 
https://github.com/llvm/llvm-project/commit/ddbd21d288f6ff7d175f18ddee0ee6407626445a.diff

LOG: [OPENMP]Do not add TGT_OMP_TARGET_PARAM flag to non-captured mapped 
arguments.

If the arguments are mapped, but are actually not used in the target
region, the compiler still adds attribute TGT_OMP_TARGET_PARAM for such
arguments. It makes the libomptarget to add such parameters to the list
of arguments, passed to the kernel at the runtime, and may lead to
incorrect results/crashes during execution.

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

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/test/OpenMP/target_map_codegen_18.inc
clang/test/OpenMP/target_map_codegen_20.cpp
clang/test/OpenMP/target_map_codegen_31.cpp
clang/test/OpenMP/target_map_codegen_32.cpp
clang/test/OpenMP/target_teams_map_codegen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 0582de5ebafe..19c703623d27 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7911,7 +7911,8 @@ class MappableExprsHandler {
   /// individual struct members.
   void emitCombinedEntry(MapCombinedInfoTy ,
  MapFlagsArrayTy ,
- const StructRangeInfoTy ) const {
+ const StructRangeInfoTy ,
+ bool NotTargetParams = false) const {
 // Base is the base of the struct
 CombinedInfo.BasePointers.push_back(PartialStruct.Base.getPointer());
 // Pointer is the address of the lowest element
@@ -7928,8 +7929,9 @@ class MappableExprsHandler {
 llvm::Value *Size = CGF.Builder.CreateIntCast(Diff, CGF.Int64Ty,
   /*isSigned=*/false);
 CombinedInfo.Sizes.push_back(Size);
-// Map type is always TARGET_PARAM
-CombinedInfo.Types.push_back(OMP_MAP_TARGET_PARAM);
+// Map type is always TARGET_PARAM, if generate info for captures.
+CombinedInfo.Types.push_back(NotTargetParams ? OMP_MAP_NONE
+ : OMP_MAP_TARGET_PARAM);
 // If any element has the present modifier, then make sure the runtime
 // doesn't attempt to allocate the struct.
 if (CurTypes.end() !=
@@ -7955,7 +7957,7 @@ class MappableExprsHandler {
   /// pair of the relevant declaration and index where it occurs is appended to
   /// the device pointers info array.
   void generateAllInfo(
-  MapCombinedInfoTy ,
+  MapCombinedInfoTy , bool NotTargetParams = false,
   const llvm::DenseSet>  =
   llvm::DenseSet>()) const {
 // We have to process the component lists that relate with the same
@@ -8078,8 +8080,9 @@ class MappableExprsHandler {
   UseDevicePtrCombinedInfo.Pointers.push_back(Ptr);
   UseDevicePtrCombinedInfo.Sizes.push_back(
   llvm::Constant::getNullValue(CGF.Int64Ty));
-  UseDevicePtrCombinedInfo.Types.push_back(OMP_MAP_RETURN_PARAM |
-   OMP_MAP_TARGET_PARAM);
+  UseDevicePtrCombinedInfo.Types.push_back(
+  OMP_MAP_RETURN_PARAM |
+  (NotTargetParams ? OMP_MAP_NONE : OMP_MAP_TARGET_PARAM));
   UseDevicePtrCombinedInfo.Mappers.push_back(nullptr);
 }
   }
@@ -8144,8 +8147,11 @@ class MappableExprsHandler {
 Ptr = CGF.EmitScalarExpr(IE);
   CombinedInfo.BasePointers.emplace_back(Ptr, VD);
   CombinedInfo.Pointers.push_back(Ptr);
-  
CombinedInfo.Sizes.push_back(llvm::Constant::getNullValue(CGF.Int64Ty));
-  CombinedInfo.Types.push_back(OMP_MAP_RETURN_PARAM | 
OMP_MAP_TARGET_PARAM);
+  CombinedInfo.Sizes.push_back(
+  llvm::Constant::getNullValue(CGF.Int64Ty));
+  CombinedInfo.Types.push_back(
+  OMP_MAP_RETURN_PARAM |
+  (NotTargetParams ? OMP_MAP_NONE : OMP_MAP_TARGET_PARAM));
   CombinedInfo.Mappers.push_back(nullptr);
 }
   }
@@ -8154,7 +8160,7 @@ class MappableExprsHandler {
 for (const auto  : Info) {
   // We need to know when we generate information for the first component
   // associated with a capture, because the mapping flags depend on it.
-  bool IsFirstComponentList = true;
+  bool IsFirstComponentList = !NotTargetParams;
 
   // Temporary generated information.
   MapCombinedInfoTy CurInfo;
@@ -8227,7 +8233,8 @@ class MappableExprsHandler {
   // If there is an entry in PartialStruct it means we have a struct with
   // individual members mapped. Emit an extra combined entry.
   if 

[PATCH] D85755: [OPENMP]Do not add TGT_OMP_TARGET_PARAM flag to non-captured mapped arguments.

2020-08-12 Thread Alexey Bataev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGddbd21d288f6: [OPENMP]Do not add TGT_OMP_TARGET_PARAM flag 
to non-captured mapped arguments. (authored by ABataev).

Changed prior to commit:
  https://reviews.llvm.org/D85755?vs=284806=285074#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85755

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/target_map_codegen_18.inc
  clang/test/OpenMP/target_map_codegen_20.cpp
  clang/test/OpenMP/target_map_codegen_31.cpp
  clang/test/OpenMP/target_map_codegen_32.cpp
  clang/test/OpenMP/target_teams_map_codegen.cpp

Index: clang/test/OpenMP/target_teams_map_codegen.cpp
===
--- clang/test/OpenMP/target_teams_map_codegen.cpp
+++ clang/test/OpenMP/target_teams_map_codegen.cpp
@@ -20,16 +20,16 @@
 #ifndef HEADER
 #define HEADER
 
-// HOST: @[[MAPTYPES_PRIVATE:.offload_maptypes[0-9.]*]] = private {{.*}}constant [2 x i64] [i64 35, i64 35]
+// HOST: @[[MAPTYPES_PRIVATE:.offload_maptypes[0-9.]*]] = private {{.*}}constant [2 x i64] [i64 3, i64 3]
 // HOST: @[[MAPTYPES_FIRSTPRIVATE:.offload_maptypes[0-9.]*]] = private {{.*}}constant [2 x i64] [i64 35, i64 35]
 // HOST: @[[MAPTYPES_REDUCTION:.offload_maptypes[0-9.]*]] = private {{.*}}constant [2 x i64] [i64 35, i64 35]
 // HOST: @[[MAPTYPES_FROM:.offload_maptypes[0-9.]*]] = private {{.*}}constant [1 x i64] [i64 34]
 // HOST: @[[MAPTYPES_TO:.offload_maptypes[0-9.]*]] = private {{.*}}constant [1 x i64] [i64 33]
 // HOST: @[[MAPTYPES_ALLOC:.offload_maptypes[0-9.]*]] = private {{.*}}constant [1 x i64] [i64 32]
-// HOST: @[[MAPTYPES_ARRAY_R0:.offload_maptypes[0-9.]*]] = private {{.*}}constant [3 x i64] [i64 35, i64 35, i64 35]
-// HOST: @[[MAPTYPES_ARRAY_R1:.offload_maptypes[0-9.]*]] = private {{.*}}constant [3 x i64] [i64 33, i64 33, i64 33]
-// HOST-INT128: @[[MAPTYPES_INT128_R0:.offload_maptypes[0-9.]*]] = private {{.*}}constant [3 x i64] [i64 35, i64 35, i64 35]
-// HOST-INT128: @[[MAPTYPES_INT128_R1:.offload_maptypes[0-9.]*]] = private {{.*}}constant [3 x i64] [i64 34, i64 34, i64 34]
+// HOST: @[[MAPTYPES_ARRAY_R0:.offload_maptypes[0-9.]*]] = private {{.*}}constant [3 x i64] [i64 35, i64 35, i64 3]
+// HOST: @[[MAPTYPES_ARRAY_R1:.offload_maptypes[0-9.]*]] = private {{.*}}constant [3 x i64] [i64 33, i64 33, i64 1]
+// HOST-INT128: @[[MAPTYPES_INT128_R0:.offload_maptypes[0-9.]*]] = private {{.*}}constant [3 x i64] [i64 35, i64 35, i64 3]
+// HOST-INT128: @[[MAPTYPES_INT128_R1:.offload_maptypes[0-9.]*]] = private {{.*}}constant [3 x i64] [i64 34, i64 34, i64 2]
 //
 // CHECK: @.omp_offloading.entry_name{{[0-9.]*}} = {{.*}} c"[[OFFLOAD_PRIVATE:__omp_offloading_[^"\\]*mapWithPrivate[^"\\]*]]\00"
 // CHECK: @.omp_offloading.entry_name{{[0-9.]*}} = {{.*}} c"[[OFFLOAD_FIRSTPRIVATE:__omp_offloading_[^"\\]*mapWithFirstprivate[^"\\]*]]\00"
Index: clang/test/OpenMP/target_map_codegen_32.cpp
===
--- clang/test/OpenMP/target_map_codegen_32.cpp
+++ clang/test/OpenMP/target_map_codegen_32.cpp
@@ -40,8 +40,10 @@
 // MEMBER_OF_1=0x1 | PRESENT=0x1000 | FROM=0x2 | TO=0x1 = 0x11003
 
 // CK31B-LABEL: @.__omp_offloading_{{.*}}test_present_members{{.*}}_l{{[0-9]+}}.region_id = weak constant i8 0
-// CK31B: [[MTYPE00:@.+]] = private {{.*}}constant [3 x i64] [i64 [[#0x1020]],
-// CK31B-SAME: {{^}} i64 [[#0x10003]], i64 [[#0x11003]]]
+// CK31B-USE: [[MTYPE00:@.+]] = private {{.*}}constant [3 x i64] [i64 [[#0x1020]],
+// CK31B-NOUSE: [[MTYPE00:@.+]] = private {{.*}}constant [3 x i64] [i64 [[#0x1000]],
+// CK31B-USE-SAME: {{^}} i64 [[#0x10003]], i64 [[#0x11003]]]
+// CK31B-NOUSE-SAME: {{^}} i64 [[#0x10003]], i64 [[#0x11003]]]
 
 struct ST {
   int i;
Index: clang/test/OpenMP/target_map_codegen_31.cpp
===
--- clang/test/OpenMP/target_map_codegen_31.cpp
+++ clang/test/OpenMP/target_map_codegen_31.cpp
@@ -42,23 +42,27 @@
 // CK31A-LABEL: @.__omp_offloading_{{.*}}explicit_maps_single{{.*}}_l{{[0-9]+}}.region_id = weak constant i8 0
 //
 // PRESENT=0x1000 | TARGET_PARAM=0x20 = 0x1020
-// CK31A: [[MTYPE00:@.+]] = private {{.*}}constant [7 x i64] [i64 [[#0x1020]],
+// CK31A-USE: [[MTYPE00:@.+]] = private {{.*}}constant [7 x i64] [i64 [[#0x1020]],
+// CK31A-NOUSE: [[MTYPE00:@.+]] = private {{.*}}constant [7 x i64] [i64 [[#0x1000]],
 //
 // MEMBER_OF_1=0x1 | FROM=0x2 | TO=0x1 = 0x10003
 // MEMBER_OF_1=0x1 | PRESENT=0x1000 | FROM=0x2 | TO=0x1 = 0x11003
 // PRESENT=0x1000 | TARGET_PARAM=0x20 | FROM=0x2 | TO=0x1 = 0x1023
-// CK31A-SAME: {{^}} i64 [[#0x10003]], i64 [[#0x11003]], i64 [[#0x1023]],
+// CK31A-USE-SAME: {{^}} i64 [[#0x10003]], i64 [[#0x11003]], i64 [[#0x1023]],
+// CK31A-NOUSE-SAME: 

[PATCH] D84886: Create LoopNestPass

2020-08-12 Thread Whitney Tsang via Phabricator via cfe-commits
Whitney added a comment.

Examples of existing transformations that can be benefited from LoopNestPass:  
LoopInterchange, LoopUnrollAndJam, LoopFusion, LoopIdiom, LoopDistribution, ...
And their corresponding cost model analyses can be benefited from 
LoopNestAnalysis, e.g. LoopCacheCost.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84886

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


[PATCH] D85819: [SyntaxTree] Split tests

2020-08-12 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas added inline comments.



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1005
+
+TEST_P(SyntaxTreeTest, QualifiedIdWithNamespace) {
+  if (!GetParam().isCXX()) {

Here I didn't merely split the tests, I also changed them a bit. PTAL.




Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1546-1548
+unsigned operator "" _r(const char*);
+template 
+unsigned operator "" _t();

Same setup for `FloatUserDefinedLiteral`. I think it is justified with the 
Annotations we can constraint the dump just to the expressions being tested



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:635
 
-TEST_P(SyntaxTreeTest, UnqualifiedId) {
+TEST_P(SyntaxTreeTest, UnqualifiedIdIdentifier) {
+  EXPECT_TRUE(treeDumpEqual(

eduucaldas wrote:
> It seems inappropriate to always repeat `UnqualfiiedId`, do you have any 
> other suggestion?
Last commit change names to `TestSuite_TestCase`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85819

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


[PATCH] D85819: [SyntaxTree] Split tests

2020-08-12 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 285071.
eduucaldas added a comment.

Rename tests following `TestSuite_TestCase`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85819

Files:
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -632,26 +632,48 @@
 )txt"));
 }
 
-TEST_P(SyntaxTreeTest, UnqualifiedId) {
+TEST_P(SyntaxTreeTest, UnqualifiedId_Identifier) {
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+void test(int a) {
+  a;
+}
+)cpp",
+  R"txt(
+*: TranslationUnit
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   `-a
+  |   `-)
+  `-CompoundStatement
+|-{
+|-ExpressionStatement
+| |-IdExpression
+| | `-UnqualifiedId
+| |   `-a
+| `-;
+`-}
+)txt"));
+}
+
+TEST_P(SyntaxTreeTest, UnqualifiedId_OperatorFunctionId) {
   if (!GetParam().isCXX()) {
 return;
   }
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
 struct X {
-  // TODO: Expose `id-expression` from `Declarator`
   friend X operator+(const X&, const X&);
-  operator int();
 };
-template
-void f(T&);
 void test(X x) {
-  x;  // identifier
-  operator+(x, x);// operator-function-id
-  f(x);// template-id
-  // TODO: Expose `id-expression` from `MemberExpr`
-  x.operator int();   // conversion-funtion-id
-  x.~X(); // ~type-name
+  operator+(x, x);
 }
 )cpp",
   R"txt(
@@ -682,35 +704,8 @@
 | |   |   |   `-&
 | |   |   `-)
 | |   `-;
-| |-SimpleDeclaration
-| | |-SimpleDeclarator
-| | | |-operator
-| | | |-int
-| | | `-ParametersAndQualifiers
-| | |   |-(
-| | |   `-)
-| | `-;
 | |-}
 | `-;
-|-TemplateDeclaration
-| |-template
-| |-<
-| |-UnknownDeclaration
-| | |-typename
-| | `-T
-| |->
-| `-SimpleDeclaration
-|   |-void
-|   |-SimpleDeclarator
-|   | |-f
-|   | `-ParametersAndQualifiers
-|   |   |-(
-|   |   |-SimpleDeclaration
-|   |   | |-T
-|   |   | `-SimpleDeclarator
-|   |   |   `-&
-|   |   `-)
-|   `-;
 `-SimpleDeclaration
   |-void
   |-SimpleDeclarator
@@ -725,11 +720,6 @@
   `-CompoundStatement
 |-{
 |-ExpressionStatement
-| |-IdExpression
-| | `-UnqualifiedId
-| |   `-x
-| `-;
-|-ExpressionStatement
 | |-UnknownExpression
 | | |-IdExpression
 | | | `-UnqualifiedId
@@ -745,20 +735,53 @@
 | | |   `-x
 | | `-)
 | `-;
-|-ExpressionStatement
-| |-UnknownExpression
-| | |-IdExpression
-| | | `-UnqualifiedId
-| | |   |-f
-| | |   |-<
-| | |   |-X
-| | |   `->
-| | |-(
-| | |-IdExpression
-| | | `-UnqualifiedId
-| | |   `-x
-| | `-)
-| `-;
+`-}
+)txt"));
+}
+
+TEST_P(SyntaxTreeTest, UnqualifiedId_ConversionFunctionId) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+struct X {
+  operator int();
+};
+void test(X x) {
+  // TODO: Expose `id-expression` from `MemberExpr`
+  x.operator int();
+}
+)cpp",
+  R"txt(
+*: TranslationUnit
+|-SimpleDeclaration
+| |-struct
+| |-X
+| |-{
+| |-SimpleDeclaration
+| | |-SimpleDeclarator
+| | | |-operator
+| | | |-int
+| | | `-ParametersAndQualifiers
+| | |   |-(
+| | |   `-)
+| | `-;
+| |-}
+| `-;
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-X
+  |   | `-SimpleDeclarator
+  |   |   `-x
+  |   `-)
+  `-CompoundStatement
+|-{
 |-ExpressionStatement
 | |-UnknownExpression
 | | |-UnknownExpression
@@ -771,6 +794,93 @@
 | | |-(
 | | `-)
 | `-;
+`-}
+)txt"));
+}
+
+TEST_P(SyntaxTreeTest, UnqualifiedId_LiteralOperatorId) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+unsigned operator "" _w(char);
+void test() {
+  operator "" _w('1');
+}
+)cpp",
+  R"txt(
+*: TranslationUnit
+|-SimpleDeclaration
+| |-unsigned
+| |-SimpleDeclarator
+| | |-operator
+| | |-""
+| | |-_w
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   |-SimpleDeclaration
+| |   | `-char
+| |   `-)
+| `-;
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
+  `-CompoundStatement
+|-{
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | `-UnqualifiedId
+| | |   |-operator
+| | |   |-""
+| | |   `-_w
+| | |-(
+| | |-CharacterLiteralExpression
+| | | `-'1'
+| | `-)
+| `-;
+`-}
+)txt"));
+}
+
+TEST_P(SyntaxTreeTest, UnqualifiedId_Destructor) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+struct 

[PATCH] D85819: [SyntaxTree] Split tests

2020-08-12 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 285070.
eduucaldas added a comment.

- [SyntaxTree] Split tests for `QualifiedId`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85819

Files:
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -632,26 +632,48 @@
 )txt"));
 }
 
-TEST_P(SyntaxTreeTest, UnqualifiedId) {
+TEST_P(SyntaxTreeTest, UnqualifiedIdIdentifier) {
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+void test(int a) {
+  a;
+}
+)cpp",
+  R"txt(
+*: TranslationUnit
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   `-a
+  |   `-)
+  `-CompoundStatement
+|-{
+|-ExpressionStatement
+| |-IdExpression
+| | `-UnqualifiedId
+| |   `-a
+| `-;
+`-}
+)txt"));
+}
+
+TEST_P(SyntaxTreeTest, UnqualifiedIdOperatorFunctionId) {
   if (!GetParam().isCXX()) {
 return;
   }
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
 struct X {
-  // TODO: Expose `id-expression` from `Declarator`
   friend X operator+(const X&, const X&);
-  operator int();
 };
-template
-void f(T&);
 void test(X x) {
-  x;  // identifier
-  operator+(x, x);// operator-function-id
-  f(x);// template-id
-  // TODO: Expose `id-expression` from `MemberExpr`
-  x.operator int();   // conversion-funtion-id
-  x.~X(); // ~type-name
+  operator+(x, x);
 }
 )cpp",
   R"txt(
@@ -682,35 +704,8 @@
 | |   |   |   `-&
 | |   |   `-)
 | |   `-;
-| |-SimpleDeclaration
-| | |-SimpleDeclarator
-| | | |-operator
-| | | |-int
-| | | `-ParametersAndQualifiers
-| | |   |-(
-| | |   `-)
-| | `-;
 | |-}
 | `-;
-|-TemplateDeclaration
-| |-template
-| |-<
-| |-UnknownDeclaration
-| | |-typename
-| | `-T
-| |->
-| `-SimpleDeclaration
-|   |-void
-|   |-SimpleDeclarator
-|   | |-f
-|   | `-ParametersAndQualifiers
-|   |   |-(
-|   |   |-SimpleDeclaration
-|   |   | |-T
-|   |   | `-SimpleDeclarator
-|   |   |   `-&
-|   |   `-)
-|   `-;
 `-SimpleDeclaration
   |-void
   |-SimpleDeclarator
@@ -725,11 +720,6 @@
   `-CompoundStatement
 |-{
 |-ExpressionStatement
-| |-IdExpression
-| | `-UnqualifiedId
-| |   `-x
-| `-;
-|-ExpressionStatement
 | |-UnknownExpression
 | | |-IdExpression
 | | | `-UnqualifiedId
@@ -745,20 +735,53 @@
 | | |   `-x
 | | `-)
 | `-;
-|-ExpressionStatement
-| |-UnknownExpression
-| | |-IdExpression
-| | | `-UnqualifiedId
-| | |   |-f
-| | |   |-<
-| | |   |-X
-| | |   `->
-| | |-(
-| | |-IdExpression
-| | | `-UnqualifiedId
-| | |   `-x
-| | `-)
-| `-;
+`-}
+)txt"));
+}
+
+TEST_P(SyntaxTreeTest, UnqualifiedIdConversionFunctionId) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+struct X {
+  operator int();
+};
+void test(X x) {
+  // TODO: Expose `id-expression` from `MemberExpr`
+  x.operator int();
+}
+)cpp",
+  R"txt(
+*: TranslationUnit
+|-SimpleDeclaration
+| |-struct
+| |-X
+| |-{
+| |-SimpleDeclaration
+| | |-SimpleDeclarator
+| | | |-operator
+| | | |-int
+| | | `-ParametersAndQualifiers
+| | |   |-(
+| | |   `-)
+| | `-;
+| |-}
+| `-;
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-X
+  |   | `-SimpleDeclarator
+  |   |   `-x
+  |   `-)
+  `-CompoundStatement
+|-{
 |-ExpressionStatement
 | |-UnknownExpression
 | | |-UnknownExpression
@@ -771,6 +794,93 @@
 | | |-(
 | | `-)
 | `-;
+`-}
+)txt"));
+}
+
+TEST_P(SyntaxTreeTest, UnqualifiedIdLiteralOperatorId) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+unsigned operator "" _w(char);
+void test() {
+  operator "" _w('1');
+}
+)cpp",
+  R"txt(
+*: TranslationUnit
+|-SimpleDeclaration
+| |-unsigned
+| |-SimpleDeclarator
+| | |-operator
+| | |-""
+| | |-_w
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   |-SimpleDeclaration
+| |   | `-char
+| |   `-)
+| `-;
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
+  `-CompoundStatement
+|-{
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | `-UnqualifiedId
+| | |   |-operator
+| | |   |-""
+| | |   `-_w
+| | |-(
+| | |-CharacterLiteralExpression
+| | | `-'1'
+| | `-)
+| `-;
+`-}
+)txt"));
+}
+
+TEST_P(SyntaxTreeTest, UnqualifiedIdDestructor) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+struct X { 

  1   2   >