[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:1307
+   (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr &&
+Current.Next->is(TT_LambdaLSquare)));
   State.Stack.back().IsInsideObjCArrayLiteral =

klimek wrote:
> I think I misunderstood some of this the first time around (and thanks for 
> bearing with me :) - iiuc we want to break for Allman even when there's only 
> one nested block. I think I'll need to take another look when I'm back from 
> vacation, unless Daniel or somebody else finds time before then (will be back 
> in 1.5 weeks)
So, HasMultipleNestedBlocks should only be true if there are multiple nested 
blocks.
What tests break if you remove this change to HasMultipleNestedBlocks?


Repository:
  rC Clang

https://reviews.llvm.org/D44609



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

[PATCH] D51261: Add preload option to clang-query

[Manuel Klimek via Phabricator via cfe-commits]

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

LG


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51261



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

[PATCH] D51259: Allow binding to NamedValue resulting from let expression

[Manuel Klimek via Phabricator via cfe-commits]

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

LG


Repository:
  rC Clang

https://reviews.llvm.org/D51259



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

[PATCH] D51258: Extract parseBindID method

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: lib/ASTMatchers/Dynamic/Parser.cpp:362
 
+bool Parser::parseBindID(std::string &BindID, TokenInfo &CloseToken) {
+  // Parse .bind("foo")

CloseToken seems to not be used afterwards either here or in the follow-up 
patch?


Repository:
  rC Clang

https://reviews.llvm.org/D51258



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

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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks for the cleanups, mostly NITs from my side.




Comment at: clangd/ClangdServer.cpp:81
 
-  SymbolIndex &index() const { return *MergedIndex; }
+  SymbolIndex &index() { return *MergedIndex; }
 

Maybe return `const SymbolIndex&` and keep the method const?



Comment at: clangd/ClangdServer.cpp:101
+};
+return llvm::make_unique(CB{this});
+  };

Maybe simplify to `make_unique(this)`? This should work, right?



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

I thought it was not true, but now I notice we actually don't index those 
symbols.
I think we should index them for workspaceSymbols, but not for completion. Any 
pointers to the code that filters them out?



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

Any reason to prefer `nullptr` over the no-op callbacks?
Might be a personal preference, my reasoning is: having an extra check for null 
before on each of the call sites both adds unnecessary boilerplate and adds an 
extra branch before the virtual call (which is, technically, another form of a 
branch).

Not opposed to doing it either way, though.



Comment at: clangd/ClangdServer.h:195
+  /// This can be useful for testing, debugging, or observing memory usage.
+  const SymbolIndex *dynamicIndex();
+

Maybe make it const?



Comment at: clangd/TUScheduler.cpp:632
  bool StorePreamblesInMemory,
- ParsingCallbacks &Callbacks,
+ std::unique_ptr Callbacks,
  std::chrono::steady_clock::duration UpdateDebounce,

Why not `ParsingCallbacks*` or `ParsingCallbacks&`? This restricts the lifetime 
of the passed values.
Our particular use-case LG this way, but it seems there is no reason why 
`TUScheduler` should own the callbacks.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51221



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

Re: [PATCH] D49840: [AST] Add MatchFinder::matchSubtree

[Manuel Klimek via cfe-commits]

On Mon, Aug 6, 2018 at 4:47 PM Gabor Marton via Phabricator <
revi...@reviews.llvm.org> wrote:

> martong added a comment.
>
> Ping.
>
> Manuel, I still don't see how could we apply `match(anyOf(node),
> hasDescendant(node))` to the problem of general subtree traversal.
> (I'd like to have support for not just `decl()` but other kind of nodes
> too.)
>
Could you please advise how to move on? Also, could you please describe
> your specific technical arguments against this patch?
>

I'm still trying to understand why it's a good idea :) The general
technical argument why I'm asking all these questions is that we shouldn't
add new things unless they provide significant new features.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50766: Fix false positive unsequenced access and modification warning in array subscript expression.

[Mateusz Janek via Phabricator via cfe-commits]

stryku updated this revision to Diff 162630.
stryku added a comment.

Added missing semicolon.


https://reviews.llvm.org/D50766

Files:
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/warn-unsequenced-cxx17.cpp
  test/SemaCXX/warn-unsequenced.cpp


Index: test/SemaCXX/warn-unsequenced.cpp
===
--- test/SemaCXX/warn-unsequenced.cpp
+++ test/SemaCXX/warn-unsequenced.cpp
@@ -81,6 +81,7 @@
   int *p = xs;
   a = *(a++, p); // ok
   a = a++ && a; // ok
+  p[(long long unsigned)(p = 0)]; // expected-warning {{unsequenced 
modification and access to 'p'}}
 
   A *q = &agg1;
   (q = &agg2)->y = q->x; // expected-warning {{unsequenced modification and 
access to 'q'}}
Index: test/SemaCXX/warn-unsequenced-cxx17.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-unsequenced-cxx17.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wno-unused %s
+
+void test() {
+  int xs[10];
+  int *p = xs;
+  // expected-no-diagnostics
+  p[(long long unsigned)(p = 0)]; // ok
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -11658,30 +11658,42 @@
   notePostUse(O, E);
   }
 
-  void VisitBinComma(BinaryOperator *BO) {
-// C++11 [expr.comma]p1:
-//   Every value computation and side effect associated with the left
-//   expression is sequenced before every value computation and side
-//   effect associated with the right expression.
-SequenceTree::Seq LHS = Tree.allocate(Region);
-SequenceTree::Seq RHS = Tree.allocate(Region);
+  void VisitSequencedExpressions(Expr *SequencedBefore, Expr *SequencedAfter) {
+SequenceTree::Seq BeforeRegion = Tree.allocate(Region);
+SequenceTree::Seq AfterRegion = Tree.allocate(Region);
 SequenceTree::Seq OldRegion = Region;
 
 {
-  SequencedSubexpression SeqLHS(*this);
-  Region = LHS;
-  Visit(BO->getLHS());
+  SequencedSubexpression SeqBefore(*this);
+  Region = BeforeRegion;
+  Visit(SequencedBefore);
 }
 
-Region = RHS;
-Visit(BO->getRHS());
+Region = AfterRegion;
+Visit(SequencedAfter);
 
 Region = OldRegion;
 
-// Forget that LHS and RHS are sequenced. They are both unsequenced
-// with respect to other stuff.
-Tree.merge(LHS);
-Tree.merge(RHS);
+Tree.merge(BeforeRegion);
+Tree.merge(AfterRegion);
+  }
+
+  void VisitArraySubscriptExpr(ArraySubscriptExpr *ASE) {
+// C++17 [expr.sub]p1:
+//   The expression E1[E2] is identical (by definition) to *((E1)+(E2)). 
The
+//   expression E1 is sequenced before the expression E2.
+if (SemaRef.getLangOpts().CPlusPlus17)
+  VisitSequencedExpressions(ASE->getLHS(), ASE->getRHS());
+else
+  Base::VisitStmt(ASE);
+  }
+
+  void VisitBinComma(BinaryOperator *BO) {
+// C++11 [expr.comma]p1:
+//   Every value computation and side effect associated with the left
+//   expression is sequenced before every value computation and side
+//   effect associated with the right expression.
+VisitSequencedExpressions(BO->getLHS(), BO->getRHS());
   }
 
   void VisitBinAssign(BinaryOperator *BO) {


Index: test/SemaCXX/warn-unsequenced.cpp
===
--- test/SemaCXX/warn-unsequenced.cpp
+++ test/SemaCXX/warn-unsequenced.cpp
@@ -81,6 +81,7 @@
   int *p = xs;
   a = *(a++, p); // ok
   a = a++ && a; // ok
+  p[(long long unsigned)(p = 0)]; // expected-warning {{unsequenced modification and access to 'p'}}
 
   A *q = &agg1;
   (q = &agg2)->y = q->x; // expected-warning {{unsequenced modification and access to 'q'}}
Index: test/SemaCXX/warn-unsequenced-cxx17.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-unsequenced-cxx17.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wno-unused %s
+
+void test() {
+  int xs[10];
+  int *p = xs;
+  // expected-no-diagnostics
+  p[(long long unsigned)(p = 0)]; // ok
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -11658,30 +11658,42 @@
   notePostUse(O, E);
   }
 
-  void VisitBinComma(BinaryOperator *BO) {
-// C++11 [expr.comma]p1:
-//   Every value computation and side effect associated with the left
-//   expression is sequenced before every value computation and side
-//   effect associated with the right expression.
-SequenceTree::Seq LHS = Tree.allocate(Region);
-SequenceTree::Seq RHS = Tree.allocate(Region);
+  void VisitSequencedExpressions(Expr *SequencedBefore, Expr *SequencedAfter) {
+SequenceTree::Seq BeforeRegion = Tree.allocate(Region);
+SequenceTree::Seq AfterRegion = Tree.allocate(Region);
 SequenceTree::Seq OldRegion

Re: r340709 - [Driver] Change MipsLinux default linker from "lld" to "ld.lld"

[Chandler Carruth via cfe-commits]

Build bots have been broken all day, so I'm trying a speculative fix in
r340727. If this doesn't work, i'll just revert all of this.

On Sun, Aug 26, 2018 at 3:51 PM Chandler Carruth 
wrote:

> FYI, test cases also seem to need updating:
>
> http://lab.llvm.org:8011/builders/clang-ppc64le-linux/builds/19575/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Amips-mti-linux.c
>
> On Sun, Aug 26, 2018 at 12:48 PM Fangrui Song via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: maskray
>> Date: Sun Aug 26 12:47:23 2018
>> New Revision: 340709
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=340709&view=rev
>> Log:
>> [Driver] Change MipsLinux default linker from "lld" to "ld.lld"
>>
>> Reviewers: kzhuravl, atanasyan
>>
>> Reviewed By: atanasyan
>>
>> Subscribers: sdardis, arichardson, jrtc27, atanasyan, cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D51234
>>
>> Modified:
>> cfe/trunk/lib/Driver/ToolChains/MipsLinux.h
>>
>> Modified: cfe/trunk/lib/Driver/ToolChains/MipsLinux.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/MipsLinux.h?rev=340709&r1=340708&r2=340709&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Driver/ToolChains/MipsLinux.h (original)
>> +++ cfe/trunk/lib/Driver/ToolChains/MipsLinux.h Sun Aug 26 12:47:23 2018
>> @@ -49,7 +49,7 @@ public:
>>}
>>
>>const char *getDefaultLinker() const override {
>> -return "lld";
>> +return "ld.lld";
>>}
>>
>>  private:
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r340727 - Try to fix this clang driver test case after r340709.

[Chandler Carruth via cfe-commits]

Author: chandlerc
Date: Mon Aug 27 01:49:20 2018
New Revision: 340727

URL: http://llvm.org/viewvc/llvm-project?rev=340727&view=rev
Log:
Try to fix this clang driver test case after r340709.

If any of the bots complain about this, I'll just revert. This test case
is essentially trying to test the exact change made, but I think this
matches the intent of the patch in question.

Modified:
cfe/trunk/test/Driver/mips-mti-linux.c

Modified: cfe/trunk/test/Driver/mips-mti-linux.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/mips-mti-linux.c?rev=340727&r1=340726&r2=340727&view=diff
==
--- cfe/trunk/test/Driver/mips-mti-linux.c (original)
+++ cfe/trunk/test/Driver/mips-mti-linux.c Mon Aug 27 01:49:20 2018
@@ -15,7 +15,7 @@
 // CHECK-BE-HF-32R2: "{{[^"]*}}clang{{[^"]*}}" {{.*}} "-triple" 
"mips-mti-linux"
 // CHECK-BE-HF-32R2-SAME: "-fuse-init-array" "-target-cpu" "mips32r2"
 // CHECK-BE-HF-32R2-SAME: "-isysroot" "{{.*}}mips_mti_linux/sysroot"
-// CHECK-BE-HF-32R2: "{{[^"]*}}lld{{[^"]*}}" "-flavor" "old-gnu" "-target" 
"mips-mti-linux"
+// CHECK-BE-HF-32R2: "{{[^"]*}}ld.lld{{[^"]*}}"
 // CHECK-BE-HF-32R2-SAME: "--sysroot=[[SYSROOT:[^"]+]]" {{.*}} 
"-dynamic-linker" "/lib/ld-musl-mips.so.1"
 // CHECK-BE-HF-32R2-SAME: 
"[[SYSROOT]]/mips-r2-hard-musl/usr/lib{{/|}}crt1.o"
 // CHECK-BE-HF-32R2-SAME: 
"[[SYSROOT]]/mips-r2-hard-musl/usr/lib{{/|}}crti.o"
@@ -33,7 +33,7 @@
 // CHECK-LE-HF-32R2: "{{[^"]*}}clang{{[^"]*}}" {{.*}} "-triple" 
"mipsel-mti-linux"
 // CHECK-LE-HF-32R2-SAME: "-fuse-init-array" "-target-cpu" "mips32r2"
 // CHECK-LE-HF-32R2-SAME: "-isysroot" "{{.*}}mips_mti_linux/sysroot"
-// CHECK-LE-HF-32R2: "{{[^"]*}}lld{{[^"]*}}" "-flavor" "old-gnu" "-target" 
"mipsel-mti-linux"
+// CHECK-LE-HF-32R2: "{{[^"]*}}ld.lld{{[^"]*}}"
 // CHECK-LE-HF-32R2-SAME: "--sysroot=[[SYSROOT:[^"]+]]" {{.*}} 
"-dynamic-linker" "/lib/ld-musl-mipsel.so.1"
 // CHECK-LE-HF-32R2-SAME: 
"[[SYSROOT]]/mipsel-r2-hard-musl/usr/lib{{/|}}crt1.o"
 // CHECK-LE-HF-32R2-SAME: 
"[[SYSROOT]]/mipsel-r2-hard-musl/usr/lib{{/|}}crti.o"


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

[PATCH] D51287: [clangd] Use TRUE iterator instead of complete posting list

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: ilya-biryukov.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
kbobyrev edited the summary of this revision.

Stop using `$$$` (empty) trigram and generating a posting list with all items. 
Since TRUE iterator is already implemented and correctly inserted when there 
are no real trigram posting lists, this is a valid transformation.

Benchmarks show that this simple change allows ~30% speedup on dataset of real 
completion queries.

Before

  ---
  BenchmarkTime   CPU Iterations
  ---
  DexAdHocQueries5640321 ns5640265 ns120
  DexRealQ 939835603 ns  939830296 ns  1

After

  ---
  BenchmarkTime   CPU Iterations
  ---
  DexAdHocQueries3452014 ns3451987 ns203
  DexRealQ 667455912 ns  667455750 ns  1


https://reviews.llvm.org/D51287

Files:
  clang-tools-extra/clangd/index/dex/Trigram.cpp


Index: clang-tools-extra/clangd/index/dex/Trigram.cpp
===
--- clang-tools-extra/clangd/index/dex/Trigram.cpp
+++ clang-tools-extra/clangd/index/dex/Trigram.cpp
@@ -67,10 +67,6 @@
 UniqueTrigrams.insert(Token(Token::Kind::Trigram, Chars));
   };
 
-  // FIXME(kbobyrev): Instead of producing empty trigram for each identifier,
-  // just use True Iterator on the query side when the query string is empty.
-  add({{END_MARKER, END_MARKER, END_MARKER}});
-
   if (TwoHeads.size() == 2)
 add({{TwoHeads.front(), TwoHeads.back(), END_MARKER}});
 


Index: clang-tools-extra/clangd/index/dex/Trigram.cpp
===
--- clang-tools-extra/clangd/index/dex/Trigram.cpp
+++ clang-tools-extra/clangd/index/dex/Trigram.cpp
@@ -67,10 +67,6 @@
 UniqueTrigrams.insert(Token(Token::Kind::Trigram, Chars));
   };
 
-  // FIXME(kbobyrev): Instead of producing empty trigram for each identifier,
-  // just use True Iterator on the query side when the query string is empty.
-  add({{END_MARKER, END_MARKER, END_MARKER}});
-
   if (TwoHeads.size() == 2)
 add({{TwoHeads.front(), TwoHeads.back(), END_MARKER}});
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50462: Try building complete AST after a fatal error was emitted if further diagnostics are expected

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Sorry for chiming in, wanted to add my 2 cents to the conversation.

In https://reviews.llvm.org/D50462#1206203, @vsapsai wrote:

> What about having a mode that treats missing header as non-fatal error? 
> Because I believe there are cases where there is no sense to continue after a 
> fatal error. For example, consider hitting the error limit. Showing too many 
> errors isn't useful and also after 50 errors it is a little bit too 
> optimistic to assume that AST is in a good shape. I don't know if there are 
> other fatal-but-we-can-continue errors and if we need a more general 
> solution. So far looks like missing include is the biggest blocker for 
> IDE-like functionality.


Your arguments are definitely valid (too many errors may be confusing and AST 
may not be prepared to recover on some fatal errors), but I would argue that 
the IDE use-case is different enough from the command-line tools to make the 
new behavior a preferred one.

Specifically, a few questions regarding the aforementioned fatal errors:

1. Why should hitting some error limit be considered a fatal error? One of the 
reasons I see for the command-line tools is not spamming the output buffers 
with too many errors, so that navigating to the first errors is easier. It 
looks like a non-issue for IDEs: they can both show all emitted errors in the 
text editor and make it easy to navigate to the first error (by providing a 
convenient UI for an error list, shortcuts to go to the first error, etc.). On 
the contrary, not seeing errors where I type because clang hit the limit before 
my line looks like a confusing experience for the editors/IDEs.

2. Why should an unresolved include (which is considered a fatal error) give a 
totally different result from the missing include (which is, obviously, 
undetectable)? They both require the same amount of work to recover and we 
obviously want clang to work in absence of some includes.

Besides, errors can also be easily "promoted" to fatal with command-line flags 
(`-Wfatal-errors`) and editor tools should probably respect those and not 
override their severity.

W.R.T. to the AST not being good enough: it may not be, but shouldn't we 
instead invest in improving the recovery on things like unresolved references?
This would give better experience in absence of non-fatal errors too (common 
case when copy-pasting code, removing too many #include directives from the 
file, etc.)  and it looks doable.

Overall, I would argue that letting clang recover from fatal errors is the 
right thing to do for IDEs and editor integrations and the original patch was 
moving in the right direction.
WDYT?


Repository:
  rC Clang

https://reviews.llvm.org/D50462



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

[PATCH] D50462: Try building complete AST after a fatal error was emitted if further diagnostics are expected

[Dmitry via Phabricator via cfe-commits]

Dmitry.Kozhevnikov added a comment.

When lookin through the list of the fatal errors, I think there are different 
categories:

1. "Scary" ones - "module was relocated", "invalid vfs overlay file", we 
probably shouldn't try to recover from it
2. "User" errors (include not found, too many errors) - we definitely shouldn't 
treat them as fatal in IDE
3. (subclass of the previous one): "something is too deep" (instantiations, 
brackets) - I'm afraid treating them as non-fatal right now would lead to a 
possibility of them happening again and again as we recover and proceed. So, in 
the future, the recovery might be more clever (i.e. going all the way up the 
instantiation/brackets stack, and then proceeding normally), however, while 
it's not done, I'm a bit uneasy demoting them from fatal.

So I'm preparing an alternative patch to demote "file not found" in include 
directive from the fatal error in .td file, and then immediately promote it 
back by default (but not in clangd).  WDYT?

In general, I find the whole concept of "Fatal error occurred/Uncompilable 
error occurred/Too many errors occurred/etc" too coarse-grained for 
tooling/IDEs. Parse/PP/Sema should probably try harder to recover, and not 
report such global state change. I'm not ready to approach it, though :)


Repository:
  rC Clang

https://reviews.llvm.org/D50462



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

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

[Eric Liu via Phabricator via cfe-commits]

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

Currently, a symbol can have only one #include header attached, which
might not work well if the symbol can be imported via different #includes 
depending
on where it's used. This patch stores multiple #include headers (with # 
references)
for each symbol, so that CodeCompletion can suggest one candidate for each
distinct #include for the same symbol and rank them according to proximity and
popularity of #include headers. In order to preserve the order of overall 
ranking
(i.e. ranking score without #include signals) and group candidates for the same 
symbol
close by, we only increment the ranking score by a small amount based on the 
relevance
and quality of #includes.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51291

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

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

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

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

@sammccall The code still needs cleanup but should be useful for providing 
high-level feedback, which I would like to get before moving further. Thanks!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51291



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

[PATCH] D51287: [clangd] Use TRUE iterator instead of complete posting list

[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


https://reviews.llvm.org/D51287



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

[PATCH] D51292: [clang-rename] Update documentation

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov.
Herald added a subscriber: kadircet.

Clangd has way better editor support compared to the ad-hoc integration I 
created before and we should point users to Clangd mentioning that they could 
still use standalone tool if they really want to.

Also, links to the Vim and Emacs scripts were not updated after the move from 
clang-tools-extra to clang.


https://reviews.llvm.org/D51292

Files:
  clang-tools-extra/docs/clang-rename.rst


Index: clang-tools-extra/docs/clang-rename.rst
===
--- clang-tools-extra/docs/clang-rename.rst
+++ clang-tools-extra/docs/clang-rename.rst
@@ -23,6 +23,12 @@
 Using Clang-Rename
 ==
 
+:program:`clangd ` uses
+:program:`clang-rename` infrastructure to handle renaming requests. Because of
+much better editor integration and support, it is advised to use
+:program:`clangd-rename` as part of :program:`clangd`. However, it is possible
+to use the standalone tool.
+
 :program:`clang-rename` is a `LibTooling
 `_-based tool, and it's easier to
 work with if you set up a compile command database for your project (for an
@@ -140,7 +146,7 @@
 You can call :program:`clang-rename` directly from Vim! To set up
 :program:`clang-rename` integration for Vim see
 `clang-rename/tool/clang-rename.py
-`_.
+`_.
 
 Please note that **you have to save all buffers, in which the replacement will
 happen before running the tool**.
@@ -157,7 +163,7 @@
 You can also use :program:`clang-rename` while using Emacs! To set up
 :program:`clang-rename` integration for Emacs see
 `clang-rename/tool/clang-rename.el
-`_.
+`_.
 
 Once installed, you can point your cursor to symbols you want to rename, press
 `M-X`, type `clang-rename` and new desired name.


Index: clang-tools-extra/docs/clang-rename.rst
===
--- clang-tools-extra/docs/clang-rename.rst
+++ clang-tools-extra/docs/clang-rename.rst
@@ -23,6 +23,12 @@
 Using Clang-Rename
 ==
 
+:program:`clangd ` uses
+:program:`clang-rename` infrastructure to handle renaming requests. Because of
+much better editor integration and support, it is advised to use
+:program:`clangd-rename` as part of :program:`clangd`. However, it is possible
+to use the standalone tool.
+
 :program:`clang-rename` is a `LibTooling
 `_-based tool, and it's easier to
 work with if you set up a compile command database for your project (for an
@@ -140,7 +146,7 @@
 You can call :program:`clang-rename` directly from Vim! To set up
 :program:`clang-rename` integration for Vim see
 `clang-rename/tool/clang-rename.py
-`_.
+`_.
 
 Please note that **you have to save all buffers, in which the replacement will
 happen before running the tool**.
@@ -157,7 +163,7 @@
 You can also use :program:`clang-rename` while using Emacs! To set up
 :program:`clang-rename` integration for Emacs see
 `clang-rename/tool/clang-rename.el
-`_.
+`_.
 
 Once installed, you can point your cursor to symbols you want to rename, press
 `M-X`, type `clang-rename` and new desired name.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51292: [clang-rename] Update documentation

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 162643.
kbobyrev added a comment.

Add missing `_` after the link.


https://reviews.llvm.org/D51292

Files:
  clang-tools-extra/docs/clang-rename.rst


Index: clang-tools-extra/docs/clang-rename.rst
===
--- clang-tools-extra/docs/clang-rename.rst
+++ clang-tools-extra/docs/clang-rename.rst
@@ -23,6 +23,12 @@
 Using Clang-Rename
 ==
 
+:program:`clangd `_ uses
+:program:`clang-rename` infrastructure to handle renaming requests. Because of
+much better editor integration and support, it is advised to use
+:program:`clangd-rename` as part of :program:`clangd`. However, it is possible
+to use the standalone tool.
+
 :program:`clang-rename` is a `LibTooling
 `_-based tool, and it's easier to
 work with if you set up a compile command database for your project (for an
@@ -140,7 +146,7 @@
 You can call :program:`clang-rename` directly from Vim! To set up
 :program:`clang-rename` integration for Vim see
 `clang-rename/tool/clang-rename.py
-`_.
+`_.
 
 Please note that **you have to save all buffers, in which the replacement will
 happen before running the tool**.
@@ -157,7 +163,7 @@
 You can also use :program:`clang-rename` while using Emacs! To set up
 :program:`clang-rename` integration for Emacs see
 `clang-rename/tool/clang-rename.el
-`_.
+`_.
 
 Once installed, you can point your cursor to symbols you want to rename, press
 `M-X`, type `clang-rename` and new desired name.


Index: clang-tools-extra/docs/clang-rename.rst
===
--- clang-tools-extra/docs/clang-rename.rst
+++ clang-tools-extra/docs/clang-rename.rst
@@ -23,6 +23,12 @@
 Using Clang-Rename
 ==
 
+:program:`clangd `_ uses
+:program:`clang-rename` infrastructure to handle renaming requests. Because of
+much better editor integration and support, it is advised to use
+:program:`clangd-rename` as part of :program:`clangd`. However, it is possible
+to use the standalone tool.
+
 :program:`clang-rename` is a `LibTooling
 `_-based tool, and it's easier to
 work with if you set up a compile command database for your project (for an
@@ -140,7 +146,7 @@
 You can call :program:`clang-rename` directly from Vim! To set up
 :program:`clang-rename` integration for Vim see
 `clang-rename/tool/clang-rename.py
-`_.
+`_.
 
 Please note that **you have to save all buffers, in which the replacement will
 happen before running the tool**.
@@ -157,7 +163,7 @@
 You can also use :program:`clang-rename` while using Emacs! To set up
 :program:`clang-rename` integration for Emacs see
 `clang-rename/tool/clang-rename.el
-`_.
+`_.
 
 Once installed, you can point your cursor to symbols you want to rename, press
 `M-X`, type `clang-rename` and new desired name.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r340729 - [clangd] Use TRUE iterator instead of complete posting list

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Mon Aug 27 02:47:50 2018
New Revision: 340729

URL: http://llvm.org/viewvc/llvm-project?rev=340729&view=rev
Log:
[clangd] Use TRUE iterator instead of complete posting list

Stop using `$$$` (empty) trigram and generating a posting list with all
items. Since TRUE iterator is already implemented and correctly inserted
when there are no real trigram posting lists, this is a valid
transformation.

Benchmarks show that this simple change allows ~30% speedup on dataset
of real completion queries.

Before

```
---
BenchmarkTime   CPU Iterations
---
DexAdHocQueries5640321 ns5640265 ns120
DexRealQ 939835603 ns  939830296 ns  1
```

After

```
---
BenchmarkTime   CPU Iterations
---
DexAdHocQueries3452014 ns3451987 ns203
DexRealQ 667455912 ns  667455750 ns  1
```

Reviewed by: ilya-biryukov

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

Modified:
clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp

Modified: clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp?rev=340729&r1=340728&r2=340729&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp Mon Aug 27 02:47:50 
2018
@@ -67,10 +67,6 @@ std::vector generateIdentifierTri
 UniqueTrigrams.insert(Token(Token::Kind::Trigram, Chars));
   };
 
-  // FIXME(kbobyrev): Instead of producing empty trigram for each identifier,
-  // just use True Iterator on the query side when the query string is empty.
-  add({{END_MARKER, END_MARKER, END_MARKER}});
-
   if (TwoHeads.size() == 2)
 add({{TwoHeads.front(), TwoHeads.back(), END_MARKER}});
 


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

[PATCH] D51287: [clangd] Use TRUE iterator instead of complete posting list

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340729: [clangd] Use TRUE iterator instead of complete 
posting list (authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51287?vs=162633&id=162644#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51287

Files:
  clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp


Index: clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
===
--- clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
+++ clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
@@ -67,10 +67,6 @@
 UniqueTrigrams.insert(Token(Token::Kind::Trigram, Chars));
   };
 
-  // FIXME(kbobyrev): Instead of producing empty trigram for each identifier,
-  // just use True Iterator on the query side when the query string is empty.
-  add({{END_MARKER, END_MARKER, END_MARKER}});
-
   if (TwoHeads.size() == 2)
 add({{TwoHeads.front(), TwoHeads.back(), END_MARKER}});
 


Index: clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
===
--- clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
+++ clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
@@ -67,10 +67,6 @@
 UniqueTrigrams.insert(Token(Token::Kind::Trigram, Chars));
   };
 
-  // FIXME(kbobyrev): Instead of producing empty trigram for each identifier,
-  // just use True Iterator on the query side when the query string is empty.
-  add({{END_MARKER, END_MARKER, END_MARKER}});
-
   if (TwoHeads.size() == 2)
 add({{TwoHeads.front(), TwoHeads.back(), END_MARKER}});
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r340376 - Update the docs for using LLVM toolset in Visual Studio

[Hans Wennborg via cfe-commits]

Merged to 7.0 in r340730.

On Wed, Aug 22, 2018 at 3:11 AM, Stephen Kelly via cfe-commits
 wrote:
> Author: steveire
> Date: Tue Aug 21 18:11:18 2018
> New Revision: 340376
>
> URL: http://llvm.org/viewvc/llvm-project?rev=340376&view=rev
> Log:
> Update the docs for using LLVM toolset in Visual Studio
>
> Reviewers: hans
>
> Subscribers: cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D51079
>
> Modified:
> cfe/trunk/docs/UsersManual.rst
>
> Modified: cfe/trunk/docs/UsersManual.rst
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=340376&r1=340375&r2=340376&view=diff
> ==
> --- cfe/trunk/docs/UsersManual.rst (original)
> +++ cfe/trunk/docs/UsersManual.rst Tue Aug 21 18:11:18 2018
> @@ -2711,16 +2711,17 @@ Command Prompt or a regular Command Prom
>  up using e.g. `vcvarsall.bat 
> `_.
>
>  clang-cl can also be used from inside Visual Studio by selecting the LLVM
> -Platform Toolset. The toolset is installed by the LLVM installer, which can 
> be
> -downloaded from the `LLVM release `_ 
> or
> -`snapshot build `_ web pages. To use the toolset,
> -select a project in Solution Explorer, open its Property Page (Alt+F7), and 
> in
> -the "General" section of "Configuration Properties" change "Platform Toolset"
> -to e.g. LLVM-vs2014.
> +Platform Toolset. The toolset is not part of the installer, but may be 
> installed
> +separately from the
> +`Visual Studio Marketplace 
> `_.
> +To use the toolset, select a project in Solution Explorer, open its Property
> +Page (Alt+F7), and in the "General" section of "Configuration Properties"
> +change "Platform Toolset" to LLVM.  Doing so enables an additional Property
> +Page for selecting the clang-cl executable to use for builds.
>
>  To use the toolset with MSBuild directly, invoke it with e.g.
> -``/p:PlatformToolset=LLVM-vs2014``. This allows trying out the clang-cl
> -toolchain without modifying your project files.
> +``/p:PlatformToolset=LLVM``. This allows trying out the clang-cl toolchain
> +without modifying your project files.
>
>  It's also possible to point MSBuild at clang-cl without changing toolset by
>  passing ``/p:CLToolPath=c:\llvm\bin /p:CLToolExe=clang-cl.exe``.
> @@ -2729,7 +2730,7 @@ When using CMake and the Visual Studio g
>
>::
>
> -cmake -G"Visual Studio 15 2017" -T LLVM-vs2014 ..
> +cmake -G"Visual Studio 15 2017" -T LLVM ..
>
>  When using CMake with the Ninja generator, set the ``CMAKE_C_COMPILER`` and
>  ``CMAKE_CXX_COMPILER`` variables to clang-cl:
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51212: [OpenCL][Docs] Release notes for OpenCL in Clang

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Anastasia: will you commit this to the branch, or would like me to do it?


https://reviews.llvm.org/D51212



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

[PATCH] D51293: [docs] Mention clangd-dev in clangd documentation

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov, sammccall.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.

Since the clangd-dev is intended to be the place for clangd-related 
discussions, we should point new users to this mailing list while probably 
mentioning cfe-dev, too.


https://reviews.llvm.org/D51293

Files:
  clang-tools-extra/docs/clangd.rst


Index: clang-tools-extra/docs/clangd.rst
===
--- clang-tools-extra/docs/clangd.rst
+++ clang-tools-extra/docs/clangd.rst
@@ -111,7 +111,10 @@
 Getting Involved
 ==
 
-A good place for interested contributors is the `Clang developer mailing list
+A good place for interested contributors is the `Clangd developer mailing list
+`_. For discussions with the
+broader community on topics not only related to Clangd, use
+`Clang developer mailing list
 `_.
 If you're also interested in contributing patches to :program:`Clangd`, take a
 look at the `LLVM Developer Policy


Index: clang-tools-extra/docs/clangd.rst
===
--- clang-tools-extra/docs/clangd.rst
+++ clang-tools-extra/docs/clangd.rst
@@ -111,7 +111,10 @@
 Getting Involved
 ==
 
-A good place for interested contributors is the `Clang developer mailing list
+A good place for interested contributors is the `Clangd developer mailing list
+`_. For discussions with the
+broader community on topics not only related to Clangd, use
+`Clang developer mailing list
 `_.
 If you're also interested in contributing patches to :program:`Clangd`, take a
 look at the `LLVM Developer Policy
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Owen Pan via Phabricator via cfe-commits]

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

See https://bugs.llvm.org/show_bug.cgi?id=38713


Repository:
  rC Clang

https://reviews.llvm.org/D51294

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


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


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

[PATCH] D51279: [clangd] Implement findOccurrences interface in dynamic index.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

The memory usage looks good. Some NITs and a major consideration wrt to the 
implementation of merging occurences from dynamic and static index.




Comment at: clangd/index/FileIndex.cpp:48
 
+  if (TopLevelDecls) { // index main AST, set occurrence flag.
+CollectorOpts.OccurrenceFilter = SymbolOccurrenceKind::Declaration |

There is an implicit assumption (`TopLevelDecls != null` iff indexing the main 
file AST in dynamic index) that does not seem quite obvious.
Maybe add two overloads (for indexing the main AST and for indexing the 
preamble AST) or add an extra parameter to be explicit more about those choices?



Comment at: clangd/index/FileIndex.cpp:74
+  if (!Slab) {
 FileToSlabs.erase(Path);
+  } else {

NIT: maybe remove braces around single-statement branches?



Comment at: clangd/index/FileIndex.cpp:126
 auto Slab = llvm::make_unique();
-*Slab = indexAST(*AST, PP, TopLevelDecls, URISchemes);
-FSymbols.update(Path, std::move(Slab));
+auto IndexResults = indexAST(*AST, PP, TopLevelDecls, URISchemes);
+*Slab = std::move(IndexResults.first);

Why not `std::tie(*Slab, *OccurenceSlab) = indexAST(...)`?
I assume this should work and give the optimal performance



Comment at: clangd/index/FileIndex.h:45
+  void update(PathRef Path, std::unique_ptr Slab,
+  std::unique_ptr Occurrences = nullptr);
 

Maybe avoid default arguments? Having clients pass nullptr explicitly seems 
like the right thing to do



Comment at: clangd/index/FileIndex.h:100
 /// level decls obtained from \p AST are indexed.
-SymbolSlab
+std::pair
 indexAST(ASTContext &AST, std::shared_ptr PP,

Maybe use a struct to allow named fields? Would mean a tiny amount of extra 
code in `indexAST`, but should make the client code slightly nicer.



Comment at: clangd/index/MemIndex.h:27
+  build(std::shared_ptr> Symbols,
+std::vector> OccurrenceSlabs = 
{});
 

Maybe remove the default parameter? Making the callers specify the occurrences 
explicitly could make their code more straight-forward to follow.



Comment at: clangd/index/Merge.cpp:88
+// in seen files.
+// FIXME: If a file has been updated, and there are no occurrences indexed
+// in dynamic index, stale results are still returned (from static index).

Do we want to fix it before checking this in? As discussed offline, this seems 
to be a limitation that could affect the design (FileOccurrences vs 
SymbolOccurrence in the symbol interface). In case we want to avoid refactoring 
the interfaces later.



Comment at: clangd/index/Merge.cpp:95
+Static->findOccurrences(Req, [&](const SymbolOccurrence& O) {
+  if (llvm::is_contained(SeenFiles, O.Location.FileURI))
+return;

`llvm::is_contained` seems to be linear time, maybe replace with hash-table 
lookup.
Or am I missing something?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51279



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

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

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 162650.
ioeric added a comment.

- minor cleanup


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51291

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

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

[PATCH] D51260: Extract runCommandsInFile method

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I'm not opposed to the changes here, but I am wondering what the benefits are 
to splitting this off into its own function?




Comment at: clang-query/tool/ClangQuery.cpp:61
 
+int runCommandsInFile(const char* exeName, std::string const& fileName, 
QuerySession& QS)
+{

aaron.ballman wrote:
> You should run your patch through clang-format to properly format only the 
> parts that you've changed. Also, it should be `ExeName` and `FileName` per 
> the coding standard.
I think this function should return a `bool` instead of an `int`. The caller 
can decide how to translate failure into a return code.



Comment at: clang-query/tool/ClangQuery.cpp:61-62
 
+int runCommandsInFile(const char* exeName, std::string const& fileName, 
QuerySession& QS)
+{
+std::ifstream Input(fileName.c_str());

You should run your patch through clang-format to properly format only the 
parts that you've changed. Also, it should be `ExeName` and `FileName` per the 
coding standard.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51260



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

[PATCH] D51292: [docs] Update clang-rename documentation

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/docs/clang-rename.rst:28
+:program:`clang-rename` infrastructure to handle renaming requests. Because of
+much better editor integration and support, it is advised to use
+:program:`clangd-rename` as part of :program:`clangd`. However, it is possible

I would not recommend `clangd` at this point, it only supports single-file 
renames, while `clang-rename` can actually do cross-TU renames IIUC.
Not opposed to putting recommendations of using `clangd`, of course, but let's 
be explicit about the limitations.


https://reviews.llvm.org/D51292



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

[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier

[Marco Elver via Phabricator via cfe-commits]

melver updated this revision to Diff 162653.
melver marked 4 inline comments as done.
melver added a comment.

Many thanks! PTAL.


Repository:
  rC Clang

https://reviews.llvm.org/D51036

Files:
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/TokenAnnotator.h
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -200,6 +200,42 @@
"inti;\n"
"}",
getGoogleStyle()));
+  EXPECT_EQ("/* something */ namespace N {\n"
+"\n"
+"int i;\n"
+"}",
+format("/* something */ namespace N {\n"
+   "\n"
+   "inti;\n"
+   "}",
+   getGoogleStyle()));
+  EXPECT_EQ("inline namespace N {\n"
+"\n"
+"int i;\n"
+"}",
+format("inline namespace N {\n"
+   "\n"
+   "inti;\n"
+   "}",
+   getGoogleStyle()));
+  EXPECT_EQ("/* something */ inline namespace N {\n"
+"\n"
+"int i;\n"
+"}",
+format("/* something */ inline namespace N {\n"
+   "\n"
+   "inti;\n"
+   "}",
+   getGoogleStyle()));
+  EXPECT_EQ("export namespace N {\n"
+"\n"
+"int i;\n"
+"}",
+format("export namespace N {\n"
+   "\n"
+   "inti;\n"
+   "}",
+   getGoogleStyle()));
   EXPECT_EQ("extern /**/ \"C\" /**/ {\n"
 "\n"
 "int i;\n"
@@ -1186,12 +1222,25 @@
"private:\n"
"  void f() {}\n"
"};");
+  verifyFormat("export class A {\n"
+   "public:\n"
+   "public: // comment\n"
+   "protected:\n"
+   "private:\n"
+   "  void f() {}\n"
+   "};");
   verifyGoogleFormat("class A {\n"
  " public:\n"
  " protected:\n"
  " private:\n"
  "  void f() {}\n"
  "};");
+  verifyGoogleFormat("export class A {\n"
+ " public:\n"
+ " protected:\n"
+ " private:\n"
+ "  void f() {}\n"
+ "};");
   verifyFormat("class A {\n"
"public slots:\n"
"  void f1() {}\n"
@@ -1563,16 +1612,36 @@
"void f() { f(); }\n"
"}",
LLVMWithNoNamespaceFix);
+  verifyFormat("/* something */ namespace some_namespace {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
   verifyFormat("namespace {\n"
"class A {};\n"
"void f() { f(); }\n"
"}",
LLVMWithNoNamespaceFix);
+  verifyFormat("/* something */ namespace {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
   verifyFormat("inline namespace X {\n"
"class A {};\n"
"void f() { f(); }\n"
"}",
LLVMWithNoNamespaceFix);
+  verifyFormat("/* something */ inline namespace X {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("export namespace X {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
   verifyFormat("using namespace some_namespace;\n"
"class A {};\n"
"void f() { f(); }",
@@ -7556,6 +7625,12 @@
   verifyFormat("inline namespace Foo\n"
"{};",
Style);
+  verifyFormat("/* something */ inline namespace Foo\n"
+   "{};",
+   Style);
+  verifyFormat("export namespace Foo\n"
+   "{};",
+   Style);
   verifyFormat("namespace Foo\n"
"{\n"
"void Bar();\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -989,13 +989,6 @@
   case tok::kw_namespace:
 parseNamespace();
 return;
-  case tok::kw_inline:
-nextToken();
-if (FormatTok->Tok.is(tok::kw_namespace)) {
-  parseNamespace();
-  return;
-}
-break;
   case tok::kw_public:
   case tok::kw_protected:
   case tok::kw_pr

[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier

[Marco Elver via Phabricator via cfe-commits]

melver added inline comments.



Comment at: lib/Format/Format.cpp:1312
   auto &Line = *AnnotatedLines[i];
   if (Line.startsWith(tok::kw_namespace) ||
+  Line.startsWith(tok::kw_inline, tok::kw_namespace) ||

owenpan wrote:
> sammccall wrote:
> > these 3 startswith checks appear 4 times now, you could pull out a helper 
> > function `startsWithNamespace` in FormatToken.h or something like that.
> > Up to you.
> I missed it. Good catch!
Added startsWithNamespace to TokenAnnotator.h.



Comment at: unittests/Format/FormatTest.cpp:7582
Style);
+  verifyFormat("export namespace Foo\n"
+   "{};",

sammccall wrote:
> you may want to add tests for other modules TS syntax (e.g. non-namespace 
> export decls).
> It seems this works well today, but without tests it could regress.
I've added a couple for 'export class' with access specifiers. Otherwise, any 
other additions should probably be in future non-namespace related patches.


Repository:
  rC Clang

https://reviews.llvm.org/D51036



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

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: lib/Basic/SourceManager.cpp:1626
+if (FileContentCache->OrigEntry == SourceFile) {
+  if (Callback(FileID::get(I)))
+return true;

Should we check `FileID::get(I)` is valid?



Comment at: lib/Basic/SourceManager.cpp:1628
+return true;
+} else if (LookForFilesystemUpdates &&
+   (SourceFileName || (SourceFileName = llvm::sys::path::filename(

The conditions here are pretty hard to follow and reason about. Could we maybe 
split them (some documentation would also help)?



Comment at: lib/Basic/SourceManager.cpp:1628
+return true;
+} else if (LookForFilesystemUpdates &&
+   (SourceFileName || (SourceFileName = llvm::sys::path::filename(

ioeric wrote:
> The conditions here are pretty hard to follow and reason about. Could we 
> maybe split them (some documentation would also help)?
In the original version, file system updates are checked last (after modules). 
Any reason to move it before modules? 

Also, it seems that this code path could also be run when `FileID`above is 
invalid? So I wonder whether `else if` is correct here.


https://reviews.llvm.org/D50926



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

[PATCH] D51295: Allow resetting of NumCreatedFIDsForFileID

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev created this revision.
v.g.vassilev added reviewers: rsmith, akyrtzi.
Herald added a subscriber: cfe-commits.

Our internal clients implement parsing cache based on FileID. In order for the 
Preprocessor to reenter the cached FileID it needs to reset its 
NumCreatedFIDsForFileID.

Alternatively we can simply remove the assert ;)


Repository:
  rC Clang

https://reviews.llvm.org/D51295

Files:
  include/clang/Basic/SourceManager.h


Index: include/clang/Basic/SourceManager.h
===
--- include/clang/Basic/SourceManager.h
+++ include/clang/Basic/SourceManager.h
@@ -1024,13 +1024,14 @@
 
   /// Set the number of FileIDs (files and macros) that were created
   /// during preprocessing of \p FID, including it.
-  void setNumCreatedFIDsForFileID(FileID FID, unsigned NumFIDs) const {
+  void setNumCreatedFIDsForFileID(FileID FID, unsigned NumFIDs,
+  bool Force = false) const {
 bool Invalid = false;
 const SrcMgr::SLocEntry &Entry = getSLocEntry(FID, &Invalid);
 if (Invalid || !Entry.isFile())
   return;
 
-assert(Entry.getFile().NumCreatedFIDs == 0 && "Already set!");
+assert(Force || Entry.getFile().NumCreatedFIDs == 0 && "Already set!");
 const_cast(Entry.getFile()).NumCreatedFIDs = NumFIDs;
   }
 


Index: include/clang/Basic/SourceManager.h
===
--- include/clang/Basic/SourceManager.h
+++ include/clang/Basic/SourceManager.h
@@ -1024,13 +1024,14 @@
 
   /// Set the number of FileIDs (files and macros) that were created
   /// during preprocessing of \p FID, including it.
-  void setNumCreatedFIDsForFileID(FileID FID, unsigned NumFIDs) const {
+  void setNumCreatedFIDsForFileID(FileID FID, unsigned NumFIDs,
+  bool Force = false) const {
 bool Invalid = false;
 const SrcMgr::SLocEntry &Entry = getSLocEntry(FID, &Invalid);
 if (Invalid || !Entry.isFile())
   return;
 
-assert(Entry.getFile().NumCreatedFIDs == 0 && "Already set!");
+assert(Force || Entry.getFile().NumCreatedFIDs == 0 && "Already set!");
 const_cast(Entry.getFile()).NumCreatedFIDs = NumFIDs;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51279: [clangd] Implement findOccurrences interface in dynamic index.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Just noticed I'm not on the reviewers list, sorry for chiming in without 
invitation. Was really excited about the change :-)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51279



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

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

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

Szelethus updated this revision to Diff 162658.
Szelethus marked 5 inline comments as done.
Szelethus added a comment.

Actually, the note messages were incorrect as in this case the static and 
dynamic type of the object differs.


https://reviews.llvm.org/D50892

Files:
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
  test/Analysis/cxx-uninitialized-object-inheritance.cpp


Index: test/Analysis/cxx-uninitialized-object-inheritance.cpp
===
--- test/Analysis/cxx-uninitialized-object-inheritance.cpp
+++ test/Analysis/cxx-uninitialized-object-inheritance.cpp
@@ -781,21 +781,38 @@
 // Dynamic type test.
 
//===--===//
 
-struct DynTBase {};
-struct DynTDerived : DynTBase {
-  // TODO: we'd expect the note: {{uninitialized field 'this->x'}}
-  int x; // no-note
+struct DynTBase1 {};
+struct DynTDerived1 : DynTBase1 {
+  int y; // expected-note{{uninitialized field 'static_cast(this->bptr)->y'}}
 };
 
-struct DynamicTypeTest {
-  DynTBase *bptr;
+struct DynamicTypeTest1 {
+  DynTBase1 *bptr;
   int i = 0;
 
-  // TODO: we'd expect the warning: {{1 uninitialized field}}
-  DynamicTypeTest(DynTBase *bptr) : bptr(bptr) {} // no-warning
+  DynamicTypeTest1(DynTBase1 *bptr) : bptr(bptr) {} // expected-warning{{1 
uninitialized field}}
 };
 
-void f() {
-  DynTDerived d;
-  DynamicTypeTest t(&d);
+void fDynamicTypeTest1() {
+  DynTDerived1 d;
+  DynamicTypeTest1 t(&d);
 };
+
+struct DynTBase2 {
+  int x; // expected-note{{uninitialized field 'static_cast(this->bptr)->DynTBase2::x'}}
+};
+struct DynTDerived2 : DynTBase2 {
+  int y; // expected-note{{uninitialized field 'static_cast(this->bptr)->y'}}
+};
+
+struct DynamicTypeTest2 {
+  DynTBase2 *bptr;
+  int i = 0;
+
+  DynamicTypeTest2(DynTBase2 *bptr) : bptr(bptr) {} // expected-warning{{2 
uninitialized fields}}
+};
+
+void fDynamicTypeTest2() {
+  DynTDerived2 d;
+  DynamicTypeTest2 t(&d);
+}
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
@@ -240,5 +240,10 @@
   break;
   }
 
+  while (R->getAs()) {
+NeedsCastBack = true;
+R = R->getSuperRegion()->getAs();
+  }
+
   return std::make_pair(R, NeedsCastBack);
 }


Index: test/Analysis/cxx-uninitialized-object-inheritance.cpp
===
--- test/Analysis/cxx-uninitialized-object-inheritance.cpp
+++ test/Analysis/cxx-uninitialized-object-inheritance.cpp
@@ -781,21 +781,38 @@
 // Dynamic type test.
 //===--===//
 
-struct DynTBase {};
-struct DynTDerived : DynTBase {
-  // TODO: we'd expect the note: {{uninitialized field 'this->x'}}
-  int x; // no-note
+struct DynTBase1 {};
+struct DynTDerived1 : DynTBase1 {
+  int y; // expected-note{{uninitialized field 'static_cast(this->bptr)->y'}}
 };
 
-struct DynamicTypeTest {
-  DynTBase *bptr;
+struct DynamicTypeTest1 {
+  DynTBase1 *bptr;
   int i = 0;
 
-  // TODO: we'd expect the warning: {{1 uninitialized field}}
-  DynamicTypeTest(DynTBase *bptr) : bptr(bptr) {} // no-warning
+  DynamicTypeTest1(DynTBase1 *bptr) : bptr(bptr) {} // expected-warning{{1 uninitialized field}}
 };
 
-void f() {
-  DynTDerived d;
-  DynamicTypeTest t(&d);
+void fDynamicTypeTest1() {
+  DynTDerived1 d;
+  DynamicTypeTest1 t(&d);
 };
+
+struct DynTBase2 {
+  int x; // expected-note{{uninitialized field 'static_cast(this->bptr)->DynTBase2::x'}}
+};
+struct DynTDerived2 : DynTBase2 {
+  int y; // expected-note{{uninitialized field 'static_cast(this->bptr)->y'}}
+};
+
+struct DynamicTypeTest2 {
+  DynTBase2 *bptr;
+  int i = 0;
+
+  DynamicTypeTest2(DynTBase2 *bptr) : bptr(bptr) {} // expected-warning{{2 uninitialized fields}}
+};
+
+void fDynamicTypeTest2() {
+  DynTDerived2 d;
+  DynamicTypeTest2 t(&d);
+}
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
@@ -240,5 +240,10 @@
   break;
   }
 
+  while (R->getAs()) {
+NeedsCastBack = true;
+R = R->getSuperRegion()->getAs();
+  }
+
   return std::make_pair(R, NeedsCastBack);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Viktoria Maximova via Phabricator via cfe-commits]

vmaksimo created this revision.
vmaksimo added reviewers: bader, Anastasia, AlexeySotkin.
vmaksimo created this object with edit policy "Only User: vmaksimo (Viktoria 
Maximova)".
Herald added a subscriber: cfe-commits.

Given the following kernel:
__kernel void foo()
{

  double d;
  double4 dd;

}

and `cl_khr_fp64` disabled, the compilation would fail due to
the presence of 'double d', but when removed, it passes.

The expectation is that extended vector types of unsupported types
will also be unsupported.

This patch adds this check for this scenario.


Repository:
  rC Clang

https://reviews.llvm.org/D51296

Files:
  lib/Sema/Sema.cpp
  test/SemaOpenCL/extensions.cl


Index: test/SemaOpenCL/extensions.cl
===
--- test/SemaOpenCL/extensions.cl
+++ test/SemaOpenCL/extensions.cl
@@ -70,6 +70,13 @@
 // expected-error@-2{{use of type 'double' requires cl_khr_fp64 extension to 
be enabled}}
 #endif
 
+  typedef double double4 __attribute__((ext_vector_type(4)));
+  double4 d4 = {0.0f, 2.0f, 3.0f, 1.0f};
+#ifdef NOFP64
+// expected-error@-3 {{use of type 'double' requires cl_khr_fp64 extension to 
be enabled}}
+// expected-error@-3 {{use of type 'double4' (vector of 4 'double' values) 
requires cl_khr_fp64 extension to be enabled}}
+#endif
+
   (void) 1.0;
 
 #ifdef NOFP64
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -39,6 +39,7 @@
 #include "clang/Sema/TemplateDeduction.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
+
 using namespace clang;
 using namespace sema;
 
@@ -1852,6 +1853,14 @@
   if (auto TagT = dyn_cast(QT.getCanonicalType().getTypePtr()))
 Decl = TagT->getDecl();
   auto Loc = DS.getTypeSpecTypeLoc();
+
+  // Check extensions for vector types.
+  // e.g. double4 is not allowed when cl_khr_fp64 is absent.
+  if (QT->isExtVectorType()) {
+auto TypePtr = QT->castAs()->getElementType().getTypePtr();
+return checkOpenCLDisabledTypeOrDecl(TypePtr, Loc, QT, OpenCLTypeExtMap);
+  }
+
   if (checkOpenCLDisabledTypeOrDecl(Decl, Loc, QT, OpenCLDeclExtMap))
 return true;
 


Index: test/SemaOpenCL/extensions.cl
===
--- test/SemaOpenCL/extensions.cl
+++ test/SemaOpenCL/extensions.cl
@@ -70,6 +70,13 @@
 // expected-error@-2{{use of type 'double' requires cl_khr_fp64 extension to be enabled}}
 #endif
 
+  typedef double double4 __attribute__((ext_vector_type(4)));
+  double4 d4 = {0.0f, 2.0f, 3.0f, 1.0f};
+#ifdef NOFP64
+// expected-error@-3 {{use of type 'double' requires cl_khr_fp64 extension to be enabled}}
+// expected-error@-3 {{use of type 'double4' (vector of 4 'double' values) requires cl_khr_fp64 extension to be enabled}}
+#endif
+
   (void) 1.0;
 
 #ifdef NOFP64
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -39,6 +39,7 @@
 #include "clang/Sema/TemplateDeduction.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
+
 using namespace clang;
 using namespace sema;
 
@@ -1852,6 +1853,14 @@
   if (auto TagT = dyn_cast(QT.getCanonicalType().getTypePtr()))
 Decl = TagT->getDecl();
   auto Loc = DS.getTypeSpecTypeLoc();
+
+  // Check extensions for vector types.
+  // e.g. double4 is not allowed when cl_khr_fp64 is absent.
+  if (QT->isExtVectorType()) {
+auto TypePtr = QT->castAs()->getElementType().getTypePtr();
+return checkOpenCLDisabledTypeOrDecl(TypePtr, Loc, QT, OpenCLTypeExtMap);
+  }
+
   if (checkOpenCLDisabledTypeOrDecl(Decl, Loc, QT, OpenCLDeclExtMap))
 return true;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov, sammccall.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.

https://reviews.llvm.org/D51297

Files:
  clang-tools-extra/docs/clangd-vim.rst
  clang-tools-extra/docs/index.rst
  clang/docs/ClangFormat.rst

Index: clang/docs/ClangFormat.rst
===
--- clang/docs/ClangFormat.rst
+++ clang/docs/ClangFormat.rst
@@ -94,6 +94,16 @@
 Available style options are described in :doc:`ClangFormatStyleOptions`.
 
 
+Clangd Integration
+==
+
+Most editors have :program:`Clangd `_
+integration and :program:`Clang-Format` can be invoked with the formatting
+request. It is advised to use :program:`Clangd` editor integration to get the
+best user experience. A complete example of setting up :program:`Clangd` for
+:program:`vim`: `Clangd Vim integration
+`_.
+
 Vim Integration
 ===
 
Index: clang-tools-extra/docs/index.rst
===
--- clang-tools-extra/docs/index.rst
+++ clang-tools-extra/docs/index.rst
@@ -26,6 +26,7 @@
pp-trace
clang-rename
clangd
+   clangd-vim
clang-doc
 
 
Index: clang-tools-extra/docs/clangd-vim.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clangd-vim.rst
@@ -0,0 +1,171 @@
+==
+Clangd Vim integration
+==
+
+.. contents::
+
+.. toctree::
+   :maxdepth: 1
+
+:program:`Clangd` can be used within (Neo)Vim.
+
+===
+Vim LSP plugins
+===
+
+There are several existing LSP plugins for (Neo)Vim, most notably:
+
+* `LanguageClient-neovim `_
+  is probably the one getting most traction at the moment. It supports more
+  features than its counterparts and also works better for NeoVim users
+  (despite the name it also works for regular Vim).
+* `vim-lsc `_ provides convenient key
+  bindings and has a lot of options, but lacks few important features such as
+  `code formatting `_ which can
+  be used instead to invoke
+  `Clang-Format `_.
+* `vim-lsp `_ is another (Neo)Vim
+  async plugin which offers LSP integration, but it also lacks several features
+  like `code actions `_ (
+  needed for applying Fix-Its).
+
+Given that `LanguageClient-neovim` supports more features at the moment, this
+document will focus on setting up this plugin in your (Neo)Vim.
+
+Setting up `LanguageClient-neovim`
+==
+
+You can see the generic setup guide in `plugin's README
+`_.
+Here we will focus on a way to build a complete :program:`Clangd` setup.
+
+The recommended way of installing plugins is `vim-plug
+`_ which offers various ``Plug`` options
+used for easier plugin setup.
+
+.. code-block:: vim
+
+  Plug 'autozimu/LanguageClient-neovim', {
+  \ 'branch': 'next',
+  \ 'do': 'bash install.sh',
+  \ }
+
+Next step is to specify :program:`Clangd` invokation command. The easiest way to
+do that is just telling ``LanguageClient-neovim`` to run :program:`clangd`
+binary (assuming it is already in the ``$PATH``):
+
+.. code-block:: vim
+
+  let g:LanguageClient_serverCommands = {
+  \ 'cpp': ['clangd'],
+  \ }
+
+That's all, this is a complete ``LanguageClient-neovim`` setup! However, it is
+recommended to use few other plugins and setup custom key bindings for better
+user experience. Please refer to the `Recommended Settings` section.
+
+Recommended settings
+
+
+``LanguageClient-neovim`` has multiple options for the completion management:
+
+* `deoplete `_
+* `ncm2 `_
+* Vim's ``omnifunc`` (used by default)
+
+For best experience, it is advised to use ``deoplete``. This is one of the most
+advanced completion manager which is very flexible and has numerous nice-to-have
+features such as snippets integration (if you are a
+`UltiSnips `_ user). You can install it via
+
+.. code-block:: vim
+
+  if has('nvim')
+Plug 'Shougo/deoplete.nvim', { 'do': ':UpdateRemotePlugins' }
+  else
+Plug 'Shougo/deoplete.nvim'
+Plug 'roxma/nvim-yarp'
+Plug 'roxma/vim-hug-neovim-rpc'
+  endif
+
+  let g:deoplete#enable_at_startup = 1
+
+Also, by default ``LanguageClient-neovim`` comes without key bindings. Typing
+``call LanguageClient#textDocument_rename()`` each time user 

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/tool/ClangdMain.cpp:202
+"Like changing an arrow(->) member access to dot(.) member access."),
+llvm::cl::init(clangd::CodeCompleteOptions().IncludeFixIts));
+

sammccall wrote:
> ilya-biryukov wrote:
> > Maybe specify the value here explicitly?
> > The defaults for users of the clangd binary (what the option is for) is not 
> > necessarily the best default for the `ClangdServer` clients (what the 
> > default in the struct is for).
> > More importantly, it's useful to be more straightforward about the defaults 
> > we have for the options.
> Not sure I agree here, this is consistent with the other options above. It's 
> the other `ClangdServer` clients that are the weirdos, they should have to be 
> explicit :-)
> 
> The defaults are clear when running with `-help`, which is how users will see 
> them.
> The defaults are clear when running with -help, which is how users will see 
> them.
Sure, but I would still argue reading the code is simpler without extra 
indirection and defaults of the binary are not necessarily the defaults we use 
in the APIs.

But also feel free to keep the code as is, don't think it's terribly important.



Comment at: clangd/tool/ClangdMain.cpp:205
+static llvm::cl::opt EnableFunctionArgSnippets(
+"enable-function-arg-snippets",
+llvm::cl::desc("Gives snippets for function arguments, when disabled only "

sammccall wrote:
> ilya-biryukov wrote:
> > I wonder if we can infer this setting from the `-completion-style' 
> > (`=bundled` implies `enable-function-arg-snippets == false`)
> > @sammccall, WDYT?
> They're not inextricably linked, the main connection is "these options both 
> need good signaturehelp support to be really useful".
> But we might want to link them just to cut down on number of options.
> 
> I don't have a strong opinion, I don't use `-completion-style=bundled`. Can 
> we find a few people who do and ask if they like this setting?
I would (obviously) want these two options to be packaged together whenever I 
use `=bundled`.
But I also use VSCode, which has signatureHelp.

I think @ioeric wanted to try out the `=bundled` completion style. @ioeric, 
WDYT?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51214



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

[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd

[Kirill Bobyrev via Phabricator via cfe-commits]

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

I need to

- Complete last two sections
- Make sure everything is rendered as expected
- Proof-read the text


https://reviews.llvm.org/D51297



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

[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Do we want to keep the docs for different editors separate or do we want to put 
them all into a single page in case we add more editors?
I would vouch for keeping them on a single page, but that's not a strong 
opinion. Wonder what other people think?


https://reviews.llvm.org/D51297



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

[PATCH] D51298: [Tooling] Allow to filter files used by AllTUsExecutor

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: ioeric.

This allows to run clang tools in parallel on a subset of files in the
repository.


Repository:
  rC Clang

https://reviews.llvm.org/D51298

Files:
  include/clang/Tooling/AllTUsExecution.h
  lib/Tooling/AllTUsExecution.cpp


Index: lib/Tooling/AllTUsExecution.cpp
===
--- lib/Tooling/AllTUsExecution.cpp
+++ lib/Tooling/AllTUsExecution.cpp
@@ -55,15 +55,18 @@
 
 AllTUsToolExecutor::AllTUsToolExecutor(
 const CompilationDatabase &Compilations, unsigned ThreadCount,
+std::vector Files,
 std::shared_ptr PCHContainerOps)
-: Compilations(Compilations), Results(new ThreadSafeToolResults),
-  Context(Results.get()), ThreadCount(ThreadCount) {}
+: Compilations(Compilations), Files(std::move(Files)),
+  Results(new ThreadSafeToolResults), Context(Results.get()),
+  ThreadCount(ThreadCount) {}
 
 AllTUsToolExecutor::AllTUsToolExecutor(
 CommonOptionsParser Options, unsigned ThreadCount,
 std::shared_ptr PCHContainerOps)
 : OptionsParser(std::move(Options)),
   Compilations(OptionsParser->getCompilations()),
+  Files(OptionsParser->getSourcePathList()),
   Results(new ThreadSafeToolResults), Context(Results.get()),
   ThreadCount(ThreadCount) {}
 
@@ -90,7 +93,8 @@
 llvm::errs() << Msg.str() << "\n";
   };
 
-  auto Files = Compilations.getAllFiles();
+  std::vector Files =
+  !this->Files.empty() ? this->Files : Compilations.getAllFiles();
   // Add a counter to track the progress.
   const std::string TotalNumStr = std::to_string(Files.size());
   unsigned Counter = 0;
Index: include/clang/Tooling/AllTUsExecution.h
===
--- include/clang/Tooling/AllTUsExecution.h
+++ include/clang/Tooling/AllTUsExecution.h
@@ -30,8 +30,11 @@
   /// Init with \p CompilationDatabase.
   /// This uses \p ThreadCount threads to exececute the actions on all files in
   /// parallel. If \p ThreadCount is 0, this uses `llvm::hardware_concurrency`.
+  /// Only processes files in \p Files. However, if \p Files is empty, will
+  /// process all the files in the compilation database.
   AllTUsToolExecutor(const CompilationDatabase &Compilations,
  unsigned ThreadCount,
+ std::vector Files = {},
  std::shared_ptr PCHContainerOps =
  std::make_shared());
 
@@ -66,6 +69,9 @@
   // Used to store the parser when the executor is initialized with parser.
   llvm::Optional OptionsParser;
   const CompilationDatabase &Compilations;
+  /// A list of files executor should run on. When empty, runs over all the
+  /// files in the compilation database.
+  std::vector Files;
   std::unique_ptr Results;
   ExecutionContext Context;
   llvm::StringMap OverlayFiles;


Index: lib/Tooling/AllTUsExecution.cpp
===
--- lib/Tooling/AllTUsExecution.cpp
+++ lib/Tooling/AllTUsExecution.cpp
@@ -55,15 +55,18 @@
 
 AllTUsToolExecutor::AllTUsToolExecutor(
 const CompilationDatabase &Compilations, unsigned ThreadCount,
+std::vector Files,
 std::shared_ptr PCHContainerOps)
-: Compilations(Compilations), Results(new ThreadSafeToolResults),
-  Context(Results.get()), ThreadCount(ThreadCount) {}
+: Compilations(Compilations), Files(std::move(Files)),
+  Results(new ThreadSafeToolResults), Context(Results.get()),
+  ThreadCount(ThreadCount) {}
 
 AllTUsToolExecutor::AllTUsToolExecutor(
 CommonOptionsParser Options, unsigned ThreadCount,
 std::shared_ptr PCHContainerOps)
 : OptionsParser(std::move(Options)),
   Compilations(OptionsParser->getCompilations()),
+  Files(OptionsParser->getSourcePathList()),
   Results(new ThreadSafeToolResults), Context(Results.get()),
   ThreadCount(ThreadCount) {}
 
@@ -90,7 +93,8 @@
 llvm::errs() << Msg.str() << "\n";
   };
 
-  auto Files = Compilations.getAllFiles();
+  std::vector Files =
+  !this->Files.empty() ? this->Files : Compilations.getAllFiles();
   // Add a counter to track the progress.
   const std::string TotalNumStr = std::to_string(Files.size());
   unsigned Counter = 0;
Index: include/clang/Tooling/AllTUsExecution.h
===
--- include/clang/Tooling/AllTUsExecution.h
+++ include/clang/Tooling/AllTUsExecution.h
@@ -30,8 +30,11 @@
   /// Init with \p CompilationDatabase.
   /// This uses \p ThreadCount threads to exececute the actions on all files in
   /// parallel. If \p ThreadCount is 0, this uses `llvm::hardware_concurrency`.
+  /// Only processes files in \p Files. However, if \p Files is empty, will
+  /// process all the files in the compilation database.
   AllTUsToolExecutor(const CompilationDatabase &Compilations,
  unsigned ThreadCount,
+ st

[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

In https://reviews.llvm.org/D51297#1214297, @ilya-biryukov wrote:

> Do we want to keep the docs for different editors separate or do we want to 
> put them all into a single page in case we add more editors?
>  I would vouch for keeping them on a single page, but that's not a strong 
> opinion. Wonder what other people think?


This one is pretty long and might scare some people off if we push this to the 
main documentation file IMO. However, creating a separate page for Vim 
integration might make it harder to discover the page itself, which is another 
problem. It seems like both options have their downsides, I'm not sure which 
one is better.


https://reviews.llvm.org/D51297



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

[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.

[Kristof Beyls via Phabricator via cfe-commits]

kristof.beyls accepted this revision.
kristof.beyls added a comment.

I'm not an expert on many of the areas touched by this patch, but it looks fine 
from me from a high-level point-of-view, modulo a few nits I have on a few 
comments.




Comment at: clang/lib/Driver/ToolChains/Arch/X86.cpp:151
+  // flags). This is a bit hacky but keeps existing usages working. We should
+  // consider deprecated this and instead warning if the user requests external
+  // retpoline thunks and *doesn't* request some form of retpolines.

s/deprecated/deprecating/
s/warning/warn/



Comment at: llvm/include/llvm/IR/Attributes.td:181-185
+/// Note that this uses the default compatibility (always compatible during
+/// inlining) and the default merge strategy of retaining the caller's
+/// attribute. This specifically matches the intent for this attribute which is
+/// that the context dominates, and inlined code will become hardened or lose
+/// its hardening based on the caller's attribute.

After updating the LangRef.rst text, I think this comment also needs to be 
updated. As is, it still documents the old inlining behaviour?
I'm also not sure in how far the comment makes most sense here. This is already 
documented in LangRef.rst, and AFAIK, the inlining compatibility mode is not 
something that is defined here?



Comment at: llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp:78-82
+static cl::opt EnableSpeculativeLoadHardening(
+"x86-speculative-load-hardening",
+cl::desc("Force enable speculative load hardening"), cl::init(false),
+cl::Hidden);
+

I'm not sure, but do you really still need an option to force enable SLH when 
you have function attributes now to enable it?
When you generate LLVM-IR using clang, you now have a clang option to enable 
that function attribute on all functions, so do you still have scenarios where 
you need an LLVM backend option to override the function attribute?



Comment at: llvm/lib/Target/X86/X86TargetMachine.cpp:474
 
-  if (EnableSpeculativeLoadHardening)
-addPass(createX86SpeculativeLoadHardeningPass());
+  // Will only run if force enabled or detects the relevant attribute.
+  addPass(createX86SpeculativeLoadHardeningPass());

I guess this is true for some other passes too, and they don't add such a 
comment here. Maybe best to remove this comment if my guess is right?


Repository:
  rL LLVM

https://reviews.llvm.org/D51157



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

[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D51297#1214297, @ilya-biryukov wrote:

> Do we want to keep the docs for different editors separate or do we want to 
> put them all into a single page in case we add more editors?
>  I would vouch for keeping them on a single page, but that's not a strong 
> opinion. Wonder what other people think?


Looking at the length of the vim integration doc, I think it might make sense 
to keep the full vim integration doc separate. However, it might still make 
sense to have short/simplified quick start instructions plus a link to the full 
doc for more details in the main doc.




Comment at: clang-tools-extra/docs/index.rst:29
clangd
+   clangd-vim
clang-doc

`clangd-vim` is not a top-level project. I think this should be linked from 
clangd documentation.



Comment at: clang/docs/ClangFormat.rst:97
 
+Clangd Integration
+==

We could recomend formatting with clangd in clangd doc, but advertising here 
seems a bit odd. I'd suggest dropping this.

And to be fair, clang-format editor plugins seem to be provide finer controls 
than LSP clients.


https://reviews.llvm.org/D51297



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

[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Very nice! Switching to clangd means that YCM is not necessary anymore?


https://reviews.llvm.org/D51297



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

[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang/docs/ClangFormat.rst:97
 
+Clangd Integration
+==

ioeric wrote:
> We could recomend formatting with clangd in clangd doc, but advertising here 
> seems a bit odd. I'd suggest dropping this.
> 
> And to be fair, clang-format editor plugins seem to be provide finer controls 
> than LSP clients.
I agree. clang-format plugins (e.g. [[ 
https://github.com/rhysd/vim-clang-format | vim-clang-format ]]) aren't 
advertised here, though. But I was going to have another patch on that, this 
might be useful for clang-format users; for example I discovered that plugin 
long after I started using clang-format.


https://reviews.llvm.org/D51297



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

[PATCH] D51295: Allow resetting of NumCreatedFIDsForFileID

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev updated this revision to Diff 162663.
v.g.vassilev added a comment.

Add parens.


https://reviews.llvm.org/D51295

Files:
  include/clang/Basic/SourceManager.h


Index: include/clang/Basic/SourceManager.h
===
--- include/clang/Basic/SourceManager.h
+++ include/clang/Basic/SourceManager.h
@@ -1024,13 +1024,14 @@
 
   /// Set the number of FileIDs (files and macros) that were created
   /// during preprocessing of \p FID, including it.
-  void setNumCreatedFIDsForFileID(FileID FID, unsigned NumFIDs) const {
+  void setNumCreatedFIDsForFileID(FileID FID, unsigned NumFIDs,
+  bool Force = false) const {
 bool Invalid = false;
 const SrcMgr::SLocEntry &Entry = getSLocEntry(FID, &Invalid);
 if (Invalid || !Entry.isFile())
   return;
 
-assert(Entry.getFile().NumCreatedFIDs == 0 && "Already set!");
+assert((Force || Entry.getFile().NumCreatedFIDs == 0) && "Already set!");
 const_cast(Entry.getFile()).NumCreatedFIDs = NumFIDs;
   }
 


Index: include/clang/Basic/SourceManager.h
===
--- include/clang/Basic/SourceManager.h
+++ include/clang/Basic/SourceManager.h
@@ -1024,13 +1024,14 @@
 
   /// Set the number of FileIDs (files and macros) that were created
   /// during preprocessing of \p FID, including it.
-  void setNumCreatedFIDsForFileID(FileID FID, unsigned NumFIDs) const {
+  void setNumCreatedFIDsForFileID(FileID FID, unsigned NumFIDs,
+  bool Force = false) const {
 bool Invalid = false;
 const SrcMgr::SLocEntry &Entry = getSLocEntry(FID, &Invalid);
 if (Invalid || !Entry.isFile())
   return;
 
-assert(Entry.getFile().NumCreatedFIDs == 0 && "Already set!");
+assert((Force || Entry.getFile().NumCreatedFIDs == 0) && "Already set!");
 const_cast(Entry.getFile()).NumCreatedFIDs = NumFIDs;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



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

This is a strange formulation where you have `anyOf(..., anything())`; can you 
explain why that's needed?



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

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

The instantiation note currently isn't being printed in the test case, but I 
suspect that will add a bit of extra clarity to the message.



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

Spurious newline change? It seems to cause a lot of churn in the test.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48714



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

[PATCH] D51299: [python bindings] Expose template argument API for Type

[Kyle Teske via Phabricator via cfe-commits]

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

Expose the C bindings for clang_Type_getNumTemplateArguments()
and clang_Type_getTemplateArgumentAsType() in the python API.


Repository:
  rC Clang

https://reviews.llvm.org/D51299

Files:
  bindings/python/clang/cindex.py
  bindings/python/tests/cindex/test_type.py


Index: bindings/python/tests/cindex/test_type.py
===
--- bindings/python/tests/cindex/test_type.py
+++ bindings/python/tests/cindex/test_type.py
@@ -436,3 +436,28 @@
 
 self.assertIsNotNone(testInteger, "Could not find testInteger.")
 self.assertEqual(testInteger.type.get_address_space(), 2)
+
+def test_template_arguments(self):
+source = """
+class Foo {
+};
+template 
+class Template {
+};
+Template instance;
+int bar;
+"""
+tu = get_tu(source, lang='cpp')
+
+# Varible with a template argument.
+cursor = get_cursor(tu, 'instance')
+cursor_type = cursor.type
+self.assertEqual(cursor.kind, CursorKind.VAR_DECL)
+self.assertEqual(cursor_type.spelling, 'Template')
+self.assertEqual(cursor_type.get_num_template_arguments(), 1)
+template_type = cursor_type.get_template_argument_type(0)
+self.assertEqual(template_type.spelling, 'Foo')
+
+# Variable without a template argument.
+cursor = get_cursor(tu, 'bar')
+self.assertEqual(cursor.get_num_template_arguments(), -1)
Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -2248,6 +2248,12 @@
 
 return res
 
+def get_num_template_arguments(self):
+return conf.lib.clang_Type_getNumTemplateArguments(self)
+
+def get_template_argument_type(self, num):
+return conf.lib.clang_Type_getTemplateArgumentAsType(self, num)
+
 def get_canonical(self):
 """
 Return the canonical type for a Type.
@@ -3992,6 +3998,15 @@
Type,
Type.from_result),
 
+  ("clang_Type_getNumTemplateArguments",
+   [Type],
+   c_int),
+
+  ("clang_Type_getTemplateArgumentAsType",
+   [Type, c_uint],
+   Type,
+   Type.from_result),
+
   ("clang_Type_getOffsetOf",
[Type, c_interop_string],
c_longlong),


Index: bindings/python/tests/cindex/test_type.py
===
--- bindings/python/tests/cindex/test_type.py
+++ bindings/python/tests/cindex/test_type.py
@@ -436,3 +436,28 @@
 
 self.assertIsNotNone(testInteger, "Could not find testInteger.")
 self.assertEqual(testInteger.type.get_address_space(), 2)
+
+def test_template_arguments(self):
+source = """
+class Foo {
+};
+template 
+class Template {
+};
+Template instance;
+int bar;
+"""
+tu = get_tu(source, lang='cpp')
+
+# Varible with a template argument.
+cursor = get_cursor(tu, 'instance')
+cursor_type = cursor.type
+self.assertEqual(cursor.kind, CursorKind.VAR_DECL)
+self.assertEqual(cursor_type.spelling, 'Template')
+self.assertEqual(cursor_type.get_num_template_arguments(), 1)
+template_type = cursor_type.get_template_argument_type(0)
+self.assertEqual(template_type.spelling, 'Foo')
+
+# Variable without a template argument.
+cursor = get_cursor(tu, 'bar')
+self.assertEqual(cursor.get_num_template_arguments(), -1)
Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -2248,6 +2248,12 @@
 
 return res
 
+def get_num_template_arguments(self):
+return conf.lib.clang_Type_getNumTemplateArguments(self)
+
+def get_template_argument_type(self, num):
+return conf.lib.clang_Type_getTemplateArgumentAsType(self, num)
+
 def get_canonical(self):
 """
 Return the canonical type for a Type.
@@ -3992,6 +3998,15 @@
Type,
Type.from_result),
 
+  ("clang_Type_getNumTemplateArguments",
+   [Type],
+   c_int),
+
+  ("clang_Type_getTemplateArgumentAsType",
+   [Type, c_uint],
+   Type,
+   Type.from_result),
+
   ("clang_Type_getOffsetOf",
[Type, c_interop_string],
c_longlong),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D51297#1214313, @kbobyrev wrote:

> In https://reviews.llvm.org/D51297#1214297, @ilya-biryukov wrote:
>
> > Do we want to keep the docs for different editors separate or do we want to 
> > put them all into a single page in case we add more editors?
> >  I would vouch for keeping them on a single page, but that's not a strong 
> > opinion. Wonder what other people think?
>
>
> This one is pretty long and might scare some people off if we push this to 
> the main documentation file IMO. However, creating a separate page for Vim 
> integration might make it harder to discover the page itself, which is 
> another problem. It seems like both options have their downsides, I'm not 
> sure which one is better.


Exactly the trade-off we're facing here. Not sure either. I'm personally not 
scared of big documents if there are links/TOCs to navigate to relevant parts. 
The "hard to find" bit looks like more of a problem to me.


https://reviews.llvm.org/D51297



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

[PATCH] D51177: [DEBUGINFO] Add support for emission of the debug directives only.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In https://reviews.llvm.org/D51177#1213079, @echristo wrote:

> Should we just have them mean the same thing and change it based on target?


I reused the same debug info level support in clang, but in LLVM they have 
different processing and I'm not sure that they are absolutely compatible.


Repository:
  rC Clang

https://reviews.llvm.org/D51177



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes added a comment.

In https://reviews.llvm.org/D34329#1213667, @sylvestre.ledru wrote:

> @arphaman @johannes Is that normal that clang-diff isn't installed by cmake? 
> (like clang-format?)


Yes, we did not add that. I don't know if anyone would use it.


Repository:
  rL LLVM

https://reviews.llvm.org/D34329



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

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

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

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

As rightly pointed out by @NoQ, `nonloc::LazyCompoundVal`s were only used to 
acquire a constructed object's region, which isn't what `LazyCompoundVal` was 
made for.


Repository:
  rC Clang

https://reviews.llvm.org/D51300

Files:
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp


Index: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
--- 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -124,12 +124,11 @@
 
 // Utility function declarations.
 
-/// Returns the object that was constructed by CtorDecl, or None if that isn't
-/// possible.
-// TODO: Refactor this function so that it returns the constructed object's
-// region.
-static Optional
-getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext &Context);
+/// Returns the region that was constructed by CtorDecl, or nullptr if that
+/// isn't possible.
+static const TypedValueRegion *
+getConstructedRegion(const CXXConstructorDecl *CtorDecl,
+ CheckerContext &Context);
 
 /// Checks whether the object constructed by \p Ctor will be analyzed later
 /// (e.g. if the object is a field of another object, in which case we'd check
@@ -159,12 +158,11 @@
   if (willObjectBeAnalyzedLater(CtorDecl, Context))
 return;
 
-  Optional Object = getObjectVal(CtorDecl, Context);
-  if (!Object)
+  const TypedValueRegion *R = getConstructedRegion(CtorDecl, Context);
+  if (!R)
 return;
 
-  FindUninitializedFields F(Context.getState(), Object->getRegion(),
-CheckPointeeInitialization);
+  FindUninitializedFields F(Context.getState(), R, CheckPointeeInitialization);
 
   const UninitFieldMap &UninitFields = F.getUninitFields();
 
@@ -443,25 +441,23 @@
 //   Utility functions.
 
//===--===//
 
-static Optional
-getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext &Context) {
+static const TypedValueRegion *
+getConstructedRegion(const CXXConstructorDecl *CtorDecl,
+ CheckerContext &Context) {
 
   Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl->getParent(),
 Context.getStackFrame());
-  // Getting the value for 'this'.
-  SVal This = Context.getState()->getSVal(ThisLoc);
 
-  // Getting the value for '*this'.
-  SVal Object = Context.getState()->getSVal(This.castAs());
+  SVal ObjectV = Context.getState()->getSVal(ThisLoc);
 
-  return Object.getAs();
+  return ObjectV.getAsRegion()->getAs();
 }
 
 static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor,
   CheckerContext &Context) {
 
-  Optional CurrentObject = getObjectVal(Ctor, 
Context);
-  if (!CurrentObject)
+  const TypedValueRegion *CurrRegion = getConstructedRegion(Ctor, Context);
+  if (!CurrRegion)
 return false;
 
   const LocationContext *LC = Context.getLocationContext();
@@ -472,14 +468,14 @@
 if (!OtherCtor)
   continue;
 
-Optional OtherObject =
-getObjectVal(OtherCtor, Context);
-if (!OtherObject)
+const TypedValueRegion *OtherRegion =
+getConstructedRegion(OtherCtor, Context);
+if (!OtherRegion)
   continue;
 
-// If the CurrentObject is a subregion of OtherObject, it will be analyzed
-// during the analysis of OtherObject.
-if (CurrentObject->getRegion()->isSubRegionOf(OtherObject->getRegion()))
+// If the CurrRegion is a subregion of OtherRegion, it will be analyzed
+// during the analysis of OtherRegion.
+if (CurrRegion->isSubRegionOf(OtherRegion))
   return true;
   }
 


Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -124,12 +124,11 @@
 
 // Utility function declarations.
 
-/// Returns the object that was constructed by CtorDecl, or None if that isn't
-/// possible.
-// TODO: Refactor this function so that it returns the constructed object's
-// region.
-static Optional
-getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext &Context);
+/// Returns the region that was constructed by CtorDecl, or nullptr if that
+/// isn't possible.
+static const TypedValueRegion *
+getConstructedRegion(const CXXConstructorDecl *CtorDecl,
+ CheckerContext &Context);
 
 /// Checks whether the object constructed by

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

[Alexey Sachkov via Phabricator via cfe-commits]

AlexeySachkov created this revision.
AlexeySachkov added reviewers: Anastasia, yaxunl.

Emit warning for multiple access qualifiers if they do not conflict.

Patch by Alexey Bader


Repository:
  rC Clang

https://reviews.llvm.org/D51302

Files:
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaType.cpp
  test/SemaOpenCL/access-qualifier.cl

Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7083,23 +7083,43 @@
   }
 
   if (const TypedefType* TypedefTy = CurType->getAs()) {
-QualType PointeeTy = TypedefTy->desugar();
-S.Diag(Attr.getLoc(), diag::err_opencl_multiple_access_qualifiers);
+QualType BaseTy = TypedefTy->desugar();
 
 std::string PrevAccessQual;
-switch (cast(PointeeTy.getTypePtr())->getKind()) {
-  #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
-case BuiltinType::Id:  \
-  PrevAccessQual = #Access;\
-  break;
-  #include "clang/Basic/OpenCLImageTypes.def"
-default:
-  assert(0 && "Unable to find corresponding image type.");
+if (BaseTy->isPipeType()) {
+  if (TypedefTy->getDecl()->hasAttr()) {
+OpenCLAccessAttr *Attr =
+TypedefTy->getDecl()->getAttr();
+PrevAccessQual = Attr->getSpelling();
+  } else {
+PrevAccessQual = "read_only";
+  }
+} else if (const BuiltinType* ImgType = BaseTy->getAs()) {
+
+  switch (ImgType->getKind()) {
+#define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
+  case BuiltinType::Id:  \
+PrevAccessQual = #Access;\
+break;
+#include "clang/Basic/OpenCLImageTypes.def"
+  default:
+llvm_unreachable("Unable to find corresponding image type.");
+  }
+} else {
+  llvm_unreachable("unexpected type");
+}
+StringRef AttrName = Attr.getName()->getName();
+if (PrevAccessQual == AttrName.ltrim("_")) {
+  // Duplicated qualifiers
+  S.Diag(Attr.getLoc(), diag::warn_duplicate_declspec)
+ << AttrName << Attr.getRange();
+} else {
+  // Contradicting qualifiers
+  S.Diag(Attr.getLoc(), diag::err_opencl_multiple_access_qualifiers);
 }
 
 S.Diag(TypedefTy->getDecl()->getBeginLoc(),
-   diag::note_opencl_typedef_access_qualifier)
-<< PrevAccessQual;
+   diag::note_opencl_typedef_access_qualifier) << PrevAccessQual;
   } else if (CurType->isPipeType()) {
 if (Attr.getSemanticSpelling() == OpenCLAccessAttr::Keyword_write_only) {
   QualType ElemType = CurType->getAs()->getElementType();
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5890,10 +5890,16 @@
 
   // Check if there is only one access qualifier.
   if (D->hasAttr()) {
-S.Diag(AL.getLoc(), diag::err_opencl_multiple_access_qualifiers)
-<< D->getSourceRange();
-D->setInvalidDecl(true);
-return;
+if (D->getAttr()->getSemanticSpelling() ==
+AL.getSemanticSpelling()) {
+  S.Diag(AL.getLoc(), diag::warn_duplicate_declspec)
+  << AL.getName()->getName() << AL.getRange();
+} else {
+  S.Diag(AL.getLoc(), diag::err_opencl_multiple_access_qualifiers)
+  << D->getSourceRange();
+  D->setInvalidDecl(true);
+  return;
+}
   }
 
   // OpenCL v2.0 s6.6 - read_write can be used for image types to specify that an
Index: test/SemaOpenCL/access-qualifier.cl
===
--- test/SemaOpenCL/access-qualifier.cl
+++ test/SemaOpenCL/access-qualifier.cl
@@ -60,7 +60,7 @@
 
 kernel void k11(read_only write_only image1d_t i){} // expected-error{{multiple access qualifiers}}
 
-kernel void k12(read_only read_only image1d_t i){} // expected-error{{multiple access qualifiers}}
+kernel void k12(read_only read_only image1d_t i){} // expected-warning {{duplicate 'read_only' declaration specifier}}
 
 #if __OPENCL_C_VERSION__ >= 200
 kernel void k13(read_write pipe int i){} // expected-error{{access qualifier 'read_write' can not be used for 'read_only pipe int'}}
@@ -78,3 +78,33 @@
 #if __OPENCL_C_VERSION__ < 200
 kernel void test_image3d_wo(write_only image3d_t img) {} // expected-error {{use of type '__write_only image3d_t' requires cl_khr_3d_image_writes extension to be enabled}}
 #endif
+
+#if __OPENCL_C_VERSION__ >= 200
+kernel void read_write_twice_typedef(read_write img1d_rw i){} // expected-warning {{duplicate 'read_write' declaration specifier}}
+// expected-note@-74 {{previously declared 'read_write' here}}
+
+kernel void pipe_ro_twice(read_only read_only pipe int i){} // expected-warning{{duplicate 'read_only' declaration specifier}}
+// Conflicting access qualifiers
+kernel void pipe_ro_twice_tw(read_

[PATCH] D49244: Always search sysroot for GCC installs

[David Greene via Phabricator via cfe-commits]

greened added a comment.

Ping.  It's been well over a month now.  What's the best way to get this 
reviewed?


Repository:
  rC Clang

https://reviews.llvm.org/D49244



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

r340743 - fix comment typo

[Nico Weber via cfe-commits]

Author: nico
Date: Mon Aug 27 07:23:50 2018
New Revision: 340743

URL: http://llvm.org/viewvc/llvm-project?rev=340743&view=rev
Log:
fix comment typo

Modified:

cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp?rev=340743&r1=340742&r2=340743&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
 (original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
 Mon Aug 27 07:23:50 2018
@@ -96,7 +96,7 @@ public:
 static bool isVoidPointer(QualType T);
 
 /// Dereferences \p V and returns the value and dynamic type of the pointee, as
-/// well as wether \p FR needs to be casted back to that type. If for whatever
+/// well as whether \p FR needs to be casted back to that type. If for whatever
 /// reason dereferencing fails, returns with None.
 static llvm::Optional>
 dereference(ProgramStateRef State, const FieldRegion *FR);


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

[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

In https://reviews.llvm.org/D51297#1214321, @JonasToth wrote:

> Very nice! Switching to clangd means that YCM is not necessary anymore?


Hi! Yeah, basically I wanted to follow up on the mailing list after I get some 
docs in, but you found this earlier :)

The good thing about Clangd (from my perspective) is that as long as the plugin 
has LSP integration, users are free to use whatever plugin they want (also, YCM 
decided  that LSP support is out 
of the scope of their project).

I am personally using LanguageClient-neovim almost all the time and I certainly 
had the best experience out of every other plugin/workflow I tried so far 
(although, as you can see, the setup is not trivial and there are actually few 
downsides of the LanguageClient-neovim). The other thing is that I'm using 
NeoVim and while I have tried this setup for the "plain" Vim I don't have too 
much user experience from that perspective yet, but most of my team members use 
Vim and we're currently looking into that.


https://reviews.llvm.org/D51297



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

[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Very nice :)

I will try to get it running for vim 8.0 on ubuntu 18.04 (hopefully this
week!) and share if i had success :)

Am 27.08.2018 um 16:57 schrieb Kirill Bobyrev via Phabricator:

> kbobyrev added a comment.
> 
> In https://reviews.llvm.org/D51297#1214321, @JonasToth wrote:
> 
>> Very nice! Switching to clangd means that YCM is not necessary anymore?
> 
> Hi! Yeah, basically I wanted to follow up on the mailing list after I get 
> some docs in, but you found this earlier :)
> 
> The good thing about Clangd (from my perspective) is that as long as the 
> plugin has LSP integration, users are free to use whatever plugin they want 
> (also, YCM decided https://github.com/Valloric/ycmd/issues/515 that LSP 
> support is out of the scope of their project).
> 
> I am personally using LanguageClient-neovim almost all the time and I 
> certainly had the best experience out of every other plugin/workflow I tried 
> so far (although, as you can see, the setup is not trivial and there are 
> actually few downsides of the LanguageClient-neovim). The other thing is that 
> I'm using NeoVim and while I have tried this setup for the "plain" Vim I 
> don't have too much user experience from that perspective yet, but most of my 
> team members use Vim and we're currently looking into that.
> 
> https://reviews.llvm.org/D51297


https://reviews.llvm.org/D51297



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

[PATCH] D43184: [WIP] [ItaniunCXXABI] Add an option for loading the type info vtable with dllimport

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo abandoned this revision.
mstorsjo added a comment.

This isn't needed now any longer, when https://reviews.llvm.org/D50917 has 
landed.


Repository:
  rC Clang

https://reviews.llvm.org/D43184



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

[PATCH] D51310: [clangd] Implement iterator cost

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov, sammccall.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, mgrang, jkorous, MaskRay.

This patch introduces iterator cost concept to improve the performance of Dex 
query iterators (mainly, AND iterator). Benchmarks show that the queries become 
~10% faster.

Before

  ---
  BenchmarkTime   CPU Iteration
  ---
  DexAdHocQueries5883074 ns5883018 ns117
  DexRealQ 959904457 ns  959898507 ns  1

After

  ---
  BenchmarkTime   CPU Iteration
  ---
  DexAdHocQueries5238403 ns5238361 ns130
  DexRealQ 873275207 ns  873269453 ns  1


https://reviews.llvm.org/D51310

Files:
  clang-tools-extra/clangd/index/dex/Iterator.cpp
  clang-tools-extra/clangd/index/dex/Iterator.h

Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -95,6 +95,8 @@
   /// consume() must *not* be called on children that don't contain the current
   /// doc.
   virtual float consume() = 0;
+  /// Returns an estimate of advance() calls before the iterator is exhausted.
+  virtual size_t cost() = 0;
 
   virtual ~Iterator() {}
 
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -48,6 +48,8 @@
 
   float consume() override { return DEFAULT_BOOST_SCORE; }
 
+  size_t cost() override { return Documents.size(); }
+
 private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << '[';
@@ -85,6 +87,11 @@
 assert(!Children.empty() && "AndIterator should have at least one child.");
 // Establish invariants.
 sync();
+std::sort(begin(Children), end(Children),
+  [](const std::unique_ptr &LHS,
+ const std::unique_ptr &RHS) {
+return LHS->cost() < RHS->cost();
+  });
   }
 
   bool reachedEnd() const override { return ReachedEnd; }
@@ -114,6 +121,14 @@
 });
   }
 
+  size_t cost() override {
+return std::accumulate(
+begin(Children), end(Children), Children.front()->cost(),
+[&](size_t Current, const std::unique_ptr &Child) {
+  return std::min(Current, Child->cost());
+});
+  }
+
 private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << "(& ";
@@ -178,6 +193,11 @@
   OrIterator(std::vector> AllChildren)
   : Children(std::move(AllChildren)) {
 assert(Children.size() > 0 && "Or Iterator must have at least one child.");
+std::sort(begin(Children), end(Children),
+  [](const std::unique_ptr &LHS,
+ const std::unique_ptr &RHS) {
+return LHS->cost() < RHS->cost();
+  });
   }
 
   /// Returns true if all children are exhausted.
@@ -235,6 +255,14 @@
 });
   }
 
+  size_t cost() override {
+return std::accumulate(
+begin(Children), end(Children), Children.front()->cost(),
+[&](size_t Current, const std::unique_ptr &Child) {
+  return std::max(Current, Child->cost());
+});
+  }
+
 private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << "(| ";
@@ -277,6 +305,8 @@
 
   float consume() override { return DEFAULT_BOOST_SCORE; }
 
+  size_t cost() override { return Size; }
+
 private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << "(TRUE {" << Index << "} out of " << Size << ")";
@@ -305,6 +335,8 @@
 
   float consume() override { return Child->consume() * Factor; }
 
+  size_t cost() override { return Child->cost(); }
+
 private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << "(BOOST " << Factor << ' ' << *Child << ')';
@@ -342,6 +374,8 @@
 return Child->consume();
   }
 
+  size_t cost() override { return Limit; }
+
 private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << "(LIMIT " << Limit << '(' << ItemsLeft << ") " << *Child << ')';
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51310: [clangd] Implement iterator cost

[Kirill Bobyrev via Phabricator via cfe-commits]

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

It's probably better to roll out proximity path boosting & actual two-stage 
filtering before rolling this out.


https://reviews.llvm.org/D51310



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

[clang-tools-extra] r340749 - [docs] Mention clangd-dev in clangd documentation

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Mon Aug 27 08:38:49 2018
New Revision: 340749

URL: http://llvm.org/viewvc/llvm-project?rev=340749&view=rev
Log:
[docs] Mention clangd-dev in clangd documentation

Since the clangd-dev is intended to be the place for clangd-related
discussions, we should point new users to this mailing list while
probably mentioning cfe-dev, too.

Reviewed by: ioeric

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

Modified:
clang-tools-extra/trunk/docs/clangd.rst

Modified: clang-tools-extra/trunk/docs/clangd.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clangd.rst?rev=340749&r1=340748&r2=340749&view=diff
==
--- clang-tools-extra/trunk/docs/clangd.rst (original)
+++ clang-tools-extra/trunk/docs/clangd.rst Mon Aug 27 08:38:49 2018
@@ -111,7 +111,10 @@ extension to the protocol.
 Getting Involved
 ==
 
-A good place for interested contributors is the `Clang developer mailing list
+A good place for interested contributors is the `Clangd developer mailing list
+`_. For discussions with the
+broader community on topics not only related to Clangd, use
+`Clang developer mailing list
 `_.
 If you're also interested in contributing patches to :program:`Clangd`, take a
 look at the `LLVM Developer Policy


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

[PATCH] D51293: [docs] Mention clangd-dev in clangd documentation

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340749: [docs] Mention clangd-dev in clangd documentation 
(authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51293?vs=162645&id=162688#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51293

Files:
  clang-tools-extra/trunk/docs/clangd.rst


Index: clang-tools-extra/trunk/docs/clangd.rst
===
--- clang-tools-extra/trunk/docs/clangd.rst
+++ clang-tools-extra/trunk/docs/clangd.rst
@@ -111,7 +111,10 @@
 Getting Involved
 ==
 
-A good place for interested contributors is the `Clang developer mailing list
+A good place for interested contributors is the `Clangd developer mailing list
+`_. For discussions with the
+broader community on topics not only related to Clangd, use
+`Clang developer mailing list
 `_.
 If you're also interested in contributing patches to :program:`Clangd`, take a
 look at the `LLVM Developer Policy


Index: clang-tools-extra/trunk/docs/clangd.rst
===
--- clang-tools-extra/trunk/docs/clangd.rst
+++ clang-tools-extra/trunk/docs/clangd.rst
@@ -111,7 +111,10 @@
 Getting Involved
 ==
 
-A good place for interested contributors is the `Clang developer mailing list
+A good place for interested contributors is the `Clangd developer mailing list
+`_. For discussions with the
+broader community on topics not only related to Clangd, use
+`Clang developer mailing list
 `_.
 If you're also interested in contributing patches to :program:`Clangd`, take a
 look at the `LLVM Developer Policy
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51311: (WIP) Type hierarchy

[Simon Marchi via Phabricator via cfe-commits]

simark created this revision.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, ioeric, 
ilya-biryukov.

This is a work in progress patch to support an eventual "get type
hierarchy" request that does not exist in the LSP today.  I only plan to
support getting parent types (i.e. base classes) for now, because it
doesn't require touching the index.  Finding child classes would come
later.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51311

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  clangd/XRefs.cpp
  clangd/XRefs.h
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -27,6 +27,7 @@
 
 namespace {
 using testing::ElementsAre;
+using testing::Eq;
 using testing::Field;
 using testing::IsEmpty;
 using testing::Matcher;
@@ -1068,6 +1069,166 @@
   ElementsAre(Location{FooCppUri, FooWithoutHeader.range()}));
 }
 
+TEST(TypeHierarchy, SimpleInheritanceOnTypeOrVariable) {
+  Annotations Source(R"cpp(
+$ParentDef^struct Parent
+{
+  int a;
+};
+
+$Child1Def^struct Child1 : Parent
+{
+  int b;
+};
+
+struct Ch$p1^ild2 : Child1
+{
+  int c;
+};
+
+struct Child3 : Child2
+{
+  int d;
+};
+
+int main()
+{
+  Ch$p2^ild2 ch$p3^ild2;
+
+  parent.a = 1;
+  ch$p4^ild2.c = 1;
+}
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+
+  llvm::Optional Result;
+
+  TypeHierarchy ExpectedResult{{TypeHierarchyResult{
+  "Child1",
+  Source.point("Child1Def"),
+  false,
+  {TypeHierarchyResult{"Parent", Source.point("ParentDef"), false, {}};
+
+  for (auto Pt : {"p1", "p2", "p3", "p4"}) {
+Result = getTypeHierarchy(AST, Source.point(Pt));
+ASSERT_TRUE(bool(Result));
+EXPECT_THAT(*Result, Eq(ExpectedResult));
+  }
+}
+
+TEST(TypeHierarchy, MultipleInheritanceOnTypeOrVariable) {
+  Annotations Source(R"cpp(
+$Parent1Def^struct Parent1
+{
+  int a;
+};
+
+$Parent2Def^struct Parent2
+{
+  int b;
+};
+
+$Parent3Def^struct Parent3 : Parent2
+{
+  int c;
+};
+
+struct Ch$c1^ild : Parent1, Parent3
+{
+  int d;
+};
+
+int main()
+{
+  Ch$c2^ild  ch$c3^ild;
+
+  ch$c4^ild.a = 1;
+}
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+
+  llvm::Optional Result;
+  TypeHierarchy ExpectedResult{{
+  TypeHierarchyResult{"Parent1", Source.point("Parent1Def"), false, {}},
+  TypeHierarchyResult{
+  "Parent3",
+  Source.point("Parent3Def"),
+  false,
+  {TypeHierarchyResult{
+  "Parent2", Source.point("Parent2Def"), false, {,
+  }};
+
+  for (auto Pt : {"c1", "c2", "c3", "c4"}) {
+Result = getTypeHierarchy(AST, Source.point(Pt));
+ASSERT_TRUE(bool(Result));
+EXPECT_THAT(*Result, Eq(ExpectedResult));
+  }
+}
+
+TEST(TypeHierarchy, OnMethod) {
+  Annotations Source(R"cpp(
+$ParentDef^struct Parent
+{
+  void method ();
+  void method () const;
+  void method (int x);
+  void method (char x);
+};
+
+$Child1Def^struct Child1 : Parent
+{
+  void method ();
+  void method (char x);
+};
+
+struct Child2 : Child1
+{
+  void met$p1^hod ();
+  void met$p2^hod (int x);
+};
+
+struct Child3 : Child2
+{
+  void method (int x);
+};
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  {
+TypeHierarchy ExpectedResult{{TypeHierarchyResult{
+"Child1",
+Source.point("Child1Def"),
+true,
+{TypeHierarchyResult{"Parent", Source.point("ParentDef"), true, {}};
+
+llvm::Optional Result =
+getTypeHierarchy(AST, Source.point("p1"));
+ASSERT_TRUE(bool(Result));
+EXPECT_THAT(*Result, Eq(ExpectedResult));
+  }
+
+  {
+TypeHierarchy ExpectedResult{{TypeHierarchyResult{
+"Child1",
+Source.point("Child1Def"),
+false,
+{TypeHierarchyResult{"Parent", Source.point("ParentDef"), true, {}};
+
+llvm::Optional Result =
+getTypeHierarchy(AST, Source.point("p2"));
+ASSERT_TRUE(bool(Result));
+EXPECT_THAT(*Result, Eq(ExpectedResult));
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/XRefs.h
===
--- clangd/XRefs.h
+++ clangd/XRefs.h
@@ -34,6 +34,9 @@
 /// Get the hover information when hovering at \p Pos.
 llvm::Optional getHover(ParsedAST &AST, Position Pos);
 
+/// Get the type hierarchy information at \p Pos.
+llvm::Optional getTypeHierarchy(ParsedAST &AST, Position Pos);
+
 } // namespace clangd
 } // namespace clang
 
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -

[PATCH] D51292: [docs] Update clang-rename documentation

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/docs/clang-rename.rst:28
+:program:`clang-rename` infrastructure to handle renaming requests. Because of
+much better editor integration and support, it is advised to use
+:program:`clangd-rename` as part of :program:`clangd`. However, it is possible

ilya-biryukov wrote:
> I would not recommend `clangd` at this point, it only supports single-file 
> renames, while `clang-rename` can actually do cross-TU renames IIUC.
> Not opposed to putting recommendations of using `clangd`, of course, but 
> let's be explicit about the limitations.
Interesting, I thought we do the same logic in Clangd. Yeah, I think we should 
fix that behavior.


https://reviews.llvm.org/D51292



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

[libcxx] r340752 - Mark P0556 as 'in progress'

[Marshall Clow via cfe-commits]

Author: marshall
Date: Mon Aug 27 09:07:01 2018
New Revision: 340752

URL: http://llvm.org/viewvc/llvm-project?rev=340752&view=rev
Log:
Mark P0556 as 'in progress'


Modified:
libcxx/trunk/www/cxx2a_status.html

Modified: libcxx/trunk/www/cxx2a_status.html
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/cxx2a_status.html?rev=340752&r1=340751&r2=340752&view=diff
==
--- libcxx/trunk/www/cxx2a_status.html (original)
+++ libcxx/trunk/www/cxx2a_status.html Mon Aug 27 09:07:01 2018
@@ -89,7 +89,7 @@
https://wg21.link/P0476R2";>P0476R2LWGBit-casting 
object representationsRapperswil
https://wg21.link/P0528R3";>P0528R3CWGThe Curious 
Case of Padding Bits, Featuring Atomic 
Compare-and-ExchangeRapperswil
https://wg21.link/P0542R5";>P0542R5CWGSupport for 
contract based programming in C++Rapperswil
-   https://wg21.link/P0556R3";>P0556R3LWGIntegral 
power-of-2 operationsRapperswil
+   https://wg21.link/P0556R3";>P0556R3LWGIntegral 
power-of-2 operationsRapperswilIn 
Progress
https://wg21.link/P0619R4";>P0619R4LWGReviewing 
Deprecated Facilities of C++17 for 
C++20Rapperswil
https://wg21.link/P0646R1";>P0646R1LWGImproving the 
Return Value of Erase-Like 
AlgorithmsRapperswil
https://wg21.link/P0722R3";>P0722R3CWGEfficient 
sized delete for variable sized 
classesRapperswil
@@ -222,7 +222,7 @@
 
   
 
-  Last Updated: 1-Aug-2018
+  Last Updated: 27-Aug-2018
 
 
 


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

[PATCH] D51312: [OpenMP][NVPTX] Use appropriate _CALL_ELF macro when offloading

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

gtbercea created this revision.
gtbercea added reviewers: Hahnfeld, ABataev, caomhin.
Herald added subscribers: cfe-commits, guansong.

When offloading to a device and using the powerpc64le version of the auxiliary 
triple, the _CALL_ELF macro is not set correctly to 2 resulting in the attempt 
to include a header that does not exist. This patch fixes this problem.


Repository:
  rC Clang

https://reviews.llvm.org/D51312

Files:
  lib/Frontend/InitPreprocessor.cpp


Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -1112,8 +1112,12 @@
 Builder.defineMacro("__x86_64__");
 break;
   case llvm::Triple::ppc64:
+Builder.defineMacro("__powerpc64__");
+Builder.defineMacro("_CALL_ELF", "1");
+break;
   case llvm::Triple::ppc64le:
 Builder.defineMacro("__powerpc64__");
+Builder.defineMacro("_CALL_ELF", "2");
 break;
   default:
 break;


Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -1112,8 +1112,12 @@
 Builder.defineMacro("__x86_64__");
 break;
   case llvm::Triple::ppc64:
+Builder.defineMacro("__powerpc64__");
+Builder.defineMacro("_CALL_ELF", "1");
+break;
   case llvm::Triple::ppc64le:
 Builder.defineMacro("__powerpc64__");
+Builder.defineMacro("_CALL_ELF", "2");
 break;
   default:
 break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51312: [OpenMP][NVPTX] Use appropriate _CALL_ELF macro when offloading

[Jonas Hahnfeld via Phabricator via cfe-commits]

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

LGTM. Can you add a comment to `InitializePredefinedAuxMacros` explaining that 
the macro is used in `gnu/stubs.h` and add a check to the test?


Repository:
  rC Clang

https://reviews.llvm.org/D51312



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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added a subscriber: jfb.

This greatly reduces the time to read 'compile_commands.json'.
For Chromium on my machine it's now 5 secs vs 30 secs before the
change.


Repository:
  rC Clang

https://reviews.llvm.org/D51314

Files:
  lib/Tooling/InterpolatingCompilationDatabase.cpp

Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -57,6 +57,7 @@
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 
 namespace clang {
 namespace tooling {
@@ -121,85 +122,57 @@
 
 // A CompileCommand that can be applied to another file.
 struct TransferableCommand {
-  // Flags that should not apply to all files are stripped from CommandLine.
-  CompileCommand Cmd;
-  // Language detected from -x or the filename.
-  types::ID Type = types::TY_INVALID;
-  // Standard specified by -std.
-  LangStandard::Kind Std = LangStandard::lang_unspecified;
-
   TransferableCommand(CompileCommand C)
-  : Cmd(std::move(C)), Type(guessType(Cmd.Filename)) {
-std::vector NewArgs = {Cmd.CommandLine.front()};
-// Parse the old args in order to strip out and record unwanted flags.
-auto OptTable = clang::driver::createDriverOptTable();
-std::vector Argv;
-for (unsigned I = 1; I < Cmd.CommandLine.size(); ++I)
-  Argv.push_back(Cmd.CommandLine[I].c_str());
-unsigned MissingI, MissingC;
-auto ArgList = OptTable->ParseArgs(Argv, MissingI, MissingC);
-for (const auto *Arg : ArgList) {
-  const auto &option = Arg->getOption();
-  // Strip input and output files.
-  if (option.matches(clang::driver::options::OPT_INPUT) ||
-  option.matches(clang::driver::options::OPT_o)) {
-continue;
-  }
-  // Strip -x, but record the overridden language.
-  if (option.matches(clang::driver::options::OPT_x)) {
-for (const char *Value : Arg->getValues())
-  Type = types::lookupTypeForTypeSpecifier(Value);
-continue;
-  }
-  // Strip --std, but record the value.
-  if (option.matches(clang::driver::options::OPT_std_EQ)) {
-for (const char *Value : Arg->getValues()) {
-  Std = llvm::StringSwitch(Value)
-#define LANGSTANDARD(id, name, lang, desc, features)   \
-  .Case(name, LangStandard::lang_##id)
-#define LANGSTANDARD_ALIAS(id, alias) .Case(alias, LangStandard::lang_##id)
-#include "clang/Frontend/LangStandards.def"
-.Default(Std);
-}
-continue;
-  }
-  llvm::opt::ArgStringList ArgStrs;
-  Arg->render(ArgList, ArgStrs);
-  NewArgs.insert(NewArgs.end(), ArgStrs.begin(), ArgStrs.end());
-}
-Cmd.CommandLine = std::move(NewArgs);
-
-if (Std != LangStandard::lang_unspecified) // -std take precedence over -x
-  Type = toType(LangStandard::getLangStandardForKind(Std).getLanguage());
-Type = foldType(Type);
-  }
+  : OriginalCmd(std::move(C)),
+Traits(nullptr) {}
 
   // Produce a CompileCommand for \p filename, based on this one.
   CompileCommand transferTo(StringRef Filename) const {
-CompileCommand Result = Cmd;
+const CommandTraits& T = getTraits();
+CompileCommand Result = T.Cmd;
 Result.Filename = Filename;
 bool TypeCertain;
 auto TargetType = guessType(Filename, &TypeCertain);
 // If the filename doesn't determine the language (.h), transfer with -x.
 if (!TypeCertain) {
   TargetType = types::onlyPrecompileType(TargetType) // header?
-   ? types::lookupHeaderTypeForSourceType(Type)
-   : Type;
+   ? types::lookupHeaderTypeForSourceType(T.Type)
+   : T.Type;
   Result.CommandLine.push_back("-x");
   Result.CommandLine.push_back(types::getTypeName(TargetType));
 }
 // --std flag may only be transferred if the language is the same.
 // We may consider "translating" these, e.g. c++11 -> c11.
-if (Std != LangStandard::lang_unspecified && foldType(TargetType) == Type) {
+if (T.Std != LangStandard::lang_unspecified &&
+foldType(TargetType) == T.Type) {
   Result.CommandLine.push_back(
   "-std=" +
-  std::string(LangStandard::getLangStandardForKind(Std).getName()));
+  std::string(LangStandard::getLangStandardForKind(T.Std).getName()));
 }
 Result.CommandLine.push_back(Filename);
 return Result;
   }
 
+  llvm::StringRef filename() const {
+return OriginalCmd.Filename;
+  }
+
+  types::ID type() const {
+return getTraits().Type;
+  }
+
 private:
+  // Extra information that cannot be inferred solely by the filename. Slow to
+  // compute, so we only compute on first access.
+  struct CommandTraits {
+// Flags that should not appl

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[JF Bastien via Phabricator via cfe-commits]

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



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:201
+
+  CommandTraits computeTraits() const {
+CommandTraits Result;

It's not clear to me that this entire function is safe to call from multiple 
threads at the same time. Even if it's safe now, I'm willing to bet it won't 
always be that way. `getTraits` should therefore use a magic static or 
`call_once` and avoid the headache entirely.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:258
+  /// Null before Traits were computed, non-null otherwise. All accesses to 
this
+  /// must be atomic.
+  mutable std::shared_ptr Traits;

The comment should say why accesses need to be atomic. Or better yet, this 
should only be usable "the right way".


Repository:
  rC Clang

https://reviews.llvm.org/D51314



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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:440
   Result.emplace_back(std::move(Command));
-  if (Result.back().Type == types::TY_INVALID)
-Result.pop_back();

We can't look at 'Type' at this point anymore, because it needs parsing of 
TranserableCommands. Not sure what's the best way to replace it. @sammccall, 
any ideas?



Repository:
  rC Clang

https://reviews.llvm.org/D51314



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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Add a comment
- Use std::call_once to compute the lazy value


Repository:
  rC Clang

https://reviews.llvm.org/D51314

Files:
  lib/Tooling/InterpolatingCompilationDatabase.cpp

Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -57,6 +57,8 @@
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
+#include 
 
 namespace clang {
 namespace tooling {
@@ -119,87 +121,64 @@
   }
 }
 
-// A CompileCommand that can be applied to another file.
+// A CompileCommand that can be applied to another file. Any instance of this
+// object is invalid after std::move() from it.
 struct TransferableCommand {
-  // Flags that should not apply to all files are stripped from CommandLine.
-  CompileCommand Cmd;
-  // Language detected from -x or the filename.
-  types::ID Type = types::TY_INVALID;
-  // Standard specified by -std.
-  LangStandard::Kind Std = LangStandard::lang_unspecified;
-
   TransferableCommand(CompileCommand C)
-  : Cmd(std::move(C)), Type(guessType(Cmd.Filename)) {
-std::vector NewArgs = {Cmd.CommandLine.front()};
-// Parse the old args in order to strip out and record unwanted flags.
-auto OptTable = clang::driver::createDriverOptTable();
-std::vector Argv;
-for (unsigned I = 1; I < Cmd.CommandLine.size(); ++I)
-  Argv.push_back(Cmd.CommandLine[I].c_str());
-unsigned MissingI, MissingC;
-auto ArgList = OptTable->ParseArgs(Argv, MissingI, MissingC);
-for (const auto *Arg : ArgList) {
-  const auto &option = Arg->getOption();
-  // Strip input and output files.
-  if (option.matches(clang::driver::options::OPT_INPUT) ||
-  option.matches(clang::driver::options::OPT_o)) {
-continue;
-  }
-  // Strip -x, but record the overridden language.
-  if (option.matches(clang::driver::options::OPT_x)) {
-for (const char *Value : Arg->getValues())
-  Type = types::lookupTypeForTypeSpecifier(Value);
-continue;
-  }
-  // Strip --std, but record the value.
-  if (option.matches(clang::driver::options::OPT_std_EQ)) {
-for (const char *Value : Arg->getValues()) {
-  Std = llvm::StringSwitch(Value)
-#define LANGSTANDARD(id, name, lang, desc, features)   \
-  .Case(name, LangStandard::lang_##id)
-#define LANGSTANDARD_ALIAS(id, alias) .Case(alias, LangStandard::lang_##id)
-#include "clang/Frontend/LangStandards.def"
-.Default(Std);
-}
-continue;
-  }
-  llvm::opt::ArgStringList ArgStrs;
-  Arg->render(ArgList, ArgStrs);
-  NewArgs.insert(NewArgs.end(), ArgStrs.begin(), ArgStrs.end());
-}
-Cmd.CommandLine = std::move(NewArgs);
-
-if (Std != LangStandard::lang_unspecified) // -std take precedence over -x
-  Type = toType(LangStandard::getLangStandardForKind(Std).getLanguage());
-Type = foldType(Type);
-  }
+  : OriginalCmd(std::move(C)),
+TraitsComputed(llvm::make_unique()),
+Traits(llvm::None) {}
 
   // Produce a CompileCommand for \p filename, based on this one.
   CompileCommand transferTo(StringRef Filename) const {
-CompileCommand Result = Cmd;
+assert(TraitsComputed && "calling transferTo on moved-from object");
+const CommandTraits& T = getTraits();
+CompileCommand Result = T.Cmd;
 Result.Filename = Filename;
 bool TypeCertain;
 auto TargetType = guessType(Filename, &TypeCertain);
 // If the filename doesn't determine the language (.h), transfer with -x.
 if (!TypeCertain) {
   TargetType = types::onlyPrecompileType(TargetType) // header?
-   ? types::lookupHeaderTypeForSourceType(Type)
-   : Type;
+   ? types::lookupHeaderTypeForSourceType(T.Type)
+   : T.Type;
   Result.CommandLine.push_back("-x");
   Result.CommandLine.push_back(types::getTypeName(TargetType));
 }
 // --std flag may only be transferred if the language is the same.
 // We may consider "translating" these, e.g. c++11 -> c11.
-if (Std != LangStandard::lang_unspecified && foldType(TargetType) == Type) {
+if (T.Std != LangStandard::lang_unspecified &&
+foldType(TargetType) == T.Type) {
   Result.CommandLine.push_back(
   "-std=" +
-  std::string(LangStandard::getLangStandardForKind(Std).getName()));
+  std::string(LangStandard::getLangStandardForKind(T.Std).getName()));
 }
 Result.CommandLine.push_back(Filename);
 return Result;
   }
 
+  llvm::StringRef filename() const {
+assert(TraitsComputed && "calling filename on moved-from object");
+return OriginalCmd.Filename;
+  }
+
+  types::ID type() const 

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

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2254
   Res |= SanitizerKind::Function;
+  if (!isTargetMacOS() || !isMacosxVersionLT(10, 9))
+Res |= SanitizerKind::Vptr;

delcypher wrote:
> Could we apply De'Morgan's rule here and write that as
> 
> ```
> if (!(isTargetMacOS() && isMacosxVersionLT(10, 9)) {
>   Res |= SanitizerKind::Vptr
> }
> ```
> 
> I find that a bit easier to read.
> 
> Is there any particular reason why vptr isn't supported for old macOS 
> versions? There's no mention of ios here which suggests that it's supported 
> on all ios versions which seems like an odd disparity. Perhaps a comment 
> briefly explaining why this is the case would be helpful?
Sure.

MacOS versions older than 10.8 shipped a version of the C++ standard library 
which is incompatible with the vptr check's implementation (see: 
https://trac.macports.org/wiki/LibcxxOnOlderSystems). I'll add a comment to 
that effect.

As far as I know, all currently-supported versions of iOS ship libc++. I'll ask 
around and double-check, just to be safe.


https://reviews.llvm.org/D51239



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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Remove computeTraits, put the body inside a lambda


Repository:
  rC Clang

https://reviews.llvm.org/D51314

Files:
  lib/Tooling/InterpolatingCompilationDatabase.cpp

Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -56,7 +56,9 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
+#include 
 
 namespace clang {
 namespace tooling {
@@ -119,87 +121,63 @@
   }
 }
 
-// A CompileCommand that can be applied to another file.
+// A CompileCommand that can be applied to another file. Any instance of this
+// object is invalid after std::move() from it.
 struct TransferableCommand {
-  // Flags that should not apply to all files are stripped from CommandLine.
-  CompileCommand Cmd;
-  // Language detected from -x or the filename.
-  types::ID Type = types::TY_INVALID;
-  // Standard specified by -std.
-  LangStandard::Kind Std = LangStandard::lang_unspecified;
-
   TransferableCommand(CompileCommand C)
-  : Cmd(std::move(C)), Type(guessType(Cmd.Filename)) {
-std::vector NewArgs = {Cmd.CommandLine.front()};
-// Parse the old args in order to strip out and record unwanted flags.
-auto OptTable = clang::driver::createDriverOptTable();
-std::vector Argv;
-for (unsigned I = 1; I < Cmd.CommandLine.size(); ++I)
-  Argv.push_back(Cmd.CommandLine[I].c_str());
-unsigned MissingI, MissingC;
-auto ArgList = OptTable->ParseArgs(Argv, MissingI, MissingC);
-for (const auto *Arg : ArgList) {
-  const auto &option = Arg->getOption();
-  // Strip input and output files.
-  if (option.matches(clang::driver::options::OPT_INPUT) ||
-  option.matches(clang::driver::options::OPT_o)) {
-continue;
-  }
-  // Strip -x, but record the overridden language.
-  if (option.matches(clang::driver::options::OPT_x)) {
-for (const char *Value : Arg->getValues())
-  Type = types::lookupTypeForTypeSpecifier(Value);
-continue;
-  }
-  // Strip --std, but record the value.
-  if (option.matches(clang::driver::options::OPT_std_EQ)) {
-for (const char *Value : Arg->getValues()) {
-  Std = llvm::StringSwitch(Value)
-#define LANGSTANDARD(id, name, lang, desc, features)   \
-  .Case(name, LangStandard::lang_##id)
-#define LANGSTANDARD_ALIAS(id, alias) .Case(alias, LangStandard::lang_##id)
-#include "clang/Frontend/LangStandards.def"
-.Default(Std);
-}
-continue;
-  }
-  llvm::opt::ArgStringList ArgStrs;
-  Arg->render(ArgList, ArgStrs);
-  NewArgs.insert(NewArgs.end(), ArgStrs.begin(), ArgStrs.end());
-}
-Cmd.CommandLine = std::move(NewArgs);
-
-if (Std != LangStandard::lang_unspecified) // -std take precedence over -x
-  Type = toType(LangStandard::getLangStandardForKind(Std).getLanguage());
-Type = foldType(Type);
-  }
+  : OriginalCmd(std::move(C)),
+TraitsComputed(llvm::make_unique()) {}
 
   // Produce a CompileCommand for \p filename, based on this one.
   CompileCommand transferTo(StringRef Filename) const {
-CompileCommand Result = Cmd;
+assert(TraitsComputed && "calling transferTo on moved-from object");
+const CommandTraits &T = getTraits();
+CompileCommand Result = T.Cmd;
 Result.Filename = Filename;
 bool TypeCertain;
 auto TargetType = guessType(Filename, &TypeCertain);
 // If the filename doesn't determine the language (.h), transfer with -x.
 if (!TypeCertain) {
   TargetType = types::onlyPrecompileType(TargetType) // header?
-   ? types::lookupHeaderTypeForSourceType(Type)
-   : Type;
+   ? types::lookupHeaderTypeForSourceType(T.Type)
+   : T.Type;
   Result.CommandLine.push_back("-x");
   Result.CommandLine.push_back(types::getTypeName(TargetType));
 }
 // --std flag may only be transferred if the language is the same.
 // We may consider "translating" these, e.g. c++11 -> c11.
-if (Std != LangStandard::lang_unspecified && foldType(TargetType) == Type) {
+if (T.Std != LangStandard::lang_unspecified &&
+foldType(TargetType) == T.Type) {
   Result.CommandLine.push_back(
   "-std=" +
-  std::string(LangStandard::getLangStandardForKind(Std).getName()));
+  std::string(LangStandard::getLangStandardForKind(T.Std).getName()));
 }
 Result.CommandLine.push_back(Filename);
 return Result;
   }
 
+  llvm::StringRef filename() const {
+assert(TraitsComputed && "calling filename on moved-from object");
+return OriginalCmd.Fi

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:201
+
+  CommandTraits computeTraits() const {
+CommandTraits Result;

jfb wrote:
> It's not clear to me that this entire function is safe to call from multiple 
> threads at the same time. Even if it's safe now, I'm willing to bet it won't 
> always be that way. `getTraits` should therefore use a magic static or 
> `call_once` and avoid the headache entirely.
Thanks, `call_once` is both simpler and more reliable.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:258
+  /// Null before Traits were computed, non-null otherwise. All accesses to 
this
+  /// must be atomic.
+  mutable std::shared_ptr Traits;

jfb wrote:
> The comment should say why accesses need to be atomic. Or better yet, this 
> should only be usable "the right way".
Clarified how it's used.


Repository:
  rC Clang

https://reviews.llvm.org/D51314



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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Remove #include , it is not used anymore


Repository:
  rC Clang

https://reviews.llvm.org/D51314

Files:
  lib/Tooling/InterpolatingCompilationDatabase.cpp

Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -57,6 +57,7 @@
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 
 namespace clang {
 namespace tooling {
@@ -119,87 +120,63 @@
   }
 }
 
-// A CompileCommand that can be applied to another file.
+// A CompileCommand that can be applied to another file. Any instance of this
+// object is invalid after std::move() from it.
 struct TransferableCommand {
-  // Flags that should not apply to all files are stripped from CommandLine.
-  CompileCommand Cmd;
-  // Language detected from -x or the filename.
-  types::ID Type = types::TY_INVALID;
-  // Standard specified by -std.
-  LangStandard::Kind Std = LangStandard::lang_unspecified;
-
   TransferableCommand(CompileCommand C)
-  : Cmd(std::move(C)), Type(guessType(Cmd.Filename)) {
-std::vector NewArgs = {Cmd.CommandLine.front()};
-// Parse the old args in order to strip out and record unwanted flags.
-auto OptTable = clang::driver::createDriverOptTable();
-std::vector Argv;
-for (unsigned I = 1; I < Cmd.CommandLine.size(); ++I)
-  Argv.push_back(Cmd.CommandLine[I].c_str());
-unsigned MissingI, MissingC;
-auto ArgList = OptTable->ParseArgs(Argv, MissingI, MissingC);
-for (const auto *Arg : ArgList) {
-  const auto &option = Arg->getOption();
-  // Strip input and output files.
-  if (option.matches(clang::driver::options::OPT_INPUT) ||
-  option.matches(clang::driver::options::OPT_o)) {
-continue;
-  }
-  // Strip -x, but record the overridden language.
-  if (option.matches(clang::driver::options::OPT_x)) {
-for (const char *Value : Arg->getValues())
-  Type = types::lookupTypeForTypeSpecifier(Value);
-continue;
-  }
-  // Strip --std, but record the value.
-  if (option.matches(clang::driver::options::OPT_std_EQ)) {
-for (const char *Value : Arg->getValues()) {
-  Std = llvm::StringSwitch(Value)
-#define LANGSTANDARD(id, name, lang, desc, features)   \
-  .Case(name, LangStandard::lang_##id)
-#define LANGSTANDARD_ALIAS(id, alias) .Case(alias, LangStandard::lang_##id)
-#include "clang/Frontend/LangStandards.def"
-.Default(Std);
-}
-continue;
-  }
-  llvm::opt::ArgStringList ArgStrs;
-  Arg->render(ArgList, ArgStrs);
-  NewArgs.insert(NewArgs.end(), ArgStrs.begin(), ArgStrs.end());
-}
-Cmd.CommandLine = std::move(NewArgs);
-
-if (Std != LangStandard::lang_unspecified) // -std take precedence over -x
-  Type = toType(LangStandard::getLangStandardForKind(Std).getLanguage());
-Type = foldType(Type);
-  }
+  : OriginalCmd(std::move(C)),
+TraitsComputed(llvm::make_unique()) {}
 
   // Produce a CompileCommand for \p filename, based on this one.
   CompileCommand transferTo(StringRef Filename) const {
-CompileCommand Result = Cmd;
+assert(TraitsComputed && "calling transferTo on moved-from object");
+const CommandTraits &T = getTraits();
+CompileCommand Result = T.Cmd;
 Result.Filename = Filename;
 bool TypeCertain;
 auto TargetType = guessType(Filename, &TypeCertain);
 // If the filename doesn't determine the language (.h), transfer with -x.
 if (!TypeCertain) {
   TargetType = types::onlyPrecompileType(TargetType) // header?
-   ? types::lookupHeaderTypeForSourceType(Type)
-   : Type;
+   ? types::lookupHeaderTypeForSourceType(T.Type)
+   : T.Type;
   Result.CommandLine.push_back("-x");
   Result.CommandLine.push_back(types::getTypeName(TargetType));
 }
 // --std flag may only be transferred if the language is the same.
 // We may consider "translating" these, e.g. c++11 -> c11.
-if (Std != LangStandard::lang_unspecified && foldType(TargetType) == Type) {
+if (T.Std != LangStandard::lang_unspecified &&
+foldType(TargetType) == T.Type) {
   Result.CommandLine.push_back(
   "-std=" +
-  std::string(LangStandard::getLangStandardForKind(Std).getName()));
+  std::string(LangStandard::getLangStandardForKind(T.Std).getName()));
 }
 Result.CommandLine.push_back(Filename);
 return Result;
   }
 
+  llvm::StringRef filename() const {
+assert(TraitsComputed && "calling filename on moved-from object");
+return OriginalCmd.Filename;
+  }
+
+  types::ID type() const {
+assert(TraitsComputed && "calling type on moved-from 

[PATCH] D51240: Add a flag to remap manglings when reading profile data information.

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In https://reviews.llvm.org/D51240#1213358, @davidxl wrote:

> Re  "Why not":
>
> The common use model of instrumentation based PGO and SamplePGO are quite 
> different. The former usually uses 'fresh' profile that matches the source. 
> If there are massive code refactoring or ABI changes, the user can simply 
> regenerate the profile.  For the latter, the profile data is usually 
> collected from production binaries, so the source is almost always newer than 
> the profile.


Frontend-based instrumentation is designed to degrade gracefully as the source 
changes.  I don't follow your conclusion that the profile is necessarily fresh. 
 (I'm fine with the patch being split though.)


https://reviews.llvm.org/D51240



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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:124
+// A CompileCommand that can be applied to another file. Any instance of this
+// object is invalid after std::move() from it.
 struct TransferableCommand {

This comment about `move` isn't really saying anything. Also, it's valid but 
unspecified (in the case of STL things). I'd drop it.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:128
+  : OriginalCmd(std::move(C)),
+TraitsComputed(llvm::make_unique()) {}
 

The `once_flag` should just be a static, don't allocate it.


Repository:
  rC Clang

https://reviews.llvm.org/D51314



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

[clang-tools-extra] r340759 - Cleanup after rL340729

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Mon Aug 27 10:26:43 2018
New Revision: 340759

URL: http://llvm.org/viewvc/llvm-project?rev=340759&view=rev
Log:
Cleanup after rL340729

Modified:
clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp?rev=340759&r1=340758&r2=340759&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp Mon Aug 27 
10:26:43 2018
@@ -335,35 +335,34 @@ trigramsAre(std::initializer_listhttp://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r340709 - [Driver] Change MipsLinux default linker from "lld" to "ld.lld"

[Fāng-ruì Sòng via cfe-commits]

Thanks!
On Mon, Aug 27, 2018 at 1:50 AM Chandler Carruth  wrote:
>
> Build bots have been broken all day, so I'm trying a speculative fix in 
> r340727. If this doesn't work, i'll just revert all of this.
>
> On Sun, Aug 26, 2018 at 3:51 PM Chandler Carruth  wrote:
>>
>> FYI, test cases also seem to need updating:
>> http://lab.llvm.org:8011/builders/clang-ppc64le-linux/builds/19575/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Amips-mti-linux.c
>>
>> On Sun, Aug 26, 2018 at 12:48 PM Fangrui Song via cfe-commits 
>>  wrote:
>>>
>>> Author: maskray
>>> Date: Sun Aug 26 12:47:23 2018
>>> New Revision: 340709
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=340709&view=rev
>>> Log:
>>> [Driver] Change MipsLinux default linker from "lld" to "ld.lld"
>>>
>>> Reviewers: kzhuravl, atanasyan
>>>
>>> Reviewed By: atanasyan
>>>
>>> Subscribers: sdardis, arichardson, jrtc27, atanasyan, cfe-commits
>>>
>>> Differential Revision: https://reviews.llvm.org/D51234
>>>
>>> Modified:
>>> cfe/trunk/lib/Driver/ToolChains/MipsLinux.h
>>>
>>> Modified: cfe/trunk/lib/Driver/ToolChains/MipsLinux.h
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/MipsLinux.h?rev=340709&r1=340708&r2=340709&view=diff
>>> ==
>>> --- cfe/trunk/lib/Driver/ToolChains/MipsLinux.h (original)
>>> +++ cfe/trunk/lib/Driver/ToolChains/MipsLinux.h Sun Aug 26 12:47:23 2018
>>> @@ -49,7 +49,7 @@ public:
>>>}
>>>
>>>const char *getDefaultLinker() const override {
>>> -return "lld";
>>> +return "ld.lld";
>>>}
>>>
>>>  private:
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits



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

[PATCH] D51312: [OpenMP][NVPTX] Use appropriate _CALL_ELF macro when offloading

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

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

Add test.


Repository:
  rC Clang

https://reviews.llvm.org/D51312

Files:
  lib/Frontend/InitPreprocessor.cpp
  test/Preprocessor/aux-triple.c


Index: test/Preprocessor/aux-triple.c
===
--- test/Preprocessor/aux-triple.c
+++ test/Preprocessor/aux-triple.c
@@ -14,7 +14,7 @@
 // RUN: %clang_cc1 -x cuda -E -dM -ffreestanding < /dev/null \
 // RUN: -triple nvptx64-none-none -aux-triple 
powerpc64le-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
-// RUN: -check-prefixes NVPTX64,PPC64,LINUX,LINUX-CPP
+// RUN: -check-prefixes NVPTX64,PPC64LE,LINUX,LINUX-CPP
 // RUN: %clang_cc1 -x cuda -E -dM -ffreestanding < /dev/null \
 // RUN: -triple nvptx64-none-none -aux-triple x86_64-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
@@ -24,22 +24,24 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple powerpc64le-unknown-linux-gnu \
-// RUN:   | FileCheck -match-full-lines -check-prefixes NVPTX64,PPC64,LINUX %s
+// RUN:   | FileCheck -match-full-lines -check-prefixes NVPTX64,PPC64LE,LINUX 
%s
 // RUN: %clang_cc1 -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple x86_64-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines -check-prefixes NVPTX64,X86_64,LINUX %s
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple powerpc64le-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
-// RUN: -check-prefixes NVPTX64,PPC64,LINUX,LINUX-CPP
+// RUN: -check-prefixes NVPTX64,PPC64LE,LINUX,LINUX-CPP
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple x86_64-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
 // RUN: -check-prefixes NVPTX64,X86_64,LINUX,LINUX-CPP
 
+// PPC64LE:#define _CALL_ELF 2
+
 // NONE-NOT:#define _GNU_SOURCE
 // LINUX-CPP:#define _GNU_SOURCE 1
 
@@ -56,7 +58,7 @@
 // LINUX:#define __linux__ 1
 
 // NONE-NOT:#define __powerpc64__
-// PPC64:#define __powerpc64__ 1
+// PPC64LE:#define __powerpc64__ 1
 
 // NONE-NOT:#define __x86_64__
 // X86_64:#define __x86_64__ 1
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -1106,14 +1106,19 @@
   auto AuxTriple = AuxTI.getTriple();
 
   // Define basic target macros needed by at least bits/wordsize.h and
-  // bits/mathinline.h
+  // bits/mathinline.h.
+  // On PowerPC, explicitely set _CALL_ELF macro needed for gnu/stubs.h.
   switch (AuxTriple.getArch()) {
   case llvm::Triple::x86_64:
 Builder.defineMacro("__x86_64__");
 break;
   case llvm::Triple::ppc64:
+Builder.defineMacro("__powerpc64__");
+Builder.defineMacro("_CALL_ELF", "1");
+break;
   case llvm::Triple::ppc64le:
 Builder.defineMacro("__powerpc64__");
+Builder.defineMacro("_CALL_ELF", "2");
 break;
   default:
 break;


Index: test/Preprocessor/aux-triple.c
===
--- test/Preprocessor/aux-triple.c
+++ test/Preprocessor/aux-triple.c
@@ -14,7 +14,7 @@
 // RUN: %clang_cc1 -x cuda -E -dM -ffreestanding < /dev/null \
 // RUN: -triple nvptx64-none-none -aux-triple powerpc64le-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
-// RUN: -check-prefixes NVPTX64,PPC64,LINUX,LINUX-CPP
+// RUN: -check-prefixes NVPTX64,PPC64LE,LINUX,LINUX-CPP
 // RUN: %clang_cc1 -x cuda -E -dM -ffreestanding < /dev/null \
 // RUN: -triple nvptx64-none-none -aux-triple x86_64-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
@@ -24,22 +24,24 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple powerpc64le-unknown-linux-gnu \
-// RUN:   | FileCheck -match-full-lines -check-prefixes NVPTX64,PPC64,LINUX %s
+// RUN:   | FileCheck -match-full-lines -check-prefixes NVPTX64,PPC64LE,LINUX %s
 // RUN: %clang_cc1 -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple x86_64-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines -check-prefixes NVPTX64,X86_64,LINUX %s
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple powerpc64le-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
-// RUN: -check-prefixes NVPTX64,PPC64,LINUX,LINUX-CPP
+// RUN: -check-prefixes N

[PATCH] D51229: [Sema/Attribute] Make types declared with address_space an AttributedType

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 162711.
leonardchan marked an inline comment as done.

Repository:
  rC Clang

https://reviews.llvm.org/D51229

Files:
  include/clang/Basic/Attr.td
  lib/AST/TypePrinter.cpp
  lib/Sema/SemaType.cpp

Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -5597,12 +5597,6 @@
   }
 
   for (unsigned i = 0, e = D.getNumTypeObjects(); i != e; ++i) {
-if (DependentAddressSpaceTypeLoc DASTL =
-CurrTL.getAs()) {
-  fillDependentAddressSpaceTypeLoc(DASTL, D.getTypeObject(i).getAttrs());
-  CurrTL = DASTL.getPointeeTypeLoc().getUnqualifiedLoc();
-}
-
 // An AtomicTypeLoc might be produced by an atomic qualifier in this
 // declarator chunk.
 if (AtomicTypeLoc ATL = CurrTL.getAs()) {
@@ -5615,6 +5609,12 @@
   CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
 }
 
+while (DependentAddressSpaceTypeLoc TL =
+   CurrTL.getAs()) {
+  fillDependentAddressSpaceTypeLoc(TL, D.getTypeObject(i).getAttrs());
+  CurrTL = TL.getPointeeTypeLoc().getUnqualifiedLoc();
+}
+
 // FIXME: Ordering here?
 while (AdjustedTypeLoc TL = CurrTL.getAs())
   CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
@@ -5767,7 +5767,10 @@
 /// specified type.  The attribute contains 1 argument, the id of the address
 /// space for the type.
 static void HandleAddressSpaceTypeAttribute(QualType &Type,
-const ParsedAttr &Attr, Sema &S) {
+const ParsedAttr &Attr,
+TypeProcessingState &State) {
+  Sema &S = State.getSema();
+
   // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "A function type shall not be
   // qualified by an address-space qualifier."
   if (Type->isFunctionType()) {
@@ -5809,10 +5812,15 @@
 // the type.
 QualType T = S.BuildAddressSpaceAttr(Type, ASArgExpr, Attr.getLoc());
 
-if (!T.isNull())
-  Type = T;
-else
+if (!T.isNull()) {
+  ASTContext &Ctx = S.Context;
+  auto *ASAttr = ::new (Ctx) AddressSpaceAttr(
+  Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(),
+  static_cast(T.getQualifiers().getAddressSpace()));
+  Type = State.getAttributedType(ASAttr, T, T);
+} else {
   Attr.setInvalid();
+}
   } else {
 // The keyword-based type attributes imply which address space to use.
 switch (Attr.getKind()) {
@@ -7282,7 +7290,7 @@
 case ParsedAttr::AT_OpenCLConstantAddressSpace:
 case ParsedAttr::AT_OpenCLGenericAddressSpace:
 case ParsedAttr::AT_AddressSpace:
-  HandleAddressSpaceTypeAttribute(type, attr, state.getSema());
+  HandleAddressSpaceTypeAttribute(type, attr, state);
   attr.setUsedAsTypeAttr();
   break;
 OBJC_POINTER_TYPE_ATTRS_CASELIST:
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1427,6 +1427,12 @@
 return;
   }
 
+  // The printing of the address_space attribute is handled by the qualifier
+  // since it is still stored in the qualifier. Return early to prevent printing
+  // this twice.
+  if (T->getAttrKind() == attr::AddressSpace)
+return;
+
   OS << " __attribute__((";
   switch (T->getAttrKind()) {
 #define TYPE_ATTR(NAME)
@@ -1456,6 +1462,7 @@
   case attr::Ptr64:
   case attr::SPtr:
   case attr::UPtr:
+  case attr::AddressSpace:
 llvm_unreachable("This attribute should have been handled already");
 
   case attr::NSReturnsRetained:
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -564,8 +564,6 @@
   let Spellings = [Clang<"address_space">];
   let Args = [IntArgument<"AddressSpace">];
   let Documentation = [Undocumented];
-  // Represented as a qualifier or DependentAddressSpaceType instead.
-  let ASTNode = 0;
 }
 
 def Alias : Attr {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r340765 - [Sema/Attribute] Make types declared with address_space an AttributedType

[Leonard Chan via cfe-commits]

Author: leonardchan
Date: Mon Aug 27 10:57:29 2018
New Revision: 340765

URL: http://llvm.org/viewvc/llvm-project?rev=340765&view=rev
Log:
[Sema/Attribute] Make types declared with address_space an AttributedType

Currently an address_space is stored in a qualifier. This makes any type
declared with an address_space attribute in the form
`__attribute__((address_space(1))) int 1;` be wrapped in an AttributedType.

This is for a later patch where if `address_space` is declared in a macro,
any diagnostics that would normally print the address space will instead dump
the macro name. This will require saving any macro information in the
AttributedType.

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

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/lib/AST/TypePrinter.cpp
cfe/trunk/lib/Sema/SemaType.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=340765&r1=340764&r2=340765&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Mon Aug 27 10:57:29 2018
@@ -564,8 +564,6 @@ def AddressSpace : TypeAttr {
   let Spellings = [Clang<"address_space">];
   let Args = [IntArgument<"AddressSpace">];
   let Documentation = [Undocumented];
-  // Represented as a qualifier or DependentAddressSpaceType instead.
-  let ASTNode = 0;
 }
 
 def Alias : Attr {

Modified: cfe/trunk/lib/AST/TypePrinter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TypePrinter.cpp?rev=340765&r1=340764&r2=340765&view=diff
==
--- cfe/trunk/lib/AST/TypePrinter.cpp (original)
+++ cfe/trunk/lib/AST/TypePrinter.cpp Mon Aug 27 10:57:29 2018
@@ -1427,6 +1427,12 @@ void TypePrinter::printAttributedAfter(c
 return;
   }
 
+  // The printing of the address_space attribute is handled by the qualifier
+  // since it is still stored in the qualifier. Return early to prevent 
printing
+  // this twice.
+  if (T->getAttrKind() == attr::AddressSpace)
+return;
+
   OS << " __attribute__((";
   switch (T->getAttrKind()) {
 #define TYPE_ATTR(NAME)
@@ -1456,6 +1462,7 @@ void TypePrinter::printAttributedAfter(c
   case attr::Ptr64:
   case attr::SPtr:
   case attr::UPtr:
+  case attr::AddressSpace:
 llvm_unreachable("This attribute should have been handled already");
 
   case attr::NSReturnsRetained:

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=340765&r1=340764&r2=340765&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Mon Aug 27 10:57:29 2018
@@ -5597,12 +5597,6 @@ GetTypeSourceInfoForDeclarator(TypeProce
   }
 
   for (unsigned i = 0, e = D.getNumTypeObjects(); i != e; ++i) {
-if (DependentAddressSpaceTypeLoc DASTL =
-CurrTL.getAs()) {
-  fillDependentAddressSpaceTypeLoc(DASTL, D.getTypeObject(i).getAttrs());
-  CurrTL = DASTL.getPointeeTypeLoc().getUnqualifiedLoc();
-}
-
 // An AtomicTypeLoc might be produced by an atomic qualifier in this
 // declarator chunk.
 if (AtomicTypeLoc ATL = CurrTL.getAs()) {
@@ -5615,6 +5609,12 @@ GetTypeSourceInfoForDeclarator(TypeProce
   CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
 }
 
+while (DependentAddressSpaceTypeLoc TL =
+   CurrTL.getAs()) {
+  fillDependentAddressSpaceTypeLoc(TL, D.getTypeObject(i).getAttrs());
+  CurrTL = TL.getPointeeTypeLoc().getUnqualifiedLoc();
+}
+
 // FIXME: Ordering here?
 while (AdjustedTypeLoc TL = CurrTL.getAs())
   CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
@@ -5767,7 +5767,10 @@ QualType Sema::BuildAddressSpaceAttr(Qua
 /// specified type.  The attribute contains 1 argument, the id of the address
 /// space for the type.
 static void HandleAddressSpaceTypeAttribute(QualType &Type,
-const ParsedAttr &Attr, Sema &S) {
+const ParsedAttr &Attr,
+TypeProcessingState &State) {
+  Sema &S = State.getSema();
+
   // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "A function type shall not be
   // qualified by an address-space qualifier."
   if (Type->isFunctionType()) {
@@ -5809,10 +5812,15 @@ static void HandleAddressSpaceTypeAttrib
 // the type.
 QualType T = S.BuildAddressSpaceAttr(Type, ASArgExpr, Attr.getLoc());
 
-if (!T.isNull())
-  Type = T;
-else
+if (!T.isNull()) {
+  ASTContext &Ctx = S.Context;
+  auto *ASAttr = ::new (Ctx) AddressSpaceAttr(
+  Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(),
+  static_cast(T.getQualifiers().getAddressSpace()));
+  Type = State.getAttri

[PATCH] D51229: [Sema/Attribute] Make types declared with address_space an AttributedType

[Leonard Chan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340765: [Sema/Attribute] Make types declared with 
address_space an AttributedType (authored by leonardchan, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D51229

Files:
  include/clang/Basic/Attr.td
  lib/AST/TypePrinter.cpp
  lib/Sema/SemaType.cpp

Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -564,8 +564,6 @@
   let Spellings = [Clang<"address_space">];
   let Args = [IntArgument<"AddressSpace">];
   let Documentation = [Undocumented];
-  // Represented as a qualifier or DependentAddressSpaceType instead.
-  let ASTNode = 0;
 }
 
 def Alias : Attr {
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1427,6 +1427,12 @@
 return;
   }
 
+  // The printing of the address_space attribute is handled by the qualifier
+  // since it is still stored in the qualifier. Return early to prevent printing
+  // this twice.
+  if (T->getAttrKind() == attr::AddressSpace)
+return;
+
   OS << " __attribute__((";
   switch (T->getAttrKind()) {
 #define TYPE_ATTR(NAME)
@@ -1456,6 +1462,7 @@
   case attr::Ptr64:
   case attr::SPtr:
   case attr::UPtr:
+  case attr::AddressSpace:
 llvm_unreachable("This attribute should have been handled already");
 
   case attr::NSReturnsRetained:
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -5597,12 +5597,6 @@
   }
 
   for (unsigned i = 0, e = D.getNumTypeObjects(); i != e; ++i) {
-if (DependentAddressSpaceTypeLoc DASTL =
-CurrTL.getAs()) {
-  fillDependentAddressSpaceTypeLoc(DASTL, D.getTypeObject(i).getAttrs());
-  CurrTL = DASTL.getPointeeTypeLoc().getUnqualifiedLoc();
-}
-
 // An AtomicTypeLoc might be produced by an atomic qualifier in this
 // declarator chunk.
 if (AtomicTypeLoc ATL = CurrTL.getAs()) {
@@ -5615,6 +5609,12 @@
   CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
 }
 
+while (DependentAddressSpaceTypeLoc TL =
+   CurrTL.getAs()) {
+  fillDependentAddressSpaceTypeLoc(TL, D.getTypeObject(i).getAttrs());
+  CurrTL = TL.getPointeeTypeLoc().getUnqualifiedLoc();
+}
+
 // FIXME: Ordering here?
 while (AdjustedTypeLoc TL = CurrTL.getAs())
   CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
@@ -5767,7 +5767,10 @@
 /// specified type.  The attribute contains 1 argument, the id of the address
 /// space for the type.
 static void HandleAddressSpaceTypeAttribute(QualType &Type,
-const ParsedAttr &Attr, Sema &S) {
+const ParsedAttr &Attr,
+TypeProcessingState &State) {
+  Sema &S = State.getSema();
+
   // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "A function type shall not be
   // qualified by an address-space qualifier."
   if (Type->isFunctionType()) {
@@ -5809,10 +5812,15 @@
 // the type.
 QualType T = S.BuildAddressSpaceAttr(Type, ASArgExpr, Attr.getLoc());
 
-if (!T.isNull())
-  Type = T;
-else
+if (!T.isNull()) {
+  ASTContext &Ctx = S.Context;
+  auto *ASAttr = ::new (Ctx) AddressSpaceAttr(
+  Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(),
+  static_cast(T.getQualifiers().getAddressSpace()));
+  Type = State.getAttributedType(ASAttr, T, T);
+} else {
   Attr.setInvalid();
+}
   } else {
 // The keyword-based type attributes imply which address space to use.
 switch (Attr.getKind()) {
@@ -7282,7 +7290,7 @@
 case ParsedAttr::AT_OpenCLConstantAddressSpace:
 case ParsedAttr::AT_OpenCLGenericAddressSpace:
 case ParsedAttr::AT_AddressSpace:
-  HandleAddressSpaceTypeAttribute(type, attr, state.getSema());
+  HandleAddressSpaceTypeAttribute(type, attr, state);
   attr.setUsedAsTypeAttr();
   break;
 OBJC_POINTER_TYPE_ATTRS_CASELIST:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D51234: [Driver] Change MipsLinux default linker from "lld" to "ld.lld"

[Fāng-ruì Sòng via cfe-commits]

On 2018-08-27, Rui Ueyama via Phabricator wrote:

ruiu added a comment.

If this piece of code used to be working correctly, there is another piece of code that 
adds `-flavor ld` to the command line. But if that's the case, this change wouldn't work 
because it constructs something like "ld.lld -flavor ld ...". ld.lld doesn't 
accept `-flavor` option.

So my guess is this code is dead. Or, am I missing something?


I also suspect the code is dead.

@chandlerc fixed the related test (which I had not noticed when I committed the 
change) in https://reviews.llvm.org/rL340727
But the support of -flavor old-gnu was removed in 2016 
https://reviews.llvm.org/rLLD262158




Repository:
 rL LLVM

https://reviews.llvm.org/D51234





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

r340770 - [PPC] Remove Darwin support from POWER backend.

[Kit Barton via cfe-commits]

Author: kbarton
Date: Mon Aug 27 12:53:19 2018
New Revision: 340770

URL: http://llvm.org/viewvc/llvm-project?rev=340770&view=rev
Log:
[PPC] Remove Darwin support from POWER backend.
This patch removes uses of the Darwin ABI for PowerPC related test cases. This
is the first step in removing Darwin support from the POWER backend.

clang/test/CodeGen/darwin-ppc-varargs.c  was deleted because it was a darwin/ppc
specific test case.

All other tests were updated to remove the darwin/ppc specific invocation.

Phabricator Review: https://reviews.llvm.org/D50989.

Removed:
cfe/trunk/test/CodeGen/darwin-ppc-varargs.c
Modified:
cfe/trunk/test/CodeGen/bool_test.c
cfe/trunk/test/CodeGen/darwin-string-literals.c
cfe/trunk/test/CodeGen/target-data.c
cfe/trunk/test/CodeGenCXX/mangle-long-double.cpp
cfe/trunk/test/Coverage/targets.c

Modified: cfe/trunk/test/CodeGen/bool_test.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/bool_test.c?rev=340770&r1=340769&r2=340770&view=diff
==
--- cfe/trunk/test/CodeGen/bool_test.c (original)
+++ cfe/trunk/test/CodeGen/bool_test.c Mon Aug 27 12:53:19 2018
@@ -1,18 +1,18 @@
 // REQUIRES: powerpc-registered-target
-// RUN: %clang_cc1 -triple powerpc-apple-macosx10.4.0 -emit-llvm -o - %s -O2 
-disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-unknown-linux-gnu -emit-llvm -o - %s -O2 
-disable-llvm-passes | FileCheck %s
 
 int boolsize = sizeof(_Bool);
-// CHECK: boolsize = global i32 4, align 4
+// CHECK: boolsize = global i32 1, align 4
 
 void f(_Bool *x, _Bool *y) {
   *x = *y;
 }
 
 // CHECK-LABEL: define void @f(
-// CHECK: [[FROMMEM:%.*]] = load i32, i32* %
-// CHECK: [[BOOLVAL:%.*]] = trunc i32 [[FROMMEM]] to i1
-// CHECK: [[TOMEM:%.*]] = zext i1 [[BOOLVAL]] to i32
-// CHECK: store i32 [[TOMEM]]
+// CHECK: [[FROMMEM:%.*]] = load i8, i8* %
+// CHECK: [[BOOLVAL:%.*]] = trunc i8 [[FROMMEM]] to i1
+// CHECK: [[TOMEM:%.*]] = zext i1 [[BOOLVAL]] to i8
+// CHECK: store i8 [[TOMEM]]
 // CHECK: ret void
 
-// CHECK:  i32 0, i32 2}
+// CHECK:  i8 0, i8 2}

Removed: cfe/trunk/test/CodeGen/darwin-ppc-varargs.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/darwin-ppc-varargs.c?rev=340769&view=auto
==
--- cfe/trunk/test/CodeGen/darwin-ppc-varargs.c (original)
+++ cfe/trunk/test/CodeGen/darwin-ppc-varargs.c (removed)
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 -triple powerpc-apple-macosx10.5.0 -target-feature +altivec 
-Os -emit-llvm -o - %s | FileCheck %s
-
-int f(__builtin_va_list args) { return __builtin_va_arg(args, int); }
-
-// CHECK: @f(i8* {{.*}}[[PARAM:%[a-zA-Z0-9]+]])
-// CHECK: [[BITCAST:%[0-9]+]] = bitcast i8* [[PARAM]] to i32*
-// CHECK: [[VALUE:%[0-9]+]] = load i32, i32* [[BITCAST]], align 4
-// CHECK: ret i32 [[VALUE]]
-
-void h(vector int);
-int g(__builtin_va_list args) {
-  int i = __builtin_va_arg(args, int);
-  h(__builtin_va_arg(args, vector int));
-  int j = __builtin_va_arg(args, int);
-  return i + j;
-}
-
-// CHECK: @g(i8* {{.*}}[[PARAM:%[a-zA-Z0-9]+]])
-// CHECK: [[NEXT:%[-_.a-zA-Z0-9]+]] = getelementptr inbounds i8, i8* 
[[PARAM]], i32 4
-// CHECK: [[BITCAST:%[0-9]+]] = bitcast i8* [[PARAM]] to i32*
-// CHECK: [[LOAD:%[0-9]+]] = load i32, i32* [[BITCAST]], align 4
-// CHECK: [[PTRTOINT:%[0-9]+]] = ptrtoint i8* [[NEXT]] to i32
-// CHECK: [[ADD:%[0-9]+]] = add i32 [[PTRTOINT]], 15
-// CHECK: [[AND:%[0-9]+]] = and i32 [[ADD]], -16
-// CHECK: [[INTTOPTR:%[0-9]+]] = inttoptr i32 [[AND]] to <4 x i32>*
-// CHECK: [[ARG:%[0-9]]] = load <4 x i32>, <4 x i32>* [[INTTOPTR]], align 16
-// CHECK: call void @h(<4 x i32> [[ARG]]
-

Modified: cfe/trunk/test/CodeGen/darwin-string-literals.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/darwin-string-literals.c?rev=340770&r1=340769&r2=340770&view=diff
==
--- cfe/trunk/test/CodeGen/darwin-string-literals.c (original)
+++ cfe/trunk/test/CodeGen/darwin-string-literals.c Mon Aug 27 12:53:19 2018
@@ -5,14 +5,6 @@
 // CHECK-LSB: @.str.2 = private unnamed_addr constant [18 x i16] [i16 104, i16 
101, i16 108, i16 108, i16 111, i16 32, i16 8594, i16 32, i16 9731, i16 32, i16 
8592, i16 32, i16 119, i16 111, i16 114, i16 108, i16 100, i16 0], section 
"__TEXT,__ustring", align 2
 // CHECK-LSB: @.str.4 = private unnamed_addr constant [6 x i16] [i16 116, i16 
101, i16 115, i16 116, i16 8482, i16 0], section "__TEXT,__ustring", align 2
 
-
-// RUN: %clang_cc1 -triple powerpc-apple-darwin9 -emit-llvm %s -o - | 
FileCheck -check-prefix CHECK-MSB %s
-
-// CHECK-MSB: @.str = private unnamed_addr constant [8 x i8] c"string0\00"
-// CHECK-MSB: @.str.1 = private unnamed_addr constant [8 x i8] c"string1\00"
-// CHECK-MSB: @.str.2 = private unnamed_addr constant [18 x i16] [i16 104, i16 
101, i16 108, i16 108, i16 111, i16 32, i16 8594, i16 32

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

[Vedant Kumar via Phabricator via cfe-commits]

vsk updated this revision to Diff 162732.
vsk added a comment.

Address some review feedback.

I'm not sure whether iOS 4 is really supported anymore. There are a handful of 
code paths in the driver which handle that target, so I've added in a version 
check for it.


https://reviews.llvm.org/D51239

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


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


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

r340772 - [OpenMP][NVPTX] Use appropriate _CALL_ELF macro when offloading

[Gheorghe-Teodor Bercea via cfe-commits]

Author: gbercea
Date: Mon Aug 27 13:16:20 2018
New Revision: 340772

URL: http://llvm.org/viewvc/llvm-project?rev=340772&view=rev
Log:
[OpenMP][NVPTX] Use appropriate _CALL_ELF macro when offloading

Summary: When offloading to a device and using the powerpc64le version of the 
auxiliary triple, the _CALL_ELF macro is not set correctly to 2 resulting in 
the attempt to include a header that does not exist. This patch fixes this 
problem.

Reviewers: Hahnfeld, ABataev, caomhin

Reviewed By: Hahnfeld

Subscribers: guansong, cfe-commits

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

Modified:
cfe/trunk/lib/Frontend/InitPreprocessor.cpp
cfe/trunk/test/Preprocessor/aux-triple.c

Modified: cfe/trunk/lib/Frontend/InitPreprocessor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitPreprocessor.cpp?rev=340772&r1=340771&r2=340772&view=diff
==
--- cfe/trunk/lib/Frontend/InitPreprocessor.cpp (original)
+++ cfe/trunk/lib/Frontend/InitPreprocessor.cpp Mon Aug 27 13:16:20 2018
@@ -1106,14 +1106,19 @@ static void InitializePredefinedAuxMacro
   auto AuxTriple = AuxTI.getTriple();
 
   // Define basic target macros needed by at least bits/wordsize.h and
-  // bits/mathinline.h
+  // bits/mathinline.h.
+  // On PowerPC, explicitely set _CALL_ELF macro needed for gnu/stubs.h.
   switch (AuxTriple.getArch()) {
   case llvm::Triple::x86_64:
 Builder.defineMacro("__x86_64__");
 break;
   case llvm::Triple::ppc64:
+Builder.defineMacro("__powerpc64__");
+Builder.defineMacro("_CALL_ELF", "1");
+break;
   case llvm::Triple::ppc64le:
 Builder.defineMacro("__powerpc64__");
+Builder.defineMacro("_CALL_ELF", "2");
 break;
   default:
 break;

Modified: cfe/trunk/test/Preprocessor/aux-triple.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/aux-triple.c?rev=340772&r1=340771&r2=340772&view=diff
==
--- cfe/trunk/test/Preprocessor/aux-triple.c (original)
+++ cfe/trunk/test/Preprocessor/aux-triple.c Mon Aug 27 13:16:20 2018
@@ -14,7 +14,7 @@
 // RUN: %clang_cc1 -x cuda -E -dM -ffreestanding < /dev/null \
 // RUN: -triple nvptx64-none-none -aux-triple 
powerpc64le-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
-// RUN: -check-prefixes NVPTX64,PPC64,LINUX,LINUX-CPP
+// RUN: -check-prefixes NVPTX64,PPC64LE,LINUX,LINUX-CPP
 // RUN: %clang_cc1 -x cuda -E -dM -ffreestanding < /dev/null \
 // RUN: -triple nvptx64-none-none -aux-triple x86_64-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
@@ -24,7 +24,7 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple powerpc64le-unknown-linux-gnu \
-// RUN:   | FileCheck -match-full-lines -check-prefixes NVPTX64,PPC64,LINUX %s
+// RUN:   | FileCheck -match-full-lines -check-prefixes NVPTX64,PPC64LE,LINUX 
%s
 // RUN: %clang_cc1 -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple x86_64-unknown-linux-gnu \
@@ -33,13 +33,15 @@
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple powerpc64le-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
-// RUN: -check-prefixes NVPTX64,PPC64,LINUX,LINUX-CPP
+// RUN: -check-prefixes NVPTX64,PPC64LE,LINUX,LINUX-CPP
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple x86_64-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
 // RUN: -check-prefixes NVPTX64,X86_64,LINUX,LINUX-CPP
 
+// PPC64LE:#define _CALL_ELF 2
+
 // NONE-NOT:#define _GNU_SOURCE
 // LINUX-CPP:#define _GNU_SOURCE 1
 
@@ -56,7 +58,7 @@
 // LINUX:#define __linux__ 1
 
 // NONE-NOT:#define __powerpc64__
-// PPC64:#define __powerpc64__ 1
+// PPC64LE:#define __powerpc64__ 1
 
 // NONE-NOT:#define __x86_64__
 // X86_64:#define __x86_64__ 1


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

[PATCH] D51312: [OpenMP][NVPTX] Use appropriate _CALL_ELF macro when offloading

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340772: [OpenMP][NVPTX] Use appropriate _CALL_ELF macro when 
offloading (authored by gbercea, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51312?vs=162708&id=162734#toc

Repository:
  rC Clang

https://reviews.llvm.org/D51312

Files:
  lib/Frontend/InitPreprocessor.cpp
  test/Preprocessor/aux-triple.c


Index: test/Preprocessor/aux-triple.c
===
--- test/Preprocessor/aux-triple.c
+++ test/Preprocessor/aux-triple.c
@@ -14,7 +14,7 @@
 // RUN: %clang_cc1 -x cuda -E -dM -ffreestanding < /dev/null \
 // RUN: -triple nvptx64-none-none -aux-triple 
powerpc64le-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
-// RUN: -check-prefixes NVPTX64,PPC64,LINUX,LINUX-CPP
+// RUN: -check-prefixes NVPTX64,PPC64LE,LINUX,LINUX-CPP
 // RUN: %clang_cc1 -x cuda -E -dM -ffreestanding < /dev/null \
 // RUN: -triple nvptx64-none-none -aux-triple x86_64-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
@@ -24,22 +24,24 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple powerpc64le-unknown-linux-gnu \
-// RUN:   | FileCheck -match-full-lines -check-prefixes NVPTX64,PPC64,LINUX %s
+// RUN:   | FileCheck -match-full-lines -check-prefixes NVPTX64,PPC64LE,LINUX 
%s
 // RUN: %clang_cc1 -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple x86_64-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines -check-prefixes NVPTX64,X86_64,LINUX %s
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple powerpc64le-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
-// RUN: -check-prefixes NVPTX64,PPC64,LINUX,LINUX-CPP
+// RUN: -check-prefixes NVPTX64,PPC64LE,LINUX,LINUX-CPP
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple x86_64-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
 // RUN: -check-prefixes NVPTX64,X86_64,LINUX,LINUX-CPP
 
+// PPC64LE:#define _CALL_ELF 2
+
 // NONE-NOT:#define _GNU_SOURCE
 // LINUX-CPP:#define _GNU_SOURCE 1
 
@@ -56,7 +58,7 @@
 // LINUX:#define __linux__ 1
 
 // NONE-NOT:#define __powerpc64__
-// PPC64:#define __powerpc64__ 1
+// PPC64LE:#define __powerpc64__ 1
 
 // NONE-NOT:#define __x86_64__
 // X86_64:#define __x86_64__ 1
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -1106,14 +1106,19 @@
   auto AuxTriple = AuxTI.getTriple();
 
   // Define basic target macros needed by at least bits/wordsize.h and
-  // bits/mathinline.h
+  // bits/mathinline.h.
+  // On PowerPC, explicitely set _CALL_ELF macro needed for gnu/stubs.h.
   switch (AuxTriple.getArch()) {
   case llvm::Triple::x86_64:
 Builder.defineMacro("__x86_64__");
 break;
   case llvm::Triple::ppc64:
+Builder.defineMacro("__powerpc64__");
+Builder.defineMacro("_CALL_ELF", "1");
+break;
   case llvm::Triple::ppc64le:
 Builder.defineMacro("__powerpc64__");
+Builder.defineMacro("_CALL_ELF", "2");
 break;
   default:
 break;


Index: test/Preprocessor/aux-triple.c
===
--- test/Preprocessor/aux-triple.c
+++ test/Preprocessor/aux-triple.c
@@ -14,7 +14,7 @@
 // RUN: %clang_cc1 -x cuda -E -dM -ffreestanding < /dev/null \
 // RUN: -triple nvptx64-none-none -aux-triple powerpc64le-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
-// RUN: -check-prefixes NVPTX64,PPC64,LINUX,LINUX-CPP
+// RUN: -check-prefixes NVPTX64,PPC64LE,LINUX,LINUX-CPP
 // RUN: %clang_cc1 -x cuda -E -dM -ffreestanding < /dev/null \
 // RUN: -triple nvptx64-none-none -aux-triple x86_64-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines %s \
@@ -24,22 +24,24 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple powerpc64le-unknown-linux-gnu \
-// RUN:   | FileCheck -match-full-lines -check-prefixes NVPTX64,PPC64,LINUX %s
+// RUN:   | FileCheck -match-full-lines -check-prefixes NVPTX64,PPC64LE,LINUX %s
 // RUN: %clang_cc1 -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-none-none \
 // RUN: -aux-triple x86_64-unknown-linux-gnu \
 // RUN:   | FileCheck -match-full-lines -check-prefixes NVPTX64,X86_64,LINUX %s
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding < /dev/null \
 // RUN: -fopenmp -fopenmp-is-device -triple nvptx64-n

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

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

Ping.


https://reviews.llvm.org/D50927



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

[PATCH] D51177: [DEBUGINFO] Add support for emission of the debug directives only.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: test/Driver/debug-options.c:259
 //
+// This tests asserts that "-glineinfo-only" "-g0" disables debug info.
+// GLIO_NO: "-cc1"

'lineinfo' seems like two words - the inconsistency with 'line-tables' seems 
like it'd be confusing.

But also I'm not sure line-table versus line-info is very differentiating. 
Maybe line-directives-only?


Repository:
  rC Clang

https://reviews.llvm.org/D51177



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

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

[Hamza Sood via Phabricator via cfe-commits]

hamzasood created this revision.
hamzasood added reviewers: rnk, hokein, sammccall.
Herald added a subscriber: cfe-commits.

This patch fixes the handling of clang-cl options in 
`InterpolatingCompilationDatabase`.
They were previously ignored completely, which led to a lot of bugs:

- Additional options were being added with the wrong syntax. E.g. a file was 
specified as C++ by adding `-x c++`, which causes an error in CL mode.
- The args were parsed and then rendered, which means that the aliasing 
information was lost. E.g. `/W3` was rendered to `-Wall`, which in CL mode 
means `-Weverything`.
- CL options were ignored when checking things like `-std=`, so a lot of logic 
was being bypassed.


Repository:
  rC Clang

https://reviews.llvm.org/D51321

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

Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -644,12 +644,15 @@
   }
 };
 
-class InterpolateTest : public ::testing::Test {
+class InterpolateTest : public ::testing::TestWithParam {
 protected:
+  bool isTestingCL() const { return GetParam(); }
+  StringRef getClangExe() const { return isTestingCL() ? "clang-cl" : "clang"; }
+
   // Adds an entry to the underlying compilation database.
   // A flag is injected: -D , so the command used can be identified.
   void add(llvm::StringRef File, llvm::StringRef Flags = "") {
-llvm::SmallVector Argv = {"clang", File, "-D", File};
+llvm::SmallVector Argv = {getClangExe(), File, "-D", File};
 llvm::SplitString(Flags, Argv);
 llvm::SmallString<32> Dir;
 llvm::sys::path::system_temp_directory(false, Dir);
@@ -675,67 +678,92 @@
 ->getCompileCommands(path(F));
 if (Results.empty())
   return "none";
-// drop the input file argument, so tests don't have to deal with path().
-EXPECT_EQ(Results[0].CommandLine.back(), path(F))
-<< "Last arg should be the file";
-Results[0].CommandLine.pop_back();
-return llvm::join(Results[0].CommandLine, " ");
+
+ArrayRef CmdLine = Results[0].CommandLine;
+
+// drop the clang path, so tests don't have to deal with getClangExe().
+EXPECT_EQ(CmdLine.front(), getClangExe()) << "First arg should be clang";
+CmdLine = CmdLine.drop_front();
+
+// Drop the input file argument, so tests don't have to deal with path().
+EXPECT_EQ(CmdLine.back(), path(F)) << "Last arg should be the file";
+CmdLine = CmdLine.drop_back();
+
+return llvm::join(CmdLine, " ");
   }
 
   MemCDB::EntryMap Entries;
 };
 
-TEST_F(InterpolateTest, Nearby) {
+TEST_P(InterpolateTest, Nearby) {
   add("dir/foo.cpp");
   add("dir/bar.cpp");
   add("an/other/foo.cpp");
 
   // great: dir and name both match (prefix or full, case insensitive)
-  EXPECT_EQ(getCommand("dir/f.cpp"), "clang -D dir/foo.cpp");
-  EXPECT_EQ(getCommand("dir/FOO.cpp"), "clang -D dir/foo.cpp");
+  EXPECT_EQ(getCommand("dir/f.cpp"), "-D dir/foo.cpp");
+  EXPECT_EQ(getCommand("dir/FOO.cpp"), "-D dir/foo.cpp");
   // no name match. prefer matching dir, break ties by alpha
-  EXPECT_EQ(getCommand("dir/a.cpp"), "clang -D dir/bar.cpp");
+  EXPECT_EQ(getCommand("dir/a.cpp"), "-D dir/bar.cpp");
   // an exact name match beats one segment of directory match
   EXPECT_EQ(getCommand("some/other/bar.h"),
-"clang -D dir/bar.cpp -x c++-header");
+isTestingCL()
+? "-D dir/bar.cpp /TP"
+: "-D dir/bar.cpp -x c++-header");
   // two segments of directory match beat a prefix name match
-  EXPECT_EQ(getCommand("an/other/b.cpp"), "clang -D an/other/foo.cpp");
+  EXPECT_EQ(getCommand("an/other/b.cpp"), "-D an/other/foo.cpp");
   // if nothing matches at all, we still get the closest alpha match
   EXPECT_EQ(getCommand("below/some/obscure/path.cpp"),
-"clang -D an/other/foo.cpp");
+"-D an/other/foo.cpp");
 }
 
-TEST_F(InterpolateTest, Language) {
-  add("dir/foo.cpp", "-std=c++17");
-  add("dir/baz.cee", "-x c");
+TEST_P(InterpolateTest, Language) {
+  add("dir/foo.cpp", isTestingCL() ? "/std:c++17" : "-std=c++17");
+  add("dir/baz.cee", isTestingCL() ? "/TC" : "-x c");
 
   // .h is ambiguous, so we add explicit language flags
   EXPECT_EQ(getCommand("foo.h"),
-"clang -D dir/foo.cpp -x c++-header -std=c++17");
+isTestingCL()
+? "-D dir/foo.cpp /TP /std:c++17"
+: "-D dir/foo.cpp -x c++-header -std=c++17");
   // and don't add -x if the inferred language is correct.
-  EXPECT_EQ(getCommand("foo.hpp"), "clang -D dir/foo.cpp -std=c++17");
+  EXPECT_EQ(getCommand("foo.hpp"),
+isTestingCL()
+? "-D dir/foo.cpp /std:c++17"
+: "-D dir/foo.cpp -std=c++17");
   // respect -x if it's already there.
-  EXPECT_EQ(getCommand("baz.h"), "clang -

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

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:182-184
+  if (Arg->getOption().matches(driver::options::OPT_driver_mode)) {
+// Process --driver-mode.
+IsCLMode = std::strcmp(Arg->getValue(), "cl") == 0;

I think you need to do this up front, otherwise you'll parse some args in 
non-cl mode and some in cl mode.


Repository:
  rC Clang

https://reviews.llvm.org/D51321



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

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

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

rsmith added a comment.

I would prefer that we use the location of the capture-default as the location 
of all the implicitly captured fields, rather than using an invalid location.




Comment at: clang/lib/Sema/SemaDecl.cpp:10689
 
-  S.DiagRuntimeBehavior(DRE->getBeginLoc(), DRE,
+  S.DiagRuntimeBehavior(DRE->getBeginOrDeclLoc(), DRE,
 S.PDiag(diag)

I'm a bit surprised you updated this call to `DeclRefExpr::getBeginLoc()` (and 
no others). I think this should be unreachable for a `DeclRefExpr` that refers 
to an implicit lambda capture, because such a lambda-capture cannot refer to 
itself from its own (implicit) initializer.


https://reviews.llvm.org/D50927



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

[libcxx] r340778 - Fix ODR violation: namespace-scope helpers should not be declared 'static'.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Aug 27 14:41:50 2018
New Revision: 340778

URL: http://llvm.org/viewvc/llvm-project?rev=340778&view=rev
Log:
Fix ODR violation: namespace-scope helpers should not be declared 'static'.

Modified:
libcxx/trunk/include/variant

Modified: libcxx/trunk/include/variant
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/variant?rev=340778&r1=340777&r2=340778&view=diff
==
--- libcxx/trunk/include/variant (original)
+++ libcxx/trunk/include/variant Mon Aug 27 14:41:50 2018
@@ -1320,7 +1320,7 @@ constexpr bool holds_alternative(const v
 
 template 
 inline _LIBCPP_INLINE_VISIBILITY
-static constexpr auto&& __generic_get(_Vp&& __v) {
+constexpr auto&& __generic_get(_Vp&& __v) {
   using __variant_detail::__access::__variant;
   if (!__holds_alternative<_Ip>(__v)) {
 __throw_bad_variant_access();


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

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

[Nick Desaulniers via Phabricator via cfe-commits]

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

- explicitly mention extern inline


Repository:
  rC Clang

https://reviews.llvm.org/D51190

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


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,20 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``extern inline``
+should follow the c89 convention that if the function cannot be inlined
+(possibly due to being referred to by function pointer), then no out of line
+definition will be emitted (instead of c99's behaviour of always emitting an
+out of line definition).
+
+If ``__GNUC_STDC_INLINE__`` is defined, then the current translation unit is
+not being compiled with ``gnu_inline`` semantics, and the ``gnu_inline``
+function attribute can be used to get c89 semantics on a per function basis.
+If ``__GNUC_STDC_INLINE__`` is not defined, then the translation unit is
+already being compiled with c89 semantics as the implied default.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,20 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``extern inline``
+should follow the c89 convention that if the function cannot be inlined
+(possibly due to being referred to by function pointer), then no out of line
+definition will be emitted (instead of c99's behaviour of always emitting an
+out of line definition).
+
+If ``__GNUC_STDC_INLINE__`` is defined, then the current translation unit is
+not being compiled with ``gnu_inline`` semantics, and the ``gnu_inline``
+function attribute can be used to get c89 semantics on a per function basis.
+If ``__GNUC_STDC_INLINE__`` is not defined, then the translation unit is
+already being compiled with c89 semantics as the implied default.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50855: [analyzer] pr37578: Fix lvalue/rvalue problem in field-of-temporary adjustments.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 162755.
NoQ added a comment.

Address comments.


https://reviews.llvm.org/D50855

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/temporaries.cpp

Index: test/Analysis/temporaries.cpp
===
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -1152,3 +1152,23 @@
 // and the non-definition decl should be found by direct lookup.
 void T::foo(C) {}
 } // namespace argument_virtual_decl_lookup
+
+namespace union_indirect_field_crash {
+union U {
+  struct {
+int x;
+  };
+};
+
+template  class C {
+public:
+  void foo() const {
+(void)(true ? U().x : 0);
+  }
+};
+
+void test() {
+  C c;
+  c.foo();
+}
+} // namespace union_indirect_field_crash
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -283,11 +283,10 @@
   return state;
 }
 
-ProgramStateRef
-ExprEngine::createTemporaryRegionIfNeeded(ProgramStateRef State,
-  const LocationContext *LC,
-  const Expr *InitWithAdjustments,
-  const Expr *Result) {
+ProgramStateRef ExprEngine::createTemporaryRegionIfNeeded(
+ProgramStateRef State, const LocationContext *LC,
+const Expr *InitWithAdjustments, const Expr *Result,
+const SubRegion **OutRegionWithAdjustments) {
   // FIXME: This function is a hack that works around the quirky AST
   // we're often having with respect to C++ temporaries. If only we modelled
   // the actual execution order of statements properly in the CFG,
@@ -297,8 +296,11 @@
   if (!Result) {
 // If we don't have an explicit result expression, we're in "if needed"
 // mode. Only create a region if the current value is a NonLoc.
-if (!InitValWithAdjustments.getAs())
+if (!InitValWithAdjustments.getAs()) {
+  if (OutRegionWithAdjustments)
+*OutRegionWithAdjustments = nullptr;
   return State;
+}
 Result = InitWithAdjustments;
   } else {
 // We need to create a region no matter what. For sanity, make sure we don't
@@ -418,11 +420,17 @@
   // The result expression would now point to the correct sub-region of the
   // newly created temporary region. Do this last in order to getSVal of Init
   // correctly in case (Result == Init).
-  State = State->BindExpr(Result, LC, Reg);
+  if (Result->isGLValue()) {
+State = State->BindExpr(Result, LC, Reg);
+  } else {
+State = State->BindExpr(Result, LC, InitValWithAdjustments);
+  }
 
   // Notify checkers once for two bindLoc()s.
   State = processRegionChange(State, TR, LC);
 
+  if (OutRegionWithAdjustments)
+*OutRegionWithAdjustments = cast(Reg.getAsRegion());
   return State;
 }
 
@@ -2532,8 +2540,12 @@
   }
 
   // Handle regular struct fields / member variables.
-  state = createTemporaryRegionIfNeeded(state, LCtx, BaseExpr);
-  SVal baseExprVal = state->getSVal(BaseExpr, LCtx);
+  const SubRegion *MR = nullptr;
+  state = createTemporaryRegionIfNeeded(state, LCtx, BaseExpr,
+/*Result=*/nullptr,
+/*OutRegionWithAdjustments=*/&MR);
+  SVal baseExprVal =
+  MR ? loc::MemRegionVal(MR) : state->getSVal(BaseExpr, LCtx);
 
   const auto *field = cast(Member);
   SVal L = state->getLValue(field, baseExprVal);
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -734,10 +734,14 @@
   ///
   /// If \p Result is provided, the new region will be bound to this expression
   /// instead of \p InitWithAdjustments.
-  ProgramStateRef createTemporaryRegionIfNeeded(ProgramStateRef State,
-const LocationContext *LC,
-const Expr *InitWithAdjustments,
-const Expr *Result = nullptr);
+  ///
+  /// Returns the temporary region with adjustments into the optional
+  /// OutRegionWithAdjustments out-parameter if a new region was indeed needed,
+  /// otherwise sets it to nullptr.
+  ProgramStateRef createTemporaryRegionIfNeeded(
+  ProgramStateRef State, const LocationContext *LC,
+  const Expr *InitWithAdjustments, const Expr *Result = nullptr,
+  const SubRegion **OutRegionWithAdjustments = nullptr);
 
   /// Returns a region representing the first element of a (possibly
   /// multi-dimensional) array, for the purposes of element construction or
_

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

[Eli Friedman via Phabricator via cfe-commits]

efriedma edited reviewers, added: efriedma, rsmith, thakis; removed: 
eli.friedman, krememek, ddunbar, lattner.
efriedma added a comment.

What is this change actually solving?  Even if the typedef is actually 
redundant, it shouldn't cause a compiler error: redundant typedefs are allowed 
in the latest versions of the C and C++ standards. (And even if you were 
targeting an earlier standard, we normally suppress warnings in system headers.)


Repository:
  rC Clang

https://reviews.llvm.org/D51265



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

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

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Thanks for taking the time to write documentation.

The phrase "C89 convention" is misleading; the original ISO C standard doesn't 
define the inline keyword at all.  Maybe something along the lines of "GNU 
inline extension".  And maybe mention it's the default with -std=gnu89.


Repository:
  rC Clang

https://reviews.llvm.org/D51190



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

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

[Nick Desaulniers via Phabricator via cfe-commits]

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

- s/c89/GNU inline extension/g and mention -std=gnu89/-fgnu89-inline


Repository:
  rC Clang

https://reviews.llvm.org/D51190

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


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,22 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``extern inline``
+should follow the GNU inline extension that if the function cannot be inlined
+(possibly due to being referred to by function pointer), then no out of line
+definition will be emitted (instead of c99's behaviour of always emitting an
+out of line definition).
+
+If ``__GNUC_STDC_INLINE__`` is defined, then the current translation unit is
+not being compiled with ``gnu_inline`` semantics, and the ``gnu_inline``
+function attribute can be used to get GNU inline semantics on a per function 
basis.
+If ``__GNUC_STDC_INLINE__`` is not defined, then the translation unit is
+already being compiled with GNU inline semantics as the implied default.
+
+This is the default behavior with ``-std=gnu89`` or ``-fgnu89-inline``.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,22 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``extern inline``
+should follow the GNU inline extension that if the function cannot be inlined
+(possibly due to being referred to by function pointer), then no out of line
+definition will be emitted (instead of c99's behaviour of always emitting an
+out of line definition).
+
+If ``__GNUC_STDC_INLINE__`` is defined, then the current translation unit is
+not being compiled with ``gnu_inline`` semantics, and the ``gnu_inline``
+function attribute can be used to get GNU inline semantics on a per function basis.
+If ``__GNUC_STDC_INLINE__`` is not defined, then the translation unit is
+already being compiled with GNU inline semantics as the implied default.
+
+This is the default behavior with ``-std=gnu89`` or ``-fgnu89-inline``.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Hamza Sood via Phabricator via cfe-commits]

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

Thanks, I've changed it so that the driver mode is calculated first.
I've also extended the unit test to include the various `--driver-mode` 
combinations.


https://reviews.llvm.org/D51321

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

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

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

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:10689
 
-  S.DiagRuntimeBehavior(DRE->getBeginLoc(), DRE,
+  S.DiagRuntimeBehavior(DRE->getBeginOrDeclLoc(), DRE,
 S.PDiag(diag)

rsmith wrote:
> I'm a bit surprised you updated this call to `DeclRefExpr::getBeginLoc()` 
> (and no others). I think this should be unreachable for a `DeclRefExpr` that 
> refers to an implicit lambda capture, because such a lambda-capture cannot 
> refer to itself from its own (implicit) initializer.
IIUC, that's exactly what this visitor (SelfReferenceChecker) diagnoses. The 
only test I found which exercises this code path is SemaCXX/uninitialized.cpp. 
The relevant portion is updated in this diff, but to summarize, there's a 
DeclRefExpr in the CXXConstructExpr initializer generated for `a1` which 
self-refers to `a1`.


https://reviews.llvm.org/D50927



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