date:20180415

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

2018-04-15 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added a comment.

In https://reviews.llvm.org/D41648#1047799, @JonasToth wrote:

> I checked several codebases and implemented a little helper script to see 
> which kind of macros exist. Then I added some filter regex as configuration 
> and tried again, here are the results:
>
> For https://github.com/nlohmann/json which is a very high quality codebase, 
> the results were as expected: very clean, and good macro usage: (except a 
> `#define private public` for tests)
>
> Filter: `JSON*|NLOHMANN*`
>  F5913499: all_macros.txt 
>
> F5913498: filtered_macros.txt 
>
> CMake on the other hand uses many macros and don't obey CAPS_ONLY rules for 
> all macros.
>
> Filter:  
> `_FILE_OFFSET_BITS|AE_*|ARCHIVE_*|cal_*|CMAKE_*|cm*|CM_*|CPP_*|CURL*|E_*|exp_*|F90*|jp*|JSON_*|KW*|kw*|O_*|REQ_*|UV_*|YY*|yy*|Z_*`
>
> F5913502: warned_macros.txt 
>
> F5913501: filtered_macros.txt 
>
> OpenCV isn't clean either, here the results:
>
> Filter: 
> `ASSERT*|ALL*|CAL*|CC*|CALC*|calc*|CL*|cl*|CUDA*|CV*|cv*|EXPECT*|GTEST*|FUNCTOR*|HAVE*|ICV*|IPL*|IPP*|ipp*|__itt*|ITT*|JAS*|jas*|MESSAGE*|MAX*|OCL*|opengl*|OPENCV*|TYP*`
>
> F5913504: all_macros.txt 
>
> F5913503: filtered_modules_macros.txt 
>
> libtorrent https://github.com/arvidn/libtorrent has reasonable macro usage
>
> Filter: `DEBUG*|LIBTORRENT*|TORRENT*|UNI*`
>
> F5913519: filtered_macros.txt 
>
> F5913518: all_macros.txt 
>
> LLVM lib/ defines many macros, almost all of them are ALL_CAPS
>
> Filter: 
> `AARCH64*|X86*|ARM*|AMDGPU*|ASAN*|ATTR*|CHECK*|COMMON*|CV*|CXX*|DEBUG*|DEC*|DEFINE*|ESAN*|ENUM*|FMA*|FUZZER*|HANDLE*|INTERCEPTOR*|LSAN*|MATH*|MSAN*|OPENMP*|TSAN*|TYPE*|UBSAN*`
>
> F5913505: all_lib_macros.txt 
>  F5913528: filtered_lib_macros.txt 
>
> LLVM tools/ is a similar story
>  Filter: 
> `ANALYSIS*|ASSERT*|CHECK*|DEBUG*|DECL*|DEPENDENT*|DIAG*|END*|ENUM*|GENERIC*|IMAGE*|KEYWORD*|LANG*|LLVM*|NON*|OBJC*|OPENMP*|OVERLOAD*|PROC*|REGISTER*|SANITIZER*|STMT*|TYPE*|X86*`
>
> F5913566: all_tools_macros.txt 
>
> F5913565: filtered_tools_macros.txt 
>
> @aaron.ballman Would you like to see other projects checked? I think this is 
> a starting point for now.
>
> My opinion based on what i see is
>
> - maybe two modes for this check make sense, having one requiring CAPS_ONLY 
> and the currently implemented stricter check. This allows easier migration to 
> safer macro usage
> - it is possible to reduce macro usage to a minimal amount, and the complex 
> macros like AST stuff can be filtered with the regex. Furthermore, 
> restricting all macros to a "macro namespace" is possible for sure.
>
>   Things I would like to add to the check:
> - my little filtering script is valuable for developers, that want to address 
> the macro issue. It should be added to the docs and everyone can make 
> something based on that. It will be linux centered.
> - enforcing ALL_CAPS, including its usage
> - (adding a prefix to all macro, passing the filter, including its usage)
>
>   Code transformation has the problems of scope and potentially breaking code 
> badly, because clang-tidy wasn't run over all of the code.
>
> The check is chatty, as expected, because macros in header files, especially 
> central ones, pollute everything. That is the reason for the check, too. 
>  Overall I am still in favor of this approach, seeing some obscure macros 
> that should be replaced with actual language features (like overloading, 
> .inline functions, constants, ...) in the checked codebases.


@aaron.ballman Did you have time to look at my analysis result?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648



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

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

2018-04-15 Thread Jonas Toth via Phabricator via cfe-commits

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

- [Misc] unify handle and value modification detection
- [Misc] found new caveats, maybe take a look at refactoring first an require 
this check to produce clean compiles on llvm


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/ConstCheck.cpp
  clang-tidy/cppcoreguidelines/ConstCheck.h
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-const.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-const-handles.cpp
  test/clang-tidy/cppcoreguidelines-const-values.cpp

Index: test/clang-tidy/cppcoreguidelines-const-values.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-const-values.cpp
@@ -0,0 +1,416 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-const %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: "cppcoreguidelines-const.AnalyzeValues", value: 1},\
+// RUN:   {key: "cppcoreguidelines-const.AnalyzeHandles", value: 0},\
+// RUN:   {key: "cppcoreguidelines-const.WarnPointersAsValues", value: 1}]}' \
+// RUN: --
+
+// --- Provide test samples for primitive builtins -
+// - every 'p_*' variable is a 'potential_const_*' variable
+// - every 'np_*' variable is a 'non_potential_const_*' variable
+
+bool global;
+char np_global = 0; // globals can't be known to be const
+
+namespace foo {
+int scoped;
+float np_scoped = 1; // namespace variables are like globals
+} // namespace foo
+
+void some_function(double, wchar_t);
+
+void some_function(double np_arg0, wchar_t np_arg1) {
+  int p_local0 = 2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+
+  int np_local0;
+  const int np_local1 = 42;
+
+  unsigned int np_local2 = 3;
+  np_local2 <<= 4;
+
+  int np_local3 = 4;
+  ++np_local3;
+  int np_local4 = 4;
+  np_local4++;
+
+  int np_local5 = 4;
+  --np_local5;
+  int np_local6 = 4;
+  np_local6--;
+}
+
+void some_lambda_environment_capture_all_by_reference(double np_arg0) {
+  int np_local0 = 0;
+  int p_local0 = 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+
+  int np_local2;
+  const int np_local3 = 2;
+
+  // Capturing all variables by reference prohibits making them const.
+  [&]() { ++np_local0; };
+
+  int p_local1 = 0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const
+}
+
+void some_lambda_environment_capture_all_by_value(double np_arg0) {
+  int np_local0 = 0;
+  int p_local0 = 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+
+  int np_local1;
+  const int np_local2 = 2;
+
+  // Capturing by value has no influence on them.
+  [=]() { (void)p_local0; };
+
+  np_local0 += 10;
+}
+
+void function_inout_pointer(int *inout);
+void function_in_pointer(const int *in);
+
+void some_pointer_taking(int *out) {
+  int np_local0 = 42;
+  const int *const p0_np_local0 = &np_local0;
+  int *const p1_np_local0 = &np_local0;
+
+  int np_local1 = 42;
+  function_inout_pointer(&np_local1);
+
+  // Prevents const.
+  int np_local2 = 42;
+  out = &np_local2; // This returns and invalid address, its just about the AST
+
+  int p_local0 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+  const int *const p0_p_local0 = &p_local0;
+
+  int p_local1 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const
+  function_in_pointer(&p_local1);
+}
+
+void function_inout_ref(int &inout);
+void function_in_ref(const int &in);
+
+void some_reference_taking() {
+  int np_local0 = 42;
+  const int &r0_np_local0 = np_local0;
+  int &r1_np_local0 = np_local0;
+  const int &r2_np_local0 = r1_np_local0;
+
+  int np_local1 = 42;
+  function_inout_ref(np_local1);
+
+  int p_local0 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+  const int &r0_p_local0 = p_local0;
+
+  int p_local1 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const
+  function_in_ref(p_local1);
+}
+
+double *non_const_pointer_return() {
+  double p_local0 = 0.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const
+  double np_local0 = 24.4;
+
+  return &np_local0;
+}
+
+const double *const_pointer_return() {
+  double p_local0 = 0.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const
+  double p_local1 = 24.4;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'double' can be declared const
+  return &p_local1;
+}
+
+double &non_cons

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

2018-04-15 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added a comment.

@aaron.ballman @lebedev.ri The check is getting really complex. I run it over 
LLVM and detected some warnings, where iam not sure if they are valid or not. 
Its already a somewhat usable state, but its hard to determine false positives 
and false negatives.

For me, that false positives are worse for now, because they annoy. Its easier 
to add false negatives especially with user feedback. My idea is now: Take a 
look at refactoring the code to introduce const as fixit and iterate the check 
until LLVM compiles after the check fixed all missing consts. What do you think 
about that strategy? And could you take a look at the todo list at the top of 
`ConstCheck.cpp`. Did I miss something?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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

[PATCH] D45665: [builtin-dump-struct] printf format checking

2018-04-15 Thread Paul Semel via Phabricator via cfe-commits

paulsemel created this revision.
paulsemel added a reviewer: aaron.ballman.

Added check for correct formats in CodeGen tests, to ensure we are printing the 
fields correctly.
Added a fix that outcomed from this previous add


Repository:
  rC Clang

https://reviews.llvm.org/D45665

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/dump-struct-builtin.c

Index: test/CodeGen/dump-struct-builtin.c
===
--- test/CodeGen/dump-struct-builtin.c
+++ test/CodeGen/dump-struct-builtin.c
@@ -2,6 +2,98 @@
 
 #include "Inputs/stdio.h"
 
+// CHECK: @unit1.a = private unnamed_addr constant %struct.U1A { i16 12 }, align 2
+// CHECK-NEXT: [[STRUCT_STR_U1:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U1A {\0A\00"
+// CHECK-NEXT: [[FIELD_U1:@[0-9]+]] = private unnamed_addr constant [11 x i8] c"short a : \00"
+// CHECK-NEXT: [[FORMAT_U1:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%hd\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U1:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit2.a = private unnamed_addr constant %struct.U2A { i16 12 }, align 2
+// CHECK-NEXT: [[STRUCT_STR_U2:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U2A {\0A\00"
+// CHECK-NEXT: [[FIELD_U2:@[0-9]+]] = private unnamed_addr constant [20 x i8] c"unsigned short a : \00"
+// CHECK-NEXT: [[FORMAT_U2:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%hu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U2:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit3.a = private unnamed_addr constant %struct.U3A { i32 12 }, align 4
+// CHECK-NEXT: [[STRUCT_STR_U3:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U3A {\0A\00"
+// CHECK-NEXT: [[FIELD_U3:@[0-9]+]] = private unnamed_addr constant [9 x i8] c"int a : \00"
+// CHECK-NEXT: [[FORMAT_U3:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%d\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U3:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit4.a = private unnamed_addr constant %struct.U4A { i32 12 }, align 4
+// CHECK-NEXT: [[STRUCT_STR_U4:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U4A {\0A\00"
+// CHECK-NEXT: [[FIELD_U4:@[0-9]+]] = private unnamed_addr constant [18 x i8] c"unsigned int a : \00"
+// CHECK-NEXT: [[FORMAT_U4:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%u\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U4:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit5.a = private unnamed_addr constant %struct.U5A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U5:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U5A {\0A\00"
+// CHECK-NEXT: [[FIELD_U5:@[0-9]+]] = private unnamed_addr constant [10 x i8] c"long a : \00"
+// CHECK-NEXT: [[FORMAT_U5:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%ld\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U5:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit6.a = private unnamed_addr constant %struct.U6A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U6:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U6A {\0A\00"
+// CHECK-NEXT: [[FIELD_U6:@[0-9]+]] = private unnamed_addr constant [19 x i8] c"unsigned long a : \00"
+// CHECK-NEXT: [[FORMAT_U6:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%lu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U6:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit7.a = private unnamed_addr constant %struct.U7A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U7:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U7A {\0A\00"
+// CHECK-NEXT: [[FIELD_U7:@[0-9]+]] = private unnamed_addr constant [15 x i8] c"long long a : \00"
+// CHECK-NEXT: [[FORMAT_U7:@[0-9]+]] = private unnamed_addr constant [6 x i8] c"%lld\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U7:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit8.a = private unnamed_addr constant %struct.U8A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U8:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U8A {\0A\00"
+// CHECK-NEXT: [[FIELD_U8:@[0-9]+]] = private unnamed_addr constant [24 x i8] c"unsigned long long a : \00"
+// CHECK-NEXT: [[FORMAT_U8:@[0-9]+]] = private unnamed_addr constant [6 x i8] c"%llu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U8:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit9.a = private unnamed_addr constant %struct.U9A { i8 97 }, align 1
+// CHECK-NEXT: [[STRUCT_STR_U9:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U9A {\0A\00"
+// CHECK-NEXT: [[FIELD_U9:@[0-9]+]] = private unnamed_addr constant [10 x i8] c"char a : \00"
+// CHECK-NEXT: [[FORMAT_U9:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%c\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U9:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @.str = private unnamed_addr constant [4 x i8] c"LSE\00", align 1
+
+// CHECK: @unit10.a = priv

[PATCH] D45665: [builtin-dump-struct] printf format checking

2018-04-15 Thread Paul Semel via Phabricator via cfe-commits

paulsemel updated this revision to Diff 142548.
paulsemel added a comment.

Fixed typo..


Repository:
  rC Clang

https://reviews.llvm.org/D45665

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/dump-struct-builtin.c

Index: test/CodeGen/dump-struct-builtin.c
===
--- test/CodeGen/dump-struct-builtin.c
+++ test/CodeGen/dump-struct-builtin.c
@@ -2,6 +2,98 @@
 
 #include "Inputs/stdio.h"
 
+// CHECK: @unit1.a = private unnamed_addr constant %struct.U1A { i16 12 }, align 2
+// CHECK-NEXT: [[STRUCT_STR_U1:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U1A {\0A\00"
+// CHECK-NEXT: [[FIELD_U1:@[0-9]+]] = private unnamed_addr constant [11 x i8] c"short a : \00"
+// CHECK-NEXT: [[FORMAT_U1:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%hd\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U1:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit2.a = private unnamed_addr constant %struct.U2A { i16 12 }, align 2
+// CHECK-NEXT: [[STRUCT_STR_U2:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U2A {\0A\00"
+// CHECK-NEXT: [[FIELD_U2:@[0-9]+]] = private unnamed_addr constant [20 x i8] c"unsigned short a : \00"
+// CHECK-NEXT: [[FORMAT_U2:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%hu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U2:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit3.a = private unnamed_addr constant %struct.U3A { i32 12 }, align 4
+// CHECK-NEXT: [[STRUCT_STR_U3:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U3A {\0A\00"
+// CHECK-NEXT: [[FIELD_U3:@[0-9]+]] = private unnamed_addr constant [9 x i8] c"int a : \00"
+// CHECK-NEXT: [[FORMAT_U3:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%d\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U3:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit4.a = private unnamed_addr constant %struct.U4A { i32 12 }, align 4
+// CHECK-NEXT: [[STRUCT_STR_U4:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U4A {\0A\00"
+// CHECK-NEXT: [[FIELD_U4:@[0-9]+]] = private unnamed_addr constant [18 x i8] c"unsigned int a : \00"
+// CHECK-NEXT: [[FORMAT_U4:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%u\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U4:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit5.a = private unnamed_addr constant %struct.U5A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U5:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U5A {\0A\00"
+// CHECK-NEXT: [[FIELD_U5:@[0-9]+]] = private unnamed_addr constant [10 x i8] c"long a : \00"
+// CHECK-NEXT: [[FORMAT_U5:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%ld\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U5:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit6.a = private unnamed_addr constant %struct.U6A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U6:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U6A {\0A\00"
+// CHECK-NEXT: [[FIELD_U6:@[0-9]+]] = private unnamed_addr constant [19 x i8] c"unsigned long a : \00"
+// CHECK-NEXT: [[FORMAT_U6:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%lu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U6:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit7.a = private unnamed_addr constant %struct.U7A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U7:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U7A {\0A\00"
+// CHECK-NEXT: [[FIELD_U7:@[0-9]+]] = private unnamed_addr constant [15 x i8] c"long long a : \00"
+// CHECK-NEXT: [[FORMAT_U7:@[0-9]+]] = private unnamed_addr constant [6 x i8] c"%lld\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U7:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit8.a = private unnamed_addr constant %struct.U8A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U8:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U8A {\0A\00"
+// CHECK-NEXT: [[FIELD_U8:@[0-9]+]] = private unnamed_addr constant [24 x i8] c"unsigned long long a : \00"
+// CHECK-NEXT: [[FORMAT_U8:@[0-9]+]] = private unnamed_addr constant [6 x i8] c"%llu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U8:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit9.a = private unnamed_addr constant %struct.U9A { i8 97 }, align 1
+// CHECK-NEXT: [[STRUCT_STR_U9:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U9A {\0A\00"
+// CHECK-NEXT: [[FIELD_U9:@[0-9]+]] = private unnamed_addr constant [10 x i8] c"char a : \00"
+// CHECK-NEXT: [[FORMAT_U9:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%c\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U9:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @.str = private unnamed_addr constant [4 x i8] c"LSE\00", align 1
+
+// CHECK: @unit10.a = private unnamed_addr constant %struct.U10A { i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i32 0, i32 0) }, align 8
+// CHECK-NEXT:

r330095 - [analyzer] Do not invalidate the `this` pointer.

2018-04-15 Thread Henry Wong via cfe-commits

Author: henrywong
Date: Sun Apr 15 03:34:06 2018
New Revision: 330095

URL: http://llvm.org/viewvc/llvm-project?rev=330095&view=rev
Log:
[analyzer] Do not invalidate the `this` pointer.

Summary:
`this` pointer is not an l-value, although we have modeled `CXXThisRegion` for 
`this` pointer, we can only bind it once, which is when we start to inline 
method. And this patch fixes https://bugs.llvm.org/show_bug.cgi?id=35506.

In addition, I didn't find any other cases other than loop-widen that could 
invalidate `this` pointer.

Reviewers: NoQ, george.karpenkov, a.sidorin, seaneveson, szepet

Reviewed By: NoQ

Subscribers: xazax.hun, rnkovacs, cfe-commits, MTC

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

Added:
cfe/trunk/test/Analysis/this-pointer.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp?rev=330095&r1=330094&r2=330095&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp Sun Apr 15 03:34:06 2018
@@ -59,6 +59,18 @@ ProgramStateRef getWidenedLoopState(Prog
 ITraits.setTrait(Region,
  RegionAndSymbolInvalidationTraits::TK_EntireMemSpace);
   }
+
+  // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
+  // is located in a method, constructor or destructor, the value of 'this'
+  // pointer shoule remain unchanged.
+  if (const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl())) {
+const CXXThisRegion *ThisR = MRMgr.getCXXThisRegion(
+CXXMD->getThisType(STC->getAnalysisDeclContext()->getASTContext()),
+STC);
+ITraits.setTrait(ThisR,
+ RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+  }
+
   return PrevState->invalidateRegions(Regions, getLoopCondition(LoopStmt),
   BlockCount, LCtx, true, nullptr, nullptr,
   &ITraits);

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=330095&r1=330094&r2=330095&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Sun Apr 15 03:34:06 2018
@@ -2050,6 +2050,9 @@ RegionStoreManager::bind(RegionBindingsC
 R = GetElementZeroRegion(SR, T);
   }
 
+  assert((!isa(R) || !B.lookup(R)) &&
+ "'this' pointer is not an l-value and is not assignable");
+
   // Clear out bindings that may overlap with this binding.
   RegionBindingsRef NewB = removeSubRegionBindings(B, cast(R));
   return NewB.addBinding(BindingKey::Make(R, BindingKey::Direct), V);

Added: cfe/trunk/test/Analysis/this-pointer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/this-pointer.cpp?rev=330095&view=auto
==
--- cfe/trunk/test/Analysis/this-pointer.cpp (added)
+++ cfe/trunk/test/Analysis/this-pointer.cpp Sun Apr 15 03:34:06 2018
@@ -0,0 +1,88 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection 
-analyzer-config widen-loops=true -analyzer-disable-retry-exhausted -verify %s
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_dump(int);
+
+// 'this' pointer is not an lvalue, we should not invalidate it.
+namespace this_pointer_after_loop_widen {
+class A {
+public:
+  A() {
+int count = 10;
+do {
+} while (count--);
+  }
+};
+
+void goo(A a);
+void test_temporary_object() {
+  goo(A()); // no-crash
+}
+
+struct B {
+  int mem;
+  B() : mem(0) {
+for (int i = 0; i < 10; ++i) {
+}
+mem = 0;
+  }
+};
+
+void test_ctor() {
+  B b;
+  clang_analyzer_eval(b.mem == 0); // expected-warning{{TRUE}}
+}
+
+struct C {
+  int mem;
+  C() : mem(0) {}
+  void set() {
+for (int i = 0; i < 10; ++i) {
+}
+mem = 10;
+  }
+};
+
+void test_method() {
+  C c;
+  clang_analyzer_eval(c.mem == 0); // expected-warning{{TRUE}}
+  c.set();
+  clang_analyzer_eval(c.mem == 10); // expected-warning{{TRUE}}
+}
+
+struct D {
+  int mem;
+  D() : mem(0) {}
+  void set() {
+for (int i = 0; i < 10; ++i) {
+}
+mem = 10;
+  }
+};
+
+void test_new() {
+  D *d = new D;
+  clang_analyzer_eval(d->mem == 0); // expected-warning{{TRUE}}
+  d->set();
+  clang_analyzer_eval(d->mem == 10); // expected-warning{{TRUE}}
+}
+
+struct E {
+  int mem;
+  E() : mem(0) {}
+  void set() {
+for (int i = 0; i < 10; ++i) {
+}
+setAux();
+  }
+  void setAux() {
+this->mem = 10;
+  }
+};
+
+void test_chained_method_call() {
+  E e;
+  e.set();
+  clang_analyzer_e

[PATCH] D45491: [analyzer] Do not invalidate the `this` pointer.

2018-04-15 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC330095: [analyzer] Do not invalidate the `this` pointer. 
(authored by henrywong, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D45491

Files:
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/this-pointer.cpp

Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2050,6 +2050,9 @@
 R = GetElementZeroRegion(SR, T);
   }
 
+  assert((!isa(R) || !B.lookup(R)) &&
+ "'this' pointer is not an l-value and is not assignable");
+
   // Clear out bindings that may overlap with this binding.
   RegionBindingsRef NewB = removeSubRegionBindings(B, cast(R));
   return NewB.addBinding(BindingKey::Make(R, BindingKey::Direct), V);
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -59,6 +59,18 @@
 ITraits.setTrait(Region,
  RegionAndSymbolInvalidationTraits::TK_EntireMemSpace);
   }
+
+  // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
+  // is located in a method, constructor or destructor, the value of 'this'
+  // pointer shoule remain unchanged.
+  if (const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl())) {
+const CXXThisRegion *ThisR = MRMgr.getCXXThisRegion(
+CXXMD->getThisType(STC->getAnalysisDeclContext()->getASTContext()),
+STC);
+ITraits.setTrait(ThisR,
+ RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+  }
+
   return PrevState->invalidateRegions(Regions, getLoopCondition(LoopStmt),
   BlockCount, LCtx, true, nullptr, nullptr,
   &ITraits);
Index: test/Analysis/this-pointer.cpp
===
--- test/Analysis/this-pointer.cpp
+++ test/Analysis/this-pointer.cpp
@@ -0,0 +1,88 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config widen-loops=true -analyzer-disable-retry-exhausted -verify %s
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_dump(int);
+
+// 'this' pointer is not an lvalue, we should not invalidate it.
+namespace this_pointer_after_loop_widen {
+class A {
+public:
+  A() {
+int count = 10;
+do {
+} while (count--);
+  }
+};
+
+void goo(A a);
+void test_temporary_object() {
+  goo(A()); // no-crash
+}
+
+struct B {
+  int mem;
+  B() : mem(0) {
+for (int i = 0; i < 10; ++i) {
+}
+mem = 0;
+  }
+};
+
+void test_ctor() {
+  B b;
+  clang_analyzer_eval(b.mem == 0); // expected-warning{{TRUE}}
+}
+
+struct C {
+  int mem;
+  C() : mem(0) {}
+  void set() {
+for (int i = 0; i < 10; ++i) {
+}
+mem = 10;
+  }
+};
+
+void test_method() {
+  C c;
+  clang_analyzer_eval(c.mem == 0); // expected-warning{{TRUE}}
+  c.set();
+  clang_analyzer_eval(c.mem == 10); // expected-warning{{TRUE}}
+}
+
+struct D {
+  int mem;
+  D() : mem(0) {}
+  void set() {
+for (int i = 0; i < 10; ++i) {
+}
+mem = 10;
+  }
+};
+
+void test_new() {
+  D *d = new D;
+  clang_analyzer_eval(d->mem == 0); // expected-warning{{TRUE}}
+  d->set();
+  clang_analyzer_eval(d->mem == 10); // expected-warning{{TRUE}}
+}
+
+struct E {
+  int mem;
+  E() : mem(0) {}
+  void set() {
+for (int i = 0; i < 10; ++i) {
+}
+setAux();
+  }
+  void setAux() {
+this->mem = 10;
+  }
+};
+
+void test_chained_method_call() {
+  E e;
+  e.set();
+  clang_analyzer_eval(e.mem == 10); // expected-warning{{TRUE}}
+}
+} // namespace this_pointer_after_loop_widen
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45668: [NEON] Define vget_high_f16() and vget_low_f16() intrinsics in AArch64 mode only

2018-04-15 Thread Ivan Kosarev via Phabricator via cfe-commits

kosarev created this revision.
kosarev added reviewers: t.p.northover, rengolin, SjoerdMeijer.
kosarev added a project: clang.
Herald added subscribers: kristof.beyls, javed.absar.

These are AArch64-specific intrinsics. The patch removes AArch32-mode test 
cases and maintains AArch64 ones in 
tools/clang/test/CodeGen/aarch64-neon-vget-hilo.c.


https://reviews.llvm.org/D45668

Files:
  include/clang/Basic/arm_neon.td
  test/CodeGen/arm_neon_intrinsics.c


Index: test/CodeGen/arm_neon_intrinsics.c
===
--- test/CodeGen/arm_neon_intrinsics.c
+++ test/CodeGen/arm_neon_intrinsics.c
@@ -3254,13 +3254,6 @@
   return vget_high_s64(a);
 }
 
-// CHECK-LABEL: @test_vget_high_f16(
-// CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <8 x half> %a, <8 x half> %a, <4 
x i32> 
-// CHECK:   ret <4 x half> [[SHUFFLE_I]]
-float16x4_t test_vget_high_f16(float16x8_t a) {
-  return vget_high_f16(a);
-}
-
 // CHECK-LABEL: @test_vget_high_f32(
 // CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <4 x float> %a, <4 x float> %a, 
<2 x i32> 
 // CHECK:   ret <2 x float> [[SHUFFLE_I]]
@@ -3560,13 +3553,6 @@
   return vget_low_s64(a);
 }
 
-// CHECK-LABEL: @test_vget_low_f16(
-// CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <8 x half> %a, <8 x half> %a, <4 
x i32> 
-// CHECK:   ret <4 x half> [[SHUFFLE_I]]
-float16x4_t test_vget_low_f16(float16x8_t a) {
-  return vget_low_f16(a);
-}
-
 // CHECK-LABEL: @test_vget_low_f32(
 // CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <4 x float> %a, <4 x float> %a, 
<2 x i32> 
 // CHECK:   ret <2 x float> [[SHUFFLE_I]]
Index: include/clang/Basic/arm_neon.td
===
--- include/clang/Basic/arm_neon.td
+++ include/clang/Basic/arm_neon.td
@@ -398,8 +398,14 @@
 

 // E.3.21 Splitting vectors
 let InstName = "vmov" in {
-def VGET_HIGH : NoTestOpInst<"vget_high", "dk", "csilhfUcUsUiUlPcPs", OP_HI>;
-def VGET_LOW  : NoTestOpInst<"vget_low", "dk", "csilhfUcUsUiUlPcPs", OP_LO>;
+def VGET_HIGH : NoTestOpInst<"vget_high", "dk", "csilfUcUsUiUlPcPs", OP_HI>;
+def VGET_LOW  : NoTestOpInst<"vget_low", "dk", "csilfUcUsUiUlPcPs", OP_LO>;
+}
+let ArchGuard = "__ARM_ARCH >= 8 && defined(__aarch64__)" in {
+  let InstName = "vmov" in {
+  def VGET_HIGH_F16 : NoTestOpInst<"vget_high", "dk", "h", OP_HI>;
+  def VGET_LOW_F16  : NoTestOpInst<"vget_low", "dk", "h", OP_LO>;
+  }
 }
 
 



Index: test/CodeGen/arm_neon_intrinsics.c
===
--- test/CodeGen/arm_neon_intrinsics.c
+++ test/CodeGen/arm_neon_intrinsics.c
@@ -3254,13 +3254,6 @@
   return vget_high_s64(a);
 }
 
-// CHECK-LABEL: @test_vget_high_f16(
-// CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <8 x half> %a, <8 x half> %a, <4 x i32> 
-// CHECK:   ret <4 x half> [[SHUFFLE_I]]
-float16x4_t test_vget_high_f16(float16x8_t a) {
-  return vget_high_f16(a);
-}
-
 // CHECK-LABEL: @test_vget_high_f32(
 // CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <4 x float> %a, <4 x float> %a, <2 x i32> 
 // CHECK:   ret <2 x float> [[SHUFFLE_I]]
@@ -3560,13 +3553,6 @@
   return vget_low_s64(a);
 }
 
-// CHECK-LABEL: @test_vget_low_f16(
-// CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <8 x half> %a, <8 x half> %a, <4 x i32> 
-// CHECK:   ret <4 x half> [[SHUFFLE_I]]
-float16x4_t test_vget_low_f16(float16x8_t a) {
-  return vget_low_f16(a);
-}
-
 // CHECK-LABEL: @test_vget_low_f32(
 // CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <4 x float> %a, <4 x float> %a, <2 x i32> 
 // CHECK:   ret <2 x float> [[SHUFFLE_I]]
Index: include/clang/Basic/arm_neon.td
===
--- include/clang/Basic/arm_neon.td
+++ include/clang/Basic/arm_neon.td
@@ -398,8 +398,14 @@
 
 // E.3.21 Splitting vectors
 let InstName = "vmov" in {
-def VGET_HIGH : NoTestOpInst<"vget_high", "dk", "csilhfUcUsUiUlPcPs", OP_HI>;
-def VGET_LOW  : NoTestOpInst<"vget_low", "dk", "csilhfUcUsUiUlPcPs", OP_LO>;
+def VGET_HIGH : NoTestOpInst<"vget_high", "dk", "csilfUcUsUiUlPcPs", OP_HI>;
+def VGET_LOW  : NoTestOpInst<"vget_low", "dk", "csilfUcUsUiUlPcPs", OP_LO>;
+}
+let ArchGuard = "__ARM_ARCH >= 8 && defined(__aarch64__)" in {
+  let InstName = "vmov" in {
+  def VGET_HIGH_F16 : NoTestOpInst<"vget_high", "dk", "h", OP_HI>;
+  def VGET_LOW_F16  : NoTestOpInst<"vget_low", "dk", "h", OP_LO>;
+  }
 }
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45669: [NEON] Fix the architecture condition for the crypto intrinsics

2018-04-15 Thread Ivan Kosarev via Phabricator via cfe-commits

kosarev created this revision.
kosarev added reviewers: t.p.northover, rengolin, SjoerdMeijer.
kosarev added a project: clang.
Herald added subscribers: kristof.beyls, javed.absar.

This is rather a cosmetic change as on ARMv7 targets we do not define 
__ARM_FEATURE_CRYPTO, even if it was explicitly requested with -target-feature. 
The crypto intrinsics test cases are in tools/clang/test/CodeGen/neon-crypto.c 
. No changes are necessary for them.


https://reviews.llvm.org/D45669

Files:
  include/clang/Basic/arm_neon.td


Index: include/clang/Basic/arm_neon.td
===
--- include/clang/Basic/arm_neon.td
+++ include/clang/Basic/arm_neon.td
@@ -913,7 +913,7 @@
 
 

 // Crypto
-let ArchGuard = "__ARM_FEATURE_CRYPTO" in {
+let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_CRYPTO)" in {
 def AESE : SInst<"vaese", "ddd", "QUc">;
 def AESD : SInst<"vaesd", "ddd", "QUc">;
 def AESMC : SInst<"vaesmc", "dd", "QUc">;


Index: include/clang/Basic/arm_neon.td
===
--- include/clang/Basic/arm_neon.td
+++ include/clang/Basic/arm_neon.td
@@ -913,7 +913,7 @@
 
 
 // Crypto
-let ArchGuard = "__ARM_FEATURE_CRYPTO" in {
+let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_CRYPTO)" in {
 def AESE : SInst<"vaese", "ddd", "QUc">;
 def AESD : SInst<"vaesd", "ddd", "QUc">;
 def AESMC : SInst<"vaesmc", "dd", "QUc">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45670: [NEON} Define vfma_n_f32() and vfmaq_n_f32() intrinsics in AArch32 mode

2018-04-15 Thread Ivan Kosarev via Phabricator via cfe-commits

kosarev created this revision.
kosarev added reviewers: t.p.northover, rengolin, SjoerdMeijer.
kosarev added a project: clang.
Herald added subscribers: kristof.beyls, javed.absar.

Currently we only support them in AArch64 mode. The AArch64 test cases are in 
tools/clang/test/CodeGen/aarch64-neon-2velem.c.


https://reviews.llvm.org/D45670

Files:
  include/clang/Basic/arm_neon.td
  test/CodeGen/arm-neon-fma.c


Index: test/CodeGen/arm-neon-fma.c
===
--- test/CodeGen/arm-neon-fma.c
+++ test/CodeGen/arm-neon-fma.c
@@ -20,3 +20,29 @@
 float32x4_t test_fmaq_order(float32x4_t accum, float32x4_t lhs, float32x4_t 
rhs) {
   return vfmaq_f32(accum, lhs, rhs);
 }
+
+// CHECK-LABEL: define <2 x float> @test_vfma_n_f32(<2 x float> %a, <2 x 
float> %b, float %n) #0 {
+// CHECK:   [[VECINIT_I:%.*]] = insertelement <2 x float> undef, float %n, i32 0
+// CHECK:   [[VECINIT1_I:%.*]] = insertelement <2 x float> [[VECINIT_I]], 
float %n, i32 1
+// CHECK:   [[TMP0:%.*]] = bitcast <2 x float> %a to <8 x i8>
+// CHECK:   [[TMP1:%.*]] = bitcast <2 x float> %b to <8 x i8>
+// CHECK:   [[TMP2:%.*]] = bitcast <2 x float> [[VECINIT1_I]] to <8 x i8>
+// CHECK:   [[TMP3:%.*]] = call <2 x float> @llvm.fma.v2f32(<2 x float> %b, <2 
x float> [[VECINIT1_I]], <2 x float> %a)
+// CHECK:   ret <2 x float> [[TMP3]]
+float32x2_t test_vfma_n_f32(float32x2_t a, float32x2_t b, float32_t n) {
+  return vfma_n_f32(a, b, n);
+}
+
+// CHECK-LABEL: define <4 x float> @test_vfmaq_n_f32(<4 x float> %a, <4 x 
float> %b, float %n) #0 {
+// CHECK:   [[VECINIT_I:%.*]] = insertelement <4 x float> undef, float %n, i32 0
+// CHECK:   [[VECINIT1_I:%.*]] = insertelement <4 x float> [[VECINIT_I]], 
float %n, i32 1
+// CHECK:   [[VECINIT2_I:%.*]] = insertelement <4 x float> [[VECINIT1_I]], 
float %n, i32 2
+// CHECK:   [[VECINIT3_I:%.*]] = insertelement <4 x float> [[VECINIT2_I]], 
float %n, i32 3
+// CHECK:   [[TMP0:%.*]] = bitcast <4 x float> %a to <16 x i8>
+// CHECK:   [[TMP1:%.*]] = bitcast <4 x float> %b to <16 x i8>
+// CHECK:   [[TMP2:%.*]] = bitcast <4 x float> [[VECINIT3_I]] to <16 x i8>
+// CHECK:   [[TMP3:%.*]] = call <4 x float> @llvm.fma.v4f32(<4 x float> %b, <4 
x float> [[VECINIT3_I]], <4 x float> %a)
+// CHECK:   ret <4 x float> [[TMP3]]
+float32x4_t test_vfmaq_n_f32(float32x4_t a, float32x4_t b, float32_t n) {
+  return vfmaq_n_f32(a, b, n);
+}
Index: include/clang/Basic/arm_neon.td
===
--- include/clang/Basic/arm_neon.td
+++ include/clang/Basic/arm_neon.td
@@ -531,6 +531,7 @@
 let ArchGuard = "defined(__ARM_FEATURE_FMA)" in {
   def VFMA : SInst<"vfma", "", "fQf">;
   def VFMS : SOpInst<"vfms", "", "fQf", OP_FMLS>;
+  def FMLA_N_F32 : SOpInst<"vfma_n", "ddds", "fQf", OP_FMLA_N>;
 }
 
 

@@ -621,7 +622,7 @@
 // MUL, MLA, MLS, FMA, FMS definitions with scalar argument
 def VMUL_N_A64 : IOpInst<"vmul_n", "dds", "Qd", OP_MUL_N>;
 
-def FMLA_N : SOpInst<"vfma_n", "ddds", "fdQfQd", OP_FMLA_N>;
+def FMLA_N : SOpInst<"vfma_n", "ddds", "dQd", OP_FMLA_N>;
 def FMLS_N : SOpInst<"vfms_n", "ddds", "fdQfQd", OP_FMLS_N>;
 
 def MLA_N : SOpInst<"vmla_n", "ddds", "Qd", OP_MLA_N>;


Index: test/CodeGen/arm-neon-fma.c
===
--- test/CodeGen/arm-neon-fma.c
+++ test/CodeGen/arm-neon-fma.c
@@ -20,3 +20,29 @@
 float32x4_t test_fmaq_order(float32x4_t accum, float32x4_t lhs, float32x4_t rhs) {
   return vfmaq_f32(accum, lhs, rhs);
 }
+
+// CHECK-LABEL: define <2 x float> @test_vfma_n_f32(<2 x float> %a, <2 x float> %b, float %n) #0 {
+// CHECK:   [[VECINIT_I:%.*]] = insertelement <2 x float> undef, float %n, i32 0
+// CHECK:   [[VECINIT1_I:%.*]] = insertelement <2 x float> [[VECINIT_I]], float %n, i32 1
+// CHECK:   [[TMP0:%.*]] = bitcast <2 x float> %a to <8 x i8>
+// CHECK:   [[TMP1:%.*]] = bitcast <2 x float> %b to <8 x i8>
+// CHECK:   [[TMP2:%.*]] = bitcast <2 x float> [[VECINIT1_I]] to <8 x i8>
+// CHECK:   [[TMP3:%.*]] = call <2 x float> @llvm.fma.v2f32(<2 x float> %b, <2 x float> [[VECINIT1_I]], <2 x float> %a)
+// CHECK:   ret <2 x float> [[TMP3]]
+float32x2_t test_vfma_n_f32(float32x2_t a, float32x2_t b, float32_t n) {
+  return vfma_n_f32(a, b, n);
+}
+
+// CHECK-LABEL: define <4 x float> @test_vfmaq_n_f32(<4 x float> %a, <4 x float> %b, float %n) #0 {
+// CHECK:   [[VECINIT_I:%.*]] = insertelement <4 x float> undef, float %n, i32 0
+// CHECK:   [[VECINIT1_I:%.*]] = insertelement <4 x float> [[VECINIT_I]], float %n, i32 1
+// CHECK:   [[VECINIT2_I:%.*]] = insertelement <4 x float> [[VECINIT1_I]], float %n, i32 2
+// CHECK:   [[VECINIT3_I:%.*]] = insertelement <4 x float> [[VECINIT2_I]], float %n, i32 3
+// CHECK:   [[TMP0:%.*]] = bitcast <4 x float> %a to <16 x i8>
+// CHECK:   [[TMP1:%.*]] = bitcast <4 x float> %b to <16 x i8>
+// CHECK:   [[TMP2:%.*]] = bitcast <4 x float> [[VECINI

[PATCH] D45671: [python bindings] Fix Cursor.result_type for ObjC method declarations - Bug 36677

2018-04-15 Thread Kyle Teske via Phabricator via cfe-commits

kjteske created this revision.
Herald added a subscriber: cfe-commits.

In cindex.py, Cursor.result_type called into the wrong libclang
function, causing cursors for ObjC method declarations to return invalid
result types. Fixes Bug 36677.


Repository:
  rC Clang

https://reviews.llvm.org/D45671

Files:
  bindings/python/clang/cindex.py
  bindings/python/tests/cindex/test_cursor.py


Index: bindings/python/tests/cindex/test_cursor.py
===
--- bindings/python/tests/cindex/test_cursor.py
+++ bindings/python/tests/cindex/test_cursor.py
@@ -429,6 +429,18 @@
 t = foo.result_type
 self.assertEqual(t.kind, TypeKind.INT)
 
+def test_result_type_objc_method_decl(self):
+code = """\
+@interface Interface : NSObject
+-(void)voidMethod;
+@end
+"""
+tu = get_tu(code, lang='objc')
+cursor = get_cursor(tu, 'voidMethod')
+result_type = cursor.result_type
+self.assertEqual(cursor.kind, CursorKind.OBJC_INSTANCE_METHOD_DECL)
+self.assertEqual(result_type.kind, TypeKind.VOID)
+
 def test_availability(self):
 tu = get_tu('class A { A(A const&) = delete; };', lang='cpp')
 
Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -1644,7 +1644,7 @@
 def result_type(self):
 """Retrieve the Type of the result for this Cursor."""
 if not hasattr(self, '_result_type'):
-self._result_type = conf.lib.clang_getResultType(self.type)
+self._result_type = conf.lib.clang_getCursorResultType(self)
 
 return self._result_type
 
@@ -3568,6 +3568,11 @@
[Cursor, c_uint, c_uint],
SourceRange),
 
+  ("clang_getCursorResultType",
+   [Cursor],
+   Type,
+   Type.from_result),
+
   ("clang_getCursorSemanticParent",
[Cursor],
Cursor,


Index: bindings/python/tests/cindex/test_cursor.py
===
--- bindings/python/tests/cindex/test_cursor.py
+++ bindings/python/tests/cindex/test_cursor.py
@@ -429,6 +429,18 @@
 t = foo.result_type
 self.assertEqual(t.kind, TypeKind.INT)
 
+def test_result_type_objc_method_decl(self):
+code = """\
+@interface Interface : NSObject
+-(void)voidMethod;
+@end
+"""
+tu = get_tu(code, lang='objc')
+cursor = get_cursor(tu, 'voidMethod')
+result_type = cursor.result_type
+self.assertEqual(cursor.kind, CursorKind.OBJC_INSTANCE_METHOD_DECL)
+self.assertEqual(result_type.kind, TypeKind.VOID)
+
 def test_availability(self):
 tu = get_tu('class A { A(A const&) = delete; };', lang='cpp')
 
Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -1644,7 +1644,7 @@
 def result_type(self):
 """Retrieve the Type of the result for this Cursor."""
 if not hasattr(self, '_result_type'):
-self._result_type = conf.lib.clang_getResultType(self.type)
+self._result_type = conf.lib.clang_getCursorResultType(self)
 
 return self._result_type
 
@@ -3568,6 +3568,11 @@
[Cursor, c_uint, c_uint],
SourceRange),
 
+  ("clang_getCursorResultType",
+   [Cursor],
+   Type,
+   Type.from_result),
+
   ("clang_getCursorSemanticParent",
[Cursor],
Cursor,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

2018-04-15 Thread Zahira Ammarguellat via Phabricator via cfe-commits

zahiraam marked 4 inline comments as done.
zahiraam added inline comments.



Comment at: lib/Parse/ParseDecl.cpp:572
+DeclSpecUuidDecl *ArgDecl =
+  DeclSpecUuidDecl::Create(Actions.getASTContext(),
+   Actions.getFunctionLevelDeclContext(),

rsmith wrote:
> What should happen if multiple declarations (of distinct entities) use the 
> same `__declspec(uuid(...))` attribute? Should you get a redefinition error 
> for the attribute or should they all share the same UUID entity? Either way, 
> we'll want to do some (minimal) UUID lookup and build a redeclaration chain 
> for `DeclSpecUuidDecl`s.
If we want to align with MS, we don't want  to signal an error. So may be I 
should have a map that assigns to each StrUuid a list of DeclSpecUuid?
So for this code:
struct
__declspec(uuid("{DDB47A6A-0F23-11D5-9109-00E0296B75D3}"))
S1 {};

struct
__declspec(uuid("{DDB47A6A-0F23-11D5-9109-00E0296B75D3}"))
S2 {};

I will have a map from DDB47A6A-0F23-11D5-9109-00E0296B75D3 to {S1, S2}. Do you 
agree?




Comment at: lib/Sema/SemaDeclAttr.cpp:4937-4938
-  return nullptr;
-Diag(UA->getLocation(), diag::err_mismatched_uuid);
-Diag(Range.getBegin(), diag::note_previous_uuid);
-D->dropAttr();

rsmith wrote:
> Do we still diagnose UUID mismatches somewhere else?
Not as far as I know. I guess I should put this diag somewhere?


https://reviews.llvm.org/D43576



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

[PATCH] D36610: [Tooling] Add option to getFullyQualifiedName using a custom PritingPolicy

2018-04-15 Thread Mikhail Ramalho via Phabricator via cfe-commits

mikhail.ramalho updated this revision to Diff 142578.
mikhail.ramalho added a comment.

Rebased to current master.

Removed the getFullyQualifiedName overloaded function.


https://reviews.llvm.org/D36610

Files:
  include/clang/AST/QualTypeNames.h
  lib/AST/QualTypeNames.cpp
  unittests/Tooling/QualTypeNamesTest.cpp


Index: unittests/Tooling/QualTypeNamesTest.cpp
===
--- unittests/Tooling/QualTypeNamesTest.cpp
+++ unittests/Tooling/QualTypeNamesTest.cpp
@@ -26,9 +26,13 @@
 std::string ExpectedName =
 ExpectedQualTypeNames.lookup(VD->getNameAsString());
 if (ExpectedName != "") {
-  std::string ActualName =
-  TypeName::getFullyQualifiedName(VD->getType(), *Context,
-  WithGlobalNsPrefix);
+  PrintingPolicy Policy(Context->getPrintingPolicy());
+  Policy.SuppressScope = false;
+  Policy.AnonymousTagLocations = true;
+  Policy.PolishForDeclaration = true;
+  Policy.SuppressUnwrittenScope = true;
+  std::string ActualName = TypeName::getFullyQualifiedName(
+  VD->getType(), *Context, Policy, WithGlobalNsPrefix);
   if (ExpectedName != ActualName) {
 // A custom message makes it much easier to see what declaration
 // failed compared to EXPECT_EQ.
@@ -217,6 +221,26 @@
   "  }\n"
   "}\n"
   );
+
+  TypeNameVisitor PrintingPolicy;
+  PrintingPolicy.ExpectedQualTypeNames["a"] = "short";
+  PrintingPolicy.ExpectedQualTypeNames["un_in_st_1"] =
+  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
+  "input.cc:2:31)";
+  PrintingPolicy.ExpectedQualTypeNames["b"] = "short";
+  PrintingPolicy.ExpectedQualTypeNames["un_in_st_2"] =
+  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
+  "input.cc:5:31)";
+  PrintingPolicy.ExpectedQualTypeNames["anon_st"] =
+  "struct (anonymous struct at input.cc:1:1)";
+  PrintingPolicy.runOver(R"(struct {
+  union {
+short a;
+  } un_in_st_1;
+  union {
+short b;
+  } un_in_st_2;
+} anon_st;)");
 }
 
 }  // end anonymous namespace
Index: lib/AST/QualTypeNames.cpp
===
--- lib/AST/QualTypeNames.cpp
+++ lib/AST/QualTypeNames.cpp
@@ -450,14 +450,9 @@
   return QT;
 }
 
-std::string getFullyQualifiedName(QualType QT,
-  const ASTContext &Ctx,
+std::string getFullyQualifiedName(QualType QT, const ASTContext &Ctx,
+  const PrintingPolicy &Policy,
   bool WithGlobalNsPrefix) {
-  PrintingPolicy Policy(Ctx.getPrintingPolicy());
-  Policy.SuppressScope = false;
-  Policy.AnonymousTagLocations = false;
-  Policy.PolishForDeclaration = true;
-  Policy.SuppressUnwrittenScope = true;
   QualType FQQT = getFullyQualifiedType(QT, Ctx, WithGlobalNsPrefix);
   return FQQT.getAsString(Policy);
 }
Index: include/clang/AST/QualTypeNames.h
===
--- include/clang/AST/QualTypeNames.h
+++ include/clang/AST/QualTypeNames.h
@@ -72,6 +72,7 @@
 /// \param[in] WithGlobalNsPrefix - If true, then the global namespace
 /// specifier "::" will be prepended to the fully qualified name.
 std::string getFullyQualifiedName(QualType QT, const ASTContext &Ctx,
+  const PrintingPolicy &Policy,
   bool WithGlobalNsPrefix = false);
 
 /// \brief Generates a QualType that can be used to name the same type


Index: unittests/Tooling/QualTypeNamesTest.cpp
===
--- unittests/Tooling/QualTypeNamesTest.cpp
+++ unittests/Tooling/QualTypeNamesTest.cpp
@@ -26,9 +26,13 @@
 std::string ExpectedName =
 ExpectedQualTypeNames.lookup(VD->getNameAsString());
 if (ExpectedName != "") {
-  std::string ActualName =
-  TypeName::getFullyQualifiedName(VD->getType(), *Context,
-  WithGlobalNsPrefix);
+  PrintingPolicy Policy(Context->getPrintingPolicy());
+  Policy.SuppressScope = false;
+  Policy.AnonymousTagLocations = true;
+  Policy.PolishForDeclaration = true;
+  Policy.SuppressUnwrittenScope = true;
+  std::string ActualName = TypeName::getFullyQualifiedName(
+  VD->getType(), *Context, Policy, WithGlobalNsPrefix);
   if (ExpectedName != ActualName) {
 // A custom message makes it much easier to see what declaration
 // failed compared to EXPECT_EQ.
@@ -217,6 +221,26 @@
   "  }\n"
   "}\n"
   );
+
+  TypeNameVisitor PrintingPolicy;
+  PrintingPolicy.ExpectedQualTypeNames["a"] = "short";
+  PrintingPolicy.ExpectedQualTypeNames["un_in_s

[PATCH] D35181: Defer addition of keywords to identifier table when loading AST

2018-04-15 Thread Johann Klähn via Phabricator via cfe-commits

jklaehn updated this revision to Diff 142579.
jklaehn added a comment.

Thanks for the review!  As I do not have commit access, it would be great if 
you could commit the updated patch.


https://reviews.llvm.org/D35181

Files:
  include/clang/Basic/IdentifierTable.h
  lib/Basic/IdentifierTable.cpp
  lib/Lex/Preprocessor.cpp
  unittests/libclang/LibclangTest.cpp

Index: unittests/libclang/LibclangTest.cpp
===
--- unittests/libclang/LibclangTest.cpp
+++ unittests/libclang/LibclangTest.cpp
@@ -698,3 +698,67 @@
 clang_disposeSourceRangeList(Ranges);
   }
 }
+
+class LibclangSerializationTest : public LibclangParseTest {
+public:
+  bool SaveAndLoadTU(const std::string &Filename) {
+unsigned options = clang_defaultSaveOptions(ClangTU);
+if (clang_saveTranslationUnit(ClangTU, Filename.c_str(), options) !=
+CXSaveError_None) {
+  DEBUG(llvm::dbgs() << "Saving failed\n");
+  return false;
+}
+
+clang_disposeTranslationUnit(ClangTU);
+
+ClangTU = clang_createTranslationUnit(Index, Filename.c_str());
+
+if (!ClangTU) {
+  DEBUG(llvm::dbgs() << "Loading failed\n");
+  return false;
+}
+
+return true;
+  }
+};
+
+TEST_F(LibclangSerializationTest, TokenKindsAreCorrectAfterLoading) {
+  // Ensure that "class" is recognized as a keyword token after serializing
+  // and reloading the AST, as it is not a keyword for the default LangOptions.
+  std::string HeaderName = "test.h";
+  WriteFile(HeaderName, "enum class Something {};");
+
+  const char *Argv[] = {"-xc++-header", "-std=c++11"};
+
+  ClangTU = clang_parseTranslationUnit(Index, HeaderName.c_str(), Argv,
+   sizeof(Argv) / sizeof(Argv[0]), nullptr,
+   0, TUFlags);
+
+  auto CheckTokenKinds = [=]() {
+CXSourceRange Range =
+clang_getCursorExtent(clang_getTranslationUnitCursor(ClangTU));
+
+CXToken *Tokens;
+unsigned int NumTokens;
+clang_tokenize(ClangTU, Range, &Tokens, &NumTokens);
+
+ASSERT_EQ(6u, NumTokens);
+EXPECT_EQ(CXToken_Keyword, clang_getTokenKind(Tokens[0]));
+EXPECT_EQ(CXToken_Keyword, clang_getTokenKind(Tokens[1]));
+EXPECT_EQ(CXToken_Identifier, clang_getTokenKind(Tokens[2]));
+EXPECT_EQ(CXToken_Punctuation, clang_getTokenKind(Tokens[3]));
+EXPECT_EQ(CXToken_Punctuation, clang_getTokenKind(Tokens[4]));
+EXPECT_EQ(CXToken_Punctuation, clang_getTokenKind(Tokens[5]));
+
+clang_disposeTokens(ClangTU, Tokens, NumTokens);
+  };
+
+  CheckTokenKinds();
+
+  std::string ASTName = "test.ast";
+  WriteFile(ASTName, "");
+
+  ASSERT_TRUE(SaveAndLoadTU(ASTName));
+
+  CheckTokenKinds();
+}
Index: lib/Lex/Preprocessor.cpp
===
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -85,12 +85,14 @@
IdentifierInfoLookup *IILookup, bool OwnsHeaders,
TranslationUnitKind TUKind)
 : PPOpts(std::move(PPOpts)), Diags(&diags), LangOpts(opts),
-  FileMgr(Headers.getFileMgr()), SourceMgr(SM),
-  PCMCache(PCMCache), ScratchBuf(new ScratchBuffer(SourceMgr)),
-  HeaderInfo(Headers), TheModuleLoader(TheModuleLoader),
-  ExternalSource(nullptr), Identifiers(opts, IILookup),
-  PragmaHandlers(new PragmaNamespace(StringRef())), TUKind(TUKind),
-  SkipMainFilePreamble(0, true),
+  FileMgr(Headers.getFileMgr()), SourceMgr(SM), PCMCache(PCMCache),
+  ScratchBuf(new ScratchBuffer(SourceMgr)), HeaderInfo(Headers),
+  TheModuleLoader(TheModuleLoader), ExternalSource(nullptr),
+  // As the language options may have not been loaded yet (when
+  // deserializing an ASTUnit), adding keywords to the identifier table is
+  // deferred to Preprocessor::Initialize().
+  Identifiers(IILookup), PragmaHandlers(new PragmaNamespace(StringRef())),
+  TUKind(TUKind), SkipMainFilePreamble(0, true),
   CurSubmoduleState(&NullSubmoduleState) {
   OwnsHeaderSearch = OwnsHeaders;
   
@@ -190,6 +192,9 @@
   // Initialize information about built-ins.
   BuiltinInfo.InitializeTarget(Target, AuxTarget);
   HeaderInfo.setTarget(Target);
+
+  // Populate the identifier table with info about keywords for the current language.
+  Identifiers.AddKeywords(LangOpts);
 }
 
 void Preprocessor::InitializeForModelFile() {
Index: lib/Basic/IdentifierTable.cpp
===
--- lib/Basic/IdentifierTable.cpp
+++ lib/Basic/IdentifierTable.cpp
@@ -79,16 +79,16 @@
   return new EmptyLookupIterator();
 }
 
+IdentifierTable::IdentifierTable(IdentifierInfoLookup *ExternalLookup)
+: HashTable(8192), // Start with space for 8K identifiers.
+  ExternalLookup(ExternalLookup) {}
+
 IdentifierTable::IdentifierTable(const LangOptions &LangOpts,
- IdentifierInfoLookup* externalLookup)
-  : HashTable(8192), // St

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-15 Thread Umann Kristóf via Phabricator via cfe-commits

Szelethus marked 6 inline comments as done.
Szelethus added a comment.

I'm about to update the diff, I changed a quite a lot of stuff, so I'm not sure 
that I'd be able to respond to these inline comments.




Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:31
+/// every other element is a field, and the element that precedes it is the
+/// object that contains it.
+class FieldChainInfo {

xazax.hun wrote:
> As far as I understand, for every operation, the only relevant contribution 
> of the non-last elements in the chain is the diagnostic message.
> I wonder if you would build a string instead of a FieldRegion chain and only 
> store the last FieldRegion would make this more explicit in the code. 
I experimented with a number of implementations along this idea, but I just 
couldn't get it to work. Here are my findings:
* `Twine`-s aren't meant to be copied, so using them didn't work out
* `StringRef`s for some reason get invalidated by the time it'd make a call to 
`FieldChainInfo::getAsString()`
* I didn't like the idea of storing the string because it'd not only impact 
performance greatly, but it'd also make the code a lot harder to understand, as 
opposed to making it more explicit

I did however try to make the implementation more easy to understand.



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:35
+
+  FieldChain Chain;
+  // If this is a fieldchain whose last element is an uninitialized region of a

xazax.hun wrote:
> I was wondering, do you need the chain at all? I think a field region might 
> be sufficient. The enclosing object of the field should be accessible by 
> querying the super region of the field region.
I tried it, but that approach made it impossible to include pointers and 
references in the fieldchain.



Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:218
+
+  std::string WarningMsg = std::to_string(UninitFields.size()) +
+   " uninitialized field" +

whisperity wrote:
> Maybe one could use a Twine or a string builder to avoid all these 
> unneccessary string instantiations? Or `std::string::append()`?
Doesn't move semantics take care of that? I'm not too sure on this one.


https://reviews.llvm.org/D45532



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

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-15 Thread Umann Kristóf via Phabricator via cfe-commits

Szelethus updated this revision to Diff 142581.
Szelethus marked 2 inline comments as done.
Szelethus edited the summary of this revision.
Szelethus added a comment.

Among many other things:

- The checker class is now on top of the file.
- Reviewed all comments, fixed typos, tried to make the general idea more 
understandable.
- Removed all (at least, all I could find) unnecessary functions and function 
arguments.
- Removed support for unions entirely.


https://reviews.llvm.org/D45532

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp
  test/Analysis/ctor-uninitialized-member-inheritance.cpp
  test/Analysis/ctor-uninitialized-member.cpp

Index: test/Analysis/ctor-uninitialized-member.cpp
===
--- /dev/null
+++ test/Analysis/ctor-uninitialized-member.cpp
@@ -0,0 +1,899 @@
+//RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.CtorUninitializedMember -std=c++11 -verify %s
+
+//===--===//
+// Default constructor test.
+//===--===//
+
+class DefaultConstructorTest {
+  int a, b, c, d, e, f, g, h, i, j;
+
+public:
+  DefaultConstructorTest() = default;
+};
+
+void f00() {
+  DefaultConstructorTest();
+}
+
+//===--===//
+// Initializer list test.
+//===--===//
+
+class InitListTest1 {
+  int a;
+  int b;
+
+public:
+  InitListTest1()
+  : a(1),
+b(2) {
+// All good!
+  }
+};
+
+void f01() {
+  InitListTest1();
+}
+
+class InitListTest2 {
+  int a;
+  int b; // expected-note{{uninitialized field 'this->b'}}
+
+public:
+  InitListTest2()
+  : a(3) {} // expected-warning{{1 uninitialized field}}
+};
+
+void f02() {
+  InitListTest2();
+}
+
+class InitListTest3 {
+  int a; // expected-note{{uninitialized field 'this->a'}}
+  int b;
+
+public:
+  InitListTest3()
+  : b(4) {} // expected-warning{{1 uninitialized field}}
+};
+
+void f03() {
+  InitListTest3();
+}
+
+//===--===//
+// Constructor body test.
+//===--===//
+
+class CtorBodyTest1 {
+  int a, b;
+
+public:
+  CtorBodyTest1() {
+a = 5;
+b = 6;
+// All good!
+  }
+};
+
+void f04() {
+  CtorBodyTest1();
+}
+
+class CtorBodyTest2 {
+  int a;
+  int b; // expected-note{{uninitialized field 'this->b'}}
+
+public:
+  CtorBodyTest2() {
+a = 7; // expected-warning{{1 uninitialized field}}
+  }
+};
+
+void f05() {
+  CtorBodyTest2();
+}
+
+class CtorBodyTest3 {
+  int a; // expected-note{{uninitialized field 'this->a'}}
+  int b;
+
+public:
+  CtorBodyTest3() {
+b = 8; // expected-warning{{1 uninitialized field}}
+  }
+};
+
+void f06() {
+  CtorBodyTest3();
+}
+
+class CtorBodyTest4 {
+  int a; // expected-note{{uninitialized field 'this->a'}}
+  int b; // expected-note{{uninitialized field 'this->b'}}
+
+public:
+  CtorBodyTest4() {}
+};
+
+void f07() {
+  CtorBodyTest4(); // expected-warning{{2 uninitialized fields}}
+}
+
+//===--===//
+// Constructor delegation test.
+//===--===//
+
+class CtorDelegationTest1 {
+  int a;
+  int b;
+
+public:
+  CtorDelegationTest1(int)
+  : a(9) {
+// leaves 'b' unintialized, but we'll never check this function
+  }
+
+  CtorDelegationTest1()
+  : CtorDelegationTest1(int{}) { // Initializing 'a'
+b = 10;
+// All good!
+  }
+};
+
+void f08() {
+  CtorDelegationTest1();
+}
+
+class CtorDelegationTest2 {
+  int a; // expected-note{{uninitialized field 'this->a'}}
+  int b;
+
+public:
+  CtorDelegationTest2(int)
+  : b(11) {
+// leaves 'a' unintialized, but we'll never check this function
+  }
+
+  CtorDelegationTest2()
+  : CtorDelegationTest2(int{}) { // expected-warning{{1 uninitialized field}}
+  }
+};
+
+void f09() {
+  CtorDelegationTest2();
+}
+
+//===--===//
+// Tests for classes containing records.
+//===--===//
+
+class ContainsRecordTest1 {
+  struct RecordType {
+int x;
+int y;
+  } rec;
+  int c, d;
+
+public:
+  ContainsRecordTest1()
+  : rec({12, 13}),
+c(14),
+d(15) {
+// All good!
+  }
+};
+
+void f10() {
+  ContainsRecordTest1();
+}
+
+class ContainsRecordTest2 {
+  struct RecordType {
+int x;
+int y; // expected-note{{uninitialized field 'this->rec.y'}}
+  } rec;
+  int c, d;
+
+public:
+  ContainsRecordTest2()
+  : c(16),
+d(17) {

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-15 Thread Umann Kristóf via Phabricator via cfe-commits

Szelethus marked 24 inline comments as done.
Szelethus added a comment.

Note that there was a comment made about the test files being too long. I still 
haven't split them, as I didn't find a good "splitting point". Is this okay, or 
shall I try to split these into numerous smaller ones?


https://reviews.llvm.org/D45532



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

[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output

2018-04-15 Thread Umann Kristóf via Phabricator via cfe-commits

Szelethus added a comment.

> Btw, what sort of UI are you trying to make these extra note pieces of mine 
> work with?

I'm also developing a checker: https://reviews.llvm.org/D45532. This checker 
emits very important information in notes. When I tried testing it with 
Ericsson's CodeChecker, I realized that notes aren't displayed. I'm planning to 
extend it so it will be able to.

> Was `-analyzer-config notes-as-events=true` of any help?

Never knew about the existence of this flag. Woops. I'll only be able to check 
it out tomorrow, but I'll post my findings here as soon as I have them :)


Repository:
  rC Clang

https://reviews.llvm.org/D45407



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

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-04-15 Thread Brooks Moses via Phabricator via cfe-commits

brooksmoses added a comment.

I have noticed two things when attempting to release LLVM with this revision 
internally at Google:

1. It's catching real bugs, all in constructors where someone wrote "member_ = 
member_" when they meant "member_ = member".

2. It's catching at least as many cases of tests where people are intentionally 
testing that self-assignment doesn't corrupt the data values.

Thus, this seems valuable but problematic, and the problems mean that initially 
we're facing turning off -Wself-assign completely until this is resolved.  
That's definitely an issue, and at least means that this needs to be placed 
under its own -W option.  And the real bugs it's finding seem to be very 
specific, and could be found by a more-focused warning.

Further, I would note that most warnings of this sort have some canonical way 
of arranging the code to avoid the warning -- for instance, casting an unused 
variable to "void" to create a no-op expression and thereby avoid the "unused 
variable" warning.  This warning doesn't seem to have one; "var = (var)", for 
instance, should IMO turn it off but doesn't.

(To add to that, we have a source file that does a lot of "var = var" to 
silence unused-variable warnings, so this breaks that on top of not having its 
own source "workaround".  Ick.)


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

[PATCH] D45677: [libcxx] [test] Fix typo in filesystem test

2018-04-15 Thread Joe Loser via Phabricator via cfe-commits

jloser created this revision.
jloser added a reviewer: EricWF.
Herald added subscribers: cfe-commits, christof.

There was a typo in "omitted" in 
`/test/std/experimental/filesystem/fs.op.funcs/fs.op.relative/relative.pass.cpp 
`


Repository:
  rCXX libc++

https://reviews.llvm.org/D45677

Files:
  
libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.relative/relative.pass.cpp


Index: 
libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.relative/relative.pass.cpp
===
--- 
libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.relative/relative.pass.cpp
+++ 
libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.relative/relative.pass.cpp
@@ -46,7 +46,7 @@
   {StaticEnv::SymlinkToDir, StaticEnv::Dir},
   {StaticEnv::SymlinkToDir / "dir2/.", StaticEnv::Dir / "dir2"},
   // FIXME? If the trailing separator occurs in a part of the path that 
exists,
-  // it is ommitted. Otherwise it is added to the end of the result.
+  // it is omitted. Otherwise it is added to the end of the result.
   {StaticEnv::SymlinkToDir / "dir2/./", StaticEnv::Dir / "dir2"},
   {StaticEnv::SymlinkToDir / "dir2/DNE/./", StaticEnv::Dir / "dir2/DNE/"},
   {StaticEnv::SymlinkToDir / "dir2", StaticEnv::Dir2},


Index: libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.relative/relative.pass.cpp
===
--- libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.relative/relative.pass.cpp
+++ libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.relative/relative.pass.cpp
@@ -46,7 +46,7 @@
   {StaticEnv::SymlinkToDir, StaticEnv::Dir},
   {StaticEnv::SymlinkToDir / "dir2/.", StaticEnv::Dir / "dir2"},
   // FIXME? If the trailing separator occurs in a part of the path that exists,
-  // it is ommitted. Otherwise it is added to the end of the result.
+  // it is omitted. Otherwise it is added to the end of the result.
   {StaticEnv::SymlinkToDir / "dir2/./", StaticEnv::Dir / "dir2"},
   {StaticEnv::SymlinkToDir / "dir2/DNE/./", StaticEnv::Dir / "dir2/DNE/"},
   {StaticEnv::SymlinkToDir / "dir2", StaticEnv::Dir2},
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-04-15 Thread Brooks Moses via Phabricator via cfe-commits

brooksmoses added a comment.

Further note: I'm noticing that nearly all the signal is from 
-Wself-assign-field and all the noise is from -Wself-assign.  So that would be 
one obvious way to make this higher-signal in what's enabled in -Wall, if that 
were a desire.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

[PATCH] D43578: -ftime-report switch support in Clang

2018-04-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added a comment.

In https://reviews.llvm.org/D43578#1068127, @mzolotukhin wrote:

> > Who is the audience for this information?
> >  What information do they want from a time report?
> >  How do we present that information in a way that's not misleading (given 
> > Clang's architecture)?
>
> I would find the timers extremely useful. Even if they overlap, they still 
> would 1) be a good indicator of a newly introduced problem and 2) give a 
> rough idea where frontend time is spent. I agree that it wouldn't be 
> super-accurate, but the numbers we usually operate are quite high (e.g.  part> started to take 1.5x time). When we decide to investigate it deeper, we 
> can use more accurate tools, such as profilers.

What kinds of  would be useful to you? (How do you want the runtime 
of Clang broken down?) Are vertical slices (through all Clang's various layers 
and potentially parts of LLVM) -- as this patch will produce -- useful, or 
would you really want horizontal slices (as in, what part of Clang is actually 
spending the time)? Or just anything that's basically expected to be consistent 
from run to run, so you can identify that //something// has slowed down, even 
if you don't have enough information to really know what?

> All that said, if we can make timers more accurate/not-overlapping, that 
> surely would be helpful as well.

I think we need to fix the overlap issue as a prerequisite to adding timers 
with large amounts of overlap, especially self-overlap. Otherwise the numbers 
will likely do more harm than good, as they will significantly misattribute 
runtime. Fortunately, I think that should only require some relatively small 
changes to the LLVM timer infrastructure.

>> Can we deliver useful value compared to a modern, dedicated profiling tool?
> 
> Having timers, especially producing results in the same format, as existing 
> LLVM timers, would be much more convenient than using profilers. With timers 
> I can simply add a compile-time flag to my test-suite run and get all the 
> numbers in the end of my usual run. With profilers it's a bit more 
> complicated.

Well, that depends on the profiler. With some profilers, you can just attach a 
profiler to your entire compilation and then ask it what the hottest functions 
were. But sure, if you have existing infrastructure built around 
`-ftime-report`, I can see that it makes sense to reuse that infrastructure.

> Speaking of timers overlapping and mutual calling: LLVM timers also have this 
> problem, and e.g. if there is problem is in some analysis (say, 
> ScalarEvolution), we see indications in several other passes using this 
> analysis (say, IndVarSimplify and LoopStrengthReduction). While it does not 
> immediately show the problematic spot, it gives you pretty strong hints to 
> where to look at first. So, having even not-perfect timers is still useful.

While LLVM may have some overlap between regions, the vertical timing slices 
are still pretty well aligned with the horizontal functionality slices. I 
expect the problems in Clang to be a //lot// worse. Suppose you enter N levels 
of parsing templates, and then trigger a whole bunch of template instantiation, 
AST deserialization, and code generation. Let's say that takes 1s in total. 
With this patch, I think we'd end up attributing Ns of compile time to "parsing 
templates", which is clearly very far from the truth, but will likely be listed 
as (by far) the slowest thing in the compilation. This does not even rise to 
the level of "not-perfect", it's going to be actively misleading in a lot of 
cases, and won't even necessarily point anywhere near the problematic spot.

I think with this patch and no fix to the overlap problem, we will find 
ourselves frequently fielding bugs where people say "X is slow" when it 
actually isn't. Plus I think we have the opportunity to deliver something 
that's hugely useful and not much harder to achieve (providing timing 
information that relates back to the source code), and I'd prefer we spend our 
complexity budget for this feature there.

> Also, I apologize for LGTMing the patch earlier while it was not properly 
> reviewed - I didn't notice it didn't have cfe-commits in subscribers, and it 
> had been waiting for a review for quite some time (so I assumed that all 
> interested parties had their chance to look at it).

No problem, these things happen :) And thank you for your feedback, it's very 
useful.

https://reviews.llvm.org/D43578

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

[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

2018-04-15 Thread Dean Michael Berris via Phabricator via cfe-commits

dberris added inline comments.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:678-679
   TC.AddCXXStdlibLibArgs(Args, CmdArgs);
+  if (TC.getTriple().getOS() == llvm::Triple::OpenBSD)
+CmdArgs.push_back(Args.hasArg(options::OPT_pg) ? "-lc++_p" : "-lc++");
+}

Maybe this should be in the `TC.AddCXXStdlibLibArgs(...)` function instead?


Repository:
  rC Clang

https://reviews.llvm.org/D45662



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

[PATCH] D45679: Add a new check, readability-unmodified-non-const-variable, that finds declarations of non-const variables that never get modified.

2018-04-15 Thread Shuai Wang via Phabricator via cfe-commits

shuaiwang created this revision.
Herald added subscribers: cfe-commits, mgorny, klimek.
shuaiwang added a reviewer: hokein.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/UnmodifiedNonConstVariableCheck.cpp
  clang-tidy/readability/UnmodifiedNonConstVariableCheck.h
  clang-tidy/utils/ASTUtils.cpp
  clang-tidy/utils/ASTUtils.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-unmodified-non-const-variable.rst
  test/clang-tidy/readability-unmodified-non-const-variable.cpp

Index: test/clang-tidy/readability-unmodified-non-const-variable.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-unmodified-non-const-variable.cpp
@@ -0,0 +1,47 @@
+// RUN: %check_clang_tidy %s readability-unmodified-non-const-variable %t
+
+template 
+struct pair {
+  T1 first;
+  T2 second;
+
+  void set(T1 f, T2 s);
+};
+
+void touch(int&);
+void touch(int*);
+
+int simple() {
+  int x = 10;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: declaring a non-const variable but never modified it [readability-unmodified-non-const-variable]
+  return x + 1;
+}
+
+void modified1() {
+  int x = 10;
+  touch(x);
+}
+
+void modified2() {
+  int x = 10;
+  touch(&x);
+}
+
+int modified3() {
+  int x = 10;
+  x += 20;
+  return x;
+}
+
+int callNonConstMember() {
+  pair p1;
+  p1.set(10, 20);
+  return p1.first + p1.second;
+}
+
+int followNonConstReference() {
+  pair p1;
+  auto& p2 = p1;
+  p2.first = 10;
+  return p1.first;
+}
Index: docs/clang-tidy/checks/readability-unmodified-non-const-variable.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-unmodified-non-const-variable.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - readability-unmodified-non-const-variable
+
+readability-unmodified-non-const-variable
+=
+
+Finds declarations of non-const variables that never get modified.
+
+For example:
+
+.. code-block:: c++
+
+  int simple() {
+int x = 10; // x is declared as non-const but is never modified.
+return x + 1;
+  }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -91,8 +91,8 @@
cppcoreguidelines-pro-type-vararg
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
-   fuchsia-header-anon-namespaces (redirects to google-build-namespaces) 
fuchsia-default-arguments
+   fuchsia-header-anon-namespaces (redirects to google-build-namespaces) 
fuchsia-multiple-inheritance
fuchsia-overloaded-operator
fuchsia-statically-constructed-objects
@@ -229,4 +229,5 @@
readability-static-definition-in-anonymous-namespace
readability-string-compare
readability-uniqueptr-delete-release
+   readability-unmodified-non-const-variable
zircon-temporary-objects
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,11 @@
 Improvements to clang-tidy
 --
 
+- New :doc:`readability-unmodified-non-const-variable
+  ` check
+
+  Finds declarations of non-const variables that never get modified.
+
 - New module `abseil` for checks related to the `Abseil `_
   library.
 
Index: clang-tidy/utils/ASTUtils.h
===
--- clang-tidy/utils/ASTUtils.h
+++ clang-tidy/utils/ASTUtils.h
@@ -27,6 +27,10 @@
 bool exprHasBitFlagWithSpelling(const Expr *Flags, const SourceManager &SM,
 const LangOptions &LangOpts,
 StringRef FlagName);
+
+// Checks whether `Exp` is (potentially) modified within `Stm`.
+bool isModified(const Expr& Exp, const Stmt& Stm, ASTContext* Context);
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tidy/utils/ASTUtils.cpp
===
--- clang-tidy/utils/ASTUtils.cpp
+++ clang-tidy/utils/ASTUtils.cpp
@@ -67,6 +67,96 @@
   return true;
 }
 
+namespace {
+class MatchCallbackAdaptor : public MatchFinder::MatchCallback {
+public:
+  explicit MatchCallbackAdaptor(
+  std::function Func)
+  : Func(std::move(Func)) {}
+  void run(const MatchFinder::MatchResult &Result) override { Func(Result); }
+
+private:
+  std::function Func;
+};
+} // namespace
+
+bool isModified(const Expr& Exp, const Stmt& Stm, ASTContext* Context) {
+  // LHS of any assignment operators.
+  const auto AsAssignmentLhs = binaryOperator(
+  anyOf(hasOperatorName("="), hasOperatorName("+="), hasOperatorName("-="),
+hasOperatorName("*="), hasOperatorName("/="

[PATCH] D45613: [X86] Introduce archs: goldmont-plus & tremont

2018-04-15 Thread Craig Topper via Phabricator via cfe-commits

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D45613



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

[PATCH] D45679: [clang-tidy] Add a new check, readability-unmodified-non-const-variable, that finds declarations of non-const variables that never get modified.

2018-04-15 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added a comment.

See also https://reviews.llvm.org/D45444.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

2018-04-15 Thread Shuai Wang via Phabricator via cfe-commits

shuaiwang added a comment.

Coming from https://reviews.llvm.org/D45679, which I should have sent out much 
earlier to get in front of you :p

Kidding aside this check is more mature than mine and I'm happy to help 
incorporate mine into this one.

I do have a strong opinion that requires non-trivial refactoring of your diff 
though: having a standalone reusable helper function like isModified(Expr, 
Stmt), that checks whether Expr is (potentially) modified within Stmt, is 
something I believe to be quite useful:

- performance/{ForRangeCopyCheck, UnnecessaryCopyInitialization} are using a 
much less sophisticated isOnlyUsedAsConst(), and can benefit from a more 
powerful one.
- I would imagine things could get messier if this check expands to also check 
for turning member functions const: it's basically checking CxxThisExpr, being 
a handle, is not modified within a member function, but note there's no VarDecl 
for "this".
- It's cleaner to follow (non-const) reference chains and only mark a decl as 
"can't be const" if everything on the chain is not modified, avoiding false 
negatives when a variable is bound to a never-modified non-const reference.
- It's also cleaner to follow member access chains, e.g. "v.a.b.c = 10" 
modifies "v". This one is particularly tricky because the modifying behavior 
happens at "c = 10" while the variable we'd like to mark as "can't be const" is 
arbitrary layers away, and MemberExpr alone doesn't warrant marking "can't be 
const" because that'll introduce way too many false negatives, ... and I 
haven't figure out a way to write a matcher to match 
memberExpr(hasObjectExpression(memberExpr(hasObjectExpression(...  ... (VarDeclRef) ...), not to mention any layer could either 
member access or array subscript access.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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

[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

2018-04-15 Thread David CARLIER via Phabricator via cfe-commits

devnexen added inline comments.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:678-679
   TC.AddCXXStdlibLibArgs(Args, CmdArgs);
+  if (TC.getTriple().getOS() == llvm::Triple::OpenBSD)
+CmdArgs.push_back(Args.hasArg(options::OPT_pg) ? "-lc++_p" : "-lc++");
+}

dberris wrote:
> Maybe this should be in the `TC.AddCXXStdlibLibArgs(...)` function instead?
I did not dare since it s the only case where it is needed (e.g. no need for 
X-ray for example)


Repository:
  rC Clang

https://reviews.llvm.org/D45662



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

[PATCH] D45578: Add a command line option `fregister_global_dtors_with_atexit` to register destructor functions annotated with attribute((destructor)) using __cxa_atexit or atexit.

2018-04-15 Thread Akira Hatanaka via Phabricator via cfe-commits

ahatanak updated this revision to Diff 142597.
ahatanak marked 4 inline comments as done.
ahatanak retitled this revision from "Add a command line option 
'fregister_dtor_with_atexit' to register destructor functions annotated 
with __attribute__((destructor)) using __cxa_atexit or atexit." to "Add a 
command line option `fregister_global_dtors_with_atexit` to register 
destructor functions annotated with __attribute__((destructor)) using 
__cxa_atexit or atexit.".
ahatanak edited the summary of this revision.
ahatanak added a comment.

Address review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D45578

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/constructor-attribute.c
  test/Driver/cxa-atexit.cpp
  test/Driver/rewrite-legacy-objc.m
  test/Driver/rewrite-objc.m

Index: test/Driver/rewrite-objc.m
===
--- test/Driver/rewrite-objc.m
+++ test/Driver/rewrite-objc.m
@@ -3,4 +3,4 @@
 // TEST0: clang{{.*}}" "-cc1"
 // TEST0: "-rewrite-objc"
 // FIXME: CHECK-NOT is broken somehow, it doesn't work here. Check adjacency instead.
-// TEST0: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fobjc-runtime=macosx" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fexceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
+// TEST0: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fregister-global-dtors-with-atexit" "-fobjc-runtime=macosx" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fexceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
Index: test/Driver/rewrite-legacy-objc.m
===
--- test/Driver/rewrite-legacy-objc.m
+++ test/Driver/rewrite-legacy-objc.m
@@ -3,11 +3,11 @@
 // TEST0: clang{{.*}}" "-cc1"
 // TEST0: "-rewrite-objc"
 // FIXME: CHECK-NOT is broken somehow, it doesn't work here. Check adjacency instead.
-// TEST0: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fobjc-runtime=macosx-fragile" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fexceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
+// TEST0: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fregister-global-dtors-with-atexit" "-fobjc-runtime=macosx-fragile" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fexceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
 // TEST0: rewrite-legacy-objc.m"
 // RUN: %clang -no-canonical-prefixes -target i386-apple-macosx10.9.0 -rewrite-legacy-objc %s -o - -### 2>&1 | \
 // RUN:   FileCheck -check-prefix=TEST1 %s
 // RUN: %clang -no-canonical-prefixes -target i386-apple-macosx10.6.0 -rewrite-legacy-objc %s -o - -### 2>&1 | \
 // RUN:   FileCheck -check-prefix=TEST2 %s
-// TEST1: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fobjc-runtime=macosx-fragile" "-fobjc-subscripting-legacy-runtime" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
-// TEST2: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fobjc-runtime=macosx-fragile" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
+// TEST1: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fregister-global-dtors-with-atexit" "-fobjc-runtime=macosx-fragile" "-fobjc-subscripting-legacy-runtime" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
+// TEST2: "-fmessage-length" "0" "-stack-protector" "1" "-fblocks" "-fencode-extended-block-signature" "-fregister-global-dtors-with-atexit" "-fobjc-runtime=macosx-fragile" "-fno-objc-infer-related-result-type" "-fobjc-exceptions" "-fmax-type-align=16" "-fdiagnostics-show-option"
Index: test/Driver/cxa-atexit.cpp
===
--- test/Driver/cxa-atexit.cpp
+++ test/Driver/cxa-atexit.cpp
@@ -25,3 +25,18 @@
 // CHECK-MTI: "-fno-use-cxa-atexit"
 // CHECK-MIPS-NOT: "-fno-use-cxa-atexit"
 
+// RUN: %clang -target x86_64-apple-darwin -fregister-global-dtors-with-atexit -fno-register-global-dtors-with-atexit -c -### %s 2>&1 | \
+// RUN: FileCheck --check-prefix=WITHOUTATEXIT %s
+// RUN: %clang -target x86_64-apple-darwin -fno-register-global-dtors-with-atexit -fregister-global-dtors-with-atexit -c -### %s 2>&1 | \
+// RUN: FileCheck --check-prefix=WITHATEXIT %s
+// RUN: %clang -target x

[PATCH] D45578: Add a command line option `fregister_global_dtors_with_atexit` to register destructor functions annotated with attribute((destructor)) using __cxa_atexit or atexit.

2018-04-15 Thread Akira Hatanaka via Phabricator via cfe-commits

ahatanak added inline comments.



Comment at: include/clang/Driver/Options.td:1613
+def fregister_dtor_with_atexit : Flag<["-"], "fregister-dtor-with-atexit">, 
Group, Flags<[CC1Option]>,
+  HelpText<"Use atexit or __cxa_atexit to register destructors">;
 def fuse_init_array : Flag<["-"], "fuse-init-array">, Group, 
Flags<[CC1Option]>,

rjmccall wrote:
> Probably worth spelling out that this is for *global* destructors, at least 
> in the help text and maybe in the option names.  At the very least, the 
> option name should say "dtors" instead of just "dtor".
I renamed the option to `-fregister-global-dtors-with-atexit`.



Comment at: lib/CodeGen/ItaniumCXXABI.cpp:2213
+void CodeGenModule::registerDtorsWithAtExit() {
+  for (const auto I : DtorsUsingAtExit) {
+int Priority = I.getFirst();

rjmccall wrote:
> This is iterating over a DenseMap and therefore making the emission order 
> dependent on details like how we hash ints for DenseMap.
> 
> Also, is this a correct way of handling Priority?  You should at least 
> justify that in comments.
I added a comment in registerGlobalDtorsWithAtExit that explains why destructor 
functions will be run in non-ascending order of their priority, which is what 
is expected.


Repository:
  rC Clang

https://reviews.llvm.org/D45578



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

[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

2018-04-15 Thread Dean Michael Berris via Phabricator via cfe-commits

dberris added inline comments.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:678-679
   TC.AddCXXStdlibLibArgs(Args, CmdArgs);
+  if (TC.getTriple().getOS() == llvm::Triple::OpenBSD)
+CmdArgs.push_back(Args.hasArg(options::OPT_pg) ? "-lc++_p" : "-lc++");
+}

devnexen wrote:
> dberris wrote:
> > Maybe this should be in the `TC.AddCXXStdlibLibArgs(...)` function instead?
> I did not dare since it s the only case where it is needed (e.g. no need for 
> X-ray for example)
Okay, let me ask this another way.

Is there ever a case for OpenBSD when we're doing `TC.AddCXXStdlibLibArgs(...)` 
that we don't need to also add these flags when `OPT_pg` is specified?


Repository:
  rC Clang

https://reviews.llvm.org/D45662



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

[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

2018-04-15 Thread David CARLIER via Phabricator via cfe-commits

devnexen added inline comments.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:678-679
   TC.AddCXXStdlibLibArgs(Args, CmdArgs);
+  if (TC.getTriple().getOS() == llvm::Triple::OpenBSD)
+CmdArgs.push_back(Args.hasArg(options::OPT_pg) ? "-lc++_p" : "-lc++");
+}

dberris wrote:
> devnexen wrote:
> > dberris wrote:
> > > Maybe this should be in the `TC.AddCXXStdlibLibArgs(...)` function 
> > > instead?
> > I did not dare since it s the only case where it is needed (e.g. no need 
> > for X-ray for example)
> Okay, let me ask this another way.
> 
> Is there ever a case for OpenBSD when we're doing 
> `TC.AddCXXStdlibLibArgs(...)` that we don't need to also add these flags when 
> `OPT_pg` is specified?
I see ... will do another version then :-)


Repository:
  rC Clang

https://reviews.llvm.org/D45662



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

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

[PATCH] D45665: [builtin-dump-struct] printf format checking

[PATCH] D45665: [builtin-dump-struct] printf format checking

r330095 - [analyzer] Do not invalidate the `this` pointer.

[PATCH] D45491: [analyzer] Do not invalidate the `this` pointer.

[PATCH] D45668: [NEON] Define vget_high_f16() and vget_low_f16() intrinsics in AArch64 mode only

[PATCH] D45669: [NEON] Fix the architecture condition for the crypto intrinsics

[PATCH] D45670: [NEON} Define vfma_n_f32() and vfmaq_n_f32() intrinsics in AArch32 mode

[PATCH] D45671: [python bindings] Fix Cursor.result_type for ObjC method declarations - Bug 36677

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

[PATCH] D36610: [Tooling] Add option to getFullyQualifiedName using a custom PritingPolicy

[PATCH] D35181: Defer addition of keywords to identifier table when loading AST

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

[PATCH] D45677: [libcxx] [test] Fix typo in filesystem test

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

[PATCH] D43578: -ftime-report switch support in Clang

[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

[PATCH] D45679: Add a new check, readability-unmodified-non-const-variable, that finds declarations of non-const variables that never get modified.

[PATCH] D45613: [X86] Introduce archs: goldmont-plus & tremont

[PATCH] D45679: [clang-tidy] Add a new check, readability-unmodified-non-const-variable, that finds declarations of non-const variables that never get modified.

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

[PATCH] D45578: Add a command line option `fregister_global_dtors_with_atexit` to register destructor functions annotated with attribute((destructor)) using __cxa_atexit or atexit.

[PATCH] D45578: Add a command line option `fregister_global_dtors_with_atexit` to register destructor functions annotated with attribute((destructor)) using __cxa_atexit or atexit.

[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

32 matches

Site Navigation

Mail list logo

Footer information