[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Yup, this makes sense now! I'll do some nit-picking for a little longer.




Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:839
+// as a 'StmtPoint' so we have to bypass it.
+const bool IsBypass = isa(S);
+

For now this hack is clearly only for CXXNewExpr. And it happens because our 
AST is weird and there's no separate sub-expression dedicated entirely to 
invoking the allocator. Therefore let's give this variable a better name, maybe 
`BypassCXXNewExprEvaluation` or something like that.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:856-860
+  if (Optional CEB = Node->getLocationAs())
+CurrentSFC = CEB->getStackFrame();
+
+  if (Optional CE = Node->getLocationAs())
+CurrentSFC = CE->getStackFrame();

Branches here are unnecessary because there's no performance win here. Let's 
just
```lang=c++
CurrentSFC = Node->getStackFrame();
```



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:862-866
+  // If that is satisfied we found our statement.
+  if (!IsBypass)
+if (Optional SP = Node->getLocationAs())
+  if (SP->getStmt() == S && SP->getStackFrame() == CurrentSFC)
+break;

This code would currently bypass the call site for the conservatively evaluated 
allocator. It doesn't seem to be immediately important but let's add a FIXME.


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

https://reviews.llvm.org/D62926



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

[PATCH] D63170: [clang][NewPM] Fix broken -O0 test from missing assumptions

[Chandler Carruth via Phabricator via cfe-commits]

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

LGTM. Bit annoying that we need to do this at O0, but fine...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63170



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

[PATCH] D63168: [clang][NewPM] Fix split debug test

[Chandler Carruth via Phabricator via cfe-commits]

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

Code change LGTM, but again, let's update the test to check both ways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63168



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

[PATCH] D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added a comment.

I understand the change to explicitly say `-O2`. I also understand the change 
to add an explicit `-fno-experimental-new-pass-manager` to a `RUN` line when we 
have another `RUN` line that explicitly uses `-fexperiemntal-new-pass-manager`.

But for many of these cases, there is no new-PM `RUN` line in the test. If 
we're going to fix one `RUN` line to a particular pass manager, we should fix 
both IMO unless there is something quite odd about the IR being tested that 
makes the result of optimizinng be weirdly different between the two pass 
managers. That would be somewhat surprising, so I kinda want to know if we 
actually see that so often to understand what is causing this divergence.

In general, Clang tests shouldn't be checking such complex optimizations that 
the differences between the pass managers really manifests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63156



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

[Chandler Carruth via Phabricator via cfe-commits]

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

Let's update the test to explicitly run w/ both PMs to make sure this keeps 
working. LGTM with that change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63155



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

[PATCH] D63174: [clang][NewPM] Add RUNS for tests that produce slightly different IR under new PM

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added a comment.

This really confused me. We shouldn't be seeing this kind of difference in the 
new PM.

But I think I figured it out.

Both PMs have to run *some* inliner at -O0. This is because we need to inline 
`always_inline` functions. The new PM has a *super* simple (and fast 
compile-time wise) inliner designed for this. It's really good at its job. The 
old PM runs the normal inliner. This thing isn't simple at all. It does complex 
stuff to incrementally simplify code while inlining because that can have a 
huge effect on the overall efficiency of the compiler *when doing full 
optimization passes*. But we're not doing that.

The result is that because all of these tests check code that is emitted by 
`always_inline` functions in our builtin headers, and then *inlined* into the 
test functions, the inliner in the old pass manager did a bunch of "cleanup" of 
the code that the new PM doesn't do.

Honestly, I think the new PM's behavior is correct for -O0, but I think it 
makes these tests less easy to understand. I think we should instead change the 
command running here, and when we do I think we will find a set of checks that 
work for both PMs.

When we are willing to do something like run `opt` in addition to clang (the 
`convergent.cl` case), instead of just doing `-instnamer` we should also do 
`-instcombine`. I suspect this will remove the differences.

When we're just directly using `%clang_cc1` I suspect that rather than checking 
the completely unoptimized code we should check with `-O1` to make the IR much 
more stable and easy to inspect. It should also result in identical results 
from the two PMs.




Comment at: clang/test/CodeGenOpenCL/convergent.cl:2
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm %s -o - 
-fno-experimental-new-pass-manager | opt -instnamer -S | FileCheck 
-enable-var-scope %s --check-prefixes=CHECK,CHECK-LEGACY
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm %s -o - 
-fexperimental-new-pass-manager | opt -instnamer -S | FileCheck 
-enable-var-scope %s --check-prefixes=CHECK,CHECK-NEWPM
 

Probably want to use `opt -passes=instnamer` or some such to use the new PM in 
the `opt` invocation as well...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63174



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

[PATCH] D63153: [clang][NewPM] Fix broken -O0 test from the AlwaysInliner

[Chandler Carruth via Phabricator via cfe-commits]

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

Code change LGTM. Can you update at least one of the tests to explicitly run 
both PMs so that we'll notice if this breaks in some weird way? Feel free to 
submit with that change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63153



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

[PATCH] D62888: [NewPM] Port Sancov

[Chandler Carruth via Phabricator via cfe-commits]

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

This was a lot easier for me to understand too, thanks. Somewhat minor code 
changes below.




Comment at: clang/lib/CodeGen/BackendUtil.cpp:1231-1232
+  MPM.addPass(ModuleSanitizerCoveragePass(SancovOpts));
+  MPM.addPass(
+  
createModuleToFunctionPassAdaptor(SanitizerCoveragePass(SancovOpts)));
+}

Is there an existing function pass pipeline we can put this in, maybe by using 
an extension point? That would make it much faster to run I suspect.



Comment at: llvm/include/llvm/Transforms/Instrumentation/SanitizerCoverage.h:5-6
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//

Need to use the new file header.



Comment at: llvm/include/llvm/Transforms/Instrumentation/SanitizerCoverage.h:14
+//===--===//
+#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_SANITIZERCOVERAGE_H
+#define LLVM_TRANSFORMS_INSTRUMENTATION_SANITIZERCOVERAGE_H

I feel like we usually have whitespace here too?



Comment at: llvm/include/llvm/Transforms/Instrumentation/SanitizerCoverage.h:24
+
+/// This is a wrapper for SanitizerCoverage usage in the new pass manager. The
+/// pass instruments functions for coverage.

Not really a wrapper, just the pass itself...

`SanitizerCoverage` is an implementation detail that the reader here doesn't 
really know about.



Comment at: llvm/include/llvm/Transforms/Instrumentation/SanitizerCoverage.h:37
+
+/// This is a wrapper for the ModuleSanitizerCoverage. This adds initialization
+/// calls to the module for trace PC guards and 8bit counters if they are

Same comment as above.



Comment at: llvm/lib/Passes/PassRegistry.def:248
 FUNCTION_PASS("tsan", ThreadSanitizerPass())
+FUNCTION_PASS("sancov-func", SanitizerCoveragePass())
 #undef FUNCTION_PASS

Consistency would suggest just `sancov` here? I don't feel strongly though.



Comment at: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp:228-229
+  : Options(OverrideFromCL(Options)) {
+initializeModuleSanitizerCoverageLegacyPassPass(
+*PassRegistry::getPassRegistry());
   }

This should be in the legacy pass below? (actually, I tihnk it already is, so 
this can likely be removed)



Comment at: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp:262-269
+bool ShouldEmitInitCalls = false;
+for (const Function  : M) {
+  if (canInstrumentWithSancov(F)) {
+ShouldEmitInitCalls = true;
+break;
+  }
+}

```
if (!llvm::any_of(M, [](const Function ) { return can 
InstrumentWithSancov(F); }))
  return false;
```



Comment at: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp:442
+  const PostDominatorTree *PDT = (F);
+  SanitizerCoverage Sancov(*F.getParent(), Options);
+  if (Sancov.instrumentFunction(F, DT, PDT))

I completely misread this the first time through thinking you used a common 
object for state

Maybe change the constructor to accept a function to make it more obvious that 
this is intended to be built per-function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62888



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

[PATCH] D62839: [clangd] Index API and implementations for relations

[Nathan Ridge via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/index/Index.h:77
+struct RelationsRequest {
+  SymbolID Subject;
+  index::SymbolRole Predicate;

kadircet wrote:
> sorry for missing it in previous iteration. but this should also enable batch 
> queries. let's make this one a `llvm::DenseMap`.
> And in the callback we should output both the `SymbolID` of the `Subject` and 
> `Object`.
> 
> We have a network based index(internally) and it uses this batch query 
> optimization to get rid of network latency.
What is an example use case for a batch query? In a typical LSP editor, the 
user can only request a type hierarchy for one class at a time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62839



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

[PATCH] D63164: [HIP] Add option to force lambda nameing following ODR in HIP/CUDA.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

ping for comment as one of HIP-based workload is blocked by this issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63164



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

r363220 - Revert r361811: 'Re-commit r357452 (take 2): "SimplifyCFG SinkCommonCodeFromPredecessors ...'

[David L. Jones via cfe-commits]

Author: dlj
Date: Wed Jun 12 19:04:45 2019
New Revision: 363220

URL: http://llvm.org/viewvc/llvm-project?rev=363220=rev
Log:
Revert r361811: 'Re-commit r357452 (take 2): "SimplifyCFG 
SinkCommonCodeFromPredecessors ...'

We have observed some failures with internal builds with this revision.

- Performance regressions:
  - llvm's SingleSource/Misc evalloop shows performance regressions (although 
these may be red herrings).
  - Benchmarks for Abseil's SwissTable.
- Correctness:
  - Failures for particular libicu tests when building the Google AppEngine SDK 
(for PHP).

hwennborg has already been notified, and is aware of reproducer failures.


Modified:
cfe/trunk/test/CodeGenCXX/nrvo.cpp
cfe/trunk/test/CodeGenCXX/stack-reuse-exceptions.cpp
cfe/trunk/test/CodeGenObjC/exceptions.m

Modified: cfe/trunk/test/CodeGenCXX/nrvo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/nrvo.cpp?rev=363220=363219=363220=diff
==
--- cfe/trunk/test/CodeGenCXX/nrvo.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/nrvo.cpp Wed Jun 12 19:04:45 2019
@@ -60,6 +60,7 @@ X test2(bool B) {
   // CHECK-NEXT: call void @llvm.lifetime.start
   // CHECK-NEXT: call {{.*}} @_ZN1XC1Ev
   // CHECK: call {{.*}} @_ZN1XC1ERKS_
+  // CHECK: call {{.*}} @_ZN1XC1ERKS_
   // CHECK: call {{.*}} @_ZN1XD1Ev
   // CHECK-NEXT: call void @llvm.lifetime.end
   // CHECK: call {{.*}} @_ZN1XD1Ev

Modified: cfe/trunk/test/CodeGenCXX/stack-reuse-exceptions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/stack-reuse-exceptions.cpp?rev=363220=363219=363220=diff
==
--- cfe/trunk/test/CodeGenCXX/stack-reuse-exceptions.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/stack-reuse-exceptions.cpp Wed Jun 12 19:04:45 
2019
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -o - -emit-llvm -O1 \
-// RUN: -fexceptions -fcxx-exceptions -mllvm 
-simplifycfg-sink-common=false | FileCheck %s
+// RUN: -fexceptions -fcxx-exceptions | FileCheck %s
 //
 // We should emit lifetime.ends for these temporaries in both the 'exception'
 // and 'normal' paths in functions.

Modified: cfe/trunk/test/CodeGenObjC/exceptions.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/exceptions.m?rev=363220=363219=363220=diff
==
--- cfe/trunk/test/CodeGenObjC/exceptions.m (original)
+++ cfe/trunk/test/CodeGenObjC/exceptions.m Wed Jun 12 19:04:45 2019
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 
-fobjc-runtime=macosx-fragile-10.5 -emit-llvm -fobjc-exceptions -mllvm 
-simplifycfg-sink-common=false -O2 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 
-fobjc-runtime=macosx-fragile-10.5 -emit-llvm -fobjc-exceptions -O2 -o - %s | 
FileCheck %s
 //
 //  [irgen] [eh] Exception code built with clang 
(x86_64) crashes
 


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

[PATCH] D62988: Add an attribute to allow fields of non-trivial types in C unions

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

John and I had a discussion offline and decided that we should not pursue the 
approach taken in this patch. I'll try to work on a patch that follows the 
C++11 approach when I have time later.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62988



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

[PATCH] D63161: Devirtualize destructor of final class.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi marked 2 inline comments as done.
yamauchi added inline comments.



Comment at: lib/CodeGen/CGExprCXX.cpp:1867-1868
 
-  if (Dtor->isVirtual()) {
+  if (Dtor->isVirtual() &&
+  !(Dtor->hasAttr() || RD->hasAttr())) {
 CGF.CGM.getCXXABI().emitVirtualObjectDelete(CGF, DE, Ptr, ElementType,

rsmith wrote:
> Can we use `getDevirtualizedMethod` here instead? That catches a few other 
> cases that just checking for `final` will miss, and disables this for cases 
> where it will currently do the wrong thing (eg, `-fapple-kext`). See 
> `EmitCXXMemberOrOperatorMemberCallExpr` for where we do this for other 
> virtual calls.
> 
> As a particularly terrible example:
> 
> ```
> struct A { virtual ~A() final = 0; };
> void evil(A *p) { delete p; }
> ```
> 
> is (amazingly) valid, and must not be devirtualized because there is no 
> requirement that the destructor of `A` have a definition in this program. (It 
> would, however, be correct to optimize out the entire `delete` expression in 
> this case, on the basis that `p` must always be `nullptr` whenever it's 
> reached. But that doesn't really seem worthwhile.)
Updated. Hopefully this does the right thing.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63161



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

[PATCH] D63161: Devirtualize destructor of final class.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi updated this revision to Diff 204365.
yamauchi added a comment.

Using getDevirtualizedMethod.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63161

Files:
  lib/CodeGen/CGExprCXX.cpp
  test/CodeGenCXX/devirtualize-dtor-final.cpp


Index: test/CodeGenCXX/devirtualize-dtor-final.cpp
===
--- /dev/null
+++ test/CodeGenCXX/devirtualize-dtor-final.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -std=c++11 %s -emit-llvm -o - 
| FileCheck %s
+
+namespace Test1 {
+  struct A { virtual ~A() {} };
+  struct B final : public A {};
+  struct C : public A { virtual ~C() final {} };
+  // CHECK-LABEL: define void @_ZN5Test13fooEPNS_1BE
+  void foo(B *b) {
+// CHECK: call void @_ZN5Test11BD1Ev
+delete b;
+  }
+  // CHECK-LABEL: define void @_ZN5Test14foo2EPNS_1CE
+  void foo2(C *c) {
+// CHECK: call void @_ZN5Test11CD1Ev
+delete c;
+  }
+}
Index: lib/CodeGen/CGExprCXX.cpp
===
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -1865,9 +1865,15 @@
   Dtor = RD->getDestructor();
 
   if (Dtor->isVirtual()) {
-CGF.CGM.getCXXABI().emitVirtualObjectDelete(CGF, DE, Ptr, ElementType,
-Dtor);
-return;
+if (auto *DevirtualizedDtor = dyn_cast_or_null(
+Dtor->getDevirtualizedMethod(DE->getArgument(),
+ CGF.CGM.getLangOpts().AppleKext)))
+  Dtor = DevirtualizedDtor;
+else {
+  CGF.CGM.getCXXABI().emitVirtualObjectDelete(CGF, DE, Ptr, 
ElementType,
+  Dtor);
+  return;
+}
   }
 }
   }


Index: test/CodeGenCXX/devirtualize-dtor-final.cpp
===
--- /dev/null
+++ test/CodeGenCXX/devirtualize-dtor-final.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -std=c++11 %s -emit-llvm -o - | FileCheck %s
+
+namespace Test1 {
+  struct A { virtual ~A() {} };
+  struct B final : public A {};
+  struct C : public A { virtual ~C() final {} };
+  // CHECK-LABEL: define void @_ZN5Test13fooEPNS_1BE
+  void foo(B *b) {
+// CHECK: call void @_ZN5Test11BD1Ev
+delete b;
+  }
+  // CHECK-LABEL: define void @_ZN5Test14foo2EPNS_1CE
+  void foo2(C *c) {
+// CHECK: call void @_ZN5Test11CD1Ev
+delete c;
+  }
+}
Index: lib/CodeGen/CGExprCXX.cpp
===
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -1865,9 +1865,15 @@
   Dtor = RD->getDestructor();
 
   if (Dtor->isVirtual()) {
-CGF.CGM.getCXXABI().emitVirtualObjectDelete(CGF, DE, Ptr, ElementType,
-Dtor);
-return;
+if (auto *DevirtualizedDtor = dyn_cast_or_null(
+Dtor->getDevirtualizedMethod(DE->getArgument(),
+ CGF.CGM.getLangOpts().AppleKext)))
+  Dtor = DevirtualizedDtor;
+else {
+  CGF.CGM.getCXXABI().emitVirtualObjectDelete(CGF, DE, Ptr, ElementType,
+  Dtor);
+  return;
+}
   }
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r363208 - [clang-scan-deps] Include in ClangScanDeps.cpp to ensure it

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Jun 12 14:52:36 2019
New Revision: 363208

URL: http://llvm.org/viewvc/llvm-project?rev=363208=rev
Log:
[clang-scan-deps] Include  in ClangScanDeps.cpp to ensure it
builds on all platforms

Modified:
cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp

Modified: cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp?rev=363208=363207=363208=diff
==
--- cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp (original)
+++ cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp Wed Jun 12 14:52:36 2019
@@ -22,6 +22,7 @@
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/Threading.h"
+#include 
 #include 
 
 using namespace clang;


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

Re: r363204 - [clang-scan-deps] initial outline of the tool that runs preprocessor to find

[Alex L via cfe-commits]

On Wed, 12 Jun 2019 at 14:29, Alex Lorenz via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: arphaman
> Date: Wed Jun 12 14:32:49 2019
> New Revision: 363204
>
> URL: http://llvm.org/viewvc/llvm-project?rev=363204=rev
> Log:
> [clang-scan-deps] initial outline of the tool that runs preprocessor to
> find
> dependencies over a JSON compilation database
>
> This commit introduces an outline for the clang-scan-deps tool that will be
> used to implement fast dependency discovery phase using implicit modules
> for
> explicit module builds.
>
> The initial version of the tool works by computing non-modular header
> dependencies
> for files in the compilation database without any optimizations
> (i.e. without source minimization from r362459).
> The tool spawns a number of worker threads to run the clang compiler
> workers in parallel.
>
> The immediate goal for clang-scan-deps is to create a ClangScanDeps library
> which will be used to build up this tool to use the source minimization and
> caching multi-threaded filesystem to implement the optimized
> non-incremental
> dependency scanning phase for a non-modular build. This will allow us to do
> benchmarks and comparisons for performance that the minimization and
> caching give us
>
> Differential Revision: https://reviews.llvm.org/D60233
>
> Added:
> cfe/trunk/test/ClangScanDeps/
> cfe/trunk/test/ClangScanDeps/Inputs/
> cfe/trunk/test/ClangScanDeps/Inputs/header.h
> cfe/trunk/test/ClangScanDeps/Inputs/header2.h
> cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json
> cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
> cfe/trunk/tools/clang-scan-deps/
> cfe/trunk/tools/clang-scan-deps/CMakeLists.txt
> cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
> Modified:
> cfe/trunk/test/CMakeLists.txt
> cfe/trunk/tools/CMakeLists.txt
>
> Modified: cfe/trunk/test/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CMakeLists.txt?rev=363204=363203=363204=diff
>
> ==
> --- cfe/trunk/test/CMakeLists.txt (original)
> +++ cfe/trunk/test/CMakeLists.txt Wed Jun 12 14:32:49 2019
> @@ -57,6 +57,7 @@ list(APPEND CLANG_TEST_DEPS
>clang-rename
>clang-refactor
>clang-diff
> +  clang-scan-deps
>diagtool
>hmaptool
>)
>
> Added: cfe/trunk/test/ClangScanDeps/Inputs/header.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/Inputs/header.h?rev=363204=auto
>
> ==
> --- cfe/trunk/test/ClangScanDeps/Inputs/header.h (added)
> +++ cfe/trunk/test/ClangScanDeps/Inputs/header.h Wed Jun 12 14:32:49 2019
> @@ -0,0 +1,3 @@
> +#ifdef INCLUDE_HEADER2
> +#include "header2.h"
> +#endif
>
> Added: cfe/trunk/test/ClangScanDeps/Inputs/header2.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/Inputs/header2.h?rev=363204=auto
>
> ==
> --- cfe/trunk/test/ClangScanDeps/Inputs/header2.h (added)
> +++ cfe/trunk/test/ClangScanDeps/Inputs/header2.h Wed Jun 12 14:32:49 2019
> @@ -0,0 +1 @@
> +// header 2.
>
> Added: cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json?rev=363204=auto
>
> ==
> --- cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json (added)
> +++ cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json Wed Jun 12
> 14:32:49 2019
> @@ -0,0 +1,12 @@
> +[
> +{
> +  "directory": "DIR",
> +  "command": "clang -c DIR/regular_cdb.cpp -IInputs -MD -MF
> DIR/regular_cdb.d",
> +  "file": "DIR/regular_cdb.cpp"
> +},
> +{
> +  "directory": "DIR",
> +  "command": "clang -c DIR/regular_cdb.cpp -IInputs -D INCLUDE_HEADER2
> -MD -MF DIR/regular_cdb2.d",
> +  "file": "DIR/regular_cdb.cpp"
> +}
> +]
>
> Added: cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/regular_cdb.cpp?rev=363204=auto
>
> ==
> --- cfe/trunk/test/ClangScanDeps/regular_cdb.cpp (added)
> +++ cfe/trunk/test/ClangScanDeps/regular_cdb.cpp Wed Jun 12 14:32:49 2019
> @@ -0,0 +1,27 @@
> +// RUN: rm -rf %t.dir
> +// RUN: rm -rf %t.cdb
> +// RUN: mkdir -p %t.dir
> +// RUN: cp %s %t.dir/regular_cdb.cpp
> +// RUN: mkdir %t.dir/Inputs
> +// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
> +// RUN: cp %S/Inputs/header2.h %t.dir/Inputs/header2.h
> +// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/regular_cdb.json > %t.cdb
> +//
> +// RUN: clang-scan-deps -compilation-database %t.cdb -j 1
> +// RUN: cat %t.dir/regular_cdb.d | FileCheck %s
> +// RUN: cat %t.dir/regular_cdb2.d | FileCheck --check-prefix=CHECK2 %s
> +// RUN: rm -rf %t.dir/regular_cdb.d %t.dir/regular_cdb2.d
> +//
> +// 

r363207 - NFC, Update the ClangScanDeps.cpp file's license comment

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Jun 12 14:45:28 2019
New Revision: 363207

URL: http://llvm.org/viewvc/llvm-project?rev=363207=rev
Log:
NFC, Update the ClangScanDeps.cpp file's license comment

The file ClangScanDeps.cpp from r363204 had the old outdated LLVM
license comment at the top of the file that I committed by accident.

Modified:
cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp

Modified: cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp?rev=363207=363206=363207=diff
==
--- cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp (original)
+++ cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp Wed Jun 12 14:45:28 2019
@@ -1,9 +1,8 @@
-//===-- ClangScanDeps.cpp - Implementation of clang-scan-deps 
-===//
+//===- ClangScanDeps.cpp - Implementation of clang-scan-deps 
--===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// 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
 //
 
//===--===//
 


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

[PATCH] D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database

[Alex Lorenz via Phabricator via cfe-commits]

arphaman marked 2 inline comments as done.
arphaman added inline comments.



Comment at: clang/test/ClangScanDeps/Inputs/regular_cdb.json:4
+  "directory": "DIR",
+  "command": "clang -c DIR/regular_cdb.cpp -IInputs -MD -MF DIR/regular_cdb.d",
+  "file": "DIR/regular_cdb.cpp"

aganea wrote:
> arphaman wrote:
> > aganea wrote:
> > > Is `-MD -MF` required for clang-scan-deps to work? Can't we append those 
> > > arguments automatically, so that only include paths are needed in the CDB?
> > > 
> > > Would it possible for the tool to produce a collated file? A real-world 
> > > usage would otherwise produce tens of thousands of files. Which then 
> > > would be opened/parsed by the calling application.
> > I agree, the tool shouldn't rely on those options being there. `-MD -MF` in 
> > this test right now a crutch to get the dependencies printed somewhere.
> > 
> > I think the tool by default should print out the collated dependencies to 
> > STDOUT instead of writing them to files that were specified for the build. 
> > The `-MD -MF` options will not be required as well. This implementation 
> > requires some changes in the dependency collector, which I am planning to 
> > do as part of clang-scan-deps in the follow-up patches. Will it be ok to 
> > keep this patch as it is right now, and transition to printing out the 
> > collated dependencies without requiring the options as a follow-up?
> > 
> > 
> Sounds good! Please cc me on the upcoming reviews, we have a good use-case 
> for clang-scan-deps in [[ http://www.fastbuild.org/docs/home.html | Fastbuild 
> ]]!
Sure, I will CC you on the upcoming patches.



Comment at: clang/tools/clang-scan-deps/CMakeLists.txt:3
+Core
+Support
+)

aganea wrote:
> Two space indent.
Oops, I missed this comment. Fixed in r363205.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60233



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

r363205 - NFC, fixup indentation in CMakeLists.txt from r363204 as requested

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Jun 12 14:37:32 2019
New Revision: 363205

URL: http://llvm.org/viewvc/llvm-project?rev=363205=rev
Log:
NFC, fixup indentation in CMakeLists.txt from r363204 as requested
in the review.

I missed that review comment from https://reviews.llvm.org/D60233 in the 
original
commit.

Modified:
cfe/trunk/tools/clang-scan-deps/CMakeLists.txt

Modified: cfe/trunk/tools/clang-scan-deps/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-scan-deps/CMakeLists.txt?rev=363205=363204=363205=diff
==
--- cfe/trunk/tools/clang-scan-deps/CMakeLists.txt (original)
+++ cfe/trunk/tools/clang-scan-deps/CMakeLists.txt Wed Jun 12 14:37:32 2019
@@ -1,7 +1,7 @@
 set(LLVM_LINK_COMPONENTS
-Core
-Support
-)
+  Core
+  Support
+  )
 
 add_clang_tool(clang-scan-deps
   ClangScanDeps.cpp


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

[PATCH] D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363204: [clang-scan-deps] initial outline of the tool that 
runs preprocessor to find (authored by arphaman, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60233?vs=203887=204358#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60233

Files:
  cfe/trunk/test/CMakeLists.txt
  cfe/trunk/test/ClangScanDeps/Inputs/header.h
  cfe/trunk/test/ClangScanDeps/Inputs/header2.h
  cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json
  cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
  cfe/trunk/tools/CMakeLists.txt
  cfe/trunk/tools/clang-scan-deps/CMakeLists.txt
  cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp

Index: cfe/trunk/tools/CMakeLists.txt
===
--- cfe/trunk/tools/CMakeLists.txt
+++ cfe/trunk/tools/CMakeLists.txt
@@ -8,6 +8,7 @@
 add_clang_subdirectory(clang-fuzzer)
 add_clang_subdirectory(clang-import-test)
 add_clang_subdirectory(clang-offload-bundler)
+add_clang_subdirectory(clang-scan-deps)
 
 add_clang_subdirectory(c-index-test)
 
Index: cfe/trunk/tools/clang-scan-deps/CMakeLists.txt
===
--- cfe/trunk/tools/clang-scan-deps/CMakeLists.txt
+++ cfe/trunk/tools/clang-scan-deps/CMakeLists.txt
@@ -0,0 +1,26 @@
+set(LLVM_LINK_COMPONENTS
+Core
+Support
+)
+
+add_clang_tool(clang-scan-deps
+  ClangScanDeps.cpp
+  )
+
+set(CLANG_SCAN_DEPS_LIB_DEPS
+  clangAST
+  clangBasic
+  clangCodeGen
+  clangDriver
+  clangFrontend
+  clangFrontendTool
+  clangLex
+  clangParse
+  clangTooling
+  )
+
+target_link_libraries(clang-scan-deps
+  PRIVATE
+  ${CLANG_SCAN_DEPS_LIB_DEPS}
+  )
+
Index: cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
+++ cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -0,0 +1,218 @@
+//===-- ClangScanDeps.cpp - Implementation of clang-scan-deps -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/FrontendTool/Utils.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/JSONCompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/Options.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "llvm/Support/Signals.h"
+#include "llvm/Support/Threading.h"
+#include 
+
+using namespace clang;
+
+namespace {
+
+/// A clang tool that runs the preprocessor only for the given compiler
+/// invocation.
+class PreprocessorOnlyTool : public tooling::ToolAction {
+public:
+  PreprocessorOnlyTool(StringRef WorkingDirectory)
+  : WorkingDirectory(WorkingDirectory) {}
+
+  bool runInvocation(std::shared_ptr Invocation,
+ FileManager *FileMgr,
+ std::shared_ptr PCHContainerOps,
+ DiagnosticConsumer *DiagConsumer) override {
+// Create a compiler instance to handle the actual work.
+CompilerInstance Compiler(std::move(PCHContainerOps));
+Compiler.setInvocation(std::move(Invocation));
+FileMgr->getFileSystemOpts().WorkingDir = WorkingDirectory;
+Compiler.setFileManager(FileMgr);
+
+// Create the compiler's actual diagnostics engine.
+Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
+if (!Compiler.hasDiagnostics())
+  return false;
+
+Compiler.createSourceManager(*FileMgr);
+
+auto Action = llvm::make_unique();
+const bool Result = Compiler.ExecuteAction(*Action);
+FileMgr->clearStatCache();
+return Result;
+  }
+
+private:
+  StringRef WorkingDirectory;
+};
+
+/// A proxy file system that doesn't call `chdir` when changing the working
+/// directory of a clang tool.
+class ProxyFileSystemWithoutChdir : public llvm::vfs::ProxyFileSystem {
+public:
+  ProxyFileSystemWithoutChdir(
+  llvm::IntrusiveRefCntPtr FS)
+  : ProxyFileSystem(std::move(FS)) {}
+
+  llvm::ErrorOr getCurrentWorkingDirectory() const override {
+assert(!CWD.empty() && "empty CWD");
+return CWD;
+  }
+
+  std::error_code setCurrentWorkingDirectory(const Twine ) override {
+CWD = Path.str();
+return {};
+  }
+
+private:
+  std::string CWD;
+};
+
+/// The high-level 

r363204 - [clang-scan-deps] initial outline of the tool that runs preprocessor to find

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Jun 12 14:32:49 2019
New Revision: 363204

URL: http://llvm.org/viewvc/llvm-project?rev=363204=rev
Log:
[clang-scan-deps] initial outline of the tool that runs preprocessor to find
dependencies over a JSON compilation database

This commit introduces an outline for the clang-scan-deps tool that will be
used to implement fast dependency discovery phase using implicit modules for
explicit module builds.

The initial version of the tool works by computing non-modular header 
dependencies
for files in the compilation database without any optimizations
(i.e. without source minimization from r362459).
The tool spawns a number of worker threads to run the clang compiler workers in 
parallel.

The immediate goal for clang-scan-deps is to create a ClangScanDeps library
which will be used to build up this tool to use the source minimization and
caching multi-threaded filesystem to implement the optimized non-incremental
dependency scanning phase for a non-modular build. This will allow us to do
benchmarks and comparisons for performance that the minimization and caching 
give us

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

Added:
cfe/trunk/test/ClangScanDeps/
cfe/trunk/test/ClangScanDeps/Inputs/
cfe/trunk/test/ClangScanDeps/Inputs/header.h
cfe/trunk/test/ClangScanDeps/Inputs/header2.h
cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json
cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
cfe/trunk/tools/clang-scan-deps/
cfe/trunk/tools/clang-scan-deps/CMakeLists.txt
cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
Modified:
cfe/trunk/test/CMakeLists.txt
cfe/trunk/tools/CMakeLists.txt

Modified: cfe/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CMakeLists.txt?rev=363204=363203=363204=diff
==
--- cfe/trunk/test/CMakeLists.txt (original)
+++ cfe/trunk/test/CMakeLists.txt Wed Jun 12 14:32:49 2019
@@ -57,6 +57,7 @@ list(APPEND CLANG_TEST_DEPS
   clang-rename
   clang-refactor
   clang-diff
+  clang-scan-deps
   diagtool
   hmaptool
   )

Added: cfe/trunk/test/ClangScanDeps/Inputs/header.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/Inputs/header.h?rev=363204=auto
==
--- cfe/trunk/test/ClangScanDeps/Inputs/header.h (added)
+++ cfe/trunk/test/ClangScanDeps/Inputs/header.h Wed Jun 12 14:32:49 2019
@@ -0,0 +1,3 @@
+#ifdef INCLUDE_HEADER2
+#include "header2.h"
+#endif

Added: cfe/trunk/test/ClangScanDeps/Inputs/header2.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/Inputs/header2.h?rev=363204=auto
==
--- cfe/trunk/test/ClangScanDeps/Inputs/header2.h (added)
+++ cfe/trunk/test/ClangScanDeps/Inputs/header2.h Wed Jun 12 14:32:49 2019
@@ -0,0 +1 @@
+// header 2.

Added: cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json?rev=363204=auto
==
--- cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json (added)
+++ cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json Wed Jun 12 14:32:49 
2019
@@ -0,0 +1,12 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -c DIR/regular_cdb.cpp -IInputs -MD -MF DIR/regular_cdb.d",
+  "file": "DIR/regular_cdb.cpp"
+},
+{
+  "directory": "DIR",
+  "command": "clang -c DIR/regular_cdb.cpp -IInputs -D INCLUDE_HEADER2 -MD -MF 
DIR/regular_cdb2.d",
+  "file": "DIR/regular_cdb.cpp"
+}
+]

Added: cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/regular_cdb.cpp?rev=363204=auto
==
--- cfe/trunk/test/ClangScanDeps/regular_cdb.cpp (added)
+++ cfe/trunk/test/ClangScanDeps/regular_cdb.cpp Wed Jun 12 14:32:49 2019
@@ -0,0 +1,27 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/regular_cdb.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
+// RUN: cp %S/Inputs/header2.h %t.dir/Inputs/header2.h
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/regular_cdb.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1
+// RUN: cat %t.dir/regular_cdb.d | FileCheck %s
+// RUN: cat %t.dir/regular_cdb2.d | FileCheck --check-prefix=CHECK2 %s
+// RUN: rm -rf %t.dir/regular_cdb.d %t.dir/regular_cdb2.d
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 2
+// RUN: cat %t.dir/regular_cdb.d | FileCheck %s
+// RUN: cat %t.dir/regular_cdb2.d | FileCheck --check-prefix=CHECK2 %s
+
+#include "header.h"
+
+// CHECK: regular_cdb.cpp
+// CHECK-NEXT: Inputs{{/|\\}}header.h
+// CHECK-NOT: header2
+
+// CHECK2: regular_cdb.cpp
+// CHECK2-NEXT: 

[PATCH] D63227: [analyzer] Better timers.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, 
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, szepet.
Herald added a project: clang.

`-analyzer-stats` now allows you to find out how much time was spent on 
AST-based analysis and on path-sensitive analysis and, separately, on bug 
visitors, as they're occasionally a performance problem on their own.

The total timer wasn't useful because there's anyway a total time printed out, 
so i removed it.

Accidentally remove the `ExprEngine`'s destructor because it's not really 
useful: reports are flushed manually anyway, which i've noticed while adding a 
timer.

Sample output:

  
===-===
  Analyzer timers
  
===-===
Total Execution Time: 7.4278 seconds (7.4293 wall clock)
  
 ---User Time---   --System Time--   --User+System--   ---Wall Time---  --- 
Name ---
 7.2345 ( 99.0%)   0.1140 ( 96.9%)   7.3485 ( 98.9%)   7.3500 ( 98.9%)  
Path exploration time
 0.0456 (  0.6%)   0.0026 (  2.2%)   0.0482 (  0.6%)   0.0482 (  0.6%)  
Path-sensitive report post-processing time
 0.0300 (  0.4%)   0.0011 (  1.0%)   0.0311 (  0.4%)   0.0312 (  0.4%)  
Syntax-based analysis time
 7.3101 (100.0%)   0.1177 (100.0%)   7.4278 (100.0%)   7.4293 (100.0%)  
Total


Repository:
  rC Clang

https://reviews.llvm.org/D63227

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

Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -196,7 +196,9 @@
 
   /// Time the analyzes time of each translation unit.
   std::unique_ptr AnalyzerTimers;
-  std::unique_ptr TUTotalTimer;
+  std::unique_ptr SyntaxCheckTimer;
+  std::unique_ptr ExprEngineTimer;
+  std::unique_ptr BugReporterTimer;
 
   /// The information about analyzed functions shared throughout the
   /// translation unit.
@@ -212,8 +214,13 @@
 if (Opts->PrintStats || Opts->ShouldSerializeStats) {
   AnalyzerTimers = llvm::make_unique(
   "analyzer", "Analyzer timers");
-  TUTotalTimer = llvm::make_unique(
-  "time", "Analyzer total time", *AnalyzerTimers);
+  SyntaxCheckTimer = llvm::make_unique(
+  "syntaxchecks", "Syntax-based analysis time", *AnalyzerTimers);
+  ExprEngineTimer = llvm::make_unique(
+  "exprengine", "Path exploration time", *AnalyzerTimers);
+  BugReporterTimer = llvm::make_unique(
+  "bugreporter", "Path-sensitive report post-processing time",
+  *AnalyzerTimers);
   llvm::EnableStatistics(/* PrintOnExit= */ false);
 }
   }
@@ -346,8 +353,13 @@
   /// Handle callbacks for arbitrary Decls.
   bool VisitDecl(Decl *D) {
 AnalysisMode Mode = getModeForDecl(D, RecVisitorMode);
-if (Mode & AM_Syntax)
+if (Mode & AM_Syntax) {
+  if (SyntaxCheckTimer)
+SyntaxCheckTimer->startTimer();
   checkerMgr->runCheckersOnASTDecl(D, *Mgr, *RecVisitorBR);
+  if (SyntaxCheckTimer)
+SyntaxCheckTimer->stopTimer();
+}
 return true;
   }
 
@@ -566,7 +578,11 @@
 void AnalysisConsumer::runAnalysisOnTranslationUnit(ASTContext ) {
   BugReporter BR(*Mgr);
   TranslationUnitDecl *TU = C.getTranslationUnitDecl();
+  if (SyntaxCheckTimer)
+SyntaxCheckTimer->startTimer();
   checkerMgr->runCheckersOnASTDecl(TU, *Mgr, BR);
+  if (SyntaxCheckTimer)
+SyntaxCheckTimer->stopTimer();
 
   // Run the AST-only checks using the order in which functions are defined.
   // If inlining is not turned on, use the simplest function order for path
@@ -608,8 +624,6 @@
   if (Diags.hasErrorOccurred() || Diags.hasFatalErrorOccurred())
 return;
 
-  if (TUTotalTimer) TUTotalTimer->startTimer();
-
   if (isBisonFile(C)) {
 reportAnalyzerProgress("Skipping bison-generated file\n");
   } else if (Opts->DisableAllChecks) {
@@ -622,8 +636,6 @@
 runAnalysisOnTranslationUnit(C);
   }
 
-  if (TUTotalTimer) TUTotalTimer->stopTimer();
-
   // Count how many basic blocks we have not covered.
   NumBlocksInAnalyzedFunctions = FunctionSummaries.getTotalNumBasicBlocks();
   NumVisitedBlocksInAnalyzedFunctions =
@@ -747,8 +759,13 @@
 
   BugReporter BR(*Mgr);
 
-  if (Mode & AM_Syntax)
+  if (Mode & AM_Syntax) {
+if (SyntaxCheckTimer)
+  SyntaxCheckTimer->startTimer();
 checkerMgr->runCheckersOnASTBody(D, *Mgr, BR);
+if (SyntaxCheckTimer)
+  SyntaxCheckTimer->stopTimer();
+  }
   if ((Mode & AM_Path) && checkerMgr->hasPathSensitiveCheckers()) {
 RunPathSensitiveChecks(D, 

[PATCH] D63222: [Clangd] Fixed clangd diagnostics priority

[Shaurya Gupta via Phabricator via cfe-commits]

SureYeaah updated this revision to Diff 204351.
SureYeaah added a comment.

Remove extra newline


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D63222

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -101,6 +102,7 @@
   Annotations Test(R"cpp(
 namespace test{};
 void $decl[[foo]]();
+class T{$explicit[[]]$constructor[[T]](int a);};
 int main() {
   $typo[[go\
 o]]();
@@ -112,8 +114,10 @@
   test::$nomembernamespace[[test]];
 }
   )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  TU.ClangTidyChecks = "-*,google-explicit-constructor";
   EXPECT_THAT(
-  TestTU::withCode(Test.code()).build().getDiagnostics(),
+  TU.build().getDiagnostics(),
   ElementsAre(
   // This range spans lines.
   AllOf(Diag(Test.range("typo"),
@@ -135,7 +139,13 @@
"of type 'const char [4]'"),
   Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
   Diag(Test.range("nomembernamespace"),
-   "no member named 'test' in namespace 'test'")));
+   "no member named 'test' in namespace 'test'"),
+  // We make sure here that the entire token is highlighted
+  AllOf(Diag(Test.range("constructor"),
+ "single-argument constructors must be marked explicit to "
+ "avoid unintentional implicit conversions"),
+WithFix(Fix(Test.range("explicit"), "explicit ",
+"insert 'explicit '");
 }
 
 TEST(DiagnosticsTest, FlagsMatter) {
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -18,6 +18,7 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
@@ -102,11 +103,21 @@
 if (R.getBegin() == R.getEnd() && Loc == R.getBegin())
   FallbackRange = halfOpenToRange(M, R);
   }
-  if (FallbackRange)
-return *FallbackRange;
-  // If no suitable range is found, just use the token at the location.
+  // If the token at location is not a comment, use the token.
+  // Otherwise use zero width insertion range
   auto R = Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Loc), M, L);
-  if (!R.isValid()) // Fall back to location only, let the editor deal with it.
+  if (R.isValid()) {
+// check if token at location is a priority i.e. not a comment
+Token token;
+bool isTokenPriority = false;
+if(!Lexer::getRawToken(Loc, token, M, L, true))
+  isTokenPriority = token.getKind() != tok::comment;
+if (!isTokenPriority && FallbackRange)
+  return *FallbackRange;
+  }
+  else if (FallbackRange)
+return *FallbackRange;
+  else // Fall back to location only, let the editor deal with it.
 R = CharSourceRange::getCharRange(Loc);
   return halfOpenToRange(M, R);
 }


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -101,6 +102,7 @@
   Annotations Test(R"cpp(
 namespace test{};
 void $decl[[foo]]();
+class T{$explicit[[]]$constructor[[T]](int a);};
 int main() {
   $typo[[go\
 o]]();
@@ -112,8 +114,10 @@
   test::$nomembernamespace[[test]];
 }
   )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  TU.ClangTidyChecks = "-*,google-explicit-constructor";
   EXPECT_THAT(
-  TestTU::withCode(Test.code()).build().getDiagnostics(),
+  TU.build().getDiagnostics(),
   ElementsAre(
   // This range spans lines.
   AllOf(Diag(Test.range("typo"),
@@ -135,7 +139,13 @@
"of type 'const char [4]'"),
   Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
   Diag(Test.range("nomembernamespace"),
-   "no member named 'test' in namespace 'test'")));
+   "no member named 'test' in namespace 'test'"),
+  // We make sure here that the entire token is highlighted
+  AllOf(Diag(Test.range("constructor"),
+ "single-argument constructors must be marked explicit to "
+ "avoid unintentional implicit conversions"),
+WithFix(Fix(Test.range("explicit"), "explicit ",
+"insert 'explicit 

Re: r363009 - [Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer

[Alex L via cfe-commits]

Thanks, that sounds good to me. I added the assignment with the new comment
in 363199.

On Wed, 12 Jun 2019 at 03:59, Ilya Biryukov  wrote:

> Sure, that should work just fine. Could you also add a comment what events
> setting the log file sets in motion? It is not obvious from the test code.
>
> On Tue, Jun 11, 2019 at 6:51 PM Alex L  wrote:
>
>> Hmm, the logging was meant to exercise the creation of chained diagnostic
>> consumer, so without it the test is kind of pointless. I'll undo your
>> change, but will set the file to "-" which will print the log to STDERR and
>> won't create new files. Does that sound reasonable?
>>
>> Cheers,
>> Alex
>>
>> On Tue, 11 Jun 2019 at 02:51, Ilya Biryukov  wrote:
>>
>>> Hi Alex,
>>>
>>> Just wanted to let you know that I removed logging of diagnostics into a
>>> file inside the unit test in r363041 to unbreak our integrate.
>>>
>>> On Tue, Jun 11, 2019 at 1:29 AM Alex Lorenz via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: arphaman
 Date: Mon Jun 10 16:32:42 2019
 New Revision: 363009

 URL: http://llvm.org/viewvc/llvm-project?rev=363009=rev
 Log:
 [Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer
 in the compiler

 The function SetUpDiagnosticLog that was called from createDiagnostics
 didn't
 handle the case where the diagnostics engine didn't own the diagnostics
 consumer.
 This is a potential problem for a clang tool, in particular some of the
 follow-up
 patches for clang-scan-deps will need this fix.

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

 Modified:
 cfe/trunk/lib/Frontend/CompilerInstance.cpp
 cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp

 Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=363009=363008=363009=diff

 ==
 --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
 +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Mon Jun 10 16:32:42 2019
 @@ -232,9 +232,13 @@ static void SetUpDiagnosticLog(Diagnosti

  std::move(StreamOwner));
if (CodeGenOpts)
  Logger->setDwarfDebugFlags(CodeGenOpts->DwarfDebugFlags);
 -  assert(Diags.ownsClient());
 -  Diags.setClient(
 -  new ChainedDiagnosticConsumer(Diags.takeClient(),
 std::move(Logger)));
 +  if (Diags.ownsClient()) {
 +Diags.setClient(
 +new ChainedDiagnosticConsumer(Diags.takeClient(),
 std::move(Logger)));
 +  } else {
 +Diags.setClient(
 +new ChainedDiagnosticConsumer(Diags.getClient(),
 std::move(Logger)));
 +  }
  }

  static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts,

 Modified: cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp?rev=363009=363008=363009=diff

 ==
 --- cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp (original)
 +++ cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp Mon Jun 10
 16:32:42 2019
 @@ -8,6 +8,7 @@

  #include "clang/Frontend/CompilerInstance.h"
  #include "clang/Frontend/CompilerInvocation.h"
 +#include "clang/Frontend/TextDiagnosticPrinter.h"
  #include "llvm/Support/FileSystem.h"
  #include "llvm/Support/Format.h"
  #include "llvm/Support/ToolOutputFile.h"
 @@ -70,4 +71,21 @@ TEST(CompilerInstance, DefaultVFSOverlay
ASSERT_TRUE(Instance.getFileManager().getFile("vfs-virtual.file"));
  }

 +TEST(CompilerInstance,
 AllowDiagnosticLogWithUnownedDiagnosticConsumer) {
 +  auto DiagOpts = new DiagnosticOptions();
 +  DiagOpts->DiagnosticLogFile = "log.diags";
 +
 +  // Create the diagnostic engine with unowned consumer.
 +  std::string DiagnosticOutput;
 +  llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput);
 +  auto DiagPrinter = llvm::make_unique(
 +  DiagnosticsOS, new DiagnosticOptions());
 +  CompilerInstance Instance;
 +  IntrusiveRefCntPtr Diags =
 Instance.createDiagnostics(
 +  DiagOpts, DiagPrinter.get(), /*ShouldOwnClient=*/false);
 +
 +  Diags->Report(diag::err_expected) << "no crash";
 +  ASSERT_EQ(DiagnosticsOS.str(), "error: expected no crash\n");
 +}
 +
  } // anonymous namespace


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

>>>
>>>
>>> --
>>> Regards,
>>> Ilya Biryukov
>>>
>>
>
> --
> Regards,
> Ilya Biryukov
>

r363199 - [test] Reinstate the assignment to the diagnostic log in the unittest

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Jun 12 13:35:44 2019
New Revision: 363199

URL: http://llvm.org/viewvc/llvm-project?rev=363199=rev
Log:
[test] Reinstate the assignment to the diagnostic log in the unittest
from r363009

The diagnostic log is now set to "-" which forces it to use STDERR
instead of the filesystem. A new comment is added to explain why
the assignment is needed in the test.

Modified:
cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp

Modified: cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp?rev=363199=363198=363199=diff
==
--- cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp (original)
+++ cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp Wed Jun 12 13:35:44 
2019
@@ -73,6 +73,10 @@ TEST(CompilerInstance, DefaultVFSOverlay
 
 TEST(CompilerInstance, AllowDiagnosticLogWithUnownedDiagnosticConsumer) {
   auto DiagOpts = new DiagnosticOptions();
+  // Tell the diagnostics engine to emit the diagnostic log to STDERR. This
+  // ensures that a chained diagnostic consumer is created so that the test can
+  // exercise the unowned diagnostic consumer in a chained consumer.
+  DiagOpts->DiagnosticLogFile = "-";
 
   // Create the diagnostic engine with unowned consumer.
   std::string DiagnosticOutput;


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

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: lib/CodeGen/BackendUtil.cpp:1345
   Conf.RemarksPasses = CGOpts.OptRecordPasses;
-  Conf.DwoPath = CGOpts.SplitDwarfFile;
+  Conf.DwoPath = CGOpts.SplitDwarfOutput;
   switch (Action) {

tejohnson wrote:
> aaronpuchert wrote:
> > tejohnson wrote:
> > > aaronpuchert wrote:
> > > > aaronpuchert wrote:
> > > > > @pcc Your documentation for `DwoPath` suggests that this should be 
> > > > > the actual output filename. However, the test that you added together 
> > > > > with this line in rC333677 doesn't fail whatever garbage I write into 
> > > > > that field here. What can I add to that so that it fails when we 
> > > > > don't do the right thing here?
> > > > @pcc Could you (or perhaps @tejohnson) comment on what the intended 
> > > > behavior is here, and how I can change the test so that it fails when I 
> > > > do the wrong thing? Is this the name of the file we write the split 
> > > > debug info to, or is it the value we use for the DW_AT_[GNU_]dwo_name 
> > > > attribute in the skeleton CU?
> > > It is the name of the file the split debug info is written to. If you 
> > > test by changing the file name given to the -split-dwarf-file option in 
> > > test/CodeGen/thinlto-split-dwarf.c, make sure you clean up the old one in 
> > > your test output directory. When I tested it just now, it initially still 
> > > passed because I had an old one sitting in the output directory from a 
> > > prior run. Once I removed that it failed with the new name (without 
> > > changing the corresponding llvm-readobj invocation).
> > Thanks, I didn't consider that. I wasn't even aware that test output is 
> > persisted.
> > 
> > It seems `DwoPath` is used both as output filename and as value for 
> > `DW_AT_[GNU_]dwo_name` in the skeleton CU. `llc` has separate options for 
> > both: `-split-dwarf-file` for the attribute and `-split-dwarf-output` for 
> > the output filename. I want to do the same for Clang. Then we should 
> > probably separate them for LTO, too.
> > 
> > What is the use case for `-split-dwarf-file` with an individual thin 
> > backend? Is that used for distributed/remote builds? Also is there any 
> > non-cc1 way to use it? There is `--plugin-opt=dwo_dir=...` for the LTO 
> > linker plugin, but that seems to go a different route.
> This is the path taken for distributed ThinLTO, the plugin-opt passed to 
> linker are for in-process ThinLTO. While -split-dwarf-file is a cc1 option, 
> it is normally set automatically from the filename under -gsplit-dwarf (see 
> where it is set in Clang::ConstructJob).
Ok, then it makes sense to do the changes for LTO as well.

You're right that `-split-dwarf-file` is produced for `-gsplit-dwarf`, but not 
with LTO, right? When I set `-gsplit-dwarf` on a vanilla ThinLTO build, then 
the flag is just ignored and I'm getting the debug info in the final 
executables/shared objects. (At least with Clang 8.)

I'm asking because this changes the cc1-interface, and while I have adapted 
`Clang::ConstructJob`, I'm not sure how distributed ThinLTO works, and whether 
there have to be changes.

If it works like test/CodeGen/thinlto-split-dwarf.c, using cc1-options, then 
users might have to adapt after this change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59673



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

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: lib/CodeGen/BackendUtil.cpp:1345
   Conf.RemarksPasses = CGOpts.OptRecordPasses;
-  Conf.DwoPath = CGOpts.SplitDwarfFile;
+  Conf.DwoPath = CGOpts.SplitDwarfOutput;
   switch (Action) {

aaronpuchert wrote:
> tejohnson wrote:
> > aaronpuchert wrote:
> > > aaronpuchert wrote:
> > > > @pcc Your documentation for `DwoPath` suggests that this should be the 
> > > > actual output filename. However, the test that you added together with 
> > > > this line in rC333677 doesn't fail whatever garbage I write into that 
> > > > field here. What can I add to that so that it fails when we don't do 
> > > > the right thing here?
> > > @pcc Could you (or perhaps @tejohnson) comment on what the intended 
> > > behavior is here, and how I can change the test so that it fails when I 
> > > do the wrong thing? Is this the name of the file we write the split debug 
> > > info to, or is it the value we use for the DW_AT_[GNU_]dwo_name attribute 
> > > in the skeleton CU?
> > It is the name of the file the split debug info is written to. If you test 
> > by changing the file name given to the -split-dwarf-file option in 
> > test/CodeGen/thinlto-split-dwarf.c, make sure you clean up the old one in 
> > your test output directory. When I tested it just now, it initially still 
> > passed because I had an old one sitting in the output directory from a 
> > prior run. Once I removed that it failed with the new name (without 
> > changing the corresponding llvm-readobj invocation).
> Thanks, I didn't consider that. I wasn't even aware that test output is 
> persisted.
> 
> It seems `DwoPath` is used both as output filename and as value for 
> `DW_AT_[GNU_]dwo_name` in the skeleton CU. `llc` has separate options for 
> both: `-split-dwarf-file` for the attribute and `-split-dwarf-output` for the 
> output filename. I want to do the same for Clang. Then we should probably 
> separate them for LTO, too.
> 
> What is the use case for `-split-dwarf-file` with an individual thin backend? 
> Is that used for distributed/remote builds? Also is there any non-cc1 way to 
> use it? There is `--plugin-opt=dwo_dir=...` for the LTO linker plugin, but 
> that seems to go a different route.
This is the path taken for distributed ThinLTO, the plugin-opt passed to linker 
are for in-process ThinLTO. While -split-dwarf-file is a cc1 option, it is 
normally set automatically from the filename under -gsplit-dwarf (see where it 
is set in Clang::ConstructJob).


Repository:
  rC Clang

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

https://reviews.llvm.org/D59673



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

[PATCH] D63175: [MS] Pretend constexpr variable template specializations are inline

[Reid Kleckner via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/ASTContext.cpp:9809
+  // variable template specializations inline.
+  if (isa(VD) && VD->isConstexpr())
+return GVA_DiscardableODR;

rsmith wrote:
> It'd be nice to include a note here that this is strictly non-conforming 
> (since another TU could be relying on this TU to provide the definition), but 
> that we don't expect that to happen in practice for variable template 
> specializations declared `constexpr`.
I forgot to add the comment before pushing, so I added it in rC363195.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63175



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

r363195 - Add comment to r363191 code as requested in code review

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Wed Jun 12 12:50:06 2019
New Revision: 363195

URL: http://llvm.org/viewvc/llvm-project?rev=363195=rev
Log:
Add comment to r363191 code as requested in code review

Modified:
cfe/trunk/lib/AST/ASTContext.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=363195=363194=363195=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Jun 12 12:50:06 2019
@@ -9804,8 +9804,11 @@ static GVALinkage basicGVALinkageForVari
   // was marked inline. MSVC 14.21.27702 headers define _Is_integral in a
   // header this way, and we don't want to emit non-discardable definitions
   // of these variables in every TU that includes . This
-  // behavior can be removed if the headers change to explicitly mark such
-  // variable template specializations inline.
+  // behavior is non-conforming, since another TU could use an extern
+  // template declaration for this variable, but for constexpr variables,
+  // it's unlikely for a user to want to do that. This behavior can be
+  // removed if the headers change to explicitly mark such variable 
template
+  // specializations inline.
   if (isa(VD) && VD->isConstexpr())
 return GVA_DiscardableODR;
 


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

[PATCH] D62988: Add an attribute to allow fields of non-trivial types in C unions

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

rsmith added a comment.

In D62988#1540359 , @rjmccall wrote:

> For ARC, you could bzero the union member; this is what how we tell people to 
> initialize malloc'ed memory as well, since there's no default-constructor 
> concept that they can invoke for such cases.
>  Our immediate need for this attribute is that we have some code that wants 
> to adopt non-trivial annotations in unions, with no intention of ever copying 
> or destroying them as aggregates.


OK, so we'd be looking at something like this when using the attribute:

  typedef struct S {
int is_b;
union {
  float f;
  __attribute__((non_trivial_union_member)) id b;
};
  } S;
  S make() {
S s = {0, 0.0f};
return s;
  }
  void set_f(S *s, float f) {
if (s->is_b) s->b = 0;
s->is_b = 0;
s->f = f;
  }
  void set_b(S *s, id b) {
if (!s->is_b) bzero(s, sizeof(S));
s->is_b = 1;
s->b = b;
  }
  void destroy_s(S *s) {
if (s->is_b) s->b = 0;
  }

versus the same code written without the attribute, which might be:

  typedef struct S {
int is_b;
union {
  float f;
  void *b;
};
  } S;
  S make() {
S s = {0, 0.0f};
return s;
  }
  void set_f(S *s, float f) {
if (s->is_b) (__bridge_transfer id)s->b;
s->is_b = 0;
s->f = f;
  }
  void set_b(S *s, id b) {
if (s->is_b) (__bridge_transfer id)s->b;
s->is_b = 1;
s->b = (__bridge_retained void*)b;
  }
  void destroy_s(S *s) {
if (s->is_b) (__bridge_transfer id)s->b;
  }

To me, using the attribute here doesn't seem worthwhile; the first form of this 
code seems scary since important reference counting actions are hidden behind 
what appear to be dead stores, and the second form (while verbose) is at least 
explicit about what's going on. If you still think this is a useful feature, so 
be it, but we should at least be explicit in Clang's ARC documentation about 
how to correctly make use of it.

I'm not sure whether we should support this as a way of disabling the union 
triviality features in general, or only the restriction on lifetime types, but 
either way, I think the support of this attribute should not be dependent on 
the language mode; I think adding a C-only extension for this does a disservice 
to our users.

> Of course a better solution would be to make any C code that would copy or 
> destroy the aggregate type illegal, but that seems like a big project.  But 
> maybe there'd be no need for this attribute in the long term if we eventually 
> do have that support.

Yeah, maybe so; adopting the C++11 "unrestricted unions" behavior for 
non-trivial types in unions in C does seem to make a certain amount of sense.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62988



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

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: lib/CodeGen/BackendUtil.cpp:1345
   Conf.RemarksPasses = CGOpts.OptRecordPasses;
-  Conf.DwoPath = CGOpts.SplitDwarfFile;
+  Conf.DwoPath = CGOpts.SplitDwarfOutput;
   switch (Action) {

tejohnson wrote:
> aaronpuchert wrote:
> > aaronpuchert wrote:
> > > @pcc Your documentation for `DwoPath` suggests that this should be the 
> > > actual output filename. However, the test that you added together with 
> > > this line in rC333677 doesn't fail whatever garbage I write into that 
> > > field here. What can I add to that so that it fails when we don't do the 
> > > right thing here?
> > @pcc Could you (or perhaps @tejohnson) comment on what the intended 
> > behavior is here, and how I can change the test so that it fails when I do 
> > the wrong thing? Is this the name of the file we write the split debug info 
> > to, or is it the value we use for the DW_AT_[GNU_]dwo_name attribute in the 
> > skeleton CU?
> It is the name of the file the split debug info is written to. If you test by 
> changing the file name given to the -split-dwarf-file option in 
> test/CodeGen/thinlto-split-dwarf.c, make sure you clean up the old one in 
> your test output directory. When I tested it just now, it initially still 
> passed because I had an old one sitting in the output directory from a prior 
> run. Once I removed that it failed with the new name (without changing the 
> corresponding llvm-readobj invocation).
Thanks, I didn't consider that. I wasn't even aware that test output is 
persisted.

It seems `DwoPath` is used both as output filename and as value for 
`DW_AT_[GNU_]dwo_name` in the skeleton CU. `llc` has separate options for both: 
`-split-dwarf-file` for the attribute and `-split-dwarf-output` for the output 
filename. I want to do the same for Clang. Then we should probably separate 
them for LTO, too.

What is the use case for `-split-dwarf-file` with an individual thin backend? 
Is that used for distributed/remote builds? Also is there any non-cc1 way to 
use it? There is `--plugin-opt=dwo_dir=...` for the LTO linker plugin, but that 
seems to go a different route.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59673



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

[PATCH] D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine

[Manikishan Ghantasala via Phabricator via cfe-commits]

Manikishan updated this revision to Diff 204340.
Manikishan marked 4 inline comments as done and 2 inline comments as done.
Manikishan retitled this revision from "[clang-format] Added New Style Rule:  
OnePerLineBitFieldDecl" to "[clang-format] Added New Style Rule: 
BitFieldDeclarationsOnePerLine".

Repository:
  rC Clang

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

https://reviews.llvm.org/D63062

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3656,6 +3656,19 @@
"#define A Just forcing a new line\n"
"ddd);");
 }
+TEST_F(FormatTest, AlignBitFieldDeclarationsOnConsecutiveLines){
+  FormatStyle Style = {};
+  Style.BitFieldDeclarationsOnePerLine = true;
+  verifyFormat(
+"unsigned int baz : 11,
+  aaa : 2,
+  foo : 3"
+  );
+  Style.BitFieldDeclarationsOnePerLine = false;
+  verifyFormat(
+"unsigned int baz : 11, aaa : 2, foo : 3"
+  );
+} 
 
 TEST_F(FormatTest, LineBreakingInBinaryExpressions) {
   verifyFormat(
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2916,6 +2916,9 @@
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
  const FormatToken ) {
   const FormatToken  = *Right.Previous;
+  if (Right.Previous->is(tok::comma) && Style.BitFieldDeclarationsOnePerLine &&
+  Right.is(tok::identifier) && (Right.Next->is(tok::colon)))
+return true;
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
 return true;
 
Index: lib/Format/FormatToken.h
===
--- lib/Format/FormatToken.h
+++ lib/Format/FormatToken.h
@@ -516,6 +516,14 @@
 return T && T->is(tok::kw_auto);
   }
 
+  /// Returns whether the token is a Bit field, and checks whether
+  /// the Style is C / C++.
+  bool isBitField() const {
+const FormatToken *T = this;
+T = T->getPreviousNonComment();
+return (T->Tok.is(tok::comma) && Tok.is(tok::identifier) &&
+T->Next->Tok.is(tok::colon));
+  }
   /// Same as opensBlockOrBlockTypeList, but for the closing token.
   bool closesBlockOrBlockTypeList(const FormatStyle ) const {
 if (is(TT_TemplateString) && closesScope())
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -451,6 +451,7 @@
 IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
 IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
+IO.mapOptional("BitFieldDeclarationsOnePerLine", Style.BitFieldDeclarationsOnePerLine);
 IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
 IO.mapOptional("MacroBlockEnd", Style.MacroBlockEnd);
 IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -329,6 +329,8 @@
 bool ContinuationIndenter::mustBreak(const LineState ) {
   const FormatToken  = *State.NextToken;
   const FormatToken  = *Current.Previous;
+  if (Style.isCpp() && Current.isBitField() && Style.BitFieldDeclarationsOnePerLine)
+return true; 
   if (Current.MustBreakBefore || Current.is(TT_InlineASMColon))
 return true;
   if (State.Stack.back().BreakBeforeClosingBrace &&
@@ -541,7 +543,7 @@
  unsigned ExtraSpaces) {
   FormatToken  = *State.NextToken;
   const FormatToken  = *State.NextToken->Previous;
-  if (Current.is(tok::equal) &&
+  if (Current.isOneOf(tok::equal, tok::colon) &&
   (State.Line->First->is(tok::kw_for) || Current.NestingLevel == 0) &&
   State.Stack.back().VariablePos == 0) {
 State.Stack.back().VariablePos = State.Column;
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -101,6 +101,16 @@
   /// \endcode
   bool AlignConsecutiveDeclarations;
 
+  /// If ``true``, Linesup Bitfield Declarations.
+  /// This will lineup Bitfield declarations on consecutive lines. This
+  /// will result in formatting like
+  /// \code
+  ///  unsigned int  baz : 1, /* Bitfield; line up entries if desire*/
+  ///fuz : 5,
+  ///zap : 2;
+  /// \endcode
+  bool BitFieldDeclarationsOnePerLine;
+

[PATCH] D63175: [MS] Pretend constexpr variable template specializations are inline

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363191: [MS] Pretend constexpr variable template 
specializations are inline (authored by rnk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63175?vs=204190=204339#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63175

Files:
  cfe/trunk/lib/AST/ASTContext.cpp
  cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp


Index: cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp
===
--- cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp
+++ cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -emit-llvm -triple=x86_64-windows-msvc -fms-compatibility 
%s -o - | FileCheck %s
+
+template  constexpr bool _Is_integer = false;
+template <> constexpr bool _Is_integer = true;
+template <> constexpr bool _Is_integer = false;
+extern "C" const bool *escape = &_Is_integer;
+
+// CHECK: @"??$_Is_integer@H@@3_NB" = linkonce_odr dso_local constant i8 1, 
comdat, align 1
+//   Should not emit _Is_integer, since it's not referenced.
+// CHECK-NOT: @"??$_Is_integer@D@@3_NB"
+// CHECK: @escape = dso_local global i8* @"??$_Is_integer@H@@3_NB", align 8
Index: cfe/trunk/lib/AST/ASTContext.cpp
===
--- cfe/trunk/lib/AST/ASTContext.cpp
+++ cfe/trunk/lib/AST/ASTContext.cpp
@@ -9799,10 +9799,22 @@
 return StrongLinkage;
 
   case TSK_ExplicitSpecialization:
-return Context.getTargetInfo().getCXXABI().isMicrosoft() &&
-   VD->isStaticDataMember()
-   ? GVA_StrongODR
-   : StrongLinkage;
+if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+  // If this is a fully specialized constexpr variable template, pretend it
+  // was marked inline. MSVC 14.21.27702 headers define _Is_integral in a
+  // header this way, and we don't want to emit non-discardable definitions
+  // of these variables in every TU that includes . This
+  // behavior can be removed if the headers change to explicitly mark such
+  // variable template specializations inline.
+  if (isa(VD) && VD->isConstexpr())
+return GVA_DiscardableODR;
+
+  // Use ODR linkage for static data members of fully specialized templates
+  // to prevent duplicate definition errors with MSVC.
+  if (VD->isStaticDataMember())
+return GVA_StrongODR;
+}
+return StrongLinkage;
 
   case TSK_ExplicitInstantiationDefinition:
 return GVA_StrongODR;


Index: cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp
===
--- cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp
+++ cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -emit-llvm -triple=x86_64-windows-msvc -fms-compatibility %s -o - | FileCheck %s
+
+template  constexpr bool _Is_integer = false;
+template <> constexpr bool _Is_integer = true;
+template <> constexpr bool _Is_integer = false;
+extern "C" const bool *escape = &_Is_integer;
+
+// CHECK: @"??$_Is_integer@H@@3_NB" = linkonce_odr dso_local constant i8 1, comdat, align 1
+//   Should not emit _Is_integer, since it's not referenced.
+// CHECK-NOT: @"??$_Is_integer@D@@3_NB"
+// CHECK: @escape = dso_local global i8* @"??$_Is_integer@H@@3_NB", align 8
Index: cfe/trunk/lib/AST/ASTContext.cpp
===
--- cfe/trunk/lib/AST/ASTContext.cpp
+++ cfe/trunk/lib/AST/ASTContext.cpp
@@ -9799,10 +9799,22 @@
 return StrongLinkage;
 
   case TSK_ExplicitSpecialization:
-return Context.getTargetInfo().getCXXABI().isMicrosoft() &&
-   VD->isStaticDataMember()
-   ? GVA_StrongODR
-   : StrongLinkage;
+if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+  // If this is a fully specialized constexpr variable template, pretend it
+  // was marked inline. MSVC 14.21.27702 headers define _Is_integral in a
+  // header this way, and we don't want to emit non-discardable definitions
+  // of these variables in every TU that includes . This
+  // behavior can be removed if the headers change to explicitly mark such
+  // variable template specializations inline.
+  if (isa(VD) && VD->isConstexpr())
+return GVA_DiscardableODR;
+
+  // Use ODR linkage for static data members of fully specialized templates
+  // to prevent duplicate definition errors with MSVC.
+  if (VD->isStaticDataMember())
+return GVA_StrongODR;
+}
+return StrongLinkage;
 
   case TSK_ExplicitInstantiationDefinition:
 return GVA_StrongODR;
___
cfe-commits 

r363191 - [MS] Pretend constexpr variable template specializations are inline

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Wed Jun 12 11:53:49 2019
New Revision: 363191

URL: http://llvm.org/viewvc/llvm-project?rev=363191=rev
Log:
[MS] Pretend constexpr variable template specializations are inline

Fixes link errors with clang and the latest Visual C++ 14.21.27702
headers, which was reported as PR42027.

I chose to intentionally make these things linkonce_odr, i.e.
discardable, so that we don't emit definitions of these things in every
translation unit that includes STL headers.

Note that this is *not* what MSVC does: MSVC has not yet implemented C++
DR2387, so they emit fully specialized constexpr variable templates with
static / internal linkage.

Reviewers: rsmith

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

Added:
cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp
Modified:
cfe/trunk/lib/AST/ASTContext.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=363191=363190=363191=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Jun 12 11:53:49 2019
@@ -9799,10 +9799,22 @@ static GVALinkage basicGVALinkageForVari
 return StrongLinkage;
 
   case TSK_ExplicitSpecialization:
-return Context.getTargetInfo().getCXXABI().isMicrosoft() &&
-   VD->isStaticDataMember()
-   ? GVA_StrongODR
-   : StrongLinkage;
+if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+  // If this is a fully specialized constexpr variable template, pretend it
+  // was marked inline. MSVC 14.21.27702 headers define _Is_integral in a
+  // header this way, and we don't want to emit non-discardable definitions
+  // of these variables in every TU that includes . This
+  // behavior can be removed if the headers change to explicitly mark such
+  // variable template specializations inline.
+  if (isa(VD) && VD->isConstexpr())
+return GVA_DiscardableODR;
+
+  // Use ODR linkage for static data members of fully specialized templates
+  // to prevent duplicate definition errors with MSVC.
+  if (VD->isStaticDataMember())
+return GVA_StrongODR;
+}
+return StrongLinkage;
 
   case TSK_ExplicitInstantiationDefinition:
 return GVA_StrongODR;

Added: cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp?rev=363191=auto
==
--- cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp Wed Jun 12 11:53:49 
2019
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -emit-llvm -triple=x86_64-windows-msvc -fms-compatibility 
%s -o - | FileCheck %s
+
+template  constexpr bool _Is_integer = false;
+template <> constexpr bool _Is_integer = true;
+template <> constexpr bool _Is_integer = false;
+extern "C" const bool *escape = &_Is_integer;
+
+// CHECK: @"??$_Is_integer@H@@3_NB" = linkonce_odr dso_local constant i8 1, 
comdat, align 1
+//   Should not emit _Is_integer, since it's not referenced.
+// CHECK-NOT: @"??$_Is_integer@D@@3_NB"
+// CHECK: @escape = dso_local global i8* @"??$_Is_integer@H@@3_NB", align 8


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

[PATCH] D63157: C++ DR712 and others: handle non-odr-use resulting from an lvalue-to-rvalue conversion applied to a member access or similar not-quite-trivial lvalue expression.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:1429
+/// for instance if a block or lambda or a member of a local class uses a
+/// const int variable or constexpr variable from an enclosing function.
 CodeGenFunction::ConstantEmission

Isn't the old comment correct here?  This is mandatory because of the 
enclosing-local-scope issues; that might be an "optimization" in the language, 
but it's not an optimization at the IRGen level because Sema literally is 
forcing us to do it. 



Comment at: lib/Sema/SemaExpr.cpp:15808
+namespace {
+// Helper to copy the template arguments from a DeclRefExpr or MemberExpr.
+class CopiedTemplateArgs {

This is really cute; I might steal this idea.  That said, it's probably worth 
explaining the lifetime mechanics here so that non-experts can understand how 
this works.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63157



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

r363188 - PR42220: take into account the possibility of aggregates with base

[Richard Smith via cfe-commits]

Author: rsmith
Date: Wed Jun 12 11:32:22 2019
New Revision: 363188

URL: http://llvm.org/viewvc/llvm-project?rev=363188=rev
Log:
PR42220: take into account the possibility of aggregates with base
classes when checking an InitListExpr for lifetime extension.

Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/CodeGenCXX/temporaries.cpp

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=363188=363187=363188=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Wed Jun 12 11:32:22 2019
@@ -6860,6 +6860,9 @@ static void visitLocalsRetainedByInitial
   RK_ReferenceBinding, Visit);
   else {
 unsigned Index = 0;
+for (; Index < RD->getNumBases() && Index < ILE->getNumInits(); 
++Index)
+  visitLocalsRetainedByInitializer(Path, ILE->getInit(Index), Visit,
+   RevisitSubinits);
 for (const auto *I : RD->fields()) {
   if (Index >= ILE->getNumInits())
 break;

Modified: cfe/trunk/test/CodeGenCXX/temporaries.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/temporaries.cpp?rev=363188=363187=363188=diff
==
--- cfe/trunk/test/CodeGenCXX/temporaries.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/temporaries.cpp Wed Jun 12 11:32:22 2019
@@ -896,3 +896,21 @@ namespace Conditional {
   // CHECK: br label
   A & = b ? static_cast(B()) : static_cast(C());
 }
+
+#if __cplusplus >= 201703L
+namespace PR42220 {
+  struct X { X(); ~X(); };
+  struct A { X & };
+  struct B : A {};
+  void g() noexcept;
+  // CHECK-CXX17-LABEL: define{{.*}} @_ZN7PR422201fEv(
+  void f() {
+// CHECK-CXX17: call{{.*}} @_ZN7PR422201XC1Ev(
+B & = {X()};
+// CHECK-CXX17-NOT: call{{.*}} @_ZN7PR422201XD1Ev(
+// CHECK-CXX17: call{{.*}} @_ZN7PR422201gEv(
+g();
+// CHECK-CXX17: call{{.*}} @_ZN7PR422201XD1Ev(
+  }
+}
+#endif


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

[PATCH] D62988: Add an attribute to allow fields of non-trivial types in C unions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

For ARC, you could bzero the union member; this is what how we tell people to 
initialize malloc'ed memory as well, since there's no default-constructor 
concept that they can invoke for such cases.

Our immediate need for this attribute is that we have some code that wants to 
adopt non-trivial annotations in unions, with no intention of ever copying or 
destroying them as aggregates.  Of course a better solution would be to make 
any C code that would copy or destroy the aggregate type illegal, but that 
seems like a big project.  But maybe there'd be no need for this attribute in 
the long term if we eventually do have that support.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62988



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

[PATCH] D62946: [analyzer] ProgramPoint: more explicit printJson()

[Csaba Dabis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363187: [analyzer] ProgramPoint: more explicit printJson() 
(authored by Charusso, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62946?vs=203328=204334#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62946

Files:
  cfe/trunk/lib/Analysis/ProgramPoint.cpp


Index: cfe/trunk/lib/Analysis/ProgramPoint.cpp
===
--- cfe/trunk/lib/Analysis/ProgramPoint.cpp
+++ cfe/trunk/lib/Analysis/ProgramPoint.cpp
@@ -99,12 +99,6 @@
   case ProgramPoint::CallExitEndKind:
 Out << "CallExitEnd\"";
 break;
-  case ProgramPoint::PostStmtPurgeDeadSymbolsKind:
-Out << "PostStmtPurgeDeadSymbols\"";
-break;
-  case ProgramPoint::PreStmtPurgeDeadSymbolsKind:
-Out << "PreStmtPurgeDeadSymbols\"";
-break;
   case ProgramPoint::EpsilonKind:
 Out << "EpsilonPoint\"";
 break;
@@ -210,20 +204,35 @@
 Out << ", ";
 printLocJson(Out, S->getBeginLoc(), SM);
 
-Out << ", \"stmt_point_kind\": ";
-if (getAs())
-  Out << "\"PreStmt\"";
+Out << ", \"stmt_point_kind\": \"";
+if (getAs())
+  Out << "PreLoad";
+else if (getAs())
+  Out << "PreStore";
+else if (getAs())
+  Out << "PostAllocatorCall";
+else if (getAs())
+  Out << "PostCondition";
 else if (getAs())
-  Out << "\"PostLoad\"";
-else if (getAs())
-  Out << "\"PostStore\"";
+  Out << "PostLoad";
 else if (getAs())
-  Out << "\"PostLValue\"";
-else if (getAs())
-  Out << "\"PostAllocatorCall\"";
-else
-  Out << "null";
+  Out << "PostLValue";
+else if (getAs())
+  Out << "PostStore";
+else if (getAs())
+  Out << "PostStmt";
+else if (getAs())
+  Out << "PostStmtPurgeDeadSymbols";
+else if (getAs())
+  Out << "PreStmtPurgeDeadSymbols";
+else if (getAs())
+  Out << "PreStmt";
+else {
+  Out << "\nKind: '" << getKind();
+  llvm_unreachable("' is unhandled StmtPoint kind!");
+}
 
+Out << '\"';
 break;
   }
   }


Index: cfe/trunk/lib/Analysis/ProgramPoint.cpp
===
--- cfe/trunk/lib/Analysis/ProgramPoint.cpp
+++ cfe/trunk/lib/Analysis/ProgramPoint.cpp
@@ -99,12 +99,6 @@
   case ProgramPoint::CallExitEndKind:
 Out << "CallExitEnd\"";
 break;
-  case ProgramPoint::PostStmtPurgeDeadSymbolsKind:
-Out << "PostStmtPurgeDeadSymbols\"";
-break;
-  case ProgramPoint::PreStmtPurgeDeadSymbolsKind:
-Out << "PreStmtPurgeDeadSymbols\"";
-break;
   case ProgramPoint::EpsilonKind:
 Out << "EpsilonPoint\"";
 break;
@@ -210,20 +204,35 @@
 Out << ", ";
 printLocJson(Out, S->getBeginLoc(), SM);
 
-Out << ", \"stmt_point_kind\": ";
-if (getAs())
-  Out << "\"PreStmt\"";
+Out << ", \"stmt_point_kind\": \"";
+if (getAs())
+  Out << "PreLoad";
+else if (getAs())
+  Out << "PreStore";
+else if (getAs())
+  Out << "PostAllocatorCall";
+else if (getAs())
+  Out << "PostCondition";
 else if (getAs())
-  Out << "\"PostLoad\"";
-else if (getAs())
-  Out << "\"PostStore\"";
+  Out << "PostLoad";
 else if (getAs())
-  Out << "\"PostLValue\"";
-else if (getAs())
-  Out << "\"PostAllocatorCall\"";
-else
-  Out << "null";
+  Out << "PostLValue";
+else if (getAs())
+  Out << "PostStore";
+else if (getAs())
+  Out << "PostStmt";
+else if (getAs())
+  Out << "PostStmtPurgeDeadSymbols";
+else if (getAs())
+  Out << "PreStmtPurgeDeadSymbols";
+else if (getAs())
+  Out << "PreStmt";
+else {
+  Out << "\nKind: '" << getKind();
+  llvm_unreachable("' is unhandled StmtPoint kind!");
+}
 
+Out << '\"';
 break;
   }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r363187 - [analyzer] ProgramPoint: more explicit printJson()

[Csaba Dabis via cfe-commits]

Author: charusso
Date: Wed Jun 12 11:24:02 2019
New Revision: 363187

URL: http://llvm.org/viewvc/llvm-project?rev=363187=rev
Log:
[analyzer] ProgramPoint: more explicit printJson()

Summary: Now we print out every possible kinds of ProgramPoints.

Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus

Reviewed By: NoQ

Subscribers: szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy,
 dkrupp, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Analysis/ProgramPoint.cpp

Modified: cfe/trunk/lib/Analysis/ProgramPoint.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ProgramPoint.cpp?rev=363187=363186=363187=diff
==
--- cfe/trunk/lib/Analysis/ProgramPoint.cpp (original)
+++ cfe/trunk/lib/Analysis/ProgramPoint.cpp Wed Jun 12 11:24:02 2019
@@ -99,12 +99,6 @@ void ProgramPoint::printJson(llvm::raw_o
   case ProgramPoint::CallExitEndKind:
 Out << "CallExitEnd\"";
 break;
-  case ProgramPoint::PostStmtPurgeDeadSymbolsKind:
-Out << "PostStmtPurgeDeadSymbols\"";
-break;
-  case ProgramPoint::PreStmtPurgeDeadSymbolsKind:
-Out << "PreStmtPurgeDeadSymbols\"";
-break;
   case ProgramPoint::EpsilonKind:
 Out << "EpsilonPoint\"";
 break;
@@ -210,20 +204,35 @@ void ProgramPoint::printJson(llvm::raw_o
 Out << ", ";
 printLocJson(Out, S->getBeginLoc(), SM);
 
-Out << ", \"stmt_point_kind\": ";
-if (getAs())
-  Out << "\"PreStmt\"";
+Out << ", \"stmt_point_kind\": \"";
+if (getAs())
+  Out << "PreLoad";
+else if (getAs())
+  Out << "PreStore";
+else if (getAs())
+  Out << "PostAllocatorCall";
+else if (getAs())
+  Out << "PostCondition";
 else if (getAs())
-  Out << "\"PostLoad\"";
-else if (getAs())
-  Out << "\"PostStore\"";
+  Out << "PostLoad";
 else if (getAs())
-  Out << "\"PostLValue\"";
-else if (getAs())
-  Out << "\"PostAllocatorCall\"";
-else
-  Out << "null";
+  Out << "PostLValue";
+else if (getAs())
+  Out << "PostStore";
+else if (getAs())
+  Out << "PostStmt";
+else if (getAs())
+  Out << "PostStmtPurgeDeadSymbols";
+else if (getAs())
+  Out << "PreStmtPurgeDeadSymbols";
+else if (getAs())
+  Out << "PreStmt";
+else {
+  Out << "\nKind: '" << getKind();
+  llvm_unreachable("' is unhandled StmtPoint kind!");
+}
 
+Out << '\"';
 break;
   }
   }


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

[PATCH] D62888: [NewPM] Port Sancov

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 204333.
leonardchan edited the summary of this revision.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62888

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/sancov-new-pm.c
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/include/llvm/Transforms/Instrumentation/SanitizerCoverage.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/test/Instrumentation/SanitizerCoverage/abort-in-entry-block.ll
  llvm/test/Instrumentation/SanitizerCoverage/backedge-pruning.ll
  llvm/test/Instrumentation/SanitizerCoverage/chains.ll
  llvm/test/Instrumentation/SanitizerCoverage/cmp-tracing-api-x86_32.ll
  llvm/test/Instrumentation/SanitizerCoverage/cmp-tracing-api-x86_64.ll
  llvm/test/Instrumentation/SanitizerCoverage/cmp-tracing.ll
  llvm/test/Instrumentation/SanitizerCoverage/coff-comdat.ll
  
llvm/test/Instrumentation/SanitizerCoverage/coff-pc-table-inline-8bit-counters.ll
  llvm/test/Instrumentation/SanitizerCoverage/coff-used-ctor.ll
  llvm/test/Instrumentation/SanitizerCoverage/const-cmp-tracing.ll
  llvm/test/Instrumentation/SanitizerCoverage/coverage-dbg.ll
  llvm/test/Instrumentation/SanitizerCoverage/coverage.ll
  llvm/test/Instrumentation/SanitizerCoverage/coverage2-dbg.ll
  llvm/test/Instrumentation/SanitizerCoverage/div-tracing.ll
  llvm/test/Instrumentation/SanitizerCoverage/gep-tracing.ll
  llvm/test/Instrumentation/SanitizerCoverage/inline-8bit-counters.ll
  llvm/test/Instrumentation/SanitizerCoverage/interposable-symbol-nocomdat.ll
  llvm/test/Instrumentation/SanitizerCoverage/no-func.ll
  llvm/test/Instrumentation/SanitizerCoverage/pc-table.ll
  llvm/test/Instrumentation/SanitizerCoverage/postdominator_check.ll
  llvm/test/Instrumentation/SanitizerCoverage/seh.ll
  
llvm/test/Instrumentation/SanitizerCoverage/stack-depth-variable-declared-by-user.ll
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
  llvm/test/Instrumentation/SanitizerCoverage/switch-tracing.ll
  llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-comdat.ll
  
llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-8bit-counters.ll
  llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-nocomdat.ll
  llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
  llvm/test/Instrumentation/SanitizerCoverage/tracing.ll
  llvm/test/Instrumentation/SanitizerCoverage/unreachable-critedge.ll
  llvm/test/Instrumentation/SanitizerCoverage/wineh.ll

Index: llvm/test/Instrumentation/SanitizerCoverage/wineh.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/wineh.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/wineh.ll
@@ -1,4 +1,5 @@
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc -S | FileCheck %s --check-prefix=CHECK
+; RUN: opt < %s -passes='module(sancov-module),function(sancov-func)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc -S | FileCheck %s --check-prefix=CHECK
 
 ; Generated from this C++ source:
 ; $ clang -O2 t.cpp -S -emit-llvm
Index: llvm/test/Instrumentation/SanitizerCoverage/unreachable-critedge.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/unreachable-critedge.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/unreachable-critedge.ll
@@ -1,4 +1,5 @@
 ; RUN: opt < %s -S -sancov -sanitizer-coverage-level=3 | FileCheck %s
+; RUN: opt < %s -S -passes='module(sancov-module),function(sancov-func)' -sanitizer-coverage-level=3 | FileCheck %s
 
 ; The critical edges to unreachable_bb should not be split.
 define i32 @foo(i32 %c, i32 %d) {
Index: llvm/test/Instrumentation/SanitizerCoverage/tracing.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/tracing.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/tracing.ll
@@ -3,6 +3,10 @@
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S | FileCheck %s --check-prefix=CHECK_PC_GUARD
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S -mtriple=x86_64-apple-macosx | FileCheck %s --check-prefix=CHECK_PC_GUARD_DARWIN
 
+; RUN: opt < %s -passes='module(sancov-module),function(sancov-func)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc  -S | FileCheck %s --check-prefix=CHECK_PC
+; RUN: opt < %s -passes='module(sancov-module),function(sancov-func)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S | FileCheck %s --check-prefix=CHECK_PC_GUARD
+; RUN: opt < %s -passes='module(sancov-module),function(sancov-func)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S 

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D62926#1539191 , @NoQ wrote:

> All right, it seems that i'm completely misunderstanding this problem and 
> we've been talking past each other this whole time.
>
> The problem is not that we need to skip the `CXXConstructExpr`. The problem 
> is that we need to skip `CXXNewExpr` (!!). CFG elements for an operator-new 
> call go like this:


I really wanted to create a generic approach without any overhead. I have added 
the proper test case what is going on: we enter into another contexts and that 
should be modeled properly. I was not sure about how to do so, but after a 
while it has been born. It contains all of your ideas from all the comments and 
now we only bypassing CXXNewExprs.


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

https://reviews.llvm.org/D62926



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

[PATCH] D62888: [NewPM] Port Sancov

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D62888#1537233 , @chandlerc wrote:

> I would just change this to have the module pass loop over the functions -- 
> that seems like it'll be much cleaner.
>
> As it is, I'm not seeing where the loop actually happens. But rather than 
> trying to figure that out, just seems better to turn it into an explicit 
> loop. That way you can get rid of all the check analysis overhead I think.


Done. This simplifies the patch a lot more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62888



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

[PATCH] D62988: Add an attribute to allow fields of non-trivial types in C unions

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

rsmith added a comment.

In D62988#1539230 , @ahatanak wrote:

> In D62988#1538577 , @rsmith wrote:
>
> > How do you write correct (non-leaking, non-double-freeing, 
> > non-releasing-invalid-pointers) code with this attribute? For example, 
> > suppose I have a `__strong` union member: does storing to it release the 
> > old value (which might be a different union member)? If so, how do you work 
> > around that? 
> > https://clang.llvm.org/docs/AutomaticReferenceCounting.html#ownership-qualified-fields-of-structs-and-unions
> >  should be updated to say what happens. If manual reference counting code 
> > is required to make any use of this feature correct (which seems 
> > superficially likely), is this a better programming model than 
> > `__unsafe_unretained`?
>
>
> I should first make it clear that this attribute is a dangerous feature that 
> makes it easier for users to write incorrect code if they aren't careful.
>
> To write correct code, users have to manually (I don't mean 
> manual-retain-release, this is compiled with ARC) do assignments to the 
> fields that are annotated with the attribute to patch up the code generated 
> by the compiler since the compiler isn't generating the kind of special 
> functions it generates for non-trivial structs. For example:
>
>   union U1 {
> id __weak __attribute__((non_trivial_union_member)) a;
> id __attribute__((non_trivial_union_member)) b;
>   };
>
>   id getObj(int);
>  
>   void foo() {
> union U1 u1 = { .b = 0}; // zero-initialize 'b'.
> u1.b = getObj(1); // assign to __strong field 'b'.
> u1.b = getObj(2); // retain and assign another object to 'b' and release 
> the previously referenced object.


This is what I expected and what I was worried about. I could be missing 
something, but this approach appears to me to cause this feature to not be 
useful in many cases. If a store to a union member now first reads from (and 
releases) that union member, then you cannot change the active union member to 
a member with lifetime type, because that would trigger a load of an inactive 
union member with undefined behavior. For example, consider:

  union U {
float f;
__attribute__((non_trivial_union_member)) id b;
  };
  void f() {
union U u = {.f = 1.0f};
u.b = getObj(1); // undefined behavior: tries to release existing value of 
u.b, which is not active union member; the implied `[u.b release]` is going to 
do Very Bad Things.

In C, storing to a union member is the way you change the active member, and I 
can't off-hand think of any sensible workaround for this problem. But maybe I 
overlooked it: can you demonstrate how you would correctly write the last line 
of the above example? If that can't be done reasonably, I do not think we 
should add this attribute.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62988



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

[PATCH] D61989: [clang-tidy] enable modernize-concat-nested-namespaces on header files

[Xiao Shi via Phabricator via cfe-commits]

shixiao added a comment.

Ping =) @JonasToth


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61989



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

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 204331.
Charusso marked an inline comment as done.

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

https://reviews.llvm.org/D62926

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/inlining/placement-new-fp-suppression.cpp
  clang/test/Analysis/new-ctor-null-throw.cpp
  clang/test/Analysis/new-ctor-null.cpp

Index: clang/test/Analysis/new-ctor-null.cpp
===
--- clang/test/Analysis/new-ctor-null.cpp
+++ clang/test/Analysis/new-ctor-null.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,debug.ExprInspection \
+// RUN:  -verify %s
 
 void clang_analyzer_eval(bool);
 void clang_analyzer_warnIfReached();
@@ -24,7 +26,8 @@
 
 void testArrays() {
   S *s = new S[10]; // no-crash
-  s[0].x = 2; // expected-warning{{Dereference of null pointer}}
+  s[0].x = 2;
+  // no-warning: 'Dereference of null pointer' suppressed by ReturnVisitor.
 }
 
 int global;
Index: clang/test/Analysis/new-ctor-null-throw.cpp
===
--- clang/test/Analysis/new-ctor-null-throw.cpp
+++ clang/test/Analysis/new-ctor-null-throw.cpp
@@ -1,4 +1,9 @@
-// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-config suppress-null-return-paths=false \
+// RUN:  -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -DSUPPRESSED \
+// RUN:  -verify %s
 
 void clang_analyzer_eval(bool);
 
@@ -9,18 +14,41 @@
 // operator new.
 void *operator new(size_t size) {
   return nullptr;
+  // expected-warning@-1 {{'operator new' should not return a null pointer unless it is declared 'throw()' or 'noexcept'}}
 }
 void *operator new[](size_t size) {
   return nullptr;
+  // expected-warning@-1 {{'operator new[]' should not return a null pointer unless it is declared 'throw()' or 'noexcept'}}
 }
 
 struct S {
   int x;
   S() : x(1) {}
   ~S() {}
+  int getX() const { return x; }
 };
 
 void testArrays() {
   S *s = new S[10]; // no-crash
-  s[0].x = 2; // expected-warning{{Dereference of null pointer}}
+  s[0].x = 2;
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Dereference of null pointer}}
+#endif
 }
+
+void testCtor() {
+  S *s = new S();
+  s->x = 13;
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Access to field 'x' results in a dereference of a null pointer (loaded from variable 's')}}
+#endif
+}
+
+void testMethod() {
+  S *s = new S();
+  const int X = s->getX();
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Called C++ object pointer is null}}
+#endif
+}
+
Index: clang/test/Analysis/inlining/placement-new-fp-suppression.cpp
===
--- /dev/null
+++ clang/test/Analysis/inlining/placement-new-fp-suppression.cpp
@@ -0,0 +1,137 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core.CallAndMessage \
+// RUN:  -analyzer-config suppress-null-return-paths=false \
+// RUN:  -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core.CallAndMessage \
+// RUN:  -DSUPPRESSED \
+// RUN:  -verify %s
+
+#ifdef SUPPRESSED
+// expected-no-diagnostics
+#endif
+
+#include "../Inputs/system-header-simulator-cxx.h"
+
+typedef unsigned long uintptr_t;
+
+void error();
+void *malloc(size_t);
+
+
+// From llvm/include/llvm/Support/MathExtras.h
+inline uintptr_t alignAddr(const void *Addr, size_t Alignment) {
+  return (((uintptr_t)Addr + Alignment - 1) & ~(uintptr_t)(Alignment - 1));
+}
+
+inline size_t alignmentAdjustment(const void *Ptr, size_t Alignment) {
+  return alignAddr(Ptr, Alignment) - (uintptr_t)Ptr;
+}
+
+
+// From llvm/include/llvm/Support/MemAlloc.h
+inline void *safe_malloc(size_t Sz) {
+  void *Result = malloc(Sz);
+  if (Result == nullptr)
+error();
+
+  return Result;
+}
+
+
+// From llvm/include/llvm/Support/Allocator.h
+class MallocAllocator {
+public:
+  void *Allocate(size_t Size, size_t /*Alignment*/) {
+return safe_malloc(Size);
+  }
+};
+
+class BumpPtrAllocator {
+public:
+  void *Allocate(size_t Size, size_t Alignment) {
+BytesAllocated += Size;
+size_t Adjustment = alignmentAdjustment(CurPtr, Alignment);
+size_t SizeToAllocate = Size;
+
+size_t PaddedSize = SizeToAllocate + Alignment - 1;
+uintptr_t AlignedAddr = alignAddr(Allocator.Allocate(PaddedSize, 0),
+  Alignment);
+char *AlignedPtr = (char*)AlignedAddr;
+
+return AlignedPtr;
+  }
+
+private:
+  char *CurPtr = nullptr;
+  size_t BytesAllocated = 0;
+  MallocAllocator Allocator;
+};
+
+
+// From clang/include/clang/AST/ASTContextAllocate.h
+class ASTContext;
+
+void *operator 

Re: [PATCH] D6833: Clang-format: Braces Indent Style Whitesmith

[JVApen via cfe-commits]

Hello Tim,

I am no longer working on this. I've spent my time convincing my colleagues
that clang-format is useful enough to change our braces, with success.

I remember also being stuck on the enumerations. Good luck!

On Wed, Jun 12, 2019, 04:16 Tim Wojtulewicz via Phabricator <
revi...@reviews.llvm.org> wrote:

> timwoj added a comment.
>
> I updated this patch to remove all of the code from `ContinuationIndenter`
> and to use the newer `BraceWrapping` style option instead of setting each
> one individually in `UnwrappedLineParser`.
>
> I still have the same issue with enums. Looking at the `RootToken` in
> `UnwrappedLineFormatter::formatFirstToken` when parsing an enum, the first
> pass through it has the entire line for the enum in it with all of the
> newlines stripped from my test case. It then doesn't ever return the braces
> as separate lines. If someone could give me a pointer as to how/why enums
> are tokenized differently from everything else, I can likely fix it. The
> test case I'm using looks like:
>
>   verifyFormat("enum X\n"
>"  {\n"
>"  Y = 0,\n"
>"  }\n",
>WhitesmithsBraceStyle);
>
> I also have the issue with a `break` after a block inside of a case
> statement, as mentioned in https://reviews.llvm.org/D6833#107539. Other
> than those two, it's mostly working correctly. I removed the test for
> lambdas because I'm not entirely sure how that should even be formatted
> with Whitesmiths, but I can add it back in again.
>
> Should I update this thread with the new diff against HEAD, or should I
> open a new one and close this one as dead?
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D6833/new/
>
> https://reviews.llvm.org/D6833
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63009: [OpenMP] Add target task alloc function with device ID

[Alexandre Eichenberger via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D63009



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

[PATCH] D63222: [Clangd] Fixed clangd diagnostics priority

[Shaurya Gupta via Phabricator via cfe-commits]

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

- Fixed diagnostics where zero width inserted ranges were being used instead of 
the whole token
  - Added unit tests


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63222

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1,3 +1,4 @@
+
 //===--- DiagnosticsTests.cpp *- 
C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
@@ -101,6 +102,7 @@
   Annotations Test(R"cpp(
 namespace test{};
 void $decl[[foo]]();
+class T{$explicit[[]]$constructor[[T]](int a);};
 int main() {
   $typo[[go\
 o]]();
@@ -112,8 +114,10 @@
   test::$nomembernamespace[[test]];
 }
   )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  TU.ClangTidyChecks = "-*,google-explicit-constructor";
   EXPECT_THAT(
-  TestTU::withCode(Test.code()).build().getDiagnostics(),
+  TU.build().getDiagnostics(),
   ElementsAre(
   // This range spans lines.
   AllOf(Diag(Test.range("typo"),
@@ -135,7 +139,13 @@
"of type 'const char [4]'"),
   Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
   Diag(Test.range("nomembernamespace"),
-   "no member named 'test' in namespace 'test'")));
+   "no member named 'test' in namespace 'test'"),
+  // We make sure here that the entire token is highlighted
+  AllOf(Diag(Test.range("constructor"),
+ "single-argument constructors must be marked explicit to "
+ "avoid unintentional implicit conversions"),
+WithFix(Fix(Test.range("explicit"), "explicit ",
+"insert 'explicit '");
 }
 
 TEST(DiagnosticsTest, FlagsMatter) {
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -18,6 +18,7 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
@@ -102,11 +103,21 @@
 if (R.getBegin() == R.getEnd() && Loc == R.getBegin())
   FallbackRange = halfOpenToRange(M, R);
   }
-  if (FallbackRange)
-return *FallbackRange;
-  // If no suitable range is found, just use the token at the location.
+  // If the token at location is not a comment, use the token.
+  // Otherwise use zero width insertion range
   auto R = Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Loc), M, L);
-  if (!R.isValid()) // Fall back to location only, let the editor deal with it.
+  if (R.isValid()) {
+// check if token at location is a priority i.e. not a comment
+Token token;
+bool isTokenPriority = false;
+if(!Lexer::getRawToken(Loc, token, M, L, true))
+  isTokenPriority = token.getKind() != tok::comment;
+if (!isTokenPriority && FallbackRange)
+  return *FallbackRange;
+  }
+  else if (FallbackRange)
+return *FallbackRange;
+  else // Fall back to location only, let the editor deal with it.
 R = CharSourceRange::getCharRange(Loc);
   return halfOpenToRange(M, R);
 }


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1,3 +1,4 @@
+
 //===--- DiagnosticsTests.cpp *- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
@@ -101,6 +102,7 @@
   Annotations Test(R"cpp(
 namespace test{};
 void $decl[[foo]]();
+class T{$explicit[[]]$constructor[[T]](int a);};
 int main() {
   $typo[[go\
 o]]();
@@ -112,8 +114,10 @@
   test::$nomembernamespace[[test]];
 }
   )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  TU.ClangTidyChecks = "-*,google-explicit-constructor";
   EXPECT_THAT(
-  TestTU::withCode(Test.code()).build().getDiagnostics(),
+  TU.build().getDiagnostics(),
   ElementsAre(
   // This range spans lines.
   AllOf(Diag(Test.range("typo"),
@@ -135,7 +139,13 @@
"of type 'const char [4]'"),
   Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
   

[PATCH] D63009: [OpenMP] Add target task alloc function with device ID

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 204320.
gtbercea added a comment.

- Fix tests.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63009

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  test/OpenMP/target_depend_codegen.cpp
  test/OpenMP/target_enter_data_depend_codegen.cpp
  test/OpenMP/target_exit_data_depend_codegen.cpp
  test/OpenMP/target_parallel_depend_codegen.cpp
  test/OpenMP/target_parallel_for_depend_codegen.cpp
  test/OpenMP/target_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_simd_depend_codegen.cpp
  test/OpenMP/target_teams_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
  test/OpenMP/target_update_depend_codegen.cpp

Index: test/OpenMP/target_update_depend_codegen.cpp
===
--- test/OpenMP/target_update_depend_codegen.cpp
+++ test/OpenMP/target_update_depend_codegen.cpp
@@ -63,7 +63,7 @@
   // CK1: [[CAP_DEVICE:%.+]] = getelementptr inbounds %struct.anon, %struct.anon* [[CAPTURES:%.+]], i32 0, i32 0
   // CK1: [[DEVICE:%.+]] = load i32, i32* %{{.+}}
   // CK1: store i32 [[DEVICE]], i32* [[CAP_DEVICE]],
-  // CK1: [[RES:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* {{.+}}, i32 {{.+}}, i32 1, i[[sz]] [[sz]], i[[sz]] 4, i32 (i32, i8*)* bitcast (i32 (i32, %struct.kmp_task_t_with_privates*)* [[TASK_ENTRY0:@.+]] to i32 (i32, i8*)*))
+  // CK1: [[RES:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* {{.+}}, i32 {{.+}}, i32 1, i[[sz]] [[sz]], i[[sz]] 4, i32 (i32, i8*)* bitcast (i32 (i32, %struct.kmp_task_t_with_privates*)* [[TASK_ENTRY0:@.+]] to i32 (i32, i8*)*), i64
   // CK1: [[BC:%.+]] = bitcast i8* [[RES]] to %struct.kmp_task_t_with_privates*
   // CK1: [[TASK_T:%.+]] = getelementptr inbounds %struct.kmp_task_t_with_privates, %struct.kmp_task_t_with_privates* [[BC]], i32 0, i32 0
   // CK1: [[SHAREDS:%.+]] = getelementptr inbounds %struct.kmp_task_t, %struct.kmp_task_t* [[TASK_T]], i32 0, i32 0
Index: test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
===
--- test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
+++ test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
@@ -133,7 +133,7 @@
   // CHECK:   [[DEV:%.+]] = load i32, i32* [[DEVICE_CAP]],
   // CHECK:   store i32 [[DEV]], i32* [[GEP]],
 
-  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{104|52}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1_:@.+]] to i32 (i32, i8*)*))
+  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{104|52}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1_:@.+]] to i32 (i32, i8*)*), i64
   // CHECK:   [[BC_TASK:%.+]] = bitcast i8* [[TASK]] to [[TASK_TY1_:%.+]]*
   // CHECK:   getelementptr inbounds [3 x %struct.kmp_depend_info], [3 x %struct.kmp_depend_info]* %{{.+}}, i[[SZ]] 0, i[[SZ]] 0
   // CHECK:   getelementptr inbounds [3 x %struct.kmp_depend_info], [3 x %struct.kmp_depend_info]* %{{.+}}, i[[SZ]] 0, i[[SZ]] 1
@@ -149,7 +149,7 @@
   // CHECK:   [[DEV:%.+]] = load i32, i32* [[DEVICE_CAP]],
   // CHECK:   store i32 [[DEV]], i32* [[GEP]],
 
-  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{56|28}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1__:@.+]] to i32 (i32, i8*)*))
+  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{56|28}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1__:@.+]] to i32 (i32, i8*)*), i64
   // CHECK:   [[BC_TASK:%.+]] = bitcast i8* [[TASK]] to [[TASK_TY1__:%.+]]*
   // CHECK:   getelementptr inbounds [3 x %struct.kmp_depend_info], [3 x %struct.kmp_depend_info]* %{{.+}}, i[[SZ]] 0, i[[SZ]] 0
   // CHECK:   getelementptr inbounds [3 x %struct.kmp_depend_info], [3 x %struct.kmp_depend_info]* %{{.+}}, i[[SZ]] 0, i[[SZ]] 1
Index: test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
===
--- test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
+++ test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
@@ -133,7 +133,7 @@
   // CHECK:   [[DEV:%.+]] = load i32, i32* [[DEVICE_CAP]],
   // CHECK:   store i32 [[DEV]], i32* [[GEP]],
 
-  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* [[ID]], i32 

[PATCH] D33841: [clang-tidy] redundant 'extern' keyword check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/readability/RedundantExternCheck.cpp:43-44
+
+  if (FD->getBeginLoc().isMacroID())
+return;
+

koldaniel wrote:
> lebedev.ri wrote:
> > Similarly, do this in `registerMatchers()`
> Could you please help me how could I achieve that? I did not find any 
> matchers which match for macros.
You can simply add your own matchers, see e.g. `AST_MATCHER()`'s in  
`AvoidCArraysCheck.cpp`.


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

https://reviews.llvm.org/D33841



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

[PATCH] D63221: Fixed clangd diagnostics priority

[Shaurya Gupta via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63221

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1,3 +1,4 @@
+
 //===--- DiagnosticsTests.cpp *- 
C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
@@ -101,6 +102,7 @@
   Annotations Test(R"cpp(
 namespace test{};
 void $decl[[foo]]();
+class T{$explicit[[]]$constructor[[T]](int a);};
 int main() {
   $typo[[go\
 o]]();
@@ -112,8 +114,10 @@
   test::$nomembernamespace[[test]];
 }
   )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  TU.ClangTidyChecks = "-*,google-explicit-constructor";
   EXPECT_THAT(
-  TestTU::withCode(Test.code()).build().getDiagnostics(),
+  TU.build().getDiagnostics(),
   ElementsAre(
   // This range spans lines.
   AllOf(Diag(Test.range("typo"),
@@ -135,7 +139,13 @@
"of type 'const char [4]'"),
   Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
   Diag(Test.range("nomembernamespace"),
-   "no member named 'test' in namespace 'test'")));
+   "no member named 'test' in namespace 'test'"),
+  // We make sure here that the entire token is highlighted
+  AllOf(Diag(Test.range("constructor"),
+ "single-argument constructors must be marked explicit to "
+ "avoid unintentional implicit conversions"),
+WithFix(Fix(Test.range("explicit"), "explicit ",
+"insert 'explicit '");
 }
 
 TEST(DiagnosticsTest, FlagsMatter) {
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -18,6 +18,7 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
@@ -102,11 +103,22 @@
 if (R.getBegin() == R.getEnd() && Loc == R.getBegin())
   FallbackRange = halfOpenToRange(M, R);
   }
-  if (FallbackRange)
-return *FallbackRange;
-  // If no suitable range is found, just use the token at the location.
+  // If the token at location is not a comment, use the token.
+  // Otherwise use zero width insertion range
   auto R = Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Loc), M, L);
-  if (!R.isValid()) // Fall back to location only, let the editor deal with it.
+  if (R.isValid()) {
+// check if token at location is a priority i.e. not a comment
+Token token;
+bool isTokenPriority = false;
+if(!Lexer::getRawToken(Loc, token, M, L, true))
+  isTokenPriority = token.getKind() != tok::comment;
+if (isTokenPriority) {}
+else if (!isTokenPriority && FallbackRange)
+  return *FallbackRange;
+  }
+  else if (FallbackRange)
+return *FallbackRange;
+  else // Fall back to location only, let the editor deal with it.
 R = CharSourceRange::getCharRange(Loc);
   return halfOpenToRange(M, R);
 }


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1,3 +1,4 @@
+
 //===--- DiagnosticsTests.cpp *- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
@@ -101,6 +102,7 @@
   Annotations Test(R"cpp(
 namespace test{};
 void $decl[[foo]]();
+class T{$explicit[[]]$constructor[[T]](int a);};
 int main() {
   $typo[[go\
 o]]();
@@ -112,8 +114,10 @@
   test::$nomembernamespace[[test]];
 }
   )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  TU.ClangTidyChecks = "-*,google-explicit-constructor";
   EXPECT_THAT(
-  TestTU::withCode(Test.code()).build().getDiagnostics(),
+  TU.build().getDiagnostics(),
   ElementsAre(
   // This range spans lines.
   AllOf(Diag(Test.range("typo"),
@@ -135,7 +139,13 @@
"of type 'const char [4]'"),
   Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
   Diag(Test.range("nomembernamespace"),
-   "no member named 'test' in namespace 'test'")));
+   "no member named 'test' in 

[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

[Alex Strelnikov via Phabricator via cfe-commits]

astrelni updated this revision to Diff 204318.
astrelni added a comment.

Fix diff issues again


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

https://reviews.llvm.org/D62977

Files:
  clang-tools-extra/clang-tidy/google/CMakeLists.txt
  clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/google-upgrade-googletest-case.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/Inputs/gtest/gtest-typed-test.h
  clang-tools-extra/test/clang-tidy/Inputs/gtest/gtest.h
  clang-tools-extra/test/clang-tidy/google-upgrade-googletest-case.cpp
  llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/google/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/google/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/google/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/google/BUILD.gn
@@ -28,6 +28,7 @@
 "OverloadedUnaryAndCheck.cpp",
 "TodoCommentCheck.cpp",
 "UnnamedNamespaceInHeaderCheck.cpp",
+"UpgradeGoogletestCaseCheck.cpp",
 "UsingNamespaceDirectiveCheck.cpp",
   ]
 }
Index: clang-tools-extra/test/clang-tidy/google-upgrade-googletest-case.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-upgrade-googletest-case.cpp
@@ -0,0 +1,991 @@
+// RUN: %check_clang_tidy %s google-upgrade-googletest-case %t -- -- -I%S/Inputs
+
+#include "gtest/gtest.h"
+
+// 
+// Macros
+
+TYPED_TEST_CASE(FooTest, FooTypes);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+// CHECK-FIXES: TYPED_TEST_SUITE(FooTest, FooTypes);
+TYPED_TEST_CASE_P(FooTest);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+// CHECK-FIXES: TYPED_TEST_SUITE_P(FooTest);
+REGISTER_TYPED_TEST_CASE_P(FooTest, FooTestName);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+// CHECK-FIXES: REGISTER_TYPED_TEST_SUITE_P(FooTest, FooTestName);
+INSTANTIATE_TYPED_TEST_CASE_P(FooPrefix, FooTest, FooTypes);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+// CHECK-FIXES: INSTANTIATE_TYPED_TEST_SUITE_P(FooPrefix, FooTest, FooTypes);
+
+#ifdef TYPED_TEST_CASE
+// CHECK-MESSAGES: [[@LINE-1]]:2: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#undef TYPED_TEST_CASE
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#define TYPED_TEST_CASE TYPED_TEST_SUITE
+#endif
+
+#ifdef TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:2: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#undef TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#define TYPED_TEST_CASE_P TYPED_TEST_SUITE_P
+#endif
+
+#ifdef REGISTER_TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:2: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#undef REGISTER_TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#define REGISTER_TYPED_TEST_CASE_P REGISTER_TYPED_TEST_SUITE_P
+#endif
+
+#ifdef INSTANTIATE_TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:2: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#undef INSTANTIATE_TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#define INSTANTIATE_TYPED_TEST_CASE_P INSTANTIATE_TYPED_TEST_SUITE_P
+#endif
+
+TYPED_TEST_CASE(FooTest, FooTypes);
+TYPED_TEST_CASE_P(FooTest);
+REGISTER_TYPED_TEST_CASE_P(FooTest, FooTestName);
+INSTANTIATE_TYPED_TEST_CASE_P(FooPrefix, FooTest, FooTypes);
+
+// 
+// testing::Test
+
+class FooTest : public testing::Test {
+public:
+  static void SetUpTestCase();
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+  // CHECK-FIXES: static void SetUpTestSuite();
+  static void TearDownTestCase();
+  // CHECK-MESSAGES: 

[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

[Alex Strelnikov via Phabricator via cfe-commits]

astrelni updated this revision to Diff 204317.
astrelni added a comment.

Fix mistake not uploading full diff


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

https://reviews.llvm.org/D62977

Files:
  clang-tools-extra/clang-tidy/google/CMakeLists.txt
  clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/google-upgrade-googletest-case.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/Inputs/gtest/gtest-typed-test.h
  clang-tools-extra/test/clang-tidy/Inputs/gtest/gtest.h
  clang-tools-extra/test/clang-tidy/google-upgrade-googletest-case.cpp
  llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/google/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/google/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/google/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/google/BUILD.gn
@@ -28,6 +28,7 @@
 "OverloadedUnaryAndCheck.cpp",
 "TodoCommentCheck.cpp",
 "UnnamedNamespaceInHeaderCheck.cpp",
+"UpgradeGoogletestCaseCheck.cpp",
 "UsingNamespaceDirectiveCheck.cpp",
   ]
 }
Index: clang-tools-extra/test/clang-tidy/google-upgrade-googletest-case.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-upgrade-googletest-case.cpp
@@ -0,0 +1,991 @@
+// RUN: %check_clang_tidy %s google-upgrade-googletest-case %t -- -- -I%S/Inputs
+
+#include "gtest/gtest.h"
+
+// 
+// Macros
+
+TYPED_TEST_CASE(FooTest, FooTypes);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+// CHECK-FIXES: TYPED_TEST_SUITE(FooTest, FooTypes);
+TYPED_TEST_CASE_P(FooTest);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+// CHECK-FIXES: TYPED_TEST_SUITE_P(FooTest);
+REGISTER_TYPED_TEST_CASE_P(FooTest, FooTestName);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+// CHECK-FIXES: REGISTER_TYPED_TEST_SUITE_P(FooTest, FooTestName);
+INSTANTIATE_TYPED_TEST_CASE_P(FooPrefix, FooTest, FooTypes);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+// CHECK-FIXES: INSTANTIATE_TYPED_TEST_SUITE_P(FooPrefix, FooTest, FooTypes);
+
+#ifdef TYPED_TEST_CASE
+// CHECK-MESSAGES: [[@LINE-1]]:2: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#undef TYPED_TEST_CASE
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#define TYPED_TEST_CASE TYPED_TEST_SUITE
+#endif
+
+#ifdef TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:2: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#undef TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#define TYPED_TEST_CASE_P TYPED_TEST_SUITE_P
+#endif
+
+#ifdef REGISTER_TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:2: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#undef REGISTER_TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#define REGISTER_TYPED_TEST_CASE_P REGISTER_TYPED_TEST_SUITE_P
+#endif
+
+#ifdef INSTANTIATE_TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:2: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#undef INSTANTIATE_TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#define INSTANTIATE_TYPED_TEST_CASE_P INSTANTIATE_TYPED_TEST_SUITE_P
+#endif
+
+TYPED_TEST_CASE(FooTest, FooTypes);
+TYPED_TEST_CASE_P(FooTest);
+REGISTER_TYPED_TEST_CASE_P(FooTest, FooTestName);
+INSTANTIATE_TYPED_TEST_CASE_P(FooPrefix, FooTest, FooTypes);
+
+// 
+// testing::Test
+
+class FooTest : public testing::Test {
+public:
+  static void SetUpTestCase();
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+  // CHECK-FIXES: static void SetUpTestSuite();
+  static void TearDownTestCase();
+  // 

[PATCH] D33841: [clang-tidy] redundant 'extern' keyword check

[Daniel Kolozsvari via Phabricator via cfe-commits]

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



Comment at: clang-tidy/readability/RedundantExternCheck.cpp:30
+
+  if (FD->getStorageClass() != SC_Extern)
+return;

lebedev.ri wrote:
> Can you do that in `registerMatchers()`?
The only way I found for that is to use the matcher `hasExternalFormalLinkage`, 
but the problem with it is that now the checker warns not only for the 
redundant `extern` usage, but for the unnecessary too - in the latter case the 
linkage is internal, and I think by checking the storage class we can determine 
whether the `extern` keyword has been used or not.



Comment at: clang-tidy/readability/RedundantExternCheck.cpp:43-44
+
+  if (FD->getBeginLoc().isMacroID())
+return;
+

lebedev.ri wrote:
> Similarly, do this in `registerMatchers()`
Could you please help me how could I achieve that? I did not find any matchers 
which match for macros.


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

https://reviews.llvm.org/D33841



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

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1690
+  std::make_pair(frontend::GenerateInterfaceTBEExpV1, false));
+  if (!ProgramActionPair.second)
+Diags.Report(diag::err_drv_invalid_value)

I think you could've used a `llvm::Optional`.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:21
+  StringRef InFile;
+  StringRef Format;
+  std::set ParsedTemplates;

An enumeration may be nicer.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:39
+  bool WriteNamedDecl(const NamedDecl *ND, MangledSymbols , int RDO) {
+if (!(RDO & FromTU))
+  return true;

I tend to prefer `if (RDO & ~FromTU)`



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:56
+auto isVisible = [this](const NamedDecl *ND) -> bool {
+  if (ND->getVisibility() != DefaultVisibility) {
+if (ND->hasAttr())

Why not `if (ND->getVisibility() == DefaultVisibility) return true;`?



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:65
+};
+auto doBail = [this, isVisible](const NamedDecl *ND) -> bool {
+  if (!isVisible(ND))

I think that using something like `ignoreDecl` is better than `doBail`.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:68
+return true;
+  if (const VarDecl *VD = dyn_cast(ND))
+if ((VD->getStorageClass() == StorageClass::SC_Extern) ||

A newline before this would be nice.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:72
+ VD->getParentFunctionOrMethod() == nullptr))
+  return true;
+  if (const FunctionDecl *FD = dyn_cast(ND)) {

If it is a `VarDecl`, it cannot be a `FunctionDecl`.  I think that this can be 
simplified a bit as:

```
if (const auto *VD = dyn_cast(VD))
  return VD->getStorageClass() == StorageClass::SC_Extern ||
  VD->getStorageClass() == StorageClass::SC_Static ||
  VD->getParentFunctionOrMethod() == nullptr;
```



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:73
+  return true;
+  if (const FunctionDecl *FD = dyn_cast(ND)) {
+if (FD->isInlined() && !isa(FD) &&

Newline before this would be nice.  I think that using `auto` here for the type 
is fine as you spelt out the type in the `dyn_cast`.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:77
+  return true;
+if (const CXXMethodDecl *MD = dyn_cast(FD)) {
+  if (const auto *RC = dyn_cast(MD->getParent())) {

I think that flipping this around would be nicer.

```
if (const auto *MD = dyn_cast(FD)) {
  ...
}

return FD->isInlined() && !Instance.getLangOpts().GNUInline;
```



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:79
+  if (const auto *RC = dyn_cast(MD->getParent())) {
+if (const auto *CTD = dyn_cast(RC->getParent()))
+  return true;

`CTD` is not used, please use `isa` and avoid the unused variable warning.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:118
+  return MangledName;
+};
+

Can we compress `getMangledName` and `getMangledNames` into `getMangledNames` 
and always return the vector?  This avoids the duplication and simplifies the 
symbol recording later as well.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:150
+if (IsRDOLate)
+  llvm_unreachable("Generating Interface Stubs is not supported with "
+   "delayed template parsing.");

You can probably hoist this to L129, and avoid the variable definition and the 
check on L131.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

[Alex Strelnikov via Phabricator via cfe-commits]

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

In D62977#1533384 , @Eugene.Zelenko 
wrote:

> I think will be good idea to replace //upgrade// with //modernize// to be 
> consistent with similar checks in other module.


I will work on changing this in the next diff update, if your opinion holds. 
The context for currently using "upgrade" is:

a) The modernize module seems to contain checks that are geared at using more 
modern language features as opposed to API breaking changes. With some 
exceptions, they seem to be about changing from old to new practice while both 
ways remain valid code. The goal of this check is to take code that will break 
with future googletest API changes and turn it into working code, allowing 
users to upgrade the version of googletest they use. Maybe upgrade is not the 
best term, but I don't think modernize is a better fit.
b) The check is meant to be consistent with 
abseil-upgrade-duration-conversions, which is aimed to upgrade code before API 
breaking changes. So if we change this check, I would like to change the Abseil 
one as well. Maybe this would be better done together in a follow-up patch?

I don't feel very strongly, just thought it would be good to provide the 
context. Please let me know how you'd like me to proceed.


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

https://reviews.llvm.org/D62977



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

[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

You need to upload the entire patch, not as compared to the previous patch.
Without seeing the tests - what version checks does this have?
It shouldn't fire if the googletest version is the one before that rename.


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

https://reviews.llvm.org/D62977



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

[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

[Alex Strelnikov via Phabricator via cfe-commits]

astrelni updated this revision to Diff 204313.
astrelni marked 2 inline comments as done.
astrelni added a comment.

Fix space.


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

https://reviews.llvm.org/D62977

Files:
  clang-tools-extra/docs/clang-tidy/checks/google-upgrade-googletest-case.rst


Index: 
clang-tools-extra/docs/clang-tidy/checks/google-upgrade-googletest-case.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/google-upgrade-googletest-case.rst
+++ clang-tools-extra/docs/clang-tidy/checks/google-upgrade-googletest-case.rst
@@ -10,7 +10,7 @@
 meanings of "test case" and "test suite" as used by the International
 Software Testing Qualifications Board and ISO 29119.
 
-The affected APIs are :
+The affected APIs are:
 
 - Member functions of ``testing::Test``, ``testing::TestInfo``,
   ``testing::TestEventListener``, ``testing::UnitTest``, and any type 
inheriting


Index: clang-tools-extra/docs/clang-tidy/checks/google-upgrade-googletest-case.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/google-upgrade-googletest-case.rst
+++ clang-tools-extra/docs/clang-tidy/checks/google-upgrade-googletest-case.rst
@@ -10,7 +10,7 @@
 meanings of "test case" and "test suite" as used by the International
 Software Testing Qualifications Board and ISO 29119.
 
-The affected APIs are :
+The affected APIs are:
 
 - Member functions of ``testing::Test``, ``testing::TestInfo``,
   ``testing::TestEventListener``, ``testing::UnitTest``, and any type inheriting
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

[Alex Strelnikov via Phabricator via cfe-commits]

astrelni updated this revision to Diff 204312.
astrelni added a comment.

Style updates.


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

https://reviews.llvm.org/D62977

Files:
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.h

Index: clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.h
===
--- clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.h
+++ clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.h
@@ -10,7 +10,6 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_UPGRADEGOOGLETESTCASECHECK_H
 
 #include "../ClangTidyCheck.h"
-
 #include 
 
 namespace clang {
Index: clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
+++ clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
@@ -15,13 +15,13 @@
 namespace clang {
 namespace tidy {
 namespace google {
-namespace {
 
-const llvm::StringRef CheckMessage =
+static const llvm::StringRef CheckMessage =
 "Googletest APIs named with 'case' are deprecated; use equivalent APIs "
 "named with 'suite'";
 
-llvm::Optional getNewMacroName(llvm::StringRef MacroName) {
+static llvm::Optional
+getNewMacroName(llvm::StringRef MacroName) {
   std::pair ReplacementMap[] = {
   {"TYPED_TEST_CASE", "TYPED_TEST_SUITE"},
   {"TYPED_TEST_CASE_P", "TYPED_TEST_SUITE_P"},
@@ -38,6 +38,8 @@
   return llvm::None;
 }
 
+namespace {
+
 class UpgradeGoogletestCasePPCallback : public PPCallbacks {
 public:
   UpgradeGoogletestCasePPCallback(UpgradeGoogletestCaseCheck *Check,
@@ -81,7 +83,7 @@
 if (!Replacement)
   return;
 
-StringRef FileName = PP->getSourceManager().getFilename(
+llvm::StringRef FileName = PP->getSourceManager().getFilename(
 MD.getMacroInfo()->getDefinitionLoc());
 if (!FileName.endswith("gtest/gtest-typed-test.h"))
   return;
@@ -165,9 +167,7 @@
   this);
 }
 
-namespace {
-
-llvm::StringRef getNewMethodName(llvm::StringRef CurrentName) {
+static llvm::StringRef getNewMethodName(llvm::StringRef CurrentName) {
   std::pair ReplacementMap[] = {
   {"SetUpTestCase", "SetUpTestSuite"},
   {"TearDownTestCase", "TearDownTestSuite"},
@@ -190,21 +190,22 @@
 }
 
 template 
-bool isInInstantiation(const NodeType ,
-   const MatchFinder::MatchResult ) {
+static bool isInInstantiation(const NodeType ,
+  const MatchFinder::MatchResult ) {
   return !match(isInTemplateInstantiation(), Node, *Result.Context).empty();
 }
 
 template 
-bool isInTemplate(const NodeType ,
-  const MatchFinder::MatchResult ) {
+static bool isInTemplate(const NodeType ,
+ const MatchFinder::MatchResult ) {
   internal::Matcher IsInsideTemplate =
   hasAncestor(decl(anyOf(classTemplateDecl(), functionTemplateDecl(;
   return !match(IsInsideTemplate, Node, *Result.Context).empty();
 }
 
-bool derivedTypeHasReplacementMethod(const MatchFinder::MatchResult ,
- llvm::StringRef ReplacementMethod) {
+static bool
+derivedTypeHasReplacementMethod(const MatchFinder::MatchResult ,
+llvm::StringRef ReplacementMethod) {
   const auto *Class = Result.Nodes.getNodeAs("class");
   return !match(cxxRecordDecl(
 unless(isExpansionInFileMatching(
@@ -214,7 +215,8 @@
   .empty();
 }
 
-CharSourceRange getAliasNameRange(const MatchFinder::MatchResult ) {
+static CharSourceRange
+getAliasNameRange(const MatchFinder::MatchResult ) {
   if (const auto *Using = Result.Nodes.getNodeAs("using")) {
 return CharSourceRange::getTokenRange(
 Using->getNameInfo().getSourceRange());
@@ -223,8 +225,6 @@
   Result.Nodes.getNodeAs("typeloc")->getSourceRange());
 }
 
-} // namespace
-
 void UpgradeGoogletestCaseCheck::check(const MatchFinder::MatchResult ) {
   llvm::StringRef ReplacementText;
   CharSourceRange ReplacementRange;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 204308.
plotfi marked 2 inline comments as done.
plotfi added a comment.

Improving support for visibility correctness with classes, methods and 
inheritance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/hidden-class-inheritance.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/virtual.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-DAG:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-DAG:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-DAG:   - Name:_Z8weakFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_WEAK
+// CHECK-YAML-DAG:   - Name:_Z10strongFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_GLOBAL
+
+// CHECK-SYMBOLS-DAG: _Z10strongFuncv
+// CHECK-SYMBOLS-DAG: _Z8weakFuncv
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() { return 42; }
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-readelf -s - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK-DAG: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK-DAG: _Z10cmdVisiblev
+void cmdVisible() {}
+
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z10cmdVisiblev
+// CHECK-SYMBOLS-DAG: HIDDEN {{.*}} _Z6hiddenv
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z9nothiddenv
Index: clang/test/InterfaceStubs/virtual.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/virtual.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck -check-prefix=CHECK-TAPI %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | \
+// RUN: llvm-readelf -s - 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+#define HIDDEN  __attribute__((__visibility__(("hidden"
+#define DEFAULT __attribute__((__visibility__(("default"
+
+// 

[PATCH] D63062: [clang-format] Added New Style Rule: OnePerLineBitFieldDecl

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Nit: if you going for "OnePerLine" in the name rather than "Break" can you keep 
it consistent with `ConstructorInitializerAllOnOneLineOrOnePerLine` and put the 
type first and then the operation i.e.  `BitFieldOnePerLine` (this will help 
keep all BitField options together if there are ever more than one),  I also 
think either don't use `Decl` at all or use the full word `Declarations`  as in 
`AlignConsecutiveDeclarations` , `AlwaysBreakTemplateDeclarations` etc


Repository:
  rC Clang

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

https://reviews.llvm.org/D63062



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

[PATCH] D63009: [OpenMP] Add target task alloc function with device ID

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 204294.
gtbercea added a comment.

- Add device ID if available.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63009

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  test/OpenMP/target_depend_codegen.cpp
  test/OpenMP/target_enter_data_depend_codegen.cpp
  test/OpenMP/target_exit_data_depend_codegen.cpp
  test/OpenMP/target_parallel_depend_codegen.cpp
  test/OpenMP/target_parallel_for_depend_codegen.cpp
  test/OpenMP/target_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_simd_depend_codegen.cpp
  test/OpenMP/target_teams_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
  test/OpenMP/target_update_depend_codegen.cpp

Index: test/OpenMP/target_update_depend_codegen.cpp
===
--- test/OpenMP/target_update_depend_codegen.cpp
+++ test/OpenMP/target_update_depend_codegen.cpp
@@ -63,7 +63,7 @@
   // CK1: [[CAP_DEVICE:%.+]] = getelementptr inbounds %struct.anon, %struct.anon* [[CAPTURES:%.+]], i32 0, i32 0
   // CK1: [[DEVICE:%.+]] = load i32, i32* %{{.+}}
   // CK1: store i32 [[DEVICE]], i32* [[CAP_DEVICE]],
-  // CK1: [[RES:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* {{.+}}, i32 {{.+}}, i32 1, i[[sz]] [[sz]], i[[sz]] 4, i32 (i32, i8*)* bitcast (i32 (i32, %struct.kmp_task_t_with_privates*)* [[TASK_ENTRY0:@.+]] to i32 (i32, i8*)*))
+  // CK1: [[RES:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* {{.+}}, i32 {{.+}}, i32 1, i[[sz]] [[sz]], i[[sz]] 4, i32 (i32, i8*)* bitcast (i32 (i32, %struct.kmp_task_t_with_privates*)* [[TASK_ENTRY0:@.+]] to i32 (i32, i8*)*), i64 -1)
   // CK1: [[BC:%.+]] = bitcast i8* [[RES]] to %struct.kmp_task_t_with_privates*
   // CK1: [[TASK_T:%.+]] = getelementptr inbounds %struct.kmp_task_t_with_privates, %struct.kmp_task_t_with_privates* [[BC]], i32 0, i32 0
   // CK1: [[SHAREDS:%.+]] = getelementptr inbounds %struct.kmp_task_t, %struct.kmp_task_t* [[TASK_T]], i32 0, i32 0
Index: test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
===
--- test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
+++ test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
@@ -133,7 +133,7 @@
   // CHECK:   [[DEV:%.+]] = load i32, i32* [[DEVICE_CAP]],
   // CHECK:   store i32 [[DEV]], i32* [[GEP]],
 
-  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{104|52}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1_:@.+]] to i32 (i32, i8*)*))
+  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{104|52}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1_:@.+]] to i32 (i32, i8*)*), i64 -1)
   // CHECK:   [[BC_TASK:%.+]] = bitcast i8* [[TASK]] to [[TASK_TY1_:%.+]]*
   // CHECK:   getelementptr inbounds [3 x %struct.kmp_depend_info], [3 x %struct.kmp_depend_info]* %{{.+}}, i[[SZ]] 0, i[[SZ]] 0
   // CHECK:   getelementptr inbounds [3 x %struct.kmp_depend_info], [3 x %struct.kmp_depend_info]* %{{.+}}, i[[SZ]] 0, i[[SZ]] 1
@@ -149,7 +149,7 @@
   // CHECK:   [[DEV:%.+]] = load i32, i32* [[DEVICE_CAP]],
   // CHECK:   store i32 [[DEV]], i32* [[GEP]],
 
-  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{56|28}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1__:@.+]] to i32 (i32, i8*)*))
+  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{56|28}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1__:@.+]] to i32 (i32, i8*)*), i64 -1)
   // CHECK:   [[BC_TASK:%.+]] = bitcast i8* [[TASK]] to [[TASK_TY1__:%.+]]*
   // CHECK:   getelementptr inbounds [3 x %struct.kmp_depend_info], [3 x %struct.kmp_depend_info]* %{{.+}}, i[[SZ]] 0, i[[SZ]] 0
   // CHECK:   getelementptr inbounds [3 x %struct.kmp_depend_info], [3 x %struct.kmp_depend_info]* %{{.+}}, i[[SZ]] 0, i[[SZ]] 1
Index: test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
===
--- test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
+++ test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
@@ -133,7 +133,7 @@
   // CHECK:   [[DEV:%.+]] = load i32, i32* [[DEVICE_CAP]],
   // CHECK:   store i32 [[DEV]], i32* [[GEP]],
 
-  // CHECK:   [[TASK:%.+]] = call i8* 

[PATCH] D58880: [clangd] Type hierarchy subtypes

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

mostly LG, thanks!




Comment at: clang-tools-extra/clangd/XRefs.cpp:1057
+  auto  = S.Definition ? S.Definition : S.CanonicalDeclaration;
+  Start.line = CD.Start.line();
+  Start.character = CD.Start.column();

nit: could we directly use `THI.range.start.line` (same for the following 3 
lines)



Comment at: clang-tools-extra/clangd/XRefs.cpp:1061
+  End.character = CD.End.column();
+  // TODO: How to get entire range like in declToTypeHierarchyItem()?
+  THI.range = {Start, End};

a few different approaches that comes to my mind:
- store the full range in index.
- check AST cache to see if we have AST for `CD.FileURI`, and use that decl.
- build AST for `CD.FileURI`
- heuristically parse `CD.FileURI`


Could you create a bug report in https://github.com/clangd/clangd/issues ?



Comment at: clang-tools-extra/clangd/XRefs.cpp:1064
+  THI.selectionRange = {Start, End};
+  // TODO: Reuse code between here and getWorkspaceSymbols().
+  auto Uri = URI::parse(CD.FileURI);

yeah it would be great to have a `Location symbolToLocation(const Symbol& S);`



Comment at: clang-tools-extra/clangd/XRefs.cpp:1071
+  }
+  // TODO: Pass in ClangdServer::WorkspaceRoot as a HintPath.
+  StringRef HintPath;

you do not need WorkspaceRoot, you can pass the translationunit 
`getTypeHierarchy` was triggered on. Which is available in `File` parameter. 

Also it always exists whereas `WorkspaceRoot` might be missing depending on the 
editor.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1212
+if (Index) {
+  if (Optional ID = getSymbolID(CXXRD)) {
+fillSubTypes(*ID, *Result->children, Index, ResolveLevels);

nit: braces(just the inner one, keep the outer one)



Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:459
+  Request.AnyScope = true;
+  int ResultCount = 0;
+  Index->fuzzyFind(Request, [&](const Symbol ) {

`bool GotResult = false;`



Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:462
+if (TemplateArgs == S.TemplateSpecializationArgs) {
+  Result = S.ID;
+  ++ResultCount;

```
EXPECT_FALSE(GotResult);
GotResult = true;
```



Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:466
+  });
+  EXPECT_EQ(1, ResultCount);
+  return Result;

`EXPECT_TRUE(GotResult);`



Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:470
+
+std::vector collectSubtypes(SymbolID Type, SymbolIndex *Index) {
+  std::vector Result;

Maybe call it `Subject` instead of `Type` ?



Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:478
+
+TEST(Subtypes, SimpleInheritance) {
+  Annotations Source(R"cpp(

could you add another child that derives `Parent` to check multiple children 
case?



Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:479
+TEST(Subtypes, SimpleInheritance) {
+  Annotations Source(R"cpp(
+struct Parent {

could you get rid of member fields and put definitions into single lines to 
make test smaller?



Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:505
+TEST(Subtypes, MultipleInheritance) {
+  Annotations Source(R"cpp(
+struct Parent1 {

same suggestion for member fields and single line definitions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58880



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

[PATCH] D63149: Added AST matcher for ignoring elidable constructors

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/unittests/ASTMatchers/ASTMatchersTest.h:69
+  CXX17OrLater,
+  CXX2AOrLater
+};

Cxx2aOrLater?

(no need to uppercase things)



Comment at: clang/unittests/ASTMatchers/ASTMatchersTest.h:159
+switch (Mode) {
+case LanguageMode::CXX11OrLater:
+case LanguageMode::CXX11:

The "orlater" variants should not appear here, please use "default: 
llvm_unreachable()".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63149



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

[PATCH] D63149: Added AST matcher for ignoring elidable constructors

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 204282.
jvikstrom marked 7 inline comments as done.
jvikstrom added a comment.

- Using switch for choosing language standard


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63149

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTest.h

Index: clang/unittests/ASTMatchers/ASTMatchersTest.h
===
--- clang/unittests/ASTMatchers/ASTMatchersTest.h
+++ clang/unittests/ASTMatchers/ASTMatchersTest.h
@@ -13,6 +13,7 @@
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Tooling/Tooling.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang {
 namespace ast_matchers {
@@ -57,6 +58,17 @@
   const std::unique_ptr FindResultReviewer;
 };
 
+enum class LanguageMode {
+  CXX11,
+  CXX14,
+  CXX17,
+  CXX2A,
+  CXX11OrLater,
+  CXX14OrLater,
+  CXX17OrLater,
+  CXX2AOrLater
+};
+
 template 
 testing::AssertionResult matchesConditionally(
 const std::string , const T , bool ExpectMatch,
@@ -116,14 +128,73 @@
 }
 
 template 
-testing::AssertionResult matches(const std::string , const T ) {
-  return matchesConditionally(Code, AMatcher, true, "-std=c++11");
+testing::AssertionResult
+matchesConditionally(const std::string , const T ,
+ bool ExpectMatch, const LanguageMode ) {
+  std::vector LangModes;
+  switch (Mode) {
+  case LanguageMode::CXX11:
+  case LanguageMode::CXX14:
+  case LanguageMode::CXX17:
+  case LanguageMode::CXX2A:
+LangModes = {Mode};
+break;
+  case LanguageMode::CXX11OrLater:
+LangModes = {LanguageMode::CXX11, LanguageMode::CXX14, LanguageMode::CXX17,
+ LanguageMode::CXX2A};
+break;
+  case LanguageMode::CXX14OrLater:
+LangModes = {LanguageMode::CXX14, LanguageMode::CXX17, LanguageMode::CXX2A};
+break;
+  case LanguageMode::CXX17OrLater:
+LangModes = {LanguageMode::CXX17, LanguageMode::CXX2A};
+break;
+  case LanguageMode::CXX2AOrLater:
+LangModes = {LanguageMode::CXX2A};
+  }
+
+  for (auto Mode : LangModes) {
+std::string LangModeArg;
+switch (Mode) {
+case LanguageMode::CXX11OrLater:
+case LanguageMode::CXX11:
+  LangModeArg = "-std=c++11";
+  break;
+case LanguageMode::CXX14OrLater:
+case LanguageMode::CXX14:
+  LangModeArg = "-std=c++14";
+  break;
+case LanguageMode::CXX17OrLater:
+case LanguageMode::CXX17:
+  LangModeArg = "-std=c++17";
+  break;
+case LanguageMode::CXX2AOrLater:
+case LanguageMode::CXX2A:
+  LangModeArg = "-std=c++2a";
+}
+
+auto Result =
+matchesConditionally(Code, AMatcher, ExpectMatch, LangModeArg);
+if (!Result) {
+  return Result;
+}
+  }
+
+  return testing::AssertionSuccess();
 }
 
 template 
-testing::AssertionResult notMatches(const std::string ,
-const T ) {
-  return matchesConditionally(Code, AMatcher, false, "-std=c++11");
+testing::AssertionResult
+matches(const std::string , const T ,
+const LanguageMode  = LanguageMode::CXX11) {
+  return matchesConditionally(Code, AMatcher, true, Modes);
+}
+
+template 
+testing::AssertionResult
+notMatches(const std::string , const T ,
+   const LanguageMode  = LanguageMode::CXX11) {
+  return matchesConditionally(Code, AMatcher, false, Modes);
 }
 
 template 
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -566,6 +566,74 @@
llvm::make_unique>("x")));
 }
 
+TEST(Matcher, IgnoresElidableConstructors) {
+  EXPECT_TRUE(
+  matches("struct H {};"
+  "template H B(T A);"
+  "void f() {"
+  "  H D1;"
+  "  D1 = B(B(1));"
+  "}",
+  cxxOperatorCallExpr(hasArgument(
+  1, callExpr(hasArgument(
+ 0, ignoringElidableConstructorCall(callExpr()),
+  LanguageMode::CXX11OrLater));
+  EXPECT_TRUE(
+  matches("struct H {};"
+  "template H B(T A);"
+  "void f() {"
+  "  H D1;"
+  "  D1 = B(1);"
+  "}",
+  cxxOperatorCallExpr(hasArgument(
+  1, callExpr(hasArgument(0, ignoringElidableConstructorCall(
+ integerLiteral()),
+  LanguageMode::CXX11OrLater));
+  EXPECT_TRUE(matches(
+  "struct H {};"
+  "H G();"
+  "void f() {"
+  "  H D = G();"
+  "}",
+  varDecl(hasInitializer(anyOf(
+  

[PATCH] D62839: [clangd] Index API and implementations for relations

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

LGTM, except the batch query support.




Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:97
+
+SymbolSlab indexHeaderSymbols(ASTContext , std::shared_ptr 
PP,
+  const CanonicalIncludes ) {

we should have only a couple call sites to this function. Let's rather change 
those instead of introducing a new helper.



Comment at: clang-tools-extra/clangd/index/Index.h:77
+struct RelationsRequest {
+  SymbolID Subject;
+  index::SymbolRole Predicate;

sorry for missing it in previous iteration. but this should also enable batch 
queries. let's make this one a `llvm::DenseMap`.
And in the callback we should output both the `SymbolID` of the `Subject` and 
`Object`.

We have a network based index(internally) and it uses this batch query 
optimization to get rid of network latency.



Comment at: clang-tools-extra/clangd/index/Index.h:113
 
+  /// Finds all symbol IDs 'Object' such that (Req.Subject, Req.Predicate,
+  /// Object) forms a relation stored in the index,  and applies \p Callback

I think the old comment is better. No need to expose internals.



Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:244
  llvm::function_ref Callback) const {
   trace::Span Tracer("Dex lookup");
   for (const auto  : Req.IDs) {

could you revert these changes?



Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:268
+llvm::function_ref Callback) const {
+  LookupRequest LookupReq;
+  uint32_t Remaining =

can you also add a span

```
trace::Span Tracer("Dex relations");
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62839



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

[PATCH] D63149: Added AST matcher for ignoring elidable constructors

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:600
+  EXPECT_TRUE(matches(code2, matcher2, LanguageMode::CXX11OrLater));
+  EXPECT_TRUE(matches(code3, matcher3, LanguageMode::CXX11OrLater));
+}

Please inline the variables that are used once.



Comment at: clang/unittests/ASTMatchers/ASTMatchersTest.h:61
+enum class LanguageMode : int {
+  CXX11 = 0,
+  CXX14 = 1,

Please drop the explicit values.



Comment at: clang/unittests/ASTMatchers/ASTMatchersTest.h:64
+  CXX17 = 2,
+  CXX20 = 3,
+  CXX11OrLater,

2a



Comment at: clang/unittests/ASTMatchers/ASTMatchersTest.h:68
+  CXX17OrLater,
+  CXX20OrLater
+};

"2a" (we don't know the year yet)



Comment at: clang/unittests/ASTMatchers/ASTMatchersTest.h:144
+  case LanguageMode::CXX11OrLater:
+LangModes = {LanguageMode::CXX11, LanguageMode::CXX14, 
LanguageMode::CXX17, LanguageMode::CXX20};
+break;

80 columns.



Comment at: clang/unittests/ASTMatchers/ASTMatchersTest.h:154
+LangModes = {LanguageMode::CXX20};
+  };
+

No need for semicolon.



Comment at: clang/unittests/ASTMatchers/ASTMatchersTest.h:157
+  for (auto Mode : LangModes) {
+auto LangModeArg = LangModeStrings[static_cast(Mode)];
+auto Result =

I'd say it is too clever, and suggest to write an explicit switch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63149



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

[PATCH] D63194: [clangd] Link in target infos and pass target and mode while invoking driver

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/test/target_info.test:8
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -e "s|file://\([A-Z]\):/|file:///\1:/|g" %t.test.1 > %t.test
+

kadircet wrote:
> ilya-biryukov wrote:
> > This is added unconditionally on all platforms, right?
> > Why not just add this extra slash directly into the input file?
> no it is not, this checks whether we have a drive letter in the file uri 
> ("[A-Z]:") and adds the slash in that case.
Ah, ok, LGTM. Missed that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63194



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

[PATCH] D63194: [clangd] Link in target infos and pass target and mode while invoking driver

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:36
   Cmd.CommandLine = ArgsAdjuster(Cmd.CommandLine, Cmd.Filename);
+  tooling::addTargetAndModeForProgramName(Cmd.CommandLine, Cmd.CommandLine[0]);
   // Inject the resource dir.

kadircet wrote:
> ilya-biryukov wrote:
> > Do we actually need this if we provide a proper `Argv[0]` to the driver?
> > My understanding is that the driver should figure out the targets from 
> > custom binary names on its own.
> unfortunately yes. 
> 
> because if the "-target" is missing, driver tries to deduce it from the 
> "running binary" which is "clangd" in our case.
I wonder if it would sense to change this? Where does this code live?

That's exactly what `argv[0]` is for in `compile_commands.json`. Fixing that at 
the driver level would "fix" all libclang- and libtooling-based binaries, like 
`clang-tidy`, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63194



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

[PATCH] D63194: [clangd] Link in target infos and pass target and mode while invoking driver

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 204278.
kadircet marked 3 inline comments as done.
kadircet added a comment.

- Add comments to the test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63194

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/test/target_info.test
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -24,6 +24,7 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -329,6 +330,7 @@
   using namespace clang;
   using namespace clang::clangd;
 
+  llvm::InitializeAllTargetInfos();
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream ) {
 OS << clang::getClangToolFullVersion("clangd") << "\n";
Index: clang-tools-extra/clangd/test/target_info.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/target_info.test
@@ -0,0 +1,36 @@
+# Mock 'compile_commands.json' to contain a driver name targeting fuchsia OS.
+# Afterwards check that correct target is passed into clang.
+
+# RUN: rm -rf %t.dir && mkdir -p %t.dir
+
+# RUN: echo '[{"directory": "%/t.dir", "command": 
"%/t.dir/x86_64-fuchsia-clang -x c++ the-file.cpp", "file": "the-file.cpp"}]' > 
%t.dir/compile_commands.json
+
+# RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
+# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -e "s|file://\([A-Z]\):/|file:///\1:/|g" %t.test.1 > %t.test
+
+# RUN: clangd -lit-test < %t.test 2>&1 | FileCheck -strict-whitespace %t.test
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
+---
+{
+  "jsonrpc":"2.0",
+  "method":"textDocument/didOpen",
+  "params": {
+"textDocument": {
+  "uri": "file://INPUT_DIR/the-file.cpp",
+  "languageId":"cpp",
+  "version":1,
+  "text":""
+}
+  }
+}
+# Make sure we have "-target" set in clangd, which is printed in TUScheduler
+# with a log like "Updating file ... with command ..."
+# CHECK: Updating file {{.*}} with command {{.*}}
+# CHECK-NEXT: [INPUT_DIR]
+# CHECK-NEXT: INPUT_DIR/x86_64-fuchsia-clang -target x86_64-fuchsia
+---
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -11,6 +11,7 @@
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -32,6 +33,7 @@
 tooling::getClangSyntaxOnlyAdjuster()));
 
   Cmd.CommandLine = ArgsAdjuster(Cmd.CommandLine, Cmd.Filename);
+  tooling::addTargetAndModeForProgramName(Cmd.CommandLine, Cmd.CommandLine[0]);
   // Inject the resource dir.
   // FIXME: Don't overwrite it if it's already there.
   if (!ResourceDir.empty())
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -21,6 +21,7 @@
 
 set(LLVM_LINK_COMPONENTS
   Support
+  AllTargetsInfos
   )
 
 if(CLANG_BUILT_STANDALONE)


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -24,6 +24,7 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -329,6 +330,7 @@
   using namespace clang;
   using namespace clang::clangd;
 
+  llvm::InitializeAllTargetInfos();
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream ) {
 OS << clang::getClangToolFullVersion("clangd") << "\n";
Index: clang-tools-extra/clangd/test/target_info.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/target_info.test
@@ -0,0 +1,36 @@
+# Mock 'compile_commands.json' to contain a driver name targeting 

[PATCH] D63194: [clangd] Link in target infos and pass target and mode while invoking driver

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:36
   Cmd.CommandLine = ArgsAdjuster(Cmd.CommandLine, Cmd.Filename);
+  tooling::addTargetAndModeForProgramName(Cmd.CommandLine, Cmd.CommandLine[0]);
   // Inject the resource dir.

ilya-biryukov wrote:
> Do we actually need this if we provide a proper `Argv[0]` to the driver?
> My understanding is that the driver should figure out the targets from custom 
> binary names on its own.
unfortunately yes. 

because if the "-target" is missing, driver tries to deduce it from the 
"running binary" which is "clangd" in our case.



Comment at: clang-tools-extra/clangd/test/target_info.test:8
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -e "s|file://\([A-Z]\):/|file:///\1:/|g" %t.test.1 > %t.test
+

ilya-biryukov wrote:
> This is added unconditionally on all platforms, right?
> Why not just add this extra slash directly into the input file?
no it is not, this checks whether we have a drive letter in the file uri 
("[A-Z]:") and adds the slash in that case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63194



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

[PATCH] D63149: Added AST matcher for ignoring elidable constructors

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 204277.
jvikstrom marked 10 inline comments as done.
jvikstrom added a comment.

- Fixed wrong formatting in test code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63149

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTest.h

Index: clang/unittests/ASTMatchers/ASTMatchersTest.h
===
--- clang/unittests/ASTMatchers/ASTMatchersTest.h
+++ clang/unittests/ASTMatchers/ASTMatchersTest.h
@@ -57,6 +57,17 @@
   const std::unique_ptr FindResultReviewer;
 };
 
+enum class LanguageMode : int {
+  CXX11 = 0,
+  CXX14 = 1,
+  CXX17 = 2,
+  CXX20 = 3,
+  CXX11OrLater,
+  CXX14OrLater,
+  CXX17OrLater,
+  CXX20OrLater
+};
+
 template 
 testing::AssertionResult matchesConditionally(
 const std::string , const T , bool ExpectMatch,
@@ -116,14 +127,56 @@
 }
 
 template 
-testing::AssertionResult matches(const std::string , const T ) {
-  return matchesConditionally(Code, AMatcher, true, "-std=c++11");
+testing::AssertionResult
+matchesConditionally(const std::string , const T ,
+ bool ExpectMatch, const LanguageMode ) {
+  std::vector LangModeStrings{"-std=c++11", "-std=c++14",
+   "-std=c++17", "-std=c++20"};
+  std::vector LangModes;
+  switch (Mode) {
+  case LanguageMode::CXX11:
+  case LanguageMode::CXX14:
+  case LanguageMode::CXX17:
+  case LanguageMode::CXX20:
+LangModes = {Mode};
+break;
+  case LanguageMode::CXX11OrLater:
+LangModes = {LanguageMode::CXX11, LanguageMode::CXX14, LanguageMode::CXX17, LanguageMode::CXX20};
+break;
+  case LanguageMode::CXX14OrLater:
+LangModes = {LanguageMode::CXX14, LanguageMode::CXX17, LanguageMode::CXX20};
+break;
+  case LanguageMode::CXX17OrLater:
+LangModes = {LanguageMode::CXX17, LanguageMode::CXX20};
+break;
+  case LanguageMode::CXX20OrLater:
+LangModes = {LanguageMode::CXX20};
+  };
+
+  for (auto Mode : LangModes) {
+auto LangModeArg = LangModeStrings[static_cast(Mode)];
+auto Result =
+matchesConditionally(Code, AMatcher, ExpectMatch, LangModeArg);
+if (!Result) {
+  return Result;
+}
+  }
+
+  return testing::AssertionSuccess();
 }
 
 template 
-testing::AssertionResult notMatches(const std::string ,
-const T ) {
-  return matchesConditionally(Code, AMatcher, false, "-std=c++11");
+testing::AssertionResult
+matches(const std::string , const T ,
+const LanguageMode  = LanguageMode::CXX11) {
+  return matchesConditionally(Code, AMatcher, true, Modes);
+}
+
+template 
+testing::AssertionResult
+notMatches(const std::string , const T ,
+   const LanguageMode  = LanguageMode::CXX11) {
+  return matchesConditionally(Code, AMatcher, false, Modes);
 }
 
 template 
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -566,6 +566,73 @@
llvm::make_unique>("x")));
 }
 
+TEST(Matcher, IgnoresElidableConstructors) {
+  auto matcher1 = cxxOperatorCallExpr(hasArgument(
+  1,
+  callExpr(hasArgument(0, ignoringElidableConstructorCall(callExpr());
+  auto matcher2 = cxxOperatorCallExpr(
+  hasArgument(1, callExpr(hasArgument(0, ignoringElidableConstructorCall(
+ integerLiteral());
+  auto matcher3 = varDecl(hasInitializer(anyOf(
+  ignoringElidableConstructorCall(callExpr()),
+  exprWithCleanups(has(ignoringElidableConstructorCall(callExpr()));
+
+  auto code1 = "struct H {};"
+   "template H B(T A);"
+   "void f() {"
+   "  H D1;"
+   "  D1 = B(B(1));"
+   "}";
+  auto code2 = "struct H {};"
+   "template H B(T A);"
+   "void f() {"
+   "  H D1;"
+   "  D1 = B(1);"
+   "}";
+  auto code3 = "struct H {};"
+   "H G();"
+   "void f() {"
+   "  H D = G();"
+   "}";
+
+  EXPECT_TRUE(matches(code1, matcher1, LanguageMode::CXX11OrLater));
+  EXPECT_TRUE(matches(code2, matcher2, LanguageMode::CXX11OrLater));
+  EXPECT_TRUE(matches(code3, matcher3, LanguageMode::CXX11OrLater));
+}
+
+TEST(Matcher, IgnoresElidableInReturn) {
+  auto matcher = expr(ignoringElidableConstructorCall(declRefExpr()));
+  EXPECT_TRUE(matches("struct H {};"
+  "H f() {"
+  "  H g;"
+  "  return g;"
+  "}",

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: lib/CodeGen/BackendUtil.cpp:1345
   Conf.RemarksPasses = CGOpts.OptRecordPasses;
-  Conf.DwoPath = CGOpts.SplitDwarfFile;
+  Conf.DwoPath = CGOpts.SplitDwarfOutput;
   switch (Action) {

aaronpuchert wrote:
> aaronpuchert wrote:
> > @pcc Your documentation for `DwoPath` suggests that this should be the 
> > actual output filename. However, the test that you added together with this 
> > line in rC333677 doesn't fail whatever garbage I write into that field 
> > here. What can I add to that so that it fails when we don't do the right 
> > thing here?
> @pcc Could you (or perhaps @tejohnson) comment on what the intended behavior 
> is here, and how I can change the test so that it fails when I do the wrong 
> thing? Is this the name of the file we write the split debug info to, or is 
> it the value we use for the DW_AT_[GNU_]dwo_name attribute in the skeleton CU?
It is the name of the file the split debug info is written to. If you test by 
changing the file name given to the -split-dwarf-file option in 
test/CodeGen/thinlto-split-dwarf.c, make sure you clean up the old one in your 
test output directory. When I tested it just now, it initially still passed 
because I had an old one sitting in the output directory from a prior run. Once 
I removed that it failed with the new name (without changing the corresponding 
llvm-readobj invocation).


Repository:
  rC Clang

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

https://reviews.llvm.org/D59673



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

[PATCH] D63194: [clangd] Link in target infos and pass target and mode while invoking driver

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:36
   Cmd.CommandLine = ArgsAdjuster(Cmd.CommandLine, Cmd.Filename);
+  tooling::addTargetAndModeForProgramName(Cmd.CommandLine, Cmd.CommandLine[0]);
   // Inject the resource dir.

Do we actually need this if we provide a proper `Argv[0]` to the driver?
My understanding is that the driver should figure out the targets from custom 
binary names on its own.



Comment at: clang-tools-extra/clangd/test/target_info.test:1
+# RUN: rm -rf %t.dir && mkdir -p %t.dir
+

Could you add a short human-readable description of the test setup here?
Reading the bash scripts is not fun. Something like
```
Try to mock 'compile_commnands.json' for fuchsia-based builds and check the 
corresponding target flags
are being passed into clang.
```



Comment at: clang-tools-extra/clangd/test/target_info.test:8
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -e "s|file://\([A-Z]\):/|file:///\1:/|g" %t.test.1 > %t.test
+

This is added unconditionally on all platforms, right?
Why not just add this extra slash directly into the input file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63194



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

[PATCH] D63194: [clangd] Link in target infos and pass target and mode while invoking driver

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

This enables clangd to pick up default include search and predefines
for toolchains defined in clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63194

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/test/target_info.test
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -24,6 +24,7 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -329,6 +330,7 @@
   using namespace clang;
   using namespace clang::clangd;
 
+  llvm::InitializeAllTargetInfos();
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream ) {
 OS << clang::getClangToolFullVersion("clangd") << "\n";
Index: clang-tools-extra/clangd/test/target_info.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/target_info.test
@@ -0,0 +1,33 @@
+# RUN: rm -rf %t.dir && mkdir -p %t.dir
+
+# RUN: echo '[{"directory": "%/t.dir", "command": 
"%/t.dir/x86_64-fuchsia-clang -x c++ the-file.cpp", "file": "the-file.cpp"}]' > 
%t.dir/compile_commands.json
+
+# RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
+# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -e "s|file://\([A-Z]\):/|file:///\1:/|g" %t.test.1 > %t.test
+
+# RUN: clangd -lit-test < %t.test 2>&1 | FileCheck -strict-whitespace %t.test
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
+---
+{
+  "jsonrpc":"2.0",
+  "method":"textDocument/didOpen",
+  "params": {
+"textDocument": {
+  "uri": "file://INPUT_DIR/the-file.cpp",
+  "languageId":"cpp",
+  "version":1,
+  "text":""
+}
+  }
+}
+# Make sure we have "-target" set in clangd, which is printed in TUScheduler
+# with a log like "Updating file ... with command ..."
+# CHECK: Updating file {{.*}} with command {{.*}}
+# CHECK-NEXT: [INPUT_DIR]
+# CHECK-NEXT: INPUT_DIR/x86_64-fuchsia-clang -target x86_64-fuchsia
+---
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -11,6 +11,7 @@
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -32,6 +33,7 @@
 tooling::getClangSyntaxOnlyAdjuster()));
 
   Cmd.CommandLine = ArgsAdjuster(Cmd.CommandLine, Cmd.Filename);
+  tooling::addTargetAndModeForProgramName(Cmd.CommandLine, Cmd.CommandLine[0]);
   // Inject the resource dir.
   // FIXME: Don't overwrite it if it's already there.
   if (!ResourceDir.empty())
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -21,6 +21,7 @@
 
 set(LLVM_LINK_COMPONENTS
   Support
+  AllTargetsInfos
   )
 
 if(CLANG_BUILT_STANDALONE)


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -24,6 +24,7 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -329,6 +330,7 @@
   using namespace clang;
   using namespace clang::clangd;
 
+  llvm::InitializeAllTargetInfos();
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream ) {
 OS << clang::getClangToolFullVersion("clangd") << "\n";
Index: clang-tools-extra/clangd/test/target_info.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/target_info.test
@@ -0,0 +1,33 @@
+# RUN: rm -rf %t.dir && mkdir -p %t.dir
+
+# RUN: echo '[{"directory": "%/t.dir", "command": "%/t.dir/x86_64-fuchsia-clang -x c++ 

[PATCH] D63149: Added AST matcher for ignoring elidable constructors

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 204269.
jvikstrom marked 3 inline comments as done.
jvikstrom added a comment.

- Added match conditionally overload to control in what language standard a 
match should run in


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63149

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTest.h

Index: clang/unittests/ASTMatchers/ASTMatchersTest.h
===
--- clang/unittests/ASTMatchers/ASTMatchersTest.h
+++ clang/unittests/ASTMatchers/ASTMatchersTest.h
@@ -57,6 +57,17 @@
   const std::unique_ptr FindResultReviewer;
 };
 
+enum class LanguageMode : int {
+  CXX11 = 0,
+  CXX14 = 1,
+  CXX17 = 2,
+  CXX20 = 3,
+  CXX11OrLater,
+  CXX14OrLater,
+  CXX17OrLater,
+  CXX20OrLater
+};
+
 template 
 testing::AssertionResult matchesConditionally(
 const std::string , const T , bool ExpectMatch,
@@ -116,14 +127,56 @@
 }
 
 template 
-testing::AssertionResult matches(const std::string , const T ) {
-  return matchesConditionally(Code, AMatcher, true, "-std=c++11");
+testing::AssertionResult
+matchesConditionally(const std::string , const T ,
+ bool ExpectMatch, const LanguageMode ) {
+  std::vector LangModeStrings{"-std=c++11", "-std=c++14",
+   "-std=c++17", "-std=c++20"};
+  std::vector LangModes;
+  switch (Mode) {
+  case LanguageMode::CXX11:
+  case LanguageMode::CXX14:
+  case LanguageMode::CXX17:
+  case LanguageMode::CXX20:
+LangModes = {Mode};
+break;
+  case LanguageMode::CXX11OrLater:
+LangModes = {LanguageMode::CXX11, LanguageMode::CXX14, LanguageMode::CXX17, LanguageMode::CXX20};
+break;
+  case LanguageMode::CXX14OrLater:
+LangModes = {LanguageMode::CXX14, LanguageMode::CXX17, LanguageMode::CXX20};
+break;
+  case LanguageMode::CXX17OrLater:
+LangModes = {LanguageMode::CXX17, LanguageMode::CXX20};
+break;
+  case LanguageMode::CXX20OrLater:
+LangModes = {LanguageMode::CXX20};
+  };
+
+  for (auto Mode : LangModes) {
+auto LangModeArg = LangModeStrings[static_cast(Mode)];
+auto Result =
+matchesConditionally(Code, AMatcher, ExpectMatch, LangModeArg);
+if (!Result) {
+  return Result;
+}
+  }
+
+  return testing::AssertionSuccess();
 }
 
 template 
-testing::AssertionResult notMatches(const std::string ,
-const T ) {
-  return matchesConditionally(Code, AMatcher, false, "-std=c++11");
+testing::AssertionResult
+matches(const std::string , const T ,
+const LanguageMode  = LanguageMode::CXX11) {
+  return matchesConditionally(Code, AMatcher, true, Modes);
+}
+
+template 
+testing::AssertionResult
+notMatches(const std::string , const T ,
+   const LanguageMode  = LanguageMode::CXX11) {
+  return matchesConditionally(Code, AMatcher, false, Modes);
 }
 
 template 
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -566,6 +566,76 @@
llvm::make_unique>("x")));
 }
 
+TEST(Matcher, IgnoresElidableConstructors) {
+  auto matcher1 = cxxOperatorCallExpr(hasArgument(
+  1, callExpr(hasArgument(
+ 0, ignoringElidableConstructorCall(callExpr());
+  auto matcher2 = cxxOperatorCallExpr(hasArgument(
+  1, callExpr(hasArgument(0, ignoringElidableConstructorCall(
+ integerLiteral());
+  auto matcher3 =
+  varDecl(hasInitializer(
+anyOf(
+ignoringElidableConstructorCall(callExpr()),
+exprWithCleanups(has(ignoringElidableConstructorCall(callExpr(
+)));
+
+  auto code1 = "struct H {};"
+   "template H B(T A);"
+   "void f() {"
+   "  H D1;"
+   "  D1 = B(B(1));"
+   "}";
+  auto code2 = "struct H {};"
+   "template H B(T A);"
+   "void f() {"
+   "  H D1;"
+   "  D1 = B(1);"
+   "}";
+  auto code3 = "struct H {};"
+   "H G();"
+   "void f() {"
+   "  H D = G();"
+   "}";
+
+  EXPECT_TRUE(matches(code1, matcher1, LanguageMode::CXX11OrLater));
+  EXPECT_TRUE(matches(code2, matcher2, LanguageMode::CXX11OrLater));
+  EXPECT_TRUE(matches(code3, matcher3, LanguageMode::CXX11OrLater));
+}
+
+TEST(Matcher, IgnoresElidableInReturn) {
+  auto matcher = expr(ignoringElidableConstructorCall(declRefExpr()));
+  EXPECT_TRUE(matches("struct H{};"
+  "H f() {"
+ 

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a subscriber: tejohnson.
aaronpuchert added inline comments.



Comment at: lib/CodeGen/BackendUtil.cpp:1345
   Conf.RemarksPasses = CGOpts.OptRecordPasses;
-  Conf.DwoPath = CGOpts.SplitDwarfFile;
+  Conf.DwoPath = CGOpts.SplitDwarfOutput;
   switch (Action) {

aaronpuchert wrote:
> @pcc Your documentation for `DwoPath` suggests that this should be the actual 
> output filename. However, the test that you added together with this line in 
> rC333677 doesn't fail whatever garbage I write into that field here. What can 
> I add to that so that it fails when we don't do the right thing here?
@pcc Could you (or perhaps @tejohnson) comment on what the intended behavior is 
here, and how I can change the test so that it fails when I do the wrong thing? 
Is this the name of the file we write the split debug info to, or is it the 
value we use for the DW_AT_[GNU_]dwo_name attribute in the skeleton CU?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59673



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

[PATCH] D63167: [Clang] Remove obsolete -enable-split-dwarf={single,split}

[Aaron Puchert via Phabricator via cfe-commits]

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



Comment at: include/clang/Basic/CodeGenOptions.def:263
 
-ENUM_CODEGENOPT(SplitDwarfMode, DwarfFissionKind, 2, NoFission) ///< DWARF 
fission mode to use.
+CODEGENOPT(EnableSplitDwarf, 1, 0) ///< Whether to enable split DWARF.
 

dblaikie wrote:
> Could we skip this and rely on "SplitDwarFile" being non-empty?
I think not, have a look at test/CodeGen/split-debug-filename.c:

```
// RUN: %clang_cc1 -debug-info-kind=limited -split-dwarf-file foo.dwo -S 
-emit-llvm -o - %s | FileCheck %s
// RUN: %clang_cc1 -debug-info-kind=limited -enable-split-dwarf 
-split-dwarf-file foo.dwo -S -emit-llvm -o - %s | FileCheck 
--check-prefix=VANILLA %s
// ...

// Testing to ensure that the dwo name gets output into the compile unit.
// CHECK: !DICompileUnit({{.*}}, splitDebugFilename: "foo.dwo"

// Testing to ensure that the dwo name is not output into the compile unit if
// it's for vanilla split-dwarf rather than split-dwarf for implicit modules.
// VANILLA-NOT: splitDebugFilename
```

This seems intentional.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63167



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

[PATCH] D62804: [clangd] Enable extraction of system includes from custom toolchains

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D62804#1538202 , @ilya-biryukov 
wrote:

> In D62804#1538155 , @kadircet wrote:
>
> > For example a gcc cross compiling to arm comes with its own system includes 
> > and has some mechanisms to discover that implicitly, without requiring any 
> > "-I" flags.
> >  Hence when used with clangd, we make use of system includes instead of the 
> > target's include library. This patch aims to solve that issue, tools like 
> > cquery also handles that problem in a similar way.
>
>
> That's exactly what driver is about. The approach is slightly different, 
> though. Instead of executing a binary, one has to mimic the toolchain search 
> logic of a particular toolchain by hand.
>  In addition to includes, it also handles adding the corresponding `-D` flags 
> and anything else that the cross-compile toolchain does. Is this toolchain 
> not currently supported by the driver? Is adding it so much work that we 
> would choose to workaround like this instead?


As discussed offline, this is also a possible way to go. but looks like it 
requires making sure every toolchain has correct logic to deduce include paths 
which is more work and could become outdated, in addition to that it is also 
possible to introduce breakages due to new search logic. whereas this solution 
fixes the problem for all target.
So we decided to keep it clangd specific.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62804



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

[PATCH] D62899: [analyzer][UninitializedObjectChecker] Mark uninitialized regions as interesting.

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Looks much better than the original one, but why did the warning move to the 
correct place just because marking the region as interesting?


Repository:
  rC Clang

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

https://reviews.llvm.org/D62899



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

[PATCH] D63140: [clangd] Return TextEdits from ClangdServer::applyTweak

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363150: [clangd] Return TextEdits from 
ClangdServer::applyTweak (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63140?vs=204067=204264#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63140

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.h


Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -482,13 +482,13 @@
 
 auto Action = [ApplyEdit](decltype(Reply) Reply, URIForFile File,
   std::string Code,
-  llvm::Expected R) {
+  llvm::Expected> R) {
   if (!R)
 return Reply(R.takeError());
 
   WorkspaceEdit WE;
   WE.changes.emplace();
-  (*WE.changes)[File.uri()] = replacementsToEdits(Code, *R);
+  (*WE.changes)[File.uri()] = std::move(*R);
 
   Reply("Fix applied.");
   ApplyEdit(std::move(WE));
Index: clang-tools-extra/trunk/clangd/ClangdServer.h
===
--- clang-tools-extra/trunk/clangd/ClangdServer.h
+++ clang-tools-extra/trunk/clangd/ClangdServer.h
@@ -234,7 +234,7 @@
 
   /// Apply the code tweak with a specified \p ID.
   void applyTweak(PathRef File, Range Sel, StringRef ID,
-  Callback CB);
+  Callback> CB);
 
   /// Only for testing purposes.
   /// Waits until all requests to worker thread are finished and dumps AST for
Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp
@@ -321,7 +321,7 @@
 }
 
 void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
-  Callback CB) {
+  Callback> CB) {
   auto Action = [Sel](decltype(CB) CB, std::string File, std::string TweakID,
   Expected InpAST) {
 if (!InpAST)
@@ -332,15 +332,18 @@
 auto A = prepareTweak(TweakID, *Selection);
 if (!A)
   return CB(A.takeError());
-auto RawReplacements = (*A)->apply(*Selection);
-if (!RawReplacements)
-  return CB(RawReplacements.takeError());
+auto Raw = (*A)->apply(*Selection);
+if (!Raw)
+  return CB(Raw.takeError());
 // FIXME: this function has I/O operations (find .clang-format file), 
figure
 // out a way to cache the format style.
 auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents,
InpAST->Inputs.FS.get());
-return CB(
-cleanupAndFormat(InpAST->Inputs.Contents, *RawReplacements, Style));
+auto Formatted =
+cleanupAndFormat(InpAST->Inputs.Contents, *Raw, Style);
+if (!Formatted)
+  return CB(Formatted.takeError());
+return CB(replacementsToEdits(InpAST->Inputs.Contents, *Formatted));
   };
   WorkScheduler.runWithAST(
   "ApplyTweak", File,


Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -482,13 +482,13 @@
 
 auto Action = [ApplyEdit](decltype(Reply) Reply, URIForFile File,
   std::string Code,
-  llvm::Expected R) {
+  llvm::Expected> R) {
   if (!R)
 return Reply(R.takeError());
 
   WorkspaceEdit WE;
   WE.changes.emplace();
-  (*WE.changes)[File.uri()] = replacementsToEdits(Code, *R);
+  (*WE.changes)[File.uri()] = std::move(*R);
 
   Reply("Fix applied.");
   ApplyEdit(std::move(WE));
Index: clang-tools-extra/trunk/clangd/ClangdServer.h
===
--- clang-tools-extra/trunk/clangd/ClangdServer.h
+++ clang-tools-extra/trunk/clangd/ClangdServer.h
@@ -234,7 +234,7 @@
 
   /// Apply the code tweak with a specified \p ID.
   void applyTweak(PathRef File, Range Sel, StringRef ID,
-  Callback CB);
+  Callback> CB);
 
   /// Only for testing purposes.
   /// Waits until all requests to worker thread are finished and dumps AST for
Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp
+++ 

[clang-tools-extra] r363150 - [clangd] Return TextEdits from ClangdServer::applyTweak

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Jun 12 05:03:24 2019
New Revision: 363150

URL: http://llvm.org/viewvc/llvm-project?rev=363150=rev
Log:
[clangd] Return TextEdits from ClangdServer::applyTweak

Summary:
Instead of `tooling::Replacements`. So that embedders do not need to store
the contents of the file.

This also aligns better with `ClangdServer::rename`.

Reviewers: kadircet, hokein

Reviewed By: hokein

Subscribers: MaskRay, jkorous, arphaman, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=363150=363149=363150=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Jun 12 05:03:24 2019
@@ -482,13 +482,13 @@ void ClangdLSPServer::onCommand(const Ex
 
 auto Action = [ApplyEdit](decltype(Reply) Reply, URIForFile File,
   std::string Code,
-  llvm::Expected R) {
+  llvm::Expected> R) {
   if (!R)
 return Reply(R.takeError());
 
   WorkspaceEdit WE;
   WE.changes.emplace();
-  (*WE.changes)[File.uri()] = replacementsToEdits(Code, *R);
+  (*WE.changes)[File.uri()] = std::move(*R);
 
   Reply("Fix applied.");
   ApplyEdit(std::move(WE));

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=363150=363149=363150=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Jun 12 05:03:24 2019
@@ -321,7 +321,7 @@ void ClangdServer::enumerateTweaks(PathR
 }
 
 void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
-  Callback CB) {
+  Callback> CB) {
   auto Action = [Sel](decltype(CB) CB, std::string File, std::string TweakID,
   Expected InpAST) {
 if (!InpAST)
@@ -332,15 +332,18 @@ void ClangdServer::applyTweak(PathRef Fi
 auto A = prepareTweak(TweakID, *Selection);
 if (!A)
   return CB(A.takeError());
-auto RawReplacements = (*A)->apply(*Selection);
-if (!RawReplacements)
-  return CB(RawReplacements.takeError());
+auto Raw = (*A)->apply(*Selection);
+if (!Raw)
+  return CB(Raw.takeError());
 // FIXME: this function has I/O operations (find .clang-format file), 
figure
 // out a way to cache the format style.
 auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents,
InpAST->Inputs.FS.get());
-return CB(
-cleanupAndFormat(InpAST->Inputs.Contents, *RawReplacements, Style));
+auto Formatted =
+cleanupAndFormat(InpAST->Inputs.Contents, *Raw, Style);
+if (!Formatted)
+  return CB(Formatted.takeError());
+return CB(replacementsToEdits(InpAST->Inputs.Contents, *Formatted));
   };
   WorkScheduler.runWithAST(
   "ApplyTweak", File,

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=363150=363149=363150=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Wed Jun 12 05:03:24 2019
@@ -234,7 +234,7 @@ public:
 
   /// Apply the code tweak with a specified \p ID.
   void applyTweak(PathRef File, Range Sel, StringRef ID,
-  Callback CB);
+  Callback> CB);
 
   /// Only for testing purposes.
   /// Waits until all requests to worker thread are finished and dumps AST for


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

[PATCH] D63140: [clangd] Return TextEdits from ClangdServer::applyTweak

[Haojian Wu via Phabricator via cfe-commits]

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

looks good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63140



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

[PATCH] D63080: [analyzer] Track indices of arrays

[Balogh, Ádám via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1681
+trackExpressionValue(
+LVNode, Arr->getIdx(), report, EnableNullFPSuppression);
+

NoQ wrote:
> Mmm, dunno about null fp suppression. We're, like, talking about integers. 
> Integers are more often zero than null. We generally do support some FP 
> suppressions for integers as well (i.e., `core.DivideZero` uses them), but in 
> this case it doesn't sound as if `0` is anyhow special.
I agree here. Forwarding `EnableNUllFPSuppression` does not really make sense 
for the array index even if it was `true` for the array element.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63080



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

[PATCH] D63140: [clangd] Return TextEdits from ClangdServer::applyTweak

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

@hokein just realized you might be a better reviewer, since this makes 
`applyTweak` aligned with `rename`. And you implemented `rename` in the first 
place


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63140



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

[PATCH] D63192: [Diagnostics] Implement -Wswitch-default

[Dávid Bolvanský via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaStmt.cpp:869
+  SwitchCase *SC = SS->getSwitchCaseList();
+  if (!SC)
+Diag(SS->getBeginLoc(), diag::warn_empty_switch_body);

TODO:

if (!SC) -> switch has no default and case labels (empty body is misleading.. 
switch(x) x = b)

switch (x) {
   // warn_empty_switch_body
}


Repository:
  rC Clang

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

https://reviews.llvm.org/D63192



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

[PATCH] D63193: [clangd] Fix typo in GUARDED_BY()

[Nikolai Kosjar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363139: [clangd] Fix typo in GUARDED_BY() (authored by nik, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63193?vs=204250=204255#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63193

Files:
  clang-tools-extra/trunk/clangd/TUScheduler.cpp


Index: clang-tools-extra/trunk/clangd/TUScheduler.cpp
===
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp
@@ -273,7 +273,7 @@
   // The lifetime of the old/new ASTWorkers will overlap, but their handles
   // don't. When the old handle is destroyed, the old worker will stop 
reporting
   // diagnostics.
-  bool ReportDiagnostics = true; /* GUARDED_BY(DiagMu) */
+  bool ReportDiagnostics = true; /* GUARDED_BY(DiagsMu) */
 };
 
 /// A smart-pointer-like class that points to an active ASTWorker.


Index: clang-tools-extra/trunk/clangd/TUScheduler.cpp
===
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp
@@ -273,7 +273,7 @@
   // The lifetime of the old/new ASTWorkers will overlap, but their handles
   // don't. When the old handle is destroyed, the old worker will stop reporting
   // diagnostics.
-  bool ReportDiagnostics = true; /* GUARDED_BY(DiagMu) */
+  bool ReportDiagnostics = true; /* GUARDED_BY(DiagsMu) */
 };
 
 /// A smart-pointer-like class that points to an active ASTWorker.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63139: [Diagnostics] Implement -Wswitch-unreachable

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 204254.
xbolva00 added a comment.

Attached forgotten tests


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

https://reviews.llvm.org/D63139

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaStmt.cpp
  test/Sema/switch_unreachable.c
  test/SemaCXX/switch_unreachable.cpp

Index: test/SemaCXX/switch_unreachable.cpp
===
--- test/SemaCXX/switch_unreachable.cpp
+++ test/SemaCXX/switch_unreachable.cpp
@@ -0,0 +1,143 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wswitch-unreachable %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
+
+void g(int x);
+
+void foo(int x) {
+  int b = 0;
+  switch (x) {
+  label:
+  case 4:
+break;
+  default:
+return;
+  }
+
+  switch (x) {
+x++; // expected-warning {{statement will be never executed}}
+  label2:
+  case 4:
+break;
+  default:
+return;
+  }
+
+  switch (x) {
+  label3:
+x++;
+  case 4:
+break;
+  default:
+return;
+  }
+
+  switch (x) {
+  case 4:
+return;
+  }
+
+  switch (x) {
+b = x; // expected-warning {{statement will be never executed}}
+  case 7:
+g(b);
+break;
+  }
+
+  switch (x) {
+break; // expected-warning {{statement will be never executed}}
+  case 7:
+break;
+  }
+
+  switch (x) {
+return; // expected-warning {{statement will be never executed}}
+  case 7:
+break;
+  }
+
+  switch (x) {
+x++;  // expected-warning {{statement will be never executed}}
+g(x); // expected-warning {{statement will be never executed}}
+  case 7:
+break;
+  }
+
+  switch (x) {
+x++; // expected-warning {{statement will be never executed}}
+  label4:
+g(x);
+  case 7:
+break;
+  }
+
+  switch (x) {
+g(x); // expected-warning {{statement will be never executed}}
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  switch (x) {
+g(x); // expected-warning {{statement will be never executed}}
+  case 1:
+  case 2:
+  case 3:
+break;
+  }
+
+  switch (x) {
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  switch (x) {
+  case 1:
+break;
+  default:
+break;
+  }
+
+  switch (x) {
+b = x; // expected-warning {{statement will be never executed}}
+  }
+
+  switch (x) {
+g(x); // expected-warning {{statement will be never executed}}
+  }
+
+  switch (x)
+b = x; // expected-warning {{statement will be never executed}}
+
+  switch (x)
+g(x); // expected-warning {{statement will be never executed}}
+
+  switch (x)
+  label5:
+g(x);
+
+  switch (x) {
+  label6:
+g(x);
+  }
+
+  switch (x)
+  case 5:
+g(x);
+
+  switch (x) {
+  case 5:
+g(x);
+  }
+
+  switch (x)
+  default:
+g(x);
+
+  switch (x) {
+  default:
+g(x);
+  }
+}
Index: test/Sema/switch_unreachable.c
===
--- test/Sema/switch_unreachable.c
+++ test/Sema/switch_unreachable.c
@@ -0,0 +1,143 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wswitch-unreachable %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
+
+void g(int x);
+
+void foo(int x) {
+  int b = 0;
+  switch (x) {
+  label:
+  case 4:
+break;
+  default:
+return;
+  }
+
+  switch (x) {
+x++; // expected-warning {{statement will be never executed}}
+  label2:
+  case 4:
+break;
+  default:
+return;
+  }
+
+  switch (x) {
+  label3:
+x++;
+  case 4:
+break;
+  default:
+return;
+  }
+
+  switch (x) {
+  case 4:
+return;
+  }
+
+  switch (x) {
+b = x; // expected-warning {{statement will be never executed}}
+  case 7:
+g(b);
+break;
+  }
+
+  switch (x) {
+break; // expected-warning {{statement will be never executed}}
+  case 7:
+break;
+  }
+
+  switch (x) {
+return; // expected-warning {{statement will be never executed}}
+  case 7:
+break;
+  }
+
+  switch (x) {
+x++;  // expected-warning {{statement will be never executed}}
+g(x); // expected-warning {{statement will be never executed}}
+  case 7:
+break;
+  }
+
+  switch (x) {
+x++; // expected-warning {{statement will be never executed}}
+  label4:
+g(x);
+  case 7:
+break;
+  }
+
+  switch (x) {
+g(x); // expected-warning {{statement will be never executed}}
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  switch (x) {
+g(x); // expected-warning {{statement will be never executed}}
+  case 1:
+  case 2:
+  case 3:
+break;
+  }
+
+  switch (x) {
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  switch (x) {
+  case 1:
+break;
+  default:
+break;
+  }
+
+  switch (x) {
+b = x; // expected-warning {{statement will be never executed}}
+  }
+
+  switch (x) {
+g(x); // expected-warning {{statement will be never executed}}
+  }
+
+  switch (x)
+b = x; // expected-warning {{statement will be never executed}}
+
+  switch (x)
+g(x); // expected-warning {{statement 

Re: r363009 - [Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer

[Ilya Biryukov via cfe-commits]

Sure, that should work just fine. Could you also add a comment what events
setting the log file sets in motion? It is not obvious from the test code.

On Tue, Jun 11, 2019 at 6:51 PM Alex L  wrote:

> Hmm, the logging was meant to exercise the creation of chained diagnostic
> consumer, so without it the test is kind of pointless. I'll undo your
> change, but will set the file to "-" which will print the log to STDERR and
> won't create new files. Does that sound reasonable?
>
> Cheers,
> Alex
>
> On Tue, 11 Jun 2019 at 02:51, Ilya Biryukov  wrote:
>
>> Hi Alex,
>>
>> Just wanted to let you know that I removed logging of diagnostics into a
>> file inside the unit test in r363041 to unbreak our integrate.
>>
>> On Tue, Jun 11, 2019 at 1:29 AM Alex Lorenz via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: arphaman
>>> Date: Mon Jun 10 16:32:42 2019
>>> New Revision: 363009
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=363009=rev
>>> Log:
>>> [Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer
>>> in the compiler
>>>
>>> The function SetUpDiagnosticLog that was called from createDiagnostics
>>> didn't
>>> handle the case where the diagnostics engine didn't own the diagnostics
>>> consumer.
>>> This is a potential problem for a clang tool, in particular some of the
>>> follow-up
>>> patches for clang-scan-deps will need this fix.
>>>
>>> Differential Revision: https://reviews.llvm.org/D63101
>>>
>>> Modified:
>>> cfe/trunk/lib/Frontend/CompilerInstance.cpp
>>> cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp
>>>
>>> Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=363009=363008=363009=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
>>> +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Mon Jun 10 16:32:42 2019
>>> @@ -232,9 +232,13 @@ static void SetUpDiagnosticLog(Diagnosti
>>>
>>>  std::move(StreamOwner));
>>>if (CodeGenOpts)
>>>  Logger->setDwarfDebugFlags(CodeGenOpts->DwarfDebugFlags);
>>> -  assert(Diags.ownsClient());
>>> -  Diags.setClient(
>>> -  new ChainedDiagnosticConsumer(Diags.takeClient(),
>>> std::move(Logger)));
>>> +  if (Diags.ownsClient()) {
>>> +Diags.setClient(
>>> +new ChainedDiagnosticConsumer(Diags.takeClient(),
>>> std::move(Logger)));
>>> +  } else {
>>> +Diags.setClient(
>>> +new ChainedDiagnosticConsumer(Diags.getClient(),
>>> std::move(Logger)));
>>> +  }
>>>  }
>>>
>>>  static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts,
>>>
>>> Modified: cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp?rev=363009=363008=363009=diff
>>>
>>> ==
>>> --- cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp (original)
>>> +++ cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp Mon Jun 10
>>> 16:32:42 2019
>>> @@ -8,6 +8,7 @@
>>>
>>>  #include "clang/Frontend/CompilerInstance.h"
>>>  #include "clang/Frontend/CompilerInvocation.h"
>>> +#include "clang/Frontend/TextDiagnosticPrinter.h"
>>>  #include "llvm/Support/FileSystem.h"
>>>  #include "llvm/Support/Format.h"
>>>  #include "llvm/Support/ToolOutputFile.h"
>>> @@ -70,4 +71,21 @@ TEST(CompilerInstance, DefaultVFSOverlay
>>>ASSERT_TRUE(Instance.getFileManager().getFile("vfs-virtual.file"));
>>>  }
>>>
>>> +TEST(CompilerInstance, AllowDiagnosticLogWithUnownedDiagnosticConsumer)
>>> {
>>> +  auto DiagOpts = new DiagnosticOptions();
>>> +  DiagOpts->DiagnosticLogFile = "log.diags";
>>> +
>>> +  // Create the diagnostic engine with unowned consumer.
>>> +  std::string DiagnosticOutput;
>>> +  llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput);
>>> +  auto DiagPrinter = llvm::make_unique(
>>> +  DiagnosticsOS, new DiagnosticOptions());
>>> +  CompilerInstance Instance;
>>> +  IntrusiveRefCntPtr Diags =
>>> Instance.createDiagnostics(
>>> +  DiagOpts, DiagPrinter.get(), /*ShouldOwnClient=*/false);
>>> +
>>> +  Diags->Report(diag::err_expected) << "no crash";
>>> +  ASSERT_EQ(DiagnosticsOS.str(), "error: expected no crash\n");
>>> +}
>>> +
>>>  } // anonymous namespace
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>> --
>> Regards,
>> Ilya Biryukov
>>
>

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

[PATCH] D63139: [Diagnostics] Implement -Wswitch-unreachable

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 204253.
xbolva00 added a comment.

Warn in more cases. Added many new tests.


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

https://reviews.llvm.org/D63139

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaStmt.cpp


Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -861,6 +861,20 @@
   typedef std::vector > CaseRangesTy;
   CaseRangesTy CaseRanges;
 
+  if (CompoundStmt *CS = dyn_cast(BodyStmt)) {
+for (auto It = CS->body_begin(); It != CS->body_end(); ++It) {
+  auto *S = *It;
+  if (isa(S) || isa(S) || isa(S))
+break;
+  Diag(S->getBeginLoc(), diag::warn_unreachable_stmt_in_switch);
+}
+  } else if (isa(BodyStmt) || isa(BodyStmt) ||
+ isa(BodyStmt)) {
+// No warning
+  } else {
+Diag(BodyStmt->getBeginLoc(), diag::warn_unreachable_stmt_in_switch);
+  }
+
   DefaultStmt *TheDefaultStmt = nullptr;
 
   bool CaseListIsErroneous = false;
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8188,6 +8188,8 @@
 def err_default_not_in_switch : Error<
   "'default' statement not in switch statement">;
 def err_case_not_in_switch : Error<"'case' statement not in switch statement">;
+def warn_unreachable_stmt_in_switch : Warning<
+  "statement will be never executed">, InGroup, 
DefaultIgnore;
 def warn_bool_switch_condition : Warning<
   "switch condition has boolean value">, InGroup;
 def warn_case_value_overflow : Warning<
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -538,6 +538,7 @@
 def CoveredSwitchDefault : DiagGroup<"covered-switch-default">;
 def SwitchBool : DiagGroup<"switch-bool">;
 def SwitchEnum : DiagGroup<"switch-enum">;
+def SwitchUnreachable : DiagGroup<"switch-unreachable">;
 def Switch : DiagGroup<"switch">;
 def EnumCompareSwitch : DiagGroup<"enum-compare-switch">;
 def EnumCompare   : DiagGroup<"enum-compare", [EnumCompareSwitch]>;
@@ -842,7 +843,7 @@
 // Note that putting warnings in -Wall will not disable them by default. If a
 // warning should be active _only_ when -Wall is passed in, mark it as
 // DefaultIgnore in addition to putting it here.
-def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool]>;
+def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool, 
SwitchUnreachable]>;
 
 // Warnings that should be in clang-cl /w4.
 def : DiagGroup<"CL4", [All, Extra]>;


Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -861,6 +861,20 @@
   typedef std::vector > CaseRangesTy;
   CaseRangesTy CaseRanges;
 
+  if (CompoundStmt *CS = dyn_cast(BodyStmt)) {
+for (auto It = CS->body_begin(); It != CS->body_end(); ++It) {
+  auto *S = *It;
+  if (isa(S) || isa(S) || isa(S))
+break;
+  Diag(S->getBeginLoc(), diag::warn_unreachable_stmt_in_switch);
+}
+  } else if (isa(BodyStmt) || isa(BodyStmt) ||
+ isa(BodyStmt)) {
+// No warning
+  } else {
+Diag(BodyStmt->getBeginLoc(), diag::warn_unreachable_stmt_in_switch);
+  }
+
   DefaultStmt *TheDefaultStmt = nullptr;
 
   bool CaseListIsErroneous = false;
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8188,6 +8188,8 @@
 def err_default_not_in_switch : Error<
   "'default' statement not in switch statement">;
 def err_case_not_in_switch : Error<"'case' statement not in switch statement">;
+def warn_unreachable_stmt_in_switch : Warning<
+  "statement will be never executed">, InGroup, DefaultIgnore;
 def warn_bool_switch_condition : Warning<
   "switch condition has boolean value">, InGroup;
 def warn_case_value_overflow : Warning<
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -538,6 +538,7 @@
 def CoveredSwitchDefault : DiagGroup<"covered-switch-default">;
 def SwitchBool : DiagGroup<"switch-bool">;
 def SwitchEnum : DiagGroup<"switch-enum">;
+def SwitchUnreachable : DiagGroup<"switch-unreachable">;
 def Switch : DiagGroup<"switch">;
 def EnumCompareSwitch : DiagGroup<"enum-compare-switch">;
 def EnumCompare   : DiagGroup<"enum-compare", [EnumCompareSwitch]>;
@@ -842,7 +843,7 @@
 // Note that putting warnings in -Wall will not disable 

[clang-tools-extra] r363139 - [clangd] Fix typo in GUARDED_BY()

[Nikolai Kosjar via cfe-commits]

Author: nik
Date: Wed Jun 12 04:01:19 2019
New Revision: 363139

URL: http://llvm.org/viewvc/llvm-project?rev=363139=rev
Log:
[clangd] Fix typo in GUARDED_BY()

Reviewers: ilya-biryukov, kadircet, sammccall

Subscribers: javed.absar, MaskRay, jkorous, arphaman, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/TUScheduler.cpp

Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=363139=363138=363139=diff
==
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Wed Jun 12 04:01:19 2019
@@ -273,7 +273,7 @@ private:
   // The lifetime of the old/new ASTWorkers will overlap, but their handles
   // don't. When the old handle is destroyed, the old worker will stop 
reporting
   // diagnostics.
-  bool ReportDiagnostics = true; /* GUARDED_BY(DiagMu) */
+  bool ReportDiagnostics = true; /* GUARDED_BY(DiagsMu) */
 };
 
 /// A smart-pointer-like class that points to an active ASTWorker.


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

[PATCH] D63193: [clangd] Fix typo in GUARDED_BY()

[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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63193



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

[PATCH] D63192: [Diagnostics] Implement -Wswitch-default

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 planned changes to this revision.
xbolva00 added a comment.

I will work on this after we land https://reviews.llvm.org/D63139.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63192



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

[PATCH] D63193: [clangd] Fix typo in GUARDED_BY()

[Nikolai Kosjar via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63193

Files:
  clang-tools-extra/clangd/TUScheduler.cpp


Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -273,7 +273,7 @@
   // The lifetime of the old/new ASTWorkers will overlap, but their handles
   // don't. When the old handle is destroyed, the old worker will stop 
reporting
   // diagnostics.
-  bool ReportDiagnostics = true; /* GUARDED_BY(DiagMu) */
+  bool ReportDiagnostics = true; /* GUARDED_BY(DiagsMu) */
 };
 
 /// A smart-pointer-like class that points to an active ASTWorker.


Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -273,7 +273,7 @@
   // The lifetime of the old/new ASTWorkers will overlap, but their handles
   // don't. When the old handle is destroyed, the old worker will stop reporting
   // diagnostics.
-  bool ReportDiagnostics = true; /* GUARDED_BY(DiagMu) */
+  bool ReportDiagnostics = true; /* GUARDED_BY(DiagsMu) */
 };
 
 /// A smart-pointer-like class that points to an active ASTWorker.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63192: [Diagnostics] Implement -Wswitch-default

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 created this revision.
xbolva00 added reviewers: hfinkel, lattner, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For MSVC compatibility, based on request in 
https://bugs.llvm.org/show_bug.cgi?id=4546.


Repository:
  rC Clang

https://reviews.llvm.org/D63192

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaStmt.cpp
  test/Sema/switch_default.c
  test/SemaCXX/switch_default.cpp

Index: test/SemaCXX/switch_default.cpp
===
--- test/SemaCXX/switch_default.cpp
+++ test/SemaCXX/switch_default.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wswitch-default %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wempty-body %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
+
+void test(int x) {
+  switch (x) {
+  default: // expected-warning {{switch contains only default label}}
+break;
+  }
+
+  switch (x) {
+  default: // expected-warning {{switch contains only default label}}
+x++;
+  }
+
+  switch (x) {
+  default:; // expected-warning {{switch contains only default label}}
+  }
+
+  switch (x)
+  default: // expected-warning {{switch contains only default label}}
+break;
+
+  switch (x) {
+  default:
+return;
+  case 1:
+break;
+  }
+
+  switch (x) {
+  case 1:
+break;
+  default:
+return;
+  }
+
+  switch (x) { // expected-warning {{switch statement has empty body}}
+  }
+}
Index: test/Sema/switch_default.c
===
--- test/Sema/switch_default.c
+++ test/Sema/switch_default.c
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wswitch-default %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wempty-body %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
+
+void test(int x) {
+  switch (x) {
+  default: // expected-warning {{switch contains only default label}}
+break;
+  }
+
+  switch (x) {
+  default: // expected-warning {{switch contains only default label}}
+x++;
+  }
+
+  switch (x) {
+  default:; // expected-warning {{switch contains only default label}}
+  }
+
+  switch (x)
+  default: // expected-warning {{switch contains only default label}}
+break;
+
+  switch (x) {
+  default:
+return;
+  case 1:
+break;
+  }
+
+  switch (x) {
+  case 1:
+break;
+  default:
+return;
+  }
+
+  switch (x) { // expected-warning {{switch statement has empty body}}
+  }
+}
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -865,8 +865,13 @@
 
   bool CaseListIsErroneous = false;
 
-  for (SwitchCase *SC = SS->getSwitchCaseList(); SC && !HasDependentValue;
-   SC = SC->getNextSwitchCase()) {
+  SwitchCase *SC = SS->getSwitchCaseList();
+  if (!SC)
+Diag(SS->getBeginLoc(), diag::warn_empty_switch_body);
+  else if (isa(SC) && !SC->getNextSwitchCase())
+Diag(SC->getBeginLoc(), diag::warn_default_only_in_switch);
+
+  for (; SC && !HasDependentValue; SC = SC->getNextSwitchCase()) {
 
 if (DefaultStmt *DS = dyn_cast(SC)) {
   if (TheDefaultStmt) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8185,6 +8185,8 @@
 def warn_break_binds_to_switch : Warning<
   "'break' is bound to loop, GCC binds it to switch">,
   InGroup;
+def warn_default_only_in_switch : Warning<
+  "switch contains only default label">, InGroup, DefaultIgnore;
 def err_default_not_in_switch : Error<
   "'default' statement not in switch statement">;
 def err_case_not_in_switch : Error<"'case' statement not in switch statement">;
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -458,7 +458,6 @@
 def : DiagGroup<"sign-promo">;
 def SignCompare : DiagGroup<"sign-compare">;
 def : DiagGroup<"stack-protector">;
-def : DiagGroup<"switch-default">;
 def : DiagGroup<"synth">;
 def SizeofArrayArgument : DiagGroup<"sizeof-array-argument">;
 def SizeofArrayDecay : DiagGroup<"sizeof-array-decay">;
@@ -537,6 +536,7 @@
 def ObjCCStringFormat : DiagGroup<"cstring-format-directive">;
 def CoveredSwitchDefault : DiagGroup<"covered-switch-default">;
 def SwitchBool : DiagGroup<"switch-bool">;
+def SwitchDefault  : DiagGroup<"switch-default">;
 def SwitchEnum : DiagGroup<"switch-enum">;
 def Switch : DiagGroup<"switch">;
 def EnumCompareSwitch : DiagGroup<"enum-compare-switch">;
@@ -842,7 +842,7 @@
 // Note that putting warnings in -Wall will not disable them by default. If a
 // warning should be active _only_ when -Wall is passed in, mark it as
 // DefaultIgnore in addition to putting 

[PATCH] D63149: Added AST matcher for ignoring elidable constructors

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 204245.
jvikstrom added a comment.

- Using CamelCase and also renamed ignoringElidableMoveConstructorCall to 
ignoringElidableConstructorCall


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63149

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -566,6 +566,81 @@
llvm::make_unique>("x")));
 }
 
+TEST(Matcher, IgnoresElidableConstructors) {
+  auto matcher1 = cxxOperatorCallExpr(hasArgument(
+  1, callExpr(hasArgument(
+ 0, ignoringElidableMoveConstructorCall(callExpr().bind("c"));
+  auto matcher2 = cxxOperatorCallExpr(hasArgument(
+  1, callExpr(hasArgument(0, ignoringElidableMoveConstructorCall(
+ integerLiteral().bind("c"));
+  auto matcher3 =
+  varDecl(hasInitializer(ignoringElidableMoveConstructorCall(callExpr(;
+
+  auto code1 = "struct H {};"
+   "template H B(T A);"
+   "void f(){"
+   "H D1;"
+   "D1=B(B(1));"
+   "}";
+  auto code2 = "struct H {};"
+   "template H B(T A);"
+   "void f(){"
+   "H D1;"
+   "D1=B(1);"
+   "}";
+  auto code3 = "struct H {};"
+   "H G();"
+   "void f(){"
+   "H D = G();"
+   "}";
+
+  EXPECT_TRUE(matchesConditionally(code1, matcher1, true, "-std=c++14"));
+  EXPECT_TRUE(matchesConditionally(code1, matcher1, true, "-std=c++17"));
+
+  EXPECT_TRUE(matchesConditionally(code2, matcher2, true, "-std=c++14"));
+  EXPECT_TRUE(matchesConditionally(code2, matcher2, true, "-std=c++17"));
+
+  EXPECT_TRUE(matchesConditionally(code3, matcher3, true, "-std=c++14"));
+  EXPECT_TRUE(matchesConditionally(code3, matcher3, true, "-std=c++17"));
+}
+
+TEST(Matcher, IgnoresElidableInReturn) {
+  auto matcher = expr(ignoringElidableMoveConstructorCall(declRefExpr()));
+  auto code1 = "struct H{};"
+   "H f(){"
+   "H g;"
+   "return g;"
+   "}";
+  auto code2 = "struct H{};"
+   "H f(){"
+   "return H();"
+   "}";
+  EXPECT_TRUE(matchesConditionally(code1, matcher, true, "-std=c++14"));
+  EXPECT_TRUE(matchesConditionally(code1, matcher, true, "-std=c++17"));
+  EXPECT_TRUE(matchesConditionally(code2, matcher, false, "-std=c++14"));
+  EXPECT_TRUE(matchesConditionally(code2, matcher, false, "-std=c++17"));
+}
+
+TEST(Matcher, IgnoreElidableConstructorDoesNotMatchConstructors) {
+  auto matcher = varDecl(
+  hasInitializer(ignoringElidableMoveConstructorCall(cxxConstructExpr(;
+  auto code = "struct H {};"
+  "void f(){"
+  "H D;"
+  "}";
+  EXPECT_TRUE(matchesConditionally(code, matcher, true, "-std=c++14"));
+  EXPECT_TRUE(matchesConditionally(code, matcher, true, "-std=c++17"));
+};
+
+TEST(Matcher, IgnoresElidableDoesNotPreventMatches) {
+  auto matcher = expr(ignoringElidableMoveConstructorCall(integerLiteral()));
+  auto code = "void f(){"
+  "int D = 10;"
+  "}";
+  EXPECT_TRUE(matchesConditionally(code, matcher, true, "-std=c++14"));
+  EXPECT_TRUE(matchesConditionally(code, matcher, true, "-std=c++17"));
+}
+
 TEST(Matcher, BindTheSameNameInAlternatives) {
   StatementMatcher matcher = anyOf(
 binaryOperator(hasOperatorName("+"),
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -320,6 +320,7 @@
   REGISTER_MATCHER(hasUnqualifiedDesugaredType);
   REGISTER_MATCHER(hasValueType);
   REGISTER_MATCHER(ifStmt);
+  REGISTER_MATCHER(ignoringElidableMoveConstructorCall);
   REGISTER_MATCHER(ignoringImpCasts);
   REGISTER_MATCHER(ignoringImplicit);
   REGISTER_MATCHER(ignoringParenCasts);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6452,6 +6452,56 @@
   return false;
 }
 
+/// Matches expressions that match InnerMatcher that are possibly wrapped in an
+/// elidable constructor.
+///
+/// In C++17 copy elidable constructors are no longer being
+/// generated in the AST as it is not permitted by the standard. They are
+/// however part of the AST in C++14 and earlier. Therefore, to write a 

[PATCH] D63149: Added AST matcher for ignoring elidable constructors

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 204244.
jvikstrom marked an inline comment as done.
jvikstrom added a comment.

- Updated example and fixed edge case in ignoringElidableConstructorCall


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63149

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -566,6 +566,81 @@
llvm::make_unique>("x")));
 }
 
+TEST(Matcher, IgnoresElidableConstructors) {
+  auto matcher1 = cxxOperatorCallExpr(hasArgument(
+  1, callExpr(hasArgument(
+ 0, ignoringElidableMoveConstructorCall(callExpr().bind("c"));
+  auto matcher2 = cxxOperatorCallExpr(hasArgument(
+  1, callExpr(hasArgument(0, ignoringElidableMoveConstructorCall(
+ integerLiteral().bind("c"));
+  auto matcher3 =
+  varDecl(hasInitializer(ignoringElidableMoveConstructorCall(callExpr(;
+
+  auto code1 = "struct H {};"
+   "template H B(T A);"
+   "void f(){"
+   "H D1;"
+   "D1=B(B(1));"
+   "}";
+  auto code2 = "struct H {};"
+   "template H B(T A);"
+   "void f(){"
+   "H D1;"
+   "D1=B(1);"
+   "}";
+  auto code3 = "struct H {};"
+   "H G();"
+   "void f(){"
+   "H D = G();"
+   "}";
+
+  EXPECT_TRUE(matchesConditionally(code1, matcher1, true, "-std=c++14"));
+  EXPECT_TRUE(matchesConditionally(code1, matcher1, true, "-std=c++17"));
+
+  EXPECT_TRUE(matchesConditionally(code2, matcher2, true, "-std=c++14"));
+  EXPECT_TRUE(matchesConditionally(code2, matcher2, true, "-std=c++17"));
+
+  EXPECT_TRUE(matchesConditionally(code3, matcher3, true, "-std=c++14"));
+  EXPECT_TRUE(matchesConditionally(code3, matcher3, true, "-std=c++17"));
+}
+
+TEST(Matcher, IgnoresElidableInReturn) {
+  auto matcher = expr(ignoringElidableMoveConstructorCall(declRefExpr()));
+  auto code1 = "struct H{};"
+   "H f(){"
+   "H g;"
+   "return g;"
+   "}";
+  auto code2 = "struct H{};"
+   "H f(){"
+   "return H();"
+   "}";
+  EXPECT_TRUE(matchesConditionally(code1, matcher, true, "-std=c++14"));
+  EXPECT_TRUE(matchesConditionally(code1, matcher, true, "-std=c++17"));
+  EXPECT_TRUE(matchesConditionally(code2, matcher, false, "-std=c++14"));
+  EXPECT_TRUE(matchesConditionally(code2, matcher, false, "-std=c++17"));
+}
+
+TEST(Matcher, IgnoreElidableConstructorDoesNotMatchConstructors) {
+  auto matcher = varDecl(
+  hasInitializer(ignoringElidableMoveConstructorCall(cxxConstructExpr(;
+  auto code = "struct H {};"
+  "void f(){"
+  "H D;"
+  "}";
+  EXPECT_TRUE(matchesConditionally(code, matcher, true, "-std=c++14"));
+  EXPECT_TRUE(matchesConditionally(code, matcher, true, "-std=c++17"));
+};
+
+TEST(Matcher, IgnoresElidableDoesNotPreventMatches) {
+  auto matcher = expr(ignoringElidableMoveConstructorCall(integerLiteral()));
+  auto code = "void f(){"
+  "int D = 10;"
+  "}";
+  EXPECT_TRUE(matchesConditionally(code, matcher, true, "-std=c++14"));
+  EXPECT_TRUE(matchesConditionally(code, matcher, true, "-std=c++17"));
+}
+
 TEST(Matcher, BindTheSameNameInAlternatives) {
   StatementMatcher matcher = anyOf(
 binaryOperator(hasOperatorName("+"),
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -320,6 +320,7 @@
   REGISTER_MATCHER(hasUnqualifiedDesugaredType);
   REGISTER_MATCHER(hasValueType);
   REGISTER_MATCHER(ifStmt);
+  REGISTER_MATCHER(ignoringElidableMoveConstructorCall);
   REGISTER_MATCHER(ignoringImpCasts);
   REGISTER_MATCHER(ignoringImplicit);
   REGISTER_MATCHER(ignoringParenCasts);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6452,6 +6452,56 @@
   return false;
 }
 
+/// Matches expressions that match InnerMatcher that are possibly wrapped in an
+/// elidable constructor.
+///
+/// In C++17 copy elidable constructors are no longer being
+/// generated in the AST as it is not permitted by the standard. They are
+/// however part of the AST in C++14 and earlier. Therefore, to 

[PATCH] D62804: [clangd] Enable extraction of system includes from custom toolchains

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 204243.
kadircet added a comment.

- Fix off-by-one bug and improve lit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62804

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  clang-tools-extra/clangd/SystemIncludeExtractor.cpp
  clang-tools-extra/clangd/test/system-include-extractor.test
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -268,6 +268,15 @@
 "Always used text-based completion")),
 llvm::cl::init(CodeCompleteOptions().RunParser), llvm::cl::Hidden);
 
+static llvm::cl::list QueryDriverGlobs(
+"query-driver",
+llvm::cl::desc(
+"Comma separated list of globs for white-listing gcc-compatible "
+"drivers that are safe to execute. Drivers matching any of these globs "
+"will be used to extract system includes. e.g. "
+"/usr/bin/**/clang-*,/path/to/repo/**/g++-*"),
+llvm::cl::CommaSeparated);
+
 namespace {
 
 /// \brief Supports a test URI scheme with relaxed constraints for lit tests.
@@ -521,6 +530,7 @@
 };
   }
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
+  Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
   llvm::Optional OffsetEncodingFromFlag;
   if (ForceOffsetEncoding != OffsetEncoding::UnsupportedEncoding)
 OffsetEncodingFromFlag = ForceOffsetEncoding;
Index: clang-tools-extra/clangd/test/system-include-extractor.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/system-include-extractor.test
@@ -0,0 +1,44 @@
+# RUN: rm -rf %t.dir && mkdir -p %t.dir
+
+# RUN: echo '#!/bin/bash' >> %t.dir/my_driver.sh
+# RUN: echo 'echo line to ignore' >> %t.dir/my_driver.sh
+# RUN: echo 'echo \#include \<...\> search starts here:' >> %t.dir/my_driver.sh
+# RUN: echo 'echo %t.dir/my/dir/' >> %t.dir/my_driver.sh
+# RUN: echo 'echo %t.dir/my/dir2/' >> %t.dir/my_driver.sh
+# RUN: echo 'echo End of search list.' >> %t.dir/my_driver.sh
+# RUN: chmod +x %t.dir/my_driver.sh
+
+# RUN: mkdir -p %t.dir/my/dir
+# RUN: touch %t.dir/my/dir/a.h
+# RUN: mkdir -p %t.dir/my/dir2
+# RUN: touch %t.dir/my/dir2/b.h
+
+# RUN: echo '[{"directory": "%/t.dir", "command": "%/t.dir/my_driver.sh the-file.cpp", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+
+# RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
+# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -e "s|file://\([A-Z]\):/|file:///\1:/|g" %t.test.1 > %t.test
+
+# RUN: clangd -lit-test -query-driver="**/*.sh" < %t.test | FileCheck -strict-whitespace %t.test
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
+---
+{
+  "jsonrpc":"2.0",
+  "method":"textDocument/didOpen",
+  "params": {
+"textDocument": {
+  "uri": "file://INPUT_DIR/the-file.cpp",
+  "languageId":"cpp",
+  "version":1,
+  "text":"#include \n#include "
+}
+  }
+}
+# CHECK:   "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:   "params": {
+# CHECK-NEXT: "diagnostics": [],
+---
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/SystemIncludeExtractor.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -0,0 +1,259 @@
+//===--- SystemIncludeExtractor.cpp --*- 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
+//
+//===--===//
+// Some compiler drivers have implicit search mechanism for system headers.
+// This compilation database implementation tries to extract that information by
+// executing the driver in verbose mode. gcc-compatible drivers print something
+// like:
+// 
+// 
+// #include <...> search starts here:
+//  /usr/lib/gcc/x86_64-linux-gnu/7/include
+//  /usr/local/include
+//  /usr/lib/gcc/x86_64-linux-gnu/7/include-fixed
+//  /usr/include/x86_64-linux-gnu
+//  /usr/include
+// End of search list.
+// 
+// 
+// This component parses that output and adds each path to command line args
+// provided by Base, after prepending them with -isystem. Therefore current
+// implementation would not work with a driver that is not gcc-compatible.
+//
+// 

[PATCH] D63149: Added AST matcher for ignoring elidable constructors

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6477
+/// matches ``D1 = B(B(1))``
+AST_MATCHER_P(Expr, ignoringElidableMoveConstructorCall,
+  ast_matchers::internal::Matcher, InnerMatcher) {

Is the matcher only strict to **move** constructor? looks like the matcher 
implementation is for general constructors, just 
`ignoringElidableConstructorCall`? we may have elidable copy constructor.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6479
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  if (const auto *cxx_construct_expr = dyn_cast()) {
+if (cxx_construct_expr->isElidable()) {

nit: 
[LLVM](https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly)
 uses `CamelCase` style. 



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6486
+  }
+  return InnerMatcher.matches(*cxx_construct_expr, Finder, Builder);
+}

I think this return is unnecessary, since we will fall through at line 6489.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63149



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

[PATCH] D63188: Fixed a crash in misc-redundant-expression ClangTidy checker

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363133: Fixed a crash in misc-redundant-expression ClangTidy 
checker (authored by gribozavr, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63188?vs=204233=204234#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63188

Files:
  clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp


Index: clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -555,10 +555,14 @@
   if (!LhsBinOp || !RhsBinOp)
 return false;
 
-  if ((LhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
-   LhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)) &&
-  (RhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
-   RhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)))
+  auto IsIntegerConstantExpr = [AstCtx](const Expr *E) {
+return !E->isValueDependent() && E->isIntegerConstantExpr(*AstCtx);
+  };
+
+  if ((IsIntegerConstantExpr(LhsBinOp->getLHS()) ||
+   IsIntegerConstantExpr(LhsBinOp->getRHS())) &&
+  (IsIntegerConstantExpr(RhsBinOp->getLHS()) ||
+   IsIntegerConstantExpr(RhsBinOp->getRHS(
 return true;
   return false;
 }
@@ -580,12 +584,14 @@
   const auto *BinOpLhs = cast(BinOp->getLHS());
   const auto *BinOpRhs = cast(BinOp->getRHS());
 
-  LhsConst = BinOpLhs->getLHS()->isIntegerConstantExpr(*AstCtx)
- ? BinOpLhs->getLHS()
- : BinOpLhs->getRHS();
-  RhsConst = BinOpRhs->getLHS()->isIntegerConstantExpr(*AstCtx)
- ? BinOpRhs->getLHS()
- : BinOpRhs->getRHS();
+  auto IsIntegerConstantExpr = [AstCtx](const Expr *E) {
+return !E->isValueDependent() && E->isIntegerConstantExpr(*AstCtx);
+  };
+
+  LhsConst = IsIntegerConstantExpr(BinOpLhs->getLHS()) ? BinOpLhs->getLHS()
+   : BinOpLhs->getRHS();
+  RhsConst = IsIntegerConstantExpr(BinOpRhs->getLHS()) ? BinOpRhs->getLHS()
+   : BinOpRhs->getRHS();
 
   if (!LhsConst || !RhsConst)
 return false;
Index: clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp
@@ -84,6 +84,14 @@
   return 0;
 }
 
+template 
+int TestSimpleEquivalentDependent() {
+  if (DX > 0 && DX > 0) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both sides of operator are 
equivalent
+
+  return 0;
+}
+
 int Valid(int X, int Y) {
   if (X != Y) return 1;
   if (X == Y + 0) return 1;
@@ -670,7 +678,7 @@
 #define FLAG3 4
 #define FLAGS (FLAG1 | FLAG2 | FLAG3)
 #define NOTFLAGS !(FLAG1 | FLAG2 | FLAG3)
-int operatorConfusion(int X, int Y, long Z)
+int TestOperatorConfusion(int X, int Y, long Z)
 {
   // Ineffective & expressions.
   Y = (Y << 8) & 0xff;
@@ -722,6 +730,12 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: ineffective logical negation 
operator
   // CHECK-FIXES: {{^}}  return ~(1 | 2 | 4);{{$}}
 }
+
+template 
+int TestOperatorConfusionDependent(int Y) {
+  int r1 = (Y << Shift) & 0xff;
+  int r2 = (Y << 8) & Mask;
+}
 #undef FLAG1
 #undef FLAG2
 #undef FLAG3


Index: clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -555,10 +555,14 @@
   if (!LhsBinOp || !RhsBinOp)
 return false;
 
-  if ((LhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
-   LhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)) &&
-  (RhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
-   RhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)))
+  auto IsIntegerConstantExpr = [AstCtx](const Expr *E) {
+return !E->isValueDependent() && E->isIntegerConstantExpr(*AstCtx);
+  };
+
+  if ((IsIntegerConstantExpr(LhsBinOp->getLHS()) ||
+   IsIntegerConstantExpr(LhsBinOp->getRHS())) &&
+  (IsIntegerConstantExpr(RhsBinOp->getLHS()) ||
+   IsIntegerConstantExpr(RhsBinOp->getRHS(
 return true;
   return false;
 }
@@ -580,12 +584,14 @@
   const auto *BinOpLhs = cast(BinOp->getLHS());
   const auto *BinOpRhs = cast(BinOp->getRHS());
 
-  LhsConst = BinOpLhs->getLHS()->isIntegerConstantExpr(*AstCtx)
- ? BinOpLhs->getLHS()
- :