[PATCH] D59756: [clangd] Support dependent bases in type hierarchy

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 194002.
nridge marked 6 inline comments as done.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59756

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp

Index: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp
===
--- clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp
+++ clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp
@@ -291,9 +291,7 @@
   EXPECT_THAT(typeParents(ChildSpec), ElementsAre(Parent));
 }
 
-// This is disabled for now, because support for dependent bases
-// requires additional measures to avoid infinite recursion.
-TEST(DISABLED_TypeParents, DependentBase) {
+TEST(TypeParents, DependentBase) {
   Annotations Source(R"cpp(
 template 
 struct Parent {};
@@ -383,10 +381,10 @@
   }
 }
 
-TEST(TypeHierarchy, RecursiveHierarchy1) {
+TEST(TypeHierarchy, RecursiveHierarchyUnbounded) {
   Annotations Source(R"cpp(
   template 
-  struct S : S {};
+  struct $SDef[[S]] : S {};
 
   S^<0> s;
   )cpp");
@@ -399,62 +397,57 @@
   ASSERT_TRUE(!AST.getDiagnostics().empty());
 
   // Make sure getTypeHierarchy() doesn't get into an infinite recursion.
+  // FIXME(nridge): It would be preferable if the type hierarchy gave us type
+  // names (e.g. "S<0>" for the child and "S<1>" for the parent) rather than
+  // template names (e.g. "S").
   llvm::Optional Result = getTypeHierarchy(
   AST, Source.points()[0], 0, TypeHierarchyDirection::Parents);
   ASSERT_TRUE(bool(Result));
-  EXPECT_THAT(*Result,
-  AllOf(WithName("S"), WithKind(SymbolKind::Struct), Parents()));
+  EXPECT_THAT(
+  *Result,
+  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+  SelectionRangeIs(Source.range("SDef")), Parents();
 }
 
-TEST(TypeHierarchy, RecursiveHierarchy2) {
+TEST(TypeHierarchy, RecursiveHierarchyBounded) {
   Annotations Source(R"cpp(
   template 
-  struct S : S {};
+  struct $SDef[[S]] : S {};
 
   template <>
   struct S<0>{};
 
-  S^<2> s;
-  )cpp");
-
-  TestTU TU = TestTU::withCode(Source.code());
-  auto AST = TU.build();
-
-  ASSERT_TRUE(AST.getDiagnostics().empty());
-
-  // Make sure getTypeHierarchy() doesn't get into an infinite recursion.
-  llvm::Optional Result = getTypeHierarchy(
-  AST, Source.points()[0], 0, TypeHierarchyDirection::Parents);
-  ASSERT_TRUE(bool(Result));
-  EXPECT_THAT(*Result,
-  AllOf(WithName("S"), WithKind(SymbolKind::Struct), Parents()));
-}
-
-TEST(TypeHierarchy, RecursiveHierarchy3) {
-  Annotations Source(R"cpp(
-  template 
-  struct S : S {};
-
-  template <>
-  struct S<0>{};
+  S$SRefConcrete^<2> s;
 
   template 
   struct Foo {
-S^ s;
-  };
-  )cpp");
+S$SRefDependent^ s;
+  };)cpp");
 
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
 
   ASSERT_TRUE(AST.getDiagnostics().empty());
 
-  // Make sure getTypeHierarchy() doesn't get into an infinite recursion.
+  // Make sure getTypeHierarchy() doesn't get into an infinite recursion
+  // for either a concrete starting point or a dependent starting point.
   llvm::Optional Result = getTypeHierarchy(
-  AST, Source.points()[0], 0, TypeHierarchyDirection::Parents);
+  AST, Source.point("SRefConcrete"), 0, TypeHierarchyDirection::Parents);
+  ASSERT_TRUE(bool(Result));
+  EXPECT_THAT(
+  *Result,
+  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+  SelectionRangeIs(Source.range("SDef")), Parents();
+  Result = getTypeHierarchy(AST, Source.point("SRefDependent"), 0,
+TypeHierarchyDirection::Parents);
   ASSERT_TRUE(bool(Result));
-  EXPECT_THAT(*Result,
-  AllOf(WithName("S"), WithKind(SymbolKind::Struct), Parents()));
+  EXPECT_THAT(
+  *Result,
+  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+  SelectionRangeIs(Source.range("SDef")), Parents();
 }
 
 } // namespace
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -876,21 +876,40 @@
   return THI;
 }
 
-static Optional getTypeAncestors(const CXXRecordDecl ,
-ASTContext ) {
+using RecursionProtectionSet = llvm::SmallSet;
+
+static Optional
+getTypeAncestors(const CXXRecordDecl , ASTContext ,
+ RecursionProtectionSet ) {
   Optional Result = declToTypeHierarchyItem(ASTCtx, CXXRD);
   if (!Result)
 return Result;
 
   

[PATCH] D59407: [clangd] Add RelationSlab

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D59407#1456394 , @sammccall wrote:

> One thing that strikes me here is that this case is very similar to our 
> existing Ref data - it's basically a subset, but with a symbolid payload 
> instead of location. We could consider just adding the SymbolID to Refs - 
> it'd blow up the size of that by 50%, but we may not do much better with some 
> other representation, and it would avoid adding any new complexity.


Note that this was considered and rejected earlier (see here 
 and here 
), though that discussion did not 
consider denser relations like callgraph.

Given the additional information and perspectives from the conversation here, I 
have two suggestions for potential ways forward:

**Approach 1: Add SymbolID to Refs**

- This would be initially wasteful when only subtypes use it, but if we then 
build callgraph on top of it as well it will become significantly less wasteful.
- This has the benefit that we don't have duplicate information for 
find-references and callgraph (they both use Refs).
- This approach probably adds the least amount of complexity overall.

**Approach 2: Add a RelationSlab storing (subject, predicate, object) triples, 
intended for sparse relations**

- This would allow us to implement features that require sparse relations, such 
as subtypes and overrides, without any significant increase to index size.
- If we later want to add dense relations like callgraph, we'd use a different 
mechanism for them (possibly by adding SymbolID to Refs as in approach 1).
- If we do end up adding SymbolID to Refs for callgraph, and want to start 
using that for sparse relations too, we can rip out RelationSlab.
- This also adds relatively little complexity, though slightly more than 
approach 1, and we are throwing away some code if we end up adding SymbolID to 
Refs eventually.

Any thoughts on which of the approaches to take, or something else altogether? 
It would be good to have confidence that the chosen approach will pass code 
review before I spend time implementing it :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[PATCH] D60308: Leave alone the semicolon after the MacroBlockBegin name

[Owen Pan via Phabricator via cfe-commits]

owenpan abandoned this revision.
owenpan added a comment.

The semicolon is redundant but makes the MacroBlockBegin name stand out more. 
Nonetheless, I agree it's not a bug and we should leave the current behavior as 
is.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60308



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

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

Got rid of the confusing SuppressSRet logic. The "inreg" attribute is now used 
to indicate sret returns in X0.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60349



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

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang created this revision.
mgrang added reviewers: rnk, efriedma, TomTan, ssijaric.
Herald added subscribers: kristof.beyls, javed.absar.
Herald added a project: clang.

Related llvm patch: D60348 .


Repository:
  rC Clang

https://reviews.llvm.org/D60349

Files:
  include/clang/CodeGen/CGFunctionInfo.h
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp
  test/CodeGen/arm64-microsoft-arguments.cpp

Index: test/CodeGen/arm64-microsoft-arguments.cpp
===
--- test/CodeGen/arm64-microsoft-arguments.cpp
+++ test/CodeGen/arm64-microsoft-arguments.cpp
@@ -14,7 +14,7 @@
 struct pod bar() { return s; }
 struct non_pod foo() { return t; }
 // CHECK: define {{.*}} void @{{.*}}bar{{.*}}(%struct.pod* noalias sret %agg.result)
-// CHECK: define {{.*}} void @{{.*}}foo{{.*}}(%struct.non_pod* noalias %agg.result)
+// CHECK: define {{.*}} void @{{.*}}foo{{.*}}(%struct.non_pod* inreg noalias sret %agg.result)
 
 
 // Check instance methods.
@@ -22,4 +22,4 @@
 struct Baz { pod2 baz(); };
 
 int qux() { return Baz().baz().x; }
-// CHECK: declare {{.*}} void @{{.*}}baz@Baz{{.*}}(%struct.Baz*, %struct.pod2*)
+// CHECK: declare {{.*}} void @{{.*}}baz@Baz{{.*}}(%struct.Baz*, %struct.pod2* sret)
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1056,28 +1056,25 @@
   if (!RD)
 return false;
 
+  // Note: The "inreg" attribute is used to signal that the struct return
+  // should be in X0.
+  bool sretInX0 = (CGM.getTarget().getTriple().getArch() ==
+   llvm::Triple::aarch64) && !RD->isPOD();
+
   CharUnits Align = CGM.getContext().getTypeAlignInChars(FI.getReturnType());
   if (FI.isInstanceMethod()) {
 // If it's an instance method, aggregates are always returned indirectly via
 // the second parameter.
 FI.getReturnInfo() = ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
 FI.getReturnInfo().setSRetAfterThis(FI.isInstanceMethod());
+FI.getReturnInfo().setInReg(sretInX0);
 
-// aarch64-windows requires that instance methods use X1 for the return
-// address. So for aarch64-windows we do not mark the
-// return as SRet.
-FI.getReturnInfo().setSuppressSRet(CGM.getTarget().getTriple().getArch() ==
-   llvm::Triple::aarch64);
 return true;
   } else if (!RD->isPOD()) {
 // If it's a free function, non-POD types are returned indirectly.
 FI.getReturnInfo() = ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
+FI.getReturnInfo().setInReg(sretInX0);
 
-// aarch64-windows requires that non-POD, non-instance returns use X0 for
-// the return address. So for aarch64-windows we do not mark the return as
-// SRet.
-FI.getReturnInfo().setSuppressSRet(CGM.getTarget().getTriple().getArch() ==
-   llvm::Triple::aarch64);
 return true;
   }
 
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -1999,8 +1999,7 @@
   // Attach attributes to sret.
   if (IRFunctionArgs.hasSRetArg()) {
 llvm::AttrBuilder SRETAttrs;
-if (!RetAI.getSuppressSRet())
-  SRETAttrs.addAttribute(llvm::Attribute::StructRet);
+SRETAttrs.addAttribute(llvm::Attribute::StructRet);
 hasUsedSRet = true;
 if (RetAI.getInReg())
   SRETAttrs.addAttribute(llvm::Attribute::InReg);
Index: include/clang/CodeGen/CGFunctionInfo.h
===
--- include/clang/CodeGen/CGFunctionInfo.h
+++ include/clang/CodeGen/CGFunctionInfo.h
@@ -95,7 +95,6 @@
   bool InReg : 1;   // isDirect() || isExtend() || isIndirect()
   bool CanBeFlattened: 1;   // isDirect()
   bool SignExt : 1; // isExtend()
-  bool SuppressSRet : 1;// isIndirect()
 
   bool canHavePaddingType() const {
 return isDirect() || isExtend() || isIndirect() || isExpand();
@@ -111,14 +110,13 @@
   }
 
   ABIArgInfo(Kind K)
-  : TheKind(K), PaddingInReg(false), InReg(false), SuppressSRet(false) {
+  : TheKind(K), PaddingInReg(false), InReg(false) {
   }
 
 public:
   ABIArgInfo()
   : TypeData(nullptr), PaddingType(nullptr), DirectOffset(0),
-TheKind(Direct), PaddingInReg(false), InReg(false),
-SuppressSRet(false) {}
+TheKind(Direct), PaddingInReg(false), InReg(false) {}
 
   static ABIArgInfo getDirect(llvm::Type *T = nullptr, unsigned Offset = 0,
   llvm::Type *Padding = nullptr,
@@ -407,16 +405,6 @@
 CanBeFlattened = Flatten;
   }
 
-  bool getSuppressSRet() const {
-assert(isIndirect() && "Invalid kind!");
-return SuppressSRet;
-  }
-
-  void setSuppressSRet(bool Suppress) {
-assert(isIndirect() && "Invalid kind!");
-SuppressSRet = Suppress;
-  }
-
   

[PATCH] D60283: [clang-cl] Don't emit checksums when compiling a preprocessed CPP

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

+@rsmith for the PresumedLoc change

From glancing on the PresumedLoc computation code, I think this bool might be 
the way to go. You could make it a bit more "free" by stealing a bit from the 
column, if we're concerned about size.

FYI, I'm off to EuroLLVM after this and return in about two weeks.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60283



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

[PATCH] D60237: [MS] Add metadata for __declspec(allocator)

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm, with some suggestions to improve the test




Comment at: clang/test/CodeGen/debug-info-codeview-heapallocsite.c:16-19
+struct Foo foo_buf[1024];
+__declspec(allocator) struct Foo *alloc_foo() {
+  return _buf[0];
+}

Personally, I think it's nicer to leave the allocator undefined, because then 
clang doesn't generate IR for it, and FileCheck doesn't have to scan through 
it. When these tests fail, humans have to generate the IR, read it, and try to 
understand it, and the less there is, the easier it is to understand.



Comment at: clang/test/CodeGen/debug-info-codeview-heapallocsite.c:28
+
+// CHECK-LABEL: define {{.*}}%struct.Foo* @alloc_foo
+// CHECK: call %struct.Foo* @alloc_foo(){{.*}} !heapallocsite [[DBG2:!.*]]

I think you want to look for `call_alloc_foo` in this CHECK.


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

https://reviews.llvm.org/D60237



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

[PATCH] D59977: [Lexer] Fix an off-by-one bug in Lexer::getAsCharRange() - NFC.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Namely, 
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/46376
 :

  Running ['clang-tidy', 
'/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp',
 '-fix', '--checks=-*,google-readability-namespace-comments', '--', 
'--std=c++11', '-nostdinc++']...
   clang-tidy output ---
  3 warnings generated.
  
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp:14:2:
 warning: namespace 'n3' not terminated with a closing comment 
[google-readability-namespace-comments]
  }}
   ^
  
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp:14:2:
 note: FIX-IT applied suggested code changes
  
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp:4:11:
 note: namespace 'n3' starts here
  namespace n3 {
^
  
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp:14:3:
 warning: namespace 'n1::n2 {' not terminated with a closing comment 
[google-readability-namespace-comments]
  }}
^
  // namespace n1::n2 {
  
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp:14:3:
 note: FIX-IT applied suggested code changes
  
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp:3:11:
 note: namespace 'n1::n2 {' starts here
  namespace n1::n2 {
^
  clang-tidy applied 2 of 2 suggested fixes.
  Suppressed 1 warnings (1 with check filters).
  
  --
  -- Fixes -
  --- 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp.orig
 2019-04-05 15:03:02.741836756 -0700
  +++ 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp
  2019-04-05 15:03:02.753836825 -0700
  @@ -11,7 +11,8 @@
   //
   //
   //
  -}}
  +}  // namespace n3
  +}  // namespace n1::n2 {
   //
   //
   
  
  --
  FileCheck failed:
  
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/google-readability-nested-namespace-comments.cpp:12:20:
 error: CHECK-MESSAGES: expected string not found in input
  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: namespace 'n1::n2' not terminated 
with a closing comment [google-readability-namespace-comments]
 ^
  
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp.msg:7:1:
 note: scanning from here
  namespace n3 {
  ^
  
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp.msg:7:1:
 note: with expression "@LINE+2" equal to "14"
  namespace n3 {
  ^
  
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp.msg:9:191:
 note: possible intended match here
  
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp:14:3:
 warning: namespace 'n1::n2 {' not terminated with a closing comment 
[google-readability-namespace-comments]



  FAIL: Extra Tools Unit Tests :: 
clangd/./ClangdTests/SelectionTest.CommonAncestor (19592 of 48320)
   TEST 'Extra Tools Unit Tests :: 
clangd/./ClangdTests/SelectionTest.CommonAncestor' FAILED 
  Note: Google Test filter = SelectionTest.CommonAncestor
  [==] Running 1 test from 1 test case.
  [--] Global test environment set-up.
 

[PATCH] D59977: [Lexer] Fix an off-by-one bug in Lexer::getAsCharRange() - NFC.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ reopened this revision.
NoQ added a reviewer: alexfh.
NoQ added a comment.

Reverted in rC357827  because clang-tidy was 
using this function in some of its checks. I'll have a look at if it is also 
affected by the bug or was already using a workaround.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59977



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

r357827 - Revert "[Lexer] NFC: Fix an off-by-one bug in getAsCharRange()."

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Fri Apr  5 15:11:28 2019
New Revision: 357827

URL: http://llvm.org/viewvc/llvm-project?rev=357827=rev
Log:
Revert "[Lexer] NFC: Fix an off-by-one bug in getAsCharRange()."

This reverts commit r357823.

Was breaking clang-tidy!

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

Modified:
cfe/trunk/include/clang/Basic/PlistSupport.h
cfe/trunk/include/clang/Lex/Lexer.h
cfe/trunk/unittests/Lex/LexerTest.cpp

Modified: cfe/trunk/include/clang/Basic/PlistSupport.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/PlistSupport.h?rev=357827=357826=357827=diff
==
--- cfe/trunk/include/clang/Basic/PlistSupport.h (original)
+++ cfe/trunk/include/clang/Basic/PlistSupport.h Fri Apr  5 15:11:28 2019
@@ -127,11 +127,7 @@ inline void EmitRange(raw_ostream , co
   assert(R.isCharRange() && "cannot handle a token range");
   Indent(o, indent) << "\n";
   EmitLocation(o, SM, R.getBegin(), FM, indent + 1);
-
-  // The ".getLocWithOffset(-1)" emulates the behavior of an off-by-one bug
-  // in Lexer that is already fixed. It is here for backwards compatibility
-  // even though it is incorrect.
-  EmitLocation(o, SM, R.getEnd().getLocWithOffset(-1), FM, indent + 1);
+  EmitLocation(o, SM, R.getEnd(), FM, indent + 1);
   Indent(o, indent) << "\n";
 }
 

Modified: cfe/trunk/include/clang/Lex/Lexer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Lexer.h?rev=357827=357826=357827=diff
==
--- cfe/trunk/include/clang/Lex/Lexer.h (original)
+++ cfe/trunk/include/clang/Lex/Lexer.h Fri Apr  5 15:11:28 2019
@@ -382,7 +382,7 @@ public:
 SourceLocation End = getLocForEndOfToken(Range.getEnd(), 0, SM, LangOpts);
 return End.isInvalid() ? CharSourceRange()
: CharSourceRange::getCharRange(
- Range.getBegin(), End);
+ Range.getBegin(), End.getLocWithOffset(-1));
   }
   static CharSourceRange getAsCharRange(CharSourceRange Range,
 const SourceManager ,

Modified: cfe/trunk/unittests/Lex/LexerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/LexerTest.cpp?rev=357827=357826=357827=diff
==
--- cfe/trunk/unittests/Lex/LexerTest.cpp (original)
+++ cfe/trunk/unittests/Lex/LexerTest.cpp Fri Apr  5 15:11:28 2019
@@ -513,23 +513,4 @@ TEST_F(LexerTest, StringizingRasString)
   EXPECT_EQ(String6, R"(a\\\n\n\nb)");
 }
 
-TEST_F(LexerTest, CharRangeOffByOne) {
-  std::vector toks = Lex(R"(#define MOO 1
-void foo() { MOO; })");
-  const Token  = toks[5];
-
-  EXPECT_EQ(getSourceText(moo, moo), "MOO");
-
-  SourceRange R{moo.getLocation(), moo.getLocation()};
-
-  EXPECT_TRUE(
-  Lexer::isAtStartOfMacroExpansion(R.getBegin(), SourceMgr, LangOpts));
-  EXPECT_TRUE(
-  Lexer::isAtEndOfMacroExpansion(R.getEnd(), SourceMgr, LangOpts));
-
-  CharSourceRange CR = Lexer::getAsCharRange(R, SourceMgr, LangOpts);
-
-  EXPECT_EQ(Lexer::getSourceText(CR, SourceMgr, LangOpts), "MOO"); // Was "MO".
-}
-
 } // anonymous namespace


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

[PATCH] D59977: [Lexer] Fix an off-by-one bug in Lexer::getAsCharRange() - NFC.

[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 rL357823: [Lexer] NFC: Fix an off-by-one bug in 
getAsCharRange(). (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59977?vs=192764=193974#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59977

Files:
  cfe/trunk/include/clang/Basic/PlistSupport.h
  cfe/trunk/include/clang/Lex/Lexer.h
  cfe/trunk/unittests/Lex/LexerTest.cpp


Index: cfe/trunk/unittests/Lex/LexerTest.cpp
===
--- cfe/trunk/unittests/Lex/LexerTest.cpp
+++ cfe/trunk/unittests/Lex/LexerTest.cpp
@@ -513,4 +513,23 @@
   EXPECT_EQ(String6, R"(a\\\n\n\nb)");
 }
 
+TEST_F(LexerTest, CharRangeOffByOne) {
+  std::vector toks = Lex(R"(#define MOO 1
+void foo() { MOO; })");
+  const Token  = toks[5];
+
+  EXPECT_EQ(getSourceText(moo, moo), "MOO");
+
+  SourceRange R{moo.getLocation(), moo.getLocation()};
+
+  EXPECT_TRUE(
+  Lexer::isAtStartOfMacroExpansion(R.getBegin(), SourceMgr, LangOpts));
+  EXPECT_TRUE(
+  Lexer::isAtEndOfMacroExpansion(R.getEnd(), SourceMgr, LangOpts));
+
+  CharSourceRange CR = Lexer::getAsCharRange(R, SourceMgr, LangOpts);
+
+  EXPECT_EQ(Lexer::getSourceText(CR, SourceMgr, LangOpts), "MOO"); // Was "MO".
+}
+
 } // anonymous namespace
Index: cfe/trunk/include/clang/Lex/Lexer.h
===
--- cfe/trunk/include/clang/Lex/Lexer.h
+++ cfe/trunk/include/clang/Lex/Lexer.h
@@ -382,7 +382,7 @@
 SourceLocation End = getLocForEndOfToken(Range.getEnd(), 0, SM, LangOpts);
 return End.isInvalid() ? CharSourceRange()
: CharSourceRange::getCharRange(
- Range.getBegin(), End.getLocWithOffset(-1));
+ Range.getBegin(), End);
   }
   static CharSourceRange getAsCharRange(CharSourceRange Range,
 const SourceManager ,
Index: cfe/trunk/include/clang/Basic/PlistSupport.h
===
--- cfe/trunk/include/clang/Basic/PlistSupport.h
+++ cfe/trunk/include/clang/Basic/PlistSupport.h
@@ -127,7 +127,11 @@
   assert(R.isCharRange() && "cannot handle a token range");
   Indent(o, indent) << "\n";
   EmitLocation(o, SM, R.getBegin(), FM, indent + 1);
-  EmitLocation(o, SM, R.getEnd(), FM, indent + 1);
+
+  // The ".getLocWithOffset(-1)" emulates the behavior of an off-by-one bug
+  // in Lexer that is already fixed. It is here for backwards compatibility
+  // even though it is incorrect.
+  EmitLocation(o, SM, R.getEnd().getLocWithOffset(-1), FM, indent + 1);
   Indent(o, indent) << "\n";
 }
 


Index: cfe/trunk/unittests/Lex/LexerTest.cpp
===
--- cfe/trunk/unittests/Lex/LexerTest.cpp
+++ cfe/trunk/unittests/Lex/LexerTest.cpp
@@ -513,4 +513,23 @@
   EXPECT_EQ(String6, R"(a\\\n\n\nb)");
 }
 
+TEST_F(LexerTest, CharRangeOffByOne) {
+  std::vector toks = Lex(R"(#define MOO 1
+void foo() { MOO; })");
+  const Token  = toks[5];
+
+  EXPECT_EQ(getSourceText(moo, moo), "MOO");
+
+  SourceRange R{moo.getLocation(), moo.getLocation()};
+
+  EXPECT_TRUE(
+  Lexer::isAtStartOfMacroExpansion(R.getBegin(), SourceMgr, LangOpts));
+  EXPECT_TRUE(
+  Lexer::isAtEndOfMacroExpansion(R.getEnd(), SourceMgr, LangOpts));
+
+  CharSourceRange CR = Lexer::getAsCharRange(R, SourceMgr, LangOpts);
+
+  EXPECT_EQ(Lexer::getSourceText(CR, SourceMgr, LangOpts), "MOO"); // Was "MO".
+}
+
 } // anonymous namespace
Index: cfe/trunk/include/clang/Lex/Lexer.h
===
--- cfe/trunk/include/clang/Lex/Lexer.h
+++ cfe/trunk/include/clang/Lex/Lexer.h
@@ -382,7 +382,7 @@
 SourceLocation End = getLocForEndOfToken(Range.getEnd(), 0, SM, LangOpts);
 return End.isInvalid() ? CharSourceRange()
: CharSourceRange::getCharRange(
- Range.getBegin(), End.getLocWithOffset(-1));
+ Range.getBegin(), End);
   }
   static CharSourceRange getAsCharRange(CharSourceRange Range,
 const SourceManager ,
Index: cfe/trunk/include/clang/Basic/PlistSupport.h
===
--- cfe/trunk/include/clang/Basic/PlistSupport.h
+++ cfe/trunk/include/clang/Basic/PlistSupport.h
@@ -127,7 +127,11 @@
   assert(R.isCharRange() && "cannot handle a token range");
   Indent(o, indent) << "\n";
   EmitLocation(o, SM, R.getBegin(), FM, indent + 1);
-  EmitLocation(o, SM, R.getEnd(), FM, indent + 1);
+
+  // The 

r357823 - [Lexer] NFC: Fix an off-by-one bug in getAsCharRange().

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Fri Apr  5 14:48:52 2019
New Revision: 357823

URL: http://llvm.org/viewvc/llvm-project?rev=357823=rev
Log:
[Lexer] NFC: Fix an off-by-one bug in getAsCharRange().

As the unit test demonstrates, subtracting 1 from the offset was unnecessary.

The only user of this function was the plist file emitter (in Static Analyzer
and ARCMigrator). It means that a lot of Static Analyzer's plist arrows
are in fact off by one character. The patch carefully preserves this
completely incorrect behavior and causes no functional change,
i.e. no plist format breakage.

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

Modified:
cfe/trunk/include/clang/Basic/PlistSupport.h
cfe/trunk/include/clang/Lex/Lexer.h
cfe/trunk/unittests/Lex/LexerTest.cpp

Modified: cfe/trunk/include/clang/Basic/PlistSupport.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/PlistSupport.h?rev=357823=357822=357823=diff
==
--- cfe/trunk/include/clang/Basic/PlistSupport.h (original)
+++ cfe/trunk/include/clang/Basic/PlistSupport.h Fri Apr  5 14:48:52 2019
@@ -127,7 +127,11 @@ inline void EmitRange(raw_ostream , co
   assert(R.isCharRange() && "cannot handle a token range");
   Indent(o, indent) << "\n";
   EmitLocation(o, SM, R.getBegin(), FM, indent + 1);
-  EmitLocation(o, SM, R.getEnd(), FM, indent + 1);
+
+  // The ".getLocWithOffset(-1)" emulates the behavior of an off-by-one bug
+  // in Lexer that is already fixed. It is here for backwards compatibility
+  // even though it is incorrect.
+  EmitLocation(o, SM, R.getEnd().getLocWithOffset(-1), FM, indent + 1);
   Indent(o, indent) << "\n";
 }
 

Modified: cfe/trunk/include/clang/Lex/Lexer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Lexer.h?rev=357823=357822=357823=diff
==
--- cfe/trunk/include/clang/Lex/Lexer.h (original)
+++ cfe/trunk/include/clang/Lex/Lexer.h Fri Apr  5 14:48:52 2019
@@ -382,7 +382,7 @@ public:
 SourceLocation End = getLocForEndOfToken(Range.getEnd(), 0, SM, LangOpts);
 return End.isInvalid() ? CharSourceRange()
: CharSourceRange::getCharRange(
- Range.getBegin(), End.getLocWithOffset(-1));
+ Range.getBegin(), End);
   }
   static CharSourceRange getAsCharRange(CharSourceRange Range,
 const SourceManager ,

Modified: cfe/trunk/unittests/Lex/LexerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/LexerTest.cpp?rev=357823=357822=357823=diff
==
--- cfe/trunk/unittests/Lex/LexerTest.cpp (original)
+++ cfe/trunk/unittests/Lex/LexerTest.cpp Fri Apr  5 14:48:52 2019
@@ -513,4 +513,23 @@ TEST_F(LexerTest, StringizingRasString)
   EXPECT_EQ(String6, R"(a\\\n\n\nb)");
 }
 
+TEST_F(LexerTest, CharRangeOffByOne) {
+  std::vector toks = Lex(R"(#define MOO 1
+void foo() { MOO; })");
+  const Token  = toks[5];
+
+  EXPECT_EQ(getSourceText(moo, moo), "MOO");
+
+  SourceRange R{moo.getLocation(), moo.getLocation()};
+
+  EXPECT_TRUE(
+  Lexer::isAtStartOfMacroExpansion(R.getBegin(), SourceMgr, LangOpts));
+  EXPECT_TRUE(
+  Lexer::isAtEndOfMacroExpansion(R.getEnd(), SourceMgr, LangOpts));
+
+  CharSourceRange CR = Lexer::getAsCharRange(R, SourceMgr, LangOpts);
+
+  EXPECT_EQ(Lexer::getSourceText(CR, SourceMgr, LangOpts), "MOO"); // Was "MO".
+}
+
 } // anonymous namespace


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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 193968.
hintonda added a comment.

- Use new isa_and_nonnull, and cleanup code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
  clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst
  
clang-tools-extra/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp

Index: clang-tools-extra/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp
@@ -0,0 +1,129 @@
+// RUN: %check_clang_tidy %s llvm-prefer-isa-or-dyn-cast-in-conditionals %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+  X *cast(Y*);
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y, Z *z) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: cast<> in conditional will assert rather than return a null pointer [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: cast<> in conditional
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: cast<> in conditional
+  // CHECK-FIXES: if (isa(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: cast<> in conditional
+  // CHECK-FIXES: while (isa(y))
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: cast<> in conditional
+  // CHECK-FIXES: while (isa(y));
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: return value from dyn_cast<> not used [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+  // CHECK-FIXES: if (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: return value from dyn_cast<> not used
+  // CHECK-FIXES: while (isa(y))
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return value from dyn_cast<> not used
+  // CHECK-FIXES: while (isa(y));
+
+  if (y && isa(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is cleaner and more efficient [llvm-prefer-isa-or-dyn-cast-in-conditionals]
+  // CHECK-FIXES: if (isa_and_nonnull(y))
+
+  if (z->bar() && isa(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  isa_and_nonnull<> is cleaner and more efficient
+  // CHECK-FIXES: if (isa_and_nonnull(z->bar()))
+
+  if (z->bar() && cast(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is cleaner and more efficient
+  // CHECK-FIXES: if (isa_and_nonnull(z->bar()))
+
+  if (z->bar() && dyn_cast(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is cleaner and more efficient
+  // CHECK-FIXES: if (isa_and_nonnull(z->bar()))
+
+  if (z->bar() && dyn_cast_or_null(z->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: isa_and_nonnull<> is cleaner and more efficient
+  // CHECK-FIXES: if (isa_and_nonnull(z->bar()))
+
+  bool b = z->bar() && cast(z->bar());
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: isa_and_nonnull<> is cleaner and more efficient
+  // CHECK-FIXES: bool b = isa_and_nonnull(z->bar());
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (auto x = z->cast(y))
+return true;
+  while (auto x = cast(y)->foo())
+break;
+  if (cast(y)->foo())
+return true;
+  if (z->cast(y))
+return true;
+  while (cast(y)->foo())
+break;
+  if (y && cast(z->bar()))
+return true;
+  if (z && cast(y)->foo())
+return true;
+  bool b2 = y && cast(z);
+  if(z->cast(y))
+return true;
+
+#define CAST(T, Obj) cast(Obj)
+#define AUTO_VAR_CAST(X, Y, Z) auto X = cast(Z)
+#define ISA(T, Obj) isa(Obj)
+#define ISA_OR_NULL(T, Obj) Obj &(Obj)
+
+  // Macros don't trigger warning.
+  if (auto x = CAST(X, y))
+return true;
+  if (AUTO_VAR_CAST(x, X, z))
+return true;
+  if (z->bar() && ISA(Y, z->bar()))
+return true;
+  if (ISA_OR_NULL(Y, z->bar()))
+return true;
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst

[PATCH] D60302: [CodeGen][ObjC] Emit the retainRV marker as a module flag instead of named metadata.

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

I talked with Akira offline and we think this is probably the best approach to 
fix this LTO issue. I will leave others to comment if they think otherwise.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60302



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

[PATCH] D60107: [analyzer] NoStoreFuncVisitor: Suppress bug reports with no-store in system headers.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357810: [analyzer] NoStoreFuncVisitor: Suppress reports with 
no-store in system headers. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60107?vs=193828=193956#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60107

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  cfe/trunk/test/Analysis/Inputs/no-store-suppression.h
  cfe/trunk/test/Analysis/no-store-suppression.cpp

Index: cfe/trunk/test/Analysis/no-store-suppression.cpp
===
--- cfe/trunk/test/Analysis/no-store-suppression.cpp
+++ cfe/trunk/test/Analysis/no-store-suppression.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+// expected-no-diagnostics
+
+#include "Inputs/no-store-suppression.h"
+
+using namespace std;
+
+namespace value_uninitialized_after_stream_shift {
+void use(char c);
+
+// Technically, it is absolutely necessary to check the status of cin after
+// read before using the value that just read from it. Practically, we don't
+// really care unless we eventually come up with a special security check
+// for just that purpose. Static Analyzer shouldn't be yelling at every person's
+// third program in their C++ 101.
+void foo() {
+  char c;
+  std::cin >> c;
+  use(c); // no-warning
+}
+} // namespace value_uninitialized_after_stream_shift
Index: cfe/trunk/test/Analysis/Inputs/no-store-suppression.h
===
--- cfe/trunk/test/Analysis/Inputs/no-store-suppression.h
+++ cfe/trunk/test/Analysis/Inputs/no-store-suppression.h
@@ -0,0 +1,17 @@
+#pragma clang system_header
+
+namespace std {
+class istream {
+public:
+  bool is_eof();
+  char get_char();
+};
+
+istream >>(istream , char ) {
+  if (is.is_eof())
+return;
+  c = is.get_char();
+}
+
+extern istream cin;
+};
Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -306,9 +306,14 @@
 ID.AddPointer(RegionOfInterest);
   }
 
+  void *getTag() const {
+static int Tag = 0;
+return static_cast();
+  }
+
   std::shared_ptr VisitNode(const ExplodedNode *N,
  BugReporterContext ,
- BugReport &) override {
+ BugReport ) override {
 
 const LocationContext *Ctx = N->getLocationContext();
 const StackFrameContext *SCtx = Ctx->getStackFrame();
@@ -322,9 +327,6 @@
 CallEventRef<> Call =
 BR.getStateManager().getCallEventManager().getCaller(SCtx, State);
 
-if (Call->isInSystemHeader())
-  return nullptr;
-
 // Region of interest corresponds to an IVar, exiting a method
 // which could have written into that IVar, but did not.
 if (const auto *MC = dyn_cast(Call)) {
@@ -333,8 +335,8 @@
 if (RegionOfInterest->isSubRegionOf(SelfRegion) &&
 potentiallyWritesIntoIvar(Call->getRuntimeDefinition().getDecl(),
   IvarR->getDecl()))
-  return notModifiedDiagnostics(N, {}, SelfRegion, "self",
-/*FirstIsReferenceType=*/false, 1);
+  return maybeEmitNode(R, *Call, N, {}, SelfRegion, "self",
+   /*FirstIsReferenceType=*/false, 1);
   }
 }
 
@@ -342,8 +344,8 @@
   const MemRegion *ThisR = CCall->getCXXThisVal().getAsRegion();
   if (RegionOfInterest->isSubRegionOf(ThisR)
   && !CCall->getDecl()->isImplicit())
-return notModifiedDiagnostics(N, {}, ThisR, "this",
-  /*FirstIsReferenceType=*/false, 1);
+return maybeEmitNode(R, *Call, N, {}, ThisR, "this",
+ /*FirstIsReferenceType=*/false, 1);
 
   // Do not generate diagnostics for not modified parameters in
   // constructors.
@@ -353,27 +355,26 @@
 ArrayRef parameters = getCallParameters(Call);
 for (unsigned I = 0; I < Call->getNumArgs() && I < parameters.size(); ++I) {
   const ParmVarDecl *PVD = parameters[I];
-  SVal S = Call->getArgSVal(I);
+  SVal V = Call->getArgSVal(I);
   bool ParamIsReferenceType = PVD->getType()->isReferenceType();
   std::string ParamName = PVD->getNameAsString();
 
   int IndirectionLevel = 1;
   QualType T = PVD->getType();
-  while (const MemRegion *R = S.getAsRegion()) {
-if (RegionOfInterest->isSubRegionOf(R) && !isPointerToConst(T))
-  return notModifiedDiagnostics(N, {}, R, 

r357810 - [analyzer] NoStoreFuncVisitor: Suppress reports with no-store in system headers.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Fri Apr  5 13:18:53 2019
New Revision: 357810

URL: http://llvm.org/viewvc/llvm-project?rev=357810=rev
Log:
[analyzer] NoStoreFuncVisitor: Suppress reports with no-store in system headers.

The idea behind this heuristic is that normally the visitor is there to
inform the user that a certain function may fail to initialize a certain
out-parameter. For system header functions this is usually dictated by the
contract, and it's unlikely that the header function has accidentally
forgot to put the value into the out-parameter; it's more likely
that the user has intentionally skipped the error check.

Warnings on skipped error checks are more like security warnings;
they aren't necessarily useful for all users, and they should instead
be introduced on a per-API basis.

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

Added:
cfe/trunk/test/Analysis/Inputs/no-store-suppression.h
cfe/trunk/test/Analysis/no-store-suppression.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=357810=357809=357810=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Fri Apr  5 
13:18:53 2019
@@ -306,9 +306,14 @@ public:
 ID.AddPointer(RegionOfInterest);
   }
 
+  void *getTag() const {
+static int Tag = 0;
+return static_cast();
+  }
+
   std::shared_ptr VisitNode(const ExplodedNode *N,
  BugReporterContext ,
- BugReport &) override {
+ BugReport ) override {
 
 const LocationContext *Ctx = N->getLocationContext();
 const StackFrameContext *SCtx = Ctx->getStackFrame();
@@ -322,9 +327,6 @@ public:
 CallEventRef<> Call =
 BR.getStateManager().getCallEventManager().getCaller(SCtx, State);
 
-if (Call->isInSystemHeader())
-  return nullptr;
-
 // Region of interest corresponds to an IVar, exiting a method
 // which could have written into that IVar, but did not.
 if (const auto *MC = dyn_cast(Call)) {
@@ -333,8 +335,8 @@ public:
 if (RegionOfInterest->isSubRegionOf(SelfRegion) &&
 potentiallyWritesIntoIvar(Call->getRuntimeDefinition().getDecl(),
   IvarR->getDecl()))
-  return notModifiedDiagnostics(N, {}, SelfRegion, "self",
-/*FirstIsReferenceType=*/false, 1);
+  return maybeEmitNode(R, *Call, N, {}, SelfRegion, "self",
+   /*FirstIsReferenceType=*/false, 1);
   }
 }
 
@@ -342,8 +344,8 @@ public:
   const MemRegion *ThisR = CCall->getCXXThisVal().getAsRegion();
   if (RegionOfInterest->isSubRegionOf(ThisR)
   && !CCall->getDecl()->isImplicit())
-return notModifiedDiagnostics(N, {}, ThisR, "this",
-  /*FirstIsReferenceType=*/false, 1);
+return maybeEmitNode(R, *Call, N, {}, ThisR, "this",
+ /*FirstIsReferenceType=*/false, 1);
 
   // Do not generate diagnostics for not modified parameters in
   // constructors.
@@ -353,27 +355,26 @@ public:
 ArrayRef parameters = getCallParameters(Call);
 for (unsigned I = 0; I < Call->getNumArgs() && I < parameters.size(); ++I) 
{
   const ParmVarDecl *PVD = parameters[I];
-  SVal S = Call->getArgSVal(I);
+  SVal V = Call->getArgSVal(I);
   bool ParamIsReferenceType = PVD->getType()->isReferenceType();
   std::string ParamName = PVD->getNameAsString();
 
   int IndirectionLevel = 1;
   QualType T = PVD->getType();
-  while (const MemRegion *R = S.getAsRegion()) {
-if (RegionOfInterest->isSubRegionOf(R) && !isPointerToConst(T))
-  return notModifiedDiagnostics(N, {}, R, ParamName,
-ParamIsReferenceType, 
IndirectionLevel);
+  while (const MemRegion *MR = V.getAsRegion()) {
+if (RegionOfInterest->isSubRegionOf(MR) && !isPointerToConst(T))
+  return maybeEmitNode(R, *Call, N, {}, MR, ParamName,
+   ParamIsReferenceType, IndirectionLevel);
 
 QualType PT = T->getPointeeType();
 if (PT.isNull() || PT->isVoidType()) break;
 
 if (const RecordDecl *RD = PT->getAsRecordDecl())
-  if (auto P = findRegionOfInterestInRecord(RD, State, R))
-return notModifiedDiagnostics(N, *P, RegionOfInterest, ParamName,
-  ParamIsReferenceType,
-  IndirectionLevel);
+  if (auto P = findRegionOfInterestInRecord(RD, State, 

[PATCH] D60335: Use -fomit-frame-pointer when optimizing PowerPC code

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

test coverage?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60335



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

[PATCH] D60335: Use -fomit-frame-pointer when optimizing PowerPC code

[George Koehler via Phabricator via cfe-commits]

kernigh created this revision.
kernigh added reviewers: joerg, brad, cfe-commits.
Herald added subscribers: jsji, krytarowski, nemanjai.
Herald added a project: clang.

This enables -fomit-frame-pointer when optimizing code for all PowerPC
targets, instead of only Linux and NetBSD.

I mailed this patch to cfe-commits earlier this week.
Roman Lebedev and Hal Finkel pointed me to Phabricator;
this is my first attempt to use Phabricator.

My earlier mail wrote:

> The attached patch is for clang to use -fomit-frame-pointer by default
>  for all PowerPC targets when optimizing code.  Right now, clang uses
>  -fomit-frame-pointer for PowerPC Linux and NetBSD but not for other
>  targets.  I have been running `clang -target powerpc-openbsd`.
> 
> The patch is for llvm-project.git master.  I previously posted this
>  patch to https://bugs.llvm.org/show_bug.cgi?id=41094 , but the patch
>  in this email is for a newer revision of master.
> 
> In most functions, the frame pointer in r31 is an unnecessary extra
>  copy of the stack pointer in r1.  GCC is using -fomit-frame-pointer by
>  default (in my PowerPC machine running OpenBSD/macppc); I want Clang
>  to be at least as good as GCC.  Also, this patch helps me to compare
>  the output of `clang -target powerpc-openbsd -O2 -S` with the output
>  for Linux or NetBSD.  In bug 41094, I showed how -fomit-frame-pointer
>  simplifies the C function `void nothing(void) {}`.


Repository:
  rC Clang

https://reviews.llvm.org/D60335

Files:
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -523,8 +523,12 @@
 // XCore never wants frame pointers, regardless of OS.
 // WebAssembly never wants frame pointers.
 return false;
+  case llvm::Triple::ppc:
+  case llvm::Triple::ppc64:
+  case llvm::Triple::ppc64le:
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
+// PowerPC's frame pointer is often an extra copy of the stack pointer.
 return !areOptimizationsEnabled(Args);
   default:
 break;
@@ -542,9 +546,6 @@
 case llvm::Triple::mips64el:
 case llvm::Triple::mips:
 case llvm::Triple::mipsel:
-case llvm::Triple::ppc:
-case llvm::Triple::ppc64:
-case llvm::Triple::ppc64le:
 case llvm::Triple::systemz:
 case llvm::Triple::x86:
 case llvm::Triple::x86_64:


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -523,8 +523,12 @@
 // XCore never wants frame pointers, regardless of OS.
 // WebAssembly never wants frame pointers.
 return false;
+  case llvm::Triple::ppc:
+  case llvm::Triple::ppc64:
+  case llvm::Triple::ppc64le:
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
+// PowerPC's frame pointer is often an extra copy of the stack pointer.
 return !areOptimizationsEnabled(Args);
   default:
 break;
@@ -542,9 +546,6 @@
 case llvm::Triple::mips64el:
 case llvm::Triple::mips:
 case llvm::Triple::mipsel:
-case llvm::Triple::ppc:
-case llvm::Triple::ppc64:
-case llvm::Triple::ppc64le:
 case llvm::Triple::systemz:
 case llvm::Triple::x86:
 case llvm::Triple::x86_64:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r357803 - [clang-fuzzer] Include ExternalProject before using it.

[Matt Morehouse via cfe-commits]

Author: morehouse
Date: Fri Apr  5 12:47:17 2019
New Revision: 357803

URL: http://llvm.org/viewvc/llvm-project?rev=357803=rev
Log:
[clang-fuzzer] Include ExternalProject before using it.

Some versions of CMake require ExternalProject to be included before we
can use ExternalProject_Add.

Modified:
cfe/trunk/cmake/modules/ProtobufMutator.cmake

Modified: cfe/trunk/cmake/modules/ProtobufMutator.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/modules/ProtobufMutator.cmake?rev=357803=357802=357803=diff
==
--- cfe/trunk/cmake/modules/ProtobufMutator.cmake (original)
+++ cfe/trunk/cmake/modules/ProtobufMutator.cmake Fri Apr  5 12:47:17 2019
@@ -1,3 +1,4 @@
+include(ExternalProject)
 set(PBM_PREFIX protobuf_mutator)
 set(PBM_PATH ${CMAKE_CURRENT_BINARY_DIR}/${PBM_PREFIX}/src/${PBM_PREFIX})
 set(PBM_LIB_PATH ${PBM_PATH}-build/src/libprotobuf-mutator.a)


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

[PATCH] D60326: Fix error in NamedDeclPrinterTest

[David Goldman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC357799: Fix error in NamedDeclPrinterTest (authored by 
dgoldman, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60326?vs=193917=193935#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60326

Files:
  unittests/AST/NamedDeclPrinterTest.cpp


Index: unittests/AST/NamedDeclPrinterTest.cpp
===
--- unittests/AST/NamedDeclPrinterTest.cpp
+++ unittests/AST/NamedDeclPrinterTest.cpp
@@ -193,29 +193,33 @@
 }
 
 TEST(NamedDeclPrinter, TestObjCClassExtension) {
+  const char *Code =
+R"(
+  @interface Obj
+  @end
+
+  @interface Obj ()
+  @property(nonatomic) int property;
+  @end
+)";
   ASSERT_TRUE(PrintedWrittenPropertyDeclObjCMatches(
-R"(
-  @interface Obj
-  @end
-
-  @interface Obj ()
-  @property(nonatomic) int property;
-  @end
-)",
+Code,
 "property",
 "Obj::property"));
 }
 
 TEST(NamedDeclPrinter, TestObjCClassExtensionWithGetter) {
+  const char *Code =
+R"(
+  @interface Obj
+  @end
+
+  @interface Obj ()
+  @property(nonatomic, getter=myPropertyGetter) int property;
+  @end
+)";
   ASSERT_TRUE(PrintedWrittenPropertyDeclObjCMatches(
-R"(
-  @interface Obj
-  @end
-
-  @interface Obj ()
-  @property(nonatomic, getter=myPropertyGetter) int property;
-  @end
-)",
+Code,
 "property",
 "Obj::property"));
 }


Index: unittests/AST/NamedDeclPrinterTest.cpp
===
--- unittests/AST/NamedDeclPrinterTest.cpp
+++ unittests/AST/NamedDeclPrinterTest.cpp
@@ -193,29 +193,33 @@
 }
 
 TEST(NamedDeclPrinter, TestObjCClassExtension) {
+  const char *Code =
+R"(
+  @interface Obj
+  @end
+
+  @interface Obj ()
+  @property(nonatomic) int property;
+  @end
+)";
   ASSERT_TRUE(PrintedWrittenPropertyDeclObjCMatches(
-R"(
-  @interface Obj
-  @end
-
-  @interface Obj ()
-  @property(nonatomic) int property;
-  @end
-)",
+Code,
 "property",
 "Obj::property"));
 }
 
 TEST(NamedDeclPrinter, TestObjCClassExtensionWithGetter) {
+  const char *Code =
+R"(
+  @interface Obj
+  @end
+
+  @interface Obj ()
+  @property(nonatomic, getter=myPropertyGetter) int property;
+  @end
+)";
   ASSERT_TRUE(PrintedWrittenPropertyDeclObjCMatches(
-R"(
-  @interface Obj
-  @end
-
-  @interface Obj ()
-  @property(nonatomic, getter=myPropertyGetter) int property;
-  @end
-)",
+Code,
 "property",
 "Obj::property"));
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r357799 - Fix error in NamedDeclPrinterTest

[David Goldman via cfe-commits]

Author: dgoldman
Date: Fri Apr  5 12:17:24 2019
New Revision: 357799

URL: http://llvm.org/viewvc/llvm-project?rev=357799=rev
Log:
Fix error in NamedDeclPrinterTest

Caused by D56924, shouldn't use raw string literals in macros.

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

Modified:
cfe/trunk/unittests/AST/NamedDeclPrinterTest.cpp

Modified: cfe/trunk/unittests/AST/NamedDeclPrinterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/NamedDeclPrinterTest.cpp?rev=357799=357798=357799=diff
==
--- cfe/trunk/unittests/AST/NamedDeclPrinterTest.cpp (original)
+++ cfe/trunk/unittests/AST/NamedDeclPrinterTest.cpp Fri Apr  5 12:17:24 2019
@@ -193,29 +193,33 @@ TEST(NamedDeclPrinter, TestLinkageInName
 }
 
 TEST(NamedDeclPrinter, TestObjCClassExtension) {
-  ASSERT_TRUE(PrintedWrittenPropertyDeclObjCMatches(
-R"(
-  @interface Obj
-  @end
+  const char *Code =
+R"(
+  @interface Obj
+  @end
 
-  @interface Obj ()
-  @property(nonatomic) int property;
-  @end
-)",
+  @interface Obj ()
+  @property(nonatomic) int property;
+  @end
+)";
+  ASSERT_TRUE(PrintedWrittenPropertyDeclObjCMatches(
+Code,
 "property",
 "Obj::property"));
 }
 
 TEST(NamedDeclPrinter, TestObjCClassExtensionWithGetter) {
-  ASSERT_TRUE(PrintedWrittenPropertyDeclObjCMatches(
-R"(
-  @interface Obj
-  @end
+  const char *Code =
+R"(
+  @interface Obj
+  @end
 
-  @interface Obj ()
-  @property(nonatomic, getter=myPropertyGetter) int property;
-  @end
-)",
+  @interface Obj ()
+  @property(nonatomic, getter=myPropertyGetter) int property;
+  @end
+)";
+  ASSERT_TRUE(PrintedWrittenPropertyDeclObjCMatches(
+Code,
 "property",
 "Obj::property"));
 }


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

[PATCH] D60279: [CUDA] Implemented _[bi]mma* builtins.

[Tim Shen via Phabricator via cfe-commits]

timshen added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:12884
+// Helper classes for mapping MMA builtins to particular LLVM intrinsic 
variant.
+class NVPTXMmaLdstInfo {
+public:

How about having a simple struct and a function?
```
struct NvptxMmaLdstInfo {
  unsigned NumResults;
  unsigned IID_col;
  unsigned IID_row;
};

NvptxMmaLdstInfo getNvptxMmaLdstInfo(unsigned BuiltinID) { ... }
```

I don't see the need for classes here.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13020
+
+class NVPTXMmaInfo {
+private:

ditto (struct + function)?


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

https://reviews.llvm.org/D60279



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

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I actually also tend to change doc/ClangFormatStyleOptions.rst too


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

https://reviews.llvm.org/D60320



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

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

just a few nits, apart form that it LG




Comment at: clang/include/clang/Format/Format.h:1721
 
+  /// Defines whether a space should be placed after a logical `!`
+  bool SpaceAfterLogicalNot;

nit: end the sentence with a "."



Comment at: clang/include/clang/Format/Format.h:1924
SpaceBeforeParens == R.SpaceBeforeParens &&
+   SpaceAfterLogicalNot == R.SpaceAfterLogicalNot &&
SpaceBeforeRangeBasedForLoopColon ==

nit: so I recently observed that these are alphabetic, move this up to line 1916



Comment at: clang/lib/Format/Format.cpp:492
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
+IO.mapOptional("SpaceAfterLogicalNot", Style.SpaceAfterLogicalNot);
 IO.mapOptional("SpaceBeforeRangeBasedForLoopColon",

nit: these are alphabetic



Comment at: clang/unittests/Format/FormatTest.cpp:9694
+  verifyFormat("\"Error!\"", Spaces);
+}
+

could you add an example where someone does `!!` would it appear as `! ! (` or 
'!!'



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

https://reviews.llvm.org/D60320



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

[PATCH] D60326: Fix error in NamedDeclPrinterTest

[Roman Lebedev via Phabricator via cfe-commits]

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

LG


Repository:
  rC Clang

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

https://reviews.llvm.org/D60326



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

[PATCH] D60283: [clang-cl] Don't emit checksums when compiling a preprocessed CPP

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In D60283#1456497 , @thakis wrote:

> See 
> http://llvm-cs.pcc.me.uk/tools/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp#756
>  for a "is same file" example. I'm not sure adding a bool to PresumedLoc is 
> worth it for this.


Yeah that works. However I'm getting two conflicting directions - I thought the 
worry was to avoid an extra runtime cost when comparing strings (see first 
version ). @rnk wanted 
at first a compare by FileID, although it looks like it's more costly to do 
that, because all paths end up in `SourceManager::getFileIDSlow`. Just by 
tracing the code on a small preprocessed CPP, it looks like a costly solution. 
Using `IsFromSameFile` ends up in `SourceManager::getFileIDSlow` several times 
per iteration - whereas the solution proposed above (boolean) has zero runtime 
cost. I'm worried that large files with lots of `#line` will be much slower to 
compile. What do you think?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60283



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

[PATCH] D60283: [clang-cl] Don't emit checksums when compiling a preprocessed CPP

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

I'll try profiling several solution on a large unity/jumbo file and get back 
with some results.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60283



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

r357793 - [docs] Fix rst title in clang langext docs. NFCI

[Kristina Brooks via cfe-commits]

Author: kristina
Date: Fri Apr  5 11:26:43 2019
New Revision: 357793

URL: http://llvm.org/viewvc/llvm-project?rev=357793=rev
Log:
[docs] Fix rst title in clang langext docs. NFCI 

Fix an odd line in LanguageExtensions.rst which
rendered incorrectly due to an underscore being
mixed in with dashes.


Modified:
cfe/trunk/docs/LanguageExtensions.rst

Modified: cfe/trunk/docs/LanguageExtensions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=357793=357792=357793=diff
==
--- cfe/trunk/docs/LanguageExtensions.rst (original)
+++ cfe/trunk/docs/LanguageExtensions.rst Fri Apr  5 11:26:43 2019
@@ -1791,7 +1791,7 @@ the arguments. Both arguments and the re
 by the name of the builtin.
 
 ``__builtin_rotateright``
-_
+-
 
 * ``__builtin_rotateright8``
 * ``__builtin_rotateright16``


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

[PATCH] D60326: Fix error in NamedDeclPrinterTest

[David Goldman via Phabricator via cfe-commits]

dgoldman created this revision.
dgoldman added reviewers: sammccall, ilya-biryukov.
Herald added subscribers: cfe-commits, jfb.
Herald added a project: clang.

Caused by D56924 , shouldn't use raw string 
literals in macros.


Repository:
  rC Clang

https://reviews.llvm.org/D60326

Files:
  unittests/AST/NamedDeclPrinterTest.cpp


Index: unittests/AST/NamedDeclPrinterTest.cpp
===
--- unittests/AST/NamedDeclPrinterTest.cpp
+++ unittests/AST/NamedDeclPrinterTest.cpp
@@ -193,29 +193,33 @@
 }
 
 TEST(NamedDeclPrinter, TestObjCClassExtension) {
+  const char *Code =
+R"(
+  @interface Obj
+  @end
+
+  @interface Obj ()
+  @property(nonatomic) int property;
+  @end
+)";
   ASSERT_TRUE(PrintedWrittenPropertyDeclObjCMatches(
-R"(
-  @interface Obj
-  @end
-
-  @interface Obj ()
-  @property(nonatomic) int property;
-  @end
-)",
+Code,
 "property",
 "Obj::property"));
 }
 
 TEST(NamedDeclPrinter, TestObjCClassExtensionWithGetter) {
+  const char *Code =
+R"(
+  @interface Obj
+  @end
+
+  @interface Obj ()
+  @property(nonatomic, getter=myPropertyGetter) int property;
+  @end
+)";
   ASSERT_TRUE(PrintedWrittenPropertyDeclObjCMatches(
-R"(
-  @interface Obj
-  @end
-
-  @interface Obj ()
-  @property(nonatomic, getter=myPropertyGetter) int property;
-  @end
-)",
+Code,
 "property",
 "Obj::property"));
 }


Index: unittests/AST/NamedDeclPrinterTest.cpp
===
--- unittests/AST/NamedDeclPrinterTest.cpp
+++ unittests/AST/NamedDeclPrinterTest.cpp
@@ -193,29 +193,33 @@
 }
 
 TEST(NamedDeclPrinter, TestObjCClassExtension) {
+  const char *Code =
+R"(
+  @interface Obj
+  @end
+
+  @interface Obj ()
+  @property(nonatomic) int property;
+  @end
+)";
   ASSERT_TRUE(PrintedWrittenPropertyDeclObjCMatches(
-R"(
-  @interface Obj
-  @end
-
-  @interface Obj ()
-  @property(nonatomic) int property;
-  @end
-)",
+Code,
 "property",
 "Obj::property"));
 }
 
 TEST(NamedDeclPrinter, TestObjCClassExtensionWithGetter) {
+  const char *Code =
+R"(
+  @interface Obj
+  @end
+
+  @interface Obj ()
+  @property(nonatomic, getter=myPropertyGetter) int property;
+  @end
+)";
   ASSERT_TRUE(PrintedWrittenPropertyDeclObjCMatches(
-R"(
-  @interface Obj
-  @end
-
-  @interface Obj ()
-  @property(nonatomic, getter=myPropertyGetter) int property;
-  @end
-)",
+Code,
 "property",
 "Obj::property"));
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r357792 - [AMDGPU] rename vi-insts into gfx8-insts

[Stanislav Mekhanoshin via cfe-commits]

Author: rampitec
Date: Fri Apr  5 11:25:00 2019
New Revision: 357792

URL: http://llvm.org/viewvc/llvm-project?rev=357792=rev
Log:
[AMDGPU] rename vi-insts into gfx8-insts

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

Modified:
cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
cfe/trunk/test/CodeGenOpenCL/amdgpu-features.cl
cfe/trunk/test/SemaOpenCL/builtins-amdgcn-error-vi.cl

Modified: cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def?rev=357792=357791=357792=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def Fri Apr  5 11:25:00 2019
@@ -133,7 +133,7 @@ TARGET_BUILTIN(__builtin_amdgcn_classh,
 TARGET_BUILTIN(__builtin_amdgcn_s_memrealtime, "LUi", "n", "s-memrealtime")
 TARGET_BUILTIN(__builtin_amdgcn_mov_dpp, "iiIiIiIiIb", "nc", "dpp")
 TARGET_BUILTIN(__builtin_amdgcn_update_dpp, "iiiIiIiIiIb", "nc", "dpp")
-TARGET_BUILTIN(__builtin_amdgcn_s_dcache_wb, "v", "n", "vi-insts")
+TARGET_BUILTIN(__builtin_amdgcn_s_dcache_wb, "v", "n", "gfx8-insts")
 
 
//===--===//
 // GFX9+ only builtins.

Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AMDGPU.cpp?rev=357792=357791=357792=diff
==
--- cfe/trunk/lib/Basic/Targets/AMDGPU.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.cpp Fri Apr  5 11:25:00 2019
@@ -150,7 +150,7 @@ bool AMDGPUTargetInfo::initFeatureMap(
 case GK_GFX803:
 case GK_GFX802:
 case GK_GFX801:
-  Features["vi-insts"] = true;
+  Features["gfx8-insts"] = true;
   Features["16-bit-insts"] = true;
   Features["dpp"] = true;
   Features["s-memrealtime"] = true;

Modified: cfe/trunk/test/CodeGenOpenCL/amdgpu-features.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/amdgpu-features.cl?rev=357792=357791=357792=diff
==
--- cfe/trunk/test/CodeGenOpenCL/amdgpu-features.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/amdgpu-features.cl Fri Apr  5 11:25:00 2019
@@ -10,9 +10,9 @@
 // RUN: %clang_cc1 -triple amdgcn -target-cpu gfx600 -S -emit-llvm -o - %s | 
FileCheck --check-prefix=GFX600 %s
 // RUN: %clang_cc1 -triple amdgcn -target-cpu gfx601 -S -emit-llvm -o - %s | 
FileCheck --check-prefix=GFX601 %s
 
-// GFX904: 
"target-features"="+16-bit-insts,+ci-insts,+dpp,+fp32-denormals,+fp64-fp16-denormals,+gfx9-insts,+s-memrealtime,+vi-insts"
-// GFX906: 
"target-features"="+16-bit-insts,+ci-insts,+dl-insts,+dot1-insts,+dot2-insts,+dpp,+fp32-denormals,+fp64-fp16-denormals,+gfx9-insts,+s-memrealtime,+vi-insts"
-// GFX801: 
"target-features"="+16-bit-insts,+ci-insts,+dpp,+fp32-denormals,+fp64-fp16-denormals,+s-memrealtime,+vi-insts"
+// GFX904: 
"target-features"="+16-bit-insts,+ci-insts,+dpp,+fp32-denormals,+fp64-fp16-denormals,+gfx8-insts,+gfx9-insts,+s-memrealtime"
+// GFX906: 
"target-features"="+16-bit-insts,+ci-insts,+dl-insts,+dot1-insts,+dot2-insts,+dpp,+fp32-denormals,+fp64-fp16-denormals,+gfx8-insts,+gfx9-insts,+s-memrealtime"
+// GFX801: 
"target-features"="+16-bit-insts,+ci-insts,+dpp,+fp32-denormals,+fp64-fp16-denormals,+gfx8-insts,+s-memrealtime"
 // GFX700: "target-features"="+ci-insts,+fp64-fp16-denormals,-fp32-denormals"
 // GFX600: "target-features"="+fp64-fp16-denormals,-fp32-denormals"
 // GFX601: "target-features"="+fp64-fp16-denormals,-fp32-denormals"

Modified: cfe/trunk/test/SemaOpenCL/builtins-amdgcn-error-vi.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/builtins-amdgcn-error-vi.cl?rev=357792=357791=357792=diff
==
--- cfe/trunk/test/SemaOpenCL/builtins-amdgcn-error-vi.cl (original)
+++ cfe/trunk/test/SemaOpenCL/builtins-amdgcn-error-vi.cl Fri Apr  5 11:25:00 
2019
@@ -4,5 +4,5 @@
 
 void test_vi_s_dcache_wb()
 {
-  __builtin_amdgcn_s_dcache_wb(); // expected-error 
{{'__builtin_amdgcn_s_dcache_wb' needs target feature vi-insts}}
+  __builtin_amdgcn_s_dcache_wb(); // expected-error 
{{'__builtin_amdgcn_s_dcache_wb' needs target feature gfx8-insts}}
 }


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

[PATCH] D60283: [clang-cl] Don't emit checksums when compiling a preprocessed CPP

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

See 
http://llvm-cs.pcc.me.uk/tools/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp#756
 for a "is same file" example. I'm not sure adding a bool to PresumedLoc is 
worth it for this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60283



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

[PATCH] D60220: [CUDA][Windows] Final fix for bug 38811 (Step 3 of 3)

[Evgeny Mankov via Phabricator via cfe-commits]

emankov added a comment.

In D60220#1456449 , @tra wrote:

> It's not a big deal at the moment -- there are no `long double` users in CUDA 
> on linux yet. You can clean up in another commit.
>  BTW, you may want to make commit description somewhat more concise than 
> rC357779 . Including all details of 
> reproduction, etc is way too much info which is better suited for the bug 
> report or review.


Ok, thanks, will do it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60220



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

[PATCH] D60220: [CUDA][Windows] Final fix for bug 38811 (Step 3 of 3)

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D60220#1456430 , @emankov wrote:

> Oooh, sorry, but I've just pushed the fix. But with the following words: "Add 
> missing long double device functions' declarations. Provide only declarations 
> to prevent any use of long double on the device side, because CUDA does not 
> support long double on the device side."


It's not a big deal at the moment -- there are no `long double` users in CUDA 
on linux yet. You can clean up in another commit.
BTW, you may want to make commit description somewhat more concise than 
rC357779 . Including all details of 
reproduction, etc is way too much info which is better suited for the bug 
report or review.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60220



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

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL added a comment.

In D60139#1456123 , @alexfh wrote:

> Looks like this check would fit better into the `bugprone` module.


Excellent suggestion. Thanks. Will follow up with an updated Diff.


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

https://reviews.llvm.org/D60139



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

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 193906.
ymandel added a comment.

Adjust to use SourceCode instead of FixIt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59371

Files:
  clang/include/clang/Tooling/Refactoring/Stencil.h
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/Stencil.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -0,0 +1,204 @@
+//===- unittest/Tooling/StencilTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/Stencil.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/FixIt.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
+namespace {
+using ::testing::AllOf;
+using ::testing::Eq;
+using ::testing::HasSubstr;
+using MatchResult = MatchFinder::MatchResult;
+using tooling::stencil::node;
+using tooling::stencil::snode;
+using tooling::stencil::text;
+
+// In tests, we can't directly match on llvm::Expected since its accessors
+// mutate the object. So, we collapse it to an Optional.
+static llvm::Optional toOptional(llvm::Expected V) {
+  if (V)
+return *V;
+  ADD_FAILURE() << "Losing error in conversion to IsSomething: "
+<< llvm::toString(V.takeError());
+  return llvm::None;
+}
+
+// A very simple matcher for llvm::Optional values.
+MATCHER_P(IsSomething, ValueMatcher, "") {
+  if (!arg)
+return false;
+  return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener);
+}
+
+// Create a valid translation-unit from a statement.
+static std::string wrapSnippet(llvm::Twine StatementCode) {
+  return ("auto stencil_test_snippet = []{" + StatementCode + "};").str();
+}
+
+static DeclarationMatcher wrapMatcher(const StatementMatcher ) {
+  return varDecl(hasName("stencil_test_snippet"),
+ hasDescendant(compoundStmt(hasAnySubstatement(Matcher;
+}
+
+struct TestMatch {
+  // The AST unit from which `result` is built. We bundle it because it backs
+  // the result. Users are not expected to access it.
+  std::unique_ptr AstUnit;
+  // The result to use in the test. References `ast_unit`.
+  MatchResult Result;
+};
+
+// Matches `Matcher` against the statement `StatementCode` and returns the
+// result. Handles putting the statement inside a function and modifying the
+// matcher correspondingly. `Matcher` should match `StatementCode` exactly --
+// that is, produce exactly one match.
+static llvm::Optional matchStmt(llvm::Twine StatementCode,
+   StatementMatcher Matcher) {
+  auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode));
+  if (AstUnit == nullptr) {
+ADD_FAILURE() << "AST construction failed";
+return llvm::None;
+  }
+  ASTContext  = AstUnit->getASTContext();
+  auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context);
+  // We expect a single, exact match for the statement.
+  if (Matches.size() != 1) {
+ADD_FAILURE() << "Wrong number of matches: " << Matches.size();
+return llvm::None;
+  }
+  return TestMatch{std::move(AstUnit), MatchResult(Matches[0], )};
+}
+
+class StencilTest : public ::testing::Test {
+protected:
+  // Verifies that the given stencil fails when evaluated on a valid match
+  // result. Binds a statement to "stmt", a (non-member) ctor-initializer to
+  // "init", an expression to "expr" and a (nameless) declaration to "decl".
+  void testError(const Stencil ,
+ ::testing::Matcher Matcher) {
+const std::string Snippet = R"cc(
+  struct A {};
+  class F : public A {
+   public:
+F(int) {}
+  };
+  F(1);
+)cc";
+auto StmtMatch = matchStmt(
+Snippet,
+stmt(hasDescendant(
+ cxxConstructExpr(
+ hasDeclaration(decl(hasDescendant(cxxCtorInitializer(
+   isBaseInitializer())
+   .bind("init")))
+.bind("decl")))
+ .bind("expr")))
+.bind("stmt"));
+ASSERT_TRUE(StmtMatch);
+if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) {
+  ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr;
+} else {
+  auto Err = 

[PATCH] D60220: [CUDA][Windows] Final fix for bug 38811 (Step 3 of 3)

[Evgeny Mankov via Phabricator via cfe-commits]

emankov added a comment.

Oooh, sorry, but I've just pushed the fix. But with the following words: "Add 
missing long double device functions' declarations. Provide only declarations 
to prevent any use of long double on the device side, because CUDA does not 
support long double on the device side."


Repository:
  rC Clang

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

https://reviews.llvm.org/D60220



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

[PATCH] D60220: [CUDA][Windows] Final fix for bug 38811 (Step 3 of 3)

[Evgeny Mankov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC357779: [CUDA][Windows] Last fix for the clang Bug 38811 
Clang fails to compile with… (authored by emankov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60220?vs=193874=193904#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60220

Files:
  lib/Headers/__clang_cuda_cmath.h
  lib/Headers/__clang_cuda_device_functions.h
  lib/Headers/__clang_cuda_math_forward_declares.h


Index: lib/Headers/__clang_cuda_cmath.h
===
--- lib/Headers/__clang_cuda_cmath.h
+++ lib/Headers/__clang_cuda_cmath.h
@@ -78,13 +78,16 @@
 #ifndef _MSC_VER
 __DEVICE__ bool isinf(float __x) { return ::__isinff(__x); }
 __DEVICE__ bool isinf(double __x) { return ::__isinf(__x); }
+__DEVICE__ bool isinf(long double __x) { return ::__isinfl(__x); }
 __DEVICE__ bool isfinite(float __x) { return ::__finitef(__x); }
 // For inscrutable reasons, __finite(), the double-precision version of
 // __finitef, does not exist when compiling for MacOS.  __isfinited is 
available
 // everywhere and is just as good.
 __DEVICE__ bool isfinite(double __x) { return ::__isfinited(__x); }
+__DEVICE__ bool isfinite(long double __x) { return ::__finitel(__x); }
 __DEVICE__ bool isnan(float __x) { return ::__isnanf(__x); }
 __DEVICE__ bool isnan(double __x) { return ::__isnan(__x); }
+__DEVICE__ bool isnan(long double __x) { return ::__isnanl(__x); }
 #endif
 
 __DEVICE__ bool isgreater(float __x, float __y) {
Index: lib/Headers/__clang_cuda_device_functions.h
===
--- lib/Headers/__clang_cuda_device_functions.h
+++ lib/Headers/__clang_cuda_device_functions.h
@@ -237,6 +237,7 @@
 __DEVICE__ int __ffsll(long long __a) { return __nv_ffsll(__a); }
 __DEVICE__ int __finite(double __a) { return __nv_isfinited(__a); }
 __DEVICE__ int __finitef(float __a) { return __nv_finitef(__a); }
+__DEVICE__ int __finitel(long double __a);
 __DEVICE__ int __float2int_rd(float __a) { return __nv_float2int_rd(__a); }
 __DEVICE__ int __float2int_rn(float __a) { return __nv_float2int_rn(__a); }
 __DEVICE__ int __float2int_ru(float __a) { return __nv_float2int_ru(__a); }
@@ -445,8 +446,10 @@
 __DEVICE__ int __isfinited(double __a) { return __nv_isfinited(__a); }
 __DEVICE__ int __isinf(double __a) { return __nv_isinfd(__a); }
 __DEVICE__ int __isinff(float __a) { return __nv_isinff(__a); }
+__DEVICE__ int __isinfl(long double __a);
 __DEVICE__ int __isnan(double __a) { return __nv_isnand(__a); }
 __DEVICE__ int __isnanf(float __a) { return __nv_isnanf(__a); }
+__DEVICE__ int __isnanl(long double __a);
 __DEVICE__ double __ll2double_rd(long long __a) {
   return __nv_ll2double_rd(__a);
 }
Index: lib/Headers/__clang_cuda_math_forward_declares.h
===
--- lib/Headers/__clang_cuda_math_forward_declares.h
+++ lib/Headers/__clang_cuda_math_forward_declares.h
@@ -98,12 +98,14 @@
 __DEVICE__ float hypot(float, float);
 __DEVICE__ int ilogb(double);
 __DEVICE__ int ilogb(float);
+__DEVICE__ bool isfinite(long double);
 __DEVICE__ bool isfinite(double);
 __DEVICE__ bool isfinite(float);
 __DEVICE__ bool isgreater(double, double);
 __DEVICE__ bool isgreaterequal(double, double);
 __DEVICE__ bool isgreaterequal(float, float);
 __DEVICE__ bool isgreater(float, float);
+__DEVICE__ bool isinf(long double);
 __DEVICE__ bool isinf(double);
 __DEVICE__ bool isinf(float);
 __DEVICE__ bool isless(double, double);
@@ -112,6 +114,7 @@
 __DEVICE__ bool isless(float, float);
 __DEVICE__ bool islessgreater(double, double);
 __DEVICE__ bool islessgreater(float, float);
+__DEVICE__ bool isnan(long double);
 __DEVICE__ bool isnan(double);
 __DEVICE__ bool isnan(float);
 __DEVICE__ bool isnormal(double);


Index: lib/Headers/__clang_cuda_cmath.h
===
--- lib/Headers/__clang_cuda_cmath.h
+++ lib/Headers/__clang_cuda_cmath.h
@@ -78,13 +78,16 @@
 #ifndef _MSC_VER
 __DEVICE__ bool isinf(float __x) { return ::__isinff(__x); }
 __DEVICE__ bool isinf(double __x) { return ::__isinf(__x); }
+__DEVICE__ bool isinf(long double __x) { return ::__isinfl(__x); }
 __DEVICE__ bool isfinite(float __x) { return ::__finitef(__x); }
 // For inscrutable reasons, __finite(), the double-precision version of
 // __finitef, does not exist when compiling for MacOS.  __isfinited is available
 // everywhere and is just as good.
 __DEVICE__ bool isfinite(double __x) { return ::__isfinited(__x); }
+__DEVICE__ bool isfinite(long double __x) { return ::__finitel(__x); }
 __DEVICE__ bool isnan(float __x) { return ::__isnanf(__x); }
 __DEVICE__ bool isnan(double __x) { return ::__isnan(__x); }
+__DEVICE__ bool isnan(long double __x) { return ::__isnanl(__x); }
 #endif
 
 __DEVICE__ bool isgreater(float __x, float 

r357779 - [CUDA][Windows] Last fix for the clang Bug 38811 "Clang fails to compile with CUDA-9.x on Windows" (https://bugs.llvm.org/show_bug.cgi?id=38811).

[Evgeny Mankov via cfe-commits]

Author: emankov
Date: Fri Apr  5 09:51:10 2019
New Revision: 357779

URL: http://llvm.org/viewvc/llvm-project?rev=357779=rev
Log:
[CUDA][Windows] Last fix for the clang Bug 38811 "Clang fails to compile with 
CUDA-9.x on Windows" (https://bugs.llvm.org/show_bug.cgi?id=38811).

[IMPORTANT]
With that last fix, CUDA has just started being compiling by clang on Windows 
after nearly a year and two clang’s major releases (7 and 8).
As long as the last LLVM release, in which clang was compiling CUDA on Windows 
successfully, was 6.0.1, this fix and two previous have to be included into 
upcoming 7.1.0 and 8.0.1 releases.

[How to repro]
clang++.exe -x cuda "c:\ProgramData\NVIDIA Corporation\CUDA 
Samples\v9.0\0_Simple\simplePrintf\simplePrintf.cu" -I"c:\ProgramData\NVIDIA 
Corporation\CUDA Samples\v9.0\common\inc" --cuda-gpu-arch=sm_50 
--cuda-path="C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v9.0" 
-L"c:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v9.0\lib\x64" 
-lcudart.lib  -v

[Output]
In file included from 
C:\GIT\LLVM\trunk-for-submits\llvm-64-release-vs2017-15.9.9\dist\lib\clang\9.0.0\include\__clang_cuda_runtime_wrapper.h:327:
C:\Program Files\NVIDIA GPU Computing 
Toolkit\CUDA\v9.0/include\crt/math_functions.hpp:390:11: error: no matching 
function for call to '__isinfl'
  return (__isinfl(a) != 0);
  ^~~~
C:\Program Files\NVIDIA GPU Computing 
Toolkit\CUDA\v9.0/include\crt/math_functions.hpp:2662:14: note: candidate 
function not viable: call to __host__ function from __device__ function
__func__(int __isinfl(long double a))
 ^
In file included from :1:
In file included from 
C:\GIT\LLVM\trunk-for-submits\llvm-64-release-vs2017-15.9.9\dist\lib\clang\9.0.0\include\__clang_cuda_runtime_wrapper.h:327:
C:\Program Files\NVIDIA GPU Computing 
Toolkit\CUDA\v9.0/include\crt/math_functions.hpp:438:11: error: no matching 
function for call to '__isnanl'
  return (__isnanl(a) != 0);
  ^~~~
C:\Program Files\NVIDIA GPU Computing 
Toolkit\CUDA\v9.0/include\crt/math_functions.hpp:2672:14: note: candidate 
function not viable: call to __host__ function from __device__ function
__func__(int __isnanl(long double a))
 ^
In file included from :1:
In file included from 
C:\GIT\LLVM\trunk-for-submits\llvm-64-release-vs2017-15.9.9\dist\lib\clang\9.0.0\include\__clang_cuda_runtime_wrapper.h:327:
C:\Program Files\NVIDIA GPU Computing 
Toolkit\CUDA\v9.0/include\crt/math_functions.hpp:486:11: error: no matching 
function for call to '__finitel'
  return (__finitel(a) != 0);
  ^
C:\Program Files\NVIDIA GPU Computing 
Toolkit\CUDA\v9.0/include\crt/math_functions.hpp:2652:14: note: candidate 
function not viable: call to __host__ function from __device__ function
__func__(int __finitel(long double a))
 ^
3 errors generated when compiling for sm_50.

[Solution]
Add missing long double device functions' declarations. Provide only 
declarations to prevent any use of long double on the device side, because CUDA 
does not support long double on the device side.

[Testing]
{Windows 10, Ubuntu 16.04.5}/{Visual C++ 2017 15.9.9, gcc+ 5.4.0}/CUDA {8.0, 
9.0, 9.1, 9.2, 10.0, 10.1}

Reviewed by: Artem Belevich

Differential Revision: http://reviews.llvm.org/D60220

Modified:
cfe/trunk/lib/Headers/__clang_cuda_cmath.h
cfe/trunk/lib/Headers/__clang_cuda_device_functions.h
cfe/trunk/lib/Headers/__clang_cuda_math_forward_declares.h

Modified: cfe/trunk/lib/Headers/__clang_cuda_cmath.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/__clang_cuda_cmath.h?rev=357779=357778=357779=diff
==
--- cfe/trunk/lib/Headers/__clang_cuda_cmath.h (original)
+++ cfe/trunk/lib/Headers/__clang_cuda_cmath.h Fri Apr  5 09:51:10 2019
@@ -78,13 +78,16 @@ __DEVICE__ float frexp(float __arg, int
 #ifndef _MSC_VER
 __DEVICE__ bool isinf(float __x) { return ::__isinff(__x); }
 __DEVICE__ bool isinf(double __x) { return ::__isinf(__x); }
+__DEVICE__ bool isinf(long double __x) { return ::__isinfl(__x); }
 __DEVICE__ bool isfinite(float __x) { return ::__finitef(__x); }
 // For inscrutable reasons, __finite(), the double-precision version of
 // __finitef, does not exist when compiling for MacOS.  __isfinited is 
available
 // everywhere and is just as good.
 __DEVICE__ bool isfinite(double __x) { return ::__isfinited(__x); }
+__DEVICE__ bool isfinite(long double __x) { return ::__finitel(__x); }
 __DEVICE__ bool isnan(float __x) { return ::__isnanf(__x); }
 __DEVICE__ bool isnan(double __x) { return ::__isnan(__x); }
+__DEVICE__ bool isnan(long double __x) { return ::__isnanl(__x); }
 #endif
 
 __DEVICE__ bool isgreater(float __x, float __y) {

Modified: cfe/trunk/lib/Headers/__clang_cuda_device_functions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/__clang_cuda_device_functions.h?rev=357779=357778=357779=diff

[PATCH] D59756: [clangd] Support dependent bases in type hierarchy

[Sam McCall via Phabricator via cfe-commits]

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

LG, though I think your patch might be based on top of some uncommitted changes.

I noticed template names rather than type names are shown (not new in this 
patch) - we should fix that I think.




Comment at: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp:384
 
 TEST(TypeHierarchy, RecursiveHierarchy1) {
   Annotations Source(R"cpp(

can we give this a more descriptive name like RecursiveHierarchyUnbounded?



Comment at: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp:405
+  *Result,
+  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),

Sorry, I realize this isn't related to this patch, but I didn't see the final 
form of the previous one.

This should be `WithName("S<0>"), ... Parents(AllOf(WithName("S<1>")), ...)`. S 
is the name of the template, not the name of the type.

Can you add a fixme?



Comment at: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp:410
 
 TEST(TypeHierarchy, RecursiveHierarchy2) {
   Annotations Source(R"cpp(

RecursiveHierarchyBounded



Comment at: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp:447
   struct Foo {
 S^ s;
   };

this can be merged with the previous test case, using two named points.



Comment at: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp:463
+  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+  SelectionRangeIs(Source.range("SDef")), 
Parents();

(in this case the printed name for the parent might end up looking odd - 
printing the spelled `S` is probably ideal but I don't really mind what 
we end up with, this is an edge case. Not for this patch, in any case)



Comment at: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp:609
 
-// Disabled for the same reason as TypeParents.DependentBase.
-TEST(DISABLED_Subtypes, DependentBase) {
+TEST(Subtypes, DependentBase) {
   Annotations Source(R"cpp(

this patch doesn't seem to be against HEAD


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59756



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

[PATCH] D60220: [CUDA][Windows] Final fix for bug 38811 (Step 3 of 3)

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

One more thing -- perhaps the `long double` declarations should be put under 
`#ifndef _MSC_VER` in all the files to make the change unobservable on 
non-windows platforms.

Adding a comment why we only have declarations for these functions would also 
be helpful.


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

https://reviews.llvm.org/D60220



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

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 193903.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.
Herald added a subscriber: mgrang.

Changes:

- Add multi-file support, record a single expanded stream and per-file-id raw 
token streams and mappings.
- Rename MacroInvocation to TokenBuffer::Mapping, make it private.
- Simplify TokenCollector, let preprocessor handle some more stuff.

TODO:

- update the docs
- go through other comments again
- write more tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887

Files:
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/TokensTest.cpp

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -0,0 +1,602 @@
+//===- TokensTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Syntax/Tokens.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Expr.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.def"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Lex/Token.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_os_ostream.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "gmock/gmock-more-matchers.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace clang::syntax;
+
+using ::testing::AllOf;
+using ::testing::Contains;
+using ::testing::ElementsAre;
+using ::testing::Matcher;
+using ::testing::Pointwise;
+
+namespace {
+// Matchers for syntax::Token.
+MATCHER_P(Kind, K, "") { return arg.kind() == K; }
+MATCHER_P2(HasText, Text, SourceMgr, "") {
+  return arg.text(*SourceMgr) == Text;
+}
+MATCHER_P2(IsIdent, Text, SourceMgr, "") {
+  return arg.kind() == tok::identifier && arg.text(*SourceMgr) == Text;
+}
+/// Checks the start and end location of a token are equal to SourceRng.
+MATCHER_P(RangeIs, SourceRng, "") {
+  return arg.location() == SourceRng.first &&
+ arg.endLocation() == SourceRng.second;
+}
+/// Checks the passed tuple has two similar tokens, i.e. both are of the same
+/// kind and have the same text if they are identifiers.
+/// Ignores differences in kind between the raw and non-raw mode.
+MATCHER_P(IsSameToken, SourceMgr, "") {
+  auto ToEquivalenceClass = [](tok::TokenKind Kind) {
+if (Kind == tok::identifier || Kind == tok::raw_identifier ||
+tok::getKeywordSpelling(Kind) != nullptr)
+  return tok::identifier;
+if (Kind == tok::string_literal || Kind == tok::header_name)
+  return tok::string_literal;
+return Kind;
+  };
+
+  auto  = std::get<0>(arg);
+  auto  = std::get<1>(arg);
+  if (ToEquivalenceClass(L.kind()) != ToEquivalenceClass(R.kind()))
+return false;
+  return L.text(*SourceMgr) == L.text(*SourceMgr);
+}
+} // namespace
+
+// Actual test fixture lives in the syntax namespace as it's a friend of
+// TokenBuffer.
+class syntax::TokensTest : public ::testing::Test {
+public:
+  /// Run the clang frontend, collect the preprocessed tokens from the frontend
+  /// invocation and store them in this->Buffer.
+  /// This also clears SourceManager before running the compiler.
+  void recordTokens(llvm::StringRef Code) {
+class RecordTokens : public ASTFrontendAction {
+public:
+  explicit RecordTokens(TokenBuffer ) : Result(Result) {}
+
+  bool BeginSourceFileAction(CompilerInstance ) override {
+

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

[Ilya Biryukov via Phabricator via cfe-commits]

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

Pass an enum indicating where the token comes from.
The enum is ad-hoc at the moment, will require some thought to turn
it into a reasonable abstraction.

The consumer of the token stream actually needs to be able to distinguish
tokens which are part of the final expanded  token stream vs those that
aren't (macro directives, intermediate macro argument expansions, etc)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59885

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Lex/TokenLexer.h
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Lex/Preprocessor.cpp

Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -864,20 +864,32 @@
 void Preprocessor::Lex(Token ) {
   // We loop here until a lex function returns a token; this avoids recursion.
   bool ReturnedToken;
+  TokenSource Source;
   do {
 switch (CurLexerKind) {
 case CLK_Lexer:
+  if (CurLexer->ParsingPreprocessorDirective)
+Source = TokenSource::MacroDirective;
+  else if (DisableMacroExpansion)
+Source = TokenSource::MacroNameOrArg;
+  else
+Source = TokenSource::File;
+
   ReturnedToken = CurLexer->Lex(Result);
   break;
 case CLK_TokenLexer:
+  Source = CurTokenLexer->isMacroExpansion() ? TokenSource::MacroExpansion
+ : TokenSource::Precached;
   ReturnedToken = CurTokenLexer->Lex(Result);
   break;
 case CLK_CachingLexer:
   CachingLex(Result);
+  Source = TokenSource::Precached;
   ReturnedToken = true;
   break;
 case CLK_LexAfterModuleImport:
   LexAfterModuleImport(Result);
+  Source = TokenSource::AfterModuleImport;
   ReturnedToken = true;
   break;
 }
@@ -893,6 +905,8 @@
   }
 
   LastTokenWasAt = Result.is(tok::at);
+  if (OnToken)
+OnToken(Result, Source);
 }
 
 /// Lex a header-name token (including one formed from header-name-tokens if
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -463,6 +463,10 @@
  const MacroDefinition ) {
   MacroInfo *MI = M.getMacroInfo();
 
+  // The macro-expanded identifiers are not seen by the Lex() method.
+  if (OnToken)
+OnToken(Identifier, TokenSource::MacroNameOrArg);
+
   // If this is a macro expansion in the "#if !defined(x)" line for the file,
   // then the macro could expand to different things in other contexts, we need
   // to disable the optimization in this case.
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -400,6 +400,9 @@
   setCodeCompletionReached();
   continue;
 }
+// This token is not reported to
+if (OnToken)
+  OnToken(Tok, TokenSource::SkippedPPBranch);
 
 // If this is the end of the buffer, we have an error.
 if (Tok.is(tok::eof)) {
@@ -865,6 +868,10 @@
   // Save the '#' token in case we need to return it later.
   Token SavedHash = Result;
 
+  // Lex() never sees the '#' token from directives, so report it here.
+  if (OnToken)
+OnToken(Result, TokenSource::MacroDirective);
+
   // Read the next token, the directive flavor.  This isn't expanded due to
   // C99 6.10.3p8.
   LexUnexpandedToken(Result);
Index: clang/include/clang/Lex/TokenLexer.h
===
--- clang/include/clang/Lex/TokenLexer.h
+++ clang/include/clang/Lex/TokenLexer.h
@@ -147,6 +147,10 @@
   /// preprocessor directive.
   bool isParsingPreprocessorDirective() const;
 
+  /// Returns true iff the TokenLexer is expanding a macro and not replaying a
+  /// stream of tokens.
+  bool isMacroExpansion() const { return Macro != nullptr; }
+
 private:
   void destroy();
 
Index: clang/include/clang/Lex/Preprocessor.h
===
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -33,6 +33,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/FoldingSet.h"
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerUnion.h"
@@ -48,8 +49,8 @@
 #include 
 #include 
 #include 
-#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -114,6 +115,21 @@
   MU_Undef  = 2
 };
 
+/// Captures some information about where the tokens come from. Used by the
+/// callback that records tokens.
+enum class 

[PATCH] D49863: [istream] Fix error flags and exceptions propagated from input stream operations

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D49863#1452154 , @ldionne wrote:

> In D49863#1452126 , @aprantl wrote:
>
> > It looks like this may have broken some LLDB data formatters:
> >  http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/22934/
> >
> > http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/22934/testReport/junit/lldb-Suite/functionalities_data-formatter_data-formatter-stl_libcxx_optional/TestDataFormatterLibcxxOptional_py/
> >  
> > http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/22934/testReport/junit/lldb-Suite/functionalities_data-formatter_data-formatter-stl_libcxx_unordered/TestDataFormatterUnordered_py/
>
>
> I reverted in r357536 to give some time to figure what the problem is.


It turns out I think only r357531 had been applied and this is what broke the 
LLDB CI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49863



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

[PATCH] D60220: [CUDA][Windows] Final fix for bug 38811 (Step 3 of 3)

[Artem Belevich via Phabricator via cfe-commits]

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

Thank you for fixing this!


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

https://reviews.llvm.org/D60220



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

[PATCH] D59407: [clangd] Add RelationSlab

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D59407#1447070 , @nridge wrote:

> @sammccall, thank you for having a look at this.
>
> I have no objection to revising the data model if there's agreement on a 
> better one.
>
> In D59407#1446464 , @sammccall wrote:
>
> > - I don't think we yet know what the more resource-critical (denser) 
> > relations and queries are, so it's unclear what to optimize for
>
>
> Based on a brief mental survey of C++ IDE features I'm familiar with, I can 
> think of the following additional uses of the relations capability:
>
> - A call hierarchy feature (which is also proposed for LSP 
> , with 
> client  and server 
>  
> implementation efforts) would need every caller-callee relationship to be 
> recorded in the index (`RelationCalledBy`).
> - Given a virtual method declaration, a user may want to see a list of 
> implementations (overriders) and navigate to one or more of them. This would 
> need every overrider relationship to be recorded in the index 
> (`RelationOverrideOf`).
>
>   Intuitively, it seems like `RelationOverrideOf` would be slightly denser 
> than `RelationChildOf` (though not by much), while `RelationCalledBy` would 
> be significantly denser. In terms of queries, I believe the key for lookups 
> for both of the above would be a (subject, predicate) pair, just like for 
> subtypes.
>
>   Does that change your analysis at all?


Sorry for the slow response here. Override and callgraph are great examples!
As you say, override is probably pretty sparse and it's probably not worth 
worrying about the storage too much.

If we stored a callgraph we'd definitely need to worry about the representation 
though. The space-saving hierarchy in this case would be map>I guess. Maybe storing one `vector>` for each relationship type would work here - querying for a bunch 
of relationship types is rare.
One thing that strikes me here is that this case is very similar to our 
existing Ref data - it's basically a subset, but with a symbolid payload 
instead of location. We could consider just adding the SymbolID to Refs - it'd 
blow up the size of that by 50%, but we may not do much better with some other 
representation, and it would avoid adding any new complexity.
Ultimately it may also be that supporting both find references and callgraph 
(which provide closely overlapping functionality) isn't a good use of index 
memory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

I cannot think of other users, so I would prefer to put it in the CTU lib for 
now.


Repository:
  rC Clang

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

https://reviews.llvm.org/D46421



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

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl marked 3 inline comments as done.
r.stahl added a comment.

In D46421#1456171 , @xazax.hun wrote:

> My last set of comments are also unresolved. Most of them are minor nits, but 
> I would love to get rid of the code duplication between ClangExtDefMapGen and 
> the Clang Static Analyzer regarding when do we consider a variable worth to 
> import. Otherwise the patch looks good to me.


Sorry, was too long ago, thought this was done!

What would be a good place for this? I'm thinking ExprClassification in the AST 
lib. There is a tail in `IsModifiable` that tests the `CanQualType` for const 
that could be moved out. And then make a new function `bool ContainsConst(const 
VarDecl *VD)` mabye? Shouldn't that be a new patch, because it touches the more 
critical AST lib?


Repository:
  rC Clang

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

https://reviews.llvm.org/D46421



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

[PATCH] D60323: [clangd] Include compile command inference in fileStatus API

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ioeric, javed.absar, ilya-biryukov.
Herald added a project: clang.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D60323

Files:
  clangd/GlobalCompilationDatabase.cpp
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  test/clangd/filestatus.test

Index: test/clangd/filestatus.test
===
--- test/clangd/filestatus.test
+++ test/clangd/filestatus.test
@@ -4,6 +4,7 @@
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"int x; int y = x;"}}}
 #  CHECK:  "method": "textDocument/clangd.fileStatus",
 # CHECK-NEXT:  "params": {
+# CHECK-NEXT:"compileCommandHeuristic": "default flags for unknown file"
 # CHECK-NEXT:"state": "parsing includes",
 # CHECK-NEXT:"uri": "{{.*}}/main.cpp"
 # CHECK-NEXT:  }
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -62,8 +62,7 @@
 Idle, // Indicates the worker thread is idle, and ready to run any upcoming
   // actions.
   };
-  TUAction(State S, llvm::StringRef Name) : S(S), Name(Name) {}
-  State S;
+  State S = State::Idle;
   /// The name of the action currently running, e.g. Update, GoToDef, Hover.
   /// Empty if we are in the idle state.
   std::string Name;
@@ -82,6 +81,7 @@
 
   TUAction Action;
   BuildDetails Details;
+  std::string CompileCommandHeuristic;
 };
 
 class ParsingCallbacks {
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -203,7 +203,7 @@
   void startTask(llvm::StringRef Name, llvm::unique_function Task,
  llvm::Optional UpdateType);
   /// Updates the TUStatus and emits it. Only called in the worker thread.
-  void emitTUStatus(TUAction FAction,
+  void emitTUStatus(TUAction::State State, llvm::StringRef Name,
 const TUStatus::BuildDetails *Detail = nullptr);
 
   /// Determines the next action to perform.
@@ -327,9 +327,7 @@
  bool StorePreamblesInMemory, ParsingCallbacks )
 : IdleASTs(LRUCache), RunSync(RunSync), UpdateDebounce(UpdateDebounce),
   FileName(FileName), StorePreambleInMemory(StorePreamblesInMemory),
-  Callbacks(Callbacks), Status{TUAction(TUAction::Idle, ""),
-   TUStatus::BuildDetails()},
-  Barrier(Barrier), Done(false) {}
+  Callbacks(Callbacks), Barrier(Barrier), Done(false) {}
 
 ASTWorker::~ASTWorker() {
   // Make sure we remove the cached AST, if any.
@@ -353,7 +351,7 @@
 bool PrevDiagsWereReported = DiagsWereReported;
 FileInputs = Inputs;
 DiagsWereReported = false;
-emitTUStatus({TUAction::BuildingPreamble, TaskName});
+emitTUStatus(TUAction::BuildingPreamble, TaskName);
 log("Updating file {0} with command [{1}] {2}", FileName,
 Inputs.CompileCommand.Directory,
 llvm::join(Inputs.CompileCommand.CommandLine, " "));
@@ -366,7 +364,7 @@
   IdleASTs.take(this);
   TUStatus::BuildDetails Details;
   Details.BuildFailed = true;
-  emitTUStatus({TUAction::BuildingPreamble, TaskName}, );
+  emitTUStatus(TUAction::BuildingPreamble, TaskName, );
   // Make sure anyone waiting for the preamble gets notified it could not
   // be built.
   PreambleWasBuilt.notify();
@@ -393,7 +391,7 @@
 // to it.
 OldPreamble.reset();
 PreambleWasBuilt.notify();
-emitTUStatus({TUAction::BuildingFile, TaskName});
+emitTUStatus(TUAction::BuildingFile, TaskName);
 if (!CanReuseAST) {
   IdleASTs.take(this); // Remove the old AST if it's still in cache.
 } else {
@@ -412,7 +410,7 @@
 FileName);
 TUStatus::BuildDetails Details;
 Details.ReuseAST = true;
-emitTUStatus({TUAction::BuildingFile, TaskName}, );
+emitTUStatus(TUAction::BuildingFile, TaskName, );
 return;
   }
 }
@@ -438,13 +436,13 @@
   if (!(*AST)) { // buildAST fails.
 TUStatus::BuildDetails Details;
 Details.BuildFailed = true;
-emitTUStatus({TUAction::BuildingFile, TaskName}, );
+emitTUStatus(TUAction::BuildingFile, TaskName, );
   }
 } else {
   // We are reusing the AST.
   TUStatus::BuildDetails Details;
   Details.ReuseAST = true;
-  emitTUStatus({TUAction::BuildingFile, TaskName}, );
+  emitTUStatus(TUAction::BuildingFile, TaskName, );
 }
 // We want to report the diagnostics even if this update was cancelled.
 // It seems more useful than making the clients wait indefinitely if they
@@ -579,11 +577,13 @@
   RequestsCV.notify_all();
 }
 
-void 

[PATCH] D60283: [clang-cl] Don't emit checksums when compiling a preprocessed CPP

[Alexandre Ganea via Phabricator via cfe-commits]

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

I made a more elegant change, where no additional lookups or string compares 
are required.

I noted btw that clang /E does not generate `#line` directives, but simply `#`. 
In contrast, MSVC cl /E generates `#line` in the preprocessed output. The issue 
is that MSVC doesn't like `#` alone and throws an error. ie:

  $ cat test.cpp
  void test() { }
  
  $ clang-cl /c /E test.cpp >test-pre.cpp
  
  $ cat test-pre.cpp
  # 1 "C:\\Users\\aganea\\Desktop\\test\\test.cpp"
  # 1 "" 1
  # 1 "" 3
  # 361 "" 3
  # 1 "" 1
  # 1 "" 2
  # 1 "C:\\Users\\aganea\\Desktop\\test\\test.cpp" 2
  void test() { }
  
  $ cl /c test-pre.cpp
  Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27027.1 for x64
  Copyright (C) Microsoft Corporation.  All rights reserved.
  
  test-pre.cpp
  C:\Users\aganea\Desktop\test\test-pre.cpp(1): error C2019: expected 
preprocessor directive, found '1'
  C:\Users\aganea\Desktop\test\test-pre.cpp(2): error C2019: expected 
preprocessor directive, found '1'
  C:\Users\aganea\Desktop\test\test-pre.cpp(3): error C2019: expected 
preprocessor directive, found '1'
  C:\Users\aganea\Desktop\test\test-pre.cpp(4): error C2019: expected 
preprocessor directive, found '3'
  C:\Users\aganea\Desktop\test\test-pre.cpp(5): error C2019: expected 
preprocessor directive, found '1'
  C:\Users\aganea\Desktop\test\test-pre.cpp(6): error C2019: expected 
preprocessor directive, found '1'
  C:\Users\aganea\Desktop\test\test-pre.cpp(7): error C2019: expected 
preprocessor directive, found '1'
  
  $ cl /c /E test.cpp >test-pre.cpp
  
  $ cat test-pre.cpp
  #line 1 "C:\\Users\\aganea\\Desktop\\test\\test.cpp"
  void test() { }
  
  $ cl /c test-pre.cpp
  Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27027.1 for x64
  Copyright (C) Microsoft Corporation.  All rights reserved.
  
  test-pre.cpp


Repository:
  rC Clang

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

https://reviews.llvm.org/D60283

Files:
  include/clang/Basic/SourceLocation.h
  lib/Basic/SourceManager.cpp
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGen/Inputs/debug-info-file-checksum-pre.cpp
  test/CodeGen/debug-info-file-checksum.c

Index: test/CodeGen/debug-info-file-checksum.c
===
--- test/CodeGen/debug-info-file-checksum.c
+++ test/CodeGen/debug-info-file-checksum.c
@@ -4,3 +4,11 @@
 // Check that "checksum" is created correctly for the compiled file.
 
 // CHECK: !DIFile(filename:{{.*}}, directory:{{.*}}, checksumkind: CSK_MD5, checksum: "a3b7d27af071accdeccaa933fc603608")
+
+// Ensure #line directives (in already pre-processed files) do not emit checksums
+
+// RUN: %clang -emit-llvm -S -g -gcodeview -x c %S/Inputs/debug-info-file-checksum-pre.cpp -o - | FileCheck %s --check-prefix NOCHECKSUM
+
+// NOCHECKSUM-LABEL: !DIFile(filename: "{{.*}}code-coverage-filter1.h", directory: "{{[^"]*}}")
+// NOCHECKSUM-LABEL: !DIFile(filename: "{{.*}}code-coverage-filter2.h", directory: "{{[^"]*}}")
+// NOCHECKSUM-LABEL: !DIFile(filename: "{{.*}}debug-info-file-checksum.c", directory: "{{[^"]*}}")
Index: test/CodeGen/Inputs/debug-info-file-checksum-pre.cpp
===
--- test/CodeGen/Inputs/debug-info-file-checksum-pre.cpp
+++ test/CodeGen/Inputs/debug-info-file-checksum-pre.cpp
@@ -0,0 +1,10 @@
+#line 1 "F:\\svn\\clang\\test\\CodeGen\\Inputs\\debug-info-file-checksum.c"
+#line 1 "f:\\svn\\clang\\test\\codegen\\inputs\\code-coverage-filter1.h"
+void test1() {}
+#line 2 "F:\\svn\\clang\\test\\CodeGen\\Inputs\\debug-info-file-checksum.c"
+#line 1 "f:\\svn\\clang\\test\\codegen\\inputs\\code-coverage-filter2.h"
+void test2() {}
+#line 3 "F:\\svn\\clang\\test\\CodeGen\\Inputs\\debug-info-file-checksum.c"
+int foo(int x) {
+  return x+1;
+}
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -422,11 +422,18 @@
   }
 
   SmallString<32> Checksum;
-  Optional CSKind =
-  computeChecksum(SM.getFileID(Loc), Checksum);
   Optional> CSInfo;
-  if (CSKind)
-CSInfo.emplace(*CSKind, Checksum);
+
+  // Don't compute checksums if the location is affected by a #line directive.
+  // We would otherwise end up with the same checksum for all the files referred
+  // by #line. Instead we simply leave the checksum empty, and the debugger will
+  // open the file without checksums.
+  if (!PLoc.isAffectedByLineDirective()) {
+Optional CSKind =
+computeChecksum(SM.getFileID(Loc), Checksum);
+if (CSKind)
+  CSInfo.emplace(*CSKind, Checksum);
+  }
   return createFile(FileName, CSInfo, getSource(SM, SM.getFileID(Loc)));
 }
 
Index: lib/Basic/SourceManager.cpp
===
--- 

[PATCH] D59425: Explicitly Craft a Path to Compiler-RT Builtins on Bare Metal Targets

[Robert Widmann via Phabricator via cfe-commits]

CodaFi added a comment.

@mehdi_amini @kristina Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59425



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

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

[Reuben Thomas via Phabricator via cfe-commits]

reuk updated this revision to Diff 193889.

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

https://reviews.llvm.org/D60320

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9683,6 +9683,16 @@
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 }
 
+TEST_F(FormatTest, SpaceAfterLogicalNot) {
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpaceAfterLogicalNot = true;
+
+  verifyFormat("bool x = ! y", Spaces);
+  verifyFormat("if (! isFailure())", Spaces);
+  verifyFormat("if (! (a && b))", Spaces);
+  verifyFormat("\"Error!\"", Spaces);
+}
+
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
   FormatStyle Spaces = getLLVMStyle();
 
@@ -11475,6 +11485,12 @@
   CHECK_PARSE("SpaceAfterControlStatementKeyword: true", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
 
+  Style.SpaceAfterLogicalNot = false;
+  CHECK_PARSE("SpaceAfterLogicalNot: true", SpaceAfterLogicalNot,
+  true);
+  CHECK_PARSE("SpaceAfterLogicalNot: false", SpaceAfterLogicalNot,
+  false);
+
   Style.ColumnLimit = 123;
   FormatStyle BaseStyle = getLLVMStyle();
   CHECK_PARSE("BasedOnStyle: LLVM", ColumnLimit, BaseStyle.ColumnLimit);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2831,8 +2831,10 @@
   return false;
 return true;
   }
-  if (Left.is(TT_UnaryOperator))
-return Right.is(TT_BinaryOperator);
+  if (Left.is(TT_UnaryOperator)) {
+return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
+   Right.is(TT_BinaryOperator);
+  }
 
   // If the next token is a binary operator or a selector name, we have
   // incorrectly classified the parenthesis as a cast. FIXME: Detect correctly.
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -489,6 +489,7 @@
 IO.mapOptional("SpaceBeforeInheritanceColon",
Style.SpaceBeforeInheritanceColon);
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
+IO.mapOptional("SpaceAfterLogicalNot", Style.SpaceAfterLogicalNot);
 IO.mapOptional("SpaceBeforeRangeBasedForLoopColon",
Style.SpaceBeforeRangeBasedForLoopColon);
 IO.mapOptional("SpaceInEmptyParentheses", Style.SpaceInEmptyParentheses);
@@ -726,6 +727,7 @@
   LLVMStyle.ReflowComments = true;
   LLVMStyle.SpacesInParentheses = false;
   LLVMStyle.SpacesInSquareBrackets = false;
+  LLVMStyle.SpaceAfterLogicalNot = false;
   LLVMStyle.SpaceInEmptyParentheses = false;
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1718,6 +1718,9 @@
   /// Defines in which cases to put a space before opening parentheses.
   SpaceBeforeParensOptions SpaceBeforeParens;
 
+  /// Defines whether a space should be placed after a logical `!`
+  bool SpaceAfterLogicalNot;
+
   /// If ``false``, spaces will be removed before range-based for loop
   /// colon.
   /// \code
@@ -1918,6 +1921,7 @@
R.SpaceBeforeCtorInitializerColon &&
SpaceBeforeInheritanceColon == R.SpaceBeforeInheritanceColon &&
SpaceBeforeParens == R.SpaceBeforeParens &&
+   SpaceAfterLogicalNot == R.SpaceAfterLogicalNot &&
SpaceBeforeRangeBasedForLoopColon ==
R.SpaceBeforeRangeBasedForLoopColon &&
SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9683,6 +9683,16 @@
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 }
 
+TEST_F(FormatTest, SpaceAfterLogicalNot) {
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpaceAfterLogicalNot = true;
+
+  verifyFormat("bool x = ! y", Spaces);
+  verifyFormat("if (! isFailure())", Spaces);
+  verifyFormat("if (! (a && b))", Spaces);
+  verifyFormat("\"Error!\"", Spaces);
+}
+
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
   FormatStyle Spaces = getLLVMStyle();
 
@@ -11475,6 +11485,12 @@
   CHECK_PARSE("SpaceAfterControlStatementKeyword: true", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
 
+  Style.SpaceAfterLogicalNot = false;
+  

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

[Reuben Thomas via Phabricator via cfe-commits]

reuk created this revision.
reuk added reviewers: MyDeveloperDay, klimek.
reuk added a project: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch aims to support the following rule from the Juce coding standards 
:

> The ! operator should always be followed by a space, e.g. if (! foo)

Leaving a space after `!` stops it from blending into the rest of the 
expression, which makes it easier to tell at a glance what the expression is 
doing.

Patch by Reuben Thomas


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D60320

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9683,6 +9683,16 @@
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 }
 
+TEST_F(FormatTest, SpaceAfterLogicalNot) {
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpaceAfterLogicalNot = true;
+
+  verifyFormat("bool x = ! y", Spaces);
+  verifyFormat("if (! isFailure())", Spaces);
+  verifyFormat("if (! (a && b))", Spaces);
+  verifyFormat("\"Error!\"", Spaces);
+}
+
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
   FormatStyle Spaces = getLLVMStyle();
 
@@ -11475,6 +11485,12 @@
   CHECK_PARSE("SpaceAfterControlStatementKeyword: true", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
 
+  Style.SpaceAfterLogicalNot = false;
+  CHECK_PARSE("SpaceAfterLogicalNot: true", SpaceAfterLogicalNot,
+  true);
+  CHECK_PARSE("SpaceAfterLogicalNot: false", SpaceAfterLogicalNot,
+  false);
+
   Style.ColumnLimit = 123;
   FormatStyle BaseStyle = getLLVMStyle();
   CHECK_PARSE("BasedOnStyle: LLVM", ColumnLimit, BaseStyle.ColumnLimit);
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2831,8 +2831,10 @@
   return false;
 return true;
   }
-  if (Left.is(TT_UnaryOperator))
-return Right.is(TT_BinaryOperator);
+  if (Left.is(TT_UnaryOperator)) {
+return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
+   Right.is(TT_BinaryOperator);
+  }
 
   // If the next token is a binary operator or a selector name, we have
   // incorrectly classified the parenthesis as a cast. FIXME: Detect correctly.
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -489,6 +489,7 @@
 IO.mapOptional("SpaceBeforeInheritanceColon",
Style.SpaceBeforeInheritanceColon);
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
+IO.mapOptional("SpaceAfterLogicalNot", Style.SpaceAfterLogicalNot);
 IO.mapOptional("SpaceBeforeRangeBasedForLoopColon",
Style.SpaceBeforeRangeBasedForLoopColon);
 IO.mapOptional("SpaceInEmptyParentheses", Style.SpaceInEmptyParentheses);
@@ -726,6 +727,7 @@
   LLVMStyle.ReflowComments = true;
   LLVMStyle.SpacesInParentheses = false;
   LLVMStyle.SpacesInSquareBrackets = false;
+  LLVMStyle.SpaceAfterLogicalNot = false;
   LLVMStyle.SpaceInEmptyParentheses = false;
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -1718,6 +1718,9 @@
   /// Defines in which cases to put a space before opening parentheses.
   SpaceBeforeParensOptions SpaceBeforeParens;
 
+  /// Defines whether a space should be placed after a logical `!`
+  bool SpaceAfterLogicalNot;
+
   /// If ``false``, spaces will be removed before range-based for loop
   /// colon.
   /// \code
@@ -1726,7 +1729,7 @@
   /// \endcode
   bool SpaceBeforeRangeBasedForLoopColon;
 
-  /// If ``true``, spaces may be inserted into ``()``.
+  /// \brief If ``true``, spaces may be inserted into ``()``.
   /// \code
   ///true:false:
   ///void f( ) {vs.   void f() {
@@ -1918,6 +1921,7 @@
R.SpaceBeforeCtorInitializerColon &&
SpaceBeforeInheritanceColon == R.SpaceBeforeInheritanceColon &&
SpaceBeforeParens == R.SpaceBeforeParens &&
+   SpaceAfterLogicalNot == R.SpaceAfterLogicalNot &&
SpaceBeforeRangeBasedForLoopColon ==
R.SpaceBeforeRangeBasedForLoopColon &&
SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r357770 - [Tooling] add a Heuristic field indicating that a CompileCommand was guessed.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Apr  5 08:22:20 2019
New Revision: 357770

URL: http://llvm.org/viewvc/llvm-project?rev=357770=rev
Log:
[Tooling] add a Heuristic field indicating that a CompileCommand was guessed.

Summary:
Use cases:
 - a tool that dumps the heuristic used for each header in a project can
   be used to evaluate changes to the heuristic
 - we want to expose this information to users in clangd as it affects
   accuracy/reliability of editor features
 - express interpolation tests more directly

Reviewers: ilya-biryukov, klimek

Subscribers: ioeric, kadircet, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Tooling/CompilationDatabase.h
cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp

Modified: cfe/trunk/include/clang/Tooling/CompilationDatabase.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/CompilationDatabase.h?rev=357770=357769=357770=diff
==
--- cfe/trunk/include/clang/Tooling/CompilationDatabase.h (original)
+++ cfe/trunk/include/clang/Tooling/CompilationDatabase.h Fri Apr  5 08:22:20 
2019
@@ -59,9 +59,15 @@ struct CompileCommand {
   /// The output file associated with the command.
   std::string Output;
 
+  /// If this compile command was guessed rather than read from an 
authoritative
+  /// source, a short human-readable explanation.
+  /// e.g. "inferred from foo/bar.h".
+  std::string Heuristic;
+
   friend bool operator==(const CompileCommand , const CompileCommand ) 
{
 return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename &&
-   LHS.CommandLine == RHS.CommandLine && LHS.Output == RHS.Output;
+   LHS.CommandLine == RHS.CommandLine && LHS.Output == RHS.Output &&
+   LHS.Heuristic == RHS.Heuristic;
   }
 
   friend bool operator!=(const CompileCommand , const CompileCommand ) 
{

Modified: cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp?rev=357770=357769=357770=diff
==
--- cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp (original)
+++ cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp Fri Apr  5 
08:22:20 2019
@@ -226,6 +226,7 @@ struct TransferableCommand {
   LangStandard::getLangStandardForKind(Std).getName()).str());
 }
 Result.CommandLine.push_back(Filename);
+Result.Heuristic = "inferred from " + Cmd.Filename;
 return Result;
   }
 

Modified: cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp?rev=357770=357769=357770=diff
==
--- cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp Fri Apr  5 08:22:20 
2019
@@ -673,6 +673,27 @@ protected:
 return llvm::join(Results[0].CommandLine, " ");
   }
 
+  // Parse the file whose command was used out of the Heuristic string.
+  std::string getProxy(llvm::StringRef F) {
+auto Results =
+inferMissingCompileCommands(llvm::make_unique(Entries))
+->getCompileCommands(path(F));
+if (Results.empty())
+  return "none";
+StringRef Proxy = Results.front().Heuristic;
+if (!Proxy.consume_front("inferred from "))
+  return "";
+// We have a proxy file, convert back to a unix relative path.
+// This is a bit messy, but we do need to test these strings somehow...
+llvm::SmallString<32> TempDir;
+llvm::sys::path::system_temp_directory(false, TempDir);
+Proxy.consume_front(TempDir);
+Proxy.consume_front(llvm::sys::path::get_separator());
+llvm::SmallString<32> Result = Proxy;
+llvm::sys::path::native(Result, llvm::sys::path::Style::posix);
+return Result.str();
+  }
+
   MemCDB::EntryMap Entries;
 };
 
@@ -682,18 +703,16 @@ TEST_F(InterpolateTest, Nearby) {
   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(getProxy("dir/f.cpp"), "dir/foo.cpp");
+  EXPECT_EQ(getProxy("dir/FOO.cpp"), "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(getProxy("dir/a.cpp"), "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");
+  EXPECT_EQ(getProxy("some/other/bar.h"), "dir/bar.cpp");
   // two segments of 

[PATCH] D60194: [Tooling] add a Heuristic field indicating that a CompileCommand was guessed.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357770: [Tooling] add a Heuristic field indicating that a 
CompileCommand was guessed. (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60194

Files:
  cfe/trunk/include/clang/Tooling/CompilationDatabase.h
  cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
  cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp


Index: cfe/trunk/include/clang/Tooling/CompilationDatabase.h
===
--- cfe/trunk/include/clang/Tooling/CompilationDatabase.h
+++ cfe/trunk/include/clang/Tooling/CompilationDatabase.h
@@ -59,9 +59,15 @@
   /// The output file associated with the command.
   std::string Output;
 
+  /// If this compile command was guessed rather than read from an 
authoritative
+  /// source, a short human-readable explanation.
+  /// e.g. "inferred from foo/bar.h".
+  std::string Heuristic;
+
   friend bool operator==(const CompileCommand , const CompileCommand ) 
{
 return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename &&
-   LHS.CommandLine == RHS.CommandLine && LHS.Output == RHS.Output;
+   LHS.CommandLine == RHS.CommandLine && LHS.Output == RHS.Output &&
+   LHS.Heuristic == RHS.Heuristic;
   }
 
   friend bool operator!=(const CompileCommand , const CompileCommand ) 
{
Index: cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -226,6 +226,7 @@
   LangStandard::getLangStandardForKind(Std).getName()).str());
 }
 Result.CommandLine.push_back(Filename);
+Result.Heuristic = "inferred from " + Cmd.Filename;
 return Result;
   }
 
Index: cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp
+++ cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -673,6 +673,27 @@
 return llvm::join(Results[0].CommandLine, " ");
   }
 
+  // Parse the file whose command was used out of the Heuristic string.
+  std::string getProxy(llvm::StringRef F) {
+auto Results =
+inferMissingCompileCommands(llvm::make_unique(Entries))
+->getCompileCommands(path(F));
+if (Results.empty())
+  return "none";
+StringRef Proxy = Results.front().Heuristic;
+if (!Proxy.consume_front("inferred from "))
+  return "";
+// We have a proxy file, convert back to a unix relative path.
+// This is a bit messy, but we do need to test these strings somehow...
+llvm::SmallString<32> TempDir;
+llvm::sys::path::system_temp_directory(false, TempDir);
+Proxy.consume_front(TempDir);
+Proxy.consume_front(llvm::sys::path::get_separator());
+llvm::SmallString<32> Result = Proxy;
+llvm::sys::path::native(Result, llvm::sys::path::Style::posix);
+return Result.str();
+  }
+
   MemCDB::EntryMap Entries;
 };
 
@@ -682,18 +703,16 @@
   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(getProxy("dir/f.cpp"), "dir/foo.cpp");
+  EXPECT_EQ(getProxy("dir/FOO.cpp"), "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(getProxy("dir/a.cpp"), "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");
+  EXPECT_EQ(getProxy("some/other/bar.h"), "dir/bar.cpp");
   // 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(getProxy("an/other/b.cpp"), "an/other/foo.cpp");
   // if nothing matches at all, we still get the closest alpha match
-  EXPECT_EQ(getCommand("below/some/obscure/path.cpp"),
-"clang -D an/other/foo.cpp");
+  EXPECT_EQ(getProxy("below/some/obscure/path.cpp"), "an/other/foo.cpp");
 }
 
 TEST_F(InterpolateTest, Language) {
@@ -727,7 +746,7 @@
   add("FOO/BAR/BAZ/SHOUT.cc");
   add("foo/bar/baz/quiet.cc");
   // Case mismatches are completely ignored, so we choose the name match.
-  EXPECT_EQ(getCommand("foo/bar/baz/shout.C"), "clang -D 
FOO/BAR/BAZ/SHOUT.cc");
+  EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
 TEST_F(InterpolateTest, Aliasing) {


Index: cfe/trunk/include/clang/Tooling/CompilationDatabase.h

r357768 - [LibTooling] Add Transformer, a library for source-to-source transformations.

[Yitzhak Mandelbaum via cfe-commits]

Author: ymandel
Date: Fri Apr  5 08:14:05 2019
New Revision: 357768

URL: http://llvm.org/viewvc/llvm-project?rev=357768=rev
Log:
[LibTooling] Add Transformer, a library for source-to-source transformations.

Summary: Adds a basic version of Transformer, a library supporting the concise 
specification of clang-based source-to-source transformations.  A full 
discussion of the end goal can be found on the cfe-dev list with subject "[RFC] 
Easier source-to-source transformations with clang tooling".

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: ioeric, ABataev, mgorny, jfb, jdoerfert, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
cfe/trunk/unittests/Tooling/TransformerTest.cpp
Modified:
cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
cfe/trunk/unittests/Tooling/CMakeLists.txt

Added: cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h?rev=357768=auto
==
--- cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h (added)
+++ cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h Fri Apr  5 
08:14:05 2019
@@ -0,0 +1,210 @@
+//===--- Transformer.h - Clang source-rewriting library -*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+///  \file
+///  Defines a library supporting the concise specification of clang-based
+///  source-to-source transformations.
+///
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_REFACTOR_TRANSFORMER_H_
+#define LLVM_CLANG_TOOLING_REFACTOR_TRANSFORMER_H_
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersInternal.h"
+#include "clang/Tooling/Refactoring/AtomicChange.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tooling {
+/// Determines the part of the AST node to replace.  We support this to work
+/// around the fact that the AST does not differentiate various syntactic
+/// elements into their own nodes, so users can specify them relative to a 
node,
+/// instead.
+enum class NodePart {
+  /// The node itself.
+  Node,
+  /// Given a \c MemberExpr, selects the member's token.
+  Member,
+  /// Given a \c NamedDecl or \c CxxCtorInitializer, selects that token of the
+  /// relevant name, not including qualifiers.
+  Name,
+};
+
+using TextGenerator =
+std::function;
+
+/// Description of a source-code transformation.
+//
+// A *rewrite rule* describes a transformation of source code. It has the
+// following components:
+//
+// * Matcher: the pattern term, expressed as clang matchers (with Transformer
+//   extensions).
+//
+// * Target: the source code impacted by the rule. This identifies an AST node,
+//   or part thereof (\c TargetPart), whose source range indicates the extent 
of
+//   the replacement applied by the replacement term.  By default, the extent 
is
+//   the node matched by the pattern term (\c NodePart::Node). Target's are
+//   typed (\c TargetKind), which guides the determination of the node extent
+//   and might, in the future, statically constrain the set of eligible
+//   NodeParts for a given node.
+//
+// * Replacement: a function that produces a replacement string for the target,
+//   based on the match result.
+//
+// * Explanation: explanation of the rewrite.  This will be displayed to the
+//   user, where possible (for example, in clang-tidy fix descriptions).
+//
+// Rules have an additional, implicit, component: the parameters. These are
+// portions of the pattern which are left unspecified, yet named so that we can
+// reference them in the replacement term.  The structure of parameters can be
+// partially or even fully specified, in which case they serve just to identify
+// matched nodes for later reference rather than abstract over portions of the
+// AST.  However, in all cases, we refer to named portions of the pattern as
+// parameters.
+//
+// RewriteRule is constructed in a "fluent" style, by creating a builder and
+// chaining setters of individual components.
+// \code
+//   RewriteRule MyRule = buildRule(functionDecl(...)).replaceWith(...);
+// \endcode
+//
+// The \c Transformer class should then be used to apply the rewrite rule and
+// obtain the corresponding replacements.
+struct RewriteRule {
+  // `Matcher` describes 

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357768: [LibTooling] Add Transformer, a library for 
source-to-source transformations. (authored by ymandel, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59376?vs=193869=193881#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59376

Files:
  cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
  cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
  cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
  cfe/trunk/unittests/Tooling/CMakeLists.txt
  cfe/trunk/unittests/Tooling/TransformerTest.cpp

Index: cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
===
--- cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
+++ cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
@@ -13,6 +13,7 @@
   Rename/USRFindingAction.cpp
   Rename/USRLocFinder.cpp
   SourceCode.cpp
+  Transformer.cpp
 
   LINK_LIBS
   clangAST
Index: cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
@@ -0,0 +1,203 @@
+//===--- Transformer.cpp - Transformer library implementation ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/Transformer.h"
+#include "clang/AST/Expr.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/Refactoring/AtomicChange.h"
+#include "clang/Tooling/Refactoring/SourceCode.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Error.h"
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace tooling;
+
+using ast_matchers::MatchFinder;
+using ast_type_traits::ASTNodeKind;
+using ast_type_traits::DynTypedNode;
+using llvm::Error;
+using llvm::Expected;
+using llvm::Optional;
+using llvm::StringError;
+using llvm::StringRef;
+using llvm::Twine;
+
+using MatchResult = MatchFinder::MatchResult;
+
+// Did the text at this location originate in a macro definition (aka. body)?
+// For example,
+//
+//   #define NESTED(x) x
+//   #define MACRO(y) { int y  = NESTED(3); }
+//   if (true) MACRO(foo)
+//
+// The if statement expands to
+//
+//   if (true) { int foo = 3; }
+//   ^ ^
+//   Loc1  Loc2
+//
+// For SourceManager SM, SM.isMacroArgExpansion(Loc1) and
+// SM.isMacroArgExpansion(Loc2) are both true, but isOriginMacroBody(sm, Loc1)
+// is false, because "foo" originated in the source file (as an argument to a
+// macro), whereas isOriginMacroBody(SM, Loc2) is true, because "3" originated
+// in the definition of MACRO.
+static bool isOriginMacroBody(const clang::SourceManager ,
+  clang::SourceLocation Loc) {
+  while (Loc.isMacroID()) {
+if (SM.isMacroBodyExpansion(Loc))
+  return true;
+// Otherwise, it must be in an argument, so we continue searching up the
+// invocation stack. getImmediateMacroCallerLoc() gives the location of the
+// argument text, inside the call text.
+Loc = SM.getImmediateMacroCallerLoc(Loc);
+  }
+  return false;
+}
+
+static llvm::Error invalidArgumentError(Twine Message) {
+  return llvm::make_error(llvm::errc::invalid_argument, Message);
+}
+
+static llvm::Error typeError(StringRef Id, const ASTNodeKind ,
+ Twine Message) {
+  return invalidArgumentError(
+  Message + " (node id=" + Id + " kind=" + Kind.asStringRef() + ")");
+}
+
+static llvm::Error missingPropertyError(StringRef Id, Twine Description,
+StringRef Property) {
+  return invalidArgumentError(Description + " requires property '" + Property +
+  "' (node id=" + Id + ")");
+}
+
+static Expected
+getTargetRange(StringRef Target, const DynTypedNode , ASTNodeKind Kind,
+   NodePart TargetPart, ASTContext ) {
+  switch (TargetPart) {
+  case NodePart::Node: {
+// For non-expression statements, associate any trailing semicolon with the
+// statement text.  However, if the target was intended as an expression (as
+// indicated by its kind) then we do not associate any trailing semicolon
+// with it.  We only associate the exact expression text.
+if (Node.get() != nullptr) 

[PATCH] D60220: [CUDA][Windows] Final fix for bug 38811 (Step 3 of 3)

[Evgeny Mankov via Phabricator via cfe-commits]

emankov updated this revision to Diff 193874.
emankov added a comment.

Provide only declarations for missing long double device functions to prevent 
any use of `long double` on the device side, because CUDA does not support 
`long double` on the device side.

[Testing]
{Windows 10, Ubuntu 16.04.5}/{Visual C++ 2017 15.9.9, gcc+ 5.4.0}/CUDA {8.0, 
9.0, 9.1, 9.2, 10.0, 10.1}


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

https://reviews.llvm.org/D60220

Files:
  clang/lib/Headers/__clang_cuda_cmath.h
  clang/lib/Headers/__clang_cuda_device_functions.h
  clang/lib/Headers/__clang_cuda_math_forward_declares.h


Index: clang/lib/Headers/__clang_cuda_math_forward_declares.h
===
--- clang/lib/Headers/__clang_cuda_math_forward_declares.h
+++ clang/lib/Headers/__clang_cuda_math_forward_declares.h
@@ -98,12 +98,14 @@
 __DEVICE__ float hypot(float, float);
 __DEVICE__ int ilogb(double);
 __DEVICE__ int ilogb(float);
+__DEVICE__ bool isfinite(long double);
 __DEVICE__ bool isfinite(double);
 __DEVICE__ bool isfinite(float);
 __DEVICE__ bool isgreater(double, double);
 __DEVICE__ bool isgreaterequal(double, double);
 __DEVICE__ bool isgreaterequal(float, float);
 __DEVICE__ bool isgreater(float, float);
+__DEVICE__ bool isinf(long double);
 __DEVICE__ bool isinf(double);
 __DEVICE__ bool isinf(float);
 __DEVICE__ bool isless(double, double);
@@ -112,6 +114,7 @@
 __DEVICE__ bool isless(float, float);
 __DEVICE__ bool islessgreater(double, double);
 __DEVICE__ bool islessgreater(float, float);
+__DEVICE__ bool isnan(long double);
 __DEVICE__ bool isnan(double);
 __DEVICE__ bool isnan(float);
 __DEVICE__ bool isnormal(double);
Index: clang/lib/Headers/__clang_cuda_device_functions.h
===
--- clang/lib/Headers/__clang_cuda_device_functions.h
+++ clang/lib/Headers/__clang_cuda_device_functions.h
@@ -237,6 +237,7 @@
 __DEVICE__ int __ffsll(long long __a) { return __nv_ffsll(__a); }
 __DEVICE__ int __finite(double __a) { return __nv_isfinited(__a); }
 __DEVICE__ int __finitef(float __a) { return __nv_finitef(__a); }
+__DEVICE__ int __finitel(long double __a);
 __DEVICE__ int __float2int_rd(float __a) { return __nv_float2int_rd(__a); }
 __DEVICE__ int __float2int_rn(float __a) { return __nv_float2int_rn(__a); }
 __DEVICE__ int __float2int_ru(float __a) { return __nv_float2int_ru(__a); }
@@ -445,8 +446,10 @@
 __DEVICE__ int __isfinited(double __a) { return __nv_isfinited(__a); }
 __DEVICE__ int __isinf(double __a) { return __nv_isinfd(__a); }
 __DEVICE__ int __isinff(float __a) { return __nv_isinff(__a); }
+__DEVICE__ int __isinfl(long double __a);
 __DEVICE__ int __isnan(double __a) { return __nv_isnand(__a); }
 __DEVICE__ int __isnanf(float __a) { return __nv_isnanf(__a); }
+__DEVICE__ int __isnanl(long double __a);
 __DEVICE__ double __ll2double_rd(long long __a) {
   return __nv_ll2double_rd(__a);
 }
Index: clang/lib/Headers/__clang_cuda_cmath.h
===
--- clang/lib/Headers/__clang_cuda_cmath.h
+++ clang/lib/Headers/__clang_cuda_cmath.h
@@ -78,13 +78,16 @@
 #ifndef _MSC_VER
 __DEVICE__ bool isinf(float __x) { return ::__isinff(__x); }
 __DEVICE__ bool isinf(double __x) { return ::__isinf(__x); }
+__DEVICE__ bool isinf(long double __x) { return ::__isinfl(__x); }
 __DEVICE__ bool isfinite(float __x) { return ::__finitef(__x); }
 // For inscrutable reasons, __finite(), the double-precision version of
 // __finitef, does not exist when compiling for MacOS.  __isfinited is 
available
 // everywhere and is just as good.
 __DEVICE__ bool isfinite(double __x) { return ::__isfinited(__x); }
+__DEVICE__ bool isfinite(long double __x) { return ::__finitel(__x); }
 __DEVICE__ bool isnan(float __x) { return ::__isnanf(__x); }
 __DEVICE__ bool isnan(double __x) { return ::__isnan(__x); }
+__DEVICE__ bool isnan(long double __x) { return ::__isnanl(__x); }
 #endif
 
 __DEVICE__ bool isgreater(float __x, float __y) {


Index: clang/lib/Headers/__clang_cuda_math_forward_declares.h
===
--- clang/lib/Headers/__clang_cuda_math_forward_declares.h
+++ clang/lib/Headers/__clang_cuda_math_forward_declares.h
@@ -98,12 +98,14 @@
 __DEVICE__ float hypot(float, float);
 __DEVICE__ int ilogb(double);
 __DEVICE__ int ilogb(float);
+__DEVICE__ bool isfinite(long double);
 __DEVICE__ bool isfinite(double);
 __DEVICE__ bool isfinite(float);
 __DEVICE__ bool isgreater(double, double);
 __DEVICE__ bool isgreaterequal(double, double);
 __DEVICE__ bool isgreaterequal(float, float);
 __DEVICE__ bool isgreater(float, float);
+__DEVICE__ bool isinf(long double);
 __DEVICE__ bool isinf(double);
 __DEVICE__ bool isinf(float);
 __DEVICE__ bool isless(double, double);
@@ -112,6 +114,7 @@
 __DEVICE__ bool isless(float, float);
 __DEVICE__ bool islessgreater(double, double);
 __DEVICE__ bool 

[PATCH] D60194: [Tooling] add a Heuristic field indicating that a CompileCommand was guessed.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D60194#1454633 , @ilya-biryukov 
wrote:

> Yet, having the new field in the `CompileCommand` feels wrong, especially in 
> combination with a fact that `CompilationDatabase` returns a vector of 
> compile commands rather just a single command, i.e. after this patch 
> implementations can technically return a combination of inferred and 
> non-inferred commands for the same file, which might force clients to think 
> about proper prioritization and filtering commands, something I'd rather keep 
> hidden.


I do think the vector return isn't ideal, though I'm not sure 
it's really related to this change - the CDB can return inferred and 
non-inferred commands today, and there's no guidance on which to pick.
(I was thinking of adding some, but different tools do different things e.g. 
pick first, or iterate over all, and I'm not sure which is best :-/)


Repository:
  rC Clang

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

https://reviews.llvm.org/D60194



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

[PATCH] D60194: [Tooling] add a Heuristic field indicating that a CompileCommand was guessed.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM, Manuel does not mind having this and the string heuristic does look like 
the easiest approach to convey this information in the API.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60194



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

[PATCH] D60316: [clangd] Include insertion: require header guards, drop other heuristics, treat .def like .inc.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: unittests/clangd/FileIndexTests.cpp:214
 
 TEST(FileIndexTest, HasSystemHeaderMappingsInPreamble) {
+  TestTU TU;

I suspect we can replace most of these tests with TestTU - here I just changed 
the ones where the include guard would otherwise be needed.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60316



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

[PATCH] D60316: [clangd] Include insertion: require header guards, drop other heuristics, treat .def like .inc.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

We do have some reports of include insertion behaving badly in some
codebases. Requiring header guards both makes sense in principle, and is
likely to disable this "nice-to-have" feature in codebases where headers don't
follow the expected pattern.

With this we can drop some other heuristics, such as looking at file
extensions to detect known non-headers - implementation files have no guards.

One wrinkle here is #import - objc headers may not have guards because
they're intended to be used via #import. If the header is the main file
or is #included, we won't collect locations - merge should take care of
this if we see the file #imported somewhere. Seems likely to be OK.

Headers which have a canonicalization (stdlib, IWYU) are exempt from this check.
*.inc files continue to be handled by looking up to the including file.
This patch also adds *.def here - tablegen wants this pattern too.

In terms of code structure, the division between SymbolCollector and
CanonicalIncludes has shifted: SymbolCollector is responsible for more.
This is because SymbolCollector has all the SourceManager/HeaderSearch access
needed for checking for guards, and we interleave these checks with the *.def
checks in a loop (potentially).
We could hand all the info into CanonicalIncludes and put the logic there
if that's preferable.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D60316

Files:
  clangd/index/CanonicalIncludes.cpp
  clangd/index/CanonicalIncludes.h
  clangd/index/SymbolCollector.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp
  unittests/clangd/TestTU.cpp
  unittests/clangd/TestTU.h

Index: unittests/clangd/TestTU.h
===
--- unittests/clangd/TestTU.h
+++ unittests/clangd/TestTU.h
@@ -52,6 +52,9 @@
   // Index to use when building AST.
   const SymbolIndex *ExternalIndex = nullptr;
 
+  // Simulate a header guard of the header (using an #import directive).
+  bool ImplicitHeaderGuard = true;
+
   ParsedAST build() const;
   SymbolSlab headerSymbols() const;
   std::unique_ptr index() const;
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -19,13 +19,19 @@
 
 ParsedAST TestTU::build() const {
   std::string FullFilename = testPath(Filename),
-  FullHeaderName = testPath(HeaderFilename);
+  FullHeaderName = testPath(HeaderFilename),
+  ImportThunk = testPath("import_thunk.h");
   std::vector Cmd = {"clang", FullFilename.c_str()};
+  // We want to implicitly include HeaderFilename without messing up offsets.
+  // -include achieves this, but sometimes we want #import (to simulate a header
+  // guard without messing up offsets). In this case, use an intermediate file.
+  std::string ThunkContents = "#import \"" + FullHeaderName + "\"\n";
   // FIXME: this shouldn't need to be conditional, but it breaks a
   // GoToDefinition test for some reason (getMacroArgExpandedLocation fails).
   if (!HeaderCode.empty()) {
 Cmd.push_back("-include");
-Cmd.push_back(FullHeaderName.c_str());
+Cmd.push_back(ImplicitHeaderGuard ? ImportThunk.c_str()
+  : FullHeaderName.c_str());
   }
   Cmd.insert(Cmd.end(), ExtraArgs.begin(), ExtraArgs.end());
   ParseInputs Inputs;
@@ -33,7 +39,9 @@
   Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
   Inputs.CompileCommand.Directory = testRoot();
   Inputs.Contents = Code;
-  Inputs.FS = buildTestFS({{FullFilename, Code}, {FullHeaderName, HeaderCode}});
+  Inputs.FS = buildTestFS({{FullFilename, Code},
+   {FullHeaderName, HeaderCode},
+   {ImportThunk, ThunkContents}});
   Inputs.Opts = ParseOptions();
   Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;
   Inputs.Index = ExternalIndex;
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -33,9 +33,7 @@
 using testing::_;
 using testing::AllOf;
 using testing::Contains;
-using testing::Eq;
 using testing::Field;
-using testing::IsEmpty;
 using testing::Not;
 using testing::Pair;
 using testing::UnorderedElementsAre;
@@ -55,6 +53,7 @@
   return StringRef(arg.CanonicalDeclaration.FileURI) == P;
 }
 MATCHER_P(DefURI, P, "") { return StringRef(arg.Definition.FileURI) == P; }
+MATCHER(NoIncludeHeader, "") { return arg.IncludeHeaders.empty(); }
 MATCHER_P(IncludeHeader, P, "") {
   return (arg.IncludeHeaders.size() == 1) &&
  (arg.IncludeHeaders.begin()->IncludeHeader == P);
@@ 

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 193869.
ymandel added a comment.

Rebase onto master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376

Files:
  clang/include/clang/Tooling/Refactoring/Transformer.h
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/Transformer.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -0,0 +1,389 @@
+//===- unittest/Tooling/TransformerTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/Transformer.h"
+
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tooling {
+namespace {
+using ast_matchers::anyOf;
+using ast_matchers::argumentCountIs;
+using ast_matchers::callee;
+using ast_matchers::callExpr;
+using ast_matchers::cxxMemberCallExpr;
+using ast_matchers::cxxMethodDecl;
+using ast_matchers::cxxRecordDecl;
+using ast_matchers::declRefExpr;
+using ast_matchers::expr;
+using ast_matchers::functionDecl;
+using ast_matchers::hasAnyName;
+using ast_matchers::hasArgument;
+using ast_matchers::hasDeclaration;
+using ast_matchers::hasElse;
+using ast_matchers::hasName;
+using ast_matchers::hasType;
+using ast_matchers::ifStmt;
+using ast_matchers::member;
+using ast_matchers::memberExpr;
+using ast_matchers::namedDecl;
+using ast_matchers::on;
+using ast_matchers::pointsTo;
+using ast_matchers::to;
+using ast_matchers::unless;
+
+using llvm::StringRef;
+
+constexpr char KHeaderContents[] = R"cc(
+  struct string {
+string(const char*);
+char* c_str();
+int size();
+  };
+  int strlen(const char*);
+
+  namespace proto {
+  struct PCFProto {
+int foo();
+  };
+  struct ProtoCommandLineFlag : PCFProto {
+PCFProto& GetProto();
+  };
+  }  // namespace proto
+)cc";
+
+static ast_matchers::internal::Matcher
+isOrPointsTo(const clang::ast_matchers::DeclarationMatcher ) {
+  return anyOf(hasDeclaration(TypeMatcher), pointsTo(TypeMatcher));
+}
+
+static std::string format(StringRef Code) {
+  const std::vector Ranges(1, Range(0, Code.size()));
+  auto Style = format::getLLVMStyle();
+  const auto Replacements = format::reformat(Style, Code, Ranges);
+  auto Formatted = applyAllReplacements(Code, Replacements);
+  if (!Formatted) {
+ADD_FAILURE() << "Could not format code: "
+  << llvm::toString(Formatted.takeError());
+return std::string();
+  }
+  return *Formatted;
+}
+
+static void compareSnippets(StringRef Expected,
+ const llvm::Optional ) {
+  ASSERT_TRUE(MaybeActual) << "Rewrite failed. Expecting: " << Expected;
+  auto Actual = *MaybeActual;
+  std::string HL = "#include \"header.h\"\n";
+  auto I = Actual.find(HL);
+  if (I != std::string::npos)
+Actual.erase(I, HL.size());
+  EXPECT_EQ(format(Expected), format(Actual));
+}
+
+// FIXME: consider separating this class into its own file(s).
+class ClangRefactoringTestBase : public testing::Test {
+protected:
+  void appendToHeader(StringRef S) { FileContents[0].second += S; }
+
+  void addFile(StringRef Filename, StringRef Content) {
+FileContents.emplace_back(Filename, Content);
+  }
+
+  llvm::Optional rewrite(StringRef Input) {
+std::string Code = ("#include \"header.h\"\n" + Input).str();
+auto Factory = newFrontendActionFactory();
+if (!runToolOnCodeWithArgs(
+Factory->create(), Code, std::vector(), "input.cc",
+"clang-tool", std::make_shared(),
+FileContents)) {
+  return None;
+}
+auto ChangedCodeOrErr =
+applyAtomicChanges("input.cc", Code, Changes, ApplyChangesSpec());
+if (auto Err = ChangedCodeOrErr.takeError()) {
+  llvm::errs() << "Change failed: " << llvm::toString(std::move(Err))
+   << "\n";
+  return None;
+}
+return *ChangedCodeOrErr;
+  }
+
+  void testRule(RewriteRule Rule, StringRef Input, StringRef Expected) {
+Transformer T(std::move(Rule),
+  [this](const AtomicChange ) { Changes.push_back(C); });
+T.registerMatchers();
+compareSnippets(Expected, rewrite(Input));
+  }
+
+  clang::ast_matchers::MatchFinder MatchFinder;
+  AtomicChanges Changes;
+
+private:
+  FileContentMappings FileContents = {{"header.h", ""}};
+};
+
+class TransformerTest : public ClangRefactoringTestBase {
+protected:
+  TransformerTest() { 

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In D46421#1456155 , @r.stahl wrote:

> Okay so I would suggest to go ahead and commit this. Rebased it succeeds 
> without modification.
>
> Still leaves the open problems with the redecls. Should I add the failing 
> test from https://reviews.llvm.org/D46421#1375147 in the same commit marked 
> as expected to fail? Or what is the procedure here?


My last set of comments are also unresolved. Most of them are minor nits, but I 
would love to get rid of the code duplication between ClangExtDefMapGen and the 
Clang Static Analyzer regarding when do we consider a variable worth to import. 
Otherwise the patch looks good to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D46421



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

[PATCH] D60269: [LibTooling] Add "SourceCode" library for functions relating to source-code manipulation.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357764: [LibTooling] Add SourceCode library for 
functions relating to source-code… (authored by ymandel, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60269?vs=193865=193867#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60269

Files:
  cfe/trunk/include/clang/Tooling/FixIt.h
  cfe/trunk/include/clang/Tooling/Refactoring/SourceCode.h
  cfe/trunk/lib/Tooling/FixIt.cpp
  cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
  cfe/trunk/lib/Tooling/Refactoring/SourceCode.cpp
  cfe/trunk/unittests/Tooling/CMakeLists.txt
  cfe/trunk/unittests/Tooling/FixItTest.cpp
  cfe/trunk/unittests/Tooling/SourceCodeTest.cpp

Index: cfe/trunk/include/clang/Tooling/FixIt.h
===
--- cfe/trunk/include/clang/Tooling/FixIt.h
+++ cfe/trunk/include/clang/Tooling/FixIt.h
@@ -20,7 +20,6 @@
 #define LLVM_CLANG_TOOLING_FIXIT_H
 
 #include "clang/AST/ASTContext.h"
-#include "clang/Basic/TokenKinds.h"
 
 namespace clang {
 namespace tooling {
@@ -44,11 +43,6 @@
 template  CharSourceRange getSourceRange(const T ) {
   return CharSourceRange::getTokenRange(Node.getSourceRange());
 }
-
-/// Extends \p Range to include the token \p Next, if it immediately follows the
-/// end of the range. Otherwise, returns \p Range unchanged.
-CharSourceRange maybeExtendRange(CharSourceRange Range, tok::TokenKind Next,
- ASTContext );
 } // end namespace internal
 
 /// Returns a textual representation of \p Node.
@@ -57,44 +51,6 @@
   return internal::getText(internal::getSourceRange(Node), Context);
 }
 
-/// Returns the source range spanning the node, extended to include \p Next, if
-/// it immediately follows \p Node. Otherwise, returns the normal range of \p
-/// Node.  See comments on `getExtendedText()` for examples.
-template 
-CharSourceRange getExtendedRange(const T , tok::TokenKind Next,
- ASTContext ) {
-  return internal::maybeExtendRange(internal::getSourceRange(Node), Next,
-Context);
-}
-
-/// Returns the source text of the node, extended to include \p Next, if it
-/// immediately follows the node. Otherwise, returns the text of just \p Node.
-///
-/// For example, given statements S1 and S2 below:
-/// \code
-///   {
-/// // S1:
-/// if (!x) return foo();
-/// // S2:
-/// if (!x) { return 3; }
-///   }
-/// \endcode
-/// then
-/// \code
-///   getText(S1, Context) = "if (!x) return foo()"
-///   getExtendedText(S1, tok::TokenKind::semi, Context)
-/// = "if (!x) return foo();"
-///   getExtendedText(*S1.getThen(), tok::TokenKind::semi, Context)
-/// = "return foo();"
-///   getExtendedText(*S2.getThen(), tok::TokenKind::semi, Context)
-/// = getText(S2, Context) = "{ return 3; }"
-/// \endcode
-template 
-StringRef getExtendedText(const T , tok::TokenKind Next,
-  ASTContext ) {
-  return internal::getText(getExtendedRange(Node, Next, Context), Context);
-}
-
 // Returns a FixItHint to remove \p Node.
 // TODO: Add support for related syntactical elements (i.e. comments, ...).
 template  FixItHint createRemoval(const T ) {
Index: cfe/trunk/include/clang/Tooling/Refactoring/SourceCode.h
===
--- cfe/trunk/include/clang/Tooling/Refactoring/SourceCode.h
+++ cfe/trunk/include/clang/Tooling/Refactoring/SourceCode.h
@@ -0,0 +1,77 @@
+//===--- SourceCode.h - Source code manipulation routines ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+//  This file provides functions that simplify extraction of source code.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_REFACTOR_SOURCE_CODE_H
+#define LLVM_CLANG_TOOLING_REFACTOR_SOURCE_CODE_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
+
+namespace clang {
+namespace tooling {
+
+/// Extends \p Range to include the token \p Next, if it immediately follows the
+/// end of the range. Otherwise, returns \p Range unchanged.
+CharSourceRange maybeExtendRange(CharSourceRange Range, tok::TokenKind Next,
+ ASTContext );
+
+/// Returns the source range spanning the node, extended to include \p Next, if
+/// it immediately follows \p Node. Otherwise, returns the normal range of \p
+/// Node.  See comments on `getExtendedText()` for examples.
+template 

[PATCH] D60269: [LibTooling] Add "SourceCode" library for functions relating to source-code manipulation.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 193865.
ymandel added a comment.

Fix trailing whitespace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60269

Files:
  clang/include/clang/Tooling/FixIt.h
  clang/include/clang/Tooling/Refactoring/SourceCode.h
  clang/lib/Tooling/FixIt.cpp
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/SourceCode.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/FixItTest.cpp
  clang/unittests/Tooling/SourceCodeTest.cpp

Index: clang/unittests/Tooling/SourceCodeTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/SourceCodeTest.cpp
@@ -0,0 +1,97 @@
+//===- unittest/Tooling/SourceCodeTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestVisitor.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Tooling/Refactoring/SourceCode.h"
+
+using namespace clang;
+
+using tooling::getText;
+using tooling::getExtendedText;
+
+namespace {
+
+struct CallsVisitor : TestVisitor {
+  bool VisitCallExpr(CallExpr *Expr) {
+OnCall(Expr, Context);
+return true;
+  }
+
+  std::function OnCall;
+};
+
+TEST(SourceCodeTest, getText) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("foo(x, y)", getText(*CE, *Context));
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("APPLY(foo, x, y)", getText(*CE, *Context));
+  };
+  Visitor.runOver("#define APPLY(f, x, y) f(x, y)\n"
+  "void foo(int x, int y) { APPLY(foo, x, y); }");
+}
+
+TEST(SourceCodeTest, getTextWithMacro) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("F OO", getText(*CE, *Context));
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("", getText(*P0, *Context));
+EXPECT_EQ("", getText(*P1, *Context));
+  };
+  Visitor.runOver("#define F foo(\n"
+  "#define OO x, y)\n"
+  "void foo(int x, int y) { F OO ; }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("", getText(*CE, *Context));
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", getText(*P0, *Context));
+EXPECT_EQ("y", getText(*P1, *Context));
+  };
+  Visitor.runOver("#define FOO(x, y) (void)x; (void)y; foo(x, y);\n"
+  "void foo(int x, int y) { FOO(x,y) }");
+}
+
+TEST(SourceCodeTest, getExtendedText) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("foo(x, y);",
+  getExtendedText(*CE, tok::TokenKind::semi, *Context));
+
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", getExtendedText(*P0, tok::TokenKind::semi, *Context));
+EXPECT_EQ("x,", getExtendedText(*P0, tok::TokenKind::comma, *Context));
+EXPECT_EQ("y", getExtendedText(*P1, tok::TokenKind::semi, *Context));
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+  Visitor.runOver("void foo(int x, int y) { if (true) foo(x, y); }");
+  Visitor.runOver("int foo(int x, int y) { if (true) return 3 + foo(x, y); }");
+  Visitor.runOver("void foo(int x, int y) { for (foo(x, y);;) ++x; }");
+  Visitor.runOver(
+  "bool foo(int x, int y) { for (;foo(x, y);) x = 1; return true; }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("foo()", getExtendedText(*CE, tok::TokenKind::semi, *Context));
+  };
+  Visitor.runOver("bool foo() { if (foo()) return true; return false; }");
+  Visitor.runOver("void foo() { int x; for (;; foo()) ++x; }");
+  Visitor.runOver("int foo() { return foo() + 3; }");
+}
+
+} // end anonymous namespace
Index: clang/unittests/Tooling/FixItTest.cpp
===
--- clang/unittests/Tooling/FixItTest.cpp
+++ clang/unittests/Tooling/FixItTest.cpp
@@ -13,7 +13,6 @@
 using namespace clang;
 
 using tooling::fixit::getText;
-using tooling::fixit::getExtendedText;
 using tooling::fixit::createRemoval;
 using tooling::fixit::createReplacement;
 
@@ -78,34 +77,6 @@
   "void foo(int x, int y) { FOO(x,y) }");
 }
 
-TEST(FixItTest, getExtendedText) {
-  CallsVisitor Visitor;
-
-  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
-EXPECT_EQ("foo(x, y);",
-  getExtendedText(*CE, tok::TokenKind::semi, *Context));
-
-Expr *P0 = CE->getArg(0);
-Expr *P1 = CE->getArg(1);
-EXPECT_EQ("x", getExtendedText(*P0, 

r357764 - [LibTooling] Add "SourceCode" library for functions relating to source-code manipulation.

[Yitzhak Mandelbaum via cfe-commits]

Author: ymandel
Date: Fri Apr  5 07:05:03 2019
New Revision: 357764

URL: http://llvm.org/viewvc/llvm-project?rev=357764=rev
Log:
[LibTooling] Add "SourceCode" library for functions relating to source-code 
manipulation.

Summary:
Introduces a utility library in Refactoring/ to collect routines related to
source-code manipulation.  In this change, we move "extended-range" functions
from the FixIt library (in clangTooling) to this new library.

We need to use this functionality in Refactoring/ and cannot access it if it
resides in Tooling/, because that would cause clangToolingRefactor to depend on
clangTooling, which would be a circular dependency.

Reviewers: ilya-biryukov, ioeric

Reviewed By: ilya-biryukov

Subscribers: mgorny, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/include/clang/Tooling/Refactoring/SourceCode.h
cfe/trunk/lib/Tooling/Refactoring/SourceCode.cpp
cfe/trunk/unittests/Tooling/SourceCodeTest.cpp
Modified:
cfe/trunk/include/clang/Tooling/FixIt.h
cfe/trunk/lib/Tooling/FixIt.cpp
cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
cfe/trunk/unittests/Tooling/CMakeLists.txt
cfe/trunk/unittests/Tooling/FixItTest.cpp

Modified: cfe/trunk/include/clang/Tooling/FixIt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/FixIt.h?rev=357764=357763=357764=diff
==
--- cfe/trunk/include/clang/Tooling/FixIt.h (original)
+++ cfe/trunk/include/clang/Tooling/FixIt.h Fri Apr  5 07:05:03 2019
@@ -20,7 +20,6 @@
 #define LLVM_CLANG_TOOLING_FIXIT_H
 
 #include "clang/AST/ASTContext.h"
-#include "clang/Basic/TokenKinds.h"
 
 namespace clang {
 namespace tooling {
@@ -44,11 +43,6 @@ inline CharSourceRange getSourceRange(co
 template  CharSourceRange getSourceRange(const T ) {
   return CharSourceRange::getTokenRange(Node.getSourceRange());
 }
-
-/// Extends \p Range to include the token \p Next, if it immediately follows 
the
-/// end of the range. Otherwise, returns \p Range unchanged.
-CharSourceRange maybeExtendRange(CharSourceRange Range, tok::TokenKind Next,
- ASTContext );
 } // end namespace internal
 
 /// Returns a textual representation of \p Node.
@@ -57,44 +51,6 @@ StringRef getText(const T , const A
   return internal::getText(internal::getSourceRange(Node), Context);
 }
 
-/// Returns the source range spanning the node, extended to include \p Next, if
-/// it immediately follows \p Node. Otherwise, returns the normal range of \p
-/// Node.  See comments on `getExtendedText()` for examples.
-template 
-CharSourceRange getExtendedRange(const T , tok::TokenKind Next,
- ASTContext ) {
-  return internal::maybeExtendRange(internal::getSourceRange(Node), Next,
-Context);
-}
-
-/// Returns the source text of the node, extended to include \p Next, if it
-/// immediately follows the node. Otherwise, returns the text of just \p Node.
-///
-/// For example, given statements S1 and S2 below:
-/// \code
-///   {
-/// // S1:
-/// if (!x) return foo();
-/// // S2:
-/// if (!x) { return 3; }
-///   }
-/// \endcode
-/// then
-/// \code
-///   getText(S1, Context) = "if (!x) return foo()"
-///   getExtendedText(S1, tok::TokenKind::semi, Context)
-/// = "if (!x) return foo();"
-///   getExtendedText(*S1.getThen(), tok::TokenKind::semi, Context)
-/// = "return foo();"
-///   getExtendedText(*S2.getThen(), tok::TokenKind::semi, Context)
-/// = getText(S2, Context) = "{ return 3; }"
-/// \endcode
-template 
-StringRef getExtendedText(const T , tok::TokenKind Next,
-  ASTContext ) {
-  return internal::getText(getExtendedRange(Node, Next, Context), Context);
-}
-
 // Returns a FixItHint to remove \p Node.
 // TODO: Add support for related syntactical elements (i.e. comments, ...).
 template  FixItHint createRemoval(const T ) {

Added: cfe/trunk/include/clang/Tooling/Refactoring/SourceCode.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/SourceCode.h?rev=357764=auto
==
--- cfe/trunk/include/clang/Tooling/Refactoring/SourceCode.h (added)
+++ cfe/trunk/include/clang/Tooling/Refactoring/SourceCode.h Fri Apr  5 
07:05:03 2019
@@ -0,0 +1,77 @@
+//===--- SourceCode.h - Source code manipulation routines ---*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+//  This file provides functions that simplify extraction of source code.
+//
+//===--===//
+

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59746#1456115 , @alexfh wrote:

> Can you give a specific example of how this problem manifests?


Any tool using `tooling::CommonOptionsParser` with the `llvm::cl::OneOrMore` 
flag will have this problem, i.e., the `-h` flag will be seen, but no action 
taken because it was never wired up.  Please note that we changed `clang-tidy` 
to use `cl::ZeroOrMore` a few years ago, so it'll spit out help with or without 
any arguments, including `-h`.  However, if you pass a bad argument with `-h` 
you'll get an error, but no help.

Here are a few you can verify (first try with `-h`, then `-help`):

  $ bin/clang-tidy -h -x
  LLVM ERROR: CommonOptionsParser: failed to parse command-line arguments. 
[CommonOptionsParser]: clang-tidy: Unknown command line argument '-x'.  Try: 
'bin/clang-tidy -help'
  clang-tidy: Did you mean '-h'?
  
  $ bin/clang-tidy -help -x
  USAGE: clang-tidy [options]  [... ]
  
  
  
  $ bin/clang-refactor -h
  error: no refactoring action given
  note: the following actions are supported:
local-rename
extract
  
  $ bin/clang-refactor -help
  OVERVIEW: Clang-based refactoring tool for C, C++ and Objective-C
  USAGE: clang-refactor [options]  [... ]
  
  
  
  $ bin/clang-apply-replacements -h
  clang-apply-replacements: Unknown command line argument '-h'.  Try: 
'bin/clang-apply-replacements -help'
  clang-apply-replacements: Did you mean '-help'?
  clang-apply-replacements: Not enough positional command line arguments 
specified!
  Must specify at least 1 positional argument: See: 
bin/clang-apply-replacements -help
  
  $ bin/clang-apply-replacements -help
  USAGE: clang-apply-replacements [options] 
  
  
  
  $ bin/clang-change-namespace -h
  clang-change-namespace: for the -old_namespace option: must be specified at 
least once!
  clang-change-namespace: for the -new_namespace option: must be specified at 
least once!
  clang-change-namespace: for the -file_pattern option: must be specified at 
least once!
  LLVM ERROR: CommonOptionsParser: failed to parse command-line arguments. 
[CommonOptionsParser]: clang-change-namespace: Not enough positional command 
line arguments specified!
  Must specify at least 1 positional argument: See: bin/clang-change-namespace 
-help
  
  $ bin/clang-change-namespace -help
  USAGE: clang-change-namespace [options]  [... ]
  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

Okay so I would suggest to go ahead and commit this. Rebased it succeeds 
without modification.

Still leaves the open problems with the redecls. Should I add the failing test 
from https://reviews.llvm.org/D46421#1375147 in the same commit marked as 
expected to fail? Or what is the procedure here?


Repository:
  rC Clang

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

https://reviews.llvm.org/D46421



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

[PATCH] D59408: [clang-format] [PR25010] Extend AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:411
   * ``SIS_Always`` (in configuration: ``Always``)
-Allow short if statements even if the else is a compound statement.
+Allow short if/else if/else statements when they are not compound 
statements.
 

nit: I prefer the term `if statement` instead of `if/else statement`.



Comment at: clang/include/clang/Format/Format.h:263
 SIS_WithoutElse,
-/// Always put short ifs on the same line if
-/// the else is not a compound statement or not.
+/// If Else statements have no braces don't put them
+/// on the same line.

MyDeveloperDay wrote:
> klimek wrote:
> > This seems weird - why would we want this?
> This is a compatiblity case for the previous "true" as it was before
> 
> Before 
> 
> 
> ```
> if (x) 
>return 1;
> else
>return 2;
> ```
> 
> would be transformed to
> 
> 
> ```
> if (x) return 1;
> else
>return 2;
> ```
> 
> but
> 
> 
> ```
> if (x) 
>return 1;
> else {
>return 2;
> }
> ```
> 
> would not be transformed at all
> 
ditto nit: replace `If Else statements` with `If statements`.



Comment at: clang/include/clang/Format/Format.h:272
+/// Always put short if/else/else if on the same line if
+/// the not compound statement.
 /// \code

this sentence does not parse. Consider rewriting.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:368
+  return (Style.AllowShortIfStatementsOnASingleLine >=
+  FormatStyle::SIS_AlwaysNoElse)
+ ? tryMergeSimpleControlStatement(I, E, Limit)

nit: this comparison is brittle; consider enumerating the states for which this 
applies explicitly.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:422
+if (Line.Last->isNot(tok::r_paren) &&
+Style.AllowShortIfStatementsOnASingleLine < FormatStyle::SIS_Always)
   return 0;

nit: this comparison is brittle; consider enumerating the states for which this 
applies explicitly.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:430
 // Only inline simple if's (no nested if or else), unless specified
-if (Style.AllowShortIfStatementsOnASingleLine != FormatStyle::SIS_Always) {
+if (Style.AllowShortIfStatementsOnASingleLine <
+FormatStyle::SIS_AlwaysNoElse) {

nit: this comparison is brittle; consider enumerating the states for which this 
applies explicitly.

Or extract a function that provides this functionality here and in other places.



Comment at: clang/unittests/Format/FormatTest.cpp:520
+  verifyFormat("if (a) f();\n"
+   "else if (b) g();\n",
+   AllowsMergedIf);

I don't understand why is this preferred over:
```
if (a) f();
else
  if (b) g();
```

Isn't syntactically the nested `if (b) g();` a compound statement? I might be 
wrong.


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

https://reviews.llvm.org/D59408



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

[PATCH] D59408: [clang-format] [PR25010] Extend AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

Generally I'm against introducing new style flags or flag options unless a 
supported style requires it.
IMO this increases maintenance burden and can quickly lead to a big space of 
style flags that contain tricky incompatibilities.


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

https://reviews.llvm.org/D59408



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

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Tooling/Core/Diagnostic.h:67-71
+  // Get the chosen fix to apply for this diagnostic.
+  // FIXME: currently we choose the first non-empty fix, extend it to support
+  // fix selection.
+  const llvm::StringMap *getChosenFix() const;
+

alexfh wrote:
> hokein wrote:
> > alexfh wrote:
> > > Do we actually need this method here? This whole structure is sort of a 
> > > data-only serialization helper, and this method is adding some 
> > > (arbitrary) logic into it.
> > We have a few places using this method, or we could move this method out of 
> > this structure?
> I'd move it out closer to the code which uses it and call it 
> "selectFirstFix", for example (the name should be a verb and it should be 
> less vague about what the function is doing).
hmm, this function is used in `ApplyReplacements.cpp`, `ClangTidy.cpp`, 
`ClangTidyTest.h`, `ClangTidyDiagnosticConsumer.cpp`.

`Diagnostic.h` seems the most suitable place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59932



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

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

I believe there is no such thing as an "short else statement". The `else` is 
part of the `if` statement and if it is present, I don't consider the whole 
`if` statement short. As such, IMO the bug is invalid.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59087



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

[PATCH] D60269: [LibTooling] Add "SourceCode" library for functions relating to source-code manipulation.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 193861.
ymandel added a comment.

Delete unused function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60269

Files:
  clang/include/clang/Tooling/FixIt.h
  clang/include/clang/Tooling/Refactoring/SourceCode.h
  clang/lib/Tooling/FixIt.cpp
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/SourceCode.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/FixItTest.cpp
  clang/unittests/Tooling/SourceCodeTest.cpp

Index: clang/unittests/Tooling/SourceCodeTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/SourceCodeTest.cpp
@@ -0,0 +1,97 @@
+//===- unittest/Tooling/SourceCodeTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestVisitor.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Tooling/Refactoring/SourceCode.h"
+
+using namespace clang;
+
+using tooling::getText;
+using tooling::getExtendedText;
+
+namespace {
+
+struct CallsVisitor : TestVisitor {
+  bool VisitCallExpr(CallExpr *Expr) {
+OnCall(Expr, Context);
+return true;
+  }
+
+  std::function OnCall;
+};
+
+TEST(SourceCodeTest, getText) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("foo(x, y)", getText(*CE, *Context));
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("APPLY(foo, x, y)", getText(*CE, *Context));
+  };
+  Visitor.runOver("#define APPLY(f, x, y) f(x, y)\n"
+  "void foo(int x, int y) { APPLY(foo, x, y); }");
+}
+
+TEST(SourceCodeTest, getTextWithMacro) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("F OO", getText(*CE, *Context));
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("", getText(*P0, *Context));
+EXPECT_EQ("", getText(*P1, *Context));
+  };
+  Visitor.runOver("#define F foo(\n"
+  "#define OO x, y)\n"
+  "void foo(int x, int y) { F OO ; }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("", getText(*CE, *Context));
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", getText(*P0, *Context));
+EXPECT_EQ("y", getText(*P1, *Context));
+  };
+  Visitor.runOver("#define FOO(x, y) (void)x; (void)y; foo(x, y);\n"
+  "void foo(int x, int y) { FOO(x,y) }");
+}
+
+TEST(SourceCodeTest, getExtendedText) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("foo(x, y);",
+  getExtendedText(*CE, tok::TokenKind::semi, *Context));
+
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", getExtendedText(*P0, tok::TokenKind::semi, *Context));
+EXPECT_EQ("x,", getExtendedText(*P0, tok::TokenKind::comma, *Context));
+EXPECT_EQ("y", getExtendedText(*P1, tok::TokenKind::semi, *Context));
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+  Visitor.runOver("void foo(int x, int y) { if (true) foo(x, y); }");
+  Visitor.runOver("int foo(int x, int y) { if (true) return 3 + foo(x, y); }");
+  Visitor.runOver("void foo(int x, int y) { for (foo(x, y);;) ++x; }");
+  Visitor.runOver(
+  "bool foo(int x, int y) { for (;foo(x, y);) x = 1; return true; }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("foo()", getExtendedText(*CE, tok::TokenKind::semi, *Context));
+  };
+  Visitor.runOver("bool foo() { if (foo()) return true; return false; }");
+  Visitor.runOver("void foo() { int x; for (;; foo()) ++x; }");
+  Visitor.runOver("int foo() { return foo() + 3; }");
+}
+
+} // end anonymous namespace
Index: clang/unittests/Tooling/FixItTest.cpp
===
--- clang/unittests/Tooling/FixItTest.cpp
+++ clang/unittests/Tooling/FixItTest.cpp
@@ -13,7 +13,6 @@
 using namespace clang;
 
 using tooling::fixit::getText;
-using tooling::fixit::getExtendedText;
 using tooling::fixit::createRemoval;
 using tooling::fixit::createReplacement;
 
@@ -78,34 +77,6 @@
   "void foo(int x, int y) { FOO(x,y) }");
 }
 
-TEST(FixItTest, getExtendedText) {
-  CallsVisitor Visitor;
-
-  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
-EXPECT_EQ("foo(x, y);",
-  getExtendedText(*CE, tok::TokenKind::semi, *Context));
-
-Expr *P0 = CE->getArg(0);
-Expr *P1 = CE->getArg(1);
-EXPECT_EQ("x", getExtendedText(*P0, 

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-type-mismatch check

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Looks like this check would fit better into the `bugprone` module.


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

https://reviews.llvm.org/D60139



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

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang/include/clang/Tooling/Core/Diagnostic.h:67-71
+  // Get the chosen fix to apply for this diagnostic.
+  // FIXME: currently we choose the first non-empty fix, extend it to support
+  // fix selection.
+  const llvm::StringMap *getChosenFix() const;
+

hokein wrote:
> alexfh wrote:
> > Do we actually need this method here? This whole structure is sort of a 
> > data-only serialization helper, and this method is adding some (arbitrary) 
> > logic into it.
> We have a few places using this method, or we could move this method out of 
> this structure?
I'd move it out closer to the code which uses it and call it "selectFirstFix", 
for example (the name should be a verb and it should be less vague about what 
the function is doing).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59932



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

[PATCH] D59746: [LibTooling] Fix '-h' option

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Can you give a specific example of how this problem manifests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D59135: Add check for matching HeaderFilter before emitting Diagnostic

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:454
+  StringRef FileName(Loc.printToString(Loc.getManager()));
+  if(getHeaderFilter()->match(FileName))
+  {

thorsten-klein wrote:
> alexfh wrote:
> > aaron.ballman wrote:
> > > Formatting is incorrect here -- you should run the patch through 
> > > clang-format 
> > > (https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting),
> > >  and elide the braces.
> > I believe, this will break the handling of notes. If the notes attached to 
> > the diagnostic are in the "interesting" part of the code (see the 
> > checkFilters method below), the whole diagnostic should be treated as such.
> Hello,
> Unfortunately the emitDiagnostic method is called also for files, which 
> should be actually excluded (since the FileName does not match the 
> HeaderFilter regex). This results in diagnostic output, also if there should 
> not be any. For this purpose I added this check here at this point. If the 
> FileName is matching the HeaderFilter regex, then all diagnostic is emitted 
> as before. 
> 
> Did you know about any issue regarding this? If this is not a working 
> solution for you: do you have any other proposal?
> 
> PS: I have also read about this issue in another project: 
> https://fuchsia.googlesource.com/peridot/+/HEAD/docs/lint.md
> --> They say that they disable some clang-tidy checks as workaround
Could you provide a specific example, where you see a problem? Ideally, an 
isolated set of files + clang-tidy arguments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:454
+  StringRef FileName(Loc.printToString(Loc.getManager()));
+  if(getHeaderFilter()->match(FileName))
+  {

alexfh wrote:
> thorsten-klein wrote:
> > alexfh wrote:
> > > aaron.ballman wrote:
> > > > Formatting is incorrect here -- you should run the patch through 
> > > > clang-format 
> > > > (https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting),
> > > >  and elide the braces.
> > > I believe, this will break the handling of notes. If the notes attached 
> > > to the diagnostic are in the "interesting" part of the code (see the 
> > > checkFilters method below), the whole diagnostic should be treated as 
> > > such.
> > Hello,
> > Unfortunately the emitDiagnostic method is called also for files, which 
> > should be actually excluded (since the FileName does not match the 
> > HeaderFilter regex). This results in diagnostic output, also if there 
> > should not be any. For this purpose I added this check here at this point. 
> > If the FileName is matching the HeaderFilter regex, then all diagnostic is 
> > emitted as before. 
> > 
> > Did you know about any issue regarding this? If this is not a working 
> > solution for you: do you have any other proposal?
> > 
> > PS: I have also read about this issue in another project: 
> > https://fuchsia.googlesource.com/peridot/+/HEAD/docs/lint.md
> > --> They say that they disable some clang-tidy checks as workaround
> Could you provide a specific example, where you see a problem? Ideally, an 
> isolated set of files + clang-tidy arguments.
> Unfortunately the emitDiagnostic method is called also for files, which 
> should be actually excluded (since the FileName does not match the 
> HeaderFilter regex).

As per my initial comment, this is done intentionally to make it possible for a 
note issued in an "interesting" part of the code to make the whole diagnostic 
"interesting". Here's a motivating example:

  $ cat a.cc
  #include "b.h"
  $ cat b.h 
  #include "c.h"
  inline void b() { c(/*y=*/42); }
  $ cat c.h
  void c(int x);

If we're only interested in the main file, we don't get any diagnostic:

  $ clang-tidy -checks=-*,bugprone-argument-comment a.cc --
  1 warning generated.
  Suppressed 1 warnings (1 in non-user code).
  Use -header-filter=.* to display errors from all non-system headers. Use 
-system-headers to display errors from system headers as well.

Now if we want diagnostics for `c.h`, we will also get all diagnostics that are 
(or have notes in) `c.h`:

  $ clang-tidy -checks=-*,bugprone-argument-comment a.cc -header-filter=c.h --
  1 warning generated.
  b.h:2:21: warning: argument name 'y' in comment does not match parameter name 
'x' [bugprone-argument-comment]
  inline void b() { c(/*y=*/42); }
  ^~
  /*x=*/
  c.h:1:12: note: 'x' declared here
  void c(int x);
 ^

The fact that we have a note in the "interesting" part of code (be it a header 
matching -header-filter, or a location matching -line-filter) means that the 
diagnostic is somehow related to the "interesting" code. It may well be caused 
by a change in the "interesting" code (e.g. if we changed the parameter name of 
the function `c` 

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang/include/clang/Tooling/Core/Diagnostic.h:67-71
+  // Get the chosen fix to apply for this diagnostic.
+  // FIXME: currently we choose the first non-empty fix, extend it to support
+  // fix selection.
+  const llvm::StringMap *getChosenFix() const;
+

alexfh wrote:
> Do we actually need this method here? This whole structure is sort of a 
> data-only serialization helper, and this method is adding some (arbitrary) 
> logic into it.
We have a few places using this method, or we could move this method out of 
this structure?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59932



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

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang/include/clang/Tooling/Core/Diagnostic.h:67-71
+  // Get the chosen fix to apply for this diagnostic.
+  // FIXME: currently we choose the first non-empty fix, extend it to support
+  // fix selection.
+  const llvm::StringMap *getChosenFix() const;
+

Do we actually need this method here? This whole structure is sort of a 
data-only serialization helper, and this method is adding some (arbitrary) 
logic into it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59932



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

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 193857.
hokein added a comment.

Remove a stale comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59932

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file1.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file2.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file1.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file2.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file3.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/file1.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/format/no.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/format/yes.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file1.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file2.yaml
  
clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file1.yaml
  
clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file2.yaml
  clang-tools-extra/test/clang-tidy/export-diagnostics.cpp
  clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
  clang/include/clang/Tooling/Core/Diagnostic.h
  clang/include/clang/Tooling/DiagnosticsYaml.h
  clang/lib/Tooling/Core/Diagnostic.cpp
  clang/unittests/Tooling/DiagnosticsYamlTest.cpp

Index: clang/unittests/Tooling/DiagnosticsYamlTest.cpp
===
--- clang/unittests/Tooling/DiagnosticsYamlTest.cpp
+++ clang/unittests/Tooling/DiagnosticsYamlTest.cpp
@@ -20,11 +20,13 @@
 using clang::tooling::Diagnostic;
 
 static DiagnosticMessage makeMessage(const std::string , int FileOffset,
- const std::string ) {
+ const std::string ,
+ const StringMap ) {
   DiagnosticMessage DiagMessage;
   DiagMessage.Message = Message;
   DiagMessage.FileOffset = FileOffset;
   DiagMessage.FilePath = FilePath;
+  DiagMessage.Fix = Fix;
   return DiagMessage;
 }
 
@@ -32,10 +34,52 @@
  const std::string , int FileOffset,
  const std::string ,
  const StringMap ) {
-  return Diagnostic(DiagnosticName, makeMessage(Message, FileOffset, FilePath),
-Fix, {}, Diagnostic::Warning, "path/to/build/directory");
+  return Diagnostic(DiagnosticName,
+makeMessage(Message, FileOffset, FilePath, Fix), {},
+Diagnostic::Warning, "path/to/build/directory");
 }
 
+static const char *YAMLContent =
+"---\n"
+"MainSourceFile:  'path/to/source.cpp'\n"
+"Diagnostics: \n"
+"  - DiagnosticName:  'diagnostic#1\'\n"
+"DiagnosticMessage: \n"
+"  Message: 'message #1'\n"
+"  FilePath:'path/to/source.cpp'\n"
+"  FileOffset:  55\n"
+"  Replacements:\n"
+"- FilePath:'path/to/source.cpp'\n"
+"  Offset:  100\n"
+"  Length:  12\n"
+"  ReplacementText: 'replacement #1'\n"
+"  - DiagnosticName:  'diagnostic#2'\n"
+"DiagnosticMessage: \n"
+"  Message: 'message #2'\n"
+"  FilePath:'path/to/header.h'\n"
+"  FileOffset:  60\n"
+"  Replacements:\n"
+"- FilePath:'path/to/header.h'\n"
+"  Offset:  62\n"
+"  Length:  2\n"
+"  ReplacementText: 'replacement #2'\n"
+"  - DiagnosticName:  'diagnostic#3'\n"
+"DiagnosticMessage: \n"
+"  Message: 'message #3'\n"
+"  FilePath:'path/to/source2.cpp'\n"
+"  FileOffset:  72\n"
+"  Replacements:[]\n"
+"Notes:   \n"
+"  - Message: Note1\n"
+"FilePath:'path/to/note1.cpp'\n"
+"FileOffset:  88\n"
+"Replacements:[]\n"
+"  - Message: Note2\n"
+"FilePath:'path/to/note2.cpp'\n"
+"FileOffset:  99\n"
+"Replacements:[]\n"
+"...\n";
+
 TEST(DiagnosticsYamlTest, serializesDiagnostics) {
   TranslationUnitDiagnostics TUD;
   TUD.MainSourceFile = "path/to/source.cpp";
@@ -55,9 +99,9 @@
   TUD.Diagnostics.push_back(makeDiagnostic("diagnostic#3", "message #3", 72,
 

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D59932#1453565 , @alexfh wrote:

> This looks like a more promising direction. Thanks for the readiness to 
> experiment with this.
>
> See the comments inline.


Thanks for the comments. Now all existing tests are passed, the patch is in a 
good shape for review.

There is one missing point -- we don't test the fix-description notes in the 
lit tests, the current test mechanism (CHECK-MESSAGE, CHECK-NOTES) doesn't 
handle it well, we need to feature it out. I think this can be done in a 
separate patch.




Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:130
+
+  // If we have alternative fixes (attached via diagnostic::Notes), we just
+  // choose the first one to apply.

alexfh wrote:
> Could you leave a FIXME here to explore options around interactive fix 
> selection?
Done. I moved this code to `Diagnostic::getChosenFix()` as we have a few places 
using it.



Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:144
+
+  if (ApplyFixes && ErrorFix) {
+for (const auto  : *ErrorFix) {

alexfh wrote:
> The nesting level starts getting out of control here. I'd try to pull the 
> loop into a function.
Agree, but making such refactoring change in this patch will add noise to the 
diff, I tend to make the change in this patch as mini as possible.

I think this can be improved in a follow-up change. 



Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:187
   }
+  reportFix(Diag, Error.Message.Fix);
 }

alexfh wrote:
> Why `Error.Message.Fix`? Should this use `*SelectedFix` instead?
I think we still want to report all the alternative fixes to clients to align 
with clang's behavior. We only use the `SelectedFix` when applying the fix. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59932



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

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Thanks, looks better now, but still a few comments, mostly nits.




Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:701
+  const ASTContext::DynTypedNodeList  = Context->getParents(*DeclRef);
+  if (!Parents.empty()) {
+const Stmt *ST = Parents[0].get();

Please use early return:

  if (Parents.empty())
return false;



Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:702
+  if (!Parents.empty()) {
+const Stmt *ST = Parents[0].get();
+// Ignore ImplicitCastExpr if we find one.

Same here:
  if (!ST)
return false;



Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:710
+}
+if (ST && isa(ST))
+  return true;

And here just `return ST && isa(ST);`



Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:810
 SourceRange Range = DeclRef->getNameInfo().getSourceRange();
-addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
- Result.SourceManager);
-return;
+bool LambdaDeclRef = isLambda(DeclRef, Result.Context);
+if (LambdaDeclRef) {

Let's drop this variable. The code is going to be simpler without it:

  if (isLambda(DeclRef, Result.Context)) {
StringRef CaptureText = ...;
if (CaptureText != "=" && CaptureText != "&") {
  addUsage(...);
  return;
}
  }



Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:812
+if (LambdaDeclRef) {
+  std::string captureText =
+  Lexer::getSourceText(CharSourceRange::getTokenRange(Range),

Lexer::getSourceText returns a StringRef, there's no need to copy its contents.



Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:812
+if (LambdaDeclRef) {
+  std::string captureText =
+  Lexer::getSourceText(CharSourceRange::getTokenRange(Range),

alexfh wrote:
> Lexer::getSourceText returns a StringRef, there's no need to copy its 
> contents.
nit: CaptureText



Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:815
+   *Result.SourceManager, getLangOpts());
+  if (!(captureText == "=" || captureText == "&"))
+LambdaDeclRef = false;

I find it easier to understand, if negation is propagated through the 
parentheses:

  if (captureText != "=" && captureText != "&")




Comment at: test/clang-tidy/readability-identifier-naming.cpp:506
+bool Foo() {
+  bool Columns=false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local 
variable 'Columns'

alexfh wrote:
> If the formatting is not critical for the logic of the test, please 
> clang-format the new test code.
nit: The formatting is still off here, e.g. missing spaces around the equals 
sign.



Comment at: test/clang-tidy/readability-identifier-naming.cpp:508
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local 
variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns=false;
+  auto ptr = [&]{

Let's make the CHECK-FIXES lines unique to reduce the chance of mixing them up 
and to make identifying broken test cases easier. One way to do this is to make 
the variable names distinct, another way is to add unique trailing comments 
both in the code and in the CHECK-FIXES patterns.


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

https://reviews.llvm.org/D59540



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

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 193856.
hokein marked 8 inline comments as done.
hokein added a comment.

Fix apply-replacements, and address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59932

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file1.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file2.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file1.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file2.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file3.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/file1.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/format/no.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/format/yes.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file1.yaml
  clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file2.yaml
  
clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file1.yaml
  
clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file2.yaml
  clang-tools-extra/test/clang-tidy/export-diagnostics.cpp
  clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
  clang/include/clang/Tooling/Core/Diagnostic.h
  clang/include/clang/Tooling/DiagnosticsYaml.h
  clang/lib/Tooling/Core/Diagnostic.cpp
  clang/unittests/Tooling/DiagnosticsYamlTest.cpp

Index: clang/unittests/Tooling/DiagnosticsYamlTest.cpp
===
--- clang/unittests/Tooling/DiagnosticsYamlTest.cpp
+++ clang/unittests/Tooling/DiagnosticsYamlTest.cpp
@@ -20,11 +20,13 @@
 using clang::tooling::Diagnostic;
 
 static DiagnosticMessage makeMessage(const std::string , int FileOffset,
- const std::string ) {
+ const std::string ,
+ const StringMap ) {
   DiagnosticMessage DiagMessage;
   DiagMessage.Message = Message;
   DiagMessage.FileOffset = FileOffset;
   DiagMessage.FilePath = FilePath;
+  DiagMessage.Fix = Fix;
   return DiagMessage;
 }
 
@@ -32,10 +34,52 @@
  const std::string , int FileOffset,
  const std::string ,
  const StringMap ) {
-  return Diagnostic(DiagnosticName, makeMessage(Message, FileOffset, FilePath),
-Fix, {}, Diagnostic::Warning, "path/to/build/directory");
+  return Diagnostic(DiagnosticName,
+makeMessage(Message, FileOffset, FilePath, Fix), {},
+Diagnostic::Warning, "path/to/build/directory");
 }
 
+static const char *YAMLContent =
+"---\n"
+"MainSourceFile:  'path/to/source.cpp'\n"
+"Diagnostics: \n"
+"  - DiagnosticName:  'diagnostic#1\'\n"
+"DiagnosticMessage: \n"
+"  Message: 'message #1'\n"
+"  FilePath:'path/to/source.cpp'\n"
+"  FileOffset:  55\n"
+"  Replacements:\n"
+"- FilePath:'path/to/source.cpp'\n"
+"  Offset:  100\n"
+"  Length:  12\n"
+"  ReplacementText: 'replacement #1'\n"
+"  - DiagnosticName:  'diagnostic#2'\n"
+"DiagnosticMessage: \n"
+"  Message: 'message #2'\n"
+"  FilePath:'path/to/header.h'\n"
+"  FileOffset:  60\n"
+"  Replacements:\n"
+"- FilePath:'path/to/header.h'\n"
+"  Offset:  62\n"
+"  Length:  2\n"
+"  ReplacementText: 'replacement #2'\n"
+"  - DiagnosticName:  'diagnostic#3'\n"
+"DiagnosticMessage: \n"
+"  Message: 'message #3'\n"
+"  FilePath:'path/to/source2.cpp'\n"
+"  FileOffset:  72\n"
+"  Replacements:[]\n"
+"Notes:   \n"
+"  - Message: Note1\n"
+"FilePath:'path/to/note1.cpp'\n"
+"FileOffset:  88\n"
+"Replacements:[]\n"
+"  - Message: Note2\n"
+"FilePath:'path/to/note2.cpp'\n"
+"FileOffset:  99\n"
+"Replacements:[]\n"
+"...\n";
+
 TEST(DiagnosticsYamlTest, serializesDiagnostics) {
   TranslationUnitDiagnostics TUD;
   TUD.MainSourceFile = "path/to/source.cpp";
@@ -55,9 +99,9 @@
   

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D59540#1456011 , @MyDeveloperDay 
wrote:

> friendly ping


Sorry for the delay. Feel free to ping earlier. And more frequently :)


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

https://reviews.llvm.org/D59540



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

[PATCH] D60308: Leave alone the semicolon after the MacroBlockBegin name

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

The documentation examples of MacroBlockBegin are:

  /// A regular expression matching macros that start a block.
  /// \code
  ///# With:
  ///MacroBlockBegin: "^NS_MAP_BEGIN|\
  ///NS_TABLE_HEAD$"
  ///MacroBlockEnd: "^\
  ///NS_MAP_END|\
  ///NS_TABLE_.*_END$"
  ///
  ///NS_MAP_BEGIN
  ///  foo();
  ///NS_MAP_END
  ///
  ///NS_TABLE_HEAD
  ///  bar();
  ///NS_TABLE_FOO_END
  ///
  ///# Without:
  ///NS_MAP_BEGIN
  ///foo();
  ///NS_MAP_END
  ///
  ///NS_TABLE_HEAD
  ///bar();
  ///NS_TABLE_FOO_END
  /// \endcode

It doesn't appear that a semicolon after the macro is expected, so the 
semicolon is treated as an empty statement inside the block.

There are some reasonable arguments for this. semicolons go after 
statement-like things, block introducers are not statement-like in general. 
Even if the particular implementation requires a semicolon (e.g. `glBegin()`), 
it breaks abstraction to make the user type it, it should be part of the macro 
instead.

If this doesn't work for your codebase, can you share some details about the 
macro (what it's for and what it expands to) that motivates this change?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60308



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

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

[Manuel Klimek via Phabricator via cfe-commits]

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

lg




Comment at: clang/lib/Format/TokenAnnotator.cpp:3177-3178
   return false;
 // Don't split tagged template literal so there is a break between the tag
 // identifier and template string.
 if (Left.is(tok::identifier) && Right.is(TT_TemplateString)) {

MyDeveloperDay wrote:
> klimek wrote:
> > I don't understand this yet, which test broke? The TT_TemplateString looks 
> > like it's a JS thing?
> I'm not sure what you mean here?
I don't see the change any more - perhaps the diff was broken? Sorry :(


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

https://reviews.llvm.org/D59309



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

[PATCH] D60046: [python, tests] Disable Clang Python tests on Solaris/SPARC

[Michał Górny via Phabricator via cfe-commits]

mgorny requested changes to this revision.
mgorny added a comment.
This revision now requires changes to proceed.

Just add it to the regex above.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60046



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

[PATCH] D60046: [python, tests] Disable Clang Python tests on Solaris/SPARC

[Rainer Orth via Phabricator via cfe-commits]

ro added a reviewer: mgorny.
ro added a comment.

Ping?  Who's an appropriate reviewer here?  `CODE_OWNERS.TXT` doesn't list 
anyone.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60046



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

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.

One more LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376



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

[PATCH] D60269: [LibTooling] Add "SourceCode" library for functions relating to source-code manipulation.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

Nice! A more specific name and a more specific library.
LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60269



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

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added a comment.

friendly ping


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

https://reviews.llvm.org/D59540



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

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added a comment.

friendly ping


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

https://reviews.llvm.org/D59309



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

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 193839.
DennisL marked an inline comment as done.
DennisL added a comment.

Sync ReleaseNotes.rst with docs


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/PlacementNewTargetTypeMismatch.cpp
  clang-tidy/misc/PlacementNewTargetTypeMismatch.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-placement-new-target-type-mismatch.rst
  test/clang-tidy/misc-placement-new-target-type-mismatch.cpp

Index: test/clang-tidy/misc-placement-new-target-type-mismatch.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-placement-new-target-type-mismatch.cpp
@@ -0,0 +1,82 @@
+// RUN: %check_clang_tidy %s misc-placement-new-target-type-mismatch %t
+
+// definitions
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+void *operator new[](size_type, void *);
+
+namespace std {
+template T* addressof(T& arg) noexcept;
+} // namespace std
+
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+};
+
+template
+T& getT() {
+  static T f;
+  return f;
+}
+
+// instances emitting warnings
+
+void f1() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new of incompatible types [misc-placement-new-target-type-mismatch]
+  delete ptr;
+}
+
+void f2() {
+  int * ptr = new int;
+  new (ptr) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new of incompatible types [misc-placement-new-target-type-mismatch]
+  delete ptr;
+}
+
+void f3() {
+  char *ptr = new char[17*sizeof(char)];
+  new(ptr) float[13];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new of incompatible types [misc-placement-new-target-type-mismatch]
+  delete[] ptr;
+}
+
+void f4() {
+  new (std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new of incompatible types [misc-placement-new-target-type-mismatch]
+}
+
+void f5() {
+  char *ptr = new char[17*sizeof(char)];
+  new(ptr) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new of incompatible types [misc-placement-new-target-type-mismatch]
+  delete[] ptr;
+}
+
+// instances not emitting a warning
+
+void f6() {
+  Foo * ptr = new Foo;
+  new (ptr) Foo;
+  delete ptr;
+}
+
+void f7() {
+  new ((void *)std::addressof(getT())) Foo;
+}
+
+void f8() {
+  char *ptr = new char[17*sizeof(char)];
+  new((float *)ptr) float{13.f};
+
+  delete[] ptr;
+}
\ No newline at end of file
Index: docs/clang-tidy/checks/misc-placement-new-target-type-mismatch.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-placement-new-target-type-mismatch.rst
@@ -0,0 +1,8 @@
+.. title:: clang-tidy - misc-placement-new-target-type-mismatch
+
+misc-placement-new-target-type-mismatch
+===
+
+Finds placement-new calls where the pointer type of the adress mismatches the
+type of the created value.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -184,6 +184,7 @@
misc-new-delete-overloads
misc-non-copyable-objects
misc-non-private-member-variables-in-classes
+   misc-placement-new-target-type-mismatch
misc-redundant-expression
misc-static-assert
misc-throw-by-value-catch-by-reference
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,12 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`misc-placement-new-target-type-mismatch
+  ` check.
+
+  Finds placement-new calls where the pointer type of the adress mismatches the
+  type of the created value.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/misc/PlacementNewTargetTypeMismatch.h
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetTypeMismatch.h
@@ -0,0 +1,36 @@
+//===--- PlacementNewTargetTypeMismatch.h - clang-tidy -*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETTYPEMISMATCHCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETTYPEMISMATCHCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds placement-new calls 

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri resigned from this revision.
lebedev.ri added a comment.

Hmm, i suppose this looks good to me now.
It would be good to early-exit, but i'm not sure how important that is.
Please see D52771  / 
`clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp`
on how to use the bool params to short-circuit the astmatchers.




Comment at: docs/ReleaseNotes.rst:114
+  ` now supports an
+  `IgnoreListInit` option.
+

... which does xyz


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

https://reviews.llvm.org/D55044



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

[PATCH] D60308: Leave alone the semicolon after the MacroBlockBegin name

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: sammccall, klimek, djasper, krasimir.
owenpan added a project: clang.
Herald added a subscriber: cfe-commits.

This patch fixes the bug below.

The code:

  FOO_BEGIN();
FOO_ENTRY
  FOO_END();

is erroneously formatted to:

  FOO_BEGIN()
;
FOO_ENTRY
  FOO_END();


Repository:
  rC Clang

https://reviews.llvm.org/D60308

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3480,6 +3480,9 @@
"  int x;\n"
"  x = 1;\n"
"FOO_END(Baz)", Style);
+  verifyFormat("FOO_BEGIN();\n"
+   "  FOO_ENTRY\n"
+   "FOO_END();", Style);
 }
 
 
//===--===//
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -540,6 +540,9 @@
   if (MacroBlock && FormatTok->is(tok::l_paren))
 parseParens();
 
+  if (MunchSemi && FormatTok->Tok.is(tok::semi))
+nextToken();
+
   size_t NbPreprocessorDirectives =
   CurrentLines ==  ? PreprocessorDirectives.size() : 0;
   addUnwrappedLine();


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3480,6 +3480,9 @@
"  int x;\n"
"  x = 1;\n"
"FOO_END(Baz)", Style);
+  verifyFormat("FOO_BEGIN();\n"
+   "  FOO_ENTRY\n"
+   "FOO_END();", Style);
 }
 
 //===--===//
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -540,6 +540,9 @@
   if (MacroBlock && FormatTok->is(tok::l_paren))
 parseParens();
 
+  if (MunchSemi && FormatTok->Tok.is(tok::semi))
+nextToken();
+
   size_t NbPreprocessorDirectives =
   CurrentLines ==  ? PreprocessorDirectives.size() : 0;
   addUnwrappedLine();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits