[PATCH] D51281: [libclang] Return the proper pointee type for 'auto' deduced to pointer

[Michael Wu via Phabricator via cfe-commits]

michaelwu added a comment.

It shouldn't be too hard to make a test using `c-index-test`. 
`-test-print-type` will print out the pointee type if it's valid.




Comment at: tools/libclang/CXType.cpp:448
   break;
+case Type::Auto:
+  TP = cast(TP)->getDeducedType().getTypePtrOrNull();

Seems like this might make sense for `Type::DeducedTemplateSpecialization` too.


https://reviews.llvm.org/D51281



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

[PATCH] D51354: Fix the -print-multi-directory flag to print the selected multilib.

[Jonathan Roelofs via Phabricator via cfe-commits]

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

LGTM. Thanks for fixing this!


Repository:
  rC Clang

https://reviews.llvm.org/D51354



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

[PATCH] D51358: [driver] Do not pass "-flavor old-gnu" option to LLD linker

[Rui Ueyama via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D51358



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

r340889 - Start reserving x18 by default on Android targets.

[Peter Collingbourne via cfe-commits]

Author: pcc
Date: Tue Aug 28 18:38:47 2018
New Revision: 340889

URL: http://llvm.org/viewvc/llvm-project?rev=340889=rev
Log:
Start reserving x18 by default on Android targets.

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

Modified:
cfe/trunk/test/Driver/sanitizer-ld.c

Modified: cfe/trunk/test/Driver/sanitizer-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/sanitizer-ld.c?rev=340889=340888=340889=diff
==
--- cfe/trunk/test/Driver/sanitizer-ld.c (original)
+++ cfe/trunk/test/Driver/sanitizer-ld.c Tue Aug 28 18:38:47 2018
@@ -575,6 +575,9 @@
 // RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
 // RUN: -target arm64-unknown-ios -fuse-ld=ld \
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18 %s
+// RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
+// RUN: -target aarch64-unknown-linux-android -fuse-ld=ld \
+// RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18 %s
 // CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18-NOT: error:
 
 // RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \


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

[PATCH] D45588: Start reserving x18 by default on Android targets.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340889: Start reserving x18 by default on Android targets. 
(authored by pcc, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45588?vs=142448=163003#toc

Repository:
  rC Clang

https://reviews.llvm.org/D45588

Files:
  test/Driver/sanitizer-ld.c


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -575,6 +575,9 @@
 // RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
 // RUN: -target arm64-unknown-ios -fuse-ld=ld \
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18 %s
+// RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
+// RUN: -target aarch64-unknown-linux-android -fuse-ld=ld \
+// RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18 %s
 // CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18-NOT: error:
 
 // RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -575,6 +575,9 @@
 // RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
 // RUN: -target arm64-unknown-ios -fuse-ld=ld \
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18 %s
+// RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
+// RUN: -target aarch64-unknown-linux-android -fuse-ld=ld \
+// RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18 %s
 // CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18-NOT: error:
 
 // RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

It says the type of the assignment expression, not the type of the LHS.

C11 [6.5.16]p2: "The type of an assignment expression is the type the left 
operand would have after lvalue conversion."

C11 [6.3.2.1]p2: "...this is called lvalue conversion. If the lvalue has 
qualified type, the value has the unqualified version of the type of the 
lvalue; additionally, if the lvalue has atomic type, the value has the 
non-atomic version of the type of the lvalue; otherwise, the value has the type 
of the lvalue."

The RHS is not converted to have atomic type.


Repository:
  rC Clang

https://reviews.llvm.org/D51084



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

[PATCH] D51007: Test the cross-product of options that affect how libgcc-related arguments are passed to the linker.

[Eric Christopher via Phabricator via cfe-commits]

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

Feel free to add any tests like this that test existing behavior without review 
in the future. If we want to change the behavior we should probably review that 
though :)

-eric


Repository:
  rC Clang

https://reviews.llvm.org/D51007



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

[PATCH] D51395: [analyzer] Dump a reproducible, deterministic ID of program state to exploded graph

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov created this revision.
george.karpenkov added reviewers: dcoughlin, NoQ.
Herald added subscribers: Szelethus, mikhail.ramalho, a.sidorin, mgrang, 
szepet, baloghadamsoftware, xazax.hun.

https://reviews.llvm.org/D51395

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp


Index: clang/lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- clang/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ clang/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -69,6 +69,17 @@
 stateMgr->getStoreManager().decrementReferenceCount(store);
 }
 
+std::pair ProgramState::getID() const {
+  auto Out = getStateManager().Alloc.identifyObject(this);
+  return *Out;
+}
+
+void ProgramState::dumpID(llvm::raw_ostream ) const {
+  auto P = getID();
+  assert(P.second % alignof(ProgramState) == 0 && "Unexpected offset");
+  Out << "(" << P.first << ", " << (P.second / alignof(ProgramState)) << ")";
+}
+
 ProgramStateManager::ProgramStateManager(ASTContext ,
  StoreManagerCreator CreateSMgr,
  ConstraintManagerCreator CreateCMgr,
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3129,8 +3129,9 @@
 }
 
 ProgramStateRef state = N->getState();
-Out << "\\|StateID: " << (const void*) state.get()
-<< " NodeID: " << (const void*) N << "\\|";
+Out << "\\|StateID: ";
+state->dumpID(Out);
+Out << " NodeID: " << (const void*) N << "\\|";
 
 state->printDOT(Out, N->getLocationContext());
 
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -107,6 +107,12 @@
 
   ~ProgramState();
 
+  /// Get a pair of numbers uniquely identifying the state.
+  std::pair getID() const;
+
+  /// Dump a unique ID to a given stream.
+  void dumpID(llvm::raw_ostream ) const;
+
   /// Return the ProgramStateManager associated with this state.
   ProgramStateManager () const {
 return *stateMgr;


Index: clang/lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- clang/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ clang/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -69,6 +69,17 @@
 stateMgr->getStoreManager().decrementReferenceCount(store);
 }
 
+std::pair ProgramState::getID() const {
+  auto Out = getStateManager().Alloc.identifyObject(this);
+  return *Out;
+}
+
+void ProgramState::dumpID(llvm::raw_ostream ) const {
+  auto P = getID();
+  assert(P.second % alignof(ProgramState) == 0 && "Unexpected offset");
+  Out << "(" << P.first << ", " << (P.second / alignof(ProgramState)) << ")";
+}
+
 ProgramStateManager::ProgramStateManager(ASTContext ,
  StoreManagerCreator CreateSMgr,
  ConstraintManagerCreator CreateCMgr,
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3129,8 +3129,9 @@
 }
 
 ProgramStateRef state = N->getState();
-Out << "\\|StateID: " << (const void*) state.get()
-<< " NodeID: " << (const void*) N << "\\|";
+Out << "\\|StateID: ";
+state->dumpID(Out);
+Out << " NodeID: " << (const void*) N << "\\|";
 
 state->printDOT(Out, N->getLocationContext());
 
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -107,6 +107,12 @@
 
   ~ProgramState();
 
+  /// Get a pair of numbers uniquely identifying the state.
+  std::pair getID() const;
+
+  /// Dump a unique ID to a given stream.
+  void dumpID(llvm::raw_ostream ) const;
+
   /// Return the ProgramStateManager associated with this state.
   ProgramStateManager () const {
 return *stateMgr;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51393: Provide a method for generating deterministic IDs for pointers allocated in BumpPtrAllocator

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov created this revision.
george.karpenkov added reviewers: NoQ, bogner, chandlerc, zturner, aprantl.
Herald added a subscriber: mgrang.

There are many "dumping" actions available from the compiler: Clang dumps AST, 
Clang static analyzer dumps generated nodes in the "exploded graph" it 
generates.

However, in many of those dumps raw pointer values are printed for the objects 
of interest, which can considerably complicate debugging due to those values 
being non-reproducible.
This patch adds a method for converting a pointer generated through an 
allocator to a deterministic (up to an architecture) pair of integers, which 
can make the debugging experience much more pleasant. 
(E.g. compare hunting for a value "0xa9273f732" which changes every run to 
hunting for "(1, 23)").

Discussion started at 
http://lists.llvm.org/pipermail/cfe-dev/2018-August/059178.html

Code in collaboration with @NoQ


https://reviews.llvm.org/D51393

Files:
  llvm/include/llvm/Support/Allocator.h


Index: llvm/include/llvm/Support/Allocator.h
===
--- llvm/include/llvm/Support/Allocator.h
+++ llvm/include/llvm/Support/Allocator.h
@@ -21,6 +21,7 @@
 #ifndef LLVM_SUPPORT_ALLOCATOR_H
 #define LLVM_SUPPORT_ALLOCATOR_H
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -283,6 +284,28 @@
 
   size_t GetNumSlabs() const { return Slabs.size() + CustomSizedSlabs.size(); }
 
+  /// \return A reproducible pair of objects, uniquely identifying
+  /// an input pointer \p Ptr in the given allocator.
+  /// Returns an empty optional if the pointer is not found in the allocator.
+  llvm::Optional> identifyObject(const void *Ptr) {
+const char *P = reinterpret_cast(Ptr);
+for (size_t Idx=0; Idx < Slabs.size(); Idx++) {
+  const char *S = reinterpret_cast(Slabs[Idx]);
+  if (P >= S && P < S + computeSlabSize(Idx))
+return std::make_pair(Idx, P - S);
+}
+for (size_t Idx=0; Idx < CustomSizedSlabs.size(); Idx++) {
+  const char *S =
+  reinterpret_cast(CustomSizedSlabs[Idx].first);
+  size_t Size = CustomSizedSlabs[Idx].second;
+
+  // Use negative index to denote custom sized slabs.
+  if (P >= S && P < S + Size)
+return std::make_pair(-Idx - 1, P - S);
+}
+return None;
+  }
+
   size_t getTotalMemory() const {
 size_t TotalMemory = 0;
 for (auto I = Slabs.begin(), E = Slabs.end(); I != E; ++I)


Index: llvm/include/llvm/Support/Allocator.h
===
--- llvm/include/llvm/Support/Allocator.h
+++ llvm/include/llvm/Support/Allocator.h
@@ -21,6 +21,7 @@
 #ifndef LLVM_SUPPORT_ALLOCATOR_H
 #define LLVM_SUPPORT_ALLOCATOR_H
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -283,6 +284,28 @@
 
   size_t GetNumSlabs() const { return Slabs.size() + CustomSizedSlabs.size(); }
 
+  /// \return A reproducible pair of objects, uniquely identifying
+  /// an input pointer \p Ptr in the given allocator.
+  /// Returns an empty optional if the pointer is not found in the allocator.
+  llvm::Optional> identifyObject(const void *Ptr) {
+const char *P = reinterpret_cast(Ptr);
+for (size_t Idx=0; Idx < Slabs.size(); Idx++) {
+  const char *S = reinterpret_cast(Slabs[Idx]);
+  if (P >= S && P < S + computeSlabSize(Idx))
+return std::make_pair(Idx, P - S);
+}
+for (size_t Idx=0; Idx < CustomSizedSlabs.size(); Idx++) {
+  const char *S =
+  reinterpret_cast(CustomSizedSlabs[Idx].first);
+  size_t Size = CustomSizedSlabs[Idx].second;
+
+  // Use negative index to denote custom sized slabs.
+  if (P >= S && P < S + Size)
+return std::make_pair(-Idx - 1, P - S);
+}
+return None;
+  }
+
   size_t getTotalMemory() const {
 size_t TotalMemory = 0;
 for (auto I = Slabs.begin(), E = Slabs.end(); I != E; ++I)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:10668
+  if (Source->isAtomicType() || Target->isAtomicType())
+S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst);
+

rjmccall wrote:
> jfb wrote:
> > rjmccall wrote:
> > > Why would the target be an atomic type?  And if it is an atomic type, 
> > > isn't that an initialization of a temporary?  In what situation does it 
> > > make sense to order the initialization of a temporary?
> > In this case:
> > 
> > ```
> > void bad_assign_1(int i) {
> >   atom = i; // expected-warning {{implicit use of sequentially-consistent 
> > atomic may incur stronger memory barriers than necessary}}
> > }
> > ```
> > 
> > We want to warn on assignment to atomics being implicitly `seq_cst`.
> > 
> > Though you're right, initialization shouldn't warn because it isn't atomic. 
> > The issue is that `ATOMIC_VAR_INIT` is what needs to get used, and that's 
> > weird to test. I'll add a test that just assigns (which is what 
> > `ATOMIC_VAR_INIT` expands to for clang), and I'll need to update the code 
> > to detect that pattern and avoid warning in that case. I guess I have to 
> > look at the `Expr` and figure out if the LHS is a `Decl` or something like 
> > that.
> Do we really implicitly convert the RHS of that assignment to atomic type?  
> That seems wrong; `_Atomic` is really a type qualifier, and the RHS should 
> not be converted to `_Atomic(T)` any more than it would be converted to 
> `volatile T` for an assignment into a `volatile` l-value.
I don't make the rules man! I just enforce (and try to warn) on them!

C17:

> 6.5.16.1 Simple assignment
> **Constraints**
> One of the following shall hold: 114)
> — the left operand has atomic, qualified, or unqualified arithmetic type, and 
> the right has arithmetic type;
> — the left operand has an atomic, qualified, or unqualified version of a 
> structure or union type compatible with the type of the right;
> — the left operand has atomic, qualified, or unqualified pointer type, and 
> (considering the type the left operand would have after lvalue conversion) 
> both operands are pointers to qualified or unqualified versions of compatible 
> types, and the type pointed to by the left has all the qualifiers of the type 
> pointed to by the right;
> — the left operand has atomic, qualified, or unqualified pointer type, and 
> (considering the type the left operand would have after lvalue conversion) 
> one operand is a pointer to an object type, and the other is a pointer to a 
> qualified or unqualified version of void, and the type pointed to by the left 
> has all the qualifiers of the type pointed to by the right;
> — the left operand is an atomic, qualified, or unqualified pointer, and the 
> right is a null pointer constant; or
> — the left operand has type atomic, qualified, or unqualified _Bool, and the 
> right is a pointer.
>
> **Semantics**
> In simple assignment (=), the value of the right operand is converted to the 
> type of the assignment expression and replaces the value stored in the object 
> designated by the left operand.
>
> 114)The asymmetric appearance of these constraints with respect to type 
> qualifiers is due to the conversion (specified in 6.3.2.1) that changes 
> lvalues to "the value of the expression" and thus removes any type qualifiers 
> that were applied to the 
> typecategoryoftheexpression(forexample,itremovesconstbutnotvolatilefromthetypeint
>  volatile * const).


Repository:
  rC Clang

https://reviews.llvm.org/D51084



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

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:10668
+  if (Source->isAtomicType() || Target->isAtomicType())
+S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst);
+

jfb wrote:
> rjmccall wrote:
> > Why would the target be an atomic type?  And if it is an atomic type, isn't 
> > that an initialization of a temporary?  In what situation does it make 
> > sense to order the initialization of a temporary?
> In this case:
> 
> ```
> void bad_assign_1(int i) {
>   atom = i; // expected-warning {{implicit use of sequentially-consistent 
> atomic may incur stronger memory barriers than necessary}}
> }
> ```
> 
> We want to warn on assignment to atomics being implicitly `seq_cst`.
> 
> Though you're right, initialization shouldn't warn because it isn't atomic. 
> The issue is that `ATOMIC_VAR_INIT` is what needs to get used, and that's 
> weird to test. I'll add a test that just assigns (which is what 
> `ATOMIC_VAR_INIT` expands to for clang), and I'll need to update the code to 
> detect that pattern and avoid warning in that case. I guess I have to look at 
> the `Expr` and figure out if the LHS is a `Decl` or something like that.
Do we really implicitly convert the RHS of that assignment to atomic type?  
That seems wrong; `_Atomic` is really a type qualifier, and the RHS should not 
be converted to `_Atomic(T)` any more than it would be converted to `volatile 
T` for an assignment into a `volatile` l-value.


Repository:
  rC Clang

https://reviews.llvm.org/D51084



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

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:10668
+  if (Source->isAtomicType() || Target->isAtomicType())
+S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst);
+

rjmccall wrote:
> Why would the target be an atomic type?  And if it is an atomic type, isn't 
> that an initialization of a temporary?  In what situation does it make sense 
> to order the initialization of a temporary?
In this case:

```
void bad_assign_1(int i) {
  atom = i; // expected-warning {{implicit use of sequentially-consistent 
atomic may incur stronger memory barriers than necessary}}
}
```

We want to warn on assignment to atomics being implicitly `seq_cst`.

Though you're right, initialization shouldn't warn because it isn't atomic. The 
issue is that `ATOMIC_VAR_INIT` is what needs to get used, and that's weird to 
test. I'll add a test that just assigns (which is what `ATOMIC_VAR_INIT` 
expands to for clang), and I'll need to update the code to detect that pattern 
and avoid warning in that case. I guess I have to look at the `Expr` and figure 
out if the LHS is a `Decl` or something like that.


Repository:
  rC Clang

https://reviews.llvm.org/D51084



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

[PATCH] D51391: [clang-cl,PCH] Add support for #pragma hdrstop

[Mike Rice via Phabricator via cfe-commits]

mikerice created this revision.
mikerice added reviewers: thakis, hans.

With clang-cl, when the user specifies /Yc or /Yu without a header
the compiler uses a #pragma hdrstop in the main source file to
determine the end of the PCH.  If a header is specified with /Yc or
/Yu #pragma hdrstop has no effect.

The optional filename argument is not yet supported.


https://reviews.llvm.org/D51391

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/DiagnosticLexKinds.td
  include/clang/Driver/CC1Options.td
  include/clang/Lex/Preprocessor.h
  include/clang/Lex/PreprocessorOptions.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Lex/PPDirectives.cpp
  lib/Lex/Pragma.cpp
  lib/Lex/Preprocessor.cpp
  lib/Parse/ParseAST.cpp
  test/Driver/cl-pch.cpp
  test/PCH/Inputs/pch-hdrstop-use.cpp
  test/PCH/Inputs/pch-no-hdrstop-use.cpp
  test/PCH/pch-hdrstop-err.cpp
  test/PCH/pch-hdrstop-warn.cpp
  test/PCH/pch-hdrstop.cpp
  test/PCH/pch-no-hdrstop.cpp

Index: lib/Parse/ParseAST.cpp
===
--- lib/Parse/ParseAST.cpp
+++ lib/Parse/ParseAST.cpp
@@ -141,26 +141,26 @@
 CleanupParser(ParseOP.get());
 
   S.getPreprocessor().EnterMainSourceFile();
-  if (!S.getPreprocessor().getCurrentLexer()) {
-// If a PCH through header is specified that does not have an include in
-// the source, there won't be any tokens or a Lexer.
-return;
-  }
-
-  P.Initialize();
-
-  Parser::DeclGroupPtrTy ADecl;
   ExternalASTSource *External = S.getASTContext().getExternalSource();
   if (External)
 External->StartTranslationUnit(Consumer);
 
-  for (bool AtEOF = P.ParseFirstTopLevelDecl(ADecl); !AtEOF;
-   AtEOF = P.ParseTopLevelDecl(ADecl)) {
-// If we got a null return and something *was* parsed, ignore it.  This
-// is due to a top-level semicolon, an action override, or a parse error
-// skipping something.
-if (ADecl && !Consumer->HandleTopLevelDecl(ADecl.get()))
-  return;
+  // If a PCH through header is specified that does not have an include in
+  // the source, or a PCH is being created with #pragma hdrstop with nothing
+  // after the pragma, there won't be any tokens or a Lexer.
+  bool HaveLexer = S.getPreprocessor().getCurrentLexer();
+
+  if (HaveLexer) {
+P.Initialize();
+Parser::DeclGroupPtrTy ADecl;
+for (bool AtEOF = P.ParseFirstTopLevelDecl(ADecl); !AtEOF;
+ AtEOF = P.ParseTopLevelDecl(ADecl)) {
+  // If we got a null return and something *was* parsed, ignore it.  This
+  // is due to a top-level semicolon, an action override, or a parse error
+  // skipping something.
+  if (ADecl && !Consumer->HandleTopLevelDecl(ADecl.get()))
+return;
+}
   }
 
   // Process any TopLevelDecls generated by #pragma weak.
@@ -179,7 +179,7 @@
   std::swap(OldCollectStats, S.CollectStats);
   if (PrintStats) {
 llvm::errs() << "\nSTATISTICS:\n";
-P.getActions().PrintStats();
+if (HaveLexer) P.getActions().PrintStats();
 S.getASTContext().PrintStats();
 Decl::PrintStats();
 Stmt::PrintStats();
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2989,16 +2989,6 @@
   // * corresponding file not also passed as /FI
   Arg *YcArg = Args.getLastArg(options::OPT__SLASH_Yc);
   Arg *YuArg = Args.getLastArg(options::OPT__SLASH_Yu);
-  if (YcArg && YcArg->getValue()[0] == '\0') {
-Diag(clang::diag::warn_drv_ycyu_no_arg_clang_cl) << YcArg->getSpelling();
-Args.eraseArg(options::OPT__SLASH_Yc);
-YcArg = nullptr;
-  }
-  if (YuArg && YuArg->getValue()[0] == '\0') {
-Diag(clang::diag::warn_drv_ycyu_no_arg_clang_cl) << YuArg->getSpelling();
-Args.eraseArg(options::OPT__SLASH_Yu);
-YuArg = nullptr;
-  }
   if (YcArg && YuArg && strcmp(YcArg->getValue(), YuArg->getValue()) != 0) {
 Diag(clang::diag::warn_drv_ycyu_different_arg_clang_cl);
 Args.eraseArg(options::OPT__SLASH_Yc);
@@ -4280,11 +4270,12 @@
 // extension of .pch is assumed. "
 if (!llvm::sys::path::has_extension(Output))
   Output += ".pch";
-  } else if (Arg *YcArg = C.getArgs().getLastArg(options::OPT__SLASH_Yc)) {
-Output = YcArg->getValue();
-llvm::sys::path::replace_extension(Output, ".pch");
   } else {
-Output = BaseName;
+Arg *YcArg = C.getArgs().getLastArg(options::OPT__SLASH_Yc);
+if (YcArg)
+  Output = YcArg->getValue();
+if (Output.empty())
+  Output = BaseName;
 llvm::sys::path::replace_extension(Output, ".pch");
   }
   return Output.str();
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1100,10 +1100,18 @@
   StringRef ThroughHeader = YcArg ? YcArg->getValue() : YuArg->getValue();
   if (!isa(JA)) {
 

[PATCH] D51294: Fix Bug 38713: clang-format mishandles a short block after "default:" in a switch statement

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 162989.
owenpan marked an inline comment as done.
owenpan added a comment.

Added a comment suggested by @sammccall


Repository:
  rC Clang

https://reviews.llvm.org/D51294

Files:
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -999,6 +999,24 @@
"  }\n"
"});",
getLLVMStyle()));
+  EXPECT_EQ("switch (n) {\n"
+"case 0: {\n"
+"  return false;\n"
+"}\n"
+"default: {\n"
+"  return true;\n"
+"}\n"
+"}",
+format("switch (n)\n"
+   "{\n"
+   "case 0: {\n"
+   "  return false;\n"
+   "}\n"
+   "default: {\n"
+   "  return true;\n"
+   "}\n"
+   "}",
+   getLLVMStyle()));
   verifyFormat("switch (a) {\n"
"case (b):\n"
"  return;\n"
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -483,6 +483,12 @@
 if (Line.First->isOneOf(tok::kw_else, tok::kw_case) ||
 (Line.First->Next && Line.First->Next->is(tok::kw_else)))
   return 0;
+// default: in switch statement
+if (Line.First->is(tok::kw_default)) {
+  const FormatToken *Tok = Line.First->getNextNonComment();
+  if (Tok && Tok->is(tok::colon))
+return 0;
+}
 if (Line.First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_do, tok::kw_try,
 tok::kw___try, tok::kw_catch, tok::kw___finally,
 tok::kw_for, tok::r_brace, Keywords.kw___except)) {


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -999,6 +999,24 @@
"  }\n"
"});",
getLLVMStyle()));
+  EXPECT_EQ("switch (n) {\n"
+"case 0: {\n"
+"  return false;\n"
+"}\n"
+"default: {\n"
+"  return true;\n"
+"}\n"
+"}",
+format("switch (n)\n"
+   "{\n"
+   "case 0: {\n"
+   "  return false;\n"
+   "}\n"
+   "default: {\n"
+   "  return true;\n"
+   "}\n"
+   "}",
+   getLLVMStyle()));
   verifyFormat("switch (a) {\n"
"case (b):\n"
"  return;\n"
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -483,6 +483,12 @@
 if (Line.First->isOneOf(tok::kw_else, tok::kw_case) ||
 (Line.First->Next && Line.First->Next->is(tok::kw_else)))
   return 0;
+// default: in switch statement
+if (Line.First->is(tok::kw_default)) {
+  const FormatToken *Tok = Line.First->getNextNonComment();
+  if (Tok && Tok->is(tok::colon))
+return 0;
+}
 if (Line.First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_do, tok::kw_try,
 tok::kw___try, tok::kw_catch, tok::kw___finally,
 tok::kw_for, tok::r_brace, Keywords.kw___except)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33314: clang-format: Add option to remove semicolon at end of namespace

[Anatol Pomozov via Phabricator via cfe-commits]

anatol.pomozov added a comment.

Would it be possible to add a Fixer that removes unneeded semicolon after C 
function?


https://reviews.llvm.org/D33314



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

[PATCH] D51294: Fix Bug 38713: clang-format mishandles a short block after "default:" in a switch statement

[Owen Pan via Phabricator via cfe-commits]

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



Comment at: lib/Format/UnwrappedLineFormatter.cpp:486
   return 0;
+if (Line.First->is(tok::kw_default)) {
+  const FormatToken *Tok = Line.First->getNextNonComment();

sammccall wrote:
> Just to check my understanding...
> we want to treat `default` the same as `case`, but the heuristics are 
> different:
>  - `case` should only appear in a switch (but might be followed by a complex 
> expression)
>  - `default` has lots of meanings (but we can disambiguate: check if it's 
> followed by a colon)
> 
> You could consider `// default: in switch statement` above this line.
Exactly!



Comment at: unittests/Format/FormatTest.cpp:1012
+   "{\n"
+   "case 0: {\n"
+   "  return false;\n"

sammccall wrote:
> the intent of this test might be clearer if the cases were formatted as `case 
> 0: { return false; }` on one line
I used the same test case that exposed this bug. Please see it in the bug 
report.

Because of the bug, "case" and "default" case labels were handled differently. 
The former was correctly left alone, but the latter was merged into one line. 
Therefore, the test case checks that neither is merged.


Repository:
  rC Clang

https://reviews.llvm.org/D51294



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

[PATCH] D51390: [analyzer] CallDescription: Improve array management.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, 
rnkovacs.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho, 
baloghadamsoftware.

`CallDescription` constructor now accepts an `ArrayRef`, instead 
of two different constructors for `StringRef` and `std::vector`.

This allows us to write an init-list of a vector of `CallDescription` objects 
as:

  vector V = {
{ { "foo" }, 1 },
{ { "q1", "q2", "bar" }, 2 },
...
  };

achieving uniformity between unqualified and qualified descriptions, while 
still allowing the old syntax of `{ "foo", 1 }` (without braces around "foo"). 
Previously `{ { "foo" }, 1 }` wouldn't have compiled, because the compiler was 
unable to figure out which constructor to call (because `{}` are allowed to be 
omitted or added indefinitely as a redundancy).

As a cost of that, i had to switch from `StringRef`s to raw pointers, because 
you can't automatically convert an initializer-list of C strings to an 
`ArrayRef` of `StringRef`s. But that doesn't seem bad, because `StringRef`s 
aren't that much different from raw pointers, and in practice only static 
strings will be used.

Also squash a mutable copy of that array.


Repository:
  rC Clang

https://reviews.llvm.org/D51390

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  lib/StaticAnalyzer/Core/CallEvent.cpp


Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -371,23 +371,26 @@
   // accuracy.
   if (CD.QualifiedName.size() > 1 && D) {
 const DeclContext *Ctx = D->getDeclContext();
-std::vector QualifiedName = CD.QualifiedName;
-QualifiedName.pop_back();
+// See if we'll be able to match them all.
+size_t NumUnmatched = CD.QualifiedName.size() - 1;
 for (; Ctx && isa(Ctx); Ctx = Ctx->getParent()) {
+  if (NumUnmatched == 0)
+break;
+
   if (const auto *ND = dyn_cast(Ctx)) {
-if (!QualifiedName.empty() && ND->getName() == QualifiedName.back())
-  QualifiedName.pop_back();
+if (ND->getName() == CD.QualifiedName[NumUnmatched - 1])
+  --NumUnmatched;
 continue;
   }
 
   if (const auto *RD = dyn_cast(Ctx)) {
-if (!QualifiedName.empty() && RD->getName() == QualifiedName.back())
-  QualifiedName.pop_back();
+if (RD->getName() == CD.QualifiedName[NumUnmatched - 1])
+  --NumUnmatched;
 continue;
   }
 }
 
-if (!QualifiedName.empty())
+if (NumUnmatched > 0)
   return false;
   }
 
Index: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -82,37 +82,26 @@
   mutable bool IsLookupDone = false;
   // The list of the qualified names used to identify the specified CallEvent,
   // e.g. "{a, b}" represent the qualified names, like "a::b".
-  std::vector QualifiedName;
+  std::vector QualifiedName;
   unsigned RequiredArgs;
 
 public:
   const static unsigned NoArgRequirement = 
std::numeric_limits::max();
 
   /// Constructs a CallDescription object.
   ///
-  /// @param QualifiedName The list of the qualified names of the function that
-  /// will be matched. It does not require the user to provide the full list of
-  /// the qualified name. The more details provided, the more accurate the
-  /// matching.
+  /// @param QualifiedName The list of the name qualifiers of the function that
+  /// will be matched. The user is allowed to skip any of the qualifiers.
+  /// For example, {"std", "basic_string", "c_str"} would match both
+  /// std::basic_string<...>::c_str() and std::__1::basic_string<...>::c_str().
   ///
   /// @param RequiredArgs The number of arguments that is expected to match a
   /// call. Omit this parameter to match every occurrence of call with a given
   /// name regardless the number of arguments.
-  CallDescription(std::vector QualifiedName,
+  CallDescription(ArrayRef QualifiedName,
   unsigned RequiredArgs = NoArgRequirement)
   : QualifiedName(QualifiedName), RequiredArgs(RequiredArgs) {}
 
-  /// Constructs a CallDescription object.
-  ///
-  /// @param FuncName The name of the function that will be matched.
-  ///
-  /// @param RequiredArgs The number of arguments that is expected to match a
-  /// call. Omit this parameter to match every occurrence of call with a given
-  /// name regardless the number of arguments.
-  CallDescription(StringRef FuncName, unsigned RequiredArgs = NoArgRequirement)
-  : CallDescription(std::vector({FuncName}), NoArgRequirement) {
-  }
-
   /// Get the name of the function that this object matches.
   StringRef getFunctionName() const { return QualifiedName.back(); 

[PATCH] D51388: [analyzer] Legalize state manager factory injection.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, 
rnkovacs.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho, 
baloghadamsoftware.

When a checker maintains a program state trait that isn't a simple 
list/set/map, but is a combination of multiple lists/sets/maps (eg., a multimap 
- which may be implemented as a map from something to set of something), 
`ProgramStateManager` only contains the factory for the trait itself. All 
auxiliary lists/sets/maps need a factory to be provided by the checker, which 
is annoying.

So far two checkers wanted a multimap, and both decided to trick the 
`ProgramStateManager` into keeping the auxiliary factory within itself by 
pretending that it's some sort of trait they're interested in, but then never 
using this trait but only using the factory.

I decided to make it legal because this functionality seems useful and i didn't 
want to dig deeply into the template magic around state traits when there's 
already a one-liner solution, though i agree that it should be possible to 
implement this in a more straightforward manner.

One thing that becomes apparent once all pieces are put together is that these 
two checkers are in fact //using the same factory//, because the type that 
identifies it, `ImmutableMap>`, is 
the same. This situation is different from two checkers registering similar 
traits.

I also moved stuff around a bit because it always bothered me.


Repository:
  rC Clang

https://reviews.llvm.org/D51388

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h
  lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -406,9 +406,8 @@
   };
 } // end anonymous namespace
 
-REGISTER_TRAIT_WITH_PROGRAMSTATE(DynamicDispatchBifurcationMap,
- CLANG_ENTO_PROGRAMSTATE_MAP(const MemRegion *,
- unsigned))
+REGISTER_MAP_WITH_PROGRAMSTATE(DynamicDispatchBifurcationMap,
+   const MemRegion *, unsigned)
 
 bool ExprEngine::inlineCall(const CallEvent , const Decl *D,
 NodeBuilder , ExplodedNode *Pred,
Index: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
+++ lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
@@ -25,23 +25,10 @@
 using namespace clang;
 using namespace ento;
 
-using PtrSet = llvm::ImmutableSet;
-
 // Associate container objects with a set of raw pointer symbols.
+REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(PtrSet, SymbolRef);
 REGISTER_MAP_WITH_PROGRAMSTATE(RawPtrMap, const MemRegion *, PtrSet)
 
-// This is a trick to gain access to PtrSet's Factory.
-namespace clang {
-namespace ento {
-template <>
-struct ProgramStateTrait : public ProgramStatePartialTrait {
-  static void *GDMIndex() {
-static int Index = 0;
-return 
-  }
-};
-} // end namespace ento
-} // end namespace clang
 
 namespace {
 
Index: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -178,20 +178,12 @@
 };
 } // End anonymous namespace.
 
-typedef llvm::ImmutableSet SymbolSet;
 
 /// Maps from the symbol for a class instance to the set of
 /// symbols remaining that must be released in -dealloc.
+REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(SymbolSet, SymbolRef)
 REGISTER_MAP_WITH_PROGRAMSTATE(UnreleasedIvarMap, SymbolRef, SymbolSet)
 
-namespace clang {
-namespace ento {
-template<> struct ProgramStateTrait
-:  public ProgramStatePartialTrait {
-  static void *GDMIndex() { static int index = 0; return  }
-};
-}
-}
 
 /// An AST check that diagnose when the class requires a -dealloc method and
 /// is missing one.
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h
@@ -30,8 +30,7 @@
 
   /// Declares a program state trait for type \p Type called \p Name, and
   /// introduce a type named \c NameTy.
-  /// The macro should not be used inside namespaces, or for traits that must
-  /// be accessible from more than one translation unit.
+  /// The macro should not be used inside namespaces.
   #define 

[PATCH] D51385: [analyzer] InnerPointerChecker: Fix a segfault.

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs added a comment.

> Return value of `dyn_cast_or_null` should be checked before use. Otherwise we 
> may put a null pointer into the map as a key and eventually crash in 
> `checkDeadSymbols`.

Hm, so with the last `CallDescription` patch we removed some code here that 
essentially checked if the same region was null before this cast, which means 
two things: a) in the previous version it probably should have been a 
`dyn_cast` instead of `dyn_cast_or_null`, but now that makes it accidentally 
fine, and b) I should have thought about this when that code was removed.

> Reka: Why did we restrict ourselves to `TypedValueRegions` here? While we are 
> mostly interested in local string variables and temporaries, which would of 
> course be typed, i guess there's nothing that prevents us from checking that 
> we don't `delete` or mutate a string in a `SymbolicRegion` somewhere between 
> obtaining and using its inner pointer.

I think the reason is that previously `CallDescription`s didn't match fully 
qualified function names and the type was needed to see if the object was a 
`string`.


Repository:
  rC Clang

https://reviews.llvm.org/D51385



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

[PATCH] D43871: [modules] No longer include stdlib.h from mm_malloc.h.

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

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



Comment at: lib/Headers/mm_malloc.h:42
 extern "C" int posix_memalign(void **__memptr, size_t __alignment, size_t 
__size);
+extern "C" void(free)(void *ptr);
+extern "C" void *(malloc)(size_t size) __attribute__((__malloc__));

Add a space before `(free)`, please. (Here and above.)



Comment at: test/CXX/except/except.spec/Inputs/clang/mm_malloc.h:2
+// missing throw() is allowed in this case as we are in a system header.
+// This is a redeclaration possibly from glibc.
+extern "C" void free(void *ptr);

Do you mean this might be a redeclaration of a glibc function? Presumably this 
file is supposed to correspond to our builtin header, not a glibc one.


https://reviews.llvm.org/D43871



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

[PATCH] D51265: Headers: fix collisions with .h files of other projects

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Can you give the text of the error messages?

If you're seeing a "typedef redefinition with different types" error, libclang 
is getting confused about the target.  And parsing code for the wrong target 
cannot work in general.


Repository:
  rC Clang

https://reviews.llvm.org/D51265



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

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

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

rsmith added a comment.

+@aaron.ballman for attributes-related changes.

It would be easier and more precise to track the macro corresponding to an 
attribute in the `Parser` rather than in `Sema` (and stash it on the 
`ParsedAttr` for consumers such as `Sema` that want it). That way, we still 
have the information about where the attribute delimiters were, and whether 
there were multiple attributes in the same set of delimiter brackets. 
Specifically, in `Parser::ParseGNUAttributes`, if we find that we parsed 
exactly one attribute, look through the source location information for the 
outermost macro expansion that exactly covers the token sequence in question, 
and that's your corresponding macro name. (Note that we don't need to check 
what the expansion of the macro is, nor track what attribute-like macros have 
been defined, to support this approach.)

Sorry for not thinking of this sooner.




Comment at: lib/AST/TypePrinter.cpp:1370
+
+// Remove the underlying address space so it won't be printed.
+SplitQualType SplitTy = T->getModifiedType().split();

leonardchan wrote:
> rsmith wrote:
> > This is unnecessary; just print the modified type here. (The modified type 
> > by definition does not have the attribute applied to it.)
> When you say the modified type, do you mean just the type without it's 
> qualifiers? I wasn't sure if removing all the qualifiers would suffice since 
> there were also other  non-address_space qualifiers that could be printed.
I mean `T->getModifiedType()`, which tracks what the type was before the 
attribute was applied.


Repository:
  rC Clang

https://reviews.llvm.org/D51329



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

r340879 - [X86] Add kadd intrinsics to match gcc and icc.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Tue Aug 28 15:32:14 2018
New Revision: 340879

URL: http://llvm.org/viewvc/llvm-project?rev=340879=rev
Log:
[X86] Add kadd intrinsics to match gcc and icc.

This adds the following intrinsics:
_kadd_mask64
_kadd_mask32
_kadd_mask16
_kadd_mask8

These are missing from the Intel Intrinsics Guide, but are implemented by both 
gcc and icc.

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/avx512bwintrin.h
cfe/trunk/lib/Headers/avx512dqintrin.h
cfe/trunk/test/CodeGen/avx512bw-builtins.c
cfe/trunk/test/CodeGen/avx512dq-builtins.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=340879=340878=340879=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Tue Aug 28 15:32:14 2018
@@ -1737,6 +1737,10 @@ TARGET_BUILTIN(__builtin_ia32_fpclassps5
 TARGET_BUILTIN(__builtin_ia32_fpclasspd512_mask, "UcV8dIiUc", "ncV:512:", 
"avx512dq")
 TARGET_BUILTIN(__builtin_ia32_fpclasssd_mask, "UcV2dIiUc", "ncV:128:", 
"avx512dq")
 TARGET_BUILTIN(__builtin_ia32_fpcla_mask, "UcV4fIiUc", "ncV:128:", 
"avx512dq")
+TARGET_BUILTIN(__builtin_ia32_kaddqi, "UcUcUc", "nc", "avx512dq")
+TARGET_BUILTIN(__builtin_ia32_kaddhi, "UsUsUs", "nc", "avx512dq")
+TARGET_BUILTIN(__builtin_ia32_kaddsi, "UiUiUi", "nc", "avx512bw")
+TARGET_BUILTIN(__builtin_ia32_kadddi, "ULLiULLiULLi", "nc", "avx512bw")
 TARGET_BUILTIN(__builtin_ia32_kandqi, "UcUcUc", "nc", "avx512dq")
 TARGET_BUILTIN(__builtin_ia32_kandhi, "UsUsUs", "nc", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_kandsi, "UiUiUi", "nc", "avx512bw")

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=340879=340878=340879=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Tue Aug 28 15:32:14 2018
@@ -8613,7 +8613,7 @@ static Value *EmitX86MaskLogic(CodeGenFu
 LHS = CGF.Builder.CreateNot(LHS);
 
   return CGF.Builder.CreateBitCast(CGF.Builder.CreateBinOp(Opc, LHS, RHS),
-  CGF.Builder.getIntNTy(std::max(NumElts, 
8U)));
+   Ops[0]->getType());
 }
 
 static Value *EmitX86Select(CodeGenFunction ,
@@ -10031,6 +10031,34 @@ Value *CodeGenFunction::EmitX86BuiltinEx
 return Builder.CreateZExt(Cmp, ConvertType(E->getType()));
   }
 
+  case X86::BI__builtin_ia32_kaddqi:
+  case X86::BI__builtin_ia32_kaddhi:
+  case X86::BI__builtin_ia32_kaddsi:
+  case X86::BI__builtin_ia32_kadddi: {
+Intrinsic::ID IID;
+switch (BuiltinID) {
+default: llvm_unreachable("Unsupported intrinsic!");
+case X86::BI__builtin_ia32_kaddqi:
+  IID = Intrinsic::x86_avx512_kadd_b;
+  break;
+case X86::BI__builtin_ia32_kaddhi:
+  IID = Intrinsic::x86_avx512_kadd_w;
+  break;
+case X86::BI__builtin_ia32_kaddsi:
+  IID = Intrinsic::x86_avx512_kadd_d;
+  break;
+case X86::BI__builtin_ia32_kadddi:
+  IID = Intrinsic::x86_avx512_kadd_q;
+  break;
+}
+
+unsigned NumElts = Ops[0]->getType()->getIntegerBitWidth();
+Value *LHS = getMaskVecValue(*this, Ops[0], NumElts);
+Value *RHS = getMaskVecValue(*this, Ops[1], NumElts);
+Function *Intr = CGM.getIntrinsic(IID);
+Value *Res = Builder.CreateCall(Intr, {LHS, RHS});
+return Builder.CreateBitCast(Res, Ops[0]->getType());
+  }
   case X86::BI__builtin_ia32_kandqi:
   case X86::BI__builtin_ia32_kandhi:
   case X86::BI__builtin_ia32_kandsi:

Modified: cfe/trunk/lib/Headers/avx512bwintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512bwintrin.h?rev=340879=340878=340879=diff
==
--- cfe/trunk/lib/Headers/avx512bwintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512bwintrin.h Tue Aug 28 15:32:14 2018
@@ -143,6 +143,18 @@ _kortest_mask64_u8(__mmask64 __A, __mmas
   return (unsigned char)__builtin_ia32_kortestzdi(__A, __B);
 }
 
+static __inline__ __mmask32 __DEFAULT_FN_ATTRS
+_kadd_mask32(__mmask32 __A, __mmask32 __B)
+{
+  return (__mmask32)__builtin_ia32_kaddsi((__mmask32)__A, (__mmask32)__B);
+}
+
+static __inline__ __mmask64 __DEFAULT_FN_ATTRS
+_kadd_mask64(__mmask64 __A, __mmask64 __B)
+{
+  return (__mmask64)__builtin_ia32_kadddi((__mmask64)__A, (__mmask64)__B);
+}
+
 /* Integer compare */
 
 #define _mm512_cmp_epi8_mask(a, b, p) \

Modified: cfe/trunk/lib/Headers/avx512dqintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512dqintrin.h?rev=340879=340878=340879=diff
==
--- cfe/trunk/lib/Headers/avx512dqintrin.h 

[PATCH] D50564: Add support for SEH unwinding on Windows.

[Charles Davis via Phabricator via cfe-commits]

cdavis5x added a comment.

Ping.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564



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

[PATCH] D50927: [Sema] Remove location from implicit capture init expr

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

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



Comment at: clang/lib/Sema/SemaLambda.cpp:1422-1424
   auto Entity = InitializedEntity::InitializeLambdaCapture(
   Var->getIdentifier(), Field->getType(), Loc);
   InitializationKind InitKind = InitializationKind::CreateDirect(Loc, Loc, 
Loc);

vsk wrote:
> rsmith wrote:
> > Should these locations also be updated to `InitLoc`? If we're modeling the 
> > initialization as happening at the capture-default, we should do that 
> > consistently.
> I've revised the patch to use one location consistently here. The tradeoff is 
> that a few diagnostics now point to CaptureDefaultLoc instead of within the 
> lambda body, but I'm happy to defer to more experienced Sema hands.
Yes, the changed diagnostics are confusing, but the old diagnostics were also 
not great. I think the right thing to do is to keep the "used here" diagnostic 
in its current location, and to add a separate note "implicitly capturing local 
variable 'x' first used here" (or similar) at the point of capture.

The way to do that is to push a `CodeSynthesisContext` for the duration where 
you're synthesizing code to initialize the lambda capture. (That'll insert a 
note at the right place in the note stack.) You'll need to add a new value to 
the `CodeSynthesisContext` enumeration and update 
`Sema::PrintInstantiationStack` to produce the note.

I'm fine with this being done in a follow-up commit (and I'm approving this on 
that basis), but I don't think that the new diagnostic is really good enough 
for the long term (while the old one was bad, the new one no longer gives any 
clue as to why we're calling a copy constructor or for what).


https://reviews.llvm.org/D50927



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

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: lib/AST/TypePrinter.cpp:1370
+
+// Remove the underlying address space so it won't be printed.
+SplitQualType SplitTy = T->getModifiedType().split();

rsmith wrote:
> This is unnecessary; just print the modified type here. (The modified type by 
> definition does not have the attribute applied to it.)
When you say the modified type, do you mean just the type without it's 
qualifiers? I wasn't sure if removing all the qualifiers would suffice since 
there were also other  non-address_space qualifiers that could be printed.


Repository:
  rC Clang

https://reviews.llvm.org/D51329



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

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 162966.
leonardchan marked 3 inline comments as done.

Repository:
  rC Clang

https://reviews.llvm.org/D51329

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/Type.h
  include/clang/Lex/Preprocessor.h
  lib/AST/ASTContext.cpp
  lib/AST/TypePrinter.cpp
  lib/Lex/PPDirectives.cpp
  lib/Sema/SemaType.cpp
  test/Sema/address_space_print_macro.c
  test/Sema/address_spaces.c

Index: test/Sema/address_spaces.c
===
--- test/Sema/address_spaces.c
+++ test/Sema/address_spaces.c
@@ -71,5 +71,5 @@
 
 // Clang extension doesn't forbid operations on pointers to different address spaces.
 char* cmp(_AS1 char *x,  _AS2 char *y) {
-  return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('__attribute__((address_space(1))) char *' and '__attribute__((address_space(2))) char *') which are pointers to non-overlapping address spaces}}
+  return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
 }
Index: test/Sema/address_space_print_macro.c
===
--- /dev/null
+++ test/Sema/address_space_print_macro.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify
+
+#define AS1 __attribute__((address_space(1)))
+#define AS2 __attribute__((address_space(2), annotate("foo")))
+
+#define AS(i) address_space(i)
+#define AS3 __attribute__((AS(3)))
+
+char *cmp(AS1 char *x, AS2 char *y) {
+  return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('AS1 char *' and 'AS2 char *') which are pointers to non-overlapping address spaces}}
+}
+
+__attribute__((address_space(1))) char test_array[10];
+void test3(void) {
+  extern void test3_helper(char *p); // expected-note{{passing argument to parameter 'p' here}}
+  test3_helper(test_array);  // expected-error{{passing '__attribute__((address_space(1))) char *' to parameter of type 'char *' changes address space of pointer}}
+}
+
+char AS2 *test4_array;
+void test4(void) {
+  extern void test3_helper(char *p); // expected-note{{passing argument to parameter 'p' here}}
+  test3_helper(test4_array); // expected-error{{passing 'AS2 char *' to parameter of type 'char *' changes address space of pointer}}
+}
+
+void func() {
+  char AS1 *x;
+  char AS3 *x2;
+  char *y;
+  y = x;  // expected-error{{assigning 'AS1 char *' to 'char *' changes address space of pointer}}
+  y = x2; // expected-error{{assigning 'AS3 char *' to 'char *' changes address space of pointer}}
+}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -240,12 +240,60 @@
 diagnoseBadTypeAttribute(getSema(), *Attr, type);
 }
 
+static bool SourceLocInSourceRange(SourceLocation SpellingLoc,
+   SourceRange Range,
+   const SourceManager ) {
+  SourceLocation RangeStart = Range.getBegin();
+  SourceLocation RangeEnd = Range.getEnd();
+  unsigned LineNo = SM.getSpellingLineNumber(SpellingLoc);
+  unsigned StartLineNo = SM.getSpellingLineNumber(RangeStart);
+  unsigned EndLineNo = SM.getSpellingLineNumber(RangeEnd);
+  unsigned ColNo = SM.getSpellingColumnNumber(SpellingLoc);
+  unsigned StartColNo = SM.getSpellingColumnNumber(RangeStart);
+  unsigned EndColNo = SM.getSpellingColumnNumber(RangeEnd);
+
+  // Outside of lines
+  if (LineNo < StartLineNo || LineNo > EndLineNo)
+return false;
+
+  // On the same line
+  if (LineNo == StartLineNo && ColNo < StartColNo)
+return false;
+  if (LineNo == EndLineNo && ColNo > EndColNo)
+return false;
+
+  return true;
+}
+
 /// Get an attributed type for the given attribute, and remember the Attr
 /// object so that we can attach it to the AttributedTypeLoc.
 QualType getAttributedType(Attr *A, QualType ModifiedType,
QualType EquivType) {
-  QualType T =
-  sema.Context.getAttributedType(A->getKind(), ModifiedType, EquivType);
+  IdentifierInfo *MacroII = nullptr;
+  SourceLocation start = A->getRange().getBegin();
+  if (start.isMacroID()) {
+// Walk the macro expansion
+SourceLocation Loc = start;
+auto  = sema.SourceMgr;
+while (Loc.isMacroID() && !MacroII) {
+  for (const auto  : sema.PP.getAttributeMacros()) {
+SourceRange Range = MacroPair.first;
+if (SourceLocInSourceRange(Loc, Range, sema.SourceMgr)) {
+  // Found the attribute declared in a macro.
+  MacroII = MacroPair.second;
+  break;
+}
+  }
+
+  

[PATCH] D51385: [analyzer] InnerPointerChecker: Fix a segfault.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, 
rnkovacs.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho, 
baloghadamsoftware.

Return value of `dyn_cast_or_null` should be checked before use. Otherwise we 
may put a null pointer into the map as a key and eventually crash in 
`checkDeadSymbols`.

Reka: Why did we restrict ourselves to `TypedValueRegion`s here? While we are 
mostly interested in local string variables and temporaries, which would of 
course be typed, i guess there's nothing that prevents us from checking that we 
don't `delete` or mutate a string in a `SymbolicRegion` somewhere between 
obtaining and using its inner pointer.


Repository:
  rC Clang

https://reviews.llvm.org/D51385

Files:
  lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
  test/Analysis/inner-pointer.cpp


Index: test/Analysis/inner-pointer.cpp
===
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -424,3 +424,7 @@
   *(void **) = c() + 1;
   *b = a; // no-crash
 }
+
+void checkReference(std::string ) {
+  const char *c = s.c_str();
+}
Index: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
+++ lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
@@ -213,6 +213,8 @@
   if (const auto *ICall = dyn_cast()) {
 const auto *ObjRegion = dyn_cast_or_null(
 ICall->getCXXThisVal().getAsRegion());
+if (!ObjRegion)
+  return;
 
 if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) {
   SVal RawPtr = Call.getReturnValue();


Index: test/Analysis/inner-pointer.cpp
===
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -424,3 +424,7 @@
   *(void **) = c() + 1;
   *b = a; // no-crash
 }
+
+void checkReference(std::string ) {
+  const char *c = s.c_str();
+}
Index: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
+++ lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
@@ -213,6 +213,8 @@
   if (const auto *ICall = dyn_cast()) {
 const auto *ObjRegion = dyn_cast_or_null(
 ICall->getCXXThisVal().getAsRegion());
+if (!ObjRegion)
+  return;
 
 if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) {
   SVal RawPtr = Call.getReturnValue();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51382: [MinGW] Don't mark external variables as DSO local

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: lib/CodeGen/CodeGenModule.cpp:737
+// potentially could come from another DLL as DSO local.
+if (GV->hasExternalLinkage() && GV->isDeclaration() &&
+isa(GV) && !GV->isThreadLocal())

I think this linkage and declaration check should be 
`GV->isDeclarationForLinker()`, since that will catch `available_externally` 
globals as well. Those would come from a static data member of a class template 
that has an explicit instantiation declaration:
```
template 
struct Foo {
  static const int x = 42;
};
extern template struct Foo;
const int *bar() {
  return ::x;
}
```
Yep, that does it on x86_64-windows-gnu. We probably want a stub for 
`Foo::x`.

Can you add a test at clang/test/CodeGenCXX/dso-local.cpp for this?


Repository:
  rC Clang

https://reviews.llvm.org/D51382



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

[PATCH] D51378: [OPENMP] Add support for nested 'declare target' directives

[Ravi Narayanaswamy via Phabricator via cfe-commits]

RaviNarayanaswamy added a comment.

Is there a way to tell if the header files have matching omp declare target/omp 
end declare target.  
The reason is if one of the header files is missing a matching omp end declare 
target all files which include it will end up having everything marked with 
declare target


Repository:
  rC Clang

https://reviews.llvm.org/D51378



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

[PATCH] D50892: [analyzer][UninitializedObjectChecker] Correct dynamic type is acquired for record pointees

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

Szelethus added inline comments.



Comment at: test/Analysis/cxx-uninitialized-object-inheritance.cpp:802
+struct DynTBase2 {
+  int x; // expected-note{{uninitialized field 'static_cast(this->bptr)->DynTBase2::x'}}
+};

NoQ wrote:
> Mmm, what's the value of casting to derived type and then specifying that we 
> access the field of the base type anyway? Isn't `this->bptr->x` exactly what 
> the user needs to know(?)
True, but it's a one tough job to write `this->bptr->x` here and also a correct 
note message for...



Comment at: test/Analysis/cxx-uninitialized-object-inheritance.cpp:805
+struct DynTDerived2 : DynTBase2 {
+  int y; // expected-note{{uninitialized field 'static_cast(this->bptr)->y'}}
+};

...this one. I'll see how I could achieve it without disrupting `FieldNode`'s 
interface too much.


https://reviews.llvm.org/D50892



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

[PATCH] D51336: [HIP] Fix output file extension

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340873: [HIP] Fix output file extension (authored by yaxunl, 
committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D51336

Files:
  lib/Driver/Action.cpp
  test/Driver/hip-output-file-name.hip


Index: test/Driver/hip-output-file-name.hip
===
--- test/Driver/hip-output-file-name.hip
+++ test/Driver/hip-output-file-name.hip
@@ -0,0 +1,9 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -c -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: {{.*}}clang-offload-bundler{{.*}}"-outputs=hip-output-file-name.o"
Index: lib/Driver/Action.cpp
===
--- lib/Driver/Action.cpp
+++ lib/Driver/Action.cpp
@@ -382,7 +382,7 @@
 void OffloadBundlingJobAction::anchor() {}
 
 OffloadBundlingJobAction::OffloadBundlingJobAction(ActionList )
-: JobAction(OffloadBundlingJobClass, Inputs, Inputs.front()->getType()) {}
+: JobAction(OffloadBundlingJobClass, Inputs, Inputs.back()->getType()) {}
 
 void OffloadUnbundlingJobAction::anchor() {}
 


Index: test/Driver/hip-output-file-name.hip
===
--- test/Driver/hip-output-file-name.hip
+++ test/Driver/hip-output-file-name.hip
@@ -0,0 +1,9 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -c -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: {{.*}}clang-offload-bundler{{.*}}"-outputs=hip-output-file-name.o"
Index: lib/Driver/Action.cpp
===
--- lib/Driver/Action.cpp
+++ lib/Driver/Action.cpp
@@ -382,7 +382,7 @@
 void OffloadBundlingJobAction::anchor() {}
 
 OffloadBundlingJobAction::OffloadBundlingJobAction(ActionList )
-: JobAction(OffloadBundlingJobClass, Inputs, Inputs.front()->getType()) {}
+: JobAction(OffloadBundlingJobClass, Inputs, Inputs.back()->getType()) {}
 
 void OffloadUnbundlingJobAction::anchor() {}
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r340873 - [HIP] Fix output file extension

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Tue Aug 28 14:09:09 2018
New Revision: 340873

URL: http://llvm.org/viewvc/llvm-project?rev=340873=rev
Log:
[HIP] Fix output file extension

OffloadBundlingJobAction constructor accepts a list of JobAction as inputs.
The host JobAction is the last one. The file type of OffloadBundlingJobAction
should be determined by the host JobAction (the last one) instead of the first
one.

Since HIP emits LLVM bitcode for device compilation, device JobAction has
different file type as host Job Action. This bug causes incorrect output file
extension for HIP.

This patch fixes it by using the last input JobAction (host JobAction) to 
determine
file type of OffloadBundlingJobAction.

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

Added:
cfe/trunk/test/Driver/hip-output-file-name.hip
Modified:
cfe/trunk/lib/Driver/Action.cpp

Modified: cfe/trunk/lib/Driver/Action.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Action.cpp?rev=340873=340872=340873=diff
==
--- cfe/trunk/lib/Driver/Action.cpp (original)
+++ cfe/trunk/lib/Driver/Action.cpp Tue Aug 28 14:09:09 2018
@@ -382,7 +382,7 @@ VerifyPCHJobAction::VerifyPCHJobAction(A
 void OffloadBundlingJobAction::anchor() {}
 
 OffloadBundlingJobAction::OffloadBundlingJobAction(ActionList )
-: JobAction(OffloadBundlingJobClass, Inputs, Inputs.front()->getType()) {}
+: JobAction(OffloadBundlingJobClass, Inputs, Inputs.back()->getType()) {}
 
 void OffloadUnbundlingJobAction::anchor() {}
 

Added: cfe/trunk/test/Driver/hip-output-file-name.hip
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hip-output-file-name.hip?rev=340873=auto
==
--- cfe/trunk/test/Driver/hip-output-file-name.hip (added)
+++ cfe/trunk/test/Driver/hip-output-file-name.hip Tue Aug 28 14:09:09 2018
@@ -0,0 +1,9 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -c -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: {{.*}}clang-offload-bundler{{.*}}"-outputs=hip-output-file-name.o"


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

[PATCH] D51382: [MinGW] Don't mark external variables as DSO local

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: rnk, pcc.

Since MinGW supports automatically importing external variables from DLLs even 
without the DLLImport attribute, we shouldn't mark them as DSO local unless we 
actually know them to be local for sure.

Keep marking thread local variables as DSO local.


Repository:
  rC Clang

https://reviews.llvm.org/D51382

Files:
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGen/dllimport.c
  test/CodeGen/dso-local-executable.c
  test/CodeGenCXX/dllexport.cpp
  test/CodeGenCXX/dllimport-members.cpp
  test/CodeGenCXX/dllimport.cpp

Index: test/CodeGenCXX/dllimport.cpp
===
--- test/CodeGenCXX/dllimport.cpp
+++ test/CodeGenCXX/dllimport.cpp
@@ -78,7 +78,7 @@
 // NB: MSC issues a warning and makes GlobalRedecl3 dllexport. We follow GCC
 // and drop the dllimport with a warning.
 // MSC-DAG: @"?GlobalRedecl3@@3HA" = external dso_local global i32
-// GNU-DAG: @GlobalRedecl3= external dso_local global i32
+// GNU-DAG: @GlobalRedecl3= external global i32
 __declspec(dllimport) extern int GlobalRedecl3;
   extern int GlobalRedecl3; // dllimport ignored
 USEVAR(GlobalRedecl3)
@@ -137,7 +137,7 @@
 USEVAR(VarTmplRedecl2)
 
 // MSC-DAG: @"??$VarTmplRedecl3@UImplicitInst_Imported3HA" = external dso_local global i32
-// GNU-DAG: @_Z14VarTmplRedecl3I21ImplicitInst_ImportedE  = external dso_local global i32
+// GNU-DAG: @_Z14VarTmplRedecl3I21ImplicitInst_ImportedE  = external global i32
 template __declspec(dllimport) extern int VarTmplRedecl3;
 template   extern int VarTmplRedecl3; // dllimport ignored
 USEVAR(VarTmplRedecl3)
Index: test/CodeGenCXX/dllimport-members.cpp
===
--- test/CodeGenCXX/dllimport-members.cpp
+++ test/CodeGenCXX/dllimport-members.cpp
@@ -854,7 +854,7 @@
 // Not importing specialization of a member variable template without explicit
 // dllimport.
 // MSC-DAG: @"??$ImportedStaticVar@UExplicitSpec_NotImported@@@MemVarTmpl@@2HB" = external dso_local constant i32
-// GNU-DAG: @_ZN10MemVarTmpl17ImportedStaticVarI24ExplicitSpec_NotImportedEE   = external dso_local constant i32
+// GNU-DAG: @_ZN10MemVarTmpl17ImportedStaticVarI24ExplicitSpec_NotImportedEE   = external constant i32
 template<> const int MemVarTmpl::ImportedStaticVar;
 USEMV(MemVarTmpl, ImportedStaticVar)
 
Index: test/CodeGenCXX/dllexport.cpp
===
--- test/CodeGenCXX/dllexport.cpp
+++ test/CodeGenCXX/dllexport.cpp
@@ -43,7 +43,7 @@
 
 // M64-DAG: @__ImageBase = external dso_local constant i8
 
-// GNU-DAG: @_ZTVN10__cxxabiv117__class_type_infoE = external dso_local global
+// GNU-DAG: @_ZTVN10__cxxabiv117__class_type_infoE = external global
 
 // dllexport implies a definition.
 // MSC-DAG: @"?GlobalDef@@3HA" = dso_local dllexport global i32 0, align 4
@@ -137,7 +137,7 @@
 // Declarations are not exported.
 
 // MSC-DAG: @"??$VarTmplImplicitDef@UImplicitInst_Exported3HA" = external dso_local global
-// GNU-DAG: @_Z18VarTmplImplicitDefI21ImplicitInst_ExportedE  = external dso_local global
+// GNU-DAG: @_Z18VarTmplImplicitDefI21ImplicitInst_ExportedE  = external global
 template __declspec(dllexport) extern int VarTmplImplicitDef;
 USEVAR(VarTmplImplicitDef)
 
Index: test/CodeGen/dso-local-executable.c
===
--- test/CodeGen/dso-local-executable.c
+++ test/CodeGen/dso-local-executable.c
@@ -9,6 +9,17 @@
 // COFF-DAG: @import_var = external dllimport global i32
 // COFF-DAG: declare dllimport void @import_func()
 
+// RUN: %clang_cc1 -triple x86_64-w64-mingw32 -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap --check-prefix=MINGW %s
+// MINGW-DAG: @bar = external global i32
+// MINGW-DAG: @weak_bar = extern_weak dso_local global i32
+// MINGW-DAG: declare dso_local void @foo()
+// MINGW-DAG: @baz = dso_local global i32 42
+// MINGW-DAG: define dso_local i32* @zed()
+// MINGW-DAG: @thread_var = external dso_local thread_local global i32
+// MINGW-DAG: @local_thread_var = dso_local thread_local global i32 42
+// MINGW-DAG: @import_var = external dllimport global i32
+// MINGW-DAG: declare dllimport void @import_func()
+
 // RUN: %clang_cc1 -triple x86_64-pc-linux -emit-llvm -mrelocation-model static %s -o - | FileCheck -allow-deprecated-dag-overlap --check-prefix=STATIC %s
 // STATIC-DAG: @bar = external dso_local global i32
 // STATIC-DAG: @weak_bar = extern_weak dso_local global i32
Index: test/CodeGen/dllimport.c
===
--- test/CodeGen/dllimport.c
+++ test/CodeGen/dllimport.c
@@ -39,7 +39,8 @@
 
 // NB: MSVC issues a warning and makes GlobalRedecl3 dllexport. We follow GCC
 // and drop the dllimport with a warning.
-// CHECK: @GlobalRedecl3 = external dso_local 

[PATCH] D51381: [clang-tidy] fix check_clang_tidy to properly mix CHECK-NOTES and CHECK-MESSAGES

[Jonas Toth via Phabricator via cfe-commits]

JonasToth created this revision.
JonasToth added reviewers: alexfh, aaron.ballman, lebedev.ri, hokein.
Herald added subscribers: cfe-commits, xazax.hun.

This patch adjusts the check_clang_tidy.py script to tolerate warnings in the
codepath that checks for notes.
Checking for warnings itself should be done with 'CHECK-MESSAGES'. This patch
ensures that mixing 'CHECK-MESSAGES' and 'CHECK-NOTES' work within the same
clang-tidy test file and can be mixed.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51381

Files:
  test/clang-tidy/check_clang_tidy.py


Index: test/clang-tidy/check_clang_tidy.py
===
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -166,7 +166,7 @@
   subprocess.check_output(
   ['FileCheck', '-input-file=' + notes_file, input_file_name,
'-check-prefix=' + check_notes_prefix,
-   '-implicit-check-not={{note|warning|error}}:'],
+   '-implicit-check-not={{note|error}}:'],
   stderr=subprocess.STDOUT)
 except subprocess.CalledProcessError as e:
   print('FileCheck failed:\n' + e.output.decode())


Index: test/clang-tidy/check_clang_tidy.py
===
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -166,7 +166,7 @@
   subprocess.check_output(
   ['FileCheck', '-input-file=' + notes_file, input_file_name,
'-check-prefix=' + check_notes_prefix,
-   '-implicit-check-not={{note|warning|error}}:'],
+   '-implicit-check-not={{note|error}}:'],
   stderr=subprocess.STDOUT)
 except subprocess.CalledProcessError as e:
   print('FileCheck failed:\n' + e.output.decode())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51380: Fix incorrect usage of std::error_category

[Alexandre Ganea via Phabricator via cfe-commits]

aganea created this revision.
aganea added reviewers: ilya-biryukov, zturner.
Herald added a subscriber: cfe-commits.

As mentionned here , this change 
prevents a dangling pointer in `std:error_code` by using a singleton.


Repository:
  rC Clang

https://reviews.llvm.org/D51380

Files:
  lib/Frontend/PrecompiledPreamble.cpp


Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -742,8 +742,10 @@
   return nullptr;
 }
 
+static llvm::ManagedStatic 
BuildPreambleErrCategory;
+
 std::error_code clang::make_error_code(BuildPreambleError Error) {
-  return std::error_code(static_cast(Error), 
BuildPreambleErrorCategory());
+  return std::error_code(static_cast(Error), *BuildPreambleErrCategory);
 }
 
 const char *BuildPreambleErrorCategory::name() const noexcept {


Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -742,8 +742,10 @@
   return nullptr;
 }
 
+static llvm::ManagedStatic BuildPreambleErrCategory;
+
 std::error_code clang::make_error_code(BuildPreambleError Error) {
-  return std::error_code(static_cast(Error), BuildPreambleErrorCategory());
+  return std::error_code(static_cast(Error), *BuildPreambleErrCategory);
 }
 
 const char *BuildPreambleErrorCategory::name() const noexcept {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

[Jonas Toth via Phabricator via cfe-commits]

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

- [Test] use CHECK-NOTES again based on the fix in check_clang_tidy


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48714

Files:
  clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
  test/clang-tidy/hicpp-exception-baseclass.cpp

Index: test/clang-tidy/hicpp-exception-baseclass.cpp
===
--- test/clang-tidy/hicpp-exception-baseclass.cpp
+++ test/clang-tidy/hicpp-exception-baseclass.cpp
@@ -2,6 +2,7 @@
 
 namespace std {
 class exception {};
+class invalid_argument : public exception {};
 } // namespace std
 
 class derived_exception : public std::exception {};
@@ -20,54 +21,54 @@
 void problematic() {
   try {
 throw int(42);
-// CHECK-NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
   } catch (int e) {
   }
   throw int(42);
-  // CHECK-NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
 
   try {
 throw 12;
-// CHECK-NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
   } catch (...) {
 throw; // Ok, even if the type is not known, conforming code can never rethrow a non-std::exception object.
   }
 
   try {
 throw non_derived_exception();
-// CHECK-NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
-// CHECK-NOTES: 9:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+// CHECK-NOTES: 10:1: note: type defined here
   } catch (non_derived_exception ) {
   }
   throw non_derived_exception();
-  // CHECK-NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
-  // CHECK-NOTES: 9:1: note: type defined here
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+  // CHECK-NOTES: 10:1: note: type defined here
 
 // FIXME: More complicated kinds of inheritance should be checked later, but there is
 // currently no way use ASTMatchers for this kind of task.
 #if 0
   // Handle private inheritance cases correctly.
   try {
 throw bad_inheritance();
 // CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
-// CHECK MESSAGES: 10:1: note: type defined here
+// CHECK MESSAGES: 11:1: note: type defined here
 throw no_good_inheritance();
 // CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
-// CHECK MESSAGES: 11:1: note: type defined here
+// CHECK MESSAGES: 12:1: note: type defined here
 throw really_creative();
 // CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
-// CHECK MESSAGES: 12:1: note: type defined here
+// CHECK MESSAGES: 13:1: note: type defined here
   } catch (...) {
   }
   throw bad_inheritance();
   // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
-  // CHECK MESSAGES: 10:1: note: type defined here
+  // CHECK MESSAGES: 11:1: note: type defined here
   throw no_good_inheritance();
   // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
-  // CHECK MESSAGES: 11:1: note: type defined here
+  // CHECK MESSAGES: 12:1: note: type defined here
   throw really_creative();
   // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
-  // CHECK MESSAGES: 12:1: note: type defined here
+  // CHECK MESSAGES: 13:1: note: type defined here
 #endif
 }
 
@@ -91,24 +92,32 @@
   throw deep_hierarchy(); // Ok
 
   try {
-throw terrible_idea(); // Ok, but multiple inheritance isn't clean
+throw terrible_idea();  // Ok, but multiple inheritance isn't clean
   } catch (std::exception ) { // Can be caught as std::exception, even with multiple inheritance
   }
   throw terrible_idea(); // Ok, but multiple inheritance
 }
 
 // Templated function that throws exception based on template type
 template 
 void ThrowException() { throw 

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 162942.
Charusso marked an inline comment as done.
Charusso retitled this revision from "[clang-tidy] New checker for not 
null-terminated result caused by strlen or wcslen" to "[clang-tidy] New checker 
for not null-terminated result caused by strlen(), size() or equal length".
Charusso edited the summary of this revision.
Charusso added a comment.

- The checker by default search for `__STDC_WANT_LIB_EXT1__` macro to determine 
if `_s` suffixed functions are available. This behaviour could be overridden by 
specifying `AreSafeFunctionsAvailable`, which is became a string (it was an 
integer, but it can be used in the same way).

- If the destination array length is based on a custom `malloc()` the current 
approach only want to rewrite it if it is 100% sure the chosen argument is 
specifying the length.

- If the checker works with `size()` function now it is only match for 
`std::string` (e.g. it matched to StringRefs).

- Now it handles macro defined lengths properly.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-in-initialization-strlen.c
  test/clang-tidy/bugprone-not-null-terminated-result-memcpy-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe-cxx.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe-other.c
  test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-wmemcpy-safe-cxx.cpp

Index: test/clang-tidy/bugprone-not-null-terminated-result-wmemcpy-safe-cxx.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wmemcpy-safe-cxx.cpp
@@ -0,0 +1,135 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c++11
+
+#define __STDC_WANT_LIB_EXT1__ 1
+
+typedef unsigned int size_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+template 
+errno_t wcsncpy_s(wchar_t ()[size], const wchar_t *src, size_t length);
+errno_t wcsncpy_s(wchar_t *, size_t, const wchar_t *, size_t);
+
+template 
+wchar_t *wcsncpy(wchar_t ()[size], const wchar_t *src, size_t length);
+wchar_t *wcsncpy(wchar_t *, const wchar_t *, size_t);
+
+template 
+errno_t wcscpy_s(wchar_t ()[size], const wchar_t *);
+errno_t wcscpy_s(wchar_t *, size_t, const wchar_t *);
+
+template 
+wchar_t *wcscpy(wchar_t ()[size], const wchar_t *);
+wchar_t *wcscpy(wchar_t *, const wchar_t *);
+
+errno_t wmemcpy_s(wchar_t *, size_t, const wchar_t *, size_t);
+wchar_t *wmemcpy(wchar_t *, const wchar_t *, size_t);
+
+
+//===--===//
+// wmemcpy() - destination array tests
+//===--===//
+
+void bad_wmemcpy_known_dest(const wchar_t *src) {
+  wchar_t dest01[13];
+  wmemcpy(dest01, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wchar_t dest01[14];
+  // CHECK-FIXES-NEXT: wcscpy_s(dest01, src);
+}
+
+void good_wmemcpy_known_dest(const wchar_t *src) {
+  wchar_t dst01[14];
+  wcscpy_s(dst01, src);
+}
+
+//===--===//
+// wmemcpy() - length tests
+//===--===//
+
+void bad_wmemcpy_full_source_length(const wchar_t *src) {
+  wchar_t dest20[13];
+  wmemcpy(dest20, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wchar_t dest20[14];
+  // CHECK-FIXES-NEXT: wcscpy_s(dest20, src);
+}
+
+void good_wmemcpy_full_source_length(const wchar_t *src) {
+  wchar_t dst20[14];
+  wcscpy_s(dst20, src);
+}
+
+void bad_wmemcpy_partial_source_length(const wchar_t *src) {
+  wchar_t dest21[13];
+  wmemcpy(dest21, src, wcslen(src) - 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wchar_t dest21[14];
+  // CHECK-FIXES-NEXT: wcsncpy_s(dest21, src, wcslen(src) - 1);
+}
+
+void good_wmemcpy_partial_source_length(const wchar_t *src) {
+  wchar_t dst21[14];
+  wcsncpy_s(dst21, src, wcslen(src) - 1);
+}
+

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a subscriber: lebedev.ri.
JonasToth added a comment.

I had to revert the `CHECK-NOTES` change that @lebedev.ri introduced with his 
revision. It fails the test, i think there is an inconsistency or so in the 
check-clang-tidy script. I will try to figure out whats the issue.




Comment at: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp:30-32
+  anyOf(has(expr(hasType(
+substTemplateTypeParmType().bind("templ_type",
+anything()),

aaron.ballman wrote:
> This is a strange formulation where you have `anyOf(..., anything())`; can 
> you explain why that's needed?
I added comments for each part of the matcher. Do you think it clarifies? It is 
just a small hack to conditionally match on something :)

But I honestly had to think a little until i remembered why i did it :D



Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:191
+void templated_thrower() { throw T{}(); }
+// CHECK-MESSAGES: [[@LINE-1]]:34: warning: throwing an exception whose type 
'int' is not derived from 'std::exception'
+

aaron.ballman wrote:
> JonasToth wrote:
> > hokein wrote:
> > > JonasToth wrote:
> > > > JonasToth wrote:
> > > > > JonasToth wrote:
> > > > > > alexfh wrote:
> > > > > > > hokein wrote:
> > > > > > > > I think giving message on the template function here is 
> > > > > > > > confusing to users even it gets instantiated somewhere in this 
> > > > > > > > TU -- because users have to find the location that triggers the 
> > > > > > > > template instantiation.
> > > > > > > > 
> > > > > > > > Maybe 
> > > > > > > > 1) Add a note which gives the instantiation location to the 
> > > > > > > > message, or
> > > > > > > > 2) ignore template case (some clang-tidy checks do this)
> > > > > > > In this particular case it seems to be useful to get warnings 
> > > > > > > from template instantiations. But the message will indeed be 
> > > > > > > confusing.
> > > > > > > 
> > > > > > > Ideally, the message should have "in instantiation of xxx 
> > > > > > > requested here" notes attached, as clang warnings do. But this is 
> > > > > > > not working automatically, and it's implemented in Sema 
> > > > > > > (`Sema::PrintInstantiationStack()` in 
> > > > > > > lib/Sema/SemaTemplateInstantiate.cpp).
> > > > > > > 
> > > > > > > I wonder whether it's feasible to produce similar notes after 
> > > > > > > Sema is dead? Maybe not the whole instantiation stack, but at 
> > > > > > > least it should be possible to figure out that the enclosing 
> > > > > > > function is a template instantiation or is a member function of 
> > > > > > > an type that is an instantiation of a template. That would be 
> > > > > > > useful for other checks as well.
> > > > > > It should be possible to figure out, that the type comes from 
> > > > > > template instantiation and that information could be added to the 
> > > > > > warning.
> > > > > > 
> > > > > > I will take a look at Sema and think about something like this. 
> > > > > > Unfortunatly i dont have a lot of time :/
> > > > > I did look further into the issue, i think it is non-trivial.
> > > > > 
> > > > > The newly added case is not a templated exception perse, but there is 
> > > > > a exception-factory, which is templated, that creates a normal 
> > > > > exception.
> > > > > 
> > > > > I did add another note for template instantiations, but i could not 
> > > > > figure out a way to give better diagnostics for the new use-case.
> > > > @hokein and @alexfh Do you still have your concerns (the exception is 
> > > > not a template value, but the factory creating them) or is this fix 
> > > > acceptable?
> > > I agree this is non-trivial. If we can't find a good solution at the 
> > > moment, I'd prefer to ignore this case instead of adding some workarounds 
> > > in the check, what do you think? 
> > Honestly I would let it as is. This test case is not well readable, but if 
> > we have something similar to
> > 
> > ```
> > template 
> > void SomeComplextFunction() {
> > T ExceptionFactory;
> > 
> >if (SomeCondition) 
> >  throw ExceptionFactory();
> > }
> > ```
> > It is not that bad. And the check is still correct, just the code 
> > triggering this condition just hides whats happening.
> I don't think the diagnostic in this test is too confusing. Having the 
> instantiation stack would be great, but that requires Sema support that we 
> don't have access to, unfortunately.
> 
> The instantiation note currently isn't being printed in the test case, but I 
> suspect that will add a bit of extra clarity to the message.
The template note does not apply here, because the thrown value is not 
templated.



Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:39
 throw non_derived_exception();
+
 // CHECK-NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 
'non_derived_exception' is not derived from 

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

[Jonas Toth via Phabricator via cfe-commits]

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

- [Misc] comment the matcher to better understand it
- [Fix] adjust the test cases to properly function now


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48714

Files:
  clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
  test/clang-tidy/hicpp-exception-baseclass.cpp

Index: test/clang-tidy/hicpp-exception-baseclass.cpp
===
--- test/clang-tidy/hicpp-exception-baseclass.cpp
+++ test/clang-tidy/hicpp-exception-baseclass.cpp
@@ -2,6 +2,7 @@
 
 namespace std {
 class exception {};
+class invalid_argument : public exception {};
 } // namespace std
 
 class derived_exception : public std::exception {};
@@ -20,54 +21,54 @@
 void problematic() {
   try {
 throw int(42);
-// CHECK-NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
   } catch (int e) {
   }
   throw int(42);
-  // CHECK-NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
 
   try {
 throw 12;
-// CHECK-NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
   } catch (...) {
 throw; // Ok, even if the type is not known, conforming code can never rethrow a non-std::exception object.
   }
 
   try {
 throw non_derived_exception();
-// CHECK-NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
-// CHECK-NOTES: 9:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 10:1: note: type defined here
   } catch (non_derived_exception ) {
   }
   throw non_derived_exception();
-  // CHECK-NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
-  // CHECK-NOTES: 9:1: note: type defined here
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+  // CHECK-MESSAGES: 10:1: note: type defined here
 
 // FIXME: More complicated kinds of inheritance should be checked later, but there is
 // currently no way use ASTMatchers for this kind of task.
 #if 0
   // Handle private inheritance cases correctly.
   try {
 throw bad_inheritance();
 // CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
-// CHECK MESSAGES: 10:1: note: type defined here
+// CHECK MESSAGES: 11:1: note: type defined here
 throw no_good_inheritance();
 // CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
-// CHECK MESSAGES: 11:1: note: type defined here
+// CHECK MESSAGES: 12:1: note: type defined here
 throw really_creative();
 // CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
-// CHECK MESSAGES: 12:1: note: type defined here
+// CHECK MESSAGES: 13:1: note: type defined here
   } catch (...) {
   }
   throw bad_inheritance();
   // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
-  // CHECK MESSAGES: 10:1: note: type defined here
+  // CHECK MESSAGES: 11:1: note: type defined here
   throw no_good_inheritance();
   // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
-  // CHECK MESSAGES: 11:1: note: type defined here
+  // CHECK MESSAGES: 12:1: note: type defined here
   throw really_creative();
   // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
-  // CHECK MESSAGES: 12:1: note: type defined here
+  // CHECK MESSAGES: 13:1: note: type defined here
 #endif
 }
 
@@ -91,24 +92,32 @@
   throw deep_hierarchy(); // Ok
 
   try {
-throw terrible_idea(); // Ok, but multiple inheritance isn't clean
+throw terrible_idea();  // Ok, but multiple inheritance isn't clean
   } catch (std::exception ) { // Can be caught as std::exception, even with multiple inheritance
   }
   throw terrible_idea(); // Ok, but multiple inheritance
 }
 
 // Templated function that throws exception based on template 

[PATCH] D51331: [OPENMP] Create non-const ident_t structs.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

When is ITT Notify used? Does it have some preconditions like debug info, some 
optimizations level etc.?


https://reviews.llvm.org/D51331



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

[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal

[Artem Dergachev via Phabricator via cfe-commits]

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

Yup, looks correct to me!




Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:448-449
 
   Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl->getParent(),
 Context.getStackFrame());
 

This totally needs `assert(CtorDecl == Context.getStackFrame()->getDecl())`. 
Otherwise we're in big trouble because we'll be looking into a this-region that 
doesn't exist on this stack frame.

On second thought, though, i guess we should put this assertion into the 
constructor of `CXXThisRegion`. I'll do this.

Also there's an overload of `getCXXThis` that accepts the method itself, no 
need to get parent.


Repository:
  rC Clang

https://reviews.llvm.org/D51300



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

[PATCH] D43871: [modules] No longer include stdlib.h from mm_malloc.h.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor updated this revision to Diff 162940.
teemperor marked an inline comment as done.
teemperor added a comment.

- Reverted unintended change to test case that slipped into the latest diff.


https://reviews.llvm.org/D43871

Files:
  lib/Headers/mm_malloc.h
  lib/Sema/SemaExceptionSpec.cpp
  test/CXX/except/except.spec/Inputs/clang/mm_malloc.h
  test/CXX/except/except.spec/Inputs/clang/module.modulemap
  test/CXX/except/except.spec/Inputs/libc/module.modulemap
  test/CXX/except/except.spec/Inputs/libc/stdlib.h
  test/CXX/except/except.spec/libc-empty-except.cpp

Index: test/CXX/except/except.spec/libc-empty-except.cpp
===
--- /dev/null
+++ test/CXX/except/except.spec/libc-empty-except.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+
+// One of the headers is in a user include, so our redeclaration should fail.
+// RUN: not %clang_cc1 -std=c++11 -I %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -std=c++11 -isystem %S/Inputs/clang -I %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+
+// The same test cases again with enabled modules.
+// The modules cases *all* pass because we marked both as [system].
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:-std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:-std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:-std=c++11 -I %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:-std=c++11 -isystem %S/Inputs/clang -I %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+
+// expected-no-diagnostics
+#ifdef REVERSE
+#include "mm_malloc.h"
+#include "stdlib.h"
+#else
+#include "mm_malloc.h"
+#include "stdlib.h"
+#endif
+
+void f() {
+  free(nullptr);
+}
Index: test/CXX/except/except.spec/Inputs/libc/stdlib.h
===
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/libc/stdlib.h
@@ -0,0 +1,2 @@
+// Declare 'free' like glibc with a empty exception specifier.
+extern "C" void free(void *ptr) throw();
Index: test/CXX/except/except.spec/Inputs/libc/module.modulemap
===
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/libc/module.modulemap
@@ -0,0 +1,4 @@
+module libc [system] {
+  header "stdlib.h"
+  export *
+}
Index: test/CXX/except/except.spec/Inputs/clang/module.modulemap
===
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/clang/module.modulemap
@@ -0,0 +1,4 @@
+module builtin [system] {
+  header "mm_malloc.h"
+  export *
+}
Index: test/CXX/except/except.spec/Inputs/clang/mm_malloc.h
===
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/clang/mm_malloc.h
@@ -0,0 +1,3 @@
+// missing throw() is allowed in this case as we are in a system header.
+// This is a redeclaration possibly from glibc.
+extern "C" void free(void *ptr);
Index: lib/Sema/SemaExceptionSpec.cpp
===
--- lib/Sema/SemaExceptionSpec.cpp
+++ lib/Sema/SemaExceptionSpec.cpp
@@ -236,6 +236,7 @@
 const FunctionProtoType *New, SourceLocation NewLoc,
 bool *MissingExceptionSpecification = nullptr,
 bool *MissingEmptyExceptionSpecification = nullptr,
+bool *ExtraEmptyExceptionSpecification = nullptr,
 bool AllowNoexceptAllMatchWithNoSpec = false, bool IsOperatorNew = false);
 
 /// Determine whether a function has an implicitly-generated exception
@@ -259,6 +260,15 @@
   return !Ty->hasExceptionSpec();
 }
 
+/// Returns true if the given function is a function/builtin with C linkage
+/// and from a system header.
+static bool isCSystemFuncOrBuiltin(FunctionDecl *D, ASTContext ) {
+  

[PATCH] D51378: [OPENMP] Add support for nested 'declare target' directives

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rC Clang

https://reviews.llvm.org/D51378



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

[PATCH] D51378: [OPENMP] Add support for nested 'declare target' directives

[Patrick Lyster via Phabricator via cfe-commits]

patricklyster created this revision.
patricklyster added reviewers: ABataev, Hahnfeld, RaviNarayanaswamy, mikerice, 
kkwli0, hfinkel, gtbercea.
Herald added subscribers: cfe-commits, guansong.

Add the capability to nest multiple declare target directives - including 
header files within a declare target region.


Repository:
  rC Clang

https://reviews.llvm.org/D51378

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/Inputs/declare_target_include.h
  clang/test/OpenMP/declare_target_ast_print.cpp
  clang/test/OpenMP/declare_target_messages.cpp

Index: clang/test/OpenMP/declare_target_messages.cpp
===
--- clang/test/OpenMP/declare_target_messages.cpp
+++ clang/test/OpenMP/declare_target_messages.cpp
@@ -59,8 +59,19 @@
   }
 }
 
+#pragma omp declare target
+  void abc();
+#pragma omp end declare target
+void cba();
+#pragma omp end declare target // expected-error {{unexpected OpenMP directive '#pragma omp end declare target'}}
 
-#pragma omp declare target // expected-note {{to match this '#pragma omp declare target'}}
+#pragma omp declare target  // expected-note {{to match this '#pragma omp declare target'}}
+  #pragma omp declare target
+void def();
+  #pragma omp end declare target
+  void fed();
+
+#pragma omp declare target // expected-note {{to match this '#pragma omp declare target'}} // expected-error {{expected '#pragma omp end declare target'}}
 #pragma omp threadprivate(a) // expected-note {{defined as threadprivate or thread local}}
 extern int b;
 int g;
Index: clang/test/OpenMP/declare_target_ast_print.cpp
===
--- clang/test/OpenMP/declare_target_ast_print.cpp
+++ clang/test/OpenMP/declare_target_ast_print.cpp
@@ -1,10 +1,10 @@
-// RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
-// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -I %S/Inputs -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -I %S/Inputs -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -I %S/Inputs -verify %s -ast-print | FileCheck %s
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s | FileCheck %s
-// RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp-simd -I %S/Inputs -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -I %S/Inputs -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only -I %S/Inputs -verify %s -ast-print | FileCheck %s
 // expected-no-diagnostics
 
 #ifndef HEADER
@@ -169,6 +169,32 @@
 // CHECK: #pragma omp end declare target
 // CHECK: int b;
 
+#pragma omp declare target
+  #include "declare_target_include.h"
+  void xyz();
+#pragma omp end declare target
+
+// CHECK: #pragma omp declare target
+// CHECK: void zyx();
+// CHECK: #pragma omp end declare target
+// CHECK: #pragma omp declare target
+// CHECK: void xyz();
+// CHECK: #pragma omp end declare target
+
+#pragma omp declare target
+  #pragma omp declare target
+void abc();
+  #pragma omp end declare target
+  void cba();
+#pragma omp end declare target
+
+// CHECK: #pragma omp declare target
+// CHECK: void abc();
+// CHECK: #pragma omp end declare target
+// CHECK: #pragma omp declare target
+// CHECK: void cba();
+// CHECK: #pragma omp end declare target
+
 int main (int argc, char **argv) {
   foo();
   foo_c();
Index: clang/test/OpenMP/Inputs/declare_target_include.h
===
--- /dev/null
+++ clang/test/OpenMP/Inputs/declare_target_include.h
@@ -0,0 +1,3 @@
+#pragma omp declare target
+  void zyx();
+#pragma omp end declare target
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -12870,19 +12870,14 @@
 Diag(Loc, diag::err_omp_region_not_file_context);
 return false;
   }
-  if (IsInOpenMPDeclareTargetContext) {
-Diag(Loc, diag::err_omp_enclosed_declare_target);
-return false;
-  }
-
-  IsInOpenMPDeclareTargetContext = true;
+  ++DeclareTargetNestingLevel;
   return true;
 }
 
 void Sema::ActOnFinishOpenMPDeclareTargetDirective() {
-  assert(IsInOpenMPDeclareTargetContext &&
+  assert(DeclareTargetNestingLevel > 0 &&
  "Unexpected ActOnFinishOpenMPDeclareTargetDirective");
-  IsInOpenMPDeclareTargetContext = false;
+  --DeclareTargetNestingLevel;
 }
 
 void Sema::ActOnOpenMPDeclareTargetName(Scope *CurScope,
Index: 

[PATCH] D51311: (WIP) Type hierarchy

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/ProtocolHandlers.cpp:70
   Register("textDocument/hover", ::onHover);
+  Register("textDocument/typeHierarchy", ::onTypeHierarchy);
   Register("textDocument/documentSymbol", 
::onDocumentSymbol);

LSP doesn't have such an entry, maybe we should use [[ 
https://microsoft.github.io/language-server-protocol/specification#workspace_executeCommand
 | workspace/executeCommand ]]. WDYT @ilya-biryukov 



Comment at: clangd/XRefs.cpp:669
+  const CXXMethodDecl *Candidate) {
+  // FIXME: How do I determine if Method overrides Candidate?
+

ilya-biryukov wrote:
> simark wrote:
> > kadircet wrote:
> > > https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp
> > > 
> > > you can check this sample, which simply traverses all base classes and 
> > > gets methods with the same name, then checks whether one is overload of 
> > > the other. If it they are not overloads then one in the base classes gets 
> > > overriden by the other.
> > > 
> > > 
> > > Another approach could be to search for one method in others 
> > > overriden_methods.
> > > 
> > > But they are both inefficient, I would suggest calling these functions 
> > > only when one of them is defined in the base class of other then you can 
> > > just check for name equality and not being an overload.
> > > https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp
> > 
> > Did you mean to link to a particular line?
> > 
> > > you can check this sample, which simply traverses all base classes and 
> > > gets methods with the same name, then checks whether one is overload of 
> > > the other. If it they are not overloads then one in the base classes gets 
> > > overriden by the other.
> > 
> > > Another approach could be to search for one method in others 
> > > overriden_methods.
> > 
> > I have tried using `overriden_methods`, but it only contains methods marked 
> > virtual.  For this feature, I would like to consider non-virtual methods 
> > too.
> > 
> > > But they are both inefficient, I would suggest calling these functions 
> > > only when one of them is defined in the base class of other then you can 
> > > just check for name equality and not being an overload.
> > 
> > I am not sure I understand, but maybe it will be clearer when I will see 
> > the sample.
> See the code that actually computes a list for `override_methods()`: 
> Sema::AddOverriddenMethods.
> Did you mean to link to a particular line?


Yeah sorry, I was trying to link the function ilya mentioned.
https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp#L7615

Since you also mentioned checking non-virtual method as well you might wanna 
look at 
https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp#L7555 as 
well.

Also I got the chance to look the rest of the patch, as I mentioned above you 
are already checking two methods iff one lies in the others base classes. So 
you can simply check for name equality and not being an overload case.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51311



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

[PATCH] D50892: [analyzer][UninitializedObjectChecker] Correct dynamic type is acquired for record pointees

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: test/Analysis/cxx-uninitialized-object-inheritance.cpp:802
+struct DynTBase2 {
+  int x; // expected-note{{uninitialized field 'static_cast(this->bptr)->DynTBase2::x'}}
+};

Mmm, what's the value of casting to derived type and then specifying that we 
access the field of the base type anyway? Isn't `this->bptr->x` exactly what 
the user needs to know(?)


https://reviews.llvm.org/D50892



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

Re: [PATCH] D51358: [driver] Do not pass "-flavor old-gnu" option to LLD linker

[Eric Christopher via cfe-commits]

LGTM

On Tue, Aug 28, 2018, 7:49 AM Simon Atanasyan via Phabricator <
revi...@reviews.llvm.org> wrote:

> atanasyan created this revision.
> atanasyan added reviewers: echristo, ruiu.
>
> The "-flavor old-gnu" option were introduced to enable old version of LLD
> ELF linker implementation. This option has been removed from the linker
> since LLD 3.9. I do not think that there is a real case when the latest
> version of Clang is used in combination with so obsoleted version of LLD
> linker. Now we can remove support of this option from the driver.
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D51358
>
> Files:
>   lib/Driver/ToolChains/Gnu.cpp
>
>
> Index: lib/Driver/ToolChains/Gnu.cpp
> ===
> --- lib/Driver/ToolChains/Gnu.cpp
> +++ lib/Driver/ToolChains/Gnu.cpp
> @@ -323,14 +323,6 @@
>// handled somewhere else.
>Args.ClaimAllArgs(options::OPT_w);
>
> -  const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
> -  if (llvm::sys::path::stem(Exec) == "lld") {
> -CmdArgs.push_back("-flavor");
> -CmdArgs.push_back("old-gnu");
> -CmdArgs.push_back("-target");
> -
> CmdArgs.push_back(Args.MakeArgString(getToolChain().getTripleString()));
> -  }
> -
>if (!D.SysRoot.empty())
>  CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
>
> @@ -539,6 +531,7 @@
>AddHIPLinkerScript(getToolChain(), C, Output, Inputs, Args, CmdArgs, JA,
>   *this);
>
> +  const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
>C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs,
> Inputs));
>  }
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51321: [Tooling] Improve handling of CL-style options

[Hamza Sood via Phabricator via cfe-commits]

hamzasood added a comment.

Thanks for the detailed feedback; I’ll try to get a new patch up in a few hours.

However I disagree with some of them (see replies).




Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:124
+// Determine whether the given file path is the clang-cl executable.
+static bool checkIsCLMode(const std::vector ,
+  const llvm::opt::OptTable ) {

sammccall wrote:
> nit: a two-value enum would be clearer and allow for terser variable names 
> (`Mode`)
The advantage of a Boolean is that it makes the checks simpler. E.g. `if 
(isCL)` instead of `if (mode == DriverMode::CL)` or something.

Happy to change it though if you still disagree.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:156
+
+// Transform the command line to an llvm ArgList.
+// Make sure that OldArgs lives for at least as long as this variable as 
the

sammccall wrote:
> would you mind reverting this change and just wrapping the old Argv in an 
> InputArgList?
> I'm not sure the lifetime comments and std::transform aid readability here.
The change was more about safety than readability. The old approach left an 
InputArgList of dangling pointers after moving the new args into the cmd 
object. This way there’s no way to accidentally access deallocated memory by 
using the InputArgList.

As above, happy to change if you still disagree.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:178
+  {
+unsigned Included, Excluded;
+if (IsCLMode) {

sammccall wrote:
> This seems a bit verbose.
> 
> First, do things actually break if we just use the default 0/0 masks? We're 
> not trying to interpret all the flags, just a few and pass the rest through.
> 
> Failing that, it seems clearer to just use a ternary to initialize 
> Included/Excluded, or inline them completely.
> (Please also drop the extra scope here).
Theoretically it could break without the flags. We need to recognise input 
files and strip them from the command line. If someone on a UNIX platform has 
an input file called `/W3`, that’d instead be interpreted as a warning flag and 
it’ll be left in. Likewise for any file in the root directory with the same 
spelling as a CL-only argument.

But yeah, I’ll inline the flags with a ternary.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:205
+  if (const auto GivenStd = isStdArg(*Arg)) {
+if (*GivenStd != LangStandard::lang_unspecified)
+  Std = *GivenStd;

sammccall wrote:
> prefer *either* optional<> or allowing the function to return 
> lang_unspecified, but not both. (There's no way a user can *explicitly* pass 
> a flag saying the language is unspecified, right?)
Kind of: `-std=hello_there` is parsed as an unspecified language. We still want 
to strip the flag in this case, which won’t be possible if we also use the 
unspecified value to denote a lack of an `-std` flag.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:231
: Type;
-  Result.CommandLine.push_back("-x");
-  Result.CommandLine.push_back(types::getTypeName(TargetType));
+  if (IsCLMode) {
+const StringRef Flag = toCLFlag(TargetType);

sammccall wrote:
> can we unify as `addTypeFlag(TargetType, Mode, Result.CommandLine)`?
To clarify, you mean extract this into a helper function?



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:258
+
+if (IsCLMode)
+  return Opt.matches(driver::options::OPT__SLASH_Fa) ||

sammccall wrote:
> Why do we need to take IsClMode into account while reading flags?
> How would `OPT__SLASH_Fa` get parsed if we weren't in CL mode? Would we care?
> 
> (and below)
It was an optimisation attempt in that we can do 5 less comparisons in the case 
where we know that there aren’t going to be any CL flags. Maybe I was trying 
too hard to micro-optimise...



Comment at: unittests/Tooling/CompilationDatabaseTest.cpp:652
+
+class InterpolateTest
+  : public ::testing::TestWithParam> {

sammccall wrote:
> I'm not sure the parameterized test is the right model here.
> The `INSTANTIATE_TEST_P` and friends makes the test fixture pretty 
> complicated, and failure messages hard to relate to input conditions.
> Then the actual tests have a bunch of assertions conditional on which mode 
> we're in.
> Finally, we don't actually test any mixed-mode database.
> 
> Could we write this a little more directly?:
>  - pass the driver mode flag to `add()` in the normal way, in the Flags param
>  - require callers to "add" to pass "clang" or "clang-cl". (It's OK that this 
> makes existing cases a bit more verbose)
>  - explicitly test the clang-cl and --driver-mode cases we care about, rather 
> than running every assertion in every 

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162932.
hugoeg added a comment.

merge conflicts resolved @aaron.ballman


https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char [](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string +=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string +=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view A, string_view B);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string );
+  AlphaNum(const string_view );
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum =(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum );
+string StrCat(const AlphaNum , const AlphaNum );
+string StrCat(const AlphaNum , const AlphaNum , const AlphaNum );
+string StrCat(const AlphaNum , const AlphaNum , const AlphaNum ,
+  const AlphaNum );
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum , const AlphaNum , const AlphaNum ,
+  const AlphaNum , const AlphaNum , const AV &... args);
+
+void StrAppend(string *Dest, const AlphaNum );
+void StrAppend(string *Dest, const AlphaNum , const AlphaNum );
+void StrAppend(string *Dest, const AlphaNum , const AlphaNum ,
+   const AlphaNum );
+void StrAppend(string *Dest, const AlphaNum , const AlphaNum ,
+   const AlphaNum , const AlphaNum );
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *Dest, const AlphaNum , const AlphaNum ,
+   const AlphaNum , const AlphaNum , const AlphaNum ,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)));
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: multiple calls to 'absl::StrCat' can be flattened into a single 

[PATCH] D51333: Diagnose likely typos in include statements

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/DiagnosticLexKinds.td:476
+  ExtWarn<"likely typo, expected \"FILENAME\" or  "
+  "but filename is '%0'">, InGroup;
+

This seems like the wrong warning group for this diagnostic as it doesn't 
relate to pragmas. However, we may not want this exposed as a separate warning, 
either.



Comment at: lib/Lex/PPDirectives.cpp:1885-1886
+}
+Diag(FilenameTok, diag::err_pp_file_not_found)
+<< Filename << FilenameRange;
+  }

I don't think we want to produce two different diagnostics for the same line of 
code. What if, instead, we augment the error diagnostic so that it can produce 
additional information in this case?
```
def err_pp_file_not_found : Error<"'%0' file not found%select{|, possibly due 
to leading or trailing non-alphanumeric characters in the file name}1">, 
DefaultFatal;
```
(or something along those lines.)



Comment at: test/Frontend/include-likely-typo.c:1-4
+// RUN: not %clang_cc1 -verify -frewrite-includes
+#include "" @expected-warning {{likely typo, expected "FILENAME" or 
 but filename is ''}}
+@expected-error {{'' file not found}}
+#include " hello.h " @expected-warning {{likely typo, expected "FILENAME" or 
 but filename is ' hello.h '}}

I think we want this test to live in test/Preprocessor instead of test/Frontend.


Repository:
  rC Clang

https://reviews.llvm.org/D51333



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

r340867 - Revert "[libFuzzer] Port to Windows"

[Matt Morehouse via cfe-commits]

Author: morehouse
Date: Tue Aug 28 12:07:24 2018
New Revision: 340867

URL: http://llvm.org/viewvc/llvm-project?rev=340867=rev
Log:
Revert "[libFuzzer] Port to Windows"

This reverts commit r340860 due to failing tests.

Modified:
cfe/trunk/lib/Driver/ToolChains/MSVC.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/MSVC.cpp?rev=340867=340866=340867=diff
==
--- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp Tue Aug 28 12:07:24 2018
@@ -365,17 +365,6 @@ void visualstudio::Linker::ConstructJob(
 CmdArgs.push_back(Args.MakeArgString(std::string("-implib:") + 
ImplibName));
   }
 
-  if (TC.getSanitizerArgs().needsFuzzer()) {
-if (!Args.hasArg(options::OPT_shared))
-  CmdArgs.push_back(
-  Args.MakeArgString(std::string("-wholearchive:") +
- TC.getCompilerRTArgString(Args, "fuzzer", 
false)));
-CmdArgs.push_back(Args.MakeArgString("-debug"));
-// Prevent the linker from padding sections we use for instrumentation
-// arrays.
-CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
-  }
-
   if (TC.getSanitizerArgs().needsAsanRt()) {
 CmdArgs.push_back(Args.MakeArgString("-debug"));
 CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
@@ -1309,8 +1298,6 @@ MSVCToolChain::ComputeEffectiveClangTrip
 SanitizerMask MSVCToolChain::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
-  Res |= SanitizerKind::Fuzzer;
-  Res |= SanitizerKind::FuzzerNoLink;
   Res &= ~SanitizerKind::CFIMFCall;
   return Res;
 }


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

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D51132#1215916, @hugoeg wrote:

> @aaron.ballman when you get the chance could you take another look at this 
> and commit if it is ready? My internship ends rather soon and this is a tad 
> time sensitive. Thank you for your time!


When I try to apply your patch on top of ToT, I get merge conflicts. Can you 
resolve those and upload a new patch?


https://reviews.llvm.org/D51132



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

[PATCH] D51372: FENV_ACCESS support for libm-style constrained intrinsics

[Kevin P. Neal via Phabricator via cfe-commits]

kpn created this revision.
kpn added a reviewer: rsmith.
Herald added a reviewer: javed.absar.
Herald added a subscriber: cfe-commits.

This builds on https://reviews.llvm.org/D49865 to get #pragma STDC FENV_ACCESS 
ON to and used by AST handling of function calls to math intrinsics. This now 
emits constrained math intrinsics instead of the regular variety for the ones 
currently implemented in llvm.

This should be good for C. More work is needed for C++.


Repository:
  rC Clang

https://reviews.llvm.org/D51372

Files:
  include/clang/AST/Expr.h
  include/clang/AST/ExprCXX.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/Analysis/BodyFarm.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Frontend/Rewrite/RewriteModernObjC.cpp
  lib/Frontend/Rewrite/RewriteObjC.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/TreeTransform.h
  test/CodeGen/fenv-access-pragma.c
  test/CodeGen/fenv-math-builtins.c
  test/Parser/pragma-fenv-access.c

Index: test/Parser/pragma-fenv-access.c
===
--- test/Parser/pragma-fenv-access.c
+++ test/Parser/pragma-fenv-access.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void f1(void) {
+  int x = 0;
+/* expected-error@+1 {{'#pragma fenv_access' can only appear at file scope or at the start of a compound statement}} */
+#pragma STDC FENV_ACCESS ON
+}
+
+void f2(void) {
+  #pragma STDC FENV_ACCESS OFF
+  #pragma STDC FENV_ACCESS ON 
+}
Index: test/CodeGen/fenv-math-builtins.c
===
--- test/CodeGen/fenv-math-builtins.c
+++ test/CodeGen/fenv-math-builtins.c
@@ -0,0 +1,84 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm  %s | FileCheck %s 
+
+// Test codegen of math builtins when using FENV_ACCESS ON.
+// Derived from math-builtins.c and keeps calls in the same order.
+#pragma STDC FENV_ACCESS ON
+
+void foo(double *d, float f, float *fp, long double *l, int *i, const char *c) {
+  __builtin_pow(f,f);__builtin_powf(f,f);   __builtin_powl(f,f);
+
+// CHECK: declare double @llvm.experimental.constrained.pow.f64(double, double, metadata, metadata) [[MATH_INTRINSIC:#[0-9]+]]
+// CHECK: declare float @llvm.experimental.constrained.pow.f32(float, float, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.pow.f80(x86_fp80, x86_fp80, metadata, metadata) [[MATH_INTRINSIC]]
+
+  __builtin_powi(f,f);__builtin_powif(f,f);   __builtin_powil(f,f);
+
+// CHECK: declare double @llvm.experimental.constrained.powi.f64(double, i32, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare float @llvm.experimental.constrained.powi.f32(float, i32, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.powi.f80(x86_fp80, i32, metadata, metadata) [[MATH_INTRINSIC]]
+
+  /* math */
+  __builtin_cos(f);__builtin_cosf(f);   __builtin_cosl(f); 
+// CHECK: declare double @llvm.experimental.constrained.cos.f64(double, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare float @llvm.experimental.constrained.cos.f32(float, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.cos.f80(x86_fp80, metadata, metadata) [[MATH_INTRINSIC]]
+
+  __builtin_exp(f);__builtin_expf(f);   __builtin_expl(f);
+// CHECK: declare double @llvm.experimental.constrained.exp.f64(double, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare float @llvm.experimental.constrained.exp.f32(float, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.exp.f80(x86_fp80, metadata, metadata) [[MATH_INTRINSIC]]
+
+  __builtin_exp2(f);   __builtin_exp2f(f);  __builtin_exp2l(f); 
+
+// CHECK: declare double @llvm.experimental.constrained.exp2.f64(double, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare float @llvm.experimental.constrained.exp2.f32(float, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.exp2.f80(x86_fp80, metadata, metadata) [[MATH_INTRINSIC]]
+
+  __builtin_fma(f,f,f);__builtin_fmaf(f,f,f);   __builtin_fmal(f,f,f);
+
+// CHECK: declare double @llvm.experimental.constrained.fma.f64(double, double, double, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare float @llvm.experimental.constrained.fma.f32(float, float, float, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.fma.f80(x86_fp80, x86_fp80, x86_fp80, metadata, metadata) [[MATH_INTRINSIC]]
+
+  __builtin_log(f);__builtin_logf(f);   __builtin_logl(f);
+
+// CHECK: declare double @llvm.experimental.constrained.log.f64(double, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare float @llvm.experimental.constrained.log.f32(float, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare 

[PATCH] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

ping.


Repository:
  rC Clang

https://reviews.llvm.org/D50901



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

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

ping.


Repository:
  rC Clang

https://reviews.llvm.org/D50250



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162921.
hugoeg added a comment.

renamed check as suggested


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDependenciesCheck.cpp
  clang-tidy/abseil/NoInternalDependenciesCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/external-file.h
  test/clang-tidy/Inputs/absl/strings/internal-file.h
  test/clang-tidy/abseil-no-internal-dependencies.cpp

Index: test/clang-tidy/abseil-no-internal-dependencies.cpp
===
--- test/clang-tidy/abseil-no-internal-dependencies.cpp
+++ test/clang-tidy/abseil-no-internal-dependencies.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-dependencies %t,  -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-internal-dependencies' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:6:24: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil [abseil-no-internal-dependencies]
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+void OpeningNamespace() {
+  strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+SomeContainer b;
+std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/internal-file.h
===
--- test/clang-tidy/Inputs/absl/strings/internal-file.h
+++ test/clang-tidy/Inputs/absl/strings/internal-file.h
@@ -1 +1,33 @@
-namespace absl {}
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/external-file.h
===
--- test/clang-tidy/Inputs/absl/external-file.h
+++ test/clang-tidy/Inputs/absl/external-file.h
@@ -1 +1,6 @@
-namespace absl {}
+namespace absl {
+namespace base_internal {
+void InternalFunction() {}
+} // namespace base_internal 
+} //namespace absl
+void DirectAccess2() { absl::base_internal::InternalFunction(); }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -6,6 +6,7 @@
 .. toctree::
abseil-duration-division
abseil-faster-strsplit-delimiter
+   abseil-no-internal-dependencies
abseil-no-namespace
abseil-string-find-startswith
android-cloexec-accept
Index: docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
+++ docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
@@ -0,0 +1,24 @@
+subl.. title:: clang-tidy - abseil-no-internal-dependencies
+
+abseil-no-internal-dependencies
+===
+
+Warns if code using Abseil depends on internal details. If something is in a
+namespace that includes the word “internal”, code is 

[PATCH] D49244: Always search sysroot for GCC installs

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

rsmith added a comment.

I think `-gcc-toolchain`, if specified, should simply be taken as the location 
of the GCC toolchain; we should never go looking for it anywhere else if 
`-gcc-toolchain` is specified. So I think the patch is not quite right as-is, 
as it also affects that case. I think these alternatives both seem reasonable:

- if `-gcc-toolchain` is not specified, and `--sysroot` is specified, then also 
look in the sysroot for GCC as normal after checking in `GCC_INSTALL_PREFIX`
- if `--sysroot` is specified, ignore `GCC_INSTALL_PREFIX` when computing the 
GCC toolchain directory

(The difference would be that in the former case we'd use a GCC that's not 
within the sysroot if `GCC_INSTALL_PREFIX` names a suitable toolchain, and in 
the latter case we would not.)

I think my preference is the second option: `GCC_INSTALL_PREFIX` should only be 
taken as specifying the installation prefix for the default sysroot specified 
at configure time. If `--sysroot` is explicitly specified, `GCC_INSTALL_PREFIX` 
should be ignored (but `-gcc-toolchain` should still be respected, and if 
specified we should not go looking for a toolchain in the sysroot ourselves.)


Repository:
  rC Clang

https://reviews.llvm.org/D49244



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

[PATCH] D50927: [Sema] Remove location from implicit capture init expr

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: clang/lib/Sema/SemaLambda.cpp:1422-1424
   auto Entity = InitializedEntity::InitializeLambdaCapture(
   Var->getIdentifier(), Field->getType(), Loc);
   InitializationKind InitKind = InitializationKind::CreateDirect(Loc, Loc, 
Loc);

rsmith wrote:
> Should these locations also be updated to `InitLoc`? If we're modeling the 
> initialization as happening at the capture-default, we should do that 
> consistently.
I've revised the patch to use one location consistently here. The tradeoff is 
that a few diagnostics now point to CaptureDefaultLoc instead of within the 
lambda body, but I'm happy to defer to more experienced Sema hands.



Comment at: clang/lib/Sema/SemaLambda.cpp:1612
+auto InitResult = performLambdaVarCaptureInitialization(
+*this, From, *CurField, IntroducerRange.getBegin(), IsImplicit);
 if (InitResult.isInvalid())

rsmith wrote:
> Use `CaptureDefaultLoc` here instead of the start of the introducer range.
Sure. This does seem better for diagnostic reporting purposes. I'll just note 
that it may make the line table look awkward in the (admittedly unlikely) event 
that 'IntroducerRange.getBegin()' and 'CaptureDefaultLoc' are on different 
lines.


https://reviews.llvm.org/D50927



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

[PATCH] D50927: [Sema] Remove location from implicit capture init expr

[Vedant Kumar via Phabricator via cfe-commits]

vsk updated this revision to Diff 162918.
vsk marked 2 inline comments as done.
vsk added a comment.

Address the latest round of review feedback.


https://reviews.llvm.org/D50927

Files:
  clang/lib/Sema/SemaLambda.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp
  clang/test/CodeGenCXX/debug-info-lambda.cpp
  clang/test/SemaCXX/uninitialized.cpp
  clang/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp

Index: clang/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp
===
--- clang/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp
@@ -75,11 +75,11 @@
 TEST(RecursiveASTVisitor, VisitsImplicitLambdaCaptureInit) {
   DeclRefExprVisitor Visitor;
   Visitor.setShouldVisitImplicitCode(true);
-  // We're expecting the "i" in the lambda to be visited twice:
-  // - Once for the DeclRefExpr in the lambda capture initialization (whose
-  //   source code location is set to the first use of the variable).
-  // - Once for the DeclRefExpr for the use of "i" inside the lambda.
-  Visitor.ExpectMatch("i", 1, 24, /*Times=*/2);
+  // We're expecting "i" to be visited twice: once for the initialization expr
+  // for the captured variable "i" outside of the lambda body, and again for
+  // the use of "i" inside the lambda.
+  Visitor.ExpectMatch("i", 1, 20, /*Times=*/1);
+  Visitor.ExpectMatch("i", 1, 24, /*Times=*/1);
   EXPECT_TRUE(Visitor.runOver(
 "void f() { int i; [=]{ i; }; }",
 DeclRefExprVisitor::Lang_CXX11));
Index: clang/test/SemaCXX/uninitialized.cpp
===
--- clang/test/SemaCXX/uninitialized.cpp
+++ clang/test/SemaCXX/uninitialized.cpp
@@ -884,8 +884,10 @@
 int x;
   };
   A a0([] { return a0.x; }); // ok
-  void f() { 
-A a1([=] { return a1.x; }); // expected-warning{{variable 'a1' is uninitialized when used within its own initialization}}
+  void f() {
+A a1([=] { // expected-warning{{variable 'a1' is uninitialized when used within its own initialization}}
+  return a1.x;
+});
 A a2([&] { return a2.x; }); // ok
   }
 }
Index: clang/test/CodeGenCXX/debug-info-lambda.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-lambda.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm \
+// RUN:   -debug-info-kind=line-tables-only -dwarf-column-info -std=c++11 %s -o - | FileCheck %s
+
+// CHECK-LABEL: define{{.*}}lambda_in_func
+void lambda_in_func(int ) {
+  // CHECK: [[ref_slot:%.*]] = getelementptr inbounds %class.anon, %class.anon* {{.*}}, i32 0, i32 0, !dbg [[lambda_decl_loc:![0-9]+]]
+  // CHECK-NEXT: %1 = load i32*, i32** %ref.addr, align 8, !dbg [[capture_init_loc:![0-9]+]]
+  // CHECK-NEXT: store i32* %1, i32** %0, align 8, !dbg [[lambda_decl_loc]]
+  // CHECK-NEXT: call void {{.*}}, !dbg [[lambda_call_loc:![0-9]+]]
+
+  auto helper = [   // CHECK: [[lambda_decl_loc]] = !DILocation(line: [[@LINE]], column: 17
+ &]() { // CHECK: [[capture_init_loc]] = !DILocation(line: [[@LINE]], column: 18
+++ref;
+  };
+  helper(); // CHECK: [[lambda_call_loc]] = !DILocation(line: [[@LINE]]
+}
Index: clang/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp
===
--- clang/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp
@@ -88,8 +88,8 @@
   template
   void odr_used2(R , Boom boom) {
 const std::type_info 
-  = typeid([=,] () -> R& {
-  boom.tickle(); // expected-note{{in instantiation of member function}}
+  = typeid([=,] () -> R& { // expected-note{{in instantiation of member function 'p2::Boom::Boom' requested here}}
+  boom.tickle();
   return r; 
 }()); 
   }
Index: clang/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp
===
--- clang/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp
@@ -15,8 +15,8 @@
 
 void capture_by_copy(NonCopyable nc, NonCopyable , const NonConstCopy nco) {
   (void)[nc] { }; // expected-error{{capture of variable 'nc' as type 'NonCopyable' calls private copy constructor}}
-  (void)[=] {
-ncr.foo(); // expected-error{{capture of variable 'ncr' as type 'NonCopyable' calls private copy constructor}} 
+  (void)[=] { // expected-error{{capture of variable 'ncr' as type 'NonCopyable' calls private copy constructor}}
+ncr.foo();
   }();
 
   [nco] {}(); // expected-error{{no matching constructor for initialization of 'const NonConstCopy'}}
Index: clang/lib/Sema/SemaLambda.cpp
===

r340860 - [libFuzzer] Port to Windows

[Matt Morehouse via cfe-commits]

Author: morehouse
Date: Tue Aug 28 11:34:32 2018
New Revision: 340860

URL: http://llvm.org/viewvc/llvm-project?rev=340860=rev
Log:
[libFuzzer] Port to Windows

Summary:
Port libFuzzer to windows-msvc.
This patch allows libFuzzer targets to be built and run on Windows, using 
-fsanitize=fuzzer and/or fsanitize=fuzzer-no-link. It allows these forms of 
coverage instrumentation to work on Windows as well.
It does not fix all issues, such as those with -fsanitize-coverage=stack-depth, 
which is not usable on Windows as of this patch.
It also does not fix any libFuzzer integration tests. Nearly all of them fail 
to compile, fixing them will come in a later patch, so libFuzzer tests are 
disabled on Windows until them.

Patch By: metzman

Reviewers: morehouse, rnk

Reviewed By: morehouse, rnk

Subscribers: morehouse, kcc, eraman

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

Modified:
cfe/trunk/lib/Driver/ToolChains/MSVC.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/MSVC.cpp?rev=340860=340859=340860=diff
==
--- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp Tue Aug 28 11:34:32 2018
@@ -365,6 +365,17 @@ void visualstudio::Linker::ConstructJob(
 CmdArgs.push_back(Args.MakeArgString(std::string("-implib:") + 
ImplibName));
   }
 
+  if (TC.getSanitizerArgs().needsFuzzer()) {
+if (!Args.hasArg(options::OPT_shared))
+  CmdArgs.push_back(
+  Args.MakeArgString(std::string("-wholearchive:") +
+ TC.getCompilerRTArgString(Args, "fuzzer", 
false)));
+CmdArgs.push_back(Args.MakeArgString("-debug"));
+// Prevent the linker from padding sections we use for instrumentation
+// arrays.
+CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
+  }
+
   if (TC.getSanitizerArgs().needsAsanRt()) {
 CmdArgs.push_back(Args.MakeArgString("-debug"));
 CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
@@ -1298,6 +1309,8 @@ MSVCToolChain::ComputeEffectiveClangTrip
 SanitizerMask MSVCToolChain::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res |= SanitizerKind::Fuzzer;
+  Res |= SanitizerKind::FuzzerNoLink;
   Res &= ~SanitizerKind::CFIMFCall;
   return Res;
 }


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

[PATCH] D51221: [clangd] Some nitpicking around the new split (preamble/main) dynamic index

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 3 inline comments as done.
sammccall added inline comments.



Comment at: clangd/ClangdServer.cpp:122
+  //  - symbols declared both in the main file and the preamble
+  // (Note that symbols *only* in the main file are not indexed).
   FileIndex MainFileIdx;

ilya-biryukov wrote:
> I thought it was not true, but now I notice we actually don't index those 
> symbols.
> I think we should index them for workspaceSymbols, but not for completion. 
> Any pointers to the code that filters them out?
https://reviews.llvm.org/source/clang-tools-extra/browse/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp;340828$272

isExpansionInMainFile()

(having this buried so deep hurts readability and probably performance).

But I think this would be the behavior we want for code complete, to keep the 
indexes tiny and incremental updates fast?
WorkspaceSymbols is indeed a problem here :-( Tradeoffs...



Comment at: clangd/ClangdServer.cpp:152
   WorkScheduler(Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
-DynamicIdx ? *DynamicIdx : noopParsingCallbacks(),
+DynamicIdx ? DynamicIdx->makeUpdateCallbacks() : nullptr,
 Opts.UpdateDebounce, Opts.RetentionPolicy) {

ilya-biryukov wrote:
> Any reason to prefer `nullptr` over the no-op callbacks?
> Might be a personal preference, my reasoning is: having an extra check for 
> null before on each of the call sites both adds unnecessary boilerplate and 
> adds an extra branch before the virtual call (which is, technically, another 
> form of a branch).
> 
> Not opposed to doing it either way, though.
Basically I prefer the old behavior here (when it was std::function).
Having to keep the callbacks object alive is a pain, and the shared instance of 
the no-op implementation for this purpose seems a little silly.

We don't have the std::function copyability stuff which is a mixed bag but nice 
for cases like this. But passing the reference from TUScheduler to its 
ASTWorkers is internal enough that it doesn't bother me, WDYT?

> extra check for null before on each of the call sites 
Note that the check is actually in the constructor, supporting `nullptr` is 
just for brevity (particularly in tests).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51221



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

[PATCH] D51321: [Tooling] Improve handling of CL-style options

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks for fixing this!
Mostly just picky style comments.
In particular, I know that some of the other maintainers here are just as 
ignorant of the cl driver as I am, and I want to make sure that it's still 
possible to follow the logic and debug unrelated problems without needing to 
know too much about it.

If some of the style bits is too annoying or unclear, happy to do some of it 
myself as a followup, let me know!




Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:156-169
+// Transform the command line to an llvm ArgList.
+// Make sure that OldArgs lives for at least as long as this variable as 
the
+// arg list contains pointers to the OldArgs strings.
+llvm::opt::InputArgList ArgList;
+{
+  // Unfortunately InputArgList requires an array of c-strings whereas we
+  // have an array of std::string; we'll need an intermediate vector.

hamzasood wrote:
> rnk wrote:
> > I think the old for loop was nicer. I don't think this code needs to 
> > change, you should be able to use ParseArgs with the extra flag args.
> The problem with the old loop is that we lose aliasing information (e.g. 
> `/W3` becomes `-Wall`). While `ParseOneArg` also performs alias conversion, 
> it gives us indices for each individual argument. The last line of the new 
> loop copies arguments by using that index information to read from `OldArgs`, 
> which gives us the original spelling.
Makes sense, please add a comment summarizing. ("We don't use ParseArgs, as we 
must pass through the exact spelling of uninteresting args. Re-rendering is 
lossy for clang-cl options, e.g. /W3 -> -Wall")



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:124
+// Determine whether the given file path is the clang-cl executable.
+static bool checkIsCLMode(const std::vector ,
+  const llvm::opt::OptTable ) {

nit: a two-value enum would be clearer and allow for terser variable names 
(`Mode`)



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:156
+
+// Transform the command line to an llvm ArgList.
+// Make sure that OldArgs lives for at least as long as this variable as 
the

would you mind reverting this change and just wrapping the old Argv in an 
InputArgList?
I'm not sure the lifetime comments and std::transform aid readability here.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:175
+Cmd.CommandLine.emplace_back(OldArgs.front());
+for (unsigned Start = 1, End = Start; End < OldArgs.size(); Start = End) {
+  std::unique_ptr Arg;

these names are somewhat confusing - "End" neither marks the end of the loop 
nor the end of the current item (as it's initialized to Start).
What about:
```
for (unsigned Pos = 1; Pos < OldArgs.size();) {
  ...
  unsigned OldPos = Pos;
  Arg = ParseOneArg(..., Pos);
  ...
  NewArgs.insert(NewArgs.end(), [OldPos], [Pos]);
}
```



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:178
+  {
+unsigned Included, Excluded;
+if (IsCLMode) {

This seems a bit verbose.

First, do things actually break if we just use the default 0/0 masks? We're not 
trying to interpret all the flags, just a few and pass the rest through.

Failing that, it seems clearer to just use a ternary to initialize 
Included/Excluded, or inline them completely.
(Please also drop the extra scope here).



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:186
+}
+Arg = std::unique_ptr(
+  OptTable->ParseOneArg(ArgList, End, Included, Excluded));

Arg.reset(OptTable->ParseOneArg...)



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:194
   // Strip input and output files.
-  if (option.matches(clang::driver::options::OPT_INPUT) ||
-  option.matches(clang::driver::options::OPT_o)) {
+  if (isUntypedInputOrOutput(*Arg))
 continue;

can we inline this and just `||` all the options together here?



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:197
+
+  // Strip type specifiers, but record the overridden language.
+  if (const auto GivenType = isTypeSpecArg(*Arg)) {

please keep the mentions of -x etc even if not comprehensive - it's hard to 
precisely+tersely describe these flags in prose.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:203
+
+  // Strip std, but record the value.
+  if (const auto GivenStd = isStdArg(*Arg)) {

ditto --std



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:205
+  if (const auto GivenStd = isStdArg(*Arg)) {
+if (*GivenStd != LangStandard::lang_unspecified)
+  Std = *GivenStd;

r340854 - Define variables in test case rather than using values from functions

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Tue Aug 28 11:18:01 2018
New Revision: 340854

URL: http://llvm.org/viewvc/llvm-project?rev=340854=rev
Log:
Define variables in test case rather than using values from functions
emitted ealier.

Modified:
cfe/trunk/test/CodeGenObjCXX/arc-blocks.mm

Modified: cfe/trunk/test/CodeGenObjCXX/arc-blocks.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/arc-blocks.mm?rev=340854=340853=340854=diff
==
--- cfe/trunk/test/CodeGenObjCXX/arc-blocks.mm (original)
+++ cfe/trunk/test/CodeGenObjCXX/arc-blocks.mm Tue Aug 28 11:18:01 2018
@@ -141,7 +141,7 @@ namespace test1 {
 
 // CHECK: [[LPAD]]:
 // CHECK: invoke void @_ZN5test12S0D1Ev(%[[STRUCT_TEST1_S0]]* %[[V5]])
-// CHECK: to label %[[INVOKE_CONT3:.*]] unwind label %[[TERMINATE_LPAD]]
+// CHECK: to label %[[INVOKE_CONT3:.*]] unwind label %[[TERMINATE_LPAD:.*]]
 
 // CHECK: [[LPAD1]]
 // CHECK: br label %[[EHCLEANUP:.*]]
@@ -154,7 +154,7 @@ namespace test1 {
 // CHECK: %[[V14:.*]] = load i8*, i8** %[[V2]], align 8
 // CHECK: call void @_Block_object_dispose(i8* %[[V14]], i32 8)
 // CHECK: call void @objc_storeStrong(i8** %[[V4]], i8* null)
-// CHECK: br label %[[EH_RESUME]]
+// CHECK: br label %[[EH_RESUME:.*]]
 
 // CHECK: [[EH_RESUME]]:
 // CHECK: resume { i8*, i32 }


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

[PATCH] D51190: [AttrDocs]: document gnu_inline function attribute

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 162908.
nickdesaulniers added a comment.

- Take rsmith's sugguested wording. Add info about __GNUC_STDC_INLINE__.


Repository:
  rC Clang

https://reviews.llvm.org/D51190

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,37 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` changes the meaning of ``extern inline`` to use GNU inline
+semantics, meaning:
+
+* If any declaration that is declared ``inline`` is not declared ``extern``,
+then the ``inline`` keyword is just a hint (note that this is different from
+the behavior of ``inline`` in both C99 inline semantics and C++ inline
+semantics).
+
+* If all declarations that are declared ``inline`` are also declared
+``extern``, then the function body is present only for inlining and no
+out-of-line version is emitted.
+
+And in particular as special cases, ``static inline`` emits an out-of-line
+version if needed, a plain ``inline`` definition emits an out-of-line version
+always, and an ``extern inline`` definition (in a header) followed by a
+(non-``extern``) ``inline`` declaration in a source file emits an out-of-line
+version of the function in that source file but provides the function body for
+inlining to all includers of the header.
+
+Either ``__GNUC_GNU_INLINE__`` (GNU inline semantics) or
+``__GNUC_STDC_INLINE__`` (C99 semantics) will be defined (they are mutually
+exclusive). If ``__GNUC_STDC_INLINE__`` is defined, then the ``gnu_inline``
+function attribute can be used to get GNU inline semantics on a per function
+basis. If ``__GNUC_GNU_INLINE__`` is defined, then the translation unit is
+already being compiled with GNU inline semantics as the implied default.
+
+GNU inline semantics are the default behavior with ``-std=gnu89``,
+``-std=c89``, ``-std=c94``, or ``-fgnu89-inline``.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,37 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` changes the meaning of ``extern inline`` to use GNU inline
+semantics, meaning:
+
+* If any declaration that is declared ``inline`` is not declared ``extern``,
+then the ``inline`` keyword is just a hint (note that this is different from
+the behavior of ``inline`` in both C99 inline semantics and C++ inline
+semantics).
+
+* If all declarations that are declared ``inline`` are also declared
+``extern``, then the function body is present only for inlining and no
+out-of-line version is emitted.
+
+And in particular as special cases, ``static inline`` emits an out-of-line
+version if needed, a plain ``inline`` definition emits an out-of-line version
+always, and an ``extern inline`` definition (in a header) followed by a
+(non-``extern``) ``inline`` declaration in a source file emits an out-of-line
+version of the function in that source file but provides the function body for
+inlining to all includers of the header.
+
+Either ``__GNUC_GNU_INLINE__`` (GNU inline semantics) or
+``__GNUC_STDC_INLINE__`` (C99 semantics) will be defined (they are mutually
+exclusive). If ``__GNUC_STDC_INLINE__`` is defined, then the ``gnu_inline``
+function attribute can be used to get GNU inline semantics on a per function
+basis. If ``__GNUC_GNU_INLINE__`` is defined, then the translation unit is
+already being compiled with GNU inline semantics as the implied default.
+
+GNU inline semantics are the default behavior with ``-std=gnu89``,
+``-std=c89``, ``-std=c94``, or ``-fgnu89-inline``.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D51239: [ubsan] Enable -fsanitize=vptr on Apple devices and simulators

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340849: [ubsan] Enable -fsanitize=vptr on Apple devices and 
simulators (authored by vedantk, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51239?vs=162732=162907#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51239

Files:
  cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
  cfe/trunk/test/Driver/fsanitize.c


Index: cfe/trunk/test/Driver/fsanitize.c
===
--- cfe/trunk/test/Driver/fsanitize.c
+++ cfe/trunk/test/Driver/fsanitize.c
@@ -423,6 +423,15 @@
 // RUN: %clang -target x86_64-apple-darwin10 -mmacosx-version-min=10.8 
-fsanitize=vptr %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-VPTR-DARWIN-OLD
 // CHECK-VPTR-DARWIN-OLD: unsupported option '-fsanitize=vptr' for target 
'x86_64-apple-darwin10'
 
+// RUN: %clang -target arm-apple-ios4 -fsanitize=vptr %s -### 2>&1 | FileCheck 
%s --check-prefix=CHECK-VPTR-IOS-OLD
+// CHECK-VPTR-IOS-OLD: unsupported option '-fsanitize=vptr' for target 
'arm-apple-ios4'
+
+// RUN: %clang -target aarch64-apple-darwin15.0.0 -fsanitize=vptr %s -### 2>&1
+// OK
+
+// RUN: %clang -target x86_64-apple-darwin15.0.0-simulator -fsanitize=vptr %s 
-### 2>&1
+// OK
+
 // RUN: %clang -target x86_64-apple-darwin10 -mmacosx-version-min=10.9 
-fsanitize=alignment,vptr %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-VPTR-DARWIN-NEW
 // CHECK-VPTR-DARWIN-NEW: -fsanitize=alignment,vptr
 
Index: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
@@ -2258,9 +2258,15 @@
   Res |= SanitizerKind::Fuzzer;
   Res |= SanitizerKind::FuzzerNoLink;
   Res |= SanitizerKind::Function;
+
+  // Prior to 10.9, macOS shipped a version of the C++ standard library without
+  // C++11 support. The same is true of iOS prior to version 5. These OS'es are
+  // incompatible with -fsanitize=vptr.
+  if (!(isTargetMacOS() && isMacosxVersionLT(10, 9))
+  && !(isTargetIPhoneOS() && isIPhoneOSVersionLT(5, 0)))
+Res |= SanitizerKind::Vptr;
+
   if (isTargetMacOS()) {
-if (!isMacosxVersionLT(10, 9))
-  Res |= SanitizerKind::Vptr;
 if (IsX86_64)
   Res |= SanitizerKind::Thread;
   } else if (isTargetIOSSimulator() || isTargetTvOSSimulator()) {


Index: cfe/trunk/test/Driver/fsanitize.c
===
--- cfe/trunk/test/Driver/fsanitize.c
+++ cfe/trunk/test/Driver/fsanitize.c
@@ -423,6 +423,15 @@
 // RUN: %clang -target x86_64-apple-darwin10 -mmacosx-version-min=10.8 -fsanitize=vptr %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-VPTR-DARWIN-OLD
 // CHECK-VPTR-DARWIN-OLD: unsupported option '-fsanitize=vptr' for target 'x86_64-apple-darwin10'
 
+// RUN: %clang -target arm-apple-ios4 -fsanitize=vptr %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-VPTR-IOS-OLD
+// CHECK-VPTR-IOS-OLD: unsupported option '-fsanitize=vptr' for target 'arm-apple-ios4'
+
+// RUN: %clang -target aarch64-apple-darwin15.0.0 -fsanitize=vptr %s -### 2>&1
+// OK
+
+// RUN: %clang -target x86_64-apple-darwin15.0.0-simulator -fsanitize=vptr %s -### 2>&1
+// OK
+
 // RUN: %clang -target x86_64-apple-darwin10 -mmacosx-version-min=10.9 -fsanitize=alignment,vptr %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-VPTR-DARWIN-NEW
 // CHECK-VPTR-DARWIN-NEW: -fsanitize=alignment,vptr
 
Index: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
@@ -2258,9 +2258,15 @@
   Res |= SanitizerKind::Fuzzer;
   Res |= SanitizerKind::FuzzerNoLink;
   Res |= SanitizerKind::Function;
+
+  // Prior to 10.9, macOS shipped a version of the C++ standard library without
+  // C++11 support. The same is true of iOS prior to version 5. These OS'es are
+  // incompatible with -fsanitize=vptr.
+  if (!(isTargetMacOS() && isMacosxVersionLT(10, 9))
+  && !(isTargetIPhoneOS() && isIPhoneOSVersionLT(5, 0)))
+Res |= SanitizerKind::Vptr;
+
   if (isTargetMacOS()) {
-if (!isMacosxVersionLT(10, 9))
-  Res |= SanitizerKind::Vptr;
 if (IsX86_64)
   Res |= SanitizerKind::Thread;
   } else if (isTargetIOSSimulator() || isTargetTvOSSimulator()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r340849 - [ubsan] Enable -fsanitize=vptr on Apple devices and simulators

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Tue Aug 28 11:01:42 2018
New Revision: 340849

URL: http://llvm.org/viewvc/llvm-project?rev=340849=rev
Log:
[ubsan] Enable -fsanitize=vptr on Apple devices and simulators

It seems like an oversight that this check was not always enabled for
on-device or device simulator targets.

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/test/Driver/fsanitize.c

Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=340849=340848=340849=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Tue Aug 28 11:01:42 2018
@@ -2258,9 +2258,15 @@ SanitizerMask Darwin::getSupportedSaniti
   Res |= SanitizerKind::Fuzzer;
   Res |= SanitizerKind::FuzzerNoLink;
   Res |= SanitizerKind::Function;
+
+  // Prior to 10.9, macOS shipped a version of the C++ standard library without
+  // C++11 support. The same is true of iOS prior to version 5. These OS'es are
+  // incompatible with -fsanitize=vptr.
+  if (!(isTargetMacOS() && isMacosxVersionLT(10, 9))
+  && !(isTargetIPhoneOS() && isIPhoneOSVersionLT(5, 0)))
+Res |= SanitizerKind::Vptr;
+
   if (isTargetMacOS()) {
-if (!isMacosxVersionLT(10, 9))
-  Res |= SanitizerKind::Vptr;
 if (IsX86_64)
   Res |= SanitizerKind::Thread;
   } else if (isTargetIOSSimulator() || isTargetTvOSSimulator()) {

Modified: cfe/trunk/test/Driver/fsanitize.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=340849=340848=340849=diff
==
--- cfe/trunk/test/Driver/fsanitize.c (original)
+++ cfe/trunk/test/Driver/fsanitize.c Tue Aug 28 11:01:42 2018
@@ -423,6 +423,15 @@
 // RUN: %clang -target x86_64-apple-darwin10 -mmacosx-version-min=10.8 
-fsanitize=vptr %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-VPTR-DARWIN-OLD
 // CHECK-VPTR-DARWIN-OLD: unsupported option '-fsanitize=vptr' for target 
'x86_64-apple-darwin10'
 
+// RUN: %clang -target arm-apple-ios4 -fsanitize=vptr %s -### 2>&1 | FileCheck 
%s --check-prefix=CHECK-VPTR-IOS-OLD
+// CHECK-VPTR-IOS-OLD: unsupported option '-fsanitize=vptr' for target 
'arm-apple-ios4'
+
+// RUN: %clang -target aarch64-apple-darwin15.0.0 -fsanitize=vptr %s -### 2>&1
+// OK
+
+// RUN: %clang -target x86_64-apple-darwin15.0.0-simulator -fsanitize=vptr %s 
-### 2>&1
+// OK
+
 // RUN: %clang -target x86_64-apple-darwin10 -mmacosx-version-min=10.9 
-fsanitize=alignment,vptr %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-VPTR-DARWIN-NEW
 // CHECK-VPTR-DARWIN-NEW: -fsanitize=alignment,vptr
 


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

[PATCH] D43871: [modules] No longer include stdlib.h from mm_malloc.h.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor marked 3 inline comments as done.
teemperor added inline comments.



Comment at: test/CXX/except/except.spec/libc-empty-except.cpp:6
+#include "libc-empty-except.h"
+
+void f() {

bruno wrote:
> In a testcase like this but using the actual real headers, if you do:
> 
> #include 
> #include 
> 
> does it also compiles fine?
(I already answered this offline back then, but for the record again): Yes, we 
tried this actual code and I think the test case (with the actual reverse fix 
applied) also should cover this case.



Comment at: test/CXX/except/except.spec/libc-empty-except.cpp:27
+#else
+#include "mm_malloc.h"
+#include "stdlib.h"

v.g.vassilev wrote:
> Maybe we should reverse the includes here as we discussed offline.
Thanks!


https://reviews.llvm.org/D43871



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

[PATCH] D43871: [modules] No longer include stdlib.h from mm_malloc.h.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor updated this revision to Diff 162902.
teemperor added a comment.

- Now using UpdateExceptionSpec.
- Added a comment to the SemaExceptionSpec.cpp code why we are permitting this.


https://reviews.llvm.org/D43871

Files:
  lib/Headers/mm_malloc.h
  lib/Sema/SemaExceptionSpec.cpp
  test/CXX/except/except.spec/Inputs/clang/mm_malloc.h
  test/CXX/except/except.spec/Inputs/clang/module.modulemap
  test/CXX/except/except.spec/Inputs/libc/module.modulemap
  test/CXX/except/except.spec/Inputs/libc/stdlib.h
  test/CXX/except/except.spec/libc-empty-except.cpp

Index: test/CXX/except/except.spec/libc-empty-except.cpp
===
--- /dev/null
+++ test/CXX/except/except.spec/libc-empty-except.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+
+// One of the headers is in a user include, so our redeclaration should fail.
+// RUN: %clang_cc1 -std=c++11 -I %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -std=c++11 -isystem %S/Inputs/clang -I %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+
+// The same test cases again with enabled modules.
+// The modules cases *all* pass because we marked both as [system].
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:-std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:-std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:-std=c++11 -I %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:-std=c++11 -isystem %S/Inputs/clang -I %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+
+// expected-no-diagnostics
+#ifdef REVERSE
+#include "mm_malloc.h"
+#include "stdlib.h"
+#else
+#include "mm_malloc.h"
+#include "stdlib.h"
+#endif
+
+void f() {
+  free(nullptr);
+}
Index: test/CXX/except/except.spec/Inputs/libc/stdlib.h
===
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/libc/stdlib.h
@@ -0,0 +1,2 @@
+// Declare 'free' like glibc with a empty exception specifier.
+extern "C" void free(void *ptr) throw();
Index: test/CXX/except/except.spec/Inputs/libc/module.modulemap
===
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/libc/module.modulemap
@@ -0,0 +1,4 @@
+module libc [system] {
+  header "stdlib.h"
+  export *
+}
Index: test/CXX/except/except.spec/Inputs/clang/module.modulemap
===
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/clang/module.modulemap
@@ -0,0 +1,4 @@
+module builtin [system] {
+  header "mm_malloc.h"
+  export *
+}
Index: test/CXX/except/except.spec/Inputs/clang/mm_malloc.h
===
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/clang/mm_malloc.h
@@ -0,0 +1,3 @@
+// missing throw() is allowed in this case as we are in a system header.
+// This is a redeclaration possibly from glibc.
+extern "C" void free(void *ptr);
Index: lib/Sema/SemaExceptionSpec.cpp
===
--- lib/Sema/SemaExceptionSpec.cpp
+++ lib/Sema/SemaExceptionSpec.cpp
@@ -236,6 +236,7 @@
 const FunctionProtoType *New, SourceLocation NewLoc,
 bool *MissingExceptionSpecification = nullptr,
 bool *MissingEmptyExceptionSpecification = nullptr,
+bool *ExtraEmptyExceptionSpecification = nullptr,
 bool AllowNoexceptAllMatchWithNoSpec = false, bool IsOperatorNew = false);
 
 /// Determine whether a function has an implicitly-generated exception
@@ -259,6 +260,15 @@
   return !Ty->hasExceptionSpec();
 }
 
+/// Returns true if the given function is a function/builtin with C linkage
+/// and from a system header.
+static bool isCSystemFuncOrBuiltin(FunctionDecl *D, ASTContext ) {
+  return 

[PATCH] D51291: [clangd] Support multiple #include headers in one symbol.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Thanks for the review! I reduced the scope of the patch. PTAL




Comment at: clangd/CodeComplete.cpp:1396
+  if (IndexResult && !IndexResult->IncludeHeaders.empty()) {
+for (const auto  : IndexResult->IncludeHeaders)
+  AddWithInclude(P);

sammccall wrote:
> I remain unconvinced that providing multiple completion candidates is the 
> right thing to do here:
>  - if the index hasn't seen a definition, then we're going to show one copy 
> of the completion for each header that has a declaration
>  - the user isn't going to have a useful way to distinguish between them. 
> Note that e.g. we're going to show the #include path in the detail, but the 
> documentation is going to be identical for each.
>  - we lose the invariant that each completion item (pre-bundling) maps to a 
> different symbol
>  - C++ embedders don't have the option of displaying the multiple options 
> once the completion is selected
> 
> The alternative approach of sorting the includes by proximity * log(refs) or 
> so, and then using the top one for scoring, seems less of a drastic change to 
> the current behavior. (Visible effect: more accurate includes inserted).
 These are all valid points. I agree that we should start with less drastic 
change in the first version.

My concern was that there can be cases where it's impossible for clangd to 
suggest the correct #include (e.g. all includes have the same proximity and 
popularity). But I guess there are also alternative solutions to these than 
creating multiple completion results. For example, we could simply give up 
inserting include during code completion and let an include-fixer-like feature 
handle it. 



Comment at: clangd/Quality.cpp:190
 
+static const float kIncludeHeaderScoreIncreaseUnit = 0.0001;
+

sammccall wrote:
> This looks a bit sketchy. Particularly the += boost where everything else is 
> *=.
> What's this trying to do?
(This is no longer needed in this patch.)

As we were generating multiple candidates for the same symbol (for each 
include), I was trying to group them together using the small score differences 
as the tie breaker. 



Comment at: clangd/index/Merge.cpp:105
+  // merge include headers from L and R, as they can both export the symbol.
+  bool MergeIncludes = !L.Definition.FileURI.empty() &&
+   !R.Definition.FileURI.empty() &&

sammccall wrote:
> sammccall wrote:
> > This describes the logic, and the logic always produces the right result, 
> > but it's not clear *why*. Maybe add something like:
> > 
> > ```This is associative because it preserves the invariant:
> >  - if we haven't seen a definition, Includes covers all declarations
> >  - if we have seen a definition, Includes covers declarations visible from 
> > any definition```
> > 
> > in fact it seems hard to reason about this field in Symbol without 
> > understanding this, so maybe this invariant belongs on the IncludeHeaders 
> > field itself.
> Thinking more about it - what's the intent here?
> I'm not sure sorting by (seen by def, #refs) produces better ranking than 
> just #refs.
> 
> But there are other possible reasons for dropping includes not seen by any 
> def:
>  - remove spam from the completion list (only a problem if we clone the 
> completion items)
>  - reduce size for items that are often redeclared (I can imagine this being 
> a problem, not obvious)
> 
> Curious what your thinking is.
> in fact it seems hard to reason about this field in Symbol without 
> understanding this, so maybe this invariant belongs on the IncludeHeaders 
> field itself.
Make sense. Thanks!

> Thinking more about it - what's the intent here?
The intention is basically to filter out headers with forward declarations 
where definition is not seen. I could hardly think of a case where we would 
favor headers where def is not seen over those where definition is seen.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51291



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

[PATCH] D51291: [clangd] *Prototype* Support multiple #include headers in one symbol.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 162897.
ioeric marked 3 inline comments as done.
ioeric retitled this revision from "[clangd] Support multiple #include headers 
in one symbol." to "[clangd] *Prototype* Support multiple #include headers in 
one symbol.".
ioeric edited the summary of this revision.
ioeric added a comment.
Herald added a subscriber: mgrang.

- Addressed review comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51291

Files:
  clangd/CodeComplete.cpp
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/Merge.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolYAML.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -53,7 +53,11 @@
 MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; }
 MATCHER_P(DefURI, P, "") { return arg.Definition.FileURI == P; }
 MATCHER_P(IncludeHeader, P, "") {
-  return arg.Detail && arg.Detail->IncludeHeader == P;
+  return (arg.IncludeHeaders.size() == 1) &&
+ (arg.IncludeHeaders.begin()->IncludeHeader == P);
+}
+MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References,  "") {
+  return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
 }
 MATCHER_P(DeclRange, Pos, "") {
   return std::tie(arg.CanonicalDeclaration.Start.Line,
@@ -696,6 +700,11 @@
 Line: 1
 Column: 1
 IsIndexedForCodeCompletion:true
+IncludeHeaders:
+  - Header:'include1'
+References:7
+  - Header:'include2'
+References:3
 Detail:
   Documentation:'Foo doc'
   ReturnType:'int'
@@ -730,6 +739,11 @@
  Doc("Foo doc"), ReturnType("int"),
  DeclURI("file:///path/foo.h"),
  ForCodeCompletion(true;
+  auto  = *Symbols1.begin();
+  assert(Sym1.Detail);
+  EXPECT_THAT(Sym1.IncludeHeaders,
+  UnorderedElementsAre(IncludeHeaderWithRef("include1", 7u),
+   IncludeHeaderWithRef("include2", 3u)));
   auto Symbols2 = symbolsFromYAML(YAML2);
   EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(
 QName("clang::Foo2"), Labeled("Foo2-sig"),
@@ -751,9 +765,10 @@
 TEST_F(SymbolCollectorTest, IncludeHeaderSameAsFileURI) {
   CollectorOpts.CollectIncludePath = true;
   runSymbolCollector("class Foo {};", /*Main=*/"");
-  EXPECT_THAT(Symbols,
-  UnorderedElementsAre(AllOf(QName("Foo"), DeclURI(TestHeaderURI),
- IncludeHeader(TestHeaderURI;
+  EXPECT_THAT(Symbols, UnorderedElementsAre(
+   AllOf(QName("Foo"), DeclURI(TestHeaderURI;
+  EXPECT_THAT(Symbols.begin()->IncludeHeaders,
+  UnorderedElementsAre(IncludeHeaderWithRef(TestHeaderURI, 1u)));
 }
 
 #ifndef _WIN32
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -243,6 +243,48 @@
   EXPECT_EQ(M.Name, "right");
 }
 
+MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References,  "") {
+  return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
+}
+
+TEST(MergeTest, MergeIncludesOnDifferentDefinitions) {
+  Symbol L, R;
+  L.Name = "left";
+  R.Name = "right";
+  L.ID = R.ID = SymbolID("hello");
+  L.IncludeHeaders.emplace_back("common", 1);
+  R.IncludeHeaders.emplace_back("common", 1);
+  R.IncludeHeaders.emplace_back("new", 1);
+
+  Symbol::Details Scratch;
+
+  // Both have no definition.
+  Symbol M = mergeSymbol(L, R, );
+  EXPECT_THAT(M.IncludeHeaders,
+  UnorderedElementsAre(IncludeHeaderWithRef("common", 2u),
+   IncludeHeaderWithRef("new", 1u)));
+
+  // Only merge references of the same includes but do not merge new #includes.
+  L.Definition.FileURI = "file:/left.h";
+  M = mergeSymbol(L, R, );
+  EXPECT_THAT(M.IncludeHeaders,
+  UnorderedElementsAre(IncludeHeaderWithRef("common", 2u)));
+
+  // Definitions are the same.
+  R.Definition.FileURI = "file:/right.h";
+  M = mergeSymbol(L, R, );
+  EXPECT_THAT(M.IncludeHeaders,
+  UnorderedElementsAre(IncludeHeaderWithRef("common", 2u),
+   IncludeHeaderWithRef("new", 1u)));
+
+  // Definitions are different.
+  R.Definition.FileURI = "file:/right.h";
+  M = mergeSymbol(L, R, );
+  EXPECT_THAT(M.IncludeHeaders,
+  UnorderedElementsAre(IncludeHeaderWithRef("common", 2u),
+   IncludeHeaderWithRef("new", 1u)));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: 

r340845 - [Driver] Delete last reference of lld -flavor old-gnu

[Fangrui Song via cfe-commits]

Author: maskray
Date: Tue Aug 28 10:20:28 2018
New Revision: 340845

URL: http://llvm.org/viewvc/llvm-project?rev=340845=rev
Log:
[Driver] Delete last reference of lld -flavor old-gnu

This is dead code because lld -flavor old-gnu was removed in 2016 by rLLD262158.

Modified:
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=340845=340844=340845=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Tue Aug 28 10:20:28 2018
@@ -323,14 +323,6 @@ void tools::gnutools::Linker::ConstructJ
   // handled somewhere else.
   Args.ClaimAllArgs(options::OPT_w);
 
-  const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
-  if (llvm::sys::path::stem(Exec) == "lld") {
-CmdArgs.push_back("-flavor");
-CmdArgs.push_back("old-gnu");
-CmdArgs.push_back("-target");
-CmdArgs.push_back(Args.MakeArgString(getToolChain().getTripleString()));
-  }
-
   if (!D.SysRoot.empty())
 CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
 
@@ -539,6 +531,7 @@ void tools::gnutools::Linker::ConstructJ
   AddHIPLinkerScript(getToolChain(), C, Output, Inputs, Args, CmdArgs, JA,
  *this);
 
+  const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
 }
 


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

[PATCH] D51239: [ubsan] Enable -fsanitize=vptr on Apple devices and simulators

[Dan Liew via Phabricator via cfe-commits]

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

LGTM.




Comment at: clang/test/Driver/fsanitize.c:426
 
+// RUN: %clang -target arm-apple-ios4 -fsanitize=vptr %s -### 2>&1 | FileCheck 
%s --check-prefix=CHECK-VPTR-IOS-OLD
+// CHECK-VPTR-IOS-OLD: unsupported option '-fsanitize=vptr' for target 
'arm-apple-ios4'

@vsk Interesting. This that what the iOS 4 target triple looked like.


https://reviews.llvm.org/D51239



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162895.
hugoeg marked 3 inline comments as done.
hugoeg added a comment.

fixed issues outlines in comments


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/external-file.h
  test/clang-tidy/Inputs/absl/strings/internal-file.h
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t,  -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-internal-deps' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:6:24: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil [abseil-no-internal-deps]
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+void OpeningNamespace() {
+  strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+SomeContainer b;
+std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/internal-file.h
===
--- test/clang-tidy/Inputs/absl/strings/internal-file.h
+++ test/clang-tidy/Inputs/absl/strings/internal-file.h
@@ -1 +1,33 @@
-namespace absl {}
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/external-file.h
===
--- test/clang-tidy/Inputs/absl/external-file.h
+++ test/clang-tidy/Inputs/absl/external-file.h
@@ -1 +1,6 @@
-namespace absl {}
+namespace absl {
+namespace base_internal {
+void InternalFunction() {}
+} // namespace base_internal 
+} //namespace absl
+void DirectAccess2() { absl::base_internal::InternalFunction(); }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -6,6 +6,7 @@
 .. toctree::
abseil-duration-division
abseil-faster-strsplit-delimiter
+   abseil-no-internal-deps
abseil-no-namespace
abseil-string-find-startswith
android-cloexec-accept
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,24 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Warns if code using Abseil depends on internal details. If something is in a
+namespace that includes the word “internal”, code is not allowed to depend upon
+it beaucse it’s an implementation detail. They cannot 

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

I still thik will be good idea to rename check (deps -> dependencies).


https://reviews.llvm.org/D50542



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

[PATCH] D51311: (WIP) Type hierarchy

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks for looking into this. Would be cool to get this supported after the 
proposal is finalized.




Comment at: clangd/Protocol.h:891
+
+  std::vector Parents;
+

What is the proposal to add children (i.e. overriden methods) to the hierarchy?
This seems like a place where we might want to have multiple requests from the 
clients when expanding the nodes.



Comment at: clangd/XRefs.cpp:669
+  const CXXMethodDecl *Candidate) {
+  // FIXME: How do I determine if Method overrides Candidate?
+

simark wrote:
> kadircet wrote:
> > https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp
> > 
> > you can check this sample, which simply traverses all base classes and gets 
> > methods with the same name, then checks whether one is overload of the 
> > other. If it they are not overloads then one in the base classes gets 
> > overriden by the other.
> > 
> > 
> > Another approach could be to search for one method in others 
> > overriden_methods.
> > 
> > But they are both inefficient, I would suggest calling these functions only 
> > when one of them is defined in the base class of other then you can just 
> > check for name equality and not being an overload.
> > https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp
> 
> Did you mean to link to a particular line?
> 
> > you can check this sample, which simply traverses all base classes and gets 
> > methods with the same name, then checks whether one is overload of the 
> > other. If it they are not overloads then one in the base classes gets 
> > overriden by the other.
> 
> > Another approach could be to search for one method in others 
> > overriden_methods.
> 
> I have tried using `overriden_methods`, but it only contains methods marked 
> virtual.  For this feature, I would like to consider non-virtual methods too.
> 
> > But they are both inefficient, I would suggest calling these functions only 
> > when one of them is defined in the base class of other then you can just 
> > check for name equality and not being an overload.
> 
> I am not sure I understand, but maybe it will be clearer when I will see the 
> sample.
See the code that actually computes a list for `override_methods()`: 
Sema::AddOverriddenMethods.



Comment at: clangd/XRefs.cpp:732
+// If this is a variable, use the type of the variable.
+CXXRD = VD->getType().getTypePtr()->getAsCXXRecordDecl();
+  } else if (Method) {

Why special-case the variables? Maybe just return empty results for consistency 
with other use-cases (typedefs, etc)?



Comment at: clangd/XRefs.cpp:741
+  if (CXXRD)
+return getTypeHierarchy(CXXRD, Method, SourceMgr);
+

Maybe also return all base types for classes?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51311



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

[PATCH] D51333: Diagnose likely typos in include statements

[Brian Gesiak via Phabricator via cfe-commits]

modocache added subscribers: aaron.ballman, erikjv, modocache.
modocache added reviewers: aaron.ballman, erikjv.
modocache added a comment.

This looks good to me, but maybe some people who've modified this part of the 
codebase before could review this as well? @aaron.ballman added a fix-it for 
angled/quoted strings in https://reviews.llvm.org/rL160406, and more recently 
@erikjv modified some of this code in https://reviews.llvm.org/rL285057. If 
anyone could suggest other reviewers that would be great as well!


Repository:
  rC Clang

https://reviews.llvm.org/D51333



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

[PATCH] D50438: [clangd] Sort GoToDefinition results.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Sorry for the delays with this review


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50438



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

[PATCH] D50438: [clangd] Sort GoToDefinition results.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.
Herald added a subscriber: kadircet.



Comment at: clangd/XRefs.cpp:105
+// Sort results. Declarations being referenced explicitly come first.
+std::sort(Result.begin(), Result.end(),
+  [](const DeclInfo , const DeclInfo ) {

ilya-biryukov wrote:
> Maybe sort further at the callers instead?
> It would be a more intrusive change, but would also give a well-defined 
> result order for findDefinitions and other cases. E.g. findDefinitions 
> currently gives results in an arbitrary order (specifically, the one supplied 
> by DenseMap+sort) when multiple decls are returned.
> WDYT?
Just to clarify the original suggestion.

Maybe we can sort the `(location, is_explicit)` pairs instead of the `(decl, 
is_explicit)` pairs?
Sorting based on pointer equality (see `L.D < R.D`) provides non-deterministic 
results and we can have fully deterministic order on locations.

DeclarationAndMacrosFinder can return the results in arbitrary orders and the 
client code would be responsible for getting locations and finally sorting them.
WDYT?



Comment at: clangd/XRefs.cpp:296
   // Emit all symbol locations (declaration or definition) from AST.
-  for (const auto *D : Symbols.Decls) {
+  for (const auto  : Symbols.Decls) {
+const auto *D = DI.D;

Maybe use explicit type here?  Using 'auto'  is a bit confusing.



Comment at: clangd/XRefs.cpp:297
+  for (const auto  : Symbols.Decls) {
+const auto *D = DI.D;
 // Fake key for symbols don't have USR (no SymbolID).

Maybe use explicit type here too or rename the field from 'D' to something more 
descriptive (e.g. `Decl `)?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50438



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

[PATCH] D51302: [OpenCL] Relax diagnostics on OpenCL access qualifiers

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!




Comment at: test/SemaOpenCL/access-qualifier.cl:107
+
+kernel void image_wo_twice(write_only write_only image1d_t i){} // 
expected-warning {{duplicate 'write_only' declaration specifier}}
+kernel void image_wo_twice_typedef(write_only img1d_wo i){} // 
expected-warning {{duplicate 'write_only' declaration specifier}}

Could we change one `write_only` to `__write_only` to increase test coverage.


Repository:
  rC Clang

https://reviews.llvm.org/D51302



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

[PATCH] D51296: [Sema] Traverse vector types for ocl extensions support

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks! Could you please change the commit title tag:
[Sema] -> [OpenCL]




Comment at: lib/Sema/Sema.cpp:42
 #include "llvm/ADT/SmallSet.h"
+
 using namespace clang;

It would be better to undo this formatting change. Clang code base seems to be 
inconsistent on that anyway.


Repository:
  rC Clang

https://reviews.llvm.org/D51296



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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340838: Parse compile commands lazily in 
InterpolatingCompilationDatabase (authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D51314

Files:
  cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
  cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp

Index: cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -123,8 +123,8 @@
 struct TransferableCommand {
   // Flags that should not apply to all files are stripped from CommandLine.
   CompileCommand Cmd;
-  // Language detected from -x or the filename.
-  types::ID Type = types::TY_INVALID;
+  // Language detected from -x or the filename. Never TY_INVALID.
+  Optional Type;
   // Standard specified by -std.
   LangStandard::Kind Std = LangStandard::lang_unspecified;
 
@@ -171,7 +171,10 @@
 
 if (Std != LangStandard::lang_unspecified) // -std take precedence over -x
   Type = toType(LangStandard::getLangStandardForKind(Std).getLanguage());
-Type = foldType(Type);
+Type = foldType(*Type);
+// The contract is to store None instead of TY_INVALID.
+if (Type == types::TY_INVALID)
+  Type = llvm::None;
   }
 
   // Produce a CompileCommand for \p filename, based on this one.
@@ -181,10 +184,10 @@
 bool TypeCertain;
 auto TargetType = guessType(Filename, );
 // If the filename doesn't determine the language (.h), transfer with -x.
-if (!TypeCertain) {
+if (TargetType != types::TY_INVALID && !TypeCertain && Type) {
   TargetType = types::onlyPrecompileType(TargetType) // header?
-   ? types::lookupHeaderTypeForSourceType(Type)
-   : Type;
+   ? types::lookupHeaderTypeForSourceType(*Type)
+   : *Type;
   Result.CommandLine.push_back("-x");
   Result.CommandLine.push_back(types::getTypeName(TargetType));
 }
@@ -217,28 +220,31 @@
   }
 };
 
-// CommandIndex does the real work: given a filename, it produces the best
-// matching TransferableCommand by matching filenames. Basic strategy:
+// Given a filename, FileIndex picks the best matching file from the underlying
+// DB. This is the proxy file whose CompileCommand will be reused. The
+// heuristics incorporate file name, extension, and directory structure.
+// Strategy:
 // - Build indexes of each of the substrings we want to look up by.
 //   These indexes are just sorted lists of the substrings.
-// - Forward requests to the inner CDB. If it fails, we must pick a proxy.
 // - Each criterion corresponds to a range lookup into the index, so we only
 //   need O(log N) string comparisons to determine scores.
-// - We then break ties among the candidates with the highest score.
-class CommandIndex {
+//
+// Apart from path proximity signals, also takes file extensions into account
+// when scoring the candidates.
+class FileIndex {
 public:
-  CommandIndex(std::vector AllCommands)
-  : Commands(std::move(AllCommands)), Strings(Arena) {
+  FileIndex(std::vector Files)
+  : OriginalPaths(std::move(Files)), Strings(Arena) {
 // Sort commands by filename for determinism (index is a tiebreaker later).
-llvm::sort(
-Commands.begin(), Commands.end(),
-[](const TransferableCommand , const TransferableCommand ) {
-  return Left.Cmd.Filename < Right.Cmd.Filename;
-});
-for (size_t I = 0; I < Commands.size(); ++I) {
-  StringRef Path =
-  Strings.save(StringRef(Commands[I].Cmd.Filename).lower());
-  Paths.push_back({Path, I});
+llvm::sort(OriginalPaths.begin(), OriginalPaths.end());
+Paths.reserve(OriginalPaths.size());
+Types.reserve(OriginalPaths.size());
+Stems.reserve(OriginalPaths.size());
+for (size_t I = 0; I < OriginalPaths.size(); ++I) {
+  StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
+
+  Paths.emplace_back(Path, I);
+  Types.push_back(foldType(guessType(Path)));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, ++Dir)
@@ -250,29 +256,28 @@
 llvm::sort(Components.begin(), Components.end());
   }
 
-  bool empty() const { return Commands.empty(); }
+  bool empty() const { return Paths.empty(); }
 
-  // Returns the command that best fits OriginalFilename.
-  // Candidates with PreferLanguage will be chosen over others (unless it's
-  // TY_INVALID, or all candidates are bad).
-  const TransferableCommand (StringRef OriginalFilename,
- 

r340838 - Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Aug 28 09:15:56 2018
New Revision: 340838

URL: http://llvm.org/viewvc/llvm-project?rev=340838=rev
Log:
Parse compile commands lazily in InterpolatingCompilationDatabase

Summary:
This greatly reduces the time to read 'compile_commands.json'.
For Chromium on my machine it's now 0.7 seconds vs 30 seconds before the
change.

Reviewers: sammccall, jfb

Reviewed By: sammccall

Subscribers: mgrang, jfb, cfe-commits

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

Modified:
cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp

Modified: cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp?rev=340838=340837=340838=diff
==
--- cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp (original)
+++ cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp Tue Aug 28 
09:15:56 2018
@@ -123,8 +123,8 @@ static types::ID foldType(types::ID Lang
 struct TransferableCommand {
   // Flags that should not apply to all files are stripped from CommandLine.
   CompileCommand Cmd;
-  // Language detected from -x or the filename.
-  types::ID Type = types::TY_INVALID;
+  // Language detected from -x or the filename. Never TY_INVALID.
+  Optional Type;
   // Standard specified by -std.
   LangStandard::Kind Std = LangStandard::lang_unspecified;
 
@@ -171,7 +171,10 @@ struct TransferableCommand {
 
 if (Std != LangStandard::lang_unspecified) // -std take precedence over -x
   Type = toType(LangStandard::getLangStandardForKind(Std).getLanguage());
-Type = foldType(Type);
+Type = foldType(*Type);
+// The contract is to store None instead of TY_INVALID.
+if (Type == types::TY_INVALID)
+  Type = llvm::None;
   }
 
   // Produce a CompileCommand for \p filename, based on this one.
@@ -181,10 +184,10 @@ struct TransferableCommand {
 bool TypeCertain;
 auto TargetType = guessType(Filename, );
 // If the filename doesn't determine the language (.h), transfer with -x.
-if (!TypeCertain) {
+if (TargetType != types::TY_INVALID && !TypeCertain && Type) {
   TargetType = types::onlyPrecompileType(TargetType) // header?
-   ? types::lookupHeaderTypeForSourceType(Type)
-   : Type;
+   ? types::lookupHeaderTypeForSourceType(*Type)
+   : *Type;
   Result.CommandLine.push_back("-x");
   Result.CommandLine.push_back(types::getTypeName(TargetType));
 }
@@ -217,28 +220,31 @@ private:
   }
 };
 
-// CommandIndex does the real work: given a filename, it produces the best
-// matching TransferableCommand by matching filenames. Basic strategy:
+// Given a filename, FileIndex picks the best matching file from the underlying
+// DB. This is the proxy file whose CompileCommand will be reused. The
+// heuristics incorporate file name, extension, and directory structure.
+// Strategy:
 // - Build indexes of each of the substrings we want to look up by.
 //   These indexes are just sorted lists of the substrings.
-// - Forward requests to the inner CDB. If it fails, we must pick a proxy.
 // - Each criterion corresponds to a range lookup into the index, so we only
 //   need O(log N) string comparisons to determine scores.
-// - We then break ties among the candidates with the highest score.
-class CommandIndex {
+//
+// Apart from path proximity signals, also takes file extensions into account
+// when scoring the candidates.
+class FileIndex {
 public:
-  CommandIndex(std::vector AllCommands)
-  : Commands(std::move(AllCommands)), Strings(Arena) {
+  FileIndex(std::vector Files)
+  : OriginalPaths(std::move(Files)), Strings(Arena) {
 // Sort commands by filename for determinism (index is a tiebreaker later).
-llvm::sort(
-Commands.begin(), Commands.end(),
-[](const TransferableCommand , const TransferableCommand ) {
-  return Left.Cmd.Filename < Right.Cmd.Filename;
-});
-for (size_t I = 0; I < Commands.size(); ++I) {
-  StringRef Path =
-  Strings.save(StringRef(Commands[I].Cmd.Filename).lower());
-  Paths.push_back({Path, I});
+llvm::sort(OriginalPaths.begin(), OriginalPaths.end());
+Paths.reserve(OriginalPaths.size());
+Types.reserve(OriginalPaths.size());
+Stems.reserve(OriginalPaths.size());
+for (size_t I = 0; I < OriginalPaths.size(); ++I) {
+  StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
+
+  Paths.emplace_back(Path, I);
+  Types.push_back(foldType(guessType(Path)));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, 
++Dir)
@@ -250,29 +256,28 @@ 

[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks for the change. Having something like this makes total sense.

Mutating existing in-memory nodes looks shaky and requires making `Status` 
mutable, which also looks undesirable.
Maybe we could consider adding a new kind of `InMemoryNode` for hard links that 
would store the link target? We would have to "resolve" the link on each 
access, but that would probably allow to avoid changing `Status` interfaces and 
get rid of the  `UniqueID -> set of files' map. WDYT?

Also some generic comments wrt to the coding style, naming, etc.




Comment at: include/clang/Basic/VirtualFileSystem.h:80
 
+  void setUniqueID(llvm::sys::fs::UniqueID UID) { this->UID = UID; }
+  void setSize(uint64_t Size) { this->Size = Size; }

It seems `Status` was designed to be immutable.
Can we copy or reassign the whole status at the points where we need it?



Comment at: include/clang/Basic/VirtualFileSystem.h:359
 
+  /// Add a HardLink from one file to another.
+  /// Makes the UniqueID of To file same as that of From file. If CopyBuffer is

Maybe use the common spelling: "hard link"? Otherwise it feels like there is a 
HardLink class somewhere.



Comment at: include/clang/Basic/VirtualFileSystem.h:362
+  /// true then contents of from buffer is copied into the buffer of To file.
+  void addHardlink(const Twine , const Twine , bool CopyBuffer);
+

The links seem to only work for files at this point.
Maybe explicitly document what happens with directories?
Also, maybe document what happens with existing hard links and existing files?



Comment at: include/clang/Basic/VirtualFileSystem.h:362
+  /// true then contents of from buffer is copied into the buffer of To file.
+  void addHardlink(const Twine , const Twine , bool CopyBuffer);
+

ilya-biryukov wrote:
> The links seem to only work for files at this point.
> Maybe explicitly document what happens with directories?
> Also, maybe document what happens with existing hard links and existing files?
Maybe s/addHardlink/addHardLink?



Comment at: include/clang/Basic/VirtualFileSystem.h:468
 #endif // LLVM_CLANG_BASIC_VIRTUALFILESYSTEM_H
+

Accidental whitespace change?



Comment at: lib/Basic/VirtualFileSystem.cpp:516
 
+  void setBuffer(std::unique_ptr buffer) {
+this->Buffer = std::move(buffer);

s/buffer/Buffer



Comment at: lib/Basic/VirtualFileSystem.cpp:645
+  // Merging ToIDSet into FromIDSet.
+  for (detail::InMemoryNode *to_node : ToIDSet) {
+to_node->setUniqueId(FromStat->getUniqueID());

s/to_node/ToNode



Comment at: unittests/Basic/VirtualFileSystemTest.cpp:1608
 }
+

Accidental whitespace change?


Repository:
  rC Clang

https://reviews.llvm.org/D51359



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

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



Comment at: test/clang-tidy/Inputs/absl/external-file.h:1
+void DirectAcess2() { absl::strings_internal::InternalFunction(); }
+

hokein wrote:
> The file can not self-compile, and we should make it compilable.
> 
> And put the newly-added code at the end of the file, so that other test file 
> can keep unchanged.
how do I make it compilable?


https://reviews.llvm.org/D50542



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

[PATCH] D51360: [clang-tidy] Use simple string matching instead of Regex

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: hokein.
kbobyrev added a project: clang-tools-extra.
Herald added a subscriber: xazax.hun.

Instead of parsing and compiling the `llvm::Regex` each time, it's faster to 
use basic string matching for filename prefix check.


https://reviews.llvm.org/D51360

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


Index: clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
===
--- clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
+++ clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
@@ -6,9 +6,10 @@
 // License. See LICENSE.TXT for details.
 //
 
//===--===//
-//
+
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include 
 
 namespace clang {
 namespace ast_matchers {
@@ -28,10 +29,9 @@
 ///
 /// Usable as: Matcher, Matcher, Matcher,
 /// Matcher
-
-AST_POLYMORPHIC_MATCHER(isInAbseilFile,
-AST_POLYMORPHIC_SUPPORTED_TYPES(
-Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
+AST_POLYMORPHIC_MATCHER(
+isInAbseilFile, AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc,
+NestedNameSpecifierLoc)) {
   auto  = Finder->getASTContext().getSourceManager();
   SourceLocation Loc = Node.getBeginLoc();
   if (Loc.isInvalid())
@@ -41,10 +41,16 @@
   if (!FileEntry)
 return false;
   StringRef Filename = FileEntry->getName();
-  llvm::Regex RE(
-  "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|"
-  "synchronization|time|types|utility)");
-  return RE.match(Filename);
+  const llvm::SmallString<5> AbslPrefix("absl/");
+  if (!Filename.startswith(AbslPrefix))
+return false;
+  Filename = Filename.drop_front(AbslPrefix.size());
+  static const char *AbseilLibraries[] = {
+  "algorithm",   "base", "container", "debugging",
+  "memory",  "meta", "numeric",   "strings",
+  "synchronization", "time", "types", "utility"};
+  return std::any_of(std::begin(AbseilLibraries), std::end(AbseilLibraries),
+ [&](const char *Library) { return Library == Filename; });
 }
 
 } // namespace ast_matchers


Index: clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
===
--- clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
+++ clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
@@ -6,9 +6,10 @@
 // License. See LICENSE.TXT for details.
 //
 //===--===//
-//
+
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include 
 
 namespace clang {
 namespace ast_matchers {
@@ -28,10 +29,9 @@
 ///
 /// Usable as: Matcher, Matcher, Matcher,
 /// Matcher
-
-AST_POLYMORPHIC_MATCHER(isInAbseilFile,
-AST_POLYMORPHIC_SUPPORTED_TYPES(
-Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
+AST_POLYMORPHIC_MATCHER(
+isInAbseilFile, AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc,
+NestedNameSpecifierLoc)) {
   auto  = Finder->getASTContext().getSourceManager();
   SourceLocation Loc = Node.getBeginLoc();
   if (Loc.isInvalid())
@@ -41,10 +41,16 @@
   if (!FileEntry)
 return false;
   StringRef Filename = FileEntry->getName();
-  llvm::Regex RE(
-  "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|"
-  "synchronization|time|types|utility)");
-  return RE.match(Filename);
+  const llvm::SmallString<5> AbslPrefix("absl/");
+  if (!Filename.startswith(AbslPrefix))
+return false;
+  Filename = Filename.drop_front(AbslPrefix.size());
+  static const char *AbseilLibraries[] = {
+  "algorithm",   "base", "container", "debugging",
+  "memory",  "meta", "numeric",   "strings",
+  "synchronization", "time", "types", "utility"};
+  return std::any_of(std::begin(AbseilLibraries), std::end(AbseilLibraries),
+ [&](const char *Library) { return Library == Filename; });
 }
 
 } // namespace ast_matchers
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51321: [Tooling] Improve handling of CL-style options

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 162872.
hamzasood added a comment.

- Re-uploaded with full context.

Yeah, I encountered these issues while using clangd on Windows. I haven't run 
into any clangd issues after applying this patch, but admittedly my usage is 
very basic (pretty much just code completion). It may well be subtly broken in 
other places.


https://reviews.llvm.org/D51321

Files:
  lib/Tooling/InterpolatingCompilationDatabase.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp

Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -14,6 +14,8 @@
 #include "clang/Tooling/JSONCompilationDatabase.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/Path.h"
+
+#define GTEST_HAS_COMBINE 1
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -644,15 +646,50 @@
   }
 };
 
-class InterpolateTest : public ::testing::Test {
+enum class DriverExe { Clang, ClangCL };
+enum class DriverMode { None, GCC, CL };
+
+class InterpolateTest
+  : public ::testing::TestWithParam> {
+
 protected:
+  bool isTestingCL() const {
+switch (std::get<1>(GetParam())) {
+case DriverMode::None: return std::get<0>(GetParam()) == DriverExe::ClangCL;
+case DriverMode::GCC:  return false;
+case DriverMode::CL:   return true;
+default: llvm_unreachable("Invalid DriverMode");
+}
+  }
+  StringRef getClangExe() const {
+switch (std::get<0>(GetParam())) {
+case DriverExe::Clang:   return "clang";
+case DriverExe::ClangCL: return "clang-cl";
+default: llvm_unreachable("Invalid DriverExe");
+}
+  }
+  StringRef getDriverModeArg() const {
+switch (std::get<1>(GetParam())) {
+case DriverMode::None: return StringRef();
+case DriverMode::GCC:  return "--driver-mode=gcc";
+case DriverMode::CL:   return "--driver-mode=cl";
+default: llvm_unreachable("Invalid DriverMode");
+}
+  }
+
   // Adds an entry to the underlying compilation database.
   // A flag is injected: -D , so the command used can be identified.
   void add(llvm::StringRef File, llvm::StringRef Flags = "") {
-llvm::SmallVector Argv = {"clang", File, "-D", File};
+llvm::SmallVector Argv;
+Argv.push_back(getClangExe());
+if (!getDriverModeArg().empty())
+  Argv.push_back(getDriverModeArg());
+Argv.append({"-D", File});
 llvm::SplitString(Flags, Argv);
+
 llvm::SmallString<32> Dir;
 llvm::sys::path::system_temp_directory(false, Dir);
+
 Entries[path(File)].push_back(
 {Dir, path(File), {Argv.begin(), Argv.end()}, "foo.o"});
   }
@@ -675,67 +712,101 @@
 ->getCompileCommands(path(F));
 if (Results.empty())
   return "none";
-// drop the input file argument, so tests don't have to deal with path().
-EXPECT_EQ(Results[0].CommandLine.back(), path(F))
-<< "Last arg should be the file";
-Results[0].CommandLine.pop_back();
-return llvm::join(Results[0].CommandLine, " ");
+
+ArrayRef CmdLine = Results[0].CommandLine;
+
+// Drop the clang executable path.
+EXPECT_EQ(CmdLine.front(), getClangExe()) << "First arg should be clang";
+CmdLine = CmdLine.drop_front();
+
+// Drop the --driver-mode flag.
+if (!getDriverModeArg().empty()) {
+  EXPECT_EQ(CmdLine.front(), getDriverModeArg())
+  << "Expected driver mode arg";
+  CmdLine = CmdLine.drop_front();
+}
+
+// Drop the input file.
+EXPECT_EQ(CmdLine.back(), path(F)) << "Last arg should be the file";
+CmdLine = CmdLine.drop_back();
+
+return llvm::join(CmdLine, " ");
   }
 
   MemCDB::EntryMap Entries;
 };
 
-TEST_F(InterpolateTest, Nearby) {
+TEST_P(InterpolateTest, Nearby) {
   add("dir/foo.cpp");
   add("dir/bar.cpp");
   add("an/other/foo.cpp");
 
   // great: dir and name both match (prefix or full, case insensitive)
-  EXPECT_EQ(getCommand("dir/f.cpp"), "clang -D dir/foo.cpp");
-  EXPECT_EQ(getCommand("dir/FOO.cpp"), "clang -D dir/foo.cpp");
+  EXPECT_EQ(getCommand("dir/f.cpp"), "-D dir/foo.cpp");
+  EXPECT_EQ(getCommand("dir/FOO.cpp"), "-D dir/foo.cpp");
   // no name match. prefer matching dir, break ties by alpha
-  EXPECT_EQ(getCommand("dir/a.cpp"), "clang -D dir/bar.cpp");
+  EXPECT_EQ(getCommand("dir/a.cpp"), "-D dir/bar.cpp");
   // an exact name match beats one segment of directory match
   EXPECT_EQ(getCommand("some/other/bar.h"),
-"clang -D dir/bar.cpp -x c++-header");
+isTestingCL()
+? "-D dir/bar.cpp /TP"
+: "-D dir/bar.cpp -x c++-header");
   // two segments of directory match beat a prefix name match
-  EXPECT_EQ(getCommand("an/other/b.cpp"), "clang -D an/other/foo.cpp");
+  EXPECT_EQ(getCommand("an/other/b.cpp"), "-D an/other/foo.cpp");
   // if nothing matches at all, we still get the closest alpha match
   EXPECT_EQ(getCommand("below/some/obscure/path.cpp"),
-

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: test/clang-tidy/Inputs/absl/external-file.h:1
+void DirectAcess2() { absl::strings_internal::InternalFunction(); }
+

The file can not self-compile, and we should make it compilable.

And put the newly-added code at the end of the file, so that other test file 
can keep unchanged.



Comment at: test/clang-tidy/Inputs/absl/strings/internal-file.h:34
+} // namespace absl
+

nit: remove the empty line.


https://reviews.llvm.org/D50542



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

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

@aaron.ballman when you get the chance could you take another look at this and 
commit if it is ready? My internship ends rather soon and this is a tad time 
sensitive and I don't have commit access. Thank you for your time!


https://reviews.llvm.org/D51061



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

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

@aaron.ballman when you get the chance could you take another look at this and 
commit if it is ready? My internship ends rather soon and this is a tad time 
sensitive. Thank you for your time!


https://reviews.llvm.org/D51132



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

[PATCH] D51311: (WIP) Type hierarchy

[Simon Marchi via Phabricator via cfe-commits]

simark added inline comments.



Comment at: clangd/Protocol.h:889
+  // Does this node implement the method targeted by the request?
+  bool DeclaresMethod;
+

kadircet wrote:
> I think comment and the name is in contradiction here, do you mean 
> DefinesMethod?
Actually I think the comment is wrong.  Even if we only see a declaration of 
the method, it's enough to say that this type has its own version of the method.



Comment at: clangd/XRefs.cpp:669
+  const CXXMethodDecl *Candidate) {
+  // FIXME: How do I determine if Method overrides Candidate?
+

kadircet wrote:
> https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp
> 
> you can check this sample, which simply traverses all base classes and gets 
> methods with the same name, then checks whether one is overload of the other. 
> If it they are not overloads then one in the base classes gets overriden by 
> the other.
> 
> 
> Another approach could be to search for one method in others 
> overriden_methods.
> 
> But they are both inefficient, I would suggest calling these functions only 
> when one of them is defined in the base class of other then you can just 
> check for name equality and not being an overload.
> https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp

Did you mean to link to a particular line?

> you can check this sample, which simply traverses all base classes and gets 
> methods with the same name, then checks whether one is overload of the other. 
> If it they are not overloads then one in the base classes gets overriden by 
> the other.

> Another approach could be to search for one method in others 
> overriden_methods.

I have tried using `overriden_methods`, but it only contains methods marked 
virtual.  For this feature, I would like to consider non-virtual methods too.

> But they are both inefficient, I would suggest calling these functions only 
> when one of them is defined in the base class of other then you can just 
> check for name equality and not being an overload.

I am not sure I understand, but maybe it will be clearer when I will see the 
sample.



Comment at: clangd/XRefs.cpp:693
+
+std::cerr << " >>> A parent is " << ParentDecl->getName().str()
+  << std::endl;

kadircet wrote:
> If you plan to keep it for later debugging info, consider using {d,v}log
Yes of course :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51311



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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 162868.
ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added a comment.

- Cleanups


Repository:
  rC Clang

https://reviews.llvm.org/D51314

Files:
  lib/Tooling/InterpolatingCompilationDatabase.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp

Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -707,6 +707,7 @@
 
 TEST_F(InterpolateTest, Language) {
   add("dir/foo.cpp", "-std=c++17");
+  add("dir/bar.c", "");
   add("dir/baz.cee", "-x c");
 
   // .h is ambiguous, so we add explicit language flags
@@ -716,9 +717,11 @@
   EXPECT_EQ(getCommand("foo.hpp"), "clang -D dir/foo.cpp -std=c++17");
   // respect -x if it's already there.
   EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c-header");
-  // prefer a worse match with the right language
-  EXPECT_EQ(getCommand("foo.c"), "clang -D dir/baz.cee");
-  Entries.erase(path(StringRef("dir/baz.cee")));
+  // prefer a worse match with the right extension.
+  EXPECT_EQ(getCommand("foo.c"), "clang -D dir/bar.c");
+  // make sure we don't crash on queries with invalid extensions.
+  EXPECT_EQ(getCommand("foo.cce"), "clang -D dir/foo.cpp");
+  Entries.erase(path(StringRef("dir/bar.c")));
   // Now we transfer across languages, so drop -std too.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/foo.cpp");
 }
Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -123,8 +123,8 @@
 struct TransferableCommand {
   // Flags that should not apply to all files are stripped from CommandLine.
   CompileCommand Cmd;
-  // Language detected from -x or the filename.
-  types::ID Type = types::TY_INVALID;
+  // Language detected from -x or the filename. Never TY_INVALID.
+  Optional Type;
   // Standard specified by -std.
   LangStandard::Kind Std = LangStandard::lang_unspecified;
 
@@ -171,7 +171,10 @@
 
 if (Std != LangStandard::lang_unspecified) // -std take precedence over -x
   Type = toType(LangStandard::getLangStandardForKind(Std).getLanguage());
-Type = foldType(Type);
+Type = foldType(*Type);
+// The contract is to store None instead of TY_INVALID.
+if (Type == types::TY_INVALID)
+  Type = llvm::None;
   }
 
   // Produce a CompileCommand for \p filename, based on this one.
@@ -181,10 +184,10 @@
 bool TypeCertain;
 auto TargetType = guessType(Filename, );
 // If the filename doesn't determine the language (.h), transfer with -x.
-if (!TypeCertain) {
+if (TargetType != types::TY_INVALID && !TypeCertain && Type) {
   TargetType = types::onlyPrecompileType(TargetType) // header?
-   ? types::lookupHeaderTypeForSourceType(Type)
-   : Type;
+   ? types::lookupHeaderTypeForSourceType(*Type)
+   : *Type;
   Result.CommandLine.push_back("-x");
   Result.CommandLine.push_back(types::getTypeName(TargetType));
 }
@@ -217,28 +220,31 @@
   }
 };
 
-// CommandIndex does the real work: given a filename, it produces the best
-// matching TransferableCommand by matching filenames. Basic strategy:
+// Given a filename, FileIndex picks the best matching file from the underlying
+// DB. This is the proxy file whose CompileCommand will be reused. The
+// heuristics incorporate file name, extension, and directory structure.
+// Strategy:
 // - Build indexes of each of the substrings we want to look up by.
 //   These indexes are just sorted lists of the substrings.
-// - Forward requests to the inner CDB. If it fails, we must pick a proxy.
 // - Each criterion corresponds to a range lookup into the index, so we only
 //   need O(log N) string comparisons to determine scores.
-// - We then break ties among the candidates with the highest score.
-class CommandIndex {
+//
+// Apart from path proximity signals, also takes file extensions into account
+// when scoring the candidates.
+class FileIndex {
 public:
-  CommandIndex(std::vector AllCommands)
-  : Commands(std::move(AllCommands)), Strings(Arena) {
+  FileIndex(std::vector Files)
+  : OriginalPaths(std::move(Files)), Strings(Arena) {
 // Sort commands by filename for determinism (index is a tiebreaker later).
-llvm::sort(
-Commands.begin(), Commands.end(),
-[](const TransferableCommand , const TransferableCommand ) {
-  return Left.Cmd.Filename < Right.Cmd.Filename;
-});
-for (size_t I = 0; I < Commands.size(); ++I) {
-  StringRef Path =
-  Strings.save(StringRef(Commands[I].Cmd.Filename).lower());
-  Paths.push_back({Path, I});
+llvm::sort(OriginalPaths.begin(), OriginalPaths.end());
+

[PATCH] D51352: [clangd] Switch to Dex by default for the static index

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340828: [clangd] Switch to Dex by default for the static 
index (authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51352?vs=162865=162866#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51352

Files:
  clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -29,10 +29,11 @@
 using namespace clang;
 using namespace clang::clangd;
 
+// FIXME: remove this option when Dex is stable enough.
 static llvm::cl::opt
 UseDex("use-dex-index",
llvm::cl::desc("Use experimental Dex static index."),
-   llvm::cl::init(false), llvm::cl::Hidden);
+   llvm::cl::init(true), llvm::cl::Hidden);
 
 namespace {
 


Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -29,10 +29,11 @@
 using namespace clang;
 using namespace clang::clangd;
 
+// FIXME: remove this option when Dex is stable enough.
 static llvm::cl::opt
 UseDex("use-dex-index",
llvm::cl::desc("Use experimental Dex static index."),
-   llvm::cl::init(false), llvm::cl::Hidden);
+   llvm::cl::init(true), llvm::cl::Hidden);
 
 namespace {
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r340828 - [clangd] Switch to Dex by default for the static index

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Tue Aug 28 07:55:05 2018
New Revision: 340828

URL: http://llvm.org/viewvc/llvm-project?rev=340828=rev
Log:
[clangd] Switch to Dex by default for the static index

Dex is now mature enough to be used as the default static index. This
patch performs the switch but introduces a hidden flag to allow users
fallback to Mem in case something happens.

Reviewed by: ioeric

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

Modified:
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=340828=340827=340828=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Aug 28 07:55:05 2018
@@ -29,10 +29,11 @@
 using namespace clang;
 using namespace clang::clangd;
 
+// FIXME: remove this option when Dex is stable enough.
 static llvm::cl::opt
 UseDex("use-dex-index",
llvm::cl::desc("Use experimental Dex static index."),
-   llvm::cl::init(false), llvm::cl::Hidden);
+   llvm::cl::init(true), llvm::cl::Hidden);
 
 namespace {
 


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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:127
   // Language detected from -x or the filename.
-  types::ID Type = types::TY_INVALID;
+  // When set, cannot be TY_INVALID.
+  llvm::Optional Type;

(or just "never TY_INVALID" which would fit on prev line :-)



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:175
   Type = toType(LangStandard::getLangStandardForKind(Std).getLanguage());
-Type = foldType(Type);
+if (Type)
+  Type = foldType(*Type);

it's always set here, drop the condition.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:408
   BumpPtrAllocator Arena;
-  StringSaver Strings;
+  llvm::StringSaver Strings;
   // Indexes of candidates by certain substrings.

nit: no llvm:: :-)


Repository:
  rC Clang

https://reviews.llvm.org/D51314



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

[PATCH] D51352: [clangd] Switch to Dex by default for the static index

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 162865.
kbobyrev marked an inline comment as done.

https://reviews.llvm.org/D51352

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -29,10 +29,11 @@
 using namespace clang;
 using namespace clang::clangd;
 
+// FIXME: remove this option when Dex is stable enough.
 static llvm::cl::opt
 UseDex("use-dex-index",
llvm::cl::desc("Use experimental Dex static index."),
-   llvm::cl::init(false), llvm::cl::Hidden);
+   llvm::cl::init(true), llvm::cl::Hidden);
 
 namespace {
 


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -29,10 +29,11 @@
 using namespace clang;
 using namespace clang::clangd;
 
+// FIXME: remove this option when Dex is stable enough.
 static llvm::cl::opt
 UseDex("use-dex-index",
llvm::cl::desc("Use experimental Dex static index."),
-   llvm::cl::init(false), llvm::cl::Hidden);
+   llvm::cl::init(true), llvm::cl::Hidden);
 
 namespace {
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162864.
hugoeg marked 2 inline comments as done.
hugoeg added a comment.

made some major updates after no-namespace landed


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/external-file.h
  test/clang-tidy/Inputs/absl/strings/internal-file.h
  test/clang-tidy/abseil-no-internal-deps.cpp
  test/clang-tidy/abseil-no-namespace.cpp

Index: test/clang-tidy/abseil-no-namespace.cpp
===
--- test/clang-tidy/abseil-no-namespace.cpp
+++ test/clang-tidy/abseil-no-namespace.cpp
@@ -7,7 +7,7 @@
 
 /// Warning will be triggered on code that is not internal that is included.
 #include "absl/external-file.h"
-// CHECK: absl/external-file.h:1:11: warning: namespace 'absl' is reserved
+// CHECK: absl/external-file.h:3:11: warning: namespace 'absl' is reserved
 
 namespace absl {}
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: namespace 'absl' is reserved for implementation of the Abseil library and should not be opened in user code [abseil-no-namespace]
Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t,  -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-internal-deps' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:1:23: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil [abseil-no-internal-deps]
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+void OpeningNamespace() {
+  strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+SomeContainer b;
+std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/internal-file.h
===
--- test/clang-tidy/Inputs/absl/strings/internal-file.h
+++ test/clang-tidy/Inputs/absl/strings/internal-file.h
@@ -1 +1,34 @@
-namespace absl {}
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
+
Index: test/clang-tidy/Inputs/absl/external-file.h
===
--- test/clang-tidy/Inputs/absl/external-file.h
+++ test/clang-tidy/Inputs/absl/external-file.h
@@ -1 +1,3 @@
+void DirectAcess2() { absl::strings_internal::InternalFunction(); }
+
 namespace absl {}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -6,6 +6,7 @@
 .. toctree::
abseil-duration-division
abseil-faster-strsplit-delimiter
+   abseil-no-internal-deps
abseil-no-namespace

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 162863.
ilya-biryukov added a comment.

- Sort Paths, they are different from OriginalPaths, i.e. lowercased.


Repository:
  rC Clang

https://reviews.llvm.org/D51314

Files:
  lib/Tooling/InterpolatingCompilationDatabase.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp

Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -707,6 +707,7 @@
 
 TEST_F(InterpolateTest, Language) {
   add("dir/foo.cpp", "-std=c++17");
+  add("dir/bar.c", "");
   add("dir/baz.cee", "-x c");
 
   // .h is ambiguous, so we add explicit language flags
@@ -716,9 +717,11 @@
   EXPECT_EQ(getCommand("foo.hpp"), "clang -D dir/foo.cpp -std=c++17");
   // respect -x if it's already there.
   EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c-header");
-  // prefer a worse match with the right language
-  EXPECT_EQ(getCommand("foo.c"), "clang -D dir/baz.cee");
-  Entries.erase(path(StringRef("dir/baz.cee")));
+  // prefer a worse match with the right extension.
+  EXPECT_EQ(getCommand("foo.c"), "clang -D dir/bar.c");
+  // make sure we don't crash on queries with invalid extensions.
+  EXPECT_EQ(getCommand("foo.cce"), "clang -D dir/foo.cpp");
+  Entries.erase(path(StringRef("dir/bar.c")));
   // Now we transfer across languages, so drop -std too.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/foo.cpp");
 }
Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -124,7 +124,8 @@
   // Flags that should not apply to all files are stripped from CommandLine.
   CompileCommand Cmd;
   // Language detected from -x or the filename.
-  types::ID Type = types::TY_INVALID;
+  // When set, cannot be TY_INVALID.
+  llvm::Optional Type;
   // Standard specified by -std.
   LangStandard::Kind Std = LangStandard::lang_unspecified;
 
@@ -171,7 +172,11 @@
 
 if (Std != LangStandard::lang_unspecified) // -std take precedence over -x
   Type = toType(LangStandard::getLangStandardForKind(Std).getLanguage());
-Type = foldType(Type);
+if (Type)
+  Type = foldType(*Type);
+// The contract is to store None instead of TY_INVALID.
+if (Type == types::TY_INVALID)
+  Type = llvm::None;
   }
 
   // Produce a CompileCommand for \p filename, based on this one.
@@ -181,10 +186,10 @@
 bool TypeCertain;
 auto TargetType = guessType(Filename, );
 // If the filename doesn't determine the language (.h), transfer with -x.
-if (!TypeCertain) {
+if (TargetType != types::TY_INVALID && !TypeCertain && Type) {
   TargetType = types::onlyPrecompileType(TargetType) // header?
-   ? types::lookupHeaderTypeForSourceType(Type)
-   : Type;
+   ? types::lookupHeaderTypeForSourceType(*Type)
+   : *Type;
   Result.CommandLine.push_back("-x");
   Result.CommandLine.push_back(types::getTypeName(TargetType));
 }
@@ -217,28 +222,31 @@
   }
 };
 
-// CommandIndex does the real work: given a filename, it produces the best
-// matching TransferableCommand by matching filenames. Basic strategy:
+// Given a filename, FileIndex picks the best matching file from the underlying
+// DB. This is the proxy file whose CompileCommand will be reused. The
+// heuristics incorporate file name, extension, and directory structure.
+// Strategy:
 // - Build indexes of each of the substrings we want to look up by.
 //   These indexes are just sorted lists of the substrings.
-// - Forward requests to the inner CDB. If it fails, we must pick a proxy.
 // - Each criterion corresponds to a range lookup into the index, so we only
 //   need O(log N) string comparisons to determine scores.
-// - We then break ties among the candidates with the highest score.
-class CommandIndex {
+//
+// Apart from path proximity signals, also takes file extensions into account
+// when scoring the candidates.
+class FileIndex {
 public:
-  CommandIndex(std::vector AllCommands)
-  : Commands(std::move(AllCommands)), Strings(Arena) {
+  FileIndex(std::vector Files)
+  : OriginalPaths(std::move(Files)), Strings(Arena) {
 // Sort commands by filename for determinism (index is a tiebreaker later).
-llvm::sort(
-Commands.begin(), Commands.end(),
-[](const TransferableCommand , const TransferableCommand ) {
-  return Left.Cmd.Filename < Right.Cmd.Filename;
-});
-for (size_t I = 0; I < Commands.size(); ++I) {
-  StringRef Path =
-  Strings.save(StringRef(Commands[I].Cmd.Filename).lower());
-  Paths.push_back({Path, I});
+llvm::sort(OriginalPaths.begin(), OriginalPaths.end());
+Paths.reserve(OriginalPaths.size());
+

[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

[UTKARSH SAXENA via Phabricator via cfe-commits]

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

Added support of creating a hardlink from one file to another file.
After a hardlink is added between two files, both file will have the same:

1. UniqueID (inode)
2. Size
3. Buffer

This will bring replay of compilation closer to the actual compilation. There 
are instances where clang checks for the UniqueID of the file/header to be 
loaded which leads to a different behavior during replay as all files have 
different UniqueIDs.


Repository:
  rC Clang

https://reviews.llvm.org/D51359

Files:
  include/clang/Basic/VirtualFileSystem.h
  lib/Basic/VirtualFileSystem.cpp
  unittests/Basic/VirtualFileSystemTest.cpp

Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -17,6 +17,7 @@
 #include "llvm/Support/SourceMgr.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 
 using namespace clang;
 using namespace llvm;
@@ -695,6 +696,20 @@
   InMemoryFileSystemTest()
   : FS(/*UseNormalizedPaths=*/false),
 NormalizedFS(/*UseNormalizedPaths=*/true) {}
+
+public:
+  void ExpectHardLink(Twine From, Twine To) {
+auto OpenedFrom = FS.openFileForRead(From);
+ASSERT_FALSE(OpenedFrom.getError());
+auto OpenedTo = FS.openFileForRead(To);
+ASSERT_FALSE(OpenedTo.getError());
+ASSERT_EQ((*OpenedFrom)->status()->getSize(),
+  (*OpenedTo)->status()->getSize());
+ASSERT_EQ((*OpenedFrom)->status()->getUniqueID(),
+  (*OpenedTo)->status()->getUniqueID());
+ASSERT_EQ((*OpenedFrom)->getBuffer(From)->get()->getBuffer(),
+  (*OpenedTo)->getBuffer(To)->get()->getBuffer());
+  }
 };
 
 TEST_F(InMemoryFileSystemTest, IsEmpty) {
@@ -958,6 +973,96 @@
   ASSERT_EQ("../b/c", getPosixPath(It->getName()));
 }
 
+TEST_F(InMemoryFileSystemTest, AddHardLinkWithToFileNotAddedBefore) {
+  std::pair From = {"/path/to/FROM/file",
+"content of FROM file"};
+  std::pair To = {"/path/to/TO/file",
+  "content of TO file"};
+  FS.addFile(From.first, 0, MemoryBuffer::getMemBuffer(From.second));
+  FS.addHardlink(From.first, To.first, /*CopyBuffer=*/true);
+  ExpectHardLink(From.first, To.first);
+}
+
+TEST_F(InMemoryFileSystemTest, AddHardLinkWithToFileThatWasAlreadyAdded) {
+  std::pair From = {"/path/to/FROM/file",
+"content of FROM file"};
+  std::pair To = {"/path/to/TO/file",
+  "content of TO file"};
+  FS.addFile(From.first, 0, MemoryBuffer::getMemBuffer(From.second));
+  FS.addFile(To.first, 0, MemoryBuffer::getMemBuffer(To.second));
+  FS.addHardlink(From.first, To.first, /*CopyBuffer=*/true);
+  ExpectHardLink(From.first, To.first);
+}
+
+TEST_F(InMemoryFileSystemTest, AddMoreThanOneHardLinkToSameFile) {
+  std::pair From = {"/path/to/FROM/file",
+"content of FROM file"};
+  std::pair To = {"/path/to/TO/file",
+  "content of TO file"};
+  FS.addFile(From.first, 0, MemoryBuffer::getMemBuffer(From.second));
+  FS.addFile(To.first, 0, MemoryBuffer::getMemBuffer(To.second));
+  for (int i = 0; i < 5; ++i)
+FS.addHardlink(From.first, To.first, /*CopyBuffer=*/true);
+  ExpectHardLink(From.first, To.first);
+}
+
+TEST_F(InMemoryFileSystemTest, AddHardLinkToAlreadyLinkedFiles) {
+  std::pair From = {"/path/to/FROM/file",
+  "content of FROM file"};
+  FS.addFile(From.first, 0, MemoryBuffer::getMemBuffer(From.second.data()));
+  std::vector> ToFiles;
+  ToFiles.reserve(5);
+  for (int i = 0; i < 5; ++i) {
+ToFiles.push_back(
+{"/path/to/TO/file_" + std::to_string(i), "content of to file"});
+  }
+  for (const auto  : ToFiles) {
+FS.addFile(to.first.data(), 0,
+   MemoryBuffer::getMemBuffer(to.second.data()));
+  }
+  for (int i = 1; i < 5; ++i) {
+FS.addHardlink(ToFiles[i - 1].first.data(), ToFiles[i].first.data(),
+   /*CopyBuffer=*/false);
+  }
+  FS.addHardlink(From.first, ToFiles[0].first.data(), /*CopyBuffer=*/true);
+  for (const auto  : ToFiles) {
+ExpectHardLink(From.first, to.first.data());
+  }
+}
+
+TEST_F(InMemoryFileSystemTest, AddHardLinkBetweenTwoGroups) {
+  std::vector> FileGroup1, FileGroup2;
+  for (int i = 0; i < 5; ++i) {
+string str_i = std::to_string(i);
+FileGroup1.push_back(
+{"/path/to/group/1/file_" + str_i, "group1, file =" + str_i});
+FileGroup2.push_back(
+{"/path/to/group/2/file_" + str_i, "group2, file =" + str_i});
+  }
+  // Create files for both groups.
+  for (int i = 0; i < 5; ++i) {
+FS.addFile(FileGroup1[i].first.data(), 0,
+   MemoryBuffer::getMemBuffer(FileGroup1[i].second.data()));
+FS.addFile(FileGroup2[i].first.data(), 0,

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 162861.
ilya-biryukov added a comment.

- Handle TransferableCommands with TY_INVALID type (never transfer -x flag for 
those)
- Add a test with invalid extensions, seen a crash while experimenting
- Update the test wrt to the new behavior.


Repository:
  rC Clang

https://reviews.llvm.org/D51314

Files:
  lib/Tooling/InterpolatingCompilationDatabase.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp

Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -707,6 +707,7 @@
 
 TEST_F(InterpolateTest, Language) {
   add("dir/foo.cpp", "-std=c++17");
+  add("dir/bar.c", "");
   add("dir/baz.cee", "-x c");
 
   // .h is ambiguous, so we add explicit language flags
@@ -716,9 +717,11 @@
   EXPECT_EQ(getCommand("foo.hpp"), "clang -D dir/foo.cpp -std=c++17");
   // respect -x if it's already there.
   EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c-header");
-  // prefer a worse match with the right language
-  EXPECT_EQ(getCommand("foo.c"), "clang -D dir/baz.cee");
-  Entries.erase(path(StringRef("dir/baz.cee")));
+  // prefer a worse match with the right extension.
+  EXPECT_EQ(getCommand("foo.c"), "clang -D dir/bar.c");
+  // make sure we don't crash on queries with invalid extensions.
+  EXPECT_EQ(getCommand("foo.cce"), "clang -D dir/foo.cpp");
+  Entries.erase(path(StringRef("dir/bar.c")));
   // Now we transfer across languages, so drop -std too.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/foo.cpp");
 }
Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -124,7 +124,8 @@
   // Flags that should not apply to all files are stripped from CommandLine.
   CompileCommand Cmd;
   // Language detected from -x or the filename.
-  types::ID Type = types::TY_INVALID;
+  // When set, cannot be TY_INVALID.
+  llvm::Optional Type;
   // Standard specified by -std.
   LangStandard::Kind Std = LangStandard::lang_unspecified;
 
@@ -171,7 +172,11 @@
 
 if (Std != LangStandard::lang_unspecified) // -std take precedence over -x
   Type = toType(LangStandard::getLangStandardForKind(Std).getLanguage());
-Type = foldType(Type);
+if (Type)
+  Type = foldType(*Type);
+// The contract is to store None instead of TY_INVALID.
+if (Type == types::TY_INVALID)
+  Type = llvm::None;
   }
 
   // Produce a CompileCommand for \p filename, based on this one.
@@ -181,10 +186,10 @@
 bool TypeCertain;
 auto TargetType = guessType(Filename, );
 // If the filename doesn't determine the language (.h), transfer with -x.
-if (!TypeCertain) {
+if (TargetType != types::TY_INVALID && !TypeCertain && Type) {
   TargetType = types::onlyPrecompileType(TargetType) // header?
-   ? types::lookupHeaderTypeForSourceType(Type)
-   : Type;
+   ? types::lookupHeaderTypeForSourceType(*Type)
+   : *Type;
   Result.CommandLine.push_back("-x");
   Result.CommandLine.push_back(types::getTypeName(TargetType));
 }
@@ -217,62 +222,64 @@
   }
 };
 
-// CommandIndex does the real work: given a filename, it produces the best
-// matching TransferableCommand by matching filenames. Basic strategy:
+// Given a filename, FileIndex picks the best matching file from the underlying
+// DB. This is the proxy file whose CompileCommand will be reused. The
+// heuristics incorporate file name, extension, and directory structure.
+// Strategy:
 // - Build indexes of each of the substrings we want to look up by.
 //   These indexes are just sorted lists of the substrings.
-// - Forward requests to the inner CDB. If it fails, we must pick a proxy.
 // - Each criterion corresponds to a range lookup into the index, so we only
 //   need O(log N) string comparisons to determine scores.
-// - We then break ties among the candidates with the highest score.
-class CommandIndex {
+//
+// Apart from path proximity signals, also takes file extensions into account
+// when scoring the candidates.
+class FileIndex {
 public:
-  CommandIndex(std::vector AllCommands)
-  : Commands(std::move(AllCommands)), Strings(Arena) {
+  FileIndex(std::vector Files)
+  : OriginalPaths(std::move(Files)), Strings(Arena) {
 // Sort commands by filename for determinism (index is a tiebreaker later).
-llvm::sort(
-Commands.begin(), Commands.end(),
-[](const TransferableCommand , const TransferableCommand ) {
-  return Left.Cmd.Filename < Right.Cmd.Filename;
-});
-for (size_t I = 0; I < Commands.size(); ++I) {
-  StringRef Path =
-  Strings.save(StringRef(Commands[I].Cmd.Filename).lower());
-