[PATCH] D66226: [clangd] Skip function bodies inside processed files while indexing

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

This looks great, thanks!




Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:745
 
+bool SymbolCollector::shouldProcessFile(FileID FID) {
+  assert(ASTCtx);

we already have `shouldIndexFile` as an anonymous function inside this file, 
which performs a similar operation with a cache (since `FileFilter` performs 
symlink resolution and a disk read).

Could you make that one a member instead and use it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66226



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

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

[Francois JEAN via Phabricator via cfe-commits]

Wawha updated this revision to Diff 216061.
Wawha added a comment.
Herald added a subscriber: ormris.

I update the patch for the last version of clang format, especially since 
option "AllowShortLambdasOnASingleLine" has been added (which require more 
change for this patch).
Indeed, for AllowShortLambdasOnASingleLine patch 
(https://reviews.llvm.org/D57687), the isAllmanBrace() function has been 
changed (adding TT_LambdaLBrace) which require more test to handle break for 
lambda body.

Unfortunately, there is one case I'm not able to handle correctly for the 
moment.
With option "BeforeLambdaBody : true", "AllowShortLambdasOnASingleLine : 
SLS_All;" and "BinPackParameters : false", the expected code below:

  FctWithLonglineInLambda(
param,
[]()
{
  return 
HereAVeryLongLineThatWillBeFormattedOnMultipleLineAndShouldNotBeConsiderAsInline;
});

Is formatted like this with the current patch:

  FctWithLonglineInLambda(param, []() {
  return 
HereAVeryLongLineThatWillBeFormattedOnMultipleLineAndShouldNotBeConsiderAsInline;
});

Can someone help me and tell where to look to set break for parameters (and 
lambda) when the whole parameters line (included the lambda body, which is not 
the case actually) do not fit the inside one line.
Before the AllowShortLambdasOnASingleLine option, there is no need for that, 
due to the fact that the lambda was always break. But with this new option, we 
need to handle that too.


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

https://reviews.llvm.org/D44609

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



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

[PATCH] D66142: clang: Don't warn on unused momit-leaf-frame-pointer when frame pointers are off.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

LGTM




Comment at: cfe/trunk/test/Driver/frame-pointer-elim.c:31
 
+// fno-omit-frame-pointer -momit-leaf-frame-pointer can be overwritten by
+// fomit-frame-pointer later on the command without warning

Omitted `-`


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66142



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

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/XRefs.cpp:389
 llvm::sort(References, [](const Reference &L, const Reference &R) {
-  return std::tie(L.Loc, L.CanonicalTarget, L.Role) <
- std::tie(R.Loc, R.CanonicalTarget, R.Role);
+  return L.Loc < R.Loc;
 });

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > hokein wrote:
> > > > ilya-biryukov wrote:
> > > > > What are the elements `References` for the problematic case?
> > > > > 
> > > > > If we have duplicate elements, then `sort()` would now give us one of 
> > > > > the items. Which exact `Decl` we're gonna end up seeing is not 
> > > > > well-defined, i.e. it's non-deterministic.
> > > > > What are the elements References for the problematic case?
> > > > 
> > > > The testcase is using declarations (see the existing test) -- we will 
> > > > get 3 refs on `using ::fo^o`, each ref has a different decl.  
> > > > 
> > > > ```
> > > > void [[foo]](int);
> > > > void [[foo]](double);
> > > > 
> > > > namespace ns {
> > > > using ::[[fo^o]];
> > > > }
> > > > ```
> > > > 
> > > > > If we have duplicate elements, then sort() would now give us one of 
> > > > > the items. Which exact Decl we're gonna end up seeing is not 
> > > > > well-defined, i.e. it's non-deterministic.
> > > > 
> > > > I think we could make it deterministic, but only return one refs, WDYT?
> > > > 
> > > > 
> > > To make sure I understand the problem: we used to get 4 references in 
> > > total:
> > > - 2 references to the functions (`foo(int)` and `foo(double)`)
> > > - 2 references pointing to the using declaration, e.g. the `using 
> > > ::[foo]]`
> > > 
> > > After this patch we would remove the duplicate `using ::[[foo]]` from the 
> > > results and get only 3 references as a result.
> > > 
> > > Is that correct?
> > Yes, that is correct.
> Interestingly enough, we always ignore the `CanonicalTarget` in the returned 
> `Reference`.
> 
> Maybe we could remove the `CanonicalTarget` from the `Reference` struct 
> instead?
> That keeps the interface more consistent: clients will always get 
> deterministic results (as there is no `CanonicalDecl`, which is different now)
> Maybe we could remove the CanonicalTarget from the Reference struct instead?

unfortunately, this may not work either because of the `Role` -- we still fail 
on the above sample, the references pointing to the using decl have different 
roles.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66349



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

[PATCH] D66350: Rudimentary support for Doxygen \retval command

[Dmitri Gribenko via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D66350



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

[PATCH] D66226: [clangd] Skip function bodies inside processed files while indexing

[Ilya Biryukov via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:745
 
+bool SymbolCollector::shouldProcessFile(FileID FID) {
+  assert(ASTCtx);

kadircet wrote:
> we already have `shouldIndexFile` as an anonymous function inside this file, 
> which performs a similar operation with a cache (since `FileFilter` performs 
> symlink resolution and a disk read).
> 
> Could you make that one a member instead and use it?
Thanks, did not notice it on the first go. Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66226



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

[PATCH] D66226: [clangd] Skip function bodies inside processed files while indexing

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Expose the existing helper instead of introducing a new one


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66226

Files:
  clang-tools-extra/clangd/index/IndexAction.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h

Index: clang-tools-extra/clangd/index/SymbolCollector.h
===
--- clang-tools-extra/clangd/index/SymbolCollector.h
+++ clang-tools-extra/clangd/index/SymbolCollector.h
@@ -112,6 +112,11 @@
   RefSlab takeRefs() { return std::move(Refs).build(); }
   RelationSlab takeRelations() { return std::move(Relations).build(); }
 
+  /// Returns true if we are interested in references and declarations from \p
+  /// FID. If this function return false, bodies of functions inside those files
+  /// will be skipped to decrease indexing time.
+  bool shouldIndexFile(FileID FID);
+
   void finish() override;
 
 private:
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -147,17 +147,6 @@
   CreatePosition(TokLoc.getLocWithOffset(TokenLength))};
 }
 
-bool shouldIndexFile(const SourceManager &SM, FileID FID,
- const SymbolCollector::Options &Opts,
- llvm::DenseMap *FilesToIndexCache) {
-  if (!Opts.FileFilter)
-return true;
-  auto I = FilesToIndexCache->try_emplace(FID);
-  if (I.second)
-I.first->second = Opts.FileFilter(SM, FID);
-  return I.first->second;
-}
-
 // Return the symbol location of the token at \p TokLoc.
 llvm::Optional
 getTokenLocation(SourceLocation TokLoc, const SourceManager &SM,
@@ -410,7 +399,7 @@
   S.SymInfo = index::getSymbolInfoForMacro(*MI);
   std::string FileURI;
   // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache);
+  shouldIndexFile(SM.getFileID(Loc));
   if (auto DeclLoc =
   getTokenLocation(DefLoc, SM, Opts, PP->getLangOpts(), FileURI))
 S.CanonicalDeclaration = *DeclLoc;
@@ -540,7 +529,7 @@
 for (const auto &LocAndRole : It.second) {
   auto FileID = SM.getFileID(LocAndRole.first);
   // FIXME: use the result to filter out references.
-  shouldIndexFile(SM, FileID, Opts, &FilesToIndexCache);
+  shouldIndexFile(FileID);
   if (auto FileURI = GetURI(FileID)) {
 auto Range =
 getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
@@ -590,7 +579,7 @@
   auto Loc = spellingLocIfSpelled(findName(&ND), SM);
   assert(Loc.isValid() && "Invalid source location for NamedDecl");
   // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache);
+  shouldIndexFile(SM.getFileID(Loc));
   if (auto DeclLoc =
   getTokenLocation(Loc, SM, Opts, ASTCtx->getLangOpts(), FileURI))
 S.CanonicalDeclaration = *DeclLoc;
@@ -650,7 +639,7 @@
   const auto &SM = ND.getASTContext().getSourceManager();
   auto Loc = spellingLocIfSpelled(findName(&ND), SM);
   // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache);
+  shouldIndexFile(SM.getFileID(Loc));
   if (auto DefLoc =
   getTokenLocation(Loc, SM, Opts, ASTCtx->getLangOpts(), FileURI))
 S.Definition = *DefLoc;
@@ -742,5 +731,14 @@
   return false;
 }
 
+bool SymbolCollector::shouldIndexFile(FileID FID) {
+  if (!Opts.FileFilter)
+return true;
+  auto I = FilesToIndexCache.try_emplace(FID);
+  if (I.second)
+I.first->second = Opts.FileFilter(ASTCtx->getSourceManager(), FID);
+  return I.first->second;
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/IndexAction.cpp
===
--- clang-tools-extra/clangd/index/IndexAction.cpp
+++ clang-tools-extra/clangd/index/IndexAction.cpp
@@ -11,9 +11,17 @@
 #include "Logger.h"
 #include "index/Relation.h"
 #include "index/SymbolOrigin.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/MultiplexConsumer.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/STLExtras.h"
+#include 
+#include 
 #include 
 
 namespace clang {
@@ -113,6 +121,40 @@
   IncludeGraph &IG;
 };
 
+/// Returns an ASTConsumer that wraps \p Inner and additionally instructs the
+/// parser to skip bodies of functions in the files that should not be
+/// processed.
+static std::unique_ptr
+skipProcessedFuncti

r369345 - Rudimentary support for Doxygen \retval command

[Stephan Bergmann via cfe-commits]

Author: sberg
Date: Tue Aug 20 01:36:21 2019
New Revision: 369345

URL: http://llvm.org/viewvc/llvm-project?rev=369345&view=rev
Log:
Rudimentary support for Doxygen \retval command

...so that at least a preceding \param etc. that lacks a description gets a
-Wdocumentation warning (instead of erroneously treating the \retval ... text as
its paragraph).

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

Modified:
cfe/trunk/include/clang/AST/CommentCommands.td
cfe/trunk/test/Sema/warn-documentation.cpp

Modified: cfe/trunk/include/clang/AST/CommentCommands.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CommentCommands.td?rev=369345&r1=369344&r2=369345&view=diff
==
--- cfe/trunk/include/clang/AST/CommentCommands.td (original)
+++ cfe/trunk/include/clang/AST/CommentCommands.td Tue Aug 20 01:36:21 2019
@@ -139,6 +139,7 @@ def Post   : BlockCommand<"post">;
 def Pre: BlockCommand<"pre">;
 def Remark : BlockCommand<"remark">;
 def Remarks: BlockCommand<"remarks">;
+def Retval : BlockCommand<"retval">;
 def Sa : BlockCommand<"sa">;
 def See: BlockCommand<"see">;
 def Since  : BlockCommand<"since">;

Modified: cfe/trunk/test/Sema/warn-documentation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-documentation.cpp?rev=369345&r1=369344&r2=369345&view=diff
==
--- cfe/trunk/test/Sema/warn-documentation.cpp (original)
+++ cfe/trunk/test/Sema/warn-documentation.cpp Tue Aug 20 01:36:21 2019
@@ -288,6 +288,11 @@ int test_param21(int a);
 /// \param x2 Ccc.
 int test_param22(int x1, int x2, int x3);
 
+// expected-warning@+1 {{empty paragraph passed to '\param' command}}
+/// \param a
+/// \retval 0 Blah blah.
+int test_param23(int a);
+
 //===---
 // Test that we treat typedefs to some non-function types as functions for the
 // purposes of documentation comment parsing.


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

[PATCH] D66350: Rudimentary support for Doxygen \retval command

[Stephan Bergmann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369345: Rudimentary support for Doxygen \retval command 
(authored by sberg, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66350?vs=215599&id=216069#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66350

Files:
  cfe/trunk/include/clang/AST/CommentCommands.td
  cfe/trunk/test/Sema/warn-documentation.cpp


Index: cfe/trunk/include/clang/AST/CommentCommands.td
===
--- cfe/trunk/include/clang/AST/CommentCommands.td
+++ cfe/trunk/include/clang/AST/CommentCommands.td
@@ -139,6 +139,7 @@
 def Pre: BlockCommand<"pre">;
 def Remark : BlockCommand<"remark">;
 def Remarks: BlockCommand<"remarks">;
+def Retval : BlockCommand<"retval">;
 def Sa : BlockCommand<"sa">;
 def See: BlockCommand<"see">;
 def Since  : BlockCommand<"since">;
Index: cfe/trunk/test/Sema/warn-documentation.cpp
===
--- cfe/trunk/test/Sema/warn-documentation.cpp
+++ cfe/trunk/test/Sema/warn-documentation.cpp
@@ -288,6 +288,11 @@
 /// \param x2 Ccc.
 int test_param22(int x1, int x2, int x3);
 
+// expected-warning@+1 {{empty paragraph passed to '\param' command}}
+/// \param a
+/// \retval 0 Blah blah.
+int test_param23(int a);
+
 //===---
 // Test that we treat typedefs to some non-function types as functions for the
 // purposes of documentation comment parsing.


Index: cfe/trunk/include/clang/AST/CommentCommands.td
===
--- cfe/trunk/include/clang/AST/CommentCommands.td
+++ cfe/trunk/include/clang/AST/CommentCommands.td
@@ -139,6 +139,7 @@
 def Pre: BlockCommand<"pre">;
 def Remark : BlockCommand<"remark">;
 def Remarks: BlockCommand<"remarks">;
+def Retval : BlockCommand<"retval">;
 def Sa : BlockCommand<"sa">;
 def See: BlockCommand<"see">;
 def Since  : BlockCommand<"since">;
Index: cfe/trunk/test/Sema/warn-documentation.cpp
===
--- cfe/trunk/test/Sema/warn-documentation.cpp
+++ cfe/trunk/test/Sema/warn-documentation.cpp
@@ -288,6 +288,11 @@
 /// \param x2 Ccc.
 int test_param22(int x1, int x2, int x3);
 
+// expected-warning@+1 {{empty paragraph passed to '\param' command}}
+/// \param a
+/// \retval 0 Blah blah.
+int test_param23(int a);
+
 //===---
 // Test that we treat typedefs to some non-function types as functions for the
 // purposes of documentation comment parsing.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66226: [clangd] Skip function bodies inside processed files while indexing

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/index/IndexAction.cpp:194
 CI.getDiagnostics().setIgnoreAllWarnings(true);
+/// Instruct the parser to ask our ASTConsumer if it should skip function
+/// bodies. The ASTConsumer will take care of skipping only functions 
inside

nit: can you make these non-doxygen(just two slashes)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66226



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

[PATCH] D66270: [clang-tidy] Migrate objc-super-self to use isDerivedFrom 🚛

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

LGTM, thanks!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66270



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

[PATCH] D66269: [clang-tidy] Migrate objc-forbidden-subclassing to use isDerivedFrom 🚛

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

LGTM, thanks!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66269



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

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:389
 llvm::sort(References, [](const Reference &L, const Reference &R) {
-  return std::tie(L.Loc, L.CanonicalTarget, L.Role) <
- std::tie(R.Loc, R.CanonicalTarget, R.Role);
+  return L.Loc < R.Loc;
 });

hokein wrote:
> ilya-biryukov wrote:
> > hokein wrote:
> > > ilya-biryukov wrote:
> > > > hokein wrote:
> > > > > ilya-biryukov wrote:
> > > > > > What are the elements `References` for the problematic case?
> > > > > > 
> > > > > > If we have duplicate elements, then `sort()` would now give us one 
> > > > > > of the items. Which exact `Decl` we're gonna end up seeing is not 
> > > > > > well-defined, i.e. it's non-deterministic.
> > > > > > What are the elements References for the problematic case?
> > > > > 
> > > > > The testcase is using declarations (see the existing test) -- we will 
> > > > > get 3 refs on `using ::fo^o`, each ref has a different decl.  
> > > > > 
> > > > > ```
> > > > > void [[foo]](int);
> > > > > void [[foo]](double);
> > > > > 
> > > > > namespace ns {
> > > > > using ::[[fo^o]];
> > > > > }
> > > > > ```
> > > > > 
> > > > > > If we have duplicate elements, then sort() would now give us one of 
> > > > > > the items. Which exact Decl we're gonna end up seeing is not 
> > > > > > well-defined, i.e. it's non-deterministic.
> > > > > 
> > > > > I think we could make it deterministic, but only return one refs, 
> > > > > WDYT?
> > > > > 
> > > > > 
> > > > To make sure I understand the problem: we used to get 4 references in 
> > > > total:
> > > > - 2 references to the functions (`foo(int)` and `foo(double)`)
> > > > - 2 references pointing to the using declaration, e.g. the `using 
> > > > ::[foo]]`
> > > > 
> > > > After this patch we would remove the duplicate `using ::[[foo]]` from 
> > > > the results and get only 3 references as a result.
> > > > 
> > > > Is that correct?
> > > Yes, that is correct.
> > Interestingly enough, we always ignore the `CanonicalTarget` in the 
> > returned `Reference`.
> > 
> > Maybe we could remove the `CanonicalTarget` from the `Reference` struct 
> > instead?
> > That keeps the interface more consistent: clients will always get 
> > deterministic results (as there is no `CanonicalDecl`, which is different 
> > now)
> > Maybe we could remove the CanonicalTarget from the Reference struct instead?
> 
> unfortunately, this may not work either because of the `Role` -- we still 
> fail on the above sample, the references pointing to the using decl have 
> different roles.
We only look at the role in `DocumentHighlights` to determine the 
`DocumentHighlightKind` (`Read`, `Write` or `Text`).
I suggest we do this earlier and replace the `Reference::Role`  field with 
`Reference::DocumentHighlightKind`.
Then we can de-duplicate here and keep deterministic interface.

If we feel that's the wrong layering (we are putting LSP-specific things into 
Reference, which only has clang-specific types stuff now), we could move 
de-duplication downstream to `findDocumentHighlights` and `findRefs`. They 
return `Loc` and `DocumentHighlighting` and can safely de-duplicate on those 
without changing observable results.

Looks like replacing `Role` with `HighlightingKind` is the simplest option, 
though. And I don't think this breaks layering that much, it's an 
implementation detail of cross-references anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66349



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

[PATCH] D66226: [clangd] Skip function bodies inside processed files while indexing

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 216073.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

- Change /// to //


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66226

Files:
  clang-tools-extra/clangd/index/IndexAction.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h

Index: clang-tools-extra/clangd/index/SymbolCollector.h
===
--- clang-tools-extra/clangd/index/SymbolCollector.h
+++ clang-tools-extra/clangd/index/SymbolCollector.h
@@ -112,6 +112,11 @@
   RefSlab takeRefs() { return std::move(Refs).build(); }
   RelationSlab takeRelations() { return std::move(Relations).build(); }
 
+  /// Returns true if we are interested in references and declarations from \p
+  /// FID. If this function return false, bodies of functions inside those files
+  /// will be skipped to decrease indexing time.
+  bool shouldIndexFile(FileID FID);
+
   void finish() override;
 
 private:
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -147,17 +147,6 @@
   CreatePosition(TokLoc.getLocWithOffset(TokenLength))};
 }
 
-bool shouldIndexFile(const SourceManager &SM, FileID FID,
- const SymbolCollector::Options &Opts,
- llvm::DenseMap *FilesToIndexCache) {
-  if (!Opts.FileFilter)
-return true;
-  auto I = FilesToIndexCache->try_emplace(FID);
-  if (I.second)
-I.first->second = Opts.FileFilter(SM, FID);
-  return I.first->second;
-}
-
 // Return the symbol location of the token at \p TokLoc.
 llvm::Optional
 getTokenLocation(SourceLocation TokLoc, const SourceManager &SM,
@@ -410,7 +399,7 @@
   S.SymInfo = index::getSymbolInfoForMacro(*MI);
   std::string FileURI;
   // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache);
+  shouldIndexFile(SM.getFileID(Loc));
   if (auto DeclLoc =
   getTokenLocation(DefLoc, SM, Opts, PP->getLangOpts(), FileURI))
 S.CanonicalDeclaration = *DeclLoc;
@@ -540,7 +529,7 @@
 for (const auto &LocAndRole : It.second) {
   auto FileID = SM.getFileID(LocAndRole.first);
   // FIXME: use the result to filter out references.
-  shouldIndexFile(SM, FileID, Opts, &FilesToIndexCache);
+  shouldIndexFile(FileID);
   if (auto FileURI = GetURI(FileID)) {
 auto Range =
 getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
@@ -590,7 +579,7 @@
   auto Loc = spellingLocIfSpelled(findName(&ND), SM);
   assert(Loc.isValid() && "Invalid source location for NamedDecl");
   // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache);
+  shouldIndexFile(SM.getFileID(Loc));
   if (auto DeclLoc =
   getTokenLocation(Loc, SM, Opts, ASTCtx->getLangOpts(), FileURI))
 S.CanonicalDeclaration = *DeclLoc;
@@ -650,7 +639,7 @@
   const auto &SM = ND.getASTContext().getSourceManager();
   auto Loc = spellingLocIfSpelled(findName(&ND), SM);
   // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache);
+  shouldIndexFile(SM.getFileID(Loc));
   if (auto DefLoc =
   getTokenLocation(Loc, SM, Opts, ASTCtx->getLangOpts(), FileURI))
 S.Definition = *DefLoc;
@@ -742,5 +731,14 @@
   return false;
 }
 
+bool SymbolCollector::shouldIndexFile(FileID FID) {
+  if (!Opts.FileFilter)
+return true;
+  auto I = FilesToIndexCache.try_emplace(FID);
+  if (I.second)
+I.first->second = Opts.FileFilter(ASTCtx->getSourceManager(), FID);
+  return I.first->second;
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/IndexAction.cpp
===
--- clang-tools-extra/clangd/index/IndexAction.cpp
+++ clang-tools-extra/clangd/index/IndexAction.cpp
@@ -11,9 +11,17 @@
 #include "Logger.h"
 #include "index/Relation.h"
 #include "index/SymbolOrigin.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/MultiplexConsumer.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/STLExtras.h"
+#include 
+#include 
 #include 
 
 namespace clang {
@@ -113,6 +121,40 @@
   IncludeGraph &IG;
 };
 
+/// Returns an ASTConsumer that wraps \p Inner and additionally instructs the
+/// parser to skip bodies of functions in the files that should not be
+/// processed.
+static std::unique_ptr
+skipProcessedF

[clang-tools-extra] r369349 - [clangd] Skip function bodies inside processed files while indexing

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Aug 20 01:54:30 2019
New Revision: 369349

URL: http://llvm.org/viewvc/llvm-project?rev=369349&view=rev
Log:
[clangd] Skip function bodies inside processed files while indexing

Summary:
This significantly improves performance of background indexing.

We do not collect references and declarations inside the processed
files, so this does not affect the final indexing results.

The idea is borrowed from libclang, which has a similar optimization in
its indexing functionality.

Measurements show a nice decrease in indexing time, up to ~40% for
building the whole index. These are not proper benchmarks, so one should
not rely on these results too much.

1. Rebuilding the whole index for LLVM:
  - Before. Total time: 14m58s.
./bin/clangd -pch-storage=memory < ./clangd.input  23917.67s user 515.86s 
system 2718% cpu 14:58.68 total
  - After. Total time: 8m41s.
./bin/clangd -pch-storage=memory < ./clangd.input  13627.29s user 288.10s 
system 2672% cpu 8:40.67 total

2. Rebuilding index after removing shards matching '*clangd*' 
(case-insensitively):
  - Before. Total time: 30s.
./bin/clangd -pch-storage=memory < ./clangd.input  130.94s user 6.82s 
system 452% cpu 30.423 total
  - After. Total time: 26s.
./bin/clangd -pch-storage=memory < ./clangd.input  80.51s user 5.40s system 
333% cpu 25.777 total

Reviewers: kadircet, sammccall

Reviewed By: kadircet

Subscribers: MaskRay, jkorous, arphaman, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/index/IndexAction.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.h

Modified: clang-tools-extra/trunk/clangd/index/IndexAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/IndexAction.cpp?rev=369349&r1=369348&r2=369349&view=diff
==
--- clang-tools-extra/trunk/clangd/index/IndexAction.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/IndexAction.cpp Tue Aug 20 01:54:30 
2019
@@ -11,9 +11,17 @@
 #include "Logger.h"
 #include "index/Relation.h"
 #include "index/SymbolOrigin.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/MultiplexConsumer.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/STLExtras.h"
+#include 
+#include 
 #include 
 
 namespace clang {
@@ -113,6 +121,40 @@ private:
   IncludeGraph &IG;
 };
 
+/// Returns an ASTConsumer that wraps \p Inner and additionally instructs the
+/// parser to skip bodies of functions in the files that should not be
+/// processed.
+static std::unique_ptr
+skipProcessedFunctions(std::unique_ptr Inner,
+   std::function ShouldIndexFile) {
+  class SkipProcessedFunctions : public ASTConsumer {
+  public:
+SkipProcessedFunctions(std::function FileFilter)
+: ShouldIndexFile(std::move(FileFilter)), Context(nullptr) {
+  assert(this->ShouldIndexFile);
+}
+
+void Initialize(ASTContext &Context) override { this->Context = &Context; }
+bool shouldSkipFunctionBody(Decl *D) override {
+  assert(Context && "Initialize() was never called.");
+  auto &SM = Context->getSourceManager();
+  auto FID = SM.getFileID(SM.getExpansionLoc(D->getLocation()));
+  if (!FID.isValid())
+return false;
+  return !ShouldIndexFile(FID);
+}
+
+  private:
+std::function ShouldIndexFile;
+const ASTContext *Context;
+  };
+  std::vector> Consumers;
+  Consumers.push_back(
+  std::make_unique(ShouldIndexFile));
+  Consumers.push_back(std::move(Inner));
+  return std::make_unique(std::move(Consumers));
+}
+
 // Wraps the index action and reports index data after each translation unit.
 class IndexAction : public WrapperFrontendAction {
 public:
@@ -137,7 +179,9 @@ public:
 if (IncludeGraphCallback != nullptr)
   CI.getPreprocessor().addPPCallbacks(
   std::make_unique(CI.getSourceManager(), IG));
-return WrapperFrontendAction::CreateASTConsumer(CI, InFile);
+return skipProcessedFunctions(
+WrapperFrontendAction::CreateASTConsumer(CI, InFile),
+[this](FileID FID) { return Collector->shouldIndexFile(FID); });
   }
 
   bool BeginInvocation(CompilerInstance &CI) override {
@@ -147,6 +191,10 @@ public:
 // Avoids some analyses too. Set in two places as we're late to the party.
 CI.getDiagnosticOpts().IgnoreWarnings = true;
 CI.getDiagnostics().setIgnoreAllWarnings(true);
+// Instruct the parser to ask our ASTConsumer if it should skip function
+// bodies. The ASTConsumer will take care of skipping only functions inside
+// the files that we have already processed.
+CI.getFrontendOpts(

[PATCH] D66226: [clangd] Skip function bodies inside processed files while indexing

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369349: [clangd] Skip function bodies inside processed files 
while indexing (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66226?vs=216073&id=216075#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66226

Files:
  clang-tools-extra/trunk/clangd/index/IndexAction.cpp
  clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
  clang-tools-extra/trunk/clangd/index/SymbolCollector.h

Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
@@ -147,17 +147,6 @@
   CreatePosition(TokLoc.getLocWithOffset(TokenLength))};
 }
 
-bool shouldIndexFile(const SourceManager &SM, FileID FID,
- const SymbolCollector::Options &Opts,
- llvm::DenseMap *FilesToIndexCache) {
-  if (!Opts.FileFilter)
-return true;
-  auto I = FilesToIndexCache->try_emplace(FID);
-  if (I.second)
-I.first->second = Opts.FileFilter(SM, FID);
-  return I.first->second;
-}
-
 // Return the symbol location of the token at \p TokLoc.
 llvm::Optional
 getTokenLocation(SourceLocation TokLoc, const SourceManager &SM,
@@ -410,7 +399,7 @@
   S.SymInfo = index::getSymbolInfoForMacro(*MI);
   std::string FileURI;
   // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache);
+  shouldIndexFile(SM.getFileID(Loc));
   if (auto DeclLoc =
   getTokenLocation(DefLoc, SM, Opts, PP->getLangOpts(), FileURI))
 S.CanonicalDeclaration = *DeclLoc;
@@ -540,7 +529,7 @@
 for (const auto &LocAndRole : It.second) {
   auto FileID = SM.getFileID(LocAndRole.first);
   // FIXME: use the result to filter out references.
-  shouldIndexFile(SM, FileID, Opts, &FilesToIndexCache);
+  shouldIndexFile(FileID);
   if (auto FileURI = GetURI(FileID)) {
 auto Range =
 getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
@@ -590,7 +579,7 @@
   auto Loc = spellingLocIfSpelled(findName(&ND), SM);
   assert(Loc.isValid() && "Invalid source location for NamedDecl");
   // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache);
+  shouldIndexFile(SM.getFileID(Loc));
   if (auto DeclLoc =
   getTokenLocation(Loc, SM, Opts, ASTCtx->getLangOpts(), FileURI))
 S.CanonicalDeclaration = *DeclLoc;
@@ -650,7 +639,7 @@
   const auto &SM = ND.getASTContext().getSourceManager();
   auto Loc = spellingLocIfSpelled(findName(&ND), SM);
   // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache);
+  shouldIndexFile(SM.getFileID(Loc));
   if (auto DefLoc =
   getTokenLocation(Loc, SM, Opts, ASTCtx->getLangOpts(), FileURI))
 S.Definition = *DefLoc;
@@ -742,5 +731,14 @@
   return false;
 }
 
+bool SymbolCollector::shouldIndexFile(FileID FID) {
+  if (!Opts.FileFilter)
+return true;
+  auto I = FilesToIndexCache.try_emplace(FID);
+  if (I.second)
+I.first->second = Opts.FileFilter(ASTCtx->getSourceManager(), FID);
+  return I.first->second;
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.h
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.h
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h
@@ -112,6 +112,11 @@
   RefSlab takeRefs() { return std::move(Refs).build(); }
   RelationSlab takeRelations() { return std::move(Relations).build(); }
 
+  /// Returns true if we are interested in references and declarations from \p
+  /// FID. If this function return false, bodies of functions inside those files
+  /// will be skipped to decrease indexing time.
+  bool shouldIndexFile(FileID FID);
+
   void finish() override;
 
 private:
Index: clang-tools-extra/trunk/clangd/index/IndexAction.cpp
===
--- clang-tools-extra/trunk/clangd/index/IndexAction.cpp
+++ clang-tools-extra/trunk/clangd/index/IndexAction.cpp
@@ -11,9 +11,17 @@
 #include "Logger.h"
 #include "index/Relation.h"
 #include "index/SymbolOrigin.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/MultiplexConsumer.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/STLExtras.h"
+#include 
+#include 
 #include 
 
 nam

[PATCH] D66294: [Docs][OpenCL] Release 9.0 notes for OpenCL

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans added a comment.

lgtm, and many thanks for writing release notes!

Go ahead and commit to the branch directly with SVN or let me know if you'd 
like me to do it.


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

https://reviews.llvm.org/D66294



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

[PATCH] D66394: clang-cl: Enable /Zc:twoPhase by default if targeting MSVC 2017 update 3 or newer

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans added a comment.

Nice! Please include a mention in docs/ReleaseNotes.rst :-)


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

https://reviews.llvm.org/D66394



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

[PATCH] D66462: Removed the 'id' AST matcher, which is superseded by '.bind()'

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The 'id' matcher is not even included in the AST Matchers Reference
document, so I don't expect there to be a significant number of users.

There's no reason to provide two ways to do the exact same thing that
only have a minor syntactic difference.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66462

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/unittests/Tooling/RefactoringCallbacksTest.cpp

Index: clang/unittests/Tooling/RefactoringCallbacksTest.cpp
===
--- clang/unittests/Tooling/RefactoringCallbacksTest.cpp
+++ clang/unittests/Tooling/RefactoringCallbacksTest.cpp
@@ -38,28 +38,28 @@
   std::string Code = "void f() { int i = 1; }";
   std::string Expected = "void f() { ; }";
   ReplaceStmtWithText Callback("id", ";");
-  expectRewritten(Code, Expected, id("id", declStmt()), Callback);
+  expectRewritten(Code, Expected, declStmt().bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, ReplacesStmtsInCalledMacros) {
   std::string Code = "#define A void f() { int i = 1; }\nA";
   std::string Expected = "#define A void f() { ; }\nA";
   ReplaceStmtWithText Callback("id", ";");
-  expectRewritten(Code, Expected, id("id", declStmt()), Callback);
+  expectRewritten(Code, Expected, declStmt().bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, IgnoresStmtsInUncalledMacros) {
   std::string Code = "#define A void f() { int i = 1; }";
   std::string Expected = "#define A void f() { int i = 1; }";
   ReplaceStmtWithText Callback("id", ";");
-  expectRewritten(Code, Expected, id("id", declStmt()), Callback);
+  expectRewritten(Code, Expected, declStmt().bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, ReplacesInteger) {
   std::string Code = "void f() { int i = 1; }";
   std::string Expected = "void f() { int i = 2; }";
   ReplaceStmtWithText Callback("id", "2");
-  expectRewritten(Code, Expected, id("id", expr(integerLiteral())), Callback);
+  expectRewritten(Code, Expected, expr(integerLiteral()).bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, ReplacesStmtWithStmt) {
@@ -68,9 +68,9 @@
   ReplaceStmtWithStmt Callback("always-false", "should-be");
   expectRewritten(
   Code, Expected,
-  id("always-false",
- conditionalOperator(hasCondition(cxxBoolLiteral(equals(false))),
- hasFalseExpression(id("should-be", expr(),
+  conditionalOperator(hasCondition(cxxBoolLiteral(equals(false))),
+  hasFalseExpression(expr().bind("should-be")))
+  .bind("always-false"),
   Callback);
 }
 
@@ -78,20 +78,20 @@
   std::string Code = "bool a; void f() { if (a) f(); else a = true; }";
   std::string Expected = "bool a; void f() { f(); }";
   ReplaceIfStmtWithItsBody Callback("id", true);
-  expectRewritten(
-  Code, Expected,
-  id("id", ifStmt(hasCondition(implicitCastExpr(hasSourceExpression(
-   declRefExpr(to(varDecl(hasName("a"),
-  Callback);
+  expectRewritten(Code, Expected,
+  ifStmt(hasCondition(implicitCastExpr(hasSourceExpression(
+ declRefExpr(to(varDecl(hasName("a"
+  .bind("id"),
+  Callback);
 }
 
 TEST(RefactoringCallbacksTest, RemovesEntireIfOnEmptyElse) {
   std::string Code = "void f() { if (false) int i = 0; }";
   std::string Expected = "void f() {  }";
   ReplaceIfStmtWithItsBody Callback("id", false);
-  expectRewritten(Code, Expected,
-  id("id", ifStmt(hasCondition(cxxBoolLiteral(equals(false),
-  Callback);
+  expectRewritten(
+  Code, Expected,
+  ifStmt(hasCondition(cxxBoolLiteral(equals(false.bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, TemplateJustText) {
@@ -99,7 +99,7 @@
   std::string Expected = "void f() { FOO }";
   auto Callback = ReplaceNodeWithTemplate::create("id", "FOO");
   EXPECT_FALSE(Callback.takeError());
-  expectRewritten(Code, Expected, id("id", declStmt()), **Callback);
+  expectRewritten(Code, Expected, declStmt().bind("id"), **Callback);
 }
 
 TEST(RefactoringCallbacksTest, TemplateSimpleSubst) {
@@ -108,7 +108,7 @@
   auto Callback = ReplaceNodeWithTemplate::create("decl", "long x = ${init}");
   EXPECT_FALSE(Callback.takeError());
   expectRewritten(Code, Expected,
-  id("decl", varDecl(hasInitializer(id("init", expr(),
+  varDecl(hasInitializer(expr().bind("init"))).bind("decl"),
   **Callback);
 }
 
@@ -119,7 +119,7 @@
   "string x = \"$$-${init}\"");
   EXPECT_FALSE(Callback.takeError());
   expectRewritten(Code, Expected,
-  id("decl", varDecl(hasInitializer(id("init", expr(),
+  varDecl(hasInitializer(expr().bind("init"))).bind("de

[PATCH] D66462: Removed the 'id' AST matcher, which is superseded by '.bind()'

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr updated this revision to Diff 216082.
gribozavr added a comment.

Fixed a typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66462

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/unittests/Tooling/RefactoringCallbacksTest.cpp

Index: clang/unittests/Tooling/RefactoringCallbacksTest.cpp
===
--- clang/unittests/Tooling/RefactoringCallbacksTest.cpp
+++ clang/unittests/Tooling/RefactoringCallbacksTest.cpp
@@ -38,28 +38,28 @@
   std::string Code = "void f() { int i = 1; }";
   std::string Expected = "void f() { ; }";
   ReplaceStmtWithText Callback("id", ";");
-  expectRewritten(Code, Expected, id("id", declStmt()), Callback);
+  expectRewritten(Code, Expected, declStmt().bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, ReplacesStmtsInCalledMacros) {
   std::string Code = "#define A void f() { int i = 1; }\nA";
   std::string Expected = "#define A void f() { ; }\nA";
   ReplaceStmtWithText Callback("id", ";");
-  expectRewritten(Code, Expected, id("id", declStmt()), Callback);
+  expectRewritten(Code, Expected, declStmt().bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, IgnoresStmtsInUncalledMacros) {
   std::string Code = "#define A void f() { int i = 1; }";
   std::string Expected = "#define A void f() { int i = 1; }";
   ReplaceStmtWithText Callback("id", ";");
-  expectRewritten(Code, Expected, id("id", declStmt()), Callback);
+  expectRewritten(Code, Expected, declStmt().bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, ReplacesInteger) {
   std::string Code = "void f() { int i = 1; }";
   std::string Expected = "void f() { int i = 2; }";
   ReplaceStmtWithText Callback("id", "2");
-  expectRewritten(Code, Expected, id("id", expr(integerLiteral())), Callback);
+  expectRewritten(Code, Expected, expr(integerLiteral()).bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, ReplacesStmtWithStmt) {
@@ -68,9 +68,9 @@
   ReplaceStmtWithStmt Callback("always-false", "should-be");
   expectRewritten(
   Code, Expected,
-  id("always-false",
- conditionalOperator(hasCondition(cxxBoolLiteral(equals(false))),
- hasFalseExpression(id("should-be", expr(),
+  conditionalOperator(hasCondition(cxxBoolLiteral(equals(false))),
+  hasFalseExpression(expr().bind("should-be")))
+  .bind("always-false"),
   Callback);
 }
 
@@ -78,20 +78,20 @@
   std::string Code = "bool a; void f() { if (a) f(); else a = true; }";
   std::string Expected = "bool a; void f() { f(); }";
   ReplaceIfStmtWithItsBody Callback("id", true);
-  expectRewritten(
-  Code, Expected,
-  id("id", ifStmt(hasCondition(implicitCastExpr(hasSourceExpression(
-   declRefExpr(to(varDecl(hasName("a"),
-  Callback);
+  expectRewritten(Code, Expected,
+  ifStmt(hasCondition(implicitCastExpr(hasSourceExpression(
+ declRefExpr(to(varDecl(hasName("a"
+  .bind("id"),
+  Callback);
 }
 
 TEST(RefactoringCallbacksTest, RemovesEntireIfOnEmptyElse) {
   std::string Code = "void f() { if (false) int i = 0; }";
   std::string Expected = "void f() {  }";
   ReplaceIfStmtWithItsBody Callback("id", false);
-  expectRewritten(Code, Expected,
-  id("id", ifStmt(hasCondition(cxxBoolLiteral(equals(false),
-  Callback);
+  expectRewritten(
+  Code, Expected,
+  ifStmt(hasCondition(cxxBoolLiteral(equals(false.bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, TemplateJustText) {
@@ -99,7 +99,7 @@
   std::string Expected = "void f() { FOO }";
   auto Callback = ReplaceNodeWithTemplate::create("id", "FOO");
   EXPECT_FALSE(Callback.takeError());
-  expectRewritten(Code, Expected, id("id", declStmt()), **Callback);
+  expectRewritten(Code, Expected, declStmt().bind("id"), **Callback);
 }
 
 TEST(RefactoringCallbacksTest, TemplateSimpleSubst) {
@@ -108,7 +108,7 @@
   auto Callback = ReplaceNodeWithTemplate::create("decl", "long x = ${init}");
   EXPECT_FALSE(Callback.takeError());
   expectRewritten(Code, Expected,
-  id("decl", varDecl(hasInitializer(id("init", expr(),
+  varDecl(hasInitializer(expr().bind("init"))).bind("decl"),
   **Callback);
 }
 
@@ -119,7 +119,7 @@
   "string x = \"$$-${init}\"");
   EXPECT_FALSE(Callback.takeError());
   expectRewritten(Code, Expected,
-  id("decl", varDecl(hasInitializer(id("init", expr(),
+  varDecl(hasInitializer(expr().bind("init"))).bind("decl"),
   **Callback);
 }
 
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers

[PATCH] D66462: Removed the 'id' AST matcher, which is superseded by '.bind()'

[Manuel Klimek via Phabricator via cfe-commits]

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

Yay, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66462



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

Re: r369043 - [Sema] Implement DR2386 for C++17 structured binding

[Hans Wennborg via cfe-commits]

Merged to release_90 in r369361.

On Thu, Aug 15, 2019 at 9:44 PM Reid Kleckner via cfe-commits
 wrote:
>
> Author: rnk
> Date: Thu Aug 15 12:45:28 2019
> New Revision: 369043
>
> URL: http://llvm.org/viewvc/llvm-project?rev=369043&view=rev
> Log:
> [Sema] Implement DR2386 for C++17 structured binding
>
> Allow implementations to provide complete definitions of
> std::tuple_size, but to omit the 'value' member to signal that T is
> not tuple-like. The Microsoft standard library implements
> std::tuple_size this way.
>
> If the value member exists, clang still validates that it is an ICE, but
> if it does not, then the type is considered to not be tuple-like.
>
> Fixes PR33236
>
> Reviewers: rsmith
>
> Differential Revision: https://reviews.llvm.org/D66040
>
> Modified:
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p3.cpp
> cfe/trunk/test/CXX/drs/dr23xx.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=369043&r1=369042&r2=369043&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Aug 15 12:45:28 2019
> @@ -1030,8 +1030,10 @@ static IsTupleLike isTupleLike(Sema &S,
>TemplateArgumentListInfo Args(Loc, Loc);
>Args.addArgument(getTrivialTypeTemplateArgument(S, Loc, T));
>
> -  // If there's no tuple_size specialization, it's not tuple-like.
> -  if (lookupStdTypeTraitMember(S, R, Loc, "tuple_size", Args, /*DiagID*/0))
> +  // If there's no tuple_size specialization or the lookup of 'value' is 
> empty,
> +  // it's not tuple-like.
> +  if (lookupStdTypeTraitMember(S, R, Loc, "tuple_size", Args, /*DiagID*/ 0) 
> ||
> +  R.empty())
>  return IsTupleLike::NotTupleLike;
>
>// If we get this far, we've committed to the tuple interpretation, but
> @@ -1048,11 +1050,6 @@ static IsTupleLike isTupleLike(Sema &S,
>  }
>} Diagnoser(R, Args);
>
> -  if (R.empty()) {
> -Diagnoser.diagnoseNotICE(S, Loc, SourceRange());
> -return IsTupleLike::Error;
> -  }
> -
>ExprResult E =
>S.BuildDeclarationNameExpr(CXXScopeSpec(), R, /*NeedsADL*/false);
>if (E.isInvalid())
>
> Modified: cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p3.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p3.cpp?rev=369043&r1=369042&r2=369043&view=diff
> ==
> --- cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p3.cpp (original)
> +++ cfe/trunk/test/CXX/dcl.decl/dcl.decomp/p3.cpp Thu Aug 15 12:45:28 2019
> @@ -12,7 +12,7 @@ void no_tuple_size_2() { auto [x, y] = A
>
>  struct Bad1 { int a, b; };
>  template<> struct std::tuple_size {};
> -void no_tuple_size_3() { auto [x, y] = Bad1(); } // expected-error {{cannot 
> decompose this type; 'std::tuple_size::value' is not a valid integral 
> constant expression}}
> +void no_tuple_size_3() { auto [x, y] = Bad1(); } // ok, omitting value is 
> valid after DR2386
>
>  struct Bad2 {};
>  template<> struct std::tuple_size { const int value = 5; };
>
> Modified: cfe/trunk/test/CXX/drs/dr23xx.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/drs/dr23xx.cpp?rev=369043&r1=369042&r2=369043&view=diff
> ==
> --- cfe/trunk/test/CXX/drs/dr23xx.cpp (original)
> +++ cfe/trunk/test/CXX/drs/dr23xx.cpp Thu Aug 15 12:45:28 2019
> @@ -40,6 +40,27 @@ namespace dr2353 { // dr2353: 9
>  #pragma clang __debug dump not_use_2
>  }
>
> +#if __cplusplus >= 201707L
> +// Otherwise, if the qualified-id std::tuple_size names a complete class
> +// type **with a member value**, the expression std::tuple_size::value 
> shall
> +// be a well-formed integral constant expression
> +namespace dr2386 { // dr2386: 9
> +struct Bad1 { int a, b; };
> +struct Bad2 { int a, b; };
> +} // namespace dr2386
> +namespace std {
> +template  struct tuple_size;
> +template <> struct std::tuple_size {};
> +template <> struct std::tuple_size {
> +  static const int value = 42;
> +};
> +} // namespace std
> +namespace dr2386 {
> +void no_value() { auto [x, y] = Bad1(); }
> +void wrong_value() { auto [x, y] = Bad2(); } // expected-error {{decomposes 
> into 42 elements}}
> +} // namespace dr2386
> +#endif
> +
>  namespace dr2387 { // dr2387: 9
>  #if __cplusplus >= 201402L
>template int a = 0;
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65526: [Clangd] Initial prototype version of ExtractFunction

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:68
+  bool hasOnlyRootStmtChildren();
+  // We only support extraction of RootStmts. A RootStmt as a statement that is
+  // fully selected including all of it's children.

Could you move the definition of `RootStmt` to class definition? Since it is 
used in a lot more places than just this one.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:68
+  bool hasOnlyRootStmtChildren();
+  // We only support extraction of RootStmts. A RootStmt as a statement that is
+  // fully selected including all of it's children.

kadircet wrote:
> Could you move the definition of `RootStmt` to class definition? Since it is 
> used in a lot more places than just this one.
> We only support extraction of RootStmts.

Can you also mention the reasoning/implications ?



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:71
+  // Returns the (unselected) parent of all RootStmts.
+  static const Node *getParentOfRootStmts(const Node *CommonAnc);
+  // Find the union of source ranges of all child nodes of Parent. Returns an

Can you make this and the `computeEnclosingFunction{,Rng}` free functions 
instead of static methods?



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:74
+  // inavlid SourceRange if it fails to do so.
+  SourceRange computeSourceRng();
+  // Finds the function in which the source lies.

haven't checked the implementation yet, but naming and the comments seem to be 
ambiguous.

Do you mean *selected* childrens of `Parent`, instead of *all*? Because IIUC, 
`Source` refers to the part of the function body that is selected, not all of 
it, right?



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:222
+
+public:
+  std::string FuncName = "extracted";

comments for some of the fields like `SemicolonPolicy` would be nice.

also most of these looks like implementation details, consider moving internal 
ones to private



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:286
+
+// Captures information about the source.
+class CapturedSourceInfo {

could you give more details on what is collected and how?



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:360
+
+CapturedSourceInfo::DeclInformation &
+CapturedSourceInfo::getDeclInformationFor(const Decl *D) {

why not move this and 4 following functions into astvisitor?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65526



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

[PATCH] D64718: [clangd] Triggering indexing on each translation unit with an error

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

this requires keeping an inverted dependency graph, which can be quite 
expensive memory-wise.
In a project like chromium there are ~40k translation units, and it is safe to 
assume there are ~1k core headers just coming from std library,
that are most likely included by all of the translation units directly or 
transatively. This results in ~320MB of memory usage even if we just
store a pointer.

It is quite likely that there would be a lot more "base" headers like that, 
which could easily drive up the memory usage.

We might need some tradeoffs to limit that usage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64718



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

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/XRefs.cpp:389
 llvm::sort(References, [](const Reference &L, const Reference &R) {
-  return std::tie(L.Loc, L.CanonicalTarget, L.Role) <
- std::tie(R.Loc, R.CanonicalTarget, R.Role);
+  return L.Loc < R.Loc;
 });

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > hokein wrote:
> > > > ilya-biryukov wrote:
> > > > > hokein wrote:
> > > > > > ilya-biryukov wrote:
> > > > > > > What are the elements `References` for the problematic case?
> > > > > > > 
> > > > > > > If we have duplicate elements, then `sort()` would now give us 
> > > > > > > one of the items. Which exact `Decl` we're gonna end up seeing is 
> > > > > > > not well-defined, i.e. it's non-deterministic.
> > > > > > > What are the elements References for the problematic case?
> > > > > > 
> > > > > > The testcase is using declarations (see the existing test) -- we 
> > > > > > will get 3 refs on `using ::fo^o`, each ref has a different decl.  
> > > > > > 
> > > > > > ```
> > > > > > void [[foo]](int);
> > > > > > void [[foo]](double);
> > > > > > 
> > > > > > namespace ns {
> > > > > > using ::[[fo^o]];
> > > > > > }
> > > > > > ```
> > > > > > 
> > > > > > > If we have duplicate elements, then sort() would now give us one 
> > > > > > > of the items. Which exact Decl we're gonna end up seeing is not 
> > > > > > > well-defined, i.e. it's non-deterministic.
> > > > > > 
> > > > > > I think we could make it deterministic, but only return one refs, 
> > > > > > WDYT?
> > > > > > 
> > > > > > 
> > > > > To make sure I understand the problem: we used to get 4 references in 
> > > > > total:
> > > > > - 2 references to the functions (`foo(int)` and `foo(double)`)
> > > > > - 2 references pointing to the using declaration, e.g. the `using 
> > > > > ::[foo]]`
> > > > > 
> > > > > After this patch we would remove the duplicate `using ::[[foo]]` from 
> > > > > the results and get only 3 references as a result.
> > > > > 
> > > > > Is that correct?
> > > > Yes, that is correct.
> > > Interestingly enough, we always ignore the `CanonicalTarget` in the 
> > > returned `Reference`.
> > > 
> > > Maybe we could remove the `CanonicalTarget` from the `Reference` struct 
> > > instead?
> > > That keeps the interface more consistent: clients will always get 
> > > deterministic results (as there is no `CanonicalDecl`, which is different 
> > > now)
> > > Maybe we could remove the CanonicalTarget from the Reference struct 
> > > instead?
> > 
> > unfortunately, this may not work either because of the `Role` -- we still 
> > fail on the above sample, the references pointing to the using decl have 
> > different roles.
> We only look at the role in `DocumentHighlights` to determine the 
> `DocumentHighlightKind` (`Read`, `Write` or `Text`).
> I suggest we do this earlier and replace the `Reference::Role`  field with 
> `Reference::DocumentHighlightKind`.
> Then we can de-duplicate here and keep deterministic interface.
> 
> If we feel that's the wrong layering (we are putting LSP-specific things into 
> Reference, which only has clang-specific types stuff now), we could move 
> de-duplication downstream to `findDocumentHighlights` and `findRefs`. They 
> return `Loc` and `DocumentHighlighting` and can safely de-duplicate on those 
> without changing observable results.
> 
> Looks like replacing `Role` with `HighlightingKind` is the simplest option, 
> though. And I don't think this breaks layering that much, it's an 
> implementation detail of cross-references anyway.
> Looks like replacing Role with HighlightingKind is the simplest option, 
> though. And I don't think this breaks layering that much, it's an 
> implementation detail of cross-references anyway.

However, we may still encounter cases where we have duplicated `Loc`s with 
different `HighlightingKind`s.

This leaves us two choices:
1) de-duplicate the refs only by loc (as the original implemenation of the 
patch)
2) keep Role in refs, and do deduplication on both `findDocumentHighlights` and 
`findRefs`

I personally prefer 1). For document highlights,  I'm not sure whether we 
should return multiple highlights on the same location with different 
`HighlightingKind`s, I think most of clients just choose one to render, so  
making clangd return only one result seems reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66349



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

[PATCH] D65526: [Clangd] Initial prototype version of ExtractFunction

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Main themes are:

- there are a few places we can reduce scope for the first patch: e.g. template 
support
- internally, it'd be clearer to pass data around in structs and avoid complex 
classes unless the abstraction is important
- high-level documentation would aid in understanding: for each 
concept/function: what is the goal, how does it fit into higher-level goals, 
are there any non-obvious reasons it's done this way
- naming questions (this can solve some of the same problems as docs, but much 
more cheaply)




Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:7
+//
+//===--===//
+#include "ClangdUnit.h"

this file will need an overview describing
 - the refactoring, and (briefly) major user-visible design decisions (e.g. 
params, return types, hoisting)
 - structure/approach/major parts



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:37
+
+// Source is the part of code that is being extracted.
+// EnclosingFunction is the function/method inside which the source lies.

this name is *also* pretty confusing because of `SourceLocation`, `SourceRange` 
etc classes.

Best option might just be to make up something distinctive, like `Zone` :-/



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:39
+// EnclosingFunction is the function/method inside which the source lies.
+// PreSource is everything before source and part of EnclosingFunction.
+// PostSource is everything after source and part of EnclosingFunction.

nit: if you're going to document the values one-by-one, do it in a comment on 
each value instead



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:42
+// We split the file into 4 parts w.r.t. the Source.
+enum LocType { PRESOURCE, INSOURCE, POSTSOURCE, OUTSIDEFUNC };
+

nit: we don't use MACROCASE for enums in LLVM (I think). This of course makes 
namespacing more important, we should consider `enum class` here.

I'd probably suggest
```
enum class ZoneRelative { // or some name consistent with above
  Before,
  After,
  Inside,
  External
};

```



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:45
+// ExtractionSourceView forms a view of the code wrt to Source.
+class ExtractionSourceView {
+public:

This looks like basically a struct (mostly public data members) where all the 
logic is in the constructor.
This would be more obvious/more consistent with style elsewhere if it were 
literally a struct and the constructor was instead a function (which could 
return `Expected` instead of setting flags on the result)



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:45
+// ExtractionSourceView forms a view of the code wrt to Source.
+class ExtractionSourceView {
+public:

sammccall wrote:
> This looks like basically a struct (mostly public data members) where all the 
> logic is in the constructor.
> This would be more obvious/more consistent with style elsewhere if it were 
> literally a struct and the constructor was instead a function (which could 
> return `Expected` instead of setting flags on the 
> result)
nit: not sure `view` adds anything here. `ExtractionSource` or `ExtractionZone`?



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:50
+  // Get the location type for a given location.
+  LocType getLocType(SourceLocation Loc) const;
+  // Parent of RootStatements being extracted.

classify?



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:59
+  SourceRange EnclosingFunctionRng;
+  const Node *LastRootStmt = nullptr;
+  bool isEligibleForExtraction() const { return IsEligibleForExtraction; }

when is this not Parent->Children.back()?



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:93
+  for (const Node *CurNode = CommonAnc; CurNode; CurNode = CurNode->Parent) {
+if (CurNode->ASTNode.get()) {
+  // FIXME: Support extraction from methods.

I think you probably want to avoid extracting from lambdas and templates for 
now, too.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:104
+SourceRange
+ExtractionSourceView::computeEnclosingFunctionRng(const Node 
*EnclosingFunction,
+  const SourceManager &SM,

nit: Please spell 'range' rather than 'rng' in function names, as it's not a 
standard abbreviation in clang[d]



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:107
+  const LangOptions

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:389
 llvm::sort(References, [](const Reference &L, const Reference &R) {
-  return std::tie(L.Loc, L.CanonicalTarget, L.Role) <
- std::tie(R.Loc, R.CanonicalTarget, R.Role);
+  return L.Loc < R.Loc;
 });

hokein wrote:
> ilya-biryukov wrote:
> > hokein wrote:
> > > ilya-biryukov wrote:
> > > > hokein wrote:
> > > > > ilya-biryukov wrote:
> > > > > > hokein wrote:
> > > > > > > ilya-biryukov wrote:
> > > > > > > > What are the elements `References` for the problematic case?
> > > > > > > > 
> > > > > > > > If we have duplicate elements, then `sort()` would now give us 
> > > > > > > > one of the items. Which exact `Decl` we're gonna end up seeing 
> > > > > > > > is not well-defined, i.e. it's non-deterministic.
> > > > > > > > What are the elements References for the problematic case?
> > > > > > > 
> > > > > > > The testcase is using declarations (see the existing test) -- we 
> > > > > > > will get 3 refs on `using ::fo^o`, each ref has a different decl. 
> > > > > > >  
> > > > > > > 
> > > > > > > ```
> > > > > > > void [[foo]](int);
> > > > > > > void [[foo]](double);
> > > > > > > 
> > > > > > > namespace ns {
> > > > > > > using ::[[fo^o]];
> > > > > > > }
> > > > > > > ```
> > > > > > > 
> > > > > > > > If we have duplicate elements, then sort() would now give us 
> > > > > > > > one of the items. Which exact Decl we're gonna end up seeing is 
> > > > > > > > not well-defined, i.e. it's non-deterministic.
> > > > > > > 
> > > > > > > I think we could make it deterministic, but only return one refs, 
> > > > > > > WDYT?
> > > > > > > 
> > > > > > > 
> > > > > > To make sure I understand the problem: we used to get 4 references 
> > > > > > in total:
> > > > > > - 2 references to the functions (`foo(int)` and `foo(double)`)
> > > > > > - 2 references pointing to the using declaration, e.g. the `using 
> > > > > > ::[foo]]`
> > > > > > 
> > > > > > After this patch we would remove the duplicate `using ::[[foo]]` 
> > > > > > from the results and get only 3 references as a result.
> > > > > > 
> > > > > > Is that correct?
> > > > > Yes, that is correct.
> > > > Interestingly enough, we always ignore the `CanonicalTarget` in the 
> > > > returned `Reference`.
> > > > 
> > > > Maybe we could remove the `CanonicalTarget` from the `Reference` struct 
> > > > instead?
> > > > That keeps the interface more consistent: clients will always get 
> > > > deterministic results (as there is no `CanonicalDecl`, which is 
> > > > different now)
> > > > Maybe we could remove the CanonicalTarget from the Reference struct 
> > > > instead?
> > > 
> > > unfortunately, this may not work either because of the `Role` -- we still 
> > > fail on the above sample, the references pointing to the using decl have 
> > > different roles.
> > We only look at the role in `DocumentHighlights` to determine the 
> > `DocumentHighlightKind` (`Read`, `Write` or `Text`).
> > I suggest we do this earlier and replace the `Reference::Role`  field with 
> > `Reference::DocumentHighlightKind`.
> > Then we can de-duplicate here and keep deterministic interface.
> > 
> > If we feel that's the wrong layering (we are putting LSP-specific things 
> > into Reference, which only has clang-specific types stuff now), we could 
> > move de-duplication downstream to `findDocumentHighlights` and `findRefs`. 
> > They return `Loc` and `DocumentHighlighting` and can safely de-duplicate on 
> > those without changing observable results.
> > 
> > Looks like replacing `Role` with `HighlightingKind` is the simplest option, 
> > though. And I don't think this breaks layering that much, it's an 
> > implementation detail of cross-references anyway.
> > Looks like replacing Role with HighlightingKind is the simplest option, 
> > though. And I don't think this breaks layering that much, it's an 
> > implementation detail of cross-references anyway.
> 
> However, we may still encounter cases where we have duplicated `Loc`s with 
> different `HighlightingKind`s.
> 
> This leaves us two choices:
> 1) de-duplicate the refs only by loc (as the original implemenation of the 
> patch)
> 2) keep Role in refs, and do deduplication on both `findDocumentHighlights` 
> and `findRefs`
> 
> I personally prefer 1). For document highlights,  I'm not sure whether we 
> should return multiple highlights on the same location with different 
> `HighlightingKind`s, I think most of clients just choose one to render, so  
> making clangd return only one result seems reasonable.
I would vouch for (2):
- It's easy to explain why we de-duplicate same locations in `findRefs` as this 
is what it is exactly the return value.
- The decision about picking the kind for a duplicated location falls onto 
`findDocumentHighlights`, which is, again, the right place to make this 
decision (even if the decision is random, e.g. preferring `Write` over `Read`)

[PATCH] D66470: [Syntax] Added function to get macro expansion tokens to TokenBuffer.

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

jvikstrom created this revision.
jvikstrom added reviewers: hokein, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet.
Herald added a project: clang.

Returns the first token in every mapping where the token is an identifier.
This API is required to be able to highlight macro expansions in clangd.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66470

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


Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -59,7 +59,6 @@
 using ::testing::Matcher;
 using ::testing::Not;
 using ::testing::StartsWith;
-
 namespace {
 // Checks the passed ArrayRef has the same begin() and end() iterators as 
the
 // argument.
@@ -755,4 +754,27 @@
   // We don't test assertion failures because death tests are slow.
 }
 
+TEST_F(TokenBufferTest, macroExpansions) {
+  llvm::Annotations Code(R"cpp(
+#define FOO B
+#define FOO2 BA
+#define CALL(X) int X
+#define G CALL(FOO2)
+int B;
+$macro[[FOO]];
+$macro[[CALL]](A);
+$macro[[G]];
+  )cpp");
+  recordTokens(Code.code());
+  auto &SM = *SourceMgr;
+  auto Expansions = Buffer.macroExpansions(SM.getMainFileID());
+  std::vector ExpectedMacroRanges;
+  for (auto Range : Code.ranges("macro"))
+ExpectedMacroRanges.push_back(
+FileRange(SM.getMainFileID(), Range.Begin, Range.End));
+  std::vector ActualMacroRanges;
+  for (auto Expansion : Expansions)
+ActualMacroRanges.push_back(Expansion->range(SM));
+  EXPECT_EQ(ExpectedMacroRanges, ActualMacroRanges);
+}
 } // namespace
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -232,6 +232,21 @@
   return E;
 }
 
+std::vector
+TokenBuffer::macroExpansions(FileID FID) const {
+  auto FileIt = Files.find(FID);
+  assert(FileIt != Files.end() && "file not tracked by token buffer");
+  auto &File = FileIt->second;
+  std::vector Expansions;
+  auto &Spelled = File.SpelledTokens;
+  for (auto Mapping : File.Mappings) {
+const syntax::Token *Token = &Spelled[Mapping.BeginSpelled];
+if (Token->kind() == tok::TokenKind::identifier)
+  Expansions.push_back(Token);
+  }
+  return Expansions;
+}
+
 std::vector syntax::tokenize(FileID FID, const SourceManager 
&SM,
 const LangOptions &LO) {
   std::vector Tokens;
Index: clang/include/clang/Tooling/Syntax/Tokens.h
===
--- clang/include/clang/Tooling/Syntax/Tokens.h
+++ clang/include/clang/Tooling/Syntax/Tokens.h
@@ -236,6 +236,15 @@
   ///#pragma, etc.
   llvm::ArrayRef spelledTokens(FileID FID) const;
 
+  /// Get all tokens that expand a macro in FID. For the following input
+  /// #define FOO B
+  /// #define FOO2(X) int X
+  /// FOO2(XY)
+  /// int B;
+  /// FOO;
+  /// macroExpansions() returns {"FOO2", "FOO"}.
+  std::vector macroExpansions(FileID FID) const;
+
   const SourceManager &sourceManager() const { return *SourceMgr; }
 
   std::string dumpForTests() const;


Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -59,7 +59,6 @@
 using ::testing::Matcher;
 using ::testing::Not;
 using ::testing::StartsWith;
-
 namespace {
 // Checks the passed ArrayRef has the same begin() and end() iterators as the
 // argument.
@@ -755,4 +754,27 @@
   // We don't test assertion failures because death tests are slow.
 }
 
+TEST_F(TokenBufferTest, macroExpansions) {
+  llvm::Annotations Code(R"cpp(
+#define FOO B
+#define FOO2 BA
+#define CALL(X) int X
+#define G CALL(FOO2)
+int B;
+$macro[[FOO]];
+$macro[[CALL]](A);
+$macro[[G]];
+  )cpp");
+  recordTokens(Code.code());
+  auto &SM = *SourceMgr;
+  auto Expansions = Buffer.macroExpansions(SM.getMainFileID());
+  std::vector ExpectedMacroRanges;
+  for (auto Range : Code.ranges("macro"))
+ExpectedMacroRanges.push_back(
+FileRange(SM.getMainFileID(), Range.Begin, Range.End));
+  std::vector ActualMacroRanges;
+  for (auto Expansion : Expansions)
+ActualMacroRanges.push_back(Expansion->range(SM));
+  EXPECT_EQ(ExpectedMacroRanges, ActualMacroRanges);
+}
 } // namespace
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -232,6 +232,21 @@
   return E;
 }
 
+std::vector
+TokenBuffer::macroExpansions(FileID FID) const {
+  auto FileIt = File

[PATCH] D66470: [Syntax] Added function to get macro expansion tokens to TokenBuffer.

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

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

Readded dissapeared newline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66470

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


Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -755,4 +755,27 @@
   // We don't test assertion failures because death tests are slow.
 }
 
+TEST_F(TokenBufferTest, macroExpansions) {
+  llvm::Annotations Code(R"cpp(
+#define FOO B
+#define FOO2 BA
+#define CALL(X) int X
+#define G CALL(FOO2)
+int B;
+$macro[[FOO]];
+$macro[[CALL]](A);
+$macro[[G]];
+  )cpp");
+  recordTokens(Code.code());
+  auto &SM = *SourceMgr;
+  auto Expansions = Buffer.macroExpansions(SM.getMainFileID());
+  std::vector ExpectedMacroRanges;
+  for (auto Range : Code.ranges("macro"))
+ExpectedMacroRanges.push_back(
+FileRange(SM.getMainFileID(), Range.Begin, Range.End));
+  std::vector ActualMacroRanges;
+  for (auto Expansion : Expansions)
+ActualMacroRanges.push_back(Expansion->range(SM));
+  EXPECT_EQ(ExpectedMacroRanges, ActualMacroRanges);
+}
 } // namespace
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -232,6 +232,21 @@
   return E;
 }
 
+std::vector
+TokenBuffer::macroExpansions(FileID FID) const {
+  auto FileIt = Files.find(FID);
+  assert(FileIt != Files.end() && "file not tracked by token buffer");
+  auto &File = FileIt->second;
+  std::vector Expansions;
+  auto &Spelled = File.SpelledTokens;
+  for (auto Mapping : File.Mappings) {
+const syntax::Token *Token = &Spelled[Mapping.BeginSpelled];
+if (Token->kind() == tok::TokenKind::identifier)
+  Expansions.push_back(Token);
+  }
+  return Expansions;
+}
+
 std::vector syntax::tokenize(FileID FID, const SourceManager 
&SM,
 const LangOptions &LO) {
   std::vector Tokens;
Index: clang/include/clang/Tooling/Syntax/Tokens.h
===
--- clang/include/clang/Tooling/Syntax/Tokens.h
+++ clang/include/clang/Tooling/Syntax/Tokens.h
@@ -236,6 +236,15 @@
   ///#pragma, etc.
   llvm::ArrayRef spelledTokens(FileID FID) const;
 
+  /// Get all tokens that expand a macro in FID. For the following input
+  /// #define FOO B
+  /// #define FOO2(X) int X
+  /// FOO2(XY)
+  /// int B;
+  /// FOO;
+  /// macroExpansions() returns {"FOO2", "FOO"}.
+  std::vector macroExpansions(FileID FID) const;
+
   const SourceManager &sourceManager() const { return *SourceMgr; }
 
   std::string dumpForTests() const;


Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -755,4 +755,27 @@
   // We don't test assertion failures because death tests are slow.
 }
 
+TEST_F(TokenBufferTest, macroExpansions) {
+  llvm::Annotations Code(R"cpp(
+#define FOO B
+#define FOO2 BA
+#define CALL(X) int X
+#define G CALL(FOO2)
+int B;
+$macro[[FOO]];
+$macro[[CALL]](A);
+$macro[[G]];
+  )cpp");
+  recordTokens(Code.code());
+  auto &SM = *SourceMgr;
+  auto Expansions = Buffer.macroExpansions(SM.getMainFileID());
+  std::vector ExpectedMacroRanges;
+  for (auto Range : Code.ranges("macro"))
+ExpectedMacroRanges.push_back(
+FileRange(SM.getMainFileID(), Range.Begin, Range.End));
+  std::vector ActualMacroRanges;
+  for (auto Expansion : Expansions)
+ActualMacroRanges.push_back(Expansion->range(SM));
+  EXPECT_EQ(ExpectedMacroRanges, ActualMacroRanges);
+}
 } // namespace
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -232,6 +232,21 @@
   return E;
 }
 
+std::vector
+TokenBuffer::macroExpansions(FileID FID) const {
+  auto FileIt = Files.find(FID);
+  assert(FileIt != Files.end() && "file not tracked by token buffer");
+  auto &File = FileIt->second;
+  std::vector Expansions;
+  auto &Spelled = File.SpelledTokens;
+  for (auto Mapping : File.Mappings) {
+const syntax::Token *Token = &Spelled[Mapping.BeginSpelled];
+if (Token->kind() == tok::TokenKind::identifier)
+  Expansions.push_back(Token);
+  }
+  return Expansions;
+}
+
 std::vector syntax::tokenize(FileID FID, const SourceManager &SM,
 const LangOptions &LO) {
   std

r369373 - [OpenCL] Add const, volatile and pointer builtin handling

[Sven van Haastregt via cfe-commits]

Author: svenvh
Date: Tue Aug 20 05:21:03 2019
New Revision: 369373

URL: http://llvm.org/viewvc/llvm-project?rev=369373&view=rev
Log:
[OpenCL] Add const, volatile and pointer builtin handling

Const, volatile, and pointer types were previously available, but not
working.  This patch adds handling for OpenCL builtin functions.

Add TableGen definitions for some atomic and asynchronous builtins to
make use of the new functionality.

Patch by Pierre Gondois and Sven van Haastregt.

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

Modified:
cfe/trunk/lib/Sema/OpenCLBuiltins.td
cfe/trunk/test/SemaOpenCL/fdeclare-opencl-builtins.cl
cfe/trunk/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

Modified: cfe/trunk/lib/Sema/OpenCLBuiltins.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/OpenCLBuiltins.td?rev=369373&r1=369372&r2=369373&view=diff
==
--- cfe/trunk/lib/Sema/OpenCLBuiltins.td (original)
+++ cfe/trunk/lib/Sema/OpenCLBuiltins.td Tue Aug 20 05:21:03 2019
@@ -51,11 +51,6 @@ class QualType {
-  string QualName = _QualName;
-}
-
 // List of integers.
 class IntList _List> {
   string Name = _Name;
@@ -79,24 +74,59 @@ class Type QualList = [];
+  // "const" qualifier.
+  bit IsConst = 0;
+  // "volatile" qualifier.
+  bit IsVolatile = 0;
   // Access qualifier. Must be one of ("RO", "WO", "RW").
   string AccessQualifier = "";
   // Address space.
-  string AddrSpace = "clang::LangAS::Default";
+  string AddrSpace = DefaultAS.Name;
 }
 
-// OpenCL vector types (e.g. int2, int3, int16, float8, ...)
+// OpenCL vector types (e.g. int2, int3, int16, float8, ...).
 class VectorType : Type<_Ty.Name, _Ty.QTName> {
-  int VecWidth = _VecWidth;
+  let VecWidth = _VecWidth;
+  // Inherited fields
+  let IsPointer = _Ty.IsPointer;
+  let IsConst = _Ty.IsConst;
+  let IsVolatile = _Ty.IsVolatile;
+  let AccessQualifier = _Ty.AccessQualifier;
+  let AddrSpace = _Ty.AddrSpace;
 }
 
 // OpenCL pointer types (e.g. int*, float*, ...).
-class PointerType :
+class PointerType :
   Type<_Ty.Name, _Ty.QTName> {
-  bit IsPointer = 1;
-  string AddrSpace = _AS.Name;
+  let AddrSpace = _AS.Name;
+  // Inherited fields
+  let VecWidth = _Ty.VecWidth;
+  let IsPointer = 1;
+  let IsConst = _Ty.IsConst;
+  let IsVolatile = _Ty.IsVolatile;
+  let AccessQualifier = _Ty.AccessQualifier;
+}
+
+// OpenCL const types (e.g. const int).
+class ConstType : Type<_Ty.Name, _Ty.QTName> {
+  let IsConst = 1;
+  // Inherited fields
+  let VecWidth = _Ty.VecWidth;
+  let IsPointer = _Ty.IsPointer;
+  let IsVolatile = _Ty.IsVolatile;
+  let AccessQualifier = _Ty.AccessQualifier;
+  let AddrSpace = _Ty.AddrSpace;
+}
+
+// OpenCL volatile types (e.g. volatile int).
+class VolatileType : Type<_Ty.Name, _Ty.QTName> {
+  let IsVolatile = 1;
+  // Inherited fields
+  let VecWidth = _Ty.VecWidth;
+  let IsPointer = _Ty.IsPointer;
+  let IsConst = _Ty.IsConst;
+  let AccessQualifier = _Ty.AccessQualifier;
+  let AddrSpace = _Ty.AddrSpace;
 }
 
 // OpenCL image types (e.g. image2d_t, ...)
@@ -242,12 +272,16 @@ def VecNoScalar : IntList<"VecNoScalar",
 def Vec1: IntList<"Vec1", [1]>;
 
 // Type lists.
+def TLAll   : TypeList<"TLAll", [Char, UChar, Short, UShort, Int, UInt, Long, 
ULong, Float, Double, Half]>;
 def TLFloat : TypeList<"TLFloat", [Float, Double, Half]>;
 
 def TLAllInts : TypeList<"TLAllInts", [Char, UChar, Short, UShort, Int, UInt, 
Long, ULong]>;
 
 // GenType definitions for multiple base types (e.g. all floating point types,
 // or all integer types).
+// All types
+def AGenTypeN  : GenericType<"AGenTypeN", TLAll, VecAndScalar>;
+def AGenTypeNNoScalar  : GenericType<"AGenTypeNNoScalar", TLAll, 
VecNoScalar>;
 // All integer
 def AIGenType1 : GenericType<"AIGenType1", TLAllInts, Vec1>;
 def AIGenTypeN : GenericType<"AIGenTypeN", TLAllInts, 
VecAndScalar>;
@@ -294,6 +328,44 @@ foreach RType = [Float, Double, Half, Ch
   }
 }
 
+//
+// OpenCL v1.1 s6.11.10, v1.2 s6.12.10, v2.0 s6.13.10: Async Copies from 
Global to Local Memory, Local to Global Memory, and Prefetch
+// OpenCL Extension v2.0 s5.1.7 and s6.1.7: Async Copies from Global to Local 
Memory, Local to Global Memory, and Prefetch
+// --- Table 18 ---
+foreach name = ["async_work_group_copy"] in {
+  def : Builtin, 
PointerType, GlobalAS>, Size, Event]>;
+  def : Builtin, 
PointerType, LocalAS>, Size, Event]>;
+}
+foreach name = ["async_work_group_strided_copy"] in {
+  def : Builtin, 
PointerType, GlobalAS>, Size, Size, Event]>;
+  def : Builtin, 
PointerType, LocalAS>, Size, Size, Event]>;
+}
+foreach name = ["wait_group_events"] in {
+  def : Builtin]>;
+}
+foreach name = ["prefetch"] in {
+  def : Builtin, GlobalAS>, 
Size]>;
+}
+
+//
+// OpenCL v2.0 s6.13.11 - Atomics

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/lib/Sema/SemaAttr.cpp:200
 CXXRecordDecl *Canonical = Record->getCanonicalDecl();
 if (Canonical->hasAttr() || Canonical->hasAttr())
   return;

mgehre wrote:
> gribozavr wrote:
> > mgehre wrote:
> > > gribozavr wrote:
> > > > Should this code not do this short-circuit check? It is prone to going 
> > > > out of sync with the code in 
> > > > `addGslOwnerPointerAttributeIfNotExisting`, like it did just now.
> > > > 
> > > > If you want to check for attribute before doing the string search, you 
> > > > could pass the string set into 
> > > > `addGslOwnerPointerAttributeIfNotExisting`, and let that decide if it 
> > > > should infer the attribute or not.
> > > Yes, the `hasAttr` check here is an optimization to avoid the string 
> > > search. I don't think I can move the string search into 
> > > `addGslOwnerPointerAttributeIfNotExisting`, because that function is 
> > > called 
> > > from other call sites that don't care about a set of names.
> > Sorry I wasn't clear enough -- the issue that I noticed is that this check 
> > looks at the canonical decl, while 
> > `addGslOwnerPointerAttributeIfNotExisting` attaches the attribute to the 
> > decl itself -- that's what I meant by "out of sync".
> I make sure that `addGslOwnerPointerAttributeIfNotExisting` is only called 
> with the canonical decl as argument, which the caller usually has around. The 
> only exception to this is when addGslOwnerPointerAttributeIfNotExisting calls 
> itself to attach an attribute to the definition of templated class.
I see. This is a tricky contract, and ".getCanonicalDecl()" at callsites feel 
somewhat random. I think it should be at least documented. However...

> I now start to wonder if we should instead put the attribute on all 
> declarations (instead of just the canonical). Later redeclarations 
> automatically inherit the attributes.
> This would make both the checking easier (just check any declaration, no need 
> to obtain the canonical first), and remove the special
> case for adding to the definition of templated record for ClassTemplateDecls.

I'd support that.



Comment at: clang/test/SemaCXX/attr-gsl-owner-pointer-std.cpp:95
 
+// The iterator typedef is a DependentNameType.
+template 

mgehre wrote:
> gribozavr wrote:
> > This test file is getting pretty long and subtle, with lots of things are 
> > being mixed into one file without isolation.
> > 
> > WDYT about refactoring it into a unit test that uses AST matchers to verify 
> > attribute presence?
> > 
> > See clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp for examples.
> > 
> > Each test case would look approximately like this:
> > 
> > ```
> > EXPECT_TRUE(matches(
> >   "template class ...",
> >   classTemplateDecl(hasName("foo"), hasAttr(clang::attr::GslPointer)));
> > ```
> > 
> > Each test case would be isolated from other tests, each test would have a 
> > name (and optionally a comment) that will make it obvious what exactly is 
> > being tested.
> > 
> > It would be also possible to verify things like "The iterator typedef is a 
> > DependentNameType" to ensure that we're testing exactly what we want to 
> > test.
> I like the AST matcher approach! This test file is really hard to debug - I 
> usually copy a test to its own file for debugging. 
> Would you be okay with deferring the testing change to the next PR?
Of course! Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66179



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

r369380 - Removed the 'id' AST matcher, which is superseded by '.bind()'

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Tue Aug 20 06:02:28 2019
New Revision: 369380

URL: http://llvm.org/viewvc/llvm-project?rev=369380&view=rev
Log:
Removed the 'id' AST matcher, which is superseded by '.bind()'

Summary:
The 'id' matcher is not even included in the AST Matchers Reference
document, so I don't expect there to be a significant number of users.

There's no reason to provide two ways to do the exact same thing that
only have a minor syntactic difference.

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/unittests/Tooling/RefactoringCallbacksTest.cpp

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=369380&r1=369379&r2=369380&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Tue Aug 20 06:02:28 2019
@@ -19,15 +19,15 @@
 //
 //  For more complicated match expressions we're often interested in accessing
 //  multiple parts of the matched AST nodes once a match is found. In that 
case,
-//  use the id(...) matcher around the match expressions that match the nodes
-//  you want to access.
+//  call `.bind("name")` on match expressions that match the nodes you want to
+//  access.
 //
 //  For example, when we're interested in child classes of a certain class, we
 //  would write:
-//cxxRecordDecl(hasName("MyClass"), has(id("child", recordDecl(
+//cxxRecordDecl(hasName("MyClass"), has(recordDecl().bind("child")))
 //  When the match is found via the MatchFinder, a user provided callback will
 //  be called with a BoundNodes instance that contains a mapping from the
-//  strings that we provided for the id(...) calls to the nodes that were
+//  strings that we provided for the `.bind()` calls to the nodes that were
 //  matched.
 //  In the given example, each time our matcher finds a match we get a callback
 //  where "child" is bound to the RecordDecl node of the matching child
@@ -131,15 +131,6 @@ private:
   internal::BoundNodesMap MyBoundNodes;
 };
 
-/// If the provided matcher matches a node, binds the node to \c ID.
-///
-/// FIXME: Do we want to support this now that we have bind()?
-template 
-internal::Matcher id(StringRef ID,
-const internal::BindableMatcher &InnerMatcher) {
-  return InnerMatcher.bind(ID);
-}
-
 /// Types of matchers for the top-level classes in the AST class
 /// hierarchy.
 /// @{

Modified: cfe/trunk/unittests/Tooling/RefactoringCallbacksTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringCallbacksTest.cpp?rev=369380&r1=369379&r2=369380&view=diff
==
--- cfe/trunk/unittests/Tooling/RefactoringCallbacksTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/RefactoringCallbacksTest.cpp Tue Aug 20 
06:02:28 2019
@@ -38,28 +38,28 @@ TEST(RefactoringCallbacksTest, ReplacesS
   std::string Code = "void f() { int i = 1; }";
   std::string Expected = "void f() { ; }";
   ReplaceStmtWithText Callback("id", ";");
-  expectRewritten(Code, Expected, id("id", declStmt()), Callback);
+  expectRewritten(Code, Expected, declStmt().bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, ReplacesStmtsInCalledMacros) {
   std::string Code = "#define A void f() { int i = 1; }\nA";
   std::string Expected = "#define A void f() { ; }\nA";
   ReplaceStmtWithText Callback("id", ";");
-  expectRewritten(Code, Expected, id("id", declStmt()), Callback);
+  expectRewritten(Code, Expected, declStmt().bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, IgnoresStmtsInUncalledMacros) {
   std::string Code = "#define A void f() { int i = 1; }";
   std::string Expected = "#define A void f() { int i = 1; }";
   ReplaceStmtWithText Callback("id", ";");
-  expectRewritten(Code, Expected, id("id", declStmt()), Callback);
+  expectRewritten(Code, Expected, declStmt().bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, ReplacesInteger) {
   std::string Code = "void f() { int i = 1; }";
   std::string Expected = "void f() { int i = 2; }";
   ReplaceStmtWithText Callback("id", "2");
-  expectRewritten(Code, Expected, id("id", expr(integerLiteral())), Callback);
+  expectRewritten(Code, Expected, expr(integerLiteral()).bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, ReplacesStmtWithStmt) {
@@ -68,9 +68,9 @@ TEST(RefactoringCallbacksTest, ReplacesS
   ReplaceStmtWithStmt Callback("always-false", "should-be");
   expectRewritten(
   Code, Expected,
-  id("always-false",
- conditionalOperator(hasCondition(cxxBoolLiteral(equals(false))),
- hasFalseExpression(id("should-be", expr(),
+  conditionalOperator(hasCondition(

[PATCH] D66462: Removed the 'id' AST matcher, which is superseded by '.bind()'

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369380: Removed the 'id' AST matcher, which is 
superseded by '.bind()' (authored by gribozavr, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66462?vs=216082&id=216119#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66462

Files:
  cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
  cfe/trunk/unittests/Tooling/RefactoringCallbacksTest.cpp

Index: cfe/trunk/unittests/Tooling/RefactoringCallbacksTest.cpp
===
--- cfe/trunk/unittests/Tooling/RefactoringCallbacksTest.cpp
+++ cfe/trunk/unittests/Tooling/RefactoringCallbacksTest.cpp
@@ -38,28 +38,28 @@
   std::string Code = "void f() { int i = 1; }";
   std::string Expected = "void f() { ; }";
   ReplaceStmtWithText Callback("id", ";");
-  expectRewritten(Code, Expected, id("id", declStmt()), Callback);
+  expectRewritten(Code, Expected, declStmt().bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, ReplacesStmtsInCalledMacros) {
   std::string Code = "#define A void f() { int i = 1; }\nA";
   std::string Expected = "#define A void f() { ; }\nA";
   ReplaceStmtWithText Callback("id", ";");
-  expectRewritten(Code, Expected, id("id", declStmt()), Callback);
+  expectRewritten(Code, Expected, declStmt().bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, IgnoresStmtsInUncalledMacros) {
   std::string Code = "#define A void f() { int i = 1; }";
   std::string Expected = "#define A void f() { int i = 1; }";
   ReplaceStmtWithText Callback("id", ";");
-  expectRewritten(Code, Expected, id("id", declStmt()), Callback);
+  expectRewritten(Code, Expected, declStmt().bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, ReplacesInteger) {
   std::string Code = "void f() { int i = 1; }";
   std::string Expected = "void f() { int i = 2; }";
   ReplaceStmtWithText Callback("id", "2");
-  expectRewritten(Code, Expected, id("id", expr(integerLiteral())), Callback);
+  expectRewritten(Code, Expected, expr(integerLiteral()).bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, ReplacesStmtWithStmt) {
@@ -68,9 +68,9 @@
   ReplaceStmtWithStmt Callback("always-false", "should-be");
   expectRewritten(
   Code, Expected,
-  id("always-false",
- conditionalOperator(hasCondition(cxxBoolLiteral(equals(false))),
- hasFalseExpression(id("should-be", expr(),
+  conditionalOperator(hasCondition(cxxBoolLiteral(equals(false))),
+  hasFalseExpression(expr().bind("should-be")))
+  .bind("always-false"),
   Callback);
 }
 
@@ -78,20 +78,20 @@
   std::string Code = "bool a; void f() { if (a) f(); else a = true; }";
   std::string Expected = "bool a; void f() { f(); }";
   ReplaceIfStmtWithItsBody Callback("id", true);
-  expectRewritten(
-  Code, Expected,
-  id("id", ifStmt(hasCondition(implicitCastExpr(hasSourceExpression(
-   declRefExpr(to(varDecl(hasName("a"),
-  Callback);
+  expectRewritten(Code, Expected,
+  ifStmt(hasCondition(implicitCastExpr(hasSourceExpression(
+ declRefExpr(to(varDecl(hasName("a"
+  .bind("id"),
+  Callback);
 }
 
 TEST(RefactoringCallbacksTest, RemovesEntireIfOnEmptyElse) {
   std::string Code = "void f() { if (false) int i = 0; }";
   std::string Expected = "void f() {  }";
   ReplaceIfStmtWithItsBody Callback("id", false);
-  expectRewritten(Code, Expected,
-  id("id", ifStmt(hasCondition(cxxBoolLiteral(equals(false),
-  Callback);
+  expectRewritten(
+  Code, Expected,
+  ifStmt(hasCondition(cxxBoolLiteral(equals(false.bind("id"), Callback);
 }
 
 TEST(RefactoringCallbacksTest, TemplateJustText) {
@@ -99,7 +99,7 @@
   std::string Expected = "void f() { FOO }";
   auto Callback = ReplaceNodeWithTemplate::create("id", "FOO");
   EXPECT_FALSE(Callback.takeError());
-  expectRewritten(Code, Expected, id("id", declStmt()), **Callback);
+  expectRewritten(Code, Expected, declStmt().bind("id"), **Callback);
 }
 
 TEST(RefactoringCallbacksTest, TemplateSimpleSubst) {
@@ -108,7 +108,7 @@
   auto Callback = ReplaceNodeWithTemplate::create("decl", "long x = ${init}");
   EXPECT_FALSE(Callback.takeError());
   expectRewritten(Code, Expected,
-  id("decl", varDecl(hasInitializer(id("init", expr(),
+  varDecl(hasInitializer(expr().bind("init"))).bind("decl"),
   **Callback);
 }
 
@@ -119,7 +119,7 @@
   "string x = \"$$-${init}\"");
   EXPECT_FALSE(Callback.takeError());
   expectRewritten(Code, Expected,
-  id("decl", varDecl(hasInitializer(id("init", expr()

[PATCH] D66470: [Syntax] Added function to get macro expansion tokens to TokenBuffer.

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:239
 
+  /// Get all tokens that expand a macro in FID. For the following input
+  /// #define FOO B

NIT:  add the doxygen tag for parameter `in \p FID`



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:245
+  /// FOO;
+  /// macroExpansions() returns {"FOO2", "FOO"}.
+  std::vector macroExpansions(FileID FID) const;

NIT: `FOO` and `FOO2` are also mentioned in the `#define` lines. Maybe clarify 
that the results are at lines 3 and 5, not 1 and 2?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66470



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

[PATCH] D57659: [Sema] SequenceChecker: Add some comments + related small NFCs in preparation of the following patches

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

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

This seems ok (not sure why stalled)


Repository:
  rC Clang

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

https://reviews.llvm.org/D57659



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

[PATCH] D65182: [analyzer] Add fix-it hint support.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:347
+
+  virtual llvm::ArrayRef getFixits() const { return Fixits; }
+

Why is it virtual? In fact, why is BugReporter subclassable at all?



Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:751
+o << "insert_string\n";
+o << "" << Hint.CodeToInsert << "\n";
+o << "   \n";

Escaping?



Comment at: clang/test/Analysis/dead-stores.c:11
   long idx=abc+3*5; // expected-warning {{never read}} 
expected-warning{{unused variable 'idx'}}
+  // expected-remark-re@-1.*}}:11 - {{.*}}:18 - ''}}
 }

If the tests ignore the line number anyway, why even print it?


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

https://reviews.llvm.org/D65182



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

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

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

xbolva00 added a comment.

Added some reviewers

Patch improves suboptimal diagnostic, which misses bugs like:
https://bugs.llvm.org/show_bug.cgi?id=43052


Repository:
  rC Clang

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

https://reviews.llvm.org/D57660



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

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D57660#1637220 , @xbolva00 wrote:

> Added some reviewers
>
> Patch improves suboptimal diagnostic, which misses bugs like:
>  https://bugs.llvm.org/show_bug.cgi?id=43052


Thanks for taking a look !


Repository:
  rC Clang

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

https://reviews.llvm.org/D57660



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

r369385 - [Syntax] Added function to get macro expansion tokens to TokenBuffer.

[Johan Vikstrom via cfe-commits]

Author: jvikstrom
Date: Tue Aug 20 06:34:01 2019
New Revision: 369385

URL: http://llvm.org/viewvc/llvm-project?rev=369385&view=rev
Log:
[Syntax] Added function to get macro expansion tokens to TokenBuffer.

Summary:
Returns the first token in every mapping where the token is an identifier.
This API is required to be able to highlight macro expansions in clangd.

Reviewers: hokein, ilya-biryukov

Subscribers: kadircet, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Tooling/Syntax/Tokens.h
cfe/trunk/lib/Tooling/Syntax/Tokens.cpp
cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp

Modified: cfe/trunk/include/clang/Tooling/Syntax/Tokens.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Syntax/Tokens.h?rev=369385&r1=369384&r2=369385&view=diff
==
--- cfe/trunk/include/clang/Tooling/Syntax/Tokens.h (original)
+++ cfe/trunk/include/clang/Tooling/Syntax/Tokens.h Tue Aug 20 06:34:01 2019
@@ -236,6 +236,16 @@ public:
   ///#pragma, etc.
   llvm::ArrayRef spelledTokens(FileID FID) const;
 
+  /// Get all tokens that expand a macro in \p FID. For the following input
+  /// #define FOO B
+  /// #define FOO2(X) int X
+  /// FOO2(XY)
+  /// int B;
+  /// FOO;
+  /// macroExpansions() returns {"FOO2", "FOO"} (from line 3 and 5
+  /// respecitvely).
+  std::vector macroExpansions(FileID FID) const;
+
   const SourceManager &sourceManager() const { return *SourceMgr; }
 
   std::string dumpForTests() const;

Modified: cfe/trunk/lib/Tooling/Syntax/Tokens.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Syntax/Tokens.cpp?rev=369385&r1=369384&r2=369385&view=diff
==
--- cfe/trunk/lib/Tooling/Syntax/Tokens.cpp (original)
+++ cfe/trunk/lib/Tooling/Syntax/Tokens.cpp Tue Aug 20 06:34:01 2019
@@ -232,6 +232,21 @@ TokenBuffer::expansionStartingAt(const s
   return E;
 }
 
+std::vector
+TokenBuffer::macroExpansions(FileID FID) const {
+  auto FileIt = Files.find(FID);
+  assert(FileIt != Files.end() && "file not tracked by token buffer");
+  auto &File = FileIt->second;
+  std::vector Expansions;
+  auto &Spelled = File.SpelledTokens;
+  for (auto Mapping : File.Mappings) {
+const syntax::Token *Token = &Spelled[Mapping.BeginSpelled];
+if (Token->kind() == tok::TokenKind::identifier)
+  Expansions.push_back(Token);
+  }
+  return Expansions;
+}
+
 std::vector syntax::tokenize(FileID FID, const SourceManager 
&SM,
 const LangOptions &LO) {
   std::vector Tokens;

Modified: cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp?rev=369385&r1=369384&r2=369385&view=diff
==
--- cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp Tue Aug 20 06:34:01 2019
@@ -755,4 +755,27 @@ TEST_F(TokenBufferTest, TokensToFileRang
   // We don't test assertion failures because death tests are slow.
 }
 
+TEST_F(TokenBufferTest, macroExpansions) {
+  llvm::Annotations Code(R"cpp(
+#define FOO B
+#define FOO2 BA
+#define CALL(X) int X
+#define G CALL(FOO2)
+int B;
+$macro[[FOO]];
+$macro[[CALL]](A);
+$macro[[G]];
+  )cpp");
+  recordTokens(Code.code());
+  auto &SM = *SourceMgr;
+  auto Expansions = Buffer.macroExpansions(SM.getMainFileID());
+  std::vector ExpectedMacroRanges;
+  for (auto Range : Code.ranges("macro"))
+ExpectedMacroRanges.push_back(
+FileRange(SM.getMainFileID(), Range.Begin, Range.End));
+  std::vector ActualMacroRanges;
+  for (auto Expansion : Expansions)
+ActualMacroRanges.push_back(Expansion->range(SM));
+  EXPECT_EQ(ExpectedMacroRanges, ActualMacroRanges);
+}
 } // namespace


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

[PATCH] D66470: [Syntax] Added function to get macro expansion tokens to TokenBuffer.

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

This revision was automatically updated to reflect the committed changes.
jvikstrom marked 2 inline comments as done.
Closed by commit rL369385: [Syntax] Added function to get macro expansion 
tokens to TokenBuffer. (authored by jvikstrom, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66470?vs=216103&id=216135#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66470

Files:
  cfe/trunk/include/clang/Tooling/Syntax/Tokens.h
  cfe/trunk/lib/Tooling/Syntax/Tokens.cpp
  cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp


Index: cfe/trunk/lib/Tooling/Syntax/Tokens.cpp
===
--- cfe/trunk/lib/Tooling/Syntax/Tokens.cpp
+++ cfe/trunk/lib/Tooling/Syntax/Tokens.cpp
@@ -232,6 +232,21 @@
   return E;
 }
 
+std::vector
+TokenBuffer::macroExpansions(FileID FID) const {
+  auto FileIt = Files.find(FID);
+  assert(FileIt != Files.end() && "file not tracked by token buffer");
+  auto &File = FileIt->second;
+  std::vector Expansions;
+  auto &Spelled = File.SpelledTokens;
+  for (auto Mapping : File.Mappings) {
+const syntax::Token *Token = &Spelled[Mapping.BeginSpelled];
+if (Token->kind() == tok::TokenKind::identifier)
+  Expansions.push_back(Token);
+  }
+  return Expansions;
+}
+
 std::vector syntax::tokenize(FileID FID, const SourceManager 
&SM,
 const LangOptions &LO) {
   std::vector Tokens;
Index: cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp
===
--- cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp
+++ cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp
@@ -755,4 +755,27 @@
   // We don't test assertion failures because death tests are slow.
 }
 
+TEST_F(TokenBufferTest, macroExpansions) {
+  llvm::Annotations Code(R"cpp(
+#define FOO B
+#define FOO2 BA
+#define CALL(X) int X
+#define G CALL(FOO2)
+int B;
+$macro[[FOO]];
+$macro[[CALL]](A);
+$macro[[G]];
+  )cpp");
+  recordTokens(Code.code());
+  auto &SM = *SourceMgr;
+  auto Expansions = Buffer.macroExpansions(SM.getMainFileID());
+  std::vector ExpectedMacroRanges;
+  for (auto Range : Code.ranges("macro"))
+ExpectedMacroRanges.push_back(
+FileRange(SM.getMainFileID(), Range.Begin, Range.End));
+  std::vector ActualMacroRanges;
+  for (auto Expansion : Expansions)
+ActualMacroRanges.push_back(Expansion->range(SM));
+  EXPECT_EQ(ExpectedMacroRanges, ActualMacroRanges);
+}
 } // namespace
Index: cfe/trunk/include/clang/Tooling/Syntax/Tokens.h
===
--- cfe/trunk/include/clang/Tooling/Syntax/Tokens.h
+++ cfe/trunk/include/clang/Tooling/Syntax/Tokens.h
@@ -236,6 +236,16 @@
   ///#pragma, etc.
   llvm::ArrayRef spelledTokens(FileID FID) const;
 
+  /// Get all tokens that expand a macro in \p FID. For the following input
+  /// #define FOO B
+  /// #define FOO2(X) int X
+  /// FOO2(XY)
+  /// int B;
+  /// FOO;
+  /// macroExpansions() returns {"FOO2", "FOO"} (from line 3 and 5
+  /// respecitvely).
+  std::vector macroExpansions(FileID FID) const;
+
   const SourceManager &sourceManager() const { return *SourceMgr; }
 
   std::string dumpForTests() const;


Index: cfe/trunk/lib/Tooling/Syntax/Tokens.cpp
===
--- cfe/trunk/lib/Tooling/Syntax/Tokens.cpp
+++ cfe/trunk/lib/Tooling/Syntax/Tokens.cpp
@@ -232,6 +232,21 @@
   return E;
 }
 
+std::vector
+TokenBuffer::macroExpansions(FileID FID) const {
+  auto FileIt = Files.find(FID);
+  assert(FileIt != Files.end() && "file not tracked by token buffer");
+  auto &File = FileIt->second;
+  std::vector Expansions;
+  auto &Spelled = File.SpelledTokens;
+  for (auto Mapping : File.Mappings) {
+const syntax::Token *Token = &Spelled[Mapping.BeginSpelled];
+if (Token->kind() == tok::TokenKind::identifier)
+  Expansions.push_back(Token);
+  }
+  return Expansions;
+}
+
 std::vector syntax::tokenize(FileID FID, const SourceManager &SM,
 const LangOptions &LO) {
   std::vector Tokens;
Index: cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp
===
--- cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp
+++ cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp
@@ -755,4 +755,27 @@
   // We don't test assertion failures because death tests are slow.
 }
 
+TEST_F(TokenBufferTest, macroExpansions) {
+  llvm::Annotations Code(R"cpp(
+#define FOO B
+#define FOO2 BA
+#define CALL(X) int X
+#define G CALL(FOO2)
+int B;
+$macro[[FOO]];
+$macro[[CALL]](A);
+$macro[[G]];
+  )cpp");
+  recordTokens(Code.code());
+  auto &SM = *SourceMgr;
+  auto Expansions = Buf

Re: r369251 - [OpenCL] Fix addr space deduction for pointers/references to arrays.

[Anastasia Stulova via cfe-commits]

Hi Hans,

Is it still possible to port this fix to the release branch?

Thanks,
Anastasia


From: cfe-commits  on behalf of Anastasia 
Stulova via cfe-commits 
Sent: 19 August 2019 12:43
To: cfe-commits@lists.llvm.org 
Subject: r369251 - [OpenCL] Fix addr space deduction for pointers/references to 
arrays.

Author: stulova
Date: Mon Aug 19 04:43:16 2019
New Revision: 369251

URL: http://llvm.org/viewvc/llvm-project?rev=369251&view=rev
Log:
[OpenCL] Fix addr space deduction for pointers/references to arrays.

Rewrite the logic for detecting if we are deducing addr space of
a pointee type to take into account special logic for arrays. For
pointers/references to arrays we can have any number of parentheses
expressions as well as nested pointers.

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


Modified:
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=369251&r1=369250&r2=369251&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Mon Aug 19 04:43:16 2019
@@ -7385,8 +7385,22 @@ static void deduceOpenCLImplicitAddrSpac
   bool IsPointee =
   ChunkIndex > 0 &&
   (D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Pointer ||
-   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::BlockPointer ||
-   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Reference);
+   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Reference ||
+   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::BlockPointer);
+  // For pointers/references to arrays the next chunk is always an array
+  // followed by any number of parentheses.
+  if (!IsPointee && ChunkIndex > 1) {
+auto AdjustedCI = ChunkIndex - 1;
+if (D.getTypeObject(AdjustedCI).Kind == DeclaratorChunk::Array)
+  AdjustedCI--;
+// Skip over all parentheses.
+while (AdjustedCI > 0 &&
+   D.getTypeObject(AdjustedCI).Kind == DeclaratorChunk::Paren)
+  AdjustedCI--;
+if (D.getTypeObject(AdjustedCI).Kind == DeclaratorChunk::Pointer ||
+D.getTypeObject(AdjustedCI).Kind == DeclaratorChunk::Reference)
+  IsPointee = true;
+  }
   bool IsFuncReturnType =
   ChunkIndex > 0 &&
   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Function;

Modified: cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl?rev=369251&r1=369250&r2=369251&view=diff
==
--- cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl (original)
+++ cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl Mon Aug 19 04:43:16 
2019
@@ -78,3 +78,25 @@ __kernel void test() {
   int foo[10];
   xxx(&foo[0]);
 }
+
+// Addr space for pointer/reference to an array
+//CHECK: FunctionDecl {{.*}} t1 'void (const __generic float (&)[2])'
+void t1(const float (&fYZ)[2]);
+//CHECK: FunctionDecl {{.*}} t2 'void (const __generic float (*)[2])'
+void t2(const float (*fYZ)[2]);
+//CHECK: FunctionDecl {{.*}} t3 'void (__generic float (((*)))[2])'
+void t3(float(((*fYZ)))[2]);
+//CHECK: FunctionDecl {{.*}} t4 'void (__generic float (((*__generic *)))[2])'
+void t4(float(((**fYZ)))[2]);
+//CHECK: FunctionDecl {{.*}} t5 'void (__generic float (*__generic (*))[2])'
+void t5(float (*(*fYZ))[2]);
+
+__kernel void k() {
+  __local float x[2];
+  __local float(*p)[2];
+  t1(x);
+  t2(&x);
+  t3(&x);
+  t4(&p);
+  t5(&p);
+}


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

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 216136.
hokein marked 2 inline comments as done.
hokein added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66349

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

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2037,35 +2037,36 @@
 TEST(FindReferences, ExplicitSymbols) {
   const char *Tests[] = {
   R"cpp(
-  struct Foo { Foo* [self]() const; };
+  struct Foo { Foo* [[self]]() const; };
   void f() {
-if (Foo* T = foo.[^self]()) {} // Foo member call expr.
+Foo foo;
+if (Foo* T = foo.[[^self]]()) {} // Foo member call expr.
   }
   )cpp",
 
   R"cpp(
   struct Foo { Foo(int); };
   Foo f() {
-int [b];
-return [^b]; // Foo constructor expr.
+int [[b]];
+return [[^b]]; // Foo constructor expr.
   }
   )cpp",
 
   R"cpp(
   struct Foo {};
   void g(Foo);
-  Foo [f]();
+  Foo [[f]]();
   void call() {
-g([^f]());  // Foo constructor expr.
+g([[^f]]());  // Foo constructor expr.
   }
   )cpp",
 
   R"cpp(
-  void [foo](int);
-  void [foo](double);
+  void [[foo]](int);
+  void [[foo]](double);
 
   namespace ns {
-  using ::[fo^o];
+  using ::[[fo^o]];
   }
   )cpp",
   };
@@ -2075,6 +2076,7 @@
 std::vector> ExpectedLocations;
 for (const auto &R : T.ranges())
   ExpectedLocations.push_back(RangeIs(R));
+ASSERT_THAT(ExpectedLocations, Not(IsEmpty()));
 EXPECT_THAT(findReferences(AST, T.point(), 0),
 ElementsAreArray(ExpectedLocations))
 << Test;
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -370,7 +370,6 @@
 class ReferenceFinder : public index::IndexDataConsumer {
 public:
   struct Reference {
-const Decl *CanonicalTarget;
 SourceLocation Loc;
 index::SymbolRoleSet Role;
   };
@@ -384,17 +383,15 @@
 
   std::vector take() && {
 llvm::sort(References, [](const Reference &L, const Reference &R) {
-  return std::tie(L.Loc, L.CanonicalTarget, L.Role) <
- std::tie(R.Loc, R.CanonicalTarget, R.Role);
+  return std::tie(L.Loc, L.Role) < std::tie(R.Loc, R.Role);
 });
 // We sometimes see duplicates when parts of the AST get traversed twice.
-References.erase(
-std::unique(References.begin(), References.end(),
-[](const Reference &L, const Reference &R) {
-  return std::tie(L.CanonicalTarget, L.Loc, L.Role) ==
- std::tie(R.CanonicalTarget, R.Loc, R.Role);
-}),
-References.end());
+References.erase(std::unique(References.begin(), References.end(),
+ [](const Reference &L, const Reference &R) {
+   return std::tie(L.Loc, L.Role) ==
+  std::tie(R.Loc, R.Role);
+ }),
+ References.end());
 return std::move(References);
   }
 
@@ -407,7 +404,7 @@
 const SourceManager &SM = AST.getSourceManager();
 Loc = SM.getFileLoc(Loc);
 if (isInsideMainFile(Loc, SM) && CanonicalTargets.count(D))
-  References.push_back({D, Loc, Roles});
+  References.push_back({Loc, Roles});
 return true;
   }
 
@@ -441,6 +438,8 @@
   // FIXME: show references to macro within file?
   auto References = findRefs(Symbols.Decls, AST);
 
+  // FIXME: we may get multiple DocumentHighlights with the same location and
+  // different kinds, deduplicate them.
   std::vector Result;
   for (const auto &Ref : References) {
 if (auto Range =
@@ -951,6 +950,15 @@
   // We traverse the AST to find references in the main file.
   // TODO: should we handle macros, too?
   auto MainFileRefs = findRefs(Symbols.Decls, AST);
+  // We may get multiple refs with the same location and different Roles, as
+  // cross-reference is only interested in locations, we deduplicate them
+  // by the location to avoid emitting duplicated locations.
+  MainFileRefs.erase(std::unique(MainFileRefs.begin(), MainFileRefs.end(),
+ [](const ReferenceFinder::Reference &L,
+const ReferenceFinder::Reference &R) {
+   return L.Loc == R.Loc;
+ }),
+ MainFileRefs.end());
   for (const auto &Ref : MainFileRefs) {
 if (auto Range =
 get

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66349



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

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp:1
+// RUN: %check_clang_tidy %s bugprone-dynamic-static-initializers %t
+

aaron.ballman wrote:
> I'm a bit confused.
> 
> 1) Why is this file a .hpp if the check is currently supposed to be C-only?
> 2) Why are there no `#include` directives in the test if the check is 
> supposed to only work on header files?
> 
> As best I can tell, this is a moral equivalent to doing: clang -x c 
> whatever.h, and so this should be a compilation unit and not a header file. 
> @alexfh, do you have thoughts there?
> Why is this file a .hpp if the check is currently supposed to be C-only?
This seems to have been cleared in a different comment: the check is C++-only.

> Why are there no #include directives in the test if the check is supposed to 
> only work on header files?
Since the check is using a list of extensions to figure out, if a file is a 
header, it's not overly important to actually add a separate file that includes 
this one. And there's a certain benefit in convenience of not doing so (it's 
nice when CHECK-... and RUN: directives just work).  At least one more check - 
misc-definitions-in-headers - does it the same way.


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

https://reviews.llvm.org/D62829



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

[PATCH] D66473: Removed some dead code in BugReporter and related files

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
gribozavr added a reviewer: NoQ.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66473

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h
  clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/unittests/StaticAnalyzer/Reusables.h

Index: clang/unittests/StaticAnalyzer/Reusables.h
===
--- clang/unittests/StaticAnalyzer/Reusables.h
+++ clang/unittests/StaticAnalyzer/Reusables.h
@@ -58,7 +58,7 @@
   ExprEngineConsumer(CompilerInstance &C)
   : C(C), ChkMgr(C.getASTContext(), *C.getAnalyzerOpts()), CTU(C),
 Consumers(),
-AMgr(C.getASTContext(), C.getDiagnostics(), Consumers,
+AMgr(C.getASTContext(), Consumers,
  CreateRegionStoreManager, CreateRangeConstraintManager, &ChkMgr,
  *C.getAnalyzerOpts()),
 VisitedCallees(), FS(),
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -311,9 +311,9 @@
 checkerMgr = createCheckerManager(
 *Ctx, *Opts, Plugins, CheckerRegistrationFns, PP.getDiagnostics());
 
-Mgr = std::make_unique(
-*Ctx, PP.getDiagnostics(), PathConsumers, CreateStoreMgr,
-CreateConstraintMgr, checkerMgr.get(), *Opts, Injector);
+Mgr = std::make_unique(*Ctx, PathConsumers, CreateStoreMgr,
+CreateConstraintMgr,
+checkerMgr.get(), *Opts, Injector);
   }
 
   /// Store the top level decls in the set to be processed later on.
Index: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -53,17 +53,6 @@
 using namespace clang;
 using namespace ento;
 
-bool PathDiagnosticMacroPiece::containsEvent() const {
-  for (const auto &P : subPieces) {
-if (isa(*P))
-  return true;
-if (const auto *MP = dyn_cast(P.get()))
-  if (MP->containsEvent())
-return true;
-  }
-  return false;
-}
-
 static StringRef StripTrailingDots(StringRef s) {
   for (StringRef::size_type i = s.size(); i != 0; --i)
 if (s[i - 1] != '.')
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1166,41 +1166,6 @@
   ID.AddBoolean(EnableNullFPSuppression);
 }
 
-void FindLastStoreBRVisitor::registerStatementVarDecls(
-BugReport &BR, const Stmt *S, bool EnableNullFPSuppression,
-TrackingKind TKind) {
-
-  const ExplodedNode *N = BR.getErrorNode();
-  std::deque WorkList;
-  WorkList.push_back(S);
-
-  while (!WorkList.empty()) {
-const Stmt *Head = WorkList.front();
-WorkList.pop_front();
-
-ProgramStateManager &StateMgr = N->getState()->getStateManager();
-
-if (const auto *DR = dyn_cast(Head)) {
-  if (const auto *VD = dyn_cast(DR->getDecl())) {
-const VarRegion *R =
-StateMgr.getRegionManager().getVarRegion(VD, N->getLocationContext());
-
-// What did we load?
-SVal V = N->getSVal(S);
-
-if (V.getAs() || V.getAs()) {
-  // Register a new visitor with the BugReport.
-  BR.addVisitor(std::make_unique(
-  V.castAs(), R, EnableNullFPSuppression, TKind));
-}
-  }
-}
-
-for (const Stmt *SubStmt : Head->children())
-  WorkList.push_back(SubStmt);
-  }
-}
-
 /// Returns true if \p N represents the DeclStmt declaring and initializing
 /// \p VR.
 static bool isInitializationOfVar(const ExplodedNode *N, const VarRegion *VR) {
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2049,8 +2049,6 @@
 // Methods for BugReport and subclasses.
 //===--===//
 
-void BugReport::NodeResolver::anchor() {}
-
 void BugReport::addVisitor(std::unique_ptr visitor) {
   if (!visitor)
 return;
@@ -2218,10 +2216,6 

[clang-tools-extra] r369387 - [clangd] Fix one testcase in XRefsTests.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Tue Aug 20 07:07:27 2019
New Revision: 369387

URL: http://llvm.org/viewvc/llvm-project?rev=369387&view=rev
Log:
[clangd] Fix one testcase in XRefsTests.

Summary:
The test didn't test anything actually -- it used "[]" as annotation which 
should be
"[[]]".

This patch also fixes a bug in XRef where we may return duplicated refs.

Reviewers: ilya-biryukov

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

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=369387&r1=369386&r2=369387&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Tue Aug 20 07:07:27 2019
@@ -370,7 +370,6 @@ namespace {
 class ReferenceFinder : public index::IndexDataConsumer {
 public:
   struct Reference {
-const Decl *CanonicalTarget;
 SourceLocation Loc;
 index::SymbolRoleSet Role;
   };
@@ -384,17 +383,15 @@ public:
 
   std::vector take() && {
 llvm::sort(References, [](const Reference &L, const Reference &R) {
-  return std::tie(L.Loc, L.CanonicalTarget, L.Role) <
- std::tie(R.Loc, R.CanonicalTarget, R.Role);
+  return std::tie(L.Loc, L.Role) < std::tie(R.Loc, R.Role);
 });
 // We sometimes see duplicates when parts of the AST get traversed twice.
-References.erase(
-std::unique(References.begin(), References.end(),
-[](const Reference &L, const Reference &R) {
-  return std::tie(L.CanonicalTarget, L.Loc, L.Role) ==
- std::tie(R.CanonicalTarget, R.Loc, R.Role);
-}),
-References.end());
+References.erase(std::unique(References.begin(), References.end(),
+ [](const Reference &L, const Reference &R) {
+   return std::tie(L.Loc, L.Role) ==
+  std::tie(R.Loc, R.Role);
+ }),
+ References.end());
 return std::move(References);
   }
 
@@ -407,7 +404,7 @@ public:
 const SourceManager &SM = AST.getSourceManager();
 Loc = SM.getFileLoc(Loc);
 if (isInsideMainFile(Loc, SM) && CanonicalTargets.count(D))
-  References.push_back({D, Loc, Roles});
+  References.push_back({Loc, Roles});
 return true;
   }
 
@@ -441,6 +438,8 @@ std::vector findDocum
   // FIXME: show references to macro within file?
   auto References = findRefs(Symbols.Decls, AST);
 
+  // FIXME: we may get multiple DocumentHighlights with the same location and
+  // different kinds, deduplicate them.
   std::vector Result;
   for (const auto &Ref : References) {
 if (auto Range =
@@ -951,6 +950,15 @@ std::vector findReferences(Par
   // We traverse the AST to find references in the main file.
   // TODO: should we handle macros, too?
   auto MainFileRefs = findRefs(Symbols.Decls, AST);
+  // We may get multiple refs with the same location and different Roles, as
+  // cross-reference is only interested in locations, we deduplicate them
+  // by the location to avoid emitting duplicated locations.
+  MainFileRefs.erase(std::unique(MainFileRefs.begin(), MainFileRefs.end(),
+ [](const ReferenceFinder::Reference &L,
+const ReferenceFinder::Reference &R) {
+   return L.Loc == R.Loc;
+ }),
+ MainFileRefs.end());
   for (const auto &Ref : MainFileRefs) {
 if (auto Range =
 getTokenRange(AST.getASTContext().getSourceManager(),

Modified: clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp?rev=369387&r1=369386&r2=369387&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp Tue Aug 20 07:07:27 
2019
@@ -2037,35 +2037,36 @@ TEST(FindReferences, WithinAST) {
 TEST(FindReferences, ExplicitSymbols) {
   const char *Tests[] = {
   R"cpp(
-  struct Foo { Foo* [self]() const; };
+  struct Foo { Foo* [[self]]() const; };
   void f() {
-if (Foo* T = foo.[^self]()) {} // Foo member call expr.
+Foo foo;
+if (Foo* T = foo.[[^self]]()) {} // Foo member call expr.
   }
   )cpp",
 
   R"cpp(
   struct Foo { Foo(int); };
   Foo f() {
-int [b];
-return [^b]; // Foo constructor expr.
+int [[b]];
+return [[^b]]; // 

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369387: [clangd] Fix one testcase in XRefsTests. (authored 
by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66349?vs=216136&id=216140#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66349

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

Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -370,7 +370,6 @@
 class ReferenceFinder : public index::IndexDataConsumer {
 public:
   struct Reference {
-const Decl *CanonicalTarget;
 SourceLocation Loc;
 index::SymbolRoleSet Role;
   };
@@ -384,17 +383,15 @@
 
   std::vector take() && {
 llvm::sort(References, [](const Reference &L, const Reference &R) {
-  return std::tie(L.Loc, L.CanonicalTarget, L.Role) <
- std::tie(R.Loc, R.CanonicalTarget, R.Role);
+  return std::tie(L.Loc, L.Role) < std::tie(R.Loc, R.Role);
 });
 // We sometimes see duplicates when parts of the AST get traversed twice.
-References.erase(
-std::unique(References.begin(), References.end(),
-[](const Reference &L, const Reference &R) {
-  return std::tie(L.CanonicalTarget, L.Loc, L.Role) ==
- std::tie(R.CanonicalTarget, R.Loc, R.Role);
-}),
-References.end());
+References.erase(std::unique(References.begin(), References.end(),
+ [](const Reference &L, const Reference &R) {
+   return std::tie(L.Loc, L.Role) ==
+  std::tie(R.Loc, R.Role);
+ }),
+ References.end());
 return std::move(References);
   }
 
@@ -407,7 +404,7 @@
 const SourceManager &SM = AST.getSourceManager();
 Loc = SM.getFileLoc(Loc);
 if (isInsideMainFile(Loc, SM) && CanonicalTargets.count(D))
-  References.push_back({D, Loc, Roles});
+  References.push_back({Loc, Roles});
 return true;
   }
 
@@ -441,6 +438,8 @@
   // FIXME: show references to macro within file?
   auto References = findRefs(Symbols.Decls, AST);
 
+  // FIXME: we may get multiple DocumentHighlights with the same location and
+  // different kinds, deduplicate them.
   std::vector Result;
   for (const auto &Ref : References) {
 if (auto Range =
@@ -951,6 +950,15 @@
   // We traverse the AST to find references in the main file.
   // TODO: should we handle macros, too?
   auto MainFileRefs = findRefs(Symbols.Decls, AST);
+  // We may get multiple refs with the same location and different Roles, as
+  // cross-reference is only interested in locations, we deduplicate them
+  // by the location to avoid emitting duplicated locations.
+  MainFileRefs.erase(std::unique(MainFileRefs.begin(), MainFileRefs.end(),
+ [](const ReferenceFinder::Reference &L,
+const ReferenceFinder::Reference &R) {
+   return L.Loc == R.Loc;
+ }),
+ MainFileRefs.end());
   for (const auto &Ref : MainFileRefs) {
 if (auto Range =
 getTokenRange(AST.getASTContext().getSourceManager(),
Index: clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp
@@ -2037,35 +2037,36 @@
 TEST(FindReferences, ExplicitSymbols) {
   const char *Tests[] = {
   R"cpp(
-  struct Foo { Foo* [self]() const; };
+  struct Foo { Foo* [[self]]() const; };
   void f() {
-if (Foo* T = foo.[^self]()) {} // Foo member call expr.
+Foo foo;
+if (Foo* T = foo.[[^self]]()) {} // Foo member call expr.
   }
   )cpp",
 
   R"cpp(
   struct Foo { Foo(int); };
   Foo f() {
-int [b];
-return [^b]; // Foo constructor expr.
+int [[b]];
+return [[^b]]; // Foo constructor expr.
   }
   )cpp",
 
   R"cpp(
   struct Foo {};
   void g(Foo);
-  Foo [f]();
+  Foo [[f]]();
   void call() {
-g([^f]());  // Foo constructor expr.
+g([[^f]]());  // Foo constructor expr.
   }
   )cpp",
 
   R"cpp(
-  void [foo](int);
-  void [foo](double);
+  void [[foo]](int);
+  void [[foo]](double);
 
   namespace ns {
-  using ::[fo^o];
+  using ::[[fo^o]];
   }
   )cpp",
   };
@@ -2075,6 +2076,7 @@
 std::vector> E

[PATCH] D66473: Removed some dead code in BugReporter and related files

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Yay, that'll make my life a lot easier! I heard you have an automatic tool for 
this sort of stuff, right?




Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:408
-public:
-  enum Kind { BasicBRKind, PathSensitiveBRKind };
-

Hey, i just added that! :D (well, renamed) (rC369320)

I believe we do want a separation between a {bug report, bug reporter} classes 
that's only suitable for path-insensitive reports (which would live in 
libAnalysis and we could handle them to clang-tidy while still being able to 
compile it without CLANG_ENABLE_STATIC_ANALYZER) and all the path-sensitive 
report logic that is pretty huge but only Static Analyzer needs it. For that 
purpose we'd better leave this in. WDYT? See also D66460.

Should i ask on the mailing list whether you're willing to sacrifice building 
clang-tidy without CLANG_ENABLE_STATIC_ANALYZER in order to transition to 
BugReporter? Cause i thought it was obvious that it's not a great idea, even if 
it causes me to do a bit of cleanup work on my end.

That said, i'm surprised that it's deadcode, i.e. that nobody ever dyn_casts 
bug reporters, even though we already have two bug reporter classes. Maybe we 
can indeed remove this facility.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2343
   InterExplodedGraphMap ForwardMap;
-  TrimmedGraph = OriginalGraph->trim(Nodes, &ForwardMap, &InverseMap);
 

Btw these days we strongly suspect that the whole graph trimming thing is 
useless and should be removed.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1169
 
-void FindLastStoreBRVisitor::registerStatementVarDecls(
-BugReport &BR, const Stmt *S, bool EnableNullFPSuppression,

Woohooo!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66473



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

[PATCH] D65182: [analyzer] Add fix-it hint support.

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:347
+
+  virtual llvm::ArrayRef getFixits() const { return Fixits; }
+

gribozavr wrote:
> Why is it virtual? In fact, why is BugReporter subclassable at all?
This clearly doesn't need to be virtual. I copy-pasted it from nearby functions 
without applying any critical thinking.

However, BugReport(er) most likely needs to be sub-classable because 
D66473#inline-596021, so i promise to clean those up in the process :)


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

https://reviews.llvm.org/D65182



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

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: 
clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp:1
+// RUN: %check_clang_tidy %s bugprone-dynamic-static-initializers %t
+

alexfh wrote:
> aaron.ballman wrote:
> > I'm a bit confused.
> > 
> > 1) Why is this file a .hpp if the check is currently supposed to be C-only?
> > 2) Why are there no `#include` directives in the test if the check is 
> > supposed to only work on header files?
> > 
> > As best I can tell, this is a moral equivalent to doing: clang -x c 
> > whatever.h, and so this should be a compilation unit and not a header file. 
> > @alexfh, do you have thoughts there?
> > Why is this file a .hpp if the check is currently supposed to be C-only?
> This seems to have been cleared in a different comment: the check is C++-only.
> 
> > Why are there no #include directives in the test if the check is supposed 
> > to only work on header files?
> Since the check is using a list of extensions to figure out, if a file is a 
> header, it's not overly important to actually add a separate file that 
> includes this one. And there's a certain benefit in convenience of not doing 
> so (it's nice when CHECK-... and RUN: directives just work).  At least one 
> more check - misc-definitions-in-headers - does it the same way.
Awesome, thank you for weighing in, @alexfh!


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

https://reviews.llvm.org/D62829



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

[PATCH] D66478: [clangd] Ignore implicit conversion-operator nodes in find refs.

[Haojian Wu via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66478

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


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2069,6 +2069,18 @@
   using ::[[fo^o]];
   }
   )cpp",
+
+  R"cpp(
+  struct X {
+operator bool();
+  };
+
+  int test() {
+X [[a]];
+[[a]].operator bool();
+if ([[a^]]) {} // ignore implicit conversion-operator AST node
+  }
+)cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -187,6 +187,11 @@
 // experssion is impossible to write down.
 if (const auto *CtorExpr = dyn_cast(E))
   return CtorExpr->getParenOrBraceRange().isInvalid();
+// Ignore implicit conversion-operator AST node.
+if (const auto *ME = dyn_cast(E)) {
+  if (isa(ME->getMemberDecl()))
+return ME->getMemberLoc().isInvalid();
+}
 return isa(E);
   };
 


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2069,6 +2069,18 @@
   using ::[[fo^o]];
   }
   )cpp",
+
+  R"cpp(
+  struct X {
+operator bool();
+  };
+
+  int test() {
+X [[a]];
+[[a]].operator bool();
+if ([[a^]]) {} // ignore implicit conversion-operator AST node
+  }
+)cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -187,6 +187,11 @@
 // experssion is impossible to write down.
 if (const auto *CtorExpr = dyn_cast(E))
   return CtorExpr->getParenOrBraceRange().isInvalid();
+// Ignore implicit conversion-operator AST node.
+if (const auto *ME = dyn_cast(E)) {
+  if (isa(ME->getMemberDecl()))
+return ME->getMemberLoc().isInvalid();
+}
 return isa(E);
   };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57659: [Sema] SequenceChecker: Add some comments + related small NFCs in preparation of the following patches

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 216152.
riccibruno added a comment.

Rebased. Thanks for the review @xbolva00 !


Repository:
  rC Clang

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

https://reviews.llvm.org/D57659

Files:
  clang/lib/Sema/SemaChecking.cpp

Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -12161,20 +12161,38 @@
 UK_Count = UK_ModAsSideEffect + 1
   };
 
+  /// Bundle together a sequencing region and the expression corresponding
+  /// to a specific usage. One Usage is stored for each usage kind in UsageInfo.
   struct Usage {
-Expr *Use;
+Expr *UsageExpr = nullptr;
 SequenceTree::Seq Seq;
-
-Usage() : Use(nullptr), Seq() {}
+Usage() = default;
+Usage(Expr *UsageExpr, SequenceTree::Seq Seq)
+: UsageExpr(UsageExpr), Seq(Seq) {}
   };
 
-  struct UsageInfo {
-Usage Uses[UK_Count];
+  class UsageInfo {
+/// The expressions corresponding to each usage kind.
+Expr *UsageExprs[UK_Count]{};
+
+/// The sequencing regions corresponding to each usage kind.
+SequenceTree::Seq Seqs[UK_Count]{};
 
-/// Have we issued a diagnostic for this variable already?
-bool Diagnosed;
+/// Have we issued a diagnostic for this object already?
+bool Diagnosed = false;
 
-UsageInfo() : Uses(), Diagnosed(false) {}
+  public:
+Usage getUsage(UsageKind UK) const {
+  assert(UK < UK_Count && "Invalid UsageKind!");
+  return Usage{UsageExprs[UK], Seqs[UK]};
+}
+void setUsage(UsageKind UK, Usage U) {
+  assert(UK < UK_Count && "Invalid UsageKind!");
+  UsageExprs[UK] = U.UsageExpr;
+  Seqs[UK] = U.Seq;
+}
+void markDiagnosed() { Diagnosed = true; }
+bool alreadyDiagnosed() const { return Diagnosed; }
   };
   using UsageInfoMap = llvm::SmallDenseMap;
 
@@ -12209,11 +12227,14 @@
 }
 
 ~SequencedSubexpression() {
-  for (auto &M : llvm::reverse(ModAsSideEffect)) {
-UsageInfo &U = Self.UsageMap[M.first];
-auto &SideEffectUsage = U.Uses[UK_ModAsSideEffect];
-Self.addUsage(U, M.first, SideEffectUsage.Use, UK_ModAsValue);
-SideEffectUsage = M.second;
+  for (std::pair &M : llvm::reverse(ModAsSideEffect)) {
+// Add a new usage with usage kind UK_ModAsValue, and then restore
+// the previous usage with UK_ModAsSideEffect (thus clearing it if
+// the previous one was empty).
+UsageInfo &UI = Self.UsageMap[M.first];
+Usage SideEffectUsage = UI.getUsage(UK_ModAsSideEffect);
+Self.addUsage(M.first, UI, SideEffectUsage.UsageExpr, UK_ModAsValue);
+UI.setUsage(UK_ModAsSideEffect, M.second);
   }
   Self.ModAsSideEffect = OldModAsSideEffect;
 }
@@ -12277,28 +12298,34 @@
   }
 
   /// Note that an object was modified or used by an expression.
-  void addUsage(UsageInfo &UI, Object O, Expr *Ref, UsageKind UK) {
-Usage &U = UI.Uses[UK];
-if (!U.Use || !Tree.isUnsequenced(Region, U.Seq)) {
+  /// UI is the UsageInfo for the object O as obtained via the UsageMap.
+  void addUsage(Object O, UsageInfo &UI, Expr *UsageExpr, UsageKind UK) {
+// Get the old usage for the given object and usage kind.
+Usage U = UI.getUsage(UK);
+if (!U.UsageExpr || !Tree.isUnsequenced(Region, U.Seq)) {
+  // If we have a modification as side effect and are in a sequenced
+  // subexpression, save the old Usage so that we can restore it later
+  // in SequencedSubexpression::~SequencedSubexpression.
   if (UK == UK_ModAsSideEffect && ModAsSideEffect)
 ModAsSideEffect->push_back(std::make_pair(O, U));
-  U.Use = Ref;
-  U.Seq = Region;
+  // Then record the new usage with the current sequencing region.
+  UI.setUsage(UK, Usage(UsageExpr, Region));
 }
   }
 
   /// Check whether a modification or use conflicts with a prior usage.
-  void checkUsage(Object O, UsageInfo &UI, Expr *Ref, UsageKind OtherKind,
+  /// UI is the UsageInfo for the object O as obtained via the UsageMap.
+  void checkUsage(Object O, UsageInfo &UI, Expr *UsageExpr, UsageKind OtherKind,
   bool IsModMod) {
-if (UI.Diagnosed)
+if (UI.alreadyDiagnosed())
   return;
 
-const Usage &U = UI.Uses[OtherKind];
-if (!U.Use || !Tree.isUnsequenced(Region, U.Seq))
+Usage U = UI.getUsage(OtherKind);
+if (!U.UsageExpr || !Tree.isUnsequenced(Region, U.Seq))
   return;
 
-Expr *Mod = U.Use;
-Expr *ModOrUse = Ref;
+Expr *Mod = U.UsageExpr;
+Expr *ModOrUse = UsageExpr;
 if (OtherKind == UK_Use)
   std::swap(Mod, ModOrUse);
 
@@ -12307,32 +12334,60 @@
 SemaRef.PDiag(IsModMod ? diag::warn_unsequenced_mod_mod
: diag::warn_unsequenced_mod_use)
 << O << SourceRange(ModOrUse->getExprLoc()));
-UI.Diagnosed = true;
+UI.markDiagnosed();
   }

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 216153.
riccibruno added a comment.

Rebased


Repository:
  rC Clang

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

https://reviews.llvm.org/D57660

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/warn-unsequenced.cpp

Index: clang/test/SemaCXX/warn-unsequenced.cpp
===
--- clang/test/SemaCXX/warn-unsequenced.cpp
+++ clang/test/SemaCXX/warn-unsequenced.cpp
@@ -160,17 +160,21 @@
   void member_f(S1 &s);
 };
 
+class SomeClass { public: static int x; };
+union SomeUnion { public: static int x; };
+
 void S1::member_f(S1 &s) {
-  ++a + ++a; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
- // cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
-  a + ++a; // cxx11-warning {{unsequenced modification and access to 'a'}}
-   // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
+  ++a + ++a; // cxx11-warning {{multiple unsequenced modifications to member 'a'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a'}}
+  a + ++a; // cxx11-warning {{unsequenced modification and access to member 'a'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'a'}}
   ++a + ++b; // no-warning
   a + ++b; // no-warning
 
-  // TODO: Warn here.
-  ++s.a + ++s.a; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.a + ++s.a; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.a + ++s.a; // cxx11-warning {{multiple unsequenced modifications to member 'a' of 's'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a' of 's'}}
+  s.a + ++s.a; // cxx11-warning {{unsequenced modification and access to member 'a' of 's'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'a' of 's'}}
   ++s.a + ++s.b; // no-warning
   s.a + ++s.b; // no-warning
 
@@ -180,16 +184,18 @@
   a + ++s.b; // no-warning
 
   // TODO Warn here for bit-fields in the same memory location.
-  ++bf1 + ++bf1; // cxx11-warning {{multiple unsequenced modifications to 'bf1'}}
- // cxx17-warning@-1 {{multiple unsequenced modifications to 'bf1'}}
-  bf1 + ++bf1; // cxx11-warning {{unsequenced modification and access to 'bf1'}}
-   // cxx17-warning@-1 {{unsequenced modification and access to 'bf1'}}
+  ++bf1 + ++bf1; // cxx11-warning {{multiple unsequenced modifications to member 'bf1'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'bf1'}}
+  bf1 + ++bf1; // cxx11-warning {{unsequenced modification and access to member 'bf1'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'bf1'}}
   ++bf1 + ++bf2; // no-warning TODO {{multiple unsequenced modifications to}}
   bf1 + ++bf2; // no-warning TODO {{unsequenced modification and access to}}
 
   // TODO Warn here for bit-fields in the same memory location.
-  ++s.bf1 + ++s.bf1; // no-warning TODO {{multiple unsequenced modifications to}}
-  s.bf1 + ++s.bf1; // no-warning TODO {{unsequenced modification and access to}}
+  ++s.bf1 + ++s.bf1; // cxx11-warning {{multiple unsequenced modifications to member 'bf1' of 's'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to member 'bf1' of 's'}}
+  s.bf1 + ++s.bf1; // cxx11-warning {{unsequenced modification and access to member 'bf1' of 's'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to member 'bf1' of 's'}}
   ++s.bf1 + ++s.bf2; // no-warning TODO {{multiple unsequenced modifications to}}
   s.bf1 + ++s.bf2; // no-warning TODO {{unsequenced modification and access to}}
 
@@ -203,19 +209,29 @@
   Der &d_ref = d;
   S1 &s1_ref = d_ref;
 
-  ++s1_ref.a + ++d_ref.a; // no-warning TODO {{multiple unsequenced modifications to member 'a' of 'd'}}
-  ++s1_ref.a + d_ref.a; // no-warning TODO {{unsequenced modification and access to member 'a' of 'd'}}
+  ++s1_ref.a + ++d_ref.a; // cxx11-warning {{multiple unsequenced modifications to member 'a' of 'd'}}
+  // cxx17-warning@-1 {{multiple unsequenced modifications to member 'a' of 'd'}}
+  ++s1_ref.a + d_ref.a; // cxx11-warning {{unsequenced modification and access to member 'a' of 'd'}}
+// cxx17-warning@-1 {{unsequenced modification and access to member 'a' of 'd'}}
   ++s1_ref.a + ++d_ref.b; // no-warning
   ++s1_ref.a + d_ref.b; // no-warning
 
-  ++x + ++x; // cxx11-warning {{multiple unsequenced modifications to 'x'}}
- // cxx17-warning@-1 {{multiple unsequenced modifications to 'x'}}
-  ++x + x; // cxx11-warning {{unsequenced modification and access to 'x'}}
-   // cxx17-warning@-1 {{unsequenced modification and access to 'x'}}
-  ++s.x + x; // no-warning TODO {{un

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: test/CodeGenCXX/debug-info-atexit-stub.cpp:14
+
+// CHECK: define internal void @"??__Ff@?1??d@@YAPEAU?$c@UBXZ@YAXXZ"()
+// CHECK-SAME: !dbg ![[SUBPROGRAM:[0-9]+]] {

Do these Windows-mangled names work on Linux?


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

https://reviews.llvm.org/D66328



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

[PATCH] D57747: [Sema] SequenceChecker: Fix handling of operator ||, && and ?:

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 216156.
riccibruno added a comment.

Rebased


Repository:
  rC Clang

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

https://reviews.llvm.org/D57747

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

Index: clang/test/SemaCXX/warn-unsequenced.cpp
===
--- clang/test/SemaCXX/warn-unsequenced.cpp
+++ clang/test/SemaCXX/warn-unsequenced.cpp
@@ -4,6 +4,8 @@
 // RUN:-Wunsequenced -Wno-c++17-extensions -Wno-c++14-extensions %s
 
 int f(int, int = 0);
+int g1();
+int g2(int);
 
 struct A {
   int x, y;
@@ -79,24 +81,28 @@
   A { ++a, a++ }.x + A { ++a, a++ }.y; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
// cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
 
-  (xs[2] && (a = 0)) + a; // ok
+  (xs[2] && (a = 0)) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
+  // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   (0 && (a = 0)) + a; // ok
   (1 && (a = 0)) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
   // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
 
-  (xs[3] || (a = 0)) + a; // ok
+  (xs[3] || (a = 0)) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
+  // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   (0 || (a = 0)) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
   // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   (1 || (a = 0)) + a; // ok
 
-  (xs[4] ? a : ++a) + a; // ok
+  (xs[4] ? a : ++a) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
+ // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   (0 ? a : ++a) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
  // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   (1 ? a : ++a) + a; // ok
   (0 ? a : a++) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
  // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   (1 ? a : a++) + a; // ok
-  (xs[5] ? ++a : ++a) + a; // FIXME: warn here
+  (xs[5] ? ++a : ++a) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
 
   (++a, xs[6] ? ++a : 0) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
   // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
@@ -122,10 +128,13 @@
   // unconditional.
   a = a++ && f(a, a);
 
-  // This has undefined behavior if a != 0. FIXME: We should diagnose this.
-  (a && a++) + a;
+  // This has undefined behavior if a != 0.
+  (a && a++) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
+  // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
 
-  (xs[7] && ++a) * (!xs[7] && ++a); // ok
+  // FIXME: Don't warn here.
+  (xs[7] && ++a) * (!xs[7] && ++a); // cxx11-warning {{multiple unsequenced modifications to 'a'}}
+// cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
 
   xs[0] = (a = 1, a); // ok
   (a -= 128) &= 128; // ok
@@ -135,13 +144,64 @@
  // cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
   xs[8] ? 0 : ++a + a++; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
  // cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
-  xs[8] ? ++a : a++; // ok
+  xs[8] ? ++a : a++; // no-warning
+  xs[8] ? a+=1 : a+= 2; // no-warning
+  (xs[8] ? a+=1 : a+= 2) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
+  // TODO cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
+  (xs[8] ? a+=1 : a) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
+  // TODO cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
+  (xs[8] ? a : a+= 2) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
+   // TODO cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
+  a = (xs[8] ? a+=1 : a+= 2); // no-warning
+  a += (xs[8] ? a+=1 : a+= 2); // cxx11-warning {{unsequenced modification and access to 'a'}}
+   // TODO cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
+
+  (false ? a+=1 : a) = a; // no-warning
+  (true ? a+=1 : a) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
+ // TODO cxx17-warning@-1 {{unsequenced modification and acce

[PATCH] D58297: [Sema] SequenceChecker: C++17 sequencing rules for built-in operators <<, >>, .*, ->*, =, op=

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 216157.
riccibruno added a comment.

Rebased


Repository:
  rC Clang

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

https://reviews.llvm.org/D58297

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CXX/drs/dr2xx.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/SemaCXX/warn-unsequenced.cpp

Index: clang/test/SemaCXX/warn-unsequenced.cpp
===
--- clang/test/SemaCXX/warn-unsequenced.cpp
+++ clang/test/SemaCXX/warn-unsequenced.cpp
@@ -26,7 +26,6 @@
   a + a++; // cxx11-warning {{unsequenced modification and access to 'a'}}
// cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   a = a++; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
-   // TODO cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
   ++ ++a; // ok
   (a++, a++); // ok
   ++a + ++a; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
@@ -36,13 +35,10 @@
   (a++, a) = 0; // ok, increment is sequenced before value computation of LHS
   a = xs[++a]; // ok
   a = xs[a++]; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
-   // TODO cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
   (a ? xs[0] : xs[1]) = ++a; // cxx11-warning {{unsequenced modification and access to 'a'}}
- // TODO cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   a = (++a, ++a); // ok
   a = (a++, ++a); // ok
   a = (a++, a++); // cxx11-warning {{multiple unsequenced modifications to 'a'}}
-  // TODO cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
   f(a, a); // ok
   f(a = 0, a); // cxx11-warning {{unsequenced modification and access to 'a'}}
// cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
@@ -61,7 +57,6 @@
   (++a, a) += 1; // ok
   a = ++a; // ok
   a += ++a; // cxx11-warning {{unsequenced modification and access to 'a'}}
-// TODO cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
 
   A agg1 = { a++, a++ }; // ok
   A agg2 = { a++ + a, a++ }; // cxx11-warning {{unsequenced modification and access to 'a'}}
@@ -77,7 +72,6 @@
   a = S { ++a, a++ }.n; // ok
   A { ++a, a++ }.x; // ok
   a = A { ++a, a++ }.x; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
-// TODO cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
   A { ++a, a++ }.x + A { ++a, a++ }.y; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
// cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
 
@@ -112,14 +106,10 @@
   a += (a++, a) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
  // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
 
-  int *p = xs;
-  a = *(a++, p); // ok
   a = a++ && a; // ok
-  p[(long long unsigned)(p = 0)]; // cxx11-warning {{unsequenced modification and access to 'p'}}
 
   A *q = &agg1;
   (q = &agg2)->y = q->x; // cxx11-warning {{unsequenced modification and access to 'q'}}
- // TODO cxx17-warning@-1 {{unsequenced modification and access to 'q'}}
 
   // This has undefined behavior if a == 0; otherwise, the side-effect of the
   // increment is sequenced before the value computation of 'f(a, a)', which is
@@ -147,20 +137,14 @@
   xs[8] ? ++a : a++; // no-warning
   xs[8] ? a+=1 : a+= 2; // no-warning
   (xs[8] ? a+=1 : a+= 2) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
-  // TODO cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   (xs[8] ? a+=1 : a) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
-  // TODO cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   (xs[8] ? a : a+= 2) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
-   // TODO cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   a = (xs[8] ? a+=1 : a+= 2); // no-warning
   a += (xs[8] ? a+=1 : a+= 2); // cxx11-warning {{unsequenced modification and access to 'a'}}
-   // TODO cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
 
   (false ? a+=1 : a) = a; // no-warning
   (true ? a+=1 : a) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
- // TODO cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   (false ? a : a+=2) = a; // cxx11-warning {{unsequenced modification and access to 'a'}}
-  // TODO cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   (true ? a : a+=2) = a; // no-warning
 
   xs[8] && (++a + a++); // cxx11-warning {{multiple unsequenced modifications to 'a'}}
@@ -180,19 +164,15 @@
   a = ((a++, false) || (a++, false) || (a++, false) || (a++,

[PATCH] D58579: [Sema] SequenceChecker: C++17 sequencing rule for call expressions.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 216158.
riccibruno retitled this revision from "[Sema] SequenceChecker: C++17 
sequencing rule for call expression." to "[Sema] SequenceChecker: C++17 
sequencing rule for call expressions.".
riccibruno added a comment.

Rebased


Repository:
  rC Clang

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

https://reviews.llvm.org/D58579

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


Index: clang/test/SemaCXX/warn-unsequenced.cpp
===
--- clang/test/SemaCXX/warn-unsequenced.cpp
+++ clang/test/SemaCXX/warn-unsequenced.cpp
@@ -15,7 +15,6 @@
   int n;
 };
 
-// TODO: Implement the C++17 sequencing rules.
 void test() {
   int a;
   int xs[10];
@@ -256,6 +255,17 @@
   p[i++] = (i = 42); // cxx11-warning {{multiple unsequenced modifications to 
'i'}}
   p++[i++] = (i = p ? i++ : i++); // cxx11-warning {{unsequenced modification 
and access to 'p'}}
   // cxx11-warning@-1 {{multiple unsequenced 
modifications to 'i'}}
+
+  (i++, f)(i++, 42); // cxx11-warning {{multiple unsequenced modifications to 
'i'}}
+  (i++ + i++, f)(42, 42); // cxx11-warning {{multiple unsequenced 
modifications to 'i'}}
+  // cxx17-warning@-1 {{multiple unsequenced 
modifications to 'i'}}
+  int (*pf)(int, int);
+  (pf = f)(pf != nullptr, pf != nullptr); // cxx11-warning {{unsequenced 
modification and access to 'pf'}}
+  pf((pf = f) != nullptr, 42); // cxx11-warning {{unsequenced modification and 
access to 'pf'}}
+  f((pf = f, 42), (pf = f, 42)); // cxx11-warning {{multiple unsequenced 
modifications to 'pf'}}
+ // cxx17-warning@-1 {{multiple unsequenced 
modifications to 'pf'}}
+  pf((pf = f) != nullptr, pf == nullptr); // cxx11-warning {{unsequenced 
modification and access to 'pf'}}
+  // cxx17-warning@-1 {{unsequenced 
modification and access to 'pf'}}
 }
 
 namespace members {
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -12918,6 +12918,11 @@
   }
 
   void VisitCallExpr(const CallExpr *CE) {
+// FIXME: CXXNewExpr and CXXDeleteExpr implicitly call functions.
+
+if (CE->isUnevaluatedBuiltinCall(Context))
+  return;
+
 // C++11 [intro.execution]p15:
 //   When calling a function [...], every value computation and side effect
 //   associated with any argument expression, or with the postfix 
expression
@@ -12925,9 +12930,40 @@
 //   expression or statement in the body of the function [and thus before
 //   the value computation of its result].
 SequencedSubexpression Sequenced(*this);
-Base::VisitCallExpr(CE);
 
-// FIXME: CXXNewExpr and CXXDeleteExpr implicitly call functions.
+// C++17 [expr.call]p5
+//   The postfix-expression is sequenced before each expression in the
+//   expression-list and any default argument. [...]
+SequenceTree::Seq CalleeRegion;
+SequenceTree::Seq OtherRegion;
+if (SemaRef.getLangOpts().CPlusPlus17) {
+  CalleeRegion = Tree.allocate(Region);
+  OtherRegion = Tree.allocate(Region);
+} else {
+  CalleeRegion = Region;
+  OtherRegion = Region;
+}
+SequenceTree::Seq OldRegion = Region;
+
+// Visit the callee expression first.
+Region = CalleeRegion;
+if (SemaRef.getLangOpts().CPlusPlus17) {
+  SequencedSubexpression Sequenced(*this);
+  Visit(CE->getCallee());
+} else {
+  Visit(CE->getCallee());
+}
+
+// Then visit the argument expressions.
+Region = OtherRegion;
+for (const Expr *Argument : CE->arguments())
+  Visit(Argument);
+
+Region = OldRegion;
+if (SemaRef.getLangOpts().CPlusPlus17) {
+  Tree.merge(CalleeRegion);
+  Tree.merge(OtherRegion);
+}
   }
 
   void VisitCXXConstructExpr(const CXXConstructExpr *CCE) {


Index: clang/test/SemaCXX/warn-unsequenced.cpp
===
--- clang/test/SemaCXX/warn-unsequenced.cpp
+++ clang/test/SemaCXX/warn-unsequenced.cpp
@@ -15,7 +15,6 @@
   int n;
 };
 
-// TODO: Implement the C++17 sequencing rules.
 void test() {
   int a;
   int xs[10];
@@ -256,6 +255,17 @@
   p[i++] = (i = 42); // cxx11-warning {{multiple unsequenced modifications to 'i'}}
   p++[i++] = (i = p ? i++ : i++); // cxx11-warning {{unsequenced modification and access to 'p'}}
   // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
+
+  (i++, f)(i++, 42); // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+  (i++ + i++, f)(42, 42); // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+  // cxx17-warning@-1 {{multiple unsequenced modifications to 'i'}}
+  int (*pf)(int, int);
+  (pf =

[PATCH] D66481: [C++20] Support for lambdas in unevaluated context

[Kris Jusiak via Phabricator via cfe-commits]

krzysztof-jusiak created this revision.
krzysztof-jusiak added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Problem:

- Lambdas in unevaluated context aren't supported in the newest clang yet but 
are required for our usage of jit.

Solution:

- Add support for lambdas in unevaluated context.


Repository:
  rC Clang

https://reviews.llvm.org/D66481

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2.cpp


Index: clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2.cpp
===
--- clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2.cpp
@@ -15,9 +15,9 @@
 };
 
 void unevaluated_operand(P &p, int i) {
-  int i2 = sizeof([] ()->int { return 0; }()); // expected-error{{lambda 
expression in an unevaluated operand}}
+  int i2 = sizeof([] ()->int { return 0; }());
   const std::type_info &ti1 = typeid([&]() -> P& { return p; }());
-  const std::type_info &ti2 = typeid([&]() -> int { return i; }());  // 
expected-error{{lambda expression in an unevaluated operand}}
+  const std::type_info &ti2 = typeid([&]() -> int { return i; }());
 }
 
 template
@@ -37,7 +37,7 @@
   // because the copy-initialization of the capture of boom_float occurs in an
   // unevaluated operand.
   const std::type_info &ti2
-= typeid([=]() -> int { boom_float.tickle(); return 0; }()); // 
expected-error{{lambda expression in an unevaluated operand}}
+= typeid([=]() -> int { boom_float.tickle(); return 0; }());
 
   auto foo = [=]() -> int { boom_double.tickle(); return 0; }; // 
expected-note{{in instantiation of member function 'Boom::Boom' 
requested here}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14577,18 +14577,7 @@
 if (Rec.ExprContext == ExpressionKind::EK_TemplateArgument || 
Rec.isUnevaluated() ||
 (Rec.isConstantEvaluated() && !getLangOpts().CPlusPlus17)) {
   unsigned D;
-  if (Rec.isUnevaluated()) {
-// C++11 [expr.prim.lambda]p2:
-//   A lambda-expression shall not appear in an unevaluated operand
-//   (Clause 5).
-D = diag::err_lambda_unevaluated_operand;
-  } else if (Rec.isConstantEvaluated() && !getLangOpts().CPlusPlus17) {
-// C++1y [expr.const]p2:
-//   A conditional-expression e is a core constant expression unless 
the
-//   evaluation of e, following the rules of the abstract machine, 
would
-//   evaluate [...] a lambda-expression.
-D = diag::err_lambda_in_constant_expression;
-  } else if (Rec.ExprContext == ExpressionKind::EK_TemplateArgument) {
+if (Rec.ExprContext == ExpressionKind::EK_TemplateArgument) {
 // C++17 [expr.prim.lamda]p2:
 // A lambda-expression shall not appear [...] in a template-argument.
 D = diag::err_lambda_in_invalid_context;


Index: clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2.cpp
===
--- clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2.cpp
@@ -15,9 +15,9 @@
 };
 
 void unevaluated_operand(P &p, int i) {
-  int i2 = sizeof([] ()->int { return 0; }()); // expected-error{{lambda expression in an unevaluated operand}}
+  int i2 = sizeof([] ()->int { return 0; }());
   const std::type_info &ti1 = typeid([&]() -> P& { return p; }());
-  const std::type_info &ti2 = typeid([&]() -> int { return i; }());  // expected-error{{lambda expression in an unevaluated operand}}
+  const std::type_info &ti2 = typeid([&]() -> int { return i; }());
 }
 
 template
@@ -37,7 +37,7 @@
   // because the copy-initialization of the capture of boom_float occurs in an
   // unevaluated operand.
   const std::type_info &ti2
-= typeid([=]() -> int { boom_float.tickle(); return 0; }()); // expected-error{{lambda expression in an unevaluated operand}}
+= typeid([=]() -> int { boom_float.tickle(); return 0; }());
 
   auto foo = [=]() -> int { boom_double.tickle(); return 0; }; // expected-note{{in instantiation of member function 'Boom::Boom' requested here}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14577,18 +14577,7 @@
 if (Rec.ExprContext == ExpressionKind::EK_TemplateArgument || Rec.isUnevaluated() ||
 (Rec.isConstantEvaluated() && !getLangOpts().CPlusPlus17)) {
   unsigned D;
-  if (Rec.isUnevaluated()) {
-// C++11 [expr.prim.lambda]p2:
-//   A lambda-expression shall not appear in an unevaluated operand
-//   (Clause 5).
-D = diag::err_lambda_unevaluated_operand;
-  } else if (Rec.isConstantEvaluated() && !getLangOpts().CPlusPlus17) {
-

[PATCH] D66481: [C++20] Support for lambdas in unevaluated context

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

Can you submit the patch with the full context (ie: git diff -U999) ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D66481



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

[PATCH] D66481: [C++20] Support for lambdas in unevaluated context

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Doesn't this inadvertently allow them in every standard? That seems wrong.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66481



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

r369397 - [clang] Use the new Regex::isValid() with no parameter

[Jan Kratochvil via cfe-commits]

Author: jankratochvil
Date: Tue Aug 20 09:07:31 2019
New Revision: 369397

URL: http://llvm.org/viewvc/llvm-project?rev=369397&view=rev
Log:
[clang] Use the new Regex::isValid() with no parameter

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

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

Modified: cfe/trunk/lib/Analysis/CloneDetection.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CloneDetection.cpp?rev=369397&r1=369396&r2=369397&view=diff
==
--- cfe/trunk/lib/Analysis/CloneDetection.cpp (original)
+++ cfe/trunk/lib/Analysis/CloneDetection.cpp Tue Aug 20 09:07:31 2019
@@ -153,9 +153,8 @@ void OnlyLargestCloneConstraint::constra
 
 bool FilenamePatternConstraint::isAutoGenerated(
 const CloneDetector::CloneGroup &Group) {
-  std::string Error;
   if (IgnoredFilesPattern.empty() || Group.empty() ||
-  !IgnoredFilesRegex->isValid(Error))
+  !IgnoredFilesRegex->isValid())
 return false;
 
   for (const StmtSequence &S : Group) {


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

[PATCH] D66364: Diagnose use of _Thread_local as an extension when appropriate

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D66364#1635863 , @ldionne wrote:

> In D66364#1635814 , @aaron.ballman 
> wrote:
>
> > [ ...]
> >
> > Adding some libc++ maintainers to see if they have opinions.
> >
> > `__extension__` is one option. Could we get away with push/pop disabling of 
> > the diagnostic? Or perhaps this is a situation where we should not diagnose 
> > use within a system header in the first place, because that's part of the 
> > implementation?
>
>
> I just learned about `__extension__`, but from my perspective it makes sense 
> to mark uses of `_Atomic` with `__extension__` (or disable the warning with a 
> `#pragma`) inside libc++ if we're using something non-standard for the 
> current dialect. I don't think Clang should bend its back for libc++ in this 
> case.


Okay, that's good feedback, thank you!


Repository:
  rC Clang

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

https://reviews.llvm.org/D66364



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

[PATCH] D66481: [C++20] Support for lambdas in unevaluated context

[Kris Jusiak via Phabricator via cfe-commits]

krzysztof-jusiak added a comment.

In D66481#1637525 , @riccibruno wrote:

> Can you submit the patch with the full context (ie: git diff -U999) ?


Sure, will do it


Repository:
  rC Clang

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

https://reviews.llvm.org/D66481



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

[PATCH] D66481: [C++20] Support for lambdas in unevaluated context

[Kris Jusiak via Phabricator via cfe-commits]

krzysztof-jusiak added a comment.

In D66481#1637530 , @lebedev.ri wrote:

> Doesn't this inadvertently allow them in every standard? That seems wrong.


Good point, you are right, will fix it and resubmit 👍


Repository:
  rC Clang

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

https://reviews.llvm.org/D66481



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

[PATCH] D66302: [SVE][Inline-Asm] Support for SVE asm operands

[Kerry McLaughlin via Phabricator via cfe-commits]

kmclaughlin updated this revision to Diff 216178.
kmclaughlin added a comment.

- Added a new test file, aarch64-sve-asm-negative.ll
- Updated description of the 'y' constraint in LangRef.rst




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

https://reviews.llvm.org/D66302

Files:
  docs/LangRef.rst
  lib/Target/AArch64/AArch64AsmPrinter.cpp
  lib/Target/AArch64/AArch64ISelLowering.cpp
  lib/Target/AArch64/AArch64InstrInfo.cpp
  lib/Target/AArch64/AArch64SVEInstrInfo.td
  test/CodeGen/AArch64/aarch64-sve-asm-negative.ll
  test/CodeGen/AArch64/aarch64-sve-asm.ll
  test/CodeGen/AArch64/arm64-inline-asm.ll

Index: test/CodeGen/AArch64/arm64-inline-asm.ll
===
--- test/CodeGen/AArch64/arm64-inline-asm.ll
+++ test/CodeGen/AArch64/arm64-inline-asm.ll
@@ -138,6 +138,8 @@
   %a = alloca [2 x float], align 4
   %arraydecay = getelementptr inbounds [2 x float], [2 x float]* %a, i32 0, i32 0
   %0 = load <2 x float>, <2 x float>* %data, align 8
+  call void asm sideeffect "ldr ${1:z}, [$0]\0A", "r,w"(float* %arraydecay, <2 x float> %0) nounwind
+  ; CHECK: ldr {{z[0-9]+}}, [{{x[0-9]+}}]
   call void asm sideeffect "ldr ${1:q}, [$0]\0A", "r,w"(float* %arraydecay, <2 x float> %0) nounwind
   ; CHECK: ldr {{q[0-9]+}}, [{{x[0-9]+}}]
   call void asm sideeffect "ldr ${1:d}, [$0]\0A", "r,w"(float* %arraydecay, <2 x float> %0) nounwind
Index: test/CodeGen/AArch64/aarch64-sve-asm.ll
===
--- /dev/null
+++ test/CodeGen/AArch64/aarch64-sve-asm.ll
@@ -0,0 +1,46 @@
+; RUN: llc < %s -mtriple aarch64-none-linux-gnu -mattr=+sve -stop-after=finalize-isel | FileCheck %s --check-prefix=CHECK
+
+target datalayout = "e-m:e-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-none-linux-gnu"
+
+; Function Attrs: nounwind readnone
+; CHECK: [[ARG1:%[0-9]+]]:zpr = COPY $z1
+; CHECK: [[ARG2:%[0-9]+]]:zpr = COPY $z0
+; CHECK: [[ARG3:%[0-9]+]]:zpr = COPY [[ARG2]]
+; CHECK: [[ARG4:%[0-9]+]]:zpr_3b = COPY [[ARG1]]
+define  @test_svadd_i8( %Zn,  %Zm) {
+  %1 = tail call  asm "add $0.b, $1.b, $2.b", "=w,w,y"( %Zn,  %Zm)
+  ret  %1
+}
+
+; Function Attrs: nounwind readnone
+; CHECK: [[ARG1:%[0-9]+]]:zpr = COPY $z1
+; CHECK: [[ARG2:%[0-9]+]]:zpr = COPY $z0
+; CHECK: [[ARG3:%[0-9]+]]:zpr = COPY [[ARG2]]
+; CHECK: [[ARG4:%[0-9]+]]:zpr_4b = COPY [[ARG1]]
+define  @test_svsub_i64( %Zn,  %Zm) {
+  %1 = tail call  asm "sub $0.d, $1.d, $2.d", "=w,w,x"( %Zn,  %Zm)
+  ret  %1
+}
+
+; Function Attrs: nounwind readnone
+; CHECK: [[ARG1:%[0-9]+]]:zpr = COPY $z1
+; CHECK: [[ARG2:%[0-9]+]]:zpr = COPY $z0
+; CHECK: [[ARG3:%[0-9]+]]:zpr = COPY [[ARG2]]
+; CHECK: [[ARG4:%[0-9]+]]:zpr_3b = COPY [[ARG1]]
+define  @test_svfmul_f16( %Zn,  %Zm) {
+  %1 = tail call  asm "fmul $0.h, $1.h, $2.h", "=w,w,y"( %Zn,  %Zm)
+  ret  %1
+}
+
+; Function Attrs: nounwind readnone
+; CHECK: [[ARG1:%[0-9]+]]:zpr = COPY $z1
+; CHECK: [[ARG2:%[0-9]+]]:zpr = COPY $z0
+; CHECK: [[ARG3:%[0-9]+]]:zpr = COPY [[ARG2]]
+; CHECK: [[ARG4:%[0-9]+]]:zpr_4b = COPY [[ARG1]]
+define  @test_svfmul_f( %Zn,  %Zm) {
+  %1 = tail call  asm "fmul $0.s, $1.s, $2.s", "=w,w,x"( %Zn,  %Zm)
+  ret  %1
+}
+
+!0 = !{i32 188, i32 210}
Index: test/CodeGen/AArch64/aarch64-sve-asm-negative.ll
===
--- /dev/null
+++ test/CodeGen/AArch64/aarch64-sve-asm-negative.ll
@@ -0,0 +1,8 @@
+; RUN: not llc -mtriple aarch64-none-linux-gnu -mattr=+neon -o %t.s -filetype=asm %s 2>&1 | FileCheck %s
+
+; Function Attrs: nounwind readnone
+; CHECK: error: couldn't allocate input reg for constraint 'y'
+define <4 x i32> @test_neon(<4 x i32> %in1, <4 x i32> %in2) {
+  %1 = tail call <4 x i32> asm "add $0.4s, $1.4s, $2.4s", "=w,w,y"(<4 x i32> %in1, <4 x i32> %in2)
+  ret <4 x i32> %1
+}
Index: lib/Target/AArch64/AArch64SVEInstrInfo.td
===
--- lib/Target/AArch64/AArch64SVEInstrInfo.td
+++ lib/Target/AArch64/AArch64SVEInstrInfo.td
@@ -1020,6 +1020,56 @@
   (FCMGT_PPzZZ_S PPR32:$Zd, PPR3bAny:$Pg, ZPR32:$Zn, ZPR32:$Zm), 0>;
   def : InstAlias<"fcmlt $Zd, $Pg/z, $Zm, $Zn",
   (FCMGT_PPzZZ_D PPR64:$Zd, PPR3bAny:$Pg, ZPR64:$Zn, ZPR64:$Zm), 0>;
+
+  def : Pat<(nxv16i8 (bitconvert (nxv8i16 ZPR:$src))), (nxv16i8 ZPR:$src)>;
+  def : Pat<(nxv16i8 (bitconvert (nxv4i32 ZPR:$src))), (nxv16i8 ZPR:$src)>;
+  def : Pat<(nxv16i8 (bitconvert (nxv2i64 ZPR:$src))), (nxv16i8 ZPR:$src)>;
+  def : Pat<(nxv16i8 (bitconvert (nxv8f16 ZPR:$src))), (nxv16i8 ZPR:$src)>;
+  def : Pat<(nxv16i8 (bitconvert (nxv4f32 ZPR:$src))), (nxv16i8 ZPR:$src)>;
+  def : Pat<(nxv16i8 (bitconvert (nxv2f64 ZPR:$src))), (nxv16i8 ZPR:$src)>;
+
+  def : Pat<(nxv8i16 (bitconvert (nxv16i8 ZPR:$src))), (nxv8i16 ZPR:$src)>;
+  def : Pat<(nxv8i16 (bitconvert (nxv4i32 ZPR:$src))), (nxv8i16 ZPR:$src)>;
+  def : Pat<(nxv8i16 (bitconvert (nxv2i64 ZPR:$src))), (nxv8i16 ZPR:$src)>;
+  def : Pat<(nxv

r369402 - win: Enable /Zc:twoPhase by default if targeting MSVC 2017 update 3 or newer

[Nico Weber via cfe-commits]

Author: nico
Date: Tue Aug 20 09:28:11 2019
New Revision: 369402

URL: http://llvm.org/viewvc/llvm-project?rev=369402&view=rev
Log:
win: Enable /Zc:twoPhase by default if targeting MSVC 2017 update 3 or newer

MSVC 2017 update 3 (_MSC_VER 1911) enables /Zc:twoPhase by default, and
so should clang-cl:
https://docs.microsoft.com/en-us/cpp/build/reference/zc-twophase

clang-cl takes the MSVC version it emulates from the -fmsc-version flag,
or if that's not passed it tries to check what the installed version of
MSVC is and uses that, and failing that it uses a default version that's
currently 1911. So this changes the default if no -fmsc-version flag is
passed and no installed MSVC is detected. (It also changes the default
if -fmsc-version is passed or MSVC is detected, and either indicates
_MSC_VER >= 1911.)

As mentioned in the MSDN article, the Windows SDK header files in
version 10.0.15063.0 (Creators Update or Redstone 2) and earlier
versions do not work correctly with /Zc:twoPhase. If you need to use
these old SDKs with a new clang-cl, explicitly pass /Zc:twoPhase- to get
the old behavior.

Fixes PR43032.

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

Modified:
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/include/clang/Driver/CLCompatOptions.td
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/cl-options.c

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=369402&r1=369401&r2=369402&view=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Tue Aug 20 09:28:11 2019
@@ -90,6 +90,9 @@ Attribute Changes in Clang
 Windows Support
 ---
 
+- clang-cl now defaults to ``/Zc:twoPhase`` if targeting MSVC2017 update 3 or
+  later (``_MSC_VER`` >= 1911). This matches MSVC's behavior. Explicitly pass
+  ``/Zc:twoPhase-`` to restore the old behavior.
 - ...
 
 C Language Changes in Clang

Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=369402&r1=369401&r2=369402&view=diff
==
--- cfe/trunk/include/clang/Driver/CLCompatOptions.td (original)
+++ cfe/trunk/include/clang/Driver/CLCompatOptions.td Tue Aug 20 09:28:11 2019
@@ -235,10 +235,10 @@ def _SLASH_Zc_trigraphs : CLFlag<"Zc:tri
 def _SLASH_Zc_trigraphs_off : CLFlag<"Zc:trigraphs-">,
   HelpText<"Disable trigraphs (default)">, Alias;
 def _SLASH_Zc_twoPhase : CLFlag<"Zc:twoPhase">,
-  HelpText<"Enable two-phase name lookup in templates">,
+  HelpText<"Enable two-phase name lookup in templates (default)">,
   Alias;
 def _SLASH_Zc_twoPhase_ : CLFlag<"Zc:twoPhase-">,
-  HelpText<"Disable two-phase name lookup in templates (default)">,
+  HelpText<"Disable two-phase name lookup in templates">,
   Alias;
 def _SLASH_Z7 : CLFlag<"Z7">,
   HelpText<"Enable CodeView debug information in object files">;

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=369402&r1=369401&r2=369402&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Aug 20 09:28:11 2019
@@ -4883,12 +4883,14 @@ void Clang::ConstructJob(Compilation &C,
 !IsWindowsMSVC || IsMSVC2015Compatible))
 CmdArgs.push_back("-fno-threadsafe-statics");
 
-  // -fno-delayed-template-parsing is default, except when targeting MSVC.
-  // Many old Windows SDK versions require this to parse.
-  // FIXME: MSVC introduced /Zc:twoPhase- to disable this behavior in their
-  // compiler. We should be able to disable this by default at some point.
+  // -fno-delayed-template-parsing is default, except when targeting MSVC
+  // earlier than MSVC 2017 15.3 (_MSC_VER 1911).  Windows SDK versions
+  // 10.0.15063.0 (Creators Update or Redstone 2) and earlier require this to
+  // parse.
+  bool IsMSVCBefore2017Update3 = !MSVT.empty() && MSVT < VersionTuple(19, 11);
   if (Args.hasFlag(options::OPT_fdelayed_template_parsing,
-   options::OPT_fno_delayed_template_parsing, IsWindowsMSVC))
+   options::OPT_fno_delayed_template_parsing,
+   IsMSVCBefore2017Update3))
 CmdArgs.push_back("-fdelayed-template-parsing");
 
   // -fgnu-keywords default varies depending on language; only pass if

Modified: cfe/trunk/test/Driver/cl-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=369402&r1=369401&r2=369402&view=diff
==
--- cfe/trunk/test/Driver/cl-options.c (original)
+++ cfe/trunk/test/Driver/cl-options.c Tue Aug 20 09:28:11 2

[PATCH] D66394: clang-cl: Enable /Zc:twoPhase by default if targeting MSVC 2017 update 3 or newer

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369402: win: Enable /Zc:twoPhase by default if targeting 
MSVC 2017 update 3 or newer (authored by nico, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66394?vs=215785&id=216181#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66394

Files:
  cfe/trunk/docs/ReleaseNotes.rst
  cfe/trunk/include/clang/Driver/CLCompatOptions.td
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/test/Driver/cl-options.c


Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -4883,12 +4883,14 @@
 !IsWindowsMSVC || IsMSVC2015Compatible))
 CmdArgs.push_back("-fno-threadsafe-statics");
 
-  // -fno-delayed-template-parsing is default, except when targeting MSVC.
-  // Many old Windows SDK versions require this to parse.
-  // FIXME: MSVC introduced /Zc:twoPhase- to disable this behavior in their
-  // compiler. We should be able to disable this by default at some point.
+  // -fno-delayed-template-parsing is default, except when targeting MSVC
+  // earlier than MSVC 2017 15.3 (_MSC_VER 1911).  Windows SDK versions
+  // 10.0.15063.0 (Creators Update or Redstone 2) and earlier require this to
+  // parse.
+  bool IsMSVCBefore2017Update3 = !MSVT.empty() && MSVT < VersionTuple(19, 11);
   if (Args.hasFlag(options::OPT_fdelayed_template_parsing,
-   options::OPT_fno_delayed_template_parsing, IsWindowsMSVC))
+   options::OPT_fno_delayed_template_parsing,
+   IsMSVCBefore2017Update3))
 CmdArgs.push_back("-fdelayed-template-parsing");
 
   // -fgnu-keywords default varies depending on language; only pass if
Index: cfe/trunk/docs/ReleaseNotes.rst
===
--- cfe/trunk/docs/ReleaseNotes.rst
+++ cfe/trunk/docs/ReleaseNotes.rst
@@ -90,6 +90,9 @@
 Windows Support
 ---
 
+- clang-cl now defaults to ``/Zc:twoPhase`` if targeting MSVC2017 update 3 or
+  later (``_MSC_VER`` >= 1911). This matches MSVC's behavior. Explicitly pass
+  ``/Zc:twoPhase-`` to restore the old behavior.
 - ...
 
 C Language Changes in Clang
Index: cfe/trunk/include/clang/Driver/CLCompatOptions.td
===
--- cfe/trunk/include/clang/Driver/CLCompatOptions.td
+++ cfe/trunk/include/clang/Driver/CLCompatOptions.td
@@ -235,10 +235,10 @@
 def _SLASH_Zc_trigraphs_off : CLFlag<"Zc:trigraphs-">,
   HelpText<"Disable trigraphs (default)">, Alias;
 def _SLASH_Zc_twoPhase : CLFlag<"Zc:twoPhase">,
-  HelpText<"Enable two-phase name lookup in templates">,
+  HelpText<"Enable two-phase name lookup in templates (default)">,
   Alias;
 def _SLASH_Zc_twoPhase_ : CLFlag<"Zc:twoPhase-">,
-  HelpText<"Disable two-phase name lookup in templates (default)">,
+  HelpText<"Disable two-phase name lookup in templates">,
   Alias;
 def _SLASH_Z7 : CLFlag<"Z7">,
   HelpText<"Enable CodeView debug information in object files">;
Index: cfe/trunk/test/Driver/cl-options.c
===
--- cfe/trunk/test/Driver/cl-options.c
+++ cfe/trunk/test/Driver/cl-options.c
@@ -316,13 +316,19 @@
 
 // We recognize -f[no-]delayed-template-parsing.
 // /Zc:twoPhase[-] has the opposite meaning.
-// RUN: %clang_cl -c -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDDEFAULT 
%s
-// DELAYEDDEFAULT: "-fdelayed-template-parsing"
-// RUN: %clang_cl -c -fdelayed-template-parsing -### -- %s 2>&1 | FileCheck 
-check-prefix=DELAYEDON %s
-// RUN: %clang_cl -c /Zc:twoPhase- -### -- %s 2>&1 | FileCheck 
-check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1910 -### -- %s 2>&1 | FileCheck 
-check-prefix=OLDDELAYEDDEFAULT %s
+// OLDDELAYEDDEFAULT: "-fdelayed-template-parsing"
+// RUN: %clang_cl -fmsc-version=1911 -c -### -- %s 2>&1 | FileCheck 
-check-prefix=NEWDELAYEDDEFAULT %s
+// NEWDELAYEDDEFAULT-NOT: "-fdelayed-template-parsing"
+// RUN: %clang_cl -c -fmsc-version=1910 -fdelayed-template-parsing -### -- %s 
2>&1 | FileCheck -check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1911 -fdelayed-template-parsing -### -- %s 
2>&1 | FileCheck -check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1910 /Zc:twoPhase- -### -- %s 2>&1 | 
FileCheck -check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1911 /Zc:twoPhase- -### -- %s 2>&1 | 
FileCheck -check-prefix=DELAYEDON %s
 // DELAYEDON: "-fdelayed-template-parsing"
-// RUN: %clang_cl -c -fno-delayed-template-parsing -### -- %s 2>&1 | FileCheck 
-check-prefix=DELAYEDOFF %s
-// RUN: %clang_cl -c /Zc:twoPhase -### -- %s 2>&1 | FileCheck 
-check-prefix=DELAYEDOFF %s
+// RUN: %clang_c

[PATCH] D66364: Diagnose use of _Thread_local as an extension when appropriate

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D66364#1637570 , @aaron.ballman 
wrote:

> In D66364#1635863 , @ldionne wrote:
>
> > In D66364#1635814 , @aaron.ballman 
> > wrote:
> >
> > > [ ...]
> > >
> > > Adding some libc++ maintainers to see if they have opinions.
> > >
> > > `__extension__` is one option. Could we get away with push/pop disabling 
> > > of the diagnostic? Or perhaps this is a situation where we should not 
> > > diagnose use within a system header in the first place, because that's 
> > > part of the implementation?
> >
> >
> > I just learned about `__extension__`, but from my perspective it makes 
> > sense to mark uses of `_Atomic` with `__extension__` (or disable the 
> > warning with a `#pragma`) inside libc++ if we're using something 
> > non-standard for the current dialect. I don't think Clang should bend its 
> > back for libc++ in this case.
>
>
> Okay, that's good feedback, thank you!


Please ping me directly if you expect libc++ maintainers to do something 
following this patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66364



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

r369408 - [LifetimeAnalysis] Add support for free functions

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Tue Aug 20 09:45:06 2019
New Revision: 369408

URL: http://llvm.org/viewvc/llvm-project?rev=369408&view=rev
Log:
[LifetimeAnalysis] Add support for free functions

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

Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=369408&r1=369407&r2=369408&view=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Tue Aug 20 09:45:06 2019
@@ -6616,6 +6616,30 @@ static bool shouldTrackImplicitObjectArg
   return false;
 }
 
+static bool shouldTrackFirstArgument(const FunctionDecl *FD) {
+  if (!FD->getIdentifier() || FD->getNumParams() != 1)
+return false;
+  const auto *RD = FD->getParamDecl(0)->getType()->getPointeeCXXRecordDecl();
+  if (!FD->isInStdNamespace() || !RD || !RD->isInStdNamespace())
+return false;
+  if (!isRecordWithAttr(QualType(RD->getTypeForDecl(), 0)) &&
+  !isRecordWithAttr(QualType(RD->getTypeForDecl(), 0)))
+return false;
+  if (FD->getReturnType()->isPointerType() ||
+  isRecordWithAttr(FD->getReturnType())) {
+return llvm::StringSwitch(FD->getName())
+.Cases("begin", "rbegin", "cbegin", "crbegin", true)
+.Cases("end", "rend", "cend", "crend", true)
+.Case("data", true)
+.Default(false);
+  } else if (FD->getReturnType()->isReferenceType()) {
+return llvm::StringSwitch(FD->getName())
+.Cases("get", "any_cast", true)
+.Default(false);
+  }
+  return false;
+}
+
 static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
 LocalVisitor Visit) {
   auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
@@ -6639,6 +6663,11 @@ static void handleGslAnnotatedTypes(Indi
 shouldTrackImplicitObjectArg(cast(Callee)))
   VisitPointerArg(Callee, OCE->getArg(0));
 return;
+  } else if (auto *CE = dyn_cast(Call)) {
+FunctionDecl *Callee = CE->getDirectCallee();
+if (Callee && shouldTrackFirstArgument(Callee))
+  VisitPointerArg(Callee, CE->getArg(0));
+return;
   }
 
   if (auto *CCE = dyn_cast(Call)) {

Modified: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp?rev=369408&r1=369407&r2=369408&view=diff
==
--- cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp (original)
+++ cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp Tue Aug 20 09:45:06 
2019
@@ -131,13 +131,16 @@ bool operator!=(basic_iterator, basic
 }
 
 namespace std {
-template struct remove_reference   { typedef T type; };
-template struct remove_reference  { typedef T type; };
-template struct remove_reference { typedef T type; };
+template struct remove_reference   { typedef T type; };
+template struct remove_reference  { typedef T type; };
+template struct remove_reference { typedef T type; };
 
-template
+template
 typename remove_reference::type &&move(T &&t) noexcept;
 
+template 
+auto data(const C &c) -> decltype(c.data());
+
 template 
 struct vector {
   typedef __gnu_cxx::basic_iterator iterator;
@@ -180,6 +183,11 @@ template
 struct stack {
   T &top();
 };
+
+struct any {};
+
+template
+T any_cast(const any& operand);
 }
 
 void modelIterators() {
@@ -191,6 +199,22 @@ std::vector::iterator modelIterator
   return std::vector().begin(); // expected-warning {{returning address 
of local temporary object}}
 }
 
+const int *modelFreeFunctions() {
+  return std::data(std::vector()); // expected-warning {{returning 
address of local temporary object}}
+}
+
+int &modelAnyCast() {
+  return std::any_cast(std::any{}); // expected-warning {{returning 
reference to local temporary object}}
+}
+
+int modelAnyCast2() {
+  return std::any_cast(std::any{}); // ok
+}
+
+int modelAnyCast3() {
+  return std::any_cast(std::any{}); // ok
+}
+
 const char *danglingRawPtrFromLocal() {
   std::basic_string s;
   return s.c_str(); // expected-warning {{address of stack memory associated 
with local variable 's' returned}}


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

[PATCH] D66303: [LifetimeAnalysis] Add support for free functions

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369408: [LifetimeAnalysis] Add support for free functions 
(authored by xazax, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66303?vs=215413&id=216188#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66303

Files:
  cfe/trunk/lib/Sema/SemaInit.cpp
  cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp

Index: cfe/trunk/lib/Sema/SemaInit.cpp
===
--- cfe/trunk/lib/Sema/SemaInit.cpp
+++ cfe/trunk/lib/Sema/SemaInit.cpp
@@ -6616,6 +6616,30 @@
   return false;
 }
 
+static bool shouldTrackFirstArgument(const FunctionDecl *FD) {
+  if (!FD->getIdentifier() || FD->getNumParams() != 1)
+return false;
+  const auto *RD = FD->getParamDecl(0)->getType()->getPointeeCXXRecordDecl();
+  if (!FD->isInStdNamespace() || !RD || !RD->isInStdNamespace())
+return false;
+  if (!isRecordWithAttr(QualType(RD->getTypeForDecl(), 0)) &&
+  !isRecordWithAttr(QualType(RD->getTypeForDecl(), 0)))
+return false;
+  if (FD->getReturnType()->isPointerType() ||
+  isRecordWithAttr(FD->getReturnType())) {
+return llvm::StringSwitch(FD->getName())
+.Cases("begin", "rbegin", "cbegin", "crbegin", true)
+.Cases("end", "rend", "cend", "crend", true)
+.Case("data", true)
+.Default(false);
+  } else if (FD->getReturnType()->isReferenceType()) {
+return llvm::StringSwitch(FD->getName())
+.Cases("get", "any_cast", true)
+.Default(false);
+  }
+  return false;
+}
+
 static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
 LocalVisitor Visit) {
   auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
@@ -6639,6 +6663,11 @@
 shouldTrackImplicitObjectArg(cast(Callee)))
   VisitPointerArg(Callee, OCE->getArg(0));
 return;
+  } else if (auto *CE = dyn_cast(Call)) {
+FunctionDecl *Callee = CE->getDirectCallee();
+if (Callee && shouldTrackFirstArgument(Callee))
+  VisitPointerArg(Callee, CE->getArg(0));
+return;
   }
 
   if (auto *CCE = dyn_cast(Call)) {
Index: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -131,13 +131,16 @@
 }
 
 namespace std {
-template struct remove_reference   { typedef T type; };
-template struct remove_reference  { typedef T type; };
-template struct remove_reference { typedef T type; };
+template struct remove_reference   { typedef T type; };
+template struct remove_reference  { typedef T type; };
+template struct remove_reference { typedef T type; };
 
-template
+template
 typename remove_reference::type &&move(T &&t) noexcept;
 
+template 
+auto data(const C &c) -> decltype(c.data());
+
 template 
 struct vector {
   typedef __gnu_cxx::basic_iterator iterator;
@@ -180,6 +183,11 @@
 struct stack {
   T &top();
 };
+
+struct any {};
+
+template
+T any_cast(const any& operand);
 }
 
 void modelIterators() {
@@ -191,6 +199,22 @@
   return std::vector().begin(); // expected-warning {{returning address of local temporary object}}
 }
 
+const int *modelFreeFunctions() {
+  return std::data(std::vector()); // expected-warning {{returning address of local temporary object}}
+}
+
+int &modelAnyCast() {
+  return std::any_cast(std::any{}); // expected-warning {{returning reference to local temporary object}}
+}
+
+int modelAnyCast2() {
+  return std::any_cast(std::any{}); // ok
+}
+
+int modelAnyCast3() {
+  return std::any_cast(std::any{}); // ok
+}
+
 const char *danglingRawPtrFromLocal() {
   std::basic_string s;
   return s.c_str(); // expected-warning {{address of stack memory associated with local variable 's' returned}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r369414 - [Attr] Support _attribute__ ((fallthrough))

[Nathan Huckleberry via cfe-commits]

Author: nathan-huckleberry
Date: Tue Aug 20 10:16:49 2019
New Revision: 369414

URL: http://llvm.org/viewvc/llvm-project?rev=369414&view=rev
Log:
[Attr] Support _attribute__ ((fallthrough))

Summary: Fixed extraneous matches of non-NullStmt

Reviewers: aaron.ballman, rsmith, efriedma, xbolva00

Reviewed By: aaron.ballman, rsmith, xbolva00

Subscribers: riccibruno, arphaman, ziangwan, ojeda, xbolva00, nickdesaulniers, 
cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/Sema/fallthrough-attr.c
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/ParseStmt.cpp
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/test/SemaCXX/switch-implicit-fallthrough.cpp
cfe/trunk/test/SemaCXX/warn-unused-label-error.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=369414&r1=369413&r2=369414&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Aug 20 10:16:49 2019
@@ -1170,7 +1170,7 @@ def ExtVectorType : Attr {
 
 def FallThrough : StmtAttr {
   let Spellings = [CXX11<"", "fallthrough", 201603>, C2x<"", "fallthrough">,
-   CXX11<"clang", "fallthrough">];
+   CXX11<"clang", "fallthrough">, GCC<"fallthrough">];
 //  let Subjects = [NullStmt];
   let Documentation = [FallthroughDocs];
 }

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=369414&r1=369413&r2=369414&view=diff
==
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Tue Aug 20 10:16:49 2019
@@ -2107,12 +2107,13 @@ private:
 
   DeclGroupPtrTy ParseDeclaration(DeclaratorContext Context,
   SourceLocation &DeclEnd,
-  ParsedAttributesWithRange &attrs);
-  DeclGroupPtrTy ParseSimpleDeclaration(DeclaratorContext Context,
-SourceLocation &DeclEnd,
-ParsedAttributesWithRange &attrs,
-bool RequireSemi,
-ForRangeInit *FRI = nullptr);
+  ParsedAttributesWithRange &attrs,
+  SourceLocation *DeclSpecStart = nullptr);
+  DeclGroupPtrTy
+  ParseSimpleDeclaration(DeclaratorContext Context, SourceLocation &DeclEnd,
+ ParsedAttributesWithRange &attrs, bool RequireSemi,
+ ForRangeInit *FRI = nullptr,
+ SourceLocation *DeclSpecStart = nullptr);
   bool MightBeDeclarator(DeclaratorContext Context);
   DeclGroupPtrTy ParseDeclGroup(ParsingDeclSpec &DS, DeclaratorContext Context,
 SourceLocation *DeclEnd = nullptr,

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=369414&r1=369413&r2=369414&view=diff
==
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Tue Aug 20 10:16:49 2019
@@ -1741,9 +1741,10 @@ void Parser::stripTypeAttributesOffDeclS
 /// [C++11/C11] static_assert-declaration
 /// others... [FIXME]
 ///
-Parser::DeclGroupPtrTy Parser::ParseDeclaration(DeclaratorContext Context,
-SourceLocation &DeclEnd,
-  ParsedAttributesWithRange &attrs) {
+Parser::DeclGroupPtrTy
+Parser::ParseDeclaration(DeclaratorContext Context, SourceLocation &DeclEnd,
+ ParsedAttributesWithRange &attrs,
+ SourceLocation *DeclSpecStart) {
   ParenBraceBracketBalancer BalancerRAIIObj(*this);
   // Must temporarily exit the objective-c container scope for
   // parsing c none objective-c decls.
@@ -1763,8 +1764,8 @@ Parser::DeclGroupPtrTy Parser::ParseDecl
   SourceLocation InlineLoc = ConsumeToken();
   return ParseNamespace(Context, DeclEnd, InlineLoc);
 }
-return ParseSimpleDeclaration(Context, DeclEnd, attrs,
-  true);
+return ParseSimpleDeclaration(Context, DeclEnd, attrs, true, nullptr,
+  DeclSpecStart);
   case tok::kw_namespace:
 ProhibitAttributes(attrs);
 return ParseNamespace(Context, DeclEnd);
@@ -1777,7 +1778,8 @@ Parser::DeclGroupPtrTy Parser::ParseDecl
 SingleDecl = ParseStaticAssertDeclaration(DeclEnd);
 break;
   default:
-return ParseSimpleDeclaration(Co

[PATCH] D66485: [Clang][Bundler] Use llvm-objcopy for creating fat object files

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev created this revision.
sdmitriev added a reviewer: ABataev.
Herald added subscribers: cfe-commits, abrachet, mgorny.
Herald added a reviewer: alexshap.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

clang-offload-bundler currently uses partial linking for creating fat object 
files, but such technique cannot be used on Windows due to the absence of 
partial linking support in the linker. This patch changes implementation to use 
llvm-objcopy for merging device and host objects instead of doing partial 
linking. This is one step forward towards enabling OpenMP offload on Windows.


Repository:
  rC Clang

https://reviews.llvm.org/D66485

Files:
  clang/test/CMakeLists.txt
  clang/test/Driver/clang-offload-bundler.c
  clang/tools/clang-offload-bundler/CMakeLists.txt
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -21,12 +21,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
-#include "llvm/Bitcode/BitcodeWriter.h"
-#include "llvm/IR/Constant.h"
-#include "llvm/IR/Constants.h"
-#include "llvm/IR/GlobalVariable.h"
-#include "llvm/IR/LLVMContext.h"
-#include "llvm/IR/Module.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/Casting.h"
@@ -39,6 +33,7 @@
 #include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/StringSaver.h"
 #include 
 #include 
 #include 
@@ -94,11 +89,6 @@
  "instead of actually executing them - for testing purposes.\n"),
 cl::init(false), cl::cat(ClangOffloadBundlerCategory));
 
-static cl::opt DumpTemporaryFiles(
-"dump-temporary-files",
-cl::desc("Dumps any temporary files created - for testing purposes.\n"),
-cl::init(false), cl::cat(ClangOffloadBundlerCategory));
-
 /// Magic string that marks the existence of offloading data.
 #define OFFLOAD_BUNDLER_MAGIC_STR "__CLANG_OFFLOAD_BUNDLE__"
 
@@ -116,12 +106,6 @@
   OffloadKind = KindTriplePair.first;
   Triple = KindTriplePair.second;
 }
-static StringRef getTriple(StringRef Target) {
-  StringRef OffloadKind;
-  StringRef Triple;
-  getOffloadKindAndTriple(Target, OffloadKind, Triple);
-  return Triple;
-}
 static bool hasHostKind(StringRef Target) {
   StringRef OffloadKind;
   StringRef Triple;
@@ -410,19 +394,6 @@
   /// read from the buffers.
   unsigned NumberOfProcessedInputs = 0;
 
-  /// LLVM context used to create the auxiliary modules.
-  LLVMContext VMContext;
-
-  /// LLVM module used to create an object with all the bundle
-  /// components.
-  std::unique_ptr AuxModule;
-
-  /// The current triple we are working with.
-  StringRef CurrentTriple;
-
-  /// The name of the main input file.
-  StringRef MainInputFileName;
-
   /// Iterator of the current and next section.
   section_iterator CurrentSection;
   section_iterator NextSection;
@@ -476,19 +447,10 @@
 
 // Record number of inputs.
 NumberOfInputs = Inputs.size();
-
-// Create an LLVM module to have the content we need to bundle.
-auto *M = new Module("clang-offload-bundle", VMContext);
-M->setTargetTriple(getTriple(TargetNames[HostInputIndex]));
-AuxModule.reset(M);
   }
 
   void WriteBundleStart(raw_fd_ostream &OS, StringRef TargetTriple) final {
 ++NumberOfProcessedInputs;
-
-// Record the triple we are using, that will be used to name the section we
-// will create.
-CurrentTriple = TargetTriple;
   }
 
   bool WriteBundleEnd(raw_fd_ostream &OS, StringRef TargetTriple) final {
@@ -500,76 +462,39 @@
 if (NumberOfProcessedInputs != NumberOfInputs)
   return false;
 
-// Create the bitcode file name to write the resulting code to. Keep it if
-// save-temps is active.
-SmallString<128> BitcodeFileName;
-if (sys::fs::createTemporaryFile("clang-offload-bundler", "bc",
- BitcodeFileName)) {
-  errs() << "error: unable to create temporary file.\n";
+// Find llvm-objcopy in order to create the bundle binary.
+ErrorOr Objcopy = sys::findProgramByName(
+"llvm-objcopy", sys::path::parent_path(BundlerExecutable));
+if (!Objcopy) {
+  errs() << "error: unable to find 'llvm-objcopy' in path.\n";
   return true;
 }
 
-// Dump the contents of the temporary file if that was requested.
-if (DumpTemporaryFiles) {
-  errs() << ";\n; Object file bundler IR file.\n;\n";
-  AuxModule.get()->print(errs(), nullptr,
- /*ShouldPreserveUseListOrder=*/false,
- /*IsForDebug=*/true);
-  errs() << '\n';
-}
-
-// Find clang in order to create the bundle binary.
-StringRef Dir = sys

[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

[Nathan Huckleberry via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369414: [Attr] Support _attribute__ ((fallthrough)) 
(authored by Nathan-Huckleberry, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64838?vs=213435&id=216191#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64838

Files:
  cfe/trunk/include/clang/Basic/Attr.td
  cfe/trunk/include/clang/Parse/Parser.h
  cfe/trunk/lib/Parse/ParseDecl.cpp
  cfe/trunk/lib/Parse/ParseStmt.cpp
  cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
  cfe/trunk/test/Sema/fallthrough-attr.c
  cfe/trunk/test/SemaCXX/switch-implicit-fallthrough.cpp
  cfe/trunk/test/SemaCXX/warn-unused-label-error.cpp

Index: cfe/trunk/test/SemaCXX/warn-unused-label-error.cpp
===
--- cfe/trunk/test/SemaCXX/warn-unused-label-error.cpp
+++ cfe/trunk/test/SemaCXX/warn-unused-label-error.cpp
@@ -18,9 +18,9 @@
   }
 
   void h() {
-D: // expected-warning {{unused label 'D'}}
-  #pragma weak unused_local_static
-  __attribute__((unused))  // expected-warning {{declaration does not declare anything}}
-  ;
+  D:
+#pragma weak unused_local_static
+__attribute__((unused)) // expected-error {{'unused' attribute cannot be applied to a statement}}
+;
   }
 }
Index: cfe/trunk/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- cfe/trunk/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ cfe/trunk/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -329,3 +329,15 @@
   }
   return n;
 }
+
+int fallthrough_attribute_spelling(int n) {
+  switch (n) {
+  case 0:
+n++;
+__attribute__((fallthrough));
+  case 1:
+n++;
+break;
+  }
+  return n;
+}
Index: cfe/trunk/test/Sema/fallthrough-attr.c
===
--- cfe/trunk/test/Sema/fallthrough-attr.c
+++ cfe/trunk/test/Sema/fallthrough-attr.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only -std=gnu89 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=gnu99 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=c11 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=c2x -DC2X -verify -Wimplicit-fallthrough %s
+
+int fallthrough_attribute_spelling(int n) {
+  switch (n) {
+  case 0:
+n++;
+  case 1:
+#if defined(C2X)
+// expected-warning@-2{{unannotated fall-through between switch labels}} expected-note@-2{{insert '[[fallthrough]];' to silence this warning}} expected-note@-2{{insert 'break;' to avoid fall-through}}
+#else
+// expected-warning@-4{{unannotated fall-through between switch labels}} expected-note@-4{{insert '__attribute__((fallthrough));' to silence this warning}} expected-note@-4{{insert 'break;' to avoid fall-through}}
+#endif
+n++;
+__attribute__((fallthrough));
+  case 2:
+n++;
+break;
+  }
+  return n;
+}
Index: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
===
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1215,7 +1215,7 @@
 tok::r_square, tok::r_square
   };
 
-  bool PreferClangAttr = !PP.getLangOpts().CPlusPlus17;
+  bool PreferClangAttr = !PP.getLangOpts().CPlusPlus17 && !PP.getLangOpts().C2x;
 
   StringRef MacroName;
   if (PreferClangAttr)
@@ -1224,24 +1224,19 @@
 MacroName = PP.getLastMacroWithSpelling(Loc, FallthroughTokens);
   if (MacroName.empty() && !PreferClangAttr)
 MacroName = PP.getLastMacroWithSpelling(Loc, ClangFallthroughTokens);
-  if (MacroName.empty())
-MacroName = PreferClangAttr ? "[[clang::fallthrough]]" : "[[fallthrough]]";
+  if (MacroName.empty()) {
+if (!PreferClangAttr)
+  MacroName = "[[fallthrough]]";
+else if (PP.getLangOpts().CPlusPlus)
+  MacroName = "[[clang::fallthrough]]";
+else
+  MacroName = "__attribute__((fallthrough))";
+  }
   return MacroName;
 }
 
 static void DiagnoseSwitchLabelsFallthrough(Sema &S, AnalysisDeclContext &AC,
 bool PerFunction) {
-  // Only perform this analysis when using [[]] attributes. There is no good
-  // workflow for this warning when not using C++11. There is no good way to
-  // silence the warning (no attribute is available) unless we are using
-  // [[]] attributes. One could use pragmas to silence the warning, but as a
-  // general solution that is gross and not in the spirit of this warning.
-  //
-  // NOTE: This an intermediate solution. There are on-going discussions on
-  // how to properly support this warning outside of C++11 with an annotation.
-  if (!AC.getASTContext().getLangOpts().DoubleSquareBracketAttribu

[PATCH] D66486: [LifetimeAnalysis] Detect more cases when the address of a local variable escapes

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

xazax.hun created this revision.
xazax.hun added reviewers: gribozavr, mgehre.
xazax.hun added a project: clang.
Herald added subscribers: Szelethus, Charusso, gamesh411, dkrupp, rnkovacs.

This patch relaxes some of the checks so we can detect more cases where local 
variable escapes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66486

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/Sema/warn-lifetime-analysis-nocfg.cpp

Index: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -141,6 +141,9 @@
 template 
 auto data(const C &c) -> decltype(c.data());
 
+template
+T *begin(T (&array)[N]);
+
 template 
 struct vector {
   typedef __gnu_cxx::basic_iterator iterator;
@@ -188,6 +191,14 @@
 
 template
 T any_cast(const any& operand);
+
+template
+struct reference_wrapper {
+  reference_wrapper(T &&);
+};
+
+template
+reference_wrapper ref(T& t) noexcept;
 }
 
 void modelIterators() {
@@ -298,3 +309,45 @@
   const std::vector &v = getVec();
   const int *val = v.data(); // Ok, it is lifetime extended.
 }
+
+std::reference_wrapper danglingPtrFromNonOwnerLocal() {
+  int i = 5;
+  return i; // expected-warning {{address of stack memory associated with local variable 'i' returned}}
+}
+
+std::reference_wrapper danglingPtrFromNonOwnerLocal2() {
+  int i = 5;
+  return std::ref(i); // expected-warning {{address of stack memory associated with local variable 'i' returned}}
+}
+
+int *returnPtrToLocalArray() {
+  int a[5];
+  return std::begin(a); // expected-warning {{address of stack memory associated with local variable 'a' returned}}
+}
+
+struct ptr_wrapper {
+  std::vector::iterator member;
+};
+
+ptr_wrapper getPtrWrapper();
+
+std::vector::iterator returnPtrFromWrapper() {
+  ptr_wrapper local = getPtrWrapper();
+  return local.member; // OK.
+}
+
+std::vector::iterator returnPtrFromWrapperThroughRef() {
+  ptr_wrapper local = getPtrWrapper();
+  ptr_wrapper &local2 = local;
+  return local2.member; // OK.
+}
+
+std::vector::iterator returnPtrFromWrapperThroughRef2() {
+  ptr_wrapper local = getPtrWrapper();
+  std::vector::iterator &local2 = local.member;
+  return local2; // OK.
+}
+
+void checkPtrMemberFromAggregate() {
+  std::vector::iterator local = getPtrWrapper().member; // OK.
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6551,6 +6551,15 @@
   });
 }
 
+static bool pathOnlyInitializesGslPointer(IndirectLocalPath &Path) {
+  for (auto It = Path.rbegin(), End = Path.rend(); It != End; ++It) {
+if (It->Kind == IndirectLocalPathEntry::VarInit)
+  continue;
+return It->Kind == IndirectLocalPathEntry::GslPointerInit;
+  }
+  return false;
+}
+
 static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
  Expr *Init, LocalVisitor Visit,
  bool RevisitSubinits);
@@ -6619,17 +6628,14 @@
 static bool shouldTrackFirstArgument(const FunctionDecl *FD) {
   if (!FD->getIdentifier() || FD->getNumParams() != 1)
 return false;
-  const auto *RD = FD->getParamDecl(0)->getType()->getPointeeCXXRecordDecl();
-  if (!FD->isInStdNamespace() || !RD || !RD->isInStdNamespace())
-return false;
-  if (!isRecordWithAttr(QualType(RD->getTypeForDecl(), 0)) &&
-  !isRecordWithAttr(QualType(RD->getTypeForDecl(), 0)))
+  if (!FD->isInStdNamespace())
 return false;
   if (FD->getReturnType()->isPointerType() ||
   isRecordWithAttr(FD->getReturnType())) {
 return llvm::StringSwitch(FD->getName())
 .Cases("begin", "rbegin", "cbegin", "crbegin", true)
 .Cases("end", "rend", "cend", "crend", true)
+.Cases("cref", "ref", true)
 .Case("data", true)
 .Default(false);
   } else if (FD->getReturnType()->isReferenceType()) {
@@ -6763,7 +6769,10 @@
 
 // Step over any subobject adjustments; we may have a materialized
 // temporary inside them.
-Init = const_cast(Init->skipRValueSubobjectAdjustments());
+// We are not interested in the temporary base objects of gsl::Pointers:
+//   Temp().ptr; // Here ptr might not dangle.
+if (!pathOnlyInitializesGslPointer(Path))
+  Init = const_cast(Init->skipRValueSubobjectAdjustments());
 
 // Per current approach for DR1376, look through casts to reference type
 // when performing lifetime extension.
@@ -6882,7 +6891,10 @@
   Init = FE->getSubExpr();
 
 // Dig out the expression which constructs the extended temporary.
-Init = const_cast(Init->skipRValueSubobjectAdjustments());
+// We are not interested in the temporary base objects of gsl::Pointers:
+//   Temp().ptr; // Here ptr might not dangle.
+if (!pathOnlyInitializesGslPointer(Path))
+  Init = cons

[PATCH] D66303: [LifetimeAnalysis] Add support for free functions

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

xazax.hun marked 2 inline comments as done.
xazax.hun added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:6622
+return false;
+  const auto *RD = FD->getParamDecl(0)->getType()->getPointeeCXXRecordDecl();
+  if (!FD->isInStdNamespace() || !RD || !RD->isInStdNamespace())

mgehre wrote:
> Maybe move the `Callee->getNumParams() == 1` check from the caller into this 
> function so it's obvious that `getParamDecl(0)` is allowed?
Thanks, I addressed this one before committing.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66303



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

LLVM buildmaster will be updated and restarted tonight

[Galina Kistanova via cfe-commits]

 Hello everyone,

LLVM buildmaster will be updated and restarted after 5PM Pacific time today.

Thanks

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

r369418 - Fix name of the error message, NFC.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Aug 20 10:50:13 2019
New Revision: 369418

URL: http://llvm.org/viewvc/llvm-project?rev=369418&view=rev
Log:
Fix name of the error message, NFC.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaOpenMP.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=369418&r1=369417&r2=369418&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Aug 20 10:50:13 
2019
@@ -9329,7 +9329,7 @@ def err_omp_wrong_dependency_iterator_ty
   "expected an integer or a pointer type of the outer loop counter '%0' for 
non-rectangular nests">;
 def err_omp_unsupported_type : Error <
   "host requires %0 bit size %1 type support, but device '%2' does not support 
it">;
-def omp_lambda_capture_in_declare_target_not_to : Error<
+def err_omp_lambda_capture_in_declare_target_not_to : Error<
   "variable captured in declare target region must appear in a to clause">;
 } // end of OpenMP category
 

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=369418&r1=369417&r2=369418&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Tue Aug 20 10:50:13 2019
@@ -15365,7 +15365,7 @@ static void checkDeclInTargetContext(Sou
   // directive, all variables that are captured by the lambda
   // expression must also appear in a to clause.
   SemaRef.Diag(VD->getLocation(),
-   diag::omp_lambda_capture_in_declare_target_not_to);
+   diag::err_omp_lambda_capture_in_declare_target_not_to);
   SemaRef.Diag(SL, diag::note_var_explicitly_captured_here)
   << VD << 0 << SR;
   return;


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

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

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

xazax.hun added inline comments.



Comment at: clang/lib/Sema/SemaAttr.cpp:94
 
-  Canonical->addAttr(::new (Context) Attribute(SourceRange{}, Context,
-   /*DerefType*/ nullptr,
-   /*Spelling=*/0));
+  Record->addAttr(::new (Context) Attribute(SourceRange{}, Context,
+/*DerefType*/ nullptr,

Doesn't the attribute have a `CreateImplicit` static method? If so, we could 
use that :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66179



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

[PATCH] D66488: [LTO] Always mark regular LTO units with EnableSplitLTOUnit=1 under the new pass manager

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: pcc, tejohnson.
leonardchan added a project: clang.
Herald added subscribers: dexonsmith, steven_wu, inglorion, mehdi_amini.

Match the behavior of D65009  under the new 
pass manager also.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66488

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/split-lto-unit.c


Index: clang/test/CodeGen/split-lto-unit.c
===
--- clang/test/CodeGen/split-lto-unit.c
+++ clang/test/CodeGen/split-lto-unit.c
@@ -7,6 +7,7 @@
 // SPLIT: !{i32 1, !"EnableSplitLTOUnit", i32 1}
 //
 // ; Check that regular LTO has EnableSplitLTOUnit = 1
-// RUN: %clang_cc1 -flto -triple x86_64-pc-linux-gnu -emit-llvm-bc < %s | 
llvm-dis -o - | FileCheck %s --implicit-check-not="EnableSplitLTOUnit" 
--check-prefix=SPLIT
+// RUN: %clang_cc1 -fno-experimental-new-pass-manager -flto -triple 
x86_64-pc-linux-gnu -emit-llvm-bc < %s | llvm-dis -o - | FileCheck %s 
--implicit-check-not="EnableSplitLTOUnit" --check-prefix=SPLIT
+// RUN: %clang_cc1 -fexperimental-new-pass-manager -flto -triple 
x86_64-pc-linux-gnu -emit-llvm-bc < %s | llvm-dis -o - | FileCheck %s 
--implicit-check-not="EnableSplitLTOUnit" --check-prefix=SPLIT
 
 int main() {}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1291,7 +1291,7 @@
 if (!TheModule->getModuleFlag("ThinLTO"))
   TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
 TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
- CodeGenOpts.EnableSplitLTOUnit);
+ uint32_t(1));
   }
   MPM.addPass(
   BitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists, 
EmitLTOSummary));


Index: clang/test/CodeGen/split-lto-unit.c
===
--- clang/test/CodeGen/split-lto-unit.c
+++ clang/test/CodeGen/split-lto-unit.c
@@ -7,6 +7,7 @@
 // SPLIT: !{i32 1, !"EnableSplitLTOUnit", i32 1}
 //
 // ; Check that regular LTO has EnableSplitLTOUnit = 1
-// RUN: %clang_cc1 -flto -triple x86_64-pc-linux-gnu -emit-llvm-bc < %s | llvm-dis -o - | FileCheck %s --implicit-check-not="EnableSplitLTOUnit" --check-prefix=SPLIT
+// RUN: %clang_cc1 -fno-experimental-new-pass-manager -flto -triple x86_64-pc-linux-gnu -emit-llvm-bc < %s | llvm-dis -o - | FileCheck %s --implicit-check-not="EnableSplitLTOUnit" --check-prefix=SPLIT
+// RUN: %clang_cc1 -fexperimental-new-pass-manager -flto -triple x86_64-pc-linux-gnu -emit-llvm-bc < %s | llvm-dis -o - | FileCheck %s --implicit-check-not="EnableSplitLTOUnit" --check-prefix=SPLIT
 
 int main() {}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1291,7 +1291,7 @@
 if (!TheModule->getModuleFlag("ThinLTO"))
   TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
 TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
- CodeGenOpts.EnableSplitLTOUnit);
+ uint32_t(1));
   }
   MPM.addPass(
   BitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists, EmitLTOSummary));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66473: Removed some dead code in BugReporter and related files

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:408
-public:
-  enum Kind { BasicBRKind, PathSensitiveBRKind };
-

NoQ wrote:
> Hey, i just added that! :D (well, renamed) (rC369320)
> 
> I believe we do want a separation between a {bug report, bug reporter} 
> classes that's only suitable for path-insensitive reports (which would live 
> in libAnalysis and we could handle them to clang-tidy while still being able 
> to compile it without CLANG_ENABLE_STATIC_ANALYZER) and all the 
> path-sensitive report logic that is pretty huge but only Static Analyzer 
> needs it. For that purpose we'd better leave this in. WDYT? See also D66460.
> 
> Should i ask on the mailing list whether you're willing to sacrifice building 
> clang-tidy without CLANG_ENABLE_STATIC_ANALYZER in order to transition to 
> BugReporter? Cause i thought it was obvious that it's not a great idea, even 
> if it causes me to do a bit of cleanup work on my end.
> 
> That said, i'm surprised that it's deadcode, i.e. that nobody ever dyn_casts 
> bug reporters, even though we already have two bug reporter classes. Maybe we 
> can indeed remove this facility.
> I believe we do want a separation between a {bug report, bug reporter} 
> classes [...]

Yes, the separation is nice.

> For that purpose we'd better leave this in.

`Kind` is only needed for dynamic casting between different bug reporters. I'm 
not sure that is useful in practice (case in point -- the `classof` is not used 
today), specifically because the client that produces diagnostics will need to 
work with a bug reporter of the correct kind. If a path-sensitive client is 
handed a pointer to the base class, `BugReporter`, would it try to `dyn_cast` 
it to the derived class?.. what if it fails?..

Basically, I don't understand why one would want dynamic casting for these 
classes. I totally agree with the separation though.

> Should i ask on the mailing list whether you're willing to sacrifice building 
> clang-tidy without CLANG_ENABLE_STATIC_ANALYZER in order to transition to 
> BugReporter?

I personally don't mind CLANG_ENABLE_STATIC_ANALYZER going away completely (I 
have a fast machine and use a build system with strong caching), however, there 
are other people who are a lot more sensitive to build time, and for whom it 
might be important.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2343
   InterExplodedGraphMap ForwardMap;
-  TrimmedGraph = OriginalGraph->trim(Nodes, &ForwardMap, &InverseMap);
 

NoQ wrote:
> Btw these days we strongly suspect that the whole graph trimming thing is 
> useless and should be removed.
TBH, I don't understand what this code is doing, I was just following the leads 
from dead code analysis :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66473



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

[PATCH] D66473: [analyzer] Removed some dead code in BugReporter and related files

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

Szelethus added a comment.

No way the entire `NodeResolver` is dead code! I spent so much time trying to 
understand it! I mean, now that I think about it, we literally deep copy every 
`ExplodedNode`, so why would we need the mapping to the original, right?

Wow. Thank you so much for clearing this out.




Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2263-2264
 /// A wrapper around an ExplodedGraph that contains a single path from the root
 /// to the error node, and a map that maps the nodes in this path to the ones 
in
 /// the original ExplodedGraph.
 class BugPathInfo {

Let's keep the comment up to date as well :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66473



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

[PATCH] D66473: [analyzer] Removed some dead code in BugReporter and related files

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

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2343
   InterExplodedGraphMap ForwardMap;
-  TrimmedGraph = OriginalGraph->trim(Nodes, &ForwardMap, &InverseMap);
 

gribozavr wrote:
> NoQ wrote:
> > Btw these days we strongly suspect that the whole graph trimming thing is 
> > useless and should be removed.
> TBH, I don't understand what this code is doing, I was just following the 
> leads from dead code analysis :)
TL;DR: When creating a linear path from the root of the `ExplodedGraph` to a 
given error node (a point at which a bug was emitted), we first trim be graph 
of all nodes that do not lead to an error node, and then create the path from 
that, instead of skipping the entire trimming process.

This isn't that simple (though probably not that difficult either), so feel 
free to leave it as it, the code is already much easier to read!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66473



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

r369427 - [Sema][Typo] Fix assertion failure for expressions with multiple typos

[David Goldman via cfe-commits]

Author: dgoldman
Date: Tue Aug 20 12:03:15 2019
New Revision: 369427

URL: http://llvm.org/viewvc/llvm-project?rev=369427&view=rev
Log:
[Sema][Typo] Fix assertion failure for expressions with multiple typos

Summary:
As Typo Resolution can create new TypoExprs while resolving typos,
it is necessary to recurse through the expression to search for more
typos.

This should fix the assertion failure in `clang::Sema::~Sema()`:
  `DelayedTypos.empty() && "Uncorrected typos!"`

Notes:
- In case some TypoExprs are created but thrown away, Sema
  now has a Vector that is used to keep track of newly created
  typos.
- For expressions with multiple typos, we only give suggestions
  if we are able to resolve all typos in the expression
- This patch is similar to D37521 except that it does not eagerly
  commit to a correction for the first typo in the expression.
  Instead, it will search for corrections which fix all of the
  typos in the expression.

Subscribers: cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/Sema/typo-correction-recursive.cpp
Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=369427&r1=369426&r2=369427&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Aug 20 12:03:15 2019
@@ -405,6 +405,12 @@ public:
   /// Source location for newly created implicit MSInheritanceAttrs
   SourceLocation ImplicitMSInheritanceAttrLoc;
 
+  /// Holds TypoExprs that are created from `createDelayedTypo`. This is used 
by
+  /// `TransformTypos` in order to keep track of any TypoExprs that are created
+  /// recursively during typo correction and wipe them away if the correction
+  /// fails.
+  llvm::SmallVector TypoExprs;
+
   /// pragma clang section kind
   enum PragmaClangSectionKind {
 PCSK_Invalid  = 0,

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=369427&r1=369426&r2=369427&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Aug 20 12:03:15 2019
@@ -7580,15 +7580,22 @@ class TransformTypos : public TreeTransf
   llvm::SmallDenseMap OverloadResolution;
 
   /// Emit diagnostics for all of the TypoExprs encountered.
+  ///
   /// If the TypoExprs were successfully corrected, then the diagnostics should
   /// suggest the corrections. Otherwise the diagnostics will not suggest
   /// anything (having been passed an empty TypoCorrection).
-  void EmitAllDiagnostics() {
+  ///
+  /// If we've failed to correct due to ambiguous corrections, we need to
+  /// be sure to pass empty corrections and replacements. Otherwise it's
+  /// possible that the Consumer has a TypoCorrection that failed to ambiguity
+  /// and we don't want to report those diagnostics.
+  void EmitAllDiagnostics(bool IsAmbiguous) {
 for (TypoExpr *TE : TypoExprs) {
   auto &State = SemaRef.getTypoExprState(TE);
   if (State.DiagHandler) {
-TypoCorrection TC = State.Consumer->getCurrentCorrection();
-ExprResult Replacement = TransformCache[TE];
+TypoCorrection TC = IsAmbiguous
+? TypoCorrection() : State.Consumer->getCurrentCorrection();
+ExprResult Replacement = IsAmbiguous ? ExprError() : 
TransformCache[TE];
 
 // Extract the NamedDecl from the transformed TypoExpr and add it to 
the
 // TypoCorrection, replacing the existing decls. This ensures the right
@@ -7650,6 +7657,145 @@ class TransformTypos : public TreeTransf
 return ExprFilter(Res.get());
   }
 
+  // Since correcting typos may intoduce new TypoExprs, this function
+  // checks for new TypoExprs and recurses if it finds any. Note that it will
+  // only succeed if it is able to correct all typos in the given expression.
+  ExprResult CheckForRecursiveTypos(ExprResult Res, bool &IsAmbiguous) {
+if (Res.isInvalid()) {
+  return Res;
+}
+// Check to see if any new TypoExprs were created. If so, we need to 
recurse
+// to check their validity.
+Expr *FixedExpr = Res.get();
+
+auto SavedTypoExprs = std::move(TypoExprs);
+auto SavedAmbiguousTypoExprs = std::move(AmbiguousTypoExprs);
+TypoExprs.clear();
+AmbiguousTypoExprs.clear();
+
+FindTypoExprs(TypoExprs).TraverseStmt(FixedExpr);
+if (!TypoExprs.empty()) {
+  // Recurse to handle newly created TypoExprs. If we're not able to
+  // handle them, discard these TypoExprs.
+  ExprResult RecurResult =
+  RecursiveTransformLoop(FixedExpr, IsAmbiguous);
+  if (RecurResult.isInvalid()) {
+

[PATCH] D44672: [CodeGen] Disable UBSan for coroutine functions

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

It seems that this test leads to an `UNREACHABLE` under the new pass manager 
(can check this by adding `-fexperimental-new-pass-manager` to the test. I 
think this is because the passes for lowering the `llvm.coro` intrinsics are 
not yet ported to the new PM. Would it be fine to add 
`-fno-experimental-new-pass-manager` to the test to indicate it should be run 
under the legacy PM only? Thanks.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D44672



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

[PATCH] D62648: [Sema][Typo] Fix assertion failure for expressions with multiple typos

[David Goldman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369427: [Sema][Typo] Fix assertion failure for expressions 
with multiple typos (authored by dgoldman, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62648

Files:
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Sema/SemaExprCXX.cpp
  cfe/trunk/lib/Sema/SemaLookup.cpp
  cfe/trunk/test/Sema/typo-correction-recursive.cpp

Index: cfe/trunk/test/Sema/typo-correction-recursive.cpp
===
--- cfe/trunk/test/Sema/typo-correction-recursive.cpp
+++ cfe/trunk/test/Sema/typo-correction-recursive.cpp
@@ -0,0 +1,120 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// Check the following typo correction behavior:
+// - multiple typos in a single member call chain are all diagnosed
+// - no typos are diagnosed for multiple typos in an expression when not all
+//   typos can be corrected
+
+class DeepClass
+{
+public:
+  void trigger() const;  // expected-note {{'trigger' declared here}}
+};
+
+class Y
+{
+public:
+  const DeepClass& getX() const { return m_deepInstance; }  // expected-note {{'getX' declared here}}
+private:
+  DeepClass m_deepInstance;
+  int m_n;
+};
+
+class Z
+{
+public:
+  const Y& getY0() const { return m_y0; }  // expected-note {{'getY0' declared here}}
+  const Y& getActiveY() const { return m_y0; }
+
+private:
+  Y m_y0;
+  Y m_y1;
+};
+
+Z z_obj;
+
+void testMultipleCorrections()
+{
+  z_obj.getY2().  // expected-error {{no member named 'getY2' in 'Z'; did you mean 'getY0'}}
+  getM(). // expected-error {{no member named 'getM' in 'Y'; did you mean 'getX'}}
+  triggee();  // expected-error {{no member named 'triggee' in 'DeepClass'; did you mean 'trigger'}}
+}
+
+void testNoCorrections()
+{
+  z_obj.getY2().  // expected-error {{no member named 'getY2' in 'Z'}}
+  getM().
+  thisDoesntSeemToMakeSense();
+}
+
+struct C {};
+struct D { int value; };
+struct A {
+  C get_me_a_C();
+};
+struct B {
+  D get_me_a_D();  // expected-note {{'get_me_a_D' declared here}}
+};
+class Scope {
+public:
+  A make_an_A();
+  B make_a_B();  // expected-note {{'make_a_B' declared here}}
+};
+
+Scope scope_obj;
+
+int testDiscardedCorrections() {
+  return scope_obj.make_an_E().  // expected-error {{no member named 'make_an_E' in 'Scope'; did you mean 'make_a_B'}}
+  get_me_a_Z().value;// expected-error {{no member named 'get_me_a_Z' in 'B'; did you mean 'get_me_a_D'}}
+}
+
+class AmbiguousHelper {
+public:
+  int helpMe();
+  int helpBe();
+};
+class Ambiguous {
+public:
+  int calculateA();
+  int calculateB();
+
+  AmbiguousHelper getHelp1();
+  AmbiguousHelper getHelp2();
+};
+
+Ambiguous ambiguous_obj;
+
+int testDirectAmbiguousCorrection() {
+  return ambiguous_obj.calculateZ();  // expected-error {{no member named 'calculateZ' in 'Ambiguous'}}
+}
+
+int testRecursiveAmbiguousCorrection() {
+  return ambiguous_obj.getHelp3().// expected-error {{no member named 'getHelp3' in 'Ambiguous'}}
+  helpCe();
+}
+
+
+class DeepAmbiguityHelper {
+public:
+  DeepAmbiguityHelper& help1();
+  DeepAmbiguityHelper& help2();
+
+  DeepAmbiguityHelper& methodA();
+  DeepAmbiguityHelper& somethingMethodB();
+  DeepAmbiguityHelper& functionC();
+  DeepAmbiguityHelper& deepMethodD();
+  DeepAmbiguityHelper& asDeepAsItGets();
+};
+
+DeepAmbiguityHelper deep_obj;
+
+int testDeepAmbiguity() {
+  deep_obj.
+  methodB(). // expected-error {{no member named 'methodB' in 'DeepAmbiguityHelper'}}
+  somethingMethodC().
+  functionD().
+  deepMethodD().
+  help3().
+  asDeepASItGet().
+  functionE();
+}
Index: cfe/trunk/lib/Sema/SemaLookup.cpp
===
--- cfe/trunk/lib/Sema/SemaLookup.cpp
+++ cfe/trunk/lib/Sema/SemaLookup.cpp
@@ -5434,6 +5434,8 @@
   State.Consumer = std::move(TCC);
   State.DiagHandler = std::move(TDG);
   State.RecoveryHandler = std::move(TRC);
+  if (TE)
+TypoExprs.push_back(TE);
   return TE;
 }
 
Index: cfe/trunk/lib/Sema/SemaExprCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp
@@ -7580,15 +7580,22 @@
   llvm::SmallDenseMap OverloadResolution;
 
   /// Emit diagnostics for all of the TypoExprs encountered.
+  ///
   /// If the TypoExprs were successfully corrected, then the diagnostics should
   /// suggest the corrections. Otherwise the diagnostics will not suggest
   /// anything (having been passed an empty TypoCorrection).
-  void EmitAllDiagnostics() {
+  ///
+  /// If we've failed to correct due to ambiguous corrections, we need to
+  /// be sure to pass empty corrections and replacements. Otherwise it's
+  /// possible that the Consumer has a TypoCorrection that fail

[PATCH] D66490: [NewPM] Enable the New Pass Manager by Default in Clang

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: chandlerc, echristo, hfinkel, lattner, rupprecht.
leonardchan added a project: clang.
Herald added subscribers: dexonsmith, mehdi_amini, mgorny.

The new PM serves as a replacement for the legacy PM, and promises better 
codegen, better inlining, faster build times, and better PGO and LTO.  Now that 
LLVM 9.0.0 has branched, we have some time before the next release to work out 
any kinks that may arise from the switch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66490

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -233,7 +233,7 @@
 set(ENABLE_X86_RELAX_RELOCATIONS OFF CACHE BOOL
 "enable x86 relax relocations by default")
 
-set(ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER FALSE CACHE BOOL
+set(ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER TRUE CACHE BOOL
   "Enable the experimental new pass manager by default.")
 
 # TODO: verify the values against LangStandards.def?


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -233,7 +233,7 @@
 set(ENABLE_X86_RELAX_RELOCATIONS OFF CACHE BOOL
 "enable x86 relax relocations by default")
 
-set(ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER FALSE CACHE BOOL
+set(ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER TRUE CACHE BOOL
   "Enable the experimental new pass manager by default.")
 
 # TODO: verify the values against LangStandards.def?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66490: [NewPM] Enable the New Pass Manager by Default in Clang

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

As of now, from running `check-llvm` and `check-clang` there's 2 failing tests, 
but I'm trying to get those addressed.

- `Clang :: CodeGen/split-lto-unit.c` which should be resolved by D66488 

- `Clang :: CodeGenCXX/ubsan-coroutines.cpp` which was introduced by D44672 
 but I think should be fixed by just disabling 
the test under the new PM since coroutines haven't been ported yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66490



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

[PATCH] D65481: NFCI: Simplify SourceManager::translateFile by removing code path that should never be taken

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Ping. I will commit it this week if there are no objections.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65481



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

[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D65575#1621425 , @Szelethus wrote:

> I think it isn't crucial of getting rid of the "The" prefix, if we append ", 
> which participates in a condition later" (which sounds so much better than 
> what I added in this patch), so maybe changing `WillBeUsedForACondition` to 
> that would be good enough.


I guess let's just land that variant! I'm also not super worried about "The" 
here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65575



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

[PATCH] D44672: [CodeGen] Disable UBSan for coroutine functions

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

In D44672#1637891 , @leonardchan wrote:

> It seems that this test leads to an `UNREACHABLE` under the new pass manager 
> (can check this by adding `-fexperimental-new-pass-manager` to the test. I 
> think this is because the passes for lowering the `llvm.coro` intrinsics are 
> not yet ported to the new PM. Would it be fine to add 
> `-fno-experimental-new-pass-manager` to the test to indicate it should be run 
> under the legacy PM only? Thanks.


Sounds reasonable to me. It would be great to include a link to some master PR 
for the new PM porting effort with the test change.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D44672



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

[PATCH] D65209: [analyzer] Fix a SARIF exporter crash with macro expansions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D65209#1606075 , @aaron.ballman 
wrote:

> This LGTM, but I'd appreciate a second reviewer chiming in only because Joe 
> is a coworker.


Sufficient time has passed that any concerns other reviewers have can be 
addressed post-commit. Continues to LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65209



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

[PATCH] D65211: [analyzer] Update the SARIF exporter to SARIF 2.1

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D65211#1606089 , @aaron.ballman 
wrote:

> This LGTM, but I'd appreciate a second reviewer chiming in only because Joe 
> is a coworker.


Sufficient time has passed that any concerns other reviewers have can be 
addressed post-commit. Continues to LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65211



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

[PATCH] D65206: [analyzer] Fix text range end columns in SARIF to be exclusive

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D65206#1606072 , @aaron.ballman 
wrote:

> This LGTM, but I'd appreciate a second reviewer chiming in only because Joe 
> is a coworker.


Sufficient time has passed that any concerns other reviewers have can be 
addressed post-commit. Continues to LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65206



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

[PATCH] D66492: [Clang][CodeGen] set alias linkage on QualType

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added a reviewer: rsmith.
Herald added subscribers: cfe-commits, kristof.beyls, javed.absar.
Herald added a project: clang.

It seems that CodeGen was always using ExternalLinkage when emitting a
GlobalDecl with __attribute__((alias)). This leads to symbol
redefinitions (ODR) that cause failures at link time for static aliases.
This is readily attempting to link an ARM (32b) allyesconfig Linux
kernel built with Clang.

Reported-by: nathanchance
Suggested-by: ihalip
Link: https://github.com/ClangBuiltLinux/linux/issues/631


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66492

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/alias.c


Index: clang/test/CodeGen/alias.c
===
--- clang/test/CodeGen/alias.c
+++ clang/test/CodeGen/alias.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -triple i386-pc-linux-gnu -emit-llvm -o - %s | FileCheck 
-check-prefix=CHECKBASIC %s
 // RUN: %clang_cc1 -triple armv7a-eabi -mfloat-abi hard -emit-llvm -o - %s | 
FileCheck -check-prefix=CHECKCC %s
 // RUN: %clang_cc1 -triple armv7a-eabi -mfloat-abi hard -S -o - %s | FileCheck 
-check-prefix=CHECKASM %s
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefix=CHECKGLOBALS %s
 
 int g0;
 // CHECKBASIC-DAG: @g0 = common global i32 0
@@ -88,3 +89,9 @@
 void test9_bar(void) { }
 void test9_zed(void) __attribute__((section("test")));
 void test9_zed(void) __attribute__((alias("test9_bar")));
+
+// Test that the alias gets its linkage from its declared qual type.
+// CHECKGLOBALS: @test10_foo = internal
+// CHECKGLOBALS-NOT: @test10_foo = dso_local
+int test10;
+static int test10_foo __attribute__((alias("test10")));
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4363,17 +4363,22 @@
   // Create a reference to the named value.  This ensures that it is emitted
   // if a deferred decl.
   llvm::Constant *Aliasee;
-  if (isa(DeclTy))
+  llvm::GlobalValue::LinkageTypes LT;
+  if (isa(DeclTy)) {
 Aliasee = GetOrCreateLLVMFunction(AA->getAliasee(), DeclTy, GD,
   /*ForVTable=*/false);
-  else
+LT = getFunctionLinkage(GD);
+  } else {
 Aliasee = GetOrCreateLLVMGlobal(AA->getAliasee(),
 llvm::PointerType::getUnqual(DeclTy),
 /*D=*/nullptr);
+LT = getLLVMLinkageVarDefinition(cast(GD.getDecl()),
+ D->getType().isConstQualified());
+  }
 
   // Create the new alias itself, but don't set a name yet.
-  auto *GA = llvm::GlobalAlias::create(
-  DeclTy, 0, llvm::Function::ExternalLinkage, "", Aliasee, &getModule());
+  auto *GA =
+  llvm::GlobalAlias::create(DeclTy, 0, LT, "", Aliasee, &getModule());
 
   if (Entry) {
 if (GA->getAliasee() == Entry) {


Index: clang/test/CodeGen/alias.c
===
--- clang/test/CodeGen/alias.c
+++ clang/test/CodeGen/alias.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -triple i386-pc-linux-gnu -emit-llvm -o - %s | FileCheck -check-prefix=CHECKBASIC %s
 // RUN: %clang_cc1 -triple armv7a-eabi -mfloat-abi hard -emit-llvm -o - %s | FileCheck -check-prefix=CHECKCC %s
 // RUN: %clang_cc1 -triple armv7a-eabi -mfloat-abi hard -S -o - %s | FileCheck -check-prefix=CHECKASM %s
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefix=CHECKGLOBALS %s
 
 int g0;
 // CHECKBASIC-DAG: @g0 = common global i32 0
@@ -88,3 +89,9 @@
 void test9_bar(void) { }
 void test9_zed(void) __attribute__((section("test")));
 void test9_zed(void) __attribute__((alias("test9_bar")));
+
+// Test that the alias gets its linkage from its declared qual type.
+// CHECKGLOBALS: @test10_foo = internal
+// CHECKGLOBALS-NOT: @test10_foo = dso_local
+int test10;
+static int test10_foo __attribute__((alias("test10")));
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4363,17 +4363,22 @@
   // Create a reference to the named value.  This ensures that it is emitted
   // if a deferred decl.
   llvm::Constant *Aliasee;
-  if (isa(DeclTy))
+  llvm::GlobalValue::LinkageTypes LT;
+  if (isa(DeclTy)) {
 Aliasee = GetOrCreateLLVMFunction(AA->getAliasee(), DeclTy, GD,
   /*ForVTable=*/false);
-  else
+LT = getFunctionLinkage(GD);
+  } else {
 Aliasee = GetOrCreateLLVMGlobal(AA->getAliasee(),
 llvm::PointerType::getUnqual(DeclTy),
 /*D=*/nullptr);
+LT = getLLVMLinkageVarDefinition(cast(GD.getDecl()),
+ D->getType().isConstQualified());
+  }
 
   // Crea

r369432 - [OPENMP]Fix delayed diagnostics for standalone declare target directive.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Aug 20 12:50:13 2019
New Revision: 369432

URL: http://llvm.org/viewvc/llvm-project?rev=369432&view=rev
Log:
[OPENMP]Fix delayed diagnostics for standalone declare target directive.

If the function is marked as declare target in a standalone directive,
the delayed diagnostics is not emitted. Patch fixes this problem.

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/nvptx_asm_delayed_diags.c
cfe/trunk/test/OpenMP/nvptx_va_arg_delayed_diags.c

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=369432&r1=369431&r2=369432&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Aug 20 12:50:13 2019
@@ -8997,7 +8997,8 @@ private:
   void popOpenMPFunctionRegion(const sema::FunctionScopeInfo *OldFSI);
 
   /// Check whether we're allowed to call Callee from the current function.
-  void checkOpenMPDeviceFunction(SourceLocation Loc, FunctionDecl *Callee);
+  void checkOpenMPDeviceFunction(SourceLocation Loc, FunctionDecl *Callee,
+ bool CheckForDelayedContext = true);
 
   /// Check if the expression is allowed to be used in expressions for the
   /// OpenMP devices.

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=369432&r1=369431&r2=369432&view=diff
==
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Tue Aug 20 12:50:13 2019
@@ -1383,7 +1383,7 @@ static void emitCallStackNotes(Sema &S,
 
 // Emit any deferred diagnostics for FD and erase them from the map in which
 // they're stored.
-static void emitDeferredDiags(Sema &S, FunctionDecl *FD) {
+static void emitDeferredDiags(Sema &S, FunctionDecl *FD, bool ShowCallStack) {
   auto It = S.DeviceDeferredDiags.find(FD);
   if (It == S.DeviceDeferredDiags.end())
 return;
@@ -1402,7 +1402,7 @@ static void emitDeferredDiags(Sema &S, F
   // FIXME: Should this be called after every warning/error emitted in the loop
   // above, instead of just once per function?  That would be consistent with
   // how we handle immediate errors, but it also seems like a bit much.
-  if (HasWarningOrError)
+  if (HasWarningOrError && ShowCallStack)
 emitCallStackNotes(S, FD);
 }
 
@@ -1505,7 +1505,7 @@ void Sema::markKnownEmitted(
 assert(!IsKnownEmitted(S, C.Callee) &&
"Worklist should not contain known-emitted functions.");
 S.DeviceKnownEmittedFns[C.Callee] = {C.Caller, C.Loc};
-emitDeferredDiags(S, C.Callee);
+emitDeferredDiags(S, C.Callee, C.Caller);
 
 // If this is a template instantiation, explore its callgraph as well:
 // Non-dependent calls are part of the template's callgraph, while 
dependent

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=369432&r1=369431&r2=369432&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Tue Aug 20 12:50:13 2019
@@ -1576,7 +1576,8 @@ Sema::DeviceDiagBuilder Sema::diagIfOpen
Loc, DiagID, getCurFunctionDecl(), *this);
 }
 
-void Sema::checkOpenMPDeviceFunction(SourceLocation Loc, FunctionDecl *Callee) 
{
+void Sema::checkOpenMPDeviceFunction(SourceLocation Loc, FunctionDecl *Callee,
+ bool CheckForDelayedContext) {
   assert(LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
  "Expected OpenMP device compilation.");
   assert(Callee && "Callee may not be null.");
@@ -1584,9 +1585,13 @@ void Sema::checkOpenMPDeviceFunction(Sou
 
   // If the caller is known-emitted, mark the callee as known-emitted.
   // Otherwise, mark the call in our call graph so we can traverse it later.
-  if (!isOpenMPDeviceDelayedContext(*this) ||
+  if ((CheckForDelayedContext && !isOpenMPDeviceDelayedContext(*this)) ||
+  (!Caller && !CheckForDelayedContext) ||
   (Caller && isKnownEmitted(*this, Caller)))
-markKnownEmitted(*this, Caller, Callee, Loc, isKnownEmitted);
+markKnownEmitted(*this, Caller, Callee, Loc,
+ [CheckForDelayedContext](Sema &S, FunctionDecl *FD) {
+   return CheckForDelayedContext && isKnownEmitted(S, FD);
+ });
   else if (Caller)
 DeviceCallGraph[Caller].insert({Callee, Loc});
 }
@@ -15406,15 +15411,17 @@ void Sema::checkDeclIsAllowedInOpenMPTar
   }
   if (const auto *FTD = dyn_cast(D))
 D = FTD->getTemplatedDecl();
-  if (const auto *FD = dyn_cast(D)) {
+  if (auto *FD = dyn_cast(D)) {
 llvm::Optional Res =
 OMPDeclareTargetD

[PATCH] D66364: Diagnose use of _Thread_local as an extension when appropriate

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

@rsmith are you fine with implementing the diagnostic for these keywords 
piecemeal based on the pattern from this patch, or do you want to see an 
omnibus patch that adds all of them at once?

In D66364#1637635 , @ldionne wrote:

> Please ping me directly if you expect libc++ maintainers to do something 
> following this patch.


Will do, thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D66364



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

[PATCH] D66493: [NewPM] Run ubsan-coroutines test under the legacy pass manager only

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: vsk, modocache.
leonardchan added a project: clang.
Herald added a subscriber: EricWF.

The passes that lower the `llvm.coro.*` instrinsics have not yet been ported, 
so only run under the legacy PM for now.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66493

Files:
  clang/test/CodeGenCXX/ubsan-coroutines.cpp


Index: clang/test/CodeGenCXX/ubsan-coroutines.cpp
===
--- clang/test/CodeGenCXX/ubsan-coroutines.cpp
+++ clang/test/CodeGenCXX/ubsan-coroutines.cpp
@@ -1,6 +1,8 @@
 // This test merely verifies that emitting the object file does not cause a
 // crash when the LLVM coroutines passes are run.
-// RUN: %clang_cc1 -emit-obj -std=c++2a -fsanitize=null %s -o %t.o
+// PR42867: Disable this test for the new PM since the passes that lower the
+// llvm.coro.* intrinsics have not yet been ported.
+// RUN: %clang_cc1 -fno-experimental-new-pass-manager -emit-obj -std=c++2a 
-fsanitize=null %s -o %t.o
 
 namespace std::experimental {
 template  struct coroutine_traits {


Index: clang/test/CodeGenCXX/ubsan-coroutines.cpp
===
--- clang/test/CodeGenCXX/ubsan-coroutines.cpp
+++ clang/test/CodeGenCXX/ubsan-coroutines.cpp
@@ -1,6 +1,8 @@
 // This test merely verifies that emitting the object file does not cause a
 // crash when the LLVM coroutines passes are run.
-// RUN: %clang_cc1 -emit-obj -std=c++2a -fsanitize=null %s -o %t.o
+// PR42867: Disable this test for the new PM since the passes that lower the
+// llvm.coro.* intrinsics have not yet been ported.
+// RUN: %clang_cc1 -fno-experimental-new-pass-manager -emit-obj -std=c++2a -fsanitize=null %s -o %t.o
 
 namespace std::experimental {
 template  struct coroutine_traits {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44672: [CodeGen] Disable UBSan for coroutine functions

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D44672#1637984 , @vsk wrote:

> In D44672#1637891 , @leonardchan 
> wrote:
>
> > It seems that this test leads to an `UNREACHABLE` under the new pass 
> > manager (can check this by adding `-fexperimental-new-pass-manager` to the 
> > test. I think this is because the passes for lowering the `llvm.coro` 
> > intrinsics are not yet ported to the new PM. Would it be fine to add 
> > `-fno-experimental-new-pass-manager` to the test to indicate it should be 
> > run under the legacy PM only? Thanks.
>
>
> Sounds reasonable to me. It would be great to include a link to some master 
> PR for the new PM porting effort with the test change.


Thanks. I added you as a reviewer to D66493  
and linked in https://bugs.llvm.org/show_bug.cgi?id=42867 which I filed a while 
back and has the same root problem.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D44672



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

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp:14-20
 const char * const CoreFoundationObjectiveC = "Core Foundation/Objective-C";
 const char * const LogicError = "Logic error";
 const char * const MemoryRefCount =
   "Memory (Core Foundation/Objective-C/OSObject)";
 const char * const MemoryError = "Memory error";
 const char * const UnixAPI = "Unix API";
+const char * const CXXObjectLifecycle = "C++ object lifecycle";

Szelethus wrote:
> `static constexpr StringLiteral`? Why is there a need to have a header and a 
> cpp file for this? Not that it matters much (definitely not in the context of 
> this patch), but compile-time stuff is cool.
Ancient stuff, i guess. We only have C++11 for like 3 years i think.


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

https://reviews.llvm.org/D64274



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

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D62899#1634657 , @Szelethus wrote:

> In D62899#1551312 , @NoQ wrote:
>
> > In D62899#1549715 , @Szelethus 
> > wrote:
> >
> > > Added a proper testfile. The only downside of it is that it doesn't test 
> > > anything.
> >
> >
> > Use creduce!
>
>
> I would, but I'm not even sure what to look for, really.


When you already have code and have a real-world example that it works but you 
don't have a reduced test case, just use "anything changes" as the reduce 
condition, because you're basically interested in the answer to "why does this 
patch change anything at all?".


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

https://reviews.llvm.org/D62899



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