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

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In https://reviews.llvm.org/D51279#1214268, @ilya-biryukov wrote:

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


Comments are always welcome :)




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).

ilya-biryukov wrote:
> 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.
Yes, I re-thought it afterwards. We could solve this issue for 
`findOccurrences` by redefining the interface.

Unfortunately, we have the same issue for `fuzzyFind`, `lookup` too -- we still 
return the stale symbols from static index if the symbol was deleted in dirty 
files. I think we should address this issue thoroughly and separately -- we 
probably want to add a method in dynamic index for query whether a file is in.


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] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager

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

baloghadamsoftware added a comment.

Of course we would like to try the Z3 for refutation, we do not dispute its 
usefulness. This patch is about something really different. It extends the 
range-based constraint manager in a very natural way. Is the code of the 
range-based constraint manager frozen for some reason? If not, then why not add 
multiplication and division as well, where addition and subtraction is already 
supported?


https://reviews.llvm.org/D49074



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

[clang-tools-extra] r340800 - [clang-tidy] Abseil: no namepsace check

[Haojian Wu via cfe-commits]

Author: hokein
Date: Tue Aug 28 00:48:28 2018
New Revision: 340800

URL: http://llvm.org/viewvc/llvm-project?rev=340800&view=rev
Log:
[clang-tidy] Abseil: no namepsace check

This check ensures that users of Abseil do not open namespace absl in their 
code, as that violates our compatibility guidelines.

AbseilMatcher.h written by Hugo Gonzalez.

Patch by Deanna Garcia!

Added:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h
clang-tools-extra/trunk/clang-tidy/abseil/NoNamespaceCheck.cpp
clang-tools-extra/trunk/clang-tidy/abseil/NoNamespaceCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-no-namespace.rst
clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/
clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/external-file.h
clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/strings/
clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/strings/internal-file.h
clang-tools-extra/trunk/test/clang-tidy/abseil-no-namespace.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Added: clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h?rev=340800&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h (added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h Tue Aug 28 
00:48:28 2018
@@ -0,0 +1,51 @@
+//===- AbseilMatcher.h - clang-tidy 
---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+namespace clang {
+namespace ast_matchers {
+
+/// Matches AST nodes that were found within Abseil files.
+///
+/// Example matches Y but not X
+/// (matcher = cxxRecordDecl(isInAbseilFile())
+/// \code
+///   #include "absl/strings/internal-file.h"
+///   class X {};
+/// \endcode
+/// absl/strings/internal-file.h:
+/// \code
+///   class Y {};
+/// \endcode
+///
+/// Usable as: Matcher, Matcher, Matcher,
+/// Matcher
+
+AST_POLYMORPHIC_MATCHER(isInAbseilFile,
+AST_POLYMORPHIC_SUPPORTED_TYPES(
+Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
+  auto &SourceManager = Finder->getASTContext().getSourceManager();
+  SourceLocation Loc = Node.getBeginLoc();
+  if (Loc.isInvalid())
+return false;
+  const FileEntry *FileEntry =
+  SourceManager.getFileEntryForID(SourceManager.getFileID(Loc));
+  if (!FileEntry)
+return false;
+  StringRef Filename = FileEntry->getName();
+  llvm::Regex RE(
+  "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|"
+  "synchronization|time|types|utility)");
+  return RE.match(Filename);
+}
+
+} // namespace ast_matchers
+} // namespace clang

Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=340800&r1=340799&r2=340800&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Tue Aug 28 
00:48:28 2018
@@ -12,6 +12,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "DurationDivisionCheck.h"
 #include "FasterStrsplitDelimiterCheck.h"
+#include "NoNamespaceCheck.h"
 #include "StringFindStartswithCheck.h"
 
 namespace clang {
@@ -25,6 +26,7 @@ public:
 "abseil-duration-division");
 CheckFactories.registerCheck(
 "abseil-faster-strsplit-delimiter");
+CheckFactories.registerCheck("abseil-no-namespace");
 CheckFactories.registerCheck(
 "abseil-string-find-startswith");
   }

Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=340800&r1=340799&r2=340800&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Tue Aug 28 
00:48:28 2018
@@ -4,6 +4,7 @@ add_clang_library(clangTidyAbseilModule
   AbseilTidyModule.cpp
   DurationDivisionCheck.cpp
   FasterStrsplitDelimiterCheck.cpp
+  NoNamespaceCheck.cpp
   StringFindStartswithCheck.cpp
 
   LINK_LIBS

Added: clang-tools-extra/trunk/clang-tidy/abseil/No

[PATCH] D50580: [clang-tidy] Abseil: no namespace check

[Haojian Wu via Phabricator via cfe-commits]

hokein closed this revision.
hokein added a comment.

I have committed the patch for you,  https://reviews.llvm.org/rL340800.




Comment at: test/clang-tidy/abseil-no-namespace.cpp:10
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:1:11: warning: namespace 'absl' is reserved
+// for implementation of the Abseil library and should not be opened in user

deannagarcia wrote:
> hokein wrote:
> > Does the test get passed on the first command `%check_clang_tidy %s 
> > abseil-no-namespace %t -- -- -I %S/Inputs`? The first command will suppress 
> > all warning in headers, I think it will fail?
> The test passes, and I'm pretty sure it's because this is a CHECK not a 
> CHECK-MESSAGES
Ah, I thought it was `CHECK-MESSAGES`, note that `CHECK` and `CHECK-MESSAGE` 
don't match multiple lines, the message should be one-line. I have fixed for 
you.


https://reviews.llvm.org/D50580



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

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

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Please rebase your patch, since the absl matcher patch is submitted. Thanks.




Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:8
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:1:23: warning: do not reference any 'internal' 
namespaces; those implementation details are reserved to Abseil 
[abseil-no-internal-deps]
+

hugoeg wrote:
> hokein wrote:
> > Does the test get passed on the first run `RUN: %check_clang_tidy %s 
> > abseil-no-internal-deps %t,  -- -- -I %S/Inputs` of the test? It will 
> > suppress clang-tidy warnings from the header, and the warning here should 
> > not appear.
> Yes, the test passes cleanly on both runs, I just re ran it a couple times to 
> make sure. 
Yeah, I misread it as `CHECK-MESSAGE`.


https://reviews.llvm.org/D50542



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

Re: [clang-tools-extra] r340800 - [clang-tidy] Abseil: no namepsace check

[Roman Lebedev via cfe-commits]

On Tue, Aug 28, 2018 at 10:48 AM, Haojian Wu via cfe-commits
 wrote:
> Author: hokein
> Date: Tue Aug 28 00:48:28 2018
> New Revision: 340800
>
> URL: http://llvm.org/viewvc/llvm-project?rev=340800&view=rev
> Log:
> [clang-tidy] Abseil: no namepsace check
>
> This check ensures that users of Abseil do not open namespace absl in their 
> code, as that violates our compatibility guidelines.
>
> AbseilMatcher.h written by Hugo Gonzalez.

Would it please be possible to use the common notation of adding a line

Differential Revision: https://reviews.llvm.org/D?

so that phabricator automatically properly associates the committed
revision with the differential, and closes it?

Roman.

> Patch by Deanna Garcia!
>
> Added:
> clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h
> clang-tools-extra/trunk/clang-tidy/abseil/NoNamespaceCheck.cpp
> clang-tools-extra/trunk/clang-tidy/abseil/NoNamespaceCheck.h
> clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-no-namespace.rst
> clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/
> clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/external-file.h
> clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/strings/
> 
> clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/strings/internal-file.h
> clang-tools-extra/trunk/test/clang-tidy/abseil-no-namespace.cpp
> Modified:
> clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
> clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
> clang-tools-extra/trunk/docs/ReleaseNotes.rst
> clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
>
> Added: clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h?rev=340800&view=auto
> ==
> --- clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h (added)
> +++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h Tue Aug 28 
> 00:48:28 2018
> @@ -0,0 +1,51 @@
> +//===- AbseilMatcher.h - clang-tidy 
> ---===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===--===//
> +//
> +#include "clang/AST/ASTContext.h"
> +#include "clang/ASTMatchers/ASTMatchFinder.h"
> +
> +namespace clang {
> +namespace ast_matchers {
> +
> +/// Matches AST nodes that were found within Abseil files.
> +///
> +/// Example matches Y but not X
> +/// (matcher = cxxRecordDecl(isInAbseilFile())
> +/// \code
> +///   #include "absl/strings/internal-file.h"
> +///   class X {};
> +/// \endcode
> +/// absl/strings/internal-file.h:
> +/// \code
> +///   class Y {};
> +/// \endcode
> +///
> +/// Usable as: Matcher, Matcher, Matcher,
> +/// Matcher
> +
> +AST_POLYMORPHIC_MATCHER(isInAbseilFile,
> +AST_POLYMORPHIC_SUPPORTED_TYPES(
> +Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
> +  auto &SourceManager = Finder->getASTContext().getSourceManager();
> +  SourceLocation Loc = Node.getBeginLoc();
> +  if (Loc.isInvalid())
> +return false;
> +  const FileEntry *FileEntry =
> +  SourceManager.getFileEntryForID(SourceManager.getFileID(Loc));
> +  if (!FileEntry)
> +return false;
> +  StringRef Filename = FileEntry->getName();
> +  llvm::Regex RE(
> +  "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|"
> +  "synchronization|time|types|utility)");
> +  return RE.match(Filename);
> +}
> +
> +} // namespace ast_matchers
> +} // namespace clang
>
> Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=340800&r1=340799&r2=340800&view=diff
> ==
> --- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original)
> +++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Tue Aug 28 
> 00:48:28 2018
> @@ -12,6 +12,7 @@
>  #include "../ClangTidyModuleRegistry.h"
>  #include "DurationDivisionCheck.h"
>  #include "FasterStrsplitDelimiterCheck.h"
> +#include "NoNamespaceCheck.h"
>  #include "StringFindStartswithCheck.h"
>
>  namespace clang {
> @@ -25,6 +26,7 @@ public:
>  "abseil-duration-division");
>  CheckFactories.registerCheck(
>  "abseil-faster-strsplit-delimiter");
> +CheckFactories.registerCheck("abseil-no-namespace");
>  CheckFactories.registerCheck(
>  "abseil-string-find-startswith");
>}
>
> Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.

[PATCH] D51332: [clang-tidy] Replace deprecated std::ios_base aliases

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Hi andobence,

thank you for the contribution. The check looks very good!
Please add it to the release notes and synchronize the first line of the doc 
with the short sentence in the release notes describing what this check does.




Comment at: clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.h:20
+/// This check warns the uses of the deprecated member types of 
``std::ios_base``
+/// and replaces those that have a non - deprecated equivalent.
+///

I I think you can write `non-deprecated` without the whitespace.



Comment at: test/clang-tidy/modernize-deprecated-ios-base-aliases.cpp:14
+
+template <
+class CharT>

That format looks weird. Could you please run clang-format over the test as 
well?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51332



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

[PATCH] D51332: [clang-tidy] Replace deprecated std::ios_base aliases

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp:34
+  // provide any benefit to other languages, despite being benign.
+  if (!getLangOpts().CPlusPlus)
+return;

Which standard supplies the replacement functionality? e.g. for C++98 the 
warning is probably not relevant because there is no way to replace the 
deprecated types, but for C++17 it is possible.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51332



___
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

kbobyrev wrote:
> 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.
For that we would need up-to-date cross references


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] 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:124
+// A CompileCommand that can be applied to another file. Any instance of this
+// object is invalid after std::move() from it.
 struct TransferableCommand {

jfb wrote:
> 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.
We specifically assert that object cannot be called after `move()` (check the 
unique_ptr that stores our `once_flag`). It's definitely undefined behavior to 
call any methods, because they will immediately dereference a null pointer (the 
aforementioned unique_ptr).

Happy to drop the comment, though, we do have asserts for that.



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

jfb wrote:
> The `once_flag` should just be a static, don't allocate it.
Sorry, I don't seem to follow. We need one `once_flag` per `TransferableCommand`


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] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:24
+  auto &SourceManager = Finder->getASTContext().getSourceManager();
+  SourceLocation loc = Node.getBeginLoc();
+  if (loc.isInvalid())

I think @hokein's comment wasn't addressed despite being checked: `loc` should 
become `Loc` to comply with the LLVM naming rules.



Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:32
+  StringRef Filename = FileEntry->getName();
+  llvm::Regex RE(
+  "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|"

Nit: maybe replace with `Filename.startswith("absl/")` then use `std::any_of` 
on `Filename.drop_front("absl/".size())`?


https://reviews.llvm.org/D50542



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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks for finding this problem, this fix *mostly* looks good (though I think 
we can probably drop memoization).

I'm a bit uncomfortable about the places where we need the type, because this 
is the only thing forcing us to parse before we've picked a command to 
transfer, and the number of commands we need to parse is data-dependent and 
hard to reason about.

Let me think about this a little - I suspect slightly more invasive changes 
(change the concept of type, tweak the heuristics, or do a "lightweight parse" 
to get the type) might make this cleaner and performance more predictable.




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

ilya-biryukov wrote:
> 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?
> 
So filtering out this has a couple of effects
 - it's a performance optimization (don't bother indexing filenames for useless 
files). We don't need this
 - it prevents a TY_INVALID command being chosen for transfer. I'm not sure 
whether this would occur often enough to be a problem in practice.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:133
+assert(TraitsComputed && "calling transferTo on moved-from object");
+const CommandTraits &T = getTraits();
+CompileCommand Result = T.Cmd;

I think you're overthinking things with the memoization here (of course I say 
this as the person who underthought it!)

AIUI, the problem is that *eagerly* parsing all the compile commands takes 3x 
as long as reading them, which hurts startup time with big 
`compile_commands.json`.

But I think we can afford to just parse them when `transferTo` is called, 
without memoization. (Remember we only hit this code path when querying a file 
*not in the CDB*, so it should never get called in a tight loop).

The benefit of slightly reducing the latency of`getCompileCommand` for unknown 
files when we happen to pick the same template file for the second time... it's 
unclear to me, and the code would be easier to follow without it.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:159
+  llvm::StringRef filename() const {
+assert(TraitsComputed && "calling filename on moved-from object");
+return OriginalCmd.Filename;

Is this so important to dynamically check? Most types don't.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:171
+  // compute, so we only compute on first access.
+  struct CommandTraits {
+// Flags that should not apply to all files are stripped from CommandLine.

Traits is a bit vague, and a bit template-nightmare-y!
maybe ParsedCommand?




Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:383
   S.Preferred = PreferredLanguage == types::TY_INVALID ||
-PreferredLanguage == Commands[S.Index].Type;
+PreferredLanguage == Commands[S.Index].type();
   S.Points = Candidate.second;

I think this is going to force parsing of all candidates that get any points at 
all, with a flat directory structure this could be quite a lot :-(



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:383
   S.Preferred = PreferredLanguage == types::TY_INVALID ||
-PreferredLanguage == Commands[S.Index].Type;
+PreferredLanguage == Commands[S.Index].type();
   S.Points = Candidate.second;

sammccall wrote:
> I think this is going to force parsing of all candidates that get any points 
> at all, with a flat directory structure this could be quite a lot :-(
ah, now I see that the memoization also allows us to pretend that this is an 
eagerly computed value, without considering exactly when it's computed.

I'm not sure I like this if we do consider it performance sensitive - it 
obfuscates exactly which set of commands we parse, it would be nice if we were 
upfront about this.


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

[Sam McCall via Phabricator via cfe-commits]

sammccall 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 {

ilya-biryukov wrote:
> jfb wrote:
> > 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.
> We specifically assert that object cannot be called after `move()` (check the 
> unique_ptr that stores our `once_flag`). It's definitely undefined behavior 
> to call any methods, because they will immediately dereference a null pointer 
> (the aforementioned unique_ptr).
> 
> Happy to drop the comment, though, we do have asserts for that.
I think the idea is "this comment just reiterates standard C++ semantics".

(FWIW I find the asserts hurt readability - it's unusual to try to detect this 
condition, and TraitsComputed is easily mistaken for a boolean)


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

r340805 - [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments

[Adam Balogh via cfe-commits]

Author: baloghadamsoftware
Date: Tue Aug 28 01:41:15 2018
New Revision: 340805

URL: http://llvm.org/viewvc/llvm-project?rev=340805&view=rev
Log:
[Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) 
assignments

We add check for invalidation of iterators. The only operation we handle here
is the (copy) assignment.

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


Added:
cfe/trunk/test/Analysis/invalidated-iterator.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h
cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=340805&r1=340804&r2=340805&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Tue Aug 28 
01:41:15 2018
@@ -313,6 +313,10 @@ def IteratorRangeChecker : Checker<"Iter
   HelpText<"Check for iterators used outside their valid ranges">,
   DescFile<"IteratorChecker.cpp">;
 
+def InvalidatedIteratorChecker : Checker<"InvalidatedIterator">,
+  HelpText<"Check for use of invalidated iterators">,
+  DescFile<"IteratorChecker.cpp">;
+
 def MisusedMovedObjectChecker: Checker<"MisusedMovedObject">,
  HelpText<"Method calls on a moved-from object and copying a moved-from "
   "object will be reported">,

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp?rev=340805&r1=340804&r2=340805&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp Tue Aug 28 
01:41:15 2018
@@ -67,11 +67,14 @@
 // a constraint which we later retrieve when doing an actual comparison.
 
 #include "ClangSACheckers.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 
+#include 
+
 using namespace clang;
 using namespace ento;
 
@@ -85,34 +88,43 @@ private:
   // Container the iterator belongs to
   const MemRegion *Cont;
 
+  // Whether iterator is valid
+  const bool Valid;
+
   // Abstract offset
   const SymbolRef Offset;
 
-  IteratorPosition(const MemRegion *C, SymbolRef Of)
-  : Cont(C), Offset(Of) {}
+  IteratorPosition(const MemRegion *C, bool V, SymbolRef Of)
+  : Cont(C), Valid(V), Offset(Of) {}
 
 public:
   const MemRegion *getContainer() const { return Cont; }
+  bool isValid() const { return Valid; }
   SymbolRef getOffset() const { return Offset; }
 
+  IteratorPosition invalidate() const {
+return IteratorPosition(Cont, false, Offset);
+  }
+
   static IteratorPosition getPosition(const MemRegion *C, SymbolRef Of) {
-return IteratorPosition(C, Of);
+return IteratorPosition(C, true, Of);
   }
 
   IteratorPosition setTo(SymbolRef NewOf) const {
-return IteratorPosition(Cont, NewOf);
+return IteratorPosition(Cont, Valid, NewOf);
   }
 
   bool operator==(const IteratorPosition &X) const {
-return Cont == X.Cont && Offset == X.Offset;
+return Cont == X.Cont && Valid == X.Valid && Offset == X.Offset;
   }
 
   bool operator!=(const IteratorPosition &X) const {
-return Cont != X.Cont || Offset != X.Offset;
+return Cont != X.Cont || Valid != X.Valid || Offset != X.Offset;
   }
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
 ID.AddPointer(Cont);
+ID.AddInteger(Valid);
 ID.Add(Offset);
   }
 };
@@ -181,15 +193,16 @@ public:
 
 class IteratorChecker
 : public Checker,
  check::PostStmt,
  check::LiveSymbols, check::DeadSymbols,
  eval::Assume> {
 
   std::unique_ptr OutOfRangeBugType;
+  std::unique_ptr InvalidatedBugType;
 
   void handleComparison(CheckerContext &C, const SVal &RetVal, const SVal 
&LVal,
 const SVal &RVal, OverloadedOperatorKind Op) const;
+  void verifyAccess(CheckerContext &C, const SVal &Val) const;
   void verifyDereference(CheckerContext &C, const SVal &Val) const;
   void handleIncrement(CheckerContext &C, const SVal &RetVal, const SVal &Iter,
bool Postfix) const;
@@ -204,17 +217,21 @@ class IteratorChecker
  const SVal &Cont) const;
   void assignToContainer(CheckerContext &C, const Expr *CE, const SVal &RetVal,
  const MemReg

[PATCH] D32747: [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340805: [Analyzer] Iterator Checker - Part 3: Invalidation 
check, first for (copy)… (authored by baloghadamsoftware, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D32747

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/diagnostics/explicit-suppression.cpp
  test/Analysis/invalidated-iterator.cpp

Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -313,6 +313,10 @@
   HelpText<"Check for iterators used outside their valid ranges">,
   DescFile<"IteratorChecker.cpp">;
 
+def InvalidatedIteratorChecker : Checker<"InvalidatedIterator">,
+  HelpText<"Check for use of invalidated iterators">,
+  DescFile<"IteratorChecker.cpp">;
+
 def MisusedMovedObjectChecker: Checker<"MisusedMovedObject">,
  HelpText<"Method calls on a moved-from object and copying a moved-from "
   "object will be reported">,
Index: test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- test/Analysis/Inputs/system-header-simulator-cxx.h
+++ test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -252,6 +252,15 @@
   return size_t(_finish - _start);
 }
 
+vector& operator=(const vector &other);
+vector& operator=(vector &&other);
+vector& operator=(std::initializer_list ilist);
+
+void assign(size_type count, const T &value);
+template 
+void assign(InputIterator first, InputIterator last);
+void assign(std::initializer_list ilist);
+
 void clear();
 
 void push_back(const T &value);
@@ -301,8 +310,21 @@
 list& operator=(list &&other);
 list& operator=(std::initializer_list ilist);
 
+void assign(size_type count, const T &value);
+template 
+void assign(InputIterator first, InputIterator last);
+void assign(std::initializer_list ilist);
+
 void clear();
 
+void push_back(const T &value);
+void push_back(T &&value);
+void pop_back();
+
+void push_front(const T &value);
+void push_front(T &&value);
+void pop_front();
+
 iterator begin() { return iterator(_start); }
 const_iterator begin() const { return const_iterator(_start); }
 const_iterator cbegin() const { return const_iterator(_start); }
@@ -338,6 +360,15 @@
   return size_t(_finish - _start);
 }
 
+deque& operator=(const deque &other);
+deque& operator=(deque &&other);
+deque& operator=(std::initializer_list ilist);
+
+void assign(size_type count, const T &value);
+template 
+void assign(InputIterator first, InputIterator last);
+void assign(std::initializer_list ilist);
+
 void clear();
 
 void push_back(const T &value);
@@ -351,11 +382,11 @@
 T &operator[](size_t n) {
   return _start[n];
 }
-
+
 const T &operator[](size_t n) const {
   return _start[n];
 }
-
+
 iterator begin() { return iterator(_start); }
 const_iterator begin() const { return const_iterator(_start); }
 const_iterator cbegin() const { return const_iterator(_start); }
@@ -367,7 +398,7 @@
 T& back() { return *(end() - 1); }
 const T& back() const { return *(end() - 1); }
   };
-  
+
   template
   class forward_list {
 struct __item {
@@ -387,6 +418,15 @@
 forward_list(forward_list &&other);
 ~forward_list();
 
+forward_list& operator=(const forward_list &other);
+forward_list& operator=(forward_list &&other);
+forward_list& operator=(std::initializer_list ilist);
+
+void assign(size_type count, const T &value);
+template 
+void assign(InputIterator first, InputIterator last);
+void assign(std::initializer_list ilist);
+
 void clear();
 
 void push_front(const T &value);
Index: test/Analysis/diagnostics/explicit-suppression.cpp
===
--- test/Analysis/diagnostics/explicit-suppression.cpp
+++ test/Analysis/diagnostics/explicit-suppression.cpp
@@ -19,6 +19,6 @@
 void testCopyNull(C *I, C *E) {
   std::copy(I, E, (C *)0);
 #ifndef SUPPRESSED
-  // expected-warning@../Inputs/system-header-simulator-cxx.h:514 {{Called C++ object pointer is null}}
+  // expected-warning@../Inputs/system-header-simulator-cxx.h:554 {{Called C++ object pointer is null}}
 #endif
 }
Index: test/Analysis/invalidated-iterator.cpp
===
--- test/Analysis/invalidated-iterator.cpp
+++ test/Analysis/invalidated-iterator.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-eagerly-assume -ana

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

[Hamza Sood via Phabricator via cfe-commits]

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



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:136
+  // Otherwise just check the clang file name.
+  return llvm::sys::path::stem(CmdLine.front()).endswith_lower("clang-cl");
+}

rnk wrote:
> We support being called "CL.exe", but with our new VS integration, I don't 
> think it matters to check for this case anymore. We may remove it over time.
Should I add the check anyway?



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:140
-unsigned MissingI, MissingC;
-auto ArgList = OptTable->ParseArgs(Argv, MissingI, MissingC);
-for (const auto *Arg : ArgList) {

rnk wrote:
> Can you not keep using ParseArgs? It takes FlagsToInclude and FlagsToExclude, 
> which you can compute outside the loop now.
Good point, I forgot to move the flags out of the loop when moving the 
—driver-mode check.

But I don’t think ParseArgs is suitable (see the other comment response).



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

rnk wrote:
> I think the old for loop was nicer. I don't think this code needs to change, 
> you should be able to use ParseArgs with the extra flag args.
The problem with the old loop is that we lose aliasing information (e.g. `/W3` 
becomes `-Wall`). While `ParseOneArg` also performs alias conversion, it gives 
us indices for each individual argument. The last line of the new loop copies 
arguments by using that index information to read from `OldArgs`, which gives 
us the original spelling.


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] D51341: [HEADER] Overloadable function candidates for half/double types

[Dmitry Sidorov via Phabricator via cfe-commits]

sidorovd created this revision.
sidorovd added reviewers: yaxunl, Anastasia.
Herald added a subscriber: cfe-commits.

Overloadable function candidate should not be viable if it uses extensions 
(Half or Double type) that are not enabled or supported.


Repository:
  rC Clang

https://reviews.llvm.org/D51341

Files:
  include/clang/AST/Type.h
  lib/AST/Type.cpp
  lib/Sema/SemaOverload.cpp
  test/SemaOpenCL/intel-half-double-overload.cl


Index: test/SemaOpenCL/intel-half-double-overload.cl
===
--- /dev/null
+++ test/SemaOpenCL/intel-half-double-overload.cl
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL1.2
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+float __attribute__((overloadable)) foo(float in1, float in2);
+float __attribute__((overloadable)) foo(half in1, half in2);
+
+int __attribute__((overloadable)) goo(float in1, float in2);
+half __attribute__((overloadable)) goo(double in1, double in2);
+#pragma OPENCL EXTENSION cl_khr_fp16 : disable
+
+__kernel void vi(int x, int y) {
+  foo(x, y);
+  goo(x, y);
+}
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+float __attribute__((overloadable)) foo_err(half in1, half in2);
+// expected-note@-1 {{candidate disabled due to OpenCL extension}}
+float __attribute__((overloadable)) foo_err(half in1, int in2);
+// expected-note@-1 {{candidate disabled due to OpenCL extension}}
+#pragma OPENCL EXTENSION cl_khr_fp16 : disable
+
+__kernel void vi_err(int x, int y) {
+  foo_err(x, y);
+  // expected-error@-1 {{no matching function for call to 'foo_err'}}
+}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -6059,6 +6059,33 @@
 return;
   }
 
+  // OpenCL: A candidate function that uses extentions that are not enabled or
+  // supported is not viable.
+  if (getLangOpts().OpenCL) {
+bool HasHalf =
+  getOpenCLOptions().isSupported("cl_khr_fp16", 
getLangOpts().OpenCLVersion)
+  && getOpenCLOptions().isEnabled("cl_khr_fp16");
+bool HasDouble =
+  getOpenCLOptions().isSupported("cl_khr_fp64", 
getLangOpts().OpenCLVersion)
+  && getOpenCLOptions().isEnabled("cl_khr_fp64");
+
+if ((!HasHalf && Function->getReturnType()->isHalfType()) ||
+(!HasDouble && Function->getReturnType()->isDoubleType())) {
+  Candidate.Viable = false;
+  Candidate.FailureKind = ovl_fail_ext_disabled;
+  return;
+}
+for (const auto *PI : Function->parameters()) {
+  QualType PTy = PI->getType();
+  if ((!HasHalf && PTy->isHalfType()) ||
+  (!HasDouble && PTy->isDoubleType())) {
+Candidate.Viable = false;
+Candidate.FailureKind = ovl_fail_ext_disabled;
+return;
+  }
+}
+  }
+
   // (CUDA B.1): Check for invalid calls between targets.
   if (getLangOpts().CUDA)
 if (const FunctionDecl *Caller = dyn_cast(CurContext))
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -1919,6 +1919,13 @@
   return false;
 }
 
+bool Type::isDoubleType() const {
+  if (const BuiltinType *BT = dyn_cast(CanonicalType))
+return BT->getKind() >= BuiltinType::Double &&
+  BT->getKind() <= BuiltinType::LongDouble;
+  return false;
+}
+
 bool Type::hasFloatingRepresentation() const {
   if (const auto *VT = dyn_cast(CanonicalType))
 return VT->getElementType()->isFloatingType();
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1924,6 +1924,7 @@
   bool isAggregateType() const;
   bool isFundamentalType() const;
   bool isCompoundType() const;
+  bool isDoubleType() const;   // (double + long double)
 
   // Type Predicates: Check to see if this type is structurally the specified
   // type, ignoring typedefs and qualifiers.


Index: test/SemaOpenCL/intel-half-double-overload.cl
===
--- /dev/null
+++ test/SemaOpenCL/intel-half-double-overload.cl
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL1.2
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+float __attribute__((overloadable)) foo(float in1, float in2);
+float __attribute__((overloadable)) foo(half in1, half in2);
+
+int __attribute__((overloadable)) goo(float in1, float in2);
+half __attribute__((overloadable)) goo(double in1, double in2);
+#pragma OPENCL EXTENSION cl_khr_fp16 : disable
+
+__kernel void vi(int x, int y) {
+  foo(x, y);
+  goo(x, y);
+}
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+float __attribute__((overloadable)) foo_err(half in1, half in2);
+// expected-note@-1 {{candidate disabled due to OpenCL extension}}
+float __attribute__((overloadable)) foo_err(half in1, int in2);
+// expected-note@-1 {{candidate disabled due t

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D51314#1215381, @sammccall wrote:

> I'm a bit uncomfortable about the places where we need the type, because this 
> is the only thing forcing us to parse before we've picked a command to 
> transfer, and the number of commands we need to parse is data-dependent and 
> hard to reason about.
>
> Let me think about this a little - I suspect slightly more invasive changes 
> (change the concept of type, tweak the heuristics, or do a "lightweight 
> parse" to get the type) might make this cleaner and performance more 
> predictable.


Having studied the code a bit - we use the parsed type to evaluate candidates, 
but we're comparing against the type extracted from the query filename (we have 
nothing else!).
So we should be fine just to compare extensions instead here. So either 
TransferableCommand would have an Extension field or a TypeFromFilename field - 
but we should be careful not to conflate the type inferred from the filename 
(fine for *selecting* a command) with the one parsed from the command (needed 
to *transfer* the command).

Then we have no need to parse for selection, and `getCompileCommands()` only 
needs to parse a single command from the underlying CDB, which should yield 
very predictable/consistent performance.
(It also couples nicely with the idea of only grabbing the full command from 
the underlying CDB lazily - we'll only do that once, too).




Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:480-481
 std::vector Result;
-for (auto Command : Inner->getAllCompileCommands()) {
+for (auto Command : Inner->getAllCompileCommands())
   Result.emplace_back(std::move(Command));
 return Result;

as you pointed out offline, we actually only need the filename for indexing, so 
we could ask the underlying DB for the filenames and get their commands on 
demand.

(we need to verify that the values returned don't differ from the filenames 
stored in CompileCommand, e.g. by filename normalization, in a way that matters)


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] D51297: [docs] Create a guide for Vim users on how to setup Clangd

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 162808.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Complete two last paragraphs, address few comments.

We haven't figured out whether we should suggest using this particular workflow 
at this point, but we're discussing the options.


https://reviews.llvm.org/D51297

Files:
  clang-tools-extra/docs/clangd-vim.rst
  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
@@ -108,6 +108,14 @@
 | Gen. Getters/Setters| No |   No |
 +-++--+
 
+
+Vim Integration
+===
+
+For more information on how to setup :program:`Clangd` for :program:`vim`,
+please see `Clangd Vim integration page
+`_.
+
 Getting Involved
 ==
 
Index: clang-tools-extra/docs/clangd-vim.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clangd-vim.rst
@@ -0,0 +1,209 @@
+==
+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.
+
+Configuring `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 would like to
+rename a symbol is not the best experience. Suggested way of a consistent setup
+with the ` key
+`_:
+
+.. code-block:: vim
+
+  function SetLSPShortcuts()
+nnoremap ld :call LanguageClient#textDocument_definition()
+nnoremap lr :call LanguageClient#textDocument_rename()
+nnor

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

[Simon Atanasyan via Phabricator via cfe-commits]

atanasyan added a comment.

In https://reviews.llvm.org/D51234#1213813, @ruiu wrote:

> 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?


The `tools::gnutools::Linker::ConstructJob` method contains code which adds 
`-flavor old-gnu` command line option if linker name is "lld". Probably this 
code can be removed now.


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

[PATCH] D51341: [HEADER] Overloadable function candidates for half/double types

[Dmitry Sidorov via Phabricator via cfe-commits]

sidorovd updated this revision to Diff 162812.

https://reviews.llvm.org/D51341

Files:
  include/clang/AST/Type.h
  lib/AST/Type.cpp
  lib/Sema/SemaOverload.cpp
  test/SemaOpenCL/half-double-overload.cl


Index: test/SemaOpenCL/half-double-overload.cl
===
--- /dev/null
+++ test/SemaOpenCL/half-double-overload.cl
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL1.2
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+float __attribute__((overloadable)) foo(float in1, float in2);
+float __attribute__((overloadable)) foo(half in1, half in2);
+
+int __attribute__((overloadable)) goo(float in1, float in2);
+half __attribute__((overloadable)) goo(double in1, double in2);
+#pragma OPENCL EXTENSION cl_khr_fp16 : disable
+
+__kernel void vi(int x, int y) {
+  foo(x, y);
+  goo(x, y);
+}
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+float __attribute__((overloadable)) foo_err(half in1, half in2);
+// expected-note@-1 {{candidate disabled due to OpenCL extension}}
+float __attribute__((overloadable)) foo_err(half in1, int in2);
+// expected-note@-1 {{candidate disabled due to OpenCL extension}}
+#pragma OPENCL EXTENSION cl_khr_fp16 : disable
+
+__kernel void vi_err(int x, int y) {
+  foo_err(x, y);
+  // expected-error@-1 {{no matching function for call to 'foo_err'}}
+}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -6059,6 +6059,33 @@
 return;
   }
 
+  // OpenCL: A candidate function that uses extentions that are not enabled or
+  // supported is not viable.
+  if (getLangOpts().OpenCL) {
+bool HasHalf =
+  getOpenCLOptions().isSupported("cl_khr_fp16", 
getLangOpts().OpenCLVersion)
+  && getOpenCLOptions().isEnabled("cl_khr_fp16");
+bool HasDouble =
+  getOpenCLOptions().isSupported("cl_khr_fp64", 
getLangOpts().OpenCLVersion)
+  && getOpenCLOptions().isEnabled("cl_khr_fp64");
+
+if ((!HasHalf && Function->getReturnType()->isHalfType()) ||
+(!HasDouble && Function->getReturnType()->isDoubleType())) {
+  Candidate.Viable = false;
+  Candidate.FailureKind = ovl_fail_ext_disabled;
+  return;
+}
+for (const auto *PI : Function->parameters()) {
+  QualType PTy = PI->getType();
+  if ((!HasHalf && PTy->isHalfType()) ||
+  (!HasDouble && PTy->isDoubleType())) {
+Candidate.Viable = false;
+Candidate.FailureKind = ovl_fail_ext_disabled;
+return;
+  }
+}
+  }
+
   // (CUDA B.1): Check for invalid calls between targets.
   if (getLangOpts().CUDA)
 if (const FunctionDecl *Caller = dyn_cast(CurContext))
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -1919,6 +1919,13 @@
   return false;
 }
 
+bool Type::isDoubleType() const {
+  if (const BuiltinType *BT = dyn_cast(CanonicalType))
+return BT->getKind() >= BuiltinType::Double &&
+  BT->getKind() <= BuiltinType::LongDouble;
+  return false;
+}
+
 bool Type::hasFloatingRepresentation() const {
   if (const auto *VT = dyn_cast(CanonicalType))
 return VT->getElementType()->isFloatingType();
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1924,6 +1924,7 @@
   bool isAggregateType() const;
   bool isFundamentalType() const;
   bool isCompoundType() const;
+  bool isDoubleType() const;   // (double + long double)
 
   // Type Predicates: Check to see if this type is structurally the specified
   // type, ignoring typedefs and qualifiers.


Index: test/SemaOpenCL/half-double-overload.cl
===
--- /dev/null
+++ test/SemaOpenCL/half-double-overload.cl
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL1.2
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+float __attribute__((overloadable)) foo(float in1, float in2);
+float __attribute__((overloadable)) foo(half in1, half in2);
+
+int __attribute__((overloadable)) goo(float in1, float in2);
+half __attribute__((overloadable)) goo(double in1, double in2);
+#pragma OPENCL EXTENSION cl_khr_fp16 : disable
+
+__kernel void vi(int x, int y) {
+  foo(x, y);
+  goo(x, y);
+}
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+float __attribute__((overloadable)) foo_err(half in1, half in2);
+// expected-note@-1 {{candidate disabled due to OpenCL extension}}
+float __attribute__((overloadable)) foo_err(half in1, int in2);
+// expected-note@-1 {{candidate disabled due to OpenCL extension}}
+#pragma OPENCL EXTENSION cl_khr_fp16 : disable
+
+__kernel void vi_err(int x, int y) {
+  foo_err(x, y);
+  // expected-error@-1 {{no matching function for call to 'foo_err'}}
+}
Index: lib/Sema/SemaOverload.cpp
=

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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D51279#1214268, @ilya-biryukov wrote:

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


fixed :-)




Comment at: clangd/index/FileIndex.cpp:58
 
-  return Collector.takeSymbols();
+  vlog("index for AST: ");
+  auto Syms = Collector.takeSymbols();

combine into one log statement



Comment at: clangd/index/FileIndex.h:50
 
+  std::vector> allOccurrenceSlabs() 
const;
+

This return type seems inconsistent with the design of this class.
The purpose of the `shared_ptr>` in `allSymbols` is to 
avoid exposing the concrete storage.
The analogue would I guess be `shared_ptr>>`.

The cost of creating that does seem a little gross though.



Comment at: clangd/index/FileIndex.h:50
 
+  std::vector> allOccurrenceSlabs() 
const;
+

sammccall wrote:
> This return type seems inconsistent with the design of this class.
> The purpose of the `shared_ptr>` in `allSymbols` is to 
> avoid exposing the concrete storage.
> The analogue would I guess be `shared_ptr vector>>`.
> 
> The cost of creating that does seem a little gross though.
allOccurrences
(name should reflect the semantics, not the type)



Comment at: clangd/index/FileIndex.h:57
   llvm::StringMap> FileToSlabs;
+  /// \breif Stores the latest occurrence slabs for all active files.
+  llvm::StringMap> FileToOccurrenceSlabs;

nit: no need for brief unless the "first sentence" heuristic fails. (also, it's 
misspelled!)





Comment at: clangd/index/FileIndex.h:95
 
 /// Retrieves namespace and class level symbols in \p AST.
 /// Exposed to assist in unit tests.

update doc



Comment at: clangd/index/MemIndex.cpp:36
   auto Idx = llvm::make_unique();
-  Idx->build(getSymbolsFromSlab(std::move(Slab)));
+  Idx->build(getSymbolsFromSlab(std::move(Slab)), {});
   return std::move(Idx);

we should find a way to avoid having functions implicitly zero out parts of the 
index like this!



Comment at: clangd/index/MemIndex.cpp:86
 const OccurrencesRequest &Req,
 llvm::function_ref Callback) const {
+  for (const auto &Slab : OccurrenceSlabs) {

This assumes there is no duplication across occurrences, but gives no mechanism 
through which deduplication could occur (because we're coupled to the 
underlying storage).

If FileIndex provides the DenseMap> then it 
could just refrain from providing duplicate pointers.



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

ilya-biryukov wrote:
> Maybe remove the default parameter? Making the callers specify the 
> occurrences explicitly could make their code more straight-forward to follow.
(type and name: same comment as on FileIndex)



Comment at: clangd/index/Merge.cpp:84
Callback) const override {
-log("findOccurrences is not implemented.");
+// Store all seen files by dynamic index.
+llvm::DenseSet SeenFiles;

I'm tempted to say remove this heuristic for now (as you say, we should solve 
this thoroughly), but the artifacts will be severe, so it's probably correct to 
include it.

But this comment doesn't explain the heuristic, it just echoes the code.
I'd suggest something like:
```// We don't want duplicate occurrences from the static/dynamic indexes,
// and we can't reliably deduplicate because offsets may differ slightly.
// We consider the dynamic index authoritative and report all its occurrences,
// and only report static index occurrences from other files.
// FIXME: this heuristic fails if the dynamic index contains a file, but all
// occurrences were removed (we will report stale ones from static).
// Ultimately we should explicitly check which index has the file instead.```





Comment at: clangd/index/Merge.cpp:85
+// Store all seen files by dynamic index.
+llvm::DenseSet SeenFiles;
+// Emit all dynamic occurrences, and static occurrences that are not

SeenFiles -> DynamicIndexFiles?



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).

hokein wrote:
> ilya-biryukov wrote:
> > 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.
> Yes, I re-thought it afterwards. We could solve this issue for 
> `findOccurrences` by redefining

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

[Sam McCall via Phabricator via cfe-commits]

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

Thanks!




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

Just to check my understanding...
we want to treat `default` the same as `case`, but the heuristics are different:
 - `case` should only appear in a switch (but might be followed by a complex 
expression)
 - `default` has lots of meanings (but we can disambiguate: check if it's 
followed by a colon)

You could consider `// default: in switch statement` above this line.



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

the intent of this test might be clearer if the cases were formatted as `case 
0: { return false; }` on one line


Repository:
  rC Clang

https://reviews.llvm.org/D51294



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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 162817.
ilya-biryukov added a comment.
Herald added a subscriber: mgrang.

- Remove mutexes, recompute every time instead
- Delay creation of TransferableCommand to avoid calling getAllCommands() on 
JSONCompilationDatabase


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
@@ -217,46 +217,43 @@
   }
 };
 
-// CommandIndex does the real work: given a filename, it produces the best
-// matching TransferableCommand by matching filenames. Basic strategy:
+// FileProximityIndex chooses the proxy file that has a compile command in a
+// compilation database when given a file that is not in the database. Basic
+// strategy:
 // - Build indexes of each of the substrings we want to look up by.
 //   These indexes are just sorted lists of the substrings.
-// - Forward requests to the inner CDB. If it fails, we must pick a proxy.
 // - Each criterion corresponds to a range lookup into the index, so we only
 //   need O(log N) string comparisons to determine scores.
-// - We then break ties among the candidates with the highest score.
-class CommandIndex {
+//
+// Apart from path proximity signals, also takes file extensions into account
+// when scoring the candidates.
+class FileProximityIndex {
 public:
-  CommandIndex(std::vector AllCommands)
-  : Commands(std::move(AllCommands)), Strings(Arena) {
-// Sort commands by filename for determinism (index is a tiebreaker later).
-llvm::sort(
-Commands.begin(), Commands.end(),
-[](const TransferableCommand &Left, const TransferableCommand &Right) {
-  return Left.Cmd.Filename < Right.Cmd.Filename;
-});
-for (size_t I = 0; I < Commands.size(); ++I) {
-  StringRef Path =
-  Strings.save(StringRef(Commands[I].Cmd.Filename).lower());
-  Paths.push_back({Path, I});
+  FileProximityIndex(std::vector Files) : Storage(Files) {
+llvm::sort(Storage.begin(), Storage.end());
+Paths.reserve(Storage.size());
+for (size_t I = 0; I < Storage.size(); ++I) {
+  StringRef Path = Storage[I];
+
+  Paths.emplace_back(Path, I);
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, ++Dir)
 if (Dir->size() > ShortDirectorySegment) // not trivial ones
   Components.emplace_back(*Dir, I);
 }
-llvm::sort(Paths.begin(), Paths.end());
+// Paths are already sorted at this point.
 llvm::sort(Stems.begin(), Stems.end());
 llvm::sort(Components.begin(), Components.end());
   }
 
-  bool empty() const { return Commands.empty(); }
+  bool empty() const { return Paths.empty(); }
 
-  // Returns the command that best fits OriginalFilename.
-  // Candidates with PreferLanguage will be chosen over others (unless it's
-  // TY_INVALID, or all candidates are bad).
-  const TransferableCommand &chooseProxy(StringRef OriginalFilename,
- types::ID PreferLanguage) const {
+  // Returns the path for the file that best fits OriginalFilename.
+  // Candidates with extensions matching PreferLanguage will be chosen over
+  // others (unless it's TY_INVALID, or all candidates are bad).
+  StringRef chooseProxy(StringRef OriginalFilename,
+types::ID PreferLanguage) const {
 assert(!empty() && "need at least one candidate!");
 std::string Filename = OriginalFilename.lower();
 auto Candidates = scoreCandidates(Filename);
@@ -266,13 +263,13 @@
 DEBUG_WITH_TYPE("interpolate",
 llvm::dbgs()
 << "interpolate: chose "
-<< Commands[Best.first].Cmd.Filename << " as proxy for "
+<< Paths[Best.first].first << " as proxy for "
 << OriginalFilename << " preferring "
 << (PreferLanguage == types::TY_INVALID
 ? "none"
 : types::getTypeName(PreferLanguage))
 << " score=" << Best.second << "\n");
-return Commands[Best.first];
+return Paths[Best.first].first;
   }
 
 private:
@@ -338,7 +335,7 @@
   ScoredCandidate S;
   S.Index = Candidate.first;
   S.Preferred = PreferredLanguage == types::TY_INVALID ||
-PreferredLanguage == Commands[S.Index].Type;
+PreferredLanguage == guessType(Paths[S.Index].first);
   S.Points = Candidate.second;
   if (!S.Preferred && Best.Preferred)
 continue;
@@ -371,16 +368,16 @@
   // If Prefix is true, it's instead the range starting w

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked 4 inline comments as done.
ilya-biryukov added inline comments.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:133
+assert(TraitsComputed && "calling transferTo on moved-from object");
+const CommandTraits &T = getTraits();
+CompileCommand Result = T.Cmd;

sammccall wrote:
> I think you're overthinking things with the memoization here (of course I say 
> this as the person who underthought it!)
> 
> AIUI, the problem is that *eagerly* parsing all the compile commands takes 3x 
> as long as reading them, which hurts startup time with big 
> `compile_commands.json`.
> 
> But I think we can afford to just parse them when `transferTo` is called, 
> without memoization. (Remember we only hit this code path when querying a 
> file *not in the CDB*, so it should never get called in a tight loop).
> 
> The benefit of slightly reducing the latency of`getCompileCommand` for 
> unknown files when we happen to pick the same template file for the second 
> time... it's unclear to me, and the code would be easier to follow without it.
Totally agree, memoization does not buy us much here.


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] r340815 - [clangd] Add some trace::Spans. NFC

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Aug 28 03:57:45 2018
New Revision: 340815

URL: http://llvm.org/viewvc/llvm-project?rev=340815&view=rev
Log:
[clangd] Add some trace::Spans. NFC

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

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=340815&r1=340814&r2=340815&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Aug 28 03:57:45 2018
@@ -495,6 +495,7 @@ void ClangdServer::consumeDiagnostics(Pa
 }
 
 tooling::CompileCommand ClangdServer::getCompileCommand(PathRef File) {
+  trace::Span Span("GetCompileCommand");
   llvm::Optional C = CDB.getCompileCommand(File);
   if (!C) // FIXME: Suppress diagnostics? Let the user know?
 C = CDB.getFallbackCommand(File);

Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=340815&r1=340814&r2=340815&view=diff
==
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Tue Aug 28 03:57:45 2018
@@ -417,6 +417,7 @@ void ASTWorker::update(
 // Note *AST can be still be null if buildAST fails.
 if (*AST) {
   OnUpdated((*AST)->getDiagnostics());
+  trace::Span Span("Running main AST callback");
   Callbacks.onMainAST(FileName, **AST);
   DiagsWereReported = true;
 }


___
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 planned changes to this revision.
ilya-biryukov added a comment.

As discussed offline, instead of changing the interface of AllTUsScheduler, we 
could add concurrency to StandloneToolExecutor


Repository:
  rC Clang

https://reviews.llvm.org/D51298



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

[clang-tools-extra] r340816 - [clangd] Remove unused parameter. NFC

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Aug 28 04:04:07 2018
New Revision: 340816

URL: http://llvm.org/viewvc/llvm-project?rev=340816&view=rev
Log:
[clangd] Remove unused parameter. NFC

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

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=340816&r1=340815&r2=340816&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Tue Aug 28 04:04:07 2018
@@ -77,8 +77,7 @@ class DeclarationAndMacrosFinder : publi
   Preprocessor &PP;
 
 public:
-  DeclarationAndMacrosFinder(raw_ostream &OS,
- const SourceLocation &SearchedLocation,
+  DeclarationAndMacrosFinder(const SourceLocation &SearchedLocation,
  ASTContext &AST, Preprocessor &PP)
   : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {}
 
@@ -163,8 +162,8 @@ struct IdentifiedSymbol {
 };
 
 IdentifiedSymbol getSymbolAtPosition(ParsedAST &AST, SourceLocation Pos) {
-  auto DeclMacrosFinder = DeclarationAndMacrosFinder(
-  llvm::errs(), Pos, AST.getASTContext(), AST.getPreprocessor());
+  auto DeclMacrosFinder = DeclarationAndMacrosFinder(Pos, AST.getASTContext(),
+ AST.getPreprocessor());
   index::IndexingOptions IndexOpts;
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
@@ -324,7 +323,7 @@ class DocumentHighlightsFinder : public
   const ASTContext *
 
 public:
-  DocumentHighlightsFinder(raw_ostream &OS, ASTContext &AST, Preprocessor &PP,
+  DocumentHighlightsFinder(ASTContext &AST, Preprocessor &PP,
std::vector &Decls)
   : Decls(Decls), AST(AST) {}
   std::vector takeHighlights() {
@@ -389,7 +388,7 @@ std::vector findDocum
   std::vector SelectedDecls = Symbols.Decls;
 
   DocumentHighlightsFinder DocHighlightsFinder(
-  llvm::errs(), AST.getASTContext(), AST.getPreprocessor(), SelectedDecls);
+  AST.getASTContext(), AST.getPreprocessor(), SelectedDecls);
 
   index::IndexingOptions IndexOpts;
   IndexOpts.SystemSymbolFilter =


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

[PATCH] D43783: [OpenCL] Remove block invoke function from emitted block literal struct

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D43783#1212485, @yaxunl wrote:

> In https://reviews.llvm.org/D43783#1204353, @svenvh wrote:
>
> > Sorry for digging up an old commit...
> >
> > Apparently this broke block arguments, e.g. the following test case:
> >
> >   int foo(int (^ bl)(void)) {
> > return bl();
> >   }
> >  
> >   int get21() {
> > return foo(^{return 21;});
> >   }
> >  
> >   int get42() {
> > return foo(^{return 42;});
> >   }
> >
> >
> > In particular, the VarDecl that `getBlockExpr()` sees doesn't have an 
> > initializer when the called block comes from an argument (causing clang to 
> > crash).
>
>
> Sorry for the delay. I think block should not be allowed as function argument 
> since this generally leads indirect function calls therefore requires support 
> of function pointer. It will rely on optimizations to get rid of indirect 
> function calls.


The idea was to allow blocks as function parameters because they are statically 
known at each function call. This can be resolved later at IR level instead of 
frontend. I am also not sure there can be other corner cases where it wouldn't 
work in Clang since we can't leverage analysis passes here. I feel this might 
be a risky thing to do in Clang. Currently it doesn't work for the examples 
provided and therefore breaking the compliance with the spec.


Repository:
  rC Clang

https://reviews.llvm.org/D43783



___
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

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.

In https://reviews.llvm.org/D51310#1214548, @kbobyrev wrote:

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


Up to you (I don't understand the interaction), but this looks good to go 
as-is. I'd expect a 10% speedup to be pretty robust to the sorts of changes 
you're talking about.




Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:90
 sync();
+std::sort(begin(Children), end(Children),
+  [](const std::unique_ptr &LHS,

this is a performance heuristic and deserves a brief comment (about *why* we're 
best to have the shortest first). Not too many details, handwavy is fine :-)



Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:93
+ const std::unique_ptr &RHS) {
+return LHS->cost() < RHS->cost();
+  });

(I'd actually suggest declaring operator< in iterator.h and making it compare 
by cost, it seems natural/important enough, and saves duplicating this lambda)



Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:125
+  size_t cost() override {
+return std::accumulate(
+begin(Children), end(Children), Children.front()->cost(),

The use of `std::accumulate` here seems less clear than the direct:
```size_t Smallest = ...;
for (const auto& Child : Children)
  Smallest = std::min(Smallest, Child->cost());
return Smallest;
```
For better or worse, functional styles don't have the most natural syntax in 
C++, and (IMO) shouldn't be used just because they fit.

(This is consistent with other places, but I also think that the same applies 
there)



Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:125
+  size_t cost() override {
+return std::accumulate(
+begin(Children), end(Children), Children.front()->cost(),

sammccall wrote:
> The use of `std::accumulate` here seems less clear than the direct:
> ```size_t Smallest = ...;
> for (const auto& Child : Children)
>   Smallest = std::min(Smallest, Child->cost());
> return Smallest;
> ```
> For better or worse, functional styles don't have the most natural syntax in 
> C++, and (IMO) shouldn't be used just because they fit.
> 
> (This is consistent with other places, but I also think that the same applies 
> there)
actually, we've sorted by cost, so don't we just want Children.front().cost() 
here?



Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:258
 
+  size_t cost() override {
+return std::accumulate(

as above, we've sorted, so just Children.back()->cost()?



Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:377
 
+  size_t cost() override { return Limit; }
+

min() this with Child->cost()?



Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:99
+  /// Returns an estimate of advance() calls before the iterator is exhausted.
+  virtual size_t cost() = 0;
 

const?



Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:99
+  /// Returns an estimate of advance() calls before the iterator is exhausted.
+  virtual size_t cost() = 0;
 

sammccall wrote:
> const?
I know this terminology comes from elsewhere, but I struggle with "cost" as a 
metaphor because I expect it to aggregate its children as a sum (at least in 
cases like "or" when all children will be advanced until end).

Maybe `estimateSize()`?


https://reviews.llvm.org/D51310



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

[PATCH] D51349: [clangd] Use buffered llvm::errs() in the clangd binary.

[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.

Unbuffered stream can cause significant (non-deterministic) latency for the 
logger.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51349

Files:
  clangd/tool/ClangdMain.cpp


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -259,6 +259,9 @@
   if (Tracer)
 TracingSession.emplace(*Tracer);
 
+  // Use buffered stream to stderr. Unbuffered stream can cause significant
+  // (non-deterministic) latency for the logger.
+  llvm::errs().SetBuffered();
   JSONOutput Out(llvm::outs(), llvm::errs(), LogLevel,
  InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,
  PrettyPrint);


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -259,6 +259,9 @@
   if (Tracer)
 TracingSession.emplace(*Tracer);
 
+  // Use buffered stream to stderr. Unbuffered stream can cause significant
+  // (non-deterministic) latency for the logger.
+  llvm::errs().SetBuffered();
   JSONOutput Out(llvm::outs(), llvm::errs(), LogLevel,
  InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,
  PrettyPrint);
___
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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D51212#1214173, @hans wrote:

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


I will commit this! Thanks!


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] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.

Awesome  :-)




Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:220
 
-// CommandIndex does the real work: given a filename, it produces the best
-// matching TransferableCommand by matching filenames. Basic strategy:
+// FileProximityIndex chooses the proxy file that has a compile command in a
+// compilation database when given a file that is not in the database. Basic

This summary is a bit unclear to me. (too many clauses, maybe too abstract).
And the high level heuristics are hidden a bit below the implementation ideas.

Maybe

```
Given a filename, FileProximityIndex picks the best matching file from the 
underlying DB. This is the proxy file whose CompileCommand will be reused.

The heuristics incorporate file name, extension, and directory structure.

Strategy:
...```



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:220
 
-// CommandIndex does the real work: given a filename, it produces the best
-// matching TransferableCommand by matching filenames. Basic strategy:
+// FileProximityIndex chooses the proxy file that has a compile command in a
+// compilation database when given a file that is not in the database. Basic

sammccall wrote:
> This summary is a bit unclear to me. (too many clauses, maybe too abstract).
> And the high level heuristics are hidden a bit below the implementation ideas.
> 
> Maybe
> 
> ```
> Given a filename, FileProximityIndex picks the best matching file from the 
> underlying DB. This is the proxy file whose CompileCommand will be reused.
> 
> The heuristics incorporate file name, extension, and directory structure.
> 
> Strategy:
> ...```
nit: I'd prefer `FileIndex` or `FilenameIndex` here - "proximity" emphasizes 
directory structure over stem/extension, which are pretty important!



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:232
-  : Commands(std::move(AllCommands)), Strings(Arena) {
-// Sort commands by filename for determinism (index is a tiebreaker later).
-llvm::sort(

restore this comment?



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:338
   S.Preferred = PreferredLanguage == types::TY_INVALID ||
-PreferredLanguage == Commands[S.Index].Type;
+PreferredLanguage == guessType(Paths[S.Index].first);
   S.Points = Candidate.second;

hmm, I would have thought we'd store the values of guessType() when building 
the index. I guess it doesn't matter, it just seems surprising to see this call 
here.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:338
   S.Preferred = PreferredLanguage == types::TY_INVALID ||
-PreferredLanguage == Commands[S.Index].Type;
+PreferredLanguage == guessType(Paths[S.Index].first);
   S.Points = Candidate.second;

sammccall wrote:
> hmm, I would have thought we'd store the values of guessType() when building 
> the index. I guess it doesn't matter, it just seems surprising to see this 
> call here.
you're calling foldType(guessType(...)) on the query, do you need to fold here 
too?



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:417
 bool TypeCertain;
 auto Lang = guessType(Filename, &TypeCertain);
 if (!TypeCertain)

this guessType/foldType call should be folded into Index.chooseProxy now I 
think - Index explicitly knows that the language it deals with must be derived 
from the filename.


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] D51349: [clangd] Use buffered llvm::errs() in the clangd binary.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clangd/tool/ClangdMain.cpp:263
+  // Use buffered stream to stderr. Unbuffered stream can cause significant
+  // (non-deterministic) latency for the logger.
+  llvm::errs().SetBuffered();

(we still flush each log message).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51349



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

[clang-tools-extra] r340822 - [clangd] Use buffered llvm::errs() in the clangd binary.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Tue Aug 28 06:15:50 2018
New Revision: 340822

URL: http://llvm.org/viewvc/llvm-project?rev=340822&view=rev
Log:
[clangd] Use buffered llvm::errs() in the clangd binary.

Summary: Unbuffered stream can cause significant (non-deterministic) latency 
for the logger.

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

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

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

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=340822&r1=340821&r2=340822&view=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Aug 28 06:15:50 2018
@@ -259,6 +259,9 @@ int main(int argc, char *argv[]) {
   if (Tracer)
 TracingSession.emplace(*Tracer);
 
+  // Use buffered stream to stderr (we still flush each log message). 
Unbuffered
+  // stream can cause significant (non-deterministic) latency for the logger.
+  llvm::errs().SetBuffered();
   JSONOutput Out(llvm::outs(), llvm::errs(), LogLevel,
  InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,
  PrettyPrint);


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

[PATCH] D51349: [clangd] Use buffered llvm::errs() in the clangd binary.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
ioeric marked an inline comment as done.
Closed by commit rCTE340822: [clangd] Use buffered llvm::errs() in the clangd 
binary. (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51349?vs=162832&id=162839#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51349

Files:
  clangd/tool/ClangdMain.cpp


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -259,6 +259,9 @@
   if (Tracer)
 TracingSession.emplace(*Tracer);
 
+  // Use buffered stream to stderr (we still flush each log message). 
Unbuffered
+  // stream can cause significant (non-deterministic) latency for the logger.
+  llvm::errs().SetBuffered();
   JSONOutput Out(llvm::outs(), llvm::errs(), LogLevel,
  InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,
  PrettyPrint);


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -259,6 +259,9 @@
   if (Tracer)
 TracingSession.emplace(*Tracer);
 
+  // Use buffered stream to stderr (we still flush each log message). Unbuffered
+  // stream can cause significant (non-deterministic) latency for the logger.
+  llvm::errs().SetBuffered();
   JSONOutput Out(llvm::outs(), llvm::errs(), LogLevel,
  InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,
  PrettyPrint);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kirill Bobyrev via Phabricator via cfe-commits]

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

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


https://reviews.llvm.org/D51352

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


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -30,8 +30,8 @@
 using namespace clang::clangd;
 
 static llvm::cl::opt
-UseDex("use-dex-index",
-   llvm::cl::desc("Use experimental Dex static index."),
+UseMem("use-mem",
+   llvm::cl::desc("Use Mem for static index."),
llvm::cl::init(false), llvm::cl::Hidden);
 
 namespace {
@@ -52,8 +52,8 @@
   for (auto Sym : Slab)
 SymsBuilder.insert(Sym);
 
-  return UseDex ? dex::DexIndex::build(std::move(SymsBuilder).build())
-: MemIndex::build(std::move(SymsBuilder).build());
+  return UseMem ? MemIndex::build(std::move(SymsBuilder).build())
+: dex::DexIndex::build(std::move(SymsBuilder).build());
 }
 
 } // namespace


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -30,8 +30,8 @@
 using namespace clang::clangd;
 
 static llvm::cl::opt
-UseDex("use-dex-index",
-   llvm::cl::desc("Use experimental Dex static index."),
+UseMem("use-mem",
+   llvm::cl::desc("Use Mem for static index."),
llvm::cl::init(false), llvm::cl::Hidden);
 
 namespace {
@@ -52,8 +52,8 @@
   for (auto Sym : Slab)
 SymsBuilder.insert(Sym);
 
-  return UseDex ? dex::DexIndex::build(std::move(SymsBuilder).build())
-: MemIndex::build(std::move(SymsBuilder).build());
+  return UseMem ? MemIndex::build(std::move(SymsBuilder).build())
+: dex::DexIndex::build(std::move(SymsBuilder).build());
 }
 
 } // namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Run `clang-format`.


https://reviews.llvm.org/D51352

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


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -29,10 +29,9 @@
 using namespace clang;
 using namespace clang::clangd;
 
-static llvm::cl::opt
-UseDex("use-dex-index",
-   llvm::cl::desc("Use experimental Dex static index."),
-   llvm::cl::init(false), llvm::cl::Hidden);
+static llvm::cl::opt UseMem("use-mem",
+  llvm::cl::desc("Use Mem for static index."),
+  llvm::cl::init(false), llvm::cl::Hidden);
 
 namespace {
 
@@ -52,8 +51,8 @@
   for (auto Sym : Slab)
 SymsBuilder.insert(Sym);
 
-  return UseDex ? dex::DexIndex::build(std::move(SymsBuilder).build())
-: MemIndex::build(std::move(SymsBuilder).build());
+  return UseMem ? MemIndex::build(std::move(SymsBuilder).build())
+: dex::DexIndex::build(std::move(SymsBuilder).build());
 }
 
 } // namespace


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -29,10 +29,9 @@
 using namespace clang;
 using namespace clang::clangd;
 
-static llvm::cl::opt
-UseDex("use-dex-index",
-   llvm::cl::desc("Use experimental Dex static index."),
-   llvm::cl::init(false), llvm::cl::Hidden);
+static llvm::cl::opt UseMem("use-mem",
+  llvm::cl::desc("Use Mem for static index."),
+  llvm::cl::init(false), llvm::cl::Hidden);
 
 namespace {
 
@@ -52,8 +51,8 @@
   for (auto Sym : Slab)
 SymsBuilder.insert(Sym);
 
-  return UseDex ? dex::DexIndex::build(std::move(SymsBuilder).build())
-: MemIndex::build(std::move(SymsBuilder).build());
+  return UseMem ? MemIndex::build(std::move(SymsBuilder).build())
+: dex::DexIndex::build(std::move(SymsBuilder).build());
 }
 
 } // namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:32
 
 static llvm::cl::opt
-UseDex("use-dex-index",

I think we should stick to the same option and just flip the default. 
Introducing yet another option (that is going to be removed) could be confusing.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:33
 static llvm::cl::opt
-UseDex("use-dex-index",
-   llvm::cl::desc("Use experimental Dex static index."),

Also add FIXME indicating that the option will be removed soon.


https://reviews.llvm.org/D51352



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

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

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 162842.
kbobyrev marked 2 inline comments as done.

https://reviews.llvm.org/D51352

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


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -29,10 +29,12 @@
 using namespace clang;
 using namespace clang::clangd;
 
+// FIXME(kbobyrev): This option should be removed as Dex is now the default
+// static index.
 static llvm::cl::opt
 UseDex("use-dex-index",
llvm::cl::desc("Use experimental Dex static index."),
-   llvm::cl::init(false), llvm::cl::Hidden);
+   llvm::cl::init(true), llvm::cl::Hidden);
 
 namespace {
 


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -29,10 +29,12 @@
 using namespace clang;
 using namespace clang::clangd;
 
+// FIXME(kbobyrev): This option should be removed as Dex is now the default
+// static index.
 static llvm::cl::opt
 UseDex("use-dex-index",
llvm::cl::desc("Use experimental Dex static index."),
-   llvm::cl::init(false), llvm::cl::Hidden);
+   llvm::cl::init(true), llvm::cl::Hidden);
 
 namespace {
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r340823 - Use addressof instead of operator& in make_shared. Fixes PR38729. As a drive-by, make the same change in raw_storage_iterator (twice).

[Marshall Clow via cfe-commits]

Author: marshall
Date: Tue Aug 28 06:29:30 2018
New Revision: 340823

URL: http://llvm.org/viewvc/llvm-project?rev=340823&view=rev
Log:
Use addressof instead of operator& in make_shared. Fixes PR38729. As a 
drive-by, make the same change in raw_storage_iterator (twice).

Modified:
libcxx/trunk/include/memory

libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.base.pass.cpp

libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.pass.cpp

libcxx/trunk/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/allocate_shared.pass.cpp

libcxx/trunk/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared.pass.cpp

Modified: libcxx/trunk/include/memory
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/memory?rev=340823&r1=340822&r2=340823&view=diff
==
--- libcxx/trunk/include/memory (original)
+++ libcxx/trunk/include/memory Tue Aug 28 06:29:30 2018
@@ -1989,10 +1989,10 @@ public:
 _LIBCPP_INLINE_VISIBILITY explicit raw_storage_iterator(_OutputIterator 
__x) : __x_(__x) {}
 _LIBCPP_INLINE_VISIBILITY raw_storage_iterator& operator*() {return *this;}
 _LIBCPP_INLINE_VISIBILITY raw_storage_iterator& operator=(const _Tp& 
__element)
-{::new(&*__x_) _Tp(__element); return *this;}
+{::new(_VSTD::addressof(*__x_)) _Tp(__element); return *this;}
 #if _LIBCPP_STD_VER >= 14
 _LIBCPP_INLINE_VISIBILITY raw_storage_iterator& operator=(_Tp&& __element)
-{::new(&*__x_) _Tp(_VSTD::move(__element)); return *this;}
+{::new(_VSTD::addressof(*__x_)) _Tp(_VSTD::move(__element)); return 
*this;}
 #endif
 _LIBCPP_INLINE_VISIBILITY raw_storage_iterator& operator++() {++__x_; 
return *this;}
 _LIBCPP_INLINE_VISIBILITY raw_storage_iterator  operator++(int)
@@ -3682,7 +3682,7 @@ private:
 virtual void __on_zero_shared_weak() _NOEXCEPT;
 public:
 _LIBCPP_INLINE_VISIBILITY
-_Tp* get() _NOEXCEPT {return &__data_.second();}
+_Tp* get() _NOEXCEPT {return _VSTD::addressof(__data_.second());}
 };
 
 template 

Modified: 
libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.base.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.base.pass.cpp?rev=340823&r1=340822&r2=340823&view=diff
==
--- 
libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.base.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.base.pass.cpp
 Tue Aug 28 06:29:30 2018
@@ -15,6 +15,13 @@
 
 #include "test_macros.h"
 
+#if TEST_STD_VER >= 11
+#define DELETE_FUNCTION = delete
+#else
+#define DELETE_FUNCTION
+#endif
+
+
 int A_constructed = 0;
 
 struct A
@@ -27,6 +34,7 @@ public:
 ~A() {--A_constructed; data_ = 0;}
 
 bool operator==(int i) const {return data_ == i;}
+A* operator& () DELETE_FUNCTION;
 };
 
 int main()

Modified: 
libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.pass.cpp?rev=340823&r1=340822&r2=340823&view=diff
==
--- 
libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.pass.cpp
 Tue Aug 28 06:29:30 2018
@@ -16,6 +16,12 @@
 #include "test_macros.h"
 #include 
 
+#if TEST_STD_VER >= 11
+#define DELETE_FUNCTION = delete
+#else
+#define DELETE_FUNCTION
+#endif
+
 int A_constructed = 0;
 
 struct A
@@ -28,6 +34,7 @@ public:
 ~A() {--A_constructed; data_ = 0;}
 
 bool operator==(int i) const {return data_ == i;}
+A* operator& () DELETE_FUNCTION;
 };
 
 int main()

Modified: 
libcxx/trunk/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/allocate_shared.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/allocate_shared.pass.cpp?rev=340823&r1=340822&r2=340823&view=diff
==
--- 
libcxx/trunk/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/allocate_shared.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/allocate_shared.pass.cpp
 Tue Aug 28 06:29:30 2018
@@ -23,6 +23,12 @@
 #include "test_allocator.h"
 #include "min_allocator.h"
 
+#if TEST_STD_VER >= 11
+#define DELETE_FUNCTION = delete
+#else
+#define DELETE_FUNCTION

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

[Christian Bruel via Phabricator via cfe-commits]

chrib created this revision.
Herald added subscribers: cfe-commits, srhines.

Fix -print-multi-directory to print the selected multilib


Repository:
  rC Clang

https://reviews.llvm.org/D51354

Files:
  include/clang/Driver/ToolChain.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Linux.cpp
  test/Driver/print-multi-directory.c

Index: test/Driver/print-multi-directory.c
===
--- /dev/null
+++ test/Driver/print-multi-directory.c
@@ -0,0 +1,28 @@
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target i386-none-linux \
+// RUN: -print-multi-directory \
+// RUN:   | FileCheck --check-prefix=CHECK-X86-MULTILIBS %s
+
+// CHECK-X86-MULTILIBS:  32
+// CHECK-X86-MULTILIBS-NOT:  x32
+// CHECK-X86-MULTILIBS-NOT:  .
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target i386-none-linux -m64 \
+// RUN: -print-multi-directory \
+// RUN:   | FileCheck --check-prefix=CHECK-X86_64-MULTILIBS %s
+
+// CHECK-X86_64-MULTILIBS:  .
+// CHECK-X86_64-MULTILIBS-NOT:  x32
+// CHECK-X86_64-MULTILIBS-NOT:  32
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target arm-linux-androideabi21 -stdlib=libstdc++ \
+// RUN: -mthumb \
+// RUN: -B%S/Inputs/basic_android_ndk_tree \
+// RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \
+// RUN: -print-multi-directory \
+// RUN:   | FileCheck  --check-prefix=CHECK-ARM-MULTILIBS %s
+
+// CHECK-ARM-MULTILIBS:  thumb
+// CHECK-ARM-MULTILIBS-NOT:  .
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -210,6 +210,7 @@
 : Generic_ELF(D, Triple, Args) {
   GCCInstallation.init(Triple, Args);
   Multilibs = GCCInstallation.getMultilibs();
+  SelectedMultilib = GCCInstallation.getMultilib();
   llvm::Triple::ArchType Arch = Triple.getArch();
   std::string SysRoot = computeSysRoot();
 
@@ -299,16 +300,14 @@
   if (GCCInstallation.isValid()) {
 const llvm::Triple &GCCTriple = GCCInstallation.getTriple();
 const std::string &LibPath = GCCInstallation.getParentLibPath();
-const Multilib &Multilib = GCCInstallation.getMultilib();
-const MultilibSet &Multilibs = GCCInstallation.getMultilibs();
 
 // Add toolchain / multilib specific file paths.
-addMultilibsFilePaths(D, Multilibs, Multilib,
+addMultilibsFilePaths(D, Multilibs, SelectedMultilib,
   GCCInstallation.getInstallPath(), Paths);
 
 // Sourcery CodeBench MIPS toolchain holds some libraries under
 // a biarch-like suffix of the GCC installation.
-addPathIfExists(D, GCCInstallation.getInstallPath() + Multilib.gccSuffix(),
+addPathIfExists(D, GCCInstallation.getInstallPath() + SelectedMultilib.gccSuffix(),
 Paths);
 
 // GCC cross compiling toolchains will install target libraries which ship
@@ -330,7 +329,7 @@
 // Note that this matches the GCC behavior. See the below comment for where
 // Clang diverges from GCC's behavior.
 addPathIfExists(D, LibPath + "/../" + GCCTriple.str() + "/lib/../" +
-   OSLibDir + Multilib.osSuffix(),
+   OSLibDir + SelectedMultilib.osSuffix(),
 Paths);
 
 // If the GCC installation we found is inside of the sysroot, we want to
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1661,14 +1661,13 @@
   }
 
   if (C.getArgs().hasArg(options::OPT_print_multi_directory)) {
-for (const Multilib &Multilib : TC.getMultilibs()) {
-  if (Multilib.gccSuffix().empty())
-llvm::outs() << ".\n";
-  else {
-StringRef Suffix(Multilib.gccSuffix());
-assert(Suffix.front() == '/');
-llvm::outs() << Suffix.substr(1) << "\n";
-  }
+const Multilib &Multilib = TC.getMultilib();
+if (Multilib.gccSuffix().empty())
+  llvm::outs() << ".\n";
+else {
+  StringRef Suffix(Multilib.gccSuffix());
+  assert(Suffix.front() == '/');
+  llvm::outs() << Suffix.substr(1) << "\n";
 }
 return false;
   }
Index: include/clang/Driver/ToolChain.h
===
--- include/clang/Driver/ToolChain.h
+++ include/clang/Driver/ToolChain.h
@@ -149,6 +149,7 @@
 
 protected:
   MultilibSet Multilibs;
+  Multilib SelectedMultilib;
 
   ToolChain(const Driver &D, const llvm::Triple &T,
 const llvm::opt::ArgList &Args);
@@ -227,6 +228,8 @@
 
   const MultilibSet &getMultilibs() const { return Multilibs; }
 
+  const Multilib &getMultilib() const { return SelectedMultilib; }
+
   const SanitizerArgs& getSanitizerArgs() const;
 
   const XRayArgs& getXRayArgs() const;
___
cfe-commits mailing lis

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

[Christian Bruel via Phabricator via cfe-commits]

chrib added a comment.

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


Repository:
  rC Clang

https://reviews.llvm.org/D51354



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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Update the comments
- Rename the new class to FileIndex
- Restore an accidentally lost comment
- Store file types in a parallel array instead of recomputing on each call
- Use foldType(guessType()) when obtaining lang type from filename


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
@@ -217,46 +217,47 @@
   }
 };
 
-// CommandIndex does the real work: given a filename, it produces the best
-// matching TransferableCommand by matching filenames. Basic strategy:
+// Given a filename, FileIndex picks the best matching file from the underlying
+// DB. This is the proxy file whose CompileCommand will be reused. The
+// heuristics incorporate file name, extension, and directory structure.
+// Strategy:
 // - Build indexes of each of the substrings we want to look up by.
 //   These indexes are just sorted lists of the substrings.
-// - Forward requests to the inner CDB. If it fails, we must pick a proxy.
 // - Each criterion corresponds to a range lookup into the index, so we only
 //   need O(log N) string comparisons to determine scores.
-// - We then break ties among the candidates with the highest score.
-class CommandIndex {
+//
+// Apart from path proximity signals, also takes file extensions into account
+// when scoring the candidates.
+class FileIndex {
 public:
-  CommandIndex(std::vector AllCommands)
-  : Commands(std::move(AllCommands)), Strings(Arena) {
+  FileIndex(std::vector Files) : Storage(Files) {
 // Sort commands by filename for determinism (index is a tiebreaker later).
-llvm::sort(
-Commands.begin(), Commands.end(),
-[](const TransferableCommand &Left, const TransferableCommand &Right) {
-  return Left.Cmd.Filename < Right.Cmd.Filename;
-});
-for (size_t I = 0; I < Commands.size(); ++I) {
-  StringRef Path =
-  Strings.save(StringRef(Commands[I].Cmd.Filename).lower());
-  Paths.push_back({Path, I});
+llvm::sort(Storage.begin(), Storage.end());
+Paths.reserve(Storage.size());
+Types.reserve(Storage.size());
+for (size_t I = 0; I < Storage.size(); ++I) {
+  StringRef Path = Storage[I];
+
+  Paths.emplace_back(Path, I);
+  Types.push_back(foldType(guessType(Path)));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, ++Dir)
 if (Dir->size() > ShortDirectorySegment) // not trivial ones
   Components.emplace_back(*Dir, I);
 }
-llvm::sort(Paths.begin(), Paths.end());
+// Paths are already sorted at this point.
 llvm::sort(Stems.begin(), Stems.end());
 llvm::sort(Components.begin(), Components.end());
   }
 
-  bool empty() const { return Commands.empty(); }
+  bool empty() const { return Paths.empty(); }
 
-  // Returns the command that best fits OriginalFilename.
-  // Candidates with PreferLanguage will be chosen over others (unless it's
-  // TY_INVALID, or all candidates are bad).
-  const TransferableCommand &chooseProxy(StringRef OriginalFilename,
- types::ID PreferLanguage) const {
+  // Returns the path for the file that best fits OriginalFilename.
+  // Candidates with extensions matching PreferLanguage will be chosen over
+  // others (unless it's TY_INVALID, or all candidates are bad).
+  StringRef chooseProxy(StringRef OriginalFilename,
+types::ID PreferLanguage) const {
 assert(!empty() && "need at least one candidate!");
 std::string Filename = OriginalFilename.lower();
 auto Candidates = scoreCandidates(Filename);
@@ -266,13 +267,13 @@
 DEBUG_WITH_TYPE("interpolate",
 llvm::dbgs()
 << "interpolate: chose "
-<< Commands[Best.first].Cmd.Filename << " as proxy for "
+<< Paths[Best.first].first << " as proxy for "
 << OriginalFilename << " preferring "
 << (PreferLanguage == types::TY_INVALID
 ? "none"
 : types::getTypeName(PreferLanguage))
 << " score=" << Best.second << "\n");
-return Commands[Best.first];
+return Paths[Best.first].first;
   }
 
 private:
@@ -338,7 +339,7 @@
   ScoredCandidate S;
   S.Index = Candidate.first;
   S.Preferred = PreferredLanguage == types::TY_INVALID ||
-PreferredLanguage == Commands[S.Index].Type;
+ 

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

[Eric Liu via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:32
 
+// FIXME(kbobyrev): This option should be removed as Dex is now the default
+// static index.

Maybe: `FIXME: remove this option when Dex is stable enough.`


https://reviews.llvm.org/D51352



___
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 162851.
ilya-biryukov added a comment.

- Reformat the code
- Minor spelling fix


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
@@ -217,62 +217,62 @@
   }
 };
 
-// CommandIndex does the real work: given a filename, it produces the best
-// matching TransferableCommand by matching filenames. Basic strategy:
+// Given a filename, FileIndex picks the best matching file from the underlying
+// DB. This is the proxy file whose CompileCommand will be reused. The
+// heuristics incorporate file name, extension, and directory structure.
+// Strategy:
 // - Build indexes of each of the substrings we want to look up by.
 //   These indexes are just sorted lists of the substrings.
-// - Forward requests to the inner CDB. If it fails, we must pick a proxy.
 // - Each criterion corresponds to a range lookup into the index, so we only
 //   need O(log N) string comparisons to determine scores.
-// - We then break ties among the candidates with the highest score.
-class CommandIndex {
+//
+// Apart from path proximity signals, also takes file extensions into account
+// when scoring the candidates.
+class FileIndex {
 public:
-  CommandIndex(std::vector AllCommands)
-  : Commands(std::move(AllCommands)), Strings(Arena) {
+  FileIndex(std::vector Files) : Storage(Files) {
 // Sort commands by filename for determinism (index is a tiebreaker later).
-llvm::sort(
-Commands.begin(), Commands.end(),
-[](const TransferableCommand &Left, const TransferableCommand &Right) {
-  return Left.Cmd.Filename < Right.Cmd.Filename;
-});
-for (size_t I = 0; I < Commands.size(); ++I) {
-  StringRef Path =
-  Strings.save(StringRef(Commands[I].Cmd.Filename).lower());
-  Paths.push_back({Path, I});
+llvm::sort(Storage.begin(), Storage.end());
+Paths.reserve(Storage.size());
+Types.reserve(Storage.size());
+for (size_t I = 0; I < Storage.size(); ++I) {
+  StringRef Path = Storage[I];
+
+  Paths.emplace_back(Path, I);
+  Types.push_back(foldType(guessType(Path)));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, ++Dir)
 if (Dir->size() > ShortDirectorySegment) // not trivial ones
   Components.emplace_back(*Dir, I);
 }
-llvm::sort(Paths.begin(), Paths.end());
+// Paths are already sorted at this point.
 llvm::sort(Stems.begin(), Stems.end());
 llvm::sort(Components.begin(), Components.end());
   }
 
-  bool empty() const { return Commands.empty(); }
+  bool empty() const { return Paths.empty(); }
 
-  // Returns the command that best fits OriginalFilename.
-  // Candidates with PreferLanguage will be chosen over others (unless it's
-  // TY_INVALID, or all candidates are bad).
-  const TransferableCommand &chooseProxy(StringRef OriginalFilename,
- types::ID PreferLanguage) const {
+  // Returns the path for the file that best fits OriginalFilename.
+  // Candidates with extensions matching PreferLanguage will be chosen over
+  // others (unless it's TY_INVALID, or all candidates are bad).
+  StringRef chooseProxy(StringRef OriginalFilename,
+types::ID PreferLanguage) const {
 assert(!empty() && "need at least one candidate!");
 std::string Filename = OriginalFilename.lower();
 auto Candidates = scoreCandidates(Filename);
 std::pair Best =
 pickWinner(Candidates, Filename, PreferLanguage);
 
 DEBUG_WITH_TYPE("interpolate",
-llvm::dbgs()
-<< "interpolate: chose "
-<< Commands[Best.first].Cmd.Filename << " as proxy for "
-<< OriginalFilename << " preferring "
-<< (PreferLanguage == types::TY_INVALID
-? "none"
-: types::getTypeName(PreferLanguage))
-<< " score=" << Best.second << "\n");
-return Commands[Best.first];
+llvm::dbgs() << "interpolate: chose "
+ << Paths[Best.first].first << " as proxy for "
+ << OriginalFilename << " preferring "
+ << (PreferLanguage == types::TY_INVALID
+ ? "none"
+ : types::getTypeName(PreferLanguage))
+ << " score=" << Best.second << "\n");
+return Paths[Best.first].first;
   }

[PATCH] D51356: [docs][mips] Clang 7.0 Release notes

[Simon Atanasyan via Phabricator via cfe-commits]

atanasyan created this revision.
atanasyan added reviewers: hans, petarj, smaksimovic, abeserminji.
Herald added subscribers: arichardson, sdardis.

MIPS specific part of Clang 7.0 Release notes. Feel free to add more notes if I 
miss something.


Repository:
  rC Clang

https://reviews.llvm.org/D51356

Files:
  ReleaseNotes.rst


Index: ReleaseNotes.rst
===
--- ReleaseNotes.rst
+++ ReleaseNotes.rst
@@ -114,6 +114,12 @@
   relocation scanning. The ``-faddrsig`` and ``-fno-addrsig`` flags can be
   used to control whether to emit the address-significance table.
 
+- Integrated assembler is enabled by default on OpenBSD / FreeBSD
+  for MIPS 64-bit targets.
+
+- On MIPS FreeBSD default CPUs have been changed to ``mips2``
+  for 32-bit targets and ``mips3`` for 32-bit targets.
+
 - ...
 
 New Compiler Flags
@@ -133,6 +139,15 @@
modules where it isn't necessary. It causes more inline virtual functions
to be emitted.
 
+- Added the``-mcrc`` and ``-mno-crc`` flags to enable/disable using
+  of MIPS Cyclic Redundancy Check instructions.
+
+- Added the``-mvirt`` and ``-mno-virt`` flags to enable/disable using
+  of MIPS Virtualization instructions.
+
+- Added the``-mginv`` and ``-mno-ginv`` flags to enable/disable using
+  of MIPS Global INValidate instructions.
+
 - ...
 
 Deprecated Compiler Flags


Index: ReleaseNotes.rst
===
--- ReleaseNotes.rst
+++ ReleaseNotes.rst
@@ -114,6 +114,12 @@
   relocation scanning. The ``-faddrsig`` and ``-fno-addrsig`` flags can be
   used to control whether to emit the address-significance table.
 
+- Integrated assembler is enabled by default on OpenBSD / FreeBSD
+  for MIPS 64-bit targets.
+
+- On MIPS FreeBSD default CPUs have been changed to ``mips2``
+  for 32-bit targets and ``mips3`` for 32-bit targets.
+
 - ...
 
 New Compiler Flags
@@ -133,6 +139,15 @@
modules where it isn't necessary. It causes more inline virtual functions
to be emitted.
 
+- Added the``-mcrc`` and ``-mno-crc`` flags to enable/disable using
+  of MIPS Cyclic Redundancy Check instructions.
+
+- Added the``-mvirt`` and ``-mno-virt`` flags to enable/disable using
+  of MIPS Virtualization instructions.
+
+- Added the``-mginv`` and ``-mno-ginv`` flags to enable/disable using
+  of MIPS Global INValidate instructions.
+
 - ...
 
 Deprecated Compiler Flags
___
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.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Nice!

We could reduce the scope of this patch somewhat by ignoring file proximity and 
just switching to return the most popular header. This would be a solid 
improvement over current behavior, and provide the infrastructure for the 
file-proximity approach.




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

I remain unconvinced that providing multiple completion candidates is the right 
thing to do here:
 - if the index hasn't seen a definition, then we're going to show one copy of 
the completion for each header that has a declaration
 - the user isn't going to have a useful way to distinguish between them. Note 
that e.g. we're going to show the #include path in the detail, but the 
documentation is going to be identical for each.
 - we lose the invariant that each completion item (pre-bundling) maps to a 
different symbol
 - C++ embedders don't have the option of displaying the multiple options once 
the completion is selected

The alternative approach of sorting the includes by proximity * log(refs) or 
so, and then using the top one for scoring, seems less of a drastic change to 
the current behavior. (Visible effect: more accurate includes inserted).



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

This looks a bit sketchy. Particularly the += boost where everything else is *=.
What's this trying to do?



Comment at: clangd/index/Index.h:220
+llvm::StringRef IncludeHeader = "";
+/// The number of translation units that reference this symbol via this
+/// header. This number is only meaningful if aggregated in an index.

via this header -> and include this header?

(otherwise, what does "via" mean?)



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

This describes the logic, and the logic always produces the right result, but 
it's not clear *why*. Maybe add something like:

```This is associative because it preserves the invariant:
 - if we haven't seen a definition, Includes covers all declarations
 - if we have seen a definition, Includes covers declarations visible from any 
definition```

in fact it seems hard to reason about this field in Symbol without 
understanding this, so maybe this invariant belongs on the IncludeHeaders field 
itself.



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

sammccall wrote:
> This describes the logic, and the logic always produces the right result, but 
> it's not clear *why*. Maybe add something like:
> 
> ```This is associative because it preserves the invariant:
>  - if we haven't seen a definition, Includes covers all declarations
>  - if we have seen a definition, Includes covers declarations visible from 
> any definition```
> 
> in fact it seems hard to reason about this field in Symbol without 
> understanding this, so maybe this invariant belongs on the IncludeHeaders 
> field itself.
Thinking more about it - what's the intent here?
I'm not sure sorting by (seen by def, #refs) produces better ranking than just 
#refs.

But there are other possible reasons for dropping includes not seen by any def:
 - remove spam from the completion list (only a problem if we clone the 
completion items)
 - reduce size for items that are often redeclared (I can imagine this being a 
problem, not obvious)

Curious what your thinking is.


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] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:229
+//
+// Apart from path proximity signals, also takes file extensions into account
+// when scoring the candidates.

(this comment is covered in the summary now)


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] D51321: [Tooling] Improve handling of CL-style options

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Hi, sorry about overlooking cl mode flags when adding this, I was blissfully 
unaware that `compile_commands.json` could use that syntax :-)

Out of curiosity, are you using this with clangd or some other tool? I'm sure 
there are places where clangd injects unixy flags...
Will take a look and try to understand.

Be aware of https://reviews.llvm.org/D51314 which is fixing some nasty 
performance pitfalls of InterpolatingCompilationDatabase (it should be logic 
neutral, but moves around the parsing code).

Could you reupload the patch with context?


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] D50385: [clangd] Collect symbol occurrences in SymbolCollector

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 162854.
hokein marked 10 inline comments as done.
hokein added a comment.

Address review comments and fix code style.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50385

Files:
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -28,9 +28,15 @@
 #include 
 #include 
 
+namespace clang {
+namespace clangd {
+
+namespace {
+
 using testing::AllOf;
 using testing::Eq;
 using testing::Field;
+using testing::IsEmpty;
 using testing::Not;
 using testing::UnorderedElementsAre;
 using testing::UnorderedElementsAreArray;
@@ -74,11 +80,18 @@
 MATCHER_P(ForCodeCompletion, IsIndexedForCodeCompletion, "") {
   return arg.IsIndexedForCodeCompletion == IsIndexedForCodeCompletion;
 }
-
-namespace clang {
-namespace clangd {
-
-namespace {
+MATCHER(OccurrenceRange, "") {
+  const SymbolOccurrence &Pos = testing::get<0>(arg);
+  const Range &Range = testing::get<1>(arg);
+  return std::tie(Pos.Location.Start.Line, Pos.Location.Start.Column,
+  Pos.Location.End.Line, Pos.Location.End.Column) ==
+ std::tie(Range.start.line, Range.start.character, Range.end.line,
+  Range.end.character);
+}
+testing::Matcher &>
+HaveRanges(const std::vector Ranges) {
+  return testing::UnorderedPointwise(OccurrenceRange(), Ranges);
+}
 
 class ShouldCollectSymbolTest : public ::testing::Test {
 public:
@@ -237,6 +250,7 @@
 llvm::MemoryBuffer::getMemBuffer(MainCode));
 Invocation.run();
 Symbols = Factory->Collector->takeSymbols();
+SymbolOccurrences = Factory->Collector->takeOccurrences();
 return true;
   }
 
@@ -247,6 +261,7 @@
   std::string TestFileName;
   std::string TestFileURI;
   SymbolSlab Symbols;
+  SymbolOccurrenceSlab SymbolOccurrences;
   SymbolCollector::Options CollectorOpts;
   std::unique_ptr PragmaHandler;
 };
@@ -413,6 +428,61 @@
   ));
 }
 
+TEST_F(SymbolCollectorTest, Occurrences) {
+  Annotations Header(R"(
+  class $foo[[Foo]] {
+  public:
+$foo[[Foo]]() {}
+$foo[[Foo]](int);
+  };
+  class $bar[[Bar]];
+  void $func[[func]]();
+  )");
+  Annotations Main(R"(
+  class $bar[[Bar]] {};
+
+  void $func[[func]]();
+
+  void fff() {
+$foo[[Foo]] foo;
+$bar[[Bar]] bar;
+$func[[func]]();
+int abc = 0;
+$foo[[Foo]] foo2 = abc;
+  }
+  )");
+  Annotations SymbolsOnlyInMainCode(R"(
+  int a;
+  void b() {}
+  static const int c = 0;
+  class d {};
+  )");
+  CollectorOpts.OccurrenceFilter = SymbolOccurrenceKind::Declaration |
+   SymbolOccurrenceKind::Definition |
+   SymbolOccurrenceKind::Reference;
+  runSymbolCollector(Header.code(),
+ (Main.code() + SymbolsOnlyInMainCode.code()).str());
+  auto HeaderSymbols = TestTU::withHeaderCode(Header.code()).headerSymbols();
+
+  EXPECT_THAT(SymbolOccurrences.find(findSymbol(Symbols, "Foo").ID),
+  HaveRanges(Main.ranges("foo")));
+  EXPECT_THAT(SymbolOccurrences.find(findSymbol(Symbols, "Bar").ID),
+  HaveRanges(Main.ranges("bar")));
+  EXPECT_THAT(SymbolOccurrences.find(findSymbol(Symbols, "func").ID),
+  HaveRanges(Main.ranges("func")));
+
+  // Retrieve IDs for symbols *only* in the main file, and verify these symbols
+  // are not collected.
+  auto MainSymbols =
+  TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
+  EXPECT_THAT(SymbolOccurrences.find(findSymbol(MainSymbols, "a").ID),
+  IsEmpty());
+  EXPECT_THAT(SymbolOccurrences.find(findSymbol(MainSymbols, "b").ID),
+  IsEmpty());
+  EXPECT_THAT(SymbolOccurrences.find(findSymbol(MainSymbols, "c").ID),
+  IsEmpty());
+}
+
 TEST_F(SymbolCollectorTest, References) {
   const std::string Header = R"(
 class W;
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -52,6 +52,10 @@
 const CanonicalIncludes *Includes = nullptr;
 // Populate the Symbol.References field.
 bool CountReferences = false;
+/// The symbol occurrence kind that will be collected.
+/// If not set (Unknown), SymbolCollector will not collect any symbol
+/// occurrences.
+SymbolOccurrenceKind OccurrenceFilter = SymbolOccurrenceKind::Unknown;
 // Every symbol collected will be stamped with this origin.
 SymbolOrigin Origin = SymbolOrigin::Unknown;
 /// Collect macros.
@@ -86,6 +90,10 @@
 
   SymbolSlab takeSymbols() { return std::move(Symbols).build(); }
 
+  SymbolOccurrenceSlab takeOccurrences() {
+return std::move(Symbol

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Lowercase everything stored in the index.


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
@@ -217,62 +217,64 @@
   }
 };
 
-// CommandIndex does the real work: given a filename, it produces the best
-// matching TransferableCommand by matching filenames. Basic strategy:
+// Given a filename, FileIndex picks the best matching file from the underlying
+// DB. This is the proxy file whose CompileCommand will be reused. The
+// heuristics incorporate file name, extension, and directory structure.
+// Strategy:
 // - Build indexes of each of the substrings we want to look up by.
 //   These indexes are just sorted lists of the substrings.
-// - Forward requests to the inner CDB. If it fails, we must pick a proxy.
 // - Each criterion corresponds to a range lookup into the index, so we only
 //   need O(log N) string comparisons to determine scores.
-// - We then break ties among the candidates with the highest score.
-class CommandIndex {
+//
+// Apart from path proximity signals, also takes file extensions into account
+// when scoring the candidates.
+class FileIndex {
 public:
-  CommandIndex(std::vector AllCommands)
-  : Commands(std::move(AllCommands)), Strings(Arena) {
+  FileIndex(std::vector Files)
+  : OriginalPaths(std::move(Files)), Strings(Arena) {
 // Sort commands by filename for determinism (index is a tiebreaker later).
-llvm::sort(
-Commands.begin(), Commands.end(),
-[](const TransferableCommand &Left, const TransferableCommand &Right) {
-  return Left.Cmd.Filename < Right.Cmd.Filename;
-});
-for (size_t I = 0; I < Commands.size(); ++I) {
-  StringRef Path =
-  Strings.save(StringRef(Commands[I].Cmd.Filename).lower());
-  Paths.push_back({Path, I});
+llvm::sort(OriginalPaths.begin(), OriginalPaths.end());
+Paths.reserve(OriginalPaths.size());
+Types.reserve(OriginalPaths.size());
+Stems.reserve(OriginalPaths.size());
+for (size_t I = 0; I < OriginalPaths.size(); ++I) {
+  StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
+
+  Paths.emplace_back(Path, I);
+  Types.push_back(foldType(guessType(Path)));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, ++Dir)
 if (Dir->size() > ShortDirectorySegment) // not trivial ones
   Components.emplace_back(*Dir, I);
 }
-llvm::sort(Paths.begin(), Paths.end());
+// Paths are already sorted at this point.
 llvm::sort(Stems.begin(), Stems.end());
 llvm::sort(Components.begin(), Components.end());
   }
 
-  bool empty() const { return Commands.empty(); }
+  bool empty() const { return Paths.empty(); }
 
-  // Returns the command that best fits OriginalFilename.
-  // Candidates with PreferLanguage will be chosen over others (unless it's
-  // TY_INVALID, or all candidates are bad).
-  const TransferableCommand &chooseProxy(StringRef OriginalFilename,
- types::ID PreferLanguage) const {
+  // Returns the path for the file that best fits OriginalFilename.
+  // Candidates with extensions matching PreferLanguage will be chosen over
+  // others (unless it's TY_INVALID, or all candidates are bad).
+  StringRef chooseProxy(StringRef OriginalFilename,
+types::ID PreferLanguage) const {
 assert(!empty() && "need at least one candidate!");
 std::string Filename = OriginalFilename.lower();
 auto Candidates = scoreCandidates(Filename);
 std::pair Best =
 pickWinner(Candidates, Filename, PreferLanguage);
 
 DEBUG_WITH_TYPE("interpolate",
-llvm::dbgs()
-<< "interpolate: chose "
-<< Commands[Best.first].Cmd.Filename << " as proxy for "
-<< OriginalFilename << " preferring "
-<< (PreferLanguage == types::TY_INVALID
-? "none"
-: types::getTypeName(PreferLanguage))
-<< " score=" << Best.second << "\n");
-return Commands[Best.first];
+llvm::dbgs() << "interpolate: chose "
+ << OriginalPaths[Best.first] << " as proxy for "
+ << OriginalFilename << " preferring "
+ << (PreferLanguage == types::TY_INVALID
+ ? "none"
+

[PATCH] D50385: [clangd] Collect symbol occurrences in SymbolCollector

[Haojian Wu via Phabricator via cfe-commits]

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

Minor cleanup.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50385

Files:
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -28,9 +28,15 @@
 #include 
 #include 
 
+namespace clang {
+namespace clangd {
+
+namespace {
+
 using testing::AllOf;
 using testing::Eq;
 using testing::Field;
+using testing::IsEmpty;
 using testing::Not;
 using testing::UnorderedElementsAre;
 using testing::UnorderedElementsAreArray;
@@ -74,11 +80,18 @@
 MATCHER_P(ForCodeCompletion, IsIndexedForCodeCompletion, "") {
   return arg.IsIndexedForCodeCompletion == IsIndexedForCodeCompletion;
 }
-
-namespace clang {
-namespace clangd {
-
-namespace {
+MATCHER(OccurrenceRange, "") {
+  const SymbolOccurrence &Pos = testing::get<0>(arg);
+  const Range &Range = testing::get<1>(arg);
+  return std::tie(Pos.Location.Start.Line, Pos.Location.Start.Column,
+  Pos.Location.End.Line, Pos.Location.End.Column) ==
+ std::tie(Range.start.line, Range.start.character, Range.end.line,
+  Range.end.character);
+}
+testing::Matcher &>
+HaveRanges(const std::vector Ranges) {
+  return testing::UnorderedPointwise(OccurrenceRange(), Ranges);
+}
 
 class ShouldCollectSymbolTest : public ::testing::Test {
 public:
@@ -237,6 +250,7 @@
 llvm::MemoryBuffer::getMemBuffer(MainCode));
 Invocation.run();
 Symbols = Factory->Collector->takeSymbols();
+SymbolOccurrences = Factory->Collector->takeOccurrences();
 return true;
   }
 
@@ -247,6 +261,7 @@
   std::string TestFileName;
   std::string TestFileURI;
   SymbolSlab Symbols;
+  SymbolOccurrenceSlab SymbolOccurrences;
   SymbolCollector::Options CollectorOpts;
   std::unique_ptr PragmaHandler;
 };
@@ -413,6 +428,61 @@
   ));
 }
 
+TEST_F(SymbolCollectorTest, Occurrences) {
+  Annotations Header(R"(
+  class $foo[[Foo]] {
+  public:
+$foo[[Foo]]() {}
+$foo[[Foo]](int);
+  };
+  class $bar[[Bar]];
+  void $func[[func]]();
+  )");
+  Annotations Main(R"(
+  class $bar[[Bar]] {};
+
+  void $func[[func]]();
+
+  void fff() {
+$foo[[Foo]] foo;
+$bar[[Bar]] bar;
+$func[[func]]();
+int abc = 0;
+$foo[[Foo]] foo2 = abc;
+  }
+  )");
+  Annotations SymbolsOnlyInMainCode(R"(
+  int a;
+  void b() {}
+  static const int c = 0;
+  class d {};
+  )");
+  CollectorOpts.OccurrenceFilter = SymbolOccurrenceKind::Declaration |
+   SymbolOccurrenceKind::Definition |
+   SymbolOccurrenceKind::Reference;
+  runSymbolCollector(Header.code(),
+ (Main.code() + SymbolsOnlyInMainCode.code()).str());
+  auto HeaderSymbols = TestTU::withHeaderCode(Header.code()).headerSymbols();
+
+  EXPECT_THAT(SymbolOccurrences.find(findSymbol(Symbols, "Foo").ID),
+  HaveRanges(Main.ranges("foo")));
+  EXPECT_THAT(SymbolOccurrences.find(findSymbol(Symbols, "Bar").ID),
+  HaveRanges(Main.ranges("bar")));
+  EXPECT_THAT(SymbolOccurrences.find(findSymbol(Symbols, "func").ID),
+  HaveRanges(Main.ranges("func")));
+
+  // Retrieve IDs for symbols *only* in the main file, and verify these symbols
+  // are not collected.
+  auto MainSymbols =
+  TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
+  EXPECT_THAT(SymbolOccurrences.find(findSymbol(MainSymbols, "a").ID),
+  IsEmpty());
+  EXPECT_THAT(SymbolOccurrences.find(findSymbol(MainSymbols, "b").ID),
+  IsEmpty());
+  EXPECT_THAT(SymbolOccurrences.find(findSymbol(MainSymbols, "c").ID),
+  IsEmpty());
+}
+
 TEST_F(SymbolCollectorTest, References) {
   const std::string Header = R"(
 class W;
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -52,6 +52,10 @@
 const CanonicalIncludes *Includes = nullptr;
 // Populate the Symbol.References field.
 bool CountReferences = false;
+/// The symbol occurrence kind that will be collected.
+/// If not set (Unknown), SymbolCollector will not collect any symbol
+/// occurrences.
+SymbolOccurrenceKind OccurrenceFilter = SymbolOccurrenceKind::Unknown;
 // Every symbol collected will be stamped with this origin.
 SymbolOrigin Origin = SymbolOrigin::Unknown;
 /// Collect macros.
@@ -86,6 +90,10 @@
 
   SymbolSlab takeSymbols() { return std::move(Symbols).build(); }
 
+  SymbolOccurrenceSlab takeOccurrences() {
+return std::move(SymbolOccurrences);
+  }
+
   void finish() override;
 
 private:
@@ -99,15 +

[PATCH] D49986: [ADT] ImmutableList::add parameters are switched

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

Szelethus abandoned this revision.
Szelethus added a comment.

In https://reviews.llvm.org/D49986#1192798, @Szelethus wrote:

> In https://reviews.llvm.org/D49985#1181568, @NoQ wrote:
>
> > In https://reviews.llvm.org/D49985#1181564, @dblaikie wrote:
> >
> > > It looks like concat orders the arguments in the same way that the output 
> > > would be (so concat(X, list) produces [X, list]) - so preserving that 
> > > argument order seems like it improves/retains readability compared to 
> > > switching them around so 'concat(list, X)' produces '[X, list]'.
> >
> >
> > Yeah, i guess that might have been the motivation behind such 
> > inconsistency. I'll be fine with fixing the order for other data structures.
>
>
> @NoQ Have your views changed about this patch? Shall I abandon it?


I've decided to do so after https://reviews.llvm.org/rL340824.


Repository:
  rC Clang

https://reviews.llvm.org/D49986



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

[PATCH] D50385: [clangd] Collect symbol occurrences in SymbolCollector

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clangd/index/Index.h:377
+   llvm::ArrayRef find(const SymbolID &ID) const {
+ auto It = Occurrences.find(ID);
+ if (It == Occurrences.end())

sammccall wrote:
> return Occurrences.lookup(ID)?
The `DenseMap::lookup` returns a copy of `Value` (`vector`) which doesn't suit 
our use case :( -- we will return an `ArrayRef` which stores an reference of a 
local `vector` object.



Comment at: unittests/clangd/SymbolCollectorTests.cpp:466
+  SymbolOccurrences.find(findSymbol(Symbols, "Foo").ID),
+  testing::UnorderedPointwise(OccurrenceRange(), Main.ranges("foo")));
+  EXPECT_THAT(

sammccall wrote:
> this is cute - if possible, consider adding a matcher factory function for 
> readability here, so you can write `EXPECT_THAT(..., 
> HaveRanges(Main.ranges("foo"))`
Wrapped this into `HaveRanges`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50385



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

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

[Simon Atanasyan via Phabricator via cfe-commits]

atanasyan created this revision.
atanasyan added reviewers: echristo, ruiu.

The "-flavor old-gnu" option were introduced to enable old version of LLD ELF 
linker implementation. This option has been removed from the linker since LLD 
3.9. I do not think that there is a real case when the latest version of Clang 
is used in combination with so obsoleted version of LLD linker. Now we can 
remove support of this option from the driver.


Repository:
  rC Clang

https://reviews.llvm.org/D51358

Files:
  lib/Driver/ToolChains/Gnu.cpp


Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -323,14 +323,6 @@
   // handled somewhere else.
   Args.ClaimAllArgs(options::OPT_w);
 
-  const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
-  if (llvm::sys::path::stem(Exec) == "lld") {
-CmdArgs.push_back("-flavor");
-CmdArgs.push_back("old-gnu");
-CmdArgs.push_back("-target");
-CmdArgs.push_back(Args.MakeArgString(getToolChain().getTripleString()));
-  }
-
   if (!D.SysRoot.empty())
 CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
 
@@ -539,6 +531,7 @@
   AddHIPLinkerScript(getToolChain(), C, Output, Inputs, Args, CmdArgs, JA,
  *this);
 
+  const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
 }
 


Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -323,14 +323,6 @@
   // handled somewhere else.
   Args.ClaimAllArgs(options::OPT_w);
 
-  const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
-  if (llvm::sys::path::stem(Exec) == "lld") {
-CmdArgs.push_back("-flavor");
-CmdArgs.push_back("old-gnu");
-CmdArgs.push_back("-target");
-CmdArgs.push_back(Args.MakeArgString(getToolChain().getTripleString()));
-  }
-
   if (!D.SysRoot.empty())
 CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
 
@@ -539,6 +531,7 @@
   AddHIPLinkerScript(getToolChain(), C, Output, Inputs, Args, CmdArgs, JA,
  *this);
 
+  const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

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

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


Repository:
  rC Clang

https://reviews.llvm.org/D51314

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

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

[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

[UTKARSH SAXENA via Phabricator via cfe-commits]

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

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

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

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


Repository:
  rC Clang

https://reviews.llvm.org/D51359

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

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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

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

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


Repository:
  rC Clang

https://reviews.llvm.org/D51314

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

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

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

[Hugo Gonzalez via Phabricator via cfe-commits]

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

made some major updates after no-namespace landed


https://reviews.llvm.org/D50542

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

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

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

[Kirill Bobyrev via Phabricator via cfe-commits]

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

https://reviews.llvm.org/D51352

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


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


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

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

[Kirill Bobyrev via cfe-commits]

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

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

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

Reviewed by: ioeric

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

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

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


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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.



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

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



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

it's always set here, drop the condition.



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

nit: no llvm:: :-)


Repository:
  rC Clang

https://reviews.llvm.org/D51314



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

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

[Kirill Bobyrev via Phabricator via cfe-commits]

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

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

Repository:
  rL LLVM

https://reviews.llvm.org/D51352

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


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


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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Cleanups


Repository:
  rC Clang

https://reviews.llvm.org/D51314

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

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

[PATCH] D51311: (WIP) Type hierarchy

[Simon Marchi via Phabricator via cfe-commits]

simark added inline comments.



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

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



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

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

Did you mean to link to a particular line?

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

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

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

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

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



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

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


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51311



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

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

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

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


https://reviews.llvm.org/D51132



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

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

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

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


https://reviews.llvm.org/D51061



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

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

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



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

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

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



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

nit: remove the empty line.


https://reviews.llvm.org/D50542



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

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

[Hamza Sood via Phabricator via cfe-commits]

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

- Re-uploaded with full context.

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


https://reviews.llvm.org/D51321

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

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

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

[Kirill Bobyrev via Phabricator via cfe-commits]

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

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


https://reviews.llvm.org/D51360

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


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


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

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

[Hugo Gonzalez via Phabricator via cfe-commits]

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



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

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


https://reviews.llvm.org/D50542



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

[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

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

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

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




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

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



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

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



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

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



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

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



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

Accidental whitespace change?



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

s/buffer/Buffer



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

s/to_node/ToNode



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

Accidental whitespace change?


Repository:
  rC Clang

https://reviews.llvm.org/D51359



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

r340838 - Parse compile commands lazily in InterpolatingCompilationDatabase

[Ilya Biryukov via cfe-commits]

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

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

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

Reviewers: sammccall, jfb

Reviewed By: sammccall

Subscribers: mgrang, jfb, cfe-commits

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

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

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

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

[Phabricator via Phabricator via cfe-commits]

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

Repository:
  rL LLVM

https://reviews.llvm.org/D51314

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

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

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

[Anastasia Stulova via Phabricator via cfe-commits]

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

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




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

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


Repository:
  rC Clang

https://reviews.llvm.org/D51296



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

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

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!




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

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


Repository:
  rC Clang

https://reviews.llvm.org/D51302



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

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

[Ilya Biryukov via Phabricator via cfe-commits]

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



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

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

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

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



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

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



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

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


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50438



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

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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Sorry for the delays with this review


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50438



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

[PATCH] D51333: Diagnose likely typos in include statements

[Brian Gesiak via Phabricator via cfe-commits]

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

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


Repository:
  rC Clang

https://reviews.llvm.org/D51333



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

[PATCH] D51311: (WIP) Type hierarchy

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

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




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

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



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

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



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

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



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

Maybe also return all base types for classes?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51311



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

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

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


https://reviews.llvm.org/D50542



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

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

[Hugo Gonzalez via Phabricator via cfe-commits]

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

fixed issues outlines in comments


https://reviews.llvm.org/D50542

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

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

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

[Dan Liew via Phabricator via cfe-commits]

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

LGTM.




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

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


https://reviews.llvm.org/D51239



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

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

[Fangrui Song via cfe-commits]

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

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

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

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

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


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

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

[Eric Liu via Phabricator via cfe-commits]

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

- Addressed review comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51291

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

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -53,7 +53,11 @@
 MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; }
 MATCHER_P(DefURI, P, "") { return arg.Definition.FileURI == P; }
 MATCHER_P(IncludeHeader, P, "") {
-  return arg.Detail && arg.Detail->IncludeHeader == P;
+  return (arg.IncludeHeaders.size() == 1) &&
+ (arg.IncludeHeaders.begin()->IncludeHeader == P);
+}
+MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References,  "") {
+  return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
 }
 MATCHER_P(DeclRange, Pos, "") {
   return std::tie(arg.CanonicalDeclaration.Start.Line,
@@ -696,6 +700,11 @@
 Line: 1
 Column: 1
 IsIndexedForCodeCompletion:true
+IncludeHeaders:
+  - Header:'include1'
+References:7
+  - Header:'include2'
+References:3
 Detail:
   Documentation:'Foo doc'
   ReturnType:'int'
@@ -730,6 +739,11 @@
  Doc("Foo doc"), ReturnType("int"),
  DeclURI("file:///path/foo.h"),
  ForCodeCompletion(true;
+  auto &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/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -243,6 +243,48 @@
   EXPECT_EQ(M.Name, "right");
 }
 
+MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References,  "") {
+  return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
+}
+
+TEST(MergeTest, MergeIncludesOnDifferentDefinitions) {
+  Symbol L, R;
+  L.Name = "left";
+  R.Name = "right";
+  L.ID = R.ID = SymbolID("hello");
+  L.IncludeHeaders.emplace_back("common", 1);
+  R.IncludeHeaders.emplace_back("common", 1);
+  R.IncludeHeaders.emplace_back("new", 1);
+
+  Symbol::Details Scratch;
+
+  // Both have no definition.
+  Symbol M = mergeSymbol(L, R, &Scratch);
+  EXPECT_THAT(M.IncludeHeaders,
+  UnorderedElementsAre(IncludeHeaderWithRef("common", 2u),
+   IncludeHeaderWithRef("new", 1u)));
+
+  // Only merge references of the same includes but do not merge new #includes.
+  L.Definition.FileURI = "file:/left.h";
+  M = mergeSymbol(L, R, &Scratch);
+  EXPECT_THAT(M.IncludeHeaders,
+  UnorderedElementsAre(IncludeHeaderWithRef("common", 2u)));
+
+  // Definitions are the same.
+  R.Definition.FileURI = "file:/right.h";
+  M = mergeSymbol(L, R, &Scratch);
+  EXPECT_THAT(M.IncludeHeaders,
+  UnorderedElementsAre(IncludeHeaderWithRef("common", 2u),
+   IncludeHeaderWithRef("new", 1u)));
+
+  // 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 clan

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

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

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




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

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

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



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

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

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



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

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

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


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51291



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

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

[Raphael Isemann via Phabricator via cfe-commits]

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

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


https://reviews.llvm.org/D43871

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

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

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

[Raphael Isemann via Phabricator via cfe-commits]

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



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

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



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

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


https://reviews.llvm.org/D43871



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

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

[Vedant Kumar via cfe-commits]

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

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

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

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

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

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

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


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

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

[Phabricator via Phabricator via cfe-commits]

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

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

Repository:
  rL LLVM

https://reviews.llvm.org/D51239

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


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


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

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

[Nick Desaulniers via Phabricator via cfe-commits]

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

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


Repository:
  rC Clang

https://reviews.llvm.org/D51190

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


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


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

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

[Akira Hatanaka via cfe-commits]

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

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

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

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


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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

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

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




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

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



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

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



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

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



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

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



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

This seems a bit verbose.

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

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



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

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



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

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



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

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



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

ditto --std



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

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

[Sam McCall via Phabricator via cfe-commits]

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



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

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

isExpansionInMainFile()

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

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



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

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

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

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


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51221



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

r340860 - [libFuzzer] Port to Windows

[Matt Morehouse via cfe-commits]

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

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

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

Patch By: metzman

Reviewers: morehouse, rnk

Reviewed By: morehouse, rnk

Subscribers: morehouse, kcc, eraman

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

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

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


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

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

[Vedant Kumar via Phabricator via cfe-commits]

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

Address the latest round of review feedback.


https://reviews.llvm.org/D50927

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

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

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

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



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

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



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

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


https://reviews.llvm.org/D50927



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

[PATCH] D49244: Always search sysroot for GCC installs

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

rsmith added a comment.

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

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

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

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


Repository:
  rC Clang

https://reviews.llvm.org/D49244



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

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

[Hugo Gonzalez via Phabricator via cfe-commits]

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

renamed check as suggested


https://reviews.llvm.org/D50542

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

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