Re: [libcxx] r297553 - Change test coverage generation to use llvm-cov instead of gcov.

2017-03-13 Thread Bruno Cardoso Lopes via cfe-commits
Hi Eric,

I fixed the build for darwin in r297703, let me know if you have any comments.

Thanks,

On Mon, Mar 13, 2017 at 3:04 PM, Bruno Cardoso Lopes
 wrote:
> Hi Eric,
>
>>  if (APPLE AND (LIBCXX_CXX_ABI_LIBNAME STREQUAL "libcxxabi" OR
>> @@ -62,12 +66,7 @@ if (APPLE AND LLVM_USE_SANITIZER)
>>  message(WARNING "LLVM_USE_SANITIZER=${LLVM_USE_SANITIZER} is not 
>> supported on OS X")
>>endif()
>>if (LIBFILE)
>> -execute_process(COMMAND ${CMAKE_CXX_COMPILER} -print-file-name=lib 
>> OUTPUT_VARIABLE LIBDIR RESULT_VARIABLE Result)
>> -if (NOT ${Result} EQUAL "0")
>> -  message(FATAL "Failed to find library resource directory")
>> -endif()
>> -string(STRIP "${LIBDIR}" LIBDIR)
>> -set(LIBDIR "${LIBDIR}/darwin/")
>> +find_compiler_rt_dir(LIBDIR)
>
> Seems like this broke ASAN+UBSAN bot in green-dragon:
> http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan_build/3812
>
> Perhaps it's missing the "${LIBDIR}/darwin/" part from above?
>
> --
> Bruno Cardoso Lopes
> http://www.brunocardoso.cc



-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D30920: Do not pass -Os and -Oz to the Gold plugin

2017-03-13 Thread Pirama Arumuga Nainar via Phabricator via cfe-commits
pirama marked 3 inline comments as done.
pirama added inline comments.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:369
 if (A->getOption().matches(options::OPT_O4) ||
-A->getOption().matches(options::OPT_Ofast))
+A->getOption().matches(options::OPT_Ofast)) {
   OOpt = "3";

tejohnson wrote:
> Remove added "{"
Done.  I thought having braces consistent across `if` and `else` branches would 
be a consistent coding style but doesn't seem so: clang-format doesn't do this.


https://reviews.llvm.org/D30920



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


[libcxx] r297703 - Fix cmake to find the compiler-rt libs on darwin

2017-03-13 Thread Bruno Cardoso Lopes via cfe-commits
Author: bruno
Date: Mon Mar 13 23:12:29 2017
New Revision: 297703

URL: http://llvm.org/viewvc/llvm-project?rev=297703=rev
Log:
Fix cmake to find the compiler-rt libs on darwin

Followup for r297553, which left darwin in a broken state
http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan_build/3812

rdar://problem/31011980

Modified:
libcxx/trunk/cmake/Modules/HandleCompilerRT.cmake

Modified: libcxx/trunk/cmake/Modules/HandleCompilerRT.cmake
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/cmake/Modules/HandleCompilerRT.cmake?rev=297703=297702=297703=diff
==
--- libcxx/trunk/cmake/Modules/HandleCompilerRT.cmake (original)
+++ libcxx/trunk/cmake/Modules/HandleCompilerRT.cmake Mon Mar 13 23:12:29 2017
@@ -25,15 +25,27 @@ function(find_compiler_rt_dir dest)
 message(FATAL_ERROR "LIBCXX_COMPILE_FLAGS must be defined when using this 
function")
   endif()
   set(dest "" PARENT_SCOPE)
-  set(CLANG_COMMAND ${CMAKE_CXX_COMPILER} ${LIBCXX_COMPILE_FLAGS}
-  "--rtlib=compiler-rt" "--print-libgcc-file-name")
-  execute_process(
-  COMMAND ${CLANG_COMMAND}
-  RESULT_VARIABLE HAD_ERROR
-  OUTPUT_VARIABLE LIBRARY_FILE
-  )
-  string(STRIP "${LIBRARY_FILE}" LIBRARY_FILE)
-  get_filename_component(LIBRARY_DIR "${LIBRARY_FILE}" DIRECTORY)
+  if (APPLE)
+set(CLANG_COMMAND ${CMAKE_CXX_COMPILER} ${LIBCXX_COMPILE_FLAGS}
+"-print-file-name=lib")
+execute_process(
+COMMAND ${CLANG_COMMAND}
+RESULT_VARIABLE HAD_ERROR
+OUTPUT_VARIABLE LIBRARY_DIR
+)
+string(STRIP "${LIBRARY_DIR}" LIBRARY_DIR)
+set(LIBRARY_DIR "${LIBRARY_DIR}/darwin")
+  else()
+set(CLANG_COMMAND ${CMAKE_CXX_COMPILER} ${LIBCXX_COMPILE_FLAGS}
+"--rtlib=compiler-rt" "--print-libgcc-file-name")
+execute_process(
+COMMAND ${CLANG_COMMAND}
+RESULT_VARIABLE HAD_ERROR
+OUTPUT_VARIABLE LIBRARY_FILE
+)
+string(STRIP "${LIBRARY_FILE}" LIBRARY_FILE)
+get_filename_component(LIBRARY_DIR "${LIBRARY_FILE}" DIRECTORY)
+  endif()
   if (NOT HAD_ERROR AND EXISTS "${LIBRARY_DIR}")
 message(STATUS "Found compiler-rt directory: ${LIBRARY_DIR}")
 set(${dest} "${LIBRARY_DIR}" PARENT_SCOPE)


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


[PATCH] D30920: Do not pass -Os and -Oz to the Gold plugin

2017-03-13 Thread Pirama Arumuga Nainar via Phabricator via cfe-commits
pirama updated this revision to Diff 91671.
pirama added a comment.

Address review comments


https://reviews.llvm.org/D30920

Files:
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/gold-lto.c


Index: test/Driver/gold-lto.c
===
--- test/Driver/gold-lto.c
+++ test/Driver/gold-lto.c
@@ -26,3 +26,9 @@
 // RUN: %clang -target i686-linux-android -### %t.o -flto 2>&1 \
 // RUN: | FileCheck %s --check-prefix=CHECK-X86-ANDROID
 // CHECK-X86-ANDROID: "-plugin" "{{.*}}/LLVMgold.so"
+//
+// Test that -Os and -Oz are not passed to the plugin
+// RUN: %clang -### %t.o -flto -Os 2>&1 \
+// RUN: | FileCheck %s --implicit-check-not "-plugin-opt=Os"
+// RUN: %clang -### %t.o -flto -Oz 2>&1 \
+// RUN: | FileCheck %s --implicit-check-not "-plugin-opt=Oz"
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -368,9 +368,13 @@
 if (A->getOption().matches(options::OPT_O4) ||
 A->getOption().matches(options::OPT_Ofast))
   OOpt = "3";
-else if (A->getOption().matches(options::OPT_O))
-  OOpt = A->getValue();
-else if (A->getOption().matches(options::OPT_O0))
+else if (A->getOption().matches(options::OPT_O)) {
+  StringRef OptLevel = A->getValue();
+  // Do not pass optimization levels pertaining to code size to the plugin.
+  // They are captured by corresponding function attributes.
+  if (OptLevel != "s" && OptLevel != "z")
+OOpt = OptLevel;
+} else if (A->getOption().matches(options::OPT_O0))
   OOpt = "0";
 if (!OOpt.empty())
   CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=O") + OOpt));


Index: test/Driver/gold-lto.c
===
--- test/Driver/gold-lto.c
+++ test/Driver/gold-lto.c
@@ -26,3 +26,9 @@
 // RUN: %clang -target i686-linux-android -### %t.o -flto 2>&1 \
 // RUN: | FileCheck %s --check-prefix=CHECK-X86-ANDROID
 // CHECK-X86-ANDROID: "-plugin" "{{.*}}/LLVMgold.so"
+//
+// Test that -Os and -Oz are not passed to the plugin
+// RUN: %clang -### %t.o -flto -Os 2>&1 \
+// RUN: | FileCheck %s --implicit-check-not "-plugin-opt=Os"
+// RUN: %clang -### %t.o -flto -Oz 2>&1 \
+// RUN: | FileCheck %s --implicit-check-not "-plugin-opt=Oz"
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -368,9 +368,13 @@
 if (A->getOption().matches(options::OPT_O4) ||
 A->getOption().matches(options::OPT_Ofast))
   OOpt = "3";
-else if (A->getOption().matches(options::OPT_O))
-  OOpt = A->getValue();
-else if (A->getOption().matches(options::OPT_O0))
+else if (A->getOption().matches(options::OPT_O)) {
+  StringRef OptLevel = A->getValue();
+  // Do not pass optimization levels pertaining to code size to the plugin.
+  // They are captured by corresponding function attributes.
+  if (OptLevel != "s" && OptLevel != "z")
+OOpt = OptLevel;
+} else if (A->getOption().matches(options::OPT_O0))
   OOpt = "0";
 if (!OOpt.empty())
   CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=O") + OOpt));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: Patch for Bug 30413, including test case

2017-03-13 Thread Akira Hatanaka via cfe-commits
Committed r297702.

> On Mar 13, 2017, at 10:02 AM, Lobron, David  wrote:
> 
> Yes, please, if you don't mind!  I'd like to commit both the path and the 
> unit test, if possible.
> 
> Thanks,
> 
> David
> 
>> On Mar 13, 2017, at 12:47 PM, Akira Hatanaka  wrote:
>> 
>> Do you need someone to commit this patch for you?
>> 
>>> On Mar 10, 2017, at 6:44 AM, Lobron, David  wrote:
>>> 
>>> Hi Akira,
>>> 
>>> Thank you very much!  Please let me know if I need to take any further 
>>> steps beyond this email to cfe-commits in order for the patch and the unit 
>>> test to be committed.
>>> 
>>> Thanks,
>>> 
>>> David
>>> 
 On Mar 9, 2017, at 4:46 PM, Akira Hatanaka  wrote:
 
 Hi David,
 
 The patch looks good to me.
 
> On Mar 9, 2017, at 1:01 PM, Lobron, David  wrote:
> 
> Hi Akira,
> 
>> My concern is that the patch changes the encoding of 
>> @encode(id) on Darwin, which I think isn’t what you are trying 
>> to fix. If you compile the following code with command “clang -cc1 
>> -triple x86_64-apple-macosx”, the type encoding changes after applying 
>> the patch.
>> 
>> const char *foo() {
>> return @encode(id);
>> }
>> 
>> It seems like you can fix your problem without affecting Darwin by 
>> passing an extra argument to getObjCEncodingForType, just like 
>> CGObjCCommonMac::GetMethodVarType does.
> 
> Ah, thanks- I understand now.  Yes, this change seems a lot safer, and I 
> verified that it passes my test.  I've attached my new patch file, and 
> I've also attached the test again.  Please let me know if this works for 
> you or if you think it needs any additional work.
> 
> --David
> 
> 
 
>>> 
>> 
> 

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


r297702 - [CodeGen][ObjC] Fix a bug where the type of an ivar wasn't encoded

2017-03-13 Thread Akira Hatanaka via cfe-commits
Author: ahatanak
Date: Mon Mar 13 23:00:52 2017
New Revision: 297702

URL: http://llvm.org/viewvc/llvm-project?rev=297702=rev
Log:
[CodeGen][ObjC] Fix a bug where the type of an ivar wasn't encoded
correctly.

This fixes PR30413.

Patch by David Lobron.

Added:
cfe/trunk/test/CodeGenObjC/ivar-type-encoding.m
Modified:
cfe/trunk/lib/CodeGen/CGObjCGNU.cpp

Modified: cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCGNU.cpp?rev=297702=297701=297702=diff
==
--- cfe/trunk/lib/CodeGen/CGObjCGNU.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjCGNU.cpp Mon Mar 13 23:00:52 2017
@@ -2207,7 +2207,7 @@ void CGObjCGNU::GenerateClass(const ObjC
   IvarNames.push_back(MakeConstantString(IVD->getNameAsString()));
   // Get the type encoding for this ivar
   std::string TypeStr;
-  Context.getObjCEncodingForType(IVD->getType(), TypeStr);
+  Context.getObjCEncodingForType(IVD->getType(), TypeStr, IVD);
   IvarTypes.push_back(MakeConstantString(TypeStr));
   // Get the offset
   uint64_t BaseOffset = ComputeIvarBaseOffset(CGM, OID, IVD);

Added: cfe/trunk/test/CodeGenObjC/ivar-type-encoding.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/ivar-type-encoding.m?rev=297702=auto
==
--- cfe/trunk/test/CodeGenObjC/ivar-type-encoding.m (added)
+++ cfe/trunk/test/CodeGenObjC/ivar-type-encoding.m Mon Mar 13 23:00:52 2017
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -S -emit-llvm -fobjc-runtime=gcc -o - %s | FileCheck %s
+
+@protocol NSCopying
+@end
+
+@interface NSObject {
+  struct objc_object *isa;
+}
++ (id) new;
+- (id) init;
+@end
+
+@interface NSString : NSObject 
++ (NSString *)foo;
+@end
+
+@interface TestClass : NSObject {
+@public
+  NSString*_stringIvar;
+  int _intIvar;
+}
+@end
+@implementation TestClass
+
+@end
+
+int main() {
+  TestClass *c = [TestClass new];
+  return 0;
+}
+
+// CHECK: @0 = private unnamed_addr constant [12 x i8] c"_stringIvar\00"
+// CHECK: @1 = private unnamed_addr constant [12 x i8] c"@\22NSString\22\00"
+// CHECK: @2 = private unnamed_addr constant [9 x i8] c"_intIvar\00"
+// CHECK: @3 = private unnamed_addr constant [2 x i8] c"i\00"


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


[PATCH] D30762: [ubsan] Add a nullability sanitizer

2017-03-13 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL297700: [ubsan] Add a nullability sanitizer (authored by 
vedantk).

Changed prior to commit:
  https://reviews.llvm.org/D30762?vs=91382=91658#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30762

Files:
  cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
  cfe/trunk/include/clang/Basic/Sanitizers.def
  cfe/trunk/lib/CodeGen/CGCall.cpp
  cfe/trunk/lib/CodeGen/CGDecl.cpp
  cfe/trunk/lib/CodeGen/CGExprScalar.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  cfe/trunk/lib/Driver/ToolChain.cpp
  cfe/trunk/test/CodeGenObjC/ubsan-nonnull-and-nullability.m
  cfe/trunk/test/CodeGenObjC/ubsan-nullability.m

Index: cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
===
--- cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
+++ cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
@@ -92,6 +92,12 @@
  parameter which is declared to never be null.
   -  ``-fsanitize=null``: Use of a null pointer or creation of a null
  reference.
+  -  ``-fsanitize=nullability-arg``: Passing null as a function parameter
+ which is annotated with ``_Nonnull``.
+  -  ``-fsanitize=nullability-assign``: Assigning null to an lvalue which
+ is annotated with ``_Nonnull``.
+  -  ``-fsanitize=nullability-return``: Returning null from a function with
+ a return type annotated with ``_Nonnull``.
   -  ``-fsanitize=object-size``: An attempt to potentially use bytes which
  the optimizer can determine are not part of the object being accessed.
  This will also detect some types of undefined behavior that may not
@@ -130,11 +136,15 @@
 
 You can also use the following check groups:
   -  ``-fsanitize=undefined``: All of the checks listed above other than
- ``unsigned-integer-overflow``.
+ ``unsigned-integer-overflow`` and the ``nullability-*`` checks.
   -  ``-fsanitize=undefined-trap``: Deprecated alias of
  ``-fsanitize=undefined``.
   -  ``-fsanitize=integer``: Checks for undefined or suspicious integer
  behavior (e.g. unsigned integer overflow).
+  -  ``-fsanitize=nullability``: Enables ``nullability-arg``,
+ ``nullability-assign``, and ``nullability-return``. While violating
+ nullability does not have undefined behavior, it is often unintentional,
+ so UBSan offers to catch it.
 
 Stack traces and report symbolization
 =
Index: cfe/trunk/include/clang/Basic/Sanitizers.def
===
--- cfe/trunk/include/clang/Basic/Sanitizers.def
+++ cfe/trunk/include/clang/Basic/Sanitizers.def
@@ -64,6 +64,11 @@
 SANITIZER("integer-divide-by-zero", IntegerDivideByZero)
 SANITIZER("nonnull-attribute", NonnullAttribute)
 SANITIZER("null", Null)
+SANITIZER("nullability-arg", NullabilityArg)
+SANITIZER("nullability-assign", NullabilityAssign)
+SANITIZER("nullability-return", NullabilityReturn)
+SANITIZER_GROUP("nullability", Nullability,
+NullabilityArg | NullabilityAssign | NullabilityReturn)
 SANITIZER("object-size", ObjectSize)
 SANITIZER("return", Return)
 SANITIZER("returns-nonnull-attribute", ReturnsNonnullAttribute)
Index: cfe/trunk/test/CodeGenObjC/ubsan-nonnull-and-nullability.m
===
--- cfe/trunk/test/CodeGenObjC/ubsan-nonnull-and-nullability.m
+++ cfe/trunk/test/CodeGenObjC/ubsan-nonnull-and-nullability.m
@@ -0,0 +1,31 @@
+// REQUIRES: asserts
+// RUN: %clang_cc1 -x objective-c -emit-llvm -triple x86_64-apple-macosx10.10.0 -fsanitize=nullability-return,returns-nonnull-attribute,nullability-arg,nonnull-attribute %s -o - -w | FileCheck %s
+
+// If both the annotation and the attribute are present, prefer the attribute,
+// since it actually affects IRGen.
+
+// CHECK-LABEL: define nonnull i32* @f1
+__attribute__((returns_nonnull)) int *_Nonnull f1(int *_Nonnull p) {
+  // CHECK: entry:
+  // CHECK-NEXT: [[ADDR:%.*]] = alloca i32*
+  // CHECK-NEXT: store i32* [[P:%.*]], i32** [[ADDR]]
+  // CHECK-NEXT: [[ARG:%.*]] = load i32*, i32** [[ADDR]]
+  // CHECK-NEXT: [[ICMP:%.*]] = icmp ne i32* [[ARG]], null, !nosanitize
+  // CHECK-NEXT: br i1 [[ICMP]], label %[[CONT:.+]], label %[[HANDLE:[^,]+]]
+  // CHECK: [[HANDLE]]:
+  // CHECK-NEXT:   call void @__ubsan_handle_nonnull_return_abort
+  // CHECK-NEXT:   unreachable, !nosanitize
+  // CHECK: [[CONT]]:
+  // CHECK-NEXT:   ret i32*
+  return p;
+}
+
+// CHECK-LABEL: define void @f2
+void f2(int *_Nonnull __attribute__((nonnull)) p) {}
+
+// CHECK-LABEL: define void @call_f2
+void call_f2() {
+  // CHECK: call void @__ubsan_handle_nonnull_arg_abort
+  // CHECK-NOT: call void @__ubsan_handle_nonnull_arg_abort
+  f2((void *)0);
+}
Index: cfe/trunk/test/CodeGenObjC/ubsan-nullability.m
===
--- 

r297700 - [ubsan] Add a nullability sanitizer

2017-03-13 Thread Vedant Kumar via cfe-commits
Author: vedantk
Date: Mon Mar 13 20:56:34 2017
New Revision: 297700

URL: http://llvm.org/viewvc/llvm-project?rev=297700=rev
Log:
[ubsan] Add a nullability sanitizer

Teach UBSan to detect when a value with the _Nonnull type annotation
assumes a null value. Call expressions, initializers, assignments, and
return statements are all checked.

Because _Nonnull does not affect IRGen, the new checks are disabled by
default. The new driver flags are:

  -fsanitize=nullability-arg  (_Nonnull violation in call)
  -fsanitize=nullability-assign   (_Nonnull violation in assignment)
  -fsanitize=nullability-return   (_Nonnull violation in return stmt)
  -fsanitize=nullability  (all of the above)

This patch builds on top of UBSan's existing support for detecting
violations of the nonnull attributes ('nonnull' and 'returns_nonnull'),
and relies on the compiler-rt support for those checks. Eventually we
will need to update the diagnostic messages in compiler-rt (there are
FIXME's for this, which will be addressed in a follow-up).

One point of note is that the nullability-return check is only allowed
to kick in if all arguments to the function satisfy their nullability
preconditions. This makes it necessary to emit some null checks in the
function body itself.

Testing: check-clang and check-ubsan. I also built some Apple ObjC
frameworks with an asserts-enabled compiler, and verified that we get
valid reports.

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

Added:
cfe/trunk/test/CodeGenObjC/ubsan-nonnull-and-nullability.m
cfe/trunk/test/CodeGenObjC/ubsan-nullability.m
Modified:
cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
cfe/trunk/include/clang/Basic/Sanitizers.def
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/lib/Driver/ToolChain.cpp

Modified: cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UndefinedBehaviorSanitizer.rst?rev=297700=297699=297700=diff
==
--- cfe/trunk/docs/UndefinedBehaviorSanitizer.rst (original)
+++ cfe/trunk/docs/UndefinedBehaviorSanitizer.rst Mon Mar 13 20:56:34 2017
@@ -92,6 +92,12 @@ Available checks are:
  parameter which is declared to never be null.
   -  ``-fsanitize=null``: Use of a null pointer or creation of a null
  reference.
+  -  ``-fsanitize=nullability-arg``: Passing null as a function parameter
+ which is annotated with ``_Nonnull``.
+  -  ``-fsanitize=nullability-assign``: Assigning null to an lvalue which
+ is annotated with ``_Nonnull``.
+  -  ``-fsanitize=nullability-return``: Returning null from a function with
+ a return type annotated with ``_Nonnull``.
   -  ``-fsanitize=object-size``: An attempt to potentially use bytes which
  the optimizer can determine are not part of the object being accessed.
  This will also detect some types of undefined behavior that may not
@@ -130,11 +136,15 @@ Available checks are:
 
 You can also use the following check groups:
   -  ``-fsanitize=undefined``: All of the checks listed above other than
- ``unsigned-integer-overflow``.
+ ``unsigned-integer-overflow`` and the ``nullability-*`` checks.
   -  ``-fsanitize=undefined-trap``: Deprecated alias of
  ``-fsanitize=undefined``.
   -  ``-fsanitize=integer``: Checks for undefined or suspicious integer
  behavior (e.g. unsigned integer overflow).
+  -  ``-fsanitize=nullability``: Enables ``nullability-arg``,
+ ``nullability-assign``, and ``nullability-return``. While violating
+ nullability does not have undefined behavior, it is often unintentional,
+ so UBSan offers to catch it.
 
 Stack traces and report symbolization
 =

Modified: cfe/trunk/include/clang/Basic/Sanitizers.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Sanitizers.def?rev=297700=297699=297700=diff
==
--- cfe/trunk/include/clang/Basic/Sanitizers.def (original)
+++ cfe/trunk/include/clang/Basic/Sanitizers.def Mon Mar 13 20:56:34 2017
@@ -64,6 +64,11 @@ SANITIZER("function", Function)
 SANITIZER("integer-divide-by-zero", IntegerDivideByZero)
 SANITIZER("nonnull-attribute", NonnullAttribute)
 SANITIZER("null", Null)
+SANITIZER("nullability-arg", NullabilityArg)
+SANITIZER("nullability-assign", NullabilityAssign)
+SANITIZER("nullability-return", NullabilityReturn)
+SANITIZER_GROUP("nullability", Nullability,
+NullabilityArg | NullabilityAssign | NullabilityReturn)
 SANITIZER("object-size", ObjectSize)
 SANITIZER("return", Return)
 SANITIZER("returns-nonnull-attribute", ReturnsNonnullAttribute)

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 

[PATCH] D30920: Do not pass -Os and -Oz to the Gold plugin

2017-03-13 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment.

Agree with @pcc. Unless anyone has a need to have "perfect" support for Os, 
this is the right direction.


https://reviews.llvm.org/D30920



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


[PATCH] D30920: Do not pass -Os and -Oz to the Gold plugin

2017-03-13 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment.

In https://reviews.llvm.org/D30920#700077, @tejohnson wrote:

> Until everything is converted to using size attributes, it seems like a 
> correct fix for the bug is to accept these options in the gold-plugin and 
> pass through the LTO API to the PassManagerBuilder.


Not necessarily. There is no requirement (from a correctness perspective) that 
`-Os` at link time should exactly match the behaviour of `-Os` at compile time.




Comment at: lib/Driver/ToolChains/CommonArgs.cpp:375
+  // They are captured by corresponding function attributes.
+  if (!OptLevel.equals("s") && !OptLevel.equals("z"))
+OOpt = OptLevel;

This can just be `OptLevel != "s"` etc.



Comment at: test/Driver/gold-lto.c:33
+// RUN: | FileCheck %s --implicit-check-not "-plugin-opt=Os"
+// RUN: %clang -### %t.o -flto -Os 2>&1 \
+// RUN: | FileCheck %s --implicit-check-not "-plugin-opt=Oz"

`-Oz` here.


https://reviews.llvm.org/D30920



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


[PATCH] D30923: Warn on enum assignment to bitfields that can't fit all values

2017-03-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 91653.
rnk marked an inline comment as done.
rnk added a comment.

- Actually make this warning off by default


https://reviews.llvm.org/D30923

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/warn-bitfield-enum-conversion.cpp

Index: test/SemaCXX/warn-bitfield-enum-conversion.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-bitfield-enum-conversion.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -verify %s -Wbitfield-enum-conversion
+
+enum TwoBits { Hi1 = 3 } two_bits;
+enum TwoBitsSigned { Lo2 = -2, Hi2 = 1 } two_bits_signed;
+enum ThreeBits { Hi3 = 7 } three_bits;
+enum ThreeBitsSigned { Lo4 = -4, Hi4 = 3 } three_bits_signed;
+enum TwoBitsFixed : unsigned { Hi5 = 3 } two_bits_fixed;
+
+struct Foo {
+  unsigned two_bits : 2;
+  int two_bits_signed : 2;
+  unsigned three_bits : 3;
+  int three_bits_signed : 3;
+
+  ThreeBits three_bits_enum : 3;
+};
+
+void f() {
+  Foo f;
+
+  f.two_bits = two_bits;
+  f.two_bits = two_bits_signed;// expected-warning {{negative enumerators}}
+  f.two_bits = three_bits; // expected-warning {{not wide enough}}
+  f.two_bits = three_bits_signed;  // expected-warning {{negative enumerators}} expected-warning {{not wide enough}}
+  f.two_bits = two_bits_fixed;
+
+  f.two_bits_signed = two_bits;// expected-warning {{needs an extra bit}}
+  f.two_bits_signed = two_bits_signed;
+  f.two_bits_signed = three_bits;  // expected-warning {{not wide enough}}
+  f.two_bits_signed = three_bits_signed;   // expected-warning {{not wide enough}}
+
+  f.three_bits = two_bits;
+  f.three_bits = two_bits_signed;  // expected-warning {{negative enumerators}}
+  f.three_bits = three_bits;
+  f.three_bits = three_bits_signed;// expected-warning {{negative enumerators}}
+
+  f.three_bits_signed = two_bits;
+  f.three_bits_signed = two_bits_signed;
+  f.three_bits_signed = three_bits;// expected-warning {{needs an extra bit}}
+  f.three_bits_signed = three_bits_signed;
+
+  f.three_bits_enum = three_bits;  // expected-warning {{needs an extra bit}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -8729,13 +8729,42 @@
 return false;
 
   Expr *OriginalInit = Init->IgnoreParenImpCasts();
+  unsigned FieldWidth = Bitfield->getBitWidthValue(S.Context);
 
   llvm::APSInt Value;
-  if (!OriginalInit->EvaluateAsInt(Value, S.Context, Expr::SE_AllowSideEffects))
+  if (!OriginalInit->EvaluateAsInt(Value, S.Context,
+   Expr::SE_AllowSideEffects)) {
+// The RHS is not constant.  If the RHS has an enum type, make sure the
+// bitfield is wide enough to hold all the values of the enum without
+// truncation.
+if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+  EnumDecl *ED = EnumTy->getDecl();
+  bool SignedBitfield = BitfieldType->isSignedIntegerType();
+
+  // Check if this might store negative values into an unsigned bitfield.
+  if (!SignedBitfield && ED->getNumNegativeBits() > 0) {
+S.Diag(InitLoc, diag::warn_unsigned_bitfield_assigned_signed_enum)
+<< Bitfield << ED;
+  }
+
+  // Check for sufficient width.
+  if (ED->getNumPositiveBits() > FieldWidth ||
+  ED->getNumNegativeBits() > FieldWidth) {
+S.Diag(InitLoc, diag::warn_bitfield_too_small_for_enum)
+<< Bitfield << ED;
+  } else if (SignedBitfield && ED->getNumPositiveBits() == FieldWidth) {
+// If the bitfield type is signed and the positive field widths are
+// equal, values with the high bit set will become negative, which is
+// usually unintended.
+S.Diag(InitLoc, diag::warn_signed_bitfield_enum_conversion)
+<< Bitfield << ED;
+  }
+}
+
 return false;
+  }
 
   unsigned OriginalWidth = Value.getBitWidth();
-  unsigned FieldWidth = Bitfield->getBitWidthValue(S.Context);
 
   if (!Value.isSigned() || Value.isNegative())
 if (UnaryOperator *UO = dyn_cast(OriginalInit))
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -4908,6 +4908,17 @@
 def warn_anon_bitfield_width_exceeds_type_width : Warning<
   "width of anonymous bit-field (%0 bits) exceeds width of its type; value "
   "will be truncated to %1 bit%s1">, InGroup;
+def warn_bitfield_too_small_for_enum: Warning<
+  "bit-field %0 is not wide enough to store all enumerators of enum type %1">,
+  InGroup, DefaultIgnore;
+def warn_unsigned_bitfield_assigned_signed_enum: Warning<
+  "assigning value of signed enum type %1 

[PATCH] D30923: Warn on enum assignment to bitfields that can't fit all values

2017-03-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk marked an inline comment as done.
rnk added inline comments.



Comment at: test/Sema/warn-bitfield-enum-conversion.c:3
+
+enum TwoBits { Hi1 = 3 } two_bits;
+enum TwoBitsSigned { Lo2 = -2, Hi2 = 1 } two_bits_signed;

thakis wrote:
> can you add an enum with an explicit underlying type? will this warning 
> always fire for those?
I was worried for a moment, but it looks like `EnumDecl::getNumPositiveBits` 
does what I want and ignores the fixed underlying type.


https://reviews.llvm.org/D30923



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


[PATCH] D30923: Warn on enum assignment to bitfields that can't fit all values

2017-03-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 91652.
rnk added a comment.

- Make test C++, add fixed type enum


https://reviews.llvm.org/D30923

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/warn-bitfield-enum-conversion.cpp

Index: test/SemaCXX/warn-bitfield-enum-conversion.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-bitfield-enum-conversion.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -verify %s -Wbitfield-enum-conversion
+
+enum TwoBits { Hi1 = 3 } two_bits;
+enum TwoBitsSigned { Lo2 = -2, Hi2 = 1 } two_bits_signed;
+enum ThreeBits { Hi3 = 7 } three_bits;
+enum ThreeBitsSigned { Lo4 = -4, Hi4 = 3 } three_bits_signed;
+enum TwoBitsFixed : unsigned { Hi5 = 3 } two_bits_fixed;
+
+struct Foo {
+  unsigned two_bits : 2;
+  int two_bits_signed : 2;
+  unsigned three_bits : 3;
+  int three_bits_signed : 3;
+
+  ThreeBits three_bits_enum : 3;
+};
+
+void f() {
+  Foo f;
+
+  f.two_bits = two_bits;
+  f.two_bits = two_bits_signed;// expected-warning {{negative enumerators}}
+  f.two_bits = three_bits; // expected-warning {{not wide enough}}
+  f.two_bits = three_bits_signed;  // expected-warning {{negative enumerators}} expected-warning {{not wide enough}}
+  f.two_bits = two_bits_fixed;
+
+  f.two_bits_signed = two_bits;// expected-warning {{needs an extra bit}}
+  f.two_bits_signed = two_bits_signed;
+  f.two_bits_signed = three_bits;  // expected-warning {{not wide enough}}
+  f.two_bits_signed = three_bits_signed;   // expected-warning {{not wide enough}}
+
+  f.three_bits = two_bits;
+  f.three_bits = two_bits_signed;  // expected-warning {{negative enumerators}}
+  f.three_bits = three_bits;
+  f.three_bits = three_bits_signed;// expected-warning {{negative enumerators}}
+
+  f.three_bits_signed = two_bits;
+  f.three_bits_signed = two_bits_signed;
+  f.three_bits_signed = three_bits;// expected-warning {{needs an extra bit}}
+  f.three_bits_signed = three_bits_signed;
+
+  f.three_bits_enum = three_bits;  // expected-warning {{needs an extra bit}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -8729,13 +8729,42 @@
 return false;
 
   Expr *OriginalInit = Init->IgnoreParenImpCasts();
+  unsigned FieldWidth = Bitfield->getBitWidthValue(S.Context);
 
   llvm::APSInt Value;
-  if (!OriginalInit->EvaluateAsInt(Value, S.Context, Expr::SE_AllowSideEffects))
+  if (!OriginalInit->EvaluateAsInt(Value, S.Context,
+   Expr::SE_AllowSideEffects)) {
+// The RHS is not constant.  If the RHS has an enum type, make sure the
+// bitfield is wide enough to hold all the values of the enum without
+// truncation.
+if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+  EnumDecl *ED = EnumTy->getDecl();
+  bool SignedBitfield = BitfieldType->isSignedIntegerType();
+
+  // Check if this might store negative values into an unsigned bitfield.
+  if (!SignedBitfield && ED->getNumNegativeBits() > 0) {
+S.Diag(InitLoc, diag::warn_unsigned_bitfield_assigned_signed_enum)
+<< Bitfield << ED;
+  }
+
+  // Check for sufficient width.
+  if (ED->getNumPositiveBits() > FieldWidth ||
+  ED->getNumNegativeBits() > FieldWidth) {
+S.Diag(InitLoc, diag::warn_bitfield_too_small_for_enum)
+<< Bitfield << ED;
+  } else if (SignedBitfield && ED->getNumPositiveBits() == FieldWidth) {
+// If the bitfield type is signed and the positive field widths are
+// equal, values with the high bit set will become negative, which is
+// usually unintended.
+S.Diag(InitLoc, diag::warn_signed_bitfield_enum_conversion)
+<< Bitfield << ED;
+  }
+}
+
 return false;
+  }
 
   unsigned OriginalWidth = Value.getBitWidth();
-  unsigned FieldWidth = Bitfield->getBitWidthValue(S.Context);
 
   if (!Value.isSigned() || Value.isNegative())
 if (UnaryOperator *UO = dyn_cast(OriginalInit))
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -4908,6 +4908,17 @@
 def warn_anon_bitfield_width_exceeds_type_width : Warning<
   "width of anonymous bit-field (%0 bits) exceeds width of its type; value "
   "will be truncated to %1 bit%s1">, InGroup;
+def warn_bitfield_too_small_for_enum: Warning<
+  "bit-field %0 is not wide enough to store all enumerators of enum type %1">,
+  InGroup;
+def warn_unsigned_bitfield_assigned_signed_enum: Warning<
+  "assigning value of signed enum type %1 to unsigned bit-field %0; "
+  "negative enumerators of 

[PATCH] D30923: Warn on enum assignment to bitfields that can't fit all values

2017-03-13 Thread Nico Weber via Phabricator via cfe-commits
thakis added inline comments.



Comment at: test/Sema/warn-bitfield-enum-conversion.c:3
+
+enum TwoBits { Hi1 = 3 } two_bits;
+enum TwoBitsSigned { Lo2 = -2, Hi2 = 1 } two_bits_signed;

can you add an enum with an explicit underlying type? will this warning always 
fire for those?


https://reviews.llvm.org/D30923



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


[PATCH] D30700: [Driver] Always add arch-specific-subdir to -rpath

2017-03-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm, thanks!


https://reviews.llvm.org/D30700



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


r297696 - clang-format: Make it very slighly more expensive to wrap between "= {".

2017-03-13 Thread Daniel Jasper via cfe-commits
Author: djasper
Date: Mon Mar 13 19:40:32 2017
New Revision: 297696

URL: http://llvm.org/viewvc/llvm-project?rev=297696=rev
Log:
clang-format: Make it very slighly more expensive to wrap between "= {".

This prevents unwanted fallout from r296664. Specifically in proto formatting,
this changed:
  optional Aaaa  = 12 [
(aaa) = ,
(bb) = {
  a: true,
  : true
}
  ];

Into:
  optional Aaaa  = 12 [
(aaa) = ,
(bb) =
{a: true, : true}
  ];

Which is considered less readable. Generally, it seems preferable to
format such dict literals as blocks rather than contract them to one
line.

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestProto.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=297696=297695=297696=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Mar 13 19:40:32 2017
@@ -1976,7 +1976,7 @@ unsigned TokenAnnotator::splitPenalty(co
   if (Right.is(TT_LambdaArrow))
 return 110;
   if (Left.is(tok::equal) && Right.is(tok::l_brace))
-return 150;
+return 160;
   if (Left.is(TT_CastRParen))
 return 100;
   if (Left.is(tok::coloncolon) ||

Modified: cfe/trunk/unittests/Format/FormatTestProto.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestProto.cpp?rev=297696=297695=297696=diff
==
--- cfe/trunk/unittests/Format/FormatTestProto.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestProto.cpp Mon Mar 13 19:40:32 2017
@@ -138,6 +138,13 @@ TEST_F(FormatTestProto, MessageFieldAttr
   verifyFormat("optional string test = 1 [default =\n"
"  \"test\"\n"
"  \"test\"];");
+  verifyFormat("optional Aaaa  = 12 [\n"
+   "  (aaa) = ,\n"
+   "  (bb) = {\n"
+   "a: true,\n"
+   ": true\n"
+   "  }\n"
+   "];");
 }
 
 TEST_F(FormatTestProto, DoesntWrapFileOptions) {


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


r297694 - Fix crash with interrupt attribute on ARM.

2017-03-13 Thread Eli Friedman via cfe-commits
Author: efriedma
Date: Mon Mar 13 19:18:29 2017
New Revision: 297694

URL: http://llvm.org/viewvc/llvm-project?rev=297694=rev
Log:
Fix crash with interrupt attribute on ARM.

An indirect call has no associated function declaration.


Modified:
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/arm-interrupt-attr.c

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=297694=297693=297694=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Mar 13 19:18:29 2017
@@ -5387,7 +5387,7 @@ Sema::BuildResolvedCallExpr(Expr *Fn, Na
   // but can be very challenging to debug.
   if (auto *Caller = getCurFunctionDecl())
 if (Caller->hasAttr())
-  if (!FDecl->hasAttr())
+  if (!FDecl || !FDecl->hasAttr())
 Diag(Fn->getExprLoc(), diag::warn_arm_interrupt_calling_convention);
 
   // Promote the function operand.

Modified: cfe/trunk/test/Sema/arm-interrupt-attr.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/arm-interrupt-attr.c?rev=297694=297693=297694=diff
==
--- cfe/trunk/test/Sema/arm-interrupt-attr.c (original)
+++ cfe/trunk/test/Sema/arm-interrupt-attr.c Mon Mar 13 19:18:29 2017
@@ -28,3 +28,8 @@ __attribute__((interrupt("IRQ"))) void c
   callee1(); // expected-warning {{call to function without interrupt 
attribute could clobber interruptee's VFP registers}}
   callee2();
 }
+
+void (*callee3)();
+__attribute__((interrupt("IRQ"))) void caller3() {
+  callee3(); // expected-warning {{call to function without interrupt 
attribute could clobber interruptee's VFP registers}}
+}


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


r297693 - Reapply [VFS] Ignore broken symlinks in the directory iterator.

2017-03-13 Thread Juergen Ributzka via cfe-commits
Author: ributzka
Date: Mon Mar 13 19:14:40 2017
New Revision: 297693

URL: http://llvm.org/viewvc/llvm-project?rev=297693=rev
Log:
Reapply [VFS] Ignore broken symlinks in the directory iterator.

Modified the tests to accept any iteration order, to run only on Unix, and added
additional error reporting to investigate SystemZ bot issue.

The VFS directory iterator and recursive directory iterator behave differently
from the LLVM counterparts. Once the VFS iterators hit a broken symlink they
immediately abort. The LLVM counterparts don't stat entries unless they have to
descend into the next directory, which allows to recover from this issue by
clearing the error code and skipping to the next entry.

This change adds similar behavior to the VFS iterators. There should be no
change in current behavior in the current CLANG source base, because all
clients have loop exit conditions that also check the error code.

This fixes rdar://problem/30934619.

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

Modified:
cfe/trunk/include/clang/Basic/VirtualFileSystem.h
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp

Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=297693=297692=297693=diff
==
--- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)
+++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Mon Mar 13 19:14:40 2017
@@ -161,7 +161,7 @@ public:
   directory_iterator (std::error_code ) {
 assert(Impl && "attempting to increment past end");
 EC = Impl->increment();
-if (EC || !Impl->CurrentEntry.isStatusKnown())
+if (!Impl->CurrentEntry.isStatusKnown())
   Impl.reset(); // Normalize the end iterator to Impl == nullptr.
 return *this;
   }

Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=297693=297692=297693=diff
==
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Mon Mar 13 19:14:40 2017
@@ -244,8 +244,7 @@ public:
 if (!EC && Iter != llvm::sys::fs::directory_iterator()) {
   llvm::sys::fs::file_status S;
   EC = Iter->status(S);
-  if (!EC)
-CurrentEntry = Status::copyWithNewName(S, Iter->path());
+  CurrentEntry = Status::copyWithNewName(S, Iter->path());
 }
   }
 
@@ -1856,7 +1855,7 @@ vfs::recursive_directory_iterator::recur
std::error_code )
 : FS(_) {
   directory_iterator I = FS->dir_begin(Path, EC);
-  if (!EC && I != directory_iterator()) {
+  if (I != directory_iterator()) {
 State = std::make_shared();
 State->push(I);
   }
@@ -1869,8 +1868,6 @@ recursive_directory_iterator::increment(
   vfs::directory_iterator End;
   if (State->top()->isDirectory()) {
 vfs::directory_iterator I = FS->dir_begin(State->top()->getName(), EC);
-if (EC)
-  return *this;
 if (I != End) {
   State->push(I);
   return *this;

Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=297693=297692=297693=diff
==
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Mon Mar 13 19:14:40 2017
@@ -305,6 +305,22 @@ struct ScopedDir {
   }
   operator StringRef() { return Path.str(); }
 };
+
+struct ScopedLink {
+  SmallString<128> Path;
+  ScopedLink(const Twine , const Twine ) {
+Path = From.str();
+std::error_code EC = sys::fs::create_link(To, From);
+if (EC)
+  Path = "";
+EXPECT_FALSE(EC);
+  }
+  ~ScopedLink() {
+if (Path != "")
+  EXPECT_FALSE(llvm::sys::fs::remove(Path.str()));
+  }
+  operator StringRef() { return Path.str(); }
+};
 } // end anonymous namespace
 
 TEST(VirtualFileSystemTest, BasicRealFSIteration) {
@@ -334,6 +350,36 @@ TEST(VirtualFileSystemTest, BasicRealFSI
   EXPECT_EQ(vfs::directory_iterator(), I);
 }
 
+#ifdef LLVM_ON_UNIX
+TEST(VirtualFileSystemTest, BrokenSymlinkRealFSIteration) {
+  ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  IntrusiveRefCntPtr FS = vfs::getRealFileSystem();
+
+  ScopedLink _a("no_such_file", TestDirectory + "/a");
+  ScopedDir _b(TestDirectory + "/b");
+  ScopedLink _c("no_such_file", TestDirectory + "/c");
+
+  std::error_code EC;
+  for (vfs::directory_iterator I = FS->dir_begin(Twine(TestDirectory), EC), E;
+   I != E; I.increment(EC)) {
+// Skip broken symlinks.
+if (EC == std::errc::no_such_file_or_directory) {
+  EC = std::error_code();
+  

[PATCH] D30920: Do not pass -Os and -Oz to the Gold plugin

2017-03-13 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added a comment.

Interested in pcc's thoughts, as https://bugs.llvm.org/show_bug.cgi?id=32155 
mentioned you already discussed with him. Note that some of the passes that 
check PassManagerBuilder::sizeLevel are added during the ThinLTO back end (e.g. 
populateModulePassManager which checks sizeLevel is invoked by 
populateThinLTOPassManager). Until everything is converted to using size 
attributes, it seems like a correct fix for the bug is to accept these options 
in the gold-plugin and pass through the LTO API to the PassManagerBuilder.




Comment at: lib/Driver/ToolChains/CommonArgs.cpp:369
 if (A->getOption().matches(options::OPT_O4) ||
-A->getOption().matches(options::OPT_Ofast))
+A->getOption().matches(options::OPT_Ofast)) {
   OOpt = "3";

Remove added "{"



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:377
+OOpt = OptLevel;
+} else if (A->getOption().matches(options::OPT_O0)) {
   OOpt = "0";

Ditto about unnecessary "{"


https://reviews.llvm.org/D30920



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


[PATCH] D30923: Warn on enum assignment to bitfields that can't fit all values

2017-03-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision.

This adds -Wbitfield-enum-conversion, which warns on implicit
conversions that happen on bitfield assignment that change the value of
some enumerators.

Values of enum type typically take on a very small range of values, so
they are frequently stored in bitfields. Unfortunately, there is no
convenient way to calculate the minimum number of bits necessary to
store all possible values at compile time, so users usually hard code a
bitwidth that works today and widen it as necessary to pass basic
testing and validation. This is very error-prone, and leads to stale
widths as enums grow. This warning aims to catch such bugs.

This would have found two real bugs in clang and two instances of
questionable code. See r297680 and r297654 for the full description of
the issues.

This warning is currently disabled by default while we investigate its
usefulness outside of LLVM.

The major cause of false positives with this warning is this kind of
enum:

  enum E { W, X, Y, Z, SENTINEL_LAST };

The last enumerator is an invalid value used to validate inputs or size
an array. Depending on the prevalance of this style of enum across a
codebase, this warning may be more or less feasible to deploy. It also
has trouble on sentinel values such as ~0U.


https://reviews.llvm.org/D30923

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Sema/warn-bitfield-enum-conversion.c

Index: test/Sema/warn-bitfield-enum-conversion.c
===
--- /dev/null
+++ test/Sema/warn-bitfield-enum-conversion.c
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -verify %s -Wbitfield-enum-conversion
+
+enum TwoBits { Hi1 = 3 } two_bits;
+enum TwoBitsSigned { Lo2 = -2, Hi2 = 1 } two_bits_signed;
+enum ThreeBits { Hi3 = 7 } three_bits;
+enum ThreeBitsSigned { Lo4 = -4, Hi4 = 3 } three_bits_signed;
+
+struct Foo {
+  unsigned two_bits : 2;
+  int two_bits_signed : 2;
+  unsigned three_bits : 3;
+  int three_bits_signed : 3;
+
+  enum ThreeBits three_bits_enum : 3;
+};
+
+void f() {
+  struct Foo f;
+
+  f.two_bits = two_bits;
+  f.two_bits = two_bits_signed;// expected-warning {{negative enumerators}}
+  f.two_bits = three_bits; // expected-warning {{not wide enough}}
+  f.two_bits = three_bits_signed;  // expected-warning {{negative enumerators}} expected-warning {{not wide enough}}
+
+  f.two_bits_signed = two_bits;// expected-warning {{needs an extra bit}}
+  f.two_bits_signed = two_bits_signed;
+  f.two_bits_signed = three_bits;  // expected-warning {{not wide enough}}
+  f.two_bits_signed = three_bits_signed;   // expected-warning {{not wide enough}}
+
+  f.three_bits = two_bits;
+  f.three_bits = two_bits_signed;  // expected-warning {{negative enumerators}}
+  f.three_bits = three_bits;
+  f.three_bits = three_bits_signed;// expected-warning {{negative enumerators}}
+
+  f.three_bits_signed = two_bits;
+  f.three_bits_signed = two_bits_signed;
+  f.three_bits_signed = three_bits;// expected-warning {{needs an extra bit}}
+  f.three_bits_signed = three_bits_signed;
+
+  f.three_bits_enum = three_bits;  // expected-warning {{needs an extra bit}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -8729,13 +8729,42 @@
 return false;
 
   Expr *OriginalInit = Init->IgnoreParenImpCasts();
+  unsigned FieldWidth = Bitfield->getBitWidthValue(S.Context);
 
   llvm::APSInt Value;
-  if (!OriginalInit->EvaluateAsInt(Value, S.Context, Expr::SE_AllowSideEffects))
+  if (!OriginalInit->EvaluateAsInt(Value, S.Context,
+   Expr::SE_AllowSideEffects)) {
+// The RHS is not constant.  If the RHS has an enum type, make sure the
+// bitfield is wide enough to hold all the values of the enum without
+// truncation.
+if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+  EnumDecl *ED = EnumTy->getDecl();
+  bool SignedBitfield = BitfieldType->isSignedIntegerType();
+
+  // Check if this might store negative values into an unsigned bitfield.
+  if (!SignedBitfield && ED->getNumNegativeBits() > 0) {
+S.Diag(InitLoc, diag::warn_unsigned_bitfield_assigned_signed_enum)
+<< Bitfield << ED;
+  }
+
+  // Check for sufficient width.
+  if (ED->getNumPositiveBits() > FieldWidth ||
+  ED->getNumNegativeBits() > FieldWidth) {
+S.Diag(InitLoc, diag::warn_bitfield_too_small_for_enum)
+<< Bitfield << ED;
+  } else if (SignedBitfield && ED->getNumPositiveBits() == FieldWidth) {
+// If the bitfield type is signed and the positive field widths are
+// equal, values with the high bit set will become negative, which is
+// usually unintended.
+   

[PATCH] D30922: [Builtins] Synchronize the definition of fma/fmaf/fmal in Builtins.def with the implementation in CGBuiltins.cpp

2017-03-13 Thread Craig Topper via Phabricator via cfe-commits
craig.topper created this revision.

The fma libcalls are defined in Builtins.def using the 'e' attribute that says 
that its only const if -fno-math-errno. It was apparently marked this way 
because that's what the posix spec says. This determines whether the call gets 
marked as const or not in SemaDecl.cpp. But in CGBuiltins.cpp we are blindly 
emitting an intrinsic no matter whether the call is const or not.

This patch changes Builtins.def to match the current CGBuiltins behavior and 
updates CGBuiltins to check if the function is const. So if anyone ever changes 
Builtins.def in the future it will do the right thing.


https://reviews.llvm.org/D30922

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp


Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -1898,6 +1898,8 @@
   case Builtin::BI__builtin_fmaf:
   case Builtin::BI__builtin_fmal: {
 // Rewrite fma to intrinsic.
+if (!FD->hasAttr())
+  break;
 Value *FirstArg = EmitScalarExpr(E->getArg(0));
 llvm::Type *ArgType = FirstArg->getType();
 Value *F = CGM.getIntrinsic(Intrinsic::fma, ArgType);
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -1067,9 +1067,10 @@
 LIBBUILTIN(floorf, "ff", "fnc", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(floorl, "LdLd", "fnc", "math.h", ALL_LANGUAGES)
 
-LIBBUILTIN(fma, "", "fne", "math.h", ALL_LANGUAGES)
-LIBBUILTIN(fmaf, "", "fne", "math.h", ALL_LANGUAGES)
-LIBBUILTIN(fmal, "LdLdLdLd", "fne", "math.h", ALL_LANGUAGES)
+// Posix standard says this updates errno, but we always emit an intrinsic.
+LIBBUILTIN(fma, "", "fnc", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(fmaf, "", "fnc", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(fmal, "LdLdLdLd", "fnc", "math.h", ALL_LANGUAGES)
 
 LIBBUILTIN(fmax, "ddd", "fnc", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(fmaxf, "fff", "fnc", "math.h", ALL_LANGUAGES)


Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -1898,6 +1898,8 @@
   case Builtin::BI__builtin_fmaf:
   case Builtin::BI__builtin_fmal: {
 // Rewrite fma to intrinsic.
+if (!FD->hasAttr())
+  break;
 Value *FirstArg = EmitScalarExpr(E->getArg(0));
 llvm::Type *ArgType = FirstArg->getType();
 Value *F = CGM.getIntrinsic(Intrinsic::fma, ArgType);
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -1067,9 +1067,10 @@
 LIBBUILTIN(floorf, "ff", "fnc", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(floorl, "LdLd", "fnc", "math.h", ALL_LANGUAGES)
 
-LIBBUILTIN(fma, "", "fne", "math.h", ALL_LANGUAGES)
-LIBBUILTIN(fmaf, "", "fne", "math.h", ALL_LANGUAGES)
-LIBBUILTIN(fmal, "LdLdLdLd", "fne", "math.h", ALL_LANGUAGES)
+// Posix standard says this updates errno, but we always emit an intrinsic.
+LIBBUILTIN(fma, "", "fnc", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(fmaf, "", "fnc", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(fmal, "LdLdLdLd", "fnc", "math.h", ALL_LANGUAGES)
 
 LIBBUILTIN(fmax, "ddd", "fnc", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(fmaxf, "fff", "fnc", "math.h", ALL_LANGUAGES)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: LLVM Lab SVN mirror is behind

2017-03-13 Thread Galina Kistanova via cfe-commits
A quick update.

The SVN mirror got corrupted by r297634. Svnsync does not like huge commits.
I'm in the middle of restoring and synch-ing up the mirror. Too soon to
give any ETA, unfortunately.

Thank you for your patience.

Thanks

Galina



On Mon, Mar 13, 2017 at 12:36 PM, Galina Kistanova 
wrote:

> Hello everyone,
>
> The SVN mirror in the LLVM Lab seems behind with the changes.
> I'm looking at the issue.
>
> Thank you for understanding.
>
> Thanks
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D30809: [coroutines] Add codegen for await and yield expressions

2017-03-13 Thread David Majnemer via Phabricator via cfe-commits
majnemer added inline comments.



Comment at: lib/CodeGen/CGCoroutine.cpp:85
+  unsigned No = 0;
+  StringRef AwaitKindStr = 0;
+  switch (Kind) {

I'd just let the default constructor do its thing.


https://reviews.llvm.org/D30809



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


[PATCH] D30920: Do not pass -Os and -Oz to the Gold plugin

2017-03-13 Thread Pirama Arumuga Nainar via Phabricator via cfe-commits
pirama created this revision.
Herald added a subscriber: mehdi_amini.

Address PR32155: Skip passing -Os and -Oz to the Gold plugin using
-plugin-opt.


https://reviews.llvm.org/D30920

Files:
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/gold-lto.c


Index: test/Driver/gold-lto.c
===
--- test/Driver/gold-lto.c
+++ test/Driver/gold-lto.c
@@ -26,3 +26,9 @@
 // RUN: %clang -target i686-linux-android -### %t.o -flto 2>&1 \
 // RUN: | FileCheck %s --check-prefix=CHECK-X86-ANDROID
 // CHECK-X86-ANDROID: "-plugin" "{{.*}}/LLVMgold.so"
+//
+// Test that -Os and -Oz are not passed to the plugin
+// RUN: %clang -### %t.o -flto -Os 2>&1 \
+// RUN: | FileCheck %s --implicit-check-not "-plugin-opt=Os"
+// RUN: %clang -### %t.o -flto -Os 2>&1 \
+// RUN: | FileCheck %s --implicit-check-not "-plugin-opt=Oz"
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -366,12 +366,17 @@
   if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
 StringRef OOpt;
 if (A->getOption().matches(options::OPT_O4) ||
-A->getOption().matches(options::OPT_Ofast))
+A->getOption().matches(options::OPT_Ofast)) {
   OOpt = "3";
-else if (A->getOption().matches(options::OPT_O))
-  OOpt = A->getValue();
-else if (A->getOption().matches(options::OPT_O0))
+} else if (A->getOption().matches(options::OPT_O)) {
+  StringRef OptLevel = A->getValue();
+  // Do not pass optimization levels pertaining to code size to the plugin.
+  // They are captured by corresponding function attributes.
+  if (!OptLevel.equals("s") && !OptLevel.equals("z"))
+OOpt = OptLevel;
+} else if (A->getOption().matches(options::OPT_O0)) {
   OOpt = "0";
+}
 if (!OOpt.empty())
   CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=O") + OOpt));
   }


Index: test/Driver/gold-lto.c
===
--- test/Driver/gold-lto.c
+++ test/Driver/gold-lto.c
@@ -26,3 +26,9 @@
 // RUN: %clang -target i686-linux-android -### %t.o -flto 2>&1 \
 // RUN: | FileCheck %s --check-prefix=CHECK-X86-ANDROID
 // CHECK-X86-ANDROID: "-plugin" "{{.*}}/LLVMgold.so"
+//
+// Test that -Os and -Oz are not passed to the plugin
+// RUN: %clang -### %t.o -flto -Os 2>&1 \
+// RUN: | FileCheck %s --implicit-check-not "-plugin-opt=Os"
+// RUN: %clang -### %t.o -flto -Os 2>&1 \
+// RUN: | FileCheck %s --implicit-check-not "-plugin-opt=Oz"
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -366,12 +366,17 @@
   if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
 StringRef OOpt;
 if (A->getOption().matches(options::OPT_O4) ||
-A->getOption().matches(options::OPT_Ofast))
+A->getOption().matches(options::OPT_Ofast)) {
   OOpt = "3";
-else if (A->getOption().matches(options::OPT_O))
-  OOpt = A->getValue();
-else if (A->getOption().matches(options::OPT_O0))
+} else if (A->getOption().matches(options::OPT_O)) {
+  StringRef OptLevel = A->getValue();
+  // Do not pass optimization levels pertaining to code size to the plugin.
+  // They are captured by corresponding function attributes.
+  if (!OptLevel.equals("s") && !OptLevel.equals("z"))
+OOpt = OptLevel;
+} else if (A->getOption().matches(options::OPT_O0)) {
   OOpt = "0";
+}
 if (!OOpt.empty())
   CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=O") + OOpt));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r297680 - Widen AST bitfields too small to represent all enumerators

2017-03-13 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Mon Mar 13 17:33:04 2017
New Revision: 297680

URL: http://llvm.org/viewvc/llvm-project?rev=297680=rev
Log:
Widen AST bitfields too small to represent all enumerators

All of these were found by a new warning that I am prototyping,
-Wbitfield-enum-conversion.

Stmt::ExprBits::ObjectKind - This was not wide enough to represent
OK_ObjSubscript, so this was a real, true positive bug.

ObjCDeclSpec::objCDeclQualifier - On Windows, setting DQ_CSNullability
would cause the bitfield to become negative because enum types are
always implicitly 'int' there. This would probably never be noticed
because this is a flag-style enum, so we only ever test one bit at a
time. Switching to 'unsigned' also makes this type pack smaller on
Windows.

FunctionDecl::SClass - Technically, we only need two bits for all valid
function storage classes. Functions can never have automatic or register
storage class. This seems a bit too clever, and we have a bit to spare,
so widening the bitfield seems like the best way to pacify the warning.
You could classify this as a false positive, but widening the bitfield
defends us from invalid ASTs.

Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/include/clang/Sema/DeclSpec.h

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=297680=297679=297680=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Mon Mar 13 17:33:04 2017
@@ -1605,7 +1605,7 @@ private:
 
   // FIXME: This can be packed into the bitfields in DeclContext.
   // NOTE: VC++ packs bitfields poorly if the types differ.
-  unsigned SClass : 2;
+  unsigned SClass : 3;
   unsigned IsInline : 1;
   unsigned IsInlineSpecified : 1;
 protected:

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=297680=297679=297680=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Mon Mar 13 17:33:04 2017
@@ -115,6 +115,7 @@ protected:
 ExprBits.InstantiationDependent = ID;
 ExprBits.ValueKind = VK;
 ExprBits.ObjectKind = OK;
+assert(ExprBits.ObjectKind == OK && "truncated kind");
 ExprBits.ContainsUnexpandedParameterPack = ContainsUnexpandedParameterPack;
 setType(T);
   }

Modified: cfe/trunk/include/clang/AST/Stmt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=297680=297679=297680=diff
==
--- cfe/trunk/include/clang/AST/Stmt.h (original)
+++ cfe/trunk/include/clang/AST/Stmt.h Mon Mar 13 17:33:04 2017
@@ -127,13 +127,13 @@ protected:
 unsigned : NumStmtBits;
 
 unsigned ValueKind : 2;
-unsigned ObjectKind : 2;
+unsigned ObjectKind : 3;
 unsigned TypeDependent : 1;
 unsigned ValueDependent : 1;
 unsigned InstantiationDependent : 1;
 unsigned ContainsUnexpandedParameterPack : 1;
   };
-  enum { NumExprBits = 16 };
+  enum { NumExprBits = 17 };
 
   class CharacterLiteralBitfields {
 friend class CharacterLiteral;
@@ -350,6 +350,8 @@ protected:
 
 public:
   Stmt(StmtClass SC) {
+static_assert(sizeof(*this) == sizeof(void *),
+  "changing bitfields changed sizeof(Stmt)");
 static_assert(sizeof(*this) % alignof(void *) == 0,
   "Insufficient alignment!");
 StmtBits.sClass = SC;

Modified: cfe/trunk/include/clang/Sema/DeclSpec.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/DeclSpec.h?rev=297680=297679=297680=diff
==
--- cfe/trunk/include/clang/Sema/DeclSpec.h (original)
+++ cfe/trunk/include/clang/Sema/DeclSpec.h Mon Mar 13 17:33:04 2017
@@ -819,7 +819,9 @@ public:
 : objcDeclQualifier(DQ_None), PropertyAttributes(DQ_PR_noattr),
   Nullability(0), GetterName(nullptr), SetterName(nullptr) { }
 
-  ObjCDeclQualifier getObjCDeclQualifier() const { return objcDeclQualifier; }
+  ObjCDeclQualifier getObjCDeclQualifier() const {
+return (ObjCDeclQualifier)objcDeclQualifier;
+  }
   void setObjCDeclQualifier(ObjCDeclQualifier DQVal) {
 objcDeclQualifier = (ObjCDeclQualifier) (objcDeclQualifier | DQVal);
   }
@@ -869,7 +871,7 @@ private:
   // FIXME: These two are unrelated and mutually exclusive. So perhaps
   // we can put them in a union to reflect their mutual exclusivity
   // (space saving is negligible).
-  ObjCDeclQualifier objcDeclQualifier : 7;
+  unsigned objcDeclQualifier : 7;
 
   // NOTE: VC++ treats enums as signed, avoid using ObjCPropertyAttributeKind
   unsigned PropertyAttributes : 15;



r297681 - Fix -Wunused-lambda-capture warning in new code

2017-03-13 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Mon Mar 13 17:33:07 2017
New Revision: 297681

URL: http://llvm.org/viewvc/llvm-project?rev=297681=rev
Log:
Fix -Wunused-lambda-capture warning in new code

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

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=297681=297680=297681=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Mon Mar 13 17:33:07 2017
@@ -177,7 +177,7 @@ namespace clang {
   Err = Linker::linkModules(
   *getModule(), std::move(LM.Module), LM.LinkFlags,
   [](llvm::Module , const llvm::StringSet<> ) {
-internalizeModule(M, [, ](const llvm::GlobalValue ) {
+internalizeModule(M, [](const llvm::GlobalValue ) {
   return !GV.hasName() || (GVS.count(GV.getName()) == 0);
 });
   });


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


[PATCH] D30733: [Driver] Add arch-specific rpath for libc++

2017-03-13 Thread Pirama Arumuga Nainar via Phabricator via cfe-commits
pirama added inline comments.



Comment at: lib/Driver/ToolChain.cpp:652
+// libc++ may be installed per arch.
+addArchSpecificRPath(*this, Args, CmdArgs);
 break;

Hahnfeld wrote:
> pirama wrote:
> > `addArchSpecificRPath` is a static function in Tools.cpp and isn't visible 
> > here.
> No, it's not since recent refactoring. I do compile test my changes usually 
> ;-)
My bad.  I'd have found about it if I actually sync to ToT more often than once 
a week.


https://reviews.llvm.org/D30733



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


[PATCH] D30700: [Driver] Always add arch-specific-subdir to -rpath

2017-03-13 Thread Pirama Arumuga Nainar via Phabricator via cfe-commits
pirama updated this revision to Diff 91635.
pirama added a comment.

*Actually* add the command line flags.


https://reviews.llvm.org/D30700

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/arch-specific-libdir-rpath.c
  test/Driver/arch-specific-libdir.c
  test/lit.cfg

Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -397,10 +397,6 @@
 if config.host_triple == config.target_triple:
 config.available_features.add("native")
 
-# Test Driver/arch-specific-libdir-rpath.c is restricted to x86_64-linux
-if re.match(r'^x86_64.*-linux', config.target_triple):
-config.available_features.add("x86_64-linux")
-
 # Case-insensitive file system
 def is_filesystem_case_insensitive():
 handle, path = tempfile.mkstemp(prefix='case-test', dir=config.test_exec_root)
Index: test/Driver/arch-specific-libdir.c
===
--- test/Driver/arch-specific-libdir.c
+++ test/Driver/arch-specific-libdir.c
@@ -1,8 +1,6 @@
 // Test that the driver adds an arch-specific subdirectory in
 // {RESOURCE_DIR}/lib/linux to the search path.
 //
-// REQUIRES: linux
-//
 // RUN: %clang %s -### 2>&1 -target i386-unknown-linux \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
 // RUN:   | FileCheck --check-prefixes=FILEPATH,ARCHDIR-i386 %s
Index: test/Driver/arch-specific-libdir-rpath.c
===
--- test/Driver/arch-specific-libdir-rpath.c
+++ test/Driver/arch-specific-libdir-rpath.c
@@ -1,50 +1,85 @@
 // Test that the driver adds an arch-specific subdirectory in
-// {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath' for native
-// compilations.
+// {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath'
 //
-// -rpath only gets added during native compilation.  To keep the test simple,
-// just test for x86_64-linux native compilation.
-// REQUIRES: x86_64-linux
+// Test the default behavior when neither -frtlib-add-rpath nor
+// -fno-rtlib-add-rpath is specified, which is to skip -rpath
+// RUN: %clang %s -### 2>&1 -target x86_64-linux \
+// RUN: -fsanitize=address -shared-libasan \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
+// RUN: -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH-X86_64 %s
+//
+// Test that -rpath is not added under -fno-rtlib-add-rpath even if other
+// conditions are met.
+// RUN: %clang %s -### 2>&1 -target x86_64-linux \
+// RUN: -fsanitize=address -shared-libasan \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
+// RUN: -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH-X86_64 %s
+//
+// Test that -rpath is added only under the right circumstance even if
+// -frtlib-add-rpath is specified.
 //
 // Add LIBPATH but no RPATH for -fsanitizer=address w/o -shared-libasan
-// RUN: %clang %s -### 2>&1 -fsanitize=undefined \
+// RUN: %clang %s -### 2>&1 -target x86_64-linux -fsanitize=undefined \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
+// RUN: -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH %s
+//
+// Add LIBPATH but no RPATH for -fsanitizer=address w/o -shared-libasan
+// RUN: %clang %s -### 2>&1 -target x86_64-linux -fsanitize=undefined \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,NO-RPATH %s
+// RUN: -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH %s
 //
 // Add LIBPATH, RPATH for -fsanitize=address -shared-libasan
 // RUN: %clang %s -### 2>&1 -target x86_64-linux \
 // RUN: -fsanitize=address -shared-libasan \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
+// RUN: -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s
+//
+// Add LIBPATH, RPATH for -fsanitize=address -shared-libasan on aarch64
+// RUN: %clang %s -### 2>&1 -target aarch64-linux \
+// RUN: -fsanitize=address -shared-libasan \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
+// RUN: -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-AArch64,RPATH-AArch64 %s
 //
 // Add LIBPATH, RPATH with -fsanitize=address for Android
 // RUN: %clang %s -### 2>&1 -target x86_64-linux-android -fsanitize=address \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
+// RUN: -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s
 //
 // Add LIBPATH, RPATH for OpenMP
-// RUN: %clang %s -### 2>&1 -fopenmp \
+// RUN: %clang %s 

[PATCH] D30700: [Driver] Always add arch-specific-subdir to -rpath

2017-03-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

In https://reviews.llvm.org/D30700#695358, @Hahnfeld wrote:

> No build system will ever set `-frtlib-add-rpath` to enable this "feature". 
> I'm for keeping this opt-out until we have configuration files to set this by 
> default. Making it opt-in would weaken its main reason of existence: Not to 
> break simple binaries for the user, and we can just drop it.


I don't agree. If we want to be good citizens on Linux, what's supposed to 
happen is that we install our shared libraries into /usr/lib/${distro_target}, 
which is what GCC does with its shared compiler runtime libraries. GCC does not 
add system-specific absolute rpaths to the binaries it produces.


https://reviews.llvm.org/D30700



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


[PATCH] D30700: [Driver] Always add arch-specific-subdir to -rpath

2017-03-13 Thread Pirama Arumuga Nainar via Phabricator via cfe-commits
pirama updated this revision to Diff 91633.
pirama added a comment.

- Rebase
- Added command line flag and updated tests.


https://reviews.llvm.org/D30700

Files:
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/arch-specific-libdir-rpath.c
  test/Driver/arch-specific-libdir.c
  test/lit.cfg

Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -397,10 +397,6 @@
 if config.host_triple == config.target_triple:
 config.available_features.add("native")
 
-# Test Driver/arch-specific-libdir-rpath.c is restricted to x86_64-linux
-if re.match(r'^x86_64.*-linux', config.target_triple):
-config.available_features.add("x86_64-linux")
-
 # Case-insensitive file system
 def is_filesystem_case_insensitive():
 handle, path = tempfile.mkstemp(prefix='case-test', dir=config.test_exec_root)
Index: test/Driver/arch-specific-libdir.c
===
--- test/Driver/arch-specific-libdir.c
+++ test/Driver/arch-specific-libdir.c
@@ -1,8 +1,6 @@
 // Test that the driver adds an arch-specific subdirectory in
 // {RESOURCE_DIR}/lib/linux to the search path.
 //
-// REQUIRES: linux
-//
 // RUN: %clang %s -### 2>&1 -target i386-unknown-linux \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
 // RUN:   | FileCheck --check-prefixes=FILEPATH,ARCHDIR-i386 %s
Index: test/Driver/arch-specific-libdir-rpath.c
===
--- test/Driver/arch-specific-libdir-rpath.c
+++ test/Driver/arch-specific-libdir-rpath.c
@@ -1,50 +1,53 @@
 // Test that the driver adds an arch-specific subdirectory in
-// {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath' for native
-// compilations.
-//
-// -rpath only gets added during native compilation.  To keep the test simple,
-// just test for x86_64-linux native compilation.
-// REQUIRES: x86_64-linux
+// {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath'
 //
 // Add LIBPATH but no RPATH for -fsanitizer=address w/o -shared-libasan
-// RUN: %clang %s -### 2>&1 -fsanitize=undefined \
+// RUN: %clang %s -### 2>&1 -target x86_64-linux -fsanitize=undefined \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,NO-RPATH %s
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH %s
 //
 // Add LIBPATH, RPATH for -fsanitize=address -shared-libasan
 // RUN: %clang %s -### 2>&1 -target x86_64-linux \
 // RUN: -fsanitize=address -shared-libasan \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s
+//
+// Add LIBPATH, RPATH for -fsanitize=address -shared-libasan on aarch64
+// RUN: %clang %s -### 2>&1 -target aarch64-linux \
+// RUN: -fsanitize=address -shared-libasan \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-AArch64,RPATH-AArch64 %s
 //
 // Add LIBPATH, RPATH with -fsanitize=address for Android
 // RUN: %clang %s -### 2>&1 -target x86_64-linux-android -fsanitize=address \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s
 //
 // Add LIBPATH, RPATH for OpenMP
-// RUN: %clang %s -### 2>&1 -fopenmp \
+// RUN: %clang %s -### 2>&1 -target x86_64-linux -fopenmp \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s
 //
 // Add LIBPATH but no RPATH for ubsan (or any other sanitizer)
 // RUN: %clang %s -### 2>&1 -fsanitize=undefined \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,NO-RPATH %s
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH %s
 //
 // Add LIBPATH but no RPATH if no sanitizer or runtime is specified
 // RUN: %clang %s -### 2>&1 \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,NO-RPATH %s
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH %s
 //
 // Do not add LIBPATH or RPATH if arch-specific subdir doesn't exist
 // RUN: %clang %s -### 2>&1 \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,NO-LIBPATH,NO-RPATH %s
+// RUN:   | FileCheck --check-prefixes=RESDIR,NO-LIBPATH,NO-RPATH %s
 //
 //
-// FILEPATH: "-x" "c" "[[FILE_PATH:.*]]/{{.*}}.c"
-// LIBPATH: -L[[FILE_PATH]]/Inputs/resource_dir_with_arch_subdir/lib/linux/x86_64
-// RPATH: "-rpath" 

[PATCH] D30896: [Clang-tidy] add check misc-prefer-switch-for-enums

2017-03-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

I'm curious what kind of results you get when running this over a large code 
base. There are definitely times when using == or != for a specific value is 
*way* cleaner than using a switch statement, and I worry about this being so 
chatty that it needs to be disabled by default.

If it turns out to be very chatty, perhaps the check could be relaxed to only 
consider cases where you have multiple if/else if clauses (tuned via an option) 
for checking several values? At the very least, the check should not be 
triggered on an enumeration that has only one enumerator.


Repository:
  rL LLVM

https://reviews.llvm.org/D30896



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


[PATCH] D30915: Canonicalize the path provided by -fmodules-cache-path

2017-03-13 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno accepted this revision.
bruno added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: lib/Frontend/CompilerInvocation.cpp:1434
+
+  // Canonicalize -fmodules-cache-path before storing it.
+  SmallString<128> P(Args.getLastArgValue(OPT_fmodules_cache_path));

I would suggest using `FileManager::makeAbsolutePath`, but at this point looks 
like we haven't set up a FileManager yet.


https://reviews.llvm.org/D30915



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


[PATCH] D30766: Add support for attribute "enum_extensibility"

2017-03-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:1969
+  let Content = [{
+Attribute ``enum_extensibility`` is used to distinguish between enum 
definitions that are extensible and those that are not. The attribute can take 
either ``closed`` or ``open`` as an argument. ``closed`` indicates a variable 
of the enum type takes a value that corresponds to one of the enumerators 
listed in the enum definition or, when the enum is annotated with 
``flag_enum``, a value that can be constructed using values corresponding to 
the enumerators. ``open`` indicates a variable of the enum type can take any 
values allowed by the standard and instructs clang to be more lenient when 
issuing warnings.
+  }];

Breaking this up into multiple lines would be appreciated (we aren't strict 
about the 80 col limit in this file, but it should be readable still).

Also, adding some examples would be appreciated.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2383-2384
   "%0 attribute requires integer constant between %1 and %2 inclusive">;
+def err_attribute_argument_not_supported : Error<
+  "%0 attribute argument not supported: %1">;
 def err_init_priority_object_attr : Error<

I'd rather see this defined next to `warn_attribute_type_not_supported` as `def 
err_attribute_type_not_supported : 
Error;`



Comment at: test/Sema/enum-attr.c:27
+
+enum __attribute__((enum_extensibility(arg1))) EnumInvalidArg { // 
expected-warning{{'enum_extensibility' attribute argument not supported: 
'arg1'}}
+  G

ahatanak wrote:
> arphaman wrote:
> > Should this be an error instead?
> Yes, I agree.
I'm not opposed to it, but we do treat it as a warning for every other 
attribute (and just ignore the attribute), FWIW.



Comment at: test/SemaCXX/enum-attr.cpp:108
+  }
+}

Missing tests for the attribute being applied to something other than an enum, 
or taking too few/many arguments.


https://reviews.llvm.org/D30766



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


[PATCH] D30915: Canonicalize the path provided by -fmodules-cache-path

2017-03-13 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl created this revision.

This fixes lookup mismatches that could previously happen when the module cache 
path contained a '/./' component.
In combination with https://reviews.llvm.org/D28299 this bug can cause a 
use-after-free.

rdar://problem/30413458


https://reviews.llvm.org/D30915

Files:
  lib/Frontend/CompilerInvocation.cpp
  test/Modules/Inputs/outofdate-rebuild/AppKit.h
  test/Modules/Inputs/outofdate-rebuild/Cocoa.h
  test/Modules/Inputs/outofdate-rebuild/CoreText.h
  test/Modules/Inputs/outofdate-rebuild/CoreVideo.h
  test/Modules/Inputs/outofdate-rebuild/Foundation.h
  test/Modules/Inputs/outofdate-rebuild/module.modulemap
  test/Modules/modules-cache-path-canonicalization.m

Index: test/Modules/modules-cache-path-canonicalization.m
===
--- /dev/null
+++ test/Modules/modules-cache-path-canonicalization.m
@@ -0,0 +1,30 @@
+// RUN: rm -rf %t/cache %T/rel
+
+// This testcase reproduces a use-after-free after looking up a PCM in
+// a non-canonical modules-cache-path.
+//
+// Prime the module cache (note the '.' in the path).
+// RUN: %clang_cc1 -fdisable-module-hash -fmodules-cache-path=%t/./cache \
+// RUN:   -fmodules -fimplicit-module-maps -I %S/Inputs/outofdate-rebuild \
+// RUN:   %s -fsyntax-only
+//
+// Force a module to be rebuilt by creating a conflict.
+// RUN: echo "@import CoreText;" > %t.m
+// RUN: %clang_cc1 -DMISMATCH -Werror -fdisable-module-hash \
+// RUN:   -fmodules-cache-path=%t/./cache -fmodules -fimplicit-module-maps \
+// RUN:   -I %S/Inputs/outofdate-rebuild %t.m -fsyntax-only
+//
+// Rebuild.
+// RUN: %clang_cc1 -fdisable-module-hash -fmodules-cache-path=%t/./cache \
+// RUN:   -fmodules -fimplicit-module-maps -I %S/Inputs/outofdate-rebuild \
+// RUN:   %s -fsyntax-only
+
+
+// Unrelated to the above: Check that a relative path is resolved correctly.
+//
+// RUN: %clang_cc1 -working-directory %T/rel -fmodules-cache-path=./cache \
+// RUN:   -fmodules -fimplicit-module-maps -I %S/Inputs/outofdate-rebuild \
+// RUN:   -fdisable-module-hash %t.m -fsyntax-only -Rmodule-build 2>&1 \
+// RUN:   | FileCheck %s
+// CHECK: /rel/cache/CoreText.pcm
+@import Cocoa;
Index: test/Modules/Inputs/outofdate-rebuild/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/outofdate-rebuild/module.modulemap
@@ -0,0 +1,19 @@
+module Cocoa {
+  header "Cocoa.h"
+}
+
+module AppKit {
+  header "AppKit.h"
+}
+
+module CoreText {
+  header "CoreText.h"
+}
+
+module Foundation {
+  header "Foundation.h"
+}
+
+module CoreVideo {
+  header "CoreVideo.h"
+}
Index: test/Modules/Inputs/outofdate-rebuild/Foundation.h
===
--- /dev/null
+++ test/Modules/Inputs/outofdate-rebuild/Foundation.h
@@ -0,0 +1,3 @@
+// Foundation
+#import "CoreText.h"
+struct D { int i; };
Index: test/Modules/Inputs/outofdate-rebuild/CoreVideo.h
===
--- /dev/null
+++ test/Modules/Inputs/outofdate-rebuild/CoreVideo.h
@@ -0,0 +1,3 @@
+// CoreVideo
+#import "Foundation.h" // Foundation
+struct E { int i; };
Index: test/Modules/Inputs/outofdate-rebuild/CoreText.h
===
--- /dev/null
+++ test/Modules/Inputs/outofdate-rebuild/CoreText.h
@@ -0,0 +1 @@
+struct C { int i; };
Index: test/Modules/Inputs/outofdate-rebuild/Cocoa.h
===
--- /dev/null
+++ test/Modules/Inputs/outofdate-rebuild/Cocoa.h
@@ -0,0 +1,5 @@
+// Cocoa
+#import "Foundation.h"
+#import "AppKit.h"
+
+struct A {  int i; };
Index: test/Modules/Inputs/outofdate-rebuild/AppKit.h
===
--- /dev/null
+++ test/Modules/Inputs/outofdate-rebuild/AppKit.h
@@ -0,0 +1,3 @@
+// AppKit
+#import "CoreVideo.h" // CoreVideo
+struct B { int i; };
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1419,7 +1419,8 @@
   return P.str();
 }
 
-static void ParseHeaderSearchArgs(HeaderSearchOptions , ArgList ) {
+static void ParseHeaderSearchArgs(HeaderSearchOptions , ArgList ,
+  const std::string ) {
   using namespace options;
   Opts.Sysroot = Args.getLastArgValue(OPT_isysroot, "/");
   Opts.Verbose = Args.hasArg(OPT_v);
@@ -1429,7 +1430,18 @@
   if (const Arg *A = Args.getLastArg(OPT_stdlib_EQ))
 Opts.UseLibcxx = (strcmp(A->getValue(), "libc++") == 0);
   Opts.ResourceDir = Args.getLastArgValue(OPT_resource_dir);
-  Opts.ModuleCachePath = Args.getLastArgValue(OPT_fmodules_cache_path);
+
+  // Canonicalize -fmodules-cache-path before storing it.
+  SmallString<128> P(Args.getLastArgValue(OPT_fmodules_cache_path));
+  if (!(P.empty() || llvm::sys::path::is_absolute(P))) {
+  

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

2017-03-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.h:20
+///\brief Warns about using throw in function declared as noexcept.
+/// It complains about every throw, even if it is caught later.
+class ThrowWithNoexceptCheck : public ClangTidyCheck {

sbarzowski wrote:
> sbarzowski wrote:
> > vmiklos wrote:
> > > Prazek wrote:
> > > > aaron.ballman wrote:
> > > > > What is the false positive rate with this check over a large codebase 
> > > > > that uses exceptions?
> > > > Do you know any good project to check it?
> > > LibreOffice might be a candidate, see 
> > >  for details 
> > > on how to generate a compile database for it (since it does not use 
> > > cmake), then you can start testing.
> > Thanks. However it's not just about using exception. To be meaningful I 
> > need a project that use noexcept in more than a few places.
> > 
> > Here are some projects that I found:
> > https://github.com/facebook/hhvm/search?utf8=%E2%9C%93=noexcept
> > https://github.com/facebook/folly/search?utf8=%E2%9C%93=noexcept
> > https://github.com/Tencent/mars/search?utf8=%E2%9C%93=noexcept
> > https://github.com/facebook/rocksdb/search?utf8=%E2%9C%93=noexcept
> > https://github.com/CRYTEK-CRYENGINE/CRYENGINE/search?utf8=%E2%9C%93=noexcept
> > https://github.com/SFTtech/openage/search?utf8=%E2%9C%93=noexcept
> > https://github.com/facebook/watchman/search?utf8=%E2%9C%93=noexcept
> > https://github.com/facebook/proxygen/search?utf8=%E2%9C%93=noexcept
> > https://github.com/philsquared/Catch/search?utf8=%E2%9C%93=noexcept
> > https://github.com/sandstorm-io/capnproto/search?utf8=%E2%9C%93=noexcept
> > https://github.com/miloyip/rapidjson/search?utf8=%E2%9C%93=noexcept
> > 
> > I've tried HHVM so far, and ran into some problems compiling it. Anyway I'm 
> > working on it.
> Ok, I was able to run it on most of the HHVM  codebase + deps. There were 
> some issues that looked like some autogenerated pieces missing, so it may 
> have been not 100% covered.
> 
> The results:
> 3 occurences in total
> 1) rethrow in destructor (http://pastebin.com/JRNMZGev)
> 2) false positive "throw and catch in the same function" 
> (http://pastebin.com/14y1AJgM)
> 3) noexcept decided during compilation (http://pastebin.com/1jZzRAbC)
> 
> That's not too many, but this is a kind of check that I guess would be most 
> useful earlier in the development - ideally before the initial code review.
> 
> I'm not sure if we should count (3) as false positive. We could potentially 
> eliminate it, by checking if the expression in noexcept is empty or true 
> literal.
> 
> (2) is def. a false positive. The potential handling of this case was already 
> proposed, but I'm not sure if it's worth it.  The code in example (2) is ugly 
> and extracting these throwing parts to separate functions looks like a good 
> way to start refactoring. 
> 
> What do you think?
> 
The fact that there's a false positive in the first large project you checked 
suggests that the false positive is likely worth it to fix. The code may be 
ugly, but it's not uncommon -- some coding guidelines explicitly disallow use 
of gotos, and this is a reasonable(ish) workaround for that.

As for #3, I would consider that to be a false-positive as well. A computed 
noexcept clause is going to be very common, especially in generic code. I think 
it's probably more important to get this one right than #2.


https://reviews.llvm.org/D19201



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


Re: [libcxx] r297553 - Change test coverage generation to use llvm-cov instead of gcov.

2017-03-13 Thread Bruno Cardoso Lopes via cfe-commits
Hi Eric,

>  if (APPLE AND (LIBCXX_CXX_ABI_LIBNAME STREQUAL "libcxxabi" OR
> @@ -62,12 +66,7 @@ if (APPLE AND LLVM_USE_SANITIZER)
>  message(WARNING "LLVM_USE_SANITIZER=${LLVM_USE_SANITIZER} is not 
> supported on OS X")
>endif()
>if (LIBFILE)
> -execute_process(COMMAND ${CMAKE_CXX_COMPILER} -print-file-name=lib 
> OUTPUT_VARIABLE LIBDIR RESULT_VARIABLE Result)
> -if (NOT ${Result} EQUAL "0")
> -  message(FATAL "Failed to find library resource directory")
> -endif()
> -string(STRIP "${LIBDIR}" LIBDIR)
> -set(LIBDIR "${LIBDIR}/darwin/")
> +find_compiler_rt_dir(LIBDIR)

Seems like this broke ASAN+UBSAN bot in green-dragon:
http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan_build/3812

Perhaps it's missing the "${LIBDIR}/darwin/" part from above?

-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D29923: PPCallbacks::MacroUndefined, change signature and add test.

2017-03-13 Thread Frederich Munch via Phabricator via cfe-commits
marsupial updated this revision to Diff 91627.
marsupial retitled this revision from "Add test for 
PPCallbacks::MacroUndefined" to "PPCallbacks::MacroUndefined, change signature 
and add test.".
marsupial edited the summary of this revision.
Herald added a subscriber: nemanjai.

https://reviews.llvm.org/D29923

Files:
  include/clang/Lex/PPCallbacks.h
  include/clang/Lex/PreprocessingRecord.h
  lib/Frontend/PrintPreprocessedOutput.cpp
  lib/Lex/PPDirectives.cpp
  lib/Lex/PreprocessingRecord.cpp
  tools/libclang/Indexing.cpp
  unittests/Basic/SourceManagerTest.cpp

Index: unittests/Basic/SourceManagerTest.cpp
===
--- unittests/Basic/SourceManagerTest.cpp
+++ unittests/Basic/SourceManagerTest.cpp
@@ -246,12 +246,18 @@
 namespace {
 
 struct MacroAction {
+  enum Kind { kExpansion, kDefinition, kUnDefinition};
+
   SourceLocation Loc;
   std::string Name;
-  bool isDefinition; // if false, it is expansion.
-  
-  MacroAction(SourceLocation Loc, StringRef Name, bool isDefinition)
-: Loc(Loc), Name(Name), isDefinition(isDefinition) { }
+  unsigned MAKind : 3;
+
+  MacroAction(SourceLocation Loc, StringRef Name, unsigned K)
+: Loc(Loc), Name(Name), MAKind(K) { }
+
+  bool isExpansion() const { return MAKind == kExpansion; }
+  bool isDefinition() const { return MAKind & kDefinition; }
+  bool isUnDefinition() const { return MAKind & kUnDefinition; }
 };
 
 class MacroTracker : public PPCallbacks {
@@ -264,21 +270,33 @@
 const MacroDirective *MD) override {
 Macros.push_back(MacroAction(MD->getLocation(),
  MacroNameTok.getIdentifierInfo()->getName(),
- true));
+ MacroAction::kDefinition));
+  }
+  void MacroUndefined(const Token ,
+  const MacroDefinition ,
+  const MacroDirective  *UD) override {
+Macros.push_back(
+MacroAction(UD ? UD->getLocation() : SourceLocation(),
+MacroNameTok.getIdentifierInfo()->getName(),
+UD ? MacroAction::kDefinition | MacroAction::kUnDefinition
+   : MacroAction::kUnDefinition));
   }
   void MacroExpands(const Token , const MacroDefinition ,
 SourceRange Range, const MacroArgs *Args) override {
 Macros.push_back(MacroAction(MacroNameTok.getLocation(),
  MacroNameTok.getIdentifierInfo()->getName(),
- false));
+ MacroAction::kExpansion));
   }
 };
 
 }
 
 TEST_F(SourceManagerTest, isBeforeInTranslationUnitWithMacroInInclude) {
   const char *header =
-"#define MACRO_IN_INCLUDE 0\n";
+"#define MACRO_IN_INCLUDE 0\n"
+"#define MACRO_DEFINED\n"
+"#undef MACRO_DEFINED\n"
+"#undef MACRO_UNDEFINED\n";
 
   const char *main =
 "#define M(x) x\n"
@@ -323,42 +341,54 @@
   // Make sure we got the tokens that we expected.
   ASSERT_EQ(0U, toks.size());
 
-  ASSERT_EQ(9U, Macros.size());
+  ASSERT_EQ(15U, Macros.size());
   // #define M(x) x
-  ASSERT_TRUE(Macros[0].isDefinition);
+  ASSERT_TRUE(Macros[0].isDefinition());
   ASSERT_EQ("M", Macros[0].Name);
   // #define INC "/test-header.h"
-  ASSERT_TRUE(Macros[1].isDefinition);
+  ASSERT_TRUE(Macros[1].isDefinition());
   ASSERT_EQ("INC", Macros[1].Name);
   // M expansion in #include M(INC)
-  ASSERT_FALSE(Macros[2].isDefinition);
+  ASSERT_FALSE(Macros[2].isDefinition());
   ASSERT_EQ("M", Macros[2].Name);
   // INC expansion in #include M(INC)
-  ASSERT_FALSE(Macros[3].isDefinition);
+  ASSERT_TRUE(Macros[3].isExpansion());
   ASSERT_EQ("INC", Macros[3].Name);
   // #define MACRO_IN_INCLUDE 0
-  ASSERT_TRUE(Macros[4].isDefinition);
+  ASSERT_TRUE(Macros[4].isDefinition());
   ASSERT_EQ("MACRO_IN_INCLUDE", Macros[4].Name);
+  // #define MACRO_DEFINED
+  ASSERT_TRUE(Macros[5].isDefinition());
+  ASSERT_FALSE(Macros[5].isUnDefinition());
+  ASSERT_EQ("MACRO_DEFINED", Macros[5].Name);
+  // #undef MACRO_DEFINED
+  ASSERT_TRUE(Macros[6].isDefinition());
+  ASSERT_TRUE(Macros[6].isUnDefinition());
+  ASSERT_EQ("MACRO_DEFINED", Macros[6].Name);
+  // #undef MACRO_UNDEFINED
+  ASSERT_FALSE(Macros[7].isDefinition());
+  ASSERT_TRUE(Macros[7].isUnDefinition());
+  ASSERT_EQ("MACRO_UNDEFINED", Macros[7].Name);
   // #define INC2 
-  ASSERT_TRUE(Macros[5].isDefinition);
-  ASSERT_EQ("INC2", Macros[5].Name);
+  ASSERT_TRUE(Macros[8].isDefinition());
+  ASSERT_EQ("INC2", Macros[8].Name);
   // M expansion in #include M(INC2)
-  ASSERT_FALSE(Macros[6].isDefinition);
-  ASSERT_EQ("M", Macros[6].Name);
+  ASSERT_FALSE(Macros[9].isDefinition());
+  ASSERT_EQ("M", Macros[9].Name);
   // INC2 expansion in #include M(INC2)
-  ASSERT_FALSE(Macros[7].isDefinition);
-  ASSERT_EQ("INC2", Macros[7].Name);
+  ASSERT_TRUE(Macros[10].isExpansion());
+  ASSERT_EQ("INC2", Macros[10].Name);
   // #define MACRO_IN_INCLUDE 0
-  

[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check

2017-03-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman added a comment.

I've commit in r297671, thanks!


https://reviews.llvm.org/D30610



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


[clang-tools-extra] r297671 - Add the 'AllowSoleDefaultDtor' and 'AllowMissingMoveFunctions' options to the cppcoreguidelines-special-member-functions check.

2017-03-13 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Mon Mar 13 16:39:00 2017
New Revision: 297671

URL: http://llvm.org/viewvc/llvm-project?rev=297671=rev
Log:
Add the 'AllowSoleDefaultDtor' and 'AllowMissingMoveFunctions' options to the 
cppcoreguidelines-special-member-functions check.

Patch by Florian Gross.

Added:

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
Modified:

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp?rev=297671=297670=297671=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
 (original)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
 Mon Mar 13 16:39:00 2017
@@ -22,6 +22,18 @@ namespace clang {
 namespace tidy {
 namespace cppcoreguidelines {
 
+SpecialMemberFunctionsCheck::SpecialMemberFunctionsCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  AllowMissingMoveFunctions(Options.get("AllowMissingMoveFunctions", 0)),
+  AllowSoleDefaultDtor(Options.get("AllowSoleDefaultDtor", 0)) {}
+
+void SpecialMemberFunctionsCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "AllowMissingMoveFunctions", AllowMissingMoveFunctions);
+  Options.store(Opts, "AllowSoleDefaultDtor", AllowSoleDefaultDtor);
+}
+
 void SpecialMemberFunctionsCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
 return;
@@ -48,6 +60,12 @@ toString(SpecialMemberFunctionsCheck::Sp
   switch (K) {
   case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::Destructor:
 return "a destructor";
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::
+  DefaultDestructor:
+return "a default destructor";
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::
+  NonDefaultDestructor:
+return "a non-default destructor";
   case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::CopyConstructor:
 return "a copy constructor";
   case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::CopyAssignment:
@@ -88,51 +106,86 @@ void SpecialMemberFunctionsCheck::check(
 
   ClassDefId ID(MatchedDecl->getLocation(), MatchedDecl->getName());
 
+  auto StoreMember = [this, ](SpecialMemberFunctionKind Kind) {
+llvm::SmallVectorImpl  =
+ClassWithSpecialMembers[ID];
+if (!llvm::is_contained(Members, Kind))
+  Members.push_back(Kind);
+  };
+
+  if (const auto *Dtor = Result.Nodes.getNodeAs("dtor")) {
+StoreMember(Dtor->isDefaulted()
+? SpecialMemberFunctionKind::DefaultDestructor
+: SpecialMemberFunctionKind::NonDefaultDestructor);
+  }
+
   std::initializer_list>
-  Matchers = {{"dtor", SpecialMemberFunctionKind::Destructor},
-  {"copy-ctor", SpecialMemberFunctionKind::CopyConstructor},
+  Matchers = {{"copy-ctor", SpecialMemberFunctionKind::CopyConstructor},
   {"copy-assign", SpecialMemberFunctionKind::CopyAssignment},
   {"move-ctor", SpecialMemberFunctionKind::MoveConstructor},
   {"move-assign", SpecialMemberFunctionKind::MoveAssignment}};
 
   for (const auto  : Matchers)
 if (Result.Nodes.getNodeAs(KV.first)) {
-  SpecialMemberFunctionKind Kind = KV.second;
-  llvm::SmallVectorImpl  =
-  ClassWithSpecialMembers[ID];
-  if (find(Members, Kind) == Members.end())
-Members.push_back(Kind);
+  StoreMember(KV.second);
 }
 }
 
 void SpecialMemberFunctionsCheck::onEndOfTranslationUnit() {
-  llvm::SmallVector AllSpecialMembers = {
-  SpecialMemberFunctionKind::Destructor,
-  SpecialMemberFunctionKind::CopyConstructor,
-  SpecialMemberFunctionKind::CopyAssignment};
-
-  if (getLangOpts().CPlusPlus11) {
-AllSpecialMembers.push_back(SpecialMemberFunctionKind::MoveConstructor);
-AllSpecialMembers.push_back(SpecialMemberFunctionKind::MoveAssignment);
+  for (const auto  : ClassWithSpecialMembers) {
+checkForMissingMembers(C.first, C.second);
   }
+}
 
-  for (const auto  : ClassWithSpecialMembers) {
-const auto  = C.second;
+void SpecialMemberFunctionsCheck::checkForMissingMembers(
+

[PATCH] D30898: Add new -fverbose-asm that enables source interleaving

2017-03-13 Thread Andrey Bokhanko via Phabricator via cfe-commits
andreybokhanko added a comment.

Hi Roger,

I'm very glad to see you started to work on this!

A somewhat obvious comment: no chance for this to be accepted without LIT 
tests. I understand you have your doubts on the best approach to testing -- and 
it's OK to ask either here or on llvm-dev -- but tests should be added 
nevertheless.

Yours,
Andrey


https://reviews.llvm.org/D30898



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


[PATCH] D30810: Preserve vec3 type.

2017-03-13 Thread JinGu Kang via Phabricator via cfe-commits
jaykang10 added a comment.

In https://reviews.llvm.org/D30810#699695, @bruno wrote:

> Hi JinGu,
>
> I just read the discussion on cfe-dev, some comments:
>
> > My colleague and I are implementing a transformation pass between LLVM IR 
> > and another IR and we want to keep the 3-component vector types in our 
> > target IR
>
> Why can't you go back through the shuffle to find out that it comes from a 
> vec3 -> vec4 transformation? Adding a flag here just for that seems very odd.


Hi Bruno,

Thanks for your comment. We have a pass to undo the vec4 to vec3. I wondered 
why clang generates the vec4 for vec3 load/store. As you can see the comment on 
clang's code, they are generated for better performance. I had a questions at 
this point. clang should consider vector load/store aligned by 4 regardless of 
target??? llvm's codegen could handle vec3 according to targets' vector 
load/store with their alignment. I agree the flag looks odd but I was concerned 
some llvm's targets depend on the vec4 so I suggested to add the flag. If I 
missed something, please let me know.


https://reviews.llvm.org/D30810



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


[PATCH] D30909: [Analyzer] Finish taint propagation to derived symbols of tainted regions

2017-03-13 Thread Vlad Tsyrklevich via Phabricator via cfe-commits
vlad.tsyrklevich updated this revision to Diff 91615.
vlad.tsyrklevich added a comment.

Fix a stray assert(), correctly this time..


https://reviews.llvm.org/D30909

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
  lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/taint-generic.c

Index: test/Analysis/taint-generic.c
===
--- test/Analysis/taint-generic.c
+++ test/Analysis/taint-generic.c
@@ -204,8 +204,10 @@
   sock = socket(AF_INET, SOCK_STREAM, 0);
   read(sock, [0], sizeof(tainted.buf));
   read(sock, [0], sizeof(tainted.st));
-  // FIXME: tainted.st[0].length should be marked tainted
-  __builtin_memcpy(buffer, tainted.buf, tainted.st[0].length); // no-warning
+  __builtin_memcpy(buffer, tainted.buf, tainted.st[0].length); // expected-warning {{Untrusted data is used to specify the buffer size}}
+
+  read(sock, , sizeof(tainted.st));
+  __builtin_memcpy(buffer, tainted.buf, tainted.st[0].length); // expected-warning {{Untrusted data is used to specify the buffer size}}
 }
 
 int testDivByZero() {
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -496,7 +496,10 @@
 
   Optional getDefaultBinding(Store S, const MemRegion *R) override {
 RegionBindingsRef B = getRegionBindings(S);
-return B.getDefaultBinding(R);
+// Default bindings are always applied over a base region so look up the
+// base region's default binding, otherwise the lookup will fail when R
+// is at an offset from R->getBaseRegion().
+return B.getDefaultBinding(R->getBaseRegion());
   }
 
   SVal getBinding(RegionBindingsConstRef B, Loc L, QualType T = QualType());
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -671,6 +671,17 @@
 
   ProgramStateRef NewState = set(Sym, Kind);
   assert(NewState);
+
+  if (const SymbolDerived *SD = dyn_cast(Sym)) {
+TaintedSymRegionsRef SymRegions(0, TSRFactory.getTreeFactory());
+if (contains(SD->getParentSymbol()))
+  SymRegions = *get(SD->getParentSymbol());
+
+SymRegions = SymRegions.add(SD->getRegion());
+NewState = NewState->set(SD->getParentSymbol(), SymRegions);
+assert(NewState);
+  }
+
   return NewState;
 }
 
@@ -723,15 +734,31 @@
 const TaintTagType *Tag = get(*SI);
 Tainted = (Tag && *Tag == Kind);
 
-// If this is a SymbolDerived with a tainted parent, it's also tainted.
-if (const SymbolDerived *SD = dyn_cast(*SI))
+if (const SymbolDerived *SD = dyn_cast(*SI)) {
+  // If this is a SymbolDerived with a tainted parent, it's also tainted.
   Tainted = Tainted || isTainted(SD->getParentSymbol(), Kind);
 
+  // If this is a SymbolDerived with the same parent symbol as another
+  // tainted SymbolDerived and a region that's a sub-region of that tainted
+  // symbol, it's also tainted.
+  if (contains(SD->getParentSymbol())) {
+const TypedValueRegion *R = SD->getRegion();
+const TaintedSymRegionsRef *SymRegions =
+get(SD->getParentSymbol());
+for (TaintedSymRegionsRef::iterator I = SymRegions->begin(),
+E = SymRegions->end();
+ I != E; ++I) {
+  if (R == *I || R->isSubRegionOf(*I))
+return true;
+}
+  }
+}
+
 // If memory region is tainted, data is also tainted.
 if (const SymbolRegionValue *SRV = dyn_cast(*SI))
   Tainted = Tainted || isTainted(SRV->getRegion(), Kind);
 
-// If If this is a SymbolCast from a tainted value, it's also tainted.
+// If this is a SymbolCast from a tainted value, it's also tainted.
 if (const SymbolCast *SC = dyn_cast(*SI))
   Tainted = Tainted || isTainted(SC->getOperand(), Kind);
 
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -72,8 +72,6 @@
   /// covers the entire region, e.g. we avoid false positives by not returning
   /// a default bindingc for an entire struct if the symbol for only a single
   /// field or element within it is requested.
-  // TODO: Return an appropriate symbol for sub-fields/elements of an LCV so
-  // that they are also appropriately tainted.
   static SymbolRef getLCVSymbol(CheckerContext ,
 nonloc::LazyCompoundVal );
 
@@ -479,19 +477,21 @@
 
   // getLCVSymbol() is reached in a PostStmt so we can always 

[PATCH] D30909: [Analyzer] Finish taint propagation to derived symbols of tainted regions

2017-03-13 Thread Vlad Tsyrklevich via Phabricator via cfe-commits
vlad.tsyrklevich updated this revision to Diff 91614.
vlad.tsyrklevich added a comment.

Fix a stray assert()


https://reviews.llvm.org/D30909

Files:
  lib/StaticAnalyzer/Core/ProgramState.cpp


Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -679,9 +679,9 @@
 
 SymRegions = SymRegions.add(SD->getRegion());
 NewState = NewState->set(SD->getParentSymbol(), 
SymRegions);
+assert(NewState);
   }
 
-  assert(NewState);
   return NewState;
 }
 


Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -679,9 +679,9 @@
 
 SymRegions = SymRegions.add(SD->getRegion());
 NewState = NewState->set(SD->getParentSymbol(), SymRegions);
+assert(NewState);
   }
 
-  assert(NewState);
   return NewState;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D30909: [Analyzer] Finish taint propagation to derived symbols of tainted regions

2017-03-13 Thread Vlad Tsyrklevich via Phabricator via cfe-commits
vlad.tsyrklevich created this revision.

This is the second part of https://reviews.llvm.org/D28445, it extends taint 
propagation to cases where the tainted region is a sub-region and we can't 
taint a conjured symbol entirely. This required adding a new map in the GDM 
that maps tainted parent symbols to tainted sub-regions (in order to avoid a 
linear scan looking for appropriate symbols in the current TaintMap.) With this 
change, tainting of structs and unions should work as expected.


https://reviews.llvm.org/D30909

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
  lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/taint-generic.c

Index: test/Analysis/taint-generic.c
===
--- test/Analysis/taint-generic.c
+++ test/Analysis/taint-generic.c
@@ -204,8 +204,10 @@
   sock = socket(AF_INET, SOCK_STREAM, 0);
   read(sock, [0], sizeof(tainted.buf));
   read(sock, [0], sizeof(tainted.st));
-  // FIXME: tainted.st[0].length should be marked tainted
-  __builtin_memcpy(buffer, tainted.buf, tainted.st[0].length); // no-warning
+  __builtin_memcpy(buffer, tainted.buf, tainted.st[0].length); // expected-warning {{Untrusted data is used to specify the buffer size}}
+
+  read(sock, , sizeof(tainted.st));
+  __builtin_memcpy(buffer, tainted.buf, tainted.st[0].length); // expected-warning {{Untrusted data is used to specify the buffer size}}
 }
 
 int testDivByZero() {
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -496,7 +496,10 @@
 
   Optional getDefaultBinding(Store S, const MemRegion *R) override {
 RegionBindingsRef B = getRegionBindings(S);
-return B.getDefaultBinding(R);
+// Default bindings are always applied over a base region so look up the
+// base region's default binding, otherwise the lookup will fail when R
+// is at an offset from R->getBaseRegion().
+return B.getDefaultBinding(R->getBaseRegion());
   }
 
   SVal getBinding(RegionBindingsConstRef B, Loc L, QualType T = QualType());
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -671,6 +671,17 @@
 
   ProgramStateRef NewState = set(Sym, Kind);
   assert(NewState);
+
+  if (const SymbolDerived *SD = dyn_cast(Sym)) {
+TaintedSymRegionsRef SymRegions(0, TSRFactory.getTreeFactory());
+if (contains(SD->getParentSymbol()))
+  SymRegions = *get(SD->getParentSymbol());
+
+SymRegions = SymRegions.add(SD->getRegion());
+NewState = NewState->set(SD->getParentSymbol(), SymRegions);
+  }
+
+  assert(NewState);
   return NewState;
 }
 
@@ -723,15 +734,31 @@
 const TaintTagType *Tag = get(*SI);
 Tainted = (Tag && *Tag == Kind);
 
-// If this is a SymbolDerived with a tainted parent, it's also tainted.
-if (const SymbolDerived *SD = dyn_cast(*SI))
+if (const SymbolDerived *SD = dyn_cast(*SI)) {
+  // If this is a SymbolDerived with a tainted parent, it's also tainted.
   Tainted = Tainted || isTainted(SD->getParentSymbol(), Kind);
 
+  // If this is a SymbolDerived with the same parent symbol as another
+  // tainted SymbolDerived and a region that's a sub-region of that tainted
+  // symbol, it's also tainted.
+  if (contains(SD->getParentSymbol())) {
+const TypedValueRegion *R = SD->getRegion();
+const TaintedSymRegionsRef *SymRegions =
+get(SD->getParentSymbol());
+for (TaintedSymRegionsRef::iterator I = SymRegions->begin(),
+E = SymRegions->end();
+ I != E; ++I) {
+  if (R == *I || R->isSubRegionOf(*I))
+return true;
+}
+  }
+}
+
 // If memory region is tainted, data is also tainted.
 if (const SymbolRegionValue *SRV = dyn_cast(*SI))
   Tainted = Tainted || isTainted(SRV->getRegion(), Kind);
 
-// If If this is a SymbolCast from a tainted value, it's also tainted.
+// If this is a SymbolCast from a tainted value, it's also tainted.
 if (const SymbolCast *SC = dyn_cast(*SI))
   Tainted = Tainted || isTainted(SC->getOperand(), Kind);
 
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -72,8 +72,6 @@
   /// covers the entire region, e.g. we avoid false positives by not returning
   /// a default bindingc for an entire struct if the symbol for only a single
   /// field or 

[PATCH] D29877: Warn about unused static file scope function template declarations.

2017-03-13 Thread Richard Smith via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: lib/Sema/Sema.cpp:472-477
+// If this is a function template, we should remove if it has no
+// specializations.
+if (FunctionTemplateDecl *Template = FD->getDescribedFunctionTemplate()) {
+  if (std::distance(Template->spec_begin(), Template->spec_end()))
+return true;
+}

The comment doesn't match the code: you're removing function templates if they 
/do/ have specializations. And I think we should probably be walking the list 
of specializations and considering the template to be used if any 
specialization is used. That would affect a case like:

```
template static void f() {}
template<> static void f() {}
```

... where the primary template is still unused despite having a specialization.



Comment at: lib/Sema/Sema.cpp:492
 
   if (const VarDecl *VD = dyn_cast(D)) {
 // If a variable usable in constant expressions is referenced,

Should we do the same thing for variable templates?



Comment at: lib/Sema/SemaDecl.cpp:1496
 return false;
+  // 'static operator' functions are defined in headers; don't warn.
+  if (FD->isOverloadedOperator() &&

Why? Defining a static operator in a header sounds like a bug to me.


https://reviews.llvm.org/D29877



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


[PATCH] D30881: Track skipped files in dependency scanning

2017-03-13 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added a comment.

Hi Pete,




Comment at: lib/Frontend/DependencyFile.cpp:191
+   const Token ,
+   SrcMgr::CharacteristicKind FileType) override;
+

Is there any `FileSkipped` callback invocation that might trigger an unwanted 
file to be added as a dependency or it will only trigger for the symlink case?



Comment at: test/Frontend/dependency-gen-symlink.c:11
+// RUN: echo "#endif" >> %t.dir/a/header.h
+// RUN: ln %t.dir/a/header.h %t.dir/b/header.h
+

It seems that it works for hard links as well right? Is this intended or do you 
miss a `-s`?


https://reviews.llvm.org/D30881



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


[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

2017-03-13 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno accepted this revision.
bruno added a comment.
This revision is now accepted and ready to land.

Hi Pete, thanks for working on this!

LGTM with the minor comments below.




Comment at: include/clang/Lex/PPCallbacks.h:264
+  virtual void HasInclude(SourceLocation Loc, const FileEntry *File) {
+  }
   

clang-format this one!



Comment at: test/Frontend/dependency-gen-has-include.c:17
+#if __has_include("a/header.h")
+#endif

Can you also add a testcase for `__has_include_next`?


https://reviews.llvm.org/D30882



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


[PATCH] D30810: Preserve vec3 type.

2017-03-13 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added a reviewer: bruno.
bruno added a comment.

Hi JinGu,

I just read the discussion on cfe-dev, some comments:

> My colleague and I are implementing a transformation pass between LLVM IR and 
> another IR and we want to keep the 3-component vector types in our target IR

Why can't you go back through the shuffle to find out that it comes from a vec3 
-> vec4 transformation? Adding a flag here just for that seems very odd.


https://reviews.llvm.org/D30810



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


r297659 - AMDGPU: Make 0 the private nullptr value

2017-03-13 Thread Matt Arsenault via cfe-commits
Author: arsenm
Date: Mon Mar 13 14:47:53 2017
New Revision: 297659

URL: http://llvm.org/viewvc/llvm-project?rev=297659=rev
Log:
AMDGPU: Make 0 the private nullptr value

We can't actually pretend that 0 is valid for address space 0.
r295877 added a workaround to stop allocating user objects
there, so we can use 0 as the invalid pointer.

Some of the tests seemed to be using private as the non-0 null
test address space, so add copies using local to make sure
this is still stressed.

Modified:
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
cfe/trunk/test/CodeGenOpenCL/blocks.cl

Modified: cfe/trunk/lib/Basic/Targets.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=297659=297658=297659=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Mon Mar 13 14:47:53 2017
@@ -2298,7 +2298,7 @@ public:
   // address space has value 0 but in private and local address space has
   // value ~0.
   uint64_t getNullPointerValue(unsigned AS) const override {
-return AS != LangAS::opencl_local && AS != 0 ? 0 : ~0;
+return AS == LangAS::opencl_local ? ~0 : 0;
   }
 };
 

Modified: cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl?rev=297659=297658=297659=diff
==
--- cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl Mon Mar 13 14:47:53 2017
@@ -20,7 +20,7 @@ typedef struct {
 
 // Test 0 as initializer.
 
-// CHECK: @private_p = local_unnamed_addr addrspace(1) global i8* 
addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// CHECK: @private_p = local_unnamed_addr addrspace(1) global i8* null, align 4
 private char *private_p = 0;
 
 // CHECK: @local_p = local_unnamed_addr addrspace(1) global i8 addrspace(3)* 
addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
@@ -37,7 +37,7 @@ generic char *generic_p = 0;
 
 // Test NULL as initializer.
 
-// CHECK: @private_p_NULL = local_unnamed_addr addrspace(1) global i8* 
addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// CHECK: @private_p_NULL = local_unnamed_addr addrspace(1) global i8* null, 
align 4
 private char *private_p_NULL = NULL;
 
 // CHECK: @local_p_NULL = local_unnamed_addr addrspace(1) global i8 
addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
@@ -58,38 +58,55 @@ generic char *generic_p_NULL = NULL;
 // CHECK: @fold_generic = local_unnamed_addr addrspace(1) global i32 
addrspace(4)* null, align 4
 generic int *fold_generic = (global int*)(generic float*)(private char*)0;
 
-// CHECK: @fold_priv = local_unnamed_addr addrspace(1) global i16* 
addrspacecast (i16 addrspace(4)* null to i16*), align 4
+// CHECK: @fold_priv = local_unnamed_addr addrspace(1) global i16* null, align 
4
 private short *fold_priv = (private short*)(generic int*)(global void*)0;
 
-// CHECK: @fold_priv_arith = local_unnamed_addr addrspace(1) global i8* 
inttoptr (i32 9 to i8*), align 4
+// CHECK: @fold_priv_arith = local_unnamed_addr addrspace(1) global i8* 
inttoptr (i32 10 to i8*), align 4
 private char *fold_priv_arith = (private char*)0 + 10;
 
-// CHECK: @fold_int = local_unnamed_addr addrspace(1) global i32 13, align 4
+// CHECK: @fold_int = local_unnamed_addr addrspace(1) global i32 14, align 4
 int fold_int = (int)(private void*)(generic char*)(global int*)0 + 14;
 
-// CHECK: @fold_int2 = local_unnamed_addr addrspace(1) global i32 12, align 4
+// CHECK: @fold_int2 = local_unnamed_addr addrspace(1) global i32 13, align 4
 int fold_int2 = (int) ((private void*)0 + 13);
 
-// CHECK: @fold_int3 = local_unnamed_addr addrspace(1) global i32 -1, align 4
+// CHECK: @fold_int3 = local_unnamed_addr addrspace(1) global i32 0, align 4
 int fold_int3 = (int) ((private int*)0);
 
-// CHECK: @fold_int4 = local_unnamed_addr addrspace(1) global i32 7, align 4
+// CHECK: @fold_int4 = local_unnamed_addr addrspace(1) global i32 8, align 4
 int fold_int4 = (int) &((private int*)0)[2];
 
-// CHECK: @fold_int5 = local_unnamed_addr addrspace(1) global i32 3, align 4
+// CHECK: @fold_int5 = local_unnamed_addr addrspace(1) global i32 4, align 4
 int fold_int5 = (int) &((private StructTy1*)0)->p2;
 
+
+// CHECK: @fold_int_local = local_unnamed_addr addrspace(1) global i32 13, 
align 4
+int fold_int_local = (int)(local void*)(generic char*)(global int*)0 + 14;
+
+// CHECK: @fold_int2_local = local_unnamed_addr addrspace(1) global i32 12, 
align 4
+int fold_int2_local = (int) ((local void*)0 + 13);
+
+// CHECK: @fold_int3_local = local_unnamed_addr addrspace(1) global i32 -1, 
align 4
+int fold_int3_local = (int) ((local int*)0);
+
+// CHECK: @fold_int4_local = local_unnamed_addr addrspace(1) global i32 7, 
align 4
+int fold_int4_local = (int) 

[PATCH] D30810: Preserve vec3 type.

2017-03-13 Thread JinGu Kang via Phabricator via cfe-commits
jaykang10 added a comment.

In https://reviews.llvm.org/D30810#699428, @Anastasia wrote:

> Would you be able to update ScalarExprEmitter::VisitAsTypeExpr implementation 
> accordingly to make things consistent?


Probably, No... the load/store with vec3 generates vec4 temporarily to be 
aligned but __builtin_astype needs to generate vec4 from vec3  or vice versa 
because spec allows it on 6.2.4.2. As we discussed on previous e-mails, we 
would need to add intrinsic function for __builtin_astype on llvm and add 
default behavior on llvm's codegen. It is beyond clang and I am not sure llvm's 
IR and codegen people allow it... If you have another idea or I missed 
something, please let me know.


https://reviews.llvm.org/D30810



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


LLVM Lab SVN mirror is behind

2017-03-13 Thread Galina Kistanova via cfe-commits
Hello everyone,

The SVN mirror in the LLVM Lab seems behind with the changes.
I'm looking at the issue.

Thank you for understanding.

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


[PATCH] D27689: Module: hash the pcm content and use it as SIGNATURE for implicit modules.

2017-03-13 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith closed this revision.
dexonsmith marked 3 inline comments as done.
dexonsmith added a comment.

Thanks for the review Richard!  Committed in r297655 with those changes.

Sorry for the long delay on this.  Somehow I missed until today the review 
after my ultimate ping.  I'd like to blame Phab, but it's more likely that I 
just failed at email :/.


https://reviews.llvm.org/D27689



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


r297655 - Modules: Use hash of PCM content for SIGNATURE

2017-03-13 Thread Duncan P. N. Exon Smith via cfe-commits
Author: dexonsmith
Date: Mon Mar 13 13:45:08 2017
New Revision: 297655

URL: http://llvm.org/viewvc/llvm-project?rev=297655=rev
Log:
Modules: Use hash of PCM content for SIGNATURE

Change ASTFileSignature from a random 32-bit number to the hash of the
PCM content.

  - Move definition ASTFileSignature to Basic/Module.h so Module and
ASTSourceDescriptor can use it.

  - Change the signature from uint64_t to std::array.

  - Stop using (saving/reading) the size and modification time of PCM
files when there is a valid SIGNATURE.

  - Add UNHASHED_CONTROL_BLOCK, and use it to store the SIGNATURE record
and other records that shouldn't affect the hash.  Because implicit
modules reuses the same file for multiple levels of -Werror, this
includes DIAGNOSTIC_OPTIONS and DIAG_PRAGMA_MAPPINGS.

This helps to solve a PCH + implicit Modules dependency issue: PCH files
are handled by the external build system, whereas implicit modules are
handled by internal compiler build system.  This prevents invalidating a
PCH when the compiler overwrites a PCM file with the same content
(modulo the diagnostic differences).

Design and original patch by Manman Ren!

Modified:
cfe/trunk/include/clang/AST/ExternalASTSource.h
cfe/trunk/include/clang/Basic/Module.h
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/Frontend/PCHContainerOperations.h
cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
cfe/trunk/include/clang/Serialization/ASTBitCodes.h
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/include/clang/Serialization/ASTWriter.h
cfe/trunk/include/clang/Serialization/Module.h
cfe/trunk/lib/Basic/Module.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/lib/Serialization/GeneratePCH.cpp
cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
cfe/trunk/test/Modules/diagnostic-options-out-of-date.m
cfe/trunk/test/Modules/module_file_info.m
cfe/trunk/test/Modules/rebuild.m

Modified: cfe/trunk/include/clang/AST/ExternalASTSource.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExternalASTSource.h?rev=297655=297654=297655=diff
==
--- cfe/trunk/include/clang/AST/ExternalASTSource.h (original)
+++ cfe/trunk/include/clang/AST/ExternalASTSource.h Mon Mar 13 13:45:08 2017
@@ -150,20 +150,20 @@ public:
 StringRef PCHModuleName;
 StringRef Path;
 StringRef ASTFile;
-uint64_t Signature = 0;
+ASTFileSignature Signature;
 const Module *ClangModule = nullptr;
 
   public:
 ASTSourceDescriptor(){};
 ASTSourceDescriptor(StringRef Name, StringRef Path, StringRef ASTFile,
-uint64_t Signature)
+ASTFileSignature Signature)
 : PCHModuleName(std::move(Name)), Path(std::move(Path)),
   ASTFile(std::move(ASTFile)), Signature(Signature){};
 ASTSourceDescriptor(const Module );
 std::string getModuleName() const;
 StringRef getPath() const { return Path; }
 StringRef getASTFile() const { return ASTFile; }
-uint64_t getSignature() const { return Signature; }
+ASTFileSignature getSignature() const { return Signature; }
 const Module *getModuleOrNull() const { return ClangModule; }
   };
 

Modified: cfe/trunk/include/clang/Basic/Module.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=297655=297654=297655=diff
==
--- cfe/trunk/include/clang/Basic/Module.h (original)
+++ cfe/trunk/include/clang/Basic/Module.h Mon Mar 13 13:45:08 2017
@@ -42,7 +42,17 @@ class IdentifierInfo;
   
 /// \brief Describes the name of a module.
 typedef SmallVector, 2> ModuleId;
-  
+
+/// The signature of a module, which is a hash of the AST content.
+struct ASTFileSignature : std::array {
+  ASTFileSignature(std::array S = {{0}})
+  : std::array(std::move(S)) {}
+
+  explicit operator bool() const {
+return *this != std::array({{0}});
+  }
+};
+
 /// \brief Describes a module or submodule.
 class Module {
 public:
@@ -65,7 +75,7 @@ public:
   llvm::PointerUnion Umbrella;
 
   /// \brief The module signature.
-  uint64_t Signature;
+  ASTFileSignature Signature;
 
   /// \brief The name of the umbrella entry, as written in the module map.
   std::string UmbrellaAsWritten;

Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=297655=297654=297655=diff

[PATCH] D30743: enable -save-temps with -finclude-defult-header

2017-03-13 Thread Guansong Zhang via Phabricator via cfe-commits
guansong updated this revision to Diff 91600.

https://reviews.llvm.org/D30743

Files:
  lib/Driver/Tools.cpp
  test/Driver/include-default-header.cl


Index: test/Driver/include-default-header.cl
===
--- /dev/null
+++ test/Driver/include-default-header.cl
@@ -0,0 +1,4 @@
+// RUN: %clang -v -save-temps -x cl -Xclang -cl-std=CL2.0 -Xclang 
-finclude-default-header -target amdgcn -S -c %s
+
+void test() {}
+
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -5285,7 +5285,20 @@
 
   // Forward -Xclang arguments to -cc1, and -mllvm arguments to the LLVM option
   // parser.
-  Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
+  if (C.getDriver().isSaveTempsEnabled() &&
+  !isa(JA)) {
+// -finclude-default-header flag is for preprocessor,
+// do not pass it to other cc1 commands when save-temps is enabled
+for (auto Arg : Args.filtered(options::OPT_Xclang)) {
+  Arg->claim();
+  if (StringRef(Arg->getValue()) != "-finclude-default-header")
+CmdArgs.push_back(Arg->getValue());
+}
+  }
+  else {
+Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
+  }
+
   for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
 A->claim();
 


Index: test/Driver/include-default-header.cl
===
--- /dev/null
+++ test/Driver/include-default-header.cl
@@ -0,0 +1,4 @@
+// RUN: %clang -v -save-temps -x cl -Xclang -cl-std=CL2.0 -Xclang -finclude-default-header -target amdgcn -S -c %s
+
+void test() {}
+
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -5285,7 +5285,20 @@
 
   // Forward -Xclang arguments to -cc1, and -mllvm arguments to the LLVM option
   // parser.
-  Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
+  if (C.getDriver().isSaveTempsEnabled() &&
+  !isa(JA)) {
+// -finclude-default-header flag is for preprocessor,
+// do not pass it to other cc1 commands when save-temps is enabled
+for (auto Arg : Args.filtered(options::OPT_Xclang)) {
+  Arg->claim();
+  if (StringRef(Arg->getValue()) != "-finclude-default-header")
+CmdArgs.push_back(Arg->getValue());
+}
+  }
+  else {
+Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
+  }
+
   for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
 A->claim();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r297654 - Widen bitfield for type specifiers for OpenCL types

2017-03-13 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Mon Mar 13 13:42:30 2017
New Revision: 297654

URL: http://llvm.org/viewvc/llvm-project?rev=297654=rev
Log:
Widen bitfield for type specifiers for OpenCL types

Added a static_assert to catch this issue at compile time.

Modified:
cfe/trunk/include/clang/Basic/Specifiers.h

Modified: cfe/trunk/include/clang/Basic/Specifiers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Specifiers.h?rev=297654=297653=297654=diff
==
--- cfe/trunk/include/clang/Basic/Specifiers.h (original)
+++ cfe/trunk/include/clang/Basic/Specifiers.h Mon Mar 13 13:42:30 2017
@@ -82,11 +82,12 @@ namespace clang {
   /// \brief Structure that packs information about the type specifiers that
   /// were written in a particular type specifier sequence.
   struct WrittenBuiltinSpecs {
-/*DeclSpec::TST*/ unsigned Type  : 5;
+static_assert(TST_error < 1 << 6, "Type bitfield not wide enough for TST");
+/*DeclSpec::TST*/ unsigned Type  : 6;
 /*DeclSpec::TSS*/ unsigned Sign  : 2;
 /*DeclSpec::TSW*/ unsigned Width : 2;
 unsigned ModeAttr : 1;
-  };  
+  };
 
   /// \brief A C++ access specifier (public, private, protected), plus the
   /// special value "none" which means different things in different contexts.


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


[PATCH] D28136: [OpenCL] Implement as_type operator as alias of __builtin_astype.

2017-03-13 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.



In https://reviews.llvm.org/D28136#697844, @bader wrote:

> > Why do you think this is a bug? It seems to follow standard behavior in C 
> > to promote char to int if required. Just like if you would have a C code:
> > 
> >   int as_int(int i);
> >   void foo() {
> >   char src = 1;
> >   int dst = as_int(src);
> >   } 
> >
> > 
> > This code would complie and the same exactly IR would be generated.
>
> as_type is defined to be used for bit re-interpretation. (see 6.2.4.2 
> Reinterpreting Types Using as_type() and as_typen()). In this sense, it's 
> exactly matches __bultin_astype built-in function.
>
> Here are a few relevant OpenCL C language specification quotes from 6.2.4 
> section:
>
> > All data types described in tables 6.1 and 6.2 (except bool, half and void) 
> > may be also reinterpreted as another data type of **the same size** using 
> > the as_type() operator for scalar data types and the as_typen() operator 
> > for vector data types.
>
>
>
> > The usual type promotion for function arguments shall not be performed.
>
>
>
> > It is an error to use as_type() or as_typen() operator to reinterpret data 
> > to a type of a different number of bytes.
>
> So, aliasing as_type to __builtin_astype provides these checks, whereas we 
> can't do it for overloadable as_type function declarations.
>
> I also would like to address your original concerns:
>
> > The main issue is after preprocessing the header the original function name 
> > is no longer available in diagnostics reported.
>
> Actually diagnostics is able to provide a hint to exact code location in the 
> header file where as_ is defined as macro, so user gets correct name of 
> the operator in diagnostics as far as I know.
>
> > The spec defines as_type as a builtin function and not a macro.
>
> To be precise, spec defines as_type as an operator. So, the best way to 
> implement it would be to add a language support of such operator, but AFAIK 
> aliasing to __bulitin_astype via macro is sufficient enough for OpenCL use 
> cases.
>
> > Additionally your patch would allow as_type to be used with extra type (not 
> > only those defined in spec).
>
> Not sure I get this. We defined limited set of as_* functions - only for 
> types from tables 6.1 and 6.2 as specified by specification, so if OpenCL 
> developer will try to call as_(type2), which is not defined in the 
> header, compiler will report en error about calling undeclared function.
>
> > Also I don't see the problem to implement as_type with just simply calling 
> > a builtin. It should be inlined later anyways.
>
> Yes, but this solution will not give us error checking as pre-processor 
> solution.
>
> Does it make sense?


From all the above arguments, I feel like the right approach would be to 
implement it as Clang builtin which closely matches the operator semantic in my 
opinion. We could of course reuse the implementation of  __bultin_astype to 
avoid unnecessary extra work and code duplication.

Using the macro seems to me more like a workaround solution and overloaded 
functions don't seem to be entirely the right thing either.  What do you think 
about it?


https://reviews.llvm.org/D28136



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


[PATCH] D30896: [CLANG-TIDY] add check misc-prefer-switch-for-enums

2017-03-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

alright. then i have a good check for myself to write ;)


https://reviews.llvm.org/D30896



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


r297649 - [Linker] Provide callback for internalization

2017-03-13 Thread Jonas Devlieghere via cfe-commits
Author: jdevlieghere
Date: Mon Mar 13 13:08:11 2017
New Revision: 297649

URL: http://llvm.org/viewvc/llvm-project?rev=297649=rev
Log:
[Linker] Provide callback for internalization

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

Modified:
cfe/trunk/include/clang/CodeGen/CodeGenAction.h
cfe/trunk/include/clang/Frontend/CodeGenOptions.h
cfe/trunk/lib/CodeGen/CodeGenAction.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/include/clang/CodeGen/CodeGenAction.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CodeGen/CodeGenAction.h?rev=297649=297648=297649=diff
==
--- cfe/trunk/include/clang/CodeGen/CodeGenAction.h (original)
+++ cfe/trunk/include/clang/CodeGen/CodeGenAction.h Mon Mar 13 13:08:11 2017
@@ -36,6 +36,9 @@ private:
 /// function ourselves.
 bool PropagateAttrs;
 
+/// If true, we use LLVM module internalizer.
+bool Internalize;
+
 /// Bitwise combination of llvm::LinkerFlags used when we link the module.
 unsigned LinkFlags;
   };

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.h?rev=297649=297648=297649=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.h (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.h Mon Mar 13 13:08:11 2017
@@ -137,6 +137,8 @@ public:
 /// our CodeGenOptions, much as we set attrs on functions that we generate
 /// ourselves.
 bool PropagateAttrs = false;
+/// If true, we use LLVM module internalizer.
+bool Internalize = false;
 /// Bitwise combination of llvm::Linker::Flags, passed to the LLVM linker.
 unsigned LinkFlags = 0;
   };

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=297649=297648=297649=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Mon Mar 13 13:08:11 2017
@@ -28,6 +28,7 @@
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/DiagnosticPrinter.h"
+#include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IRReader/IRReader.h"
@@ -38,6 +39,8 @@
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/YAMLTraits.h"
+#include "llvm/Transforms/IPO/Internalize.h"
+
 #include 
 using namespace clang;
 using namespace llvm;
@@ -168,8 +171,22 @@ namespace clang {
 Gen->CGM().AddDefaultFnAttrs(F);
 
 CurLinkModule = LM.Module.get();
-if (Linker::linkModules(*getModule(), std::move(LM.Module),
-LM.LinkFlags))
+
+bool Err;
+if (LM.Internalize) {
+  Err = Linker::linkModules(
+  *getModule(), std::move(LM.Module), LM.LinkFlags,
+  [](llvm::Module , const llvm::StringSet<> ) {
+internalizeModule(M, [, ](const llvm::GlobalValue ) {
+  return !GV.hasName() || (GVS.count(GV.getName()) == 0);
+});
+  });
+} else {
+  Err = Linker::linkModules(*getModule(), std::move(LM.Module),
+LM.LinkFlags);
+}
+
+if (Err)
   return true;
   }
   return false; // success
@@ -319,7 +336,7 @@ namespace clang {
 void OptimizationFailureHandler(
 const llvm::DiagnosticInfoOptimizationFailure );
   };
-  
+
   void BackendConsumer::anchor() {}
 }
 
@@ -388,7 +405,7 @@ void BackendConsumer::InlineAsmDiagHandl
   // code.
   if (LocCookie.isValid()) {
 Diags.Report(LocCookie, DiagID).AddString(Message);
-
+
 if (D.getLoc().isValid()) {
   DiagnosticBuilder B = Diags.Report(Loc, diag::note_fe_inline_asm_here);
   // Convert the SMDiagnostic ranges into SourceRange and attach them
@@ -401,7 +418,7 @@ void BackendConsumer::InlineAsmDiagHandl
 }
 return;
   }
-  
+
   // Otherwise, report the backend issue as occurring in the generated .s file.
   // If Loc is invalid, we still need to report the issue, it just gets no
   // location info.
@@ -815,8 +832,8 @@ CodeGenAction::CreateASTConsumer(Compile
 LinkModules.clear();
 return nullptr;
   }
-  LinkModules.push_back(
-  {std::move(ModuleOrErr.get()), F.PropagateAttrs, F.LinkFlags});
+  LinkModules.push_back({std::move(ModuleOrErr.get()), F.PropagateAttrs,
+ F.Internalize, F.LinkFlags});
 }
 
   CoverageSourceInfo *CoverageInfo = nullptr;

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 

[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check

2017-03-13 Thread Florian Gross via Phabricator via cfe-commits
fgross added a comment.

No commit access, could someone please take care of this? Thanks!


https://reviews.llvm.org/D30610



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


[PATCH] D30643: [OpenCL] Extended diagnostics for atomic initialization

2017-03-13 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8263
+def err_atomic_init_addressspace : Error<
+  "initialization of atomic variables is restricted to variables in global 
address space">;
 def err_atomic_init_constant : Error<

bader wrote:
> bader wrote:
> > Anastasia wrote:
> > > echuraev wrote:
> > > > Anastasia wrote:
> > > > > Could we combine this error diag with the one below? I guess they are 
> > > > > semantically very similar apart from one is about initialization and 
> > > > > another is about assignment?
> > > > I'm not sure that it is a good idea to combine these errors. For 
> > > > example, if developer had declared a variable non-constant and not in 
> > > > global address space he would have got the same message for both 
> > > > errors. And it can be difficult to determine what the exact problem is. 
> > > > He can fix one of the problems but he will still get the same error.
> > > Well, I don't actually see that we check for constant anywhere so it's 
> > > also OK if you want to drop this bit. Although I think the original 
> > > intension of this message as I understood was to provide the most 
> > > complete hint.
> > > 
> > > My concern is that these two errors seem to be reporting nearly the same 
> > > issue and ideally we would like to keep diagnostic list as small as 
> > > possible. This also makes the file more concise and messages more 
> > > consistent.
> > I suggest adding a test case with non-constant initialization case to 
> > validate that existing checks cover this case for atomic types already.
> > If so, we can adjust existing diagnostic message to cover both cases: 
> > initialization and assignment expression.
> I don't think it's quite true.
> There are two requirements here that must be met the same time. Atomic 
> variables *declared in the global address space* can be initialized only with 
> "compile time constant'.
> If understand the spec correctly this code is also valid:
> 
>   kernel void foo() {
> static global atomic_int a = 42; // although it's not clear if we must 
> use ATOMIC_VAR_INIT here.
> ...
>   }
Precisely, but I think checking for compile time constant should be inherited 
from the general C implementation? I don't think we do anything extra for it.  
Regarding the macro I am not sure we can suitably diagnose it anyways...



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8268
+// Atomics
+def err_atomic_init: Error<
+  "atomic variable can only be assigned to a compile time constant or to 
variables in global adress space">;

also we should add opencl there:

err_opencl_...


https://reviews.llvm.org/D30643



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


[PATCH] D30896: [CLANG-TIDY] add check misc-prefer-switch-for-enums

2017-03-13 Thread Jonathan B Coe via Phabricator via cfe-commits
jbcoe marked 2 inline comments as done.
jbcoe added a comment.

Handling

  enum Kind k = Kind::a;
  if (k == 3) { /* something */ }

is intentionally out of scope for now as the author is doing something that I 
can't trivially replace with a switch.




Comment at: clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp:3
+
+enum class kind { a, b, c, d };
+

JonasToth wrote:
> maybe another test for non enum class values would be nice.
> 
> ```
> enum another_kind {e, f, g};
> ```
> 
> 
I don't need to write code to specifically deal with enum classes. Perhaps 
changing the test to just test enums is simpler. I'm not sure what the 
LLVM/Clang approach normally is.


https://reviews.llvm.org/D30896



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


[PATCH] D30743: enable -save-temps with -finclude-defult-header

2017-03-13 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: lib/Driver/Tools.cpp:5288
   // parser.
-  Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
+  if (C.getDriver().isSaveTempsEnabled() &&
+  !isa(JA)) {

It would be nice to add a comment around here explaining this case.



Comment at: lib/Driver/Tools.cpp:5290
+  !isa(JA)) {
+//Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
+for (auto Arg : Args.filtered(options::OPT_Xclang)) {

Remove the commented code, please!


https://reviews.llvm.org/D30743



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


[PATCH] D30896: [CLANG-TIDY] add check misc-prefer-switch-for-enums

2017-03-13 Thread Jonathan B Coe via Phabricator via cfe-commits
jbcoe updated this revision to Diff 91588.
jbcoe added a comment.

Minor edits in response to review comments. A few questions outstanding.


https://reviews.llvm.org/D30896

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.cpp
  clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst
  clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp

Index: clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp
@@ -0,0 +1,40 @@
+// RUN: %check_clang_tidy %s misc-prefer-switch-for-enums %t 
+
+enum class kind { a, b, c, d };
+
+int foo(kind k)
+{
+  if (k == kind::a)
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer a switch statement for enum-value checks [misc-prefer-switch-for-enums]
+return -1;
+  return 1;
+}
+
+int antifoo(kind k)
+{
+  if (k != kind::a)
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer a switch statement for enum-value checks [misc-prefer-switch-for-enums]
+return 1;
+  return -1;
+}
+
+int bar(int i)
+{
+  if (i == 0)
+return -1;
+  return 1;
+}
+
+int foobar(kind k)
+{
+  switch(k)
+  {
+case kind::a:
+  return -1;
+case kind::b:
+case kind::c:
+case kind::d:
+  return 1;
+  }
+}
+
Index: clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst
@@ -0,0 +1,40 @@
+.. title:: clang-tidy - misc-prefer-switch-for-enums
+
+misc-prefer-switch-for-enums
+
+
+This check finds enum values used in ``==`` and ``!=`` expressions.
+
+A switch statement will robustly identify unhandled enum cases with a compiler
+warning. No such warning exists for control flow based on enum value
+comparison.  Use of switches rather than if statements leads to easier to
+maintain code as adding values to an enum will trigger compiler warnings for
+unhandled cases.
+
+
+.. code-block:: c++
+
+  enum class kind { a, b, c};
+
+  int foo(kind k) {
+return k == kind::a
+   ? 1
+   : -1;
+  }
+
+is more maintainably (but more verbosely) written as:
+
+.. code-block:: c++
+
+  enum class kind { a, b, c};
+
+  int foo(kind k) {
+switch(k) {
+  case kind::a:
+return 1;
+  case kind::b:
+  case kind::c:
+return -1;
+}
+  }
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -78,6 +78,7 @@
misc-new-delete-overloads
misc-noexcept-move-constructor
misc-non-copyable-objects
+   misc-prefer-switch-for-enums
misc-redundant-expression
misc-sizeof-container
misc-sizeof-expression
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -90,6 +90,12 @@
 - Support clang-formatting of the code around applied fixes (``-format-style``
   command-line option).
 
+- New `misc-prefer-switch-for-enums
+  `_ check
+
+  Finds uses of enumeration values in equality and inequality expressions where a switch would be preferred.
+
+
 Improvements to include-fixer
 -
 
Index: clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.h
@@ -0,0 +1,36 @@
+//===--- PreferSwitchForEnumsCheck.h - clang-tidy*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PREFER_SWITCH_FOR_ENUMS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PREFER_SWITCH_FOR_ENUMS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Find places where an enumeration value is used in `==` or `!=`. 
+/// Using `switch` leads to more maintainable code.
+///
+/// For the user-facing documentation see:
+/// 

[PATCH] D30875: [X86] Add checking of the scale argument to scatter/gather builtins

2017-03-13 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL297642: [X86] Add checking of the scale argument to 
scatter/gather builtins (authored by ctopper).

Changed prior to commit:
  https://reviews.llvm.org/D30875?vs=91508=91586#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30875

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/Sema/builtins-x86.c

Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -9993,6 +9993,7 @@
   bool CheckMipsBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckSystemZBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckX86BuiltinRoundingOrSAE(unsigned BuiltinID, CallExpr *TheCall);
+  bool CheckX86BuiltinGatherScatterScale(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckX86BuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckPPCBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
 
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8005,6 +8005,8 @@
   "this builtin is only available on x86-64 targets">;
 def err_x86_builtin_invalid_rounding : Error<
   "invalid rounding argument">;
+def err_x86_builtin_invalid_scale : Error<
+  "scale argument must be 1, 2, 4, or 8">;
 
 def err_builtin_longjmp_unsupported : Error<
   "__builtin_longjmp is not supported for the current target">;
Index: cfe/trunk/test/Sema/builtins-x86.c
===
--- cfe/trunk/test/Sema/builtins-x86.c
+++ cfe/trunk/test/Sema/builtins-x86.c
@@ -4,6 +4,7 @@
 typedef float __m128 __attribute__((__vector_size__(16)));
 typedef double __m128d __attribute__((__vector_size__(16)));
 
+typedef long long __m512i __attribute__((__vector_size__(64)));
 typedef float __m512 __attribute__((__vector_size__(64)));
 typedef double __m512d __attribute__((__vector_size__(64)));
 
@@ -69,3 +70,16 @@
 __mmask16 test__builtin_ia32_cmpps512_mask_rounding(__m512 __a, __m512 __b, __mmask16 __u) {
   __builtin_ia32_cmpps512_mask(__a, __b, 0, __u, 0); // expected-error {{invalid rounding argument}}
 }
+
+__m128i test_mm_mask_i32gather_epi32(__m128i a, int const *b, __m128i c, __m128i mask) {
+  return __builtin_ia32_gatherd_d(a, b, c, mask, 5); // expected-error {{scale argument must be 1, 2, 4, or 8}}
+}
+
+__m512i _mm512_mask_prefetch_i32gather_ps(__m512i index, __mmask16 mask, int const *addr) {
+  return __builtin_ia32_gatherpfdps(mask, index, addr, 5, 1); // expected-error {{scale argument must be 1, 2, 4, or 8}}
+}
+
+__m512 _mm512_mask_prefetch_i32gather_ps_2(__m512i index, __mmask16 mask, int const *addr) {
+  return __builtin_ia32_gatherpfdps(mask, index, addr, 1, 3); // expected-error {{argument should be a value from 1 to 2}}
+}
+
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -1986,6 +1986,109 @@
 << Arg->getSourceRange();
 }
 
+// Check if the gather/scatter scale is legal.
+bool Sema::CheckX86BuiltinGatherScatterScale(unsigned BuiltinID,
+ CallExpr *TheCall) {
+  unsigned ArgNum = 0;
+  switch (BuiltinID) {
+  default:
+return false;
+  case X86::BI__builtin_ia32_gatherpfdpd:
+  case X86::BI__builtin_ia32_gatherpfdps:
+  case X86::BI__builtin_ia32_gatherpfqpd:
+  case X86::BI__builtin_ia32_gatherpfqps:
+  case X86::BI__builtin_ia32_scatterpfdpd:
+  case X86::BI__builtin_ia32_scatterpfdps:
+  case X86::BI__builtin_ia32_scatterpfqpd:
+  case X86::BI__builtin_ia32_scatterpfqps:
+ArgNum = 3;
+break;
+  case X86::BI__builtin_ia32_gatherd_pd:
+  case X86::BI__builtin_ia32_gatherd_pd256:
+  case X86::BI__builtin_ia32_gatherq_pd:
+  case X86::BI__builtin_ia32_gatherq_pd256:
+  case X86::BI__builtin_ia32_gatherd_ps:
+  case X86::BI__builtin_ia32_gatherd_ps256:
+  case X86::BI__builtin_ia32_gatherq_ps:
+  case X86::BI__builtin_ia32_gatherq_ps256:
+  case X86::BI__builtin_ia32_gatherd_q:
+  case X86::BI__builtin_ia32_gatherd_q256:
+  case X86::BI__builtin_ia32_gatherq_q:
+  case X86::BI__builtin_ia32_gatherq_q256:
+  case X86::BI__builtin_ia32_gatherd_d:
+  case X86::BI__builtin_ia32_gatherd_d256:
+  case X86::BI__builtin_ia32_gatherq_d:
+  case X86::BI__builtin_ia32_gatherq_d256:
+  case X86::BI__builtin_ia32_gather3div2df:
+  case X86::BI__builtin_ia32_gather3div2di:
+  case X86::BI__builtin_ia32_gather3div4df:
+  case X86::BI__builtin_ia32_gather3div4di:
+  case X86::BI__builtin_ia32_gather3div4sf:
+  case 

r297642 - [X86] Add checking of the scale argument to scatter/gather builtins

2017-03-13 Thread Craig Topper via cfe-commits
Author: ctopper
Date: Mon Mar 13 12:16:50 2017
New Revision: 297642

URL: http://llvm.org/viewvc/llvm-project?rev=297642=rev
Log:
[X86] Add checking of the scale argument to scatter/gather builtins

The only valid values for scale immediate of scatter/gather builtins are 1, 2, 
4, or 8. This patch enforces this in the frontend otherwise we generate invalid 
instruction encodings in the backend.

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



Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/builtins-x86.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=297642=297641=297642=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Mar 13 12:16:50 
2017
@@ -8005,6 +8005,8 @@ def err_x86_builtin_32_bit_tgt : Error<
   "this builtin is only available on x86-64 targets">;
 def err_x86_builtin_invalid_rounding : Error<
   "invalid rounding argument">;
+def err_x86_builtin_invalid_scale : Error<
+  "scale argument must be 1, 2, 4, or 8">;
 
 def err_builtin_longjmp_unsupported : Error<
   "__builtin_longjmp is not supported for the current target">;

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=297642=297641=297642=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon Mar 13 12:16:50 2017
@@ -9993,6 +9993,7 @@ private:
   bool CheckMipsBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckSystemZBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckX86BuiltinRoundingOrSAE(unsigned BuiltinID, CallExpr *TheCall);
+  bool CheckX86BuiltinGatherScatterScale(unsigned BuiltinID, CallExpr 
*TheCall);
   bool CheckX86BuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckPPCBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
 

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=297642=297641=297642=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Mar 13 12:16:50 2017
@@ -1986,6 +1986,109 @@ bool Sema::CheckX86BuiltinRoundingOrSAE(
 << Arg->getSourceRange();
 }
 
+// Check if the gather/scatter scale is legal.
+bool Sema::CheckX86BuiltinGatherScatterScale(unsigned BuiltinID,
+ CallExpr *TheCall) {
+  unsigned ArgNum = 0;
+  switch (BuiltinID) {
+  default:
+return false;
+  case X86::BI__builtin_ia32_gatherpfdpd:
+  case X86::BI__builtin_ia32_gatherpfdps:
+  case X86::BI__builtin_ia32_gatherpfqpd:
+  case X86::BI__builtin_ia32_gatherpfqps:
+  case X86::BI__builtin_ia32_scatterpfdpd:
+  case X86::BI__builtin_ia32_scatterpfdps:
+  case X86::BI__builtin_ia32_scatterpfqpd:
+  case X86::BI__builtin_ia32_scatterpfqps:
+ArgNum = 3;
+break;
+  case X86::BI__builtin_ia32_gatherd_pd:
+  case X86::BI__builtin_ia32_gatherd_pd256:
+  case X86::BI__builtin_ia32_gatherq_pd:
+  case X86::BI__builtin_ia32_gatherq_pd256:
+  case X86::BI__builtin_ia32_gatherd_ps:
+  case X86::BI__builtin_ia32_gatherd_ps256:
+  case X86::BI__builtin_ia32_gatherq_ps:
+  case X86::BI__builtin_ia32_gatherq_ps256:
+  case X86::BI__builtin_ia32_gatherd_q:
+  case X86::BI__builtin_ia32_gatherd_q256:
+  case X86::BI__builtin_ia32_gatherq_q:
+  case X86::BI__builtin_ia32_gatherq_q256:
+  case X86::BI__builtin_ia32_gatherd_d:
+  case X86::BI__builtin_ia32_gatherd_d256:
+  case X86::BI__builtin_ia32_gatherq_d:
+  case X86::BI__builtin_ia32_gatherq_d256:
+  case X86::BI__builtin_ia32_gather3div2df:
+  case X86::BI__builtin_ia32_gather3div2di:
+  case X86::BI__builtin_ia32_gather3div4df:
+  case X86::BI__builtin_ia32_gather3div4di:
+  case X86::BI__builtin_ia32_gather3div4sf:
+  case X86::BI__builtin_ia32_gather3div4si:
+  case X86::BI__builtin_ia32_gather3div8sf:
+  case X86::BI__builtin_ia32_gather3div8si:
+  case X86::BI__builtin_ia32_gather3siv2df:
+  case X86::BI__builtin_ia32_gather3siv2di:
+  case X86::BI__builtin_ia32_gather3siv4df:
+  case X86::BI__builtin_ia32_gather3siv4di:
+  case X86::BI__builtin_ia32_gather3siv4sf:
+  case X86::BI__builtin_ia32_gather3siv4si:
+  case X86::BI__builtin_ia32_gather3siv8sf:
+  case X86::BI__builtin_ia32_gather3siv8si:
+  case X86::BI__builtin_ia32_gathersiv8df:
+  case X86::BI__builtin_ia32_gathersiv16sf:
+  case X86::BI__builtin_ia32_gatherdiv8df:
+  case X86::BI__builtin_ia32_gatherdiv16sf:
+  case 

Re: Patch for Bug 30413, including test case

2017-03-13 Thread Lobron, David via cfe-commits
Yes, please, if you don't mind!  I'd like to commit both the path and the unit 
test, if possible.

Thanks,

David

> On Mar 13, 2017, at 12:47 PM, Akira Hatanaka  wrote:
> 
> Do you need someone to commit this patch for you?
> 
>> On Mar 10, 2017, at 6:44 AM, Lobron, David  wrote:
>> 
>> Hi Akira,
>> 
>> Thank you very much!  Please let me know if I need to take any further steps 
>> beyond this email to cfe-commits in order for the patch and the unit test to 
>> be committed.
>> 
>> Thanks,
>> 
>> David
>> 
>>> On Mar 9, 2017, at 4:46 PM, Akira Hatanaka  wrote:
>>> 
>>> Hi David,
>>> 
>>> The patch looks good to me.
>>> 
 On Mar 9, 2017, at 1:01 PM, Lobron, David  wrote:
 
 Hi Akira,
 
> My concern is that the patch changes the encoding of 
> @encode(id) on Darwin, which I think isn’t what you are trying 
> to fix. If you compile the following code with command “clang -cc1 
> -triple x86_64-apple-macosx”, the type encoding changes after applying 
> the patch.
> 
> const char *foo() {
> return @encode(id);
> }
> 
> It seems like you can fix your problem without affecting Darwin by 
> passing an extra argument to getObjCEncodingForType, just like 
> CGObjCCommonMac::GetMethodVarType does.
 
 Ah, thanks- I understand now.  Yes, this change seems a lot safer, and I 
 verified that it passes my test.  I've attached my new patch file, and 
 I've also attached the test again.  Please let me know if this works for 
 you or if you think it needs any additional work.
 
 --David
 
 
>>> 
>> 
> 

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


Re: Patch for Bug 30413, including test case

2017-03-13 Thread Akira Hatanaka via cfe-commits
Do you need someone to commit this patch for you?

> On Mar 10, 2017, at 6:44 AM, Lobron, David  wrote:
> 
> Hi Akira,
> 
> Thank you very much!  Please let me know if I need to take any further steps 
> beyond this email to cfe-commits in order for the patch and the unit test to 
> be committed.
> 
> Thanks,
> 
> David
> 
>> On Mar 9, 2017, at 4:46 PM, Akira Hatanaka  wrote:
>> 
>> Hi David,
>> 
>> The patch looks good to me.
>> 
>>> On Mar 9, 2017, at 1:01 PM, Lobron, David  wrote:
>>> 
>>> Hi Akira,
>>> 
 My concern is that the patch changes the encoding of @encode(id) 
 on Darwin, which I think isn’t what you are trying to fix. If you compile 
 the following code with command “clang -cc1 -triple x86_64-apple-macosx”, 
 the type encoding changes after applying the patch.
 
 const char *foo() {
 return @encode(id);
 }
 
 It seems like you can fix your problem without affecting Darwin by passing 
 an extra argument to getObjCEncodingForType, just like 
 CGObjCCommonMac::GetMethodVarType does.
>>> 
>>> Ah, thanks- I understand now.  Yes, this change seems a lot safer, and I 
>>> verified that it passes my test.  I've attached my new patch file, and I've 
>>> also attached the test again.  Please let me know if this works for you or 
>>> if you think it needs any additional work.
>>> 
>>> --David
>>> 
>>> 
>> 
> 

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


[PATCH] D30810: Preserve vec3 type.

2017-03-13 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

Would you be able to update ScalarExprEmitter::VisitAsTypeExpr implementation 
accordingly to make things consistent?


https://reviews.llvm.org/D30810



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


[PATCH] D30816: [OpenCL] Added implicit conversion rank for overloading functions with vector data type in OpenCL

2017-03-13 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: test/SemaOpenCL/overload-scalar-widening.cl:4
+
+typedef short short4 __attribute__((ext_vector_type(4)));
+

I am thinking could this be a CodeGen test instead and we could check that the 
right overload is selected based on mangled name?

I think in this case it would be good to unify with 
test/SemaOpenCL/overload_addrspace_resolution.cl which has similar purpose. 
Also I think CodeGenOpenCL would be a better place for it. :) 


https://reviews.llvm.org/D30816



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


[PATCH] D30766: Add support for attribute "enum_extensibility"

2017-03-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments.



Comment at: test/Sema/enum-attr.c:27
+
+enum __attribute__((enum_extensibility(arg1))) EnumInvalidArg { // 
expected-warning{{'enum_extensibility' attribute argument not supported: 
'arg1'}}
+  G

arphaman wrote:
> Should this be an error instead?
Yes, I agree.


https://reviews.llvm.org/D30766



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


[PATCH] D30766: Add support for attribute "enum_extensibility"

2017-03-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 91577.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.

Address Alex's review comments.


https://reviews.llvm.org/D30766

Files:
  include/clang/AST/Decl.h
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/Decl.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaStmt.cpp
  test/Sema/enum-attr.c
  test/SemaCXX/attr-flag-enum-reject.cpp
  test/SemaCXX/enum-attr.cpp

Index: test/SemaCXX/enum-attr.cpp
===
--- /dev/null
+++ test/SemaCXX/enum-attr.cpp
@@ -0,0 +1,108 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wassign-enum -Wswitch-enum -Wcovered-switch-default -std=c++11 %s
+
+enum Enum {
+  A0 = 1, A1 = 10
+};
+
+enum __attribute__((enum_extensibility(closed))) EnumClosed {
+  B0 = 1, B1 = 10
+};
+
+enum [[clang::enum_extensibility(open)]] EnumOpen {
+  C0 = 1, C1 = 10
+};
+
+enum __attribute__((flag_enum)) EnumFlag {
+  D0 = 1, D1 = 8
+};
+
+enum __attribute__((flag_enum,enum_extensibility(closed))) EnumFlagClosed {
+  E0 = 1, E1 = 8
+};
+
+enum __attribute__((flag_enum,enum_extensibility(open))) EnumFlagOpen {
+  F0 = 1, F1 = 8
+};
+
+void test() {
+  enum Enum t0;
+
+  switch (t0) { // expected-warning{{enumeration value 'A1' not handled in switch}}
+  case A0: break;
+  case 16: break; // expected-warning{{case value not in enumerated type}}
+  }
+
+  switch (t0) {
+  case A0: break;
+  case A1: break;
+  default: break; // expected-warning{{default label in switch which covers all enumeration}}
+  }
+
+  enum EnumClosed t1;
+
+  switch (t1) { // expected-warning{{enumeration value 'B1' not handled in switch}}
+  case B0: break;
+  case 16: break; // expected-warning{{case value not in enumerated type}}
+  }
+
+  switch (t1) {
+  case B0: break;
+  case B1: break;
+  default: break; // expected-warning{{default label in switch which covers all enumeration}}
+  }
+
+  enum EnumOpen t2;
+
+  switch (t2) { // expected-warning{{enumeration value 'C1' not handled in switch}}
+  case C0: break;
+  case 16: break;
+  }
+
+  switch (t2) {
+  case C0: break;
+  case C1: break;
+  default: break;
+  }
+
+  enum EnumFlag t3;
+
+  switch (t3) { // expected-warning{{enumeration value 'D1' not handled in switch}}
+  case D0: break;
+  case 9: break;
+  case 16: break; // expected-warning{{case value not in enumerated type}}
+  }
+
+  switch (t3) {
+  case D0: break;
+  case D1: break;
+  default: break;
+  }
+
+  enum EnumFlagClosed t4;
+
+  switch (t4) { // expected-warning{{enumeration value 'E1' not handled in switch}}
+  case E0: break;
+  case 9: break;
+  case 16: break; // expected-warning{{case value not in enumerated type}}
+  }
+
+  switch (t4) {
+  case E0: break;
+  case E1: break;
+  default: break;
+  }
+
+  enum EnumFlagOpen t5;
+
+  switch (t5) { // expected-warning{{enumeration value 'F1' not handled in switch}}
+  case F0: break;
+  case 9: break;
+  case 16: break;
+  }
+
+  switch (t5) {
+  case F0: break;
+  case F1: break;
+  default: break;
+  }
+}
Index: test/SemaCXX/attr-flag-enum-reject.cpp
===
--- test/SemaCXX/attr-flag-enum-reject.cpp
+++ /dev/null
@@ -1,4 +0,0 @@
-// RUN: %clang_cc1 -verify -fsyntax-only -x c++ -Wassign-enum %s
-
-enum __attribute__((flag_enum)) flag { // expected-warning {{ignored}}
-};
Index: test/Sema/enum-attr.c
===
--- /dev/null
+++ test/Sema/enum-attr.c
@@ -0,0 +1,118 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wassign-enum -Wswitch-enum -Wcovered-switch-default %s
+
+enum Enum {
+  A0 = 1, A1 = 10
+};
+
+enum __attribute__((enum_extensibility(closed))) EnumClosed {
+  B0 = 1, B1 = 10
+};
+
+enum __attribute__((enum_extensibility(open))) EnumOpen {
+  C0 = 1, C1 = 10
+};
+
+enum __attribute__((flag_enum)) EnumFlag {
+  D0 = 1, D1 = 8
+};
+
+enum __attribute__((flag_enum,enum_extensibility(closed))) EnumFlagClosed {
+  E0 = 1, E1 = 8
+};
+
+enum __attribute__((flag_enum,enum_extensibility(open))) EnumFlagOpen {
+  F0 = 1, F1 = 8
+};
+
+enum __attribute__((enum_extensibility(arg1))) EnumInvalidArg { // expected-error{{'enum_extensibility' attribute argument not supported: 'arg1'}}
+  G
+};
+
+void test() {
+  enum Enum t0 = 100; // expected-warning{{integer constant not in range of enumerated type}}
+  t0 = 1;
+
+  switch (t0) { // expected-warning{{enumeration value 'A1' not handled in switch}}
+  case A0: break;
+  case 16: break; // expected-warning{{case value not in enumerated type}}
+  }
+
+  switch (t0) {
+  case A0: break;
+  case A1: break;
+  default: break; // expected-warning{{default label in switch which covers all enumeration}}
+  }
+
+  enum EnumClosed t1 = 100; // expected-warning{{integer constant not in range of enumerated type}}
+  t1 = 1;
+
+  switch (t1) { // expected-warning{{enumeration value 'B1' not handled 

[PATCH] D30898: Add new -fverbose-asm that enables source interleaving

2017-03-13 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits
rogfer01 created this revision.

This is the clang side of the RFC in 
http://lists.llvm.org/pipermail/cfe-dev/2017-February/052549.html

Note that in contrast to the original suggestion `-fsource-asm` here we use the 
preferred `-fverbose-asm`. Basically explicitly saying `-fverbose-asm` in the 
command line enables a minimum amount of debugging, so in AsmPrinter we can use 
it to print the source code.

This patch introduces a `-masm-source` flag for cc1 that maps to the AsmSource 
value in the llvm code generation.


https://reviews.llvm.org/D30898

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp


Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -560,6 +560,7 @@
   Args.hasFlag(OPT_fcoverage_mapping, OPT_fno_coverage_mapping, false);
   Opts.DumpCoverageMapping = Args.hasArg(OPT_dump_coverage_mapping);
   Opts.AsmVerbose = Args.hasArg(OPT_masm_verbose);
+  Opts.AsmSource = getLastArgIntValue(Args, OPT_masm_source, 0, Diags);
   Opts.PreserveAsmComments = !Args.hasArg(OPT_fno_preserve_as_comments);
   Opts.AssumeSaneOperatorNew = !Args.hasArg(OPT_fno_assume_sane_operator_new);
   Opts.ObjCAutoRefCountExceptions = Args.hasArg(OPT_fobjc_arc_exceptions);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2748,6 +2748,20 @@
 CmdArgs.push_back("-split-dwarf=Enable");
   }
 
+  // If -fverbose-asm explicitly appears enable at least DebugLineTablesOnly
+  // but remember that no debug info was requested, to avoid cluttering
+  // assembler output with debug directives.
+  if (Args.hasArg(options::OPT_fverbose_asm)) {
+CmdArgs.push_back("-masm-source");
+if (DebugInfoKind == codegenoptions::NoDebugInfo) {
+  // FIXME: Check whether LimitedDebugInfo will give us line information,
+  // otherwise we should be overriding it as well.
+  DebugInfoKind = codegenoptions::DebugLineTablesOnly;
+  CmdArgs.push_back("1");
+} else
+  CmdArgs.push_back("2");
+  }
+
   // After we've dealt with all combinations of things that could
   // make DebugInfoKind be other than None or DebugLineTablesOnly,
   // figure out if we need to "upgrade" it to standalone debug info.
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -604,6 +604,7 @@
   Options.MCOptions.MCPIECopyRelocations = CodeGenOpts.PIECopyRelocations;
   Options.MCOptions.MCFatalWarnings = CodeGenOpts.FatalWarnings;
   Options.MCOptions.AsmVerbose = CodeGenOpts.AsmVerbose;
+  Options.MCOptions.AsmSource = CodeGenOpts.AsmSource;
   Options.MCOptions.PreserveAsmComments = CodeGenOpts.PreserveAsmComments;
   Options.MCOptions.ABIName = TargetOpts.ABI;
   for (const auto  : HSOpts.UserEntries)
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -32,6 +32,7 @@
 CODEGENOPT(CompressDebugSections, 1, 0) ///< -Wa,-compress-debug-sections
 CODEGENOPT(RelaxELFRelocations, 1, 0) ///< -Wa,--mrelax-relocations
 CODEGENOPT(AsmVerbose, 1, 0) ///< -dA, -fverbose-asm.
+CODEGENOPT(AsmSource , 2, 0) ///< -fverbose-asm.
 CODEGENOPT(PreserveAsmComments, 1, 1) ///< -dA, -fno-preserve-as-comments.
 CODEGENOPT(AssumeSaneOperatorNew , 1, 1) ///< implicit __attribute__((malloc)) 
operator new
 CODEGENOPT(Autolink  , 1, 1) ///< -fno-autolink
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -225,6 +225,8 @@
   HelpText<"Turn off struct-path aware Type Based Alias Analysis">;
 def masm_verbose : Flag<["-"], "masm-verbose">,
   HelpText<"Generate verbose assembly output">;
+def masm_source : Separate<["-"], "masm-source">,
+  HelpText<"Annotate assembly output with source code lines.">;
 def mcode_model : Separate<["-"], "mcode-model">,
   HelpText<"The code model to use">;
 def mdebug_pass : Separate<["-"], "mdebug-pass">,


Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -560,6 +560,7 @@
   Args.hasFlag(OPT_fcoverage_mapping, OPT_fno_coverage_mapping, false);
   Opts.DumpCoverageMapping = Args.hasArg(OPT_dump_coverage_mapping);
   Opts.AsmVerbose = Args.hasArg(OPT_masm_verbose);
+  Opts.AsmSource = getLastArgIntValue(Args, OPT_masm_source, 0, Diags);
   

[PATCH] D30720: [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

2017-03-13 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL297630: [include-fixer] Add fuzzy SymbolIndex, where 
identifier needn't match exactly. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D30720?vs=91042=91570#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30720

Files:
  clang-tools-extra/trunk/include-fixer/CMakeLists.txt
  clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.cpp
  clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.h
  clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
  clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
  clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml
  clang-tools-extra/trunk/test/include-fixer/yaml_fuzzy.cpp
  clang-tools-extra/trunk/unittests/include-fixer/CMakeLists.txt
  clang-tools-extra/trunk/unittests/include-fixer/FuzzySymbolIndexTests.cpp

Index: clang-tools-extra/trunk/unittests/include-fixer/CMakeLists.txt
===
--- clang-tools-extra/trunk/unittests/include-fixer/CMakeLists.txt
+++ clang-tools-extra/trunk/unittests/include-fixer/CMakeLists.txt
@@ -13,6 +13,7 @@
 
 add_extra_unittest(IncludeFixerTests
   IncludeFixerTest.cpp
+  FuzzySymbolIndexTests.cpp
   )
 
 target_link_libraries(IncludeFixerTests
Index: clang-tools-extra/trunk/unittests/include-fixer/FuzzySymbolIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/include-fixer/FuzzySymbolIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/include-fixer/FuzzySymbolIndexTests.cpp
@@ -0,0 +1,61 @@
+//===-- FuzzySymbolIndexTests.cpp - Fuzzy symbol index unit tests -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "FuzzySymbolIndex.h"
+#include "gmock/gmock.h"
+#include "llvm/Support/Regex.h"
+#include "gtest/gtest.h"
+
+using testing::ElementsAre;
+using testing::Not;
+
+namespace clang {
+namespace include_fixer {
+namespace {
+
+TEST(FuzzySymbolIndexTest, Tokenize) {
+  EXPECT_THAT(FuzzySymbolIndex::tokenize("URLHandlerCallback"),
+  ElementsAre("url", "handler", "callback"));
+  EXPECT_THAT(FuzzySymbolIndex::tokenize("snake_case11"),
+  ElementsAre("snake", "case", "11"));
+  EXPECT_THAT(FuzzySymbolIndex::tokenize("__$42!!BOB\nbob"),
+  ElementsAre("42", "bob", "bob"));
+}
+
+MATCHER_P(MatchesSymbol, Identifier, "") {
+  llvm::Regex Pattern("^" + arg);
+  std::string err;
+  if (!Pattern.isValid(err)) {
+*result_listener << "invalid regex: " << err;
+return false;
+  }
+  auto Tokens = FuzzySymbolIndex::tokenize(Identifier);
+  std::string Target = llvm::join(Tokens.begin(), Tokens.end(), " ");
+  *result_listener << "matching against '" << Target << "'";
+  return llvm::Regex("^" + arg).match(Target);
+}
+
+TEST(FuzzySymbolIndexTest, QueryRegexp) {
+  auto QueryRegexp = [](const std::string ) {
+return FuzzySymbolIndex::queryRegexp(FuzzySymbolIndex::tokenize(query));
+  };
+  EXPECT_THAT(QueryRegexp("uhc"), MatchesSymbol("URLHandlerCallback"));
+  EXPECT_THAT(QueryRegexp("urhaca"), MatchesSymbol("URLHandlerCallback"));
+  EXPECT_THAT(QueryRegexp("uhcb"), Not(MatchesSymbol("URLHandlerCallback")))
+  << "Non-prefix";
+  EXPECT_THAT(QueryRegexp("uc"), Not(MatchesSymbol("URLHandlerCallback")))
+  << "Skip token";
+
+  EXPECT_THAT(QueryRegexp("uptr"), MatchesSymbol("unique_ptr"));
+  EXPECT_THAT(QueryRegexp("UniP"), MatchesSymbol("unique_ptr"));
+}
+
+} // namespace
+} // namespace include_fixer
+} // namespace clang
Index: clang-tools-extra/trunk/include-fixer/CMakeLists.txt
===
--- clang-tools-extra/trunk/include-fixer/CMakeLists.txt
+++ clang-tools-extra/trunk/include-fixer/CMakeLists.txt
@@ -6,6 +6,7 @@
   IncludeFixer.cpp
   IncludeFixerContext.cpp
   InMemorySymbolIndex.cpp
+  FuzzySymbolIndex.cpp
   SymbolIndexManager.cpp
   YamlSymbolIndex.cpp
 
Index: clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
===
--- clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
+++ clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
@@ -7,6 +7,7 @@
 //
 //===--===//
 
+#include "FuzzySymbolIndex.h"
 #include "InMemorySymbolIndex.h"
 #include "IncludeFixer.h"
 #include "IncludeFixerContext.h"
@@ -83,14 +84,16 @@
 cl::OptionCategory IncludeFixerCategory("Tool options");
 
 enum DatabaseFormatTy {
-  fixed, ///< Hard-coded mapping.
-  yaml,  ///< Yaml database created by find-all-symbols.
+  fixed, ///< Hard-coded mapping.
+  yaml,  

[clang-tools-extra] r297630 - [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

2017-03-13 Thread Sam McCall via cfe-commits
Author: sammccall
Date: Mon Mar 13 10:55:59 2017
New Revision: 297630

URL: http://llvm.org/viewvc/llvm-project?rev=297630=rev
Log:
[include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

Summary:
Add fuzzy SymbolIndex, where identifier needn't match exactly.

The purpose for this is global autocomplete in clangd. The query will be a
partial identifier up to the cursor, and the results will be suggestions.

It's in include-fixer because:

  - it handles SymbolInfos, actually SymbolIndex is exactly the right interface
  - it's a good harness for lit testing the fuzzy YAML index
  - (Laziness: we can't unit test clangd until reorganizing with a tool/ dir)

Other questionable choices:

  - FuzzySymbolIndex, which just refines the contract of SymbolIndex. This is
an interface to allow extension to large monorepos (*cough*)
  - an always-true safety check that Identifier == Name is removed from
SymbolIndexManager, as it's not true for fuzzy matching
  - exposing -db=fuzzyYaml from include-fixer is not a very useful feature, and
a non-orthogonal ui (fuzziness vs data source). -db=fixed is similar though.

Reviewers: bkramer

Subscribers: cfe-commits, mgorny

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

Added:
clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.cpp
clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.h
clang-tools-extra/trunk/test/include-fixer/yaml_fuzzy.cpp
clang-tools-extra/trunk/unittests/include-fixer/FuzzySymbolIndexTests.cpp
Modified:
clang-tools-extra/trunk/include-fixer/CMakeLists.txt
clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml
clang-tools-extra/trunk/unittests/include-fixer/CMakeLists.txt

Modified: clang-tools-extra/trunk/include-fixer/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/CMakeLists.txt?rev=297630=297629=297630=diff
==
--- clang-tools-extra/trunk/include-fixer/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/include-fixer/CMakeLists.txt Mon Mar 13 10:55:59 
2017
@@ -6,6 +6,7 @@ add_clang_library(clangIncludeFixer
   IncludeFixer.cpp
   IncludeFixerContext.cpp
   InMemorySymbolIndex.cpp
+  FuzzySymbolIndex.cpp
   SymbolIndexManager.cpp
   YamlSymbolIndex.cpp
 

Added: clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.cpp?rev=297630=auto
==
--- clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.cpp (added)
+++ clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.cpp Mon Mar 13 
10:55:59 2017
@@ -0,0 +1,143 @@
+//===--- FuzzySymbolIndex.cpp - Lookup symbols for autocomplete -*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "FuzzySymbolIndex.h"
+#include "llvm/Support/Regex.h"
+
+using clang::find_all_symbols::SymbolAndSignals;
+using llvm::StringRef;
+
+namespace clang {
+namespace include_fixer {
+namespace {
+
+class MemSymbolIndex : public FuzzySymbolIndex {
+public:
+  MemSymbolIndex(std::vector Symbols) {
+for (auto  : Symbols) {
+  auto Tokens = tokenize(Symbol.Symbol.getName());
+  this->Symbols.emplace_back(
+  StringRef(llvm::join(Tokens.begin(), Tokens.end(), " ")),
+  std::move(Symbol));
+}
+  }
+
+  std::vector search(StringRef Query) override {
+auto Tokens = tokenize(Query);
+llvm::Regex Pattern("^" + queryRegexp(Tokens));
+std::vector Results;
+for (const Entry  : Symbols)
+  if (Pattern.match(E.first))
+Results.push_back(E.second);
+return Results;
+  }
+
+private:
+  using Entry = std::pair, SymbolAndSignals>;
+  std::vector Symbols;
+};
+
+// Helpers for tokenize state machine.
+enum TokenizeState {
+  EMPTY,  // No pending characters.
+  ONE_BIG,// Read one uppercase letter, could be WORD or Word.
+  BIG_WORD,   // Reading an uppercase WORD.
+  SMALL_WORD, // Reading a lowercase word.
+  NUMBER  // Reading a number.
+};
+
+enum CharType { UPPER, LOWER, DIGIT, MISC };
+CharType classify(char c) {
+  if (isupper(c))
+return UPPER;
+  if (islower(c))
+return LOWER;
+  if (isdigit(c))
+return DIGIT;
+  return MISC;
+}
+
+} // namespace
+
+std::vector FuzzySymbolIndex::tokenize(StringRef Text) {
+  std::vector Result;
+  // State describes the treatment of text from Start to I.
+  // Once text is Flush()ed into Result, we're done with it and advance Start.
+  

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

2017-03-13 Thread Jonathan Roelofs via Phabricator via cfe-commits
jroelofs added a comment.

In https://reviews.llvm.org/D30158#699132, @madsravn wrote:

> In https://reviews.llvm.org/D30158#698871, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D30158#696534, @madsravn wrote:
> >
> > > Any updates on this?
> >
> >
> > Have you run it over the test suite on the revision before random_shuffle 
> > was removed from libc++?
>
>
> I can't find any more tests for random_shuffle. I have looked in the SVN log 
> for from december 2014 until now. It works on the three tests currently in 
> trunk.


Try just before r294328.


https://reviews.llvm.org/D30158



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


[PATCH] D30896: [CLANG-TIDY] add check misc-prefer-switch-for-enums

2017-03-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

I like that check. But I think it could take another feature.
In my opinion it would be valueable to check if enums are compared against ints 
as well.

For example

  enum Kind k = Kind::a;
  if (k == 3) { /* something */ }

This usecase is not specially considered here, but i think that check would be 
the right place. Maybe it is already covered by your matcher, since you check 
for everything that compares against enums.
If yes, additional diagnostics would be nice.

This check should be added to the ReleaseNotes as well.




Comment at: clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.cpp:21
+void PreferSwitchForEnumsCheck::registerMatchers(MatchFinder *Finder) {
+  // FIXME: Add matchers.
+  Finder->addMatcher(

remove `FIXME`



Comment at: clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.cpp:27
+ hasLHS(declRefExpr(hasDeclaration(enumConstantDecl()))
+  .bind("x"),
+  this);

maybe a more telling name than `x` would be better?



Comment at: clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp:3
+
+enum class kind { a, b, c, d };
+

maybe another test for non enum class values would be nice.

```
enum another_kind {e, f, g};
```




Repository:
  rL LLVM

https://reviews.llvm.org/D30896



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


[PATCH] D30720: [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

2017-03-13 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision.
bkramer added a comment.

lg as a prototype.


https://reviews.llvm.org/D30720



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


r297628 - [CodeCompletion] Format block parameter placeholders in implicit property

2017-03-13 Thread Alex Lorenz via cfe-commits
Author: arphaman
Date: Mon Mar 13 10:43:42 2017
New Revision: 297628

URL: http://llvm.org/viewvc/llvm-project?rev=297628=rev
Log:
[CodeCompletion] Format block parameter placeholders in implicit property
setters using the block type information that's obtained from the property

rdar://12604235

Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/Index/complete-block-properties.m

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=297628=297627=297628=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Mon Mar 13 10:43:42 2017
@@ -2288,6 +2288,15 @@ static std::string FormatFunctionParamet
   FunctionProtoTypeLoc BlockProto;
   findTypeLocationForBlockDecl(Param->getTypeSourceInfo(), Block, BlockProto,
SuppressBlock);
+  // Try to retrieve the block type information from the property if this is a
+  // parameter in a setter.
+  if (!Block && ObjCMethodParam &&
+  cast(Param->getDeclContext())->isPropertyAccessor()) {
+if (const auto *PD = cast(Param->getDeclContext())
+ ->findPropertyDecl(/*CheckOverrides=*/false))
+  findTypeLocationForBlockDecl(PD->getTypeSourceInfo(), Block, BlockProto,
+   SuppressBlock);
+  }
 
   if (!Block) {
 // We were unable to find a FunctionProtoTypeLoc with parameter names

Modified: cfe/trunk/test/Index/complete-block-properties.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/complete-block-properties.m?rev=297628=297627=297628=diff
==
--- cfe/trunk/test/Index/complete-block-properties.m (original)
+++ cfe/trunk/test/Index/complete-block-properties.m Mon Mar 13 10:43:42 2017
@@ -68,8 +68,8 @@ void noQualifierParens(NoQualifierParens
 // RUN: c-index-test -code-completion-at=%s:65:6 %s | FileCheck 
-check-prefix=CHECK-CC2 %s
 //CHECK-CC2: ObjCInstanceMethodDecl:{ResultType void (^)(void)}{TypedText 
blockProperty} (35)
 //CHECK-CC2-NEXT: ObjCInstanceMethodDecl:{ResultType BarBlock}{TypedText 
blockProperty2} (35)
-//CHECK-CC2-NEXT: ObjCInstanceMethodDecl:{ResultType void}{TypedText 
setBlockProperty2:}{Placeholder BarBlock blockProperty2} (35)
-//CHECK-CC2-NEXT: ObjCInstanceMethodDecl:{ResultType void}{TypedText 
setBlockProperty:}{Placeholder void (^)(void)blockProperty} (35)
+//CHECK-CC2-NEXT: ObjCInstanceMethodDecl:{ResultType void}{TypedText 
setBlockProperty2:}{Placeholder ^int(int *)blockProperty2} (35)
+//CHECK-CC2-NEXT: ObjCInstanceMethodDecl:{ResultType void}{TypedText 
setBlockProperty:}{Placeholder ^(void)blockProperty} (35)
 
 @interface ClassProperties
 
@@ -86,3 +86,9 @@ void classBlockProperties() {
 //CHECK-CC3: ObjCPropertyDecl:{ResultType void}{TypedText explicit}{LeftParen 
(}{RightParen )} (35)
 //CHECK-CC3-NEXT: ObjCPropertyDecl:{ResultType void (^)()}{TypedText 
explicit}{Equal  = }{Placeholder ^(void)} (38)
 //CHECK-CC3-NEXT: ObjCPropertyDecl:{ResultType void}{TypedText 
explicitReadonly}{LeftParen (}{RightParen )} (35)
+
+void implicitSetterBlockPlaceholder(Test* test) {
+  [test setBlock: ^{}];
+}
+// RUN: c-index-test -code-completion-at=%s:91:9 %s | FileCheck 
-check-prefix=CHECK-CC4 %s
+// CHECK-CC4: ObjCInstanceMethodDecl:{ResultType void}{TypedText 
setBlocker:}{Placeholder ^Foo(int x, Foo y, FooBlock foo)blocker} (37)


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


[PATCH] D30896: [CLANG-TIDY] add check misc-prefer-switch-for-enums

2017-03-13 Thread Jonathan B Coe via Phabricator via cfe-commits
jbcoe created this revision.
jbcoe added a project: clang-tools-extra.
Herald added subscribers: JDevlieghere, mgorny.

Add a check to find enums used in `==` or `!=` expressions. Using a switch 
statement leads to more easily maintained code.


Repository:
  rL LLVM

https://reviews.llvm.org/D30896

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.cpp
  clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.h
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst
  clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp

Index: clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp
@@ -0,0 +1,40 @@
+// RUN: %check_clang_tidy %s misc-prefer-switch-for-enums %t 
+
+enum class kind { a, b, c, d };
+
+int foo(kind k)
+{
+  if (k == kind::a)
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer a switch statement for enum-value checks [misc-prefer-switch-for-enums]
+return -1;
+  return 1;
+}
+
+int antifoo(kind k)
+{
+  if (k != kind::a)
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer a switch statement for enum-value checks [misc-prefer-switch-for-enums]
+return 1;
+  return -1;
+}
+
+int bar(int i)
+{
+  if (i == 0)
+return -1;
+  return 1;
+}
+
+int foobar(kind k)
+{
+  switch(k)
+  {
+case kind::a:
+  return -1;
+case kind::b:
+case kind::c:
+case kind::d:
+  return 1;
+  }
+}
+
Index: clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst
@@ -0,0 +1,40 @@
+.. title:: clang-tidy - misc-prefer-switch-for-enums
+
+misc-prefer-switch-for-enums
+
+
+This check finds enum values used in ``==`` and ``!=`` expressions.
+
+A switch statement will robustly identify unhandled enum cases with a compiler
+warning. No such warning exists for control flow based on enum value
+comparison.  Use of switches rather than if statements leads to easier to
+maintain code as adding values to an enum will trigger compiler warnings for
+unhandled cases.
+
+
+.. code-block:: c++
+
+  enum class kind { a, b, c};
+  
+  int foo(kind k) { 
+return k == kind::a 
+   ? 1 
+   : -1; 
+  }
+
+is more maintainably (but more verbosely) written as:
+
+.. code-block:: c++
+
+  enum class kind { a, b, c};
+  
+  int foo(kind k) { 
+switch(k) { 
+  case kind::a:
+return 1;
+  case kind::b:
+  case kind::c:
+return -1;
+}
+  }
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -78,6 +78,7 @@
misc-new-delete-overloads
misc-noexcept-move-constructor
misc-non-copyable-objects
+   misc-prefer-switch-for-enums
misc-redundant-expression
misc-sizeof-container
misc-sizeof-expression
Index: clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.h
@@ -0,0 +1,36 @@
+//===--- PreferSwitchForEnumsCheck.h - clang-tidy*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PREFER_SWITCH_FOR_ENUMS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PREFER_SWITCH_FOR_ENUMS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Find places where an enumeration value is used in `==` or `!=`. 
+/// Using `switch` leads to more maintainable code.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-prefer-switch-for-enums.html
+class PreferSwitchForEnumsCheck : public ClangTidyCheck {
+public:
+  PreferSwitchForEnumsCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PREFER_SWITCH_FOR_ENUMS_H
Index: 

[PATCH] D30720: [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

2017-03-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

bkramer: ping


https://reviews.llvm.org/D30720



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


[PATCH] D30831: [ASTImporter] Import fix of GCCAsmStmts w/ missing symbolic operands

2017-03-13 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL297627: [ASTImporter] Import fix of GCCAsmStmts w/ missing 
symbolic operands (authored by xazax).

Changed prior to commit:
  https://reviews.llvm.org/D30831?vs=91541=91567#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30831

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
  cfe/trunk/test/ASTMerge/asm/test.cpp


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -5218,13 +5218,17 @@
   SmallVector Names;
   for (unsigned I = 0, E = S->getNumOutputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for output operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getOutputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
   for (unsigned I = 0, E = S->getNumInputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getInputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for input operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getInputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
Index: cfe/trunk/test/ASTMerge/asm/test.cpp
===
--- cfe/trunk/test/ASTMerge/asm/test.cpp
+++ cfe/trunk/test/ASTMerge/asm/test.cpp
@@ -4,4 +4,5 @@
 
 void testAsmImport() {
   asmFunc(12, 42);
+  asmFunc2(42);
 }
Index: cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
===
--- cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
+++ cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
@@ -9,3 +9,13 @@
   res = bigres;
   return res;
 }
+
+int asmFunc2(int i) {
+  int res;
+  asm ("mov %1, %0 \t\n"
+   "inc %0 "
+  : "=r" (res)
+  : "r" (i)
+  : "cc");
+  return res;
+}


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -5218,13 +5218,17 @@
   SmallVector Names;
   for (unsigned I = 0, E = S->getNumOutputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for output operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getOutputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
   for (unsigned I = 0, E = S->getNumInputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getInputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for input operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getInputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
Index: cfe/trunk/test/ASTMerge/asm/test.cpp
===
--- cfe/trunk/test/ASTMerge/asm/test.cpp
+++ cfe/trunk/test/ASTMerge/asm/test.cpp
@@ -4,4 +4,5 @@
 
 void testAsmImport() {
   asmFunc(12, 42);
+  asmFunc2(42);
 }
Index: cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
===
--- cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
+++ cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
@@ -9,3 +9,13 @@
   res = bigres;
   return res;
 }
+
+int asmFunc2(int i) {
+  int res;
+  asm ("mov %1, %0 \t\n"
+   "inc %0 "
+  : "=r" (res)
+  : "r" (i)
+  : "cc");
+  return res;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r297627 - [ASTImporter] Import fix of GCCAsmStmts w/ missing symbolic operands

2017-03-13 Thread Gabor Horvath via cfe-commits
Author: xazax
Date: Mon Mar 13 10:32:24 2017
New Revision: 297627

URL: http://llvm.org/viewvc/llvm-project?rev=297627=rev
Log:
[ASTImporter] Import fix of GCCAsmStmts w/ missing symbolic operands

Patch by Zoltan Gera!

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

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
cfe/trunk/test/ASTMerge/asm/test.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=297627=297626=297627=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Mar 13 10:32:24 2017
@@ -5218,13 +5218,17 @@ Stmt *ASTNodeImporter::VisitGCCAsmStmt(G
   SmallVector Names;
   for (unsigned I = 0, E = S->getNumOutputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for output operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getOutputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
   for (unsigned I = 0, E = S->getNumInputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getInputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for input operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getInputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }

Modified: cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp?rev=297627=297626=297627=diff
==
--- cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp (original)
+++ cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp Mon Mar 13 10:32:24 2017
@@ -9,3 +9,13 @@ unsigned char asmFunc(unsigned char a, u
   res = bigres;
   return res;
 }
+
+int asmFunc2(int i) {
+  int res;
+  asm ("mov %1, %0 \t\n"
+   "inc %0 "
+  : "=r" (res)
+  : "r" (i)
+  : "cc");
+  return res;
+}

Modified: cfe/trunk/test/ASTMerge/asm/test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ASTMerge/asm/test.cpp?rev=297627=297626=297627=diff
==
--- cfe/trunk/test/ASTMerge/asm/test.cpp (original)
+++ cfe/trunk/test/ASTMerge/asm/test.cpp Mon Mar 13 10:32:24 2017
@@ -4,4 +4,5 @@
 
 void testAsmImport() {
   asmFunc(12, 42);
+  asmFunc2(42);
 }


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


[PATCH] D30831: [ASTImporter] Import fix of GCCAsmStmts w/ missing symbolic operands

2017-03-13 Thread Aleksei Sidorin via Phabricator via cfe-commits
a.sidorin accepted this revision.
a.sidorin added a comment.
This revision is now accepted and ready to land.

Looks good, thank you!


https://reviews.llvm.org/D30831



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


[PATCH] D29923: Add test for PPCallbacks::MacroUndefined

2017-03-13 Thread Frederich Munch via Phabricator via cfe-commits
marsupial updated this revision to Diff 91563.
marsupial retitled this revision from "Send UndefMacroDirective to MacroDefined 
callback" to "Add test for PPCallbacks::MacroUndefined".
marsupial edited the summary of this revision.

https://reviews.llvm.org/D29923

Files:
  unittests/Basic/SourceManagerTest.cpp

Index: unittests/Basic/SourceManagerTest.cpp
===
--- unittests/Basic/SourceManagerTest.cpp
+++ unittests/Basic/SourceManagerTest.cpp
@@ -246,12 +246,18 @@
 namespace {
 
 struct MacroAction {
+  enum Kind { kDefinition, kUnDefinition, kExpansion};
+
   SourceLocation Loc;
   std::string Name;
-  bool isDefinition; // if false, it is expansion.
-  
-  MacroAction(SourceLocation Loc, StringRef Name, bool isDefinition)
-: Loc(Loc), Name(Name), isDefinition(isDefinition) { }
+  unsigned MAKind : 2;
+
+  MacroAction(SourceLocation Loc, StringRef Name, unsigned K)
+: Loc(Loc), Name(Name), MAKind(K) { }
+
+  bool isExpansion() const { return MAKind == kExpansion; }
+  bool isDefinition() const { return MAKind == kDefinition; }
+  bool isUnDefinition() const { return MAKind == kUnDefinition; }
 };
 
 class MacroTracker : public PPCallbacks {
@@ -264,21 +270,29 @@
 const MacroDirective *MD) override {
 Macros.push_back(MacroAction(MD->getLocation(),
  MacroNameTok.getIdentifierInfo()->getName(),
- true));
+ MacroAction::kDefinition));
+  }
+  void MacroUndefined(const Token ,
+  const MacroDefinition ) override {
+Macros.push_back(MacroAction(MD.getMacroInfo()->getDefinitionLoc(),
+ MacroNameTok.getIdentifierInfo()->getName(),
+ MacroAction::kUnDefinition));
   }
   void MacroExpands(const Token , const MacroDefinition ,
 SourceRange Range, const MacroArgs *Args) override {
 Macros.push_back(MacroAction(MacroNameTok.getLocation(),
  MacroNameTok.getIdentifierInfo()->getName(),
- false));
+ MacroAction::kExpansion));
   }
 };
 
 }
 
 TEST_F(SourceManagerTest, isBeforeInTranslationUnitWithMacroInInclude) {
   const char *header =
-"#define MACRO_IN_INCLUDE 0\n";
+"#define MACRO_IN_INCLUDE 0\n"
+"#define MACRO_DEFINED\n"
+"#undef MACRO_DEFINED\n";
 
   const char *main =
 "#define M(x) x\n"
@@ -323,42 +337,50 @@
   // Make sure we got the tokens that we expected.
   ASSERT_EQ(0U, toks.size());
 
-  ASSERT_EQ(9U, Macros.size());
+  ASSERT_EQ(13U, Macros.size());
   // #define M(x) x
-  ASSERT_TRUE(Macros[0].isDefinition);
+  ASSERT_TRUE(Macros[0].isDefinition());
   ASSERT_EQ("M", Macros[0].Name);
   // #define INC "/test-header.h"
-  ASSERT_TRUE(Macros[1].isDefinition);
+  ASSERT_TRUE(Macros[1].isDefinition());
   ASSERT_EQ("INC", Macros[1].Name);
   // M expansion in #include M(INC)
-  ASSERT_FALSE(Macros[2].isDefinition);
+  ASSERT_FALSE(Macros[2].isDefinition());
   ASSERT_EQ("M", Macros[2].Name);
   // INC expansion in #include M(INC)
-  ASSERT_FALSE(Macros[3].isDefinition);
+  ASSERT_TRUE(Macros[3].isExpansion());
   ASSERT_EQ("INC", Macros[3].Name);
   // #define MACRO_IN_INCLUDE 0
-  ASSERT_TRUE(Macros[4].isDefinition);
+  ASSERT_TRUE(Macros[4].isDefinition());
   ASSERT_EQ("MACRO_IN_INCLUDE", Macros[4].Name);
+  // #define MACRO_DEFINED
+  ASSERT_TRUE(Macros[5].isDefinition());
+  ASSERT_FALSE(Macros[5].isUnDefinition());
+  ASSERT_EQ("MACRO_DEFINED", Macros[5].Name);
+  // #undef MACRO_DEFINED
+  ASSERT_FALSE(Macros[6].isDefinition());
+  ASSERT_TRUE(Macros[6].isUnDefinition());
+  ASSERT_EQ("MACRO_DEFINED", Macros[6].Name);
   // #define INC2 
-  ASSERT_TRUE(Macros[5].isDefinition);
-  ASSERT_EQ("INC2", Macros[5].Name);
+  ASSERT_TRUE(Macros[7].isDefinition());
+  ASSERT_EQ("INC2", Macros[7].Name);
   // M expansion in #include M(INC2)
-  ASSERT_FALSE(Macros[6].isDefinition);
-  ASSERT_EQ("M", Macros[6].Name);
+  ASSERT_FALSE(Macros[8].isDefinition());
+  ASSERT_EQ("M", Macros[8].Name);
   // INC2 expansion in #include M(INC2)
-  ASSERT_FALSE(Macros[7].isDefinition);
-  ASSERT_EQ("INC2", Macros[7].Name);
+  ASSERT_TRUE(Macros[9].isExpansion());
+  ASSERT_EQ("INC2", Macros[9].Name);
   // #define MACRO_IN_INCLUDE 0
-  ASSERT_TRUE(Macros[8].isDefinition);
-  ASSERT_EQ("MACRO_IN_INCLUDE", Macros[8].Name);
+  ASSERT_TRUE(Macros[10].isDefinition());
+  ASSERT_EQ("MACRO_IN_INCLUDE", Macros[10].Name);
 
   // The INC expansion in #include M(INC) comes before the first
   // MACRO_IN_INCLUDE definition of the included file.
   EXPECT_TRUE(SourceMgr.isBeforeInTranslationUnit(Macros[3].Loc, Macros[4].Loc));
 
   // The INC2 expansion in #include M(INC2) comes before the second
   // MACRO_IN_INCLUDE definition of the included file.
-  EXPECT_TRUE(SourceMgr.isBeforeInTranslationUnit(Macros[7].Loc, 

r297623 - [clang-format] Add more examples and fix a bug in the py generation script

2017-03-13 Thread Sylvestre Ledru via cfe-commits
Author: sylvestre
Date: Mon Mar 13 09:42:47 2017
New Revision: 297623

URL: http://llvm.org/viewvc/llvm-project?rev=297623=rev
Log:
[clang-format] Add more examples and fix a bug in the py generation script

Reviewers: djasper

Reviewed By: djasper

Subscribers: cfe-commits, klimek

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

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/docs/tools/dump_format_style.py
cfe/trunk/include/clang/Format/Format.h

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=297623=297622=297623=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Mon Mar 13 09:42:47 2017
@@ -252,6 +252,13 @@ the configuration (without a prefix: ``A
   Allow putting all parameters of a function declaration onto
   the next line even if ``BinPackParameters`` is ``false``.
 
+  .. code-block:: c++
+
+true:   false:
+myFunction(foo, vs. myFunction(foo, bar, plop);
+   bar,
+   plop);
+
 **AllowShortBlocksOnASingleLine** (``bool``)
   Allows contracting simple braced statements to a single line.
 
@@ -460,16 +467,148 @@ the configuration (without a prefix: ``A
 
   Nested configuration flags:
 
+
   * ``bool AfterClass`` Wrap class definitions.
+
+  .. code-block:: c++
+
+true:
+class foo {};
+
+false:
+class foo
+{};
+
   * ``bool AfterControlStatement`` Wrap control statements 
(``if``/``for``/``while``/``switch``/..).
+
+  .. code-block:: c++
+
+true:
+if (foo())
+{
+} else
+{}
+for (int i = 0; i < 10; ++i)
+{}
+
+false:
+if (foo()) {
+} else {
+}
+for (int i = 0; i < 10; ++i) {
+}
+
   * ``bool AfterEnum`` Wrap enum definitions.
+
+  .. code-block:: c++
+
+true:
+enum X : int
+{
+  B
+};
+
+false:
+enum X : int { B };
+
   * ``bool AfterFunction`` Wrap function definitions.
+
+  .. code-block:: c++
+
+true:
+void foo()
+{
+  bar();
+  bar2();
+}
+
+false:
+void foo() {
+  bar();
+  bar2();
+}
+
   * ``bool AfterNamespace`` Wrap namespace definitions.
+
+  .. code-block:: c++
+
+true:
+namespace
+{
+int foo();
+int bar();
+}
+
+false:
+namespace {
+int foo();
+int bar();
+}
+
   * ``bool AfterObjCDeclaration`` Wrap ObjC definitions (``@autoreleasepool``, 
interfaces, ..).
+
   * ``bool AfterStruct`` Wrap struct definitions.
+
+  .. code-block:: c++
+
+true:
+struct foo
+{
+  int x;
+}
+
+false:
+struct foo {
+  int x;
+}
+
   * ``bool AfterUnion`` Wrap union definitions.
+
+  .. code-block:: c++
+
+true:
+union foo
+{
+  int x;
+}
+
+false:
+union foo {
+  int x;
+}
+
   * ``bool BeforeCatch`` Wrap before ``catch``.
+
+  .. code-block:: c++
+
+true:
+try {
+  foo();
+}
+catch () {
+}
+
+false:
+try {
+  foo();
+} catch () {
+}
+
   * ``bool BeforeElse`` Wrap before ``else``.
+
+  .. code-block:: c++
+
+true:
+if (foo()) {
+}
+else {
+}
+
+false:
+if (foo()) {
+} else {
+}
+
   * ``bool IndentBraces`` Indent the wrapped braces themselves.
 
 
@@ -500,29 +639,146 @@ the configuration (without a prefix: ``A
   * ``BS_Attach`` (in configuration: ``Attach``)
 Always attach braces to surrounding context.
 
+.. code-block:: c++
+
+  try {
+foo();
+  } catch () {
+  }
+  void foo() { bar(); }
+  class foo {};
+  if (foo()) {
+  } else {
+  }
+  enum X : int { A, B };
+
   * ``BS_Linux`` (in configuration: ``Linux``)
 Like ``Attach``, but break before braces on function, namespace and
 class definitions.
 
+.. code-block:: c++
+
+  try {
+foo();
+  } catch () {
+  }
+  void foo() { bar(); }
+  class foo
+  {
+  };
+  if (foo()) {
+  } else {
+  }
+  enum X : int { A, B };
+
   * ``BS_Mozilla`` (in configuration: ``Mozilla``)
 Like ``Attach``, but break before braces on enum, function, and record
 definitions.
 
+.. code-block:: c++
+
+  try {
+foo();
+  } catch () {
+  }
+  void foo() { bar(); }
+  class foo
+  {
+  };
+  if (foo()) {
+  } else {
+  }
+  enum X : int { A, B };
+
   * ``BS_Stroustrup`` (in configuration: ``Stroustrup``)
 Like ``Attach``, but break before function definitions, ``catch``, and
 ``else``.
 
+.. code-block:: c++
+
+  try {
+foo();
+  } catch () {
+  }
+  void foo() { bar(); }
+  class foo
+  {
+  };
+  if (foo()) {
+  } else {
+  }
+  enum X : int
+  {
+A,

[PATCH] D29877: Warn about unused static file scope function template declarations.

2017-03-13 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added a comment.

@rsmith ping...


https://reviews.llvm.org/D29877



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


[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

2017-03-13 Thread Stanisław Barzowski via Phabricator via cfe-commits
sbarzowski added inline comments.



Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.h:20
+///\brief Warns about using throw in function declared as noexcept.
+/// It complains about every throw, even if it is caught later.
+class ThrowWithNoexceptCheck : public ClangTidyCheck {

sbarzowski wrote:
> vmiklos wrote:
> > Prazek wrote:
> > > aaron.ballman wrote:
> > > > What is the false positive rate with this check over a large codebase 
> > > > that uses exceptions?
> > > Do you know any good project to check it?
> > LibreOffice might be a candidate, see 
> >  for details on 
> > how to generate a compile database for it (since it does not use cmake), 
> > then you can start testing.
> Thanks. However it's not just about using exception. To be meaningful I need 
> a project that use noexcept in more than a few places.
> 
> Here are some projects that I found:
> https://github.com/facebook/hhvm/search?utf8=%E2%9C%93=noexcept
> https://github.com/facebook/folly/search?utf8=%E2%9C%93=noexcept
> https://github.com/Tencent/mars/search?utf8=%E2%9C%93=noexcept
> https://github.com/facebook/rocksdb/search?utf8=%E2%9C%93=noexcept
> https://github.com/CRYTEK-CRYENGINE/CRYENGINE/search?utf8=%E2%9C%93=noexcept
> https://github.com/SFTtech/openage/search?utf8=%E2%9C%93=noexcept
> https://github.com/facebook/watchman/search?utf8=%E2%9C%93=noexcept
> https://github.com/facebook/proxygen/search?utf8=%E2%9C%93=noexcept
> https://github.com/philsquared/Catch/search?utf8=%E2%9C%93=noexcept
> https://github.com/sandstorm-io/capnproto/search?utf8=%E2%9C%93=noexcept
> https://github.com/miloyip/rapidjson/search?utf8=%E2%9C%93=noexcept
> 
> I've tried HHVM so far, and ran into some problems compiling it. Anyway I'm 
> working on it.
Ok, I was able to run it on most of the HHVM  codebase + deps. There were some 
issues that looked like some autogenerated pieces missing, so it may have been 
not 100% covered.

The results:
3 occurences in total
1) rethrow in destructor (http://pastebin.com/JRNMZGev)
2) false positive "throw and catch in the same function" 
(http://pastebin.com/14y1AJgM)
3) noexcept decided during compilation (http://pastebin.com/1jZzRAbC)

That's not too many, but this is a kind of check that I guess would be most 
useful earlier in the development - ideally before the initial code review.

I'm not sure if we should count (3) as false positive. We could potentially 
eliminate it, by checking if the expression in noexcept is empty or true 
literal.

(2) is def. a false positive. The potential handling of this case was already 
proposed, but I'm not sure if it's worth it.  The code in example (2) is ugly 
and extracting these throwing parts to separate functions looks like a good way 
to start refactoring. 

What do you think?



https://reviews.llvm.org/D19201



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


[PATCH] D30884: When diagnosing taking address of packed members skip __unaligned-qualified expressions

2017-03-13 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL297620: When diagnosing taking address of packed members 
skip __unaligned-qualified… (authored by rogfer01).

Changed prior to commit:
  https://reviews.llvm.org/D30884?vs=91531=91548#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30884

Files:
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/Sema/address-unaligned.c


Index: cfe/trunk/test/Sema/address-unaligned.c
===
--- cfe/trunk/test/Sema/address-unaligned.c
+++ cfe/trunk/test/Sema/address-unaligned.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s
+// expected-no-diagnostics
+
+typedef
+struct __attribute__((packed)) S1 {
+  char c0;
+  int x;
+  char c1;
+} S1;
+
+void bar(__unaligned int *);
+
+void foo(__unaligned S1* s1)
+{
+bar(>x);
+}
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -11851,6 +11851,10 @@
   if (!ME)
 return;
 
+  // No need to check expressions with an __unaligned-qualified type.
+  if (E->getType().getQualifiers().hasUnaligned())
+return;
+
   // For a chain of MemberExpr like "a.b.c.d" this list
   // will keep FieldDecl's like [d, c, b].
   SmallVector ReverseMemberChain;


Index: cfe/trunk/test/Sema/address-unaligned.c
===
--- cfe/trunk/test/Sema/address-unaligned.c
+++ cfe/trunk/test/Sema/address-unaligned.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s
+// expected-no-diagnostics
+
+typedef
+struct __attribute__((packed)) S1 {
+  char c0;
+  int x;
+  char c1;
+} S1;
+
+void bar(__unaligned int *);
+
+void foo(__unaligned S1* s1)
+{
+bar(>x);
+}
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -11851,6 +11851,10 @@
   if (!ME)
 return;
 
+  // No need to check expressions with an __unaligned-qualified type.
+  if (E->getType().getQualifiers().hasUnaligned())
+return;
+
   // For a chain of MemberExpr like "a.b.c.d" this list
   // will keep FieldDecl's like [d, c, b].
   SmallVector ReverseMemberChain;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r297620 - When diagnosing taking address of packed members skip __unaligned-qualified expressions

2017-03-13 Thread Roger Ferrer Ibanez via cfe-commits
Author: rogfer01
Date: Mon Mar 13 08:18:21 2017
New Revision: 297620

URL: http://llvm.org/viewvc/llvm-project?rev=297620=rev
Log:
When diagnosing taking address of packed members skip __unaligned-qualified 
expressions

Given that we have already explicitly stated in the qualifier that the
expression is __unaligned, it makes little sense to diagnose that the address
of the packed member may not be aligned.

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


Added:
cfe/trunk/test/Sema/address-unaligned.c
Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=297620=297619=297620=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Mar 13 08:18:21 2017
@@ -11851,6 +11851,10 @@ void Sema::RefersToMemberWithReducedAlig
   if (!ME)
 return;
 
+  // No need to check expressions with an __unaligned-qualified type.
+  if (E->getType().getQualifiers().hasUnaligned())
+return;
+
   // For a chain of MemberExpr like "a.b.c.d" this list
   // will keep FieldDecl's like [d, c, b].
   SmallVector ReverseMemberChain;

Added: cfe/trunk/test/Sema/address-unaligned.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/address-unaligned.c?rev=297620=auto
==
--- cfe/trunk/test/Sema/address-unaligned.c (added)
+++ cfe/trunk/test/Sema/address-unaligned.c Mon Mar 13 08:18:21 2017
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s
+// expected-no-diagnostics
+
+typedef
+struct __attribute__((packed)) S1 {
+  char c0;
+  int x;
+  char c1;
+} S1;
+
+void bar(__unaligned int *);
+
+void foo(__unaligned S1* s1)
+{
+bar(>x);
+}


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


r297619 - [analyzer] Fix a rare crash for valist check.

2017-03-13 Thread Gabor Horvath via cfe-commits
Author: xazax
Date: Mon Mar 13 07:48:26 2017
New Revision: 297619

URL: http://llvm.org/viewvc/llvm-project?rev=297619=rev
Log:
[analyzer] Fix a rare crash for valist check.

It looks like on some host-triples the result of a valist related expr can be
a LazyCompoundVal. Handle that case in the check.

Patch by Abramo Bagnara!

Added:
cfe/trunk/test/Analysis/valist-as-lazycompound.c
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
cfe/trunk/test/Analysis/valist-uninitialized-no-undef.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp?rev=297619=297618=297619=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp Mon Mar 13 07:48:26 
2017
@@ -165,11 +165,8 @@ void ValistChecker::checkPreCall(const C
 const MemRegion *ValistChecker::getVAListAsRegion(SVal SV, const Expr *E,
   bool ,
   CheckerContext ) const {
-  // FIXME: on some platforms CallAndMessage checker finds some instances of
-  // the uninitialized va_list usages. CallAndMessage checker is disabled in
-  // the tests so they can verify platform independently those issues. As a
-  // side effect, this check is required here.
-  if (SV.isUnknownOrUndef())
+  const MemRegion *Reg = SV.getAsRegion();
+  if (!Reg)
 return nullptr;
   // TODO: In the future this should be abstracted away by the analyzer.
   bool VaListModelledAsArray = false;
@@ -178,7 +175,6 @@ const MemRegion *ValistChecker::getVALis
 VaListModelledAsArray =
 Ty->isPointerType() && Ty->getPointeeType()->isRecordType();
   }
-  const MemRegion *Reg = SV.getAsRegion();
   if (const auto *DeclReg = Reg->getAs()) {
 if (isa(DeclReg->getDecl()))
   Reg = C.getState()->getSVal(SV.castAs()).getAsRegion();

Added: cfe/trunk/test/Analysis/valist-as-lazycompound.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/valist-as-lazycompound.c?rev=297619=auto
==
--- cfe/trunk/test/Analysis/valist-as-lazycompound.c (added)
+++ cfe/trunk/test/Analysis/valist-as-lazycompound.c Mon Mar 13 07:48:26 2017
@@ -0,0 +1,21 @@
+// RUN: %clang_analyze_cc1 -triple gcc-linaro-arm-linux-gnueabihf 
-analyzer-checker=core,valist.Uninitialized,valist.CopyToSelf 
-analyzer-output=text -analyzer-store=region -verify %s
+// expected-no-diagnostics
+
+typedef unsigned int size_t;
+typedef __builtin_va_list __gnuc_va_list;
+typedef __gnuc_va_list va_list;
+
+extern int vsprintf(char *__restrict __s,
+const char *__restrict __format, __gnuc_va_list
+ __arg);
+
+void _dprintf(const char *function, int flen, int line, int level,
+ const char *prefix, const char *fmt, ...) {
+  char raw[10];
+  int err;
+  va_list ap;
+
+  __builtin_va_start(ap, fmt);
+  err = vsprintf(raw, fmt, ap);
+  __builtin_va_end(ap);
+}

Modified: cfe/trunk/test/Analysis/valist-uninitialized-no-undef.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/valist-uninitialized-no-undef.c?rev=297619=297618=297619=diff
==
--- cfe/trunk/test/Analysis/valist-uninitialized-no-undef.c (original)
+++ cfe/trunk/test/Analysis/valist-uninitialized-no-undef.c Mon Mar 13 07:48:26 
2017
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze 
-analyzer-checker=core,valist.Uninitialized,valist.CopyToSelf 
-analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu 
-analyzer-checker=core,valist.Uninitialized,valist.CopyToSelf 
-analyzer-output=text -analyzer-store=region -verify %s
 
 #include "Inputs/system-header-simulator-for-valist.h"
 


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


[PATCH] D30831: [ASTImporter] Import fix of GCCAsmStmts w/ missing symbolic operands

2017-03-13 Thread Zoltán Gera via Phabricator via cfe-commits
gerazo marked an inline comment as done.
gerazo added inline comments.



Comment at: lib/AST/ASTImporter.cpp:5221
 IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(I));
-if (!ToII)
-  return nullptr;
+// ToII is nullptr when no symbolic name is given for output operand
+// see ParseStmtAsm::ParseAsmOperandsOpt

a.sidorin wrote:
> In such cases, we should check that FromIdentifier is `nullptr` too (to 
> detect cases where the result is `nullptr` due to import error). The check 
> will be still present but will look like:
> ```
> if (!ToII && S->getOutputIdentifier())
>   return nullptr;
> ```
> The same below.
Thank you Aleksei, done.


https://reviews.llvm.org/D30831



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


[PATCH] D30643: [OpenCL] Extended diagnostics for atomic initialization

2017-03-13 Thread Alexey Bader via Phabricator via cfe-commits
bader added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8263
+def err_atomic_init_addressspace : Error<
+  "initialization of atomic variables is restricted to variables in global 
address space">;
 def err_atomic_init_constant : Error<

bader wrote:
> Anastasia wrote:
> > echuraev wrote:
> > > Anastasia wrote:
> > > > Could we combine this error diag with the one below? I guess they are 
> > > > semantically very similar apart from one is about initialization and 
> > > > another is about assignment?
> > > I'm not sure that it is a good idea to combine these errors. For example, 
> > > if developer had declared a variable non-constant and not in global 
> > > address space he would have got the same message for both errors. And it 
> > > can be difficult to determine what the exact problem is. He can fix one 
> > > of the problems but he will still get the same error.
> > Well, I don't actually see that we check for constant anywhere so it's also 
> > OK if you want to drop this bit. Although I think the original intension of 
> > this message as I understood was to provide the most complete hint.
> > 
> > My concern is that these two errors seem to be reporting nearly the same 
> > issue and ideally we would like to keep diagnostic list as small as 
> > possible. This also makes the file more concise and messages more 
> > consistent.
> I suggest adding a test case with non-constant initialization case to 
> validate that existing checks cover this case for atomic types already.
> If so, we can adjust existing diagnostic message to cover both cases: 
> initialization and assignment expression.
I don't think it's quite true.
There are two requirements here that must be met the same time. Atomic 
variables *declared in the global address space* can be initialized only with 
"compile time constant'.
If understand the spec correctly this code is also valid:

  kernel void foo() {
static global atomic_int a = 42; // although it's not clear if we must use 
ATOMIC_VAR_INIT here.
...
  }



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8267
   "variable in constant address space must be initialized">;
-def err_atomic_init_constant : Error<
-  "atomic variable can only be assigned to a compile time constant"
-  " in the declaration statement in the program scope">;
+// Atomics
+def err_atomic_init: Error<

I suggest removing this comment.
If you are going to add other diagnostic messages specific to OpenCL atomics, 
then separate them from this list of unordered diagnostics similar to pipe 
built-in functions below.


https://reviews.llvm.org/D30643



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


[PATCH] D30831: [ASTImporter] Import fix of GCCAsmStmts w/ missing symbolic operands

2017-03-13 Thread Zoltán Gera via Phabricator via cfe-commits
gerazo updated this revision to Diff 91541.
gerazo added a comment.

Better check not letting a real import problem passing through


https://reviews.llvm.org/D30831

Files:
  lib/AST/ASTImporter.cpp
  test/ASTMerge/asm/Inputs/asm-function.cpp
  test/ASTMerge/asm/test.cpp


Index: test/ASTMerge/asm/test.cpp
===
--- test/ASTMerge/asm/test.cpp
+++ test/ASTMerge/asm/test.cpp
@@ -4,4 +4,5 @@
 
 void testAsmImport() {
   asmFunc(12, 42);
+  asmFunc2(42);
 }
Index: test/ASTMerge/asm/Inputs/asm-function.cpp
===
--- test/ASTMerge/asm/Inputs/asm-function.cpp
+++ test/ASTMerge/asm/Inputs/asm-function.cpp
@@ -9,3 +9,13 @@
   res = bigres;
   return res;
 }
+
+int asmFunc2(int i) {
+  int res;
+  asm ("mov %1, %0 \t\n"
+   "inc %0 "
+  : "=r" (res)
+  : "r" (i)
+  : "cc");
+  return res;
+}
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -5218,13 +5218,17 @@
   SmallVector Names;
   for (unsigned I = 0, E = S->getNumOutputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for output operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getOutputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
   for (unsigned I = 0, E = S->getNumInputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getInputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for input operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getInputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }


Index: test/ASTMerge/asm/test.cpp
===
--- test/ASTMerge/asm/test.cpp
+++ test/ASTMerge/asm/test.cpp
@@ -4,4 +4,5 @@
 
 void testAsmImport() {
   asmFunc(12, 42);
+  asmFunc2(42);
 }
Index: test/ASTMerge/asm/Inputs/asm-function.cpp
===
--- test/ASTMerge/asm/Inputs/asm-function.cpp
+++ test/ASTMerge/asm/Inputs/asm-function.cpp
@@ -9,3 +9,13 @@
   res = bigres;
   return res;
 }
+
+int asmFunc2(int i) {
+  int res;
+  asm ("mov %1, %0 \t\n"
+   "inc %0 "
+  : "=r" (res)
+  : "r" (i)
+  : "cc");
+  return res;
+}
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -5218,13 +5218,17 @@
   SmallVector Names;
   for (unsigned I = 0, E = S->getNumOutputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for output operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getOutputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
   for (unsigned I = 0, E = S->getNumInputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getInputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for input operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getInputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

2017-03-13 Thread Mads Ravn via Phabricator via cfe-commits
madsravn added a comment.

In https://reviews.llvm.org/D30158#698871, @aaron.ballman wrote:

> In https://reviews.llvm.org/D30158#696534, @madsravn wrote:
>
> > Any updates on this?
>
>
> Have you run it over the test suite on the revision before random_shuffle was 
> removed from libc++?


I can't find any more tests for random_shuffle. I have looked in the SVN log 
for from december 2014 until now. It works on the three tests currently in 
trunk.


https://reviews.llvm.org/D30158



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


[PATCH] D30884: When diagnosing taking address of packed members skip __unaligned-qualified expressions

2017-03-13 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits
rogfer01 added a comment.

Thanks for the review @aaron.ballman!


https://reviews.llvm.org/D30884



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


[PATCH] D30884: When diagnosing taking address of packed members skip __unaligned-qualified expressions

2017-03-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


https://reviews.llvm.org/D30884



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


[PATCH] D30643: [OpenCL] Extended diagnostics for atomic initialization

2017-03-13 Thread Egor Churaev via Phabricator via cfe-commits
echuraev updated this revision to Diff 91536.

https://reviews.llvm.org/D30643

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  test/Parser/opencl-atomics-cl20.cl
  test/SemaOpenCL/atomic-init.cl


Index: test/SemaOpenCL/atomic-init.cl
===
--- /dev/null
+++ test/SemaOpenCL/atomic-init.cl
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -fsyntax-only -verify  %s
+
+global atomic_int a1 = 0;
+
+kernel void test_atomic_initialization() {
+  a1 = 1; // expected-error {{atomic variable can only be assigned to a 
compile time constant or to variables in global adress space}}
+  atomic_int a2 = 0; // expected-error {{atomic variable can only be assigned 
to a compile time constant or to variables in global adress space}}
+  private atomic_int a3 = 0; // expected-error {{atomic variable can only be 
assigned to a compile time constant or to variables in global adress space}}
+  local atomic_int a4 = 0; // expected-error {{'__local' variable cannot have 
an initializer}}
+  global atomic_int a5 = 0; // expected-error {{function scope variable cannot 
be declared in global address space}}
+}
Index: test/Parser/opencl-atomics-cl20.cl
===
--- test/Parser/opencl-atomics-cl20.cl
+++ test/Parser/opencl-atomics-cl20.cl
@@ -67,7 +67,7 @@
   foo();
 // OpenCL v2.0 s6.13.11.8, arithemtic operations are not permitted on atomic 
types.
   i++; // expected-error {{invalid argument type 'atomic_int' (aka 
'_Atomic(int)') to unary expression}}
-  i = 1; // expected-error {{atomic variable can only be assigned to a compile 
time constant in the declaration statement in the program scope}}
+  i = 1; // expected-error {{atomic variable can only be assigned to a compile 
time constant or to variables in global adress space}}
   i += 1; // expected-error {{invalid operands to binary expression 
('atomic_int' (aka '_Atomic(int)') and 'int')}}
   i = i + i; // expected-error {{invalid operands to binary expression 
('atomic_int' (aka '_Atomic(int)') and 'atomic_int')}}
 }
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -6489,6 +6489,20 @@
   << Init->getSourceRange();
   }
 
+  // OpenCL v2.0 s6.13.11.1. atomic variables can be initialized in global 
scope
+  QualType ETy = Entity.getType();
+  Qualifiers TyQualifiers = ETy.getQualifiers();
+  bool HasGlobalAS = TyQualifiers.hasAddressSpace() &&
+ TyQualifiers.getAddressSpace() == LangAS::opencl_global;
+
+  if (S.getLangOpts().OpenCLVersion >= 200 &&
+  ETy->isAtomicType() && !HasGlobalAS &&
+  Entity.getKind() == InitializedEntity::EK_Variable && Args.size() > 0) {
+S.Diag(Args[0]->getLocStart(), diag::err_atomic_init) <<
+SourceRange(Entity.getDecl()->getLocStart(), Args[0]->getLocEnd());
+return ExprError();
+  }
+
   // Diagnose cases where we initialize a pointer to an array temporary, and 
the
   // pointer obviously outlives the temporary.
   if (Args.size() == 1 && Args[0]->getType()->isArrayType() &&
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -11091,7 +11091,7 @@
 if (LHSTy->isAtomicType() || RHSTy->isAtomicType()) {
   SourceRange SR(LHSExpr->getLocStart(), RHSExpr->getLocEnd());
   if (BO_Assign == Opc)
-Diag(OpLoc, diag::err_atomic_init_constant) << SR;
+Diag(OpLoc, diag::err_atomic_init) << SR;
   else
 ResultTy = InvalidOperands(OpLoc, LHS, RHS);
   return ExprError();
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8264,9 +8264,9 @@
   "return value cannot be qualified with address space">;
 def err_opencl_constant_no_init : Error<
   "variable in constant address space must be initialized">;
-def err_atomic_init_constant : Error<
-  "atomic variable can only be assigned to a compile time constant"
-  " in the declaration statement in the program scope">;
+// Atomics
+def err_atomic_init: Error<
+  "atomic variable can only be assigned to a compile time constant or to 
variables in global adress space">;
 def err_opencl_implicit_vector_conversion : Error<
   "implicit conversions between vector types (%0 and %1) are not permitted">;
 def err_opencl_invalid_type_array : Error<


Index: test/SemaOpenCL/atomic-init.cl
===
--- /dev/null
+++ test/SemaOpenCL/atomic-init.cl
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -fsyntax-only -verify  %s
+
+global atomic_int a1 = 0;
+
+kernel void test_atomic_initialization() {
+  a1 = 1; // expected-error {{atomic 

[PATCH] D30547: [clang-tidy] Forwarding reference overload in constructors

2017-03-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tidy/misc/MiscTidyModule.cpp:70
+CheckFactories.registerCheck(
+"misc-forwarding-reference-overload");
 CheckFactories.registerCheck("misc-misplaced-const");

xazax.hun wrote:
> malcolm.parsons wrote:
> > aaron.ballman wrote:
> > > leanil wrote:
> > > > aaron.ballman wrote:
> > > > > I wonder if the name might be slightly misleading -- I've always 
> > > > > understood these to be universal references rather than forwarding 
> > > > > references. I don't have the Meyers book in front of me -- what 
> > > > > nomenclature does he use?
> > > > > 
> > > > > Once we pick the name, we should use it consistently in the source 
> > > > > code (like the file name for the check and the check name), the 
> > > > > documentation, and the release notes.
> > > > Meyers calls them universal references, but it's //forwarding 
> > > > reference// in the standard (14.8.2.1).
> > > Hmm, the terms are a bit too new to really get a good idea from google's 
> > > ngram viewer, but the search result counts are:
> > > 
> > > Google:
> > > "universal reference" : 270,000
> > > "forwarding reference" : 9650
> > > 
> > > Stack Overflow:
> > > universal reference : 3016
> > > forwarding reference: 1654
> > > 
> > > So I think that these are probably more well-known as universal 
> > > references, despite the standard's nomenclature being forwarding 
> > > reference.
> > The Q section in https://isocpp.org/files/papers/N4164.pdf explains why 
> > "universal reference" is a bad name.
> I think in a compiler related project at least in the source code we should 
> stick the the standard's nomenclature. In the documentation, however, it is 
> worth to mention that, this very same thing is called universal reference by 
> Scott Meyers. 
Okay, I'm convinced enough that "forward reference" is okay. :-)


Repository:
  rL LLVM

https://reviews.llvm.org/D30547



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


[PATCH] D30183: Add -iframeworkwithsysroot compiler option

2017-03-13 Thread Alex Lorenz via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL297614: Add -iframeworkwithsysroot compiler option (authored 
by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D30183?vs=89152=91534#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30183

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/Frontend/iframework.c


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1524,6 +1524,11 @@
   HelpText<"Add directory to AFTER include search path">;
 def iframework : JoinedOrSeparate<["-"], "iframework">, Group, 
Flags<[CC1Option]>,
   HelpText<"Add directory to SYSTEM framework search path">;
+def iframeworkwithsysroot : JoinedOrSeparate<["-"], "iframeworkwithsysroot">,
+  Group,
+  HelpText<"Add directory to SYSTEM framework search path, "
+   "absolute paths are relative to -isysroot">,
+  MetaVarName<"">, Flags<[CC1Option]>;
 def imacros : JoinedOrSeparate<["-", "--"], "imacros">, Group, 
Flags<[CC1Option]>,
   HelpText<"Include macros from file before parsing">, MetaVarName<"">;
 def image__base : Separate<["-"], "image_base">;
Index: cfe/trunk/test/Frontend/iframework.c
===
--- cfe/trunk/test/Frontend/iframework.c
+++ cfe/trunk/test/Frontend/iframework.c
@@ -1,4 +1,5 @@
 // RUN: %clang -fsyntax-only -iframework %S/Inputs %s -Xclang -verify
+// RUN: %clang -fsyntax-only -isysroot %S -iframeworkwithsysroot /Inputs %s 
-Xclang -verify
 // expected-no-diagnostics
 
 #include 
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -1507,6 +1507,9 @@
  !A->getOption().matches(OPT_iwithsysroot));
   for (const Arg *A : Args.filtered(OPT_iframework))
 Opts.AddPath(A->getValue(), frontend::System, true, true);
+  for (const Arg *A : Args.filtered(OPT_iframeworkwithsysroot))
+Opts.AddPath(A->getValue(), frontend::System, /*IsFramework=*/true,
+ /*IgnoreSysRoot=*/false);
 
   // Add the paths for the various language specific isystem flags.
   for (const Arg *A : Args.filtered(OPT_c_isystem))


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1524,6 +1524,11 @@
   HelpText<"Add directory to AFTER include search path">;
 def iframework : JoinedOrSeparate<["-"], "iframework">, Group, Flags<[CC1Option]>,
   HelpText<"Add directory to SYSTEM framework search path">;
+def iframeworkwithsysroot : JoinedOrSeparate<["-"], "iframeworkwithsysroot">,
+  Group,
+  HelpText<"Add directory to SYSTEM framework search path, "
+   "absolute paths are relative to -isysroot">,
+  MetaVarName<"">, Flags<[CC1Option]>;
 def imacros : JoinedOrSeparate<["-", "--"], "imacros">, Group, Flags<[CC1Option]>,
   HelpText<"Include macros from file before parsing">, MetaVarName<"">;
 def image__base : Separate<["-"], "image_base">;
Index: cfe/trunk/test/Frontend/iframework.c
===
--- cfe/trunk/test/Frontend/iframework.c
+++ cfe/trunk/test/Frontend/iframework.c
@@ -1,4 +1,5 @@
 // RUN: %clang -fsyntax-only -iframework %S/Inputs %s -Xclang -verify
+// RUN: %clang -fsyntax-only -isysroot %S -iframeworkwithsysroot /Inputs %s -Xclang -verify
 // expected-no-diagnostics
 
 #include 
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -1507,6 +1507,9 @@
  !A->getOption().matches(OPT_iwithsysroot));
   for (const Arg *A : Args.filtered(OPT_iframework))
 Opts.AddPath(A->getValue(), frontend::System, true, true);
+  for (const Arg *A : Args.filtered(OPT_iframeworkwithsysroot))
+Opts.AddPath(A->getValue(), frontend::System, /*IsFramework=*/true,
+ /*IgnoreSysRoot=*/false);
 
   // Add the paths for the various language specific isystem flags.
   for (const Arg *A : Args.filtered(OPT_c_isystem))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r297614 - Add -iframeworkwithsysroot compiler option

2017-03-13 Thread Alex Lorenz via cfe-commits
Author: arphaman
Date: Mon Mar 13 06:17:41 2017
New Revision: 297614

URL: http://llvm.org/viewvc/llvm-project?rev=297614=rev
Log:
Add -iframeworkwithsysroot compiler option

This commit adds support for a new -iframeworkwithsysroot compiler option which
allows the user to specify a framework path that can be prefixed with the
sysroot. This option is similar to the -iwithsysroot option that exists to
supplement -isystem.

rdar://21316352

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

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/Frontend/iframework.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=297614=297613=297614=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Mar 13 06:17:41 2017
@@ -1524,6 +1524,11 @@ def idirafter : JoinedOrSeparate<["-"],
   HelpText<"Add directory to AFTER include search path">;
 def iframework : JoinedOrSeparate<["-"], "iframework">, Group, 
Flags<[CC1Option]>,
   HelpText<"Add directory to SYSTEM framework search path">;
+def iframeworkwithsysroot : JoinedOrSeparate<["-"], "iframeworkwithsysroot">,
+  Group,
+  HelpText<"Add directory to SYSTEM framework search path, "
+   "absolute paths are relative to -isysroot">,
+  MetaVarName<"">, Flags<[CC1Option]>;
 def imacros : JoinedOrSeparate<["-", "--"], "imacros">, Group, 
Flags<[CC1Option]>,
   HelpText<"Include macros from file before parsing">, MetaVarName<"">;
 def image__base : Separate<["-"], "image_base">;

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=297614=297613=297614=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Mon Mar 13 06:17:41 2017
@@ -1507,6 +1507,9 @@ static void ParseHeaderSearchArgs(Header
  !A->getOption().matches(OPT_iwithsysroot));
   for (const Arg *A : Args.filtered(OPT_iframework))
 Opts.AddPath(A->getValue(), frontend::System, true, true);
+  for (const Arg *A : Args.filtered(OPT_iframeworkwithsysroot))
+Opts.AddPath(A->getValue(), frontend::System, /*IsFramework=*/true,
+ /*IgnoreSysRoot=*/false);
 
   // Add the paths for the various language specific isystem flags.
   for (const Arg *A : Args.filtered(OPT_c_isystem))

Modified: cfe/trunk/test/Frontend/iframework.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/iframework.c?rev=297614=297613=297614=diff
==
--- cfe/trunk/test/Frontend/iframework.c (original)
+++ cfe/trunk/test/Frontend/iframework.c Mon Mar 13 06:17:41 2017
@@ -1,4 +1,5 @@
 // RUN: %clang -fsyntax-only -iframework %S/Inputs %s -Xclang -verify
+// RUN: %clang -fsyntax-only -isysroot %S -iframeworkwithsysroot /Inputs %s 
-Xclang -verify
 // expected-no-diagnostics
 
 #include 


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


  1   2   >