r279722 - Fix memory leaks in clang-offload-bundler

2016-08-25 Thread Vitaly Buka via cfe-commits
Author: vitalybuka
Date: Thu Aug 25 02:21:34 2016
New Revision: 279722

URL: http://llvm.org/viewvc/llvm-project?rev=279722&view=rev
Log:
Fix memory leaks in clang-offload-bundler

Summary:
1. Pair removed from StringMap was not destroyed
2. ObjectFile had no owner

Reviewers: sfantao

Subscribers: llvm-commits

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

Modified:
cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Modified: cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp?rev=279722&r1=279721&r2=279722&view=diff
==
--- cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp (original)
+++ cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp Thu Aug 25 
02:21:34 2016
@@ -348,10 +348,10 @@ public:
 class ObjectFileHandler final : public FileHandler {
 
   /// The object file we are currently dealing with.
-  ObjectFile &Obj;
+  std::unique_ptr Obj;
 
   /// Return the input file contents.
-  StringRef getInputFileContents() const { return Obj.getData(); }
+  StringRef getInputFileContents() const { return Obj->getData(); }
 
   /// Return true if the provided section is an offload section and return the
   /// triple by reference.
@@ -400,7 +400,7 @@ public:
   void ReadHeader(MemoryBuffer &Input) {}
   StringRef ReadBundleStart(MemoryBuffer &Input) {
 
-while (NextSection != Obj.section_end()) {
+while (NextSection != Obj->section_end()) {
   CurrentSection = NextSection;
   ++NextSection;
 
@@ -561,9 +561,10 @@ public:
 GV->setSection(SectionName);
   }
 
-  ObjectFileHandler(ObjectFile &Obj)
-  : FileHandler(), Obj(Obj), CurrentSection(Obj.section_begin()),
-NextSection(Obj.section_begin()) {}
+  ObjectFileHandler(std::unique_ptr ObjIn)
+  : FileHandler(), Obj(std::move(ObjIn)),
+CurrentSection(Obj->section_begin()),
+NextSection(Obj->section_begin()) {}
   ~ObjectFileHandler() {}
 };
 
@@ -677,12 +678,13 @@ static FileHandler *CreateObjectFileHand
   // We only support regular object files. If this is not an object file,
   // default to the binary handler. The handler will be owned by the client of
   // this function.
-  ObjectFile *Obj = dyn_cast(BinaryOrErr.get().release());
+  std::unique_ptr Obj(
+  dyn_cast(BinaryOrErr.get().release()));
 
   if (!Obj)
 return new BinaryFileHandler();
 
-  return new ObjectFileHandler(*Obj);
+  return new ObjectFileHandler(std::move(Obj));
 }
 
 /// Return an appropriate handler given the input files and options.
@@ -821,7 +823,7 @@ static bool UnbundleFiles() {
 }
 FH.get()->ReadBundle(OutputFile, Input);
 FH.get()->ReadBundleEnd(Input);
-Worklist.remove(&*Output);
+Worklist.erase(Output);
 
 // Record if we found the host bundle.
 if (hasHostKind(CurTriple))


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


[libclc] r279723 - amdgcn: Fix return type of get_num_groups

2016-08-25 Thread Matt Arsenault via cfe-commits
Author: arsenm
Date: Thu Aug 25 02:31:40 2016
New Revision: 279723

URL: http://llvm.org/viewvc/llvm-project?rev=279723&view=rev
Log:
amdgcn: Fix return type of get_num_groups

Added:
libclc/trunk/amdgcn/lib/workitem/get_num_groups.ll
libclc/trunk/r600/lib/workitem/get_num_groups.ll
  - copied, changed from r279692, 
libclc/trunk/amdgpu/lib/workitem/get_num_groups.ll
Removed:
libclc/trunk/amdgpu/lib/workitem/get_num_groups.ll
Modified:
libclc/trunk/amdgcn/lib/SOURCES
libclc/trunk/amdgpu/lib/SOURCES
libclc/trunk/r600/lib/SOURCES

Modified: libclc/trunk/amdgcn/lib/SOURCES
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgcn/lib/SOURCES?rev=279723&r1=279722&r2=279723&view=diff
==
--- libclc/trunk/amdgcn/lib/SOURCES (original)
+++ libclc/trunk/amdgcn/lib/SOURCES Thu Aug 25 02:31:40 2016
@@ -5,4 +5,5 @@ workitem/get_group_id.cl
 workitem/get_global_size.ll
 workitem/get_local_id.cl
 workitem/get_local_size.ll
+workitem/get_num_groups.ll
 workitem/get_work_dim.cl

Added: libclc/trunk/amdgcn/lib/workitem/get_num_groups.ll
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgcn/lib/workitem/get_num_groups.ll?rev=279723&view=auto
==
--- libclc/trunk/amdgcn/lib/workitem/get_num_groups.ll (added)
+++ libclc/trunk/amdgcn/lib/workitem/get_num_groups.ll Thu Aug 25 02:31:40 2016
@@ -0,0 +1,21 @@
+declare i32 @llvm.r600.read.ngroups.x() nounwind readnone
+declare i32 @llvm.r600.read.ngroups.y() nounwind readnone
+declare i32 @llvm.r600.read.ngroups.z() nounwind readnone
+
+define i64 @get_num_groups(i32 %dim) nounwind readnone alwaysinline {
+  switch i32 %dim, label %default [i32 0, label %x_dim i32 1, label %y_dim i32 
2, label %z_dim]
+x_dim:
+  %x = call i32 @llvm.r600.read.ngroups.x()
+  %x.ext = zext i32 %x to i64
+  ret i64 %x.ext
+y_dim:
+  %y = call i32 @llvm.r600.read.ngroups.y()
+  %y.ext = zext i32 %y to i64
+  ret i64 %y.ext
+z_dim:
+  %z = call i32 @llvm.r600.read.ngroups.z()
+  %z.ext = zext i32 %z to i64
+  ret i64 %z.ext
+default:
+  ret i64 1
+}

Modified: libclc/trunk/amdgpu/lib/SOURCES
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/SOURCES?rev=279723&r1=279722&r2=279723&view=diff
==
--- libclc/trunk/amdgpu/lib/SOURCES (original)
+++ libclc/trunk/amdgpu/lib/SOURCES Thu Aug 25 02:31:40 2016
@@ -16,4 +16,3 @@ image/write_imagef.cl
 image/write_imagei.cl
 image/write_imageui.cl
 image/write_image_impl.ll
-workitem/get_num_groups.ll

Removed: libclc/trunk/amdgpu/lib/workitem/get_num_groups.ll
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/workitem/get_num_groups.ll?rev=279722&view=auto
==
--- libclc/trunk/amdgpu/lib/workitem/get_num_groups.ll (original)
+++ libclc/trunk/amdgpu/lib/workitem/get_num_groups.ll (removed)
@@ -1,18 +0,0 @@
-declare i32 @llvm.r600.read.ngroups.x() nounwind readnone
-declare i32 @llvm.r600.read.ngroups.y() nounwind readnone
-declare i32 @llvm.r600.read.ngroups.z() nounwind readnone
-
-define i32 @get_num_groups(i32 %dim) nounwind readnone alwaysinline {
-  switch i32 %dim, label %default [i32 0, label %x_dim i32 1, label %y_dim i32 
2, label %z_dim]
-x_dim:
-  %x = call i32 @llvm.r600.read.ngroups.x() nounwind readnone
-  ret i32 %x
-y_dim:
-  %y = call i32 @llvm.r600.read.ngroups.y() nounwind readnone
-  ret i32 %y
-z_dim:
-  %z = call i32 @llvm.r600.read.ngroups.z() nounwind readnone
-  ret i32 %z
-default:
-  ret i32 0
-}

Modified: libclc/trunk/r600/lib/SOURCES
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/r600/lib/SOURCES?rev=279723&r1=279722&r2=279723&view=diff
==
--- libclc/trunk/r600/lib/SOURCES (original)
+++ libclc/trunk/r600/lib/SOURCES Thu Aug 25 02:31:40 2016
@@ -4,4 +4,5 @@ workitem/get_group_id.cl
 workitem/get_global_size.ll
 workitem/get_local_id.cl
 workitem/get_local_size.ll
+workitem/get_num_groups.ll
 workitem/get_work_dim.cl

Copied: libclc/trunk/r600/lib/workitem/get_num_groups.ll (from r279692, 
libclc/trunk/amdgpu/lib/workitem/get_num_groups.ll)
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/r600/lib/workitem/get_num_groups.ll?p2=libclc/trunk/r600/lib/workitem/get_num_groups.ll&p1=libclc/trunk/amdgpu/lib/workitem/get_num_groups.ll&r1=279692&r2=279723&rev=279723&view=diff
==
--- libclc/trunk/amdgpu/lib/workitem/get_num_groups.ll (original)
+++ libclc/trunk/r600/lib/workitem/get_num_groups.ll Thu Aug 25 02:31:40 2016
@@ -14,5 +14,5 @@ z_dim:
   %z = call i32 @llvm.r600.read.ngroups.z() nounwind readnone
   ret i32 %z
 default:
-  ret i32 0
+  ret i32 1
 }


___
cfe-commits

Re: r279653 - Fix offload bundler tests so that diagnostic can start with caps.

2016-08-25 Thread Vitaly Buka via cfe-commits
It still fails on Windows on another line, \0A vs \0D\0A

C:\buildbot\slave-config\clang-x86-win2008-selfhost\llvm\tools\clang\test\Driver\clang-offload-bundler.c:233:16:
error: expected string not found in input

// CK-OBJ-CMD: private constant [25 x i8] c"Content of device file
1\0A", section "__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu"

   ^

:9:1: note: scanning from here

@1 = private constant [26 x i8] c"Content of device file 1\0D\0A",
section "__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu"

^

:9:6: note: possible intended match here

@1 = private constant [26 x i8] c"Content of device file 1\0D\0A",
section "__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu"




http://lab.llvm.org:8011/builders/clang-x86-win2008-selfhost/builds/9871/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aclang-offload-bundler
.c

On Wed, Aug 24, 2016 at 12:00 PM Samuel Antao via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: sfantao
> Date: Wed Aug 24 13:52:18 2016
> New Revision: 279653
>
> URL: http://llvm.org/viewvc/llvm-project?rev=279653&view=rev
> Log:
> Fix offload bundler tests so that diagnostic can start with caps.
>
> Windows require that.
>
> Modified:
> cfe/trunk/test/Driver/clang-offload-bundler.c
>
> Modified: cfe/trunk/test/Driver/clang-offload-bundler.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang-offload-bundler.c?rev=279653&r1=279652&r2=279653&view=diff
>
> ==
> --- cfe/trunk/test/Driver/clang-offload-bundler.c (original)
> +++ cfe/trunk/test/Driver/clang-offload-bundler.c Wed Aug 24 13:52:18 2016
> @@ -68,7 +68,7 @@
>
>  // RUN: not clang-offload-bundler -type=i
> -targets=host-powerpc64le-ibm-linux-gnu,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
> -inputs=%t.i,%t.tgt1,%t.tgt2.notexist -outputs=%t.bundle.i 2>&1 | FileCheck
> %s --check-prefix CK-ERR5
>  // RUN: not clang-offload-bundler -type=i
> -targets=host-powerpc64le-ibm-linux-gnu,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
> -outputs=%t.i,%t.tgt1,%t.tgt2 -inputs=%t.bundle.i.notexist -unbundle 2>&1 |
> FileCheck %s --check-prefix CK-ERR5
> -// CK-ERR5: error: Can't open file {{.+}}.notexist: No such file or
> directory
> +// CK-ERR5: error: Can't open file {{.+}}.notexist: {{N|n}}o such file or
> directory
>
>  // RUN: not clang-offload-bundler -type=invalid
> -targets=host-powerpc64le-ibm-linux-gnu,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
> -inputs=%t.i,%t.tgt1,%t.tgt2 -outputs=%t.bundle.i 2>&1 | FileCheck %s
> --check-prefix CK-ERR6
>  // CK-ERR6: error: invalid file type specified.
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D22431: clang-format: [JS] nested and tagged template strings.

2016-08-25 Thread Martin Probst via cfe-commits
mprobst added a comment.

No worries, thanks for the review.


https://reviews.llvm.org/D22431



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


Re: [PATCH] D23353: [clang-tidy] Add check 'misc-use-after-move'

2016-08-25 Thread Martin Böhme via cfe-commits
mboehme updated this revision to Diff 69206.
mboehme added a comment.

Responses to reviewer comments.


https://reviews.llvm.org/D23353

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/UseAfterMoveCheck.cpp
  clang-tidy/misc/UseAfterMoveCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-use-after-move.rst
  test/clang-tidy/misc-use-after-move.cpp

Index: test/clang-tidy/misc-use-after-move.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-use-after-move.cpp
@@ -0,0 +1,1039 @@
+// RUN: %check_clang_tidy %s misc-use-after-move %t
+
+typedef decltype(nullptr) nullptr_t;
+
+namespace std {
+typedef unsigned size_t;
+
+template 
+struct unique_ptr {
+  unique_ptr();
+  T *get() const;
+};
+
+template 
+struct shared_ptr {
+  shared_ptr();
+  T *get() const;
+};
+
+#define DECLARE_STANDARD_CONTAINER(name) \
+  template   \
+  struct name {  \
+name();  \
+void clear();\
+bool empty();\
+  }
+
+#define DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(name) \
+  template   \
+  struct name {  \
+name();  \
+void clear();\
+bool empty();\
+void assign(size_t, const T &);  \
+  }
+
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(basic_string);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(vector);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(deque);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(forward_list);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(list);
+DECLARE_STANDARD_CONTAINER(set);
+DECLARE_STANDARD_CONTAINER(map);
+DECLARE_STANDARD_CONTAINER(multiset);
+DECLARE_STANDARD_CONTAINER(multimap);
+DECLARE_STANDARD_CONTAINER(unordered_set);
+DECLARE_STANDARD_CONTAINER(unordered_map);
+DECLARE_STANDARD_CONTAINER(unordered_multiset);
+DECLARE_STANDARD_CONTAINER(unordered_multimap);
+
+typedef basic_string string;
+
+template 
+struct remove_reference;
+
+template 
+struct remove_reference {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &> {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &&> {
+  typedef _Tp type;
+};
+
+template 
+constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) noexcept {
+  return static_cast::type &&>(__t);
+}
+
+} // namespace std
+
+class A {
+public:
+  A();
+  A(const A &);
+  A(A &&);
+
+  A &operator=(const A &);
+  A &operator=(A &&);
+
+  void foo() const;
+  int getInt() const;
+
+  operator bool() const;
+
+  int i;
+};
+
+
+// General tests.
+
+// Simple case.
+void simple() {
+  A a;
+  a.foo();
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:15: note: move occurred here
+}
+
+// A warning should only be emitted for one use-after-move.
+void onlyFlagOneUseAfterMove() {
+  A a;
+  a.foo();
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:15: note: move occurred here
+  a.foo();
+}
+
+void moveAfterMove() {
+  // Move-after-move also counts as a use.
+  {
+A a;
+std::move(a);
+std::move(a);
+// CHECK-MESSAGES: [[@LINE-1]]:15: warning: 'a' used after it was moved
+// CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+  }
+  // This is also true if the move itself turns into the use on the second loop
+  // iteration.
+  {
+A a;
+for (int i = 0; i < 10; ++i) {
+  std::move(a);
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-2]]:7: note: move occurred here
+}
+  }
+}
+
+// Checks also works on function parameters that have a use-after move.
+void parameters(A a) {
+  std::move(a);
+  a.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:3: note: move occurred here
+}
+
+void uniquePtrAndSharedPtr() {
+  // Use-after-moves on std::unique_ptr<> or std::shared_ptr<> aren't flagged.
+  {
+std::unique_ptr ptr;
+std::move(ptr);
+ptr.get();
+  }
+  {
+std::shared_ptr ptr;
+std::move(ptr);
+ptr.get();
+  }
+  // This is also true if the std::unique_ptr<> or std::shared_ptr<> is wrapped
+  // in a typedef.
+  {
+typedef std::unique_ptr PtrToA;
+PtrToA ptr;
+std::move(ptr);
+ptr.get();
+  }
+  {
+typedef std::shared_ptr PtrToA;
+PtrToA ptr;
+std::move(ptr);
+ptr.get();
+  }
+  // And it's also true if the template argument is a little more involved.
+  {
+struct B {
+  typedef A AnotherNa

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

2016-08-25 Thread Andi via cfe-commits
Abpostelnicu updated this revision to Diff 69205.

https://reviews.llvm.org/D22910

Files:
  include/clang/AST/ExprCXX.h
  lib/AST/Expr.cpp

Index: lib/AST/Expr.cpp
===
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -2861,8 +2861,19 @@
 // These never have a side-effect.
 return false;
 
+  case CXXOperatorCallExprClass: {
+// When looking for potential side-effects, we assume that an overloaded
+// assignment operator is intended to have a side-effect and other 
overloaded
+// operators are not.
+OverloadedOperatorKind Op = cast(this)->getOperator();
+if (CXXOperatorCallExpr::isAssignmentOp(Op)) {
+  const Decl *FD = cast(this)->getCalleeDecl();
+  bool IsPure = FD && (FD->hasAttr() || 
FD->hasAttr());
+  if (!IsPure)
+return true;
+}
+  }
   case CallExprClass:
-  case CXXOperatorCallExprClass:
   case CXXMemberCallExprClass:
   case CUDAKernelCallExprClass:
   case UserDefinedLiteralClass: {
Index: include/clang/AST/ExprCXX.h
===
--- include/clang/AST/ExprCXX.h
+++ include/clang/AST/ExprCXX.h
@@ -106,6 +106,16 @@
   // operations on floating point types.
   bool isFPContractable() const { return FPContractable; }
 
+  // Check to see if a given overloaded operator is of assignment kind
+  static bool isAssignmentOp(OverloadedOperatorKind Opc) {
+return Opc == OO_Equal || Opc == OO_StarEqual ||
+   Opc == OO_SlashEqual || Opc == OO_PercentEqual ||
+   Opc == OO_PlusEqual || Opc == OO_MinusEqual ||
+   Opc == OO_LessLessEqual || Opc == OO_GreaterGreaterEqual ||
+   Opc == OO_AmpEqual || Opc == OO_CaretEqual ||
+   Opc == OO_PipeEqual;
+  }
+  
   friend class ASTStmtReader;
   friend class ASTStmtWriter;
 };


Index: lib/AST/Expr.cpp
===
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -2861,8 +2861,19 @@
 // These never have a side-effect.
 return false;
 
+  case CXXOperatorCallExprClass: {
+// When looking for potential side-effects, we assume that an overloaded
+// assignment operator is intended to have a side-effect and other overloaded
+// operators are not.
+OverloadedOperatorKind Op = cast(this)->getOperator();
+if (CXXOperatorCallExpr::isAssignmentOp(Op)) {
+  const Decl *FD = cast(this)->getCalleeDecl();
+  bool IsPure = FD && (FD->hasAttr() || FD->hasAttr());
+  if (!IsPure)
+return true;
+}
+  }
   case CallExprClass:
-  case CXXOperatorCallExprClass:
   case CXXMemberCallExprClass:
   case CUDAKernelCallExprClass:
   case UserDefinedLiteralClass: {
Index: include/clang/AST/ExprCXX.h
===
--- include/clang/AST/ExprCXX.h
+++ include/clang/AST/ExprCXX.h
@@ -106,6 +106,16 @@
   // operations on floating point types.
   bool isFPContractable() const { return FPContractable; }
 
+  // Check to see if a given overloaded operator is of assignment kind
+  static bool isAssignmentOp(OverloadedOperatorKind Opc) {
+return Opc == OO_Equal || Opc == OO_StarEqual ||
+   Opc == OO_SlashEqual || Opc == OO_PercentEqual ||
+   Opc == OO_PlusEqual || Opc == OO_MinusEqual ||
+   Opc == OO_LessLessEqual || Opc == OO_GreaterGreaterEqual ||
+   Opc == OO_AmpEqual || Opc == OO_CaretEqual ||
+   Opc == OO_PipeEqual;
+  }
+  
   friend class ASTStmtReader;
   friend class ASTStmtWriter;
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

2016-08-25 Thread Andi via cfe-commits
Abpostelnicu marked 4 inline comments as done.
Abpostelnicu added a comment.

I've added also support for pure in case 
CXXOperatorCallExprClass::isAssignmentOperator is true.


https://reviews.llvm.org/D22910



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


Re: [PATCH] D23353: [clang-tidy] Add check 'misc-use-after-move'

2016-08-25 Thread Martin Böhme via cfe-commits
mboehme marked 9 inline comments as done.
mboehme added a comment.

> > > 2. Also it would be good to make link in cppcoreguidelines.

> 

> > 

> 

> > 

> 

> > How exactly would I create such a "link"? Are you just thinking of a link 
> > in the documentation, or is there a way to have one clang-tidy check 
> > activate another (and is this what you're thinking of)?

> 

> 

> I am not sure if there is any other mechanism than just links in 
> documentation.


I've taken a look, but I'm not sure where I would put this link. I could add 
another file that starts with "cppcoreguidelines-", but that would make it look 
as if an actual check with that name exists, and I'm not sure that's what we 
want. Do you have a suggestion?

> In the perfect word it would be nice to invoke this check using 
> cppcoreguidelines-use-after-move also with some special options like 
> Pedantic=1 (That would warn about any use after move, even after 
> reinitialization - this is what cppcoreguidelines says)


Do you have a pointer to something in CppCoreGuidelines that says this 
explicitly?

The closest I've been able to find is in F.18: "Flag access to moved-from 
objects." It's not entirely clear here whether "access" is meant to include 
reinitialization.

However, other parts of CppCoreGuidelines seem to imply that it _is_ OK to 
reinitialize an object:

From ES.56:
"Usually, a std::move() is used as an argument to a && parameter. And after you 
do that, assume the object has been moved from (see C.64) and don't read its 
state again until you first set it to a new value."

And from C.64:
"Unless there is an exceptionally strong reason not to, make x = std::move(y); 
y = z; work with the conventional semantics."



Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:29
@@ +28,3 @@
+/// Provides information about the evaluation order of (sub-)expressions within
+/// a CFGBlock.
+///

alexfh wrote:
> Please enclose inline code snippets in double backquotes (doxygen supports 
> markdown to a certain degree).
Shouldn't those be single backquotes? (See 
https://www.stack.nl/~dimitri/doxygen/manual/markdown.html)

Done with single backquotes.


Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:46
@@ +45,3 @@
+///   API),
+/// - Removing the dependency of SequenceChecker on Sema, and
+/// - (Probably) modifying SequenceChecker to make it suitable to be used in

alexfh wrote:
> Does it look feasible?
At least it's not something that I feel I would be able to do without breaking 
things -- I'm not familiar enough with SequenceChecker for that.


Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:135
@@ +134,3 @@
+class UseAfterMoveFinder {
+public:
+  UseAfterMoveFinder(ASTContext *TheContext);

alexfh wrote:
> It's definitely subjective, but I don't think it's scary. And the size of the 
> file doesn't seem to be an issue yet. IMO, a reason to move this to a header 
> would appear once this class is needed elsewhere. 
Leaving it here for now.


Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:217
@@ +216,3 @@
+  for (const auto &SyntheticStmt : TheCFG->synthetic_stmts()) {
+SyntheticStmtSourceMap[SyntheticStmt.first] = SyntheticStmt.second;
+  }

I've added CFG::synthetic_stmts() in D23842.


Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:436
@@ +435,3 @@
+return true;
+}
+  }

Added CFGBlock::succs() in D23842. This looks much nicer now!


Comment at: test/clang-tidy/misc-use-after-move.cpp:505-506
@@ +504,4 @@
+void noreturnDestructor() {
+  A a;
+  // The while loop in the ASSERT() would ordinarily have the potential to 
cause
+  // a use-after-move because the second iteration of the loop would be using a

Noted for a future addition. I'd like to get this patch in though without 
further expanding the functionality (it's already pretty big...).


https://reviews.llvm.org/D23353



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


Re: [PATCH] D23842: [CFG] Add iterator_ranges to CFG and CFGBlock.

2016-08-25 Thread Martin Böhme via cfe-commits
mboehme marked an inline comment as done.


Comment at: include/clang/Analysis/CFG.h:526
@@ -524,1 +525,3 @@
   typedef AdjacentBlocks::const_reverse_iterator  const_pred_reverse_iterator;
+  typedef llvm::iterator_range  pred_range;
+  typedef llvm::iterator_range  pred_const_range;

alexfh wrote:
> Most of the time the new functions will be used in a range-based for loop. I 
> don't think the type returned by the functions will need to be explicitly 
> specified anywhere. I'd remove the typedefs.
I agree the typedefs will probably never be used outside this class, but 
typedefs for iterator_ranges seem to be a common pattern in the Clang codebase. 
(EnumDecl::enumerator_range and CallExpr::arg_range are two random examples.) I 
assume the intent is to make the definition of the functions that return these 
ranges less verbose.

Are you OK with me submitting this patch as-is (with the typedefs)?


https://reviews.llvm.org/D23842



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


Re: [PATCH] D23651: [clang-rename] improve performance for rename-all

2016-08-25 Thread Kirill Bobyrev via cfe-commits
omtcyfz updated this revision to Diff 69214.
omtcyfz marked 2 inline comments as done.
omtcyfz added a comment.

Address couple of comments.


https://reviews.llvm.org/D23651

Files:
  clang-rename/USRFindingAction.cpp
  clang-rename/USRFindingAction.h
  clang-rename/tool/ClangRename.cpp

Index: clang-rename/tool/ClangRename.cpp
===
--- clang-rename/tool/ClangRename.cpp
+++ clang-rename/tool/ClangRename.cpp
@@ -56,24 +56,17 @@
 static int renameAllMain(int argc, const char *argv[]);
 static int helpMain(int argc, const char *argv[]);
 
-/// \brief An oldname -> newname rename.
-struct RenameAllInfo {
-  std::string OldName;
-  unsigned Offset = 0;
-  std::string NewName;
-};
-
-LLVM_YAML_IS_SEQUENCE_VECTOR(RenameAllInfo)
+LLVM_YAML_IS_SEQUENCE_VECTOR(RenamePairInfo)
 
 namespace llvm {
 namespace yaml {
 
-/// \brief Specialized MappingTraits to describe how a RenameAllInfo is
+/// \brief Specialized MappingTraits to describe how a RenamePairInfo is
 /// (de)serialized.
-template <> struct MappingTraits {
-  static void mapping(IO &IO, RenameAllInfo &Info) {
-IO.mapOptional("OldName", Info.OldName);
+template <> struct MappingTraits {
+  static void mapping(IO &IO, RenamePairInfo &Info) {
 IO.mapOptional("Offset", Info.Offset);
+IO.mapOptional("OldName", Info.OldName);
 IO.mapRequired("NewName", Info.NewName);
   }
 };
@@ -155,7 +148,7 @@
   return 1;
 }
 
-std::vector Infos;
+std::vector Infos;
 llvm::yaml::Input YAML(Buffer.get()->getBuffer());
 YAML >> Infos;
 for (const auto &Info : Infos) {
@@ -175,11 +168,11 @@
   }
 
   // Check if NewNames is a valid identifier in C++17.
+  LangOptions Options;
+  Options.CPlusPlus = true;
+  Options.CPlusPlus1z = true;
+  IdentifierTable Table(Options);
   for (const auto &NewName : NewNames) {
-LangOptions Options;
-Options.CPlusPlus = true;
-Options.CPlusPlus1z = true;
-IdentifierTable Table(Options);
 auto NewNameTokKind = Table.get(NewName).getTokenID();
 if (!tok::isAnyIdentifier(NewNameTokKind)) {
   errs() << "ERROR: new name is not a valid identifier in C++17.\n\n";
@@ -203,39 +196,31 @@
 exit(1);
   }
 
-  std::vector> USRList;
-  std::vector PrevNames;
   auto Files = OP.getSourcePathList();
   tooling::RefactoringTool Tool(OP.getCompilations(), Files);
+
   unsigned Count = OldNames.size() ? OldNames.size() : SymbolOffsets.size();
+  std::vector Pairs(Count);
   for (unsigned I = 0; I < Count; ++I) {
-unsigned SymbolOffset = SymbolOffsets.empty() ? 0 : SymbolOffsets[I];
-const std::string &OldName = OldNames.empty() ? std::string() : OldNames[I];
-
-// Get the USRs.
-rename::USRFindingAction USRAction(SymbolOffset, OldName);
-
-// Find the USRs.
-Tool.run(tooling::newFrontendActionFactory(&USRAction).get());
-const auto &USRs = USRAction.getUSRs();
-USRList.push_back(USRs);
-const auto &PrevName = USRAction.getUSRSpelling();
-PrevNames.push_back(PrevName);
-
-if (PrevName.empty()) {
-  // An error should have already been printed.
-  exit(1);
-}
+Pairs[I].Offset = SymbolOffsets.empty() ? 0 : SymbolOffsets[I];
+Pairs[I].OldName = OldNames.empty() ? "" : OldNames[I];
+  }
 
-if (PrintName) {
-  errs() << "clang-rename: found name: " << PrevName << '\n';
+  rename::USRFindingAction USRAction(Pairs);
+  Tool.run(tooling::newFrontendActionFactory(&USRAction).get());
+  const std::vector> &USRList = USRAction.getUSRList();
+  const std::vector &PrevNames = USRAction.getUSRSpellings();
+  if (PrintName) {
+for (const auto &PrevName : PrevNames) {
+  outs() << "clang-rename found name: " << PrevName << '\n';
 }
   }
 
   // Perform the renaming.
   rename::RenamingAction RenameAction(NewNames, PrevNames, USRList,
   Tool.getReplacements(), PrintLocations);
-  auto Factory = tooling::newFrontendActionFactory(&RenameAction);
+  std::unique_ptr Factory =
+  tooling::newFrontendActionFactory(&RenameAction);
   int ExitCode;
 
   if (Inplace) {
Index: clang-rename/USRFindingAction.h
===
--- clang-rename/USRFindingAction.h
+++ clang-rename/USRFindingAction.h
@@ -17,28 +17,31 @@
 
 #include "clang/Frontend/FrontendAction.h"
 
+/// \brief An oldname -> newname rename.
+struct RenamePairInfo {
+  unsigned Offset = 0;
+  std::string OldName;
+  std::string NewName;
+};
+
 namespace clang {
 class ASTConsumer;
 class CompilerInstance;
 class NamedDecl;
 
 namespace rename {
 
 struct USRFindingAction {
-  USRFindingAction(unsigned Offset, const std::string &Name)
-  : SymbolOffset(Offset), OldName(Name) {}
+  USRFindingAction(ArrayRef Pairs) : Pairs(Pairs) {}
   std::unique_ptr newASTConsumer();
 
-  // \brief get the spelling of the USR(s) as it would appear in source files.
-  const std::string &getUSRSpelling() { return SpellingName; }
-
-  co

Re: [PATCH] D23651: [clang-rename] improve performance for rename-all

2016-08-25 Thread Kirill Bobyrev via cfe-commits
omtcyfz added a comment.

Actually, it might make sense to merge `rename-at` and `rename-all` after all. 
Passing a list of `old-name`/`offset` -> `new-name` is just easier.

But that's for another patch.


https://reviews.llvm.org/D23651



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


Re: [PATCH] D23860: [Sema][Comments] Add support for TypeAliasTemplate

2016-08-25 Thread Dmitri Gribenko via cfe-commits
gribozavr accepted this revision.
gribozavr added a comment.
This revision is now accepted and ready to land.

LGTM with two more tests.



Comment at: test/Sema/warn-documentation.cpp:433
@@ +432,3 @@
+template
+using test_function_like_usingA = foo::function_wrapper;
+

For consistency with the rest of the file, could you use '11' instead of 'A'?

Could you also add these two tests:

```
/// 
template
using test_function_like_using10 = int (*)(T aaa, int ccc);

/// 
template
using test_function_like_using12 = foo::function_wrapper 
*;
```


https://reviews.llvm.org/D23860



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


r279727 - clang-format: [JS] nested and tagged template strings.

2016-08-25 Thread Martin Probst via cfe-commits
Author: mprobst
Date: Thu Aug 25 05:13:21 2016
New Revision: 279727

URL: http://llvm.org/viewvc/llvm-project?rev=279727&view=rev
Log:
clang-format: [JS] nested and tagged template strings.

JavaScript template strings can be nested arbitrarily:

foo = `text ${es.map(e => { return `<${e}>`; })} text`;

This change lexes nested template strings using a stack of lexer states to
correctly switch back to template string lexing on closing braces.

Also, reuse the same stack for the token-stashed logic.

Reviewers: djasper

Subscribers: cfe-commits, klimek

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

Modified:
cfe/trunk/lib/Format/FormatTokenLexer.cpp
cfe/trunk/lib/Format/FormatTokenLexer.h
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/FormatTokenLexer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatTokenLexer.cpp?rev=279727&r1=279726&r2=279727&view=diff
==
--- cfe/trunk/lib/Format/FormatTokenLexer.cpp (original)
+++ cfe/trunk/lib/Format/FormatTokenLexer.cpp Thu Aug 25 05:13:21 2016
@@ -26,12 +26,11 @@ namespace format {
 FormatTokenLexer::FormatTokenLexer(const SourceManager &SourceMgr, FileID ID,
const FormatStyle &Style,
encoding::Encoding Encoding)
-: FormatTok(nullptr), IsFirstToken(true), GreaterStashed(false),
-  LessStashed(false), Column(0), TrailingWhitespace(0),
-  SourceMgr(SourceMgr), ID(ID), Style(Style),
-  IdentTable(getFormattingLangOpts(Style)), Keywords(IdentTable),
-  Encoding(Encoding), FirstInLineIndex(0), FormattingDisabled(false),
-  MacroBlockBeginRegex(Style.MacroBlockBegin),
+: FormatTok(nullptr), IsFirstToken(true), StateStack({LexerState::NORMAL}),
+  Column(0), TrailingWhitespace(0), SourceMgr(SourceMgr), ID(ID),
+  Style(Style), IdentTable(getFormattingLangOpts(Style)),
+  Keywords(IdentTable), Encoding(Encoding), FirstInLineIndex(0),
+  FormattingDisabled(false), MacroBlockBeginRegex(Style.MacroBlockBegin),
   MacroBlockEndRegex(Style.MacroBlockEnd) {
   Lex.reset(new Lexer(ID, SourceMgr.getBuffer(ID), SourceMgr,
   getFormattingLangOpts(Style)));
@@ -49,7 +48,7 @@ ArrayRef FormatTokenLexer
 Tokens.push_back(getNextToken());
 if (Style.Language == FormatStyle::LK_JavaScript) {
   tryParseJSRegexLiteral();
-  tryParseTemplateString();
+  handleTemplateStrings();
 }
 tryMergePreviousTokens();
 if (Tokens.back()->NewlinesBefore > 0 || Tokens.back()->IsMultiline)
@@ -228,17 +227,42 @@ void FormatTokenLexer::tryParseJSRegexLi
   resetLexer(SourceMgr.getFileOffset(Lex->getSourceLocation(Offset)));
 }
 
-void FormatTokenLexer::tryParseTemplateString() {
+void FormatTokenLexer::handleTemplateStrings() {
   FormatToken *BacktickToken = Tokens.back();
-  if (!BacktickToken->is(tok::unknown) || BacktickToken->TokenText != "`")
+
+  if (BacktickToken->is(tok::l_brace)) {
+StateStack.push(LexerState::NORMAL);
 return;
+  }
+  if (BacktickToken->is(tok::r_brace)) {
+StateStack.pop();
+if (StateStack.top() != LexerState::TEMPLATE_STRING)
+  return;
+// If back in TEMPLATE_STRING, fallthrough and continue parsing the
+  } else if (BacktickToken->is(tok::unknown) &&
+ BacktickToken->TokenText == "`") {
+StateStack.push(LexerState::TEMPLATE_STRING);
+  } else {
+return; // Not actually a template
+  }
 
   // 'Manually' lex ahead in the current file buffer.
   const char *Offset = Lex->getBufferLocation();
   const char *TmplBegin = Offset - BacktickToken->TokenText.size(); // at "`"
-  for (; Offset != Lex->getBuffer().end() && *Offset != '`'; ++Offset) {
-if (*Offset == '\\')
+  for (; Offset != Lex->getBuffer().end(); ++Offset) {
+if (Offset[0] == '`') {
+  StateStack.pop();
+  break;
+}
+if (Offset[0] == '\\') {
   ++Offset; // Skip the escaped character.
+} else if (Offset + 1 < Lex->getBuffer().end() && Offset[0] == '$' &&
+   Offset[1] == '{') {
+  // '${' introduces an expression interpolation in the template string.
+  StateStack.push(LexerState::NORMAL);
+  ++Offset;
+  break;
+}
   }
 
   StringRef LiteralText(TmplBegin, Offset - TmplBegin + 1);
@@ -262,7 +286,10 @@ void FormatTokenLexer::tryParseTemplateS
 Style.TabWidth, Encoding);
   }
 
-  resetLexer(SourceMgr.getFileOffset(Lex->getSourceLocation(Offset + 1)));
+  SourceLocation loc = Offset < Lex->getBuffer().end()
+   ? Lex->getSourceLocation(Offset + 1)
+   : SourceMgr.getLocForEndOfFile(ID);
+  resetLexer(SourceMgr.getFileOffset(loc));
 }
 
 bool FormatTokenLexer::tryMerge_TMacro() {
@@ -384,12 +411,8 @@ FormatToken *FormatTokenLexer::getStashe
 }
 
 FormatToken *FormatTokenLexer::getNextToken() {
-  

Re: [PATCH] D22431: clang-format: [JS] nested and tagged template strings.

2016-08-25 Thread Martin Probst via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL279727: clang-format: [JS] nested and tagged template 
strings. (authored by mprobst).

Changed prior to commit:
  https://reviews.llvm.org/D22431?vs=68853&id=69219#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D22431

Files:
  cfe/trunk/lib/Format/FormatTokenLexer.cpp
  cfe/trunk/lib/Format/FormatTokenLexer.h
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTestJS.cpp

Index: cfe/trunk/lib/Format/FormatTokenLexer.h
===
--- cfe/trunk/lib/Format/FormatTokenLexer.h
+++ cfe/trunk/lib/Format/FormatTokenLexer.h
@@ -23,9 +23,17 @@
 #include "clang/Format/Format.h"
 #include "llvm/Support/Regex.h"
 
+#include 
+
 namespace clang {
 namespace format {
 
+enum LexerState {
+  NORMAL,
+  TEMPLATE_STRING,
+  TOKEN_STASHED,
+};
+
 class FormatTokenLexer {
 public:
   FormatTokenLexer(const SourceManager &SourceMgr, FileID ID,
@@ -53,7 +61,16 @@
   // its text if successful.
   void tryParseJSRegexLiteral();
 
-  void tryParseTemplateString();
+  // Handles JavaScript template strings.
+  //
+  // JavaScript template strings use backticks ('`') as delimiters, and allow
+  // embedding expressions nested in ${expr-here}. Template strings can be
+  // nested recursively, i.e. expressions can contain template strings in turn.
+  //
+  // The code below parses starting from a backtick, up to a closing backtick or
+  // an opening ${. It also maintains a stack of lexing contexts to handle
+  // nested template parts by balancing curly braces.
+  void handleTemplateStrings();
 
   bool tryMerge_TMacro();
 
@@ -65,7 +82,7 @@
 
   FormatToken *FormatTok;
   bool IsFirstToken;
-  bool GreaterStashed, LessStashed;
+  std::stack StateStack;
   unsigned Column;
   unsigned TrailingWhitespace;
   std::unique_ptr Lex;
Index: cfe/trunk/lib/Format/FormatTokenLexer.cpp
===
--- cfe/trunk/lib/Format/FormatTokenLexer.cpp
+++ cfe/trunk/lib/Format/FormatTokenLexer.cpp
@@ -26,12 +26,11 @@
 FormatTokenLexer::FormatTokenLexer(const SourceManager &SourceMgr, FileID ID,
const FormatStyle &Style,
encoding::Encoding Encoding)
-: FormatTok(nullptr), IsFirstToken(true), GreaterStashed(false),
-  LessStashed(false), Column(0), TrailingWhitespace(0),
-  SourceMgr(SourceMgr), ID(ID), Style(Style),
-  IdentTable(getFormattingLangOpts(Style)), Keywords(IdentTable),
-  Encoding(Encoding), FirstInLineIndex(0), FormattingDisabled(false),
-  MacroBlockBeginRegex(Style.MacroBlockBegin),
+: FormatTok(nullptr), IsFirstToken(true), StateStack({LexerState::NORMAL}),
+  Column(0), TrailingWhitespace(0), SourceMgr(SourceMgr), ID(ID),
+  Style(Style), IdentTable(getFormattingLangOpts(Style)),
+  Keywords(IdentTable), Encoding(Encoding), FirstInLineIndex(0),
+  FormattingDisabled(false), MacroBlockBeginRegex(Style.MacroBlockBegin),
   MacroBlockEndRegex(Style.MacroBlockEnd) {
   Lex.reset(new Lexer(ID, SourceMgr.getBuffer(ID), SourceMgr,
   getFormattingLangOpts(Style)));
@@ -49,7 +48,7 @@
 Tokens.push_back(getNextToken());
 if (Style.Language == FormatStyle::LK_JavaScript) {
   tryParseJSRegexLiteral();
-  tryParseTemplateString();
+  handleTemplateStrings();
 }
 tryMergePreviousTokens();
 if (Tokens.back()->NewlinesBefore > 0 || Tokens.back()->IsMultiline)
@@ -228,17 +227,42 @@
   resetLexer(SourceMgr.getFileOffset(Lex->getSourceLocation(Offset)));
 }
 
-void FormatTokenLexer::tryParseTemplateString() {
+void FormatTokenLexer::handleTemplateStrings() {
   FormatToken *BacktickToken = Tokens.back();
-  if (!BacktickToken->is(tok::unknown) || BacktickToken->TokenText != "`")
+
+  if (BacktickToken->is(tok::l_brace)) {
+StateStack.push(LexerState::NORMAL);
 return;
+  }
+  if (BacktickToken->is(tok::r_brace)) {
+StateStack.pop();
+if (StateStack.top() != LexerState::TEMPLATE_STRING)
+  return;
+// If back in TEMPLATE_STRING, fallthrough and continue parsing the
+  } else if (BacktickToken->is(tok::unknown) &&
+ BacktickToken->TokenText == "`") {
+StateStack.push(LexerState::TEMPLATE_STRING);
+  } else {
+return; // Not actually a template
+  }
 
   // 'Manually' lex ahead in the current file buffer.
   const char *Offset = Lex->getBufferLocation();
   const char *TmplBegin = Offset - BacktickToken->TokenText.size(); // at "`"
-  for (; Offset != Lex->getBuffer().end() && *Offset != '`'; ++Offset) {
-if (*Offset == '\\')
+  for (; Offset != Lex->getBuffer().end(); ++Offset) {
+if (Offset[0] == '`') {
+  StateStack.pop();
+  break;
+}
+if (Offset[0] == '\\') {
   ++Offset; // Skip the escaped character.
+} else if (Offset + 1 < Lex->getBuffer().end() && Offset[0] == '$' &&
+ 

Re: [PATCH] D23837: Fix colored diagnostics from tools

2016-08-25 Thread Olivier Goffart via cfe-commits
ogoffart updated this revision to Diff 69221.
ogoffart added a comment.

Added a test.


https://reviews.llvm.org/D23837

Files:
  lib/Tooling/Tooling.cpp
  unittests/Tooling/ToolingTest.cpp

Index: unittests/Tooling/ToolingTest.cpp
===
--- unittests/Tooling/ToolingTest.cpp
+++ unittests/Tooling/ToolingTest.cpp
@@ -332,6 +332,47 @@
   EXPECT_FALSE(llvm::sys::fs::remove(DepFilePath.str()));
 }
 
+struct CheckColoredDiagnosticsAction : public clang::ASTFrontendAction {
+  CheckColoredDiagnosticsAction(bool ShouldShowColor)
+  : ShouldShowColor(ShouldShowColor) {}
+  std::unique_ptr CreateASTConsumer(CompilerInstance &Compiler,
+ StringRef) override {
+if (Compiler.getDiagnosticOpts().ShowColors != ShouldShowColor)
+  Compiler.getDiagnostics().Report(
+  Compiler.getDiagnostics().getCustomDiagID(
+  DiagnosticsEngine::Fatal,
+  "getDiagnosticOpts().ShowColors != ShouldShowColor"));
+return llvm::make_unique();
+  }
+
+private:
+  bool ShouldShowColor = true;
+};
+
+TEST(runToolOnCodeWithArgs, DiagnosticsColor) {
+  // Defaults to showing the colors.
+  EXPECT_TRUE(
+  runToolOnCodeWithArgs(new CheckColoredDiagnosticsAction(true), "", {}));
+  // Check that this test would fail if ShowColors is not what it should.
+  EXPECT_FALSE(
+  runToolOnCodeWithArgs(new CheckColoredDiagnosticsAction(false), "", {}));
+
+  // Last argument wins.
+  EXPECT_TRUE(runToolOnCodeWithArgs(new CheckColoredDiagnosticsAction(true), 
"",
+{"-fcolor-diagnostics"}));
+  EXPECT_TRUE(runToolOnCodeWithArgs(new CheckColoredDiagnosticsAction(false),
+"", {"-fno-color-diagnostics"}));
+  EXPECT_TRUE(
+  runToolOnCodeWithArgs(new CheckColoredDiagnosticsAction(true), "",
+{"-fno-color-diagnostics", 
"-fcolor-diagnostics"}));
+  EXPECT_TRUE(
+  runToolOnCodeWithArgs(new CheckColoredDiagnosticsAction(false), "",
+{"-fcolor-diagnostics", 
"-fno-color-diagnostics"}));
+  EXPECT_TRUE(runToolOnCodeWithArgs(
+  new CheckColoredDiagnosticsAction(true), "",
+  {"-fno-color-diagnostics", "-fdiagnostics-color=always"}));
+}
+
 TEST(ClangToolTest, ArgumentAdjusters) {
   FixedCompilationDatabase Compilations("/", std::vector());
 
Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -15,6 +15,7 @@
 #include "clang/Tooling/Tooling.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
+#include "clang/Driver/Options.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
 #include "clang/Frontend/ASTUnit.h"
@@ -26,6 +27,7 @@
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/Option/ArgList.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FileSystem.h"
@@ -241,6 +243,12 @@
 Argv.push_back(Str.c_str());
   const char *const BinaryName = Argv[0];
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  unsigned MissingArgIndex, MissingArgCount;
+  llvm::opt::InputArgList ParsedArgs =
+  driver::createDriverOptTable()->ParseArgs(
+  ArrayRef(Argv).slice(1), MissingArgIndex,
+  MissingArgCount);
+  ParseDiagnosticArgs(*DiagOpts, ParsedArgs);
   TextDiagnosticPrinter DiagnosticPrinter(
   llvm::errs(), &*DiagOpts);
   DiagnosticsEngine Diagnostics(


Index: unittests/Tooling/ToolingTest.cpp
===
--- unittests/Tooling/ToolingTest.cpp
+++ unittests/Tooling/ToolingTest.cpp
@@ -332,6 +332,47 @@
   EXPECT_FALSE(llvm::sys::fs::remove(DepFilePath.str()));
 }
 
+struct CheckColoredDiagnosticsAction : public clang::ASTFrontendAction {
+  CheckColoredDiagnosticsAction(bool ShouldShowColor)
+  : ShouldShowColor(ShouldShowColor) {}
+  std::unique_ptr CreateASTConsumer(CompilerInstance &Compiler,
+ StringRef) override {
+if (Compiler.getDiagnosticOpts().ShowColors != ShouldShowColor)
+  Compiler.getDiagnostics().Report(
+  Compiler.getDiagnostics().getCustomDiagID(
+  DiagnosticsEngine::Fatal,
+  "getDiagnosticOpts().ShowColors != ShouldShowColor"));
+return llvm::make_unique();
+  }
+
+private:
+  bool ShouldShowColor = true;
+};
+
+TEST(runToolOnCodeWithArgs, DiagnosticsColor) {
+  // Defaults to showing the colors.
+  EXPECT_TRUE(
+  runToolOnCodeWithArgs(new CheckColoredDiagnosticsAction(true), "", {}));
+  // Check that this test would fail if ShowColors is not what it should.
+  EXPECT_FALSE(
+  runToolOnCodeWithArgs(new CheckColoredDiagnosticsAction(false), "", {}));
+
+  // Last argument wins.
+  EXP

Re: [PATCH] D21502: Fix heuristics skipping invalid ctor-initializers with C++11

2016-08-25 Thread Olivier Goffart via cfe-commits
ogoffart updated this revision to Diff 69223.
ogoffart added a comment.

Made the requested changes


https://reviews.llvm.org/D21502

Files:
  lib/Parse/ParseCXXInlineMethods.cpp
  test/CodeCompletion/ctor-initializer.cpp

Index: test/CodeCompletion/ctor-initializer.cpp
===
--- test/CodeCompletion/ctor-initializer.cpp
+++ test/CodeCompletion/ctor-initializer.cpp
@@ -1,11 +1,13 @@
 struct Base1 {
   Base1() : {}
-  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:2:12 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:2:12 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:2:12 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
   // CHECK-CC1: COMPLETION: Pattern : member1(<#args#>)
   // CHECK-CC1: COMPLETION: Pattern : member2(<#args#>
 
   Base1(int) : member1(123), {}
-  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:7:30 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:8:30 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:8:30 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
   // CHECK-CC2-NOT: COMPLETION: Pattern : member1(<#args#>)
   // CHECK-CC2: COMPLETION: Pattern : member2(<#args#>
 
@@ -21,21 +23,40 @@
 };
 
 Derived::Derived() : {}
-// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:23:22 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:25:22 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:25:22 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
 // CHECK-CC3: COMPLETION: Pattern : Base1(<#args#>)
 // CHECK-CC3: COMPLETION: Pattern : deriv1(<#args#>)
 
 Derived::Derived(int) try : {
 } catch (...) {
 }
-// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:28:29 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:31:29 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:31:29 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
 // CHECK-CC4: COMPLETION: Pattern : Base1(<#args#>)
 // CHECK-CC4: COMPLETION: Pattern : deriv1(<#args#>)
 
 Derived::Derived(float) try : Base1(),
 {
 } catch (...) {
 }
-// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:35:39 %s -o - | FileCheck -check-prefix=CHECK-CC5 %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:39:39 %s -o - | FileCheck -check-prefix=CHECK-CC5 %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:39:39 %s -o - | FileCheck -check-prefix=CHECK-CC5 %s
 // CHECK-CC5-NOT: COMPLETION: Pattern : Base1(<#args#>)
 // CHECK-CC5: COMPLETION: Pattern : deriv1(<#args#>)
+
+struct A {
+  A() : , member2() {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:49:9 %s -o - | FileCheck -check-prefix=CHECK-CC6 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:49:9 %s -o - | FileCheck -check-prefix=CHECK-CC6 %s
+  // CHECK-CC6: COMPLETION: Pattern : member1(<#args#>
+  int member1, member2;
+};
+
+struct B {
+  B() : member2() {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:57:9 %s -o - | FileCheck -check-prefix=CHECK-CC7 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:57:9 %s -o - | FileCheck -check-prefix=CHECK-CC7 %s
+  // CHECK-CC7: COMPLETION: Pattern : member1(<#args#>
+  int member1, member2;
+};
Index: lib/Parse/ParseCXXInlineMethods.cpp
===
--- lib/Parse/ParseCXXInlineMethods.cpp
+++ lib/Parse/ParseCXXInlineMethods.cpp
@@ -832,22 +832,30 @@
 }
   }
 
-  if (Tok.isOneOf(tok::identifier, tok::kw_template)) {
+  if (Tok.is(tok::identifier)) {
 Toks.push_back(Tok);
 ConsumeToken();
-  } else if (Tok.is(tok::code_completion)) {
-Toks.push_back(Tok);
-ConsumeCodeCompletionToken();
-// Consume the rest of the initializers permissively.
-// FIXME: We should be able to perform code-completion here even if
-//there isn't a subsequent '{' token.
-MightBeTemplateArgument = true;
-break;
   } else {
 break;
   }
 } while (Tok.is(tok::coloncolon));
 
+if (Tok.is(tok::code_completion)) {
+  Toks.push_back(Tok);
+  ConsumeCodeCompletionToken();
+  if (Tok.isOneOf(tok::identifier, tok::coloncolon, tok::kw_decltype)) {
+// Could be the start of another member initializer (the ',' has not
+// been written yet)
+continue;
+  }
+}
+
+if (Tok.is(tok::comma)) {
+  // The initialization is missing, we'll diagnose i

Re: [PATCH] D21502: Fix heuristics skipping invalid ctor-initializers with C++11

2016-08-25 Thread Olivier Goffart via cfe-commits
ogoffart added a comment.

Regarding this:

  struct Foo {
Foo() : some_long_x(0), some_| {}
int some_long_x, some_long_y;
  };

That should work fine because the token before the { is the code completion 
token, not an identifier.  This is basicaly tested by the test with CHECK-CC2.

However, this does not work because if the completion is within an identifier, 
the Lexer will abort by calling cutOffLexing from Lexer::LexIdentifier


https://reviews.llvm.org/D21502



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


Re: [PATCH] D23855: Make exception-throwing from a noexcept build `abort()`.

2016-08-25 Thread Sebastian Pop via cfe-commits
sebpop accepted this revision.
sebpop added a comment.
This revision is now accepted and ready to land.

Very nice cleanup.
Maybe you can move some more #ifdefs into __throw_* functions, although as is 
LGTM.
Thanks!


https://reviews.llvm.org/D23855



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


[libcxx] r279731 - Remove trailing WS [NFC]

2016-08-25 Thread Aditya Kumar via cfe-commits
Author: hiraditya
Date: Thu Aug 25 06:52:38 2016
New Revision: 279731

URL: http://llvm.org/viewvc/llvm-project?rev=279731&view=rev
Log:
Remove trailing WS [NFC]

Modified:
libcxx/trunk/include/algorithm

Modified: libcxx/trunk/include/algorithm
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/algorithm?rev=279731&r1=279730&r2=279731&view=diff
==
--- libcxx/trunk/include/algorithm (original)
+++ libcxx/trunk/include/algorithm Thu Aug 25 06:52:38 2016
@@ -89,7 +89,7 @@ template 
 pair
-mismatch(InputIterator1 first1, InputIterator1 last1, 
+mismatch(InputIterator1 first1, InputIterator1 last1,
  InputIterator2 first2, InputIterator2 last2); // **C++14**
 
 template 
@@ -109,7 +109,7 @@ template 
 bool
-equal(InputIterator1 first1, InputIterator1 last1, 
+equal(InputIterator1 first1, InputIterator1 last1,
   InputIterator2 first2, InputIterator2 last2); // **C++14**
 
 template 
@@ -688,7 +688,7 @@ struct __equal_to<_T1, const _T1>
 template 
 struct __less
 {
-_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11 
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
 bool operator()(const _T1& __x, const _T1& __y) const {return __x < __y;}
 
 _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
@@ -1229,7 +1229,7 @@ equal(_InputIterator1 __first1, _InputIt
 template 
 inline _LIBCPP_INLINE_VISIBILITY
 bool
-__equal(_InputIterator1 __first1, _InputIterator1 __last1, 
+__equal(_InputIterator1 __first1, _InputIterator1 __last1,
 _InputIterator2 __first2, _InputIterator2 __last2, _BinaryPredicate 
__pred,
 input_iterator_tag, input_iterator_tag )
 {
@@ -1242,8 +1242,8 @@ __equal(_InputIterator1 __first1, _Input
 template 
 inline _LIBCPP_INLINE_VISIBILITY
 bool
-__equal(_RandomAccessIterator1 __first1, _RandomAccessIterator1 __last1, 
-_RandomAccessIterator2 __first2, _RandomAccessIterator2 __last2, 
_BinaryPredicate __pred, 
+__equal(_RandomAccessIterator1 __first1, _RandomAccessIterator1 __last1,
+_RandomAccessIterator2 __first2, _RandomAccessIterator2 __last2, 
_BinaryPredicate __pred,
   random_access_iterator_tag, random_access_iterator_tag )
 {
 if ( _VSTD::distance(__first1, __last1) != _VSTD::distance(__first2, 
__last2))
@@ -1256,11 +1256,11 @@ __equal(_RandomAccessIterator1 __first1,
 template 
 inline _LIBCPP_INLINE_VISIBILITY
 bool
-equal(_InputIterator1 __first1, _InputIterator1 __last1, 
+equal(_InputIterator1 __first1, _InputIterator1 __last1,
   _InputIterator2 __first2, _InputIterator2 __last2, _BinaryPredicate 
__pred )
 {
 return _VSTD::__equal::type>
-   (__first1, __last1, __first2, __last2, __pred, 
+   (__first1, __last1, __first2, __last2, __pred,
 typename iterator_traits<_InputIterator1>::iterator_category(),
 typename iterator_traits<_InputIterator2>::iterator_category());
 }
@@ -1268,7 +1268,7 @@ equal(_InputIterator1 __first1, _InputIt
 template 
 inline _LIBCPP_INLINE_VISIBILITY
 bool
-equal(_InputIterator1 __first1, _InputIterator1 __last1, 
+equal(_InputIterator1 __first1, _InputIterator1 __last1,
   _InputIterator2 __first2, _InputIterator2 __last2)
 {
 typedef typename iterator_traits<_InputIterator1>::value_type __v1;
@@ -1342,7 +1342,7 @@ is_permutation(_ForwardIterator1 __first
 template
 bool
 __is_permutation(_ForwardIterator1 __first1, _ForwardIterator1 __last1,
- _ForwardIterator2 __first2, _ForwardIterator2 __last2, 
+ _ForwardIterator2 __first2, _ForwardIterator2 __last2,
  _BinaryPredicate __pred,
  forward_iterator_tag, forward_iterator_tag )
 {
@@ -1393,7 +1393,7 @@ __next_iter:;
 template
 bool
 __is_permutation(_RandomAccessIterator1 __first1, _RandomAccessIterator2 
__last1,
-   _RandomAccessIterator1 __first2, _RandomAccessIterator2 
__last2, 
+   _RandomAccessIterator1 __first2, _RandomAccessIterator2 __last2,
_BinaryPredicate __pred,
random_access_iterator_tag, random_access_iterator_tag )
 {
@@ -1472,7 +1472,7 @@ __search(_ForwardIterator1 __first1, _Fo
 }
 
 template 
-_LIBCPP_CONSTEXPR_AFTER_CXX11 
+_LIBCPP_CONSTEXPR_AFTER_CXX11
 pair<_RandomAccessIterator1, _RandomAccessIterator1>
 __search(_RandomAccessIterator1 __first1, _RandomAccessIterator1 __last1,
  _RandomAccessIterator2 __first2, _RandomAccessIterator2 __last2, 
_BinaryPredicate __pred,
@@ -2799,7 +2799,7 @@ minmax(initializer_list<_Tp> __t, _Compa
 __result.second = *__first;
 ++__first;
 }
-
+
 while (__first != __last)
 {
 _Tp __prev = *__first++;
@@ -2811,7 +2811,7 @@ minmax(initializer_list<_Tp> __t, _Compa
 if ( __comp(__prev, __result.first))__result.first  = __prev;
 if (!__comp(*__first, __result.second)) __result.second = *__first;
 }
- 

Re: [PATCH] D23837: Fix colored diagnostics from tools

2016-08-25 Thread Olivier Goffart via cfe-commits
ogoffart updated this revision to Diff 69225.
ogoffart added a comment.

This new patch make sure the test run fine as part of the testsuite, where the 
output is not a terminal and the color are disabled by default


https://reviews.llvm.org/D23837

Files:
  lib/Tooling/Tooling.cpp
  unittests/Tooling/ToolingTest.cpp

Index: unittests/Tooling/ToolingTest.cpp
===
--- unittests/Tooling/ToolingTest.cpp
+++ unittests/Tooling/ToolingTest.cpp
@@ -332,6 +332,44 @@
   EXPECT_FALSE(llvm::sys::fs::remove(DepFilePath.str()));
 }
 
+struct CheckColoredDiagnosticsAction : public clang::ASTFrontendAction {
+  CheckColoredDiagnosticsAction(bool ShouldShowColor)
+  : ShouldShowColor(ShouldShowColor) {}
+  std::unique_ptr CreateASTConsumer(CompilerInstance &Compiler,
+ StringRef) override {
+if (Compiler.getDiagnosticOpts().ShowColors != ShouldShowColor)
+  Compiler.getDiagnostics().Report(
+  Compiler.getDiagnostics().getCustomDiagID(
+  DiagnosticsEngine::Fatal,
+  "getDiagnosticOpts().ShowColors != ShouldShowColor"));
+return llvm::make_unique();
+  }
+
+private:
+  bool ShouldShowColor = true;
+};
+
+TEST(runToolOnCodeWithArgs, DiagnosticsColor) {
+
+  EXPECT_TRUE(runToolOnCodeWithArgs(new CheckColoredDiagnosticsAction(true), 
"",
+{"-fcolor-diagnostics"}));
+  EXPECT_TRUE(runToolOnCodeWithArgs(new CheckColoredDiagnosticsAction(false),
+"", {"-fno-color-diagnostics"}));
+  EXPECT_TRUE(
+  runToolOnCodeWithArgs(new CheckColoredDiagnosticsAction(true), "",
+{"-fno-color-diagnostics", 
"-fcolor-diagnostics"}));
+  EXPECT_TRUE(
+  runToolOnCodeWithArgs(new CheckColoredDiagnosticsAction(false), "",
+{"-fcolor-diagnostics", 
"-fno-color-diagnostics"}));
+  EXPECT_TRUE(runToolOnCodeWithArgs(
+  new CheckColoredDiagnosticsAction(true), "",
+  {"-fno-color-diagnostics", "-fdiagnostics-color=always"}));
+
+  // Check that this test would fail if ShowColors is not what it should.
+  EXPECT_FALSE(runToolOnCodeWithArgs(new CheckColoredDiagnosticsAction(false),
+ "", {"-fcolor-diagnostics"}));
+}
+
 TEST(ClangToolTest, ArgumentAdjusters) {
   FixedCompilationDatabase Compilations("/", std::vector());
 
Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -15,6 +15,7 @@
 #include "clang/Tooling/Tooling.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
+#include "clang/Driver/Options.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
 #include "clang/Frontend/ASTUnit.h"
@@ -26,6 +27,7 @@
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/Option/ArgList.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FileSystem.h"
@@ -241,6 +243,12 @@
 Argv.push_back(Str.c_str());
   const char *const BinaryName = Argv[0];
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  unsigned MissingArgIndex, MissingArgCount;
+  llvm::opt::InputArgList ParsedArgs =
+  driver::createDriverOptTable()->ParseArgs(
+  ArrayRef(Argv).slice(1), MissingArgIndex,
+  MissingArgCount);
+  ParseDiagnosticArgs(*DiagOpts, ParsedArgs);
   TextDiagnosticPrinter DiagnosticPrinter(
   llvm::errs(), &*DiagOpts);
   DiagnosticsEngine Diagnostics(


Index: unittests/Tooling/ToolingTest.cpp
===
--- unittests/Tooling/ToolingTest.cpp
+++ unittests/Tooling/ToolingTest.cpp
@@ -332,6 +332,44 @@
   EXPECT_FALSE(llvm::sys::fs::remove(DepFilePath.str()));
 }
 
+struct CheckColoredDiagnosticsAction : public clang::ASTFrontendAction {
+  CheckColoredDiagnosticsAction(bool ShouldShowColor)
+  : ShouldShowColor(ShouldShowColor) {}
+  std::unique_ptr CreateASTConsumer(CompilerInstance &Compiler,
+ StringRef) override {
+if (Compiler.getDiagnosticOpts().ShowColors != ShouldShowColor)
+  Compiler.getDiagnostics().Report(
+  Compiler.getDiagnostics().getCustomDiagID(
+  DiagnosticsEngine::Fatal,
+  "getDiagnosticOpts().ShowColors != ShouldShowColor"));
+return llvm::make_unique();
+  }
+
+private:
+  bool ShouldShowColor = true;
+};
+
+TEST(runToolOnCodeWithArgs, DiagnosticsColor) {
+
+  EXPECT_TRUE(runToolOnCodeWithArgs(new CheckColoredDiagnosticsAction(true), "",
+{"-fcolor-diagnostics"}));
+  EXPECT_TRUE(runToolOnCodeWithArgs(new CheckColoredDiagnosticsAction(false),
+"", {"-fno-color-diagnostics"}));
+  EXPECT_TRUE(
+

[PATCH] D23871: [Modules] Add 'freestanding' to the 'requires-declaration' feature-list.

2016-08-25 Thread Elad Cohen via cfe-commits
eladcohen created this revision.
eladcohen added reviewers: rsmith, andreybokhanko.
eladcohen added a subscriber: cfe-commits.

This adds support for modules that require (non-)freestanding environment such 
as the compiler builtin mm_malloc submodule.

https://reviews.llvm.org/D23871

Files:
  docs/Modules.rst
  lib/Basic/Module.cpp
  lib/Headers/module.modulemap
  test/Modules/compiler_builtins_x86.c

Index: test/Modules/compiler_builtins_x86.c
===
--- test/Modules/compiler_builtins_x86.c
+++ test/Modules/compiler_builtins_x86.c
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t %s -verify -ffreestanding
+// expected-no-diagnostics
+
+#include
+
Index: lib/Headers/module.modulemap
===
--- lib/Headers/module.modulemap
+++ lib/Headers/module.modulemap
@@ -63,6 +63,7 @@
 textual header "mwaitxintrin.h"
 
 explicit module mm_malloc {
+  requires !freestanding
   header "mm_malloc.h"
   export * // note: for  dependency
 }
Index: lib/Basic/Module.cpp
===
--- lib/Basic/Module.cpp
+++ lib/Basic/Module.cpp
@@ -64,6 +64,7 @@
 .Case("blocks", LangOpts.Blocks)
 .Case("cplusplus", LangOpts.CPlusPlus)
 .Case("cplusplus11", LangOpts.CPlusPlus11)
+.Case("freestanding", LangOpts.Freestanding)
 .Case("objc", LangOpts.ObjC1)
 .Case("objc_arc", LangOpts.ObjCAutoRefCount)
 .Case("opencl", LangOpts.OpenCL)
Index: docs/Modules.rst
===
--- docs/Modules.rst
+++ docs/Modules.rst
@@ -413,6 +413,9 @@
 cplusplus11
   C++11 support is available.
 
+freestanding
+  A freestanding environment is available.
+
 objc
   Objective-C support is available.
 


Index: test/Modules/compiler_builtins_x86.c
===
--- test/Modules/compiler_builtins_x86.c
+++ test/Modules/compiler_builtins_x86.c
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -verify -ffreestanding
+// expected-no-diagnostics
+
+#include
+
Index: lib/Headers/module.modulemap
===
--- lib/Headers/module.modulemap
+++ lib/Headers/module.modulemap
@@ -63,6 +63,7 @@
 textual header "mwaitxintrin.h"
 
 explicit module mm_malloc {
+  requires !freestanding
   header "mm_malloc.h"
   export * // note: for  dependency
 }
Index: lib/Basic/Module.cpp
===
--- lib/Basic/Module.cpp
+++ lib/Basic/Module.cpp
@@ -64,6 +64,7 @@
 .Case("blocks", LangOpts.Blocks)
 .Case("cplusplus", LangOpts.CPlusPlus)
 .Case("cplusplus11", LangOpts.CPlusPlus11)
+.Case("freestanding", LangOpts.Freestanding)
 .Case("objc", LangOpts.ObjC1)
 .Case("objc_arc", LangOpts.ObjCAutoRefCount)
 .Case("opencl", LangOpts.OpenCL)
Index: docs/Modules.rst
===
--- docs/Modules.rst
+++ docs/Modules.rst
@@ -413,6 +413,9 @@
 cplusplus11
   C++11 support is available.
 
+freestanding
+  A freestanding environment is available.
+
 objc
   Objective-C support is available.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r279741 - Fix offload bundler test to support Windows new lines.

2016-08-25 Thread Samuel Antao via cfe-commits
Author: sfantao
Date: Thu Aug 25 09:35:20 2016
New Revision: 279741

URL: http://llvm.org/viewvc/llvm-project?rev=279741&view=rev
Log:
Fix offload bundler test to support Windows new lines.

Modified:
cfe/trunk/test/Driver/clang-offload-bundler.c

Modified: cfe/trunk/test/Driver/clang-offload-bundler.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang-offload-bundler.c?rev=279741&r1=279740&r2=279741&view=diff
==
--- cfe/trunk/test/Driver/clang-offload-bundler.c (original)
+++ cfe/trunk/test/Driver/clang-offload-bundler.c Thu Aug 25 09:35:20 2016
@@ -230,8 +230,8 @@
 // RUN: clang-offload-bundler -type=o 
-targets=host-powerpc64le-ibm-linux-gnu,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
 -inputs=%t.o,%t.tgt1,%t.tgt2 -outputs=%t.bundle3.o -### -dump-temporary-files 
2>&1 \
 // RUN: | FileCheck %s --check-prefix CK-OBJ-CMD
 // CK-OBJ-CMD: private constant [1 x i8] zeroinitializer, section 
"__CLANG_OFFLOAD_BUNDLE__host-powerpc64le-ibm-linux-gnu"
-// CK-OBJ-CMD: private constant [25 x i8] c"Content of device file 1\0A", 
section "__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu"
-// CK-OBJ-CMD: private constant [25 x i8] c"Content of device file 2\0A", 
section "__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu"
+// CK-OBJ-CMD: private constant [25 x i8] c"Content of device file 1{{.+}}", 
section "__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu"
+// CK-OBJ-CMD: private constant [25 x i8] c"Content of device file 2{{.+}}", 
section "__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu"
 // CK-OBJ-CMD: clang" "-r" "-target" "powerpc64le-ibm-linux-gnu" "-o" 
"{{.+}}.o" "{{.+}}.o" "{{.+}}.bc" "-nostdlib"
 
 // RUN: clang-offload-bundler -type=o 
-targets=host-powerpc64le-ibm-linux-gnu,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
 -outputs=%t.res.o,%t.res.tgt1,%t.res.tgt2 -inputs=%s.o -unbundle


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


Re: r267338 - Improve diagnostic checking for va_start to also warn on other instances of undefined behavior, such as a parameter declared with the register keyword in C, or a parameter of a type that

2016-08-25 Thread Aaron Ballman via cfe-commits
On Tue, Aug 23, 2016 at 9:32 PM, Frédéric Riss  wrote:
> Hey Aaron,
>
> This commit triggers warnings when the argument to va_start is an enum type. 
> The standard says:
>
> 7.16.1.4 The va_start macro
>
> • 4  The parameter parmN is the identifier of the rightmost parameter 
> in the variable parameter list in the function definition (the one just 
> before the , ...). If the parameter parmN is declared with the register 
> storage class, with a function or array type, or with a type that is not 
> compatible with the type that results after application of the default 
> argument promotions, the behavior is undefined.
>
> It seems to me that using an enum as the argument to va_start is not UB when 
> the implementation chose to use an int to store the enum.

The implementation isn't required to choose int as the compatible type
for the enumeration. See 6.7.2.2p4: "Each enumerated type shall be
compatible with char, a signed integer type, or an unsigned integer
type." In the case where the compatible type is char, this use is UB.

> Would you agree the warning as implemented gives false positives in that case?

It depends entirely on the enumeration and what compatible type is
chosen for it. If the compatible type is signed or unsigned int, then
the warning would be a false-positive and we should silence it. If the
compatible type is char, then the warning is not a false positive.

~Aaron

>
> Thanks,
> Fred
>
>
>> On Apr 24, 2016, at 6:30 AM, Aaron Ballman via cfe-commits 
>>  wrote:
>>
>> Author: aaronballman
>> Date: Sun Apr 24 08:30:21 2016
>> New Revision: 267338
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=267338&view=rev
>> Log:
>> Improve diagnostic checking for va_start to also warn on other instances of 
>> undefined behavior, such as a parameter declared with the register keyword 
>> in C, or a parameter of a type that undergoes default argument promotion.
>>
>> This helps cover some more of the CERT secure coding rule EXP58-CPP. Pass an 
>> object of the correct type to va_start 
>> (https://www.securecoding.cert.org/confluence/display/cplusplus/EXP58-CPP.+Pass+an+object+of+the+correct+type+to+va_start).
>>
>> Added:
>>cfe/trunk/test/SemaCXX/varargs.cpp
>> Removed:
>>cfe/trunk/test/Sema/varargs.cpp
>> Modified:
>>cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>cfe/trunk/lib/Sema/SemaChecking.cpp
>>cfe/trunk/test/Sema/varargs-x86-64.c
>>cfe/trunk/test/Sema/varargs.c
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=267338&r1=267337&r2=267338&view=diff
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Apr 24 08:30:21 
>> 2016
>> @@ -7435,8 +7435,10 @@ def err_ms_va_start_used_in_sysv_functio
>> def warn_second_arg_of_va_start_not_last_named_param : Warning<
>>   "second argument to 'va_start' is not the last named parameter">,
>>   InGroup;
>> -def warn_va_start_of_reference_type_is_undefined : Warning<
>> -  "'va_start' has undefined behavior with reference types">, 
>> InGroup;
>> +def warn_va_start_type_is_undefined : Warning<
>> +  "passing %select{an object that undergoes default argument promotion|"
>> +  "an object of reference type|a parameter declared with the 'register' "
>> +  "keyword}0 to 'va_start' has undefined behavior">, InGroup;
>> def err_first_argument_to_va_arg_not_of_type_va_list : Error<
>>   "first argument to 'va_arg' is of type %0 and not 'va_list'">;
>> def err_second_parameter_to_va_arg_incomplete: Error<
>>
>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=267338&r1=267337&r2=267338&view=diff
>> ==
>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Sun Apr 24 08:30:21 2016
>> @@ -2702,6 +2702,7 @@ bool Sema::SemaBuiltinVAStartImpl(CallEx
>>   // block.
>>   QualType Type;
>>   SourceLocation ParamLoc;
>> +  bool IsCRegister = false;
>>
>>   if (const DeclRefExpr *DR = dyn_cast(Arg)) {
>> if (const ParmVarDecl *PV = dyn_cast(DR->getDecl())) {
>> @@ -2718,15 +2719,21 @@ bool Sema::SemaBuiltinVAStartImpl(CallEx
>>
>>   Type = PV->getType();
>>   ParamLoc = PV->getLocation();
>> +  IsCRegister =
>> +  PV->getStorageClass() == SC_Register && !getLangOpts().CPlusPlus;
>> }
>>   }
>>
>>   if (!SecondArgIsLastNamedArgument)
>> Diag(TheCall->getArg(1)->getLocStart(),
>>  diag::warn_second_arg_of_va_start_not_last_named_param);
>> -  else if (Type->isReferenceType()) {
>> -Diag(Arg->getLocStart(),
>> - diag::warn_va_start_of_reference_type_is_undefined);
>> +  else if (IsCRegister || Type->i

Re: r279653 - Fix offload bundler tests so that diagnostic can start with caps.

2016-08-25 Thread Samuel F Antao via cfe-commits
Thanks for letting me know, should be fixed in r279741.
Samuel
 
- Original message -From: Vitaly Buka To: Samuel F Antao/Watson/IBM@IBMUS, cfe-commits@lists.llvm.orgCc:Subject: Re: r279653 - Fix offload bundler tests so that diagnostic can start with caps.Date: Thu, Aug 25, 2016 4:04 AM 
It still fails on Windows on another line, \0A vs \0D\0A
C:\buildbot\slave-config\clang-x86-win2008-selfhost\llvm\tools\clang\test\Driver\clang-offload-bundler.c:233:16: error: expected string not found in input// CK-OBJ-CMD: private constant [25 x i8] c"Content of device file 1\0A", section "__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu"               ^:9:1: note: scanning from here@1 = private constant [26 x i8] c"Content of device file 1\0D\0A", section "__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu"^:9:6: note: possible intended match here@1 = private constant [26 x i8] c"Content of device file 1\0D\0A", section "__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu"
 
 http://lab.llvm.org:8011/builders/clang-x86-win2008-selfhost/builds/9871/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aclang-offload-bundler.c 

On Wed, Aug 24, 2016 at 12:00 PM Samuel Antao via cfe-commits  wrote:
Author: sfantaoDate: Wed Aug 24 13:52:18 2016New Revision: 279653URL: http://llvm.org/viewvc/llvm-project?rev=279653&view=revLog:Fix offload bundler tests so that diagnostic can start with caps.Windows require that.Modified:    cfe/trunk/test/Driver/clang-offload-bundler.cModified: cfe/trunk/test/Driver/clang-offload-bundler.cURL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang-offload-bundler.c?rev=279653&r1=279652&r2=279653&view=diff==--- cfe/trunk/test/Driver/clang-offload-bundler.c (original)+++ cfe/trunk/test/Driver/clang-offload-bundler.c Wed Aug 24 13:52:18 2016@@ -68,7 +68,7 @@ // RUN: not clang-offload-bundler -type=i -targets=host-powerpc64le-ibm-linux-gnu,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -inputs=%t.i,%t.tgt1,%t.tgt2.notexist -outputs=%t.bundle.i 2>&1 | FileCheck %s --check-prefix CK-ERR5 // RUN: not clang-offload-bundler -type=i -targets=host-powerpc64le-ibm-linux-gnu,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.i,%t.tgt1,%t.tgt2 -inputs=%t.bundle.i.notexist -unbundle 2>&1 | FileCheck %s --check-prefix CK-ERR5-// CK-ERR5: error: Can't open file {{.+}}.notexist: No such file or directory+// CK-ERR5: error: Can't open file {{.+}}.notexist: {{N|n}}o such file or directory // RUN: not clang-offload-bundler -type=invalid -targets=host-powerpc64le-ibm-linux-gnu,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -inputs=%t.i,%t.tgt1,%t.tgt2 -outputs=%t.bundle.i 2>&1 | FileCheck %s --check-prefix CK-ERR6 // CK-ERR6: error: invalid file type specified.___cfe-commits mailing listcfe-commits@lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
 

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


Re: r267338 - Improve diagnostic checking for va_start to also warn on other instances of undefined behavior, such as a parameter declared with the register keyword in C, or a parameter of a type that

2016-08-25 Thread Frédéric Riss via cfe-commits

> On Aug 25, 2016, at 7:44 AM, Aaron Ballman  wrote:
> 
> On Tue, Aug 23, 2016 at 9:32 PM, Frédéric Riss  wrote:
>> Hey Aaron,
>> 
>> This commit triggers warnings when the argument to va_start is an enum type. 
>> The standard says:
>> 
>> 7.16.1.4 The va_start macro
>> 
>>• 4  The parameter parmN is the identifier of the rightmost parameter 
>> in the variable parameter list in the function definition (the one just 
>> before the , ...). If the parameter parmN is declared with the register 
>> storage class, with a function or array type, or with a type that is not 
>> compatible with the type that results after application of the default 
>> argument promotions, the behavior is undefined.
>> 
>> It seems to me that using an enum as the argument to va_start is not UB when 
>> the implementation chose to use an int to store the enum.
> 
> The implementation isn't required to choose int as the compatible type
> for the enumeration. See 6.7.2.2p4: "Each enumerated type shall be
> compatible with char, a signed integer type, or an unsigned integer
> type." In the case where the compatible type is char, this use is UB.
> 
>> Would you agree the warning as implemented gives false positives in that 
>> case?
> 
> It depends entirely on the enumeration and what compatible type is
> chosen for it. If the compatible type is signed or unsigned int, then
> the warning would be a false-positive and we should silence it. If the
> compatible type is char, then the warning is not a false positive.

I think we agree? If I’m not mistaken, in clang’s case, an int compatible type 
will always be chosen except if -fshort-enums is passed and the enum fits in a 
smaller type. Do you want a PR?

Fred

> ~Aaron
> 
>> 
>> Thanks,
>> Fred
>> 
>> 
>>> On Apr 24, 2016, at 6:30 AM, Aaron Ballman via cfe-commits 
>>>  wrote:
>>> 
>>> Author: aaronballman
>>> Date: Sun Apr 24 08:30:21 2016
>>> New Revision: 267338
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=267338&view=rev
>>> Log:
>>> Improve diagnostic checking for va_start to also warn on other instances of 
>>> undefined behavior, such as a parameter declared with the register keyword 
>>> in C, or a parameter of a type that undergoes default argument promotion.
>>> 
>>> This helps cover some more of the CERT secure coding rule EXP58-CPP. Pass 
>>> an object of the correct type to va_start 
>>> (https://www.securecoding.cert.org/confluence/display/cplusplus/EXP58-CPP.+Pass+an+object+of+the+correct+type+to+va_start).
>>> 
>>> Added:
>>>   cfe/trunk/test/SemaCXX/varargs.cpp
>>> Removed:
>>>   cfe/trunk/test/Sema/varargs.cpp
>>> Modified:
>>>   cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>>   cfe/trunk/lib/Sema/SemaChecking.cpp
>>>   cfe/trunk/test/Sema/varargs-x86-64.c
>>>   cfe/trunk/test/Sema/varargs.c
>>> 
>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=267338&r1=267337&r2=267338&view=diff
>>> ==
>>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Apr 24 
>>> 08:30:21 2016
>>> @@ -7435,8 +7435,10 @@ def err_ms_va_start_used_in_sysv_functio
>>> def warn_second_arg_of_va_start_not_last_named_param : Warning<
>>>  "second argument to 'va_start' is not the last named parameter">,
>>>  InGroup;
>>> -def warn_va_start_of_reference_type_is_undefined : Warning<
>>> -  "'va_start' has undefined behavior with reference types">, 
>>> InGroup;
>>> +def warn_va_start_type_is_undefined : Warning<
>>> +  "passing %select{an object that undergoes default argument promotion|"
>>> +  "an object of reference type|a parameter declared with the 'register' "
>>> +  "keyword}0 to 'va_start' has undefined behavior">, InGroup;
>>> def err_first_argument_to_va_arg_not_of_type_va_list : Error<
>>>  "first argument to 'va_arg' is of type %0 and not 'va_list'">;
>>> def err_second_parameter_to_va_arg_incomplete: Error<
>>> 
>>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=267338&r1=267337&r2=267338&view=diff
>>> ==
>>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Sun Apr 24 08:30:21 2016
>>> @@ -2702,6 +2702,7 @@ bool Sema::SemaBuiltinVAStartImpl(CallEx
>>>  // block.
>>>  QualType Type;
>>>  SourceLocation ParamLoc;
>>> +  bool IsCRegister = false;
>>> 
>>>  if (const DeclRefExpr *DR = dyn_cast(Arg)) {
>>>if (const ParmVarDecl *PV = dyn_cast(DR->getDecl())) {
>>> @@ -2718,15 +2719,21 @@ bool Sema::SemaBuiltinVAStartImpl(CallEx
>>> 
>>>  Type = PV->getType();
>>>  ParamLoc = PV->getLocation();
>>> +  IsCRegister =
>>> +  PV->getStorageClass() == SC_Register && !g

Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files

2016-08-25 Thread Mads Ravn via cfe-commits
madsravn updated this revision to Diff 69250.
madsravn added a comment.

Last change - documentation should be fine now.


https://reviews.llvm.org/D20512

Files:
  clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tidy/llvm/HeaderGuardCheck.h
  clang-tidy/utils/HeaderFileExtensionsUtils.cpp
  clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tidy/utils/HeaderGuard.cpp
  clang-tidy/utils/HeaderGuard.h
  docs/clang-tidy/checks/llvm-header-guard.rst

Index: docs/clang-tidy/checks/llvm-header-guard.rst
===
--- docs/clang-tidy/checks/llvm-header-guard.rst
+++ docs/clang-tidy/checks/llvm-header-guard.rst
@@ -3,4 +3,11 @@
 llvm-header-guard
 =
 
-TODO: add docs
+Finds and fixes header guards that do not adhere to LLVM style.
+
+Options
+---
+
+.. option:: HeaderFileExtensions
+
+  A comma-separated list of filename extensions of header files (The filename extension should not contain "." prefix). Default value is ",h,hh,hpp,hxx".  For extension-less header files, using an empty string or leaving an empty string between "," if there are other filename extensions.
Index: clang-tidy/utils/HeaderGuard.h
===
--- clang-tidy/utils/HeaderGuard.h
+++ clang-tidy/utils/HeaderGuard.h
@@ -11,16 +11,28 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARD_H
 
 #include "../ClangTidy.h"
+#include "../utils/HeaderFileExtensionsUtils.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 
 /// Finds and fixes header guards.
+/// The check supports these options:
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
+/// header files (The filename extension should not contain "." prefix).
+/// ",h,hh,hpp,hxx" by default.
+/// For extension-less header files, using an empty string or leaving an
+/// empty string between "," if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+RawStringHeaderFileExtensions(
+Options.getLocalOrGlobal("HeaderFileExtensions", ",h,hh,hpp,hxx")) {
+utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+ HeaderFileExtensions, ',');
+  }
   void registerPPCallbacks(CompilerInstance &Compiler) override;
 
   /// \brief Returns true if the checker should suggest inserting a trailing
@@ -39,6 +51,10 @@
   /// \brief Get the canonical header guard for a file.
   virtual std::string getHeaderGuard(StringRef Filename,
  StringRef OldGuard = StringRef()) = 0;
+
+private:
+  std::string RawStringHeaderFileExtensions;
+  utils::HeaderFileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace utils
Index: clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tidy/utils/HeaderGuard.cpp
+++ clang-tidy/utils/HeaderGuard.cpp
@@ -20,7 +20,7 @@
 
 /// \brief canonicalize a path by removing ./ and ../ components.
 static std::string cleanPath(StringRef Path) {
-  SmallString<256> Result =  Path;
+  SmallString<256> Result = Path;
   llvm::sys::path::remove_dots(Result, true);
   return Result.str();
 }
@@ -274,13 +274,13 @@
 }
 
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 bool HeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) { return true; }
 
 bool HeaderGuardCheck::shouldSuggestToAddHeaderGuard(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 std::string HeaderGuardCheck::formatEndIf(StringRef HeaderGuard) {
Index: clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -12,8 +12,8 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 namespace tidy {
@@ -41,6 +41,10 @@
HeaderFileExtensionsSet &HeaderFileExtensions,
char delimiter);
 
+/// \brief Decides whether a file has a header file extension.
+bool isHeaderFileExtension(StringRef FileName,
+   HeaderFileExtensionsSet HeaderFileExtensions);
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tidy/utils/HeaderFileExtensionsUtils.cpp
===
--- clang-tidy/utils/HeaderFileExtensionsUtils.cpp
+++ clang-tid

[libcxx] r279744 - Add an _LIBCPP_NORETURN inline function named __throw_XXX for each exception type we define. They either construct and throw the exception, or abort() (if exceptions are disabled).

2016-08-25 Thread Marshall Clow via cfe-commits
Author: marshall
Date: Thu Aug 25 10:09:01 2016
New Revision: 279744

URL: http://llvm.org/viewvc/llvm-project?rev=279744&view=rev
Log:
Add an _LIBCPP_NORETURN inline function named __throw_XXX for each exception 
type we define. They either construct and throw the exception, or abort() (if 
exceptions are disabled). Use these functions everywhere instead of assert()ing 
when exceptions are disabled. WARNING: This is a behavior change - but only 
with exceptions disabled.  Reviewed as: https://reviews.llvm.org/D23855.

Modified:
libcxx/trunk/include/__functional_03
libcxx/trunk/include/__locale
libcxx/trunk/include/any
libcxx/trunk/include/array
libcxx/trunk/include/bitset
libcxx/trunk/include/complex
libcxx/trunk/include/deque
libcxx/trunk/include/experimental/any
libcxx/trunk/include/experimental/dynarray
libcxx/trunk/include/experimental/optional
libcxx/trunk/include/experimental/string_view
libcxx/trunk/include/fstream
libcxx/trunk/include/functional
libcxx/trunk/include/future
libcxx/trunk/include/locale
libcxx/trunk/include/memory
libcxx/trunk/include/new
libcxx/trunk/include/regex
libcxx/trunk/include/stdexcept
libcxx/trunk/include/string
libcxx/trunk/include/typeinfo
libcxx/trunk/include/vector
libcxx/trunk/src/debug.cpp
libcxx/trunk/src/experimental/memory_resource.cpp
libcxx/trunk/src/locale.cpp
libcxx/trunk/src/new.cpp
libcxx/trunk/src/string.cpp
libcxx/trunk/src/system_error.cpp
libcxx/trunk/src/thread.cpp
libcxx/trunk/src/typeinfo.cpp

Modified: libcxx/trunk/include/__functional_03
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__functional_03?rev=279744&r1=279743&r2=279744&view=diff
==
--- libcxx/trunk/include/__functional_03 (original)
+++ libcxx/trunk/include/__functional_03 Thu Aug 25 10:09:01 2016
@@ -679,10 +679,8 @@ template
 _Rp
 function<_Rp()>::operator()() const
 {
-#ifndef _LIBCPP_NO_EXCEPTIONS
 if (__f_ == 0)
-throw bad_function_call();
-#endif  // _LIBCPP_NO_EXCEPTIONS
+__throw_bad_function_call();
 return (*__f_)();
 }
 
@@ -955,10 +953,8 @@ template
 _Rp
 function<_Rp(_A0)>::operator()(_A0 __a0) const
 {
-#ifndef _LIBCPP_NO_EXCEPTIONS
 if (__f_ == 0)
-throw bad_function_call();
-#endif  // _LIBCPP_NO_EXCEPTIONS
+__throw_bad_function_call();
 return (*__f_)(__a0);
 }
 
@@ -1231,10 +1227,8 @@ template::operator()(_A0 __a0, _A1 __a1) const
 {
-#ifndef _LIBCPP_NO_EXCEPTIONS
 if (__f_ == 0)
-throw bad_function_call();
-#endif  // _LIBCPP_NO_EXCEPTIONS
+__throw_bad_function_call();
 return (*__f_)(__a0, __a1);
 }
 
@@ -1507,10 +1501,8 @@ template::operator()(_A0 __a0, _A1 __a1, _A2 __a2) const
 {
-#ifndef _LIBCPP_NO_EXCEPTIONS
 if (__f_ == 0)
-throw bad_function_call();
-#endif  // _LIBCPP_NO_EXCEPTIONS
+__throw_bad_function_call();
 return (*__f_)(__a0, __a1, __a2);
 }
 

Modified: libcxx/trunk/include/__locale
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__locale?rev=279744&r1=279743&r2=279744&view=diff
==
--- libcxx/trunk/include/__locale (original)
+++ libcxx/trunk/include/__locale Thu Aug 25 10:09:01 2016
@@ -165,10 +165,9 @@ template 
 locale
 locale::combine(const locale& __other) const
 {
-#ifndef _LIBCPP_NO_EXCEPTIONS
 if (!_VSTD::has_facet<_Facet>(__other))
-throw runtime_error("locale::combine: locale missing facet");
-#endif  // _LIBCPP_NO_EXCEPTIONS
+__throw_runtime_error("locale::combine: locale missing facet");
+
 return locale(*this, 
&const_cast<_Facet&>(_VSTD::use_facet<_Facet>(__other)));
 }
 
@@ -1184,8 +1183,6 @@ _LIBCPP_EXTERN_TEMPLATE2(class _LIBCPP_T
 _LIBCPP_EXTERN_TEMPLATE2(class _LIBCPP_TYPE_VIS codecvt_byname)
 _LIBCPP_EXTERN_TEMPLATE2(class _LIBCPP_TYPE_VIS codecvt_byname)
 
-_LIBCPP_FUNC_VIS void __throw_runtime_error(const char*);
-
 template 
 struct __narrow_to_utf8
 {

Modified: libcxx/trunk/include/any
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/any?rev=279744&r1=279743&r2=279744&view=diff
==
--- libcxx/trunk/include/any (original)
+++ libcxx/trunk/include/any Thu Aug 25 10:09:01 2016
@@ -85,7 +85,6 @@ namespace std {
 #include 
 #include 
 #include 
-#include 
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header

Modified: libcxx/trunk/include/array
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/array?rev=279744&r1=279743&r2=279744&view=diff
==
--- libcxx/trunk/include/array (original)
+++ libcxx/trunk/include/array Thu Aug 25 10:09:01 2016
@@ -108,9 +108,6 @@ template  c
 #include 
 #include 
 #include 
-#if defined(_LI

Re: r267338 - Improve diagnostic checking for va_start to also warn on other instances of undefined behavior, such as a parameter declared with the register keyword in C, or a parameter of a type that

2016-08-25 Thread Aaron Ballman via cfe-commits
On Thu, Aug 25, 2016 at 11:04 AM, Frédéric Riss  wrote:
>
>> On Aug 25, 2016, at 7:44 AM, Aaron Ballman  wrote:
>>
>> On Tue, Aug 23, 2016 at 9:32 PM, Frédéric Riss  wrote:
>>> Hey Aaron,
>>>
>>> This commit triggers warnings when the argument to va_start is an enum 
>>> type. The standard says:
>>>
>>> 7.16.1.4 The va_start macro
>>>
>>>• 4  The parameter parmN is the identifier of the rightmost 
>>> parameter in the variable parameter list in the function definition (the 
>>> one just before the , ...). If the parameter parmN is declared with the 
>>> register storage class, with a function or array type, or with a type that 
>>> is not compatible with the type that results after application of the 
>>> default argument promotions, the behavior is undefined.
>>>
>>> It seems to me that using an enum as the argument to va_start is not UB 
>>> when the implementation chose to use an int to store the enum.
>>
>> The implementation isn't required to choose int as the compatible type
>> for the enumeration. See 6.7.2.2p4: "Each enumerated type shall be
>> compatible with char, a signed integer type, or an unsigned integer
>> type." In the case where the compatible type is char, this use is UB.
>>
>>> Would you agree the warning as implemented gives false positives in that 
>>> case?
>>
>> It depends entirely on the enumeration and what compatible type is
>> chosen for it. If the compatible type is signed or unsigned int, then
>> the warning would be a false-positive and we should silence it. If the
>> compatible type is char, then the warning is not a false positive.
>
> I think we agree? If I’m not mistaken, in clang’s case, an int compatible 
> type will always be chosen except if -fshort-enums is passed and the enum 
> fits in a smaller type. Do you want a PR?

Yes, I think a PR is reasonable. There's also the packed attribute on
an enum which acts the same as passing -fshort-enums, and there is the
language extension for allowing enumerators larger than what fits into
an int.

~Aaron

>
> Fred
>
>> ~Aaron
>>
>>>
>>> Thanks,
>>> Fred
>>>
>>>
 On Apr 24, 2016, at 6:30 AM, Aaron Ballman via cfe-commits 
  wrote:

 Author: aaronballman
 Date: Sun Apr 24 08:30:21 2016
 New Revision: 267338

 URL: http://llvm.org/viewvc/llvm-project?rev=267338&view=rev
 Log:
 Improve diagnostic checking for va_start to also warn on other instances 
 of undefined behavior, such as a parameter declared with the register 
 keyword in C, or a parameter of a type that undergoes default argument 
 promotion.

 This helps cover some more of the CERT secure coding rule EXP58-CPP. Pass 
 an object of the correct type to va_start 
 (https://www.securecoding.cert.org/confluence/display/cplusplus/EXP58-CPP.+Pass+an+object+of+the+correct+type+to+va_start).

 Added:
   cfe/trunk/test/SemaCXX/varargs.cpp
 Removed:
   cfe/trunk/test/Sema/varargs.cpp
 Modified:
   cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
   cfe/trunk/lib/Sema/SemaChecking.cpp
   cfe/trunk/test/Sema/varargs-x86-64.c
   cfe/trunk/test/Sema/varargs.c

 Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
 URL: 
 http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=267338&r1=267337&r2=267338&view=diff
 ==
 --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
 +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Apr 24 
 08:30:21 2016
 @@ -7435,8 +7435,10 @@ def err_ms_va_start_used_in_sysv_functio
 def warn_second_arg_of_va_start_not_last_named_param : Warning<
  "second argument to 'va_start' is not the last named parameter">,
  InGroup;
 -def warn_va_start_of_reference_type_is_undefined : Warning<
 -  "'va_start' has undefined behavior with reference types">, 
 InGroup;
 +def warn_va_start_type_is_undefined : Warning<
 +  "passing %select{an object that undergoes default argument promotion|"
 +  "an object of reference type|a parameter declared with the 'register' "
 +  "keyword}0 to 'va_start' has undefined behavior">, InGroup;
 def err_first_argument_to_va_arg_not_of_type_va_list : Error<
  "first argument to 'va_arg' is of type %0 and not 'va_list'">;
 def err_second_parameter_to_va_arg_incomplete: Error<

 Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
 URL: 
 http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=267338&r1=267337&r2=267338&view=diff
 ==
 --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
 +++ cfe/trunk/lib/Sema/SemaChecking.cpp Sun Apr 24 08:30:21 2016
 @@ -2702,6 +2702,7 @@ bool Sema::SemaBuiltinVAStartImpl(CallEx
  // block.
  QualType Type;
  SourceLocation Pa

Re: [PATCH] D23855: Make exception-throwing from a noexcept build `abort()`.

2016-08-25 Thread Marshall Clow via cfe-commits
mclow.lists closed this revision.
mclow.lists added a comment.

Committed as revision 279744


https://reviews.llvm.org/D23855



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


Re: [PATCH] D23855: Make exception-throwing from a noexcept build `abort()`.

2016-08-25 Thread Asiri Rathnayake via cfe-commits
rmaprath added a subscriber: rmaprath.
rmaprath added a comment.

I don't know how I missed this. Thanks for the patch!!!


https://reviews.llvm.org/D23855



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


[PATCH] D23878: [libc++abi] Fix test under ASAN and MSAN

2016-08-25 Thread Shoaib Meenai via cfe-commits
smeenai created this revision.
smeenai added reviewers: compnerd, EricWF, rsmith.
smeenai added a subscriber: cfe-commits.

When we're running tests under ASAN or MSAN, they're compiled with -O1,
which enables tail call elimination. This causes backtrace_test to fail:
the compiler performs tail call elimination for call3_nothrow, but it
can't for call3_throw, leading to a mismatched frame count. Disable tail
call elimination (and inlining, just to be explicit) to avoid this.

Test Plan:
Configured locally with -DLLVM_USE_SANITIZER=Address and was able to
reproduce the test failure. The test passes with this change.

https://reviews.llvm.org/D23878

Files:
  test/backtrace_test.pass.cpp
  test/backtrace_test.sh.cpp

Index: test/backtrace_test.sh.cpp
===
--- test/backtrace_test.sh.cpp
+++ test/backtrace_test.sh.cpp
@@ -9,6 +9,11 @@
 
 // UNSUPPORTED: libcxxabi-no-exceptions
 
+// We're comparing frame counts, so suppress inlining and tail call elimination
+// to avoid mismatches caused by optimizations.
+// RUN: %build -fno-inline -fno-optimize-sibling-calls
+// RUN: %run
+
 #include 
 #include 
 #include 


Index: test/backtrace_test.sh.cpp
===
--- test/backtrace_test.sh.cpp
+++ test/backtrace_test.sh.cpp
@@ -9,6 +9,11 @@
 
 // UNSUPPORTED: libcxxabi-no-exceptions
 
+// We're comparing frame counts, so suppress inlining and tail call elimination
+// to avoid mismatches caused by optimizations.
+// RUN: %build -fno-inline -fno-optimize-sibling-calls
+// RUN: %run
+
 #include 
 #include 
 #include 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23858: Don't diagnose non-modular includes when we are not compiling a module

2016-08-25 Thread Ben Langmuir via cfe-commits
benlangmuir added a comment.

LGTM, but @rsmith should glance at this too.


https://reviews.llvm.org/D23858



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


Re: [PATCH] D22871: Fix incorrect -Wtautological-constant-out-of-range warnings with enums

2016-08-25 Thread Ismail Badawi via cfe-commits
ibadawi abandoned this revision.
ibadawi added a comment.

Abandoning for now -- might submit another patch later according to new 
discussion on https://llvm.org/bugs/show_bug.cgi?id=16154


https://reviews.llvm.org/D22871



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


[libcxx] r279746 - Remove duplicate inline

2016-08-25 Thread Marshall Clow via cfe-commits
Author: marshall
Date: Thu Aug 25 10:56:55 2016
New Revision: 279746

URL: http://llvm.org/viewvc/llvm-project?rev=279746&view=rev
Log:
Remove duplicate inline

Modified:
libcxx/trunk/include/experimental/any

Modified: libcxx/trunk/include/experimental/any
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/experimental/any?rev=279746&r1=279745&r2=279746&view=diff
==
--- libcxx/trunk/include/experimental/any (original)
+++ libcxx/trunk/include/experimental/any Thu Aug 25 10:56:55 2016
@@ -98,7 +98,7 @@ public:
 #if _LIBCPP_STD_VER > 11// C++ > 11
 
 _LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
-inline void __throw_bad_any_cast()
+void __throw_bad_any_cast()
 {
 #ifndef _LIBCPP_NO_EXCEPTIONS
 throw bad_any_cast();


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


Re: [PATCH] D23852: [SemaObjC] Fix crash while parsing type arguments and protocols

2016-08-25 Thread Doug Gregor via cfe-commits
doug.gregor added a comment.

This will work, but it's *really* unfortunate to put tentative parsing into 
this code path because tentative parsing is far from free, and specialized 
Objective-C types are getting pretty common in Objective-C headers. Why can't 
the callers be taught to handle EOF and bail out?


https://reviews.llvm.org/D23852



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


Re: r267338 - Improve diagnostic checking for va_start to also warn on other instances of undefined behavior, such as a parameter declared with the register keyword in C, or a parameter of a type that

2016-08-25 Thread Frédéric Riss via cfe-commits

> On Aug 25, 2016, at 8:17 AM, Aaron Ballman  wrote:
> 
> On Thu, Aug 25, 2016 at 11:04 AM, Frédéric Riss  wrote:
>> 
>>> On Aug 25, 2016, at 7:44 AM, Aaron Ballman  wrote:
>>> 
>>> On Tue, Aug 23, 2016 at 9:32 PM, Frédéric Riss  wrote:
 Hey Aaron,
 
 This commit triggers warnings when the argument to va_start is an enum 
 type. The standard says:
 
 7.16.1.4 The va_start macro
 
   • 4  The parameter parmN is the identifier of the rightmost 
 parameter in the variable parameter list in the function definition (the 
 one just before the , ...). If the parameter parmN is declared with the 
 register storage class, with a function or array type, or with a type that 
 is not compatible with the type that results after application of the 
 default argument promotions, the behavior is undefined.
 
 It seems to me that using an enum as the argument to va_start is not UB 
 when the implementation chose to use an int to store the enum.
>>> 
>>> The implementation isn't required to choose int as the compatible type
>>> for the enumeration. See 6.7.2.2p4: "Each enumerated type shall be
>>> compatible with char, a signed integer type, or an unsigned integer
>>> type." In the case where the compatible type is char, this use is UB.
>>> 
 Would you agree the warning as implemented gives false positives in that 
 case?
>>> 
>>> It depends entirely on the enumeration and what compatible type is
>>> chosen for it. If the compatible type is signed or unsigned int, then
>>> the warning would be a false-positive and we should silence it. If the
>>> compatible type is char, then the warning is not a false positive.
>> 
>> I think we agree? If I’m not mistaken, in clang’s case, an int compatible 
>> type will always be chosen except if -fshort-enums is passed and the enum 
>> fits in a smaller type. Do you want a PR?
> 
> Yes, I think a PR is reasonable. There's also the packed attribute on
> an enum which acts the same as passing -fshort-enums, and there is the
> language extension for allowing enumerators larger than what fits into
> an int.

PR29140

Thanks!
Fred

> ~Aaron
> 
>> 
>> Fred
>> 
>>> ~Aaron
>>> 
 
 Thanks,
 Fred
 
 
> On Apr 24, 2016, at 6:30 AM, Aaron Ballman via cfe-commits 
>  wrote:
> 
> Author: aaronballman
> Date: Sun Apr 24 08:30:21 2016
> New Revision: 267338
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=267338&view=rev
> Log:
> Improve diagnostic checking for va_start to also warn on other instances 
> of undefined behavior, such as a parameter declared with the register 
> keyword in C, or a parameter of a type that undergoes default argument 
> promotion.
> 
> This helps cover some more of the CERT secure coding rule EXP58-CPP. Pass 
> an object of the correct type to va_start 
> (https://www.securecoding.cert.org/confluence/display/cplusplus/EXP58-CPP.+Pass+an+object+of+the+correct+type+to+va_start).
> 
> Added:
>  cfe/trunk/test/SemaCXX/varargs.cpp
> Removed:
>  cfe/trunk/test/Sema/varargs.cpp
> Modified:
>  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>  cfe/trunk/lib/Sema/SemaChecking.cpp
>  cfe/trunk/test/Sema/varargs-x86-64.c
>  cfe/trunk/test/Sema/varargs.c
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=267338&r1=267337&r2=267338&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Apr 24 
> 08:30:21 2016
> @@ -7435,8 +7435,10 @@ def err_ms_va_start_used_in_sysv_functio
> def warn_second_arg_of_va_start_not_last_named_param : Warning<
> "second argument to 'va_start' is not the last named parameter">,
> InGroup;
> -def warn_va_start_of_reference_type_is_undefined : Warning<
> -  "'va_start' has undefined behavior with reference types">, 
> InGroup;
> +def warn_va_start_type_is_undefined : Warning<
> +  "passing %select{an object that undergoes default argument promotion|"
> +  "an object of reference type|a parameter declared with the 'register' "
> +  "keyword}0 to 'va_start' has undefined behavior">, InGroup;
> def err_first_argument_to_va_arg_not_of_type_va_list : Error<
> "first argument to 'va_arg' is of type %0 and not 'va_list'">;
> def err_second_parameter_to_va_arg_incomplete: Error<
> 
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=267338&r1=267337&r2=267338&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaChecking.cpp

r279754 - [Sema][Comments] Add support for TypeAliasTemplate

2016-08-25 Thread Bruno Cardoso Lopes via cfe-commits
Author: bruno
Date: Thu Aug 25 12:09:33 2016
New Revision: 279754

URL: http://llvm.org/viewvc/llvm-project?rev=279754&view=rev
Log:
[Sema][Comments] Add support for TypeAliasTemplate

Emit proper diagnostics when -Wdocumentation is used with constructs such as:

  template
  using fn = int(T aaa, int ccc);

Previously clang wouldn't recognize the function and complain with
'comment that is not attached to a function declaration'.

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

rdar://problem/27300695

Modified:
cfe/trunk/lib/AST/Comment.cpp
cfe/trunk/test/Sema/warn-documentation.cpp

Modified: cfe/trunk/lib/AST/Comment.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Comment.cpp?rev=279754&r1=279753&r2=279754&view=diff
==
--- cfe/trunk/lib/AST/Comment.cpp (original)
+++ cfe/trunk/lib/AST/Comment.cpp Thu Aug 25 12:09:33 2016
@@ -310,6 +310,20 @@ void DeclInfo::fill() {
 Kind = TypedefKind;
 TemplateKind = Template;
 TemplateParameters = TAT->getTemplateParameters();
+TypeAliasDecl *TAD = TAT->getTemplatedDecl();
+if (!TAD)
+  break;
+
+const TypeSourceInfo *TSI = TAD->getTypeSourceInfo();
+if (!TSI)
+  break;
+TypeLoc TL = TSI->getTypeLoc().getUnqualifiedLoc();
+FunctionTypeLoc FTL;
+if (getFunctionTypeLoc(TL, FTL)) {
+  Kind = FunctionKind;
+  ParamVars = FTL.getParams();
+  ReturnType = FTL.getReturnLoc().getType();
+}
 break;
   }
   case Decl::Enum:

Modified: cfe/trunk/test/Sema/warn-documentation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-documentation.cpp?rev=279754&r1=279753&r2=279754&view=diff
==
--- cfe/trunk/test/Sema/warn-documentation.cpp (original)
+++ cfe/trunk/test/Sema/warn-documentation.cpp Thu Aug 25 12:09:33 2016
@@ -416,6 +416,38 @@ using test_function_like_using7 = foo::f
 /// \returns aaa.
 using test_function_like_using8 = foo::function_wrapper &&;
 
+// expected-warning@+4 {{template parameter 'U' not found in the template 
declaration}} expected-note@+4 {{did you mean 'T'?}}
+// expected-warning@+2 {{parameter 'bbb' not found in the function 
declaration}} expected-note@+2 {{did you mean 'ccc'?}}
+/// \param aaa Meow.
+/// \param bbb Bbb.
+/// \tparam U Uuu.
+template
+using test_function_like_using9 = int(T aaa, int ccc);
+
+// expected-warning@+4 {{template parameter 'U' not found in the template 
declaration}} expected-note@+4 {{did you mean 'T'?}}
+// expected-warning@+2 {{parameter 'bbb' not found in the function 
declaration}} expected-note@+2 {{did you mean 'ccc'?}}
+/// \param aaa Meow.
+/// \param bbb Bbb.
+/// \tparam U Uuu.
+template
+using test_function_like_using10 = int (*)(T aaa, int ccc);
+
+// expected-warning@+4 {{template parameter 'U' not found in the template 
declaration}} expected-note@+4 {{did you mean 'T'?}}
+// expected-warning@+2 {{parameter 'bbb' not found in the function 
declaration}} expected-note@+2 {{did you mean 'ccc'?}}
+/// \param aaa Meow.
+/// \param bbb Bbb.
+/// \tparam U Uuu.
+template
+using test_function_like_using11 = foo::function_wrapper;
+
+// expected-warning@+4 {{template parameter 'U' not found in the template 
declaration}} expected-note@+4 {{did you mean 'T'?}}
+// expected-warning@+2 {{parameter 'bbb' not found in the function 
declaration}} expected-note@+2 {{did you mean 'ccc'?}}
+/// \param aaa Meow.
+/// \param bbb Bbb.
+/// \tparam U Uuu.
+template
+using test_function_like_using12 = foo::function_wrapper 
*;
+
 using test_not_function_like_using1 = int (*)(int aaa);
 
 // expected-warning@+1 {{'\param' command used in a comment that is not 
attached to a function declaration}}


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


Re: [PATCH] D23860: [Sema][Comments] Add support for TypeAliasTemplate

2016-08-25 Thread Bruno Cardoso Lopes via cfe-commits
bruno closed this revision.
bruno added a comment.

Applied your suggestions and committed r279754. Thanks!


https://reviews.llvm.org/D23860



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


Re: [PATCH] D23720: Omit column info for CodeView by default

2016-08-25 Thread Reid Kleckner via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


https://reviews.llvm.org/D23720



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


Re: [PATCH] D23651: [clang-rename] improve performance for rename-all

2016-08-25 Thread Miklos Vajna via cfe-commits
vmiklos added a comment.

> If I understood correctly, you and Alex are talking about different things.


Yes, sorry, the context of my above comment was the cl::opt instances in the 
tool itself, not the various parameters to the different actions called from 
the tool.


https://reviews.llvm.org/D23651



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


Re: [PATCH] D23878: [libc++abi] Fix test under ASAN and MSAN

2016-08-25 Thread Saleem Abdulrasool via cfe-commits
compnerd added a comment.

I think it would be better to use `assert(true)` to avoid the TCO from kicking 
in.


https://reviews.llvm.org/D23878



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


[libcxx] r279763 - Followon to r279744. Find the other exception types and make __throw_XXX routines (and call them). Remove the generic __libcpp_throw routine, since no one uses it anymore.

2016-08-25 Thread Marshall Clow via cfe-commits
Author: marshall
Date: Thu Aug 25 12:47:09 2016
New Revision: 279763

URL: http://llvm.org/viewvc/llvm-project?rev=279763&view=rev
Log:
Followon to r279744. Find the other exception types and make __throw_XXX 
routines (and call them).  Remove the generic __libcpp_throw routine, since no 
one uses it anymore.

Modified:
libcxx/trunk/include/any
libcxx/trunk/include/exception
libcxx/trunk/include/experimental/filesystem
libcxx/trunk/include/experimental/memory_resource
libcxx/trunk/include/experimental/string_view
libcxx/trunk/include/memory
libcxx/trunk/include/string_view
libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp
libcxx/trunk/src/experimental/filesystem/operations.cpp

Modified: libcxx/trunk/include/any
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/any?rev=279763&r1=279762&r2=279763&view=diff
==
--- libcxx/trunk/include/any (original)
+++ libcxx/trunk/include/any Thu Aug 25 12:47:09 2016
@@ -102,6 +102,16 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 #if _LIBCPP_STD_VER > 14
 
+_LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
+void __throw_bad_any_cast()
+{
+#ifndef _LIBCPP_NO_EXCEPTIONS
+throw bad_any_cast();
+#else
+   _VSTD::abort();
+#endif
+}
+
 // Forward declarations
 class _LIBCPP_TYPE_VIS_ONLY any;
 
@@ -579,7 +589,7 @@ _ValueType any_cast(any const & __v)
 using _Tp = add_const_t>;
 _Tp * __tmp = _VSTD::any_cast<_Tp>(&__v);
 if (__tmp == nullptr)
-__libcpp_throw(bad_any_cast());
+__throw_bad_any_cast();
 return *__tmp;
 }
 
@@ -594,7 +604,7 @@ _ValueType any_cast(any & __v)
 typedef typename remove_reference<_ValueType>::type _Tp;
 _Tp * __tmp = _VSTD::any_cast<_Tp>(&__v);
 if (__tmp == nullptr)
-__libcpp_throw(bad_any_cast());
+__throw_bad_any_cast();
 return *__tmp;
 }
 
@@ -614,7 +624,7 @@ _ValueType any_cast(any && __v)
 >;
 _Tp * __tmp = _VSTD::any_cast<_Tp>(&__v);
 if (__tmp == nullptr)
-__libcpp_throw(bad_any_cast());
+__throw_bad_any_cast();
 return _VSTD::forward<_ForwardTp>(*__tmp);
 }
 

Modified: libcxx/trunk/include/exception
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/exception?rev=279763&r1=279762&r2=279763&view=diff
==
--- libcxx/trunk/include/exception (original)
+++ libcxx/trunk/include/exception Thu Aug 25 12:47:09 2016
@@ -255,19 +255,4 @@ rethrow_if_nested(const _Ep&, typename e
 
 }  // std
 
-_LIBCPP_BEGIN_NAMESPACE_STD
-
-template 
-_LIBCPP_INLINE_VISIBILITY
-inline void __libcpp_throw(_Exception const& __e) {
-#ifndef _LIBCPP_NO_EXCEPTIONS
-throw __e;
-#else
-_VSTD::fprintf(stderr, "%s\n", __e.what());
-_VSTD::abort();
-#endif
-}
-
-_LIBCPP_END_NAMESPACE_STD
-
 #endif  // _LIBCPP_EXCEPTION

Modified: libcxx/trunk/include/experimental/filesystem
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/experimental/filesystem?rev=279763&r1=279762&r2=279763&view=diff
==
--- libcxx/trunk/include/experimental/filesystem (original)
+++ libcxx/trunk/include/experimental/filesystem Thu Aug 25 12:47:09 2016
@@ -1176,6 +1176,17 @@ private:
 shared_ptr<_Storage> __paths_;
 };
 
+template 
+_LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
+void __throw_filesystem_error(_Args && ...__args)
+{
+#ifndef _LIBCPP_NO_EXCEPTIONS
+throw filesystem_error(std::forward<_Args>(__args)...);
+#else
+_VSTD::abort();
+#endif
+}
+
 // operational functions
 
 _LIBCPP_FUNC_VIS

Modified: libcxx/trunk/include/experimental/memory_resource
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/experimental/memory_resource?rev=279763&r1=279762&r2=279763&view=diff
==
--- libcxx/trunk/include/experimental/memory_resource (original)
+++ libcxx/trunk/include/experimental/memory_resource Thu Aug 25 12:47:09 2016
@@ -182,9 +182,9 @@ public:
 _LIBCPP_INLINE_VISIBILITY
 _ValueType* allocate(size_t __n) {
 if (__n > max_size()) {
-__libcpp_throw(length_error(
+__throw_length_error(
 
"std::experimental::pmr::polymorphic_allocator::allocate(size_t n)"
-" 'n' exceeds maximum supported size"));
+" 'n' exceeds maximum supported size");
 }
 return static_cast<_ValueType*>(
 __res_->allocate(__n * sizeof(_ValueType), alignof(_ValueType))
@@ -383,9 +383,9 @@ protected:
 virtual void * do_allocate(size_t __bytes, size_t)
 {
 if (__bytes > __max_size()) {
-__libcpp_throw(length_error(
+__throw_length_error(
 
"std::experimental::pmr::resource_adaptor::do_allocate(size_t bytes, size_t 
align)"
-" 'bytes' exceeds maximum suppor

r279764 - [MS] Pass non-trivially-copyable objects indirectly on Windows ARM

2016-08-25 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Thu Aug 25 13:23:28 2016
New Revision: 279764

URL: http://llvm.org/viewvc/llvm-project?rev=279764&view=rev
Log:
[MS] Pass non-trivially-copyable objects indirectly on Windows ARM

This isn't exactly what MSVC does, unfortunately. MSVC does not pass
objects with destructors but no copy constructors by address. More ARM
expertise is required to really understand what should be done here.

Fixes PR29136.

Modified:
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp

Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=279764&r1=279763&r2=279764&view=diff
==
--- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Thu Aug 25 13:23:28 2016
@@ -794,6 +794,12 @@ MicrosoftCXXABI::getRecordArgABI(const C
 // FIXME: Implement for other architectures.
 return RAA_Default;
 
+  case llvm::Triple::thumb:
+// Use the simple Itanium rules for now.
+// FIXME: This is incompatible with MSVC for arguments with a dtor and no
+// copy ctor.
+return !canCopyArgument(RD) ? RAA_Indirect : RAA_Default;
+
   case llvm::Triple::x86:
 // All record arguments are passed in memory on x86.  Decide whether to
 // construct the object directly in argument memory, or to construct the

Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp?rev=279764&r1=279763&r2=279764&view=diff
==
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp Thu Aug 25 
13:23:28 2016
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -emit-llvm %s -o - -triple=i386-pc-linux | FileCheck 
-check-prefix LINUX %s
 // RUN: %clang_cc1 -emit-llvm %s -o - -triple=i386-pc-win32 
-mconstructor-aliases -fno-rtti | FileCheck -check-prefix WIN32 %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple=thumb-pc-win32 
-mconstructor-aliases -fno-rtti | FileCheck -check-prefix WOA %s
 // RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-pc-win32 
-mconstructor-aliases -fno-rtti | FileCheck -check-prefix WIN64 %s
 
 struct Empty {};
@@ -92,6 +93,9 @@ SmallWithCtor small_with_ctor_return() {
 // LINUX-LABEL: define void 
@_Z22small_with_ctor_returnv(%struct.SmallWithCtor* noalias sret %agg.result)
 // WIN32: define void 
@"\01?small_with_ctor_return@@YA?AUSmallWithCtor@@XZ"(%struct.SmallWithCtor* 
noalias sret %agg.result)
 // WIN64: define void 
@"\01?small_with_ctor_return@@YA?AUSmallWithCtor@@XZ"(%struct.SmallWithCtor* 
noalias sret %agg.result)
+// FIXME: The 'sret' mark here doesn't seem to be enough to convince LLVM to
+// preserve the hidden sret pointer in R0 across the function.
+// WOA: define arm_aapcs_vfpcc void 
@"\01?small_with_ctor_return@@YA?AUSmallWithCtor@@XZ"(%struct.SmallWithCtor* 
noalias sret %agg.result)
 
 SmallWithVftable small_with_vftable_return() { return SmallWithVftable(); }
 // LINUX-LABEL: define void 
@_Z25small_with_vftable_returnv(%struct.SmallWithVftable* noalias sret 
%agg.result)
@@ -102,6 +106,7 @@ MediumWithCopyCtor medium_with_copy_ctor
 // LINUX-LABEL: define void 
@_Z28medium_with_copy_ctor_returnv(%struct.MediumWithCopyCtor* noalias sret 
%agg.result)
 // WIN32: define void 
@"\01?medium_with_copy_ctor_return@@YA?AUMediumWithCopyCtor@@XZ"(%struct.MediumWithCopyCtor*
 noalias sret %agg.result)
 // WIN64: define void 
@"\01?medium_with_copy_ctor_return@@YA?AUMediumWithCopyCtor@@XZ"(%struct.MediumWithCopyCtor*
 noalias sret %agg.result)
+// WOA: define arm_aapcs_vfpcc void 
@"\01?medium_with_copy_ctor_return@@YA?AUMediumWithCopyCtor@@XZ"(%struct.MediumWithCopyCtor*
 noalias sret %agg.result)
 
 // Returning a large struct that doesn't fit into a register.
 Big big_return() { return Big(); }
@@ -114,22 +119,26 @@ void small_arg(Small s) {}
 // LINUX-LABEL: define void @_Z9small_arg5Small(i32 %s.0)
 // WIN32: define void @"\01?small_arg@@YAXUSmall@@@Z"(i32 %s.0)
 // WIN64: define void @"\01?small_arg@@YAXUSmall@@@Z"(i32 %s.coerce)
+// WOA: define arm_aapcs_vfpcc void @"\01?small_arg@@YAXUSmall@@@Z"([1 x i32] 
%s.coerce)
 
 void medium_arg(Medium s) {}
 // LINUX-LABEL: define void @_Z10medium_arg6Medium(i32 %s.0, i32 %s.1)
 // WIN32: define void @"\01?medium_arg@@YAXUMedium@@@Z"(i32 %s.0, i32 %s.1)
 // WIN64: define void @"\01?medium_arg@@YAXUMedium@@@Z"(i64 %s.coerce)
+// WOA: define arm_aapcs_vfpcc void @"\01?medium_arg@@YAXUMedium@@@Z"([2 x 
i32] %s.coerce)
 
 void small_arg_with_ctor(SmallWithCtor s) {}
 // LINUX-LABEL: define void 
@_Z19small_arg_with_ctor13SmallWithCtor(%struct.SmallWithCtor* byval align 4 %s)
 // WIN32: define void @"\01?small_arg_with_ctor@@YAXUSmallWithCtor@

r279765 - Omit column info for CodeView by default

2016-08-25 Thread Adrian McCarthy via cfe-commits
Author: amccarth
Date: Thu Aug 25 13:24:35 2016
New Revision: 279765

URL: http://llvm.org/viewvc/llvm-project?rev=279765&view=rev
Log:
Omit column info for CodeView by default

Clang tracks only start columns, not start-end ranges. CodeView allows for 
that, but the VS debugger doesn't handle anything less than a complete range 
well--it either highlights the wrong part of a statement or truncates source 
lines in the assembly view. It's better to have no column information at all.

So by default, we'll omit the column information for CodeView targeting Windows.

Since the column info is still useful for sanitizers, I've promoted 
-gcolumn-info (and -gno-column-info) to a CoreOption and added a couple tests 
to make sure that works for clang-cl.

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

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/test/Driver/cl-options.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=279765&r1=279764&r2=279765&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Aug 25 13:24:35 2016
@@ -1305,8 +1305,8 @@ def gno_record_gcc_switches : Flag<["-"]
   Group;
 def gstrict_dwarf : Flag<["-"], "gstrict-dwarf">, Group;
 def gno_strict_dwarf : Flag<["-"], "gno-strict-dwarf">, Group;
-def gcolumn_info : Flag<["-"], "gcolumn-info">, Group;
-def gno_column_info : Flag<["-"], "gno-column-info">, Group;
+def gcolumn_info : Flag<["-"], "gcolumn-info">, Group, 
Flags<[CoreOption]>;
+def gno_column_info : Flag<["-"], "gno-column-info">, Group, 
Flags<[CoreOption]>;
 def gsplit_dwarf : Flag<["-"], "gsplit-dwarf">, Group;
 def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group;
 def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group;

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=279765&r1=279764&r2=279765&view=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Thu Aug 25 13:24:35 2016
@@ -2574,7 +2574,7 @@ static void getTargetFeatures(const Tool
   case llvm::Triple::wasm32:
   case llvm::Triple::wasm64:
 getWebAssemblyTargetFeatures(Args, Features);
-break; 
+break;
   case llvm::Triple::sparc:
   case llvm::Triple::sparcel:
   case llvm::Triple::sparcv9:
@@ -4656,9 +4656,13 @@ void Clang::ConstructJob(Compilation &C,
   // We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now.
   Args.ClaimAllArgs(options::OPT_g_flags_Group);
 
-  // PS4 defaults to no column info
+  // Column info is included by default for everything except PS4 and CodeView.
+  // Clang doesn't track end columns, just starting columns, which, in theory,
+  // is fine for CodeView (and PDB).  In practice, however, the Microsoft
+  // debuggers don't handle missing end columns well, so it's better not to
+  // include any column info.
   if (Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
-   /*Default=*/ !IsPS4CPU))
+   /*Default=*/ !IsPS4CPU && !(IsWindowsMSVC && EmitCodeView)))
 CmdArgs.push_back("-dwarf-column-info");
 
   // FIXME: Move backend command line options to the module.
@@ -6712,7 +6716,7 @@ void ClangAs::ConstructJob(Compilation &
   case llvm::Triple::mips64el:
 AddMIPSTargetArgs(Args, CmdArgs);
 break;
-
+
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
 AddX86TargetArgs(Args, CmdArgs);

Modified: cfe/trunk/test/Driver/cl-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=279765&r1=279764&r2=279765&view=diff
==
--- cfe/trunk/test/Driver/cl-options.c (original)
+++ cfe/trunk/test/Driver/cl-options.c Thu Aug 25 13:24:35 2016
@@ -50,6 +50,15 @@
 // fpstrict-NOT: -menable-unsafe-fp-math
 // fpstrict-NOT: -ffast-math
 
+// RUN: %clang_cl /Z7 -gcolumn-info -### -- %s 2>&1 | FileCheck 
-check-prefix=gcolumn %s
+// gcolumn: -dwarf-column-info
+
+// RUN: %clang_cl /Z7 -gno-column-info -### -- %s 2>&1 | FileCheck 
-check-prefix=gnocolumn %s
+// gnocolumn-NOT: -dwarf-column-info
+
+// RUN: %clang_cl /Z7 -### -- %s 2>&1 | FileCheck -check-prefix=gdefcolumn %s
+// gdefcolumn-NOT: -dwarf-column-info
+
 // RUN: %clang_cl /GA -### -- %s 2>&1 | FileCheck -check-prefix=GA %s
 // GA: -ftls-model=local-exec
 


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


Re: [PATCH] D23720: Omit column info for CodeView by default

2016-08-25 Thread Adrian McCarthy via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL279765: Omit column info for CodeView by default (authored 
by amccarth).

Changed prior to commit:
  https://reviews.llvm.org/D23720?vs=68713&id=69274#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23720

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/Tools.cpp
  cfe/trunk/test/Driver/cl-options.c

Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1305,8 +1305,8 @@
   Group;
 def gstrict_dwarf : Flag<["-"], "gstrict-dwarf">, Group;
 def gno_strict_dwarf : Flag<["-"], "gno-strict-dwarf">, Group;
-def gcolumn_info : Flag<["-"], "gcolumn-info">, Group;
-def gno_column_info : Flag<["-"], "gno-column-info">, Group;
+def gcolumn_info : Flag<["-"], "gcolumn-info">, Group, 
Flags<[CoreOption]>;
+def gno_column_info : Flag<["-"], "gno-column-info">, Group, 
Flags<[CoreOption]>;
 def gsplit_dwarf : Flag<["-"], "gsplit-dwarf">, Group;
 def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group;
 def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group;
Index: cfe/trunk/test/Driver/cl-options.c
===
--- cfe/trunk/test/Driver/cl-options.c
+++ cfe/trunk/test/Driver/cl-options.c
@@ -50,6 +50,15 @@
 // fpstrict-NOT: -menable-unsafe-fp-math
 // fpstrict-NOT: -ffast-math
 
+// RUN: %clang_cl /Z7 -gcolumn-info -### -- %s 2>&1 | FileCheck 
-check-prefix=gcolumn %s
+// gcolumn: -dwarf-column-info
+
+// RUN: %clang_cl /Z7 -gno-column-info -### -- %s 2>&1 | FileCheck 
-check-prefix=gnocolumn %s
+// gnocolumn-NOT: -dwarf-column-info
+
+// RUN: %clang_cl /Z7 -### -- %s 2>&1 | FileCheck -check-prefix=gdefcolumn %s
+// gdefcolumn-NOT: -dwarf-column-info
+
 // RUN: %clang_cl /GA -### -- %s 2>&1 | FileCheck -check-prefix=GA %s
 // GA: -ftls-model=local-exec
 
Index: cfe/trunk/lib/Driver/Tools.cpp
===
--- cfe/trunk/lib/Driver/Tools.cpp
+++ cfe/trunk/lib/Driver/Tools.cpp
@@ -2574,7 +2574,7 @@
   case llvm::Triple::wasm32:
   case llvm::Triple::wasm64:
 getWebAssemblyTargetFeatures(Args, Features);
-break; 
+break;
   case llvm::Triple::sparc:
   case llvm::Triple::sparcel:
   case llvm::Triple::sparcv9:
@@ -4656,9 +4656,13 @@
   // We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now.
   Args.ClaimAllArgs(options::OPT_g_flags_Group);
 
-  // PS4 defaults to no column info
+  // Column info is included by default for everything except PS4 and CodeView.
+  // Clang doesn't track end columns, just starting columns, which, in theory,
+  // is fine for CodeView (and PDB).  In practice, however, the Microsoft
+  // debuggers don't handle missing end columns well, so it's better not to
+  // include any column info.
   if (Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
-   /*Default=*/ !IsPS4CPU))
+   /*Default=*/ !IsPS4CPU && !(IsWindowsMSVC && EmitCodeView)))
 CmdArgs.push_back("-dwarf-column-info");
 
   // FIXME: Move backend command line options to the module.
@@ -6712,7 +6716,7 @@
   case llvm::Triple::mips64el:
 AddMIPSTargetArgs(Args, CmdArgs);
 break;
-
+
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
 AddX86TargetArgs(Args, CmdArgs);


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1305,8 +1305,8 @@
   Group;
 def gstrict_dwarf : Flag<["-"], "gstrict-dwarf">, Group;
 def gno_strict_dwarf : Flag<["-"], "gno-strict-dwarf">, Group;
-def gcolumn_info : Flag<["-"], "gcolumn-info">, Group;
-def gno_column_info : Flag<["-"], "gno-column-info">, Group;
+def gcolumn_info : Flag<["-"], "gcolumn-info">, Group, Flags<[CoreOption]>;
+def gno_column_info : Flag<["-"], "gno-column-info">, Group, Flags<[CoreOption]>;
 def gsplit_dwarf : Flag<["-"], "gsplit-dwarf">, Group;
 def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group;
 def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group;
Index: cfe/trunk/test/Driver/cl-options.c
===
--- cfe/trunk/test/Driver/cl-options.c
+++ cfe/trunk/test/Driver/cl-options.c
@@ -50,6 +50,15 @@
 // fpstrict-NOT: -menable-unsafe-fp-math
 // fpstrict-NOT: -ffast-math
 
+// RUN: %clang_cl /Z7 -gcolumn-info -### -- %s 2>&1 | FileCheck -check-prefix=gcolumn %s
+// gcolumn: -dwarf-column-info
+
+// RUN: %clang_cl /Z7 -gno-column-info -### -- %s 2>&1 | FileCheck -check-prefix=gnocolumn %s
+// gnocolumn-NOT: -dwarf-column-info
+
+// RUN: %clang_cl /Z7 -### -- %s 2>&1 | FileCheck -check-prefix=gdefcolumn %s
+// gdefcolumn-NOT: -dwarf-column-info
+
 // RUN: %clang_cl /GA -### -- %s 2>&

Re: [PATCH] D23878: [libc++abi] Fix test under ASAN and MSAN

2016-08-25 Thread Shoaib Meenai via cfe-commits
smeenai updated this revision to Diff 69275.
smeenai added a comment.

Using attributes instead of compile flags, per compnerd's suggestion


https://reviews.llvm.org/D23878

Files:
  test/backtrace_test.pass.cpp

Index: test/backtrace_test.pass.cpp
===
--- test/backtrace_test.pass.cpp
+++ test/backtrace_test.pass.cpp
@@ -21,26 +21,30 @@
   return _URC_NO_REASON;
 }
 
+__attribute__ ((__noinline__))
 void call3_throw(size_t* ntraced) {
   try {
 _Unwind_Backtrace(trace_function, ntraced);
   } catch (...) {
 assert(false);
   }
 }
 
+__attribute__ ((__noinline__, __disable_tail_calls__))
 void call3_nothrow(size_t* ntraced) {
   _Unwind_Backtrace(trace_function, ntraced);
 }
 
+__attribute__ ((__noinline__, __disable_tail_calls__))
 void call2(size_t* ntraced, bool do_throw) {
   if (do_throw) {
 call3_throw(ntraced);
   } else {
 call3_nothrow(ntraced);
   }
 }
 
+__attribute__ ((__noinline__, __disable_tail_calls__))
 void call1(size_t* ntraced, bool do_throw) {
   call2(ntraced, do_throw);
 }


Index: test/backtrace_test.pass.cpp
===
--- test/backtrace_test.pass.cpp
+++ test/backtrace_test.pass.cpp
@@ -21,26 +21,30 @@
   return _URC_NO_REASON;
 }
 
+__attribute__ ((__noinline__))
 void call3_throw(size_t* ntraced) {
   try {
 _Unwind_Backtrace(trace_function, ntraced);
   } catch (...) {
 assert(false);
   }
 }
 
+__attribute__ ((__noinline__, __disable_tail_calls__))
 void call3_nothrow(size_t* ntraced) {
   _Unwind_Backtrace(trace_function, ntraced);
 }
 
+__attribute__ ((__noinline__, __disable_tail_calls__))
 void call2(size_t* ntraced, bool do_throw) {
   if (do_throw) {
 call3_throw(ntraced);
   } else {
 call3_nothrow(ntraced);
   }
 }
 
+__attribute__ ((__noinline__, __disable_tail_calls__))
 void call1(size_t* ntraced, bool do_throw) {
   call2(ntraced, do_throw);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r279766 - Refactor to remove the assumption that we know the name of the module we're emitting at the point when we create a PCHGenerator (with the C++ modules TS, we find that out part way through pa

2016-08-25 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Thu Aug 25 13:26:30 2016
New Revision: 279766

URL: http://llvm.org/viewvc/llvm-project?rev=279766&view=rev
Log:
Refactor to remove the assumption that we know the name of the module we're 
emitting at the point when we create a PCHGenerator (with the C++ modules TS, 
we find that out part way through parsing the input).

Modified:
cfe/trunk/include/clang/Serialization/ASTWriter.h
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp
cfe/trunk/lib/Frontend/FrontendActions.cpp
cfe/trunk/lib/Serialization/GeneratePCH.cpp

Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=279766&r1=279765&r2=279766&view=diff
==
--- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTWriter.h Thu Aug 25 13:26:30 2016
@@ -909,7 +909,6 @@ public:
 class PCHGenerator : public SemaConsumer {
   const Preprocessor &PP;
   std::string OutputFile;
-  clang::Module *Module;
   std::string isysroot;
   Sema *SemaPtr;
   std::shared_ptr Buffer;
@@ -925,7 +924,7 @@ protected:
 public:
   PCHGenerator(
 const Preprocessor &PP, StringRef OutputFile,
-clang::Module *Module, StringRef isysroot,
+StringRef isysroot,
 std::shared_ptr Buffer,
 ArrayRef> Extensions,
 bool AllowASTWithErrors = false,

Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=279766&r1=279765&r2=279766&view=diff
==
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Thu Aug 25 13:26:30 2016
@@ -925,7 +925,7 @@ public:
   PrecompilePreambleConsumer(ASTUnit &Unit, PrecompilePreambleAction *Action,
  const Preprocessor &PP, StringRef isysroot,
  std::unique_ptr Out)
-  : PCHGenerator(PP, "", nullptr, isysroot, std::make_shared(),
+  : PCHGenerator(PP, "", isysroot, std::make_shared(),
  ArrayRef>(),
  /*AllowASTWithErrors=*/true),
 Unit(Unit), Hash(Unit.getCurrentTopLevelHashValue()), Action(Action),

Modified: cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp?rev=279766&r1=279765&r2=279766&view=diff
==
--- cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp (original)
+++ cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp Thu Aug 25 13:26:30 2016
@@ -161,7 +161,7 @@ IntrusiveRefCntPtr c
 auto Buffer = std::make_shared();
 ArrayRef> Extensions;
 auto consumer = llvm::make_unique(
-Clang->getPreprocessor(), "-", nullptr, /*isysroot=*/"", Buffer,
+Clang->getPreprocessor(), "-", /*isysroot=*/"", Buffer,
 Extensions, /*AllowASTWithErrors=*/true);
 Clang->getASTContext().setASTMutationListener(
 
consumer->GetASTMutationListener());

Modified: cfe/trunk/lib/Frontend/FrontendActions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendActions.cpp?rev=279766&r1=279765&r2=279766&view=diff
==
--- cfe/trunk/lib/Frontend/FrontendActions.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendActions.cpp Thu Aug 25 13:26:30 2016
@@ -91,7 +91,7 @@ GeneratePCHAction::CreateASTConsumer(Com
   auto Buffer = std::make_shared();
   std::vector> Consumers;
   Consumers.push_back(llvm::make_unique(
-CI.getPreprocessor(), OutputFile, nullptr, Sysroot,
+CI.getPreprocessor(), OutputFile, Sysroot,
 Buffer, CI.getFrontendOpts().ModuleFileExtensions,
 /*AllowASTWithErrors*/false,
 /*IncludeTimestamps*/
@@ -141,7 +141,7 @@ GenerateModuleAction::CreateASTConsumer(
   std::vector> Consumers;
 
   Consumers.push_back(llvm::make_unique(
-CI.getPreprocessor(), OutputFile, Module, Sysroot,
+CI.getPreprocessor(), OutputFile, Sysroot,
 Buffer, CI.getFrontendOpts().ModuleFileExtensions,
 /*AllowASTWithErrors=*/false,
 /*IncludeTimestamps=*/

Modified: cfe/trunk/lib/Serialization/GeneratePCH.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/GeneratePCH.cpp?rev=279766&r1=279765&r2=279766&view=diff
==
--- cfe/trunk/lib/Serialization/GeneratePCH.cpp (original)
+++ cfe/trunk/lib/Serialization/GeneratePCH.cpp Thu Aug 25 13:26:30 2

Re: [PATCH] D23848: Add a clang-tidy Visual Studio extension

2016-08-25 Thread Eugene Zelenko via cfe-commits
Eugene.Zelenko added a comment.

I think will be good idea to mention this plugin in  docs/ReleaseNotes.rst.


https://reviews.llvm.org/D23848



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


Re: [PATCH] D23848: Add a clang-tidy Visual Studio extension

2016-08-25 Thread Zachary Turner via cfe-commits
zturner added a comment.

In https://reviews.llvm.org/D23848#525485, @Eugene.Zelenko wrote:

> I think will be good idea to mention this plugin in  docs/ReleaseNotes.rst.


I was planning to wait until it's "finished".  This patch will give you a user 
interface for editing .clang-tidy files, but it still won't provide any means 
of *running* clang-tidy on the currently active source file, project, or 
solution.

I had planned to add those features incrementally to make the reviews more 
manageable.  What do you guys think?


https://reviews.llvm.org/D23848



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


Re: [PATCH] D23816: clang-cl: Add support for /FC: absolute paths in diagnostics and __FILE__

2016-08-25 Thread Nico Weber via cfe-commits
thakis added inline comments.


Comment at: include/clang/Driver/CLCompatOptions.td:68
@@ +67,3 @@
+def _SLASH_FC : CLFlag<"FC">,
+  HelpText<"Use absolute paths in diagnostics and __FILE__">,
+  Alias;

Hm, I'm not sure this should affect __FILE__. It sounds people want this for 
diagnostics, but making it affect __FILE__ makes reproducible builds harder. 
(There's -ffile-macro-prefix-to-remove from http://reviews.llvm.org/D17741 for 
that too, but that's not in.)


https://reviews.llvm.org/D23816



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


Re: [PATCH] D23816: clang-cl: Add support for /FC: absolute paths in diagnostics and __FILE__

2016-08-25 Thread Hans Wennborg via cfe-commits
hans added inline comments.


Comment at: include/clang/Driver/CLCompatOptions.td:68
@@ +67,3 @@
+def _SLASH_FC : CLFlag<"FC">,
+  HelpText<"Use absolute paths in diagnostics and __FILE__">,
+  Alias;

thakis wrote:
> Hm, I'm not sure this should affect __FILE__. It sounds people want this for 
> diagnostics, but making it affect __FILE__ makes reproducible builds harder. 
> (There's -ffile-macro-prefix-to-remove from http://reviews.llvm.org/D17741 
> for that too, but that's not in.)
The MSVC docs say it applies to __FILE__ though 
https://msdn.microsoft.com/en-us/library/027c4t2s.aspx and I worry that 
implementing only half of it might surprise users.

Chromium currently builds with MSVC and /FC, so we should already have full 
paths in __FILE__ there.


https://reviews.llvm.org/D23816



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


Re: [PATCH] D23848: Add a clang-tidy Visual Studio extension

2016-08-25 Thread Zachary Turner via cfe-commits
zturner added inline comments.


Comment at: clang-tidy-vs/ClangTidy/CheckTree.cs:67
@@ +66,3 @@
+if (ParentPath == null)
+return Name_;
+return ParentPath + "-" + Name_;

amccarth wrote:
> This seems overly complicated so I assume I'm missing some nuance.  Why not:
> 
> if (Parent_ == null)
> return Name_;
> string ParentPath = Parent_.Path;
> 
> Is there a case where Parent_.Path could return null besides the one that you 
> created?
> 
> 
If `Parent_ == null`, that is the root note, or default node.  It doesn't 
represent any particualr check, it's just the node at the top of the tree 
containing all the branches.  So we have to return `null` in that case.  We 
could probably return `Name_` as it would probably be equal to `null`, but it 
seems more clear to just return a constant.

Even if `Parent_` is not `null` though, `Parent_.Path` could equal `null`.  
This would happen for any node immediately under the root, because the parent 
would be the root, and by the previous explanation, `Parent_.Path` would then 
return `null`.  So, for top-level check nodes we just want to return the name, 
because there is nothing before it to prepend a `-` to.

For deeper nodes though, we want to get the path from the parent, then join it 
with a `-`.  


https://reviews.llvm.org/D23848



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


Re: [PATCH] D23816: clang-cl: Add support for /FC: absolute paths in diagnostics and __FILE__

2016-08-25 Thread Nico Weber via cfe-commits
thakis added inline comments.


Comment at: include/clang/Driver/CLCompatOptions.td:68
@@ +67,3 @@
+def _SLASH_FC : CLFlag<"FC">,
+  HelpText<"Use absolute paths in diagnostics and __FILE__">,
+  Alias;

hans wrote:
> thakis wrote:
> > Hm, I'm not sure this should affect __FILE__. It sounds people want this 
> > for diagnostics, but making it affect __FILE__ makes reproducible builds 
> > harder. (There's -ffile-macro-prefix-to-remove from 
> > http://reviews.llvm.org/D17741 for that too, but that's not in.)
> The MSVC docs say it applies to __FILE__ though 
> https://msdn.microsoft.com/en-us/library/027c4t2s.aspx and I worry that 
> implementing only half of it might surprise users.
> 
> Chromium currently builds with MSVC and /FC, so we should already have full 
> paths in __FILE__ there.
Sure, but the MSVC build is not deterministic in several ways that folks aren't 
happy about, and we're supposed to improve that :-)

As far as I know __FILE__ defaults to just the file's basename in cl.exe (is 
that true?). So without /FC __FILE__ is useless there and people more or less 
have to pass /FC. With clang, we emit the full relative path to the file, so 
it's not useless already.

For diagnostics, I can kind of see how full paths are useful: You likely 
develop on the same machine as you see the diagnostics on, so people want real 
absolute paths so they can click on them in MSVC. For __FILE__, that's less 
convincing, since that gets embedded into the binary that's shipped elsewhere.


https://reviews.llvm.org/D23816



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


Re: [PATCH] D23816: clang-cl: Add support for /FC: absolute paths in diagnostics and __FILE__

2016-08-25 Thread Hans Wennborg via cfe-commits
hans added inline comments.


Comment at: include/clang/Driver/CLCompatOptions.td:68
@@ +67,3 @@
+def _SLASH_FC : CLFlag<"FC">,
+  HelpText<"Use absolute paths in diagnostics and __FILE__">,
+  Alias;

thakis wrote:
> hans wrote:
> > thakis wrote:
> > > Hm, I'm not sure this should affect __FILE__. It sounds people want this 
> > > for diagnostics, but making it affect __FILE__ makes reproducible builds 
> > > harder. (There's -ffile-macro-prefix-to-remove from 
> > > http://reviews.llvm.org/D17741 for that too, but that's not in.)
> > The MSVC docs say it applies to __FILE__ though 
> > https://msdn.microsoft.com/en-us/library/027c4t2s.aspx and I worry that 
> > implementing only half of it might surprise users.
> > 
> > Chromium currently builds with MSVC and /FC, so we should already have full 
> > paths in __FILE__ there.
> Sure, but the MSVC build is not deterministic in several ways that folks 
> aren't happy about, and we're supposed to improve that :-)
> 
> As far as I know __FILE__ defaults to just the file's basename in cl.exe (is 
> that true?). So without /FC __FILE__ is useless there and people more or less 
> have to pass /FC. With clang, we emit the full relative path to the file, so 
> it's not useless already.
> 
> For diagnostics, I can kind of see how full paths are useful: You likely 
> develop on the same machine as you see the diagnostics on, so people want 
> real absolute paths so they can click on them in MSVC. For __FILE__, that's 
> less convincing, since that gets embedded into the binary that's shipped 
> elsewhere.
> As far as I know FILE defaults to just the file's basename in cl.exe (is that 
> true?).

In my testing, MSVC does include the full relative path in __FILE__ by default.

I agree that diagnostics seems to be the main motivation here. I'm just worried 
that if we implement /FC but leave out the FILE part, that's confusing for 
whoever might rely on it.

Maybe we should just do an -fabs-path-diagnostics option instead?


https://reviews.llvm.org/D23816



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


Re: [PATCH] D23816: clang-cl: Add support for /FC: absolute paths in diagnostics and __FILE__

2016-08-25 Thread Nico Weber via cfe-commits
thakis added inline comments.


Comment at: include/clang/Driver/CLCompatOptions.td:68
@@ +67,3 @@
+def _SLASH_FC : CLFlag<"FC">,
+  HelpText<"Use absolute paths in diagnostics and __FILE__">,
+  Alias;

hans wrote:
> thakis wrote:
> > hans wrote:
> > > thakis wrote:
> > > > Hm, I'm not sure this should affect __FILE__. It sounds people want 
> > > > this for diagnostics, but making it affect __FILE__ makes reproducible 
> > > > builds harder. (There's -ffile-macro-prefix-to-remove from 
> > > > http://reviews.llvm.org/D17741 for that too, but that's not in.)
> > > The MSVC docs say it applies to __FILE__ though 
> > > https://msdn.microsoft.com/en-us/library/027c4t2s.aspx and I worry that 
> > > implementing only half of it might surprise users.
> > > 
> > > Chromium currently builds with MSVC and /FC, so we should already have 
> > > full paths in __FILE__ there.
> > Sure, but the MSVC build is not deterministic in several ways that folks 
> > aren't happy about, and we're supposed to improve that :-)
> > 
> > As far as I know __FILE__ defaults to just the file's basename in cl.exe 
> > (is that true?). So without /FC __FILE__ is useless there and people more 
> > or less have to pass /FC. With clang, we emit the full relative path to the 
> > file, so it's not useless already.
> > 
> > For diagnostics, I can kind of see how full paths are useful: You likely 
> > develop on the same machine as you see the diagnostics on, so people want 
> > real absolute paths so they can click on them in MSVC. For __FILE__, that's 
> > less convincing, since that gets embedded into the binary that's shipped 
> > elsewhere.
> > As far as I know FILE defaults to just the file's basename in cl.exe (is 
> > that true?).
> 
> In my testing, MSVC does include the full relative path in __FILE__ by 
> default.
> 
> I agree that diagnostics seems to be the main motivation here. I'm just 
> worried that if we implement /FC but leave out the FILE part, that's 
> confusing for whoever might rely on it.
> 
> Maybe we should just do an -fabs-path-diagnostics option instead?
Works for me :-)


https://reviews.llvm.org/D23816



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


r279772 - Fix clang-offload-bundler.c test on Windows

2016-08-25 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Thu Aug 25 15:40:23 2016
New Revision: 279772

URL: http://llvm.org/viewvc/llvm-project?rev=279772&view=rev
Log:
Fix clang-offload-bundler.c test on Windows

Modified:
cfe/trunk/test/Driver/clang-offload-bundler.c

Modified: cfe/trunk/test/Driver/clang-offload-bundler.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang-offload-bundler.c?rev=279772&r1=279771&r2=279772&view=diff
==
--- cfe/trunk/test/Driver/clang-offload-bundler.c (original)
+++ cfe/trunk/test/Driver/clang-offload-bundler.c Thu Aug 25 15:40:23 2016
@@ -232,7 +232,7 @@
 // CK-OBJ-CMD: private constant [1 x i8] zeroinitializer, section 
"__CLANG_OFFLOAD_BUNDLE__host-powerpc64le-ibm-linux-gnu"
 // CK-OBJ-CMD: private constant [25 x i8] c"Content of device file 1{{.+}}", 
section "__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu"
 // CK-OBJ-CMD: private constant [25 x i8] c"Content of device file 2{{.+}}", 
section "__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu"
-// CK-OBJ-CMD: clang" "-r" "-target" "powerpc64le-ibm-linux-gnu" "-o" 
"{{.+}}.o" "{{.+}}.o" "{{.+}}.bc" "-nostdlib"
+// CK-OBJ-CMD: clang{{(.exe)?}}" "-r" "-target" "powerpc64le-ibm-linux-gnu" 
"-o" "{{.+}}.o" "{{.+}}.o" "{{.+}}.bc" "-nostdlib"
 
 // RUN: clang-offload-bundler -type=o 
-targets=host-powerpc64le-ibm-linux-gnu,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
 -outputs=%t.res.o,%t.res.tgt1,%t.res.tgt2 -inputs=%s.o -unbundle
 // RUN: diff %s.o %t.res.o


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


Re: r279632 - clang-offload-bundler - offload files bundling/unbundling tool

2016-08-25 Thread Reid Kleckner via cfe-commits
This test failed on Windows because clang is called "clang.exe" not
"clang". Fixed in r279772.

On Wed, Aug 24, 2016 at 8:21 AM, Samuel Antao via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: sfantao
> Date: Wed Aug 24 10:21:05 2016
> New Revision: 279632
>
> URL: http://llvm.org/viewvc/llvm-project?rev=279632&view=rev
> Log:
> clang-offload-bundler - offload files bundling/unbundling tool
>
> Summary:
> One of the goals of programming models that support offloading (e.g.
> OpenMP) is to enable users to offload with little effort, by annotating the
> code with a few pragmas. I'd also like to save users the trouble of
> changing their existent applications' build system. So having the compiler
> always return a single file instead of one for the host and each target
> even if the user is doing separate compilation is desirable.
>
> This diff proposes a tool named clang-offload-bundler (happy to change the
> name if required) that is used to bundle files associated with the same
> user source file but different targets, or to unbundle a file into separate
> files associated with different targets.
>
> This tool supports the driver support for OpenMP under review in
> http://reviews.llvm.org/D9888. The tool is used there to enable separate
> compilation, so that the very first action on input files that are not
> source files is a "unbundling action" and the very last non-linking action
> is a "bundling action".
>
> The format of the bundled files is currently very simple: text formats are
> concatenated with comments that have a magic string and target identifying
> triple in between, and binary formats have a header that contains the
> triple and the offset and size of the code for host and each target.
>
> The goal is to improve this tool in the future to deal with archive files
> so that each individual file in the archive is properly dealt with. We see
> that archives are very commonly used in current applications to combine
> separate compilation results. So I'm convinced users would enjoy this
> feature.
>
> This tool can be used like this:
>
> `clang-offload-bundler -targets=triple1,triple2 -type=ii
> -inputs=a.triple1.ii,a.triple2.ii -outputs=a.ii`
>
> or
>
> `clang-offload-bundler -targets=triple1,triple2 -type=ii
> -outputs=a.triple1.ii,a.triple2.ii -inputs=a.ii -unbundle`
>
> I implemented the tool under clang/tools. Please let me know if something
> like this should live somewhere else.
>
> This patch is prerequisite for http://reviews.llvm.org/D9888.
>
> Reviewers: hfinkel, rsmith, echristo, chandlerc, tra, jlebar, ABataev,
> Hahnfeld
>
> Subscribers: whchung, caomhin, andreybokhanko, arpith-jacob,
> carlo.bertolli, mehdi_amini, guansong, Hahnfeld, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D13909
>
> Added:
> cfe/trunk/test/Driver/clang-offload-bundler.c
> cfe/trunk/tools/clang-offload-bundler/
> cfe/trunk/tools/clang-offload-bundler/CMakeLists.txt
> cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
> Modified:
> cfe/trunk/test/CMakeLists.txt
> cfe/trunk/tools/CMakeLists.txt
>
> Modified: cfe/trunk/test/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> CMakeLists.txt?rev=279632&r1=279631&r2=279632&view=diff
> 
> ==
> --- cfe/trunk/test/CMakeLists.txt (original)
> +++ cfe/trunk/test/CMakeLists.txt Wed Aug 24 10:21:05 2016
> @@ -29,6 +29,7 @@ list(APPEND CLANG_TEST_DEPS
>clang-format
>c-index-test diagtool
>clang-tblgen
> +  clang-offload-bundler
>)
>
>  if(CLANG_ENABLE_STATIC_ANALYZER)
>
> Added: cfe/trunk/test/Driver/clang-offload-bundler.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/
> clang-offload-bundler.c?rev=279632&view=auto
> 
> ==
> --- cfe/trunk/test/Driver/clang-offload-bundler.c (added)
> +++ cfe/trunk/test/Driver/clang-offload-bundler.c Wed Aug 24 10:21:05 2016
> @@ -0,0 +1,222 @@
> +//
> +// Generate all the types of files we can bundle.
> +//
> +// RUN: %clang -O0 -target powerpc64le-ibm-linux-gnu %s -E -o %t.i
> +// RUN: %clangxx -O0 -target powerpc64le-ibm-linux-gnu -x c++ %s -E -o
> %t.ii
> +// RUN: %clang -O0 -target powerpc64le-ibm-linux-gnu %s -S -emit-llvm -o
> %t.ll
> +// RUN: %clang -O0 -target powerpc64le-ibm-linux-gnu %s -c -emit-llvm -o
> %t.bc
> +// RUN: %clang -O0 -target powerpc64le-ibm-linux-gnu %s -S -o %t.s
> +// RUN: %clang -O0 -target powerpc64le-ibm-linux-gnu %s -emit-ast -o
> %t.ast
> +
> +//
> +// Generate an empty file to help with the checks of empty files.
> +//
> +// RUN: touch %t.empty
> +
> +//
> +// Generate a couple of files to bundle with.
> +//
> +// RUN: echo 'Content of device file 1' > %t.tgt1
> +// RUN: echo 'Content of device file 2' > %t.tgt2
> +
> +//
> +// Check help message.
> +//
> +// RUN: clang-offload-bundler --help | FileCheck %s --check-prefix CK-HELP
>

r279774 - [MS] Win64 va_arg should expect large arguments to be passed indirectly

2016-08-25 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Thu Aug 25 15:42:26 2016
New Revision: 279774

URL: http://llvm.org/viewvc/llvm-project?rev=279774&view=rev
Log:
[MS] Win64 va_arg should expect large arguments to be passed indirectly

Fixes PR20569

Modified:
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/test/CodeGen/ms_abi.c

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=279774&r1=279773&r2=279774&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Thu Aug 25 15:42:26 2016
@@ -3651,7 +3651,17 @@ void WinX86_64ABIInfo::computeInfo(CGFun
 
 Address WinX86_64ABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
 QualType Ty) const {
-  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*indirect*/ false,
+
+  bool IsIndirect = false;
+
+  // MS x64 ABI requirement: "Any argument that doesn't fit in 8 bytes, or is
+  // not 1, 2, 4, or 8 bytes, must be passed by reference."
+  if (isAggregateTypeForABI(Ty) || Ty->isMemberPointerType()) {
+uint64_t Width = getContext().getTypeSize(Ty);
+IsIndirect = Width > 64 || !llvm::isPowerOf2_64(Width);
+  }
+
+  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, IsIndirect,
   CGF.getContext().getTypeInfoInChars(Ty),
   CharUnits::fromQuantity(8),
   /*allowHigherAlign*/ false);

Modified: cfe/trunk/test/CodeGen/ms_abi.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms_abi.c?rev=279774&r1=279773&r2=279774&view=diff
==
--- cfe/trunk/test/CodeGen/ms_abi.c (original)
+++ cfe/trunk/test/CodeGen/ms_abi.c Thu Aug 25 15:42:26 2016
@@ -45,24 +45,27 @@ void __attribute__((ms_abi)) f4(int a, .
   // WIN64-NEXT: %[[AP_NEXT:.*]] = getelementptr inbounds i8, i8* %[[AP_CUR]], 
i64 8
   // WIN64-NEXT: store i8* %[[AP_NEXT]], i8** %[[AP]]
   // WIN64-NEXT: bitcast i8* %[[AP_CUR]] to i32*
+  // FIXME: These are different now. We probably need __builtin_ms_va_arg.
   double _Complex c = __builtin_va_arg(ap, double _Complex);
   // FREEBSD: %[[AP_CUR2:.*]] = load i8*, i8** %[[AP]]
   // FREEBSD-NEXT: %[[AP_NEXT2:.*]] = getelementptr inbounds i8, i8* 
%[[AP_CUR2]], i64 16
   // FREEBSD-NEXT: store i8* %[[AP_NEXT2]], i8** %[[AP]]
   // FREEBSD-NEXT: bitcast i8* %[[AP_CUR2]] to { double, double }*
   // WIN64: %[[AP_CUR2:.*]] = load i8*, i8** %[[AP]]
-  // WIN64-NEXT: %[[AP_NEXT2:.*]] = getelementptr inbounds i8, i8* 
%[[AP_CUR2]], i64 16
+  // WIN64-NEXT: %[[AP_NEXT2:.*]] = getelementptr inbounds i8, i8* 
%[[AP_CUR2]], i64 8
   // WIN64-NEXT: store i8* %[[AP_NEXT2]], i8** %[[AP]]
-  // WIN64-NEXT: bitcast i8* %[[AP_CUR2]] to { double, double }*
+  // WIN64-NEXT: %[[CUR2:.*]] = bitcast i8* %[[AP_CUR2]] to { double, double 
}**
+  // WIN64-NEXT: load { double, double }*, { double, double }** %[[CUR2]]
   struct foo d = __builtin_va_arg(ap, struct foo);
   // FREEBSD: %[[AP_CUR3:.*]] = load i8*, i8** %[[AP]]
   // FREEBSD-NEXT: %[[AP_NEXT3:.*]] = getelementptr inbounds i8, i8* 
%[[AP_CUR3]], i64 16
   // FREEBSD-NEXT: store i8* %[[AP_NEXT3]], i8** %[[AP]]
   // FREEBSD-NEXT: bitcast i8* %[[AP_CUR3]] to %[[STRUCT_FOO]]*
   // WIN64: %[[AP_CUR3:.*]] = load i8*, i8** %[[AP]]
-  // WIN64-NEXT: %[[AP_NEXT3:.*]] = getelementptr inbounds i8, i8* 
%[[AP_CUR3]], i64 16
+  // WIN64-NEXT: %[[AP_NEXT3:.*]] = getelementptr inbounds i8, i8* 
%[[AP_CUR3]], i64 8
   // WIN64-NEXT: store i8* %[[AP_NEXT3]], i8** %[[AP]]
-  // WIN64-NEXT: bitcast i8* %[[AP_CUR3]] to %[[STRUCT_FOO]]*
+  // WIN64-NEXT: %[[CUR3:.*]] = bitcast i8* %[[AP_CUR3]] to %[[STRUCT_FOO]]*
+  // WIN64-NEXT: load %[[STRUCT_FOO]]*, %[[STRUCT_FOO]]** %[[CUR3]]
   __builtin_ms_va_list ap2;
   __builtin_ms_va_copy(ap2, ap);
   // FREEBSD: %[[AP_VAL:.*]] = load i8*, i8** %[[AP]]
@@ -88,12 +91,12 @@ void f5(int a, ...) {
   // WIN64-NEXT: bitcast i8* %[[AP_CUR]] to i32*
   double _Complex c = __builtin_va_arg(ap, double _Complex);
   // WIN64: %[[AP_CUR2:.*]] = load i8*, i8** %[[AP]]
-  // WIN64-NEXT: %[[AP_NEXT2:.*]] = getelementptr inbounds i8, i8* 
%[[AP_CUR2]], i64 16
+  // WIN64-NEXT: %[[AP_NEXT2:.*]] = getelementptr inbounds i8, i8* 
%[[AP_CUR2]], i64 8
   // WIN64-NEXT: store i8* %[[AP_NEXT2]], i8** %[[AP]]
   // WIN64-NEXT: bitcast i8* %[[AP_CUR2]] to { double, double }*
   struct foo d = __builtin_va_arg(ap, struct foo);
   // WIN64: %[[AP_CUR3:.*]] = load i8*, i8** %[[AP]]
-  // WIN64-NEXT: %[[AP_NEXT3:.*]] = getelementptr inbounds i8, i8* 
%[[AP_CUR3]], i64 16
+  // WIN64-NEXT: %[[AP_NEXT3:.*]] = getelementptr inbounds i8, i8* 
%[[AP_CUR3]], i64 8
   // WIN64-NEXT: store i8* %[[AP_NEXT3]], i8** %[[AP]]
   // WIN64-NEXT: bitcast i8* %[[AP_CUR3]] to %[[STRUCT_FOO]]*
   __builtin_va_list ap2;
@@ -124,7 +127,7 @@ void __attribute__((sysv_abi)) f6(__bui

Re: [PATCH] D23858: Don't diagnose non-modular includes when we are not compiling a module

2016-08-25 Thread Richard Smith via cfe-commits
rsmith added a comment.

It seems to me like this is papering over an issue rather than fixing it.

`diagnoseHeaderInclusion` calls `isHeaderInUmbrellaDirs` here, which is 
presumably failing to determine that `Nonmodular/A.h` is in the umbrella 
directory for the `Nonmodular` module. My first guess would be that this code 
from `ModuleMap::findHeaderInUmbrellaDirs` may be responsible:

  // Note: as an egregious but useful hack we use the real path here, because
  // frameworks moving from top-level frameworks to embedded frameworks tend
  // to be symlinked from the top-level location to the embedded location,
  // and we need to resolve lookups as if we had found the embedded location.
  StringRef DirName = SourceMgr.getFileManager().getCanonicalName(Dir);



Comment at: lib/Lex/PPDirectives.cpp:749-750
@@ -748,3 +748,4 @@
   Module *RequestingModule = getModuleForLocation(FilenameLoc);
-  bool RequestingModuleIsModuleInterface = 
!SourceMgr.isInMainFile(FilenameLoc);
+  bool RequestingModuleIsModuleInterface =
+  !SourceMgr.isInMainFile(FilenameLoc) && getLangOpts().CompilingModule;
 

I don't see why this should depend on whether we're compiling a module. If 
we're asked to diagnose non-modular #includes in modular headers, why should it 
matter whether we entered that header textually or by triggering precompilation 
of the corresponding module?


https://reviews.llvm.org/D23858



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


Re: [PATCH] D23871: [Modules] Add 'freestanding' to the 'requires-declaration' feature-list.

2016-08-25 Thread Richard Smith via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Makes sense to me. I imagine this would also be useful for libc++.



Comment at: lib/Headers/module.modulemap:66
@@ -65,2 +65,3 @@
 explicit module mm_malloc {
+  requires !freestanding
   header "mm_malloc.h"

Are there more parts of the intrinsics modules to which this should be applied?


https://reviews.llvm.org/D23871



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


Re: [PATCH] D23848: Add a clang-tidy Visual Studio extension

2016-08-25 Thread Adrian McCarthy via cfe-commits
amccarth added inline comments.


Comment at: clang-tidy-vs/ClangTidy/CheckTree.cs:68
@@ +67,3 @@
+return Name_;
+return ParentPath + "-" + Name_;
+}

OK, cool.  I assumed that the root had a name.  Thus a node with no parent (a 
root) could return its name and the rest of the children would each append '-' 
+ Name_.  But if roots are nameless, then your solution is fine.


https://reviews.llvm.org/D23848



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


Re: [PATCH] D23858: Don't diagnose non-modular includes when we are not compiling a module

2016-08-25 Thread Manman Ren via cfe-commits
manmanren added a comment.

In https://reviews.llvm.org/D23858#525647, @rsmith wrote:

> It seems to me like this is papering over an issue rather than fixing it.


I guess that is why we introduced fmodule-implementation-of, to work around 
issues like "relative includes to a VFS-mapped module". Ben should know more 
about the background.

> `diagnoseHeaderInclusion` calls `isHeaderInUmbrellaDirs` here, which is 
> presumably failing to determine that `Nonmodular/A.h` is in the umbrella 
> directory for the `Nonmodular` module.


Bruno and I looked at this together yesterday. With vfsoverlay, the header 
files in the module map are using the real path, while the umbrella directories 
in the module map are using the virtual path. We have issue figuring out the 
header file from the real path is in the umbrella directory. We have longer 
term goals on fixing up this. But this is a regression on our side and we are 
hoping to get back to our previous behavior.

Cheers,
Manman



Comment at: lib/Lex/PPDirectives.cpp:749-750
@@ -748,3 +748,4 @@
   Module *RequestingModule = getModuleForLocation(FilenameLoc);
-  bool RequestingModuleIsModuleInterface = 
!SourceMgr.isInMainFile(FilenameLoc);
+  bool RequestingModuleIsModuleInterface =
+  !SourceMgr.isInMainFile(FilenameLoc) && getLangOpts().CompilingModule;
 

rsmith wrote:
> I don't see why this should depend on whether we're compiling a module. If 
> we're asked to diagnose non-modular #includes in modular headers, why should 
> it matter whether we entered that header textually or by triggering 
> precompilation of the corresponding module?
I agree that here, we are actually including a non modular header from a 
modular header. But is it okay to not diagnose when we have specified 
fmodule-name and we are building the implementation of it? We should have 
diagnosed this when building the unit.


https://reviews.llvm.org/D23858



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


r279786 - Widen type of __offset_flags in RTTI on Mingw64

2016-08-25 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Thu Aug 25 17:16:30 2016
New Revision: 279786

URL: http://llvm.org/viewvc/llvm-project?rev=279786&view=rev
Log:
Widen type of __offset_flags in RTTI on Mingw64

Otherwise we can't handle secondary base classes at offsets greater than
2**24. This agrees with libstdc++abi.

We could extend this change to other LLP64 platforms, but then we would
want to update libc++abi and it would require additional review.

Fixes PR29116

Added:
cfe/trunk/test/CodeGenCXX/rtti-mingw64.cpp
Modified:
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp

Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=279786&r1=279785&r2=279786&view=diff
==
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Thu Aug 25 17:16:30 2016
@@ -3217,9 +3217,6 @@ void ItaniumRTTIBuilder::BuildVMIClassTy
   if (!RD->getNumBases())
 return;
 
-  llvm::Type *LongLTy =
-CGM.getTypes().ConvertType(CGM.getContext().LongTy);
-
   // Now add the base class descriptions.
 
   // Itanium C++ ABI 2.9.5p6c:
@@ -3237,6 +3234,19 @@ void ItaniumRTTIBuilder::BuildVMIClassTy
   //   __offset_shift = 8
   // };
   //   };
+
+  // If we're in mingw and 'long' isn't wide enough for a pointer, use 'long
+  // long' instead of 'long' for __offset_flags. libstdc++abi uses long long on
+  // LLP64 platforms.
+  // FIXME: Consider updating libc++abi to match, and extend this logic to all
+  // LLP64 platforms.
+  QualType OffsetFlagsTy = CGM.getContext().LongTy;
+  const TargetInfo &TI = CGM.getContext().getTargetInfo();
+  if (TI.getTriple().isOSCygMing() && TI.getPointerWidth(0) > 
TI.getLongWidth())
+OffsetFlagsTy = CGM.getContext().LongLongTy;
+  llvm::Type *OffsetFlagsLTy =
+  CGM.getTypes().ConvertType(OffsetFlagsTy);
+
   for (const auto &Base : RD->bases()) {
 // The __base_type member points to the RTTI for the base type.
 Fields.push_back(ItaniumRTTIBuilder(CXXABI).BuildTypeInfo(Base.getType()));
@@ -3268,7 +3278,7 @@ void ItaniumRTTIBuilder::BuildVMIClassTy
 if (Base.getAccessSpecifier() == AS_public)
   OffsetFlags |= BCTI_Public;
 
-Fields.push_back(llvm::ConstantInt::get(LongLTy, OffsetFlags));
+Fields.push_back(llvm::ConstantInt::get(OffsetFlagsLTy, OffsetFlags));
   }
 }
 

Added: cfe/trunk/test/CodeGenCXX/rtti-mingw64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/rtti-mingw64.cpp?rev=279786&view=auto
==
--- cfe/trunk/test/CodeGenCXX/rtti-mingw64.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/rtti-mingw64.cpp Thu Aug 25 17:16:30 2016
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu %s -emit-llvm -o - | FileCheck %s
+struct A { int a; };
+struct B : virtual A { int b; };
+B b;
+
+// CHECK: @_ZTI1B = linkonce_odr constant { i8*, i8*, i32, i32, i8*, i64 }
+// CHECK-SAME:  i8* bitcast (i8** getelementptr inbounds (i8*, i8** 
@_ZTVN10__cxxabiv121__vmi_class_type_infoE, i64 2) to i8*),
+// CHECK-SAME:  i8* getelementptr inbounds ([3 x i8], [3 x i8]* @_ZTS1B, i32 
0, i32 0),
+// CHECK-SAME:  i32 0,
+// CHECK-SAME:  i32 1,
+// CHECK-SAME:  i8* bitcast ({ i8*, i8* }* @_ZTI1A to i8*),
+//This i64 is important, it should be an i64, not an i32.
+// CHECK-SAME:  i64 -6141 }, comdat


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


Re: [PATCH] D23816: Add support for -fdiagnostics-abs-path: printing absolute paths in diagnostics

2016-08-25 Thread Hans Wennborg via cfe-commits
hans retitled this revision from "clang-cl: Add support for /FC: absolute paths 
in diagnostics and __FILE__" to "Add support for -fdiagnostics-abs-path: 
printing absolute paths in diagnostics".
hans updated the summary for this revision.
hans updated this revision to Diff 69300.
hans added a comment.

Just doing -fdiagnostics-abs-path.

Please take another look.


https://reviews.llvm.org/D23816

Files:
  include/clang/Basic/DiagnosticOptions.def
  include/clang/Driver/CLCompatOptions.td
  include/clang/Driver/Options.td
  include/clang/Frontend/TextDiagnostic.h
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/TextDiagnostic.cpp
  test/Driver/cl-options.c
  test/Frontend/Inputs/absolute-path.h
  test/Frontend/absolute-path.c

Index: test/Frontend/absolute-path.c
===
--- /dev/null
+++ test/Frontend/absolute-path.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -I %S/Inputs/SystemHeaderPrefix/.. %s 2>&1 | FileCheck -check-prefix=NORMAL -check-prefix=CHECK %s
+// RUN: %clang_cc1 -fsyntax-only -I %S/Inputs/SystemHeaderPrefix/.. -fdiagnostics-abs-path %s 2>&1 | FileCheck -check-prefix=ABSOLUTE -check-prefix=CHECK %s
+
+#include "absolute-path.h"
+
+// Check whether the diagnostic from the header above includes the dummy
+// directory in the path.
+
+// NORMAL: SystemHeaderPrefix
+// ABSOLUTE-NOT: SystemHeaderPrefix
+
+// CHECK: warning: control reaches end of non-void function
Index: test/Frontend/Inputs/absolute-path.h
===
--- /dev/null
+++ test/Frontend/Inputs/absolute-path.h
@@ -0,0 +1,3 @@
+int f() {
+  // Oops, no return.
+}
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -339,7 +339,6 @@
 // RUN: /FAs \
 // RUN: /FAu \
 // RUN: /favor:blend \
-// RUN: /FC \
 // RUN: /Fifoo \
 // RUN: /Fmfoo \
 // RUN: /FpDebug\main.pch \
@@ -491,6 +490,7 @@
 // RUN: -fdiagnostics-color \
 // RUN: -fno-diagnostics-color \
 // RUN: -fdiagnostics-parseable-fixits \
+// RUN: -fdiagnostics-abs-path \
 // RUN: -ferror-limit=10 \
 // RUN: -fmsc-version=1800 \
 // RUN: -fno-strict-aliasing \
Index: lib/Frontend/TextDiagnostic.cpp
===
--- lib/Frontend/TextDiagnostic.cpp
+++ lib/Frontend/TextDiagnostic.cpp
@@ -18,6 +18,7 @@
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Locale.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 
@@ -763,6 +764,20 @@
   OS << '\n';
 }
 
+void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM) {
+  SmallVector AbsoluteFilename;
+  if (DiagOpts->AbsolutePath) {
+AbsoluteFilename.insert(AbsoluteFilename.begin(), Filename.begin(),
+Filename.end());
+SM.getFileManager().makeAbsolutePath(AbsoluteFilename);
+llvm::sys::path::remove_dots(AbsoluteFilename, true);
+llvm::sys::path::native(AbsoluteFilename);
+Filename = StringRef(AbsoluteFilename.data(), AbsoluteFilename.size());
+  }
+
+  OS << Filename;
+}
+
 /// \brief Print out the file/line/column information and include trace.
 ///
 /// This method handlen the emission of the diagnostic location information.
@@ -779,7 +794,7 @@
 if (FID.isValid()) {
   const FileEntry* FE = SM.getFileEntryForID(FID);
   if (FE && FE->isValid()) {
-OS << FE->getName();
+emitFilename(FE->getName(), SM);
 if (FE->isInPCH())
   OS << " (in PCH)";
 OS << ": ";
@@ -795,7 +810,7 @@
   if (DiagOpts->ShowColors)
 OS.changeColor(savedColor, true);
 
-  OS << PLoc.getFilename();
+  emitFilename(PLoc.getFilename(), SM);
   switch (DiagOpts->getFormat()) {
   case DiagnosticOptions::Clang: OS << ':'  << LineNo; break;
   case DiagnosticOptions::MSVC:  OS << '('  << LineNo; break;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -945,6 +945,7 @@
  /*Default=*/true);
   Opts.ShowFixits = !Args.hasArg(OPT_fno_diagnostics_fixit_info);
   Opts.ShowLocation = !Args.hasArg(OPT_fno_show_source_location);
+  Opts.AbsolutePath = Args.hasArg(OPT_fdiagnostics_abs_path);
   Opts.ShowOptionNames = Args.hasArg(OPT_fdiagnostics_show_option);
 
   llvm::sys::Process::UseANSIEscapeCodes(Args.hasArg(OPT_fansi_escape_codes));
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -5914,6 +5914,9 @@
 options::OPT_fno_show_source_location))
 CmdArgs.push_back("-fno-show-source-location");
 
+  if (Args.hasArg(options

Re: [PATCH] D23816: Add support for -fdiagnostics-abs-path: printing absolute paths in diagnostics

2016-08-25 Thread Nico Weber via cfe-commits
thakis accepted this revision.
thakis added a comment.
This revision is now accepted and ready to land.

I like this approach.


https://reviews.llvm.org/D23816



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


[PATCH] D23894: [Clang-tidy] Fix some checks documentation style

2016-08-25 Thread Eugene Zelenko via cfe-commits
Eugene.Zelenko created this revision.
Eugene.Zelenko added a reviewer: alexfh.
Eugene.Zelenko added a subscriber: cfe-commits.
Eugene.Zelenko set the repository for this revision to rL LLVM.
Herald added a subscriber: nemanjai.

Repository:
  rL LLVM

https://reviews.llvm.org/D23894

Files:
  docs/clang-tidy/checks/cert-err34-c.rst
  docs/clang-tidy/checks/cppcoreguidelines-slicing.rst
  docs/clang-tidy/checks/google-build-using-namespace.rst
  docs/clang-tidy/checks/google-runtime-member-string-references.rst
  docs/clang-tidy/checks/misc-bool-pointer-implicit-conversion.rst
  docs/clang-tidy/checks/misc-fold-init-type.rst
  docs/clang-tidy/checks/misc-forward-declaration-namespace.rst
  docs/clang-tidy/checks/misc-misplaced-const.rst
  docs/clang-tidy/checks/misc-move-const-arg.rst
  docs/clang-tidy/checks/misc-multiple-statement-macro.rst
  docs/clang-tidy/checks/misc-pointer-and-integral-operation.rst
  docs/clang-tidy/checks/misc-redundant-expression.rst
  docs/clang-tidy/checks/misc-sizeof-container.rst
  docs/clang-tidy/checks/misc-sizeof-expression.rst
  docs/clang-tidy/checks/misc-string-integer-assignment.rst
  docs/clang-tidy/checks/misc-string-literal-with-embedded-nul.rst
  docs/clang-tidy/checks/misc-suspicious-string-compare.rst
  docs/clang-tidy/checks/misc-uniqueptr-reset-release.rst
  docs/clang-tidy/checks/misc-unused-raii.rst
  docs/clang-tidy/checks/misc-unused-using-decls.rst
  docs/clang-tidy/checks/modernize-avoid-bind.rst
  docs/clang-tidy/checks/modernize-use-using.rst
  docs/clang-tidy/checks/performance-implicit-cast-in-loop.rst
  docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
  docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst
  docs/clang-tidy/checks/readability-deleted-default.rst
  docs/clang-tidy/checks/readability-implicit-bool-cast.rst
  docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
  docs/clang-tidy/checks/readability-redundant-string-init.rst
  
docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst

Index: docs/clang-tidy/checks/misc-multiple-statement-macro.rst
===
--- docs/clang-tidy/checks/misc-multiple-statement-macro.rst
+++ docs/clang-tidy/checks/misc-multiple-statement-macro.rst
@@ -3,14 +3,14 @@
 misc-multiple-statement-macro
 =
 
-Detect multiple statement macros that are used in unbraced conditionals.
-Only the first statement of the macro will be inside the conditional and the other ones will be executed unconditionally.
+Detect multiple statement macros that are used in unbraced conditionals. Only
+the first statement of the macro will be inside the conditional and the other
+ones will be executed unconditionally.
 
 Example:
 
-.. code:: c++
+.. code-block:: c++
 
   #define INCREMENT_TWO(x, y) (x)++; (y)++
   if (do_increment)
-INCREMENT_TWO(a, b);  // `(b)++;` will be executed unconditionally.
-
+INCREMENT_TWO(a, b);  // (b)++ will be executed unconditionally.
Index: docs/clang-tidy/checks/misc-redundant-expression.rst
===
--- docs/clang-tidy/checks/misc-redundant-expression.rst
+++ docs/clang-tidy/checks/misc-redundant-expression.rst
@@ -6,14 +6,18 @@
 Detect redundant expressions which are typically errors due to copy-paste.
 
 Depending on the operator expressions may be
-  * redundant,
-  * always be `true`,
-  * always be `false`,
-  * always be a constant (zero or one)
+
+- redundant,
+
+- always be ``true``,
+
+- always be ``false``,
+
+- always be a constant (zero or one).
 
 Example:
 
-.. code:: c++
+.. code-block:: c++
 
   ((x+1) | (x+1)) // (x+1) is redundant
   (p->x == p->x)  // always true
Index: docs/clang-tidy/checks/modernize-avoid-bind.rst
===
--- docs/clang-tidy/checks/modernize-avoid-bind.rst
+++ docs/clang-tidy/checks/modernize-avoid-bind.rst
@@ -10,22 +10,22 @@
 
 Given:
 
-.. code:: c++
+.. code-block:: c++
 
   int add(int x, int y) { return x + y; }
 
 Then:
 
-.. code:: c++
+.. code-block:: c++
 
   void f() {
 int x = 2;
 auto clj = std::bind(add, x, _1);
   }
 
 is replaced by:
 
-.. code:: c++
+.. code-block:: c++
 
   void f() {
 int x = 2;
@@ -35,4 +35,3 @@
 ``std::bind`` can be hard to read and can result in larger object files and
 binaries due to type information that will not be produced by equivalent
 lambdas.
-
Index: docs/clang-tidy/checks/misc-fold-init-type.rst
===
--- docs/clang-tidy/checks/misc-fold-init-type.rst
+++ docs/clang-tidy/checks/misc-fold-init-type.rst
@@ -14,14 +14,14 @@
   initial value, so trucation wil happen at every application of ``operator+``
   and the result will be `0`, which might not be what the user expected.
 
-.. code:: c++
+.. code-block:: c++
 
   auto a = {0.5f,

Re: [PATCH] D23816: Add support for -fdiagnostics-abs-path: printing absolute paths in diagnostics

2016-08-25 Thread Richard Smith via cfe-commits
rsmith added inline comments.


Comment at: include/clang/Driver/Options.td:994
@@ -993,1 +993,3 @@
   Flags<[CC1Option]>, HelpText<"Do not include source location information 
with diagnostics">;
+def fdiagnostics_abs_path : Flag<["-"], "fdiagnostics-abs-path">, 
Group,
+  Flags<[CC1Option, CoreOption]>, HelpText<"Print absolute paths in 
diagnostics">;

How about `-fdiagnostics-[use-]absolute-paths` rather than 
`-fdiagnostics-abs-path` to avoid any confusion about this referring to the 
`abs` function?


Comment at: lib/Frontend/TextDiagnostic.cpp:773
@@ +772,3 @@
+SM.getFileManager().makeAbsolutePath(AbsoluteFilename);
+llvm::sys::path::remove_dots(AbsoluteFilename, true);
+llvm::sys::path::native(AbsoluteFilename);

Err, setting the second parameter to `true` here causes it to generate 
incorrect paths. I wouldn't do that... If you want to remove `..` components, 
you should use `FileManager::getCanonicalName` instead, which will do the 
appropriate symlink resolution here.


https://reviews.llvm.org/D23816



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


Re: [PATCH] D23858: Don't diagnose non-modular includes when we are not compiling a module

2016-08-25 Thread Richard Smith via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

OK, if this unblocks you and you're working towards properly handling umbrella 
headers in the VFS, I'm happy to go ahead with this.

Please either rename the variable or move that part of the check into 
`diagnoseHeaderInclusion`, though -- with this change, the variable means 
something quite different from its name.



Comment at: lib/Lex/PPDirectives.cpp:749-750
@@ -748,3 +748,4 @@
   Module *RequestingModule = getModuleForLocation(FilenameLoc);
-  bool RequestingModuleIsModuleInterface = 
!SourceMgr.isInMainFile(FilenameLoc);
+  bool RequestingModuleIsModuleInterface =
+  !SourceMgr.isInMainFile(FilenameLoc) && getLangOpts().CompilingModule;
 

manmanren wrote:
> rsmith wrote:
> > I don't see why this should depend on whether we're compiling a module. If 
> > we're asked to diagnose non-modular #includes in modular headers, why 
> > should it matter whether we entered that header textually or by triggering 
> > precompilation of the corresponding module?
> I agree that here, we are actually including a non modular header from a 
> modular header. But is it okay to not diagnose when we have specified 
> fmodule-name and we are building the implementation of it? We should have 
> diagnosed this when building the unit.
Sure, if we're running in a mode where we actually build the modules, rather 
than just including them textually. Under 
`-fmodules-local-submodule-visibility`, we support providing modules semantics 
without doing separate compilation for module headers.


https://reviews.llvm.org/D23858



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


[PATCH] D23895: [ms] Add support for parsing uuid as a MS attribute

2016-08-25 Thread Nico Weber via cfe-commits
thakis created this revision.
thakis added a reviewer: rnk.
thakis added a subscriber: cfe-commits.

Some Windows SDK classes, for example 
Windows::Storage::Streams::IBufferByteAccess, use the ATL way of spelling 
attributes:

  [uuid("")] class IBufferByteAccess {};

To be able to use __uuidof() to grab the uuid off these types, clang needs to 
support uuid as a Microsoft attribute. There was already code to skip Microsoft 
attributes, extend that to look for uuid and parse it (using the existing code 
to parse the same attribute in __declspec). Add a new "MS" attribute type for 
this syntax.

There's already a function that moves attributes off the declspec into an 
attribute list for attributes applying to the type, teach that function to also 
move MS attributes around.  This requires that MS attributes make it to the DS 
in the first place, so move the call to MaybeParseMicrosoftAttributes() into 
ParseDeclOrFunctionDefInternal() so that it can store its results in 
DS.getAttributes().

https://reviews.llvm.org/D23895

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/Attributes.h
  include/clang/Parse/Parser.h
  include/clang/Sema/AttributeList.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseObjc.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Parse/Parser.cpp
  test/Parser/MicrosoftExtensions.cpp
  test/Sema/MicrosoftExtensions.c

Index: test/Sema/MicrosoftExtensions.c
===
--- test/Sema/MicrosoftExtensions.c
+++ test/Sema/MicrosoftExtensions.c
@@ -28,6 +28,8 @@
 
 struct __declspec(uuid("---C000-0046")) IUnknown {}; /* expected-error {{'uuid' attribute is not supported in C}} */
 
+[uuid("---C000-0046")] struct IUnknown2 {}; /* expected-error {{'uuid' attribute is not supported in C}} */
+
 typedef struct notnested {
   long bad1;
   long bad2;
Index: test/Parser/MicrosoftExtensions.cpp
===
--- test/Parser/MicrosoftExtensions.cpp
+++ test/Parser/MicrosoftExtensions.cpp
@@ -49,6 +49,7 @@
 struct __declspec(uuid("000---1234-50047")) uuid_attr_bad3 { };// expected-error {{uuid attribute contains a malformed GUID}}
 struct __declspec(uuid("000---Z234-0047")) uuid_attr_bad4 { };// expected-error {{uuid attribute contains a malformed GUID}}
 struct __declspec(uuid("--1234-0047")) uuid_attr_bad5 { };// expected-error {{uuid attribute contains a malformed GUID}}
+[uuid("--1234-0047")] struct uuid_attr_bad6 { };// expected-error {{uuid attribute contains a malformed GUID}}
 
 __declspec(uuid("00A0---C000-0046")) int i; // expected-warning {{'uuid' attribute only applies to classes}}
 
@@ -59,6 +60,8 @@
 struct __declspec(uuid("00A0---C000-0049"))
 struct_with_uuid2;
 
+[uuid("00A0---C000-0049")] struct struct_with_uuid3;
+
 struct
 struct_with_uuid2 {} ;
 
@@ -69,6 +72,7 @@
 
__uuidof(struct_with_uuid);
__uuidof(struct_with_uuid2);
+   __uuidof(struct_with_uuid3);
__uuidof(struct_without_uuid); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
__uuidof(struct_with_uuid*);
__uuidof(struct_without_uuid*); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
Index: lib/Parse/Parser.cpp
===
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -608,7 +608,6 @@
 
   ParsedAttributesWithRange attrs(AttrFactory);
   MaybeParseCXX11Attributes(attrs);
-  MaybeParseMicrosoftAttributes(attrs);
 
   Result = ParseExternalDeclaration(attrs);
   return false;
@@ -871,11 +870,10 @@
  Tok.is(tok::kw_try);  // X() try { ... }
 }
 
-/// ParseDeclarationOrFunctionDefinition - Parse either a function-definition or
-/// a declaration.  We can't tell which we have until we read up to the
-/// compound-statement in function-definition. TemplateParams, if
-/// non-NULL, provides the template parameters when we're parsing a
-/// C++ template-declaration.
+/// Parse either a function-definition or a declaration.  We can't tell which
+/// we have until we read up to the compound-statement in function-definition.
+/// TemplateParams, if non-NULL, provides the template parameters when we're
+/// parsing a C++ template-declaration.
 ///
 ///   function-definition: [C99 6.9.1]
 /// decl-specs  declarator declaration-list[opt] compound-statement
@@ -891,6 +889,7 @@
 Parser::ParseDeclOrFunctionDefInternal(ParsedAttributesWithRange &attrs,
ParsingDeclSpec &DS,
AccessSpecifier AS) {
+  MaybeParseMicrosoftAttributes(DS.getAttributes());
   // Parse the common declaration-specifiers piece.
   ParseDeclarationSpecifiers(DS, ParsedTemplateInfo(), AS, DSC_top_lev

Re: [PATCH] D22794: [Sema] Propagate nullability when deducing type of auto

2016-08-25 Thread Akira Hatanaka via cfe-commits
ahatanak abandoned this revision.
ahatanak added a comment.

Abandoning this revision.

It looks like we cannot correctly implement this warning without introducing a 
lot of complexity to clang. Users can use the static analyzer instead as it 
already warns about this case.


https://reviews.llvm.org/D22794



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


Re: [PATCH] D23816: Add support for -fdiagnostics-abs-path: printing absolute paths in diagnostics

2016-08-25 Thread Hans Wennborg via cfe-commits
hans updated this revision to Diff 69304.
hans added a comment.

Addressing Richard's comments.


https://reviews.llvm.org/D23816

Files:
  include/clang/Basic/DiagnosticOptions.def
  include/clang/Driver/CLCompatOptions.td
  include/clang/Driver/Options.td
  include/clang/Frontend/TextDiagnostic.h
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/TextDiagnostic.cpp
  test/Driver/cl-options.c
  test/Frontend/Inputs/absolute-paths.h
  test/Frontend/absolute-paths.c

Index: test/Frontend/absolute-paths.c
===
--- /dev/null
+++ test/Frontend/absolute-paths.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -I %S/Inputs/SystemHeaderPrefix/.. %s 2>&1 | FileCheck -check-prefix=NORMAL -check-prefix=CHECK %s
+// RUN: %clang_cc1 -fsyntax-only -I %S/Inputs/SystemHeaderPrefix/.. -fdiagnostics-absolute-paths %s 2>&1 | FileCheck -check-prefix=ABSOLUTE -check-prefix=CHECK %s
+
+#include "absolute-paths.h"
+
+// Check whether the diagnostic from the header above includes the dummy
+// directory in the path.
+// NORMAL: SystemHeaderPrefix
+// ABSOLUTE-NOT: SystemHeaderPrefix
+// CHECK: warning: control reaches end of non-void function
+
+
+// For files which don't exist, just print the filename.
+#line 123 "non-existant.c"
+int g() {}
+// NORMAL: non-existant.c:123:10: warning: control reaches end of non-void function
+// ABSOLUTE: non-existant.c:123:10: warning: control reaches end of non-void function
Index: test/Frontend/Inputs/absolute-paths.h
===
--- /dev/null
+++ test/Frontend/Inputs/absolute-paths.h
@@ -0,0 +1,3 @@
+int f() {
+  // Oops, no return.
+}
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -339,7 +339,6 @@
 // RUN: /FAs \
 // RUN: /FAu \
 // RUN: /favor:blend \
-// RUN: /FC \
 // RUN: /Fifoo \
 // RUN: /Fmfoo \
 // RUN: /FpDebug\main.pch \
@@ -491,6 +490,7 @@
 // RUN: -fdiagnostics-color \
 // RUN: -fno-diagnostics-color \
 // RUN: -fdiagnostics-parseable-fixits \
+// RUN: -fdiagnostics-absolute-paths \
 // RUN: -ferror-limit=10 \
 // RUN: -fmsc-version=1800 \
 // RUN: -fno-strict-aliasing \
Index: lib/Frontend/TextDiagnostic.cpp
===
--- lib/Frontend/TextDiagnostic.cpp
+++ lib/Frontend/TextDiagnostic.cpp
@@ -18,6 +18,7 @@
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Locale.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 
@@ -763,6 +764,22 @@
   OS << '\n';
 }
 
+void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM) {
+  SmallVector AbsoluteFilename;
+  if (DiagOpts->AbsolutePath) {
+const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
+llvm::sys::path::parent_path(Filename));
+if (Dir) {
+  StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
+  llvm::sys::path::append(AbsoluteFilename, DirName,
+  llvm::sys::path::filename(Filename));
+  Filename = StringRef(AbsoluteFilename.data(), AbsoluteFilename.size());
+}
+  }
+
+  OS << Filename;
+}
+
 /// \brief Print out the file/line/column information and include trace.
 ///
 /// This method handlen the emission of the diagnostic location information.
@@ -779,7 +796,7 @@
 if (FID.isValid()) {
   const FileEntry* FE = SM.getFileEntryForID(FID);
   if (FE && FE->isValid()) {
-OS << FE->getName();
+emitFilename(FE->getName(), SM);
 if (FE->isInPCH())
   OS << " (in PCH)";
 OS << ": ";
@@ -795,7 +812,7 @@
   if (DiagOpts->ShowColors)
 OS.changeColor(savedColor, true);
 
-  OS << PLoc.getFilename();
+  emitFilename(PLoc.getFilename(), SM);
   switch (DiagOpts->getFormat()) {
   case DiagnosticOptions::Clang: OS << ':'  << LineNo; break;
   case DiagnosticOptions::MSVC:  OS << '('  << LineNo; break;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -945,6 +945,7 @@
  /*Default=*/true);
   Opts.ShowFixits = !Args.hasArg(OPT_fno_diagnostics_fixit_info);
   Opts.ShowLocation = !Args.hasArg(OPT_fno_show_source_location);
+  Opts.AbsolutePath = Args.hasArg(OPT_fdiagnostics_absolute_paths);
   Opts.ShowOptionNames = Args.hasArg(OPT_fdiagnostics_show_option);
 
   llvm::sys::Process::UseANSIEscapeCodes(Args.hasArg(OPT_fansi_escape_codes));
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -5914,6 +5914,9 @@
 options::OPT_fno_show_source_location))
 

Re: [PATCH] D23816: Add support for -fdiagnostics-abs-path: printing absolute paths in diagnostics

2016-08-25 Thread Hans Wennborg via cfe-commits
hans marked an inline comment as done.


Comment at: include/clang/Driver/Options.td:994
@@ -993,1 +993,3 @@
   Flags<[CC1Option]>, HelpText<"Do not include source location information 
with diagnostics">;
+def fdiagnostics_abs_path : Flag<["-"], "fdiagnostics-abs-path">, 
Group,
+  Flags<[CC1Option, CoreOption]>, HelpText<"Print absolute paths in 
diagnostics">;

rsmith wrote:
> How about `-fdiagnostics-[use-]absolute-paths` rather than 
> `-fdiagnostics-abs-path` to avoid any confusion about this referring to the 
> `abs` function?
Sounds good to me.


https://reviews.llvm.org/D23816



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


Re: [PATCH] D23816: Add support for -fdiagnostics-abs-path: printing absolute paths in diagnostics

2016-08-25 Thread Richard Smith via cfe-commits
rsmith accepted this revision.


Comment at: lib/Frontend/TextDiagnostic.cpp:770-777
@@ +769,10 @@
+  if (DiagOpts->AbsolutePath) {
+const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
+llvm::sys::path::parent_path(Filename));
+if (Dir) {
+  StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
+  llvm::sys::path::append(AbsoluteFilename, DirName,
+  llvm::sys::path::filename(Filename));
+  Filename = StringRef(AbsoluteFilename.data(), AbsoluteFilename.size());
+}
+  }

Why split off the filename and rejoin it here? Is this to better handle cases 
where the directory exists but the file doesn't, or an attempt to avoid 
resolving file symlinks? A comment in the code explaining this would be useful.


https://reviews.llvm.org/D23816



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


Re: [PATCH] D23816: Add support for -fdiagnostics-abs-path: printing absolute paths in diagnostics

2016-08-25 Thread Hans Wennborg via cfe-commits
hans added inline comments.


Comment at: lib/Frontend/TextDiagnostic.cpp:770-777
@@ +769,10 @@
+  if (DiagOpts->AbsolutePath) {
+const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
+llvm::sys::path::parent_path(Filename));
+if (Dir) {
+  StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
+  llvm::sys::path::append(AbsoluteFilename, DirName,
+  llvm::sys::path::filename(Filename));
+  Filename = StringRef(AbsoluteFilename.data(), AbsoluteFilename.size());
+}
+  }

rsmith wrote:
> Why split off the filename and rejoin it here? Is this to better handle cases 
> where the directory exists but the file doesn't, or an attempt to avoid 
> resolving file symlinks? A comment in the code explaining this would be 
> useful.
If I understand correctly, getCanonicalName only works with a DirectoryEntry, 
which is supposed to be a directory, not a full path to a file. So I didn't see 
any other way than getting the canonical name for the dir, and then joining it 
with the filename.


https://reviews.llvm.org/D23816



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


r279794 - C++ Modules TS: add frontend support for building pcm files from module

2016-08-25 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Thu Aug 25 19:14:38 2016
New Revision: 279794

URL: http://llvm.org/viewvc/llvm-project?rev=279794&view=rev
Log:
C++ Modules TS: add frontend support for building pcm files from module
interface files. At the moment, all declarations (and no macros) are exported,
and 'export' declarations are not supported yet.

Added:
cfe/trunk/test/Parser/cxx-modules-interface.cppm
Modified:
cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Basic/LangOptions.h
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/Frontend/FrontendActions.h
cfe/trunk/include/clang/Frontend/FrontendOptions.h
cfe/trunk/include/clang/Lex/ModuleMap.h
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/FrontendActions.cpp
cfe/trunk/lib/Frontend/FrontendOptions.cpp
cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
cfe/trunk/lib/Lex/ModuleMap.cpp
cfe/trunk/lib/Lex/PPLexerChange.cpp
cfe/trunk/lib/Lex/PPMacroExpansion.cpp
cfe/trunk/lib/Lex/Preprocessor.cpp
cfe/trunk/lib/Parse/ParseAST.cpp
cfe/trunk/lib/Parse/Parser.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclObjC.cpp
cfe/trunk/lib/Serialization/GeneratePCH.cpp
cfe/trunk/test/Parser/cxx-modules-import.cpp
cfe/trunk/test/lit.cfg

Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=279794&r1=279793&r2=279794&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Thu Aug 25 
19:14:38 2016
@@ -174,6 +174,8 @@ def warn_incompatible_analyzer_plugin_ap
 def note_incompatible_analyzer_plugin_api : Note<
 "current API version is '%0', but plugin was compiled with version '%1'">;
 
+def err_module_interface_requires_modules_ts : Error<
+  "module interface compilation requires '-fmodules-ts'">;
 def warn_module_config_mismatch : Warning<
   "module file %0 cannot be loaded due to a configuration mismatch with the 
current "
   "compilation">, InGroup>, 
DefaultError;
@@ -217,4 +219,4 @@ def err_invalid_vfs_overlay : Error<
 
 def warn_option_invalid_ocl_version : Warning<
   "OpenCL version %0 does not support the option '%1'">, InGroup;
-}
\ No newline at end of file
+}

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=279794&r1=279793&r2=279794&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Thu Aug 25 19:14:38 
2016
@@ -1021,6 +1021,10 @@ def warn_pragma_unroll_cuda_value_in_par
 } // end of Parse Issue category.
 
 let CategoryName = "Modules Issue" in {
+def err_expected_module_interface_decl : Error<
+  "expected module declaration at start of module interface">;
+def err_unexpected_module_decl : Error<
+  "module declaration must be the first declaration in the translation unit">;
 def err_module_expected_ident : Error<
   "expected a module name after module%select{| import}0">;
 def err_unexpected_module_kind : Error<

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=279794&r1=279793&r2=279794&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Aug 25 19:14:38 
2016
@@ -8467,6 +8467,10 @@ def err_module_interface_implementation_
   "found while %select{not |not |}0building module interface">;
 def err_current_module_name_mismatch : Error<
   "module name '%0' specified on command line does not match name of module">;
+def err_module_redefinition : Error<
+  "redefinition of module '%0'">;
+def note_prev_module_definition : Note<"previously defined here">;
+def note_prev_module_definition_from_ast_file : Note<"module loaded from 
'%0'">;
 def err_module_private_specialization : Error<
   "%select{template|partial|member}0 specialization cannot be "
   "declared __module_private__">;

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=279794&r1=279793&r2=279794&view=diff

Re: [PATCH] D23816: Add support for -fdiagnostics-abs-path: printing absolute paths in diagnostics

2016-08-25 Thread Richard Smith via cfe-commits
On Thu, Aug 25, 2016 at 5:22 PM, Hans Wennborg  wrote:

> hans added inline comments.
>
> 
> Comment at: lib/Frontend/TextDiagnostic.cpp:770-777
> @@ +769,10 @@
> +  if (DiagOpts->AbsolutePath) {
> +const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
> +llvm::sys::path::parent_path(Filename));
> +if (Dir) {
> +  StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
> +  llvm::sys::path::append(AbsoluteFilename, DirName,
> +  llvm::sys::path::filename(Filename));
> +  Filename = StringRef(AbsoluteFilename.data(),
> AbsoluteFilename.size());
> +}
> +  }
> 
> rsmith wrote:
> > Why split off the filename and rejoin it here? Is this to better handle
> cases where the directory exists but the file doesn't, or an attempt to
> avoid resolving file symlinks? A comment in the code explaining this would
> be useful.
> If I understand correctly, getCanonicalName only works with a
> DirectoryEntry, which is supposed to be a directory, not a full path to a
> file. So I didn't see any other way than getting the canonical name for the
> dir, and then joining it with the filename.


Huh, so it does. That's an odd choice. Anyway, let's go with this, unless
you feel like refactoring it to work on an arbitrary path.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D23898: Minor cleanup of MismatchingNewDeleteDetector

2016-08-25 Thread Alexander Shaposhnikov via cfe-commits
alexshap created this revision.
alexshap added reviewers: ismailp, rsmith, bkramer.
alexshap added a subscriber: cfe-commits.
alexshap set the repository for this revision to rL LLVM.
alexshap changed the visibility of this Differential Revision from "Public (No 
Login Required)" to "All Users".

The class MismatchingNewDeleteDetector is in 
lib/Sema/SemaExprCXX.cpp inside the anonymous namespace.
This diff reorders the fields and removes the excessive padding.
Test plan: make -j8 check-clang



Repository:
  rL LLVM

https://reviews.llvm.org/D23898

Files:
  lib/Sema/SemaExprCXX.cpp

Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -2593,7 +2593,7 @@
   /// translation unit. False, if this is the initial analysis at the point
   /// delete-expression was encountered.
   explicit MismatchingNewDeleteDetector(bool EndOfTU)
-  : IsArrayForm(false), Field(nullptr), EndOfTU(EndOfTU),
+  : Field(nullptr), IsArrayForm(false), EndOfTU(EndOfTU),
 HasUndefinedConstructors(false) {}
 
   /// \brief Checks whether pointee of a delete-expression is initialized with
@@ -2612,11 +2612,11 @@
   /// \param DeleteWasArrayForm Array form-ness of the delete-expression used
   /// for deleting the \p Field.
   MismatchResult analyzeField(FieldDecl *Field, bool DeleteWasArrayForm);
+  FieldDecl *Field;
   /// List of mismatching new-expressions used for initialization of the 
pointee
   llvm::SmallVector NewExprs;
   /// Indicates whether delete-expression was in array form.
   bool IsArrayForm;
-  FieldDecl *Field;
 
 private:
   const bool EndOfTU;


Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -2593,7 +2593,7 @@
   /// translation unit. False, if this is the initial analysis at the point
   /// delete-expression was encountered.
   explicit MismatchingNewDeleteDetector(bool EndOfTU)
-  : IsArrayForm(false), Field(nullptr), EndOfTU(EndOfTU),
+  : Field(nullptr), IsArrayForm(false), EndOfTU(EndOfTU),
 HasUndefinedConstructors(false) {}
 
   /// \brief Checks whether pointee of a delete-expression is initialized with
@@ -2612,11 +2612,11 @@
   /// \param DeleteWasArrayForm Array form-ness of the delete-expression used
   /// for deleting the \p Field.
   MismatchResult analyzeField(FieldDecl *Field, bool DeleteWasArrayForm);
+  FieldDecl *Field;
   /// List of mismatching new-expressions used for initialization of the pointee
   llvm::SmallVector NewExprs;
   /// Indicates whether delete-expression was in array form.
   bool IsArrayForm;
-  FieldDecl *Field;
 
 private:
   const bool EndOfTU;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23898: Minor cleanup of MismatchingNewDeleteDetector

2016-08-25 Thread Alexander Shaposhnikov via cfe-commits
alexshap added a comment.

F2334424: Screen Shot 2016-08-25 at 5.30.51 PM.png 



Repository:
  rL LLVM

https://reviews.llvm.org/D23898



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


Re: [PATCH] D23895: [ms] Add support for parsing uuid as a MS attribute

2016-08-25 Thread Richard Smith via cfe-commits
rsmith added inline comments.


Comment at: include/clang/Basic/Attr.td:201
@@ -200,2 +200,3 @@
 class Declspec : Spelling;
+class MS : Spelling;
 class CXX11

Is there some better description for this than MS? `__declspec` is also an MS 
attribute. Is it fair to call this IDL or MSIDL or similar?


Comment at: include/clang/Parse/Parser.h:2109-2110
@@ -2108,4 +2108,4 @@
 
-  void handleDeclspecAlignBeforeClassKey(ParsedAttributesWithRange &Attrs,
- DeclSpec &DS, Sema::TagUseKind TUK);
+  void stripSomeAttribsOffDeclSpec(ParsedAttributesWithRange &Attrs,
+   DeclSpec &DS, Sema::TagUseKind TUK);
 

Can you give this a less vague-sounding name? :)


Comment at: lib/Parse/ParseDeclCXX.cpp:3950
@@ +3949,3 @@
+  ConsumeToken();
+  if (Name->getName() == "uuid")
+ParseMicrosoftDeclSpecArgs(Name, Loc, attrs, AttributeList::AS_MS);

Is anything known to fail if you accept arbitrary attributes here, or is this 
just the only one you /know/ you need right now? (If we can ditch the 
whitelist, that would seem preferable.)


https://reviews.llvm.org/D23895



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


Re: [PATCH] D23895: [ms] Add support for parsing uuid as a MS attribute

2016-08-25 Thread Reid Kleckner via cfe-commits
rnk added a comment.

I think these are known as "IDL attributes":
https://msdn.microsoft.com/en-us/library/8tesw2eh.aspx

Let's update the naming to use that terminology, so AS_MS should be AS_IDL, and 
MaybeParseMicrosoftAttributes should be MaybeParseMicrosoftIDLAttributes, etc.

Also, doesn't this introduce ambiguities into the grammar? Something like this:

  void useit(int);
  int main() {
int uuid = 42;
[uuid]() {
  useit(uuid);
}();
  }

Will we keep parsing that as a lambda after this change or not?


https://reviews.llvm.org/D23895



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


Re: [PATCH] D21959: [X86] Add xgetbv xsetbv intrinsics

2016-08-25 Thread Reid Kleckner via cfe-commits
rnk added a comment.

Now might be the time to solve the larger problem of wider intrinsic 
availability. Like I mentioned, all these intrinsics really ought to be 
available all the time, regardless of CPU subtarget.


Repository:
  rL LLVM

https://reviews.llvm.org/D21959



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


r279799 - Sort list of driver-known file extensions. It was previously approximately

2016-08-25 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Thu Aug 25 19:41:59 2016
New Revision: 279799

URL: http://llvm.org/viewvc/llvm-project?rev=279799&view=rev
Log:
Sort list of driver-known file extensions. It was previously approximately
ordered by length then alphabetically; apply that order consistently.

Modified:
cfe/trunk/lib/Driver/Types.cpp

Modified: cfe/trunk/lib/Driver/Types.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Types.cpp?rev=279799&r1=279798&r2=279799&view=diff
==
--- cfe/trunk/lib/Driver/Types.cpp (original)
+++ cfe/trunk/lib/Driver/Types.cpp Thu Aug 25 19:41:59 2016
@@ -156,55 +156,55 @@ bool types::isCuda(ID Id) {
 types::ID types::lookupTypeForExtension(const char *Ext) {
   return llvm::StringSwitch(Ext)
.Case("c", TY_C)
+   .Case("C", TY_CXX)
+   .Case("F", TY_Fortran)
+   .Case("f", TY_PP_Fortran)
+   .Case("h", TY_CHeader)
+   .Case("H", TY_CXXHeader)
.Case("i", TY_PP_C)
.Case("m", TY_ObjC)
.Case("M", TY_ObjCXX)
-   .Case("h", TY_CHeader)
-   .Case("C", TY_CXX)
-   .Case("H", TY_CXXHeader)
-   .Case("f", TY_PP_Fortran)
-   .Case("F", TY_Fortran)
-   .Case("s", TY_PP_Asm)
-   .Case("asm", TY_PP_Asm)
-   .Case("S", TY_Asm)
.Case("o", TY_Object)
-   .Case("obj", TY_Object)
-   .Case("lib", TY_Object)
-   .Case("ii", TY_PP_CXX)
-   .Case("mi", TY_PP_ObjC)
-   .Case("mm", TY_ObjCXX)
+   .Case("S", TY_Asm)
+   .Case("s", TY_PP_Asm)
.Case("bc", TY_LLVM_BC)
.Case("cc", TY_CXX)
.Case("CC", TY_CXX)
.Case("cl", TY_CL)
.Case("cp", TY_CXX)
.Case("cu", TY_CUDA)
-   .Case("cui", TY_PP_CUDA)
.Case("hh", TY_CXXHeader)
+   .Case("ii", TY_PP_CXX)
.Case("ll", TY_LLVM_IR)
-   .Case("hpp", TY_CXXHeader)
-   .Case("ads", TY_Ada)
+   .Case("mi", TY_PP_ObjC)
+   .Case("mm", TY_ObjCXX)
+   .Case("rs", TY_RenderScript)
.Case("adb", TY_Ada)
+   .Case("ads", TY_Ada)
+   .Case("asm", TY_PP_Asm)
.Case("ast", TY_AST)
+   .Case("cpp", TY_CXX)
+   .Case("CPP", TY_CXX)
.Case("c++", TY_CXX)
.Case("C++", TY_CXX)
+   .Case("cui", TY_PP_CUDA)
.Case("cxx", TY_CXX)
-   .Case("cpp", TY_CXX)
-   .Case("CPP", TY_CXX)
.Case("CXX", TY_CXX)
+   .Case("F90", TY_Fortran)
+   .Case("f90", TY_PP_Fortran)
+   .Case("F95", TY_Fortran)
+   .Case("f95", TY_PP_Fortran)
.Case("for", TY_PP_Fortran)
.Case("FOR", TY_PP_Fortran)
.Case("fpp", TY_Fortran)
.Case("FPP", TY_Fortran)
-   .Case("f90", TY_PP_Fortran)
-   .Case("f95", TY_PP_Fortran)
-   .Case("F90", TY_Fortran)
-   .Case("F95", TY_Fortran)
+   .Case("gch", TY_PCH)
+   .Case("hpp", TY_CXXHeader)
+   .Case("lib", TY_Object)
.Case("mii", TY_PP_ObjCXX)
-   .Case("pcm", TY_ModuleFile)
+   .Case("obj", TY_Object)
.Case("pch", TY_PCH)
-   .Case("gch", TY_PCH)
-   .Case("rs", TY_RenderScript)
+   .Case("pcm", TY_ModuleFile)
.Default(TY_INVALID);
 }
 


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


Re: r253647 - [Myriad]: handle Preprocess job action (-E)

2016-08-25 Thread Richard Smith via cfe-commits
On Thu, Nov 19, 2015 at 8:58 PM, Douglas Katzman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: dougk
> Date: Thu Nov 19 22:58:12 2015
> New Revision: 253647
>
> URL: http://llvm.org/viewvc/llvm-project?rev=253647&view=rev
> Log:
> [Myriad]: handle Preprocess job action (-E)
>
> Modified:
> cfe/trunk/lib/Driver/ToolChains.cpp
> cfe/trunk/lib/Driver/Tools.cpp
> cfe/trunk/test/Driver/myriad-toolchain.c
>
> Modified: cfe/trunk/lib/Driver/ToolChains.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/
> ToolChains.cpp?rev=253647&r1=253646&r2=253647&view=diff
> 
> ==
> --- cfe/trunk/lib/Driver/ToolChains.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains.cpp Thu Nov 19 22:58:12 2015
> @@ -4434,6 +4434,7 @@ Tool *MyriadToolChain::SelectTool(const
>if (!isShaveCompilation(getTriple()))
>  return ToolChain::SelectTool(JA);
>switch (JA.getKind()) {
> +  case Action::PreprocessJobClass:
>case Action::CompileJobClass:
>  if (!Compiler)
>Compiler.reset(new tools::SHAVE::Compiler(*this));
>
> Modified: cfe/trunk/lib/Driver/Tools.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/
> Tools.cpp?rev=253647&r1=253646&r2=253647&view=diff
> 
> ==
> --- cfe/trunk/lib/Driver/Tools.cpp (original)
> +++ cfe/trunk/lib/Driver/Tools.cpp Thu Nov 19 22:58:12 2015
> @@ -9895,17 +9895,22 @@ void tools::SHAVE::Compiler::ConstructJo
>const InputInfoList &Inputs,
>const ArgList &Args,
>const char *LinkingOutput)
> const {
> -
>ArgStringList CmdArgs;
> -
>assert(Inputs.size() == 1);
>const InputInfo &II = Inputs[0];
> -  assert(II.getType() == types::TY_C || II.getType() == types::TY_CXX);
> -  assert(Output.getType() == types::TY_PP_Asm); // Require preprocessed
> asm.
> +  assert(II.getType() == types::TY_C || II.getType() == types::TY_CXX ||
> + II.getType() == types::TY_PP_CXX);
>

This assert fails when compiling a .i file (TY_PP_C).


> -  CmdArgs.push_back("-DMYRIAD2");
> +  if (JA.getKind() == Action::PreprocessJobClass) {
> +Args.ClaimAllArgs();
> +CmdArgs.push_back("-E");
> +  } else {
> +assert(Output.getType() == types::TY_PP_Asm); // Require preprocessed
> asm.
> +CmdArgs.push_back("-S");
> +CmdArgs.push_back("-fno-exceptions"); // Always do this even if
> unspecified.
> +  }
>CmdArgs.push_back("-mcpu=myriad2");
> -  CmdArgs.push_back("-S");
> +  CmdArgs.push_back("-DMYRIAD2");
>
>// Append all -I, -iquote, -isystem paths, defines/undefines,
>// 'f' flags, optimize flags, and warning options.
> @@ -9931,8 +9936,6 @@ void tools::SHAVE::Compiler::ConstructJo
>  }
>}
>
> -  CmdArgs.push_back("-fno-exceptions"); // Always do this even if
> unspecified.
> -
>CmdArgs.push_back(II.getFilename());
>CmdArgs.push_back("-o");
>CmdArgs.push_back(Output.getFilename());
>
> Modified: cfe/trunk/test/Driver/myriad-toolchain.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/
> myriad-toolchain.c?rev=253647&r1=253646&r2=253647&view=diff
> 
> ==
> --- cfe/trunk/test/Driver/myriad-toolchain.c (original)
> +++ cfe/trunk/test/Driver/myriad-toolchain.c Thu Nov 19 22:58:12 2015
> @@ -38,7 +38,7 @@
>
>  // RUN: %clang -target shave-myriad -c -### %s -isystem somewhere
> -Icommon -Wa,-yippee 2>&1 \
>  // RUN:   | FileCheck %s -check-prefix=MOVICOMPILE
> -// MOVICOMPILE: moviCompile" "-DMYRIAD2" "-mcpu=myriad2" "-S" "-isystem"
> "somewhere" "-I" "common"
> +// MOVICOMPILE: moviCompile" "-S" "-fno-exceptions" "-mcpu=myriad2"
> "-DMYRIAD2" "-isystem" "somewhere" "-I" "common"
>  // MOVICOMPILE: moviAsm" "-no6thSlotCompression" "-cv:myriad2"
> "-noSPrefixing" "-a"
>  // MOVICOMPILE: "-yippee" "-i:somewhere" "-i:common" "-elf"
>
> @@ -58,11 +58,15 @@
>
>  // RUN: %clang -target shave-myriad -c %s -o foo.o -### -MD -MF dep.d
> 2>&1 \
>  // RUN:   | FileCheck %s -check-prefix=MDMF
> -// MDMF: "-S" "-MD" "-MF" "dep.d" "-MT" "foo.o"
> +// MDMF: "-S" "-fno-exceptions" "-mcpu=myriad2" "-DMYRIAD2" "-MD" "-MF"
> "dep.d" "-MT" "foo.o"
>
>  // RUN: %clang -target shave-myriad -std=gnu++11 -S %s -o foo.o -### 2>&1
> \
>  // RUN:   | FileCheck %s -check-prefix=STDEQ
> -// STDEQ: "-mcpu=myriad2" "-S" "-std=gnu++11"
> +// STDEQ: "-S" "-fno-exceptions" "-mcpu=myriad2" "-DMYRIAD2"
> "-std=gnu++11"
> +
> +// RUN: %clang -target shave-myriad -E -Ifoo %s -o foo.i -### 2>&1 \
> +// RUN:   | FileCheck %s -check-prefix=PREPROCESS
> +// PREPROCESS: "-E" "-mcpu=myriad2" "-DMYRIAD2" "-I" "foo"
>
>  // RUN: %clang -target sparc-myriad -### --driver-mode=g++ %s 2>&1 |
> FileCheck %s --check-prefix=STDLIBCXX
>  // STDLIBCXX: "-lstdc++" "-lc" "-lgcc"
>
>
>

Re: [PATCH] D23895: [ms] Add support for parsing uuid as a MS attribute

2016-08-25 Thread Richard Smith via cfe-commits
On Thu, Aug 25, 2016 at 5:46 PM, Reid Kleckner  wrote:

> rnk added a comment.
>
> I think these are known as "IDL attributes":
> https://msdn.microsoft.com/en-us/library/8tesw2eh.aspx
>
> Let's update the naming to use that terminology, so AS_MS should be
> AS_IDL, and MaybeParseMicrosoftAttributes should be
> MaybeParseMicrosoftIDLAttributes, etc.
>
> Also, doesn't this introduce ambiguities into the grammar? Something like
> this:
>
>   void useit(int);
>   int main() {
> int uuid = 42;
> [uuid]() {
>   useit(uuid);
> }();
>   }
>
> Will we keep parsing that as a lambda after this change or not?


I can see why (paraphrasing that MSDN article) parsing these attributes
requires you to be a wizard.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23895: [ms] Add support for parsing uuid as a MS attribute

2016-08-25 Thread Aaron Ballman via cfe-commits
On Thu, Aug 25, 2016 at 9:16 PM, Richard Smith  wrote:
> On Thu, Aug 25, 2016 at 5:46 PM, Reid Kleckner  wrote:
>>
>> rnk added a comment.
>>
>> I think these are known as "IDL attributes":
>> https://msdn.microsoft.com/en-us/library/8tesw2eh.aspx
>>
>> Let's update the naming to use that terminology, so AS_MS should be
>> AS_IDL, and MaybeParseMicrosoftAttributes should be
>> MaybeParseMicrosoftIDLAttributes, etc.

I disagree that these are IDL attributes, though they are certainly
used in IDL. They're also used for SAL, COM, the compiler, etc. For
instance: https://msdn.microsoft.com/en-us/library/3c3t8ddh.aspx

We've usually called these "Microsoft" attributes in the past (see
Parser::ParseMicrosoftAttributes()), and I think that's reasonable
terminology for them (I am comfortable having declspec attributes be
different than whatever we wind up calling these because they are
distinct syntactic constructs anyway).

>>
>> Also, doesn't this introduce ambiguities into the grammar? Something like
>> this:
>>
>>   void useit(int);
>>   int main() {
>> int uuid = 42;
>> [uuid]() {
>>   useit(uuid);
>> }();
>>   }
>>
>> Will we keep parsing that as a lambda after this change or not?
>
>
> I can see why (paraphrasing that MSDN article) parsing these attributes
> requires you to be a wizard.

We tried implementing this once before and ran into some considerable
roadblocks, both with C++ lambdas and with Objective-C message sends
(which is somewhat less concerning than lambdas).

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


Re: [PATCH] D23901: Minor cleanup of the class CallStackFrame

2016-08-25 Thread Alexander Shaposhnikov via cfe-commits
alexshap added a comment.

F2334496: Screen Shot 2016-08-25 at 6.20.05 PM.png 



Repository:
  rL LLVM

https://reviews.llvm.org/D23901



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


[PATCH] D23901: Minor cleanup of the class CallStackFrame

2016-08-25 Thread Alexander Shaposhnikov via cfe-commits
alexshap created this revision.
alexshap added reviewers: rsmith, gbiv, bkramer.
alexshap added a subscriber: cfe-commits.
alexshap set the repository for this revision to rL LLVM.
alexshap changed the visibility of this Differential Revision from "Public (No 
Login Required)" to "All Users".

The struct CallStackFrame is in lib/AST/ExprConstant.cpp
inside anonymous namespace.
This diff reorders the fields and removes excessive padding.
Test plan: make -j8 check-clang

Repository:
  rL LLVM

https://reviews.llvm.org/D23901

Files:
  lib/AST/ExprConstant.cpp

Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -310,15 +310,9 @@
 /// Parent - The caller of this stack frame.
 CallStackFrame *Caller;
 
-/// CallLoc - The location of the call expression for this call.
-SourceLocation CallLoc;
-
 /// Callee - The function which was called.
 const FunctionDecl *Callee;
 
-/// Index - The call index of this call.
-unsigned Index;
-
 /// This - The binding for the this pointer in this call, if any.
 const LValue *This;
 
@@ -333,6 +327,12 @@
 /// Temporaries - Temporary lvalues materialized within this stack frame.
 MapTy Temporaries;
 
+/// CallLoc - The location of the call expression for this call.
+SourceLocation CallLoc;
+
+/// Index - The call index of this call.
+unsigned Index;
+
 CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
APValue *Arguments);
@@ -961,8 +961,8 @@
 CallStackFrame::CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
APValue *Arguments)
-: Info(Info), Caller(Info.CurrentCall), CallLoc(CallLoc), Callee(Callee),
-  Index(Info.NextCallIndex++), This(This), Arguments(Arguments) {
+: Info(Info), Caller(Info.CurrentCall), Callee(Callee), This(This),
+  Arguments(Arguments), CallLoc(CallLoc), Index(Info.NextCallIndex++) {
   Info.CurrentCall = this;
   ++Info.CallStackDepth;
 }


Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -310,15 +310,9 @@
 /// Parent - The caller of this stack frame.
 CallStackFrame *Caller;
 
-/// CallLoc - The location of the call expression for this call.
-SourceLocation CallLoc;
-
 /// Callee - The function which was called.
 const FunctionDecl *Callee;
 
-/// Index - The call index of this call.
-unsigned Index;
-
 /// This - The binding for the this pointer in this call, if any.
 const LValue *This;
 
@@ -333,6 +327,12 @@
 /// Temporaries - Temporary lvalues materialized within this stack frame.
 MapTy Temporaries;
 
+/// CallLoc - The location of the call expression for this call.
+SourceLocation CallLoc;
+
+/// Index - The call index of this call.
+unsigned Index;
+
 CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
APValue *Arguments);
@@ -961,8 +961,8 @@
 CallStackFrame::CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
APValue *Arguments)
-: Info(Info), Caller(Info.CurrentCall), CallLoc(CallLoc), Callee(Callee),
-  Index(Info.NextCallIndex++), This(This), Arguments(Arguments) {
+: Info(Info), Caller(Info.CurrentCall), Callee(Callee), This(This),
+  Arguments(Arguments), CallLoc(CallLoc), Index(Info.NextCallIndex++) {
   Info.CurrentCall = this;
   ++Info.CallStackDepth;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D23902: Minor cleanup of PTHWriter

2016-08-25 Thread Alexander Shaposhnikov via cfe-commits
alexshap created this revision.
alexshap added reviewers: bogner, dblaikie, rnk.
alexshap added a subscriber: cfe-commits.
alexshap set the repository for this revision to rL LLVM.
alexshap changed the visibility of this Differential Revision from "Public (No 
Login Required)" to "All Users".

The class PTHWriter is in lib/Frontend/CacheTokens.cpp inside anonymous 
namespace.
This diff changes the order of fields an removes excessive padding.


Repository:
  rL LLVM

https://reviews.llvm.org/D23902

Files:
  lib/Frontend/CacheTokens.cpp

Index: lib/Frontend/CacheTokens.cpp
===
--- lib/Frontend/CacheTokens.cpp
+++ lib/Frontend/CacheTokens.cpp
@@ -182,14 +182,14 @@
   typedef llvm::DenseMap IDMap;
   typedef llvm::StringMap CachedStrsTy;
 
-  IDMap IM;
   raw_pwrite_stream &Out;
   Preprocessor& PP;
-  uint32_t idcount;
+  IDMap IM;
+  std::vector*> StrEntries;
   PTHMap PM;
   CachedStrsTy CachedStrs;
+  uint32_t idcount;
   Offset CurStrOffset;
-  std::vector*> StrEntries;
 
    Get the persistent id for the given IdentifierInfo*.
   uint32_t ResolveID(const IdentifierInfo* II);


Index: lib/Frontend/CacheTokens.cpp
===
--- lib/Frontend/CacheTokens.cpp
+++ lib/Frontend/CacheTokens.cpp
@@ -182,14 +182,14 @@
   typedef llvm::DenseMap IDMap;
   typedef llvm::StringMap CachedStrsTy;
 
-  IDMap IM;
   raw_pwrite_stream &Out;
   Preprocessor& PP;
-  uint32_t idcount;
+  IDMap IM;
+  std::vector*> StrEntries;
   PTHMap PM;
   CachedStrsTy CachedStrs;
+  uint32_t idcount;
   Offset CurStrOffset;
-  std::vector*> StrEntries;
 
    Get the persistent id for the given IdentifierInfo*.
   uint32_t ResolveID(const IdentifierInfo* II);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23902: Minor cleanup of PTHWriter

2016-08-25 Thread Alexander Shaposhnikov via cfe-commits
alexshap added a comment.

F2334541: Screen Shot 2016-08-25 at 6.57.45 PM.png 



Repository:
  rL LLVM

https://reviews.llvm.org/D23902



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


Re: [PATCH] D23895: [ms] Add support for parsing uuid as a MS attribute

2016-08-25 Thread Nico Weber via cfe-commits
thakis marked an inline comment as done.
thakis added a comment.

Thanks for the fast review and the good comments! I'll leave it to y'all to 
agree on some name if you don't like the one I picked.

> Also, doesn't this introduce ambiguities into the grammar? Something like 
> this:

> 

>   void useit(int);

>   int main() {

> int uuid = 42;

> [uuid]() {

>   useit(uuid);

> }();

>   }

>

> 

> Will we keep parsing that as a lambda after this change or not?


This change shouldn't change how things are parsed. It moved calls of 
MaybeParseMicrosoftAttributes(attrs) that precede 
ParseExternalDeclaration(attrs) into ParseExternalDeclaration(). As far as I 
can tell, every call of ParseExternalDeclaration() was preceded by a call to 
MaybeParseMicrosoftAttributes(), so this should be behavior-preserving. In your 
example, the lamda is not an external declaration, so it's not affected. (That 
means [uuid(...)] won't work for local classes, but that's probably ok.)

Now that I looked at this more, behavior changes slightly for empty external 
declarations. Before,

[foob];

didn't produce any output, now it produces "declaration doesn't declare 
anything". I don't know if that's important.



Comment at: include/clang/Basic/Attr.td:201
@@ -200,2 +200,3 @@
 class Declspec : Spelling;
+class MS : Spelling;
 class CXX11

rsmith wrote:
> Is there some better description for this than MS? `__declspec` is also an MS 
> attribute. Is it fair to call this IDL or MSIDL or similar?
There are many IDL compilers. MSIDL works for me, so does anything else you 3 
agree on :-)


Comment at: include/clang/Parse/Parser.h:2109-2110
@@ -2108,4 +2108,4 @@
 
-  void handleDeclspecAlignBeforeClassKey(ParsedAttributesWithRange &Attrs,
- DeclSpec &DS, Sema::TagUseKind TUK);
+  void stripSomeAttribsOffDeclSpec(ParsedAttributesWithRange &Attrs,
+   DeclSpec &DS, Sema::TagUseKind TUK);
 

rsmith wrote:
> Can you give this a less vague-sounding name? :)
Changed to stripTypeAttribsOffDeclSpec


Comment at: lib/Parse/ParseDeclCXX.cpp:3950
@@ +3949,3 @@
+  ConsumeToken();
+  if (Name->getName() == "uuid")
+ParseMicrosoftDeclSpecArgs(Name, Loc, attrs, AttributeList::AS_MS);

rsmith wrote:
> Is anything known to fail if you accept arbitrary attributes here, or is this 
> just the only one you /know/ you need right now? (If we can ditch the 
> whitelist, that would seem preferable.)
These attributes can be different from decl spec args. For example, we don't 
have anything that parses `threading("apartment")`, `module(name="MyLib")` at 
the moment (I suppose ParseAttributeArgsCommon does that at a token level, but 
it won't know all these attributes that are allowed here).


https://reviews.llvm.org/D23895



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


Re: [PATCH] D23895: [ms] Add support for parsing uuid as a MS attribute

2016-08-25 Thread Nico Weber via cfe-commits
thakis updated this revision to Diff 69312.
thakis marked an inline comment as done.
thakis added a comment.

comments round 1


https://reviews.llvm.org/D23895

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/Attributes.h
  include/clang/Parse/Parser.h
  include/clang/Sema/AttributeList.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseObjc.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Parse/Parser.cpp
  test/Parser/MicrosoftExtensions.cpp
  test/Sema/MicrosoftExtensions.c

Index: test/Sema/MicrosoftExtensions.c
===
--- test/Sema/MicrosoftExtensions.c
+++ test/Sema/MicrosoftExtensions.c
@@ -28,6 +28,8 @@
 
 struct __declspec(uuid("---C000-0046")) IUnknown {}; /* expected-error {{'uuid' attribute is not supported in C}} */
 
+[uuid("---C000-0046")] struct IUnknown2 {}; /* expected-error {{'uuid' attribute is not supported in C}} */
+
 typedef struct notnested {
   long bad1;
   long bad2;
Index: test/Parser/MicrosoftExtensions.cpp
===
--- test/Parser/MicrosoftExtensions.cpp
+++ test/Parser/MicrosoftExtensions.cpp
@@ -49,6 +49,7 @@
 struct __declspec(uuid("000---1234-50047")) uuid_attr_bad3 { };// expected-error {{uuid attribute contains a malformed GUID}}
 struct __declspec(uuid("000---Z234-0047")) uuid_attr_bad4 { };// expected-error {{uuid attribute contains a malformed GUID}}
 struct __declspec(uuid("--1234-0047")) uuid_attr_bad5 { };// expected-error {{uuid attribute contains a malformed GUID}}
+[uuid("--1234-0047")] struct uuid_attr_bad6 { };// expected-error {{uuid attribute contains a malformed GUID}}
 
 __declspec(uuid("00A0---C000-0046")) int i; // expected-warning {{'uuid' attribute only applies to classes}}
 
@@ -59,6 +60,8 @@
 struct __declspec(uuid("00A0---C000-0049"))
 struct_with_uuid2;
 
+[uuid("00A0---C000-0049")] struct struct_with_uuid3;
+
 struct
 struct_with_uuid2 {} ;
 
@@ -69,6 +72,7 @@
 
__uuidof(struct_with_uuid);
__uuidof(struct_with_uuid2);
+   __uuidof(struct_with_uuid3);
__uuidof(struct_without_uuid); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
__uuidof(struct_with_uuid*);
__uuidof(struct_without_uuid*); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
Index: lib/Parse/Parser.cpp
===
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -608,7 +608,6 @@
 
   ParsedAttributesWithRange attrs(AttrFactory);
   MaybeParseCXX11Attributes(attrs);
-  MaybeParseMicrosoftAttributes(attrs);
 
   Result = ParseExternalDeclaration(attrs);
   return false;
@@ -871,11 +870,10 @@
  Tok.is(tok::kw_try);  // X() try { ... }
 }
 
-/// ParseDeclarationOrFunctionDefinition - Parse either a function-definition or
-/// a declaration.  We can't tell which we have until we read up to the
-/// compound-statement in function-definition. TemplateParams, if
-/// non-NULL, provides the template parameters when we're parsing a
-/// C++ template-declaration.
+/// Parse either a function-definition or a declaration.  We can't tell which
+/// we have until we read up to the compound-statement in function-definition.
+/// TemplateParams, if non-NULL, provides the template parameters when we're
+/// parsing a C++ template-declaration.
 ///
 ///   function-definition: [C99 6.9.1]
 /// decl-specs  declarator declaration-list[opt] compound-statement
@@ -891,6 +889,7 @@
 Parser::ParseDeclOrFunctionDefInternal(ParsedAttributesWithRange &attrs,
ParsingDeclSpec &DS,
AccessSpecifier AS) {
+  MaybeParseMicrosoftAttributes(DS.getAttributes());
   // Parse the common declaration-specifiers piece.
   ParseDeclarationSpecifiers(DS, ParsedTemplateInfo(), AS, DSC_top_level);
 
@@ -970,7 +969,7 @@
 // parsing c constructs and re-enter objc container scope
 // afterwards.
 ObjCDeclContextSwitch ObjCDC(*this);
-  
+
 return ParseDeclOrFunctionDefInternal(attrs, PDS, AS);
   }
 }
@@ -2006,7 +2005,6 @@
   while (Tok.isNot(tok::r_brace) && !isEofOrEom()) {
 ParsedAttributesWithRange attrs(AttrFactory);
 MaybeParseCXX11Attributes(attrs);
-MaybeParseMicrosoftAttributes(attrs);
 DeclGroupPtrTy Result = ParseExternalDeclaration(attrs);
 if (Result && !getCurScope()->getParent())
   Actions.getASTConsumer().HandleTopLevelDecl(Result.get());
Index: lib/Parse/ParseOpenMP.cpp
===
--- lib/Parse/ParseOpenMP.cpp
+++ lib/Parse/ParseOpenMP.cpp
@@ -610,7 +610,6 @@
   if (AS == AS_none) {
 assert(TagType == DeclSpec::TST_unspecified);

[PATCH] D23905: [Modules] Add 'gnuinlineasm' to the 'requires-declaration' feature-list.

2016-08-25 Thread Bruno Cardoso Lopes via cfe-commits
bruno created this revision.
bruno added a reviewer: rsmith.
bruno added subscribers: cfe-commits, eladcohen.

This adds support for modules that require (no-)gnu-inline-asm
environment, such as the compiler builtin cpuid submodule.

This is the gnu-inline-asm variant of https://reviews.llvm.org/D23871

https://reviews.llvm.org/D23905

Files:
  docs/Modules.rst
  lib/Basic/Module.cpp
  lib/Headers/module.modulemap
  
test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/Headers/NeedsGNUAsmInline.h
  test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/Headers/asm.h
  test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/module.map
  test/Modules/requires-gnuasminline.m

Index: test/Modules/requires-gnuasminline.m
===
--- /dev/null
+++ test/Modules/requires-gnuasminline.m
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules \
+// RUN: -fimplicit-module-maps -F %S/Inputs/GNUAsm %s \
+// RUN: -fno-gnu-inline-asm -verify
+
+@import NeedsGNUAsmInline.Asm; // expected-error{{module 
'NeedsGNUAsmInline.Asm' requires feature 'gnuasminline'}}
Index: test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/module.map
===
--- /dev/null
+++ test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/module.map
@@ -0,0 +1,8 @@
+framework module NeedsGNUAsmInline {
+  header "NeedsGNUAsmInline.h"
+
+  explicit module Asm {
+requires gnuasminline
+header "asm.h"
+  }
+}
Index: test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/Headers/asm.h
===
--- /dev/null
+++ test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/Headers/asm.h
@@ -0,0 +1 @@
+__asm("foo");
Index: 
test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/Headers/NeedsGNUAsmInline.h
===
--- /dev/null
+++ 
test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/Headers/NeedsGNUAsmInline.h
@@ -0,0 +1 @@
+// NeedsGNUAsmInline.h
Index: lib/Headers/module.modulemap
===
--- lib/Headers/module.modulemap
+++ lib/Headers/module.modulemap
@@ -68,6 +68,7 @@
 }
 
 explicit module cpuid {
+  requires gnuinlineasm
   header "cpuid.h"
 }
 
Index: lib/Basic/Module.cpp
===
--- lib/Basic/Module.cpp
+++ lib/Basic/Module.cpp
@@ -64,6 +64,7 @@
 .Case("blocks", LangOpts.Blocks)
 .Case("cplusplus", LangOpts.CPlusPlus)
 .Case("cplusplus11", LangOpts.CPlusPlus11)
+.Case("gnuinlineasm", LangOpts.GNUAsm)
 .Case("objc", LangOpts.ObjC1)
 .Case("objc_arc", LangOpts.ObjCAutoRefCount)
 .Case("opencl", LangOpts.OpenCL)
Index: docs/Modules.rst
===
--- docs/Modules.rst
+++ docs/Modules.rst
@@ -413,6 +413,9 @@
 cplusplus11
   C++11 support is available.
 
+gnuinlineasm
+  GNU inline ASM is available.
+
 objc
   Objective-C support is available.
 


Index: test/Modules/requires-gnuasminline.m
===
--- /dev/null
+++ test/Modules/requires-gnuasminline.m
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules \
+// RUN: -fimplicit-module-maps -F %S/Inputs/GNUAsm %s \
+// RUN: -fno-gnu-inline-asm -verify
+
+@import NeedsGNUAsmInline.Asm; // expected-error{{module 'NeedsGNUAsmInline.Asm' requires feature 'gnuasminline'}}
Index: test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/module.map
===
--- /dev/null
+++ test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/module.map
@@ -0,0 +1,8 @@
+framework module NeedsGNUAsmInline {
+  header "NeedsGNUAsmInline.h"
+
+  explicit module Asm {
+requires gnuasminline
+header "asm.h"
+  }
+}
Index: test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/Headers/asm.h
===
--- /dev/null
+++ test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/Headers/asm.h
@@ -0,0 +1 @@
+__asm("foo");
Index: test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/Headers/NeedsGNUAsmInline.h
===
--- /dev/null
+++ test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/Headers/NeedsGNUAsmInline.h
@@ -0,0 +1 @@
+// NeedsGNUAsmInline.h
Index: lib/Headers/module.modulemap
===
--- lib/Headers/module.modulemap
+++ lib/Headers/module.modulemap
@@ -68,6 +68,7 @@
 }
 
 explicit module cpuid {
+  requires gnu

Re: [PATCH] D23859: [Preprocessor] Add a macro for -fno-gnu-inline-asm

2016-08-25 Thread Bruno Cardoso Lopes via cfe-commits
bruno abandoned this revision.
bruno added a comment.

Abandon in favor of https://reviews.llvm.org/D23905


https://reviews.llvm.org/D23859



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


Re: [PATCH] D22227: [ubsan] Disable bounds-check for flexible array ivars

2016-08-25 Thread Vedant Kumar via cfe-commits
vsk added a comment.

Ping.


https://reviews.llvm.org/D7



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


Re: [PATCH] D23279: clang-reorder-fields

2016-08-25 Thread Alexander Shaposhnikov via cfe-commits
alexshap added a comment.

F2334952: pingpingpingping 


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



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


Re: [PATCH] D23905: [Modules] Add 'gnuinlineasm' to the 'requires-declaration' feature-list.

2016-08-25 Thread Richard Smith via cfe-commits
On 25 Aug 2016 7:37 p.m., "Bruno Cardoso Lopes" 
wrote:

bruno created this revision.
bruno added a reviewer: rsmith.
bruno added subscribers: cfe-commits, eladcohen.

This adds support for modules that require (no-)gnu-inline-asm
environment, such as the compiler builtin cpuid submodule.

This is the gnu-inline-asm variant of https://reviews.llvm.org/D23871

https://reviews.llvm.org/D23905

Files:
  docs/Modules.rst
  lib/Basic/Module.cpp
  lib/Headers/module.modulemap
  test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/
Headers/NeedsGNUAsmInline.h
  test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/Headers/asm.h
  test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/module.map
  test/Modules/requires-gnuasminline.m

Index: test/Modules/requires-gnuasminline.m
===
--- /dev/null
+++ test/Modules/requires-gnuasminline.m
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules \
+// RUN: -fimplicit-module-maps -F %S/Inputs/GNUAsm %s \
+// RUN: -fno-gnu-inline-asm -verify
+
+@import NeedsGNUAsmInline.Asm; // expected-error{{module
'NeedsGNUAsmInline.Asm' requires feature 'gnuasminline'}}


You should add a positive test for the case where inline asm is available,
too...

Index: test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/module.map
===
--- /dev/null
+++ test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/module.map
@@ -0,0 +1,8 @@
+framework module NeedsGNUAsmInline {
+  header "NeedsGNUAsmInline.h"
+
+  explicit module Asm {
+requires gnuasminline


... that way, you'd catch this typo in the test :)

+header "asm.h"
+  }
+}
Index: test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/Headers/asm.h
===
--- /dev/null
+++ test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/Headers/asm.h
@@ -0,0 +1 @@
+__asm("foo");
Index: test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/
Headers/NeedsGNUAsmInline.h
===
--- /dev/null
+++ test/Modules/Inputs/GNUAsm/NeedsGNUAsmInline.framework/
Headers/NeedsGNUAsmInline.h
@@ -0,0 +1 @@
+// NeedsGNUAsmInline.h
Index: lib/Headers/module.modulemap
===
--- lib/Headers/module.modulemap
+++ lib/Headers/module.modulemap
@@ -68,6 +68,7 @@
 }

 explicit module cpuid {
+  requires gnuinlineasm
   header "cpuid.h"
 }

Index: lib/Basic/Module.cpp
===
--- lib/Basic/Module.cpp
+++ lib/Basic/Module.cpp
@@ -64,6 +64,7 @@
 .Case("blocks", LangOpts.Blocks)
 .Case("cplusplus", LangOpts.CPlusPlus)
 .Case("cplusplus11", LangOpts.CPlusPlus11)
+.Case("gnuinlineasm", LangOpts.GNUAsm)
 .Case("objc", LangOpts.ObjC1)
 .Case("objc_arc", LangOpts.ObjCAutoRefCount)
 .Case("opencl", LangOpts.OpenCL)
Index: docs/Modules.rst
===
--- docs/Modules.rst
+++ docs/Modules.rst
@@ -413,6 +413,9 @@
 cplusplus11
   C++11 support is available.

+gnuinlineasm
+  GNU inline ASM is available.
+
 objc
   Objective-C support is available.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23895: [ms] Add support for parsing uuid as a MS attribute

2016-08-25 Thread Kim Gräsman via cfe-commits
Microsoft used to call their IDL dialect 'MIDL' (which is kind of punny),
don't know if it makes sense to stick with that over 'MSIDL'.

- Kim

Den 26 aug. 2016 4:09 fm skrev "Nico Weber via cfe-commits" <
cfe-commits@lists.llvm.org>:

> thakis updated this revision to Diff 69312.
> thakis marked an inline comment as done.
> thakis added a comment.
>
> comments round 1
>
>
> https://reviews.llvm.org/D23895
>
> Files:
>   include/clang/Basic/Attr.td
>   include/clang/Basic/Attributes.h
>   include/clang/Parse/Parser.h
>   include/clang/Sema/AttributeList.h
>   lib/Parse/ParseDecl.cpp
>   lib/Parse/ParseDeclCXX.cpp
>   lib/Parse/ParseObjc.cpp
>   lib/Parse/ParseOpenMP.cpp
>   lib/Parse/Parser.cpp
>   test/Parser/MicrosoftExtensions.cpp
>   test/Sema/MicrosoftExtensions.c
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D23907: Fix documentation of MemberExpr::getMemberDecl

2016-08-25 Thread Stephan Bergmann via cfe-commits
sberg created this revision.
sberg added reviewers: rsmith, doug.gregor.
sberg added a subscriber: cfe-commits.

https://reviews.llvm.org/D23907

Files:
  include/clang/AST/Expr.h

Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -2406,8 +2406,8 @@
 
   /// \brief Retrieve the member declaration to which this expression refers.
   ///
-  /// The returned declaration will either be a FieldDecl or (in C++)
-  /// a CXXMethodDecl.
+  /// The returned declaration will be a FieldDecl or (in C++) a VarDecl (for
+  /// static data members), a CXXMethodDecl, or an EnumConstantDecl.
   ValueDecl *getMemberDecl() const { return MemberDecl; }
   void setMemberDecl(ValueDecl *D) { MemberDecl = D; }
 


Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -2406,8 +2406,8 @@
 
   /// \brief Retrieve the member declaration to which this expression refers.
   ///
-  /// The returned declaration will either be a FieldDecl or (in C++)
-  /// a CXXMethodDecl.
+  /// The returned declaration will be a FieldDecl or (in C++) a VarDecl (for
+  /// static data members), a CXXMethodDecl, or an EnumConstantDecl.
   ValueDecl *getMemberDecl() const { return MemberDecl; }
   void setMemberDecl(ValueDecl *D) { MemberDecl = D; }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files

2016-08-25 Thread Mads Ravn via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL279803: [clang-tidy] Added hh, hxx and hpp to header guard 
checks. (authored by madsravn).

Changed prior to commit:
  https://reviews.llvm.org/D20512?vs=69250&id=69320#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D20512

Files:
  clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
  clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
  clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.h
  clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-header-guard.rst

Index: clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -12,8 +12,8 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 namespace tidy {
@@ -41,6 +41,10 @@
HeaderFileExtensionsSet &HeaderFileExtensions,
char delimiter);
 
+/// \brief Decides whether a file has a header file extension.
+bool isHeaderFileExtension(StringRef FileName,
+   HeaderFileExtensionsSet HeaderFileExtensions);
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
@@ -61,6 +61,15 @@
   return true;
 }
 
+bool isHeaderFileExtension(StringRef FileName,
+   HeaderFileExtensionsSet HeaderFileExtensions) {
+  StringRef extension = ::llvm::sys::path::extension(FileName);
+  if (extension.startswith("."))
+extension = extension.substr(1);
+
+  return HeaderFileExtensions.count(extension) > 0;
+}
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.h
@@ -11,16 +11,28 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARD_H
 
 #include "../ClangTidy.h"
+#include "../utils/HeaderFileExtensionsUtils.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 
 /// Finds and fixes header guards.
+/// The check supports these options:
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
+/// header files (The filename extension should not contain "." prefix).
+/// ",h,hh,hpp,hxx" by default.
+/// For extension-less header files, using an empty string or leaving an
+/// empty string between "," if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+RawStringHeaderFileExtensions(
+Options.getLocalOrGlobal("HeaderFileExtensions", ",h,hh,hpp,hxx")) {
+utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+ HeaderFileExtensions, ',');
+  }
   void registerPPCallbacks(CompilerInstance &Compiler) override;
 
   /// Returns ``true`` if the check should suggest inserting a trailing comment
@@ -39,6 +51,10 @@
   /// Gets the canonical header guard for a file.
   virtual std::string getHeaderGuard(StringRef Filename,
  StringRef OldGuard = StringRef()) = 0;
+
+private:
+  std::string RawStringHeaderFileExtensions;
+  utils::HeaderFileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace utils
Index: clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
@@ -20,7 +20,7 @@
 
 /// \brief canonicalize a path by removing ./ and ../ components.
 static std::string cleanPath(StringRef Path) {
-  SmallString<256> Result =  Path;
+  SmallString<256> Result = Path;
   llvm::sys::path::remove_dots(Result, true);
   return Result.str();
 }
@@ -274,13 +274,13 @@
 }
 
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtensi

[clang-tools-extra] r279803 - [clang-tidy] Added hh, hxx and hpp to header guard checks.

2016-08-25 Thread Mads Ravn via cfe-commits
Author: madsravn
Date: Fri Aug 26 00:59:53 2016
New Revision: 279803

URL: http://llvm.org/viewvc/llvm-project?rev=279803&view=rev
Log:
[clang-tidy] Added hh, hxx and hpp to header guard checks.

Changed the extension check to include the option of ",h,hh,hpp,hxx" instead of 
just returning whether the file ended with ".h".

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

Modified:
clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.cpp
clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.h
clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.h
clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.h
clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-header-guard.rst

Modified: clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.cpp?rev=279803&r1=279802&r2=279803&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.cpp Fri Aug 26 
00:59:53 2016
@@ -13,8 +13,8 @@ namespace clang {
 namespace tidy {
 namespace llvm {
 
-bool LLVMHeaderGuardCheck::shouldFixHeaderGuard(StringRef Filename) {
-  return Filename.endswith(".h");
+bool LLVMHeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) {
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 std::string LLVMHeaderGuardCheck::getHeaderGuard(StringRef Filename,

Modified: clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.h?rev=279803&r1=279802&r2=279803&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.h Fri Aug 26 
00:59:53 2016
@@ -17,13 +17,30 @@ namespace tidy {
 namespace llvm {
 
 /// Finds and fixes header guards that do not adhere to LLVM style.
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/llvm-header-guard.html
+/// The check supports these options:
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions 
of
+/// header files (The filename extension should not contain "." prefix).
+/// ",h,hh,hpp,hxx" by default.
+/// For extension-less header files, using an empty string or leaving an
+/// empty string between "," if there are other filename extensions.
 class LLVMHeaderGuardCheck : public utils::HeaderGuardCheck {
 public:
   LLVMHeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
-  : HeaderGuardCheck(Name, Context) {}
+  : HeaderGuardCheck(Name, Context),
+RawStringHeaderFileExtensions(
+Options.getLocalOrGlobal("HeaderFileExtensions", ",h,hh,hpp,hxx")) 
{
+utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+ HeaderFileExtensions, ',');
+  }
   bool shouldSuggestEndifComment(StringRef Filename) override { return false; }
   bool shouldFixHeaderGuard(StringRef Filename) override;
   std::string getHeaderGuard(StringRef Filename, StringRef OldGuard) override;
+
+private:
+  std::string RawStringHeaderFileExtensions;
+  utils::HeaderFileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace llvm

Modified: clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp?rev=279803&r1=279802&r2=279803&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp Fri 
Aug 26 00:59:53 2016
@@ -61,6 +61,15 @@ bool parseHeaderFileExtensions(StringRef
   return true;
 }
 
+bool isHeaderFileExtension(StringRef FileName,
+   HeaderFileExtensionsSet HeaderFileExtensions) {
+  StringRef extension = ::llvm::sys::path::extension(FileName);
+  if (extension.startswith("."))
+extension = extension.substr(1);
+
+  return HeaderFileExtensions.count(extension) > 0;
+}
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang

Modified: clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.h?rev=279803&r1=279802&r2=279803&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExten

  1   2   >